Skip to content

fix(moderation): write the address ranges the ModCP lookup reads - #121

Merged
jouwdan merged 1 commit into
mainfrom
claude/mei52-ip-prefixes
Aug 16, 2026
Merged

fix(moderation): write the address ranges the ModCP lookup reads#121
jouwdan merged 1 commit into
mainfrom
claude/mei52-ip-prefixes

Conversation

@jouwdan

@jouwdan jouwdan commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

Fixes MEI-52.

The bug

The ModCP "Address lookup" screen and the admin member search's IP filter both query users.registration_ip_prefix and users.last_ip_prefix — and nothing in the board ever wrote either column.

  • /modcp/ip reads both (packages/db/src/modcp-repo.ts, ipPrefixesFor / ipMatches); the member search filters on the same pair (packages/db/src/user-admin-repo.ts), as does the "shares a network" panel on a member's admin page.
  • Registration (packages/db/src/account-repos.ts), sign-in and the presence write all omitted them. Repo-wide, only the schema and the readers mentioned the columns; the db tests passed because they populated them with raw UPDATEs.

So every lookup answered "Ranges on record: none" forever, while still writing a modcp.ip_lookup audit row for a query against empty columns — and the page's lede ("Finds accounts that share a stored address range with a member") and docs/mybb-parity.md ("it matches the truncated prefix the board stores") both described something that could not happen.

The fix

Make the columns real, using the plumbing that already exists.

  • Registration stores the truncated range it came from, via a new optional NewAccount.registrationIpPrefix.
  • A successful sign-in rewrites the last-visit range, via a new AccountRepository.recordLastIpPrefix(userId, prefix).
  • The range is resolved in the app exactly where every other request-derived address already is: truncateIp(await remoteAddress()) in auth-actions.ts — the same pair recordAdminAction uses for the moderator log. No second truncation was written.
  • It reaches the domain as an argument on the existing RequestContext that register() and login() already accept, alongside ip. No package imports next/headers, and nothing in packages/ reaches for request state.

Decisions

Sign-in, not the presence write, for last_ip_prefix. The presence path (sessions.touchLocation) fires once a minute per active member and would put a users UPDATE on the read path of every page view; a sign-in is the moment the board actually learns an account is being used from somewhere, and it costs one UPDATE per session. The cost is that a member signed in from before this ships shows no last-visit range until they sign in again, and a member who only ever resumes by remember-me cookie keeps the range of their last real sign-in. Both are written down in docs/mybb-parity.md.

Only the truncated prefix crosses the boundary. The app truncates before the value enters the domain, so a full address is never handed to a repository — the same posture the moderator log already holds. truncateIp yields /24 for v4 and /48 for v6; nothing else was touched, and the "IP starts with" filter's own copy ("Only a prefix is ever stored, so this is a network") is now true.

posts.ip_prefix is deliberately left unwritten — it is not silently dead by accident. Nothing reads it: the address lookup searches the two users columns only. Writing it would mean threading a prefix through NewThreadRecord, NewReplyRecord, both insert sites in thread-writes.ts, the copy path in thread-tools.ts and the fixture repositories, to produce a second address trail with no reader. Out of scope here, and said so in docs/mybb-parity.md rather than left to be rediscovered.

The MyBB importer still carries neither range. It does not read regip/lastip from the source board, so an imported board's lookups stay empty until its members register or sign in here. Recorded as a cost in the parity doc; changing the import contract is a separate change.

No next build risk: nothing under apps/community/app and no "use client" component was touched; no domain package is value-imported anywhere new.

Validation

  • pnpm verify — green (exit 0). 310 test files, 5728 tests. The two warnings it prints (consistent-type-imports in admin.test.ts, no-orphans on apps/web/src/format.ts) are pre-existing on main and unrelated.
  • DATA_SOURCE=fixture pnpm build — green (exit 0).
  • No e2e spec was changed, and pnpm test:e2e was not run.

New tests, each proved to fail first by breaking the fix and restoring it:

Test Where Broken by
registration keeps the range, sign-in records it, neither writes without an address, a failed sign-in writes nothing packages/accounts/src/service.test.ts dropping the two writes from IdentityService → 3 red
the range a registration came from round-trips through the real columns; a sign-in range does not disturb it; a lookup finds the account sharing a /24 and leaves an unrelated one out; an account with no range matches nothing packages/db/src/account-repos.test.ts (PGlite, boot once + clear in beforeEach) removing the column write and the UPDATE → 3 red
the stored value is the truncated range and never the address, at the real call site, for both registration and sign-in; a refused sign-in records nothing apps/community/src/server/auth-actions.test.ts passing remoteAddress() without truncateIp → 2 red

The db test drives the writer (PostgresAccountRepository) and the reader (PostgresModCpRepository) in one place, which is the pair that was silently disagreeing.

Docs updated in the same commit: docs/mybb-parity.md (when each range is written, why sign-in and not the presence write, and the costs) and docs/operating.md (the address now keys five things, not four, and TRUSTED_PROXY_HOPS=0 records no ranges either).

🤖 Generated with Claude Code


Generated by Claude Code

The ModCP address lookup and the admin member search's IP filter both
query users.registration_ip_prefix and users.last_ip_prefix, and nothing
ever wrote either column: every lookup answered "Ranges on record: none"
while still writing a modcp.ip_lookup audit row.

Registration now stores the truncated range it came from, and a
successful sign-in rewrites the last-visit range. The range is resolved
where every other request-derived address already is — remoteAddress()
and truncateIp() in the app — and passed into the domain as an argument
on the existing RequestContext, so no package reaches for request state.
Only the /24 or /48 prefix crosses that boundary; a full address is never
handed to a repository.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014kP43A5shJWmAKqkBovhaF
@jouwdan
jouwdan merged commit 8e73814 into main Aug 16, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants