Keep the deployed API's OpenAPI document closed, deliberately #127

Merged
rob merged 2 commits from feat/close-openapi-in-production into main 2026-08-11 21:30:29 +00:00
Owner

Task 189. The Compose full-stack API container runs as Production (no ASPNETCORE_ENVIRONMENT set), so /openapi and /swagger 404 there by ADR-0029's own design — task 119 worked around it with a committed snapshot. This was never actually decided; it was the accidental side effect of an unset variable.

Decision: keep it closed. IsDocumented is unchanged — no code change makes this true, only a record saying it's deliberate. The repo has been public since 2026-08-03, so an interactive explorer doesn't reveal the API's shape; what it hands an anonymous visitor is a ready, pre-wired client against the live deployed origin, "Try it out" included — real attack surface for a hobby-scale, conservative posture, bought for nothing this project currently needs. Opening even the raw JSON alone was considered and rejected too: a stable Production URL is a published contract on ADR-0085's own terms, and trips that record's own named trigger for reopening it.

That makes docs/openapi/v1.json the supported answer rather than a workaround, which makes ADR-0108's own accepted gap — nothing checks it against the live document — the thing actually worth fixing.

OpenApiSnapshotTests.CommittedSnapshot_MatchesTheLiveDocument starts a Development-environment host, fetches /openapi/v1.json exactly as a real caller would, and compares it against the committed file — parsed as JSON (JsonNode.DeepEquals), not as text. A literal string comparison was tried first and rejected on real evidence: the live document's non-ASCII characters arrive over HTTP unescaped, the committed file holds them \u-escaped, and that alone reddened a snapshot with no actual drift. Mutation-tested: changed the committed file's info.title, confirmed the test reddens with the live document written to a temp file and the exact cp command to fix it, restored, confirmed green.

