feat(feedback): link the captured IP into the ban flow (BUNYIP-436) #428

Merged

longjacksonle merged 1 commit from feat/BUNYIP-436-feedback-ip-ban-link into main

2026-08-01 20:16:02 +02:00

longjacksonle commented

2026-08-01 19:50:55 +02:00

Owner

What and why

BUNYIP-436 asked to capture the client IP and user agent on feedback and surface them so repeat spam sources can be identified and banned. The first task was to establish what the earlier BUNYIP-411 actually delivered before re-implementing anything.

Finding on BUNYIP-411 (acceptance criterion 1)

BUNYIP-411 shipped almost all of it, verified against the code on main:

Columns submitter_ip (INET) and user_agent exist on feedback (migration 20260730000010_feedback_request_metadata.sql).
submit_feedback populates both: the IP via the shared extract_client_ip resolver, the user agent bounded to 256 chars.
The request metadata is mirrored to the audit log (FeedbackSubmitted, IP in actor_ip_address, UA in the metadata JSON), and the password is never recorded.
The admin detail view already renders both fields, admin-only.

Address resolution was already correct, not a Docker IP: submit_feedback uses the same extract_client_ip helper the Autoban capture uses, which trusts X-Forwarded-For only from a configured trusted proxy and otherwise falls back to the socket peer. The separately-tracked audit-address defect does not affect feedback, which goes through the shared resolver. So no capture, persistence, resolution, or audit work was needed here.

The one remaining gap (criterion 5): make the IP actionable

The captured IP was shown as plain text with no path to a ban. This PR links it into the existing ban flow:

The feedback detail IP is now an anchor to /admin/ip-bans?ip=<addr> (URL-encoded through the shared urlenc).
The IP Bans page reads that ip query param and seeds the add-ban form's address field from it, so the admin lands on the ban screen with the address filled in and only adds a reason.
The user agent stays plain admin-only text. Both fields remain behind admin_guard; nothing is added to any user-facing view.

Acceptance criteria

What BUNYIP-411 delivered is established and recorded (on the issue and above).
Client IP and user agent persisted with each new submission (BUNYIP-411; verified).
Address resolution uses the same shared helper as the Autoban capture and records the real external address, not a Docker IP (BUNYIP-411; verified).
Both fields visible in the feedback admin view and not in any user-facing view.
The IP links into the existing ban flow (this PR: prefilled add-ban form).
Tests confirming the correct client address is recorded through the reverse proxy (existing resolve_client_ip unit tests) plus new tests for the prefill link.

Tests and verification

just check-container green: fmt + clippy -D warnings + full workspace suite (166 web tests incl. the 2 new, 312 domain, others).
New tests: the add-ban card seeds its address from the prefill (blank without one); the feedback detail renders the IP as a link into the ban flow with the UA shown.
End-to-end on the dev stack (feedback row seeded with a resolved IP, since dev has no trusted proxy to forward one): the detail page shows the IP as a link, and following it lands on IP Bans with the add form pre-filled to 203.0.113.7.

🤖 Generated with Claude Code

https://claude.ai/code/session_01US9AFL2ZTGrLH9TEPSmNfm

