fix(policy): classify advisory private-IP notes with the canonical is_internal_ip (#1777)

[latenighthackathon]

Summary

When OpenShell proposes a network-policy rule it annotates the rule with a security note flagging whether the destination looks like an internal/private address. That check used naive string prefixes - notably host.starts_with("172."), which matches all of 172.0.0.0-172.255.255.255 even though only 172.16.0.0/12 is private. So the advisory flagged public addresses (172.15.x, 172.32.x) as internal, missed ranges like CGNAT (100.64.0.0/10) and IPv6 ULA entirely, and even flagged non-IP hostnames that merely start with 10./172.. OpenShell already has a canonical, RFC-accurate, unit-tested classifier for exactly this (openshell-core net::is_internal_ip), and the runtime SSRF enforcement path already uses it. This routes the advisory's two copies of generate_security_notes() through that same classifier, so the proposed-rule note matches what enforcement actually does.

Related Issue

Closes #1777

Changes

• In crates/openshell-sandbox/src/mechanistic_mapper.rs and crates/openshell-server/src/grpc/policy.rs, replace the duplicated host.starts_with("10.") || host.starts_with("172.") || ... block in generate_security_notes() with host.parse::<IpAddr>().is_ok_and(is_internal_ip) (the "localhost" hostname is not an IP literal, so it stays a separate check). This fixes the 172.16.0.0/12 boundary, adds CGNAT / IPv6 ULA / other reserved coverage, and stops flagging hostnames that merely begin with a private prefix.
• Unit tests in both crates asserting 172.15/172.32 are not flagged, 172.16 and 100.64 (CGNAT) are, the classic ranges plus localhost still are, and a public IP (8.8.8.8) is not.

Testing

Ran the full gate in a container matching CI (rust 1.95.0 per rust-toolchain.toml plus the mise toolchain):

• mise run pre-commit passes (rust format/lint/check, test:rust, python lint/typecheck, helm, markdown, license).
• Unit tests added in both crates and passing.

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Architecture docs: N/A (no bootstrap/manifest/entrypoint change).

[johntmyers]

gator-agent

PR Review Status

Validation: this PR is project-valid because it directly addresses validated issue #1777 with a small, concentrated fix in the policy advisory note paths.
Head SHA: 8c0f70e6994d67846dfad060b44968a3eb890fca

Review findings:

• Please add direct regression coverage for the hostname false positives called out in bug: flaky private/reserved IP detection in policy advisory security notes #1777. The implementation fixes this by parsing IP literals before calling is_internal_ip, but the new tests do not currently assert that hosts such as 10.example.com or 172.example.com are not flagged as internal/private.
• Please also add a direct IPv6 ULA advisory-note assertion, for example fd00::1 or fd12:3456:789a:1::1, in both touched generate_security_notes() test suites.

Docs: not needed; this corrects advisory text accuracy and does not change a CLI/API/workflow surface or documented configuration.

Checks: DCO, vouch-gate, Branch Checks, Helm Lint, Rust, Python, license, and markdown are passing for this head. No test:* labels are currently required.

Next state: gator:in-review

[alangou]

I would suggest to also modify is_internal_v4 in net.rs to use as much as we can functions from the rust standard library

Ipv4Addr in core::net - Rust  RFC 1918 (private ips)
Ipv4Addr in core::net - Rust RFC 1122 (loopback)
Ipv4Addr in core::net - Rust RFC 3927 (APIPA range)
Ipv4Addr in core::net - Rust RFC 5737 (reserved for documentation)
Ipv4Addr in core::net - Rust RFC 919 (broadcast ip)

missing function from the stable standard library that would need to be implemented manually in is_internal_v4

Ipv4Addr in core::net - Rust RFC 2544 (benchmarking)
Ipv4Addr in core::net - Rust RFC 6598 (CGNAT ips)
Ipv4Addr in core::net - Rust RFC 1112 (reserved ips)

[johntmyers]

gator-agent

Re-check After Maintainer Update

I re-evaluated latest head 8c0f70e6994d67846dfad060b44968a3eb890fca after @alangou's 2026-06-09 08:57 UTC comment suggesting is_internal_v4 should use available stable std::net::Ipv4Addr predicates where possible.

Disposition: not resolved yet. There have been no author commits after the prior gator review, and the independent reviewer confirmed the implementation direction is correct but the review feedback still needs an author update or maintainer waiver.

Remaining items:

• Add direct regression coverage in both touched generate_security_notes() test suites showing hostname prefix false positives such as 10.example.com and 172.example.com are not flagged as internal/private.
• Add direct IPv6 ULA advisory-note coverage in both touched test suites, for example fd00::1 or fd12:3456:789a:1::1.
• Address @alangou's is_internal_v4 suggestion, or clarify why that classifier cleanup should remain out of scope for this PR.

Checks remain green for this head. No test:* labels are currently required.

Next state: gator:in-review

[latenighthackathon]

Thanks @alangou, @johntmyers — applied.

is_internal_v4 now uses the stable Ipv4Addr predicates as far as they go: is_documentation() (RFC 5737) replaces the two manual TEST-NET-2/3 checks and also closes a gap, since 192.0.2.0/24 (TEST-NET-1) was previously unflagged, plus is_broadcast(). Manual checks remain only for the ranges without a stable predicate: CGNAT 100.64.0.0/10, IETF protocol assignments 192.0.0.0/24, and benchmarking 198.18.0.0/15.

Added regression coverage: both note suites now assert hostname-prefix false positives (10.example.com, 172.example.com) are not flagged and IPv6 ULA (fd00::1) is flagged; net.rs covers the newly classified TEST-NET-1 and broadcast addresses.

Cheers!