Repository navigation
Parse pin refs into validated types - #35
Open
nodeselector wants to merge 3 commits into
Open
nodeselector wants to merge 3 commits into
nodeselector wants to merge 3 commits into
Conversation
The text after "@" in pin keys and `uses:` values, and the recorded entry ref, were exposed as raw strings with only a non-empty/no-colon check. Parse them once into explicit types instead: - PinRef: either a CommitID (full 40/64-hex SHA, lowercased) or a RefName. - RefName: a short tag/branch name valid per git check-ref-format --allow-onelevel, not SHA-shaped, without a refs/ prefix or leading '-'. TagRef()/BranchRef() give the qualified names for exact lookup. - Action.Ref is a RefName; Parse rejects entry refs that are invalid or that differ from a name-keyed pin key (ValidateEntryRef). - ParseActionRefStrict / ParseReusableWorkflowRefStrict report invalid refs instead of folding them into nil. - v0.0.1 entries with no usable tag/branch name fail with ErrRefNeedsRelock. Tighten the v0.0.3 schema ref constraints to match, add parity tests against the schema and git check-ref-format, and add fuzzers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ParseActionRef and ParseReusableWorkflowRef now return (nil, *RefError) for a repository-shaped uses: value whose ref is invalid, instead of nil. Drop the separate Strict variants. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Ref-validation gaps can still skip invalid references or return inconsistent dependency metadata.
5 open issues
1 file not reviewed
go/pkg/lockfile/schema_gen.go: Generated file
What changed in this PR
Introduces validated ref types throughout the Go lockfile API, separating commit hashes from tag and branch names.
Changes:
- Replaces string refs with typed values and validation errors.
- Adds Strict
uses:parsers and legacy migration checks. - Tightens schema constraints and adds validation, parity, and fuzz tests.
| File | Description |
|---|---|
| schema/lockfile-v0.0.3.json | Tightens entry-ref constraints. |
| README.md | Documents typed refs and compatibility changes. |
| go/pkg/lockfile/uses.go | Adds typed refs and Strict parsers. |
| go/pkg/lockfile/uses_test.go | Updates assertions for typed refs. |
| go/pkg/lockfile/schema_gen.go | Refreshes the embedded schema. |
| go/pkg/lockfile/pinref.go | Defines validated types, parsers, and errors. |
| go/pkg/lockfile/pinref_test.go | Adds validation, parity, serialization, and fuzz tests. |
| go/pkg/lockfile/pin.go | Uses typed refs in pin parsing and lookup keys. |
| go/pkg/lockfile/pin_test.go | Tests typed refs and SHA canonicalization. |
| go/pkg/lockfile/lockfile.go | Validates typed dependency refs. |
| go/pkg/lockfile/lockfile_test.go | Updates ref validation expectations. |
| go/pkg/lockfile/doc.go | Explains typed refs and exact lookup. |
| go/pkg/lockfile/compat.go | Migrates legacy refs and detects unresolved names. |
| go/pkg/lockfile/compat_test.go | Adapts legacy assertions to typed refs. |
Review effort: Max
| // A ref that is not a valid tag/branch name (for example a | ||
| // SHA-only legacy key) stays zero; rejectLegacyUnresolvedRefs | ||
| // reports it for in-scope entries. | ||
| if n, err := ParseRefName(ref); err == nil { |
| Line: pinKey.Line, | ||
| Column: pinKey.Column, | ||
| Msg: fmt.Sprintf("dependency %q: %v", pinKey.Value, ErrRefNeedsRelock), | ||
| err: ErrRefNeedsRelock, |
| if key.Value != "ref" { | ||
| continue | ||
| } | ||
| if _, err := ValidateEntryRef(pin.Ref, val.Value); err != nil { |
| if err != nil { | ||
| return RefName{}, err | ||
| } | ||
| if kn, ok := key.Name(); ok && kn != n { |
|
|
||
| if uses == "" || containsControlChars(uses) { | ||
| return nil | ||
| return nil, nil |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.