Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 15 additions & 8 deletions remediation/workflow/permissions/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func githubTokenInJobLevelEnv(job metadata.Job) bool {
return false
}

func AddWorkflowLevelPermissions(inputYaml string, addProjectComment bool) (string, error) {
func AddWorkflowLevelPermissions(inputYaml string, addProjectComment bool, addEmptyTopLevelPermissions bool) (string, error) {
workflow := metadata.Workflow{}

err := yaml.Unmarshal([]byte(inputYaml), &workflow)
Expand Down Expand Up @@ -138,13 +138,20 @@ func AddWorkflowLevelPermissions(inputYaml string, addProjectComment bool) (stri
spaces += " "
}

if addProjectComment {
output = append(output, spaces+"permissions: # added using https://github.com/step-security/secure-repo")
if addEmptyTopLevelPermissions {
if addProjectComment {
output = append(output, spaces+"permissions: {} # added using https://github.com/step-security/secure-repo")
} else {
output = append(output, spaces+"permissions: {}")
}
} else {
output = append(output, spaces+"permissions:")
if addProjectComment {
output = append(output, spaces+"permissions: # added using https://github.com/step-security/secure-repo")
} else {
output = append(output, spaces+"permissions:")
}
output = append(output, spaces+" contents: read")
}

output = append(output, spaces+" contents: read")
output = append(output, "")

for i := line - 1; i < len(inputLines); i++ {
Expand All @@ -154,7 +161,7 @@ func AddWorkflowLevelPermissions(inputYaml string, addProjectComment bool) (stri
return strings.Join(output, "\n"), nil
}

func AddJobLevelPermissions(inputYaml string) (*SecureWorkflowReponse, error) {
func AddJobLevelPermissions(inputYaml string, addEmptyTopLevelPermissions bool) (*SecureWorkflowReponse, error) {

workflow := metadata.Workflow{}
errors := make(map[string][]string)
Expand Down Expand Up @@ -216,7 +223,7 @@ func AddJobLevelPermissions(inputYaml string) (*SecureWorkflowReponse, error) {
if strings.Compare(inputYaml, fixWorkflowPermsReponse.FinalOutput) != 0 {
fixWorkflowPermsReponse.IsChanged = true

if len(perms) == 1 && strings.Contains(perms[0], contents_read) {
if len(perms) == 1 && strings.Contains(perms[0], contents_read) && !addEmptyTopLevelPermissions {
// Don't add contents: read, because it will get defined at workflow level
continue
} else {
Expand Down
92 changes: 90 additions & 2 deletions remediation/workflow/permissions/permissions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ func TestAddJobLevelPermissions(t *testing.T) {
}

for _, f := range files {

if f.Name() == "empty-top-level-permissions.yml" {
continue
}

input, err := ioutil.ReadFile(path.Join(inputDirectory, f.Name()))

if err != nil {
Expand All @@ -26,7 +31,7 @@ func TestAddJobLevelPermissions(t *testing.T) {

os.Setenv("KBFolder", "../../../knowledge-base/actions")

fixWorkflowPermsResponse, err := AddJobLevelPermissions(string(input))
fixWorkflowPermsResponse, err := AddJobLevelPermissions(string(input), false)
output := fixWorkflowPermsResponse.FinalOutput
jobErrors := fixWorkflowPermsResponse.JobErrors

Expand Down Expand Up @@ -68,6 +73,47 @@ func TestAddJobLevelPermissions(t *testing.T) {
}
}

func TestAddJobLevelPermissionsWithEmptyTopLevel(t *testing.T) {
const inputDirectory = "../../../testfiles/joblevelpermskb/input"
const outputDirectory = "../../../testfiles/joblevelpermskb/output"

// Test the empty-top-level-permissions.yml file
input, err := ioutil.ReadFile(path.Join(inputDirectory, "empty-top-level-permissions.yml"))
if err != nil {
t.Fatal(err)
}

expectedOutput, err := ioutil.ReadFile(path.Join(outputDirectory, "empty-top-level-permissions.yml"))
if err != nil {
t.Fatal(err)
}

os.Setenv("KBFolder", "../../../knowledge-base/actions")

// Test with addEmptyTopLevelPermissions = true
fixWorkflowPermsResponse, err := AddJobLevelPermissions(string(input), true)
if err != nil {
t.Errorf("Unexpected error with addEmptyTopLevelPermissions=true: %v", err)
}

if fixWorkflowPermsResponse.FinalOutput != string(expectedOutput) {
t.Errorf("test failed with addEmptyTopLevelPermissions=true for empty-top-level-permissions.yml\nExpected:\n%s\n\nGot:\n%s",
string(expectedOutput), fixWorkflowPermsResponse.FinalOutput)
}

// Test with addEmptyTopLevelPermissions = false (should skip contents: read)
fixWorkflowPermsResponse2, err2 := AddJobLevelPermissions(string(input), false)
if err2 != nil {
t.Errorf("Unexpected error with addEmptyTopLevelPermissions=false: %v", err2)
}

// With false, contents: read should be skipped at job level
if fixWorkflowPermsResponse2.FinalOutput != string(input) {
t.Errorf("test failed with addEmptyTopLevelPermissions=false for empty-top-level-permissions.yml\nExpected:\n%s\n\nGot:\n%s",
string(input), fixWorkflowPermsResponse2.FinalOutput)
}
}

func Test_addPermissions(t *testing.T) {
type args struct {
inputYaml string
Expand Down Expand Up @@ -112,6 +158,10 @@ func TestAddWorkflowLevelPermissions(t *testing.T) {
continue
}

if f.Name() == "empty-permissions.yml" {
continue
}

input, err := ioutil.ReadFile(path.Join(inputDirectory, f.Name()))

if err != nil {
Expand All @@ -125,7 +175,7 @@ func TestAddWorkflowLevelPermissions(t *testing.T) {
addProjectComment = true
}

output, err := AddWorkflowLevelPermissions(string(input), addProjectComment)
output, err := AddWorkflowLevelPermissions(string(input), addProjectComment, false)

if err != nil {
t.Errorf("Error not expected")
Expand All @@ -143,3 +193,41 @@ func TestAddWorkflowLevelPermissions(t *testing.T) {
}

}

func TestAddWorkflowLevelPermissionsWithEmpty(t *testing.T) {
const inputDirectory = "../../../testfiles/toplevelperms/input"
const outputDirectory = "../../../testfiles/toplevelperms/output"

// Test the empty-permissions.yml file
input, err := ioutil.ReadFile(path.Join(inputDirectory, "empty-permissions.yml"))
if err != nil {
t.Fatal(err)
}

expectedOutput, err := ioutil.ReadFile(path.Join(outputDirectory, "empty-permissions.yml"))
if err != nil {
t.Fatal(err)
}

// Test with addEmptyTopLevelPermissions = true
output, err := AddWorkflowLevelPermissions(string(input), false, true)
if err != nil {
t.Errorf("Unexpected error with addEmptyTopLevelPermissions=true: %v", err)
}

if output != string(expectedOutput) {
t.Errorf("test failed with addEmptyTopLevelPermissions=true for empty-permissions.yml\nExpected:\n%s\n\nGot:\n%s",
string(expectedOutput), output)
}

// Test with addEmptyTopLevelPermissions = false (should add contents: read)
output2, err2 := AddWorkflowLevelPermissions(string(input), false, false)
if err2 != nil {
t.Errorf("Unexpected error with addEmptyTopLevelPermissions=false: %v", err2)
}

// With false, should add contents: read instead of empty permissions
if !strings.Contains(output2, "contents: read") || strings.Contains(output2, "permissions: {}") {
t.Errorf("test failed with addEmptyTopLevelPermissions=false for empty-permissions.yml - should contain 'contents: read' but not 'permissions: {}'\nGot:\n%s", output2)
}
}
9 changes: 7 additions & 2 deletions remediation/workflow/secureworkflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d
pinnedActions, addedHardenRunner, addedPermissions, replacedMaintainedActions := false, false, false, false
ignoreMissingKBs := false
enableLogging := false
addEmptyTopLevelPermissions := false
exemptedActions, pinToImmutable, maintainedActionsMap := []string{}, false, map[string]string{}

if len(params) > 0 {
Expand Down Expand Up @@ -68,6 +69,10 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d
enableLogging = true
}

if queryStringParams["addEmptyTopLevelPermissions"] == "true" {
addEmptyTopLevelPermissions = true
}

if enableLogging {
// Log query parameters
paramsJSON, _ := json.MarshalIndent(queryStringParams, "", " ")
Expand All @@ -83,7 +88,7 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d
if enableLogging {
log.Printf("Adding job level permissions")
}
secureWorkflowReponse, err = permissions.AddJobLevelPermissions(secureWorkflowReponse.FinalOutput)
secureWorkflowReponse, err = permissions.AddJobLevelPermissions(secureWorkflowReponse.FinalOutput, addEmptyTopLevelPermissions)
secureWorkflowReponse.OriginalInput = inputYaml
if err != nil {
if enableLogging {
Expand All @@ -95,7 +100,7 @@ func SecureWorkflow(queryStringParams map[string]string, inputYaml string, svc d
if enableLogging {
log.Printf("Adding workflow level permissions")
}
secureWorkflowReponse.FinalOutput, err = permissions.AddWorkflowLevelPermissions(secureWorkflowReponse.FinalOutput, addProjectComment)
secureWorkflowReponse.FinalOutput, err = permissions.AddWorkflowLevelPermissions(secureWorkflowReponse.FinalOutput, addProjectComment, addEmptyTopLevelPermissions)
if err != nil {
if enableLogging {
log.Printf("Error adding workflow level permissions: %v", err)
Expand Down
100 changes: 100 additions & 0 deletions remediation/workflow/secureworkflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,3 +277,103 @@ func TestSecureWorkflow(t *testing.T) {

}
}

func TestSecureWorkflowEmptyPermissions(t *testing.T) {
const inputDirectory = "../../testfiles/secureworkflow/input"
const outputDirectory = "../../testfiles/secureworkflow/output"

httpmock.Activate()
defer httpmock.DeactivateAndReset()

// Mock APIs for actions/checkout
httpmock.RegisterResponder("GET", "https://api.github.com/repos/actions/checkout/commits/v2",
httpmock.NewStringResponder(200, `ee0669bd1cc54295c223e0bb666b733df41de1c5`))

httpmock.RegisterResponder("GET", "https://api.github.com/repos/actions/checkout/git/matching-refs/tags/v2.",
httpmock.NewStringResponder(200,
`[
{
"ref": "refs/tags/v2.7.0",
"object": {
"sha": "ee0669bd1cc54295c223e0bb666b733df41de1c5",
"type": "commit"
}
}
]`),
)

// Mock APIs for actions/setup-node
httpmock.RegisterResponder("GET", "https://api.github.com/repos/actions/setup-node/commits/v1",
httpmock.NewStringResponder(200, `f1f314fca9dfce2769ece7d933488f076716723e`))

httpmock.RegisterResponder("GET", "https://api.github.com/repos/actions/setup-node/git/matching-refs/tags/v1.",
httpmock.NewStringResponder(200,
`[
{
"ref": "refs/tags/v1.4.6",
"object": {
"sha": "f1f314fca9dfce2769ece7d933488f076716723e",
"type": "commit"
}
}
]`),
)

// Mock APIs for step-security/harden-runner
httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/harden-runner/commits/v2",
httpmock.NewStringResponder(200, `17d0e2bd7d51742c71671bd19fa12bdc9d40a3d6`))

httpmock.RegisterResponder("GET", "https://api.github.com/repos/step-security/harden-runner/git/matching-refs/tags/v2.",
httpmock.NewStringResponder(200,
`[
{
"ref": "refs/tags/v2.8.1",
"object": {
"sha": "17d0e2bd7d51742c71671bd19fa12bdc9d40a3d6",
"type": "commit"
}
}
]`),
)

var err error
var input []byte
input, err = ioutil.ReadFile(path.Join(inputDirectory, "empty-permissions.yml"))

if err != nil {
log.Fatal(err)
}

os.Setenv("KBFolder", "../../knowledge-base/actions")

queryParams := make(map[string]string)
queryParams["addEmptyTopLevelPermissions"] = "true"
queryParams["addProjectComment"] = "false"

output, err := SecureWorkflow(queryParams, string(input), &mockDynamoDBClient{})

if err != nil {
t.Errorf("Error not expected")
}

expectedOutput, err := ioutil.ReadFile(path.Join(outputDirectory, "empty-permissions.yml"))

if err != nil {
log.Fatal(err)
}

if output.FinalOutput != string(expectedOutput) {
// Write the actual output to a file for debugging
debugFile := path.Join(outputDirectory, "empty-permissions-debug.yml")
err := ioutil.WriteFile(debugFile, []byte(output.FinalOutput), 0644)
if err != nil {
t.Logf("Failed to write debug file: %v", err)
} else {
t.Logf("Actual output written to: %s", debugFile)
}

t.Errorf("test failed empty-permissions.yml did not match expected output\nExpected:\n%s\n\nGot:\n%s",
string(expectedOutput), output.FinalOutput)
}

}
13 changes: 13 additions & 0 deletions testfiles/joblevelpermskb/input/empty-top-level-permissions.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: "checkout only workflow"

on:
push:


jobs:
checkout-job:
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v3
15 changes: 15 additions & 0 deletions testfiles/joblevelpermskb/output/empty-top-level-permissions.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: "checkout only workflow"

on:
push:


jobs:
checkout-job:
permissions:
contents: read # for actions/checkout to fetch code
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v3
12 changes: 12 additions & 0 deletions testfiles/secureworkflow/input/empty-permissions.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
on:
push:

jobs:
test:
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v2
- name: Setup Node
uses: actions/setup-node@v1
21 changes: 21 additions & 0 deletions testfiles/secureworkflow/output/empty-permissions.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
on:
push:

permissions: {}

jobs:
test:
permissions:
contents: read # for actions/checkout to fetch code
runs-on: ubuntu-latest

steps:
- name: Harden the runner (Audit all outbound calls)
uses: step-security/harden-runner@17d0e2bd7d51742c71671bd19fa12bdc9d40a3d6 # v2.8.1
with:
egress-policy: audit

- name: Checkout
uses: actions/checkout@ee0669bd1cc54295c223e0bb666b733df41de1c5 # v2.7.0
- name: Setup Node
uses: actions/setup-node@f1f314fca9dfce2769ece7d933488f076716723e # v1.4.6
Loading