feat(#123): add IP-level SSRF defense to Gitea client and action #129

Merged

rodin merged 5 commits from issue-123 into main 2026-05-14 19:10:20 +00:00

Conversation 18 Commits 5 Files Changed 11

+852 -51

5 Commits

Author	SHA1	Message	Date
Rodin	c349986187	fix(#123): add RFC6598 CGN check to Python SSRF validation in action.yml ... Python's ipaddress module does NOT classify 100.64.0.0/10 (RFC6598 carrier-grade NAT) as private/loopback/link_local/multicast/reserved. This means a SERVER_URL resolving to a CGN address would bypass the Python SSRF check and reach curl with ACTION_TOKEN. Add an explicit network membership check for 100.64.0.0/10 to both Python validation blocks in action.yml: - _ssrf_check.py (VCS URL pre-flight check) - _ssrf_check_install.py (binary download URL check) The Go-level IsBlockedIP already covers this range correctly (ipcheck.go); this fix closes the gap in the action.yml Python layer. Also update comments to mention RFC6598 explicitly.	2026-05-14 13:41:17 +00:00
claw	934c6728ee	fix(#123): address review feedback on SSRF defense ... - Clone http.DefaultTransport instead of bare &http.Transport{} to preserve ProxyFromEnvironment, TLSHandshakeTimeout, IdleConnTimeout, connection pooling, and HTTP/2 support (fixes transport regression). - Add IPv6-mapped IPv4 normalization in action.yml Python SSRF checks to prevent bypass via ::ffff:10.0.0.1 style AAAA records. - Reject URLs with user-info (user:pass@host) in action.yml Python checks to match validate-url subcommand behavior. - Add test verifying DefaultTransport settings are preserved.	2026-05-14 04:49:21 -07:00
claw	5ac93bea70	fix(#123): add IP fallback dialing in safeDialContext ... Previously safeDialContext only dialed the first resolved IP. If the connection failed, it returned an error without trying other IPs. Now it iterates all validated IPs and returns the first successful connection, or the last error if all fail. This matches the resilience behavior of a plain net.Dialer on multi-IP hostnames. Addresses review finding: safeDialContext only dials first resolved IP. All IPs are still validated before any dial attempt is made.	2026-05-14 01:44:32 -07:00
claw	f84cc3bbcf	fix(#123): address all review findings from PR #129 ... MAJOR fixes: - gitea/ipcheck.go: replace startup panic with init()+error list pattern Hard-coded CIDRs that fail to parse now recorded in blockedCIDRParseErrors instead of panicking. TestBlockedCIDRsValid catches programming errors in CI without violating CONVENTIONS.md 'never panic' rule. - .gitea/actions/review/action.yml: re-validate SERVER_URL at start of 'Install review-bot' step to close DNS rebinding window between 'Determine version' and install-step curl calls. MINOR fixes: - gitea/client.go: add Timeout: 10*time.Second to net.Dialer per PLAN.md spec - cmd/review-bot/validateurl.go: switch isValidateError to errors.As so wrapped *validateError values are also detected - gitea/ipcheck_test.go: clarify 198.51.100.1 (RFC5737 TEST-NET-2) comment; add TestBlockedCIDRsValid to surface CIDR parse errors as test failures NIT fixes: - .gitea/actions/review/action.yml: refactor Python list comprehension in SSRF check to for-loop (avoids side-effect-only comprehension, runner compat) - gitea/export_test.go: expand comment explaining white-box test pattern (why package gitea not gitea_test, Go stdlib precedent) Remove PLAN.md (implementation complete)	2026-05-14 01:42:47 -07:00
aweiker	8c8f3ab4b3	feat(#123): add IP-level SSRF defense to Gitea client and action ... ## Changes ### Go: IP-level SSRF protection in gitea.Client (primary defense) - Add gitea/ipcheck.go with IsBlockedIP() covering all blocked CIDR ranges: loopback (127.0.0.0/8, ::1), RFC1918 (10/8, 172.16/12, 192.168/16), link-local (169.254/16, fe80::/10), ULA (fc00::/7), CGN (100.64/10), multicast, reserved, and unspecified ranges. - IPv6-mapped IPv4 addresses (::ffff:x.x.x.x) are normalized before checking. - Add safeDialContext to gitea.Client: resolves DNS, rejects any IP in a blocked CIDR, then dials the resolved IP directly to narrow the DNS rebinding window. NewClient now uses this safe transport by default. - Add WithUnsafeDialer() for test code using httptest.Server (127.0.0.1). - Update NewTestClient helper in export_test.go for all gitea unit tests. - Update SetHTTPClient(nil) to restore the safe transport (not the plain one). ### Go: validate-url subcommand (defense-in-depth for future bash callers) - Add 'review-bot validate-url <url>' subcommand: validates https scheme, no user-info, resolves hostname, rejects any blocked IP. - Exit 0=safe, 1=blocked, 2=validation error/dns failure. - Add outWriter/errWriter vars to main.go for testable output capture. ### action.yml: Python3 IP check in 'Determine version' step - After the https scheme validation, resolve SERVER_URL hostname with socket.getaddrinfo and reject any result where ipaddress.ip_address(ip).is_private/is_loopback/is_link_local/etc. is true. - python3 is required on ubuntu-* runners (noted in existing comments). - Covers the version-check curl that sends ACTION_TOKEN to SERVER_URL. - SERVER_URL for install-step curls is covered by the same pre-check. ### Tests - gitea/ipcheck_test.go: 30+ cases covering all blocked families + public IPs - gitea/client_test.go: safe transport presence, WithUnsafeDialer, SSRF blocking - cmd/review-bot/validateurl_test.go: scheme validation, user-info, exit codes Closes #123	2026-05-14 07:44:51 +00:00