From c5574599b192328c9e667d0b293b226e11dbb68d Mon Sep 17 00:00:00 2001 From: edgardfrz <131891878+edgardfrz@users.noreply.github.com> Date: Wed, 12 Aug 2026 20:08:31 -0300 Subject: [PATCH] fix(routines): exclude assignee configuration from detail responses (#9818) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 Co-authored-by: Paperclip --- server/src/__tests__/routines-service.test.ts | 66 +++++++++++++++++++ server/src/services/routines.ts | 22 ++++++- 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/server/src/__tests__/routines-service.test.ts b/server/src/__tests__/routines-service.test.ts index 9ffcf04f7e..b634b81330 100644 --- a/server/src/__tests__/routines-service.test.ts +++ b/server/src/__tests__/routines-service.test.ts @@ -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(); diff --git a/server/src/services/routines.ts b/server/src/services/routines.ts index 279232552f..f501696b15 100644 --- a/server/src/services/routines.ts +++ b/server/src/services/routines.ts @@ -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 { + 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),