Copilot commented on code in PR #4453:
URL: https://github.com/apache/solr/pull/4453#discussion_r3272036331
##########
.github/workflows/renovate-changelog-push.yml:
##########
@@ -0,0 +1,155 @@
+name: Generate Renovate Changelog (Stage 2 - Push)
+
+# Stage 2: runs after Stage 1 completes, in the base-repo context with access
to
+# SOLRBOT_GITHUB_TOKEN. Downloads the pre-generated artifact (no fork code
executed here),
+# validates metadata, and pushes the changelog file to the fork branch.
+on:
+ workflow_run:
+ workflows:
+ - "Generate Renovate Changelog (Stage 1 - Prepare)"
+ types:
+ - completed
+
+# Each Stage 2 run corresponds to a unique Stage 1 run (unique
workflow_run.id).
+# No cancel-in-progress: Stage 1's concurrency already serializes per-PR.
+concurrency:
+ group: renovate-changelog-push-${{ github.event.workflow_run.id }}
+
+permissions:
+ actions: read # Required to download artifacts from another workflow run
+ contents: read # Minimal; actual write access to fork uses
SOLRBOT_GITHUB_TOKEN PAT
+
+jobs:
+ push-changelog:
+ # Only proceed if Stage 1 succeeded for the expected fork.
+ # Checking head_repository here avoids a spurious artifact-not-found
failure
+ # when Stage 1 ran but skipped its generate job (e.g. non-solrbot PR).
+ if: |
+ github.event.workflow_run.conclusion == 'success' &&
+ github.event.workflow_run.head_repository.full_name ==
'solrbot/apache-_-solr'
+ runs-on: ubuntu-latest
+
+ steps:
+ - name: Download artifact from Stage 1
+ uses:
actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8
+ with:
+ name: renovate-changelog-artifact
+ run-id: ${{ github.event.workflow_run.id }}
+ github-token: ${{ secrets.GITHUB_TOKEN }}
+ path: downloaded-artifact/
+
+ - name: Read and validate PR metadata
+ id: meta
+ run: |
+ set -euo pipefail
+ META_FILE="downloaded-artifact/pr-metadata.env"
+ if [ ! -f "$META_FILE" ]; then
+ echo "::error::Metadata file missing from artifact"; exit 1
+ fi
+
+ # Parse with grep/cut rather than source to avoid executing file
content as shell
+ PR_NUMBER=$(grep '^PR_NUMBER=' "$META_FILE" | cut -d= -f2-)
+ HEAD_REF=$(grep '^HEAD_REF=' "$META_FILE" | cut -d= -f2-)
+ HEAD_REPO=$(grep '^HEAD_REPO=' "$META_FILE" | cut -d= -f2-)
+
+ if [ -z "$PR_NUMBER" ] || [ -z "$HEAD_REF" ] || [ -z "$HEAD_REPO" ];
then
+ echo "::error::Missing required metadata fields (PR_NUMBER,
HEAD_REF, or HEAD_REPO)"; exit 1
+ fi
Review Comment:
With `set -euo pipefail`, if any of these keys are missing from the metadata
file then `grep` exits non-zero and the step fails immediately, so the explicit
`::error::Missing required metadata fields ...` message below never runs.
Consider making these assignments non-fatal (e.g., `... || true`) and then
relying on the subsequent emptiness check for a clear error message.
##########
.github/workflows/renovate-changelog-push.yml:
##########
@@ -0,0 +1,155 @@
+name: Generate Renovate Changelog (Stage 2 - Push)
+
+# Stage 2: runs after Stage 1 completes, in the base-repo context with access
to
+# SOLRBOT_GITHUB_TOKEN. Downloads the pre-generated artifact (no fork code
executed here),
+# validates metadata, and pushes the changelog file to the fork branch.
+on:
+ workflow_run:
+ workflows:
+ - "Generate Renovate Changelog (Stage 1 - Prepare)"
+ types:
+ - completed
+
+# Each Stage 2 run corresponds to a unique Stage 1 run (unique
workflow_run.id).
+# No cancel-in-progress: Stage 1's concurrency already serializes per-PR.
+concurrency:
+ group: renovate-changelog-push-${{ github.event.workflow_run.id }}
+
+permissions:
+ actions: read # Required to download artifacts from another workflow run
+ contents: read # Minimal; actual write access to fork uses
SOLRBOT_GITHUB_TOKEN PAT
+
+jobs:
+ push-changelog:
+ # Only proceed if Stage 1 succeeded for the expected fork.
+ # Checking head_repository here avoids a spurious artifact-not-found
failure
+ # when Stage 1 ran but skipped its generate job (e.g. non-solrbot PR).
+ if: |
+ github.event.workflow_run.conclusion == 'success' &&
+ github.event.workflow_run.head_repository.full_name ==
'solrbot/apache-_-solr'
+ runs-on: ubuntu-latest
+
+ steps:
+ - name: Download artifact from Stage 1
+ uses:
actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8
+ with:
+ name: renovate-changelog-artifact
+ run-id: ${{ github.event.workflow_run.id }}
+ github-token: ${{ secrets.GITHUB_TOKEN }}
+ path: downloaded-artifact/
+
+ - name: Read and validate PR metadata
+ id: meta
+ run: |
+ set -euo pipefail
+ META_FILE="downloaded-artifact/pr-metadata.env"
+ if [ ! -f "$META_FILE" ]; then
+ echo "::error::Metadata file missing from artifact"; exit 1
+ fi
+
+ # Parse with grep/cut rather than source to avoid executing file
content as shell
+ PR_NUMBER=$(grep '^PR_NUMBER=' "$META_FILE" | cut -d= -f2-)
+ HEAD_REF=$(grep '^HEAD_REF=' "$META_FILE" | cut -d= -f2-)
+ HEAD_REPO=$(grep '^HEAD_REPO=' "$META_FILE" | cut -d= -f2-)
+
+ if [ -z "$PR_NUMBER" ] || [ -z "$HEAD_REF" ] || [ -z "$HEAD_REPO" ];
then
+ echo "::error::Missing required metadata fields (PR_NUMBER,
HEAD_REF, or HEAD_REPO)"; exit 1
+ fi
+
+ # Security: verify this is the expected fork before using
SOLRBOT_GITHUB_TOKEN
+ if [ "$HEAD_REPO" != "solrbot/apache-_-solr" ]; then
+ echo "::error::Unexpected HEAD_REPO: '$HEAD_REPO'. Expected
'solrbot/apache-_-solr'. Aborting."; exit 1
+ fi
+
+ # Validate PR_NUMBER is a plain positive integer (prevents injection)
+ if ! [[ "$PR_NUMBER" =~ ^[1-9][0-9]*$ ]]; then
+ echo "::error::PR_NUMBER is not a valid positive integer:
'$PR_NUMBER'"; exit 1
+ fi
+
+ # Validate HEAD_REF looks like a safe branch name (prevents path
traversal).
+ # Includes @ because Renovate uses it in branch names (e.g.
renovate/node@lts).
+ if ! [[ "$HEAD_REF" =~ ^[a-zA-Z0-9._/@:-]+$ ]]; then
+ echo "::error::HEAD_REF contains unexpected characters:
'$HEAD_REF'"; exit 1
+ fi
+
+ echo "pr_number=$PR_NUMBER" >> "$GITHUB_OUTPUT"
+ echo "head_ref=$HEAD_REF" >> "$GITHUB_OUTPUT"
+ echo "head_repo=$HEAD_REPO" >> "$GITHUB_OUTPUT"
+ echo "Validated: PR#${PR_NUMBER} on ${HEAD_REPO}@${HEAD_REF}"
+
+ - name: Clone fork branch
+ env:
+ SOLRBOT_TOKEN: ${{ secrets.SOLRBOT_GITHUB_TOKEN }}
+ HEAD_REPO: ${{ steps.meta.outputs.head_repo }}
+ HEAD_REF: ${{ steps.meta.outputs.head_ref }}
+ run: |
+ set -euo pipefail
+ # Store credentials so the token never appears in the command line
or process list
+ git config --global credential.helper store
+ printf 'https://x-access-token:%[email protected]\n' "$SOLRBOT_TOKEN"
> ~/.git-credentials
Review Comment:
After writing `~/.git-credentials`, it should be restricted and ideally
cleaned up (e.g., `chmod 600 ~/.git-credentials` and remove it after the push).
This reduces the chance of accidental exposure in later steps or
troubleshooting output within the same job.
##########
.github/workflows/renovate-changelog-push.yml:
##########
@@ -0,0 +1,155 @@
+name: Generate Renovate Changelog (Stage 2 - Push)
+
+# Stage 2: runs after Stage 1 completes, in the base-repo context with access
to
+# SOLRBOT_GITHUB_TOKEN. Downloads the pre-generated artifact (no fork code
executed here),
+# validates metadata, and pushes the changelog file to the fork branch.
+on:
+ workflow_run:
+ workflows:
+ - "Generate Renovate Changelog (Stage 1 - Prepare)"
+ types:
+ - completed
+
+# Each Stage 2 run corresponds to a unique Stage 1 run (unique
workflow_run.id).
+# No cancel-in-progress: Stage 1's concurrency already serializes per-PR.
+concurrency:
+ group: renovate-changelog-push-${{ github.event.workflow_run.id }}
+
+permissions:
+ actions: read # Required to download artifacts from another workflow run
+ contents: read # Minimal; actual write access to fork uses
SOLRBOT_GITHUB_TOKEN PAT
+
+jobs:
+ push-changelog:
+ # Only proceed if Stage 1 succeeded for the expected fork.
+ # Checking head_repository here avoids a spurious artifact-not-found
failure
+ # when Stage 1 ran but skipped its generate job (e.g. non-solrbot PR).
+ if: |
+ github.event.workflow_run.conclusion == 'success' &&
+ github.event.workflow_run.head_repository.full_name ==
'solrbot/apache-_-solr'
+ runs-on: ubuntu-latest
+
+ steps:
+ - name: Download artifact from Stage 1
+ uses:
actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8
+ with:
+ name: renovate-changelog-artifact
+ run-id: ${{ github.event.workflow_run.id }}
+ github-token: ${{ secrets.GITHUB_TOKEN }}
+ path: downloaded-artifact/
+
+ - name: Read and validate PR metadata
+ id: meta
+ run: |
+ set -euo pipefail
+ META_FILE="downloaded-artifact/pr-metadata.env"
+ if [ ! -f "$META_FILE" ]; then
+ echo "::error::Metadata file missing from artifact"; exit 1
+ fi
+
+ # Parse with grep/cut rather than source to avoid executing file
content as shell
+ PR_NUMBER=$(grep '^PR_NUMBER=' "$META_FILE" | cut -d= -f2-)
+ HEAD_REF=$(grep '^HEAD_REF=' "$META_FILE" | cut -d= -f2-)
+ HEAD_REPO=$(grep '^HEAD_REPO=' "$META_FILE" | cut -d= -f2-)
+
+ if [ -z "$PR_NUMBER" ] || [ -z "$HEAD_REF" ] || [ -z "$HEAD_REPO" ];
then
+ echo "::error::Missing required metadata fields (PR_NUMBER,
HEAD_REF, or HEAD_REPO)"; exit 1
+ fi
+
+ # Security: verify this is the expected fork before using
SOLRBOT_GITHUB_TOKEN
+ if [ "$HEAD_REPO" != "solrbot/apache-_-solr" ]; then
+ echo "::error::Unexpected HEAD_REPO: '$HEAD_REPO'. Expected
'solrbot/apache-_-solr'. Aborting."; exit 1
+ fi
+
+ # Validate PR_NUMBER is a plain positive integer (prevents injection)
+ if ! [[ "$PR_NUMBER" =~ ^[1-9][0-9]*$ ]]; then
+ echo "::error::PR_NUMBER is not a valid positive integer:
'$PR_NUMBER'"; exit 1
+ fi
+
+ # Validate HEAD_REF looks like a safe branch name (prevents path
traversal).
+ # Includes @ because Renovate uses it in branch names (e.g.
renovate/node@lts).
+ if ! [[ "$HEAD_REF" =~ ^[a-zA-Z0-9._/@:-]+$ ]]; then
+ echo "::error::HEAD_REF contains unexpected characters:
'$HEAD_REF'"; exit 1
Review Comment:
The HEAD_REF allowlist includes ":", which is meaningful in `git push`
refspecs and could lead to unexpected behavior if it ever appears in the value.
Consider validating `HEAD_REF` using `git check-ref-format --branch` (or
explicitly rejecting refname edge cases like `:`, `@{`, `..`, trailing `.lock`,
etc.) before using it in `git clone --branch` and `git push ...
refs/heads/${HEAD_REF}`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
