Skip to content

fix(security): log 3 crypto-relevant silent catches - #509

Merged
KooshaPari merged 1 commit into
agent/migration-version-collision-fixfrom
fix/crypto-catches-clean-20260806
Aug 7, 2026
Merged

KooshaPari merged 1 commit into
agent/migration-version-collision-fixfrom
fix/crypto-catches-clean-20260806

Conversation

@KooshaPari

@KooshaPari KooshaPari commented Aug 7, 2026 •

Copy link
Copy Markdown
Owner

User description

fix(security): log 3 crypto-relevant silent catches

This branch has been surgically rebased onto origin/agent/migration-version-collision-fix to remove accidental contamination from PR #507's branch base.

In all three cases, the empty-string return collapses HMAC/HMAC-SHA256
inputs into a constant-key value, so security-relevant operations on the
fallback path produce identical tokens regardless of input.

Fixes:

  1. src/lib/machineToken.ts:44 — getMachineTokenSync catch:
    log.error + return "" (matches PR fix(governance): broken featureFlags imports + 3 silent fail-opens + extras test #507 module-load catch pattern)
  2. src/lib/machineToken.ts:62 — getLegacyCliTokenSync catch:
    log.error + return "" (same pattern)
  3. src/lib/db/encryption.ts:87-95 — getLegacyDynamicKey catch:
    added pino logger (createLogger("db:encryption")) + log.error
    instead of returning null silently

Incidental: this branch also adds the createLogger import + log constant
that the 2 runtime catches depend on. PR #507's machineToken.ts hunk
already adds these, so when #507 merges there will be a trivial conflict
on those lines (resolve by taking this branch's version).

Out of scope (separate work):

  • encryption.ts:144-148 (encrypt() catch falls back to plaintext when
    crypto fails — needs design discussion; spec at plans/encryption-failclosed-spec.md)
  • src/lib/db/.ts console. migration to pino (large, separate PR)

Verification:


CodeAnt-AI Description

Log security-relevant crypto fallback failures

What Changed

  • Records errors when machine-token generation fails and returns an empty token
  • Records when the machine identity module is unavailable and token generation falls back to an empty salt
  • Records failures while deriving legacy encryption keys so stalled token migrations are visible

Impact

✅ Visible security fallback failures
✅ Easier diagnosis of constant-key token generation
✅ Clearer legacy encryption migration failures

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

…onical base)

This branch has been surgically rebased onto origin/agent/migration-version-collision-fix
to remove accidental contamination from PR #507's branch base.

In all three cases, the empty-string return collapses HMAC/HMAC-SHA256
inputs into a constant-key value, so security-relevant operations on the
fallback path produce identical tokens regardless of input.

Fixes:
1. src/lib/machineToken.ts:44 - getMachineTokenSync catch:
   log.error + return ""
2. src/lib/machineToken.ts:62 - getLegacyCliTokenSync catch:
   log.error + return ""
3. src/lib/db/encryption.ts:87-95 - getLegacyDynamicKey catch:
   added pino logger (createLogger("db:encryption")) + log.error
   instead of returning null silently
4. (incidental) src/lib/machineToken.ts module-load catch: also
   gains log.error so the new runtime catches have a 'log' constant
   to reference. This subsumes PR #507's machineToken.ts hunk.

NOTE: PR #507 will need its machineToken.ts hunk resolved when it merges
(this branch already provides the 'log' logger constant it tries to add).

Verification:
- TSC: 8 unchanged (pre-existing quota keystore errors, PR #505 territory)
- Targeted machineToken + encryption unit tests pass
- All success-path behaviors preserved
Copilot AI lite review requested due to automatic review settings August 7, 2026 02:47
@codeant-ai

codeant-ai Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Reviewed your PR 8941194 Aug 07, 2026 · 02:47 02:51

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@codeant-ai

codeant-ai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4124c9bb-bb56-4e56-b26b-6c76d02600f1

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Note

.coderabbit.yaml has unrecognized properties

CodeRabbit is using all valid settings from your configuration. Unrecognized properties (listed below) have been ignored and may indicate typos or deprecated fields that can be removed.

⚠️ Parsing warnings (1)
Validation error: Unrecognized key: "review"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codeant-ai codeant-ai Bot added the size:M This PR changes 30-99 lines, ignoring generated files label Aug 7, 2026
@mergify mergify Bot added the typescript label Aug 7, 2026
@mergify

mergify Bot commented Aug 7, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

Comment thread src/lib/machineToken.ts
Comment on lines +48 to 53
} catch (err) {
log.error(
{ err },
"machineToken.getMachineTokenSync: deriveToken failed — returning empty string (security-relevant)"
);
return "";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: When machineIdSync or HMAC derivation continues to fail, this catch logs synchronously on every authentication request. Both CLI authentication callers invoke this function for each request, so a persistent machine-ID failure can generate unbounded structured file writes and add request latency. Rate-limit or record this fallback after the first occurrence, similar to the module-load fallback. [performance]

Severity Level: Major ⚠️
- ⚠️ Loopback management authentication logs every failed token attempt.
- ⚠️ Repeated failures increase Pino queue and application-log volume.
- ⚠️ Authentication requests incur repeated token-derivation overhead.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** src/lib/machineToken.ts
**Line:** 48:53
**Comment:**
	*Performance: When `machineIdSync` or HMAC derivation continues to fail, this catch logs synchronously on every authentication request. Both CLI authentication callers invoke this function for each request, so a persistent machine-ID failure can generate unbounded structured file writes and add request latency. Rate-limit or record this fallback after the first occurrence, similar to the module-load fallback.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

@kilo-code-bot

kilo-code-bot Bot commented Aug 7, 2026 •

Copy link
Copy Markdown

Code Review Summary

Status: 1 Suggestion | Recommendation: Merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
src/lib/machineToken.ts 48-53 getMachineTokenSync catch logs on every auth request without the fallbackLogged gate that the module-load catch uses. Same applies to getLegacyCliTokenSync (line 65-70). Already flagged by CodeAnt (line 53). See call sites at cliTokenAuth.ts:82 and management.ts:75. Recommend applying the same fallbackLogged-style deduplication to the runtime catches, or logging only on state transition. Tradeoff: unconditional logging gives better auditability for security-relevant crypto failures, so this is optional.
Files Reviewed (2 files)
  • src/lib/db/encryption.ts - 1 change (logging added to getLegacyDynamicKey catch)
  • src/lib/machineToken.ts - 3 changes (logging added to module-load, getMachineTokenSync, and getLegacyCliTokenSync catches)

Notes

  • The { err } logging pattern matches existing convention (src/mitm/manager.ts uses it 12×; src/shared/utils/logger.ts:8 documents it).
  • createLogger import verified against @/shared/utils/logger.
  • Existing tests (tests/unit/lib/machineToken.test.ts, tests/unit/db-encryption.test.ts) cover success paths only — the new catch logging doesn't alter return behavior, so tests should still pass.
  • encryption.ts still uses console.error in getStaticKey/encrypt/decrypt — acknowledged as out-of-scope in the PR description.

Reviewed by laguna-s-2.1:free · Input: 315.3K · Output: 25K · Cached: 378.1K

@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

@KooshaPari
KooshaPari merged commit fd818aa into agent/migration-version-collision-fix Aug 7, 2026
12 of 13 checks passed
@KooshaPari
KooshaPari deleted the fix/crypto-catches-clean-20260806 branch August 7, 2026 06:12
KooshaPari added a commit that referenced this pull request Aug 7, 2026
Completes the audit-driven governance work that PR #509 started. Eight
independent fixes:

Security fixes (audit findings F5, F6, F9, F10):

1. src/lib/cloudSync.ts:47-56 — HMAC verification fail-open when
   CLOUD_SYNC_SECRET is unset. Now returns false (fail-closed) when a
   sigHeader is present but cannot be verified, instead of accepting
   any response. Legacy unverified mode (no sigHeader) is preserved.

2. src/lib/oauth/providers/antigravity.ts:101 — fetch() of userInfo
   during OAuth was swallowed silently, causing downstream code to use
   default projectId/tierId. Now logs warn with structured context.

3. open-sse/services/autoRefreshDaemon.ts:77,80 — periodic credential
   refresh errors were silently swallowed, allowing expired tokens to
   persist undetected. Now logs error per cycle.

4. src/lib/vscode/serviceTierVariants.ts:184 — request body parse
   failures during tier-rewrite were silently no-op'd. Now logs warn.

Refactor:

5. src/lib/db/encryption.ts — migrated ~7 console.* calls to pino
   logger (encryptionLog). Following the PR #509 pattern of
   createLogger("db:encryption"). The catch fallback behavior is
   preserved exactly; only logging changes.

Out of scope:
- encryption.ts:144-148 plaintext fallback (separate spec at
  plans/encryption-failclosed-spec.md, deferred for design discussion)
- src/lib/db/*.ts console.* migration for other files (separate PR)

Co-authored-by: KooshaPari <koosha@example.com>
KooshaPari added a commit that referenced this pull request Aug 7, 2026
Follows the PR #506/#507/#509/#510 pattern of replacing console.*
with createLogger("db:<subsystem>") to provide structured logging
across the SQLite persistence layer.

Files modified (~155 callsites across 17 files):
- src/lib/db/adapters/driverFactory.ts
- src/lib/db/adapters/sqljsAdapter.ts
- src/lib/db/apiKeys.ts
- src/lib/db/backup.ts
- src/lib/db/cleanup.ts (~30 sites)
- src/lib/db/core.ts (~30 sites)
- src/lib/db/migrationRunner.ts (~22 sites, removed test-suppressing local console wrapper)
- src/lib/db/models.ts
- src/lib/db/optimizationSettings.ts
- src/lib/db/providers.ts
- src/lib/db/quotaPools.ts
- src/lib/db/quotaSnapshots.ts
- src/lib/db/schemaColumns.ts (~35 sites)
- src/lib/db/sessionAccountAffinity.ts
- src/lib/db/settings.ts
- src/lib/db/settings/cacheMetrics.ts
- src/lib/db/stateReset.ts

Per AGENTS.md guidance, never log SQLite encryption keys or raw secrets;
all logger calls redact sensitive material.

Behavior preservation:
- All success-path behavior unchanged
- Only the logging mechanism changes
- Targeted tests pass

One console.error preserved in core.ts:migrateFromJson because
tests/unit/db-core-migration.test.ts overrides console.error to detect
the failure; both console.error and log.error are emitted so the test
passes and pino is the canonical sink.

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
KooshaPari added a commit that referenced this pull request Aug 7, 2026
* chore(governance): rebase mergify config request-review fixes onto main

* fix(desktop): target fork-owned Electron releases

* ci: align workflows with selected action policy

* fix(governance): replace silent try/catch with explicit log.error in 5 modules (#506)

Builds on PR #505 (quota keystore type-drift fix). The audit of that PR
revealed 5 additional silent fail-open catch patterns across `src/` and
`open-sse/` that hide the same class of bug: a TypeScript compile error
or missing module is silently swallowed at runtime, falling back to a
default with no operator-visible signal.

This commit replaces those silent catches with explicit `log.error` calls
that surface the actual error to monitoring. Fallback behavior is
preserved (each fallback is intentional, but it must be LOUD).

Changes:

1. `src/lib/quota/storeFactory.ts:67-77` — `readDbSettings` now logs the
   actual error when `getSettings()` fails or `@/lib/db/settings` import
   fails. Same root cause as the previously-fixed Keyv/Redis catches.

2. `src/lib/quota/storeFactory.ts:138-147` — Redis driver catch upgraded
   from `log.warn` to `log.error`. Includes the configured Redis URL
   with the password segment redacted (`:***@`).

3. `src/lib/resilience/anomalyHook.ts` — `getProviderManagerRegistry`
   now logs the actual error when `@/engine/providers` fails to load.
   Empty Map fallback retained (resilience must continue running), but
   the failure is now visible in monitoring.

4. `open-sse/services/tierResolver.ts` — `setTierConfig` now logs the
   actual error when `../../src/lib/db/tierConfig` fails to load.
   `DEFAULT_TIER_CONFIG` fallback retained (pricing must continue), but
   the failure is now visible.

5. `open-sse/config/credentialLoader.ts` — `resolveCredentialsPath` now
   uses `log.error` (pino) instead of `console.warn`. Includes both the
   original error and the fallback path. Security-sensitive path; must
   keep working, but the failure must be loud.

6. `.gitignore` — exclude `.agileplus/` and `agileplus-*.db*` (local
   AgilePlus DB state, regenerated from `.md` specs via `agileplus
   specify`). The DB contains transient per-machine state and shouldn't
   be in version control.

Verification:
- TSC: 0 NEW errors (8 pre-existing quota keystore errors remain, those
  are what PR #505 fixes; this PR is independent of PR #505)
- Vitest quota suite: 18/18 pass (6 keyv + 4 contract + 8 e2e)
- Node:test factory: 6/6 pass
- Resilience tests: 61 pass / 1 pre-existing flake
  (resilience-provider-cooldown-api-3556.test.ts: "rejects providerCooldown
  max below min" — verified pre-existing by reverting only anomalyHook.ts
  and reproducing the same failure)

Out of scope (filed as separate bugs):
- `src/lib/resilience/anomalyHook.ts:13` imports `isFeatureFlagEnabled`
  from `@/lib/featureFlags`, but the module lives at
  `src/lib/db/featureFlags.ts`. The file is currently unloadable from
  tests; this PR doesn't touch the import because that's a separate bug.
- The Resilience subagent identified but did not fix the
  `tsconfig.typecheck-core.json` doesn't include `src/lib/resilience/`
  files — separate follow-up.

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(governance): broken featureFlags imports + 3 silent fail-opens + extras test (#507)

Continuation of the governance-debt cleanup started in PRs #505 and #506.
Six independent fixes, each addressing a class of silent-failure pattern
that the audits surfaced.

Changes:

1. **`src/lib/resilience/anomalyHook.ts:12`** — Broken import path.
   `isFeatureFlagEnabled` was imported from `@/lib/featureFlags`, but
   the module lives at `@/shared/utils/featureFlags`. This bug was
   masked because `tsconfig.typecheck-core.json` does not include
   `src/lib/resilience/` and no test loads the module successfully.
   After the fix, the module loads and exports the expected surface.

2. **`src/server-init.ts:128`** — Same broken import path. The
   surrounding try/catch at lines 130-139 silently swallowed the
   import failure. Fix: correct the path AND upgrade the catch log
   from `warn` to `error` (matches PR #506 pattern).

3. **`src/lib/versionManager/processManager.ts:155`** — `getProcessInfo`
   catch returned `{pid, alive: true}` after `ps`/readFile failures,
   which lies when the process is actually gone. Fix: catch now
   logs the error and returns `{pid, alive: false}` (honest about
   not being able to read process state).

4. **`src/server/ws/liveServer.ts:479`** — `loadAuthModule().catch(() => {})`
   silently swallowed initial auth module load failures, allowing the
   WS server to come up without auth configured. Fix: catch now logs
   `log.error`; fallback behavior preserved per the existing comment
   ("handler retries the import lazily").

5. **`src/lib/machineToken.ts:1-10`** — Crypto-relevant: empty catch
   around `require("node-machine-id")` silently fell back to
   `() => ""`, which collapses HMAC inputs to a constant. Fix: catch
   now logs `log.error` with security-context message, gated by a
   `fallbackLogged` flag so the log fires only once per process
   (avoiding log spam from any downstream reload).

6. **`tests/unit/quota/keyvQuotaStoreExtras.test.ts`** (new) — Closes
   spec §8.2 reachability test gap. 5 sanity tests for the
   `recordPlanUsage` / `upsertProviderPlan` / `listProviderPlans` /
   `setPools` / `getPool` surface on KeyvQuotaStore. Note: this
   branch is based on `origin/agent/migration-version-collision-fix`
   (pre-PR-#505), so the methods live directly on KeyvQuotaStore.
   When PR #505 lands, update the test to import from
   `keyvQuotaStoreExtras.ts` instead.

Verification:
- TSC error count: 8 unchanged (all pre-existing quota keystore
  errors that PR #505 fixes; this PR is independent of #505)
- Vitest extras test: 5/5 pass
- Node:test combined (processManager + machineToken + WS): 47/47 pass
- All success-path behavior preserved; only catch/fallback paths now log

Out of scope (separate PRs):
- Promote Keyv to "embedded default" driver (spec §10)
- Pre-existing e2e failure at `tests/e2e/quota-store.e2e.ts:117`
  (poolUsageWithDimensions shape mismatch — fixed in PR #505)

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(security): log 3 crypto-relevant silent catches (rebased onto canonical base) (#509)

This branch has been surgically rebased onto origin/agent/migration-version-collision-fix
to remove accidental contamination from PR #507's branch base.

In all three cases, the empty-string return collapses HMAC/HMAC-SHA256
inputs into a constant-key value, so security-relevant operations on the
fallback path produce identical tokens regardless of input.

Fixes:
1. src/lib/machineToken.ts:44 - getMachineTokenSync catch:
   log.error + return ""
2. src/lib/machineToken.ts:62 - getLegacyCliTokenSync catch:
   log.error + return ""
3. src/lib/db/encryption.ts:87-95 - getLegacyDynamicKey catch:
   added pino logger (createLogger("db:encryption")) + log.error
   instead of returning null silently
4. (incidental) src/lib/machineToken.ts module-load catch: also
   gains log.error so the new runtime catches have a 'log' constant
   to reference. This subsumes PR #507's machineToken.ts hunk.

NOTE: PR #507 will need its machineToken.ts hunk resolved when it merges
(this branch already provides the 'log' logger constant it tries to add).

Verification:
- TSC: 8 unchanged (pre-existing quota keystore errors, PR #505 territory)
- Targeted machineToken + encryption unit tests pass
- All success-path behaviors preserved

Co-authored-by: KooshaPari <koosha@example.com>

* fix(security): 4 audit findings + migrate encryption.ts to pino (#510)

Completes the audit-driven governance work that PR #509 started. Eight
independent fixes:

Security fixes (audit findings F5, F6, F9, F10):

1. src/lib/cloudSync.ts:47-56 — HMAC verification fail-open when
   CLOUD_SYNC_SECRET is unset. Now returns false (fail-closed) when a
   sigHeader is present but cannot be verified, instead of accepting
   any response. Legacy unverified mode (no sigHeader) is preserved.

2. src/lib/oauth/providers/antigravity.ts:101 — fetch() of userInfo
   during OAuth was swallowed silently, causing downstream code to use
   default projectId/tierId. Now logs warn with structured context.

3. open-sse/services/autoRefreshDaemon.ts:77,80 — periodic credential
   refresh errors were silently swallowed, allowing expired tokens to
   persist undetected. Now logs error per cycle.

4. src/lib/vscode/serviceTierVariants.ts:184 — request body parse
   failures during tier-rewrite were silently no-op'd. Now logs warn.

Refactor:

5. src/lib/db/encryption.ts — migrated ~7 console.* calls to pino
   logger (encryptionLog). Following the PR #509 pattern of
   createLogger("db:encryption"). The catch fallback behavior is
   preserved exactly; only logging changes.

Out of scope:
- encryption.ts:144-148 plaintext fallback (separate spec at
  plans/encryption-failclosed-spec.md, deferred for design discussion)
- src/lib/db/*.ts console.* migration for other files (separate PR)

Co-authored-by: KooshaPari <koosha@example.com>

* fix(governance): log empty catches in binaryManager + db/adapters (#512)

22 silent fail-open sites in the version manager + SQLite adapter layer
were just swallows. Per the audit:

- src/lib/versionManager/binaryManager.ts (6 catches): symlink/rollback/
  remove errors were silent; filesystem permission bugs were invisible
- src/lib/db/adapters/sqljsAdapter.ts (6 catches): save/close errors
  were silent; DB corruption during shutdown was undetectable
- src/lib/db/adapters/betterSqliteAdapter.ts (1 catch): same pattern
- src/lib/db/adapters/nodeSqliteAdapter.ts (2 catches): same pattern
- src/lib/db/adapters/nodeSqliteShared.ts (7 catches): same pattern

All 22 catches now log structured error context. Behavior unchanged -
only logging added. Per AGENTS.md, no encryption keys or raw secrets
are logged.

TSC: 8 unchanged
Tests: existing pass (any new behavior is logging-only)

Co-authored-by: KooshaPari <koosha@example.com>

* ci: pin cross-platform Rust toolchain

* docs(plans): track 2 governance specs for future implementation (#511)

Two deferred-implementation specs are now tracked in version control so
the work product is preserved and discoverable.

1. plans/encryption-failclosed-spec.md (883 lines) — Hardening
   encryption.ts:144-148 (the silent plaintext fallback). Compares 3
   design options with detailed tradeoffs. Recommended: C+A
   (startup canary + runtime throw). Registered as AgilePlus
   feature 'encryption-failclosed'.

2. plans/keyv-as-embedded-default-spec.md (698 lines) — Promote Keyv
   from optional driver to embedded default for fresh installs.
   Includes backwards-compat plan, config migration, and rollout
   sequence. Registered as AgilePlus feature 'keyv-as-embedded-default'.

These specs intentionally do NOT include code changes — they are
design-only and gate on answering the open questions before
implementation. See spec section 9 for each spec's open questions.

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* wip: auto-commit daemon 2026-08-06T09:18:52Z (#505)

Co-authored-by: Airlock Bot <airlock@phenoforge.local>

* refactor(db): migrate console.* to pino in src/lib/db/ (#522)

Follows the PR #506/#507/#509/#510 pattern of replacing console.*
with createLogger("db:<subsystem>") to provide structured logging
across the SQLite persistence layer.

Files modified (~155 callsites across 17 files):
- src/lib/db/adapters/driverFactory.ts
- src/lib/db/adapters/sqljsAdapter.ts
- src/lib/db/apiKeys.ts
- src/lib/db/backup.ts
- src/lib/db/cleanup.ts (~30 sites)
- src/lib/db/core.ts (~30 sites)
- src/lib/db/migrationRunner.ts (~22 sites, removed test-suppressing local console wrapper)
- src/lib/db/models.ts
- src/lib/db/optimizationSettings.ts
- src/lib/db/providers.ts
- src/lib/db/quotaPools.ts
- src/lib/db/quotaSnapshots.ts
- src/lib/db/schemaColumns.ts (~35 sites)
- src/lib/db/sessionAccountAffinity.ts
- src/lib/db/settings.ts
- src/lib/db/settings/cacheMetrics.ts
- src/lib/db/stateReset.ts

Per AGENTS.md guidance, never log SQLite encryption keys or raw secrets;
all logger calls redact sensitive material.

Behavior preservation:
- All success-path behavior unchanged
- Only the logging mechanism changes
- Targeted tests pass

One console.error preserved in core.ts:migrateFromJson because
tests/unit/db-core-migration.test.ts overrides console.error to detect
the failure; both console.error and log.error are emitted so the test
passes and pino is the canonical sink.

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* ci: use cache-pinned Trunk action

---------

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Airlock Bot <airlock@phenoforge.local>
KooshaPari pushed a commit that referenced this pull request Aug 7, 2026
Follows the PR #506/#507/#509/#510/#512/#518/#521/#522 pattern of replacing
console.* with createLogger("domain:subsystem") to provide structured
logging across the OmniRoute codebase.

PR #522 already migrated src/lib/db/*.ts (~155 callsites). This PR
migrates ~80 additional callsites across 37 files in:

- src/lib/resilience/* (normalize.ts, anomalyHook.ts)
- src/lib/oauth/* (connectionPersistence.ts)
- src/lib/vscode/* (tokenizedRequest.ts, dual-emit for test capture)
- src/lib/services/* (ringBuffer.ts, bootstrap.ts, embedWsProxy.ts, modelSync.ts)
- src/lib/sseTextTransform.ts, src/lib/dataPaths.ts, src/lib/initCloudSync.ts
- src/lib/events/eventBus.ts, src/lib/jobs/{budgetResetJob,reasoningCacheCleanupJob}.ts
- src/lib/cloudSync.ts, src/lib/localHealthCheck.ts
- src/lib/arenaEloSync.ts, src/lib/pricingSync.ts, src/lib/modelsDevSync.ts
- src/lib/tokenHealthCheck.ts, src/lib/gracefulShutdown.ts
- src/lib/apiBridgeServer.ts, src/lib/proxyLogger.ts
- src/lib/middleware/registry.ts, src/lib/quota/connectionRecovery.ts
- src/lib/credentialHealth/scheduler.ts
- src/middleware/promptInjectionGuard.ts
- src/server/ws/liveServer.ts (preserves [LiveWS] startup banner via log.info)
- src/sse/services/auth.ts, src/sse/services/streamState.ts
- open-sse/config/{constants,credentialLoader}.ts
- open-sse/services/{autoRefreshDaemon,quotaMonitor}.ts
- open-sse/mcp-server/audit.ts
- open-sse/utils/proxyFetch.ts
- open-sse/handlers/chatCore.ts (account fallback warnings)

User-facing startup banners and intentional CLI output are preserved:
- src/server/ws/liveServer.ts '[LiveWS] Dashboard WebSocket server listening'
  (now via log.info with structured host/port)
- src/lib/vscode/tokenizedRequest.ts '[VSCODE][SECURITY]' warning kept
  as console.warn alongside log.warn so tests/unit/vscode-token-in-url-warning.test.ts
  (which captures console.warn to verify once-per-process dedup) keeps passing —
  same pattern as PR #522's preservation in db/core.ts:migrateFromJson
- src/lib/oauth/utils/ui.ts (CLI formatting with picocolors) preserved as console
- src/mitm/* (CLI-driven tooling) preserved
- Next.js dashboard React components preserved (browser console, not server-side)

Behavior preservation:
- All success-path behavior unchanged
- Only the logging mechanism changes
- TSC baseline (typecheck-core.json) remains 0 errors
- Targeted tests pass: resilience-settings-normalize-split (9/9),
  resilience-settings-stream-recovery (9/9), resilience-settings-provider-breaker (9/9),
  oauth-refresh-error-resilience (12/12), vscode-tokenized-request (3/3),
  vscode-token-in-url-warning (4/4), services/embedWsProxy (30/30)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
KooshaPari added a commit that referenced this pull request Aug 7, 2026
…ollowup) (#526)

* chore(governance): rebase mergify config request-review fixes onto main

* fix(desktop): target fork-owned Electron releases

* ci: align workflows with selected action policy

* fix(governance): replace silent try/catch with explicit log.error in 5 modules (#506)

Builds on PR #505 (quota keystore type-drift fix). The audit of that PR
revealed 5 additional silent fail-open catch patterns across `src/` and
`open-sse/` that hide the same class of bug: a TypeScript compile error
or missing module is silently swallowed at runtime, falling back to a
default with no operator-visible signal.

This commit replaces those silent catches with explicit `log.error` calls
that surface the actual error to monitoring. Fallback behavior is
preserved (each fallback is intentional, but it must be LOUD).

Changes:

1. `src/lib/quota/storeFactory.ts:67-77` — `readDbSettings` now logs the
   actual error when `getSettings()` fails or `@/lib/db/settings` import
   fails. Same root cause as the previously-fixed Keyv/Redis catches.

2. `src/lib/quota/storeFactory.ts:138-147` — Redis driver catch upgraded
   from `log.warn` to `log.error`. Includes the configured Redis URL
   with the password segment redacted (`:***@`).

3. `src/lib/resilience/anomalyHook.ts` — `getProviderManagerRegistry`
   now logs the actual error when `@/engine/providers` fails to load.
   Empty Map fallback retained (resilience must continue running), but
   the failure is now visible in monitoring.

4. `open-sse/services/tierResolver.ts` — `setTierConfig` now logs the
   actual error when `../../src/lib/db/tierConfig` fails to load.
   `DEFAULT_TIER_CONFIG` fallback retained (pricing must continue), but
   the failure is now visible.

5. `open-sse/config/credentialLoader.ts` — `resolveCredentialsPath` now
   uses `log.error` (pino) instead of `console.warn`. Includes both the
   original error and the fallback path. Security-sensitive path; must
   keep working, but the failure must be loud.

6. `.gitignore` — exclude `.agileplus/` and `agileplus-*.db*` (local
   AgilePlus DB state, regenerated from `.md` specs via `agileplus
   specify`). The DB contains transient per-machine state and shouldn't
   be in version control.

Verification:
- TSC: 0 NEW errors (8 pre-existing quota keystore errors remain, those
  are what PR #505 fixes; this PR is independent of PR #505)
- Vitest quota suite: 18/18 pass (6 keyv + 4 contract + 8 e2e)
- Node:test factory: 6/6 pass
- Resilience tests: 61 pass / 1 pre-existing flake
  (resilience-provider-cooldown-api-3556.test.ts: "rejects providerCooldown
  max below min" — verified pre-existing by reverting only anomalyHook.ts
  and reproducing the same failure)

Out of scope (filed as separate bugs):
- `src/lib/resilience/anomalyHook.ts:13` imports `isFeatureFlagEnabled`
  from `@/lib/featureFlags`, but the module lives at
  `src/lib/db/featureFlags.ts`. The file is currently unloadable from
  tests; this PR doesn't touch the import because that's a separate bug.
- The Resilience subagent identified but did not fix the
  `tsconfig.typecheck-core.json` doesn't include `src/lib/resilience/`
  files — separate follow-up.

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(governance): broken featureFlags imports + 3 silent fail-opens + extras test (#507)

Continuation of the governance-debt cleanup started in PRs #505 and #506.
Six independent fixes, each addressing a class of silent-failure pattern
that the audits surfaced.

Changes:

1. **`src/lib/resilience/anomalyHook.ts:12`** — Broken import path.
   `isFeatureFlagEnabled` was imported from `@/lib/featureFlags`, but
   the module lives at `@/shared/utils/featureFlags`. This bug was
   masked because `tsconfig.typecheck-core.json` does not include
   `src/lib/resilience/` and no test loads the module successfully.
   After the fix, the module loads and exports the expected surface.

2. **`src/server-init.ts:128`** — Same broken import path. The
   surrounding try/catch at lines 130-139 silently swallowed the
   import failure. Fix: correct the path AND upgrade the catch log
   from `warn` to `error` (matches PR #506 pattern).

3. **`src/lib/versionManager/processManager.ts:155`** — `getProcessInfo`
   catch returned `{pid, alive: true}` after `ps`/readFile failures,
   which lies when the process is actually gone. Fix: catch now
   logs the error and returns `{pid, alive: false}` (honest about
   not being able to read process state).

4. **`src/server/ws/liveServer.ts:479`** — `loadAuthModule().catch(() => {})`
   silently swallowed initial auth module load failures, allowing the
   WS server to come up without auth configured. Fix: catch now logs
   `log.error`; fallback behavior preserved per the existing comment
   ("handler retries the import lazily").

5. **`src/lib/machineToken.ts:1-10`** — Crypto-relevant: empty catch
   around `require("node-machine-id")` silently fell back to
   `() => ""`, which collapses HMAC inputs to a constant. Fix: catch
   now logs `log.error` with security-context message, gated by a
   `fallbackLogged` flag so the log fires only once per process
   (avoiding log spam from any downstream reload).

6. **`tests/unit/quota/keyvQuotaStoreExtras.test.ts`** (new) — Closes
   spec §8.2 reachability test gap. 5 sanity tests for the
   `recordPlanUsage` / `upsertProviderPlan` / `listProviderPlans` /
   `setPools` / `getPool` surface on KeyvQuotaStore. Note: this
   branch is based on `origin/agent/migration-version-collision-fix`
   (pre-PR-#505), so the methods live directly on KeyvQuotaStore.
   When PR #505 lands, update the test to import from
   `keyvQuotaStoreExtras.ts` instead.

Verification:
- TSC error count: 8 unchanged (all pre-existing quota keystore
  errors that PR #505 fixes; this PR is independent of #505)
- Vitest extras test: 5/5 pass
- Node:test combined (processManager + machineToken + WS): 47/47 pass
- All success-path behavior preserved; only catch/fallback paths now log

Out of scope (separate PRs):
- Promote Keyv to "embedded default" driver (spec §10)
- Pre-existing e2e failure at `tests/e2e/quota-store.e2e.ts:117`
  (poolUsageWithDimensions shape mismatch — fixed in PR #505)

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(security): log 3 crypto-relevant silent catches (rebased onto canonical base) (#509)

This branch has been surgically rebased onto origin/agent/migration-version-collision-fix
to remove accidental contamination from PR #507's branch base.

In all three cases, the empty-string return collapses HMAC/HMAC-SHA256
inputs into a constant-key value, so security-relevant operations on the
fallback path produce identical tokens regardless of input.

Fixes:
1. src/lib/machineToken.ts:44 - getMachineTokenSync catch:
   log.error + return ""
2. src/lib/machineToken.ts:62 - getLegacyCliTokenSync catch:
   log.error + return ""
3. src/lib/db/encryption.ts:87-95 - getLegacyDynamicKey catch:
   added pino logger (createLogger("db:encryption")) + log.error
   instead of returning null silently
4. (incidental) src/lib/machineToken.ts module-load catch: also
   gains log.error so the new runtime catches have a 'log' constant
   to reference. This subsumes PR #507's machineToken.ts hunk.

NOTE: PR #507 will need its machineToken.ts hunk resolved when it merges
(this branch already provides the 'log' logger constant it tries to add).

Verification:
- TSC: 8 unchanged (pre-existing quota keystore errors, PR #505 territory)
- Targeted machineToken + encryption unit tests pass
- All success-path behaviors preserved

Co-authored-by: KooshaPari <koosha@example.com>

* fix(security): 4 audit findings + migrate encryption.ts to pino (#510)

Completes the audit-driven governance work that PR #509 started. Eight
independent fixes:

Security fixes (audit findings F5, F6, F9, F10):

1. src/lib/cloudSync.ts:47-56 — HMAC verification fail-open when
   CLOUD_SYNC_SECRET is unset. Now returns false (fail-closed) when a
   sigHeader is present but cannot be verified, instead of accepting
   any response. Legacy unverified mode (no sigHeader) is preserved.

2. src/lib/oauth/providers/antigravity.ts:101 — fetch() of userInfo
   during OAuth was swallowed silently, causing downstream code to use
   default projectId/tierId. Now logs warn with structured context.

3. open-sse/services/autoRefreshDaemon.ts:77,80 — periodic credential
   refresh errors were silently swallowed, allowing expired tokens to
   persist undetected. Now logs error per cycle.

4. src/lib/vscode/serviceTierVariants.ts:184 — request body parse
   failures during tier-rewrite were silently no-op'd. Now logs warn.

Refactor:

5. src/lib/db/encryption.ts — migrated ~7 console.* calls to pino
   logger (encryptionLog). Following the PR #509 pattern of
   createLogger("db:encryption"). The catch fallback behavior is
   preserved exactly; only logging changes.

Out of scope:
- encryption.ts:144-148 plaintext fallback (separate spec at
  plans/encryption-failclosed-spec.md, deferred for design discussion)
- src/lib/db/*.ts console.* migration for other files (separate PR)

Co-authored-by: KooshaPari <koosha@example.com>

* fix(governance): log empty catches in binaryManager + db/adapters (#512)

22 silent fail-open sites in the version manager + SQLite adapter layer
were just swallows. Per the audit:

- src/lib/versionManager/binaryManager.ts (6 catches): symlink/rollback/
  remove errors were silent; filesystem permission bugs were invisible
- src/lib/db/adapters/sqljsAdapter.ts (6 catches): save/close errors
  were silent; DB corruption during shutdown was undetectable
- src/lib/db/adapters/betterSqliteAdapter.ts (1 catch): same pattern
- src/lib/db/adapters/nodeSqliteAdapter.ts (2 catches): same pattern
- src/lib/db/adapters/nodeSqliteShared.ts (7 catches): same pattern

All 22 catches now log structured error context. Behavior unchanged -
only logging added. Per AGENTS.md, no encryption keys or raw secrets
are logged.

TSC: 8 unchanged
Tests: existing pass (any new behavior is logging-only)

Co-authored-by: KooshaPari <koosha@example.com>

* ci: pin cross-platform Rust toolchain

* docs(plans): track 2 governance specs for future implementation (#511)

Two deferred-implementation specs are now tracked in version control so
the work product is preserved and discoverable.

1. plans/encryption-failclosed-spec.md (883 lines) — Hardening
   encryption.ts:144-148 (the silent plaintext fallback). Compares 3
   design options with detailed tradeoffs. Recommended: C+A
   (startup canary + runtime throw). Registered as AgilePlus
   feature 'encryption-failclosed'.

2. plans/keyv-as-embedded-default-spec.md (698 lines) — Promote Keyv
   from optional driver to embedded default for fresh installs.
   Includes backwards-compat plan, config migration, and rollout
   sequence. Registered as AgilePlus feature 'keyv-as-embedded-default'.

These specs intentionally do NOT include code changes — they are
design-only and gate on answering the open questions before
implementation. See spec section 9 for each spec's open questions.

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* wip: auto-commit daemon 2026-08-06T09:18:52Z (#505)

Co-authored-by: Airlock Bot <airlock@phenoforge.local>

* refactor(db): migrate console.* to pino in src/lib/db/ (#522)

Follows the PR #506/#507/#509/#510 pattern of replacing console.*
with createLogger("db:<subsystem>") to provide structured logging
across the SQLite persistence layer.

Files modified (~155 callsites across 17 files):
- src/lib/db/adapters/driverFactory.ts
- src/lib/db/adapters/sqljsAdapter.ts
- src/lib/db/apiKeys.ts
- src/lib/db/backup.ts
- src/lib/db/cleanup.ts (~30 sites)
- src/lib/db/core.ts (~30 sites)
- src/lib/db/migrationRunner.ts (~22 sites, removed test-suppressing local console wrapper)
- src/lib/db/models.ts
- src/lib/db/optimizationSettings.ts
- src/lib/db/providers.ts
- src/lib/db/quotaPools.ts
- src/lib/db/quotaSnapshots.ts
- src/lib/db/schemaColumns.ts (~35 sites)
- src/lib/db/sessionAccountAffinity.ts
- src/lib/db/settings.ts
- src/lib/db/settings/cacheMetrics.ts
- src/lib/db/stateReset.ts

Per AGENTS.md guidance, never log SQLite encryption keys or raw secrets;
all logger calls redact sensitive material.

Behavior preservation:
- All success-path behavior unchanged
- Only the logging mechanism changes
- Targeted tests pass

One console.error preserved in core.ts:migrateFromJson because
tests/unit/db-core-migration.test.ts overrides console.error to detect
the failure; both console.error and log.error are emitted so the test
passes and pino is the canonical sink.

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* ci: use cache-pinned Trunk action

* fix(governance): log empty catches + weak peer-dep fallbacks (audit followup)

Continues the audit-driven governance cleanup that PRs #506, #507, #509,
#510, #512, #522, #525 started. This batch addresses the remaining empty
catches and weak peer-dependency fallbacks in plugins, embeddings,
oauth, monitoring, and combo paths.

Empty catches fixed (~6 sites):
- src/lib/plugins/loader.ts:200, 301 (plugin load/cleanup)
- src/lib/plugins/manager.ts:151, 270 (plugin staging cleanup)
- src/lib/embeddings/service.ts:50, 73 (embedding combo lookup)
- src/lib/credentialHealth/scheduler.ts:122 (credential health sweep)
- src/lib/usage/usageStats.ts:301 (usage stats)
- src/lib/oauth/providers/kimi-coding.ts:44 (OAuth provider init)

Weak peer-dep fallbacks fixed (~10 sites):
- src/lib/a2a/skills/providerDiscovery.ts:388 (MCP module load)
- open-sse/rpc/dispatchEdges.ts:189, 199, 209 (FFI/UDS transport —
  HIGH RISK, used log.error not warn)
- src/lib/monitoring/providerHealthAutopilot.ts:262 (quota monitor)
- open-sse/services/combo.ts:2222 (fetchCodexQuota)
- open-sse/services/combo/quotaStrategies.ts:431 (getRuntimeProviderProfile)
- open-sse/handlers/chatCore/codexFailover.ts:19 (provider connection)
- open-sse/handlers/chatCore/comboContextCache.ts:57 (proxy config)

All conversions preserve existing behavior (return null/false/etc.).
Only logging is added — no semantic change. Test results unchanged
from baseline.

TSC: 8 unchanged (pre-existing quota keystore errors, PR #505 territory).

---------

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Airlock Bot <airlock@phenoforge.local>
KooshaPari added a commit that referenced this pull request Aug 7, 2026
* chore(governance): rebase mergify config request-review fixes onto main

* fix(desktop): target fork-owned Electron releases

* ci: align workflows with selected action policy

* fix(governance): replace silent try/catch with explicit log.error in 5 modules (#506)

Builds on PR #505 (quota keystore type-drift fix). The audit of that PR
revealed 5 additional silent fail-open catch patterns across `src/` and
`open-sse/` that hide the same class of bug: a TypeScript compile error
or missing module is silently swallowed at runtime, falling back to a
default with no operator-visible signal.

This commit replaces those silent catches with explicit `log.error` calls
that surface the actual error to monitoring. Fallback behavior is
preserved (each fallback is intentional, but it must be LOUD).

Changes:

1. `src/lib/quota/storeFactory.ts:67-77` — `readDbSettings` now logs the
   actual error when `getSettings()` fails or `@/lib/db/settings` import
   fails. Same root cause as the previously-fixed Keyv/Redis catches.

2. `src/lib/quota/storeFactory.ts:138-147` — Redis driver catch upgraded
   from `log.warn` to `log.error`. Includes the configured Redis URL
   with the password segment redacted (`:***@`).

3. `src/lib/resilience/anomalyHook.ts` — `getProviderManagerRegistry`
   now logs the actual error when `@/engine/providers` fails to load.
   Empty Map fallback retained (resilience must continue running), but
   the failure is now visible in monitoring.

4. `open-sse/services/tierResolver.ts` — `setTierConfig` now logs the
   actual error when `../../src/lib/db/tierConfig` fails to load.
   `DEFAULT_TIER_CONFIG` fallback retained (pricing must continue), but
   the failure is now visible.

5. `open-sse/config/credentialLoader.ts` — `resolveCredentialsPath` now
   uses `log.error` (pino) instead of `console.warn`. Includes both the
   original error and the fallback path. Security-sensitive path; must
   keep working, but the failure must be loud.

6. `.gitignore` — exclude `.agileplus/` and `agileplus-*.db*` (local
   AgilePlus DB state, regenerated from `.md` specs via `agileplus
   specify`). The DB contains transient per-machine state and shouldn't
   be in version control.

Verification:
- TSC: 0 NEW errors (8 pre-existing quota keystore errors remain, those
  are what PR #505 fixes; this PR is independent of PR #505)
- Vitest quota suite: 18/18 pass (6 keyv + 4 contract + 8 e2e)
- Node:test factory: 6/6 pass
- Resilience tests: 61 pass / 1 pre-existing flake
  (resilience-provider-cooldown-api-3556.test.ts: "rejects providerCooldown
  max below min" — verified pre-existing by reverting only anomalyHook.ts
  and reproducing the same failure)

Out of scope (filed as separate bugs):
- `src/lib/resilience/anomalyHook.ts:13` imports `isFeatureFlagEnabled`
  from `@/lib/featureFlags`, but the module lives at
  `src/lib/db/featureFlags.ts`. The file is currently unloadable from
  tests; this PR doesn't touch the import because that's a separate bug.
- The Resilience subagent identified but did not fix the
  `tsconfig.typecheck-core.json` doesn't include `src/lib/resilience/`
  files — separate follow-up.

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(governance): broken featureFlags imports + 3 silent fail-opens + extras test (#507)

Continuation of the governance-debt cleanup started in PRs #505 and #506.
Six independent fixes, each addressing a class of silent-failure pattern
that the audits surfaced.

Changes:

1. **`src/lib/resilience/anomalyHook.ts:12`** — Broken import path.
   `isFeatureFlagEnabled` was imported from `@/lib/featureFlags`, but
   the module lives at `@/shared/utils/featureFlags`. This bug was
   masked because `tsconfig.typecheck-core.json` does not include
   `src/lib/resilience/` and no test loads the module successfully.
   After the fix, the module loads and exports the expected surface.

2. **`src/server-init.ts:128`** — Same broken import path. The
   surrounding try/catch at lines 130-139 silently swallowed the
   import failure. Fix: correct the path AND upgrade the catch log
   from `warn` to `error` (matches PR #506 pattern).

3. **`src/lib/versionManager/processManager.ts:155`** — `getProcessInfo`
   catch returned `{pid, alive: true}` after `ps`/readFile failures,
   which lies when the process is actually gone. Fix: catch now
   logs the error and returns `{pid, alive: false}` (honest about
   not being able to read process state).

4. **`src/server/ws/liveServer.ts:479`** — `loadAuthModule().catch(() => {})`
   silently swallowed initial auth module load failures, allowing the
   WS server to come up without auth configured. Fix: catch now logs
   `log.error`; fallback behavior preserved per the existing comment
   ("handler retries the import lazily").

5. **`src/lib/machineToken.ts:1-10`** — Crypto-relevant: empty catch
   around `require("node-machine-id")` silently fell back to
   `() => ""`, which collapses HMAC inputs to a constant. Fix: catch
   now logs `log.error` with security-context message, gated by a
   `fallbackLogged` flag so the log fires only once per process
   (avoiding log spam from any downstream reload).

6. **`tests/unit/quota/keyvQuotaStoreExtras.test.ts`** (new) — Closes
   spec §8.2 reachability test gap. 5 sanity tests for the
   `recordPlanUsage` / `upsertProviderPlan` / `listProviderPlans` /
   `setPools` / `getPool` surface on KeyvQuotaStore. Note: this
   branch is based on `origin/agent/migration-version-collision-fix`
   (pre-PR-#505), so the methods live directly on KeyvQuotaStore.
   When PR #505 lands, update the test to import from
   `keyvQuotaStoreExtras.ts` instead.

Verification:
- TSC error count: 8 unchanged (all pre-existing quota keystore
  errors that PR #505 fixes; this PR is independent of #505)
- Vitest extras test: 5/5 pass
- Node:test combined (processManager + machineToken + WS): 47/47 pass
- All success-path behavior preserved; only catch/fallback paths now log

Out of scope (separate PRs):
- Promote Keyv to "embedded default" driver (spec §10)
- Pre-existing e2e failure at `tests/e2e/quota-store.e2e.ts:117`
  (poolUsageWithDimensions shape mismatch — fixed in PR #505)

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(security): log 3 crypto-relevant silent catches (rebased onto canonical base) (#509)

This branch has been surgically rebased onto origin/agent/migration-version-collision-fix
to remove accidental contamination from PR #507's branch base.

In all three cases, the empty-string return collapses HMAC/HMAC-SHA256
inputs into a constant-key value, so security-relevant operations on the
fallback path produce identical tokens regardless of input.

Fixes:
1. src/lib/machineToken.ts:44 - getMachineTokenSync catch:
   log.error + return ""
2. src/lib/machineToken.ts:62 - getLegacyCliTokenSync catch:
   log.error + return ""
3. src/lib/db/encryption.ts:87-95 - getLegacyDynamicKey catch:
   added pino logger (createLogger("db:encryption")) + log.error
   instead of returning null silently
4. (incidental) src/lib/machineToken.ts module-load catch: also
   gains log.error so the new runtime catches have a 'log' constant
   to reference. This subsumes PR #507's machineToken.ts hunk.

NOTE: PR #507 will need its machineToken.ts hunk resolved when it merges
(this branch already provides the 'log' logger constant it tries to add).

Verification:
- TSC: 8 unchanged (pre-existing quota keystore errors, PR #505 territory)
- Targeted machineToken + encryption unit tests pass
- All success-path behaviors preserved

Co-authored-by: KooshaPari <koosha@example.com>

* fix(security): 4 audit findings + migrate encryption.ts to pino (#510)

Completes the audit-driven governance work that PR #509 started. Eight
independent fixes:

Security fixes (audit findings F5, F6, F9, F10):

1. src/lib/cloudSync.ts:47-56 — HMAC verification fail-open when
   CLOUD_SYNC_SECRET is unset. Now returns false (fail-closed) when a
   sigHeader is present but cannot be verified, instead of accepting
   any response. Legacy unverified mode (no sigHeader) is preserved.

2. src/lib/oauth/providers/antigravity.ts:101 — fetch() of userInfo
   during OAuth was swallowed silently, causing downstream code to use
   default projectId/tierId. Now logs warn with structured context.

3. open-sse/services/autoRefreshDaemon.ts:77,80 — periodic credential
   refresh errors were silently swallowed, allowing expired tokens to
   persist undetected. Now logs error per cycle.

4. src/lib/vscode/serviceTierVariants.ts:184 — request body parse
   failures during tier-rewrite were silently no-op'd. Now logs warn.

Refactor:

5. src/lib/db/encryption.ts — migrated ~7 console.* calls to pino
   logger (encryptionLog). Following the PR #509 pattern of
   createLogger("db:encryption"). The catch fallback behavior is
   preserved exactly; only logging changes.

Out of scope:
- encryption.ts:144-148 plaintext fallback (separate spec at
  plans/encryption-failclosed-spec.md, deferred for design discussion)
- src/lib/db/*.ts console.* migration for other files (separate PR)

Co-authored-by: KooshaPari <koosha@example.com>

* fix(governance): log empty catches in binaryManager + db/adapters (#512)

22 silent fail-open sites in the version manager + SQLite adapter layer
were just swallows. Per the audit:

- src/lib/versionManager/binaryManager.ts (6 catches): symlink/rollback/
  remove errors were silent; filesystem permission bugs were invisible
- src/lib/db/adapters/sqljsAdapter.ts (6 catches): save/close errors
  were silent; DB corruption during shutdown was undetectable
- src/lib/db/adapters/betterSqliteAdapter.ts (1 catch): same pattern
- src/lib/db/adapters/nodeSqliteAdapter.ts (2 catches): same pattern
- src/lib/db/adapters/nodeSqliteShared.ts (7 catches): same pattern

All 22 catches now log structured error context. Behavior unchanged -
only logging added. Per AGENTS.md, no encryption keys or raw secrets
are logged.

TSC: 8 unchanged
Tests: existing pass (any new behavior is logging-only)

Co-authored-by: KooshaPari <koosha@example.com>

* ci: pin cross-platform Rust toolchain

* docs(plans): track 2 governance specs for future implementation (#511)

Two deferred-implementation specs are now tracked in version control so
the work product is preserved and discoverable.

1. plans/encryption-failclosed-spec.md (883 lines) — Hardening
   encryption.ts:144-148 (the silent plaintext fallback). Compares 3
   design options with detailed tradeoffs. Recommended: C+A
   (startup canary + runtime throw). Registered as AgilePlus
   feature 'encryption-failclosed'.

2. plans/keyv-as-embedded-default-spec.md (698 lines) — Promote Keyv
   from optional driver to embedded default for fresh installs.
   Includes backwards-compat plan, config migration, and rollout
   sequence. Registered as AgilePlus feature 'keyv-as-embedded-default'.

These specs intentionally do NOT include code changes — they are
design-only and gate on answering the open questions before
implementation. See spec section 9 for each spec's open questions.

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* wip: auto-commit daemon 2026-08-06T09:18:52Z (#505)

Co-authored-by: Airlock Bot <airlock@phenoforge.local>

* refactor(db): migrate console.* to pino in src/lib/db/ (#522)

Follows the PR #506/#507/#509/#510 pattern of replacing console.*
with createLogger("db:<subsystem>") to provide structured logging
across the SQLite persistence layer.

Files modified (~155 callsites across 17 files):
- src/lib/db/adapters/driverFactory.ts
- src/lib/db/adapters/sqljsAdapter.ts
- src/lib/db/apiKeys.ts
- src/lib/db/backup.ts
- src/lib/db/cleanup.ts (~30 sites)
- src/lib/db/core.ts (~30 sites)
- src/lib/db/migrationRunner.ts (~22 sites, removed test-suppressing local console wrapper)
- src/lib/db/models.ts
- src/lib/db/optimizationSettings.ts
- src/lib/db/providers.ts
- src/lib/db/quotaPools.ts
- src/lib/db/quotaSnapshots.ts
- src/lib/db/schemaColumns.ts (~35 sites)
- src/lib/db/sessionAccountAffinity.ts
- src/lib/db/settings.ts
- src/lib/db/settings/cacheMetrics.ts
- src/lib/db/stateReset.ts

Per AGENTS.md guidance, never log SQLite encryption keys or raw secrets;
all logger calls redact sensitive material.

Behavior preservation:
- All success-path behavior unchanged
- Only the logging mechanism changes
- Targeted tests pass

One console.error preserved in core.ts:migrateFromJson because
tests/unit/db-core-migration.test.ts overrides console.error to detect
the failure; both console.error and log.error are emitted so the test
passes and pino is the canonical sink.

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

* ci: use cache-pinned Trunk action

* refactor: migrate console.* to pino in remaining src/ + open-sse/

Follows the PR #506/#507/#509/#510/#512/#518/#521/#522 pattern of replacing
console.* with createLogger("domain:subsystem") to provide structured
logging across the OmniRoute codebase.

PR #522 already migrated src/lib/db/*.ts (~155 callsites). This PR
migrates ~80 additional callsites across 37 files in:

- src/lib/resilience/* (normalize.ts, anomalyHook.ts)
- src/lib/oauth/* (connectionPersistence.ts)
- src/lib/vscode/* (tokenizedRequest.ts, dual-emit for test capture)
- src/lib/services/* (ringBuffer.ts, bootstrap.ts, embedWsProxy.ts, modelSync.ts)
- src/lib/sseTextTransform.ts, src/lib/dataPaths.ts, src/lib/initCloudSync.ts
- src/lib/events/eventBus.ts, src/lib/jobs/{budgetResetJob,reasoningCacheCleanupJob}.ts
- src/lib/cloudSync.ts, src/lib/localHealthCheck.ts
- src/lib/arenaEloSync.ts, src/lib/pricingSync.ts, src/lib/modelsDevSync.ts
- src/lib/tokenHealthCheck.ts, src/lib/gracefulShutdown.ts
- src/lib/apiBridgeServer.ts, src/lib/proxyLogger.ts
- src/lib/middleware/registry.ts, src/lib/quota/connectionRecovery.ts
- src/lib/credentialHealth/scheduler.ts
- src/middleware/promptInjectionGuard.ts
- src/server/ws/liveServer.ts (preserves [LiveWS] startup banner via log.info)
- src/sse/services/auth.ts, src/sse/services/streamState.ts
- open-sse/config/{constants,credentialLoader}.ts
- open-sse/services/{autoRefreshDaemon,quotaMonitor}.ts
- open-sse/mcp-server/audit.ts
- open-sse/utils/proxyFetch.ts
- open-sse/handlers/chatCore.ts (account fallback warnings)

User-facing startup banners and intentional CLI output are preserved:
- src/server/ws/liveServer.ts '[LiveWS] Dashboard WebSocket server listening'
  (now via log.info with structured host/port)
- src/lib/vscode/tokenizedRequest.ts '[VSCODE][SECURITY]' warning kept
  as console.warn alongside log.warn so tests/unit/vscode-token-in-url-warning.test.ts
  (which captures console.warn to verify once-per-process dedup) keeps passing —
  same pattern as PR #522's preservation in db/core.ts:migrateFromJson
- src/lib/oauth/utils/ui.ts (CLI formatting with picocolors) preserved as console
- src/mitm/* (CLI-driven tooling) preserved
- Next.js dashboard React components preserved (browser console, not server-side)

Behavior preservation:
- All success-path behavior unchanged
- Only the logging mechanism changes
- TSC baseline (typecheck-core.json) remains 0 errors
- Targeted tests pass: resilience-settings-normalize-split (9/9),
  resilience-settings-stream-recovery (9/9), resilience-settings-provider-breaker (9/9),
  oauth-refresh-error-resilience (12/12), vscode-tokenized-request (3/3),
  vscode-token-in-url-warning (4/4), services/embedWsProxy (30/30)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: KooshaPari <koosha@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Airlock Bot <airlock@phenoforge.local>
KooshaPari added a commit that referenced this pull request Aug 8, 2026
The F8 audit (PRs #509, #510, #518, #527) addressed several silent crypto-relevant catches, but scripts/check/crypto-failures still flagged 34 findings as of August 2026. This PR fixes the 11 security-critical sites and allow-lists the 16 remaining low-risk sites.

Fixed sites (all silent catches now log via pino):

1. src/lib/cloudSync.ts:74 — HMAC verify for cloud webhook. Silent failure hid forged signatures.

2. src/lib/middleware/cliTokenAuth.ts:87 — timingSafeEqual for CLI token verify.

3. src/server/authz/peerStamp.ts:31, 68 — timingSafeEqual for LOCAL_ONLY gate + reverse-proxy stamp.

4. src/shared/utils/apiAuth.ts:243 — jwtVerify for API auth (log.debug to avoid flooding on legitimate expired sessions).

5. src/shared/utils/machineId.ts:98 — randomUUID dynamic-import fallback. Replaced console.log with log.error.

6. src/shared/utils/machineId.ts:140 — createHash for machine ID. Replaced console.log with log.error.

7. src/lib/ws/handshake.ts:50 — jwtVerify for WS handshake (log.debug).

8. src/server/ws/liveServer.ts:195 — jwtVerify for live dashboard WS (log.debug).

9. src/app/api/auth/status/route.ts:22 — jwtVerify for auth status endpoint (log.debug).

10. src/lib/semanticCache.ts:251 — randomUUID + DB INSERT fallback chain.

Allow-list (16 remaining findings, documented as low-risk):

- Deploy routes use randomBytes for URL suffixes (non-security crypto)

- Test route uses randomUUID (non-production code path)

- Traffic inspector fingerprints + compares (intentional false-on-mismatch)

- MITM inspector context keys (non-security derivation)

- open-sse executor crypto (deterministic fallback on failure)

- open-sse TLS client cleanup (process-exit handlers, errors ignored intentionally)

- Responses logger sampling nonce (non-security)

- sha3-512 wrapper (callers handle via return type)

Check script improvements:

- Catch-body regex now matches domain loggers (encryptionLog.error, etc.) and all log levels (error|warn|info|debug|trace).

- DEFAULT_ALLOW_LIST with 8 regex patterns for known low-risk sites.

- --allow-list=<regex> flag for custom CI overrides.

Adds docs/crypto-failures-audit-2026-08.md with full audit ledger: 34 -> 18 (regex improvement) -> 16 (after fixes) -> 0 (after allow-list).

TSC: 0 errors baseline unchanged.

Co-authored-by: KooshaPari <koosha@example.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files typescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants