Skip to content

fix(sdk-api): keep browser v1 decrypt to SJCL - #9684

Open
pranavjain97 wants to merge 1 commit into
masterfrom
pranavjain/wcn-2576-route-browser-v1-sjcl-decrypt-directly-to-legacy-decoder
Open

pranavjain97 wants to merge 1 commit into
masterfrom
pranavjain/wcn-2576-route-browser-v1-sjcl-decrypt-directly-to-legacy-decoder

Conversation

@pranavjain97

Copy link
Copy Markdown
Contributor

crypto-browserify's AES-CCM is reported broken in real browsers, so skip the native attempt there and go directly to sjcl.decrypt after the existing iteration-cap check. Also drops the unused, unreviewed WebCrypto CCM candidate (decryptV1WebCrypto.ts) that had no caller.

TICKET: WCN-2576

@linear-code

linear-code Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

WCN-2576

@ralph-bitgo
ralph-bitgo Bot force-pushed the pranavjain/wcn-2576-route-browser-v1-sjcl-decrypt-directly-to-legacy-decoder branch from 5f25978 to 8c12c59 Compare October 5, 2026 19:08
In browser bundles webpack maps `crypto` to `crypto-browserify`,
whose AES-CCM (browserify-aes) does not work. Every v1 decrypt
therefore paid for a full PBKDF2 run that was guaranteed to fail,
and then logged a fallback warning on every successful decrypt.
Route browser and worker runtimes straight to the frozen SJCL
decoder, gated by the same parseV1Envelope iter-cap check the
native path uses so DoS protection is preserved. Node keeps
native-first with the SJCL fallback.

Also drop the adjacent dead test seams: the Node-injected
decryptV1.browser.ts suite (it injected crypto-browserify but never
exercised real browser dispatch), the decryptV1WithCrypto injection
parameter that existed only for it, and the now-unused
crypto-browserify devDependency. Real browser dispatch is now
proven by a Cypress component spec on BitGoAPI.decrypt covering the
absent-`v` legacy envelope and the iter-cap boundary.
docs/sjcl-replacement.md states the engine boundary precisely.

Why: customers decrypting legacy v1 key material in browser bundles
pay a wasted PBKDF2 and see a misleading console warning per call,
and the SJCL deprecation effort needs an exact, documented boundary
between the native and frozen-SJCL code paths.

Ticket: WCN-2576
Session-Id: b906da45-6cf8-444a-9623-1e04551347ae
Task-Id: f81a1b1b-b7ee-46e9-a522-7708ed6ccc64
Requested-By: Pranav Jain
@pranavjain97
pranavjain97 force-pushed the pranavjain/wcn-2576-route-browser-v1-sjcl-decrypt-directly-to-legacy-decoder branch from 8c12c59 to 1ba0e92 Compare October 7, 2026 19:30
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

⚠️ Unit tests are failing on Node 26.x (Current release line, non-blocking). This is not an LTS version yet, so it does not block merge, but it signals an incompatibility to fix before Node 26.x becomes LTS.

View run

@pranavjain97
pranavjain97 marked this pull request as ready for review October 8, 2026 15:35
@pranavjain97
pranavjain97 requested review from a team as code owners October 8, 2026 15:35
Comment on lines +80 to +81
expect(error, 'iter above the cap must be rejected').to.be.an('error');
expect((error as Error).message).to.match(/iter/);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(error, 'iter above the cap must be rejected').to.be.an('error');
expect((error as Error).message).to.match(/iter/);
if (!(error instanceof Error)) {
throw new Error('Expected an Error rejection');
}
expect(error.message).to.match(/iter/);

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.

3 participants