From 65fe7ce04f3ade232200786af24dc600fcc9c4a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hartmut=20N=C3=B6renberg?= Date: Mon, 30 Mar 2026 10:11:55 +0200 Subject: [PATCH] feat(assistant): align resource tool visibility with read audiences --- docs/route-access-matrix.md | 2 + .../src/__tests__/assistant-router.test.ts | 4 ++ .../api/src/__tests__/resource-router.test.ts | 46 +++++++++++++++++++ packages/api/src/router/assistant.ts | 3 +- 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/docs/route-access-matrix.md b/docs/route-access-matrix.md index 31de257..5b196ed 100644 --- a/docs/route-access-matrix.md +++ b/docs/route-access-matrix.md @@ -106,6 +106,8 @@ Reasoning: - assistant tool visibility must never widen the audience of the backing router - router audience is the source of truth; assistant gating mirrors it - when a route becomes narrower, update assistant visibility in the same hardening slice +- `search_resources` must follow `resourceOverviewProcedure`, not broad authenticated access +- `search_by_skill` must follow `controllerProcedure`, not broad authenticated or planning-only access - if `assistant-tools.ts` already has unrelated local edits, prefer updating `packages/api/src/router/assistant.ts` and parity tests first instead of mixing concerns into the tool implementation file ## Rollout Discipline diff --git a/packages/api/src/__tests__/assistant-router.test.ts b/packages/api/src/__tests__/assistant-router.test.ts index 56a08be..8b0cc5a 100644 --- a/packages/api/src/__tests__/assistant-router.test.ts +++ b/packages/api/src/__tests__/assistant-router.test.ts @@ -357,6 +357,7 @@ describe("assistant router tool gating", () => { expect(controllerNames).toContain("query_change_history"); expect(controllerNames).toContain("get_entity_timeline"); + expect(controllerNames).toContain("search_by_skill"); expect(controllerNames).toContain("export_resources_csv"); expect(controllerNames).toContain("export_projects_csv"); expect(controllerNames).toContain("list_audit_log_entries"); @@ -368,6 +369,7 @@ describe("assistant router tool gating", () => { expect(controllerNames).toContain("get_project_computation_graph"); expect(userNames).not.toContain("query_change_history"); expect(userNames).not.toContain("get_entity_timeline"); + expect(userNames).not.toContain("search_by_skill"); expect(userNames).not.toContain("export_resources_csv"); expect(userNames).not.toContain("export_projects_csv"); expect(userNames).not.toContain("list_audit_log_entries"); @@ -660,8 +662,10 @@ describe("assistant router tool gating", () => { expect(managerNames).not.toContain("create_metro_city"); expect(managerNames).not.toContain("update_metro_city"); expect(managerNames).not.toContain("delete_metro_city"); + expect(userNames).not.toContain("search_resources"); expect(userNames).not.toContain("get_country"); expect(userNames).not.toContain("list_org_units"); + expect(userWithResourceOverview).toContain("search_resources"); expect(userWithResourceOverview).toContain("get_country"); expect(userWithResourceOverview).toContain("list_org_units"); }); diff --git a/packages/api/src/__tests__/resource-router.test.ts b/packages/api/src/__tests__/resource-router.test.ts index 85e618b..9a82726 100644 --- a/packages/api/src/__tests__/resource-router.test.ts +++ b/packages/api/src/__tests__/resource-router.test.ts @@ -697,6 +697,52 @@ describe("resource router", () => { }); }); + it("blocks controller-only resource analytics for plain users", async () => { + const db = { + resource: { + findMany: vi.fn(), + }, + assignment: { + findMany: vi.fn(), + }, + demandRequirement: { + findMany: vi.fn(), + }, + }; + + const caller = createProtectedCaller(db); + + await expect(caller.getSkillsAnalytics()).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Controller access required", + }); + await expect(caller.searchBySkills({ + rules: [{ skill: "Houdini", minProficiency: 4 }], + })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Controller access required", + }); + await expect(caller.listWithUtilization({})).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Controller access required", + }); + await expect(caller.getChargeabilityStats({})).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Controller access required", + }); + await expect(caller.getSkillMarketplace({ + searchSkill: "Houdini", + availableOnly: true, + })).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Controller access required", + }); + + expect(db.resource.findMany).not.toHaveBeenCalled(); + expect(db.assignment.findMany).not.toHaveBeenCalled(); + expect(db.demandRequirement.findMany).not.toHaveBeenCalled(); + }); + it("rejects chargeability summary access for foreign resources", async () => { const db = { resource: { diff --git a/packages/api/src/router/assistant.ts b/packages/api/src/router/assistant.ts index c4a9261..fb850d2 100644 --- a/packages/api/src/router/assistant.ts +++ b/packages/api/src/router/assistant.ts @@ -24,7 +24,6 @@ const MAX_OPENAI_TOOL_DEFINITIONS = 128; const ALWAYS_INCLUDED_TOOL_NAMES = new Set([ "get_current_user", - "search_resources", "get_resource", "search_projects", "get_project", @@ -238,12 +237,14 @@ const PLANNING_READ_TOOLS = new Set([ /** Tools that require broad people-directory visibility because the backing routes expose resource-linked counts. */ const RESOURCE_OVERVIEW_TOOLS = new Set([ + "search_resources", "get_country", "list_org_units", ]); /** Tools that follow controllerProcedure access rules in the main API. */ const CONTROLLER_ONLY_TOOLS = new Set([ + "search_by_skill", "search_projects", "get_project", "search_estimates",