fix(auth): revoke one access token by prefix and stop rewriting last use

docs/openapi.yaml

@@ -4037,11 +4037,13 @@ paths:
           schema: { type: string }
       responses:
         "200":
-          description: Token revoked
+          description: Token revoked (alreadyRevoked is true when it was already revoked)
         "403":
           description: Insufficient scope
         "404":
-          description: Token not found or already revoked
+          description: Token not found
+        "409":
+          description: Prefix matches more than one live token; nothing revoked
 
   # ─── CLI Tools ─────────────────────────────────────────────────
 

public/openapi.yaml

@@ -2812,11 +2812,13 @@ paths:
           schema: { type: string }
       responses:
         "200":
-          description: Token revoked
+          description: Token revoked (alreadyRevoked is true when it was already revoked)
         "403":
           description: Insufficient scope
         "404":
-          description: Token not found or already revoked
+          description: Token not found
+        "409":
+          description: Prefix matches more than one live token; nothing revoked
 
   # ─── CLI Tools ─────────────────────────────────────────────────
 

src/app/api/cli/tokens/[id]/route.ts

@@ -4,20 +4,25 @@ import { revokeAccessToken } from "@/lib/db/accessTokens";
 
 /**
  * DELETE /api/cli/tokens/:id — revoke an access token (by id or display prefix).
- * Admin-only (same enforcement as the collection route). Idempotent: revoking
- * an unknown/already-revoked token returns 404.
+ * Admin-only (same enforcement as the collection route). Revoking an
+ * already-revoked token returns 200 with alreadyRevoked so a retry after a
+ * lost response succeeds. An unknown token is 404. A prefix that matches more
+ * than one live token revokes nothing and returns 409.
  */
