docs: package-wide comment sweep

Audit prompted by review feedback on PR #290's docstrings — the security PR
landed several long, rationale-heavy doc comments and a couple of them turned
out factually wrong. A combined Codex + Explore-agent sweep over the rest of
the v1 package surfaced ~20 more drift items in pre-existing comments. This
commit fixes both classes in one pass and trims the rationale prose from the
security-fix docs down to API-contract sized statements.

Net 28 files changed across v1 and v2; the line count drops by ~30 because
most of the change is shortening, not adding. Build green, lint clean, go fix
clean on both modules.

Drift fixes (pre-existing wrong/misleading comments):

  * avatar/gridfs.go — ID doc claimed MD5 sourced from gridfs; in reality
    metadata.hash is the sha1 written at Put time.
  * avatar/bolt.go, avatar/gridfs.go — Put docs and BoltDB type doc claimed
    these layers "resize" the image; they only copy bytes verbatim, resize
    happens in Proxy.resize upstream.
  * avatar/store.go — Migrate doc didn't mention that per-avatar Get/Put/Close
    errors are logged and skipped, and that the returned count is "ids
    attempted", not "ids stored".
  * provider/oauth1.go — initOauth1Handler doc and DEBUG log both said
    "oauth2"; the function and the protocol are OAuth 1.
  * provider/service.go — Handler doc said it "returns auth routes"; it
    dispatches login/callback/logout.
  * provider/telegram.go — processUpdates claimed to return an offset (no
    return value); checkToken doc described an "address or empty string"
    return shape but the signature is (*token.User, error).
  * provider/verify.go — Sender interface doc locked the contract to "send
    emails", contradicting the broader "email, IM, or anything else"
    promise on VerifyHandler at line 23. AuthHandler comment was copy-pasted
    from the direct provider.
  * provider/dev_provider.go — NewDev doc said "for admin user"; it makes
    the dev oauth2 provider; admin role is separate.
  * middleware/user_updater.go — UserUpdFunc adapter doc said the result
    "is a Handler"; it implements UserUpdater.
  * logger/interface.go — Func adapter doc said "Logf calls f(id)"; calls
    f(format, args...).
  * token/jwt.go — SendJWTHeader option said "instead of cookie"; Set sends
    header and cookie. Set doc referenced a "permanent flag" that doesn't
    exist in the signature (controlled by claims.SessionOnly/Handshake). Get
    doc oversimplified the XSRF gate (skipped when DisableXSRF, method in
    XSRFIgnoreMethods, or claims have no user). Update/Validate adapter docs
    said "calls f(id)"; actually f(claims) and f(token, claims).
  * token/user.go — HashID inline "or empty" was wrong; an empty val never
    matches reValidSha.
  * auth.go — BasicAuthChecker doc grammar was broken and understated
    behavior; AdminPasswd is bypassed entirely when a checker is set, not
    "ignored". "peak dev provider" typo for "peek". AvatarProxy doc was a
    sentence fragment.

