refactor(stdlib): remove jsonwebtoken native binding - #10687
proggeramlug wants to merge 2 commits into
Conversation
Fixes #10683. The hand-written native jsonwebtoken binding (crates/perry-ext-jsonwebtoken, plus a second duplicate implementation in crates/perry-stdlib/src/jsonwebtoken.rs exporting the same js_jwt_* symbols per #10678) has a live security defect: verify() returns null instead of throwing on every forgery case (tampered payload, wrong secret, alg:none, garbage token, tampered signature, expired token), so `try { jwt.verify(...) } catch { reject() }` never rejects a forgery. sign(..., { expiresIn: "1h" }) also silently drops the expiry (string coerces to NaN, and the runtime only writes `exp` when > 0.0). Removes both copies plus the dedicated codegen lowering path (lower_call/native/jsonwebtoken.rs's lower_jsonwebtoken_sign/_verify and its native_runtime_branch.rs dispatch), the decode-only NativeModSig row in native_table/utils_crypto.rs, the js_jwt_* FFI declarations in runtime_decls/stdlib_ffi/third_party.rs, the well_known_bindings.toml entry, the NATIVE_MODULES/manifest rows, the bundled-jsonwebtoken stdlib feature (re-wiring dep:rsa/dep:spki directly onto perry-stdlib's `crypto` feature, since webcrypto/key_object.rs and keys.rs need them independently of jsonwebtoken), and the Android stub exports. The real `jsonwebtoken` crates.io dependency stays — it is unrelated Rust tooling used by perry's own Apple code-signing (commands/run/resign.rs, commands/setup/common_apple.rs). Regenerated docs/api/perry.d.ts, docs/src/api/reference.md (--print-api-manifest) and docs/src/native-libraries/governance.md (binding_governance.py --table). Updated workspace-architecture.json (workspace_members 83->82, externalize 33->32) and scripts/string_payload_access_baseline.txt (perry-stdlib inline-offset sites 40->39, from the deleted stdlib file).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (23)
💤 Files with no reviewable changes (17)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe native ChangesNative runtime removal
Priority: ⬆️ High Estimated code review effort: 2 (Simple) | ~15 minutes Change: Bug fix · Severity of issue fixed: High 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
|
Landed via merge train #10716 (v0.5.1598). All source commits preserve authorship; merged main matches the validated train exactly. |
The rebase over origin/main (which already applied #10687's jsonwebtoken removal) resolved the manifest header-count conflict with placeholder values from before the rebase. Recompute them from the actual resolved tree via scripts/regen_api_docs.sh's two perry --print-api-manifest invocations: 2085 entries across 134 modules (perry.d.ts), 3027 entries across 136 modules (reference.md).
The rebase over origin/main (which already applied #10687's jsonwebtoken removal) resolved the manifest header-count conflict with placeholder values from before the rebase. Recompute them from the actual resolved tree via scripts/regen_api_docs.sh's two perry --print-api-manifest invocations: 2085 entries across 134 modules (perry.d.ts), 3027 entries across 136 modules (reference.md).
Summary
Fixes #10683 — the removal is the fix.
Removes the native
jsonwebtokenbinding soimport jwt from "jsonwebtoken"(no
perry.compilePackagesentry) resolves to the real npm package,compiled from source, per the owner's decision to stop shipping
hand-written Rust reimplementations of npm packages.
The security defect this closes
Native
verify()returnednullinstead of throwing on every forgerycase: tampered payload, wrong secret,
alg:none, garbage token, tamperedsignature, expired token. Any app that wrote the idiomatic
try { jwt.verify(token, secret) } catch { reject() }never rejected aforged token — it silently treated the forgery as valid-but-unauthenticated.
Native
sign(payload, secret, { expiresIn: "1h" })also silently droppedthe expiry: the string
"1h"coerced toNaNvia a numeric cast, and theruntime only wrote the
expclaim when the value was> 0.0, so the tokennever expired.
The real jsonwebtoken source (compiled from npm, no wrapper) matches Node
exactly, including all six thrown error names/messages.
What was found and removed
Per #10678 (duplicate
extern "C"exports acrossperry-ext-*/perry-stdlibpairs), this binding existed twice, plus a dedicated codegen fast path:
crates/perry-ext-jsonwebtoken/— the governance-tracked binding cratewired into
well_known_bindings.toml's[bindings.jsonwebtoken].crates/perry-stdlib/src/jsonwebtoken.rs(904 lines) — a second,independent implementation behind perry-stdlib's
bundled-jsonwebtokenfeature (default-on via the
cryptoumbrella), exporting the exact samejs_jwt_sign/js_jwt_verify/js_jwt_decode/*_es256/*_rs256/*_dyn/*_dyn_optssymbols as perry-ext-jsonwebtoken. This is the copy thecompiler actually calls:
sign/verifydon't go through the genericwell-known-binding FFI dispatch at all —
crates/perry-codegen/src/lower_call/native/jsonwebtoken.rs(
lower_jsonwebtoken_sign/_verify, algorithm-aware routing added injwt.sign algorithm option only matches inline string literals — const refs fall back to HS256 #1074) special-cases
module == "jsonwebtoken"directly innative_runtime_branch.rsand always lowers to perry-stdlib's symbols,bypassing whichever crate
well_known_bindings.tomlnames.decode(noalgorithm options) stayed in the generic
NativeModSigtable(
native_table/utils_crypto.rs) pointing atjs_jwt_decode.All three removed together — the dedicated codegen lowering path, both
duplicate FFI implementations, and every registry row:
crates/perry-ext-jsonwebtoken/(crate deleted)crates/perry-stdlib/src/jsonwebtoken.rs(deleted) + itsbundled-jsonwebtokenfeature and#[cfg(feature = "bundled-jsonwebtoken")]module wiring in
perry-stdlib/src/lib.rscrates/perry-codegen/src/lower_call/native/jsonwebtoken.rs(deleted) +its
mod/useinnative/mod.rsand the two dispatch branches innative_runtime_branch.rsjsonwebtokenNativeModSigrow innative_table/utils_crypto.rsjs_jwt_*FFI declarations inruntime_decls/stdlib_ffi/third_party.rs[bindings.jsonwebtoken]+.upstreaminwell_known_bindings.toml"jsonwebtoken"NATIVE_MODULESentry and its three manifest rows(
sign/verify/decode) inperry-api-manifest"jsonwebtoken" => &["bundled-jsonwebtoken"]line instdlib_features.rsjs_jwt_decode/sign/sign_es256/sign_rs256/verify)in
perry-ui-android/src/stdlib_stubs.rsperry-ext-jsonwebtokenworkspace member + path dependency in the rootCargo.tomlworkspace-architecture.json'sperry-ext-jsonwebtokendecision entry(
workspace_members83→82,externalize33→32)scripts/unrooted_local_shape_baseline.jsonOne subtlety, fixed in the same PR:
perry-stdlib'sbundled-jsonwebtokenfeature was
["dep:jsonwebtoken", "dep:p256", "dep:rsa", "dep:spki"]. Deletingit outright broke the build —
crates/perry-stdlib/src/webcrypto/key_object.rsand
keys.rsneeddep:rsa/dep:spkiunconditionally for WebCrypto(
to_pkcs8_pem/to_public_key_pem/to_pkcs8_der), unrelated to jsonwebtoken;they were only reachable through this feature by historical accident. Re-wired
dep:rsa/dep:spkidirectly onto perry-stdlib'scryptofeature so removingbundled-jsonwebtokendoesn't take WebCrypto down with it. Confirmeddep:jsonwebtoken(the crates.iojsonwebtokenRust crate) has no othercallers in perry-stdlib.
Left alone, deliberately: the real
jsonwebtokencrates.io dependency inthe root
Cargo.toml/crates/perry/Cargo.toml— this is unrelated Rusttooling used by Perry's own Apple code-signing
(
crates/perry/src/commands/run/resign.rs,crates/perry/src/commands/setup/common_apple.rs), nothing to do with thenpm binding. Also left alone: prose comments across perry-runtime/perry-codegen
that cite jsonwebtoken as a motivating example for unrelated compiler/runtime
behavior (e.g.
null_stub.rs'sjs_unresolved_default_callcomment,callable_exports.rs's safe-buffername-descriptor comment) — those explainwhy the compiler behaves a certain way, not the binding being removed.
test-files/test_issue_6675_createprivatekey_throws.tsmirrors jsonwebtoken'snormalizeSecretpattern in its comments but imports onlycrypto, notjsonwebtoken— untouched.Not touched, and why it matters: five standalone
test-files/*.tsregression fixtures (
test_issue_915_jwt_sign.ts,test_issue_915_jwt_sign_after_async_route.ts,test_issue_915_native_module_after_async_resume.ts,test_issue_927_jwt_verify_returns_object.ts,test_jwt_sign_dynamic_alg.ts)import jwt from "jsonwebtoken"and exercise the now-deleted dedicatedcodegen dispatch mechanism directly (dispatch-table row order across async
resume, algorithm-aware routing, etc.). Four of the five are already tracked
untriaged in
test-parity/known_failures.jsonunder the unrelated #8271 audit(dated 2026-08-17, long before this campaign); none has a
package.json/node_modulesof its own, so with the native module gone they now fail withan unresolved-import compile error instead of a parity mismatch —
verified directly (
test_jwt_sign_dynamic_alg.tsnow reports "no stdlibbinding / package not found" rather than a byte diff). Left as-is rather than
deleted or hand-patched: their entire subject (the deleted native dispatch
internals) no longer exists, updating them to install a real
jsonwebtokenwould test something they weren't written to test, and deleting fixtures is
out of scope for a removal PR per this campaign's own convention. Flagging
explicitly rather than leaving it for CI to discover silently.
Acceptance test: real sign/verify + 6 forgery cases, no
compilePackagesentryBuilt on perrymaster (
--profile perry-dev,-p perry -p perry-runtime-static -p perry-stdlib-static), confirmed.amtimes moved. Test project:{ "dependencies": { "jsonwebtoken": "^9.0.2" } }No
perry.compilePackagesentry at all. Compile log:Compile package wildcard: expanded to 15 installed package(s)(jsonwebtoken + jws, jwa, ms,lodash.*, semver, etc.), 77 native modules, real AOT compile from source.
Ran the binary and diffed against
node --experimental-strip-types(Node26.5.1, the pinned oracle): byte-for-byte identical except one unrelated,
pre-existing cosmetic difference — Perry's
JSON.parseerror message textfor malformed JSON doesn't match V8's exact wording (
"JSON parse error: malformed input"vs"Expected ',' or '}' after property value in JSON at position 44…"); the throw itself is identical on both sides, and this is ageneral JSON.parse gap unrelated to jsonwebtoken. All 6 forgery cases threw
with matching error names/messages on both sides:
expiresIn: "1h"now correctly encodes a realexpclaim (exp present: true), fixing the second reported defect.Verification
cargo check --workspace --all-targets(defaultdevprofile, notperry-dev— see note below) under-D warnings: clean, excluding thecross-host UI crates per this repo's own convention.
cargo build --profile perry-dev -p perry -p perry-runtime-static -p perry-stdlib-static: clean; confirmed the re-wireddep:rsa/dep:spkifix (without it,
perry-stdlibfails with 48 errors — multiple-spki-version trait resolution breaking
p256'sEncodePrivateKey/EncodePublicKeyimpls inwebcrypto/key_object.rs/keys.rs, unrelated tojsonwebtoken but exposed by deleting
bundled-jsonwebtokennaively).cargo test -p perry-api-manifest: 39+4 passing.cargo test -p perry-codegen --test manifest_consistency: 5/5 passing(
every_native_module_has_at_least_one_manifest_entry,every_well_known_binding_has_manifest_entry, etc.).python3 scripts/binding_governance.py --check: OK (39 extension crates).node scripts/binding_pins.mjs --check: OK (37 pinned, lock-step holds).python3 scripts/workspace_architecture.py --check: OK.cargo fmt --all -- --check: clean.scripts/run_lint_gates.sh(SKIP_COMPILE_GATES=1): 77 of 79 passed(compile tier skipped). Two reds: "Public benchmark evidence freshness" —
pre-existing on every PR in this repo, not touched here; "String
payload-access inventory" — genuinely caused by this PR (perry-stdlib's
inline-offset count dropped 40→39 with
jsonwebtoken.rsdeleted), fixed inthis PR via
--write-baseline.Not run / out of scope
run_lint_gates.sh(known-red on Linux per this campaign'scontract).
targeted registry/consistency tests plus the direct acceptance test instead.
CLAUDE.mdedit — per this campaign's convention, themaintainer bumps at merge time.
Summary by CodeRabbit
jsonwebtokenbinding.jsonwebtokenimports now resolve to the package fromnode_modules; otherwise, resolution may fail.sign,verify, anddecodeAPI support and their type declarations.jsonwebtoken.