[Deicyde]

Problem

POST /api/flag/:slug (anonymous reader reports) stores flags.ip_hash as an unsalted SHA-256 of CF-Connecting-IP (wiki/src/index.ts:2450 at 37b7036: ipHash: ip ? await sha256Hex(ip) : null). There are only 2^32 IPv4 addresses, so anyone holding a row can recover the address by enumeration. For IPv4 the column is effectively plaintext.

The value leaves D1 in full database exports (wrangler d1 export). The research export (/api/research/export.jsonl) does not include the flags table.

Nothing reads the column:

• The flag rate limit keys FLAG_LIMITER on the raw IP inside the ratelimit binding, which is not persisted.
• The anti-spam cap counts open flags per target, not per IP.
• /flags, /api/flags, /stats and the research export never select it.
• A repo-wide git grep -i ip_hash finds only the writer, the schema and docs.

Change

• The flag endpoint stores ip_hash = NULL. No IP-derived value is kept. FLAG_LIMITER and the per-target open-flag cap remain the abuse defenses; there have been 2 flags ever.
• Migration 0013_flags_ip_hash_scrub.sql is an unconditional UPDATE flags SET ip_hash = NULL WHERE ip_hash IS NOT NULL. It is idempotent and safe to run at any time, because no Worker version reads the column.
• Comments and docs: the schema.ts flags comment and docs/research-plan.md now describe the column as no longer written, and the comment above sha256Hex (still used by the research export) no longer calls it the flag handle.

The first revision of this PR stored an HMAC under a new IP_HASH_SECRET secret. Review pointed out that nothing reads the handle, and that wrangler secret put deploys a Worker outside the Brain promoter, which can abort a frozen promotion. So this revision drops the handle and the secret (0acb967).

How tested

• npx tsc --noEmit passes.
• npx vitest run: 917 pass, including:
  • an anonymous flag with CF-Connecting-IP set stores ip_hash: null;
  • the migration, applied on top of the real migration files, NULLs a legacy bare-hex value and any other non-null shape, keeps NULLs, and is idempotent.
• Research-export pseudonyms are unchanged: sha256Hex is main's implementation.

Risks

• The IP-derived abuse handle is gone. If anonymous flag abuse appears later, re-introduce a keyed handle then; the rate limiter does not depend on this column.
• The scrub only reaches the live table. D1 Time Travel history and any existing wrangler d1 export dumps keep the legacy values until they expire or are deleted.
• A rollback can bring the hash back. A rollback to a Worker version from before this PR writes the unsalted hash again, so re-run 0013 after one.

Release-affecting (wiki/src, wiki/migrations): ships only through the Brain release and promotion path. No new secret.

What Jack must do

After the new Worker is live (running it earlier is harmless, but then repeat it), apply the scrub. Remote migration tracking is out of sync, as noted in 0012, so execute the file directly:

cd wiki && npx wrangler d1 execute wikilean --remote --file=migrations/0013_flags_ip_hash_scrub.sql

Re-run it after any rollback to an older Worker version, or a D1 restore to a point before the scrub. If this ships in the same release as #63, apply 0014 before the deploy and 0013 after it.

[Deicyde]

Review status: merged in 61321b5e; all required fixes verified

Update 2026-10-14 (re-checked at c5e59ac1):

• Landed (verified at c5e59ac and on main): POST /api/flag/:slug now stores ipHash: null (wiki/src/index.ts:2454). The HMAC path, IP_HASH_SECRET (env.ts, README) and the HMAC tests are gone. FLAG_LIMITER still keys on the raw IP, and sha256Hex is main's body again, so research-export pseudonyms are unchanged.
• Landed: 0013 is an unconditional UPDATE flags SET ip_hash = NULL WHERE ip_hash IS NOT NULL. Its test runs it twice over mixed rows and expects all NULL (wiki/test/flags.test.ts:221-238).
• Landed: schema.ts:127-130, the sha256Hex comment and docs/research-plan.md:157-158 now describe the column as no longer written.
• Departure: the PR body adopts the no-handle default rather than leaving it as an open question. Reasonable: the merge settles it.
• Still open: the owner must run 0013 on production after the promotion that ships this change, and again after any rollback or pre-scrub restore. better-auth sessions.ip_address stays separate work.
• No new issues in the post-review commits; all checks are green.

Original review (2026-10-14)

Independent adversarial review of 7526a96e: a defect reviewer with skeptics, a design reviewer with a devil's advocate, and an arbiter.

Verdict: the code is correct, but nothing reads flags.ip_hash. Stop writing it and make the scrub unconditional.

Required before merge

• Store ipHash: null and remove flagIpHash, IP_HASH_PREFIX, the hexOf refactor, IP_HASH_SECRET (env.ts, wiki/README.md:151) and the HMAC tests; keep sha256Hex, which the research export uses. No code reads the column, and step 1's wrangler secret put deploys a Worker outside the promoter, which can abort a frozen Brain promotion.
• Make 0013 an unconditional UPDATE flags SET ip_hash = NULL WHERE ip_hash IS NOT NULL (keep the number; 0014 is taken). No Worker version has ever read the column, so it is safe any time; rerun it after promotion, rollback, or a restore to before the scrub.
• Update the schema.ts flags comment, the comment above sha256Hex, and docs/research-plan.md:157-158. They describe data that will no longer be collected.

