frontend
frontend copied to clipboard
GitHub Workflows security hardening
This PR adds explicit permissions section to workflows. This is a security best practice because by default workflows run with extended set of permissions (except from on: pull_request
from external forks). By specifying any permission explicitly all others are set to none. By using the principle of least privilege the damage a compromised workflow can do (because of an injection or compromised third party tool or action) is restricted.
It is recommended to have most strict permissions on the top level and grant write permissions on job level case by case.
It looks that this has prevented the Typescript
and TeamCity
… which means that we have no way of testing this PR against our CI/CD pipeline.
It looks that this has prevented the Typescript and TeamCity… which means that we have no way of testing this PR against our CI/CD pipeline.
I believe we've configured our GHA to require an approval to run if the PR is from a fork. I'm not sure how Teamcity is configured though.
Hey @sashashura,
Had a quick peek at your Github history, wow so many PR's! Are you using some kind of script to do this? Awesome!
Just wondering, why is this permission only added to 2 of our actions? Is it not needed for the other ones? e.g this action runs on PR, does it not need a permission section too? EDIT: I've just seen your comment on https://github.com/git/git/pull/1335#issuecomment-1250919707 explaining that it's not needed for actions triggered by pull_request
events.
Hi @AshCorr, Yes I have automated as much as possible. Still I review all the changes before the script creates a PR. Though it is impossible to read all contribution guides, so I apologize if my PR didn't follow a check list in case you have one. Let me know if you have any questions about the PR.
I suspect we might want to review whether we can enable the org-wide setting for this (read-only by default), rather than harden individual workflows. Happy to suggest that to the DevX team and explore.
Yes, and I encourage you doing so. Just keep in mind that any workflow that needs write permissions (like stale.yml for example), but do not have permissions explicitly specified will stop working after the org-wide switch. You might need to re-run and check if any write permission needs to be granted.
@jfsoul - maybe we could:
- switch it on for this repo directly
- test which actions need changing
- switch back to unsafer mode
- change them and then switch back
This would mean we're ready for a org wide switch, and might be a way to do it more incrementally.
"This PR is stale because it has been open 30 days with no activity. Unless a comment is added or the “stale” label removed, this will be closed in 3 days"
As suggested by @akash1810, we managed to run this in TeamCity with the following script from Grid:
#!/usr/bin/env bash
set -e
if [ -z $1 ]
then
echo "Please run this script with a PR number as its argument."
exit 1
fi
PR=$1
echo "Pushing PR merge commit for ${PR} to remote pr${PR} branch"
git fetch -f origin pull/${PR}/merge:pr${PR}
git push -f origin pr${PR}:pr${PR}
Seen on PROD (created by @sashashura and merged by @mxdvl 18 minutes and 21 seconds ago)
- Check your changes on www.theguardian.com ✔️
- Keep an eye on the deploy dashboard 📉