Skip to content

Take the patched dev tools in #37: vite, undici, shell-quote, source-map-js - #39

Open
threeqube wants to merge 1 commit into
mainfrom
dev-tool-patches
Open

threeqube wants to merge 1 commit into
mainfrom
dev-tool-patches

Conversation

@threeqube

Copy link
Copy Markdown
Collaborator

Part of #37 (PR 2 of 3). It leaves #37 open: the @stablelib/ed25519 alert waits on digitalcredentials/verifier-core#84, and the Vitest alerts are PR 3.

What changes

package-lock.json only; package.json is untouched. Each package moves to a fixed version within the range its parent already asks for:

Package From → to Comes in through Alerts
vite 8.3.0 → 8.3.5 our own devDependency (^8.2.0) 6
vite (two copies) 7.3.6 → 7.3.7 Vitest 3 and vite-node 5
undici 6.28.1 → 6.29.1 security-document-loader → http-client 4
undici 7.29.1 → 7.30.1 verifier-core → … → @interop/http-client 5
shell-quote 1.10.0 → 1.11.0 bnid → react-native-securerandom → react-native → react-devtools-core 1
source-map-js 1.2.1 → 1.2.2 vite → postcss 1

vite 8.3.5 needs newer rolldown (~1.2.14) and postcss (^8.5.30), so those move too: rolldown 1.2.9 → 1.2.14 with its 15 per-platform binaries and @oxc-project/types 0.150.0 → 0.154.0, postcss 8.5.28 → 8.5.30, and nanoid 3.3.19 → 3.3.20 for postcss. These are the minimum versions vite allows, not the latest: rolldown 1.2.16 was published on the day of this change, and a build tool that fresh isn't worth the risk.

Every new version was published by the same project or maintainer as the one it replaces (vite, undici, rolldown, postcss and nanoid by their GitHub Actions, with npm provenance; shell-quote and source-map-js by their usual maintainers). Every changed entry's checksum matches the npm registry.

This should close 22 alerts: vite #9–#19, shell-quote #8, source-map-js #7, undici #29, #30, #32–#38. Six stay open: @stablelib/ed25519 #1, and Vitest, @vitest/mocker and tinypool #2, #3, #5, #6, #20 (PR 3).

None of these ship, but the build that ships changes (verified)

None of these packages is bundled into dist/: THIRD_PARTY_LICENSES.md is byte-identical, as is every .d.ts. But vite builds dist/, and the shipped JavaScript does change:

  • dist/index.js: about 1,200 lines across 97 places;
  • the small ky chunk (distribution-*.js): 3 places in its options-merge loop;
  • the rolldown runtime chunk: 1 line.

Where it comes from. Built without minifying, the two bundles are the same code except for three harmless details: the spacing of two empty loop bodies in pako, and how rolldown 1.2.14 passes a CommonJS module's top-level this to ed2curve (still the same object; rolldown/rolldown#11135). Everything else is the minifier, oxc 0.150 → 0.154, which now writes the same logic differently (continue in a loop becomes if … else, different short names). Minifying is meant never to change behaviour, so the question is whether the new oxc keeps that promise on our code.

Behaviour, side by side. Both builds' verify() ran in one Node process on the same credentials, with the clock frozen and the network recorded once and replayed, so each saw identical inputs. Each run compared the full result and what every exported reader makes of it (summarise, listChecks, issuerIdentity, schemaFinding, and so on):

  • 3,810 runs, 0 differences:

    • the demo fixtures;
    • all 91 vc-test-fixtures credentials (v1 and v2, Ed25519 and eddsa-rdfc-2022, did:key and did:web, live status lists);
    • the 25 hostile credentials from Move fast-uri to 3.1.9 #38's review;
    • 1,711 altered fixtures (595 re-signed so the later checks run, 1,116 with a broken seal);
    • 32 did:web issuers whose documents send the controller check through jsonld's frame and compact;
    • 12 ECDSA (ecdsa-rdfc-2019) credentials.

    Each was run with registry lookups on and off.

  • The harness catches a real fault: with one step deleted from the new build's frame code, the did:web cases flip from verified to not verified (16 differences).

  • Coverage: node's own coverage shows 42 of the 97 changed places in index.js ran, including jsonld (expand, frame and compact), the ajv schema check, pako's decompression (status lists) and verifier-core's checks.

  • Places that never ran:

    • 27 are in the second jsonld copy's compact and frame, which nothing can call: that copy's only user is ECDSA, which calls only toRDF.
    • pako's compression side (deflate) can't run either: it only creates status lists. Five of pako's changed places are on the decompression side. Some of them ran, and the reviewer read the rest as equivalent: dropping |0 on values that are already 32-bit integers, and & 4294967295 written as | 0.
    • The rest are rare paths, which the reviewer read line by line, old against new, and judged equivalent:
      • jsonld compact and url, ajv, fast-uri, verifier-core's cache and achieved-level check, and serialize-error;
      • our element's credential setter;
      • the ky chunk, which verification doesn't use: it fetches through verifier-core's own code.

    In every case, a removed continue or return was the last thing in its loop or function, with no labels or finally blocks, and the renamed variables shadow nothing new.

  • Limits:

    • This harness runs the plugin's functions, not the <verifier-credential> element on a page.
    • The stand-in network never answers with a redirect; none of the changed places handles redirects.
    • The element in a real browser is covered by CI's browser tests against the preview, and by the smoke test.