Decisions for the maintainer

• Keep an IP-derived abuse handle? Recommended: no; there have been 2 flags ever, and FLAG_LIMITER plus the open-flag cap are the live defenses. The swarm will apply the NULL fix unless you want the handle; if so, merge as written plus a note on when the secret may be set.
• Scrub the 2 legacy values now? Safe against the current Worker, but it is a production write, so yours to run.

Merge order: release-affecting; merge with the other wiki/src PRs before freezing the next Brain release, never between freeze and promotion.

Fixes are in progress; this comment will be updated.

Posted by the WikiLean PR swarm (lead); updated 2026-10-14.

[Deicyde]

Update (0acb967): this review covered the HMAC version (7526a96e). The branch now follows the other review: it stores NULL, drops IP_HASH_SECRET, and makes the 0013 scrub unconditional. That resolves the secret-timing nit and the research-plan.md nit below. Typecheck is clean and 917/917 worker tests pass.

Status review (2026-10-14): ✅ Ready, with nits

The code holds up. There is no keyless fallback, an empty secret counts as unset, and the 0013 scrub is idempotent and case-sensitive on the hmac-sha256: prefix. The tests cover secret set/unset/empty/no-IP/rotated and replay the real migrations. A combined run with #63 passes the flags and queue-rotation tests and tsc.

Nits

• Secret timing (PR body, step 1): "Set the secret, before or after the deploy" leaves out that wrangler secret put itself deploys a new Worker version. Done after a release's P1B dry-run or bundle freeze, it invalidates the frozen predeploy evidence. Done mid-promotion, it trips the promoter's deployment fences. Suggest saying: set it before the release's dry-run, or use wrangler versions secret put, and never during a promotion. Setting it now is harmless, since no bundle is frozen.
• docs/research-plan.md:157 still says flags.ip_hash "is never exported". That is the wording this PR removed from schema.ts.
• Out of scope: better-auth also records sessions.ip_address (read from x-forwarded-for by default), so full D1 exports likely still hold client IPs for signed-in sessions. This is already tracked in docs/ROADMAP.md ("stop storing session ip_address"). Setting better-auth's advanced.ipAddress.disableIpTracking would close it.

To ship (nothing deploys on merge)

• This is a Worker change, so it reaches production only with the next Brain release and promotion.
• Run 0013 with wrangler d1 execute --remote --file=… after the new Worker is live. Running it earlier is harmless but must then be repeated.
• Re-run 0013 after any rollback to a pre-wiki: stop storing flags.ip_hash; scrub legacy unsalted values #46 Worker version.
• If it ships in the same release as wiki: serve wp-update the whole drifted set (cap 1000) instead of attempt stamps #63, apply 0014 before the deploy and 0013 after it.

Order: independent. Merge before #63 so the migrations land in numeric order.

---

_{🤖 Automated status pass by Claude Code over all 18 open PRs (#46–#63), posted for the repo owner. An independent verifier re-checked each finding. All 18 heads were also merged and tested together in a scratch tree; the only test failure that comes from combining them is #47 × #55 (three other pairs had mechanical text conflicts). Reviewed head: 7526a96e.}

[Deicyde]

Fixes pushed (2026-10-14)

These commits address the two earlier reviews on this PR: all three required items from the PR swarm's review, plus the research-plan.md wording and 0013 timing from the "Status review (2026-10-14)" comment.

What changed

• The flag endpoint stores ipHash: null. flagIpHash, IP_HASH_PREFIX, the hexOf refactor, IP_HASH_SECRET and the HMAC tests are removed, and the body of sha256Hex is main's again. The secret-timing nit no longer applies. (swarm 1; 0acb9675)
• 0013 is an unconditional UPDATE flags SET ip_hash = NULL WHERE ip_hash IS NOT NULL. Its test runs it twice over mixed rows and expects every row to be NULL. (swarm 2; 0acb9675)
• The 0013 header says to run it once the new Worker is live (57f1e521), and again after a rollback or a pre-scrub restore (0acb9675). (swarm 2, status review "To ship")
• The schema.ts flags comment and docs/research-plan.md now say the column is no longer written, replacing "never exported". The sha256Hex comment is reworded. (swarm 3, status review nit; 0acb9675)

Verification

• At 57f1e521: tsc is clean and the worker suite passes 917/917.
• Mutation checks: restoring the sha256Hex(ip) writer, or a 0013 that keeps hmac-sha256: rows, makes a test fail.
• Merged with wiki: serve wp-update the whole drifted set (cap 1000) instead of attempt stamps #63: tsc is clean and 928/928 pass.
• CI on 57f1e521: all checks green.

Not changed / still open

• Following the swarm's default, there is no IP-derived handle; the owner can still ask for a keyed one.
• better-auth sessions.ip_address is out of scope and stays on the roadmap.

Merge notes

• Merge with the other wiki/src PRs before the next Brain release freeze, and before wiki: serve wp-update the whole drifted set (cap 1000) instead of attempt stamps #63. If they ship together, apply 0014 before the deploy and 0013 after it.
• Running 0013 on production remains a manual owner step, as the PR body describes.

---

_{🤖 Posted by Claude Code for the repo owner. An independent verifier checked the whole PR before 57f1e521 was pushed. Head: 57f1e521.}