fix(env-leak-gate): skip pre-spawn scan for unregistered cwd paths (#991)

[Wirasm]

Summary

The pre-spawn env-leak guard in ClaudeClient.sendQuery() / CodexClient.sendQuery() used if (!codebase?.allow_env_keys), which evaluates truthy when codebase is null. Any sendQuery call with an unregistered cwd — title generation, codebase-less orchestrator runs, DAG executor paths — ran the sensitive-key scanner and threw EnvLeakError, blocking every conversation creation on deployed servers whose .env is in scope.

Root Cause

!undefined === true, so the defense-in-depth scanner branch was entered for unregistered paths it was never meant to cover. The canonical gate is registerRepoAtPath() in clone.ts; the pre-spawn check only exists for registered codebases without explicit consent.

Evidence:

• packages/core/src/clients/claude.ts:279
• packages/core/src/clients/codex.ts:166
• Trigger path: packages/core/src/services/title-generator.ts:53 → sendQuery with unregistered cwd.

Changes

File	Change
packages/core/src/clients/claude.ts	!codebase?.allow_env_keys → codebase && !codebase.allow_env_keys
packages/core/src/clients/codex.ts	Same predicate fix
packages/core/src/clients/claude.test.ts	Added regression test for unregistered cwd; updated existing tests that relied on the null-codebase path to use a registered codebase with allow_env_keys: false
packages/core/src/clients/codex.test.ts	Same test updates

Testing

• bun run type-check passes
• bun --filter @archon/core test passes (all 394+ tests)
• bun run lint passes
• New regression tests assert unregistered cwd skips the scan entirely
• Existing "registered codebase without consent" path still throws EnvLeakError
• Existing "registered codebase with allow_env_keys: true" path still skips scan

Why This Is Safe

The pre-spawn check is a safety net, not the primary gate:

• Registered without consent — still gated (predicate still enters branch)
• Registered with allow_env_keys: true — still skipped
• Unregistered cwd — no codebase row exists to flip allow_env_keys on, so gating there was a dead-end; registration remains the enforcement point

Scope

This is the shallow fix. The deeper architectural issue — that four upstream code paths call sendQuery with an unregistered cwd at all — is tracked in a companion issue for post-v0.3.2.

Issue

Fixes #991

Summary by CodeRabbit

• Bug Fixes
  • Corrected environment variable scanning behavior to properly skip when no registered codebase is detected, improving the accuracy of the security check logic.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The env-leak gate condition in two client files is modified to enforce scanning only when a registered codebase explicitly disallows environment key access, rather than treating unregistered codebases as blocked. Corresponding test coverage validates both registered and unregistered cwd scenarios.

Changes

Cohort / File(s)	Summary
Client Logic packages/core/src/clients/claude.ts, packages/core/src/clients/codex.ts	Changed pre-spawn env-leak gate condition from !codebase?.allow_env_keys to codebase && !codebase.allow_env_keys, restricting gate enforcement to registered codebases with explicit denial, bypassing scan for unregistered cwd paths.
Test Coverage packages/core/src/clients/claude.test.ts, packages/core/src/clients/codex.test.ts	Updated env-leak gate tests to mock registered codebase with allow_env_keys: false for denial scenario, added new test asserting scan is skipped for unregistered cwd paths, and corrected prior "no codebase" test to verify scanner is not invoked.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• bug(orchestrator): multiple code paths call sendQuery with unregistered cwd, violating pre-spawn check assumptions #993: Proposes a larger architectural refactor to pass explicit codebaseId parameter to sendQuery and remove cwd-based lookup heuristics entirely, building on the foundational gate logic fix introduced in this PR.

Poem

🐰 A gate that once locked all unregistered doors,
Now only guards what the registered stores.
No scan where no codebase exists to allow,
The warren workflows now freely flow! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and accurately describes the main fix: changing the env-leak gate predicate to skip scanning for unregistered cwd paths.
Description check	✅ Passed	The description is well-structured with Summary, Root Cause, Changes, Testing, Why Safe, Scope, and Issue sections covering the problem, solution, and evidence comprehensively.
Linked Issues check	✅ Passed	All code changes directly address #991 objectives: fixing the predicate to codebase && !codebase.allow_env_keys, preserving intended behaviors, and adding regression tests for unregistered cwd paths.
Out of Scope Changes check	✅ Passed	All changes are tightly scoped to the env-leak gate logic and its tests; no unrelated modifications detected across the four files.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-991-env-leak-unregistered-cwd

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Wirasm]

🔍 Automated Code Review

Summary

Fix is minimal, correct, and symmetric across both Claude and Codex clients. Tests accurately reflect the new contract. No critical or important issues identified.

Findings

✅ Strengths

• Predicate change directly addresses the root cause (!undefined === true bug with optional chaining on null codebase).
• Identical fix applied symmetrically in claude.ts and codex.ts — no drift between clients.
• Existing "throws when sensitive keys present" test is updated to inject a registered codebase with allow_env_keys: false, keeping the assertion meaningful.
• New "skips scan for unregistered cwd" regression test directly exercises the fixed code path.
• allowTargetRepoKeys and fail-closed tests are correctly updated so those branches remain exercised.

⚠️ Suggestions (non-blocking)

• None.

🔒 Security

• No new attack surface. registerRepoAtPath() in clone.ts remains the canonical gate. The pre-spawn check was documented defense-in-depth for registered codebases; loosening it for unregistered cwd is consistent with the stated architecture and matches the threat model.
• Edge case: if allow_env_keys column is ever null (vs false), codebase && !codebase.allow_env_keys still enters the branch — same behavior as pre-fix for registered paths, no regression.

Checklist

• Fix addresses root cause from investigation
• Code follows codebase patterns
• Tests cover the change (new test + updated existing tests)
• No silent failures introduced
• Type check / tests / lint all pass

---

Self-reviewed by Claude • Ready for human review