Trims (rationale prose I wrote in #290; reduced to API contract):

  * avatar/avatar.go — maxAvatarFetchSize, maxAvatarPixels, Proxy.Put,
    Proxy.Handler godoc, Handler inline sniff comment, Proxy.resize,
    safeImgContentType — each cut to what callers/maintainers need to know
    about behavior, with rationale moved to the commit history.
  * auth.go — withSecurityHeaders dropped its bullet-list explanation of
    standard HTTP headers; the CONSUMER NOTE is preserved (real footgun)
    but tightened.

All changes mirrored across v1 and v2.

Author: paskal

auth.go

@@ -81,7 +81,7 @@ type Opts struct {
 	UseGravatar       bool         // for email based auth (verified provider) use gravatar service
 
 	AdminPasswd      string                   // if presented, allows basic auth with user admin and given password
-	BasicAuthChecker middleware.BasicAuthFunc // user custom checker for basic auth, if one defined then "AdminPasswd" will ignored
+	BasicAuthChecker middleware.BasicAuthFunc // custom checker for basic auth; when set, AdminPasswd is bypassed entirely
 	AudienceReader   token.Audience           // list of allowed aud values, default (empty) allows any
 	AudSecrets       bool                     // allow multiple secrets (secret per aud)
 	Logger           logger.L                 // logger interface, default is no logging at all
@@ -239,30 +239,17 @@ func (s *Service) Handlers() (authHandler, avatarHandler http.Handler) {
 	return withSecurityHeaders(http.HandlerFunc(ah)), withSecurityHeaders(http.HandlerFunc(s.avatarProxy.Handler))
 }
 
-// withSecurityHeaders wraps an auth response handler to apply strict CSP and nosniff
-// on every response. The go-pkgz/auth package's own response surface is JSON-only
-// for auth routes and images for the avatar route — no built-in HTML rendering
-// anywhere — so this CSP is unconditionally safe and gives the auth origin
-// defense-in-depth against any future trust-boundary regression that might emit a
-// renderable body.
+// withSecurityHeaders wraps a handler to set Content-Security-Policy
+// "default-src 'none'; sandbox; frame-ancestors 'none'" and X-Content-Type-Options
+// "nosniff" on every response. Safe to apply unconditionally because go-pkgz/auth
+// only emits JSON (auth routes) and images (avatar route).
 //
-//   - Content-Security-Policy: default-src 'none'; sandbox — blocks inline scripts
-//     and event handlers even if a body is ever served as HTML by mistake; the
-//     sandbox directive additionally isolates any rendered document from this origin.
-//   - X-Content-Type-Options: nosniff — prevents browsers from MIME-overriding the
-//     declared Content-Type to a more dangerous one.
-//
-// The avatar Handler additionally sets Content-Disposition: inline; filename="avatar"
-// inside itself, so direct callers (tests, custom mounts) still get the full header
-// set without going through this wrapper.
-//
-// CONSUMER NOTE: custom providers added via Service.AddCustomHandler / AddProvider
-// are also wrapped. If a custom provider renders HTML (login forms, JS-based flows,
-// the dev_provider's login page, etc.), the strict CSP will block inline scripts and
-// event handlers on those pages. Such providers should either (a) override the CSP
-// for their own response by calling w.Header().Set("Content-Security-Policy", ...)
-// before writing — Set replaces the wrapper's value — or (b) move any required
-// scripts/styles to external files served from 'self'.
+// CONSUMER NOTE: custom providers registered through AddCustomHandler / AddProvider
+// are wrapped too. A provider that renders HTML (login form, JS flow, custom server
+// login page, etc.) will be blocked by this CSP — default-src 'none' plus sandbox
+// stop scripts, styles, forms, and images even when served from 'self'. Such
+// providers must override the CSP on their own response (call w.Header().Set before
+// writing — Set replaces the wrapper's value) and relax only the directives needed.
 func withSecurityHeaders(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Security-Policy", "default-src 'none'; sandbox; frame-ancestors 'none'")
@@ -516,7 +503,7 @@ func (s *Service) AddCustomHandler(p provider.Provider) {
 
 // DevAuth makes dev oauth2 server, for testing and development only!
 func (s *Service) DevAuth() (*provider.DevAuthServer, error) {
-	p, err := s.Provider("dev") // peak dev provider
+	p, err := s.Provider("dev") // peek dev provider
 	if err != nil {
 		return nil, fmt.Errorf("dev provider not registered: %w", err)
 	}
@@ -544,7 +531,7 @@ func (s *Service) TokenService() *token.Service {
 	return s.jwtService
 }
 
-// AvatarProxy returns stored in service
+// AvatarProxy returns the avatar.Proxy configured on the service, or nil if no AvatarStore was set.
 func (s *Service) AvatarProxy() *avatar.Proxy {
 	return s.avatarProxy
 }

avatar/avatar.go

@@ -28,17 +28,12 @@ import (
 // http.sniffLen is 512 bytes which is how much we need to read to detect content type
 const sniffLen = 512
 
-// maxAvatarFetchSize bounds the bytes read from a remote avatar URL. 10 MiB is
-// generous for any reasonable avatar (Telegram caps photo at 5 MiB; Gravatar is
-// much smaller); the cap protects Proxy.Put against an upstream sending an
-// unbounded body that would exhaust process memory inside resize.
+// maxAvatarFetchSize caps the byte length of any avatar accepted by load,
+// PutContent, or resize — i.e. the upper bound on bytes that ever reach Store.Put.
 const maxAvatarFetchSize = 10 << 20
 
-// maxAvatarPixels caps the declared pixel count of an avatar before any raster
-// decode is allowed. Without this, a tiny compressed "decompression bomb" image
-// declaring e.g. 65535x65535 px would force image.Decode to allocate gigabytes
-// of pixel memory and OOM the auth service on a single login attempt. 16 MP
-// covers any realistic avatar (~4096x4096) while keeping peak allocation bounded.
+// maxAvatarPixels caps cfg.Width*cfg.Height (from image.DecodeConfig) before any
+// full image.Decode runs. Defends against decompression-bomb inputs.
 const maxAvatarPixels = 16 * 1024 * 1024
 
 // Proxy provides http handler for avatars from avatar.Store
@@ -51,7 +46,11 @@ type Proxy struct {
 	ResizeLimit int
 }
 
-// Put stores retrieved avatar to avatar.Store. Gets image from user info. Returns proxied url
+// Put fetches u.Picture, validates it via resize, stores it via Store, and returns
+// the proxied avatar URL. If u.Picture is empty, the fetch fails, or the body is
+// not a recognized image within the size and dimension limits, Put silently stores
+// a generated identicon and returns its URL — the caller is not told upstream was
+// rejected.
 func (p *Proxy) Put(u token.User, client *http.Client) (avatarURL string, err error) {
 
 	genIdenticon := func(userID string) (avatarURL string, err error) {
@@ -168,7 +167,9 @@ func (p *Proxy) load(url string, client *http.Client) ([]byte, error) {
 	return body, nil
 }
 
-// Handler returns token routes for given provider
+// Handler serves stored avatar content by avatar id. GET only; rejects invalid ids
+// (403) and stored bytes that fail the safeImgContentType sniff (415). Layered
+// defense headers are set on every response via setAvatarDefenseHeaders.
 func (p *Proxy) Handler(w http.ResponseWriter, r *http.Request) {
 	setAvatarDefenseHeaders(w)
 
@@ -195,11 +196,9 @@ func (p *Proxy) Handler(w http.ResponseWriter, r *http.Request) {
 		}
 	}()
 
-	// io.ReadFull keeps reading until the buffer is full or EOF, so a Store
-	// implementation that returns a buffered reader with a small first-Read size
-	// won't cause DetectContentType to misclassify a real image. Short bodies
-	// (avatars under sniffLen) return ErrUnexpectedEOF — that's expected, we sniff
-	// what we got.
+	// ReadFull (not Read) so a Store backend that hands back a short first Read
+	// doesn't truncate the sniff window. Short bodies are signaled by
+	// ErrUnexpectedEOF and treated like EOF.
 	buf := make([]byte, sniffLen)
 	n, err := io.ReadFull(avReader, buf)
 	if err != nil && err != io.EOF && err != io.ErrUnexpectedEOF {
@@ -208,11 +207,9 @@ func (p *Proxy) Handler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// validate the bytes really are an image before declaring a content type. Even
-	// though Put() now refuses to store non-image content, this catches stores poisoned
-	// before the fix and any future regression — never trust the bytes at the store
-	// boundary alone, validate again at serve time. An empty body (e.g. NoOp store)
-	// is treated as a benign no-content case: nothing to render, no XSS surface.
+	// sniff buf[:n] against the safeImgContentType allowlist. This is not a full
+	// image decode; it catches stores poisoned before Put validated. An empty body
+	// (e.g. NoOp store) is treated as benign — nothing to render.
 	var contentType string
 	if n > 0 {
 		var ctErr error
@@ -248,20 +245,12 @@ func (p *Proxy) Handler(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-// resize validates that the input is a real image and, if needed, re-encodes it to
-// fit within "limit" px on the larger side preserving aspect ratio. Returns nil for
-// non-image content or for declared dimensions exceeding maxAvatarPixels so attacker
-// payloads (HTML/SVG/decompression bombs) never reach the store. With limit <= 0 or
-// when the image already fits, the original bytes are returned verbatim so animated
-// GIFs and other multi-frame formats round-trip without being flattened to one frame.
-//
-// Validation uses image.DecodeConfig (cheap — declares dimensions, allocates nothing)
-// before any full image.Decode, so a 100 KB compressed image declaring 65535x65535 px
-// is rejected without ever materializing the raster.
-//
-// Callers (load, PutContent, identicon generation) must ensure body is bounded by
-// maxAvatarFetchSize before calling; resize trusts the size invariant rather than
-// re-buffering. An empty body or a body over the cap is refused defensively.
+// resize checks body via image.DecodeConfig (format + dimensions only, no raster
+// allocation), then either returns the original bytes verbatim (limit <= 0, or
+// dimensions already within limit) or fully decodes and re-encodes to PNG fitting
+// within limit px on the larger side. Returns nil for non-image input or for
+// dimensions exceeding maxAvatarPixels. Caller must ensure body is within
+// maxAvatarFetchSize; a body over the cap is also refused defensively.
 func (p *Proxy) resize(body []byte, limit int) io.Reader {
 	if len(body) == 0 || int64(len(body)) > maxAvatarFetchSize {
 		p.Logf("[WARN] avatar resize(): refusing body of size %d (cap %d)", len(body), maxAvatarFetchSize)
@@ -315,13 +304,9 @@ func (p *Proxy) resize(body []byte, limit int) io.Reader {
 	return &out
 }
 
-// safeImgContentType returns the sniffed content type if the bytes look like a safe
-// raster image format. The set is an explicit allowlist (PNG, JPEG, GIF, WebP, BMP,
-// ICO) — no HasPrefix("image/") catch-all — so future scriptable image/* MIME types
-// added by http.DetectContentType cannot silently pass. Returns an error otherwise.
-// image/svg+xml is excluded because SVG can execute scripts when navigated to top
-// level; image/* coverage of icon files uses both spellings http.DetectContentType
-// is known to return.
+// safeImgContentType returns the sniffed Content-Type for img if it is one of the
+// allow-listed raster image formats (PNG, JPEG, GIF, WebP, BMP, ICO under both
+// MIME spellings http.DetectContentType uses), else an error. SVG is excluded.
 func safeImgContentType(img []byte) (string, error) {
 	ct := http.DetectContentType(img)
 	base := ct

avatar/bolt.go

@@ -11,7 +11,7 @@ import (
 
 // BoltDB implements avatar store with bolt
 // using separate db (file) with "avatars" bucket to keep image bin and "metas" bucket
-// to keep sha1 of picture. avatarID (base file name) used as a key for both.
+// to keep a sha1 fingerprint of the stored bytes. avatarID (base file name) used as a key for both.
 type BoltDB struct {
 	fileName string // full path to boltdb
 	db       *bolt.DB
@@ -41,7 +41,9 @@ func NewBoltDB(fileName string, options bolt.Options) (*BoltDB, error) {
 	return &BoltDB{db: db, fileName: fileName}, nil
 }
 
-// Put avatar to bolt, key by avatarID. Trying to resize image and lso calculates sha1 of the file for ID func
+// Put stores avatar bytes read from reader in the "avatars" bucket and a sha1 of the
+// same bytes in the "metas" bucket, both keyed by the encoded userID with .image suffix.
+// Resizing happens upstream in Proxy.resize; this layer only writes what it is given.
 func (b *BoltDB) Put(userID string, reader io.Reader) (avatar string, err error) {
 	id := encodeID(userID)
 

avatar/gridfs.go

@@ -27,7 +27,9 @@ type GridFS struct {
 	timeout    time.Duration
 }
 
-// Put avatar to gridfs object, try to resize
+// Put stores avatar bytes read from reader in GridFS, keyed by the encoded userID
+// with .image suffix, and records the sha1 of those bytes in the file's metadata.
+// Resizing happens upstream in Proxy.resize; this layer only writes what it is given.
 func (gf *GridFS) Put(userID string, reader io.Reader) (avatar string, err error) {
 	id := encodeID(userID)
 	bucket, err := gridfs.NewBucket(gf.db, &options.BucketOptions{Name: &gf.bucketName})
@@ -59,7 +61,8 @@ func (gf *GridFS) Get(avatar string) (reader io.ReadCloser, size int, err error)
 	return io.NopCloser(buf), int(sz), nil
 }
 
-// ID returns a fingerprint of the avatar content. Uses MD5 because gridfs provides it directly
+// ID returns the sha1 fingerprint of the avatar content stored in the GridFS file's
+// metadata at Put time, or an encoded fallback id when the file or hash is missing.
 func (gf *GridFS) ID(avatar string) (id string) {
 
 	finfo := struct {

avatar/store.go

@@ -70,7 +70,10 @@ func NewStore(uri string) (Store, error) {
 	return nil, fmt.Errorf("can't parse store url %s", uri)
 }
 
-// Migrate avatars between stores
+// Migrate copies avatars from src to dst, returning the number of ids enumerated
+// from src and the first List error if any. Per-avatar Get/Put/Close failures are
+// logged with [WARN] and skipped — they do not abort the migration or surface in
+// the returned error, so the returned count is "ids attempted", not "ids stored".
 func Migrate(dst, src Store) (int, error) {
 	ids, err := src.List()
 	if err != nil {

logger/interface.go

@@ -12,7 +12,7 @@ type L interface {
 // Func type is an adapter to allow the use of ordinary functions as Logger.
 type Func func(format string, args ...any)
 
-// Logf calls f(id)
+// Logf calls f(format, args...).
 func (f Func) Logf(format string, args ...any) { f(format, args...) }
 
 // NoOp logger

middleware/user_updater.go

@@ -12,7 +12,7 @@ type UserUpdater interface {
 }
 
 // UserUpdFunc type is an adapter to allow the use of ordinary functions as UserUpdater. If f is a function
-// with the appropriate signature, UserUpdFunc(f) is a Handler that calls f.
+// with the appropriate signature, UserUpdFunc(f) is a UserUpdater that calls f.
 type UserUpdFunc func(user token.User) token.User
 
 // Update calls f(user)

provider/dev_provider.go

@@ -167,7 +167,7 @@ func (d *DevAuthServer) Shutdown() {
 	d.lock.Unlock()
 }
 
-// NewDev makes dev oauth2 provider for admin user
+// NewDev makes a dev oauth2 provider intended for local development only.
 func NewDev(p Params) Oauth2Handler {
 	if p.Port == 0 {
 		p.Port = defDevAuthPort

provider/oauth1.go

@@ -190,7 +190,7 @@ func (h Oauth1Handler) makeRedirURL(path string) string {
 	return strings.TrimSuffix(h.URL, "/") + strings.TrimSuffix(newPath, "/") + urlCallbackSuffix
 }
 
-// initOauth2Handler makes oauth1 handler for given provider
+// initOauth1Handler makes oauth1 handler for given provider
 func initOauth1Handler(p Params, service Oauth1Handler) Oauth1Handler {
 	if p.L == nil {
 		p.L = logger.NoOp
@@ -200,7 +200,7 @@ func initOauth1Handler(p Params, service Oauth1Handler) Oauth1Handler {
 	service.conf.ConsumerKey = p.Cid
 	service.conf.ConsumerSecret = p.Csecret
 
-	p.Logf("[DEBUG] created %s oauth2, id=%s, redir=%s, endpoint=%s",
+	p.Logf("[DEBUG] created %s oauth1, id=%s, redir=%s, endpoint=%s",
 		service.name, service.Cid, service.makeRedirURL("/{route}/"+service.name+"/"), service.conf.Endpoint)
 	return service
 }

provider/service.go

@@ -63,7 +63,7 @@ type Provider interface {
 	LogoutHandler(w http.ResponseWriter, r *http.Request)
 }
 
-// Handler returns auth routes for given provider
+// Handler dispatches login, callback, and logout requests to the underlying provider.
 func (p Service) Handler(w http.ResponseWriter, r *http.Request) {
 
 	if r.Method != http.MethodGet && r.Method != http.MethodPost {