Pipeline: retain symbolic roots for SHA uses

Author: nodeselector

cmd/gh-actions-lock/command_test.go

@@ -610,6 +610,64 @@ jobs:
 	assert.Equal(t, []string{"example/action@main"}, transitiveDep.RequiredBy)
 }
 
+func TestCheckCommand_BareSHAKeepsSymbolicLockRootDirect(t *testing.T) {
+	reg := &httpmock.Registry{}
+	defer reg.Verify(t)
+
+	sha := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+	reg.Register(
+		httpmock.GraphQLForRepo("example", "action"),
+		httpmock.JSONResponse(map[string]any{
+			"data": map[string]any{
+				"a0": testRepoResponse("example/action", sha, nodeActionYAML),
+				"a1": testRepoResponse("example/action", sha, nodeActionYAML),
+			},
+		}),
+	)
+	reg.Register(
+		httpmock.REST("GET", "repos/example/action/branches"),
+		httpmock.JSONResponse([]any{}),
+	)
+	reg.Register(
+		httpmock.REST("GET", "repos/example/action/branches"),
+		httpmock.JSONResponse([]any{}),
+	)
+	reg.Register(
+		httpmock.REST("GET", "repos/example/action/tags"),
+		httpmock.JSONResponse([]any{
+			map[string]any{"name": "v1", "commit": map[string]any{"sha": sha}},
+		}),
+	)
+
+	workflowPath := writeTempWorkflow(t, `
+name: ci
+on: push
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: example/action@`+sha+`
+`,
+		"example/action@v1=sha1-"+sha,
+	)
+
+	stdout, _, err := runCommandWithHTTP(t, reg,
+		"--no-narrow", "--json=workflows", workflowPath,
+	)
+	require.NoError(t, err)
+
+	var payload struct {
+		Workflows []struct {
+			Dependencies []format.Dependency `json:"dependencies"`
+		} `json:"workflows"`
+	}
+	require.NoError(t, json.Unmarshal([]byte(stdout), &payload))
+	require.Len(t, payload.Workflows, 1)
+	require.Len(t, payload.Workflows[0].Dependencies, 1)
+	assert.True(t, payload.Workflows[0].Dependencies[0].Direct)
+	assert.Contains(t, readTempLockfilePins(t), "    - 'example/action@v1'")
+}
+
 func TestCheckCommand_JSONDefaultFieldsExcludesDependencies(t *testing.T) {
 	reg := &httpmock.Registry{}
 	defer reg.Verify(t)

internal/pipeline/diagnose.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"strings"
 
+	parserlock "github.com/github/actions-lockfile/go/pkg/lockfile"
 	"github.com/github/gh-actions-lock/internal/dep"
 	"github.com/github/gh-actions-lock/internal/ghapi"
 	"github.com/github/gh-actions-lock/internal/lockfile"
@@ -61,6 +62,9 @@ func diagnoseOneParsed(ctx context.Context, pw checks.ParsedWorkflow, r *resolve
 	directRefs := make(map[ghapi.NWORef]bool, len(pw.Refs))
 	for _, ref := range pw.Refs {
 		directRefs[ghapi.ForNWORef(ref.Owner, ref.Repo, ref.Ref)] = true
+		if parserlock.IsFullSha(ref.Ref) {
+			directRefs[ghapi.ForNWORef(ref.Owner, ref.Repo, strings.ToLower(ref.Ref))] = true
+		}
 	}
 
 	// Resolve live state: hits cache when ParseAll's caller pre-warmed the
@@ -117,11 +121,10 @@ func diagnoseOneParsed(ctx context.Context, pw checks.ParsedWorkflow, r *resolve
 	}
 
 	for _, dep := range pw.RecordedDeps {
-		owner, repo := dep.OwnerRepo()
 		wr.Inventory = append(wr.Inventory, checks.InventoryEntry{
 			Dep:    dep,
 			File:   pw.Path,
-			Direct: directRefs[ghapi.ForNWORef(owner, repo, dep.Ref)],
+			Direct: isDirectDependency(dep, directRefs),
 		})
 	}
 	parentMap := mergeParentMaps(pw.RecordedParents, resolvedParents)

internal/pipeline/finding_enrich.go

@@ -1,6 +1,8 @@
 package pipeline
 
 import (
+	"strings"
+
 	"github.com/github/gh-actions-lock/internal/dep"
 	"github.com/github/gh-actions-lock/internal/ghapi"
 	"github.com/github/gh-actions-lock/internal/pipeline/checks"
@@ -18,8 +20,7 @@ func attachParent(f *checks.Finding, depByKey map[string]dep.Dependency, directR
 	if f.Dependency == nil {
 		return
 	}
-	owner, repo := f.Dependency.OwnerRepo()
-	if directRefs[ghapi.ForNWORef(owner, repo, f.Dependency.Ref)] {
+	if isDirectDependency(*f.Dependency, directRefs) {
 		return
 	}
 	// Prefer the dep snapshot from the workflow's RecordedDeps (it has the
@@ -34,6 +35,12 @@ func attachParent(f *checks.Finding, depByKey map[string]dep.Dependency, directR
 	}
 }
 
+func isDirectDependency(d dep.Dependency, directRefs map[ghapi.NWORef]bool) bool {
+	owner, repo := d.OwnerRepo()
+	return directRefs[ghapi.ForNWORef(owner, repo, d.Ref)] ||
+		d.SHA != "" && directRefs[ghapi.ForNWORef(owner, repo, strings.ToLower(d.SHA))]
+}
+
 // isTransitivePin reports whether the finding refers to a dep reached via
 // composite expansion (i.e. has parents in the parent map).
 func isTransitivePin(f checks.Finding, depByKey map[string]dep.Dependency, parentMap map[string][]string) bool {