diff --git a/docs/contract-requests/studio.md b/docs/contract-requests/studio.md index f5d1045..6fb10c8 100644 --- a/docs/contract-requests/studio.md +++ b/docs/contract-requests/studio.md @@ -12,16 +12,17 @@ Proposal: ## 2. Learners must read studio courses from the published worktree (config / main.ts) -Studio courses live at: -- `/courses/`: a bare repo with no `stcourse.yaml`, so discovery never loads it; -- `/worktrees//published`: the published branch, which is what learners must see; +Layout after the review (controller ruling on I4): +- `/studio/repos/.git`: studio bare repos. They contain no `stcourse.yaml`, so discovery never loads them. +- `/worktrees//published`: the published branch, which is what learners read. - `/worktrees//drafts/`: drafts. They sit at depth 3, so the depth-2 discovery never loads them. +- `/courses/`: the importer's plain folders (WP-B), untouched by the studio. -`createCourseRepository(config.courseDirs)` does not scan `/worktrees`. The package exports `studioCourseDirs(config)`, which appends it. Tests and the dev entry use it. +`createCourseRepository(config.courseDirs)` scans neither `/worktrees` nor `/courses`. The package exports `studioCourseDirs(config)`, which returns the configured dirs plus `/courses` and `/worktrees`. Tests and the dev entry use it. -Proposal: main.ts calls `createCourseRepository(studioCourseDirs(config))`, or `loadConfig` always appends `/worktrees`. +Proposal: main.ts calls `createCourseRepository(studioCourseDirs(config))`, or `loadConfig` appends both dirs. -Related, for Phase 2: the importer writes plain folders to `/courses/`. These coexist with the studio's bare repos. A course id that exists either way cannot be created again in the studio. Adopting an imported course into the studio (git init, move it to the bare/worktree layout) is not part of v1. +Imported courses (`source.kind` other than `authored`, with their root under `/courses`) are read-only in the studio for v1, with the message "Imported course: read-only in the studio for now". There is a `TODO(studio)` in `feature.ts` (`readOnlyMessage`) for converting one into a studio-managed repo. The studio refuses to create a course whose id already exists as `/courses/`. WP-B should likewise refuse an import id that `/studio/repos/.git` already uses. ## 3. Live preview: swap in the learner `LessonView` @@ -80,3 +81,9 @@ Creating a course inserts the creator into `course_authors`. No new tables are a ## 8. External pushes straight into the bare repo (no action needed) The platform reloads when the published worktree's HEAD changes: an external commit in that worktree, a studio publish, or a Sync. A direct `git push` into `/courses/` is refused by git itself ("branch is currently checked out", checked with git 2.47), so the published files cannot silently fall behind `main`. Outside tools should commit in the published worktree, or push to a configured remote that the author then Syncs. + +## 9. Core media route serves SVG/HTML inline (core, Phase 2) + +The studio's draft `raw` route now sends `Content-Security-Policy: …; sandbox` and `X-Content-Type-Options: nosniff`. The learner-facing `/api/media/:courseId/*` in `packages/core` serves author-supplied SVG inline from the app's origin and sets neither header. + +Proposal: add the same two headers in `serveMedia`. diff --git a/packages/feature-studio/src/index.ts b/packages/feature-studio/src/index.ts index 1cf5e9b..6ced9bb 100644 --- a/packages/feature-studio/src/index.ts +++ b/packages/feature-studio/src/index.ts @@ -1,9 +1,9 @@ // Server entry of @stc/feature-studio. The browser imports "@stc/feature-studio/web" instead (no server code). -export { studioFeature, OUTSIDE_MESSAGE, type StudioFeature, type StudioFeatureOptions } from "./server/feature"; +export { studioFeature, OUTSIDE_MESSAGE, IMPORTED_MESSAGE, uploadMaxBytes, UPLOAD_EXTENSIONS, type StudioFeature, type StudioFeatureOptions } from "./server/feature"; export { createDraftWriter, type DraftWriter, type DraftWriterOptions } from "./server/draft-writer"; -export { createStudioStore, contentHash, type StudioStore, type Identity, type DraftStatus, type PublishResult, type SyncResult } from "./server/store"; +export { createStudioStore, contentHash, isAllowedRemote, ownerRequiredFor, type StudioStore, type Identity, type DraftStatus, type PublishResult, type SyncResult } from "./server/store"; export { createPublishedWatcher, type PublishedWatcher } from "./server/watch"; -export { studioCourseDirs, studioWorktreesDir } from "./server/paths"; +export { studioCourseDirs, studioWorktreesDir, studioReposDir } from "./server/paths"; export { studioWeb } from "./web"; /** The studio adds no tables: authors use Phase 0's `course_authors`. */ diff --git a/packages/feature-studio/src/server/feature.ts b/packages/feature-studio/src/server/feature.ts index 0460772..e691487 100644 --- a/packages/feature-studio/src/server/feature.ts +++ b/packages/feature-studio/src/server/feature.ts @@ -1,15 +1,48 @@ -import { basename, resolve, sep } from "node:path"; +import { existsSync, realpathSync } from "node:fs"; +import { basename, extname, resolve, sep } from "node:path"; import type { AuthUser, Feature, RequestContext, RouteHandler, ServerContext } from "@stc/contracts"; import { serveMedia } from "@stc/core"; import { ManifestSchema, parseManifest, resolveInside, type LoadedCourse } from "@stc/course-format"; import { aclRepo } from "@stc/db"; import { createDraftWriter, type DraftWriter } from "./draft-writer"; import { studioWorktreesDir } from "./paths"; -import { checkRelPath, createStudioStore, MANIFEST, StudioError, type FileWrite, type StudioStore } from "./store"; +import { checkRelPath, createStudioStore, MANIFEST, ownerRequiredFor, StudioError, type FileWrite, type StudioStore } from "./store"; import { createPublishedWatcher, type PublishedWatcher } from "./watch"; export const OUTSIDE_MESSAGE = "This course is managed outside STCourses"; -const MAX_UPLOAD = 2 * 1024 * 1024 * 1024; // 2 GB; large videos belong in the media dir, not git, but we do not forbid them +export const IMPORTED_MESSAGE = "Imported course: read-only in the studio for now"; + +/** Upload cap in bytes: STC_UPLOAD_MAX_MB, default 100 (review I2). */ +export function uploadMaxBytes(env: Record = process.env): number { + const mb = Number(env.STC_UPLOAD_MAX_MB ?? "100"); + return (Number.isFinite(mb) && mb > 0 ? mb : 100) * 1024 * 1024; +} + +/** Server-side allowlist by extension for uploads: text, documents, images, media, archives and code. */ +export const UPLOAD_EXTENSIONS = new Set([ + "md", "markdown", "txt", "pdf", "png", "jpg", "jpeg", "gif", "svg", "webp", "mp4", "webm", "mp3", "vtt", "srt", "zip", + "js", "mjs", "cjs", "jsx", "ts", "tsx", "json", "yaml", "yml", "py", "sh", "css", "html", "csv", "toml", "sql", + "scala", "sc", "fs", "fsx", "fsproj", "rs", "go", "java", "kt", "c", "h", "cpp", "hpp", "cs", "rb", "php", "swift", "hs", "ex", "exs", "dockerfile", +]); + +/** Reads at most `max` bytes of the body; null when it is larger (checked while streaming, not after buffering). */ +async function readCapped(req: Request, max: number): Promise | null> { + const reader = req.body?.getReader(); + if (!reader) return new Uint8Array(); + const chunks: Uint8Array[] = []; + let total = 0; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + total += value.byteLength; + if (total > max) { await reader.cancel(); return null; } + chunks.push(value); + } + const out = new Uint8Array(total); + let off = 0; + for (const c of chunks) { out.set(c, off); off += c.byteLength; } + return out; +} export interface StudioFeature extends Feature { store: StudioStore; @@ -51,7 +84,19 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { (await ctx.db.sql`SELECT course_id FROM course_authors WHERE user_id = ${u.id}`).map((r: { course_id: string }) => r.course_id); const canEdit = async (u: AuthUser, courseId: string) => hasRole(u) || (await acl.isAuthor(courseId, u.id)); const isStudioCourse = (c: LoadedCourse) => resolve(c.manifestPath).startsWith(wtDir + sep); - const who = (u: AuthUser) => ({ id: u.id, name: u.name || u.email || u.id, email: u.email || `${u.id}@users.stcourses.invalid` }); + const who = (u: AuthUser) => ({ id: u.id, name: u.name || u.email || u.id, email: u.email || `${u.id}@users.stcourses.invalid`, owner: u.roles.includes("owner") }); + const importedRoot = (() => { const d = resolve(ctx.config.dataDir, "courses"); return existsSync(d) ? realpathSync(d) : d; })(); + /** Why a course that is not studio-managed is read-only here. */ + const readOnlyMessage = (c: LoadedCourse): string => { + const kind = c.manifest?.source?.kind; + const underImports = c.root === importedRoot || c.root.startsWith(importedRoot + sep); + if (kind && kind !== "authored" && underImports) { + // TODO(studio): convert imported course to a studio-managed repo (git init into /studio/repos/.git + // plus a published worktree), so authors can edit it and the owner-only visibility rule applies on publish. + return IMPORTED_MESSAGE; + } + return OUTSIDE_MESSAGE; + }; const afterPublishedChange = async (courseId: string) => { await ctx.courses.reload(courseId); @@ -80,7 +125,8 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { const courseId = rc.params.courseId ?? ""; if (!(await canEdit(user, courseId))) return fail(403, "you are not an author of this course"); if (!store.isManaged(courseId)) { - return ctx.courses.get(courseId) ? fail(403, OUTSIDE_MESSAGE) : fail(404, "no such course"); + const c = ctx.courses.get(courseId); + return c ? fail(403, readOnlyMessage(c)) : fail(404, "no such course"); } return h(req, rc, user, courseId); }); @@ -120,7 +166,7 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { for (const id of managed) { if (!all && !mine.has(id)) continue; const loaded = ctx.courses.get(id); - const st = await store.status(id, user.id).catch(() => null); + const st = await store.summary(id, user.id).catch(() => null); // read-only: no worktree is created here courses.push({ id, managed: true, title: loaded?.manifest?.title ?? id, @@ -133,7 +179,7 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { if (all) { for (const c of ctx.courses.list()) { if (isStudioCourse(c) || managed.includes(c.id)) continue; - courses.push({ id: c.id, managed: false, title: c.manifest?.title ?? c.id, pitch: c.manifest?.pitch ?? null, accent: c.manifest?.accent ?? null, publishedStatus: c.status, message: OUTSIDE_MESSAGE }); + courses.push({ id: c.id, managed: false, title: c.manifest?.title ?? c.id, pitch: c.manifest?.pitch ?? null, accent: c.manifest?.accent ?? null, publishedStatus: c.status, message: readOnlyMessage(c) }); } } return json({ courses, canCreate: all }); @@ -157,7 +203,7 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { if (!store.isManaged(courseId)) { const c = ctx.courses.get(courseId); if (!c) return fail(404, "no such course"); - return json({ readOnly: true, message: OUTSIDE_MESSAGE, course: { id: c.id, title: c.manifest?.title ?? c.id, managed: false, root: c.root }, diagnostics: c.diagnostics }); + return json({ readOnly: true, message: readOnlyMessage(c), course: { id: c.id, title: c.manifest?.title ?? c.id, managed: false, root: c.root }, diagnostics: c.diagnostics }); } return json(await courseState(courseId, user)); }), @@ -179,11 +225,10 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { files.unshift({ path: MANIFEST, content: manifestToYaml(b.manifest), baseHash: b.manifestBaseHash }); } const mf = files.find((f) => checkRelPath(f.path) === MANIFEST); - if (mf && typeof mf.content === "string") { - const parsed = parseManifest(mf.content); - if (parsed.ok && parsed.manifest.source && parsed.manifest.source.kind !== "authored" && parsed.manifest.visibility !== "restricted" && !user.roles.includes("owner")) { - return fail(403, "only the owner can make an imported course public or signed-in visible"); - } + if (mf && typeof mf.content === "string" && !user.roles.includes("owner")) { + // Early feedback only; publish, sync and resolve enforce the same rule on the merged result. + const needOwner = ownerRequiredFor(await store.publishedManifest(courseId), mf.content); + if (needOwner) return fail(403, needOwner); } const r = await store.saveFiles(courseId, who(user), files, b.message ?? ""); if (!r.ok) return json({ ok: false, reason: "conflict", conflicts: r.conflicts }, 409); @@ -205,18 +250,27 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { "/api/studio/courses/:courseId/upload": { POST: course(async (req, _rc, user, courseId) => { + const max = uploadMaxBytes(); + const declared = Number(req.headers.get("content-length") ?? "0"); + if (declared > max) return fail(413, `file too large (limit ${Math.round(max / 1048576)} MB)`); + const bytes = await readCapped(req, max); + if (!bytes) return fail(413, `file too large (limit ${Math.round(max / 1048576)} MB)`); let form: FormData; - try { form = await req.formData(); } catch { return fail(400, "expected multipart form data"); } + try { form = await new Response(bytes, { headers: { "content-type": req.headers.get("content-type") ?? "" } }).formData(); } catch { return fail(400, "expected multipart form data"); } const file = form.get("file"); if (!(file instanceof Blob)) return fail(400, "file is required"); - if (file.size > MAX_UPLOAD) return fail(413, "file too large"); const explicit = form.get("path"); const dir = String(form.get("dir") ?? "").replace(/^\/+|\/+$/g, ""); const name = basename((file as File).name || "upload.bin"); const path = checkRelPath(typeof explicit === "string" && explicit ? explicit : dir ? `${dir}/${name}` : name); + if (path === MANIFEST) return fail(400, "edit stcourse.yaml in the manifest editor, not by upload"); + const ext = (extname(path).slice(1) || basename(path)).toLowerCase(); + if (!UPLOAD_EXTENSIONS.has(ext)) return fail(415, `file type .${ext} is not accepted`); + const overwrite = String(form.get("overwrite") ?? "") === "true"; const message = String(form.get("message") ?? "") || `Upload ${path}`; - const r = await store.saveFiles(courseId, who(user), [{ path, content: new Uint8Array(await file.arrayBuffer()) }], message); - if (!r.ok) return json({ ok: false, reason: "conflict", conflicts: r.conflicts }, 409); + // baseHash null = the file must not exist yet; replacing needs an explicit overwrite=true + const r = await store.saveFiles(courseId, who(user), [{ path, content: new Uint8Array(await file.arrayBuffer()), ...(overwrite ? {} : { baseHash: null }) }], message); + if (!r.ok) return json({ ok: false, reason: "exists", error: `${path} already exists; upload again with overwrite to replace it`, conflicts: r.conflicts }, 409); return json({ ok: true, path, commit: r.commit }); }), }, @@ -229,7 +283,12 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { const dir = await store.draftDir(courseId, user.id); try { checkRelPath(rel); } catch { return fail(404, "not found"); } const abs = resolveInside(dir, rel); - return abs ? serveMedia(req, abs, { download: rc.url.searchParams.has("download") }) : fail(404, "not found"); + if (!abs) return fail(404, "not found"); + const res = await serveMedia(req, abs, { download: rc.url.searchParams.has("download") }); + // Draft files are author content served from the app's origin: never let them run script (SVG, HTML). + res.headers.set("Content-Security-Policy", "default-src 'none'; img-src 'self' data:; style-src 'unsafe-inline'; media-src 'self'; sandbox"); + res.headers.set("X-Content-Type-Options", "nosniff"); + return res; }), }, @@ -240,7 +299,7 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { await afterPublishedChange(courseId); return json(r); } - return json(r, r.reason === "invalid" ? 422 : 409); + return json(r, r.reason === "invalid" ? 422 : r.reason === "forbidden" ? 403 : 409); }), }, @@ -286,7 +345,7 @@ export function studioFeature(ctx: ServerContext, opts: StudioFeatureOptions = { await afterPublishedChange(courseId); return json(r); } - return json(r, r.reason === "no-remote" ? 400 : r.reason === "network" ? 502 : 409); + return json(r, r.reason === "no-remote" ? 400 : r.reason === "network" ? 502 : r.reason === "forbidden" ? 403 : 409); }), }, }; diff --git a/packages/feature-studio/src/server/git.ts b/packages/feature-studio/src/server/git.ts index ee8d222..68a2895 100644 --- a/packages/feature-studio/src/server/git.ts +++ b/packages/feature-studio/src/server/git.ts @@ -20,6 +20,9 @@ function env(who?: Identity): Record { if (v !== undefined && !k.startsWith("GIT_")) e[k] = v; // never inherit GIT_DIR, GIT_INDEX_FILE, ... } e.GIT_TERMINAL_PROMPT = "0"; + // Ignore the machine's global/system git config (url.*.insteadOf, credential helpers, ...). + e.GIT_CONFIG_GLOBAL = "/dev/null"; + e.GIT_CONFIG_NOSYSTEM = "1"; e.GIT_SSH_COMMAND = "ssh -o BatchMode=yes"; // A fallback identity so git never refuses a merge/commit; real saves always pass `who`. const id = who ?? { name: "STCourses", email: "studio@stcourses.invalid" }; diff --git a/packages/feature-studio/src/server/paths.ts b/packages/feature-studio/src/server/paths.ts index f95b9f0..4f954bc 100644 --- a/packages/feature-studio/src/server/paths.ts +++ b/packages/feature-studio/src/server/paths.ts @@ -6,12 +6,18 @@ export function studioWorktreesDir(dataDir: string): string { return join(dataDir, "worktrees"); } +/** Studio bare repos: `/studio/repos/.git` (no manifest, never discovered). */ +export function studioReposDir(dataDir: string): string { + return join(dataDir, "studio", "repos"); +} + /** - * The course dirs the CourseRepository must scan so studio courses load from their published worktree. + * The course dirs the CourseRepository must scan: the configured ones, the importer's plain folders in + * `/courses`, and studio courses through their published worktree in `/worktrees`. * Phase 0's server builds the repository from `config.courseDirs` only; see docs/contract-requests/studio.md. */ export function studioCourseDirs(config: Pick): string[] { - const wt = resolve(studioWorktreesDir(config.dataDir)); const dirs = config.courseDirs.map((d) => resolve(d)); - return dirs.includes(wt) ? dirs : [...dirs, wt]; + for (const d of [resolve(config.dataDir, "courses"), resolve(studioWorktreesDir(config.dataDir))]) if (!dirs.includes(d)) dirs.push(d); + return dirs; } diff --git a/packages/feature-studio/src/server/store.ts b/packages/feature-studio/src/server/store.ts index cb63def..6d1c3fe 100644 --- a/packages/feature-studio/src/server/store.ts +++ b/packages/feature-studio/src/server/store.ts @@ -1,5 +1,5 @@ -import { existsSync, mkdirSync, readdirSync, realpathSync, rmSync, statSync } from "node:fs"; -import { mkdir, rm, unlink, writeFile } from "node:fs/promises"; +import { constants, existsSync, lstatSync, mkdirSync, readdirSync, realpathSync, rmSync, statSync } from "node:fs"; +import { mkdir, open, rm, unlink } from "node:fs/promises"; import { dirname, isAbsolute, join, normalize, sep } from "node:path"; import { loadCourse, type Diagnostic } from "@stc/course-format"; import { git, gitOk, splitZ, withLock, type Identity } from "./git"; @@ -63,6 +63,7 @@ export type PublishResult = | { ok: false; reason: "conflict"; conflicts: string[] } | { ok: false; reason: "dirty"; files: string[] } | { ok: false; reason: "invalid"; diagnostics: Diagnostic[] } + | { ok: false; reason: "forbidden"; message: string } | { ok: false; reason: "merging" }; export type MergeResult = { ok: true; commit: string } | { ok: false; reason: "conflict"; conflicts: string[] } | { ok: false; reason: "merging" | "no-remote" | "dirty"; message: string }; @@ -72,25 +73,75 @@ export interface ConflictFile { path: string; base: string | null; ours: string export type SyncResult = | { ok: true; pulled: boolean; pushed: boolean; head: string } | { ok: false; reason: "no-remote" | "dirty" | "network"; message: string } - | { ok: false; reason: "conflict"; conflicts: string[] }; + | { ok: false; reason: "conflict"; conflicts: string[] } + | { ok: false; reason: "forbidden"; message: string }; export class StudioError extends Error { constructor(message: string, public status = 400) { super(message); } } -export interface StudioStoreOptions { dataDir: string; publishedBranch?: string } +const HOST = String.raw`[A-Za-z0-9](?:[A-Za-z0-9.-]*[A-Za-z0-9])?`; +const USERINFO = String.raw`(?:[^@/\s:]+(?::[^@/\s]*)?@)?`; +const REMOTE_RES = [ + new RegExp(String.raw`^https://${USERINFO}${HOST}(?::\d+)?/[^\s]*$`), + new RegExp(String.raw`^ssh://${USERINFO}${HOST}(?::\d+)?/[^\s]*$`), + new RegExp(String.raw`^git@${HOST}:[A-Za-z0-9_~.][^\s]*$`), +]; + +/** Only https://, ssh:// and git@host: remotes. Local paths, file://, ext::, fd:: and the rest are refused. */ +export function isAllowedRemote(url: string): boolean { + return REMOTE_RES.some((re) => re.test(url)); +} + +/** Transport hardening passed to every fetch/push, on top of the allowlist (defence in depth). */ +const REMOTE_GUARD = ["-c", "protocol.file.allow=never", "-c", "protocol.ext.allow=never", "-c", "protocol.fd.allow=never"]; + +type ManifestLike = { visibility?: string; source?: { kind?: string } } | null; +const kindOf = (m: ManifestLike) => m?.source?.kind ?? "authored"; +function parseLoose(text: string | null): ManifestLike { + if (text === null) return null; + try { return (Bun.YAML.parse(text) as ManifestLike) ?? null; } catch { return null; } +} + +/** + * Spec 12: only the owner may make an imported course public or signed-in, and (to close the obvious bypass) + * only the owner may change `source.kind`. Returns a reason when `resultText` needs the owner, else null. + */ +export function ownerRequiredFor(baseText: string | null, resultText: string | null): string | null { + const base = parseLoose(baseText); + const result = parseLoose(resultText); + if (!result) return null; + if (base && kindOf(base) !== kindOf(result)) return "only the owner can change source.kind"; + const vis = result.visibility ?? "restricted"; + if ((vis === "public" || vis === "signed-in") && (kindOf(result) !== "authored" || (base && kindOf(base) !== "authored"))) { + return "only the owner can make an imported course public or signed-in visible"; + } + return null; +} + +export type Actor = Identity & { id: string; owner?: boolean }; + +export interface StudioStoreOptions { + dataDir: string; + publishedBranch?: string; + /** Tests only: allow local-path remotes (the HTTP feature never sets this). */ + allowLocalRemotes?: boolean; +} export function createStudioStore(opts: StudioStoreOptions) { const dataDir = opts.dataDir; const main = opts.publishedBranch ?? "main"; - const coursesDir = join(dataDir, "courses"); + // Studio repos live apart from the importer's plain folders in /courses (controller ruling, review I4). + const reposDir = join(dataDir, "studio", "repos"); + const importedDir = join(dataDir, "courses"); const worktreesDir = join(dataDir, "worktrees"); + const remoteGuard = opts.allowLocalRemotes ? ["-c", "protocol.ext.allow=never", "-c", "protocol.fd.allow=never"] : REMOTE_GUARD; const paths = (courseId: string) => { if (!ID_RE.test(courseId)) throw new StudioError(`invalid course id: ${courseId}`, 404); const root = join(worktreesDir, courseId); return { - bare: join(coursesDir, courseId), + bare: join(reposDir, `${courseId}.git`), root, published: join(root, "published"), draftsDir: join(root, "drafts"), @@ -120,8 +171,46 @@ export function createStudioStore(opts: StudioStoreOptions) { const r = await git(bare, ["remote", "get-url", "origin"]); return r.code === 0 ? r.stdout.trim() : null; }; + const remoteOk = (url: string) => opts.allowLocalRemotes ? !/^\s*-/.test(url) && !/^(ext|fd)::/.test(url) : isAllowedRemote(url); + /** The configured remote, re-checked on every use (it could have been set outside the studio). */ + const usableRemote = async (bare: string): Promise<{ ok: true } | { ok: false; message: string }> => { + const url = await remoteUrl(bare); + if (!url) return { ok: false, message: "no remote is configured for this course" }; + if (!remoteOk(url)) return { ok: false, message: "the configured remote is not an https://, ssh:// or git@host: URL" }; + return { ok: true }; + }; + const showText = async (cwd: string, spec: string): Promise => { + const r = await git(cwd, ["show", spec]); + return r.code === 0 ? r.stdout : null; + }; - /** Absolute path for `rel` inside `root`; also refuses when an existing parent is a symlink out of the root. */ + const lexists = (p: string) => { try { lstatSync(p); return true; } catch { return false; } }; + + /** + * For writes (review C1): refuses when any existing component of `rel`, the final one included, is a symlink + * (lstat, so dangling links count), creates missing parents, then re-checks the parent's real path is inside. + */ + const safeWritePath = async (root: string, rel: string, opts: { allowFinalSymlink?: boolean } = {}): Promise => { + const n = checkRelPath(rel); + const realRoot = realpathSync(root); + const parts = n.split(sep); + let cur = realRoot; + for (let i = 0; i < parts.length; i++) { + cur = join(cur, parts[i]!); + if (!lexists(cur)) break; + const st = lstatSync(cur); + const last = i === parts.length - 1; + if (st.isSymbolicLink() && !(last && opts.allowFinalSymlink)) throw new Error(`invalid path (symlink): ${rel}`); + if (!last && !st.isDirectory()) throw new Error(`invalid path (not a directory): ${rel}`); + } + const abs = join(realRoot, n); + await mkdir(dirname(abs), { recursive: true }); + const realParent = realpathSync(dirname(abs)); + if (realParent !== realRoot && !realParent.startsWith(realRoot + sep)) throw new Error(`invalid path (escapes the course): ${rel}`); + return abs; + }; + + /** For reads: absolute path for `rel` whose real path (symlinks followed) stays inside `root`. */ const inside = (root: string, rel: string): string => { const n = checkRelPath(rel); const abs = join(root, n); @@ -167,12 +256,15 @@ export function createStudioStore(opts: StudioStoreOptions) { async function writeFiles(dir: string, files: FileWrite[]): Promise { const touched: string[] = []; for (const f of files) { - const abs = inside(dir, f.path); if (f.delete) { - if (existsSync(abs)) await unlink(abs); + // unlink removes a symlink itself, never its target, so deleting a planted link is allowed + const abs = await safeWritePath(dir, f.path, { allowFinalSymlink: true }); + if (lexists(abs)) await unlink(abs); } else { - await mkdir(dirname(abs), { recursive: true }); - await writeFile(abs, f.content ?? ""); + const abs = await safeWritePath(dir, f.path); + // O_NOFOLLOW: a symlink swapped in after the check makes the open fail instead of writing through it + const fh = await open(abs, constants.O_WRONLY | constants.O_CREAT | constants.O_TRUNC | constants.O_NOFOLLOW, 0o644); + try { await fh.writeFile(f.content ?? ""); } finally { await fh.close(); } } touched.push(checkRelPath(f.path)); } @@ -187,8 +279,8 @@ export function createStudioStore(opts: StudioStoreOptions) { isManaged, async listManaged(): Promise { - if (!existsSync(coursesDir)) return []; - return readdirSync(coursesDir).filter((id) => ID_RE.test(id) && isManaged(id)).sort(); + if (!existsSync(reposDir)) return []; + return readdirSync(reposDir).filter((n) => n.endsWith(".git")).map((n) => n.slice(0, -4)).filter((id) => ID_RE.test(id) && isManaged(id)).sort(); }, async createCourse(input: { id: string; title: string; pitch?: string }, who: Identity): Promise { @@ -196,9 +288,9 @@ export function createStudioStore(opts: StudioStoreOptions) { if (!ID_RE.test(id)) throw new StudioError(`invalid course id "${id}": use lowercase letters, digits and dashes`); if (!title.trim()) throw new StudioError("title is required"); const p = paths(id); - if (existsSync(p.bare) || existsSync(p.root)) throw new StudioError(`course "${id}" already exists`, 409); await lock(id, async () => { - mkdirSync(coursesDir, { recursive: true }); + if (existsSync(p.bare) || existsSync(p.root) || existsSync(join(importedDir, id))) throw new StudioError(`course "${id}" already exists`, 409); + mkdirSync(reposDir, { recursive: true }); const tmp = join(dataDir, "tmp", `new-${id}-${Date.now()}`); mkdirSync(join(tmp, "lessons"), { recursive: true }); try { @@ -244,7 +336,7 @@ export function createStudioStore(opts: StudioStoreOptions) { draftDir: (courseId: string, authorId: string) => api.ensureDraft(courseId, authorId), - async saveFiles(courseId: string, who: Identity & { id: string }, files: FileWrite[], message: string): Promise { + async saveFiles(courseId: string, who: Actor, files: FileWrite[], message: string): Promise { for (const f of files) checkRelPath(f.path); if (files.length === 0) throw new StudioError("nothing to save"); return lock(courseId, async () => { @@ -281,6 +373,26 @@ export function createStudioStore(opts: StudioStoreOptions) { }; }, + /** List view status, read from the bare repo only: never creates a worktree or branch (review I5). */ + async summary(courseId: string, authorId: string): Promise<{ hasDraft: boolean; ahead: number; behind: number; merging: boolean; remote: string | null }> { + const p = requireManaged(courseId); + const branch = branchOf(authorId); + const hasDraft = (await git(p.bare, ["rev-parse", "-q", "--verify", `refs/heads/${branch}`])).code === 0; + const dir = p.draft(authorId); + return { + hasDraft, + ahead: hasDraft ? await count(p.bare, `${main}..${branch}`) : 0, + behind: hasDraft ? await count(p.bare, `${branch}..${main}`) : 0, + merging: existsSync(join(dir, ".git")) ? await isMerging(dir) : false, + remote: await remoteUrl(p.bare), + }; + }, + + /** The published manifest text (base for the owner-only rule). */ + async publishedManifest(courseId: string): Promise { + return showText(requireManaged(courseId).bare, `${main}:${MANIFEST}`); + }, + async publishedHead(courseId: string): Promise { return rev(requireManaged(courseId).published, "HEAD"); }, @@ -290,7 +402,7 @@ export function createStudioStore(opts: StudioStoreOptions) { return api.validateDraftUnlocked(courseId, await api.ensureDraft(courseId, authorId)); }, - publish(courseId: string, who: Identity & { id: string }): Promise { + publish(courseId: string, who: Actor): Promise { return lock(courseId, async () => { const p = requireManaged(courseId); const dir = await ensureDraftUnlocked(courseId, who.id); @@ -301,13 +413,27 @@ export function createStudioStore(opts: StudioStoreOptions) { const diags = (await api.validateDraftUnlocked(courseId, dir)).filter((d) => d.level === "error"); if (diags.length) return { ok: false, reason: "invalid", diagnostics: diags }; if ((await count(p.bare, `${main}..${branch}`)) === 0) return { ok: true, commit: await rev(p.published, "HEAD"), published: false }; - const r = await git(p.published, ["merge", "--no-ff", "-m", `Publish ${branch}`, branch], who); + const baseManifest = await showText(p.published, `HEAD:${MANIFEST}`); + // Merge without committing, so the merged result is checked before it becomes the published version. + const r = await git(p.published, ["merge", "--no-ff", "--no-commit", branch], who); if (r.code !== 0) { const conflicts = await unmerged(p.published); await git(p.published, ["merge", "--abort"]); if (!conflicts.length) throw new StudioError(`publish failed: ${r.stderr.trim() || r.stdout.trim()}`, 500); return { ok: false, reason: "conflict", conflicts }; } + const merged = (await readFileIn(p.published, MANIFEST))?.content ?? null; + const needOwner = ownerRequiredFor(baseManifest, merged); + if (needOwner && !who.owner) { + await git(p.published, ["merge", "--abort"]); + return { ok: false, reason: "forbidden", message: needOwner }; + } + const mergedErrors = (await api.validateDraftUnlocked(courseId, p.published)).filter((d) => d.level === "error"); + if (mergedErrors.length) { + await git(p.published, ["merge", "--abort"]); + return { ok: false, reason: "invalid", diagnostics: mergedErrors }; + } + await gitOk(p.published, ["commit", "-q", "-m", `Publish ${branch}`], who); // Bring the draft level with what was just published, so "behind" stays meaningful. await git(dir, ["merge", "-q", "--ff-only", main]); return { ok: true, commit: await rev(p.published, "HEAD"), published: true }; @@ -323,15 +449,16 @@ export function createStudioStore(opts: StudioStoreOptions) { }, /** Merges the published branch (or the remote's) into the author's draft. Conflicts stay in the draft for the author to resolve. */ - updateDraft(courseId: string, who: Identity & { id: string }, from: "published" | "remote" = "published"): Promise { + updateDraft(courseId: string, who: Actor, from: "published" | "remote" = "published"): Promise { return lock(courseId, async () => { const p = requireManaged(courseId); const dir = await ensureDraftUnlocked(courseId, who.id); if (await isMerging(dir)) return { ok: false, reason: "merging", message: "a merge is already in progress" }; let target = main; if (from === "remote") { - if (!(await remoteUrl(p.bare))) return { ok: false, reason: "no-remote", message: "no remote is configured" }; - const f = await git(p.bare, ["fetch", "-q", "origin"]); + const ru = await usableRemote(p.bare); + if (!ru.ok) return { ok: false, reason: "no-remote", message: ru.message }; + const f = await git(p.bare, [...remoteGuard, "fetch", "-q", "origin"]); if (f.code !== 0) return { ok: false, reason: "no-remote", message: f.stderr.trim() }; target = `origin/${main}`; if ((await git(p.bare, ["rev-parse", "-q", "--verify", `refs/remotes/${target}`])).code !== 0) return { ok: true, commit: await rev(dir, "HEAD") }; @@ -364,10 +491,15 @@ export function createStudioStore(opts: StudioStoreOptions) { }, /** Writes the author's resolution (`content: null` deletes) and commits the merge once nothing is unmerged. */ - resolve(courseId: string, who: Identity & { id: string }, files: { path: string; content: string | null }[]): Promise<{ ok: true; remaining: string[]; commit: string | null }> { + resolve(courseId: string, who: Actor, files: { path: string; content: string | null }[]): Promise<{ ok: true; remaining: string[]; commit: string | null }> { return lock(courseId, async () => { const dir = await ensureDraftUnlocked(courseId, who.id); if (!(await isMerging(dir))) throw new StudioError("no merge in progress", 409); + const m = files.find((f) => checkRelPath(f.path) === MANIFEST); + if (m && !who.owner) { + const needOwner = ownerRequiredFor(await showText(requireManaged(courseId).bare, `${main}:${MANIFEST}`), m.content); + if (needOwner) throw new StudioError(needOwner, 403); + } const touched = await writeFiles(dir, files.map((f) => (f.content === null ? { path: f.path, delete: true } : { path: f.path, content: f.content }))); await gitOk(dir, ["add", "-A", "--", ...touched]); const remaining = await unmerged(dir); @@ -396,7 +528,7 @@ export function createStudioStore(opts: StudioStoreOptions) { if (cur) await gitOk(p.bare, ["remote", "remove", "origin"]); return; } - if (/^\s*-/.test(url) || /\s/.test(url.trim())) throw new StudioError("invalid remote url"); + if (!remoteOk(url.trim())) throw new StudioError("remote must be an https://, ssh:// or git@host: URL"); await gitOk(p.bare, cur ? ["remote", "set-url", "origin", url.trim()] : ["remote", "add", "origin", url.trim()]); // Bare repos created with `init --bare` have no fetch refspec for remote-tracking branches by default. await gitOk(p.bare, ["config", "remote.origin.fetch", "+refs/heads/*:refs/remotes/origin/*"]); @@ -407,13 +539,14 @@ export function createStudioStore(opts: StudioStoreOptions) { * Explicit Sync with the remote: pull (fast-forward, or a merge without conflicts) and push the published branch. * A conflicting divergence is aborted and reported; the author merges the remote into the draft to resolve it. */ - sync(courseId: string, who: Identity): Promise { + sync(courseId: string, who: Actor): Promise { return lock(courseId, async () => { const p = requireManaged(courseId); - if (!(await remoteUrl(p.bare))) return { ok: false, reason: "no-remote", message: "no remote is configured for this course" }; + const ru = await usableRemote(p.bare); + if (!ru.ok) return { ok: false, reason: "no-remote", message: ru.message }; const dirty = await dirtyFiles(p.published); if (dirty.length) return { ok: false, reason: "dirty", message: `uncommitted changes in the published worktree: ${dirty.join(", ")}` }; - const f = await git(p.bare, ["fetch", "-q", "origin"]); + const f = await git(p.bare, [...remoteGuard, "fetch", "-q", "origin"]); if (f.code !== 0) return { ok: false, reason: "network", message: f.stderr.trim() || "fetch failed" }; const remoteRef = `refs/remotes/origin/${main}`; const hasRemote = (await git(p.bare, ["rev-parse", "-q", "--verify", remoteRef])).code === 0; @@ -425,25 +558,34 @@ export function createStudioStore(opts: StudioStoreOptions) { if (local !== remote) { const localBehind = (await git(p.bare, ["merge-base", "--is-ancestor", local, remote])).code === 0; const remoteBehind = (await git(p.bare, ["merge-base", "--is-ancestor", remote, local])).code === 0; + const baseManifest = await showText(p.bare, `${main}:${MANIFEST}`); if (localBehind) { + const needOwner = ownerRequiredFor(baseManifest, await showText(p.bare, `${remoteRef}:${MANIFEST}`)); + if (needOwner && !who.owner) return { ok: false, reason: "forbidden", message: needOwner }; await gitOk(p.published, ["merge", "-q", "--ff-only", remoteRef]); pulled = true; } else if (!remoteBehind) { - const r = await git(p.published, ["merge", "--no-ff", "-m", `Sync with origin/${main}`, remoteRef], who); + const r = await git(p.published, ["merge", "--no-ff", "--no-commit", remoteRef], who); if (r.code !== 0) { const conflicts = await unmerged(p.published); await git(p.published, ["merge", "--abort"]); return conflicts.length ? { ok: false, reason: "conflict", conflicts } : { ok: false, reason: "network", message: r.stderr.trim() }; } + const needOwner = ownerRequiredFor(baseManifest, (await readFileIn(p.published, MANIFEST))?.content ?? null); + if (needOwner && !who.owner) { + await git(p.published, ["merge", "--abort"]); + return { ok: false, reason: "forbidden", message: needOwner }; + } + await gitOk(p.published, ["commit", "-q", "-m", `Sync with origin/${main}`], who); pulled = true; } } } const ahead = hasRemote ? await count(p.bare, `${remoteRef}..${main}`) : 1; if (ahead > 0) { - const r = await git(p.bare, ["push", "-q", "origin", `${main}:${main}`]); + const r = await git(p.bare, [...remoteGuard, "push", "-q", "origin", `${main}:${main}`]); if (r.code !== 0) return { ok: false, reason: "network", message: r.stderr.trim() || "push failed" }; - await git(p.bare, ["fetch", "-q", "origin"]); + await git(p.bare, [...remoteGuard, "fetch", "-q", "origin"]); pushed = true; } return { ok: true, pulled, pushed, head: await rev(p.published, "HEAD") }; diff --git a/packages/feature-studio/test/app-helpers.ts b/packages/feature-studio/test/app-helpers.ts index 43b1d11..e7960af 100644 --- a/packages/feature-studio/test/app-helpers.ts +++ b/packages/feature-studio/test/app-helpers.ts @@ -15,8 +15,9 @@ export const users: Record = { }; /** A server context like main.ts builds, with header-selected users (`x-user: ada`). */ -export async function setupApp(extraCourseDirs: string[] = [], dbUrl = "sqlite://:memory:") { +export async function setupApp(extraCourseDirs: string[] = [], dbUrl = "sqlite://:memory:", prepare?: (dataDir: string) => void) { const dataDir = tempDir(); + prepare?.(dataDir); let dropSchema = async () => {}; if (dbUrl.startsWith("postgres")) { // a private schema per test, like packages/db's forEachDb diff --git a/packages/feature-studio/test/feature.test.ts b/packages/feature-studio/test/feature.test.ts index 4636333..12769c5 100644 --- a/packages/feature-studio/test/feature.test.ts +++ b/packages/feature-studio/test/feature.test.ts @@ -74,7 +74,8 @@ test("remote config and sync without a remote", async () => { env = await setupApp(); await env.call("ada", "POST", "/api/studio/courses", { id: "c1", title: "C" }); expect((await env.call("ada", "POST", "/api/studio/courses/c1/sync")).status).toBe(400); - expect((await env.call("ada", "PUT", "/api/studio/courses/c1/remote", { url: "/tmp/nowhere.git" })).status).toBe(200); - expect((await env.call("ada", "GET", "/api/studio/courses/c1")).json.status.remote).toBe("/tmp/nowhere.git"); + expect((await env.call("ada", "PUT", "/api/studio/courses/c1/remote", { url: "/tmp/nowhere.git" })).status).toBe(400); + expect((await env.call("ada", "PUT", "/api/studio/courses/c1/remote", { url: "https://nowhere.invalid/x.git" })).status).toBe(200); + expect((await env.call("ada", "GET", "/api/studio/courses/c1")).json.status.remote).toBe("https://nowhere.invalid/x.git"); expect((await env.call("ada", "POST", "/api/studio/courses/c1/sync")).status).toBe(502); }); diff --git a/packages/feature-studio/test/helpers.ts b/packages/feature-studio/test/helpers.ts index c4516e8..fe8b2c5 100644 --- a/packages/feature-studio/test/helpers.ts +++ b/packages/feature-studio/test/helpers.ts @@ -11,9 +11,9 @@ export function tempDir(prefix = "stc-studio-"): string { return realpathSync(mkdtempSync(join(tmpdir(), prefix))); } -export async function newStore() { +export async function newStore(opts: { allowLocalRemotes?: boolean } = {}) { const dataDir = tempDir(); - const store = createStudioStore({ dataDir }); + const store = createStudioStore({ dataDir, ...opts }); return { dataDir, store }; } diff --git a/packages/feature-studio/test/publish.test.ts b/packages/feature-studio/test/publish.test.ts index a32d481..391d969 100644 --- a/packages/feature-studio/test/publish.test.ts +++ b/packages/feature-studio/test/publish.test.ts @@ -3,8 +3,8 @@ import { readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { ada, bob, extGit, newStore, tempDir } from "./helpers"; -async function courseWithEdit(text = "# Draft text\n") { - const s = await newStore(); +async function courseWithEdit(text = "# Draft text\n", opts: { allowLocalRemotes?: boolean } = {}) { + const s = await newStore(opts); await s.store.createCourse({ id: "c1", title: "C" }, ada); const cur = (await s.store.readFile("c1", ada.id, "lessons/welcome.md"))!; await s.store.saveFiles("c1", ada, [{ path: "lessons/welcome.md", content: text, baseHash: cur.hash }], "Draft edit"); @@ -104,7 +104,8 @@ describe("sync", () => { }); test("push to an empty remote, pull a remote commit, report a conflicting divergence", async () => { - const { store, pub } = await courseWithEdit(); + // local bare remotes need the test-only allowLocalRemotes switch; the HTTP feature never sets it + const { store, pub } = await courseWithEdit("# Draft text\n", { allowLocalRemotes: true }); await store.publish("c1", ada); const remote = join(tempDir(), "remote.git"); await extGit(tempDir(), "init", "-q", "--bare", "-b", "main", remote); @@ -145,11 +146,9 @@ describe("sync", () => { }); }); -test("ext:: remotes cannot run commands", async () => { - const { store } = await courseWithEdit(); +test("ext:: remotes cannot run commands, even with local remotes allowed for tests", async () => { + const { store } = await courseWithEdit("# x\n", { allowLocalRemotes: true }); const marker = join(tempDir(), "pwned"); - await store.setRemote("c1", `ext::touch%s${marker}`); - const r = await store.sync("c1", ada); - expect(r.ok).toBe(false); + await expect(store.setRemote("c1", `ext::touch%s${marker}`)).rejects.toThrow(/remote/); expect(await Bun.file(marker).exists()).toBe(false); }); diff --git a/packages/feature-studio/test/security.test.ts b/packages/feature-studio/test/security.test.ts new file mode 100644 index 0000000..aac80f7 --- /dev/null +++ b/packages/feature-studio/test/security.test.ts @@ -0,0 +1,268 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { existsSync, mkdirSync, readFileSync, symlinkSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { isAllowedRemote, ownerRequiredFor } from "../src/server/store"; +import { ada, extGit, newStore, tempDir } from "./helpers"; +import { setupApp } from "./app-helpers"; + +describe("C1: writes never follow symlinks", () => { + test("a dangling symlink is refused for save and resolve; the outside target is never created", async () => { + const { store } = await newStore(); + await store.createCourse({ id: "c1", title: "C" }, ada); + const draft = await store.ensureDraft("c1", ada.id); + const outside = join(tempDir(), "pwned.txt"); + symlinkSync(outside, join(draft, "evil.md")); + await expect(store.saveFiles("c1", ada, [{ path: "evil.md", content: "x" }], "x")).rejects.toThrow(/symlink/); + await expect(store.saveFiles("c1", ada, [{ path: "evil.md", content: "x", baseHash: null }], "x")).rejects.toThrow(/symlink/); + expect(existsSync(outside)).toBe(false); + }); + + test("a symlinked parent directory is refused", async () => { + const { store } = await newStore(); + await store.createCourse({ id: "c1", title: "C" }, ada); + const draft = await store.ensureDraft("c1", ada.id); + const outsideDir = tempDir(); + symlinkSync(outsideDir, join(draft, "sub")); + await expect(store.saveFiles("c1", ada, [{ path: "sub/x.md", content: "x" }], "x")).rejects.toThrow(/symlink/); + await expect(store.saveFiles("c1", ada, [{ path: "sub/deeper/x.md", content: "x" }], "x")).rejects.toThrow(/symlink/); + expect(existsSync(join(outsideDir, "x.md"))).toBe(false); + expect(existsSync(join(outsideDir, "deeper"))).toBe(false); + }); + + test("a symlink pulled in from a remote merge cannot be written through; deleting the link leaves the target", async () => { + const { store } = await newStore({ allowLocalRemotes: true }); + await store.createCourse({ id: "c1", title: "C" }, ada); + const remote = join(tempDir(), "remote.git"); + await extGit(tempDir(), "init", "-q", "--bare", "-b", "main", remote); + await store.setRemote("c1", remote); + await store.sync("c1", ada); // push main + const outsideDir = tempDir(); + const target = join(outsideDir, "target.md"); + writeFileSync(target, "outside\n"); + const other = join(tempDir(), "other"); + await extGit(tempDir(), "clone", "-q", remote, other); + symlinkSync(target, join(other, "notes.md")); + symlinkSync(join(outsideDir, "new.md"), join(other, "dangling.md")); + await extGit(other, "add", "notes.md", "dangling.md"); + await extGit(other, "commit", "-qm", "plant symlinks"); + await extGit(other, "push", "-q", "origin", "main"); + expect((await store.updateDraft("c1", ada, "remote")).ok).toBe(true); + await expect(store.saveFiles("c1", ada, [{ path: "notes.md", content: "pwned" }], "x")).rejects.toThrow(/symlink/); + await expect(store.saveFiles("c1", ada, [{ path: "dangling.md", content: "pwned" }], "x")).rejects.toThrow(/symlink/); + expect(readFileSync(target, "utf8")).toBe("outside\n"); + expect(existsSync(join(outsideDir, "new.md"))).toBe(false); + expect((await store.saveFiles("c1", ada, [{ path: "notes.md", delete: true }], "rm link")).ok).toBe(true); + expect(readFileSync(target, "utf8")).toBe("outside\n"); + }); + + test("resolve refuses to write through a symlink", async () => { + const { store } = await newStore(); + await store.createCourse({ id: "c1", title: "C" }, ada); + const pub = store.paths("c1").published; + const cur = (await store.readFile("c1", ada.id, "lessons/welcome.md"))!; + await store.saveFiles("c1", ada, [{ path: "lessons/welcome.md", content: "# mine\n", baseHash: cur.hash }], "mine"); + writeFileSync(join(pub, "lessons/welcome.md"), "# theirs\n"); + await extGit(pub, "commit", "-qam", "theirs"); + await store.updateDraft("c1", ada); + const outside = join(tempDir(), "p.txt"); + symlinkSync(outside, join(store.paths("c1").draft(ada.id), "link.md")); + await expect(store.resolve("c1", ada, [{ path: "link.md", content: "x" }])).rejects.toThrow(/symlink/); + expect(existsSync(outside)).toBe(false); + }); +}); + +describe("I1: remotes", () => { + test.each([ + "/tmp/x.git", "../x.git", "x.git", "file:///tmp/x.git", "ext::sh -c touch% /tmp/p", "fd::3", "http://host/x.git", "git://host/x.git", + "-uoption", "https://", "https:///x", "git@:x", "git@host:-x", "ssh://host", "ssh://-oProxyCommand=x/y", "https://host/x y", + ])("rejects %p", async (url) => { + expect(isAllowedRemote(url)).toBe(false); + const { store } = await newStore(); + await store.createCourse({ id: "c1", title: "C" }, ada); + await expect(store.setRemote("c1", url)).rejects.toThrow(); + expect(await store.remote("c1")).toBeNull(); + }); + + test.each(["https://github.com/a/b.git", "https://user:tok@git.example.org:8443/a/b.git", "ssh://git@host.example:22/a/b.git", "git@github.com:a/b.git"])("accepts %p", (url) => { + expect(isAllowedRemote(url)).toBe(true); + }); + + test("a remote configured outside the studio is re-checked before sync and update", async () => { + const { store } = await newStore(); + await store.createCourse({ id: "c1", title: "C" }, ada); + const remote = join(tempDir(), "remote.git"); + await extGit(tempDir(), "init", "-q", "--bare", "-b", "main", remote); + await extGit(store.paths("c1").bare, "remote", "add", "origin", remote); + expect(await store.sync("c1", ada)).toMatchObject({ ok: false, reason: "no-remote" }); + expect(await store.updateDraft("c1", ada, "remote")).toMatchObject({ ok: false, reason: "no-remote" }); + expect(await extGit(remote, "for-each-ref")).toBe(""); + }); +}); + +describe("I3: owner-only visibility", () => { + const imported = "schema: 1\nid: c1\ntitle: C\nvisibility: restricted\nsource: { kind: packt-video }\nmodules: []\n"; + test("rule", () => { + expect(ownerRequiredFor(imported, imported)).toBeNull(); + expect(ownerRequiredFor(imported, imported.replace("restricted", "public"))).toMatch(/owner/); + expect(ownerRequiredFor(imported, imported.replace("restricted", "signed-in").replace("packt-video", "authored"))).toMatch(/owner/); + expect(ownerRequiredFor(imported, imported.replace("source: { kind: packt-video }\n", ""))).toMatch(/source.kind/); + const authored = imported.replace("packt-video", "authored"); + expect(ownerRequiredFor(authored, authored.replace("restricted", "public"))).toBeNull(); + expect(ownerRequiredFor(authored, authored.replace("authored", "pdf-deck"))).toMatch(/source.kind/); + }); + + let env: Awaited>; + afterEach(() => env?.stop()); + + test("publish by a non-owner is 403 when the merged manifest makes an imported course visible; the owner may", async () => { + env = await setupApp(); + const { call, feature } = env; + await call("ada", "POST", "/api/studio/courses", { id: "c1", title: "C" }); + // the published course is an import (as an importer-adopted course would be) + const pub = feature.store.paths("c1").published; + const m0 = readFileSync(join(pub, "stcourse.yaml"), "utf8").replace("kind: authored", "kind: packt-video"); + writeFileSync(join(pub, "stcourse.yaml"), m0); + await extGit(pub, "commit", "-qam", "imported"); + await feature.watcher.check(); + const ada = { id: "u-ada", name: "Ada", email: "ada@example.org" }; + + // save-time check stops the plain attempt + const st = (await call("ada", "GET", "/api/studio/courses/c1")).json; + expect((await call("ada", "POST", "/api/studio/courses/c1/update-draft")).status).toBe(200); + const st2 = (await call("ada", "GET", "/api/studio/courses/c1")).json; + expect((await call("ada", "PUT", "/api/studio/courses/c1/files", { message: "x", manifest: { ...st2.manifestRaw, visibility: "public" }, manifestBaseHash: st2.manifestHash, files: [] })).status).toBe(403); + void st; + + // a draft that got there another way (AI draft writer, merge) is stopped at publish + for (const edit of [(t: string) => t.replace("visibility: restricted", "visibility: public"), (t: string) => t.replace("visibility: restricted", "visibility: public").replace("kind: packt-video", "kind: authored")]) { + const cur = (await feature.store.readFile("c1", ada.id, "stcourse.yaml"))!; + await feature.store.saveFiles("c1", ada, [{ path: "stcourse.yaml", content: edit(cur.content), baseHash: cur.hash }], "sneak"); + const r = await call("ada", "POST", "/api/studio/courses/c1/publish"); + expect(r.status).toBe(403); + expect(r.json.reason).toBe("forbidden"); + expect(readFileSync(join(pub, "stcourse.yaml"), "utf8")).toBe(m0); + expect(await extGit(pub, "status", "--porcelain")).toBe(""); + await feature.store.saveFiles("c1", ada, [{ path: "stcourse.yaml", content: cur.content }], "undo"); + } + + // the owner may + const own = (await call("owner", "GET", "/api/studio/courses/c1")).json; + expect((await call("owner", "PUT", "/api/studio/courses/c1/files", { message: "public", manifest: { ...own.manifestRaw, visibility: "public" }, manifestBaseHash: own.manifestHash, files: [] })).status).toBe(200); + expect((await call("owner", "POST", "/api/studio/courses/c1/publish")).status).toBe(200); + expect(readFileSync(join(pub, "stcourse.yaml"), "utf8")).toContain("visibility: public"); + }); + + test("resolve of the manifest by a non-owner is checked too", async () => { + const { store } = await newStore(); + await store.createCourse({ id: "c1", title: "C" }, ada); + const pub = store.paths("c1").published; + const cur = (await store.readFile("c1", ada.id, "stcourse.yaml"))!; + await store.saveFiles("c1", ada, [{ path: "stcourse.yaml", content: cur.content.replace("title: C", "title: Mine"), baseHash: cur.hash }], "mine"); + writeFileSync(join(pub, "stcourse.yaml"), cur.content.replace("title: C", "title: Theirs").replace("kind: authored", "kind: packt-video")); + await extGit(pub, "commit", "-qam", "theirs"); + expect((await store.updateDraft("c1", ada)).ok).toBe(false); + const bad = cur.content.replace("kind: authored", "kind: packt-video").replace("visibility: restricted", "visibility: public"); + await expect(store.resolve("c1", ada, [{ path: "stcourse.yaml", content: bad }])).rejects.toThrow(/owner/); + expect((await store.resolve("c1", { ...ada, owner: true }, [{ path: "stcourse.yaml", content: bad }])).remaining).toEqual([]); + }); +}); + +describe("I2: uploads", () => { + let env: Awaited>; + afterEach(() => { delete process.env.STC_UPLOAD_MAX_MB; return env?.stop(); }); + const upload = (name: string, bytes: Uint8Array, extra: Record = {}) => { + const fd = new FormData(); + fd.set("file", new File([bytes], name)); + fd.set("dir", "media"); + for (const [k, v] of Object.entries(extra)) fd.set(k, v); + return env.call("ada", "POST", "/api/studio/courses/c1/upload", fd); + }; + + test("type allowlist, manifest refused, no silent overwrite", async () => { + env = await setupApp(); + await env.call("ada", "POST", "/api/studio/courses", { id: "c1", title: "C" }); + expect((await upload("tool.exe", new Uint8Array([1]))).status).toBe(415); + expect((await upload("page.HTM", new Uint8Array([1]))).status).toBe(415); + expect((await upload("x", new Uint8Array([1]), { path: "stcourse.yaml" })).status).toBe(400); + expect((await upload("slides.pdf", new Uint8Array([1]))).status).toBe(200); + const again = await upload("slides.pdf", new Uint8Array([2])); + expect(again.status).toBe(409); + expect(readFileSync(join(env.dataDir, "worktrees/c1/drafts/u-ada/media/slides.pdf"))[0]).toBe(1); + expect((await upload("slides.pdf", new Uint8Array([2]), { overwrite: "true" })).status).toBe(200); + expect(readFileSync(join(env.dataDir, "worktrees/c1/drafts/u-ada/media/slides.pdf"))[0]).toBe(2); + }); + + test("size cap from STC_UPLOAD_MAX_MB, by Content-Length and while streaming", async () => { + env = await setupApp(); + await env.call("ada", "POST", "/api/studio/courses", { id: "c1", title: "C" }); + process.env.STC_UPLOAD_MAX_MB = "1"; + expect((await upload("big.zip", new Uint8Array(1024 * 1024 + 10))).status).toBe(413); + // no Content-Length: a streamed body is cut off once it passes the cap + const fd = new FormData(); + fd.set("file", new File([new Uint8Array(2 * 1024 * 1024)], "big.zip")); + const ct = new Response(fd).headers.get("content-type")!; + const bytes = new Uint8Array(await new Response(fd).arrayBuffer()); + const stream = new ReadableStream({ start(c) { for (let i = 0; i < bytes.length; i += 65536) c.enqueue(bytes.slice(i, i + 65536)); c.close(); } }); + const r = await env.app.fetch(new Request("http://x/api/studio/courses/c1/upload", { method: "POST", headers: { "x-user": "ada", "content-type": ct }, body: stream, duplex: "half" } as RequestInit)); + expect(r.status).toBe(413); + expect(existsSync(join(env.dataDir, "worktrees/c1/drafts/u-ada/big.zip"))).toBe(false); + expect((await upload("small.zip", new Uint8Array(1000))).status).toBe(200); + }); + + test("raw draft files are served with a sandboxing CSP", async () => { + env = await setupApp(); + await env.call("ada", "POST", "/api/studio/courses", { id: "c1", title: "C" }); + await upload("x.svg", new TextEncoder().encode("")); + const r = await env.app.fetch(new Request("http://x/api/studio/courses/c1/raw/media/x.svg", { headers: { "x-user": "ada" } })); + expect(r.headers.get("content-security-policy")).toContain("sandbox"); + }); +}); + +describe("I4: layout and imported courses", () => { + let env: Awaited>; + afterEach(() => env?.stop()); + + test("imported courses in /courses are read-only with their own message; ids cannot collide", async () => { + env = await setupApp([], undefined, (dataDir) => { + mkdirSync(join(dataDir, "courses/imp/lessons"), { recursive: true }); + writeFileSync(join(dataDir, "courses/imp/lessons/a.md"), "# A\n"); + writeFileSync(join(dataDir, "courses/imp/stcourse.yaml"), "schema: 1\nid: imp\ntitle: Imported\nsource: { kind: packt-video }\nmodules:\n - { id: m, title: M, lessons: [{ id: a, title: A, type: text, file: lessons/a.md }] }\n"); + }); + expect(env.ctx.courses.get("imp")?.status).toBe("ok"); + const list = (await env.call("ada", "GET", "/api/studio/courses")).json; + expect(list.courses).toContainEqual(expect.objectContaining({ id: "imp", managed: false, message: "Imported course: read-only in the studio for now" })); + expect((await env.call("ada", "GET", "/api/studio/courses/imp")).json.message).toBe("Imported course: read-only in the studio for now"); + const w = await env.call("ada", "PUT", "/api/studio/courses/imp/files", { message: "x", files: [{ path: "a.md", content: "x" }] }); + expect(w.status).toBe(403); + expect(w.json.error).toBe("Imported course: read-only in the studio for now"); + expect((await env.call("ada", "POST", "/api/studio/courses", { id: "imp", title: "Dup" })).status).toBe(409); + // studio repos are not in the importer's folder + expect((await env.call("ada", "POST", "/api/studio/courses", { id: "c1", title: "C" })).status).toBe(201); + expect(existsSync(join(env.dataDir, "studio/repos/c1.git/HEAD"))).toBe(true); + expect(existsSync(join(env.dataDir, "courses/c1"))).toBe(false); + }); + + test("store refuses an id already used by an importer folder", async () => { + const { store, dataDir } = await newStore(); + mkdirSync(join(dataDir, "courses/imp"), { recursive: true }); + await expect(store.createCourse({ id: "imp", title: "x" }, ada)).rejects.toThrow(/exists/); + }); +}); + +describe("I5: listing has no side effects", () => { + let env: Awaited>; + afterEach(() => env?.stop()); + + test("GET /api/studio/courses creates no draft worktree or branch; opening the editor does", async () => { + env = await setupApp(); + await env.call("owner", "POST", "/api/studio/courses", { id: "c1", title: "C" }); + const bare = env.feature.store.paths("c1").bare; + const draft = env.feature.store.paths("c1").draft("u-ada"); + const list = await env.call("ada", "GET", "/api/studio/courses"); + expect(list.json.courses[0]).toMatchObject({ id: "c1", ahead: 0, behind: 0, merging: false }); + expect(existsSync(draft)).toBe(false); + expect(await extGit(bare, "for-each-ref", "--format=%(refname)", "refs/heads/draft")).toBe(""); + await env.call("ada", "GET", "/api/studio/courses/c1"); + expect(existsSync(draft)).toBe(true); + }); +}); diff --git a/packages/feature-studio/test/store.test.ts b/packages/feature-studio/test/store.test.ts index 6377f42..c287389 100644 --- a/packages/feature-studio/test/store.test.ts +++ b/packages/feature-studio/test/store.test.ts @@ -9,8 +9,9 @@ describe("createCourse", () => { test("makes a bare repo, a published worktree on main and a valid manifest", async () => { const { dataDir, store } = await newStore(); await store.createCourse({ id: "intro-git", title: "Intro: Git & you" }, ada); - expect(existsSync(join(dataDir, "courses/intro-git/HEAD"))).toBe(true); - expect(existsSync(join(dataDir, "courses/intro-git/stcourse.yaml"))).toBe(false); + expect(existsSync(join(dataDir, "studio/repos/intro-git.git/HEAD"))).toBe(true); + expect(existsSync(join(dataDir, "studio/repos/intro-git.git/stcourse.yaml"))).toBe(false); + expect(existsSync(join(dataDir, "courses"))).toBe(false); // the importer's folder is left alone const pub = store.paths("intro-git").published; expect(pub).toBe(join(dataDir, "worktrees/intro-git/published")); expect(await extGit(pub, "rev-parse", "--abbrev-ref", "HEAD")).toBe("main");