diff --git a/hub/prisma/migrations/20260707101108_add_role_trigger_grant/migration.sql b/hub/prisma/migrations/20260707101108_add_role_trigger_grant/migration.sql new file mode 100644 index 0000000..80735e9 --- /dev/null +++ b/hub/prisma/migrations/20260707101108_add_role_trigger_grant/migration.sql @@ -0,0 +1,27 @@ +-- CreateTable +CREATE TABLE "RoleTriggerGrant" ( + "id" TEXT NOT NULL, + "projectId" TEXT NOT NULL, + "roleId" TEXT NOT NULL, + "principal" TEXT NOT NULL, + "createdByUserId" TEXT, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "revokedAt" TIMESTAMP(3), + + CONSTRAINT "RoleTriggerGrant_pkey" PRIMARY KEY ("id") +); + +-- CreateIndex +CREATE INDEX "RoleTriggerGrant_projectId_roleId_revokedAt_idx" ON "RoleTriggerGrant"("projectId", "roleId", "revokedAt"); + +-- CreateIndex +CREATE INDEX "RoleTriggerGrant_principal_revokedAt_idx" ON "RoleTriggerGrant"("principal", "revokedAt"); + +-- CreateIndex +CREATE UNIQUE INDEX "RoleTriggerGrant_projectId_roleId_principal_revokedAt_key" ON "RoleTriggerGrant"("projectId", "roleId", "principal", "revokedAt"); + +-- AddForeignKey +ALTER TABLE "RoleTriggerGrant" ADD CONSTRAINT "RoleTriggerGrant_projectId_fkey" FOREIGN KEY ("projectId") REFERENCES "Project"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "RoleTriggerGrant" ADD CONSTRAINT "RoleTriggerGrant_createdByUserId_fkey" FOREIGN KEY ("createdByUserId") REFERENCES "User"("id") ON DELETE SET NULL ON UPDATE CASCADE; diff --git a/hub/prisma/schema.prisma b/hub/prisma/schema.prisma index ce1499b..c0bc8b4 100644 --- a/hub/prisma/schema.prisma +++ b/hub/prisma/schema.prisma @@ -40,6 +40,7 @@ model User { heldLocks ProjectAgentLock[] @relation("lockHolder") feishuBindings ProjectGroupBinding[] @relation("bindingCreator") permissionGrants PermissionGrant[] @relation("grantCreator") + roleTriggerGrants RoleTriggerGrant[] @relation("roleGrantCreator") auditEntries AuditEntry[] @relation("auditActor") } @@ -81,6 +82,7 @@ model Project { agentLock ProjectAgentLock? permissionGrants PermissionGrant[] @relation("projectGrants") permissionSettings PermissionSettings[] @relation("projectSettings") + roleTriggerGrants RoleTriggerGrant[] @relation("projectRoleGrants") @@index([archivedAt]) } @@ -260,6 +262,34 @@ model PermissionSettings { @@index([resourceType, resourceId]) } + +/// Per-role agent trigger grant. Orthogonal to ADR-0004's PermissionGrant: +/// ADR-0004 decides "can trigger an agent at all" (triggerAgent capability, +/// edit+ role); this table decides "can trigger *which* agent role" (e.g. +/// /review vs /draft). Two gates in series — both must pass. +/// +/// `roleId` is an opaque string matching RoleEntry.id (ADR-0017: roles are +/// data, not a code enum). `principal` mirrors ADR-0004's opaque principal +/// (sub-typology OPEN). A project-scoped row grants the role on that project; +/// the compound unique covers "one active grant per (project, principal, role)". +/// Revocation via `revokedAt`, same pattern as PermissionGrant. +model RoleTriggerGrant { + id String @id @default(cuid()) + projectId String + roleId String + principal String + createdByUserId String? + createdAt DateTime @default(now()) + revokedAt DateTime? + + project Project @relation("projectRoleGrants", fields: [projectId], references: [id], onDelete: Cascade) + createdBy User? @relation("roleGrantCreator", fields: [createdByUserId], references: [id], onDelete: SetNull) + + @@unique([projectId, roleId, principal, revokedAt]) + @@index([projectId, roleId, revokedAt]) + @@index([principal, revokedAt]) +} + // --- Audit (ADR Audit, content OPEN) ------------------------------------- /// spec AuditEntry: minimal skeleton — one entry relates to a run. Event type, diff --git a/hub/src/agent/models.ts b/hub/src/agent/models.ts index 8778fbc..84830fb 100644 --- a/hub/src/agent/models.ts +++ b/hub/src/agent/models.ts @@ -9,11 +9,17 @@ */ /** - * A named role preset that maps to a default model. Roles are **data**, not a - * code enum: admin/teachers define them (ADR-0017: role-based routing is - * product config, not a spec invariant). `roleId` is an opaque string here — - * the registry holds the role set, so new roles are added by configuration, - * not by editing this file. + * A named role preset — the full per-run agent bundle. Roles are **data**, not + * a code enum: admin/teachers define them (ADR-0017: role-based routing is + * product config, not a spec invariant). `roleId` is an opaque string and + * doubles as the slash-command name (`/draft ...`) — the registry holds the + * role set, so new roles are added by configuration, not by editing code. + * + * A role bundles everything that distinguishes one agent persona from another: + * model, system prompt, and the tool surface (files / cph / feishu / skills / + * mcps). Per-run tool whitelisting is enforced by {@link ToolRegistry.subset}; + * the model never sees tools outside its role's whitelist, even if it tries to + * call them — security does not rely on the system prompt. */ export interface RoleEntry { readonly id: string; @@ -21,6 +27,18 @@ export interface RoleEntry { readonly label: string; /** Default model id for runs under this role, if a routing rule is set. */ readonly defaultModel: string | undefined; + /** + * System prompt seeding the agent's persona/instructions. Prepended to the + * run's messages only at session start (resume reads it from the transcript, + * see runner.ts). `undefined` ⇒ no system prompt (bare run). + */ + readonly systemPrompt: string | undefined; + /** + * Tool names this role may use (whitelist). `undefined` ⇒ the full registered + * set (back-compat / unrestricted roles). An empty array ⇒ no tools at all. + * Names not present in the registry are silently dropped at run setup. + */ + readonly tools: readonly string[] | undefined; } /** A model the admin has enabled for use by the Hub. */ diff --git a/hub/src/agent/runner.ts b/hub/src/agent/runner.ts index 5409873..d56277e 100644 --- a/hub/src/agent/runner.ts +++ b/hub/src/agent/runner.ts @@ -27,7 +27,7 @@ export interface RunRequest { /** Resolved model id (already chosen by the caller per ADR-0017). */ readonly model: string; readonly project: ProjectContext; - readonly systemPrompt?: string; + readonly systemPrompt: string | undefined; /** Iteration cap before the run is returned as `length`. Default 25. */ readonly maxIterations?: number; /** Path to the session transcript JSONL. If set, prior messages are loaded diff --git a/hub/src/agent/tools.ts b/hub/src/agent/tools.ts index 2790b70..f4bf4c9 100644 --- a/hub/src/agent/tools.ts +++ b/hub/src/agent/tools.ts @@ -60,6 +60,31 @@ export class ToolRegistry { } return tool.execute(args, ctx); } + + /** + * Return a per-run view restricted to `names`. Names not registered are + * silently dropped (a role's whitelist may name tools that a particular Hub + * instance doesn't register). The returned registry shares the handler + * references — no copy of the closures. + * + * Per-role tool whitelisting (ADR-0017: role-based routing is product + * config). The runner receives a subset registry so the model literally + * never sees tools outside its role's whitelist: the tool list sent to the + * provider is `subset.specs()`, and `execute` on a name not in the subset + * returns an error. + */ + subset(names: readonly string[]): ToolRegistry { + const allowed = new Set(names); + const view = new ToolRegistry(); + for (const [name, tool] of this.byName) { + if (allowed.has(name)) { + // Bypass the duplicate check: we're constructing from an already-valid + // registry, not re-registering. + view.byName.set(name, tool); + } + } + return view; + } } /** diff --git a/hub/src/feishu/trigger.ts b/hub/src/feishu/trigger.ts index cd4cd2c..6270b84 100644 --- a/hub/src/feishu/trigger.ts +++ b/hub/src/feishu/trigger.ts @@ -21,7 +21,7 @@ import type { ToolRegistry } from "../agent/tools.js"; import type { ModelRegistry } from "../agent/models.js"; import { runAgent, type ProjectContext } from "../agent/runner.js"; import { acquireLock, currentLockRunId, releaseLock } from "../lock.js"; -import { canTriggerAgent } from "../permission.js"; +import { canTriggerAgent, canTriggerRole } from "../permission.js"; import { transcriptPath } from "../agent/transcript.js"; interface TriggerDeps { @@ -132,11 +132,31 @@ export function makeTriggerHandler(deps: TriggerDeps) { return; } - // ADR-0017: role-as-data; model resolved by registry. "draft" is the - // fallback role id; real wiring lets the trigger carry a role hint (slash - // command, card selector). The registry degrades unknown roles gracefully. - const model = deps.models.resolve(undefined, "draft"); + // ADR-0017: role-as-data. Parse a leading `/` command; unknown + // slashes are left as literal text (extractRole returns null). Falls back + // to "draft" when no role is named — the registry degrades gracefully. + const { roleId: parsedRole, prompt: cleanPrompt } = extractRole(prompt, deps.models); + const roleId = parsedRole ?? "draft"; + // Per-role trigger gate (ADR-0017). Orthogonal to ADR-0004's + // canTriggerAgent above: that checked "can trigger an agent at all"; + // this checks "can trigger *this* role". Unconfigured role ⇒ open. + if (senderOpenId !== "") { + const rolePerm = await canTriggerRole(deps.prisma, projectId, roleId, senderOpenId); + if (!rolePerm.allowed) { + deps.logger.info({ projectId, roleId, senderOpenId, reason: rolePerm.reason }, "feishu trigger: role permission denied"); + await sendText(rt, chatId, `无权限使用角色 ${roleId}。`); + return; + } + } + const role = deps.models.role(roleId); + const model = deps.models.resolve(undefined, roleId); const providerId = deps.provider.id; + // Per-role bundle (ADR-0017): systemPrompt seeds persona; tools whitelist + // is enforced by constructing a per-run registry subset. `tools: undefined` + // means the full registered set (unrestricted role). + const systemPrompt = role?.systemPrompt; + const runTools = + role?.tools !== undefined ? deps.tools.subset(role.tools) : deps.tools; // ADR-0017: provider+model-bound session. Reuse if one exists for this // (project, provider, model) triple; otherwise create. @@ -156,7 +176,7 @@ export function makeTriggerHandler(deps: TriggerDeps) { projectId, provider: providerId, model, - title: prompt.slice(0, 40) || null, + title: cleanPrompt.slice(0, 40) || null, metadata: {}, }, select: { id: true }, @@ -169,10 +189,10 @@ export function makeTriggerHandler(deps: TriggerDeps) { requestedByUserId: null, // sender→user mapping is OPEN (principal sub-typology) entrypoint: "FEISHU", status: "ACTIVE", - prompt, + prompt: cleanPrompt, model, provider: providerId, - metadata: {}, + metadata: { roleId }, }, select: { id: true }, }); @@ -190,7 +210,7 @@ export function makeTriggerHandler(deps: TriggerDeps) { return; } - await sendText(rt, chatId, `已开始处理(model: ${model})。`); + await sendText(rt, chatId, `已开始处理(role: ${roleId}, model: ${model})。`); const projectCtx: ProjectContext = { projectId, @@ -199,7 +219,14 @@ export function makeTriggerHandler(deps: TriggerDeps) { }; // Run the agent — fire-and-forget from the ws handler's perspective. - runAgent(deps.provider, deps.tools, { prompt, model, project: projectCtx, transcriptPath: transcriptPath(project.workspaceDir, session.id) }) + // Per-run tools + systemPrompt come from the role bundle (ADR-0017). + runAgent(deps.provider, runTools, { + prompt: cleanPrompt, + model, + project: projectCtx, + systemPrompt, + transcriptPath: transcriptPath(project.workspaceDir, session.id), + }) .then(async (result) => { await deps.prisma.agentRun.update({ where: { id: run.id }, @@ -224,10 +251,17 @@ export function makeTriggerHandler(deps: TriggerDeps) { })); }) .catch(async (e) => { - await deps.prisma.agentRun.update({ - where: { id: run.id }, - data: { status: "FAILED", error: String(e), finishedAt: new Date() }, - }); + // The run may have been removed (admin force-delete, or test reset) + // between creation and this callback. A P2025 from update is not + // actionable here — log and still post the status card. + try { + await deps.prisma.agentRun.update({ + where: { id: run.id }, + data: { status: "FAILED", error: String(e), finishedAt: new Date() }, + }); + } catch (updateErr) { + deps.logger.warn({ runId: run.id, err: String(updateErr) }, "trigger: could not mark run FAILED (already gone?)"); + } await sendCard(rt, chatId, buildRunStatusCard({ status: "处理出错", model, @@ -275,3 +309,26 @@ export function extractPrompt(msg: MessageReceiveEvent["message"]): string | nul return null; } } +/** + * Parse a leading `/` command from a prompt (e.g. `/draft 帮我写教案`). + * Returns the role id and the remaining prompt with the command stripped. + * Control commands already handled upstream (`/new`, `/resume`, `/reset`) are + * ignored here — they never reach this function. + * + * Unknown `/foo` that isn't a registered role: returns `null` role and the + * original prompt unchanged (the slash is treated as literal text). Role + * resolution still happens via the registry's fallback. + */ +export function extractRole( + prompt: string, + registry: { role(id: string): unknown }, +): { roleId: string | null; prompt: string } { + const m = /^\/(\S+)\s*/.exec(prompt); + if (m === null || m[1] === undefined) return { roleId: null, prompt }; + const roleId = m[1]; + if (registry.role(roleId) === undefined) { + // Unknown slash command — leave the prompt as-is (no silent role switch). + return { roleId: null, prompt }; + } + return { roleId, prompt: prompt.slice(m[0].length) }; +} diff --git a/hub/src/permission.ts b/hub/src/permission.ts index 017ffb9..41c157d 100644 --- a/hub/src/permission.ts +++ b/hub/src/permission.ts @@ -68,6 +68,57 @@ export async function canTriggerAgent( return { allowed: false, reason: `permission check failed: ${e instanceof Error ? e.message : String(e)}` }; } } +/** + * Per-role trigger gate (ADR-0017: roles are product config). Orthogonal to + * {@link canTriggerAgent}: that decides "can trigger an agent at all" + * (ADR-0004 triggerAgent capability); this decides "can trigger *which* role". + * Two gates in series — both must pass. + * + * Semantics: if **no** `RoleTriggerGrant` row exists for `(projectId, roleId)` + * (regardless of principal), the role is unconfigured on this project → + * **allow** (back-compat: a project that hasn't configured per-role grants + * doesn't get locked down). Once any grant exists for that role on the + * project, only principals with an active grant may trigger it. "Grants exist + * but this principal has none" → deny (admin explicitly restricted the role). + * + * `roleId` matches `RoleEntry.id` (the slash-command name). `principal` is the + * same opaque string ADR-0004 uses (sender open_id here; principal + * sub-typology OPEN). + */ +export async function canTriggerRole( + prisma: PrismaClient, + projectId: string, + roleId: string, + principal: string, +): Promise { + try { + // Any grant record (active OR revoked) for this (project, role)? If none, + // the role is unconfigured → allow (back-compat: no per-role restriction). + // A revoked grant still counts as "configured" — revoking one person must + // not silently reopen the role to everyone. + const anyGrant = await prisma.roleTriggerGrant.findFirst({ + where: { projectId, roleId }, + select: { id: true }, + }); + if (anyGrant === null) { + return { allowed: true, reason: `role ${roleId} unconfigured on project (open)` }; + } + // Grants exist — this principal must hold one. + const grant = await prisma.roleTriggerGrant.findFirst({ + where: { projectId, roleId, principal, revokedAt: null }, + select: { id: true }, + }); + if (grant !== null) { + return { allowed: true, reason: `granted role ${roleId}` }; + } + return { + allowed: false, + reason: `no active ${roleId} grant for ${principal} on project ${projectId}`, + }; + } catch (e) { + return { allowed: false, reason: `role permission check failed: ${e instanceof Error ? e.message : String(e)}` }; + } +} /// Whether a role grants edit capability (edit or manage). Exported for tests. export function roleGrantsEdit(role: PermissionRole): boolean { diff --git a/hub/src/server.ts b/hub/src/server.ts index 1a3dc89..25755e5 100644 --- a/hub/src/server.ts +++ b/hub/src/server.ts @@ -52,8 +52,22 @@ async function main(): Promise { { id: "anthropic/claude-3.5-sonnet", label: "Claude 3.5 Sonnet", toolCapable: true }, ], [ - { id: "draft", label: "草稿", defaultModel: "z-ai/glm-4.6" }, - { id: "review", label: "审校", defaultModel: "anthropic/claude-3.5-sonnet" }, + { + id: "draft", + label: "草稿", + defaultModel: "z-ai/glm-4.6", + systemPrompt: undefined, + // Drafting needs full workspace + cph access. + tools: ["read_file", "write_file", "list_files", "cph_check", "cph_build", "feishu_read_context"], + }, + { + id: "review", + label: "审校", + defaultModel: "anthropic/claude-3.5-sonnet", + systemPrompt: undefined, + // Review is read-only on artifacts; no write_file, no build. + tools: ["read_file", "list_files", "cph_check", "feishu_read_context"], + }, ], ); diff --git a/hub/test/integration/helpers.ts b/hub/test/integration/helpers.ts index a8a55e7..cd94378 100644 --- a/hub/test/integration/helpers.ts +++ b/hub/test/integration/helpers.ts @@ -22,6 +22,7 @@ export async function resetDb(): Promise { "AuditEntry", "PermissionSettings", "PermissionGrant", + "RoleTriggerGrant", "ProjectAgentLock", "AgentRun", "AgentSession", diff --git a/hub/test/integration/role-permission.test.ts b/hub/test/integration/role-permission.test.ts new file mode 100644 index 0000000..7c2ae10 --- /dev/null +++ b/hub/test/integration/role-permission.test.ts @@ -0,0 +1,71 @@ +import { describe, it, expect, beforeEach, afterAll } from "vitest"; +import { prisma, resetDb } from "./helpers.js"; +import { canTriggerRole } from "../../src/permission.js"; + +describe("canTriggerRole (integration, per-role gate)", () => { + beforeEach(async () => { + await resetDb(); + }); + + afterAll(async () => { + await prisma.$disconnect(); + }); + + it("allows when role is unconfigured on the project (back-compat: open)", async () => { + const project = await prisma.project.create({ + data: { id: "p-role-1", name: "T", workspaceDir: "/tmp/x" }, + }); + const r = await canTriggerRole(prisma, project.id, "draft", "ou_a"); + expect(r.allowed).toBe(true); + }); + + it("allows when principal holds an active grant for the role", async () => { + const project = await prisma.project.create({ + data: { id: "p-role-2", name: "T", workspaceDir: "/tmp/x" }, + }); + await prisma.roleTriggerGrant.create({ + data: { projectId: project.id, roleId: "review", principal: "ou_a" }, + }); + const r = await canTriggerRole(prisma, project.id, "review", "ou_a"); + expect(r.allowed).toBe(true); + }); + + it("denies when grants exist for the role but principal has none", async () => { + const project = await prisma.project.create({ + data: { id: "p-role-3", name: "T", workspaceDir: "/tmp/x" }, + }); + // Someone else has the role; ou_b does not. + await prisma.roleTriggerGrant.create({ + data: { projectId: project.id, roleId: "review", principal: "ou_a" }, + }); + const r = await canTriggerRole(prisma, project.id, "review", "ou_b"); + expect(r.allowed).toBe(false); + }); + + it("denies when the principal's grant was revoked", async () => { + const project = await prisma.project.create({ + data: { id: "p-role-4", name: "T", workspaceDir: "/tmp/x" }, + }); + await prisma.roleTriggerGrant.create({ + data: { projectId: project.id, roleId: "review", principal: "ou_a", revokedAt: new Date() }, + }); + const r = await canTriggerRole(prisma, project.id, "review", "ou_a"); + expect(r.allowed).toBe(false); + }); + + it("scopes grants per-project (grant on p1 does not allow on p2)", async () => { + const p1 = await prisma.project.create({ data: { id: "p-role-5a", name: "T", workspaceDir: "/tmp/x" } }); + const p2 = await prisma.project.create({ data: { id: "p-role-5b", name: "T", workspaceDir: "/tmp/x" } }); + await prisma.roleTriggerGrant.create({ + data: { projectId: p1.id, roleId: "review", principal: "ou_a" }, + }); + // p2 has the role configured for someone else; ou_a has no grant there. + await prisma.roleTriggerGrant.create({ + data: { projectId: p2.id, roleId: "review", principal: "ou_other" }, + }); + const onP1 = await canTriggerRole(prisma, p1.id, "review", "ou_a"); + expect(onP1.allowed).toBe(true); + const onP2 = await canTriggerRole(prisma, p2.id, "review", "ou_a"); + expect(onP2.allowed).toBe(false); + }); +}); diff --git a/hub/test/integration/trigger.test.ts b/hub/test/integration/trigger.test.ts index bf8aefe..155ddbb 100644 --- a/hub/test/integration/trigger.test.ts +++ b/hub/test/integration/trigger.test.ts @@ -33,7 +33,10 @@ describe("trigger full lifecycle (integration)", () => { tools = new ToolRegistry(); models = new InMemoryModelRegistry( [{ id: "mock-model", label: "Mock", toolCapable: true }], - [{ id: "draft", label: "草稿", defaultModel: "mock-model" }], + [ + { id: "draft", label: "草稿", defaultModel: "mock-model", systemPrompt: undefined, tools: undefined }, + { id: "review", label: "审校", defaultModel: "mock-model", systemPrompt: undefined, tools: ["read_file"] }, + ], ); rt = mockFeishuRuntime(); }); @@ -57,7 +60,7 @@ describe("trigger full lifecycle (integration)", () => { // A status card was sent. expect(rt.sentCards.length).toBeGreaterThanOrEqual(1); - expect(rt.sentTexts).toContain("已开始处理(model: mock-model)。"); + expect(rt.sentTexts).toContain("已开始处理(role: draft, model: mock-model)。"); }); it("rejects a sender without edit grant (ADR-0004)", async () => { @@ -198,6 +201,52 @@ describe("trigger full lifecycle (integration)", () => { expect(runs[0]?.prompt).toBe("/unknown"); }); }); + + it("denies /review when sender has no role grant (per-role gate)", async () => { + await seedProject("proj-10", "chat-10"); + // Someone else holds review; ou_test_user does not. + await prisma.roleTriggerGrant.create({ + data: { projectId: "proj-10", roleId: "review", principal: "ou_other" }, + }); + const trigger = makeTriggerHandler({ prisma, provider, tools, models, logger: silentLogger }); + + await trigger(makeEvent("chat-10", "@_user_1 /review 看看这节"), rt); + + expect(rt.sentTexts).toContain("无权限使用角色 review。"); + const runs = await prisma.agentRun.findMany(); + expect(runs).toHaveLength(0); + }); + + it("allows /review when sender holds the role grant", async () => { + await seedProject("proj-11", "chat-11"); + await prisma.roleTriggerGrant.create({ + data: { projectId: "proj-11", roleId: "review", principal: "ou_test_user" }, + }); + const trigger = makeTriggerHandler({ prisma, provider, tools, models, logger: silentLogger }); + + await trigger(makeEvent("chat-11", "@_user_1 /review 看看这节"), rt); + + await vi.waitFor(async () => { + const runs = await prisma.agentRun.findMany(); + expect(runs).toHaveLength(1); + expect(runs[0]?.status).toBe("COMPLETED"); + expect(runs[0]?.metadata).toMatchObject({ roleId: "review" }); + }); + }); + + it("extractRole: /draft sets roleId=draft, strips command from prompt", async () => { + await seedProject("proj-12", "chat-12"); + const trigger = makeTriggerHandler({ prisma, provider, tools, models, logger: silentLogger }); + + await trigger(makeEvent("chat-12", "@_user_1 /draft 写第三单元"), rt); + + await vi.waitFor(async () => { + const runs = await prisma.agentRun.findMany(); + expect(runs).toHaveLength(1); + expect(runs[0]?.prompt).toBe("写第三单元"); + expect(runs[0]?.metadata).toMatchObject({ roleId: "draft" }); + }); + }); }); afterAll(async () => {