Skip to content

fix(core): use os.homedir() instead of trusting HOME env var - #434

Merged
christso merged 1 commit into
mainfrom
fix/433-home-dir-resolution
Jul 13, 2026
Merged

fix(core): use os.homedir() instead of trusting HOME env var#434
christso merged 1 commit into
mainfrom
fix/433-home-dir-resolution

Conversation

@christso

Copy link
Copy Markdown
Contributor

Summary

  • getHomeDir() (src/constants.ts) prioritized process.env.HOME over USERPROFILE. Git Bash/MSYS shells set HOME, and MSYS mistranslates a misconfigured HOME=/c into the literal Windows path C:\ when spawning node.exe. Since getHomeDir() is the workspace root for every --scope user operation, this made allagents treat the entire C: drive as "home" — a user hit this exactly, crashing with EPERM: operation not permitted, scandir 'C:\$WinREAgent\Scratch\Mount\Windows\System32\LogFiles\WMI\RtBackup' (a SYSTEM-only folder at the drive root).
  • Fix: delegate to Node's built-in os.homedir(), which ignores HOME on Windows entirely (uses USERPROFILE/native API), so it's immune to this failure mode.
  • Side effect: ~16 existing test files sandbox user-scope tests by stubbing process.env.HOME to a temp dir — a pattern that no longer takes effect on Windows once HOME is ignored. Added tests/helpers/env.ts (stubHomeDir) to stub both HOME/USERPROFILE correctly (and restore — deleting, not stringifying undefined into — whichever wasn't originally set), and migrated all affected test files to use it.
  • tests/unit/utils/plugin-path.test.ts previously re-derived the old HOME || USERPROFILE formula to compute an expected value; changed it to call the real getHomeDir() so the test can't silently encode the same bug as production code.

Closes #433

Root cause / investigation

Full writeup in #433, reproduced directly:

$ HOME=/c node -e "console.log(process.env.HOME)"
C:\
$ HOME=/c node -e "console.log(require('os').homedir())"
C:\Users\<real-user>   <- unaffected

Test plan

  • Red: added tests/unit/constants.test.ts reproducing the bug against the old implementation (failed as expected)
  • Green: same test passes after switching to os.homedir()
  • bun test — 1232 pass, 2 pre-existing failures unrelated to this change (Windows path-separator assertions in tests/unit/core/repo-skills.test.ts, present on main too)
  • bun run typecheck — clean
  • Code review via superpowers:code-reviewer — 3 findings, all addressed (platform-gated the new test since POSIX os.homedir() behaves differently, clarified an intentionally-discarded restore-fn return, minor comment cleanup)

getHomeDir() prioritized process.env.HOME, which Git Bash/MSYS shells
can mistranslate into a bare drive root (e.g. "C:\") if HOME is
misconfigured — causing user-scope sync to treat the whole drive as
home and walk into permission-restricted system folders. os.homedir()
ignores HOME on Windows entirely, so it's immune to this failure mode.

Also updates the ~16 tests that stub HOME to sandbox user-scope
behavior in a temp dir, since os.homedir() no longer reads it on
Windows; added tests/helpers/env.ts to centralize the stub/restore
logic across them.

Closes #433
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying allagents with  Cloudflare Pages  Cloudflare Pages

Latest commit: 6261ebf
Status: ✅  Deploy successful!
Preview URL: https://e9c967d8.allagents.pages.dev
Branch Preview URL: https://fix-433-home-dir-resolution.allagents.pages.dev

View logs

@christso
christso merged commit 2afa965 into main Jul 13, 2026
1 check passed
@christso
christso deleted the fix/433-home-dir-resolution branch July 13, 2026 00:40
christso added a commit that referenced this pull request Jul 15, 2026
…-root plugin sources (#440)

A user hit `EPERM: operation not permitted, scandir 'C:\$WinREAgent\...'`
right after a successful `plugin uninstall`, on v1.13.2 (already past the
#434 getHomeDir() fix). Root cause: their workspace.yaml had a corrupted
plugin entry that was a bare `\`, which resolves to the drive root on
Windows — walkForSkillMd (skills.ts) recursed into it unguarded (no
try/catch, no symlink guard) and crashed the whole post-uninstall sync,
even though uninstall itself had already succeeded.

- walkForSkillMd now catches readdir errors and records a warning with
  remediation steps instead of throwing; also skips symlinks/junctions to
  avoid unbounded recursion through loops. Warnings thread through
  collectPluginSkills -> collectAllSkills/collectAvailableSkillNames ->
  SyncResult.warnings, deduplicated since the same bad plugin is scanned
  twice internally.
- addPlugin/addUserPlugin now reject a local-path source that resolves to
  a filesystem root (new isFilesystemRoot helper), closing the entry point
  that let the bare `\` get written in the first place.
- runUserSyncAndPrint (plugin.ts) was silently never printing the
  Warnings: section at all (unlike its project-scope sibling) - user-scope
  syncs were swallowing every warning, not just the new ones. Fixed to
  match runSyncAndPrint.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(core): getHomeDir() can resolve to drive root, causing full-disk scan on Windows

1 participant