## What and why BUNYIP-436 asked to capture the client IP and user agent on feedback and surface them so repeat spam sources can be identified and banned. The first task was to establish what the earlier BUNYIP-411 actually delivered before re-implementing anything. ## Finding on BUNYIP-411 (acceptance criterion 1) BUNYIP-411 shipped almost all of it, verified against the code on `main`: - Columns `submitter_ip` (INET) and `user_agent` exist on `feedback` (migration `20260730000010_feedback_request_metadata.sql`). - `submit_feedback` populates both: the IP via the shared `extract_client_ip` resolver, the user agent bounded to 256 chars. - The request metadata is mirrored to the audit log (`FeedbackSubmitted`, IP in `actor_ip_address`, UA in the metadata JSON), and the password is never recorded. - The admin detail view already renders both fields, admin-only. Address resolution was already correct, not a Docker IP: `submit_feedback` uses the same `extract_client_ip` helper the Autoban capture uses, which trusts `X-Forwarded-For` only from a configured trusted proxy and otherwise falls back to the socket peer. The separately-tracked audit-address defect does not affect feedback, which goes through the shared resolver. So no capture, persistence, resolution, or audit work was needed here. ## The one remaining gap (criterion 5): make the IP actionable The captured IP was shown as plain text with no path to a ban. This PR links it into the existing ban flow: - The feedback detail IP is now an anchor to `/admin/ip-bans?ip=<addr>` (URL-encoded through the shared `urlenc`). - The IP Bans page reads that `ip` query param and seeds the add-ban form's address field from it, so the admin lands on the ban screen with the address filled in and only adds a reason. - The user agent stays plain admin-only text. Both fields remain behind `admin_guard`; nothing is added to any user-facing view. ## Acceptance criteria - [x] What BUNYIP-411 delivered is established and recorded (on the issue and above). - [x] Client IP and user agent persisted with each new submission (BUNYIP-411; verified). - [x] Address resolution uses the same shared helper as the Autoban capture and records the real external address, not a Docker IP (BUNYIP-411; verified). - [x] Both fields visible in the feedback admin view and not in any user-facing view. - [x] The IP links into the existing ban flow (this PR: prefilled add-ban form). - [x] Tests confirming the correct client address is recorded through the reverse proxy (existing `resolve_client_ip` unit tests) plus new tests for the prefill link. ## Tests and verification - `just check-container` green: fmt + clippy `-D warnings` + full workspace suite (166 web tests incl. the 2 new, 312 domain, others). - New tests: the add-ban card seeds its address from the prefill (blank without one); the feedback detail renders the IP as a link into the ban flow with the UA shown. - End-to-end on the dev stack (feedback row seeded with a resolved IP, since dev has no trusted proxy to forward one): the detail page shows the IP as a link, and following it lands on IP Bans with the add form pre-filled to `203.0.113.7`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01US9AFL2ZTGrLH9TEPSmNfm

longjacksonle added 1 commit

2026-08-01 19:50:56 +02:00

feat(feedback): link the captured IP into the ban flow (BUNYIP-436)

E2E PR gate / Install + reachability (no deployment secrets) (pull_request) Successful in 27s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 25m4s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

3916f1dce1

BUNYIP-411 already captures the client IP and User-Agent on each feedback submission (columns on `feedback`, populated in `submit_feedback` via the shared `extract_client_ip` resolver, mirrored to the audit log), and the admin detail view already surfaces both. The one remaining acceptance criterion was making the IP actionable: a link into the existing ban flow so a repeat spam source can be banned in one hop.

The feedback detail IP is now an anchor to `/admin/ip-bans?ip=<addr>` (URL-encoded through the shared `urlenc`). The IP Bans page reads that `ip` query param and seeds the add-ban form's address field from it, so the admin lands on the ban screen with the address filled in and only has to add a reason. The user agent stays plain admin-only text. Both fields remain admin-only (the detail view is behind `admin_guard`); nothing is added to any user-facing view.

Address resolution is unchanged and already correct: `submit_feedback` uses the same `extract_client_ip` helper the Autoban capture uses, which honours `X-Forwarded-For` only from a trusted proxy and otherwise falls back to the socket peer, so it records the real external client behind Traefik rather than a Docker IP. That resolver is covered by existing `resolve_client_ip` unit tests exercising the reverse-proxy path.

New tests: the add-ban card seeds its address field from the prefill (and stays blank without one), and the feedback detail renders the IP as a link into the ban flow with the user agent shown.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01US9AFL2ZTGrLH9TEPSmNfm

longjacksonle scheduled this pull request to auto merge when all checks succeed

2026-08-01 19:52:54 +02:00