refactor: audit-driven cleanup wave + zero out PR/issue backlog#111
Merged
refactor: audit-driven cleanup wave + zero out PR/issue backlog#111
Conversation
Closes the post-PR110 audit loop addressing all 4 explore agents'
verified findings. Folds in all 6 open Dependabot bumps, fixes the
3 doc-doctrine drifts that contradict DOCTRINE.md, and adds a chunk
of real prod-readiness coverage.
## Wave A — DOCTRINE drift in 3 docs (CRITICAL)
* docs/validation.md (lines 54, 57, 156) — replaced
'[Authorize(Roles = "Tasks.Process.All")]' (silently no-ops on
MapInboundClaims=false JWT schemes) with the named-policy form
'[Authorize(Policy = OrdersAuthorizationPolicies.App)]'. Switched
generic 'Files.Read' / 'Tasks.Process.All' examples to the FTGO
domain ('orders.read' / 'Orders.Process'). Added pointer to
DOCTRINE.md § 'Authorization-policy doctrine'.
* docs/decision-trees.md Tree 4 — same fix on the mermaid + body.
* docs/operations.md — env-rename runbook (lines 184–) marked as a
template / historical example (the dev->ci rename happened in PR
#103; current main has no 'dev' tier).
## Wave B — Test coverage gaps (security-relevant)
* tests/EntraAuthForwardedHeadersExtensionsTests.cs (new) — 4 facts
covering X-Forwarded-{For,Proto,Host} parsing + ForwardLimit cap.
* tests/EntraAuthRateLimiterTests.cs — added Theory cases for
long-form claim fallbacks (http://schemas.microsoft.com/identity/
claims/{tenantid,objectidentifier,appid}), unknown-* paths, plus
partition isolation across 2 distinct users + Retry-After-on-429
RFC 6585 §3 conformance.
* tests/DownstreamApiClientClaimsChallengeTests.cs — added Theory
for RFC 7235 edge cases (case-insensitive params, comma in quoted
value, empty Bearer, multi-Bearer first-wins, non-Bearer scheme).
* src/Ftgo.Auth/EntraAuthRateLimiterExtensions.cs — fixed RFC 6585
Retry-After header bug surfaced by new test: lease metadata is
not always populated by sliding-window limiters; fall back to the
configured Window so 429 responses always carry Retry-After.
## Wave C — Infrastructure adds
* infra/bicep/modules/log-analytics.bicep — retentionInDays now
parameterized (@minValue 30 @MaxValue 730); azure.bicep wires
90 d for prod, 30 d for ci/ppe (audit compliance floor for prod).
* infra/bicep/modules/budget.bicep (new) — Microsoft.Consumption/
budgets with Forecasted+Actual notifications at 80% and 100%.
* infra/bicep/azure.bicep — wires the budget module; defaults
USD 50/mo for prod, USD 10/mo for ci/ppe.
* infra/bicep/modules/container-app.bicep — transport='https' for
prod (was 'auto', allowing HTTP fallback).
* .github/workflows/drift-detection.yml (new) — weekly + manual
dispatch, matrix [ci, ppe], 'az deployment group what-if' with
::warning:: annotation if drift detected. Skips tiers whose RG
doesn't exist (PPE-not-yet-bootstrapped case).
## Wave D — Polish
* src/Ftgo.Auth.Client/DownstreamApiClient.cs — ConfigureAwait(false)
on the 3 awaits in ProbeAsync (library-code best practice).
* .github/workflows/cd-cleanup.yml — pre-checks AZURE_CLIENT_ID
before azure/login. Fixes the long-standing PPE auth failure
(PPE env never bootstrapped, login fails, job fails). Now logs
::notice:: and exits 0.
## Wave E — All 6 open Dependabot bumps folded in
* PR #104 (actions group): codeql-action/upload-sarif, markdownlint-
cli2-action v20->v23, lychee-action SHA refresh.
* PR #105: Microsoft.Identity.Web 4.8.0 -> 4.9.0.
* PR #106: Azure.Monitor.OpenTelemetry.Exporter 1.7.0 -> 1.8.0.
* PR #107: Meziantou.Analyzer + SonarAnalyzer.CSharp.
* PR #108: Microsoft.NET.Test.Sdk 18.4.0 -> 18.5.1.
* PR #109: Scalar.AspNetCore 2.14.4 -> 2.14.9.
* markdownlint v23 introduced new MD060 (table-column-style)
enforcement; realigned 5 doc tables; disabled MD060 in
.markdownlint.yaml because emoji-width breaks 'aligned' style
detection (real-world tables look fine in render).
## Wave F — GitHub issues
* .github/dependabot.yml — added 'Microsoft.Testing.*' to the test
group so MTP packages bump together (closes #80 follow-up).
* #80, #83, #85 — closed/updated separately (see PR description).
## Verification
* dotnet test EntraAuthPatterns.slnx -> 93/93 (was 72; +21 new).
* az bicep build on every .bicep + .bicepparam clean.
* actionlint .github/workflows/*.yml clean.
* shellcheck scripts/*.sh clean.
* markdownlint-cli2 (CI version) clean.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced May 2, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Audit-driven cleanup folding in all 6 open Dependabot PRs + closing the 4 high-impact findings from this session's parallel audit run + addressing all 3 open issues.
Headline change: 3 docs prescribed a forbidden pattern
docs/validation.md:54,57,156anddocs/decision-trees.md:76,82showed[Authorize(Roles = "...")]— explicitly forbidden byDOCTRINE.md(added in PR #110): silently no-ops on Microsoft.Identity.Web JWT schemes (MapInboundClaims=falselooks forClaimTypes.Role, Entra emits short-nameroles). Readers copying these snippets would deploy a privilege-escalation bug.Six Dependabot PRs absorbed
Each as part of this single squash, with their breakage transitively fixed:
markdownlint-cli2-action v20→v23introduced new MD060 enforcement. Realigned 5 doc tables + disabled MD060 in.markdownlint.yaml(emoji-width breaks 'aligned' detection).All 4 audit waves
EntraAuthForwardedHeadersExtensionsTests(X-Forwarded-* + ForwardLimit cap)http://schemas.microsoft.com/identity/claims/{tid,oid,appid}) + Retry-After RFC 6585 + 2-user partition isolationMetadataName.RetryAfterisn't always populated; falls back toopts.Windowso 429 responses always carry the header.log-analytics.bicepretentionInDaysparameterized (prod=90d audit floor, others=30d)modules/budget.bicep(Microsoft.Consumption/budgets, Forecasted+Actual at 80/100%, prod $50/mo, others $10/mo)drift-detection.ymlworkflow (weeklyaz deployment group what-if, matrix [ci, ppe],::warning::on drift)transport='https'for prodDownstreamApiClient.ProbeAsyncConfigureAwait(false)(library code)cd-cleanup.ymlPPE auth failure fixed via pre-check beforeazure/loginIssues closed
wontfix. AddedMicrosoft.Testing.*to dependabot test-group so future bumps land grouped.IHostBuilder + TestServertest covers the same matrix).Verification
dotnet test EntraAuthPatterns.slnxaz bicep build(every.bicep+.bicepparam)actionlintshellcheckmarkdownlint-cli2(CI version)