docs: fix missing Pragma: no-cache and clarify silent-200 behaviours#71
Merged
veverkap merged 2 commits intoApr 26, 2026
Merged
Conversation
- APIKeyHandler.Create: add missing Pragma: no-cache to response table (code sets both Cache-Control: no-store and Pragma: no-cache, but only Cache-Control was documented) - MagicLinkHandler.RequestMagicLink: add note that email delivery failures are logged but still return HTTP 200 - EmailVerificationHandler.SendVerification: add note clarifying that ALL internal failures (store errors, email delivery) return HTTP 200, not only the user-not-found case; this intentional anti-enumeration design was undocumented - PasswordResetHandler.RequestReset: explicitly state that the response is HTTP 200 even when SendResetEmail fails Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates README documentation to accurately reflect existing handler behaviors around anti-caching headers and intentionally “silent 200” responses (anti-enumeration / non-disclosure of email delivery failures).
Changes:
- Document
Pragma: no-cachealongsideCache-Control: no-storeforAPIKeyHandler.Createresponses. - Add explicit notes that certain email-delivery and internal failures intentionally still return HTTP 200 for
MagicLinkHandler.RequestMagicLink,EmailVerificationHandler.SendVerification, andPasswordResetHandler.RequestReset.
Show a summary per file
| File | Description |
|---|---|
| README.md | Aligns documented response headers and “silent 200” semantics with current handler implementations. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
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
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.
Summary
Four documentation gaps found by cross-referencing the README against the handler source files.
Changes
1.
APIKeyHandler.Create— add missingPragma: no-cacheThe code sets both
Cache-Control: no-storeandPragma: no-cacheon theCreate(201) response, but the README response table only mentionedCache-Control: no-store. Updated to match the code, consistent with how other handlers (AuthHandler, MagicLinkHandler, TOTPHandler) document the same header pair.Files:
handler/apikey.go:105-1062.
MagicLinkHandler.RequestMagicLink— document email-delivery-failure → 200When
Senderis non-nil but returns an error, the handler logs the failure and continues to return HTTP 200 (see the// Do not surface delivery failurescomment inhandler/magiclink.go). This was undocumented. Added a blockquote note after the error table.Files:
handler/magiclink.go:84-873.
EmailVerificationHandler.SendVerification— clarify all internal failures return 200The section said the handler "prevents enumeration" for the user-not-found case, but didn't make clear that all internal failures (store errors, token generation failures, email delivery failures) also return 200. Added a blockquote note after the error table explaining this intentional anti-enumeration design.
Files:
handler/email_verification.go:62-1024.
PasswordResetHandler.RequestReset— state that email-send failure returns 200The existing prose described the orphaned-token cleanup on email failure but never explicitly said the response was still HTTP 200. Added "and still returns HTTP 200" to make the control flow unambiguous.
Files:
handler/password_reset.go:96-107Testing
Documentation-only changes. No behaviour was altered. The Go toolchain required by the repository (
go1.26.1) is not available in the CI sandbox, so automated tests could not be run; all changes are verified by reading the source code directly.Warning
The following domain was blocked by the firewall during workflow execution:
proxy.golang.orgTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.
Greptile Summary
This PR adds four documentation corrections to README.md, each cross-verified against the corresponding handler source. All changes accurately reflect the code:
Pragma: no-cacheis confirmed atapikey.go:98, the MagicLink silent-200 path is confirmed atmagiclink.go:77-84, all fiveSendVerificationanti-enumeration 200 paths (including the previously-missing already-verified case) are confirmed atemail_verification.go:62-101, and the PasswordReset email-failure 200 return is confirmed atpassword_reset.go:96-110.Confidence Score: 5/5
Safe to merge — documentation-only changes that accurately reflect the source code.
All four documentation changes were verified line-by-line against the corresponding handler source files. No behaviour was altered. The previously-flagged omission of the already-verified 200 path in SendVerification has been correctly addressed in this PR.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[RequestMagicLink] --> B{Sender == nil?} B -- Yes --> C[503 Service Unavailable] B -- No --> D{Sender returns error?} D -- Yes --> E[Log failure, continue] D -- No --> F[200 OK] E --> F G[SendVerification] --> H{email empty?} H -- Yes --> I[400 Bad Request] H -- No --> J{User found?} J -- No --> K[200 OK — silent] J -- Yes --> L{Already verified?} L -- Yes --> K L -- No --> M{Token generation OK?} M -- No --> K M -- Yes --> N{Store OK?} N -- No --> K N -- Yes --> O{SendEmail != nil?} O -- No --> P[200 OK] O -- Yes --> Q{Delivery error?} Q -- Yes --> R[Log, continue] Q -- No --> P R --> P S[RequestReset] --> T{User found?} T -- No --> U[200 OK — silent] T -- Yes --> V{Store token OK?} V -- No --> W[500 Internal Server Error] V -- Yes --> X{SendResetEmail != nil?} X -- No --> U X -- Yes --> Y{Delivery error?} Y -- Yes --> Z[Attempt token cleanup, 200 OK] Y -- No --> UReviews (2): Last reviewed commit: "docs: add 'already verified' path to Sen..." | Re-trigger Greptile