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. |
📝 WalkthroughWalkthroughThis change adds persistent agent-token revocation. It adds database state, engine and authentication logic, a workspace-key-protected endpoint, A2A integration, telemetry, tests, API contracts, and operational documentation. ChangesAgent token revocation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant AgentRoute
participant AgentEngine
participant AgentDatabase
participant AgentAuth
Operator->>AgentRoute: POST /v1/agents/:name/revoke with workspace key
AgentRoute->>AgentEngine: revokeAgentCredential
AgentEngine->>AgentDatabase: Store agents.revoked_at
AgentDatabase-->>AgentEngine: Return persisted timestamp
AgentEngine-->>AgentRoute: Return revocation receipt
Operator->>AgentAuth: Authenticate with revoked token
AgentAuth->>AgentDatabase: Read agent revokedAt
AgentDatabase-->>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 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea82dc6f1d
ℹ️ 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".
| // today, anything added later, and whatever a custom provider enforces — is | ||
| // silently skipped on this route. Binding is then re-checked explicitly: | ||
| // the token must resolve to *this* proxy's relay agent. | ||
| const authResult = await c.get('engine').auth.authenticate({ token, require: 'agent', db }); |
There was a problem hiding this comment.
In deployments whose custom AuthProvider validates agent tokens only in an external identity store—explicitly permitted by the provider contract—registerA2aAgent still mints the webhook's relay_token directly in the engine database, so this call asks the provider to authenticate a credential it never issued and every webhook request is rejected. Keep engine-owned proxy-token validation in the engine, including the revokedAt check, or add a provider contract for provisioning/authenticating these tokens; otherwise custom-provider A2A integrations stop working.
AGENTS.md reference: AGENTS.md:L10-L11
Useful? React with 👍 / 👎.
| Never add `-v`, `--trace`, or `--trace-ascii` to a command carrying the token — | ||
| they print the `Authorization` header. If a token does appear in a transcript, | ||
| flag it for rotation of the *workspace* key and record it against relay#1389. |
There was a problem hiding this comment.
If the token exposed in a transcript is the at_live_ agent token handled by this section, rotating the workspace key does not invalidate it, so following this incident-response instruction can leave the exposed credential usable. Direct operators to revoke or rotate the exposed agent credential, and mention workspace-key rotation only when the workspace key itself was disclosed.
Useful? React with 👍 / 👎.
|
|
||
| ```sh | ||
| curl -s -X POST \ | ||
| -H "Authorization: Bearer $WS_KEY" \ |
There was a problem hiding this comment.
When an operator runs this exact command, the shell expands the privileged workspace key into the -H argument, exposing it through process arguments to same-host observers and process auditing—the unsafe channel the preceding section explicitly avoids for the agent token. curl --help all describes -H as passing a custom header and -K as reading configuration from a file; feed this authorization header through --config - as well.
Useful? React with 👍 / 👎.
| '404': | ||
| description: No such agent, or the revocation could not be confirmed |
There was a problem hiding this comment.
When the configured provider leaves the new optional revokeAgentCredential method undefined, the route deliberately returns HTTP 501 with revocation_unsupported, but this operation documents only 200 and 404. The optional provider contract is fresh evidence that the earlier endpoint documentation remains incomplete: generated clients and operators cannot distinguish an unsupported deployment from an absent agent, so add the 501 response and error code.
AGENTS.md reference: AGENTS.md:L34-L35
Useful? React with 👍 / 👎.
|
|
||
| ### 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.
This entry combines user impact, authentication internals, migration mechanics, deletion foreign-key behavior, and operational recommendations into a single long bullet, while the root entry similarly duplicates the runbook. The repository requires one short, impact-first bullet per visible change and omission of implementation backstory; retain the shipped API and migration impact here and leave the detailed rationale in the runbook.
AGENTS.md reference: AGENTS.md:L43-L46
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/engine/src/engine/__tests__/agentRevocation.test.ts (1)
70-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider stating that
raceDbnever runs the real guarded UPDATE.The proxy replaces the whole
update().set().where().returning()chain with a stub. The tests therefore simulate the race outcome, but they do not exercise theisNull(agents.revokedAt)guard in SQL. Add one test that revokes twice through the real database path to cover the guard, or note the limitation in the docblock.🤖 Prompt for 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. In `@packages/engine/src/engine/__tests__/agentRevocation.test.ts` around lines 70 - 88, Update the `raceDb` docblock to explicitly state that its stubbed `update().set().where().returning()` chain never executes the real guarded UPDATE or evaluates the `agents.revokedAt` condition. Preserve the existing test behavior and scope without adding unrelated changes.packages/engine/src/routes/a2a.ts (1)
327-327: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConstrain the forwarded status instead of casting it.
authResult.statusis a plainnumber. A customAuthProvidercan return a status thatc.jsoncannot serialize, for example 204 or 304. The cast toContentfulStatusCodehides that at compile time and produces a runtime failure on this route. Clamp the value to the authentication range.♻️ Suggested guard
- return jsonError(c, authResult.code, authResult.message, authResult.status as ContentfulStatusCode); + const status: ContentfulStatusCode = + authResult.status >= 400 && authResult.status <= 599 + ? (authResult.status as ContentfulStatusCode) + : 401; + return jsonError(c, authResult.code, authResult.message, status);🤖 Prompt for 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. In `@packages/engine/src/routes/a2a.ts` at line 327, Update the authentication error response in the route handling authResult to constrain authResult.status to the supported authentication status range before passing it to jsonError. Remove the ContentfulStatusCode cast and ensure custom provider statuses such as 204 or 304 cannot reach c.json.packages/engine/CHANGELOG.md (1)
14-14: 🗄️ Data Integrity & Integration | 🔵 TrivialVerify migration 0034 is enforced by deployment automation.
The entry states that migration
0034must run before or with the application release. Confirm that the deployment pipeline applies and validates0034before serving code that queriesagents.revoked_at; otherwise a partial rollout can fail authentication and revocation requests.🤖 Prompt for 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. In `@packages/engine/CHANGELOG.md` at line 14, Verify the deployment automation applies migration 0034 before or alongside the release and validates its successful completion before serving code that queries agents.revoked_at. Update the relevant deployment migration and rollout checks to block or fail the release when 0034 is missing or unsuccessful.CHANGELOG.md (1)
23-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep both release-note bullets concise.
Both bullets combine public behavior with detailed implementation rationale. Apply one short, impact-first structure at each changelog level.
CHANGELOG.md#L23-L23: Keep the revocation result and migration requirement. Remove foreign-key mechanics and move detailed alternatives to the runbook.packages/engine/CHANGELOG.md#L14-L14: Keep the API, migration, and authentication impact. Remove schema-query and foreign-key implementation details.As per coding guidelines: “changelog entries should be one short, impact-first bullet per user-visible change and omit internal or test-only details unless they explain shipped impact.”
🤖 Prompt for 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. In `@CHANGELOG.md` at line 23, Shorten the revocation changelog bullet in CHANGELOG.md: retain the user-visible revocation result and migration 0034 requirement, but remove foreign-key mechanics and detailed comparisons with deletion and token rotation, which belong in the runbook. Apply the same concise, impact-first edit to packages/engine/CHANGELOG.md, retaining the API, migration, and authentication impact while removing schema-query and foreign-key implementation details.Source: Coding guidelines
🤖 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 `@openapi.yaml`:
- Around line 2997-3014: Add a 501 response to the revoke operation’s documented
responses in openapi.yaml, describing provider-unsupported revocation and
referencing the existing components/schemas/ErrorResponse schema, while
preserving the existing 200 and 404 responses.
In `@packages/engine/src/auth/index.ts`:
- Around line 68-77: Remove the stale A2A direct-hash-comparison description
from the revocation comment near the auth provider’s revokedAt check, while
retaining the rule that new agent-token paths must use this provider; update
packages/engine/src/auth/index.ts lines 68-77 accordingly. In
packages/engine/src/__tests__/agentRevocationReceipt.test.ts lines 13-19,
rewrite the docblock and test title to state that the webhook authenticates
through the provider and preserve the test as a regression guard against bypass
reintroduction.
In `@packages/engine/src/ports/auth.ts`:
- Around line 41-60: Change revokeAgentCredential and the related
revokeAgentToken flow to return a discriminated result that separately
identifies an unknown agent from an unconfirmed revocation, rather than using
null for both. Update SqliteApiKeyAuthProvider.revokeAgentCredential and the
revoke route mapping to preserve 404 only for unknown agents and report
unconfirmed revocations as containment failures, while retaining idempotency and
the existing revokedAt behavior.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Line 23: Shorten the revocation changelog bullet in CHANGELOG.md: retain the
user-visible revocation result and migration 0034 requirement, but remove
foreign-key mechanics and detailed comparisons with deletion and token rotation,
which belong in the runbook. Apply the same concise, impact-first edit to
packages/engine/CHANGELOG.md, retaining the API, migration, and authentication
impact while removing schema-query and foreign-key implementation details.
In `@packages/engine/CHANGELOG.md`:
- Line 14: Verify the deployment automation applies migration 0034 before or
alongside the release and validates its successful completion before serving
code that queries agents.revoked_at. Update the relevant deployment migration
and rollout checks to block or fail the release when 0034 is missing or
unsuccessful.
In `@packages/engine/src/engine/__tests__/agentRevocation.test.ts`:
- Around line 70-88: Update the `raceDb` docblock to explicitly state that its
stubbed `update().set().where().returning()` chain never executes the real
guarded UPDATE or evaluates the `agents.revokedAt` condition. Preserve the
existing test behavior and scope without adding unrelated changes.
In `@packages/engine/src/routes/a2a.ts`:
- Line 327: Update the authentication error response in the route handling
authResult to constrain authResult.status to the supported authentication status
range before passing it to jsonError. Remove the ContentfulStatusCode cast and
ensure custom provider statuses such as 204 or 304 cannot reach c.json.
🪄 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: fefc9012-3cd7-4800-b225-8e7ae239dc46
📒 Files selected for processing (16)
CHANGELOG.mdREADME.mddocs/revoking-an-agent-credential.mdopenapi.yamlpackages/engine/CHANGELOG.mdpackages/engine/src/__tests__/agentRevocationReceipt.test.tspackages/engine/src/auth/index.tspackages/engine/src/engine/__tests__/agentRevocation.test.tspackages/engine/src/engine/agent.tspackages/engine/src/ports/auth.tspackages/engine/src/routes/a2a.tspackages/engine/src/routes/agent.tspackages/sdk-typescript/CHANGELOG.mdpackages/sdk-typescript/src/errors.tspackages/types/CHANGELOG.mdpackages/types/src/__tests__/sdk-openapi-sync.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/engine/src/routes/agent.ts
- packages/engine/src/engine/agent.ts
- docs/revoking-an-agent-credential.md
| responses: | ||
| '200': | ||
| description: >- | ||
| Token revoked. Returns `revoked_at` and `already_revoked`. A success | ||
| response means the revocation is persisted; it is not by itself | ||
| proof of containment — confirm by presenting the credential and | ||
| observing the 401. | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/SuccessResponse' | ||
| '404': | ||
| description: No such agent, or the revocation could not be confirmed | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/ErrorResponse' | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Document the provider-unsupported response.
The revoke handler in packages/engine/src/routes/agent.ts returns revocation_unsupported with HTTP 501 when the configured provider cannot revoke credentials. This operation documents only 200 and 404. Add a 501 response using #/components/schemas/ErrorResponse.
🤖 Prompt for 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.
In `@openapi.yaml` around lines 2997 - 3014, Add a 501 response to the revoke
operation’s documented responses in openapi.yaml, describing
provider-unsupported revocation and referencing the existing
components/schemas/ErrorResponse schema, while preserving the existing 200 and
404 responses.
| // 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 | ||
| // stops covering it. Distinct code from `agent_token_invalid` so an | ||
| // operator can tell "revoked" from "never existed" — a deleted row reports | ||
| // the latter, and that difference is the point of keeping the record. | ||
| if (agent.revokedAt) return unauthorized('Agent token revoked', 'agent_token_revoked'); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale prose describes the removed A2A bypass. packages/engine/src/routes/a2a.ts line 325 now authenticates the webhook through engine.auth.authenticate({ token, require: 'agent', db }), and line 123-126 removed agents.tokenHash from the projection. Two places still describe the old direct hash comparison, which misleads future maintainers about where revocation is enforced.
packages/engine/src/auth/index.ts#L68-L77: remove the statement that the A2A webhook compares the stored hash directly and carries its own check. Keep the rule that any new path resolving an agent token must go through this provider.packages/engine/src/__tests__/agentRevocationReceipt.test.ts#L13-L19: rewrite the docblock and the test title at line 82 to state that the webhook must authenticate through the provider, and keep the test as a regression guard against a reintroduced bypass.
📍 Affects 2 files
packages/engine/src/auth/index.ts#L68-L77(this comment)packages/engine/src/__tests__/agentRevocationReceipt.test.ts#L13-L19
🤖 Prompt for 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.
In `@packages/engine/src/auth/index.ts` around lines 68 - 77, Remove the stale A2A
direct-hash-comparison description from the revocation comment near the auth
provider’s revokedAt check, while retaining the rule that new agent-token paths
must use this provider; update packages/engine/src/auth/index.ts lines 68-77
accordingly. In packages/engine/src/__tests__/agentRevocationReceipt.test.ts
lines 13-19, rewrite the docblock and test title to state that the webhook
authenticates through the provider and preserve the test as a regression guard
against bypass reintroduction.
| /** | ||
| * Invalidate an agent's credential while leaving the agent and its history in | ||
| * place. Returns `null` if there is no such agent, or if the invalidation | ||
| * could not be confirmed against stored state. | ||
| * | ||
| * Optional, and deliberately so: a provider backed by an external identity | ||
| * store may have no authority to invalidate a credential it did not issue. | ||
| * Such a provider must leave this undefined rather than implementing a no-op — | ||
| * `POST /agents/{name}/revoke` refuses outright when it is absent, because a | ||
| * successful-looking response from a provider that cannot enforce revocation | ||
| * is a false containment receipt, which is worse than no revoke at all. | ||
| * | ||
| * Implementations must be idempotent and must not move the original | ||
| * invalidation timestamp on a repeat call. | ||
| */ | ||
| revokeAgentCredential?(args: { | ||
| workspaceId: string; | ||
| agentName: string; | ||
| db: EngineDb; | ||
| }): Promise<{ revokedAt: Date; alreadyRevoked: boolean } | null>; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Separate "no such agent" from "revocation not confirmed" in the contract.
The contract returns null for two different outcomes. The route cannot distinguish them, so an unconfirmed revocation is reported to the operator as a missing agent (the receipt test at packages/engine/src/__tests__/agentRevocationReceipt.test.ts line 141 asserts 404 for the unknown-agent case). A containment failure then looks like a name typo, which is the failure mode this endpoint exists to prevent. Return a discriminated result instead.
♻️ Suggested contract change
revokeAgentCredential?(args: {
workspaceId: string;
agentName: string;
db: EngineDb;
- }): Promise<{ revokedAt: Date; alreadyRevoked: boolean } | null>;
+ }): Promise<
+ | { outcome: 'revoked'; revokedAt: Date; alreadyRevoked: boolean }
+ | { outcome: 'agent_not_found' }
+ | { outcome: 'not_confirmed' }
+ >;This also requires updating SqliteApiKeyAuthProvider.revokeAgentCredential, revokeAgentToken, and the revoke route mapping.
🤖 Prompt for 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.
In `@packages/engine/src/ports/auth.ts` around lines 41 - 60, Change
revokeAgentCredential and the related revokeAgentToken flow to return a
discriminated result that separately identifies an unknown agent from an
unconfirmed revocation, rather than using null for both. Update
SqliteApiKeyAuthProvider.revokeAgentCredential and the revoke route mapping to
preserve 404 only for unknown agents and report unconfirmed revocations as
containment failures, while retaining idempotency and the existing revokedAt
behavior.
There was a problem hiding this comment.
2 issues found across 19 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="docs/revoking-an-agent-credential.md">
<violation number="1" location="docs/revoking-an-agent-credential.md:66">
P3: The curl snippets build the request from TOKEN_FILE after only `TOKEN_FILE=$(mktemp)` and a comment — the file is never populated in the shown steps, so following them verbatim sends an empty bearer token and yields `agent_token_invalid` instead of the `agent_token_revoked` receipt the doc's own 'What counts as done' section demands. Add an explicit population step (e.g. a placeholder `printf ... >"$TOKEN_FILE"` or a note that the value must be written there first) so the receipt expectation is reproducible.</violation>
</file>
<file name="packages/engine/src/routes/agent.ts">
<violation number="1" location="packages/engine/src/routes/agent.ts:380">
P2: Deployments using an authentication provider without revocation support can return `501 revocation_unsupported`, but the published OpenAPI contract does not describe it; generated clients and contract consumers will treat a real response as unspecified. Adding the 501 response and provider-capability explanation to the endpoint schema would keep the new route contract complete.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| c, | ||
| 'revocation_unsupported', | ||
| 'The configured authentication provider cannot revoke agent credentials', | ||
| 501, |
There was a problem hiding this comment.
P2: Deployments using an authentication provider without revocation support can return 501 revocation_unsupported, but the published OpenAPI contract does not describe it; generated clients and contract consumers will treat a real response as unspecified. Adding the 501 response and provider-capability explanation to the endpoint schema would keep the new route contract complete.
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 380:
<comment>Deployments using an authentication provider without revocation support can return `501 revocation_unsupported`, but the published OpenAPI contract does not describe it; generated clients and contract consumers will treat a real response as unspecified. Adding the 501 response and provider-capability explanation to the endpoint schema would keep the new route contract complete.</comment>
<file context>
@@ -347,6 +347,60 @@ agentRoutes.patch(
+ c,
+ 'revocation_unsupported',
+ 'The configured authentication provider cannot revoke agent credentials',
+ 501,
+ );
+ }
</file context>
| # Populate this from your secret store — do not paste it into the shell. | ||
| TOKEN_FILE=$(mktemp) | ||
|
|
||
| printf 'header = "Authorization: Bearer %s"\n' "$(cat "$TOKEN_FILE")" \ |
There was a problem hiding this comment.
P3: The curl snippets build the request from TOKEN_FILE after only TOKEN_FILE=$(mktemp) and a comment — the file is never populated in the shown steps, so following them verbatim sends an empty bearer token and yields agent_token_invalid instead of the agent_token_revoked receipt the doc's own 'What counts as done' section demands. Add an explicit population step (e.g. a placeholder printf ... >"$TOKEN_FILE" or a note that the value must be written there first) so the receipt expectation is reproducible.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/revoking-an-agent-credential.md, line 66:
<comment>The curl snippets build the request from TOKEN_FILE after only `TOKEN_FILE=$(mktemp)` and a comment — the file is never populated in the shown steps, so following them verbatim sends an empty bearer token and yields `agent_token_invalid` instead of the `agent_token_revoked` receipt the doc's own 'What counts as done' section demands. Add an explicit population step (e.g. a placeholder `printf ... >"$TOKEN_FILE"` or a note that the value must be written there first) so the receipt expectation is reproducible.</comment>
<file context>
@@ -0,0 +1,150 @@
+# Populate this from your secret store — do not paste it into the shell.
+TOKEN_FILE=$(mktemp)
+
+printf 'header = "Authorization: Bearer %s"\n' "$(cat "$TOKEN_FILE")" \
+ | curl --config - -s -o /dev/null -w '%{http_code}\n' \
+ https://<relaycast-host>/v1/agent
</file context>
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.