fix: remove CWD env stripping, load ~/.archon/.env with override for binary support#1045
Merged
fix: remove CWD env stripping, load ~/.archon/.env with override for binary support#1045
Conversation
…binary support The CWD .env stripping was redundant — SUBPROCESS_ENV_ALLOWLIST already blocks target repo credentials from reaching AI subprocesses, and the env-leak gate scans target repos before spawning. The stripping also broke compiled binaries: it nuked all env vars, then tried to reload from a baked CI path that doesn't exist on user machines. Changes: - Remove CWD .env stripping from both server and CLI - Server loads ~/.archon/.env with override: true (all keys, not just DATABASE_URL) - Skip import.meta.dir .env loading in binary mode (path is frozen at build time) - Add CLAUDE_USE_GLOBAL_AUTH defaulting to server (same as CLI) - Fix envFile reference in no_ai_credentials error to show useful path
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughRemoved logic that parsed and deleted CWD Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI
participant Server
participant GlobalEnv as "~/.archon/.env"
participant RepoEnv as "repo/.env (dev only)"
participant EnvGate as "SUBPROCESS_ENV_ALLOWLIST"
participant Subprocess as "AI subprocess (Claude/Code)"
rect rgba(200,220,255,0.5)
User->>CLI: run command
CLI->>GlobalEnv: load with override:true
CLI->>RepoEnv: (not parsed/stripped) repo env remains in process.env
CLI->>EnvGate: enforce allowlist at subprocess spawn
CLI->>Subprocess: spawn with filtered env
end
rect rgba(200,255,200,0.5)
User->>Server: start server
Server->>GlobalEnv: load with override:true
Server->>RepoEnv: if BUNDLED_IS_BINARY==false -> load repo/.env
Server->>EnvGate: ensure allowlist blocks sensitive keys for subprocesses
Server->>Subprocess: spawn filtered AI subprocess
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Collaborator
Author
PR Review SummaryCritical Issues (0 found)None. Important Issues (3 found)
Suggestions (6 found)
Strengths
Documentation Issues
VerdictNEEDS FIXES — 1 blocking issue (silent dotenv parse error in server), 1 type safety fix, and stale docs. Recommended Actions
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/cli.ts`:
- Around line 15-24: The CLI env-isolation test still deletes DATABASE_URL and
asserts the old behavior; update the test to stop removing DATABASE_URL and
instead verify the new contract: that globalEnvPath is loaded with config({
path: globalEnvPath, override: true }) (so ~/.archon/.env overrides CWD), and
that subprocess environment isolation relies on the SUBPROCESS_ENV_ALLOWLIST and
the env-leak gate scan before spawning (assert the allowlist is consulted and
the env-leak scanner is invoked or its result is respected); keep references to
the symbols globalEnvPath, config(... override: true), SUBPROCESS_ENV_ALLOWLIST
and the env-leak gate when adding assertions.
In `@packages/server/src/index.ts`:
- Around line 34-36: The current branch calling existsSync(globalEnvPath) then
config({ path: globalEnvPath, override: true }) ignores any errors from
dotenv.config; update the startup to check the result of config(...) (or catch
thrown errors) when globalEnvPath exists and, on parse failure, log the specific
error and abort startup (throw or process.exit(1)) so malformed ~/.archon/.env
is surfaced immediately; locate the call to config in
packages/server/src/index.ts and add explicit error handling around the
config(...) call referencing globalEnvPath, config, and existsSync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a3109b7-22a3-4015-9943-e422ee99f03a
📒 Files selected for processing (2)
packages/cli/src/cli.tspackages/server/src/index.ts
Comment on lines
+15
to
24
| // Load .env from global Archon config (override: true so ~/.archon/.env | ||
| // always wins over any Bun-auto-loaded CWD vars). | ||
| // | ||
| // Credential safety: target repo .env keys that Bun auto-loads from CWD | ||
| // cannot leak into AI subprocesses — SUBPROCESS_ENV_ALLOWLIST blocks them. | ||
| // The env-leak gate provides a second layer by scanning target repos before | ||
| // spawning. No CWD stripping needed. | ||
| const globalEnvPath = resolve(process.env.HOME ?? '~', '.archon', '.env'); | ||
| if (existsSync(globalEnvPath)) { | ||
| const result = config({ path: globalEnvPath, override: true }); |
There was a problem hiding this comment.
Update the CLI env-isolation test to match the new contract.
packages/cli/src/cli.test.ts:215-250 still describes the removed DATABASE_URL deletion behavior and manually does the delete itself, so it can keep passing without verifying anything in cli.ts. This change should be paired with test coverage for the new behavior instead: global ~/.archon/.env override semantics and reliance on the subprocess allowlist/env-leak gate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/cli.ts` around lines 15 - 24, The CLI env-isolation test
still deletes DATABASE_URL and asserts the old behavior; update the test to stop
removing DATABASE_URL and instead verify the new contract: that globalEnvPath is
loaded with config({ path: globalEnvPath, override: true }) (so ~/.archon/.env
overrides CWD), and that subprocess environment isolation relies on the
SUBPROCESS_ENV_ALLOWLIST and the env-leak gate scan before spawning (assert the
allowlist is consulted and the env-leak scanner is invoked or its result is
respected); keep references to the symbols globalEnvPath, config(... override:
true), SUBPROCESS_ENV_ALLOWLIST and the env-leak gate when adding assertions.
Comment on lines
34
to
+36
| if (existsSync(globalEnvPath)) { | ||
| const globalResult = config({ path: globalEnvPath, processEnv: {} }); | ||
| if (globalResult.parsed?.DATABASE_URL) { | ||
| process.env.DATABASE_URL = globalResult.parsed.DATABASE_URL; | ||
| } | ||
| config({ path: globalEnvPath, override: true }); | ||
| } |
There was a problem hiding this comment.
Handle ~/.archon/.env parse failures explicitly.
In bundled mode this file is now the only .env source, but this branch ignores dotenv.config() errors. A malformed global env file will fall through and later fail as no_ai_credentials or other config issues instead of surfacing the real startup problem. As per coding guidelines, "Prefer throwing early with a clear error for unsupported/unsafe states - never silently swallow errors or broaden permissions (Fail Fast + Explicit Errors)".
Suggested fix
const globalEnvPath = resolve(process.env.HOME ?? '~', '.archon', '.env');
if (existsSync(globalEnvPath)) {
- config({ path: globalEnvPath, override: true });
+ const globalEnvResult = config({ path: globalEnvPath, override: true });
+ if (globalEnvResult.error) {
+ console.error(
+ `Failed to load .env from ${globalEnvPath}: ${globalEnvResult.error.message}`
+ );
+ process.exit(1);
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/index.ts` around lines 34 - 36, The current branch
calling existsSync(globalEnvPath) then config({ path: globalEnvPath, override:
true }) ignores any errors from dotenv.config; update the startup to check the
result of config(...) (or catch thrown errors) when globalEnvPath exists and, on
parse failure, log the specific error and abort startup (throw or
process.exit(1)) so malformed ~/.archon/.env is surfaced immediately; locate the
call to config in packages/server/src/index.ts and add explicit error handling
around the config(...) call referencing globalEnvPath, config, and existsSync.
Merged
This was referenced Apr 10, 2026
Tyone88
pushed a commit
to Tyone88/Archon
that referenced
this pull request
Apr 16, 2026
fix: remove CWD env stripping, load ~/.archon/.env with override for binary support
joaobmonteiro
pushed a commit
to joaobmonteiro/Archon
that referenced
this pull request
Apr 26, 2026
fix: remove CWD env stripping, load ~/.archon/.env with override for binary support
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
archon servebecause CWD.envstripping nukes all env vars, and the reload path (import.meta.dir) is baked to the CI runner's filesystemarchon serveis unusable for binary users — the server exits withno_ai_credentialsbefore it can startSUBPROCESS_ENV_ALLOWLIST+ env-leak gate already protect), server loads~/.archon/.envwithoverride: truefor all keys, skipsimport.meta.dirin binary mode, addsCLAUDE_USE_GLOBAL_AUTHdefaultingSUBPROCESS_ENV_ALLOWLIST,buildSubprocessEnv(), env-leak gate, per-codebase DB env vars, Web UI env management — all untouchedUX Journey
Before
After
Architecture Diagram
Before
After
Connection inventory:
Label Snapshot
risk: mediumsize: Sserver,cliserver:env-loading,cli:env-loadingChange Metadata
bugmultiLinked Issue
archon servecrashes (discovered during v0.3.3 release testing)Validation Evidence (required)
server_listeningon port 3000Security Impact (required)
NoNoYesNoSUBPROCESS_ENV_ALLOWLIST(blocksANTHROPIC_API_KEY,ANTHROPIC_AUTH_TOKEN,OPENAI_API_KEYfrom reaching subprocesses) and the env-leak gate (scans target repos for sensitive keys before spawning). Both remain unchanged.Compatibility / Migration
YesYes—~/.archon/.envnow applies all keys to the server (previously onlyDATABASE_URL). Users who had conflicting keys in both repo.envand~/.archon/.envwill now see~/.archon/.envwin (override: true).NoHuman Verification (required)
Side Effects / Blast Radius (required)
Rollback Plan (required)
git revert <commit>— single commit, clean revertno_ai_credentials(same as current broken state)Risks and Mitigations
SUBPROCESS_ENV_ALLOWLISTblocks all non-whitelisted keys, andbuildSubprocessEnv()strips auth tokens when using global authSummary by CodeRabbit
Refactor
Security
Bug Fixes
Documentation