ORACIS
Start a project

Field Notes

The reviewer was wrong about the client IP

Sheet N-101 · By the Oracis engineering team · · 4 min read

An independent AI reviewer looked at the rate limiter on this website's project-intake form and flagged it: the client address came from the wrong place, so a bot could forge its way past the limit. The finding sounded right. It cited the standard reasoning about proxies. The fix was merged, the tests passed, and the site shipped.

The fix was wrong, and it made the limiter weaker than before. This note is about how we found that out, and what we changed so the same kind of mistake gets caught before it ships.

The finding

A server behind a proxy never sees the visitor's address on its own socket; it sees the proxy's. Proxies pass the original address along in the X-Forwarded-For header, a comma-separated list that grows by one entry at each hop. A client can put anything it likes in that header before the request reaches the first proxy, so the usual advice is: trust only the entries your own proxies added, which on a single proxy means the last one.

The reviewer applied that advice. Our code keyed the limit on the first entry; the reviewer said to key it on the last. An engineer made the change, wrote tests that forged the first entry and proved the limit held, and a second review approved it.

Why it was wrong here

The advice is correct for a proxy that appends to whatever the client sent. The platform this site runs on does something else: its edge strips any client-supplied X-Forwarded-For, writes the real client address first, and then its own internal hops append theirs. On this platform the first entry is the one a client cannot forge, and the entries after it are internal addresses that change from request to request.

So the "fix" keyed the limiter on a rotating internal hop. Every request from the same visitor could land in a different bucket.

Which X-Forwarded-For entry a client can forge A request leaves the visitor, crosses the platform edge that strips any client header and writes the client address first, passes an internal hop that appends its own address, and reaches the app server. The resulting header lists the client, 198.51.100.7, then two internal hops, 10.0.4.12 and 10.0.9.31. The first entry is the client and cannot be forged here; the last entry is an internal hop that rotates. The limiter must key on the first entry. Visitor Platform edge strips client header, writes client first Internal hop appends its own address App server X-Forwarded-For: 198.51.100.7 10.0.4.12 10.0.9.31 first entry: the client (unforgeable here) last entry: an internal hop (rotates) our key: first entry the reviewer's fix keyed on the last Which X-Forwarded-For entry a client can forge A request leaves the visitor, crosses the platform edge that strips any client header and writes the client address first, passes an internal hop that appends its own address, and reaches the app server. The resulting header lists the client, 198.51.100.7, then two internal hops, 10.0.4.12 and 10.0.9.31. The first entry is the client and cannot be forged here; the last entry is an internal hop that rotates. The limiter must key on the first entry. Visitor Platform edge strips client header, writes client first Internal hop appends its own address App server X-Forwarded-For: 198.51.100.7 10.0.4.12 10.0.9.31 first entry: the client (unforgeable here) last entry: an internal hop (rotates) our key: first entry the reviewer's fix keyed on the last
Fig. N-101Which entry of the header a client can forge depends on the platform

How we found it

Not by reading the code again. The review, the tests and the code all agreed with each other; they were all built on the same assumption about the proxy. What settled it was a probe against the live site: seven submissions from one machine, each with a different forged header. They filled in the form's honeypot field, so the server accepted them without storing anything.

for (let i = 1; i <= 7; i++) {
  const r = await fetch(API + '/intake', {
    method: 'POST',
    headers: { 'Content-Type': 'application/json', 'X-Forwarded-For': '203.0.113.' + i },
    body: JSON.stringify({ website: 'http://bot.example', name: 'xff probe', email: 'probe@example.com', message: 'honeypot probe' }),
  });
  console.log(i, 'spoof 203.0.113.' + i, '->', r.status, await r.text());
}
1 spoof 203.0.113.1 -> 200 {"status":"ok"}
2 spoof 203.0.113.2 -> 200 {"status":"ok"}
3 spoof 203.0.113.3 -> 200 {"status":"ok"}
4 spoof 203.0.113.4 -> 200 {"status":"ok"}
5 spoof 203.0.113.5 -> 200 {"status":"ok"}
6 spoof 203.0.113.6 -> 200 {"status":"ok"}
7 spoof 203.0.113.7 -> 200 {"status":"ok"}

Seven accepted, none refused. The limit is five. The same seven requests without a forged header were refused from the third onward, because that bucket had already been used. The platform's own guidance confirmed the rest: the client is the leftmost entry.

We changed the key back to the first entry, wrote a test that sends requests sharing a first entry but with varying later hops, and probed the live site again:

1 spoof 203.0.113.1 -> 200
2 spoof 203.0.113.2 -> 200
3 spoof 203.0.113.3 -> 200
4 spoof 203.0.113.4 -> 200
5 spoof 203.0.113.5 -> 200
6 spoof 203.0.113.6 -> 429
7 spoof 203.0.113.7 -> 429

What we changed

The code fix was three lines. The process fix mattered more:

  • Platform-dependent findings are questions, not facts. Our review system now returns a claim whose truth depends on the deployment platform or network topology as a question, with the check that would settle it, instead of as a defect to fix. The reviewer is also told what platform the code runs on.
  • Security fixes to request handling get a live probe. Tests prove the code does what the tests assume. Only a request through the real edge proves the assumption.
  • The comment in the code now says what it depends on. The rule holds only behind this platform's edge; a server exposed directly to the internet would need a different one, and the code says so.

The general lesson

Independent review works: the same reviewer found three real defects in this site that we then reproduced and fixed. But a reviewer, human or model, judges against the context it is given. When the answer depends on something outside the code, the review can be unanimous and still wrong. The check that catches it has to touch the real system.

Want this kind of engineering on your system?

Start a project
Next sheet · N‑102 Row-level security first