Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/braces-nesting-guard.md
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 2 additions & 2 deletions docs-lab/reference/schemas/schema-yaml.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:

Expand Down
8 changes: 8 additions & 0 deletions openspec/specs/artifact-graph/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
10 changes: 5 additions & 5 deletions pnpm-workspace.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
37 changes: 36 additions & 1 deletion src/core/artifact-graph/outputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

function hasBraceExpansion(pattern: string): boolean {
const openings: number[] = [];
Expand Down Expand Up @@ -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)]
Expand Down
40 changes: 40 additions & 0 deletions test/core/artifact-graph/outputs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
Loading