Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/maint-auto-label-dep-prs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ name: Auto-label dependency PRs
# became the fleet's bumper; not a gate check, so the check-run rename is safe.

on:
# zizmor: ignore[dangerous-triggers] labels dependency-bot PRs only; does not checkout or run PR code
pull_request_target:
types: [opened]

Expand All @@ -17,6 +18,8 @@ concurrency:
jobs:
label:
# Dependabot and Renovate both produce dependency PRs that should carry agents:allow-change.
# This pull_request_target workflow must trust the immutable PR author, not
# github.actor, because actor can be a maintainer rerunning a bot-authored PR.
if: github.event.pull_request.user.login == 'dependabot[bot]' || github.event.pull_request.user.login == 'renovate[bot]'
runs-on: ubuntu-latest
steps:
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/maint-auto-lock-deps.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,14 @@ jobs:
regenerate-lock:
name: Regenerate requirements.lock
runs-on: ubuntu-latest
# This workflow runs on pull_request, not pull_request_target, so checking
# out github.head_ref is not an untrusted-code privileged checkout.
# Gate on the immutable PR author rather than github.actor, which can be a
# maintainer rerunning a bot-authored PR.
if: github.event.pull_request.user.login == 'dependabot[bot]' || github.event.pull_request.user.login == 'renovate[bot]'

steps:
# zizmor: ignore[artipacked] this backstop intentionally pushes regenerated requirements.lock
- uses: actions/checkout@v6
with:
ref: ${{ github.head_ref }}
Expand Down
40 changes: 40 additions & 0 deletions tests/workflows/test_dependency_bot_conditions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import pathlib

import yaml

WORKFLOW_ROOT = pathlib.Path(".github/workflows")
SYNC_MANIFEST = pathlib.Path(".github/sync-manifest.yml")
ACTIVE_DEPENDENCY_BOT_WORKFLOWS = (
WORKFLOW_ROOT / "maint-auto-label-dep-prs.yml",
WORKFLOW_ROOT / "maint-auto-lock-deps.yml",
)
RETIRED_CONSUMER_AUTOMERGE = pathlib.Path(
"templates/consumer-repo/.github/workflows/dependabot-automerge.yml"
)


def _workflow_source(path: pathlib.Path) -> str:
assert path.exists(), f"Expected workflow file to exist: {path}"
return path.read_text(encoding="utf-8")


def test_dependency_bot_workflows_gate_on_pr_author_not_trigger_actor():
for workflow in ACTIVE_DEPENDENCY_BOT_WORKFLOWS:
source = _workflow_source(workflow)
assert "github.event.pull_request.user.login" in source
assert "github.actor == 'dependabot[bot]'" not in source
assert "github.actor == 'renovate[bot]'" not in source
Comment on lines +25 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Harden the actor-regression assertions against equivalent syntax variants.

These negatives only block one exact string form (== 'bot'). An unsafe gate like github.actor == "dependabot[bot]" (or spacing variants) would pass this test and reintroduce spoofable-actor logic undetected.

Suggested fix
 import pathlib
+import re
 
 import yaml
@@
 def test_dependency_bot_workflows_gate_on_pr_author_not_trigger_actor():
     for workflow in ACTIVE_DEPENDENCY_BOT_WORKFLOWS:
         source = _workflow_source(workflow)
         assert "github.event.pull_request.user.login" in source
-        assert "github.actor == 'dependabot[bot]'" not in source
-        assert "github.actor == 'renovate[bot]'" not in source
+        assert not re.search(
+            r"github\.actor\s*==\s*['\"]dependabot\[bot\]['\"]",
+            source,
+        )
+        assert not re.search(
+            r"github\.actor\s*==\s*['\"]renovate\[bot\]['\"]",
+            source,
+        )

As per coding guidelines, “Flag ... spoofable bot-actor checks — this workflow YAML is synced to 9 consumer repos, so one bug replicates fleet-wide.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/workflows/test_dependency_bot_conditions.py` around lines 26 - 27, The
assertions at lines 26-27 that check for the absence of `github.actor ==
'dependabot[bot]'` and `github.actor == 'renovate[bot]'` are too narrow and only
block one exact string form with single quotes. These assertions will miss
equivalent syntax variants such as double quotes around the bot names or
different spacing patterns, allowing unsafe actor checks to slip through.
Refactor these assertions to check for multiple variant forms of the same
dangerous pattern, including both single and double quoted versions, to ensure
that any equivalent syntax variant of the bot-actor check is properly caught and
blocked.

Source: Coding guidelines



def test_auto_lock_documents_pull_request_head_ref_trust_boundary():
source = _workflow_source(WORKFLOW_ROOT / "maint-auto-lock-deps.yml")
assert "pull_request, not pull_request_target" in source
assert "github.head_ref is not an untrusted-code privileged checkout" in source


def test_retired_consumer_dependabot_automerge_template_stays_removed():
assert not RETIRED_CONSUMER_AUTOMERGE.exists()

manifest = yaml.safe_load(SYNC_MANIFEST.read_text(encoding="utf-8")) or {}
removal_targets = {entry.get("target") for entry in manifest.get("removals", [])}
assert ".github/workflows/dependabot-automerge.yml" in removal_targets
Loading