Repository navigation
fix(storage): accept paths under a symlinked root in its original spelling - #2492
Merged
Merged
Conversation
…lling FSStorageProvider stores its root resolved (/private/tmp/x) but callers pass absolute paths in the spelling they were given (/tmp/x/team.md), so the lexical containment check refused them as "Path traversal blocked". That broke squad_state for team roots reached through a symlink (found in the #2487 scenario pass) and caused 19 local-only test failures on macOS. Paths under the original spelling are now re-expressed under the resolved root before the existing checks, which still refuse real escapes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
🟡 Impact Analysis — PR #2492Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affectedroot (2 files)
squad-sdk (1 file)
tests (1 file)
This report is generated automatically for every PR. See #733 for details. |
Contributor
🛫 PR Readiness Check
PR Scope: 📦🔧 Mixed (product + infrastructure)
|
| Status | Check | Details |
|---|---|---|
| ✅ | Single commit | 1 commit — clean history |
| ✅ | Not in draft | Ready for review |
| ✅ | Branch up to date | Up to date with dev |
| ❌ | Copilot review | No Copilot review yet — it may still be processing |
| ✅ | Changeset present | Changeset file found |
| ✅ | Scope clean | No .squad/ or docs/proposals/ files |
| ✅ | No merge conflicts | No merge conflicts |
| ✅ | Copilot threads resolved | No Copilot review threads |
| ❌ | CI passing | 2 check(s) still running |
Files Changed (4 files, +506 −422)
| File | +/− |
|---|---|
.changeset/symlinked-team-root.md |
+6 −0 |
CHANGELOG.md |
+1 −0 |
packages/squad-sdk/src/storage/fs-storage-provider.ts |
+441 −422 |
test/fs-storage-symlinked-root.test.ts |
+58 −0 |
Total: +506 −422
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
bradygaster
enabled auto-merge (squash)
November 13, 2026 09:11
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Path rewriting can misroute reads and writes on case-sensitive macOS volumes, and the regression test fails on Windows without symlink privileges.
2 open issues
What changed in this PR
Fixes FSStorageProvider path handling so squad_state can access team folders through their original symlinked paths, including macOS /tmp.
Changes:
- Preserve the original root spelling and rebase paths before existing confinement checks.
- Add regression tests for linked-root access and rejected escapes.
- Document the fix and request SDK and CLI patch releases.
| File | Description |
|---|---|
test/fs-storage-symlinked-root.test.ts |
Tests linked-root access and traversal protection. |
packages/squad-sdk/src/storage/fs-storage-provider.ts |
Rebases original-spelling paths for async and sync operations. |
CHANGELOG.md |
Describes the user-visible storage fix. |
.changeset/symlinked-team-root.md |
Requests SDK and CLI patch releases. |
Review effort: Max (dynamic)
Comment on lines
+38
to
+40
| if (!lex || !root || this.pathEquals(lex, root)) return resolved; | ||
| if (this.pathEquals(resolved, lex)) return root; | ||
| if (this.pathStartsWith(resolved, lex + sep)) return root + resolved.slice(lex.length); |
Comment on lines
+53
to
+56
| fs.symlinkSync(path.join(base, 'secret.txt'), path.join(real, 'escape.md')); | ||
| const p = new FSStorageProvider(link); | ||
| await expect(p.read(path.join(link, 'escape.md'))).rejects.toThrow(/Symlink traversal blocked/); | ||
| expect(() => p.readSync(path.join(link, 'escape.md'))).toThrow(/Symlink traversal blocked/); |
bradygaster
added a commit
that referenced
this pull request
Nov 13, 2026
…ts (#2494) - squad doctor: the bridge reports its team dir resolved (/private/var/...) while doctor expected the unresolved spelling (/var/...), so the check failed on macOS temp and symlinked folders. Compare real paths. - Tests that assumed os.tmpdir() is already resolved, or that macOS ignores XDG_CONFIG_HOME, now hold on macOS: upstream-source-hardening, profile-fs-guard, export-import. Local full suite on macOS: 26 failures before #2492, now 0 besides timing flakes that pass when re-run alone. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2 of 6 tasks
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.
Found in the live scenario pass for #2487 (results).
Problem
FSStorageProviderstores its confinement root resolved through symlinks, for example/private/tmp/xon macOS. Callers pass absolute paths in the spelling they were given, such as/tmp/x/team.md. The first check compares the two as text, so every such path was refused with "Path traversal blocked: [team-root]/team.md".So
squad_statecouldn't read or write a team folder reached through a symlink: macOS/tmp, or a team root that is itself a link.Fix
A path under the root's original spelling is re-expressed under the resolved root before any check runs. Both spellings name the same folder, so this doesn't widen what is reachable. The existing checks still run on the result: the textual containment check, then the symlink check through
realpath, in both the async and sync paths.Tests
test/fs-storage-symlinked-root.test.ts(new):../out of the root, in either spelling;devand pass with the fix. The third shows escapes are still refused.Full suite on macOS: local-only failures drop from 26 to 7. The fix clears 19, including both
squad-20bridge tests and the memory and upstream tests, all of which hit this bug through/tmp. The remaining failures are in other areas: export/import, upstream source hardening, the profile guard and the gh-aw staging tests. Two of them are known timing flakes.🤖 Generated with Claude Code