[#1811] Trim path env vars before resolving venv directories #2012
Replies: 2 comments
|
Investigated this report and produced a candidate fix, validated on a fork of this repository. Root cause, fix and validation from the working notes:
Diff: awhite0030/prime-agent@main...fix/env-path-trim-1811-5450901552170605134 The change passes |
|
@awhite0030 Thanks for picking this up — and yes, please open the upstream PR. I reviewed the diff on your fork: the Once the PR is up against the branch CONTRIBUTING.md prescribes, happy to verify it there. |
Uh oh!
There was an error while loading. Please reload this page.
Hi, following up on #1811. I located the vulnerable path handling, implemented the proposed
getEnvPathutility, and have a minimal patch ready. Per CONTRIBUTING.md I'm posting here first instead of opening a PR, and would like an implementation invite if the approach looks good.Where the bug bites
packages/coding-agent/src/core/kernel/bootstrap.ts:getKernelVenvDir:if (override)is truthy for" ", soPRIME_AGENT_KERNEL_VENV=" "resolves to a bogus cwd-relative directory instead of falling back to~/.prime/agent/kernel-venv.getXdgKernelVenvDir: same shape withXDG_DATA_HOME— a whitespace value skips the~/.local/sharefallback.Both matter most in Docker/container environments where vars are often present-but-blank.
Fix
Added an exported
getEnvPath(name)helper that returns the trimmed value, orundefinedwhen missing, empty, or whitespace-only. Both venv-dir functions now consume it; since callers still pass the result throughexpandHome/path.resolve, the returned path is always absolute. No behavior change for well-formed values (including~-prefixed ones).Validation
New tests in
packages/coding-agent/test/kernel-bootstrap.test.ts:PRIME_AGENT_KERNEL_VENV→ falls back to the default dir;getEnvPathunit cases: missing/empty/whitespace →undefined, padded value → trimmed, and the trimmed override still wins ingetKernelVenvDir.Both new tests fail on unfixed
mainand pass with the patch. Also verified:biome checkclean,tsgo --noEmitclean. (Note: most other tests inkernel-bootstrap.test.tsfail on Windows because they shell out touvwith POSIX venv paths — pre-existing environment issue, unrelated to this change.)Plus the required changelog fragment
packages/coding-agent/.changes/1811-env-path-trim.md.Patch branch (on my fork, not a PR):
FekyBaz/prime-agent@fix/env-path-trim-1811— 3 files, +47/−6.One scoping note:
getHomeDirinpackage-manager.ts(process.env.HOME || homedir()) and theHOME/USERPROFILEfallback intree-selector.tshave the same whitespace-truthy shape. I left them out to keep this change focused, but happy to extend the helper there too if you'd like. Thanks!All reactions