chore: triage review findings

2026-05-15 17:07:00 +00:00
parent fadcf9b14c
commit 9e0253f74d
3 changed files with 126 additions and 20 deletions
@@ -28,5 +28,41 @@
    "role": "Rex",
    "location": "entrypoint.sh:109",
    "suggestion": "將 NEW_VERSION 輸出到 GITHUB_OUTPUT 時，雖然目前 NEW_VERSION 的格式（如 `X.Y.Z` 或 `X.Y.Z-beta.N`）相對安全，不太可能包含換行符或其他特殊字元，但一般而言，應確保輸出到環境變數或檔案的內容不包含可能導致指令注入的特殊字元。對於複雜或多行內容，應使用 GitHub Actions 建議的多行輸出語法以確保安全。"
+  },
+  {
+    "level": "critical",
+    "role": "Leo",
+    "location": "entrypoint.sh:98-142",
+    "suggestion": "在 `calculate_version` 函式中，`jq` 腳本的邏輯過於複雜且龐大。重新實作了 `latest_stable_version`、`next_release_version`、`next_beta_number` 等邏輯。Bash 與 jq 的跨語言邏輯重複，增加維護難度與錯誤風險。建議：將 `calculate_version` 改寫成純 Bash。使用外部 `jq` 篩選器而非 inline jq。若必須使用 jq，建議拆分成多個函式並使用：`read -r -d '' JQ_SCRIPT << 'EOF'` 以提高可讀性。"
+  },
+  {
+    "level": "critical",
+    "role": "Leo",
+    "location": "tests/entrypoint_test.sh:50-109",
+    "suggestion": "在 `make_mock_jq` 函式中，為了測試 `entrypoint.sh`，於 Python 中重新實作核心版本計算邏輯。測試程式重複了正式程式的 jq 腳本。若 `entrypoint.sh` 的版本計算邏輯變更，Mock 也需同步更新，容易造成錯誤。建議：簡化 `entrypoint.sh` 的 jq 邏輯（如前一建議）。Mock jq 僅根據輸入回傳預定義輸出，而非重新計算邏輯。"
+  },
+  {
+    "level": "warning",
+    "role": "Zara",
+    "location": "entrypoint.sh:65, entrypoint.sh:90",
+    "suggestion": "腳本多次對完整的 `RELEASE_JSON` 字串執行 `jq` 命令來提取不同資訊。儘管 `RELEASE_JSON` 已載入記憶體，但每次 `jq` 呼叫都會啟動新程序並重新解析字串。對於擁有大量發行版本的儲存庫，這可能導致顯著的效能開銷。建議考慮將這些 `jq` 操作合併單次處理，以一次性提取所有所需資料（最新的穩定版本和首個出的新版本最高 beta 編號），從而減少 CPU 週期和程序啟動的開銷。"
+  },
+  {
+    "level": "warning",
+    "role": "Zara",
+    "location": "entrypoint.sh:58",
+    "suggestion": "腳本從 Gitea API 獲取所有發行版本。如果儲存庫中的發行版本數量非常龐大，這可能會消耗大量的網路頻寬和記憶體。儘管目前的邏輯可能需要檢查多個發行版本（例如，尋找最新的穩定版或特定的 beta 版本），但值得研究 Gitea API 是否提供更精細的過濾（例如，按標籤名稱模式或排除 beta 標籤）或分頁功能，以減少獲取的資料量，特別是當腳本只需要部分發行版本資訊時。"
+  },
+  {
+    "level": "warning",
+    "role": "Leo",
+    "location": "entrypoint.sh:159",
+    "suggestion": "在 `main` 函式中，`curl -fsS` 會抑制進度與錯誤細節。雖然 `set -e` 會在 curl 失敗時終止，但 stderr 的輸出資訊不足。建議在 `fail` 區塊中提供更明確錯誤內容，或包裝 `curl -fsS -H ... \"$RELEASE_URL\"` 以便記錄更具體的失敗原因。"
+  },
+  {
+    "level": "warning",
+    "role": "Leo",
+    "location": "entrypoint.sh:110",
+    "suggestion": "`calculate_version` 中的 jq 腳本使用魔術數字 `10` 用於 patch >= 10 判斷。建議提取為具名常數，或改用 Bash `readonly` 常數。"
  }
 ]
