Skip to content

sec(api/auth): low-severity authorization-surface hardening from the 2026-09-02 audit #310

Closed

Description

@cristim

Summary

Three low-severity findings on the backend authorization surface (internal/api, internal/auth), grouped because they are fixed in one sitting by someone already in the middleware, registration and account-scope code. None is exploitable on its own today; each is a contract the code states and then does not enforce.

Location

All at 3c0f8ac94048a2c36fce5ccddee54e6c4849a5cd:

  • internal/api/middleware.go:274 (A02-015)
  • internal/api/handler_registrations.go:156 (A02-020)
  • internal/auth/types.go:207 and internal/auth/account_scope.go:112 (A03-015)

Items

  • A02-015: PATCH is a registered mutating route but is excluded from CSRF validation and the CORS method list. router.go:150 serves PATCH /api/plans/{id}; requiresCSRFValidation returns false for any method other than POST/PUT/DELETE, so the CSRF header is never checked on PATCH, and buildResponseHeaders (handler.go:720) advertises only GET, POST, PUT, DELETE, OPTIONS, so a cross-origin deployment's preflight for PATCH fails. Exploitability is low because the session credential is a bearer header rather than a cookie, but the middleware's own contract ("state-changing requests need CSRF protection") is not met for this verb and nothing tests it.

    if method != "POST" && method != "PUT" && method != "DELETE" {
        return false
    }

    Fix: add PATCH to the CSRF method set and to Access-Control-Allow-Methods, with a middleware test for PATCH /api/plans/x. Related: test(api): add a class-level CSRF guard for AuthPublic money routes, and remove the vacuous test with its dead code #193 (class-level CSRF guard for AuthPublic money routes).

  • A02-020: registration list and detail expose reference_token, which reject deliberately strips. config.AccountRegistration.ReferenceToken is tagged json:"reference_token" (internal/config/types.go:1078) and registrationColumns selects it, so listRegistrations and getRegistration return the registrant's private status token to every admin and into any log or export of the response, while rejectRegistration (handler_registrations.go:373-381) builds a filtered map precisely so the token is "not exposed to admin". The token is the only credential for GET /api/register/{token}.

    regs, err := h.config.ListAccountRegistrations(ctx, filter)
    ...
    return regs, nil

    Fix: tag ReferenceToken with json:"-" and return it only from submitRegistration's explicit map.

  • A03-015: account scope matches on display name, so two accounts sharing a name share scope. MatchesAccount and AccountScope.Allows treat a scope entry equal to accountName as a match. Display names are free text and not unique, production callers pass real display names (handler_ri_exchange.go:431, :809; handler_ladder.go:60; handler_analytics.go:245; handler_history.go:1015), and the group write path compares allowed_accounts entries only against the actor's own scope (group_ceiling.go:322-338), never against the account table. A group scoped to ["prod"] by name gains access to every account later registered with the display name "prod", and renaming an account silently changes who can see it. Note chore(api): medium/low findings from the 2026-07-28 full review #100 has a checklist item that assumes name-based entries are a supported shape and asks two bespoke matchers to honour them; the two directions should be decided together.

    for _, a := range allowed {
        if a == accountID {
            return true
        }
        if accountName != "" && a == accountName {
            return true
        }
    }

    Fix: match on account ID only; migrate existing name-based entries to IDs once and drop the name parameter.

Suggested fix

Three small, independent changes; each item carries its own. No new abstractions needed.


Found by the 2026-09-02 codebase audit, finding A02-015, A02-020, A03-015, reported by one reviewer and independently confirmed by a second. Full report: docs/audits/codebase-audit-2026-09-02.md.

Activity

  1. IAmJSD commented on Oct 23, 2026

    @IAmJSD
    Collaborator

    Claiming: working on this

  2. self-assigned this
    on Oct 23, 2026
  3. IAmJSD commented on Oct 26, 2026

    @IAmJSD
    Collaborator

    Closing as done.

    Checked against main at 3bfc2fe. Closed by IAmJSD's agent at IAmJSD's request (acting owner) as part of a sweep of stale assigned issues; reopen if anything is missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions