fix(config): resolve the home and writable dirs like the legacy CLI

[pjcdawkins]

Split out of #195, which needs the Go and PHP layers to agree on these directories.

• HomeDir checks <PREFIX>HOME, HOME, then USERPROFILE, as the legacy CLI does. On Windows, HOME can differ from USERPROFILE, e.g. in MSYS2 or Cygwin.
• WritableUserDir falls back to a temporary directory (<tmp>/<tmp_sub_dir>) if the directory in the home directory cannot be written, e.g. on an application container, or if a file is in its place.
• Writability is checked by permissions only, like the legacy CLI's Filesystem::canWrite, so both choose the same directory, even on a full disk. On Windows this checks the read-only attribute.

🤖 Generated with Claude Code

[upsun-dispatch]

Warning

Changes suggested — 🟡 2 warnings · ⚪ 1 nitpick

🔍 Full review · 4 files reviewed

⚪ Nitpick

• internal/config/dir_unix.go:1 — The constraint //go:build !windows also selects plan9, js/wasm and wasip1, but golang.org/x/sys/unix does not provide unix.Access there. The sibling internal/config/alt/fs_unix.go uses //go:build unix.

Verification

• canWrite matches the legacy Filesystem::canWrite: it stops at the first existing ancestor and checks only that one, as the PHP while loop does.
• The fallback condition matches legacy getWritableUserDir: unwritable, or a non-directory at the path, both go to <tmp>/<tmp_sub_dir>.
• The env var order in HomeDir (<PREFIX>HOME, HOME, USERPROFILE) matches legacy Config::getHomeDirectory.
• golang.org/x/sys is already a direct requirement in go.mod, so the new unix import needs no module change.

The new internal/config/dir_test.go covers the env var order in HomeDir and two temp-fallback cases: a read-only home (skipped on Windows and as root) and a file in place of the directory. The existing TestFromYAML checks the normal writable path. No test covers the Windows read-only-attribute branch or a nonexistent home directory. These tests run under make test (go test ./...).

Review details

• Commit: c5ca72a
• Model: claude-opus-5-5

---

Review 1 of 10 for this pull request · View the full run

[upsun-dispatch]

📋 PR Summary

Changes how the Go config layer finds the home and writable user directories so it picks the same ones as the legacy PHP CLI. HomeDir checks <PREFIX>HOME, then HOME, then USERPROFILE. WritableUserDir falls back to <tmp>/<tmp_sub_dir> when the home directory can't be written to, judging writability by permissions only. On Unix, a directory under a world-writable parent is also checked to confirm it is private. The latest push only adds a doc comment to hasSharedParent, explaining why group-writable parents don't count as shared.

Changes

Layer / File(s)	Summary
Directory resolution
internal/config/dir.go	HomeDir now checks the prefixed HOME variable, then HOME, then USERPROFILE. WritableUserDir falls back to a temporary subdirectory when the home directory can't be written to.
internal/config/dir_unix.go	Adds Unix checks for permissions, private directories and shared parents. The latest push documents that hasSharedParent only treats world-writable parents as shared. Group-writable parents are left out because they are common with umask 002 and in containers that run with arbitrary UIDs.
internal/config/dir_windows.go	Adds the Windows writability check, which looks at the read-only attribute.
Tests
internal/config/dir_test.go	Tests the home variable order and the fallback to the temporary directory.
internal/config/dir_unix_test.go	Tests the Unix permission and shared-parent checks.

[upsun-dispatch]

Warning

Changes suggested — 🟡 1 warning · 2 still open (1 nitpick)

🔁 Incremental · 4 files reviewed

Outstanding from earlier reviews:

• 🟡 #4166327757 — internal/config/dir.go:117: Go and PHP layers can choose different state directories for odd HOME values. — HomeDir still returns the first non-empty variable as given: it does not check the directory exists and does not apply realpath, unlike the legacy getHomeDirectory.
• ⚪ internal/config/dir_unix.go:1: Inconsistent build constraint with sibling file. — dir_unix.go is still //go:build !windows, and now also uses syscall.Stat_t, which plan9/wasm/wasip1 don't provide. (first raised)

