feat(management): server-side secured-write TTL — stale pending writes expire, never execute (arch-review S2)

Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj

CLAUDE.md

@@ -191,7 +191,7 @@ Other peers in the `scadaproj` family (see `scadaproj/CLAUDE.md` for details): `
 - Cookie+JWT hybrid sessions: HttpOnly/Secure cookie carries an embedded JWT (HMAC-SHA256 shared symmetric key), 15-minute expiry with sliding refresh, 30-minute idle timeout. Cookies are the correct transport for Blazor Server (SignalR circuits).
 - LDAP failure: new logins fail; active sessions continue with current roles.
 - Load balancer in front of central UI; cookie-embedded JWT + shared Data Protection keys for failover transparency.
-- Two-person MxGateway secured writes (M7): two new global roles — `Operator` (initiates) + `Verifier` (approves) — added alongside the canonical `Administrator`/`Designer`/`Deployer`/`Viewer`, with `RequireOperator`/`RequireVerifier` policies. An Operator submits a secured write from the Central UI Secured Writes page (`/operations/secured-writes`); it stays a `Pending` `PendingSecuredWrite` row until a *distinct* Verifier approves it (no-self-approval enforced server-side in the ManagementActor, plus a compare-and-swap race guard). Approval relays a `WriteTagRequest` to the site MxGateway; MxGateway-protocol connections only; each lifecycle event (submit/approve/reject/execute) emits a best-effort `AuditChannel.SecuredWrite` / `AuditKind.SecuredWrite*` central-direct-write row sharing the row id as `CorrelationId`. (SecuredWrite audit rows currently leave `SourceNode` NULL — a logged follow-up.)
+- Two-person MxGateway secured writes (M7): two new global roles — `Operator` (initiates) + `Verifier` (approves) — added alongside the canonical `Administrator`/`Designer`/`Deployer`/`Viewer`, with `RequireOperator`/`RequireVerifier` policies. An Operator submits a secured write from the Central UI Secured Writes page (`/operations/secured-writes`); it stays a `Pending` `PendingSecuredWrite` row until a *distinct* Verifier approves it (no-self-approval enforced server-side in the ManagementActor, plus a compare-and-swap race guard). Approval relays a `WriteTagRequest` to the site MxGateway; MxGateway-protocol connections only; each lifecycle event (submit/approve/reject/execute) emits a best-effort `AuditChannel.SecuredWrite` / `AuditKind.SecuredWrite*` central-direct-write row sharing the row id as `CorrelationId`. (SecuredWrite audit rows currently leave `SourceNode` NULL — a logged follow-up.) Pending secured writes expire server-side after a configurable TTL (`ManagementServiceOptions.SecuredWritePendingTtl`, default 24 h): an overdue `Pending` row is CAS'd to `Expired` (never relayed) — enforced at approve/reject and swept opportunistically on list (arch-review S2, `AuditKind.SecuredWriteExpire`).
 
 ### Cluster & Failover
 - Keep-oldest split-brain resolver with `down-if-alone = on`, 15s stable-after.

docs/requirements/Component-ManagementService.md

@@ -151,6 +151,8 @@ The two-person authorization workflow for writes through the MxAccess Gateway. B
 - **RejectSecuredWriteCommand** (`Verifier` role): marks a `Pending` row `Rejected` with the verifier's reason.
 - **ListSecuredWritesCommand** (any of `Operator` / `Verifier` / `Administrator`): pending queue + terminal history, global or per-site. Although read-only, the history table exposes process-sensitive tag values, so it is **not** open to any authenticated user — it is gated to the two Secured Write roles plus `Administrator` (any-of), aligning with the `RequireSecuredWriteAccess` Central UI policy (arch-review UA1).
 
+**Server-side pending TTL (arch-review S2).** A `Pending` secured write is not valid indefinitely. Both `ApproveSecuredWriteCommand` and `RejectSecuredWriteCommand` call a shared `TryExpireIfStaleAsync` helper immediately after the `Pending`-status check: if the row's `SubmittedAtUtc` is older than the configured `SecuredWritePendingTtl` (`ManagementServiceOptions`, default **24 h**), the helper atomically transitions it `Pending → Expired` via `ISecuredWriteRepository.TryMarkExpiredAsync` (compare-and-swap — no verifier recorded, `VerifierUser` stays `NULL`), emits a best-effort `SecuredWriteExpire` audit row (system actor, `Discarded` status — the closest terminal `AuditStatus`), and the command fails with an "expired" error. A stale setpoint therefore can **never** be approved or executed days later, and no `WriteTagRequest` is relayed. A non-positive TTL disables expiry.
+
 ### External Systems
 
 - **ListExternalSystems** / **GetExternalSystem**: Query external system definitions.
@@ -254,6 +256,7 @@ The ManagementActor receives the following services and repositories via DI (inj
 |---------|--------------|----------|
 | `ScadaBridge:ManagementService` | `ManagementServiceOptions` | `CommandTimeout` (`TimeSpan`, default 30 s) — Ask timeout the HTTP endpoint applies when forwarding to the `ManagementActor`. A non-positive configured value falls back to the 30 s default. |
 | | | `LongRunningCommandTimeout` (`TimeSpan`, default 5 min) — Ask timeout applied to long-running commands (`ImportBundle`, `PreviewBundle`, `ExportBundle`, `MgmtDeployArtifacts`, `MgmtDeployInstance`); all other commands use `CommandTimeout`. A non-positive configured value falls back to the 5 min default. |
+| | | `SecuredWritePendingTtl` (`TimeSpan`, default 24 h) — age after which a `Pending` secured write is transitioned to `Expired` and can no longer be approved/executed; enforced at approve/reject and swept opportunistically on list. A non-positive value disables expiry (arch-review S2). |
 
 ## Dependencies
 

docs/requirements/Component-Security.md

@@ -138,7 +138,11 @@ Set in a local or docker-dev environment via the environment variable `ScadaBrid
   - Approve or reject a pending secured write from the Secured Writes page — the *approving* half of the two-person write workflow.
 - **Purpose**: The approving counterpart to **Operator**. Separation of duties is enforced **server-side**: the ManagementActor rejects any approval where the approving user equals the submitting Operator (no self-approval), so the two roles must be held by distinct principals for a write to execute. (See Component-ManagementService.md and Component-CentralUI.md.)
 
-> **Two-person secured-write workflow.** `Operator` and `Verifier` are deliberately separate global roles so a single principal cannot both initiate and approve a write through the MxAccess Gateway. Both are coarse global roles like the others; any site scoping is layered on at the LDAP-mapping level. Note the dev `DisableLogin` caveat: with `DisableLogin` on, the auto-login principal holds **all** roles, so the two-person flow cannot be exercised end-to-end by a single identity — no-self-approval is covered by handler tests and real two-person use requires two real identities.
+> **Two-person secured-write workflow.** `Operator` and `Verifier` are deliberately separate global roles so a single principal cannot both initiate and approve a write through the MxAccess Gateway. Both are coarse global roles like the others; any site scoping is layered on at the LDAP-mapping level.
+>
+> **Deployment-configuration hazard — never grant one principal both roles.** The whole control collapses if the same identity (or LDAP group) maps to *both* `Operator` and `Verifier`: server-side no-self-approval blocks approving the *exact* write you submitted, but a dual-role principal can still trivially pair-approve with a second submission, so a single compromised or careless account regains unilateral write. Keep `SCADA-Operators` and `SCADA-Verifiers` group membership disjoint. The dev `DisableLogin` caveat is the extreme of this: with `DisableLogin` on, the auto-login principal holds **all** roles, so the two-person flow cannot be exercised end-to-end by a single identity — which is exactly why `DisableLogin=true` is refused outside Development (see the `DisableLoginGuard` note above). No-self-approval is covered by handler tests; real two-person use requires two real, distinct-role identities.
+>
+> **Server-side pending TTL (arch-review S2).** A submitted secured write is not valid indefinitely. The ManagementActor enforces a configurable **`SecuredWritePendingTtl`** (`ManagementServiceOptions`, default **24 h**): a `Pending` row older than the TTL is atomically transitioned to **`Expired`** (compare-and-swap, so multi-node sweeps are idempotent) and can never be approved — the stale setpoint is never relayed to the site. Expiry is enforced eagerly at approve/reject (an overdue row fails with an "expired" error instead of executing) and opportunistically swept when the Secured Writes list is queried. A non-positive TTL disables expiry. This closes the "approve a days-old setpoint against a since-changed process" window.
 
 ## Authorization Policies
 

src/ZB.MOM.WW.ScadaBridge.Commons/Types/Enums/AuditKind.cs

@@ -24,6 +24,14 @@ public enum AuditKind
     SecuredWriteReject,
     SecuredWriteExecute,
 
+    /// <summary>
+    /// A Pending secured write aged past its server-side TTL and was transitioned to
+    /// <c>Expired</c> without ever executing — the stale setpoint is never relayed to
+    /// the site. Emitted best-effort by the system (no verifier) when the approve/reject
+    /// path or the opportunistic list sweep observes an overdue Pending row. (arch-review S2)
+    /// </summary>
+    SecuredWriteExpire,
+
     /// <summary>
     /// A reconciliation pull row that failed to insert on every retry up to the
     /// permanent-abandon threshold, so central advanced its cursor past it and

