Skip to content

[weekly-grumpy-reviewer] Add json:"-" to sensitive fields in auth/types.go (PasswordHash, KeyHash, RefreshTokenHash, TOTPSecret.Secret) #586

Description

@github-actions

Summary

auth/types.go defines several domain types — User, APIKey, Session, TOTPSecret — that contain sensitive cryptographic material. None of these fields have json:"-" tags, meaning any consumer that serialises these types directly (logging, caching, admin endpoints, debugging middleware) will silently leak:

Type Field Risk
User PasswordHash bcrypt hash — offline cracking possible
APIKey KeyHash SHA-256 of live API key — lookup-table attack
Session RefreshTokenHash SHA-256 of active refresh token — session hijack
TOTPSecret Secret base32 TOTP seed — full MFA bypass

Reproduction

u := &auth.User{PasswordHash: "$2a$12$..."}
json.NewEncoder(os.Stdout).Encode(u)
// Output: {"id":"","name":"","email":"","password_hash":"$2a$12$...","oidc_subject":null,...}

No error, no warning — the hash is just... there.

Fix

Add json:"-" to each sensitive field:

// auth/types.go

type User struct {
    ID            string
    Name          string
    Email         string
    PasswordHash  string  `json:"-"`
    OIDCSubject   *string `json:"-"`
    // ...
}

type APIKey struct {
    // ...
    KeyHash   string `json:"-"`
    // ...
}

type Session struct {
    // ...
    RefreshTokenHash string `json:"-"`
    // ...
}

type TOTPSecret struct {
    // ...
    Secret string `json:"-"` // base32-encoded; must never appear in JSON
}

PasskeyChallenge.SessionData (WebAuthn session blob) and PasskeyCredential.CredentialData (raw CBOR) should also be reviewed — both contain authentication ceremony state that should not be serialised into general-purpose JSON responses.

Notes

  • The handler package already defines DTOs (UserDTO, APIKeyDTO, SessionDTO, PasskeyCredentialDTO) that strip sensitive fields before HTTP responses. The json tags are a defence-in-depth measure for the raw domain types that consumers work with directly.
  • This is a non-breaking change for any consumer that already uses the DTO layer correctly. Consumers that were accidentally serialising raw types will start getting null/omitted values — which is the desired behaviour.

Identified by the weekly automated code review.

Generated by weekly Grumpy Reviewer · 617.4 AIC · ⌖ 20.9 AIC · ⊞ 28.2K ·

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions