From c2ab8c4a21970f66390078f80913edd4de3b6ce7 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Thu, 6 Aug 2026 06:32:18 +0200 Subject: [PATCH 1/6] feat(auth): revoke an agent credential without deleting the record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/revoking-an-agent-credential.md | 128 ++++++++++++ packages/engine/src/auth/index.ts | 7 + .../0034_agent_token_revocation.sql | 22 +++ packages/engine/src/db/schema.ts | 5 + .../engine/__tests__/agentRevocation.test.ts | 187 ++++++++++++++++++ packages/engine/src/engine/agent.ts | 54 ++++- packages/engine/src/routes/agent.ts | 38 ++++ 7 files changed, 440 insertions(+), 1 deletion(-) create mode 100644 docs/revoking-an-agent-credential.md create mode 100644 packages/engine/src/db/migrations/0034_agent_token_revocation.sql create mode 100644 packages/engine/src/engine/__tests__/agentRevocation.test.ts diff --git a/docs/revoking-an-agent-credential.md b/docs/revoking-an-agent-credential.md new file mode 100644 index 00000000..84f53e00 --- /dev/null +++ b/docs/revoking-an-agent-credential.md @@ -0,0 +1,128 @@ +# Revoking an agent credential + +For containing a leaked `at_live_` agent token. This invalidates the credential +and keeps the record. + +## What counts as done + +**A negative-auth receipt: the credential is presented and authentication is +refused.** Nothing weaker is evidence. In particular these are *not* receipts, +and each has been mistaken for one: + +| Observation | Why it proves nothing | +|---|---| +| The agent process is gone | `remove_agent` dispatches a *release* to the node. It stops a process. It never touches the credential. | +| The agent is absent from the roster | The roster reflects records, not credentials. | +| `status` is `offline` | `status` is not consulted during authentication at all. | +| An API call returned `dispatched` / `200` | Return shape is not behaviour. Read the state back. | + +The only thing that settles it is a request carrying the token coming back +`401 agent_token_revoked`. + +## Do not use DELETE for this + +`DELETE /v1/agents/:name` is not a containment tool and cannot be made into one: + +- **It fails 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`. A seat that has posted a single + message fails with `FOREIGN KEY constraint failed`. +- **It destroys history on the seats where it does succeed.** + `dm_participants.agent_id` cascades, which is how ordinary two-party DMs + collapsed into one-row rosters (see `scripts/audit-dm-reservations.mjs`). +- **It erases the distinction you need.** A deleted token authenticates as + `agent_token_invalid` — identical to a token that was never issued. You lose + the ability to prove the credential was deliberately contained. + +Deletion succeeds only where there is no audit trail to protect and fails exactly +where there is one. + +## Do not rotate + +Do not re-register the seat to "replace" the credential. `register_agent` +returns the live token in its reply (relay#1389), so a replacement lands straight +back into a transcript — the leak you are containing. Revoke without replacement; +issue a new seat separately if the work still needs doing. + +## Handling the token safely + +The token must never reach a shell argument, an environment listing, or shell +history. Keep it in a file with tight permissions and feed it to `curl` on stdin +via `--config`, which is the one path where the value is neither in `argv` nor +echoed: + +```sh +umask 077 +# 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:///v1/agent + +shred -u "$TOKEN_FILE" 2>/dev/null || rm -P "$TOKEN_FILE" +``` + +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. + +## Procedure + +Per seat, with `$WS_KEY` a workspace key (`rk_live_`) — agents cannot revoke +themselves or each other. + +**1. Revoke.** + +```sh +curl -s -X POST \ + -H "Authorization: Bearer $WS_KEY" \ + https:///v1/agents//revoke +``` + +Returns `revoked_at` and `already_revoked`. It is idempotent: re-running reports +`already_revoked: true` and preserves the original timestamp, so the record of +when containment took effect cannot be rewritten by a repeat run. + +**2. Take the receipt.** Present the leaked credential using the `--config` +pattern above and record the response: + +```sh +printf 'header = "Authorization: Bearer %s"\n' "$(cat "$TOKEN_FILE")" \ + | curl --config - -s -w '\n%{http_code}\n' https:///v1/agent +``` + +Expected — and the only acceptable result: + +``` +{"ok":false,"error":{"code":"agent_token_revoked","message":"Agent token revoked"}} +401 +``` + +`GET /v1/agent` is the right probe: it is read-only and does nothing but resolve +a token to its identity, so a live credential is confirmed without acting as the +agent. + +`401 agent_token_revoked` is the receipt. Record the seat name, the timestamp, +and that code. Do not record the token. + +If you get `200`, the credential is live and the seat is **not** contained — +check you targeted the right workspace, and that the deployed build includes +migration 0034 and the enforcement in `SqliteApiKeyAuthProvider.authenticate`. +An unmigrated deployment accepts the revoke call and still authenticates the +token: the write lands in a column nothing reads. + +**3. Confirm history survived.** The agent row and its messages must still be +present. Revocation that took history with it has traded one problem for a worse +one. + +## Scope limits + +- **Node tokens are separate credentials.** This revokes the agent's own token. A + node token that posts on the agent's behalf is unaffected and needs its own + decision. +- **A seat already deleted cannot be revoked.** There is no row to mark, and its + token now reports `agent_token_invalid`. That is containment by accident, not a + revocation receipt, and the audit trail for that seat is already gone. +- **Revocation is one-way here.** There is deliberately no un-revoke endpoint; + restoring access means issuing a new seat. diff --git a/packages/engine/src/auth/index.ts b/packages/engine/src/auth/index.ts index 26a0ee96..70df29f1 100644 --- a/packages/engine/src/auth/index.ts +++ b/packages/engine/src/auth/index.ts @@ -51,6 +51,13 @@ export class SqliteApiKeyAuthProvider implements AuthProvider { if (parsedToken.kind === 'agent') { const [agent] = await db.select().from(agents).where(eq(agents.tokenHash, hash)); if (!agent) return unauthorized('Invalid agent token', 'agent_token_invalid'); + // A revoked credential is refused here and nowhere else: this is the only + // lookup that resolves an `at_live_` token to an identity. The realtime WS + // path rejects agent tokens outright (see engine/wsAuth.ts), so there is no + // 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'); return { ok: true, workspace, agent }; diff --git a/packages/engine/src/db/migrations/0034_agent_token_revocation.sql b/packages/engine/src/db/migrations/0034_agent_token_revocation.sql new file mode 100644 index 00000000..43110201 --- /dev/null +++ b/packages/engine/src/db/migrations/0034_agent_token_revocation.sql @@ -0,0 +1,22 @@ +-- Agent token revocation. +-- +-- WHY THIS IS NOT A DELETE. `DELETE FROM agents` cannot contain a leaked agent +-- credential, because four foreign keys onto `agents(id)` are declared +-- 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 +-- is how ordinary 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. The agent and +-- every message it sent stay on the record; the credential stops authenticating. +-- Enforcement lives in the agent branch of SqliteApiKeyAuthProvider.authenticate +-- — the single lookup that turns an `at_live_` token into an identity. +-- +-- 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; diff --git a/packages/engine/src/db/schema.ts b/packages/engine/src/db/schema.ts index 27cf3a9c..b70415d4 100644 --- a/packages/engine/src/db/schema.ts +++ b/packages/engine/src/db/schema.ts @@ -82,6 +82,11 @@ export const agents = sqliteTable( deliverySeq: integer('delivery_seq').notNull().default(0), createdAt: integer('created_at', { mode: 'timestamp' }).notNull().default(sql`(unixepoch())`), lastSeen: integer('last_seen', { mode: 'timestamp' }).notNull().default(sql`(unixepoch())`), + // When set, the agent's `at_live_` token no longer authenticates. The row and + // everything referencing it stay on the record — revocation contains the + // credential without touching history. NULL means active. Deliberately not + // folded into `status`, which presence rewrites to 'active' on every touch. + revokedAt: integer('revoked_at', { mode: 'timestamp' }), }, (table) => [ uniqueIndex('agents_workspace_name_unique').on(table.workspaceId, table.name), diff --git a/packages/engine/src/engine/__tests__/agentRevocation.test.ts b/packages/engine/src/engine/__tests__/agentRevocation.test.ts new file mode 100644 index 00000000..bc516b5a --- /dev/null +++ b/packages/engine/src/engine/__tests__/agentRevocation.test.ts @@ -0,0 +1,187 @@ +/** + * Agent credential revocation. + * + * The property under test is a NEGATIVE AUTH RECEIPT: the credential is + * presented and authentication is refused. Nothing weaker counts. An agent being + * offline, absent from a roster, or the subject of a successful-looking API call + * are all compatible with a token that still works, so none of them are asserted + * here — every test drives `authenticate()`, the single lookup that turns an + * `at_live_` token into an identity. + * + * The last block pins down why this primitive exists at all: `deleteAgent` + * cannot contain a credential on any seat that has posted a message, and the + * seats it can delete are the ones with no history worth keeping. + */ + +import { afterEach, describe, expect, it } from 'vitest'; +import { eq } from 'drizzle-orm'; + +import { getSqliteDb, runMigrations, type SqliteDbHandle } from '../../adapters/node/database.js'; +import { agents, channels, messages, workspaces } from '../../db/schema.js'; +import { SqliteApiKeyAuthProvider, hashToken } from '../../auth/index.js'; +import { deleteAgent, revokeAgentToken } from '../agent.js'; + +type Db = SqliteDbHandle['db']; + +const handles: SqliteDbHandle[] = []; + +afterEach(() => { + for (const handle of handles.splice(0)) { + try { + handle.sqlite.close(); + } catch { + /* already closed */ + } + } +}); + +const auth = new SqliteApiKeyAuthProvider(); + +let seq = 0; + +interface Fixture { + db: Db; + ws: string; + agentId: string; + /** Synthetic test token. Never a real credential. */ + token: string; +} + +async function seed(): Promise { + const handle = getSqliteDb(':memory:'); + runMigrations(handle); + handles.push(handle); + + const n = ++seq; + const ws = `ws_${n}`; + const agentId = `ag_${n}`; + const token = `at_live_synthetic_test_value_${n}`; + + handle.db.insert(workspaces).values({ id: ws, name: `w${n}`, apiKeyHash: `hash_${n}` }).run(); + handle.db + .insert(agents) + .values({ id: agentId, workspaceId: ws, name: 'seat', tokenHash: await hashToken(token) }) + .run(); + + return { db: handle.db, ws, agentId, token }; +} + +/** Give the seat the audit history that makes it undeletable. */ +function givePostHistory(db: Db, ws: string, agentId: string, n: number): void { + db.insert(channels).values({ id: `ch_${n}`, workspaceId: ws, name: 'general' }).run(); + db.insert(messages) + .values({ id: `m_${n}`, workspaceId: ws, channelId: `ch_${n}`, agentId, body: 'hello' }) + .run(); +} + +describe('agent token revocation — negative auth receipt', () => { + it('authenticates the token before revocation', async () => { + const { db, token } = await seed(); + + const result = await auth.authenticate({ token, require: 'agent', db }); + + expect(result.ok).toBe(true); + }); + + it('refuses the same token after revocation', async () => { + const { db, ws, token } = await seed(); + + await revokeAgentToken(db, ws, 'seat'); + const result = await auth.authenticate({ token, require: 'agent', db }); + + // This assertion IS the receipt: credential presented, authentication refused. + expect(result).toMatchObject({ + ok: false, + status: 401, + code: 'agent_token_revoked', + }); + }); + + it('reports revoked distinctly from never-existed', async () => { + const { db, ws, token } = await seed(); + await revokeAgentToken(db, ws, 'seat'); + + const revoked = await auth.authenticate({ token, require: 'agent', db }); + const unknown = await auth.authenticate({ + token: 'at_live_synthetic_never_issued', + require: 'agent', + db, + }); + + // A deleted row would have reported `agent_token_invalid` — indistinguishable + // from a token that was never issued. Keeping the record keeps the distinction. + expect(revoked).toMatchObject({ ok: false, code: 'agent_token_revoked' }); + expect(unknown).toMatchObject({ ok: false, code: 'agent_token_invalid' }); + }); + + it('does not revoke unrelated seats in the same workspace', async () => { + const { db, ws, token } = await seed(); + const peerToken = 'at_live_synthetic_peer_value'; + db.insert(agents) + .values({ id: 'ag_peer', workspaceId: ws, name: 'peer', tokenHash: await hashToken(peerToken) }) + .run(); + + await revokeAgentToken(db, ws, 'seat'); + + expect(await auth.authenticate({ token, require: 'agent', db })).toMatchObject({ ok: false }); + expect(await auth.authenticate({ token: peerToken, require: 'agent', db })).toMatchObject({ ok: true }); + }); +}); + +describe('agent token revocation — history survives', () => { + it('keeps the agent row and its messages', async () => { + const { db, ws, agentId } = await seed(); + givePostHistory(db, ws, agentId, seq); + + await revokeAgentToken(db, ws, 'seat'); + + const [row] = await db.select().from(agents).where(eq(agents.id, agentId)); + const posted = await db.select().from(messages).where(eq(messages.agentId, agentId)); + expect(row).toBeDefined(); + expect(row.revokedAt).toBeInstanceOf(Date); + expect(posted).toHaveLength(1); + }); + + it('succeeds on a seat with history, where deletion cannot', async () => { + const { db, ws, agentId, token } = await seed(); + givePostHistory(db, ws, agentId, seq); + + // The path the operator was originally told to use. + await expect(deleteAgent(db, ws, 'seat')).rejects.toThrow(); + + // The path that works, on the identical seat. + await expect(revokeAgentToken(db, ws, 'seat')).resolves.toMatchObject({ alreadyRevoked: false }); + expect(await auth.authenticate({ token, require: 'agent', db })).toMatchObject({ + ok: false, + code: 'agent_token_revoked', + }); + }); +}); + +describe('agent token revocation — operational properties', () => { + it('is idempotent and preserves the original timestamp', async () => { + const { db, ws } = await seed(); + + const first = await revokeAgentToken(db, ws, 'seat'); + const second = await revokeAgentToken(db, ws, 'seat'); + + // An operator re-running the runbook must not be able to rewrite when + // containment actually took effect. + expect(first).toMatchObject({ alreadyRevoked: false }); + expect(second).toMatchObject({ alreadyRevoked: true }); + expect(second!.revokedAt.getTime()).toBe(first!.revokedAt.getTime()); + }); + + it('returns null for an unknown agent rather than inventing a receipt', async () => { + const { db, ws } = await seed(); + + await expect(revokeAgentToken(db, ws, 'no-such-seat')).resolves.toBeNull(); + }); + + it('does not revoke across workspace boundaries', async () => { + const { db, token } = await seed(); + + await expect(revokeAgentToken(db, 'ws_other', 'seat')).resolves.toBeNull(); + expect(await auth.authenticate({ token, require: 'agent', db })).toMatchObject({ ok: true }); + }); +}); diff --git a/packages/engine/src/engine/agent.ts b/packages/engine/src/engine/agent.ts index a3c94c5e..a47ea261 100644 --- a/packages/engine/src/engine/agent.ts +++ b/packages/engine/src/engine/agent.ts @@ -1,4 +1,4 @@ -import { eq, and, lt, inArray } from 'drizzle-orm'; +import { eq, and, lt, inArray, isNull } from 'drizzle-orm'; import type { getDb } from '../db/index.js'; import { agents, channels, channelMembers, actions, deliveries, nodes } from '../db/schema.js'; import { randomHex, sha256Hex } from '../lib/crypto.js'; @@ -275,6 +275,58 @@ export async function updateAgent( }; } +/** + * Revoke an agent's credential without deleting anything. + * + * The token stops authenticating at the next request; the agent row, its + * messages, the channels it created and the files it uploaded all stay exactly + * where they are. This is the containment primitive `deleteAgent` cannot be: + * that one fails outright on any seat with message history (four FKs onto + * `agents(id)` are ON DELETE NO ACTION), and destroys history on the seats where + * it does succeed. + * + * Idempotent by design. Re-revoking preserves the original `revoked_at` rather + * than sliding it forward, so the timestamp remains an accurate record of when + * containment actually took effect — an operator re-running the runbook must not + * be able to rewrite that. `alreadyRevoked` lets the caller tell a fresh + * revocation from a repeat without a second query. + * + * Note the deliberate scope limit: this invalidates the agent's own credential. + * A node token that posts on this agent's behalf is a separate credential and + * needs its own decision. + */ +export async function revokeAgentToken( + db: Db, + workspaceId: string, + name: string, +): Promise<{ revokedAt: Date; alreadyRevoked: boolean } | null> { + const [agent] = await db + .select() + .from(agents) + .where(and(eq(agents.workspaceId, workspaceId), eq(agents.name, name))); + + if (!agent) return null; + + if (agent.revokedAt) { + return { revokedAt: agent.revokedAt, alreadyRevoked: true }; + } + + const revokedAt = new Date(); + await db + .update(agents) + .set({ revokedAt }) + .where(and(eq(agents.id, agent.id), isNull(agents.revokedAt))); + + // Re-read rather than trusting the write: under a concurrent revoke the + // guarded UPDATE is a no-op and the stored timestamp is the other caller's. + // The receipt has to report what the row actually holds. + const [settled] = await db.select().from(agents).where(eq(agents.id, agent.id)); + return { + revokedAt: settled?.revokedAt ?? revokedAt, + alreadyRevoked: false, + }; +} + export async function deleteAgent(db: Db, workspaceId: string, name: string) { const [agent] = await db .select() diff --git a/packages/engine/src/routes/agent.ts b/packages/engine/src/routes/agent.ts index 6af50a83..1158a0af 100644 --- a/packages/engine/src/routes/agent.ts +++ b/packages/engine/src/routes/agent.ts @@ -347,6 +347,44 @@ agentRoutes.patch( }, ); +// POST /v1/agents/:name/revoke - invalidate the agent's token, keep the record. +// +// Prefer this to DELETE for credential containment. DELETE cannot contain a +// leaked token on any seat that has posted a message (FK, ON DELETE NO ACTION) +// and destroys audit history on the seats where it does succeed. This endpoint +// always works and always keeps the record. +// +// 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, + async (c) => { + try { + 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); + if (!result) { + return agentNotFound(c, name); + } + emitServerEvent(c, workspace.id, 'relaycast_server_agent_token_revoked', { + agent_name: name, + already_revoked: result.alreadyRevoked, + }); + return jsonOk(c, { + name, + revoked: true, + revoked_at: result.revokedAt.toISOString(), + already_revoked: result.alreadyRevoked, + }); + } catch (err: unknown) { + return errorResponse(c, err); + } + }, +); + // DELETE /v1/agents/:name - delete agent agentRoutes.delete( '/agents/:name', From a6499b3ed9ab609a1aced59122af4e2ea50160f9 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Thu, 6 Aug 2026 06:36:08 +0200 Subject: [PATCH 2/6] fix(telemetry): declare the agent token revoked event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/types/src/telemetry.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/types/src/telemetry.ts b/packages/types/src/telemetry.ts index e2f91f7f..aa49d3f9 100644 --- a/packages/types/src/telemetry.ts +++ b/packages/types/src/telemetry.ts @@ -39,6 +39,7 @@ export const SERVER_TELEMETRY_EVENTS = [ 'relaycast_server_agent_updated', 'relaycast_server_agent_deleted', 'relaycast_server_agent_token_rotated', + 'relaycast_server_agent_token_revoked', 'relaycast_server_channel_created', 'relaycast_server_channel_updated', 'relaycast_server_channel_topic_updated', @@ -147,6 +148,7 @@ const REQUIRED_SERVER_EVENT_PROPS: Record Date: Thu, 6 Aug 2026 06:38:10 +0200 Subject: [PATCH 3/6] docs(runbook): correct the unmigrated-deployment failure mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/revoking-an-agent-credential.md | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/docs/revoking-an-agent-credential.md b/docs/revoking-an-agent-credential.md index 84f53e00..ead9d76f 100644 --- a/docs/revoking-an-agent-credential.md +++ b/docs/revoking-an-agent-credential.md @@ -107,10 +107,25 @@ agent. and that code. Do not record the token. If you get `200`, the credential is live and the seat is **not** contained — -check you targeted the right workspace, and that the deployed build includes -migration 0034 and the enforcement in `SqliteApiKeyAuthProvider.authenticate`. -An unmigrated deployment accepts the revoke call and still authenticates the -token: the write lands in a column nothing reads. +check you targeted the right workspace and that the deployed build includes the +enforcement in `SqliteApiKeyAuthProvider.authenticate`. If the endpoint 404s, the +code is not deployed and no revocation has occurred, whatever else you saw. + +## Deploy ordering (read before shipping this) + +**Migration 0034 must be applied before or with the code, never after.** The +drizzle schema enumerates every column on each query, so a build that knows about +`revoked_at` cannot talk to an `agents` table that lacks it — verified: an insert +against an unmigrated schema fails with `table agents has no column named +revoked_at`. That is agent registration and agent authentication down, not a +degraded revoke. The failure is at least loud rather than silent, but the +ordering is not optional. + +Confirm which database you are migrating. Production is the D1 instance the +worker binds — resolve it through the SST resource `RelaycastDatabase`, never by +the name that happens to match the repo, and pass `--remote`. Two live instances +carry this data under confusingly similar names and audits have been run against +the wrong one before. **3. Confirm history survived.** The agent row and its messages must still be present. Revocation that took history with it has traded one problem for a worse From 759ceeb053e4f0d0d67caf5ec5573b45eb21b21d Mon Sep 17 00:00:00 2001 From: Khaliq Date: Thu, 6 Aug 2026 06:39:09 +0200 Subject: [PATCH 4/6] docs(runbook): name the rotate endpoint an operator will reach for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- docs/revoking-an-agent-credential.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/docs/revoking-an-agent-credential.md b/docs/revoking-an-agent-credential.md index ead9d76f..a1b810f7 100644 --- a/docs/revoking-an-agent-credential.md +++ b/docs/revoking-an-agent-credential.md @@ -39,10 +39,17 @@ where there is one. ## Do not rotate -Do not re-register the seat to "replace" the credential. `register_agent` -returns the live token in its reply (relay#1389), so a replacement lands straight -back into a transcript — the leak you are containing. Revoke without replacement; -issue a new seat separately if the work still needs doing. +`POST /v1/agents/:name/rotate-token` will look like the answer. It does +invalidate the leaked credential — it overwrites `token_hash`, so the old token +stops authenticating immediately. Do not use it for containment anyway: it +returns the replacement token in its response body, and `register_agent` returns +a live token in its reply too (relay#1389). Both put a working credential +straight back into a transcript, which is the leak you are containing. You would +trade a known-leaked token for a freshly-leaked one and call it done. + +Rotation is the right tool when a seat must keep working and you control where +the new token lands. It is the wrong tool when the goal is containment. Revoke +without replacement; issue a new seat separately if the work still needs doing. ## Handling the token safely From 112b31ade9d32b85cb3d426ad19b0489006c51a1 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Thu, 6 Aug 2026 07:08:10 +0200 Subject: [PATCH 5/6] fix(auth): make revocation a provider contract, not a per-route patch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CHANGELOG.md | 6 +- README.md | 2 + docs/revoking-an-agent-credential.md | 2 +- openapi.yaml | 55 +++++- packages/engine/CHANGELOG.md | 12 +- .../__tests__/agentRevocationReceipt.test.ts | 161 ++++++++++++++++++ packages/engine/src/auth/index.ts | 29 +++- .../engine/__tests__/agentRevocation.test.ts | 111 +++++++++++- packages/engine/src/engine/agent.ts | 26 ++- packages/engine/src/ports/auth.ts | 21 +++ packages/engine/src/routes/a2a.ts | 21 ++- packages/engine/src/routes/agent.ts | 18 +- packages/sdk-typescript/CHANGELOG.md | 7 +- packages/sdk-typescript/src/errors.ts | 5 + packages/types/CHANGELOG.md | 7 +- 15 files changed, 456 insertions(+), 27 deletions(-) create mode 100644 packages/engine/src/__tests__/agentRevocationReceipt.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c81e106..0f73528b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,11 @@ This project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Packages without a separate changelog are covered by the cross-package notes below. -## [Unreleased] +## [Unreleased - Minor] + +### 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 diff --git a/README.md b/README.md index 7850cf30..2bec2e97 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,8 @@ Relaycast is the messaging backbone: API errors use `{ ok: false, error: { code, message } }`. Invalid or expired agent tokens return `agent_token_invalid` with HTTP 401; clients should recover by re-registering or rotating the agent identity, then retrying the failed operation. +A deliberately revoked agent token returns `agent_token_revoked` with HTTP 401 instead. This is not a transient failure and retrying will not clear it: the seat was contained on purpose via `POST /agents/{name}/revoke`, which invalidates the credential while keeping the agent and its history on the record. Re-registering under the same identity is not the recovery path — issue a new seat if the work still needs doing. + ## Telemetry Attribution Clients may declare who is driving a request so server-side product telemetry diff --git a/docs/revoking-an-agent-credential.md b/docs/revoking-an-agent-credential.md index a1b810f7..734bc6d3 100644 --- a/docs/revoking-an-agent-credential.md +++ b/docs/revoking-an-agent-credential.md @@ -101,7 +101,7 @@ printf 'header = "Authorization: Bearer %s"\n' "$(cat "$TOKEN_FILE")" \ Expected — and the only acceptable result: -``` +```text {"ok":false,"error":{"code":"agent_token_revoked","message":"Agent token revoked"}} 401 ``` diff --git a/openapi.yaml b/openapi.yaml index 072f15d2..4748b80b 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -1123,7 +1123,12 @@ components: properties: code: type: string - description: Machine-readable error code. Invalid agent tokens are reported as `agent_token_invalid` so clients can re-register or rotate the agent identity. + description: >- + Machine-readable error code. Invalid agent tokens are reported as + `agent_token_invalid` so clients can re-register or rotate the agent + identity. A deliberately revoked token is reported as + `agent_token_revoked` instead — a permanent refusal that retrying or + re-registering the same identity will not clear. message: type: string @@ -2959,6 +2964,54 @@ paths: schema: $ref: '#/components/schemas/SuccessResponse' + /agents/{name}/revoke: + post: + summary: Revoke agent token + description: >- + Invalidate an agent's token while keeping the agent and its history on + the record. The credential stops authenticating immediately and + subsequent requests carrying it are refused with `agent_token_revoked`. + + Prefer this to `DELETE /agents/{name}` for credential containment: + deletion fails outright for any agent that has posted a message (foreign + keys onto `agents.id` are declared ON DELETE NO ACTION) and destroys + audit history where it does succeed. + + Prefer this to `POST /agents/{name}/rotate-token` when the goal is + containment rather than continuity: rotation also invalidates the old + token, but returns a live replacement in its response body. + + Idempotent — repeating the call reports `already_revoked` and preserves + the original `revoked_at`. There is no un-revoke; issue a new agent + instead. + tags: + - Agents + security: + - workspaceKey: [] + parameters: + - name: name + in: path + required: true + schema: + type: string + 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' + /agents/{name}/rotate-token: post: summary: Rotate agent token diff --git a/packages/engine/CHANGELOG.md b/packages/engine/CHANGELOG.md index c1d82fd8..8c655007 100644 --- a/packages/engine/CHANGELOG.md +++ b/packages/engine/CHANGELOG.md @@ -7,7 +7,17 @@ See the [root changelog](../../CHANGELOG.md) for cross-package release highlight The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [Unreleased - Minor] + +### 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. + +### Changed + +- The A2A webhook resolves its bearer token through the configured `AuthProvider` instead of comparing `agents.token_hash` directly, so provider-level checks apply to that route. Previously a revoked A2A proxy credential still authenticated there. + ## [6.3.2] - 2026-08-02 diff --git a/packages/engine/src/__tests__/agentRevocationReceipt.test.ts b/packages/engine/src/__tests__/agentRevocationReceipt.test.ts new file mode 100644 index 00000000..eb49017d --- /dev/null +++ b/packages/engine/src/__tests__/agentRevocationReceipt.test.ts @@ -0,0 +1,161 @@ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import type { AddressInfo } from 'node:net'; +import { startServer, type RunningServer } from '../entrypoints/node.js'; + +/** + * End-to-end negative-auth receipts, taken against the real HTTP surface. + * + * The unit tests drive the auth provider directly. These drive the server, + * because the thing that actually has to be true is that a request carrying a + * revoked credential is refused — on **every** path that accepts that + * credential, not just the one the provider owns. + * + * The A2A webhook is the reason this file exists. It compares + * `agents.token_hash` itself rather than calling the auth provider, so the + * provider-level revocation check does not cover it. Before that was fixed, + * revoking an A2A proxy seat produced a clean `revoked_at`, a passing unit + * suite, and a credential that still worked here — a false receipt, which is + * strictly worse than no revocation at all. + */ +describe('agent credential revocation — negative-auth receipts over HTTP', () => { + let running: RunningServer; + let base: string; + + beforeAll(async () => { + running = startServer({ + dbPath: ':memory:', + port: 0, + migrate: true, + config: { environment: 'test' }, + }); + base = `http://127.0.0.1:${(running.server.address() as AddressInfo).port}`; + }); + + afterAll(() => running.stop()); + + async function api(path: string, init: RequestInit = {}) { + const res = await fetch(`${base}${path}`, init); + return { status: res.status, body: (await res.json().catch(() => ({}))) as any }; + } + + async function newWorkspace(name: string): Promise { + const ws = await api('/v1/workspaces', { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ name }), + }); + return ws.body.data.api_key as string; + } + + function revoke(workspaceKey: string, name: string) { + return api(`/v1/agents/${name}/revoke`, { + method: 'POST', + headers: { authorization: `Bearer ${workspaceKey}` }, + }); + } + + /** Resolve a token to an identity — read-only, so it probes auth and nothing else. */ + function whoami(token: string) { + return api('/v1/agent', { headers: { authorization: `Bearer ${token}` } }); + } + + it('refuses a revoked credential on the ordinary agent path', async () => { + const workspaceKey = await newWorkspace('receipt-plain'); + const created = await api('/v1/agents', { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${workspaceKey}` }, + body: JSON.stringify({ name: 'seat' }), + }); + const token = created.body.data.token as string; + + expect((await whoami(token)).status).toBe(200); + + const receipt = await revoke(workspaceKey, 'seat'); + expect(receipt.status).toBe(200); + expect(receipt.body.data).toMatchObject({ revoked: true, already_revoked: false }); + + const refused = await whoami(token); + expect(refused.status).toBe(401); + expect(refused.body.error.code).toBe('agent_token_revoked'); + }); + + it('refuses a revoked credential on the A2A webhook, which bypasses the auth provider', async () => { + const workspaceKey = await newWorkspace('receipt-a2a'); + + const registered = await api('/v1/a2a/register', { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${workspaceKey}` }, + body: JSON.stringify({ + agent_card: { + name: 'external-proxy', + url: 'https://example.invalid/a2a', + version: '1.0.0', + skills: [{ name: 'echo' }], + }, + }), + }); + expect(registered.status).toBe(201); + + const relayName = registered.body.data.relay_name as string; + const relayToken = registered.body.data.relay_token as string; + const webhookPath = new URL(registered.body.data.webhook_url as string).pathname; + + const callWebhook = () => + api(webhookPath, { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${relayToken}` }, + body: JSON.stringify({ jsonrpc: '2.0', id: '1', method: 'message/send', params: {} }), + }); + + // Before revocation the credential is accepted here. Whatever the handler + // then does with the payload, it is not an auth refusal. + const before = await callWebhook(); + expect(before.body?.error?.code).not.toBe('agent_token_revoked'); + + expect((await revoke(workspaceKey, relayName)).status).toBe(200); + + const after = await callWebhook(); + expect(after.status).toBe(401); + expect(after.body.error.code).toBe('agent_token_revoked'); + }); + + it('reports a repeat revoke as already revoked without moving the timestamp', async () => { + const workspaceKey = await newWorkspace('receipt-idempotent'); + await api('/v1/agents', { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${workspaceKey}` }, + body: JSON.stringify({ name: 'seat' }), + }); + + const first = await revoke(workspaceKey, 'seat'); + const second = await revoke(workspaceKey, 'seat'); + + expect(first.body.data.already_revoked).toBe(false); + expect(second.body.data.already_revoked).toBe(true); + expect(second.body.data.revoked_at).toBe(first.body.data.revoked_at); + }); + + it('404s on an unknown agent instead of issuing a receipt', async () => { + const workspaceKey = await newWorkspace('receipt-unknown'); + + expect((await revoke(workspaceKey, 'no-such-seat')).status).toBe(404); + }); + + it('refuses to let an agent revoke itself or a peer', async () => { + const workspaceKey = await newWorkspace('receipt-authz'); + const created = await api('/v1/agents', { + method: 'POST', + headers: { 'content-type': 'application/json', authorization: `Bearer ${workspaceKey}` }, + body: JSON.stringify({ name: 'seat' }), + }); + const token = created.body.data.token as string; + + const attempt = await api('/v1/agents/seat/revoke', { + method: 'POST', + headers: { authorization: `Bearer ${token}` }, + }); + + expect(attempt.status).toBe(401); + expect((await whoami(token)).status).toBe(200); + }); +}); diff --git a/packages/engine/src/auth/index.ts b/packages/engine/src/auth/index.ts index 70df29f1..31b27b22 100644 --- a/packages/engine/src/auth/index.ts +++ b/packages/engine/src/auth/index.ts @@ -2,6 +2,7 @@ import { eq } from 'drizzle-orm'; import { workspaces, agents, nodes } from '../db/schema.js'; import { sha256Hex } from '../lib/crypto.js'; import { getActiveObserverTokenByHash } from '../engine/observerToken.js'; +import { revokeAgentToken } from '../engine/agent.js'; import type { AuthProvider, AuthResult, AuthRequire } from '../ports/auth.js'; import type { EngineDb } from '../ports/database.js'; import { parseAuthToken, validateTokenRequirement } from './tokenKind.js'; @@ -28,6 +29,19 @@ export class SqliteApiKeyAuthProvider implements AuthProvider { return hashToken(token); } + /** + * This provider owns `agents.token_hash`, so it can enforce revocation itself + * — the check lives in `authenticate` below, which every path resolving an + * agent token goes through. + */ + revokeAgentCredential(args: { + workspaceId: string; + agentName: string; + db: EngineDb; + }): Promise<{ revokedAt: Date; alreadyRevoked: boolean } | null> { + return revokeAgentToken(args.db as Parameters[0], args.workspaceId, args.agentName); + } + async authenticate(args: { token: string; require: AuthRequire; db: EngineDb }): Promise { const { token, require, db } = args; const parsedToken = parseAuthToken(token); @@ -51,12 +65,15 @@ export class SqliteApiKeyAuthProvider implements AuthProvider { if (parsedToken.kind === 'agent') { const [agent] = await db.select().from(agents).where(eq(agents.tokenHash, hash)); if (!agent) return unauthorized('Invalid agent token', 'agent_token_invalid'); - // A revoked credential is refused here and nowhere else: this is the only - // lookup that resolves an `at_live_` token to an identity. The realtime WS - // path rejects agent tokens outright (see engine/wsAuth.ts), so there is no - // 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. + // 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'); const [workspace] = await db.select().from(workspaces).where(eq(workspaces.id, agent.workspaceId)); if (!workspace) return unauthorized('Workspace not found'); diff --git a/packages/engine/src/engine/__tests__/agentRevocation.test.ts b/packages/engine/src/engine/__tests__/agentRevocation.test.ts index bc516b5a..e0b3ea6b 100644 --- a/packages/engine/src/engine/__tests__/agentRevocation.test.ts +++ b/packages/engine/src/engine/__tests__/agentRevocation.test.ts @@ -17,9 +17,10 @@ import { afterEach, describe, expect, it } from 'vitest'; import { eq } from 'drizzle-orm'; import { getSqliteDb, runMigrations, type SqliteDbHandle } from '../../adapters/node/database.js'; -import { agents, channels, messages, workspaces } from '../../db/schema.js'; +import { agents, channels, messages, nodes, workspaces } from '../../db/schema.js'; import { SqliteApiKeyAuthProvider, hashToken } from '../../auth/index.js'; import { deleteAgent, revokeAgentToken } from '../agent.js'; +import { registerAgentViaNode } from '../node.js'; type Db = SqliteDbHandle['db']; @@ -66,6 +67,26 @@ async function seed(): Promise { return { db: handle.db, ws, agentId, token }; } +/** + * Wrap a db so the guarded UPDATE inside `revokeAgentToken` is replaced by + * `interfere` — the window between reading the active row and writing to it. + * Everything else passes through to the real database. + */ +function raceDb(db: Db, interfere: () => Promise): Db { + return new Proxy(db, { + get(target, prop, receiver) { + if (prop === 'update') { + return () => ({ + set: () => ({ + where: () => ({ returning: async () => { await interfere(); return []; } }), + }), + }); + } + return Reflect.get(target, prop, receiver); + }, + }) as Db; +} + /** Give the seat the audit history that makes it undeletable. */ function givePostHistory(db: Db, ws: string, agentId: string, n: number): void { db.insert(channels).values({ id: `ch_${n}`, workspaceId: ws, name: 'general' }).run(); @@ -158,6 +179,50 @@ describe('agent token revocation — history survives', () => { }); }); +describe('agent token revocation — survives re-registration', () => { + /** + * `registerAgentViaNode` upserts on `(workspace_id, name)` and its `setWhere` + * fires for any seat whose status is not 'active'. Its `set` clause rewrites + * `token_hash`, so a containment marker stored *in that column* — a sentinel + * hash, say — is silently overwritten the next time any node registers the + * name, and the seat comes back live. + * + * `revoked_at` is not in that `set` clause. This test is what holds that true: + * if someone adds it, containment becomes undoable by a heartbeat and this + * fails. Do not "fix" it by clearing `revoked_at` on registration. + */ + it('a node re-registering the name does not resurrect a revoked credential', async () => { + const { db, ws, agentId, token } = await seed(); + await revokeAgentToken(db, ws, 'seat'); + + // An *offline* seat is what makes the upsert fire: `setWhere` matches on + // `status != 'active'`, so any node can reclaim the name — not just the one + // the seat was bound to. Four of the seats this was built for are offline. + db.update(agents).set({ status: 'offline' }).where(eq(agents.id, agentId)).run(); + db.insert(nodes) + .values({ id: 'nd_1', workspaceId: ws, name: 'node-1', tokenHash: 'node-hash-1' }) + .run(); + + // Same workspace, same name — the upsert path, on an offline seat. + const reregistered = await registerAgentViaNode(db, ws, 'nd_1', 'default', { + name: 'seat', + } as Parameters[4]); + + // Registration hands back a fresh token, and `token_hash` really was rewritten. + expect(reregistered.token).toBeTruthy(); + + // But the seat stays contained: the marker survived, so the brand-new + // credential is refused exactly like the old one. + const [row] = await db.select().from(agents).where(eq(agents.workspaceId, ws)); + expect(row.revokedAt).toBeInstanceOf(Date); + expect(await auth.authenticate({ token: reregistered.token, require: 'agent', db })).toMatchObject({ + ok: false, + code: 'agent_token_revoked', + }); + expect(await auth.authenticate({ token, require: 'agent', db })).toMatchObject({ ok: false }); + }); +}); + describe('agent token revocation — operational properties', () => { it('is idempotent and preserves the original timestamp', async () => { const { db, ws } = await seed(); @@ -178,6 +243,50 @@ describe('agent token revocation — operational properties', () => { await expect(revokeAgentToken(db, ws, 'no-such-seat')).resolves.toBeNull(); }); + it('issues no receipt when the row is deleted mid-operation', async () => { + const { db, ws, agentId } = await seed(); + + // The row disappears between the initial read and the guarded update, so the + // update matches nothing and there is no persisted `revoked_at` to report. + // Returning a locally-generated timestamp here would be a receipt for a + // revocation that never happened. + const raced = raceDb(db, async () => { + await db.delete(agents).where(eq(agents.id, agentId)); + }); + + await expect(revokeAgentToken(raced, ws, 'seat')).resolves.toBeNull(); + }); + + it('issues no receipt when the update does not land', async () => { + const { db, ws, token } = await seed(); + + const swallowed = raceDb(db, async () => { + /* drop the write on the floor */ + }); + + await expect(revokeAgentToken(swallowed, ws, 'seat')).resolves.toBeNull(); + // And the credential must still work — no silent half-revocation. + expect(await auth.authenticate({ token, require: 'agent', db })).toMatchObject({ ok: true }); + }); + + it('marks the loser of a concurrent revoke as already revoked', async () => { + const { db, ws } = await seed(); + + // Another operator's revoke lands after this call has read the active row, + // so this call's guarded update matches nothing. Both used to claim to have + // performed the fresh revocation. + let other: Date | undefined; + const raced = raceDb(db, async () => { + other = new Date(Date.now() - 1000); + await db.update(agents).set({ revokedAt: other }).where(eq(agents.workspaceId, ws)); + }); + + const result = await revokeAgentToken(raced, ws, 'seat'); + + expect(result).toMatchObject({ alreadyRevoked: true }); + expect(result!.revokedAt.getTime()).toBe(Math.floor(other!.getTime() / 1000) * 1000); + }); + it('does not revoke across workspace boundaries', async () => { const { db, token } = await seed(); diff --git a/packages/engine/src/engine/agent.ts b/packages/engine/src/engine/agent.ts index a47ea261..343b9c17 100644 --- a/packages/engine/src/engine/agent.ts +++ b/packages/engine/src/engine/agent.ts @@ -311,19 +311,27 @@ export async function revokeAgentToken( return { revokedAt: agent.revokedAt, alreadyRevoked: true }; } - const revokedAt = new Date(); - await db + // `returning()` tells us whether *this* call was the one that set the column. + // Under a concurrent revoke the guarded UPDATE matches no row and comes back + // empty, which is what distinguishes the winner from the loser. + const applied = await db .update(agents) - .set({ revokedAt }) - .where(and(eq(agents.id, agent.id), isNull(agents.revokedAt))); + .set({ revokedAt: new Date() }) + .where(and(eq(agents.id, agent.id), isNull(agents.revokedAt))) + .returning(); - // Re-read rather than trusting the write: under a concurrent revoke the - // guarded UPDATE is a no-op and the stored timestamp is the other caller's. - // The receipt has to report what the row actually holds. + // Re-read rather than trusting the write. A receipt must report what the row + // actually holds, so if there is no persisted `revoked_at` — the row was + // deleted from under us, or the update never landed — report no revocation at + // all. Returning a locally-generated timestamp here would hand back a receipt + // for something that did not happen, which is the exact failure this endpoint + // exists to make impossible. const [settled] = await db.select().from(agents).where(eq(agents.id, agent.id)); + if (!settled?.revokedAt) return null; + return { - revokedAt: settled?.revokedAt ?? revokedAt, - alreadyRevoked: false, + revokedAt: settled.revokedAt, + alreadyRevoked: applied.length === 0, }; } diff --git a/packages/engine/src/ports/auth.ts b/packages/engine/src/ports/auth.ts index 5fff8b77..bb367303 100644 --- a/packages/engine/src/ports/auth.ts +++ b/packages/engine/src/ports/auth.ts @@ -37,4 +37,25 @@ export interface AuthProvider { * the token unchanged. */ hashToken(token: string): Promise; + + /** + * 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>; } diff --git a/packages/engine/src/routes/a2a.ts b/packages/engine/src/routes/a2a.ts index 2cf9a8fc..acac8ec2 100644 --- a/packages/engine/src/routes/a2a.ts +++ b/packages/engine/src/routes/a2a.ts @@ -4,7 +4,8 @@ import { and, eq } from 'drizzle-orm'; import { z } from 'zod'; import type { AppEnv } from '../env.js'; import { a2aAgents, agents, messages, workspaces } from '../db/schema.js'; -import { requireAuth, hashToken } from '../middleware/auth.js'; +import { requireAuth } from '../middleware/auth.js'; +import type { ContentfulStatusCode } from 'hono/utils/http-status'; import { asCodedError, errorResponse, type CodedError } from '../lib/httpError.js'; import { rateLimit } from '../middleware/rateLimit.js'; import * as a2aEngine from '../engine/a2a.js'; @@ -122,7 +123,6 @@ async function findWebhookAgentByName(db: AppEnv['Variables']['db'], relayName: workspaceId: a2aAgents.workspaceId, relayAgentId: a2aAgents.relayAgentId, relayName: agents.name, - tokenHash: agents.tokenHash, }) .from(a2aAgents) .innerJoin(agents, eq(a2aAgents.relayAgentId, agents.id)) @@ -312,8 +312,21 @@ a2aRoutes.post('/a2a/webhook/:workspace_id/:agent_name', async (c) => { const token = c.req.header('Authorization')?.startsWith('Bearer ') ? c.req.header('Authorization')!.slice(7) : null; - const tokenHash = token ? await hashToken(token) : null; - if (!tokenHash || tokenHash !== relayAgent.tokenHash) { + if (!token) { + return jsonError(c, 'unauthorized', 'Missing or invalid bearer token', 401); + } + + // Resolve through the auth provider rather than comparing `agents.token_hash` + // here. Comparing the hash directly authenticates the credential without + // consulting the provider, so every check the provider owns — revocation + // 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 }); + 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); } diff --git a/packages/engine/src/routes/agent.ts b/packages/engine/src/routes/agent.ts index 1158a0af..cd46b892 100644 --- a/packages/engine/src/routes/agent.ts +++ b/packages/engine/src/routes/agent.ts @@ -365,7 +365,23 @@ agentRoutes.post( 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); + + // Delegate to the configured auth provider rather than writing the engine + // row directly. A deployment with a provider backed by an external identity + // store would otherwise get a clean receipt from a column its authenticator + // never reads — containment on paper, live credential in fact. Fail closed + // instead: no capability, no receipt. + const revoke = c.get('engine').auth.revokeAgentCredential; + if (!revoke) { + return jsonError( + c, + 'revocation_unsupported', + 'The configured authentication provider cannot revoke agent credentials', + 501, + ); + } + + const result = await revoke.call(c.get('engine').auth, { workspaceId: workspace.id, agentName: name, db }); if (!result) { return agentNotFound(c, name); } diff --git a/packages/sdk-typescript/CHANGELOG.md b/packages/sdk-typescript/CHANGELOG.md index 5abf7a3f..8e7eddfa 100644 --- a/packages/sdk-typescript/CHANGELOG.md +++ b/packages/sdk-typescript/CHANGELOG.md @@ -7,7 +7,12 @@ See the [root changelog](../../CHANGELOG.md) for cross-package release highlight The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [Unreleased - Minor] + +### Added + +- `agent_token_revoked` is recognized as a distinct `RelayErrorCode` instead of collapsing to `unauthorized`. It means the credential was deliberately revoked: retrying or re-registering the same identity will not clear it. + ## [6.3.0] - 2026-07-28 diff --git a/packages/sdk-typescript/src/errors.ts b/packages/sdk-typescript/src/errors.ts index a73259c9..242ec813 100644 --- a/packages/sdk-typescript/src/errors.ts +++ b/packages/sdk-typescript/src/errors.ts @@ -5,6 +5,10 @@ export type RelayErrorCode = | 'backpressure' | 'unauthorized' | 'agent_token_invalid' + // Distinct from `agent_token_invalid`: the credential existed and was + // deliberately revoked. Retrying or re-registering under the same identity is + // not the recovery path — the seat was contained on purpose. + | 'agent_token_revoked' | 'workspace_mismatch' | 'transport_error'; @@ -26,6 +30,7 @@ const RAW_CODE_MAP: Record = { workspace_stream_backpressure: 'backpressure', unauthorized: 'unauthorized', agent_token_invalid: 'agent_token_invalid', + agent_token_revoked: 'agent_token_revoked', workspace_mismatch: 'workspace_mismatch', workspace_not_found: 'workspace_mismatch', }; diff --git a/packages/types/CHANGELOG.md b/packages/types/CHANGELOG.md index e4cf256e..6845a5a4 100644 --- a/packages/types/CHANGELOG.md +++ b/packages/types/CHANGELOG.md @@ -7,7 +7,12 @@ See the [root changelog](../../CHANGELOG.md) for cross-package release highlight The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Unreleased] +## [Unreleased - Minor] + +### Added + +- `relaycast_server_agent_token_revoked` server telemetry event, emitted when an agent credential is revoked. + ## [6.3.0] - 2026-07-28 From ea82dc6f1dc5809bc27502ab63e7780c999f0a60 Mon Sep 17 00:00:00 2001 From: Khaliq Date: Thu, 6 Aug 2026 07:20:16 +0200 Subject: [PATCH 6/6] test(types): classify the revoke route in the SDK/OpenAPI sync guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/types/src/__tests__/sdk-openapi-sync.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/types/src/__tests__/sdk-openapi-sync.test.ts b/packages/types/src/__tests__/sdk-openapi-sync.test.ts index 46e0e24c..14d1f7e2 100644 --- a/packages/types/src/__tests__/sdk-openapi-sync.test.ts +++ b/packages/types/src/__tests__/sdk-openapi-sync.test.ts @@ -44,6 +44,14 @@ const NON_SDK_OPENAPI_PATHS = new Set([ // node-providers work; the engine surface ships first. '/v1/nodes/{param}/actions/{param}/invoke', '/v1/nodes/{param}/providers/{param}', + // Operator containment, driven from a runbook with a workspace key rather + // than by an agent. Note this is a deliberate scoping decision and not an + // oversight: `rotate-token` sits in CORE_SDK_PATHS, so the parity argument for + // SDK coverage is real, but exposing revoke means a method across all four + // SDKs and that is a product decision rather than part of the engine change. + // Revoking is also break-glass — an agent runtime has no reason to reach for + // it, and the operator path is `docs/revoking-an-agent-credential.md`. + '/v1/agents/{param}/revoke', ]); const CORE_SDK_PATHS = new Set([