Skip to content

ci/README: add github-script policy#453985

Merged
wolfgangwalther merged 1 commit intoNixOS:masterfrom
wolfgangwalther:ci-github-script-policy
Oct 22, 2025
Merged

ci/README: add github-script policy#453985
wolfgangwalther merged 1 commit intoNixOS:masterfrom
wolfgangwalther:ci-github-script-policy

Conversation

@wolfgangwalther
Copy link
Contributor

@wolfgangwalther wolfgangwalther commented Oct 20, 2025

Over the last couple of months we have been migrating a lot of the old bash code to JavaScript, which is supported in GitHub Actions via actions/github-script. This change documents a "manual ratchet check" for this migration - new code should only be introduced as JavaScript and not as Bash. This will help us to eventually succeed with the migration and ensure quality and maintainability.

We are migrating to JavaScript, because:

  1. Using JavaScript is GitHub's recommendation against injection attacks. Using actions/github-script has first-class support for the event context and does not require to resort back to environment variables in most cases. When environment variables need to be used, these are accessed via process.env, without a risk for accidental injections. Using actions/github-script is also recommended in a recent survey of open source supply chain compromises:

    Finally, since two out of three compromises were due to shell injection,
    it might be safer to use a proper programming language, like JavaScript
    with actions/github-script, or any other language accessing the context
    via environment variables instead of YAML interpolation.

  2. Handling even environment variables in Bash safely is almost impossible. For example arithmetic expressions cause arbitrary code execution vulnerabilities. While a lot of contributors are somehwat familiar writing Bash code for builders, writing safe Bash code for CI is a very different matter. Few people, if any, know how to do this.

  3. GitHub Action's security model is quite unintuitive and even if some code runs with trusted inputs today, it may later be used in a more exposed context. Instead of making judgement calls about language choice case by case, a clear policy helps writing things defensively from the beginning.

  4. We have developed a framework around our github-script based tools in ci/github-script. This provides a local nix-shell environment with the right dependencies and a local runner for these scripts for quick testing, debugging and development. No matter, whether you're developing a new feature, fixing bugs or reviewing a PR - this allows much quicker verification of the scripts, without running everything in a fork or test organization.

  5. This framework also provides helpers for challenges that come up with GHA. One example is rate-limiting, where we have a helper script that will handle all rate-limiting needs for us, preventing us from running out of API calls and thus breaking CI entirely. We can only use these tools consistently, if we consistently use JavaScript code.

  6. Using JavaScript allows us to handle JSON natively. Using octokit/rest.js provides first-class integration with GitHub's API. Together, this makes these scripts much more maintainable than resorting to gh and jq.

Context

This came up in #450864 (comment). The migration has been happening in practice for some months already, but since it was mostly myself who was working on the code, it was never formalized.

This PR seeks to clarify the situation, asking specifically for input of the remaining @NixOS/nixpkgs-ci team and for additional input from @NixOS/security.

Things done


Add a 👍 reaction to pull requests you find important.

@wolfgangwalther wolfgangwalther requested review from a team October 20, 2025 20:11
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions backport release-25.05 labels Oct 20, 2025
Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

My main argument against this is really that everybody is already familiar with bash just because it's so prevalent in Nixpkgs. So requiring JS instead makes it much harder for people to contribute.

You do make good arguments for JS. A large part comes from the fact that you already spent so much effort on it though, when much would've also worked for any other language, including bash.

@wolfgangwalther wolfgangwalther force-pushed the ci-github-script-policy branch from 9de7625 to be1d034 Compare October 21, 2025 07:40
@wolfgangwalther
Copy link
Contributor Author

My main argument against this is really that everybody is already familiar with bash just because it's so prevalent in Nixpkgs. So requiring JS instead makes it much harder for people to contribute.

I extended the commit message reasoning / PR body with some new points, one of them addressing this as well. I don't think that's actually true. A lot of people are familiar with bash to write builder. Few people, if any, are familiar with writing safe Bash code for CI purposes. Also, it's not only Bash, you need to do a lot of jq wrangling, too.

Requiring JS does not make it harder to contribute, I believe. Initially, it is a hurdle to take, especially for someone like you, who is one of the few contributors actually having experience with writing CI in Bash. But on the bigger scale, this doesn't change a bit.

You do make good arguments for JS. A large part comes from the fact that you already spent so much effort on it though, when much would've also worked for any other language, including bash.

When we started the migration, I had exactly zero experience with actions/github-script. I had never written a single line in it, nor did @MattSturgeon or @philiptaron have any experience reviewing it. Still, the majority of arguments was already clear back then. I believe the learning was not hard, on the contrary. All the 🤦 moments we had and still have are from GitHub Actions in general, not from using JavaScript to do it. And no, this would have not worked for Bash.

I'd like to encourage you and just try it. I'm sure you will eventually be convinced that this is much better.

@philiptaron
Copy link
Contributor

In addition to everything that Wolfgang argued above, which I agree with, I have to double down on my own perspective about reviewing the new CI code versus the bash CI code.

