Harden runtime, storage, gateway, credentials, and IPC inputs

CHANGELOG.md

@@ -0,0 +1,25 @@
+# Changelog
+
+## Unreleased
+
+### Hardening phases 1-5
+
+This change set collects the first five maintenance hardening phases from the
+Rive deep-read follow-up work.
+
+| Phase | Area | Summary |
+| --- | --- | --- |
+| 1 | Runtime permission and usage handling | Made stream watchdog pause/resume accounting robust for concurrent tool calls, added permission timeout handling, integrated Office document abort propagation, and fixed cache/reasoning token usage extraction. |
+| 2 | Session JSONL recovery | Recovered sessions with corrupt JSONL rows by parsing message lines independently, surfacing landed corrupt rows as `system_note`, and dropping malformed truncated tail rows. |
+| 3 | Bot and OpenGateway abuse controls | Added bot inbound rate and session-binding limits, bounded bot dedupe state, forced bot-bound sessions to `explore`, and capped OpenGateway SSE connections with idle cleanup. |
+| 4 | Credential-store secret kind expansion | Extended encrypted credential-store support for bot tokens, bot app secrets, proxy passwords, gateway tokens, and Tavily API keys while preserving legacy API-key/OAuth-token key formats. |
+| 5 | Connection credential IPC input hardening | Added shared main-process validation for renderer-controlled connection slugs and API keys before store, credential, or provider side effects. |
+
+### Verification
+
+- Runtime package typecheck/build and focused runtime tests.
+- Storage package build and focused session-store tests.
+- Desktop main build/typecheck and focused bot/OpenGateway, credential-store,
+  settings/web-search, connection IPC, OAuth, and model-provider regression
+  suites.
+- `git diff --check` before each pushed phase.

apps/desktop/src/main/__tests__/bot-incoming-idempotency-contract.test.ts

@@ -29,4 +29,103 @@ describe('Bot incoming idempotency contract (PR-BOT-INCOMING-IDEMPOTENCY-0)', ()
     assert.match(main, /const BOT_RECENT_SOURCE_EVENT_LIMIT = 1_000;/, 'dedupe set must stay bounded');
     assert.match(main, /while \(botRecentSourceEventKeys\.size > BOT_RECENT_SOURCE_EVENT_LIMIT\)/, 'dedupe set must evict old entries');
   });
