Skip to content

ci: guard pr-notify against Dependabot runs explicitly - #744

Merged
jamesnrokt merged 1 commit into
mainfrom
fix/ci-skip-dependabot-notify
Jul 30, 2026
Merged

ci: guard pr-notify against Dependabot runs explicitly#744
jamesnrokt merged 1 commit into
mainfrom
fix/ci-skip-dependabot-notify

Conversation

@jamesnrokt

Copy link
Copy Markdown
Collaborator

Background

Dependabot-triggered runs resolve secrets against a separate, repository-scoped store that GCHAT_PRS_WEBHOOK is not in, so on a Dependabot run gchat_webhook arrives blank and the notifier fails with Input required and not supplied: webhookUrl.

That does not happen today — but only incidentally. pr-notify needs security-checks; the shared security-checks workflow already guards on this same condition, so it skips on Dependabot PRs; and because pr-notify has no always(), a skipped dependency skips it too. Verified as skipping rather than failing on #722, #720 and #705.

Nothing local to this repo expresses that constraint, so two ordinary edits would silently turn every Dependabot PR red: adding always() to pr-notify (a reasonable-looking change, used elsewhere to keep a notifier alive when an upstream job is skipped by a path filter), or dropping security-checks from its needs while reshuffling required checks.

What changed

  • pull-request.yml — added github.actor != 'dependabot[bot]' to the pr-notify job's if.

github.actor is what GitHub uses to decide which secrets store a run reads from, so the guard matches the exact circumstance under which the webhook is unavailable. A human pushing to a Dependabot branch still gets the notification.

Validation

actionlint and trunk check clean. No behaviour change: pr-notify already skipped on Dependabot PRs and still does; it already ran on human PRs and still does. This only makes the existing protection intentional and local rather than a side effect of the needs graph.

Being human-authored, this PR exercises the happy path — Notify GChat running and succeeding here is the regression signal.

Dependabot-triggered runs resolve secrets against a separate, repository-scoped store that GCHAT_PRS_WEBHOOK is not in, so the notifier would fail with "Input required and not supplied: webhookUrl".

That does not happen today, but only incidentally: pr-notify needs security-checks, the shared security-checks workflow already guards on this same condition, and a skipped dependency skips this job. Adding always() to pr-notify, or dropping security-checks from its needs, would silently turn every Dependabot PR red.

github.actor is what determines which secrets store a run reads from, so a human pushing to a Dependabot branch is unaffected. No behaviour change - this makes the existing protection intentional and local.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jamesnrokt
jamesnrokt requested a review from a team as a code owner July 29, 2026 21:13
@cursor

cursor Bot commented Jul 29, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
CI-only workflow condition with no intended behavior change on human-authored PRs.

Overview
Adds github.actor != 'dependabot[bot]' to the pr-notify job’s if in pull-request.yml, so Notify GChat is skipped when the workflow run is triggered by Dependabot.

That documents locally why Dependabot PRs should not hit the GChat notifier: Dependabot runs use a different secrets scope, so GCHAT_PRS_WEBHOOK would be empty and the reusable workflow would fail on a missing webhookUrl. Skipping was already happening indirectly via security-checks in needs; this guard keeps the same runtime behavior if someone later adds always() or reshuffles needs.

Reviewed by Cursor Bugbot for commit c8fe400. Bugbot is set up for automated code reviews on this repo. Configure here.

@sonarqubecloud

Copy link
Copy Markdown

@jamesnrokt
jamesnrokt merged commit c2649a8 into main Jul 30, 2026
42 of 43 checks passed
@jamesnrokt
jamesnrokt deleted the fix/ci-skip-dependabot-notify branch July 30, 2026 14:20
jamesnrokt added a commit that referenced this pull request Jul 30, 2026
…746)

* ci: remove hardcoded secrets check from PR workflow

Trunk already runs trufflehog at the PR level (trufflehog@3.90.6 in
.trunk/trunk.yaml), alongside a custom mparticle-api-key-check rule, so
this job was duplicating secret scanning that Trunk Check already covers.

The job was also intermittently red -- 3 failures in 70 recent runs, always
in ~5s. It failed on #742 while passing on #743 and #744 with identical
config. The cause is in the reusable workflow, which resolves and downloads
"latest" trufflehog at runtime by curling the GitHub releases API and
grepping the response; that step breaks when the API rate-limits. Trunk
pins its trufflehog version and has no runtime download, so it does not
share this failure mode.

Nothing in the workflow depends on the removed job; no other job referenced
it via needs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* ci: enable trufflehog-git to scan the commits a PR adds

The already-enabled trufflehog linter runs `trufflehog filesystem` against
changed files in the working tree, so it cannot see a secret that was
committed and then removed in a later commit on the same branch -- the file
no longer exists to scan.

trufflehog-git closes that gap. It runs `trufflehog git --since-commit
${upstream-ref}`, walking only the commits the PR adds, so it stays fast
(~1.7s) and does not resurface historical findings.

Demonstrated with a private key committed then removed on a branch:

  filesystem scan  -> missed it (only the pre-existing daily.yml FP)
  git-history scan -> PrivateKey <- probe-key.pem @ f48c6617

Both linters keep Trunk's default --only-verified, so this widens scan
scope, not the reporting threshold.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants