Skip to content

Commit 59ccd75

Browse files
updating pinnig logic to use immutable version instead of hash
1 parent 1281489 commit 59ccd75

14 files changed

+184
-31
lines changed

remediation/workflow/pin/action_image_manifest.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,16 @@ import (
66
"regexp"
77
"strings"
88

9+
"net/http"
10+
911
"github.com/google/go-containerregistry/pkg/name"
1012
"github.com/google/go-containerregistry/pkg/v1/remote"
1113
"github.com/sirupsen/logrus"
1214
)
1315

1416
var (
1517
githubImmutableActionArtifactType = "application/vnd.github.actions.package.v1+json"
16-
tagRegex = regexp.MustCompile(`v[0-9]+\.[0-9]+\.[0-9]+$`)
18+
semanticTagRegex = regexp.MustCompile(`v[0-9]+\.[0-9]+\.[0-9]+$`)
1719
)
1820

1921
type ociManifest struct {
@@ -71,7 +73,7 @@ func getOCIImageArtifactTypeForGhAction(action string) (string, error) {
7173
// use regexp to match tag version format and replace v in prefix
7274
// as immutable actions image tag is in format 1.x.x (without v prefix)
7375
// REF - https://github.com/actions/publish-immutable-action/issues/216#issuecomment-2549914784
74-
if tagRegex.MatchString(parts[1]) {
76+
if semanticTagRegex.MatchString(parts[1]) {
7577
// v1.x.x -> 1.x.x
7678
parts[1] = strings.TrimPrefix(parts[1], "v")
7779
}
@@ -101,7 +103,7 @@ func getOCIManifestForImage(imageRef string) (string, error) {
101103
}
102104

103105
// Get the image manifest
104-
desc, err := remote.Get(ref)
106+
desc, err := remote.Get(ref, remote.WithTransport(http.DefaultTransport))
105107
if err != nil {
106108
return "", fmt.Errorf("error getting manifest: %v", err)
107109
}

remediation/workflow/pin/action_image_manifest_test.go

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,45 @@
11
package pin
22

33
import (
4+
"crypto/tls"
45
"io/ioutil"
56
"net/http"
67
"net/http/httptest"
7-
"net/url"
8+
"path/filepath"
89
"strings"
910
"testing"
1011
)
1112

12-
var (
13-
testFilesDir = "../../../testfiles/pinactions/immutableActionResponses/"
14-
)
13+
type customTransport struct {
14+
base http.RoundTripper
15+
baseURL string
16+
}
17+
18+
func (t *customTransport) RoundTrip(req *http.Request) (*http.Response, error) {
19+
if strings.Contains(req.URL.Host, "ghcr.io") {
20+
req2 := req.Clone(req.Context())
21+
req2.URL.Scheme = "https"
22+
req2.URL.Host = strings.TrimPrefix(t.baseURL, "https://")
23+
return t.base.RoundTrip(req2)
24+
}
25+
return t.base.RoundTrip(req)
26+
}
1527

16-
func createTestServer(t *testing.T) *httptest.Server {
17-
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
28+
func createGhesTestServer(t *testing.T) *httptest.Server {
29+
return httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
1830

1931
w.Header().Set("Content-Type", "application/json")
2032

33+
if !strings.Contains(r.Host, "ghcr.io") {
34+
w.WriteHeader(http.StatusNotFound)
35+
return
36+
}
2137
// Mock manifest endpoints
2238
switch r.URL.Path {
2339

40+
case "/v2/": // simulate ping request
41+
w.WriteHeader(http.StatusOK)
42+
2443
case "/token":
2544
// for immutable actions, since image will be present in registry...it returns 200 OK with token
2645
// otherwise it returns 403 Forbidden
@@ -29,6 +48,7 @@ func createTestServer(t *testing.T) *httptest.Server {
2948
case "repository:actions/checkout:pull":
3049
fallthrough
3150
case "repository:step-security/wait-for-secrets:pull":
51+
3252
w.WriteHeader(http.StatusOK)
3353
w.Write([]byte(`{"token": "test-token", "access_token": "test-token"}`))
3454
default:
@@ -38,6 +58,8 @@ func createTestServer(t *testing.T) *httptest.Server {
3858

3959
case "/v2/actions/checkout/manifests/4.2.2":
4060
fallthrough
61+
case "/v2/actions/checkout/manifests/1.2.0":
62+
fallthrough
4163
case "/v2/step-security/wait-for-secrets/manifests/1.2.0":
4264
w.Write(readHttpResponseForAction(t, r.URL.Path))
4365
case "/v2/actions/checkout/manifests/1.2.3": // since this version doesn't exist
@@ -51,23 +73,29 @@ func createTestServer(t *testing.T) *httptest.Server {
5173

5274
func Test_isImmutableAction(t *testing.T) {
5375
// Create test server that mocks GitHub Container Registry
54-
server := createTestServer(t)
76+
server := createGhesTestServer(t)
5577
defer server.Close()
5678

5779
// Create a custom client that redirects ghcr.io to our test server
5880
originalClient := http.DefaultClient
5981
http.DefaultClient = &http.Client{
60-
Transport: &http.Transport{
61-
Proxy: func(req *http.Request) (*url.URL, error) {
62-
if strings.Contains(req.URL.Host, "ghcr.io") {
63-
return url.Parse(server.URL)
64-
}
65-
return nil, nil
82+
Transport: &customTransport{
83+
base: &http.Transport{
84+
TLSClientConfig: &tls.Config{
85+
InsecureSkipVerify: true,
86+
},
6687
},
88+
baseURL: server.URL,
6789
},
6890
}
91+
92+
// update default transport
93+
OriginalTransport := http.DefaultTransport
94+
http.DefaultTransport = http.DefaultClient.Transport
95+
6996
defer func() {
7097
http.DefaultClient = originalClient
98+
http.DefaultTransport = OriginalTransport
7199
}()
72100

73101
tests := []struct {
@@ -120,14 +148,15 @@ func Test_isImmutableAction(t *testing.T) {
120148

121149
func readHttpResponseForAction(t *testing.T, actionPath string) []byte {
122150
// remove v2 prefix from action path
123-
actionPath = strings.TrimPrefix(actionPath, "v2/")
151+
actionPath = strings.TrimPrefix(actionPath, "/v2/")
124152

125-
fileName := strings.ReplaceAll(actionPath, "/", "-")
126-
respFilePath := testFilesDir + fileName
153+
fileName := strings.ReplaceAll(actionPath, "/", "-") + ".json"
154+
testFilesDir := "../../../testfiles/pinactions/immutableActionResponses/"
155+
respFilePath := filepath.Join(testFilesDir, fileName)
127156

128157
resp, err := ioutil.ReadFile(respFilePath)
129158
if err != nil {
130-
t.Fatalf("error reading test file")
159+
t.Fatalf("error reading test file:%v", err)
131160
}
132161

133162
return resp

remediation/workflow/pin/pinactions.go

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"os"
7+
"regexp"
78
"strings"
89

910
"github.com/google/go-github/v40/github"
@@ -74,8 +75,32 @@ func PinAction(action, inputYaml string) (string, bool) {
7475
}
7576

7677
pinnedAction := fmt.Sprintf("%s@%s # %s", leftOfAt[0], commitSHA, tagOrBranch)
78+
79+
// if the action with version is immutable, then pin the action with version instead of sha
80+
pinnedActionWithVersion := fmt.Sprintf("%s@%s", leftOfAt[0], tagOrBranch)
81+
if semanticTagRegex.MatchString(tagOrBranch) && IsImmutableAction(pinnedActionWithVersion) {
82+
pinnedAction = pinnedActionWithVersion
83+
}
84+
7785
updated = !strings.EqualFold(action, pinnedAction)
78-
inputYaml = strings.ReplaceAll(inputYaml, action, pinnedAction)
86+
87+
// strings.ReplaceAll is not suitable here because it would incorrectly replace substrings
88+
// For example, if we want to replace "actions/checkout@v1" to "actions/[email protected]", it would also incorrectly match and replace in "actions/[email protected]"
89+
// making new string to "actions/[email protected]"
90+
//
91+
// Instead, we use a regex pattern that ensures we only replace complete action references:
92+
// Pattern: (<action>@<version>)($|\s|"|')
93+
// - Group 1 (<action>@<version>): Captures the exact action reference
94+
// - Group 2 ($|\s|"|'): Captures the delimiter that follows (end of line, whitespace, or quotes)
95+
//
96+
// Examples:
97+
// - "actions/[email protected]" - No match (no delimiter after v1)
98+
// - "actions/checkout@v1 " - Matches (space delimiter)
99+
// - "actions/checkout@v1"" - Matches (quote delimiter)
100+
// - "actions/checkout@v1" - Matches (quote delimiter)
101+
// - "actions/checkout@v1\n" - Matches (newline is considered whitespace \s)
102+
actionRegex := regexp.MustCompile(`(` + regexp.QuoteMeta(action) + `)($|\s|"|')`)
103+
inputYaml = actionRegex.ReplaceAllString(inputYaml, pinnedAction+"$2")
79104
yamlWithPreviousActionCommentsRemoved, wasModified := removePreviousActionComments(pinnedAction, inputYaml)
80105
if wasModified {
81106
return yamlWithPreviousActionCommentsRemoved, updated

remediation/workflow/pin/pinactions_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ package pin
33
import (
44
"io/ioutil"
55
"log"
6+
"net/http"
67
"path"
8+
"strings"
79
"testing"
810

911
"github.com/jarcoal/httpmock"
@@ -171,6 +173,78 @@ func TestPinActions(t *testing.T) {
171173
}
172174
]`))
173175

176+
// mock ping response
177+
httpmock.RegisterResponder("GET", "https://ghcr.io/v2/",
178+
httpmock.NewStringResponder(200, ``))
179+
180+
// Mock token endpoints
181+
httpmock.RegisterResponder("GET", "https://ghcr.io/token",
182+
func(req *http.Request) (*http.Response, error) {
183+
scope := req.URL.Query().Get("scope")
184+
switch scope {
185+
// Following are the ones which simulate the image existance in ghcr
186+
case "repository:actions/checkout:pull",
187+
"repository:step-security/wait-for-secrets:pull",
188+
"repository:actions/setup-node:pull",
189+
"repository:peter-evans/close-issue:pull",
190+
"repository:borales/actions-yarn:pull",
191+
"repository:JS-DevTools/npm-publish:pull",
192+
"repository:elgohr/Publish-Docker-Github-Action:pull",
193+
"repository:brandedoutcast/publish-nuget:pull",
194+
"repository:rohith/publish-nuget:pull":
195+
return httpmock.NewJsonResponse(http.StatusOK, map[string]string{
196+
"token": "test-token",
197+
"access_token": "test-token",
198+
})
199+
default:
200+
return httpmock.NewJsonResponse(http.StatusForbidden, map[string]interface{}{
201+
"errors": []map[string]string{
202+
{
203+
"code": "DENIED",
204+
"message": "requested access to the resource is denied",
205+
},
206+
},
207+
})
208+
}
209+
})
210+
211+
// Mock manifest endpoints for specific versions and commit hashes
212+
manifestResponders := []string{
213+
// the following list will contain the list of actions with versions
214+
// which are mocked to be immutable
215+
"actions/[email protected]",
216+
}
217+
218+
for _, action := range manifestResponders {
219+
actionPath := strings.Split(action, "@")[0]
220+
version := strings.TrimPrefix(strings.Split(action, "@")[1], "v")
221+
// Mock manifest response so that we can treat action as immutable
222+
httpmock.RegisterResponder("GET", "https://ghcr.io/v2/"+actionPath+"/manifests/"+version,
223+
func(req *http.Request) (*http.Response, error) {
224+
return httpmock.NewJsonResponse(http.StatusOK, map[string]interface{}{
225+
"schemaVersion": 2,
226+
"mediaType": "application/vnd.github.actions.package.v1+json",
227+
"artifactType": "application/vnd.github.actions.package.v1+json",
228+
"config": map[string]interface{}{
229+
"mediaType": "application/vnd.github.actions.package.v1+json",
230+
},
231+
})
232+
})
233+
}
234+
235+
// Default manifest response for non-existent tags
236+
httpmock.RegisterResponder("GET", `=~^https://ghcr\.io/v2/.*/manifests/.*`,
237+
func(req *http.Request) (*http.Response, error) {
238+
return httpmock.NewJsonResponse(http.StatusNotFound, map[string]interface{}{
239+
"errors": []map[string]string{
240+
{
241+
"code": "MANIFEST_UNKNOWN",
242+
"message": "manifest unknown",
243+
},
244+
},
245+
})
246+
})
247+
174248
tests := []struct {
175249
fileName string
176250
wantUpdated bool
@@ -184,6 +258,7 @@ func TestPinActions(t *testing.T) {
184258
{fileName: "multipleactions.yml", wantUpdated: true},
185259
{fileName: "actionwithcomment.yml", wantUpdated: true},
186260
{fileName: "repeatedactionwithcomment.yml", wantUpdated: true},
261+
{fileName: "immutableaction-1.yml", wantUpdated: true},
187262
}
188263
for _, tt := range tests {
189264
input, err := ioutil.ReadFile(path.Join(inputDirectory, tt.fileName))
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
name: Integration Test Github
2+
on: [push]
3+
jobs:
4+
build:
5+
runs-on: ubuntu-latest
6+
steps:
7+
- uses: actions/[email protected]
8+
- uses: borales/[email protected]
9+
with:
10+
auth-token: ${{ secrets.GITHUB_TOKEN }}
11+
registry-url: npm.pkg.github.com

testfiles/pinactions/output/actionwithcomment.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ jobs:
1616
publish:
1717
runs-on: ubuntu-latest
1818
steps:
19-
- uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0
19+
- uses: actions/[email protected]
2020
- uses: actions/setup-node@f1f314fca9dfce2769ece7d933488f076716723e # v1.4.6
2121
with:
2222
node-version: 10

testfiles/pinactions/output/dockeraction.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ jobs:
1010
runs-on: ubuntu-latest
1111
steps:
1212
- name: Checkout
13-
uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0
13+
uses: actions/[email protected]
1414
- name: Integration test
1515
uses: docker://ghcr.io/step-security/integration-test/int:latest
1616
env:
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
name: Integration Test Github
2+
on: [push]
3+
jobs:
4+
build:
5+
runs-on: ubuntu-latest
6+
steps:
7+
- uses: actions/[email protected]
8+
- uses: borales/actions-yarn@4965e1a0f0ae9c422a9a5748ebd1fb5e097d22b9 # v2.3.0
9+
with:
10+
auth-token: ${{ secrets.GITHUB_TOKEN }}
11+
registry-url: npm.pkg.github.com

testfiles/pinactions/output/localaction.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ jobs:
1414
- uses: actions/setup-node@f1f314fca9dfce2769ece7d933488f076716723e # v1.4.6
1515
with:
1616
node-version: 12.x
17-
- uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0
17+
- uses: actions/[email protected]
1818
- run: npm ci
1919
- run: npm run build
2020
- run: npm run format-check
@@ -32,7 +32,7 @@ jobs:
3232
steps:
3333
# Clone this repo
3434
- name: Checkout
35-
uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0
35+
uses: actions/[email protected]
3636

3737
# Basic checkout
3838
- name: Checkout basic
@@ -150,7 +150,7 @@ jobs:
150150
steps:
151151
# Clone this repo
152152
- name: Checkout
153-
uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0
153+
uses: actions/[email protected]
154154

155155
# Basic checkout using git
156156
- name: Checkout basic
@@ -182,7 +182,7 @@ jobs:
182182
steps:
183183
# Clone this repo
184184
- name: Checkout
185-
uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0
185+
uses: actions/[email protected]
186186

187187
# Basic checkout using git
188188
- name: Checkout basic

testfiles/pinactions/output/multipleactions.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ jobs:
44
publish:
55
runs-on: ubuntu-latest
66
steps:
7-
- uses: actions/checkout@544eadc6bf3d226fd7a7a9f0dc5b5bf7ca0675b9 # v1.2.0
7+
- uses: actions/[email protected]
88
- uses: actions/setup-node@f1f314fca9dfce2769ece7d933488f076716723e # v1.4.6
99
with:
1010
node-version: 10

0 commit comments

Comments
 (0)