Refactor documentation deploy #1165

ericpre · 2025-10-05T12:13:16Z

@CSSFrancis, I tried to restructure to simplify and separate the build and push steps in two separate workflow. This approach is inspired from https://securitylab.github.com/resources/github-actions-preventing-pwn-requests.

separate the build from the deploy workflow to use different context:
- the build run on push or pull_request events and doesn't have write permission to the repository
- the deploy workflow have write permission and doesn't build anything (to avoid malicious code accessing secrets, etc.), it only download the html and push it to the right place.
use workflow_dispatch event for the deploy workflow, and trigger it at the end of the build workflow.

codecov · 2025-10-05T12:19:05Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.70%. Comparing base (facf6b7) to head (8b05576).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1165   +/-   ##
=======================================
  Coverage   92.70%   92.70%           
=======================================
  Files         158      158           
  Lines       16158    16158           
=======================================
  Hits        14979    14979           
  Misses       1179     1179

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2025-10-05T12:19:19Z

PR Preview
Preview removed because the pull request was closed.
2025-10-10 14:41 UTC

CSSFrancis · 2025-10-05T12:47:05Z

@ericpre i hadn't thought about someone modifying the make file. That is a really good catch.

Do we want to add this in the hyperspy/.github shared workflows?

ericpre · 2025-10-05T13:09:33Z

Yes, I will add some documentation.

ericpre · 2025-10-05T13:43:21Z

Done in hyperspy/.github@3d6506b. I have also set the permissions to be downgraded in the build stage to be on the safe side - see notes from https://docs.github.com/en/actions/reference/workflows-and-actions/reusing-workflow-configurations#supported-keywords-for-jobs-that-call-a-reusable-workflow on passing permission from caller to called workflow.

CSSFrancis

Just a couple of comments

.github/workflows/docs-deploy.yml

CSSFrancis · 2025-10-05T13:52:40Z

.github/workflows/docs-deploy.yml

+
+      - name: Deploy versioned docs
+        # run on tag only
+        if: github.ref_type == 'tag'


This should be on a tag or a workflow dispatch.

Just if we make a new release and the documentation build fails then the idea is that we can just run the workflow dispatch to build a new version of the docs. That and we can overwrite a version if necessary.

I don't think that it is necessary to specify a tag (this was actually a comment that I had when reviewing #1164) because I expect that it will work as follow:

select a tag from the "Run workflow" menu - see screenshot below

github.ref_type should be tag and therefore the condition for that step true.

Maybe something to add the documentation?

Of course, this is not to say that they may some teething issue with this refactor, but this is the idea!

Ahhh that's interesting... I guess I'm learning a lot about GitHub workflows :)

…s (dev, pr-preview and tag) in the same workflow

ericpre · 2025-10-06T20:14:18Z

I have checked the workflow by pushing on main in my fork. With the fixes in 4fe4229 and f50064f, it is working:

dev deploy: https://github.com/ericpre/pyxem/actions/runs/18288070398/job/52068206083
tag deploy: https://github.com/ericpre/pyxem/actions/runs/18292776298/job/52083883234

I haven't checked yet if this works as expected when running the workflow manually from a tag.

CSSFrancis · 2025-10-07T02:25:34Z

@ericpre Sounds great! Do you want me to merge this to see if the preview deploy still works?

ericpre · 2025-10-07T08:19:42Z

I had another look and I don't think that using workflow_run and rossjrw/pr-preview-action will work for PR preview, because the pull_request event context will be missing for rossjrw/pr-preview-action, and this is needed for the PR number... See rossjrw/pr-preview-action#101 and rossjrw/pr-preview-action#6.
The issue is that pr-preview-action currently doesn't support PR-preview from fork, because it needs a pull_request or pull_request_target event. If the pull_request event is used, then the token is not available to push and if pull_request_target, the source of the documentation doesn't come from the PR branch...

This could still be possible using different actions, such as for example in wpaccessibility/wp-a11y-docs#136. What do you think?

CSSFrancis · 2025-10-07T12:22:59Z

I had another look and I don't think that using workflow_run and rossjrw/pr-preview-action will work for PR preview, because the pull_request event context will be missing for rossjrw/pr-preview-action, and this is needed for the PR number... See rossjrw/pr-preview-action#101 and rossjrw/pr-preview-action#6. The issue is that pr-preview-action currently doesn't support PR-preview from fork, because it needs a pull_request or pull_request_target event. If the pull_request event is used, then the token is not available to push and if pull_request_target, the source of the documentation doesn't come from the PR branch...

This could still be possible using different actions, such as for example in wpaccessibility/wp-a11y-docs#136. What do you think?

Seems sensible to me :) It's just pushing to a different repo so there isn't anything that says it has to specifically be an action designed for a PR-preview.

ericpre · 2025-10-08T11:58:26Z

I have tried a different approach using workflow_dispatch and tested it for push to main and on tag in my fork and it is working.
The approach should in principle work for PR-preview too but it is not there yet. As suggested in rossjrw/pr-preview-action#101 (comment), I tried to adapt https://github.com/rossjrw/pr-preview-action to support passing PR number but I don't think that it will work in its current shape because it will still expect a pull_request or pull_request_target event in one of the script.

CSSFrancis · 2025-10-09T03:01:13Z

@ericpre i would just do like you suggested and switch to the

JamesIves/github-pages-deploy-action@v4

In that case we can just get the PR number and push to the right branch. Basically following this:

https://github.com/wpaccessibility/wp-a11y-docs/pull/136/files

CSSFrancis · 2025-10-09T03:01:37Z

Let me know if you want me to try that tomorrow.

ericpre · 2025-10-09T08:52:40Z

I thought that I would give a go at keeping the cleanup steps out of the deploy workflow itself but adding the customised action is indeed a bit messy! This PR now has the clean up steps in the deploy workflow.

Here is my security vulnerability assessment of the deploy workflow:

building doc is done in the doc_build workflow that runs in the context of the fork repository in the case of pull request, because it is triggered by pull_request event is used - this is the same as for running test suite, etc.
deploy is done in the doc_deploy workflow, which is triggered by the doc_build workflow. This workflow runs in the context of the pyxem/pyxem repository to have access to GitHub secrets. It is not susceptible to pwn requests attack because:
- it doesn't build any documentation, that could expose GitHub secrets. The documentation build is downloaded from the corresponding doc_build workflow run.
- if the workflow is edited in a pull request, these changes will not be used when triggering the workflow on the dispatch event because workflow_dispatch used the workflow defined in the main branch. It will need to be merged to main to be effective. For example, the doc_build workflow is failing because it can't find the workflow (added in this PR) on the main branch.

…review in the workflow dispatch

CSSFrancis

@ericpre This looks great!

ericpre mentioned this pull request Oct 5, 2025

Documentation: Add information about workflow #1164

Open

ericpre force-pushed the improve_doc_deploy_workflow branch from fa76f62 to 9669199 Compare October 5, 2025 12:58

CSSFrancis reviewed Oct 5, 2025

View reviewed changes

ericpre added 2 commits October 6, 2025 08:02

Split building and deploy in two different workflows. Keep all deploy…

1abbddf

…s (dev, pr-preview and tag) in the same workflow

Add redirect to stable documentation on tag

6a84d74

ericpre force-pushed the improve_doc_deploy_workflow branch from 9669199 to 6a84d74 Compare October 6, 2025 07:02

ericpre force-pushed the improve_doc_deploy_workflow branch from f50064f to 538a35e Compare October 8, 2025 11:52

Specify run_id and github-token to download from different workflow

a6b4d73

ericpre force-pushed the improve_doc_deploy_workflow branch 2 times, most recently from 330fe75 to 8c04180 Compare October 9, 2025 08:32

Use workflow_dispatch approach and implement logic to clean-up PR p…

8b05576

…review in the workflow dispatch

ericpre force-pushed the improve_doc_deploy_workflow branch from 8c04180 to 8b05576 Compare October 9, 2025 09:01

CSSFrancis approved these changes Oct 10, 2025

View reviewed changes

CSSFrancis merged commit 03977f4 into pyxem:main Oct 10, 2025
18 of 19 checks passed

Refactor documentation deploy #1165

Refactor documentation deploy #1165

Uh oh!

Conversation

ericpre commented Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CSSFrancis commented Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericpre commented Oct 5, 2025

Uh oh!

ericpre commented Oct 5, 2025

Uh oh!

CSSFrancis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CSSFrancis Oct 5, 2025

Choose a reason for hiding this comment

Uh oh!

ericpre Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

CSSFrancis Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

ericpre commented Oct 6, 2025

Uh oh!

CSSFrancis commented Oct 7, 2025

Uh oh!

ericpre commented Oct 7, 2025

Uh oh!

CSSFrancis commented Oct 7, 2025

Uh oh!

ericpre commented Oct 8, 2025

Uh oh!

CSSFrancis commented Oct 9, 2025

Uh oh!

CSSFrancis commented Oct 9, 2025

Uh oh!

ericpre commented Oct 9, 2025

Uh oh!

CSSFrancis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ericpre commented Oct 5, 2025 •

edited

Loading

codecov bot commented Oct 5, 2025 •

edited

Loading

github-actions bot commented Oct 5, 2025 •

edited

Loading

CSSFrancis commented Oct 5, 2025 •

edited

Loading