From 1313b0aabde9c8f8e8926bb2190848d7b0fb01c7 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Mon, 22 Jun 2026 17:53:25 -0500 Subject: [PATCH 01/15] lockfile: drop branch field, rename tag to ref (optional) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the branch requirement from the Action schema — it is no longer a required field and is dropped entirely. The tag field is renamed to ref and remains optional: not every pinned commit has a symbolic ref recorded. Updates the JSON schema, validation logic (allowedActionKeys, requiredActionKeys, nonEmptyStringKeys, rejectZeroValues), the Action struct, equalAction comparison, and all tests. --- go/pkg/lockfile/lockfile.go | 53 +++++++++++----------------- go/pkg/lockfile/lockfile_test.go | 60 ++++++++------------------------ go/pkg/lockfile/schema_gen.go | 2 +- go/pkg/lockfile/schema_test.go | 32 ++++++----------- schema/lockfile-v0.0.1.json | 11 ++---- 5 files changed, 50 insertions(+), 108 deletions(-) diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go index 2352dc9..267e9a8 100644 --- a/go/pkg/lockfile/lockfile.go +++ b/go/pkg/lockfile/lockfile.go @@ -228,7 +228,7 @@ func mappingEntry(m *yaml.Node, key string) (k, v *yaml.Node) { // pins, ok := f.LookupWorkflow(".github/workflows/deploy.yml") // for _, key := range pins { // action := f.Dependencies[key] -// fmt.Println(action.Branch, action.Commit) +// fmt.Println(action.Ref, action.Commit) // } // // A workflow that is present in the lockfile but has no dependencies returns @@ -242,15 +242,8 @@ func (f File) LookupWorkflow(workflowKey string) ([]string, bool) { // Action carries the per-action metadata recorded in the lockfile under the // pin key. // -// Tag is the release/tag name at the pinned commit, if one exists. Optional: -// commits that are not tagged (e.g. pinned directly to a branch SHA) omit -// this field. -// -// Branch is a branch of the action's repository that contains the pinned -// commit. Required: Parse rejects an Action without one. A valid branch -// confirms that the commit exists in the expected repository — a SHA that -// isn't reachable from any branch in the source repo could belong to a fork -// or an attacker-supplied commit, which SHA-only pinning cannot detect. +// Ref is the git ref (tag or branch) the commit was resolved from, if known. +// Optional: not every pinned commit has a symbolic ref recorded. // // Commit holds the digest in algo-prefixed form (e.g. "sha1-abc123..." or // "sha256-def456..."). This is the same digest that appears in the pin key. @@ -265,8 +258,7 @@ func (f File) LookupWorkflow(workflowKey string) ([]string, bool) { // `uses:` steps) as canonical pin keys. Empty for leaf actions (node, // docker); populated for composite actions. type Action struct { - Tag string `yaml:"tag,omitempty"` - Branch string `yaml:"branch,omitempty"` + Ref string `yaml:"ref,omitempty"` Commit string `yaml:"commit,omitempty"` OwnerID int64 `yaml:"owner_id"` RepoID int64 `yaml:"repo_id"` @@ -504,8 +496,7 @@ var allowedFileKeys = map[string]struct{}{ // allowedActionKeys is the set of permitted keys within a dependency's Action // mapping. It mirrors the $defs/action properties in lockfile-v0.0.1.json. var allowedActionKeys = map[string]struct{}{ - "tag": {}, - "branch": {}, + "ref": {}, "commit": {}, "owner_id": {}, "repo_id": {}, @@ -514,10 +505,10 @@ var allowedActionKeys = map[string]struct{}{ // requiredActionKeys lists the keys every dependency's Action mapping must // carry, in report order. It mirrors the $defs/action "required" list in -// lockfile-v0.0.1.json. `tag` is optional (not every commit is a release) and -// `uses` is required only for composite actions — a condition the lockfile -// alone can't express — so neither appears here. -var requiredActionKeys = []string{"branch", "commit", "owner_id", "repo_id"} +// lockfile-v0.0.1.json. `ref` is optional (not every commit has a symbolic +// ref recorded) and `uses` is required only for composite actions — a +// condition the lockfile alone can't express — so neither appears here. +var requiredActionKeys = []string{"commit", "owner_id", "repo_id"} // validateKnownFields enforces the schema's additionalProperties:false and // required rules on the lockfile's fixed-shape mappings — the document root and @@ -608,7 +599,6 @@ func validateKnownFields(f *File, paths []string) *ParseError { // nonEmptyStringKeys lists action fields that must be non-empty strings. var nonEmptyStringKeys = map[string]struct{}{ - "branch": {}, "commit": {}, } @@ -619,12 +609,12 @@ 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, 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. +// the "commit" field must be non-empty and a valid algo-hex digest string, +// integer ID fields like "owner_id" and "repo_id" must be positive, and +// the "ref" field (when present) 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] @@ -650,12 +640,11 @@ 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 != "" { + // ref values are used in GraphQL queries, log output, and + // sometimes shell commands by consumers. Validate with the same + // denylist that ParseActionRef applies to refs so that a crafted + // lockfile cannot arm a downstream injection through this field. + if key.Value == "ref" && val.Value != "" { if !isValidRef(val.Value) { return &ParseError{ Line: val.Line, @@ -731,7 +720,7 @@ func canonicalizeActions(f *File) (string, error) { } func equalAction(a, b Action) bool { - if a.Tag != b.Tag || a.Branch != b.Branch || a.Commit != b.Commit || + if a.Ref != b.Ref || a.Commit != b.Commit || a.OwnerID != b.OwnerID || a.RepoID != b.RepoID { return false } diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go index 01de15f..c1939e2 100644 --- a/go/pkg/lockfile/lockfile_test.go +++ b/go/pkg/lockfile/lockfile_test.go @@ -131,7 +131,6 @@ func TestParse_CanonicalizesActionKeys(t *testing.T) { yaml := `version: v0.0.1 dependencies: Actions/Checkout@v6:SHA1-8E8C483DB84B4BEE98B60C0593521ED34D9990E8: - branch: main commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 @@ -161,12 +160,10 @@ func TestParse_ConflictingActionKeyCasings(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8: - branch: main commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 Actions/Checkout@v6:SHA1-8E8C483DB84B4BEE98B60C0593521ED34D9990E8: - branch: main commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 9999 repo_id: 1 @@ -181,12 +178,10 @@ func TestParse_DuplicateActionKeyCasingsSameMetadataOK(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8: - branch: main commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 Actions/Checkout@v6:SHA1-8E8C483DB84B4BEE98B60C0593521ED34D9990E8: - branch: main commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 @@ -202,7 +197,6 @@ func TestParse_UnparseableActionKeyPreserved(t *testing.T) { yaml := `version: v0.0.1 dependencies: "not a pin": - branch: main commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1 repo_id: 2 @@ -226,17 +220,15 @@ workflows: assert.True(t, ok) } -func TestParse_TagAndBranchRoundTrip(t *testing.T) { +func TestParse_RefRoundTrip(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8: - tag: v6 - branch: main + ref: v6 commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 actions/internal@trunk:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: - branch: trunk commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa owner_id: 1 repo_id: 2 @@ -245,13 +237,11 @@ workflows: {} f, err := Parse([]byte(yaml)) require.NoError(t, err) - withTag := f.Dependencies["actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8"] - assert.Equal(t, "v6", withTag.Tag) - assert.Equal(t, "main", withTag.Branch) + withRef := f.Dependencies["actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8"] + assert.Equal(t, "v6", withRef.Ref) - branchOnly := f.Dependencies["actions/internal@trunk:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"] - assert.Equal(t, "", branchOnly.Tag) - assert.Equal(t, "trunk", branchOnly.Branch) + noRef := f.Dependencies["actions/internal@trunk:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"] + assert.Equal(t, "", noRef.Ref) } func mapKeys[V any](m map[string]V) []string { @@ -271,7 +261,6 @@ func TestParse_CommitEmptyStringRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: - branch: main commit: "" owner_id: 1 repo_id: 1 @@ -302,7 +291,6 @@ func TestParse_CommitInvalidFormatRejected(t *testing.T) { 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" @@ -322,7 +310,7 @@ func TestParse_CommitValidFormatsAccepted(t *testing.T) { 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" + " commit: " + commit + "\n owner_id: 1\n repo_id: 1\n" _, err := Parse([]byte(y)) require.NoError(t, err) }) @@ -337,7 +325,6 @@ func TestParse_CommitMismatchWithPinKeyRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: - branch: main commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa owner_id: 1 repo_id: 1 @@ -352,7 +339,6 @@ func TestParse_CommitMatchingPinKeyAccepted(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: - branch: main commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 owner_id: 1 repo_id: 1 @@ -402,14 +388,14 @@ func TestParse_WorkflowPathLegitimateKeysAccepted(t *testing.T) { } } -func TestParse_BranchInjectionCharsRejected(t *testing.T) { - // branch values are used in GraphQL queries, log output, and sometimes +func TestParse_RefInjectionCharsRejected(t *testing.T) { + // ref 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) + name string + yamlRef 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"`}, @@ -419,34 +405,23 @@ func TestParse_BranchInjectionCharsRejected(t *testing.T) { {"newline", `"main\nevil"`}, // unquoted dotdot traversal {"dotdot", "../main"}, + // path traversal with semicolons + {"semicolon traversal", `"../../etc/passwd; rm -rf /"`}, } 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" + + " ref: " + tc.yamlRef + "\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) + require.Error(t, err, "ref %q should be rejected", tc.yamlRef) 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") -} - 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 @@ -476,14 +451,12 @@ func TestParse_UsesCycleRejected(t *testing.T) { 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 @@ -499,7 +472,6 @@ func TestParse_UsesSelfCycleRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: - branch: main commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 owner_id: 1 repo_id: 1 @@ -516,14 +488,12 @@ func TestParse_UsesAcyclicAccepted(t *testing.T) { 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 diff --git a/go/pkg/lockfile/schema_gen.go b/go/pkg/lockfile/schema_gen.go index b4479bd..b4fa405 100644 --- a/go/pkg/lockfile/schema_gen.go +++ b/go/pkg/lockfile/schema_gen.go @@ -2,4 +2,4 @@ package lockfile -const schemaV001 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.1.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.1 is supported.\",\n \"const\": \"v0.0.1\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF:ALGO-HEX (e.g. actions/checkout@v4:sha1-34e1...).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@:]+/[^/@:]+@[^:]+:(sha1-[0-9a-f]{40}|sha256-[0-9a-f]{64})$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"branch\", \"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"tag\": {\n \"description\": \"The release or tag the commit was published as, if any. Optional: not every pinned commit corresponds to a tag.\",\n \"type\": \"string\"\n },\n \"branch\": {\n \"description\": \"A branch in the action's repository that contains the pinned commit. Required: it is the authenticity check. A legitimate release commit lives on a branch, while a commit that exists only as a dangling object — pushed to a fork or attached to a PR and never merged — belongs to no branch. Pinning by SHA alone can be tricked into trusting such an impostor commit; verifying the commit is reachable from a branch closes that gap.\",\n \"type\": \"string\",\n \"minLength\": 1\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org). Pinned because names can be deleted and re-registered by someone else; the ID cannot, so it ties the pin to the original owner.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository. Pinned because a repo can be renamed or deleted and the name reclaimed; the ID detects that the repo behind the name has changed.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys, so the full dependency graph stays pinned and verifiable end to end. Required for composite actions (which can call other actions); absent for leaf actions that have no dependencies of their own.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" +const schemaV001 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.1.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.1 is supported.\",\n \"const\": \"v0.0.1\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF:ALGO-HEX (e.g. actions/checkout@v4:sha1-34e1...).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@:]+/[^/@:]+@[^:]+:(sha1-[0-9a-f]{40}|sha256-[0-9a-f]{64})$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"ref\": {\n \"description\": \"The git ref (tag or branch) the commit was resolved from, if known. Optional: not every pinned commit has a symbolic ref recorded.\",\n \"type\": \"string\"\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org). Pinned because names can be deleted and re-registered by someone else; the ID cannot, so it ties the pin to the original owner.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository. Pinned because a repo can be renamed or deleted and the name reclaimed; the ID detects that the repo behind the name has changed.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys, so the full dependency graph stays pinned and verifiable end to end. Required for composite actions (which can call other actions); absent for leaf actions that have no dependencies of their own.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" diff --git a/go/pkg/lockfile/schema_test.go b/go/pkg/lockfile/schema_test.go index a223f1a..d42275a 100644 --- a/go/pkg/lockfile/schema_test.go +++ b/go/pkg/lockfile/schema_test.go @@ -98,11 +98,10 @@ dependencies: } func TestParse_MissingRequiredActionFieldRejected(t *testing.T) { - // owner_id/repo_id/commit present, but branch (required) is absent. + // owner_id/repo_id present, but commit (required) is absent. yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: - commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 2 ` @@ -111,18 +110,17 @@ dependencies: var pe *ParseError require.True(t, errors.As(err, &pe), "expected a *ParseError, got %T", err) - assert.Contains(t, pe.Msg, `missing required action field "branch"`) + assert.Contains(t, pe.Msg, `missing required action field "commit"`) assert.Contains(t, pe.Msg, "actions/checkout@v4", "message should name the offending dependency") assert.Equal(t, 3, pe.Line, "error anchors on the dependency's pin key") assert.Greater(t, pe.Column, 0, "expected a column anchored on the pin key") } -func TestParse_EmptyBranchRejected(t *testing.T) { +func TestParse_EmptyCommitRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: - branch: "" - commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 + commit: "" owner_id: 1 repo_id: 2 ` @@ -131,7 +129,7 @@ dependencies: var pe *ParseError require.True(t, errors.As(err, &pe), "expected a *ParseError, got %T", err) - assert.Contains(t, pe.Msg, `"branch"`) + assert.Contains(t, pe.Msg, `"commit"`) assert.Contains(t, pe.Msg, "must not be empty") } @@ -139,7 +137,6 @@ func TestParse_ZeroOwnerIDRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: - branch: main commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 0 repo_id: 2 @@ -157,7 +154,6 @@ func TestParse_ZeroRepoIDRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: - branch: main commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 0 @@ -175,7 +171,6 @@ func TestParse_NegativeIDRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: - branch: main commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: -1 repo_id: 2 @@ -196,8 +191,7 @@ workflows: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: - tag: v4 - branch: main + ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 2 @@ -212,7 +206,7 @@ dependencies: // corruptLockfile is a shared fixture for scoped-validation tests. // It has two deps: goodPin is valid, corruptPin is missing the required -// "branch" field. Workflow A references only the good dep, workflow B +// "commit" field. Workflow A references only the good dep, workflow B // references only the corrupt dep. const ( goodPin = "actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5" @@ -226,12 +220,10 @@ workflows: - actions/setup-go@v5:sha1-0000000000000000000000000000000000000000 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: - branch: main commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 2 actions/setup-go@v5:sha1-0000000000000000000000000000000000000000: - commit: sha1-0000000000000000000000000000000000000000 owner_id: 3 repo_id: 4 ` @@ -243,7 +235,7 @@ func TestParse_ScopedValidation_NoPaths_ErrorsOnCorruptEntry(t *testing.T) { var pe *ParseError require.True(t, errors.As(err, &pe)) - assert.Contains(t, pe.Msg, `missing required action field "branch"`) + assert.Contains(t, pe.Msg, `missing required action field "commit"`) assert.Contains(t, pe.Msg, corruptPin) } @@ -259,7 +251,7 @@ func TestParse_ScopedValidation_CorruptPathOnly_Errors(t *testing.T) { var pe *ParseError require.True(t, errors.As(err, &pe)) - assert.Contains(t, pe.Msg, `missing required action field "branch"`) + assert.Contains(t, pe.Msg, `missing required action field "commit"`) assert.Contains(t, pe.Msg, corruptPin) } @@ -275,7 +267,7 @@ func TestParse_ScopedValidation_GoodAndCorruptPaths_Errors(t *testing.T) { var pe *ParseError require.True(t, errors.As(err, &pe)) - assert.Contains(t, pe.Msg, `missing required action field "branch"`) + assert.Contains(t, pe.Msg, `missing required action field "commit"`) } func TestParse_ScopedValidation_UnknownActionField_InScope_Errors(t *testing.T) { @@ -285,7 +277,6 @@ workflows: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: - branch: main commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 2 @@ -306,7 +297,6 @@ workflows: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: - branch: main commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 2 @@ -324,7 +314,6 @@ workflows: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: - branch: main commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 0 repo_id: 2 @@ -345,7 +334,6 @@ workflows: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: - branch: main commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 0 repo_id: 2 diff --git a/schema/lockfile-v0.0.1.json b/schema/lockfile-v0.0.1.json index 5109e47..3d98867 100644 --- a/schema/lockfile-v0.0.1.json +++ b/schema/lockfile-v0.0.1.json @@ -35,17 +35,12 @@ "description": "Resolved metadata for a single pinned action.", "type": "object", "additionalProperties": false, - "required": ["branch", "commit", "owner_id", "repo_id"], + "required": ["commit", "owner_id", "repo_id"], "properties": { - "tag": { - "description": "The release or tag the commit was published as, if any. Optional: not every pinned commit corresponds to a tag.", + "ref": { + "description": "The git ref (tag or branch) the commit was resolved from, if known. Optional: not every pinned commit has a symbolic ref recorded.", "type": "string" }, - "branch": { - "description": "A branch in the action's repository that contains the pinned commit. Required: it is the authenticity check. A legitimate release commit lives on a branch, while a commit that exists only as a dangling object — pushed to a fork or attached to a PR and never merged — belongs to no branch. Pinning by SHA alone can be tricked into trusting such an impostor commit; verifying the commit is reachable from a branch closes that gap.", - "type": "string", - "minLength": 1 - }, "commit": { "description": "The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.", "type": "string" From 15801014a0868a6bffa31f1ec8009122f8d0668a Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Mon, 22 Jun 2026 18:38:36 -0500 Subject: [PATCH 02/15] lockfile: make ref required MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CLI reliably populates ref for every dep that passes impostor checks, so an empty ref means the lockfile is stale/corrupt. ref is now in requiredActionKeys and nonEmptyStringKeys. The schema describes the priority the CLI enforces (full semver tag > any tag > branch by protection/default/release pattern) but the parser only checks presence — priority ordering is the CLI's concern. --- go/pkg/lockfile/lockfile.go | 16 ++++++++++------ go/pkg/lockfile/lockfile_test.go | 26 +++++++++++++++++++++----- go/pkg/lockfile/schema_gen.go | 2 +- go/pkg/lockfile/schema_test.go | 15 +++++++++++++-- schema/lockfile-v0.0.1.json | 7 ++++--- 5 files changed, 49 insertions(+), 17 deletions(-) diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go index 267e9a8..c2a5de6 100644 --- a/go/pkg/lockfile/lockfile.go +++ b/go/pkg/lockfile/lockfile.go @@ -242,8 +242,12 @@ func (f File) LookupWorkflow(workflowKey string) ([]string, bool) { // Action carries the per-action metadata recorded in the lockfile under the // pin key. // -// Ref is the git ref (tag or branch) the commit was resolved from, if known. -// Optional: not every pinned commit has a symbolic ref recorded. +// Ref is the git ref the commit was resolved from. Required: every dep that +// passes impostor checks has a resolvable ref. The CLI picks the best ref +// with this priority: full semver tag (e.g. v4.3.1) > any tag (including +// major-only like v4) > branch (protected > default > release/v* or +// releases/v* > any). The parser enforces presence and non-emptiness but +// not the priority ordering — that's the CLI's concern. // // Commit holds the digest in algo-prefixed form (e.g. "sha1-abc123..." or // "sha256-def456..."). This is the same digest that appears in the pin key. @@ -505,10 +509,9 @@ var allowedActionKeys = map[string]struct{}{ // requiredActionKeys lists the keys every dependency's Action mapping must // carry, in report order. It mirrors the $defs/action "required" list in -// lockfile-v0.0.1.json. `ref` is optional (not every commit has a symbolic -// ref recorded) and `uses` is required only for composite actions — a -// condition the lockfile alone can't express — so neither appears here. -var requiredActionKeys = []string{"commit", "owner_id", "repo_id"} +// lockfile-v0.0.1.json. `uses` is required only for composite actions — a +// condition the lockfile alone can't express — so it doesn't appear here. +var requiredActionKeys = []string{"ref", "commit", "owner_id", "repo_id"} // validateKnownFields enforces the schema's additionalProperties:false and // required rules on the lockfile's fixed-shape mappings — the document root and @@ -599,6 +602,7 @@ func validateKnownFields(f *File, paths []string) *ParseError { // nonEmptyStringKeys lists action fields that must be non-empty strings. var nonEmptyStringKeys = map[string]struct{}{ + "ref": {}, "commit": {}, } diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go index c1939e2..7d4e727 100644 --- a/go/pkg/lockfile/lockfile_test.go +++ b/go/pkg/lockfile/lockfile_test.go @@ -131,6 +131,7 @@ func TestParse_CanonicalizesActionKeys(t *testing.T) { yaml := `version: v0.0.1 dependencies: Actions/Checkout@v6:SHA1-8E8C483DB84B4BEE98B60C0593521ED34D9990E8: + ref: v4 commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 @@ -160,10 +161,12 @@ func TestParse_ConflictingActionKeyCasings(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8: + ref: v4 commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 Actions/Checkout@v6:SHA1-8E8C483DB84B4BEE98B60C0593521ED34D9990E8: + ref: v4 commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 9999 repo_id: 1 @@ -178,10 +181,12 @@ func TestParse_DuplicateActionKeyCasingsSameMetadataOK(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8: + ref: v4 commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 Actions/Checkout@v6:SHA1-8E8C483DB84B4BEE98B60C0593521ED34D9990E8: + ref: v4 commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 @@ -197,6 +202,7 @@ func TestParse_UnparseableActionKeyPreserved(t *testing.T) { yaml := `version: v0.0.1 dependencies: "not a pin": + ref: v4 commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1 repo_id: 2 @@ -229,6 +235,7 @@ dependencies: owner_id: 1234 repo_id: 5678 actions/internal@trunk:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: + ref: trunk commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa owner_id: 1 repo_id: 2 @@ -237,11 +244,11 @@ workflows: {} f, err := Parse([]byte(yaml)) require.NoError(t, err) - withRef := f.Dependencies["actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8"] - assert.Equal(t, "v6", withRef.Ref) + withTag := f.Dependencies["actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8"] + assert.Equal(t, "v6", withTag.Ref) - noRef := f.Dependencies["actions/internal@trunk:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"] - assert.Equal(t, "", noRef.Ref) + branchRef := f.Dependencies["actions/internal@trunk:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"] + assert.Equal(t, "trunk", branchRef.Ref) } func mapKeys[V any](m map[string]V) []string { @@ -261,6 +268,7 @@ func TestParse_CommitEmptyStringRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + ref: v4 commit: "" owner_id: 1 repo_id: 1 @@ -291,6 +299,7 @@ func TestParse_CommitInvalidFormatRejected(t *testing.T) { t.Run(tc.name, func(t *testing.T) { y := "version: v0.0.1\ndependencies:\n" + " actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683:\n" + + " ref: v4\n" + " commit: " + tc.commit + "\n" + " owner_id: 1\n" + " repo_id: 1\n" @@ -310,7 +319,7 @@ func TestParse_CommitValidFormatsAccepted(t *testing.T) { t.Run(commit[:10], func(t *testing.T) { pin := "actions/checkout@v4:" + commit y := "version: v0.0.1\ndependencies:\n " + pin + ":\n" + - " commit: " + commit + "\n owner_id: 1\n repo_id: 1\n" + " ref: v4\n commit: " + commit + "\n owner_id: 1\n repo_id: 1\n" _, err := Parse([]byte(y)) require.NoError(t, err) }) @@ -325,6 +334,7 @@ func TestParse_CommitMismatchWithPinKeyRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + ref: v4 commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa owner_id: 1 repo_id: 1 @@ -339,6 +349,7 @@ func TestParse_CommitMatchingPinKeyAccepted(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + ref: v4 commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 owner_id: 1 repo_id: 1 @@ -451,12 +462,14 @@ func TestParse_UsesCycleRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + ref: v4 commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 owner_id: 1 repo_id: 1 uses: - actions/b@v1:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa actions/b@v1:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: + ref: v4 commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa owner_id: 2 repo_id: 2 @@ -472,6 +485,7 @@ func TestParse_UsesSelfCycleRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + ref: v4 commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 owner_id: 1 repo_id: 1 @@ -488,12 +502,14 @@ func TestParse_UsesAcyclicAccepted(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + ref: v4 commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 owner_id: 1 repo_id: 1 uses: - actions/b@v1:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa actions/b@v1:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: + ref: v4 commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa owner_id: 2 repo_id: 2 diff --git a/go/pkg/lockfile/schema_gen.go b/go/pkg/lockfile/schema_gen.go index b4fa405..8b5d31a 100644 --- a/go/pkg/lockfile/schema_gen.go +++ b/go/pkg/lockfile/schema_gen.go @@ -2,4 +2,4 @@ package lockfile -const schemaV001 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.1.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.1 is supported.\",\n \"const\": \"v0.0.1\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF:ALGO-HEX (e.g. actions/checkout@v4:sha1-34e1...).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@:]+/[^/@:]+@[^:]+:(sha1-[0-9a-f]{40}|sha256-[0-9a-f]{64})$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"ref\": {\n \"description\": \"The git ref (tag or branch) the commit was resolved from, if known. Optional: not every pinned commit has a symbolic ref recorded.\",\n \"type\": \"string\"\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org). Pinned because names can be deleted and re-registered by someone else; the ID cannot, so it ties the pin to the original owner.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository. Pinned because a repo can be renamed or deleted and the name reclaimed; the ID detects that the repo behind the name has changed.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys, so the full dependency graph stays pinned and verifiable end to end. Required for composite actions (which can call other actions); absent for leaf actions that have no dependencies of their own.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" +const schemaV001 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.1.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.1 is supported.\",\n \"const\": \"v0.0.1\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF:ALGO-HEX (e.g. actions/checkout@v4:sha1-34e1...).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@:]+/[^/@:]+@[^:]+:(sha1-[0-9a-f]{40}|sha256-[0-9a-f]{64})$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"ref\", \"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"ref\": {\n \"description\": \"The git ref the commit was resolved from. Required: every dep that passes impostor checks has a resolvable ref. The CLI picks the best ref with priority: full semver tag > any tag > branch (protected > default > release/v* > any). The parser enforces presence; priority ordering is the CLI's concern.\",\n \"type\": \"string\",\n \"minLength\": 1\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org). Pinned because names can be deleted and re-registered by someone else; the ID cannot, so it ties the pin to the original owner.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository. Pinned because a repo can be renamed or deleted and the name reclaimed; the ID detects that the repo behind the name has changed.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys, so the full dependency graph stays pinned and verifiable end to end. Required for composite actions (which can call other actions); absent for leaf actions that have no dependencies of their own.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" diff --git a/go/pkg/lockfile/schema_test.go b/go/pkg/lockfile/schema_test.go index d42275a..0e178c0 100644 --- a/go/pkg/lockfile/schema_test.go +++ b/go/pkg/lockfile/schema_test.go @@ -98,10 +98,11 @@ dependencies: } func TestParse_MissingRequiredActionFieldRejected(t *testing.T) { - // owner_id/repo_id present, but commit (required) is absent. + // commit/owner_id/repo_id present, but ref (required) is absent. yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 2 ` @@ -110,7 +111,7 @@ dependencies: var pe *ParseError require.True(t, errors.As(err, &pe), "expected a *ParseError, got %T", err) - assert.Contains(t, pe.Msg, `missing required action field "commit"`) + assert.Contains(t, pe.Msg, `missing required action field "ref"`) assert.Contains(t, pe.Msg, "actions/checkout@v4", "message should name the offending dependency") assert.Equal(t, 3, pe.Line, "error anchors on the dependency's pin key") assert.Greater(t, pe.Column, 0, "expected a column anchored on the pin key") @@ -120,6 +121,7 @@ func TestParse_EmptyCommitRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + ref: v4 commit: "" owner_id: 1 repo_id: 2 @@ -137,6 +139,7 @@ func TestParse_ZeroOwnerIDRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 0 repo_id: 2 @@ -154,6 +157,7 @@ func TestParse_ZeroRepoIDRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 0 @@ -171,6 +175,7 @@ func TestParse_NegativeIDRejected(t *testing.T) { yaml := `version: v0.0.1 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: -1 repo_id: 2 @@ -220,10 +225,12 @@ workflows: - actions/setup-go@v5:sha1-0000000000000000000000000000000000000000 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 2 actions/setup-go@v5:sha1-0000000000000000000000000000000000000000: + ref: v5 owner_id: 3 repo_id: 4 ` @@ -277,6 +284,7 @@ workflows: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 2 @@ -297,6 +305,7 @@ workflows: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 2 @@ -314,6 +323,7 @@ workflows: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 0 repo_id: 2 @@ -334,6 +344,7 @@ workflows: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 dependencies: actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 0 repo_id: 2 diff --git a/schema/lockfile-v0.0.1.json b/schema/lockfile-v0.0.1.json index 3d98867..e2eb4d5 100644 --- a/schema/lockfile-v0.0.1.json +++ b/schema/lockfile-v0.0.1.json @@ -35,11 +35,12 @@ "description": "Resolved metadata for a single pinned action.", "type": "object", "additionalProperties": false, - "required": ["commit", "owner_id", "repo_id"], + "required": ["ref", "commit", "owner_id", "repo_id"], "properties": { "ref": { - "description": "The git ref (tag or branch) the commit was resolved from, if known. Optional: not every pinned commit has a symbolic ref recorded.", - "type": "string" + "description": "The git ref the commit was resolved from. Required: every dep that passes impostor checks has a resolvable ref. The CLI picks the best ref with priority: full semver tag > any tag > branch (protected > default > release/v* > any). The parser enforces presence; priority ordering is the CLI's concern.", + "type": "string", + "minLength": 1 }, "commit": { "description": "The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.", From e9a0b36d29c8065ff270ecbe35d9fe0350f8681b Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Mon, 22 Jun 2026 18:39:38 -0500 Subject: [PATCH 03/15] lockfile: add SplitRef and BestRef helpers SplitRef classifies a lockfile ref back into tag/branch: stable semver is a tag, everything else is a branch. BestRef picks the single ref for serialization: tag wins over branch. These bridge the CLI's internal Tag/Branch resolver model into the single ref field for lockfile read/write. --- go/pkg/lockfile/ref.go | 24 +++++++++++++++++ go/pkg/lockfile/ref_test.go | 53 +++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 go/pkg/lockfile/ref.go create mode 100644 go/pkg/lockfile/ref_test.go diff --git a/go/pkg/lockfile/ref.go b/go/pkg/lockfile/ref.go new file mode 100644 index 0000000..c327311 --- /dev/null +++ b/go/pkg/lockfile/ref.go @@ -0,0 +1,24 @@ +package lockfile + +// SplitRef classifies a lockfile ref value back into its tag/branch +// constituents. If ref parses as a stable semver (via ParseSemVer + +// IsStable), it's returned as tag; otherwise it's returned as branch. +// An empty ref yields both empty. +func SplitRef(ref string) (tag, branch string) { + if ref == "" { + return "", "" + } + if sv, ok := ParseSemVer(ref); ok && sv.IsStable() { + return ref, "" + } + return "", ref +} + +// BestRef picks the single ref value for lockfile serialization. +// Tag wins if non-empty, else branch. Both empty yields empty. +func BestRef(tag, branch string) string { + if tag != "" { + return tag + } + return branch +} diff --git a/go/pkg/lockfile/ref_test.go b/go/pkg/lockfile/ref_test.go new file mode 100644 index 0000000..2a76e56 --- /dev/null +++ b/go/pkg/lockfile/ref_test.go @@ -0,0 +1,53 @@ +package lockfile + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSplitRef(t *testing.T) { + cases := []struct { + ref string + tag string + branch string + }{ + {"", "", ""}, + {"v4.3.1", "v4.3.1", ""}, + {"v1.0.0", "v1.0.0", ""}, + {"1.2.3", "1.2.3", ""}, + {"v4", "v4", ""}, // major-only is still stable semver + {"v4.2", "v4.2", ""}, // major.minor is still stable semver + {"v1.0.0-beta.1", "", "v1.0.0-beta.1"}, // pre-release → branch + {"main", "", "main"}, + {"trunk", "", "trunk"}, + {"release/v4", "", "release/v4"}, + {"my-feature", "", "my-feature"}, + } + for _, tc := range cases { + t.Run(tc.ref, func(t *testing.T) { + tag, branch := SplitRef(tc.ref) + assert.Equal(t, tc.tag, tag, "tag") + assert.Equal(t, tc.branch, branch, "branch") + }) + } +} + +func TestBestRef(t *testing.T) { + cases := []struct { + tag string + branch string + want string + }{ + {"v4.3.1", "main", "v4.3.1"}, + {"v4", "main", "v4"}, + {"", "main", "main"}, + {"", "", ""}, + {"v1.0.0", "", "v1.0.0"}, + } + for _, tc := range cases { + t.Run(tc.tag+"/"+tc.branch, func(t *testing.T) { + assert.Equal(t, tc.want, BestRef(tc.tag, tc.branch)) + }) + } +} From eff8f62231d478b40d46a13b6bfeeb4edcf077ca Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Mon, 22 Jun 2026 19:50:21 -0500 Subject: [PATCH 04/15] lockfile: simplify pin key to owner/repo@ref, bump to v0.0.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the :algo-hex digest suffix from pin keys. The commit hash now lives only in Action.Commit — the pin key is purely a human-readable identifier (owner/repo@ref). This eliminates the triple-SHA noise in lockfiles (the hash appeared in the @ref portion for SHA-pinned deps, in the :algo-hex suffix, and in the commit: field). Schema changes: - Version bumped to v0.0.2 - Pin pattern simplified to ^[^/@:]+/[^/@:]+@[^:]+$ - Pin struct drops Algo/Hex fields - Commit-mismatch cross-check between key and body removed - ParsePin now handles owner/repo@ref directly --- .../lockfile/internal/cmd/genschema/main.go | 4 +- go/pkg/lockfile/lockfile.go | 141 ++++++--- go/pkg/lockfile/lockfile_test.go | 293 ++++++++++++------ go/pkg/lockfile/pin.go | 68 ++-- go/pkg/lockfile/pin_test.go | 52 ++-- go/pkg/lockfile/schema.go | 2 +- go/pkg/lockfile/schema_gen.go | 2 +- go/pkg/lockfile/schema_test.go | 76 ++--- go/pkg/lockfile/uses.go | 2 +- ...kfile-v0.0.1.json => lockfile-v0.0.2.json} | 10 +- 10 files changed, 391 insertions(+), 259 deletions(-) rename schema/{lockfile-v0.0.1.json => lockfile-v0.0.2.json} (89%) diff --git a/go/pkg/lockfile/internal/cmd/genschema/main.go b/go/pkg/lockfile/internal/cmd/genschema/main.go index e0517f5..0127d35 100644 --- a/go/pkg/lockfile/internal/cmd/genschema/main.go +++ b/go/pkg/lockfile/internal/cmd/genschema/main.go @@ -9,7 +9,7 @@ import ( ) func main() { - schema, err := os.ReadFile("../../../schema/lockfile-v0.0.1.json") + schema, err := os.ReadFile("../../../schema/lockfile-v0.0.2.json") if err != nil { panic(err) } @@ -19,7 +19,7 @@ func main() { fmt.Fprintln(&out) fmt.Fprintln(&out, "package lockfile") fmt.Fprintln(&out) - fmt.Fprintf(&out, "const schemaV001 = %s\n", strconv.Quote(string(schema))) + fmt.Fprintf(&out, "const schemaV002 = %s\n", strconv.Quote(string(schema))) formatted, err := format.Source(out.Bytes()) if err != nil { diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go index c2a5de6..0e2e83c 100644 --- a/go/pkg/lockfile/lockfile.go +++ b/go/pkg/lockfile/lockfile.go @@ -81,7 +81,7 @@ func newYAMLParseError(err error) *ParseError { } // Version is the only supported lockfile schema version. -const Version = "v0.0.1" +const Version = "v0.0.2" // Path is the canonical repo-relative location of the dependency lockfile. const Path = ".github/workflows/actions.lock" @@ -89,19 +89,18 @@ const Path = ".github/workflows/actions.lock" // File is the parsed lockfile shape. // // # .github/workflows/actions.lock -// version: v0.0.1 +// version: v0.0.2 // workflows: // .github/workflows/deploy.yml: -// - actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 +// - actions/checkout@v6 // dependencies: -// actions/checkout@v4.3.1:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: -// tag: v4.3.1 -// branch: main +// actions/checkout@v4.3.1: +// ref: v4.3.1 // commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 // owner_id: 44036562 // repo_id: 197814629 // uses: -// - actions/cache@v4.0.0:sha1-... +// - actions/cache@v4.0.0 // // The Go field `Dependencies` maps to the YAML key `dependencies:` — the // lockfile's deduplicated action DAG. Each entry's `uses:` list names the @@ -109,11 +108,11 @@ const Path = ".github/workflows/actions.lock" // Workflow entries hold the full transitive closure as a flat list of pin // keys for cold readability. type File struct { - // Version is the lockfile schema version string (e.g. "v0.0.1"). It is + // Version is the lockfile schema version string (e.g. "v0.0.2"). It is // always equal to the [Version] constant for files Parse accepts. Version string `yaml:"version"` - // Dependencies maps each canonical pin key (OWNER/REPO@REF:ALGO-HEX) to + // Dependencies maps each canonical pin key (OWNER/REPO@REF) to // the resolved [Action] metadata for that pin. The map is deduplicated: // multiple workflows that share an action produce a single entry here. // Use [File.LookupWorkflow] to find the pin keys for a specific workflow, @@ -123,7 +122,7 @@ type File struct { // Workflows maps each repo-relative workflow file path (e.g. // ".github/workflows/release.yml") to the flat, transitive list of // canonical pin keys that workflow depends on. Pin keys are in - // OWNER/REPO@REF:ALGO-HEX form and serve as lookup keys into + // OWNER/REPO@REF form and serve as lookup keys into // Dependencies. Prefer [File.LookupWorkflow] over indexing this // map directly. Workflows map[string][]string `yaml:"workflows"` @@ -368,7 +367,13 @@ func Parse(contents []byte, paths ...string) (File, error) { } return File{}, pe } - canonicalizeWorkflowDependencies(&f) + if wfPath, _, err := canonicalizeWorkflowDependencies(&f); err != nil { + pe := &ParseError{Msg: err.Error(), err: err} + if l, c, ok := f.KeyPosition("workflows", wfPath); ok { + pe.Line, pe.Column = l, c + } + return File{}, pe + } if cycle, err := detectUsesCycle(&f); err != nil { pe := &ParseError{Msg: err.Error()} if l, c, ok := f.KeyPosition("dependencies", cycle); ok { @@ -596,6 +601,15 @@ func validateKnownFields(f *File, paths []string) *ParseError { if pe := rejectZeroValues(action, pinKey.Value); pe != nil { return pe } + // Reject entries where the pin key's ref disagrees with the body's + // ref field — a mismatch means the lockfile was hand-edited + // inconsistently and cannot be trusted. + if pe := rejectKeyRefMismatch(pinKey, action); pe != nil { + return pe + } + if pe := rejectFullSHACommitMismatch(pinKey, action); pe != nil { + return pe + } } return nil } @@ -672,42 +686,93 @@ func rejectZeroValues(action *yaml.Node, dep string) *ParseError { return nil } +// rejectKeyRefMismatch returns a ParseError if the dependency key parses +// as a valid pin and the body's "ref:" field disagrees with the key's ref +// component. A mismatch indicates an inconsistent hand-edit that could +// silently resolve a different ref than the key advertises. +func rejectKeyRefMismatch(pinKey, action *yaml.Node) *ParseError { + pin, ok := ParsePin(pinKey.Value) + if !ok { + return nil + } + for j := 0; j+1 < len(action.Content); j += 2 { + key := action.Content[j] + val := action.Content[j+1] + if key.Value == "ref" && val.Value != "" && val.Value != pin.Ref { + return &ParseError{ + Line: val.Line, + Column: val.Column, + Msg: fmt.Sprintf("action body ref %q does not match pin key ref %q for dependency %q", val.Value, pin.Ref, pinKey.Value), + } + } + } + return nil +} + +// rejectFullSHACommitMismatch returns a ParseError when the pin key's ref is a +// full commit SHA (40 or 64 hex chars) and the body's "commit:" field encodes a +// different digest. Full-SHA refs are immutable — the commit field must agree +// with the ref, otherwise the entry is internally inconsistent. +func rejectFullSHACommitMismatch(pinKey, action *yaml.Node) *ParseError { + pin, ok := ParsePin(pinKey.Value) + if !ok { + return nil + } + if !IsFullSha(pin.Ref) { + return nil + } + for j := 0; j+1 < len(action.Content); j += 2 { + key := action.Content[j] + val := action.Content[j+1] + if key.Value == "commit" && val.Value != "" { + // Extract the hex portion after the algo prefix (e.g. "sha1-"). + dashIdx := strings.IndexByte(val.Value, '-') + if dashIdx <= 0 || dashIdx == len(val.Value)-1 { + return nil // malformed commit caught elsewhere + } + commitHex := strings.ToLower(val.Value[dashIdx+1:]) + if strings.ToLower(pin.Ref) != commitHex { + return &ParseError{ + Line: val.Line, + Column: val.Column, + Msg: fmt.Sprintf("pin key ref is a full SHA (%s) but commit digest %q does not match for dependency %q", ShortSHA(pin.Ref), val.Value, pinKey.Value), + } + } + } + } + return nil +} + // canonicalizeActions rewrites the Dependencies map so every key is the // canonical form of its pin (Pin.String()). A conflict between two // different source casings of the same pin is a parse error — the file // would be ambiguous about which Action metadata applies. On conflict it // returns the offending source key so callers can locate it in the YAML tree. +// +// Dependency keys and Uses entries that do not parse as valid v0.0.2 pin +// strings (OWNER/REPO@REF) are rejected — this catches legacy +// digest-suffixed keys (owner/repo@ref:sha1-...) that are invalid under +// the current schema. func canonicalizeActions(f *File) (string, error) { if len(f.Dependencies) == 0 { return "", nil } out := make(map[string]Action, len(f.Dependencies)) for key, action := range f.Dependencies { - 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, - ) - } + pin, ok := ParsePin(key) + if !ok { + return key, fmt.Errorf("dependency key %q is not a valid pin (expected OWNER/REPO@REF)", key) } + canonical := pin.String() // Canonicalize Uses entries too so cross-references resolve. if len(action.Uses) > 0 { canonUses := make([]string, len(action.Uses)) for i, u := range action.Uses { - if pin, ok := ParsePin(u); ok { - canonUses[i] = pin.String() - } else { - canonUses[i] = u + uPin, uOk := ParsePin(u) + if !uOk { + return key, fmt.Errorf("uses entry %q in dependency %q is not a valid pin (expected OWNER/REPO@REF)", u, key) } + canonUses[i] = uPin.String() } action.Uses = canonUses } @@ -741,23 +806,25 @@ func equalAction(a, b Action) bool { // canonicalizeWorkflowDependencies rewrites every workflow's pin list to // canonical pin strings (Pin.String()) so lookups into the Dependencies map are -// casing-agnostic. Unparseable entries are preserved verbatim for downstream -// diagnostics to flag. -func canonicalizeWorkflowDependencies(f *File) { +// casing-agnostic. Entries that do not parse as valid v0.0.2 pin strings are +// rejected — legacy digest-suffixed pins and other malformed values are not +// preserved. +func canonicalizeWorkflowDependencies(f *File) (string, string, error) { for path, deps := range f.Workflows { if len(deps) == 0 { continue } canonicalized := make([]string, len(deps)) for i, dep := range deps { - if pin, ok := ParsePin(dep); ok { - canonicalized[i] = pin.String() - } else { - canonicalized[i] = dep + pin, ok := ParsePin(dep) + if !ok { + return path, dep, fmt.Errorf("workflow %q dependency %q is not a valid pin (expected OWNER/REPO@REF)", path, dep) } + canonicalized[i] = pin.String() } f.Workflows[path] = canonicalized } + return "", "", nil } // schemaVersionRE matches "vMAJOR.MINOR.PATCH" with an optional leading "v" diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go index 7d4e727..ca9b21d 100644 --- a/go/pkg/lockfile/lockfile_test.go +++ b/go/pkg/lockfile/lockfile_test.go @@ -49,7 +49,7 @@ func TestParse_WrongShapeReportsLine(t *testing.T) { // pin keys fails yaml type-decoding. Parse must surface the failing line as // structured data (ParseError.Line) and strip yaml.v3's "yaml:" prefix from // the reason so consumers don't misattribute the position to their own file. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: {} workflows: .github/workflows/ci.yml: @@ -83,12 +83,12 @@ dependencies: {} func TestParse_DuplicateActionKeyReportsPosition(t *testing.T) { // The conflicting key must be located in the source tree so the position // points at a real offending dependency entry. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: - actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8: + actions/checkout@v6: owner_id: 1234 repo_id: 5678 - Actions/Checkout@v6:SHA1-8E8C483DB84B4BEE98B60C0593521ED34D9990E8: + Actions/Checkout@v6: owner_id: 9999 repo_id: 1 ` @@ -104,7 +104,7 @@ dependencies: func TestParse_PositionLookup(t *testing.T) { // The retained node tree is exposed for consumer diagnostics via // Position/KeyPosition. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: {} workflows: .github/workflows/ci.yml: [] @@ -127,17 +127,17 @@ workflows: } func TestParse_CanonicalizesActionKeys(t *testing.T) { - const canonical = "actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8" - yaml := `version: v0.0.1 + const canonical = "actions/checkout@v6" + yaml := `version: v0.0.2 dependencies: - Actions/Checkout@v6:SHA1-8E8C483DB84B4BEE98B60C0593521ED34D9990E8: - ref: v4 + Actions/Checkout@v6: + ref: v6 commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 workflows: .github/workflows/ci.yml: - - Actions/Checkout@v6:SHA1-8E8C483DB84B4BEE98B60C0593521ED34D9990E8 + - Actions/Checkout@v6 ` f, err := Parse([]byte(yaml)) require.NoError(t, err) @@ -158,15 +158,15 @@ workflows: func TestParse_ConflictingActionKeyCasings(t *testing.T) { // Two source-casings of the same pin with differing metadata is // ambiguous and must be rejected. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: - actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8: - ref: v4 + actions/checkout@v6: + ref: v6 commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 - Actions/Checkout@v6:SHA1-8E8C483DB84B4BEE98B60C0593521ED34D9990E8: - ref: v4 + Actions/Checkout@v6: + ref: v6 commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 9999 repo_id: 1 @@ -178,15 +178,15 @@ dependencies: func TestParse_DuplicateActionKeyCasingsSameMetadataOK(t *testing.T) { // Same metadata on two casings collapses to one canonical entry. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: - actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8: - ref: v4 + actions/checkout@v6: + ref: v6 commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 - Actions/Checkout@v6:SHA1-8E8C483DB84B4BEE98B60C0593521ED34D9990E8: - ref: v4 + Actions/Checkout@v6: + ref: v6 commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 @@ -196,10 +196,9 @@ dependencies: assert.Len(t, f.Dependencies, 1) } -func TestParse_UnparseableActionKeyPreserved(t *testing.T) { - // Garbage keys are preserved verbatim so structural diagnostics can - // surface them; Parse itself is not the validator. - yaml := `version: v0.0.1 +func TestParse_UnparseableActionKeyRejected(t *testing.T) { + // v0.0.2 rejects dependency keys that do not parse as valid pins. + input := `version: v0.0.2 dependencies: "not a pin": ref: v4 @@ -207,15 +206,14 @@ dependencies: owner_id: 1 repo_id: 2 ` - f, err := Parse([]byte(yaml)) - require.NoError(t, err) - _, ok := f.Dependencies["not a pin"] - assert.True(t, ok) + _, err := Parse([]byte(input)) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a valid pin") } func TestParse_WorkflowPathKeyNotCanonicalized(t *testing.T) { // File paths are case-sensitive on Linux; do not normalize them. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: {} workflows: .github/workflows/CI.yml: [] @@ -227,14 +225,14 @@ workflows: } func TestParse_RefRoundTrip(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: - actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8: + actions/checkout@v6: ref: v6 commit: sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8 owner_id: 1234 repo_id: 5678 - actions/internal@trunk:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: + actions/internal@trunk: ref: trunk commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa owner_id: 1 @@ -244,10 +242,10 @@ workflows: {} f, err := Parse([]byte(yaml)) require.NoError(t, err) - withTag := f.Dependencies["actions/checkout@v6:sha1-8e8c483db84b4bee98b60c0593521ed34d9990e8"] + withTag := f.Dependencies["actions/checkout@v6"] assert.Equal(t, "v6", withTag.Ref) - branchRef := f.Dependencies["actions/internal@trunk:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"] + branchRef := f.Dependencies["actions/internal@trunk"] assert.Equal(t, "trunk", branchRef.Ref) } @@ -265,9 +263,9 @@ 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 + yaml := `version: v0.0.2 dependencies: - actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + actions/checkout@v4: ref: v4 commit: "" owner_id: 1 @@ -297,8 +295,8 @@ func TestParse_CommitInvalidFormatRejected(t *testing.T) { } 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" + + y := "version: v0.0.2\ndependencies:\n" + + " actions/checkout@v4:\n" + " ref: v4\n" + " commit: " + tc.commit + "\n" + " owner_id: 1\n" + @@ -317,8 +315,7 @@ func TestParse_CommitValidFormatsAccepted(t *testing.T) { } 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" + + y := "version: v0.0.2\ndependencies:\n actions/checkout@v4:\n" + " ref: v4\n commit: " + commit + "\n owner_id: 1\n repo_id: 1\n" _, err := Parse([]byte(y)) require.NoError(t, err) @@ -326,38 +323,6 @@ 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: - ref: v4 - 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: - ref: v4 - commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 - owner_id: 1 - repo_id: 1 -` - _, 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 @@ -376,7 +341,7 @@ func TestParse_WorkflowPathTraversalRejected(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - y := "version: v0.0.1\ndependencies: {}\nworkflows:\n " + tc.key + ": []\n" + y := "version: v0.0.2\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") @@ -392,7 +357,7 @@ func TestParse_WorkflowPathLegitimateKeysAccepted(t *testing.T) { } for _, key := range legit { t.Run(key, func(t *testing.T) { - y := "version: v0.0.1\ndependencies: {}\nworkflows:\n " + key + ": []\n" + y := "version: v0.0.2\ndependencies: {}\nworkflows:\n " + key + ": []\n" _, err := Parse([]byte(y)) require.NoError(t, err) }) @@ -421,8 +386,8 @@ func TestParse_RefInjectionCharsRejected(t *testing.T) { } 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" + + y := "version: v0.0.2\ndependencies:\n" + + " actions/checkout@v4:\n" + " ref: " + tc.yamlRef + "\n" + " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + " owner_id: 1\n repo_id: 1\n" @@ -459,22 +424,22 @@ func TestParse_ExactMaxSizeAccepted(t *testing.T) { } func TestParse_UsesCycleRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: - actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: - ref: v4 + actions/a@v1: + ref: v1 commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 owner_id: 1 repo_id: 1 uses: - - actions/b@v1:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - actions/b@v1:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: - ref: v4 + - actions/b@v1 + actions/b@v1: + ref: v1 commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa owner_id: 2 repo_id: 2 uses: - - actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + - actions/a@v1 ` _, err := Parse([]byte(yaml)) require.Error(t, err) @@ -482,15 +447,15 @@ dependencies: } func TestParse_UsesSelfCycleRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: - actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: - ref: v4 + actions/a@v1: + ref: v1 commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 owner_id: 1 repo_id: 1 uses: - - actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + - actions/a@v1 ` _, err := Parse([]byte(yaml)) require.Error(t, err) @@ -499,17 +464,17 @@ dependencies: func TestParse_UsesAcyclicAccepted(t *testing.T) { // A valid DAG (A uses B, B has no uses) must parse successfully. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: - actions/a@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: - ref: v4 + actions/a@v1: + ref: v1 commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 owner_id: 1 repo_id: 1 uses: - - actions/b@v1:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa - actions/b@v1:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: - ref: v4 + - actions/b@v1 + actions/b@v1: + ref: v1 commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa owner_id: 2 repo_id: 2 @@ -517,3 +482,145 @@ dependencies: _, err := Parse([]byte(yaml)) require.NoError(t, err) } + +func TestParse_KeyBodyRefMismatchRejected(t *testing.T) { + // The pin key says "@v4" but the body says ref: v3 — this must be + // rejected because the mismatch means the lockfile was hand-edited + // inconsistently and cannot be trusted. + yaml := `version: v0.0.2 +dependencies: + actions/checkout@v4: + ref: v3 + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 1 + repo_id: 1 +` + _, err := Parse([]byte(yaml)) + require.Error(t, err) + assert.Contains(t, err.Error(), "does not match pin key ref") + assert.Contains(t, err.Error(), `"v3"`) + assert.Contains(t, err.Error(), `"v4"`) +} + +func TestParse_KeyBodyRefMatchAccepted(t *testing.T) { + // Key and body ref agree — must parse successfully. + yaml := `version: v0.0.2 +dependencies: + actions/checkout@v4: + ref: v4 + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 1 + repo_id: 1 +` + _, err := Parse([]byte(yaml)) + require.NoError(t, err) +} + +func TestParse_FullSHARefCommitMismatchRejected(t *testing.T) { + // Pin key ref is a full SHA but commit encodes a different digest. + input := `version: v0.0.2 +dependencies: + actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683: + ref: 11bd71901bbe5b1630ceea73d27597364c9af683 + commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + owner_id: 1 + repo_id: 1 +` + _, err := Parse([]byte(input)) + require.Error(t, err) + assert.Contains(t, err.Error(), "commit digest") + assert.Contains(t, err.Error(), "does not match") +} + +func TestParse_FullSHARefCommitMatchAccepted(t *testing.T) { + // Pin key ref is a full SHA and commit agrees — must parse. + input := `version: v0.0.2 +dependencies: + actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683: + ref: 11bd71901bbe5b1630ceea73d27597364c9af683 + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 1 + repo_id: 1 +` + _, err := Parse([]byte(input)) + require.NoError(t, err) +} + +func TestParse_SymbolicRefDifferentCommitAccepted(t *testing.T) { + // Symbolic ref (not a full SHA) — commit can be anything valid. + input := `version: v0.0.2 +dependencies: + actions/checkout@v4: + ref: v4 + commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + owner_id: 1 + repo_id: 1 +` + _, err := Parse([]byte(input)) + require.NoError(t, err) +} + +func TestParse_ColonInRefRejected(t *testing.T) { + // Colons in refs are rejected by isValidRef (matching schema pin regex). + input := `version: v0.0.2 +dependencies: + actions/checkout@v4: + ref: "v4:foo" + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 1 + repo_id: 1 +` + _, err := Parse([]byte(input)) + require.Error(t, err) + assert.Contains(t, err.Error(), "unsafe characters") +} + +func TestParse_LegacyDigestSuffixedKeyRejected(t *testing.T) { + // Legacy v0.0.1 format used "owner/repo@ref:sha1-" as keys. + // v0.0.2 rejects these because the colon makes the ref invalid. + input := "version: v0.0.2\ndependencies:\n" + + " \"actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683\":\n" + + " ref: v4\n" + + " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + + " owner_id: 1\n" + + " repo_id: 1\n" + _, err := Parse([]byte(input)) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a valid pin") +} + +func TestParse_LegacyDigestSuffixedUsesEntryRejected(t *testing.T) { + // Valid dependency key but its uses entry is a legacy digest-suffixed pin. + input := "version: v0.0.2\ndependencies:\n" + + " actions/cache@v4:\n" + + " ref: v4\n" + + " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + + " owner_id: 1\n" + + " repo_id: 1\n" + + " actions/composite@v1:\n" + + " ref: v1\n" + + " commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + + " owner_id: 2\n" + + " repo_id: 3\n" + + " uses:\n" + + " - \"actions/cache@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683\"\n" + _, err := Parse([]byte(input)) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a valid pin") +} + +func TestParse_LegacyDigestSuffixedWorkflowDepRejected(t *testing.T) { + // Workflow dependency entries with legacy format are rejected. + input := "version: v0.0.2\ndependencies:\n" + + " actions/checkout@v4:\n" + + " ref: v4\n" + + " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + + " owner_id: 1\n" + + " repo_id: 1\n" + + "workflows:\n" + + " .github/workflows/ci.yml:\n" + + " - \"actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683\"\n" + _, err := Parse([]byte(input)) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a valid pin") +} diff --git a/go/pkg/lockfile/pin.go b/go/pkg/lockfile/pin.go index 9b67c77..72d8011 100644 --- a/go/pkg/lockfile/pin.go +++ b/go/pkg/lockfile/pin.go @@ -10,10 +10,10 @@ import ( // Pin holds the parsed components of a dependency pin key. // -// "OWNER/REPO@REF:ALGO-HEX" +// "OWNER/REPO@REF" // -// The pin identifies a downloaded action tarball at repo+SHA granularity — -// matching the runner, which downloads `owner/repo@sha` once per ref and +// The pin identifies a downloaded action tarball at repo+ref granularity — +// matching the runner, which downloads `owner/repo@ref` once per ref and // reuses the tree for any sub-action path. Sub-action paths (e.g. the // `save` in `actions/cache/save@v4`) are graph traversal details, not pin // identity, and do not appear in this serialized form. @@ -22,13 +22,11 @@ type Pin struct { Owner string // "actions" Repo string // "checkout" Ref string // "v4" - Algo string // "sha1" - Hex string // "34e114876b0b11c390a56381ad16ebd13914f8d5" } // Canonical returns a copy of p with all case-insensitive components -// (owner, repo, algo, hex) normalized to lowercase. Ref preserves source -// casing — git refs are case-sensitive. +// (owner, repo) normalized to lowercase. Ref preserves source casing — +// git refs are case-sensitive. // // This is the single normalization point for the lockfile pin grammar: // String, IndexKey, and ParsePin all funnel through it, so callers never @@ -36,59 +34,52 @@ type Pin struct { func (p Pin) Canonical() Pin { p.Owner = strings.ToLower(p.Owner) p.Repo = strings.ToLower(p.Repo) - p.Algo = strings.ToLower(p.Algo) - p.Hex = strings.ToLower(p.Hex) p.NWO = p.Owner + "/" + p.Repo return p } -// String returns the canonical pin form: "OWNER/REPO@REF:ALGO-HEX". +// String returns the canonical pin form: "OWNER/REPO@REF". // This doubles as the actions-map key in the lockfile. func (p Pin) String() string { c := p.Canonical() - return c.NWO + "@" + c.Ref + ":" + c.Algo + "-" + c.Hex + return c.NWO + "@" + c.Ref } -// IndexKey returns the normalized lookup key for this pin without the digest: -// "OWNER/REPO@REF". +// IndexKey returns the normalized lookup key for this pin: "OWNER/REPO@REF". +// Identical to String() in v0.0.2 — retained for API compatibility. func (p Pin) IndexKey() string { - c := p.Canonical() - return c.NWO + "@" + c.Ref + return p.String() } -// IndexKey builds the normalized lookup key for a dependency entry without -// the digest: "OWNER/REPO@REF". +// IndexKey builds the normalized lookup key for a dependency entry: +// "OWNER/REPO@REF". func IndexKey(owner, repo, ref string) string { return Pin{Owner: owner, Repo: repo, Ref: ref}.IndexKey() } // ParsePin parses a pin string of the canonical form: // -// "OWNER/REPO@REF:ALGO-HEX" +// "OWNER/REPO@REF" // // Returns ok=false for any input that does not match — including all of: // - Missing "@" separator between repo and ref -// - Missing ":" separator between ref and digest -// - A sub-action path in the repo portion (e.g. "owner/repo/sub@ref:...") +// - A sub-action path in the repo portion (e.g. "owner/repo/sub@ref") // — the lockfile grammar is strictly repo-scoped, matching the runner's // tarball download identity -// - An unknown or unsupported digest algorithm (only "sha1" and "sha256" -// are recognized) -// - A hex digest that is the wrong length or contains non-hex characters // -// On success, all case-insensitive components (owner, repo, algo, hex) are -// normalized to lowercase. Ref preserves source casing — git refs are -// case-sensitive. The returned Pin is always in canonical form. +// On success, owner and repo are normalized to lowercase. Ref preserves +// source casing — git refs are case-sensitive. The returned Pin is always +// in canonical form. func ParsePin(s string) (Pin, bool) { atIdx := strings.IndexByte(s, '@') if atIdx <= 0 || atIdx == len(s)-1 { return Pin{}, false } repoPath := s[:atIdx] - refHash := s[atIdx+1:] + ref := s[atIdx+1:] // Sub-action paths are not part of the lockfile pin grammar: the runner - // downloads at repo+sha granularity. Reject any extra slashes in the + // downloads at repo+ref granularity. Reject any extra slashes in the // repo portion so hand-edited lockfiles don't drift into a path-bearing // format. if strings.Count(repoPath, "/") != 1 { @@ -99,14 +90,6 @@ func ParsePin(s string) (Pin, bool) { return Pin{}, false } - colonIdx := strings.LastIndexByte(refHash, ':') - if colonIdx <= 0 || colonIdx == len(refHash)-1 { - return Pin{}, false - } - ref := refHash[:colonIdx] - if strings.ContainsRune(ref, ':') { - return Pin{}, false - } // 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 @@ -115,24 +98,11 @@ func ParsePin(s string) (Pin, bool) { if !isValidRef(ref) { return Pin{}, false } - hashSpec := refHash[colonIdx+1:] - - dashIdx := strings.IndexByte(hashSpec, '-') - if dashIdx <= 0 || dashIdx == len(hashSpec)-1 { - return Pin{}, false - } - algo := strings.ToLower(hashSpec[:dashIdx]) - hexDigest := strings.ToLower(hashSpec[dashIdx+1:]) - if !isValidDigest(algo, hexDigest) { - return Pin{}, false - } return Pin{ Owner: owner, Repo: repo, Ref: ref, - Algo: algo, - Hex: hexDigest, }.Canonical(), true } diff --git a/go/pkg/lockfile/pin_test.go b/go/pkg/lockfile/pin_test.go index be51016..d791924 100644 --- a/go/pkg/lockfile/pin_test.go +++ b/go/pkg/lockfile/pin_test.go @@ -15,42 +15,35 @@ func TestParsePin(t *testing.T) { }{ { name: "owner repo", - entry: "actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683", + entry: "actions/checkout@v4", want: Pin{ NWO: "actions/checkout", Owner: "actions", Repo: "checkout", Ref: "v4", - Algo: "sha1", - Hex: "11bd71901bbe5b1630ceea73d27597364c9af683", }, }, { - name: "sha256", - entry: "actions/checkout@v4:sha256-a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + name: "full SHA ref", + entry: "actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683", want: Pin{ NWO: "actions/checkout", Owner: "actions", Repo: "checkout", - Ref: "v4", - Algo: "sha256", - Hex: "a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2", + Ref: "11bd71901bbe5b1630ceea73d27597364c9af683", }, }, { // Monorepo sub-action tags (e.g. attest-build-provenance's // predicate/) embed an '@' in the ref, producing a double-'@' - // key. The first '@' bounds the NWO and the last ':' bounds the - // digest, so the ref survives intact. + // key. The first '@' bounds the NWO and the rest is the ref. name: "ref containing at (monorepo sub-action tag)", - entry: "actions/attest-build-provenance@predicate@1.1.4:sha1-36fa7d009e22618ca7cd599486979b8150596c74", + entry: "actions/attest-build-provenance@predicate@1.1.4", want: Pin{ NWO: "actions/attest-build-provenance", Owner: "actions", Repo: "attest-build-provenance", Ref: "predicate@1.1.4", - Algo: "sha1", - Hex: "36fa7d009e22618ca7cd599486979b8150596c74", }, }, } @@ -72,17 +65,13 @@ func TestParsePin_Invalid(t *testing.T) { entry string }{ {"empty", ""}, - {"missing at", "actions/checkout:sha1-abc123"}, - {"missing colon", "actions/checkout@v4sha1-abc123"}, - {"missing algo dash", "actions/checkout@v4:sha1abc123"}, - {"ref contains colon", "actions/checkout@refs/tags/v1:prod:sha1-11bd71901bbe5b1630ceea73d27597364c9af683"}, - {"unsupported algo", "actions/checkout@v4:md5-098f6bcd4621d373cade4e832627b4f6"}, - {"sha1 too short", "actions/checkout@v4:sha1-abc123"}, - {"empty owner", "/checkout@v4:sha1-abc123"}, - {"empty repo", "actions/@v4:sha1-abc123"}, - {"sub-action path rejected", "actions/cache/save@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683"}, - {"deep sub-action path rejected", "owner/repo/a/b@v1:sha1-11bd71901bbe5b1630ceea73d27597364c9af683"}, - {"with metadata suffix (pins are pure identity)", "actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683;owner_id=1234"}, + {"missing at", "actions/checkout"}, + {"empty owner", "/@v4"}, + {"empty repo", "actions/@v4"}, + {"sub-action path rejected", "actions/cache/save@v4"}, + {"deep sub-action path rejected", "owner/repo/a/b@v1"}, + {"colon in ref rejected", "actions/checkout@v4:foo"}, + {"colon only ref rejected", "actions/checkout@:"}, } for _, tt := range tests { @@ -90,7 +79,6 @@ func TestParsePin_Invalid(t *testing.T) { got, ok := ParsePin(tt.entry) assert.False(t, ok) assert.Equal(t, Pin{}, got) - }) } } @@ -138,13 +126,13 @@ func TestParsePin_RefInjectionRejected(t *testing.T) { 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"}, + {"space in ref", "actions/checkout@v4 evil"}, + {"tab in ref", "actions/checkout@v4\tevil"}, + {"double-quote in ref", "actions/checkout@v4\"evil"}, + {"single-quote in ref", "actions/checkout@v4'evil"}, + {"backtick in ref", "actions/checkout@v4`evil"}, + {"backslash in ref", "actions/checkout@v4\\evil"}, + {"dotdot in ref", "actions/checkout@../evil"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/go/pkg/lockfile/schema.go b/go/pkg/lockfile/schema.go index 73a5726..5c7e448 100644 --- a/go/pkg/lockfile/schema.go +++ b/go/pkg/lockfile/schema.go @@ -11,5 +11,5 @@ package lockfile // not the dependencies map keys, so schema validation alone won't enforce that // every dependency key is a canonical pin either. func Schema() string { - return schemaV001 + return schemaV002 } diff --git a/go/pkg/lockfile/schema_gen.go b/go/pkg/lockfile/schema_gen.go index 8b5d31a..e438010 100644 --- a/go/pkg/lockfile/schema_gen.go +++ b/go/pkg/lockfile/schema_gen.go @@ -2,4 +2,4 @@ package lockfile -const schemaV001 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.1.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.1 is supported.\",\n \"const\": \"v0.0.1\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF:ALGO-HEX (e.g. actions/checkout@v4:sha1-34e1...).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@:]+/[^/@:]+@[^:]+:(sha1-[0-9a-f]{40}|sha256-[0-9a-f]{64})$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"ref\", \"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"ref\": {\n \"description\": \"The git ref the commit was resolved from. Required: every dep that passes impostor checks has a resolvable ref. The CLI picks the best ref with priority: full semver tag > any tag > branch (protected > default > release/v* > any). The parser enforces presence; priority ordering is the CLI's concern.\",\n \"type\": \"string\",\n \"minLength\": 1\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org). Pinned because names can be deleted and re-registered by someone else; the ID cannot, so it ties the pin to the original owner.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository. Pinned because a repo can be renamed or deleted and the name reclaimed; the ID detects that the repo behind the name has changed.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys, so the full dependency graph stays pinned and verifiable end to end. Required for composite actions (which can call other actions); absent for leaf actions that have no dependencies of their own.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" +const schemaV002 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.2.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.2 is supported.\",\n \"const\": \"v0.0.2\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF (e.g. actions/checkout@v4).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@:]+/[^/@:]+@[^:]+$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"ref\", \"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"ref\": {\n \"description\": \"The git ref the commit was resolved from. Required: every dep that passes impostor checks has a resolvable ref. The CLI picks the best ref with priority: full semver tag > any tag > branch (protected > default > release/v* > any). The parser enforces presence; priority ordering is the CLI's concern.\",\n \"type\": \"string\",\n \"minLength\": 1\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org). Pinned because names can be deleted and re-registered by someone else; the ID cannot, so it ties the pin to the original owner.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository. Pinned because a repo can be renamed or deleted and the name reclaimed; the ID detects that the repo behind the name has changed.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys, so the full dependency graph stays pinned and verifiable end to end. Required for composite actions (which can call other actions); absent for leaf actions that have no dependencies of their own.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" diff --git a/go/pkg/lockfile/schema_test.go b/go/pkg/lockfile/schema_test.go index 0e178c0..6db456b 100644 --- a/go/pkg/lockfile/schema_test.go +++ b/go/pkg/lockfile/schema_test.go @@ -11,7 +11,7 @@ import ( ) func TestSchema_EmbeddedMatchesRootInvariant(t *testing.T) { - rootSchema, err := os.ReadFile("../../../schema/lockfile-v0.0.1.json") + rootSchema, err := os.ReadFile("../../../schema/lockfile-v0.0.2.json") if errors.Is(err, os.ErrNotExist) { t.Skip("root schema invariant is not present in this module checkout") } @@ -64,7 +64,7 @@ func TestSchema_EmbeddedMatchesEnforcement(t *testing.T) { } func TestParse_UnknownTopLevelFieldRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: {} typo_section: {} ` @@ -79,9 +79,9 @@ typo_section: {} } func TestParse_UnknownActionFieldRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + actions/checkout@v4: owner_id: 1 repo_id: 2 flavor: spicy @@ -99,9 +99,9 @@ dependencies: func TestParse_MissingRequiredActionFieldRejected(t *testing.T) { // commit/owner_id/repo_id present, but ref (required) is absent. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + actions/checkout@v4: commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 2 @@ -118,9 +118,9 @@ dependencies: } func TestParse_EmptyCommitRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + actions/checkout@v4: ref: v4 commit: "" owner_id: 1 @@ -136,9 +136,9 @@ dependencies: } func TestParse_ZeroOwnerIDRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + actions/checkout@v4: ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 0 @@ -154,9 +154,9 @@ dependencies: } func TestParse_ZeroRepoIDRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + actions/checkout@v4: ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 @@ -172,9 +172,9 @@ dependencies: } func TestParse_NegativeIDRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + actions/checkout@v4: ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: -1 @@ -190,18 +190,18 @@ dependencies: } func TestParse_KnownFieldsAccepted(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 workflows: .github/workflows/ci.yml: - - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 + - actions/checkout@v4 dependencies: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + actions/checkout@v4: ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 2 uses: - - actions/cache@v4:sha1-0000000000000000000000000000000000000000 + - actions/cache@v4 ` f, err := Parse([]byte(yaml)) require.NoError(t, err) @@ -214,22 +214,22 @@ dependencies: // "commit" field. Workflow A references only the good dep, workflow B // references only the corrupt dep. const ( - goodPin = "actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5" - corruptPin = "actions/setup-go@v5:sha1-0000000000000000000000000000000000000000" + goodPin = "actions/checkout@v4" + corruptPin = "actions/setup-go@v5" - corruptLockfile = `version: v0.0.1 + corruptLockfile = `version: v0.0.2 workflows: .github/workflows/a.yml: - - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 + - actions/checkout@v4 .github/workflows/b.yml: - - actions/setup-go@v5:sha1-0000000000000000000000000000000000000000 + - actions/setup-go@v5 dependencies: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + actions/checkout@v4: ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 repo_id: 2 - actions/setup-go@v5:sha1-0000000000000000000000000000000000000000: + actions/setup-go@v5: ref: v5 owner_id: 3 repo_id: 4 @@ -278,12 +278,12 @@ func TestParse_ScopedValidation_GoodAndCorruptPaths_Errors(t *testing.T) { } func TestParse_ScopedValidation_UnknownActionField_InScope_Errors(t *testing.T) { - data := `version: v0.0.1 + data := `version: v0.0.2 workflows: .github/workflows/a.yml: - - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 + - actions/checkout@v4 dependencies: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + actions/checkout@v4: ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 @@ -299,12 +299,12 @@ dependencies: } func TestParse_ScopedValidation_UnknownActionField_OutOfScope_OK(t *testing.T) { - data := `version: v0.0.1 + data := `version: v0.0.2 workflows: .github/workflows/b.yml: - - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 + - actions/checkout@v4 dependencies: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + actions/checkout@v4: ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 1 @@ -317,12 +317,12 @@ dependencies: } func TestParse_ScopedValidation_ZeroValue_InScope_Errors(t *testing.T) { - data := `version: v0.0.1 + data := `version: v0.0.2 workflows: .github/workflows/a.yml: - - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 + - actions/checkout@v4 dependencies: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + actions/checkout@v4: ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 0 @@ -338,12 +338,12 @@ dependencies: } func TestParse_ScopedValidation_ZeroValue_OutOfScope_OK(t *testing.T) { - data := `version: v0.0.1 + data := `version: v0.0.2 workflows: .github/workflows/a.yml: - - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 + - actions/checkout@v4 dependencies: - actions/checkout@v4:sha1-34e114876b0b11c390a56381ad16ebd13914f8d5: + actions/checkout@v4: ref: v4 commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 owner_id: 0 @@ -355,7 +355,7 @@ dependencies: } func TestParse_ScopedValidation_UnknownTopLevel_StillErrors(t *testing.T) { - data := `version: v0.0.1 + data := `version: v0.0.2 typo_section: {} dependencies: {} ` diff --git a/go/pkg/lockfile/uses.go b/go/pkg/lockfile/uses.go index 22c1780..af7b468 100644 --- a/go/pkg/lockfile/uses.go +++ b/go/pkg/lockfile/uses.go @@ -245,7 +245,7 @@ func isValidRef(ref string) bool { } for _, r := range ref { switch r { - case ' ', '\t', '\n', '\r', '\v', '\f', '"', '\'', '`', '\\': + case ' ', '\t', '\n', '\r', '\v', '\f', '"', '\'', '`', '\\', ':': return false } } diff --git a/schema/lockfile-v0.0.1.json b/schema/lockfile-v0.0.2.json similarity index 89% rename from schema/lockfile-v0.0.1.json rename to schema/lockfile-v0.0.2.json index e2eb4d5..10055bf 100644 --- a/schema/lockfile-v0.0.1.json +++ b/schema/lockfile-v0.0.2.json @@ -1,6 +1,6 @@ { "$schema": "https://json-schema.org/draft/2020-12/schema", - "$id": "https://gh.io/actions-lockfile/v0.0.1.json", + "$id": "https://gh.io/actions-lockfile/v0.0.2.json", "title": "GitHub Actions dependency lockfile", "description": "Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.", "type": "object", @@ -8,8 +8,8 @@ "required": ["version"], "properties": { "version": { - "description": "Lockfile schema version. Only v0.0.1 is supported.", - "const": "v0.0.1" + "description": "Lockfile schema version. Only v0.0.2 is supported.", + "const": "v0.0.2" }, "workflows": { "description": "Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.", @@ -27,9 +27,9 @@ }, "$defs": { "pin": { - "description": "Canonical dependency pin: OWNER/REPO@REF:ALGO-HEX (e.g. actions/checkout@v4:sha1-34e1...).", + "description": "Canonical dependency pin: OWNER/REPO@REF (e.g. actions/checkout@v4).", "type": "string", - "pattern": "^[^/@:]+/[^/@:]+@[^:]+:(sha1-[0-9a-f]{40}|sha256-[0-9a-f]{64})$" + "pattern": "^[^/@:]+/[^/@:]+@[^:]+$" }, "action": { "description": "Resolved metadata for a single pinned action.", From 6c64832e3d6089b9bb9962c2a08bb9816aefaacb Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 23 Jun 2026 08:40:43 -0500 Subject: [PATCH 05/15] lockfile: keep schema version at v0.0.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No version bump — the pin key format change is backward-incompatible but we're pre-GA and don't want to deal with multi-version compat machinery yet. --- .../lockfile/internal/cmd/genschema/main.go | 4 +- go/pkg/lockfile/lockfile.go | 10 ++-- go/pkg/lockfile/lockfile_test.go | 58 +++++++++---------- go/pkg/lockfile/pin.go | 2 +- go/pkg/lockfile/schema.go | 2 +- go/pkg/lockfile/schema_gen.go | 2 +- go/pkg/lockfile/schema_test.go | 30 +++++----- ...kfile-v0.0.2.json => lockfile-v0.0.1.json} | 6 +- 8 files changed, 57 insertions(+), 57 deletions(-) rename schema/{lockfile-v0.0.2.json => lockfile-v0.0.1.json} (95%) diff --git a/go/pkg/lockfile/internal/cmd/genschema/main.go b/go/pkg/lockfile/internal/cmd/genschema/main.go index 0127d35..e0517f5 100644 --- a/go/pkg/lockfile/internal/cmd/genschema/main.go +++ b/go/pkg/lockfile/internal/cmd/genschema/main.go @@ -9,7 +9,7 @@ import ( ) func main() { - schema, err := os.ReadFile("../../../schema/lockfile-v0.0.2.json") + schema, err := os.ReadFile("../../../schema/lockfile-v0.0.1.json") if err != nil { panic(err) } @@ -19,7 +19,7 @@ func main() { fmt.Fprintln(&out) fmt.Fprintln(&out, "package lockfile") fmt.Fprintln(&out) - fmt.Fprintf(&out, "const schemaV002 = %s\n", strconv.Quote(string(schema))) + fmt.Fprintf(&out, "const schemaV001 = %s\n", strconv.Quote(string(schema))) formatted, err := format.Source(out.Bytes()) if err != nil { diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go index 0e2e83c..6fb111f 100644 --- a/go/pkg/lockfile/lockfile.go +++ b/go/pkg/lockfile/lockfile.go @@ -81,7 +81,7 @@ func newYAMLParseError(err error) *ParseError { } // Version is the only supported lockfile schema version. -const Version = "v0.0.2" +const Version = "v0.0.1" // Path is the canonical repo-relative location of the dependency lockfile. const Path = ".github/workflows/actions.lock" @@ -89,7 +89,7 @@ const Path = ".github/workflows/actions.lock" // File is the parsed lockfile shape. // // # .github/workflows/actions.lock -// version: v0.0.2 +// version: v0.0.1 // workflows: // .github/workflows/deploy.yml: // - actions/checkout@v6 @@ -108,7 +108,7 @@ const Path = ".github/workflows/actions.lock" // Workflow entries hold the full transitive closure as a flat list of pin // keys for cold readability. type File struct { - // Version is the lockfile schema version string (e.g. "v0.0.2"). It is + // Version is the lockfile schema version string (e.g. "v0.0.1"). It is // always equal to the [Version] constant for files Parse accepts. Version string `yaml:"version"` @@ -749,7 +749,7 @@ func rejectFullSHACommitMismatch(pinKey, action *yaml.Node) *ParseError { // would be ambiguous about which Action metadata applies. On conflict it // returns the offending source key so callers can locate it in the YAML tree. // -// Dependency keys and Uses entries that do not parse as valid v0.0.2 pin +// Dependency keys and Uses entries that do not parse as valid v0.0.1 pin // strings (OWNER/REPO@REF) are rejected — this catches legacy // digest-suffixed keys (owner/repo@ref:sha1-...) that are invalid under // the current schema. @@ -806,7 +806,7 @@ func equalAction(a, b Action) bool { // canonicalizeWorkflowDependencies rewrites every workflow's pin list to // canonical pin strings (Pin.String()) so lookups into the Dependencies map are -// casing-agnostic. Entries that do not parse as valid v0.0.2 pin strings are +// casing-agnostic. Entries that do not parse as valid v0.0.1 pin strings are // rejected — legacy digest-suffixed pins and other malformed values are not // preserved. func canonicalizeWorkflowDependencies(f *File) (string, string, error) { diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go index ca9b21d..39171c5 100644 --- a/go/pkg/lockfile/lockfile_test.go +++ b/go/pkg/lockfile/lockfile_test.go @@ -49,7 +49,7 @@ func TestParse_WrongShapeReportsLine(t *testing.T) { // pin keys fails yaml type-decoding. Parse must surface the failing line as // structured data (ParseError.Line) and strip yaml.v3's "yaml:" prefix from // the reason so consumers don't misattribute the position to their own file. - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: {} workflows: .github/workflows/ci.yml: @@ -83,7 +83,7 @@ dependencies: {} func TestParse_DuplicateActionKeyReportsPosition(t *testing.T) { // The conflicting key must be located in the source tree so the position // points at a real offending dependency entry. - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/checkout@v6: owner_id: 1234 @@ -104,7 +104,7 @@ dependencies: func TestParse_PositionLookup(t *testing.T) { // The retained node tree is exposed for consumer diagnostics via // Position/KeyPosition. - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: {} workflows: .github/workflows/ci.yml: [] @@ -128,7 +128,7 @@ workflows: func TestParse_CanonicalizesActionKeys(t *testing.T) { const canonical = "actions/checkout@v6" - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: Actions/Checkout@v6: ref: v6 @@ -158,7 +158,7 @@ workflows: func TestParse_ConflictingActionKeyCasings(t *testing.T) { // Two source-casings of the same pin with differing metadata is // ambiguous and must be rejected. - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/checkout@v6: ref: v6 @@ -178,7 +178,7 @@ dependencies: func TestParse_DuplicateActionKeyCasingsSameMetadataOK(t *testing.T) { // Same metadata on two casings collapses to one canonical entry. - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/checkout@v6: ref: v6 @@ -197,8 +197,8 @@ dependencies: } func TestParse_UnparseableActionKeyRejected(t *testing.T) { - // v0.0.2 rejects dependency keys that do not parse as valid pins. - input := `version: v0.0.2 + // v0.0.1 rejects dependency keys that do not parse as valid pins. + input := `version: v0.0.1 dependencies: "not a pin": ref: v4 @@ -213,7 +213,7 @@ dependencies: func TestParse_WorkflowPathKeyNotCanonicalized(t *testing.T) { // File paths are case-sensitive on Linux; do not normalize them. - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: {} workflows: .github/workflows/CI.yml: [] @@ -225,7 +225,7 @@ workflows: } func TestParse_RefRoundTrip(t *testing.T) { - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/checkout@v6: ref: v6 @@ -263,7 +263,7 @@ 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.2 + yaml := `version: v0.0.1 dependencies: actions/checkout@v4: ref: v4 @@ -295,7 +295,7 @@ func TestParse_CommitInvalidFormatRejected(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - y := "version: v0.0.2\ndependencies:\n" + + y := "version: v0.0.1\ndependencies:\n" + " actions/checkout@v4:\n" + " ref: v4\n" + " commit: " + tc.commit + "\n" + @@ -315,7 +315,7 @@ func TestParse_CommitValidFormatsAccepted(t *testing.T) { } for _, commit := range cases { t.Run(commit[:10], func(t *testing.T) { - y := "version: v0.0.2\ndependencies:\n actions/checkout@v4:\n" + + y := "version: v0.0.1\ndependencies:\n actions/checkout@v4:\n" + " ref: v4\n commit: " + commit + "\n owner_id: 1\n repo_id: 1\n" _, err := Parse([]byte(y)) require.NoError(t, err) @@ -341,7 +341,7 @@ func TestParse_WorkflowPathTraversalRejected(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - y := "version: v0.0.2\ndependencies: {}\nworkflows:\n " + tc.key + ": []\n" + 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") @@ -357,7 +357,7 @@ func TestParse_WorkflowPathLegitimateKeysAccepted(t *testing.T) { } for _, key := range legit { t.Run(key, func(t *testing.T) { - y := "version: v0.0.2\ndependencies: {}\nworkflows:\n " + key + ": []\n" + y := "version: v0.0.1\ndependencies: {}\nworkflows:\n " + key + ": []\n" _, err := Parse([]byte(y)) require.NoError(t, err) }) @@ -386,7 +386,7 @@ func TestParse_RefInjectionCharsRejected(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - y := "version: v0.0.2\ndependencies:\n" + + y := "version: v0.0.1\ndependencies:\n" + " actions/checkout@v4:\n" + " ref: " + tc.yamlRef + "\n" + " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + @@ -424,7 +424,7 @@ func TestParse_ExactMaxSizeAccepted(t *testing.T) { } func TestParse_UsesCycleRejected(t *testing.T) { - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/a@v1: ref: v1 @@ -447,7 +447,7 @@ dependencies: } func TestParse_UsesSelfCycleRejected(t *testing.T) { - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/a@v1: ref: v1 @@ -464,7 +464,7 @@ dependencies: func TestParse_UsesAcyclicAccepted(t *testing.T) { // A valid DAG (A uses B, B has no uses) must parse successfully. - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/a@v1: ref: v1 @@ -487,7 +487,7 @@ func TestParse_KeyBodyRefMismatchRejected(t *testing.T) { // The pin key says "@v4" but the body says ref: v3 — this must be // rejected because the mismatch means the lockfile was hand-edited // inconsistently and cannot be trusted. - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/checkout@v4: ref: v3 @@ -504,7 +504,7 @@ dependencies: func TestParse_KeyBodyRefMatchAccepted(t *testing.T) { // Key and body ref agree — must parse successfully. - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/checkout@v4: ref: v4 @@ -518,7 +518,7 @@ dependencies: func TestParse_FullSHARefCommitMismatchRejected(t *testing.T) { // Pin key ref is a full SHA but commit encodes a different digest. - input := `version: v0.0.2 + input := `version: v0.0.1 dependencies: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683: ref: 11bd71901bbe5b1630ceea73d27597364c9af683 @@ -534,7 +534,7 @@ dependencies: func TestParse_FullSHARefCommitMatchAccepted(t *testing.T) { // Pin key ref is a full SHA and commit agrees — must parse. - input := `version: v0.0.2 + input := `version: v0.0.1 dependencies: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683: ref: 11bd71901bbe5b1630ceea73d27597364c9af683 @@ -548,7 +548,7 @@ dependencies: func TestParse_SymbolicRefDifferentCommitAccepted(t *testing.T) { // Symbolic ref (not a full SHA) — commit can be anything valid. - input := `version: v0.0.2 + input := `version: v0.0.1 dependencies: actions/checkout@v4: ref: v4 @@ -562,7 +562,7 @@ dependencies: func TestParse_ColonInRefRejected(t *testing.T) { // Colons in refs are rejected by isValidRef (matching schema pin regex). - input := `version: v0.0.2 + input := `version: v0.0.1 dependencies: actions/checkout@v4: ref: "v4:foo" @@ -577,8 +577,8 @@ dependencies: func TestParse_LegacyDigestSuffixedKeyRejected(t *testing.T) { // Legacy v0.0.1 format used "owner/repo@ref:sha1-" as keys. - // v0.0.2 rejects these because the colon makes the ref invalid. - input := "version: v0.0.2\ndependencies:\n" + + // v0.0.1 rejects these because the colon makes the ref invalid. + input := "version: v0.0.1\ndependencies:\n" + " \"actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683\":\n" + " ref: v4\n" + " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + @@ -591,7 +591,7 @@ func TestParse_LegacyDigestSuffixedKeyRejected(t *testing.T) { func TestParse_LegacyDigestSuffixedUsesEntryRejected(t *testing.T) { // Valid dependency key but its uses entry is a legacy digest-suffixed pin. - input := "version: v0.0.2\ndependencies:\n" + + input := "version: v0.0.1\ndependencies:\n" + " actions/cache@v4:\n" + " ref: v4\n" + " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + @@ -611,7 +611,7 @@ func TestParse_LegacyDigestSuffixedUsesEntryRejected(t *testing.T) { func TestParse_LegacyDigestSuffixedWorkflowDepRejected(t *testing.T) { // Workflow dependency entries with legacy format are rejected. - input := "version: v0.0.2\ndependencies:\n" + + input := "version: v0.0.1\ndependencies:\n" + " actions/checkout@v4:\n" + " ref: v4\n" + " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + diff --git a/go/pkg/lockfile/pin.go b/go/pkg/lockfile/pin.go index 72d8011..2149ad7 100644 --- a/go/pkg/lockfile/pin.go +++ b/go/pkg/lockfile/pin.go @@ -46,7 +46,7 @@ func (p Pin) String() string { } // IndexKey returns the normalized lookup key for this pin: "OWNER/REPO@REF". -// Identical to String() in v0.0.2 — retained for API compatibility. +// Identical to String() — retained for API compatibility. func (p Pin) IndexKey() string { return p.String() } diff --git a/go/pkg/lockfile/schema.go b/go/pkg/lockfile/schema.go index 5c7e448..73a5726 100644 --- a/go/pkg/lockfile/schema.go +++ b/go/pkg/lockfile/schema.go @@ -11,5 +11,5 @@ package lockfile // not the dependencies map keys, so schema validation alone won't enforce that // every dependency key is a canonical pin either. func Schema() string { - return schemaV002 + return schemaV001 } diff --git a/go/pkg/lockfile/schema_gen.go b/go/pkg/lockfile/schema_gen.go index e438010..0ebb626 100644 --- a/go/pkg/lockfile/schema_gen.go +++ b/go/pkg/lockfile/schema_gen.go @@ -2,4 +2,4 @@ package lockfile -const schemaV002 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.2.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.2 is supported.\",\n \"const\": \"v0.0.2\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF (e.g. actions/checkout@v4).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@:]+/[^/@:]+@[^:]+$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"ref\", \"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"ref\": {\n \"description\": \"The git ref the commit was resolved from. Required: every dep that passes impostor checks has a resolvable ref. The CLI picks the best ref with priority: full semver tag > any tag > branch (protected > default > release/v* > any). The parser enforces presence; priority ordering is the CLI's concern.\",\n \"type\": \"string\",\n \"minLength\": 1\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org). Pinned because names can be deleted and re-registered by someone else; the ID cannot, so it ties the pin to the original owner.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository. Pinned because a repo can be renamed or deleted and the name reclaimed; the ID detects that the repo behind the name has changed.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys, so the full dependency graph stays pinned and verifiable end to end. Required for composite actions (which can call other actions); absent for leaf actions that have no dependencies of their own.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" +const schemaV001 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.1.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.1 is supported.\",\n \"const\": \"v0.0.1\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF (e.g. actions/checkout@v4).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@:]+/[^/@:]+@[^:]+$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"ref\", \"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"ref\": {\n \"description\": \"The git ref the commit was resolved from. Required: every dep that passes impostor checks has a resolvable ref. The CLI picks the best ref with priority: full semver tag > any tag > branch (protected > default > release/v* > any). The parser enforces presence; priority ordering is the CLI's concern.\",\n \"type\": \"string\",\n \"minLength\": 1\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org). Pinned because names can be deleted and re-registered by someone else; the ID cannot, so it ties the pin to the original owner.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository. Pinned because a repo can be renamed or deleted and the name reclaimed; the ID detects that the repo behind the name has changed.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys, so the full dependency graph stays pinned and verifiable end to end. Required for composite actions (which can call other actions); absent for leaf actions that have no dependencies of their own.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" diff --git a/go/pkg/lockfile/schema_test.go b/go/pkg/lockfile/schema_test.go index 6db456b..03733fc 100644 --- a/go/pkg/lockfile/schema_test.go +++ b/go/pkg/lockfile/schema_test.go @@ -11,7 +11,7 @@ import ( ) func TestSchema_EmbeddedMatchesRootInvariant(t *testing.T) { - rootSchema, err := os.ReadFile("../../../schema/lockfile-v0.0.2.json") + rootSchema, err := os.ReadFile("../../../schema/lockfile-v0.0.1.json") if errors.Is(err, os.ErrNotExist) { t.Skip("root schema invariant is not present in this module checkout") } @@ -64,7 +64,7 @@ func TestSchema_EmbeddedMatchesEnforcement(t *testing.T) { } func TestParse_UnknownTopLevelFieldRejected(t *testing.T) { - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: {} typo_section: {} ` @@ -79,7 +79,7 @@ typo_section: {} } func TestParse_UnknownActionFieldRejected(t *testing.T) { - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/checkout@v4: owner_id: 1 @@ -99,7 +99,7 @@ dependencies: func TestParse_MissingRequiredActionFieldRejected(t *testing.T) { // commit/owner_id/repo_id present, but ref (required) is absent. - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/checkout@v4: commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 @@ -118,7 +118,7 @@ dependencies: } func TestParse_EmptyCommitRejected(t *testing.T) { - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/checkout@v4: ref: v4 @@ -136,7 +136,7 @@ dependencies: } func TestParse_ZeroOwnerIDRejected(t *testing.T) { - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/checkout@v4: ref: v4 @@ -154,7 +154,7 @@ dependencies: } func TestParse_ZeroRepoIDRejected(t *testing.T) { - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/checkout@v4: ref: v4 @@ -172,7 +172,7 @@ dependencies: } func TestParse_NegativeIDRejected(t *testing.T) { - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 dependencies: actions/checkout@v4: ref: v4 @@ -190,7 +190,7 @@ dependencies: } func TestParse_KnownFieldsAccepted(t *testing.T) { - yaml := `version: v0.0.2 + yaml := `version: v0.0.1 workflows: .github/workflows/ci.yml: - actions/checkout@v4 @@ -217,7 +217,7 @@ const ( goodPin = "actions/checkout@v4" corruptPin = "actions/setup-go@v5" - corruptLockfile = `version: v0.0.2 + corruptLockfile = `version: v0.0.1 workflows: .github/workflows/a.yml: - actions/checkout@v4 @@ -278,7 +278,7 @@ func TestParse_ScopedValidation_GoodAndCorruptPaths_Errors(t *testing.T) { } func TestParse_ScopedValidation_UnknownActionField_InScope_Errors(t *testing.T) { - data := `version: v0.0.2 + data := `version: v0.0.1 workflows: .github/workflows/a.yml: - actions/checkout@v4 @@ -299,7 +299,7 @@ dependencies: } func TestParse_ScopedValidation_UnknownActionField_OutOfScope_OK(t *testing.T) { - data := `version: v0.0.2 + data := `version: v0.0.1 workflows: .github/workflows/b.yml: - actions/checkout@v4 @@ -317,7 +317,7 @@ dependencies: } func TestParse_ScopedValidation_ZeroValue_InScope_Errors(t *testing.T) { - data := `version: v0.0.2 + data := `version: v0.0.1 workflows: .github/workflows/a.yml: - actions/checkout@v4 @@ -338,7 +338,7 @@ dependencies: } func TestParse_ScopedValidation_ZeroValue_OutOfScope_OK(t *testing.T) { - data := `version: v0.0.2 + data := `version: v0.0.1 workflows: .github/workflows/a.yml: - actions/checkout@v4 @@ -355,7 +355,7 @@ dependencies: } func TestParse_ScopedValidation_UnknownTopLevel_StillErrors(t *testing.T) { - data := `version: v0.0.2 + data := `version: v0.0.1 typo_section: {} dependencies: {} ` diff --git a/schema/lockfile-v0.0.2.json b/schema/lockfile-v0.0.1.json similarity index 95% rename from schema/lockfile-v0.0.2.json rename to schema/lockfile-v0.0.1.json index 10055bf..99839a5 100644 --- a/schema/lockfile-v0.0.2.json +++ b/schema/lockfile-v0.0.1.json @@ -1,6 +1,6 @@ { "$schema": "https://json-schema.org/draft/2020-12/schema", - "$id": "https://gh.io/actions-lockfile/v0.0.2.json", + "$id": "https://gh.io/actions-lockfile/v0.0.1.json", "title": "GitHub Actions dependency lockfile", "description": "Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.", "type": "object", @@ -8,8 +8,8 @@ "required": ["version"], "properties": { "version": { - "description": "Lockfile schema version. Only v0.0.2 is supported.", - "const": "v0.0.2" + "description": "Lockfile schema version. Only v0.0.1 is supported.", + "const": "v0.0.1" }, "workflows": { "description": "Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.", From 66dfeda9f4d38f52a7cc1391d040aa7572c6ba85 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 23 Jun 2026 08:52:11 -0500 Subject: [PATCH 06/15] lockfile: multi-version parsing with VersionPolicy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bump current schema to v0.0.2 (simplified pin keys, ref required). Restore v0.0.1 as the legacy format (tag/branch fields, :algo-hex pin suffix). Parse() now accepts both versions and normalizes to the latest File struct. New API surface: - VersionPolicy{Min, Max} — servers declare which versions they read - ParseWithPolicy(data, policy, paths...) — version-gated parsing - DefaultPolicy() — accepts all known versions (v0.0.1–v0.0.2) - SchemaForVersion(v) — returns embedded JSON schema by version - ErrUnsupportedVersion sentinel — version below consumer minimum v0.0.1 compat path: - Accepts :algo-hex pin key suffix, strips during canonicalization - Maps tag/branch fields → ref via BestRef - Does not require ref field (only commit, owner_id, repo_id) - Rejects ref field (unknown in v0.0.1 schema) Rollout: ship parser+launch with {Min:v0.0.1, Max:v0.0.2} first, then ship CLI writing v0.0.2. Eventually bump Min to drop v0.0.1. --- go/pkg/lockfile/compat.go | 489 ++++++++++++++++++ go/pkg/lockfile/compat_test.go | 239 +++++++++ .../lockfile/internal/cmd/genschema/main.go | 9 +- go/pkg/lockfile/lockfile.go | 186 +------ go/pkg/lockfile/lockfile_test.go | 54 +- go/pkg/lockfile/schema.go | 26 +- go/pkg/lockfile/schema_gen.go | 4 +- go/pkg/lockfile/schema_test.go | 50 +- schema/lockfile-v0.0.1.json | 25 +- schema/lockfile-v0.0.2.json | 67 +++ 10 files changed, 903 insertions(+), 246 deletions(-) create mode 100644 go/pkg/lockfile/compat.go create mode 100644 go/pkg/lockfile/compat_test.go create mode 100644 schema/lockfile-v0.0.2.json diff --git a/go/pkg/lockfile/compat.go b/go/pkg/lockfile/compat.go new file mode 100644 index 0000000..2b9130a --- /dev/null +++ b/go/pkg/lockfile/compat.go @@ -0,0 +1,489 @@ +package lockfile + +import ( + "fmt" + "strings" + + "gopkg.in/yaml.v3" +) + +// supportedVersions lists all schema versions this binary can parse, +// ordered from oldest to newest. +var supportedVersions = []string{"v0.0.1", "v0.0.2"} + +// ErrUnsupportedVersion is the sentinel returned when ParseWithPolicy refuses a +// lockfile whose version is older than the consumer's minimum. +var ErrUnsupportedVersion = fmt.Errorf("lockfile version is older than this consumer supports") + +// VersionPolicy controls which lockfile schema versions a consumer accepts. +// Servers set their own policy to control the rollout of new formats. +type VersionPolicy struct { + // Min is the oldest version the consumer can read. Lockfiles older than + // this are rejected with ErrUnsupportedVersion. + Min string + // Max is the newest version the consumer can read. Lockfiles newer than + // this are rejected with ErrFutureVersion. + Max string +} + +// DefaultPolicy returns a policy that accepts all versions this binary can +// parse — from the oldest supported through the latest. +func DefaultPolicy() VersionPolicy { + return VersionPolicy{ + Min: supportedVersions[0], + Max: supportedVersions[len(supportedVersions)-1], + } +} + +// ParseWithPolicy is like Parse but enforces version bounds. The lockfile's +// declared version must be within [policy.Min, policy.Max] inclusive. +func ParseWithPolicy(contents []byte, policy VersionPolicy, paths ...string) (File, error) { + return parseInternal(contents, &policy, paths) +} + +// parseInternal is the shared implementation for Parse and ParseWithPolicy. +// When policy is nil, DefaultPolicy() is used. +func parseInternal(contents []byte, policy *VersionPolicy, 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) + } + var f File + if err := root.Decode(&f); err != nil { + return File{}, newYAMLParseError(err) + } + f.node = &root + + if f.Version == "" { + pe := &ParseError{Msg: "dependency lockfile version is required"} + if m := docMapping(f.node); m != nil { + pe.Line, pe.Column = m.Line, m.Column + } + return File{}, pe + } + + // Apply version policy. + p := DefaultPolicy() + if policy != nil { + p = *policy + } + if err := checkVersionPolicy(f.Version, p, f.node); err != nil { + return File{}, err + } + + // Version-aware validation and canonicalization. + if pe := validateKnownFieldsVersioned(&f, paths, f.Version); pe != nil { + return File{}, pe + } + if pe := validateWorkflowPaths(&f); pe != nil { + return File{}, pe + } + + // For v0.0.1 files, migrate branch/tag → ref and canonicalize legacy pin keys. + if f.Version == "v0.0.1" { + migrateV001Actions(&f) + if conflictKey, err := canonicalizeActionsV001(&f); err != nil { + pe := &ParseError{Msg: err.Error(), err: err} + if l, c, ok := f.KeyPosition("dependencies", conflictKey); ok { + pe.Line, pe.Column = l, c + } + return File{}, pe + } + if wfPath, _, err := canonicalizeWorkflowDepsV001(&f); err != nil { + pe := &ParseError{Msg: err.Error(), err: err} + if l, c, ok := f.KeyPosition("workflows", wfPath); ok { + pe.Line, pe.Column = l, c + } + return File{}, pe + } + } else { + if conflictKey, err := canonicalizeActions(&f); err != nil { + pe := &ParseError{Msg: err.Error(), err: err} + if l, c, ok := f.KeyPosition("dependencies", conflictKey); ok { + pe.Line, pe.Column = l, c + } + return File{}, pe + } + if wfPath, _, err := canonicalizeWorkflowDependencies(&f); err != nil { + pe := &ParseError{Msg: err.Error(), err: err} + if l, c, ok := f.KeyPosition("workflows", wfPath); ok { + pe.Line, pe.Column = l, c + } + return File{}, pe + } + } + + 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 + } + + // Normalize the version to the latest — the File struct always represents + // the current schema regardless of what was on disk. + f.Version = Version + + return f, nil +} + +// checkVersionPolicy validates the lockfile version against the policy bounds. +func checkVersionPolicy(version string, policy VersionPolicy, node *yaml.Node) error { + if !isKnownVersion(version) { + // Unknown version — is it a future version? + if isFutureVersion(version, policy.Max) { + msg := fmt.Sprintf( + "lockfile version %s is newer than this binary supports (%s); "+ + "upgrade the tool that reads this lockfile to a build that supports %s", + version, policy.Max, version, + ) + pe := &ParseError{Msg: msg, err: ErrFutureVersion} + if l, c, ok := positionFromNode(node, "version"); ok { + pe.Line, pe.Column = l, c + } + return pe + } + msg := fmt.Sprintf("unsupported dependency lockfile version %q", version) + pe := &ParseError{Msg: msg} + if l, c, ok := positionFromNode(node, "version"); ok { + pe.Line, pe.Column = l, c + } + return pe + } + + // Check against policy min. + if versionLessThan(version, policy.Min) { + msg := fmt.Sprintf( + "lockfile version %s is older than this consumer supports (minimum %s); "+ + "regenerate the lockfile with a newer CLI", + version, policy.Min, + ) + pe := &ParseError{Msg: msg, err: ErrUnsupportedVersion} + if l, c, ok := positionFromNode(node, "version"); ok { + pe.Line, pe.Column = l, c + } + return pe + } + + // Check against policy max. + if versionLessThan(policy.Max, version) { + msg := fmt.Sprintf( + "lockfile version %s is newer than this consumer supports (maximum %s); "+ + "upgrade the tool that reads this lockfile", + version, policy.Max, + ) + pe := &ParseError{Msg: msg, err: ErrFutureVersion} + if l, c, ok := positionFromNode(node, "version"); ok { + pe.Line, pe.Column = l, c + } + return pe + } + + return nil +} + +func isKnownVersion(v string) bool { + for _, sv := range supportedVersions { + if sv == v { + return true + } + } + return false +} + +// versionLessThan reports whether a < b using semver comparison. +func versionLessThan(a, b string) bool { + av, aOK := parseSchemaVersion(a) + bv, bOK := parseSchemaVersion(b) + if !aOK || !bOK { + return false + } + for i := 0; i < 3; i++ { + if av[i] != bv[i] { + return av[i] < bv[i] + } + } + return false +} + +func positionFromNode(node *yaml.Node, key string) (line, col int, ok bool) { + m := docMapping(node) + if m == nil { + return 0, 0, false + } + _, v := mappingEntry(m, key) + if v == nil { + return 0, 0, false + } + return v.Line, v.Column, true +} + +// ── v0.0.1 compat layer ───────────────────────────────────────────────────── + +// allowedActionKeysV001 extends the v0.0.2 set with the legacy branch/tag fields. +var allowedActionKeysV001 = map[string]struct{}{ + "tag": {}, + "branch": {}, + "commit": {}, + "owner_id": {}, + "repo_id": {}, + "uses": {}, +} + +// requiredActionKeysV001 — v0.0.1 did not require ref (it used tag/branch). +var requiredActionKeysV001 = []string{"commit", "owner_id", "repo_id"} + +// migrateV001Actions walks the YAML node tree for a v0.0.1 lockfile and +// populates Action.Ref from the tag/branch fields using BestRef. +func migrateV001Actions(f *File) { + root := docMapping(f.node) + if root == nil { + return + } + _, deps := mappingEntry(root, "dependencies") + if deps == nil || deps.Kind != yaml.MappingNode { + return + } + for i := 0; i+1 < len(deps.Content); i += 2 { + pinKey := deps.Content[i] + actionNode := deps.Content[i+1] + if actionNode.Kind != yaml.MappingNode { + continue + } + + var tag, branch string + for j := 0; j+1 < len(actionNode.Content); j += 2 { + k := actionNode.Content[j] + v := actionNode.Content[j+1] + switch k.Value { + case "tag": + tag = v.Value + case "branch": + branch = v.Value + } + } + + ref := BestRef(tag, branch) + if ref == "" { + // Fall back to extracting ref from the pin key. + if pin, ok := parsePinV001(pinKey.Value); ok { + ref = pin.Ref + } + } + + // Update the Action in the File struct. + key := pinKey.Value + if a, ok := f.Dependencies[key]; ok { + if a.Ref == "" { + a.Ref = ref + f.Dependencies[key] = a + } + } + } +} + +// parsePinV001 parses a v0.0.1 pin key: "OWNER/REPO@REF:ALGO-HEX" or the +// simpler "OWNER/REPO@REF" form (some v0.0.1 lockfiles used tags without +// the digest suffix in certain contexts). +func parsePinV001(s string) (Pin, bool) { + atIdx := strings.IndexByte(s, '@') + if atIdx <= 0 || atIdx == len(s)-1 { + return Pin{}, false + } + repoPath := s[:atIdx] + refAndMaybeSuffix := s[atIdx+1:] + + if strings.Count(repoPath, "/") != 1 { + return Pin{}, false + } + owner, repo, ok := SplitNWO(repoPath) + if !ok { + return Pin{}, false + } + + // Strip the optional :algo-hex suffix. + ref := refAndMaybeSuffix + if colonIdx := strings.LastIndexByte(refAndMaybeSuffix, ':'); colonIdx > 0 { + // Only strip if the part after the colon looks like algo-hex. + possibleDigest := refAndMaybeSuffix[colonIdx+1:] + possibleRef := refAndMaybeSuffix[:colonIdx] + if looksLikeAlgoHex(possibleDigest) { + ref = possibleRef + } + } + + return Pin{ + Owner: owner, + Repo: repo, + Ref: ref, + }.Canonical(), true +} + +// looksLikeAlgoHex checks if a string matches the "algo-hexdigest" pattern +// (e.g. "sha1-abc123..." or "sha256-def456..."). +func looksLikeAlgoHex(s string) bool { + dashIdx := strings.IndexByte(s, '-') + if dashIdx <= 0 || dashIdx == len(s)-1 { + return false + } + algo := s[:dashIdx] + if algo != "sha1" && algo != "sha256" { + return false + } + hex := s[dashIdx+1:] + for _, c := range hex { + if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f')) { + return false + } + } + return len(hex) > 0 +} + +// canonicalizeActionsV001 canonicalizes dependency keys from v0.0.1 format. +// It strips the :algo-hex suffix and normalizes owner/repo casing. +func canonicalizeActionsV001(f *File) (string, error) { + if len(f.Dependencies) == 0 { + return "", nil + } + out := make(map[string]Action, len(f.Dependencies)) + for key, action := range f.Dependencies { + pin, ok := parsePinV001(key) + if !ok { + return key, fmt.Errorf("dependency key %q is not a valid v0.0.1 pin", key) + } + canonical := pin.String() + if len(action.Uses) > 0 { + canonUses := make([]string, len(action.Uses)) + for i, u := range action.Uses { + uPin, uOk := parsePinV001(u) + if !uOk { + return key, fmt.Errorf("uses entry %q in dependency %q is not a valid v0.0.1 pin", u, key) + } + canonUses[i] = uPin.String() + } + action.Uses = canonUses + } + if existing, dup := out[canonical]; dup { + if !equalAction(existing, action) { + return key, fmt.Errorf("duplicate action key %q after canonicalization with differing metadata", canonical) + } + continue + } + out[canonical] = action + } + f.Dependencies = out + return "", nil +} + +// canonicalizeWorkflowDepsV001 canonicalizes workflow dependency entries from +// v0.0.1 format (strips :algo-hex suffix). +func canonicalizeWorkflowDepsV001(f *File) (string, string, error) { + for path, deps := range f.Workflows { + if len(deps) == 0 { + continue + } + canonicalized := make([]string, len(deps)) + for i, dep := range deps { + pin, ok := parsePinV001(dep) + if !ok { + return path, dep, fmt.Errorf("workflow %q dependency %q is not a valid v0.0.1 pin", path, dep) + } + canonicalized[i] = pin.String() + } + f.Workflows[path] = canonicalized + } + return "", "", nil +} + +// validateKnownFieldsVersioned is the version-aware variant of validateKnownFields. +// It selects the correct allowed/required key sets based on the lockfile version. +func validateKnownFieldsVersioned(f *File, paths []string, version string) *ParseError { + allowed := allowedActionKeys + required := requiredActionKeys + + if version == "v0.0.1" { + allowed = allowedActionKeysV001 + required = requiredActionKeysV001 + } + + root := docMapping(f.node) + if root == nil { + return nil + } + for i := 0; i+1 < len(root.Content); i += 2 { + k := root.Content[i] + if _, ok := allowedFileKeys[k.Value]; !ok { + return &ParseError{Line: k.Line, Column: k.Column, Msg: fmt.Sprintf("unknown lockfile field %q", k.Value)} + } + } + _, deps := mappingEntry(root, "dependencies") + if deps == nil || deps.Kind != yaml.MappingNode { + return nil + } + + var inScope map[string]struct{} + if len(paths) > 0 { + inScope = make(map[string]struct{}) + for _, p := range paths { + for _, pin := range f.Workflows[p] { + inScope[pin] = struct{}{} + } + } + } + + for i := 0; i+1 < len(deps.Content); i += 2 { + pinKey := deps.Content[i] + action := deps.Content[i+1] + if action.Kind != yaml.MappingNode { + continue + } + + if inScope != nil { + if _, ok := inScope[pinKey.Value]; !ok { + continue + } + } + + present := make(map[string]struct{}, len(action.Content)/2) + for j := 0; j+1 < len(action.Content); j += 2 { + ak := action.Content[j] + if _, ok := allowed[ak.Value]; !ok { + return &ParseError{ + Line: ak.Line, + Column: ak.Column, + Msg: fmt.Sprintf("unknown action field %q for dependency %q", ak.Value, pinKey.Value), + } + } + present[ak.Value] = struct{}{} + } + for _, req := range required { + if _, ok := present[req]; !ok { + return &ParseError{ + Line: pinKey.Line, + Column: pinKey.Column, + Msg: fmt.Sprintf("missing required action field %q for dependency %q", req, pinKey.Value), + } + } + } + if pe := rejectZeroValues(action, pinKey.Value); pe != nil { + return pe + } + // Key-ref mismatch and full-SHA checks only apply for v0.0.2+ + // where the pin key format is owner/repo@ref. + if version != "v0.0.1" { + if pe := rejectKeyRefMismatch(pinKey, action); pe != nil { + return pe + } + if pe := rejectFullSHACommitMismatch(pinKey, action); pe != nil { + return pe + } + } + } + return nil +} diff --git a/go/pkg/lockfile/compat_test.go b/go/pkg/lockfile/compat_test.go new file mode 100644 index 0000000..04cc805 --- /dev/null +++ b/go/pkg/lockfile/compat_test.go @@ -0,0 +1,239 @@ +package lockfile + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// ── v0.0.1 compat tests ───────────────────────────────────────────────────── + +func TestParse_V001_TagBranchMigratedToRef(t *testing.T) { + // A v0.0.1 lockfile with tag/branch fields gets migrated to ref. + input := `version: v0.0.1 +dependencies: + actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + tag: v4 + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 1 + repo_id: 2 + actions/internal@trunk:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: + branch: trunk + commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + owner_id: 3 + repo_id: 4 +` + f, err := Parse([]byte(input)) + require.NoError(t, err) + + // Version is normalized to latest. + assert.Equal(t, Version, f.Version) + + // Pin keys are canonicalized to the v0.0.2 format (no :algo-hex suffix). + checkout := f.Dependencies["actions/checkout@v4"] + assert.Equal(t, "v4", checkout.Ref) + assert.Equal(t, "sha1-11bd71901bbe5b1630ceea73d27597364c9af683", checkout.Commit) + + internal := f.Dependencies["actions/internal@trunk"] + assert.Equal(t, "trunk", internal.Ref) +} + +func TestParse_V001_TagWinsOverBranch(t *testing.T) { + input := `version: v0.0.1 +dependencies: + actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + tag: v4 + branch: main + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 1 + repo_id: 2 +` + f, err := Parse([]byte(input)) + require.NoError(t, err) + + checkout := f.Dependencies["actions/checkout@v4"] + assert.Equal(t, "v4", checkout.Ref, "tag should win over branch in BestRef") +} + +func TestParse_V001_NoTagNoBranch_FallsBackToPinRef(t *testing.T) { + // If neither tag nor branch is present, ref is extracted from the pin key. + input := `version: v0.0.1 +dependencies: + actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 1 + repo_id: 2 +` + f, err := Parse([]byte(input)) + require.NoError(t, err) + + checkout := f.Dependencies["actions/checkout@v4"] + assert.Equal(t, "v4", checkout.Ref, "ref should be extracted from pin key when tag/branch absent") +} + +func TestParse_V001_LegacyPinKeysCanonicalizedToSimpleFormat(t *testing.T) { + input := `version: v0.0.1 +workflows: + .github/workflows/ci.yml: + - actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683 +dependencies: + actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + tag: v4 + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 1 + repo_id: 2 +` + f, err := Parse([]byte(input)) + require.NoError(t, err) + + // Dependency key is canonicalized to simple format. + _, ok := f.Dependencies["actions/checkout@v4"] + assert.True(t, ok, "expected simple key format after canonicalization") + + // Workflow deps are also canonicalized. + wf := f.Workflows[".github/workflows/ci.yml"] + require.Len(t, wf, 1) + assert.Equal(t, "actions/checkout@v4", wf[0]) +} + +func TestParse_V001_RefFieldIsRejected(t *testing.T) { + // v0.0.1 does not allow the "ref" field — it uses tag/branch. + input := `version: v0.0.1 +dependencies: + actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + ref: v4 + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 1 + repo_id: 2 +` + _, err := Parse([]byte(input)) + require.Error(t, err) + assert.Contains(t, err.Error(), `unknown action field "ref"`) +} + +func TestParse_V001_UsesEntriesCanonicalized(t *testing.T) { + input := `version: v0.0.1 +dependencies: + actions/composite@v1:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: + tag: v1 + commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + owner_id: 1 + repo_id: 1 + uses: + - actions/cache@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + actions/cache@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + tag: v4 + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 2 + repo_id: 3 +` + f, err := Parse([]byte(input)) + require.NoError(t, err) + + composite := f.Dependencies["actions/composite@v1"] + require.Len(t, composite.Uses, 1) + assert.Equal(t, "actions/cache@v4", composite.Uses[0]) +} + +// ── VersionPolicy tests ────────────────────────────────────────────────────── + +func TestParseWithPolicy_AcceptsVersionInRange(t *testing.T) { + input := `version: v0.0.2 +dependencies: + actions/checkout@v4: + ref: v4 + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 1 + repo_id: 2 +` + policy := VersionPolicy{Min: "v0.0.1", Max: "v0.0.2"} + f, err := ParseWithPolicy([]byte(input), policy) + require.NoError(t, err) + assert.Equal(t, Version, f.Version) +} + +func TestParseWithPolicy_RejectsVersionBelowMin(t *testing.T) { + input := `version: v0.0.1 +dependencies: + actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + tag: v4 + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 1 + repo_id: 2 +` + policy := VersionPolicy{Min: "v0.0.2", Max: "v0.0.2"} + _, err := ParseWithPolicy([]byte(input), policy) + require.Error(t, err) + assert.True(t, errors.Is(err, ErrUnsupportedVersion)) + assert.Contains(t, err.Error(), "older than this consumer supports") +} + +func TestParseWithPolicy_RejectsVersionAboveMax(t *testing.T) { + input := `version: v0.0.2 +dependencies: {} +` + policy := VersionPolicy{Min: "v0.0.1", Max: "v0.0.1"} + _, err := ParseWithPolicy([]byte(input), policy) + require.Error(t, err) + assert.True(t, errors.Is(err, ErrFutureVersion)) + assert.Contains(t, err.Error(), "newer than this consumer supports") +} + +func TestParseWithPolicy_V001Only(t *testing.T) { + input := `version: v0.0.1 +dependencies: + actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + tag: v4 + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 1 + repo_id: 2 +` + policy := VersionPolicy{Min: "v0.0.1", Max: "v0.0.1"} + f, err := ParseWithPolicy([]byte(input), policy) + require.NoError(t, err) + assert.Equal(t, Version, f.Version, "parsed file should be normalized to latest version") + assert.Equal(t, "v4", f.Dependencies["actions/checkout@v4"].Ref) +} + +func TestParseWithPolicy_UnknownFutureVersion(t *testing.T) { + input := `version: v1.0.0 +dependencies: {} +` + policy := VersionPolicy{Min: "v0.0.1", Max: "v0.0.2"} + _, err := ParseWithPolicy([]byte(input), policy) + require.Error(t, err) + assert.True(t, errors.Is(err, ErrFutureVersion)) +} + +func TestParsePinV001(t *testing.T) { + cases := []struct { + input string + wantOK bool + wantRef string + }{ + {"actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683", true, "v4"}, + {"actions/checkout@v4", true, "v4"}, + {"ACTIONS/Checkout@v4:sha1-abc123", true, "v4"}, // algo-hex-ish stripped, case normalized + {"org/repo@release/v1.2:sha256-" + "ab" + "cd" + "ef" + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", true, "release/v1.2"}, + {"not-a-pin", false, ""}, + {"actions/checkout", false, ""}, + {"actions/sub/path@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683", false, ""}, // sub-path rejected + } + for _, tc := range cases { + t.Run(tc.input, func(t *testing.T) { + pin, ok := parsePinV001(tc.input) + assert.Equal(t, tc.wantOK, ok) + if ok { + assert.Equal(t, tc.wantRef, pin.Ref) + } + }) + } +} + +func TestDefaultPolicy(t *testing.T) { + p := DefaultPolicy() + assert.Equal(t, "v0.0.1", p.Min) + assert.Equal(t, Version, p.Max) +} diff --git a/go/pkg/lockfile/internal/cmd/genschema/main.go b/go/pkg/lockfile/internal/cmd/genschema/main.go index e0517f5..d1feaa7 100644 --- a/go/pkg/lockfile/internal/cmd/genschema/main.go +++ b/go/pkg/lockfile/internal/cmd/genschema/main.go @@ -9,7 +9,11 @@ import ( ) func main() { - schema, err := os.ReadFile("../../../schema/lockfile-v0.0.1.json") + schemaV001, err := os.ReadFile("../../../schema/lockfile-v0.0.1.json") + if err != nil { + panic(err) + } + schemaV002, err := os.ReadFile("../../../schema/lockfile-v0.0.2.json") if err != nil { panic(err) } @@ -19,7 +23,8 @@ func main() { fmt.Fprintln(&out) fmt.Fprintln(&out, "package lockfile") fmt.Fprintln(&out) - fmt.Fprintf(&out, "const schemaV001 = %s\n", strconv.Quote(string(schema))) + fmt.Fprintf(&out, "const schemaV001 = %s\n\n", strconv.Quote(string(schemaV001))) + fmt.Fprintf(&out, "const schemaV002 = %s\n", strconv.Quote(string(schemaV002))) formatted, err := format.Source(out.Bytes()) if err != nil { diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go index 6fb111f..d093a7b 100644 --- a/go/pkg/lockfile/lockfile.go +++ b/go/pkg/lockfile/lockfile.go @@ -80,8 +80,8 @@ func newYAMLParseError(err error) *ParseError { } } -// Version is the only supported lockfile schema version. -const Version = "v0.0.1" +// Version is the latest lockfile schema version this binary writes. +const Version = "v0.0.2" // Path is the canonical repo-relative location of the dependency lockfile. const Path = ".github/workflows/actions.lock" @@ -221,7 +221,7 @@ func mappingEntry(m *yaml.Node, key string) (k, v *yaml.Node) { // The returned bool reports whether the workflow path was found in the lockfile. // // Each string in the returned slice is a canonical pin key in -// OWNER/REPO@REF:ALGO-HEX form. To retrieve the full action metadata for a +// OWNER/REPO@REF form. To retrieve the full action metadata for a // pin, look it up in File.Dependencies: // // pins, ok := f.LookupWorkflow(".github/workflows/deploy.yml") @@ -312,76 +312,7 @@ const MaxParseSize = 1 << 20 // 1 MiB // diagnostics. Workflow path keys are NOT canonicalized — file paths are // case-sensitive on Linux. 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) - } - var f File - if err := root.Decode(&f); err != nil { - return File{}, newYAMLParseError(err) - } - // Retain the tree so semantic errors below (and consumers) can resolve - // precise line+column positions within the lockfile. - f.node = &root - - if f.Version == "" { - // No version node to point at; anchor at the top of the document. - pe := &ParseError{Msg: "dependency lockfile version is required"} - if m := docMapping(f.node); m != nil { - pe.Line, pe.Column = m.Line, m.Column - } - return File{}, pe - } - if f.Version != Version { - msg := fmt.Sprintf("unsupported dependency lockfile version %q", f.Version) - var wrapped error - if isFutureVersion(f.Version, Version) { - msg = fmt.Sprintf( - "lockfile version %s is newer than this binary supports (%s); "+ - "upgrade the tool that reads this lockfile to a build that supports %s", - f.Version, Version, f.Version, - ) - wrapped = ErrFutureVersion - } - pe := &ParseError{Msg: msg, err: wrapped} - if l, c, ok := f.Position("version"); ok { - pe.Line, pe.Column = l, c - } - return File{}, pe - } - 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 { - pe.Line, pe.Column = l, c - } - return File{}, pe - } - if wfPath, _, err := canonicalizeWorkflowDependencies(&f); err != nil { - pe := &ParseError{Msg: err.Error(), err: err} - if l, c, ok := f.KeyPosition("workflows", wfPath); ok { - pe.Line, pe.Column = l, c - } - return File{}, pe - } - 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 + return parseInternal(contents, nil, paths) } // detectUsesCycle reports a cycle in the action uses graph using @@ -494,16 +425,15 @@ func checkWorkflowPathKey(p string) error { 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. +// allowedFileKeys is the set of permitted top-level lockfile keys. var allowedFileKeys = map[string]struct{}{ "version": {}, "workflows": {}, "dependencies": {}, } -// allowedActionKeys is the set of permitted keys within a dependency's Action -// mapping. It mirrors the $defs/action properties in lockfile-v0.0.1.json. +// allowedActionKeys is the set of permitted keys within a v0.0.2 dependency's +// Action mapping. var allowedActionKeys = map[string]struct{}{ "ref": {}, "commit": {}, @@ -512,108 +442,10 @@ var allowedActionKeys = map[string]struct{}{ "uses": {}, } -// requiredActionKeys lists the keys every dependency's Action mapping must -// carry, in report order. It mirrors the $defs/action "required" list in -// lockfile-v0.0.1.json. `uses` is required only for composite actions — a -// condition the lockfile alone can't express — so it doesn't appear here. +// requiredActionKeys lists the keys every v0.0.2 dependency's Action mapping +// must carry, in report order. var requiredActionKeys = []string{"ref", "commit", "owner_id", "repo_id"} -// validateKnownFields enforces the schema's additionalProperties:false and -// required rules on the lockfile's fixed-shape mappings — the document root and -// each dependency's metadata block. A stray, misspelled, or missing key is a -// positioned parse error rather than a silently dropped or defaulted field, -// matching the stricter parsing the embedded schema describes. Map-valued -// sections (workflow paths, dependency pin keys) carry arbitrary data keys and -// are intentionally not constrained here. -// -// When paths is non-empty, per-dependency checks (unknown keys, required keys, -// zero-value rejection) are scoped to only the dependency entries referenced -// by the union of f.Workflows[p] for each requested path. This prevents a -// single corrupt entry from failing every workflow that shares the lockfile. -// When paths is empty, every dependency entry is validated. -func validateKnownFields(f *File, paths []string) *ParseError { - root := docMapping(f.node) - if root == nil { - return nil - } - for i := 0; i+1 < len(root.Content); i += 2 { - k := root.Content[i] - if _, ok := allowedFileKeys[k.Value]; !ok { - return &ParseError{Line: k.Line, Column: k.Column, Msg: fmt.Sprintf("unknown lockfile field %q", k.Value)} - } - } - _, deps := mappingEntry(root, "dependencies") - if deps == nil || deps.Kind != yaml.MappingNode { - return nil - } - - // Build the in-scope set from the raw (pre-canonicalization) workflow - // entries. nil means "validate all" (len(paths)==0); a non-nil but empty - // map means "validate nothing" (requested paths had no matching deps). - var inScope map[string]struct{} - if len(paths) > 0 { - inScope = make(map[string]struct{}) - for _, p := range paths { - for _, pin := range f.Workflows[p] { - inScope[pin] = struct{}{} - } - } - } - - for i := 0; i+1 < len(deps.Content); i += 2 { - pinKey := deps.Content[i] - action := deps.Content[i+1] - if action.Kind != yaml.MappingNode { - continue - } - - if inScope != nil { - if _, ok := inScope[pinKey.Value]; !ok { - continue - } - } - - present := make(map[string]struct{}, len(action.Content)/2) - for j := 0; j+1 < len(action.Content); j += 2 { - ak := action.Content[j] - if _, ok := allowedActionKeys[ak.Value]; !ok { - return &ParseError{ - Line: ak.Line, - Column: ak.Column, - Msg: fmt.Sprintf("unknown action field %q for dependency %q", ak.Value, pinKey.Value), - } - } - present[ak.Value] = struct{}{} - } - for _, req := range requiredActionKeys { - if _, ok := present[req]; !ok { - // The missing key has no node to point at; anchor the error on - // the dependency's pin key so callers can locate the entry. - return &ParseError{ - Line: pinKey.Line, - Column: pinKey.Column, - Msg: fmt.Sprintf("missing required action field %q for dependency %q", req, pinKey.Value), - } - } - } - // Enforce non-zero values for fields where the zero value is - // meaningless and would silently disable a security check. - if pe := rejectZeroValues(action, pinKey.Value); pe != nil { - return pe - } - // Reject entries where the pin key's ref disagrees with the body's - // ref field — a mismatch means the lockfile was hand-edited - // inconsistently and cannot be trusted. - if pe := rejectKeyRefMismatch(pinKey, action); pe != nil { - return pe - } - if pe := rejectFullSHACommitMismatch(pinKey, action); pe != nil { - return pe - } - } - return nil -} - // nonEmptyStringKeys lists action fields that must be non-empty strings. var nonEmptyStringKeys = map[string]struct{}{ "ref": {}, diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go index 39171c5..6a67ed1 100644 --- a/go/pkg/lockfile/lockfile_test.go +++ b/go/pkg/lockfile/lockfile_test.go @@ -49,7 +49,7 @@ func TestParse_WrongShapeReportsLine(t *testing.T) { // pin keys fails yaml type-decoding. Parse must surface the failing line as // structured data (ParseError.Line) and strip yaml.v3's "yaml:" prefix from // the reason so consumers don't misattribute the position to their own file. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: {} workflows: .github/workflows/ci.yml: @@ -83,7 +83,7 @@ dependencies: {} func TestParse_DuplicateActionKeyReportsPosition(t *testing.T) { // The conflicting key must be located in the source tree so the position // points at a real offending dependency entry. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/checkout@v6: owner_id: 1234 @@ -104,7 +104,7 @@ dependencies: func TestParse_PositionLookup(t *testing.T) { // The retained node tree is exposed for consumer diagnostics via // Position/KeyPosition. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: {} workflows: .github/workflows/ci.yml: [] @@ -128,7 +128,7 @@ workflows: func TestParse_CanonicalizesActionKeys(t *testing.T) { const canonical = "actions/checkout@v6" - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: Actions/Checkout@v6: ref: v6 @@ -158,7 +158,7 @@ workflows: func TestParse_ConflictingActionKeyCasings(t *testing.T) { // Two source-casings of the same pin with differing metadata is // ambiguous and must be rejected. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/checkout@v6: ref: v6 @@ -178,7 +178,7 @@ dependencies: func TestParse_DuplicateActionKeyCasingsSameMetadataOK(t *testing.T) { // Same metadata on two casings collapses to one canonical entry. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/checkout@v6: ref: v6 @@ -198,7 +198,7 @@ dependencies: func TestParse_UnparseableActionKeyRejected(t *testing.T) { // v0.0.1 rejects dependency keys that do not parse as valid pins. - input := `version: v0.0.1 + input := `version: v0.0.2 dependencies: "not a pin": ref: v4 @@ -213,7 +213,7 @@ dependencies: func TestParse_WorkflowPathKeyNotCanonicalized(t *testing.T) { // File paths are case-sensitive on Linux; do not normalize them. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: {} workflows: .github/workflows/CI.yml: [] @@ -225,7 +225,7 @@ workflows: } func TestParse_RefRoundTrip(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/checkout@v6: ref: v6 @@ -263,7 +263,7 @@ 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 + yaml := `version: v0.0.2 dependencies: actions/checkout@v4: ref: v4 @@ -295,7 +295,7 @@ func TestParse_CommitInvalidFormatRejected(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - y := "version: v0.0.1\ndependencies:\n" + + y := "version: v0.0.2\ndependencies:\n" + " actions/checkout@v4:\n" + " ref: v4\n" + " commit: " + tc.commit + "\n" + @@ -315,7 +315,7 @@ func TestParse_CommitValidFormatsAccepted(t *testing.T) { } for _, commit := range cases { t.Run(commit[:10], func(t *testing.T) { - y := "version: v0.0.1\ndependencies:\n actions/checkout@v4:\n" + + y := "version: v0.0.2\ndependencies:\n actions/checkout@v4:\n" + " ref: v4\n commit: " + commit + "\n owner_id: 1\n repo_id: 1\n" _, err := Parse([]byte(y)) require.NoError(t, err) @@ -341,7 +341,7 @@ func TestParse_WorkflowPathTraversalRejected(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - y := "version: v0.0.1\ndependencies: {}\nworkflows:\n " + tc.key + ": []\n" + y := "version: v0.0.2\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") @@ -357,7 +357,7 @@ func TestParse_WorkflowPathLegitimateKeysAccepted(t *testing.T) { } for _, key := range legit { t.Run(key, func(t *testing.T) { - y := "version: v0.0.1\ndependencies: {}\nworkflows:\n " + key + ": []\n" + y := "version: v0.0.2\ndependencies: {}\nworkflows:\n " + key + ": []\n" _, err := Parse([]byte(y)) require.NoError(t, err) }) @@ -386,7 +386,7 @@ func TestParse_RefInjectionCharsRejected(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - y := "version: v0.0.1\ndependencies:\n" + + y := "version: v0.0.2\ndependencies:\n" + " actions/checkout@v4:\n" + " ref: " + tc.yamlRef + "\n" + " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + @@ -424,7 +424,7 @@ func TestParse_ExactMaxSizeAccepted(t *testing.T) { } func TestParse_UsesCycleRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/a@v1: ref: v1 @@ -447,7 +447,7 @@ dependencies: } func TestParse_UsesSelfCycleRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/a@v1: ref: v1 @@ -464,7 +464,7 @@ dependencies: func TestParse_UsesAcyclicAccepted(t *testing.T) { // A valid DAG (A uses B, B has no uses) must parse successfully. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/a@v1: ref: v1 @@ -487,7 +487,7 @@ func TestParse_KeyBodyRefMismatchRejected(t *testing.T) { // The pin key says "@v4" but the body says ref: v3 — this must be // rejected because the mismatch means the lockfile was hand-edited // inconsistently and cannot be trusted. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/checkout@v4: ref: v3 @@ -504,7 +504,7 @@ dependencies: func TestParse_KeyBodyRefMatchAccepted(t *testing.T) { // Key and body ref agree — must parse successfully. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/checkout@v4: ref: v4 @@ -518,7 +518,7 @@ dependencies: func TestParse_FullSHARefCommitMismatchRejected(t *testing.T) { // Pin key ref is a full SHA but commit encodes a different digest. - input := `version: v0.0.1 + input := `version: v0.0.2 dependencies: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683: ref: 11bd71901bbe5b1630ceea73d27597364c9af683 @@ -534,7 +534,7 @@ dependencies: func TestParse_FullSHARefCommitMatchAccepted(t *testing.T) { // Pin key ref is a full SHA and commit agrees — must parse. - input := `version: v0.0.1 + input := `version: v0.0.2 dependencies: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683: ref: 11bd71901bbe5b1630ceea73d27597364c9af683 @@ -548,7 +548,7 @@ dependencies: func TestParse_SymbolicRefDifferentCommitAccepted(t *testing.T) { // Symbolic ref (not a full SHA) — commit can be anything valid. - input := `version: v0.0.1 + input := `version: v0.0.2 dependencies: actions/checkout@v4: ref: v4 @@ -562,7 +562,7 @@ dependencies: func TestParse_ColonInRefRejected(t *testing.T) { // Colons in refs are rejected by isValidRef (matching schema pin regex). - input := `version: v0.0.1 + input := `version: v0.0.2 dependencies: actions/checkout@v4: ref: "v4:foo" @@ -578,7 +578,7 @@ dependencies: func TestParse_LegacyDigestSuffixedKeyRejected(t *testing.T) { // Legacy v0.0.1 format used "owner/repo@ref:sha1-" as keys. // v0.0.1 rejects these because the colon makes the ref invalid. - input := "version: v0.0.1\ndependencies:\n" + + input := "version: v0.0.2\ndependencies:\n" + " \"actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683\":\n" + " ref: v4\n" + " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + @@ -591,7 +591,7 @@ func TestParse_LegacyDigestSuffixedKeyRejected(t *testing.T) { func TestParse_LegacyDigestSuffixedUsesEntryRejected(t *testing.T) { // Valid dependency key but its uses entry is a legacy digest-suffixed pin. - input := "version: v0.0.1\ndependencies:\n" + + input := "version: v0.0.2\ndependencies:\n" + " actions/cache@v4:\n" + " ref: v4\n" + " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + @@ -611,7 +611,7 @@ func TestParse_LegacyDigestSuffixedUsesEntryRejected(t *testing.T) { func TestParse_LegacyDigestSuffixedWorkflowDepRejected(t *testing.T) { // Workflow dependency entries with legacy format are rejected. - input := "version: v0.0.1\ndependencies:\n" + + input := "version: v0.0.2\ndependencies:\n" + " actions/checkout@v4:\n" + " ref: v4\n" + " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + diff --git a/go/pkg/lockfile/schema.go b/go/pkg/lockfile/schema.go index 73a5726..e36beba 100644 --- a/go/pkg/lockfile/schema.go +++ b/go/pkg/lockfile/schema.go @@ -2,14 +2,22 @@ package lockfile //go:generate go run ./internal/cmd/genschema -// Schema returns the embedded JSON Schema document for the supported lockfile -// version. Callers can surface it for editor integration or external -// validation. Parse checks the document's shape — known keys, required fields, -// version — and canonicalizes pin keys, but does not reject entries whose keys -// aren't canonical pins; they're preserved for consumer diagnostics. Note the -// schema's pin pattern constrains pin *values* (the workflow and uses arrays), -// not the dependencies map keys, so schema validation alone won't enforce that -// every dependency key is a canonical pin either. +// Schema returns the embedded JSON Schema document for the latest lockfile +// version (v0.0.2). Callers can surface it for editor integration or external +// validation. func Schema() string { - return schemaV001 + return schemaV002 +} + +// SchemaForVersion returns the embedded JSON Schema for a specific lockfile +// version. Returns ("", false) for unknown versions. +func SchemaForVersion(version string) (string, bool) { + switch version { + case "v0.0.1": + return schemaV001, true + case "v0.0.2": + return schemaV002, true + default: + return "", false + } } diff --git a/go/pkg/lockfile/schema_gen.go b/go/pkg/lockfile/schema_gen.go index 0ebb626..3d95601 100644 --- a/go/pkg/lockfile/schema_gen.go +++ b/go/pkg/lockfile/schema_gen.go @@ -2,4 +2,6 @@ package lockfile -const schemaV001 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.1.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.1 is supported.\",\n \"const\": \"v0.0.1\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF (e.g. actions/checkout@v4).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@:]+/[^/@:]+@[^:]+$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"ref\", \"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"ref\": {\n \"description\": \"The git ref the commit was resolved from. Required: every dep that passes impostor checks has a resolvable ref. The CLI picks the best ref with priority: full semver tag > any tag > branch (protected > default > release/v* > any). The parser enforces presence; priority ordering is the CLI's concern.\",\n \"type\": \"string\",\n \"minLength\": 1\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org). Pinned because names can be deleted and re-registered by someone else; the ID cannot, so it ties the pin to the original owner.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository. Pinned because a repo can be renamed or deleted and the name reclaimed; the ID detects that the repo behind the name has changed.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys, so the full dependency graph stays pinned and verifiable end to end. Required for composite actions (which can call other actions); absent for leaf actions that have no dependencies of their own.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" +const schemaV001 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.1.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.1 is supported.\",\n \"const\": \"v0.0.1\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF:ALGO-HEX (e.g. actions/checkout@v4:sha1-abc123...).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@]+/[^/@]+@[^:]+:[a-z0-9]+-[a-f0-9]+$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"tag\": {\n \"description\": \"The tag the commit was resolved from, if any.\",\n \"type\": \"string\"\n },\n \"branch\": {\n \"description\": \"The branch the commit was resolved from, if no tag was available.\",\n \"type\": \"string\"\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...).\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org).\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" + +const schemaV002 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.2.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.2 is supported.\",\n \"const\": \"v0.0.2\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF (e.g. actions/checkout@v4).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@:]+/[^/@:]+@[^:]+$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"ref\", \"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"ref\": {\n \"description\": \"The git ref the commit was resolved from. Required: every dep that passes impostor checks has a resolvable ref. The CLI picks the best ref with priority: full semver tag > any tag > branch (protected > default > release/v* > any). The parser enforces presence; priority ordering is the CLI's concern.\",\n \"type\": \"string\",\n \"minLength\": 1\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org). Pinned because names can be deleted and re-registered by someone else; the ID cannot, so it ties the pin to the original owner.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository. Pinned because a repo can be renamed or deleted and the name reclaimed; the ID detects that the repo behind the name has changed.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys, so the full dependency graph stays pinned and verifiable end to end. Required for composite actions (which can call other actions); absent for leaf actions that have no dependencies of their own.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" diff --git a/go/pkg/lockfile/schema_test.go b/go/pkg/lockfile/schema_test.go index 03733fc..9d952e3 100644 --- a/go/pkg/lockfile/schema_test.go +++ b/go/pkg/lockfile/schema_test.go @@ -11,12 +11,24 @@ import ( ) func TestSchema_EmbeddedMatchesRootInvariant(t *testing.T) { - rootSchema, err := os.ReadFile("../../../schema/lockfile-v0.0.1.json") - if errors.Is(err, os.ErrNotExist) { - t.Skip("root schema invariant is not present in this module checkout") + for _, ver := range []struct { + version string + file string + }{ + {"v0.0.1", "../../../schema/lockfile-v0.0.1.json"}, + {"v0.0.2", "../../../schema/lockfile-v0.0.2.json"}, + } { + t.Run(ver.version, func(t *testing.T) { + rootSchema, err := os.ReadFile(ver.file) + if errors.Is(err, os.ErrNotExist) { + t.Skip("root schema invariant is not present in this module checkout") + } + require.NoError(t, err) + embedded, ok := SchemaForVersion(ver.version) + require.True(t, ok, "SchemaForVersion(%q) returned false", ver.version) + assert.JSONEq(t, string(rootSchema), embedded) + }) } - require.NoError(t, err) - assert.JSONEq(t, string(rootSchema), Schema()) } // TestSchema_EmbeddedMatchesEnforcement guards against drift between the @@ -64,7 +76,7 @@ func TestSchema_EmbeddedMatchesEnforcement(t *testing.T) { } func TestParse_UnknownTopLevelFieldRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: {} typo_section: {} ` @@ -79,7 +91,7 @@ typo_section: {} } func TestParse_UnknownActionFieldRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/checkout@v4: owner_id: 1 @@ -99,7 +111,7 @@ dependencies: func TestParse_MissingRequiredActionFieldRejected(t *testing.T) { // commit/owner_id/repo_id present, but ref (required) is absent. - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/checkout@v4: commit: sha1-34e114876b0b11c390a56381ad16ebd13914f8d5 @@ -118,7 +130,7 @@ dependencies: } func TestParse_EmptyCommitRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/checkout@v4: ref: v4 @@ -136,7 +148,7 @@ dependencies: } func TestParse_ZeroOwnerIDRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/checkout@v4: ref: v4 @@ -154,7 +166,7 @@ dependencies: } func TestParse_ZeroRepoIDRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/checkout@v4: ref: v4 @@ -172,7 +184,7 @@ dependencies: } func TestParse_NegativeIDRejected(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 dependencies: actions/checkout@v4: ref: v4 @@ -190,7 +202,7 @@ dependencies: } func TestParse_KnownFieldsAccepted(t *testing.T) { - yaml := `version: v0.0.1 + yaml := `version: v0.0.2 workflows: .github/workflows/ci.yml: - actions/checkout@v4 @@ -217,7 +229,7 @@ const ( goodPin = "actions/checkout@v4" corruptPin = "actions/setup-go@v5" - corruptLockfile = `version: v0.0.1 + corruptLockfile = `version: v0.0.2 workflows: .github/workflows/a.yml: - actions/checkout@v4 @@ -278,7 +290,7 @@ func TestParse_ScopedValidation_GoodAndCorruptPaths_Errors(t *testing.T) { } func TestParse_ScopedValidation_UnknownActionField_InScope_Errors(t *testing.T) { - data := `version: v0.0.1 + data := `version: v0.0.2 workflows: .github/workflows/a.yml: - actions/checkout@v4 @@ -299,7 +311,7 @@ dependencies: } func TestParse_ScopedValidation_UnknownActionField_OutOfScope_OK(t *testing.T) { - data := `version: v0.0.1 + data := `version: v0.0.2 workflows: .github/workflows/b.yml: - actions/checkout@v4 @@ -317,7 +329,7 @@ dependencies: } func TestParse_ScopedValidation_ZeroValue_InScope_Errors(t *testing.T) { - data := `version: v0.0.1 + data := `version: v0.0.2 workflows: .github/workflows/a.yml: - actions/checkout@v4 @@ -338,7 +350,7 @@ dependencies: } func TestParse_ScopedValidation_ZeroValue_OutOfScope_OK(t *testing.T) { - data := `version: v0.0.1 + data := `version: v0.0.2 workflows: .github/workflows/a.yml: - actions/checkout@v4 @@ -355,7 +367,7 @@ dependencies: } func TestParse_ScopedValidation_UnknownTopLevel_StillErrors(t *testing.T) { - data := `version: v0.0.1 + data := `version: v0.0.2 typo_section: {} dependencies: {} ` diff --git a/schema/lockfile-v0.0.1.json b/schema/lockfile-v0.0.1.json index 99839a5..49890c4 100644 --- a/schema/lockfile-v0.0.1.json +++ b/schema/lockfile-v0.0.1.json @@ -27,37 +27,40 @@ }, "$defs": { "pin": { - "description": "Canonical dependency pin: OWNER/REPO@REF (e.g. actions/checkout@v4).", + "description": "Canonical dependency pin: OWNER/REPO@REF:ALGO-HEX (e.g. actions/checkout@v4:sha1-abc123...).", "type": "string", - "pattern": "^[^/@:]+/[^/@:]+@[^:]+$" + "pattern": "^[^/@]+/[^/@]+@[^:]+:[a-z0-9]+-[a-f0-9]+$" }, "action": { "description": "Resolved metadata for a single pinned action.", "type": "object", "additionalProperties": false, - "required": ["ref", "commit", "owner_id", "repo_id"], + "required": ["commit", "owner_id", "repo_id"], "properties": { - "ref": { - "description": "The git ref the commit was resolved from. Required: every dep that passes impostor checks has a resolvable ref. The CLI picks the best ref with priority: full semver tag > any tag > branch (protected > default > release/v* > any). The parser enforces presence; priority ordering is the CLI's concern.", - "type": "string", - "minLength": 1 + "tag": { + "description": "The tag the commit was resolved from, if any.", + "type": "string" + }, + "branch": { + "description": "The branch the commit was resolved from, if no tag was available.", + "type": "string" }, "commit": { - "description": "The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.", + "description": "The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...).", "type": "string" }, "owner_id": { - "description": "The numeric ID of the action's owner (user or org). Pinned because names can be deleted and re-registered by someone else; the ID cannot, so it ties the pin to the original owner.", + "description": "The numeric ID of the action's owner (user or org).", "type": "integer", "minimum": 1 }, "repo_id": { - "description": "The numeric ID of the action's repository. Pinned because a repo can be renamed or deleted and the name reclaimed; the ID detects that the repo behind the name has changed.", + "description": "The numeric ID of the action's repository.", "type": "integer", "minimum": 1 }, "uses": { - "description": "The action's own direct dependencies, as canonical pin keys, so the full dependency graph stays pinned and verifiable end to end. Required for composite actions (which can call other actions); absent for leaf actions that have no dependencies of their own.", + "description": "The action's own direct dependencies, as canonical pin keys.", "type": "array", "items": { "$ref": "#/$defs/pin" } } diff --git a/schema/lockfile-v0.0.2.json b/schema/lockfile-v0.0.2.json new file mode 100644 index 0000000..10055bf --- /dev/null +++ b/schema/lockfile-v0.0.2.json @@ -0,0 +1,67 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://gh.io/actions-lockfile/v0.0.2.json", + "title": "GitHub Actions dependency lockfile", + "description": "Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.", + "type": "object", + "additionalProperties": false, + "required": ["version"], + "properties": { + "version": { + "description": "Lockfile schema version. Only v0.0.2 is supported.", + "const": "v0.0.2" + }, + "workflows": { + "description": "Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.", + "type": "object", + "additionalProperties": { + "type": "array", + "items": { "$ref": "#/$defs/pin" } + } + }, + "dependencies": { + "description": "Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.", + "type": "object", + "additionalProperties": { "$ref": "#/$defs/action" } + } + }, + "$defs": { + "pin": { + "description": "Canonical dependency pin: OWNER/REPO@REF (e.g. actions/checkout@v4).", + "type": "string", + "pattern": "^[^/@:]+/[^/@:]+@[^:]+$" + }, + "action": { + "description": "Resolved metadata for a single pinned action.", + "type": "object", + "additionalProperties": false, + "required": ["ref", "commit", "owner_id", "repo_id"], + "properties": { + "ref": { + "description": "The git ref the commit was resolved from. Required: every dep that passes impostor checks has a resolvable ref. The CLI picks the best ref with priority: full semver tag > any tag > branch (protected > default > release/v* > any). The parser enforces presence; priority ordering is the CLI's concern.", + "type": "string", + "minLength": 1 + }, + "commit": { + "description": "The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.", + "type": "string" + }, + "owner_id": { + "description": "The numeric ID of the action's owner (user or org). Pinned because names can be deleted and re-registered by someone else; the ID cannot, so it ties the pin to the original owner.", + "type": "integer", + "minimum": 1 + }, + "repo_id": { + "description": "The numeric ID of the action's repository. Pinned because a repo can be renamed or deleted and the name reclaimed; the ID detects that the repo behind the name has changed.", + "type": "integer", + "minimum": 1 + }, + "uses": { + "description": "The action's own direct dependencies, as canonical pin keys, so the full dependency graph stays pinned and verifiable end to end. Required for composite actions (which can call other actions); absent for leaf actions that have no dependencies of their own.", + "type": "array", + "items": { "$ref": "#/$defs/pin" } + } + } + } + } +} From f0234c473abcd1fdfc39c67d6af391d32fc7ea9a Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 23 Jun 2026 09:58:37 -0500 Subject: [PATCH 07/15] lockfile: export CLIName const for consistent messaging Parser and launch both reference the CLI in user-facing messages. Centralizing the name here avoids stale references when the CLI gets renamed (previously 'gh actions-pin', now 'gh actions-lock'). --- go/pkg/lockfile/lockfile.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go index d093a7b..e327cc3 100644 --- a/go/pkg/lockfile/lockfile.go +++ b/go/pkg/lockfile/lockfile.go @@ -86,6 +86,11 @@ const Version = "v0.0.2" // Path is the canonical repo-relative location of the dependency lockfile. const Path = ".github/workflows/actions.lock" +// CLIName is the canonical name of the CLI extension that manages lockfiles. +// Use this in user-facing messages instead of hardcoding the string, so all +// consumers (parser, launch, docs) stay consistent if it ever changes again. +const CLIName = "gh actions-lock" + // File is the parsed lockfile shape. // // # .github/workflows/actions.lock From 1ee4c251236e4431abe826db328df31845acaee6 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 23 Jun 2026 10:20:44 -0500 Subject: [PATCH 08/15] lockfile: reuse isFutureVersion for versionLessThan --- go/pkg/lockfile/compat.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/go/pkg/lockfile/compat.go b/go/pkg/lockfile/compat.go index 2b9130a..1ef6354 100644 --- a/go/pkg/lockfile/compat.go +++ b/go/pkg/lockfile/compat.go @@ -198,18 +198,10 @@ func isKnownVersion(v string) bool { } // versionLessThan reports whether a < b using semver comparison. +// Reuses the existing parseSchemaVersion + isFutureVersion helpers from +// lockfile.go rather than reimplementing the comparison. func versionLessThan(a, b string) bool { - av, aOK := parseSchemaVersion(a) - bv, bOK := parseSchemaVersion(b) - if !aOK || !bOK { - return false - } - for i := 0; i < 3; i++ { - if av[i] != bv[i] { - return av[i] < bv[i] - } - } - return false + return isFutureVersion(b, a) } func positionFromNode(node *yaml.Node, key string) (line, col int, ok bool) { From db53379e5f33ca3ead0248cbcd9904012dcc359c Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 23 Jun 2026 10:59:24 -0500 Subject: [PATCH 09/15] lockfile: add parse benchmarks --- go/pkg/lockfile/bench_test.go | 80 +++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 go/pkg/lockfile/bench_test.go diff --git a/go/pkg/lockfile/bench_test.go b/go/pkg/lockfile/bench_test.go new file mode 100644 index 0000000..a1db06f --- /dev/null +++ b/go/pkg/lockfile/bench_test.go @@ -0,0 +1,80 @@ +package lockfile + +import "testing" + +var benchV002 = []byte(`version: v0.0.2 +workflows: + .github/workflows/ci.yml: + - actions/checkout@v4 + - actions/setup-go@v5 + - actions/cache@v4 +dependencies: + actions/checkout@v4: + ref: v4 + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 44036562 + repo_id: 197814629 + actions/setup-go@v5: + ref: v5 + commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + owner_id: 44036562 + repo_id: 249058325 + actions/cache@v4: + ref: v4 + commit: sha1-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + owner_id: 44036562 + repo_id: 251882839 + uses: + - actions/checkout@v4 +`) + +var benchV001 = []byte(`version: v0.0.1 +workflows: + .github/workflows/ci.yml: + - actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + - actions/setup-go@v5:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + - actions/cache@v4:sha1-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb +dependencies: + actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683: + tag: v4 + commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683 + owner_id: 44036562 + repo_id: 197814629 + actions/setup-go@v5:sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa: + tag: v5 + commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + owner_id: 44036562 + repo_id: 249058325 + actions/cache@v4:sha1-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb: + tag: v4 + commit: sha1-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + owner_id: 44036562 + repo_id: 251882839 + uses: + - actions/checkout@v4:sha1-11bd71901bbe5b1630ceea73d27597364c9af683 +`) + +func BenchmarkParse_V002(b *testing.B) { + for i := 0; i < b.N; i++ { + if _, err := Parse(benchV002); err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkParse_V001_Compat(b *testing.B) { + for i := 0; i < b.N; i++ { + if _, err := Parse(benchV001); err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkParseWithPolicy_V002(b *testing.B) { + policy := VersionPolicy{Min: "v0.0.1", Max: "v0.0.2"} + for i := 0; i < b.N; i++ { + if _, err := ParseWithPolicy(benchV002, policy); err != nil { + b.Fatal(err) + } + } +} From 5de7d93e56ba66901b5b188146cfe60a1b46f498 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 23 Jun 2026 11:02:33 -0500 Subject: [PATCH 10/15] lockfile: validate refs in v0.0.1 compat path parsePinV001 and migrateV001Actions were not calling isValidRef on extracted/synthesized refs, allowing shell metacharacters from crafted tag/branch values to flow into Action.Ref unchecked. Consumers use Action.Ref in GraphQL queries and shell commands. Now: parsePinV001 rejects pins with invalid refs (matching ParsePin), and migrateV001Actions drops refs that fail validation rather than propagating them. --- go/pkg/lockfile/compat.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/go/pkg/lockfile/compat.go b/go/pkg/lockfile/compat.go index 1ef6354..eac6fe4 100644 --- a/go/pkg/lockfile/compat.go +++ b/go/pkg/lockfile/compat.go @@ -269,6 +269,13 @@ func migrateV001Actions(f *File) { } } + // Validate the synthesized ref against the same denylist that + // rejectZeroValues applies to v0.0.2 ref fields — prevents shell + // metacharacters in tag/branch values from flowing into Action.Ref. + if ref != "" && !isValidRef(ref) { + ref = "" + } + // Update the Action in the File struct. key := pinKey.Value if a, ok := f.Dependencies[key]; ok { @@ -310,6 +317,12 @@ func parsePinV001(s string) (Pin, bool) { } } + // Validate the ref with the same denylist as ParsePin — reject shell + // metacharacters, whitespace, and traversal sequences. + if !isValidRef(ref) { + return Pin{}, false + } + return Pin{ Owner: owner, Repo: repo, From d89b79283b8b0e45b70be7032a2f4251d27832c7 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 23 Jun 2026 11:12:31 -0500 Subject: [PATCH 11/15] lockfile: fall back to pin key ref when tag/branch fails validation If BestRef(tag, branch) produces an unsafe ref that isValidRef rejects, try extracting the ref from the pin key before giving up. This prevents migrated v0.0.1 entries from ending up with empty Ref values that violate the v0.0.2 schema invariant. --- go/pkg/lockfile/compat.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/go/pkg/lockfile/compat.go b/go/pkg/lockfile/compat.go index eac6fe4..d32869d 100644 --- a/go/pkg/lockfile/compat.go +++ b/go/pkg/lockfile/compat.go @@ -272,8 +272,12 @@ func migrateV001Actions(f *File) { // Validate the synthesized ref against the same denylist that // rejectZeroValues applies to v0.0.2 ref fields — prevents shell // metacharacters in tag/branch values from flowing into Action.Ref. + // Fall back to pin key ref if tag/branch was invalid. if ref != "" && !isValidRef(ref) { ref = "" + if pin, ok := parsePinV001(pinKey.Value); ok && isValidRef(pin.Ref) { + ref = pin.Ref + } } // Update the Action in the File struct. From 13c755d842bff84f0e1a0c29c306a4f79c96f405 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 23 Jun 2026 11:27:52 -0500 Subject: [PATCH 12/15] lockfile: drop ref character validation denylist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workflow parser does no character validation on uses: refs — just a structural @ split. Align the lockfile library: isValidRef now only rejects empty strings and colons (colon is the legacy pin key separator, needed for grammar correctness). Removes ~110 lines of security theater. --- go/pkg/lockfile/compat.go | 14 +----------- go/pkg/lockfile/doc.go | 12 +++++----- go/pkg/lockfile/lockfile.go | 14 ------------ go/pkg/lockfile/lockfile_test.go | 38 ++------------------------------ go/pkg/lockfile/pin.go | 4 +--- go/pkg/lockfile/pin_test.go | 25 --------------------- go/pkg/lockfile/uses.go | 22 +++--------------- go/pkg/lockfile/uses_test.go | 11 ++------- 8 files changed, 14 insertions(+), 126 deletions(-) diff --git a/go/pkg/lockfile/compat.go b/go/pkg/lockfile/compat.go index d32869d..26eb1bb 100644 --- a/go/pkg/lockfile/compat.go +++ b/go/pkg/lockfile/compat.go @@ -269,17 +269,6 @@ func migrateV001Actions(f *File) { } } - // Validate the synthesized ref against the same denylist that - // rejectZeroValues applies to v0.0.2 ref fields — prevents shell - // metacharacters in tag/branch values from flowing into Action.Ref. - // Fall back to pin key ref if tag/branch was invalid. - if ref != "" && !isValidRef(ref) { - ref = "" - if pin, ok := parsePinV001(pinKey.Value); ok && isValidRef(pin.Ref) { - ref = pin.Ref - } - } - // Update the Action in the File struct. key := pinKey.Value if a, ok := f.Dependencies[key]; ok { @@ -321,8 +310,7 @@ func parsePinV001(s string) (Pin, bool) { } } - // Validate the ref with the same denylist as ParsePin — reject shell - // metacharacters, whitespace, and traversal sequences. + // Validate the ref with the same check as ParsePin — reject empty refs. if !isValidRef(ref) { return Pin{}, false } diff --git a/go/pkg/lockfile/doc.go b/go/pkg/lockfile/doc.go index 2068d84..a8b6c13 100644 --- a/go/pkg/lockfile/doc.go +++ b/go/pkg/lockfile/doc.go @@ -33,11 +33,9 @@ // # Security note for contributors // // owner/repo/path components pass isValidSegment (fixed character set, -// ".."/"." barred) before reaching any URL or GraphQL builder. The ref is -// validated by a denylist rather than an allowlist because git refs carry -// slashes, dots, and embedded @; isValidRef only ensures the ref cannot escape -// a quoted string or smuggle a path traversal. A ref still needs escaping -// before use in URL paths. These validators are hand-rolled, allocation-free, -// and single-pass because they run on the hot path. Do not replace them with -// regular expressions. +// ".."/"." barred) before reaching any URL or GraphQL builder. Ref validation +// is minimal (non-empty, no colons) — the workflow parser itself does no ref +// character validation, so neither do we. These validators are hand-rolled, +// allocation-free, and single-pass because they run on the hot path. Do not +// replace them with regular expressions. package lockfile diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go index e327cc3..cbac946 100644 --- a/go/pkg/lockfile/lockfile.go +++ b/go/pkg/lockfile/lockfile.go @@ -495,20 +495,6 @@ func rejectZeroValues(action *yaml.Node, dep string) *ParseError { } } - // ref values are used in GraphQL queries, log output, and - // sometimes shell commands by consumers. Validate with the same - // denylist that ParseActionRef applies to refs so that a crafted - // lockfile cannot arm a downstream injection through this field. - if key.Value == "ref" && 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 6a67ed1..1946036 100644 --- a/go/pkg/lockfile/lockfile_test.go +++ b/go/pkg/lockfile/lockfile_test.go @@ -364,40 +364,6 @@ func TestParse_WorkflowPathLegitimateKeysAccepted(t *testing.T) { } } -func TestParse_RefInjectionCharsRejected(t *testing.T) { - // ref 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 - yamlRef 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"}, - // path traversal with semicolons - {"semicolon traversal", `"../../etc/passwd; rm -rf /"`}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - y := "version: v0.0.2\ndependencies:\n" + - " actions/checkout@v4:\n" + - " ref: " + tc.yamlRef + "\n" + - " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + - " owner_id: 1\n repo_id: 1\n" - _, err := Parse([]byte(y)) - require.Error(t, err, "ref %q should be rejected", tc.yamlRef) - 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 @@ -561,7 +527,7 @@ dependencies: } func TestParse_ColonInRefRejected(t *testing.T) { - // Colons in refs are rejected by isValidRef (matching schema pin regex). + // Colons in refs cause a pin key mismatch (pin key ref is "v4" but body ref is "v4:foo"). input := `version: v0.0.2 dependencies: actions/checkout@v4: @@ -572,7 +538,7 @@ dependencies: ` _, err := Parse([]byte(input)) require.Error(t, err) - assert.Contains(t, err.Error(), "unsafe characters") + assert.Contains(t, err.Error(), "does not match pin key ref") } func TestParse_LegacyDigestSuffixedKeyRejected(t *testing.T) { diff --git a/go/pkg/lockfile/pin.go b/go/pkg/lockfile/pin.go index 2149ad7..441169d 100644 --- a/go/pkg/lockfile/pin.go +++ b/go/pkg/lockfile/pin.go @@ -92,9 +92,7 @@ func ParsePin(s string) (Pin, bool) { // 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. + // Reject empty refs and colons (colon is the legacy pin key separator). if !isValidRef(ref) { return Pin{}, false } diff --git a/go/pkg/lockfile/pin_test.go b/go/pkg/lockfile/pin_test.go index d791924..ec32adf 100644 --- a/go/pkg/lockfile/pin_test.go +++ b/go/pkg/lockfile/pin_test.go @@ -116,28 +116,3 @@ 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"}, - {"tab in ref", "actions/checkout@v4\tevil"}, - {"double-quote in ref", "actions/checkout@v4\"evil"}, - {"single-quote in ref", "actions/checkout@v4'evil"}, - {"backtick in ref", "actions/checkout@v4`evil"}, - {"backslash in ref", "actions/checkout@v4\\evil"}, - {"dotdot in ref", "actions/checkout@../evil"}, - } - 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) - }) - } -} diff --git a/go/pkg/lockfile/uses.go b/go/pkg/lockfile/uses.go index af7b468..96ef4d7 100644 --- a/go/pkg/lockfile/uses.go +++ b/go/pkg/lockfile/uses.go @@ -110,9 +110,6 @@ func splitUsesRef(uses string) *usesRef { return nil } ref := atParts[1] - if !isValidRef(ref) { - return nil - } segments := strings.SplitN(atParts[0], "/", 3) if len(segments) < 2 || segments[0] == "" || segments[1] == "" { @@ -230,26 +227,13 @@ func isValidSegment(s string) bool { return true } -// isValidRef gates the ref (the part after @). Git refs are permissive — -// slashes, dots, plus signs, even an embedded @ (foo/bar@a@b -> ref "a@b") — -// so this is a denylist, not an allowlist. It rejects what cannot survive -// interpolation: whitespace, quotes, backtick, backslash, and the ".." -// sequence (which git itself forbids, so no valid ref is lost). It does NOT -// make the ref URL-path-safe; refs hold slashes. Escape downstream. +// isValidRef gates the ref (the part after @). Only rejects empty refs and +// colons (which would be ambiguous with the legacy pin key separator). func isValidRef(ref string) bool { if ref == "" { return false } - if strings.Contains(ref, "..") { - return false - } - for _, r := range ref { - switch r { - case ' ', '\t', '\n', '\r', '\v', '\f', '"', '\'', '`', '\\', ':': - return false - } - } - return true + return !strings.Contains(ref, ":") } func containsControlChars(s string) bool { diff --git a/go/pkg/lockfile/uses_test.go b/go/pkg/lockfile/uses_test.go index 99c59cb..52260be 100644 --- a/go/pkg/lockfile/uses_test.go +++ b/go/pkg/lockfile/uses_test.go @@ -47,13 +47,8 @@ func TestParseActionRef(t *testing.T) { {name: "dotdot repo segment", input: "foo/..@v1", wantNil: true}, {name: "dot repo segment", input: "foo/.@v1", wantNil: true}, {name: "dot owner segment", input: "./repo@v1", wantNil: true}, - {name: "ref with double quote", input: `a/b@v1"inj`, wantNil: true}, - {name: "ref with space", input: "a/b@v1 space", wantNil: true}, - {name: "ref with single quote", input: "a/b@v1'inj", wantNil: true}, - {name: "ref with backtick", input: "a/b@v1`inj", wantNil: true}, - {name: "ref with backslash", input: `a/b@v1\inj`, wantNil: true}, - {name: "ref with dotdot traversal", input: "a/b@heads/../../x", wantNil: true}, - {name: "ref with consecutive dots", input: "a/b@v1..v2", wantNil: true}, + // Refs are not validated beyond structural checks (colons for pin + // grammar). The workflow parser accepts any characters in refs. } for _, tt := range tests { @@ -108,8 +103,6 @@ func TestParseReusableWorkflowRef(t *testing.T) { // Security boundary still applies. {name: "local reusable workflow", input: "./.github/workflows/ci.yml@v1", wantNil: true}, {name: "expression ref", input: "${{ matrix.wf }}@v1", wantNil: true}, - {name: "ref injection double quote", input: `octo/repo/.github/workflows/ci.yml@v1"inj`, wantNil: true}, - {name: "ref injection space", input: "octo/repo/.github/workflows/ci.yml@v1 x", wantNil: true}, {name: "path traversal in workflow path", input: "octo/repo/.github/workflows/../../x.yml@v1", wantNil: true}, {name: "empty ref", input: "octo/repo/.github/workflows/ci.yml@", wantNil: true}, {name: "control char in path", input: "octo/repo/.github/workflows/ci\t.yml@v1", wantNil: true}, From ed33efba3f6d5c4eb097bb64f6e057b1a5d19f39 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 23 Jun 2026 12:05:15 -0500 Subject: [PATCH 13/15] readme: update for v0.0.2 schema --- README.md | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 8dd9bde..16e8178 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ func main() { return } for _, key := range pins { - fmt.Println(key) // e.g. actions/checkout@v6.0.2:sha1-de0fac2e... + fmt.Println(key) // e.g. actions/checkout@v6.0.2 } } ``` @@ -74,39 +74,43 @@ _ = file The lockfile is a YAML document whose shape is defined by a JSON Schema 2020-12 document embedded in the package and reachable via `lockfile.Schema()`. -The current schema version is `v0.0.1` -([`schema/lockfile-v0.0.1.json`](https://github.com/github/actions-lockfile/blob/main/schema/lockfile-v0.0.1.json)). +The current schema version is `v0.0.2` +([`schema/lockfile-v0.0.2.json`](https://github.com/github/actions-lockfile/blob/main/schema/lockfile-v0.0.2.json)). The on-disk file lives at [`Path`](https://github.com/github/actions-lockfile/blob/main/go/pkg/lockfile/lockfile.go) (`.github/workflows/actions.lock`) and has three top-level keys: ```yaml -version: v0.0.1 +version: v0.0.2 workflows: - # workflow path → flat, transitive list of canonical pin keys + # workflow path -> flat, transitive list of pin keys .github/workflows/release.yml: - - actions/checkout@v6.0.2:sha1-de0fac2e... + - actions/checkout@v6.0.2 dependencies: - # canonical pin key → resolved action metadata - actions/checkout@v6.0.2:sha1-de0fac2e...: - tag: v6.0.2 - branch: main + # pin key -> resolved action metadata + actions/checkout@v6.0.2: + ref: v6.0.2 commit: sha1-de0fac2e... owner_id: 44036562 repo_id: 197814629 ``` -A canonical pin key is `OWNER/REPO@REF:ALGO-HEX`. The same key appears in both -`workflows` (as flat transitive lists) and `dependencies` (as deduplicated -graph entries with `uses:` links to direct dependencies). +A pin key is `OWNER/REPO@REF`. The same key appears in both `workflows` (as +flat transitive lists) and `dependencies` (as deduplicated graph entries with +`uses:` links to direct dependencies). + +The parser also reads v0.0.1 lockfiles (which used `tag`/`branch` fields and +`:algo-hex` suffixed pin keys) and normalizes them to the v0.0.2 `File` struct. +Use `ParseWithPolicy` with a `VersionPolicy` to control which versions are +accepted. ## Compatibility and stability - The Go module follows [semver](https://semver.org/). The publicly documented exported surface is intended to be stable across minor versions. - The lockfile schema is versioned independently. The current schema version - is `v0.0.1`, embedded in the package and emitted as the `version` field of - every lockfile. + is `v0.0.2`, embedded in the package and emitted as the `version` field of + every lockfile. The parser reads both v0.0.1 and v0.0.2. - Pre-1.0, the package reserves the right to remove any incidentally-exported helper not covered by the [Usage](#usage) and [What this package does](#what-this-package-does) sections. Those sections From c745c7cdf2838ed0d5ca54191d75ea71eac60bc6 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 23 Jun 2026 12:27:27 -0500 Subject: [PATCH 14/15] lockfile: address PR review feedback - Re-add isValidRef check in splitUsesRef (colons must be rejected for pin grammar compatibility) - Fix stale version references in comments (v0.0.1 -> v0.0.2) - Update rejectZeroValues doc comment to reflect relaxed ref validation - Add colon-in-ref test case for ParseActionRef - Tighten v0.0.1 schema pin pattern (reject colons in NWO, restrict algo to sha1|sha256) --- go/pkg/lockfile/lockfile.go | 10 ++++------ go/pkg/lockfile/lockfile_test.go | 2 +- go/pkg/lockfile/pin.go | 2 -- go/pkg/lockfile/schema_gen.go | 2 +- go/pkg/lockfile/uses.go | 3 +++ go/pkg/lockfile/uses_test.go | 1 + schema/lockfile-v0.0.1.json | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go index cbac946..1d8d27a 100644 --- a/go/pkg/lockfile/lockfile.go +++ b/go/pkg/lockfile/lockfile.go @@ -466,10 +466,8 @@ var positiveIntKeys = map[string]struct{}{ // rejectZeroValues checks that required action fields carry meaningful values: // the "commit" field must be non-empty and a valid algo-hex digest string, // integer ID fields like "owner_id" and "repo_id" must be positive, and -// the "ref" field (when present) 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. +// string fields in nonEmptyStringKeys must not be blank. A present-but-zero +// value 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] @@ -572,7 +570,7 @@ func rejectFullSHACommitMismatch(pinKey, action *yaml.Node) *ParseError { // would be ambiguous about which Action metadata applies. On conflict it // returns the offending source key so callers can locate it in the YAML tree. // -// Dependency keys and Uses entries that do not parse as valid v0.0.1 pin +// Dependency keys and Uses entries that do not parse as valid v0.0.2 pin // strings (OWNER/REPO@REF) are rejected — this catches legacy // digest-suffixed keys (owner/repo@ref:sha1-...) that are invalid under // the current schema. @@ -629,7 +627,7 @@ func equalAction(a, b Action) bool { // canonicalizeWorkflowDependencies rewrites every workflow's pin list to // canonical pin strings (Pin.String()) so lookups into the Dependencies map are -// casing-agnostic. Entries that do not parse as valid v0.0.1 pin strings are +// casing-agnostic. Entries that do not parse as valid v0.0.2 pin strings are // rejected — legacy digest-suffixed pins and other malformed values are not // preserved. func canonicalizeWorkflowDependencies(f *File) (string, string, error) { diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go index 1946036..e4d2520 100644 --- a/go/pkg/lockfile/lockfile_test.go +++ b/go/pkg/lockfile/lockfile_test.go @@ -197,7 +197,7 @@ dependencies: } func TestParse_UnparseableActionKeyRejected(t *testing.T) { - // v0.0.1 rejects dependency keys that do not parse as valid pins. + // v0.0.2 rejects dependency keys that do not parse as valid pins. input := `version: v0.0.2 dependencies: "not a pin": diff --git a/go/pkg/lockfile/pin.go b/go/pkg/lockfile/pin.go index 441169d..e8b7c19 100644 --- a/go/pkg/lockfile/pin.go +++ b/go/pkg/lockfile/pin.go @@ -90,8 +90,6 @@ func ParsePin(s string) (Pin, bool) { return Pin{}, false } - // Validate the ref with the same denylist used by ParseActionRef to - // reject shell metacharacters, whitespace, and traversal sequences. // Reject empty refs and colons (colon is the legacy pin key separator). if !isValidRef(ref) { return Pin{}, false diff --git a/go/pkg/lockfile/schema_gen.go b/go/pkg/lockfile/schema_gen.go index 3d95601..57e13e0 100644 --- a/go/pkg/lockfile/schema_gen.go +++ b/go/pkg/lockfile/schema_gen.go @@ -2,6 +2,6 @@ package lockfile -const schemaV001 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.1.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.1 is supported.\",\n \"const\": \"v0.0.1\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF:ALGO-HEX (e.g. actions/checkout@v4:sha1-abc123...).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@]+/[^/@]+@[^:]+:[a-z0-9]+-[a-f0-9]+$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"tag\": {\n \"description\": \"The tag the commit was resolved from, if any.\",\n \"type\": \"string\"\n },\n \"branch\": {\n \"description\": \"The branch the commit was resolved from, if no tag was available.\",\n \"type\": \"string\"\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...).\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org).\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" +const schemaV001 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.1.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.1 is supported.\",\n \"const\": \"v0.0.1\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF:ALGO-HEX (e.g. actions/checkout@v4:sha1-abc123...).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@:]+/[^/@:]+@[^:]+:(sha1|sha256)-[a-f0-9]+$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"tag\": {\n \"description\": \"The tag the commit was resolved from, if any.\",\n \"type\": \"string\"\n },\n \"branch\": {\n \"description\": \"The branch the commit was resolved from, if no tag was available.\",\n \"type\": \"string\"\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...).\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org).\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" const schemaV002 = "{\n \"$schema\": \"https://json-schema.org/draft/2020-12/schema\",\n \"$id\": \"https://gh.io/actions-lockfile/v0.0.2.json\",\n \"title\": \"GitHub Actions dependency lockfile\",\n \"description\": \"Machine-generated lockfile describing the pinned action dependency graph for a repository's workflows. Written and updated by `gh actions-lock`.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"version\"],\n \"properties\": {\n \"version\": {\n \"description\": \"Lockfile schema version. Only v0.0.2 is supported.\",\n \"const\": \"v0.0.2\"\n },\n \"workflows\": {\n \"description\": \"Map of repo-relative workflow path to the flat, transitive list of canonical pin keys it depends on.\",\n \"type\": \"object\",\n \"additionalProperties\": {\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n },\n \"dependencies\": {\n \"description\": \"Deduplicated action graph keyed by canonical pin. Each entry records the resolved metadata for one action tarball.\",\n \"type\": \"object\",\n \"additionalProperties\": { \"$ref\": \"#/$defs/action\" }\n }\n },\n \"$defs\": {\n \"pin\": {\n \"description\": \"Canonical dependency pin: OWNER/REPO@REF (e.g. actions/checkout@v4).\",\n \"type\": \"string\",\n \"pattern\": \"^[^/@:]+/[^/@:]+@[^:]+$\"\n },\n \"action\": {\n \"description\": \"Resolved metadata for a single pinned action.\",\n \"type\": \"object\",\n \"additionalProperties\": false,\n \"required\": [\"ref\", \"commit\", \"owner_id\", \"repo_id\"],\n \"properties\": {\n \"ref\": {\n \"description\": \"The git ref the commit was resolved from. Required: every dep that passes impostor checks has a resolvable ref. The CLI picks the best ref with priority: full semver tag > any tag > branch (protected > default > release/v* > any). The parser enforces presence; priority ordering is the CLI's concern.\",\n \"type\": \"string\",\n \"minLength\": 1\n },\n \"commit\": {\n \"description\": \"The exact commit the action resolves to, in algo-prefixed digest form (e.g. sha1-...). This is the immutable identity the runner checks out; tags and branches can be moved, this cannot.\",\n \"type\": \"string\"\n },\n \"owner_id\": {\n \"description\": \"The numeric ID of the action's owner (user or org). Pinned because names can be deleted and re-registered by someone else; the ID cannot, so it ties the pin to the original owner.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"repo_id\": {\n \"description\": \"The numeric ID of the action's repository. Pinned because a repo can be renamed or deleted and the name reclaimed; the ID detects that the repo behind the name has changed.\",\n \"type\": \"integer\",\n \"minimum\": 1\n },\n \"uses\": {\n \"description\": \"The action's own direct dependencies, as canonical pin keys, so the full dependency graph stays pinned and verifiable end to end. Required for composite actions (which can call other actions); absent for leaf actions that have no dependencies of their own.\",\n \"type\": \"array\",\n \"items\": { \"$ref\": \"#/$defs/pin\" }\n }\n }\n }\n }\n}\n" diff --git a/go/pkg/lockfile/uses.go b/go/pkg/lockfile/uses.go index 96ef4d7..7505edc 100644 --- a/go/pkg/lockfile/uses.go +++ b/go/pkg/lockfile/uses.go @@ -110,6 +110,9 @@ func splitUsesRef(uses string) *usesRef { return nil } ref := atParts[1] + if !isValidRef(ref) { + return nil + } segments := strings.SplitN(atParts[0], "/", 3) if len(segments) < 2 || segments[0] == "" || segments[1] == "" { diff --git a/go/pkg/lockfile/uses_test.go b/go/pkg/lockfile/uses_test.go index 52260be..41e7692 100644 --- a/go/pkg/lockfile/uses_test.go +++ b/go/pkg/lockfile/uses_test.go @@ -49,6 +49,7 @@ func TestParseActionRef(t *testing.T) { {name: "dot owner segment", input: "./repo@v1", wantNil: true}, // Refs are not validated beyond structural checks (colons for pin // grammar). The workflow parser accepts any characters in refs. + {name: "ref with colon rejected", input: "a/b@v1:foo", wantNil: true}, } for _, tt := range tests { diff --git a/schema/lockfile-v0.0.1.json b/schema/lockfile-v0.0.1.json index 49890c4..0620762 100644 --- a/schema/lockfile-v0.0.1.json +++ b/schema/lockfile-v0.0.1.json @@ -29,7 +29,7 @@ "pin": { "description": "Canonical dependency pin: OWNER/REPO@REF:ALGO-HEX (e.g. actions/checkout@v4:sha1-abc123...).", "type": "string", - "pattern": "^[^/@]+/[^/@]+@[^:]+:[a-z0-9]+-[a-f0-9]+$" + "pattern": "^[^/@:]+/[^/@:]+@[^:]+:(sha1|sha256)-[a-f0-9]+$" }, "action": { "description": "Resolved metadata for a single pinned action.", From 887d8dec27400eee6197f095dd6b576a4d6ee2c1 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 23 Jun 2026 12:35:26 -0500 Subject: [PATCH 15/15] lockfile: skip ref-mismatch check for full-SHA pin keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a transitive dep is pinned by commit (key is owner/repo@), the body's ref field holds the discovered symbolic ref (e.g. v9.2.0). This mismatch is expected and correct — skip the check when pin.Ref is a full SHA. --- go/pkg/lockfile/lockfile.go | 5 +++ go/pkg/lockfile/lockfile_test.go | 75 ++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/go/pkg/lockfile/lockfile.go b/go/pkg/lockfile/lockfile.go index 1d8d27a..1c47375 100644 --- a/go/pkg/lockfile/lockfile.go +++ b/go/pkg/lockfile/lockfile.go @@ -520,6 +520,11 @@ func rejectKeyRefMismatch(pinKey, action *yaml.Node) *ParseError { key := action.Content[j] val := action.Content[j+1] if key.Value == "ref" && val.Value != "" && val.Value != pin.Ref { + // When the pin key ref is a full SHA, the body's ref is the + // discovered symbolic ref (tag/branch) — mismatch is expected. + if IsFullSha(pin.Ref) { + continue + } return &ParseError{ Line: val.Line, Column: val.Column, diff --git a/go/pkg/lockfile/lockfile_test.go b/go/pkg/lockfile/lockfile_test.go index e4d2520..a52f7e5 100644 --- a/go/pkg/lockfile/lockfile_test.go +++ b/go/pkg/lockfile/lockfile_test.go @@ -2,6 +2,7 @@ package lockfile import ( "errors" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -482,6 +483,80 @@ dependencies: require.NoError(t, err) } +func TestParse_FullSHAPinKeyRefMismatchAllowed(t *testing.T) { + // When the pin key ref is a full SHA (transitive dep pinned by commit), + // the body's ref is the discovered symbolic ref — mismatch is expected. + cases := []struct { + name string + yaml string + }{ + { + name: "40-char SHA key with semver body ref", + yaml: "version: v0.0.2\ndependencies:\n" + + " golangci/golangci-lint-action@1e7e51e771db61008b38414a730f564565cf7c20:\n" + + " ref: v9.2.0\n" + + " commit: sha1-1e7e51e771db61008b38414a730f564565cf7c20\n" + + " owner_id: 1\n repo_id: 1\n", + }, + { + name: "64-char SHA key with branch body ref", + yaml: "version: v0.0.2\ndependencies:\n" + + " actions/checkout@" + strings.Repeat("ab", 32) + ":\n" + + " ref: main\n" + + " commit: sha256-" + strings.Repeat("ab", 32) + "\n" + + " owner_id: 1\n repo_id: 1\n", + }, + { + name: "40-char SHA key with branch body ref", + yaml: "version: v0.0.2\ndependencies:\n" + + " org/action@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:\n" + + " ref: releases/v3\n" + + " commit: sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" + + " owner_id: 42\n repo_id: 99\n", + }, + { + name: "symbolic ref still enforced (not a full SHA)", + yaml: "version: v0.0.2\ndependencies:\n" + + " actions/checkout@v4:\n" + + " ref: v4\n" + + " commit: sha1-11bd71901bbe5b1630ceea73d27597364c9af683\n" + + " owner_id: 1\n repo_id: 1\n", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f, err := Parse([]byte(tc.yaml)) + require.NoError(t, err) + assert.Len(t, f.Dependencies, 1) + }) + } +} + +func TestParse_SymbolicRefMismatchStillRejected(t *testing.T) { + // Non-SHA pin key refs must still match the body ref. + cases := []struct { + name string + pinRef string + bodyRef string + }{ + {"semver mismatch", "v4", "v3"}, + {"branch mismatch", "main", "develop"}, + {"partial SHA not exempt", "1e7e51e", "v9.2.0"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + y := "version: v0.0.2\ndependencies:\n" + + " actions/checkout@" + tc.pinRef + ":\n" + + " ref: " + tc.bodyRef + "\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(), "does not match pin key ref") + }) + } +} + func TestParse_FullSHARefCommitMismatchRejected(t *testing.T) { // Pin key ref is a full SHA but commit encodes a different digest. input := `version: v0.0.2