fix(enrollment): order the stored and staged session without the clock

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>

docs/QQQ-SIGN-IN.md

@@ -20,7 +20,7 @@ The server and its identity provider govern session lifetime, renewal and curren
 
 Explicit sign-in can replace unreadable saved QQQ session data only after fresh server authentication and, when device proof survives, authoritative device binding. Rejected binding leaves the unreadable file untouched. Successful recovery preserves its ciphertext in a uniquely named `qqq-session.recovery-*.dpapi` file in the protected data directory.
 
-Enrollment replaces the pairing and its session as one staged step. The session for the new pairing is first written as `qqq-session.staged.dpapi`; once the pairing is replaced, the next read of the session (enrollment itself, a sign-in or a service start) moves it into place, preserving an unreadable earlier session as `qqq-session.recovery-*.dpapi`. A staged session that does not belong to the stored pairing is discarded, and one that cannot be read is preserved under a recovery name. The activity log records each of these.
+Enrollment replaces the pairing and its session as one staged step. The session for the new pairing is first written as `qqq-session.staged.dpapi`; once the pairing is replaced, the next read of the session (enrollment itself, a sign-in or a service start) moves it into place, preserving an unreadable earlier session as `qqq-session.recovery-*.dpapi`. A staged session is discarded when it does not belong to the stored pairing, or when the stored session already does or was saved after it; one that cannot be read is preserved under a recovery name. Until a staged session can be moved into place it is still the one that is used. The activity log records each of these once it has happened; a leftover that cannot be removed yet is dealt with, and logged, by a later read.
 
 If device proof itself is unreadable, sign-in does not enable printing or reconstruct that proof. Explicit enrollment is required; it preserves the old ciphertext as `connection.recovery-*.dpapi`, printer choices and the old connection's journal without attaching those receipts to the new device. Read-only status remains available. Explicit sign-out can quarantine unreadable session data locally, but cannot revoke an unavailable server session; revoke that previous session in QQQ. Automatic connection recovery does not perform this explicit credential replacement.
 

src/Qqq.PrintAgent.Windows/AgentRuntime.Authentication.cs

@@ -159,7 +159,7 @@ public sealed partial class AgentRuntime
                + (store.StagedPairingFailure is null ? "not recorded." : QqqSessionDiagnostics.Describe(store.StagedPairingFailure)));
             break;
          case StagedPairingOutcome.Discarded:
-            WriteLog("INFO", "Discarded a session left over from a pairing change: it did not belong to the stored pairing, or the stored session already did or is newer.");
+            WriteLog("INFO", "Discarded a session left over from a pairing change: it was for another pairing, or the stored session is newer or already belongs to this one.");
             break;
          case StagedPairingOutcome.SetAside:
             WriteLog("WARN", "A session staged for a pairing change could not be read and was moved aside for recovery.");

src/Qqq.PrintAgent.Windows/AgentStore.Authentication.cs

@@ -125,15 +125,20 @@ public sealed partial class AgentStore
    /// <summary>
    /// Saves the session protected for the service account, writing a temporary file and moving it into place so a crash cannot
    /// leave a half-written secret. An unreadable earlier file is quarantined first. A staged session from an unfinished pairing
-   /// change is removed, because what is saved here is newer than it.
+   /// change is removed, because what is saved here is newer than it. When it cannot be removed, the saved file is stamped
+   /// later than the staged one, so the order the two were written in does not depend on the clock having moved forward.
    /// </summary>
    public void SaveAuthentication(AgentAuthentication authentication)
    {
       string path = Path.Combine(_directory, SessionFile);
+      string staged = Path.Combine(_directory, StagedSessionFile);
       WriteProtected(path + ".tmp", authentication);
       _ = QuarantineUnreadableAuthentication();
       File.Move(path + ".tmp", path, overwrite: true);
-      _ = DeleteQuietly(Path.Combine(_directory, StagedSessionFile));
+      if(!DeleteQuietly(staged) && File.GetLastWriteTimeUtc(path) <= File.GetLastWriteTimeUtc(staged))
+      {
+         File.SetLastWriteTimeUtc(path, File.GetLastWriteTimeUtc(staged).AddTicks(1));
+      }
    }
 
 
@@ -143,13 +148,19 @@ public sealed partial class AgentStore
    /// staged name, and the pairing is replaced second. From then on the change is decided: every reader of the session
    /// uses the staged one and moves it into place, because it names the stored pairing's server and device. If the pairing
    /// cannot be written the staged session is removed and nothing has changed; a staged session left by a crash before the
-   /// pairing was replaced names a different device and is discarded by the next reader.
+   /// pairing was replaced names a different device and is discarded by the next reader. The staged file is never stamped
+   /// earlier than the stored session it is about to replace, whatever the clock did in between.
    /// </summary>
    internal void StagePairing(AgentConfiguration configuration, AgentAuthentication authentication)
    {
       string staged = Path.Combine(_directory, StagedSessionFile);
+      string path = Path.Combine(_directory, SessionFile);
       WriteProtected(staged + ".tmp", authentication);
       File.Move(staged + ".tmp", staged, overwrite: true);
+      if(File.Exists(path) && File.GetLastWriteTimeUtc(path) > File.GetLastWriteTimeUtc(staged))
+      {
+         File.SetLastWriteTimeUtc(staged, File.GetLastWriteTimeUtc(path));
+      }
       try
       {
          SaveConfiguration(configuration);

src/Qqq.PrintAgent.Windows/StagedPairingOutcome.cs

@@ -12,7 +12,7 @@ namespace Qqq.PrintAgent.Windows;
 /// </summary>
 internal enum StagedPairingOutcome
 {
-   /// <summary>Nothing was staged.</summary>
+   /// <summary>Nothing was staged, or a leftover could not be removed yet and waits for the next read.</summary>
    None,
 
    /// <summary>The staged session belonged to the stored pairing and was moved into place.</summary>

tests/Qqq.PrintAgent.Cli.Tests/AgentNewPairingTests.cs

@@ -646,8 +646,9 @@ public sealed class AgentNewPairingTests
 
    /// <summary>
    /// A session saved after a pairing change was staged is newer than the staged one, even when it does not belong to the
-   /// stored pairing (a sign-in for a new pairing, say) and the staged file could not be removed at the time. The staged
-   /// session must not replace it later.
+   /// stored pairing (a sign-in for a new pairing, say) and the staged file could not be removed at the time. The save
+   /// stamps itself later than the staged file, here although the staged file carries a time in the future as it would
+   /// after the clock stepped back, and the staged session does not replace it later.
    /// </summary>
    [Fact]
    public void SessionSavedAfterStagingIsNotReplacedByTheStagedOne()
@@ -656,13 +657,14 @@ public sealed class AgentNewPairingTests
       fixture.SaveSession();
       string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi");
       Stage(fixture, "replacement-device", "https://synthetic.invalid/");
+      File.SetLastWriteTimeUtc(StagedSession(fixture), DateTime.UtcNow.AddHours(1));
       using(FileStream held = new(StagedSession(fixture), FileMode.Open, FileAccess.Read, FileShare.Read))
       {
          using AgentStore saving = new(fixture.Local.DirectoryPath);
          saving.SaveAuthentication(new AgentAuthentication(fixture.Session.Binding with { DeviceId = null }, fixture.Session.ExportCredential()));
       }
       Assert.True(File.Exists(StagedSession(fixture)));
-      File.SetLastWriteTimeUtc(authPath, File.GetLastWriteTimeUtc(StagedSession(fixture)).AddSeconds(1));
+      Assert.True(File.GetLastWriteTimeUtc(authPath) > File.GetLastWriteTimeUtc(StagedSession(fixture)));
       byte[] saved = File.ReadAllBytes(authPath);
       using AgentStore store = new(fixture.Local.DirectoryPath);
       Assert.Null(store.LoadAuthentication()!.Binding.DeviceId);
@@ -673,6 +675,28 @@ public sealed class AgentNewPairingTests
 
 
 
+   /// <summary>
+   /// Staging never stamps the staged session earlier than the stored session it is about to replace, so a clock that
+   /// stepped back between the two writes cannot make the new pairing's session look like a leftover: it is still moved
+   /// into place. The stored session here carries a time in the future, as it would after such a step.
+   /// </summary>
+   [Fact]
+   public void StagedSessionIsNotOlderThanTheSessionItReplaces()
+   {
+      using SessionFixture fixture = new();
+      fixture.SaveSession();
+      string authPath = Path.Combine(fixture.Local.DirectoryPath, "qqq-session.dpapi");
+      File.SetLastWriteTimeUtc(authPath, DateTime.UtcNow.AddHours(1));
+      Stage(fixture, "replacement-device", "https://synthetic.invalid/");
+      Assert.Equal(File.GetLastWriteTimeUtc(authPath), File.GetLastWriteTimeUtc(StagedSession(fixture)));
+      using AgentStore store = new(fixture.Local.DirectoryPath);
+      Assert.Equal("replacement-device", store.LoadAuthentication()!.Binding.DeviceId);
+      Assert.Equal(StagedPairingOutcome.Completed, store.StagedPairing);
+      Assert.False(File.Exists(StagedSession(fixture)));
+   }
+
+
+
    /// <summary>Stages a pairing for the given device together with a session that names the given device on the given server, without finishing the change.</summary>
    private static void Stage(SessionFixture fixture, string deviceId, string sessionOrigin)
    {