fix(auth): decode pushbox Buffer data and add Firefox device tests - #21362
Closed
fxa-agent[bot] wants to merge 1 commit into
Closed
fxa-agent[bot] wants to merge 1 commit into
fxa-agent[bot] wants to merge 1 commit into
Conversation
## Because - Send Tab is broken on the receiving device. `GET /v1/account/device/commands` returns a 500 (errno 999). MySQL returns the pushbox `data` blob as a Buffer, and `Buffer.from(buffer, 'base64url')` copies a Buffer without decoding it. The bug came in with 1956b6d. - No functional test runs Firefox's own FxA device code, so no test caught it. ## This pull request - Fixes `decodeFromStorage` in `lib/pushbox/index.ts` so that it decodes Buffer data and string data. - Adds a unit test with Buffer data to `lib/pushbox/index.spec.ts`, and a store and retrieve round trip through MySQL to `pushbox_db.in.spec.ts`. Both fail without the fix. - Adds `firefox-device-helpers.ts` and `syncV3/firefoxDevices.spec.ts`. They sign two real Firefox profiles in to one account and check: - both profiles register as Send Tab devices and show in Connected services - Send Tab from A opens the tab on B (the end-to-end check for the fix) - disconnect B from Settings on A signs B out - a password change on A keeps A signed in and signs B out - Exports `pollUntil` from `pairing-helpers.ts`. ## Issue that this pull request solves Closes:
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Resolve the functional-test TypeScript error and correct the Pushbox Buffer type contract.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Fixes Pushbox Buffer decoding and adds Firefox device coverage for Send Tab and session invalidation.
Changes:
- Decode string and Buffer payloads correctly.
- Add unit and MySQL round-trip tests.
- Add Firefox device functional tests and polling helpers.
| File | Summary |
|---|---|
packages/fxa-auth-server/test/remote/pushbox_db.in.spec.ts |
Adds database round-trip coverage. |
packages/fxa-auth-server/lib/pushbox/index.ts |
Decodes Buffer payloads; update types to include Buffer and remove the cast. |
packages/fxa-auth-server/lib/pushbox/index.spec.ts |
Tests Buffer decoding. |
packages/functional-tests/tests/syncV3/firefoxDevices.spec.ts |
Adds Firefox device scenarios; fixes the boolean versus true return-type error. |
packages/functional-tests/lib/pairing-helpers.ts |
Exports pollUntil. |
packages/functional-tests/lib/firefox-device-helpers.ts |
Adds Firefox device automation helpers. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+53
to
+56
| return pollUntil( | ||
| async () => | ||
| (await getDeviceList(client)).every((d) => d.id !== deviceId) || | ||
| undefined, |
vbudhram
reviewed
Oct 1, 2026
| ); | ||
| } | ||
|
|
||
| test.describe('severity-2', () => { |
Contributor
There was a problem hiding this comment.
We will need to move this from tests/syncV3 folder, it isnt related to syncV3, perhaps we can have a generic sync folder?
Contributor
|
Closing for now |
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.
Because
GET /v1/account/device/commandsreturns a 500 (errno 999). MySQL returns the pushboxdatablob as a Buffer, andBuffer.from(buffer, 'base64url')copies a Buffer without decoding it. The bug came in with 1956b6d.This pull request
decodeFromStorageinlib/pushbox/index.tsso that it decodes Buffer data and string data.lib/pushbox/index.spec.ts, and a store and retrieve round trip through MySQL topushbox_db.in.spec.ts. Both fail without the fix.firefox-device-helpers.tsandsyncV3/firefoxDevices.spec.ts. They sign two real Firefox profiles in to one account and check:pollUntilfrompairing-helpers.ts.Issue that this pull request solves
Closes:
Checklist
Put an
xin the boxes that applyHow to review (Optional)
localtarget, because it disconnects devices and changes passwords. Is that right for the smoke runs?JWT_ACCESS_TOKENS_ENABLED_CLIENT_IDSset, because the sandbox has no CMS or Stripe and so no CapabilityManager. I expect CI to have both.Other information (Optional)
/fxa-verify --run --plan: unit 12/12, integration 7/7, lint pass, and the 4 functional tests pass. Without the fix, the Send Tab test fails with errno 999./pairflow gives Firefox no Sync keys.