From 7e4458724f75ddd6a71e3cace00d371101e2e544 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Thu, 11 Jun 2026 19:30:55 -0400 Subject: [PATCH] Harden release workflow and regex against CodeQL findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address three code-scanning alerts on the PR: - Critical (actions/untrusted-checkout): split release.yaml into a read-only `prepare` job that checks out and runs repo code, and a privileged `publish` job (contents:write + OIDC) that never checks out repo code — it tags, releases, and invokes purely through the GitHub and AWS APIs. Also assert head_branch == main. - High x2 (py/polynomial-redos): rewrite the italic and link regexes in markdown_to_mrkdwn with possessive quantifiers and exclusive character classes so they run in linear time on adversarial input. Adds a regression test. --- .github/workflows/release.yaml | 113 +++++++++++++++++++-------------- src/shared/shared/blocks.py | 10 ++- tests/shared/test_blocks.py | 11 ++++ 3 files changed, 85 insertions(+), 49 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 2bc2e94..1125056 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -1,36 +1,42 @@ name: Release # Runs after a successful Deploy. When the top of CHANGELOG.md names a version -# that has no tag yet, this tags it, publishes a GitHub Release with the notes, -# and — for minor/major bumps only — invokes the release-notifier Lambda to -# announce it in Slack. Triggering on Deploy completion (not release:published / -# tag push) is deliberate: GITHUB_TOKEN-created events do not start downstream +# that has no Release yet, this tags it, publishes a GitHub Release with the +# notes, and — for minor/major bumps only — invokes the release-notifier Lambda +# to announce it in Slack. Triggering on Deploy completion (not release:published +# / tag push) is deliberate: GITHUB_TOKEN-created events do not start downstream # workflows, and gating on Deploy success means we never announce a version that # is not actually live. +# +# Two jobs by design (CodeQL actions/untrusted-checkout): the only job that +# checks out and runs repo code (`prepare`) is read-only and unprivileged; the +# job that holds write + OIDC (`publish`) never checks out repo code — it acts +# purely through the GitHub and AWS APIs. on: workflow_run: workflows: ["Deploy"] types: [completed] -permissions: - contents: write # create the tag + GitHub Release - id-token: write # OIDC to assume the notifier-invoke role - -# Serialize so back-to-back releases announce in order (queued Deploys -> queued -# Releases), never overlapping. +# Serialize so back-to-back releases announce in order, never overlapping. concurrency: group: release-announce cancel-in-progress: false jobs: - release: - if: ${{ github.event.workflow_run.conclusion == 'success' }} + prepare: + # Deploy only runs on push to main, so head_sha is always a trusted main + # commit; assert head_branch == main to make that boundary explicit. + if: ${{ github.event.workflow_run.conclusion == 'success' && github.event.workflow_run.head_branch == 'main' }} runs-on: ubuntu-latest + permissions: + contents: read + outputs: + release: ${{ steps.rel.outputs.release }} + version: ${{ steps.rel.outputs.version }} + kind: ${{ steps.rel.outputs.kind }} steps: - uses: actions/checkout@v6 with: - # The exact commit Deploy deployed — NOT the branch HEAD, which a later - # merge may have already moved past. ref: ${{ github.event.workflow_run.head_sha }} fetch-depth: 0 fetch-tags: true @@ -41,6 +47,8 @@ jobs: - name: Determine release id: rel + env: + GH_TOKEN: ${{ github.token }} run: | TOP=$(python scripts/changelog_cli.py top-version CHANGELOG.md) if [ -z "$TOP" ]; then @@ -52,52 +60,64 @@ jobs: PREV="${PREV:-v0.0.0}" KIND=$(python scripts/changelog_cli.py bump-kind CHANGELOG.md "$PREV") - TAG_EXISTS=false - git rev-parse "v$TOP" >/dev/null 2>&1 && TAG_EXISTS=true RELEASE_EXISTS=false gh release view "v$TOP" >/dev/null 2>&1 && RELEASE_EXISTS=true - echo "version=$TOP" >> "$GITHUB_OUTPUT" - echo "kind=$KIND" >> "$GITHUB_OUTPUT" - echo "tag_exists=$TAG_EXISTS" >> "$GITHUB_OUTPUT" - echo "release_exists=$RELEASE_EXISTS" >> "$GITHUB_OUTPUT" - # "release" gates the rest: a clean bump whose Release isn't published yet. + echo "version=$TOP" >> "$GITHUB_OUTPUT" + echo "kind=$KIND" >> "$GITHUB_OUTPUT" + # Gate the publish job on a clean bump whose Release isn't published yet. if [ "$KIND" != "none" ] && [ "$RELEASE_EXISTS" = "false" ]; then echo "release=true" >> "$GITHUB_OUTPUT" else echo "release=false" >> "$GITHUB_OUTPUT" echo "v$TOP: kind=$KIND release_exists=$RELEASE_EXISTS — no action." fi - env: - GH_TOKEN: ${{ github.token }} - # Each artifact is created independently and idempotently so a re-run after - # a mid-job failure can finish the release rather than skip it forever. - - name: Create tag - if: ${{ steps.rel.outputs.release == 'true' && steps.rel.outputs.tag_exists == 'false' }} - run: | - V="v${{ steps.rel.outputs.version }}" - git tag -a "$V" -m "$V" - git push origin "$V" - - - name: Build payload + - name: Build release notes if: ${{ steps.rel.outputs.release == 'true' }} run: | python scripts/changelog_cli.py payload CHANGELOG.md "${{ steps.rel.outputs.version }}" > payload.json python -c "import json; print(json.load(open('payload.json'))['notes'])" > notes.md + - name: Upload notes artifact + if: ${{ steps.rel.outputs.release == 'true' }} + uses: actions/upload-artifact@v4 + with: + name: release-notes + path: | + payload.json + notes.md + retention-days: 1 + + publish: + needs: prepare + if: ${{ needs.prepare.outputs.release == 'true' }} + runs-on: ubuntu-latest + permissions: + contents: write # create the tag + GitHub Release + id-token: write # OIDC to assume the notifier-invoke role + env: + VERSION: ${{ needs.prepare.outputs.version }} + KIND: ${{ needs.prepare.outputs.kind }} + HEAD_SHA: ${{ github.event.workflow_run.head_sha }} + steps: + - uses: actions/download-artifact@v4 + with: + name: release-notes + # Announce BEFORE publishing the Release: the Release is the durable "done" - # marker, so announcing first keeps the step retryable. Minor/major only, - # and only once the invoke-role variable has been bootstrapped (see README). + # marker (prepare skips once it exists), so announcing first keeps this + # retryable. Minor/major only, and only once the invoke-role variable has + # been bootstrapped (see README). - name: Configure AWS credentials - if: ${{ steps.rel.outputs.release == 'true' && steps.rel.outputs.kind != 'patch' && vars.RELEASE_NOTIFY_INVOKE_ROLE_ARN != '' }} + if: ${{ env.KIND != 'patch' && vars.RELEASE_NOTIFY_INVOKE_ROLE_ARN != '' }} uses: aws-actions/configure-aws-credentials@v6 with: role-to-assume: ${{ vars.RELEASE_NOTIFY_INVOKE_ROLE_ARN }} aws-region: us-east-1 - name: Announce in Slack - if: ${{ steps.rel.outputs.release == 'true' && steps.rel.outputs.kind != 'patch' && vars.RELEASE_NOTIFY_INVOKE_ROLE_ARN != '' }} + if: ${{ env.KIND != 'patch' && vars.RELEASE_NOTIFY_INVOKE_ROLE_ARN != '' }} run: | aws lambda invoke \ --function-name afterhours-release-notifier \ @@ -108,18 +128,19 @@ jobs: if grep -q '"FunctionError"' invoke-meta.json; then echo "::error::release-notifier returned an error"; cat response.json; exit 1 fi - echo "Announced v${{ steps.rel.outputs.version }}." + echo "Announced v${VERSION}." - name: Warn if announcement skipped (not bootstrapped) - if: ${{ steps.rel.outputs.release == 'true' && steps.rel.outputs.kind != 'patch' && vars.RELEASE_NOTIFY_INVOKE_ROLE_ARN == '' }} - run: echo "::warning::RELEASE_NOTIFY_INVOKE_ROLE_ARN is unset — tagged + released but did not announce. Set the repo variable from the stack output." + if: ${{ env.KIND != 'patch' && vars.RELEASE_NOTIFY_INVOKE_ROLE_ARN == '' }} + run: echo "::warning::RELEASE_NOTIFY_INVOKE_ROLE_ARN is unset — tagging + releasing but not announcing. Set the repo variable from the stack output." + # No checkout: gh creates the tag at HEAD_SHA and the Release together. - name: Publish GitHub Release - if: ${{ steps.rel.outputs.release == 'true' }} - run: | - gh release create "v${{ steps.rel.outputs.version }}" \ - --title "v${{ steps.rel.outputs.version }}" \ - --notes-file notes.md \ - --target "${{ github.event.workflow_run.head_sha }}" env: GH_TOKEN: ${{ github.token }} + run: | + gh release create "v${VERSION}" \ + --repo "${{ github.repository }}" \ + --title "v${VERSION}" \ + --notes-file notes.md \ + --target "${HEAD_SHA}" diff --git a/src/shared/shared/blocks.py b/src/shared/shared/blocks.py index 5b6ce3f..a0e5881 100644 --- a/src/shared/shared/blocks.py +++ b/src/shared/shared/blocks.py @@ -19,11 +19,15 @@ def markdown_to_mrkdwn(text: str) -> str: ``*italic*``, ``[text](url)``, ``- `` bullets), but Slack uses a different dialect (``*bold*``, ``_italic_``, ````, ``•`` bullets). Bold is swapped via a placeholder first so the italic pass cannot mangle it. + + The italic and link patterns use possessive quantifiers (``++``) and exclusive + character classes so they run in linear time on adversarial input — no + catastrophic backtracking (py/polynomial-redos). """ - text = re.sub(r"\*\*(.+?)\*\*", "\x00\\1\x00", text) # bold -> placeholder - text = re.sub(r"(? placeholder + text = re.sub(r"\*([^*\n]++)\*", r"_\1_", text) # italic (single asterisks) text = text.replace("\x00", "*") # placeholder -> Slack bold - text = re.sub(r"\[([^\]]+)\]\(([^)]+)\)", r"<\2|\1>", text) # links + text = re.sub(r"\[([^\]]++)\]\(([^)\n]++)\)", r"<\2|\1>", text) # links text = re.sub(r"(?m)^(\s*)[-*]\s+", r"\1• ", text) # bullets return text diff --git a/tests/shared/test_blocks.py b/tests/shared/test_blocks.py index c338336..2308662 100644 --- a/tests/shared/test_blocks.py +++ b/tests/shared/test_blocks.py @@ -178,6 +178,17 @@ class TestReleaseAnnouncement: assert "• see " in out assert "• next" in out + def test_adversarial_input_runs_in_linear_time(self): + # py/polynomial-redos regression: possessive quantifiers must keep these + # patterns from catastrophic backtracking. Pathological inputs that would + # hang a backtracking engine complete effectively instantly here. + import time + + for evil in ("*" + "*a" * 4000, "[" + "[\\(" * 4000, "[" + "](" * 4000): + start = time.perf_counter() + markdown_to_mrkdwn(evil) + assert time.perf_counter() - start < 1.0 + def test_blocks_have_header_and_notes(self): blocks = build_release_announcement_blocks( "1.10.0", "**Release notes** now self-announce.", "June 11, 2026"