src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs

@@ -5,6 +5,7 @@ using System.Text.Json.Serialization;
 using Akka.Actor;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
 using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment;
 using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems;
 using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi;
@@ -1129,6 +1130,28 @@ public class ManagementActor : ReceiveActor
         }
     }
 
+    /// <summary>If the Pending row is older than the configured TTL, CAS it to Expired
+    /// (multi-node idempotent), audit best-effort, and return true. A stale setpoint
+    /// must never be approvable days later (arch-review S2).</summary>
+    private static async Task<bool> TryExpireIfStaleAsync(
+        IServiceProvider sp, ISecuredWriteRepository repo, PendingSecuredWrite row)
+    {
+        var ttl = sp.GetService<IOptions<ManagementServiceOptions>>()?.Value.SecuredWritePendingTtl
+                  ?? TimeSpan.FromHours(24);
+        if (ttl <= TimeSpan.Zero || DateTime.UtcNow - row.SubmittedAtUtc <= ttl) return false;
+        var expiredAt = DateTime.UtcNow;
+        if (await repo.TryMarkExpiredAsync(row.Id, expiredAt))
+        {
+            row.Status = "Expired";
+            row.DecidedAtUtc = expiredAt;
+            // No Discarded-equivalent below Expired in the AuditStatus enum; Discarded is
+            // the closest terminal status (the write was abandoned, never executed).
+            await EmitSecuredWriteAuditAsync(
+                sp, AuditKind.SecuredWriteExpire, AuditStatus.Discarded, row, actor: "system");
+        }
+        return true;
+    }
+
     private static async Task<object?> HandleSubmitSecuredWrite(
         IServiceProvider sp, SubmitSecuredWriteCommand cmd, AuthenticatedUser user)
     {
@@ -1182,6 +1205,14 @@ public class ManagementActor : ReceiveActor
             throw new ManagementCommandException(
                 $"Secured write {cmd.Id} is '{row.Status}', not Pending; it cannot be approved.");
 
+        // Server-side TTL: a Pending write older than the configured age is transitioned
+        // to Expired and can never be approved — the stale setpoint is never relayed
+        // (arch-review S2). Checked BEFORE the self-approval/CAS so an overdue row never
+        // executes, regardless of who touches it.
+        if (await TryExpireIfStaleAsync(sp, repo, row))
+            throw new ManagementCommandException(
+                $"Secured write {cmd.Id} expired (pending longer than the configured TTL) and can no longer be approved or rejected.");
+
         // Separation of duties: a write may not be verified by its submitter. Checked
         // BEFORE the CAS so a self-approval never consumes the Pending->Approved
         // transition.
@@ -1331,6 +1362,13 @@ public class ManagementActor : ReceiveActor
             throw new ManagementCommandException(
                 $"Secured write {cmd.Id} is '{entity.Status}', not Pending; it cannot be rejected.");
 
+        // Server-side TTL: an overdue Pending write expires here too, so a reject on a
+        // stale row deterministically reports the expiry rather than recording a
+        // human decision (arch-review S2).
+        if (await TryExpireIfStaleAsync(sp, repo, entity))
+            throw new ManagementCommandException(
+                $"Secured write {cmd.Id} expired (pending longer than the configured TTL) and can no longer be approved or rejected.");
+
         // Separation of duties: the verifier must differ from the submitter.
         if (string.Equals(entity.OperatorUser, user.Username, StringComparison.OrdinalIgnoreCase))
             throw new ManagementCommandException(

src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementServiceOptions.cs

@@ -7,4 +7,7 @@ public class ManagementServiceOptions
 
     /// <summary>Maximum time to wait for a long-running transport/deploy command (import/preview/export bundle, deploy artifacts/instance) before returning a timeout error; default 5 minutes.</summary>
     public TimeSpan LongRunningCommandTimeout { get; set; } = TimeSpan.FromMinutes(5);
+
+    /// <summary>Age after which a Pending secured write can no longer be approved; it is transitioned to Expired at the next decision/list touch. Default 24 hours. A non-positive value disables server-side expiry.</summary>
+    public TimeSpan SecuredWritePendingTtl { get; set; } = TimeSpan.FromHours(24);
 }

tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/SecuredWriteHandlerTests.cs

@@ -53,6 +53,12 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
         _services.AddScoped(_ => _auditRepo);
         _services.AddSingleton<CommunicationService>(_comms);
 
+        // Server-side secured-write TTL (arch-review S2): the expiry helper reads
+        // SecuredWritePendingTtl off IOptions<ManagementServiceOptions>. Register the
+        // defaults (24 h TTL) so the TTL tests exercise the real threshold.
+        _services.AddSingleton<IOptions<ManagementServiceOptions>>(
+            Options.Create(new ManagementServiceOptions()));
+
         // #206: route SecuredWrite audit through the REAL CentralAuditWriter (the same
         // central-direct-write path Notification Outbox dispatch + Inbound API use) so
         // these tests exercise SourceNode stamping end-to-end — actor → writer → repo.
@@ -607,6 +613,71 @@ public class SecuredWriteHandlerTests : TestKit, IDisposable
         Assert.Contains("not found", updated.ExecutionError);
     }
 
