Skip to content

ci: pin every action to a commit SHA and let Dependabot update them (#1968) - #1981

Merged
JarryShaw merged 5 commits into
mainfrom
ci/1968-pin-actions
Oct 20, 2026
Merged

JarryShaw merged 5 commits into
mainfrom
ci/1968-pin-actions

Conversation

@JarryShaw

Copy link
Copy Markdown
Owner

What is the purpose of your pull request?

  • ci — workflows or build tooling

Description of your pull request and other information

#1968 part 2. Every third-party uses: is pinned to the commit its current tag points at, with an exact-version comment (@<sha> # v7.0.1), staying on the same major line. Dependabot gains a weekly github-actions ecosystem, so pins update by PR.

  • create-release.yml changes in its uses: lines only.
  • tests/_workflows.py gains pinned()/versions(). The tests that hard-coded tags now check the pinned commit and its version comment.
  • The new tests/project/test_actions_pinned_1968.py fails on any unpinned uses: or a missing ecosystem. It fails on main and passes here.
  • workflows.rst gains an "Action Pins" section, and releasing.rst and process.rst are updated.

The github-actions entry has no labels: key, like the pip entry, so Dependabot's defaults apply (dependencies, github_actions). actionlint is not installed locally, so it was not run.

Refs #1968

…1968)

- Every `uses:` of an action outside the repository now names the commit
  its tag resolved to, with the release as a `# vX.Y.Z` comment; each
  action keeps its current major line.
- `.github/dependabot.yml` adds the `github-actions` ecosystem from `/`,
  weekly like `pip`, with dependabot's default labels.
- `tests/_workflows.py` gains `pinned()` and `versions()`; the tests that
  named an action by tag now check the pin and its version comment.
- `tests/project/test_actions_pinned_1968.py` fails on an unpinned
  `uses:` or a missing `github-actions` ecosystem.
- Docs: an Action Pins section in workflows.rst, the pinned versions in
  releasing.rst, and dependabot's labels in conventions/process.rst.
@JarryShaw JarryShaw added ci Pull requests that change CI or workflow configuration (ci: subject prefix) review: running A cross-review is in flight against the current head - no verdict yet labels Oct 20, 2026
@JarryShaw JarryShaw moved this to In review in PyPCAPKit Oct 20, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 9558d4472. A Sonnet review found:

  • Pins: each SHA is the commit its stated tag points at, and each is the commit the old ref points at today, so nothing was upgraded.
  • Scope: the workflow diff touches only uses: lines.
  • Pin test: reverting one uses: to a tag makes it fail.
  • Labels: the repo already has a github_actions label, so Dependabot's default labels need nothing new. Say if you'd rather actions bumps carry ci instead.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 20, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

i thought pinned at version is already enough?

what else should Dependabot watch and update?

@JarryShaw

Copy link
Copy Markdown
Owner Author

Why a SHA and not a version. A tag such as v7 or v7.0.1 is a movable pointer. Anyone with write access to the action's repository can move it, and so can an attacker who takes that access. That is exactly what happened with tj-actions/changed-files in March 2025, when its tags were repointed to a malicious commit and every workflow using them ran it on its next build. A full commit SHA can't be moved, which is why GitHub's security-hardening guide recommends it. The # v7.0.1 comment keeps the version readable. If tags are enough for you, I'll drop the SHAs and keep the Dependabot entry.

What Dependabot watches. The pip entry at / covers only the root manifests (pyproject.toml, Pipfile). Not watched today:

  • docs/requirements.txt, the docs build pins, including the deliberately pinned sphinx-autodoc-typehints;
  • conda/requirements.txt;
  • examples/benchmark/requirements-*.txt and its Dockerfile (docker ecosystem).

conda/pypcapkit/meta.yaml has no Dependabot support. My suggestion is to add docs/ and conda/ to the pip entry, and to leave examples/benchmark/ alone. Do you want docs/ watched, given its pins are deliberate?

@JarryShaw JarryShaw added the needs: decision Waiting on the maintainer to decide — not blocked by other work label Oct 20, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

sure go with your suggestion.

im thinking benchmark can probably be watched as well? we dont normally update those and they can easily drift.

also, i think we had PyUp to do some of the dependency updates as well. might worth considering opt out that if Dependabot is already covering.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Thanks. The SHA pins stay. Dependabot's pip entry will also cover docs/, conda/ and examples/benchmark/, plus a docker entry for the benchmark's Dockerfile. .pyup.yml will stop PyUp's update PRs. Uninstalling the PyUp app, and dropping pyup.io/safety-ci if it is a required check, are repository settings, so I'll list exactly what's left for you. A new head and review follow.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed needs: decision Waiting on the maintainer to decide — not blocked by other work review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 20, 2026
…Up's updates (#1968)

- `pip` in `.github/dependabot.yml` now lists `directories:` `/`, `/docs`,
  `/conda` and `/examples/benchmark`; a `docker` entry watches
  `/examples/benchmark`.
- `.pyup.yml` sets a global `update: False` and drops its `requirements:`
  list, whose per-file default of `all` would override it.
- workflows.rst gains a Dependency Updates section; process.rst names the
  `docker` label and PyUp's opt-out.
- `test_actions_pinned_1968` pins the pip and docker directories and the
  PyUp opt-out; `test_conventions_doc_claims` expects the `docker`
  ecosystem.
@JarryShaw

Copy link
Copy Markdown
Owner Author

Pushed 5bfb27279 on top of your merge of main.

  • Dependabot: pip now covers /, /docs, /conda and /examples/benchmark, and a docker entry watches /examples/benchmark.
  • PyUp: .pyup.yml keeps update: False and no requirements: list. In pyup's code, deleting the file just runs the bot on its defaults, and any file listed there falls back to update: all.
  • pyup.io/safety-ci: not a required check. The main ruleset requires only Required checks passed and the Compat legs. Uninstalling the PyUp app stops that status; nothing else is left for you in settings.

Two of my earlier suggestions turn out to buy nothing:

  • The docker entry finds nothing to update. The benchmark uses FROM ${PYTHON_IMAGE}, and Dependabot only reads a literal image in FROM. Its digests are in python-images.txt, which is maintained by hand.
  • conda/requirements.txt is regenerated by Conda Update's Update requirements.txt step. Its next run would overwrite any Dependabot bump there.

I'd drop both, keeping /, /docs and /examples/benchmark for pip. Drop them, or keep them as they are?

@JarryShaw JarryShaw added needs: decision Waiting on the maintainer to decide — not blocked by other work review: pending No verdict for the current head - never reviewed, or the head moved since the last one review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment review: pending No verdict for the current head - never reviewed, or the head moved since the last one labels Oct 20, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

Sonnet delta review of 5bfb27279: the claims hold.

  • Dependabot: the config validates against SchemaStore's schema, and each pip directory holds a manifest.
  • PyUp: pyup's Config and RequirementConfig defaults confirm the .pyup.yml reasoning.
  • safety-ci: it is in no ruleset.
  • Conda: Conda Update rewrites conda/requirements.txt.

Three comments carry timed wording ("now", "next run"). They get fixed in the same round as your answer on dropping docker and /conda, so the verdict waits for that round.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 20, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

drop both, keeping /, /docs and /examples/benchmark for pip.

@JarryShaw

Copy link
Copy Markdown
Owner Author

Dropping the docker entry and /conda; pip keeps /, /docs and /examples/benchmark. The same round fixes the timed wording. A new head and review follow.

@JarryShaw JarryShaw removed the needs: decision Waiting on the maintainer to decide — not blocked by other work label Oct 20, 2026
- `.github/dependabot.yml` drops the `docker` entry and `/conda` from the
  `pip` directories, keeping `/`, `/docs` and `/examples/benchmark`, and
  says why the two are left out.
- workflows.rst and process.rst match, and the timed wording in
  `.pyup.yml`, workflows.rst and the pin's docstring goes.
- `test_actions_pinned_1968` pins the three pip directories and the
  absence of `/conda` and `docker`; `test_conventions_doc_claims` expects
  `pip` and `github-actions` only.
@JarryShaw

Copy link
Copy Markdown
Owner Author

Pushed 452715fa3. Dependabot pip now covers /, /docs and /examples/benchmark, alongside github-actions. The docker entry and /conda are dropped, and workflows.rst gives the reason, naming Conda Update's steps and python-images.txt. The timed wording is fixed. The pins fail if either removed entry comes back. A delta review is running.

@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 20, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 452715fa3. A Sonnet delta review found the config is exactly pip for /, /docs and /examples/benchmark, plus github-actions. The cited step names and python-images.txt exist, and the docs match. The delta adds no timed wording, and its tests pass (210 passed).

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 20, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

NEEDS CHANGES at 452715fa3. Project tests fails in test_workflow_apt_cache's test_restore_and_save_use_the_plan_and_the_same_path for engine-tests and pypcap-parity. The test still matches actions/cache@v6 by tag, and that action is now SHA-pinned. The fix goes in the test.

@JarryShaw JarryShaw added review: needs-changes Cross-review at the current head says changes are required; see the verdict comment and removed review: good-to-go Cross-review at the current head says ready; CI state is separate labels Oct 20, 2026
@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Coverage: 91.52% (unit tier, Python 3.15, 1aa2186f3, Unit Tests run success)

Package Statements Missed Branches Partial Cover
pcapkit (top level) 104 4 20 2 95.16%
pcapkit/const 19597 881 2618 867 92.11%
pcapkit/corekit 2524 70 800 36 96.57%
pcapkit/dumpkit 276 3 100 3 98.40%
pcapkit/foundation 2916 23 1016 25 98.78%
pcapkit/interface 138 4 52 3 96.32%
pcapkit/protocols 17411 224 4580 189 98.10%
pcapkit/toolkit 872 36 298 10 95.21%
pcapkit/utilities 522 2 158 4 99.12%
pcapkit/vendor 4431 1995 1018 167 51.73%

Per-file detail: the coverage-html artifact of this run.

- `test_workflow_apt_cache` finds the `actions/cache/restore` and
  `actions/cache/save` steps through `_workflows.PINNED`, so the pinned
  `@<sha> # vX.Y.Z` form is read instead of failing as `None`.
- `test_ci_timing` reads its `actions/upload-artifact` steps the same way
  rather than with a regex of its own.
@JarryShaw

Copy link
Copy Markdown
Owner Author

Pushed 1aa2186f3. test_workflow_apt_cache and test_ci_timing's upload-artifact check now find their steps through _workflows.PINNED, not a tag (85 passed). Every tests/project module and every workflow-reading test passes locally. A delta review is running.

@JarryShaw JarryShaw added review: running A cross-review is in flight against the current head - no verdict yet and removed review: needs-changes Cross-review at the current head says changes are required; see the verdict comment labels Oct 20, 2026
@JarryShaw

Copy link
Copy Markdown
Owner Author

GOOD TO GO at 1aa2186f3. A Sonnet delta review found that only how the steps are found changed, and the assertions are intact. Altering the cache path or key makes the test fail. No other test matches the real workflows by tag.

@JarryShaw JarryShaw added review: good-to-go Cross-review at the current head says ready; CI state is separate and removed review: running A cross-review is in flight against the current head - no verdict yet labels Oct 20, 2026
@JarryShaw
JarryShaw merged commit 8b90f20 into main Oct 20, 2026
57 checks passed
@JarryShaw
JarryShaw deleted the ci/1968-pin-actions branch October 20, 2026 17:43
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 20, 2026
@JarryShaw JarryShaw removed the review: good-to-go Cross-review at the current head says ready; CI state is separate label Oct 20, 2026
@JarryShaw JarryShaw added this to the PyPCAPKit v1.5 milestone Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Pull requests that change CI or workflow configuration (ci: subject prefix)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant