diff --git a/.changeset/symlinked-team-root.md b/.changeset/symlinked-team-root.md new file mode 100644 index 000000000..fe01f3b8c --- /dev/null +++ b/.changeset/symlinked-team-root.md @@ -0,0 +1,6 @@ +--- +"@bradygaster/squad-sdk": patch +"@bradygaster/squad-cli": patch +--- + +`squad_state` works when the team folder is reached through a symlink, such as macOS `/tmp` or a linked team root. A confined storage provider stored its root resolved but compared it with paths in the original spelling, so it refused them with "Path traversal blocked". Paths that really leave the folder are still refused. diff --git a/CHANGELOG.md b/CHANGELOG.md index 53434240a..5796a55d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,7 @@ Claude Code is the first host after GitHub Copilot where Squad uses the host's o - Claude hooks reach the launcher through `$CLAUDE_PROJECT_DIR`. Standalone bundle installs write `squad state-mcp`. A CLI run through `npx` keeps the exact-version `npx` command. - `squad doctor` flags machine-specific launch commands and a missing launcher. Run `squad upgrade` once to fix an existing repository. - **Coordinators wait for a human's sign-off.** When `team.md` or `routing.md` says a human must sign off on work (a release, tag, publish, merge or deploy), the Claude, Kiro, OpenCode, Cursor, Codex and Antigravity coordinators no longer treat the request itself as the sign-off. They name the rule and the human, ask whether you are that human or have their sign-off, go ahead only on an explicit yes, and record it in the decisions inbox. In a non-interactive run they stop at the gate. Seen in a live test: asked to "cut the 1.0.0 release", the Claude coordinator used to bump, tag and run `npm publish` itself. +- `squad_state` works when the team folder is reached through a symlink, such as macOS `/tmp` (really `/private/tmp`) or a linked team root. Squad stores the folder's resolved path but was given paths in the original spelling, so it refused reads and writes there with "Path traversal blocked". Paths that really leave the team folder, including through a symlink inside it, are still refused. - `squad init --client claude` tells you to run `claude --agent squad --mcp-config .mcp.json`, not Copilot. With several hosts it gives one line per host (#2453). - `squad_state` starts when Claude Code is started in a subfolder of the repository, such as a package in a monorepo. Claude Code finds the root `.mcp.json` from there but starts the server in the subfolder, so `node .squad/squad.mjs state-mcp` failed with "Cannot find module". The committed command now runs a short `node -e` script that finds the repository root (the nearest folder with `.git`) and runs only the `.squad/squad.mjs` there, so a launcher in a package's own `.squad/` or above the repository is never run. Run `squad upgrade` to rewrite an existing `.mcp.json`. - `.squad/squad.mjs` finds a `squad` on PATH. It used to skip every PATH entry, so a repository with only a global Squad CLI failed with "no Squad CLI found". Arguments reach the CLI unchanged on Windows (through npm's `.cmd` shims, never cmd.exe) and Linux, and a new CI job runs the launcher tests on both. diff --git a/packages/squad-sdk/src/storage/fs-storage-provider.ts b/packages/squad-sdk/src/storage/fs-storage-provider.ts index 2adc14efb..3fe6dfa9e 100644 --- a/packages/squad-sdk/src/storage/fs-storage-provider.ts +++ b/packages/squad-sdk/src/storage/fs-storage-provider.ts @@ -1,422 +1,441 @@ -import { readFile, writeFile, appendFile, access, readdir, unlink, mkdir as fsMkdir, realpath, rm, stat as fsStat, rename as fsRename, copyFile, open } from 'fs/promises'; -import { readFileSync, writeFileSync, appendFileSync, existsSync as fsExistsSync, mkdirSync as fsMkdirSync, realpathSync, readdirSync, statSync, unlinkSync, renameSync as fsRenameSync, copyFileSync, rmSync } from 'fs'; -import { dirname, resolve, sep } from 'path'; -import type { StorageProvider, StorageStats } from './storage-provider.js'; -import { StorageError, StateKeyConflictError, StateBackendUncertaintyError } from './storage-error.js'; - -/** - * FSStorageProvider — Node.js `fs` implementation of StorageProvider. - * - * - ENOENT reads return `undefined` (no throw). - * - Writes create parent directories recursively. - * - Appends create the file (and parent dirs) if missing. - * - delete is a no-op when the file does not exist. - * - list returns an empty array for a missing directory. - * - Optional `rootDir` confines all operations to a specific directory tree. - */ -export class FSStorageProvider implements StorageProvider { - private static readonly CASE_INSENSITIVE = process.platform === 'win32' || process.platform === 'darwin'; - private readonly rootDir?: string; - - constructor(rootDir?: string) { - this.rootDir = rootDir ? realpathSync(resolve(rootDir)) : undefined; - } - - private pathStartsWith(fullPath: string, prefix: string): boolean { - if (FSStorageProvider.CASE_INSENSITIVE) { - return fullPath.toLowerCase().startsWith(prefix.toLowerCase()); - } - return fullPath.startsWith(prefix); - } - - private pathEquals(a: string, b: string): boolean { - if (FSStorageProvider.CASE_INSENSITIVE) { - return a.toLowerCase() === b.toLowerCase(); - } - return a === b; - } - - /** - * Validates that filePath resolves within rootDir and does not escape - * via path traversal or symlinks. - * - * ⚠️ TOCTOU: This is a user-space check. Between validation and the - * subsequent fs operation, a path component could theoretically be - * swapped for a symlink. This is an inherent limitation of user-space - * path validation on POSIX systems. For defense-in-depth, callers - * operating in hostile environments should use OS-level confinement - * (chroot, namespaces, or sandboxing) in addition to this check. - */ - private async assertSafePath(filePath: string): Promise { - if (!this.rootDir) return filePath; - - const resolved = resolve(this.rootDir, filePath); - - // Check if resolved path is within rootDir - if (!this.pathStartsWith(resolved, this.rootDir + sep) && !this.pathEquals(resolved, this.rootDir)) { - throw new Error(`Path traversal blocked: ${filePath}`); - } - - // Check for symlink traversal - try { - const real = await realpath(resolved); - if (!this.pathStartsWith(real, this.rootDir + sep) && !this.pathEquals(real, this.rootDir)) { - throw new Error(`Symlink traversal blocked: ${filePath}`); - } - return real; - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') { - // Walk up to nearest existing ancestor and verify it resolves within rootDir - let checkPath = resolved; - while (!this.pathEquals(checkPath, this.rootDir)) { - checkPath = dirname(checkPath); - try { - const realAncestor = await realpath(checkPath); - if (!this.pathStartsWith(realAncestor, this.rootDir + sep) && !this.pathEquals(realAncestor, this.rootDir)) { - throw new Error(`Symlink traversal blocked: ${filePath}`); - } - return resolved; - } catch (ancestorErr: unknown) { - if ((ancestorErr as NodeJS.ErrnoException).code === 'ENOENT') continue; - throw ancestorErr; - } - } - return resolved; - } - throw err; - } - } - - private assertSafePathSync(filePath: string): string { - if (!this.rootDir) return filePath; - - const resolved = resolve(this.rootDir, filePath); - - // Check if resolved path is within rootDir - if (!this.pathStartsWith(resolved, this.rootDir + sep) && !this.pathEquals(resolved, this.rootDir)) { - throw new Error(`Path traversal blocked: ${filePath}`); - } - - // Check for symlink traversal - try { - const real = realpathSync(resolved); - if (!this.pathStartsWith(real, this.rootDir + sep) && !this.pathEquals(real, this.rootDir)) { - throw new Error(`Symlink traversal blocked: ${filePath}`); - } - return real; - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') { - // Walk up to nearest existing ancestor and verify it resolves within rootDir - let checkPath = resolved; - while (!this.pathEquals(checkPath, this.rootDir)) { - checkPath = dirname(checkPath); - try { - const realAncestor = realpathSync(checkPath); - if (!this.pathStartsWith(realAncestor, this.rootDir + sep) && !this.pathEquals(realAncestor, this.rootDir)) { - throw new Error(`Symlink traversal blocked: ${filePath}`); - } - return resolved; - } catch (ancestorErr: unknown) { - if ((ancestorErr as NodeJS.ErrnoException).code === 'ENOENT') continue; - throw ancestorErr; - } - } - return resolved; - } - throw err; - } - } - - async createIfAbsent(filePath: string, data: string): Promise { - const safePath = await this.assertSafePath(filePath); - // Create parent dirs before attempting exclusive open so mkdir failure - // is distinguished from the key-already-exists conflict. - try { - await fsMkdir(dirname(safePath), { recursive: true }); - } catch (mkdirErr: unknown) { - throw new StateBackendUncertaintyError( - 'createIfAbsent', - `mkdir failed for "${filePath}": ${(mkdirErr as NodeJS.ErrnoException).code ?? 'UNKNOWN'}`, - ); - } - let fh: import('fs/promises').FileHandle | undefined; - try { - // 'wx' = O_WRONLY | O_CREAT | O_EXCL — fails atomically with EEXIST - // if the file already exists, with no race between check and create. - fh = await open(safePath, 'wx'); - } catch (openErr: unknown) { - const code = (openErr as NodeJS.ErrnoException).code; - // Windows can report a sharing violation instead of EEXIST for a racing creator. - let existingFile = false; - if (code === 'EACCES' || code === 'EPERM') { - try { - await access(safePath); - existingFile = true; - } catch { /* It may have disappeared after the exclusive open failed. */ } - } - if (code === 'EEXIST' || existingFile) { - throw new StateKeyConflictError(filePath); - } - throw new StateBackendUncertaintyError( - 'createIfAbsent', - `exclusive open failed for "${filePath}": ${code ?? 'UNKNOWN'}`, - ); - } - try { - await fh.writeFile(data, 'utf-8'); - } catch (writeErr: unknown) { - // Write failed after exclusive open: state is uncertain (file exists but - // may be empty or partial). Attempt cleanup; if cleanup fails, the - // uncertainty persists but we still surface the original error. - try { await unlink(safePath); } catch { /* best-effort cleanup */ } - throw new StateBackendUncertaintyError( - 'createIfAbsent', - `write failed after exclusive open for "${filePath}": ${(writeErr as NodeJS.ErrnoException).code ?? 'UNKNOWN'}`, - ); - } finally { - try { await fh.close(); } catch { /* best-effort */ } - } - } - - async read(filePath: string): Promise { - const safePath = await this.assertSafePath(filePath); - try { - return await readFile(safePath, 'utf-8'); - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') return undefined; - throw new StorageError('read', filePath, err as NodeJS.ErrnoException); - } - } - - async write(filePath: string, data: string): Promise { - const safePath = await this.assertSafePath(filePath); - try { - await fsMkdir(dirname(safePath), { recursive: true }); - await writeFile(safePath, data, 'utf-8'); - } catch (err: unknown) { - throw new StorageError('write', filePath, err as NodeJS.ErrnoException); - } - } - - async append(filePath: string, data: string): Promise { - const safePath = await this.assertSafePath(filePath); - try { - await fsMkdir(dirname(safePath), { recursive: true }); - await appendFile(safePath, data, 'utf-8'); - } catch (err: unknown) { - throw new StorageError('append', filePath, err as NodeJS.ErrnoException); - } - } - - async exists(filePath: string): Promise { - const safePath = await this.assertSafePath(filePath); - try { - await access(safePath); - return true; - } catch { - return false; - } - } - - async list(dirPath: string): Promise { - const safePath = await this.assertSafePath(dirPath); - try { - return await readdir(safePath); - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') return []; - throw new StorageError('list', dirPath, err as NodeJS.ErrnoException); - } - } - - async delete(filePath: string): Promise { - const safePath = await this.assertSafePath(filePath); - try { - await unlink(safePath); - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') return; - throw new StorageError('delete', filePath, err as NodeJS.ErrnoException); - } - } - - readSync(filePath: string): string | undefined { - const safePath = this.assertSafePathSync(filePath); - try { - return readFileSync(safePath, 'utf-8'); - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') return undefined; - throw new StorageError('read', filePath, err as NodeJS.ErrnoException); - } - } - - writeSync(filePath: string, data: string): void { - const safePath = this.assertSafePathSync(filePath); - try { - fsMkdirSync(dirname(safePath), { recursive: true }); - writeFileSync(safePath, data, 'utf-8'); - } catch (err: unknown) { - throw new StorageError('write', filePath, err as NodeJS.ErrnoException); - } - } - - existsSync(filePath: string): boolean { - const safePath = this.assertSafePathSync(filePath); - return fsExistsSync(safePath); - } - - listSync(dirPath: string): string[] { - const safePath = this.assertSafePathSync(dirPath); - try { - return readdirSync(safePath); - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') return []; - throw new StorageError('list', dirPath, err as NodeJS.ErrnoException); - } - } - - async deleteDir(dirPath: string): Promise { - const safePath = await this.assertSafePath(dirPath); - try { - await rm(safePath, { recursive: true, force: true }); - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') return; - throw new StorageError('deleteDir', dirPath, err as NodeJS.ErrnoException); - } - } - - // ── Phase 3 Wave 1 additions ──────────────────────────────────────────── - - async isDirectory(targetPath: string): Promise { - const safePath = await this.assertSafePath(targetPath); - try { - const s = await fsStat(safePath); - return s.isDirectory(); - } catch { - return false; - } - } - - async mkdir(dirPath: string, options?: { recursive?: boolean }): Promise { - const safePath = await this.assertSafePath(dirPath); - try { - await fsMkdir(safePath, { recursive: options?.recursive ?? false }); - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'EEXIST') return; - throw new StorageError('mkdir', dirPath, err as NodeJS.ErrnoException); - } - } - - async rename(oldPath: string, newPath: string): Promise { - const safeOld = await this.assertSafePath(oldPath); - const safeNew = await this.assertSafePath(newPath); - try { - await fsMkdir(dirname(safeNew), { recursive: true }); - await fsRename(safeOld, safeNew); - } catch (err: unknown) { - throw new StorageError('rename', oldPath, err as NodeJS.ErrnoException); - } - } - - async copy(srcPath: string, destPath: string): Promise { - const safeSrc = await this.assertSafePath(srcPath); - const safeDest = await this.assertSafePath(destPath); - try { - await fsMkdir(dirname(safeDest), { recursive: true }); - await copyFile(safeSrc, safeDest); - } catch (err: unknown) { - throw new StorageError('copy', srcPath, err as NodeJS.ErrnoException); - } - } - - async stat(targetPath: string): Promise { - const safePath = await this.assertSafePath(targetPath); - try { - const s = await fsStat(safePath); - return { size: s.size, mtimeMs: s.mtimeMs, isDirectory: s.isDirectory() }; - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') return undefined; - throw new StorageError('stat', targetPath, err as NodeJS.ErrnoException); - } - } - - isDirectorySync(targetPath: string): boolean { - const safePath = this.assertSafePathSync(targetPath); - try { - return statSync(safePath).isDirectory(); - } catch { - return false; - } - } - - mkdirSync(dirPath: string, options?: { recursive?: boolean }): void { - const safePath = this.assertSafePathSync(dirPath); - try { - fsMkdirSync(safePath, { recursive: options?.recursive ?? false }); - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'EEXIST') return; - throw new StorageError('mkdir', dirPath, err as NodeJS.ErrnoException); - } - } - - statSync(targetPath: string): StorageStats | undefined { - const safePath = this.assertSafePathSync(targetPath); - try { - const s = statSync(safePath); - return { size: s.size, mtimeMs: s.mtimeMs, isDirectory: s.isDirectory() }; - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') return undefined; - throw new StorageError('stat', targetPath, err as NodeJS.ErrnoException); - } - } - - appendSync(filePath: string, data: string): void { - const safePath = this.assertSafePathSync(filePath); - try { - fsMkdirSync(dirname(safePath), { recursive: true }); - appendFileSync(safePath, data, 'utf-8'); - } catch (err: unknown) { - throw new StorageError('append', filePath, err as NodeJS.ErrnoException); - } - } - - deleteSync(filePath: string): void { - const safePath = this.assertSafePathSync(filePath); - try { - unlinkSync(safePath); - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') return; - throw new StorageError('delete', filePath, err as NodeJS.ErrnoException); - } - } - - renameSync(oldPath: string, newPath: string): void { - const safeOld = this.assertSafePathSync(oldPath); - const safeNew = this.assertSafePathSync(newPath); - try { - fsMkdirSync(dirname(safeNew), { recursive: true }); - fsRenameSync(safeOld, safeNew); - } catch (err: unknown) { - throw new StorageError('rename', oldPath, err as NodeJS.ErrnoException); - } - } - - copySync(srcPath: string, destPath: string): void { - const safeSrc = this.assertSafePathSync(srcPath); - const safeDest = this.assertSafePathSync(destPath); - try { - fsMkdirSync(dirname(safeDest), { recursive: true }); - copyFileSync(safeSrc, safeDest); - } catch (err: unknown) { - throw new StorageError('copy', srcPath, err as NodeJS.ErrnoException); - } - } - - deleteDirSync(dirPath: string): void { - const safePath = this.assertSafePathSync(dirPath); - try { - rmSync(safePath, { recursive: true, force: true }); - } catch (err: unknown) { - if ((err as NodeJS.ErrnoException).code === 'ENOENT') return; - throw new StorageError('deleteDir', dirPath, err as NodeJS.ErrnoException); - } - } -} +import { readFile, writeFile, appendFile, access, readdir, unlink, mkdir as fsMkdir, realpath, rm, stat as fsStat, rename as fsRename, copyFile, open } from 'fs/promises'; +import { readFileSync, writeFileSync, appendFileSync, existsSync as fsExistsSync, mkdirSync as fsMkdirSync, realpathSync, readdirSync, statSync, unlinkSync, renameSync as fsRenameSync, copyFileSync, rmSync } from 'fs'; +import { dirname, resolve, sep } from 'path'; +import type { StorageProvider, StorageStats } from './storage-provider.js'; +import { StorageError, StateKeyConflictError, StateBackendUncertaintyError } from './storage-error.js'; + +/** + * FSStorageProvider — Node.js `fs` implementation of StorageProvider. + * + * - ENOENT reads return `undefined` (no throw). + * - Writes create parent directories recursively. + * - Appends create the file (and parent dirs) if missing. + * - delete is a no-op when the file does not exist. + * - list returns an empty array for a missing directory. + * - Optional `rootDir` confines all operations to a specific directory tree. + */ +export class FSStorageProvider implements StorageProvider { + private static readonly CASE_INSENSITIVE = process.platform === 'win32' || process.platform === 'darwin'; + private readonly rootDir?: string; + /** The root as the caller spelled it, before symlinks were resolved (e.g. macOS `/tmp/x`). */ + private readonly lexicalRoot?: string; + + constructor(rootDir?: string) { + this.lexicalRoot = rootDir ? resolve(rootDir) : undefined; + this.rootDir = this.lexicalRoot ? realpathSync(this.lexicalRoot) : undefined; + } + + /** + * Re-express a path under the root's original spelling under its resolved spelling. The root is + * stored resolved (`/private/tmp/x`), but callers build absolute paths from the spelling they were + * given (`/tmp/x/team.md`); without this, the lexical containment check rejects them as traversal. + * Both spellings name the same directory, so this changes nothing about what is reachable: the + * symlink checks that follow still run on the result. + */ + private rebase(resolved: string): string { + const lex = this.lexicalRoot; + const root = this.rootDir; + 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); + return resolved; + } + + private pathStartsWith(fullPath: string, prefix: string): boolean { + if (FSStorageProvider.CASE_INSENSITIVE) { + return fullPath.toLowerCase().startsWith(prefix.toLowerCase()); + } + return fullPath.startsWith(prefix); + } + + private pathEquals(a: string, b: string): boolean { + if (FSStorageProvider.CASE_INSENSITIVE) { + return a.toLowerCase() === b.toLowerCase(); + } + return a === b; + } + + /** + * Validates that filePath resolves within rootDir and does not escape + * via path traversal or symlinks. + * + * ⚠️ TOCTOU: This is a user-space check. Between validation and the + * subsequent fs operation, a path component could theoretically be + * swapped for a symlink. This is an inherent limitation of user-space + * path validation on POSIX systems. For defense-in-depth, callers + * operating in hostile environments should use OS-level confinement + * (chroot, namespaces, or sandboxing) in addition to this check. + */ + private async assertSafePath(filePath: string): Promise { + if (!this.rootDir) return filePath; + + const resolved = this.rebase(resolve(this.rootDir, filePath)); + + // Check if resolved path is within rootDir + if (!this.pathStartsWith(resolved, this.rootDir + sep) && !this.pathEquals(resolved, this.rootDir)) { + throw new Error(`Path traversal blocked: ${filePath}`); + } + + // Check for symlink traversal + try { + const real = await realpath(resolved); + if (!this.pathStartsWith(real, this.rootDir + sep) && !this.pathEquals(real, this.rootDir)) { + throw new Error(`Symlink traversal blocked: ${filePath}`); + } + return real; + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') { + // Walk up to nearest existing ancestor and verify it resolves within rootDir + let checkPath = resolved; + while (!this.pathEquals(checkPath, this.rootDir)) { + checkPath = dirname(checkPath); + try { + const realAncestor = await realpath(checkPath); + if (!this.pathStartsWith(realAncestor, this.rootDir + sep) && !this.pathEquals(realAncestor, this.rootDir)) { + throw new Error(`Symlink traversal blocked: ${filePath}`); + } + return resolved; + } catch (ancestorErr: unknown) { + if ((ancestorErr as NodeJS.ErrnoException).code === 'ENOENT') continue; + throw ancestorErr; + } + } + return resolved; + } + throw err; + } + } + + private assertSafePathSync(filePath: string): string { + if (!this.rootDir) return filePath; + + const resolved = this.rebase(resolve(this.rootDir, filePath)); + + // Check if resolved path is within rootDir + if (!this.pathStartsWith(resolved, this.rootDir + sep) && !this.pathEquals(resolved, this.rootDir)) { + throw new Error(`Path traversal blocked: ${filePath}`); + } + + // Check for symlink traversal + try { + const real = realpathSync(resolved); + if (!this.pathStartsWith(real, this.rootDir + sep) && !this.pathEquals(real, this.rootDir)) { + throw new Error(`Symlink traversal blocked: ${filePath}`); + } + return real; + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') { + // Walk up to nearest existing ancestor and verify it resolves within rootDir + let checkPath = resolved; + while (!this.pathEquals(checkPath, this.rootDir)) { + checkPath = dirname(checkPath); + try { + const realAncestor = realpathSync(checkPath); + if (!this.pathStartsWith(realAncestor, this.rootDir + sep) && !this.pathEquals(realAncestor, this.rootDir)) { + throw new Error(`Symlink traversal blocked: ${filePath}`); + } + return resolved; + } catch (ancestorErr: unknown) { + if ((ancestorErr as NodeJS.ErrnoException).code === 'ENOENT') continue; + throw ancestorErr; + } + } + return resolved; + } + throw err; + } + } + + async createIfAbsent(filePath: string, data: string): Promise { + const safePath = await this.assertSafePath(filePath); + // Create parent dirs before attempting exclusive open so mkdir failure + // is distinguished from the key-already-exists conflict. + try { + await fsMkdir(dirname(safePath), { recursive: true }); + } catch (mkdirErr: unknown) { + throw new StateBackendUncertaintyError( + 'createIfAbsent', + `mkdir failed for "${filePath}": ${(mkdirErr as NodeJS.ErrnoException).code ?? 'UNKNOWN'}`, + ); + } + let fh: import('fs/promises').FileHandle | undefined; + try { + // 'wx' = O_WRONLY | O_CREAT | O_EXCL — fails atomically with EEXIST + // if the file already exists, with no race between check and create. + fh = await open(safePath, 'wx'); + } catch (openErr: unknown) { + const code = (openErr as NodeJS.ErrnoException).code; + // Windows can report a sharing violation instead of EEXIST for a racing creator. + let existingFile = false; + if (code === 'EACCES' || code === 'EPERM') { + try { + await access(safePath); + existingFile = true; + } catch { /* It may have disappeared after the exclusive open failed. */ } + } + if (code === 'EEXIST' || existingFile) { + throw new StateKeyConflictError(filePath); + } + throw new StateBackendUncertaintyError( + 'createIfAbsent', + `exclusive open failed for "${filePath}": ${code ?? 'UNKNOWN'}`, + ); + } + try { + await fh.writeFile(data, 'utf-8'); + } catch (writeErr: unknown) { + // Write failed after exclusive open: state is uncertain (file exists but + // may be empty or partial). Attempt cleanup; if cleanup fails, the + // uncertainty persists but we still surface the original error. + try { await unlink(safePath); } catch { /* best-effort cleanup */ } + throw new StateBackendUncertaintyError( + 'createIfAbsent', + `write failed after exclusive open for "${filePath}": ${(writeErr as NodeJS.ErrnoException).code ?? 'UNKNOWN'}`, + ); + } finally { + try { await fh.close(); } catch { /* best-effort */ } + } + } + + async read(filePath: string): Promise { + const safePath = await this.assertSafePath(filePath); + try { + return await readFile(safePath, 'utf-8'); + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return undefined; + throw new StorageError('read', filePath, err as NodeJS.ErrnoException); + } + } + + async write(filePath: string, data: string): Promise { + const safePath = await this.assertSafePath(filePath); + try { + await fsMkdir(dirname(safePath), { recursive: true }); + await writeFile(safePath, data, 'utf-8'); + } catch (err: unknown) { + throw new StorageError('write', filePath, err as NodeJS.ErrnoException); + } + } + + async append(filePath: string, data: string): Promise { + const safePath = await this.assertSafePath(filePath); + try { + await fsMkdir(dirname(safePath), { recursive: true }); + await appendFile(safePath, data, 'utf-8'); + } catch (err: unknown) { + throw new StorageError('append', filePath, err as NodeJS.ErrnoException); + } + } + + async exists(filePath: string): Promise { + const safePath = await this.assertSafePath(filePath); + try { + await access(safePath); + return true; + } catch { + return false; + } + } + + async list(dirPath: string): Promise { + const safePath = await this.assertSafePath(dirPath); + try { + return await readdir(safePath); + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return []; + throw new StorageError('list', dirPath, err as NodeJS.ErrnoException); + } + } + + async delete(filePath: string): Promise { + const safePath = await this.assertSafePath(filePath); + try { + await unlink(safePath); + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return; + throw new StorageError('delete', filePath, err as NodeJS.ErrnoException); + } + } + + readSync(filePath: string): string | undefined { + const safePath = this.assertSafePathSync(filePath); + try { + return readFileSync(safePath, 'utf-8'); + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return undefined; + throw new StorageError('read', filePath, err as NodeJS.ErrnoException); + } + } + + writeSync(filePath: string, data: string): void { + const safePath = this.assertSafePathSync(filePath); + try { + fsMkdirSync(dirname(safePath), { recursive: true }); + writeFileSync(safePath, data, 'utf-8'); + } catch (err: unknown) { + throw new StorageError('write', filePath, err as NodeJS.ErrnoException); + } + } + + existsSync(filePath: string): boolean { + const safePath = this.assertSafePathSync(filePath); + return fsExistsSync(safePath); + } + + listSync(dirPath: string): string[] { + const safePath = this.assertSafePathSync(dirPath); + try { + return readdirSync(safePath); + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return []; + throw new StorageError('list', dirPath, err as NodeJS.ErrnoException); + } + } + + async deleteDir(dirPath: string): Promise { + const safePath = await this.assertSafePath(dirPath); + try { + await rm(safePath, { recursive: true, force: true }); + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return; + throw new StorageError('deleteDir', dirPath, err as NodeJS.ErrnoException); + } + } + + // ── Phase 3 Wave 1 additions ──────────────────────────────────────────── + + async isDirectory(targetPath: string): Promise { + const safePath = await this.assertSafePath(targetPath); + try { + const s = await fsStat(safePath); + return s.isDirectory(); + } catch { + return false; + } + } + + async mkdir(dirPath: string, options?: { recursive?: boolean }): Promise { + const safePath = await this.assertSafePath(dirPath); + try { + await fsMkdir(safePath, { recursive: options?.recursive ?? false }); + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'EEXIST') return; + throw new StorageError('mkdir', dirPath, err as NodeJS.ErrnoException); + } + } + + async rename(oldPath: string, newPath: string): Promise { + const safeOld = await this.assertSafePath(oldPath); + const safeNew = await this.assertSafePath(newPath); + try { + await fsMkdir(dirname(safeNew), { recursive: true }); + await fsRename(safeOld, safeNew); + } catch (err: unknown) { + throw new StorageError('rename', oldPath, err as NodeJS.ErrnoException); + } + } + + async copy(srcPath: string, destPath: string): Promise { + const safeSrc = await this.assertSafePath(srcPath); + const safeDest = await this.assertSafePath(destPath); + try { + await fsMkdir(dirname(safeDest), { recursive: true }); + await copyFile(safeSrc, safeDest); + } catch (err: unknown) { + throw new StorageError('copy', srcPath, err as NodeJS.ErrnoException); + } + } + + async stat(targetPath: string): Promise { + const safePath = await this.assertSafePath(targetPath); + try { + const s = await fsStat(safePath); + return { size: s.size, mtimeMs: s.mtimeMs, isDirectory: s.isDirectory() }; + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return undefined; + throw new StorageError('stat', targetPath, err as NodeJS.ErrnoException); + } + } + + isDirectorySync(targetPath: string): boolean { + const safePath = this.assertSafePathSync(targetPath); + try { + return statSync(safePath).isDirectory(); + } catch { + return false; + } + } + + mkdirSync(dirPath: string, options?: { recursive?: boolean }): void { + const safePath = this.assertSafePathSync(dirPath); + try { + fsMkdirSync(safePath, { recursive: options?.recursive ?? false }); + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'EEXIST') return; + throw new StorageError('mkdir', dirPath, err as NodeJS.ErrnoException); + } + } + + statSync(targetPath: string): StorageStats | undefined { + const safePath = this.assertSafePathSync(targetPath); + try { + const s = statSync(safePath); + return { size: s.size, mtimeMs: s.mtimeMs, isDirectory: s.isDirectory() }; + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return undefined; + throw new StorageError('stat', targetPath, err as NodeJS.ErrnoException); + } + } + + appendSync(filePath: string, data: string): void { + const safePath = this.assertSafePathSync(filePath); + try { + fsMkdirSync(dirname(safePath), { recursive: true }); + appendFileSync(safePath, data, 'utf-8'); + } catch (err: unknown) { + throw new StorageError('append', filePath, err as NodeJS.ErrnoException); + } + } + + deleteSync(filePath: string): void { + const safePath = this.assertSafePathSync(filePath); + try { + unlinkSync(safePath); + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return; + throw new StorageError('delete', filePath, err as NodeJS.ErrnoException); + } + } + + renameSync(oldPath: string, newPath: string): void { + const safeOld = this.assertSafePathSync(oldPath); + const safeNew = this.assertSafePathSync(newPath); + try { + fsMkdirSync(dirname(safeNew), { recursive: true }); + fsRenameSync(safeOld, safeNew); + } catch (err: unknown) { + throw new StorageError('rename', oldPath, err as NodeJS.ErrnoException); + } + } + + copySync(srcPath: string, destPath: string): void { + const safeSrc = this.assertSafePathSync(srcPath); + const safeDest = this.assertSafePathSync(destPath); + try { + fsMkdirSync(dirname(safeDest), { recursive: true }); + copyFileSync(safeSrc, safeDest); + } catch (err: unknown) { + throw new StorageError('copy', srcPath, err as NodeJS.ErrnoException); + } + } + + deleteDirSync(dirPath: string): void { + const safePath = this.assertSafePathSync(dirPath); + try { + rmSync(safePath, { recursive: true, force: true }); + } catch (err: unknown) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return; + throw new StorageError('deleteDir', dirPath, err as NodeJS.ErrnoException); + } + } +} diff --git a/test/fs-storage-symlinked-root.test.ts b/test/fs-storage-symlinked-root.test.ts new file mode 100644 index 000000000..2acd1c7c7 --- /dev/null +++ b/test/fs-storage-symlinked-root.test.ts @@ -0,0 +1,58 @@ +/** + * A confined FSStorageProvider whose root is reached through a symlink (macOS `/tmp` is + * `/private/tmp`; a linked team root in remote mode). The provider stores its root resolved, but + * callers pass absolute paths in the spelling they were given. Those used to be rejected as + * "Path traversal blocked", which broke squad_state for such team roots (#2487 scenarios). + */ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { FSStorageProvider } from '../packages/squad-sdk/src/storage/fs-storage-provider.js'; + +let base: string; +let real: string; +let link: string; + +beforeEach(() => { + base = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'squad-symlinked-root-'))); + real = path.join(base, 'real-team'); + fs.mkdirSync(real); + fs.writeFileSync(path.join(real, 'team.md'), '# Team\n'); + link = path.join(base, 'team-link'); + fs.symlinkSync(real, link, process.platform === 'win32' ? 'junction' : 'dir'); + fs.writeFileSync(path.join(base, 'secret.txt'), 'outside\n'); +}); +afterEach(() => fs.rmSync(base, { recursive: true, force: true })); + +describe('FSStorageProvider with a symlinked root', () => { + it('reads and writes absolute paths spelled through the link', async () => { + const p = new FSStorageProvider(link); + expect(await p.read(path.join(link, 'team.md'))).toBe('# Team\n'); + expect(p.readSync(path.join(link, 'team.md'))).toBe('# Team\n'); + expect(p.existsSync(path.join(link, 'team.md'))).toBe(true); + await p.write(path.join(link, 'decisions', 'inbox', 'a.md'), 'x\n'); + expect(fs.readFileSync(path.join(real, 'decisions', 'inbox', 'a.md'), 'utf8')).toBe('x\n'); + // The root itself and relative paths keep working. + expect(await p.isDirectory(link)).toBe(true); + expect(await p.read('team.md')).toBe('# Team\n'); + }); + + it('still blocks paths that leave the root, in either spelling', async () => { + const p = new FSStorageProvider(link); + await expect(p.read(path.join(link, '..', 'secret.txt'))).rejects.toThrow(/traversal blocked/); + expect(() => p.readSync(path.join(link, '..', 'secret.txt'))).toThrow(/traversal blocked/); + await expect(p.read(path.join(real, '..', 'secret.txt'))).rejects.toThrow(/traversal blocked/); + // A sibling that only shares the link's name as a prefix is outside. + fs.mkdirSync(`${link}-evil`); + fs.writeFileSync(path.join(`${link}-evil`, 'x.md'), 'evil\n'); + await expect(p.read(path.join(`${link}-evil`, 'x.md'))).rejects.toThrow(/traversal blocked/); + }); + + it('still blocks a symlink inside the root that points outside it', async () => { + 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/); + }); +});