@@ -197,18 +197,10 @@ main() {
    VERSION_INFO="$(calculate_version "$RELEASE_JSON" "$IS_BETA")"
    IFS=$'\t' read -r LATEST_TAG NEW_VERSION <<< "$VERSION_INFO"

-    if [ -z "$LATEST_TAG" ] || [ "$LATEST_TAG" = "null" ]; then
-        LATEST_TAG="0.0.0"
-    fi
-
    info "LATEST_VERSION=$LATEST_TAG"

    section "計算版本號"

-    if [ -z "$NEW_VERSION" ] || [ "$NEW_VERSION" = "null" ]; then
-        NEW_VERSION="0.0.1"
-    fi
-
    info "NEW_VERSION=$NEW_VERSION"
    write_output "$NEW_VERSION"
 }
@@ -6,6 +6,16 @@ ROOT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"

 source "$ROOT_DIR/entrypoint.sh"

+declare -a CLEANUP_PATHS=()
+
+cleanup() {
+    if [ "${#CLEANUP_PATHS[@]}" -gt 0 ]; then
+        rm -rf "${CLEANUP_PATHS[@]}"
+    fi
+}
+
+trap cleanup EXIT
+
 fail() {
    printf '[error] %s\n' "$1" >&2
    exit 1
@@ -27,6 +37,10 @@ make_mock_curl() {

    cat >"$bin_dir/curl" <<'EOF'
 #!/bin/sh
+if [ -n "${FAKE_CURL_LOG_FILE:-}" ]; then
+    printf '%s\n' "$*" >> "$FAKE_CURL_LOG_FILE"
+fi
+
 if [ "${FAKE_CURL_STATUS:-0}" != "0" ]; then
    exit "$FAKE_CURL_STATUS"
 fi
@@ -135,6 +149,7 @@ run_entrypoint() {
    local stderr_file

    workdir="$(mktemp -d)"
+    CLEANUP_PATHS+=("$workdir")
    bin_dir="$workdir/bin"
    mkdir -p "$bin_dir"
    make_mock_curl "$bin_dir" "$response_file"
@@ -147,6 +162,7 @@ run_entrypoint() {
    if [ -n "$token" ]; then
        FAKE_CURL_STATUS="$fake_status" \
        FAKE_CURL_RESPONSE_FILE="$response_file" \
+        FAKE_CURL_LOG_FILE="$workdir/curl_args" \
        PATH="$bin_dir:$PATH" \
        GITEA_SERVER_URL="https://gitea.example.com" \
        GITEA_REPOSITORY="org/repo" \
@@ -157,6 +173,7 @@ run_entrypoint() {
    else
        FAKE_CURL_STATUS="$fake_status" \
        FAKE_CURL_RESPONSE_FILE="$response_file" \
+        FAKE_CURL_LOG_FILE="$workdir/curl_args" \
        PATH="$bin_dir:$PATH" \
        GITEA_SERVER_URL="https://gitea.example.com" \
        GITEA_REPOSITORY="org/repo" \
@@ -174,13 +191,17 @@ test_unit_helpers() {
    assert_eq "true" "$(normalize_beta_flag "true")" "normalize_beta_flag true"
    assert_eq "0.1.0" "$(next_release_version "0.0.9")" "next_release_version carry patch"
    assert_eq "2.0.0" "$(next_release_version "1.9.9")" "next_release_version carry minor"
+    assert_eq "100.0.0" "$(next_release_version "99.9.9")" "next_release_version carry major"
 }

 test_stable_release_flow() {
+    local workdir
    local response_file
    local output_file

-    response_file="$(mktemp)"
+    workdir="$(mktemp -d)"
+    CLEANUP_PATHS+=("$workdir")
+    response_file="$workdir/releases.json"
    cat >"$response_file" <<'EOF'
 [
  {"tag_name":"v1.2.3-beta.1"},
@@ -194,10 +215,13 @@ EOF
 }

 test_beta_release_flow() {
+    local workdir
    local response_file
    local output_file

-    response_file="$(mktemp)"
+    workdir="$(mktemp -d)"
+    CLEANUP_PATHS+=("$workdir")
+    response_file="$workdir/releases.json"
    cat >"$response_file" <<'EOF'
 [
  {"tag_name":"v1.2.3"},
@@ -211,10 +235,13 @@ EOF
 }

 test_empty_release_list() {
+    local workdir
    local response_file
    local output_file

-    response_file="$(mktemp)"
+    workdir="$(mktemp -d)"
+    CLEANUP_PATHS+=("$workdir")
+    response_file="$workdir/releases.json"
    printf '%s\n' '[]' >"$response_file"

    output_file="$(run_entrypoint "$response_file" "false")"
@@ -222,10 +249,13 @@ test_empty_release_list() {
 }

 test_only_beta_releases() {
+    local workdir
    local response_file
    local output_file

-    response_file="$(mktemp)"
+    workdir="$(mktemp -d)"
+    CLEANUP_PATHS+=("$workdir")
+    response_file="$workdir/releases.json"
    cat >"$response_file" <<'EOF'
 [
  {"tag_name":"v2.4.6-beta.1"},
@@ -238,27 +268,73 @@ EOF
 }

 test_null_release_payload() {
+    local workdir
    local response_file
    local output_file

-    response_file="$(mktemp)"
+    workdir="$(mktemp -d)"
+    CLEANUP_PATHS+=("$workdir")
+    response_file="$workdir/releases.json"
    printf '%s\n' 'null' >"$response_file"

    output_file="$(run_entrypoint "$response_file" "false")"
    assert_eq "version=0.0.1" "$(cat "$output_file")" "null release payload"
 }

-test_malformed_release_payload() {
-    local response_file
+test_token_auth_header() {
    local workdir
+    local response_file
+    local bin_dir
+    local output_file
+    local curl_args
+    local stdout_file
+    local stderr_file
+
+    workdir="$(mktemp -d)"
+    CLEANUP_PATHS+=("$workdir")
+    response_file="$workdir/releases.json"
+    printf '%s\n' '[]' >"$response_file"
+
+    bin_dir="$workdir/bin"
+    mkdir -p "$bin_dir"
+    make_mock_curl "$bin_dir" "$response_file"
+    make_mock_jq "$bin_dir"
+
+    output_file="$workdir/github_output"
+    stdout_file="$workdir/stdout"
+    stderr_file="$workdir/stderr"
+
+    FAKE_CURL_STATUS=0 \
+    FAKE_CURL_RESPONSE_FILE="$response_file" \
+    FAKE_CURL_LOG_FILE="$workdir/curl_args" \
+    PATH="$bin_dir:$PATH" \
+    GITEA_SERVER_URL="https://gitea.example.com" \
+    GITEA_REPOSITORY="org/repo" \
+    RUNNER_TOKEN="secret-token" \
+    IS_BETA="false" \
+    GITHUB_OUTPUT="$output_file" \
+    bash "$ROOT_DIR/entrypoint.sh" >"$stdout_file" 2>"$stderr_file"
+
+    curl_args="$workdir/curl_args"
+    if ! grep -q 'Authorization: token secret-token' "$curl_args"; then
+        fail "token auth header: missing Authorization header"
+    fi
+
+    assert_eq "version=0.0.1" "$(cat "$output_file")" "token auth output"
+}
+
+test_malformed_release_payload() {
+    local workdir
+    local response_file
    local bin_dir
    local output_file
    local stdout_file
    local stderr_file

-    response_file="$(mktemp)"
-    printf '%s\n' '{' >"$response_file"
    workdir="$(mktemp -d)"
+    CLEANUP_PATHS+=("$workdir")
+    response_file="$workdir/releases.json"
+    printf '%s\n' '{' >"$response_file"
    bin_dir="$workdir/bin"
    mkdir -p "$bin_dir"
    make_mock_curl "$bin_dir" "$response_file"
@@ -280,16 +356,17 @@ test_malformed_release_payload() {
 }

 test_curl_failure() {
-    local response_file
    local workdir
+    local response_file
    local bin_dir
    local output_file
    local stdout_file
    local stderr_file

-    response_file="$(mktemp)"
-    printf '%s\n' '[]' >"$response_file"
    workdir="$(mktemp -d)"
+    CLEANUP_PATHS+=("$workdir")
+    response_file="$workdir/releases.json"
+    printf '%s\n' '[]' >"$response_file"
    bin_dir="$workdir/bin"
    mkdir -p "$bin_dir"
    make_mock_curl "$bin_dir" "$response_file"
@@ -315,6 +392,7 @@ test_beta_release_flow
 test_empty_release_list
 test_only_beta_releases
 test_null_release_payload
+test_token_auth_header
 test_malformed_release_payload
 test_curl_failure