From b2881099d42f47290bd3247e568ca64f279bde2b Mon Sep 17 00:00:00 2001 From: Clay Good Date: Wed, 7 Oct 2026 07:59:11 -0500 Subject: [PATCH 1/3] fix(artifact-graph): reject brace nesting deeper than 16 levels braces <=3.0.3 (GHSA-vfj7-8cjw-p6xm, no patched release) can exhaust the stack on deeply nested patterns, and schema `generates` / `apply.tracks` globs reach it through fast-glob. Reject glob patterns nested more than 16 levels before any path or glob work, document the limit in docs-lab, add it to the artifact-graph spec, and update the pnpm audit exception note. Co-Authored-By: Claude Opus 5.5 --- .changeset/braces-nesting-guard.md | 7 +++++++ docs-lab/reference/schemas/schema-yaml.md | 4 ++-- openspec/specs/artifact-graph/spec.md | 7 +++++++ pnpm-workspace.yaml | 10 +++++----- src/core/artifact-graph/outputs.ts | 20 +++++++++++++++++++- test/core/artifact-graph/outputs.test.ts | 20 ++++++++++++++++++++ 6 files changed, 60 insertions(+), 8 deletions(-) create mode 100644 .changeset/braces-nesting-guard.md diff --git a/.changeset/braces-nesting-guard.md b/.changeset/braces-nesting-guard.md new file mode 100644 index 0000000000..fd5ecc5ba1 --- /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 more than 16 levels deep is now rejected with a clear error before glob matching, so a malformed schema cannot crash the CLI 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..3ae61e9f1d 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 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..d4e6ecd05e 100644 --- a/openspec/specs/artifact-graph/spec.md +++ b/openspec/specs/artifact-graph/spec.md @@ -57,6 +57,8 @@ The system SHALL recognize `generates` values containing `*`, `?`, or `[` as glo The system SHALL preserve literal filenames with a bare leading `!`, plain parentheses, or single-element braces when no supported glob syntax is present. Brace expansion SHALL preserve literal brace groups and recognize later and nested expansion groups. Expanded output paths and traversed symbolic links SHALL remain within the change directory. +The system SHALL reject a glob `generates` value that nests braces more than 16 levels deep, with an error naming the limit, before any glob matching runs. + #### Scenario: Simple file exists - **WHEN** an artifact generates "proposal.md" and the file exists - **THEN** the artifact is marked as completed @@ -65,6 +67,11 @@ The system SHALL preserve literal filenames with a bare leading `!`, plain paren - **WHEN** an artifact generates "proposal.md" and the file does not exist - **THEN** the artifact is not marked as completed +#### 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 + #### Scenario: Glob pattern with files - **WHEN** an artifact generates "specs/*.md" and the specs/ directory contains .md files - **THEN** the artifact is marked as completed diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index c1459699b6..7c2d7acaf8 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -31,10 +31,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 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..dba8db9035 100644 --- a/src/core/artifact-graph/outputs.ts +++ b/src/core/artifact-graph/outputs.ts @@ -5,6 +5,22 @@ 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 { + let depth = 0; + for (const char of pattern) { + if (char === '{') depth += 1; + else if (char === '}' && depth > 0) depth -= 1; + if (depth > MAX_BRACE_NESTING) { + throw new Error( + `Artifact output pattern nests braces more than ${MAX_BRACE_NESTING} levels deep: ${pattern}` + ); + } + } +} function hasBraceExpansion(pattern: string): boolean { const openings: number[] = []; @@ -115,9 +131,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..105165c443 100644 --- a/test/core/artifact-graph/outputs.test.ts +++ b/test/core/artifact-graph/outputs.test.ts @@ -272,6 +272,26 @@ 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' + ); + 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'); From 3066cac54d2f874ec66c4a6e5c5cee0ee6630f31 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 9 Oct 2026 09:05:29 -0500 Subject: [PATCH 2/3] fix(artifact-graph): count nesting the way the braces parser does The guard decremented on every `}` and ignored parentheses, but braces treats `[...]` and quoted runs as literal and nests `(` groups in the same tree it walks recursively. So `*{a,[}]...`, `*{a,"}"...`, `*{a,(}...` and `*{a,b}((((...))))` passed the 16-level guard and still nested arbitrarily deep inside braces. Mirror the parser's block stack instead, with tests for each bypass. Also: - Move the limit into its own requirement, so it no longer lengthens the State Detection requirement (which is already over the 500-char strict warning on main), and name apply.tracks too. - Reword the changeset: the guard stops the stack overflow from the advisory; it does not stop every malformed schema from crashing (wide sibling brace groups can still exhaust memory, a separate issue). Co-Authored-By: Claude Opus 5.5 --- .changeset/braces-nesting-guard.md | 2 +- docs-lab/reference/schemas/schema-yaml.md | 2 +- openspec/specs/artifact-graph/spec.md | 15 ++++++----- pnpm-workspace.yaml | 2 +- src/core/artifact-graph/outputs.ts | 31 +++++++++++++++++------ test/core/artifact-graph/outputs.test.ts | 15 +++++++++++ 6 files changed, 49 insertions(+), 18 deletions(-) diff --git a/.changeset/braces-nesting-guard.md b/.changeset/braces-nesting-guard.md index fd5ecc5ba1..759e1fbebf 100644 --- a/.changeset/braces-nesting-guard.md +++ b/.changeset/braces-nesting-guard.md @@ -4,4 +4,4 @@ ### Bug Fixes -- **Guard against deeply nested brace patterns** — An artifact `generates` or `apply.tracks` pattern that nests braces more than 16 levels deep is now rejected with a clear error before glob matching, so a malformed schema cannot crash the CLI through the unpatched `braces` advisory GHSA-vfj7-8cjw-p6xm. +- **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 3ae61e9f1d..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, paths containing a `..` segment, and globs that nest braces more than 16 levels deep, such as `{a,{b,{c,...}}}`. Real patterns need one or two levels. +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 diff --git a/openspec/specs/artifact-graph/spec.md b/openspec/specs/artifact-graph/spec.md index d4e6ecd05e..0d676e32d3 100644 --- a/openspec/specs/artifact-graph/spec.md +++ b/openspec/specs/artifact-graph/spec.md @@ -57,8 +57,6 @@ The system SHALL recognize `generates` values containing `*`, `?`, or `[` as glo The system SHALL preserve literal filenames with a bare leading `!`, plain parentheses, or single-element braces when no supported glob syntax is present. Brace expansion SHALL preserve literal brace groups and recognize later and nested expansion groups. Expanded output paths and traversed symbolic links SHALL remain within the change directory. -The system SHALL reject a glob `generates` value that nests braces more than 16 levels deep, with an error naming the limit, before any glob matching runs. - #### Scenario: Simple file exists - **WHEN** an artifact generates "proposal.md" and the file exists - **THEN** the artifact is marked as completed @@ -67,11 +65,6 @@ The system SHALL reject a glob `generates` value that nests braces more than 16 - **WHEN** an artifact generates "proposal.md" and the file does not exist - **THEN** the artifact is not marked as completed -#### 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 - #### Scenario: Glob pattern with files - **WHEN** an artifact generates "specs/*.md" and the specs/ directory contains .md files - **THEN** the artifact is marked as completed @@ -126,6 +119,14 @@ The system SHALL reject a glob `generates` value that nests braces more than 16 - **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 7c2d7acaf8..62b45dfe27 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -33,7 +33,7 @@ auditConfig: # patterns. Runtime (fast-glob > micromatch > braces), and no patched braces # exists, so no override can fix it. Schema artifact `generates` and # `apply.tracks` patterns reach fast-glob only through resolveArtifactOutputs, - # which rejects brace nesting deeper than 16 levels before globbing. Accepted + # 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). diff --git a/src/core/artifact-graph/outputs.ts b/src/core/artifact-graph/outputs.ts index dba8db9035..e3a9de04be 100644 --- a/src/core/artifact-graph/outputs.ts +++ b/src/core/artifact-graph/outputs.ts @@ -10,14 +10,29 @@ const BRACE_EXPANSION_SEPARATORS_RE = /,|\.\./u; const MAX_BRACE_NESTING = 16; function assertBraceNesting(pattern: string): void { - let depth = 0; - for (const char of pattern) { - if (char === '{') depth += 1; - else if (char === '}' && depth > 0) depth -= 1; - if (depth > MAX_BRACE_NESTING) { - throw new Error( - `Artifact output pattern nests braces more than ${MAX_BRACE_NESTING} levels deep: ${pattern}` - ); + // 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) { + throw new Error( + `Artifact output pattern nests braces more than ${MAX_BRACE_NESTING} levels deep: ${pattern}` + ); + } + } else if ((char === '}' && open.at(-1) === '{') || (char === ')' && open.at(-1) === '(')) { + open.pop(); } } } diff --git a/test/core/artifact-graph/outputs.test.ts b/test/core/artifact-graph/outputs.test.ts index 105165c443..c52fbf0db3 100644 --- a/test/core/artifact-graph/outputs.test.ts +++ b/test/core/artifact-graph/outputs.test.ts @@ -292,6 +292,21 @@ describe('artifact-graph/outputs', () => { 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'); From e1f4511c8981b6dd1f1f87c1fc6a9089dc14214c Mon Sep 17 00:00:00 2001 From: Clay Good Date: Wed, 14 Oct 2026 15:56:16 -0500 Subject: [PATCH 3/3] fix(artifact-graph): quote only the start of a rejected deep pattern A pattern deep enough to hit the guard can run to thousands of characters, and the error printed all of it. Co-Authored-By: Claude Opus 5.5 --- src/core/artifact-graph/outputs.ts | 4 +++- test/core/artifact-graph/outputs.test.ts | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/core/artifact-graph/outputs.ts b/src/core/artifact-graph/outputs.ts index e3a9de04be..c37153fc7e 100644 --- a/src/core/artifact-graph/outputs.ts +++ b/src/core/artifact-graph/outputs.ts @@ -27,8 +27,10 @@ function assertBraceNesting(pattern: string): void { } 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: ${pattern}` + `Artifact output pattern nests braces more than ${MAX_BRACE_NESTING} levels deep: ${shown}` ); } } else if ((char === '}' && open.at(-1) === '{') || (char === ')' && open.at(-1) === '(')) { diff --git a/test/core/artifact-graph/outputs.test.ts b/test/core/artifact-graph/outputs.test.ts index c52fbf0db3..edc9de74d3 100644 --- a/test/core/artifact-graph/outputs.test.ts +++ b/test/core/artifact-graph/outputs.test.ts @@ -288,6 +288,11 @@ describe('artifact-graph/outputs', () => { 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(); });