fix(gateway): reject reserved IP ranges in OBJECT_STORE_HOSTS

[marcusrbrown]

OBJECT_STORE_HOSTS previously accepted any IP literal because RFC 1123 labels permit pure digits. That left an SSRF vector via the env var: an operator (or attacker with deploy-config access) could point at cloud metadata services (169.254.169.254), loopback (127.0.0.1), or RFC 1918 private ranges — all reachable from inside the workspace network.

The MinIO/self-hosted use case still needs IP support, so a blanket ban would over-correct. Instead each entry is parsed through Python's ipaddress module. Public IPs (v4 and v6) pass through. Anything that reports is_private, is_loopback, is_link_local, is_multicast, is_reserved, or is_unspecified raises ValueError at mitmproxy startup with a clear message explaining the blocked range and pointing to public-IP / hostname-mapping alternatives.

Validation order

empty-skip
→ wildcard-reject
→ ip-literal-validate (new)
→ port-reject
→ hostname-validate
→ lowercase-normalize
→ append

The IP check fires before port-reject so bare IPv6 literals (::1, fe80::1, fc00::1) are caught by the IP validator rather than the colon-detection heuristic. The port-reject step still catches bracket-form IPv6 with a port ([2001:db8::1]:9000) — that case is not supported.

Tests

deploy/mitmproxy/test_allowlist.py: 31 → 42 (+11).

• accepts_public_ipv4 (8.8.8.8)
• rejects_metadata_service_ipv4 (169.254.169.254)
• rejects_private_ipv4_10, _172, _192
• rejects_loopback_ipv4 (127.0.0.1)
• rejects_unspecified_ipv4 (0.0.0.0)
• accepts_public_ipv6 (2001:4860:4860::8888)
• rejects_loopback_ipv6 (::1)
• rejects_link_local_ipv6 (fe80::1)
• rejects_unique_local_ipv6 (fc00::1)

Two existing tests updated:

• test_object_store_hosts_accepts_ipv4_literal now asserts that a public IP is accepted AND a private IP is rejected, with the docstring documenting the decision evolution.
• test_object_store_hosts_rejects_ipv6_literal now asserts the IP validator catches ::1 with a 'reserved'/'loopback' message instead of the previous IPv6+port heuristic.

Verification

• python3 deploy/mitmproxy/test_allowlist.py: 42/42
• pnpm --filter @fro-bot/gateway test: 82/82
• pnpm test: workspace stays green
• pnpm --filter @fro-bot/gateway lint: 0 errors
• pnpm --filter @fro-bot/gateway check-types: 0 errors
• docker compose -f deploy/compose.yaml config: validates

[fro-bot]

Verdict: CONDITIONAL

