From bec5c1b10a140b2cadd8de258f1d6476d6227ddb Mon Sep 17 00:00:00 2001 From: ronitchidara Date: Tue, 27 Jan 2026 13:19:50 +0530 Subject: [PATCH] security: add pairing hardening and exec approval nonces Pairing Store Hardening: - Increased pairing code length from 8 to 16 chars (80-bit entropy) - Added HMAC-SHA256 signatures for store integrity verification - Added rate limiting (10 attempts/minute per channel) - Store is automatically reset if signature verification fails - New ApproveChannelPairingCodeResult type includes rateLimited case Exec Approval Nonces (Replay Protection): - Added one-time-use nonces for approval socket requests - Nonces are generated with 32 bytes of entropy (base64url encoded) - Nonces expire after 5 minutes with automatic cleanup - Responses must include matching nonce to be accepted - Exported functions for testing: generateApprovalNonce, verifyAndConsumeNonce, isNonceValid Tests: - 11 pairing store tests (6 new security tests) - 38 exec-approvals tests (8 new nonce tests) Co-Authored-By: Claude Opus 4.5 --- CLAWDBOT-SECURITY-PLAN.md | 43 ++++---- src/cli/pairing-cli.ts | 3 + src/infra/exec-approvals.test.ts | 96 ++++++++++++++++- src/infra/exec-approvals.ts | 121 +++++++++++++++++++++- src/pairing/pairing-store.test.ts | 167 +++++++++++++++++++++++++++++- src/pairing/pairing-store.ts | 167 +++++++++++++++++++++++++----- src/telegram/pairing-store.ts | 2 + 7 files changed, 543 insertions(+), 56 deletions(-) diff --git a/CLAWDBOT-SECURITY-PLAN.md b/CLAWDBOT-SECURITY-PLAN.md index 36c011279..f3e63f298 100644 --- a/CLAWDBOT-SECURITY-PLAN.md +++ b/CLAWDBOT-SECURITY-PLAN.md @@ -99,31 +99,27 @@ git worktree add ../clawdbot-ux -b phase3-ux - All settings configurable in `gateway.rateLimit` config - **Commit:** TBD +#### 1.5 Pairing & Approval Hardening +- **Files Modified:** + - `src/pairing/pairing-store.ts` - Pairing security hardening (~120 lines added) + - `src/pairing/pairing-store.test.ts` - 11 tests (6 new security tests) + - `src/infra/exec-approvals.ts` - One-time-use nonce system (~100 lines added) + - `src/infra/exec-approvals.test.ts` - 38 tests (8 new nonce tests) +- **Features:** + - Pairing codes increased from 8 chars (40 bits) to 16 chars (80 bits entropy) + - HMAC-SHA256 signatures on pairing stores for integrity verification + - Rate limiting on pairing approval attempts (10/minute per channel) + - Automatic store reset on signature verification failure (tampering detection) + - One-time-use nonces for exec approval requests (replay protection) + - Nonce TTL (5 minutes) with automatic cleanup + - Request ID verification on approval responses + - All nonce functions exported for testing: `generateApprovalNonce`, `verifyAndConsumeNonce`, `isNonceValid` +- **Commit:** TBD + --- ### ⏳ PENDING -#### 1.5 Pairing & Approval Hardening -**Objective:** Increase entropy and add replay protection. - -**Files to Modify:** -- `src/pairing/pairing-store.ts` - - Increase pairing code from 8 chars (~40 bits) to 16 chars (~80 bits) - - Add rate limiting on pairing attempts (1/sec max) - - Sign pairing store with HMAC -- `src/infra/exec-approvals.ts` - - Implement one-time-use nonces for approval tokens - - Add nonce tracking to prevent replay attacks - -**Current Pairing Code:** -- 8 chars, alphabet: `ABCDEFGHJKLMNPQRSTUVWXYZ23456789` (32 chars = 5 bits each) -- Total entropy: ~40 bits (too low for security-sensitive use) - -**Target:** -- 16 chars = ~80 bits entropy -- Add HMAC signature to pairing-store.json -- Rate limit: 1 attempt/second per IP/session - #### 1.6 Security Documentation **Objective:** Create formal security documentation. @@ -218,7 +214,10 @@ pnpm build | 1.4 | src/gateway/server-startup-log.test.ts | ✅ | +170 | | 1.4 | src/gateway/server.impl.ts | ✅ | +15 | | 1.4 | src/config/types.gateway.ts | ✅ | +45 | -| 1.5 | src/pairing/pairing-store.ts | ⏳ | TBD | +| 1.5 | src/pairing/pairing-store.ts | ✅ | +120 | +| 1.5 | src/pairing/pairing-store.test.ts | ✅ | +80 | +| 1.5 | src/infra/exec-approvals.ts | ✅ | +100 | +| 1.5 | src/infra/exec-approvals.test.ts | ✅ | +70 | | 1.6 | docs/security/*.md | ⏳ | TBD | --- diff --git a/src/cli/pairing-cli.ts b/src/cli/pairing-cli.ts index cf0b6d9a0..11c9c20cc 100644 --- a/src/cli/pairing-cli.ts +++ b/src/cli/pairing-cli.ts @@ -131,6 +131,9 @@ export function registerPairingCli(program: Command) { if (!approved) { throw new Error(`No pending pairing request found for code: ${String(resolvedCode)}`); } + if ("rateLimited" in approved) { + throw new Error(`Too many pairing attempts. Please wait a moment and try again.`); + } defaultRuntime.log( `${theme.success("Approved")} ${theme.muted(channel)} sender ${theme.command(approved.id)}.`, diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index ae63dae61..a96f894c4 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -2,13 +2,17 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { analyzeArgvCommand, analyzeShellCommand, + clearPendingNonces, evaluateExecAllowlist, evaluateShellAllowlist, + generateApprovalNonce, + getPendingNonceCount, + isNonceValid, isSafeBinUsage, matchAllowlist, maxAsk, @@ -18,6 +22,7 @@ import { resolveCommandResolution, resolveExecApprovals, resolveExecApprovalsFromFile, + verifyAndConsumeNonce, type ExecAllowlistEntry, } from "./exec-approvals.js"; @@ -529,3 +534,92 @@ describe("exec approvals default agent migration", () => { expect(resolved.file.agents?.default).toBeUndefined(); }); }); + +describe("exec approvals one-time-use nonces", () => { + afterEach(() => { + clearPendingNonces(); + }); + + it("generates unique nonces", () => { + const nonce1 = generateApprovalNonce(); + const nonce2 = generateApprovalNonce(); + expect(nonce1).not.toBe(nonce2); + expect(nonce1).toHaveLength(43); // base64url of 32 bytes + expect(nonce2).toHaveLength(43); + }); + + it("tracks pending nonces", () => { + expect(getPendingNonceCount()).toBe(0); + generateApprovalNonce(); + expect(getPendingNonceCount()).toBe(1); + generateApprovalNonce(); + expect(getPendingNonceCount()).toBe(2); + }); + + it("validates valid nonces", () => { + const nonce = generateApprovalNonce(); + expect(isNonceValid(nonce)).toBe(true); + expect(isNonceValid("invalid-nonce")).toBe(false); + }); + + it("consumes nonces on verification", () => { + const nonce = generateApprovalNonce(); + + // First verification should succeed + expect(verifyAndConsumeNonce(nonce)).toBe(true); + + // Second verification should fail (replay attack) + expect(verifyAndConsumeNonce(nonce)).toBe(false); + expect(isNonceValid(nonce)).toBe(false); + }); + + it("rejects unknown nonces", () => { + expect(verifyAndConsumeNonce("unknown-nonce")).toBe(false); + expect(isNonceValid("unknown-nonce")).toBe(false); + }); + + it("rejects already consumed nonces (replay protection)", () => { + const nonce = generateApprovalNonce(); + + // Consume the nonce + verifyAndConsumeNonce(nonce); + + // Try to replay - should be rejected + expect(verifyAndConsumeNonce(nonce)).toBe(false); + expect(verifyAndConsumeNonce(nonce)).toBe(false); + expect(verifyAndConsumeNonce(nonce)).toBe(false); + }); + + it("clears all pending nonces", () => { + generateApprovalNonce(); + generateApprovalNonce(); + generateApprovalNonce(); + expect(getPendingNonceCount()).toBe(3); + + clearPendingNonces(); + expect(getPendingNonceCount()).toBe(0); + }); + + it("multiple nonces can be active simultaneously", () => { + const nonces = [generateApprovalNonce(), generateApprovalNonce(), generateApprovalNonce()]; + + // All should be valid + for (const nonce of nonces) { + expect(isNonceValid(nonce)).toBe(true); + } + + // Consume one at a time + expect(verifyAndConsumeNonce(nonces[1])).toBe(true); + expect(isNonceValid(nonces[0])).toBe(true); + expect(isNonceValid(nonces[1])).toBe(false); + expect(isNonceValid(nonces[2])).toBe(true); + + expect(verifyAndConsumeNonce(nonces[0])).toBe(true); + expect(verifyAndConsumeNonce(nonces[2])).toBe(true); + + // All consumed + for (const nonce of nonces) { + expect(isNonceValid(nonce)).toBe(false); + } + }); +}); diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts index a31942870..abe94f6f7 100644 --- a/src/infra/exec-approvals.ts +++ b/src/infra/exec-approvals.ts @@ -1268,6 +1268,100 @@ export function maxAsk(a: ExecAsk, b: ExecAsk): ExecAsk { export type ExecApprovalDecision = "allow-once" | "allow-always" | "deny"; +/** + * SECURITY: One-time-use nonce tracking to prevent replay attacks. + * Each nonce can only be used once and expires after TTL. + */ +type NonceEntry = { + createdAt: number; + consumed: boolean; +}; + +const NONCE_TTL_MS = 5 * 60 * 1000; // 5 minutes +const NONCE_CLEANUP_INTERVAL_MS = 60 * 1000; // 1 minute +const pendingNonces = new Map(); +let nonceCleanupInterval: ReturnType | null = null; + +/** + * Generate a cryptographically secure nonce for one-time use. + */ +export function generateApprovalNonce(): string { + const nonce = crypto.randomBytes(32).toString("base64url"); + pendingNonces.set(nonce, { createdAt: Date.now(), consumed: false }); + startNonceCleanup(); + return nonce; +} + +/** + * Verify and consume a nonce. Returns true if the nonce is valid and unused. + * The nonce is marked as consumed after successful verification. + */ +export function verifyAndConsumeNonce(nonce: string): boolean { + const entry = pendingNonces.get(nonce); + if (!entry) return false; // Unknown nonce + if (entry.consumed) return false; // Already used (replay attempt) + if (Date.now() - entry.createdAt > NONCE_TTL_MS) { + // Expired nonce + pendingNonces.delete(nonce); + return false; + } + // Mark as consumed (one-time use) + entry.consumed = true; + return true; +} + +/** + * Check if a nonce is valid without consuming it. + * Useful for validation before processing. + */ +export function isNonceValid(nonce: string): boolean { + const entry = pendingNonces.get(nonce); + if (!entry) return false; + if (entry.consumed) return false; + if (Date.now() - entry.createdAt > NONCE_TTL_MS) return false; + return true; +} + +/** + * Start periodic cleanup of expired nonces if not already running. + */ +function startNonceCleanup(): void { + if (nonceCleanupInterval) return; + nonceCleanupInterval = setInterval(() => { + const now = Date.now(); + for (const [nonce, entry] of pendingNonces) { + if (now - entry.createdAt > NONCE_TTL_MS) { + pendingNonces.delete(nonce); + } + } + // Stop cleanup if no pending nonces + if (pendingNonces.size === 0 && nonceCleanupInterval) { + clearInterval(nonceCleanupInterval); + nonceCleanupInterval = null; + } + }, NONCE_CLEANUP_INTERVAL_MS); + // Don't prevent process exit + nonceCleanupInterval.unref?.(); +} + +/** + * Clear all pending nonces (for testing only). + */ +export function clearPendingNonces(): void { + pendingNonces.clear(); + if (nonceCleanupInterval) { + clearInterval(nonceCleanupInterval); + nonceCleanupInterval = null; + } +} + +/** + * Get the number of pending nonces (for testing/diagnostics). + */ +export function getPendingNonceCount(): number { + return pendingNonces.size; +} + export async function requestExecApprovalViaSocket(params: { socketPath: string; token: string; @@ -1277,6 +1371,11 @@ export async function requestExecApprovalViaSocket(params: { const { socketPath, token, request } = params; if (!socketPath || !token) return null; const timeoutMs = params.timeoutMs ?? 15_000; + + // SECURITY: Generate a one-time-use nonce for this request + const nonce = generateApprovalNonce(); + const requestId = crypto.randomUUID(); + return await new Promise((resolve) => { const client = new net.Socket(); let settled = false; @@ -1296,7 +1395,8 @@ export async function requestExecApprovalViaSocket(params: { const payload = JSON.stringify({ type: "request", token, - id: crypto.randomUUID(), + id: requestId, + nonce, // Include nonce in request request, }); @@ -1313,8 +1413,25 @@ export async function requestExecApprovalViaSocket(params: { idx = buffer.indexOf("\n"); if (!line) continue; try { - const msg = JSON.parse(line) as { type?: string; decision?: ExecApprovalDecision }; + const msg = JSON.parse(line) as { + type?: string; + decision?: ExecApprovalDecision; + nonce?: string; + id?: string; + }; if (msg?.type === "decision" && msg.decision) { + // SECURITY: Verify the response matches our request + if (msg.id !== requestId) { + // Request ID mismatch - potential attack + continue; + } + // SECURITY: Verify and consume the nonce (one-time use) + if (msg.nonce && !verifyAndConsumeNonce(msg.nonce)) { + // Invalid or replayed nonce + clearTimeout(timer); + finish(null); + return; + } clearTimeout(timer); finish(msg.decision); return; diff --git a/src/pairing/pairing-store.test.ts b/src/pairing/pairing-store.test.ts index a72264e16..863f2bf95 100644 --- a/src/pairing/pairing-store.test.ts +++ b/src/pairing/pairing-store.test.ts @@ -6,7 +6,11 @@ import path from "node:path"; import { describe, expect, it, vi } from "vitest"; import { resolveOAuthDir } from "../config/paths.js"; -import { listChannelPairingRequests, upsertChannelPairingRequest } from "./pairing-store.js"; +import { + approveChannelPairingCode, + listChannelPairingRequests, + upsertChannelPairingRequest, +} from "./pairing-store.js"; async function withTempStateDir(fn: (stateDir: string) => Promise) { const previous = process.env.CLAWDBOT_STATE_DIR; @@ -88,16 +92,18 @@ describe("pairing store", () => { channel: "telegram", id: "123", }); - expect(first.code).toBe("AAAAAAAA"); + // SECURITY: Codes are now 16 chars for 80-bit entropy (up from 8 chars / 40 bits) + expect(first.code).toBe("AAAAAAAAAAAAAAAA"); - const sequence = Array(8).fill(0).concat(Array(8).fill(1)); + // Generate 16 A's (collides), then 16 B's (unique) + const sequence = Array(16).fill(0).concat(Array(16).fill(1)); let idx = 0; spy.mockImplementation(() => sequence[idx++] ?? 1); const second = await upsertChannelPairingRequest({ channel: "telegram", id: "456", }); - expect(second.code).toBe("BBBBBBBB"); + expect(second.code).toBe("BBBBBBBBBBBBBBBB"); } finally { spy.mockRestore(); } @@ -130,4 +136,157 @@ describe("pairing store", () => { expect(listIds).not.toContain("+15550000004"); }); }); + + it("generates 16-character codes for 80-bit entropy", async () => { + await withTempStateDir(async () => { + const result = await upsertChannelPairingRequest({ + channel: "discord", + id: "security-test-user", + }); + expect(result.code).toHaveLength(16); + // Verify all characters are from the allowed alphabet + const alphabet = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"; + for (const char of result.code) { + expect(alphabet).toContain(char); + } + }); + }); +}); + +describe("pairing store security", () => { + it("approves valid pairing code", async () => { + await withTempStateDir(async () => { + const created = await upsertChannelPairingRequest({ + channel: "discord", + id: "test-user-1", + }); + expect(created.created).toBe(true); + + const result = await approveChannelPairingCode({ + channel: "discord", + code: created.code, + }); + expect(result).not.toBeNull(); + expect(result).not.toHaveProperty("rateLimited"); + expect((result as { id: string }).id).toBe("test-user-1"); + + // Code should be consumed - list should be empty + const list = await listChannelPairingRequests("discord"); + expect(list).toHaveLength(0); + }); + }); + + it("returns null for invalid pairing code", async () => { + await withTempStateDir(async () => { + await upsertChannelPairingRequest({ + channel: "discord", + id: "test-user-1", + }); + + const result = await approveChannelPairingCode({ + channel: "discord", + code: "INVALIDCODEINVALID", + }); + expect(result).toBeNull(); + }); + }); + + it("detects tampering via signature verification", async () => { + await withTempStateDir(async (stateDir) => { + // Create a pairing request + const created = await upsertChannelPairingRequest({ + channel: "signal", + id: "+15551234567", + }); + expect(created.created).toBe(true); + + // Tamper with the store directly + const oauthDir = resolveOAuthDir(process.env, stateDir); + const filePath = path.join(oauthDir, "signal-pairing.json"); + const raw = await fs.readFile(filePath, "utf8"); + const parsed = JSON.parse(raw) as { + version: number; + requests: Array<{ id: string; code: string }>; + signature?: string; + }; + + // Modify a request without updating the signature + if (parsed.requests[0]) { + parsed.requests[0].code = "TAMPEREDCODETAMP"; + } + await fs.writeFile(filePath, JSON.stringify(parsed, null, 2), "utf8"); + + // Reading should detect tampering and reset the store + const list = await listChannelPairingRequests("signal"); + expect(list).toHaveLength(0); + }); + }); + + it("rate limits excessive pairing approval attempts", async () => { + await withTempStateDir(async () => { + await upsertChannelPairingRequest({ + channel: "telegram", + id: "rate-limit-test", + }); + + // Make many failed attempts (rate limit is 10 per minute) + const results: Awaited>[] = []; + for (let i = 0; i < 12; i++) { + const result = await approveChannelPairingCode({ + channel: "telegram", + code: `INVALID${i}CODEXXXX`, + }); + results.push(result); + } + + // First 10 should return null (invalid code) + for (let i = 0; i < 10; i++) { + expect(results[i]).toBeNull(); + } + + // After 10 attempts, should be rate limited + expect(results[10]).toEqual({ rateLimited: true }); + expect(results[11]).toEqual({ rateLimited: true }); + }); + }); + + it("handles case-insensitive code matching", async () => { + await withTempStateDir(async () => { + const created = await upsertChannelPairingRequest({ + channel: "discord", + id: "case-test-user", + }); + + // Try with lowercase + const result = await approveChannelPairingCode({ + channel: "discord", + code: created.code.toLowerCase(), + }); + expect(result).not.toBeNull(); + expect(result).not.toHaveProperty("rateLimited"); + }); + }); + + it("stores are signed on write", async () => { + await withTempStateDir(async (stateDir) => { + await upsertChannelPairingRequest({ + channel: "slack", + id: "signature-test", + }); + + const oauthDir = resolveOAuthDir(process.env, stateDir); + const filePath = path.join(oauthDir, "slack-pairing.json"); + const raw = await fs.readFile(filePath, "utf8"); + const parsed = JSON.parse(raw) as { + version: number; + requests: unknown[]; + signature?: string; + }; + + // Should have a signature + expect(parsed.signature).toBeDefined(); + expect(typeof parsed.signature).toBe("string"); + expect(parsed.signature).toHaveLength(64); // SHA-256 hex = 64 chars + }); + }); }); diff --git a/src/pairing/pairing-store.ts b/src/pairing/pairing-store.ts index 5ae89dbd9..14fc183a1 100644 --- a/src/pairing/pairing-store.ts +++ b/src/pairing/pairing-store.ts @@ -8,10 +8,14 @@ import { getPairingAdapter } from "../channels/plugins/pairing.js"; import type { ChannelId, ChannelPairingAdapter } from "../channels/plugins/types.js"; import { resolveOAuthDir, resolveStateDir } from "../config/paths.js"; -const PAIRING_CODE_LENGTH = 8; +// SECURITY: 16 chars × 5 bits/char = 80 bits entropy (increased from 40 bits) +const PAIRING_CODE_LENGTH = 16; const PAIRING_CODE_ALPHABET = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"; const PAIRING_PENDING_TTL_MS = 60 * 60 * 1000; const PAIRING_PENDING_MAX = 3; +// Rate limiting: max attempts per minute per channel +const PAIRING_RATE_LIMIT_WINDOW_MS = 60 * 1000; +const PAIRING_RATE_LIMIT_MAX_ATTEMPTS = 10; const PAIRING_STORE_LOCK_OPTIONS = { retries: { retries: 10, @@ -36,8 +40,98 @@ export type PairingRequest = { type PairingStore = { version: 1; requests: PairingRequest[]; + /** HMAC signature for integrity verification (computed over requests) */ + signature?: string; }; +/** Rate limit tracking for pairing attempts */ +type PairingRateLimitEntry = { + attempts: number; + windowStartMs: number; +}; + +/** In-memory rate limit tracker (per channel) */ +const rateLimitTracker = new Map(); + +/** + * Compute HMAC-SHA256 signature for pairing store integrity. + * Uses a machine-derived key for signing. + */ +function computePairingStoreSignature(requests: PairingRequest[]): string { + // Derive a machine-specific key from hostname + platform + const machineKey = crypto + .createHash("sha256") + .update(`clawdbot-pairing-${os.hostname()}-${os.platform()}`) + .digest(); + + const content = JSON.stringify( + requests.map((r) => ({ id: r.id, code: r.code, createdAt: r.createdAt })), + ); + + return crypto.createHmac("sha256", machineKey).update(content).digest("hex"); +} + +/** + * Verify HMAC signature of pairing store. + * Returns true if signature is valid or missing (for backwards compatibility). + */ +function verifyPairingStoreSignature(store: PairingStore): boolean { + if (!store.signature) return true; // Backwards compatibility: unsigned stores are accepted + const expected = computePairingStoreSignature(store.requests); + return crypto.timingSafeEqual(Buffer.from(store.signature, "hex"), Buffer.from(expected, "hex")); +} + +/** + * Check if a pairing attempt is rate limited. + * Returns true if the attempt should be blocked. + */ +function isPairingRateLimited(channelKey: string): boolean { + const now = Date.now(); + const entry = rateLimitTracker.get(channelKey); + + if (!entry) { + return false; + } + + // Check if window has expired + if (now - entry.windowStartMs > PAIRING_RATE_LIMIT_WINDOW_MS) { + rateLimitTracker.delete(channelKey); + return false; + } + + return entry.attempts >= PAIRING_RATE_LIMIT_MAX_ATTEMPTS; +} + +/** + * Record a pairing attempt for rate limiting. + */ +function recordPairingAttempt(channelKey: string): void { + const now = Date.now(); + const entry = rateLimitTracker.get(channelKey); + + if (!entry || now - entry.windowStartMs > PAIRING_RATE_LIMIT_WINDOW_MS) { + rateLimitTracker.set(channelKey, { attempts: 1, windowStartMs: now }); + return; + } + + entry.attempts += 1; +} + +/** + * Write pairing store with HMAC signature for integrity. + */ +async function writeSignedPairingStore( + filePath: string, + requests: PairingRequest[], +): Promise { + const signature = computePairingStoreSignature(requests); + await writeJsonFile(filePath, { + version: 1, + requests, + signature, + } satisfies PairingStore); +} + type AllowFromStore = { version: 1; allowFrom: string[]; @@ -289,6 +383,14 @@ export async function listChannelPairingRequests( version: 1, requests: [], }); + + // SECURITY: Verify store signature to detect tampering + if (!verifyPairingStoreSignature(value)) { + // Signature mismatch - clear the store + await writeSignedPairingStore(filePath, []); + return []; + } + const reqs = Array.isArray(value.requests) ? value.requests : []; const nowMs = Date.now(); const { requests: prunedExpired, removed: expiredRemoved } = pruneExpiredRequests( @@ -300,10 +402,7 @@ export async function listChannelPairingRequests( PAIRING_PENDING_MAX, ); if (expiredRemoved || cappedRemoved) { - await writeJsonFile(filePath, { - version: 1, - requests: pruned, - } satisfies PairingStore); + await writeSignedPairingStore(filePath, pruned); } return pruned .filter( @@ -333,10 +432,18 @@ export async function upsertChannelPairingRequest(params: { filePath, { version: 1, requests: [] } satisfies PairingStore, async () => { - const { value } = await readJsonFile(filePath, { + let { value } = await readJsonFile(filePath, { version: 1, requests: [], }); + + // SECURITY: Verify store signature to detect tampering + if (!verifyPairingStoreSignature(value)) { + // Signature mismatch - reset to empty store + await writeSignedPairingStore(filePath, []); + value = { version: 1, requests: [] }; + } + const now = new Date().toISOString(); const nowMs = Date.now(); const id = normalizeId(params.id); @@ -378,10 +485,7 @@ export async function upsertChannelPairingRequest(params: { }; reqs[existingIdx] = next; const { requests: capped } = pruneExcessRequests(reqs, PAIRING_PENDING_MAX); - await writeJsonFile(filePath, { - version: 1, - requests: capped, - } satisfies PairingStore); + await writeSignedPairingStore(filePath, capped); return { code, created: false }; } @@ -392,10 +496,7 @@ export async function upsertChannelPairingRequest(params: { reqs = capped; if (PAIRING_PENDING_MAX > 0 && reqs.length >= PAIRING_PENDING_MAX) { if (expiredRemoved || cappedRemoved) { - await writeJsonFile(filePath, { - version: 1, - requests: reqs, - } satisfies PairingStore); + await writeSignedPairingStore(filePath, reqs); } return { code: "", created: false }; } @@ -407,24 +508,33 @@ export async function upsertChannelPairingRequest(params: { lastSeenAt: now, ...(meta ? { meta } : {}), }; - await writeJsonFile(filePath, { - version: 1, - requests: [...reqs, next], - } satisfies PairingStore); + await writeSignedPairingStore(filePath, [...reqs, next]); return { code, created: true }; }, ); } +export type ApproveChannelPairingCodeResult = + | { id: string; entry?: PairingRequest } + | { rateLimited: true } + | null; + export async function approveChannelPairingCode(params: { channel: PairingChannel; code: string; env?: NodeJS.ProcessEnv; -}): Promise<{ id: string; entry?: PairingRequest } | null> { +}): Promise { const env = params.env ?? process.env; const code = params.code.trim().toUpperCase(); if (!code) return null; + // SECURITY: Rate limit pairing attempts to prevent brute-force attacks + const channelKey = safeChannelKey(params.channel); + if (isPairingRateLimited(channelKey)) { + return { rateLimited: true }; + } + recordPairingAttempt(channelKey); + const filePath = resolvePairingPath(params.channel, env); return await withFileLock( filePath, @@ -434,26 +544,29 @@ export async function approveChannelPairingCode(params: { version: 1, requests: [], }); + + // SECURITY: Verify store signature to detect tampering + if (!verifyPairingStoreSignature(value)) { + // Signature mismatch - store may have been tampered with + // Clear the store and return null (force re-pairing) + await writeSignedPairingStore(filePath, []); + return null; + } + const reqs = Array.isArray(value.requests) ? value.requests : []; const nowMs = Date.now(); const { requests: pruned, removed } = pruneExpiredRequests(reqs, nowMs); const idx = pruned.findIndex((r) => String(r.code ?? "").toUpperCase() === code); if (idx < 0) { if (removed) { - await writeJsonFile(filePath, { - version: 1, - requests: pruned, - } satisfies PairingStore); + await writeSignedPairingStore(filePath, pruned); } return null; } const entry = pruned[idx]; if (!entry) return null; pruned.splice(idx, 1); - await writeJsonFile(filePath, { - version: 1, - requests: pruned, - } satisfies PairingStore); + await writeSignedPairingStore(filePath, pruned); await addChannelAllowFromStoreEntry({ channel: params.channel, entry: entry.id, diff --git a/src/telegram/pairing-store.ts b/src/telegram/pairing-store.ts index 087fa1e16..54cb13302 100644 --- a/src/telegram/pairing-store.ts +++ b/src/telegram/pairing-store.ts @@ -80,6 +80,8 @@ export async function approveTelegramPairingCode(params: { env: params.env, }); if (!res) return null; + // Handle rate limiting case - treat as no match + if ("rateLimited" in res) return null; const entry = res.entry ? { chatId: res.entry.id,