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

.github/workflows/codeql-analysis.yml

@@ -9,6 +9,9 @@ on:
   schedule:
     - cron: '0 2 * * 6'
 
+# #1968: no scopes by default. `analyze` declares the ones it needs.
+permissions: {}
+
 # Only pull-request runs may be superseded -- see the note in unit-tests.yml. A
 # ref-keyed group drops a *pending* run when a newer one joins it, which for a
 # security scan means a commit reaching main with no analysis on record.

.github/workflows/create-release.yml

@@ -7,8 +7,8 @@ on:
     workflows: [ "Vendor Update" ]
     types:
       - completed
-  #release:
-  #  types: ['published']
+# #1968: no scopes for a job without `permissions:` of its own. Each job has one.
+permissions: {}
 
 name: Create Release
 

.github/workflows/cron-conda.yml

@@ -192,9 +192,21 @@ jobs:
 
           python util/conda_vendor.py
 
+      # #1968: `git status` in place of tj-actions/verify-changed-files@v17, so
+      # no third-party code runs while the deploy key is loaded. The script and
+      # its outputs are cron-vendor.yml's, whose comment gives the rules.
       - name: Verify Changed files
-        uses: tj-actions/verify-changed-files@v17
         id: verify-changed-files
+        run: |
+          listing="$(git -c core.quotepath=on status --porcelain --untracked-files=all --no-renames)"
+          printf '%s\n' "$listing"
+          changed="$(printf '%s\n' "$listing" | cut -c4- | LC_ALL=C sort -u | paste -sd ' ' -)"
+          if [ -n "$changed" ]; then
+            echo "files_changed=true" >> "$GITHUB_OUTPUT"
+            echo "changed_files=$changed" >> "$GITHUB_OUTPUT"
+          else
+            echo "files_changed=false" >> "$GITHUB_OUTPUT"
+          fi
 
       - name: Bump Version
         if: steps.verify-changed-files.outputs.files_changed == 'true'
@@ -203,13 +215,15 @@ jobs:
 
       - name: Commit Changes
         if: steps.verify-changed-files.outputs.files_changed == 'true'
+        env:
+          CHANGED_FILES: ${{ steps.verify-changed-files.outputs.changed_files }}
         run: |
           git config --local user.email "github-actions[bot]@users.noreply.github.com"
           git config --local user.name "github-actions[bot]"
           git commit -am"Bumped build to $(cat conda/build)
 
           Updated conda distribution at $(date). The following files have been changed:
-            ${{ steps.verify-changed-files.outputs.changed_files }}"
+            $CHANGED_FILES"
 
       # Writes `pushed` and `commit`, which `CONDA_CHANGED` and `CONDA_COMMIT`
       # below hand to `conda-tag` and `conda-dist` (#1632).

.github/workflows/cron-vendor.yml

@@ -297,9 +297,31 @@ jobs:
             echo 'The step that tests the regenerated tree failed or timed out, so nothing from this crawl is committed.'
           } >> "$GITHUB_STEP_SUMMARY"
 
+      # #1968: `git status` in place of tj-actions/verify-changed-files@v17, so
+      # no third-party code runs while the deploy key is loaded. The outputs are
+      # the action's with no `files:` input, which it never had here:
+      # `files_changed`, and `changed_files`, the paths joined by spaces and
+      # written only when there are some. Every changed tracked file counts,
+      # untracked files are listed one by one, and ignored files are left out.
+      # A staged change counts too, which the action's `git diff` missed;
+      # nothing before this step stages one. git C-quotes a path holding a
+      # space, a quote, a control character or a non-ASCII byte, so the list is
+      # one line. Such a path still splits on its space, and
+      # util/vendor_code_changed.py counts a name it cannot read as code. The
+      # steps below read the list through `env:`, so a path is never shell text.
+      # cron-conda.yml runs the same script.
       - name: Verify Changed files
-        uses: tj-actions/verify-changed-files@v17
         id: verify-changed-files
+        run: |
+          listing="$(git -c core.quotepath=on status --porcelain --untracked-files=all --no-renames)"
+          printf '%s\n' "$listing"
+          changed="$(printf '%s\n' "$listing" | cut -c4- | LC_ALL=C sort -u | paste -sd ' ' -)"
+          if [ -n "$changed" ]; then
+            echo "files_changed=true" >> "$GITHUB_OUTPUT"
+            echo "changed_files=$changed" >> "$GITHUB_OUTPUT"
+          else
+            echo "files_changed=false" >> "$GITHUB_OUTPUT"
+          fi
 
       # #1674: only a code change cuts a beta. util/vendor_code_changed.py,
       # whose docstring gives the rule, writes `code_changed=false` when every
@@ -332,6 +354,8 @@ jobs:
         if: >-
           steps.verify-changed-files.outputs.files_changed == 'true' &&
           steps.classify.outputs.code_changed != 'false'
+        env:
+          CHANGED_FILES: ${{ steps.verify-changed-files.outputs.changed_files }}
         run: |
           git config --local user.email "github-actions[bot]@users.noreply.github.com"
           git config --local user.name "github-actions[bot]"
@@ -345,7 +369,7 @@ jobs:
           git commit -am"Bumped version to $(python -c 'import pcapkit; print(pcapkit.__version__)')
 
           Updated vendor constant enumerations at $(date). The following files
-          have been changed: ${{ steps.verify-changed-files.outputs.changed_files }}"
+          have been changed: $CHANGED_FILES"
 
       # #1674: the same commit without the bump. Create Release still runs when
       # this workflow completes, as after a crawl that changed nothing, and sees

.github/workflows/deploy-pages.yml

@@ -21,7 +21,7 @@ on:
     branches: [main]
 
 permissions:
-  contents: write
+  contents: read
 
 # Deliberately still keyed on the ref, unlike the validation workflows. This one
 # publishes a *latest state* rather than a verdict on a commit, so coalescing is
@@ -173,11 +173,88 @@ jobs:
           # disable Jekyll
           touch docs/build/html/.nojekyll
 
+      # `.nojekyll` is hidden, and upload-artifact leaves hidden files out unless
+      # told otherwise.
+      - name: Upload the built site
+        if: ${{ github.event_name != 'pull_request' }}
+        uses: actions/upload-artifact@v7
+        with:
+          name: pages-html
+          path: docs/build/html
+          include-hidden-files: true
+          retention-days: 1
+
+  # #1968: the push to `gh-pages`, split out of `deploy-pages` so that the job
+  # running the tree's own code, a pull request's included, holds only the
+  # top-level `contents: read`. This job runs no project code, and never on a
+  # pull request. It needs `contents: write` to push `gh-pages`, and `actions:
+  # read` to download the site `deploy-pages` uploaded in this run. That job's
+  # result is checked explicitly, under `!cancelled()`: `unit-tests` or `verdict`
+  # always skips, and a skip anywhere in the chain skips a job left with the
+  # implicit `success()` (#1632). Its own group keeps one push to `gh-pages` at
+  # a time, and its check below keeps an older build from replacing a newer one.
+  publish-pages:
+    name: Publish to gh-pages
+    needs: [ deploy-pages ]
+    if: >-
+      !cancelled() && needs.deploy-pages.result == 'success' &&
+      github.event_name != 'pull_request'
+    concurrency:
+      group: publish-pages-${{ github.ref }}
+      cancel-in-progress: false
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    permissions:
+      actions: read
+      contents: write
+    steps:
+      # The deploy action pushes from a checkout of this repository.
+      - name: Checkout 🛎️
+        uses: actions/checkout@v7
+        with:
+          persist-credentials: false
+
+      - name: Download the built site
+        env:
+          GH_TOKEN: ${{ github.token }}
+        run: >-
+          gh run download "$GITHUB_RUN_ID" --repo "$GITHUB_REPOSITORY" --name pages-html
+          --dir docs/build/html
+
+      # Builds finish in any order, and one group per job cannot order them: a
+      # job joining a group cancels the one pending there, so sharing the build's
+      # group would cancel the next build. So each deploy names its commit, and
+      # this step reads the commit `gh-pages` last published. Where that one is
+      # newer than this run's -- this run's commit is behind it on `main` -- the
+      # deploy is skipped. Anything else publishes, as before: no `gh-pages`, a
+      # head this workflow did not write, an API error, the same commit, or one
+      # that is not an ancestor. A head check alone would also skip when the
+      # newer commit's own run never publishes, a red one, leaving the docs
+      # older than this build.
+      - name: Check gh-pages holds no newer build
+        id: fresh
+        env:
+          GH_TOKEN: ${{ github.token }}
+        run: |
+          published="$(gh api "repos/$GITHUB_REPOSITORY/commits/gh-pages" --jq .commit.message |
+                       sed -n 's/^Publish the documentation of \([0-9a-f]\{40\}\)$/\1/p' | head -n 1)"
+          if [ -n "$published" ] && [ "$published" != "$GITHUB_SHA" ]; then
+            status="$(gh api "repos/$GITHUB_REPOSITORY/compare/$published...$GITHUB_SHA" --jq .status || true)"
+            if [ "$status" = behind ]; then
+              echo "::notice title=Newer documentation already published::gh-pages holds the build of $published, which $GITHUB_SHA is behind on main, so this older build is not published"
+              echo "publish=false" >> "$GITHUB_OUTPUT"
+              exit 0
+            fi
+          fi
+          echo "publish=true" >> "$GITHUB_OUTPUT"
+
       - name: Deploy 🚀
+        if: steps.fresh.outputs.publish == 'true'
         uses: JamesIves/github-pages-deploy-action@v4
-        if: ${{ github.event_name != 'pull_request' }}
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
           branch: gh-pages # The branch the action should deploy to.
           folder: docs/build/html # The folder the action should deploy.
           clean: true # Automatically remove deleted files from the deploy branch
+          # Read back by the step above; keep the two in step.
+          commit-message: Publish the documentation of ${{ github.sha }}

docs/source/contributing/workflows.rst

@@ -1013,9 +1013,15 @@ approval, in ``create-release.yml`` and ``cron-python-versions.yml``:
   `Python Versions`_)
 
 No other workflow in scope here declares one -- including
-``deploy-pages.yml``, whose ``deploy-pages`` job pushes straight to the
+``deploy-pages.yml``, whose ``publish-pages`` job pushes straight to the
 ``gh-pages`` branch via ``JamesIves/github-pages-deploy-action`` rather than
-through an ``environment:``-gated deployment. Of these environments, only
+through an ``environment:``-gated deployment. Its ``deploy-pages`` job builds
+the site, a pull request's included, with ``contents: read``, and uploads it
+for ``publish-pages``, which alone holds ``contents: write`` and never runs on
+a pull request. Its ``Check gh-pages holds no newer build`` step skips the
+deploy when ``gh-pages`` already holds the build of a commit this run's commit
+is behind, so a build that finishes late cannot replace newer documentation
+(`#1968 <https://github.com/JarryShaw/PyPCAPKit/issues/1968>`__). Of these environments, only
 ``github-release`` carries a required reviewer; :doc:`releasing`
 covers why one approval on that single environment is enough to gate the
 whole release graph, and is the page to read for the reasoning rather than

tests/project/test_conda_platform_pin_refresh_1851.py

@@ -662,7 +662,7 @@ def test_the_refresh_runs_between_the_install_and_the_writer(self) -> None:
         refresh = self.index(lambda step: self.runs(step) == ['python util/conda_vendor.py --refresh-pins'])
         install = self.index(lambda step: 'python -m pip install -e .[all]' in self.runs(step))
         writer = self.index(lambda step: 'python util/conda_vendor.py' in self.runs(step))
-        changed = self.index(lambda step: str(step.get('uses', '')).startswith('tj-actions/verify-changed-files@'))
+        changed = self.index(lambda step: step.get('id') == 'verify-changed-files')
         self.assertLess(install, refresh)
         self.assertLess(refresh, writer)
         self.assertLess(writer, changed)

tests/project/test_vendor_commits_new_modules_1685.py

@@ -3,13 +3,14 @@
 
 ``Commit changes`` in :file:`.github/workflows/cron-vendor.yml` ran ``git commit
 -am``, which stages modified and deleted tracked files and never an untracked
-one. ``tj-actions/verify-changed-files@v17`` reports untracked files too (the
-``??`` entries of ``git status -u --porcelain``), so a crawl that generated a
-module for a new registry bumped the version and left the module out of the
-commit. Every later crawl found it untracked again, and bumped again.
-
-:class:`TestTheBumpCommitStep` runs that step's script in a throwaway repository
-the way the runner runs it, with ``${{ }}`` substituted first. The second test
+one. ``Verify Changed files`` reports untracked files too (the ``??`` entries of
+``git status -u --porcelain``), so a crawl that generated a module for a new
+registry bumped the version and left the module out of the commit. Every later
+crawl found it untracked again, and bumped again.
+
+:class:`TestTheBumpCommitStep` runs that step's script, and ``Verify Changed
+files``', in a throwaway repository the way the runner runs them, with ``${{ }}``
+substituted first. The second test
 classifies a new module with :file:`util/vendor_code_changed.py` and walks the
 steps, which shows ``Commit documentation changes``, still ``-a`` alone, never
 sees one. The harness is #1674's, from
@@ -78,10 +79,11 @@ def setUp(self) -> None:
         self.commit('chore(const): the package marker', HUMAN)
         self.base = self.git('rev-parse', 'HEAD')
 
-    def reported(self) -> 'list[str]':
-        """What ``verify-changed-files@v17`` reports: its ``entrypoint.sh`` with no
-        ``files`` pattern, so ``git diff --diff-filter=ACMUXTR``, the ``??`` lines
-        of ``git status -u --porcelain`` and ``git ls-files --deleted``, de-duplicated.
+    def v17(self) -> 'list[str]':
+        """What ``tj-actions/verify-changed-files@v17``, which ``Verify Changed
+        files`` replaced (#1968), reported: its ``entrypoint.sh`` with no ``files``
+        pattern, so ``git diff --diff-filter=ACMUXTR``, the ``??`` lines of ``git
+        status -u --porcelain`` and ``git ls-files --deleted``, de-duplicated.
 
         """
         tracked = self.git('diff', '--diff-filter=ACMUXTR', '--name-only').splitlines()
@@ -90,6 +92,28 @@ def reported(self) -> 'list[str]':
         deleted = self.git('ls-files', '--deleted').splitlines()
         return sorted(set(tracked + untracked + deleted) - {''})
 
+    def reported(self) -> 'list[str]':
+        """``Verify Changed files``' ``changed_files``, run as the runner runs it and
+        split on spaces as its readers split it, after checking it agrees with
+        :meth:`v17` on this tree.
+
+        """
+        path = self.root / 'verify.sh'
+        path.write_text(block_scalar_after(step_block(self.block, 'Verify Changed files'), _RUN_LITERAL),
+                        encoding='utf-8')
+        output = self.root / 'verify.output'
+        output.write_text('', encoding='utf-8')
+        result = subprocess.run(['bash', '-e', str(path)], cwd=self.repo,
+                                env=dict(self.env, GITHUB_OUTPUT=str(output)),
+                                capture_output=True, text=True, check=False,
+                                timeout=scale_timeout(60))
+        self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+        outputs = dict(line.split('=', 1) for line in output.read_text(encoding='utf-8').splitlines())
+        changed = outputs.get('changed_files', '').split()
+        self.assertEqual(outputs['files_changed'], 'true' if changed else 'false', outputs)
+        self.assertEqual(changed, self.v17())
+        return changed
+
     def classify(self, paths: 'list[str]') -> 'tuple[tuple[str, dict[str, object]], str]':
         """``Classify Changed files``' outcome and outputs, from the script itself,
         and what it printed.