diff --git a/.changeset/scratch-dir-utility.md b/.changeset/scratch-dir-utility.md index d353b3c51..9fd5d5769 100644 --- a/.changeset/scratch-dir-utility.md +++ b/.changeset/scratch-dir-utility.md @@ -6,6 +6,5 @@ Add `.squad/.scratch/` directory for organized temp file management (#790) - `scratchDir()` — resolve and optionally create the scratch directory - `scratchFile()` — create named temp files inside `.scratch/` -- Path traversal protection: prefix/ext sanitized to prevent `../` attacks -- Monotonic counter ensures unique filenames even within the same millisecond +- Init scaffolds `.squad/.scratch/` and adds it to `.gitignore` - Agents and CLI should use these utilities instead of writing temp files to repo root diff --git a/packages/squad-sdk/src/resolution.ts b/packages/squad-sdk/src/resolution.ts index b2ec13dd0..217a4f1ae 100644 --- a/packages/squad-sdk/src/resolution.ts +++ b/packages/squad-sdk/src/resolution.ts @@ -14,6 +14,7 @@ import path from 'node:path'; import os from 'node:os'; +import crypto from 'node:crypto'; import { FSStorageProvider } from './storage/fs-storage-provider.js'; const storage = new FSStorageProvider(); @@ -499,10 +500,10 @@ export function scratchDir(squadRoot: string, create: boolean = true): string { /** * Return a unique file path inside the scratch directory. * - * Returns the absolute path to the file. Caller is responsible for writing - * content to the returned path (unless `content` is provided, in which case - * it is written immediately). The caller is also responsible for deleting - * the file when done (or relying on the cleanup capability). + * Writes content to the file if `content` is provided; otherwise returns + * the path only and the caller is responsible for writing to it. + * The caller is also responsible for deleting the file when done + * (or relying on the cleanup capability). * * @param squadRoot - Absolute path to the `.squad/` directory. * @param prefix - Filename prefix (e.g. `"fleet-prompt"`). @@ -510,27 +511,17 @@ export function scratchDir(squadRoot: string, create: boolean = true): string { * @param content - Optional content to write immediately. * @returns Absolute path to the temp file. */ -let _scratchCounter = 0; -let _scratchLastTs = 0; - export function scratchFile(squadRoot: string, prefix: string, ext: string = '.tmp', content?: string): string { - // Sanitize prefix/ext to prevent path traversal via '../' sequences - const safePrefix = prefix.replace(/[\/\\]/g, '_'); + // Sanitize prefix to prevent path traversal — strip directory components + const safePrefix = path.basename(prefix); const safeExt = ext.replace(/[\/\\]/g, '_'); const dir = scratchDir(squadRoot); - // Monotonic counter: if two calls happen in the same millisecond, - // the counter increments to guarantee unique filenames. const now = Date.now(); - if (now === _scratchLastTs) { - _scratchCounter++; - } else { - _scratchCounter = 0; - _scratchLastTs = now; - } + const rand = crypto.randomBytes(4).toString('hex'); - const filename = `${safePrefix}-${now}-${_scratchCounter}${safeExt}`; + const filename = `${safePrefix}-${now}-${rand}${safeExt}`; const filePath = path.join(dir, filename); if (content !== undefined) { storage.writeSync(filePath, content); diff --git a/test/scratch-dir.test.ts b/test/scratch-dir.test.ts index 516584168..e93d23cb9 100644 --- a/test/scratch-dir.test.ts +++ b/test/scratch-dir.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { scratchDir, scratchFile } from '@bradygaster/squad-sdk'; import { mkdirSync, rmSync, existsSync, readFileSync } from 'node:fs'; import path from 'node:path'; @@ -39,7 +39,7 @@ describe('scratchFile', () => { it('creates a temp file with prefix and default .tmp extension', () => { const filePath = scratchFile(SQUAD_ROOT, 'test-prompt'); expect(filePath).toContain('.scratch'); - expect(filePath).toMatch(/test-prompt-\d+-\d+\.tmp$/); + expect(filePath).toMatch(/test-prompt-\d+-[0-9a-f]{8}\.tmp$/); // File exists only if content was provided — otherwise just returns path expect(existsSync(path.dirname(filePath))).toBe(true); }); @@ -53,14 +53,14 @@ describe('scratchFile', () => { it('uses custom extension', () => { const filePath = scratchFile(SQUAD_ROOT, 'fleet', '.md'); - expect(filePath).toMatch(/fleet-\d+-\d+\.md$/); + expect(filePath).toMatch(/fleet-\d+-[0-9a-f]{8}\.md$/); }); it('generates unique filenames on successive calls', () => { const a = scratchFile(SQUAD_ROOT, 'dup', '.txt', 'a'); const b = scratchFile(SQUAD_ROOT, 'dup', '.txt', 'b'); - // No delay needed: the monotonic counter inside scratchFile guarantees - // unique filenames even when Date.now() returns the same millisecond value. + // Random hex suffix from crypto.randomBytes guarantees uniqueness + // even when Date.now() returns the same millisecond value. expect(a).not.toBe(b); expect(existsSync(a)).toBe(true); expect(existsSync(b)).toBe(true);