diff --git a/.changeset/braces-nesting-guard.md b/.changeset/braces-nesting-guard.md new file mode 100644 index 0000000000..759e1fbebf --- /dev/null +++ b/.changeset/braces-nesting-guard.md @@ -0,0 +1,7 @@ +--- +"@fission-ai/openspec": patch +--- + +### Bug Fixes + +- **Guard against deeply nested brace patterns** — An artifact `generates` or `apply.tracks` pattern that nests braces or parentheses more than 16 levels deep is now rejected with a clear error before glob matching, so a schema cannot overflow the stack through the unpatched `braces` advisory GHSA-vfj7-8cjw-p6xm. diff --git a/docs-lab/reference/schemas/schema-yaml.md b/docs-lab/reference/schemas/schema-yaml.md index 08a06c92a0..8623f33d73 100644 --- a/docs-lab/reference/schemas/schema-yaml.md +++ b/docs-lab/reference/schemas/schema-yaml.md @@ -84,7 +84,7 @@ OpenSpec recognizes these glob forms in `generates`: **Literal filenames**: a leading `!` alone does not make a glob. Use `generates: '!review.md'` to name that file. Plain parentheses such as `(proposal|design).md` and single-element braces such as `review-{api}.md` also remain literal. -OpenSpec rejects absolute paths and paths containing a `..` segment. +OpenSpec rejects absolute paths, paths containing a `..` segment, and globs that nest braces or parentheses more than 16 levels deep, such as `{a,{b,{c,...}}}`. Real patterns need one or two levels. #### Completion @@ -140,7 +140,7 @@ The path starts from the change folder. For a change named `add-auth`, `tracks: openspec/changes/add-auth/tasks.md ``` -A glob such as `tracks: "**/tasks.md"` reads every matching file, such as `backend/tasks.md` and `frontend/tasks.md`. OpenSpec combines their tasks and progress. Use the same value for an artifact's `generates` field so status and list track the same files. +A glob such as `tracks: "**/tasks.md"` reads every matching file, such as `backend/tasks.md` and `frontend/tasks.md`. OpenSpec combines their tasks and progress. Use the same value for an artifact's `generates` field so status and list track the same files. The 16-level brace limit from `generates` applies here too. Apply stays blocked if no file matches or the matched files contain no checkbox with task text. OpenSpec counts these checkbox forms: diff --git a/openspec/specs/artifact-graph/spec.md b/openspec/specs/artifact-graph/spec.md index c98f448c1f..0d676e32d3 100644 --- a/openspec/specs/artifact-graph/spec.md +++ b/openspec/specs/artifact-graph/spec.md @@ -119,6 +119,14 @@ The system SHALL preserve literal filenames with a bare leading `!`, plain paren - **AND** "content/linked" is a symbolic link to a directory outside the change directory - **THEN** output resolution rejects traversal through that link even when no matching files exist +### Requirement: Glob Nesting Limit +The system SHALL reject a glob `generates` or `apply.tracks` value that nests braces or parentheses more than 16 levels deep, counting them as the brace parser does, with an error naming the limit, before any glob matching runs. + +#### Scenario: Brace nesting beyond the limit +- **WHEN** an artifact generates a pattern whose braces nest 17 levels deep +- **THEN** resolving its outputs fails with an error naming the 16-level limit +- **AND** no glob matching runs for that pattern + ### Requirement: Ready Artifact Query The system SHALL identify which artifacts are ready to be created based on dependency completion. diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 21a1e2c0f2..bed8666f0e 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -36,10 +36,10 @@ auditConfig: ignoreGhsas: # GHSA-vfj7-8cjw-p6xm — braces <=3.0.3 stack exhaustion on deeply nested # patterns. Runtime (fast-glob > micromatch > braces), and no patched braces - # exists, so no override can fix it. Accepted residual risk: schema artifact - # `generates` and `apply.tracks` patterns reach fast-glob through - # resolveArtifactOutputs without a brace-depth guard, and the 10,000-character - # input limit does not prevent stack exhaustion. A crafted project-local or - # user schema can still terminate the openspec process. Remove this exception + # exists, so no override can fix it. Schema artifact `generates` and + # `apply.tracks` patterns reach fast-glob only through resolveArtifactOutputs, + # which rejects brace or parenthesis nesting deeper than 16 levels before globbing. Accepted + # residual risk: braces itself stays unpatched, so any new caller that passes + # it unchecked patterns is exposed. Remove this exception # once braces ships a patched release (check: pnpm why braces; pnpm audit --prod). - GHSA-vfj7-8cjw-p6xm diff --git a/src/core/artifact-graph/outputs.ts b/src/core/artifact-graph/outputs.ts index 1ad4917e76..c37153fc7e 100644 --- a/src/core/artifact-graph/outputs.ts +++ b/src/core/artifact-graph/outputs.ts @@ -5,6 +5,39 @@ import { FileSystemUtils } from '../../utils/file-system.js'; const EXTGLOB_RE = /[!*+?@]\([^(]*\)/u; const BRACE_EXPANSION_SEPARATORS_RE = /,|\.\./u; +// braces <=3.0.3 (fast-glob > micromatch) has no depth guard (GHSA-vfj7-8cjw-p6xm), +// so reject nesting no real artifact pattern needs before it reaches the parser. +const MAX_BRACE_NESTING = 16; + +function assertBraceNesting(pattern: string): void { + // Mirror the braces parser: `[...]` and quoted runs are literal, `(` nests like + // `{`, and a closer only closes the innermost group of its own kind. + const open: string[] = []; + for (let index = 0; index < pattern.length; index += 1) { + const char = pattern[index]; + if (char === '[') { + for (let brackets = 1; brackets > 0 && index + 1 < pattern.length; ) { + index += 1; + if (pattern[index] === '[') brackets += 1; + else if (pattern[index] === ']') brackets -= 1; + } + } else if (char === '"' || char === "'" || char === '`') { + const end = pattern.indexOf(char, index + 1); + index = end === -1 ? pattern.length : end; + } else if (char === '{' || char === '(') { + open.push(char); + if (open.length > MAX_BRACE_NESTING) { + // A pattern this deep can run to thousands of characters; quote only its start. + const shown = pattern.length > 80 ? `${pattern.slice(0, 80)}...` : pattern; + throw new Error( + `Artifact output pattern nests braces more than ${MAX_BRACE_NESTING} levels deep: ${shown}` + ); + } + } else if ((char === '}' && open.at(-1) === '{') || (char === ')' && open.at(-1) === '(')) { + open.pop(); + } + } +} function hasBraceExpansion(pattern: string): boolean { const openings: number[] = []; @@ -115,9 +148,11 @@ function assertGlobDirectoryTraversal( * Returns absolute file paths. Glob matches are sorted for deterministic output. */ export function resolveArtifactOutputs(changeDir: string, generates: string): string[] { + const isGlob = isGlobPattern(generates); + if (isGlob) assertBraceNesting(FileSystemUtils.toPosixPath(generates)); const outputPath = resolveArtifactOutputPath(changeDir, generates); - if (!isGlobPattern(generates)) { + if (!isGlob) { try { return fs.statSync(outputPath).isFile() ? [FileSystemUtils.canonicalizeExistingPath(outputPath)] diff --git a/test/core/artifact-graph/outputs.test.ts b/test/core/artifact-graph/outputs.test.ts index e79898d450..edc9de74d3 100644 --- a/test/core/artifact-graph/outputs.test.ts +++ b/test/core/artifact-graph/outputs.test.ts @@ -272,6 +272,46 @@ describe('artifact-graph/outputs', () => { expect(artifactOutputExists(tempDir, 'file-{1..3}.md')).toBe(true); }); + it('rejects brace nesting deeper than any artifact pattern needs', () => { + const fileA = path.join(tempDir, 'a.md'); + fs.writeFileSync(fileA, 'content'); + const nested = (depth: number) => `${'{a,'.repeat(depth)}b${'}'.repeat(depth)}.md`; + const globSpy = vi.spyOn(fg, 'generateTasks'); + + expect(resolveArtifactOutputs(tempDir, nested(16))).toEqual([canonical(fileA)]); + globSpy.mockClear(); + expect(() => resolveArtifactOutputs(tempDir, nested(17))).toThrow( + 'Artifact output pattern nests braces more than 16 levels deep' + ); + // A pattern deep enough to crash braces is also too long for the filesystem; + // the guard must still be the error the user sees. + expect(() => resolveArtifactOutputs(tempDir, nested(4000))).toThrow( + 'Artifact output pattern nests braces more than 16 levels deep' + ); + // The error quotes only the start of a pattern that long. + expect(() => resolveArtifactOutputs(tempDir, nested(4000))).toThrow( + `16 levels deep: ${nested(4000).slice(0, 80)}...` + ); + expect(() => resolveArtifactOutputs(tempDir, nested(4000))).not.toThrow(nested(4000)); + expect(globSpy).not.toHaveBeenCalled(); + globSpy.mockRestore(); + }); + + it.each([ + ['a bracket', (depth: number) => `*${'{a,[}]'.repeat(depth)}${'}'.repeat(depth)}`], + ['a quote', (depth: number) => `*${'{a,"}"'.repeat(depth)}${'}'.repeat(depth)}`], + ['an open parenthesis', (depth: number) => `*${'{a,(}'.repeat(depth)}${')}'.repeat(depth)}`], + ['parentheses alone', (depth: number) => `*{a,b}${'('.repeat(depth)}${')'.repeat(depth)}`], + ])('counts nesting the braces parser sees past %s', (_label, nested) => { + const globSpy = vi.spyOn(fg, 'generateTasks'); + + expect(() => resolveArtifactOutputs(tempDir, nested(17))).toThrow( + 'Artifact output pattern nests braces more than 16 levels deep' + ); + expect(globSpy).not.toHaveBeenCalled(); + globSpy.mockRestore(); + }); + it.each(['@(proposal|design).md', '+(proposal|design).md'])('supports extglob %s', (pattern) => { const proposalPath = path.join(tempDir, 'proposal.md'); fs.writeFileSync(proposalPath, 'content');