-export async function DELETE(
-  request: Request,
-  { params }: { params: Promise<{ id: string }> }
-) {
+export async function DELETE(request: Request, { params }: { params: Promise<{ id: string }> }) {
   const authError = await requireManagementAuth(request);
   if (authError) return authError;
 
   const { id } = await params;
-  const revoked = revokeAccessToken(id);
-  if (!revoked) {
-    return NextResponse.json({ error: "Token not found or already revoked" }, { status: 404 });
+  const result = revokeAccessToken(id);
+  if (result.ambiguous) {
+    return NextResponse.json(
+      { error: "Token prefix matches more than one token" },
+      { status: 409 }
+    );
   }
-  return NextResponse.json({ success: true, id });
+  if (!result.revoked) {
+    return NextResponse.json({ error: "Token not found" }, { status: 404 });
+  }
+  return NextResponse.json({ success: true, id, alreadyRevoked: result.alreadyRevoked });
 }

src/lib/db/accessTokens.ts

@@ -16,6 +16,9 @@ const TOKEN_SECRET_PREFIX = "oma_live_";
 /** How many leading chars of the secret are kept for display (prefix). */
 const DISPLAY_PREFIX_LEN = TOKEN_SECRET_PREFIX.length + 6;
 
+/** How long a successful verify may reuse the stored last_used_at stamp. */
+const LAST_USED_WRITE_INTERVAL_MS = 60_000;
+
 export interface AccessTokenRecord {
   id: string;
   name: string;
@@ -34,6 +37,15 @@ export interface VerifiedAccessToken {
   scope: AccessScope;
 }
 
+/** Outcome of a revoke. A retry of an already-revoked token is still a success. */
+export interface RevokeAccessTokenResult {
+  /** True when the token is revoked after this call, including a prior revoke. */
+  revoked: boolean;
+  alreadyRevoked: boolean;
+  /** True when a prefix matched more than one live token and nothing was revoked. */
+  ambiguous: boolean;
+}
+
 interface AccessTokenRow {
   id: string;
   token_hash: string;
@@ -135,14 +147,20 @@ export function verifyAccessToken(secret: string | null | undefined): VerifiedAc
   if (row.revoked_at) return null;
   if (isExpired(row.expires_at)) return null;
 
-  // Best-effort usage stamp; never block validation on the write.
-  try {
-    db.prepare("UPDATE cli_access_tokens SET last_used_at = ? WHERE id = ?").run(
-      new Date().toISOString(),
-      row.id
-    );
-  } catch {
-    /* non-fatal */
+  // A management request verifies the same token at the policy and the route.
+  // Skip the write while the stored stamp is still inside the window.
+  const lastUsedMs = row.last_used_at ? Date.parse(row.last_used_at) : Number.NaN;
+  const stampIsFresh =
+    Number.isFinite(lastUsedMs) && Date.now() - lastUsedMs < LAST_USED_WRITE_INTERVAL_MS;
+  if (!stampIsFresh) {
+    try {
+      db.prepare("UPDATE cli_access_tokens SET last_used_at = ? WHERE id = ?").run(
+        new Date().toISOString(),
+        row.id
+      );
+    } catch {
+      /* non-fatal */
+    }
   }
 
   return { id: row.id, name: row.name, scope: normalizeScope(row.scope) };
@@ -161,23 +179,53 @@ export function listAccessTokens(): AccessTokenRecord[] {
 export function getAccessToken(id: string): AccessTokenRecord | null {
   const db = getDbInstance();
   const row = db.prepare("SELECT * FROM cli_access_tokens WHERE id = ?").get(id) as
-    | AccessTokenRow
-    | undefined;
+    AccessTokenRow | undefined;
   return row ? rowToRecord(row) : null;
 }
 
 /**
- * Revoke a token by id or by its display prefix. Idempotent: revoking an
- * already-revoked token is a no-op. Returns true when a row was newly revoked.
+ * Revoke a token by id, or by a display prefix that matches exactly one live
+ * token. A prefix shared by two or more live tokens revokes nothing.
+ * Revoking an already-revoked token succeeds and reports alreadyRevoked.
  */
-export function revokeAccessToken(idOrPrefix: string): boolean {
-  if (!idOrPrefix) return false;
+export function revokeAccessToken(idOrPrefix: string): RevokeAccessTokenResult {
+  const none = { revoked: false, alreadyRevoked: false, ambiguous: false };
+  if (!idOrPrefix) return none;
   const db = getDbInstance();
+
+  const matches = db
+    .prepare(
+      `SELECT id, revoked_at FROM cli_access_tokens
+         WHERE id = ? OR token_prefix = ?`
+    )
+    .all(idOrPrefix, idOrPrefix) as Array<{ id: string; revoked_at: string | null }>;
+  if (matches.length === 0) return none;
+
+  const exact = matches.find((row) => row.id === idOrPrefix);
+  // Revoked siblings do not make the one remaining live prefix ambiguous.
+  const live = matches.filter((row) => row.revoked_at == null);
+  let target = exact ?? null;
+  if (!target) {
+    if (live.length > 1) {
+      return { revoked: false, alreadyRevoked: false, ambiguous: true };
+    }
+    if (live.length === 1) target = live[0];
+    else if (matches.length === 1) target = matches[0];
+    else return none;
+  }
+  if (target.revoked_at) {
+    return { revoked: true, alreadyRevoked: true, ambiguous: false };
+  }
+
   const res = db
     .prepare(
       `UPDATE cli_access_tokens SET revoked_at = ?
-         WHERE (id = ? OR token_prefix = ?) AND revoked_at IS NULL`
+         WHERE id = ? AND revoked_at IS NULL`
     )
-    .run(new Date().toISOString(), idOrPrefix, idOrPrefix);
-  return res.changes > 0;
+    .run(new Date().toISOString(), target.id);
+  if (res.changes > 0) {
+    return { revoked: true, alreadyRevoked: false, ambiguous: false };
+  }
+  // Lost the race to another revoke of this same id.
+  return { revoked: true, alreadyRevoked: true, ambiguous: false };
 }

tests/unit/access-tokens-db.test.ts

@@ -70,21 +70,115 @@ test("verifyAccessToken stamps last_used_at", () => {
   assert.notEqual(at.getAccessToken(record.id)?.lastUsedAt, null);
 });
 
+test("verifyAccessToken writes last_used_at once per window", () => {
+  const { secret, record } = at.createAccessToken({ name: "throttle", scope: "read" });
+  const db = core.getDbInstance();
+  let writes = 0;
+  const originalPrepare = db.prepare.bind(db);
+  db.prepare = ((sql: string) => {
+    const statement = originalPrepare(sql);
+    if (/UPDATE\s+cli_access_tokens\s+SET\s+last_used_at/i.test(sql)) {
+      const originalRun = statement.run.bind(statement);
+      statement.run = ((...args: unknown[]) => {
+        writes += 1;
+        return originalRun(...args);
+      }) as typeof statement.run;
+    }
+    return statement;
+  }) as typeof db.prepare;
+
+  try {
+    assert.ok(at.verifyAccessToken(secret));
+    assert.ok(at.verifyAccessToken(secret));
+    assert.equal(writes, 1, "a repeat verify inside the window must not write again");
+    assert.notEqual(at.getAccessToken(record.id)?.lastUsedAt, null);
+
+    const stale = new Date(Date.now() - 61_000).toISOString();
+    originalPrepare("UPDATE cli_access_tokens SET last_used_at = ? WHERE id = ?").run(
+      stale,
+      record.id
+    );
+    assert.ok(at.verifyAccessToken(secret));
+    assert.equal(writes, 2, "a stamp older than the window must be refreshed");
+    assert.notEqual(at.getAccessToken(record.id)?.lastUsedAt, stale);
+  } finally {
+    db.prepare = originalPrepare;
+  }
+});
+
 test("revoked tokens fail verification", () => {
   const { secret, record } = at.createAccessToken({ name: "to-revoke", scope: "write" });
   assert.ok(at.verifyAccessToken(secret));
-  assert.equal(at.revokeAccessToken(record.id), true);
+  const first = at.revokeAccessToken(record.id);
+  assert.equal(first.revoked, true);
+  assert.equal(first.alreadyRevoked, false);
   assert.equal(at.verifyAccessToken(secret), null);
-  // idempotent: revoking again is a no-op
-  assert.equal(at.revokeAccessToken(record.id), false);
+  // A retry after a lost response must not look like a miss.
+  const second = at.revokeAccessToken(record.id);
+  assert.equal(second.revoked, true);
+  assert.equal(second.alreadyRevoked, true);
+  assert.notEqual(at.getAccessToken(record.id)?.revokedAt, null);
 });
 
 test("revokeAccessToken works by display prefix too", () => {
   const { secret, record } = at.createAccessToken({ name: "by-prefix", scope: "read" });
-  assert.equal(at.revokeAccessToken(record.tokenPrefix), true);
+  const result = at.revokeAccessToken(record.tokenPrefix);
+  assert.equal(result.revoked, true);
+  assert.equal(result.alreadyRevoked, false);
   assert.equal(at.verifyAccessToken(secret), null);
 });
 
+test("revoke by a shared prefix refuses instead of revoking every match", () => {
+  const first = at.createAccessToken({ name: "shared-a", scope: "read" });
+  const second = at.createAccessToken({ name: "shared-b", scope: "write" });
+  const db = core.getDbInstance();
+  db.prepare("UPDATE cli_access_tokens SET token_prefix = ? WHERE id = ?").run(
+    first.record.tokenPrefix,
+    second.record.id
+  );
+
+  const result = at.revokeAccessToken(first.record.tokenPrefix);
+  assert.equal(result.revoked, false);
+  assert.equal(result.ambiguous, true);
+  assert.equal(at.getAccessToken(first.record.id)?.revokedAt, null);
+  assert.equal(at.getAccessToken(second.record.id)?.revokedAt, null);
+  // The id is unique, so the same collision does not block an id revoke.
+  assert.equal(at.revokeAccessToken(first.record.id).revoked, true);
+  assert.equal(at.getAccessToken(second.record.id)?.revokedAt, null);
+});
+
+test("revoke by prefix ignores already-revoked siblings", () => {
+  const live = at.createAccessToken({ name: "live-sibling", scope: "read" });
+  const dead = at.createAccessToken({ name: "dead-sibling", scope: "write" });
+  const db = core.getDbInstance();
+  db.prepare("UPDATE cli_access_tokens SET token_prefix = ?, revoked_at = ? WHERE id = ?").run(
+    live.record.tokenPrefix,
+    new Date().toISOString(),
+    dead.record.id
+  );
+
+  const result = at.revokeAccessToken(live.record.tokenPrefix);
+  assert.equal(result.revoked, true);
+  assert.equal(result.ambiguous, false);
+  assert.notEqual(at.getAccessToken(live.record.id)?.revokedAt, null);
+});
+
+test("revoke by prefix of an already-revoked unique prefix reports alreadyRevoked", () => {
+  const { record } = at.createAccessToken({ name: "prefix-retry", scope: "read" });
+  assert.equal(at.revokeAccessToken(record.tokenPrefix).alreadyRevoked, false);
+  const retry = at.revokeAccessToken(record.tokenPrefix);
+  assert.equal(retry.revoked, true);
+  assert.equal(retry.alreadyRevoked, true);
+  assert.equal(retry.ambiguous, false);
+});
+
+test("revoke of an unknown id is not a success", () => {
+  const result = at.revokeAccessToken("tok_does_not_exist");
+  assert.equal(result.revoked, false);
+  assert.equal(result.alreadyRevoked, false);
+  assert.equal(result.ambiguous, false);
+});
+
 test("expired tokens fail verification", () => {
   const past = new Date(Date.now() - 60_000).toISOString();
   const { secret } = at.createAccessToken({ name: "expired", scope: "admin", expiresAt: past });

tests/unit/cli-token-delete-route.test.ts

@@ -0,0 +1,75 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+// DELETE /api/cli/tokens/:id. Auth is forced on so a retry of an already-revoked
+// token is the behavior under test, not an open management endpoint.
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-cli-token-delete-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+process.env.DISABLE_SQLITE_AUTO_BACKUP = "true";
+process.env.INITIAL_PASSWORD = "test-pass";
+
+const core = await import("../../src/lib/db/core.ts");
+const at = await import("../../src/lib/db/accessTokens.ts");
+const { DELETE } = await import("../../src/app/api/cli/tokens/[id]/route.ts");
+
+test.after(() => {
+  try {
+    core.resetDbInstance();
+  } catch {}
+  try {
+    fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
+  } catch {}
+  delete process.env.INITIAL_PASSWORD;
+});
+
+function deleteToken(id: string, bearer: string): Promise<Response> {
+  return DELETE(
+    new Request(`http://localhost:20128/api/cli/tokens/${id}`, {
+      method: "DELETE",
+      headers: { authorization: `Bearer ${bearer}` },
+    }),
+    { params: Promise.resolve({ id }) }
+  );
+}
+
+test("DELETE of an already-revoked token succeeds so a lost response can be retried", async () => {
+  const admin = at.createAccessToken({ name: "admin", scope: "admin" });
+  const target = at.createAccessToken({ name: "target", scope: "read" });
+
+  const first = await deleteToken(target.record.id, admin.secret);
+  assert.equal(first.status, 200);
+  const firstBody = await first.json();
+  assert.equal(firstBody.success, true);
+  assert.equal(firstBody.alreadyRevoked, false);
+
+  const retry = await deleteToken(target.record.id, admin.secret);
+  assert.equal(retry.status, 200);
+  const retryBody = await retry.json();
+  assert.equal(retryBody.success, true);
+  assert.equal(retryBody.alreadyRevoked, true);
+  assert.equal(retryBody.id, target.record.id);
+});
+
+test("DELETE of an unknown token is still 404", async () => {
+  const admin = at.createAccessToken({ name: "admin-miss", scope: "admin" });
+  const missing = await deleteToken("tok_missing", admin.secret);
+  assert.equal(missing.status, 404);
+});
+
+test("DELETE by a prefix shared by two live tokens refuses both", async () => {
+  const admin = at.createAccessToken({ name: "admin-amb", scope: "admin" });
+  const first = at.createAccessToken({ name: "amb-a", scope: "read" });
+  const second = at.createAccessToken({ name: "amb-b", scope: "write" });
+  core
+    .getDbInstance()
+    .prepare("UPDATE cli_access_tokens SET token_prefix = ? WHERE id = ?")
+    .run(first.record.tokenPrefix, second.record.id);
+
+  const response = await deleteToken(first.record.tokenPrefix, admin.secret);
+  assert.equal(response.status, 409);
+  assert.equal(at.getAccessToken(first.record.id)?.revokedAt, null);
+  assert.equal(at.getAccessToken(second.record.id)?.revokedAt, null);
+});