Repository navigation
refactor(tools)!: replace webassemblyjs with a built-in wasm parser - #104
Merged
Merged
Conversation
`parseWasm` only needs a module's imports and exports to generate the ESM facade, but pulled in `@webassemblyjs/wasm-parser` to get them. That was vendored as a 245 kB pre-bundled `lib/wasm-parser.mjs` (regenerated by an esbuild build hook), four `resolutions` pins, and a patch adding missing opcodes (bulk-memory, saturating truncation, exception handling) so modern binaries would not fail to parse. Wasm sections are length prefixed, so extracting the module interface only requires decoding the type, import, export and `name` sections and stepping over everything else by size. The new reader never decodes instructions, so the opcode coverage problem the patch worked around no longer exists. Removes lib/, the esbuild build hook, the `@webassemblyjs/wasm-parser` and now unused `esbuild` devDependencies, the `resolutions` pins, pnpm-workspace.yaml and the patch. dist/tools/index.mjs: 201 kB -> 5.99 kB (26.8 kB -> 1.35 kB min+gzip) Total dist: 414 kB -> 24 kB `ModuleExport["type"]` is widened to a new exported `ExternalKind`, adding the `Table` and `Global` kinds that were previously unrepresented. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthrough
ChangesWebAssembly interface parser
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant parseWasm
participant WasmReader
participant ParseResult
Caller->>parseWasm: provide WebAssembly bytes
parseWasm->>WasmReader: read type, import, export, and name sections
WasmReader-->>parseWasm: decoded interface data
parseWasm->>ParseResult: assemble modules
ParseResult-->>Caller: return ParseResult
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tools/parser.ts`:
- Around line 168-208: Thread each section’s declared end offset from _parseWasm
into readImportSection and readExportSection, and bound their entry loops so
parsing fails or stops when reader.pos reaches the section end instead of
spilling into the next section. In readFunctionNames, apply the same
subsection-end check within the inner name-entry loop before the existing
position reset, preserving exact subsection consumption validation. Align these
readers with readTypeSection’s bounded parsing behavior.
- Around line 210-226: Update readExportSection to validate kind against
EXTERNAL_KINDS and throw for unknown values, matching readImportSection; remove
the "Func" fallback while preserving valid export parsing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6daebd4c-d4ff-4c21-ba36-b6b644e4153f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
README.mdbuild.config.tslib/wasm-parser.in.mjslib/wasm-parser.mjspackage.jsonpatches/@webassemblyjs__helper-wasm-bytecode.patchpnpm-workspace.yamlsrc/tools/parser.tstest/parse.test.ts
💤 Files with no reviewable changes (5)
- pnpm-workspace.yaml
- lib/wasm-parser.in.mjs
- patches/@webassemblyjs__helper-wasm-bytecode.patch
- build.config.ts
- package.json
Comment on lines
+168
to
+208
| function readImportSection(reader: WasmReader, types: FuncType[]) { | ||
| const imports: ModuleImport[] = []; | ||
| const length = reader.varuint(); | ||
| for (let i = 0; i < length; i++) { | ||
| const module = reader.name(); | ||
| const name = reader.name(); | ||
| const kind = reader.u8(); | ||
| const entry: ModuleImport = { module, name }; | ||
| switch (kind) { | ||
| case 0: { | ||
| // func | ||
| const type = types[reader.varuint()]; | ||
| entry.returnType = type?.results[0]; | ||
| entry.params = type?.params.map((type) => ({ type })); | ||
| break; | ||
| } | ||
| case 1: { | ||
| // table | ||
| reader.u8(); // reftype | ||
| reader.limits(); | ||
| break; | ||
| } | ||
| case 2: { | ||
| // memory | ||
| reader.limits(); | ||
| break; | ||
| } | ||
| case 3: { | ||
| // global | ||
| reader.u8(); // valtype | ||
| reader.u8(); // mutability | ||
| break; | ||
| } | ||
| default: { | ||
| throw new Error(`unsupported import kind: ${kind}`); | ||
| } | ||
| } | ||
| imports.push(entry); | ||
| } | ||
| return imports; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Import/export/name readers don't verify they consumed exactly their section's declared bytes.
readTypeSection bails via reader.pos >= end when the declared count overruns the actual data, but readImportSection has no end parameter at all, and readFunctionNames's inner subsection reads aren't checked against subsectionEnd either (it just force-resets reader.pos = subsectionEnd afterward). A corrupted/malformed count field in any of these sections lets reads silently spill into the next section's bytes — bounded by total buffer length, so no crash, but producing confidently-wrong parsed imports/exports/names instead of a clear parse failure.
♻️ Suggested fix: assert full consumption after each section loop
-function readImportSection(reader: WasmReader, types: FuncType[]) {
+function readImportSection(reader: WasmReader, types: FuncType[], end: number) {
const imports: ModuleImport[] = [];
const length = reader.varuint();
for (let i = 0; i < length; i++) {
...
}
+ if (reader.pos !== end) {
+ throw new Error("import section length does not match its declared size");
+ }
return imports;
}Apply the analogous check (with end threaded from the call sites in _parseWasm) to readExportSection and to the subsection loop in readFunctionNames.
Also applies to: 227-247
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/tools/parser.ts` around lines 168 - 208, Thread each section’s declared
end offset from _parseWasm into readImportSection and readExportSection, and
bound their entry loops so parsing fails or stops when reader.pos reaches the
section end instead of spilling into the next section. In readFunctionNames,
apply the same subsection-end check within the inner name-entry loop before the
existing position reset, preserving exact subsection consumption validation.
Align these readers with readTypeSection’s bounded parsing behavior.
Independent review found four cases where the new parser returned wrong data instead of failing. Silent misparses are worse than errors here: the plugin already warns and falls back to a plain `WebAssembly.Module` when `parseWasm` throws, so an error is reported while wrong names are not. - Validate the version field. Components share the wasm magic but use a different layout, and previously decoded as a core module with no imports or exports and no error at all. - Stop mapping unknown export kinds to `Func`. An exported tag was reported as a function and then resolved through the function name map, taking on the name of whichever function shared its index. Adds the `Tag` kind and rejects anything still unknown. - Pass `ignoreBOM` to `TextDecoder`. U+FEFF is a legal character in a wasm name, and a leading one was being stripped from the module and field names the plugin consumes. - Reject the typed reference forms `(ref ht)` and `(ref null ht)`. These are two bytes wide, so reading one byte desynced the reader and renamed every following import. Also bounds vector reads by their section length, bounds name subsections by the name section, limits `varuint` to the 5 bytes a u32 can occupy, and supports tag imports. Tests add threads, memory64, exceptions and multi-memory to the wabt feature set, and hand assemble binaries for shapes wabt cannot express. The interface is now also cross checked against `WebAssembly.Module` rather than only against recorded snapshots. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Re-validation found the bounds checks were applied too broadly. A malformed `name` section made `parseWasm` reject modules the engine accepts, even though custom sections carry no semantics (spec 5.5.3) and the function names only ever feed `ModuleExport.id`. Custom sections are now best effort, like the type section: unreadable names leave their exports with a numeric id, and a custom section whose own name cannot be read is simply left unidentified. The interface sections stay strict, since a misparse there is silently wrong. The test named for the vector bounds check never reached it. It used a bare `toThrow()` and passed on an unrelated `unsupported export kind` error, leaving the guard uncovered. It now uses a matcher and bytes whose trailing custom section decodes as a syntactically valid export entry, so only the section length check can reject it. Adds coverage for the u32 upper bound, unknown import kinds, unknown value types, and both custom section degradation paths, and extends the `WebAssembly.Module` cross-check to compare export kinds so the `ExternalKind` mapping is checked against the engine. parser.ts coverage: 99.4% statements, 98.6% branches, 100% functions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
pi0
changed the title
pi0
changed the title
pi0
approved these changes
Jul 19, 2026
This was referenced Jul 19, 2026
Closed
Closed
This was referenced Jul 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
parseWasmonly needs a module's imports and exports — the plugin uses them to generate the ESM facade (runtime/binding.tsemits oneexport constper export;runtime/imports.tsresolves each import module to a real JS module).To get those two lists we pulled in
@webassemblyjs/wasm-parser, which came with real baggage:lib/wasm-parser.mjschecked into the repostarthook inbuild.config.tsto regenerate itresolutionspinspatches/@webassemblyjs__helper-wasm-bytecode.patch, which bolted missing opcodes (bulk-memory, saturating truncation, exception handling) onto its instruction table so modern binaries would not fail to parseThat last one is the telling part: we were maintaining an opcode table to decode instructions we never look at.
Approach
Wasm sections are length-prefixed, so extracting the module interface only requires decoding the type (1), import (2), export (7) and
namecustom sections — everything else is stepped over by size. Because instructions are never decoded, the opcode-coverage treadmill the patch worked around stops existing. (This is specifically about instruction decoding — see the EH note below.)The guiding rule in the reader: anything it cannot model must raise rather than be guessed at. A misparse silently reports the wrong import/export names into generated code, whereas a throw is caught at
src/plugin/index.ts:50, warned about, and falls back to a plainWebAssembly.Module.Results
dist/tools/index.mjsdistAlso runtime-agnostic now — the old bundle was built with
platform: "node".Verification
pnpm test(lint + typecheck + vitest) is green: 41 tests pass, parser at 97% statement / 100% function coverage.The existing
rand.wasminline snapshot passes byte-identical and unmodified, includingname-sectionidresolution.This branch was reviewed by an independent adversarial pass, which validated the parser against
wasm-objdump -xas an oracle across every.wasmon the machine:externref, dylink/PIC tree-sitter modules, and WASI ripgrepUnexpected instruction: 0xfd,tried to decode an illegal bytecode: 0x19)So this is a substantial real-world robustness win, not only a size win.
Second commit: four silent-misparse bugs found in review
The first commit had four cases that returned wrong data instead of failing. All are fixed in
d3a8869, each with a regression test:"Func", and the tag index was then resolved through the function name map — so(export "myTag" (tag $t))could be reported with an unrelated function's name. Adds theTagkind; unknown kinds now throw.jco/ WASI 0.2 output) decoded as a core module and returned{imports:[],exports:[]}with no error, so the plugin's warning never fired.TextDecoderstripped a leading U+FEFF from names — legal in a wasm name, and it corrupted exactly the fields the plugin consumes. Fixed withignoreBOM: true.(ref ht)/(ref null ht)desynced the reader. These are two bytes; reading one shifted every following read. A brute force found 1944 variants that parsed "successfully" with wrong names. Now rejected explicitly.Plus: vector reads bounded by section length, name subsections bounded by the name section,
varuintlimited to the 5 bytes a u32 can occupy, and tag imports supported.Tests now enable threads/memory64/exceptions/multi-memory in wabt, hand-assemble binaries for shapes wabt cannot express (GC type forms, typed refs, BOM names, malformed sections), and cross-check the interface against
WebAssembly.Module.imports/exportsrather than only against recorded snapshots.Reviewer notes
API change.
ModuleExport["type"]widens from"Func" | "Memory"to an exportedExternalKind = "Func" | "Table" | "Memory" | "Global" | "Tag". Additive for consumers reading the field; breaks only an exhaustiveswitchwith anevercheck. Probably warrants a minor bump.EH support is partial, not complete. Tag imports and exports parse, but this parser models the module interface only — it makes no claim about full exception-handling support.
🤖 Generated with Claude Code
Summary by CodeRabbit
Buffer,ArrayBuffer, andUint8Arrayinputs.Tag) and better debug-name handling, including non-ASCII names.parseWasmREADME documentation to reflect its dependency-free, interface-only behavior.parseWasmtest coverage, including additional edge cases and malformed inputs.