-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(cli): strip CWD .env leakage and respect platform adapters in serve #1068
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| /** | ||
| * Side-effect boot module: strip Bun-auto-loaded CWD `.env` keys immediately | ||
| * on import. Import this as the FIRST import in CLI/server entry points to | ||
| * guarantee the strip runs before any module that reads `process.env` at | ||
| * load time (e.g. the Pino logger in `@archon/paths/logger`). | ||
| * | ||
| * Usage: | ||
| * import '@archon/paths/strip-cwd-env-boot'; // must be the first import | ||
| * // ...other imports... | ||
| * | ||
| * The separation between `strip-cwd-env.ts` (pure function, testable) and | ||
| * this boot file (side-effect wrapper) keeps the stripping logic unit-testable | ||
| * while still providing the "runs before everything else" guarantee that | ||
| * entry points need. | ||
| */ | ||
| import { stripCwdEnv } from './strip-cwd-env'; | ||
|
|
||
| stripCwdEnv(); |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; | ||
| import { mkdtempSync, rmSync, writeFileSync } from 'fs'; | ||
| import { tmpdir } from 'os'; | ||
| import { join } from 'path'; | ||
| import { stripCwdEnv } from './strip-cwd-env'; | ||
|
|
||
| describe('stripCwdEnv', () => { | ||
| let tmpDir: string; | ||
| let originalCwd: string; | ||
| const testKeys = [ | ||
| 'STRIP_TEST_MARKER_A', | ||
| 'STRIP_TEST_MARKER_B', | ||
| 'STRIP_TEST_LOCAL_MARKER', | ||
| 'STRIP_TEST_DEV_MARKER', | ||
| 'STRIP_TEST_PROD_MARKER', | ||
| 'STRIP_TEST_OVERLAP_KEY', | ||
| 'STRIP_TEST_PRESERVED_KEY', | ||
| 'STRIP_TEST_MALFORMED_KEY', | ||
| ]; | ||
|
|
||
| beforeEach(() => { | ||
| tmpDir = mkdtempSync(join(tmpdir(), 'strip-cwd-env-')); | ||
| originalCwd = process.cwd(); | ||
| process.chdir(tmpDir); | ||
| // Clean any leaked keys from earlier runs | ||
| for (const key of testKeys) { | ||
| delete process.env[key]; | ||
| } | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| process.chdir(originalCwd); | ||
| rmSync(tmpDir, { recursive: true, force: true }); | ||
| for (const key of testKeys) { | ||
| delete process.env[key]; | ||
| } | ||
| }); | ||
|
|
||
| test('strips keys present in CWD .env', () => { | ||
| writeFileSync(join(tmpDir, '.env'), 'STRIP_TEST_MARKER_A=from_target_repo\n'); | ||
| // Simulate Bun's auto-load | ||
| process.env.STRIP_TEST_MARKER_A = 'from_target_repo'; | ||
|
|
||
| const stripped = stripCwdEnv(); | ||
|
|
||
| expect(process.env.STRIP_TEST_MARKER_A).toBeUndefined(); | ||
| expect(stripped).toContain('STRIP_TEST_MARKER_A'); | ||
| }); | ||
|
|
||
| test('strips keys from .env.local, .env.development, .env.production', () => { | ||
| writeFileSync(join(tmpDir, '.env.local'), 'STRIP_TEST_LOCAL_MARKER=local\n'); | ||
| writeFileSync(join(tmpDir, '.env.development'), 'STRIP_TEST_DEV_MARKER=dev\n'); | ||
| writeFileSync(join(tmpDir, '.env.production'), 'STRIP_TEST_PROD_MARKER=prod\n'); | ||
| process.env.STRIP_TEST_LOCAL_MARKER = 'local'; | ||
| process.env.STRIP_TEST_DEV_MARKER = 'dev'; | ||
| process.env.STRIP_TEST_PROD_MARKER = 'prod'; | ||
|
|
||
| const stripped = stripCwdEnv(); | ||
|
|
||
| expect(process.env.STRIP_TEST_LOCAL_MARKER).toBeUndefined(); | ||
| expect(process.env.STRIP_TEST_DEV_MARKER).toBeUndefined(); | ||
| expect(process.env.STRIP_TEST_PROD_MARKER).toBeUndefined(); | ||
| expect(stripped).toContain('STRIP_TEST_LOCAL_MARKER'); | ||
| expect(stripped).toContain('STRIP_TEST_DEV_MARKER'); | ||
| expect(stripped).toContain('STRIP_TEST_PROD_MARKER'); | ||
| }); | ||
|
|
||
| test('does nothing when no CWD .env files exist', () => { | ||
| process.env.STRIP_TEST_PRESERVED_KEY = 'should_remain'; | ||
|
|
||
| const stripped = stripCwdEnv(); | ||
|
|
||
| expect(process.env.STRIP_TEST_PRESERVED_KEY).toBe('should_remain'); | ||
| expect(stripped).toEqual([]); | ||
| }); | ||
|
|
||
| test('preserves keys not present in any CWD .env', () => { | ||
| writeFileSync(join(tmpDir, '.env'), 'STRIP_TEST_MARKER_A=from_target\n'); | ||
| process.env.STRIP_TEST_MARKER_A = 'from_target'; | ||
| process.env.STRIP_TEST_PRESERVED_KEY = 'should_remain'; | ||
|
|
||
| stripCwdEnv(); | ||
|
|
||
| expect(process.env.STRIP_TEST_MARKER_A).toBeUndefined(); | ||
| expect(process.env.STRIP_TEST_PRESERVED_KEY).toBe('should_remain'); | ||
| }); | ||
|
|
||
| test('ignores parse errors in target repo .env', () => { | ||
| // Write a .env with syntactically dubious content; dotenv's parser is | ||
| // lenient but we still want to verify nothing throws. | ||
| writeFileSync(join(tmpDir, '.env'), 'STRIP_TEST_MALFORMED_KEY="unterminated\n=noKey\n \n'); | ||
| process.env.STRIP_TEST_MALFORMED_KEY = 'set_before_strip'; | ||
|
|
||
| // Should not throw | ||
| expect(() => stripCwdEnv()).not.toThrow(); | ||
| }); | ||
|
|
||
| test('does not strip keys that dotenv parses but are absent from process.env', () => { | ||
| writeFileSync(join(tmpDir, '.env'), 'STRIP_TEST_MARKER_A=only_in_file\n'); | ||
| // Intentionally do NOT set process.env.STRIP_TEST_MARKER_A | ||
| // (simulates a .env file that Bun didn't auto-load — e.g. wrong CWD) | ||
|
|
||
| const stripped = stripCwdEnv(); | ||
|
|
||
| expect(stripped).not.toContain('STRIP_TEST_MARKER_A'); | ||
| }); | ||
| }); |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import { parse } from 'dotenv'; | ||
| import { readFileSync, existsSync } from 'fs'; | ||
| import { resolve } from 'path'; | ||
|
|
||
| /** | ||
| * Strip Bun-auto-loaded CWD `.env` keys from `process.env`. | ||
| * | ||
| * Bun's runtime (and compiled binaries) auto-load `.env` files from the current | ||
| * working directory before any user code runs. When `archon` is invoked from | ||
| * inside a target repo, that repo's `.env` leaks into the Archon process env, | ||
| * contaminating logging, config, and any `process.env.X` reads. | ||
| * | ||
| * The design rule is: **the CLI must never load target repo env**. Call this | ||
| * function at the very top of the CLI/server entry point — before loading | ||
| * `~/.archon/.env` — to undo Bun's auto-load. | ||
| * | ||
| * Files checked (matches Bun's auto-load set): `.env.local`, `.env.development`, | ||
| * `.env.production`, `.env`. For each existing file, parsed keys are deleted | ||
| * from `process.env`. Parse errors are ignored — a broken target repo `.env` | ||
| * is not our concern; we only need to strip keys, not validate them. | ||
| * | ||
| * Returns the list of keys that were stripped (useful for tests and debug logs). | ||
| */ | ||
| export function stripCwdEnv(): string[] { | ||
| const cwdEnvFiles = ['.env.local', '.env.development', '.env.production', '.env']; | ||
| const stripped: string[] = []; | ||
|
|
||
| for (const filename of cwdEnvFiles) { | ||
| const path = resolve(process.cwd(), filename); | ||
| if (!existsSync(path)) continue; | ||
| try { | ||
| const parsed = parse(readFileSync(path)); | ||
| for (const key of Object.keys(parsed)) { | ||
| if (key in process.env) { | ||
| // Dynamic delete is required: keys come from the target repo's .env | ||
| // at runtime, so they cannot be known statically. | ||
| // eslint-disable-next-line @typescript-eslint/no-dynamic-delete | ||
| delete process.env[key]; | ||
| stripped.push(key); | ||
| } | ||
| } | ||
| } catch { | ||
| // Ignore parse errors — we're only trying to undo Bun's auto-load, | ||
| // not validate the target repo's .env file. | ||
| } | ||
| } | ||
|
|
||
| return stripped; | ||
| } | ||
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid deleting shell-provided env vars while stripping CWD leakage
At Line 34, the current
key in process.envcheck deletes any existing env var if that key is present in CWD.env*. This can wipe legitimate operator/shell values (including critical vars) that were never leaked by Bun.💡 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents