fix(audit): ScadaBridge C3 review — safe enum-parse (fallback) in SqliteAuditWriter.MapRow + AuditEventDtoMapper.FromDto (Task 2.5)
This commit is contained in:
@@ -800,9 +800,9 @@ public class SqliteAuditWriter : IAuditWriter, ISiteAuditQueue, IAsyncDisposable
|
|||||||
System.Globalization.CultureInfo.InvariantCulture,
|
System.Globalization.CultureInfo.InvariantCulture,
|
||||||
System.Globalization.DateTimeStyles.RoundtripKind),
|
System.Globalization.DateTimeStyles.RoundtripKind),
|
||||||
IngestedAtUtc: null,
|
IngestedAtUtc: null,
|
||||||
Channel: Enum.Parse<AuditChannel>(reader.GetString(2)),
|
Channel: AuditRowProjection.ParseEnum<AuditChannel>(reader.GetString(2), AuditChannel.ApiInbound),
|
||||||
Kind: Enum.Parse<AuditKind>(reader.GetString(3)),
|
Kind: AuditRowProjection.ParseEnum<AuditKind>(reader.GetString(3), AuditKind.InboundRequest),
|
||||||
Status: Enum.Parse<AuditStatus>(reader.GetString(11)),
|
Status: AuditRowProjection.ParseEnum<AuditStatus>(reader.GetString(11), AuditStatus.Submitted),
|
||||||
CorrelationId: reader.IsDBNull(4) ? null : Guid.Parse(reader.GetString(4)),
|
CorrelationId: reader.IsDBNull(4) ? null : Guid.Parse(reader.GetString(4)),
|
||||||
ExecutionId: reader.IsDBNull(21) ? null : Guid.Parse(reader.GetString(21)),
|
ExecutionId: reader.IsDBNull(21) ? null : Guid.Parse(reader.GetString(21)),
|
||||||
ParentExecutionId: reader.IsDBNull(22) ? null : Guid.Parse(reader.GetString(22)),
|
ParentExecutionId: reader.IsDBNull(22) ? null : Guid.Parse(reader.GetString(22)),
|
||||||
|
|||||||
@@ -173,7 +173,13 @@ public static class AuditRowProjection
|
|||||||
return evt with { DetailsJson = AuditDetailsCodec.Serialize(d) };
|
return evt with { DetailsJson = AuditDetailsCodec.Serialize(d) };
|
||||||
}
|
}
|
||||||
|
|
||||||
private static TEnum ParseEnum<TEnum>(string? value, TEnum fallback) where TEnum : struct, Enum
|
/// <summary>
|
||||||
|
/// Case-sensitive <see cref="Enum.TryParse{TEnum}"/> with a caller-supplied fallback.
|
||||||
|
/// Returns <paramref name="fallback"/> when <paramref name="value"/> is null, empty,
|
||||||
|
/// or does not match any declared member name — so callers never throw on an
|
||||||
|
/// unknown/renamed enum string (legacy or corrupt rows degrade gracefully).
|
||||||
|
/// </summary>
|
||||||
|
public static TEnum ParseEnum<TEnum>(string? value, TEnum fallback) where TEnum : struct, Enum
|
||||||
=> !string.IsNullOrEmpty(value) && Enum.TryParse<TEnum>(value, ignoreCase: false, out var parsed)
|
=> !string.IsNullOrEmpty(value) && Enum.TryParse<TEnum>(value, ignoreCase: false, out var parsed)
|
||||||
? parsed
|
? parsed
|
||||||
: fallback;
|
: fallback;
|
||||||
|
|||||||
@@ -105,9 +105,9 @@ public static class AuditEventDtoMapper
|
|||||||
EventId: Guid.Parse(dto.EventId),
|
EventId: Guid.Parse(dto.EventId),
|
||||||
OccurredAtUtc: DateTime.SpecifyKind(dto.OccurredAtUtc.ToDateTime(), DateTimeKind.Utc),
|
OccurredAtUtc: DateTime.SpecifyKind(dto.OccurredAtUtc.ToDateTime(), DateTimeKind.Utc),
|
||||||
IngestedAtUtc: null,
|
IngestedAtUtc: null,
|
||||||
Channel: Enum.Parse<AuditChannel>(dto.Channel),
|
Channel: AuditRowProjection.ParseEnum<AuditChannel>(dto.Channel, AuditChannel.ApiInbound),
|
||||||
Kind: Enum.Parse<AuditKind>(dto.Kind),
|
Kind: AuditRowProjection.ParseEnum<AuditKind>(dto.Kind, AuditKind.InboundRequest),
|
||||||
Status: Enum.Parse<AuditStatus>(dto.Status),
|
Status: AuditRowProjection.ParseEnum<AuditStatus>(dto.Status, AuditStatus.Submitted),
|
||||||
CorrelationId: NullIfEmpty(dto.CorrelationId) is { } cid ? Guid.Parse(cid) : null,
|
CorrelationId: NullIfEmpty(dto.CorrelationId) is { } cid ? Guid.Parse(cid) : null,
|
||||||
ExecutionId: NullIfEmpty(dto.ExecutionId) is { } eid ? Guid.Parse(eid) : null,
|
ExecutionId: NullIfEmpty(dto.ExecutionId) is { } eid ? Guid.Parse(eid) : null,
|
||||||
ParentExecutionId: NullIfEmpty(dto.ParentExecutionId) is { } pid ? Guid.Parse(pid) : null,
|
ParentExecutionId: NullIfEmpty(dto.ParentExecutionId) is { } pid ? Guid.Parse(pid) : null,
|
||||||
|
|||||||
@@ -457,4 +457,47 @@ public class SqliteAuditWriterWriteTests
|
|||||||
var row = Assert.Single(rows);
|
var row = Assert.Single(rows);
|
||||||
Assert.Null(row.SourceNode);
|
Assert.Null(row.SourceNode);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ----- C3 hardening: safe enum-parse in MapRow ----- //
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// C3 hardening (Task 2.5): a row whose Channel/Kind/Status columns hold
|
||||||
|
/// an unknown/renamed enum string must NOT fault the read path; it degrades
|
||||||
|
/// gracefully to the same fallbacks used by <c>AuditRowProjection.Decompose</c>
|
||||||
|
/// (ApiInbound / InboundRequest / Submitted). The read is exercised via the
|
||||||
|
/// public <see cref="SqliteAuditWriter.ReadPendingAsync"/> surface which calls
|
||||||
|
/// the private <c>MapRow</c>.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public async Task ReadPendingAsync_UnknownEnumStrings_DoNotThrow_YieldFallbackValues()
|
||||||
|
{
|
||||||
|
var (writer, dataSource) = CreateWriter(
|
||||||
|
nameof(ReadPendingAsync_UnknownEnumStrings_DoNotThrow_YieldFallbackValues));
|
||||||
|
await using var _ = writer;
|
||||||
|
|
||||||
|
var evt = NewEvent();
|
||||||
|
await writer.WriteAsync(evt);
|
||||||
|
|
||||||
|
// Tamper: overwrite the three enum columns with unknown strings that are
|
||||||
|
// not declared AuditChannel/AuditKind/AuditStatus member names.
|
||||||
|
using (var conn = OpenVerifierConnection(dataSource))
|
||||||
|
using (var cmd = conn.CreateCommand())
|
||||||
|
{
|
||||||
|
cmd.CommandText =
|
||||||
|
"UPDATE AuditLog SET Channel = 'ObsoleteChannelV0', " +
|
||||||
|
"Kind = 'LegacyKindName', Status = 'RenamedStatus99' " +
|
||||||
|
"WHERE EventId = $id;";
|
||||||
|
cmd.Parameters.AddWithValue("$id", evt.EventId.ToString());
|
||||||
|
cmd.ExecuteNonQuery();
|
||||||
|
}
|
||||||
|
|
||||||
|
// Must not throw (previously would throw ArgumentException from Enum.Parse).
|
||||||
|
var rows = await writer.ReadPendingAsync(limit: 10);
|
||||||
|
|
||||||
|
var row = Assert.Single(rows);
|
||||||
|
var typedRow = row.AsRow();
|
||||||
|
Assert.Equal(AuditChannel.ApiInbound, typedRow.Channel);
|
||||||
|
Assert.Equal(AuditKind.InboundRequest, typedRow.Kind);
|
||||||
|
Assert.Equal(AuditStatus.Submitted, typedRow.Status);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -268,4 +268,30 @@ public class AuditEventDtoMapperTests
|
|||||||
// …and FromDto rehydrates empty → null.
|
// …and FromDto rehydrates empty → null.
|
||||||
Assert.Null(roundTripped.SourceNode);
|
Assert.Null(roundTripped.SourceNode);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// C3 hardening (Task 2.5): a DTO that carries an unknown/renamed enum string
|
||||||
|
/// for Channel, Kind, or Status must NOT throw on <see cref="AuditEventDtoMapper.FromDto"/>;
|
||||||
|
/// it degrades gracefully to the same fallbacks used by <c>AuditRowProjection.Decompose</c>
|
||||||
|
/// (ApiInbound / InboundRequest / Submitted).
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public void FromDto_UnknownEnumStrings_DoNotThrow_YieldFallbackValues()
|
||||||
|
{
|
||||||
|
var dto = new AuditEventDto
|
||||||
|
{
|
||||||
|
EventId = Guid.NewGuid().ToString(),
|
||||||
|
OccurredAtUtc = Timestamp.FromDateTime(DateTime.UtcNow),
|
||||||
|
Channel = "ObsoleteChannelV0", // unknown — not a declared AuditChannel member
|
||||||
|
Kind = "LegacyKindName", // unknown — not a declared AuditKind member
|
||||||
|
Status = "RenamedStatus99", // unknown — not a declared AuditStatus member
|
||||||
|
};
|
||||||
|
|
||||||
|
// Must not throw (previously would throw ArgumentException from Enum.Parse).
|
||||||
|
var row = AuditEventDtoMapper.FromDto(dto).AsRow();
|
||||||
|
|
||||||
|
Assert.Equal(AuditChannel.ApiInbound, row.Channel);
|
||||||
|
Assert.Equal(AuditKind.InboundRequest, row.Kind);
|
||||||
|
Assert.Equal(AuditStatus.Submitted, row.Status);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user