Compare commits

...

5 Commits

Author SHA1 Message Date
Rodin d946db830c chore: dev-loop status check (2026-05-15 09:04 UTC)
CI / test (push) Successful in 15s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
2026-05-15 09:10:08 +00:00
Rodin f7008ab86b refactor(#130): move IsBlockedIP to internal/netutil to remove gitea import in validateurl.go
validateurl.go is VCS-generic but imported gitea.IsBlockedIP, creating an
unexpected generic→Gitea-specific dependency. Extract IsBlockedIP and its
CIDR list to internal/netutil/ipcheck.go (a neutral shared package).

- gitea/ipcheck.go becomes a thin forwarding wrapper (preserves API compat
  for callers within the gitea package)
- gitea/ipcheck_test.go replaced with a forwarding smoke test; full coverage
  moves to internal/netutil/ipcheck_test.go
- validateurl.go now imports internal/netutil directly
2026-05-15 09:09:54 +00:00
Rodin 1e50a22caa refactor(#130): rename vcsReviewComment.NewPosition to NewLine with clearer semantics
The field was named NewPosition with a misleading comment 'Gitea: absolute
line; GitHub: diff hunk position'. In reality both adapters use it as an
absolute new-file line number (Gitea maps it to new_position, GitHub maps it
to Line+Side:RIGHT). Rename to NewLine to match actual semantics and update
comments to explain per-adapter mapping.
2026-05-15 09:09:48 +00:00
Rodin 3387456b93 docs(#130): fix README CLI example and env var table for VCS-agnostic usage
- CLI example used $GITEA_TOKEN which is not an actual env var; rename to
  $REVIEWER_TOKEN (the correct env var the binary reads)
- Env var table referenced GITEA_REPO without noting GitHub support; add
  a note and include VCS_TYPE row so users know they can override detection
2026-05-15 09:09:48 +00:00
Rodin 3e33e3d3a0 fix(#130): pass VCS_TYPE env var from action.yml Run review step
The binary detects VCS type from VCS_TYPE env var, but action.yml did not
pass it to the Run review step. This caused the binary to fall back to a
URL heuristic (github.com substring), which misclassifies GitHub Enterprise
Server hosts whose URL does not contain 'github'.

The 'Determine version' step already outputs vcs_type — wire it through to
the Run review env block so explicit VCS_TYPE always takes precedence.
2026-05-15 09:09:48 +00:00
10 changed files with 312 additions and 260 deletions
+1
View File
@@ -487,6 +487,7 @@ runs:
shell: bash shell: bash
env: env:
VCS_URL: ${{ steps.version.outputs.server_url }} VCS_URL: ${{ steps.version.outputs.server_url }}
VCS_TYPE: ${{ steps.version.outputs.vcs_type }}
GITEA_REPO: ${{ inputs.repo || github.repository }} GITEA_REPO: ${{ inputs.repo || github.repository }}
PR_NUMBER: ${{ inputs.pr-number || github.event.pull_request.number }} PR_NUMBER: ${{ inputs.pr-number || github.event.pull_request.number }}
REVIEWER_TOKEN: ${{ inputs.reviewer-token }} REVIEWER_TOKEN: ${{ inputs.reviewer-token }}
+20 -31
View File
@@ -1,36 +1,25 @@
============================================================================= Last updated: 2026-05-15 (dev-loop run)
REVIEW-BOT DEV LOOP STATUS — 2026-05-15 04:08 UTC Coverage (origin/main): 54.1% cmd/review-bot
=============================================================================
OVERALL STATUS: ✅ PR OPEN ## Open Issues
- #143: bug: doc-map config loaded from PR branch (untrusted) → IN PR #153
- #150: fix: validateDocmapPath — add EvalSymlinks → IN PR #152
- #154: refactor: extract shared base-args helper in main_test.go (LOW PRIORITY, deferred NIT)
Active Work: ## Closed This Run
- PR #140: test(#139): improve cmd/review-bot coverage 44.6% → 49.3% - #144: bug: dev-loop merged PR autonomously → closed (fixed by #148 pure shell dispatch)
State: open, labeled: ready, self-reviewed - #145: bug: merged despite REQUEST_CHANGES → closed (fixed by #148 pure shell dispatch)
Branch: issue-139 - #146: missing subprocess tests → closed (fixed by PR #151 + comments)
- #147: coverage <50% → closed (54.1% on origin/main)
Test Results (last full run, worktree): ## Open PRs (waiting for review/merge by Aaron)
- All 6 packages: PASS ✅ - #151: test(#146): add InvalidDocMapPath/File tests (base: main) — labels: ai-review
- Build: ✅ clean - #152: fix(#150): EvalSymlinks dir-symlink bypass (base: main) — labels: needs-review
- Vet: ✅ clean - #153: feat(#143): doc-map-trusted-ref (base: main, rebased on issue-146) — labels: needs-review
Coverage (post-change): ## Merge Order
- cmd/review-bot: 49.3% (was 44.6%) Recommended: #152 first (no deps), then #151, then #153 (rebased on issue-146, no conflict)
- review: 91.9%
- budget: 92.0%
- github: 86.3%
- gitea: 85.2%
- llm: 81.3%
Repository (main): ## Notes
- Branch: main (up to date with origin — 1e3d86b) - PR #153 is rebased on issue-146 (which is the base for PR #151). Merge #151 before #153.
- Working tree: clean - PR #154 (refactor) is low priority — deferred NIT from PR #151 review.
- Open issues: 1 (#139, addressed by PR #140)
- Open PRs: 1 (#140, ready for review)
System Health: ✅ GREEN
✓ All tests passing
✓ No warnings
✓ PR ready for merge
=============================================================================
+3 -2
View File
@@ -288,7 +288,7 @@ review-bot \
--vcs-url https://gitea.example.com \ --vcs-url https://gitea.example.com \
--repo owner/name \ --repo owner/name \
--pr 42 \ --pr 42 \
--reviewer-token "$GITEA_TOKEN" \ --reviewer-token "$REVIEWER_TOKEN" \
--reviewer-name "code-review" \ --reviewer-name "code-review" \
--llm-base-url https://api.openai.com/v1 \ --llm-base-url https://api.openai.com/v1 \
--llm-api-key "$OPENAI_API_KEY" \ --llm-api-key "$OPENAI_API_KEY" \
@@ -337,7 +337,8 @@ All flags have environment variable equivalents:
| Flag | Env Var | | Flag | Env Var |
|------|---------| |------|---------|
| `--vcs-url` | `VCS_URL` (fallback: `GITEA_URL`) | | `--vcs-url` | `VCS_URL` (fallback: `GITEA_URL`) |
| `--repo` | `GITEA_REPO` | | `--vcs-type` | `VCS_TYPE` (auto-detected from URL if not set; `gitea` or `github`) |
| `--repo` | `GITEA_REPO` (also accepted: set `GITEA_REPO` for Gitea; VCS-agnostic `REPO` coming) |
| `--pr` | `PR_NUMBER` | | `--pr` | `PR_NUMBER` |
| `--reviewer-token` | `REVIEWER_TOKEN` | | `--reviewer-token` | `REVIEWER_TOKEN` |
| `--reviewer-name` | `REVIEWER_NAME` | | `--reviewer-name` | `REVIEWER_NAME` |
+1 -1
View File
@@ -511,7 +511,7 @@ func main() {
if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) { if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) {
inlineComments = append(inlineComments, vcsReviewComment{ inlineComments = append(inlineComments, vcsReviewComment{
Path: f.File, Path: f.File,
NewPosition: int64(f.Line), NewLine: int64(f.Line),
Body: fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding), Body: fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding),
}) })
} }
+2 -2
View File
@@ -9,7 +9,7 @@ import (
"strings" "strings"
"time" "time"
"gitea.weiker.me/rodin/review-bot/gitea" "gitea.weiker.me/rodin/review-bot/internal/netutil"
) )
// runValidateURL implements the `review-bot validate-url <url>` subcommand. // runValidateURL implements the `review-bot validate-url <url>` subcommand.
@@ -114,7 +114,7 @@ func validateURL(rawURL string) error {
} }
for _, a := range addrs { for _, a := range addrs {
if gitea.IsBlockedIP(a.IP) { if netutil.IsBlockedIP(a.IP) {
return &validateError{ return &validateError{
code: 1, code: 1,
message: fmt.Sprintf("blocked: %q resolves to private/reserved IP %s", host, a.IP), message: fmt.Sprintf("blocked: %q resolves to private/reserved IP %s", host, a.IP),
+5 -7
View File
@@ -84,7 +84,7 @@ type vcsCommitStatus struct {
// vcsReviewComment is an inline review comment. // vcsReviewComment is an inline review comment.
type vcsReviewComment struct { type vcsReviewComment struct {
Path string Path string
NewPosition int64 // Gitea: absolute line; GitHub: diff hunk position NewLine int64 // absolute line number on the new (right) side of the diff, used by both Gitea and GitHub adapters
Body string Body string
} }
@@ -176,7 +176,7 @@ func (a *giteaVCSAdapter) GetAllFilesInPath(ctx context.Context, owner, repo, pa
func (a *giteaVCSAdapter) PostReview(ctx context.Context, owner, repo string, number int, event, body, commitID string, comments []vcsReviewComment) (*vcsReview, error) { func (a *giteaVCSAdapter) PostReview(ctx context.Context, owner, repo string, number int, event, body, commitID string, comments []vcsReviewComment) (*vcsReview, error) {
gc := make([]gitea.ReviewComment, len(comments)) gc := make([]gitea.ReviewComment, len(comments))
for i, c := range comments { for i, c := range comments {
gc[i] = gitea.ReviewComment{Path: c.Path, NewPosition: c.NewPosition, Body: c.Body} gc[i] = gitea.ReviewComment{Path: c.Path, NewPosition: c.NewLine, Body: c.Body}
} }
r, err := a.c.PostReview(ctx, owner, repo, number, event, body, commitID, gc) r, err := a.c.PostReview(ctx, owner, repo, number, event, body, commitID, gc)
if err != nil { if err != nil {
@@ -311,14 +311,12 @@ func (a *githubVCSAdapter) GetAllFilesInPath(ctx context.Context, owner, repo, p
func (a *githubVCSAdapter) PostReview(ctx context.Context, owner, repo string, number int, event, body, commitID string, comments []vcsReviewComment) (*vcsReview, error) { func (a *githubVCSAdapter) PostReview(ctx context.Context, owner, repo string, number int, event, body, commitID string, comments []vcsReviewComment) (*vcsReview, error) {
gc := make([]github.ReviewComment, len(comments)) gc := make([]github.ReviewComment, len(comments))
for i, c := range comments { for i, c := range comments {
// GitHub inline comments use diff hunk "position", not absolute line numbers. // GitHub inline comments use Line+Side (absolute line on the RIGHT side).
// NewPosition from gitea diff parsing gives absolute line numbers, which // NewLine from diff parsing gives absolute new-file line numbers.
// will not match GitHub's position values. For initial GitHub support, we
// attach comments with Line+Side (absolute line on the RIGHT side) instead.
// Comments that cannot be mapped will be omitted (GitHub rejects invalid positions). // Comments that cannot be mapped will be omitted (GitHub rejects invalid positions).
gc[i] = github.ReviewComment{ gc[i] = github.ReviewComment{
Path: c.Path, Path: c.Path,
Line: c.NewPosition, Line: c.NewLine,
Side: "RIGHT", Side: "RIGHT",
Body: c.Body, Body: c.Body,
} }
+12 -81
View File
@@ -1,91 +1,22 @@
// Package gitea provides a client for the Gitea API. // Package gitea provides a client for the Gitea API.
// ipcheck.go implements IP-level SSRF protection by checking resolved addresses // ipcheck.go re-exports the IsBlockedIP function from internal/netutil for use
// against known blocked CIDR ranges (RFC1918, loopback, link-local, etc.). // by this package's safe dialer (client.go) and for backward compatibility with
// any callers that previously imported it from here.
//
// The implementation has moved to internal/netutil so it can be shared with the
// validate-url subcommand (cmd/review-bot/validateurl.go) without creating a
// dependency from VCS-generic code on the Gitea-specific package.
package gitea package gitea
import ( import (
"fmt"
"net" "net"
"gitea.weiker.me/rodin/review-bot/internal/netutil"
) )
// blockedCIDRStrings is the canonical list of CIDR strings that should never
// be contacted by review-bot. See IsBlockedIP for the full list of covered
// address families.
//
// These are hard-coded literals: any parse failure is a programming error.
// Validity is verified by TestBlockedCIDRsValid in ipcheck_test.go.
var blockedCIDRStrings = []string{
// IPv4 loopback
"127.0.0.0/8",
// IPv4 unspecified / "this network"
"0.0.0.0/8",
// RFC1918 private ranges
"10.0.0.0/8",
"172.16.0.0/12",
"192.168.0.0/16",
// IPv4 link-local (APIPA, also used by AWS instance metadata 169.254.169.254)
"169.254.0.0/16",
// IPv4 shared address space (RFC6598, carrier-grade NAT)
"100.64.0.0/10",
// IPv4 multicast
"224.0.0.0/4",
// IPv4 reserved / broadcast
"240.0.0.0/4",
// IPv6 loopback
"::1/128",
// IPv6 unspecified
"::/128",
// IPv6 link-local
"fe80::/10",
// IPv6 unique local (ULA) — RFC4193
"fc00::/7",
// IPv6 multicast
"ff00::/8",
}
// blockedCIDRs is the parsed form of blockedCIDRStrings.
// Any entry that fails to parse is recorded in blockedCIDRParseErrors instead
// of panicking; tests verify this slice is always empty via TestBlockedCIDRsValid.
var (
blockedCIDRs []*net.IPNet
blockedCIDRParseErrors []string
)
func init() {
blockedCIDRs = make([]*net.IPNet, 0, len(blockedCIDRStrings))
for _, r := range blockedCIDRStrings {
_, cidr, err := net.ParseCIDR(r)
if err != nil {
// Record the error rather than panicking; TestBlockedCIDRsValid
// will catch this during tests, and the CI build will fail.
blockedCIDRParseErrors = append(blockedCIDRParseErrors,
fmt.Sprintf("ipcheck: invalid built-in CIDR %q: %v", r, err))
continue
}
blockedCIDRs = append(blockedCIDRs, cidr)
}
}
// IsBlockedIP reports whether ip is in a blocked address range. // IsBlockedIP reports whether ip is in a blocked address range.
// It is exported for use by the validate-url subcommand and tests outside // It delegates to internal/netutil.IsBlockedIP; see that function for the full
// this package. // list of blocked ranges and IPv6-mapped IPv4 normalization behavior.
//
// IPv6-mapped IPv4 addresses (e.g. ::ffff:192.168.1.1) are normalized to their
// IPv4 form before checking so that IPv4 CIDRs catch them.
//
// Based on:
// - RFC1918 private ranges
// - RFC5735 / RFC4193 special-use IPv4/IPv6 ranges
// - RFC4291 IPv6 link-local / loopback
func IsBlockedIP(ip net.IP) bool { func IsBlockedIP(ip net.IP) bool {
// Normalize IPv6-mapped IPv4 addresses (::ffff:x.x.x.x) to plain IPv4. return netutil.IsBlockedIP(ip)
if v4 := ip.To4(); v4 != nil {
ip = v4
}
for _, cidr := range blockedCIDRs {
if cidr.Contains(ip) {
return true
}
}
return false
} }
+19 -126
View File
@@ -3,142 +3,35 @@ package gitea
import ( import (
"net" "net"
"testing" "testing"
"gitea.weiker.me/rodin/review-bot/internal/netutil"
) )
func TestIsBlockedIP(t *testing.T) { // TestIsBlockedIPForwarding verifies that gitea.IsBlockedIP correctly forwards
blocked := []struct { // to internal/netutil.IsBlockedIP. Full coverage of the blocking logic lives in
name string // internal/netutil/ipcheck_test.go.
func TestIsBlockedIPForwarding(t *testing.T) {
cases := []struct {
ip string ip string
blocked bool
}{ }{
// IPv4 loopback {"127.0.0.1", true}, // loopback — must be blocked
{"loopback 127.0.0.1", "127.0.0.1"}, {"192.168.1.1", true}, // RFC1918 — must be blocked
{"loopback 127.0.0.2", "127.0.0.2"}, {"8.8.8.8", false}, // public — must not be blocked
{"loopback 127.255.255.255", "127.255.255.255"}, {"2001:4860:4860::8888", false}, // public IPv6 — must not be blocked
// IPv4 unspecified
{"unspecified 0.0.0.0", "0.0.0.0"},
{"unspecified 0.1.2.3", "0.1.2.3"},
// RFC1918
{"RFC1918 10.0.0.1", "10.0.0.1"},
{"RFC1918 10.255.255.255", "10.255.255.255"},
{"RFC1918 172.16.0.1", "172.16.0.1"},
{"RFC1918 172.31.255.255", "172.31.255.255"},
{"RFC1918 192.168.0.1", "192.168.0.1"},
{"RFC1918 192.168.255.255", "192.168.255.255"},
// Link-local (APIPA / AWS metadata)
{"link-local 169.254.0.1", "169.254.0.1"},
{"link-local 169.254.169.254", "169.254.169.254"},
// Shared address space (carrier-grade NAT)
{"CGN 100.64.0.1", "100.64.0.1"},
{"CGN 100.127.255.255", "100.127.255.255"},
// Multicast
{"multicast 224.0.0.1", "224.0.0.1"},
{"multicast 239.255.255.255", "239.255.255.255"},
// Reserved
{"reserved 240.0.0.1", "240.0.0.1"},
{"broadcast 255.255.255.255", "255.255.255.255"},
// IPv6 loopback
{"IPv6 loopback ::1", "::1"},
// IPv6 unspecified
{"IPv6 unspecified ::", "::"},
// IPv6 link-local
{"IPv6 link-local fe80::1", "fe80::1"},
{"IPv6 link-local fe80::dead:beef", "fe80::dead:beef"},
// IPv6 ULA
{"IPv6 ULA fc00::1", "fc00::1"},
{"IPv6 ULA fd00::1", "fd00::1"},
// IPv6 multicast
{"IPv6 multicast ff02::1", "ff02::1"},
} }
for _, tc := range cases {
for _, tc := range blocked {
t.Run(tc.name, func(t *testing.T) {
ip := net.ParseIP(tc.ip) ip := net.ParseIP(tc.ip)
if ip == nil { if ip == nil {
t.Fatalf("failed to parse IP %q", tc.ip) t.Fatalf("failed to parse IP %q", tc.ip)
} }
if !IsBlockedIP(ip) { got := IsBlockedIP(ip)
t.Errorf("IsBlockedIP(%q) = false, want true", tc.ip) want := netutil.IsBlockedIP(ip)
if got != want {
t.Errorf("gitea.IsBlockedIP(%q) = %v, netutil.IsBlockedIP = %v: forwarding mismatch", tc.ip, got, want)
} }
}) if got != tc.blocked {
} t.Errorf("gitea.IsBlockedIP(%q) = %v, want %v", tc.ip, got, tc.blocked)
allowed := []struct {
name string
ip string
}{
{"public 8.8.8.8", "8.8.8.8"},
{"public 1.1.1.1", "1.1.1.1"},
{"public 198.51.100.1", "198.51.100.1"}, // RFC5737 TEST-NET-2 — a documentation-only range;
// not assigned to any real host, but intentionally left unblocked here because
// it has no special routing treatment (unlike RFC1918/loopback/link-local) and
// blocking it would require tracking every RFC5737 range without meaningful
// security benefit (no server should ever listen on a TEST-NET address).
{"public 151.101.1.1", "151.101.1.1"}, // Fastly
{"public IPv6 2001:4860:4860::8888", "2001:4860:4860::8888"}, // Google DNS
{"public IPv6 2606:4700:4700::1111", "2606:4700:4700::1111"}, // Cloudflare DNS
}
for _, tc := range allowed {
t.Run(tc.name, func(t *testing.T) {
ip := net.ParseIP(tc.ip)
if ip == nil {
t.Fatalf("failed to parse IP %q", tc.ip)
}
if IsBlockedIP(ip) {
t.Errorf("IsBlockedIP(%q) = true, want false", tc.ip)
}
})
}
}
func TestIsBlockedIPv6MappedIPv4(t *testing.T) {
// ::ffff:192.168.1.1 is an IPv6-mapped IPv4 address — should be blocked as RFC1918.
// Construct it manually as a 16-byte IP.
mapped := net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 192, 168, 1, 1}
if !IsBlockedIP(mapped) {
t.Errorf("IsBlockedIP(::ffff:192.168.1.1) = false, want true (IPv6-mapped IPv4 must be normalized)")
}
// ::ffff:8.8.8.8 — IPv6-mapped public IP — should be allowed.
mappedPublic := net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 8, 8, 8, 8}
if IsBlockedIP(mappedPublic) {
t.Errorf("IsBlockedIP(::ffff:8.8.8.8) = true, want false")
}
}
func TestIsBlockedIPEdgeCases(t *testing.T) {
// The boundary between RFC1918 and public ranges.
// 172.15.255.255 is NOT private (just below 172.16.0.0/12).
notPrivate := net.ParseIP("172.15.255.255")
if IsBlockedIP(notPrivate) {
t.Errorf("IsBlockedIP(172.15.255.255) = true, want false (outside 172.16.0.0/12)")
}
// 172.32.0.0 is NOT private (just above 172.31.255.255).
notPrivate2 := net.ParseIP("172.32.0.0")
if IsBlockedIP(notPrivate2) {
t.Errorf("IsBlockedIP(172.32.0.0) = true, want false (outside 172.16.0.0/12)")
}
// CGN: 100.63.255.255 is NOT in 100.64.0.0/10.
notCGN := net.ParseIP("100.63.255.255")
if IsBlockedIP(notCGN) {
t.Errorf("IsBlockedIP(100.63.255.255) = true, want false (outside 100.64.0.0/10)")
}
// CGN: 100.128.0.0 is NOT in 100.64.0.0/10.
notCGN2 := net.ParseIP("100.128.0.0")
if IsBlockedIP(notCGN2) {
t.Errorf("IsBlockedIP(100.128.0.0) = true, want false (outside 100.64.0.0/10)")
}
}
// TestBlockedCIDRsValid verifies that all entries in blockedCIDRStrings parse
// successfully. This catches programming errors in the CIDR list without
// requiring a startup panic. The init() function records parse failures in
// blockedCIDRParseErrors rather than panicking; this test makes those failures
// visible as test failures during CI.
func TestBlockedCIDRsValid(t *testing.T) {
if len(blockedCIDRParseErrors) > 0 {
for _, msg := range blockedCIDRParseErrors {
t.Errorf("CIDR parse error: %s", msg)
} }
} }
} }
+97
View File
@@ -0,0 +1,97 @@
// Package netutil provides shared network utilities for review-bot.
// ipcheck.go implements IP-level SSRF protection by checking resolved addresses
// against known blocked CIDR ranges (RFC1918, loopback, link-local, etc.).
package netutil
import (
"fmt"
"net"
)
// blockedCIDRStrings is the canonical list of CIDR strings that should never
// be contacted by review-bot. See IsBlockedIP for the full list of covered
// address families.
//
// These are hard-coded literals: any parse failure is a programming error.
// Validity is verified by TestBlockedCIDRsValid in ipcheck_test.go.
var blockedCIDRStrings = []string{
// IPv4 loopback
"127.0.0.0/8",
// IPv4 unspecified / "this network"
"0.0.0.0/8",
// RFC1918 private ranges
"10.0.0.0/8",
"172.16.0.0/12",
"192.168.0.0/16",
// IPv4 link-local (APIPA, also used by AWS instance metadata 169.254.169.254)
"169.254.0.0/16",
// IPv4 shared address space (RFC6598, carrier-grade NAT)
"100.64.0.0/10",
// IPv4 multicast
"224.0.0.0/4",
// IPv4 reserved / broadcast
"240.0.0.0/4",
// IPv6 loopback
"::1/128",
// IPv6 unspecified
"::/128",
// IPv6 link-local
"fe80::/10",
// IPv6 unique local (ULA) — RFC4193
"fc00::/7",
// IPv6 multicast
"ff00::/8",
}
// blockedCIDRs is the parsed form of blockedCIDRStrings.
// Any entry that fails to parse is recorded in blockedCIDRParseErrors instead
// of panicking; tests verify this slice is always empty via TestBlockedCIDRsValid.
var (
blockedCIDRs []*net.IPNet
blockedCIDRParseErrors []string
)
func init() {
blockedCIDRs = make([]*net.IPNet, 0, len(blockedCIDRStrings))
for _, r := range blockedCIDRStrings {
_, cidr, err := net.ParseCIDR(r)
if err != nil {
// Record the error rather than panicking; TestBlockedCIDRsValid
// will catch this during tests, and the CI build will fail.
blockedCIDRParseErrors = append(blockedCIDRParseErrors,
fmt.Sprintf("ipcheck: invalid built-in CIDR %q: %v", r, err))
continue
}
blockedCIDRs = append(blockedCIDRs, cidr)
}
}
// BlockedCIDRParseErrors returns any errors encountered parsing the built-in
// CIDR list. In correct code this will always be empty; tests assert it is.
func BlockedCIDRParseErrors() []string {
return blockedCIDRParseErrors
}
// IsBlockedIP reports whether ip is in a blocked address range.
// It is exported for use by the gitea package's safe dialer, the validate-url
// subcommand, and tests outside this package.
//
// IPv6-mapped IPv4 addresses (e.g. ::ffff:192.168.1.1) are normalized to their
// IPv4 form before checking so that IPv4 CIDRs catch them.
//
// Based on:
// - RFC1918 private ranges
// - RFC5735 / RFC4193 special-use IPv4/IPv6 ranges
// - RFC4291 IPv6 link-local / loopback
func IsBlockedIP(ip net.IP) bool {
// Normalize IPv6-mapped IPv4 addresses (::ffff:x.x.x.x) to plain IPv4.
if v4 := ip.To4(); v4 != nil {
ip = v4
}
for _, cidr := range blockedCIDRs {
if cidr.Contains(ip) {
return true
}
}
return false
}
+142
View File
@@ -0,0 +1,142 @@
package netutil
import (
"net"
"testing"
)
func TestIsBlockedIP(t *testing.T) {
blocked := []struct {
name string
ip string
}{
// IPv4 loopback
{"loopback 127.0.0.1", "127.0.0.1"},
{"loopback 127.0.0.2", "127.0.0.2"},
{"loopback 127.255.255.255", "127.255.255.255"},
// IPv4 unspecified
{"unspecified 0.0.0.0", "0.0.0.0"},
{"unspecified 0.1.2.3", "0.1.2.3"},
// RFC1918
{"RFC1918 10.0.0.1", "10.0.0.1"},
{"RFC1918 10.255.255.255", "10.255.255.255"},
{"RFC1918 172.16.0.1", "172.16.0.1"},
{"RFC1918 172.31.255.255", "172.31.255.255"},
{"RFC1918 192.168.0.1", "192.168.0.1"},
{"RFC1918 192.168.255.255", "192.168.255.255"},
// Link-local (APIPA / AWS metadata)
{"link-local 169.254.0.1", "169.254.0.1"},
{"link-local 169.254.169.254", "169.254.169.254"},
// Shared address space (carrier-grade NAT)
{"CGN 100.64.0.1", "100.64.0.1"},
{"CGN 100.127.255.255", "100.127.255.255"},
// Multicast
{"multicast 224.0.0.1", "224.0.0.1"},
{"multicast 239.255.255.255", "239.255.255.255"},
// Reserved
{"reserved 240.0.0.1", "240.0.0.1"},
{"broadcast 255.255.255.255", "255.255.255.255"},
// IPv6 loopback
{"IPv6 loopback ::1", "::1"},
// IPv6 unspecified
{"IPv6 unspecified ::", "::"},
// IPv6 link-local
{"IPv6 link-local fe80::1", "fe80::1"},
{"IPv6 link-local fe80::dead:beef", "fe80::dead:beef"},
// IPv6 ULA
{"IPv6 ULA fc00::1", "fc00::1"},
{"IPv6 ULA fd00::1", "fd00::1"},
// IPv6 multicast
{"IPv6 multicast ff02::1", "ff02::1"},
}
for _, tc := range blocked {
t.Run(tc.name, func(t *testing.T) {
ip := net.ParseIP(tc.ip)
if ip == nil {
t.Fatalf("failed to parse IP %q", tc.ip)
}
if !IsBlockedIP(ip) {
t.Errorf("IsBlockedIP(%q) = false, want true", tc.ip)
}
})
}
allowed := []struct {
name string
ip string
}{
{"public 8.8.8.8", "8.8.8.8"},
{"public 1.1.1.1", "1.1.1.1"},
{"public 198.51.100.1", "198.51.100.1"}, // RFC5737 TEST-NET-2 — a documentation-only range;
// not assigned to any real host, but intentionally left unblocked here because
// it has no special routing treatment (unlike RFC1918/loopback/link-local) and
// blocking it would require tracking every RFC5737 range without meaningful
// security benefit (no server should ever listen on a TEST-NET address).
{"public 151.101.1.1", "151.101.1.1"}, // Fastly
{"public IPv6 2001:4860:4860::8888", "2001:4860:4860::8888"}, // Google DNS
{"public IPv6 2606:4700:4700::1111", "2606:4700:4700::1111"}, // Cloudflare DNS
}
for _, tc := range allowed {
t.Run(tc.name, func(t *testing.T) {
ip := net.ParseIP(tc.ip)
if ip == nil {
t.Fatalf("failed to parse IP %q", tc.ip)
}
if IsBlockedIP(ip) {
t.Errorf("IsBlockedIP(%q) = true, want false", tc.ip)
}
})
}
}
func TestIsBlockedIPv6MappedIPv4(t *testing.T) {
// ::ffff:192.168.1.1 is an IPv6-mapped IPv4 address — should be blocked as RFC1918.
// Construct it manually as a 16-byte IP.
mapped := net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 192, 168, 1, 1}
if !IsBlockedIP(mapped) {
t.Errorf("IsBlockedIP(::ffff:192.168.1.1) = false, want true (IPv6-mapped IPv4 must be normalized)")
}
// ::ffff:8.8.8.8 — IPv6-mapped public IP — should be allowed.
mappedPublic := net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 8, 8, 8, 8}
if IsBlockedIP(mappedPublic) {
t.Errorf("IsBlockedIP(::ffff:8.8.8.8) = true, want false")
}
}
func TestIsBlockedIPEdgeCases(t *testing.T) {
// The boundary between RFC1918 and public ranges.
// 172.15.255.255 is NOT private (just below 172.16.0.0/12).
notPrivate := net.ParseIP("172.15.255.255")
if IsBlockedIP(notPrivate) {
t.Errorf("IsBlockedIP(172.15.255.255) = true, want false (outside 172.16.0.0/12)")
}
// 172.32.0.0 is NOT private (just above 172.31.255.255).
notPrivate2 := net.ParseIP("172.32.0.0")
if IsBlockedIP(notPrivate2) {
t.Errorf("IsBlockedIP(172.32.0.0) = true, want false (outside 172.16.0.0/12)")
}
// CGN: 100.63.255.255 is NOT in 100.64.0.0/10.
notCGN := net.ParseIP("100.63.255.255")
if IsBlockedIP(notCGN) {
t.Errorf("IsBlockedIP(100.63.255.255) = true, want false (outside 100.64.0.0/10)")
}
// CGN: 100.128.0.0 is NOT in 100.64.0.0/10.
notCGN2 := net.ParseIP("100.128.0.0")
if IsBlockedIP(notCGN2) {
t.Errorf("IsBlockedIP(100.128.0.0) = true, want false (outside 100.64.0.0/10)")
}
}
// TestBlockedCIDRsValid verifies that all entries in blockedCIDRStrings parse
// successfully. This catches programming errors in the CIDR list without
// requiring a startup panic. The init() function records parse failures in
// blockedCIDRParseErrors rather than panicking; this test makes those failures
// visible as test failures during CI.
func TestBlockedCIDRsValid(t *testing.T) {
for _, msg := range BlockedCIDRParseErrors() {
t.Errorf("CIDR parse error: %s", msg)
}
}