Skip to content

fix(pwa): claim device codes atomically in /cli/device/token#51

Open
khipu-agent wants to merge 1 commit into
moshcoder:mainfrom
khipu-agent:fix/device-token-atomic-claim
Open

fix(pwa): claim device codes atomically in /cli/device/token#51
khipu-agent wants to merge 1 commit into
moshcoder:mainfrom
khipu-agent:fix/device-token-atomic-claim

Conversation

@khipu-agent

Copy link
Copy Markdown
Contributor

Bug

POST /cli/device/token (the moshcode login --device flow) reads the device code row, checks status === 'approved', and only then marks it claimed with an unconditional UPDATE. With a remote (network) database, two concurrent polls with the same device_code can both pass the status check before either UPDATE lands — so one single-use device code mints two API keys.

This is the same race as the one fixed in /cli/token (#46); the device-code exchange right below it has the identical read-check-write sequence.

Repro: apps/pwa/test/cli-device-token.test.mjs fires two concurrent polls (separate connections, statements deferred to macrotasks to simulate the remote DB the app runs against in production) — before the fix both return 200 and two rows land in api_keys.

Fix

Claim the code with a conditional UPDATE … SET status = 'claimed' WHERE device_code = ? AND status = 'approved' and reject the poll when no row was claimed — the same pattern as /cli/token.

Tests

New integration tests in apps/pwa/test/cli-device-token.test.mjs:

  • an approved code exchanges exactly once, then is rejected;
  • two concurrent polls → exactly one 200 and exactly one API key minted;
  • a pending code still returns authorization_pending and mints nothing.

Full suite: 167/167 green.

The read-check-claim sequence let two concurrent polls both read
"approved" before either UPDATE landed, so one single-use device code
minted two API keys. Make the claim conditional (status = 'approved')
and reject the loser, mirroring the /cli/token fix (moshcoder#46).
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