Architecture remediation: P1 tier (process & hardening) #121

Merged
dohertj2 merged 19 commits from fix/archreview-p1 into main 2026-07-09 09:59:49 -04:00
12 changed files with 192 additions and 14 deletions
Showing only changes of commit a038363e74 - Show all commits
+1
View File
@@ -257,6 +257,7 @@ Findings the review flagged as one coordinated design pass — sequence them tog
| 2026-07-09 | P0 tier executed via parallel agents. GWC-01/02/03, TST-02, TST-12, CLI-01, CLI-03 → `Done` (NonWindows build clean; gateway 135/135 targeted tests pass, Go `go test ./...` + `-race` pass, Rust `test`/`clippy -D warnings` pass). | | 2026-07-09 | P0 tier executed via parallel agents. GWC-01/02/03, TST-02, TST-12, CLI-01, CLI-03 → `Done` (NonWindows build clean; gateway 135/135 targeted tests pass, Go `go test ./...` + `-race` pass, Rust `test`/`clippy -D warnings` pass). |
| 2026-07-09 | Windows full-suite temp-file-lock flakiness (discovered via TST-08) → **fixed** (commit `11a716a`). Two shared-state parallel collisions that macOS never surfaces: (1) self-signed cert generation — `SelfSignedCertificateProvider` now stages the PFX in a unique `<path>.<guid>.tmp` (was a fixed name that concurrent/interrupted writers collided on); `TestHostEnvironmentInitializer` defaults the cert path to a per-process temp dir (so the suite never writes shared `ProgramData` or fights the deployed service); `GatewayTlsBootstrapTests` moved to a `DisableParallelization` collection so its process-global env-var mutation can't bleed into parallel tests. (2) `AuthStoreHealthCheckTests` — reuse the existing `TempDatabaseDirectory` helper (`SqliteConnection.ClearAllPools()` before delete) so the Microsoft.Data.Sqlite pool releases the temp `.db` handle. **windev-verified:** 3 consecutive full-suite runs all **793 passed / 2 failed, 0 new orphans** (was flaky 24 fails, count-varying). The 2 stable remaining failures are pre-existing windev-environmental (`SelfSignedCertificateProviderTests.GenerateCertificate_HasExpectedSansEkuAndValidity` machine-name/FQDN SAN assertion; `EventStreamServiceTests…TracksAggregateQueueDepth` timing) — both pass on macOS, unrelated to this change. | | 2026-07-09 | Windows full-suite temp-file-lock flakiness (discovered via TST-08) → **fixed** (commit `11a716a`). Two shared-state parallel collisions that macOS never surfaces: (1) self-signed cert generation — `SelfSignedCertificateProvider` now stages the PFX in a unique `<path>.<guid>.tmp` (was a fixed name that concurrent/interrupted writers collided on); `TestHostEnvironmentInitializer` defaults the cert path to a per-process temp dir (so the suite never writes shared `ProgramData` or fights the deployed service); `GatewayTlsBootstrapTests` moved to a `DisableParallelization` collection so its process-global env-var mutation can't bleed into parallel tests. (2) `AuthStoreHealthCheckTests` — reuse the existing `TempDatabaseDirectory` helper (`SqliteConnection.ClearAllPools()` before delete) so the Microsoft.Data.Sqlite pool releases the temp `.db` handle. **windev-verified:** 3 consecutive full-suite runs all **793 passed / 2 failed, 0 new orphans** (was flaky 24 fails, count-varying). The 2 stable remaining failures are pre-existing windev-environmental (`SelfSignedCertificateProviderTests.GenerateCertificate_HasExpectedSansEkuAndValidity` machine-name/FQDN SAN assertion; `EventStreamServiceTests…TracksAggregateQueueDepth` timing) — both pass on macOS, unrelated to this change. |
| 2026-07-09 | P1 TST-08 (orphaned testhost) → `Done` by evidence: the claimed leak does **not** reproduce. Full `ZB.MOM.WW.MxGateway.Tests` suite exits cleanly on macOS (0 survivors) and on the Windows dev box (windev, `origin/main` worktree: 0 new `testhost` and 0 new `ZB.MOM.WW.MxGateway.Worker` processes after a full run, isolated by StartTime). Static audit confirms the prime-suspect fixtures already dispose deterministically (WorkerClient cancels its `_stopCts` + awaits read/write/heartbeat tasks with a 5 s timeout; GatewaySession disposes the client; E2E/harness fixtures use `await using`). CLAUDE.md "Source Update Workflow" updated to drop the stale "leaves orphaned testhost processes" rationale (filtered runs kept as a speed guideline). **Discovered separately (NOT TST-08, still open):** Windows-only temp-file-lock flakiness in ~4 full-host test classes — `AuthStoreHealthCheckTests` (Microsoft.Data.Sqlite connection-pool holds the temp `.db` handle after `await using`, so the finally `File.Delete` throws "used by another process"), `GatewayTlsBootstrapTests`/`DashboardCookieOptionsTests`/`DashboardHubsRegistrationTests` (a `gw.pfx.tmp` handle from `SelfSignedCertificateProvider`'s fixed-name `path+".tmp"` atomic-write racing teardown). Count varies run-to-run (24 fails); macOS never sees it (Unix allows deleting open files). Fix = `SqliteConnection.ClearAllPools()` before temp delete + a unique/guarded `.tmp` name; verify on windev. | | 2026-07-09 | P1 TST-08 (orphaned testhost) → `Done` by evidence: the claimed leak does **not** reproduce. Full `ZB.MOM.WW.MxGateway.Tests` suite exits cleanly on macOS (0 survivors) and on the Windows dev box (windev, `origin/main` worktree: 0 new `testhost` and 0 new `ZB.MOM.WW.MxGateway.Worker` processes after a full run, isolated by StartTime). Static audit confirms the prime-suspect fixtures already dispose deterministically (WorkerClient cancels its `_stopCts` + awaits read/write/heartbeat tasks with a 5 s timeout; GatewaySession disposes the client; E2E/harness fixtures use `await using`). CLAUDE.md "Source Update Workflow" updated to drop the stale "leaves orphaned testhost processes" rationale (filtered runs kept as a speed guideline). **Discovered separately (NOT TST-08, still open):** Windows-only temp-file-lock flakiness in ~4 full-host test classes — `AuthStoreHealthCheckTests` (Microsoft.Data.Sqlite connection-pool holds the temp `.db` handle after `await using`, so the finally `File.Delete` throws "used by another process"), `GatewayTlsBootstrapTests`/`DashboardCookieOptionsTests`/`DashboardHubsRegistrationTests` (a `gw.pfx.tmp` handle from `SelfSignedCertificateProvider`'s fixed-name `path+".tmp"` atomic-write racing teardown). Count varies run-to-run (24 fails); macOS never sees it (Unix allows deleting open files). Fix = `SqliteConnection.ClearAllPools()` before temp delete + a unique/guarded `.tmp` name; verify on windev. |
| 2026-07-09 | SEC-10 polish (gateway-side follow-up to the G-2 auth-lib core) → done. `apikey create-key` gains an optional `--expires` (absolute ISO-8601 UTC or relative `<N>d`/`<N>h`; omit = non-expiring, opt-in) threaded through `ApiKeyAdminCommand`/parser/runner into the library's `CreateKeyAsync(..., expiresUtc, ...)`; `list-keys` shows an expiry column + `active`/`expired`/`revoked` status. Dashboard API Keys page surfaces expiry: an `Expires` column (`Never` when unset) and a status badge reading `Expired` (red) / `Expiring` (≤7 days, amber) / `Revoked` / `Active` (`DashboardApiKeySummary.ExpiresUtc` projected in `DashboardSnapshotService`; `StatusBadge` maps the new states). Server build clean; targeted tests 32/32 (parser abs/relative/invalid + end-to-end past-expiry rejection via the live verifier) + dashboard 28/28. Docs: `docs/Authentication.md` (verification flow expiry step, CLI table + examples, dashboard badge). SEC-10 was already `Done` (core); this closes the tracked polish. |
| 2026-07-09 | P1 Wave 3 (event-channel decoupling). GWC-04 → `Done`. `WorkerClient`'s read loop awaited `EnqueueWorkerEventAsync` inline, which blocks in the bounded event channel's timed `WriteAsync` (≤ `EventChannelFullModeTimeout`, 5 s) when the channel is full with a slow/absent `StreamEvents` consumer — stalling any `WorkerCommandReply`/heartbeat queued behind an event frame, so an in-flight `InvokeAsync` could hit `CommandTimeout` despite a timely worker reply. Fix mirrors the existing outbound `WriteLoopAsync`: an unbounded event **staging** channel + a dedicated `EventWriteLoopAsync`; `DispatchEnvelope` is now fully synchronous and the `WorkerEvent` branch hands off with a non-blocking `TryWrite`, so the read loop never awaits event enqueue. The event write loop owns the timed write + the sustained-overflow `ProtocolViolation` fault (unchanged contract); registered in `WaitForBackgroundTasks`, completed on close/fault/dispose. Server build clean (0 warnings); non-pipe event tests 37/38 (the 1 is the known parallel-load `EventStreamServiceTests…TracksAggregateQueueDepth` timing flake — passes in isolation). New pipe-harness test (reply after events with a full consumer-less channel dispatches without `CommandTimeout`) verified on windev. Docs: GatewayProcessDesign read/write/event-loop section. Commit `32d6b48`. **Wave 3 complete.** | | 2026-07-09 | P1 Wave 3 (event-channel decoupling). GWC-04 → `Done`. `WorkerClient`'s read loop awaited `EnqueueWorkerEventAsync` inline, which blocks in the bounded event channel's timed `WriteAsync` (≤ `EventChannelFullModeTimeout`, 5 s) when the channel is full with a slow/absent `StreamEvents` consumer — stalling any `WorkerCommandReply`/heartbeat queued behind an event frame, so an in-flight `InvokeAsync` could hit `CommandTimeout` despite a timely worker reply. Fix mirrors the existing outbound `WriteLoopAsync`: an unbounded event **staging** channel + a dedicated `EventWriteLoopAsync`; `DispatchEnvelope` is now fully synchronous and the `WorkerEvent` branch hands off with a non-blocking `TryWrite`, so the read loop never awaits event enqueue. The event write loop owns the timed write + the sustained-overflow `ProtocolViolation` fault (unchanged contract); registered in `WaitForBackgroundTasks`, completed on close/fault/dispose. Server build clean (0 warnings); non-pipe event tests 37/38 (the 1 is the known parallel-load `EventStreamServiceTests…TracksAggregateQueueDepth` timing flake — passes in isolation). New pipe-harness test (reply after events with a full consumer-less channel dispatches without `CommandTimeout`) verified on windev. Docs: GatewayProcessDesign read/write/event-loop section. Commit `32d6b48`. **Wave 3 complete.** |
| 2026-07-09 | P1 Wave 3 (size/backpressure topology + write ordering). IPC-02/03/04 + WRK-04/07 → `Done`. **Size negotiation (IPC-02):** added `GatewayHello.max_frame_bytes` (regen `Generated/` + `clients/proto` descriptor refresh with pinned protoc 34.1); the gateway sends its negotiated worker-frame max and the worker adopts it (`WorkerFrameProtocolOptions.AdoptNegotiatedMaxMessageBytes`, 0 = keep default, >256 MiB rejected) instead of a hard-coded default. **Headroom (IPC-03):** the pipe frame max now sits `EnvelopeOverheadReserveBytes` (64 KiB) above the public gRPC cap (default `Worker.MaxMessageBytes` 16 MiB→16 MiB+64 KiB), cross-validated at startup; `WorkerClient` pre-checks command envelope size and fails only the offending correlation (`ResourceExhausted`) instead of `SetFaulted`ing the session. **Drain bound (IPC-04):** gateway request validator rejects `DrainEvents max_events` above 10 000; the worker caps each reply at `MaxDrainEventsPerReply` (10 000) and treats `max_events = 0` as that cap, not "drain all". **Sequence (WRK-04):** `WorkerFrameWriter` stamps the envelope `Sequence` at the point of writing under the write lock, so wire order and stamped sequence always agree under concurrent producers. **Priority (WRK-07):** the worker writer is now a cooperative priority scheduler — control frames (reply/fault/heartbeat/shutdown-ack) drain ahead of event frames; per-frame validation/size rejections fail only that frame, a stream failure fails all queued. Docs same-change (GatewayConfiguration, WorkerFrameProtocol, gateway.md). **Verified:** macOS NonWindows build clean + validator/grpc tests green; **windev** x86 worker builds clean, `Worker.Tests` 352 passed / 0 failed / 11 skipped (incl. new monotonic-sequence, control-before-event priority, negotiated-max, drain-bound tests), gateway `Tests` 799 passed / 3 failed — all 3 pre-existing windev-environmental (SelfSigned SAN + 2 `EventStreamServiceTests` timing, both pass in isolation). Commits `c8b3a22` (gateway half), `ebe6aea` (worker half), `309296f` (descriptor + default-expectation refresh). GWC-04 (event-channel decoupling) is the remaining Wave 3 item. | | 2026-07-09 | P1 Wave 3 (size/backpressure topology + write ordering). IPC-02/03/04 + WRK-04/07 → `Done`. **Size negotiation (IPC-02):** added `GatewayHello.max_frame_bytes` (regen `Generated/` + `clients/proto` descriptor refresh with pinned protoc 34.1); the gateway sends its negotiated worker-frame max and the worker adopts it (`WorkerFrameProtocolOptions.AdoptNegotiatedMaxMessageBytes`, 0 = keep default, >256 MiB rejected) instead of a hard-coded default. **Headroom (IPC-03):** the pipe frame max now sits `EnvelopeOverheadReserveBytes` (64 KiB) above the public gRPC cap (default `Worker.MaxMessageBytes` 16 MiB→16 MiB+64 KiB), cross-validated at startup; `WorkerClient` pre-checks command envelope size and fails only the offending correlation (`ResourceExhausted`) instead of `SetFaulted`ing the session. **Drain bound (IPC-04):** gateway request validator rejects `DrainEvents max_events` above 10 000; the worker caps each reply at `MaxDrainEventsPerReply` (10 000) and treats `max_events = 0` as that cap, not "drain all". **Sequence (WRK-04):** `WorkerFrameWriter` stamps the envelope `Sequence` at the point of writing under the write lock, so wire order and stamped sequence always agree under concurrent producers. **Priority (WRK-07):** the worker writer is now a cooperative priority scheduler — control frames (reply/fault/heartbeat/shutdown-ack) drain ahead of event frames; per-frame validation/size rejections fail only that frame, a stream failure fails all queued. Docs same-change (GatewayConfiguration, WorkerFrameProtocol, gateway.md). **Verified:** macOS NonWindows build clean + validator/grpc tests green; **windev** x86 worker builds clean, `Worker.Tests` 352 passed / 0 failed / 11 skipped (incl. new monotonic-sequence, control-before-event priority, negotiated-max, drain-bound tests), gateway `Tests` 799 passed / 3 failed — all 3 pre-existing windev-environmental (SelfSigned SAN + 2 `EventStreamServiceTests` timing, both pass in isolation). Commits `c8b3a22` (gateway half), `ebe6aea` (worker half), `309296f` (descriptor + default-expectation refresh). GWC-04 (event-channel decoupling) is the remaining Wave 3 item. |
| 2026-07-09 | P1 S-misc (dashboard/observability hardening). SEC-02/12/20 → `Done`. SEC-02: `DashboardAuthorizationHandler` restricts the loopback + `Authentication:Mode=Disabled` bypasses to read-only (they satisfy a Viewer-bearing requirement but never `AdminOnly`), closing the policy-layer gap where anonymous localhost was authorized for Admin surfaces. SEC-12: `DashboardSessionAdminService` now emits canonical `AuditEvent`s (`dashboard-close-session`/`dashboard-kill-worker`, category `SessionAdmin`) through `IAuditWriter` on Success/Failure/Denied, so Close/Kill land durable audit rows. SEC-20: dropped the unbounded `session_id` tag from the exported `mxgateway.heartbeats.failed` counter. Docs updated same-change (CLAUDE.md, GatewayDashboardDesign.md, Metrics.md). Server build clean (0 warnings); targeted classes 30/30 pass; broader Dashboard+Security+GatewayApplication+Metrics sweep 295/295 pass. | | 2026-07-09 | P1 S-misc (dashboard/observability hardening). SEC-02/12/20 → `Done`. SEC-02: `DashboardAuthorizationHandler` restricts the loopback + `Authentication:Mode=Disabled` bypasses to read-only (they satisfy a Viewer-bearing requirement but never `AdminOnly`), closing the policy-layer gap where anonymous localhost was authorized for Admin surfaces. SEC-12: `DashboardSessionAdminService` now emits canonical `AuditEvent`s (`dashboard-close-session`/`dashboard-kill-worker`, category `SessionAdmin`) through `IAuditWriter` on Success/Failure/Denied, so Close/Kill land durable audit rows. SEC-20: dropped the unbounded `session_id` tag from the exported `mxgateway.heartbeats.failed` counter. Docs updated same-change (CLAUDE.md, GatewayDashboardDesign.md, Metrics.md). Server build clean (0 warnings); targeted classes 30/30 pass; broader Dashboard+Security+GatewayApplication+Metrics sweep 295/295 pass. |
+7 -3
View File
@@ -73,7 +73,7 @@ The pepper is intentionally not stored alongside the hash: an attacker who exfil
1. Parse the `Authorization` header into a `ParsedApiKey`. 1. Parse the `Authorization` header into a `ParsedApiKey`.
2. Look up the `ApiKeyRecord` by `KeyId` through `IApiKeyStore.FindByKeyIdAsync`. 2. Look up the `ApiKeyRecord` by `KeyId` through `IApiKeyStore.FindByKeyIdAsync`.
3. Reject revoked records (`RevokedUtc is not null`). 3. Reject revoked records (`RevokedUtc is not null`) and expired records (`ExpiresUtc` in the past). Expiry is opt-in — keys created without an expiry never expire; an expired key fails opaquely, indistinguishable to the client from any other auth failure.
4. Hash the presented secret with the configured pepper. 4. Hash the presented secret with the configured pepper.
5. Compare hashes with `CryptographicOperations.FixedTimeEquals` to avoid timing oracles. 5. Compare hashes with `CryptographicOperations.FixedTimeEquals` to avoid timing oracles.
6. Record a `LastUsedUtc` timestamp via `MarkKeyUsedAsync` and return an `ApiKeyIdentity`. 6. Record a `LastUsedUtc` timestamp via `MarkKeyUsedAsync` and return an `ApiKeyIdentity`.
@@ -193,6 +193,8 @@ public static ApiKeyRecord Read(SqliteDataReader reader)
Because `RotateAsync` clears `revoked_utc`, rotating a previously revoked key reactivates it. The dashboard API Keys page therefore offers the Rotate (and Revoke) actions only for keys whose status is `Active`; revoked keys instead show a Delete action that calls `DeleteAsync`, so an operator can permanently remove a revoked row without ever risking un-revocation as a side effect of a rotation. Because `RotateAsync` clears `revoked_utc`, rotating a previously revoked key reactivates it. The dashboard API Keys page therefore offers the Rotate (and Revoke) actions only for keys whose status is `Active`; revoked keys instead show a Delete action that calls `DeleteAsync`, so an operator can permanently remove a revoked row without ever risking un-revocation as a side effect of a rotation.
The dashboard API Keys page also surfaces expiry: each row shows an `Expires` column (`Never` when unset) and a status badge that reads `Expired` (past expiry, red), `Expiring` (within seven days, amber), `Revoked`, or `Active`. This is display-only staleness surfacing; expiry is set at creation time via the `apikey create-key --expires` CLI, not from the dashboard.
### Audit trail ### Audit trail
`SqliteApiKeyAuditStore` (`IApiKeyAuditStore`) appends `ApiKeyAuditEntry` values to the `api_key_audit` table and stamps each row with a UTC timestamp inside the store rather than trusting the caller. `ListRecentAsync` returns the most recent rows ordered by `audit_id` descending and projects them into `ApiKeyAuditRecord`. Rows are kept even after the referenced key is revoked because the audit history is the durable record of administrative action; the `key_id` column is nullable to accommodate non-key-scoped events such as `init-db`. `SqliteApiKeyAuditStore` (`IApiKeyAuditStore`) appends `ApiKeyAuditEntry` values to the `api_key_audit` table and stamps each row with a UTC timestamp inside the store rather than trusting the caller. `ListRecentAsync` returns the most recent rows ordered by `audit_id` descending and projects them into `ApiKeyAuditRecord`. Rows are kept even after the referenced key is revoked because the audit history is the durable record of administrative action; the `key_id` column is nullable to accommodate non-key-scoped events such as `init-db`.
@@ -226,8 +228,8 @@ The supported subcommands match `ApiKeyAdminCommandKind` exactly:
| Subcommand | Required options | Behaviour | | Subcommand | Required options | Behaviour |
|------------|------------------|-----------| |------------|------------------|-----------|
| `init-db` | none | Runs the migrator and records an audit entry. | | `init-db` | none | Runs the migrator and records an audit entry. |
| `create-key` | `--key-id`, `--display-name` | Generates a new secret, stores its peppered hash and optional constraints, and prints the assembled `mxgw_<keyId>_<secret>` token. | | `create-key` | `--key-id`, `--display-name` | Generates a new secret, stores its peppered hash and optional constraints, and prints the assembled `mxgw_<keyId>_<secret>` token. Optional `--expires` sets an expiry (absolute ISO-8601 UTC, or a relative `<N>d`/`<N>h` from now); omit it for a non-expiring key. |
| `list-keys` | none | Lists every stored key with its scopes, constraints, and revocation state. | | `list-keys` | none | Lists every stored key with its scopes, constraints, revocation state, and expiry (`active`/`expired`/`revoked`). |
| `revoke-key` | `--key-id` | Sets `revoked_utc` if the key is currently active. | | `revoke-key` | `--key-id` | Sets `revoked_utc` if the key is currently active. |
| `rotate-key` | `--key-id` | Replaces the secret hash and prints the new token. | | `rotate-key` | `--key-id` | Replaces the secret hash and prints the new token. |
@@ -237,6 +239,8 @@ Examples:
mxgateway apikey init-db mxgateway apikey init-db
mxgateway apikey create-key --key-id ops.alice --display-name "Alice (ops)" --scopes read,write mxgateway apikey create-key --key-id ops.alice --display-name "Alice (ops)" --scopes read,write
mxgateway apikey create-key --key-id area1.reader --display-name "Area 1 reader" --scopes invoke:read,metadata:read --read-subtree "Area1/*" --browse-subtree "Area1/*" mxgateway apikey create-key --key-id area1.reader --display-name "Area 1 reader" --scopes invoke:read,metadata:read --read-subtree "Area1/*" --browse-subtree "Area1/*"
mxgateway apikey create-key --key-id ops.temp --display-name "Temp contractor" --scopes invoke:read --expires 90d
mxgateway apikey create-key --key-id ops.audit --display-name "Audit window" --scopes metadata:read --expires 2027-01-01T00:00:00Z
mxgateway apikey list-keys --json mxgateway apikey list-keys --json
mxgateway apikey revoke-key --key-id ops.alice mxgateway apikey revoke-key --key-id ops.alice
mxgateway apikey rotate-key --key-id ops.alice mxgateway apikey rotate-key --key-id ops.alice
@@ -164,6 +164,7 @@ else
<th scope="col">Constraints</th> <th scope="col">Constraints</th>
<th scope="col">Created</th> <th scope="col">Created</th>
<th scope="col">Last Used</th> <th scope="col">Last Used</th>
<th scope="col">Expires</th>
@if (CanManageApiKeys) @if (CanManageApiKeys)
{ {
<th scope="col">Actions</th> <th scope="col">Actions</th>
@@ -175,12 +176,13 @@ else
{ {
<tr> <tr>
<td><code>@key.KeyId</code></td> <td><code>@key.KeyId</code></td>
<td><StatusBadge Text="@(key.RevokedUtc is null ? "Active" : "Revoked")" /></td> <td><StatusBadge Text="@KeyStatus(key)" /></td>
<td>@DashboardDisplay.Text(key.DisplayName)</td> <td>@DashboardDisplay.Text(key.DisplayName)</td>
<td>@DashboardDisplay.Text(string.Join(", ", key.Scopes.Order(StringComparer.Ordinal)))</td> <td>@DashboardDisplay.Text(string.Join(", ", key.Scopes.Order(StringComparer.Ordinal)))</td>
<td>@DashboardDisplay.Text(ConstraintText(key.Constraints))</td> <td>@DashboardDisplay.Text(ConstraintText(key.Constraints))</td>
<td>@DashboardDisplay.DateTime(key.CreatedUtc)</td> <td>@DashboardDisplay.DateTime(key.CreatedUtc)</td>
<td>@DashboardDisplay.DateTime(key.LastUsedUtc)</td> <td>@DashboardDisplay.DateTime(key.LastUsedUtc)</td>
<td>@(key.ExpiresUtc is null ? "Never" : DashboardDisplay.DateTime(key.ExpiresUtc))</td>
@if (CanManageApiKeys) @if (CanManageApiKeys)
{ {
<td> <td>
@@ -468,6 +470,35 @@ else
} }
} }
// Window before an expiry within which a key is flagged as "Expiring" (warn) rather than "Active".
private static readonly TimeSpan ExpiringSoonWindow = TimeSpan.FromDays(7);
// Status vocabulary understood by StatusBadge: Revoked wins over expiry; a past expiry is Expired
// (bad), an expiry inside ExpiringSoonWindow is Expiring (warn), otherwise Active (SEC-10).
private static string KeyStatus(DashboardApiKeySummary key)
{
if (key.RevokedUtc is not null)
{
return "Revoked";
}
if (key.ExpiresUtc is { } expiresAt)
{
DateTimeOffset now = DateTimeOffset.UtcNow;
if (expiresAt <= now)
{
return "Expired";
}
if (expiresAt - now <= ExpiringSoonWindow)
{
return "Expiring";
}
}
return "Active";
}
private static string ConstraintText(ApiKeyConstraints constraints) private static string ConstraintText(ApiKeyConstraints constraints)
{ {
if (constraints.IsEmpty) if (constraints.IsEmpty)
@@ -9,8 +9,8 @@
{ {
"Ready" or "Healthy" or "Active" => StatusState.Ok, "Ready" or "Healthy" or "Active" => StatusState.Ok,
"Creating" or "StartingWorker" or "WaitingForPipe" or "InitializingWorker" or "Closing" "Creating" or "StartingWorker" or "WaitingForPipe" or "InitializingWorker" or "Closing"
or "Stale" or "Degraded" => StatusState.Warn, or "Stale" or "Degraded" or "Expiring" => StatusState.Warn,
"Faulted" or "Unavailable" => StatusState.Bad, "Faulted" or "Unavailable" or "Expired" => StatusState.Bad,
_ => StatusState.Idle, _ => StatusState.Idle,
}; };
} }
@@ -9,4 +9,5 @@ public sealed record DashboardApiKeySummary(
ApiKeyConstraints Constraints, ApiKeyConstraints Constraints,
DateTimeOffset CreatedUtc, DateTimeOffset CreatedUtc,
DateTimeOffset? LastUsedUtc, DateTimeOffset? LastUsedUtc,
DateTimeOffset? RevokedUtc); DateTimeOffset? RevokedUtc,
DateTimeOffset? ExpiresUtc = null);
@@ -273,7 +273,8 @@ public sealed class DashboardSnapshotService : IDashboardSnapshotService
Constraints: ApiKeyConstraintSerializer.Deserialize(key.ConstraintsJson), Constraints: ApiKeyConstraintSerializer.Deserialize(key.ConstraintsJson),
CreatedUtc: key.CreatedUtc, CreatedUtc: key.CreatedUtc,
LastUsedUtc: key.LastUsedUtc, LastUsedUtc: key.LastUsedUtc,
RevokedUtc: key.RevokedUtc)) RevokedUtc: key.RevokedUtc,
ExpiresUtc: key.ExpiresUtc))
.ToArray(); .ToArray();
Volatile.Write(ref _apiKeySummaries, summaries); Volatile.Write(ref _apiKeySummaries, summaries);
@@ -67,6 +67,7 @@ public sealed class ApiKeyAdminCliRunner(ApiKeyAdminCommands commands)
Required(command.DisplayName), Required(command.DisplayName),
command.Scopes, command.Scopes,
ApiKeyConstraintSerializer.Serialize(command.Constraints), ApiKeyConstraintSerializer.Serialize(command.Constraints),
command.ExpiresUtc,
remoteAddress: null, remoteAddress: null,
cancellationToken) cancellationToken)
.ConfigureAwait(false); .ConfigureAwait(false);
@@ -134,8 +135,16 @@ public sealed class ApiKeyAdminCliRunner(ApiKeyAdminCommands commands)
foreach (ApiKeyAdminListedKey key in result.Keys) foreach (ApiKeyAdminListedKey key in result.Keys)
{ {
string revoked = key.RevokedUtc is null ? "active" : "revoked"; string status = key.RevokedUtc is not null
await output.WriteLineAsync($"{key.KeyId}\t{key.DisplayName}\t{revoked}\t{string.Join(',', key.Scopes)}") ? "revoked"
: key.ExpiresUtc is { } expiresAt && expiresAt <= DateTimeOffset.UtcNow
? "expired"
: "active";
string expiry = key.ExpiresUtc is { } expires
? expires.ToUniversalTime().ToString("u", System.Globalization.CultureInfo.InvariantCulture)
: "-";
await output.WriteLineAsync(
$"{key.KeyId}\t{key.DisplayName}\t{status}\t{expiry}\t{string.Join(',', key.Scopes)}")
.ConfigureAwait(false); .ConfigureAwait(false);
} }
} }
@@ -150,7 +159,8 @@ public sealed class ApiKeyAdminCliRunner(ApiKeyAdminCommands commands)
Constraints: ApiKeyConstraintSerializer.Deserialize(key.ConstraintsJson), Constraints: ApiKeyConstraintSerializer.Deserialize(key.ConstraintsJson),
CreatedUtc: key.CreatedUtc, CreatedUtc: key.CreatedUtc,
LastUsedUtc: key.LastUsedUtc, LastUsedUtc: key.LastUsedUtc,
RevokedUtc: key.RevokedUtc); RevokedUtc: key.RevokedUtc,
ExpiresUtc: key.ExpiresUtc);
} }
private static string Required(string? value) private static string Required(string? value)
@@ -8,4 +8,5 @@ public sealed record ApiKeyAdminCommand(
string? KeyId, string? KeyId,
string? DisplayName, string? DisplayName,
IReadOnlySet<string> Scopes, IReadOnlySet<string> Scopes,
ApiKeyConstraints Constraints); ApiKeyConstraints Constraints,
DateTimeOffset? ExpiresUtc = null);
@@ -82,9 +82,11 @@ public static class ApiKeyAdminCommandLineParser
string? displayName = GetOption(options, "display-name"); string? displayName = GetOption(options, "display-name");
IReadOnlySet<string> scopes = ParseScopes(GetOption(options, "scopes")); IReadOnlySet<string> scopes = ParseScopes(GetOption(options, "scopes"));
ApiKeyConstraints constraints; ApiKeyConstraints constraints;
DateTimeOffset? expiresUtc;
try try
{ {
constraints = ParseConstraints(options); constraints = ParseConstraints(options);
expiresUtc = ParseExpiry(GetOption(options, "expires"));
} }
catch (FormatException exception) catch (FormatException exception)
{ {
@@ -111,7 +113,8 @@ public static class ApiKeyAdminCommandLineParser
KeyId: keyId, KeyId: keyId,
DisplayName: displayName, DisplayName: displayName,
Scopes: scopes, Scopes: scopes,
Constraints: constraints)); Constraints: constraints,
ExpiresUtc: expiresUtc));
} }
private static bool TryParseKind(string value, out ApiKeyAdminCommandKind kind) private static bool TryParseKind(string value, out ApiKeyAdminCommandKind kind)
@@ -233,6 +236,42 @@ public static class ApiKeyAdminCommandLineParser
ReadHistorizedOnly: HasFlag(options, "read-historized-only")); ReadHistorizedOnly: HasFlag(options, "read-historized-only"));
} }
// Parses the optional --expires value into an absolute UTC expiry (SEC-10). Accepts a relative
// "<N>d"/"<N>h" duration from now (operator-friendly) or an absolute ISO-8601 instant/date
// (assumed UTC). Null/blank means no expiry — expiry stays opt-in, preserving prior behaviour.
private static DateTimeOffset? ParseExpiry(string? value)
{
if (string.IsNullOrWhiteSpace(value))
{
return null;
}
string trimmed = value.Trim();
char unit = char.ToLowerInvariant(trimmed[^1]);
if (unit is 'd' or 'h'
&& int.TryParse(
trimmed[..^1],
System.Globalization.NumberStyles.None,
System.Globalization.CultureInfo.InvariantCulture,
out int amount))
{
TimeSpan offset = unit == 'd' ? TimeSpan.FromDays(amount) : TimeSpan.FromHours(amount);
return DateTimeOffset.UtcNow + offset;
}
if (DateTimeOffset.TryParse(
trimmed,
System.Globalization.CultureInfo.InvariantCulture,
System.Globalization.DateTimeStyles.AssumeUniversal | System.Globalization.DateTimeStyles.AdjustToUniversal,
out DateTimeOffset parsed))
{
return parsed;
}
throw new FormatException(
"--expires must be a relative duration ('<N>d' or '<N>h') or an absolute ISO-8601 UTC timestamp.");
}
private static int? ParseNullableInt(string? value) private static int? ParseNullableInt(string? value)
{ {
if (string.IsNullOrWhiteSpace(value)) if (string.IsNullOrWhiteSpace(value))
@@ -8,4 +8,5 @@ public sealed record ApiKeyAdminListedKey(
ApiKeyConstraints Constraints, ApiKeyConstraints Constraints,
DateTimeOffset CreatedUtc, DateTimeOffset CreatedUtc,
DateTimeOffset? LastUsedUtc, DateTimeOffset? LastUsedUtc,
DateTimeOffset? RevokedUtc); DateTimeOffset? RevokedUtc,
DateTimeOffset? ExpiresUtc = null);
@@ -52,6 +52,40 @@ public sealed class ApiKeyAdminCliRunnerTests : IDisposable
Assert.Contains(auditRecords, record => record.EventType == "create-key" && record.KeyId == "operator01"); Assert.Contains(auditRecords, record => record.EventType == "create-key" && record.KeyId == "operator01");
} }
/// <summary>
/// Verifies that a key created with an already-past <c>--expires</c> is rejected by the verifier
/// — the CLI expiry wiring reaches the store and the library enforces it end-to-end (SEC-10).
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task CreateKeyAsync_WithPastExpiry_KeyIsRejectedByVerifier()
{
await using ServiceProvider services = BuildServices(CreateTempDatabasePath());
ApiKeyAdminCliRunner runner = services.GetRequiredService<ApiKeyAdminCliRunner>();
StringWriter output = new();
await runner.RunAsync(
new ApiKeyAdminCommand(
Kind: ApiKeyAdminCommandKind.CreateKey,
Json: true,
SqlitePath: null,
Pepper: null,
KeyId: "operator01",
DisplayName: "Operator",
Scopes: new HashSet<string>(StringComparer.Ordinal) { "session:open" },
Constraints: ApiKeyConstraints.Empty,
ExpiresUtc: DateTimeOffset.UtcNow - TimeSpan.FromMinutes(1)),
output,
CancellationToken.None);
string apiKey = ReadApiKey(output.ToString());
IApiKeyVerifier verifier = services.GetRequiredService<IApiKeyVerifier>();
ApiKeyVerification verification = await verifier.VerifyAsync($"Bearer {apiKey}", CancellationToken.None);
Assert.False(verification.Succeeded);
}
/// <summary>Verifies that ListKeysAsync does not print the raw secret.</summary> /// <summary>Verifies that ListKeysAsync does not print the raw secret.</summary>
/// <returns>A task that represents the asynchronous operation.</returns> /// <returns>A task that represents the asynchronous operation.</returns>
[Fact] [Fact]
@@ -52,6 +52,61 @@ public sealed class ApiKeyAdminCommandLineParserTests
Assert.Contains("events:read", result.Command.Scopes); Assert.Contains("events:read", result.Command.Scopes);
} }
/// <summary>A create-key command without --expires leaves the key non-expiring (opt-in, SEC-10).</summary>
[Fact]
public void Parse_CreateKeyCommand_WithoutExpires_HasNoExpiry()
{
ApiKeyAdminParseResult result = ApiKeyAdminCommandLineParser.Parse(
["apikey", "create-key", "--key-id", "operator01", "--display-name", "Operator", "--scopes", "session:open"]);
Assert.NotNull(result.Command);
Assert.Null(result.Command.ExpiresUtc);
}
/// <summary>An absolute ISO-8601 --expires value is parsed to that exact UTC instant (SEC-10).</summary>
[Fact]
public void Parse_CreateKeyCommand_WithAbsoluteExpires_ParsesInstant()
{
ApiKeyAdminParseResult result = ApiKeyAdminCommandLineParser.Parse(
["apikey", "create-key", "--key-id", "operator01", "--display-name", "Operator", "--scopes", "session:open",
"--expires", "2030-01-02T03:04:05Z"]);
Assert.NotNull(result.Command);
Assert.Equal(
new DateTimeOffset(2030, 1, 2, 3, 4, 5, TimeSpan.Zero),
result.Command.ExpiresUtc);
}
/// <summary>A relative "&lt;N&gt;d" --expires value resolves to a future UTC instant (SEC-10).</summary>
[Fact]
public void Parse_CreateKeyCommand_WithRelativeExpires_ResolvesToFuture()
{
DateTimeOffset before = DateTimeOffset.UtcNow;
ApiKeyAdminParseResult result = ApiKeyAdminCommandLineParser.Parse(
["apikey", "create-key", "--key-id", "operator01", "--display-name", "Operator", "--scopes", "session:open",
"--expires", "30d"]);
Assert.NotNull(result.Command);
Assert.NotNull(result.Command.ExpiresUtc);
Assert.InRange(
result.Command.ExpiresUtc!.Value,
before + TimeSpan.FromDays(30) - TimeSpan.FromMinutes(1),
DateTimeOffset.UtcNow + TimeSpan.FromDays(30) + TimeSpan.FromMinutes(1));
}
/// <summary>An unparseable --expires value fails at parse time (SEC-10).</summary>
[Fact]
public void Parse_CreateKeyCommand_WithInvalidExpires_Fails()
{
ApiKeyAdminParseResult result = ApiKeyAdminCommandLineParser.Parse(
["apikey", "create-key", "--key-id", "operator01", "--display-name", "Operator", "--scopes", "session:open",
"--expires", "not-a-date"]);
Assert.Null(result.Command);
Assert.NotNull(result.Error);
Assert.Contains("--expires", result.Error, StringComparison.Ordinal);
}
/// <summary> /// <summary>
/// A create-key command with a non-canonical scope /// A create-key command with a non-canonical scope
/// string (e.g. CLAUDE.md's stale <c>invoke</c> instead of <c>invoke:read</c>) /// string (e.g. CLAUDE.md's stale <c>invoke</c> instead of <c>invoke:read</c>)