Skip to content

fix: stop deeply nested patterns from overflowing the stack - #79

Open
stefanstankovic1 wants to merge 2 commits into
micromatch:masterfrom
stefanstankovic1:fix/nesting-depth
Open

stefanstankovic1 wants to merge 2 commits into
micromatch:masterfrom
stefanstankovic1:fix/nesting-depth

Conversation

@stefanstankovic1

Copy link
Copy Markdown

maxLength is documented as the option to use when users can pass in patterns, but the
default of 10,000 doesn't quite cover nesting. This overflows the stack on Node 20:

braces('{'.repeat(3500) + 'a' + '}'.repeat(3500)) // ~7,000 chars
// RangeError: Maximum call stack size exceeded

The exact point varies by Node version and stack size, so there's no maxLength default
that's reliably safe.

This PR has the parser (which is already iterative) stop nesting after 256 levels and keep
anything deeper as plain text. Some notes:

  • Nothing new is thrown, so micromatch, fast-glob, chokidar etc. don't need to change.
  • compile, expand and stringify aren't touched. Output for anything under the limit is
    the same as on master. I compared them on a large set of random patterns with the common
    options and they matched.
  • options.maxDepth can lower the limit, not raise it, same as maxLength.
  • It doesn't try to do anything about regex complexity. It's only about the recursion
    inside braces itself.

The overflow tests in  test/nesting-depth.js  hit the  RangeError  on master; the rest cover the new option and check that normal output doesn't change. The suite passes on Node 8, 14 and 22.

cursor Bot pushed a commit to Ivano-Technologies/kompleet that referenced this pull request Oct 6, 2026
GHSA-vfj7-8cjw-p6xm (braces) and GHSA-86w9-cpqp-85rv (node-forge) have
no installable npm releases: registry latest is still braces@3.0.3 and
node-forge@1.4.0. Vendor patched workspace copies (depth guard from
micromatch/braces#79; DigestAlgorithm length check from
digitalbazaar/forge#1152) and pin them with pnpm.overrides.
Kezi3 added a commit to Ivano-Technologies/kompleet that referenced this pull request Oct 6, 2026
* docs: drop check-schema-drift from IVA-7 required checks

* ci: retire Supabase schema-drift and security-advisors gates

* fix(deps): patch braces 3.0.4 and node-forge 1.4.1 for audit-ci

GHSA-vfj7-8cjw-p6xm (braces) and GHSA-86w9-cpqp-85rv (node-forge) have
no installable npm releases: registry latest is still braces@3.0.3 and
node-forge@1.4.0. Vendor patched workspace copies (depth guard from
micromatch/braces#79; DigestAlgorithm length check from
digitalbazaar/forge#1152) and pin them with pnpm.overrides.

* fix(ci): exclude vendored packages from gitleaks and eslint

secret-scan flagged forge.pbe.generatePkcs12Key in packages/node-forge
(API name, not a secret). lint was applying project rules to upstream
braces/forge sources. Allowlist/ignore packages/ so the workspace
overrides stay in place without rewriting vendor style.

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
@jonschlinkert

Copy link
Copy Markdown
Member

Comment thread lib/parse.js Outdated
Comment on lines +88 to +89
} else if (value === CHAR_RIGHT_CURLY_BRACE || value === CHAR_RIGHT_PARENTHESES) {
literalDepth--;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The literal nesting counter treats any closing ) or } as the end of the current level, so a literal character can prematurely return control to the normal parser and corrupt the surrounding alternatives. For example, braces.expand('{{a),b},c}', { maxDepth: 1 }) currently returns ['{a),c}', 'b,c}']; keeping the inner brace group literal should return ['{a),b}', 'c']. A closing brace inside a quoted string or character class has the same effect (for example {{[}],a},b}). This also occurs at the default limit when the example is wrapped in 255 pairs of parentheses.

Please preserve matching delimiter types and quoted/bracketed spans while skipping the deeper group, and add regressions that check the expanded values and following alternatives.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, good catch! Counter ignored delimiter type entirely. I've reworked it so groups past the limit are now parsed normally (this does include quotes, brackets, escapes, to be clear matching closers all apply as usual) and each one is collapsed back to its source text when it closes.
An unclosed one turns the rest of the input into text. Added your examples plus a quoted } case wrapped in 255 parens so it's checked at the default limit too.

