fix(agent/agentcontext): canonicalize scan root in symlink boundary check#26175
Open
kylecarbs wants to merge 1 commit into
Open
fix(agent/agentcontext): canonicalize scan root in symlink boundary check#26175kylecarbs wants to merge 1 commit into
kylecarbs wants to merge 1 commit into
Conversation
2509213 to
d9ed34c
Compare
…heck On macOS, /var is a symlink to /private/var, so filepath.EvalSymlinks on a symlink target inside a scan root produces a /private/var path even when the caller's scanRoot is still /var/.... The boundary check in resolveReadTarget compared the canonical target against an un-canonical scan root and rejected legitimate in-root symlinks as StatusInvalid. This matches the lenient EvalSymlinks pattern already used in agentgit and agentmcp. Canonicalize scanRoot symmetrically with the target before the prefix check so symlinks inside the scan root are accepted while escaping targets are still rejected. Fixes the macOS test flakes in agent/agentcontext where t.TempDir() returns /var/folders/... paths that diverge from the canonical form the Manager and API surface. The affected tests now pin to the canonical temp dir via testutil.TempDirResolved, the same helper already used in agentmcp and agentgit.
d9ed34c to
2c3b78e
Compare
sreya
approved these changes
Jun 9, 2026
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
2 participants
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.
Summary
Fixes coder/internal#1574: macOS test flakes in
agent/agentcontextcaused by/varbeing a symlink to/private/var.Root cause
On macOS,
t.TempDir()returns paths under/var/folders/...whilefilepath.EvalSymlinks(and the package'sCanonicalizePath) resolves them to/private/var/folders/.... Two distinct symptoms followed:resolveReadTarget(causedTestResolver_SymlinkInsideScanRootAllowed). The symlink boundary check resolved the target viaEvalSymlinksbut onlyfilepath.Cleaned the scan root, so a/private/var/.../targetwas rejected as escaping a/var/.../scanRoot. This is a real production hazard: the working directory provided to theManageris not canonicalized before being added to scan roots, so a workspace mounted under a symlinked prefix would have legitimate in-root symlinks rejected asStatusInvalid.t.TempDir()against paths returned byManager.AddSource,Manager.HasSource,Manager.Sources,Manager.Snapshot, and the HTTP API, all of which return canonical paths.Fix
resolveReadTargetnow appliesfilepath.EvalSymlinksto the scan root before the prefix check, symmetrically with the target. Escaping targets are still rejected. This mirrors the same lenientEvalSymlinkspattern already used inagent/agentgit/agentgit.goandagent/x/agentmcp/configwatcher.go.testutil.TempDirResolved(t)helper (already used byagentmcpandagentgittests for exactly this macOS issue) for temp dirs whose paths are compared againstManager/API outputs.The pre-existing security tests (
TestResolver_SymlinkOutsideScanRootRejected,TestValidateSourcePath_RejectsOutsideRoot, etc.) still pass, so the symlink boundary remains enforced.Verification
Reproduced the macOS failure on Linux by constructing a temp tree where the scan root is reached through a synthetic symlinked prefix (mimicking
/var->/private/var). With only the production fix applied, the resolver case passes. With the test-side fixes applied, all previously-failing tests in the issue pass undergo test ./agent/agentcontext -count=5andgo test -race ./agent/agentcontext -count=2.Failing tests covered
TestResolver_SymlinkInsideScanRootAllowedTestManager_HasSourceTestManager_SeedSourcesLateBindsAfterManifestTestManager_InitialSourcesSeededTestManager_AddSourceTriggersResolveTestValidateSourcePath_AllowsInsideRootTestAPI_GetAndDeleteSourceTestAPI_AddAndListSourceAuthored by Coder Agents on behalf of @kylecarbs.