feat(audit): log the X-Forwarded-For entry count, never the entries #18

Merged

jlxq0 merged 1 commit from feat/log-xff-entry-count into main

2026-08-27 03:13:54 +00:00

jlxq0 commented

2026-08-27 03:12:24 +00:00

Owner

The measurement Alan asked for before any number is proposed. Refs #14.

Why there is nothing to read today

DEFAULT_TRUSTED_PROXY_HOPS = 1, commented "Default 1 (Traefik)", and a test comment reads "that is the IP Traefik saw". There is no Traefik on Fondue. Ingress is a Gateway API HTTPRoute on gatewayClassName: cilium, and jmap-mcp.kampong.social resolves to 203.24.209.8 while the web gateway address is 203.24.209.5, so something sits in front of the gateway as well. The Deployment does not set JMAP_MCP_TRUSTED_PROXY_HOPS, so the pod runs on that default.

Nothing reports the chain length the pod receives: RUST_LOG=jmap_mcp=info,tower_http=info,info, and 200 lines of pod log contain zero matches for forwarded, client_ip or x-forwarded.

parse_client_ip counts in from the right, so a hop count one too low selects the entry to the client's right and writes the proxy's address into the audit trail as the client's. Absent is recoverable; a confident wrong address in a provenance record is not.

What this logs

The count, never the entries. The entries are sender-influenced and identify people; the count is what the configuration must agree with and carries no identity.

It counts on the raw header bytes rather than through to_str(). Codex flagged that a non-UTF-8 header counts 0 through to_str().ok(), which is indistinguishable from the header being absent — a measurement reporting one value for "no proxy" and "unreadable" cannot settle the question it exists to answer. Empty and whitespace-only are 0 entries, not 1.

Every test mutation-checked

mutation	expected red	observed
`xff_entry_count` always returns 1	both count tests	both red
`parse_client_ip` indexes from the left	direction test only	direction test only
drop the empty/whitespace guard	collapsed-cases test only	that test only
count through `to_str()` again	collapsed-cases test only	that test only

The second and fourth rows are the point: each new test goes red on its own mutation and stays green on the others, so none of them merely restates another.

Gates

All five on the pinned 1.93.0, not on local stable: fmt, clippy --all-targets --all-features -D warnings, test (168 passed), audit, deny.

clippy::naive_bytecount rejected the first spelling of the counter. That is the argument for the pin: mise's rustc shim resolves stable (1.98.0) and ignores rust-toolchain.toml, so cargo +1.93.0 is required locally or a local green is on the wrong compiler.

Codex review

Asked whether the change can leak any sender-controlled or personally-identifying value into a log line or elsewhere, considering tracing field expansion, other fields on the event, log volume as amplification, and whether the counter can panic or be made expensive.

Personally identifying content leaked by this change: none. Sender-controlled metadata leaked: the chosen entry count only.

It noted one extra info line per authenticated request. That is bounded by the existing rate limits (JMAP_MCP_RATE_LIMIT_READS_PER_MIN, ..._WRITES_PER_MIN) and requires a valid bearer, and the two defects it found in the measurement are fixed above.

This is not a fix

No number is proposed and JMAP_MCP_TRUSTED_PROXY_HOPS is unchanged. Reading the count needs this deployed, which is a release and therefore Alan's call, not mine.

The measurement Alan asked for before any number is proposed. Refs #14. ## Why there is nothing to read today `DEFAULT_TRUSTED_PROXY_HOPS = 1`, commented **"Default 1 (Traefik)"**, and a test comment reads *"that is the IP Traefik saw"*. **There is no Traefik on Fondue.** Ingress is a Gateway API `HTTPRoute` on `gatewayClassName: cilium`, and `jmap-mcp.kampong.social` resolves to `203.24.209.8` while the `web` gateway address is `203.24.209.5`, so something sits in front of the gateway as well. The Deployment does not set `JMAP_MCP_TRUSTED_PROXY_HOPS`, so the pod runs on that default. Nothing reports the chain length the pod receives: `RUST_LOG=jmap_mcp=info,tower_http=info,info`, and 200 lines of pod log contain zero matches for `forwarded`, `client_ip` or `x-forwarded`. `parse_client_ip` counts in from the right, so a hop count one too low selects the entry to the client's right and writes **the proxy's address into the audit trail as the client's**. Absent is recoverable; a confident wrong address in a provenance record is not. ## What this logs The count, never the entries. The entries are sender-influenced and identify people; the count is what the configuration must agree with and carries no identity. It counts on the **raw header bytes** rather than through `to_str()`. Codex flagged that a non-UTF-8 header counts 0 through `to_str().ok()`, which is indistinguishable from the header being absent — a measurement reporting one value for "no proxy" and "unreadable" cannot settle the question it exists to answer. Empty and whitespace-only are 0 entries, not 1. ## Every test mutation-checked | mutation | expected red | observed | |---|---|---| | `xff_entry_count` always returns 1 | both count tests | both red | | `parse_client_ip` indexes from the left | direction test only | direction test only | | drop the empty/whitespace guard | collapsed-cases test only | that test only | | count through `to_str()` again | collapsed-cases test only | that test only | The second and fourth rows are the point: each new test goes red on its own mutation and stays green on the others, so none of them merely restates another. ## Gates All five on the pinned **1.93.0**, not on local stable: `fmt`, `clippy --all-targets --all-features -D warnings`, `test` (168 passed), `audit`, `deny`. `clippy::naive_bytecount` rejected the first spelling of the counter. That is the argument for the pin: **mise's `rustc` shim resolves `stable` (1.98.0) and ignores `rust-toolchain.toml`**, so `cargo +1.93.0` is required locally or a local green is on the wrong compiler. ## Codex review Asked whether the change can leak any sender-controlled or personally-identifying value into a log line or elsewhere, considering tracing field expansion, other fields on the event, log volume as amplification, and whether the counter can panic or be made expensive. > Personally identifying content leaked by this change: none. Sender-controlled metadata leaked: the chosen entry count only. It noted one extra info line per authenticated request. That is bounded by the existing rate limits (`JMAP_MCP_RATE_LIMIT_READS_PER_MIN`, `..._WRITES_PER_MIN`) and requires a valid bearer, and the two defects it found in the measurement are fixed above. ## This is not a fix No number is proposed and `JMAP_MCP_TRUSTED_PROXY_HOPS` is unchanged. Reading the count needs this deployed, which is a release and therefore Alan's call, not mine.

jlxq0 added 1 commit

2026-08-27 03:12:24 +00:00

feat(audit): log the X-Forwarded-For entry count, never the entries

CI / cargo (pull_request) Successful in 1m8s

Details

CI / tag-ancestry (pull_request) Successful in 3s

Details

CI / docker (pull_request) Successful in 53s

Details

6e5b3d8091

JMAP_MCP_TRUSTED_PROXY_HOPS defaults to 1 and its comment reads "Default 1
(Traefik)". There is no Traefik on Fondue: ingress is a Gateway API HTTPRoute
on gatewayClassName cilium, and jmap-mcp.kampong.social resolves to
203.24.209.8 while the web gateway's own address is 203.24.209.5, so something
sits in front of the gateway as well. The Deployment does not set the variable,
so the pod runs on that default, and nothing anywhere reports the chain length
the pod actually receives.

parse_client_ip counts in from the right, so a hop count one too low selects
the entry to the client's right and records the proxy's address as the
client's in the audit trail. Absent is recoverable; a confident wrong address
in a provenance record is not.

Logs the count only. The entries are sender-influenced and identify people;
the count is what the configuration must agree with and carries no identity.

Counts on the raw header bytes rather than through to_str(): a non-UTF-8
header would otherwise count 0, indistinguishable from the header being
absent, and a measurement that reports one value for "no proxy" and
"unreadable" cannot settle the question it exists to answer. Empty and
whitespace-only values are 0 entries rather than 1.

Tests, each mutation-checked on the pinned 1.93.0 toolchain:
  - xff_entry_count always returning 1     -> both count tests red
  - parse_client_ip indexing from the left -> only the direction test red,
      so it pins the direction rather than restating the count test
  - dropping the empty/whitespace guard    -> only the collapsed-cases test red
  - counting through to_str() again        -> only the collapsed-cases test red

Five gates green on 1.93.0: fmt, clippy -D warnings, test (168), audit, deny.
clippy::naive_bytecount caught the first spelling of the counter, which is why
the pinned toolchain matters: mise's rustc shim resolves stable (1.98.0) and
ignores rust-toolchain.toml, so `cargo +1.93.0` is required locally.

This is a measurement, not a fix. No number is proposed until the deployed pod
reports a count.

Refs #14

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jlxq0 merged commit 2227329eb5 into main

2026-08-27 03:13:54 +00:00

jlxq0 deleted branch feat/log-xff-entry-count

2026-08-27 03:13:54 +00:00

jlxq0 referenced this pull request from a commit

2026-08-27 03:13:55 +00:00

Merge pull request 'feat(audit): log the X-Forwarded-For entry count, never the entries' (#18) from feat/log-xff-entry-count into main

jlxq0 referenced this pull request

2026-08-27 03:14:45 +00:00

chore(release): v0.2.15 #19