jkim323 added a commit to akhil9tiet/hve-core that referenced this pull request Oct 14, 2026
# Pull Request

## Description

Resolves two of the three OSV advisories that OpenSSF Scorecard reports,
using public patched releases, and records the fixes in the rolling
OpenVEX document. The third advisory, for braces, still has no patched
release, so its existing exception stays in place.

**Dependency floors**

* PowerPoint skill: `cairosvg` now requires `>=2.9.1`, and `uv.lock`
resolves 2.9.1. This clears
[GHSA-c3jg-qh8m-j3h2](https://osv.dev/GHSA-c3jg-qh8m-j3h2), a crafted
SVG `<path>` denial of service. The skill renders sanitized SVG through
`cairosvg.svg2png`, so this code path is reachable.
* Moderation eval: adds an explicit `fsspec>=2026.6.0` floor, and
`uv.lock` resolves 2026.9.0. This clears
[GHSA-27vj-qcqg-25rc](https://osv.dev/GHSA-27vj-qcqg-25rc). fsspec is
transitive through `torch` and `huggingface-hub`, and the direct floor
matches existing security-floor practice in other skill manifests. The
`torch`, `transformers`, `huggingface-hub`, and `detoxify` versions do
not change. uv only normalized platform markers on the existing `torch`
dependency entries.

**OpenVEX**

* `security/vex/hve-core.openvex.json` moves to version 10 with a new
`@id` and issuance timestamps. It adds `fixed` statements for
`pkg:pypi/cairosvg@2.9.1` and `pkg:pypi/fsspec@2026.9.0`.
* All 36 existing statements, including their timestamps, are unchanged.
This includes the `not_affected` statement for braces.

**braces ([GHSA-vfj7-8cjw-p6xm](https://osv.dev/GHSA-vfj7-8cjw-p6xm)) is
intentionally unchanged**

* braces 3.0.3 is still the latest release, and the advisory lists no
patched version. Current `micromatch`, `markdownlint-cli2`, and
Docusaurus releases still resolve 3.0.3.
* The upstream fix (micromatch/braces#79) is open but unpublished. This
PR does not pin an unpublished commit or accept npm audit's suggested
`markdownlint-cli2` downgrade.
* The `audit-ci.json` allowlist, npm lockfiles, and VEX statement are
unchanged, so Scorecard will keep reporting this advisory until a fix is
released.

## Related Issue(s)

Fixes microsoft#3182

Related to microsoft#3109
Related to microsoft#3147

## Type of Change

Select all that apply:

**Code & Documentation:**

* [x] Bug fix (non-breaking change fixing an issue)
* [ ] New feature (non-breaking change adding functionality)
* [ ] Breaking change (fix or feature causing existing functionality to
change)
* [ ] Documentation update

**Infrastructure & Configuration:**

* [ ] GitHub Actions workflow
* [ ] Linting configuration (markdown, PowerShell, etc.)
* [x] Security configuration
* [ ] DevContainer configuration
* [x] Dependency update

**AI Artifacts:**

* [ ] Reviewed contribution with `hve-builder` and addressed all
actionable findings
* [ ] Copilot instructions (`.github/instructions/*.instructions.md`)
* [ ] Copilot prompt (`.github/prompts/*.prompt.md`)
* [ ] Copilot agent (`.github/agents/*.agent.md`)
* [ ] Copilot skill (`.github/skills/*/SKILL.md`)
* [ ] Copilot hook (`.github/hooks/*/*.json`)
* [ ] Eval spec added/updated for changed AI artifacts (`evals/`)

> Note for AI Artifact Contributors:
>
> * Agents: Research, indexing/referencing other project (using standard
VS Code GitHub Copilot/MCP tools), planning, and general implementation
agents likely already exist. Review `.github/agents/` before creating
new ones.
> * Skills: Must include both bash and PowerShell scripts. See
[Skills](../docs/contributing/skills.md).
> * Model Versions: Contributions **MUST** target models listed in the
model catalog (`scripts/linting/model-catalog.json`) whose provider
appears in `providerAllowlist` and whose status is `ga` or `preview`.
Run `npm run lint:models` to validate references.
> * See [Agents Not
Accepted](../docs/contributing/custom-agents.md#agents-not-accepted) and
[Model Version
Requirements](../docs/contributing/ai-artifacts-common.md#model-version-requirements).

**Other:**

* [ ] Script/automation (`.ps1`, `.sh`, `.py`)
* [ ] Other (please describe):

## Testing

Commands ran locally against identical content for the five changed
files. The PowerPoint and moderation code and tests are byte-identical
between the tested tree and this branch's base.

* `uv lock --check` in both the PowerPoint skill and moderation
projects: passed
* `uv run pytest tests/test_extract_content.py` (PowerPoint): 140 passed
and 2 skipped. The skips are unrelated, and the real CairoSVG conversion
test passed.
* `uv run pytest tests/test_moderate.py` (moderation): 10 passed
* `uv run ruff check .` and `uv run ruff format --check .` in both
changed projects: passed
* `npm run lint:public-dependency-feeds`: passed, with 12,960 sources
across 70 files and no violations
* `npm run validate:skills`: passed for 87 skills with no errors or
warnings
* `pip-audit` on the exported PowerPoint lock: no known vulnerabilities
* OSV package queries for `cairosvg 2.9.1` and `fsspec 2026.9.0`: no
vulnerabilities returned
* `npm run audit:npm` (root and Docusaurus): passed. Only the existing
GHSA-vfj7-8cjw-p6xm allowlist entry matched.
* `pwsh scripts/linting/Invoke-JsonLint.ps1 -Paths
security/vex/hve-core.openvex.json`: passed
* A bounded OpenVEX conformance check confirmed:
  * required document and statement fields are present
  * the version increased by exactly one
  * both fixed statements use exact PURLs
  * all 36 prior statements are preserved
* `git diff --check origin/main...HEAD`: clean

Not run locally or not passing:

* Moderation `pip-audit` was unavailable locally because `pip-audit`
cannot resolve the public PyTorch `torch==2.14.0+cpu` build. The
reusable pip-audit and moderation CI lanes should cover it.
* `npm run lint:py` failed on content outside the changed projects,
mainly an ignored local `.survey-corpus/` directory. Both changed
projects pass direct ruff checks.

## Checklist

### Required Checks

* [ ] Documentation is updated (if applicable)
* [x] Files follow existing naming conventions
* [x] Changes are backwards compatible (if applicable)
* [ ] Tests added for new functionality (if applicable)

### AI Artifact Contributions
<!-- If contributing an agent, prompt, instruction, or skill, complete
these checks -->
* [ ] Used `hve-builder` review mode to review contribution
* [ ] Addressed all actionable findings from the `hve-builder` review
* [ ] Verified contribution follows common standards and type-specific
requirements

### Required Local Checks

The following local-safe validation commands must pass before merging:

* [ ] Local validation aggregate: `npm run validate:local`
* [ ] Documentation validation (if docs changed): `npm run
validate:docs`
* [ ] Spell checking: `npm run spell-check`
* [ ] Link validation: `npm run lint:md-links`

## Security Considerations
<!-- ⚠️ WARNING: Do not commit sensitive information such as API keys,
passwords, or personal data -->
* [ ] This PR does not contain any sensitive or NDA information
* [x] Any new dependencies have been reviewed for security issues
* [ ] Security-related scripts follow the principle of least privilege

## Additional Notes

* No new packages are introduced. `fsspec` was already in the moderation
lock as a transitive dependency. The new manifest entry only enforces a
minimum version.
* VEX statements are AI-drafted. Under the repository's VEX
author-of-record contract, the merging maintainer is the accountable
author, and the `vex-attest` workflow provides Sigstore attestation at
release.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Jamie Kim <jamiekim500@gmail.com>
Co-authored-by: William Berry <4399970+WilliamBerryiii@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants