feat(auth): revoke an agent credential without deleting the record - #305
feat(auth): revoke an agent credential without deleting the record#305khaliqgant wants to merge 6 commits into
Conversation
Containing a leaked `at_live_` token had no supported path. Both routes operators were pointed at fail, in ways that look like success: `remove_agent` dispatches a release to the node. It stops a process and returns `dispatched`. It never touches the credential — `status` is not consulted during authentication at all, so a released, offline, roster-absent agent authenticates exactly as well as a running one. `DELETE /v1/agents/:name` fails server-side on any seat with history. Four foreign keys onto `agents(id)` are ON DELETE NO ACTION (`messages.agent_id`, `channels.created_by`, `files.uploaded_by`, `webhooks.created_by`), so a seat that has posted one message aborts with a FOREIGN KEY constraint error. The seats it can delete are the ones that never posted — deletion succeeds only where there is nothing to preserve and fails exactly where there is. Where it does succeed it takes history with it: `dm_participants.agent_id` cascades, which is how two-party DMs collapsed to one-row rosters (see the note in scripts/audit-dm-reservations.mjs). So revocation is a state on the row, not the absence of the row. `revoked_at` is checked in the agent branch of authenticate(), the only lookup that resolves an agent token to an identity — the realtime WS path rejects agent tokens outright, so there is no second door. Refusal reports `agent_token_revoked`, distinct from `agent_token_invalid`, so an operator can tell a contained credential from one that never existed; a deleted row cannot express that difference. Deliberately not folded into `status`, which presence rewrites to 'active' on every touch. Deliberately not a rotation: re-registering returns the live token in its reply (relay#1389), putting a fresh credential straight back into a transcript. Tests assert the only thing that counts — the credential is presented and authentication is refused — and pin the contrast: on one seat with one message, deleteAgent throws while revokeAgentToken succeeds and the message survives. The migration is additive (NULL means active); an unmigrated deployment accepts the revoke call and still authenticates the token, so the runbook makes the 401 receipt, not the API response, the evidence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The repo's own catalog guard caught this: an event name not present in SERVER_TELEMETRY_EVENTS fails the zod enum inside a floating promise, so it is dropped before reaching PostHog with no error surfaced. The revoke endpoint would have emitted into nothing — leaving the one action that contains a credential as the one action with no telemetry.
The first draft claimed an unmigrated deployment would accept the revoke call and keep authenticating the token. That is wrong, and wrong in the more dangerous direction. The drizzle schema enumerates every column on each query, so a build carrying `revoked_at` cannot talk to an `agents` table without it — verified against an unmigrated schema, where an ordinary insert fails with "table agents has no column named revoked_at". Deploying the code ahead of the migration takes agent registration and authentication down. Also names the instance to migrate: the SST resource, not the repo-matching name, with --remote.
`POST /v1/agents/:name/rotate-token` does invalidate a leaked credential — it overwrites token_hash, so the old token stops authenticating. An operator hunting for an invalidation path will find it and use it. The reason not to is not that it fails, it is that it returns the replacement token in its response body (relay#1389), trading a known-leaked credential for a freshly-leaked one. Saying only "do not rotate" does not survive contact with someone who can see that the endpoint works.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 25 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThis change adds persistent agent-token revocation. It introduces database storage, engine logic, authentication rejection, a workspace-key-protected endpoint, telemetry, tests, and operational documentation. ChangesAgent token revocation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant AgentRoute
participant AgentEngine
participant Database
participant AgentAuth
Operator->>AgentRoute: POST /v1/agents/:name/revoke with workspace key
AgentRoute->>AgentEngine: Revoke agent token
AgentEngine->>Database: Store revoked_at
Database-->>AgentEngine: Return revocation timestamp
AgentEngine-->>AgentRoute: Return revocation details
AgentRoute-->>Operator: Return receipt and telemetry
Operator->>AgentAuth: Authenticate with revoked token
AgentAuth->>Database: Read agent revokedAt
Database-->>AgentAuth: Return revoked agent
AgentAuth-->>Operator: Return 401 agent_token_revoked
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/revoking-an-agent-credential.md`:
- Around line 104-107: Update the fenced response example in the
credential-revocation documentation to declare the JSON language, changing the
untyped fence around the response body to a json-tagged fence while preserving
its contents and status code.
In `@packages/engine/src/engine/agent.ts`:
- Around line 323-327: Update the revocation result logic after the re-read in
the agent revocation flow to return null when no persisted row or revokedAt
value is found, rather than falling back to the local revokedAt timestamp.
Preserve the existing settled.revokedAt value and alreadyRevoked behavior when
revocation data exists.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 64c5e338-b7d0-40b3-83e7-45d705c0c533
📒 Files selected for processing (8)
docs/revoking-an-agent-credential.mdpackages/engine/src/auth/index.tspackages/engine/src/db/migrations/0034_agent_token_revocation.sqlpackages/engine/src/db/schema.tspackages/engine/src/engine/__tests__/agentRevocation.test.tspackages/engine/src/engine/agent.tspackages/engine/src/routes/agent.tspackages/types/src/telemetry.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 759ceeb053
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // second door. Distinct code from `agent_token_invalid` so an operator can | ||
| // tell "revoked" from "never existed" — a deleted row would report the | ||
| // latter, and the difference is the whole point of keeping the record. | ||
| if (agent.revokedAt) return unauthorized('Agent token revoked', 'agent_token_revoked'); |
There was a problem hiding this comment.
When the revoked seat is an A2A proxy, its issued relay_token remains usable: routes/a2a.ts:119-129 fetches only agents.tokenHash, and the webhook at lines 312-318 authenticates by comparing that hash directly instead of calling this provider or checking revokedAt. Consequently, revoking the proxy reports successful containment while the leaked credential can still submit webhook payloads; include the revocation state in that lookup and reject revoked agents there.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in 112b31a. This was the most important finding on the PR — it falsified the claim in my own code comment that there was "no second door".
Verified before fixing: the webhook has no auth middleware and compared agents.token_hash itself, so the revocation check never ran. I did not patch the comparison in place, because patching known routes one at a time leaves the class of defect intact. The route now resolves the bearer token through the configured AuthProvider and re-checks the binding afterwards, so provider-level checks apply there by construction.
Guarded by an HTTP-level test that registers a real A2A proxy, confirms the webhook accepts the credential, revokes, and requires 401 agent_token_revoked. I verified the test fails against the pre-fix code (the revoked credential got through to payload validation and returned 400).
| const db = c.get('db'); | ||
| const workspace = c.get('workspace'); | ||
| const name = c.req.param('name'); | ||
| const result = await agentEngine.revokeAgentToken(db, workspace.id, name); |
There was a problem hiding this comment.
In deployments that inject a custom AuthProvider, particularly one backed by an external identity store as permitted by ports/auth.ts, this call only marks the engine database row and then returns a successful receipt; the provider has no revocation operation and may continue accepting the token indefinitely. Delegate revocation through the provider or add provider-independent enforcement so the endpoint works for both the hosted gateway and self-hosting.
AGENTS.md reference: AGENTS.md:L10-L11
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, fixed in 112b31a. Marking the engine row while a custom provider authenticates from its own store is a clean receipt over a live credential — the exact failure mode this endpoint exists to prevent.
Revocation is now an optional revokeAgentCredential on the AuthProvider port. The built-in provider implements it; a provider with no authority to invalidate a credential it did not issue leaves it undefined and the endpoint fails closed with revocation_unsupported (501). No capability, no receipt — rather than a 200 that means nothing.
| agentRoutes.post( | ||
| '/agents/:name/revoke', |
There was a problem hiding this comment.
The new POST /agents/{name}/revoke operation and agent_token_revoked error are absent from openapi.yaml, while README.md still states that invalid or expired agent tokens always return agent_token_invalid and recommends rotation. This leaves generated clients without the endpoint and gives users contradictory recovery guidance, so update both public API sources with this behavior.
AGENTS.md reference: AGENTS.md:L34-L36
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 112b31a. Added POST /agents/{name}/revoke to openapi.yaml with its 200/404 responses, extended the Error.code description to cover agent_token_revoked, and corrected the README, which previously told users that rotation was the recovery path for a bad agent token — actively wrong advice for a deliberately revoked credential. Verified the spec still parses and the $refs resolve.
| -- Additive and reversible: a NULL `revoked_at` is an active credential, so | ||
| -- existing rows keep their current behaviour with no backfill. | ||
|
|
||
| ALTER TABLE agents ADD COLUMN revoked_at INTEGER; |
There was a problem hiding this comment.
This commit adds a user-facing endpoint, database migration, and exported telemetry event, but the root, engine, and types changelogs remain at empty [Unreleased] headings. A release cut would therefore omit the feature and migration requirements and lack the required SemVer level; add concise pending entries and raise the applicable headings.
AGENTS.md reference: AGENTS.md:L39-L44
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 112b31a. Pending [Unreleased - Minor] entries added to the root, engine, types, and SDK changelogs per AGENTS.md, including the migration-ordering requirement (0034 must be applied before or with the code — the schema enumerates every column per query, so the build cannot talk to an agents table without it).
| // second door. Distinct code from `agent_token_invalid` so an operator can | ||
| // tell "revoked" from "never existed" — a deleted row would report the | ||
| // latter, and the difference is the whole point of keeping the record. | ||
| if (agent.revokedAt) return unauthorized('Agent token revoked', 'agent_token_revoked'); |
There was a problem hiding this comment.
When a TypeScript SDK request uses a revoked token, the server now returns this distinct code, but packages/sdk-typescript/src/errors.ts neither includes nor maps agent_token_revoked; its 401 fallback converts it to generic unauthorized. That erases the distinction this change intentionally introduces and prevents typed clients from recognizing deliberate revocation, so add the code to the SDK error union and normalization map.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 112b31a — agent_token_revoked added to the RelayErrorCode union and to RAW_CODE_MAP, so it survives normalization instead of collapsing to unauthorized.
| const [settled] = await db.select().from(agents).where(eq(agents.id, agent.id)); | ||
| return { | ||
| revokedAt: settled?.revokedAt ?? revokedAt, | ||
| alreadyRevoked: false, |
There was a problem hiding this comment.
When two revoke requests select the active row before either update completes, one guarded update is a no-op, but both callers unconditionally return alreadyRevoked: false. The re-read preserves the winning timestamp but does not correct the receipt semantics, so concurrent operator requests both claim to have performed the fresh revocation; inspect the update result or use an atomic update/returning path to mark the loser as already revoked.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 112b31a. The guarded UPDATE now uses returning(), so the call that did not set the column reports already_revoked: true. Covered by a test that lands another revoke inside the read-then-write window and asserts both the flag and that the winning timestamp is the one reported.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/engine/src/routes/agent.ts">
<violation number="1" location="packages/engine/src/routes/agent.ts:359">
P3: This adds a new public endpoint `POST /v1/agents/:name/revoke` without updating the API reference. The repo's docs convention (AGENTS.md "Docs Hygiene") requires README and openapi.yaml to be updated together when API behavior changes; openapi.yaml documents the sibling `/agents/{name}/rotate-token` but has no `/revoke` entry. Please add the new endpoint to openapi.yaml (and note it in README) so the documented surface stays in sync with the implemented routes before this ships.</violation>
<violation number="2" location="packages/engine/src/routes/agent.ts:360">
P2: The new HTTP containment workflow has no request-level test, so regressions in route mounting, `requireWorkspaceKey`, the unknown-agent response, or the documented idempotent response could pass the current suite. A conformance test covering workspace-key success, agent-token rejection, unknown-agent 404, and repeated revocation would make the runbook contract executable.</violation>
</file>
<file name="packages/engine/src/db/migrations/0034_agent_token_revocation.sql">
<violation number="1" location="packages/engine/src/db/migrations/0034_agent_token_revocation.sql:8">
P3: The migration rationale incorrectly implies that a successful agent delete means there is no history; cascade-only references can make the delete succeed while silently removing agent-owned records. Please explain that `NO ACTION` references block only some deletes and that cascade references can still discard history.</violation>
<violation number="2" location="packages/engine/src/db/migrations/0034_agent_token_revocation.sql:22">
P3: This change adds a new endpoint, migration, and telemetry event but doesn't add a corresponding changelog entry, so a release cut would omit the feature and its migration requirement.</violation>
</file>
<file name="packages/engine/src/auth/index.ts">
<violation number="1" location="packages/engine/src/auth/index.ts:60">
P1: Revoking an A2A relay agent does not contain its credential: `/a2a/webhook/:workspace_id/:agent_name` bypasses `SqliteApiKeyAuthProvider` and still accepts the revoked `at_live_` token via a direct hash comparison. Applying the same revocation check there or routing this authentication through the provider would close the second door.</violation>
<violation number="2" location="packages/engine/src/auth/index.ts:60">
P2: The new agent_token_revoked error code isn't added to the TypeScript SDK's error union/normalization map, so its 401 fallback collapses it into a generic unauthorized error, erasing the distinction between a revoked token and other auth failures for SDK consumers.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // second door. Distinct code from `agent_token_invalid` so an operator can | ||
| // tell "revoked" from "never existed" — a deleted row would report the | ||
| // latter, and the difference is the whole point of keeping the record. | ||
| if (agent.revokedAt) return unauthorized('Agent token revoked', 'agent_token_revoked'); |
There was a problem hiding this comment.
P1: Revoking an A2A relay agent does not contain its credential: /a2a/webhook/:workspace_id/:agent_name bypasses SqliteApiKeyAuthProvider and still accepts the revoked at_live_ token via a direct hash comparison. Applying the same revocation check there or routing this authentication through the provider would close the second door.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/engine/src/auth/index.ts, line 60:
<comment>Revoking an A2A relay agent does not contain its credential: `/a2a/webhook/:workspace_id/:agent_name` bypasses `SqliteApiKeyAuthProvider` and still accepts the revoked `at_live_` token via a direct hash comparison. Applying the same revocation check there or routing this authentication through the provider would close the second door.</comment>
<file context>
@@ -51,6 +51,13 @@ export class SqliteApiKeyAuthProvider implements AuthProvider {
+ // second door. Distinct code from `agent_token_invalid` so an operator can
+ // tell "revoked" from "never existed" — a deleted row would report the
+ // latter, and the difference is the whole point of keeping the record.
+ if (agent.revokedAt) return unauthorized('Agent token revoked', 'agent_token_revoked');
const [workspace] = await db.select().from(workspaces).where(eq(workspaces.id, agent.workspaceId));
if (!workspace) return unauthorized('Workspace not found');
</file context>
| // Workspace key only: an agent must not be able to revoke itself or a peer. | ||
| // Returns the revocation timestamp so the caller has a receipt to record. | ||
| agentRoutes.post( | ||
| '/agents/:name/revoke', |
There was a problem hiding this comment.
P2: The new HTTP containment workflow has no request-level test, so regressions in route mounting, requireWorkspaceKey, the unknown-agent response, or the documented idempotent response could pass the current suite. A conformance test covering workspace-key success, agent-token rejection, unknown-agent 404, and repeated revocation would make the runbook contract executable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/engine/src/routes/agent.ts, line 360:
<comment>The new HTTP containment workflow has no request-level test, so regressions in route mounting, `requireWorkspaceKey`, the unknown-agent response, or the documented idempotent response could pass the current suite. A conformance test covering workspace-key success, agent-token rejection, unknown-agent 404, and repeated revocation would make the runbook contract executable.</comment>
<file context>
@@ -347,6 +347,44 @@ agentRoutes.patch(
+// Workspace key only: an agent must not be able to revoke itself or a peer.
+// Returns the revocation timestamp so the caller has a receipt to record.
+agentRoutes.post(
+ '/agents/:name/revoke',
+ requireWorkspaceKey,
+ rateLimit,
</file context>
| // second door. Distinct code from `agent_token_invalid` so an operator can | ||
| // tell "revoked" from "never existed" — a deleted row would report the | ||
| // latter, and the difference is the whole point of keeping the record. | ||
| if (agent.revokedAt) return unauthorized('Agent token revoked', 'agent_token_revoked'); |
There was a problem hiding this comment.
P2: The new agent_token_revoked error code isn't added to the TypeScript SDK's error union/normalization map, so its 401 fallback collapses it into a generic unauthorized error, erasing the distinction between a revoked token and other auth failures for SDK consumers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/engine/src/auth/index.ts, line 60:
<comment>The new agent_token_revoked error code isn't added to the TypeScript SDK's error union/normalization map, so its 401 fallback collapses it into a generic unauthorized error, erasing the distinction between a revoked token and other auth failures for SDK consumers.</comment>
<file context>
@@ -51,6 +51,13 @@ export class SqliteApiKeyAuthProvider implements AuthProvider {
+ // second door. Distinct code from `agent_token_invalid` so an operator can
+ // tell "revoked" from "never existed" — a deleted row would report the
+ // latter, and the difference is the whole point of keeping the record.
+ if (agent.revokedAt) return unauthorized('Agent token revoked', 'agent_token_revoked');
const [workspace] = await db.select().from(workspaces).where(eq(workspaces.id, agent.workspaceId));
if (!workspace) return unauthorized('Workspace not found');
</file context>
| -- ON DELETE NO ACTION in 0000: `messages.agent_id`, `channels.created_by`, | ||
| -- `files.uploaded_by` and `webhooks.created_by`. Any seat that has ever posted a | ||
| -- message therefore fails the delete outright with a FOREIGN KEY constraint | ||
| -- error. The delete only succeeds for a seat with no history — that is, exactly |
There was a problem hiding this comment.
P3: The migration rationale incorrectly implies that a successful agent delete means there is no history; cascade-only references can make the delete succeed while silently removing agent-owned records. Please explain that NO ACTION references block only some deletes and that cascade references can still discard history.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/engine/src/db/migrations/0034_agent_token_revocation.sql, line 8:
<comment>The migration rationale incorrectly implies that a successful agent delete means there is no history; cascade-only references can make the delete succeed while silently removing agent-owned records. Please explain that `NO ACTION` references block only some deletes and that cascade references can still discard history.</comment>
<file context>
@@ -0,0 +1,22 @@
+-- ON DELETE NO ACTION in 0000: `messages.agent_id`, `channels.created_by`,
+-- `files.uploaded_by` and `webhooks.created_by`. Any seat that has ever posted a
+-- message therefore fails the delete outright with a FOREIGN KEY constraint
+-- error. The delete only succeeds for a seat with no history — that is, exactly
+-- when there is nothing to contain and nothing worth keeping. Worse, the deletes
+-- that do land take history with them: `dm_participants.agent_id` cascades, which
</file context>
| // | ||
| // Workspace key only: an agent must not be able to revoke itself or a peer. | ||
| // Returns the revocation timestamp so the caller has a receipt to record. | ||
| agentRoutes.post( |
There was a problem hiding this comment.
P3: This adds a new public endpoint POST /v1/agents/:name/revoke without updating the API reference. The repo's docs convention (AGENTS.md "Docs Hygiene") requires README and openapi.yaml to be updated together when API behavior changes; openapi.yaml documents the sibling /agents/{name}/rotate-token but has no /revoke entry. Please add the new endpoint to openapi.yaml (and note it in README) so the documented surface stays in sync with the implemented routes before this ships.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/engine/src/routes/agent.ts, line 359:
<comment>This adds a new public endpoint `POST /v1/agents/:name/revoke` without updating the API reference. The repo's docs convention (AGENTS.md "Docs Hygiene") requires README and openapi.yaml to be updated together when API behavior changes; openapi.yaml documents the sibling `/agents/{name}/rotate-token` but has no `/revoke` entry. Please add the new endpoint to openapi.yaml (and note it in README) so the documented surface stays in sync with the implemented routes before this ships.</comment>
<file context>
@@ -347,6 +347,44 @@ agentRoutes.patch(
+//
+// Workspace key only: an agent must not be able to revoke itself or a peer.
+// Returns the revocation timestamp so the caller has a receipt to record.
+agentRoutes.post(
+ '/agents/:name/revoke',
+ requireWorkspaceKey,
</file context>
| @@ -0,0 +1,22 @@ | |||
| -- Agent token revocation. | |||
There was a problem hiding this comment.
P3: This change adds a new endpoint, migration, and telemetry event but doesn't add a corresponding changelog entry, so a release cut would omit the feature and its migration requirement.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/engine/src/db/migrations/0034_agent_token_revocation.sql, line 22:
<comment>This change adds a new endpoint, migration, and telemetry event but doesn't add a corresponding changelog entry, so a release cut would omit the feature and its migration requirement.</comment>
<file context>
@@ -0,0 +1,22 @@
+-- Additive and reversible: a NULL `revoked_at` is an active credential, so
+-- existing rows keep their current behaviour with no backfill.
+
+ALTER TABLE agents ADD COLUMN revoked_at INTEGER;
</file context>
Security review found a revoked agent token still authenticating on the A2A webhook. Patching that one route would have left the class of defect intact, so the choke point is now structural in both directions. Inbound: `routes/a2a.ts` no longer compares `agents.token_hash` itself. It resolves the bearer token through the configured AuthProvider and re-checks the binding afterwards, so every check the provider owns — revocation today, whatever is added later — applies there automatically instead of having to be remembered. The bypass was real: the accompanying HTTP test fails against the previous code, where a revoked credential reached the handler and was stopped only by payload validation. Outbound: revocation is now an optional `revokeAgentCredential` on the AuthProvider port. A deployment injecting a provider backed by an external identity store has no authority to invalidate a credential it did not issue, and previously would have written `revoked_at` into a column its authenticator never reads — a clean receipt over a live credential. The endpoint fails closed with `revocation_unsupported` when the capability is absent. No capability, no receipt. Also fixes the vanished-row false receipt from review: the post-update re-read returned a locally-generated timestamp when the row had been deleted concurrently or the update never landed, reporting a revocation that did not happen. It returns null now. `returning()` separates the winner from the loser of a concurrent revoke so the loser reports `already_revoked`. Adds the regression test that matters most for containment durability. `registerAgentViaNode` upserts on (workspace_id, name) and its `setWhere` fires for any seat whose status is not 'active', rewriting `token_hash` — so a containment marker held in that column is silently overwritten the next time any node claims the name. `revoked_at` is deliberately absent from that set clause: a re-registering node gets a fresh token that is still refused. Do not "fix" that test by clearing `revoked_at` on registration. Docs, SDK, and changelogs per AGENTS.md: openapi.yaml and README.md together, `agent_token_revoked` in the SDK error union and normalization map, pending Minor entries in the root, engine, types, and SDK changelogs.
There was a problem hiding this comment.
4 issues found across 15 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/engine/src/auth/index.ts">
<violation number="1" location="packages/engine/src/auth/index.ts:71">
P3: The comment misdescribes the A2A webhook and can send future maintainers toward a duplicate or conflicting revocation check; it should describe that the webhook now routes through this provider while warning only about future direct lookups.</violation>
</file>
<file name="CHANGELOG.md">
<violation number="1" location="CHANGELOG.md:23">
P3: This entry is one long bullet full of implementation backstory (why deletion fails, what rotate-token returns) rather than a short impact-first note. Per AGENTS.md CHANGELOG rules, each user-visible change should be "one short impact-first bullet" with backstory omitted.</violation>
</file>
<file name="packages/engine/src/routes/a2a.ts">
<violation number="1" location="packages/engine/src/routes/a2a.ts:329">
P2: A valid custom-provider A2A credential can now be rejected after authentication because the binding requires a local `AuthResult.agent` that the provider contract does not require. Require an agent identity for `require: 'agent'` in the provider contract, or add a provider-level binding operation that can validate this token against `relayAgentId`.</violation>
</file>
<file name="packages/engine/CHANGELOG.md">
<violation number="1" location="packages/engine/CHANGELOG.md:14">
P3: This changelog entry is far longer and more implementation-heavy than the project's documented changelog convention. `AGENTS.md` asks for "one short bullet per user-visible change" and says to "omit ... implementation backstory unless they explain shipped impact." The `Added` bullet here packs in internal mechanics (the `revoked_at` column, `SqliteApiKeyAuthProvider.authenticate`, "the schema enumerates every column per query", the ON DELETE NO ACTION foreign-key count, and DM-cascade internals). Consider trimming the paragraph to the user-facing contract: the new `POST /agents/{name}/revoke` endpoint, that it returns `agent_token_revoked` (401) distinct from `agent_token_invalid`, the migration `0034` requirement, and a one-line note on when to prefer it over `DELETE`/`rotate-token`. The migration syntax requirement and `AuthProvider.revokeAgentCredential` provider-contract detail can stay in the operator runbook (`docs/revoking-an-agent-credential.md`) rather than the changelog.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| if (!authResult.ok) { | ||
| return jsonError(c, authResult.code, authResult.message, authResult.status as ContentfulStatusCode); | ||
| } | ||
| if (authResult.agent?.id !== relayAgent.relayAgentId) { |
There was a problem hiding this comment.
P2: A valid custom-provider A2A credential can now be rejected after authentication because the binding requires a local AuthResult.agent that the provider contract does not require. Require an agent identity for require: 'agent' in the provider contract, or add a provider-level binding operation that can validate this token against relayAgentId.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/engine/src/routes/a2a.ts, line 329:
<comment>A valid custom-provider A2A credential can now be rejected after authentication because the binding requires a local `AuthResult.agent` that the provider contract does not require. Require an agent identity for `require: 'agent'` in the provider contract, or add a provider-level binding operation that can validate this token against `relayAgentId`.</comment>
<file context>
@@ -312,8 +312,21 @@ a2aRoutes.post('/a2a/webhook/:workspace_id/:agent_name', async (c) => {
+ if (!authResult.ok) {
+ return jsonError(c, authResult.code, authResult.message, authResult.status as ContentfulStatusCode);
+ }
+ if (authResult.agent?.id !== relayAgent.relayAgentId) {
return jsonError(c, 'unauthorized', 'Missing or invalid bearer token', 401);
}
</file context>
| // Refuse a revoked credential. This is the main lookup resolving an | ||
| // `at_live_` token to an identity, and the realtime WS path rejects agent | ||
| // tokens outright (see engine/wsAuth.ts) — but it is NOT the only one. The | ||
| // A2A webhook (`routes/a2a.ts`) compares the stored hash directly without |
There was a problem hiding this comment.
P3: The comment misdescribes the A2A webhook and can send future maintainers toward a duplicate or conflicting revocation check; it should describe that the webhook now routes through this provider while warning only about future direct lookups.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/engine/src/auth/index.ts, line 71:
<comment>The comment misdescribes the A2A webhook and can send future maintainers toward a duplicate or conflicting revocation check; it should describe that the webhook now routes through this provider while warning only about future direct lookups.</comment>
<file context>
@@ -51,12 +65,15 @@ export class SqliteApiKeyAuthProvider implements AuthProvider {
+ // Refuse a revoked credential. This is the main lookup resolving an
+ // `at_live_` token to an identity, and the realtime WS path rejects agent
+ // tokens outright (see engine/wsAuth.ts) — but it is NOT the only one. The
+ // A2A webhook (`routes/a2a.ts`) compares the stored hash directly without
+ // going through this provider and carries its own check; any new path that
+ // matches on `agents.token_hash` must do the same, or revocation silently
</file context>
|
|
||
| ### Added | ||
|
|
||
| - `POST /agents/{name}/revoke` invalidates an agent's token while keeping the agent and its history on the record. Requests carrying a revoked credential are refused with `agent_token_revoked` (HTTP 401), which is distinct from `agent_token_invalid` so a deliberate revocation is not mistaken for an unknown token. Use it instead of `DELETE /agents/{name}` to contain a leaked credential — deletion fails for any agent that has posted a message and destroys audit history where it succeeds — and instead of `POST /agents/{name}/rotate-token`, which also invalidates but returns a live replacement token in its response. Requires migration `0034`, which must be applied before or with this release; see `docs/revoking-an-agent-credential.md` for the operator runbook. |
There was a problem hiding this comment.
P3: This entry is one long bullet full of implementation backstory (why deletion fails, what rotate-token returns) rather than a short impact-first note. Per AGENTS.md CHANGELOG rules, each user-visible change should be "one short impact-first bullet" with backstory omitted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At CHANGELOG.md, line 23:
<comment>This entry is one long bullet full of implementation backstory (why deletion fails, what rotate-token returns) rather than a short impact-first note. Per AGENTS.md CHANGELOG rules, each user-visible change should be "one short impact-first bullet" with backstory omitted.</comment>
<file context>
@@ -16,7 +16,11 @@ This project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+
+### Added
+
+- `POST /agents/{name}/revoke` invalidates an agent's token while keeping the agent and its history on the record. Requests carrying a revoked credential are refused with `agent_token_revoked` (HTTP 401), which is distinct from `agent_token_invalid` so a deliberate revocation is not mistaken for an unknown token. Use it instead of `DELETE /agents/{name}` to contain a leaked credential — deletion fails for any agent that has posted a message and destroys audit history where it succeeds — and instead of `POST /agents/{name}/rotate-token`, which also invalidates but returns a live replacement token in its response. Requires migration `0034`, which must be applied before or with this release; see `docs/revoking-an-agent-credential.md` for the operator runbook.
## [6.3.2] - 2026-08-02
</file context>
|
|
||
| ### Added | ||
|
|
||
| - `POST /agents/{name}/revoke` invalidates an agent credential while leaving the agent row, its messages, and every record referencing it in place. Enforcement is a new `revoked_at` column checked in the agent branch of `SqliteApiKeyAuthProvider.authenticate`; refused requests return `agent_token_revoked` (401), distinct from `agent_token_invalid`. Requires migration `0034`, which must be applied **before or with** this release — the schema enumerates every column per query, so the code cannot talk to an `agents` table without it. Prefer this to `DELETE /agents/{name}`, which fails for any agent that has posted a message (four foreign keys onto `agents.id` are ON DELETE NO ACTION) and cascades away DM history where it succeeds. |
There was a problem hiding this comment.
P3: This changelog entry is far longer and more implementation-heavy than the project's documented changelog convention. AGENTS.md asks for "one short bullet per user-visible change" and says to "omit ... implementation backstory unless they explain shipped impact." The Added bullet here packs in internal mechanics (the revoked_at column, SqliteApiKeyAuthProvider.authenticate, "the schema enumerates every column per query", the ON DELETE NO ACTION foreign-key count, and DM-cascade internals). Consider trimming the paragraph to the user-facing contract: the new POST /agents/{name}/revoke endpoint, that it returns agent_token_revoked (401) distinct from agent_token_invalid, the migration 0034 requirement, and a one-line note on when to prefer it over DELETE/rotate-token. The migration syntax requirement and AuthProvider.revokeAgentCredential provider-contract detail can stay in the operator runbook (docs/revoking-an-agent-credential.md) rather than the changelog.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/engine/CHANGELOG.md, line 14:
<comment>This changelog entry is far longer and more implementation-heavy than the project's documented changelog convention. `AGENTS.md` asks for "one short bullet per user-visible change" and says to "omit ... implementation backstory unless they explain shipped impact." The `Added` bullet here packs in internal mechanics (the `revoked_at` column, `SqliteApiKeyAuthProvider.authenticate`, "the schema enumerates every column per query", the ON DELETE NO ACTION foreign-key count, and DM-cascade internals). Consider trimming the paragraph to the user-facing contract: the new `POST /agents/{name}/revoke` endpoint, that it returns `agent_token_revoked` (401) distinct from `agent_token_invalid`, the migration `0034` requirement, and a one-line note on when to prefer it over `DELETE`/`rotate-token`. The migration syntax requirement and `AuthProvider.revokeAgentCredential` provider-contract detail can stay in the operator runbook (`docs/revoking-an-agent-credential.md`) rather than the changelog.</comment>
<file context>
@@ -7,7 +7,17 @@ See the [root changelog](../../CHANGELOG.md) for cross-package release highlight
+
+### Added
+
+- `POST /agents/{name}/revoke` invalidates an agent credential while leaving the agent row, its messages, and every record referencing it in place. Enforcement is a new `revoked_at` column checked in the agent branch of `SqliteApiKeyAuthProvider.authenticate`; refused requests return `agent_token_revoked` (401), distinct from `agent_token_invalid`. Requires migration `0034`, which must be applied **before or with** this release — the schema enumerates every column per query, so the code cannot talk to an `agents` table without it. Prefer this to `DELETE /agents/{name}`, which fails for any agent that has posted a message (four foreign keys onto `agents.id` are ON DELETE NO ACTION) and cascades away DM history where it succeeds.
+- `AuthProvider.revokeAgentCredential` (optional). Providers backed by an external identity store leave it undefined and the endpoint refuses with `revocation_unsupported` (501) rather than recording a revocation their authenticator never consults.
+
</file context>
| - `POST /agents/{name}/revoke` invalidates an agent credential while leaving the agent row, its messages, and every record referencing it in place. Enforcement is a new `revoked_at` column checked in the agent branch of `SqliteApiKeyAuthProvider.authenticate`; refused requests return `agent_token_revoked` (401), distinct from `agent_token_invalid`. Requires migration `0034`, which must be applied **before or with** this release — the schema enumerates every column per query, so the code cannot talk to an `agents` table without it. Prefer this to `DELETE /agents/{name}`, which fails for any agent that has posted a message (four foreign keys onto `agents.id` are ON DELETE NO ACTION) and cascades away DM history where it succeeds. | |
| - `POST /agents/{name}/revoke` invalidates an agent credential while leaving the agent row and its history in place. Refused requests return `agent_token_revoked` (401), distinct from `agent_token_invalid`. Requires migration `0034`. Prefer it to `DELETE /agents/{name}` to contain a leaked credential; see `docs/revoking-an-agent-credential.md` for the operator runbook. |
CI failed on `@relaycast/types#test`: the sync guard requires every path in
openapi.yaml to be either referenced by an SDK or listed as intentionally
non-SDK, and `/v1/agents/{param}/revoke` was neither.
Classified as non-SDK, with the reasoning recorded next to it rather than left
implicit. `rotate-token` is in CORE_SDK_PATHS, so the parity argument for
covering revoke in the SDKs is real — but that means a method across four SDKs,
which is a product decision and not part of an engine change. Revoking is also
break-glass: an agent runtime has no reason to call it, and the operator path is
the runbook.
Caught only in CI because the previous push validated the packages that changed
and not the ones that assert across them.
|
@coderabbitai review Full re-review requested on
Please look hardest at whether any other path authenticates without consulting the provider, and at the deploy-ordering hazard (migration 0034 must precede the code, or every |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== Current revision and changed files ==='
git rev-parse HEAD
git diff --name-status "$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)" HEAD 2>/dev/null || true
printf '%s\n' '=== Authentication, webhook, and migration structure ==='
ast-grep outline packages/engine/src --items all --type function,class,interface --match 'Auth|auth|Agent|agent|Webhook|webhook|Migrat|migrat' 2>/dev/null || true
printf '%s\n' '=== AuthProvider declarations, implementations, and calls ==='
rg -n -P -C 4 '\b(AuthProvider|revokeAgentCredential|authenticateAgent|agent_token_revoked|revocation_unsupported)\b' packages --glob '*.{ts,tsx}' || true
printf '%s\n' '=== Direct agent token/hash access and agent-table queries ==='
rg -n -P -C 4 '\b(tokenHash|token_hash)\b|(?:from|update|insert into|delete from)\s*\(?\s*agents\b|\bagents\.(tokenHash|revokedAt)\b' packages --glob '*.{ts,sql}' || true
printf '%s\n' '=== Routes and request authentication middleware ==='
ast-grep outline packages/engine/src/routes --items all --type function --match 'agent|Agent|webhook|Webhook|auth|Auth' 2>/dev/null || true
rg -n -P -C 5 '\b(router|app)\.(get|post|put|patch|delete)\s*\(|\b(authenticate|authorize|verify|tokenHash|token_hash)\b' packages/engine/src --glob '*.{ts,tsx}' || true
printf '%s\n' '=== Migrations, migration runner, and schema references ==='
fd -t f . packages/engine/src/db | sort
rg -n -P -C 4 '\b(migrate|migrationsFolder|0034_agent_token_revocation|revoked_at|revokedAt)\b' packages --glob '*.{ts,sql,json}' || true
printf '%s\n' '=== Changed source and tests ==='
for f in \
packages/engine/src/auth/index.ts \
packages/engine/src/engine/agent.ts \
packages/engine/src/routes/agent.ts \
packages/engine/src/db/schema.ts \
packages/engine/src/db/migrations/0034_agent_token_revocation.sql \
packages/engine/src/engine/__tests__/agentRevocation.test.ts \
docs/revoking-an-agent-credential.md
do
if [ -f "$f" ]; then
echo "--- $f"
cat -n "$f"
fi
doneLength of output: 117274 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== Candidate files ==='
fd -t f . packages | rg '/(auth|middleware|routes|adapters|engine)/|migrat|deploy|worker|wrangler|sst' | sort
printf '%s\n' '=== Auth provider and middleware ==='
for f in packages/engine/src/auth/index.ts packages/engine/src/middleware/auth.ts; do
if [ -f "$f" ]; then
echo "--- $f"
cat -n "$f"
fi
done
printf '%s\n' '=== Exact direct credential comparisons ==='
rg -n -P -C 8 \
'(?:eq|ne|and|or)\s*\(\s*(?:agents|a2aAgents)\.(?:tokenHash|id|name)|\.(?:tokenHash|token_hash)\s*(?:===|!==|==|!=)|\bhashToken\s*\(|\bverifyToken\s*\(|\btoken\s*[:=]' \
packages --glob '*.{ts,tsx}' || true
printf '%s\n' '=== Provider authentication call sites and all request token readers ==='
rg -n -P -C 6 \
'\.(?:authenticate|revokeAgentCredential)\s*\(|\bAuthProvider\b|Authorization|Bearer|at_live_|requireAgentToken|requireAuth' \
packages --glob '*.{ts,tsx}' || true
printf '%s\n' '=== A2A inbound routes and webhook handlers ==='
rg -n -i -P -C 8 \
'\ba2a\b|agent.?card|webhook|inbound|relayAgentId|relay_agent_id' \
packages/engine/src --glob '*.{ts,tsx}' || true
printf '%s\n' '=== Upgrade, WebSocket, and non-Hono request authentication ==='
rg -n -i -P -C 8 \
'\bwebsocket\b|\bupgrade\b|\bfetch\s*\(|serve\s*\(|onRequest|authorization|bearer|tokenHash' \
packages/engine/src --glob '*.{ts,tsx}' || true
printf '%s\n' '=== Migration runner and deployment configuration ==='
rg -n -P -C 8 \
'\b(runMigrations|migrate|migrationsFolder|wrangler|d1|database_id|RelaycastDatabase|migration)\b' \
. \
-g '!node_modules/**' -g '!dist/**' -g '!build/**' || true
printf '%s\n' '=== Migration journal/order ==='
fd -t f . packages/engine/src/db/migrations | sort | while read -r f; do
echo "--- $f"
sed -n '1,120p' "$f"
doneLength of output: 144530 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== Auth and A2A candidate files ==='
fd -t f -E node_modules -E dist -E build . \
| rg -i '(^|/)(auth|a2a|webhook|cloudflare|worker|app|server|routes).*\.(ts|tsx|yml|yaml|json|toml)$|(^|/)(wrangler\.(toml|json|jsonc)|sst\.config\.ts|package\.json)$' \
| sort
printf '%s\n' '=== Files that contain both A2A or inbound webhooks and agent credential terms ==='
rg -l -i -P '(a2a|inbound.{0,20}webhook|webhook.{0,20}inbound)' packages/engine/src --glob '*.{ts,tsx}' \
| while read -r f; do
if rg -q -i -P '(auth|token|bearer|agent|credential|hash)' "$f"; then
echo "$f"
fi
done
printf '%s\n' '=== Exact authentication provider ==='
cat -n packages/engine/src/auth/index.ts
printf '%s\n' '=== Exact HTTP authentication middleware ==='
cat -n packages/engine/src/middleware/auth.ts
printf '%s\n' '=== A2A and inbound-webhook implementations ==='
fd -t f -E node_modules -E dist -E build . packages/engine/src \
| rg -i '(a2a|inbound.*webhook|webhook.*inbound)' \
| sort \
| while read -r f; do
echo "--- $f"
cat -n "$f"
done
printf '%s\n' '=== Revocation HTTP and A2A receipt tests ==='
fd -t f packages/engine/src -E node_modules \
| rg -i '(agentRevocationReceipt|a2a|webhook)' \
| while read -r f; do
echo "--- $f"
cat -n "$f"
done
printf '%s\n' '=== Deployment workflow and D1 configuration ==='
fd -t f -E node_modules -E dist -E build . .github \
| rg -i '(^|/)(workflows/.*\.(yml|yaml)|wrangler\.(toml|json|jsonc)|sst\.config\.ts|package\.json)$' \
| sort \
| while read -r f; do
echo "--- $f"
cat -n "$f"
doneLength of output: 116458 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '=== WebSocket authentication and runtime entry points ==='
fd . packages/engine/src -t f \
| rg -i '(wsAuth|websocket|upgrade|entrypoint|engine\.ts$|index\.ts$)' \
| sort \
| while read -r f; do
echo "--- $f"
cat -n "$f"
done
printf '%s\n' '=== All direct agents.tokenHash access outside tests ==='
rg -n -P -C 6 '\bagents\.tokenHash\b|\btoken_hash\b' \
packages/engine/src \
--glob '*.{ts,tsx}' \
-g '!**/__tests__/**' || true
printf '%s\n' '=== Authentication invocation sites outside tests ==='
rg -n -P -C 4 '\bauthenticate\s*\(\s*\{' \
packages/engine/src \
--glob '*.{ts,tsx}' \
-g '!**/__tests__/**' || true
printf '%s\n' '=== Revocation HTTP and A2A receipt tests ==='
fd . packages/engine/src -t f \
| rg -i '(agentRevocationReceipt|a2a.*test|test.*a2a)' \
| sort \
| while read -r f; do
echo "--- $f"
cat -n "$f"
done
printf '%s\n' '=== CI, deploy, and D1 configuration ==='
{
fd . .github -t f 2>/dev/null || true
fd -t f -d 2 . 2>/dev/null | rg '(^|/)(wrangler\.(toml|json|jsonc)|sst\.config\.(ts|js)|package\.json)$' || true
} | sort -u | while read -r f; do
echo "--- $f"
cat -n "$f"
doneLength of output: 156442
I found no remaining agent credential path that bypasses
The A2A HTTP receipt test covers the previous bypass. It verifies that a revoked relay token returns I found one non-functional issue:
Migration ordering remains an operational requirement.
I found no blocking implementation issue in the reviewed changes. You are interacting with an AI system. |
Veto triple-scan —
|
| Check | Score | Result |
|---|---|---|
| Code Review | 78 | approved_with_warnings |
| Security | 82 | approved_with_warnings |
| Secrets | 100 | approved |
Overall verdict warn, exit code 0. The one "blocking issue" is an artifact: Veto's secrets check sets clean: false whenever the findings array is non-empty, and both secrets findings are info with "none required" — the summary literally reads no credential material present. I left them in rather than returning an empty array to force a green, since suppressing accurate notes to game a gate is the wrong trade on this PR of all PRs.
MCP sampling was unavailable, so Veto delegated the three specialist analyses back to this session. Flagging that plainly: these are self-authored findings on my own change, and should be weighted accordingly against the independent CodeRabbit and Codex passes.
New finding — verified, and NOT fixed here
A node token can still post as a revoked agent. routes/message.ts:111 resolves the from agent by (workspace_id, name) with no revoked_at predicate, so a revoked identity can still emit messages via any valid node token.
The agent's own credential is contained; the identity is not. An operator will reasonably read "revoked seat" as "this identity can no longer speak", and today that is not true.
Fix is small — an isNull(agents.revokedAt) predicate on that lookup, or reject with agent_token_revoked. I deliberately have not made it here: this PR is mid security review, and quietly widening it while reviewers are reading is how a review loses its meaning. Reviewer's call whether it lands in this PR or a follow-up — but it should be an explicit decision, not an omission.
Other residual items from the scans, all unfixed and deliberate:
- Migration ordering is enforced by documentation only. 0034 must precede the code or every
agentsquery breaks. Worth a pipeline guard. - No structural guard against a future direct
token_hashmatch. The A2A bypass is closed and current matches are limited to the provider (verified by exhaustive grep), but a lint rule would make the next one fail CI instead of shipping. revokeAgentTokenreturnsnullfor both "no such agent" and "could not confirm", so the route reports 404 for an unconfirmed revocation. In a containment runbook that could read as a wrong seat name.
CI: green on ea82dc6 (previous failure was @relaycast/types#test, an unclassified OpenAPI route in the SDK sync guard). No merge, no deploy.
1 participant
Summary
Adds a minimal, explicit way to invalidate an agent credential without deleting anything: a
revoked_atcolumn, a check at the single agent-auth lookup, an idempotent engine operation, one endpoint, and an operator runbook.Do not merge or deploy. Review only — deploy authorization has not been given. See the deploy-ordering note below, which is a hard constraint if this ever does ship.
Why this exists — the two paths that looked like they worked
Containing a leaked
at_live_token had no usable path. Both routes operators were pointed at fail, in ways that return success.remove_agentnever touches the credential. It dispatches a release to the agent's node, which stops a process, and returnsstatus: dispatched. Authentication never consultsstatus, so a released, offline, roster-absent agent authenticates exactly as well as a running one.DELETE /v1/agents/:namefails server-side on any seat with history. Four foreign keys ontoagents(id)areON DELETE NO ACTIONin migration 0000:messagesagent_idchannelscreated_byfilesuploaded_bywebhookscreated_byA seat that has posted a single message aborts with
FOREIGN KEY constraint failed. Reproduced from the real migrations withforeign_keys=ON, and pinned as a test here: on one seat with one message,deleteAgentthrows whilerevokeAgentTokensucceeds and the message survives.Note what that means. The seats deletion can remove are the ones that never posted — it succeeds only where there is nothing to preserve and fails exactly where there is. Where it does succeed it takes history with it:
dm_participants.agent_idcascades, which is how ordinary two-party DMs collapsed into one-row rosters (see the note inscripts/audit-dm-reservations.mjs). Deletion is the wrong axis for containment.Does a supported path already exist? Partly — and it is worth being precise
Yes:
POST /v1/agents/:name/rotate-tokengenuinely invalidates. It overwritestoken_hash, so the leaked credential stops authenticating immediately. It is not a broken endpoint and this PR does not replace it.It is unusable for containment for one structural reason: it returns the replacement token in its response body, and
register_agentreturns a live token in its reply too (relay#1389). Both put a working credential straight back into a transcript — trading a known-leaked token for a freshly-leaked one.So the gap is narrow and real: there is no way to invalidate without issuing a replacement. That is what this adds. Rotation stays the right tool when a seat must keep working and you control where the new token lands.
The change
0034_agent_token_revocation.sql—ALTER TABLE agents ADD COLUMN revoked_at INTEGER. Additive;NULLmeans active, so no backfill.auth/index.ts— one check in the agent branch ofauthenticate(). That is the only lookup resolving anat_live_token to an identity; the realtime WS path rejects agent tokens outright (engine/wsAuth.ts), so there is no second door. Refusal reportsagent_token_revoked, deliberately distinct fromagent_token_invalid— a deleted row reports the latter and is indistinguishable from a token that was never issued.revokeAgentToken()— idempotent, and preserves the original timestamp rather than sliding it forward, so re-running the runbook cannot rewrite when containment took effect.POST /v1/agents/:name/revoke— workspace key only; an agent must not revoke itself or a peer.docs/revoking-an-agent-credential.md— operator runbook and receipts path.Deliberately not folded into
status, which presence rewrites to'active'on every touch.Receipts
The tests assert the only thing that counts: the credential is presented and authentication is refused. They do not assert absence from a roster, a stopped process, or a successful-looking response — each of those has already been mistaken for containment on this incident, and none of them is evidence.
Current durable receipts across the seven affected seats are 0/7. Nothing here changes that; producing receipts needs a deploy, which is out of scope for this PR.
Deploy ordering — hard constraint
Migration 0034 must be applied before or with the code, never after. The drizzle schema enumerates every column on each query, so a build carrying
revoked_atcannot talk to anagentstable that lacks it. Verified: an ordinary insert against an unmigrated schema fails withtable agents has no column named revoked_at. Code-first takes agent registration and authentication down, not just revoke.Confirm which database you are migrating — production is the D1 instance the worker binds, resolved through the SST resource
RelaycastDatabase, never by the name matching the repo, and with--remote.Verification
No credential value appears in this branch: the only token-shaped strings are non-hex synthetic fixtures that have never authenticated anything.
Refs relay#1389. Related: #1379, #1409, #1370, #1059.