From c0ff40e292888a30cb232737ee314d696648b139 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Thu, 12 Nov 2026 00:20:29 +0000 Subject: [PATCH] ci(release): open the release pull request with the delivery app Release Please used github.token, so the release pull request it opened started pull request runs only after manual approval, and the organization auto-merge workflow did not treat it as the delivery app's. It now mints the delivery app token, as SQLBuild does, and passes it to release-please-action, keeping github.token when the App's variable is not set. Publish and the dispatched release checks keep github.token. The clutter guard allows the App's key name only in the release workflow, and only while that workflow has no pull request trigger. --- .github/workflows/release-please.yml | 24 +++++++++-- scripts/clutter_guard/_helpers/content.py | 16 ++++++- scripts/clutter_guard/constants.py | 7 +++ tests/unit/scripts/clutter_guard/helpers.py | 43 +++++++++++++++++++ .../unit/scripts/clutter_guard/test_check.py | 28 ++++++++++++ 5 files changed, 114 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index 0416ae7e2..ec9341706 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -22,9 +22,27 @@ jobs: release-please: runs-on: ubuntu-24.04 steps: + # The organization's delivery App opens and updates the release pull request, so GitHub + # emits its pull request events and the organization auto-merge workflow treats it as its own. + # Without the App's variable, Release Please keeps github.token. + - name: Create delivery token + id: delivery-token + if: vars.CHIO_DELIVERY_APP_ID != '' + uses: actions/create-github-app-token@v2 + with: + app-id: ${{ vars.CHIO_DELIVERY_APP_ID }} + private-key: ${{ secrets.CHIO_DELIVERY_PRIVATE_KEY }} + owner: ${{ github.repository_owner }} + repositories: ${{ github.event.repository.name }} + permission-actions: write + permission-contents: write + permission-issues: write + permission-pull-requests: write + - uses: googleapis/release-please-action@v4 id: release with: + token: ${{ steps.delivery-token.outputs.token || github.token }} config-file: release-please-config.json manifest-file: .release-please-manifest.json @@ -37,9 +55,9 @@ jobs: gh workflow run publish.yml --repo "${GITHUB_REPOSITORY}" --ref main \ -f "tag=${RELEASE_TAG}" - # A pull request this workflow opens starts no pull_request runs, so the release pull - # request's required checks are dispatched on its branch: CI replays every recorded case - # before the release, alongside the distribution build. + # The release pull request's required checks are dispatched on its branch, since a pull request + # github.token opens starts no pull_request runs: CI replays every recorded case before the + # release, alongside the distribution build. - name: Check the release pull request and build its distributions if: steps.release.outputs.prs_created == 'true' env: diff --git a/scripts/clutter_guard/_helpers/content.py b/scripts/clutter_guard/_helpers/content.py index a5768cb8b..c8e12f931 100644 --- a/scripts/clutter_guard/_helpers/content.py +++ b/scripts/clutter_guard/_helpers/content.py @@ -14,6 +14,7 @@ CLASSES_NOT_COUNTED_IN_RECORD_DATA, CONTENT_PATTERNS, ENVIRONMENT_VARIABLE_PROSE_PATTERN, + PULL_REQUEST_TRIGGER_PATTERN, RECORD_DATA_FIELD_PATTERN, SECRET_PART_PAIRS, SECRET_PART_SEARCH_PATTERN, @@ -23,6 +24,7 @@ SNIPPET_LIMIT, VARIABLE_NAME_CHARACTERS, VARIABLE_NAME_PATTERN, + WORKFLOW_SECRET_NAMES, ) from scripts.clutter_guard.models import Finding @@ -43,7 +45,11 @@ def text_findings(*, path: str, text: str) -> list[Finding]: spans: list[tuple[str, int, int]] = [] for clutter_class, pattern in CONTENT_PATTERNS: spans.extend(_pattern_spans(text=text, clutter_class=clutter_class, pattern=pattern)) - spans.extend((SECRET_VARIABLE_NAME, *span) for span in _secret_variables(text)) + spans.extend( + (SECRET_VARIABLE_NAME, *span) + for span in _secret_variables(text) + if not _workflow_secret(path=path, name=text[span[0] : span[1]], text=text) + ) found: dict[tuple[int, str], Finding] = {} line: int = 1 counted_to: int = 0 @@ -125,6 +131,14 @@ def _is_secret_variable(*, text: str, start: int, end: int) -> bool: ) +def _workflow_secret(*, path: str, name: str, text: str) -> bool: + """Whether `name` is a secret only `path` may read, and `path` has no pull request trigger.""" + return ( + WORKFLOW_SECRET_NAMES.get(name) == path + and re.search(PULL_REQUEST_TRIGGER_PATTERN, text) is None + ) + + def _snippet(*, text: str, start: int, end: int) -> str: line_start: int = text.rfind("\n", 0, start) + 1 line_end: int = text.find("\n", end) diff --git a/scripts/clutter_guard/constants.py b/scripts/clutter_guard/constants.py index 9bb413993..e8e5e3781 100644 --- a/scripts/clutter_guard/constants.py +++ b/scripts/clutter_guard/constants.py @@ -134,6 +134,13 @@ AFTER_ASSIGNMENT_PATTERN: str = r"^(?:=|\s*[?:+]=|:\s*[\"']?\$)" ENVIRONMENT_VARIABLE_PROSE_PATTERN: str = r"(?i)\benv(?:ironment)?[ _-]?var(?:iable)?s?\b" ALLOWED_SECRET_NAMES: frozenset[str] = frozenset({"GITHUB_TOKEN", "GH_TOKEN"}) +WORKFLOW_SECRET_NAMES: dict[str, str] = { + "CHIO_DELIVERY_PRIVATE_KEY": ".github/workflows/release-please.yml", +} +PULL_REQUEST_TRIGGER_PATTERN: str = ( + r"(?m)^(?:on:\s*(?:\[[^\]\n]*\bpull_request|pull_request)" + r"|\s+(?:-\s+)?pull_request(?:_target)?\s*(?::|$))" +) BINARY_PROBE_BYTES: int = 8192 SNIPPET_CONTEXT: int = 30 diff --git a/tests/unit/scripts/clutter_guard/helpers.py b/tests/unit/scripts/clutter_guard/helpers.py index a4743aefe..b4fa9fa03 100644 --- a/tests/unit/scripts/clutter_guard/helpers.py +++ b/tests/unit/scripts/clutter_guard/helpers.py @@ -18,6 +18,7 @@ _HOME: str = "/" + "home/alice/" _SECRET_NAME: str = "SERVICE_" + "TOKEN" _GITHUB_SECRET_NAME: str = "GITHUB_" + "TOKEN" +_DELIVERY_SECRET_NAME: str = "CHIO_DELIVERY_" + "PRIVATE_KEY" _REPOSITORY_GITIGNORE: Path = Path(__file__).resolve().parents[4] / ".gitignore" _IGNORED_CLUTTER: tuple[str, ...] = ( "AGENTS.md", @@ -229,6 +230,48 @@ def add_github_token(repository: Path) -> None: ) +def _delivery_workflow(triggers: str) -> str: + """A workflow with `triggers` that reads the delivery App key, mentioning pull_request runs.""" + return ( + f"on:\n{triggers}\n" + "# A pull request it opens starts no pull_request runs.\n" + "jobs:\n release:\n steps:\n - with:\n" + f" private-key: ${{{{ secrets.{_DELIVERY_SECRET_NAME} }}}}\n" + ) + + +def add_release_workflow_delivery_key(repository: Path) -> None: + """Adds the delivery App key to the release workflow, on push and dispatch only.""" + _write( + repository, + ".github/workflows/release-please.yml", + _delivery_workflow(" push:\n workflow_dispatch:"), + ) + + +def add_delivery_key_in_another_workflow(repository: Path) -> None: + """Adds the delivery App key to a workflow other than the release workflow.""" + _write(repository, ".github/workflows/ci.yml", _delivery_workflow(" push:")) + + +def add_delivery_key_with_pull_request_trigger(repository: Path) -> None: + """Adds the delivery App key to the release workflow, which also runs on pull requests.""" + _write( + repository, + ".github/workflows/release-please.yml", + _delivery_workflow(" push:\n pull_request:"), + ) + + +def add_delivery_key_with_pull_request_target_trigger(repository: Path) -> None: + """Adds the delivery App key to the release workflow, which also runs on pull_request_target.""" + _write( + repository, + ".github/workflows/release-please.yml", + _delivery_workflow(" push:\n pull_request_target:\n types: [opened]"), + ) + + def add_sql_system_function(repository: Path) -> None: """Adds SQL calling a Snowflake system function whose name has a secret-ish part.""" _write(repository, "docs/sql.md", f"SELECT SYSTEM${_SECRET_NAME}();\n") diff --git a/tests/unit/scripts/clutter_guard/test_check.py b/tests/unit/scripts/clutter_guard/test_check.py index eee91570c..280456c4e 100644 --- a/tests/unit/scripts/clutter_guard/test_check.py +++ b/tests/unit/scripts/clutter_guard/test_check.py @@ -15,6 +15,9 @@ add_case_with_digest_alias, add_case_with_digest_alias_and_decision, add_decision, + add_delivery_key_in_another_workflow, + add_delivery_key_with_pull_request_target_trigger, + add_delivery_key_with_pull_request_trigger, add_github_token, add_hex_and_recorded_setup, add_home_path, @@ -24,6 +27,7 @@ add_placeholder_paths, add_product_named_source, add_release_changelog, + add_release_workflow_delivery_key, add_review_record, add_reviewer_word, add_run_record, @@ -203,6 +207,30 @@ expected_return_code=0, expected_fragment="passed, 0 findings", ), + GuardTestCase( + description="the release workflow reading the delivery App key passes", + change=add_release_workflow_delivery_key, + expected_return_code=0, + expected_fragment="passed, 0 findings", + ), + GuardTestCase( + description="the delivery App key in any other workflow is rejected", + change=add_delivery_key_in_another_workflow, + expected_return_code=1, + expected_fragment=".github/workflows/ci.yml:8: secret-variable-name:", + ), + GuardTestCase( + description="the delivery App key in a release workflow with a pull_request trigger is rejected", + change=add_delivery_key_with_pull_request_trigger, + expected_return_code=1, + expected_fragment=".github/workflows/release-please.yml:9: secret-variable-name:", + ), + GuardTestCase( + description="the delivery App key in a release workflow on pull_request_target is rejected", + change=add_delivery_key_with_pull_request_target_trigger, + expected_return_code=1, + expected_fragment=".github/workflows/release-please.yml:10: secret-variable-name:", + ), GuardTestCase( description="a SQL system function with a secret-ish part passes", change=add_sql_system_function,