fix(js): close JS/WASM surface findings M2, M3, L1#61
Merged
Conversation
M2: dithering.test.ts's per-mode matrix passed `mode` as ditherImage's
third positional argument, which is `DitherOptions` (an object), so
destructuring a number silently yielded `undefined` for every field
and all nine "produces valid output for mode %s" cases ran BURKES.
Fixed the call to `{ mode }` and added an assertion tying each mode's
case to its own actual dithering behavior (diffing against DitherMode.NONE)
plus a cross-mode difference check, so this kind of hollowing fails loudly.
M3: wasm dither_image's `scheme_id: u8` meant any parsable value with a
non-empty `palette_bytes` silently enabled canonical pinning, so a direct
wasm consumer leaving scheme_id at its default got Mono-canonical pinning
unasked. Changed the parameter to `Option<u8>` (BREAKING WASM SIGNATURE:
scheme_id is now optional/nullable instead of a mandatory u8; an
unparsable Some(id) is now a hard error instead of a silent fallthrough
to the uncanonicalized path) and dropped the 255 sentinel in core.ts for
a real `undefined`.
L1: core.ts trusted the caller's width/height while wasm derives height
from buffer length, so a short buffer silently produced
indices.length !== width * height. core.ts now validates
data.length === width * height * 4 and throws, matching Python's
existing cross-check.
Not a breaking change for the TypeScript public API.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Summary
Closes three audit findings on the JavaScript/WASM surface (Task 4 of the audit-remediation plan):
dithering.test.tspassedmodeasditherImage's third positional argument (DitherOptions, an object), so destructuring aNumberyieldedundefinedfor every field and all nine "produces valid output for mode %s" cases silently ran BURKES. Fixed the call to{ mode }. Strengthened the per-mode test to diff each mode's output againstDitherMode.NONE(so a broken kernel for that mode fails that mode's case) and added a cross-mode "not all identical" guard against future hollowing.wasm/src/lib.rs'sdither_imagetookscheme_id: u8; with non-emptypalette_bytes, any parsablescheme_idsilently enabled canonical pinning, so a direct wasm consumer leaving it at 0 got Mono-canonical pinning unasked. Changed the parameter toOption<u8>, made an unparsableSome(id)a hard error instead of a silent fallthrough, and dropped the255sentinel incore.tsin favor ofundefined. This changes the wasmdither_imageexport's signature (not a breaking change for the TypeScript public API —ditherImage's TS surface is unchanged).core.tsreturned the caller's claimedwidth/heightwhile wasm derives height from buffer length, so a short buffer silently producedindices.length !== width * height.core.tsnow validatesdata.length === width * height * 4and throws, matching Python's existing cross-check style.Test plan
cd packages/javascript && bun run type-check— passescd packages/javascript && bun run test— 44/44 passcd packages/javascript && bun run build— passescd packages/rust && cargo test --workspace— 61/61 pass (58 unit + 3 regression fixtures, untouched)cd packages/rust && cargo clippy --workspace -- -D warnings— cleanpython3 scripts/check_enum_parity.py— exits 0algorithms.rs, rebuilt wasm — theproduces valid output for mode 3case (and the pre-existing serpentine test) failed; reverted, rebuilt, confirmed green andgit statusclean on that file.scheme: 99and assertsditherImagethrows instead of silently dithering without canonical pinning.ImageBufferwhosedatais shorter thanwidth * height * 4and assertsditherImagethrows.