fix(deployment): audit-write fault can no longer flip a committed Success deployment to Failed
Route all post-terminal-status lifecycle audit writes (Deploy/Disable/Enable/Delete/DeleteOrphaned) through a swallow-and-warn TryLogAuditAsync helper so a failed best-effort audit write can neither reach the outer catch (flipping Success to Failed) nor propagate to the caller. Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
This commit is contained in:
@@ -326,8 +326,11 @@ public class DeploymentService
|
|||||||
forceEnabledState: true, cancellationToken);
|
forceEnabledState: true, cancellationToken);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Audit log
|
// Audit log. Best-effort and post-terminal: the deployment record's
|
||||||
await _auditService.LogAsync(user, "Deploy", "Instance", instanceId.ToString(),
|
// status is already committed above, so a failed audit write must NOT
|
||||||
|
// fall through to the outer catch and flip a committed Success back to
|
||||||
|
// Failed (audit is best-effort — the action's own result is authoritative).
|
||||||
|
await TryLogAuditAsync(user, "Deploy", "Instance", instanceId.ToString(),
|
||||||
instance.UniqueName, new { DeploymentId = deploymentId, Status = record.Status.ToString() },
|
instance.UniqueName, new { DeploymentId = deploymentId, Status = record.Status.ToString() },
|
||||||
cancellationToken);
|
cancellationToken);
|
||||||
|
|
||||||
@@ -458,7 +461,7 @@ public class DeploymentService
|
|||||||
await _repository.SaveChangesAsync(cancellationToken);
|
await _repository.SaveChangesAsync(cancellationToken);
|
||||||
}
|
}
|
||||||
|
|
||||||
await _auditService.LogAsync(user, "Disable", "Instance", instanceId.ToString(),
|
await TryLogAuditAsync(user, "Disable", "Instance", instanceId.ToString(),
|
||||||
instance.UniqueName, new { CommandId = commandId, response.Success },
|
instance.UniqueName, new { CommandId = commandId, response.Success },
|
||||||
cancellationToken);
|
cancellationToken);
|
||||||
|
|
||||||
@@ -522,7 +525,7 @@ public class DeploymentService
|
|||||||
await _repository.SaveChangesAsync(cancellationToken);
|
await _repository.SaveChangesAsync(cancellationToken);
|
||||||
}
|
}
|
||||||
|
|
||||||
await _auditService.LogAsync(user, "Enable", "Instance", instanceId.ToString(),
|
await TryLogAuditAsync(user, "Enable", "Instance", instanceId.ToString(),
|
||||||
instance.UniqueName, new { CommandId = commandId, response.Success },
|
instance.UniqueName, new { CommandId = commandId, response.Success },
|
||||||
cancellationToken);
|
cancellationToken);
|
||||||
|
|
||||||
@@ -609,7 +612,7 @@ public class DeploymentService
|
|||||||
"removed -- the central record is now orphaned and must be reconciled manually",
|
"removed -- the central record is now orphaned and must be reconciled manually",
|
||||||
instance.UniqueName);
|
instance.UniqueName);
|
||||||
|
|
||||||
await _auditService.LogAsync(user, "DeleteOrphaned", "Instance", instanceId.ToString(),
|
await TryLogAuditAsync(user, "DeleteOrphaned", "Instance", instanceId.ToString(),
|
||||||
instance.UniqueName, new { CommandId = commandId, Error = ex.Message },
|
instance.UniqueName, new { CommandId = commandId, Error = ex.Message },
|
||||||
CancellationToken.None);
|
CancellationToken.None);
|
||||||
|
|
||||||
@@ -619,7 +622,7 @@ public class DeploymentService
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
await _auditService.LogAsync(user, "Delete", "Instance", instanceId.ToString(),
|
await TryLogAuditAsync(user, "Delete", "Instance", instanceId.ToString(),
|
||||||
instance.UniqueName, new { CommandId = commandId, response.Success },
|
instance.UniqueName, new { CommandId = commandId, response.Success },
|
||||||
cancellationToken);
|
cancellationToken);
|
||||||
|
|
||||||
@@ -1035,6 +1038,37 @@ public class DeploymentService
|
|||||||
/// <param name="instanceUniqueName">The instance unique name used as the audit target name.</param>
|
/// <param name="instanceUniqueName">The instance unique name used as the audit target name.</param>
|
||||||
/// <param name="commandId">The lifecycle command's correlation id, so the audit entry can be matched to logs.</param>
|
/// <param name="commandId">The lifecycle command's correlation id, so the audit entry can be matched to logs.</param>
|
||||||
/// <param name="timeoutException">The captured <see cref="TimeoutException"/> or <see cref="OperationCanceledException"/>.</param>
|
/// <param name="timeoutException">The captured <see cref="TimeoutException"/> or <see cref="OperationCanceledException"/>.</param>
|
||||||
|
/// <summary>
|
||||||
|
/// Best-effort audit write for a post-terminal-status action. By the time this
|
||||||
|
/// runs, the deployment/lifecycle action has already committed its authoritative
|
||||||
|
/// result; a failed audit write must be logged for operator reconciliation but
|
||||||
|
/// must NEVER surface to the caller or flip the committed status — "audit-write
|
||||||
|
/// failure NEVER aborts the user-facing action" (Centralized Audit Log design).
|
||||||
|
/// Any exception is swallowed and logged at Warning.
|
||||||
|
/// </summary>
|
||||||
|
private async Task TryLogAuditAsync(
|
||||||
|
string user,
|
||||||
|
string action,
|
||||||
|
string entityType,
|
||||||
|
string entityId,
|
||||||
|
string entityName,
|
||||||
|
object? afterState,
|
||||||
|
CancellationToken cancellationToken)
|
||||||
|
{
|
||||||
|
try
|
||||||
|
{
|
||||||
|
await _auditService.LogAsync(
|
||||||
|
user, action, entityType, entityId, entityName, afterState, cancellationToken);
|
||||||
|
}
|
||||||
|
catch (Exception auditEx)
|
||||||
|
{
|
||||||
|
_logger.LogWarning(auditEx,
|
||||||
|
"Failed to write {Action} audit entry for {EntityType} {EntityName} -- the action " +
|
||||||
|
"itself succeeded and its committed status is authoritative; audit is best-effort",
|
||||||
|
action, entityType, entityName);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private async Task TryLogLifecycleTimeoutAsync(
|
private async Task TryLogLifecycleTimeoutAsync(
|
||||||
string user,
|
string user,
|
||||||
string action,
|
string action,
|
||||||
|
|||||||
@@ -878,6 +878,36 @@ public class DeploymentServiceTests : TestKit
|
|||||||
Arg.Any<object>(), Arg.Any<CancellationToken>());
|
Arg.Any<object>(), Arg.Any<CancellationToken>());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task DeployInstanceAsync_DeployAuditWriteThrows_KeepsCommittedSuccess()
|
||||||
|
{
|
||||||
|
// #05-T17: the post-success "Deploy" audit write is best-effort. If it
|
||||||
|
// throws, the already-committed Success record must NOT be flipped to
|
||||||
|
// Failed by the outer catch — "audit-write failure NEVER aborts the
|
||||||
|
// user-facing action".
|
||||||
|
var instance = new Instance("AuditFaultInst") { Id = 52, SiteId = 1, State = InstanceState.NotDeployed };
|
||||||
|
_repo.GetInstanceByIdAsync(52, Arg.Any<CancellationToken>()).Returns(instance);
|
||||||
|
SetupValidPipeline(52, "AuditFaultInst", "sha256:target");
|
||||||
|
_repo.GetCurrentDeploymentStatusAsync(52, Arg.Any<CancellationToken>())
|
||||||
|
.Returns((DeploymentRecord?)null);
|
||||||
|
|
||||||
|
// The Deploy audit LogAsync faults.
|
||||||
|
_audit.When(a => a.LogAsync(
|
||||||
|
Arg.Any<string>(), "Deploy", Arg.Any<string>(), Arg.Any<string>(),
|
||||||
|
Arg.Any<string>(), Arg.Any<object>(), Arg.Any<CancellationToken>()))
|
||||||
|
.Do(_ => throw new InvalidOperationException("audit pipeline down"));
|
||||||
|
|
||||||
|
var counters = new ReconcileProbeCounters();
|
||||||
|
var commActor = Sys.ActorOf(Props.Create(() =>
|
||||||
|
new ReconcileProbeActor(counters, siteHash: "sha256:target", failQuery: false)));
|
||||||
|
var service = CreateServiceWithCommActor(commActor);
|
||||||
|
|
||||||
|
var result = await service.DeployInstanceAsync(52, "admin");
|
||||||
|
|
||||||
|
Assert.True(result.IsSuccess);
|
||||||
|
Assert.Equal(DeploymentStatus.Success, result.Value.Status);
|
||||||
|
}
|
||||||
|
|
||||||
// ── Telemetry follow-on: scadabridge.deployments.applied on deploy success ──
|
// ── Telemetry follow-on: scadabridge.deployments.applied on deploy success ──
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
Reference in New Issue
Block a user