Verification

• checkPrivateDir refuses a symlink not owned by the uid before following it, so a /tmp symlink planted by another user is rejected.
• A pre-existing shared /tmp/<tmp_sub_dir> owned by another user now makes WritableUserDir return an error, so the CLI does not write state into it.
• The 0o077 check makes checkPrivateDir chmod a directory the user owns back to 0700, and the 'shared directory is tightened' test case covers that.

The new table-driven TestCheckPrivateDir in internal/config/dir_unix_test.go covers ownership, symlinks, non-directories and the mode change. No test calls TempDir or WritableUserDir with a directory that is writable but owned by someone else.

Review details

• Commits: c5ca72a..22f810d
• Model: claude-opus-5-5

---

Review 2 of 10 for this pull request · View the full run

[upsun-dispatch]

Note

Reviewed — No blocking findings · 🔵 1 minor point

🔁 Incremental · 4 files reviewed

🔵 Minor point

• internal/config/dir_unix.go:57 — hasSharedParent only looks at the parent's world-writable bit (0o002). A parent that is group-writable but not world-writable (for example TMPDIR or <PREFIX>TMP set to a setgid project directory with mode 2775) counts as private. In that case checkPrivateDir skips both the ownership check and the 0700 tightening. Any member of that group can create upsun-cli-tmp there in advance with mode 0777. TempDir and WritableUserDir then accept that directory, so the cached phar/PHP binary and state.json end up in a directory the other user controls. Before this commit, ownership was enforced no matter what the parent's mode was.

Verification

• HomeDir now fails for a missing or non-directory value and returns the realpath. That matches legacy Config::getHomeDirectory, including the error text and the realpath() ?: $value fallback.
• checkPrivateDir recurses on the fully resolved target from filepath.EvalSymlinks. A second Lstat therefore cannot see a symlink again, so the recursion stops.
• Run under sudo -E, the ~user/.cache/upsun-cli-tmp directory is no longer rejected, because ~user/.cache is not world-writable. A dir pre-created in /tmp by another uid still fails the ownership check.

The commit adds table tests for HomeDir (env-var order, a symlink, a missing dir, a relative path) and for checkPrivateDir (shared vs. private parents, symlinks, another uid). These tests run in the go test jobs in .github/workflows/ci.yml, on both the Ubuntu and Windows runners. No test covers a group-writable parent.

Review details

• Commits: 22f810d..c00c0a5
• Model: claude-opus-5-5

---

Review 3 of 10 for this pull request · View the full run

[upsun-dispatch]

Note

Reviewed — No new issues found · 1 still open

🔁 Incremental · 1 file reviewed

Outstanding from earlier reviews:

• 🔵 internal/config/dir_unix.go:61: Other members of the group can plant or read CLI files in shared-group temp dirs. — hasSharedParent still checks only the 0o002 bit. The new comment documents this, but any member of a group-writable parent's group can still pre-create the directory there, and it is accepted without an ownership check or chmod. (first raised)

Verification

• The new doc comment on hasSharedParent matches what the code does: it checks only the parent's world-writable bit (Perm()&0o002).
• The /tmp fallback in WritableUserDir still goes through ensurePrivateDir, and that rejects a directory owned by someone else when the parent is world-writable, as /tmp is.
• HomeDir now fails with "invalid environment variable" when the value is not an existing directory, and it returns the EvalSymlinks-resolved absolute path, as the legacy realpath does.

This push only adds a doc comment. dir_unix_test.go, added earlier in the PR, covers checkPrivateDir and hasSharedParent. No new test was needed for a comment, and none was added.

Review details

• Commits: c00c0a5..6e6e69e
• Model: claude-opus-5-5

---

Review 4 of 10 for this pull request · View the full run