From 29db3dd742d7f9003a3c43c6fd9649592d5e82df Mon Sep 17 00:00:00 2001
From: Jeff Martin <nodeselector@github.com>
Date: Fri, 19 Jun 2026 18:34:40 -0500
Subject: [PATCH 01/12] Reject empty commit SHA in lockfile dependency entries

commit:"" passes requiredActionKeys presence check but bypasses
rejectZeroValues because nonEmptyStringKeys only listed "branch".
An attacker-controlled lockfile could set commit:"" on every action,
silently converting a required integrity field into a no-op for any
consumer that reads Action.Commit without separately checking for
the empty-string case.

Fix: add "commit" to nonEmptyStringKeys so the same zero-value
rejection that guards "branch" now guards "commit" too.
---
 go/pkg/lockfile/lockfile.go      |  1 +
 go/pkg/lockfile/lockfile_test.go | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go
index 1380e8f..5fa3bf5 100644
--- a/go/pkg/lockfile/lockfile.go
+++ b/go/pkg/lockfile/lockfile.go
@@ -428,6 +428,7 @@ func validateKnownFields(f *File, paths []string) *ParseError {
 // nonEmptyStringKeys lists action fields that must be non-empty strings.
 var nonEmptyStringKeys = map[string]struct{}{
 	"branch": {},
+	"commit": {},
 }
 
 // positiveIntKeys lists action fields that must be positive integers (> 0).
diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go
index f1185fa..fe50368 100644
--- a/go/pkg/lockfile/lockfile_test.go
+++ b/go/pkg/lockfile/lockfile_test.go
@@ -261,3 +261,23 @@ func mapKeys[V any](m map[string]V) []string {
 	}
 	return out
 }
+
+// ── Security hardening tests ──────────────────────────────────────────────────
+
+func TestParse_CommitEmptyStringRejected(t *testing.T) {
+	// commit:"" must be rejected: an empty commit SHA disables every downstream
+	// integrity check that reads Action.Commit, silently converting a
+	// required field into a no-op.
+	yaml := `version: v0.0.1
+dependencies:
+  actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:
+    branch: main
+    commit: ""
+    owner_id: 1
+    repo_id: 1
+`
+	_, err := Parse([]byte(yaml))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), `"commit"`)
+	assert.Contains(t, err.Error(), "must not be empty")
+}

From 5b9599bd8a448efde48293b6aadf240dbc491ab2 Mon Sep 17 00:00:00 2001
From: Jeff Martin <nodeselector@github.com>
Date: Fri, 19 Jun 2026 18:35:44 -0500
Subject: [PATCH 02/12] Validate commit field as algo-hex digest

commit:"notadigest" or commit:"HEAD" passed validation because
rejectZeroValues only checked for the empty string. A crafted lockfile
could supply a plausible-looking but structurally invalid commit,
making downstream integrity checks produce false results.

Fix: after the empty-string gate, validate the commit value via
isValidAlgoHex, which calls the same isValidDigest path used by
ParsePin so the two never drift apart.
---
 go/pkg/lockfile/lockfile.go      | 32 ++++++++++++++++++++--
 go/pkg/lockfile/lockfile_test.go | 47 ++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go
index 5fa3bf5..586a8d0 100644
--- a/go/pkg/lockfile/lockfile.go
+++ b/go/pkg/lockfile/lockfile.go
@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"regexp"
 	"strconv"
+	"strings"
 
 	"gopkg.in/yaml.v3"
 )
@@ -438,9 +439,10 @@ var positiveIntKeys = map[string]struct{}{
 }
 
 // rejectZeroValues checks that required action fields carry meaningful values:
-// string fields like "branch" must be non-empty, and integer ID fields like
-// "owner_id" and "repo_id" must be positive. A present-but-zero-value field
-// would silently disable the security check it's meant to enforce.
+// string fields like "branch" must be non-empty, integer ID fields like
+// "owner_id" and "repo_id" must be positive, and the "commit" field must be
+// a valid algo-hex digest string. A present-but-zero-value field would
+// silently disable the security check it's meant to enforce.
 func rejectZeroValues(action *yaml.Node, dep string) *ParseError {
 	for j := 0; j+1 < len(action.Content); j += 2 {
 		key := action.Content[j]
@@ -456,6 +458,16 @@ func rejectZeroValues(action *yaml.Node, dep string) *ParseError {
 			}
 		}
 
+		if key.Value == "commit" && val.Value != "" {
+			if !isValidAlgoHex(val.Value) {
+				return &ParseError{
+					Line:   val.Line,
+					Column: val.Column,
+					Msg:    fmt.Sprintf("action field \"commit\" must be an algo-hex digest (e.g. \"sha1-abc...\") for dependency %q, got %q", dep, val.Value),
+				}
+			}
+		}
+
 		if _, ok := positiveIntKeys[key.Value]; ok {
 			n, err := strconv.ParseInt(val.Value, 10, 64)
 			if err != nil || n <= 0 {
@@ -586,3 +598,17 @@ func parseSchemaVersion(v string) ([3]int, bool) {
 	}
 	return out, true
 }
+
+// isValidAlgoHex reports whether s is a properly-formed algo-hex digest string
+// in the lockfile's "algo-hexdigest" format (e.g. "sha1-abc123..." or
+// "sha256-abc123..."). It delegates hex and length validation to isValidDigest
+// so the two never drift apart.
+func isValidAlgoHex(s string) bool {
+	dashIdx := strings.IndexByte(s, '-')
+	if dashIdx <= 0 || dashIdx == len(s)-1 {
+		return false
+	}
+	algo := strings.ToLower(s[:dashIdx])
+	hex := strings.ToLower(s[dashIdx+1:])
+	return isValidDigest(algo, hex)
+}
diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go
index fe50368..4e8a1d5 100644
--- a/go/pkg/lockfile/lockfile_test.go
+++ b/go/pkg/lockfile/lockfile_test.go
@@ -281,3 +281,50 @@ dependencies:
 	assert.Contains(t, err.Error(), `"commit"`)
 	assert.Contains(t, err.Error(), "must not be empty")
 }
+
+func TestParse_CommitInvalidFormatRejected(t *testing.T) {
+	// A non-empty commit that isn't a valid algo-hex digest must be rejected.
+	// "notadigest", "sha1-", and "HEAD" look plausible but carry no integrity
+	// guarantee; consumers checking the algo and hex individually would silently
+	// accept them, defeating the lockfile's purpose.
+	cases := []struct {
+		name   string
+		commit string
+	}{
+		{"arbitrary string", "notadigest"},
+		{"no hex after dash", "sha1-"},
+		{"wrong length hex", "sha1-abc123"},
+		{"symbolic ref", "HEAD"},
+		{"sha1 prefix only", "sha1"},
+		{"non-hex chars", "sha1-zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			y := "version: v0.0.1\ndependencies:\n" +
+				"  actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:\n" +
+				"    branch: main\n" +
+				"    commit: " + tc.commit + "\n" +
+				"    owner_id: 1\n" +
+				"    repo_id: 1\n"
+			_, err := Parse([]byte(y))
+			require.Error(t, err, "commit %q should be rejected", tc.commit)
+			assert.Contains(t, err.Error(), "commit")
+		})
+	}
+}
+
+func TestParse_CommitValidFormatsAccepted(t *testing.T) {
+	cases := []string{
+		"sha1-11bd71901bbe5b1630ceea73d27597364c9af683",
+		"sha256-a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2",
+	}
+	for _, commit := range cases {
+		t.Run(commit[:10], func(t *testing.T) {
+			pin := "actions/checkout@v4:" + commit
+			y := "version: v0.0.1\ndependencies:\n  " + pin + ":\n" +
+				"    branch: main\n    commit: " + commit + "\n    owner_id: 1\n    repo_id: 1\n"
+			_, err := Parse([]byte(y))
+			require.NoError(t, err)
+		})
+	}
+}

