From 27d7fd3a93e54da20994ec6869dd6e8831add91d Mon Sep 17 00:00:00 2001 From: Aaron Weiker Date: Thu, 14 May 2026 05:04:14 +0000 Subject: [PATCH] address review feedback: portability, docs, and security hardening - Replace grep -qP with POSIX-compatible LC_ALL=C grep -q '[^[:print:]]' - Inline auth headers directly in curl calls (eliminate AUTH_HEADER variable) - Remove redundant sys.exit(1) from Python for-else (shell empty-check suffices) - Update top-of-file comment to match actual detection mechanism - Remove -L from Gitea download curl calls to prevent auth header forwarding on potential redirects (defense-in-depth) --- .gitea/actions/review/action.yml | 42 +++++++++++++++----------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/.gitea/actions/review/action.yml b/.gitea/actions/review/action.yml index 1859d41..111a284 100644 --- a/.gitea/actions/review/action.yml +++ b/.gitea/actions/review/action.yml @@ -1,7 +1,8 @@ # This composite action supports both Gitea Actions and GitHub Actions runners. -# It detects the VCS host type using the github.api_url context (set only on -# GitHub/GHES runners) and uses the appropriate releases API for version -# resolution and binary download (REST API on GitHub, direct URLs on Gitea). +# 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.gitea-url is IGNORED to prevent @@ -198,7 +199,7 @@ runs: # Validate token contains no control characters (defense-in-depth against header injection) if [ -n "$ACTION_TOKEN" ]; then - if printf '%s' "$ACTION_TOKEN" | grep -qP '[\x00-\x1f\x7f]'; then + if printf '%s' "$ACTION_TOKEN" | LC_ALL=C grep -q '[^[:print:]]'; then echo "Error: ACTION_TOKEN contains control characters" >&2 exit 1 fi @@ -214,20 +215,17 @@ runs: API_URL="${SERVER_URL}/api/v1/repos/${ACTION_REPO}/releases?limit=1" fi - # Build auth header if token is available - AUTH_HEADER="" + # Fetch latest version with inline auth header (no intermediate variable) if [ -n "$ACTION_TOKEN" ]; then if [ "$VCS_TYPE" = "github" ]; then - AUTH_HEADER="Authorization: Bearer ${ACTION_TOKEN}" + 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 - AUTH_HEADER="Authorization: token ${ACTION_TOKEN}" + 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 - fi - - if [ -n "$AUTH_HEADER" ]; then - VERSION=$(curl -sSf --connect-timeout 10 --max-time 30 \ - -H "$AUTH_HEADER" "$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 "$API_URL" \ | python3 -c "import sys, json; releases = json.load(sys.stdin); print(releases[0]['tag_name'] if releases else '')") @@ -307,8 +305,6 @@ for a in assets: if a['name'] == '${BINARY}': print(a['id']) break -else: - sys.exit(1) ") if [ -z "$BINARY_ASSET_ID" ]; then echo "Error: could not find asset '${BINARY}' in release ${VERSION}" >&2 @@ -322,8 +318,6 @@ for a in assets: if a['name'] == 'checksums.txt': print(a['id']) break -else: - sys.exit(1) ") if [ -z "$CHECKSUMS_ASSET_ID" ]; then echo "Error: could not find asset 'checksums.txt' in release ${VERSION}" >&2 @@ -354,20 +348,22 @@ else: fi else # Gitea: Direct download via web release URLs (Gitea serves assets - # directly and supports token auth on these URLs) + # 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 -sSfL --connect-timeout 10 --max-time 120 \ + curl -sSf --connect-timeout 10 --max-time 120 \ -H "Authorization: token ${ACTION_TOKEN}" \ "${DOWNLOAD_URL}/${BINARY}" -o "${{ runner.temp }}/review-bot" - curl -sSfL --connect-timeout 10 --max-time 30 \ + curl -sSf --connect-timeout 10 --max-time 30 \ -H "Authorization: token ${ACTION_TOKEN}" \ "${DOWNLOAD_URL}/checksums.txt" -o "${{ runner.temp }}/checksums.txt" else - curl -sSfL --connect-timeout 10 --max-time 120 \ + curl -sSf --connect-timeout 10 --max-time 120 \ "${DOWNLOAD_URL}/${BINARY}" -o "${{ runner.temp }}/review-bot" - curl -sSfL --connect-timeout 10 --max-time 30 \ + curl -sSf --connect-timeout 10 --max-time 30 \ "${DOWNLOAD_URL}/checksums.txt" -o "${{ runner.temp }}/checksums.txt" fi fi