Skip to content

IMPORTANT: prove the hand-rolled lexer matches an AST (auth gate / elision / check rest on it) #753

Description

@vivek7405

Problem

Three correctness-critical, partly security-relevant subsystems all rest on a single hand-rolled lexical scanner, not an AST:

  • the fail-closed browser-bound authorization gate (module-graph.js),
  • component elision (component-elision.js), and
  • the webjs check convention validator (check.js).

The lexer (js-scan.js) is careful (it tracks the regex-vs-division ambiguity, template nesting, comments) and the elision verdict is deliberately conservative, so the blast radius is mostly contained. But any lexer desync is a bug class an AST would not have: a misclassified string boundary or template-interpolation edge can drop a real import edge (gate mis-authorizes / a module 404s) or a real register() (scanner misses a component). The scanner header itself documents accepted blind spots (e.g. a register() inside a tagged-template interpolation "disappears from the lint surface").

Because webjs is no-build, a scanner mistake degrades to a silent runtime 404 / component mis-upgrade rather than a compile error, which is exactly the class a solo maintainer is least able to catch by eye. From an architecture audit.

Design / approach

Do NOT replace the lexer with a full parser on the hot path (the no-build, fast-scan design is intentional and a parser is slower for these shallow patterns). Instead, prove the hand-rolled lexer agrees with a real parser via a differential / property test:

  • Run BOTH the hand-rolled scanner and a real parser (Node's own module.stripTypeScriptTypes is already a dep; or use the TS/acorn AST already available in the toolchain) over a large corpus of real source (the repo's own packages/**, examples/**, plus adversarial fixtures: regex-vs-division, nested templates, import inside strings/templates/comments, register() in interpolations, division after a paren).
  • Assert the import-edge set and the register()/component-class set the lexer extracts MATCH what the AST extracts, OR are a documented conservative superset/subset (the gate may admit more, elision may ship more, but must never admit fewer for the gate / never elide a component the AST says is interactive).

Optionally, add an AST fallback when ambiguity is detected: if the lexer hits a construct it knows it cannot classify confidently, escalate that one file to a real parse rather than guessing. Scope this issue to the differential test first; the fallback can be a follow-up if the test surfaces real disagreements.

Implementation notes (for the implementing agent)

  • The lexer: packages/server/src/js-scan.js. scanLiterals(src, opts) (L111) is the core string/template/comment lexer; redactStringsAndTemplates / redactToPlaceholders (fix: false positives in component scanner and elision analyzer due to string/template literals in code samples #634) / extractWebComponentClassBodies / matchClosingBrace build on it. Header at L5-L9 states it is "deliberately a hand-rolled lexer, NOT a full TS parse" and documents trade-offs.
  • Consumers to cover: module-graph.js (IMPORT_RE + EXPORT_FROM_RE scanned over the redaction mask, L344-L361; the auth gate reachableFromEntries), component-elision.js (~1207 LOC, import + side-effect + WHEN_DEFINED_RE / TAG_DEFINED_RE / INSTANCEOF_RE scans), component-scanner.js (maps component class to URL), and check.js (22 named rules).
  • A real parser is available: Node 24's module.stripTypeScriptTypes (via ts-strip.js) erases types; for AST extraction, the project already depends on amaro (optionalDep) which wraps swc. Pick whatever yields a usable AST in the test layer without adding a heavy new runtime dep (a test-only devDep parser is acceptable, but per project memory npm install here can contaminate the lockfile, so add it carefully / --no-save in CI or as an explicit devDependency reviewed against CI tsc).
  • Landmine (project memory sandbox_npm_install_contaminates_lock): installing a parser as a dep can break CI's real tsc. Prefer a parser already in the dep tree; if a new devDep is needed, verify CI before marking ready.
  • Invariant: elision must stay verdict-safe (ship-more on ambiguity, AGENTS.md invariant 7); the test must encode the asymmetric direction (gate: never admit fewer; elision: never elide a real-interactive).
  • Corpus: reuse the existing fixtures under packages/server/test/scanner/, packages/server/test/module-graph/, packages/server/test/elision/, and add adversarial cases.

Acceptance criteria

  • A differential test runs the hand-rolled scanner and a real parser over a large corpus and asserts agreement (or a documented, direction-safe divergence) on: import/export-from edges, register() call sites, and WebComponent class bodies.
  • Adversarial fixtures included: regex-vs-division, nested templates, import/register inside strings/templates/comments.
  • Any real disagreement found is either fixed in the lexer or documented as a known, verdict-safe limitation with a justification.
  • No change to the hot-path scanning approach (still lexer-based at runtime; the parser runs only in the test).
  • Tests under packages/server/test/scanner/ (or a new test/scanner-fuzz/); no new runtime dependency added without CI verification.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Fields

No fields configured for issues without a type.

Projects

Status
Todo

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions