fix: replace crypto-js with Web Crypto to remove DEP0169 warning#270
Open
CahidArda wants to merge 13 commits into
Open
fix: replace crypto-js with Web Crypto to remove DEP0169 warning#270CahidArda wants to merge 13 commits into
CahidArda wants to merge 13 commits into
Conversation
crypto-js (v4.2.0) internally calls the deprecated url.parse(), triggering Node.js DEP0169 warnings on every signature verification (#267). It was used in a single place to hash the request body, so replace it with the native Web Crypto API (crypto.subtle.digest), which jose already relies on and which works across Node 16+, browsers, Cloudflare Workers, Deno and edge runtimes. Also extend the Cloudflare Workers CI: the deployed worker now exposes a Receiver-backed /verify endpoint and a /publish endpoint that posts a message to it, and a new verify.test.ts polls the message logs until the signed message is delivered end-to-end.
…ntrol tests The DLQ and flow-control integration tests waited a fixed duration before asserting, which raced against QStash eventual consistency and flaked in CI. Convert them to poll via the existing eventually helper with timeouts longer than the sleeps they replace.
Replace every fixed sleep() in dlq.test.ts with eventually polling and remove the now-unused bun sleep import. Standardize all DLQ polls to a 20s timeout with 30s outer test timeouts so slower httpbin-backed cases (e.g. filter by label, filter by flowControlKey) stop flaking under backend latency.
- logs 'filter by label' (and the other label round-trip tests) called eventually without an explicit timeout, so they fell back to the helper's 5s default and timed out before the log appeared. Give them a 20s poll / 30s outer timeout. - url-group tests ran out of the default 5s while making several sequential API calls; bump them to 30s. The longer timeout also surfaced a latent bug: the add-endpoint test asserted on list[0], but list() returns every url group in the account. Look the group up by name instead.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the crypto-js dependency (which triggers Node.js DEP0169 warnings via deprecated url.parse()) by switching request-body hashing in the Receiver signature verification path to the native Web Crypto API. It also strengthens CI coverage by adding a Cloudflare Workers deployed end-to-end delivery round-trip test and increases resiliency of several integration tests via longer timeouts/retry polling.
Changes:
- Replace
crypto-jsSHA-256 + base64url hashing with Web Crypto (crypto.subtle.digest) inReceiver. - Improve stability of integration tests by using
eventually()polling and higher timeouts (logs/DLQ/flow-control/url-groups). - Extend Cloudflare Workers deployed CI to expose
/publish+/verifyendpoints and add an end-to-end delivery verification test.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/receiver.ts | Replaces crypto-js hashing with Web Crypto for signature verification. |
| src/client/url-groups.test.ts | Makes url-group tests less flaky by increasing timeouts and avoiding list-order assumptions. |
| src/client/logs.test.ts | Increases polling/test timeouts for logs integration tests. |
| src/client/flow-control.test.ts | Uses eventually() to avoid race when verifying flow-control reset behavior. |
| src/client/dlq.test.ts | Replaces fixed sleeps with eventually() polling and increases timeouts for DLQ tests. |
| package.json | Removes crypto-js and its types from dependencies. |
| examples/cloudflare-workers/verify.test.ts | Adds deployed CF Worker delivery round-trip test (publish → verify → delivered). |
| examples/cloudflare-workers/src/constants.ts | Adds a shared request body constant used by the new round-trip test. |
| examples/cloudflare-workers/src/ci.ts | Adds /publish and /verify endpoints; /verify uses Receiver for signature validation. |
| .github/workflows/test.yaml | Passes signing keys to deployed worker and runs the new round-trip test in CI. |
Comments suppressed due to low confidence (1)
src/client/dlq.test.ts:8
- This test imports
eventuallyfrom./logs.test, which will load and register the logs test suite wheneverdlq.test.tsis run in isolation (potentially adding unrelated tests and side effects). Extracteventuallyinto a shared non-test helper module and import it from there to keep test files independent.
import { afterAll, beforeAll, describe, expect, test } from "bun:test";
import { Client } from "./client";
import { eventually } from "./logs.test";
import { MOCK_QSTASH_SERVER_URL, mockQStashServer } from "./workflow/test-utils";
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Mirror the Cloudflare delivery round-trip in the Next.js example: a /roundtrip/publish route publishes a message to a verifySignatureAppRouter-backed /roundtrip/verify endpoint on the same app, and verify.test.ts polls the message logs until QStash reports DELIVERED. The nextjs-deployed job injects the token and signing keys into the Vercel deployment and runs the test. Also add a negative test to both examples that hits the verifier endpoints directly without a signature and asserts they reject with 403 instead of 200, and make the cloudflare worker's /verify return 403 to match.
- sha256Base64url now reads globalThis.crypto explicitly and throws a clear '[Upstash QStash] Web Crypto API is not available' error when it's missing, and uses jose.base64url.encode (already a dependency) instead of a manual btoa+replace base64url transform. - Move the eventually test helper out of logs.test.ts into test-utils/eventually.ts so importing it no longer loads the entire logs suite as a side effect. Update all importers.
Add scripts/check-webcrypto-node.mjs, which signs a request and verifies it through both the global Web Crypto path and a forced node:crypto fallback, and run it in CI on Node 18/20/22/24 and Bun. release depends on these jobs. This is committed before the receiver fix on purpose, to confirm the new jobs catch the older-Node regression (they should fail until the fallback lands).
A published message isn't immediately retrievable via messages.get, so the label/multiple-labels E2E tests 404'd intermittently. Wrap the lookups in eventually polling instead of a single immediate get.
The forced phase removed globalThis.crypto on every runtime, so a missing fallback failed all versions and obscured which ones break in practice. Keep only the native-path check so the matrix reflects real behavior: Node < 19 (no global Web Crypto) is the version that actually needs the node:crypto fallback.
globalThis.crypto is only a default global on Node >= 19, so the Web-Crypto-only hashing threw 'Web Crypto API is not available' on Node 16/17/18 (where crypto-js used to work). sha256Base64url now prefers the global and falls back to node:crypto's webcrypto on older Node — the same approach jose uses — so signature verification keeps working on every runtime. Confirmed by the receiver-node-versions CI matrix, which failed on Node 18 until this change.
The static import("node:crypto") broke Next.js's webpack build (UnhandledScheme
error), including the edge route, even though that branch never runs on edge.
Hold the specifier in a variable and mark the dynamic import webpackIgnore /
@vite-ignore so webpack and Vite leave it as a runtime import. Verified the
Next.js example (edge + serverless routes) now builds, and the node:crypto
fallback still works when globalThis.crypto is absent.
Build and run the Next.js example on Node 18/20/22/24 so bundler/runtime regressions (like the node:crypto import that broke the webpack build) are caught on every supported Node version, not just the latest.
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.
Replace
crypto-jswith Web Crypto to remove the DEP0169 warningCloses #267.
Problem
On recent Node.js (e.g. v24), every signature verification logged a deprecation
warning:
The source was our
crypto-jsdependency (v4.2.0), which internally calls thedeprecated
url.parse(). We only usedcrypto-jsin one place — hashing therequest body in
Receiver— so it was easy to drop.What changed
Drop
crypto-js, hash with Web Crypto (src/receiver.ts)The body hash now uses the platform-native Web Crypto API plus
jose(already adependency) for base64url encoding:
Why the
node:cryptofallback matters (don't break older Node)globalThis.cryptois only exposed as a global by default on Node.js ≥ 19.On Node 16/17/18 it is not present unless you pass
--experimental-global-webcrypto.A Web-Crypto-only implementation would therefore throw on Node 18 (still LTS),
where
crypto-jspreviously worked — a silent regression for those users.The fallback mirrors exactly what
josedoes internally (its Node build readsnode:crypto'swebcrypto), soReceiver.verify()keeps working on everyruntime.
Dependency change
crypto-js(dependency) and@types/crypto-js(devDependency).jose(base64url.encode) and the runtime'sbuilt-in Web Crypto.
Runtime support
crypto.subtlenode:cryptofallbacknodejs_compat)New CI: cross-runtime guard
scripts/check-webcrypto-node.mjssigns a request and assertsReceiver.verify()accepts it, run across a matrix so this can't regress:Receiver Web Crypto (Node 18/20/22/24)— Node 18 exercises thenode:cryptofallback; 20/22/24 use the global.Receiver Web Crypto (Bun)— exercises the Bun runtime.The
releasejob now depends on both. These jobs were intentionally landedbefore the fix to confirm they actually catch the regression: the Node 18 job
was red until the
node:cryptofallback was added.End-to-end delivery tests for the examples
Added a real publish → verify → delivered round-trip to both examples (runs in
the deployed CI jobs, which have a public URL QStash can reach):
examples/cloudflare-workers): a/publishroutepublishes to a
Receiver-backed/verifyroute on the same worker; the testpolls the message logs until
DELIVERED. Signing keys are injected intowranglervars at deploy.examples/nextjs):/roundtrip/publishpublishes to averifySignatureAppRouter-backed/roundtrip/verifyroute; the test pollsthe logs until
DELIVERED. The token and signing keys are injected into theVercel deployment.
Each example also gets a negative test that hits the verifier endpoints directly
without a signature and asserts they reject (Cloudflare 401, Next.js 403)
instead of returning 200.
Test flakiness cleanup
Several integration tests waited a fixed
sleep()before asserting, which racedagainst QStash's eventual consistency and flaked in CI. They now poll via the
shared
eventuallyhelper:sleep()→eventuallypolling.logslabel tests: gaveeventuallyan explicit timeout (they were using thehelper's short default).
url-grouptests: longer timeout; this surfaced and fixed a latent bug — thetest asserted on
list[0], buturlGroups.list()returns every group, soit now looks the group up by name.
E2E Publishlabel tests: pollmessages.getinstead of a single immediateget that 404'd before the message was queryable.
eventuallyhelper out oflogs.test.tsintotest-utils/eventually.tsso importing it no longer loads the entire logssuite as a side effect.
Verification
bun run lint(tsc + eslint) — clean.globalThis.cryptoand verifying a real signature (verify: true).