Skip to content

ci: drop the third-party step after the deploy key and declare workflow permissions (#1968) - #1970

Merged
JarryShaw merged 2 commits into
mainfrom
ci/1968-harden-workflows
Oct 20, 2026
Merged

JarryShaw merged 2 commits into
mainfrom
ci/1968-harden-workflows

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

Part 1 of #1968, covering the code-side items that were accepted.

  • Vendor Update and Conda Update. tj-actions/verify-changed-files@v17 becomes a git status --porcelain step with the same id and outputs. No third-party code runs after the deploy key is loaded. The commit steps now read the file list through env: rather than ${{ }}.

  • Create Release and CodeQL. Both get top-level permissions: {}. Every job keeps its own block, so no job's permissions change. In create-release.yml the line takes the place of the commented-out #release: trigger, which keeps the docs' line citations stable.

  • GitHub Pages. The top level becomes contents: read, and the build, including on pull requests, is read-only. A new publish-pages job, which never runs on a pull request, holds contents: write and pushes the built artifact.

  • Pins. These cover:

    • no third-party action after the deploy key;
    • the new step matching v17's outputs in a scratch repo;
    • every workflow declaring top-level permissions;
    • the Pages split.

    Each fails against main's files.

SHA-pinning and Dependabot come after #1958 phase 1. The secret moves are repository settings. The Pages publish path is only proven by a real run on main.

Refs #1968

…ow permissions (#1968)

- cron-vendor/cron-conda: replace tj-actions/verify-changed-files@v17 with a
  `git status --porcelain` step that writes the same outputs; the commit steps
  read the file list through `env:` instead of `${{ }}` in the script
- create-release, CodeQL: top-level `permissions: {}`; every job keeps its own
- deploy-pages: top-level `contents: read`; the push to gh-pages moves to a
  `publish-pages` job holding `contents: write`, which never runs on a PR
- tests: pin no third-party action after the deploy key, the step's outputs
  against v17's, top-level permissions in every workflow, and the Pages split

Refs #1968
@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

NEEDS CHANGES at 66f03078b, one regression. A Sonnet review confirmed most of the PR: the new step matches v17's outputs, path data reaches run: only through env:, and every job declares the permissions it uses. The Pages artifact keeps .nojekyll, and 18 of the 22 new pins fail on main's files. The one problem is the split Pages job. publish-pages has its own publish-pages-<ref> group with cancel-in-progress: false. If two builds finish out of order, the older run can publish last and leave gh-pages stale, which the old single job, serialised under ci-<ref>, could not do. A follow-up commit makes the publish skip when its commit is no longer the ref's head.

@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
- deploy-pages: each deploy names its commit in the gh-pages commit message,
  and `publish-pages` first reads the commit gh-pages last published; when
  this run's commit is behind it on main, the deploy is skipped with a notice.
  Anything it cannot read publishes, as before
- tests: run the check against a stubbed `gh`, and pin that the deploy waits
  on it and writes the message it reads

Refs #1968
@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 7a4a2cf71. A Sonnet delta review checked the new freshness step. It compares gh-pages' recorded build with this run, as compare/{published}...{run}, and skips only on behind. A failed gh call, an unrecognised message, the same sha, or a diverged history all publish, so no failure path blocks a publish. GitHub's concurrency docs confirm the rejected shared group would cancel the pending job. Path data still reaches the shell only through env:. There is still one edge, inherent to concurrency queuing: with three overlapping runs, a pending newer run can be cancelled. The next push repairs it.

@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 c68a5ac into main Oct 20, 2026
58 checks passed
@github-project-automation github-project-automation Bot moved this from In review to Done in PyPCAPKit Oct 20, 2026
@JarryShaw
JarryShaw deleted the ci/1968-harden-workflows branch October 20, 2026 02:06
@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