From df9a049dc8e259f975ceb6042930d82476a29604 Mon Sep 17 00:00:00 2001
From: Jeff Martin <nodeselector@github.com>
Date: Fri, 19 Jun 2026 18:36:25 -0500
Subject: [PATCH 03/12] Reject commit field that disagrees with pin key digest
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Parse accepted action entries where commit:"sha1-AAAA" lived under
a key ending in :sha1-BBBB. A consumer checking action.Commit trusts
a different hash than the pin key they used for the lookup — the
lockfile's two representations of the same digest point at different
commits, enabling a bait-and-switch on consumers that only check one.

Fix: in canonicalizeActions, compare action.Commit against the
pin key's algo+hex; return an error (locatable in the YAML tree)
when they disagree.
---
 go/pkg/lockfile/lockfile.go      | 12 ++++++++++++
 go/pkg/lockfile/lockfile_test.go | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go
index 586a8d0..572321c 100644
--- a/go/pkg/lockfile/lockfile.go
+++ b/go/pkg/lockfile/lockfile.go
@@ -496,6 +496,18 @@ func canonicalizeActions(f *File) (string, error) {
 		canonical := key
 		if pin, ok := ParsePin(key); ok {
 			canonical = pin.String()
+			// The commit field in the action body must match the digest
+			// embedded in the pin key. A mismatch is a trust-confusion
+			// attack: the caller looks up the action by key (and its
+			// embedded hash) but the body carries a different hash that
+			// a different downstream check might trust.
+			pinDigest := pin.Algo + "-" + pin.Hex
+			if action.Commit != "" && action.Commit != pinDigest {
+				return key, fmt.Errorf(
+					"action %q commit field %q disagrees with pin key digest %q",
+					canonical, action.Commit, pinDigest,
+				)
+			}
 		}
 		// Canonicalize Uses entries too so cross-references resolve.
 		if len(action.Uses) > 0 {
diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go
index 4e8a1d5..f03b1db 100644
--- a/go/pkg/lockfile/lockfile_test.go
+++ b/go/pkg/lockfile/lockfile_test.go
@@ -328,3 +328,35 @@ func TestParse_CommitValidFormatsAccepted(t *testing.T) {
 		})
 	}
 }
+
+func TestParse_CommitMismatchWithPinKeyRejected(t *testing.T) {
+	// The commit field in the action body must match the digest embedded in
+	// the pin key. A mismatch is a trust-confusion attack: a consumer that
+	// checks action.Commit trusts a different hash than the pin key they
+	// used to look up the action.
+	yaml := `version: v0.0.1
+dependencies:
+  actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:
+    branch: main
+    commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+    owner_id: 1
+    repo_id: 1
+`
+	_, err := Parse([]byte(yaml))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "disagrees with pin key digest")
+}
+
+func TestParse_CommitMatchingPinKeyAccepted(t *testing.T) {
+	// When commit matches the pin key digest, parse must succeed.
+	yaml := `version: v0.0.1
+dependencies:
+  actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:
+    branch: main
+    commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683
+    owner_id: 1
+    repo_id: 1
+`
+	_, err := Parse([]byte(yaml))
+	require.NoError(t, err)
+}

From f1bde12cb251c0929da48ff470fce42e2486fa5f Mon Sep 17 00:00:00 2001
From: Jeff Martin <nodeselector@github.com>
Date: Fri, 19 Jun 2026 18:37:56 -0500
Subject: [PATCH 04/12] Validate ref component in ParsePin via isValidRef
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ParsePin extracted the ref from a pin string but only checked for
an embedded colon — it never validated the ref against the same
denylist that ParseActionRef applies via isValidRef. A crafted pin
key with a ref containing spaces, quotes, backtick, or backslash
returned ok=true and handed the caller a Pin.Ref loaded with shell
metacharacters. Any consumer that passes Pin.Ref to a shell command,
URL builder, or GraphQL literal is directly exploitable.

Fix: apply isValidRef to the extracted ref before accepting the
parse; the function is the single denylist definition shared with
ParseActionRef, so the two parsers cannot drift apart.
---
 go/pkg/lockfile/pin.go      |  8 ++++++++
 go/pkg/lockfile/pin_test.go | 25 +++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/go/pkg/lockfile/pin.go b/go/pkg/lockfile/pin.go
index a2ec105..bfad468 100644
--- a/go/pkg/lockfile/pin.go
+++ b/go/pkg/lockfile/pin.go
@@ -98,6 +98,14 @@ func ParsePin(s string) (Pin, bool) {
 	if strings.ContainsRune(ref, ':') {
 		return Pin{}, false
 	}
+	// Validate the ref with the same denylist used by ParseActionRef so that
+	// a parsed Pin is safe to pass to URL builders and GraphQL string
+	// literals without per-call escaping. Without this check a crafted pin
+	// key like "owner/repo@v1 ; malicious:sha1-..." parses successfully and
+	// the caller receives a Pin.Ref containing shell metacharacters.
+	if !isValidRef(ref) {
+		return Pin{}, false
+	}
 	hashSpec := refHash[colonIdx+1:]
 
 	dashIdx := strings.IndexByte(hashSpec, '-')
diff --git a/go/pkg/lockfile/pin_test.go b/go/pkg/lockfile/pin_test.go
index d120441..be51016 100644
--- a/go/pkg/lockfile/pin_test.go
+++ b/go/pkg/lockfile/pin_test.go
@@ -128,3 +128,28 @@ func TestPin_IndexKey(t *testing.T) {
 		})
 	}
 }
+
+func TestParsePin_RefInjectionRejected(t *testing.T) {
+	// ParsePin must reject refs containing characters that are unsafe for
+	// URL paths and GraphQL string literals (spaces, quotes, backticks,
+	// backslash). Without this check a caller receives a Pin.Ref that
+	// injects into downstream shell commands or API calls.
+	cases := []struct {
+		name  string
+		input string
+	}{
+		{"space in ref", "actions/checkout@v4 evil:sha1-11bd71901bbe5b1630ceea73d27597364c9af683"},
+		{"tab in ref", "actions/checkout@v4\tevil:sha1-11bd71901bbe5b1630ceea73d27597364c9af683"},
+		{"double-quote in ref", "actions/checkout@v4\"evil:sha1-11bd71901bbe5b1630ceea73d27597364c9af683"},
+		{"single-quote in ref", "actions/checkout@v4'evil:sha1-11bd71901bbe5b1630ceea73d27597364c9af683"},
+		{"backtick in ref", "actions/checkout@v4`evil:sha1-11bd71901bbe5b1630ceea73d27597364c9af683"},
+		{"backslash in ref", "actions/checkout@v4\\evil:sha1-11bd71901bbe5b1630ceea73d27597364c9af683"},
+		{"dotdot in ref", "actions/checkout@../evil:sha1-11bd71901bbe5b1630ceea73d27597364c9af683"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			_, ok := ParsePin(tc.input)
+			assert.False(t, ok, "ParsePin should reject ref with injection chars: %q", tc.input)
+		})
+	}
+}

From 772ab0a205cb838d3c7edd2a6e74031d4a8498ee Mon Sep 17 00:00:00 2001
From: Jeff Martin <nodeselector@github.com>
Date: Fri, 19 Jun 2026 18:38:48 -0500
Subject: [PATCH 05/12] Reject path traversal and absolute paths in workflow
 map keys

Workflow keys (e.g. ".github/workflows/ci.yml") are used by consumers
as repo-relative file paths. Parse never validated them, so a crafted
lockfile could include "../../../etc/passwd" or "/etc/shadow" as a key.
Any consumer that calls os.Open(key) or feeds the key to filepath.Join
without sanitizing gets an arbitrary-file-read primitive.

Fix: validateWorkflowPaths checks every key in f.Workflows:
- no leading "/" (no absolute paths)
- no ".." segment (no traversal)
- no control characters
Errors are anchored to the offending YAML key node when available.
---
 go/pkg/lockfile/lockfile.go      | 51 ++++++++++++++++++++++++++++++++
 go/pkg/lockfile/lockfile_test.go | 38 ++++++++++++++++++++++++
 2 files changed, 89 insertions(+)

diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go
index 572321c..fab4ca4 100644
--- a/go/pkg/lockfile/lockfile.go
+++ b/go/pkg/lockfile/lockfile.go
@@ -302,6 +302,9 @@ func Parse(contents []byte, paths ...string) (File, error) {
 	if pe := validateKnownFields(&f, paths); pe != nil {
 		return File{}, pe
 	}
+	if pe := validateWorkflowPaths(&f); pe != nil {
+		return File{}, pe
+	}
 	if conflictKey, err := canonicalizeActions(&f); err != nil {
 		pe := &ParseError{Msg: err.Error(), err: err}
 		if l, c, ok := f.KeyPosition("dependencies", conflictKey); ok {
@@ -313,6 +316,54 @@ func Parse(contents []byte, paths ...string) (File, error) {
 	return f, nil
 }
 
+// validateWorkflowPaths checks that every key in f.Workflows is a safe
+// repo-relative file path. Workflow keys are used by consumers as file paths
+// (e.g. to open the workflow file on disk), so a crafted lockfile with a key
+// like "../../../etc/passwd" or an absolute path "/etc/shadow" would give
+// any consumer that calls os.Open(key) an arbitrary-read primitive.
+//
+// Rules:
+//   - Must not be empty.
+//   - Must not be an absolute path (no leading "/").
+//   - Must not contain ".." as a path segment — rejects traversal after
+//     path.Clean even if embedded in a longer path.
+//   - Must not contain null bytes or other control characters.
+func validateWorkflowPaths(f *File) *ParseError {
+	_, workflowsNode := mappingEntry(docMapping(f.node), "workflows")
+	for key := range f.Workflows {
+		if err := checkWorkflowPathKey(key); err != nil {
+			pe := &ParseError{Msg: err.Error()}
+			if workflowsNode != nil {
+				if k, _ := mappingEntry(workflowsNode, key); k != nil {
+					pe.Line, pe.Column = k.Line, k.Column
+				}
+			}
+			return pe
+		}
+	}
+	return nil
+}
+
+func checkWorkflowPathKey(p string) error {
+	if p == "" {
+		return fmt.Errorf("workflow path key must not be empty")
+	}
+	if strings.HasPrefix(p, "/") {
+		return fmt.Errorf("workflow path key must be repo-relative, not absolute: %q", p)
+	}
+	for _, c := range p {
+		if c <= 0x1F || c == 0x7F {
+			return fmt.Errorf("workflow path key contains control characters: %q", p)
+		}
+	}
+	for _, seg := range strings.Split(p, "/") {
+		if seg == ".." {
+			return fmt.Errorf("workflow path key contains path traversal: %q", p)
+		}
+	}
+	return nil
+}
+
 // allowedFileKeys is the set of permitted top-level lockfile keys. It mirrors
 // the document-level properties declared in lockfile-v0.0.1.json.
 var allowedFileKeys = map[string]struct{}{
diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go
index f03b1db..a8611ba 100644
--- a/go/pkg/lockfile/lockfile_test.go
+++ b/go/pkg/lockfile/lockfile_test.go
@@ -360,3 +360,41 @@ dependencies:
 	_, err := Parse([]byte(yaml))
 	require.NoError(t, err)
 }
+
+func TestParse_WorkflowPathTraversalRejected(t *testing.T) {
+	// Workflow map keys are consumed as file paths by callers — accepting
+	// "../../../etc/passwd" or "/etc/shadow" as a key is an arbitrary read
+	// primitive for any consumer that calls os.Open(key).
+	cases := []struct {
+		name string
+		key  string
+	}{
+		{"parent traversal", "../../../etc/passwd"},
+		{"embedded traversal", ".github/../../../etc/passwd"},
+		{"absolute path", "/etc/shadow"},
+		{"double-dot segment", ".github/workflows/../../evil.yml"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			y := "version: v0.0.1\ndependencies: {}\nworkflows:\n  " + tc.key + ": []\n"
+			_, err := Parse([]byte(y))
+			require.Error(t, err, "workflow key %q should be rejected", tc.key)
+			assert.Contains(t, err.Error(), "workflow path key")
+		})
+	}
+}
+
+func TestParse_WorkflowPathLegitimateKeysAccepted(t *testing.T) {
+	legit := []string{
+		".github/workflows/ci.yml",
+		".github/workflows/release.yaml",
+		"custom/path/workflow.yml",
+	}
+	for _, key := range legit {
+		t.Run(key, func(t *testing.T) {
+			y := "version: v0.0.1\ndependencies: {}\nworkflows:\n  " + key + ": []\n"
+			_, err := Parse([]byte(y))
+			require.NoError(t, err)
+		})
+	}
+}

From 1875e94a74dd7ef73ef49c9ca68737d77859dd76 Mon Sep 17 00:00:00 2001
From: Jeff Martin <nodeselector@github.com>
Date: Fri, 19 Jun 2026 18:40:40 -0500
Subject: [PATCH 06/12] Validate branch and tag fields against injection
 denylist
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

branch and tag values in action metadata are used in GraphQL queries,
log output, and sometimes shell commands by consumers. Parse accepted
any non-empty string — "main\ninjected-header: value" or
"main\\evil" were both valid. An attacker-controlled lockfile could
arm a downstream injection through either field.

Fix: rejectZeroValues now applies isValidRef to branch and tag values
when present, using the same denylist as ParseActionRef (rejects
whitespace, quotes, backslash, backtick, and ".." sequences). This
is the single shared denylist definition so the two parsers cannot
drift apart.
---
 go/pkg/lockfile/lockfile.go      | 23 +++++++++++++---
 go/pkg/lockfile/lockfile_test.go | 45 ++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go
index fab4ca4..a8a318c 100644
--- a/go/pkg/lockfile/lockfile.go
+++ b/go/pkg/lockfile/lockfile.go
@@ -491,9 +491,11 @@ var positiveIntKeys = map[string]struct{}{
 
 // rejectZeroValues checks that required action fields carry meaningful values:
 // string fields like "branch" must be non-empty, integer ID fields like
-// "owner_id" and "repo_id" must be positive, and the "commit" field must be
-// a valid algo-hex digest string. A present-but-zero-value field would
-// silently disable the security check it's meant to enforce.
+// "owner_id" and "repo_id" must be positive, the "commit" field must be
+// a valid algo-hex digest string, and the "branch"/"tag" fields must not
+// contain characters that are unsafe for downstream interpolation. A
+// present-but-zero-value or injection-bearing field would silently disable
+// the security check it's meant to enforce, or arm a downstream injection.
 func rejectZeroValues(action *yaml.Node, dep string) *ParseError {
 	for j := 0; j+1 < len(action.Content); j += 2 {
 		key := action.Content[j]
@@ -519,6 +521,21 @@ func rejectZeroValues(action *yaml.Node, dep string) *ParseError {
 			}
 		}
 
+		// branch and tag values are used in GraphQL queries, log output,
+		// and sometimes shell commands by consumers. Validate them with
+		// the same denylist that ParseActionRef applies to refs so that a
+		// crafted lockfile cannot arm a downstream injection through these
+		// fields.
+		if (key.Value == "branch" || key.Value == "tag") && val.Value != "" {
+			if !isValidRef(val.Value) {
+				return &ParseError{
+					Line:   val.Line,
+					Column: val.Column,
+					Msg:    fmt.Sprintf("action field %q contains unsafe characters for dependency %q: %q", key.Value, dep, val.Value),
+				}
+			}
+		}
+
 		if _, ok := positiveIntKeys[key.Value]; ok {
 			n, err := strconv.ParseInt(val.Value, 10, 64)
 			if err != nil || n <= 0 {
diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go
index a8611ba..6793b66 100644
--- a/go/pkg/lockfile/lockfile_test.go
+++ b/go/pkg/lockfile/lockfile_test.go
@@ -398,3 +398,48 @@ func TestParse_WorkflowPathLegitimateKeysAccepted(t *testing.T) {
 		})
 	}
 }
+
+func TestParse_BranchInjectionCharsRejected(t *testing.T) {
+	// branch values are used in GraphQL queries, log output, and sometimes
+	// shell commands. Characters that survive YAML parsing but are unsafe
+	// for downstream interpolation (backslash, `..`, whitespace) must be
+	// rejected by our validator.
+	cases := []struct {
+		name       string
+		yamlBranch string // value as it appears in YAML (double-quoted to reach our validator)
+	}{
+		// YAML double-quoted escape \\ → literal backslash in parsed value
+		{"backslash", `"main\\evil"`},
+		// YAML double-quoted \t → literal tab
+		{"tab", `"main\tevil"`},
+		// YAML double-quoted \n → literal newline
+		{"newline", `"main\nevil"`},
+		// unquoted dotdot traversal
+		{"dotdot", "../main"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			y := "version: v0.0.1\ndependencies:\n" +
+				"  actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:\n" +
+				"    branch: " + tc.yamlBranch + "\n" +
+				"    commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" +
+				"    owner_id: 1\n    repo_id: 1\n"
+			_, err := Parse([]byte(y))
+			require.Error(t, err, "branch %q should be rejected", tc.yamlBranch)
+			assert.Contains(t, err.Error(), "unsafe characters")
+		})
+	}
+}
+
+func TestParse_TagInjectionCharsRejected(t *testing.T) {
+	// tag is optional but when present must not carry injection characters.
+	y := "version: v0.0.1\ndependencies:\n" +
+		"  actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:\n" +
+		"    branch: main\n" +
+		"    tag: \"../../etc/passwd; rm -rf /\"\n" +
+		"    commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" +
+		"    owner_id: 1\n    repo_id: 1\n"
+	_, err := Parse([]byte(y))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "unsafe characters")
+}

From da9f42b49b50718925cbd8047241297de7e2351f Mon Sep 17 00:00:00 2001
From: Jeff Martin <nodeselector@github.com>
Date: Fri, 19 Jun 2026 18:41:23 -0500
Subject: [PATCH 07/12] Add MaxParseSize input size limit (1 MiB) to Parse
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Parse accepted inputs of unlimited size. A crafted multi-gigabyte
YAML document — or one using YAML alias expansion — would cause
memory exhaustion in yaml.Unmarshal / Decode before any validation
runs. This is a trivially exploitable remote DoS for any service
that feeds untrusted lockfile bytes to Parse.

Fix: reject inputs larger than MaxParseSize (1 MiB) before calling
yaml.Unmarshal. Legitimate lockfiles are orders of magnitude smaller
than this cap. Export the constant so consumers can describe the
limit in their own error messages.
---
 go/pkg/lockfile/lockfile.go      | 10 ++++++++++
 go/pkg/lockfile/lockfile_test.go | 25 +++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go
index a8a318c..f2189a9 100644
--- a/go/pkg/lockfile/lockfile.go
+++ b/go/pkg/lockfile/lockfile.go
@@ -245,6 +245,11 @@ type Action struct {
 //
 // The optional paths parameter scopes per-dependency validation to only the
 // entries referenced by the named workflow paths (via f.Workflows[p]).
+// MaxParseSize is the maximum number of bytes Parse will accept. Inputs larger
+// than this are rejected before any YAML parsing takes place to prevent
+// memory-exhaustion DoS from oversized or yaml-bomb documents.
+const MaxParseSize = 1 << 20 // 1 MiB
+
 // When paths is empty, every dependency entry is validated — the default for
 // whole-file tooling (CLI regen, Dependabot). When paths is non-empty, a
 // dependency entry outside the referenced set is left unchecked so one
@@ -262,6 +267,11 @@ type Action struct {
 // can flag them via diagnostics. Workflow path keys are NOT canonicalized
 // — filesystem paths are case-sensitive on the platforms we run on.
 func Parse(contents []byte, paths ...string) (File, error) {
+	if len(contents) > MaxParseSize {
+		return File{}, &ParseError{
+			Msg: fmt.Sprintf("lockfile too large: %d bytes (max %d)", len(contents), MaxParseSize),
+		}
+	}
 	var root yaml.Node
 	if err := yaml.Unmarshal(contents, &root); err != nil {
 		return File{}, newYAMLParseError(err)
diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go
index 6793b66..bd49c4c 100644
--- a/go/pkg/lockfile/lockfile_test.go
+++ b/go/pkg/lockfile/lockfile_test.go
@@ -443,3 +443,28 @@ func TestParse_TagInjectionCharsRejected(t *testing.T) {
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "unsafe characters")
 }
+
+func TestParse_OversizedInputRejected(t *testing.T) {
+	// An input larger than MaxParseSize must be rejected before any YAML
+	// parsing so that memory-exhaustion DoS from oversized documents is
+	// prevented at the library boundary.
+	oversized := make([]byte, MaxParseSize+1)
+	for i := range oversized {
+		oversized[i] = 'x'
+	}
+	_, err := Parse(oversized)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "too large")
+}
+
+func TestParse_ExactMaxSizeAccepted(t *testing.T) {
+	// A document at exactly MaxParseSize must not be size-rejected
+	// (it will fail for other reasons, but not the size check).
+	atMax := make([]byte, MaxParseSize)
+	for i := range atMax {
+		atMax[i] = 'x'
+	}
+	_, err := Parse(atMax)
+	require.Error(t, err)
+	assert.NotContains(t, err.Error(), "too large")
+}

From 5eaa668e18b63b96717cb0229e2d83542a950df9 Mon Sep 17 00:00:00 2001
From: Jeff Martin <nodeselector@github.com>
Date: Fri, 19 Jun 2026 18:42:13 -0500
Subject: [PATCH 08/12] Detect and reject cycles in the action uses graph
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Parse accepted circular uses references (A uses B, B uses A; or
self-loops A uses A). The lockfile is meant to record a DAG of
action dependencies. Any consumer that walks Action.Uses naively
— build dependency trees, topological sort, transitive closure
computation — will loop infinitely on a crafted lockfile.

Fix: run a three-colour DFS (white/grey/black) over the uses graph
after canonicalization. Any back-edge (grey node revisited) triggers
a ParseError locatable to the offending dependency key in the YAML
tree.
---
 go/pkg/lockfile/lockfile.go      | 57 ++++++++++++++++++++++++++++
 go/pkg/lockfile/lockfile_test.go | 64 ++++++++++++++++++++++++++++++++
 2 files changed, 121 insertions(+)

diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go
index f2189a9..347bb85 100644
--- a/go/pkg/lockfile/lockfile.go
+++ b/go/pkg/lockfile/lockfile.go
@@ -323,9 +323,66 @@ func Parse(contents []byte, paths ...string) (File, error) {
 		return File{}, pe
 	}
 	canonicalizeWorkflowDependencies(&f)
+	if cycle, err := detectUsesCycle(&f); err != nil {
+		pe := &ParseError{Msg: err.Error()}
+		if l, c, ok := f.KeyPosition("dependencies", cycle); ok {
+			pe.Line, pe.Column = l, c
+		}
+		return File{}, pe
+	}
 	return f, nil
 }
 
+// detectUsesCycle reports a cycle in the action uses graph using
+// iterative DFS with three-colour marking. It returns the key of the node
+// that forms the back-edge, or ("", nil) when the graph is acyclic.
+//
+// A cycle in the uses graph means the lockfile's dependency set is not a
+// DAG. Any consumer that naively walks Action.Uses without its own cycle
+// guard would loop infinitely on a crafted lockfile.
+func detectUsesCycle(f *File) (cycleKey string, err error) {
+	const (
+		white = 0 // unvisited
+		grey  = 1 // on the current DFS stack
+		black = 2 // fully processed
+	)
+	color := make(map[string]int, len(f.Dependencies))
+
+	var visit func(key string) bool
+	visit = func(key string) bool {
+		if color[key] == grey {
+			cycleKey = key
+			return true
+		}
+		if color[key] == black {
+			return false
+		}
+		color[key] = grey
+		action, ok := f.Dependencies[key]
+		if ok {
+			for _, dep := range action.Uses {
+				if visit(dep) {
+					if cycleKey == "" {
+						cycleKey = key
+					}
+					return true
+				}
+			}
+		}
+		color[key] = black
+		return false
+	}
+
+	for key := range f.Dependencies {
+		if color[key] == white {
+			if visit(key) {
+				return cycleKey, fmt.Errorf("uses cycle detected at dependency %q", cycleKey)
+			}
+		}
+	}
+	return "", nil
+}
+
 // validateWorkflowPaths checks that every key in f.Workflows is a safe
 // repo-relative file path. Workflow keys are used by consumers as file paths
 // (e.g. to open the workflow file on disk), so a crafted lockfile with a key
diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go
index bd49c4c..96fcec6 100644
--- a/go/pkg/lockfile/lockfile_test.go
+++ b/go/pkg/lockfile/lockfile_test.go
@@ -468,3 +468,67 @@ func TestParse_ExactMaxSizeAccepted(t *testing.T) {
 	require.Error(t, err)
 	assert.NotContains(t, err.Error(), "too large")
 }
+
+func TestParse_UsesCycleRejected(t *testing.T) {
+	// A cycle in the uses graph (A uses B, B uses A) must be rejected.
+	// Any consumer that walks Action.Uses without its own cycle guard
+	// would loop infinitely on such a lockfile.
+	yaml := `version: v0.0.1
+dependencies:
+  actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:
+    branch: main
+    commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683
+    owner_id: 1
+    repo_id: 1
+    uses:
+      - actions/b@v1:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  actions/b@v1:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
+    branch: main
+    commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+    owner_id: 2
+    repo_id: 2
+    uses:
+      - actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683
+`
+	_, err := Parse([]byte(yaml))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "cycle")
+}
+
+func TestParse_UsesSelfCycleRejected(t *testing.T) {
+	// Self-referencing uses (A uses A) must also be caught.
+	yaml := `version: v0.0.1
+dependencies:
+  actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:
+    branch: main
+    commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683
+    owner_id: 1
+    repo_id: 1
+    uses:
+      - actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683
+`
+	_, err := Parse([]byte(yaml))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "cycle")
+}
+
+func TestParse_UsesAcyclicAccepted(t *testing.T) {
+	// A valid DAG (A uses B, B has no uses) must parse successfully.
+	yaml := `version: v0.0.1
+dependencies:
+  actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:
+    branch: main
+    commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683
+    owner_id: 1
+    repo_id: 1
+    uses:
+      - actions/b@v1:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  actions/b@v1:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:
+    branch: main
+    commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+    owner_id: 2
+    repo_id: 2
+`
+	_, err := Parse([]byte(yaml))
+	require.NoError(t, err)
+}

From 365b997019006299579f9f5d1aad05b05cfb19d3 Mon Sep 17 00:00:00 2001
From: Jeff Martin <nodeselector@github.com>
Date: Fri, 19 Jun 2026 18:43:25 -0500
Subject: [PATCH 09/12] Cap uses list length per action at MaxUsesPerAction
 (500)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

With a 1 MiB input cap, a single dependency entry can still pack
~20,000 entries into its uses: sequence (each ~50-byte pin string).
canonicalizeActions allocates a new slice of the same length for
every action — a 20× in-memory amplification on a maximum-size
input. The legitimate upper bound for a real composite action's
uses list is tens of entries.

Fix: add MaxUsesPerAction (500) constant and rejectOverlongUses,
which checks the raw YAML sequence length during validateKnownFields
before any allocation occurs.
---
 go/pkg/lockfile/lockfile.go      | 37 ++++++++++++++++++++++++++
 go/pkg/lockfile/lockfile_test.go | 45 ++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go
index 347bb85..3dbef6b 100644
--- a/go/pkg/lockfile/lockfile.go
+++ b/go/pkg/lockfile/lockfile.go
@@ -540,6 +540,43 @@ func validateKnownFields(f *File, paths []string) *ParseError {
 		if pe := rejectZeroValues(action, pinKey.Value); pe != nil {
 			return pe
 		}
+		// Cap the uses list length to prevent an OOM amplification where
+		// a single dependency entry near the parse-size limit is packed
+		// with thousands of uses entries. The in-memory representation
+		// after canonicalization is proportional to the uses count.
+		if pe := rejectOverlongUses(action, pinKey.Value); pe != nil {
+			return pe
+		}
+	}
+	return nil
+}
+
+// MaxUsesPerAction is the maximum number of entries permitted in a single
+// action's uses list. A lockfile at MaxParseSize could pack thousands of
+// short pin strings into a single uses sequence; canonicalizeActions then
+// allocates a new slice of the same length, amplifying memory use. A
+// genuine composite action uses: list is a small constant (tens of entries
+// at most).
+const MaxUsesPerAction = 500
+
+// rejectOverlongUses returns a ParseError when the uses sequence in action
+// exceeds MaxUsesPerAction.
+func rejectOverlongUses(action *yaml.Node, dep string) *ParseError {
+	for j := 0; j+1 < len(action.Content); j += 2 {
+		key := action.Content[j]
+		val := action.Content[j+1]
+		if key.Value == "uses" && val.Kind == yaml.SequenceNode {
+			if len(val.Content) > MaxUsesPerAction {
+				return &ParseError{
+					Line:   val.Line,
+					Column: val.Column,
+					Msg: fmt.Sprintf(
+						"dependency %q has %d uses entries (max %d)",
+						dep, len(val.Content), MaxUsesPerAction,
+					),
+				}
+			}
+		}
 	}
 	return nil
 }
diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go
index 96fcec6..63b40ff 100644
--- a/go/pkg/lockfile/lockfile_test.go
+++ b/go/pkg/lockfile/lockfile_test.go
@@ -2,6 +2,8 @@ package lockfile
 
 import (
 	"errors"
+	"fmt"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -532,3 +534,46 @@ dependencies:
 	_, err := Parse([]byte(yaml))
 	require.NoError(t, err)
 }
+
+func TestParse_OverlongUsesListRejected(t *testing.T) {
+	// A uses list longer than MaxUsesPerAction must be rejected. At the
+	// 1 MiB input cap, thousands of ~50-byte pin strings fit in one uses
+	// list; canonicalizeActions allocates a new slice of the same length,
+	// amplifying memory well beyond the input size.
+	var sb strings.Builder
+	sb.WriteString("version: v0.0.1\ndependencies:\n")
+	sb.WriteString("  actions/composite@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:\n")
+	sb.WriteString("    branch: main\n")
+	sb.WriteString("    commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n")
+	sb.WriteString("    owner_id: 1\n    repo_id: 1\n    uses:\n")
+
+	// Add MaxUsesPerAction+1 distinct entries. We only need the list to be
+	// over the limit; they don't all need to be valid pins.
+	for i := 0; i <= MaxUsesPerAction; i++ {
+		sb.WriteString(fmt.Sprintf("      - notapin%d\n", i))
+	}
+
+	_, err := Parse([]byte(sb.String()))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "uses entries")
+}
+
+func TestParse_UsesListAtMaxAccepted(t *testing.T) {
+	// A uses list at exactly MaxUsesPerAction entries must be accepted.
+	// (The individual entries needn't be valid pins for this size check.)
+	var sb strings.Builder
+	sb.WriteString("version: v0.0.1\ndependencies:\n")
+	sb.WriteString("  actions/composite@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:\n")
+	sb.WriteString("    branch: main\n")
+	sb.WriteString("    commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n")
+	sb.WriteString("    owner_id: 1\n    repo_id: 1\n    uses:\n")
+	for i := 0; i < MaxUsesPerAction; i++ {
+		sb.WriteString(fmt.Sprintf("      - notapin%d\n", i))
+	}
+
+	_, err := Parse([]byte(sb.String()))
+	// Should NOT error due to the size check (may error for other reasons).
+	if err != nil {
+		assert.NotContains(t, err.Error(), "uses entries")
+	}
+}

From 3701b4631ac74352c43e62f511062a548bf90109 Mon Sep 17 00:00:00 2001
From: Jeff Martin <nodeselector@github.com>
Date: Fri, 19 Jun 2026 18:46:55 -0500
Subject: [PATCH 10/12] ParseActionMeta: add MaxActionMetaSize and
 MaxNestedUses limits
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ParseActionMeta accepted unbounded string input and would collect an
unbounded NestedUses slice from a crafted composite action.yml:

  - A 3.5 MB action.yml with 100,000 composite steps parsed cleanly,
    allocating a 100K-element []string with no back-pressure.
  - No size check ran before yaml.Unmarshal, so yaml.v3 itself had to
    absorb the full document before any limit could be applied.

