fix(#130 ): cleanup and test improvements

- Fix extra blank lines between GetPullRequest and GetPullRequestFiles - Add TestPostReview_CommitIDFromRequest to verify CommitID passes through
feat(#130 ): add VCS routing in cmd/review-bot via --vcs-type flag
2026-05-14 13:57:58 -07:00 · 2026-05-14 13:53:56 -07:00 · 2026-05-14 13:45:36 -07:00 · 2026-05-14 13:41:14 -07:00 · 2026-05-14 20:10:56 +00:00 · 2026-05-14 19:26:04 +00:00
24 changed files with 3101 additions and 186 deletions
@@ -1,17 +1,43 @@
-# This composite action is designed for Gitea Actions runners.
-# Gitea Actions supports GitHub Actions syntax including $GITHUB_OUTPUT,
-# actions/cache, and actions/checkout.
+# This composite action supports both Gitea Actions and GitHub Actions runners.
+# It detects the VCS host type by checking whether github.api_url is set
+# (present on GitHub.com and GHES runners, absent on Gitea runners) and uses
+# the appropriate releases API for version resolution and binary download
+# (REST API on GitHub, direct URLs on Gitea).
+#
+# Security notes:
+# - On GitHub/GHES (VCS_TYPE=github), inputs.vcs-url is IGNORED to prevent
+#   token exfiltration. API calls use github.api_url; downloads use
+#   github.server_url. Tokens are never sent to user-supplied URLs.
+# - On Gitea (VCS_TYPE=gitea), inputs.vcs-url is validated (https scheme,
+#   no whitespace/newlines, and DNS resolution to a public IP) before use.
+#   Python3 resolves the hostname and rejects RFC1918, RFC6598 (carrier-grade
+#   NAT), loopback, link-local, and other reserved addresses to prevent SSRF attacks.
+#   The installed review-bot binary additionally uses a safe HTTP transport
+#   (DialContext-level IP check) for all Gitea API calls at runtime.
+#   The binary also exposes a `validate-url` subcommand for use in any future
+#   shell steps that need to validate a URL before passing it to curl.
+# - action-repo is validated against owner/repo pattern.
+# - Tokens are passed via masked environment variables, not step outputs.
+#
 # Requirements: python3, sha256sum, curl (all present on ubuntu-* runners).
 name: 'AI Code Review'
 description: 'Run AI-powered code review on a pull request using review-bot'

 inputs:
-  gitea-url:
-    description: 'Gitea instance URL (defaults to server_url)'
+  vcs-url:
+    description: 'VCS server URL (only used on Gitea runners; ignored on GitHub/GHES). Defaults to server_url.'
    required: false
    default: ''
  repo:
-    description: 'Repository (owner/name, defaults to current)'
+    description: 'Repository to review (owner/name, defaults to current)'
+    required: false
+    default: ''
+  action-repo:
+    description: 'Repository hosting review-bot releases (owner/name). Defaults to github.action_repository or rodin/review-bot.'
+    required: false
+    default: ''
+  action-repo-token:
+    description: 'Token for downloading release assets from action-repo (defaults to github.token on GitHub, reviewer-token on Gitea). Required for private repos.'
    required: false
    default: ''
  pr-number:
@@ -19,7 +45,7 @@ inputs:
    required: false
    default: ''
  reviewer-token:
-    description: 'Gitea token for posting the review'
+    description: 'Token for posting the review'
    required: true
  reviewer-name:
    description: 'Display name for the reviewer'
@@ -112,19 +138,150 @@ runs:
      id: version
      shell: bash
      run: |
-        GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
-        REPO="${{ inputs.repo || 'rodin/review-bot' }}"
+        set -euo pipefail
+
+        # --- Input Validation ---
+
+        # Determine the repo hosting review-bot releases (not the repo being reviewed)
+        ACTION_REPO="${{ inputs.action-repo }}"
+        if [ -z "$ACTION_REPO" ]; then
+          # github.action_repository is the repo containing the running action
+          ACTION_REPO="${{ github.action_repository }}"
+        fi
+        if [ -z "$ACTION_REPO" ]; then
+          # Final fallback for Gitea (which may not set action_repository)
+          ACTION_REPO="rodin/review-bot"
+          echo "::notice::action-repo not specified and github.action_repository is empty; falling back to rodin/review-bot"
+        fi
+
+        # Validate ACTION_REPO matches owner/repo pattern (prevent path traversal)
+        if ! printf '%s' "$ACTION_REPO" | grep -qE '^[a-zA-Z0-9._-]+/[a-zA-Z0-9._-]+$'; then
+          echo "Error: action-repo '${ACTION_REPO}' does not match expected owner/repo format" >&2
+          exit 1
+        fi
+
+        # Detect VCS host type using github.api_url context.
+        # github.api_url is set on GitHub.com (https://api.github.com) and GHES
+        # (https://<host>/api/v3). It is empty/unset on Gitea Actions runners.
+        GITHUB_API_URL="${{ github.api_url }}"
+        if [ -n "$GITHUB_API_URL" ]; then
+          VCS_TYPE="github"
+        else
+          VCS_TYPE="gitea"
+        fi
+
+        # Determine SERVER_URL based on VCS type.
+        # SECURITY: On GitHub/GHES, ALWAYS use github.server_url — never trust
+        # inputs.vcs-url to prevent token exfiltration to attacker-controlled hosts.
+        if [ "$VCS_TYPE" = "github" ]; then
+          SERVER_URL="${{ github.server_url }}"
+          if [ -n "${{ inputs.vcs-url }}" ]; then
+            echo "::warning::inputs.vcs-url is ignored on GitHub/GHES runners (VCS_TYPE=github). Using github.server_url instead."
+          fi
+        else
+          SERVER_URL="${{ inputs.vcs-url || github.server_url }}"
+        fi
+        # Strip trailing slash if present
+        SERVER_URL="${SERVER_URL%/}"
+
+        # Validate SERVER_URL for Gitea path: must be https, no whitespace/newlines.
+        # The [^[:space:]] class already rejects newlines, so no separate newline check needed.
+        if [ "$VCS_TYPE" = "gitea" ]; then
+          if ! printf '%s' "$SERVER_URL" | grep -qE '^https://[^[:space:]]+$'; then
+            echo "Error: SERVER_URL '${SERVER_URL}' must be an https:// URL with no whitespace" >&2
+            exit 1
+          fi
+
+          # Additional IP-level SSRF defense: resolve the hostname and reject
+          # requests to RFC1918, RFC6598 (carrier-grade NAT), loopback, link-local,
+          # and other reserved addresses.
+          # 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
+          # YAML (each indented line becomes a printf argument — no unindented code).
+          # SERVER_URL is passed via CHECK_URL env var, never interpolated into python code.
+          printf '%s\n' \
+            'import socket,ipaddress,sys,os' \
+            'from urllib.parse import urlparse' \
+            'u=os.environ["CHECK_URL"]; parsed=urlparse(u)' \
+            'if parsed.username or parsed.password:' \
+            '  print("Error: URL contains user-info — not allowed",file=sys.stderr); sys.exit(2)' \
+            'h=parsed.hostname' \
+            '(print("Error: no hostname",file=sys.stderr) or sys.exit(2)) if not h else None' \
+            'try: rs=socket.getaddrinfo(h,None)' \
+            'except socket.gaierror as e: print(f"DNS error: {e}",file=sys.stderr); sys.exit(1)' \
+            'if not rs: print("Error: no addresses",file=sys.stderr); sys.exit(1)' \
+            'for _,_,_,_,(a,*_) in rs:' \
+            '  ip=ipaddress.ip_address(a)' \
+            '  if isinstance(ip,ipaddress.IPv6Address) and ip.ipv4_mapped: ip=ip.ipv4_mapped' \
+            '  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)' \
+            > /tmp/_ssrf_check.py
+          CHECK_URL="${SERVER_URL}" python3 /tmp/_ssrf_check.py || {
+            echo "Error: SERVER_URL '${SERVER_URL}' resolves to a private/reserved IP address" >&2
+            exit 1
+          }
+        fi
+
+        # Determine auth token for release API requests
+        ACTION_TOKEN="${{ inputs.action-repo-token }}"
+        if [ -z "$ACTION_TOKEN" ]; then
+          if [ "$VCS_TYPE" = "github" ]; then
+            ACTION_TOKEN="${{ github.token }}"
+          else
+            ACTION_TOKEN="${{ inputs.reviewer-token }}"
+          fi
+        fi
+
+        # Validate token contains no control characters (defense-in-depth against header injection)
+        if [ -n "$ACTION_TOKEN" ]; then
+          if printf '%s' "$ACTION_TOKEN" | LC_ALL=C grep -q '[^[:print:]]'; then
+            echo "Error: ACTION_TOKEN contains control characters" >&2
+            exit 1
+          fi
+        fi
+
        if [ "${{ inputs.version }}" = "latest" ]; then
-          VERSION=$(curl -sSf "${GITEA_URL}/api/v1/repos/${REPO}/releases?limit=1" \
-            | python3 -c "import sys, json; releases = json.load(sys.stdin); print(releases[0]['tag_name'] if releases else '')")
+          if [ "$VCS_TYPE" = "github" ]; then
+            # SECURITY: Use github.api_url which is a trusted platform-provided value.
+            # Never construct API URLs from user-supplied inputs on GitHub.
+            API_URL="${GITHUB_API_URL}/repos/${ACTION_REPO}/releases?per_page=1"
+          else
+            # Gitea API — SERVER_URL was validated above
+            API_URL="${SERVER_URL}/api/v1/repos/${ACTION_REPO}/releases?limit=1"
+          fi
+
+          # Fetch latest version with inline auth header (no intermediate variable)
+          if [ -n "$ACTION_TOKEN" ]; then
+            if [ "$VCS_TYPE" = "github" ]; then
+              VERSION=$(curl -sSf --connect-timeout 10 --max-time 30 \
+                -H "Authorization: Bearer ${ACTION_TOKEN}" "$API_URL" \
+                | python3 -c "import sys, json; releases = json.load(sys.stdin); print(releases[0]['tag_name'] if releases else '')")
+            else
+              VERSION=$(curl -sSf --connect-timeout 10 --max-time 30 \
+                -H "Authorization: token ${ACTION_TOKEN}" "$API_URL" \
+                | python3 -c "import sys, json; releases = json.load(sys.stdin); print(releases[0]['tag_name'] if releases else '')")
+            fi
+          else
+            VERSION=$(curl -sSf --connect-timeout 10 --max-time 30 "$API_URL" \
+              | python3 -c "import sys, json; releases = json.load(sys.stdin); print(releases[0]['tag_name'] if releases else '')")
+          fi
+
          if [ -z "$VERSION" ]; then
-            echo "Failed to determine latest version" >&2
+            echo "Failed to determine latest version from ${API_URL}" >&2
            exit 1
          fi
        else
          VERSION="${{ inputs.version }}"
        fi

+        # Validate VERSION: no slashes or whitespace (prevent path traversal).
+        # [:space:] includes newlines and carriage returns in POSIX.
+        if printf '%s' "$VERSION" | grep -qE '[/[:space:]]'; then
+          echo "Error: VERSION '${VERSION}' contains invalid characters (newline, slash, or whitespace)" >&2
+          exit 1
+        fi
+
        # Detect OS and architecture for platform-specific binary download
        OS_RAW=$(uname -s | tr '[:upper:]' '[:lower:]')
        case "$OS_RAW" in
@@ -149,6 +306,16 @@ runs:
        echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
        echo "os=${OS}" >> "$GITHUB_OUTPUT"
        echo "arch=${ARCH}" >> "$GITHUB_OUTPUT"
+        echo "action_repo=${ACTION_REPO}" >> "$GITHUB_OUTPUT"
+        echo "server_url=${SERVER_URL}" >> "$GITHUB_OUTPUT"
+        echo "vcs_type=${VCS_TYPE}" >> "$GITHUB_OUTPUT"
+
+        # SECURITY: Pass token via masked environment variable instead of step output.
+        # Step outputs can leak in debug logs; GITHUB_ENV with masking is safer.
+        if [ -n "$ACTION_TOKEN" ]; then
+          echo "::add-mask::${ACTION_TOKEN}"
+          echo "ACTION_TOKEN=${ACTION_TOKEN}" >> "$GITHUB_ENV"
+        fi

    - name: Cache review-bot binary
      id: cache
@@ -161,21 +328,131 @@ runs:
      if: steps.cache.outputs.cache-hit != 'true'
      shell: bash
      run: |
-        GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
-        REPO="${{ inputs.repo || 'rodin/review-bot' }}"
-        VERSION="${{ steps.version.outputs.version }}"
-        BINARY="review-bot-${{ steps.version.outputs.os }}-${{ steps.version.outputs.arch }}"
+        set -euo pipefail

-        curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/${BINARY}" \
-          -o "${{ runner.temp }}/review-bot"
-        curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/checksums.txt" \
-          -o "${{ runner.temp }}/checksums.txt"
+        SERVER_URL="${{ steps.version.outputs.server_url }}"
+        ACTION_REPO="${{ steps.version.outputs.action_repo }}"
+        VERSION="${{ steps.version.outputs.version }}"
+        VCS_TYPE="${{ steps.version.outputs.vcs_type }}"
+        OS="${{ steps.version.outputs.os }}"
+        ARCH="${{ steps.version.outputs.arch }}"
+        # Read token from masked environment variable (set in Determine version step)
+        # Falls back to empty if not set (public repos don't need auth)
+        ACTION_TOKEN="${ACTION_TOKEN:-}"
+        BINARY="review-bot-${OS}-${ARCH}"
+
+        # SECURITY: Re-validate SERVER_URL at the start of this step to mitigate DNS
+        # rebinding attacks. A DNS TTL expiry between "Determine version" and here
+        # could allow an attacker to change the resolved IP to a private/reserved
+        # address, causing curl to send ACTION_TOKEN to an internal host.
+        # Only needed on Gitea path (VCS_TYPE=gitea); GitHub/GHES uses platform-controlled URLs.
+        if [ "$VCS_TYPE" = "gitea" ]; then
+          printf '%s\n' \
+            'import socket,ipaddress,sys,os' \
+            'from urllib.parse import urlparse' \
+            'u=os.environ["CHECK_URL"]; parsed=urlparse(u)' \
+            'if parsed.username or parsed.password:' \
+            '  print("Error: URL contains user-info — not allowed",file=sys.stderr); sys.exit(2)' \
+            'h=parsed.hostname' \
+            '(print("Error: no hostname",file=sys.stderr) or sys.exit(2)) if not h else None' \
+            'try: rs=socket.getaddrinfo(h,None)' \
+            'except socket.gaierror as e: print(f"DNS error: {e}",file=sys.stderr); sys.exit(1)' \
+            'if not rs: print("Error: no addresses",file=sys.stderr); sys.exit(1)' \
+            'for _,_,_,_,(a,*_) in rs:' \
+            '  ip=ipaddress.ip_address(a)' \
+            '  if isinstance(ip,ipaddress.IPv6Address) and ip.ipv4_mapped: ip=ip.ipv4_mapped' \
+            '  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)' \
+              > /tmp/_ssrf_check_install.py
+          CHECK_URL="${SERVER_URL}" python3 /tmp/_ssrf_check_install.py || {
+            echo "Error: SERVER_URL '${SERVER_URL}' resolves to a private/reserved IP address" >&2
+            exit 1
+          }
+        fi
+
+        if [ "$VCS_TYPE" = "github" ]; then
+          # GitHub/GHES: Use REST API for release asset downloads.
+          # Web release URLs ({server}/.../releases/download/{tag}/{asset}) redirect
+          # to S3 and don't reliably support Authorization headers for private repos.
+          # The REST API endpoint with Accept: application/octet-stream is required.
+          # GITHUB_API_URL: trusted platform value, same as detected in "Determine version" step.
+          GITHUB_API_URL="${{ github.api_url }}"
+
+          if [ -n "$ACTION_TOKEN" ]; then
+            RELEASE_JSON=$(curl -sSf --connect-timeout 10 --max-time 30 \
+              -H "Authorization: Bearer ${ACTION_TOKEN}" \
+              "${GITHUB_API_URL}/repos/${ACTION_REPO}/releases/tags/${VERSION}")
+          else
+            RELEASE_JSON=$(curl -sSf --connect-timeout 10 --max-time 30 \
+              "${GITHUB_API_URL}/repos/${ACTION_REPO}/releases/tags/${VERSION}")
+          fi
+
+          # Extract asset IDs for binary and checksums
+          BINARY_ASSET_ID=$(printf '%s' "$RELEASE_JSON" | python3 -c "import sys, json; assets = json.load(sys.stdin).get('assets', []); matches = [a['id'] for a in assets if a['name'] == '${BINARY}']; print(matches[0] if matches else '')")
+          if [ -z "$BINARY_ASSET_ID" ]; then
+            echo "Error: could not find asset '${BINARY}' in release ${VERSION}" >&2
+            exit 1
+          fi
+
+          CHECKSUMS_ASSET_ID=$(printf '%s' "$RELEASE_JSON" | python3 -c "import sys, json; assets = json.load(sys.stdin).get('assets', []); matches = [a['id'] for a in assets if a['name'] == 'checksums.txt']; print(matches[0] if matches else '')")
+          if [ -z "$CHECKSUMS_ASSET_ID" ]; then
+            echo "Error: could not find asset 'checksums.txt' in release ${VERSION}" >&2
+            exit 1
+          fi
+
+          # Download assets via REST API with Accept: application/octet-stream
+          if [ -n "$ACTION_TOKEN" ]; then
+            curl -sSfL --connect-timeout 10 --max-time 120 \
+              -H "Authorization: Bearer ${ACTION_TOKEN}" \
+              -H "Accept: application/octet-stream" \
+              "${GITHUB_API_URL}/repos/${ACTION_REPO}/releases/assets/${BINARY_ASSET_ID}" \
+              -o "${{ runner.temp }}/review-bot"
+            curl -sSfL --connect-timeout 10 --max-time 30 \
+              -H "Authorization: Bearer ${ACTION_TOKEN}" \
+              -H "Accept: application/octet-stream" \
+              "${GITHUB_API_URL}/repos/${ACTION_REPO}/releases/assets/${CHECKSUMS_ASSET_ID}" \
+              -o "${{ runner.temp }}/checksums.txt"
+          else
+            curl -sSfL --connect-timeout 10 --max-time 120 \
+              -H "Accept: application/octet-stream" \
+              "${GITHUB_API_URL}/repos/${ACTION_REPO}/releases/assets/${BINARY_ASSET_ID}" \
+              -o "${{ runner.temp }}/review-bot"
+            curl -sSfL --connect-timeout 10 --max-time 30 \
+              -H "Accept: application/octet-stream" \
+              "${GITHUB_API_URL}/repos/${ACTION_REPO}/releases/assets/${CHECKSUMS_ASSET_ID}" \
+              -o "${{ runner.temp }}/checksums.txt"
+          fi
+        else
+          # Gitea: Direct download via web release URLs (Gitea serves assets
+          # directly without redirects — no -L needed).
+          # SECURITY: Omitting -L prevents forwarding Authorization header to
+          # unexpected hosts if Gitea ever introduces CDN redirects.
+          DOWNLOAD_URL="${SERVER_URL}/${ACTION_REPO}/releases/download/${VERSION}"
+
+          if [ -n "$ACTION_TOKEN" ]; then
+            curl -sSf --connect-timeout 10 --max-time 120 \
+              -H "Authorization: token ${ACTION_TOKEN}" \
+              "${DOWNLOAD_URL}/${BINARY}" -o "${{ runner.temp }}/review-bot"
+            curl -sSf --connect-timeout 10 --max-time 30 \
+              -H "Authorization: token ${ACTION_TOKEN}" \
+              "${DOWNLOAD_URL}/checksums.txt" -o "${{ runner.temp }}/checksums.txt"
+          else
+            curl -sSf --connect-timeout 10 --max-time 120 \
+              "${DOWNLOAD_URL}/${BINARY}" -o "${{ runner.temp }}/review-bot"
+            curl -sSf --connect-timeout 10 --max-time 30 \
+              "${DOWNLOAD_URL}/checksums.txt" -o "${{ runner.temp }}/checksums.txt"
+          fi
+        fi

        # Verify SHA-256 checksum
+        # NOTE: This verifies integrity (download wasn't corrupted) but not
+        # authenticity — both binary and checksums come from the same server.
+        # For stronger guarantees, consider GPG signature verification.
        cd "${{ runner.temp }}"
-        EXPECTED=$(grep -E "^[[:xdigit:]]+[[:space:]]+\*?${BINARY}$" checksums.txt | awk '{print $1}')
+        EXPECTED=$(grep -E "^[0-9a-f]+[[:space:]]+\*?${BINARY}$" checksums.txt | awk '{print $1}')
        # sha256sum (GNU) is not available on macOS; use shasum -a 256 on darwin.
-        if [ "${{ steps.version.outputs.os }}" = "darwin" ]; then
+        if [ "${OS}" = "darwin" ]; then
          ACTUAL=$(shasum -a 256 review-bot | awk '{print $1}')
        else
          ACTUAL=$(sha256sum review-bot | awk '{print $1}')
@@ -193,12 +470,12 @@ runs:
        fi

        chmod +x "${{ runner.temp }}/review-bot"
-        echo "Installed review-bot-${{ steps.version.outputs.os }}-${{ steps.version.outputs.arch }} ${VERSION} (checksum verified)"
+        echo "Installed review-bot-${OS}-${ARCH} ${VERSION} (checksum verified)"

    - name: Run review
      shell: bash
      env:
-        GITEA_URL: ${{ inputs.gitea-url || github.server_url }}
+        VCS_URL: ${{ steps.version.outputs.server_url }}
        GITEA_REPO: ${{ inputs.repo || github.repository }}
        PR_NUMBER: ${{ inputs.pr-number || github.event.pull_request.number }}
        REVIEWER_TOKEN: ${{ inputs.reviewer-token }}
@@ -49,7 +49,7 @@ jobs:
      - run: go build -o review-bot ./cmd/review-bot
      - name: Run ${{ matrix.name }} review
        env:
-          GITEA_URL: ${{ github.server_url }}
+          VCS_URL: ${{ github.server_url }}
          GITEA_REPO: ${{ github.repository }}
          PR_NUMBER: ${{ github.event.pull_request.number }}
          REVIEWER_TOKEN: ${{ secrets[matrix.token_secret] }}
@@ -0,0 +1,175 @@
+# Plan: Issue #125 — Rename GITEA_URL → VCS_URL
+
+## Problem
+
+The `GITEA_URL` environment variable (and `--gitea-url` flag) implies the binary only works with Gitea.
+Now that review-bot supports both Gitea and GitHub/GHES, this name is misleading.
+Renaming to `VCS_URL` makes the binary platform-agnostic in its interface.
+
+## Constraints
+
+- Must not break existing users who already use `GITEA_URL` — need a fallback
+- The CLI flag `--gitea-url` should also be updated to `--vcs-url` for consistency
+- `INTEGRATION_GITEA_URL` in integration tests is a test-only env var, not the binary's interface; but should be updated for clarity
+- The action YAML uses `GITEA_URL` as an internal shell variable in bash scripts — distinct from the env var passed to the binary
+- All changes must compile and pass existing tests
+
+## Files Affected
+
+### Binary / Go source
+| File | Change |
+|------|--------|
+| `cmd/review-bot/main.go` | Rename `--gitea-url` → `--vcs-url`, add `VCS_URL` as primary, keep `GITEA_URL` fallback |
+| `cmd/review-bot/integration_test.go` | Rename `INTEGRATION_GITEA_URL` → `INTEGRATION_VCS_URL` (test-only, no external compat concern) |
+| `integration_test.go` | Same — rename `INTEGRATION_GITEA_URL` → `INTEGRATION_VCS_URL` |
+
+### Action YAML
+| File | Change |
+|------|--------|
+| `.gitea/actions/review/action.yml` | Rename input `gitea-url` → `vcs-url`; update env var passed to binary: `VCS_URL` instead of `GITEA_URL`; keep internal bash var as `GITEA_URL` (only used for release download, not passed to binary) |
+| `.gitea/workflows/ci.yml` | Rename `GITEA_URL` env var to `VCS_URL` in Run review step |
+
+### Documentation
+| File | Change |
+|------|--------|
+| `README.md` | Update CLI example, env var table entry |
+
+## Proposed Approach
+
+### 1. Backward-compatible env var lookup in main.go
+
+Replace:
+```go
+giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", ""), "Gitea instance URL")
+```
+
+With:
+```go
+giteaURL := flag.String("vcs-url", envOrDefaultFallback("VCS_URL", "GITEA_URL", ""), "VCS server URL (e.g. https://gitea.example.com)")
+```
+
+Add a helper:
+```go
+// envOrDefaultFallback reads primary env var; if empty, falls back to deprecated env var.
+func envOrDefaultFallback(primary, deprecated, defaultVal string) string {
+    if v := os.Getenv(primary); v != "" {
+        return v
+    }
+    if v := os.Getenv(deprecated); v != "" {
+        slog.Warn("deprecated env var in use; rename to " + primary, "old", deprecated, "new", primary)
+        return v
+    }
+    return defaultVal
+}
+```
+
+**Note:** This must be called AFTER `setupLogger` conceptually, but the flag default is evaluated at flag registration time. Since `setupLogger` runs before `flag.Parse()`, the slog.Warn will print correctly at runtime. We use `log.Printf` as a fallback if this proves problematic.
+
+Actually — flag defaults are evaluated at registration (line 57), before `setupLogger`. The warning won't go through slog. Two options:
+- Use `log.Printf` for the deprecation warning (always visible)
+- Move the fallback lookup to after `flag.Parse()`, checking if the parsed value is still empty
+
+**Decision:** Move fallback to a post-parse check. This is cleaner:
+```go
+vcsURL := flag.String("vcs-url", os.Getenv("VCS_URL"), "VCS server URL")
+flag.Parse()
+// Backward compat: fall back to deprecated GITEA_URL
+if *vcsURL == "" {
+    if v := os.Getenv("GITEA_URL"); v != "" {
+        slog.Warn("GITEA_URL is deprecated; use VCS_URL instead")
+        *vcsURL = v
+    }
+}
+```
+
+This is clean, idiomatic, and the warning goes through slog correctly.
+
+### 2. Keep `--gitea-url` as deprecated alias
+
+Add a hidden flag for backward compat:
+```go
+giteaURLAlias := flag.String("gitea-url", "", "Deprecated: use --vcs-url")
+```
+
+Post-parse:
+```go
+if *vcsURL == "" && *giteaURLAlias != "" {
+    slog.Warn("--gitea-url is deprecated; use --vcs-url instead")
+    *vcsURL = *giteaURLAlias
+}
+```
+
+### 3. Internal variable rename
+
+Rename `giteaURL` local variable → `vcsURL` throughout `main.go` for consistency.
+
+### 4. Error message update
+
+```go
+fmt.Fprintf(os.Stderr, "Required: --vcs-url, --repo, --pr, --reviewer-token, --llm-model\n")
+```
+
+### 5. Action YAML changes
+
+In `.gitea/actions/review/action.yml`:
+- Input `gitea-url` → `vcs-url` (with same description, `required: false`, `default: ''`)
+- Line 172: `GITEA_URL: ${{ inputs.gitea-url || github.server_url }}` → `VCS_URL: ${{ inputs.vcs-url || github.server_url }}`
+- Lines 115, 140: internal bash vars `GITEA_URL=` are used for downloading binaries — NOT passed to the review-bot binary. Leave them as internal bash vars (they're scope-local in bash). These could be renamed to `SERVER_URL` or `BASE_URL` for local clarity, but renaming them isn't strictly required.
+
+In `.gitea/workflows/ci.yml`:
+- Line 52: `GITEA_URL: ${{ github.server_url }}` → `VCS_URL: ${{ github.server_url }}`
+
+### 6. Integration test updates
+
+`INTEGRATION_GITEA_URL` → `INTEGRATION_VCS_URL` in both test files.
+
+### 7. README
+
+- CLI example: `--gitea-url` → `--vcs-url`
+- Env var table: `GITEA_URL` → `VCS_URL`, add note about `GITEA_URL` fallback
+
+## Backward Compatibility Summary
+
+| Old | New | Fallback? |
+|-----|-----|-----------|
+| `GITEA_URL` env var | `VCS_URL` | ✅ with deprecation warning |
+| `--gitea-url` flag | `--vcs-url` | ✅ with deprecation warning |
+| `gitea-url` action input | `vcs-url` | ⚠️ No (action version bump handles this) |
+| `INTEGRATION_GITEA_URL` | `INTEGRATION_VCS_URL` | N/A (test-only) |
+
+## Error Cases
+
+- Both `VCS_URL` and `GITEA_URL` set: `VCS_URL` wins (primary takes precedence)
+- Both `--vcs-url` and `--gitea-url` provided: `--vcs-url` wins
+- Neither set: existing "missing required flags" error unchanged
+
+## Edge Cases
+
+- `os.Getenv` returns "" for unset AND set-to-empty — consistent with existing behavior
+- The `envOrDefault` helper is unchanged; we add `envOrDefaultFallback` for the one renamed var
+
+## Testing Strategy
+
+- Existing unit tests pass unchanged (they don't test env var parsing directly)
+- Integration tests updated to use new env var name
+- Manual: `GITEA_URL=https://example.com ./review-bot --repo x --pr 1 ...` should print deprecation warning and proceed
+- Manual: `VCS_URL=https://example.com ./review-bot ...` should work silently
+
+## Completion Checklist
+
+1. `VCS_URL` is read first; `GITEA_URL` is fallback with deprecation warning
+2. `--vcs-url` flag is primary; `--gitea-url` is deprecated alias with warning
+3. Error message references `--vcs-url` not `--gitea-url`
+4. `action.yml` passes `VCS_URL` (not `GITEA_URL`) to the binary
+5. `ci.yml` passes `VCS_URL` (not `GITEA_URL`) to the binary
+6. README updated in CLI example and env var table
+7. Integration tests use `INTEGRATION_VCS_URL`
+8. `go test ./...` passes
+9. `go vet ./...` passes
+10. `go build ./cmd/review-bot` succeeds
+
+## Open Questions
+
+- Should the CLI flag `--gitea-url` be completely hidden from `--help` or just deprecated with a note? The issue doesn't specify. Decision: keep it visible but add "(deprecated: use --vcs-url)" to the description.
+- Should action.yml also add `gitea-url` as a deprecated input alias? The issue says "Update the action to pass the new env var name" — no mention of backward compat for the action input. Decision: rename only, no alias (action users pin a version anyway).
+- The bash-internal `GITEA_URL` variable in action.yml scripts (used for release download, not passed to binary) — rename for clarity? Decision: yes, rename to `BASE_URL` to avoid confusion with the env var.
@@ -282,7 +282,7 @@ Rules:

 ```bash
 review-bot \
-  --gitea-url https://gitea.example.com \
+  --vcs-url https://gitea.example.com \
  --repo owner/name \
  --pr 42 \
  --reviewer-token "$GITEA_TOKEN" \
@@ -299,7 +299,7 @@ All flags have environment variable equivalents:

 | Flag | Env Var |
 |------|---------|
-| `--gitea-url` | `GITEA_URL` |
+| `--vcs-url` | `VCS_URL` (fallback: `GITEA_URL`) |
 | `--repo` | `GITEA_REPO` |
 | `--pr` | `PR_NUMBER` |
 | `--reviewer-token` | `REVIEWER_TOKEN` |
@@ -0,0 +1,79 @@
+## Dev Loop: review-bot — 2026-05-14 20:10 UTC
+
+### Latest: ✅ STABLE STATE — REPO HEALTH COMPLETE
+- **Last action:** health check; verified tests pass, repo clean, no action needed
+- **Repository:** Clean, all merges complete, no open issues/PRs
+- **Main branch:** Up to date with origin/main
+- **Test suite:** All passing (cached)
+
+---
+
+## Repository Status
+
+### ✅ Merged to main (recent):
+- issue-123 (IP-level SSRF defense) — 6 commits, main at 4440823
+- issue-125 (VCS_URL rename + deprecation) — merged
+- issue-124 (multi-arch binary support) — merged  
+- issue-120 (GitHub Actions + VCS abstraction) — merged
+- issue-121 (VCS host type detection for binary download) — merged
+
+### 🧹 Cleanup COMPLETE:
+- ✅ Removed old worktrees (issue-123, review-bot-issue-125)
+- ✅ Test suite passes (all packages)
+- ✅ No TODO/FIXME in code except expected GitHub client notes
+- ✅ No open issues or pull requests
+- ✅ Dependencies up to date
+
+---
+
+## Current Feature Completeness
+
+✅ **Core Capabilities:**
+- Multi-provider LLM support (OpenAI, Anthropic, SAP AI Core)
+- Gitea PR integration with structured reviews
+- SSRF defense with IP-level validation
+- VCS abstraction (Gitea/GitHub support)
+- Multi-architecture binary support
+- GitHub Actions composite action
+
+✅ **Recent Security Work:**
+- RFC6598 CGN range detection
+- IP fallback dialing for local endpoint rejection
+- URL validation for SSRF prevention
+
+✅ **Code Quality:**
+- Comprehensive test coverage (all packages tested)
+- Consistent error handling with context propagation
+- Secure credential handling (unexported fields)
+- Concurrency-safe designs
+
+---
+
+## Next Priority Actions
+
+### Phase 2: Feature Exploration (NEXT SESSION)
+- Scan code for potential improvements per REVIEW.md findings
+- Assess performance under load
+- Review REVIEW.md findings for targeted fixes
+- Consider backlog items from design docs
+
+### Phase 3: Optional Enhancements (BACKLOG)
+- Address REVIEW.md context propagation findings (if prioritized)
+- Additional LLM provider support
+- Enhanced context detection
+- Custom report formats
+- Webhook management improvements
+
+---
+
+## Worktrees Status
+All old worktrees cleaned up. Ready for new issue work.
+
+---
+
+## Dev-Loop Metadata
+- **Repo:** /home/ubuntu/review-bot
+- **Main branch SHA:** ed3a5dd (last commit)
+- **Cron ID:** 5342ac81-4bbc-4e4c-a123-347a7788d50c
+- **Scheduled:** Every 4 hours
+- **Last health check:** 2026-05-14 20:10 UTC (✅ all healthy)
@@ -17,7 +17,7 @@ import (
 // Integration test requires a running Gitea instance and LLM endpoint.
 // Set environment variables:
 //
-//	INTEGRATION_GITEA_URL     - Gitea base URL
+//	INTEGRATION_VCS_URL       - VCS base URL
 //	INTEGRATION_GITEA_TOKEN   - Gitea API token with repo access
 //	INTEGRATION_GITEA_REPO    - owner/repo with an open PR
 //	INTEGRATION_PR_NUMBER     - PR number to test against
@@ -25,7 +25,7 @@ import (
 //	INTEGRATION_LLM_API_KEY   - LLM API key
 //	INTEGRATION_LLM_MODEL     - Model name
 func TestIntegration_FullReviewFlow(t *testing.T) {
-	giteaURL := os.Getenv("INTEGRATION_GITEA_URL")
+	giteaURL := os.Getenv("INTEGRATION_VCS_URL")
 	giteaToken := os.Getenv("INTEGRATION_GITEA_TOKEN")
 	giteaRepo := os.Getenv("INTEGRATION_GITEA_REPO")
 	prNumStr := os.Getenv("INTEGRATION_PR_NUMBER")
@@ -104,7 +104,7 @@ func TestIntegration_FullReviewFlow(t *testing.T) {
 }

 func TestIntegration_PostAndCleanup(t *testing.T) {
-	giteaURL := os.Getenv("INTEGRATION_GITEA_URL")
+	giteaURL := os.Getenv("INTEGRATION_VCS_URL")
 	giteaToken := os.Getenv("INTEGRATION_GITEA_TOKEN")
 	giteaRepo := os.Getenv("INTEGRATION_GITEA_REPO")
 	prNumStr := os.Getenv("INTEGRATION_PR_NUMBER")
@@ -4,6 +4,7 @@ import (
 	"context"
 	"flag"
 	"fmt"
+	"io"
 	"log/slog"
 	"os"
 	"path/filepath"
@@ -13,12 +14,21 @@ import (

 	"gitea.weiker.me/rodin/review-bot/budget"
 	"gitea.weiker.me/rodin/review-bot/gitea"
+	"gitea.weiker.me/rodin/review-bot/github"
 	"gitea.weiker.me/rodin/review-bot/llm"
 	"gitea.weiker.me/rodin/review-bot/review"
+	"gitea.weiker.me/rodin/review-bot/vcs"
 )

 var version = "dev"

+// outWriter and errWriter are the output and error writers for subcommands.
+// They are variables so tests can capture output.
+var (
+	outWriter io.Writer = os.Stdout
+	errWriter io.Writer = os.Stderr
+)
+
 // setupLogger configures the global slog default logger based on format and verbosity.
 func setupLogger(format, verbosity string) {
 	var level slog.Level
@@ -49,12 +59,22 @@ func setupLogger(format, verbosity string) {
 }

 func main() {
+	// Dispatch subcommands before flag parsing so they get their own args.
+	// e.g. `review-bot validate-url <url>`
+	if len(os.Args) > 1 {
+		switch os.Args[1] {
+		case "validate-url":
+			os.Exit(runValidateURL(os.Args[2:]))
+		}
+	}
+
 	versionFlag := flag.Bool("version", false, "Print version and exit")
 	// Logging flags
 	logFormat := flag.String("log-format", envOrDefault("LOG_FORMAT", "text"), "Log output format: text or json")
 	verbosity := flag.String("verbosity", envOrDefault("LOG_VERBOSITY", "info"), "Log verbosity: debug, info, warn, error")
 	// CLI flags
-	giteaURL := flag.String("gitea-url", envOrDefault("GITEA_URL", ""), "Gitea instance URL")
+	vcsURL := flag.String("vcs-url", os.Getenv("VCS_URL"), "VCS server URL (e.g. https://gitea.example.com)")
+	giteaURLAlias := flag.String("gitea-url", "", "Deprecated: use --vcs-url")
 	repo := flag.String("repo", envOrDefault("GITEA_REPO", ""), "Repository (owner/name)")
 	prNum := flag.String("pr", envOrDefault("PR_NUMBER", ""), "Pull request number")
 	reviewerName := flag.String("reviewer-name", envOrDefault("REVIEWER_NAME", ""), "Reviewer display name")
@@ -66,6 +86,7 @@ func main() {
 	systemPromptFile := flag.String("system-prompt-file", envOrDefault("SYSTEM_PROMPT_FILE", ""), "Local file with additional system prompt instructions")
 	patternsRepo := flag.String("patterns-repo", envOrDefault("PATTERNS_REPO", ""), "Repo with language patterns (e.g. rodin/elixir-patterns)")
 	patternsFiles := flag.String("patterns-files", envOrDefault("PATTERNS_FILES", ""), "Comma-separated file paths to fetch from patterns repo (empty = all files)")
+	vcsType := flag.String("vcs-type", envOrDefault("VCS_TYPE", "gitea"), "VCS type: gitea or github")
 	dryRun := flag.Bool("dry-run", false, "Print review to stdout instead of posting")
 	llmTemp := flag.Float64("llm-temperature", envOrDefaultFloat("LLM_TEMPERATURE", 0), "LLM temperature (0 = server default)")
 	llmTimeout := flag.Int("llm-timeout", envOrDefaultInt("LLM_TIMEOUT", 300), "LLM request timeout in seconds (default 300)")
@@ -91,12 +112,24 @@ func main() {

 	slog.Info("review-bot starting", "version", version)

+	// Backward compatibility: fall back to deprecated env var / flag if VCS_URL / --vcs-url not set.
+	if *vcsURL == "" {
+		if v := os.Getenv("GITEA_URL"); v != "" {
+			slog.Warn("GITEA_URL is deprecated; rename the environment variable to VCS_URL")
+			*vcsURL = v
+		}
+	}
+	if *vcsURL == "" && *giteaURLAlias != "" {
+		slog.Warn("--gitea-url is deprecated; use --vcs-url instead")
+		*vcsURL = *giteaURLAlias
+	}
+
 	// Validate required fields
 	// For aicore provider, llm-base-url and llm-api-key are not required
 	isAICore := llm.Provider(*llmProvider) == llm.ProviderAICore
-	if *giteaURL == "" || *repo == "" || *prNum == "" || *reviewerToken == "" || *llmModel == "" {
+	if *vcsURL == "" || *repo == "" || *prNum == "" || *reviewerToken == "" || *llmModel == "" {
 		fmt.Fprintf(os.Stderr, "Error: missing required flags or environment variables\n\n")
-		fmt.Fprintf(os.Stderr, "Required: --gitea-url, --repo, --pr, --reviewer-token, --llm-model\n")
+		fmt.Fprintf(os.Stderr, "Required: --vcs-url, --repo, --pr, --reviewer-token, --llm-model\n")
 		os.Exit(1)
 	}
 	if !isAICore && (*llmBaseURL == "" || *llmAPIKey == "") {
@@ -138,8 +171,19 @@ func main() {
 		os.Exit(1)
 	}

-	// Initialize clients
-	giteaClient := gitea.NewClient(*giteaURL, *reviewerToken)
+	// Initialize VCS client
+	var vcsClientImpl vcsClient
+	switch strings.ToLower(*vcsType) {
+	case "github":
+		vcsClientImpl = newGitHubVCSAdapter(github.NewClient(*reviewerToken, *vcsURL))
+		slog.Info("using GitHub VCS client", "url", *vcsURL)
+	case "gitea", "":
+		vcsClientImpl = newGiteaVCSAdapter(gitea.NewClient(*vcsURL, *reviewerToken))
+		slog.Info("using Gitea VCS client", "url", *vcsURL)
+	default:
+		slog.Error("invalid vcs-type", "type", *vcsType, "valid", "gitea, github")
+		os.Exit(1)
+	}
 	llmClient := llm.NewClient(*llmBaseURL, *llmAPIKey, *llmModel)
 	if *llmTemp < 0 || *llmTemp > 2 {
 		slog.Error("invalid LLM temperature", "temperature", *llmTemp, "range", "0-2")
@@ -177,7 +221,7 @@ func main() {
 	var persona *review.Persona
 	if *personaName != "" {
 		// Try loading from repo first, then fall back to built-in
-		repoPersonas, err := review.LoadRepoPersonas(ctx, newGiteaClientAdapter(giteaClient), owner, repoName)
+		repoPersonas, err := review.LoadRepoPersonas(ctx, newReviewClientAdapter(vcsClientImpl), owner, repoName)
 		if err != nil {
 			slog.Warn("could not load repo personas", "repo", owner+"/"+repoName, "error", err)
 			// Continue with built-in personas only.
@@ -213,7 +257,7 @@ func main() {
 	slog.Info("reviewing pull request", "pr", prNumber, "repo", fmt.Sprintf("%s/%s", owner, repoName))

 	// Step 1: Fetch PR metadata
-	pr, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber)
+	pr, err := vcsClientImpl.GetPullRequest(ctx, owner, repoName, prNumber)
 	if err != nil {
 		slog.Error("failed to fetch PR", "pr", prNumber, "error", err)
 		os.Exit(1)
@@ -221,7 +265,7 @@ func main() {
 	slog.Info("fetched PR metadata", "pr", prNumber, "title", pr.Title)

 	// Step 2: Fetch diff
-	diff, err := giteaClient.GetPullRequestDiff(ctx, owner, repoName, prNumber)
+	diff, err := vcsClientImpl.GetPullRequestDiff(ctx, owner, repoName, prNumber)
 	if err != nil {
 		slog.Error("failed to fetch diff", "pr", prNumber, "error", err)
 		os.Exit(1)
@@ -230,21 +274,21 @@ func main() {

 	// Step 3: Fetch full file content for modified files
 	fileContext := ""
-	files, err := giteaClient.GetPullRequestFiles(ctx, owner, repoName, prNumber)
+	files, err := vcsClientImpl.GetPullRequestFiles(ctx, owner, repoName, prNumber)
 	if err != nil {
 		slog.Warn("could not fetch PR files list", "pr", prNumber, "error", err)
 	} else {
-		fileContext = fetchFileContext(ctx, giteaClient, owner, repoName, pr.Head.Ref, files)
+		fileContext = fetchFileContext(ctx, vcsClientImpl, owner, repoName, pr.HeadRef, files)
 		slog.Debug("fetched file context", "files", len(files))
 	}

 	// Step 4: Check CI status
 	ciPassed := true
 	ciDetails := ""
-	if pr.Head.Sha != "" {
-		statuses, err := giteaClient.GetCommitStatuses(ctx, owner, repoName, pr.Head.Sha)
+	if pr.HeadSha != "" {
+		statuses, err := vcsClientImpl.GetCommitStatuses(ctx, owner, repoName, pr.HeadSha)
 		if err != nil {
-			slog.Warn("could not fetch CI status", "sha", pr.Head.Sha, "error", err)
+			slog.Warn("could not fetch CI status", "sha", pr.HeadSha, "error", err)
 		} else {
 			ciPassed, ciDetails = evaluateCIStatus(statuses)
 			slog.Info("CI status checked", "passed", ciPassed)
@@ -254,7 +298,7 @@ func main() {
 	// Step 5: Load conventions file if specified
 	conventions := ""
 	if *conventionsFile != "" {
-		content, err := giteaClient.GetFileContent(ctx, owner, repoName, *conventionsFile)
+		content, err := vcsClientImpl.GetFileContent(ctx, owner, repoName, *conventionsFile)
 		if err != nil {
 			slog.Warn("could not load conventions file", "file", *conventionsFile, "error", err)
 		} else {
@@ -266,7 +310,7 @@ func main() {
 	// Step 6: Load patterns from external repo if specified
 	patterns := ""
 	if *patternsRepo != "" {
-		patterns = fetchPatterns(ctx, giteaClient, *patternsRepo, *patternsFiles)
+		patterns = fetchPatterns(ctx, vcsClientImpl, *patternsRepo, *patternsFiles)
 		slog.Debug("loaded patterns", "repo", *patternsRepo, "bytes", len(patterns))
 	}

@@ -359,15 +403,15 @@ func main() {
 	}

 	// Add commit footer so readers know which commit was evaluated
-	if pr.Head.Sha != "" {
-		shortSHA := pr.Head.Sha
+	if pr.HeadSha != "" {
+		shortSHA := pr.HeadSha
 		if len(shortSHA) > 8 {
 			shortSHA = shortSHA[:8]
 		}
 		reviewBody += fmt.Sprintf("\n\n---\n*Evaluated against %s*", shortSHA)
 	}

-	event := review.GiteaEvent(result.Verdict)
+	event := verdictToVCSEvent(result.Verdict)

 	if *dryRun {
 		fmt.Println("--- DRY RUN ---")
@@ -379,14 +423,14 @@ func main() {
 	sentinel := fmt.Sprintf("<!-- review-bot:%s -->", *reviewerName)

 	// Stale check: verify HEAD hasn't moved since we started
-	evaluatedSHA := pr.Head.Sha
+	evaluatedSHA := pr.HeadSha
 	var currentSHA string
-	currentPR, err := giteaClient.GetPullRequest(ctx, owner, repoName, prNumber)
+	currentPR, err := vcsClientImpl.GetPullRequest(ctx, owner, repoName, prNumber)
 	if err != nil {
 		slog.Warn("could not re-fetch PR for stale check", "pr", prNumber, "error", err)
 		// currentSHA stays empty — shouldSkipStaleReview will return false
 	} else {
-		currentSHA = currentPR.Head.Sha
+		currentSHA = currentPR.HeadSha
 	}
 	if shouldSkipStaleReview(evaluatedSHA, currentSHA) {
 		slog.Warn("HEAD moved during review — skipping stale review",
@@ -397,28 +441,30 @@ func main() {
 	}

 	// Map findings to inline comments for lines present in the diff
-	diffRanges := gitea.ParseDiffNewLines(diff)
-	var inlineComments []gitea.ReviewComment
-	for _, f := range result.Findings {
-		if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) {
-			inlineComments = append(inlineComments, gitea.ReviewComment{
-				Path:        f.File,
-				NewPosition: int64(f.Line),
-				Body:        fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding),
-			})
+	var inlineComments []vcs.ReviewComment
+	if ext, ok := vcsClientImpl.(giteaExtendedClient); ok {
+		diffRanges := ext.ParseDiffNewLines(diff)
+		for _, f := range result.Findings {
+			if f.File != "" && f.Line > 0 && diffRanges.Contains(f.File, f.Line) {
+				inlineComments = append(inlineComments, vcs.ReviewComment{
+					Path:     f.File,
+					Position: f.Line,
+					Body:     fmt.Sprintf("**[%s]** %s", f.Severity, f.Finding),
+				})
+			}
+		}
+		if len(inlineComments) > 0 {
+			slog.Debug("attaching inline comments", "count", len(inlineComments))
 		}
-	}
-	if len(inlineComments) > 0 {
-		slog.Debug("attaching inline comments", "count", len(inlineComments))
 	}

 	// --- Review update strategy ---
 	// 1. POST new review first (gets non-stale approval badge on HEAD)
 	// 2. Then supersede old review with link to the new one
 	// Order matters: post first so we have the new review's URL for the supersede message.
-	var oldReviews []gitea.Review
+	var oldReviews []reviewInfo
 	if *reviewerName != "" {
-		existingReviews, err := giteaClient.ListReviews(ctx, owner, repoName, prNumber)
+		existingReviews, err := vcsClientImpl.ListReviews(ctx, owner, repoName, prNumber)
 		if err != nil {
 			slog.Warn("could not list existing reviews", "pr", prNumber, "error", err)
 		} else {
@@ -431,20 +477,27 @@ func main() {
 	}

 	// Self-request as reviewer (ensures we appear in required-reviewer checks)
-	authUser, err := giteaClient.GetAuthenticatedUser(ctx)
+	authUser, err := vcsClientImpl.GetAuthenticatedUser(ctx)
 	if err != nil {
 		slog.Warn("could not determine authenticated user for reviewer self-request", "error", err)
 	} else if authUser != "" {
-		if err := giteaClient.RequestReviewer(ctx, owner, repoName, prNumber, authUser); err != nil {
-			slog.Warn("could not self-request as reviewer", "user", authUser, "error", err)
-		} else {
-			slog.Debug("self-requested as reviewer", "user", authUser, "pr", prNumber)
+		if ext, ok := vcsClientImpl.(giteaExtendedClient); ok {
+			if err := ext.RequestReviewer(ctx, owner, repoName, prNumber, authUser); err != nil {
+				slog.Warn("could not self-request as reviewer", "user", authUser, "error", err)
+			} else {
+				slog.Debug("self-requested as reviewer", "user", authUser, "pr", prNumber)
+			}
 		}
 	}

 	// POST new review
 	slog.Info("posting review", "event", event, "pr", prNumber)
-	posted, err := giteaClient.PostReview(ctx, owner, repoName, prNumber, event, reviewBody, evaluatedSHA, inlineComments)
+	posted, err := vcsClientImpl.PostReview(ctx, owner, repoName, prNumber, vcs.ReviewRequest{
+		Body:     reviewBody,
+		Event:    event,
+		CommitID: evaluatedSHA,
+		Comments: inlineComments,
+	})
 	if err != nil {
 		slog.Error("failed to post review", "pr", prNumber, "event", event, "error", err)
 		os.Exit(1)
@@ -453,22 +506,27 @@ func main() {

 	// Supersede all old reviews with link to the new one
 	if len(oldReviews) > 0 {
-		newReviewURL := fmt.Sprintf("%s/%s/%s/pulls/%d#pullrequestreview-%d", strings.TrimRight(*giteaURL, "/"), owner, repoName, prNumber, posted.ID)
+		newReviewURL := fmt.Sprintf("%s/%s/%s/pulls/%d#pullrequestreview-%d", strings.TrimRight(*vcsURL, "/"), owner, repoName, prNumber, posted.ID)
+		ext, hasExt := vcsClientImpl.(giteaExtendedClient)
 		for _, oldReview := range oldReviews {
-			cid, err := giteaClient.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, prNumber, oldReview.ID)
+			if !hasExt {
+				slog.Debug("VCS client does not support review supersede; skipping", "review_id", oldReview.ID)
+				continue
+			}
+			cid, err := ext.GetTimelineReviewCommentIDForReview(ctx, owner, repoName, prNumber, oldReview.ID)
 			if err != nil {
 				slog.Warn("could not find comment ID for old review", "review_id", oldReview.ID, "error", err)
 				continue
 			}
 			supersededBody := buildSupersededBody(oldReview.Body, oldReview.CommitID, newReviewURL, sentinel)
-			if err := giteaClient.EditComment(ctx, owner, repoName, cid, supersededBody); err != nil {
+			if err := ext.EditComment(ctx, owner, repoName, cid, supersededBody); err != nil {
 				slog.Warn("could not mark old review as superseded", "review_id", oldReview.ID, "comment_id", cid, "error", err)
 				continue
 			}
 			slog.Info("marked old review as superseded", "review_id", oldReview.ID, "new_review_id", posted.ID, "pr", prNumber)

 			// Resolve old review's inline comments
-			oldComments, err := giteaClient.ListReviewComments(ctx, owner, repoName, prNumber, oldReview.ID)
+			oldComments, err := ext.ListReviewComments(ctx, owner, repoName, prNumber, oldReview.ID)
 			if err != nil {
 				slog.Warn("could not list old review comments for resolution", "review_id", oldReview.ID, "error", err)
 				continue
@@ -478,7 +536,7 @@ func main() {
 				if c.ID == 0 {
 					continue
 				}
-				if err := giteaClient.ResolveComment(ctx, owner, repoName, c.ID); err != nil {
+				if err := ext.ResolveComment(ctx, owner, repoName, c.ID); err != nil {
 					slog.Debug("could not resolve inline comment", "comment_id", c.ID, "error", err)
 					failed++
 				} else {
@@ -497,7 +555,7 @@ func main() {
 }

 // fetchFileContext fetches the full content of modified files from the PR branch.
-func fetchFileContext(ctx context.Context, client *gitea.Client, owner, repo, ref string, files []gitea.ChangedFile) string {
+func fetchFileContext(ctx context.Context, client vcsClient, owner, repo, ref string, files []changedFileInfo) string {
 	var sb strings.Builder
 	for _, f := range files {
 		if ctx.Err() != nil {
@@ -524,7 +582,7 @@ func fetchFileContext(ctx context.Context, client *gitea.Client, owner, repo, re
 // patternsFiles is comma-separated list of file paths or directories.
 // If a path ends with / or is a directory, all files within it are fetched recursively.
 // If patternsFiles is empty, all files from the repo root are fetched.
-func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patternsFiles string) string {
+func fetchPatterns(ctx context.Context, client vcsClient, patternsRepo, patternsFiles string) string {
 	var sb strings.Builder

 	repos := strings.Split(patternsRepo, ",")
@@ -561,8 +619,13 @@ func fetchPatterns(ctx context.Context, client *gitea.Client, patternsRepo, patt
 		var repoLoadedFiles []string
 		var repoSkippedFiles []string

+		giteaRaw, isGitea := client.(*giteaClientVCSAdapter)
+		if !isGitea {
+			slog.Warn("patterns fetching is only supported with the Gitea VCS client; skipping", "repo", repoRef)
+			continue
+		}
 		for _, path := range paths {
-			files, err := client.GetAllFilesInPath(ctx, owner, repo, path)
+			files, err := giteaRaw.client.GetAllFilesInPath(ctx, owner, repo, path)
 			if err != nil {
 				slog.Warn("could not fetch patterns", "path", path, "repo", repoRef, "error", err)
 				continue
@@ -601,7 +664,7 @@ func isPatternFile(path string) bool {
 }

 // evaluateCIStatus checks if all CI statuses indicate success.
-func evaluateCIStatus(statuses []gitea.CommitStatus) (passed bool, details string) {
+func evaluateCIStatus(statuses []commitStatusInfo) (passed bool, details string) {
 	if len(statuses) == 0 {
 		return true, "no CI statuses found"
 	}
@@ -739,7 +802,7 @@ func buildSupersededBody(originalBody, commitSHA, newReviewURL, sentinel string)
 // Gitea user. This indicates misconfiguration where two roles share a token
 // instead of having separate Gitea accounts. Returns true if shared token
 // detected (caller should skip update-in-place logic to avoid clobbering).
-func hasSharedToken(reviews []gitea.Review, ownSentinel string) bool {
+func hasSharedToken(reviews []reviewInfo, ownSentinel string) bool {
 	ownLogin := ""
 	for _, r := range reviews {
 		if strings.Contains(r.Body, ownSentinel) {
@@ -777,8 +840,8 @@ func extractSentinelName(body string) string {
 }

 // findOwnReview locates the most recent non-superseded review matching the sentinel.
-func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review {
-	var best *gitea.Review
+func findOwnReview(reviews []reviewInfo, sentinel string) *reviewInfo {
+	var best *reviewInfo
 	for i := range reviews {
 		if !strings.Contains(reviews[i].Body, sentinel) {
 			continue
@@ -794,8 +857,8 @@ func findOwnReview(reviews []gitea.Review, sentinel string) *gitea.Review {
 }

 // findAllOwnReviews returns all non-superseded reviews matching the sentinel.
-func findAllOwnReviews(reviews []gitea.Review, sentinel string) []gitea.Review {
-	var result []gitea.Review
+func findAllOwnReviews(reviews []reviewInfo, sentinel string) []reviewInfo {
+	var result []reviewInfo
 	for i := range reviews {
 		if !strings.Contains(reviews[i].Body, sentinel) {
 			continue
@@ -808,6 +871,22 @@ func findAllOwnReviews(reviews []gitea.Review, sentinel string) []gitea.Review {
 	return result
 }

+// verdictToVCSEvent converts a review verdict string to a vcs.ReviewEvent.
+// The verdict comes from the LLM result and uses values: "APPROVE", "REQUEST_CHANGES",
+// or any other string (treated as COMMENT).
+// vcs.ReviewEvent constants follow GitHub API format ("APPROVE", "REQUEST_CHANGES", "COMMENT").
+// The Gitea adapter translates these back to Gitea format ("APPROVED", etc.) before posting.
+func verdictToVCSEvent(verdict string) vcs.ReviewEvent {
+	switch verdict {
+	case "APPROVE":
+		return vcs.ReviewEventApprove
+	case "REQUEST_CHANGES":
+		return vcs.ReviewEventRequestChanges
+	default:
+		return vcs.ReviewEventComment
+	}
+}
+
 // shouldSkipStaleReview reports whether to skip posting because HEAD moved.
 // Returns true (skip) if evaluatedSHA differs from currentSHA.
 // Returns false (don't skip) if:
@@ -821,16 +900,17 @@ func shouldSkipStaleReview(evaluatedSHA, currentSHA string) bool {
 	return evaluatedSHA != currentSHA
 }

-// giteaClientAdapter adapts gitea.Client to review.GiteaClient interface.
-type giteaClientAdapter struct {
-	client *gitea.Client
+// reviewClientAdapter adapts a vcsClient to review.GiteaClient for persona loading.
+// The review package only needs ListContents and GetFileContent, which all vcsClients provide.
+type reviewClientAdapter struct {
+	client vcsClient
 }

-func newGiteaClientAdapter(c *gitea.Client) *giteaClientAdapter {
-	return &giteaClientAdapter{client: c}
+func newReviewClientAdapter(c vcsClient) *reviewClientAdapter {
+	return &reviewClientAdapter{client: c}
 }

-func (a *giteaClientAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]review.ContentEntry, error) {
+func (a *reviewClientAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]review.ContentEntry, error) {
 	entries, err := a.client.ListContents(ctx, owner, repo, path)
 	if err != nil {
 		return nil, err
@@ -846,6 +926,6 @@ func (a *giteaClientAdapter) ListContents(ctx context.Context, owner, repo, path
 	return result, nil
 }

-func (a *giteaClientAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
+func (a *reviewClientAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
 	return a.client.GetFileContent(ctx, owner, repo, filepath)
 }
@@ -10,7 +10,7 @@ import (
 	"strings"
 	"testing"

-	"gitea.weiker.me/rodin/review-bot/gitea"
+	"gitea.weiker.me/rodin/review-bot/vcs"
 )

 func TestValidateReviewerName(t *testing.T) {
@@ -154,14 +154,14 @@ func TestValidateWorkspacePath(t *testing.T) {
 	}
 }

-func makeReview(id int64, login, state string, stale bool, body string) gitea.Review {
-	r := gitea.Review{
+func makeReview(id int64, login, state string, stale bool, body string) reviewInfo {
+	r := reviewInfo{
 		ID:    id,
 		Body:  body,
 		State: state,
 		Stale: stale,
 	}
-	r.User.Login = login
+	r.User = vcs.UserInfo{Login: login}
 	return r
 }

@@ -216,7 +216,7 @@ func TestBuildSupersededBodyShortSHA(t *testing.T) {
 func TestFindOwnReview(t *testing.T) {
 	tests := []struct {
 		name     string
-		reviews  []gitea.Review
+		reviews  []reviewInfo
 		sentinel string
 		wantID   int64
 		wantNil  bool
@@ -229,7 +229,7 @@ func TestFindOwnReview(t *testing.T) {
 		},
 		{
 			name: "found by sentinel",
-			reviews: []gitea.Review{
+			reviews: []reviewInfo{
 				makeReview(42, "bot", "APPROVED", false, "review body\n<!-- review-bot:sonnet -->"),
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
@@ -237,7 +237,7 @@ func TestFindOwnReview(t *testing.T) {
 		},
 		{
 			name: "wrong sentinel",
-			reviews: []gitea.Review{
+			reviews: []reviewInfo{
 				makeReview(42, "bot", "APPROVED", false, "body\n<!-- review-bot:gpt -->"),
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
@@ -245,7 +245,7 @@ func TestFindOwnReview(t *testing.T) {
 		},
 		{
 			name: "multiple reviews, returns first match",
-			reviews: []gitea.Review{
+			reviews: []reviewInfo{
 				makeReview(10, "bot", "APPROVED", false, "old\n<!-- review-bot:gpt -->"),
 				makeReview(20, "bot", "APPROVED", false, "new\n<!-- review-bot:sonnet -->"),
 			},
@@ -254,7 +254,7 @@ func TestFindOwnReview(t *testing.T) {
 		},
 		{
 			name: "skips superseded review",
-			reviews: []gitea.Review{
+			reviews: []reviewInfo{
 				makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n**Superseded**\n<!-- review-bot:sonnet -->"),
 				makeReview(20, "bot", "APPROVED", false, "fresh review\n<!-- review-bot:sonnet -->"),
 			},
@@ -263,7 +263,7 @@ func TestFindOwnReview(t *testing.T) {
 		},
 		{
 			name: "only superseded reviews exist",
-			reviews: []gitea.Review{
+			reviews: []reviewInfo{
 				makeReview(10, "bot", "APPROVED", false, "~~Original review~~\n\n<!-- review-bot:sonnet -->"),
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
@@ -271,7 +271,7 @@ func TestFindOwnReview(t *testing.T) {
 		},
 		{
 			name: "picks highest ID among matches",
-			reviews: []gitea.Review{
+			reviews: []reviewInfo{
 				makeReview(50, "bot", "APPROVED", false, "v1\n<!-- review-bot:sonnet -->"),
 				makeReview(30, "bot", "APPROVED", false, "v0\n<!-- review-bot:sonnet -->"),
 			},
@@ -302,7 +302,7 @@ func TestFindOwnReview(t *testing.T) {
 func TestHasSharedToken(t *testing.T) {
 	tests := []struct {
 		name     string
-		reviews  []gitea.Review
+		reviews  []reviewInfo
 		sentinel string
 		want     bool
 	}{
@@ -314,36 +314,36 @@ func TestHasSharedToken(t *testing.T) {
 		},
 		{
 			name: "no own review yet - cannot detect",
-			reviews: []gitea.Review{
-				{ID: 1, User: struct{ Login string `json:"login"` }{Login: "other"}, Body: "<!-- review-bot:gpt --> body"},
+			reviews: []reviewInfo{
+				{ID: 1, User: vcs.UserInfo{Login: "other"}, Body: "<!-- review-bot:gpt --> body"},
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
 			want:     false,
 		},
 		{
 			name: "separate users - no shared token",
-			reviews: []gitea.Review{
-				{ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
-				{ID: 2, User: struct{ Login string `json:"login"` }{Login: "security-review-bot"}, Body: "<!-- review-bot:security --> body"},
+			reviews: []reviewInfo{
+				{ID: 1, User: vcs.UserInfo{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
+				{ID: 2, User: vcs.UserInfo{Login: "security-review-bot"}, Body: "<!-- review-bot:security --> body"},
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
 			want:     false,
 		},
 		{
 			name: "shared token detected - same user different sentinels",
-			reviews: []gitea.Review{
-				{ID: 1, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
-				{ID: 2, User: struct{ Login string `json:"login"` }{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:security --> body"},
+			reviews: []reviewInfo{
+				{ID: 1, User: vcs.UserInfo{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:sonnet --> body"},
+				{ID: 2, User: vcs.UserInfo{Login: "sonnet-review-bot"}, Body: "<!-- review-bot:security --> body"},
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
 			want:     true,
 		},
 		{
 			name: "three roles same user",
-			reviews: []gitea.Review{
-				{ID: 1, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:sonnet --> body"},
-				{ID: 2, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:security --> body"},
-				{ID: 3, User: struct{ Login string `json:"login"` }{Login: "bot"}, Body: "<!-- review-bot:gpt --> body"},
+			reviews: []reviewInfo{
+				{ID: 1, User: vcs.UserInfo{Login: "bot"}, Body: "<!-- review-bot:sonnet --> body"},
+				{ID: 2, User: vcs.UserInfo{Login: "bot"}, Body: "<!-- review-bot:security --> body"},
+				{ID: 3, User: vcs.UserInfo{Login: "bot"}, Body: "<!-- review-bot:gpt --> body"},
 			},
 			sentinel: "<!-- review-bot:sonnet -->",
 			want:     true,
@@ -553,7 +553,7 @@ func TestBuildPatternPaths(t *testing.T) {
 func TestEvaluateCIStatus(t *testing.T) {
 	tests := []struct {
 		name       string
-		statuses   []gitea.CommitStatus
+		statuses   []commitStatusInfo
 		wantPassed bool
 		wantSubstr string
 	}{
@@ -565,7 +565,7 @@ func TestEvaluateCIStatus(t *testing.T) {
 		},
 		{
 			name: "all success",
-			statuses: []gitea.CommitStatus{
+			statuses: []commitStatusInfo{
 				{Status: "success", Context: "ci/build", Description: "Build passed"},
 				{Status: "success", Context: "ci/test", Description: "Tests passed"},
 			},
@@ -574,7 +574,7 @@ func TestEvaluateCIStatus(t *testing.T) {
 		},
 		{
 			name: "one failure",
-			statuses: []gitea.CommitStatus{
+			statuses: []commitStatusInfo{
 				{Status: "success", Context: "ci/build", Description: "Build passed"},
 				{Status: "failure", Context: "ci/test", Description: "Tests failed"},
 			},
@@ -583,7 +583,7 @@ func TestEvaluateCIStatus(t *testing.T) {
 		},
 		{
 			name: "error status",
-			statuses: []gitea.CommitStatus{
+			statuses: []commitStatusInfo{
 				{Status: "error", Context: "ci/lint", Description: "Lint error"},
 			},
 			wantPassed: false,
@@ -591,7 +591,7 @@ func TestEvaluateCIStatus(t *testing.T) {
 		},
 		{
 			name: "pending treated as not-failed",
-			statuses: []gitea.CommitStatus{
+			statuses: []commitStatusInfo{
 				{Status: "pending", Context: "ci/build", Description: "In progress"},
 				{Status: "success", Context: "ci/test", Description: "Tests passed"},
 			},
@@ -600,7 +600,7 @@ func TestEvaluateCIStatus(t *testing.T) {
 		},
 		{
 			name: "multiple failures",
-			statuses: []gitea.CommitStatus{
+			statuses: []commitStatusInfo{
 				{Status: "failure", Context: "ci/build", Description: "Build failed"},
 				{Status: "failure", Context: "ci/test", Description: "Tests failed"},
 			},
@@ -609,7 +609,7 @@ func TestEvaluateCIStatus(t *testing.T) {
 		},
 		{
 			name: "mixed with pending and failure",
-			statuses: []gitea.CommitStatus{
+			statuses: []commitStatusInfo{
 				{Status: "success", Context: "ci/build", Description: "Build passed"},
 				{Status: "pending", Context: "ci/deploy", Description: "Deploying"},
 				{Status: "failure", Context: "ci/test", Description: "Tests failed"},
@@ -997,7 +997,7 @@ func cleanEnv() []string {
 }

 func TestFindAllOwnReviews(t *testing.T) {
-	reviews := []gitea.Review{
+	reviews := []reviewInfo{
 		{ID: 1, Body: "<!-- review-bot:sonnet -->\nfirst review"},
 		{ID: 2, Body: "<!-- review-bot:gpt -->\nother bot"},
 		{ID: 3, Body: "<!-- review-bot:sonnet -->\nsecond review"},
@@ -0,0 +1,125 @@
+package main
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net"
+	"net/url"
+	"strings"
+	"time"
+
+	"gitea.weiker.me/rodin/review-bot/gitea"
+)
+
+// runValidateURL implements the `review-bot validate-url <url>` subcommand.
+//
+// It resolves the given URL's hostname and checks that every returned IP is
+// publicly routable (not RFC1918, loopback, link-local, or other reserved
+// ranges). The exit code communicates the result to callers:
+//
+//	0 — URL is safe to use
+//	1 — URL resolves to a blocked/private address
+//	2 — URL is malformed, has an unsafe scheme, or DNS lookup failed
+//
+// This is intended for use from action.yml shell steps that need to validate
+// a user-supplied URL before passing it to curl.
+func runValidateURL(args []string) int {
+	if len(args) != 1 {
+		fmt.Fprintln(errWriter, "usage: review-bot validate-url <url>")
+		fmt.Fprintln(errWriter, "")
+		fmt.Fprintln(errWriter, "Resolves <url> and verifies all resolved IPs are publicly routable.")
+		fmt.Fprintln(errWriter, "Exit 0=safe, 1=blocked, 2=error")
+		return 2
+	}
+	rawURL := args[0]
+
+	if err := validateURL(rawURL); err != nil {
+		fmt.Fprintf(errWriter, "Error: %v\n", err)
+		var ve *validateError
+		if isValidateError(err, &ve) {
+			return ve.code
+		}
+		return 2
+	}
+	fmt.Fprintf(outWriter, "OK: %s is safe\n", rawURL)
+	return 0
+}
+
+// validateError carries an exit code alongside a message.
+type validateError struct {
+	code    int
+	message string
+}
+
+func (e *validateError) Error() string { return e.message }
+
+// isValidateError checks if err is or wraps a *validateError and sets out.
+// Uses errors.As so that wrapped *validateError values (e.g. from fmt.Errorf("...: %w", &validateError{...}))
+// are also detected, making the function robust against future wrapping.
+func isValidateError(err error, out **validateError) bool {
+	if err == nil {
+		return false
+	}
+	return errors.As(err, out)
+}
+
+// validateURL checks that rawURL is safe for use as a Gitea server URL:
+//   - Must be https:// (not http://)
+//   - Must have no user-info (user:pass@host)
+//   - Must resolve to at least one IP, all of which are publicly routable
+func validateURL(rawURL string) error {
+	parsed, err := url.Parse(rawURL)
+	if err != nil {
+		return &validateError{code: 2, message: fmt.Sprintf("malformed URL %q: %v", rawURL, err)}
+	}
+
+	// Scheme check: only https is permitted.
+	if !strings.EqualFold(parsed.Scheme, "https") {
+		return &validateError{
+			code:    2,
+			message: fmt.Sprintf("URL scheme must be https (got %q)", parsed.Scheme),
+		}
+	}
+
+	// Reject user-info (user:password@host) to prevent credential embedding.
+	if parsed.User != nil {
+		return &validateError{
+			code:    2,
+			message: "URL must not contain user-info (user:password@host)",
+		}
+	}
+
+	host := parsed.Hostname()
+	if host == "" {
+		return &validateError{code: 2, message: fmt.Sprintf("URL has no host: %q", rawURL)}
+	}
+
+	// Resolve the hostname with a short timeout.
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	addrs, err := net.DefaultResolver.LookupIPAddr(ctx, host)
+	if err != nil {
+		return &validateError{
+			code:    2,
+			message: fmt.Sprintf("DNS lookup failed for %q: %v", host, err),
+		}
+	}
+	if len(addrs) == 0 {
+		return &validateError{
+			code:    2,
+			message: fmt.Sprintf("DNS lookup returned no addresses for %q", host),
+		}
+	}
+
+	for _, a := range addrs {
+		if gitea.IsBlockedIP(a.IP) {
+			return &validateError{
+				code:    1,
+				message: fmt.Sprintf("blocked: %q resolves to private/reserved IP %s", host, a.IP),
+			}
+		}
+	}
+	return nil
+}
@@ -0,0 +1,127 @@
+package main
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+)
+
+func TestRunValidateURL_Usage(t *testing.T) {
+	var errBuf bytes.Buffer
+	origErr := errWriter
+	errWriter = &errBuf
+	defer func() { errWriter = origErr }()
+
+	code := runValidateURL(nil)
+	if code != 2 {
+		t.Errorf("expected exit code 2 for no args, got %d", code)
+	}
+	if !strings.Contains(errBuf.String(), "usage") {
+		t.Errorf("expected usage in stderr, got %q", errBuf.String())
+	}
+
+	errBuf.Reset()
+	code = runValidateURL([]string{"arg1", "arg2"})
+	if code != 2 {
+		t.Errorf("expected exit code 2 for too many args, got %d", code)
+	}
+}
+
+func TestValidateURL_MalformedURL(t *testing.T) {
+	cases := []struct {
+		name    string
+		url     string
+		wantMsg string
+	}{
+		{"empty", "", "must be https"},
+		{"http scheme", "http://example.com/", "must be https"},
+		{"ftp scheme", "ftp://example.com/", "must be https"},
+		{"no scheme", "example.com", "must be https"},
+		{"user info", "https://user:pass@example.com/", "user-info"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := validateURL(tc.url)
+			if err == nil {
+				t.Errorf("expected error for URL %q, got nil", tc.url)
+				return
+			}
+			if !strings.Contains(err.Error(), tc.wantMsg) {
+				t.Errorf("error %q does not contain %q", err.Error(), tc.wantMsg)
+			}
+			var ve *validateError
+			if !isValidateError(err, &ve) {
+				t.Fatalf("expected *validateError, got %T", err)
+			}
+			if ve.code != 2 {
+				t.Errorf("expected code 2, got %d", ve.code)
+			}
+		})
+	}
+}
+
+func TestValidateURL_BlockedPrivateIP(t *testing.T) {
+	// localhost always resolves to 127.0.0.1 (loopback).
+	err := validateURL("https://localhost/")
+	if err == nil {
+		t.Skip("localhost did not resolve (network unavailable in test environment)")
+	}
+	var ve *validateError
+	if !isValidateError(err, &ve) {
+		t.Fatalf("expected *validateError, got %T: %v", err, err)
+	}
+	if ve.code != 1 && ve.code != 2 {
+		t.Errorf("expected code 1 (blocked) or 2 (dns fail), got %d: %s", ve.code, ve.message)
+	}
+	// If it resolved (code 1), the message must say "blocked".
+	if ve.code == 1 && !strings.Contains(ve.message, "blocked") {
+		t.Errorf("expected 'blocked' in message, got %q", ve.message)
+	}
+}
+
+func TestValidateURL_ExitCodes(t *testing.T) {
+	cases := []struct {
+		name     string
+		url      string
+		wantCode int
+	}{
+		{"http scheme", "http://example.com/", 2},
+		{"no scheme", "example.com", 2},
+		{"user info", "https://admin:secret@example.com/", 2},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := validateURL(tc.url)
+			if err == nil {
+				t.Fatalf("expected error for %q", tc.url)
+			}
+			var ve *validateError
+			if !isValidateError(err, &ve) {
+				t.Fatalf("expected *validateError, got %T", err)
+			}
+			if ve.code != tc.wantCode {
+				t.Errorf("code = %d, want %d (url=%q, msg=%s)", ve.code, tc.wantCode, tc.url, ve.message)
+			}
+		})
+	}
+}
+
+func TestRunValidateURL_WithCapture(t *testing.T) {
+	var outBuf, errBuf bytes.Buffer
+	origOut, origErr := outWriter, errWriter
+	outWriter = &outBuf
+	errWriter = &errBuf
+	defer func() {
+		outWriter = origOut
+		errWriter = origErr
+	}()
+
+	// http:// scheme should fail with code 2.
+	code := runValidateURL([]string{"http://example.com/"})
+	if code != 2 {
+		t.Errorf("expected code 2 for http:// URL, got %d", code)
+	}
+	if !strings.Contains(errBuf.String(), "must be https") {
+		t.Errorf("expected error about https in stderr, got %q", errBuf.String())
+	}
+}
@@ -0,0 +1,405 @@
+package main
+
+import (
+	"context"
+
+	"gitea.weiker.me/rodin/review-bot/gitea"
+	"gitea.weiker.me/rodin/review-bot/github"
+	"gitea.weiker.me/rodin/review-bot/vcs"
+)
+
+// vcsClient is the unified interface for VCS operations used by the review flow.
+// Both gitea.Client and github.Client satisfy this interface via their respective adapters.
+type vcsClient interface {
+	// GetPullRequest fetches PR metadata.
+	GetPullRequest(ctx context.Context, owner, repo string, number int) (*pullRequestInfo, error)
+
+	// GetPullRequestDiff fetches the unified diff.
+	GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error)
+
+	// GetPullRequestFiles fetches the list of changed files.
+	GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]changedFileInfo, error)
+
+	// GetCommitStatuses fetches CI statuses for a commit.
+	GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]commitStatusInfo, error)
+
+	// GetFileContent fetches the content of a file at HEAD.
+	GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error)
+
+	// GetFileContentRef fetches the content of a file at a given ref.
+	GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error)
+
+	// ListContents lists the files and directories at a path.
+	ListContents(ctx context.Context, owner, repo, path string) ([]contentEntryInfo, error)
+
+	// ListReviews returns all reviews for a PR.
+	ListReviews(ctx context.Context, owner, repo string, number int) ([]reviewInfo, error)
+
+	// PostReview submits a review.
+	PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*reviewInfo, error)
+
+	// GetAuthenticatedUser returns the login of the authenticated user.
+	GetAuthenticatedUser(ctx context.Context) (string, error)
+}
+
+// giteaExtendedClient is implemented by the Gitea adapter and exposes
+// Gitea-specific operations that have no GitHub equivalent in the current scope.
+// Callers should type-assert to this interface and skip gracefully when it is absent.
+type giteaExtendedClient interface {
+	// RequestReviewer adds the authenticated user as a reviewer on a PR.
+	RequestReviewer(ctx context.Context, owner, repo string, number int, user string) error
+
+	// GetTimelineReviewCommentIDForReview maps a review ID to its timeline comment ID.
+	GetTimelineReviewCommentIDForReview(ctx context.Context, owner, repo string, number int, reviewID int64) (int64, error)
+
+	// EditComment updates the body of an existing PR comment.
+	EditComment(ctx context.Context, owner, repo string, commentID int64, body string) error
+
+	// ListReviewComments lists the inline comments attached to a review.
+	ListReviewComments(ctx context.Context, owner, repo string, number int, reviewID int64) ([]inlineCommentInfo, error)
+
+	// ResolveComment marks an inline comment as resolved.
+	ResolveComment(ctx context.Context, owner, repo string, commentID int64) error
+
+	// ParseDiffNewLines returns the diff line ranges for inline comment positioning.
+	ParseDiffNewLines(diff string) diffLineRanges
+}
+
+// Shared adapter types to avoid duplicating gitea/github-specific types throughout main.go.
+
+type pullRequestInfo struct {
+	Title   string
+	Body    string
+	HeadSha string
+	HeadRef string
+}
+
+type changedFileInfo struct {
+	Filename string
+	Status   string
+}
+
+type commitStatusInfo struct {
+	Status      string
+	Context     string
+	Description string
+	TargetURL   string
+}
+
+type contentEntryInfo struct {
+	Name string
+	Path string
+	Type string
+}
+
+type reviewInfo struct {
+	ID       int64
+	Body     string
+	User     vcs.UserInfo
+	State    string
+	CommitID string
+	Stale    bool
+}
+
+type inlineCommentInfo struct {
+	ID          int64
+	Path        string
+	NewPosition int64
+	Body        string
+}
+
+// diffLineRanges is a type alias for gitea.DiffLineRanges to allow the
+// extended client interface to be defined without importing gitea directly.
+// In practice, only the giteaClientVCSAdapter returns this type, and callers
+// that use it will already have performed the type assertion.
+type diffLineRanges = *gitea.DiffLineRanges
+
+// --- Gitea adapter ---
+
+// giteaClientVCSAdapter wraps gitea.Client to satisfy the vcsClient interface.
+// It also implements giteaExtendedClient for Gitea-specific operations.
+type giteaClientVCSAdapter struct {
+	client *gitea.Client
+}
+
+func newGiteaVCSAdapter(c *gitea.Client) *giteaClientVCSAdapter {
+	return &giteaClientVCSAdapter{client: c}
+}
+
+func (a *giteaClientVCSAdapter) GetPullRequest(ctx context.Context, owner, repo string, number int) (*pullRequestInfo, error) {
+	pr, err := a.client.GetPullRequest(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	return &pullRequestInfo{
+		Title:   pr.Title,
+		Body:    pr.Body,
+		HeadSha: pr.Head.Sha,
+		HeadRef: pr.Head.Ref,
+	}, nil
+}
+
+func (a *giteaClientVCSAdapter) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) {
+	return a.client.GetPullRequestDiff(ctx, owner, repo, number)
+}
+
+func (a *giteaClientVCSAdapter) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]changedFileInfo, error) {
+	files, err := a.client.GetPullRequestFiles(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]changedFileInfo, len(files))
+	for i, f := range files {
+		result[i] = changedFileInfo{Filename: f.Filename, Status: f.Status}
+	}
+	return result, nil
+}
+
+func (a *giteaClientVCSAdapter) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]commitStatusInfo, error) {
+	statuses, err := a.client.GetCommitStatuses(ctx, owner, repo, sha)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]commitStatusInfo, len(statuses))
+	for i, s := range statuses {
+		result[i] = commitStatusInfo{
+			Status:      s.Status,
+			Context:     s.Context,
+			Description: s.Description,
+			TargetURL:   s.TargetURL,
+		}
+	}
+	return result, nil
+}
+
+func (a *giteaClientVCSAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
+	return a.client.GetFileContent(ctx, owner, repo, filepath)
+}
+
+func (a *giteaClientVCSAdapter) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) {
+	return a.client.GetFileContentRef(ctx, owner, repo, filepath, ref)
+}
+
+func (a *giteaClientVCSAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]contentEntryInfo, error) {
+	entries, err := a.client.ListContents(ctx, owner, repo, path)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]contentEntryInfo, len(entries))
+	for i, e := range entries {
+		result[i] = contentEntryInfo{Name: e.Name, Path: e.Path, Type: e.Type}
+	}
+	return result, nil
+}
+
+func (a *giteaClientVCSAdapter) ListReviews(ctx context.Context, owner, repo string, number int) ([]reviewInfo, error) {
+	reviews, err := a.client.ListReviews(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]reviewInfo, len(reviews))
+	for i, r := range reviews {
+		result[i] = reviewInfo{
+			ID:       r.ID,
+			Body:     r.Body,
+			User:     vcs.UserInfo{Login: r.User.Login},
+			State:    r.State,
+			CommitID: r.CommitID,
+			Stale:    r.Stale,
+		}
+	}
+	return result, nil
+}
+
+func (a *giteaClientVCSAdapter) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*reviewInfo, error) {
+	// Translate vcs.ReviewComment to gitea.ReviewComment.
+	// The Gitea API uses NewPosition instead of Position.
+	var comments []gitea.ReviewComment
+	for _, c := range req.Comments {
+		comments = append(comments, gitea.ReviewComment{
+			Path:        c.Path,
+			NewPosition: int64(c.Position),
+			Body:        c.Body,
+		})
+	}
+
+	// Translate vcs.ReviewEvent (GitHub format) to Gitea API event string.
+	// vcs uses "APPROVE"; Gitea API expects "APPROVED".
+	var event string
+	switch req.Event {
+	case vcs.ReviewEventApprove:
+		event = "APPROVED"
+	case vcs.ReviewEventRequestChanges:
+		event = "REQUEST_CHANGES"
+	default:
+		event = "COMMENT"
+	}
+
+	posted, err := a.client.PostReview(ctx, owner, repo, number, event, req.Body, req.CommitID, comments)
+	if err != nil {
+		return nil, err
+	}
+	return &reviewInfo{
+		ID:       posted.ID,
+		Body:     posted.Body,
+		User:     vcs.UserInfo{Login: posted.User.Login},
+		State:    posted.State,
+		CommitID: posted.CommitID,
+	}, nil
+}
+
+func (a *giteaClientVCSAdapter) GetAuthenticatedUser(ctx context.Context) (string, error) {
+	return a.client.GetAuthenticatedUser(ctx)
+}
+
+// giteaExtendedClient implementation:
+
+func (a *giteaClientVCSAdapter) RequestReviewer(ctx context.Context, owner, repo string, number int, user string) error {
+	return a.client.RequestReviewer(ctx, owner, repo, number, user)
+}
+
+func (a *giteaClientVCSAdapter) GetTimelineReviewCommentIDForReview(ctx context.Context, owner, repo string, number int, reviewID int64) (int64, error) {
+	return a.client.GetTimelineReviewCommentIDForReview(ctx, owner, repo, number, reviewID)
+}
+
+func (a *giteaClientVCSAdapter) EditComment(ctx context.Context, owner, repo string, commentID int64, body string) error {
+	return a.client.EditComment(ctx, owner, repo, commentID, body)
+}
+
+func (a *giteaClientVCSAdapter) ListReviewComments(ctx context.Context, owner, repo string, number int, reviewID int64) ([]inlineCommentInfo, error) {
+	comments, err := a.client.ListReviewComments(ctx, owner, repo, number, reviewID)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]inlineCommentInfo, len(comments))
+	for i, c := range comments {
+		result[i] = inlineCommentInfo{
+			ID:          c.ID,
+			Path:        c.Path,
+			NewPosition: c.NewPosition,
+			Body:        c.Body,
+		}
+	}
+	return result, nil
+}
+
+func (a *giteaClientVCSAdapter) ResolveComment(ctx context.Context, owner, repo string, commentID int64) error {
+	return a.client.ResolveComment(ctx, owner, repo, commentID)
+}
+
+func (a *giteaClientVCSAdapter) ParseDiffNewLines(diff string) diffLineRanges {
+	return gitea.ParseDiffNewLines(diff)
+}
+
+// --- GitHub adapter ---
+
+// githubClientVCSAdapter wraps github.Client to satisfy the vcsClient interface.
+type githubClientVCSAdapter struct {
+	client *github.Client
+}
+
+func newGitHubVCSAdapter(c *github.Client) *githubClientVCSAdapter {
+	return &githubClientVCSAdapter{client: c}
+}
+
+func (a *githubClientVCSAdapter) GetPullRequest(ctx context.Context, owner, repo string, number int) (*pullRequestInfo, error) {
+	pr, err := a.client.GetPullRequest(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	return &pullRequestInfo{
+		Title:   pr.Title,
+		Body:    pr.Body,
+		HeadSha: pr.Head.Sha,
+		HeadRef: pr.Head.Ref,
+	}, nil
+}
+
+func (a *githubClientVCSAdapter) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) {
+	return a.client.GetPullRequestDiff(ctx, owner, repo, number)
+}
+
+func (a *githubClientVCSAdapter) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]changedFileInfo, error) {
+	files, err := a.client.GetPullRequestFiles(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]changedFileInfo, len(files))
+	for i, f := range files {
+		result[i] = changedFileInfo{Filename: f.Filename, Status: f.Status}
+	}
+	return result, nil
+}
+
+func (a *githubClientVCSAdapter) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]commitStatusInfo, error) {
+	statuses, err := a.client.GetCommitStatuses(ctx, owner, repo, sha)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]commitStatusInfo, len(statuses))
+	for i, s := range statuses {
+		result[i] = commitStatusInfo{
+			Status:      s.Status,
+			Context:     s.Context,
+			Description: s.Description,
+			TargetURL:   s.TargetURL,
+		}
+	}
+	return result, nil
+}
+
+func (a *githubClientVCSAdapter) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
+	return a.client.GetFileContent(ctx, owner, repo, filepath)
+}
+
+func (a *githubClientVCSAdapter) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) {
+	return a.client.GetFileContentRef(ctx, owner, repo, filepath, ref)
+}
+
+func (a *githubClientVCSAdapter) ListContents(ctx context.Context, owner, repo, path string) ([]contentEntryInfo, error) {
+	entries, err := a.client.ListContents(ctx, owner, repo, path)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]contentEntryInfo, len(entries))
+	for i, e := range entries {
+		result[i] = contentEntryInfo{Name: e.Name, Path: e.Path, Type: e.Type}
+	}
+	return result, nil
+}
+
+func (a *githubClientVCSAdapter) ListReviews(ctx context.Context, owner, repo string, number int) ([]reviewInfo, error) {
+	reviews, err := a.client.ListReviews(ctx, owner, repo, number)
+	if err != nil {
+		return nil, err
+	}
+	result := make([]reviewInfo, len(reviews))
+	for i, r := range reviews {
+		result[i] = reviewInfo{
+			ID:       r.ID,
+			Body:     r.Body,
+			User:     r.User,
+			State:    r.State,
+			CommitID: r.CommitID,
+		}
+	}
+	return result, nil
+}
+
+func (a *githubClientVCSAdapter) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*reviewInfo, error) {
+	posted, err := a.client.PostReview(ctx, owner, repo, number, req)
+	if err != nil {
+		return nil, err
+	}
+	return &reviewInfo{
+		ID:       posted.ID,
+		Body:     posted.Body,
+		User:     posted.User,
+		State:    posted.State,
+		CommitID: posted.CommitID,
+	}, nil
+}
+
+func (a *githubClientVCSAdapter) GetAuthenticatedUser(ctx context.Context) (string, error) {
+	return a.client.GetAuthenticatedUser(ctx)
+}
@@ -106,34 +106,113 @@ func defaultCheckRedirect(req *http.Request, via []*http.Request) error {
 	return nil
 }

+// safeDialContext is the default DialContext for NewClient.
+// It resolves the hostname and checks every returned IP against the blocked
+// CIDR list before establishing a connection. This prevents SSRF attacks
+// where user-supplied URLs resolve to internal/private addresses.
+//
+// After validating all IPs, we dial the first resolved IP directly to avoid
+// a second DNS lookup (which could return a different IP in a DNS rebinding
+// attack). This narrows — but does not fully eliminate — the DNS rebinding
+// window to the time between LookupIPAddr and DialContext.
+//
+// If the host is already an IP literal, LookupIPAddr returns it directly
+// (no DNS query issued), so IP literals like https://127.0.0.1/ are blocked.
+func safeDialContext(ctx context.Context, network, addr string) (net.Conn, error) {
+	host, port, err := net.SplitHostPort(addr)
+	if err != nil {
+		return nil, fmt.Errorf("safeDialContext: invalid address %q: %w", addr, err)
+	}
+	addrs, err := net.DefaultResolver.LookupIPAddr(ctx, host)
+	if err != nil {
+		return nil, fmt.Errorf("safeDialContext: DNS lookup %q: %w", host, err)
+	}
+	if len(addrs) == 0 {
+		return nil, fmt.Errorf("safeDialContext: no addresses returned for %q", host)
+	}
+	for _, a := range addrs {
+		if IsBlockedIP(a.IP) {
+			return nil, fmt.Errorf("safeDialContext: blocked: %q resolves to private/reserved IP %s", host, a.IP)
+		}
+	}
+	// Try each resolved IP in order, returning the first successful connection.
+	// Fallback is important when a hostname resolves to multiple IPs and the first
+	// is temporarily unreachable. All IPs were already validated above, so dialing
+	// any of them is safe.
+	//
+	// Timeout: 10s per the design (PLAN.md); the outer http.Client has a 30s
+	// total timeout, but the per-dial timeout ensures a slow TCP connect on one IP
+	// doesn't consume the budget needed to try others.
+	d := &net.Dialer{Timeout: 10 * time.Second}
+	var lastErr error
+	for _, a := range addrs {
+		conn, err := d.DialContext(ctx, network, net.JoinHostPort(a.IP.String(), port))
+		if err == nil {
+			return conn, nil
+		}
+		lastErr = err
+	}
+	return nil, fmt.Errorf("safeDialContext: all %d addresses for %q failed, last error: %w", len(addrs), host, lastErr)
+}
+
+// newSafeHTTPClient returns an *http.Client with the SSRF-blocking safeDialContext
+// transport and the cross-host redirect rejection policy.
+//
+// We clone http.DefaultTransport to preserve its production-ready defaults
+// (ProxyFromEnvironment, TLSHandshakeTimeout, IdleConnTimeout, connection
+// pooling, HTTP/2 support) and override only DialContext with safeDialContext.
+func newSafeHTTPClient() *http.Client {
+	transport := http.DefaultTransport.(*http.Transport).Clone()
+	transport.DialContext = safeDialContext
+	return &http.Client{
+		Timeout:       30 * time.Second,
+		Transport:     transport,
+		CheckRedirect: defaultCheckRedirect,
+	}
+}
+
 // NewClient creates a new Gitea API client.
+//
+// The client uses a safe HTTP transport by default: DNS resolution is performed
+// before connecting and any IP in a private/reserved range is rejected
+// (RFC1918, loopback, link-local, ULA, etc.). Cross-host and HTTPS→HTTP
+// redirects are also rejected.
+//
+// For tests that use httptest.NewServer (which listens on 127.0.0.1), call
+// WithUnsafeDialer() to bypass the IP check.
 func NewClient(baseURL, token string) *Client {
 	return &Client{
 		baseURL: strings.TrimRight(baseURL, "/"),
 		token:   token,
-		http: &http.Client{
-			Timeout:       30 * time.Second,
-			CheckRedirect: defaultCheckRedirect,
-		},
+		http:    newSafeHTTPClient(),
 	}
 }

+// WithUnsafeDialer returns the client configured with a plain HTTP client that
+// has no IP-level SSRF protection. It preserves the redirect-rejection policy.
+//
+// This MUST only be used in tests. Production code must never call this method.
+func (c *Client) WithUnsafeDialer() *Client {
+	c.http = &http.Client{
+		Timeout:       30 * time.Second,
+		CheckRedirect: defaultCheckRedirect,
+	}
+	return c
+}
+
 // SetHTTPClient sets the underlying HTTP client used for requests.
 // This is intended for test setup only to inject mock transports; it must be
 // called before any goroutines issue requests.
 //
-// Passing nil restores the default client (30s timeout + redirect-rejecting
-// CheckRedirect policy matching NewClient).
+// Passing nil restores the default safe client (30s timeout, IP-blocking
+// safeDialContext, and redirect-rejecting CheckRedirect policy matching NewClient).
 //
 // Callers providing a non-nil client are responsible for configuring a safe
 // CheckRedirect policy. Without one, the default net/http behavior will follow
 // redirects and may forward the Authorization header to untrusted hosts.
 func (c *Client) SetHTTPClient(hc *http.Client) {
 	if hc == nil {
-		hc = &http.Client{
-			Timeout:       30 * time.Second,
-			CheckRedirect: defaultCheckRedirect,
-		}
+		hc = newSafeHTTPClient()
 	}
 	c.http = hc
 }
@@ -36,7 +36,7 @@ func TestGetPullRequest(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	got, err := client.GetPullRequest(context.Background(), "owner", "repo", 1)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -63,7 +63,7 @@ func TestGetPullRequestDiff(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	got, err := client.GetPullRequestDiff(context.Background(), "owner", "repo", 5)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -88,7 +88,7 @@ func TestGetCommitStatuses(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	got, err := client.GetCommitStatuses(context.Background(), "owner", "repo", "abc123")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -138,7 +138,7 @@ func TestPostReview(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	review, err := client.PostReview(context.Background(), "owner", "repo", 3, "APPROVED", "LGTM", "abc123def", nil)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -158,7 +158,7 @@ func TestGetPullRequest_Non200(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	_, err := client.GetPullRequest(context.Background(), "owner", "repo", 999)
 	if err == nil {
 		t.Fatal("expected error for 404, got nil")
@@ -171,7 +171,7 @@ func TestGetPullRequest_BadJSON(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	_, err := client.GetPullRequest(context.Background(), "owner", "repo", 1)
 	if err == nil {
 		t.Fatal("expected error for bad JSON, got nil")
@@ -185,7 +185,7 @@ func TestPostReview_Non200(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "test", "", nil)
 	if err == nil {
 		t.Fatal("expected error for 403, got nil")
@@ -208,7 +208,7 @@ func TestPostReview_EmptyCommitID_OmittedFromPayload(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "ok", "", nil)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -226,7 +226,7 @@ func TestGetFileContent(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	got, err := client.GetFileContent(context.Background(), "owner", "repo", "CONVENTIONS.md")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -246,7 +246,7 @@ func TestGetPullRequestFiles(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	files, err := client.GetPullRequestFiles(context.Background(), "owner", "repo", 1)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -271,7 +271,7 @@ func TestGetFileContentRef(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	content, err := client.GetFileContentRef(context.Background(), "owner", "repo", "main.go", "feature-branch")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -291,7 +291,7 @@ func TestListContents(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	entries, err := client.ListContents(context.Background(), "owner", "repo", "docs")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -318,7 +318,7 @@ func TestListContents_DotPath(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	entries, err := client.ListContents(context.Background(), "owner", "repo", ".")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -343,7 +343,7 @@ func TestListContents_FilePath(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	entries, err := client.ListContents(context.Background(), "owner", "repo", "README.md")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -375,7 +375,7 @@ func TestGetAllFilesInPath_File(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	files, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "README.md")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -428,7 +428,7 @@ func TestListReviews(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	reviews, err := client.ListReviews(context.Background(), "owner", "repo", 5)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -468,7 +468,7 @@ func TestListReviews_Pagination(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	reviews, err := client.ListReviews(context.Background(), "owner", "repo", 5)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -493,7 +493,7 @@ func TestDeleteReview(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	err := client.DeleteReview(context.Background(), "owner", "repo", 5, 10)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -507,7 +507,7 @@ func TestDeleteReview_Forbidden(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	err := client.DeleteReview(context.Background(), "owner", "repo", 5, 10)
 	if err == nil {
 		t.Fatal("expected error for 403, got nil")
@@ -536,7 +536,7 @@ func TestEditComment(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	err := client.EditComment(context.Background(), "owner", "repo", 42, "updated body")
 	if err != nil {
 		t.Fatalf("EditComment() error = %v", err)
@@ -550,7 +550,7 @@ func TestEditComment_Forbidden(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	err := client.EditComment(context.Background(), "owner", "repo", 42, "new body")
 	if err == nil {
 		t.Fatal("expected error for 403 response")
@@ -570,7 +570,7 @@ func TestGetTimelineReviewCommentID(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	id, err := client.GetTimelineReviewCommentID(context.Background(), "owner", "repo", 5, "<!-- review-bot:sonnet -->")
 	if err != nil {
 		t.Fatalf("GetTimelineReviewCommentID() error = %v", err)
@@ -586,7 +586,7 @@ func TestGetTimelineReviewCommentID_NotFound(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	_, err := client.GetTimelineReviewCommentID(context.Background(), "owner", "repo", 5, "<!-- review-bot:sonnet -->")
 	if err == nil {
 		t.Fatal("expected error when sentinel not found")
@@ -609,7 +609,7 @@ func TestGetAllFilesInPath_404FallsBackToFile(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	files, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "README.md")
 	if err != nil {
 		t.Fatalf("expected fallback to file on 404, got error: %v", err)
@@ -630,7 +630,7 @@ func TestGetAllFilesInPath_500Propagates(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	_, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "somepath")
 	if err == nil {
 		t.Fatal("expected error to propagate for 500, got nil")
@@ -652,7 +652,7 @@ func TestGetAllFilesInPath_403Propagates(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	_, err := client.GetAllFilesInPath(context.Background(), "owner", "repo", "private/stuff")
 	if err == nil {
 		t.Fatal("expected error to propagate for 403, got nil")
@@ -704,7 +704,7 @@ func TestGetAuthenticatedUser(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	login, err := client.GetAuthenticatedUser(context.Background())
 	if err != nil {
 		t.Fatalf("GetAuthenticatedUser() error = %v", err)
@@ -729,7 +729,7 @@ func TestRequestReviewer(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	err := client.RequestReviewer(context.Background(), "owner", "repo", 7, "bot-user")
 	if err != nil {
 		t.Fatalf("RequestReviewer() error = %v", err)
@@ -745,7 +745,7 @@ func TestRequestReviewer_204(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	err := client.RequestReviewer(context.Background(), "owner", "repo", 1, "user")
 	if err != nil {
 		t.Fatalf("RequestReviewer() should accept 204, got error = %v", err)
@@ -759,7 +759,7 @@ func TestRequestReviewer_Error(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	err := client.RequestReviewer(context.Background(), "owner", "repo", 1, "user")
 	if err == nil {
 		t.Fatal("expected error for 403 response")
@@ -779,7 +779,7 @@ func TestListReviewComments(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	comments, err := client.ListReviewComments(context.Background(), "owner", "repo", 1, 42)
 	if err != nil {
 		t.Fatalf("ListReviewComments() error = %v", err)
@@ -807,7 +807,7 @@ func TestResolveComment(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	err := client.ResolveComment(context.Background(), "owner", "repo", 99)
 	if err != nil {
 		t.Fatalf("ResolveComment() error = %v", err)
@@ -821,7 +821,7 @@ func TestResolveComment_Error(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	err := client.ResolveComment(context.Background(), "owner", "repo", 99)
 	if err == nil {
 		t.Fatal("expected error for 404 response")
@@ -870,7 +870,7 @@ func TestDoGet_RetriesOn500(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	// Use short backoff for fast tests
 	client.RetryBackoff = []time.Duration{1 * time.Millisecond, 1 * time.Millisecond}

@@ -895,7 +895,7 @@ func TestDoGet_FailsAfterMaxRetries(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	// Use short backoff for fast tests
 	client.RetryBackoff = []time.Duration{1 * time.Millisecond, 1 * time.Millisecond}

@@ -924,7 +924,7 @@ func TestDoGet_NoRetryOn4xx(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	_, err := client.doGet(context.Background(), server.URL+"/test")
 	if err == nil {
 		t.Fatal("expected error for 403")
@@ -952,7 +952,7 @@ func TestDoGet_RespectsContextCancellation(t *testing.T) {

 	ctx, cancel := context.WithCancel(context.Background())

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	// Use longer backoff to give us time to cancel during the wait
 	client.RetryBackoff = []time.Duration{100 * time.Millisecond, 100 * time.Millisecond}

@@ -1285,3 +1285,137 @@ func TestSetHTTPClient_NilRestoresDefault(t *testing.T) {
 		t.Fatal("expected CheckRedirect policy after SetHTTPClient(nil)")
 	}
 }
+
+// TestSafeDialContextBlocksPrivateIPs verifies that NewClient (which uses
+// safeDialContext by default) refuses to connect to private/reserved IPs.
+func TestSafeDialContextBlocksPrivateIPs(t *testing.T) {
+	// These servers listen on 127.0.0.1, so the safe dialer will block them.
+	// We use NewClient (NOT NewTestClient) to exercise the real safe dialer.
+	privateURLs := []struct {
+		name string
+		url  string
+	}{
+		{"loopback localhost", "http://localhost/"},
+		{"loopback 127.0.0.1", "http://127.0.0.1/"},
+	}
+
+	for _, tc := range privateURLs {
+		t.Run(tc.name, func(t *testing.T) {
+			c := NewClient(tc.url, "token")
+			_, err := c.GetPullRequest(context.Background(), "owner", "repo", 1)
+			if err == nil {
+				t.Errorf("expected error connecting to %s, got nil", tc.url)
+			}
+			// Error must mention SSRF/blocked, not a random network error.
+			if !strings.Contains(err.Error(), "blocked") &&
+				!strings.Contains(err.Error(), "private") &&
+				!strings.Contains(err.Error(), "loopback") &&
+				!strings.Contains(err.Error(), "reserved") {
+				t.Logf("error: %v", err)
+				// Allow other errors (connection refused, DNS) since the point
+				// is that we don't silently succeed — but prefer the explicit block message.
+			}
+		})
+	}
+}
+
+// TestWithUnsafeDialerAllowsLocalhost verifies that WithUnsafeDialer bypasses
+// the IP check, allowing tests to connect to httptest.Server (127.0.0.1).
+func TestWithUnsafeDialerAllowsLocalhost(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.Write([]byte(`{"title":"test","body":"","head":{"sha":"abc","ref":"main"}}`))
+	}))
+	defer server.Close()
+
+	// WithUnsafeDialer should allow connecting to 127.0.0.1.
+	c := NewClient(server.URL, "token").WithUnsafeDialer()
+	pr, err := c.GetPullRequest(context.Background(), "owner", "repo", 1)
+	if err != nil {
+		t.Fatalf("unexpected error with unsafe dialer: %v", err)
+	}
+	if pr.Title != "test" {
+		t.Errorf("expected title 'test', got %q", pr.Title)
+	}
+}
+
+// TestNewClient_HasSafeTransport verifies that NewClient installs the
+// SSRF-blocking transport (i.e. Transport is not nil and DialContext is set).
+func TestNewClient_HasSafeTransport(t *testing.T) {
+	c := NewClient("https://gitea.example.com", "token")
+	if c.http.Transport == nil {
+		t.Fatal("expected Transport to be set on NewClient (safe dialer)")
+	}
+	transport, ok := c.http.Transport.(*http.Transport)
+	if !ok {
+		t.Fatalf("expected *http.Transport, got %T", c.http.Transport)
+	}
+	if transport.DialContext == nil {
+		t.Fatal("expected DialContext to be set on transport (safe dialer)")
+	}
+}
+
+// TestSetHTTPClient_NilRestoresSafeTransport verifies that SetHTTPClient(nil)
+// restores the safe transport (not just any client).
+func TestSetHTTPClient_NilRestoresSafeTransport(t *testing.T) {
+	c := NewClient("https://gitea.example.com", "token")
+	c.SetHTTPClient(&http.Client{}) // replace with plain client
+	c.SetHTTPClient(nil)            // restore
+	transport, ok := c.http.Transport.(*http.Transport)
+	if !ok {
+		t.Fatalf("expected *http.Transport after SetHTTPClient(nil), got %T", c.http.Transport)
+	}
+	if transport.DialContext == nil {
+		t.Fatal("expected DialContext to be restored after SetHTTPClient(nil)")
+	}
+}
+
+// TestNewSafeHTTPClient_PreservesDefaultTransportSettings verifies that
+// newSafeHTTPClient clones http.DefaultTransport to retain proxy support,
+// TLS handshake timeout, idle connection limits, and HTTP/2.
+func TestNewSafeHTTPClient_PreservesDefaultTransportSettings(t *testing.T) {
+	c := NewClient("https://gitea.example.com", "token")
+	transport, ok := c.http.Transport.(*http.Transport)
+	if !ok {
+		t.Fatalf("expected *http.Transport, got %T", c.http.Transport)
+	}
+
+	defaults := http.DefaultTransport.(*http.Transport)
+
+	// TLSHandshakeTimeout must be inherited (non-zero), not the zero value
+	// that a bare &http.Transport{} would have.
+	if transport.TLSHandshakeTimeout == 0 {
+		t.Error("TLSHandshakeTimeout is 0; expected inherited value from DefaultTransport")
+	}
+	if transport.TLSHandshakeTimeout != defaults.TLSHandshakeTimeout {
+		t.Errorf("TLSHandshakeTimeout = %v, want %v", transport.TLSHandshakeTimeout, defaults.TLSHandshakeTimeout)
+	}
+
+	// IdleConnTimeout must be inherited.
+	if transport.IdleConnTimeout == 0 {
+		t.Error("IdleConnTimeout is 0; expected inherited value from DefaultTransport")
+	}
+	if transport.IdleConnTimeout != defaults.IdleConnTimeout {
+		t.Errorf("IdleConnTimeout = %v, want %v", transport.IdleConnTimeout, defaults.IdleConnTimeout)
+	}
+
+	// MaxIdleConns must be inherited.
+	if transport.MaxIdleConns == 0 {
+		t.Error("MaxIdleConns is 0; expected inherited value from DefaultTransport")
+	}
+
+	// ForceAttemptHTTP2 must be inherited.
+	if !transport.ForceAttemptHTTP2 {
+		t.Error("ForceAttemptHTTP2 is false; expected true from DefaultTransport")
+	}
+
+	// Proxy must be set (ProxyFromEnvironment).
+	if transport.Proxy == nil {
+		t.Error("Proxy is nil; expected ProxyFromEnvironment from DefaultTransport")
+	}
+
+	// DialContext must be our safe dialer, not the default.
+	if transport.DialContext == nil {
+		t.Error("DialContext is nil; expected safeDialContext")
+	}
+}
@@ -70,7 +70,7 @@ func TestGetPullRequestDiff_SizeLimits(t *testing.T) {
 			}))
 			defer server.Close()

-			client := NewClient(server.URL, "test-token")
+			client := NewTestClient(server.URL, "test-token")
 			client.MaxDiffSize = tt.maxDiffSize
 			client.RetryBackoff = []time.Duration{}

@@ -0,0 +1,18 @@
+// Package gitea — export_test.go exposes test helpers to test files in this
+// package. It uses `package gitea` (not `package gitea_test`) so it can access
+// unexported identifiers; Go only compiles it into the test binary, never into
+// the production binary. This is the idiomatic pattern for white-box testing
+// in Go (see net/http/export_test.go in the stdlib for the same approach).
+package gitea
+
+// NewTestClient creates a Gitea client configured for use in unit tests.
+// It bypasses the IP-level SSRF protection so that tests can connect to
+// httptest.Server instances (which listen on 127.0.0.1).
+//
+// Using the internal package gitea declaration (not gitea_test) means this
+// symbol is available to all _test.go files in this package. It is ONLY
+// compiled into the test binary; production binaries never include it.
+// Production code must use NewClient, which enables the safe dialer.
+func NewTestClient(baseURL, token string) *Client {
+	return NewClient(baseURL, token).WithUnsafeDialer()
+}
@@ -0,0 +1,91 @@
+// Package gitea provides a client for the Gitea API.
+// ipcheck.go implements IP-level SSRF protection by checking resolved addresses
+// against known blocked CIDR ranges (RFC1918, loopback, link-local, etc.).
+package gitea
+
+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)
+	}
+}
+
+// IsBlockedIP reports whether ip is in a blocked address range.
+// It is exported for use by 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
+}
@@ -0,0 +1,144 @@
+package gitea
+
+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) {
+	if len(blockedCIDRParseErrors) > 0 {
+		for _, msg := range blockedCIDRParseErrors {
+			t.Errorf("CIDR parse error: %s", msg)
+		}
+	}
+}
@@ -31,7 +31,7 @@ func TestPostReview_WithComments(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	comments := []ReviewComment{
 		{Path: "main.go", NewPosition: 42, Body: "[MAJOR] Something bad"},
 		{Path: "util.go", NewPosition: 10, Body: "[MINOR] Style issue"},
@@ -71,7 +71,7 @@ func TestPostReview_NilComments(t *testing.T) {
 	}))
 	defer server.Close()

-	client := NewClient(server.URL, "test-token")
+	client := NewTestClient(server.URL, "test-token")
 	_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "all good", "", nil)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
@@ -0,0 +1,582 @@
+package github
+
+import (
+	"context"
+	"encoding/base64"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"gitea.weiker.me/rodin/review-bot/vcs"
+)
+
+func TestGetPullRequest(t *testing.T) {
+	want := PullRequest{
+		Title: "My PR",
+		Body:  "Description",
+	}
+	want.Head.Sha = "abc123"
+	want.Head.Ref = "feature/foo"
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/repos/owner/repo/pulls/42" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(want)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	pr, err := c.GetPullRequest(context.Background(), "owner", "repo", 42)
+	if err != nil {
+		t.Fatalf("GetPullRequest: %v", err)
+	}
+	if pr.Title != want.Title {
+		t.Errorf("Title = %q, want %q", pr.Title, want.Title)
+	}
+	if pr.Head.Sha != want.Head.Sha {
+		t.Errorf("Head.Sha = %q, want %q", pr.Head.Sha, want.Head.Sha)
+	}
+	if pr.Head.Ref != want.Head.Ref {
+		t.Errorf("Head.Ref = %q, want %q", pr.Head.Ref, want.Head.Ref)
+	}
+}
+
+func TestGetPullRequest_NotFound(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+		w.Write([]byte("not found"))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	_, err := c.GetPullRequest(context.Background(), "owner", "repo", 1)
+	if err == nil {
+		t.Fatal("expected error, got nil")
+	}
+	if !IsNotFound(err) {
+		t.Errorf("expected IsNotFound, got %v", err)
+	}
+}
+
+func TestGetPullRequestDiff(t *testing.T) {
+	const wantDiff = "diff --git a/foo.go b/foo.go\n+added line\n"
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get("Accept") != "application/vnd.github.diff" {
+			t.Errorf("Accept = %q, want application/vnd.github.diff", r.Header.Get("Accept"))
+		}
+		w.Header().Set("Content-Type", "text/plain")
+		fmt.Fprint(w, wantDiff)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	diff, err := c.GetPullRequestDiff(context.Background(), "owner", "repo", 1)
+	if err != nil {
+		t.Fatalf("GetPullRequestDiff: %v", err)
+	}
+	if diff != wantDiff {
+		t.Errorf("diff = %q, want %q", diff, wantDiff)
+	}
+}
+
+func TestGetPullRequestDiff_TooLarge(t *testing.T) {
+	// Return a diff larger than DefaultMaxDiffSize.
+	hugeDiff := make([]byte, DefaultMaxDiffSize+1)
+	for i := range hugeDiff {
+		hugeDiff[i] = 'x'
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "text/plain")
+		w.Write(hugeDiff)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	_, err := c.GetPullRequestDiff(context.Background(), "owner", "repo", 1)
+	if err == nil {
+		t.Fatal("expected ErrDiffTooLarge, got nil")
+	}
+	if err != ErrDiffTooLarge {
+		t.Errorf("err = %v, want ErrDiffTooLarge", err)
+	}
+}
+
+func TestGetPullRequestFiles(t *testing.T) {
+	files := []ChangedFile{
+		{Filename: "foo.go", Status: "modified"},
+		{Filename: "bar.go", Status: "added"},
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/repos/owner/repo/pulls/7/files" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(files)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.GetPullRequestFiles(context.Background(), "owner", "repo", 7)
+	if err != nil {
+		t.Fatalf("GetPullRequestFiles: %v", err)
+	}
+	if len(got) != len(files) {
+		t.Fatalf("len = %d, want %d", len(got), len(files))
+	}
+	for i, f := range files {
+		if got[i].Filename != f.Filename || got[i].Status != f.Status {
+			t.Errorf("file[%d] = %+v, want %+v", i, got[i], f)
+		}
+	}
+}
+
+func TestGetCommitStatuses(t *testing.T) {
+	statuses := []CommitStatus{
+		{Status: "success", Context: "ci/tests", Description: "All tests passed", TargetURL: "https://ci.example.com"},
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/repos/owner/repo/commits/deadbeef/statuses" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(statuses)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.GetCommitStatuses(context.Background(), "owner", "repo", "deadbeef")
+	if err != nil {
+		t.Fatalf("GetCommitStatuses: %v", err)
+	}
+	if len(got) != 1 {
+		t.Fatalf("len = %d, want 1", len(got))
+	}
+	if got[0].Status != "success" || got[0].Context != "ci/tests" {
+		t.Errorf("status = %+v, want %+v", got[0], statuses[0])
+	}
+}
+
+func TestGetFileContent(t *testing.T) {
+	const wantContent = "package main\n\nfunc main() {}\n"
+	encoded := base64.StdEncoding.EncodeToString([]byte(wantContent))
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/repos/owner/repo/contents/main.go" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(contentResponse{
+			Type:     "file",
+			Name:     "main.go",
+			Path:     "main.go",
+			Encoding: "base64",
+			Content:  encoded,
+		})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.GetFileContent(context.Background(), "owner", "repo", "main.go")
+	if err != nil {
+		t.Fatalf("GetFileContent: %v", err)
+	}
+	if got != wantContent {
+		t.Errorf("content = %q, want %q", got, wantContent)
+	}
+}
+
+func TestGetFileContentRef(t *testing.T) {
+	const wantContent = "version: 2\n"
+	encoded := base64.StdEncoding.EncodeToString([]byte(wantContent))
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/repos/owner/repo/contents/config.yml" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		if r.URL.Query().Get("ref") != "feature/x" {
+			t.Errorf("ref = %q, want feature/x", r.URL.Query().Get("ref"))
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(contentResponse{
+			Type:     "file",
+			Name:     "config.yml",
+			Path:     "config.yml",
+			Encoding: "base64",
+			Content:  encoded,
+		})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.GetFileContentRef(context.Background(), "owner", "repo", "config.yml", "feature/x")
+	if err != nil {
+		t.Fatalf("GetFileContentRef: %v", err)
+	}
+	if got != wantContent {
+		t.Errorf("content = %q, want %q", got, wantContent)
+	}
+}
+
+func TestGetFileContent_NewlinesInBase64(t *testing.T) {
+	// GitHub inserts newlines every 60 chars in the base64-encoded content.
+	// Verify we strip them before decoding.
+	const wantContent = "hello world this is a long string that gets split by github with newlines in the base64 encoding"
+	rawEncoded := base64.StdEncoding.EncodeToString([]byte(wantContent))
+	// Insert newlines to simulate GitHub's format.
+	var chunked string
+	for i := 0; i < len(rawEncoded); i += 60 {
+		end := i + 60
+		if end > len(rawEncoded) {
+			end = len(rawEncoded)
+		}
+		chunked += rawEncoded[i:end] + "\n"
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(contentResponse{
+			Type:     "file",
+			Encoding: "base64",
+			Content:  chunked,
+		})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.GetFileContent(context.Background(), "owner", "repo", "file.txt")
+	if err != nil {
+		t.Fatalf("GetFileContent: %v", err)
+	}
+	if got != wantContent {
+		t.Errorf("content = %q, want %q", got, wantContent)
+	}
+}
+
+func TestListContents_Directory(t *testing.T) {
+	entries := []ContentEntry{
+		{Name: "main.go", Path: "main.go", Type: "file"},
+		{Name: "lib", Path: "lib", Type: "dir"},
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/repos/owner/repo/contents/" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(entries)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.ListContents(context.Background(), "owner", "repo", "")
+	if err != nil {
+		t.Fatalf("ListContents: %v", err)
+	}
+	if len(got) != len(entries) {
+		t.Fatalf("len = %d, want %d", len(got), len(entries))
+	}
+	for i, e := range entries {
+		if got[i].Name != e.Name || got[i].Type != e.Type {
+			t.Errorf("entry[%d] = %+v, want %+v", i, got[i], e)
+		}
+	}
+}
+
+func TestListContents_File(t *testing.T) {
+	// When path points to a single file, GitHub returns an object, not array.
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		// Return a single file object (not an array).
+		json.NewEncoder(w).Encode(contentResponse{
+			Type:     "file",
+			Name:     "README.md",
+			Path:     "README.md",
+			Encoding: "base64",
+			Content:  base64.StdEncoding.EncodeToString([]byte("# Hello")),
+		})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.ListContents(context.Background(), "owner", "repo", "README.md")
+	if err != nil {
+		t.Fatalf("ListContents (file): %v", err)
+	}
+	if len(got) != 1 {
+		t.Fatalf("len = %d, want 1", len(got))
+	}
+	if got[0].Name != "README.md" || got[0].Type != "file" {
+		t.Errorf("entry = %+v", got[0])
+	}
+}
+
+func TestGetAuthenticatedUser(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/user" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(userResponse{Login: "rodin"})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	login, err := c.GetAuthenticatedUser(context.Background())
+	if err != nil {
+		t.Fatalf("GetAuthenticatedUser: %v", err)
+	}
+	if login != "rodin" {
+		t.Errorf("login = %q, want %q", login, "rodin")
+	}
+}
+
+func TestPostReview(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPost {
+			t.Errorf("method = %s, want POST", r.Method)
+		}
+		if r.URL.Path != "/repos/owner/repo/pulls/5/reviews" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		var payload postReviewRequest
+		if err := json.NewDecoder(r.Body).Decode(&payload); err != nil {
+			t.Errorf("decode body: %v", err)
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+		if payload.Event != "REQUEST_CHANGES" {
+			t.Errorf("event = %q, want REQUEST_CHANGES", payload.Event)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(reviewResponse{
+			ID:    99,
+			Body:  payload.Body,
+			State: "CHANGES_REQUESTED",
+			User:  struct{ Login string `json:"login"` }{Login: "rodin"},
+		})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	review, err := c.PostReview(context.Background(), "owner", "repo", 5, vcs.ReviewRequest{
+		Body:  "needs work",
+		Event: vcs.ReviewEventRequestChanges,
+	})
+	if err != nil {
+		t.Fatalf("PostReview: %v", err)
+	}
+	if review.ID != 99 {
+		t.Errorf("review.ID = %d, want 99", review.ID)
+	}
+	// Verify state translation: CHANGES_REQUESTED -> REQUEST_CHANGES
+	if review.State != "REQUEST_CHANGES" {
+		t.Errorf("review.State = %q, want REQUEST_CHANGES", review.State)
+	}
+	if review.User.Login != "rodin" {
+		t.Errorf("review.User.Login = %q, want rodin", review.User.Login)
+	}
+}
+
+func TestPostReview_ConflictingCommitIDs(t *testing.T) {
+	c := NewClient("tok", "https://api.github.com")
+	_, err := c.PostReview(context.Background(), "owner", "repo", 1, vcs.ReviewRequest{
+		Body:  "test",
+		Event: vcs.ReviewEventComment,
+		Comments: []vcs.ReviewComment{
+			{Path: "a.go", Position: 1, Body: "comment 1", CommitID: "sha1"},
+			{Path: "b.go", Position: 2, Body: "comment 2", CommitID: "sha2"},
+		},
+	})
+	if err == nil {
+		t.Fatal("expected ErrConflictingCommitIDs, got nil")
+	}
+	if err != ErrConflictingCommitIDs {
+		t.Errorf("err = %v, want ErrConflictingCommitIDs", err)
+	}
+}
+
+func TestListReviews(t *testing.T) {
+	reviews := []reviewResponse{
+		{ID: 1, Body: "lgtm", State: "APPROVED", User: struct{ Login string `json:"login"` }{Login: "alice"}},
+		{ID: 2, Body: "needs work", State: "CHANGES_REQUESTED", User: struct{ Login string `json:"login"` }{Login: "bob"}},
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/repos/owner/repo/pulls/3/reviews" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(reviews)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	got, err := c.ListReviews(context.Background(), "owner", "repo", 3)
+	if err != nil {
+		t.Fatalf("ListReviews: %v", err)
+	}
+	if len(got) != 2 {
+		t.Fatalf("len = %d, want 2", len(got))
+	}
+	if got[0].State != "APPROVED" {
+		t.Errorf("got[0].State = %q, want APPROVED", got[0].State)
+	}
+	if got[1].State != "REQUEST_CHANGES" {
+		t.Errorf("got[1].State = %q, want REQUEST_CHANGES (translated from CHANGES_REQUESTED)", got[1].State)
+	}
+}
+
+func TestDeleteReview_Pending(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodDelete {
+			t.Errorf("method = %s, want DELETE", r.Method)
+		}
+		if r.URL.Path != "/repos/owner/repo/pulls/1/reviews/42" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.WriteHeader(http.StatusNoContent)
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	err := c.DeleteReview(context.Background(), "owner", "repo", 1, 42)
+	if err != nil {
+		t.Fatalf("DeleteReview: %v", err)
+	}
+}
+
+func TestDeleteReview_Submitted_Returns422(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// GitHub returns 422 when trying to delete a submitted review.
+		w.WriteHeader(http.StatusUnprocessableEntity)
+		w.Write([]byte(`{"message":"Cannot delete a submitted review"}`))
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	err := c.DeleteReview(context.Background(), "owner", "repo", 1, 42)
+	if err == nil {
+		t.Fatal("expected error for submitted review deletion")
+	}
+	// Should be wrapped as ErrCannotDeleteSubmittedReview.
+	if !isErrCannotDeleteSubmittedReview(err) {
+		t.Errorf("err = %v, want ErrCannotDeleteSubmittedReview", err)
+	}
+}
+
+// isErrCannotDeleteSubmittedReview checks if err wraps ErrCannotDeleteSubmittedReview.
+func isErrCannotDeleteSubmittedReview(err error) bool {
+	return err != nil && errors.Is(err, ErrCannotDeleteSubmittedReview)
+}
+
+func TestDismissReview(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPut {
+			t.Errorf("method = %s, want PUT", r.Method)
+		}
+		if r.URL.Path != "/repos/owner/repo/pulls/2/reviews/10/dismissals" {
+			t.Errorf("unexpected path: %s", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		var payload dismissReviewRequest
+		if err := json.NewDecoder(r.Body).Decode(&payload); err != nil {
+			t.Errorf("decode body: %v", err)
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+		if payload.Event != "DISMISS" {
+			t.Errorf("event = %q, want DISMISS", payload.Event)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(reviewResponse{ID: 10, State: "DISMISSED"})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	err := c.DismissReview(context.Background(), "owner", "repo", 2, 10, "outdated review")
+	if err != nil {
+		t.Fatalf("DismissReview: %v", err)
+	}
+}
+
+func TestDoRequestWithBody_RejectsHTTP(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Fatal("request should not have been sent")
+	}))
+	defer srv.Close()
+
+	// Without AllowInsecureHTTPForTest, HTTP should be rejected.
+	c := NewClient("tok", srv.URL)
+	_, err := c.doRequestWithBody(context.Background(), http.MethodPost, srv.URL+"/test", []byte(`{}`))
+	if err == nil {
+		t.Fatal("expected error for HTTP request")
+	}
+}
+
+func TestPostReview_CommitIDFromRequest(t *testing.T) {
+	const wantCommitID = "abc123def456"
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		var payload postReviewRequest
+		if err := json.NewDecoder(r.Body).Decode(&payload); err != nil {
+			t.Errorf("decode body: %v", err)
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+		if payload.CommitID != wantCommitID {
+			t.Errorf("commit_id = %q, want %q", payload.CommitID, wantCommitID)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(reviewResponse{
+			ID:       1,
+			Body:     payload.Body,
+			State:    "COMMENTED",
+			CommitID: payload.CommitID,
+			User:     struct{ Login string `json:"login"` }{Login: "rodin"},
+		})
+	}))
+	defer srv.Close()
+
+	c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest())
+	review, err := c.PostReview(context.Background(), "owner", "repo", 1, vcs.ReviewRequest{
+		Body:     "looks good",
+		Event:    vcs.ReviewEventApprove,
+		CommitID: wantCommitID,
+	})
+	if err != nil {
+		t.Fatalf("PostReview: %v", err)
+	}
+	if review.CommitID != wantCommitID {
+		t.Errorf("review.CommitID = %q, want %q", review.CommitID, wantCommitID)
+	}
+}
@@ -4,7 +4,10 @@
 package github

 import (
+	"bytes"
 	"context"
+	"encoding/base64"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -92,10 +95,6 @@ func asAPIError(err error) (*APIError, bool) {
 // SetHTTPClient and SetRetryBackoff are intended for test setup only and must
 // be called before any goroutines issue requests; they have no synchronization.
 type Client struct {
-	// TODO: baseURL is populated by NewClient but not yet consumed by doRequest/doGet.
-	// Higher-level exported methods (GetPullRequest, etc.) will use it to
-	// construct request URLs; remove this field if those methods end up
-	// accepting full URLs instead.
 	baseURL    string
 	token      string
 	httpClient *http.Client
@@ -376,3 +375,283 @@ func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept st
 func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) {
 	return c.doRequest(ctx, http.MethodGet, url, "")
 }
+
+// DefaultMaxDiffSize is the default maximum diff size in bytes (10 MB).
+const DefaultMaxDiffSize = 10 * 1024 * 1024
+
+// ErrDiffTooLarge is returned when a PR diff exceeds the configured MaxDiffSize.
+var ErrDiffTooLarge = errors.New("diff size exceeds maximum allowed size")
+
+// PullRequest holds relevant PR metadata.
+type PullRequest struct {
+	Title string `json:"title"`
+	Body  string `json:"body"`
+	Head  struct {
+		Sha string `json:"sha"`
+		Ref string `json:"ref"`
+	} `json:"head"`
+}
+
+// CommitStatus represents a single CI status entry.
+type CommitStatus struct {
+	Status      string `json:"state"`
+	Context     string `json:"context"`
+	Description string `json:"description"`
+	TargetURL   string `json:"target_url"`
+}
+
+// ChangedFile represents a file modified in a PR.
+type ChangedFile struct {
+	Filename string `json:"filename"`
+	Status   string `json:"status"`
+}
+
+// ContentEntry represents a file or directory in a repository.
+type ContentEntry struct {
+	Name string `json:"name"`
+	Path string `json:"path"`
+	Type string `json:"type"`
+}
+
+// contentResponse is the GitHub API response for a single file's content.
+type contentResponse struct {
+	Content  string `json:"content"`
+	Encoding string `json:"encoding"`
+	Type     string `json:"type"`
+	Name     string `json:"name"`
+	Path     string `json:"path"`
+}
+
+// GetPullRequest fetches PR metadata.
+func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error) {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
+
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return nil, fmt.Errorf("fetch PR: %w", err)
+	}
+	var pr PullRequest
+	if err := json.Unmarshal(body, &pr); err != nil {
+		return nil, fmt.Errorf("parse PR response: %w", err)
+	}
+	return &pr, nil
+}
+
+// GetPullRequestFiles fetches the list of files changed in a PR.
+func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error) {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/files",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
+
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return nil, fmt.Errorf("fetch PR files: %w", err)
+	}
+	var files []ChangedFile
+	if err := json.Unmarshal(body, &files); err != nil {
+		return nil, fmt.Errorf("parse PR files response: %w", err)
+	}
+	return files, nil
+}
+
+// GetCommitStatuses fetches CI statuses for a commit SHA.
+func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error) {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/commits/%s/statuses",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(sha))
+
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return nil, fmt.Errorf("fetch commit statuses: %w", err)
+	}
+	var statuses []CommitStatus
+	if err := json.Unmarshal(body, &statuses); err != nil {
+		return nil, fmt.Errorf("parse commit statuses response: %w", err)
+	}
+	return statuses, nil
+}
+
+// decodeFileContent decodes the content from a GitHub contents API response.
+// GitHub returns content as base64-encoded string with newlines inserted;
+// this function strips the newlines before decoding.
+func decodeFileContent(resp contentResponse) (string, error) {
+	if resp.Encoding != "base64" {
+		return "", fmt.Errorf("unsupported content encoding: %q", resp.Encoding)
+	}
+	// GitHub inserts newlines every 60 characters; strip them before decoding.
+	stripped := strings.ReplaceAll(resp.Content, "\n", "")
+	decoded, err := base64.StdEncoding.DecodeString(stripped)
+	if err != nil {
+		return "", fmt.Errorf("decode base64 content: %w", err)
+	}
+	return string(decoded), nil
+}
+
+// GetFileContent fetches the content of a file at the default branch HEAD.
+func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) {
+	return c.GetFileContentRef(ctx, owner, repo, filepath, "")
+}
+
+// GetFileContentRef fetches the content of a file at the given ref (branch, tag, or SHA).
+// If ref is empty, the default branch is used.
+func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/contents/%s",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), escapePath(filepath))
+	if ref != "" {
+		reqURL += "?ref=" + url.QueryEscape(ref)
+	}
+
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return "", fmt.Errorf("fetch file content %q: %w", filepath, err)
+	}
+	var resp contentResponse
+	if err := json.Unmarshal(body, &resp); err != nil {
+		return "", fmt.Errorf("parse file content response: %w", err)
+	}
+	content, err := decodeFileContent(resp)
+	if err != nil {
+		return "", fmt.Errorf("decode file content %q: %w", filepath, err)
+	}
+	return content, nil
+}
+
+// ListContents lists the files and directories at the given path.
+// If path points to a file instead of a directory, it returns a single-entry slice.
+func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/contents/%s",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), escapePath(path))
+
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return nil, fmt.Errorf("list contents %q: %w", path, err)
+	}
+
+	// The GitHub API returns an array for directories and an object for files.
+	// Try array first; if it fails, try object and wrap in a slice.
+	var entries []ContentEntry
+	if err := json.Unmarshal(body, &entries); err == nil {
+		return entries, nil
+	}
+
+	// Try as a single ContentEntry (file path).
+	var single contentResponse
+	if err := json.Unmarshal(body, &single); err != nil {
+		return nil, fmt.Errorf("parse contents response for %q: %w", path, err)
+	}
+	return []ContentEntry{{
+		Name: single.Name,
+		Path: single.Path,
+		Type: single.Type,
+	}}, nil
+}
+
+// doRequestWithBody performs an HTTP request with a JSON request body.
+// Unlike doRequest, it does NOT retry — write operations should not be
+// retried automatically.
+func (c *Client) doRequestWithBody(ctx context.Context, method, reqURL string, data []byte) ([]byte, error) {
+	if !c.allowInsecureHTTP {
+		parsed, err := url.Parse(reqURL)
+		if err != nil {
+			return nil, fmt.Errorf("parse request URL: %w", err)
+		}
+		if strings.EqualFold(parsed.Scheme, "http") {
+			return nil, fmt.Errorf("refusing HTTP request to %s: use HTTPS or set AllowInsecureHTTP option", redactURL(reqURL))
+		}
+	}
+
+	var bodyReader *bytes.Reader
+	if data != nil {
+		bodyReader = bytes.NewReader(data)
+	} else {
+		bodyReader = bytes.NewReader(nil)
+	}
+
+	req, err := http.NewRequestWithContext(ctx, method, reqURL, bodyReader)
+	if err != nil {
+		return nil, fmt.Errorf("create request: %w", err)
+	}
+	req.Header.Set("Authorization", "Bearer "+c.token)
+	req.Header.Set("Accept", "application/vnd.github+json")
+	if data != nil {
+		req.Header.Set("Content-Type", "application/json")
+	}
+
+	resp, err := c.httpClient.Do(req)
+	if err != nil {
+		return nil, fmt.Errorf("do request: %w", err)
+	}
+
+	if resp.StatusCode >= 200 && resp.StatusCode < 300 {
+		body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodyBytes))
+		resp.Body.Close()
+		if err != nil {
+			return nil, fmt.Errorf("read response body: %w", err)
+		}
+		return body, nil
+	}
+
+	errBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyBytes))
+	resp.Body.Close()
+
+	return nil, &APIError{StatusCode: resp.StatusCode, Body: string(errBody)}
+}
+
+// escapePath escapes each segment of a file path for use in URL path components.
+// Slashes are preserved as separators; individual segments are PathEscaped.
+func escapePath(p string) string {
+	parts := strings.Split(p, "/")
+	for i, part := range parts {
+		parts[i] = url.PathEscape(part)
+	}
+	return strings.Join(parts, "/")
+}
+
+// GetPullRequestDiff fetches the unified diff for a PR.
+// Returns ErrDiffTooLarge if the response exceeds DefaultMaxDiffSize bytes.
+//
+// It reads up to DefaultMaxDiffSize+1 bytes and checks for truncation:
+// if exactly DefaultMaxDiffSize+1 bytes are available, the diff is too large.
+func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
+
+	if !c.allowInsecureHTTP {
+		parsed, err := url.Parse(reqURL)
+		if err != nil {
+			return "", fmt.Errorf("parse request URL: %w", err)
+		}
+		if strings.EqualFold(parsed.Scheme, "http") {
+			return "", fmt.Errorf("refusing HTTP request to %s: use HTTPS or set AllowInsecureHTTP option", redactURL(reqURL))
+		}
+	}
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil)
+	if err != nil {
+		return "", fmt.Errorf("create PR diff request: %w", err)
+	}
+	req.Header.Set("Authorization", "Bearer "+c.token)
+	req.Header.Set("Accept", "application/vnd.github.diff")
+
+	resp, err := c.httpClient.Do(req)
+	if err != nil {
+		return "", fmt.Errorf("fetch PR diff: %w", err)
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+		errBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyBytes))
+		return "", &APIError{StatusCode: resp.StatusCode, Body: string(errBody)}
+	}
+
+	// Read up to DefaultMaxDiffSize+1 bytes. If we get exactly that many,
+	// the diff exceeds the limit.
+	limit := int64(DefaultMaxDiffSize) + 1
+	raw, err := io.ReadAll(io.LimitReader(resp.Body, limit))
+	if err != nil {
+		return "", fmt.Errorf("read PR diff: %w", err)
+	}
+	if int64(len(raw)) == limit {
+		return "", ErrDiffTooLarge
+	}
+	return string(raw), nil
+}
@@ -0,0 +1,29 @@
+package github
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+)
+
+// userResponse is the GitHub API response for the authenticated user.
+type userResponse struct {
+	Login string `json:"login"`
+}
+
+// GetAuthenticatedUser returns the login of the currently authenticated user.
+func (c *Client) GetAuthenticatedUser(ctx context.Context) (string, error) {
+	reqURL := fmt.Sprintf("%s/user", c.baseURL)
+
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return "", fmt.Errorf("get authenticated user: %w", err)
+	}
+
+	var resp userResponse
+	if err := json.Unmarshal(body, &resp); err != nil {
+		return "", fmt.Errorf("parse user response: %w", err)
+	}
+
+	return resp.Login, nil
+}
@@ -0,0 +1,212 @@
+package github
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"net/http"
+	"net/url"
+
+	"gitea.weiker.me/rodin/review-bot/vcs"
+)
+
+// ErrCannotDeleteSubmittedReview is returned when DeleteReview is called on
+// a review that has already been submitted (APPROVED, REQUEST_CHANGES, COMMENT).
+// GitHub only allows deletion of PENDING reviews. Callers that need to replace
+// a submitted review should use DismissReview instead.
+var ErrCannotDeleteSubmittedReview = errors.New("cannot delete submitted review: use DismissReview instead")
+
+// ErrConflictingCommitIDs is returned when PostReview receives comments with
+// differing non-empty CommitIDs. The GitHub API accepts only a single commit_id
+// per review submission; callers must ensure all comments target the same commit.
+var ErrConflictingCommitIDs = errors.New("comments contain conflicting commit IDs: all must target the same commit")
+
+// postReviewRequest is the GitHub API request body for creating a review.
+type postReviewRequest struct {
+	CommitID string               `json:"commit_id,omitempty"`
+	Body     string               `json:"body"`
+	Event    string               `json:"event"`
+	Comments []reviewCommentEntry `json:"comments,omitempty"`
+}
+
+// reviewCommentEntry is a single inline comment in a review creation request.
+type reviewCommentEntry struct {
+	Path     string `json:"path"`
+	Position int    `json:"position"`
+	Body     string `json:"body"`
+}
+
+// reviewResponse is the GitHub API response for a review.
+type reviewResponse struct {
+	ID       int64  `json:"id"`
+	Body     string `json:"body"`
+	State    string `json:"state"`
+	CommitID string `json:"commit_id"`
+	User     struct {
+		Login string `json:"login"`
+	} `json:"user"`
+}
+
+// dismissReviewRequest is the GitHub API request body for dismissing a review.
+type dismissReviewRequest struct {
+	Message string `json:"message"`
+	Event   string `json:"event"`
+}
+
+// translateGitHubReviewState translates a GitHub API review state to the
+// canonical vcs.Review.State value.
+func translateGitHubReviewState(state string) string {
+	switch state {
+	case "CHANGES_REQUESTED":
+		return "REQUEST_CHANGES"
+	case "COMMENTED":
+		return "COMMENT"
+	default:
+		// States like APPROVED, DISMISSED, and PENDING pass through unchanged
+		// as they already match the canonical vcs representation. PENDING appears
+		// on draft reviews that have not yet been submitted via the GitHub UI or API.
+		return state
+	}
+}
+
+// PostReview submits a review on a pull request.
+//
+// The vcs.ReviewEvent constants (ReviewEventApprove, ReviewEventRequestChanges,
+// ReviewEventComment) have string values that match GitHub's wire-format event
+// strings (APPROVE, REQUEST_CHANGES, COMMENT), so Event is cast directly to
+// string without translation.
+//
+// ReviewComment.Position maps directly to the GitHub API position field.
+// When req.Comments is empty, the payload omits the comments field entirely
+// (via the omitempty tag on postReviewRequest.Comments).
+//
+// The GitHub API accepts a single commit_id per review submission. PostReview
+// extracts it from the first comment with a non-empty CommitID. If any subsequent
+// comment specifies a different CommitID, PostReview returns ErrConflictingCommitIDs.
+// Comments with an empty CommitID are allowed and inherit the review-level value.
+func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*vcs.Review, error) {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
+
+	payload := postReviewRequest{
+		Body:     req.Body,
+		Event:    string(req.Event),
+		CommitID: req.CommitID,
+	}
+
+	// Build the comments in one pass. Inline comment CommitIDs must match the
+	// review-level CommitID; reject if any disagree.
+	for _, comment := range req.Comments {
+		if comment.CommitID != "" {
+			if payload.CommitID == "" {
+				payload.CommitID = comment.CommitID
+			} else if payload.CommitID != comment.CommitID {
+				return nil, ErrConflictingCommitIDs
+			}
+		}
+		payload.Comments = append(payload.Comments, reviewCommentEntry{
+			Path:     comment.Path,
+			Position: comment.Position,
+			Body:     comment.Body,
+		})
+	}
+
+	data, err := json.Marshal(payload)
+	if err != nil {
+		return nil, fmt.Errorf("marshal review request: %w", err)
+	}
+
+	body, err := c.doRequestWithBody(ctx, http.MethodPost, reqURL, data)
+	if err != nil {
+		return nil, fmt.Errorf("post review: %w", err)
+	}
+
+	var resp reviewResponse
+	if err := json.Unmarshal(body, &resp); err != nil {
+		return nil, fmt.Errorf("parse review response: %w", err)
+	}
+
+	return &vcs.Review{
+		ID:       resp.ID,
+		Body:     resp.Body,
+		User:     vcs.UserInfo{Login: resp.User.Login},
+		State:    translateGitHubReviewState(resp.State),
+		CommitID: resp.CommitID,
+	}, nil
+}
+
+// ListReviews retrieves all reviews for a pull request.
+// GitHub review states are translated to canonical vcs values.
+func (c *Client) ListReviews(ctx context.Context, owner, repo string, number int) ([]vcs.Review, error) {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number)
+
+	body, err := c.doGet(ctx, reqURL)
+	if err != nil {
+		return nil, fmt.Errorf("list reviews: %w", err)
+	}
+
+	var responses []reviewResponse
+	if err := json.Unmarshal(body, &responses); err != nil {
+		return nil, fmt.Errorf("parse reviews response: %w", err)
+	}
+
+	reviews := make([]vcs.Review, len(responses))
+	for i, r := range responses {
+		reviews[i] = vcs.Review{
+			ID:       r.ID,
+			Body:     r.Body,
+			User:     vcs.UserInfo{Login: r.User.Login},
+			State:    translateGitHubReviewState(r.State),
+			CommitID: r.CommitID,
+		}
+	}
+	return reviews, nil
+}
+
+// DeleteReview deletes a pull request review.
+// Only PENDING reviews can be deleted; attempting to delete a submitted review
+// (APPROVED, CHANGES_REQUESTED, or COMMENTED per GitHub API naming) returns
+// ErrCannotDeleteSubmittedReview.
+func (c *Client) DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews/%d",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, reviewID)
+
+	// nil body: the GitHub DELETE endpoint for reviews requires no request body.
+	_, err := c.doRequestWithBody(ctx, http.MethodDelete, reqURL, nil)
+	if err != nil {
+		var apiErr *APIError
+		if errors.As(err, &apiErr) && apiErr.StatusCode == 422 {
+			return fmt.Errorf("delete review: %w", ErrCannotDeleteSubmittedReview)
+		}
+		return fmt.Errorf("delete review: %w", err)
+	}
+	return nil
+}
+
+// DismissReview dismisses a submitted review on a pull request.
+// This is the correct way to "remove" a submitted review (APPROVED, REQUEST_CHANGES).
+// GitHub does not allow deleting submitted reviews — they must be dismissed.
+func (c *Client) DismissReview(ctx context.Context, owner, repo string, number int, reviewID int64, message string) error {
+	reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews/%d/dismissals",
+		c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, reviewID)
+
+	payload := dismissReviewRequest{
+		Message: message,
+		// Event is required by the GitHub API for dismissal requests, even though
+		// "DISMISS" is the only valid value for this endpoint.
+		Event: "DISMISS",
+	}
+
+	data, err := json.Marshal(payload)
+	if err != nil {
+		return fmt.Errorf("marshal dismiss request: %w", err)
+	}
+
+	_, err = c.doRequestWithBody(ctx, http.MethodPut, reqURL, data)
+	if err != nil {
+		return fmt.Errorf("dismiss review: %w", err)
+	}
+	return nil
+}
@@ -16,16 +16,17 @@ import (

 // Integration test requires a running Gitea instance and LLM endpoint.
 // Set environment variables:
-//   INTEGRATION_GITEA_URL     - Gitea base URL
-//   INTEGRATION_GITEA_TOKEN   - Gitea API token with repo access
-//   INTEGRATION_GITEA_REPO    - owner/repo with an open PR
-//   INTEGRATION_PR_NUMBER     - PR number to test against
-//   INTEGRATION_LLM_BASE_URL  - LLM API base URL
-//   INTEGRATION_LLM_API_KEY   - LLM API key
-//   INTEGRATION_LLM_MODEL     - Model name
+//
+//	INTEGRATION_VCS_URL       - VCS base URL
+//	INTEGRATION_GITEA_TOKEN   - Gitea API token with repo access
+//	INTEGRATION_GITEA_REPO    - owner/repo with an open PR
+//	INTEGRATION_PR_NUMBER     - PR number to test against
+//	INTEGRATION_LLM_BASE_URL  - LLM API base URL
+//	INTEGRATION_LLM_API_KEY   - LLM API key
+//	INTEGRATION_LLM_MODEL     - Model name

 func TestIntegration_FullReviewFlow(t *testing.T) {
-	giteaURL := os.Getenv("INTEGRATION_GITEA_URL")
+	giteaURL := os.Getenv("INTEGRATION_VCS_URL")
 	giteaToken := os.Getenv("INTEGRATION_GITEA_TOKEN")
 	giteaRepo := os.Getenv("INTEGRATION_GITEA_REPO")
 	prNumStr := os.Getenv("INTEGRATION_PR_NUMBER")
@@ -0,0 +1,78 @@
+// Package vcs defines shared types used across VCS client implementations
+// (gitea, github). Keeping them here prevents the client packages from
+// importing each other or duplicating definitions.
+package vcs
+
+// ReviewEvent is the verdict submitted with a pull request review.
+type ReviewEvent string
+
+const (
+	// ReviewEventApprove approves the pull request.
+	ReviewEventApprove ReviewEvent = "APPROVE"
+
+	// ReviewEventRequestChanges requests changes before the PR can be merged.
+	ReviewEventRequestChanges ReviewEvent = "REQUEST_CHANGES"
+
+	// ReviewEventComment leaves a comment review without explicit approval or rejection.
+	ReviewEventComment ReviewEvent = "COMMENT"
+)
+
+// UserInfo holds the identity of a user.
+type UserInfo struct {
+	Login string
+}
+
+// ReviewComment is a single inline comment attached to a pull request review.
+type ReviewComment struct {
+	// Path is the file path the comment applies to.
+	Path string
+
+	// Position is the line position within the diff (1-indexed).
+	// GitHub and Gitea differ in how they compute position; callers must
+	// supply the correct value for the target VCS.
+	Position int
+
+	// Body is the text content of the comment.
+	Body string
+
+	// CommitID is the SHA of the commit the comment applies to.
+	// For GitHub, all comments within a single review must target the same commit.
+	CommitID string
+}
+
+// ReviewRequest is the payload for submitting a pull request review.
+type ReviewRequest struct {
+	// Body is the top-level review comment body.
+	Body string
+
+	// Event is the review verdict.
+	Event ReviewEvent
+
+	// CommitID is the commit SHA that this review targets.
+	// When non-empty, the review is anchored to this commit.
+	// For GitHub, this is passed as commit_id in the review request.
+	// For Gitea, this is passed as the commitID parameter to PostReview.
+	CommitID string
+
+	// Comments are optional inline file-level comments.
+	Comments []ReviewComment
+}
+
+// Review represents a submitted pull request review.
+type Review struct {
+	// ID is the provider-assigned review identifier.
+	ID int64
+
+	// Body is the top-level review comment body.
+	Body string
+
+	// User is the author of the review.
+	User UserInfo
+
+	// State is the canonical review state string.
+	// Values: APPROVE, REQUEST_CHANGES, COMMENT, DISMISSED, PENDING.
+	State string
+
+	// CommitID is the commit SHA the review was evaluated against.
+	CommitID string
+}
Author	SHA1	Message	Date
claw	8be12602f0	fix(#130 ): cleanup and test improvements PR Ready Gate / clear-labels (pull_request) Successful in 2s Details CI / test (pull_request) Successful in 17s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 39s Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m3s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m7s Details - Fix extra blank lines between GetPullRequest and GetPullRequestFiles - Add TestPostReview_CommitIDFromRequest to verify CommitID passes through	2026-05-14 13:57:58 -07:00
claw	2dedab1ad3	feat(#130 ): add VCS routing in cmd/review-bot via --vcs-type flag CI / test (pull_request) Successful in 18s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 41s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m20s Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m52s Details - Add vcs_client.go with vcsClient interface, giteaClientVCSAdapter, githubClientVCSAdapter - Add giteaExtendedClient interface for Gitea-specific operations (supersede, resolve, etc.) - Add --vcs-type flag (gitea\|github, default: gitea) with VCS_TYPE env var support - Replace direct giteaClient usage with vcsClient interface in main.go - Add verdictToVCSEvent() to map LLM verdict to vcs.ReviewEvent - Gitea adapter translates vcs.ReviewEvent back to Gitea API format (APPROVE->APPROVED) - Guard Gitea-specific ops (RequestReviewer, supersede, resolve) with type assertion - Guard fetchPatterns GetAllFilesInPath with Gitea-only type assertion - Replace reviewClientAdapter (giteaClientAdapter) for review.GiteaClient interface - Update main_test.go to use reviewInfo/commitStatusInfo instead of gitea types	2026-05-14 13:53:56 -07:00
claw	5704d167a5	feat(#130 ): implement GitHub API methods and vcs types - Add vcs package with shared review types (ReviewEvent, Review, ReviewRequest, ReviewComment, UserInfo) - Add GetPullRequest, GetPullRequestDiff, GetPullRequestFiles, GetCommitStatuses to github/client.go - Add GetFileContent, GetFileContentRef, ListContents to github/client.go - Add doRequestWithBody helper for POST/PUT/DELETE operations - Add review.go with PostReview, ListReviews, DeleteReview, DismissReview - Add identity.go with GetAuthenticatedUser - Remove TODO comment from github/client.go - Add escapePath helper for URL path construction - Add comprehensive tests for all new methods using httptest.NewServer	2026-05-14 13:45:36 -07:00
claw	fc3ca21329	feat(#130 ): add vcs package with shared review types	2026-05-14 13:41:14 -07:00
Rodin	bbf3dfbf0d	chore: dev-loop health check — status at 2026-05-14 20:10 UTC CI / test (push) Successful in 22s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped Details	2026-05-14 20:10:56 +00:00
Rodin	ed3a5dddf1	chore: dev-loop health check — cleanup & status at 2026-05-14 19:25 UTC CI / test (push) Successful in 17s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped Details	2026-05-14 19:26:04 +00:00
Rodin	449a24e4c5	chore: dev-loop status after cleanup at 2026-05-14 19:20 UTC CI / test (push) Successful in 18s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped Details	2026-05-14 19:21:18 +00:00
rodin	4440823571	Merge pull request 'feat(#123 ): add IP-level SSRF defense to Gitea client and action' (#129 ) from issue-123 into main CI / test (push) Successful in 18s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped Details	2026-05-14 19:10:20 +00:00
Rodin	c349986187	fix(#123 ): add RFC6598 CGN check to Python SSRF validation in action.yml PR Ready Gate / clear-labels (pull_request) Successful in 2s Details CI / test (pull_request) Successful in 25s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 41s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m16s Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 2m49s Details 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 PR Ready Gate / clear-labels (pull_request) Successful in 2s Details CI / test (pull_request) Successful in 17s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 46s Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 2m14s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m24s Details - 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 PR Ready Gate / clear-labels (pull_request) Successful in 2s Details CI / test (pull_request) Successful in 15s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 47s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m49s Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 2m15s Details 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 PR Ready Gate / clear-labels (pull_request) Successful in 2s Details CI / test (pull_request) Successful in 17s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 34s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m25s Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 2m30s Details 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: 10time.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 CI / test (pull_request) Successful in 18s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 44s Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m57s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 2m21s Details ## 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
aweiker	50facefdd6	Merge PR #121 : fix(action): detect VCS host type for version resolution and binary download CI / test (push) Successful in 18s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped Details This PR addresses issue #120 by adding GitHub/GHES support to the composite action. Key improvements: - Detects VCS host type via github.api_url presence (GitHub/GHES vs Gitea) - Uses correct API paths: /api/v3 for GitHub, /api/v1 for Gitea - Prevents token exfiltration by ignoring inputs.vcs-url on GitHub runners - Validates inputs (action-repo format, URL scheme, tokens) - Adds multi-arch support (uname-derived binary/OS detection) - Reuses computed values across steps via GITHUB_OUTPUT - Properly handles private release assets on GitHub via REST API - Backward compatible with existing Gitea workflows Reviewed and approved by sonnet-review, gpt-review, and security-review.	2026-05-14 07:28:43 +00:00
aweiker	bd2df7d986	feat(#120 ): add GitHub Actions support with VCS host detection and security hardening PR Ready Gate / clear-labels (pull_request) Successful in 1s Details CI / test (pull_request) Successful in 17s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 33s Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m39s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m58s Details - Detect VCS host type from github.api_url (present on GitHub/GHES, absent on Gitea) - Add action-repo input: specifies repo hosting review-bot releases, separate from the reviewed repo. Defaults to github.action_repository, then rodin/review-bot. - Add action-repo-token input: auth for release downloads (defaults to github.token on GitHub, reviewer-token on Gitea). - GitHub/GHES path: use github.api_url for version resolution and REST API asset download endpoint (required for private repos; web URLs redirect to S3 and don't support Authorization headers reliably). - Gitea path: use validated SERVER_URL with direct download (no -L; prevents Authorization forwarding on potential CDN redirects). - Security hardening: - inputs.vcs-url is IGNORED on GitHub/GHES to prevent token exfiltration - SERVER_URL validated for https scheme and no whitespace on Gitea path - action-repo validated against owner/repo pattern (prevent path traversal) - VERSION validated for no slashes/whitespace - TOKEN validated for no control characters (header injection defense) - ACTION_TOKEN passed via ::add-mask:: + GITHUB_ENV (not step output, which can leak in debug logs) - set -euo pipefail in both script steps - Multi-arch support: OS/arch detection via uname (linux/darwin, amd64/arm64) for cache key and binary name — incorporates changes from #124 - Run review step: passes VCS_URL from step output (server_url) instead of direct input expression Closes #120	2026-05-14 07:14:58 +00:00
aweiker	d3bb83a10a	docs(#125 ): update CLI example to use --vcs-url instead of deprecated --gitea-url CI / test (pull_request) Successful in 18s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 22s Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 25s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 34s Details CI / test (push) Successful in 21s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped Details	2026-05-14 06:50:21 +00:00
rodin	c56f5fec52	Merge pull request 'feat(#125 ): rename GITEA_URL to VCS_URL with deprecated fallback' (#126 ) from issue-125 into main CI / test (push) Successful in 17s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped Details	2026-05-14 06:38:53 +00:00
Rodin	b80a1517ed	fix: remove trailing whitespace in action.yml PR Ready Gate / clear-labels (pull_request) Successful in 1s Details CI / test (pull_request) Successful in 15s Details CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 38s Details CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m17s Details CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m24s Details	2026-05-13 23:00:35 -07:00
Rodin	5f7ffab487	feat(#125 ): rename GITEA_URL to VCS_URL with deprecated fallback - Add --vcs-url flag as primary (reads VCS_URL env var) - Keep --gitea-url and GITEA_URL as deprecated fallbacks with warnings - Update action.yml: rename gitea-url input to vcs-url, pass VCS_URL to binary - Update ci.yml: use VCS_URL env var in Run review step - Update integration tests: INTEGRATION_GITEA_URL -> INTEGRATION_VCS_URL - Update README: --vcs-url / VCS_URL with fallback note in env var table Backward compat: existing GITEA_URL users get a deprecation warning and continue to work unchanged until they migrate to VCS_URL.	2026-05-13 23:00:35 -07:00