Skip to content

build(deps): Pin third-party pre-commit hook revs to commit SHAs - #120642

Open
timhaines wants to merge 1 commit into
getsentry:masterfrom
timhaines:build/pin-precommit-hook-revs
Open

build(deps): Pin third-party pre-commit hook revs to commit SHAs#120642
timhaines wants to merge 1 commit into
getsentry:masterfrom
timhaines:build/pin-precommit-hook-revs

Conversation

@timhaines

@timhaines timhaines commented Jul 27, 2026

Copy link
Copy Markdown

I do supply-chain and CI/CD security review. Found this reading public CI configs; nothing commercial behind it.

Four rev: lines move from a git tag to the commit that tag points at today, with the tag kept in a # frozen: comment. Same repos, same versions, no behaviour change.

Why

A tag is a mutable git ref. Whoever controls the hook repo can repoint v5.0.0 at different (potentially malicious) code, and every consumer picks it up on the next environment build. That's what happened to tj-actions/changed-files in 2025, and to aquasecurity/trivy-action in March 2026 when 76 of its 77 tags were force-pushed to code that scraped credentials off the runner (GHSA-69fq-xp46-6x23). Those are Actions, but uses: and rev: are the same kind of reference.

These four repos execute in two places. On every engineer's machine, since default_install_hook_types is ['pre-commit', 'pre-push'] and devenv sync runs prek install, so they run on every commit with whatever credentials that laptop is carrying. And in pre-commit lint, one of the five required checks on master, which mints a GitHub App token from SENTRY_INTERNAL_APP_PRIVATE_KEY (pre-commit.yml:34-40), runs the hooks, then commits whatever they modified back to the PR branch with that token (:84-90).

Every non-local uses: under .github/ is already SHA-pinned. These four rev: lines are the exception. cpython, ruff, airflow, jax and flask pin hook revs the same way. No other getsentry repo does, so I'm not asking you to roll this out anywhere else.

Tested

Against prek 0.3.6, which is what uv.lock pins.

  • prek prepare-hooks, which is CI's command and what devenv sync runs, from a cleared cache: exit 0. A well-formed but nonexistent SHA fails with fatal: unable to read tree, so that isn't a silent skip.
  • All hooks from the four repos pass. In a scratch repo with planted fixtures they still caught them.

check-jsonschema 0.29.3 is an annotated tag, so a plain git ls-remote refs/tags/0.29.3 returns the tag object rather than the commit. The pin is the dereferenced ^{}.

Also worth doing, separately

  • Bump prek. uv.lock is on 0.3.6. update --check as a CI drift gate and impostor-commit detection landed in 0.3.9, and update.freeze in 0.4.10 makes freezing the default so a stray prek update can't quietly un-pin these.
  • Add a release-age cooldown. There isn't one at any layer: no cooldown: in .github/dependabot.yml, no minimumReleaseAge in the pnpm block, no exclude-newer under [tool.uv]. cpython uses 14 days, jax 7, airflow 4. Version updates only, not security updates.
  • Optionally take the upgrades. prek update --freeze would also move check-jsonschema to 0.37.4, pre-commit-hooks to v6.0.0 and shellcheck-py to v0.11.0.1. I ran all three across the full tree and they pass with nothing modified, but that's a behaviour change and belongs in its own PR.

And now the PR template's boilerplate:

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

@timhaines
timhaines requested review from a team as code owners July 27, 2026 10:01
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 27, 2026
@timhaines

Copy link
Copy Markdown
Author

getsentry dispatch is red because a fork PR can't apply its own Trigger: getsentry tests label — could a collaborator add it? Backend tests are green.

@joshuarli tagging you since you did the prek migration in #110808 and the uses: pinning in #37166 / #111336 — this is the rev: counterpart to those, same argument, four lines. Happy to drop it if dev-infra would rather handle pre-commit revs with tooling the way pin_gha handles Actions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant