Skip to content

fix: preflight AST walkers and derive expansion queues from traversal - #82

Open
Passpass92 wants to merge 5 commits into
micromatch:masterfrom
Passpass92:fix/bounded-ast-preflight
Open

Passpass92 wants to merge 5 commits into
micromatch:masterfrom
Passpass92:fix/bounded-ast-preflight

Conversation

@Passpass92

@Passpass92 Passpass92 commented Oct 7, 2026 •

Copy link
Copy Markdown

Deep brace/parenthesis inputs and caller-created ASTs can overflow recursive walkers. This change adds iterative structural preflight, prevents foreign parent queues from entering expansion, and preserves range arguments across shared/repeated AST traversal.

Current head: 1f11eb558be9ea0cda87861408bb766e2e714086.

Changes:

  • Reject excessive nesting, child cycles and more than 65,536 AST visits before recursive processing. Parser container depth and AST child-edge depth are capped at 128; terminal child nodes are counted separately. Violations use SyntaxError / ERR_BRACES_COMPLEXITY.
  • F1: expansion derives queues from the actual child traversal, without reading parent metadata or consuming external queues.
  • Shared expansion P2: range normalization uses local type/value copies and retains original children, so every shared occurrence is expanded.
  • R1 (22a70381bdc89126033155fc00e5ce58f10f7776): fixed-position range-limit checking with inclusive cardinality, absolute direction/step normalization, character ranges and fill-range step precedence. Manual-AST function/options steps and boxed options steps are handled. Invalid ranges/steps retain dependency fallback/errors. false/Infinity still disable the size cap. Unsafe numeric endpoints are rejected even without a cap, because floating-point increments can stop making progress.
  • R2 (1f11eb558be9ea0cda87861408bb766e2e714086): compilation normalizes local copies, preserving range markers for shared subtrees, repeated compile and mixed compile/expand sequences. No result cache or parent traversal is introduced.

Validation, performed by the implementer:

  • R1: 129 targeted tests; final negative control on f6e4d5d gives 54 passing / 75 failing. A second targeted RED caught nine manual/boxed-step cases on the provisional R1 before they were corrected.
  • R1 standalone: 1,115 tests pass on macOS Node 22.23.2 and Linux Node 8.3.0 / 20.20.2 / 22.23.2.
  • R2: 22 targeted tests, with 2 passing / 20 failing before the change; all pass after it, including with a reduced stack.
  • Final full suite: 1,137 tests passing on all four environments. Linux images are pinned by digest, networking is disabled, and source mounts are read-only. Dependencies remain fill-range 7.1.1 and Mocha 6.2.3.
  • R1 ordinary-expression corpus: 448,952 comparisons against f6e4d5d, with 444,640 identical, 3,136 previously permitted over-limit expansions now rejected, and 1,176 previously erroneous rejections now accepted. Every raw difference is preserved and adjudicated using bounded reference generation with the actual fill-range dependency; no unexplained differences. The raw generic comparator returns 1 for these intended policy changes, not a misleading blanket PASS.
  • R2 corpus: all 448,952 comparisons against the final R1 are identical. Each corpus run also includes 3,000 unchanged poisoned-parent comparisons.
  • Range oracle: 41,184 checks with a normal stack and 41,184 with a reduced stack, comparing the guard/output/error behavior with bounded real fill-range generation across endpoints, step placements, strictness and limits.
  • Final reruns: 144 structural/F1 probes and 86 shared-expansion probes pass at each of two stack sizes.
  • Offline npm pack dry-run, git diff checks and redacted full-patch Gitleaks checks pass. Package name/version (3.0.3), dependency declarations and Node >=8.3 remain unchanged.

Independent review previously closed F1 and the shared-expansion P2 on f6e4d5d. Independent review of the two new commits/current head is pending. Current reruns are not presented as a fresh independent review, maintainer approval, CI qualification or npm publication.

