fix(studio): review round 1 (symlink-safe writes, remote allowlist, upload cap/types/no-overwrite, owner rule at publish/sync/resolve, studio/repos layout and read-only imports, side-effect-free list)

C1: writes lstat every path component and refuse symlinks (dangling included),
check the parent's real path after mkdir, and open with O_NOFOLLOW.
I1: only https://, ssh:// and git@host: remotes; protocol.file/ext/fd=never on
every fetch and push; global/system git config ignored.
I2: STC_UPLOAD_MAX_MB cap (default 100) by Content-Length and while streaming;
extension allowlist (415); stcourse.yaml refused; 409 unless overwrite=true;
sandboxing CSP on draft raw files.
I3: owner-only visibility/source.kind rule checked on the merged result at
publish and sync, and at resolve; publish also validates the merged tree.
I4: bare repos in <dataDir>/studio/repos/<id>.git; imported courses read-only.
I5: list status comes from the bare repo; drafts are created lazily.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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<string, string | undefined> = 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<Uint8Array<ArrayBuffer> | 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 <dataDir>/studio/repos/<id>.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);
       }),
     },
   };