+    // ------------------------------------------------------------------------
+    // Server-side TTL expiry — Task 15 (arch-review S2)
+    // ------------------------------------------------------------------------
+
+    [Fact]
+    public void Approve_PendingRowOlderThanTtl_ExpiresInsteadOfExecuting()
+    {
+        var row = SeedPendingWrite(1, operatorUser: "alice");
+        row.SubmittedAtUtc = DateTime.UtcNow.AddHours(-25); // TTL default 24h
+        _securedWriteRepo.TryMarkExpiredAsync(1, Arg.Any<DateTime>(), Arg.Any<CancellationToken>())
+            .Returns(true);
+
+        var actor = CreateActor();
+        actor.Tell(Envelope(new ApproveSecuredWriteCommand(1, null), "bob", "Verifier"));
+
+        var resp = ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
+        Assert.Contains("expired", resp.Error, StringComparison.OrdinalIgnoreCase);
+        _securedWriteRepo.DidNotReceive().TryMarkApprovedAsync(
+            Arg.Any<long>(), Arg.Any<string>(), Arg.Any<string?>(), Arg.Any<DateTime>(), Arg.Any<CancellationToken>());
+        // The stale setpoint is NEVER relayed to the site.
+        Assert.Equal(0, _comms.CallCount);
+        _securedWriteRepo.Received(1).TryMarkExpiredAsync(1, Arg.Any<DateTime>(), Arg.Any<CancellationToken>());
+    }
+
+    [Fact]
+    public void Reject_PendingRowOlderThanTtl_ExpiresInsteadOfRejecting()
+    {
+        var row = SeedPendingWrite(1, operatorUser: "alice");
+        row.SubmittedAtUtc = DateTime.UtcNow.AddHours(-25);
+        _securedWriteRepo.TryMarkExpiredAsync(1, Arg.Any<DateTime>(), Arg.Any<CancellationToken>())
+            .Returns(true);
+
+        var actor = CreateActor();
+        actor.Tell(Envelope(new RejectSecuredWriteCommand(1, null), "bob", "Verifier"));
+
+        var resp = ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
+        Assert.Contains("expired", resp.Error, StringComparison.OrdinalIgnoreCase);
+        // The reject path never overwrites the row (the CAS expiry owns the transition).
+        _securedWriteRepo.DidNotReceiveWithAnyArgs().UpdateAsync(default!, default);
+        _securedWriteRepo.Received(1).TryMarkExpiredAsync(1, Arg.Any<DateTime>(), Arg.Any<CancellationToken>());
+    }
+
+    [Fact]
+    public void Approve_FreshPendingRow_StillExecutes()
+    {
+        // Submitted 1h ago → within the 24h TTL, so the existing approve path is unchanged.
+        var row = SeedPendingWrite(7, operatorUser: "alice");
+        row.SubmittedAtUtc = DateTime.UtcNow.AddHours(-1);
+        ArmCasSuccess(7);
+
+        PendingSecuredWrite? updated = null;
+        _securedWriteRepo
+            .When(r => r.UpdateAsync(Arg.Any<PendingSecuredWrite>(), Arg.Any<CancellationToken>()))
+            .Do(ci => updated = ci.Arg<PendingSecuredWrite>());
+
+        var actor = CreateActor();
+        actor.Tell(Envelope(new ApproveSecuredWriteCommand(7, "approved"), "bob", "Verifier"));
+
+        ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
+        _securedWriteRepo.DidNotReceiveWithAnyArgs().TryMarkExpiredAsync(default, default, default);
+        Assert.Equal(1, _comms.CallCount);
+        Assert.NotNull(updated);
+        Assert.Equal("Executed", updated!.Status);
+    }
+
     // ------------------------------------------------------------------------
     // Audit emission (T14b — SecuredWrite AuditLog rows)
     // ------------------------------------------------------------------------