Skip to content

Updates GitHub actions to latest, removes non-conforming whitespace & narrows pemissions#329

Merged
Exairnous merged 3 commits intomasterfrom
update-github-actions
Nov 13, 2025
Merged

Updates GitHub actions to latest, removes non-conforming whitespace & narrows pemissions#329
Exairnous merged 3 commits intomasterfrom
update-github-actions

Conversation

@DougReeder
Copy link
Member

@DougReeder DougReeder commented Nov 10, 2025

What?

Updates GitHub actions to latest, removes non-conforming whitespace & narrows pemissions

Why?

  • To ensure this workflow doesn't break later on
  • To make code style consistent
  • To guard against vulnerabilities in actions

How to test

  1. On GitHub, navigate to the Actions tab of this repo
  2. Observe that the lint and test and publish jobs complete successfully

Documentation of functionality

No changes to functionality.

Limitations

Doesn't run lint and test on local commit

@DougReeder DougReeder marked this pull request as draft November 10, 2025 14:34
@DougReeder DougReeder force-pushed the update-github-actions branch from 1a7ddf4 to d75598b Compare November 10, 2025 14:53
@DougReeder DougReeder marked this pull request as ready for review November 10, 2025 14:56
Copy link
Member

@Exairnous Exairnous left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to keep things updated. Thanks.

This seems to be good, except for the addition of the if guard in the publish step, which I've left an inline comment on.

@vincentfretin
Copy link

While you're at it, you should specify only the needed permissions to run those jobs.
Probably define globally

permissions:
  contents: read

and for the publish job override it:

jobs:
  publish:
    permissions:
      contents: read
      actions: write

that should be enough I think.

https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#permissions

@DougReeder
Copy link
Member Author

While you're at it, you should specify only the needed permissions to run those jobs. Probably define globally

permissions:
  contents: read

and for the publish job override it:

jobs:
  publish:
    permissions:
      contents: read
      actions: write

that should be enough I think.

https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#permissions

This looks plausible, but what would this actually do for us?

@vincentfretin
Copy link

This is just a security measure in case there is a vulnerability somewhere in the actions to not allow more permissions that what the action was supposed to do. This is really to adhere strictly to the principle of least privilege. Not sure what is the default set of permissions here for the GITHUB_TOKEN, that may depends of the configuration on the repo or organization. Some more info: https://graphite.com/guides/github-actions-permissions

@DougReeder DougReeder changed the title Updates GitHub actions to latest & only publishes from master Updates GitHub actions to latest & narrows pemissions Nov 12, 2025
@DougReeder DougReeder force-pushed the update-github-actions branch from c79ae15 to 4597b7f Compare November 12, 2025 03:03
@DougReeder
Copy link
Member Author

I couldn't find documentation elsewhere, so I asked Phind 70-B what permissions each action requires, and added them to the commit. I really want to avoid a cycle of pushing to GitHub, having the workflow fail, and needing to debug which exact permission is needed.

@DougReeder
Copy link
Member Author

If no one objects, I will merge commit 0618829 "Removes guard clause" into commit ab6f87f "Updates GitHub actions to latest & only publishes from master", so we are not adding code only to remove it in the same PR.

@DougReeder DougReeder changed the title Updates GitHub actions to latest & narrows pemissions Updates GitHub actions to latest, removes non-conforming whitespace & narrows pemissions Nov 12, 2025
@Exairnous
Copy link
Member

If no one objects, I will merge commit 0618829 "Removes guard clause" into commit ab6f87f "Updates GitHub actions to latest & only publishes from master", so we are not adding code only to remove it in the same PR.

That sounds fine, I think. I would also update the commit message to reflect the changes, and the PR description.

@Exairnous
Copy link
Member

Exairnous commented Nov 12, 2025

I couldn't find documentation elsewhere, so I asked Phind 70-B what permissions each action requires, and added them to the commit.

You're right, the documentation is a bit sparse. Here's what I found:

From what I could find out from the documentation and from my own testing, it seems that the only permission that may be required is the contents: read one. See: https://github.com/Exairnous/hubs-blender-exporter/actions/runs/19286488107

Although, my attempts with even that set to none completed successfully. So it looks like no permissions are required (which I'll admit is a bit weird, but the run logs record the different permissions I've tried, so it seems to be valid).

Since the contents: read permission is set globally at the top of the file, I think the per job permission blocks can be removed (and probably also the actions: write in the top block).

The run logs do report a Metadata permission that's set to read so maybe that's what's allowing it. I even tried setting all the documented permissions to none in the top level block and everything still worked (and that Metadata permission was still there). Reference: https://github.com/Exairnous/hubs-blender-exporter/actions/runs/19287689775/job/55151776063

I really want to avoid a cycle of pushing to GitHub, having the workflow fail, and needing to debug which exact permission is needed.

It has been my experience that AI assisted changes should be checked more carefully than regular changes.

Edit: I forgot to mention, thank you for the AI disclosure, that was quite helpful when reviewing.

@DougReeder DougReeder force-pushed the update-github-actions branch from 4597b7f to 4c38edd Compare November 12, 2025 15:10
Why: To ensure this workflow doesn't break later on
Why: To appease the latest version of python-lint-plus
Why: The principle of Least Privilege guards against a vulnerability in the actions.
@DougReeder DougReeder force-pushed the update-github-actions branch 3 times, most recently from 2672583 to 8525f22 Compare November 12, 2025 15:42
Copy link
Member

@Exairnous Exairnous left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks. Merging.

@Exairnous Exairnous merged commit 0c67270 into master Nov 13, 2025
12 checks passed
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.

3 participants