commit 2d641433aa8b42158fdf5dae904ff136456313e5
parent b8fe7c2c2baf6fe48619f88bc92b83ef5bd745e2
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Tue, 22 Sep 2026 16:06:09 -0400
editor+common: the 503 branch of the worker gate is asserted, not assumed
`WORKER_TOKEN` unset means the endpoint is OFF — you opt in — and that branch
was pinned by nothing. The e2e suite could not reach it (one server, booted with
the token set) and no unit test existed either, so the answer that decides
whether an instance is an open transcription server was the one nobody checked.
common/lib/workerToken.test.ts covers the table: unset and empty => 503 whatever
the header carries, set + missing/malformed => 401, set + mismatched => 401 at
any length, set + match => ok. It also pins the property the route below leans
on — the token is read from process.env per call, never at import.
/api/test/worker-token turns the variable off and back on inside the running
server, the way /api/test/resume-lane fakes a restart. The spec restores it in a
`finally`: the variable is process-wide and the server outlives the spec.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
3 files changed, 240 insertions(+), 6 deletions(-)
diff --git a/common/lib/workerToken.test.ts b/common/lib/workerToken.test.ts
@@ -0,0 +1,121 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import {
+ authorizeWorkerRequest,
+ getWorkerToken,
+ workerEndpointEnabled,
+} from "./workerToken";
+
+// Run with: node_modules/.bin/tsx --test common/lib/workerToken.test.ts
+//
+// THE 503 BRANCH HAD NO COVERAGE ANYWHERE, and it is the one that decides
+// whether an instance is an open transcription server. The e2e suite cannot
+// reach it: the test server boots with WORKER_TOKEN=test-worker-token
+// (editor/package.json, dev:test) and one server serves the whole suite, so no
+// spec can observe the endpoint disabled without restarting it. That left the
+// most consequential of the three answers — "unset means OFF, not means open" —
+// asserted by nothing at all.
+//
+// `getWorkerToken()` reads `process.env` on every call, never at import, which
+// is exactly what makes this testable: set the variable, ask, restore. Each
+// test restores in a `finally` so a failure cannot leak a token into the next.
+
+function withToken<T>(value: string | undefined, fn: () => T): T {
+ const before = process.env.WORKER_TOKEN;
+ if (value === undefined) delete process.env.WORKER_TOKEN;
+ else process.env.WORKER_TOKEN = value;
+ try {
+ return fn();
+ } finally {
+ if (before === undefined) delete process.env.WORKER_TOKEN;
+ else process.env.WORKER_TOKEN = before;
+ }
+}
+
+// UNSET IS OFF, AND OFF IS 503 — never 401 and never ok. The distinction is the
+// whole opt-in: 401 would tell a scanner "there is a secret here, guess it",
+// and ok would make every instance that never set the variable a public
+// transcription server.
+test("no token configured: every request is 503, whatever it carries", () => {
+ withToken(undefined, () => {
+ assert.equal(getWorkerToken(), "");
+ assert.equal(workerEndpointEnabled(), false);
+ for (const header of [
+ null,
+ "",
+ "Bearer anything",
+ "Bearer ",
+ "Basic dXNlcjpwYXNz",
+ ]) {
+ const auth = authorizeWorkerRequest(header);
+ assert.equal(auth.ok, false);
+ assert.equal(auth.ok === false && auth.status, 503);
+ assert.match(
+ auth.ok === false ? auth.error : "",
+ /disabled \(set WORKER_TOKEN to enable\)/,
+ );
+ }
+ });
+});
+
+// An EMPTY string is unset. `WORKER_TOKEN=` in a compose file is a variable
+// somebody meant to fill in, not a secret of length zero that every caller
+// matches.
+test("an empty token is not a token", () => {
+ withToken("", () => {
+ assert.equal(workerEndpointEnabled(), false);
+ const auth = authorizeWorkerRequest("Bearer ");
+ assert.equal(auth.ok === false && auth.status, 503);
+ });
+});
+
+test("token configured: a missing or malformed header is 401", () => {
+ withToken("s3cret", () => {
+ assert.equal(workerEndpointEnabled(), true);
+ for (const header of [null, "", "s3cret", "Basic s3cret", "Bearer"]) {
+ const auth = authorizeWorkerRequest(header);
+ assert.equal(auth.ok, false);
+ assert.equal(auth.ok === false && auth.status, 401);
+ assert.match(auth.ok === false ? auth.error : "", /missing bearer token/);
+ }
+ });
+});
+
+// A WRONG TOKEN IS 401 AND NOT 503: the surface is on, the caller is not
+// welcome. Lengths that differ are checked before timingSafeEqual, which throws
+// on mismatched buffers — a prefix of the real token is the case that proves it.
+test("token configured: a mismatched token is 401, at any length", () => {
+ withToken("s3cret", () => {
+ for (const bad of ["wrong", "s3cre", "s3crets", "S3CRET", "s3cret "]) {
+ const auth = authorizeWorkerRequest(`Bearer ${bad}`);
+ assert.equal(auth.ok, false);
+ assert.equal(auth.ok === false && auth.status, 401);
+ assert.match(auth.ok === false ? auth.error : "", /invalid worker token/);
+ }
+ });
+});
+
+test("token configured: the matching token is accepted", () => {
+ withToken("s3cret", () => {
+ assert.deepEqual(authorizeWorkerRequest("Bearer s3cret"), { ok: true });
+ // The scheme is case-insensitive — a worker written against `bearer` is not
+ // a different caller.
+ assert.deepEqual(authorizeWorkerRequest("bearer s3cret"), { ok: true });
+ // Several spaces after the scheme are still one separator — `\s+` eats
+ // them all, so leading whitespace in the value is not a different token.
+ // TRAILING whitespace is: it lands inside the capture and mismatches.
+ assert.deepEqual(authorizeWorkerRequest("Bearer s3cret"), { ok: true });
+ });
+});
+
+// THE TOKEN IS READ PER CALL, never captured at import. `/api/test/worker-token`
+// leans on exactly this to unset and restore the variable inside a running
+// server, and a module-level cache would silently make that route a no-op.
+test("the token is read from the environment on every call", () => {
+ withToken("first", () => {
+ assert.deepEqual(authorizeWorkerRequest("Bearer first"), { ok: true });
+ process.env.WORKER_TOKEN = "second";
+ assert.equal(authorizeWorkerRequest("Bearer first").ok, false);
+ assert.deepEqual(authorizeWorkerRequest("Bearer second"), { ok: true });
+ });
+});
diff --git a/editor/app/api/test/worker-token/route.ts b/editor/app/api/test/worker-token/route.ts
@@ -0,0 +1,50 @@
+import { NextResponse } from "next/server";
+import { workerEndpointEnabled } from "yt-dlp-transcript-common/lib/workerToken";
+
+export const dynamic = "force-dynamic";
+
+// E2E test harness only. Unsets and restores `WORKER_TOKEN` inside the running
+// server.
+//
+// WHY A ROUTE AND NOT A SPEC FIXTURE. The 503 branch is the one that decides
+// whether an instance is an open transcription server — unset means OFF, you
+// opt in — and no spec could observe it: the test server boots with
+// WORKER_TOKEN=test-worker-token (editor/package.json, dev:test) and ONE server
+// serves the whole suite, so the only way to see the endpoint disabled is to
+// turn it off in the process that is answering. A suite cannot restart the dev
+// server mid-run, which is the same reason /api/test/resume-lane exists.
+//
+// IT WORKS BECAUSE `getWorkerToken()` READS process.env PER CALL, never at
+// import (common/lib/workerToken.test.ts pins that). A module-level cache would
+// make this route a silent no-op and the spec below would pass by proving
+// nothing.
+//
+// ⚠️ THE SPEC MUST RESTORE IN A `finally`. The variable is process-wide and the
+// server outlives the spec, so a run that unsets it and throws leaves every
+// later /api/ops and /api/worker spec answering 503 — a whole suite red from
+// one failure. `?set=` with no value is the restore, and it is idempotent.
+//
+// Mounted unconditionally, like the other /api/test routes: the editor is a
+// localhost admin tool, not a deployed service.
+export async function GET(req: Request) {
+ const url = new URL(req.url);
+ // `?set=<token>` restores (or changes) it; `?unset=1` removes it entirely.
+ // Exactly one of the two, so a typo cannot silently do nothing.
+ const set = url.searchParams.get("set");
+ const unset = url.searchParams.get("unset");
+ if ((set === null) === (unset === null)) {
+ return NextResponse.json(
+ { ok: false, error: "pass exactly one of ?set=<token> or ?unset=1" },
+ { status: 400 },
+ );
+ }
+ const before = workerEndpointEnabled();
+ if (unset !== null) delete process.env.WORKER_TOKEN;
+ else process.env.WORKER_TOKEN = set as string;
+ return NextResponse.json({
+ ok: true,
+ was: before,
+ // Never the token itself: this response goes into a test log.
+ enabled: workerEndpointEnabled(),
+ });
+}
diff --git a/editor/e2e/ops-api.spec.ts b/editor/e2e/ops-api.spec.ts
@@ -7,12 +7,24 @@
// title-filter.spec.ts reads off the form, the busy-channel refusal the Storage
// panel shows — and about the files on disk, not about the routes' own shapes.
//
-// THE 503 BRANCH IS NOT REACHABLE FROM HERE. The test server runs with
-// WORKER_TOKEN=test-worker-token (editor/package.json, dev:test) and there is one
-// server for the whole suite, so no spec can observe the endpoint disabled. 401
-// (missing and wrong) is covered below; the 503 is authorizeWorkerRequest's own
-// first branch, shared with /api/worker/* and unit-tested by nothing else
-// either. See plans/FACTS.md.
+// THE TOKEN GATE HAS THREE ANSWERS, AND ALL THREE ARE PINNED HERE.
+//
+// `WORKER_TOKEN` unset => 503, wrong or missing => 401, matching => the route
+// runs. The 503 is the consequential one: it is what stops an instance that
+// never set the variable being an open transcription server, and 401 in its
+// place would tell a scanner "there is a secret here, guess it".
+//
+// It used to be unreachable from a spec — the test server boots with
+// WORKER_TOKEN=test-worker-token (editor/package.json, dev:test) and one server
+// serves the whole suite, so nothing could observe the endpoint disabled. It is
+// reachable now because /api/test/worker-token turns the variable off and back
+// on INSIDE that server, which works only because `getWorkerToken()` reads
+// process.env per call (common/lib/workerToken.test.ts pins that, and covers
+// the branch table itself).
+//
+// ⚠️ THE VARIABLE IS PROCESS-WIDE AND THE SERVER OUTLIVES THE SPEC. Restoring
+// it is a `finally`, never a trailing line: one failed assertion with the token
+// still unset leaves every later /api/ops and /api/worker spec answering 503.
import { readdir, rm } from "node:fs/promises";
import { test, expect, type APIRequestContext } from "@playwright/test";
@@ -77,6 +89,57 @@ test("the token gate answers 401 for a missing and for a wrong bearer", async ({
}
});
+// UNSET IS OFF, on the ops door and on the worker door alike — one branch,
+// shared, and neither surface may decide for itself that "no token configured"
+// means "let them in".
+test("with no token configured every guarded route answers 503", async ({
+ request,
+}) => {
+ await resetData("empty");
+ const toggle = async (query: string) => {
+ const res = await request.get(`${baseUrl}/api/test/worker-token?${query}`);
+ expect(res.status(), query).toBe(200);
+ return (await res.json()) as { ok: boolean; enabled: boolean };
+ };
+
+ const off = await toggle("unset=1");
+ expect(off.enabled).toBe(false);
+ try {
+ // The ops door, with the RIGHT token: it is the surface being off that
+ // answers, not the credential being wrong.
+ const tags = await request.get(`${baseUrl}/api/ops/tags`, {
+ headers: AUTH,
+ });
+ expect(tags.status()).toBe(503);
+ expect(((await tags.json()) as OpsResponse).error).toMatch(
+ /set WORKER_TOKEN to enable/,
+ );
+
+ // And the LAN worker door, which shares the branch.
+ const health = await request.get(`${baseUrl}/api/worker/health`, {
+ headers: AUTH,
+ });
+ expect(health.status()).toBe(503);
+ expect(((await health.json()) as OpsResponse).error).toMatch(
+ /set WORKER_TOKEN to enable/,
+ );
+
+ // With no token configured a MISSING header is still 503, not 401: there is
+ // nothing to be unauthorized against.
+ const bare = await request.get(`${baseUrl}/api/ops/tags`);
+ expect(bare.status()).toBe(503);
+ } finally {
+ // NOT a trailing line. See the header: the server outlives this spec.
+ const on = await toggle(`set=${TOKEN}`);
+ expect(on.enabled).toBe(true);
+ }
+
+ // Restored, and the same request now works — which is also the assertion
+ // that the `finally` above did what it claims.
+ const after = await request.get(`${baseUrl}/api/ops/tags`, { headers: AUTH });
+ expect(after.status()).toBe(200);
+});
+
test("an unknown body key is a 400 that names the accepted keys", async ({
request,
}) => {