Can merge after addressing the CGNAT gap (Blocking #1). Everything else is non-blocking.

Blocking issues

1. CGNAT range (RFC 6598) passes the IP validator on Python 3.11–3.12

Python's ipaddress module does not set is_private, is_reserved, or is_link_local for 100.64.x.x addresses (Shared Address Space / Carrier-Grade NAT). The current guard:

if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_multicast or ip.is_reserved or ip.is_unspecified:

…returns False for all flags on e.g. 100.64.0.1, so that address would be accepted as a "public" IP and written into the allowlist. CGNAT space is routable only inside carrier networks and is reachable from cloud VMs on some providers, so it is a meaningful SSRF surface.

Quickest fix — use not ip.is_global as the primary guard (Python 3.11+ is_global correctly excludes CGNAT, documentation ranges, benchmarking ranges, etc.) and keep the individual flags as belt-and-suspenders:

if not ip.is_global:
    raise ValueError(...)

This is a single-line change in _validate_ip_literal_or_none and would replace the long OR chain with something that is both simpler and more complete. A regression test for 100.64.0.1 should accompany the fix.

Verified locally: python3 -c "import ipaddress; a=ipaddress.ip_address('100.64.0.1'); print(a.is_private, a.is_global, a.is_reserved)" → False False False on Python 3.12.

---

Non-blocking concerns

1. Duplicate public-IP test — test_object_store_hosts_accepts_ipv4_literal (line 374) and test_object_store_hosts_accepts_public_ipv4 (line 405) both assert that 8.8.8.8 is accepted. The former's docstring explains the history (good), but the assertion itself is fully redundant with the latter. Not a correctness issue, just test noise.

2. CI paths-filter: adding deploy/** to the config anchor (line 49) will now trigger should-lint and should-build on any change to the deploy directory. That's the right intent for this PR, but it also means a YAML-only deploy change (e.g. editing compose.yaml) will invoke pnpm lint and pnpm build. Worth confirming that's the desired cost/benefit — deploy Python files won't be linted by pnpm lint anyway, so CI adds overhead without actually checking the Python code. A dedicated python -m pytest step scoped to deploy/mitmproxy/ would give more signal. (Out of scope for this PR; flagging for tracking.)

3. Error message says "private, loopback, link-local, or otherwise reserved" — if the fix above switches to not is_global, the human-readable error should mention CGNAT/shared-address-space so operators aren't confused when their ISP-allocated 100.64.x.x address is rejected.

Missing tests

• 100.64.0.1 (RFC 6598 CGNAT / Shared Address Space) — currently accepted, should be rejected after the fix.
• 192.0.2.1 (TEST-NET-1, RFC 5737) — correctly rejected by is_private today; a test would guard against regressions if the guard logic changes.

Risk assessment (LOW/MED/HIGH): MED

The SSRF surface narrowed substantially vs the prior state (any private IP now rejected). The residual CGNAT gap is real but narrow: an attacker would need deploy-config write access and a CGNAT endpoint worth targeting. The hostname-validation and wildcard-reject paths are correct and well-tested. CI coverage gap (Python tests not in pnpm test) is pre-existing.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	26002260154
Cache	hit
Session	ses_1c84e1ecaffeYXquz54gPOVmY9

[fro-bot]

Verdict: CONDITIONAL

Can merge after addressing the CGNAT gap (Blocking #1). Everything else is non-blocking.

Blocking issues

1. CGNAT range 100.64.0.0/10 (RFC 6598) passes the IP validator on Python 3.11–3.12

Python's ipaddress module does not set is_private, is_reserved, or is_link_local for 100.64.x.x addresses (Shared Address Space / Carrier-Grade NAT). The current guard in _validate_ip_literal_or_none:

if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_multicast or ip.is_reserved or ip.is_unspecified:

…returns False for all flags on e.g. 100.64.0.1, so that address is accepted as a "public" IP and written into the allowlist. CGNAT space is routable only inside carrier networks and is reachable from cloud VMs on some providers — making it a meaningful SSRF surface.

Quickest fix — use not ip.is_global as the primary guard (Python 3.11+ is_global correctly excludes CGNAT, documentation ranges, benchmarking ranges, etc.):

if not ip.is_global:
    raise ValueError(
        f"OBJECT_STORE_HOSTS entry '{entry}' is not a publicly routable IP address ..."
    )

This replaces the long OR chain with something simpler and more complete. A regression test for 100.64.0.1 should accompany the fix.

Verified locally: python3 -c "import ipaddress; a=ipaddress.ip_address('100.64.0.1'); print(a.is_private, a.is_global, a.is_reserved)" → False False False on Python 3.12.

---

Non-blocking concerns

1. Duplicate public-IP test — test_object_store_hosts_accepts_ipv4_literal (line 374) and test_object_store_hosts_accepts_public_ipv4 (line 405) both assert that 8.8.8.8 is accepted. The docstring on the former explains the history well, but the acceptance assertion is fully redundant with the latter. Not a correctness issue, just test noise.

2. CI paths-filter: adding deploy/** to the config anchor will now trigger should-lint and should-build on any change under deploy/. That's the right intent for this PR, but a YAML-only deploy change (e.g. editing compose.yaml) will invoke pnpm lint and pnpm build without those jobs actually checking Python code. A dedicated python -m pytest step scoped to deploy/mitmproxy/ would give more signal. (Out of scope for this PR — flagging for follow-up.)

3. Error message wording: if the guard switches to not is_global, the human-readable error should mention CGNAT/shared-address-space so operators aren't confused when a 100.64.x.x address is rejected with a message that only references RFC 1918 and loopback ranges.

Missing tests

• 100.64.0.1 (RFC 6598 CGNAT / Shared Address Space) — currently accepted, should be rejected after the fix.
• 192.0.2.1 (TEST-NET-1, RFC 5737) — correctly rejected by is_private today; a test would guard against regressions if the guard logic changes.

Risk assessment (LOW/MED/HIGH): MED

The SSRF surface narrowed substantially vs the prior state (any private IP now rejected). The residual CGNAT gap is real but narrow: an attacker would need deploy-config write access and a CGNAT endpoint worth targeting. The hostname-validation and wildcard-reject paths are correct and well-tested. CI coverage gap for Python tests is pre-existing.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	26002260154
Cache	hit
Session	ses_1c84e1ecaffeYXquz54gPOVmY9

[fro-bot]

Verdict: CONDITIONAL

The CGNAT blocking issue from the previous review is not resolved. The commit message for 7f2ba31 accurately describes the correct fix (not ip.is_global) and the rationale, but allowlist.py was not updated — the OR-chain on line 89 is byte-for-byte identical to the previous version. is_global does not appear anywhere in the deploy directory.

Blocking issues

1. CGNAT 100.64.0.0/10 gap still present — fix not applied

deploy/mitmproxy/allowlist.py:89 still reads:

if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_multicast or ip.is_reserved or ip.is_unspecified:

The commit message (7f2ba31) correctly states that 100.64.0.0/10 falls through all of those flags and proposes replacing the guard with not ip.is_global. That change is absent from the file. Verified: grep -r is_global deploy/mitmproxy/ returns no matches.

The fix is a one-line change:

# allowlist.py line 89 — replace the OR chain with:
if not ip.is_global:
    raise ValueError(
        f"OBJECT_STORE_HOSTS entry '{entry}' is not a publicly routable IP address. "
        "Blocked ranges include RFC 1918 private (10/8, 172.16/12, 192.168/16), "
        "loopback (127/8, ::1), link-local (169.254/16, fe80::/10), CGNAT (100.64/10), "
        "documentation (192.0.2/24, 198.51.100/24, 203.0.113/24, 2001:db8::/32), "
        "and other non-globally-routable ranges. "
        "Use a publicly routable IP or a hostname."
    )

The test file also needs the two tests cited in the commit message (rejects_cgnat_ipv4 for 100.64.0.1 and rejects_documentation_ipv4 for 192.0.2.1) — both are absent from test_allowlist.py as well.

---

Non-blocking concerns

Same as prior review — no new ones introduced.

1. Duplicate public-IP test — test_object_store_hosts_accepts_ipv4_literal and test_object_store_hosts_accepts_public_ipv4 both assert 8.8.8.8 acceptance.
2. CI paths-filter — deploy/** in config anchor triggers pnpm lint/pnpm build on Python-only changes without linting the Python code. A python -m pytest step scoped to deploy/mitmproxy/ would give more signal.

Missing tests

• 100.64.0.1 (CGNAT) — must reject after fix is applied.
• 192.0.2.1 (TEST-NET-1, RFC 5737) — should reject (already does via is_private, but a test guards against regression when guard logic changes).

Risk assessment (LOW/MED/HIGH): MED

Same assessment as before. The SSRF surface is meaningfully narrowed vs the pre-PR state. The residual CGNAT gap requires deploy-config write access plus a CGNAT endpoint worth targeting. The commit intent is correct; only the code needs to catch up to the commit message.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	26002412612
Cache	hit
Session	ses_1c84e1ecaffeYXquz54gPOVmY9

[fro-bot]

Verdict: PASS

All blocking issues from the previous reviews are resolved in ba57eb6.

allowlist.py:94 now reads if not ip.is_global: — correctly rejecting CGNAT (100.64.0.0/10), documentation ranges, benchmarking ranges, and all other non-globally-routable addresses in a single, complete guard. The error message explicitly names CGNAT and documentation ranges so operators get actionable feedback. Tests pass 44/44 including the new rejects_cgnat_ipv4, rejects_documentation_ipv4, and rejects_documentation_ipv6 regression guards.

Blocking issues

None.

Non-blocking concerns

1. Duplicate public-IP acceptance test — test_object_store_hosts_accepts_ipv4_literal (line 374) and the acceptance leg of this PR both cover 8.8.8.8. The former is justified by its history-documenting docstring; the duplication is benign.
2. CI paths-filter — deploy/** in the config anchor still triggers pnpm lint/pnpm build on Python-only deploy changes without running python -m pytest. Pre-existing gap, flagged in prior reviews for follow-up.

Missing tests

None — the two tests specifically called out in prior reviews (rejects_cgnat_ipv4 and rejects_documentation_ipv4) are both present and passing, plus an additional rejects_documentation_ipv6 for 2001:db8::1.

Risk assessment (LOW/MED/HIGH): LOW

The guard change is a strict tightening — it rejects a superset of what the OR-chain rejected, and the is_global semantics are stable across Python 3.11+. Public IPs (8.8.8.8, 2001:4860:4860::8888) continue to pass. The hostname and wildcard paths are unchanged. Test coverage is thorough.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/agent
Run ID	26002711440
Cache	hit
Session	ses_1c84e1ecaffeYXquz54gPOVmY9