From de41ea18832b5eb7d2e68e88aa1ea341a876a4be Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 17:21:00 +0100 Subject: [PATCH] Preserve workflow taint across branches Change-source: pulse-maintainer --- .github/workflows/create-release.yml | 2 +- .github/workflows/publish-helm-chart.yml | 6 ++-- .github/workflows/release-dry-run.yml | 2 +- .github/workflows/update-demo-server.yml | 6 ++-- scripts/check_workflow_trust.py | 18 ++++++++++- scripts/tests/test_workflow_trust.py | 39 ++++++++++++++++++++++++ 6 files changed, 64 insertions(+), 9 deletions(-) diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index fca418974..152811c3a 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -1099,7 +1099,7 @@ jobs: rm -f "$NOTES_FILE" "$RELEASE_PAYLOAD" "$RELEASE_JSON_FILE" "$ACTUAL_BODY_FILE" echo "release_url=${RELEASE_URL}" >> $GITHUB_OUTPUT - echo "release_id=${RELEASE_ID}" >> $GITHUB_OUTPUT + python3 scripts/write_github_output.py release_id "${RELEASE_ID}" echo "[OK] Draft release: ${TAG} (ID: ${RELEASE_ID})" - name: Upload checksums diff --git a/.github/workflows/publish-helm-chart.yml b/.github/workflows/publish-helm-chart.yml index 1337cf545..83763fa26 100644 --- a/.github/workflows/publish-helm-chart.yml +++ b/.github/workflows/publish-helm-chart.yml @@ -88,9 +88,9 @@ jobs: IS_PRERELEASE="true" fi - echo "chart_version=$CHART_VERSION" >> "$GITHUB_OUTPUT" - echo "app_version=$APP_VERSION" >> "$GITHUB_OUTPUT" - echo "release_tag=$RELEASE_TAG" >> "$GITHUB_OUTPUT" + python3 scripts/write_github_output.py chart_version "$CHART_VERSION" + python3 scripts/write_github_output.py app_version "$APP_VERSION" + python3 scripts/write_github_output.py release_tag "$RELEASE_TAG" echo "is_prerelease=$IS_PRERELEASE" >> "$GITHUB_OUTPUT" - name: Checkout repository diff --git a/.github/workflows/release-dry-run.yml b/.github/workflows/release-dry-run.yml index dd4f923e2..ca1a32f62 100644 --- a/.github/workflows/release-dry-run.yml +++ b/.github/workflows/release-dry-run.yml @@ -136,7 +136,7 @@ jobs: VERSION="$(tr -d '\r\n' < VERSION)" fi REQUIRED_BRANCH="$(python3 scripts/release_control/control_plane.py --branch-for-version "${VERSION}")" - echo "required_branch=${REQUIRED_BRANCH}" >> "$GITHUB_OUTPUT" + python3 scripts/write_github_output.py required_branch "${REQUIRED_BRANCH}" echo "[OK] Governed release branch for ${VERSION} is ${REQUIRED_BRANCH}" - name: Resolve rehearsal metadata diff --git a/.github/workflows/update-demo-server.yml b/.github/workflows/update-demo-server.yml index c0075119e..7e6862f26 100644 --- a/.github/workflows/update-demo-server.yml +++ b/.github/workflows/update-demo-server.yml @@ -108,7 +108,7 @@ jobs: TARGET="${REQUESTED_TARGET:-auto}" if [ -z "$TARGET" ] || [ "$TARGET" = "auto" ]; then if [ "$IS_PRERELEASE" = "true" ]; then - echo "tag=$TAG" >> "$GITHUB_OUTPUT" + python3 scripts/write_github_output.py tag "$TAG" echo "target=stable" >> "$GITHUB_OUTPUT" echo "environment_name=demo-stable" >> "$GITHUB_OUTPUT" echo "skip=true" >> "$GITHUB_OUTPUT" @@ -132,8 +132,8 @@ jobs: ;; esac - echo "tag=$TAG" >> "$GITHUB_OUTPUT" - echo "target=$TARGET" >> "$GITHUB_OUTPUT" + python3 scripts/write_github_output.py tag "$TAG" + python3 scripts/write_github_output.py target "$TARGET" echo "environment_name=$ENVIRONMENT_NAME" >> "$GITHUB_OUTPUT" echo "Resolved demo deployment: tag=${TAG}, target=${TARGET}, environment=${ENVIRONMENT_NAME}" diff --git a/scripts/check_workflow_trust.py b/scripts/check_workflow_trust.py index 10dbfc5c2..639f02d1f 100644 --- a/scripts/check_workflow_trust.py +++ b/scripts/check_workflow_trust.py @@ -34,6 +34,13 @@ SHELL_ASSIGNMENT_RE = re.compile( POWERSHELL_ASSIGNMENT_RE = re.compile( r"^\s*(?:\[[^\]\r\n]+\]\s*)?\$([A-Za-z_][A-Za-z0-9_]*)\s*=" ) +# A trusted reassignment only clears possible taint when it is guaranteed to +# execute. Assignments inside these Bash compound commands affect one branch or +# iteration, so a later command can still observe the original workflow value. +BASH_CONTROL_OPEN_RE = re.compile( + r"^\s*(?:if|case|for|select|while|until)\b" +) +BASH_CONTROL_CLOSE_RE = re.compile(r"^\s*(?:fi|esac|done)\b") # Workflow-call and dispatch inputs are data, not shell source. The legacy # github.event.inputs alias is identical data, and repository_dispatch callers # fully control client_payload. Step and job outputs are data too: they can @@ -334,8 +341,12 @@ def _audit_command_file_data( if _is_untrusted_expression(value) } unsafe_names = set(bindings) + bash_control_depth = 0 for script_index, script_line in _run_script_lines(lines, run_index): + if BASH_CONTROL_CLOSE_RE.match(script_line): + bash_control_depth = max(0, bash_control_depth - 1) + opens_bash_control = bool(BASH_CONTROL_OPEN_RE.match(script_line)) assignment = SHELL_ASSIGNMENT_RE.match(script_line) powershell_assignment = False if assignment is None: @@ -362,9 +373,14 @@ def _audit_command_file_data( for name in unsafe_names if name.casefold() != assigned_name.casefold() } - elif _bash_assignment_persists(script_line, assignment): + elif ( + bash_control_depth == 0 + and _bash_assignment_persists(script_line, assignment) + ): unsafe_names.discard(assigned_name) + if opens_bash_control: + bash_control_depth += 1 if not GITHUB_COMMAND_FILE_RE.search(script_line): continue referenced_unsafe_names = sorted( diff --git a/scripts/tests/test_workflow_trust.py b/scripts/tests/test_workflow_trust.py index 09f598e87..bfcd82298 100644 --- a/scripts/tests/test_workflow_trust.py +++ b/scripts/tests/test_workflow_trust.py @@ -491,6 +491,45 @@ jobs: 1, ) + def test_conditional_bash_reassignment_does_not_clear_possible_taint(self) -> None: + findings = self.audit( + """permissions: {} +jobs: + unsafe: + runs-on: ubuntu-24.04 + timeout-minutes: 10 + steps: + - env: + RELEASE_NAME: ${{ inputs.release_name }} + run: | + if [ "$RELEASE_NAME" = latest ]; then + RELEASE_NAME=v1.2.3 + fi + printf 'release=%s\\n' "$RELEASE_NAME" >> "$GITHUB_OUTPUT" +""" + ) + self.assertEqual( + sum("validated or encoded" in finding for finding in findings), + 1, + ) + + def test_unconditional_bash_reassignment_clears_possible_taint(self) -> None: + findings = self.audit( + """permissions: {} +jobs: + safe: + runs-on: ubuntu-24.04 + timeout-minutes: 10 + steps: + - env: + RELEASE_NAME: ${{ inputs.release_name }} + run: | + RELEASE_NAME=v1.2.3 + printf 'release=%s\\n' "$RELEASE_NAME" >> "$GITHUB_OUTPUT" +""" + ) + self.assertEqual(findings, []) + def test_powershell_trusted_reassignment_is_case_insensitive(self) -> None: findings = self.audit( """permissions: {}