tidy-security: two seams documented as wired - #223
Merged
Conversation
The client-identity table marked "Operator pre-provisions a client_id" as ✅, citing NewStaticRegistrar and a --client-id flag, and the paragraph under it named that flag as the way out of the document's self-declared biggest spec gap: an AS that implements only 2025-11-25 and does not enable DCR. Neither half holds. There is no --client-id flag on auth login — the full flag set is manual/device/loopback/no-browser/scopes/authorization-endpoint/issuer/ redirect-uri/allow-local/timeout. And while LoginRequest.Registrar is a real seam and NewStaticRegistrar a real implementation, nothing in the tree ever sets that field, so Flow.register always falls through to DCR; NewStaticRegistrar's only caller is its own test. The direction matters. A row marked ✅ is one a reader stops investigating, and this is precisely the row someone reaches for after reading that our only path is the one the spec demoted. Today such a provider has no route through auth login at all, which the table now says. The neighbouring rows were checked the same way: --redirect-uri exists, State.ClientID reuse is real, and CIMD was already marked ❌ correctly. Docs only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Its comment read "Wire it for --no-browser so mode selection takes the manual path through the same code." Following that advice would change the mode that gets chosen. SelectMode reads a non-nil Open as "this host can open a browser", so an always-failing opener selects ModeLoopback and reaches manual only by the downgrade path — after binding a listener and failing to open. What the CLI actually does is leave Open nil, which is the signal SelectMode reads to choose manual outright, and auth.go says so at the call site. So the one instruction the comment gives is the one thing not to do, on a function with no production caller. The comment now says which mechanism is real, why the other is not equivalent, and what the function is still for. While in the package: ran make fuzz FUZZ=FuzzScanAuthParam for 45s over the WWW-Authenticate scanner this slice owns — 5.6M execs, no crashes. Nothing in this commit touches it; the round is the slice's due diligence on a hand-written parser reading untrusted input. Comment only. make ci green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Nightly-tidy slice over the security layer —
internal/guard/spawnguard,guard/netguard,oauthflow,oauthlogin— anddocs/modules/security.md/docs/modules/oauth.md.Both findings are the same shape: oauthflow offers a seam, the document says it is wired,
and nothing wires it. No predicate, failure direction or gate is touched.