fix(server): hand a new pairing code once the previous one was accepted by otavio · Pull Request #7599 · shellhub-io/shellhub

otavio · 2026-10-17T23:42:09Z

Summary

 CreateDevicePairing(public key)
   accepted device holds the key   -> answer accepted + its tenant
   cached code for the key
-    any status                    -> answer that code, its status and tenant
+    still pending                 -> answer that code, pending
+    accepted, device still absent -> drop it
   otherwise                       -> mint a new code

 AcceptDevicePairing(code), ResolveDeviceLoginCode(code)
   unknown or expired              -> not found
+  pairing code no longer pending  -> not found
   otherwise                       -> accept / preview

An agent whose paired device is removed gets a 401, forgets its tenant and asks for a pairing again
(66eaaa8). For the ten minutes the first code stays cached, the server handed back that code's
accepted status and tenant. The agent persisted the tenant, authenticated, got 401 for the removed
device, and started over. It never showed a new code and kept the server busy with about 30
pairing and auth requests a second until the cached code expired.

Accept now refuses a code that is no longer pending. Otherwise an accept page left open on the old
code could put the removed device back while the agent waits on its new code. Dropping the old code
alone doesn't close that, because an accept already in flight writes it back.

A store error during the accepted-device lookup is now returned instead of read as "not accepted",
so it can't discard a pairing that went through. An accepted cached code is checked against the store
again before it is dropped, in case its accept landed after the first lookup.

Evidence

Before: TestCreateDevicePairing/mints_a_new_code_when_the_public_key's_code_was_already_accepted
fails with expected: "pending" actual: "accepted". In the e2e stacked on this PR, the stack log
shows 607 POST /api/devices/pairing in 20 s, each followed by a 401 on /api/devices/auth.
After: the unit test passes, and the agent prints a second pairing code, is accepted and
connects again (TestRemovedPairedDeviceAgent/an_agent_that_learned_its_tenant_from_pairing_pairs_again).

Merge Danger

Door: two-way

Blast Radius: pairing

An agent that restarts after its code was accepted no longer gets the cached acceptance. It still
gets accepted + tenant from the device lookup that runs first, as long as the device is accepted.

Re-submitting an accepted code (a double click, a retry) now answers 404 instead of succeeding
again. In the console, reloading the accept page after a pairing was accepted, or opening it in a
second tab, now shows "This code is invalid or has expired" instead of the namespace picker.

otavio · 2026-10-18T20:03:41Z

/review

otavio · 2026-10-18T23:15:40Z

/review

otavio · 2026-10-19T00:36:36Z

/review

otavio · 2026-10-19T00:41:47Z

/review

otavio · 2026-10-19T00:48:27Z

/review

otavio · 2026-10-19T01:01:44Z

/review

otavio · 2026-10-19T01:19:42Z

/review

CreateDevicePairing reused the code cached for a public key for the whole code lifetime, including after the code was accepted, and then answered with the accepted status and tenant. An agent whose paired device is removed forgets its tenant and asks for a pairing again. If that happened within ten minutes of the pairing, it got the stale acceptance back, kept the tenant, authenticated, got 401 for the removed device, and started over: about 30 pairing and auth requests a second until the code expired. Only a pending code is reused now. AcceptDevicePairing now refuses a code that is no longer pending, the same not-found answer as an expired one. Without that, an accept page left open on the old code could put the removed device back in the namespace while the agent waits on its new code, which nobody accepts. The accepted code is also dropped from the cache when the new one is minted, but that alone does not close the hole: an accept already in flight on the old code writes it back after the drop. Refusing at accept time holds whichever side wins. Re-submitting an accepted code, a double click or a retry, now answers not found instead of succeeding again. The console's accept page resolves the code first, and the resolve used to answer for a pairing code whatever its status, so a reloaded or second-tab accept page offered the namespace picker and then failed with a generic error. The resolve now refuses a pairing code that is no longer pending too, and the page shows its usual "invalid or has expired" message. Dropping the accepted code made the accepted-device lookup load-bearing. Any lookup error used to read as "no accepted device", which was harmless while the cached code still answered accepted. Now it would discard a pairing that went through: an agent restarted before saving its tenant would be handed a fresh code nobody will accept, and it never asks again. Only a not-found result moves on to the cache; any other store error is returned, so the agent exits and retries. For the same reason, an accepted cached code triggers a second lookup before it is dropped: an accept can land between the first lookup and the cache read, as when `shellhub-agent login` runs while the daemon waits on the same code, and dropping it then would leave neither process with the tenant.

otavio · 2026-10-19T01:27:10Z

/review

github-actions · 2026-10-19T01:27:41Z

Claude finished @otavio's task in 3m 8s —— View job

Code Review Complete

Reviewed 4 files for Standards (repo rules, language conventions, tests, code smells) and Correctness (security and runtime defects). The Spec axis did not run: the PR closes no issue and its commit has no Fixes:/Closes:/Refs: trailer, so there is nothing to review it against. No new issues found. The code looks good as-is.

Notes

Last round's finding is fixed. The CreateDevicePairing and ResolveDeviceLoginCode doc comments now state only the contract. The reasons are in the commit message.
Dropped, below the bar:
- A double-click or retry after an accept that actually succeeded gets a 404. If AcceptDevicePairing fails at the device read-back (device-pairing.go:220) after it has cached the code as accepted (line 209), or the response is lost, a retry now gets 404. The console shows that as the generic "try again" error, not success. The device is still accepted and the agent still learns its tenant. The PR body already accepts 404 for retries, so I didn't post this.
- Doc comment wording. One agent wanted device-login.go:33-35 to name the not-found error for a non-pending code outright. It ends where the last round suggested, so raising it again would be churn. A rewording of the "Otherwise" sentence in device-pairing.go:44-47 was also dropped.
- Untested error path. No case covers a store error on the second public-key lookup (device-pairing.go:89). It doesn't break a rule I can quote.
Checked, nothing found:
- errors.Is against store.ErrNoDocuments (fromSQLError).
- Context reaches every store and cache call.
- Routes and permissions are unchanged.
- The agent persists a tenant only on accepted with a tenant, so the request loop can't come back.
- Nothing in cloud/ uses the changed symbols or the pairing_code keys.

otavio requested a review from a team as a code owner October 17, 2026 23:42

otavio added this pull request to stack #7601 October 17, 2026 23:42

otavio changed the title ~~fix/pairing pending code only~~ fix(server): hand a new pairing code once the previous one was accepted Oct 17, 2026

This comment has been minimized.

Sign in to view

github-actions Bot reviewed Oct 17, 2026

View reviewed changes

Comment thread server/api/services/device-pairing.go Outdated

github-actions Bot reviewed Oct 17, 2026

View reviewed changes

Comment thread server/api/services/device-pairing_test.go Outdated

github-actions Bot reviewed Oct 17, 2026

View reviewed changes

Comment thread server/api/services/device-pairing_test.go Outdated

otavio force-pushed the fix/pairing-pending-code-only branch from 79c675f to 00aa912 Compare October 18, 2026 20:03

This comment has been minimized.

Sign in to view

otavio force-pushed the fix/pairing-pending-code-only branch from 00aa912 to bbe178d Compare October 18, 2026 23:11

This comment has been minimized.

Sign in to view

github-actions Bot reviewed Oct 18, 2026

View reviewed changes

Comment thread server/api/services/device-pairing.go Outdated

github-actions Bot reviewed Oct 18, 2026

View reviewed changes

Comment thread server/api/services/device-pairing.go

otavio force-pushed the fix/pairing-pending-code-only branch from bbe178d to 0d47c63 Compare October 19, 2026 00:36

This comment has been minimized.

Sign in to view

github-actions Bot reviewed Oct 19, 2026

View reviewed changes

Comment thread server/api/services/device-pairing_test.go

otavio force-pushed the fix/pairing-pending-code-only branch from 0d47c63 to 3c2aa31 Compare October 19, 2026 00:41

This comment has been minimized.

Sign in to view

github-actions Bot reviewed Oct 19, 2026

View reviewed changes

Comment thread server/api/services/device-pairing.go Outdated

otavio force-pushed the fix/pairing-pending-code-only branch from 3c2aa31 to cefc124 Compare October 19, 2026 00:48

This comment has been minimized.

Sign in to view

github-actions Bot reviewed Oct 19, 2026

View reviewed changes

Comment thread server/api/services/device-pairing.go

otavio force-pushed the fix/pairing-pending-code-only branch from cefc124 to c4c4af5 Compare October 19, 2026 01:01

This comment has been minimized.

Sign in to view

github-actions Bot reviewed Oct 19, 2026

View reviewed changes

Comment thread server/api/services/device-pairing.go

github-actions Bot reviewed Oct 19, 2026

View reviewed changes

Comment thread server/api/services/device-pairing_test.go Outdated

otavio force-pushed the fix/pairing-pending-code-only branch from c4c4af5 to c1a4858 Compare October 19, 2026 01:19

This comment has been minimized.

Sign in to view

github-actions Bot reviewed Oct 19, 2026

View reviewed changes

Comment thread server/api/services/device-pairing.go Outdated

otavio force-pushed the fix/pairing-pending-code-only branch from c1a4858 to bb1b31d Compare October 19, 2026 01:27

gustavosbarreto merged commit a9e95df into master Oct 19, 2026
18 checks passed

gustavosbarreto deleted the fix/pairing-pending-code-only branch October 19, 2026 17:02

Conversation

otavio commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Evidence

Merge Danger

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

otavio commented Oct 18, 2026

Uh oh!

This comment has been minimized.

otavio commented Oct 18, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

otavio commented Oct 19, 2026

Uh oh!

This comment has been minimized.

Uh oh!

otavio commented Oct 19, 2026

Uh oh!

This comment has been minimized.

Uh oh!

otavio commented Oct 19, 2026

Uh oh!

This comment has been minimized.

Uh oh!

otavio commented Oct 19, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

otavio commented Oct 19, 2026

Uh oh!

This comment has been minimized.

Uh oh!

otavio commented Oct 19, 2026

Uh oh!

github-actions Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Complete

Notes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

otavio commented Oct 17, 2026 •

edited

Loading

github-actions Bot commented Oct 19, 2026 •

edited

Loading