Fix TestHardenConfigDir on macOS: resolve TMPDIR symlinks in test dirs#536
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a macOS-only unit test failure in TestHardenConfigDir by ensuring test temp directories don’t include symlinked ancestors (notably /var -> /private/var on darwin), so hardenConfigDir’s O_NOFOLLOW descent actually reaches and hardens the intended directories during positive-case assertions.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Changes:
- Add a
hardenTempDir(t)test helper thatfilepath.EvalSymlinks-resolvest.TempDir(). - Route all
TestHardenConfigDirtemp-dir usage throughhardenTempDirto avoid symlinked TMPDIR ancestors on macOS.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3 tasks
On darwin, t.TempDir() lives under /var -> private/var. hardenConfigDir's per-component O_NOFOLLOW descent correctly refuses the symlinked ancestor, so every positive-case subtest silently no-op'd (dir stayed 0755, expected 0700) and the negative cases passed vacuously. Linux CI never sees this because its TMPDIR has no symlinked ancestors. Route all TestHardenConfigDir temp dirs through a hardenTempDir helper that EvalSymlinks-resolves t.TempDir(), matching what the symlinked-final- component subtest already did inline. No production code change; unblocks make release (release-check -> make test) on macOS. Claude-Session: https://claude.ai/code/session_01CSpm1M4ZQwmurqqNb5uQx8
jeremy
force-pushed
the
harden-test-darwin
branch
from
July 17, 2026 15:40
3db8c86 to
fddb1d4
Compare
jeremy
enabled auto-merge (squash)
July 17, 2026 15:43
jeremy
added a commit
to yigitkonur/basecamp-cli
that referenced
this pull request
Jul 20, 2026
…e-codex-plugin * origin/main: Bump SDK to d15f023f and collapse todos update onto TodosService.Edit (basecamp#543) Fix agent-help hidden-flag leak; make check-surface a real drift gate (basecamp#542) Fix todos update silently clearing completion subscribers (basecamp#540) Fix TestHardenConfigDir on macOS: resolve TMPDIR symlinks in test dirs (basecamp#536) Bump golang.org/x/net to v0.56.0 for CVE-2026-46600 (basecamp#537)
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
make test(and thereforemake release→release-check) fails on macOS: every positive-case subtest ofTestHardenConfigDirreports the dir stayed 0755 when 0700 was expected.On darwin,
t.TempDir()lives under/var → private/var.hardenConfigDir's per-componentO_NOFOLLOWdescent correctly refuses the symlinked ancestor and silently aborts — so the positive cases never chmod, and the negative cases pass vacuously. Linux CI never sees this because its TMPDIR has no symlinked ancestors (CI is ubuntu-only across the matrix).Fix
Route all
TestHardenConfigDirtemp dirs through ahardenTempDirhelper thatEvalSymlinks-resolvest.TempDir()— the same thing the symlinked-final-component subtest already did inline. Test-only change; no production code touched. The production feature is unaffected for real config paths (~/.config/...has no symlinked ancestors).Testing
GOWORK=off go test ./internal/cli -run 'TestHardenConfigDir|TestOwnedByUID'green on macOS (previously 6 subtests failing)GOWORK=off make teston darwin (arm64) greenContext
Found while prepping the v0.8.0 release (73 commits unreleased since v0.7.2; the release is wanted for the completed-todos fix in #456, T2 card BC-9988776161). This unblocks running
make releasefrom a Mac.https://claude.ai/code/session_01CSpm1M4ZQwmurqqNb5uQx8
Summary by cubic
Fixes macOS failures in
TestHardenConfigDirby resolving symlinks in temp test dirs. This makes the hardening assertions run correctly and unblocks make release on macOS.hardenTempDir(t)thatEvalSymlinksthe result oft.TempDir()and used it acrossTestHardenConfigDirsubtests to avoid TMPDIR symlink ancestors.make test/release-checknow pass on macOS.Written for commit fddb1d4. Summary will update on new commits.