diff --git a/docs/contract-requests/ai.md b/docs/contract-requests/ai.md index 222d977..c29e6af 100644 --- a/docs/contract-requests/ai.md +++ b/docs/contract-requests/ai.md @@ -57,5 +57,25 @@ Proposal: export these paths from `@stc/core` (e.g. `cachePaths(dataDir)`) so bo ## 7. Learner lesson URL -Citations carry `lessonId`; the assistant links them to `/c//` (a guess at WP-A's route). Proposal: a `lessonUrl(courseId, lessonId)` -helper in `@stc/contracts/web` so WP-A owns the route shape. +Citations carry `lessonId`; the assistant links them to `/c//`. Every such link goes through +`lessonUrl(courseId, lessonId)` / `courseUrl(courseId)` in `packages/feature-ai/src/web/urls.ts` (also exported from +`@stc/feature-ai/web`), so a different WP-A route is a one-line change. Proposal: move that helper into +`@stc/contracts/web` so WP-A owns the route shape. + +## 8. A global `author` role in core's `canView` + +Generation and the studio file listing now require an authoring right (owner, the `author` role or a `course_authors` +row) and core's `canView(user, course, acl, isAuthor)`. `canView` only knows per-course authors (`isAuthor`), so a user +with the global `author` role cannot reach a `restricted` course without an ACL grant or a `course_authors` row. +Spec section 6 allows author rights "on all courses or per course", and the studio branch (`feature-studio` +`server/feature.ts:82-85`) currently grants the global role on every course. Proposal: core exports one +`canAuthor(user, course, acl, isAuthor)` (or `canView` treats `roles.includes("author")` like `isAuthor`), and both +the studio and the AI package call it. Until then the AI package follows core's `canView` (the stricter rule). + +## 9. `STC_AI_ALLOWED_BASE_URLS` in `AppConfig` + +Learners' own keys (BYOK) go only to `https://api.anthropic.com`, `https://api.openai.com/v1` or an origin on the +allowlist (SSRF guard). The allowlist is `STC_AI_ALLOWED_BASE_URLS` (comma-separated exact origins) plus the admin +setting `allowedBaseUrls` in the server AI config. `AppConfig` is frozen, so the package reads the variable from +`process.env` (injectable through `aiFeature(ctx, { env })`). Proposal: add `aiAllowedBaseUrls: string[]` to +`AppConfig` and `loadConfig`. diff --git a/packages/feature-ai/src/feature.ts b/packages/feature-ai/src/feature.ts index ad521f8..d17f3a7 100644 --- a/packages/feature-ai/src/feature.ts +++ b/packages/feature-ai/src/feature.ts @@ -184,8 +184,17 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat const ok = canView(user, c, await acl.listForCourse(c.id), user ? await acl.isAuthor(c.id, user.id) : false); return ok ? c : null; }; - const canAuthor = async (user: AuthUser, courseId: string) => - user.roles.includes("owner") || user.roles.includes("author") || (await acl.isAuthor(courseId, user.id)); + /** + * Generation and the studio file listing: the user must hold an authoring right (owner, the `author` role, or a + * `course_authors` row) AND see the course under core's `canView`, the same visibility rule as everywhere else. + * A global `author` therefore cannot reach a restricted course without an ACL grant or a course_authors row + * (see contract request 8). + */ + const canAuthor = async (user: AuthUser, course: LoadedCourse) => { + const isAuthorRow = await acl.isAuthor(course.id, user.id); + const mayAuthor = user.roles.includes("owner") || user.roles.includes("author") || isAuthorRow; + return mayAuthor && canView(user, course, await acl.listForCourse(course.id), isAuthorRow); + }; const conversationOf = async (id: string, userId: string) => { const rows = await ctx.db.sql`SELECT id, user_id, course_id, title, created_at FROM ai_conversations WHERE id = ${id} AND user_id = ${userId}`; @@ -427,7 +436,7 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat if (!rc.user) return err(401, "Sign in first."); const out = []; for (const c of ctx.courses.list()) { - if (c.status === "ok" && (await canAuthor(rc.user, c.id))) out.push({ id: c.id, title: c.manifest?.title ?? c.id, tracks: c.manifest?.tracks ?? [] }); + if (c.status === "ok" && (await canAuthor(rc.user, c))) out.push({ id: c.id, title: c.manifest?.title ?? c.id, tracks: c.manifest?.tracks ?? [] }); } return json(out); }, @@ -438,7 +447,7 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat const courseId = rc.url.searchParams.get("courseId") ?? ""; const course = ctx.courses.get(courseId); if (!course || course.status !== "ok") return err(404, "Course not found."); - if (!(await canAuthor(rc.user, course.id))) return err(403, "Only authors can generate drafts."); + if (!(await canAuthor(rc.user, course))) return err(403, "Only authors can generate drafts."); const lessons = (course.manifest?.modules ?? []).flatMap((m) => m.lessons.map((l) => ({ id: l.id, title: l.title, type: l.type, module: m.title, hasText: l.type !== "video" || !!l.transcript }))); const files = await listFiles(course.root, 800); // authors may use any course file, solutions included const cache = await listFiles(join(ctx.config.dataDir, "cache", course.id), 400); @@ -453,7 +462,7 @@ export function aiFeature(ctx: ServerContext, opts: AiFeatureOptions = {}): Feat if (!g) return err(400, "Choose a course, a kind (document, exercise or questions), a style and a topic."); const course = ctx.courses.get(g.courseId); if (!course || course.status !== "ok") return err(404, "Course not found."); - if (!(await canAuthor(rc.user, course.id))) return err(403, "Only authors can generate drafts."); + if (!(await canAuthor(rc.user, course))) return err(403, "Only authors can generate drafts."); if (!(await resolveProvider(rc.user.id))) return err(409, "AI is not set up."); const jobId = await ctx.jobs.enqueue("ai.generate", { req: g, userId: rc.user.id }); return json({ jobId }, 202); diff --git a/packages/feature-ai/src/web/AssistantPage.tsx b/packages/feature-ai/src/web/AssistantPage.tsx index 3d180a8..d9c761a 100644 --- a/packages/feature-ai/src/web/AssistantPage.tsx +++ b/packages/feature-ai/src/web/AssistantPage.tsx @@ -2,6 +2,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { Link, useParams } from "react-router-dom"; import { Pill, SegmentedToggle } from "@stc/ui"; import { api, readNdjson, type ChatEvent, type Citation, type Status } from "./api"; +import { courseUrl, lessonUrl } from "./urls"; import { Markdown, MentorBanner, PageHead } from "./components"; type Turn = { role: "user" | "assistant"; text: string; citations?: Citation[]; tools?: string[]; error?: string; notices?: string[] }; @@ -21,7 +22,7 @@ function Sources({ courseId, citations }: { courseId: string; citations: Citatio {rows.map((c) => (
  • {c.path} - {c.lessonId ? {c.lessonTitle ?? c.lessonId} : {c.kind}} + {c.lessonId ? {c.lessonTitle ?? c.lessonId} : {c.kind}}
    • ))} @@ -120,7 +121,7 @@ export default function AssistantPage() { const newConversation = () => { abortRef.current?.abort(); setConversationId(null); setTurns([]); }; const courseTitle = status?.course?.title ?? courseId; - const chap = {courseTitle}; + const chap = {courseTitle}; if (loadError && !status) return

    {loadError}

    ; if (!status) return
    ; diff --git a/packages/feature-ai/src/web/index.ts b/packages/feature-ai/src/web/index.ts index b3c7387..181fc97 100644 --- a/packages/feature-ai/src/web/index.ts +++ b/packages/feature-ai/src/web/index.ts @@ -9,3 +9,4 @@ export const aiWeb: WebFeature = { { path: "/studio/generate", element: () => import("./GeneratePage") }, ], }; +export { courseUrl, lessonUrl } from "./urls"; diff --git a/packages/feature-ai/src/web/urls.ts b/packages/feature-ai/src/web/urls.ts new file mode 100644 index 0000000..d08fb57 --- /dev/null +++ b/packages/feature-ai/src/web/urls.ts @@ -0,0 +1,11 @@ +/** + * Learner URLs the AI pages link to. The learner app (WP-A) owns these routes; keep every builder here so + * Phase 2 can change the shape in one place (contract request 7). + */ +export function courseUrl(courseId: string): string { + return `/c/${encodeURIComponent(courseId)}`; +} + +export function lessonUrl(courseId: string, lessonId: string): string { + return `${courseUrl(courseId)}/${encodeURIComponent(lessonId)}`; +} diff --git a/packages/feature-ai/test/feature.test.ts b/packages/feature-ai/test/feature.test.ts index 5ff2293..96fa0bd 100644 --- a/packages/feature-ai/test/feature.test.ts +++ b/packages/feature-ai/test/feature.test.ts @@ -218,3 +218,29 @@ forEachDb("provider errors on a user's own key show the status and a fixed messa const t2 = await (await call("POST", "/api/ai/courses/sample/chat", "owner", { message: "hello" })).text(); expect(t2).toContain("502: INTERNAL-SECRET-BODY"); }); + +forEachDb("authoring uses core canView: a global author needs a grant for a restricted course", async (db) => { + const { call } = await setup(db); + await call("PUT", "/api/ai/admin/config", "owner", { provider: { kind: "anthropic", apiKey: "k" }, mcpServers: [] }); + const body = { courseId: "secret", kind: "questions", style: "pure-snake", topic: "Adding", sources: [{ kind: "lesson", lessonId: "m2-text" }] }; + const courses = async (u: string) => ((await (await call("GET", "/api/ai/studio/courses", u)).json()) as { id: string }[]).map((c) => c.id).sort(); + // Public course: the author role is enough. Restricted course: not without a grant. + expect(await courses("auth")).toEqual(["sample"]); + expect((await call("GET", "/api/ai/studio/sources?courseId=secret", "auth")).status).toBe(403); + expect((await call("POST", "/api/ai/studio/generate", "auth", body)).status).toBe(403); + // The owner sees everything. + expect(await courses("owner")).toEqual(["sample", "secret"]); + expect((await call("POST", "/api/ai/studio/generate", "owner", body)).status).toBe(202); + // An ACL grant (core canView) lets the global author in. + await db.sql`INSERT INTO course_acl (course_id, principal_type, principal_id) VALUES ('secret', 'user', 'auth')`; + expect(await courses("auth")).toEqual(["sample", "secret"]); + expect((await call("POST", "/api/ai/studio/generate", "auth", body)).status).toBe(202); + // A course_authors row makes a learner an author of that course only. + expect((await call("POST", "/api/ai/studio/generate", "bob", body)).status).toBe(403); + await db.sql`INSERT INTO course_authors (course_id, user_id) VALUES ('secret', 'bob')`; + expect(await courses("bob")).toEqual(["secret"]); + expect((await call("POST", "/api/ai/studio/generate", "bob", body)).status).toBe(202); + // A grant to view alone does not make a learner an author. + await db.sql`INSERT INTO course_acl (course_id, principal_type, principal_id) VALUES ('secret', 'user', 'ann')`; + expect((await call("POST", "/api/ai/studio/generate", "ann", body)).status).toBe(403); +}); diff --git a/packages/feature-ai/test/web.test.tsx b/packages/feature-ai/test/web.test.tsx index 3f0d540..683652d 100644 --- a/packages/feature-ai/test/web.test.tsx +++ b/packages/feature-ai/test/web.test.tsx @@ -34,3 +34,13 @@ test("web routes", () => { expect(aiWeb.routes.map((r) => r.path)).toEqual(["/c/:courseId/assistant", "/admin/ai", "/account/ai", "/studio/generate"]); expect(aiWeb.routes.find((r) => r.path === "/admin/ai")!.nav).toEqual({ label: "AI", area: "admin" }); }); + +test("lesson links go through one builder", async () => { + const { lessonUrl, courseUrl } = await import("../src/web/urls"); + expect(lessonUrl("sample", "m2-ex-add")).toBe("/c/sample/m2-ex-add"); + expect(lessonUrl("S08 - Docker", "l 1/x")).toBe("/c/S08%20-%20Docker/l%201%2Fx"); + expect(courseUrl("sample")).toBe("/c/sample"); + const src = await Bun.file(new URL("../src/web/AssistantPage.tsx", import.meta.url)).text(); + expect(src).toContain("lessonUrl(courseId, c.lessonId)"); + expect(src).not.toMatch(/`\/c\//); +});