From ab9ed4748edcdc2fb088ed4706aaf02371bdb00c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Can=20G=C3=BCm=C3=BC=C5=9F?= Date: Sun, 9 Aug 2026 12:51:05 +0300 Subject: [PATCH] fix(sdk): wire FSStorageProvider rootDir in resolveSquadState, fix teamRoot='.' sentinel Fixes #1555 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .changeset/1555-fs-storage-rootdir.md | 5 +++ packages/squad-sdk/src/resolution.ts | 12 +++++-- test/state-backend.test.ts | 47 ++++++++++++++++++++++++++- 3 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 .changeset/1555-fs-storage-rootdir.md diff --git a/.changeset/1555-fs-storage-rootdir.md b/.changeset/1555-fs-storage-rootdir.md new file mode 100644 index 000000000..cfb5b49ad --- /dev/null +++ b/.changeset/1555-fs-storage-rootdir.md @@ -0,0 +1,5 @@ +--- +"@bradygaster/squad-sdk": patch +--- + +`resolveSquadState()` now constructs the local-backend `FSStorageProvider` with `rootDir` set to `paths.teamDir`, so its path-traversal guard actually validates state writes instead of no-op'ing on an unset rootDir. Also fixed `resolveSquadPaths()` treating a `config.teamRoot` of `"."` (the sentinel `squad externalize` writes) as remote mode, which pointed `teamDir` one level above `.squad/` — it now correctly falls through to local mode. Together these were breaking `squad_decide`/`squad_state_*` MCP tools on externalized projects with a "Path traversal blocked" error. diff --git a/packages/squad-sdk/src/resolution.ts b/packages/squad-sdk/src/resolution.ts index b3293e415..dfff4fd74 100644 --- a/packages/squad-sdk/src/resolution.ts +++ b/packages/squad-sdk/src/resolution.ts @@ -365,8 +365,12 @@ export function resolveSquadPaths(startDir?: string): ResolvedSquadPaths | null const isLegacy = name === '.ai-team'; const config = loadDirConfig(projectDir); - if (config && config.teamRoot) { + if (config && config.teamRoot && config.teamRoot !== '.') { // Remote mode: teamDir resolved relative to the project root (parent of .squad/) + // '.' is the sentinel externalize.ts writes for "no separate team root" — + // falls through to local mode below instead of resolving to the parent + // of .squad/ (path.resolve(projectRoot, '.') === projectRoot, one level + // too high). const projectRoot = path.resolve(projectDir, '..'); const teamDir = path.resolve(projectRoot, config.teamRoot); return { @@ -847,8 +851,12 @@ export function resolveSquadState(startDir?: string, cliOverride?: StateBackendT // For local backend, use FSStorageProvider directly (more capable). // For git-notes/orphan, bridge via StateBackendStorageAdapter. + // rootDir is paths.teamDir (matches the squadRoot every local-backend + // caller — e.g. ToolRegistry in state-mcp.ts — builds its paths against), + // so the traversal guard actually validates instead of no-op'ing on an + // unset rootDir and letting a bad upstream path resolve silently. const stateStorage: StorageProvider = backend.name === 'local' - ? new FSStorageProvider() + ? new FSStorageProvider(paths.teamDir) : new StateBackendStorageAdapter(backend, paths.projectDir); return { paths, backend, repoRoot, storage: stateStorage }; diff --git a/test/state-backend.test.ts b/test/state-backend.test.ts index ea7bdb350..f43de8cfe 100644 --- a/test/state-backend.test.ts +++ b/test/state-backend.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { mkdirSync, rmSync, writeFileSync, readFileSync, existsSync } from 'node:fs'; +import { mkdirSync, rmSync, writeFileSync, readFileSync, existsSync, readdirSync } from 'node:fs'; import { join } from 'node:path'; import { execSync, execFileSync } from 'node:child_process'; import { randomBytes } from 'node:crypto'; @@ -460,6 +460,51 @@ describe('resolveSquadState()', () => { expect(ctx).not.toBeNull(); expect(ctx!.storage.constructor.name).toBe('StateBackendStorageAdapter'); }); + + describe('#1555 regression: rootDir wiring + teamRoot=. sentinel', () => { + it('teamDir stays inside .squad/ when config.teamRoot is the "." sentinel externalize.ts writes', () => { + writeFileSync(join(squadDir(), 'config.json'), JSON.stringify({ version: 1, teamRoot: '.', projectKey: 'some-key', stateLocation: 'external' })); + const ctx = resolveSquadState(TMP); + expect(ctx).not.toBeNull(); + // Before the fix this resolved one level too high (path.resolve(projectRoot, '.') === projectRoot, + // the parent of .squad/) because the truthy check on config.teamRoot didn't special-case '.'. + expect(ctx!.paths.mode).toBe('local'); + expect(ctx!.paths.teamDir).toBe(squadDir()); + }); + + it('local-backend storage rootDir is wired to teamDir, not left unset', () => { + writeFileSync(join(squadDir(), 'config.json'), JSON.stringify({ version: 1, teamRoot: '.' })); + const ctx = resolveSquadState(TMP); + expect(ctx).not.toBeNull(); + + // A key that escapes teamDir must now be rejected instead of the traversal + // guard silently no-op'ing on an unset rootDir. + expect(() => ctx!.storage.readSync(join(ctx!.paths.teamDir, '..', 'outside.md'))) + .toThrow(/Path traversal blocked/); + + // A key inside teamDir still round-trips normally. + ctx!.storage.writeSync(join(ctx!.paths.teamDir, 'agents', 'data', 'history.md'), '# Data\n'); + expect(existsSync(join(squadDir(), 'agents', 'data', 'history.md'))).toBe(true); + }); + + it('squad_decide via ToolRegistry writes into .squad/decisions/inbox/, matching what state-mcp.ts wires up', async () => { + writeFileSync(join(squadDir(), 'config.json'), JSON.stringify({ version: 1, teamRoot: '.', projectKey: 'some-key', stateLocation: 'external' })); + const ctx = resolveSquadState(TMP); + expect(ctx).not.toBeNull(); + + // Same construction as createStateMcpToolRegistry() in state-mcp.ts. + const registry = new ToolRegistry(ctx!.paths.teamDir, undefined, ctx!.storage); + const decide = registry.getTool('squad_decide')!; + const result = await decide.handler({ author: 'test-agent', summary: 'Use FSStorageProvider rootDir', body: 'Confine local-backend writes to teamDir.' }); + + expect(result.resultType).toBe('success'); + const inboxDir = join(squadDir(), 'decisions', 'inbox'); + expect(existsSync(inboxDir)).toBe(true); + expect(readdirSync(inboxDir).length).toBeGreaterThan(0); + // Must not have landed one level up, in the repo root. + expect(existsSync(join(TMP, 'decisions'))).toBe(false); + }); + }); }); // ============================================================================