fix(security): scope persistent-agent routes to the caller workspace - #574
Open
jonwiggins wants to merge 1 commit into
Open
fix(security): scope persistent-agent routes to the caller workspace#574jonwiggins wants to merge 1 commit into
jonwiggins wants to merge 1 commit into
Conversation
Every id-addressed route in routes/persistent-agents.ts resolved the agent
by primary key with no workspace check, letting any authenticated tenant
read, modify, delete, or wake another tenant's persistent agent — a full
cross-tenant takeover (found in an internal security review).
- Add getPersistentAgentScoped(id, workspaceId); rename the unscoped getter
to getPersistentAgentUnscoped so worker/reconciler/internal/webhook/WS
callers keep an explicit unscoped path and routes can't reach it by habit.
- Scope updatePersistentAgent/deletePersistentAgent/setControlIntent by
workspace via a shared wsPredicate.
- Funnel every :id handler through requireAgent(), which 404s (not 403 — no
cross-tenant existence oracle) on a missing/foreign agent. Scope the turn
detail and trigger DELETE by agent id too.
- Guard mutating routes with requireRole("member"), matching tasks.ts.
- Add cross-workspace route tests (404 for foreign caller, same-workspace
still works) for detail/patch/delete/messages/turns/turn-detail/
trigger-delete/control.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SECURITY
Found in an internal security review (no issue number). Cross-tenant takeover of Persistent Agents.
The vulnerability
Every id-addressed route in
apps/api/src/routes/persistent-agents.tsresolved the agent by primary key with no workspace check (getPersistentAgent(id)). Sibling resources (tasks.ts,comments.ts) already scope by workspace; persistent-agents simply omitted it.Because Persistent Agent ids are globally unique UUIDs surfaced in list/detail responses, any authenticated user in workspace B could take the id of an agent in workspace A and:
GET /:id), its inbox messages, its turns, and turn logsPATCH /:id) — including overwritingsystemPrompt/initialPromptDELETE /:id), or add/remove its triggersPOST /:id/messages) and drive its control intent (POST /:id/control)The most severe path: an attacker edits a victim agent's prompt and then wakes it, causing attacker-controlled instructions to execute inside the victim workspace's pod with the victim's connections/secrets. This is a full cross-tenant takeover, not just an info leak. (No working exploit payload is included here by design.)
The fix
Service (
persistent-agent-service.ts)getPersistentAgentScoped(id, workspaceId)— resolves an agent only within a workspace, via a sharedwsPredicate(null→isNull(workspace_id), elseeq), matching the existinggetPersistentAgentBySlug/getPersistentAgentStatspattern.getPersistentAgentUnscopedso the legitimate non-user callers — workers, reconciler, internal inter-agent routes, webhook/schedule trigger dispatch, and the WS stream — keep an explicit unscoped path, and route code cannot reach the unscoped getter by habit.updatePersistentAgent/deletePersistentAgent/setControlIntentworkspace-scoped (newworkspaceIdparam +and(eq(id), wsPredicate(workspaceId))).Routes (
persistent-agents.ts)requireAgent(req, reply, id)helper that fetches via the scoped getter and returns 404 (not 403 — no cross-tenant existence oracle) when the agent is missing or foreign. Every:idhandler funnels through it (detail, patch, delete, messages POST/GET, turns, turn detail, triggers list/create, trigger delete, control).turn.agentId === id; trigger-delete additionally scopes the delete itself to(targetType='persistent_agent', targetId=id)so a foreign trigger id can't be removed via a valid-for-caller:id.preHandler: [requireRole("member")](create, patch, delete, messages POST, triggers POST/DELETE, control), matchingtasks.ts. Read routes are left unguarded.Both
requireRoleand the workspace scoping short-circuit correctly when auth is disabled (isAuthDisabled/ null workspace), so local dev is unaffected.Tests
apps/api/src/routes/persistent-agents.test.tshad zero cross-workspace coverage. Added a suite asserting, for detail / PATCH / DELETE / messages / turns / turn-detail / trigger-delete / control:Verification
cd apps/api && npx tsc --noEmit— cleannpx vitest run(full apps/api unit tier) — 123 files, 2184 tests passpnpm format:checkandpnpm turbo typecheck— pass (pre-commit hooks green)Follow-up (out of scope)
The WebSocket stream
ws/persistent-agents/:agentId/eventsstill resolves the agent unscoped (getPersistentAgentUnscoped), so it can leak another tenant's live events/logs. It is left unchanged here becauseauthenticateWsreturns only the user's default workspace and does no per-request workspace resolution (unlike the HTTP auth plugin'sx-workspace-idhandling), so scoping it correctly for multi-workspace users is a separate change. Recommend a dedicated follow-up.