The real challenge is that bash has an extensive surface area of implicit behaviors that interact in non-obvious ways. I need to verify proper quoting around variable expansions, understand whether you’re dealing with the test command [ or the bash keyword [[, and trace how exit codes propagate through pipelines and subshells. The language resists static analysis in fundamental ways, so I can’t rely on tooling to catch issues the way I would with strongly-typed compiled languages. Tools like shellcheck are valuable, but they can only catch a subset of potential issues because so much depends on runtime context and the actual data flowing through your commands. And in our CI, even shellcheck isn't readily available at hand.

The documentation landscape also presents challenges. I often need to cross-reference the bash manual, POSIX specifications, and community knowledge bases to determine the semantics of particular constructs are in the first place... let alone whether they’ll behave differently across environments. Complex scripts with heavy use of parameter expansion, process substitution, and pipeline chains require careful analysis to understand both intent and correctness. This isn’t to diminish bash’s utility—it remains an excellent tool for its domain—but effective code review in bash demands a level of detail-oriented scrutiny and deep language knowledge that exceeds what’s typically required for even notoriously "wat‽" languages like JavaScript.

Another significant factor is bash’s limitation to primitive data structures—essentially strings, arrays, and associative arrays. JavaScript provides rich object models where you can encapsulate data with behavior, define clear interfaces, and leverage type systems (even dynamic ones) to make code intent explicit. I see objects with methods, clear data transformations, and structured error handling. In bash, everything ultimately reduces to text manipulation, which means you’re constantly parsing and validating data inline. There’s no .parse() method that returns a well-defined object—instead, you’re using cut, awk, or parameter expansion to extract fields, hoping the input format hasn’t changed. This lack of abstraction makes it difficult to review for correctness because I can’t reason about well-defined types or contracts; I'm always tracking raw strings and their various transformations through the script’s execution flow.​​​​​​​​​​​​​​​​

For this specific domain, I'm in pretty strong agreement that bash is not the right language for future additions.

Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

I tried to caveat it to take into account @MattSturgeon's feedback. If it's a feature: let's not implement it in Bash.

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 22, 2025
Over the last couple of months we have been migrating a lot of the old
bash code to JavaScript, which is supported in GitHub Actions via
`actions/github-script`. This change documents a "manual ratchet check"
for this migration - new code should only be introduced as JavaScript
and not as Bash. This will help us to eventually succeed with the
migration and ensure quality and maintainability.

We are migrating to JavaScript, because:

1. Using JavaScript is GitHub's [recommendation] against injection attacks.
   Using `actions/github-script` has first-class support for the event
   context and does not require to resort back to environment variables in
   most cases. When environment variables need to be used, these are
   accessed via `process.env`, without a risk for accidental injections.
   Using `actions/github-script` is also recommended in a recent
   [survey] of open source supply chain compromises:

   > Finally, since two out of three compromises were due to shell injection,
   > it might be safer to use a proper programming language, like JavaScript
   > with actions/github-script, or any other language accessing the context
   > via environment variables instead of YAML interpolation.

2. Handling even environment variables in Bash safely is almost
   impossible. For example arithmetic expressions cause arbitrary code
   execution vulnerabilities. While a lot of contributors are somehwat
   familiar writing Bash code for builders, writing *safe* Bash code for
   CI is a very different matter. Few people, if any, know how to do
   this.

3. GitHub Action's security model is quite unintuitive and even if some
   code runs with trusted inputs today, it may later be used in a more
   exposed context. Instead of making judgement calls about language
   choice case by case, a clear policy helps writing things defensively
   from the beginning.

4. We have developed a framework around our github-script based tools in
   `ci/github-script`. This provides a local `nix-shell` environment
   with the right dependencies and a local runner for these scripts for
   quick testing, debugging and development. No matter, whether you're
   developing a new feature, fixing bugs or reviewing a PR - this allows
   much quicker verification of the scripts, *without* running
   everything in a fork or test organization.

5. This framework also provides helpers for challenges that come up with
   GHA. One example is rate-limiting, where we have a helper script that
   will handle all rate-limiting needs for us, preventing us from
   running out of API calls and thus breaking CI entirely. We can only
   use these tools consistently, if we consistently use JavaScript code.

6. Using JavaScript allows us to handle JSON natively. Using
   `octokit/rest.js` provides first-class integration with GitHub's API.
   Together, this makes these scripts much more maintainable than
   resorting to `gh` and `jq`.

[recommendation]: https://docs.github.com/en/actions/reference/security/secure-use#use-an-action-instead-of-an-inline-script
[survey]: https://words.filippo.io/compromise-survey/
@wolfgangwalther wolfgangwalther force-pushed the ci-github-script-policy branch from be1d034 to ce8c42d Compare October 22, 2025 10:59
@wolfgangwalther
Copy link
Contributor Author

Two approvals + more feedback in the form of reactions (also by the security team). Let's do it.

@wolfgangwalther wolfgangwalther added this pull request to the merge queue Oct 22, 2025
Merged via the queue into NixOS:master with commit 5fde0d7 Oct 22, 2025
26 of 30 checks passed
@wolfgangwalther wolfgangwalther deleted the ci-github-script-policy branch October 22, 2025 12:44
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Oct 22, 2025

Successfully created backport PR for release-25.05:

@github-actions github-actions bot added the 8.has: port to stable This PR already has a backport to the stable release. label Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 8.has: port to stable This PR already has a backport to the stable release. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants