fix(canvas): boot-time matched-pair guard for ADMIN_TOKEN env vars (#175)
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
a2970db8ed
commit
a6477d2b0c
@ -17,6 +17,24 @@ import { dirname, join } from "node:path";
|
|||||||
// update one heuristic. Production is unaffected: `output: "standalone"`
|
// update one heuristic. Production is unaffected: `output: "standalone"`
|
||||||
// bakes resolved env into the build, and the marker file isn't shipped.
|
// bakes resolved env into the build, and the marker file isn't shipped.
|
||||||
loadMonorepoEnv();
|
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 = {
|
const nextConfig: NextConfig = {
|
||||||
output: "standalone",
|
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 {
|
function findMonorepoRoot(start: string): string | null {
|
||||||
let dir = start;
|
let dir = start;
|
||||||
for (let i = 0; i < 6; i++) {
|
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