diff --git a/remediation/workflow/pin/pinactions.go b/remediation/workflow/pin/pinactions.go index 0b66e63ad..bbf47d7d9 100644 --- a/remediation/workflow/pin/pinactions.go +++ b/remediation/workflow/pin/pinactions.go @@ -80,37 +80,67 @@ func PinAction(action, inputYaml string, exemptedActions []string, pinToImmutabl return inputYaml, updated } - pinnedAction := fmt.Sprintf("%s@%s # %s", leftOfAt[0], commitSHA, tagOrBranch) + // pinnedAction := fmt.Sprintf("%s@%s # %s", leftOfAt[0], commitSHA, tagOrBranch) + // build separately so we can quote only the ref, not the comment + pinnedRef := fmt.Sprintf("%s@%s", leftOfAt[0], commitSHA) + comment := fmt.Sprintf(" # %s", tagOrBranch) + fullPinned := pinnedRef + comment // if the action with version is immutable, then pin the action with version instead of sha pinnedActionWithVersion := fmt.Sprintf("%s@%s", leftOfAt[0], tagOrBranch) if pinToImmutable && semanticTagRegex.MatchString(tagOrBranch) && IsImmutableAction(pinnedActionWithVersion) { - pinnedAction = pinnedActionWithVersion - } + // strings.ReplaceAll is not suitable here because it would incorrectly replace substrings + // For example, if we want to replace "actions/checkout@v1" to "actions/checkout@v1.2.3", it would also incorrectly match and replace in "actions/checkout@v1.2.3" + // making new string to "actions/checkout@v1.2.3.2.3" + // + // Instead, we use a regex pattern that ensures we only replace complete action references: + // Pattern: (@)($|\s|"|') + // - Group 1 (@): Captures the exact action reference + // - Group 2 ($|\s|"|'): Captures the delimiter that follows (end of line, whitespace, or quotes) + // + // Examples: + // - "actions/checkout@v1.2.3" - No match (no delimiter after v1) + // - "actions/checkout@v1 " - Matches (space delimiter) + // - "actions/checkout@v1"" - Matches (quote delimiter) + // - "actions/checkout@v1" - Matches (quote delimiter) + // - "actions/checkout@v1\n" - Matches (newline is considered whitespace \s) + + actionRegex := regexp.MustCompile(`(` + regexp.QuoteMeta(action) + `)($|\s|"|')`) + inputYaml = actionRegex.ReplaceAllString(inputYaml, pinnedActionWithVersion+"$2") + + inputYaml, _ = removePreviousActionComments(pinnedActionWithVersion, inputYaml) + return inputYaml, !strings.EqualFold(action, pinnedActionWithVersion) + } + + updated = !strings.EqualFold(action, fullPinned) + + // 1) Double-quoted form: "owner/repo@oldRef" + doubleQuotedPattern := `"` + regexp.QuoteMeta(action) + `"` + `($|\s|"|')` + doubleQuotedRe := regexp.MustCompile(doubleQuotedPattern) + inputYaml = doubleQuotedRe.ReplaceAllString( + inputYaml, + fmt.Sprintf(`"%s"%s$1`, pinnedRef, comment), + ) + inputYaml, _ = removePreviousActionComments(fmt.Sprintf(`"%s"%s`, pinnedRef, comment), inputYaml) + + // 2) Single-quoted form: 'owner/repo@oldRef' + singleQuotedPattern := `'` + regexp.QuoteMeta(action) + `'` + `($|\s|"|')` + singleQuotedRe := regexp.MustCompile(singleQuotedPattern) + inputYaml = singleQuotedRe.ReplaceAllString( + inputYaml, + fmt.Sprintf(`'%s'%s$1`, pinnedRef, comment), + ) + inputYaml, _ = removePreviousActionComments(fmt.Sprintf(`'%s'%s`, pinnedRef, comment), inputYaml) + + // 3) Unquoted form: owner/repo@oldRef + unqPattern := `\b` + regexp.QuoteMeta(action) + `\b` + `($|\s|"|')` + unqRe := regexp.MustCompile(unqPattern) + inputYaml = unqRe.ReplaceAllString( + inputYaml, + fullPinned+`$1`, + ) + inputYaml, _ = removePreviousActionComments(fullPinned, inputYaml) - updated = !strings.EqualFold(action, pinnedAction) - - // strings.ReplaceAll is not suitable here because it would incorrectly replace substrings - // For example, if we want to replace "actions/checkout@v1" to "actions/checkout@v1.2.3", it would also incorrectly match and replace in "actions/checkout@v1.2.3" - // making new string to "actions/checkout@v1.2.3.2.3" - // - // Instead, we use a regex pattern that ensures we only replace complete action references: - // Pattern: (@)($|\s|"|') - // - Group 1 (@): Captures the exact action reference - // - Group 2 ($|\s|"|'): Captures the delimiter that follows (end of line, whitespace, or quotes) - // - // Examples: - // - "actions/checkout@v1.2.3" - No match (no delimiter after v1) - // - "actions/checkout@v1 " - Matches (space delimiter) - // - "actions/checkout@v1"" - Matches (quote delimiter) - // - "actions/checkout@v1" - Matches (quote delimiter) - // - "actions/checkout@v1\n" - Matches (newline is considered whitespace \s) - actionRegex := regexp.MustCompile(`(` + regexp.QuoteMeta(action) + `)($|\s|"|')`) - inputYaml = actionRegex.ReplaceAllString(inputYaml, pinnedAction+"$2") - yamlWithPreviousActionCommentsRemoved, wasModified := removePreviousActionComments(pinnedAction, inputYaml) - if wasModified { - return yamlWithPreviousActionCommentsRemoved, updated - } return inputYaml, updated } diff --git a/remediation/workflow/pin/pinactions_test.go b/remediation/workflow/pin/pinactions_test.go index 7b36a2cb5..37a842cbe 100644 --- a/remediation/workflow/pin/pinactions_test.go +++ b/remediation/workflow/pin/pinactions_test.go @@ -295,6 +295,7 @@ func TestPinActions(t *testing.T) { {fileName: "immutableaction-1.yml", wantUpdated: true, pinToImmutable: true}, {fileName: "exemptaction.yml", wantUpdated: true, exemptedActions: []string{"actions/checkout", "rohith/*"}, pinToImmutable: true}, {fileName: "donotpintoimmutable.yml", wantUpdated: true, pinToImmutable: false}, + {fileName: "invertedcommas.yml", wantUpdated: true, pinToImmutable: false}, } for _, tt := range tests { input, err := ioutil.ReadFile(path.Join(inputDirectory, tt.fileName)) diff --git a/testfiles/pinactions/input/invertedcommas.yml b/testfiles/pinactions/input/invertedcommas.yml new file mode 100644 index 000000000..c4fce8311 --- /dev/null +++ b/testfiles/pinactions/input/invertedcommas.yml @@ -0,0 +1,15 @@ +name: "close issue" + +on: + push: + +jobs: + closeissue: + runs-on: ubuntu-latest + + steps: + - name: Close Issue + uses: "peter-evans/close-issue@v1" + with: + issue-number: 1 + comment: Auto-closing issue diff --git a/testfiles/pinactions/output/invertedcommas.yml b/testfiles/pinactions/output/invertedcommas.yml new file mode 100644 index 000000000..d5691aaf2 --- /dev/null +++ b/testfiles/pinactions/output/invertedcommas.yml @@ -0,0 +1,15 @@ +name: "close issue" + +on: + push: + +jobs: + closeissue: + runs-on: ubuntu-latest + + steps: + - name: Close Issue + uses: "peter-evans/close-issue@a700eac5bf2a1c7a8cb6da0c13f93ed96fd53dbe" # v1.0.3 + with: + issue-number: 1 + comment: Auto-closing issue