commit 20685d1914f71966f58a9a253c9dddde63ef8277
parent 801b25407fcfd5cb11fb086e885d92493a351117
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Sun, 20 Sep 2026 19:15:57 -0400
Check the slug's SHAPE on every route that takes one
Only the read route did. The other ten hand the raw string to readChannelConfig
(and friends), which path.joins it under channelsDir with no check — so
"../../x" escapes the corpus. The readers swallow their own errors, which is
what makes it nasty: a traversing segment fails SILENTLY, as an empty config
read back as "channel not found", and any writer later added on that path would
land outside the tree.
reqSlug/reqSlugs in _lib.ts apply isValidChannelSlug (CHANNEL_SLUG_RE — the same
predicate every other slug-taking surface uses) and 400 with the sentence the
channel form shows. One reader rather than a check per route: a route added
later gets this by calling reqSlug instead of reqString, and there is one place
to be wrong.
The spec walks all ten POST routes plus the GET, then asserts nothing moved —
the channels dir holds exactly the fixture channel and its config is unchanged.
The GET case uses a leading dot rather than a slash: the router never matches a
slash-bearing value to one dynamic segment, so that 404s before the handler
exists, and the shapes worth asserting are the ones that DO reach it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
13 files changed, 136 insertions(+), 23 deletions(-)
diff --git a/editor/app/api/ops/_lib.ts b/editor/app/api/ops/_lib.ts
@@ -1,5 +1,6 @@
import { NextResponse } from "next/server";
import { authorizeWorkerRequest } from "yt-dlp-transcript-common/lib/workerToken";
+import { isValidChannelSlug } from "yt-dlp-transcript-common/controller/channels";
import type { StreamActionResult } from "yt-dlp-transcript-common/jobs/streamCommand";
import type { QueueOutcome } from "../../channels/lib/queueForSlugs";
@@ -94,6 +95,37 @@ export function reqString(body: OpsBody, key: string): string {
return v.trim();
}
+// A CHANNEL SLUG, NOT MERELY A STRING. Every slug below reaches a `path.join`
+// under `channelsDir`, and the readers swallow their own errors — so a
+// traversing segment would fail SILENTLY (an empty config, an "empty channel")
+// rather than loudly. `isValidChannelSlug` is CHANNEL_SLUG_RE, which forbids
+// "/" and "..", and is what every other slug-taking surface in the app uses.
+//
+// One reader for every route rather than a check per route: a route added later
+// gets this for free by calling reqSlug instead of reqString, and there is one
+// place to be wrong.
+export function reqSlug(body: OpsBody, key: string): string {
+ const v = reqString(body, key);
+ if (!isValidChannelSlug(v)) {
+ throw new OpsInputError(
+ `"${v}" is not a valid channel slug (letters, digits, ".", "_", "-"; must start with a letter or digit)`,
+ );
+ }
+ return v;
+}
+
+export function reqSlugs(body: OpsBody, key: string): string[] {
+ const values = reqStringArray(body, key);
+ for (const v of values) {
+ if (!isValidChannelSlug(v)) {
+ throw new OpsInputError(
+ `"${v}" is not a valid channel slug (letters, digits, ".", "_", "-"; must start with a letter or digit)`,
+ );
+ }
+ }
+ return values;
+}
+
export function optString(body: OpsBody, key: string): string | undefined {
const v = body[key];
if (v === undefined) return undefined;
diff --git a/editor/app/api/ops/channel-config/route.ts b/editor/app/api/ops/channel-config/route.ts
@@ -6,7 +6,7 @@ import {
validateChannelFormPatch,
} from "../../../channels/components/channelConfigToForm";
import { updateChannelAction } from "../../../channels/actions";
-import { actionResponse, OpsInputError, ops, reqString } from "../_lib";
+import { actionResponse, OpsInputError, ops, reqSlug } from "../_lib";
export const dynamic = "force-dynamic";
@@ -21,7 +21,7 @@ export const dynamic = "force-dynamic";
// `""` (or null) clears a field, exactly as clearing the input does.
export async function POST(request: Request) {
return ops(request, ["slug", "patch"], async (body) => {
- const slug = reqString(body, "slug");
+ const slug = reqSlug(body, "slug");
const patch = body.patch;
if (
typeof patch !== "object" ||
diff --git a/editor/app/api/ops/channel-priority/route.ts b/editor/app/api/ops/channel-priority/route.ts
@@ -15,7 +15,7 @@ import {
OpsInputError,
ops,
optString,
- reqStringArray,
+ reqSlugs,
} from "../_lib";
export const dynamic = "force-dynamic";
@@ -32,7 +32,7 @@ export async function POST(request: Request) {
request,
["slugs", "tier", "operation", "preset"],
async (body) => {
- const slugs = reqStringArray(body, "slugs");
+ const slugs = reqSlugs(body, "slugs");
const preset = optString(body, "preset");
if (preset !== undefined) {
if (preset !== "sync-only" && preset !== "clear") {
diff --git a/editor/app/api/ops/channel/[slug]/route.ts b/editor/app/api/ops/channel/[slug]/route.ts
@@ -44,9 +44,10 @@ export async function GET(
const denied = opsAuth(request);
if (denied) return denied;
const { slug } = await params;
- // The slug reaches three path joins below. readChannelConfig swallows its own
- // errors, so a traversing segment would fail silently rather than loudly —
- // refuse it at the door instead (CHANNEL_SLUG_RE forbids "/" and "..").
+ // The same shape check every POST route applies through reqSlug (_lib.ts);
+ // spelled out here only because the slug arrives as a route param, not in a
+ // body. readChannelConfig swallows its own errors, so a traversing segment
+ // would fail silently rather than loudly.
if (!isValidChannelSlug(slug)) {
return NextResponse.json(
{ ok: false, error: `"${slug}" is not a valid channel slug` },
diff --git a/editor/app/api/ops/download-missing/route.ts b/editor/app/api/ops/download-missing/route.ts
@@ -1,5 +1,5 @@
import { downloadMissingAction } from "../../../channels/[slug]/pipelineActions";
-import { jobResponse, ops, optBool, optString, reqString } from "../_lib";
+import { jobResponse, ops, optBool, optString, reqSlug } from "../_lib";
export const dynamic = "force-dynamic";
@@ -11,7 +11,7 @@ export async function POST(request: Request) {
async (body) =>
jobResponse(
await downloadMissingAction(
- reqString(body, "slug"),
+ reqSlug(body, "slug"),
optString(body, "queueKey"),
optBool(body, "ignoreArchive"),
optBool(body, "abortOnError"),
diff --git a/editor/app/api/ops/import-video/route.ts b/editor/app/api/ops/import-video/route.ts
@@ -1,5 +1,5 @@
import { importVideoAction } from "../../../channels/[slug]/pipelineActions";
-import { jobResponse, ops, optString, reqString } from "../_lib";
+import { jobResponse, ops, optString, reqSlug, reqString } from "../_lib";
export const dynamic = "force-dynamic";
@@ -8,7 +8,7 @@ export async function POST(request: Request) {
return ops(request, ["slug", "url", "queueKey"], async (body) =>
jobResponse(
await importVideoAction(
- reqString(body, "slug"),
+ reqSlug(body, "slug"),
reqString(body, "url"),
optString(body, "queueKey"),
),
diff --git a/editor/app/api/ops/metadata-scan/route.ts b/editor/app/api/ops/metadata-scan/route.ts
@@ -1,5 +1,5 @@
import { runMetadataScanAction } from "../../../channels/[slug]/pipelineActions";
-import { jobResponse, ops, optString, reqString } from "../_lib";
+import { jobResponse, ops, optString, reqSlug } from "../_lib";
export const dynamic = "force-dynamic";
@@ -8,7 +8,7 @@ export async function POST(request: Request) {
return ops(request, ["slug", "queueKey"], async (body) =>
jobResponse(
await runMetadataScanAction(
- reqString(body, "slug"),
+ reqSlug(body, "slug"),
optString(body, "queueKey"),
),
),
diff --git a/editor/app/api/ops/refresh-report/route.ts b/editor/app/api/ops/refresh-report/route.ts
@@ -3,7 +3,14 @@ import {
refreshAllChannelSnapshotsAction,
refreshChannelSnapshotAction,
} from "../../../channels/actions";
-import { actionResponse, OpsInputError, ops, optBool, optString } from "../_lib";
+import {
+ actionResponse,
+ OpsInputError,
+ ops,
+ optBool,
+ optString,
+ reqSlug,
+} from "../_lib";
export const dynamic = "force-dynamic";
@@ -27,6 +34,10 @@ export async function POST(request: Request) {
if (!slug?.trim()) {
throw new OpsInputError('"slug" is required (or send { "all": true })');
}
- return actionResponse(await refreshChannelSnapshotAction(slug.trim()));
+ // Re-read through reqSlug now that we know it is the single-channel form:
+ // the shape check belongs on the value that reaches a path.join.
+ return actionResponse(
+ await refreshChannelSnapshotAction(reqSlug(body, "slug")),
+ );
});
}
diff --git a/editor/app/api/ops/relocate-back/route.ts b/editor/app/api/ops/relocate-back/route.ts
@@ -1,5 +1,5 @@
import { moveChannelMediaBackAction } from "../../../channels/[slug]/storageActions";
-import { ops, queueResponse, reqStringArray } from "../_lib";
+import { ops, queueResponse, reqSlugs } from "../_lib";
import { queueForSlugs } from "../../../channels/lib/queueForSlugs";
export const dynamic = "force-dynamic";
@@ -12,7 +12,7 @@ export const dynamic = "force-dynamic";
export async function POST(request: Request) {
return ops(request, ["slugs"], async (body) =>
queueResponse(
- await queueForSlugs(reqStringArray(body, "slugs"), {
+ await queueForSlugs(reqSlugs(body, "slugs"), {
run: (slug) => moveChannelMediaBackAction(slug),
}),
),
diff --git a/editor/app/api/ops/relocate/route.ts b/editor/app/api/ops/relocate/route.ts
@@ -1,5 +1,11 @@
import { bulkRelocateChannelMediaAction } from "../../../channels/bulkStorageActions";
-import { OpsInputError, ops, optString, queueResponse, reqStringArray } from "../_lib";
+import {
+ OpsInputError,
+ ops,
+ optString,
+ queueResponse,
+ reqSlugs,
+} from "../_lib";
export const dynamic = "force-dynamic";
@@ -13,7 +19,7 @@ export const dynamic = "force-dynamic";
// did not queue comes back with the same sentence the bulk bar shows.
export async function POST(request: Request) {
return ops(request, ["slugs", "locationId", "root"], async (body) => {
- const slugs = reqStringArray(body, "slugs");
+ const slugs = reqSlugs(body, "slugs");
const locationId = optString(body, "locationId");
const root = optString(body, "root");
if ((locationId ? 1 : 0) + (root ? 1 : 0) !== 1) {
diff --git a/editor/app/api/ops/retry-bucket/route.ts b/editor/app/api/ops/retry-bucket/route.ts
@@ -8,6 +8,7 @@ import {
ops,
optBool,
optString,
+ reqSlug,
reqString,
} from "../_lib";
@@ -35,7 +36,7 @@ export async function POST(request: Request) {
"replaceAutoSubs",
],
async (body) => {
- const slug = reqString(body, "slug");
+ const slug = reqSlug(body, "slug");
const bucket = reqString(body, "bucket");
const snapshot = await readChannelSnapshot(getPaths(), slug);
if (!snapshot) {
diff --git a/editor/app/api/ops/sync/route.ts b/editor/app/api/ops/sync/route.ts
@@ -1,5 +1,5 @@
import { syncAction } from "../../../channels/[slug]/pipelineActions";
-import { jobResponse, ops, optBool, optString, reqString } from "../_lib";
+import { jobResponse, ops, optBool, optString, reqSlug } from "../_lib";
export const dynamic = "force-dynamic";
@@ -11,7 +11,7 @@ export async function POST(request: Request) {
return ops(request, ["slug", "full", "queueKey"], async (body) =>
jobResponse(
await syncAction(
- reqString(body, "slug"),
+ reqSlug(body, "slug"),
optString(body, "queueKey"),
optBool(body, "full"),
),
diff --git a/editor/e2e/ops-api.spec.ts b/editor/e2e/ops-api.spec.ts
@@ -14,7 +14,7 @@
// first branch, shared with /api/worker/* and unit-tested by nothing else
// either. See plans/FACTS.md.
-import { rm } from "node:fs/promises";
+import { readdir, rm } from "node:fs/promises";
import { test, expect, type APIRequestContext } from "@playwright/test";
import { baseUrl } from "./baseUrl";
import {
@@ -99,6 +99,68 @@ test("an unknown body key is a 400 that names the accepted keys", async ({
expect(patch.body.error).toContain("downloadFilterExcluded");
});
+test("a traversing slug is refused at the door, on every route that takes one", async ({
+ request,
+}) => {
+ await resetData("title-filter-channel");
+ const channelsDir = resolvePath("test-transcripts/channels");
+ const before = (await readdir(channelsDir)).sort();
+ expect(before).toEqual(["test-filter"]);
+
+ // EVERY SLUG BELOW REACHES A path.join UNDER channelsDir, and the readers
+ // swallow their own errors — so an unchecked traversing segment would fail
+ // SILENTLY (an empty config read as "channel not found") rather than loudly,
+ // and any future writer on that path would land outside the corpus. reqSlug
+ // is one check for all of them; this is the assertion that it is wired to
+ // each.
+ const cases: [string, Record<string, unknown>][] = [
+ ["metadata-scan", { slug: "../../escape" }],
+ ["sync", { slug: "../../escape" }],
+ ["download-missing", { slug: "../../escape" }],
+ ["import-video", { slug: "../../escape", url: "https://example.com/v" }],
+ ["retry-bucket", { slug: "../../escape", bucket: "noTranscript" }],
+ ["refresh-report", { slug: "../../escape" }],
+ ["channel-config", { slug: "../../escape", patch: { cookieMode: "always" } }],
+ ["channel-priority", { slugs: ["../../escape"], tier: "paused" }],
+ ["relocate", { slugs: ["../../escape"], root: "/tmp/ops-api-never" }],
+ ["relocate-back", { slugs: ["../../escape"] }],
+ ];
+ for (const [action, data] of cases) {
+ const { status, body } = await ops(request, action, data);
+ expect(status, action).toBe(400);
+ expect(body.error, action).toMatch(/is not a valid channel slug/);
+ }
+
+ // The read route takes its slug as a path SEGMENT rather than in a body, so
+ // it spells the same check out. A slash-bearing value is not the case to
+ // assert here — the router never matches one to a single dynamic segment, so
+ // it 404s before the handler exists. What DOES reach the handler is a
+ // one-segment name CHANNEL_SLUG_RE still refuses, and a leading dot is the
+ // one that matters: it is how a dotfile beside the channels dir would be
+ // named at.
+ const read = await request.get(`${baseUrl}/api/ops/channel/.escape`, {
+ headers: AUTH,
+ });
+ expect(read.status()).toBe(400);
+ expect(((await read.json()) as OpsResponse).error).toMatch(
+ /is not a valid channel slug/,
+ );
+
+ // NOTHING WAS TOUCHED: the corpus still holds exactly the fixture channel,
+ // and the fixture's own config is byte-identical.
+ expect((await readdir(channelsDir)).sort()).toEqual(before);
+ expect(
+ await readJson<Record<string, unknown>>(
+ "test-transcripts/channels/test-filter/config.json",
+ ),
+ ).toEqual({
+ handling: "youtube",
+ name: "Test Title Filter",
+ url: "https://www.youtube.com/@example/videos",
+ downloadFilter: { include: "guest" },
+ });
+});
+
test("channel-config round-trips a download filter and refuses a bad regex", async ({
page,
request,
