fix(zgw): keep the BSN out of the recorded ZGW failure message (RB-05)
ZgwHttpClient interpolated the full request uri and up to 500 characters of the response body into its failure message. That message is persisted as Aanvraag.ZgwError in SQLite and written to the log, and both halves can carry a BSN: ZGW filters travel as query parameters (the citizen-scoped zaken list filters on rol__betrokkeneIdentificatie__natuurlijkPersoon__inpBsn), and OpenZaak echoes the offending request in its error bodies, so a rejected POST /rollen comes back holding the owner BSN it was sent. All three interpolation sites now use Redact(url) — the path without its query — and the body snippet is replaced by the reason phrase. Status plus path still routes a failure to the right endpoint; the lost detail already has a deliberate home in ZGW_DEBUG_HTTP=1 (ZgwDiagnosticHandler), which is opt-in, dev-only and not persisted. The new test fails the one call in the fixture whose url carries a query string and asserts the persisted ZgwError has neither the body snippet nor a "?", while keeping the path and the 503. Verified red without the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -24,7 +24,7 @@ internal sealed class ZgwHttpClient(HttpClient http, ZgwTokenProvider tokens)
|
|||||||
{
|
{
|
||||||
using var res = await SendWithRetryAsync(() => new HttpRequestMessage(HttpMethod.Get, url), caller);
|
using var res = await SendWithRetryAsync(() => new HttpRequestMessage(HttpMethod.Get, url), caller);
|
||||||
return (await res.Content.ReadFromJsonAsync<T>())
|
return (await res.Content.ReadFromJsonAsync<T>())
|
||||||
?? throw new InvalidOperationException($"ZGW GET {url} returned null body.");
|
?? throw new InvalidOperationException($"ZGW GET {Redact(url)} returned null body.");
|
||||||
}
|
}
|
||||||
|
|
||||||
public async Task<T> PostAsync<T>(string url, object body, CallerIdentity? caller = null)
|
public async Task<T> PostAsync<T>(string url, object body, CallerIdentity? caller = null)
|
||||||
@@ -32,7 +32,7 @@ internal sealed class ZgwHttpClient(HttpClient http, ZgwTokenProvider tokens)
|
|||||||
using var res = await SendWithRetryAsync(
|
using var res = await SendWithRetryAsync(
|
||||||
() => new HttpRequestMessage(HttpMethod.Post, url) { Content = JsonContent.Create(body) }, caller);
|
() => new HttpRequestMessage(HttpMethod.Post, url) { Content = JsonContent.Create(body) }, caller);
|
||||||
return (await res.Content.ReadFromJsonAsync<T>())
|
return (await res.Content.ReadFromJsonAsync<T>())
|
||||||
?? throw new InvalidOperationException($"ZGW POST {url} returned null body.");
|
?? throw new InvalidOperationException($"ZGW POST {Redact(url)} returned null body.");
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
@@ -42,7 +42,7 @@ internal sealed class ZgwHttpClient(HttpClient http, ZgwTokenProvider tokens)
|
|||||||
/// partial commit on the two non-idempotent ZGW POSTs (<c>/statussen</c>, <c>/rollen</c>) and
|
/// partial commit on the two non-idempotent ZGW POSTs (<c>/statussen</c>, <c>/rollen</c>) and
|
||||||
/// retrying risks a duplicate write — the create-zaak/document POSTs are additionally
|
/// retrying risks a duplicate write — the create-zaak/document POSTs are additionally
|
||||||
/// protected by OpenZaak's own uniqueness constraint on (bronorganisatie, identificatie).
|
/// protected by OpenZaak's own uniqueness constraint on (bronorganisatie, identificatie).
|
||||||
/// A non-transient (or exhausted) failure throws with the status + a body snippet, which
|
/// A non-transient (or exhausted) failure throws with the status + the redacted path, which
|
||||||
/// <c>Program.cs</c>'s submit endpoint catches and records as a flagged divergence rather
|
/// <c>Program.cs</c>'s submit endpoint catches and records as a flagged divergence rather
|
||||||
/// than letting it diverge silently (see openzaak-integration.md's "Write resilience" section).
|
/// than letting it diverge silently (see openzaak-integration.md's "Write resilience" section).
|
||||||
/// </summary>
|
/// </summary>
|
||||||
@@ -73,15 +73,26 @@ internal sealed class ZgwHttpClient(HttpClient http, ZgwTokenProvider tokens)
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
var body = await res.Content.ReadAsStringAsync();
|
// RB-05/BIO-009: path only — no query string, no response-body snippet. The
|
||||||
var snippet = body.Length > 500 ? body[..500] : body;
|
// BSN-filtered zaken list puts a BSN in the query, and OpenZaak echoes the request in
|
||||||
var message = $"ZGW {req.Method} {req.RequestUri} failed: {(int)res.StatusCode} {snippet}";
|
// its error bodies, so both used to reach a message Program.cs persists as a flagged
|
||||||
|
// divergence and writes to the application log. Status + path routes the failure;
|
||||||
|
// ZGW_DEBUG_HTTP=1 (ZgwDiagnosticHandler) is the deliberate opt-in for the rest.
|
||||||
|
var message = $"ZGW {req.Method} {Redact(req.RequestUri)} failed: {(int)res.StatusCode} {res.ReasonPhrase}";
|
||||||
var status = res.StatusCode;
|
var status = res.StatusCode;
|
||||||
res.Dispose();
|
res.Dispose();
|
||||||
throw new HttpRequestException(message, null, status);
|
throw new HttpRequestException(message, null, status);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>The path without its query string — ZGW filters travel as query parameters and
|
||||||
|
/// one of them is a BSN (<c>rol__betrokkeneIdentificatie__natuurlijkPersoon__inpBsn</c>), so no
|
||||||
|
/// ZGW url may be interpolated into a message that is logged or persisted (RB-05).</summary>
|
||||||
|
private static string Redact(string url) =>
|
||||||
|
Uri.TryCreate(url, UriKind.Absolute, out var u) ? u.GetLeftPart(UriPartial.Path) : url.Split('?')[0];
|
||||||
|
|
||||||
|
private static string Redact(Uri? url) => url is null ? "(no uri)" : url.GetLeftPart(UriPartial.Path);
|
||||||
|
|
||||||
private static bool IsTransient(HttpStatusCode status) => status is
|
private static bool IsTransient(HttpStatusCode status) => status is
|
||||||
HttpStatusCode.RequestTimeout or HttpStatusCode.TooManyRequests or
|
HttpStatusCode.RequestTimeout or HttpStatusCode.TooManyRequests or
|
||||||
HttpStatusCode.BadGateway or HttpStatusCode.ServiceUnavailable or HttpStatusCode.GatewayTimeout;
|
HttpStatusCode.BadGateway or HttpStatusCode.ServiceUnavailable or HttpStatusCode.GatewayTimeout;
|
||||||
|
|||||||
@@ -111,6 +111,32 @@ public class ZgwDivergenceTests
|
|||||||
Assert.Null(stored.ZgwError);
|
Assert.Null(stored.ZgwError);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// RB-05/BIO-009: `ZgwError` is persisted to SQLite and written to the application log, so
|
||||||
|
/// the message it carries may not include the response body (OpenZaak echoes the request in
|
||||||
|
/// its errors) or the request's query string (ZGW filters travel there, and one of them is
|
||||||
|
/// `rol__betrokkeneIdentificatie__natuurlijkPersoon__inpBsn`).
|
||||||
|
[Fact]
|
||||||
|
public async Task A_recorded_divergence_carries_no_response_body_and_no_query_string()
|
||||||
|
{
|
||||||
|
// The zaak POST succeeds; the statustypen GET — the one call here that carries a query
|
||||||
|
// string — fails, so the recorded message is built from a url that has one.
|
||||||
|
var stub = new ZgwStubHandler(SuccessBody,
|
||||||
|
(url, _) => url.StartsWith($"{ZtBase}/statustypen") ? HttpStatusCode.ServiceUnavailable : HttpStatusCode.OK);
|
||||||
|
using var factory = Factory(stub);
|
||||||
|
using var client = factory.CreateClient();
|
||||||
|
|
||||||
|
var id = await CreateConcept(client);
|
||||||
|
(await client.PostAsJsonAsync($"/api/v1/applications/{id}/submit", new { diplomaHerkomst = "duo" }))
|
||||||
|
.EnsureSuccessStatusCode();
|
||||||
|
|
||||||
|
var error = ApplicationStore.ListAll().Single(a => a.Id == id).ZgwError;
|
||||||
|
Assert.NotNull(error);
|
||||||
|
Assert.DoesNotContain("stub failure", error); // no response-body snippet
|
||||||
|
Assert.DoesNotContain("?", error); // no query string
|
||||||
|
Assert.Contains($"{ZtBase}/statustypen", error); // the path still routes the failure
|
||||||
|
Assert.Contains("503", error);
|
||||||
|
}
|
||||||
|
|
||||||
private static HttpRequestMessage AdminRequest(HttpMethod method, string path)
|
private static HttpRequestMessage AdminRequest(HttpMethod method, string path)
|
||||||
{
|
{
|
||||||
var req = new HttpRequestMessage(method, path);
|
var req = new HttpRequestMessage(method, path);
|
||||||
|
|||||||
@@ -0,0 +1,50 @@
|
|||||||
|
# RB-05 — drop the BSN-bearing query and body snippet from the ZGW failure message
|
||||||
|
|
||||||
|
Status: **implemented** · 2026-08-27 · Source findings: `07-bio2-compliance.md` BIO-009 · `99-backlog.md` RB-05
|
||||||
|
|
||||||
|
## What was wrong
|
||||||
|
|
||||||
|
`ZgwHttpClient.SendWithRetryAsync` built its failure message as
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
$"ZGW {req.Method} {req.RequestUri} failed: {(int)res.StatusCode} {snippet}"
|
||||||
|
```
|
||||||
|
|
||||||
|
with `snippet` being up to 500 characters of the **response body**. That message is not
|
||||||
|
transient: `Program.cs`'s submit endpoint catches it and stores it as `Aanvraag.ZgwError`
|
||||||
|
in SQLite, and logs it.
|
||||||
|
|
||||||
|
Two BSN paths into it:
|
||||||
|
|
||||||
|
- **the query string.** ZGW filters travel as query parameters, and the citizen-scoped zaken
|
||||||
|
list filters on `rol__betrokkeneIdentificatie__natuurlijkPersoon__inpBsn=<BSN>`.
|
||||||
|
- **the body snippet.** OpenZaak's error responses echo the offending request, so a rejected
|
||||||
|
`POST /rollen` (whose body carries `BetrokkeneIdentificatie(aanvraag.Owner)`) comes back
|
||||||
|
with the BSN in it.
|
||||||
|
|
||||||
|
The two `"returned null body"` throws in `GetAsync`/`PostAsync` interpolated the same url.
|
||||||
|
|
||||||
|
## What changed
|
||||||
|
|
||||||
|
| File | Change |
|
||||||
|
| ------------------------ | --------------------------------------------------------------------------------- |
|
||||||
|
| `Zgw/ZgwHttpClient.cs` | `Redact(url)` (path only) at all three sites; snippet → `res.ReasonPhrase` |
|
||||||
|
| `ZgwDivergenceTests.cs` | **new** `A_recorded_divergence_carries_no_response_body_and_no_query_string` |
|
||||||
|
|
||||||
|
Status + path is enough to route a failure to the right endpoint. The diagnostic detail
|
||||||
|
that was lost already has a deliberate home: `ZGW_DEBUG_HTTP=1` wires
|
||||||
|
`ZgwDiagnosticHandler`, which logs the full url and request bytes — opt-in, dev-only, and
|
||||||
|
not persisted.
|
||||||
|
|
||||||
|
## The test
|
||||||
|
|
||||||
|
Fails the `statustypen` GET (the only call in that fixture whose url carries a query
|
||||||
|
string) after the zaak POST succeeds, then asserts on the persisted `ZgwError`:
|
||||||
|
no `"stub failure"` (the body snippet), no `"?"` (the query string), but still the path and
|
||||||
|
the `503`. **Confirmed it fails without the fix** — restoring the old interpolation turns it
|
||||||
|
red on both counts.
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
`dotnet format --verify-no-changes` clean. `dotnet test`: **253 passed, 1 failed** — the
|
||||||
|
pre-existing `OpenZaakIntegrationTests.Admin_cases_…`, which needs a live container.
|
||||||
Reference in New Issue
Block a user