fix(ci): use Gitea file API to push CHANGELOG.md #40

Merged

sarman merged 2 commits from fix/changelog-push into master 2026-04-13 03:18:11 +00:00

Conversation 0 Commits 2 Files Changed 1 +37 -8

sarman commented 2026-04-13 03:07:04 +00:00

Owner

Copy Link

Problem

The changelog job was failing at Commit CHANGELOG.md to master with a non-fast-forward rejection. This is a race condition: git push origin HEAD:master fails whenever master advances between the jobs fetch and its push.

Fix

Replace git commit + git push with PUT /repos/.../contents/CHANGELOG.md via the Gitea file API. This atomically updates the file on master regardless of where local HEAD is — no fast-forward required.

The step now:

1. Fetches the current file blob SHA from master (required by API when file already exists)
2. Base64-encodes the generated CHANGELOG.md
3. PUTs to the Gitea contents API with branch: master

Test plan

• Merge triggers auto-tag.yml; Commit CHANGELOG.md to master step passes
• CHANGELOG.md updated in master git log
• CHANGELOG.md uploaded as release asset

## Problem The changelog job was failing at `Commit CHANGELOG.md to master` with a non-fast-forward rejection. This is a race condition: `git push origin HEAD:master` fails whenever master advances between the jobs fetch and its push. ## Fix Replace `git commit + git push` with `PUT /repos/.../contents/CHANGELOG.md` via the Gitea file API. This atomically updates the file on master regardless of where local HEAD is — no fast-forward required. The step now: 1. Fetches the current file blob SHA from master (required by API when file already exists) 2. Base64-encodes the generated CHANGELOG.md 3. PUTs to the Gitea contents API with `branch: master` ## Test plan - [ ] Merge triggers auto-tag.yml; Commit CHANGELOG.md to master step passes - [ ] CHANGELOG.md updated in master git log - [ ] CHANGELOG.md uploaded as release asset

sarman added 1 commit 2026-04-13 03:07:05 +00:00

fix(ci): use Gitea file API to push CHANGELOG.md — eliminates non-fast-forward rejection ... 2da529fb75

git push origin HEAD:master fails when master advances between the job's
fetch and its push. Replace with PUT /repos/.../contents/CHANGELOG.md
which atomically updates the file on master regardless of HEAD position.

sarman reviewed 2026-04-13 03:10:03 +00:00

sarman left a comment

Author

Owner

Copy Link

🤖 Automated PR Review:\n\n### 1) Summary
The PR replaces a Git CLI-based workflow (staging, committing, and pushing CHANGELOG.md via git add/commit/push) with a direct call to the Gitea REST API to update the file. This avoids local Git state management and potential merge conflicts, and aligns with CI best practices for declarative file updates.

However, the implementation contains critical bugs, security vulnerabilities, and best-practice violations that must be addressed before merging.

---

2) Bugs / Errors

❌ Missing closing quote and backslash in curl command

CURRENT_SHA=$(curl -sf "$API/contents/CHANGELOG.md?ref=master" \

→ Missing ) — the command substitution is unclosed. This will cause a syntax error or hang the job.

✅ Fix:

CURRENT_SHA=$(curl -sf "$API/contents/CHANGELOG.md?ref=master" | jq -r '.sha')

(Also: the API returns JSON — you need to extract .sha, not use raw output.)

❌ No fallback for new files (SHA missing)

If CHANGELOG.md doesn’t exist yet (e.g., first release), curl returns 404, and CURRENT_SHA becomes empty or invalid. The subsequent PUT with "sha": "" will fail.

✅ Fix: Handle the 404 case:

CURRENT_SHA=$(curl -sf "$API/contents/CHANGELOG.md?ref=master" 2>/dev/null | jq -r '.sha // empty')
# If file doesn't exist, omit `sha` (API allows omitting it for new files)
if [ -z "$CURRENT_SHA" ]; then
  CURRENT_SHA="null"  # placeholder for conditional logic below
fi

Then conditionally include "sha" in the JSON payload only if the file exists.

❌ No error handling for API calls

The curl commands use -sf (silent fail), but if the API returns a non-2xx (e.g., 401, 403, 422, 409), the script continues, potentially leaving the repo in an inconsistent state.

✅ Fix: Check HTTP status explicitly:

RESP=$(curl -s -w "\n%{http_code}" "$API/contents/CHANGELOG.md?ref=master" -H "Authorization: token $GITHUB_TOKEN")
HTTP_CODE=$(echo "$RESP" | tail -n1)
BODY=$(echo "$RESP" | sed '$d')

if [ "$HTTP_CODE" != "200" ]; then
  echo "Failed to fetch file SHA: HTTP $HTTP_CODE"
  echo "$BODY" >&2
  exit 1
fi
CURRENT_SHA=$(echo "$BODY" | jq -r '.sha')

❌ Hardcoded internal IP (172.0.0.29) and port (3000)

This assumes the Gitea instance is always reachable at that exact address/port — fragile and non-portable across environments (e.g., production, different runners, cloud).

✅ Fix: Use environment variables or a configurable base URL:

env:
  GITEA_API_URL: ${{ vars.GITEA_API_URL || 'http://gitea.default.svc:3000' }}

Then:

API="$GITEA_API_URL/api/v1/repos/$GITHUB_REPOSITORY"

💡 Note: GITHUB_REPOSITORY is GitHub-specific; in Gitea workflows, use GITEA_REPO or GITEA_REPOSITORY if available. If this workflow runs in Gitea, prefer GITEA_REPO (or derive from GITHUB_REPOSITORY only if GitHub Actions compatibility is needed).

---

3) Security Issues

⚠️ Exposing RELEASE_TOKEN in logs

The token is passed via env:, but the curl command and JSON payload are printed in logs if set -x is enabled (or if the workflow runner logs the full script). Worse, if curl fails, the full request (including token in Authorization header) may leak.

✅ Fix:

• Use -H "Authorization: token $RELEASE_TOKEN" explicitly and ensure no debug logging.
• Prefer GITHUB_TOKEN if it has write permissions (Gitea supports GitHub-style tokens), but verify scope.
• Sanitize logs: avoid echoing sensitive data.

⚠️ No input sanitization for TAG

TAG=$(git describe --tags --abbrev=0) could return malicious strings (e.g., v1.0.0"; rm -rf /). Used in commit message and API request — potential injection.

✅ Fix: Validate TAG against a safe regex:

TAG=$(git describe --tags --abbrev=0 2>/dev/null) || { echo "No tags found"; exit 1; }
if ! [[ "$TAG" =~ ^v?[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9._-]+)?$ ]]; then
  echo "Invalid tag format: $TAG"
  exit 1
fi

⚠️ No rate limiting or conflict retry

Concurrent CI runs could cause 409 Conflict (if CURRENT_SHA is stale). The current script fails on conflict.

✅ Fix: Implement retry logic with exponential backoff for 409 (or use HEAD to get latest SHA before commit).

---

4) Best Practices

✅ Good: Avoiding git push in CI

Using the API is more reliable than git push (avoids SSH key setup, merge conflicts, and git config requirements).

❌ Missing: Content-Type: application/json in GET request

The curl for fetching SHA doesn’t specify Accept: application/json, though Gitea defaults to JSON. Still, be explicit.

✅ Fix:

curl -sfH "Accept: application/json" "$API/contents/CHANGELOG.md?ref=master"

❌ No handling of base64 encoding edge cases

base64 -w 0 is correct, but ensure CHANGELOG.md is UTF-8 and not binary. Gitea expects base64-encoded content.

✅ Fix: Add encoding check:

if ! file -b --mime-encoding CHANGELOG.md | grep -q '^utf-8$'; then
  echo "CHANGELOG.md must be UTF-8 encoded"
  exit 1
fi

❌ Hardcoded master branch

Assumes master exists (not main). Modern repos often use main.

✅ Fix: Use a configurable branch (e.g., GITEA_DEFAULT_BRANCH):

env:
  TARGET_BRANCH: ${{ vars.GITEA_DEFAULT_BRANCH || 'master' }}

Then:

API="$API/contents/CHANGELOG.md?ref=$TARGET_BRANCH"
...
"branch": "$TARGET_BRANCH"

❌ No cleanup of temp files

CHANGELOG.md is read directly — safe if it’s in the repo, but ensure no sensitive data leaks.

✅ Good: set -eu

Prevents silent failures.

---

🔧 Suggested Complete Fix (Key Snippet)

run: |
  set -euo pipefail
  API="https://gitea.example.com/api/v1/repos/$GITHUB_REPOSITORY"
  TARGET_BRANCH="${TARGET_BRANCH:-main}"
  TOKEN="${RELEASE_TOKEN:-$GITHUB_TOKEN}"

  TAG=$(git describe --tags --abbrev=0)
  if ! [[ "$TAG" =~ ^v?[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9._-]+)?$ ]]; then
    echo "Invalid tag: $TAG"
    exit 1
  fi

  # Fetch current SHA (handle 404 for new files)
  RESPONSE=$(curl -sf -H "Authorization: token $TOKEN" \
                   -H "Accept: application/json" \
                   "$API/contents/CHANGELOG.md?ref=$TARGET_BRANCH" 2>&1) || {
    HTTP_CODE=$(echo "$RESPONSE" | grep -oP '(?<=HTTP/)[0-9]+')
    if [[ "$HTTP_CODE" == "404" ]]; then
      CURRENT_SHA=""
    else
      echo "Failed to fetch file: $RESPONSE"
      exit 1
    fi
  }

  if [ -n "$RESPONSE" ]; then
    CURRENT_SHA=$(echo "$RESPONSE" | jq -r '.sha // empty')
  fi

  CONTENT=$(base64 -w 0 CHANGELOG.md)

  # Build JSON payload (omit `sha` for new files)
  if [ -z "$CURRENT_SHA" ]; then
    PAYLOAD="{\"message\": \"chore: update CHANGELOG.md for $TAG [skip ci]\", \"content\": \"$CONTENT\", \"branch\": \"$TARGET_BRANCH\"}"
  else
    PAYLOAD="{\"message\": \"chore: update CHANGELOG.md for $TAG [skip ci]\", \"content\": \"$CONTENT\", \"sha\": \"$CURRENT_SHA\", \"branch\": \"$TARGET_BRANCH\"}"
  fi

  RESULT=$(curl -sf -X PUT -H "Authorization: token $TOKEN" \
                  -H "Content-Type: application/json" \
                  -d "$PAYLOAD" \
                  "$API/contents/CHANGELOG.md" 2>&1) || {
    echo "Failed to update file: $RESULT"
    exit 1
  }

  echo "✓ CHANGELOG.md committed to $TARGET_BRANCH"  

---

Final Recommendation

Do not merge as-is. Fix all critical bugs (especially the unclosed command substitution and missing SHA extraction) and security issues (token exposure, input validation) before merging. Consider adding tests (e.g., mock API responses) and documenting assumptions (e.g., GITHUB_REPOSITORY compatibility).\n\n---\nthis is an automated review from Ollama

🤖 Automated PR Review:\n\n### 1) Summary The PR replaces a Git CLI-based workflow (staging, committing, and pushing `CHANGELOG.md` via `git add/commit/push`) with a direct call to the Gitea REST API to update the file. This avoids local Git state management and potential merge conflicts, and aligns with CI best practices for declarative file updates. However, the implementation contains **critical bugs**, **security vulnerabilities**, and **best-practice violations** that must be addressed before merging. --- ### 2) Bugs / Errors #### ❌ **Missing closing quote and backslash in `curl` command** ```bash CURRENT_SHA=$(curl -sf "$API/contents/CHANGELOG.md?ref=master" \ ``` → **Missing `)`** — the command substitution is unclosed. This will cause a syntax error or hang the job. ✅ **Fix:** ```bash CURRENT_SHA=$(curl -sf "$API/contents/CHANGELOG.md?ref=master" | jq -r '.sha') ``` *(Also: the API returns JSON — you need to extract `.sha`, not use raw output.)* #### ❌ **No fallback for new files (SHA missing)** If `CHANGELOG.md` doesn’t exist yet (e.g., first release), `curl` returns `404`, and `CURRENT_SHA` becomes empty or invalid. The subsequent `PUT` with `"sha": ""` will fail. ✅ **Fix:** Handle the `404` case: ```bash CURRENT_SHA=$(curl -sf "$API/contents/CHANGELOG.md?ref=master" 2>/dev/null | jq -r '.sha // empty') # If file doesn't exist, omit `sha` (API allows omitting it for new files) if [ -z "$CURRENT_SHA" ]; then CURRENT_SHA="null" # placeholder for conditional logic below fi ``` Then conditionally include `"sha"` in the JSON payload only if the file exists. #### ❌ **No error handling for API calls** The `curl` commands use `-sf` (silent fail), but if the API returns a non-2xx (e.g., `401`, `403`, `422`, `409`), the script continues, potentially leaving the repo in an inconsistent state. ✅ **Fix:** Check HTTP status explicitly: ```bash RESP=$(curl -s -w "\n%{http_code}" "$API/contents/CHANGELOG.md?ref=master" -H "Authorization: token $GITHUB_TOKEN") HTTP_CODE=$(echo "$RESP" | tail -n1) BODY=$(echo "$RESP" | sed '$d') if [ "$HTTP_CODE" != "200" ]; then echo "Failed to fetch file SHA: HTTP $HTTP_CODE" echo "$BODY" >&2 exit 1 fi CURRENT_SHA=$(echo "$BODY" | jq -r '.sha') ``` #### ❌ **Hardcoded internal IP (`172.0.0.29`) and port (`3000`)** This assumes the Gitea instance is always reachable at that exact address/port — fragile and non-portable across environments (e.g., production, different runners, cloud). ✅ **Fix:** Use environment variables or a configurable base URL: ```yaml env: GITEA_API_URL: ${{ vars.GITEA_API_URL || 'http://gitea.default.svc:3000' }} ``` Then: ```bash API="$GITEA_API_URL/api/v1/repos/$GITHUB_REPOSITORY" ``` > 💡 Note: `GITHUB_REPOSITORY` is GitHub-specific; in Gitea workflows, use `GITEA_REPO` or `GITEA_REPOSITORY` if available. If this workflow runs in Gitea, prefer `GITEA_REPO` (or derive from `GITHUB_REPOSITORY` only if GitHub Actions compatibility is needed). --- ### 3) Security Issues #### ⚠️ **Exposing `RELEASE_TOKEN` in logs** The token is passed via `env:`, but the `curl` command and JSON payload are printed in logs if `set -x` is enabled (or if the workflow runner logs the full script). Worse, if `curl` fails, the full request (including token in `Authorization` header) may leak. ✅ **Fix:** - Use `-H "Authorization: token $RELEASE_TOKEN"` explicitly and ensure no debug logging. - Prefer `GITHUB_TOKEN` if it has write permissions (Gitea supports GitHub-style tokens), but verify scope. - Sanitize logs: avoid echoing sensitive data. #### ⚠️ **No input sanitization for `TAG`** `TAG=$(git describe --tags --abbrev=0)` could return malicious strings (e.g., `v1.0.0"; rm -rf /`). Used in commit message and API request — potential injection. ✅ **Fix:** Validate `TAG` against a safe regex: ```bash TAG=$(git describe --tags --abbrev=0 2>/dev/null) || { echo "No tags found"; exit 1; } if ! [[ "$TAG" =~ ^v?[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9._-]+)?$ ]]; then echo "Invalid tag format: $TAG" exit 1 fi ``` #### ⚠️ **No rate limiting or conflict retry** Concurrent CI runs could cause `409 Conflict` (if `CURRENT_SHA` is stale). The current script fails on conflict. ✅ **Fix:** Implement retry logic with exponential backoff for `409` (or use `HEAD` to get latest SHA before commit). --- ### 4) Best Practices #### ✅ **Good: Avoiding `git push` in CI** Using the API is more reliable than `git push` (avoids SSH key setup, merge conflicts, and `git config` requirements). #### ❌ **Missing: `Content-Type: application/json` in GET request** The `curl` for fetching SHA doesn’t specify `Accept: application/json`, though Gitea defaults to JSON. Still, be explicit. ✅ **Fix:** ```bash curl -sfH "Accept: application/json" "$API/contents/CHANGELOG.md?ref=master" ``` #### ❌ **No handling of `base64` encoding edge cases** `base64 -w 0` is correct, but ensure `CHANGELOG.md` is UTF-8 and not binary. Gitea expects base64-encoded content. ✅ **Fix:** Add encoding check: ```bash if ! file -b --mime-encoding CHANGELOG.md | grep -q '^utf-8$'; then echo "CHANGELOG.md must be UTF-8 encoded" exit 1 fi ``` #### ❌ **Hardcoded `master` branch** Assumes `master` exists (not `main`). Modern repos often use `main`. ✅ **Fix:** Use a configurable branch (e.g., `GITEA_DEFAULT_BRANCH`): ```yaml env: TARGET_BRANCH: ${{ vars.GITEA_DEFAULT_BRANCH || 'master' }} ``` Then: ```bash API="$API/contents/CHANGELOG.md?ref=$TARGET_BRANCH" ... "branch": "$TARGET_BRANCH" ``` #### ❌ **No cleanup of temp files** `CHANGELOG.md` is read directly — safe if it’s in the repo, but ensure no sensitive data leaks. #### ✅ **Good: `set -eu`** Prevents silent failures. --- ### 🔧 Suggested Complete Fix (Key Snippet) ```yaml run: | set -euo pipefail API="https://gitea.example.com/api/v1/repos/$GITHUB_REPOSITORY" TARGET_BRANCH="${TARGET_BRANCH:-main}" TOKEN="${RELEASE_TOKEN:-$GITHUB_TOKEN}" TAG=$(git describe --tags --abbrev=0) if ! [[ "$TAG" =~ ^v?[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9._-]+)?$ ]]; then echo "Invalid tag: $TAG" exit 1 fi # Fetch current SHA (handle 404 for new files) RESPONSE=$(curl -sf -H "Authorization: token $TOKEN" \ -H "Accept: application/json" \ "$API/contents/CHANGELOG.md?ref=$TARGET_BRANCH" 2>&1) || { HTTP_CODE=$(echo "$RESPONSE" | grep -oP '(?<=HTTP/)[0-9]+') if [[ "$HTTP_CODE" == "404" ]]; then CURRENT_SHA="" else echo "Failed to fetch file: $RESPONSE" exit 1 fi } if [ -n "$RESPONSE" ]; then CURRENT_SHA=$(echo "$RESPONSE" | jq -r '.sha // empty') fi CONTENT=$(base64 -w 0 CHANGELOG.md) # Build JSON payload (omit `sha` for new files) if [ -z "$CURRENT_SHA" ]; then PAYLOAD="{\"message\": \"chore: update CHANGELOG.md for $TAG [skip ci]\", \"content\": \"$CONTENT\", \"branch\": \"$TARGET_BRANCH\"}" else PAYLOAD="{\"message\": \"chore: update CHANGELOG.md for $TAG [skip ci]\", \"content\": \"$CONTENT\", \"sha\": \"$CURRENT_SHA\", \"branch\": \"$TARGET_BRANCH\"}" fi RESULT=$(curl -sf -X PUT -H "Authorization: token $TOKEN" \ -H "Content-Type: application/json" \ -d "$PAYLOAD" \ "$API/contents/CHANGELOG.md" 2>&1) || { echo "Failed to update file: $RESULT" exit 1 } echo "✓ CHANGELOG.md committed to $TARGET_BRANCH" ``` --- ### Final Recommendation **Do not merge as-is.** Fix all critical bugs (especially the unclosed command substitution and missing SHA extraction) and security issues (token exposure, input validation) before merging. Consider adding tests (e.g., mock API responses) and documenting assumptions (e.g., `GITHUB_REPOSITORY` compatibility).\n\n---\n*this is an automated review from Ollama*

sarman added 1 commit 2026-04-13 03:13:33 +00:00

fix(ci): harden CHANGELOG.md API push step per review ... f74238a65a

- set -euo pipefail (was -eu; pipefail catches silent pipe failures)
- Validate TAG against ^v[0-9]+\.[0-9]+\.[0-9]+$ before use in commit
  message and JSON payload — prevents shell injection
- Tolerate 404 on SHA fetch (new file): curl 2>/dev/null or true keeps
  CURRENT_SHA empty rather than causing jq to abort
- Use jq -n to build JSON payload — conditionally omits sha field when
  file does not exist yet; eliminates manual string escaping
- Check HTTP status of PUT; print response body and exit 1 on non-2xx
- Add Accept: application/json header to SHA fetch request

sarman reviewed 2026-04-13 03:17:25 +00:00

sarman left a comment

Author

Owner

Copy Link

🤖 Automated PR Review:\n\n### Code Review: .gitea/workflows/auto-tag.yml — Using Gitea File API to Push CHANGELOG.md

---

1. Summary

This change replaces a git add/commit/push workflow (which requires local Git state and remote branch tracking) with a direct Gitea REST API call to atomically update CHANGELOG.md on the master branch. This is a solid improvement for CI reliability, avoids merge conflicts, and removes dependency on Git credentials/SSH keys in the runner.

