From 36382521c48ee2c81c61bb5563c93d2cceedc7d7 Mon Sep 17 00:00:00 2001 From: kai-agent-free Date: Mon, 16 Mar 2026 19:40:56 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20critical=20security=20hardening=20?= =?UTF-8?q?=E2=80=94=20trust=20self-manipulation,=20webhook=20auth,=20HKDF?= =?UTF-8?q?=20salt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security fixes: 1. Trust score self-manipulation (#11): PATCH /passports/:id/trust/verify-owner and /payment-method now require admin role. Owners can no longer set trust flags on their own passports. Added 'role' column to owners table with migration for existing databases. 2. Unauthenticated webhook polling (#12): GET /webhook/email-notifications/:address and GET /webhook/sms-notifications/:phone now require Bearer token auth and scope access to the owner's own email/phone only. 3. HKDF static salt: deriveVaultKey() now supports per-vault random salt. New vaults automatically generate and store a 256-bit random salt. Existing vaults maintain backward compatibility with the legacy static salt. Also fixes pre-existing build errors (unused variables in trust.ts and captcha-service.ts). --- packages/api-server/src/db/schema.ts | 10 +++++ packages/api-server/src/middleware/auth.ts | 31 ++++++++++++- packages/api-server/src/routes/trust.ts | 38 ++++++++++------ packages/api-server/src/routes/webhooks.ts | 35 +++++++++++++-- packages/core/src/crypto/encryption.ts | 23 +++++++--- packages/core/src/crypto/index.ts | 2 +- packages/core/src/vault/vault.ts | 43 +++++++++++++++++-- .../src/services/captcha-service.ts | 2 +- 8 files changed, 155 insertions(+), 29 deletions(-) diff --git a/packages/api-server/src/db/schema.ts b/packages/api-server/src/db/schema.ts index 7359f0f..046c1d6 100644 --- a/packages/api-server/src/db/schema.ts +++ b/packages/api-server/src/db/schema.ts @@ -33,6 +33,7 @@ export async function initDatabase(connectionString?: string): Promise { password_hash TEXT NOT NULL, name TEXT NOT NULL DEFAULT '', verified BOOLEAN NOT NULL DEFAULT false, + role TEXT NOT NULL DEFAULT 'user', created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() ) @@ -40,6 +41,15 @@ export async function initDatabase(connectionString?: string): Promise { await sql`CREATE INDEX IF NOT EXISTS idx_owners_email ON owners(email)`; + // Add role column if it doesn't exist (migration for existing databases) + await sql` + DO $$ BEGIN + ALTER TABLE owners ADD COLUMN role TEXT NOT NULL DEFAULT 'user'; + EXCEPTION WHEN duplicate_column THEN + NULL; + END $$ + `; + // Create passports table await sql` CREATE TABLE IF NOT EXISTS passports ( diff --git a/packages/api-server/src/middleware/auth.ts b/packages/api-server/src/middleware/auth.ts index 1695a84..3cdeea1 100644 --- a/packages/api-server/src/middleware/auth.ts +++ b/packages/api-server/src/middleware/auth.ts @@ -17,6 +17,7 @@ import type { Sql } from "../db/schema.js"; export interface OwnerPayload extends JWTPayload { owner_id: string; email: string; + role?: string; } /** @@ -106,6 +107,7 @@ interface ApiKeyRow { interface OwnerRow { id: string; email: string; + role: string; } /** @@ -135,7 +137,7 @@ export async function authenticateApiKey( // Resolve owner email const ownerRows = await db` - SELECT id, email FROM owners WHERE id = ${row.owner_id} + SELECT id, email, role FROM owners WHERE id = ${row.owner_id} `; const owner = ownerRows[0]; if (!owner) continue; @@ -146,6 +148,7 @@ export async function authenticateApiKey( return { owner_id: owner.id, email: owner.email, + role: owner.role, }; } @@ -201,6 +204,15 @@ export function requireAuth(db?: Sql) { // Fall back to JWT verification try { const payload = await verifyJwt(token); + // Resolve role from DB if available + if (db) { + const ownerRows = await db` + SELECT id, email, role FROM owners WHERE id = ${payload.owner_id} + `; + if (ownerRows[0]) { + payload.role = ownerRows[0].role; + } + } c.set("owner", payload); await next(); } catch { @@ -211,3 +223,20 @@ export function requireAuth(db?: Sql) { } }; } + +/** + * Hono middleware that requires admin role. + * Must be used AFTER requireAuth middleware. + */ +export function requireAdmin() { + return async (c: Context, next: Next) => { + const owner = c.get("owner") as OwnerPayload | undefined; + if (!owner || owner.role !== "admin") { + return c.json( + { error: "Admin access required", code: "FORBIDDEN" }, + 403, + ); + } + await next(); + }; +} diff --git a/packages/api-server/src/routes/trust.ts b/packages/api-server/src/routes/trust.ts index aa46438..426151f 100644 --- a/packages/api-server/src/routes/trust.ts +++ b/packages/api-server/src/routes/trust.ts @@ -11,7 +11,7 @@ import { Hono } from "hono"; import { z } from "zod"; import type { Sql } from "../db/schema.js"; import { zValidator, getValidatedBody } from "../middleware/validation.js"; -import { requireAuth, type OwnerPayload, type AuthVariables } from "../middleware/auth.js"; +import { requireAuth, requireAdmin, type OwnerPayload, type AuthVariables } from "../middleware/auth.js"; import { calculateTrustScore, getTrustLevel, @@ -176,14 +176,19 @@ export function createTrustRouter(db: Sql): Hono<{ Variables: AuthVariables }> { }); }); - // PATCH /passports/:id/trust/verify-owner — set owner_verified flag - router.patch("/:id/trust/verify-owner", requireAuth(db), async (c) => { - const owner = c.get("owner") as OwnerPayload; + // PATCH /passports/:id/trust/verify-owner — set owner_verified flag (admin only) + router.patch("/:id/trust/verify-owner", requireAuth(db), requireAdmin(), async (c) => { const passportId = c.req.param("id"); - const result = await fetchOwnedPassport(c, passportId, owner); - if (result instanceof Response) return result; - const row = result; + // Admin can verify any passport — look it up without ownership check + const rows = await db` + SELECT id, owner_email, trust_score, status, metadata, created_at + FROM passports WHERE id = ${passportId} + `; + const row = rows[0]; + if (!row) { + return c.json({ error: "Passport not found", code: "NOT_FOUND" }, 404); + } const metadata = parseMetadata(row.metadata); metadata.owner_verified = true; @@ -202,14 +207,19 @@ export function createTrustRouter(db: Sql): Hono<{ Variables: AuthVariables }> { }); }); - // PATCH /passports/:id/trust/payment-method — set payment_method flag - router.patch("/:id/trust/payment-method", requireAuth(db), async (c) => { - const owner = c.get("owner") as OwnerPayload; + // PATCH /passports/:id/trust/payment-method — set payment_method flag (admin only) + router.patch("/:id/trust/payment-method", requireAuth(db), requireAdmin(), async (c) => { const passportId = c.req.param("id"); - const result = await fetchOwnedPassport(c, passportId, owner); - if (result instanceof Response) return result; - const row = result; + // Admin can set payment method on any passport — look it up without ownership check + const rows = await db` + SELECT id, owner_email, trust_score, status, metadata, created_at + FROM passports WHERE id = ${passportId} + `; + const row = rows[0]; + if (!row) { + return c.json({ error: "Passport not found", code: "NOT_FOUND" }, 404); + } const metadata = parseMetadata(row.metadata); metadata.payment_method = true; @@ -292,7 +302,7 @@ export function createTrustRouter(db: Sql): Hono<{ Variables: AuthVariables }> { (metadata.external_attestations as ExternalAttestationFactor[]).push(attestation); - const { score, level, factors } = await recalculateAndPersist( + const { score, level } = await recalculateAndPersist( passportId, metadata, row.created_at, diff --git a/packages/api-server/src/routes/webhooks.ts b/packages/api-server/src/routes/webhooks.ts index 4c986da..d9485c7 100644 --- a/packages/api-server/src/routes/webhooks.ts +++ b/packages/api-server/src/routes/webhooks.ts @@ -8,6 +8,7 @@ import { Hono } from 'hono'; import type { Sql } from '../db/schema.js'; import { createHmac, timingSafeEqual } from 'crypto'; +import { requireAuth, type OwnerPayload, type AuthVariables } from '../middleware/auth.js'; /** * Constant-time string comparison to prevent timing attacks. @@ -17,8 +18,8 @@ function constantTimeCompare(a: string, b: string): boolean { return timingSafeEqual(Buffer.from(a, 'utf-8'), Buffer.from(b, 'utf-8')); } -export function createWebhookRouter(db: Sql): Hono { - const app = new Hono(); +export function createWebhookRouter(db: Sql): Hono<{ Variables: AuthVariables }> { + const app = new Hono<{ Variables: AuthVariables }>(); /** * POST /webhook/email-received @@ -66,9 +67,20 @@ export function createWebhookRouter(db: Sql): Hono { * Poll for new email notifications for a specific address. * Returns list of pending notifications and marks them as retrieved. */ - app.get('/email-notifications/:address', async (c) => { + app.get('/email-notifications/:address', requireAuth(db), async (c) => { + const owner = c.get('owner') as OwnerPayload; const address = c.req.param('address').toLowerCase(); + // Scope to owner's own email address or passport email addresses + const ownerPassports = await db<{ owner_email: string }[]>` + SELECT DISTINCT owner_email FROM passports WHERE owner_email = ${address} + `; + const isOwnerEmail = owner.email.toLowerCase() === address; + const isPassportEmail = ownerPassports.length > 0 && ownerPassports[0]?.owner_email === owner.email; + if (!isOwnerEmail && !isPassportEmail) { + return c.json({ error: 'Access denied: address does not belong to you', code: 'FORBIDDEN' }, 403); + } + // Get all unprocessed notifications for this address interface EmailNotificationRow { email_id: string; @@ -193,9 +205,24 @@ export function createWebhookRouter(db: Sql): Hono { * Poll for new SMS notifications for a specific phone number. * Returns list of pending notifications and marks them as retrieved. */ - app.get('/sms-notifications/:phoneNumber', async (c) => { + app.get('/sms-notifications/:phoneNumber', requireAuth(db), async (c) => { + const owner = c.get('owner') as OwnerPayload; const phoneNumber = c.req.param('phoneNumber'); + // Scope to owner's own phone — check settings table for phone association + const phoneRows = await db<{ value: string }[]>` + SELECT value FROM owner_settings + WHERE owner_id = ${owner.owner_id} AND key = 'phone_number' AND value = ${phoneNumber} + `; + // Also check if phone matches any passport metadata phone + const passportPhoneRows = await db<{ id: string }[]>` + SELECT id FROM passports + WHERE owner_email = ${owner.email} AND metadata->>'phone' = ${phoneNumber} + `; + if (phoneRows.length === 0 && passportPhoneRows.length === 0) { + return c.json({ error: 'Access denied: phone number does not belong to you', code: 'FORBIDDEN' }, 403); + } + // Get all unprocessed notifications for this phone number interface SmsNotificationRow { sms_id: string; diff --git a/packages/core/src/crypto/encryption.ts b/packages/core/src/crypto/encryption.ts index 7fb4139..6ca03f7 100644 --- a/packages/core/src/crypto/encryption.ts +++ b/packages/core/src/crypto/encryption.ts @@ -22,7 +22,8 @@ const KEY_LENGTH = 32; // 256-bit key /** HKDF parameters for vault key derivation */ const HKDF_DIGEST = "sha256"; -const HKDF_SALT = "agentpass-vault"; +const HKDF_SALT_LEGACY = "agentpass-vault"; +const HKDF_SALT_LENGTH = 32; // 256-bit random salt const HKDF_INFO = "credential-vault-key"; /** @@ -107,27 +108,39 @@ export function decrypt(encoded: string, key: Buffer): string { return decrypted.toString("utf8"); } +/** + * Generate a cryptographically random salt for HKDF key derivation. + * + * @returns A 32-byte random salt as a base64url string + */ +export function generateVaultSalt(): string { + return randomBytes(HKDF_SALT_LENGTH).toString("base64url"); +} + /** * Derive a 32-byte AES-256 vault key from an Ed25519 private key using HKDF. * - * Uses HKDF with SHA-256, a fixed salt and info string so the same - * private key always produces the same vault key. + * When `salt` is provided, it is used as the HKDF salt (recommended for new vaults). + * When `salt` is omitted, falls back to the legacy static salt for backward compatibility. * * @param privateKey - base64url-encoded Ed25519 private key + * @param salt - Optional base64url-encoded random salt (recommended) * @returns A 32-byte Buffer suitable for AES-256-GCM */ -export async function deriveVaultKey(privateKey: string): Promise { +export async function deriveVaultKey(privateKey: string, salt?: string): Promise { const ikm = Buffer.from(privateKey, "base64url"); if (ikm.length === 0) { throw new Error("Private key must not be empty"); } + const hkdfSalt = salt ? Buffer.from(salt, "base64url") : HKDF_SALT_LEGACY; + const derived = await new Promise((resolve, reject) => { hkdf( HKDF_DIGEST, ikm, - HKDF_SALT, + hkdfSalt, HKDF_INFO, KEY_LENGTH, (err, derivedKey) => { diff --git a/packages/core/src/crypto/index.ts b/packages/core/src/crypto/index.ts index 40e60bf..278af3e 100644 --- a/packages/core/src/crypto/index.ts +++ b/packages/core/src/crypto/index.ts @@ -16,4 +16,4 @@ export { verifyChallenge, } from "./signing.js"; -export { encrypt, decrypt, deriveVaultKey } from "./encryption.js"; +export { encrypt, decrypt, deriveVaultKey, generateVaultSalt } from "./encryption.js"; diff --git a/packages/core/src/vault/vault.ts b/packages/core/src/vault/vault.ts index 746401d..b6dbc72 100644 --- a/packages/core/src/vault/vault.ts +++ b/packages/core/src/vault/vault.ts @@ -9,7 +9,7 @@ import Database from "better-sqlite3"; import type { StoredCredential, AgentPassport } from "../types/index.js"; -import { encrypt, decrypt, deriveVaultKey } from "../crypto/index.js"; +import { encrypt, decrypt, deriveVaultKey, generateVaultSalt } from "../crypto/index.js"; /** Metadata returned by `list()` — never includes passwords or cookies. */ export interface CredentialListEntry { @@ -64,8 +64,6 @@ export class CredentialVault { * Must be called before any other method. */ async init(): Promise { - this.vaultKey = await deriveVaultKey(this.privateKey); - this.db = new Database(this.dbPath); // Enable WAL mode for better concurrent read performance @@ -88,6 +86,45 @@ export class CredentialVault { updated_at TEXT NOT NULL ) `); + + // Vault metadata table for storing per-vault salt + this.db.exec(` + CREATE TABLE IF NOT EXISTS vault_metadata ( + key TEXT PRIMARY KEY, + value TEXT NOT NULL + ) + `); + + // Get or generate salt + const saltRow = this.db.prepare( + "SELECT value FROM vault_metadata WHERE key = 'hkdf_salt'" + ).get() as { value: string } | undefined; + + let salt: string | undefined; + if (saltRow) { + salt = saltRow.value; + } else { + // Check if vault has existing data (legacy vault with static salt) + const hasData = this.db.prepare( + "SELECT 1 FROM credentials LIMIT 1" + ).get(); + const hasIdentities = this.db.prepare( + "SELECT 1 FROM identities LIMIT 1" + ).get(); + + if (hasData || hasIdentities) { + // Legacy vault — use no salt (falls back to static salt) for compatibility + salt = undefined; + } else { + // New vault — generate random salt + salt = generateVaultSalt(); + this.db.prepare( + "INSERT INTO vault_metadata (key, value) VALUES ('hkdf_salt', ?)" + ).run(salt); + } + } + + this.vaultKey = await deriveVaultKey(this.privateKey, salt); } /** diff --git a/packages/mcp-server/src/services/captcha-service.ts b/packages/mcp-server/src/services/captcha-service.ts index 76dab5a..51528fd 100644 --- a/packages/mcp-server/src/services/captcha-service.ts +++ b/packages/mcp-server/src/services/captcha-service.ts @@ -254,7 +254,7 @@ export class CaptchaService { } // Wait before next poll - await new Promise((resolve, reject) => { + await new Promise((resolve, _reject) => { const timer = setTimeout(resolve, pollIntervalMs); if (signal) { const onAbort = () => {