+
+  it('expires source-event dedupe entries by TTL before checking duplicates', async () => {
+    const main = await readRepo('apps/desktop/src/main/main.ts');
+    const remember = main.match(/function rememberBotSourceEvent\([^)]*\): boolean \{[\s\S]*?\n\}/);
+    const prune = main.match(/function pruneExpiredBotSourceEvents\([^)]*\): void \{[\s\S]*?\n\}/);
+
+    assert.ok(remember, 'rememberBotSourceEvent must exist');
+    assert.ok(prune, 'pruneExpiredBotSourceEvents must exist');
+    assert.match(main, /const BOT_RECENT_SOURCE_EVENT_TTL_MS = 60 \* 60 \* 1_000;/);
+    assert.ok(
+      remember![0].indexOf('pruneExpiredBotSourceEvents(now)') < remember![0].indexOf('botRecentSourceEventKeys.has(key)'),
+      'expired dedupe entries must be pruned before duplicate lookup',
+    );
+    assert.match(
+      prune![0],
+      /if \(now - seenAt <= BOT_RECENT_SOURCE_EVENT_TTL_MS\) break;[\s\S]*botRecentSourceEventKeys\.delete\(key\)/,
+      'dedupe TTL cleanup should delete expired oldest entries and keep the hard cap as secondary protection',
+    );
+  });
+
+  it('rate-limits and session-caps bot turns before create/send side effects', async () => {
+    const main = await readRepo('apps/desktop/src/main/main.ts');
+    const processBlock = main.match(/async function processBotIncomingMessage\([^)]*\): Promise<void> \{[\s\S]*?\n\}\n\nasync function collectBotReply/);
+
+    assert.ok(processBlock, 'processBotIncomingMessage block must exist');
+    assert.match(main, /const BOT_CONVERSATION_SESSION_LIMIT = 500;/);
+    assert.match(main, /const BOT_CONVERSATION_RATE_BURST = 8;/);
+    assert.match(main, /const BOT_CONVERSATION_RATE_REFILL_MS = 5_000;/);
+    assert.match(main, /const BOT_CONVERSATION_RATE_BUCKET_TTL_MS = 60 \* 60 \* 1_000;/);
+    assert.match(main, /const BOT_CONVERSATION_RATE_BUCKET_LIMIT = 1_000;/);
+    const consumeToken = main.match(/function consumeBotConversationToken\([^]*?\n\}\n\nasync function sendTransientBotNotice/);
+    const pruneBuckets = main.match(/function pruneExpiredBotConversationRateBuckets\([^)]*\): void \{[\s\S]*?\n\}/);
+    assert.ok(consumeToken, 'consumeBotConversationToken helper must exist');
+    assert.ok(pruneBuckets, 'rate bucket TTL pruning helper must exist');
+    assert.match(consumeToken![0], /BOT_CONVERSATION_RATE_BURST/);
+    assert.match(consumeToken![0], /BOT_CONVERSATION_RATE_REFILL_MS/);
+    assert.match(consumeToken![0], /pruneExpiredBotConversationRateBuckets\(now\)/);
+    assert.match(consumeToken![0], /bucket\.tokens -= 1/);
+    assert.match(consumeToken![0], /while \(botConversationRateBuckets\.size > BOT_CONVERSATION_RATE_BUCKET_LIMIT\)/);
+    assert.match(
+      pruneBuckets![0],
+      /now - bucket\.updatedAt > BOT_CONVERSATION_RATE_BUCKET_TTL_MS[\s\S]*botConversationRateBuckets\.delete\(key\)/,
+      'stale rate buckets must be pruned independently of the session cap',
+    );
+    assert.match(main, /async function sendTransientBotNotice[\s\S]*ephemeralTtlMs: ttlMs/);
+
+    const block = processBlock![0];
+    const newSessionBranch = block.match(/if \(!sessionId\) \{[\s\S]*?const ready = await getReadyConnection/);
+    assert.ok(newSessionBranch, 'new bot conversation branch must exist');
+    assert.ok(
+      block.indexOf('consumeBotConversationToken(conversationKey)') < block.indexOf('runtime.createSession'),
+      'rate limit must run before creating a bot session',
+    );
+    assert.ok(
+      block.indexOf('consumeBotConversationToken(conversationKey)') < block.indexOf('runtime.sendMessage'),
+      'rate limit must run before runtime.sendMessage',
+    );
+    assert.ok(
+      block.indexOf('botConversationSessions.size >= BOT_CONVERSATION_SESSION_LIMIT') < block.indexOf('runtime.createSession'),
+      'new bot binding cap must run before creating the 501st session',
+    );
+    assert.ok(
+      newSessionBranch![0].indexOf('botConversationSessions.size >= BOT_CONVERSATION_SESSION_LIMIT')
+        < newSessionBranch![0].indexOf('consumeBotConversationToken(conversationKey)'),
+      'new conversations rejected by a full binding cap must not allocate rate buckets',
+    );
+    assert.match(
+      block,
+      /if \(!consumeBotConversationToken\(conversationKey\)\) \{[\s\S]*sendTransientBotNotice[\s\S]*return;/,
+      'rate-limited turns must send at most a transient notice and return',
+    );
+    assert.match(
+      block,
+      /if \(botConversationSessions\.size >= BOT_CONVERSATION_SESSION_LIMIT\) \{[\s\S]*sendTransientBotNotice[\s\S]*return;/,
+      'session-cap rejections must send at most a transient notice and return',
+    );
+  });
+
+  it('forces existing bot-bound sessions back to explore before send or refuses the turn', async () => {
+    const main = await readRepo('apps/desktop/src/main/main.ts');
+    const processBlock = main.match(/async function processBotIncomingMessage\([^)]*\): Promise<void> \{[\s\S]*?\n\}\n\nasync function collectBotReply/);
+    const guard = main.match(/async function ensureBotSessionExploreMode\([^)]*\): Promise<boolean> \{[\s\S]*?\n\}/);
+
+    assert.ok(processBlock, 'processBotIncomingMessage block must exist');
+    assert.ok(guard, 'ensureBotSessionExploreMode guard must exist');
+    const block = processBlock![0];
+    assert.ok(
+      block.indexOf('ensureBotSessionExploreMode(sessionId, message, SYSTEM_NOTICE_TTL_MS)') < block.indexOf('ensureSessionCanSend(sessionId)'),
+      'existing bot sessions must be permission-checked before generic send readiness',
+    );
+    assert.ok(
+      block.indexOf('ensureBotSessionExploreMode(sessionId, message, SYSTEM_NOTICE_TTL_MS)') < block.indexOf('runtime.sendMessage'),
+      'existing bot sessions must be forced/refused before runtime.sendMessage',
+    );
+    assert.match(guard![0], /const header = await store\.readHeader\(sessionId\)/);
+    assert.match(guard![0], /if \(header\.permissionMode === 'explore'\) return true;/);
+    assert.match(guard![0], /await runtime\.updateSession\(sessionId, \{ permissionMode: 'explore' \}\);[\s\S]*return true;/);
+    assert.match(guard![0], /catch \{[\s\S]*sendTransientBotNotice[\s\S]*return false;/);
+  });
 });

apps/desktop/src/main/__tests__/connection-credential-ipc-hardening-contract.test.ts

@@ -0,0 +1,167 @@
+import assert from 'node:assert/strict';
+import { readFileSync } from 'node:fs';
+import { join } from 'node:path';
+import { describe, it } from 'node:test';
+
+const mainSource = readFileSync(join(process.cwd(), 'src/main/main.ts'), 'utf8');
+
+function handlerBlock(channel: string): string {
+  const start = mainSource.indexOf(`ipcMain.handle('${channel}'`);
+  assert.notEqual(start, -1, `${channel} handler must exist`);
+  const next = mainSource.indexOf('ipcMain.handle(', start + 1);
+  return mainSource.slice(start, next === -1 ? undefined : next);
+}
+
+describe('connection credential IPC hardening contract', () => {
+  it('defines shared fail-closed slug and apiKey IPC validators', () => {
+    assert.match(mainSource, /const IPC_CONNECTION_SLUG_MAX_LENGTH = 64;/);
+    assert.match(mainSource, /const IPC_CONNECTION_SECRET_MAX_LENGTH = 4096;/);
+    assert.match(mainSource, /const IPC_CONTROL_CHARACTER_PATTERN = \/\[\\u0000-\\u001F\\u007F\]\//);
+    assert.match(mainSource, /const IPC_CONNECTION_SLUG_PATTERN = \/\^\[A-Za-z0-9\._-\]\+\$\//);
+    assert.match(
+      mainSource,
+      /function normalizeConnectionSlugForIpc\(value: unknown, label: string\): string \{[\s\S]*typeof value !== 'string'[\s\S]*value\.length === 0[\s\S]*value\.length > IPC_CONNECTION_SLUG_MAX_LENGTH[\s\S]*!IPC_CONNECTION_SLUG_PATTERN\.test\(value\) \|\| IPC_CONTROL_CHARACTER_PATTERN\.test\(value\)[\s\S]*return value;/,
+    );
+    assert.match(
+      mainSource,
+      /function normalizeConnectionApiKeyForIpc\(value: unknown, label: string\): string \{[\s\S]*typeof value !== 'string'[\s\S]*value\.length > IPC_CONNECTION_SECRET_MAX_LENGTH[\s\S]*IPC_CONTROL_CHARACTER_PATTERN\.test\(value\)[\s\S]*return value;/,
+    );
+  });
+
+  it('rejects unsafe slug classes while preserving representative existing valid slugs', () => {
+    assert.match(mainSource, /const IPC_CONNECTION_SLUG_MAX_LENGTH = 64;/);
+    assert.match(mainSource, /const IPC_CONNECTION_SLUG_PATTERN = \/\^\[A-Za-z0-9\._-\]\+\$\//);
+    assert.match(mainSource, /const IPC_CONTROL_CHARACTER_PATTERN = \/\[\\u0000-\\u001F\\u007F\]\//);
+    assert.match(mainSource, /value\.length === 0/);
+    assert.match(mainSource, /value\.length > IPC_CONNECTION_SLUG_MAX_LENGTH/);
+    assert.match(mainSource, /IPC_CONTROL_CHARACTER_PATTERN\.test\(value\)/);
+
+    const helper = mainSource.match(/function normalizeConnectionSlugForIpc\(value: unknown, label: string\): string \{[\s\S]*?\n\}/)?.[0] ?? '';
+    assert.ok(helper, 'normalizeConnectionSlugForIpc helper must exist');
+    assert.match(helper, /IPC_CONNECTION_SLUG_PATTERN\.test\(value\)/, 'slug validator must reject whitespace, path separators, and colon via an allowlist');
+    assert.match(
+      helper,
+      /(^|[^\w])\.\.(?!\.)|includes\('\\.\\.'\)|includes\("\.\."\)|traversal|path traversal/i,
+      'slug validator must explicitly reject traversal-looking ".." slugs even though dots are otherwise allowed for compatibility',
+    );
+    for (const validSlug of ['claude-subscription', 'codex-subscription', 'zai-coding-plan', 'env-openai']) {
+      assert.doesNotMatch(validSlug, /[\u0000-\u001F\u007F/:\\]/, `${validSlug} should stay representative-valid`);
+      assert.ok(validSlug.length <= 64, `${validSlug} should stay under the IPC slug cap`);
+    }
+  });
+
+  it('normalizes create slug and apiKey before store or credential writes', () => {
+    const helper = mainSource.match(/function normalizeCreateConnectionInput\(input: CreateConnectionInput\): CreateConnectionInput \{[\s\S]*?\n\}/)?.[0] ?? '';
+    assert.match(helper, /normalizeConnectionApiKeyForIpc\(input\.apiKey, 'apiKey'\)/);
+    assert.match(helper, /normalizeConnectionSlugForIpc\(input\.slug, 'connection slug'\)/);
+
+    const handler = handlerBlock('connections:create');
+    assert.match(handler, /const normalizedInput = normalizeCreateConnectionInput\(input\);[\s\S]*connectionStore\.create\(normalizedInput\);/);
+    assert.match(handler, /credentialStore\.setSecret\(connection\.slug, 'api_key', normalizedInput\.apiKey\)/);
+  });
+
+  it('caps and validates create apiKey before persistence without echoing the secret value', () => {
+    const apiKeyHelper = mainSource.match(/function normalizeConnectionApiKeyForIpc\(value: unknown, label: string\): string \{[\s\S]*?\n\}/)?.[0] ?? '';
+    const createHelper = mainSource.match(/function normalizeCreateConnectionInput\(input: CreateConnectionInput\): CreateConnectionInput \{[\s\S]*?\n\}/)?.[0] ?? '';
+    const handler = handlerBlock('connections:create');
+
+    assert.match(apiKeyHelper, /value\.length > IPC_CONNECTION_SECRET_MAX_LENGTH/);
+    assert.match(apiKeyHelper, /IPC_CONTROL_CHARACTER_PATTERN\.test\(value\)/);
+    assert.doesNotMatch(apiKeyHelper, /String\(value\)|\$\{value\}|value\}/, 'apiKey validation errors must not echo cleartext secret values');
+    assert.ok(
+      createHelper.indexOf('normalizeConnectionApiKeyForIpc(input.apiKey, \'apiKey\')')
+        < createHelper.indexOf('normalizeConnectionSlugForIpc(input.slug, \'connection slug\')'),
+      'create must validate apiKey before constructing normalized input for persistence',
+    );
+    assert.ok(
+      handler.indexOf('normalizeCreateConnectionInput(input)')
+        < handler.indexOf('connectionStore.create(normalizedInput)'),
+      'create must normalize and cap apiKey before connection persistence',
+    );
+    assert.ok(
+      handler.indexOf('normalizeCreateConnectionInput(input)')
+        < handler.indexOf('credentialStore.setSecret'),
+      'create must normalize and cap apiKey before credential persistence',
+    );
+  });
+
+  it('normalizes update slug and apiKey before side-effecting update or credential writes', () => {
+    const helper = mainSource.match(/async function normalizeUpdateConnectionInput\([\s\S]*?\n\}/)?.[0] ?? '';
+    assert.match(helper, /const normalizedPatch = normalizeConnectionPatchSecretsForIpc\(patch\);[\s\S]*const existing = await connectionStore\.get\(slug\);/);
+
+    const handler = handlerBlock('connections:update');
+    assert.match(handler, /slug = normalizeConnectionSlugForIpc\(slug, 'connection slug'\);/);
+    assert.match(handler, /const normalizedPatch = await normalizeUpdateConnectionInput\(slug, patch\);/);
+    assert.match(handler, /connectionStore\.update\(slug, normalizedPatch\)/);
+    assert.match(handler, /credentialStore\.(?:setSecret|deleteSecret)\(slug, 'api_key'/);
+  });
+
+  it('validates renderer-controlled slug handlers before store, credential, or provider work', () => {
+    for (const channel of [
+      'connections:setDefault',
+      'connections:delete',
+      'connections:test',
+      'connections:fetchModels',
+    ]) {
+      const handler = handlerBlock(channel);
+      const normalizeAt = handler.indexOf('normalizeConnectionSlugForIpc');
+      assert.notEqual(normalizeAt, -1, `${channel} must normalize slug`);
+      for (const sideEffect of [
+        'connectionStore.',
+        'credentialStore.',
+        'resolveConnectionSecret(',
+        'testConnection(',
+        'fetchProviderModels(',
+      ]) {
+        const sideEffectAt = handler.indexOf(sideEffect);
+        if (sideEffectAt !== -1) {
+          assert.ok(normalizeAt < sideEffectAt, `${channel} must normalize before ${sideEffect}`);
+        }
+      }
+    }
+
+    const hasSecret = handlerBlock('connections:hasSecret');
+    assert.match(
+      hasSecret,
+      /slug = normalizeConnectionSlugForIpc\(slug, 'connection slug'\);[\s\S]*return Boolean\(await resolveConnectionSecret\(slug\)\);/,
+    );
+  });
+
+  it('preserves update apiKey clearing semantics while rejecting invalid provided strings', () => {
+    const helper = mainSource.match(/function normalizeConnectionPatchSecretsForIpc\(patch: UpdateConnectionInput\): UpdateConnectionInput \{[\s\S]*?\n\}/)?.[0] ?? '';
+    assert.match(helper, /if \(!Object\.prototype\.hasOwnProperty\.call\(patch, 'apiKey'\)\) return patch;/);
+    assert.match(helper, /if \(patch\.apiKey === undefined\) return patch;/);
+    assert.match(helper, /apiKey: normalizeConnectionApiKeyForIpc\(patch\.apiKey, 'apiKey'\)/);
+
+    const handler = handlerBlock('connections:update');
+    assert.match(handler, /if \(normalizedPatch\.apiKey !== undefined\) \{[\s\S]*if \(normalizedPatch\.apiKey\) await credentialStore\.setSecret\(slug, 'api_key', normalizedPatch\.apiKey\);[\s\S]*else await credentialStore\.deleteSecret\(slug, 'api_key'\);/);
+  });
+
+  it('keeps OAuth baseUrl normalization and provider-aware secret resolution wired', () => {
+    assert.match(
+      mainSource,
+      /function normalizeCreateConnectionInput\(input: CreateConnectionInput\): CreateConnectionInput \{[\s\S]*defaults\.authKind === 'oauth_token'[\s\S]*baseUrl: defaults\.baseUrl/,
+      'create must continue forcing canonical OAuth provider baseUrl',
+    );
+    assert.match(
+      mainSource,
+      /async function normalizeUpdateConnectionInput\([\s\S]*PROVIDER_DEFAULTS\[providerType\]\.authKind === 'oauth_token'[\s\S]*baseUrl: PROVIDER_DEFAULTS\[providerType\]\.baseUrl/,
+      'update must continue forcing canonical OAuth provider baseUrl',
+    );
+    assert.match(mainSource, /connections:test[\s\S]*const apiKey = await resolveConnectionSecret\(slug\)/);
+    assert.match(mainSource, /connections:fetchModels[\s\S]*const apiKey = await resolveConnectionSecret\(slug\)/);
+    assert.match(mainSource, /connections:hasSecret[\s\S]*return Boolean\(await resolveConnectionSecret\(slug\)\)/);
+  });
+
+  it('does not echo cleartext API keys in thrown errors or IPC return values', () => {
+    const validatorRegion = mainSource.match(/function normalizeConnectionApiKeyForIpc[\s\S]*?function normalizeCreateConnectionInput/)?.[0] ?? '';
+    const createHandler = handlerBlock('connections:create');
+    const updateHandler = handlerBlock('connections:update');
+
+    for (const source of [validatorRegion, createHandler, updateHandler]) {
+      assert.doesNotMatch(source, /errorMessage:\s*[^,\n]*apiKey/);
+      assert.doesNotMatch(source, /throw new Error\([^)]*apiKey[^)]*\$\{[^}]*value/);
+      assert.doesNotMatch(source, /return \{[\s\S]*apiKey[\s\S]*\}/);
+    }
+  });
+});