rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 6 Wiki Activity

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
+8 -5
1 changed files with 8 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit c349986187 - Show all commits
.gitea/actions/review/action.yml
+8 -5
View File
@@ -10,8 +10,8 @@
# github.server_url. Tokens are never sent to user-supplied URLs. # github.server_url. Tokens are never sent to user-supplied URLs.
# - On Gitea (VCS_TYPE=gitea), inputs.vcs-url is validated (https scheme, # - On Gitea (VCS_TYPE=gitea), inputs.vcs-url is validated (https scheme,
# no whitespace/newlines, and DNS resolution to a public IP) before use. # no whitespace/newlines, and DNS resolution to a public IP) before use.
# Python3 resolves the hostname and rejects RFC1918, loopback, link-local, # Python3 resolves the hostname and rejects RFC1918, RFC6598 (carrier-grade
# and other reserved addresses to prevent SSRF attacks. # NAT), loopback, link-local, and other reserved addresses to prevent SSRF attacks.
# The installed review-bot binary additionally uses a safe HTTP transport # The installed review-bot binary additionally uses a safe HTTP transport
# (DialContext-level IP check) for all Gitea API calls at runtime. # (DialContext-level IP check) for all Gitea API calls at runtime.
# The binary also exposes a `validate-url` subcommand for use in any future # The binary also exposes a `validate-url` subcommand for use in any future
4
 @@ -193,7 +193,8 @@ runs:
fi fi
# Additional IP-level SSRF defense: resolve the hostname and reject # Additional IP-level SSRF defense: resolve the hostname and reject
# requests to RFC1918, loopback, link-local, and other reserved addresses. # requests to RFC1918, RFC6598 (carrier-grade NAT), loopback, link-local,
# and other reserved addresses.
# python3 is required on ubuntu-* runners (see requirements comment above). # python3 is required on ubuntu-* runners (see requirements comment above).
# Use printf to write the script to a temp file so the python lines are valid # Use printf to write the script to a temp file so the python lines are valid
# YAML (each indented line becomes a printf argument — no unindented code). # YAML (each indented line becomes a printf argument — no unindented code).
gpt-review-bot commented 2026-05-14 07:48:23 +00:00
Review
Copy Link
Copy Source

[NIT] The Python SSRF check uses a list comprehension with side effects (sys.exit(1)), which hurts readability and can surprise maintainers. A simple for-loop with explicit exits would be clearer and easier to audit.

**[NIT]** The Python SSRF check uses a list comprehension with side effects (`sys.exit(1)`), which hurts readability and can surprise maintainers. A simple for-loop with explicit exits would be clearer and easier to audit.
gpt-review-bot commented 2026-05-14 11:52:20 +00:00
Review
Copy Link
Copy Source

[NIT] The inline Python script uses a Unicode em dash (—) in an error message. While runners typically use UTF-8, non-ASCII characters in shell-embedded scripts can cause encoding issues in some environments. Consider using a plain hyphen '-' for maximum portability.

**[NIT]** The inline Python script uses a Unicode em dash (—) in an error message. While runners typically use UTF-8, non-ASCII characters in shell-embedded scripts can cause encoding issues in some environments. Consider using a plain hyphen '-' for maximum portability.
5
 @@ -212,7 +213,8 @@ runs:
'for _,_,_,_,(a,*_) in rs:' \ 'for _,_,_,_,(a,*_) in rs:' \
' ip=ipaddress.ip_address(a)' \ ' ip=ipaddress.ip_address(a)' \
' if isinstance(ip,ipaddress.IPv6Address) and ip.ipv4_mapped: ip=ip.ipv4_mapped' \ ' if isinstance(ip,ipaddress.IPv6Address) and ip.ipv4_mapped: ip=ip.ipv4_mapped' \
' if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_multicast or ip.is_reserved:' \ ' cgn=ipaddress.ip_network("100.64.0.0/10")' \
' if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_multicast or ip.is_reserved or ip in cgn:' \
' print(f"blocked: {a}",file=sys.stderr); sys.exit(1)' \ ' print(f"blocked: {a}",file=sys.stderr); sys.exit(1)' \
> /tmp/_ssrf_check.py > /tmp/_ssrf_check.py
CHECK_URL="${SERVER_URL}" python3 /tmp/_ssrf_check.py || { CHECK_URL="${SERVER_URL}" python3 /tmp/_ssrf_check.py || {
4
 @@ -359,7 +361,8 @@ runs:
'for _,_,_,_,(a,*_) in rs:' \ 'for _,_,_,_,(a,*_) in rs:' \
' ip=ipaddress.ip_address(a)' \ ' ip=ipaddress.ip_address(a)' \
' if isinstance(ip,ipaddress.IPv6Address) and ip.ipv4_mapped: ip=ip.ipv4_mapped' \ ' if isinstance(ip,ipaddress.IPv6Address) and ip.ipv4_mapped: ip=ip.ipv4_mapped' \
' if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_multicast or ip.is_reserved:' \ ' cgn=ipaddress.ip_network("100.64.0.0/10")' \
' if ip.is_private or ip.is_loopback or ip.is_link_local or ip.is_multicast or ip.is_reserved or ip in cgn:' \
' print(f"blocked: {a}",file=sys.stderr); sys.exit(1)' \ ' print(f"blocked: {a}",file=sys.stderr); sys.exit(1)' \
> /tmp/_ssrf_check_install.py > /tmp/_ssrf_check_install.py
CHECK_URL="${SERVER_URL}" python3 /tmp/_ssrf_check_install.py || { CHECK_URL="${SERVER_URL}" python3 /tmp/_ssrf_check_install.py || {