Skip to content

Pin every CI action to a commit SHA, and stop handing the workflow a write token - #15

Merged
stormer78 merged 1 commit into
mainfrom
sec-4045/ci-pinning
Sep 12, 2026
Merged

stormer78 merged 1 commit into
mainfrom
sec-4045/ci-pinning

Conversation

@stormer78

Copy link
Copy Markdown
Contributor

Why this repository in particular

The plugin is installed by cloning main
(claude plugin marketplace add OpenVTC/vta-agent-memory), and hooks/hooks.json
runs the plugin binary at every SessionStart. So whatever can write to main
can run code on every installing machine. Two things made that reachable from
CI: every third-party action was a mutable ref, and the workflow declared no
permissions: block at all, so it inherited the repository default token.

What changed in .github/workflows/ci.yml

  • Every uses: is pinned to a full 40-character commit SHA, with the release
    it corresponds to in a trailing comment:
    • actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
    • Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2
      (the commit the annotated v2.9.2 tag points at, not the tag object)
  • dtolnay/rust-toolchain@stable is gone. That was a branch ref, and
    pinning the action by SHA would have silently changed which toolchain it
    installs, because it derives that from the ref name. Hosted runners ship
    rustup, so Install Rust is now a run: step installing an explicit
    version. The version lives in one place (RUST_TOOLCHAIN), and the step fails
    if Cargo.toml's rust-version disagrees with it — so the toolchain the
    README promises is the one CI actually builds with.
  • permissions: contents: read at the top level, and
    persist-credentials: false on all three checkouts so the token is not left
    behind in .git/config. Nothing in this workflow pushes, comments or
    publishes.
  • The cache is only written from main (save-if), so a pull request can
    read it but cannot seed what later runs restore.
  • A new workflow hygiene job fails on any uses: that is not a 40-hex SHA
    (local ./… actions exempt; a docker:// image must be pinned by digest), and
    on any workflow file without a top-level permissions: block. A mutable ref
    added six months from now cannot slip back in unnoticed.

New .github/dependabot.yml

github-actions and cargo, weekly. Dependabot rewrites a SHA pin and its
version comment together, so pinning is not freezing.

cooldown: default-days: 7 holds a new release back for a week before it is
proposed: a hijacked or re-tagged release is usually noticed and pulled within
days, and waiting keeps it from arriving here as a routine bump on day one.
Security updates are not delayed by it. Cargo patch releases are grouped; minor
bumps stay separate, because for the 0.x vta-sdk family a minor bump is a
breaking one and deserves its own review.

Still needs a repository admin

Neither of these can be done from a workflow file, and both are what make the
pinning above matter:

  • Set the default workflow permissions to read-only
    (default_workflow_permissions=read, can_approve_pull_request_reviews=false).
    It is currently write.
  • Protect main (require a pull request and passing CI). It is currently
    unprotected, so a write-scoped token could push to it directly.

Checks

The guard was run locally over .github/: all four uses: refs pass, and an
unpinned @v4 / @main line fails as intended. Both changed YAML files parse
with python3 -c "yaml.safe_load(...)". No Rust source changed on this branch.

The plugin is installed by cloning main, and hooks.json runs the plugin
binary at every SessionStart, so whatever can write to main runs code on
every installing machine. Two things made that reachable from CI: every
third-party action was a mutable ref (actions/checkout@v4,
Swatinem/rust-cache@v2, and dtolnay/rust-toolchain@stable, which is a
branch), and the workflow declared no permissions block at all, so it
inherited the repository default token - which is write.

Every uses: is now a full 40-character commit SHA with the release it
corresponds to in a trailing comment:

  actions/checkout     34e114876b0b11c390a56381ad16ebd13914f8d5  v4.3.1
  Swatinem/rust-cache  6323deb102c322ba6fcbdcafc7e3dddab59af2b6  v2.9.2

The rust-cache SHA is the commit the annotated v2.9.2 tag points at, not
the tag object.

dtolnay/rust-toolchain is gone rather than pinned. It derives the
toolchain from the ref name, so pinning the branch by SHA would have
quietly frozen which toolchain CI installs. Hosted runners ship rustup,
so Install Rust is a run: step with an explicit version, and that version
lives in one place (RUST_TOOLCHAIN) with a check that Cargo.toml's
rust-version agrees. The toolchain the README promises is now the one CI
actually builds with, and a drift between the two fails the job instead
of going unnoticed.

Least privilege, because none of this workflow pushes, comments or
publishes: permissions: contents: read at the top level, and
persist-credentials: false on all three checkouts so the token is not
left behind in .git/config. The cache is written only from main, so a
pull request can read it but cannot seed what later runs restore.

A new "workflow hygiene" job keeps it that way: it fails on any uses: that
is not a 40-hex SHA (local ./ actions exempt, a docker:// image must be
pinned by digest) and on any workflow without a top-level permissions
block. Pinning that nothing enforces lasts until the next person adds a
step.

dependabot.yml covers github-actions and cargo, weekly. Dependabot
rewrites a SHA pin and its version comment together, so pinning is not
freezing. cooldown holds a release back for seven days before proposing
it: a hijacked or re-tagged release is usually noticed and pulled within
days, and this keeps it from arriving as a routine bump on day one.
Security updates are not delayed by it.

Two things still need a repository admin, and they are what make the
pinning matter: set the default workflow permissions to read-only, and
protect main so a write token cannot push to it directly.

Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
@stormer78
stormer78 merged commit ae5d31d into main Sep 12, 2026
4 checks passed
@stormer78
stormer78 deleted the sec-4045/ci-pinning branch September 12, 2026 05:48
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.

1 participant