fix(enrollment): keep the stored pairing until the new device is bound #21

Merged
james.maes merged 10 commits from fix/enrollment-keeps-pairing-20261110 into release/2.0 2026-11-10 19:19:09 +00:00
Owner

What

EnrollNativeAsync now binds the new device to the session and checks the identity before it replaces the stored pairing. Saving the pairing, resetting the connection state and activating the session happen only after that.

Why

Found by the 2.0 baseline audit, present in 1.2.1. Enrollment saved the new pairing and session as soon as the server accepted the code, then bound the device. When the bind was refused or the connection dropped, the previous pairing was already overwritten while the log said "any existing pairing is unchanged". A transient failure could therefore cost a working pairing.

After this change a failed bind costs one enrollment code. The server keeps an unbound device record for the consumed code; the operator enrolls again with a fresh code.

How it was tested

  • New AgentNewPairingTests.BindFailureAfterAcceptedEnrollmentKeepsThePreviousPairing, two cases (bind refused, connection lost). Before the fix both fail at the byte comparison of connection.dpapi; after it both pass, and a second enrollment in the same runtime completes.
  • Full Windows suite locally: 379 passed. NativeCI Release build with -warnaserror: zero warnings.
  • Core.Tests not affected (no Core change) and not re-run for this branch.

Not done: no installer build, install or printing. Targets release/2.0; a separate pull request will offer the same fix to main for 1.2.x.

Checklist

  • Commits are signed and signed off (git commit -S -s)
  • Zero build warnings; format and lint are clean
  • Tests added or updated; the coverage gate still passes (the gate itself arrives with pull request 20)
  • Docs updated for user-visible changes (none: the operator-facing message was already "unchanged"; it is now true)
  • No secrets, credentials, or personal data
  • New dependencies are justified below, or none were added (none)
  • Substantial AI assistance, if any, is noted here: written with Claude Code (Claude Fable 5.2) under the owner's direction of 2026-11-10.

🤖 Generated with Claude Code

## What `EnrollNativeAsync` now binds the new device to the session and checks the identity before it replaces the stored pairing. Saving the pairing, resetting the connection state and activating the session happen only after that. ## Why Found by the 2.0 baseline audit, present in 1.2.1. Enrollment saved the new pairing and session as soon as the server accepted the code, then bound the device. When the bind was refused or the connection dropped, the previous pairing was already overwritten while the log said "any existing pairing is unchanged". A transient failure could therefore cost a working pairing. After this change a failed bind costs one enrollment code. The server keeps an unbound device record for the consumed code; the operator enrolls again with a fresh code. ## How it was tested - New `AgentNewPairingTests.BindFailureAfterAcceptedEnrollmentKeepsThePreviousPairing`, two cases (bind refused, connection lost). Before the fix both fail at the byte comparison of `connection.dpapi`; after it both pass, and a second enrollment in the same runtime completes. - Full Windows suite locally: 379 passed. NativeCI Release build with `-warnaserror`: zero warnings. - `Core.Tests` not affected (no Core change) and not re-run for this branch. Not done: no installer build, install or printing. Targets `release/2.0`; a separate pull request will offer the same fix to `main` for 1.2.x. ## Checklist - [x] Commits are signed and signed off (`git commit -S -s`) - [x] Zero build warnings; format and lint are clean - [x] Tests added or updated; the coverage gate still passes (the gate itself arrives with pull request 20) - [x] Docs updated for user-visible changes (none: the operator-facing message was already "unchanged"; it is now true) - [x] No secrets, credentials, or personal data - [x] New dependencies are justified below, or none were added (none) - [x] Substantial AI assistance, if any, is noted here: written with Claude Code (Claude Fable 5.2) under the owner's direction of 2026-11-10. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
james.maes added 1 commit 2026-11-10 17:16:59 +00:00
Enrollment saved the new pairing and session as soon as the server accepted
the code, then bound the device. When the bind was refused or the connection
dropped, the previous pairing had already been overwritten while the log said
"any existing pairing is unchanged".

The device is now bound and the identity checked first; the pairing is saved,
the connection state reset and the session activated only after that. A failed
bind costs one enrollment code and leaves both protected files untouched.
The failure message says so only when it is true.

Test: BindFailureAfterAcceptedEnrollmentKeepsThePreviousPairing (refused and
connection-lost cases) fails on the old order at the pairing-file comparison
and passes now; the full Windows suite passes (379).

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>
james.maes added 1 commit 2026-11-10 17:34:20 +00:00
Independent review of the reorder found no blockers and two new windows:

- The pairing file and the session file were no longer written together:
  resetting the connection state sat between them, so a fault there left
  the pairing naming the new device and the stored session naming the old
  identity. The session is now saved straight after the pairing.
- After the server bound the new device, a later failure left the live
  session bound to it while the stored files and the runtime still
  described the old pairing; nothing compared the two until the next
  renewal. Printing is now blocked until a sign-in in that case.

The failure message follows how far the enrollment got, and is logged for
any exception once the device is bound.

Tests: the bind-failure case also runs with an unreadable old pairing and
checks no recovery copy is made; a working pairing is shown to stay
authorized after a failed bind; and three post-bind failures (another
identity, pairing not writable, session not exportable) are shown to block
printing and say what was saved. The seven changed or new cases fail on the
previous commit; the Windows suite passes (385).

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>
james.maes added 1 commit 2026-11-10 17:47:30 +00:00
A second review found no blocker and two gaps in the previous commit.

The message for "pairing saved, session not" told the operator to sign in
again. When the stored session still carried the old device, that sign-in
is refused, as is a restart. The session to save is now built before the
pairing is written, so only one file write separates the two saves, and the
progress has a state for each: after the pairing alone the log says to
enroll again or start a new pairing; after both, that a sign-in or a
restart finishes the switch. Printing is blocked before the failure is
logged, so a fault while logging cannot leave the session usable.

The tests now exercise every recovery the messages promise: an ordinary
sign-in after the device was bound but nothing saved (three causes),
enrolling again after the pairing alone was saved, and a restarted service
after both were saved. The bind-failure cases assert that printing was not
blocked, which the earlier substring could not tell apart, and the working
pairing is shown to pass a further authorization pass. The fake session now
takes on a device when it is bound, as the server does, instead of when the
enrollment is accepted.

Windows suite: 388 passed; build has zero warnings.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>
james.maes added 2 commits 2026-11-10 18:16:02 +00:00
A confirmation review found one gap left. When the new pairing was saved
and its session was not, the log named the right recovery, but blocking
printing still set the status line and a warning to "Sign in again", and
that sign-in is refused because the stored session names the old device.
The block now takes the recovery to show; for this state it says to enroll
again or sign in choosing a new pairing.

The test asserts the status and log do not say "Sign in again", that an
ordinary sign-in is refused, and that enrolling again then completes. The
device-bound cases also pin the session file.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>
james.maes added 1 commit 2026-11-10 18:27:10 +00:00
Review found that the earlier fix only held while the service kept running.
The pairing and the session are two files, and a failure between the two
writes left them disagreeing on disk. After a restart the service told the
operator to sign in again, and that sign-in was refused.

The store now stages the change: the session is written under a staged
name, then the pairing is replaced, then the staged session is moved into
place. Any reader of the session (enrollment itself, a sign-in, the next
service start) finishes a staged change whose session names the stored
pairing's device, and discards one that does not, which is what a crash
before the pairing was replaced leaves behind. So the stored pairing and
session always agree by the time anything acts on them, and "sign in
again" is a true instruction in every blocked state. The separate
"pairing only" state and its special status text are gone.

Authorization is also re-checked after waiting for the gate, so a pass
that was waiting while enrollment blocked printing does not carry on.

Tests: an interrupted move into place is finished by an ordinary sign-in
and by a restart; a staged session for another device, or an unreadable
one, is discarded on read; a normal save removes a staged session; and the
bound-but-not-saved cases assert nothing is left staged.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>
james.maes added 1 commit 2026-11-10 18:41:01 +00:00
Review of the staged save found no blocker and these gaps, now closed:

- When the staged session could not be moved into place, the reader fell
  back to the session of the previous pairing. A sign-out then saved that
  binding and removed the staged one, after which every ordinary sign-in
  was refused. A staged session that belongs to the stored pairing is now
  the one every reader gets, whether or not the move has happened yet.
- An unreadable staged session was deleted. After the pairing has been
  replaced it may be the only copy, so it is moved aside under a recovery
  name instead. The staged file is flushed to disk before the pairing is
  replaced.
- A staged session is a leftover, and is deleted, when it names another
  device or server, when no pairing is stored, or when the stored session
  already belongs to the pairing; so a stale staged credential cannot
  replace a newer stored one.
- Moving, discarding or setting aside a session file was silent. The store
  reports what it did and the runtime logs it; sign-out's "revoke the
  previous session" warning also fires when the unreadable session was
  moved aside by this path.

Tests cover each outcome at the store, an interrupted save followed by a
sign-out and a sign-in, and promotion over an unreadable stored session.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>
james.maes added 1 commit 2026-11-10 18:54:11 +00:00
A confirmation review found one regression in the previous commit and two
test gaps.

- Discarding a leftover staged session, and setting an unreadable one
  aside, could throw when the file was held open, although the stored
  session was already the right answer. Both are now best-effort: the read
  returns the stored session and the next reader deals with the leftover.
- When the staged session cannot be moved into place, the cause is kept
  and logged, and enrollment's error carries it.
- A session file that was set aside is reported at sign-out whether it was
  the stored one or the staged one.

Tests: a sign-out while the staged file is held open, followed by a
sign-in; recovery attempted while the stored file is still held, which
must fail without changing the pairing or the staged session and succeed
after release; the log lines for a deferred move, a discard and a file set
aside; and the "another device" discard isolated from the "already
stored" rule.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>
james.maes added 1 commit 2026-11-10 19:05:09 +00:00
A confirmation review found the earlier items resolved and three left.

- A staged session that could not be deleted when a newer session was
  saved (the file was held open) could later replace that newer session,
  if the newer one did not belong to the stored pairing, such as a sign-in
  for a new pairing. A stored session written after the staged one now
  supersedes it.
- A leftover that could not be deleted was still reported as discarded,
  on every read. It is reported only once it is gone.
- The guarded discard and set-aside paths had no test. Both are now run
  with the staged file held open, and again after it is released.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>
james.maes added 1 commit 2026-11-10 19:14:26 +00:00
A confirmation review found the previous items resolved and one left: the
rule that a session saved after staging supersedes the staged one compared
file times, so a clock that stepped back between the two writes could make
the new pairing's session look like a leftover and delete it.

The order is now a logical one. Staging never stamps the staged session
earlier than the stored session it is about to replace, and a save that
cannot remove a staged session stamps itself later than it.

The tests set the other file's time into the future, as a clock step would
leave it, instead of forging the time the rule depends on: one shows the
save stamping itself later and the staged session being discarded, the
other shows staging keeping its order and the staged session being moved
into place.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>
james.maes merged commit 17797cd89a into release/2.0 2026-11-10 19:19:09 +00:00
Sign in to join this conversation.