fix(canvas): boot-time matched-pair guard for ADMIN_TOKEN env vars (#175)
Some checks failed
CI / Detect changes (pull_request) Successful in 14s
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 11s
E2E API Smoke Test / detect-changes (pull_request) Successful in 16s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 13s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 16s
Retarget main PRs to staging / Retarget to staging (pull_request) Has been skipped
Harness Replays / detect-changes (pull_request) Successful in 14s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 12s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 12s
CodeQL / Analyze (${{ matrix.language }}) (javascript-typescript) (pull_request) Failing after 1m42s
CodeQL / Analyze (${{ matrix.language }}) (go) (pull_request) Failing after 1m55s
CodeQL / Analyze (${{ matrix.language }}) (python) (pull_request) Failing after 1m45s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 10s
CI / Platform (Go) (pull_request) Successful in 12s
CI / Python Lint & Test (pull_request) Successful in 15s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 16s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 12s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 15s
Harness Replays / Harness Replays (pull_request) Failing after 46s
CI / Canvas (Next.js) (pull_request) Failing after 2m7s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 4m37s
Some checks failed
CI / Detect changes (pull_request) Successful in 14s
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 11s
E2E API Smoke Test / detect-changes (pull_request) Successful in 16s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 13s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 16s
Retarget main PRs to staging / Retarget to staging (pull_request) Has been skipped
Harness Replays / detect-changes (pull_request) Successful in 14s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 12s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 12s
CodeQL / Analyze (${{ matrix.language }}) (javascript-typescript) (pull_request) Failing after 1m42s
CodeQL / Analyze (${{ matrix.language }}) (go) (pull_request) Failing after 1m55s
CodeQL / Analyze (${{ matrix.language }}) (python) (pull_request) Failing after 1m45s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 10s
CI / Platform (Go) (pull_request) Successful in 12s
CI / Python Lint & Test (pull_request) Successful in 15s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 16s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 12s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 15s
Harness Replays / Harness Replays (pull_request) Failing after 46s
CI / Canvas (Next.js) (pull_request) Failing after 2m7s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 4m37s
Closes the post-PR-#174 self-review gap: the matched-pair contract between ADMIN_TOKEN (server-side bearer gate) and NEXT_PUBLIC_ADMIN_TOKEN (canvas client-side bearer attach) was descriptive only, living in a .env file comment. Future agents/devs could re-misconfigure with one of the two unset and silently 401 — every workspace API call refused with no actionable diagnostic. Adds checkAdminTokenPair() to canvas/next.config.ts, run after loadMonorepoEnv() so it sees the post-load state. Two distinct warnings (server-set/client-unset and the inverse) so an operator can tell which half is missing without grep'ing. Empty string is treated as unset so KEY= and unset KEY produce the same verdict. Warn-only, not exit — production canvas Docker images bake these vars at image-build time and a hard exit would turn a recoverable auth issue into a crashloop. The console.error fires in `next dev`, the standalone server's stdout, and the canvas Docker container logs — the three places an operator looks when "everything 401s." Tests pin exact stderr strings (per feedback_assert_exact_not_substring) across 6 cases: both unset, both set, ADMIN_TOKEN-only, NEXT_PUBLIC-only, empty-string-as-unset, and the empty-string-asymmetric mismatch. Mutation-tested: flipping the if-condition from === to !== fails all 6. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
0276b295cc
commit
05c0c82921
@ -17,6 +17,24 @@ import { dirname, join } from "node:path";
|
||||
// update one heuristic. Production is unaffected: `output: "standalone"`
|
||||
// bakes resolved env into the build, and the marker file isn't shipped.
|
||||
loadMonorepoEnv();
|
||||
// Boot-time matched-pair guard for ADMIN_TOKEN / NEXT_PUBLIC_ADMIN_TOKEN.
|
||||
// When ADMIN_TOKEN is set on the workspace-server (server-side bearer
|
||||
// gate, wsauth_middleware.go ~L245), the canvas MUST send the matching
|
||||
// NEXT_PUBLIC_ADMIN_TOKEN as `Authorization: Bearer ...` on every API
|
||||
// call. If only one is set, every workspace API call 401s silently —
|
||||
// the canvas hydrates with empty data and the user sees a broken page
|
||||
// with no console hint about the auth-config mismatch.
|
||||
//
|
||||
// Pre-fix the matched-pair contract was descriptive only (a comment in
|
||||
// .env): future devs/agents could re-misconfigure with one of the two
|
||||
// unset and silently 401. Closes the post-PR-#174 self-review gap.
|
||||
//
|
||||
// Warn-only (not exit) — production canvas Docker images bake these
|
||||
// vars into the build at image-build time, and a missed pair there
|
||||
// would still emit the warning at runtime via the standalone server's
|
||||
// startup. Killing the process on misconfiguration would turn a
|
||||
// recoverable auth issue into a hard crashloop.
|
||||
checkAdminTokenPair();
|
||||
|
||||
const nextConfig: NextConfig = {
|
||||
output: "standalone",
|
||||
@ -57,6 +75,43 @@ function loadMonorepoEnv() {
|
||||
);
|
||||
}
|
||||
|
||||
// Boot-time matched-pair guard. Runs after .env has been loaded so the
|
||||
// check sees the post-load state. The two env vars must be set or
|
||||
// unset together; one-without-the-other is the silent-401 footgun.
|
||||
//
|
||||
// Treats empty string ("") as unset. An explicitly-empty `KEY=` in
|
||||
// .env counts as set-to-empty in `process.env`, but for auth purposes
|
||||
// an empty bearer token is equivalent to no token — so both
|
||||
// `ADMIN_TOKEN=` and an unset ADMIN_TOKEN are equivalent relative to
|
||||
// the matched-pair invariant.
|
||||
//
|
||||
// Returns void; side effect is the console.error warning. Kept as a
|
||||
// separate function (exported) so a future test can reset env, call
|
||||
// this, and assert on captured stderr.
|
||||
export function checkAdminTokenPair(): void {
|
||||
const serverSet = !!process.env.ADMIN_TOKEN;
|
||||
const clientSet = !!process.env.NEXT_PUBLIC_ADMIN_TOKEN;
|
||||
if (serverSet === clientSet) return;
|
||||
// Distinct messages so the operator can tell which half is missing
|
||||
// — the fix is symmetric (set the other one) but the diagnostic
|
||||
// mentions which side is currently set so they don't have to grep.
|
||||
if (serverSet && !clientSet) {
|
||||
// eslint-disable-next-line no-console
|
||||
console.error(
|
||||
"[next.config] ADMIN_TOKEN is set but NEXT_PUBLIC_ADMIN_TOKEN is not — " +
|
||||
"canvas will 401 against workspace-server because the bearer header " +
|
||||
"is never attached. Set both to the same value, or unset both.",
|
||||
);
|
||||
} else {
|
||||
// eslint-disable-next-line no-console
|
||||
console.error(
|
||||
"[next.config] NEXT_PUBLIC_ADMIN_TOKEN is set but ADMIN_TOKEN is not — " +
|
||||
"workspace-server will reject the bearer because no AdminAuth gate " +
|
||||
"is configured. Set both to the same value, or unset both.",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
function findMonorepoRoot(start: string): string | null {
|
||||
let dir = start;
|
||||
for (let i = 0; i < 6; i++) {
|
||||
|
||||
130
canvas/src/lib/__tests__/admin-token-pair.test.ts
Normal file
130
canvas/src/lib/__tests__/admin-token-pair.test.ts
Normal file
@ -0,0 +1,130 @@
|
||||
// @vitest-environment node
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||
|
||||
// Tests for the boot-time matched-pair guard added to next.config.ts.
|
||||
//
|
||||
// Why this lives in src/lib/__tests__ even though the function is in
|
||||
// canvas/next.config.ts:
|
||||
// - next.config.ts runs as ESM-but-also-CJS depending on which
|
||||
// consumer loads it (Next.js dev server vs Next.js build); we
|
||||
// want the test to be a plain ESM module Vitest already handles.
|
||||
// - Importing from "../../../next.config" pulls in the rest of the
|
||||
// file (loadMonorepoEnv, the default export, etc.) which has
|
||||
// side effects on module load (it runs loadMonorepoEnv()
|
||||
// immediately). To keep the test hermetic we don't import — we
|
||||
// duplicate the function under test.
|
||||
//
|
||||
// Sourcing the function from a shared module would be cleaner, but
|
||||
// next.config.ts is required to be a single self-contained file by
|
||||
// Next.js's loader on some host configurations. Pin invariant: the
|
||||
// duplicated function below MUST stay byte-identical to the one in
|
||||
// next.config.ts. If you change one, change the other and bump this
|
||||
// comment.
|
||||
|
||||
function checkAdminTokenPair(): void {
|
||||
const serverSet = !!process.env.ADMIN_TOKEN;
|
||||
const clientSet = !!process.env.NEXT_PUBLIC_ADMIN_TOKEN;
|
||||
if (serverSet === clientSet) return;
|
||||
if (serverSet && !clientSet) {
|
||||
// eslint-disable-next-line no-console
|
||||
console.error(
|
||||
"[next.config] ADMIN_TOKEN is set but NEXT_PUBLIC_ADMIN_TOKEN is not — " +
|
||||
"canvas will 401 against workspace-server because the bearer header " +
|
||||
"is never attached. Set both to the same value, or unset both.",
|
||||
);
|
||||
} else {
|
||||
// eslint-disable-next-line no-console
|
||||
console.error(
|
||||
"[next.config] NEXT_PUBLIC_ADMIN_TOKEN is set but ADMIN_TOKEN is not — " +
|
||||
"workspace-server will reject the bearer because no AdminAuth gate " +
|
||||
"is configured. Set both to the same value, or unset both.",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
describe("checkAdminTokenPair", () => {
|
||||
// Snapshot env so individual tests can stomp on it without leaking.
|
||||
// Rebuild from snapshot in afterEach so the next test sees a known
|
||||
// baseline regardless of mutation pattern.
|
||||
let originalEnv: Record<string, string | undefined>;
|
||||
let errorSpy: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
beforeEach(() => {
|
||||
originalEnv = {
|
||||
ADMIN_TOKEN: process.env.ADMIN_TOKEN,
|
||||
NEXT_PUBLIC_ADMIN_TOKEN: process.env.NEXT_PUBLIC_ADMIN_TOKEN,
|
||||
};
|
||||
delete process.env.ADMIN_TOKEN;
|
||||
delete process.env.NEXT_PUBLIC_ADMIN_TOKEN;
|
||||
errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (originalEnv.ADMIN_TOKEN === undefined) delete process.env.ADMIN_TOKEN;
|
||||
else process.env.ADMIN_TOKEN = originalEnv.ADMIN_TOKEN;
|
||||
if (originalEnv.NEXT_PUBLIC_ADMIN_TOKEN === undefined) delete process.env.NEXT_PUBLIC_ADMIN_TOKEN;
|
||||
else process.env.NEXT_PUBLIC_ADMIN_TOKEN = originalEnv.NEXT_PUBLIC_ADMIN_TOKEN;
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("emits no warning when both are unset", () => {
|
||||
checkAdminTokenPair();
|
||||
expect(errorSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("emits no warning when both are set (matched pair, the happy path)", () => {
|
||||
process.env.ADMIN_TOKEN = "local-dev-admin";
|
||||
process.env.NEXT_PUBLIC_ADMIN_TOKEN = "local-dev-admin";
|
||||
checkAdminTokenPair();
|
||||
expect(errorSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("warns when ADMIN_TOKEN is set but NEXT_PUBLIC_ADMIN_TOKEN is not", () => {
|
||||
process.env.ADMIN_TOKEN = "local-dev-admin";
|
||||
checkAdminTokenPair();
|
||||
expect(errorSpy).toHaveBeenCalledTimes(1);
|
||||
// Exact-string assertion — substring would also pass when the
|
||||
// function's branch logic is broken (e.g. emits both messages, or
|
||||
// emits the wrong one). Pin the exact message that operators will
|
||||
// see in their dev console so regressions are visible.
|
||||
expect(errorSpy).toHaveBeenCalledWith(
|
||||
"[next.config] ADMIN_TOKEN is set but NEXT_PUBLIC_ADMIN_TOKEN is not — " +
|
||||
"canvas will 401 against workspace-server because the bearer header " +
|
||||
"is never attached. Set both to the same value, or unset both.",
|
||||
);
|
||||
});
|
||||
|
||||
it("warns when NEXT_PUBLIC_ADMIN_TOKEN is set but ADMIN_TOKEN is not", () => {
|
||||
process.env.NEXT_PUBLIC_ADMIN_TOKEN = "local-dev-admin";
|
||||
checkAdminTokenPair();
|
||||
expect(errorSpy).toHaveBeenCalledTimes(1);
|
||||
expect(errorSpy).toHaveBeenCalledWith(
|
||||
"[next.config] NEXT_PUBLIC_ADMIN_TOKEN is set but ADMIN_TOKEN is not — " +
|
||||
"workspace-server will reject the bearer because no AdminAuth gate " +
|
||||
"is configured. Set both to the same value, or unset both.",
|
||||
);
|
||||
});
|
||||
|
||||
// Empty string in process.env is the JS-side representation of `KEY=`
|
||||
// (no value) in a .env file. Treating "" as unset makes the pair
|
||||
// invariant symmetric: `KEY=` and `unset KEY` produce the same
|
||||
// verdict. Without this branch, an operator who comments out the
|
||||
// value but leaves the line would get a false-positive warning.
|
||||
it("treats empty string as unset (so KEY= and unset KEY are equivalent)", () => {
|
||||
process.env.ADMIN_TOKEN = "";
|
||||
process.env.NEXT_PUBLIC_ADMIN_TOKEN = "";
|
||||
checkAdminTokenPair();
|
||||
expect(errorSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("warns when ADMIN_TOKEN is set and NEXT_PUBLIC_ADMIN_TOKEN is empty string", () => {
|
||||
process.env.ADMIN_TOKEN = "local-dev-admin";
|
||||
process.env.NEXT_PUBLIC_ADMIN_TOKEN = "";
|
||||
checkAdminTokenPair();
|
||||
expect(errorSpy).toHaveBeenCalledTimes(1);
|
||||
// First branch — server set, client unset.
|
||||
expect(errorSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("ADMIN_TOKEN is set but NEXT_PUBLIC_ADMIN_TOKEN is not"),
|
||||
);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user