ARROW-17621: [CI] Audit workflows #14155
Conversation
|
It looks like Dependabot can also scan & update actions used in workflows: https://docs.github.com/en/code-security/dependabot/working-with-dependabot/keeping-your-actions-up-to-date-with-dependabot This would remove most of the workload of updating the actions, while this would still not protect against properly obfuscated attacks combined with the other mitigations in this PR it might be a good compromise. @amol- @raulcd thoughts? |
|
|
Dependabot makes a PR to suggest the update right? |
|
Yes Dependabot opens a PR, I think we can even configure a PR title format via a config yaml to match our needs. |
|
(I'll create a follow up jira for dependabot) |
Set envvars with secrets only in the step they are needed in
assignUser
force-pushed
the
arrow-wf-audit
branch
from
0162b03 to
88c73ce
Compare
| uses: actions/[email protected] | ||
| (github.event.action == 'opened' || | ||
| github.event.action == 'synchronize') | ||
| uses: actions/labeler@4 |
This is a follow-up of ARROW-17621 / apache#14155.
This is a follow-up of ARROW-17621 / #14155. Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
|
Benchmark runs are scheduled for baseline = 44ae852 and contender = 36928ec. 36928ec is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
In this PR I: - reduced the scope of the automatically generated `GITHUB_TOKEN` as much as possible (technically `contents:none` would be the minimum but it is a bit unintuitive as it does not prevent checkout of public repos, I set `contents:read` in those cases) - update all actions used to the newest version (checking for breaking changes, only case is actions/github-script which remains on v3 for that reason -> follow up) - move the creation of envvars containing secrets as close to their usage as possible (-> the step they are used in), this duplicates them in workflows with multiple jobs but is safer. I have opted **NOT** to pin the different actions by SHA as recommended in some places as the con outweigh the possible protection in my opinion. The main danger with pinning tags or branches is that a malicious actor changes the commit the tag points to and exfiltrates secrets (either repository secrets or in case of private repos code/ip) or takes some other damaging action like deleting branches, rewriting history etc.. We only ever pass actions the `GITHUB_TOKEN` which is ephemeral (deleted after workflow is finished) and scope limited so exfiltration of that token would worst case allow an attacker to create/delete labels and pr comments as well as modify PR branches (if the submitter activated the checkbox for maintainer access). Actions can not access secrets without the workflow author explicitly passing them as input (envvars might reveal them though) The Apache Org limits the actions that can be used in repos, so we only use well known allow-listed actions, while this does of course not prevent malicious actions it reduces the risk substantially. Pinning SHAs would mitigate these risks (provided the action at that sha was audited...) but would also necessitate regularly checking + re-auditing the actions as to not miss security patches in these actions (e.g. [here](https://github.com/matlab-actions/setup-matlab/releases/tag/v1.1.1)). IMHO that would be a considerable effort (+ needing real expertise in typescript/node to spot any malicious additions outside of blatant secret exfiltration or nuking) resulting in a small gain. Lead-authored-by: Jacob Wujciak-Jens <[email protected]> Co-authored-by: assignUser <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
This is a follow-up of ARROW-17621 / apache#14155. Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
In this PR I: - reduced the scope of the automatically generated `GITHUB_TOKEN` as much as possible (technically `contents:none` would be the minimum but it is a bit unintuitive as it does not prevent checkout of public repos, I set `contents:read` in those cases) - update all actions used to the newest version (checking for breaking changes, only case is actions/github-script which remains on v3 for that reason -> follow up) - move the creation of envvars containing secrets as close to their usage as possible (-> the step they are used in), this duplicates them in workflows with multiple jobs but is safer. I have opted **NOT** to pin the different actions by SHA as recommended in some places as the con outweigh the possible protection in my opinion. The main danger with pinning tags or branches is that a malicious actor changes the commit the tag points to and exfiltrates secrets (either repository secrets or in case of private repos code/ip) or takes some other damaging action like deleting branches, rewriting history etc.. We only ever pass actions the `GITHUB_TOKEN` which is ephemeral (deleted after workflow is finished) and scope limited so exfiltration of that token would worst case allow an attacker to create/delete labels and pr comments as well as modify PR branches (if the submitter activated the checkbox for maintainer access). Actions can not access secrets without the workflow author explicitly passing them as input (envvars might reveal them though) The Apache Org limits the actions that can be used in repos, so we only use well known allow-listed actions, while this does of course not prevent malicious actions it reduces the risk substantially. Pinning SHAs would mitigate these risks (provided the action at that sha was audited...) but would also necessitate regularly checking + re-auditing the actions as to not miss security patches in these actions (e.g. [here](https://github.com/matlab-actions/setup-matlab/releases/tag/v1.1.1)). IMHO that would be a considerable effort (+ needing real expertise in typescript/node to spot any malicious additions outside of blatant secret exfiltration or nuking) resulting in a small gain. Lead-authored-by: Jacob Wujciak-Jens <[email protected]> Co-authored-by: assignUser <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
This is a follow-up of ARROW-17621 / apache#14155. Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
In this PR I:
GITHUB_TOKENas much as possible (technicallycontents:nonewould be the minimum but it is a bit unintuitive as it does not prevent checkout of public repos, I setcontents:readin those cases)I have opted NOT to pin the different actions by SHA as recommended in some places as the con outweigh the possible protection in my opinion. The main danger with pinning tags or branches is that a malicious actor changes the commit the tag points to and exfiltrates secrets (either repository secrets or in case of private repos code/ip) or takes some other damaging action like deleting branches, rewriting history etc..
We only ever pass actions the
GITHUB_TOKENwhich is ephemeral (deleted after workflow is finished) and scope limited so exfiltration of that token would worst case allow an attacker to create/delete labels and pr comments as well as modify PR branches (if the submitter activated the checkbox for maintainer access). Actions can not access secrets without the workflow author explicitly passing them as input (envvars might reveal them though)The Apache Org limits the actions that can be used in repos, so we only use well known allow-listed actions, while this does of course not prevent malicious actions it reduces the risk substantially.
Pinning SHAs would mitigate these risks (provided the action at that sha was audited...) but would also necessitate regularly checking + re-auditing the actions as to not miss security patches in these actions (e.g. here). IMHO that would be a considerable effort (+ needing real expertise in typescript/node to spot any malicious additions outside of blatant secret exfiltration or nuking) resulting in a small gain.