Classify each forwarded-for entry as public or private, never the entry #44

Merged
jlxq0 merged 2 commits from feat/xff-scope-classification into main 2026-09-06 01:01:05 +00:00
Owner

47 unanimous xff_entries=2 over 24 hours answered the count and left the classification open. Nothing distinguishes a spoofable public first entry from a private one our own fabric appended, and that single distinction settles four constants at once: this server's trusted_proxy_hops, webmail's depth 2, Synapse's x_forwarded: true, and Mastodon's absence. Three of those record nothing and cannot be checked after the fact.

The line gains one field:

xff_entries        2
xff_scopes         "public,private"
trusted_proxy_hops 0
client_ip_resolved false

One token per entry, in order, and never an address. An address in a log is the thing the rule against logging them exists to keep out, so the classification is the whole payload.

An entry that does not parse is unparseable rather than assumed either way, because a port-bearing or obfuscated entry counted as public would manufacture the finding this exists to test.

Ipv4Addr::is_global and Ipv6Addr::is_unique_local are unstable, so the v6 cases are the prefix tests they would perform: fc00::/7 and fe80::/10.

The order test was green and pinned nothing

Seven tests. Three mutations, and the third earned its keep.

is_private_scope always true    3 red, including the public-first case
BTreeSet, sorted and deduped    4 red, compound and isolates nothing
order reversed, dupes kept      position test stayed GREEN

It asserted only that the two orders differ, which survives reversing the chain because that reverses both sides. It pinned that the function is not set-valued while its name claimed it pinned order. Both sides are now named exactly, public,private and private,public, and it goes red under that reversal.

Had I stopped at the BTreeSet mutation, which reddened four tests and looked like ample proof, the weak test would have shipped under a name asserting a property no test held.

Scope

No edge configuration is touched, which is what makes this a measurement rather than a decision. If entry two comes back public, choosing a hop count is Clark's gate, not mine. oddie-apps/infrastructure#65 labels the externalTrafficPolicy: Cluster explanation a candidate rather than the answer, and it should stay labelled that way until this line lands and reports.

cargo clippy --all-targets -- -D warnings clean, 148 + 9 tests green.

47 unanimous `xff_entries=2` over 24 hours answered the count and left the classification open. Nothing distinguishes a spoofable public first entry from a private one our own fabric appended, and **that single distinction settles four constants at once**: this server's `trusted_proxy_hops`, webmail's depth 2, Synapse's `x_forwarded: true`, and Mastodon's absence. Three of those record nothing and cannot be checked after the fact. The line gains one field: xff_entries 2 xff_scopes "public,private" trusted_proxy_hops 0 client_ip_resolved false One token per entry, in order, and **never an address**. An address in a log is the thing the rule against logging them exists to keep out, so the classification is the whole payload. An entry that does not parse is `unparseable` rather than assumed either way, because a port-bearing or obfuscated entry counted as public would manufacture the finding this exists to test. `Ipv4Addr::is_global` and `Ipv6Addr::is_unique_local` are unstable, so the v6 cases are the prefix tests they would perform: `fc00::/7` and `fe80::/10`. ### The order test was green and pinned nothing Seven tests. Three mutations, and the third earned its keep. is_private_scope always true 3 red, including the public-first case BTreeSet, sorted and deduped 4 red, compound and isolates nothing order reversed, dupes kept position test stayed GREEN It asserted only that the two orders differ, which **survives reversing the chain** because that reverses both sides. It pinned that the function is not set-valued while its name claimed it pinned order. Both sides are now named exactly, `public,private` and `private,public`, and it goes red under that reversal. Had I stopped at the `BTreeSet` mutation, which reddened four tests and looked like ample proof, the weak test would have shipped under a name asserting a property no test held. ### Scope No edge configuration is touched, which is what makes this a measurement rather than a decision. **If entry two comes back public, choosing a hop count is Clark's gate**, not mine. `oddie-apps/infrastructure#65` labels the `externalTrafficPolicy: Cluster` explanation a candidate rather than the answer, and it should stay labelled that way until this line lands and reports. `cargo clippy --all-targets -- -D warnings` clean, 148 + 9 tests green.
Classify each forwarded-for entry as public or private, never the entry
All checks were successful
CI / cargo (pull_request) Successful in 1m8s
CI / docker (pull_request) Successful in 54s
c259706dec
47 unanimous xff_entries=2 over 24 hours answered the count and left the
classification open. Nothing distinguishes a spoofable public first entry from
a private one our own fabric appended, and that single distinction settles four
constants at once: this server's trusted_proxy_hops, webmail's depth 2,
Synapse's x_forwarded, and Mastodon's absence. Three of those record nothing and
cannot be checked after the fact.

classify_xff_entries emits one token per entry, in order, and never an address:
"public,private" for a two-entry chain whose head came from outside. An address
in a log is the thing the rule against logging them exists to keep out, so the
classification is the whole payload.

An entry that does not parse is "unparseable" rather than assumed either way.
A port-bearing or obfuscated entry counted as public would manufacture the
finding this exists to test.

Ipv4Addr::is_global and Ipv6Addr::is_unique_local are unstable, so the v6 cases
are the prefix tests they would perform: fc00::/7 and fe80::/10.

Seven tests, and the order one was rewritten because a mutation found it green.
It first asserted only that the two orders differ, which survives reversing the
chain because that reverses both sides. It pinned that the function is not
set-valued and its name claimed it pinned order. Both sides are now named
exactly, and it goes red under that reversal.
Act on the review: public means not private, not routable
All checks were successful
CI / cargo (pull_request) Successful in 1m1s
CI / docker (pull_request) Successful in 50s
ba2fb00ea3
A cross-engine review of the diff found that CGNAT (100.64.0.0/10) and the
documentation ranges are labelled public while being non-routable. False
positives, and they err in the recoverable direction: one makes somebody look
at a chain that is fine, where the opposite hides the entry this exists to
catch.

The v6 public case asserted 2001:db8::1, which is the documentation range, so
it pinned the caveat as though it were the property. The v4 case has the same
shape with 203.0.113.7, TEST-NET-3. Both conventions are now asserted: the
documentation ranges because they are the right test data, and a routable
address of each family because that is the property.
jlxq0 merged commit 64d5a53f6a into main 2026-09-06 01:01:05 +00:00
jlxq0 deleted branch feat/xff-scope-classification 2026-09-06 01:01:05 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
jlxq0/caldav-mcp!44
No description provided.