However, the implementation introduces critical security and correctness issues related to:

• Shell injection (despite the tag validation, the JSON payload construction is unsafe),
• Missing input sanitization (file content not validated),
• Brittle API assumptions (hardcoded IP, missing auth header),
• Insecure temp file handling.

---

2. Bugs & Correctness Issues

2.1 Hardcoded API endpoint (http://172.0.0.29:3000)

• ❌ Critical: Hardcoded internal IP assumes Gitea is always at that address — will break in other environments (e.g., GitHub Actions, multi-runner setups, or non-local deployments).
• ✅ Fix: Use ${GITEA_SERVER_URL:-http://localhost:3000} or derive from environment (e.g., GITHUB_SERVER_URL is not Gitea-compatible, but Gitea CI sets GITEA_SERVER_URL). If this workflow runs inside the same Docker network (e.g., Gitea self-hosted), document the requirement and use a config variable with fallback.

2.2 Missing Authorization Header

• ❌ Critical: The curl calls lack -H "Authorization: token $RELEASE_TOKEN". Without this, the API request will return 404 (for private repos) or 401, causing silent failure.
• ✅ Fix:

  -H "Authorization: token $RELEASE_TOKEN"
  

2.3 Unsafe JSON generation with jq + untrusted $TAG

• ❌ High: Although $TAG is validated with grep, the validation is insufficient and the JSON construction is still vulnerable:

  --arg msg "chore: update CHANGELOG.md for ${TAG} [skip ci]"
  

  jq --arg does escape strings safely for JSON, so the JSON itself is safe.
  BUT: the real issue is that the tag validation regex ^v[0-9]+\.[0-9]+\.[0-9]+$ is too permissive:

  • It allows leading zeros (e.g., v1.02.03) → may cause issues downstream (e.g., semantic versioning tools reject this).
  • It doesn’t validate 0 ≤ major ≤ 2^31-1, etc. — but more critically, it doesn’t reject empty or malformed tags early enough.

  However, the bigger correctness bug is:

  • jq -r '.sha // empty' returns "null" if .sha is missing, but // empty converts it to empty string — good.
  • But jq -r may output null without quotes if no match → CURRENT_SHA="null" → PAYLOAD includes "sha": "null" (a string "null", not JSON null) → Gitea API will reject it.

  ✅ Fix:

  CURRENT_SHA=$(curl -sf ... | jq -r '.sha // empty' 2>/dev/null || true)
  # Ensure it's either a non-empty string or truly empty (not "null")
  if [ "$CURRENT_SHA" = "null" ]; then CURRENT_SHA=""; fi
  

2.4 base64 -w 0 may not be portable

• ⚠️ Medium: -w 0 is GNU base64-specific. On macOS/BSD, use -b 0 (or base64 -w 0 may fail silently or wrap).
• ✅ Fix: Use base64 -w0 only if GNU base64 is detected, or use portable alternative:

  CONTENT=$(base64 -e | tr -d '\n') < CHANGELOG.md
  # or
  CONTENT=$(cat CHANGELOG.md | base64 | tr -d '\n')
  

2.5 Temp file not cleaned up

• ❌ Medium: RESP_FILE=$(mktemp) but never rm "$RESP_FILE". In CI, this leaks temp files (though minor, it’s bad practice).
• ✅ Fix:

  RESP_FILE=$(mktemp)
  trap 'rm -f "$RESP_FILE"' EXIT
  

---

3. Security Issues

3.1 Shell injection via $TAG in echo (low risk but avoidable)

• ❌ Low-Medium: echo "$TAG" is safe if $TAG is validated, but the validation regex is insufficient:

  • v1.2.3 is OK, but v1.2.3-rc1 is rejected (false positive for pre-release tags).
  • Worse: v1.2.3; rm -rf / would be rejected by the regex — but only if the regex is applied before shell expansion. Since it’s echo "$TAG" | grep, and echo doesn’t interpret shell metacharacters, it’s safe — but the real risk is downstream if $TAG is used in other contexts (e.g., git tag "$TAG" later?).

  However, the current code only uses $TAG in:

  • echo "ERROR: ... $TAG" → safe (quoted)
  • jq --arg msg → safe (jq --arg escapes)

  So no injection here, but the regex is too strict for real-world SemVer.

• ✅ Fix: Use stricter validation and support pre-release tags:

  if ! echo "$TAG" | grep -qE '^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?(\+[0-9A-Za-z.-]+)?$'; then
    echo "ERROR: Invalid tag format (expected SemVer): $TAG"
    exit 1
  fi
  

3.2 No validation of CHANGELOG.md content

• ❌ High: base64 -w 0 CHANGELOG.md uploads any content — including malicious payloads (e.g., if CHANGELOG.md is tampered with by a previous step or attacker).

• ✅ Fix: Validate content before upload:

  • Check file size (e.g., < 1MB)
  • Ensure it’s UTF-8 (or at least ASCII-safe)
  • Reject if it contains suspicious patterns (e.g., </script>, javascript:, etc.) — though less critical for Markdown.

  Example:

  if [ ! -s CHANGELOG.md ]; then
    echo "ERROR: CHANGELOG.md is empty or missing"
    exit 1
  fi
  # Optional: basic UTF-8 check
  if ! file --mime-encoding CHANGELOG.md | grep -q 'utf-8\|us-ascii'; then
    echo "ERROR: CHANGELOG.md is not UTF-8/ASCII"
    exit 1
  fi
  

3.3 curl without --fail

• ⚠️ Medium: curl -s does not fail on HTTP error codes (e.g., 4xx/5xx). You check $HTTP_CODE, but curl -s may still output error HTML to stdout (which gets piped to jq → jq fails with parse error).
• ✅ Fix: Add -f (fail silently on HTTP errors) or keep HTTP_CODE check (as done), but ensure curl doesn’t output to stdout/stderr in success case (already done with -s).

---

4. Best Practices

4.1 Use environment variables for config

• ✅ Recommendation: Move API and RELEASE_TOKEN to environment variables (already done for RELEASE_TOKEN, but API is hardcoded).

  env:
    GITEA_API_URL: "http://gitea:3000/api/v1/repos/${GITHUB_REPOSITORY}"
    # or better: use GITHUB_REPOSITORY and GITHUB_SERVER_URL to derive
  

4.2 Idempotency & atomicity

• ✅ Good: Using PUT /contents/{path} is atomic and handles create/update in one call — much better than git push.

4.3 Logging & debugging

• ✅ Good: Printing HTTP_CODE and response body on error is helpful.
• ⚠️ Improve: Log TAG (sanitized) for traceability:

  echo "Processing tag: ${TAG:0:12}..."
  

4.4 Error handling with set -euo pipefail

• ✅ Good: set -euo pipefail is correctly used.

4.5 jq version compatibility

• ⚠️ Low: jq -n --arg ... 'if ... then ... else ... end' is fine, but ensure jq >= 1.6 (older versions may have issues with // empty in conditionals). Consider using jq --argjson for booleans, but here it’s not needed.

---

✅ Recommended Fixes Summary

Issue	Fix
Hardcoded IP	Use ${GITEA_API_URL:-http://gitea:3000/api/v1/repos/$GITHUB_REPOSITORY} or derive from CI env
Missing auth header	Add -H "Authorization: token $RELEASE_TOKEN"
CURRENT_SHA = "null"	Filter: CURRENT_SHA="${CURRENT_SHA:-}"; [ "$CURRENT_SHA" = "null" ] && CURRENT_SHA=""
base64 portability	Use tr -d '\n' after base64
Temp file leak	trap 'rm -f "$RESP_FILE"' EXIT
Weak tag validation	Use full SemVer regex
No CHANGELOG.md validation	Check size, encoding, non-empty
(Optional) Pre-release tags	Support -rc1, +build, etc. in tag regex

---

Final Verdict

• Security: ❌ High-risk due to missing auth and unvalidated content.
• Correctness: ⚠️ Moderate-risk due to API assumptions and edge cases.
• Best practices: ✅ Mostly good, but could be improved.

Do not merge as-is. Fix the auth header and tag validation first — those are blocking issues.

Let me know if you'd like a revised diff with all fixes applied!\n\n---\nthis is an automated review from Ollama

🤖 Automated PR Review:\n\n### Code Review: `.gitea/workflows/auto-tag.yml` — Using Gitea File API to Push `CHANGELOG.md` --- #### **1. Summary** This change replaces a `git add/commit/push` workflow (which requires local Git state and remote branch tracking) with a direct **Gitea REST API call** to atomically update `CHANGELOG.md` on the `master` branch. This is a solid improvement for CI reliability, avoids merge conflicts, and removes dependency on Git credentials/SSH keys in the runner. However, the implementation introduces **critical security and correctness issues** related to: - **Shell injection** (despite the tag validation, the JSON payload construction is unsafe), - **Missing input sanitization** (file content not validated), - **Brittle API assumptions** (hardcoded IP, missing auth header), - **Insecure temp file handling**. --- #### **2. Bugs & Correctness Issues** ##### **2.1 Hardcoded API endpoint (`http://172.0.0.29:3000`)** - ❌ **Critical**: Hardcoded internal IP assumes Gitea is always at that address — will break in other environments (e.g., GitHub Actions, multi-runner setups, or non-local deployments). - ✅ **Fix**: Use `${GITEA_SERVER_URL:-http://localhost:3000}` or derive from environment (e.g., `GITHUB_SERVER_URL` is *not* Gitea-compatible, but Gitea CI sets `GITEA_SERVER_URL`). If this workflow runs *inside* the same Docker network (e.g., Gitea self-hosted), document the requirement and use a config variable with fallback. ##### **2.2 Missing Authorization Header** - ❌ **Critical**: The `curl` calls lack `-H "Authorization: token $RELEASE_TOKEN"`. Without this, the API request will return `404` (for private repos) or `401`, causing silent failure. - ✅ **Fix**: ```bash -H "Authorization: token $RELEASE_TOKEN" ``` ##### **2.3 Unsafe JSON generation with `jq` + untrusted `$TAG`** - ❌ **High**: Although `$TAG` is validated with `grep`, the *validation is insufficient* and the JSON construction is still vulnerable: ```bash --arg msg "chore: update CHANGELOG.md for ${TAG} [skip ci]" ``` `jq --arg` *does* escape strings safely **for JSON**, so the JSON itself is safe. **BUT**: the *real* issue is that the tag validation regex `^v[0-9]+\.[0-9]+\.[0-9]+$` is too permissive: - It allows leading zeros (e.g., `v1.02.03`) → may cause issues downstream (e.g., semantic versioning tools reject this). - It doesn’t validate `0 ≤ major ≤ 2^31-1`, etc. — but more critically, **it doesn’t reject empty or malformed tags early enough**. However, the bigger correctness bug is: - `jq -r '.sha // empty'` returns `"null"` if `.sha` is missing, but `// empty` converts it to empty string — *good*. - But `jq -r` may output `null` *without* quotes if no match → `CURRENT_SHA="null"` → `PAYLOAD` includes `"sha": "null"` (a string `"null"`, not JSON `null`) → Gitea API will reject it. ✅ **Fix**: ```bash CURRENT_SHA=$(curl -sf ... | jq -r '.sha // empty' 2>/dev/null || true) # Ensure it's either a non-empty string or truly empty (not "null") if [ "$CURRENT_SHA" = "null" ]; then CURRENT_SHA=""; fi ``` ##### **2.4 `base64 -w 0` may not be portable** - ⚠️ **Medium**: `-w 0` is GNU `base64`-specific. On macOS/BSD, use `-b 0` (or `base64 -w 0` may fail silently or wrap). - ✅ **Fix**: Use `base64 -w0` only if GNU `base64` is detected, or use portable alternative: ```bash CONTENT=$(base64 -e | tr -d '\n') < CHANGELOG.md # or CONTENT=$(cat CHANGELOG.md | base64 | tr -d '\n') ``` ##### **2.5 Temp file not cleaned up** - ❌ **Medium**: `RESP_FILE=$(mktemp)` but never `rm "$RESP_FILE"`. In CI, this leaks temp files (though minor, it’s bad practice). - ✅ **Fix**: ```bash RESP_FILE=$(mktemp) trap 'rm -f "$RESP_FILE"' EXIT ``` --- #### **3. Security Issues** ##### **3.1 Shell injection via `$TAG` in `echo` (low risk but avoidable)** - ❌ **Low-Medium**: `echo "$TAG"` is safe *if* `$TAG` is validated, but the validation regex is insufficient: - `v1.2.3` is OK, but `v1.2.3-rc1` is rejected (false positive for pre-release tags). - Worse: `v1.2.3; rm -rf /` would be rejected by the regex — but **only if the regex is applied *before* shell expansion**. Since it’s `echo "$TAG" | grep`, and `echo` doesn’t interpret shell metacharacters, it’s *safe* — but the *real* risk is downstream if `$TAG` is used in other contexts (e.g., `git tag "$TAG"` later?). However, the current code *only* uses `$TAG` in: - `echo "ERROR: ... $TAG"` → safe (quoted) - `jq` `--arg msg` → safe (`jq --arg` escapes) So **no injection here**, but the regex is too strict for real-world SemVer. - ✅ **Fix**: Use stricter validation *and* support pre-release tags: ```bash if ! echo "$TAG" | grep -qE '^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?(\+[0-9A-Za-z.-]+)?$'; then echo "ERROR: Invalid tag format (expected SemVer): $TAG" exit 1 fi ``` ##### **3.2 No validation of `CHANGELOG.md` content** - ❌ **High**: `base64 -w 0 CHANGELOG.md` uploads *any* content — including malicious payloads (e.g., if `CHANGELOG.md` is tampered with by a previous step or attacker). - ✅ **Fix**: Validate content before upload: - Check file size (e.g., `< 1MB`) - Ensure it’s UTF-8 (or at least ASCII-safe) - Reject if it contains suspicious patterns (e.g., `</script>`, `javascript:`, etc.) — though less critical for Markdown. Example: ```bash if [ ! -s CHANGELOG.md ]; then echo "ERROR: CHANGELOG.md is empty or missing" exit 1 fi # Optional: basic UTF-8 check if ! file --mime-encoding CHANGELOG.md | grep -q 'utf-8\|us-ascii'; then echo "ERROR: CHANGELOG.md is not UTF-8/ASCII" exit 1 fi ``` ##### **3.3 `curl` without `--fail`** - ⚠️ **Medium**: `curl -s` does *not* fail on HTTP error codes (e.g., 4xx/5xx). You check `$HTTP_CODE`, but `curl -s` may still output error HTML to stdout (which gets piped to `jq` → `jq` fails with parse error). - ✅ **Fix**: Add `-f` (fail silently on HTTP errors) *or* keep `HTTP_CODE` check (as done), but ensure `curl` doesn’t output to stdout/stderr in success case (already done with `-s`). --- #### **4. Best Practices** ##### **4.1 Use environment variables for config** - ✅ **Recommendation**: Move `API` and `RELEASE_TOKEN` to environment variables (already done for `RELEASE_TOKEN`, but `API` is hardcoded). ```yaml env: GITEA_API_URL: "http://gitea:3000/api/v1/repos/${GITHUB_REPOSITORY}" # or better: use GITHUB_REPOSITORY and GITHUB_SERVER_URL to derive ``` ##### **4.2 Idempotency & atomicity** - ✅ **Good**: Using `PUT /contents/{path}` is atomic and handles create/update in one call — much better than `git push`. ##### **4.3 Logging & debugging** - ✅ **Good**: Printing `HTTP_CODE` and response body on error is helpful. - ⚠️ **Improve**: Log `TAG` (sanitized) for traceability: ```bash echo "Processing tag: ${TAG:0:12}..." ``` ##### **4.4 Error handling with `set -euo pipefail`** - ✅ **Good**: `set -euo pipefail` is correctly used. ##### **4.5 `jq` version compatibility** - ⚠️ **Low**: `jq -n --arg ... 'if ... then ... else ... end'` is fine, but ensure `jq >= 1.6` (older versions may have issues with `// empty` in conditionals). Consider using `jq --argjson` for booleans, but here it’s not needed. --- ### ✅ **Recommended Fixes Summary** | Issue | Fix | |------|-----| | Hardcoded IP | Use `${GITEA_API_URL:-http://gitea:3000/api/v1/repos/$GITHUB_REPOSITORY}` or derive from CI env | | Missing auth header | Add `-H "Authorization: token $RELEASE_TOKEN"` | | `CURRENT_SHA` = `"null"` | Filter: `CURRENT_SHA="${CURRENT_SHA:-}"; [ "$CURRENT_SHA" = "null" ] && CURRENT_SHA=""` | | `base64` portability | Use `tr -d '\n'` after `base64` | | Temp file leak | `trap 'rm -f "$RESP_FILE"' EXIT` | | Weak tag validation | Use full SemVer regex | | No `CHANGELOG.md` validation | Check size, encoding, non-empty | | (Optional) Pre-release tags | Support `-rc1`, `+build`, etc. in tag regex | --- ### Final Verdict - **Security**: ❌ **High-risk** due to missing auth and unvalidated content. - **Correctness**: ⚠️ **Moderate-risk** due to API assumptions and edge cases. - **Best practices**: ✅ Mostly good, but could be improved. **Do not merge as-is.** Fix the auth header and tag validation first — those are blocking issues. Let me know if you'd like a revised diff with all fixes applied!\n\n---\n*this is an automated review from Ollama*

sarman merged commit f7011c8837 into master 2026-04-13 03:18:11 +00:00

sarman referenced this issue from a commit 2026-04-13 03:18:12 +00:00

Merge pull request 'fix(ci): use Gitea file API to push CHANGELOG.md' (#40) from fix/changelog-push into master

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

Compat/Breaking

Breaking change that won't be backward compatible

  

Kind/Bug

Something is not working

  

Kind/Documentation

Documentation changes

  

Kind/Enhancement

Improve existing functionality

  

Kind/Feature

New functionality

  

Kind/Security

This is security issue

  

Kind/Testing

Issue or pull request related to testing

  

Priority

Critical

The priority is critical

  

Priority

High

The priority is high

  

Priority

Low

The priority is low

  

Priority

Medium

The priority is medium

  

Reviewed

Confirmed

Issue has been confirmed

  

Reviewed

Duplicate

This issue or pull request already exists

  

Reviewed

Invalid

Invalid issue

  

Reviewed

Won't Fix

This issue won't be fixed

  

Status

Abandoned

Somebody has started to work on this but abandoned work

  

Status

Blocked

Something is blocking this issue or pull request

  

Status

Need More Info

Feedback is required to reproduce issue or to continue work

No Label

Compat/Breaking

Kind/Bug

Kind/Documentation

Kind/Enhancement

Kind/Feature

Kind/Security

Kind/Testing

Priority

Critical

Priority

High

Priority

Low

Priority

Medium

Reviewed

Confirmed

Reviewed

Duplicate

Reviewed

Invalid

Reviewed

Won't Fix

Status

Abandoned

Status

Blocked

Status

Need More Info

Milestone

Clear milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

1 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: sarman/tftsr-devops_investigation#40

No description provided.