From f048dbd2f178951ee2532ae9b6f0c641b9e97964 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Wed, 22 Apr 2026 20:47:10 -0500 Subject: [PATCH 1/4] feat: add commit reachability checks to detect fork-network injection Adds reachability verification using the GitHub compare API to catch supply chain attacks where a SHA exists in the shared object store but is not on the canonical repository's lineage. Detection: - check command: UNREACHABLE = validation failure (fail-closed) - pin/upgrade commands: UNREACHABLE = warning only (defense-in-depth) - API errors (rate limit, 500) = Unknown, warn but don't block Tests model 4 real-world supply chain attacks: - tj-actions/changed-files (CVE-2025-30066) - reviewdog/action-setup (CVE-2025-30154) - aquasecurity/trivy-action (CVE-2026-33634) - Checkmarx KICS (TeamPCP lateral movement) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- command_test.go | 464 +++++++++++++++++++++++++++++ internal/httpmock/httpmock.go | 25 ++ internal/resolver/resolver.go | 125 ++++++++ internal/resolver/resolver_test.go | 185 ++++++++++++ root.go | 34 +++ 5 files changed, 833 insertions(+) diff --git a/command_test.go b/command_test.go index 180914f9..4e85cd94 100644 --- a/command_test.go +++ b/command_test.go @@ -97,6 +97,8 @@ func TestCheckCommand_JSONWithHTTPMocks(t *testing.T) { }, }), ) + compareReachable(reg, `/repos/actions/checkout/compare/`) + compareReachable(reg, `/repos/actions/setup-go/compare/`) workflowPath := writeTempWorkflow(t, ` name: ci @@ -287,6 +289,24 @@ dependencies: const nodeActionYAML = "name: Test Action\nruns:\n using: node20\n" +// compareReachable registers a REST compare stub that returns "identical" for +// any compare request matching the given path pattern, simulating a reachable commit. +func compareReachable(reg *httpmock.Registry, pathPattern string) { + reg.Register( + httpmock.REST("GET", pathPattern), + httpmock.JSONResponse(map[string]any{"status": "identical"}), + ) +} + +// compareUnreachable registers a REST compare stub that returns "diverged", +// simulating a fork-network injected commit. +func compareUnreachable(reg *httpmock.Registry, pathPattern string) { + reg.Register( + httpmock.REST("GET", pathPattern), + httpmock.JSONResponse(map[string]any{"status": "diverged"}), + ) +} + func testRepoResponse(nameWithOwner, oid, actionYAML string) map[string]any { return map[string]any{ "nameWithOwner": nameWithOwner, @@ -348,3 +368,447 @@ func runCommandWithHTTP(t *testing.T, rt http.RoundTripper, args ...string) (str return string(stdoutBytes), string(stderrBytes), runErr } + +// ========================================================================== +// Supply Chain Attack Reachability Tests +// +// These tests model real-world attacks where tag mutation or fork-network +// injection was used to compromise GitHub Actions. The reachability check +// should catch cases where a pinned SHA exists in the GitHub fork network +// but is NOT on the canonical repository's ref lineage. +// +// References: +// - tj-actions/changed-files (CVE-2025-30066): tag v44 pointed to malicious commit from fork +// - reviewdog/action-setup: tag mutation via compromised PAT +// - xygeni/xygeni-action: C2 reverse shell backdoor via tag poisoning +// - aquasecurity/trivy-action: scanner-to-stealer tag manipulation +// ========================================================================== + +// TestCheck_TjActionsChangedFiles_TagMutationAttack models the March 2025 +// tj-actions/changed-files attack (CVE-2025-30066) where attackers +// compromised a maintainer PAT and force-pushed tag v44 to a malicious +// commit. The malicious commit is NOT reachable from the legitimate tag. +func TestCheck_TjActionsChangedFiles_TagMutationAttack(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + // The legitimate pinned SHA from before the attack + legitimateSHA := "4edd678ac3f81e2dc578756871e4d00c19191c4e" + // The attacker's SHA that the tag was force-pushed to + maliciousSHA := "0e58ed8671d6b60d0890c21b07f8835ace038e67" + + // Live resolution returns the MALICIOUS SHA (tag was moved) + reg.Register( + httpmock.GraphQL(`repository\(owner: "tj-actions", name: "changed-files"\)`), + httpmock.JSONResponse(map[string]any{ + "data": map[string]any{ + "a0": testRepoResponse("tj-actions/changed-files", maliciousSHA, nodeActionYAML), + }, + }), + ) + // The malicious SHA is diverged from the legitimate branch + compareUnreachable(reg, `/repos/tj-actions/changed-files/compare/`) + + workflowPath := writeTempWorkflow(t, ` +name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: tj-actions/changed-files@v44 + +# Automatically generated and managed by: gh actions-pin --write +dependencies: + - github.com/tj-actions/changed-files@v44:sha1-`+legitimateSHA+` +`) + + stdout, _, err := runCommandWithHTTP(t, reg, + "check", "--json", "valid,errors", workflowPath, + ) + require.NoError(t, err, "JSON mode communicates errors in payload") + + var payload struct { + Valid bool `json:"valid"` + Errors []validationError `json:"errors"` + } + require.NoError(t, json.Unmarshal([]byte(stdout), &payload)) + assert.False(t, payload.Valid) + + // Should detect TAMPERED (SHA changed) AND UNREACHABLE (fork-network) + errorTypes := map[string]bool{} + for _, e := range payload.Errors { + errorTypes[e.Type] = true + } + assert.True(t, errorTypes["TAMPERED"], "should detect SHA tamper: %+v", payload.Errors) + assert.True(t, errorTypes["UNREACHABLE"], "should detect unreachable commit: %+v", payload.Errors) +} + +// TestCheck_ReviewdogActionSetup_ForkNetworkInjection models the reviewdog +// attack where a malicious commit from a fork was referenced via tag +// manipulation. The commit exists in GitHub's shared object store but is +// NOT reachable from the canonical repository's refs. +func TestCheck_ReviewdogActionSetup_ForkNetworkInjection(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + // Attacker's commit from a fork - exists in the network but not the canonical repo + forkNetworkSHA := "b0c14eb73e15d54af9e97eb7fe20e74fa238fd07" + + // Live resolution returns the fork-network SHA (tag was moved to it) + reg.Register( + httpmock.GraphQL(`repository\(owner: "reviewdog", name: "action-setup"\)`), + httpmock.JSONResponse(map[string]any{ + "data": map[string]any{ + "a0": testRepoResponse("reviewdog/action-setup", forkNetworkSHA, nodeActionYAML), + }, + }), + ) + // Compare returns diverged: SHA exists in fork network but not on ref lineage + compareUnreachable(reg, `/repos/reviewdog/action-setup/compare/`) + + workflowPath := writeTempWorkflow(t, ` +name: lint +on: pull_request +jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: reviewdog/action-setup@v1 + +# Automatically generated and managed by: gh actions-pin --write +dependencies: + - github.com/reviewdog/action-setup@v1:sha1-`+forkNetworkSHA+` +`) + + stdout, _, err := runCommandWithHTTP(t, reg, + "check", "--json", "valid,errors", workflowPath, + ) + require.NoError(t, err, "JSON mode communicates errors in payload") + + var payload struct { + Valid bool `json:"valid"` + Errors []validationError `json:"errors"` + } + require.NoError(t, json.Unmarshal([]byte(stdout), &payload)) + assert.False(t, payload.Valid) + + // Even though the SHA matches live resolution (tag still points to malicious commit), + // the reachability check catches the fork-network injection. + hasUnreachable := false + for _, e := range payload.Errors { + if e.Type == "UNREACHABLE" { + hasUnreachable = true + assert.Contains(t, e.Details, "not reachable") + } + } + assert.True(t, hasUnreachable, "should detect fork-network injected commit: %+v", payload.Errors) +} + +// TestCheck_XygeniAction_TagPoisoningWithBackdoor models the xygeni-action +// compromise where a tag was poisoned to inject a C2 reverse shell backdoor. +// The malicious commit is from outside the canonical repo's history. +func TestCheck_XygeniAction_TagPoisoningWithBackdoor(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + maliciousSHA := "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2" + + reg.Register( + httpmock.GraphQL(`repository\(owner: "xygeni", name: "xygeni-action"\)`), + httpmock.JSONResponse(map[string]any{ + "data": map[string]any{ + "a0": testRepoResponse("xygeni/xygeni-action", maliciousSHA, nodeActionYAML), + }, + }), + ) + // The poisoned SHA doesn't exist in the canonical repo at all (404) + reg.Register( + httpmock.REST("GET", `/repos/xygeni/xygeni-action/compare/`), + httpmock.StatusResponse(404), + ) + + workflowPath := writeTempWorkflow(t, ` +name: security-scan +on: push +jobs: + scan: + runs-on: ubuntu-latest + steps: + - uses: xygeni/xygeni-action@v3 + +# Automatically generated and managed by: gh actions-pin --write +dependencies: + - github.com/xygeni/xygeni-action@v3:sha1-`+maliciousSHA+` +`) + + stdout, _, err := runCommandWithHTTP(t, reg, + "check", "--json", "valid,errors", workflowPath, + ) + require.NoError(t, err, "JSON mode communicates errors in payload") + + var payload struct { + Valid bool `json:"valid"` + Errors []validationError `json:"errors"` + } + require.NoError(t, json.Unmarshal([]byte(stdout), &payload)) + assert.False(t, payload.Valid) + + hasUnreachable := false + for _, e := range payload.Errors { + if e.Type == "UNREACHABLE" { + hasUnreachable = true + assert.Contains(t, e.Details, "not found in repository") + } + } + assert.True(t, hasUnreachable, "should detect SHA not in canonical repo: %+v", payload.Errors) +} + +// TestCheck_TrivyAction_ScannerToStealer models the aquasecurity/trivy-action +// compromise where the tag was manipulated to redirect to a malicious version +// that exfiltrated secrets instead of scanning. +func TestCheck_TrivyAction_ScannerToStealer(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + legitimateSHA := "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" + maliciousSHA := "cafebabecafebabecafebabecafebabecafebabe" + + // Live resolution returns the malicious SHA (tag was moved) + reg.Register( + httpmock.GraphQL(`repository\(owner: "aquasecurity", name: "trivy-action"\)`), + httpmock.JSONResponse(map[string]any{ + "data": map[string]any{ + "a0": testRepoResponse("aquasecurity/trivy-action", maliciousSHA, nodeActionYAML), + }, + }), + ) + // The malicious SHA diverges from the legitimate lineage + compareUnreachable(reg, `/repos/aquasecurity/trivy-action/compare/`) + + workflowPath := writeTempWorkflow(t, ` +name: security +on: push +jobs: + scan: + runs-on: ubuntu-latest + steps: + - uses: aquasecurity/trivy-action@master + +# Automatically generated and managed by: gh actions-pin --write +dependencies: + - github.com/aquasecurity/trivy-action@master:sha1-`+legitimateSHA+` +`) + + stdout, _, err := runCommandWithHTTP(t, reg, + "check", "--json", "valid,errors", workflowPath, + ) + require.NoError(t, err, "JSON mode communicates errors in payload") + + var payload struct { + Valid bool `json:"valid"` + Errors []validationError `json:"errors"` + } + require.NoError(t, json.Unmarshal([]byte(stdout), &payload)) + assert.False(t, payload.Valid) + + errorTypes := map[string]bool{} + for _, e := range payload.Errors { + errorTypes[e.Type] = true + } + assert.True(t, errorTypes["TAMPERED"], "should detect SHA changed: %+v", payload.Errors) + assert.True(t, errorTypes["UNREACHABLE"], "should detect fork-network SHA: %+v", payload.Errors) +} + +// TestCheck_CheckmarxKICS_TagForceViaStoredCreds models the March 2026 Checkmarx +// KICS compromise where credentials stolen during the Trivy breach were used to +// force-push malicious code to KICS GitHub Action tags (TeamPCP lateral movement). +func TestCheck_CheckmarxKICS_TagForceViaStoredCreds(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + legitimateSHA := "1111111111111111111111111111111111111111" + maliciousSHA := "2222222222222222222222222222222222222222" + + // Live resolution returns the malicious SHA (tag force-pushed with stolen creds) + reg.Register( + httpmock.GraphQL(`repository\(owner: "Checkmarx", name: "kics-github-action"\)`), + httpmock.JSONResponse(map[string]any{ + "data": map[string]any{ + "a0": testRepoResponse("Checkmarx/kics-github-action", maliciousSHA, nodeActionYAML), + }, + }), + ) + // The malicious SHA diverges from the legitimate lineage + compareUnreachable(reg, `/repos/Checkmarx/kics-github-action/compare/`) + + workflowPath := writeTempWorkflow(t, ` +name: sast +on: push +jobs: + kics: + runs-on: ubuntu-latest + steps: + - uses: Checkmarx/kics-github-action@v2 + +# Automatically generated and managed by: gh actions-pin --write +dependencies: + - github.com/Checkmarx/kics-github-action@v2:sha1-`+legitimateSHA+` +`) + + stdout, _, err := runCommandWithHTTP(t, reg, + "check", "--json", "valid,errors", workflowPath, + ) + require.NoError(t, err, "JSON mode communicates errors in payload") + + var payload struct { + Valid bool `json:"valid"` + Errors []validationError `json:"errors"` + } + require.NoError(t, json.Unmarshal([]byte(stdout), &payload)) + assert.False(t, payload.Valid) + + errorTypes := map[string]bool{} + for _, e := range payload.Errors { + errorTypes[e.Type] = true + } + assert.True(t, errorTypes["TAMPERED"], "should detect SHA changed: %+v", payload.Errors) + assert.True(t, errorTypes["UNREACHABLE"], "should detect fork-network SHA: %+v", payload.Errors) +} + +// TestCheck_ReachabilityUnknown_DoesNotFailValidation verifies that when the +// compare endpoint returns an error (rate limit, timeout, etc.), the check +// command issues a warning but does NOT mark the validation as failed. +func TestCheck_ReachabilityUnknown_DoesNotFailValidation(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + sha := "de0fac2e4500dabe0009e67214ff5f5447ce83dd" + + reg.Register( + httpmock.GraphQL(`repository\(owner: "actions", name: "checkout"\)`), + httpmock.JSONResponse(map[string]any{ + "data": map[string]any{ + "a0": testRepoResponse("actions/checkout", sha, nodeActionYAML), + }, + }), + ) + // Simulate a 500 error from the compare endpoint + reg.Register( + httpmock.REST("GET", `/repos/actions/checkout/compare/`), + httpmock.StatusResponse(500), + ) + + workflowPath := writeTempWorkflow(t, ` +name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + +# Automatically generated and managed by: gh actions-pin --write +dependencies: + - github.com/actions/checkout@v6:sha1-`+sha+` +`) + + stdout, _, err := runCommandWithHTTP(t, reg, + "check", "--json", "valid,errors,warnings", workflowPath, + ) + require.NoError(t, err, "unknown reachability should not fail the check") + + var payload struct { + Valid bool `json:"valid"` + Errors []validationError `json:"errors"` + Warnings []string `json:"warnings"` + } + require.NoError(t, json.Unmarshal([]byte(stdout), &payload)) + assert.True(t, payload.Valid, "valid should be true when reachability is unknown") + assert.Empty(t, payload.Errors) + assert.NotEmpty(t, payload.Warnings, "should have a reachability warning") + assert.Contains(t, payload.Warnings[0], "reachability check inconclusive") +} + +// TestCheck_Reachable_CleanValidation verifies the happy path: pinned SHA +// is reachable, live resolution matches, everything is valid. +func TestCheck_Reachable_CleanValidation(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + sha := "de0fac2e4500dabe0009e67214ff5f5447ce83dd" + + reg.Register( + httpmock.GraphQL(`repository\(owner: "actions", name: "checkout"\)`), + httpmock.JSONResponse(map[string]any{ + "data": map[string]any{ + "a0": testRepoResponse("actions/checkout", sha, nodeActionYAML), + }, + }), + ) + compareReachable(reg, `/repos/actions/checkout/compare/`) + + workflowPath := writeTempWorkflow(t, ` +name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + +# Automatically generated and managed by: gh actions-pin --write +dependencies: + - github.com/actions/checkout@v6:sha1-`+sha+` +`) + + stdout, _, err := runCommandWithHTTP(t, reg, + "check", "--json", "valid,errors,warnings", workflowPath, + ) + require.NoError(t, err) + + var payload struct { + Valid bool `json:"valid"` + Errors []validationError `json:"errors"` + Warnings []string `json:"warnings"` + } + require.NoError(t, json.Unmarshal([]byte(stdout), &payload)) + assert.True(t, payload.Valid) + assert.Empty(t, payload.Errors) + assert.Empty(t, payload.Warnings) +} + +// TestPin_UnreachableSHA_WarnsButDoesNotBlock verifies that when a freshly +// resolved SHA fails the reachability check during pinning, the CLI warns +// but does not block the pin operation (defense-in-depth, not a hard gate). +func TestPin_UnreachableSHA_WarnsButDoesNotBlock(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + sha := "de0fac2e4500dabe0009e67214ff5f5447ce83dd" + + reg.Register( + httpmock.GraphQL(`repository\(owner: "actions", name: "checkout"\)`), + httpmock.JSONResponse(map[string]any{ + "data": map[string]any{ + "a0": testRepoResponse("actions/checkout", sha, nodeActionYAML), + }, + }), + ) + compareUnreachable(reg, `/repos/actions/checkout/compare/`) + + workflowPath := writeTempWorkflow(t, ` +name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 +`) + + _, stderr, err := runCommandWithHTTP(t, reg, "--diff", workflowPath) + require.NoError(t, err, "pin should succeed even with unreachable warning") + assert.Contains(t, stderr, "NOT reachable") + assert.Contains(t, stderr, "fork-network injection") +} diff --git a/internal/httpmock/httpmock.go b/internal/httpmock/httpmock.go index d976c06e..fb341d48 100644 --- a/internal/httpmock/httpmock.go +++ b/internal/httpmock/httpmock.go @@ -147,6 +147,31 @@ func GraphQLQuery(body string, cb func(query string, variables map[string]any)) } } +// REST matches a request by method and URL path pattern (regex). +func REST(method, pathPattern string) Matcher { + re := regexp.MustCompile(pathPattern) + + return func(req *http.Request) bool { + if !strings.EqualFold(req.Method, method) { + return false + } + return re.MatchString(req.URL.Path) + } +} + +// StatusResponse returns a response with the given status code and empty body. +func StatusResponse(code int) Responder { + return func(req *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: code, + Header: http.Header{}, + Body: io.NopCloser(bytes.NewBuffer(nil)), + Request: req, + Status: fmt.Sprintf("%d", code), + }, nil + } +} + func decodeJSONBody(req *http.Request, dest any) error { b, err := readBody(req) if err != nil { diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index b2f95ac1..6df85a75 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "regexp" "strconv" "strings" @@ -15,6 +16,20 @@ import ( "github.com/github/gh-actions-pin/internal/lockfile" ) +// ReachabilityStatus represents the result of a commit reachability check. +type ReachabilityStatus string + +const ( + // Reachable means the SHA is confirmed on the ref's lineage. + Reachable ReachabilityStatus = "reachable" + // Unreachable means the SHA is confirmed NOT on the ref's lineage + // (e.g. it exists only in a fork network). + Unreachable ReachabilityStatus = "unreachable" + // ReachabilityUnknown means the check could not be completed + // (timeout, rate limit, API error). + ReachabilityUnknown ReachabilityStatus = "unknown" +) + // DefaultMaxRecursionDepth matches the runner's composite action recursion limit. const DefaultMaxRecursionDepth = 10 @@ -26,13 +41,25 @@ type resolvedEntry struct { actionYML string } +// ReachabilityResult holds the outcome of a single reachability check. +type ReachabilityResult struct { + Owner string + Repo string + Ref string + SHA string + Status ReachabilityStatus + Detail string // human-readable detail (e.g. compare status or error) +} + // Resolver resolves action refs to commit SHAs. type Resolver struct { client *api.GraphQLClient + restClient *api.RESTClient hostname string MaxRecursionDepth int cache map[string]resolvedEntry latestRefCache map[string]string + reachCache map[string]ReachabilityStatus } // New creates a resolver using the authenticated gh context. @@ -53,12 +80,19 @@ func NewWithOptions(opts api.ClientOptions) (*Resolver, error) { return nil, err } + restClient, err := api.NewRESTClient(opts) + if err != nil { + return nil, err + } + return &Resolver{ client: client, + restClient: restClient, hostname: hostname, MaxRecursionDepth: DefaultMaxRecursionDepth, cache: make(map[string]resolvedEntry), latestRefCache: make(map[string]string), + reachCache: make(map[string]ReachabilityStatus), }, nil } @@ -78,6 +112,97 @@ func (r *Resolver) Hostname() string { return r.hostname } +// CheckReachability verifies that a resolved SHA is on the lineage of the +// given ref within the repository. This catches fork-network injection where +// a SHA exists in GitHub's shared object store but is not actually part of +// the canonical repository's history. +// +// Uses the compare endpoint: GET /repos/{owner}/{repo}/compare/{sha}...{ref} +// - "identical" or "behind" or "ahead" → same lineage → Reachable +// - "diverged" → different lineage → Unreachable +// - 404 → SHA not in repo → Unreachable +// - other errors → Unknown +func (r *Resolver) CheckReachability(owner, repo, sha, ref string) ReachabilityResult { + result := ReachabilityResult{ + Owner: owner, + Repo: repo, + Ref: ref, + SHA: sha, + } + + cacheKey := owner + "/" + repo + "/" + sha + "/" + ref + if status, ok := r.reachCache[cacheKey]; ok { + result.Status = status + result.Detail = "cached" + return result + } + + escapedRef := url.PathEscape(ref) + path := fmt.Sprintf("repos/%s/%s/compare/%s...%s", owner, repo, sha, escapedRef) + + var compare struct { + Status string `json:"status"` + } + err := r.restClient.Get(path, &compare) + if err != nil { + // 404 means the SHA doesn't exist in this repository at all + if strings.Contains(err.Error(), "404") || strings.Contains(err.Error(), "Not Found") { + result.Status = Unreachable + result.Detail = "commit not found in repository" + r.reachCache[cacheKey] = Unreachable + return result + } + result.Status = ReachabilityUnknown + result.Detail = err.Error() + return result + } + + switch compare.Status { + case "identical", "behind", "ahead": + // All mean the SHA is on the same lineage as the ref + result.Status = Reachable + result.Detail = compare.Status + case "diverged": + // SHA exists in the network but is NOT on the ref's lineage + result.Status = Unreachable + result.Detail = "commit exists in fork network but is not on ref lineage" + default: + result.Status = ReachabilityUnknown + result.Detail = fmt.Sprintf("unexpected compare status: %q", compare.Status) + } + + if result.Status != ReachabilityUnknown { + r.reachCache[cacheKey] = result.Status + } + return result +} + +// CheckReachabilityAll runs reachability checks on a batch of dependencies, +// deduplicating by owner/repo/sha/ref. +func (r *Resolver) CheckReachabilityAll(deps []lockfile.Dependency) []ReachabilityResult { + var results []ReachabilityResult + seen := make(map[string]bool) + + for _, dep := range deps { + parts := strings.SplitN(dep.NWO, "/", 3) + if len(parts) < 2 { + continue + } + owner, repo := parts[0], parts[1] + + key := dep.NWO + "/" + dep.SHA + "/" + dep.Ref + if seen[key] { + continue + } + seen[key] = true + + result := r.CheckReachability(owner, repo, dep.SHA, dep.Ref) + results = append(results, result) + } + + return results +} + // LatestRef returns the highest stable tag for an action repository. func (r *Resolver) LatestRef(owner, repo string) (string, error) { key := owner + "/" + repo diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 5fdb11e2..1c5af663 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -131,6 +131,7 @@ func TestResolveAllRecursiveWithCacheAndCompositeExpansion(t *testing.T) { }, }, latestRefCache: map[string]string{}, + reachCache: map[string]ReachabilityStatus{}, } r.cache["owner/composite@v1"] = resolvedEntry{ @@ -173,6 +174,7 @@ func TestResolveAllRecursiveRespectsMaxDepth(t *testing.T) { }, }, latestRefCache: map[string]string{}, + reachCache: map[string]ReachabilityStatus{}, } _, err := r.ResolveAllRecursive([]lockfile.ActionRef{{Owner: "owner", Repo: "composite", Ref: "v1"}}) @@ -285,3 +287,186 @@ func TestResolveAllRecursiveWithHTTPTransport(t *testing.T) { t.Fatalf("expected composite dep to be present, got %+v", deps) } } + +func TestCheckReachability_Reachable(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.REST("GET", `/repos/actions/checkout/compare/`), + httpmock.JSONResponse(map[string]any{"status": "identical"}), + ) + + r, err := NewWithTransport("github.com", reg) + if err != nil { + t.Fatalf("NewWithTransport returned error: %v", err) + } + + result := r.CheckReachability("actions", "checkout", "abc123", "v6") + if result.Status != Reachable { + t.Fatalf("expected Reachable, got %s (%s)", result.Status, result.Detail) + } +} + +func TestCheckReachability_Ahead(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.REST("GET", `/repos/actions/checkout/compare/`), + httpmock.JSONResponse(map[string]any{"status": "ahead"}), + ) + + r, err := NewWithTransport("github.com", reg) + if err != nil { + t.Fatalf("NewWithTransport returned error: %v", err) + } + + result := r.CheckReachability("actions", "checkout", "abc123", "v6") + if result.Status != Reachable { + t.Fatalf("expected Reachable for ahead status, got %s (%s)", result.Status, result.Detail) + } +} + +func TestCheckReachability_Behind(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.REST("GET", `/repos/actions/checkout/compare/`), + httpmock.JSONResponse(map[string]any{"status": "behind"}), + ) + + r, err := NewWithTransport("github.com", reg) + if err != nil { + t.Fatalf("NewWithTransport returned error: %v", err) + } + + result := r.CheckReachability("actions", "checkout", "abc123", "v6") + if result.Status != Reachable { + t.Fatalf("expected Reachable for behind status (tag rolled back), got %s (%s)", result.Status, result.Detail) + } +} + +func TestCheckReachability_Diverged_Unreachable(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.REST("GET", `/repos/evil/repo/compare/`), + httpmock.JSONResponse(map[string]any{"status": "diverged"}), + ) + + r, err := NewWithTransport("github.com", reg) + if err != nil { + t.Fatalf("NewWithTransport returned error: %v", err) + } + + result := r.CheckReachability("evil", "repo", "deadbeef", "v1") + if result.Status != Unreachable { + t.Fatalf("expected Unreachable for diverged status, got %s (%s)", result.Status, result.Detail) + } + if !strings.Contains(result.Detail, "fork network") { + t.Fatalf("expected fork network detail, got %q", result.Detail) + } +} + +func TestCheckReachability_404_Unreachable(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.REST("GET", `/repos/evil/repo/compare/`), + httpmock.StatusResponse(404), + ) + + r, err := NewWithTransport("github.com", reg) + if err != nil { + t.Fatalf("NewWithTransport returned error: %v", err) + } + + result := r.CheckReachability("evil", "repo", "deadbeef", "v1") + if result.Status != Unreachable { + t.Fatalf("expected Unreachable for 404, got %s (%s)", result.Status, result.Detail) + } + if !strings.Contains(result.Detail, "not found") { + t.Fatalf("expected 'not found' detail, got %q", result.Detail) + } +} + +func TestCheckReachability_ServerError_Unknown(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.REST("GET", `/repos/actions/checkout/compare/`), + httpmock.StatusResponse(500), + ) + + r, err := NewWithTransport("github.com", reg) + if err != nil { + t.Fatalf("NewWithTransport returned error: %v", err) + } + + result := r.CheckReachability("actions", "checkout", "abc123", "v6") + if result.Status != ReachabilityUnknown { + t.Fatalf("expected Unknown for 500, got %s (%s)", result.Status, result.Detail) + } +} + +func TestCheckReachability_CachesResults(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + // Only one stub — second call must hit cache + reg.Register( + httpmock.REST("GET", `/repos/actions/checkout/compare/`), + httpmock.JSONResponse(map[string]any{"status": "identical"}), + ) + + r, err := NewWithTransport("github.com", reg) + if err != nil { + t.Fatalf("NewWithTransport returned error: %v", err) + } + + r1 := r.CheckReachability("actions", "checkout", "abc123", "v6") + r2 := r.CheckReachability("actions", "checkout", "abc123", "v6") + + if r1.Status != Reachable || r2.Status != Reachable { + t.Fatalf("expected both calls to return Reachable, got %s and %s", r1.Status, r2.Status) + } + if r2.Detail != "cached" { + t.Fatalf("expected second call to be cached, got detail %q", r2.Detail) + } +} + +func TestCheckReachabilityAll_DeduplicatesRequests(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + // Only one stub for checkout — dedup should prevent double-call + reg.Register( + httpmock.REST("GET", `/repos/actions/checkout/compare/`), + httpmock.JSONResponse(map[string]any{"status": "identical"}), + ) + reg.Register( + httpmock.REST("GET", `/repos/actions/setup-go/compare/`), + httpmock.JSONResponse(map[string]any{"status": "ahead"}), + ) + + r, err := NewWithTransport("github.com", reg) + if err != nil { + t.Fatalf("NewWithTransport returned error: %v", err) + } + + deps := []lockfile.Dependency{ + {NWO: "actions/checkout", Ref: "v6", SHA: "aaa"}, + {NWO: "actions/checkout", Ref: "v6", SHA: "aaa"}, // duplicate + {NWO: "actions/setup-go", Ref: "v6", SHA: "bbb"}, + } + + results := r.CheckReachabilityAll(deps) + if len(results) != 2 { + t.Fatalf("expected 2 unique results, got %d: %+v", len(results), results) + } +} diff --git a/root.go b/root.go index cc11fef8..182aba42 100644 --- a/root.go +++ b/root.go @@ -246,6 +246,7 @@ func newCheckCmd() *cobra.Command { MISSING - uses: ref has no dependencies: entry STALE - dependencies: entry is no longer discoverable SHA_MISMATCH - uses: ref looks like a SHA but resolves elsewhere + UNREACHABLE - SHA is not on the ref's lineage (possible fork-network injection) `), Example: heredoc.Doc(` # Verify all workflows @@ -527,6 +528,20 @@ func pinOneFile(opts *pinOptions, workflowPath string, r *resolver.Resolver) err return fmt.Errorf("%d action ref(s) have SHA-like names that point to different commits", len(mismatches)) } + // Reachability check on freshly resolved deps (warns, does not block) + reachResults := r.CheckReachabilityAll(deps) + for _, rr := range reachResults { + switch rr.Status { + case resolver.Unreachable: + fmt.Fprintf(os.Stderr, "warning: %s/%s@%s: SHA %s is NOT reachable from ref (%s)\n", + rr.Owner, rr.Repo, rr.Ref, rr.SHA[:12], rr.Detail) + fmt.Fprintf(os.Stderr, " This may indicate a fork-network injection attack.\n") + case resolver.ReachabilityUnknown: + fmt.Fprintf(os.Stderr, "warning: %s/%s@%s: reachability check inconclusive (%s)\n", + rr.Owner, rr.Repo, rr.Ref, rr.Detail) + } + } + if len(opts.Actions) > 0 && len(existingDeps) > 0 { deps = mergeTargetedDeps(existingDeps, deps, opts.Actions) } else if len(existingDeps) > 0 && opts.Write { @@ -785,6 +800,25 @@ func validateOneFile(workflowPath string, r *resolver.Resolver) (*validationResu }) } + // Reachability: verify pinned SHAs are on the ref's lineage in the + // canonical repository, not injected from a fork network. + fmt.Fprintf(os.Stderr, "Checking commit reachability for %d dependency(ies)...\n", len(existingDeps)) + reachResults := r.CheckReachabilityAll(existingDeps) + for _, rr := range reachResults { + switch rr.Status { + case resolver.Unreachable: + result.Valid = false + result.Errors = append(result.Errors, validationError{ + Type: "UNREACHABLE", + Dependency: fmt.Sprintf("%s/%s@%s", rr.Owner, rr.Repo, rr.Ref), + Details: fmt.Sprintf("SHA %s is not reachable from ref %s (%s)", rr.SHA[:12], rr.Ref, rr.Detail), + }) + case resolver.ReachabilityUnknown: + result.Warnings = append(result.Warnings, + fmt.Sprintf("%s/%s@%s: reachability check inconclusive (%s)", rr.Owner, rr.Repo, rr.Ref, rr.Detail)) + } + } + if result.Valid { fmt.Fprintf(os.Stderr, "%s valid\n", workflowPath) } From dad274daca1653f4ffcd756093e4b359fcf9226d Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Wed, 22 Apr 2026 20:55:00 -0500 Subject: [PATCH 2/4] test: add live integration tests for reachability checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uses nodeselector/actions-test-fixtures with: - tag v1 on main HEAD (identical → reachable) - root commit of main (ancestor → reachable) - orphan-poison branch commit (no common ancestor → unreachable) - fabricated SHA (404 → unreachable) - cache consistency verification Guarded by //go:build integration — won't run without -tags integration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- command_test.go | 340 ++++-------------- .../resolver/reachability_integration_test.go | 124 +++++++ internal/resolver/resolver.go | 121 ++++--- internal/resolver/resolver_test.go | 173 ++------- 4 files changed, 323 insertions(+), 435 deletions(-) create mode 100644 internal/resolver/reachability_integration_test.go diff --git a/command_test.go b/command_test.go index 4e85cd94..7201e10d 100644 --- a/command_test.go +++ b/command_test.go @@ -97,8 +97,6 @@ func TestCheckCommand_JSONWithHTTPMocks(t *testing.T) { }, }), ) - compareReachable(reg, `/repos/actions/checkout/compare/`) - compareReachable(reg, `/repos/actions/setup-go/compare/`) workflowPath := writeTempWorkflow(t, ` name: ci @@ -116,7 +114,7 @@ dependencies: - github.com/actions/setup-go@v6:sha1-4a3601121dd01d1626a1e23e37211e3254c1c06c `) - stdout, stderr, err := runCommandWithHTTP(t, reg, + stdout, stderr, err := runCommandWithHTTPAndReach(t, reg, reachableFunc(), "check", "--json", "valid,errors,warnings", workflowPath, ) require.NoError(t, err) @@ -289,22 +287,25 @@ dependencies: const nodeActionYAML = "name: Test Action\nruns:\n using: node20\n" -// compareReachable registers a REST compare stub that returns "identical" for -// any compare request matching the given path pattern, simulating a reachable commit. -func compareReachable(reg *httpmock.Registry, pathPattern string) { - reg.Register( - httpmock.REST("GET", pathPattern), - httpmock.JSONResponse(map[string]any{"status": "identical"}), - ) +// reachableFunc returns a checkReachFn that reports all commits as reachable. +func reachableFunc() func(string, string, string, string) (resolver.ReachabilityStatus, string) { + return func(owner, repo, sha, ref string) (resolver.ReachabilityStatus, string) { + return resolver.Reachable, "ancestor of " + ref + } } -// compareUnreachable registers a REST compare stub that returns "diverged", -// simulating a fork-network injected commit. -func compareUnreachable(reg *httpmock.Registry, pathPattern string) { - reg.Register( - httpmock.REST("GET", pathPattern), - httpmock.JSONResponse(map[string]any{"status": "diverged"}), - ) +// unreachableFunc returns a checkReachFn that reports all commits as unreachable. +func unreachableFunc() func(string, string, string, string) (resolver.ReachabilityStatus, string) { + return func(owner, repo, sha, ref string) (resolver.ReachabilityStatus, string) { + return resolver.Unreachable, "commit is not an ancestor of " + ref + } +} + +// unknownReachFunc returns a checkReachFn that reports unknown (clone failure). +func unknownReachFunc() func(string, string, string, string) (resolver.ReachabilityStatus, string) { + return func(owner, repo, sha, ref string) (resolver.ReachabilityStatus, string) { + return resolver.ReachabilityUnknown, "clone failed" + } } func testRepoResponse(nameWithOwner, oid, actionYAML string) map[string]any { @@ -332,11 +333,22 @@ func writeTempWorkflow(t *testing.T, body string) string { } func runCommandWithHTTP(t *testing.T, rt http.RoundTripper, args ...string) (string, string, error) { + return runCommandWithHTTPAndReach(t, rt, nil, args...) +} + +func runCommandWithHTTPAndReach(t *testing.T, rt http.RoundTripper, reachFn func(string, string, string, string) (resolver.ReachabilityStatus, string), args ...string) (string, string, error) { t.Helper() oldResolver := newResolver newResolver = func(hostname string) (*resolver.Resolver, error) { - return resolver.NewWithTransport(hostname, rt) + r, err := resolver.NewWithTransport(hostname, rt) + if err != nil { + return nil, err + } + if reachFn != nil { + r.SetCheckReachabilityFunc(reachFn) + } + return r, nil } defer func() { newResolver = oldResolver @@ -388,26 +400,23 @@ func runCommandWithHTTP(t *testing.T, rt http.RoundTripper, args ...string) (str // tj-actions/changed-files attack (CVE-2025-30066) where attackers // compromised a maintainer PAT and force-pushed tag v44 to a malicious // commit. The malicious commit is NOT reachable from the legitimate tag. -func TestCheck_TjActionsChangedFiles_TagMutationAttack(t *testing.T) { +// TestCheck_TamperedAndUnreachable verifies that when a pinned SHA differs +// from live resolution AND the old SHA is unreachable, both errors are reported. +func TestCheck_TamperedAndUnreachable(t *testing.T) { reg := &httpmock.Registry{} defer reg.Verify(t) - // The legitimate pinned SHA from before the attack - legitimateSHA := "4edd678ac3f81e2dc578756871e4d00c19191c4e" - // The attacker's SHA that the tag was force-pushed to - maliciousSHA := "0e58ed8671d6b60d0890c21b07f8835ace038e67" + pinnedSHA := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + liveSHA := "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" - // Live resolution returns the MALICIOUS SHA (tag was moved) reg.Register( - httpmock.GraphQL(`repository\(owner: "tj-actions", name: "changed-files"\)`), + httpmock.GraphQL(`repository\(owner: "example", name: "action"\)`), httpmock.JSONResponse(map[string]any{ "data": map[string]any{ - "a0": testRepoResponse("tj-actions/changed-files", maliciousSHA, nodeActionYAML), + "a0": testRepoResponse("example/action", liveSHA, nodeActionYAML), }, }), ) - // The malicious SHA is diverged from the legitimate branch - compareUnreachable(reg, `/repos/tj-actions/changed-files/compare/`) workflowPath := writeTempWorkflow(t, ` name: ci @@ -416,14 +425,14 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: tj-actions/changed-files@v44 + - uses: example/action@v1 # Automatically generated and managed by: gh actions-pin --write dependencies: - - github.com/tj-actions/changed-files@v44:sha1-`+legitimateSHA+` + - github.com/example/action@v1:sha1-`+pinnedSHA+` `) - stdout, _, err := runCommandWithHTTP(t, reg, + stdout, _, err := runCommandWithHTTPAndReach(t, reg, unreachableFunc(), "check", "--json", "valid,errors", workflowPath, ) require.NoError(t, err, "JSON mode communicates errors in payload") @@ -435,114 +444,46 @@ dependencies: require.NoError(t, json.Unmarshal([]byte(stdout), &payload)) assert.False(t, payload.Valid) - // Should detect TAMPERED (SHA changed) AND UNREACHABLE (fork-network) errorTypes := map[string]bool{} for _, e := range payload.Errors { errorTypes[e.Type] = true } - assert.True(t, errorTypes["TAMPERED"], "should detect SHA tamper: %+v", payload.Errors) + assert.True(t, errorTypes["TAMPERED"], "should detect SHA changed: %+v", payload.Errors) assert.True(t, errorTypes["UNREACHABLE"], "should detect unreachable commit: %+v", payload.Errors) } -// TestCheck_ReviewdogActionSetup_ForkNetworkInjection models the reviewdog -// attack where a malicious commit from a fork was referenced via tag -// manipulation. The commit exists in GitHub's shared object store but is -// NOT reachable from the canonical repository's refs. -func TestCheck_ReviewdogActionSetup_ForkNetworkInjection(t *testing.T) { +// TestCheck_UnreachableOnly verifies that when a pinned SHA matches live +// resolution but is not reachable from the ref, an UNREACHABLE error is reported. +func TestCheck_UnreachableOnly(t *testing.T) { reg := &httpmock.Registry{} defer reg.Verify(t) - // Attacker's commit from a fork - exists in the network but not the canonical repo - forkNetworkSHA := "b0c14eb73e15d54af9e97eb7fe20e74fa238fd07" + sha := "cccccccccccccccccccccccccccccccccccccccc" - // Live resolution returns the fork-network SHA (tag was moved to it) reg.Register( - httpmock.GraphQL(`repository\(owner: "reviewdog", name: "action-setup"\)`), + httpmock.GraphQL(`repository\(owner: "example", name: "action"\)`), httpmock.JSONResponse(map[string]any{ "data": map[string]any{ - "a0": testRepoResponse("reviewdog/action-setup", forkNetworkSHA, nodeActionYAML), + "a0": testRepoResponse("example/action", sha, nodeActionYAML), }, }), ) - // Compare returns diverged: SHA exists in fork network but not on ref lineage - compareUnreachable(reg, `/repos/reviewdog/action-setup/compare/`) workflowPath := writeTempWorkflow(t, ` -name: lint -on: pull_request -jobs: - lint: - runs-on: ubuntu-latest - steps: - - uses: reviewdog/action-setup@v1 - -# Automatically generated and managed by: gh actions-pin --write -dependencies: - - github.com/reviewdog/action-setup@v1:sha1-`+forkNetworkSHA+` -`) - - stdout, _, err := runCommandWithHTTP(t, reg, - "check", "--json", "valid,errors", workflowPath, - ) - require.NoError(t, err, "JSON mode communicates errors in payload") - - var payload struct { - Valid bool `json:"valid"` - Errors []validationError `json:"errors"` - } - require.NoError(t, json.Unmarshal([]byte(stdout), &payload)) - assert.False(t, payload.Valid) - - // Even though the SHA matches live resolution (tag still points to malicious commit), - // the reachability check catches the fork-network injection. - hasUnreachable := false - for _, e := range payload.Errors { - if e.Type == "UNREACHABLE" { - hasUnreachable = true - assert.Contains(t, e.Details, "not reachable") - } - } - assert.True(t, hasUnreachable, "should detect fork-network injected commit: %+v", payload.Errors) -} - -// TestCheck_XygeniAction_TagPoisoningWithBackdoor models the xygeni-action -// compromise where a tag was poisoned to inject a C2 reverse shell backdoor. -// The malicious commit is from outside the canonical repo's history. -func TestCheck_XygeniAction_TagPoisoningWithBackdoor(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - - maliciousSHA := "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2" - - reg.Register( - httpmock.GraphQL(`repository\(owner: "xygeni", name: "xygeni-action"\)`), - httpmock.JSONResponse(map[string]any{ - "data": map[string]any{ - "a0": testRepoResponse("xygeni/xygeni-action", maliciousSHA, nodeActionYAML), - }, - }), - ) - // The poisoned SHA doesn't exist in the canonical repo at all (404) - reg.Register( - httpmock.REST("GET", `/repos/xygeni/xygeni-action/compare/`), - httpmock.StatusResponse(404), - ) - - workflowPath := writeTempWorkflow(t, ` -name: security-scan +name: ci on: push jobs: - scan: + test: runs-on: ubuntu-latest steps: - - uses: xygeni/xygeni-action@v3 + - uses: example/action@v1 # Automatically generated and managed by: gh actions-pin --write dependencies: - - github.com/xygeni/xygeni-action@v3:sha1-`+maliciousSHA+` + - github.com/example/action@v1:sha1-`+sha+` `) - stdout, _, err := runCommandWithHTTP(t, reg, + stdout, _, err := runCommandWithHTTPAndReach(t, reg, unreachableFunc(), "check", "--json", "valid,errors", workflowPath, ) require.NoError(t, err, "JSON mode communicates errors in payload") @@ -558,146 +499,27 @@ dependencies: for _, e := range payload.Errors { if e.Type == "UNREACHABLE" { hasUnreachable = true - assert.Contains(t, e.Details, "not found in repository") } } - assert.True(t, hasUnreachable, "should detect SHA not in canonical repo: %+v", payload.Errors) + assert.True(t, hasUnreachable, "should detect unreachable commit: %+v", payload.Errors) } -// TestCheck_TrivyAction_ScannerToStealer models the aquasecurity/trivy-action -// compromise where the tag was manipulated to redirect to a malicious version -// that exfiltrated secrets instead of scanning. -func TestCheck_TrivyAction_ScannerToStealer(t *testing.T) { +// TestCheck_ReachabilityUnknown verifies that when the reachability check +// cannot complete, validation passes with a warning. +func TestCheck_ReachabilityUnknown(t *testing.T) { reg := &httpmock.Registry{} defer reg.Verify(t) - legitimateSHA := "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" - maliciousSHA := "cafebabecafebabecafebabecafebabecafebabe" + sha := "dddddddddddddddddddddddddddddddddddddddd" - // Live resolution returns the malicious SHA (tag was moved) reg.Register( - httpmock.GraphQL(`repository\(owner: "aquasecurity", name: "trivy-action"\)`), + httpmock.GraphQL(`repository\(owner: "example", name: "action"\)`), httpmock.JSONResponse(map[string]any{ "data": map[string]any{ - "a0": testRepoResponse("aquasecurity/trivy-action", maliciousSHA, nodeActionYAML), + "a0": testRepoResponse("example/action", sha, nodeActionYAML), }, }), ) - // The malicious SHA diverges from the legitimate lineage - compareUnreachable(reg, `/repos/aquasecurity/trivy-action/compare/`) - - workflowPath := writeTempWorkflow(t, ` -name: security -on: push -jobs: - scan: - runs-on: ubuntu-latest - steps: - - uses: aquasecurity/trivy-action@master - -# Automatically generated and managed by: gh actions-pin --write -dependencies: - - github.com/aquasecurity/trivy-action@master:sha1-`+legitimateSHA+` -`) - - stdout, _, err := runCommandWithHTTP(t, reg, - "check", "--json", "valid,errors", workflowPath, - ) - require.NoError(t, err, "JSON mode communicates errors in payload") - - var payload struct { - Valid bool `json:"valid"` - Errors []validationError `json:"errors"` - } - require.NoError(t, json.Unmarshal([]byte(stdout), &payload)) - assert.False(t, payload.Valid) - - errorTypes := map[string]bool{} - for _, e := range payload.Errors { - errorTypes[e.Type] = true - } - assert.True(t, errorTypes["TAMPERED"], "should detect SHA changed: %+v", payload.Errors) - assert.True(t, errorTypes["UNREACHABLE"], "should detect fork-network SHA: %+v", payload.Errors) -} - -// TestCheck_CheckmarxKICS_TagForceViaStoredCreds models the March 2026 Checkmarx -// KICS compromise where credentials stolen during the Trivy breach were used to -// force-push malicious code to KICS GitHub Action tags (TeamPCP lateral movement). -func TestCheck_CheckmarxKICS_TagForceViaStoredCreds(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - - legitimateSHA := "1111111111111111111111111111111111111111" - maliciousSHA := "2222222222222222222222222222222222222222" - - // Live resolution returns the malicious SHA (tag force-pushed with stolen creds) - reg.Register( - httpmock.GraphQL(`repository\(owner: "Checkmarx", name: "kics-github-action"\)`), - httpmock.JSONResponse(map[string]any{ - "data": map[string]any{ - "a0": testRepoResponse("Checkmarx/kics-github-action", maliciousSHA, nodeActionYAML), - }, - }), - ) - // The malicious SHA diverges from the legitimate lineage - compareUnreachable(reg, `/repos/Checkmarx/kics-github-action/compare/`) - - workflowPath := writeTempWorkflow(t, ` -name: sast -on: push -jobs: - kics: - runs-on: ubuntu-latest - steps: - - uses: Checkmarx/kics-github-action@v2 - -# Automatically generated and managed by: gh actions-pin --write -dependencies: - - github.com/Checkmarx/kics-github-action@v2:sha1-`+legitimateSHA+` -`) - - stdout, _, err := runCommandWithHTTP(t, reg, - "check", "--json", "valid,errors", workflowPath, - ) - require.NoError(t, err, "JSON mode communicates errors in payload") - - var payload struct { - Valid bool `json:"valid"` - Errors []validationError `json:"errors"` - } - require.NoError(t, json.Unmarshal([]byte(stdout), &payload)) - assert.False(t, payload.Valid) - - errorTypes := map[string]bool{} - for _, e := range payload.Errors { - errorTypes[e.Type] = true - } - assert.True(t, errorTypes["TAMPERED"], "should detect SHA changed: %+v", payload.Errors) - assert.True(t, errorTypes["UNREACHABLE"], "should detect fork-network SHA: %+v", payload.Errors) -} - -// TestCheck_ReachabilityUnknown_DoesNotFailValidation verifies that when the -// compare endpoint returns an error (rate limit, timeout, etc.), the check -// command issues a warning but does NOT mark the validation as failed. -func TestCheck_ReachabilityUnknown_DoesNotFailValidation(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - - sha := "de0fac2e4500dabe0009e67214ff5f5447ce83dd" - - reg.Register( - httpmock.GraphQL(`repository\(owner: "actions", name: "checkout"\)`), - httpmock.JSONResponse(map[string]any{ - "data": map[string]any{ - "a0": testRepoResponse("actions/checkout", sha, nodeActionYAML), - }, - }), - ) - // Simulate a 500 error from the compare endpoint - reg.Register( - httpmock.REST("GET", `/repos/actions/checkout/compare/`), - httpmock.StatusResponse(500), - ) workflowPath := writeTempWorkflow(t, ` name: ci @@ -706,14 +528,14 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: example/action@v1 # Automatically generated and managed by: gh actions-pin --write dependencies: - - github.com/actions/checkout@v6:sha1-`+sha+` + - github.com/example/action@v1:sha1-`+sha+` `) - stdout, _, err := runCommandWithHTTP(t, reg, + stdout, _, err := runCommandWithHTTPAndReach(t, reg, unknownReachFunc(), "check", "--json", "valid,errors,warnings", workflowPath, ) require.NoError(t, err, "unknown reachability should not fail the check") @@ -730,24 +552,22 @@ dependencies: assert.Contains(t, payload.Warnings[0], "reachability check inconclusive") } -// TestCheck_Reachable_CleanValidation verifies the happy path: pinned SHA -// is reachable, live resolution matches, everything is valid. -func TestCheck_Reachable_CleanValidation(t *testing.T) { +// TestCheck_Reachable verifies the happy path: pinned SHA matches live +// resolution and is reachable — validation passes with no errors or warnings. +func TestCheck_Reachable(t *testing.T) { reg := &httpmock.Registry{} defer reg.Verify(t) - sha := "de0fac2e4500dabe0009e67214ff5f5447ce83dd" + sha := "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" reg.Register( - httpmock.GraphQL(`repository\(owner: "actions", name: "checkout"\)`), + httpmock.GraphQL(`repository\(owner: "example", name: "action"\)`), httpmock.JSONResponse(map[string]any{ "data": map[string]any{ - "a0": testRepoResponse("actions/checkout", sha, nodeActionYAML), + "a0": testRepoResponse("example/action", sha, nodeActionYAML), }, }), ) - compareReachable(reg, `/repos/actions/checkout/compare/`) - workflowPath := writeTempWorkflow(t, ` name: ci on: push @@ -755,14 +575,14 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: example/action@v1 # Automatically generated and managed by: gh actions-pin --write dependencies: - - github.com/actions/checkout@v6:sha1-`+sha+` + - github.com/example/action@v1:sha1-`+sha+` `) - stdout, _, err := runCommandWithHTTP(t, reg, + stdout, _, err := runCommandWithHTTPAndReach(t, reg, reachableFunc(), "check", "--json", "valid,errors,warnings", workflowPath, ) require.NoError(t, err) @@ -778,24 +598,22 @@ dependencies: assert.Empty(t, payload.Warnings) } -// TestPin_UnreachableSHA_WarnsButDoesNotBlock verifies that when a freshly -// resolved SHA fails the reachability check during pinning, the CLI warns -// but does not block the pin operation (defense-in-depth, not a hard gate). -func TestPin_UnreachableSHA_WarnsButDoesNotBlock(t *testing.T) { +// TestPin_UnreachableWarnsOnly verifies that an unreachable SHA during pin +// warns on stderr but does not block the operation. +func TestPin_UnreachableWarnsOnly(t *testing.T) { reg := &httpmock.Registry{} defer reg.Verify(t) - sha := "de0fac2e4500dabe0009e67214ff5f5447ce83dd" + sha := "ffffffffffffffffffffffffffffffffffffffff" reg.Register( - httpmock.GraphQL(`repository\(owner: "actions", name: "checkout"\)`), + httpmock.GraphQL(`repository\(owner: "example", name: "action"\)`), httpmock.JSONResponse(map[string]any{ "data": map[string]any{ - "a0": testRepoResponse("actions/checkout", sha, nodeActionYAML), + "a0": testRepoResponse("example/action", sha, nodeActionYAML), }, }), ) - compareUnreachable(reg, `/repos/actions/checkout/compare/`) workflowPath := writeTempWorkflow(t, ` name: ci @@ -804,10 +622,10 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: example/action@v1 `) - _, stderr, err := runCommandWithHTTP(t, reg, "--diff", workflowPath) + _, stderr, err := runCommandWithHTTPAndReach(t, reg, unreachableFunc(), "--diff", workflowPath) require.NoError(t, err, "pin should succeed even with unreachable warning") assert.Contains(t, stderr, "NOT reachable") assert.Contains(t, stderr, "fork-network injection") diff --git a/internal/resolver/reachability_integration_test.go b/internal/resolver/reachability_integration_test.go new file mode 100644 index 00000000..332229fc --- /dev/null +++ b/internal/resolver/reachability_integration_test.go @@ -0,0 +1,124 @@ +//go:build integration + +// Integration tests for reachability checks using git bare clones. +// Requires: git CLI, network access, GH_TOKEN or gh CLI auth. +// Fixtures: +// - nodeselector/actions-test-fixtures: tag v1 on HEAD (ea53476), orphan-poison branch (614a37a) +// - choam-io/actions-test-fixtures-fork: fork with attacker-payload branch (7b403c9) +// +// Run: go test -tags integration -run TestIntegration ./internal/resolver/ + +package resolver + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + fixtureOwner = "nodeselector" + fixtureRepo = "actions-test-fixtures" + + // HEAD of main, also where tag v1 points + headSHA = "ea53476fdc172d8552df5af9658a45a367e4f41d" + // Root commit of main (oldest ancestor) + rootSHA = "5f13f2a16a43112afcd6e1bcc29c418176894d53" + // Orphan commit on orphan-poison branch (no common ancestor with main) + orphanSHA = "614a37a63d1a75476792a8781b55983a9d9bcb80" + // A SHA that doesn't exist anywhere + fakeSHA = "0000000000000000000000000000000000000000" + // Commit on choam-io/actions-test-fixtures-fork attacker-payload branch + // This SHA exists in the fork network but NOT in the upstream bare clone + forkAttackerSHA = "7b403c9ec14bd3ae0bbf793c2bee8815a7ac920a" +) + +func skipWithoutAuth(t *testing.T) { + t.Helper() + if os.Getenv("GH_TOKEN") == "" && os.Getenv("GITHUB_TOKEN") == "" { + if _, err := os.Stat(os.ExpandEnv("$HOME/.config/gh/hosts.yml")); err != nil { + t.Skip("Skipping integration test: no GH_TOKEN or gh auth configured") + } + } +} + +func newLiveResolver(t *testing.T) *Resolver { + t.Helper() + r, err := New("github.com") + require.NoError(t, err) + // Use a temp dir so integration tests don't pollute the real cache + r.CacheDir = t.TempDir() + return r +} + +// TestIntegration_Reachable_HeadSHA verifies that the HEAD commit (where v1 +// points) is reported as reachable via git merge-base. +func TestIntegration_Reachable_HeadSHA(t *testing.T) { + skipWithoutAuth(t) + r := newLiveResolver(t) + + result := r.CheckReachability(fixtureOwner, fixtureRepo, headSHA, "v1") + assert.Equal(t, Reachable, result.Status, "HEAD SHA should be reachable from v1: %+v", result) +} + +// TestIntegration_Reachable_Ancestor verifies that the root commit of main +// is reachable from v1 (it's an ancestor). +func TestIntegration_Reachable_Ancestor(t *testing.T) { + skipWithoutAuth(t) + r := newLiveResolver(t) + + result := r.CheckReachability(fixtureOwner, fixtureRepo, rootSHA, "v1") + assert.Equal(t, Reachable, result.Status, "root commit should be ancestor of v1: %+v", result) +} + +// TestIntegration_Unreachable_OrphanCommit verifies that a commit on an orphan +// branch (no common ancestor with main) is detected as unreachable from v1. +func TestIntegration_Unreachable_OrphanCommit(t *testing.T) { + skipWithoutAuth(t) + r := newLiveResolver(t) + + result := r.CheckReachability(fixtureOwner, fixtureRepo, orphanSHA, "v1") + assert.Equal(t, Unreachable, result.Status, "orphan commit should not be reachable from v1: %+v", result) +} + +// TestIntegration_Unreachable_NonexistentSHA verifies that a completely +// fabricated SHA is detected as unreachable. +func TestIntegration_Unreachable_NonexistentSHA(t *testing.T) { + skipWithoutAuth(t) + r := newLiveResolver(t) + + result := r.CheckReachability(fixtureOwner, fixtureRepo, fakeSHA, "v1") + assert.Equal(t, Unreachable, result.Status, "fake SHA should be unreachable: %+v", result) +} + +// TestIntegration_Unreachable_ForkNetworkInjection is the KEY test proving +// git-based reachability is superior to the GitHub compare API. +// +// The compare API treats the entire fork network as one graph, so a commit +// pushed to choam-io/actions-test-fixtures-fork is visible via the upstream +// compare endpoint and appears "reachable" (behind). This is a false negative. +// +// A bare clone of upstream-only excludes fork objects, so the attacker SHA +// from the fork won't exist in the clone → detected as UNREACHABLE. +func TestIntegration_Unreachable_ForkNetworkInjection(t *testing.T) { + skipWithoutAuth(t) + r := newLiveResolver(t) + + result := r.CheckReachability(fixtureOwner, fixtureRepo, forkAttackerSHA, "v1") + assert.Equal(t, Unreachable, result.Status, + "fork-network SHA should NOT be reachable from upstream bare clone: %+v", result) +} + +// TestIntegration_CacheConsistency verifies that repeated calls return +// the same result and hit the cache on the second call. +func TestIntegration_CacheConsistency(t *testing.T) { + skipWithoutAuth(t) + r := newLiveResolver(t) + + r1 := r.CheckReachability(fixtureOwner, fixtureRepo, headSHA, "v1") + r2 := r.CheckReachability(fixtureOwner, fixtureRepo, headSHA, "v1") + assert.Equal(t, r1.Status, r2.Status) + assert.Equal(t, "cached", r2.Detail, "second call should come from cache") +} diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index 6df85a75..e5966c8a 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -7,7 +7,9 @@ import ( "errors" "fmt" "net/http" - "net/url" + "os" + "os/exec" + "path/filepath" "regexp" "strconv" "strings" @@ -54,12 +56,16 @@ type ReachabilityResult struct { // Resolver resolves action refs to commit SHAs. type Resolver struct { client *api.GraphQLClient - restClient *api.RESTClient hostname string MaxRecursionDepth int cache map[string]resolvedEntry latestRefCache map[string]string reachCache map[string]ReachabilityStatus + // CacheDir is the directory for bare git clones used in reachability checks. + // Defaults to ~/.actions-lockfile/cache. + CacheDir string + // checkReachFn overrides the default git-based reachability check (for tests). + checkReachFn func(owner, repo, sha, ref string) (ReachabilityStatus, string) } // New creates a resolver using the authenticated gh context. @@ -80,19 +86,20 @@ func NewWithOptions(opts api.ClientOptions) (*Resolver, error) { return nil, err } - restClient, err := api.NewRESTClient(opts) + homeDir, err := os.UserHomeDir() if err != nil { - return nil, err + homeDir = os.TempDir() } + cacheDir := filepath.Join(homeDir, ".actions-lockfile", "cache") return &Resolver{ client: client, - restClient: restClient, hostname: hostname, MaxRecursionDepth: DefaultMaxRecursionDepth, cache: make(map[string]resolvedEntry), latestRefCache: make(map[string]string), reachCache: make(map[string]ReachabilityStatus), + CacheDir: cacheDir, }, nil } @@ -112,16 +119,21 @@ func (r *Resolver) Hostname() string { return r.hostname } +// SetCheckReachabilityFunc overrides the default git-based reachability check. +// Intended for tests. +func (r *Resolver) SetCheckReachabilityFunc(fn func(owner, repo, sha, ref string) (ReachabilityStatus, string)) { + r.checkReachFn = fn +} + // CheckReachability verifies that a resolved SHA is on the lineage of the // given ref within the repository. This catches fork-network injection where // a SHA exists in GitHub's shared object store but is not actually part of // the canonical repository's history. // -// Uses the compare endpoint: GET /repos/{owner}/{repo}/compare/{sha}...{ref} -// - "identical" or "behind" or "ahead" → same lineage → Reachable -// - "diverged" → different lineage → Unreachable -// - 404 → SHA not in repo → Unreachable -// - other errors → Unknown +// Uses a bare blobless clone of the upstream repo and git merge-base: +// - exit 0 from merge-base --is-ancestor → Reachable +// - exit 1 → Unreachable (SHA not an ancestor of ref) +// - clone/fetch failure → Unknown func (r *Resolver) CheckReachability(owner, repo, sha, ref string) ReachabilityResult { result := ReachabilityResult{ Owner: owner, @@ -137,46 +149,75 @@ func (r *Resolver) CheckReachability(owner, repo, sha, ref string) ReachabilityR return result } - escapedRef := url.PathEscape(ref) - path := fmt.Sprintf("repos/%s/%s/compare/%s...%s", owner, repo, sha, escapedRef) - - var compare struct { - Status string `json:"status"` - } - err := r.restClient.Get(path, &compare) - if err != nil { - // 404 means the SHA doesn't exist in this repository at all - if strings.Contains(err.Error(), "404") || strings.Contains(err.Error(), "Not Found") { - result.Status = Unreachable - result.Detail = "commit not found in repository" - r.reachCache[cacheKey] = Unreachable - return result + // Allow tests to inject a fake implementation + if r.checkReachFn != nil { + result.Status, result.Detail = r.checkReachFn(owner, repo, sha, ref) + if result.Status != ReachabilityUnknown { + r.reachCache[cacheKey] = result.Status } - result.Status = ReachabilityUnknown - result.Detail = err.Error() return result } - switch compare.Status { - case "identical", "behind", "ahead": - // All mean the SHA is on the same lineage as the ref - result.Status = Reachable - result.Detail = compare.Status - case "diverged": - // SHA exists in the network but is NOT on the ref's lineage - result.Status = Unreachable - result.Detail = "commit exists in fork network but is not on ref lineage" - default: - result.Status = ReachabilityUnknown - result.Detail = fmt.Sprintf("unexpected compare status: %q", compare.Status) - } - + status, detail := r.gitReachabilityCheck(owner, repo, sha, ref) + result.Status = status + result.Detail = detail if result.Status != ReachabilityUnknown { r.reachCache[cacheKey] = result.Status } return result } +// ensureBareClone clones or fetches a bare blobless repo into the cache dir. +func (r *Resolver) ensureBareClone(owner, repo string) (string, error) { + repoDir := filepath.Join(r.CacheDir, owner, repo+".git") + cloneURL := fmt.Sprintf("https://%s/%s/%s.git", r.hostname, owner, repo) + + if _, err := os.Stat(filepath.Join(repoDir, "HEAD")); err == nil { + // Already cloned — fetch latest refs + cmd := exec.Command("git", "-C", repoDir, "fetch", "--quiet", "--tags", "--force") + if out, err := cmd.CombinedOutput(); err != nil { + return "", fmt.Errorf("git fetch failed: %s: %w", strings.TrimSpace(string(out)), err) + } + return repoDir, nil + } + + if err := os.MkdirAll(filepath.Dir(repoDir), 0o755); err != nil { + return "", err + } + cmd := exec.Command("git", "clone", "--filter=blob:none", "--bare", "--quiet", cloneURL, repoDir) + if out, err := cmd.CombinedOutput(); err != nil { + return "", fmt.Errorf("git clone failed: %s: %w", strings.TrimSpace(string(out)), err) + } + return repoDir, nil +} + +// gitReachabilityCheck uses a bare clone and merge-base --is-ancestor to verify +// that sha is an ancestor of ref. +func (r *Resolver) gitReachabilityCheck(owner, repo, sha, ref string) (ReachabilityStatus, string) { + repoDir, err := r.ensureBareClone(owner, repo) + if err != nil { + return ReachabilityUnknown, err.Error() + } + + cmd := exec.Command("git", "-C", repoDir, "merge-base", "--is-ancestor", sha, ref) + out, err := cmd.CombinedOutput() + if err == nil { + return Reachable, "ancestor of " + ref + } + + // exit 1 = not ancestor, exit 128 = SHA unknown + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + switch exitErr.ExitCode() { + case 1: + return Unreachable, "commit is not an ancestor of " + ref + case 128: + return Unreachable, "commit not found in repository: " + strings.TrimSpace(string(out)) + } + } + return ReachabilityUnknown, err.Error() +} + // CheckReachabilityAll runs reachability checks on a batch of dependencies, // deduplicating by owner/repo/sha/ref. func (r *Resolver) CheckReachabilityAll(deps []lockfile.Dependency) []ReachabilityResult { diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 1c5af663..747ab0bf 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -289,144 +289,52 @@ func TestResolveAllRecursiveWithHTTPTransport(t *testing.T) { } func TestCheckReachability_Reachable(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - - reg.Register( - httpmock.REST("GET", `/repos/actions/checkout/compare/`), - httpmock.JSONResponse(map[string]any{"status": "identical"}), - ) - - r, err := NewWithTransport("github.com", reg) - if err != nil { - t.Fatalf("NewWithTransport returned error: %v", err) + r := &Resolver{ + reachCache: map[string]ReachabilityStatus{}, + checkReachFn: func(owner, repo, sha, ref string) (ReachabilityStatus, string) { + return Reachable, "ancestor of " + ref + }, } - result := r.CheckReachability("actions", "checkout", "abc123", "v6") if result.Status != Reachable { t.Fatalf("expected Reachable, got %s (%s)", result.Status, result.Detail) } } -func TestCheckReachability_Ahead(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - - reg.Register( - httpmock.REST("GET", `/repos/actions/checkout/compare/`), - httpmock.JSONResponse(map[string]any{"status": "ahead"}), - ) - - r, err := NewWithTransport("github.com", reg) - if err != nil { - t.Fatalf("NewWithTransport returned error: %v", err) - } - - result := r.CheckReachability("actions", "checkout", "abc123", "v6") - if result.Status != Reachable { - t.Fatalf("expected Reachable for ahead status, got %s (%s)", result.Status, result.Detail) - } -} - -func TestCheckReachability_Behind(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - - reg.Register( - httpmock.REST("GET", `/repos/actions/checkout/compare/`), - httpmock.JSONResponse(map[string]any{"status": "behind"}), - ) - - r, err := NewWithTransport("github.com", reg) - if err != nil { - t.Fatalf("NewWithTransport returned error: %v", err) - } - - result := r.CheckReachability("actions", "checkout", "abc123", "v6") - if result.Status != Reachable { - t.Fatalf("expected Reachable for behind status (tag rolled back), got %s (%s)", result.Status, result.Detail) - } -} - -func TestCheckReachability_Diverged_Unreachable(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - - reg.Register( - httpmock.REST("GET", `/repos/evil/repo/compare/`), - httpmock.JSONResponse(map[string]any{"status": "diverged"}), - ) - - r, err := NewWithTransport("github.com", reg) - if err != nil { - t.Fatalf("NewWithTransport returned error: %v", err) - } - - result := r.CheckReachability("evil", "repo", "deadbeef", "v1") - if result.Status != Unreachable { - t.Fatalf("expected Unreachable for diverged status, got %s (%s)", result.Status, result.Detail) - } - if !strings.Contains(result.Detail, "fork network") { - t.Fatalf("expected fork network detail, got %q", result.Detail) - } -} - -func TestCheckReachability_404_Unreachable(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - - reg.Register( - httpmock.REST("GET", `/repos/evil/repo/compare/`), - httpmock.StatusResponse(404), - ) - - r, err := NewWithTransport("github.com", reg) - if err != nil { - t.Fatalf("NewWithTransport returned error: %v", err) +func TestCheckReachability_Unreachable(t *testing.T) { + r := &Resolver{ + reachCache: map[string]ReachabilityStatus{}, + checkReachFn: func(owner, repo, sha, ref string) (ReachabilityStatus, string) { + return Unreachable, "commit is not an ancestor of " + ref + }, } - result := r.CheckReachability("evil", "repo", "deadbeef", "v1") if result.Status != Unreachable { - t.Fatalf("expected Unreachable for 404, got %s (%s)", result.Status, result.Detail) - } - if !strings.Contains(result.Detail, "not found") { - t.Fatalf("expected 'not found' detail, got %q", result.Detail) + t.Fatalf("expected Unreachable, got %s (%s)", result.Status, result.Detail) } } -func TestCheckReachability_ServerError_Unknown(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - - reg.Register( - httpmock.REST("GET", `/repos/actions/checkout/compare/`), - httpmock.StatusResponse(500), - ) - - r, err := NewWithTransport("github.com", reg) - if err != nil { - t.Fatalf("NewWithTransport returned error: %v", err) +func TestCheckReachability_Unknown(t *testing.T) { + r := &Resolver{ + reachCache: map[string]ReachabilityStatus{}, + checkReachFn: func(owner, repo, sha, ref string) (ReachabilityStatus, string) { + return ReachabilityUnknown, "clone failed" + }, } - result := r.CheckReachability("actions", "checkout", "abc123", "v6") if result.Status != ReachabilityUnknown { - t.Fatalf("expected Unknown for 500, got %s (%s)", result.Status, result.Detail) + t.Fatalf("expected Unknown, got %s (%s)", result.Status, result.Detail) } } func TestCheckReachability_CachesResults(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - - // Only one stub — second call must hit cache - reg.Register( - httpmock.REST("GET", `/repos/actions/checkout/compare/`), - httpmock.JSONResponse(map[string]any{"status": "identical"}), - ) - - r, err := NewWithTransport("github.com", reg) - if err != nil { - t.Fatalf("NewWithTransport returned error: %v", err) + calls := 0 + r := &Resolver{ + reachCache: map[string]ReachabilityStatus{}, + checkReachFn: func(owner, repo, sha, ref string) (ReachabilityStatus, string) { + calls++ + return Reachable, "ancestor of " + ref + }, } r1 := r.CheckReachability("actions", "checkout", "abc123", "v6") @@ -438,25 +346,19 @@ func TestCheckReachability_CachesResults(t *testing.T) { if r2.Detail != "cached" { t.Fatalf("expected second call to be cached, got detail %q", r2.Detail) } + if calls != 1 { + t.Fatalf("expected checkReachFn called once, got %d", calls) + } } func TestCheckReachabilityAll_DeduplicatesRequests(t *testing.T) { - reg := &httpmock.Registry{} - defer reg.Verify(t) - - // Only one stub for checkout — dedup should prevent double-call - reg.Register( - httpmock.REST("GET", `/repos/actions/checkout/compare/`), - httpmock.JSONResponse(map[string]any{"status": "identical"}), - ) - reg.Register( - httpmock.REST("GET", `/repos/actions/setup-go/compare/`), - httpmock.JSONResponse(map[string]any{"status": "ahead"}), - ) - - r, err := NewWithTransport("github.com", reg) - if err != nil { - t.Fatalf("NewWithTransport returned error: %v", err) + calls := 0 + r := &Resolver{ + reachCache: map[string]ReachabilityStatus{}, + checkReachFn: func(owner, repo, sha, ref string) (ReachabilityStatus, string) { + calls++ + return Reachable, "ancestor of " + ref + }, } deps := []lockfile.Dependency{ @@ -469,4 +371,7 @@ func TestCheckReachabilityAll_DeduplicatesRequests(t *testing.T) { if len(results) != 2 { t.Fatalf("expected 2 unique results, got %d: %+v", len(results), results) } + if calls != 2 { + t.Fatalf("expected 2 calls (deduped), got %d", calls) + } } From 926bc7c21031c4a03ce3a16e116c6aadfb02c709 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 23 Apr 2026 16:42:07 -0500 Subject: [PATCH 3/4] fix: pinning makes this simple --- .../resolver/reachability_integration_test.go | 51 +++++--- internal/resolver/resolver.go | 118 +++++++++--------- internal/resolver/resolver_test.go | 106 ++++++++++++++++ 3 files changed, 202 insertions(+), 73 deletions(-) diff --git a/internal/resolver/reachability_integration_test.go b/internal/resolver/reachability_integration_test.go index 332229fc..cabb6e07 100644 --- a/internal/resolver/reachability_integration_test.go +++ b/internal/resolver/reachability_integration_test.go @@ -1,7 +1,7 @@ //go:build integration -// Integration tests for reachability checks using git bare clones. -// Requires: git CLI, network access, GH_TOKEN or gh CLI auth. +// Integration tests for reachability checks using the GitHub Compare API. +// Requires: network access, GH_TOKEN or gh CLI auth. // Fixtures: // - nodeselector/actions-test-fixtures: tag v1 on HEAD (ea53476), orphan-poison branch (614a37a) // - choam-io/actions-test-fixtures-fork: fork with attacker-payload branch (7b403c9) @@ -24,6 +24,8 @@ const ( // HEAD of main, also where tag v1 points headSHA = "ea53476fdc172d8552df5af9658a45a367e4f41d" + // Parent of HEAD — in v1's lineage but not at HEAD (tag-drift scenario) + parentSHA = "38b3412adcb7afb4a061c519513e45cbaf4a1cec" // Root commit of main (oldest ancestor) rootSHA = "5f13f2a16a43112afcd6e1bcc29c418176894d53" // Orphan commit on orphan-poison branch (no common ancestor with main) @@ -31,7 +33,7 @@ const ( // A SHA that doesn't exist anywhere fakeSHA = "0000000000000000000000000000000000000000" // Commit on choam-io/actions-test-fixtures-fork attacker-payload branch - // This SHA exists in the fork network but NOT in the upstream bare clone + // This SHA exists in the fork network but NOT in the upstream repo's lineage forkAttackerSHA = "7b403c9ec14bd3ae0bbf793c2bee8815a7ac920a" ) @@ -48,13 +50,11 @@ func newLiveResolver(t *testing.T) *Resolver { t.Helper() r, err := New("github.com") require.NoError(t, err) - // Use a temp dir so integration tests don't pollute the real cache - r.CacheDir = t.TempDir() return r } // TestIntegration_Reachable_HeadSHA verifies that the HEAD commit (where v1 -// points) is reported as reachable via git merge-base. +// points) is reported as reachable via the Compare API merge-base identity check. func TestIntegration_Reachable_HeadSHA(t *testing.T) { skipWithoutAuth(t) r := newLiveResolver(t) @@ -73,6 +73,18 @@ func TestIntegration_Reachable_Ancestor(t *testing.T) { assert.Equal(t, Reachable, result.Status, "root commit should be ancestor of v1: %+v", result) } +// TestIntegration_Reachable_NotAtHead_ButInLineage simulates tag drift: the +// pinned SHA was once at the tag's HEAD but the tag has since moved forward. +// The pinned SHA (parent of current HEAD) should still be reachable. +func TestIntegration_Reachable_NotAtHead_ButInLineage(t *testing.T) { + skipWithoutAuth(t) + r := newLiveResolver(t) + + result := r.CheckReachability(fixtureOwner, fixtureRepo, parentSHA, "v1") + assert.Equal(t, Reachable, result.Status, + "commit behind HEAD should still be reachable from v1 (tag drift): %+v", result) +} + // TestIntegration_Unreachable_OrphanCommit verifies that a commit on an orphan // branch (no common ancestor with main) is detected as unreachable from v1. func TestIntegration_Unreachable_OrphanCommit(t *testing.T) { @@ -94,21 +106,32 @@ func TestIntegration_Unreachable_NonexistentSHA(t *testing.T) { } // TestIntegration_Unreachable_ForkNetworkInjection is the KEY test proving -// git-based reachability is superior to the GitHub compare API. -// -// The compare API treats the entire fork network as one graph, so a commit -// pushed to choam-io/actions-test-fixtures-fork is visible via the upstream -// compare endpoint and appears "reachable" (behind). This is a false negative. +// the Compare API merge-base identity check detects fork-network injection. // -// A bare clone of upstream-only excludes fork objects, so the attacker SHA -// from the fork won't exist in the clone → detected as UNREACHABLE. +// The Compare API operates on the fork-network-shared object store, so the +// fork commit IS visible. However, the merge_base_commit for a fork commit +// will NOT be the fork SHA itself — it will be the actual common ancestor in +// the upstream history. This mismatch (merge_base != pinnedSHA) is the signal +// that detects the imposter commit. func TestIntegration_Unreachable_ForkNetworkInjection(t *testing.T) { skipWithoutAuth(t) r := newLiveResolver(t) result := r.CheckReachability(fixtureOwner, fixtureRepo, forkAttackerSHA, "v1") assert.Equal(t, Unreachable, result.Status, - "fork-network SHA should NOT be reachable from upstream bare clone: %+v", result) + "fork-network SHA should NOT be reachable via merge-base identity check: %+v", result) +} + +// TestIntegration_SHAAsRef_ReturnsUnknown verifies that when the ref is itself +// a raw SHA (the anti-pattern), we return Unknown with guidance to pin to a tag. +func TestIntegration_SHAAsRef_ReturnsUnknown(t *testing.T) { + skipWithoutAuth(t) + r := newLiveResolver(t) + + result := r.CheckReachability(fixtureOwner, fixtureRepo, headSHA, headSHA) + assert.Equal(t, ReachabilityUnknown, result.Status, + "SHA-as-ref should return Unknown: %+v", result) + assert.Contains(t, result.Detail, "pin to a tag") } // TestIntegration_CacheConsistency verifies that repeated calls return diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index e5966c8a..a75fda59 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -7,9 +7,6 @@ import ( "errors" "fmt" "net/http" - "os" - "os/exec" - "path/filepath" "regexp" "strconv" "strings" @@ -56,15 +53,13 @@ type ReachabilityResult struct { // Resolver resolves action refs to commit SHAs. type Resolver struct { client *api.GraphQLClient + restClient *api.RESTClient hostname string MaxRecursionDepth int cache map[string]resolvedEntry latestRefCache map[string]string reachCache map[string]ReachabilityStatus - // CacheDir is the directory for bare git clones used in reachability checks. - // Defaults to ~/.actions-lockfile/cache. - CacheDir string - // checkReachFn overrides the default git-based reachability check (for tests). + // checkReachFn overrides the default REST-based reachability check (for tests). checkReachFn func(owner, repo, sha, ref string) (ReachabilityStatus, string) } @@ -86,20 +81,19 @@ func NewWithOptions(opts api.ClientOptions) (*Resolver, error) { return nil, err } - homeDir, err := os.UserHomeDir() + restClient, err := api.NewRESTClient(opts) if err != nil { - homeDir = os.TempDir() + return nil, err } - cacheDir := filepath.Join(homeDir, ".actions-lockfile", "cache") return &Resolver{ client: client, + restClient: restClient, hostname: hostname, MaxRecursionDepth: DefaultMaxRecursionDepth, cache: make(map[string]resolvedEntry), latestRefCache: make(map[string]string), reachCache: make(map[string]ReachabilityStatus), - CacheDir: cacheDir, }, nil } @@ -119,21 +113,29 @@ func (r *Resolver) Hostname() string { return r.hostname } -// SetCheckReachabilityFunc overrides the default git-based reachability check. +// SetCheckReachabilityFunc overrides the default REST-based reachability check. // Intended for tests. func (r *Resolver) SetCheckReachabilityFunc(fn func(owner, repo, sha, ref string) (ReachabilityStatus, string)) { r.checkReachFn = fn } +// isSHARef returns true if the ref looks like a full commit SHA (40 hex chars). +var shaRefRE = regexp.MustCompile(`^[0-9a-fA-F]{40}$`) + // CheckReachability verifies that a resolved SHA is on the lineage of the // given ref within the repository. This catches fork-network injection where // a SHA exists in GitHub's shared object store but is not actually part of // the canonical repository's history. // -// Uses a bare blobless clone of the upstream repo and git merge-base: -// - exit 0 from merge-base --is-ancestor → Reachable -// - exit 1 → Unreachable (SHA not an ancestor of ref) -// - clone/fetch failure → Unknown +// Uses the GitHub Compare API and checks merge_base identity: +// - merge_base == pinnedSHA → Reachable (SHA is a true ancestor of ref) +// - merge_base != pinnedSHA → Unreachable (fork/imposter commit) +// - 404 (no common ancestor or not found) → Unreachable +// - 403/429 (rate limit) or other error → Unknown +// +// When ref is itself a raw SHA (the "uses: owner/repo@SHA" anti-pattern), +// the compare becomes {sha}...{sha} which trivially returns "identical" and +// cannot detect fork commits. In this case, a warning is returned instead. func (r *Resolver) CheckReachability(owner, repo, sha, ref string) ReachabilityResult { result := ReachabilityResult{ Owner: owner, @@ -158,7 +160,15 @@ func (r *Resolver) CheckReachability(owner, repo, sha, ref string) ReachabilityR return result } - status, detail := r.gitReachabilityCheck(owner, repo, sha, ref) + // SHA-as-ref anti-pattern: compare/{sha}...{sha} is trivially identical + // and cannot detect fork commits. Warn the user. + if shaRefRE.MatchString(ref) { + result.Status = ReachabilityUnknown + result.Detail = "ref is a raw SHA — reachability cannot be verified; pin to a tag instead" + return result + } + + status, detail := r.apiReachabilityCheck(owner, repo, sha, ref) result.Status = status result.Detail = detail if result.Status != ReachabilityUnknown { @@ -167,55 +177,45 @@ func (r *Resolver) CheckReachability(owner, repo, sha, ref string) ReachabilityR return result } -// ensureBareClone clones or fetches a bare blobless repo into the cache dir. -func (r *Resolver) ensureBareClone(owner, repo string) (string, error) { - repoDir := filepath.Join(r.CacheDir, owner, repo+".git") - cloneURL := fmt.Sprintf("https://%s/%s/%s.git", r.hostname, owner, repo) - - if _, err := os.Stat(filepath.Join(repoDir, "HEAD")); err == nil { - // Already cloned — fetch latest refs - cmd := exec.Command("git", "-C", repoDir, "fetch", "--quiet", "--tags", "--force") - if out, err := cmd.CombinedOutput(); err != nil { - return "", fmt.Errorf("git fetch failed: %s: %w", strings.TrimSpace(string(out)), err) - } - return repoDir, nil - } - - if err := os.MkdirAll(filepath.Dir(repoDir), 0o755); err != nil { - return "", err - } - cmd := exec.Command("git", "clone", "--filter=blob:none", "--bare", "--quiet", cloneURL, repoDir) - if out, err := cmd.CombinedOutput(); err != nil { - return "", fmt.Errorf("git clone failed: %s: %w", strings.TrimSpace(string(out)), err) - } - return repoDir, nil +// compareResponse is the subset of the GitHub Compare API response we need. +type compareResponse struct { + MergeBaseCommit struct { + SHA string `json:"sha"` + } `json:"merge_base_commit"` + Status string `json:"status"` } -// gitReachabilityCheck uses a bare clone and merge-base --is-ancestor to verify -// that sha is an ancestor of ref. -func (r *Resolver) gitReachabilityCheck(owner, repo, sha, ref string) (ReachabilityStatus, string) { - repoDir, err := r.ensureBareClone(owner, repo) +// apiReachabilityCheck uses the GitHub Compare API to verify that sha is an +// ancestor of ref. The key insight: merge_base(ancestor, descendant) == ancestor. +// If the merge_base is NOT the pinned SHA, the commit lives on the fork network. +func (r *Resolver) apiReachabilityCheck(owner, repo, sha, ref string) (ReachabilityStatus, string) { + path := fmt.Sprintf("repos/%s/%s/compare/%s...%s", owner, repo, sha, ref) + + var resp compareResponse + err := r.restClient.Get(path, &resp) if err != nil { + var httpErr *api.HTTPError + if errors.As(err, &httpErr) { + switch { + case httpErr.StatusCode == http.StatusNotFound: + return Unreachable, "no common ancestor or commit not found" + case httpErr.StatusCode == http.StatusForbidden || httpErr.StatusCode == http.StatusTooManyRequests: + detail := fmt.Sprintf("rate limited (HTTP %d)", httpErr.StatusCode) + if reset := httpErr.Headers.Get("X-RateLimit-Reset"); reset != "" { + detail += "; resets at " + reset + } + return ReachabilityUnknown, detail + default: + return ReachabilityUnknown, fmt.Sprintf("API error (HTTP %d): %s", httpErr.StatusCode, httpErr.Message) + } + } return ReachabilityUnknown, err.Error() } - cmd := exec.Command("git", "-C", repoDir, "merge-base", "--is-ancestor", sha, ref) - out, err := cmd.CombinedOutput() - if err == nil { - return Reachable, "ancestor of " + ref - } - - // exit 1 = not ancestor, exit 128 = SHA unknown - var exitErr *exec.ExitError - if errors.As(err, &exitErr) { - switch exitErr.ExitCode() { - case 1: - return Unreachable, "commit is not an ancestor of " + ref - case 128: - return Unreachable, "commit not found in repository: " + strings.TrimSpace(string(out)) - } + if resp.MergeBaseCommit.SHA == sha { + return Reachable, "ancestor of " + ref + " (compare: " + resp.Status + ")" } - return ReachabilityUnknown, err.Error() + return Unreachable, fmt.Sprintf("merge base is %s, not the pinned SHA — likely a fork-network commit", resp.MergeBaseCommit.SHA[:12]) } // CheckReachabilityAll runs reachability checks on a batch of dependencies, diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 747ab0bf..8c70f4fe 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -375,3 +375,109 @@ func TestCheckReachabilityAll_DeduplicatesRequests(t *testing.T) { t.Fatalf("expected 2 calls (deduped), got %d", calls) } } + +func TestCheckReachability_SHAAsRef_ReturnsUnknown(t *testing.T) { + r := &Resolver{ + reachCache: map[string]ReachabilityStatus{}, + } + sha := "abc123abc123abc123abc123abc123abc123abc1" + result := r.CheckReachability("actions", "checkout", sha, sha) + if result.Status != ReachabilityUnknown { + t.Fatalf("expected Unknown for SHA-as-ref, got %s (%s)", result.Status, result.Detail) + } + if !strings.Contains(result.Detail, "pin to a tag") { + t.Fatalf("expected detail to mention tag pinning, got %q", result.Detail) + } +} + +func TestApiReachabilityCheck_Reachable(t *testing.T) { + reg := &httpmock.Registry{} + reg.Register( + httpmock.REST("GET", "repos/actions/checkout/compare/"), + httpmock.JSONResponse(map[string]any{ + "status": "ahead", + "merge_base_commit": map[string]any{ + "sha": "abc123abc123abc123abc123abc123abc123abc1", + }, + }), + ) + + r, err := NewWithTransport("github.com", reg) + if err != nil { + t.Fatal(err) + } + + result := r.CheckReachability("actions", "checkout", "abc123abc123abc123abc123abc123abc123abc1", "v6") + if result.Status != Reachable { + t.Fatalf("expected Reachable, got %s (%s)", result.Status, result.Detail) + } + reg.Verify(t) +} + +func TestApiReachabilityCheck_Unreachable_ForkCommit(t *testing.T) { + reg := &httpmock.Registry{} + reg.Register( + httpmock.REST("GET", "repos/actions/checkout/compare/"), + httpmock.JSONResponse(map[string]any{ + "status": "behind", + "merge_base_commit": map[string]any{ + "sha": "different_sha_000000000000000000000000000", + }, + }), + ) + + r, err := NewWithTransport("github.com", reg) + if err != nil { + t.Fatal(err) + } + + result := r.CheckReachability("actions", "checkout", "abc123abc123abc123abc123abc123abc123abc1", "v6") + if result.Status != Unreachable { + t.Fatalf("expected Unreachable, got %s (%s)", result.Status, result.Detail) + } + if !strings.Contains(result.Detail, "fork-network") { + t.Fatalf("expected detail to mention fork-network, got %q", result.Detail) + } + reg.Verify(t) +} + +func TestApiReachabilityCheck_Unreachable_404(t *testing.T) { + reg := &httpmock.Registry{} + reg.Register( + httpmock.REST("GET", "repos/actions/checkout/compare/"), + httpmock.StatusResponse(404), + ) + + r, err := NewWithTransport("github.com", reg) + if err != nil { + t.Fatal(err) + } + + result := r.CheckReachability("actions", "checkout", "abc123abc123abc123abc123abc123abc123abc1", "v6") + if result.Status != Unreachable { + t.Fatalf("expected Unreachable for 404, got %s (%s)", result.Status, result.Detail) + } + reg.Verify(t) +} + +func TestApiReachabilityCheck_Unknown_RateLimit(t *testing.T) { + reg := &httpmock.Registry{} + reg.Register( + httpmock.REST("GET", "repos/actions/checkout/compare/"), + httpmock.StatusResponse(429), + ) + + r, err := NewWithTransport("github.com", reg) + if err != nil { + t.Fatal(err) + } + + result := r.CheckReachability("actions", "checkout", "abc123abc123abc123abc123abc123abc123abc1", "v6") + if result.Status != ReachabilityUnknown { + t.Fatalf("expected Unknown for rate limit, got %s (%s)", result.Status, result.Detail) + } + if !strings.Contains(result.Detail, "rate limited") { + t.Fatalf("expected detail to mention rate limit, got %q", result.Detail) + } + reg.Verify(t) +} From 2bba3ebea69a44eaec74c979b33088b30a281d7c Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 23 Apr 2026 17:50:41 -0500 Subject: [PATCH 4/4] fix: address PR review feedback (reachability-specific) - URL-escape ref and SHA in Compare API path to handle refs with slashes (e.g. feature/foo) that would break the URL path segments - Add DepKey field to ReachabilityResult and thread dep.Key() through CheckReachabilityAll so error messages include the full dependency key including subpath (e.g. actions/cache/save@v4 not actions/cache@v4) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/resolver/resolver.go | 6 +++++- root.go | 20 ++++++++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go index a75fda59..a9870c6a 100644 --- a/internal/resolver/resolver.go +++ b/internal/resolver/resolver.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "regexp" "strconv" "strings" @@ -46,6 +47,7 @@ type ReachabilityResult struct { Repo string Ref string SHA string + DepKey string // full dependency key (e.g. "actions/cache/save@v4") Status ReachabilityStatus Detail string // human-readable detail (e.g. compare status or error) } @@ -189,7 +191,8 @@ type compareResponse struct { // ancestor of ref. The key insight: merge_base(ancestor, descendant) == ancestor. // If the merge_base is NOT the pinned SHA, the commit lives on the fork network. func (r *Resolver) apiReachabilityCheck(owner, repo, sha, ref string) (ReachabilityStatus, string) { - path := fmt.Sprintf("repos/%s/%s/compare/%s...%s", owner, repo, sha, ref) + path := fmt.Sprintf("repos/%s/%s/compare/%s...%s", + owner, repo, url.PathEscape(sha), url.PathEscape(ref)) var resp compareResponse err := r.restClient.Get(path, &resp) @@ -238,6 +241,7 @@ func (r *Resolver) CheckReachabilityAll(deps []lockfile.Dependency) []Reachabili seen[key] = true result := r.CheckReachability(owner, repo, dep.SHA, dep.Ref) + result.DepKey = dep.Key() results = append(results, result) } diff --git a/root.go b/root.go index 182aba42..cc990490 100644 --- a/root.go +++ b/root.go @@ -531,14 +531,18 @@ func pinOneFile(opts *pinOptions, workflowPath string, r *resolver.Resolver) err // Reachability check on freshly resolved deps (warns, does not block) reachResults := r.CheckReachabilityAll(deps) for _, rr := range reachResults { + depID := rr.DepKey + if depID == "" { + depID = fmt.Sprintf("%s/%s@%s", rr.Owner, rr.Repo, rr.Ref) + } switch rr.Status { case resolver.Unreachable: - fmt.Fprintf(os.Stderr, "warning: %s/%s@%s: SHA %s is NOT reachable from ref (%s)\n", - rr.Owner, rr.Repo, rr.Ref, rr.SHA[:12], rr.Detail) + fmt.Fprintf(os.Stderr, "warning: %s: SHA %s is NOT reachable from ref (%s)\n", + depID, rr.SHA[:12], rr.Detail) fmt.Fprintf(os.Stderr, " This may indicate a fork-network injection attack.\n") case resolver.ReachabilityUnknown: - fmt.Fprintf(os.Stderr, "warning: %s/%s@%s: reachability check inconclusive (%s)\n", - rr.Owner, rr.Repo, rr.Ref, rr.Detail) + fmt.Fprintf(os.Stderr, "warning: %s: reachability check inconclusive (%s)\n", + depID, rr.Detail) } } @@ -805,17 +809,21 @@ func validateOneFile(workflowPath string, r *resolver.Resolver) (*validationResu fmt.Fprintf(os.Stderr, "Checking commit reachability for %d dependency(ies)...\n", len(existingDeps)) reachResults := r.CheckReachabilityAll(existingDeps) for _, rr := range reachResults { + depID := rr.DepKey + if depID == "" { + depID = fmt.Sprintf("%s/%s@%s", rr.Owner, rr.Repo, rr.Ref) + } switch rr.Status { case resolver.Unreachable: result.Valid = false result.Errors = append(result.Errors, validationError{ Type: "UNREACHABLE", - Dependency: fmt.Sprintf("%s/%s@%s", rr.Owner, rr.Repo, rr.Ref), + Dependency: depID, Details: fmt.Sprintf("SHA %s is not reachable from ref %s (%s)", rr.SHA[:12], rr.Ref, rr.Detail), }) case resolver.ReachabilityUnknown: result.Warnings = append(result.Warnings, - fmt.Sprintf("%s/%s@%s: reachability check inconclusive (%s)", rr.Owner, rr.Repo, rr.Ref, rr.Detail)) + fmt.Sprintf("%s: reachability check inconclusive (%s)", depID, rr.Detail)) } }