From 4cf26bb70b6830eb63b1adef0a5ef16bec3729d2 Mon Sep 17 00:00:00 2001 From: Matsu Date: Tue, 28 Apr 2026 10:44:44 +0300 Subject: [PATCH] ci: Clean up Template Injection surface in Actions (#29354) --- .github/actions/setup-nodejs/action.yml | 8 +++- .github/scripts/retry.mjs | 45 +++++++++++++------ .github/workflows/docker-build-push.yml | 10 +++-- .../workflows/sbom-generation-callable.yml | 3 +- .../workflows/sec-sync-public-to-private.yml | 14 ++++-- .../security-trivy-scan-callable.yml | 14 ++++-- .github/workflows/test-e2e-reusable.yml | 11 ++++- .github/workflows/test-evals-ai-reusable.yml | 30 ++++++++----- .../util-approve-and-set-automerge.yml | 12 +++-- .github/workflows/util-claude-task.yml | 4 +- .../util-qa-metrics-comment-reusable.yml | 6 ++- 11 files changed, 111 insertions(+), 46 deletions(-) diff --git a/.github/actions/setup-nodejs/action.yml b/.github/actions/setup-nodejs/action.yml index 2fb400a46bd..65bdce28f06 100644 --- a/.github/actions/setup-nodejs/action.yml +++ b/.github/actions/setup-nodejs/action.yml @@ -59,8 +59,10 @@ runs: - name: Install Dependencies if: ${{ inputs.install-command != '' }} + env: + INSTALL_COMMAND: ${{ inputs.install-command }} run: | - ${{ inputs.install-command }} + $INSTALL_COMMAND shell: bash - name: Disable safe-chain @@ -81,8 +83,10 @@ runs: - name: Build Project if: ${{ inputs.build-command != '' }} + env: + BUILD_COMMAND: ${{ inputs.build-command }} run: | - ${{ inputs.build-command }} --summarize + $BUILD_COMMAND --summarize node .github/scripts/send-build-stats.mjs || true node .github/scripts/send-docker-stats.mjs || true shell: bash diff --git a/.github/scripts/retry.mjs b/.github/scripts/retry.mjs index c928a79cdcc..e23f9b9103c 100644 --- a/.github/scripts/retry.mjs +++ b/.github/scripts/retry.mjs @@ -2,16 +2,18 @@ /** * Retry a shell command with configurable attempts and delay. * - * Usage: node retry.mjs [--attempts N] [--delay N] '' + * Usage (safe): node retry.mjs [--attempts N] [--delay N] -- [args...] + * Usage (legacy): node retry.mjs [--attempts N] [--delay N] '' * * Options: * --attempts N Maximum number of attempts (default: 4) * --delay N Seconds to wait between retries (default: 15) * - * The command is executed via shell, so pipes and env-var expansion work. + * The -- form passes args directly to the process (no shell, safe for untrusted input). + * The legacy form executes via shell, so pipes and env-var expansion work but injection is possible. * Exits 0 on first success, 1 if all attempts fail. */ -import { execSync } from 'node:child_process'; +import { execSync, spawnSync } from 'node:child_process'; const args = process.argv.slice(2); @@ -29,23 +31,40 @@ function getFlag(name, defaultValue) { const attempts = getFlag('attempts', 4); const delay = getFlag('delay', 15); -// Command is the last positional arg (skip flags and their values) -const command = args - .filter((a, i) => { - if (a.startsWith('--')) return false; - if (i > 0 && args[i - 1].startsWith('--')) return false; - return true; - }) - .pop(); +// Preferred form: -- cmd arg1 arg2 ... (no shell, safe for untrusted input) +// Legacy form: '' (uses shell; kept for backwards compat) +const separatorIndex = args.indexOf('--'); + +let command; +let commandArgs = []; + +const isSafeRetry = separatorIndex !== -1; + +if (isSafeRetry) { + [command, ...commandArgs] = args.slice(separatorIndex + 1); +} else { + command = args + .filter((a, i) => { + if (a.startsWith('--')) return false; + if (i > 0 && args[i - 1].startsWith('--')) return false; + return true; + }) + .pop(); +} if (!command) { - console.error("Usage: node retry.mjs [--attempts N] [--delay N] ''"); + console.error('Usage: node retry.mjs [--attempts N] [--delay N] -- [args...]'); process.exit(1); } for (let i = 1; i <= attempts; i++) { try { - execSync(command, { shell: true, stdio: 'inherit' }); + if (isSafeRetry) { + const result = spawnSync(command, commandArgs, { stdio: 'inherit' }); + if (result.status !== 0) throw new Error(`Exit code ${result.status}`); + } else { + execSync(command, { shell: true, stdio: 'inherit' }); + } process.exit(0); } catch { if (i < attempts) { diff --git a/.github/workflows/docker-build-push.yml b/.github/workflows/docker-build-push.yml index 5f26e326b2e..de43c3971fb 100644 --- a/.github/workflows/docker-build-push.yml +++ b/.github/workflows/docker-build-push.yml @@ -58,14 +58,18 @@ jobs: - name: Determine build context id: context + env: + N8N_VERSION: ${{ inputs.n8n_version }} + RELEASE_TYPE: ${{ inputs.release_type }} + PUSH_ENABLED: ${{ inputs.push_enabled }} run: | node .github/scripts/docker/docker-config.mjs \ --event "${{ github.event_name }}" \ --pr "${{ github.event.pull_request.number }}" \ --branch "${{ github.ref_name }}" \ - --version "${{ inputs.n8n_version }}" \ - --release-type "${{ inputs.release_type }}" \ - --push-enabled "${{ inputs.push_enabled }}" + --version "$N8N_VERSION" \ + --release-type "$RELEASE_TYPE" \ + --push-enabled "$PUSH_ENABLED" build-and-push-docker: name: Build App, then Build and Push Docker Image (${{ matrix.platform }}) diff --git a/.github/workflows/sbom-generation-callable.yml b/.github/workflows/sbom-generation-callable.yml index 559ef3b74c5..0b2fe2a9b17 100644 --- a/.github/workflows/sbom-generation-callable.yml +++ b/.github/workflows/sbom-generation-callable.yml @@ -64,9 +64,10 @@ jobs: - name: Attach SBOM and VEX files to release env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + RELEASE_TAG_REF: ${{ inputs.release_tag_ref }} run: | # Upload SBOM and VEX files to the existing release - gh release upload "${{ inputs.release_tag_ref }}" \ + gh release upload "$RELEASE_TAG_REF" \ sbom-source.cdx.json \ security/vex.openvex.json \ --clobber diff --git a/.github/workflows/sec-sync-public-to-private.yml b/.github/workflows/sec-sync-public-to-private.yml index fdba7351344..ab66a32c4a7 100644 --- a/.github/workflows/sec-sync-public-to-private.yml +++ b/.github/workflows/sec-sync-public-to-private.yml @@ -40,6 +40,9 @@ jobs: token: ${{ steps.app-token.outputs.token }} - name: Sync master from public + env: + EVENT_NAME: ${{ github.event_name }} + FORCE: ${{ inputs.force }} run: | git fetch https://github.com/n8n-io/n8n.git master:public-master @@ -47,10 +50,10 @@ jobs: AHEAD_COUNT=$(git rev-list public-master..HEAD --pretty=oneline --grep="chore: Bundle" --invert-grep --count) if [ "$AHEAD_COUNT" -gt 0 ]; then - if [ "${{ github.event_name }}" = "schedule" ]; then + if [ "$EVENT_NAME" = "schedule" ]; then echo "Private is $AHEAD_COUNT commit(s) ahead of public, skipping scheduled sync" exit 0 - elif [ "${{ inputs.force }}" != "true" ]; then + elif [ "$FORCE" != "true" ]; then echo "Private is $AHEAD_COUNT commit(s) ahead of public, skipping (force not enabled)" exit 0 else @@ -62,6 +65,9 @@ jobs: git push origin master --force-with-lease - name: Sync 1.x from public + env: + EVENT_NAME: ${{ github.event_name }} + FORCE: ${{ inputs.force }} run: | git fetch https://github.com/n8n-io/n8n.git 1.x:public-1.x git checkout 1.x @@ -70,10 +76,10 @@ jobs: AHEAD_COUNT=$(git rev-list public-1.x..HEAD --pretty=oneline --grep="chore: Bundle" --invert-grep --count) if [ "$AHEAD_COUNT" -gt 0 ]; then - if [ "${{ github.event_name }}" = "schedule" ]; then + if [ "$EVENT_NAME" = "schedule" ]; then echo "Private 1.x is $AHEAD_COUNT commit(s) ahead of public, skipping scheduled sync" exit 0 - elif [ "${{ inputs.force }}" != "true" ]; then + elif [ "$FORCE" != "true" ]; then echo "Private 1.x is $AHEAD_COUNT commit(s) ahead of public, skipping (force not enabled)" exit 0 else diff --git a/.github/workflows/security-trivy-scan-callable.yml b/.github/workflows/security-trivy-scan-callable.yml index 0c61486b7ab..3bf970924c0 100644 --- a/.github/workflows/security-trivy-scan-callable.yml +++ b/.github/workflows/security-trivy-scan-callable.yml @@ -40,7 +40,9 @@ jobs: sparse-checkout-cone-mode: false - name: Pull Docker image with retry - run: node .github/scripts/retry.mjs --attempts 4 --delay 15 'docker pull "${{ inputs.image_ref }}"' + env: + IMAGE_REF: ${{ inputs.image_ref }} + run: node .github/scripts/retry.mjs --attempts 4 --delay 15 -- docker pull "$IMAGE_REF" - name: Run Trivy vulnerability scanner uses: aquasecurity/trivy-action@e368e328979b113139d6f9068e03accaed98a518 # v0.34.1 @@ -90,11 +92,13 @@ jobs: - name: Generate GitHub Job Summary if: always() + env: + IMAGE_REF: ${{ inputs.image_ref }} run: | { echo "# 🛡️ Trivy Security Scan Results" echo "" - echo "**Image:** \`${{ inputs.image_ref }}\`" + echo "**Image:** \`$IMAGE_REF\`" echo "**Scan Date:** $(date -u '+%Y-%m-%d %H:%M:%S UTC')" echo "" } >> "$GITHUB_STEP_SUMMARY" @@ -125,7 +129,7 @@ jobs: { # Generate detailed vulnerability table - jq -r --arg image_ref "${{ inputs.image_ref }}" ' + jq -r --arg image_ref "$IMAGE_REF" ' # Collect all vulnerabilities [.Results[] | select(.Vulnerabilities != null) | .Vulnerabilities[]] | # Group by CVE ID to avoid duplicates @@ -165,8 +169,10 @@ jobs: - name: Generate Slack Blocks JSON if: steps.process_results.outputs.vulnerabilities_found == 'true' id: generate_blocks + env: + IMAGE_REF: ${{ inputs.image_ref }} run: | - BLOCKS_JSON=$(jq -c --arg image_ref "${{ inputs.image_ref }}" \ + BLOCKS_JSON=$(jq -c --arg image_ref "$IMAGE_REF" \ --arg repo_url "${{ github.server_url }}/${{ github.repository }}" \ --arg repo_name "${{ github.repository }}" \ --arg run_url "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" \ diff --git a/.github/workflows/test-e2e-reusable.yml b/.github/workflows/test-e2e-reusable.yml index 99ee29f5a3b..3c43190e704 100644 --- a/.github/workflows/test-e2e-reusable.yml +++ b/.github/workflows/test-e2e-reusable.yml @@ -102,9 +102,16 @@ jobs: run: npx tsx packages/testing/containers/pull-test-images.ts ${{ matrix.images }} || true - name: Run Tests - # Uses pre-distributed specs if orchestration enabled, otherwise falls back to Playwright sharding - run: ${{ inputs.test-command }} --workers=${{ env.PLAYWRIGHT_WORKERS }} ${{ matrix.specs || format('--shard={0}/{1}', matrix.shard, strategy.job-total) }} + run: | + # shellcheck disable=SC2086 + $TEST_COMMAND --workers="$WORKERS" $SHARD_ARGS env: + # Protect args from template injections + TEST_COMMAND: ${{ inputs.test-command }} + # Uses pre-distributed specs if orchestration enabled, otherwise falls back to Playwright sharding + WORKERS: ${{ env.PLAYWRIGHT_WORKERS }} + SHARD_ARGS: ${{ matrix.specs || format('--shard={0}/{1}', matrix.shard, strategy.job-total) }} + # Args for actual test command runner CURRENTS_RECORD_KEY: ${{ secrets.CURRENTS_RECORD_KEY }} QA_METRICS_WEBHOOK_URL: ${{ secrets.QA_METRICS_WEBHOOK_URL }} QA_METRICS_WEBHOOK_USER: ${{ secrets.QA_METRICS_WEBHOOK_USER }} diff --git a/.github/workflows/test-evals-ai-reusable.yml b/.github/workflows/test-evals-ai-reusable.yml index 4b5a653de5d..98f1614df72 100644 --- a/.github/workflows/test-evals-ai-reusable.yml +++ b/.github/workflows/test-evals-ai-reusable.yml @@ -48,15 +48,16 @@ jobs: - name: Generate experiment name id: experiment + env: + PREFIX: ${{ inputs.experiment_name_prefix }} + BRANCH: ${{ inputs.branch }} run: | DATE=$(date +%Y_%m_%d) - PREFIX="${{ inputs.experiment_name_prefix }}" if [ -n "$PREFIX" ]; then NAME="${PREFIX}_${DATE}" else # Extract ticket ID from branch name (e.g., AI-1234 from ai-1234-feature-name) - BRANCH="${{ inputs.branch }}" TICKET=$(echo "$BRANCH" | grep -oE '^[Aa][Ii]-[0-9]+' | tr '[:lower:]' '[:upper:]' || true) if [ -n "$TICKET" ]; then NAME="${TICKET}_${DATE}" @@ -95,14 +96,23 @@ jobs: - name: Run Evaluations working-directory: packages/@n8n/ai-workflow-builder.ee/evaluations + env: + SUITE: ${{ inputs.suite }} + DATASET: ${{ inputs.dataset }} + REPETITIONS: ${{ inputs.repetitions }} + JUDGES: ${{ inputs.judges }} + CONCURRENCY: ${{ inputs.concurrency }} + EXPERIMENT_NAME: ${{ steps.experiment.outputs.name }} + WEBHOOK_URL_ARG: ${{ secrets.EVALS_WEBHOOK_URL && format('--webhook-url={0}', secrets.EVALS_WEBHOOK_URL) || '' }} + WEBHOOK_SECRET_ARG: ${{ secrets.EVALS_WEBHOOK_SECRET && format('--webhook-secret={0}', secrets.EVALS_WEBHOOK_SECRET) || '' }} run: | pnpm eval \ - --suite "${{ inputs.suite }}" \ + --suite "$SUITE" \ --backend langsmith \ - --dataset "${{ inputs.dataset }}" \ - --repetitions ${{ inputs.repetitions }} \ - --judges ${{ inputs.judges }} \ - --concurrency ${{ inputs.concurrency }} \ - --name "${{ steps.experiment.outputs.name }}" \ - ${{ secrets.EVALS_WEBHOOK_URL && format('--webhook-url "{0}"', secrets.EVALS_WEBHOOK_URL) || '' }} \ - ${{ secrets.EVALS_WEBHOOK_SECRET && format('--webhook-secret "{0}"', secrets.EVALS_WEBHOOK_SECRET) || '' }} + --dataset "$DATASET" \ + --repetitions "$REPETITIONS" \ + --judges "$JUDGES" \ + --concurrency "$CONCURRENCY" \ + --name "$EXPERIMENT_NAME" \ + "$WEBHOOK_URL_ARG" \ + "$WEBHOOK_SECRET_ARG" diff --git a/.github/workflows/util-approve-and-set-automerge.yml b/.github/workflows/util-approve-and-set-automerge.yml index 0c2971e73ce..d820b1c35ce 100644 --- a/.github/workflows/util-approve-and-set-automerge.yml +++ b/.github/workflows/util-approve-and-set-automerge.yml @@ -30,16 +30,20 @@ jobs: - name: Approve PR (as the App) env: GH_TOKEN: ${{ steps.generate-token.outputs.token }} + PR_NUMBER: ${{ inputs.pull-request-number }} + REPOSITORY: ${{ github.repository }} run: | - gh pr review "${{ inputs.pull-request-number }}" \ + gh pr review "$PR_NUMBER" \ --approve \ - --repo "${{ github.repository }}" + --repo "$REPOSITORY" - name: Enable auto-merge (merge when checks pass) env: GH_TOKEN: ${{ steps.generate-token.outputs.token }} + PR_NUMBER: ${{ inputs.pull-request-number }} + REPOSITORY: ${{ github.repository }} run: | - gh pr merge "${{ inputs.pull-request-number }}" \ + gh pr merge "$PR_NUMBER" \ --auto \ --squash \ - --repo "${{ github.repository }}" + --repo "$REPOSITORY" diff --git a/.github/workflows/util-claude-task.yml b/.github/workflows/util-claude-task.yml index 26b36a9b90f..719defeab70 100644 --- a/.github/workflows/util-claude-task.yml +++ b/.github/workflows/util-claude-task.yml @@ -169,8 +169,10 @@ jobs: - name: Push branch if: always() + env: + REF: ${{ inputs.ref }} run: | - if ! git diff --quiet "${{ inputs.ref }}" 2>/dev/null; then + if ! git diff --quiet "$REF" 2>/dev/null; then git push -u origin "$BRANCH_NAME" echo "::notice::Changes pushed to branch $BRANCH_NAME" else diff --git a/.github/workflows/util-qa-metrics-comment-reusable.yml b/.github/workflows/util-qa-metrics-comment-reusable.yml index f9f4419ccd0..543452588c8 100644 --- a/.github/workflows/util-qa-metrics-comment-reusable.yml +++ b/.github/workflows/util-qa-metrics-comment-reusable.yml @@ -35,7 +35,9 @@ jobs: QA_METRICS_WEBHOOK_USER: ${{ secrets.QA_METRICS_WEBHOOK_USER }} QA_METRICS_WEBHOOK_PASSWORD: ${{ secrets.QA_METRICS_WEBHOOK_PASSWORD }} GITHUB_TOKEN: ${{ github.token }} + METRICS: ${{ inputs.metrics }} + BASELINE_DAYS: ${{ inputs.baseline-days }} run: | node .github/scripts/post-qa-metrics-comment.mjs \ - --metrics "${{ inputs.metrics }}" \ - --baseline-days "${{ inputs.baseline-days }}" + --metrics "$METRICS" \ + --baseline-days "$BASELINE_DAYS"