diff --git a/lib/db/migrations/010_token_selector.sql b/lib/db/migrations/010_token_selector.sql new file mode 100644 index 0000000..7654e5a --- /dev/null +++ b/lib/db/migrations/010_token_selector.sql @@ -0,0 +1,12 @@ +-- Selector+verifier tokens: make verifyToken O(1) instead of an O(n) bcrypt scan +-- over every non-revoked token (code-review-2026-06-01.md / security-sweep HIGH). +-- The selector is a non-secret public lookup key; the verifier stays bcrypt-hashed +-- in token_hash. Legacy rows keep selector NULL and verify via the fallback path. +ALTER TABLE agent_tokens ADD COLUMN IF NOT EXISTS selector text; + +-- One row per selector (partial: legacy NULLs are exempt). +CREATE UNIQUE INDEX IF NOT EXISTS idx_agent_tokens_selector + ON agent_tokens(selector) WHERE selector IS NOT NULL; + +-- The old hash index was useless — bcrypt hashes can't be looked up by value. +DROP INDEX IF EXISTS idx_agent_tokens_hash; diff --git a/lib/db/repos/agents.js b/lib/db/repos/agents.js index 28dd5c2..41c2d2d 100644 --- a/lib/db/repos/agents.js +++ b/lib/db/repos/agents.js @@ -42,29 +42,59 @@ export async function setCapabilities(id, capabilities, scopes) { return r; } +// Token format: vk_. +// selector — non-secret, indexed, locates exactly one row (O(1)) +// verifier — the secret; only this is bcrypt-hashed into token_hash +// base64url never contains '.', so the delimiter is unambiguous. export async function createToken(agent_id, label) { - const plaintext = 'vk_' + crypto.randomBytes(32).toString('base64url'); - const token_hash = await bcrypt.hash(plaintext, 12); + const selector = crypto.randomBytes(9).toString('base64url'); // ~12 chars + const verifier = crypto.randomBytes(32).toString('base64url'); // ~43 chars + const plaintext = `vk_${selector}.${verifier}`; + const token_hash = await bcrypt.hash(verifier, 12); const { rows: [t] } = await pool.query( - `INSERT INTO agent_tokens(agent_id, label, token_hash) VALUES($1,$2,$3) RETURNING id`, - [agent_id, label || null, token_hash] + `INSERT INTO agent_tokens(agent_id, label, token_hash, selector) VALUES($1,$2,$3,$4) RETURNING id`, + [agent_id, label || null, token_hash, selector] ); return { token: plaintext, id: t.id }; } +const AGENT_SELECT = + `SELECT t.id, t.token_hash, t.agent_id, a.* + FROM agent_tokens t JOIN agents a ON a.id = t.agent_id`; + +async function finalizeVerify(row, secret) { + if (await bcrypt.compare(secret, row.token_hash)) { + await pool.query(`UPDATE agent_tokens SET last_used=now() WHERE id=$1`, [row.id]); + const { token_hash, selector, ...agent } = row; + return agent; + } + return null; +} + export async function verifyToken(plaintext) { if (!plaintext?.startsWith('vk_')) return null; + const rest = plaintext.slice(3); + const dot = rest.indexOf('.'); + + if (dot !== -1) { + // New format — single indexed lookup by selector, bcrypt the verifier only. + const selector = rest.slice(0, dot); + const verifier = rest.slice(dot + 1); + const { rows: [row] } = await pool.query( + `${AGENT_SELECT} WHERE t.selector = $1 AND t.revoked_at IS NULL`, [selector] + ); + if (!row) return null; + return finalizeVerify(row, verifier); + } + + // Legacy fallback — pre-migration tokens hashed the full plaintext and have + // no selector. Scan only the (shrinking) NULL-selector set. const { rows } = await pool.query( - `SELECT t.id, t.token_hash, t.agent_id, a.* - FROM agent_tokens t JOIN agents a ON a.id = t.agent_id - WHERE t.revoked_at IS NULL` + `${AGENT_SELECT} WHERE t.selector IS NULL AND t.revoked_at IS NULL` ); for (const row of rows) { - if (await bcrypt.compare(plaintext, row.token_hash)) { - await pool.query(`UPDATE agent_tokens SET last_used=now() WHERE id=$1`, [row.id]); - const { token_hash, ...agent } = row; - return agent; - } + const agent = await finalizeVerify(row, plaintext); + if (agent) return agent; } return null; } @@ -72,3 +102,15 @@ export async function verifyToken(plaintext) { export async function revokeToken(token_id) { await pool.query(`UPDATE agent_tokens SET revoked_at=now() WHERE id=$1`, [token_id]); } + +// Token metadata for security review — label/usage/revocation joined with the +// owning agent. NEVER selects token_hash. +export async function listTokenMeta() { + const { rows } = await pool.query( + `SELECT t.id, t.agent_id, a.slug AS agent_slug, a.name AS agent_name, + t.label, t.last_used, t.created_at, t.revoked_at + FROM agent_tokens t JOIN agents a ON a.id = t.agent_id + ORDER BY t.created_at DESC` + ); + return rows; +} diff --git a/tests/repos/token_selector.test.js b/tests/repos/token_selector.test.js new file mode 100644 index 0000000..4d030cd --- /dev/null +++ b/tests/repos/token_selector.test.js @@ -0,0 +1,48 @@ +import { describe, it, expect, beforeAll } from 'vitest'; +import bcrypt from 'bcrypt'; +import { pool } from '../../lib/db/pool.js'; +import { resetDb } from '../helpers/db.js'; +import { migrateUp } from '../../lib/db/migrate.js'; +import * as agents from '../../lib/db/repos/agents.js'; + +// verifyToken must be O(1): a public "selector" indexes the one candidate row, +// then bcrypt verifies only that row's "verifier". (Previously it bcrypt-scanned +// every token — code-review/security-sweep HIGH finding.) + +const owner = { kind: 'user', id: null }; +let agent; +beforeAll(async () => { + await resetDb(); await migrateUp(); + agent = await agents.create({ slug: 'tok', name: 'Tok', kind: 'claude', model: 'sonnet', + capabilities: { read: true }, scopes: {} }, owner); +}); + +describe('selector+verifier tokens', () => { + it('new tokens are vk_. and verify O(1) by selector', async () => { + const { token } = await agents.createToken(agent.id, 'k1'); + expect(token).toMatch(/^vk_[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+$/); + // the selector is stored in plaintext and indexed + const selector = token.slice(3, token.indexOf('.')); + const { rows } = await pool.query('SELECT selector FROM agent_tokens WHERE selector=$1', [selector]); + expect(rows).toHaveLength(1); + const found = await agents.verifyToken(token); + expect(found.id).toBe(agent.id); + }); + + it('a tampered verifier (right selector, wrong secret) does not verify', async () => { + const { token } = await agents.createToken(agent.id, 'k2'); + const tampered = token.slice(0, -3) + 'AAA'; + expect(await agents.verifyToken(tampered)).toBeNull(); + }); + + it('legacy tokens (selector NULL, full-plaintext hash) still verify', async () => { + const legacyPlain = 'vk_legacyflatToken1234567890'; + const hash = await bcrypt.hash(legacyPlain, 12); + await pool.query( + `INSERT INTO agent_tokens(agent_id, label, token_hash, selector) VALUES($1,'legacy',$2,NULL)`, + [agent.id, hash] + ); + const found = await agents.verifyToken(legacyPlain); + expect(found.id).toBe(agent.id); + }); +});