Fix: add two new exported constants checked at the start of the function:

  MaxActionMetaSize = 64 KiB  (size check before any YAML parsing)
  MaxNestedUses     = 500     (cap on NestedUses slice growth)

64 KiB is generous for action.yml — the largest in the wild is under
20 KiB. 500 composite steps is far beyond any real action; GitHub's
own largest composite action has fewer than 30 steps.

Tests added: OversizedInputRejected, ExactMaxSizeAccepted,
OverlongUsesListRejected, UsesListAtMaxAccepted.
---
 go/pkg/lockfile/action_meta.go      | 24 +++++++++++++--
 go/pkg/lockfile/action_meta_test.go | 46 +++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/go/pkg/lockfile/action_meta.go b/go/pkg/lockfile/action_meta.go
index 0fda6ae..8517af8 100644
--- a/go/pkg/lockfile/action_meta.go
+++ b/go/pkg/lockfile/action_meta.go
@@ -26,6 +26,18 @@ type ActionMeta struct {
 	NestedUses []string
 }
 
+// MaxActionMetaSize is the maximum byte length ParseActionMeta will accept.
+// action.yml files in the wild are well under 64 KB; this limit prevents
+// memory-exhaustion from oversized or yaml-bomb documents before any YAML
+// parsing takes place.
+const MaxActionMetaSize = 64 * 1024 // 64 KiB
+
+// MaxNestedUses is the maximum number of composite-action step `uses:` entries
+// ParseActionMeta will collect. Real composite actions rarely exceed a dozen
+// steps; this cap prevents a crafted action.yml from inflating the NestedUses
+// slice into a large allocation.
+const MaxNestedUses = 500
+
 // ParseActionMeta parses the contents of an action.yml file into an
 // ActionMeta. Composite actions emit their nested step `uses:` strings
 // in NestedUses; non-composite actions return an empty NestedUses.
