Skip to content

fix: contain parser panics and stack overflows, and decide from the CST - #39

Merged
shahryarjb merged 7 commits into
ash-project:mainfrom
shahryarjb:chore/dependency-updates
Aug 4, 2026
Merged

fix: contain parser panics and stack overflows, and decide from the CST#39
shahryarjb merged 7 commits into
ash-project:mainfrom
shahryarjb:chore/dependency-updates

Conversation

@shahryarjb

Copy link
Copy Markdown
Collaborator

Hardening pass over the Rust side, plus a dependency audit. Nothing here changes the public API; the headline is that two ways of taking the BEAM down were found and closed, both of them in input handling rather than in the codemods themselves.

  • A parser panic can no longer escape. biome_css_parser 0.5.8 asserts that it keeps making progress and panics on some input — the property tests found one, which shrinks to @layer/*x*/\n color: var(--brand);\n}\n. Rustler turns that into an exception in the calling process, which the VM survives but which breaks the promise that unpatchable input returns {:error, reason}. Every parse now goes through ParseCtx::try_new, which catches it and reports it as Unparseable. Pinned as a deterministic test, including one that asserts the input still panics the raw parser — so if Biome fixes it upstream, that test fails and says the guard may be removable.
  • Deeply nested input is refused rather than overflowing the stack. A stack overflow aborts; catch_unwind cannot intercept it, so in a NIF it ends the VM rather than the call. Measured on a 2 MB stack, both @media{ nesting and :not( nesting survive 1000 levels and abort at 2000. Every entry point now measures nesting first — a byte scan that skips strings and comments and cannot itself recurse — and refuses past 256. Real CSS measures depth 7, including a 60 KB ex_doc stylesheet. The same limit applies to caller-supplied text, which turned up a second problem: a selector past the limit did not crash, but ensure_rule would splice it in and produce a file we would then refuse to read.
  • Native CSS nesting is now visible to queries that descend. Biome models &:hover { } as CSS_NESTED_QUALIFIED_RULE with a CSS_RELATIVE_SELECTOR_LIST, not CSS_QUALIFIED_RULE, so find_all_rules skipped nested rules entirely and analyze reported one rule for a file containing three. Top-level queries are unchanged — nesting stays scoped.
  • analyze is no longer quadratic in nesting depth. conditions_of rebuilt the list of every at-rule in the file once per ancestor, for a node it already held, and prelude_norm scanned a whole subtree when it needed only the tokens before the block. Depth 128 went from 20.6 ms to 1.9 ms, and growth is roughly linear.
  • Decisions come from the CST, not from scanning text. Selector normalisation was a character scanner tracking quote state and bracket depth; it now walks the tree, which also fixed a real bug — :nth-child(2n + 1) and :nth-child(2n+1) are one selector, and the scanner kept them distinct. Colour detection matched 148 names against raw text and needed a hack so url(/red.png) was not read as red; it now matches CSS_COLOR, CSS_FUNCTION and CSS_IDENTIFIER node kinds, and the hack is gone. At-rule preludes and caller-supplied declaration lists are likewise token-derived now.
  • No panicking paths remain outside tests — no unwrap, expect, panic!, todo!, unreachable!, or unchecked indexing reachable from a NIF.
  • Dependencies. ex_doc and igniter declared floors that sat well behind what the lock resolved (~> 0.38 against 0.40.3, ~> 0.5 against 0.8.3); mix hex.outdated does not catch this, because it compares resolved versions rather than requirements. Everything else, Elixir and Rust, is already at its newest published version. Cargo.toml now records why the biome pin is 0.5.8 and what would break it: biome_css_formatter is stuck at 0.5.7 and requires biome_css_syntax ^0.5.7, so depending on it would drag the whole graph back a release — which is exactly why igniter_js pins its set to 0.5.7.
  • Docs. Sixteen comments cited a ROADMAP §… that is not in this repository, and others carried its rule letters, risk IDs and constraint numbers; all are now self-contained or link to the module that holds the behaviour. Migration commentary about the previous implementation is gone from the code — the changelog is the right place for it.
  • Two agent skills under .claude/skills: the release flow, and the codemod conventions with the failure modes behind them.

Verified on Elixir 1.20.2 / OTP 28.0.2: 355 Rust tests, 307 Elixir, clippy -D warnings clean, credo, sobelow, dialyzer and both formatters clean, and no warnings from this project. Target coverage was checked against rustler_precompiled's defaults — we build nine of its ten plus FreeBSD; the missing one is 32-bit ARM, which ash-ci disables for all rustler projects because of a cranelift bug.

shahryarjb and others added 7 commits August 4, 2026 07:45
…o not ship

Dependency audit found nothing to upgrade. Every direct and transitive
dependency, Elixir and Rust, is already at its own newest published version:
`mix hex.outdated` reports all up to date, and `cargo update --dry-run` locks
zero packages.

That includes the biome crates, and the reason is worth writing down rather than
rediscovering. biome_css_parser, biome_css_syntax and biome_rowan are all at
0.5.8, the newest published. We can be there only because this crate does not
depend on biome_css_formatter, which is stuck at 0.5.7 and requires
biome_css_syntax ^0.5.7 and biome_rowan ^0.5.7 -- adding it would pull the whole
graph back a release. That is exactly why igniter_js, which does format CSS,
pins its entire biome set to 0.5.7. Cargo.toml now says so, including the
condition that keeps it true: our pretty-printer is written against the CST, not
routed through biome_css_formatter.

The lock also holds biome_diagnostics_categories, biome_diagnostics_macros and
biome_markup at 0.5.7, which is their own latest, and biome_unicode_table at
0.5.9. Nothing is held back.

Separately, sixteen doc comments across thirteen files cited "ROADMAP §..." for
a document that is not in this repository -- it was a planning note, never
committed. Every citation is now self-contained prose. A pointer to something a
reader cannot open is worse than no pointer.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sweep of every comment and doc-comment in the Elixir and Rust sources for
references a reader cannot follow, and for history that belongs in the changelog
rather than in the code.

Removed the planning-document vocabulary. Sixteen comments cited "ROADMAP §..."
for a file that was never committed; others carried its rule letters (rule A
through rule E), its risk IDs (R1, R2, R4) and its "hard constraint #N"
numbering. None of those resolve to anything in this repository. Each is now
either stated plainly or replaced by a link to the module that actually holds
the behaviour -- "see [`crate::trivia`]" instead of "rule D".

Removed the migration commentary. The Parsers.Parser moduledoc explained what
the Python/tinycss2 implementation did and how this differs; it now just
describes what the module does. Comments in locate.rs and nif.rs justified
decisions by reference to the old implementation, and one in locate.rs described
a text scanner this branch had already deleted. The changelog is the right place
for all of it, and it still says so there.

Renamed tests/phase0_roundtrip.rs to tests/roundtrip.rs. The gate is not a
phase, and its doc now says why it matters -- every byte-range edit depends on
the parse reproducing its input -- rather than where it sat in a plan.

No behaviour change. 339 Rust tests, 268 Elixir + 39 doctests, clippy clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s in use

Both declared floors sat well behind what the lock actually resolved: ex_doc at
`~> 0.38` while 0.40.3 was installed, and igniter at `~> 0.5` while 0.8.3 was.

`mix hex.outdated` does not catch this. It compares the resolved version against
the newest published one, and both resolved fine, so it reported everything up
to date -- the stale part was the requirement, not the dependency.

No resolution changes; this only stops the declared floors drifting further from
reality. Docs still build and the suite is unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tack

Audit for anything in the Rust that could take the BEAM down.

Rustler wraps every NIF body in `catch_unwind` and turns a panic into an Erlang
exception in the calling process, so panics are survivable, and there are none
left outside tests anyway: no unwrap, expect, panic!, todo!, unreachable! or
indexing that is not bounds-checked first.

A stack overflow is different. It aborts the process, `catch_unwind` cannot
intercept it, and in a NIF that ends the VM rather than the call. Biome's CSS
parser is recursive descent and rowan's tree drop recurses too, so deeply nested
input overflows. Measured on a 2 MB stack, `@media print{` nesting and `:not(`
nesting both survive 1000 levels and abort at 2000 -- and a dirty scheduler
thread may have less headroom than that.

Every entry point that reaches a parse now measures nesting first, with a plain
byte scan that skips strings and comments and cannot itself recurse, and refuses
past 256 levels. That is an order of magnitude below the observed failure and
far above real CSS, which rarely exceeds ten.

The guard also applies to caller-supplied text, which turned up a second
problem: a selector nested past the limit did not crash, but `ensure_rule` would
splice it in and produce a file we would then refuse to read. Writing what we
will not read is its own bug, so `validate_snippet` enforces the same limit, and
`set_declaration` now validates the selector it may create a rule from.

tests/stack_safety.rs covers both nesting dimensions across every mutating,
read-only and transform op, and asserts ordinary CSS is nowhere near the limit.
Verified end to end through the real NIF: all ten paths refuse, the VM survives,
and ordinary CSS is unaffected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… walk

Three findings from auditing what can take the VM down, and from testing on
Elixir 1.20.

Biome's CSS parser can panic. It asserts that it keeps making progress and
aborts with "The parser is no longer progressing" on some inputs; the fuzzer
found one by splicing comments into generated CSS at arbitrary offsets. Rustler
would turn that into an exception in the calling process, which is survivable
but breaks the promise that unpatchable input returns {:error, reason}.
ParseCtx::try_new now catches it and reports it as one, and every production
parse -- ops, transforms, validate, and the probe parses for a caller's selector
and value -- goes through it. No unguarded parse_css call remains outside ctx.

The property test asserting that spliced input always parses was asserting
something Biome cannot guarantee. It now asserts what this library actually
promises: either a context that round-trips, or a clean error, never a panic
that escapes.

Native CSS nesting was invisible to any query that descends. Biome models
`&:hover { }` as CSS_NESTED_QUALIFIED_RULE holding a CSS_RELATIVE_SELECTOR_LIST,
not CSS_QUALIFIED_RULE, so find_all_rules skipped nested rules entirely and
analyze under-reported: a file with three rules counted one. Both shapes are now
recognised. Top-level queries are unaffected -- nesting stays scoped, and
`.card`'s declarations remain its own.

analyze was quadratic in nesting depth. conditions_of rebuilt the list of every
at-rule in the file once per ancestor, for a node it already held, and
prelude_norm filtered the whole subtree when it only needed the tokens before
the block. Reading each ancestor directly and bounding the token scan with
take_while made depth 128 eleven times faster, 20.6ms to 1.9ms, and the growth
roughly linear.

352 Rust tests, 307 Elixir. Verified on Elixir 1.20.2/OTP 28: no warnings from
this project, credo, sobelow, dialyzer and both formatters clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… seed

The proptest regression file committed in the previous change is the seed store
proptest writes when a property fails. Its header recommends checking it in so
the failing case is replayed for everyone, which is worth having -- but it
carried no SPDX header, so REUSE would have failed on it. Adding the sidecar.

More usefully, it already held the minimal reproducer I had been searching for
by hand. proptest had shrunk the failure to a 24-byte edit, which reconstructs
to:

    @layer/*x*/
      color: var(--brand);
    }

Raw biome_css_parser 0.5.8 panics on that; the guarded path returns Err.

That is now a deterministic test rather than something only a random fuzzer
finds: every entry point is asserted to return Unparseable, and one test asserts
the input still panics the underlying parser -- so if Biome fixes it upstream,
that test fails and tells us the guard may be removable rather than leaving it
in place forever with nobody knowing why.

Worth reporting upstream to Biome.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two project skills, written for an agent rather than a human reader:
constraints first, exact commands, and the failure modes with their causes.

`release` covers the tag-triggered flow. The point worth encoding is that a tag
is the only manual step -- the checksum generation and Hex publish run in CI,
and doing them by hand is a sign something is broken. It also lists the four
failures seen in practice: the missing permissions block, OTP too old for
hex.pm's certificate chain, a poisoned Actions cache, and the 404 you get from
building locally before a release exists.

`css-codemod` covers the parts an agent gets wrong here. Chiefly: never reprint
the tree, and decide from the CST rather than by scanning text -- with a table of
what Biome already models, because every text scanner in this codebase was
eventually replaced by a node kind. It also records the safety rules that are
not obvious, notably that rustler catches panics but a stack overflow aborts and
takes the VM down, and the quadratic trap of calling find_all_* inside a
per-node loop.

Both carry SPDX sidecars so REUSE stays green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@shahryarjb shahryarjb self-assigned this Aug 4, 2026
@shahryarjb
shahryarjb merged commit f096cf1 into ash-project:main Aug 4, 2026
26 checks passed
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.

1 participant