docs(audit): implementation plan — full request/response capture for inbound API audit rows
Plan companion to the 2026-05-23 design doc. Seven tasks (#0 prep, #1-3 implementation TDD, #4-5 doc updates, #6 final sweep). Tracks via .tasks.json for resumability.
This commit is contained in:
Joseph Doherty
745
docs/plans/2026-05-23-inbound-api-full-response-audit.md
Normal file
745
docs/plans/2026-05-23-inbound-api-full-response-audit.md
Normal file
@@ -0,0 +1,745 @@
|
||||
# Inbound API: Full Request/Response Audit Capture — Implementation Plan
|
||||
|
||||
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development to execute this plan task-by-task (fresh implementer per task + spec review + code-quality review).
|
||||
|
||||
**Goal:** Inbound API audit rows (`AuditChannel.ApiInbound`) capture the FULL request and response body verbatim up to a configurable 1 MB per-body ceiling, instead of the global 8 KB / 64 KB caps. Other channels are untouched.
|
||||
|
||||
**Architecture:** Two changes. (1) `AuditLogOptions` gains an `InboundMaxBytes` knob; `DefaultAuditPayloadFilter` branches on `Channel == ApiInbound` to use it. (2) `AuditWriteMiddleware` finally implements the M5-deferred response-body capture — wraps `HttpContext.Response.Body` with a buffering `MemoryStream` swap, reads it after the pipeline runs, restores and flushes the original body. The redaction stages (headers, body regexes, SQL params) keep their existing semantics; only the truncation cap changes for ApiInbound rows. Validated design: `docs/plans/2026-05-23-inbound-api-full-response-audit-design.md`.
|
||||
|
||||
**Tech Stack:** .NET 10, xUnit, ASP.NET Core Minimal API, `Microsoft.Extensions.Options.IOptionsMonitor`.
|
||||
|
||||
**Ground rules (every task):** create + work on branch `feature/inbound-api-full-response-audit` — never commit to `main`. TDD: failing test first, then minimal implementation, then verify. Edit in place; never edit `infra/*`, `alog.md`, or `docker/*` unless a task names them (none here). Stage with explicit `git add <path>` — never `git add .` / `commit -am`. Solution stays green: `dotnet build ScadaLink.slnx` 0 warnings (`TreatWarningsAsErrors` on). Do not push.
|
||||
|
||||
---
|
||||
|
||||
## Task 0: Prep — branch, baseline build
|
||||
|
||||
**Files:** none.
|
||||
|
||||
**Steps:**
|
||||
1. `git status --short` — confirm you are starting from the `main` revision that already contains commit `0670864` (`docs(audit): design — full request/response capture for inbound API rows`).
|
||||
2. `git checkout -b feature/inbound-api-full-response-audit`.
|
||||
3. `git branch --show-current` — expect `feature/inbound-api-full-response-audit`.
|
||||
4. `dotnet build ScadaLink.slnx` from repo root — expect 0 warnings, 0 errors.
|
||||
|
||||
**Acceptance:** on the feature branch; solution builds clean.
|
||||
|
||||
**Commit:** none (no changes yet).
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Add `InboundMaxBytes` to `AuditLogOptions` (TDD)
|
||||
|
||||
**What:** New `int InboundMaxBytes` property on `AuditLogOptions` with default 1 048 576 bytes, validated to `[8192, 16777216]`.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ScadaLink.AuditLog/Configuration/AuditLogOptions.cs` — add property + XML doc.
|
||||
- Modify: `src/ScadaLink.AuditLog/Configuration/AuditLogOptionsValidator.cs` — add min/max constants + validation branch.
|
||||
- Modify: `tests/ScadaLink.AuditLog.Tests/Configuration/AuditLogOptionsBindingTests.cs` — extend the binding test to assert the new field round-trips from JSON.
|
||||
- Test (new): `tests/ScadaLink.AuditLog.Tests/Configuration/AuditLogOptionsValidatorTests.cs` — if this file does not exist, create it with the four cases below; if a validator-tests file already exists (search for it under `tests/ScadaLink.AuditLog.Tests/Configuration/`), extend it instead.
|
||||
|
||||
**Step 1: Write the failing tests**
|
||||
|
||||
Add to a validator-tests file (create if missing — namespace `ScadaLink.AuditLog.Tests.Configuration`):
|
||||
|
||||
```csharp
|
||||
public class AuditLogOptionsValidatorTests
|
||||
{
|
||||
[Fact]
|
||||
public void Validate_InboundMaxBytes_DefaultOptions_IsOneMebibyte()
|
||||
{
|
||||
// The doc'd default per docs/plans/2026-05-23-inbound-api-full-response-audit-design.md
|
||||
// is 1 048 576 bytes (1 MiB). Pin it so a config drift is a test failure,
|
||||
// not a silent operational surprise.
|
||||
var opts = new AuditLogOptions();
|
||||
Assert.Equal(1_048_576, opts.InboundMaxBytes);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData(8_192)] // documented min
|
||||
[InlineData(1_048_576)] // default
|
||||
[InlineData(16_777_216)] // documented max
|
||||
public void Validate_InboundMaxBytes_InRange_Passes(int value)
|
||||
{
|
||||
var validator = new AuditLogOptionsValidator();
|
||||
var opts = new AuditLogOptions { InboundMaxBytes = value };
|
||||
Assert.True(validator.Validate(null, opts).Succeeded);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData(0)]
|
||||
[InlineData(8_191)]
|
||||
[InlineData(16_777_217)]
|
||||
[InlineData(int.MaxValue)]
|
||||
public void Validate_InboundMaxBytes_OutOfRange_Fails(int value)
|
||||
{
|
||||
var validator = new AuditLogOptionsValidator();
|
||||
var opts = new AuditLogOptions { InboundMaxBytes = value };
|
||||
var result = validator.Validate(null, opts);
|
||||
Assert.False(result.Succeeded);
|
||||
Assert.Contains(
|
||||
result.Failures!,
|
||||
f => f.Contains(nameof(AuditLogOptions.InboundMaxBytes), StringComparison.Ordinal));
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Extend `AuditLogOptionsBindingTests.AuditLog_Section_Binds_AllFields` — add `"InboundMaxBytes": 524288` to the JSON literal and a matching `Assert.Equal(524_288, opts.InboundMaxBytes)`.
|
||||
|
||||
**Step 2: Run tests — confirm they fail**
|
||||
|
||||
```
|
||||
dotnet test tests/ScadaLink.AuditLog.Tests \
|
||||
--filter "FullyQualifiedName~AuditLogOptionsValidatorTests|FullyQualifiedName~AuditLogOptionsBindingTests"
|
||||
```
|
||||
|
||||
Expected: the new validator tests fail (no `InboundMaxBytes` property), the binding test fails (property does not bind).
|
||||
|
||||
**Step 3: Add the property**
|
||||
|
||||
In `AuditLogOptions.cs`, insert after `RetentionDays`:
|
||||
|
||||
```csharp
|
||||
/// <summary>
|
||||
/// Per-body byte ceiling applied to <see cref="AuditEvent.RequestSummary"/> and
|
||||
/// <see cref="AuditEvent.ResponseSummary"/> for <see cref="AuditChannel.ApiInbound"/> rows
|
||||
/// (default 1 MiB). The 8 KiB / 64 KiB default/error caps that apply to other channels
|
||||
/// do not apply here — inbound traffic captures verbatim up to this ceiling and only
|
||||
/// then sets <see cref="AuditEvent.PayloadTruncated"/>. See
|
||||
/// <c>docs/plans/2026-05-23-inbound-api-full-response-audit-design.md</c>.
|
||||
/// </summary>
|
||||
public int InboundMaxBytes { get; set; } = 1_048_576;
|
||||
```
|
||||
|
||||
**Step 4: Add the validator branch**
|
||||
|
||||
In `AuditLogOptionsValidator.cs`, add the constants beside the existing retention bounds:
|
||||
|
||||
```csharp
|
||||
public const int MinInboundMaxBytes = 8_192;
|
||||
public const int MaxInboundMaxBytes = 16_777_216;
|
||||
```
|
||||
|
||||
Add the validation block inside `Validate` (after the retention check, before the `return`):
|
||||
|
||||
```csharp
|
||||
if (options.InboundMaxBytes < MinInboundMaxBytes || options.InboundMaxBytes > MaxInboundMaxBytes)
|
||||
{
|
||||
failures.Add(
|
||||
$"AuditLog:{nameof(AuditLogOptions.InboundMaxBytes)} ({options.InboundMaxBytes}) " +
|
||||
$"must be in [{MinInboundMaxBytes}, {MaxInboundMaxBytes}] bytes.");
|
||||
}
|
||||
```
|
||||
|
||||
**Step 5: Run tests — confirm they pass**
|
||||
|
||||
```
|
||||
dotnet test tests/ScadaLink.AuditLog.Tests
|
||||
```
|
||||
|
||||
Expected: all green, including the extended binding test and the new validator tests.
|
||||
|
||||
**Step 6: Build the whole solution**
|
||||
|
||||
```
|
||||
dotnet build ScadaLink.slnx
|
||||
```
|
||||
|
||||
Expected: 0 warnings, 0 errors.
|
||||
|
||||
**Step 7: Commit**
|
||||
|
||||
```
|
||||
git add src/ScadaLink.AuditLog/Configuration/AuditLogOptions.cs \
|
||||
src/ScadaLink.AuditLog/Configuration/AuditLogOptionsValidator.cs \
|
||||
tests/ScadaLink.AuditLog.Tests/Configuration/AuditLogOptionsBindingTests.cs \
|
||||
tests/ScadaLink.AuditLog.Tests/Configuration/AuditLogOptionsValidatorTests.cs
|
||||
git commit -m "feat(auditlog): add AuditLog:InboundMaxBytes option (default 1 MiB, [8 KiB, 16 MiB])"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Wire `InboundMaxBytes` into `DefaultAuditPayloadFilter` (TDD)
|
||||
|
||||
**What:** When `AuditEvent.Channel == AuditChannel.ApiInbound`, the filter selects `InboundMaxBytes` as the truncation cap instead of `DefaultCapBytes` / `ErrorCapBytes`. Redaction stages run unchanged.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ScadaLink.AuditLog/Payload/DefaultAuditPayloadFilter.cs` — change the one cap-selection line.
|
||||
- Test (new): `tests/ScadaLink.AuditLog.Tests/Payload/InboundChannelCapTests.cs` — pin the new behaviour.
|
||||
|
||||
**Step 1: Write the failing tests**
|
||||
|
||||
Create `tests/ScadaLink.AuditLog.Tests/Payload/InboundChannelCapTests.cs`:
|
||||
|
||||
```csharp
|
||||
using System.Text;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using ScadaLink.AuditLog.Configuration;
|
||||
using ScadaLink.AuditLog.Payload;
|
||||
using ScadaLink.AuditLog.Tests.Configuration; // for TestOptionsMonitor — confirm namespace via existing file
|
||||
using ScadaLink.Commons.Entities.Audit;
|
||||
using ScadaLink.Commons.Types.Enums;
|
||||
|
||||
namespace ScadaLink.AuditLog.Tests.Payload;
|
||||
|
||||
/// <summary>
|
||||
/// Pins the docs/plans/2026-05-23-inbound-api-full-response-audit-design.md
|
||||
/// inbound carve-out: ApiInbound rows use InboundMaxBytes (default 1 MiB) for
|
||||
/// RequestSummary / ResponseSummary truncation, NOT DefaultCapBytes /
|
||||
/// ErrorCapBytes. Other channels keep the existing caps.
|
||||
/// </summary>
|
||||
public class InboundChannelCapTests
|
||||
{
|
||||
private static AuditEvent MakeInbound(
|
||||
AuditStatus status,
|
||||
string? request = null,
|
||||
string? response = null) =>
|
||||
new()
|
||||
{
|
||||
EventId = Guid.NewGuid(),
|
||||
OccurredAtUtc = DateTime.UtcNow,
|
||||
Channel = AuditChannel.ApiInbound,
|
||||
Kind = status == AuditStatus.Delivered
|
||||
? AuditKind.InboundRequest
|
||||
: AuditKind.InboundRequest,
|
||||
Status = status,
|
||||
RequestSummary = request,
|
||||
ResponseSummary = response,
|
||||
};
|
||||
|
||||
[Fact]
|
||||
public void ApiInbound_Delivered_RequestBody_BelowInboundMaxBytes_NotTruncated()
|
||||
{
|
||||
// Body well above the legacy 8 KiB default cap but under the 1 MiB
|
||||
// inbound ceiling — must NOT truncate.
|
||||
var body = new string('a', 100_000);
|
||||
var opts = new AuditLogOptions(); // defaults
|
||||
var filter = new DefaultAuditPayloadFilter(
|
||||
new TestOptionsMonitor<AuditLogOptions>(opts),
|
||||
NullLogger<DefaultAuditPayloadFilter>.Instance);
|
||||
|
||||
var result = filter.Apply(MakeInbound(AuditStatus.Delivered, request: body));
|
||||
|
||||
Assert.False(result.PayloadTruncated);
|
||||
Assert.Equal(body.Length, result.RequestSummary!.Length);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ApiInbound_Delivered_ResponseBody_BelowInboundMaxBytes_NotTruncated()
|
||||
{
|
||||
var body = new string('a', 100_000);
|
||||
var opts = new AuditLogOptions();
|
||||
var filter = new DefaultAuditPayloadFilter(
|
||||
new TestOptionsMonitor<AuditLogOptions>(opts),
|
||||
NullLogger<DefaultAuditPayloadFilter>.Instance);
|
||||
|
||||
var result = filter.Apply(MakeInbound(AuditStatus.Delivered, response: body));
|
||||
|
||||
Assert.False(result.PayloadTruncated);
|
||||
Assert.Equal(body.Length, result.ResponseSummary!.Length);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ApiInbound_Failed_BodyAboveInboundMaxBytes_TruncatedToInboundMaxBytes()
|
||||
{
|
||||
// Even on error rows, the inbound cap is InboundMaxBytes (NOT ErrorCapBytes).
|
||||
var opts = new AuditLogOptions { InboundMaxBytes = 16_384 };
|
||||
var oversized = new string('z', 50_000);
|
||||
var filter = new DefaultAuditPayloadFilter(
|
||||
new TestOptionsMonitor<AuditLogOptions>(opts),
|
||||
NullLogger<DefaultAuditPayloadFilter>.Instance);
|
||||
|
||||
var result = filter.Apply(MakeInbound(AuditStatus.Failed, response: oversized));
|
||||
|
||||
Assert.True(result.PayloadTruncated);
|
||||
Assert.True(Encoding.UTF8.GetByteCount(result.ResponseSummary!) <= 16_384);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ApiOutbound_StillUsesDefaultCap_NotInboundMaxBytes()
|
||||
{
|
||||
// Regression guard: lifting the inbound cap MUST NOT change other
|
||||
// channels. An ApiOutbound 100 KB body still hits the 8 KiB cap.
|
||||
var opts = new AuditLogOptions();
|
||||
var body = new string('a', 100_000);
|
||||
var filter = new DefaultAuditPayloadFilter(
|
||||
new TestOptionsMonitor<AuditLogOptions>(opts),
|
||||
NullLogger<DefaultAuditPayloadFilter>.Instance);
|
||||
|
||||
var evt = new AuditEvent
|
||||
{
|
||||
EventId = Guid.NewGuid(),
|
||||
OccurredAtUtc = DateTime.UtcNow,
|
||||
Channel = AuditChannel.ApiOutbound,
|
||||
Kind = AuditKind.ApiCall,
|
||||
Status = AuditStatus.Delivered,
|
||||
RequestSummary = body,
|
||||
};
|
||||
var result = filter.Apply(evt);
|
||||
|
||||
Assert.True(result.PayloadTruncated);
|
||||
Assert.True(Encoding.UTF8.GetByteCount(result.RequestSummary!) <= opts.DefaultCapBytes);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Verify the `TestOptionsMonitor<T>` helper lives in `tests/ScadaLink.AuditLog.Tests/`. Grep:
|
||||
|
||||
```
|
||||
grep -rn "class TestOptionsMonitor" tests/ScadaLink.AuditLog.Tests
|
||||
```
|
||||
|
||||
If its namespace differs from `ScadaLink.AuditLog.Tests.Configuration`, update the `using` accordingly.
|
||||
|
||||
**Step 2: Run tests — confirm they fail**
|
||||
|
||||
```
|
||||
dotnet test tests/ScadaLink.AuditLog.Tests \
|
||||
--filter "FullyQualifiedName~InboundChannelCapTests"
|
||||
```
|
||||
|
||||
Expected: the inbound tests fail (filter still applies the 8 KiB cap to ApiInbound). The `ApiOutbound_StillUsesDefaultCap` test SHOULD pass even before the change — that's the regression baseline and stays green after.
|
||||
|
||||
**Step 3: Add the channel branch to the filter**
|
||||
|
||||
In `DefaultAuditPayloadFilter.cs`, replace the single line:
|
||||
|
||||
```csharp
|
||||
var cap = IsErrorStatus(rawEvent.Status) ? opts.ErrorCapBytes : opts.DefaultCapBytes;
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```csharp
|
||||
// Inbound API gets a dedicated, larger ceiling — request/response bodies are
|
||||
// captured verbatim up to InboundMaxBytes (default 1 MiB) so support can
|
||||
// replay exactly what the caller sent and what we returned. Other channels
|
||||
// keep the global 8 KiB / 64 KiB policy.
|
||||
// See docs/plans/2026-05-23-inbound-api-full-response-audit-design.md.
|
||||
var cap = rawEvent.Channel == AuditChannel.ApiInbound
|
||||
? opts.InboundMaxBytes
|
||||
: (IsErrorStatus(rawEvent.Status) ? opts.ErrorCapBytes : opts.DefaultCapBytes);
|
||||
```
|
||||
|
||||
**Step 4: Run tests — confirm they pass**
|
||||
|
||||
```
|
||||
dotnet test tests/ScadaLink.AuditLog.Tests
|
||||
```
|
||||
|
||||
Expected: all green. The existing `FilterIntegrationTests`, `BodyRegexRedactionTests`, `RedactionSafetyNetTests`, `SqlParamRedactionTests`, etc., MUST stay passing — the change is channel-scoped and the non-inbound cases never see the new branch.
|
||||
|
||||
**Step 5: Build the whole solution**
|
||||
|
||||
```
|
||||
dotnet build ScadaLink.slnx
|
||||
```
|
||||
|
||||
Expected: 0 warnings.
|
||||
|
||||
**Step 6: Commit**
|
||||
|
||||
```
|
||||
git add src/ScadaLink.AuditLog/Payload/DefaultAuditPayloadFilter.cs \
|
||||
tests/ScadaLink.AuditLog.Tests/Payload/InboundChannelCapTests.cs
|
||||
git commit -m "feat(auditlog): payload filter uses InboundMaxBytes for ApiInbound rows"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Capture the response body in `AuditWriteMiddleware` (TDD)
|
||||
|
||||
**What:** Implement the M5-deferred response-body capture. Wrap `HttpContext.Response.Body` with a buffering `MemoryStream` BEFORE `_next(ctx)`, restore + copy the buffered bytes back to the original stream AFTER the pipeline runs, then read the buffer as UTF-8 into `ResponseSummary` on the audit event. The `AuditEvent.ResponseSummary = null` line goes away.
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddleware.cs`.
|
||||
- Modify: `tests/ScadaLink.InboundAPI.Tests/Middleware/AuditWriteMiddlewareTests.cs` — extend.
|
||||
|
||||
**Step 1: Write the failing tests**
|
||||
|
||||
Append to `AuditWriteMiddlewareTests.cs` (before the closing class brace), keeping the existing `BuildContext` / `CreateMiddleware` helpers:
|
||||
|
||||
```csharp
|
||||
// ---------------------------------------------------------------------
|
||||
// Response body capture — Audit Log #23 (inbound full-response feature).
|
||||
// Until the M5-deferred work landed, ResponseSummary was always null.
|
||||
// These tests pin the new contract: the middleware wraps Response.Body,
|
||||
// runs the pipeline, copies the buffered bytes back to the real stream,
|
||||
// and stashes a UTF-8 string copy on ResponseSummary.
|
||||
// ---------------------------------------------------------------------
|
||||
|
||||
[Fact]
|
||||
public async Task ResponseBody_IsCaptured_OnResponseSummary()
|
||||
{
|
||||
var writer = new RecordingAuditWriter();
|
||||
var ctx = BuildContext();
|
||||
var responseJson = "{\"result\":42}";
|
||||
var mw = CreateMiddleware(async hc =>
|
||||
{
|
||||
hc.Response.StatusCode = 200;
|
||||
hc.Response.ContentType = "application/json";
|
||||
await hc.Response.WriteAsync(responseJson);
|
||||
}, writer);
|
||||
|
||||
await mw.InvokeAsync(ctx);
|
||||
|
||||
var evt = Assert.Single(writer.Events);
|
||||
Assert.Equal(responseJson, evt.ResponseSummary);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ResponseBody_IsForwardedToOriginalStream_DownstreamReadersSeeIt()
|
||||
{
|
||||
// Wrapping the response body must be TRANSPARENT — the real client
|
||||
// stream still receives every byte the pipeline wrote.
|
||||
var writer = new RecordingAuditWriter();
|
||||
var ctx = BuildContext();
|
||||
var captured = new MemoryStream();
|
||||
ctx.Response.Body = captured; // simulate the client/test sink
|
||||
|
||||
var responseJson = "{\"ok\":true}";
|
||||
var mw = CreateMiddleware(async hc =>
|
||||
{
|
||||
hc.Response.StatusCode = 200;
|
||||
await hc.Response.WriteAsync(responseJson);
|
||||
}, writer);
|
||||
|
||||
await mw.InvokeAsync(ctx);
|
||||
|
||||
Assert.Equal(responseJson, Encoding.UTF8.GetString(captured.ToArray()));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ResponseBody_Empty_LeavesResponseSummaryNull()
|
||||
{
|
||||
// No bytes written => null, not empty-string. Mirrors the request-body
|
||||
// contract in ReadBufferedRequestBodyAsync.
|
||||
var writer = new RecordingAuditWriter();
|
||||
var ctx = BuildContext();
|
||||
var mw = CreateMiddleware(hc =>
|
||||
{
|
||||
hc.Response.StatusCode = 204;
|
||||
return Task.CompletedTask;
|
||||
}, writer);
|
||||
|
||||
await mw.InvokeAsync(ctx);
|
||||
|
||||
var evt = Assert.Single(writer.Events);
|
||||
Assert.Null(evt.ResponseSummary);
|
||||
Assert.Equal(204, evt.HttpStatus);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ResponseBody_OnHandlerThrow_BodyCapturedUpToTheThrow()
|
||||
{
|
||||
// If the handler writes some bytes then throws, the audit row still
|
||||
// surfaces whatever the framework had flushed. The middleware re-throws
|
||||
// (audit is best-effort, the request's error path stays authoritative).
|
||||
var writer = new RecordingAuditWriter();
|
||||
var ctx = BuildContext();
|
||||
var boom = new InvalidOperationException("kaboom");
|
||||
var mw = CreateMiddleware(async hc =>
|
||||
{
|
||||
hc.Response.StatusCode = 500;
|
||||
await hc.Response.WriteAsync("partial");
|
||||
throw boom;
|
||||
}, writer);
|
||||
|
||||
var thrown = await Assert.ThrowsAsync<InvalidOperationException>(
|
||||
() => mw.InvokeAsync(ctx));
|
||||
Assert.Same(boom, thrown);
|
||||
|
||||
var evt = Assert.Single(writer.Events);
|
||||
Assert.Equal(AuditStatus.Failed, evt.Status);
|
||||
Assert.Equal("partial", evt.ResponseSummary);
|
||||
}
|
||||
```
|
||||
|
||||
**Step 2: Run tests — confirm they fail**
|
||||
|
||||
```
|
||||
dotnet test tests/ScadaLink.InboundAPI.Tests \
|
||||
--filter "FullyQualifiedName~AuditWriteMiddlewareTests"
|
||||
```
|
||||
|
||||
Expected: the four new tests fail (ResponseSummary stays null today). Pre-existing tests still pass (they don't assert ResponseSummary).
|
||||
|
||||
**Step 3: Implement response capture in the middleware**
|
||||
|
||||
Edit `AuditWriteMiddleware.cs`:
|
||||
|
||||
(a) Update the XML doc at the top — remove the "Response body capture is deferred to M5…" paragraph (lines 42-50 in the current file). Replace with:
|
||||
|
||||
```csharp
|
||||
/// <para>
|
||||
/// <b>Body capture.</b> The request body is buffered via
|
||||
/// <see cref="HttpRequestRewindExtensions.EnableBuffering(HttpRequest)"/> then
|
||||
/// rewound so the downstream endpoint handler still sees the full payload. The
|
||||
/// response body is captured by swapping <see cref="HttpResponse.Body"/> for a
|
||||
/// <see cref="MemoryStream"/> before the pipeline runs; after the pipeline
|
||||
/// returns, the buffered bytes are copied to the original stream (transparent
|
||||
/// to the real client) and read into <see cref="AuditEvent.ResponseSummary"/>.
|
||||
/// Truncation to the configured inbound ceiling happens in
|
||||
/// <see cref="ScadaLink.AuditLog.Payload.DefaultAuditPayloadFilter"/>; the
|
||||
/// middleware itself stores the full buffered content.
|
||||
/// </para>
|
||||
```
|
||||
|
||||
(b) Rewrite `InvokeAsync` so the response stream is swapped, the buffer is read post-pipeline (in `finally`, even on a thrown handler), and the original stream receives the bytes back:
|
||||
|
||||
```csharp
|
||||
public async Task InvokeAsync(HttpContext ctx)
|
||||
{
|
||||
var sw = Stopwatch.StartNew();
|
||||
ctx.Items[InboundExecutionIdItemKey] = Guid.NewGuid();
|
||||
|
||||
// Request body — buffer for both audit + downstream handler.
|
||||
ctx.Request.EnableBuffering();
|
||||
var requestBody = await ReadBufferedRequestBodyAsync(ctx.Request).ConfigureAwait(false);
|
||||
|
||||
// Response body — swap in a MemoryStream so the pipeline writes are
|
||||
// captured. The original Response.Body is restored in the finally block,
|
||||
// and the captured bytes are copied back to it so the real client still
|
||||
// receives every byte (transparent wrap). The captured string is then
|
||||
// available for the audit row.
|
||||
var originalResponseBody = ctx.Response.Body;
|
||||
using var responseBuffer = new MemoryStream();
|
||||
ctx.Response.Body = responseBuffer;
|
||||
|
||||
string? responseBody = null;
|
||||
Exception? thrown = null;
|
||||
try
|
||||
{
|
||||
await _next(ctx).ConfigureAwait(false);
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
thrown = ex;
|
||||
throw;
|
||||
}
|
||||
finally
|
||||
{
|
||||
sw.Stop();
|
||||
|
||||
// Whatever the handler managed to write — full success, partial
|
||||
// success before throwing, or nothing at all — copy back to the
|
||||
// original stream and read for audit.
|
||||
responseBody = await DrainResponseBufferAsync(responseBuffer, originalResponseBody)
|
||||
.ConfigureAwait(false);
|
||||
ctx.Response.Body = originalResponseBody;
|
||||
|
||||
EmitInboundAudit(ctx, sw.ElapsedMilliseconds, thrown, requestBody, responseBody);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
(c) Add the new helper (place beside `ReadBufferedRequestBodyAsync`):
|
||||
|
||||
```csharp
|
||||
/// <summary>
|
||||
/// Copies the bytes buffered in <paramref name="buffer"/> to
|
||||
/// <paramref name="originalBody"/> (so the real client still receives them)
|
||||
/// and returns a UTF-8 string copy for <see cref="AuditEvent.ResponseSummary"/>.
|
||||
/// Returns null when no bytes were written, mirroring the
|
||||
/// <see cref="ReadBufferedRequestBodyAsync"/> empty-body contract.
|
||||
/// </summary>
|
||||
private static async Task<string?> DrainResponseBufferAsync(
|
||||
MemoryStream buffer,
|
||||
Stream originalBody)
|
||||
{
|
||||
if (buffer.Length == 0)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
|
||||
buffer.Position = 0;
|
||||
// Copy first so the client never misses bytes even if the read for audit
|
||||
// throws somehow (defensive — MemoryStream.CopyToAsync to a sink shouldn't
|
||||
// throw on its own, but the original body may).
|
||||
try
|
||||
{
|
||||
await buffer.CopyToAsync(originalBody).ConfigureAwait(false);
|
||||
}
|
||||
catch
|
||||
{
|
||||
// Best-effort: a sink that refuses our copy is the sink's problem;
|
||||
// the audit still records what the handler produced. Do NOT rethrow.
|
||||
}
|
||||
|
||||
buffer.Position = 0;
|
||||
using var reader = new StreamReader(
|
||||
buffer,
|
||||
Encoding.UTF8,
|
||||
detectEncodingFromByteOrderMarks: false,
|
||||
bufferSize: 1024,
|
||||
leaveOpen: true);
|
||||
var content = await reader.ReadToEndAsync().ConfigureAwait(false);
|
||||
return string.IsNullOrEmpty(content) ? null : content;
|
||||
}
|
||||
```
|
||||
|
||||
(d) Change `EmitInboundAudit`'s signature to take the response body, and drop the `ResponseSummary = null` line:
|
||||
|
||||
```csharp
|
||||
private void EmitInboundAudit(
|
||||
HttpContext ctx,
|
||||
long durationMs,
|
||||
Exception? thrown,
|
||||
string? requestBody,
|
||||
string? responseBody)
|
||||
{
|
||||
// ... unchanged up to the AuditEvent constructor ...
|
||||
|
||||
var evt = new AuditEvent
|
||||
{
|
||||
// ... existing fields ...
|
||||
RequestSummary = requestBody,
|
||||
ResponseSummary = responseBody, // was null in the M4 deliverable
|
||||
PayloadTruncated = false,
|
||||
Extra = extra,
|
||||
ForwardState = null,
|
||||
};
|
||||
|
||||
// ... unchanged fire-and-forget write ...
|
||||
}
|
||||
```
|
||||
|
||||
**Step 4: Run tests — confirm they pass**
|
||||
|
||||
```
|
||||
dotnet test tests/ScadaLink.InboundAPI.Tests
|
||||
```
|
||||
|
||||
Expected: all green, including the four new response-body tests AND every pre-existing middleware test.
|
||||
|
||||
**Step 5: Build the whole solution**
|
||||
|
||||
```
|
||||
dotnet build ScadaLink.slnx
|
||||
```
|
||||
|
||||
Expected: 0 warnings.
|
||||
|
||||
**Step 6: Commit**
|
||||
|
||||
```
|
||||
git add src/ScadaLink.InboundAPI/Middleware/AuditWriteMiddleware.cs \
|
||||
tests/ScadaLink.InboundAPI.Tests/Middleware/AuditWriteMiddlewareTests.cs
|
||||
git commit -m "feat(inboundapi): AuditWriteMiddleware captures response body on ApiInbound audit rows"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Update `Component-AuditLog.md`
|
||||
|
||||
**What:** Reflect the inbound carve-out in the requirements doc — schema row descriptions + Payload Capture Policy.
|
||||
|
||||
**Files:**
|
||||
- Modify: `docs/requirements/Component-AuditLog.md`.
|
||||
|
||||
**Edits (all in-place — no copies):**
|
||||
|
||||
1. Schema table — find the `RequestSummary` row. Change its `Description` cell from:
|
||||
> Truncated request payload (configurable cap). Headers redacted.
|
||||
to:
|
||||
> Truncated request payload (configurable cap). Headers redacted. For `Channel = ApiInbound`, captured in full up to `AuditLog:InboundMaxBytes` (default 1 MiB) — see Payload Capture Policy.
|
||||
|
||||
2. Schema table — `ResponseSummary` row. Change its `Description` cell from:
|
||||
> Truncated response payload. Full on errors.
|
||||
to:
|
||||
> Truncated response payload. For `Channel = ApiInbound`, captured in full up to `AuditLog:InboundMaxBytes` (default 1 MiB). For other channels, capped at `DefaultCapBytes` by default and `ErrorCapBytes` on error rows.
|
||||
|
||||
3. **Payload Capture Policy** section — after the existing **Default cap** bullet, insert:
|
||||
|
||||
> - **Inbound API exception.** For `Channel = ApiInbound`, `RequestSummary` and `ResponseSummary` are captured in full up to a per-body hard ceiling of 1 MiB (configurable via `AuditLog:InboundMaxBytes`; default 1 048 576 bytes; min 8 192; max 16 777 216). The 8 KiB / 64 KiB default/error caps that apply to other channels do not apply here. `PayloadTruncated = 1` is set only when the inbound ceiling is hit — verbatim capture is the normal case. The ceiling applies independently to each body. Header redaction and per-target body redactors still run before persistence.
|
||||
|
||||
**Verify:** `git diff docs/requirements/Component-AuditLog.md` — three edits, no other lines touched.
|
||||
|
||||
**Commit:**
|
||||
|
||||
```
|
||||
git add docs/requirements/Component-AuditLog.md
|
||||
git commit -m "docs(audit): schema + Payload Capture Policy note inbound full-body carve-out"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 5: Update `Component-InboundAPI.md`
|
||||
|
||||
**What:** Update the two existing references that say "truncated request/response bodies per the Audit Log capture policy" to reflect the new wording.
|
||||
|
||||
**Files:**
|
||||
- Modify: `docs/requirements/Component-InboundAPI.md`.
|
||||
|
||||
**Edits:**
|
||||
|
||||
1. Around line 119 (the inbound-audit description in §Operational Audit / Logging) — change:
|
||||
> Every request — success or failure — emits one `ApiInbound.Completed` row to `ICentralAuditWriter` from request middleware before the HTTP response is flushed. The row captures the API key **name** (never the key material), remote IP, user-agent, response status, duration, and truncated request/response bodies per the Audit Log capture policy (see Component-AuditLog.md, Payload Capture Policy).
|
||||
to:
|
||||
> Every request — success or failure — emits one `ApiInbound.Completed` row to `ICentralAuditWriter` from request middleware before the HTTP response is flushed. The row captures the API key **name** (never the key material), remote IP, user-agent, response status, duration, and the request/response bodies. Bodies are captured in full up to `AuditLog:InboundMaxBytes` (default 1 MiB); `PayloadTruncated = 1` only when that ceiling is hit. Header redaction and per-target body redactors still apply (see Component-AuditLog.md, Payload Capture Policy).
|
||||
|
||||
2. Around line 202 (Dependencies → Audit Log) — change:
|
||||
> **Audit Log (#23)**: Every inbound API request emits an `ApiInbound.Completed` row via `ICentralAuditWriter` from request middleware (non-blocking for the HTTP response). Payload truncation/redaction follows the Audit Log Payload Capture Policy.
|
||||
to:
|
||||
> **Audit Log (#23)**: Every inbound API request emits an `ApiInbound.Completed` row via `ICentralAuditWriter` from request middleware (non-blocking for the HTTP response). Request and response bodies are captured in full up to `AuditLog:InboundMaxBytes` (default 1 MiB) per the Audit Log Payload Capture Policy; redaction (headers + per-target body redactors) still applies before persistence.
|
||||
|
||||
**Verify:**
|
||||
|
||||
```
|
||||
grep -nE "truncated request/response|InboundMaxBytes" docs/requirements/Component-InboundAPI.md
|
||||
```
|
||||
|
||||
Expected: no "truncated request/response" hits remain; two "InboundMaxBytes" hits land on the updated lines.
|
||||
|
||||
**Commit:**
|
||||
|
||||
```
|
||||
git add docs/requirements/Component-InboundAPI.md
|
||||
git commit -m "docs(inboundapi): note request/response bodies captured in full to InboundMaxBytes"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 6: Final solution build + full test run + branch summary
|
||||
|
||||
**What:** Confirm the cumulative change is green end-to-end and summarise the branch.
|
||||
|
||||
**Steps:**
|
||||
|
||||
1. `dotnet build ScadaLink.slnx` from the repo root — expect 0 warnings, 0 errors.
|
||||
|
||||
2. Run the affected test projects:
|
||||
|
||||
```
|
||||
dotnet test tests/ScadaLink.AuditLog.Tests
|
||||
dotnet test tests/ScadaLink.InboundAPI.Tests
|
||||
```
|
||||
|
||||
Expect both green. (The wider solution test run is optional but cheap — `dotnet test ScadaLink.slnx` if you want full coverage.)
|
||||
|
||||
3. `git log --oneline main..HEAD` — expect exactly five commits:
|
||||
- feat(auditlog): add AuditLog:InboundMaxBytes option …
|
||||
- feat(auditlog): payload filter uses InboundMaxBytes for ApiInbound rows
|
||||
- feat(inboundapi): AuditWriteMiddleware captures response body on ApiInbound audit rows
|
||||
- docs(audit): schema + Payload Capture Policy note inbound full-body carve-out
|
||||
- docs(inboundapi): note request/response bodies captured in full to InboundMaxBytes
|
||||
|
||||
4. `git status --short` — expect a clean tree (no uncommitted files; pre-existing uncommitted files from `git status` at session start may still be present and are unrelated to this work — leave them).
|
||||
|
||||
**Acceptance:**
|
||||
- All acceptance criteria in `docs/plans/2026-05-23-inbound-api-full-response-audit-design.md` met.
|
||||
- Solution builds clean.
|
||||
- All targeted tests pass.
|
||||
- Five commits on the feature branch, no commits on `main`, branch not pushed.
|
||||
|
||||
**Commit:** none. Do not merge or push — that is the user's call.
|
||||
|
||||
---
|
||||
|
||||
## Notes for the executor
|
||||
|
||||
- The `ResponseSummary = null` comment in `AuditWriteMiddleware.cs` (line ~191 today) is the smoking-gun: response capture was always intended, just deferred. The design doc explicitly authorises closing that gap.
|
||||
- The `TestOptionsMonitor<T>` helper is already used by `AuditLogOptionsBindingTests.cs` — reuse it; do not introduce a second one. If its public location moves between when this plan was written and execution, `grep -rn "class TestOptionsMonitor" tests/` and adjust the `using`.
|
||||
- `appsettings.json` files in `docker/central-node-a` and `docker/central-node-b` do not currently override `AuditLog:*` — leave them alone. The 1 MiB default takes effect automatically from `AuditLogOptions`.
|
||||
- No EF migration is needed — schema is unchanged (`nvarchar(max)` already).
|
||||
- No new health metric — `PayloadTruncated = 1` carries the ceiling-hit signal. The design doc explicitly defers a dedicated `AuditInboundCeilingHits` counter.
|
||||
@@ -0,0 +1,13 @@
|
||||
{
|
||||
"planPath": "docs/plans/2026-05-23-inbound-api-full-response-audit.md",
|
||||
"tasks": [
|
||||
{"id": 1, "subject": "Task 0: Prep — branch, baseline build", "status": "pending"},
|
||||
{"id": 2, "subject": "Task 1: Add InboundMaxBytes to AuditLogOptions (TDD)", "status": "pending", "blockedBy": [1]},
|
||||
{"id": 3, "subject": "Task 2: Wire InboundMaxBytes into DefaultAuditPayloadFilter (TDD)", "status": "pending", "blockedBy": [2]},
|
||||
{"id": 4, "subject": "Task 3: Capture response body in AuditWriteMiddleware (TDD)", "status": "pending", "blockedBy": [3]},
|
||||
{"id": 5, "subject": "Task 4: Update Component-AuditLog.md", "status": "pending", "blockedBy": [4]},
|
||||
{"id": 6, "subject": "Task 5: Update Component-InboundAPI.md", "status": "pending", "blockedBy": [5]},
|
||||
{"id": 7, "subject": "Task 6: Final build + full test run + branch summary", "status": "pending", "blockedBy": [6]}
|
||||
],
|
||||
"lastUpdated": "2026-05-23"
|
||||
}
|
||||
Reference in New Issue
Block a user