Skip to content

Commit fd7eb27

Browse files
roodboiclaude
andcommitted
fix: spawn resolves runtime PATH; doctor --fix root-ignores the secret key
Bun.spawn without an explicit env resolves argv[0] against the startup PATH snapshot — CI (no real docker) failed where dev machines masked it. buildSpawnEnv now always passes the live process env. That exposed a real test leak: remote-caddy-routes restored HOME from an unset per-test capture, unsetting HOME for the rest of the run. doctor --fix now also ensures the root .gitignore entry when untracking a leaked .hack.secret.key (nested .hack/.gitignore cannot cover a repo-root file) — from PR review. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 47ca25a commit fd7eb27

4 files changed

Lines changed: 67 additions & 11 deletions

File tree

src/commands/doctor.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import {
5252
import { parseDotEnv } from "../lib/env.ts";
5353
import {
5454
ensureDir,
55+
ensureGitignoreEntry,
5556
pathExists,
5657
readTextFile,
5758
writeTextFileIfChanged,
@@ -2726,7 +2727,16 @@ async function maybeUntrackGeneratedFiles(opts: {
27262727
"Commit the removal to stop sharing these files.",
27272728
];
27282729
if (inspection.secretKeyTracked) {
2730+
// The nested .hack/.gitignore cannot cover the repo-root key file; without
2731+
// the root entry the just-untracked key shows up as untracked and gets
2732+
// re-committed.
2733+
await ensureGitignoreEntry({
2734+
gitignorePath: resolve(project.projectRoot, ".gitignore"),
2735+
entry: PROJECT_ENV_KEY_FILENAME,
2736+
comment: "# project env key",
2737+
});
27292738
notes.push(
2739+
`Ensured ${PROJECT_ENV_KEY_FILENAME} is ignored in the root .gitignore.`,
27302740
`${PROJECT_ENV_KEY_FILENAME} was committed — treat it as leaked: rotate the key and re-encrypt secrets, and consider rewriting git history if the repo is shared.`
27312741
);
27322742
}

src/lib/shell.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,25 @@ export interface ExecOptions {
1010
readonly stdin?: "inherit" | "pipe" | "ignore";
1111
}
1212

13+
/**
14+
* Build the child env from the CURRENT `process.env` plus overrides.
15+
*
16+
* Always returns an explicit env (never undefined): `Bun.spawn` without an
17+
* env resolves argv[0] against the PATH snapshot captured at process
18+
* startup, which ignores runtime PATH changes — the same pitfall
19+
* `findExecutableInPath` documents. Passing the live env keeps child
20+
* behavior identical for normal runs while honoring runtime PATH.
21+
*/
1322
function buildSpawnEnv(
1423
override: Record<string, string> | undefined
15-
): Record<string, string> | undefined {
16-
if (!override) {
17-
return undefined;
18-
}
19-
24+
): Record<string, string> {
2025
const base: Record<string, string> = {};
2126
for (const [key, value] of Object.entries(process.env)) {
2227
if (typeof value === "string") {
2328
base[key] = value;
2429
}
2530
}
26-
return { ...base, ...override };
31+
return override ? { ...base, ...override } : base;
2732
}
2833

2934
export async function exec(

tests/doctor-generated-files.test.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
inspectTrackedGeneratedFiles,
1010
untrackGeneratedFiles,
1111
} from "../src/lib/doctor-generated-files.ts";
12-
import { pathExists } from "../src/lib/fs.ts";
12+
import { ensureGitignoreEntry, pathExists } from "../src/lib/fs.ts";
1313
import { ensureHackDirGitignore } from "../src/lib/project-env-config.ts";
1414

1515
const tempDirs = new Set<string>();
@@ -179,3 +179,38 @@ test("untrackGeneratedFiles with no paths is a no-op", async () => {
179179
});
180180
expect(result).toEqual({ ok: true, error: null });
181181
});
182+
183+
test("untracked secret key stays ignored once the root gitignore entry is ensured", async () => {
184+
const repoRoot = await createLeakedRepo();
185+
186+
const inspection = await inspectTrackedGeneratedFiles({
187+
projectRoot: repoRoot,
188+
projectDirName: ".hack",
189+
});
190+
expect(inspection?.secretKeyTracked).toBe(true);
191+
192+
const untracked = await untrackGeneratedFiles({
193+
projectRoot: repoRoot,
194+
paths: inspection?.trackedPaths ?? [],
195+
});
196+
expect(untracked.ok).toBe(true);
197+
198+
await ensureHackDirGitignore({ projectDir: resolve(repoRoot, ".hack") });
199+
await ensureGitignoreEntry({
200+
gitignorePath: resolve(repoRoot, ".gitignore"),
201+
entry: PROJECT_ENV_KEY_FILENAME,
202+
comment: "# project env key",
203+
});
204+
205+
const ignored = await runGit(
206+
["check-ignore", PROJECT_ENV_KEY_FILENAME],
207+
repoRoot
208+
);
209+
expect(ignored).toBe(PROJECT_ENV_KEY_FILENAME);
210+
211+
const status = await runGit(["status", "--porcelain"], repoRoot);
212+
expect(status).not.toContain(`?? ${PROJECT_ENV_KEY_FILENAME}`);
213+
expect(await pathExists(resolve(repoRoot, PROJECT_ENV_KEY_FILENAME))).toBe(
214+
true
215+
);
216+
});

tests/remote-caddy-routes.test.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,22 @@ import {
1212
} from "../src/lib/remote-caddy-routes.ts";
1313

1414
let tempHome: string | null = null;
15-
let previousHome: string | undefined;
15+
// Captured at module scope: afterEach runs after EVERY test, including ones
16+
// that never reassign HOME — a per-test capture would restore `undefined`
17+
// and unset HOME for the rest of the process (Bun deletes on
18+
// undefined-assign, unlike Node's string coercion).
19+
const previousHome: string | undefined = process.env.HOME;
1620

1721
afterEach(async () => {
1822
if (tempHome) {
1923
await rm(tempHome, { recursive: true, force: true });
2024
tempHome = null;
2125
}
22-
process.env.HOME = previousHome;
26+
if (previousHome === undefined) {
27+
Reflect.deleteProperty(process.env, "HOME");
28+
} else {
29+
process.env.HOME = previousHome;
30+
}
2331
});
2432

2533
test("extractCaddyHostsFromCompose returns normalized hostnames from caddy labels", () => {
@@ -65,7 +73,6 @@ test("resolveProjectHostsForBridge falls back to <project>.hack when compose has
6573

6674
test("reconcileRemoteCaddyRoutesForProject writes registry and compose when global caddy is not installed", async () => {
6775
tempHome = await mkdtemp(join(tmpdir(), "hack-remote-routes-reconcile-"));
68-
previousHome = process.env.HOME;
6976
process.env.HOME = tempHome;
7077

7178
const projectDir = resolve(tempHome, "workspace", "bridge-project", ".hack");
@@ -108,7 +115,6 @@ test("reconcileRemoteCaddyRoutesForProject writes registry and compose when glob
108115

109116
test("readRemoteCaddyRoutesState reports persisted registry/compose metadata", async () => {
110117
tempHome = await mkdtemp(join(tmpdir(), "hack-remote-routes-state-"));
111-
previousHome = process.env.HOME;
112118
process.env.HOME = tempHome;
113119

114120
const projectDir = resolve(tempHome, "workspace", "state-project", ".hack");

0 commit comments

Comments
 (0)