ADR-0111 records both halves. ADR-0108 gains Answered by: ADR-0111 for the gap it named and left open (ADR-0107's mechanism, its second real use).

Merged main (#125's ADR-0110 landed since branching); index regenerated via AdrIndexTests, not hand-merged.

Full suite, dotnet format --verify-no-changes, and the coverage ratchet all clean on the pinned SDK.

Task 189. The Compose full-stack API container runs as Production (no `ASPNETCORE_ENVIRONMENT` set), so `/openapi` and `/swagger` 404 there by ADR-0029's own design — task 119 worked around it with a committed snapshot. This was never actually decided; it was the accidental side effect of an unset variable. **Decision: keep it closed.** `IsDocumented` is unchanged — no code change makes this true, only a record saying it's deliberate. The repo has been public since 2026-08-03, so an interactive explorer doesn't reveal the API's shape; what it hands an anonymous visitor is a ready, pre-wired client against the *live* deployed origin, "Try it out" included — real attack surface for a hobby-scale, conservative posture, bought for nothing this project currently needs. Opening even the raw JSON alone was considered and rejected too: a stable Production URL is a published contract on ADR-0085's own terms, and trips that record's own named trigger for reopening it. That makes `docs/openapi/v1.json` the supported answer rather than a workaround, which makes ADR-0108's own accepted gap — nothing checks it against the live document — the thing actually worth fixing. **`OpenApiSnapshotTests.CommittedSnapshot_MatchesTheLiveDocument`** starts a `Development`-environment host, fetches `/openapi/v1.json` exactly as a real caller would, and compares it against the committed file — parsed as JSON (`JsonNode.DeepEquals`), not as text. A literal string comparison was tried first and rejected on real evidence: the live document's non-ASCII characters arrive over HTTP unescaped, the committed file holds them `\u`-escaped, and that alone reddened a snapshot with no actual drift. Mutation-tested: changed the committed file's `info.title`, confirmed the test reddens with the live document written to a temp file and the exact `cp` command to fix it, restored, confirmed green. ADR-0111 records both halves. ADR-0108 gains `Answered by: ADR-0111` for the gap it named and left open (ADR-0107's mechanism, its second real use). Merged `main` (#125's ADR-0110 landed since branching); index regenerated via `AdrIndexTests`, not hand-merged. Full suite, `dotnet format --verify-no-changes`, and the coverage ratchet all clean on the pinned SDK.
Merge remote-tracking branch 'origin/main' into feat/close-openapi-in-production
All checks were successful
CI / build (pull_request) Successful in 3m16s
CI / container-images (pull_request) Has been skipped
CI / e2e (pull_request) Successful in 1m51s
9d1e3cc696
# Conflicts:
#	docs/adr/README.md
rob left a comment

Verdict: mergeable

CI green on 9d1e3cc (run #464: build 648 success, e2e 650 success) — checked via /actions/tasks, not get_workflow_run.

1. Mutation-tested the drift guard myself, with drift that matters. Removed /health/live entirely from the committed docs/openapi/v1.json — reddened, with the temp-file path and exact cp fix in the message. Restored, re-tried by emptying a schema's required array (BeginOidcLinkResponse) — reddened again. Restored both times, confirmed green. This guard catches real shape drift, not only info.title.

2. JsonNode.DeepEquals semantics, measured, not assumed. A throwaway check confirms it's order-insensitive for object keys ({"a":1,"b":2} == {"b":2,"a":1}) but order-sensitive for arrays ([1,2,3] != [3,2,1]), including nested arrays inside objects. So paths being a JSON object is safe regardless of route-enumeration order, but any array in the document (tags, required, security) isn't protected by DeepEquals alone — it needs the underlying generation to be deterministic. Checked that directly: fetched the live document from two independent WebApplicationFactory instances, five separate process runs (ten fetches total), asserted byte-for-byte text equality every time. No ordering flake found.

3. The test's Development host doesn't leak into deployment. Diff is confined to the test file, a doc comment, the ADRs and README — no Program.cs, docker-compose.yml, or appsettings.* touched. Connection string is the same unroutable TEST-NET-1 pattern already used elsewhere (CorsPolicyTests and others), not a new convention.

4. The decision holds up. Public-since-2026-08-03 makes the "reveals nothing not already in source" claim correct, and the real distinction (pre-wired live client vs. static disclosure) is the one actually argued, not asserted. Both alternatives — open the UI, open raw JSON alone — get real reasoning in ADR-0111's own Alternatives section, tied concretely to ADR-0085's named reopening trigger, not a token mention.

5. Answered by: ADR-0111 fits ADR-0107's narrow definition. ADR-0108's own Consequences and Alternatives sections explicitly name the missing drift check as a specific, deferred gap ("Whoever picks up the CI-check alternative above should read this record first") — a genuine explicit deferral, not a later record merely touching the same area. ADR-0111 answers that exact question, even though the mechanism (in-process test vs. the CI job ADR-0108 sketched) differs — the deferred question was "is there a check", not "is it a CI job", and ADR-0111's own Alternatives section explains that substitution.

AdrIndexTests pass locally (12/12), index correctly carries 0110/0111 and not 0112 (mine, still open). dotnet format --verify-no-changes clean.

Verdict: mergeable CI green on `9d1e3cc` (run #464: `build` 648 success, `e2e` 650 success) — checked via `/actions/tasks`, not `get_workflow_run`. **1. Mutation-tested the drift guard myself, with drift that matters.** Removed `/health/live` entirely from the committed `docs/openapi/v1.json` — reddened, with the temp-file path and exact `cp` fix in the message. Restored, re-tried by emptying a schema's `required` array (`BeginOidcLinkResponse`) — reddened again. Restored both times, confirmed green. This guard catches real shape drift, not only `info.title`. **2. `JsonNode.DeepEquals` semantics, measured, not assumed.** A throwaway check confirms it's order-insensitive for object keys (`{"a":1,"b":2}` == `{"b":2,"a":1}`) but order-*sensitive* for arrays (`[1,2,3]` != `[3,2,1]`), including nested arrays inside objects. So `paths` being a JSON object is safe regardless of route-enumeration order, but any array in the document (`tags`, `required`, `security`) isn't protected by DeepEquals alone — it needs the underlying generation to be deterministic. Checked that directly: fetched the live document from two independent `WebApplicationFactory` instances, five separate process runs (ten fetches total), asserted byte-for-byte text equality every time. No ordering flake found. **3. The test's `Development` host doesn't leak into deployment.** Diff is confined to the test file, a doc comment, the ADRs and README — no `Program.cs`, `docker-compose.yml`, or `appsettings.*` touched. Connection string is the same unroutable TEST-NET-1 pattern already used elsewhere (`CorsPolicyTests` and others), not a new convention. **4. The decision holds up.** Public-since-2026-08-03 makes the "reveals nothing not already in source" claim correct, and the real distinction (pre-wired live client vs. static disclosure) is the one actually argued, not asserted. Both alternatives — open the UI, open raw JSON alone — get real reasoning in ADR-0111's own Alternatives section, tied concretely to ADR-0085's named reopening trigger, not a token mention. **5. `Answered by: ADR-0111` fits ADR-0107's narrow definition.** ADR-0108's own Consequences and Alternatives sections explicitly name the missing drift check as a specific, deferred gap ("Whoever picks up the CI-check alternative above should read this record first") — a genuine explicit deferral, not a later record merely touching the same area. ADR-0111 answers that exact question, even though the mechanism (in-process test vs. the CI job ADR-0108 sketched) differs — the deferred question was "is there a check", not "is it a CI job", and ADR-0111's own Alternatives section explains that substitution. `AdrIndexTests` pass locally (12/12), index correctly carries 0110/0111 and not 0112 (mine, still open). `dotnet format --verify-no-changes` clean.
rob merged commit 02ae097ca8 into main 2026-08-11 21:30:29 +00:00
rob deleted branch feat/close-openapi-in-production 2026-08-11 21:30:29 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rob/PlaceMark!127
No description provided.