From f71a31a5f157d13403138793997e8c9a277040b2 Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Fri, 30 Oct 2026 02:55:38 +0100 Subject: [PATCH] fix(course-format): validate anchor file via requireFile; real throw test for loadCourse catch Co-Authored-By: Claude Opus 5.5 --- packages/course-format/src/load.ts | 11 ++++++- packages/course-format/test/load.test.ts | 40 +++++++++++++++++------- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/packages/course-format/src/load.ts b/packages/course-format/src/load.ts index 32065f6..579d6ee 100644 --- a/packages/course-format/src/load.ts +++ b/packages/course-format/src/load.ts @@ -138,7 +138,16 @@ async function loadCourseUnsafe(manifestPath: string): Promise { return mdCache.get(abs)!; }; /** Reports an error and returns false when `rel` is not a regular file (or directory, if asked) inside the root. */ + const checked = new Map(); // one diagnostic per (lesson, path) const requireFile = (rel: string, what: string, lessonId?: string, dir = false): boolean => { + const key = `${lessonId ?? ""}\0${rel}\0${dir}`; + const prior = checked.get(key); + if (prior !== undefined) return prior; + const ok = checkFile(rel, what, lessonId, dir); + checked.set(key, ok); + return ok; + }; + const checkFile = (rel: string, what: string, lessonId: string | undefined, dir: boolean): boolean => { const abs = resolveInside(root, rel); if (!abs) { err(`${what} not found: ${rel}`, { lessonId, path: rel }); @@ -174,7 +183,7 @@ async function loadCourseUnsafe(manifestPath: string): Promise { const file = l.file ?? l.hints?.file; if (!file) { err(`anchor "${l.anchor}" given but the lesson has no file`, { lessonId: id }); - } else if (fileOk) { + } else if (fileOk && requireFile(file, l.file ? "lesson file" : "hints file", id)) { const md = await readMd(file); if (md !== null && extractSection(md, l.anchor) === null) { err(`anchor "${l.anchor}" not found in ${file}`, { lessonId: id, path: file }); diff --git a/packages/course-format/test/load.test.ts b/packages/course-format/test/load.test.ts index ad14892..2a8c0d6 100644 --- a/packages/course-format/test/load.test.ts +++ b/packages/course-format/test/load.test.ts @@ -1,5 +1,5 @@ import { afterAll, describe, expect, test } from "bun:test"; -import { mkdtemp, rm, symlink } from "node:fs/promises"; +import { chmod, mkdtemp, rm, symlink } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; import { createCourseRepository, discoverManifests, loadAll, loadCourse } from "../src/index"; @@ -92,20 +92,38 @@ describe("overlays and errors", () => { expect(e[0]!.path).toBe("adir"); }); - test("one throwing course does not break loadAll", async () => { + test("anchor resolved through a hints.file directory (no lesson file) gives one lesson diagnostic and loading continues", async () => { + const d = await tmp(); + await Bun.write(join(d, "adir/x.md"), "# X\n"); + const lessons = [ + " - { id: via-hints, title: L, type: exercise, anchor: sec, hints: { file: adir, match: 'E' } }", + " - { id: after, title: L, type: text, file: missing.md }", + ].join("\n"); + await Bun.write(join(d, "stcourse.yaml"), manifest("dirs2", "", lessons)); + const c = await loadCourse(join(d, "stcourse.yaml")); + const e = c.diagnostics.filter((x) => x.level === "error"); + expect(e.map((x) => x.lessonId)).toEqual(["via-hints", "after"]); + expect(e[0]!.path).toBe("adir"); + }); + + // An unreadable file passes requireFile (it is a regular file) but Bun.file().text() rejects. + test.skipIf(process.getuid?.() === 0)("one throwing course does not break loadAll", async () => { const d = await tmp(); - await Bun.write(join(d, "bad/adir/x.md"), "# X\n"); await Bun.write(join(d, "bad/a.md"), "# A\n## Sec\n"); - await Bun.write( - join(d, "bad/stcourse.yaml"), - manifest("bad", "", " - { id: h, title: L, type: exercise, file: a.md, hints: { file: adir, match: 'E' } }"), - ); + await chmod(join(d, "bad/a.md"), 0o000); + await Bun.write(join(d, "bad/stcourse.yaml"), manifest("bad", "", " - { id: l, title: L, type: text, file: a.md, anchor: sec }")); await Bun.write(join(d, "good/a.md"), "# A\n"); await Bun.write(join(d, "good/stcourse.yaml"), manifest("good", "", " - { id: l, title: L, type: text, file: a.md }")); - const all = await loadAll([d]); - expect(all.map((c) => c.id).sort()).toEqual(["bad", "good"]); - expect(all.find((c) => c.id === "bad")!.status).toBe("error"); - expect(all.find((c) => c.id === "good")!.status).toBe("ok"); + try { + const all = await loadAll([d]); + expect(all.map((c) => c.id).sort()).toEqual(["bad", "good"]); + const bad = all.find((c) => c.id === "bad")!; + expect(bad.status).toBe("error"); + expect(bad.diagnostics[0]!.message).toContain("unexpected error"); + expect(all.find((c) => c.id === "good")!.status).toBe("ok"); + } finally { + await chmod(join(d, "bad/a.md"), 0o644); + } }); test("invalid manifest gives an error course and does not throw", async () => {