fix(routines): exclude assignee configuration from detail responses (#9818)
## Thinking Path > - Paperclip is the open-source control plane people use to manage AI agents for work. > - Routines are the subsystem that schedules recurring work and returns routine detail to authorized company actors. > - Routine detail embedded the complete assignee database row even though its shared contract requires only assignee identity. > - That full row can contain protected adapter and runtime configuration, including environment bindings. > - The service boundary should project only the fields the routine contract actually needs. > - This pull request replaces the full-row query with a company-scoped identity projection and adds sentinel-based regression coverage. > - The benefit is useful routine detail without exposing protected assignee configuration. ## Linked Issues or Issue Description No public issue exactly tracks this service-level exposure. - Related prior PR: Refs #4967, an older route-level redaction approach with broader changes and no focused routine serialization test. - Related closed PR: Refs #5144, an unmerged prior implementation of the same identity-projection approach. - Related agent-route hardening: Refs #8779; that work covers direct agent responses, while this PR removes protected fields from the routine embed itself. Bug details: - Actual behavior: `GET /api/routines/{routineId}` could serialize the complete assignee row, including protected adapter/runtime configuration. - Expected behavior: routine detail exposes only the assignee identity required by `RoutineDetail`, including its derived `urlKey`. - Reproduction: assign an agent with sentinel-only protected configuration to a routine, retrieve routine detail, and inspect key presence or serialize the response; no production value is needed or recorded. - Version/commit reproduced: upstream `master` immediately before this PR. - Deployment mode: service-level embedded Postgres test; the vulnerable serializer is shared by supported deployments. ## What Changed - Added a company-scoped assignee summary query in `server/src/services/routines.ts` that selects only `id`, `name`, `role`, and `title`, then derives the non-sensitive `urlKey` from the name. - Updated `getDetail()` to use that projection instead of selecting the complete agent row. - Added focused negative and positive identity assertions, including the derived `urlKey`, in `server/src/__tests__/routines-service.test.ts`. - Audited routine list/detail serialization and broader embedded-agent query sites; routine list exposes only `assigneeAgentId`, while other agent embeds use explicit projections or authorized agent endpoints. ## Verification - `pnpm exec vitest run server/src/__tests__/routines-service.test.ts` — 57/57 passed. - Focused sentinel regression test — passed. - `pnpm -r typecheck` — passed. - Server, UI, and CLI builds — passed; UI gzip-size completion used a 4096 MB Node heap. - `git diff --check` — passed. - Full `pnpm test:run` — 2,699 passed, 1 skipped, 9 failed in untouched tests. The failures reproduce outside this change and are limited to local-adapter `nohup`/PTY behavior, macOS `/tmp` versus `/private/tmp` normalization, and one workspace-runtime auto-port fixture. ## Risks - Low compatibility risk: the returned shape now matches the existing shared `RoutineDetail` contract. - A consumer relying on undocumented protected agent fields inside routine detail will stop receiving them. - No schema, migration, deployment, credential, or production-secret changes are included. - Rollback is a single commit revert, but reverting would restore the exposure. > This is security hardening for the already-shipped routines subsystem; `ROADMAP.md` marks Scheduled Routines complete, and this PR does not add or duplicate roadmap feature work. ## Model Used - OpenAI GPT-5 via Codex, with repository search, local code execution, tests, TypeScript typechecking, builds, Git, and GitHub API use. The runtime does not expose a more granular snapshot ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and relevant tests pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes (no documentation change is required for this contract-preserving security fix) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green 5 with no open P2s, recommendations, or follow-ups/- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: ClawdeBot <clawdebot@Mac-mini-de-ClawdeBot.local> Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
edgardfrz
GitHub
parent
8b1483e601
commit
c5574599b1
|
|
@ -612,6 +612,72 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => {
|
|||
expect(routine.status).toBe("paused");
|
||||
});
|
||||
|
||||
it("serializes routine detail with assignee identity but without protected agent configuration", async () => {
|
||||
const { agentId, companyId, routine, svc } = await seedFixture();
|
||||
const sentinelSecret = "routine-assignee-secret-sentinel";
|
||||
await db
|
||||
.update(agents)
|
||||
.set({
|
||||
adapterConfig: {
|
||||
env: {
|
||||
ROUTINE_ASSIGNEE_SECRET: { type: "plain", value: sentinelSecret },
|
||||
},
|
||||
},
|
||||
runtimeConfig: {
|
||||
modelProfiles: {
|
||||
cheap: {
|
||||
adapterConfig: {
|
||||
env: {
|
||||
ROUTINE_ASSIGNEE_RUNTIME_SECRET: { type: "plain", value: sentinelSecret },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
})
|
||||
.where(eq(agents.id, agentId));
|
||||
const { trigger } = await svc.createTrigger(routine.id, {
|
||||
kind: "schedule",
|
||||
label: "Daily",
|
||||
cronExpression: "0 10 * * *",
|
||||
timezone: "UTC",
|
||||
}, {});
|
||||
|
||||
const detail = await svc.getDetail(routine.id);
|
||||
|
||||
expect(detail).toMatchObject({
|
||||
id: routine.id,
|
||||
companyId,
|
||||
title: "ascii frog",
|
||||
assignee: {
|
||||
id: agentId,
|
||||
name: "CodexCoder",
|
||||
role: "engineer",
|
||||
title: null,
|
||||
urlKey: "codexcoder",
|
||||
},
|
||||
triggers: [{
|
||||
id: trigger.id,
|
||||
kind: "schedule",
|
||||
label: "Daily",
|
||||
cronExpression: "0 10 * * *",
|
||||
timezone: "UTC",
|
||||
}],
|
||||
});
|
||||
expect(detail?.assignee).toEqual({
|
||||
id: agentId,
|
||||
name: "CodexCoder",
|
||||
role: "engineer",
|
||||
title: null,
|
||||
urlKey: "codexcoder",
|
||||
});
|
||||
|
||||
const serialized = JSON.stringify(detail);
|
||||
expect(serialized).not.toContain(sentinelSecret);
|
||||
expect(serialized).not.toContain("adapterConfig");
|
||||
expect(serialized).not.toContain("runtimeConfig");
|
||||
});
|
||||
|
||||
it("creates revision 1 on routine create and appends revisions for real updates only", async () => {
|
||||
const { routine, svc } = await seedFixture();
|
||||
|
||||
|
|
|
|||
|
|
@ -50,6 +50,7 @@ import {
|
|||
extractRoutineVariableNames,
|
||||
interpolateRoutineTemplate,
|
||||
isValidRoutineDateString,
|
||||
normalizeAgentUrlKey,
|
||||
pluginOperationIssueOriginKind,
|
||||
routineRevisionSnapshotSchema,
|
||||
stringifyRoutineVariableValue,
|
||||
|
|
@ -644,6 +645,25 @@ export function routineService(
|
|||
.then((rows) => rows[0] ?? null);
|
||||
}
|
||||
|
||||
async function getRoutineAgentSummary(
|
||||
companyId: string,
|
||||
agentId: string,
|
||||
): Promise<RoutineDetail["assignee"]> {
|
||||
return db
|
||||
.select({
|
||||
id: agents.id,
|
||||
name: agents.name,
|
||||
role: agents.role,
|
||||
title: agents.title,
|
||||
})
|
||||
.from(agents)
|
||||
.where(and(eq(agents.companyId, companyId), eq(agents.id, agentId)))
|
||||
.then((rows) => {
|
||||
const row = rows[0];
|
||||
return row ? { ...row, urlKey: normalizeAgentUrlKey(row.name) ?? row.id } : null;
|
||||
});
|
||||
}
|
||||
|
||||
async function getManagedRoutineBinding(routine: typeof routines.$inferSelect) {
|
||||
return db
|
||||
.select({
|
||||
|
|
@ -1979,7 +1999,7 @@ export function routineService(
|
|||
? db.select().from(projects).where(eq(projects.id, row.projectId)).then((rows) => rows[0] ?? null)
|
||||
: null,
|
||||
row.assigneeAgentId
|
||||
? db.select().from(agents).where(eq(agents.id, row.assigneeAgentId)).then((rows) => rows[0] ?? null)
|
||||
? getRoutineAgentSummary(row.companyId, row.assigneeAgentId)
|
||||
: null,
|
||||
row.parentIssueId ? issueSvc.getById(row.parentIssueId) : null,
|
||||
getRoutineDescriptionDocument(row.id),
|
||||
|
|
|
|||
Loading…
Reference in New Issue