From b361c53209303024fc0bc82b3c78ef7e8d33fa36 Mon Sep 17 00:00:00 2001 From: Stephen Crowe <6042774+crowecawcaw@users.noreply.github.com> Date: Tue, 7 Jul 2026 10:36:53 -0700 Subject: [PATCH] ci: gate Security Scan on high findings and fix all high findings Change the reusable Security Scan to a blocking gate that reports in the run log / PR annotations instead of the Security tab: - advanced-security: false + annotations: true (findings in logs) - min-severity input (default high) fails the job on high findings only, so PRs are gated on serious issues without red-walling on hygiene noise - drop continue-on-error and the now-unneeded security-events permission Fix all 22 high findings so .github passes its own gate: - template-injection (18): move ${{ inputs.* }} / ${{ github.* }} out of run: blocks into env: vars referenced as shell $VARS. Behavior-preserving. - unsound-condition (4): nested ${{ }} inside an if: expression made the condition always true, defeating the guard. Rewrite as pure expressions (format() for the repo check; inputs.installer_oses != ''). Signed-off-by: Stephen Crowe <6042774+crowecawcaw@users.noreply.github.com> --- .github/actions/check-release/action.yml | 16 ++++++---- .github/actions/tag-release/action.yml | 23 +++++++------ .github/workflows/reusable_bump.yml | 11 +++++-- .github/workflows/reusable_canary.yml | 2 +- .github/workflows/reusable_e2e_test.yml | 2 +- .../workflows/reusable_integration_test.yml | 2 +- .github/workflows/reusable_label_pr.yml | 8 +++-- .github/workflows/reusable_publish_v2.yml | 2 +- .github/workflows/reusable_release.yml | 7 ++-- .github/workflows/reusable_security_scan.yml | 32 ++++++++++++------- 10 files changed, 65 insertions(+), 40 deletions(-) diff --git a/.github/actions/check-release/action.yml b/.github/actions/check-release/action.yml index 641896d..74b9d14 100644 --- a/.github/actions/check-release/action.yml +++ b/.github/actions/check-release/action.yml @@ -21,20 +21,22 @@ runs: - name: Check if release exists and verify author shell: bash run: | - echo "Checking if release exists for tag ${{ inputs.tag }}..." - if gh release view "${{ inputs.tag }}" &> /dev/null; then - echo "::error::Release for tag ${{ inputs.tag }} already exists!" + echo "Checking if release exists for tag $TAG..." + if gh release view "$TAG" &> /dev/null; then + echo "::error::Release for tag $TAG already exists!" exit 1 fi - + echo "Verifying commit author..." - EXPECTED_AUTHOR="${{ inputs.expected_author }}" AUTHOR=$(git show -s --format='%ae' HEAD) if [[ $AUTHOR != $EXPECTED_AUTHOR ]]; then echo "::error::Expected author email to be '$EXPECTED_AUTHOR', but got '$AUTHOR'. Aborting release." exit 1 fi - + echo "All checks passed: Tag exists, no release found, and author verified." env: - GH_TOKEN: ${{ github.token }} \ No newline at end of file + GH_TOKEN: ${{ github.token }} + # Pass inputs via env, not inline ${{ }}, to avoid template injection. + TAG: ${{ inputs.tag }} + EXPECTED_AUTHOR: ${{ inputs.expected_author }} \ No newline at end of file diff --git a/.github/actions/tag-release/action.yml b/.github/actions/tag-release/action.yml index 4833cfd..8451768 100644 --- a/.github/actions/tag-release/action.yml +++ b/.github/actions/tag-release/action.yml @@ -23,17 +23,22 @@ runs: - name: Determine tag id: determine-tag shell: bash + env: + # Pass inputs via env, not inline ${{ }}, to avoid template injection. + INPUT_TAG: ${{ inputs.tag }} + INPUT_EMAIL: ${{ inputs.email }} + INPUT_USERNAME: ${{ inputs.username }} run: | - if [[ -n "${{ inputs.tag }}" ]]; then - echo "Using provided tag: ${{ inputs.tag }}" - echo "tag=${{ inputs.tag }}" >> $GITHUB_OUTPUT + if [[ -n "$INPUT_TAG" ]]; then + echo "Using provided tag: $INPUT_TAG" + echo "tag=$INPUT_TAG" >> $GITHUB_OUTPUT echo "is_new_tag=false" >> $GITHUB_OUTPUT else echo "No tag provided, will create a new one" echo "is_new_tag=true" >> $GITHUB_OUTPUT - + # Verify author - EXPECTED_AUTHOR="${{ inputs.email }}" + EXPECTED_AUTHOR="$INPUT_EMAIL" AUTHOR=$(git show -s --format='%ae' HEAD) if [[ $AUTHOR != $EXPECTED_AUTHOR ]]; then echo "ERROR: Expected author email to be '$EXPECTED_AUTHOR', but got '$AUTHOR'. Aborting release." @@ -41,11 +46,11 @@ runs: else echo "Verified author email ($AUTHOR) is as expected ($EXPECTED_AUTHOR)" fi - + # Configure git - git config user.name "${{ inputs.username }}" - git config user.email "${{ inputs.email }}" - + git config user.name "$INPUT_USERNAME" + git config user.email "$INPUT_EMAIL" + COMMIT_TITLE=$(git show -s --format='%s' HEAD) NEXT_SEMVER=$(python -c 'import sys, re; print(re.match(r"chore\(release\): ([0-9]+\.[0-9]+\.[0-9]+).*", sys.argv[1]).group(1))' "$COMMIT_TITLE") diff --git a/.github/workflows/reusable_bump.yml b/.github/workflows/reusable_bump.yml index 4982088..452ee99 100644 --- a/.github/workflows/reusable_bump.yml +++ b/.github/workflows/reusable_bump.yml @@ -54,10 +54,13 @@ jobs: mask-aws-account-id: true - name: NextSemver + env: + # Pass inputs via env, not inline ${{ }}, to avoid template injection. + FORCE_VERSION_BUMP: ${{ inputs.force_version_bump }} run: | BUMP_ARGS="" - if [[ "${{ inputs.force_version_bump }}" != "" ]]; then - BUMP_ARGS="$BUMP_ARGS --${{ inputs.force_version_bump }}" + if [[ "$FORCE_VERSION_BUMP" != "" ]]; then + BUMP_ARGS="$BUMP_ARGS --$FORCE_VERSION_BUMP" fi # Backup actual changelog to preserve its contents @@ -111,8 +114,10 @@ jobs: - name: PushPR env: GH_TOKEN: ${{ secrets.CI_TOKEN }} + # Pass context via env, not inline ${{ }}, to avoid template injection. + BASE_REF: ${{ github.ref_name }} run: | git push -u origin bump/$NEXT_SEMVER # Needs "Allow GitHub Actions to create and approve pull requests" under Settings > Actions - gh pr create --base ${{ github.ref_name }} --title "chore(release): $NEXT_SEMVER" --body "$RELEASE_NOTES" + gh pr create --base "$BASE_REF" --title "chore(release): $NEXT_SEMVER" --body "$RELEASE_NOTES" diff --git a/.github/workflows/reusable_canary.yml b/.github/workflows/reusable_canary.yml index 5ad87b1..cc238c3 100644 --- a/.github/workflows/reusable_canary.yml +++ b/.github/workflows/reusable_canary.yml @@ -18,7 +18,7 @@ on: jobs: Canary: - if: (github.repository == 'aws-deadline/${{inputs.repository}}') + if: github.repository == format('aws-deadline/{0}', inputs.repository) name: Canary runs-on: ubuntu-latest environment: ${{inputs.environment}} diff --git a/.github/workflows/reusable_e2e_test.yml b/.github/workflows/reusable_e2e_test.yml index fe5f004..4b50f13 100644 --- a/.github/workflows/reusable_e2e_test.yml +++ b/.github/workflows/reusable_e2e_test.yml @@ -21,7 +21,7 @@ on: jobs: E2ETests: - if: (github.repository == 'aws-deadline/${{inputs.repository}}') + if: github.repository == format('aws-deadline/{0}', inputs.repository) name: E2E Tests runs-on: ubuntu-latest environment: ${{inputs.environment}} diff --git a/.github/workflows/reusable_integration_test.yml b/.github/workflows/reusable_integration_test.yml index e477409..42ec8dd 100644 --- a/.github/workflows/reusable_integration_test.yml +++ b/.github/workflows/reusable_integration_test.yml @@ -22,7 +22,7 @@ on: jobs: IntegrationTests: - if: (github.repository == 'aws-deadline/${{inputs.repository}}') + if: github.repository == format('aws-deadline/{0}', inputs.repository) name: Integration Tests runs-on: ubuntu-latest environment: ${{inputs.environment}} diff --git a/.github/workflows/reusable_label_pr.yml b/.github/workflows/reusable_label_pr.yml index d28e28b..7bc4f79 100644 --- a/.github/workflows/reusable_label_pr.yml +++ b/.github/workflows/reusable_label_pr.yml @@ -22,7 +22,11 @@ jobs: steps: - name: Add label to PR run: | - echo "Adding label '${{ inputs.label_name }}' to PR #${{ inputs.pr_number }}" - gh pr edit ${{ inputs.pr_number }} --add-label "${{ inputs.label_name }}" --repo ${{ github.repository }} + echo "Adding label '$LABEL_NAME' to PR #$PR_NUMBER" + gh pr edit "$PR_NUMBER" --add-label "$LABEL_NAME" --repo "$REPO" env: GITHUB_TOKEN: ${{ github.token }} + # Pass inputs via env, not inline ${{ }}, to avoid template injection. + PR_NUMBER: ${{ inputs.pr_number }} + LABEL_NAME: ${{ inputs.label_name }} + REPO: ${{ github.repository }} diff --git a/.github/workflows/reusable_publish_v2.yml b/.github/workflows/reusable_publish_v2.yml index 5fefcf7..a5006e2 100644 --- a/.github/workflows/reusable_publish_v2.yml +++ b/.github/workflows/reusable_publish_v2.yml @@ -220,7 +220,7 @@ jobs: needs: [Release, PreRelease] runs-on: ubuntu-latest environment: release - if: (needs.Release.result == 'success' && needs.PreRelease.result == 'success') && ${{ inputs.installer_oses }} + if: needs.Release.result == 'success' && needs.PreRelease.result == 'success' && inputs.installer_oses != '' permissions: id-token: write strategy: diff --git a/.github/workflows/reusable_release.yml b/.github/workflows/reusable_release.yml index 3418088..0fcd312 100644 --- a/.github/workflows/reusable_release.yml +++ b/.github/workflows/reusable_release.yml @@ -60,12 +60,13 @@ jobs: - name: PushRelease env: GH_TOKEN: ${{ secrets.CI_TOKEN }} + # Pass inputs via env, not inline ${{ }}, to avoid template injection. + TAG: ${{ inputs.tag }} run: | RELEASE_NOTES=$(python .github/scripts/get_latest_changelog.py) - TAG=${{inputs.tag}} - + shopt -s nullglob - gh release create $TAG build-artifact/dist/* build-artifact/dist_extras/* --notes "$RELEASE_NOTES" + gh release create "$TAG" build-artifact/dist/* build-artifact/dist_extras/* --notes "$RELEASE_NOTES" \ No newline at end of file diff --git a/.github/workflows/reusable_security_scan.yml b/.github/workflows/reusable_security_scan.yml index e178f17..6a21c3b 100644 --- a/.github/workflows/reusable_security_scan.yml +++ b/.github/workflows/reusable_security_scan.yml @@ -20,15 +20,17 @@ on: required: false type: boolean default: true - fail-on-findings: + min-severity: description: >- - Fail the check when findings are present. Defaults to false so the - scan is advisory (findings still upload to the Security tab) while the - existing backlog is burned down; flip to true per-repo once findings - reach zero. + Minimum finding severity that fails the check. Findings at or above + this level are reported in the step log and fail the job; anything + below is not reported. Defaults to 'high' so PRs are gated on serious + findings without being red-walled by hygiene noise. Lower it (e.g. + 'medium') once a repo's high-severity backlog is at zero. + One of: unknown, informational, low, medium, high. required: false - type: boolean - default: false + type: string + default: high permissions: {} @@ -38,8 +40,9 @@ jobs: if: ${{ inputs.zizmor }} runs-on: ubuntu-latest permissions: - # Required for zizmor-action to upload SARIF results to the Security tab. - security-events: write + # Findings are reported to the step log / PR annotations, not the Security + # tab, so no security-events permission is needed. + contents: read steps: - name: Checkout repository uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 @@ -57,9 +60,9 @@ jobs: persist-credentials: false - name: Run zizmor 🌈 - # While advisory, don't fail the check on findings — SARIF is still - # uploaded to the Security tab so results stay visible. - continue-on-error: ${{ !inputs.fail-on-findings }} + # zizmor exits non-zero when it reports findings, so the job fails on any + # finding at or above min-severity. No continue-on-error: the check is a + # gate, not advisory. uses: zizmorcore/zizmor-action@71321a20a9ded102f6e9ce5718a2fcec2c4f70d8 # v0.5.2 with: # Scope to .github/ so the nested central-config checkout at @@ -67,3 +70,8 @@ jobs: # actions live under .github/, so coverage is complete. inputs: .github/ config: .zizmor-config/zizmor.yml + min-severity: ${{ inputs.min-severity }} + # Report findings in the run log and as inline PR annotations rather + # than uploading SARIF to the Security tab. + advanced-security: false + annotations: true