Skip to content

security(audit): the auth audit trail records a client-chosen IP from raw X-Forwarded-For #877

Description

@alex-dembele

Problem

AuditService.LogFiber (backend/internal/auth/audit.go) records the client IP like this:

ip := c.IP()
if xff := c.Get("X-Forwarded-For"); xff != "" {
    ip = xff
}

c.IP() already resolves the forwarded header, and only when the peer is a configured trusted proxy (middleware.TrustedProxies, EnableTrustedProxyCheck). The raw read overrides that. Any client can write any value it likes into the ip column of auth_audit_logs for every login, refresh, MFA and SSO event, so an attacker can hide their address or blame someone else's. The rate limiter fixed the same pattern under audit finding F-04; the audit trail was left behind.

Also, the column is varchar(45), so a long multi-hop header (a, b, c) can fail the insert. Audit writes are best-effort (_ =), so the event is then lost without a trace.

Acceptance criteria

  1. LogFiber records c.IP() only.
  2. Test: a request from an untrusted peer that sends X-Forwarded-For: 203.0.113.9 is audited with the peer address, not 203.0.113.9.
  3. Test: behind a trusted proxy, the forwarded client address is recorded (this is c.IP()'s own behaviour; the test proves it is not lost).
  4. No other X-Forwarded-For read is left in backend/internal/auth. A grep is pasted in the PR.

Definition of Done

  • go build ./... && go vet ./... && go test ./... -race green, output pasted.
  • Progress comment in the CLAUDE.md format; PR with Closes.

Activity

  1. added this to the trust-v1 milestone on Oct 2, 2026
  2. added
    area:securityCybersecurity and threat intelligence
    type:securitySecurity defect or hardening
    status:readyMeets the ready definition
    tier:0-trustTrust: security, isolation, evidence integrity
    and removed
    status:readyMeets the ready definition
    on Oct 2, 2026
  3. alex-dembele commented on Oct 2, 2026

    @alex-dembele
    MemberAuthor

    claude — 2026-10-02

    Done — backend/internal/auth/audit.go: LogFiber records c.IP() only. backend/internal/middleware/auth.go: same fix in the unmounted MFARateLimit/OAuthRateLimit. Test: backend/internal/auth/audit_ip_test.go. PR #883.
    Verified — the new test fails on master and passes here; full backend gate green (80 ok, 0 FAIL), run with #882 applied temporarily because master doesn't build (#881); grep for raw X-Forwarded-For reads → 0.
    Criteria — 1 ✅ · 2 ✅ · 3 ✅ · 4 ✅
    Next — owner merges #882 (P0), then this PR.
    Blocked on — #881 / PR #882 for a green master build

  4. added a commit that references this issue on Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions