feat(assistant): align resource tool visibility with read audiences
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
|
||||
@@ -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: {
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user