Skip to content

[Fix] Job tokens can no longer reassign their run's acting user - #82

Merged
mrubens merged 6 commits into
developfrom
fix/harden-acting-user-write
Jul 10, 2026
Merged

[Fix] Job tokens can no longer reassign their run's acting user#82
mrubens merged 6 commits into
developfrom
fix/harden-acting-user-write

Conversation

@mrubens

@mrubens mrubens commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Vulnerability (flagged in PR #80 review)

The job-scoped cloudJobs.update tRPC mutation (packages/sdk/src/server/routers/cloud-jobs.ts) accepted actingUserId: z.string().optional(). A run-scoped job token is held by the sandbox runtime, and resolveActorScopedUserContext (packages/sdk/src/server/lib/auth/resolve-actor-scoped-user.ts) — plus the MCP proxy's resolveActingUserIdprefer the run's live task_runs.actingUserId when resolving the effective user for actor-scoped credential routes (userApiKeys.getDecryptedKey, user-scoped mcpConnections).

Confused-deputy chain: a compromised/misbehaving sandbox → cloudJobs.update({ id, actingUserId: <victim> }) with its own job token → then userApiKeys.getDecryptedKey (or a user-scoped MCP connection lookup) resolves to the victim → reads the victim's decrypted API keys / connections.

Fix

Removed actingUserId from the job-token-writable update input schema. zod strips the field, so a job-token caller can never persist it. Acting-user reassignment remains available only to the trusted server-side writers, which write task_runs directly and are unchanged:

  • web steer — apps/web/src/trpc/commands/sandbox-session/index.ts (sets it from the authenticated web user)
  • follow-up delivery — apps/api/src/handlers/tasks/sendMessageToTask.ts syncActingUserIdAfterDelivery

The read-side model (PR #80) is unchanged: actingUserId is still the live-trusted attribution source and is still preferred over the token's mint-time userId.

Worker audit

The only worker path that wrote actingUserId through the job token was the SDK helper syncActingUserId (packages/sdk/src/cloud-jobs.ts), called during actor-scoped turn preparation to switch the actor to a queued follow-up sender before delivering that sender's turn. No code in apps/worker/src puts actingUserId into an update call directly.

This switch is a legitimate relay of a server-provided queued-sender id, but it cannot be safely preserved:

  • the sandbox and worker host share one run-scoped token (createJobToken mints userId = actingUserId, no worker-vs-sandbox principal distinction), so the legit switch is indistinguishable from the attack at the server;
  • the queued-message store is Redis, destructive-on-read (getCommunicationMessages deletes the key), so there is no durable record of pending senders to validate a target against.

Per the reviewer's stated intent ("the worker should never choose its own acting user"), syncActingUserId is now reconcile-only: it observes the server-authoritative value and never writes, logging a warning if the requested actor diverges.

Trade-off (reported, not a leak): in a rare multi-user snapshot-resume batch, a later sender's turn now runs under the earlier, server-set actor instead of switching mid-batch. Both are legitimate participants of the same run who already share its actor-scoped surface turn-by-turn, so this is a within-run mis-attribution, not a cross-user credential leak. A future server-authoritative per-turn actor sync could restore precise switching if desired.

Other job-token-writable fields audited

While in the update schema I checked the remaining fields for confused-deputy potential:

  • taskId (worth a follow-up): writable by a job token but not sent by any worker update call (all taskId: references in the worker are reads). Re-pointing a run to another task would corrupt run→task attribution/visibility, but taskId does not feed resolveActorScopedUserContext, so it is a data-integrity risk, strictly lesser than credential theft. Left as-is to keep this PR scoped to the credential vector; recommend removing it from the job-token input in a follow-up.
  • status / result / taskPhase / sleepAt: legitimate worker-written runtime/output fields, no auth/attribution role.
  • Sibling job-token mutations (done, updateRuntimeState, stampMilestone, touchCloudJobHeartbeat): none write auth/attribution fields.

Tests

  • packages/sdk/src/server/routers/cloud-jobs.test.ts: a job-token update carrying actingUserId is stripped (never reaches updateCloudJob); legitimate status/result fields still persist.
  • packages/sdk/src/server/routers/user-api-keys.test.ts: end-to-end shape — a job token cannot pivot getDecryptedKey to a victim; the lookup is scoped only to the persisted (legitimate) actingUserId.
  • packages/sdk/src/cloud-jobs.test.ts: syncActingUserId never writes on divergence and warns.
  • Existing web-steer / follow-up-sync / worker actor-scoped tests still pass.

Validation

pnpm format + full pnpm check (lint, check-types, test, knip) all pass.

Flagged in PR #80 review. The job-scoped cloudJobs.update mutation accepted
`actingUserId`, and a run-scoped job token is held by the sandbox runtime.
`resolveActorScopedUserContext` (and the MCP proxy's resolveActingUserId)
PREFER the run's live `task_runs.actingUserId` when resolving the effective
user for actor-scoped credential routes (userApiKeys.getDecryptedKey,
user-scoped mcpConnections). Chain: a compromised/misbehaving sandbox could
self-reassign actingUserId to any user id, then read that user's decrypted
API keys or user-scoped connections — a confused deputy.

Fix: remove `actingUserId` from the job-token-writable `update` input schema
(zod strips it). Acting-user reassignment stays with the trusted server-side
writers that write task_runs directly: web steer (apps/web sandbox-session)
and follow-up delivery (apps/api sendMessageToTask syncActingUserIdAfterDelivery).

Worker audit: the only worker path that wrote actingUserId via the job token
was the SDK helper syncActingUserId (packages/sdk/src/cloud-jobs.ts), used
during actor-scoped turn preparation. It set actingUserId to a queued
follow-up sender for per-turn actor switching. Because the sandbox and worker
host share one run-scoped token with no principal distinction, this legitimate
switch is indistinguishable from the attack at the server, and the Redis
message queue is destructive-on-read so no durable participant record exists to
validate against. Per the reviewer's stated intent ("the worker should never
choose its own acting user"), the helper is now reconcile-only: it observes the
server-authoritative value and never writes. Trade-off: in a rare multi-user
snapshot-resume batch, a later sender's turn runs under the earlier
(server-set) actor rather than switching — a within-run mis-attribution among
participants, not a cross-user credential leak.

Tests: schema strips actingUserId from job-token update input; job token cannot
pivot getDecryptedKey to a victim; SDK syncActingUserId never writes on
divergence. Comments updated in resolve-actor-scoped-user, proxy-utils, and
user-api-keys.
@roomote-roomote

roomote-roomote Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

2 issues outstanding. See task

  • High: Do not execute a mismatched sender's queued prompt as the server actor (apps/worker/src/run-task/prepare-actor-scoped-turn.ts:124-162, with callers such as apps/worker/src/run-task/polling/slack.ts:206-243). The distinct mismatch result detects the problem, but follow-server still delivers user B's prompt text while actor-scoped MCP and API-key routes resolve user A; replacing the prompt's userId metadata with A does not change who supplied the instructions. Concurrent pre-queue writes, the intentionally non-fatal inbound sync, and a failed direct delivery after its pre-delivery write all make this reachable. Block/requeue mismatched content or perform a trusted, per-message actor switch immediately before delivery.
  • Medium: Restore or remove the now-unreachable updated contract (packages/sdk/src/cloud-jobs.ts:28,131-139). syncActingUserId can now return only unchanged or not-found, so the worker's syncResult === 'updated' branch and its only call to syncRuntimeGitAuthor are dead; commits after an actor switch can retain the previous user's git identity. Make git-author synchronization follow server-authoritative actor changes and update the worker test, or remove the dead contract and explicitly accept the behavior change.
  • Medium: Retry git-author synchronization after a transient failure (apps/worker/src/run-task/prepare-actor-scoped-turn.ts:145-159). The catch logs syncRuntimeGitAuthor failures but still calls onActorSynced, advancing lastPreparedActorUserId; the next turn therefore receives unchanged and never retries the idempotent git-author update, so subsequent commits can retain the previous actor's identity for the rest of the run. Advance the local actor marker only after the author sync succeeds, or track the failed sync separately for retry.
  • High: Keep skipped Slack question answers recoverable (apps/worker/src/run-task/polling/slack.ts:134-138). submitPendingSlackRequestUserInputAnswer atomically queues the answer and marks the pending request submitted; dropping that queued answer on an actor mismatch leaves the harness waiting, while the notice's suggested resend is rejected as “already received” by the Slack webhook. Perform the trusted actor sync before accepting the answer, or restore the pending request to a state in which the sender can answer again instead of permanently discarding it.
  • High: Keep skipped Linear question answers recoverable (apps/worker/src/run-task/polling/linear.ts:104-108). The Linear webhook queues the answer and marks the request submitted, so dropping the queued answer on mismatch leaves the harness waiting and every later answer is rejected by the submitted guard. Perform the trusted actor sync before accepting the answer, or reset the pending request before asking the sender to resend.
  • Medium: Retry a partially written git identity even after the actor switches back (apps/worker/src/run-task/prepare-actor-scoped-turn.ts:175-193). syncRuntimeGitAuthor writes user.email before user.name; if the second command fails during A→B and the server switches back to A before another turn, the marker still equals A, reconciliation returns unchanged, and the mixed B-email/A-name configuration is never repaired. Track a failed author sync independently of actor identity or update both fields atomically.
  • High: Do not roll the actor back after an ambiguous sandbox RPC failure (apps/api/src/handlers/tasks/sendMessageToTask.ts:815-822,972-979). A timeout or lost response can occur after the sandbox accepted B's native-steered prompt; the unconditional B→A rollback then lets B's injected instructions continue while live actor-scoped credential routes resolve A. Roll back only deterministic pre-accept failures, or add an acceptance/idempotency protocol and revalidate native-steered turns before execution.
  • High: Do not treat Redis answer claims as atomic with a PostgreSQL transaction (packages/db/src/lib/acting-user.ts:44-69). operation() commits the Redis queue/submitted side effects before the actor update and PostgreSQL commit; if the update or commit fails, the answer is permanently marked submitted while the actor stays unchanged, and the worker requeues the mismatch forever. Use a recoverable cross-store protocol such as compensation or an outbox instead of relying on the row lock as atomicity.
  • Medium: Make the Linear answer claim reject concurrent losers (apps/api/src/handlers/linear/index.ts:586-603). — dismissed: every valid event in a Linear agent session derives userId from that session's fixed agentSession.creator, so concurrent claimants cannot race with different actors; duplicate same-actor answers do not create the reported actor mismatch.

…re delivery, worker follows the server

Addresses both PR #82 review findings on the reconcile-only change.

High (mismatch must not run the turn): syncActingUserId previously warned on
divergence and returned 'unchanged', so a sender's prompt could run while
actor-scoped routes still resolved the previous actor. Now:

- Trusted writes happen BEFORE delivery everywhere. sendMessageToTask and
  steerMessageToTask write task_runs.actingUserId (updateActingUserIdIfNeeded,
  moved to apps/api/src/handlers/tasks/acting-user-sync.ts) before the sandbox
  RPC; a failed required switch aborts delivery (500) instead of the old
  swallowed post-delivery sync. Webhook handlers that feed the worker's polled
  queues (Slack active-job, Teams x2, Telegram, Linear) perform a trusted
  pre-queue actor sync from the sender they resolved themselves
  (syncActingUserForInboundMessage, non-fatal), replacing the worker-side
  write these paths previously relied on.
- SDK syncActingUserId returns { result, actingUserId } with a distinct
  'mismatch' result when the server actor differs from the requested sender,
  and never writes.
- The worker enforces the invariant: sandbox RPC surfaces (sendPrompt,
  steerTask, answerUserInputRequest) use the default 'block' policy - a
  mismatch there means the trusted pre-delivery write did not happen, so the
  turn is refused with a retryable error. Queued/polled deliveries
  (Slack/Teams/Telegram/Linear polls, snapshot-resume replays, queued-prompt
  boundaries) use 'follow-server': requeueing cannot converge (the server
  value only moves via trusted writes, so a blocked queue would stall until
  the Redis TTL drops the messages), so the turn runs AND is attributed as
  the server actor - integrations refresh and sendPrompt userId both use the
  server value, never the sender's identity over someone else's credentials.

Medium ('updated' unreachable / dead git-author branch): 'updated' now means
the server actor matches the sender but differs from the worker's
last-prepared actor (tracked per run, seeded from cloudJob.actingUserId), and
the revived branch runs syncRuntimeGitAuthor FROM the server value - commits
after a trusted actor switch carry the new identity. Follow-server mismatches
that change the local actor refresh the git author the same way.

Ordering reasoning (pre-delivery write failure mode): if the actor switch
lands but delivery then fails, the run attributes to the latest human who
attempted to engage; none of their prompts ran, the write is idempotent, and
the next successful send from any sender re-syncs it.

Tests: sendMessageToTask/steerMessageToTask assert write-before-RPC invocation
order and no-delivery-on-write-failure; SDK tests cover all four outcomes and
assert no job-token write ever happens; worker tests cover block-on-mismatch,
follow-server delivery under the server actor (integrations + git author +
attribution from the server value), updated-triggers-git-author-sync, and the
polled surfaces attributing turns to the effective user. New unit tests for
the acting-user-sync helpers.
mrubens pushed a commit that referenced this pull request Jul 10, 2026
Remove taskId from the job-scoped cloudJobs.update input schema. The
mutation is reachable with a run-scoped job token held by the sandbox
runtime, and a run's task binding drives attribution, visibility, and
PR linkage; runs bind to a task at enqueue and never re-parent. No
worker code path sends the field. Mirrors the actingUserId hardening
in #82 (lower severity: taskId does not feed credential resolution).
roomote added 4 commits July 10, 2026 10:38
Reconciles the two mirrored job-token hardening PRs: this branch stripped
actingUserId from the job-token update schema, develop's #83 stripped
taskId — the resolution strips both, keeps both guard comments, and keeps
both regression tests (ported to the #81 Run/Task vocabulary).
…til it lands

A message whose sender is not the run's server-side acting user is no
longer delivered at all: the former follow-server policy relabeled the
turn's attribution but still executed the mismatched sender's content
under the server actor's credential resolution. The skip policy drops
that message's content (never requeued), posts a best-effort resend
notice to the task's chat thread (one per sender per run), and keeps
draining the rest of the queue. Resending re-enters the webhook's
trusted pre-queue actor sync, so the resent message runs as its sender.
The block policy on sandbox RPC surfaces is unchanged, and the harness
queued-prompt boundary gains a shouldSkipPrompt outcome so a skipped
prompt cannot crowd the queue with block-and-retry stalls.

Separately, a failed runtime git-author sync no longer advances the
worker's last-prepared actor marker, so the next turn reports `updated`
again and retries the author sync until it succeeds instead of leaving
the previous actor's git identity on all later commits.
@mrubens
mrubens merged commit b0488c5 into develop Jul 10, 2026
@mrubens
mrubens deleted the fix/harden-acting-user-write branch July 10, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants