Skip to content

Conversation

@2bndy5
Copy link
Member

@2bndy5 2bndy5 commented Nov 23, 2025

This is just an update to address common security concerns about GitHub Actions syntax. Most changes were the reesult of concerns raised by zizmor. Since the majority of this repo is for reusable CI workflows, I add a linter workflow to have zizmor check any YAML files in ".github/*" directory.

Summary

  • Do not persist git credentials when they are not needed (eg. no git push done)
  • Pin third-party actions to their SHA instead of a rolling major tag. Any actions/* is officially maintained by GitHub, so they are the exception here.
  • Replace third-party actions with official GitHub actions where possible. This involves using gh-cli for uploading release assets.
  • Rely more on env variable instead of template substitutions (ie. ${{ inputs.rf24_ref }})
  • Move increment_version.py to .github/directory and address various typing errors (raised by latest mypy version).
  • Moved git-cliff config (cliff.toml) to .github/ directory and updated the config's template for the newer version of git-cliff (whitespace handling).
  • Pin git-cliff version via pip requirements.txt constraint. This will be auto-updated by dependabot.
  • Do no inherit default/global permissions for secrets.GITHUB_TOKEN. Instead, each job explicitly specifies what permissions shall be used with explanatory comments.
  • Add to .gitattributes to ensure that all files checked in are using LF, not CRLF
  • Explicitly specify a coll-down period for when dependabot checks for updates.

Note

CMake version v3.29 or newer is expected due to the new reliance on environment variables (instead of arguments in long-winded cmake commands). At this time, CMake v3.31 is provided by default.

This does not apply to the install script, nor impose a (newer) minimum version of CMake on RF24* libs.

@2bndy5
Copy link
Member Author

2bndy5 commented Nov 23, 2025

Do no inherit default/global permissions for secrets.GITHUB_TOKEN. Instead, each job explicitly specifies what permissions shall be used with explanatory comments

This will require some changes in other nRF24 repos' CI. Really just the build workflows that upload release assets

jobs:
  build:
    runs-on: ubuntu-latest
    permissions:
      # only used to upload release assets to GitHub Releases.
      # non-tagged commits don't need this at all,
      # but this value cannot be dynamically set
      contents: write

Ideally, the deployed assets would be uploaded in a separate job (using artifacts from previous jobs) to further restrict the write access to repos.

This is just an update to address common security concerns about GitHub Actions syntax.
Most changes were the reesult of concerns raised by [zizmor].
Since the majority of this repo is for reusable CI workflows, I add a linter workflow to have [zizmor] check any YAML files in ".github/*" directory.

[zizmor]: https://docs.zizmor.sh/

# Summary

- Do not persist git credentials when they are not needed (eg. no `git push` done)
- Pin third-party actions to their SHA instead of a rolling major tag. Any `actions/*` is officially maintained by GitHub, so they are the exception here.
- Replace third-party actions with official GitHub actions where possible. This involves using gh-cli for uploading release assets.
- Rely more on env variable instead of template substitutions (ie. `${{ inputs.rf24_ref }}`)
- Move increment_version.py to .github/ directory and address various typing errors (raised by latest mypy version).
- Moved git-cliff config (cliff.toml) to .github/ directory and updated the config's template for the newer version of git-cliff (whitespace handling).
- Pin git-cliff version via pip requirements.txt constraint. This will be auto-updated by dependabot.
- Do no inherit default/global permissions for `secrets.GITHUB_TOKEN`. Instead, each job explicitly specifies what permissions shall be used with explanatory comments.
- Add to .gitattributes to ensure that all files checked in are using LF, not CRLF.
- Explicitly specify a cool-down period for when dependabot checks for updates.

> [!NOTE]
> CMake version v3.29 or newer is expected due to the new reliance on environment variables (instead of arguments in long-winded CMake commands). At this time, CMake v3.31 is provided by default.
>
> This does not apply to the install script, nor impose a (newer) minimum version of CMake on RF24* libs.
@2bndy5
Copy link
Member Author

2bndy5 commented Nov 24, 2025

Ok, bump-n-release script seems to be working (now after some testing).

2bndy5 added a commit to nRF24/RF24 that referenced this pull request Nov 25, 2025
2bndy5 added a commit to nRF24/RF24 that referenced this pull request Nov 25, 2025
@2bndy5
Copy link
Member Author

2bndy5 commented Nov 25, 2025

I've separated the various build and deploy steps into separate jobs (invoked as reusable workflows). This limits our attack exposure by only using write access when it is absolutely needed.

The true measure of success will have to wait until next release, but I'm rather confident these changes will behave as expected.

Tested these changes with nRF24/RF24#1050, now on to the network layers...

2bndy5 added a commit to nRF24/RF24Mesh that referenced this pull request Nov 25, 2025
2bndy5 added a commit to nRF24/RF24Gateway that referenced this pull request Nov 25, 2025
2bndy5 added a commit to nRF24/RF24Ethernet that referenced this pull request Nov 25, 2025
2bndy5 added a commit to nRF24/RF24Audio that referenced this pull request Nov 25, 2025
corresponds to nRF24/.github#23

But really this repo just uses CI to deploy docs.
So, I updated the docs sources to work better with doxygen v1.15.0

And I found/fixed a typo.
Makes sure the saved artifacts' names/paths match the names/paths of the artifacts being deployed (in separate workflows
@2bndy5 2bndy5 merged commit e53ea17 into main Nov 27, 2025
1 check passed
@2bndy5 2bndy5 deleted the review-ci branch November 27, 2025 09:35
2bndy5 added a commit to nRF24/RF24 that referenced this pull request Nov 27, 2025
corresponding to nRF24/.github#23

* auto-cancel CI runs when a new run is queued
  does not apply to events on master branch
* use LF everywhere
* use doxygen v1.15.0 and adjust path to images and some other MD lint rules
2bndy5 added a commit to nRF24/RF24Mesh that referenced this pull request Nov 27, 2025
corresponds to nRF24/.github#23

* use doxygen v1.15.0
2bndy5 added a commit to nRF24/RF24Gateway that referenced this pull request Nov 27, 2025
corresponds to nRF24/.github#23

* use doxygen v1.15.0
2bndy5 added a commit to nRF24/RF24Audio that referenced this pull request Nov 27, 2025
corresponds to nRF24/.github#23

really this repo just uses CI to deploy docs.
So, I updated the docs sources to work better with doxygen v1.15.0

And I found/fixed a typo.

* enable RTD builds for PRs
2bndy5 added a commit to nRF24/RF24Ethernet that referenced this pull request Nov 27, 2025
corresponds to nRF24/.github#23

* use doxygen v1.15.0
2bndy5 added a commit to nRF24/RF24Network that referenced this pull request Nov 27, 2025
corresponds to nRF24/.github#23

* use LF everywhere
* use doxygen v1.15.0
2bndy5 added a commit to nRF24/RF24Mesh that referenced this pull request Nov 27, 2025
corresponds to nRF24/.github#23

* use doxygen v1.15.0
2bndy5 added a commit to nRF24/RF24Gateway that referenced this pull request Nov 27, 2025
corresponds to nRF24/.github#23

* use doxygen v1.15.0
@2bndy5
Copy link
Member Author

2bndy5 commented Nov 27, 2025

Phew, I'm glad that went well. The next bump-n-release attempt (if any) will be the final test; should be fine though 🤞🏼 .

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.

2 participants