Skip to content

DECIDED (delete): the admin IP column and burst panel can never populate — no IP source exists and the architecture has nowhere to observe one #839

Description

@TortoiseWolfe

The admin console ships an IP-address feature that cannot ever populate, because nothing writes an IP anywhere.

Split out of #609, which closed as a stale premise: its motivating "attack volume" turned out to be our own E2E suite (#608). This is the part of it that is a real defect rather than a security question.

The dead surface

where what it does why it can't work
AdminAuditTrail.tsx:132-137 renders an ip_address column always 'N/A'
AdminAuditTrail.tsx:79-91 groups rows into bursts keyed on ip_address the panel can never fill
rate-limit-check.ts:52,58 accepts ipAddress?, forwards p_ip_address every caller passes only an email — checkRateLimit(email, 'sign_up')
admin-audit-analytics, ip_bursts exist in the live DB not in the monolithic migration — schema drift

The root cause is one line: log_auth_event (monolithic migration :1117) has no p_ip_address parameter, so no row can carry one.

The decision this needs

Capture, or delete? They are opposite directions and only the owner can pick:

Either way the admin-audit-analytics / ip_bursts drift should be reconciled with the monolithic migration, since prod currently differs from the file that is supposed to be able to rebuild it.

Refs #609, #608, #442.


Measured 2026-08-22: the panel populates, but only from fake data

Applied seed-admin-demo.sql to a local stack and queried. The picture is sharper than "nothing
writes an IP":

source writes ip_address?
the app (log_auth_event) no — the function has no p_ip_address parameter (migration:1117, confirmed live: log_auth_event(p_event_type text, p_user_id uuid, p_event_data jsonb, p_success boolean, p_error_message text, p_user_agent text))
seed-admin-demo.sql:192 yes — ~55 events with 10.0.0.x addresses and a deliberate burst (192.168.99.99, 7 failures). Its own header says "including a burst".

Live counts on that stack: 52 rows, 39 with an IP — every one of them seeded. The IP-less rows
are the real ones, written by the app's sign-up form.

So the burst panel is not merely empty; it is demoable and unreachable. It renders correctly for
anyone who has applied the demo seed, and can never populate from real traffic. That is worse than a
blank panel for the decision at hand, because a maintainer who seeds a dev database sees a working
feature.

eventsInBurst (AdminAuditTrail.tsx:81-91) filters e.ip_address === b.ip_address, so burst
membership is keyed on the field nothing real ever sets. The IP Address column renders
|| 'N/A' (:132-140).

Bearing on the decision

Either direction should be taken deliberately: this is currently the only admin surface whose
correctness is demonstrated exclusively by fixture data.

Related, and not blocked on this decision

#914 (admin E2E specs) has four tests whose only assertions sit inside if (burstCount > 0) /
if (anomalyHeading.isVisible()) guards. They are satisfiable because of the seeded burst — so
if the burst feature is deleted, those four tests go back to asserting nothing and become
zero-assertion failures under ZERO_ASSERTION_GATE_MODE: block. Delete and #914 must be
sequenced
, not landed independently.


DECISION 2026-08-22: DELETE.

Delegated to me and taken. The deciding evidence is that capture is not cheap and not honest here,
and the measurements below are what settled it — not a preference.

Why capture is not available

1. There is no upstream IP source. Measured, not assumed.

source rows with a real IP
auth.audit_log_entries — production 0 0
auth.audit_log_entries — local stack 1622 0 (all empty strings)
auth_audit_logs — production 7440 0

Supabase's own GoTrue audit log is the obvious place to look, and it is empty in production. Locally
it has rows and every ip_address is ''. (Careless note: a first pass read count(ip_address) as
"has an IP" — an empty string is non-NULL and counts. The corrected query is
count(nullif(ip_address,'')).)

2. GoTrue records no failure events at all, so even a populated audit log could not feed burst
detection. Actions present locally: login 800, user_signedup 278, user_deleted 273, logout
179, user_modified 46, token_revoked 23, token_refreshed 23. Burst detection is entirely about
failed attempts; the one signal it needs is the one GoTrue does not emit.

3. The architecture has no place to observe an IP. This is a static export on GitHub Pages —
there is no server. Auth goes browser → Supabase directly. So the options are:

  • browser self-reports its IP → worthless, and worse than nothing: the field is set by exactly
    the attacker the panel exists to detect. A burst panel keyed on an attacker-controlled value is
    security theatre that can actively mislead.
  • an edge function on the auth critical path → a real observation point, and a real change: new
    function, production RPC signature change to log_auth_event, and the auth path becomes dependent
    on it.

4. The privacy cost is uncovered. IPs are personal data, and #442 records that the privacy policy
does not yet disclose Cloudflare/Turnstile. Adding a new PII collection while a known disclosure gap
is open moves the wrong way.

5. The security value is already covered. Turnstile is live on the auth forms (#353), Supabase
Auth rate-limits, and rate_limit_attempts works on email — which is captured and real.

6. This is a template. Every fork inherits it. Shipping a half-built security-analytics surface
that only fixture data can fill is worse for a fork than shipping none.

Production has 7440 real audit rows and the burst panel has never had a single row to group.

Scope — narrower than this ticket assumed

Anomaly Alerts SURVIVES. top_failed_logins is keyed on user_id, not ip_address
(migration:1524). It works on real data today. This ticket's framing implied the whole anomaly
surface was dead; it is not.

Goes:

  • AdminAuditTrail.tsx — the ip_address column, the Failed Login Bursts section, and the
    AuditBurst helpers (burstSpanMinutes, burstKindLabel, eventsInBurst)
  • admin-audit-service.ts — the AuditBurst interface and bursts / totals.bursts fields
  • admin_audit_trends() — the burst half only. daily_series feeds the sparkline and stays, so
    the function is amended, not dropped.
  • rate-limit-check.ts:52,58 — the ipAddress? parameter and p_ip_address forwarding no caller uses
  • seed-admin-demo.sql — the seeded IPs and the synthetic burst, which would otherwise seed a column
    nothing renders

Stays: Anomaly Alerts, the event log, the stat cards, daily_series, and everything keyed on email
or user_id.

Consequence for #914two tests, not four

Earlier notes said deleting sends four admin tests back to asserting nothing. Measured: it is
twoshould display burst detection cards and should expand burst card to show event details.
Both are deleted with the feature. should display anomaly alerts and should filter events by type
are unaffected and keep asserting.

#914 steps 5–7 are unblocked once that lands.

Reversibility

Cheap and deliberate to undo: a fork that genuinely needs IP capture adds an edge function, widens
log_auth_event, and reinstates the panel — with the privacy disclosure that decision requires. What
is being removed is the pretence that it already works.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority:p2Medium — schedule (feature gaps, partial implementations)

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions