fix(action): address review findings - validation hardening and cleanup
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 16s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 38s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 59s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 2m14s

Addresses findings from reviews #3655 (sonnet), #3657 (security), #3658 (gpt):

- Add set -euo pipefail to both script steps for fail-fast behavior
- Remove redundant newline check ([:space:] already covers it)
- Simplify VERSION regex: remove non-portable \n\r in POSIX ERE
- Add ACTION_TOKEN control character validation (defense-in-depth)
- Anchor checksum grep to exact filename match (prevent substring collision)
- Add ::notice:: when falling back to default ACTION_REPO
- Translate Chinese comments to English for consistency
- Add comment linking GITHUB_API_URL usage back to VCS detection
This commit is contained in:
claw
2026-05-13 21:54:08 -07:00
parent e709956d0b
commit 220f6e7369
+21 -9
View File
@@ -131,6 +131,8 @@ runs:
id: version
shell: bash
run: |
set -euo pipefail
# --- Input Validation ---
# Determine the repo hosting review-bot releases (not the repo being reviewed)
@@ -142,9 +144,10 @@ runs:
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 (防止 path traversal)
# 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
@@ -174,16 +177,13 @@ runs:
# Strip trailing slash if present
SERVER_URL="${SERVER_URL%/}"
# Validate SERVER_URL for Gitea path: must be https, no whitespace/newlines
# 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
if printf '%s' "$SERVER_URL" | grep -q $'\n'; then
echo "Error: SERVER_URL contains unexpected newline" >&2
exit 1
fi
fi
# Determine auth token for release API requests
@@ -196,6 +196,14 @@ runs:
fi
fi
# 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
echo "Error: ACTION_TOKEN contains control characters" >&2
exit 1
fi
fi
if [ "${{ inputs.version }}" = "latest" ]; then
if [ "$VCS_TYPE" = "github" ]; then
# SECURITY: Use github.api_url which is a trusted platform-provided value.
@@ -233,8 +241,9 @@ runs:
VERSION="${{ inputs.version }}"
fi
# Validate VERSION: no newlines, no slashes, no whitespace (防止 path traversal)
if printf '%s' "$VERSION" | grep -qE '[\n\r/[:space:]]'; then
# 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
@@ -262,6 +271,8 @@ runs:
if: steps.cache.outputs.cache-hit != 'true'
shell: bash
run: |
set -euo pipefail
SERVER_URL="${{ steps.version.outputs.server_url }}"
ACTION_REPO="${{ steps.version.outputs.action_repo }}"
VERSION="${{ steps.version.outputs.version }}"
@@ -276,6 +287,7 @@ runs:
# 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
@@ -365,7 +377,7 @@ else:
# authenticity — both binary and checksums come from the same server.
# For stronger guarantees, consider GPG signature verification.
cd "${{ runner.temp }}"
EXPECTED=$(grep "${BINARY}" checksums.txt | awk '{print $1}')
EXPECTED=$(grep -E "^[0-9a-f]+[[:space:]]+\*?${BINARY}$" checksums.txt | awk '{print $1}')
ACTUAL=$(sha256sum review-bot | awk '{print $1}')
if [ -z "$EXPECTED" ]; then