What was exposed

These are tools on developer machines and in CI. Nothing here reaches earners or the wallet. Each advisory names its conditions. Read against our setup (reasoned from the advisories and our config; not tested):

  • vite:
    • Files outside the project could be read only from a dev server exposed to the network. Ours sets neither server.host nor --host.
    • Cross-origin scripts need middleware mode or a custom app type; we use neither.
    • The launch-editor issues need Windows, or Vim, Emacs or Notepad++ together with a file an attacker has named.
  • undici:
    • The TLS bypass needs a custom checkServerIdentity function; the cache poisoning needs undici's shared cache composed with redirects. Checked: our two HTTP clients use undici only to turn a caller's agent option into an undici dispatcher, and nothing in our code or node_modules outside undici mentions checkServerIdentity or undici's cache or redirect interceptors.
    • The denial-of-service issues need a hostile server.
  • shell-quote: needs quote() called on attacker-controlled text. It arrives through React Native's developer tools, which nothing here runs.
  • source-map-js: needs a malicious indexed source map in the build.

Checks

  • Lint, typecheck and 105,720 unit tests, all through Vitest's updated vite 7.3.7. 60 browser tests against the vite 8.3.5 dev server (1 skipped: it runs only against a published copy, which CI's preview job does). Run locally on Node 26; CI runs Node 24.
  • Reviewed independently:
    • Rebuilt from scratch: fresh worktrees, npm ci. The builds are byte-identical to the author's, and identical on Node 24.
    • Lockfile: every entry, range, checksum and dependency list rechecked against the registry and the published manifests. npm ci installs exactly the lockfile's 482 packages.
    • Unminified differences: confirmed to be the three described above.
    • Side-by-side run: re-run with a stricter comparison that uses util.inspect, so undefined and key order count. It also compares requests and console output and calls all 14 exports, in both load orders. The reviewer ran it on the first 3,722 runs and I ran it on the did:web and ECDSA 88. All are identical, and the broken frame code is caught.
    • Changed places no test ran: read by hand, as above.
    • Site build: built from both commits; local browser runs fail the same 24 tests on both, because a built plugin refuses localhost. So CI's browser tests against this PR's preview are the first real-browser run of the new minified code.
    • Findings: no behaviour changes. Its four notes on this description are folded in: the ky and runtime chunks, the pako wording, the limits, and the run count.
  • Smoke test, comparing the live demo (main) with this PR's preview. Expected, written before running: no case differs. The preview is built by the new vite and minifier, so this exercises the new build in a real browser.
    Result: to be filled in after the preview is up.

Reaching the wallet

Nothing changes in the wallet until its verifier-plugin pin is bumped (it pins release commit a4ca3e6). When it is, it gets a dist/ minified by the new oxc, which is what the side-by-side run and the smoke test cover.

🤖 Generated with Claude Code

…map-js

Lockfile only, each within the range its parent already asks for:
vite 8.3.0 -> 8.3.5, both vite 7.3.6 copies (Vitest, vite-node) -> 7.3.7,
undici 6.28.1 -> 6.29.1 and 7.29.1 -> 7.30.1, shell-quote 1.10.0 -> 1.11.0,
source-map-js 1.2.1 -> 1.2.2. vite 8.3.5 needs rolldown ~1.2.14 and
postcss ^8.5.30; this takes those minimums (rolldown 1.2.14, published
14 October, rather than 1.2.16, published the day of this change), with
nanoid 3.3.20 for postcss. package.json is untouched.

None of these packages is bundled, but vite builds dist, and the build
does change: rolldown and its minifier (oxc 0.150 -> 0.154) shape the
same code differently. Built without minifying, the two bundles differ
only in two blank loop bodies' spacing and in how one CommonJS module's
top-level `this` is passed (rolldown 1.2.14's fix), which is the same
object. THIRD_PARTY_LICENSES.md and the .d.ts files are identical.

Checked: lint, typecheck, 105,720 tests, 60 browser tests (1 skipped, as
on any local run).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Try it: https://digitalcredentials.github.io/verifier-plugin/pr-39/

Updates on every push to this PR, and goes away when it closes. The browser tests run against this copy too.

This branch has not been deployed

No deployments
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