fix(import): allowed import roots, hostile zip symlink removal, tests

Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Lucas Winther
2026-10-30 03:26:22 +01:00
co-authored by Claude Opus 5.5
parent c4df51f7ca
commit 7adf9765b1
15 changed files with 164 additions and 12 deletions
+1
View File
@@ -9,3 +9,4 @@
- `<dataDir>/cache/transcripts/<courseId>/<lessonId>.json` (`{ lessonId, text, segments: [{ start, end, text }] }`).
Suggest a `cacheDir`/`deckPageUrl` helper in core, and a media route for `cache/decks` so the deck viewer can load pages.
5. **Video lessons have no description field.** Packt clips come with a one-line description. It is dropped, because `transcript` is the only Markdown a video lesson can carry and the transcription job writes it.
6. **`STC_IMPORT_ROOTS`** (":"-separated, default `<dataDir>/import-inbox`, created when `importFeature` is built) is read inside the import package. Core config could own it later.
+2
View File
@@ -5,6 +5,7 @@ import { importPacktVideo } from "./jobs/packt-video";
import { importPdfDeck } from "./jobs/pdf-deck";
import { transcribeCourse } from "./jobs/transcribe";
import { importVideoFolder } from "./jobs/video-folder";
import { ensureRoots } from "./roots";
import { importRoutes } from "./routes";
export type { ImportOptions } from "./context";
@@ -21,6 +22,7 @@ export function importFeature(ctx: ServerContext, opts: ImportOptions = {}): Fea
return { ctx, env, tools: { ffmpeg: env.STC_FFMPEG || "ffmpeg", ffprobe: env.STC_FFPROBE || "ffprobe" }, python: env.STC_PYTHON || "python3" };
};
const job = (fn: (d: Deps, p: unknown, jc: Parameters<JobHandler>[1]) => Promise<unknown>): JobHandler => (p, jc) => fn(deps(), p, jc);
ensureRoots({ ctx, env: opts.env ?? process.env });
return {
name: "import",
routes: importRoutes(deps),
@@ -5,6 +5,7 @@ import { finish, managedCourse, saveManifest, stageDir, type Deps } from "../con
import { attachDeck } from "../deck";
import { putFile } from "../media";
import { run } from "../util/exec";
import { assertAllowedPath } from "../roots";
import { openSource } from "../util/source";
import { containment } from "../util/text";
import { optStr, parsePayload, withStage } from "./common";
@@ -50,6 +51,7 @@ export async function importCodeRepo(d: Deps, payload: unknown, jc: JobContext):
const { path, options } = parsePayload(payload, "code-repo");
const warnings: string[] = [];
const course = await managedCourse(d, optStr(options, "courseId"));
if (!GIT_URL.test(path)) await assertAllowedPath(d, path);
const stage = await stageDir(d, "repo");
return withStage(stage, async () => {
@@ -4,12 +4,14 @@ import type { ImportResult, JobContext } from "@stc/contracts";
import { finish, frameCandidates, freshCourseId, publishCourse, saveManifest, sourceBlock, stageDir, type Deps } from "../context";
import { probeDuration } from "../media";
import { parsePacktIndex } from "../packt";
import { assertAllowedPath } from "../roots";
import { openSource } from "../util/source";
import { adoptCover, maybeQueueTranscription, optBool, optStr, parsePayload, withStage } from "./common";
export async function importPacktVideo(d: Deps, payload: unknown, jc: JobContext): Promise<ImportResult> {
const { path, options } = parsePayload(payload, "packt-video");
const warnings: string[] = [];
await assertAllowedPath(d, path);
await jc.reportProgress(0.02, "Reading the package");
const src = await openSource(path);
@@ -2,12 +2,14 @@ import { stat } from "node:fs/promises";
import { join } from "node:path";
import type { ImportResult, JobContext } from "@stc/contracts";
import { finish, managedCourse, saveManifest, type Deps } from "../context";
import { assertAllowedPath } from "../roots";
import { attachDeck } from "../deck";
import { optStr, parsePayload } from "./common";
export async function importPdfDeck(d: Deps, payload: unknown, jc: JobContext): Promise<ImportResult> {
const { path, options } = parsePayload(payload, "pdf-deck");
const warnings: string[] = [];
await assertAllowedPath(d, path);
if (!(await stat(path).catch(() => null))?.isFile() || !/\.pdf$/i.test(path)) throw new Error(`not a PDF file: ${path}`);
const course = await managedCourse(d, optStr(options, "courseId"));
const attachTo = optStr(options, "attachTo");
@@ -3,6 +3,7 @@ import { basename, extname, join } from "node:path";
import type { ImportResult, JobContext } from "@stc/contracts";
import { finish, frameCandidates, freshCourseId, publishCourse, saveManifest, sourceBlock, stageDir, type Deps } from "../context";
import { probeDuration, putFile } from "../media";
import { assertAllowedPath } from "../roots";
import { naturalSort } from "../util/text";
import { openSource } from "../util/source";
import { adoptCover, maybeQueueTranscription, optBool, optStr, parsePayload, withStage } from "./common";
@@ -19,6 +20,7 @@ export function lessonTitle(file: string): string {
export async function importVideoFolder(d: Deps, payload: unknown, jc: JobContext): Promise<ImportResult> {
const { path, options } = parsePayload(payload, "video-folder");
const warnings: string[] = [];
await assertAllowedPath(d, path);
if (!(await stat(path).catch(() => null))?.isDirectory()) throw new Error(`not a folder: ${path}`);
const src = await openSource(path);
const files = naturalSort(src.entries.filter((e) => VIDEO.test(e) && !e.split("/").some((p) => p.startsWith("."))));
+30
View File
@@ -0,0 +1,30 @@
import { mkdirSync } from "node:fs";
import { realpath } from "node:fs/promises";
import { resolve, sep } from "node:path";
import type { Deps } from "./context";
export const OUTSIDE = "path is outside the allowed import folders";
/** STC_IMPORT_ROOTS (":"-separated), default <dataDir>/import-inbox. */
export function importRoots(d: Pick<Deps, "ctx" | "env">): string[] {
const raw = d.env.STC_IMPORT_ROOTS;
const list = raw ? raw.split(":").filter(Boolean) : [resolve(d.ctx.config.dataDir, "import-inbox")];
return list.map((r) => resolve(r));
}
export function ensureRoots(d: Pick<Deps, "ctx" | "env">): void {
for (const r of importRoots(d)) { try { mkdirSync(r, { recursive: true }); } catch { /* reported when used */ } }
}
/** Real path (symlinks followed) must lie inside the real path of one root. Throws OUTSIDE otherwise. */
export async function assertAllowedPath(d: Pick<Deps, "ctx" | "env">, path: string): Promise<string> {
let real: string;
try { real = await realpath(resolve(path)); } catch { throw new Error(OUTSIDE); }
for (const root of importRoots(d)) {
const rr = await realpath(root).catch(() => null);
if (rr && (real === rr || real.startsWith(rr + sep))) return real;
}
throw new Error(OUTSIDE);
}
export const isGitUrl = (p: string) => /^(https?:\/\/|ssh:\/\/|git@)\S+$/.test(p);
+10 -5
View File
@@ -1,9 +1,9 @@
import { copyFile, mkdir, readdir } from "node:fs/promises";
import { existsSync } from "node:fs";
import { join, relative, resolve } from "node:path";
import type { Feature, RouteHandler } from "@stc/contracts";
import { serveMedia } from "@stc/core";
import { coverCacheDir, coursesDir, exists, managedCourse, saveManifest, type Deps } from "./context";
import { assertAllowedPath, importRoots, OUTSIDE } from "./roots";
import { coverCacheDir, coursesDir, exists, isPlainId, managedCourse, saveManifest, type Deps } from "./context";
export const KINDS = ["packt-video", "code-repo", "pdf-deck", "video-folder"] as const;
const OPTION_KEYS = ["title", "courseId", "attachTo", "media", "transcribe"] as const;
@@ -26,7 +26,7 @@ export function importRoutes(deps: () => Deps): Feature["routes"] {
const d = deps();
const dir = coursesDir(d);
const served = d.ctx.config.courseDirs.some((c) => resolve(c) === dir);
return json({ transcribe: Boolean(d.env.STC_TRANSCRIBE_URL), coursesDir: dir, coursesDirServed: served });
return json({ transcribe: Boolean(d.env.STC_TRANSCRIBE_URL), coursesDir: dir, coursesDirServed: served, importRoots: importRoots(d) });
}),
},
"/api/import/jobs": {
@@ -50,8 +50,10 @@ export function importRoutes(deps: () => Deps): Feature["routes"] {
const path = typeof body?.path === "string" ? body.path.trim() : "";
if (!path) return json({ error: "path is required" }, 400);
const isGit = kind === "code-repo" && GIT_URL.test(path);
if (!isGit && !existsSync(path)) return json({ error: `Path not found on the server: ${path}` }, 400);
if (!isGit && !path.startsWith("/")) return json({ error: "Use an absolute path" }, 400);
if (!isGit) {
if (!path.startsWith("/")) return json({ error: "Use an absolute path" }, 400);
try { await assertAllowedPath(d, path); } catch { return json({ error: OUTSIDE }, 400); }
}
if ((kind === "code-repo" || kind === "pdf-deck") && typeof options.courseId !== "string") return json({ error: "courseId is required" }, 400);
if (kind === "pdf-deck" && typeof options.attachTo !== "string") return json({ error: "attachTo is required" }, 400);
return json({ id: await d.ctx.jobs.enqueue(`import.${kind}`, { kind, path, options }) }, 202);
@@ -74,6 +76,7 @@ export function importRoutes(deps: () => Deps): Feature["routes"] {
GET: owner(async (_req, rc) => {
const d = deps();
const id = rc.params.courseId!;
if (!isPlainId(id)) return json({ error: "Invalid course id" }, 400);
const c = d.ctx.courses.get(id);
const names = (await readdir(coverCacheDir(d, id)).catch(() => [] as string[])).filter((n) => COVER_NAME.test(n)).sort();
return json({ current: c?.manifest?.cover ?? null, candidates: names });
@@ -84,6 +87,7 @@ export function importRoutes(deps: () => Deps): Feature["routes"] {
try { body = await req.json(); } catch { return json({ error: "Body must be JSON" }, 400); }
const name = String(body?.name ?? "");
if (!COVER_NAME.test(name)) return json({ error: "Invalid candidate name" }, 400);
if (!isPlainId(rc.params.courseId!)) return json({ error: "Invalid course id" }, 400);
const src = join(coverCacheDir(d, rc.params.courseId!), name);
if (!(await exists(src))) return json({ error: "No such candidate" }, 404);
let course;
@@ -100,6 +104,7 @@ export function importRoutes(deps: () => Deps): Feature["routes"] {
GET: owner(async (req, rc) => {
const name = rc.params.name!;
if (!COVER_NAME.test(name)) return new Response("Not found", { status: 404 });
if (!isPlainId(rc.params.courseId!)) return new Response("Not found", { status: 404 });
const file = join(coverCacheDir(deps(), rc.params.courseId!), name);
if (!(await exists(file))) return new Response("Not found", { status: 404 });
return serveMedia(req, file);
+15 -3
View File
@@ -1,4 +1,4 @@
import { cp, mkdir, readdir, readFile, stat } from "node:fs/promises";
import { cp, mkdir, readdir, readFile, rm, stat } from "node:fs/promises";
import { dirname, join } from "node:path";
import { run } from "./exec";
@@ -22,6 +22,14 @@ async function walk(root: string, rel = "", out: string[] = []): Promise<string[
return out;
}
async function removeSymlinks(dir: string): Promise<void> {
for (const e of await readdir(dir, { withFileTypes: true })) {
const p = join(dir, e.name);
if (e.isSymbolicLink()) await rm(p, { force: true });
else if (e.isDirectory()) await removeSymlinks(p);
}
}
export async function openSource(path: string): Promise<Source> {
const st = await stat(path).catch(() => null);
if (!st) throw new Error(`source not found: ${path}`);
@@ -47,8 +55,12 @@ export async function openSource(path: string): Promise<Source> {
async extract(list, dest, signal) {
await mkdir(dest, { recursive: true });
// unzip treats arguments as patterns, so glob characters in names are escaped; batches keep argv small.
for (let i = 0; i < list.length; i += 200) {
await run(["unzip", "-q", "-o", path, ...list.slice(i, i + 200).map(globEscape), "-d", dest], { signal });
try {
for (let i = 0; i < list.length; i += 200) {
await run(["unzip", "-q", "-o", path, ...list.slice(i, i + 200).map(globEscape), "-d", dest], { signal });
}
} finally {
await removeSymlinks(dest); // a zip can carry symlinks; none may survive extraction
}
},
};
@@ -5,7 +5,7 @@ import "./import.css";
type Kind = "packt-video" | "code-repo" | "pdf-deck" | "video-folder";
interface Job { id: string; kind: string; status: "queued" | "running" | "done" | "failed"; progress: number; message: string | null; payload: any; result: any; error: string | null; createdAt: string }
interface CourseInfo { id: string; title: string; modules: { id: string; title: string; lessons: { id: string; title: string }[] }[] }
interface Info { transcribe: boolean; coursesDir: string; coursesDirServed: boolean }
interface Info { importRoots: string[]; transcribe: boolean; coursesDir: string; coursesDirServed: boolean }
const KINDS: { value: Kind; label: string; hint: string; placeholder: string }[] = [
{ value: "packt-video", label: "Packt video zip", hint: "A video package whose index.html lists the sections and clips.", placeholder: "/data/downloads/video_zip.zip" },
@@ -139,6 +139,7 @@ export default function ImportPage() {
<label className="imp-field">Path on the server
<input type="text" className="mono" value={path} onChange={(e) => setPath(e.target.value)} placeholder={spec.placeholder} required />
<span className="imp-hint">{spec.hint}</span>
{info ? <span className="imp-hint">Allowed folders: {info.importRoots.map((r) => <code key={r}>{r} </code>)}(set <code>STC_IMPORT_ROOTS</code>; git URLs are exempt)</span> : null}
</label>
{!needsCourse ? (
<label className="imp-field">Course title (optional)
+2 -1
View File
@@ -1,3 +1,4 @@
import { tmpdir } from "node:os";
import { join } from "node:path";
import type { JobContext, ServerContext } from "@stc/contracts";
import { createCourseRepository } from "@stc/course-format";
@@ -17,5 +18,5 @@ export async function makeEnv(env: Record<string, string | undefined> = {}): Pro
jobs: { enqueue: async (kind: string, payload: unknown) => { queued.push({ kind, payload }); return "j1"; }, get: async () => null, list: async () => [] },
} as unknown as ServerContext;
const jc: JobContext = { signal: new AbortController().signal, reportProgress: async (f, m) => { progress.push([f, m]); } };
return { deps: { ctx, env, tools: { ffmpeg: "ffmpeg", ffprobe: "ffprobe" }, python: PYTHON }, dataDir, queued, jc, progress };
return { deps: { ctx, env: { STC_IMPORT_ROOTS: tmpdir(), ...env }, tools: { ffmpeg: "ffmpeg", ffprobe: "ffprobe" }, python: PYTHON }, dataDir, queued, jc, progress };
}
@@ -0,0 +1,39 @@
import { expect, test } from "bun:test";
import { existsSync } from "node:fs";
import { readdir } from "node:fs/promises";
import { join } from "node:path";
import { openSource } from "../src/util/source";
import { PYTHON, tmp } from "./fixtures";
import { run } from "../src/util/exec";
/** Builds a zip by hand with traversal, absolute and symlink entries. */
async function hostileZip(path: string) {
await run([PYTHON, "-I", "-c", `
import sys, zipfile, stat
z = zipfile.ZipFile(sys.argv[1], 'w')
z.writestr('../../evil-up.txt', 'x')
z.writestr('/tmp/stc-evil-abs.txt', 'x')
z.writestr('ok/fine.txt', 'fine')
i = zipfile.ZipInfo('ok/link'); i.create_system = 3; i.external_attr = (stat.S_IFLNK | 0o777) << 16
z.writestr(i, '/etc')
i2 = zipfile.ZipInfo('ok/link/pwned.txt'); z.writestr(i2, 'x')
z.close()`, path]);
}
test("hostile zip entries never write outside the extraction dir", async () => {
const base = await tmp("stc-zip-");
await hostileZip(join(base, "h.zip"));
const src = await openSource(join(base, "h.zip"));
const dest = join(base, "a", "b", "out");
await src.extract(src.entries, dest).catch(() => {}); // unzip may warn; what matters is where files land
expect(existsSync(join(base, "evil-up.txt"))).toBe(false);
expect(existsSync(join(base, "a", "evil-up.txt"))).toBe(false);
expect(existsSync(join(base, "..", "evil-up.txt"))).toBe(false);
expect(existsSync("/tmp/stc-evil-abs.txt")).toBe(false);
expect(existsSync("/etc/pwned.txt")).toBe(false);
expect(await Bun.file(join(dest, "ok/fine.txt")).text()).toBe("fine");
const left: string[] = [];
const walk = async (d: string) => { for (const e of await readdir(d, { withFileTypes: true })) { if (e.isSymbolicLink()) left.push(e.name); else if (e.isDirectory()) await walk(join(d, e.name)); } };
await walk(dest);
expect(left).toEqual([]); // the symlink entry is dropped
});
@@ -0,0 +1,26 @@
import { expect, test } from "bun:test";
import { mkdir, symlink, writeFile } from "node:fs/promises";
import { join } from "node:path";
import { importCodeRepo } from "../src/jobs/code-repo";
import { importPdfDeck } from "../src/jobs/pdf-deck";
import { importVideoFolder } from "../src/jobs/video-folder";
import { importPacktVideo } from "../src/jobs/packt-video";
import { makeEnv } from "./harness";
import { makePdf, tmp } from "./fixtures";
test("every job re-checks its path against the roots", async () => {
const inbox = await tmp("stc-inbox-");
const outside = await tmp("stc-outside-");
await makePdf(join(outside, "a.pdf"));
await symlink(outside, join(inbox, "link"));
const env = await makeEnv({ STC_IMPORT_ROOTS: inbox });
const msg = /outside the allowed import folders/;
await expect(importVideoFolder(env.deps, { path: outside }, env.jc)).rejects.toThrow(msg);
await expect(importVideoFolder(env.deps, { path: join(inbox, "link") }, env.jc)).rejects.toThrow(msg);
await expect(importPacktVideo(env.deps, { path: join(outside, "x.zip") }, env.jc)).rejects.toThrow(msg);
await expect(importPdfDeck(env.deps, { path: join(outside, "a.pdf"), options: {} }, env.jc)).rejects.toThrow(msg);
await expect(importCodeRepo(env.deps, { path: outside, options: { courseId: "x" } }, env.jc)).rejects.toThrow(/created|outside/);
await mkdir(join(inbox, "empty"));
await writeFile(join(inbox, "empty", "n.txt"), "");
await expect(importVideoFolder(env.deps, { path: join(inbox, "empty") }, env.jc)).rejects.toThrow(/no video files/); // allowed, then fails on content
});
+28 -1
View File
@@ -1,10 +1,12 @@
import { expect } from "bun:test";
import { tmpdir } from "node:os";
import { join } from "node:path";
import type { AuthUser, ServerContext } from "@stc/contracts";
import { createApp, createJobQueue } from "@stc/core";
import { createCourseRepository } from "@stc/course-format";
import { forEachDb } from "../../db/test/helpers";
import { importFeature } from "../src";
import { mkdir, stat, symlink } from "node:fs/promises";
import { makeVideo, tmp } from "./fixtures";
const owner: AuthUser = { id: "o", email: "o@x", name: "O", roles: ["owner"], groups: [] };
@@ -18,7 +20,7 @@ async function setup(db: any, env: Record<string, string | undefined> = {}) {
db, courses,
auth: { userFromRequest: async (req: Request) => ({ owner, learner }[req.headers.get("x-user") ?? ""] ?? null) },
} as unknown as ServerContext;
const feature = importFeature(ctx, { env: { STC_PYTHON: process.env.STC_PYTHON, ...env } });
const feature = importFeature(ctx, { env: { STC_PYTHON: process.env.STC_PYTHON, STC_IMPORT_ROOTS: tmpdir(), ...env } });
const queue = createJobQueue(db, feature.jobs!, { pollMs: 10 });
ctx.jobs = queue;
const app = createApp(ctx, [feature]);
@@ -46,6 +48,7 @@ forEachDb("job creation validates input", async (db) => {
expect((await post({ kind: "nope", path: "/tmp" })).status).toBe(400);
expect((await post({ kind: "video-folder" })).status).toBe(400);
expect((await post({ kind: "video-folder", path: "/definitely/not/here" })).status).toBe(400);
expect((await post({ kind: "video-folder", path: "relative" })).status).toBe(400);
expect((await post({ kind: "pdf-deck", path: "/tmp", options: { courseId: "x" } })).status).toBe(400);
expect((await post({ kind: "code-repo", path: "/tmp" })).status).toBe(400);
expect((await post({ kind: "transcribe", options: { courseId: "x" } })).status).toBe(409);
@@ -86,3 +89,27 @@ forEachDb("a folder import runs through the real queue, then cover candidates ca
expect(await Bun.file(join(dataDir, "courses/queued/media/cover.jpg")).exists()).toBe(true);
} finally { await queue.stop(); }
});
forEachDb("paths must stay inside the import roots", async (db) => {
const inbox = await tmp("stc-inbox-");
const outside = await tmp("stc-outside-");
await mkdir(join(inbox, "ok"));
await symlink(outside, join(inbox, "escape"));
const { call, ctx } = await setup(db, { STC_IMPORT_ROOTS: inbox });
const post = (path: string) => call("/api/import/jobs", { as: "owner", method: "POST", json: { kind: "video-folder", path } });
const bad = await post(outside);
expect(bad.status).toBe(400);
expect(await bad.json()).toEqual({ error: "path is outside the allowed import folders" });
expect((await post(join(inbox, "..", "x"))).status).toBe(400);
expect((await post(join(inbox, "escape"))).status).toBe(400); // symlink out of the root
expect((await post("/etc")).status).toBe(400);
expect((await post(join(inbox, "ok"))).status).toBe(202);
const info = (await (await call("/api/import/info", { as: "owner" })).json()) as any;
expect(info.importRoots).toEqual([inbox]);
void ctx;
});
forEachDb("the default root is created under dataDir", async (db) => {
const { dataDir } = await setup(db, { STC_IMPORT_ROOTS: undefined });
expect((await stat(join(dataDir, "import-inbox"))).isDirectory()).toBe(true);
});
@@ -61,7 +61,7 @@ test("deck attach validates the target and refuses courses the importer did not
await expect(importPdfDeck(env.deps, { path: join(d, "a.pdf"), options: { courseId: r.courseId } }, env.jc)).rejects.toThrow(/attachTo is required/);
await expect(importPdfDeck(env.deps, { path: join(d, "a.pdf"), options: { courseId: "sample", attachTo: "x" } }, env.jc)).rejects.toThrow(/only edits courses it created/);
await expect(importPdfDeck(env.deps, { path: join(d, "a.pdf"), options: { courseId: "../x", attachTo: "x" } }, env.jc)).rejects.toThrow(/only edits courses it created/);
await expect(importPdfDeck(env.deps, { path: join(d, "missing.pdf"), options: {} }, env.jc)).rejects.toThrow(/not a PDF/);
await expect(importPdfDeck(env.deps, { path: join(d, "missing.pdf"), options: {} }, env.jc)).rejects.toThrow(/not a PDF|outside the allowed/);
});
test("without PyMuPDF the deck is still attached, with a warning", async () => {