@@ -33,6 +45,10 @@ type ActionMeta struct {
 // Returns an error only on malformed YAML — unknown `runs.using` values
 // resolve to ExecUnknown rather than failing.
 func ParseActionMeta(content string) (*ActionMeta, error) {
+	if len(content) > MaxActionMetaSize {
+		return nil, fmt.Errorf("action.yml too large: %d bytes (max %d)", len(content), MaxActionMetaSize)
+	}
+
 	var raw struct {
 		Name string `yaml:"name"`
 		Runs struct {
@@ -54,9 +70,13 @@ func ParseActionMeta(content string) (*ActionMeta, error) {
 	case using == "composite":
 		meta.Execution = ExecComposite
 		for _, step := range raw.Runs.Steps {
-			if step.Uses != "" {
-				meta.NestedUses = append(meta.NestedUses, step.Uses)
+			if step.Uses == "" {
+				continue
+			}
+			if len(meta.NestedUses) >= MaxNestedUses {
+				return nil, fmt.Errorf("action.yml has too many composite steps with uses: (max %d)", MaxNestedUses)
 			}
+			meta.NestedUses = append(meta.NestedUses, step.Uses)
 		}
 	case using == "docker":
 		meta.Execution = ExecDocker
diff --git a/go/pkg/lockfile/action_meta_test.go b/go/pkg/lockfile/action_meta_test.go
index 732bcb3..b781e43 100644
--- a/go/pkg/lockfile/action_meta_test.go
+++ b/go/pkg/lockfile/action_meta_test.go
@@ -2,7 +2,9 @@ package lockfile
 
 import (
 	"errors"
+	"fmt"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -35,3 +37,47 @@ func TestParseActionMeta(t *testing.T) {
 		})
 	}
 }
+
+func TestParseActionMeta_OversizedInputRejected(t *testing.T) {
+	// Build an input just over MaxActionMetaSize.
+	oversized := "name: big\n" + strings.Repeat("# padding\n", (MaxActionMetaSize/10)+1)
+	_, err := ParseActionMeta(oversized)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "too large")
+}
+
+func TestParseActionMeta_ExactMaxSizeAccepted(t *testing.T) {
+	// A valid minimal composite action padded to exactly MaxActionMetaSize must be accepted.
+	base := "name: padded\nruns:\n  using: composite\n  steps:\n    - uses: actions/checkout@v4\n"
+	pad := strings.Repeat("#", MaxActionMetaSize-len(base))
+	_, err := ParseActionMeta(base + pad)
+	require.NoError(t, err)
+}
+
+func TestParseActionMeta_OverlongUsesListRejected(t *testing.T) {
+	// Build a composite action with MaxNestedUses+1 steps.
+	var sb strings.Builder
+	sb.WriteString("name: bloated\nruns:\n  using: composite\n  steps:\n")
+	for i := 0; i <= MaxNestedUses; i++ {
+		sb.WriteString(fmt.Sprintf("    - uses: actions/checkout@v%d\n", i))
+	}
+	input := sb.String()
+	require.LessOrEqual(t, len(input), MaxActionMetaSize, "test input must fit within size limit")
+	_, err := ParseActionMeta(input)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "too many composite steps")
+}
+
+func TestParseActionMeta_UsesListAtMaxAccepted(t *testing.T) {
+	// Exactly MaxNestedUses steps must be accepted.
+	var sb strings.Builder
+	sb.WriteString("name: maxok\nruns:\n  using: composite\n  steps:\n")
+	for i := 0; i < MaxNestedUses; i++ {
+		sb.WriteString(fmt.Sprintf("    - uses: actions/checkout@v%d\n", i))
+	}
+	input := sb.String()
+	require.LessOrEqual(t, len(input), MaxActionMetaSize, "test input must fit within size limit")
+	meta, err := ParseActionMeta(input)
+	require.NoError(t, err)
+	assert.Len(t, meta.NestedUses, MaxNestedUses)
+}

From ce7d703fa73380b9d181ad0adcdcfa8fd2ecd8bc Mon Sep 17 00:00:00 2001
From: Jeff Martin <nodeselector@github.com>
Date: Mon, 22 Jun 2026 16:55:49 -0500
Subject: [PATCH 11/12] Address review: reject YAML anchors, drop limits runner
 doesn't enforce
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review feedback applied:

- ParseActionMeta now rejects YAML anchors/aliases explicitly (action.yml
  doesn't use them; their presence is either a mistake or an exploit).
- Removed MaxUsesPerAction (500) and MaxNestedUses (500) — the runner has
  no per-action step count limit, so we can't be stricter.
- Updated detectUsesCycle comment to cite where the runner rejects cycles
  (CompositeActionsMaxDepth in src/Runner.Common/Constants.cs) and frame
  our check as shifting the failure left.
---
 go/pkg/lockfile/action_meta.go      | 47 ++++++++++++++++++---------
 go/pkg/lockfile/action_meta_test.go | 45 ++++++++++----------------
 go/pkg/lockfile/lockfile.go         | 44 +++-----------------------
 go/pkg/lockfile/lockfile_test.go    | 49 -----------------------------
 4 files changed, 53 insertions(+), 132 deletions(-)

diff --git a/go/pkg/lockfile/action_meta.go b/go/pkg/lockfile/action_meta.go
index 8517af8..58e3075 100644
--- a/go/pkg/lockfile/action_meta.go
+++ b/go/pkg/lockfile/action_meta.go
@@ -27,16 +27,8 @@ type ActionMeta struct {
 }
 
 // MaxActionMetaSize is the maximum byte length ParseActionMeta will accept.
-// action.yml files in the wild are well under 64 KB; this limit prevents
-// memory-exhaustion from oversized or yaml-bomb documents before any YAML
-// parsing takes place.
-const MaxActionMetaSize = 64 * 1024 // 64 KiB
-
-// MaxNestedUses is the maximum number of composite-action step `uses:` entries
-// ParseActionMeta will collect. Real composite actions rarely exceed a dozen
-// steps; this cap prevents a crafted action.yml from inflating the NestedUses
-// slice into a large allocation.
-const MaxNestedUses = 500
+// action.yml files in the wild are well under 1 MiB.
+const MaxActionMetaSize = 1 << 20 // 1 MiB
 
 // ParseActionMeta parses the contents of an action.yml file into an
 // ActionMeta. Composite actions emit their nested step `uses:` strings
@@ -49,6 +41,15 @@ func ParseActionMeta(content string) (*ActionMeta, error) {
 		return nil, fmt.Errorf("action.yml too large: %d bytes (max %d)", len(content), MaxActionMetaSize)
 	}
 
+	var doc yaml.Node
+	if err := yaml.Unmarshal([]byte(content), &doc); err != nil {
+		return nil, fmt.Errorf("parsing action.yml: %w", err)
+	}
+
+	if err := rejectYAMLAnchors(&doc); err != nil {
+		return nil, err
+	}
+
 	var raw struct {
 		Name string `yaml:"name"`
 		Runs struct {
@@ -59,7 +60,7 @@ func ParseActionMeta(content string) (*ActionMeta, error) {
 		} `yaml:"runs"`
 	}
 
-	if err := yaml.Unmarshal([]byte(content), &raw); err != nil {
+	if err := doc.Decode(&raw); err != nil {
 		return nil, fmt.Errorf("parsing action.yml: %w", err)
 	}
 
@@ -73,9 +74,6 @@ func ParseActionMeta(content string) (*ActionMeta, error) {
 			if step.Uses == "" {
 				continue
 			}
-			if len(meta.NestedUses) >= MaxNestedUses {
-				return nil, fmt.Errorf("action.yml has too many composite steps with uses: (max %d)", MaxNestedUses)
-			}
 			meta.NestedUses = append(meta.NestedUses, step.Uses)
 		}
 	case using == "docker":
@@ -88,3 +86,24 @@ func ParseActionMeta(content string) (*ActionMeta, error) {
 
 	return meta, nil
 }
+
+// rejectYAMLAnchors walks a yaml.Node tree and returns an error if any anchor
+// definition or alias reference is found. action.yml does not use YAML anchors,
+// so their presence is either a mistake or an attempted exploit.
+func rejectYAMLAnchors(n *yaml.Node) error {
+	if n == nil {
+		return nil
+	}
+	if n.Kind == yaml.AliasNode {
+		return fmt.Errorf("action.yml: YAML anchors and aliases are not supported (line %d)", n.Line)
+	}
+	if n.Anchor != "" {
+		return fmt.Errorf("action.yml: YAML anchors and aliases are not supported (line %d)", n.Line)
+	}
+	for _, child := range n.Content {
+		if err := rejectYAMLAnchors(child); err != nil {
+			return err
+		}
+	}
+	return nil
+}
diff --git a/go/pkg/lockfile/action_meta_test.go b/go/pkg/lockfile/action_meta_test.go
index b781e43..c75731a 100644
--- a/go/pkg/lockfile/action_meta_test.go
+++ b/go/pkg/lockfile/action_meta_test.go
@@ -2,7 +2,6 @@ package lockfile
 
 import (
 	"errors"
-	"fmt"
 	"os"
 	"strings"
 	"testing"
@@ -38,6 +37,22 @@ func TestParseActionMeta(t *testing.T) {
 	}
 }
 
+func TestParseActionMeta_YAMLAnchorsRejected(t *testing.T) {
+	// Anchor definition
+	withAnchor := `
+name: anchored
+runs:
+  using: composite
+  steps:
+    - &step
+      uses: actions/checkout@v4
+    - *step
+`
+	_, err := ParseActionMeta(withAnchor)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "anchors and aliases are not supported")
+}
+
 func TestParseActionMeta_OversizedInputRejected(t *testing.T) {
 	// Build an input just over MaxActionMetaSize.
 	oversized := "name: big\n" + strings.Repeat("# padding\n", (MaxActionMetaSize/10)+1)
@@ -53,31 +68,3 @@ func TestParseActionMeta_ExactMaxSizeAccepted(t *testing.T) {
 	_, err := ParseActionMeta(base + pad)
 	require.NoError(t, err)
 }
-
-func TestParseActionMeta_OverlongUsesListRejected(t *testing.T) {
-	// Build a composite action with MaxNestedUses+1 steps.
-	var sb strings.Builder
-	sb.WriteString("name: bloated\nruns:\n  using: composite\n  steps:\n")
-	for i := 0; i <= MaxNestedUses; i++ {
-		sb.WriteString(fmt.Sprintf("    - uses: actions/checkout@v%d\n", i))
-	}
-	input := sb.String()
-	require.LessOrEqual(t, len(input), MaxActionMetaSize, "test input must fit within size limit")
-	_, err := ParseActionMeta(input)
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "too many composite steps")
-}
-
-func TestParseActionMeta_UsesListAtMaxAccepted(t *testing.T) {
-	// Exactly MaxNestedUses steps must be accepted.
-	var sb strings.Builder
-	sb.WriteString("name: maxok\nruns:\n  using: composite\n  steps:\n")
-	for i := 0; i < MaxNestedUses; i++ {
-		sb.WriteString(fmt.Sprintf("    - uses: actions/checkout@v%d\n", i))
-	}
-	input := sb.String()
-	require.LessOrEqual(t, len(input), MaxActionMetaSize, "test input must fit within size limit")
-	meta, err := ParseActionMeta(input)
-	require.NoError(t, err)
-	assert.Len(t, meta.NestedUses, MaxNestedUses)
-}
diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go
index 3dbef6b..a3c8f2a 100644
--- a/go/pkg/lockfile/lockfile.go
+++ b/go/pkg/lockfile/lockfile.go
@@ -337,9 +337,10 @@ func Parse(contents []byte, paths ...string) (File, error) {
 // iterative DFS with three-colour marking. It returns the key of the node
 // that forms the back-edge, or ("", nil) when the graph is acyclic.
 //
-// A cycle in the uses graph means the lockfile's dependency set is not a
-// DAG. Any consumer that naively walks Action.Uses without its own cycle
-// guard would loop infinitely on a crafted lockfile.
+// The runner rejects cycles at execution time via CompositeActionsMaxDepth
+// (actions/runner: src/Runner.Common/Constants.cs). Detecting them at parse
+// time shifts the failure left so consumers never receive a File whose uses
+// graph is not a DAG.
 func detectUsesCycle(f *File) (cycleKey string, err error) {
 	const (
 		white = 0 // unvisited
@@ -540,43 +541,6 @@ func validateKnownFields(f *File, paths []string) *ParseError {
 		if pe := rejectZeroValues(action, pinKey.Value); pe != nil {
 			return pe
 		}
-		// Cap the uses list length to prevent an OOM amplification where
-		// a single dependency entry near the parse-size limit is packed
-		// with thousands of uses entries. The in-memory representation
-		// after canonicalization is proportional to the uses count.
-		if pe := rejectOverlongUses(action, pinKey.Value); pe != nil {
-			return pe
-		}
-	}
-	return nil
-}
-
-// MaxUsesPerAction is the maximum number of entries permitted in a single
-// action's uses list. A lockfile at MaxParseSize could pack thousands of
-// short pin strings into a single uses sequence; canonicalizeActions then
-// allocates a new slice of the same length, amplifying memory use. A
-// genuine composite action uses: list is a small constant (tens of entries
-// at most).
-const MaxUsesPerAction = 500
-
-// rejectOverlongUses returns a ParseError when the uses sequence in action
-// exceeds MaxUsesPerAction.
-func rejectOverlongUses(action *yaml.Node, dep string) *ParseError {
-	for j := 0; j+1 < len(action.Content); j += 2 {
-		key := action.Content[j]
-		val := action.Content[j+1]
-		if key.Value == "uses" && val.Kind == yaml.SequenceNode {
-			if len(val.Content) > MaxUsesPerAction {
-				return &ParseError{
-					Line:   val.Line,
-					Column: val.Column,
-					Msg: fmt.Sprintf(
-						"dependency %q has %d uses entries (max %d)",
-						dep, len(val.Content), MaxUsesPerAction,
-					),
-				}
-			}
-		}
 	}
 	return nil
 }
diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go
index 63b40ff..2a74c06 100644
--- a/go/pkg/lockfile/lockfile_test.go
+++ b/go/pkg/lockfile/lockfile_test.go
@@ -2,8 +2,6 @@ package lockfile
 
 import (
 	"errors"
-	"fmt"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -472,9 +470,6 @@ func TestParse_ExactMaxSizeAccepted(t *testing.T) {
 }
 
 func TestParse_UsesCycleRejected(t *testing.T) {
-	// A cycle in the uses graph (A uses B, B uses A) must be rejected.
-	// Any consumer that walks Action.Uses without its own cycle guard
-	// would loop infinitely on such a lockfile.
 	yaml := `version: v0.0.1
 dependencies:
   actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:
@@ -498,7 +493,6 @@ dependencies:
 }
 
 func TestParse_UsesSelfCycleRejected(t *testing.T) {
-	// Self-referencing uses (A uses A) must also be caught.
 	yaml := `version: v0.0.1
 dependencies:
   actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:
@@ -534,46 +528,3 @@ dependencies:
 	_, err := Parse([]byte(yaml))
 	require.NoError(t, err)
 }
-
-func TestParse_OverlongUsesListRejected(t *testing.T) {
-	// A uses list longer than MaxUsesPerAction must be rejected. At the
-	// 1 MiB input cap, thousands of ~50-byte pin strings fit in one uses
-	// list; canonicalizeActions allocates a new slice of the same length,
-	// amplifying memory well beyond the input size.
-	var sb strings.Builder
-	sb.WriteString("version: v0.0.1\ndependencies:\n")
-	sb.WriteString("  actions/composite@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:\n")
-	sb.WriteString("    branch: main\n")
-	sb.WriteString("    commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n")
-	sb.WriteString("    owner_id: 1\n    repo_id: 1\n    uses:\n")
-
-	// Add MaxUsesPerAction+1 distinct entries. We only need the list to be
-	// over the limit; they don't all need to be valid pins.
-	for i := 0; i <= MaxUsesPerAction; i++ {
-		sb.WriteString(fmt.Sprintf("      - notapin%d\n", i))
-	}
-
-	_, err := Parse([]byte(sb.String()))
-	require.Error(t, err)
-	assert.Contains(t, err.Error(), "uses entries")
-}
-
-func TestParse_UsesListAtMaxAccepted(t *testing.T) {
-	// A uses list at exactly MaxUsesPerAction entries must be accepted.
-	// (The individual entries needn't be valid pins for this size check.)
-	var sb strings.Builder
-	sb.WriteString("version: v0.0.1\ndependencies:\n")
-	sb.WriteString("  actions/composite@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:\n")
-	sb.WriteString("    branch: main\n")
-	sb.WriteString("    commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n")
-	sb.WriteString("    owner_id: 1\n    repo_id: 1\n    uses:\n")
-	for i := 0; i < MaxUsesPerAction; i++ {
-		sb.WriteString(fmt.Sprintf("      - notapin%d\n", i))
-	}
-
-	_, err := Parse([]byte(sb.String()))
-	// Should NOT error due to the size check (may error for other reasons).
-	if err != nil {
-		assert.NotContains(t, err.Error(), "uses entries")
-	}
-}

From 484f6800660d28a6d4371294dcd9e6ff4a796263 Mon Sep 17 00:00:00 2001
From: Jeff Martin <nodeselector@github.com>
Date: Mon, 22 Jun 2026 17:06:01 -0500
Subject: [PATCH 12/12] Address Copilot review: Windows paths, recursive DFS
 docs, pin comment

- checkWorkflowPathKey: reject backslash and colon characters to prevent
  Windows-style absolute paths (C:\...) and UNC paths (\server\share)
  from bypassing the forward-slash-only traversal checks.
- detectUsesCycle: fix comment to say 'recursive DFS' (not iterative) and
  document why recursion depth is bounded (MaxParseSize limits dependency
  count to ~5,000; Go stacks grow to 1 GB).
- ParsePin ref comment: remove misleading claim about escaping being
  unnecessary. The denylist rejects obviously-malicious refs but callers
  must still escape for their target context.
---
 go/pkg/lockfile/lockfile.go      | 13 ++++++++++++-
 go/pkg/lockfile/lockfile_test.go |  3 +++
 go/pkg/lockfile/pin.go           | 10 +++++-----
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go
index a3c8f2a..ff5a85d 100644
--- a/go/pkg/lockfile/lockfile.go
+++ b/go/pkg/lockfile/lockfile.go
@@ -334,9 +334,14 @@ func Parse(contents []byte, paths ...string) (File, error) {
 }
 
 // detectUsesCycle reports a cycle in the action uses graph using
-// iterative DFS with three-colour marking. It returns the key of the node
+// recursive DFS with three-colour marking. It returns the key of the node
 // that forms the back-edge, or ("", nil) when the graph is acyclic.
 //
+// The recursion depth is bounded by the number of unique keys in
+// f.Dependencies, which is itself bounded by MaxParseSize (a 1 MiB file
+// can hold at most ~5,000 dependency entries). Go's default goroutine stack
+// grows dynamically up to 1 GB, so 5,000 frames is well within budget.
+//
 // The runner rejects cycles at execution time via CompositeActionsMaxDepth
 // (actions/runner: src/Runner.Common/Constants.cs). Detecting them at parse
 // time shifts the failure left so consumers never receive a File whose uses
@@ -423,6 +428,12 @@ func checkWorkflowPathKey(p string) error {
 		if c <= 0x1F || c == 0x7F {
 			return fmt.Errorf("workflow path key contains control characters: %q", p)
 		}
+		// Reject backslash and colon to prevent Windows-style absolute paths
+		// (e.g. "C:\..." or UNC "\\server\...") and backslash-based traversal
+		// (e.g. "..\\..\\.." ) from bypassing the forward-slash checks above.
+		if c == '\\' || c == ':' {
+			return fmt.Errorf("workflow path key contains invalid character %q: %q", string(c), p)
+		}
 	}
 	for _, seg := range strings.Split(p, "/") {
 		if seg == ".." {
diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go
index 2a74c06..01de15f 100644
--- a/go/pkg/lockfile/lockfile_test.go
+++ b/go/pkg/lockfile/lockfile_test.go
@@ -373,6 +373,9 @@ func TestParse_WorkflowPathTraversalRejected(t *testing.T) {
 		{"embedded traversal", ".github/../../../etc/passwd"},
 		{"absolute path", "/etc/shadow"},
 		{"double-dot segment", ".github/workflows/../../evil.yml"},
+		{"windows absolute", "C:/Windows/system.ini"},
+		{"backslash traversal", "..\\\\..\\\\secret"},
+		{"UNC path", "\\\\\\\\server\\\\share"},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
diff --git a/go/pkg/lockfile/pin.go b/go/pkg/lockfile/pin.go
index bfad468..0b2a73a 100644
--- a/go/pkg/lockfile/pin.go
+++ b/go/pkg/lockfile/pin.go
@@ -98,11 +98,11 @@ func ParsePin(s string) (Pin, bool) {
 	if strings.ContainsRune(ref, ':') {
 		return Pin{}, false
 	}
-	// Validate the ref with the same denylist used by ParseActionRef so that
-	// a parsed Pin is safe to pass to URL builders and GraphQL string
-	// literals without per-call escaping. Without this check a crafted pin
-	// key like "owner/repo@v1 ; malicious:sha1-..." parses successfully and
-	// the caller receives a Pin.Ref containing shell metacharacters.
+	// Validate the ref with the same denylist used by ParseActionRef to
+	// reject shell metacharacters, whitespace, and traversal sequences.
+	// This does NOT make the ref safe for verbatim interpolation into URLs
+	// or shell commands -- callers must still escape appropriately for their
+	// context. The goal is to reject obviously-malicious refs at parse time.
 	if !isValidRef(ref) {
 		return Pin{}, false
 	}