fix: portable checksum on darwin, anchor grep pattern

- sha256sum is not available on macOS; use shasum -a 256 on darwin.
  Select based on steps.version.outputs.os which is already computed.
  Fixes MAJOR finding from gpt-review-bot (PR #127 review).

- Anchor checksum grep with ^ to avoid matching on partial lines.
  Fixes MINOR finding from gpt-review-bot (PR #127 review).

.gitea/actions/review/action.yml

@@ -124,14 +124,38 @@ runs:
         else
           VERSION="${{ inputs.version }}"
         fi
+
+        # Detect OS and architecture for platform-specific binary download
+        OS_RAW=$(uname -s | tr '[:upper:]' '[:lower:]')
+        case "$OS_RAW" in
+          linux)  OS="linux" ;;
+          darwin) OS="darwin" ;;
+          *)
+            echo "Error: unsupported OS: $(uname -s)" >&2
+            exit 1
+            ;;
+        esac
+
+        RAW_ARCH=$(uname -m)
+        case "$RAW_ARCH" in
+          x86_64)           ARCH="amd64" ;;
+          aarch64 | arm64)  ARCH="arm64" ;;
+          *)
+            echo "Error: unsupported architecture: $RAW_ARCH" >&2
+            exit 1
+            ;;
+        esac
+
         echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
+        echo "os=${OS}" >> "$GITHUB_OUTPUT"
+        echo "arch=${ARCH}" >> "$GITHUB_OUTPUT"
 
     - name: Cache review-bot binary
       id: cache
       uses: actions/cache@v4
       with:
         path: ${{ runner.temp }}/review-bot
-        key: review-bot-linux-amd64-${{ steps.version.outputs.version }}
+        key: review-bot-${{ steps.version.outputs.os }}-${{ steps.version.outputs.arch }}-${{ steps.version.outputs.version }}
 
     - name: Install review-bot
       if: steps.cache.outputs.cache-hit != 'true'
@@ -140,7 +164,7 @@ runs:
         GITEA_URL="${{ inputs.gitea-url || github.server_url }}"
         REPO="${{ inputs.repo || 'rodin/review-bot' }}"
         VERSION="${{ steps.version.outputs.version }}"
-        BINARY="review-bot-linux-amd64"
+        BINARY="review-bot-${{ steps.version.outputs.os }}-${{ steps.version.outputs.arch }}"
 
         curl -sSfL "${GITEA_URL}/${REPO}/releases/download/${VERSION}/${BINARY}" \
           -o "${{ runner.temp }}/review-bot"
@@ -149,8 +173,13 @@ runs:
 
         # Verify SHA-256 checksum
         cd "${{ runner.temp }}"
-        EXPECTED=$(grep "${BINARY}" checksums.txt | awk '{print $1}')
+        EXPECTED=$(grep -E "^[[:xdigit:]]+[[:space:]]+\*?${BINARY}$" checksums.txt | awk '{print $1}')
-        ACTUAL=$(sha256sum review-bot | awk '{print $1}')
+        # sha256sum (GNU) is not available on macOS; use shasum -a 256 on darwin.
+        if [ "${{ steps.version.outputs.os }}" = "darwin" ]; then
+          ACTUAL=$(shasum -a 256 review-bot | awk '{print $1}')
+        else
+          ACTUAL=$(sha256sum review-bot | awk '{print $1}')
+        fi
 
         if [ -z "$EXPECTED" ]; then
           echo "Error: no checksum found for ${BINARY}" >&2
@@ -164,7 +193,7 @@ runs:
         fi
 
         chmod +x "${{ runner.temp }}/review-bot"
-        echo "Installed review-bot ${VERSION} (checksum verified)"
+        echo "Installed review-bot-${{ steps.version.outputs.os }}-${{ steps.version.outputs.arch }} ${VERSION} (checksum verified)"
 
     - name: Run review
       shell: bash

github/client.go

@@ -10,6 +10,7 @@ import (
 	"io"
 	"log/slog"
 	"net/http"
+	"net/url"
 	"os"
 	"strconv"
 	"strings"
@@ -95,8 +96,8 @@ type Client struct {
 	// 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
+	baseURL    string
-	token   string
+	token      string
 	httpClient *http.Client
 
 	// allowInsecureHTTP permits requests to HTTP (non-TLS) endpoints.
@@ -151,26 +152,16 @@ type clientConfig struct {
 
 // AllowInsecureHTTP permits sending credentials over plaintext HTTP connections.
 // In production, this option is gated by the REVIEW_BOT_ALLOW_INSECURE=1
-// environment variable. Without the env var set, the option is silently ignored
+// environment variable. Without the env var set, the option is ignored
 // and a warning is logged.
 //
-// For tests, prefer AllowInsecureHTTPForTest which bypasses the env gate.
+// For tests, use AllowInsecureHTTPForTest (defined in a _test.go file in the same package) which bypasses the env gate.
 func AllowInsecureHTTP() ClientOption {
 	return func(cfg *clientConfig) {
 		cfg.allowInsecureHTTP = true
 	}
 }
 
-// AllowInsecureHTTPForTest permits sending credentials over plaintext HTTP
-// without requiring the REVIEW_BOT_ALLOW_INSECURE environment variable.
-// This is intended exclusively for test code using httptest.Server.
-func AllowInsecureHTTPForTest() ClientOption {
-	return func(cfg *clientConfig) {
-		cfg.allowInsecureHTTP = true
-		cfg.insecureIsTestBypass = true
-	}
-}
-
 // NewClient creates a new GitHub API client.
 // If baseURL is empty, it defaults to https://api.github.com.
 // For GitHub Enterprise, pass the API base URL (e.g. https://github.concur.com/api/v3).
@@ -195,8 +186,8 @@ func NewClient(token, baseURL string, opts ...ClientOption) *Client {
 	}
 
 	return &Client{
-		baseURL:          strings.TrimRight(baseURL, "/"),
+		baseURL:           strings.TrimRight(baseURL, "/"),
-		token:            token,
+		token:             token,
 		allowInsecureHTTP: cfg.allowInsecureHTTP,
 		httpClient: &http.Client{
 			Timeout:       30 * time.Second,
@@ -268,21 +259,37 @@ func (c *Client) parseRetryAfter(value string) (time.Duration, bool) {
 	return 0, false
 }
 
-// redactURL redacts query parameters from a URL for safe inclusion in error
+// redactURL redacts sensitive components from a URL for safe inclusion in error
-// messages and log output.
+// messages and log output. It removes userinfo (e.g., user:pass@) and replaces
+// query parameters with a placeholder.
 func redactURL(rawURL string) string {
-	if idx := strings.IndexByte(rawURL, '?'); idx != -1 {
+	u, err := url.Parse(rawURL)
-		return rawURL[:idx] + "?<redacted>"
+	if err != nil {
+		return "<unparseable URL>"
 	}
-	return rawURL
+	u.User = nil
+
+	if u.RawQuery != "" {
+		u.RawQuery = "<redacted>"
+	}
+	return u.String()
 }
 
 // doRequest performs an HTTP request with retry on 429 rate limit responses.
 // It respects the Retry-After header when present, supporting both integer
 // seconds and HTTP-date formats (capped at maxRetryAfter).
 func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept string) ([]byte, error) {
-	if !c.allowInsecureHTTP && strings.HasPrefix(reqURL, "http://") {
+	// NOTE: This parses reqURL a second time (http.NewRequestWithContext parses it
-		return nil, fmt.Errorf("refusing HTTP request to %s: use HTTPS or set AllowInsecureHTTP option", redactURL(reqURL))
+	// again internally). Acceptable cost: URL parsing is cheap and threading the
+	// parsed *url.URL through would complicate the interface for negligible gain.
+	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 backoff []time.Duration
@@ -303,7 +310,7 @@ func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept st
 				timer := time.NewTimer(delay)
 				select {
 				case <-timer.C:
-					timer.Stop()
+					timer.Stop() // no-op after fire; kept for symmetry with the ctx.Done case
 				case <-ctx.Done():
 					timer.Stop()
 					return nil, ctx.Err()

github/client_test.go

@@ -545,6 +545,30 @@ func TestNoInsecureOption_RejectsHTTP(t *testing.T) {
 	}
 }
 
+func TestNoInsecureOption_RejectsUppercaseHTTP(t *testing.T) {
+	// Verify case-insensitive scheme check (RFC 3986).
+	c := NewClient("tok", "HTTP://127.0.0.1:1")
+	_, err := c.doGet(context.Background(), "HTTP://127.0.0.1:1/test")
+	if err == nil {
+		t.Fatal("expected error for uppercase HTTP scheme")
+	}
+	if !strings.Contains(err.Error(), "refusing HTTP request") {
+		t.Errorf("unexpected error message: %v", err)
+	}
+}
+
+func TestNoInsecureOption_RejectsMixedCaseHTTP(t *testing.T) {
+	// Verify mixed case like "Http://" is also rejected.
+	c := NewClient("tok", "Http://127.0.0.1:1")
+	_, err := c.doGet(context.Background(), "Http://127.0.0.1:1/test")
+	if err == nil {
+		t.Fatal("expected error for mixed-case HTTP scheme")
+	}
+	if !strings.Contains(err.Error(), "refusing HTTP request") {
+		t.Errorf("unexpected error message: %v", err)
+	}
+}
+
 func TestAllowInsecureHTTP_WithoutEnvVar_Rejected(t *testing.T) {
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		t.Fatal("request should not have been sent")
@@ -616,3 +640,19 @@ func TestRedactURL_NoQuery(t *testing.T) {
 		t.Errorf("redactURL = %q, want %q", got, want)
 	}
 }
+
+func TestRedactURL_Userinfo(t *testing.T) {
+	got := redactURL("http://user:pass@localhost:1234/path")
+	want := "http://localhost:1234/path"
+	if got != want {
+		t.Errorf("redactURL = %q, want %q", got, want)
+	}
+}
+
+func TestRedactURL_UserinfoWithQuery(t *testing.T) {
+	got := redactURL("http://user:pass@localhost:1234/path?secret=token")
+	want := "http://localhost:1234/path?<redacted>"
+	if got != want {
+		t.Errorf("redactURL = %q, want %q", got, want)
+	}
+}

github/export_test.go

@@ -0,0 +1,13 @@
+package github
+
+// AllowInsecureHTTPForTest permits sending credentials over plaintext HTTP
+// without requiring the REVIEW_BOT_ALLOW_INSECURE environment variable.
+// This is intended exclusively for test code using httptest.Server.
+//
+// Defined in a _test.go file so it is only available to test binaries.
+func AllowInsecureHTTPForTest() ClientOption {
+	return func(cfg *clientConfig) {
+		cfg.allowInsecureHTTP = true
+		cfg.insecureIsTestBypass = true
+	}
+}