diff --git a/backend/src/BigRegister.Api/Zgw/ZgwHttpClient.cs b/backend/src/BigRegister.Api/Zgw/ZgwHttpClient.cs index 6890fa6..6882c5d 100644 --- a/backend/src/BigRegister.Api/Zgw/ZgwHttpClient.cs +++ b/backend/src/BigRegister.Api/Zgw/ZgwHttpClient.cs @@ -24,7 +24,7 @@ internal sealed class ZgwHttpClient(HttpClient http, ZgwTokenProvider tokens) { using var res = await SendWithRetryAsync(() => new HttpRequestMessage(HttpMethod.Get, url), caller); return (await res.Content.ReadFromJsonAsync()) - ?? throw new InvalidOperationException($"ZGW GET {url} returned null body."); + ?? throw new InvalidOperationException($"ZGW GET {Redact(url)} returned null body."); } public async Task PostAsync(string url, object body, CallerIdentity? caller = null) @@ -32,7 +32,7 @@ internal sealed class ZgwHttpClient(HttpClient http, ZgwTokenProvider tokens) using var res = await SendWithRetryAsync( () => new HttpRequestMessage(HttpMethod.Post, url) { Content = JsonContent.Create(body) }, caller); return (await res.Content.ReadFromJsonAsync()) - ?? throw new InvalidOperationException($"ZGW POST {url} returned null body."); + ?? throw new InvalidOperationException($"ZGW POST {Redact(url)} returned null body."); } /// @@ -42,7 +42,7 @@ internal sealed class ZgwHttpClient(HttpClient http, ZgwTokenProvider tokens) /// partial commit on the two non-idempotent ZGW POSTs (/statussen, /rollen) and /// retrying risks a duplicate write — the create-zaak/document POSTs are additionally /// 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 /// Program.cs's submit endpoint catches and records as a flagged divergence rather /// than letting it diverge silently (see openzaak-integration.md's "Write resilience" section). /// @@ -73,15 +73,26 @@ internal sealed class ZgwHttpClient(HttpClient http, ZgwTokenProvider tokens) continue; } - var body = await res.Content.ReadAsStringAsync(); - var snippet = body.Length > 500 ? body[..500] : body; - var message = $"ZGW {req.Method} {req.RequestUri} failed: {(int)res.StatusCode} {snippet}"; + // RB-05/BIO-009: path only — no query string, no response-body snippet. The + // BSN-filtered zaken list puts a BSN in the query, and OpenZaak echoes the request in + // 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; res.Dispose(); throw new HttpRequestException(message, null, status); } } + /// The path without its query string — ZGW filters travel as query parameters and + /// one of them is a BSN (rol__betrokkeneIdentificatie__natuurlijkPersoon__inpBsn), so no + /// ZGW url may be interpolated into a message that is logged or persisted (RB-05). + 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 HttpStatusCode.RequestTimeout or HttpStatusCode.TooManyRequests or HttpStatusCode.BadGateway or HttpStatusCode.ServiceUnavailable or HttpStatusCode.GatewayTimeout; diff --git a/backend/tests/BigRegister.Tests/ZgwDivergenceTests.cs b/backend/tests/BigRegister.Tests/ZgwDivergenceTests.cs index 881e922..be845bb 100644 --- a/backend/tests/BigRegister.Tests/ZgwDivergenceTests.cs +++ b/backend/tests/BigRegister.Tests/ZgwDivergenceTests.cs @@ -111,6 +111,32 @@ public class ZgwDivergenceTests 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) { var req = new HttpRequestMessage(method, path); diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-05.md b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-05.md new file mode 100644 index 0000000..d633420 --- /dev/null +++ b/docs/project/refactor-backlog-setup/refactor-backlog/implementation/rb-05.md @@ -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=`. +- **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.