Scope: rangeLimit bounds each expansion range, not Cartesian-product cardinality or compiled stepped-regex generation. Structural bounds do not qualify arbitrary getters/proxies, array-valued text nodes, or generated-regex execution time. Work queues and normalization metadata can still change; only range arguments/children are preserved by the relevant branches. Standalone brace nodes without an expansion container remain unsupported. No downstream deployment or alert closure is implied.

@Passpass92 Passpass92 changed the title fix: preflight recursive AST walkers and bound parent traversal fix: preflight AST walkers and derive expansion queues from traversal Oct 7, 2026
ashlessscythe added a commit to ashlessscythe/supli that referenced this pull request Oct 8, 2026
* Patch Dependabot/npm audit: Next 15.5.27, sharp 0.35.5, braces backport.

Upgrade next/eslint-config-next to 15.5.27 for RCE and cache-poisoning
fixes, sharp to 0.35.5, vitest to 4.1.11, and pin transitive overrides
(brace-expansion, js-yaml, source-map-js, postcss-selector-parser).

Vendor braces 3.0.4 from micromatch/braces#82 until upstream publishes a
release past 3.0.3 for GHSA-vfj7-8cjw-p6xm.

Co-authored-by: Tony <ashlessscythe@users.noreply.github.com>

* Sync package-lock for npm ci without legacy-peer-deps.

Regenerate the lockfile so CI's plain `npm ci` stays in sync and
picomatch nesting resolves cleanly.

Co-authored-by: Tony <ashlessscythe@users.noreply.github.com>

* Include vendor braces tarball in Docker npm ci stages.

The deps stage only copied package manifests, so `npm ci` could not
resolve the local braces file: override. Copy vendor into deps and
runner images.

Co-authored-by: Tony <ashlessscythe@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Tony <ashlessscythe@users.noreply.github.com>
ashlessscythe added a commit to ashlessscythe/supli that referenced this pull request Oct 8, 2026
)

* Patch Dependabot/npm audit: Next 15.5.27, sharp 0.35.5, braces backport.

Upgrade next/eslint-config-next to 15.5.27 for RCE and cache-poisoning
fixes, sharp to 0.35.5, vitest to 4.1.11, and pin transitive overrides
(brace-expansion, js-yaml, source-map-js, postcss-selector-parser).

Vendor braces 3.0.4 from micromatch/braces#82 until upstream publishes a
release past 3.0.3 for GHSA-vfj7-8cjw-p6xm.



* Sync package-lock for npm ci without legacy-peer-deps.

Regenerate the lockfile so CI's plain `npm ci` stays in sync and
picomatch nesting resolves cleanly.



* Include vendor braces tarball in Docker npm ci stages.

The deps stage only copied package manifests, so `npm ci` could not
resolve the local braces file: override. Copy vendor into deps and
runner images.



---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Tony <ashlessscythe@users.noreply.github.com>
@jonschlinkert

Copy link
Copy Markdown
Member

@BenkiNew BenkiNew left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Independent verification of head 1f11eb558be9ea0cda87861408bb766e2e714086 on Node 22.23.2:

  • Existing suite: 1,137 passing.
  • 2,061 compile/expand/stringify comparisons against master agree for ordinary nested patterns and ranges.
  • 22 depth/cycle guard checks pass at both 512 KB and 984 KB V8 stack sizes, in child processes bounded to 128 MB / 5 seconds. Both unpatched negative controls fail as expected.
  • 3,768 numeric/character range-limit cases agree with fill-range cardinality (including negative endpoints, descending ranges and negative steps). Three additional AST reuse checks preserve repeated compile/expand results and the original AST fields. These cover the R1/R2 changes.

Reproducible independent scripts: https://gist.github.com/BenkiNew/40827449253eb003106875520c792597 . Place clones at ./braces (this exact PR head) and ./braces-baseline (master e53730e); install fill-range 7.1.1 and run the scripts from their parent directory, with NODE_PATH pointing at those dependencies if necessary. The upstream suite additionally used Mocha 6.2.3 and bash-path 2.0.1.

