From 318c1a9ce927ef1b6ac6d65711467dc414edbbc7 Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 30 Oct 2026 05:30:42 +0900 Subject: [PATCH] fix: align permission management with integrated Worker and chat operations --- .../server-api/src/authorization/catalog.rs | 30 +++++++++++----- .../src/authorization/catalog_tests.rs | 25 ++++++++++++- .../tests/subject_authorization_tests.rs | 4 +-- .../subject_permission_management_tests.rs | 19 ++++------ .../memory/SubjectPermissions.browser.spec.ts | 35 +++++++++++++++++-- .../lib/workspace/memory/permissions.test.ts | 35 +++++++++++++++++++ .../src/lib/workspace/memory/permissions.ts | 5 +++ .../memory/memory-page.browser.spec.ts | 6 ---- 8 files changed, 127 insertions(+), 32 deletions(-) diff --git a/crates/server-api/src/authorization/catalog.rs b/crates/server-api/src/authorization/catalog.rs index 87171aa26..8bd75e144 100644 --- a/crates/server-api/src/authorization/catalog.rs +++ b/crates/server-api/src/authorization/catalog.rs @@ -247,7 +247,7 @@ pub fn subject_permission_operations() -> Vec { ( "worker.create", "Create background Workers", - "Use existing Profile-resolved Worker creation with explicit execution grants. Control and reply without duplicate target permissions await T-803.", + "Use Profile-resolved Worker creation with explicit execution grants. Directional Backend grants separately control the created Worker and its message-only reply.", ), ( "drive.capabilities", @@ -342,12 +342,12 @@ pub fn subject_permission_operations() -> Vec { ( "ticket.assignment.assign", "Delegate Ticket", - "Delegate to an authorized Orchestrator principal; does not start a Worker. Subject assignment awaits T-778.", + "Assign to an authorized Subject; current body, Ticket targets and execution grants are checked separately. Assignment does not start a Worker.", ), ( "ticket.assignment.clear", "Release Ticket delegation", - "Release the observed current Orchestrator delegation; does not stop Workers.", + "Release the observed current Subject assignment; does not stop Workers.", ), ( "ticket.assignment.update", @@ -817,6 +817,21 @@ pub fn subject_permission_operations() -> Vec { ), ], ); + add( + K::Subject, + &[ + ( + "chat.read", + "Read Subject conversations", + "Read conversations belonging to the current Subject body; membership and current ownership remain required.", + ), + ( + "chat.send", + "Send Subject message", + "Explicitly publish to a permitted conversation as the current Subject body. Background Worker messaging does not grant publication.", + ), + ], + ); // The same exact operation may target either independent Memory domain. This // adds accepted kinds, never cross-kind containment or a second descriptor. for descriptor in &mut catalog { @@ -843,9 +858,8 @@ pub fn subject_permission_operations() -> Vec { resource_kinds: vec![K::Workspace, K::Ticket, K::MergeRequest, K::Job], availability: PermissionOperationAvailability::Available, }); - // T-803 owns operation-only control and name migration. No Worker resource - // kinds are offered here: adding a per-target setting would duplicate Backend - // control grants and dropping an old target would widen authority. + // Operation use is Workspace-bound but never grants a target. Legacy Worker + // scopes stay exact; management does not convert or discard their limits. for (operation, label) in [ ("worker.messaging", "Message Workers"), ("worker.send_input", "Submit to Worker"), @@ -858,8 +872,8 @@ pub fn subject_permission_operations() -> Vec { catalog.push(PermissionOperationDescriptor { operation: operation.into(), label: label.into(), description: "Operation use permission and directional Backend target control grants are separate. Acceptance does not prove processing or success; lifecycle state conditions still apply.".into(), - resource_kinds: vec![], - availability: PermissionOperationAvailability::Unavailable { reason: "T-803 connection pending: current control still requires target-specific Worker permissions and uses worker.notify/worker.result. Existing restricted entries are preserved without conversion.".into() }, + resource_kinds: vec![K::WorkerOperation], + availability: PermissionOperationAvailability::Available, }); } for (operation, label) in [ diff --git a/crates/server-api/src/authorization/catalog_tests.rs b/crates/server-api/src/authorization/catalog_tests.rs index 109ded979..dffc98d65 100644 --- a/crates/server-api/src/authorization/catalog_tests.rs +++ b/crates/server-api/src/authorization/catalog_tests.rs @@ -115,5 +115,28 @@ fn catalog_wire_has_no_worker_target_editors_and_no_implicit_feature_grants() { .iter() .find(|d| d["operation"] == "worker.messaging") .unwrap(); - assert_eq!(messaging["availability"]["kind"], "unavailable"); + assert_eq!(messaging["availability"]["kind"], "available"); + assert_eq!(messaging["resource_kinds"], json!(["worker_operation"])); +} + +#[test] +fn managed_worker_operations_accept_only_current_operation_use_in_each_lane() { + for name in [ + "worker.messaging", + "worker.send_input", + "worker.cancel", + "worker.stop", + "worker.restore", + "worker.remove", + "worker.observe", + ] { + let p = permission(name, json!({"kind":"worker_operation","workspace_id":"w"})); + assert!( + validate_managed_subject_permissions(None, &update(vec![p.clone()], vec![p])).is_ok() + ); + } + for name in ["worker.notify", "worker.result", "worker.future"] { + let p = permission(name, json!({"kind":"worker_operation","workspace_id":"w"})); + assert!(validate_managed_subject_permissions(None, &update(vec![p], vec![])).is_err()); + } } diff --git a/crates/workspace-server/src/server/tests/subject_authorization_tests.rs b/crates/workspace-server/src/server/tests/subject_authorization_tests.rs index f7b711151..f0befbf33 100644 --- a/crates/workspace-server/src/server/tests/subject_authorization_tests.rs +++ b/crates/workspace-server/src/server/tests/subject_authorization_tests.rs @@ -11,14 +11,14 @@ mod chat_delegation_connected; mod chat_tests; #[path = "subject_job_model_supply_tests.rs"] mod subject_job_model_supply; +#[path = "subject_permission_management_tests.rs"] +mod subject_permission_management; #[path = "ticket_worker_messaging_connected_tests.rs"] mod ticket_worker_messaging_connected; #[path = "worker_messaging_connected_tests.rs"] mod worker_messaging_connected; #[path = "worker_operation_authorization_tests.rs"] mod worker_operation_authorization; -#[path = "subject_permission_management_tests.rs"] -mod subject_permission_management; use agen::tool::{Tool, ToolExecutionContext, ToolOutput}; use server_api::authorization::{ PermissionResource, ScopedPermission, SubjectPermissions, SubjectPermissionsUpdate, diff --git a/crates/workspace-server/src/server/tests/subject_permission_management_tests.rs b/crates/workspace-server/src/server/tests/subject_permission_management_tests.rs index 6b5d0a6f4..b93b58d5c 100644 --- a/crates/workspace-server/src/server/tests/subject_permission_management_tests.rs +++ b/crates/workspace-server/src/server/tests/subject_permission_management_tests.rs @@ -71,8 +71,7 @@ fn update(screen: &Value, operation: &str, execution: Value, delegation: Value) } #[tokio::test] -async fn management_read_exposes_capability_and_catalog_without_enabling_pending_worker_operations() -{ +async fn management_read_exposes_operation_use_without_duplicate_worker_target_grants() { let f = ManagementFixture::new().await; let screen = f.read().await; assert!(screen["permissions"].is_null()); @@ -95,13 +94,7 @@ async fn management_read_exposes_capability_and_catalog_without_enabling_pending .iter() .find(|entry| entry["operation"] == "worker.messaging") .unwrap(); - assert_eq!(messaging["availability"]["kind"], "unavailable"); - assert!( - !messaging["availability"]["reason"] - .as_str() - .unwrap() - .is_empty() - ); + assert_eq!(messaging["availability"]["kind"], "available"); for name in [ "worker.messaging", "worker.send_input", @@ -116,13 +109,13 @@ async fn management_read_exposes_capability_and_catalog_without_enabling_pending .find(|entry| entry["operation"] == name) .unwrap(); assert_eq!( - operation["availability"]["kind"], "unavailable", - "pending Worker contracts: {operation}" + operation["availability"]["kind"], "available", + "implemented Worker contracts: {operation}" ); assert_eq!( operation["resource_kinds"], - json!([]), - "do not offer duplicate target control grants" + json!(["worker_operation"]), + "operation use must not offer duplicate target control grants" ); } } diff --git a/web/workspace/src/lib/workspace/memory/SubjectPermissions.browser.spec.ts b/web/workspace/src/lib/workspace/memory/SubjectPermissions.browser.spec.ts index 0c58cb7c3..d37bdbffc 100644 --- a/web/workspace/src/lib/workspace/memory/SubjectPermissions.browser.spec.ts +++ b/web/workspace/src/lib/workspace/memory/SubjectPermissions.browser.spec.ts @@ -46,7 +46,11 @@ afterEach(() => { vi.unstubAllGlobals(); sessionStorage.clear(); }); -function harness(initial: Policy | null = null, canManage = true) { +function harness( + initial: Policy | null = null, + canManage = true, + operations = ops, +) { let policy = initial; const requests: PolicyUpdate[] = []; const fetch = vi.fn(async (_path: string, init?: RequestInit) => { @@ -63,7 +67,7 @@ function harness(initial: Policy | null = null, canManage = true) { return Response.json({ permissions: policy, can_manage: canManage, - operations: ops, + operations, }); }); vi.stubGlobal("fetch", fetch); @@ -262,3 +266,30 @@ test("deleted Subject is refused with no success indication", async () => { expect(screen.queryByRole("combobox")).toBeNull(); expect(h.requests).toHaveLength(0); }); +test("current Worker operation use saves in both lanes without a target editor", async () => { + const available = ops.map((o) => + o.operation === "worker.messaging" + ? { + ...o, + resource_kinds: ["worker_operation"], + availability: { kind: "available" }, + } + : o + ); + const h = harness(null, true, available); + render(SubjectPermissions, props); + await add("execution", "worker.messaging"); + await add("delegation", "worker.messaging"); + expect(screen.queryByRole("textbox", { name: "Worker identifier" })) + .toBeNull(); + expect(screen.queryByRole("textbox", { name: "Runtime identifier" })) + .toBeNull(); + await confirmSave(); + await screen.findByText("Saved and re-read from the current policy."); + const permission = { + operation: "worker.messaging", + resource: { kind: "worker_operation", workspace_id: "w" }, + }; + expect(h.requests[0].execution).toEqual([permission]); + expect(h.requests[0].delegation).toEqual([permission]); +}); diff --git a/web/workspace/src/lib/workspace/memory/permissions.test.ts b/web/workspace/src/lib/workspace/memory/permissions.test.ts index f50038624..d98e03648 100644 --- a/web/workspace/src/lib/workspace/memory/permissions.test.ts +++ b/web/workspace/src/lib/workspace/memory/permissions.test.ts @@ -147,3 +147,38 @@ Deno.test("read permission projection uses the existing scoped endpoint and no f view.operations.length === 0, ); }); + +Deno.test("Worker operation editor binds operation use without a target and preserves installed Plugin scopes", () => { + const resource = makeResource("worker_operation", "w", "s", {}, false); + assert( + JSON.stringify(resource) === + JSON.stringify({ kind: "worker_operation", workspace_id: "w" }), + ); + const view = parseView({ + permissions: { + change_id: "saved", + execution: [{ operation: "worker.messaging", resource }], + delegation: [], + }, + can_manage: true, + operations: [{ + operation: "worker.messaging", + label: "Message Workers", + description: "Directional target grants remain separate", + resource_kinds: ["worker_operation"], + availability: { kind: "available" }, + }], + }); + assert( + view.operations[0].available && + view.permissions?.execution[0].resource.kind === "worker_operation", + ); + const installed = makeResource("plugin_installation", "w", "s", { + installation_id: "installed", + }, false); + assert( + installed.kind === "plugin_installation" && + installed.workspace_id === "w" && + installed.installation_id === "installed", + ); +}); diff --git a/web/workspace/src/lib/workspace/memory/permissions.ts b/web/workspace/src/lib/workspace/memory/permissions.ts index 9708beaf0..dd8007189 100644 --- a/web/workspace/src/lib/workspace/memory/permissions.ts +++ b/web/workspace/src/lib/workspace/memory/permissions.ts @@ -27,6 +27,8 @@ export type Lane = "execution" | "delegation"; const MAX_BYTES = 1_048_576; const fields = { workspace: ["workspace_id"], + worker_operation: ["workspace_id"], + plugin_installation: ["workspace_id", "installation_id"], ticket: ["workspace_id", "ticket_id"], objective: ["workspace_id", "objective_id"], merge_request: ["workspace_id", "merge_request_id"], @@ -195,6 +197,8 @@ export function policyDiff( export function resourceLabel(resource: PermissionResource): string { const labels: Record = { workspace: "Current workspace", + worker_operation: "Worker operation use (target control grant required)", + plugin_installation: "Plugin installation", subject: "Subject", ticket: "Ticket", objective: "Objective", @@ -277,6 +281,7 @@ export function resourceFields(kind: PermissionResource["kind"]): string[] { ); } export const fieldLabel: Record = { + installation_id: "Plugin installation identifier", ticket_id: "Ticket identifier", objective_id: "Objective identifier", merge_request_id: "Merge request identifier", diff --git a/web/workspace/src/routes/w/[workspaceId]/memory/memory-page.browser.spec.ts b/web/workspace/src/routes/w/[workspaceId]/memory/memory-page.browser.spec.ts index d5db94396..3e54b167c 100644 --- a/web/workspace/src/routes/w/[workspaceId]/memory/memory-page.browser.spec.ts +++ b/web/workspace/src/routes/w/[workspaceId]/memory/memory-page.browser.spec.ts @@ -275,7 +275,6 @@ test("warns about an unknown create outcome without replaying the POST", async ( test("renders a ready resident surface and committed Memory lifecycle states", async () => { render(SubjectPage, { data: { - workspace: { permissions: { delete_subjects: false } }, workspaceId: "workspace-1", workspace: memoryWorkspace(), subjectId: "subject-1", @@ -371,7 +370,6 @@ test("edits and clears user-managed behavior with CAS while distinguishing save vi.stubGlobal("fetch", fetchMock); render(SubjectPage, { data: { - workspace: { permissions: { delete_subjects: false } }, workspaceId: "workspace-1", workspace: memoryWorkspace(), subjectId: "subject-1", @@ -444,7 +442,6 @@ test.each([false, true])( function subjectPageData(record = subject(), workspaceId = "workspace-1") { return { - workspace: { permissions: { delete_subjects: false } }, workspaceId, workspace: memoryWorkspace(workspaceId), subjectId: record.id, @@ -778,7 +775,6 @@ test("candidate-only Subjects show no committed Memories, not an inferred candid // the same empty committed list as a new Subject; candidates are not a page input. render(SubjectPage, { data: { - workspace: { permissions: { delete_subjects: false } }, workspaceId: "workspace-1", workspace: memoryWorkspace(), subjectId: "subject-1", @@ -863,7 +859,6 @@ test.each(["ready", "ungenerated", "stale", "failed"] as const)( test("distinguishes ready-empty, stale, failed, unavailable, and request error surfaces", async () => { const base = { - workspace: { permissions: { delete_subjects: false } }, workspaceId: "workspace-1", workspace: memoryWorkspace(), subjectId: "subject-1", @@ -950,7 +945,6 @@ test("distinguishes ready-empty, stale, failed, unavailable, and request error s test("renders committed Memory detail, provenance, derivation, and immutable changes", async () => { render(MemoryDetailPage, { data: { - workspace: { permissions: { delete_subjects: false } }, workspaceId: "workspace-1", workspace: memoryWorkspace(), subjectId: "subject-1",