workflows/eval: avoid potential script injection attack#357753
workflows/eval: avoid potential script injection attack#357753JohnRTitor merged 1 commit intoNixOS:masterfrom
Conversation
Although matrix.system is supposed to be generated from trusted code, we'd better follow [Github Actions good practices][1]. [1]: https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#using-an-intermediate-environment-variable
|
Could you make the changes for #355847 (comment), in a seperate commit? |
infinisil
left a comment
There was a problem hiding this comment.
Oh yeah nice catch. This workflow doesn't (currently) deal with secrets, so nothing bad could've happened, but still
|
@infinisil I am seeing another CI fail (likely also related to high per chunk memory)? Would reducing chunk size to 750 be sufficient? |
|
I'm thinking 9000, we can easily lower it further if needed |
|
oh I missed an extra 0. 🤦 |
.github/workflows/eval.yml
Outdated
There seems to be plenty of memory available. Infinite recursion errors are related to stack memory limits. Should we not rather increase the stack size? |
|
Sorry shouldn't have mixed these two |
085c522 to
f807208
Compare
|
Here is my proposal to better understand where the infinite recursion errors come from: #357805 |
|
Oof, merged this one, and #357805 seemed to merge automatically as well (Github has a nice way of detecting this) |
|
@JohnRTitor that's normal and not problematic. It's not #357805 that was merged but #357800 |
|
By the way, any idea why backport PRs are not automatically evaluated? #357821 |
|
@JohnRTitor It's because the backport was created by the default GitHub Actions account, which GitHub prevents from triggering other actions to prevent too easy infinite CI recursions. If we use a GitHub App instead tp create the PR that would be fixed |
|
Could we create a listener to automatically start the eval action, like |
Github Actions limitation. Some documentations and workarounds https://github.com/peter-evans/create-pull-request/blob/main/docs/concepts-guidelines.md#triggering-further-workflow-runs |
Although matrix.system is supposed to be generated from trusted code, we'd better follow Github Actions good practices.
CC: @infinisil
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.