Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Forgive job-level permissions #3162

Merged
merged 12 commits into from
Jul 14, 2023
Merged

🐛 Forgive job-level permissions #3162

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
8b9fec3
Forgive all job-level permissions
pnacht Jun 13, 2023
082803b
Update tests
pnacht Jun 13, 2023
39d138b
Replace magic number
pnacht Jun 13, 2023
dd75c17
Rename test
pnacht Jun 14, 2023
8e75f9f
Test that multiple job-level permissions are forgiven
pnacht Jun 14, 2023
57efdf4
Drop unused permissionIsPresent
pnacht Jun 14, 2023
066a4f5
Update documentation
pnacht Jun 14, 2023
5d11540
Modify score descriptions
pnacht Jul 6, 2023
7240653
Document warning for job-level permissions
pnacht Jul 10, 2023
a68162e
List job-level permissions that get WARNed
pnacht Jul 13, 2023
163bf7d
Merge branch 'main' into forgive_job_permissions
spencerschrock Jul 13, 2023
59d9429
Merge branch 'main' into forgive_job_permissions
spencerschrock Jul 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions checks/evaluation/permissions/permissions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,21 +325,21 @@ func calculateScore(result map[string]permissions) int {
// status: https://docs.github.com/en/rest/reference/repos#statuses.
// May allow an attacker to change the result of pre-submit and get a PR merged.
// Low risk: -0.5.
if permissionIsPresent(perms, "statuses") {
if permissionIsPresentInTopLevel(perms, "statuses") {
spencerschrock marked this conversation as resolved.
Show resolved Hide resolved
score -= 0.5
}

// checks.
// May allow an attacker to edit checks to remove pre-submit and introduce a bug.
// Low risk: -0.5.
if permissionIsPresent(perms, "checks") {
if permissionIsPresentInTopLevel(perms, "checks") {
score -= 0.5
}

// secEvents.
// May allow attacker to read vuln reports before patch available.
// Low risk: -1
if permissionIsPresent(perms, "security-events") {
if permissionIsPresentInTopLevel(perms, "security-events") {
score--
}

Expand All @@ -348,7 +348,7 @@ func calculateScore(result map[string]permissions) int {
// and tiny chance an attacker can trigger a remote
// service with code they own if server accepts code/location var unsanitized.
// Low risk: -1
if permissionIsPresent(perms, "deployments") {
if permissionIsPresentInTopLevel(perms, "deployments") {
score--
}

Expand Down
6 changes: 3 additions & 3 deletions checks/permissions_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestGithubTokenPermissions(t *testing.T) {
filenames: []string{"./testdata/.github/workflows/github-workflow-permissions-run-no-codeql-write.yaml"},
expected: scut.TestReturn{
Error: nil,
Score: checker.MaxResultScore - 1,
Score: checker.MaxResultScore,
NumberOfWarn: 1,
NumberOfInfo: 1,
NumberOfDebug: 4,
Expand Down Expand Up @@ -306,7 +306,7 @@ func TestGithubTokenPermissions(t *testing.T) {
filenames: []string{"./testdata/.github/workflows/github-workflow-permissions-jobs-only.yaml"},
expected: scut.TestReturn{
Error: nil,
Score: 9,
Score: checker.MaxResultScore - 1,
spencerschrock marked this conversation as resolved.
Show resolved Hide resolved
NumberOfWarn: 1,
NumberOfInfo: 4,
NumberOfDebug: 4,
Expand All @@ -317,7 +317,7 @@ func TestGithubTokenPermissions(t *testing.T) {
filenames: []string{"./testdata/.github/workflows/github-workflow-permissions-run-write-codeql-comment.yaml"},
expected: scut.TestReturn{
Error: nil,
Score: checker.MaxResultScore - 1,
Score: checker.MaxResultScore,
NumberOfWarn: 1,
NumberOfInfo: 1,
NumberOfDebug: 4,
Expand Down