Harden internal endpoints: allow-list, /config scrub, loopback bind#239
Merged
Conversation
Security: - INTERNAL_ALLOW_IPS CIDR allow-list gates the internal router: a peer outside the list gets 403 before routing on /metrics, /config, plugin /__ routes and unknown paths, so it cannot probe which paths exist. Health endpoints (/health, /healthz, /readyz, /startupz and their long forms) are always served so orchestrator and load-balancer checks never break. Unset/empty allows all; loopback is not implicit (list 127.0.0.1/32 to keep localhost). A malformed list aborts startup. No bearer-token option by design — a token invites exposing the port "because it's protected". - Capture the accepted peer IP (previously discarded) and canonicalize IPv4-mapped IPv6 before matching. - /config no longer reports internal_addr or error_pages_dir — deployment topology and filesystem paths that aid an attacker and are not needed by metrics scrapers. - Private-aware startup warning: an off-host bind (0.0.0.0/::/public) with no INTERNAL_ALLOW_IPS warns; a private interface logs info; loopback is silent; the warning is suppressed once an allow-list is set, so it marks real exposure instead of firing for every deployment. Config: - A port-only INTERNAL_ADDR (:9090) now binds 127.0.0.1 instead of failing to resolve; bind an explicit 0.0.0.0:9090 to expose it off-host. Additive — fully-qualified values pass through unchanged. Refactor: - Extract parse_cidr_list(val, var_name), shared by TRUSTED_PROXIES and INTERNAL_ALLOW_IPS; add IpAllowList and the BindExposure classifier. Error messages keep the underlying CIDR parse cause. Docs: - README (en/ru/zh) and CHANGELOG document INTERNAL_ALLOW_IPS, the loopback default, the /config scrub, the exposed-bind warning, and the always-allowed health endpoints. 15 tests (15 unit).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds access control to the internal server (
/health*,/metrics,/config), which is served by a separate Rust listener that PHP never touches — so it can only be guarded in-process.What changed
INTERNAL_ALLOW_IPSallow-list (new)403before routing on/metrics,/config, plugin/__*routes, and unknown paths — so an outsider cannot even probe which paths exist./health,/healthz,/readyz,/startupzand their long forms) are always served, so an orchestrator liveness/readiness probe or a load-balancer health check is never blocked into killing a healthy backend.127.0.0.1/32to keep localhost/sidecar access. A malformed list aborts startup.Loopback-default bind
INTERNAL_ADDR(e.g.:9090) now binds127.0.0.1instead of failing to resolve. Bind an explicit0.0.0.0:9090to expose it off-host. Additive — fully-qualified values are unchanged, so this is not a breaking change./configscrub/configno longer reportsinternal_addrorerror_pages_dir— deployment topology and filesystem paths that aid an attacker and are not needed by metrics scrapers.Private-aware startup warning
0.0.0.0,::, or a public address) with noINTERNAL_ALLOW_IPSlogs awarn!; a private-interface bind logsinfo!; loopback is silent. The warning is suppressed once an allow-list is set, so it signals real exposure instead of firing for every deployment.Implementation notes
parse_cidr_listreused byTRUSTED_PROXIESandINTERNAL_ALLOW_IPS; newIpAllowListand aBindExposureclassifier sit alongside it.Verification
cargo fmt -- --check,cargo clippy --no-default-features -- -D warnings: clean.cargo test --no-default-features: 863 unit + 10 integration passing (15 new unit tests).0.0.0.0with no allow-list warns; a port-only:9099(coerced to loopback) is silent;0.0.0.0+ allow-list is silent.Docs
README (en/ru/zh) and CHANGELOG document the new variable, the loopback default, the scrub, the warning, and the always-allowed health endpoints.