This is evidence for the pinned implementation, not blanket validation of arbitrary Cartesian expansion or regex execution. I did not independently test Node 8 compatibility. No blocking defect was found in these checks. Thank you for keeping the preflight iterative and the range transformations non-mutating.

ryanportfolio added a commit to ryanportfolio/Truenote that referenced this pull request Oct 11, 2026
…262)

## What

The daily "Full dependency audit"
(`.github/workflows/dependency-scan.yml`) failed on every run: 24
advisories, 3 critical and 6 high, all in development and build tooling.
This upgrades that tooling together, because Vitest 4 and later need
Vite 6 or newer (Dependabot PR #219 bumped Vitest alone and the rag-app
tests crashed with `ERR_PACKAGE_PATH_NOT_EXPORTED "./module-runner"`).

Catalog changes in `pnpm-workspace.yaml`:

| Package | Before | After | Clears |
|---|---|---|---|
| vite | ^5.4.8 (5.4.21) | ^8.3.4 | `server.fs.deny` bypass on Windows
(high), two moderates, esbuild dev-server moderate, source-map-js (high,
through vite's postcss) |
| vitest | ^2.1.1 (2.1.9) | ^5.0.3 | Vitest UI file read (critical),
Tinypool prototype pollution x2 (critical; Vitest 4+ has no Tinypool),
`@vitest/mocker` path traversal |
| @vitejs/plugin-react | ^4.3.1 | ^6.1.2 | Browserslist x2 (high) and
`@babel/core` (low): plugin-react 6 drops Babel |
| postcss | ^8.4.47 (8.5.14) | ^8.5.29 | source-map path traversal
(high) and its incomplete-fix follow-up (moderate) |
| autoprefixer | ^10.4.20 | ^10.6.1 | pulls browserslist 4.29.3 |
| @types/node | ^20.16.5 | ^24.0.0 | not an advisory: Vitest 5's peer
range is `^22 \|\| >=24`, and the image runs Node 24 since #253 |

Vite 8 is the newest major; `@vitejs/plugin-react` 6.1.2 requires it and
Vitest 5.0.3 accepts it. All chosen versions are older than the repo's
three-day `minimumReleaseAge`.

Overrides: the old `"postcss@<8.5.18": ">=8.5.18"` override is removed,
because the catalog floor and Vite 8 (`postcss ^8.5.29`) now reach a
fixed PostCSS without it. No override was added.

**braces, the one remaining high.** GHSA-vfj7-8cjw-p6xm (CVE-2026-93687,
stack-exhaustion DoS) covers braces through 3.0.3, and no patched
release exists: the advisory lists no first patched version, npm's
latest is 3.0.3, and the upstream fix PRs (micromatch/braces#82, #87,
#88) are open. It reaches this repo only through tailwindcss 3
(chokidar, micromatch, fast-glob), a rag-app devDependency that expands
the content globs in `tailwind.config.ts`; no request input reaches it,
and the runtime image drops development dependencies. With the owner's
approval it is ignored through `auditConfig.ignoreGhsas` in
`pnpm-workspace.yaml`, recorded as TN-VULN-2026-009 in
`docs/compliance/pci/vulnerability-register.md` with a review date of
2026-11-10. Remove the ignore once braces 3.0.4 or later is in the
lockfile. The retained JSON report's severity totals still count it
(`high: 1`); the gate step prints `1 high (1 ignored)` and passes.

Left as they are (below the High gate, and outside this tooling change):
4 moderate and 3 low in runtime dependencies or unreachable through
Tailwind 3: `qs` and `body-parser` through express 4, `sprintf-js`
through mammoth's argparse 1, `esbuild` 0.27 through tsx (Windows
dev-server file read, low), `postcss-selector-parser` 6 through
tailwindcss 3.

Other changes:
- `Dockerfile.railway`: the comment listing the dev tooling kept out of
the runtime image names rolldown and lightningcss instead of esbuild
0.21.
- Vite 8 bundles with Rolldown and minifies CSS with Lightning CSS
instead of esbuild. The main stylesheet's rules are reordered and vendor
prefixes follow Vite 8's default browser targets (102.0 KB to 103.6 KB).
The screenshot comparison below found no visible change.
- `pnpm-lock.yaml`: every new Linux gnu/musl binary (rolldown,
lightningcss) keeps its `libc:` line; the rollup entries that carried
the old ones are gone with Vite 5.
- Vite 8 prints a warning that `vite.config.ts` uses `__dirname` and an
extensionless import, which its future native config loader will not
support. The build works; left for a later change.

## Change identity

- Change ID: `TN-CHG-2026-262`
- Change type: `normal`
- Author: `@ryanportfolio`
- Target release/commit: `Head of claude/upgrade-vite-vitest at merge`
- Target environment: `GitHub Actions (daily full dependency audit) and
the Railway production image build; production deploy only after the
owner's approval`
- Significant change: `no`
- Significant-change rationale: `Build and test tooling versions only.
The runtime image's production dependency set is unchanged apart from
the SPA bundle that Vite 8 builds; no application code, provider, data
store, credential or schema changes.`

## Purpose and scope

- Purpose: `The daily full dependency audit fails every run on 3
critical and 6 high advisories in dev and build tooling, and the
security reviewer sees a red daily audit. This brings it to zero
unaccepted High or Critical.`
- Affected components and data boundaries: `pnpm-workspace.yaml
(catalog, overrides, auditConfig), pnpm-lock.yaml, Dockerfile.railway
(comment only), docs/compliance/pci/vulnerability-register.md
(TN-VULN-2026-009). Build of artifacts/rag-app (Vite 8, Rolldown,
Lightning CSS) and the vitest runs of rag-app and api-server. No data
path changes.`

## Security and PCI impact

- CDE impact: `none`
- CDE-impact rationale: `No payment data path; build and test tooling
only.`
- Security impact and threat considered: `Removes 3 critical and 5 high
advisories in tooling that runs in CI and on the developer machine
(Vitest UI file read and execution, Tinypool prototype pollution, Vite
server.fs.deny bypass on Windows, PostCSS source-map path traversal,
Browserslist, source-map-js). Accepts one high (braces DoS) as a
documented, owner-approved exception with a review date, because no fix
exists and the vulnerable code only expands repository-authored globs at
build time. The built compliance pages are byte-identical, so the public
security pages are unchanged.`

## PCI DSS 6.5 change controls

- 6.5.1 bespoke/custom security testing: `not applicable`
- 6.5.1 evidence or rationale: `No bespoke application code changed;
dependency versions only, checked by the full test suites and the
dependency audit.`
- 6.5.2 completion revalidation: `completed`
- 6.5.2 evidence owner and plan/result: `@ryanportfolio. The
vulnerability register gains TN-VULN-2026-009; verify:vulnerabilities
passes locally. After merge, the next scheduled Full dependency audit
run must pass.`
- 6.5.3 pre-production separation: `confirmed`
- 6.5.3 evidence or rationale: `Built and checked locally against the
fixture API; no production database, variable or deployment touched.`
- 6.5.4 role/function separation: `not applicable`
- 6.5.4 evidence or accountability rationale: `Single-owner repository
has no truthful non-author reviewer identity. CI and the Codex review
supply technical gates.`
- 6.5.5 live PAN in pre-production: `no live PAN`
- 6.5.5 evidence or rationale: `Fixture data only.`
- 6.5.6 test data/accounts removal: `not applicable`
- 6.5.6 evidence owner and plan/result: `No test data or accounts
created outside in-memory fixtures.`

## Verification

- Commands and results: `corepack pnpm install --frozen-lockfile passes.
corepack pnpm -r run check: 0 errors in all four projects. corepack pnpm
-r run test: rag-app 25 files / 143 tests passed, api-server 82 files /
936 tests passed (both on Vitest 5.0.3; file counts match the test files
on disk), scripts 88 passed. corepack pnpm --filter @workspace/rag-app
run build passes on Vite 8.3.4. Audit, the workflow's exact commands
with pnpm 11.13.0: before 3 critical, 6 high, 11 moderate, 4 low; after
0 critical, 1 high (braces, ignored), 4 moderate, 3 low; "audit
--audit-level high" exits 0; "audit --prod --audit-level high"
(security.yml) exits 0. verify:vulnerabilities passes.
verify:pci-evidence locally reports only the two CRLF stale-hash lines
that every Windows checkout reports (pitfalls.md, 2026-10-10); CI
decides.`
- Negative/security tests: `Compliance page output: dist/security/** and
dist/security-pci.html from the Vite 8 build are byte-identical to the
Vite 5 build of origin/main (diff -r, no differences), so
compliance-pages.ts renders the same HTML and CSS.`
- Runtime/integration verification: `Local fixture preview only
(headless Chrome over CDP, fixture API from scripts/preview/mock-api).
Dev server (review-env.mjs, Vite 8): signed-out login page, login form
submit, CSR ask page with a submitted question rendering an answer with
citations, manager Sources, Documents, Users, Teams, Source usage, Gaps,
super-user /compliance list and a compliance document (GFM table and
footnote), and the static /security/compliance/ page; no console errors
other than the expected 401 on /api/me when signed out and a favicon 404
on the static page. Production build: the same 16 steps against vite
preview of the Vite 8 build and of the Vite 5 build, same fixture state:
page text identical on all 16; 11 screenshots pixel-identical; the other
5 differ only inside animated regions (login background drift, the Gaps
portal loader, the ask-page animation). Screenshots:
D:/screenshots/Truenote/vite8-upgrade/. Railway: Pending, owner, after
deploy.`
- Evidence not collected locally: `The Docker image build on Linux
(Container scan runs it on this PR because pnpm-lock.yaml changed); the
deployed SPA on Railway; the first scheduled Full dependency audit after
merge. Owner @ryanportfolio.`

## Vulnerabilities and dependencies

- Dependency/SBOM impact: `Dev and build dependencies: vite 5.4.21 to
8.3.4 (adds rolldown and lightningcss, drops rollup and esbuild 0.21),
vitest 2.1.9 to 5.0.3 (drops tinypool and vite-node),
@vitejs/plugin-react 4 to 6.1.2 (drops Babel), postcss 8.5.14/8.5.24 to
8.5.29, autoprefixer to 10.6.1, browserslist to 4.29.3, @types/node 20
to 24. Production dependency set unchanged; the production audit
(security.yml) still passes.`
- Finding/exception links: `TN-VULN-2026-009 (braces
GHSA-vfj7-8cjw-p6xm, CVE-2026-93687), accepted exception approved by
@ryanportfolio on 2026-10-11, review by 2026-11-10 or when braces 3.0.4
publishes.`

## Deployment and secure recovery

- Deployment/configuration/DDL steps: `Merge. No DDL, no variable. The
next production deploy (owner's approval) builds the SPA with Vite 8.`
- Post-deployment verification: `After merge: the next scheduled Full
dependency audit (04:37 UTC) passes. After the next deploy: truenote.org
login, ask page and /security/compliance/ load with styles; railway logs
for web show no static-asset errors.`
- Failure signal: `Full dependency audit fails, Container scan fails to
build the image, or the deployed SPA loads without styles or with
console errors.`
- Secure recovery procedure: `Revert this pull request and redeploy the
previous web image.`
- Incident ID: `Not applicable: normal change`
- Emergency authority: `Not applicable: normal change`
- Retrospective review due: `Not applicable: normal change`

## Approval and closure

- Non-author reviewer: `Unassigned: single-owner repository`
- Review evidence: `This pull request, its hosted checks and the Codex
review.`
- Specialist approval: `Not applicable: no CDE control change; the
braces exception is approved by the owner in TN-VULN-2026-009`
- Release/change-authority decision: `pending`

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.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