diff --git a/.github/actions/fork-review-check/README.md b/.github/actions/fork-review-check/README.md new file mode 100644 index 00000000..b7db9a37 --- /dev/null +++ b/.github/actions/fork-review-check/README.md @@ -0,0 +1,71 @@ +# fork-review-check + +CLI behind the fork review check in [loft-sh/architecture `engineering-enablement/designs/fork-review-check.md`](https://github.com/loft-sh/architecture/blob/main/engineering-enablement/designs/fork-review-check.md). It verifies that every fork commit a product build consumes reached its loft-sh fork through a merged pull request approved by someone other than its author. + +This is build step 1 of the design: the CLI, run read-only. The workflow and release receipt that make it a required check (step 4) build on it. + +## Rules + +For every fork commit S the build consumes, over the range from the trusted upstream base to S: + +1. S is the approved final head, or the merge result, of a merged fork PR. +2. Every commit in the range must be an ancestor of an approved final head, that head itself, or a verified `web-flow` merge result of the merged PR. The approval must be after the last force push, by someone with write or higher permission on the fork who is neither the PR author nor the commit author. For a verified merge commit recorded as the PR's merge result with `web-flow` as committer, the code author is the PR author, so the approver may also be the merger. Rewritten commits from rebase merges that cannot be tied to the approved head fail closed. +3. With `-as-of`, every approval predates that time, normally the product release. +4. The check fails and names every commit that breaks a rule. + +The upstream comes from the `fork-upstream` repository property, never from GitHub's fork parent. A fork without it fails. The base is: + +| Pin | Base | +| --- | --- | +| Fork release tag (`v0.16.5.vcluster.1`, `v1.96.4-loft.3`) | The upstream tag before the suffix (`v0.16.5`), which must be an ancestor of S | +| Commit or Go pseudo-version | The merge base of S with the upstream default branch | + +A tag without a fork suffix, a missing upstream tag, a base that is not an ancestor, or a range over 250 commits fails. + +## Usage + +```bash +export GITHUB_TOKEN=... # read access to the forks; permission lookups need more, see below + +# One fork ref: a tag or a full commit SHA. +go run . ref -repo loft-sh/kine -ref v0.16.5.vcluster.1 + +# Every loft-sh fork in the effective Go module graph of one or more modules. +GOPRIVATE=github.com/loft-sh/* go run . go -dir /path/to/vnode-internal +``` + +| Flag | Meaning | +| --- | --- | +| `-repo`, `-ref` | `ref` mode: the fork and the tag or commit to check | +| `-dir` | `go` mode: module directory, repeatable. S is the commit Go downloaded (`go mod download -json`, `Origin.Hash`), not today's tag | +| `-vendor-verifier` | `go` mode: trusted command that verifies the committed vendor tree in a temporary checkout. Required when a module has `vendor/` | +| `-upstream` | `fork=upstream[@branch]`, used only while a fork has no `fork-upstream` property. The missing property still fails; the override only keeps the per-commit evidence coming | +| `-as-of` | RFC 3339 time for rule 3 | +| `-receipt` | Write the JSON receipt to this path | +| `-advisory` | Report failures as warnings and exit 0 | + +Exit codes: 0 pass or advisory, 1 a rule failed, 2 the check could not run. + +`go` mode needs `GOPRIVATE=github.com/loft-sh/*` so Go fetches loft-sh modules from git and reports the origin commit; without an origin the module fails. + +When a module contains `vendor/`, the downloaded module graph alone does not verify the code compiled from that directory. The check requires `-vendor-verifier`, for example `-vendor-verifier 'bash hack/verify-vendor.sh'` for a repository that provides that verifier. It also requires a clean Git checkout, including no ignored files in `vendor/`. The command runs in a temporary copy of the repository and must fail when the committed vendor tree differs from the generated tree. It is trusted code, not a sandbox: run this check only with a trusted command and repository. A missing or failing verifier fails the check. The consumer is responsible for selecting a verifier that preserves its vendor symlinks and checks all relevant files. + +Workspace-level `vendor/` is not supported and fails closed. Use a module without workspace vendoring until a workspace verifier is supported. + +## Receipt + +The JSON receipt lists each input with its hash, each fork's `fork-upstream` value and source, the base, and every fork commit with its PR, approver, approval time and the approver's permission as read at check time. The permission endpoint returns current access only, so the receipt is the record and a later rerun can differ. + +## Token + +Reading PRs, reviews and compares on public forks works with any token. The approver permission lookup (`repos/{owner}/{repo}/collaborators/{user}/permission`) is the open question for the required workflow: `test-fork-review-check.yaml` runs the CLI against kine, tailscale and fuse with the workflow token to show whether that token can read it cross-repo. + +## Not yet covered + +Container images by digest, release binaries by sha256, source clones, and the fail-closed net for unknown fork references are step 4 of the design. + +## Tests + +```bash +make test-fork-review-check +``` diff --git a/.github/actions/fork-review-check/src/check.go b/.github/actions/fork-review-check/src/check.go new file mode 100644 index 00000000..d25278da --- /dev/null +++ b/.github/actions/fork-review-check/src/check.go @@ -0,0 +1,453 @@ +package main + +import ( + "context" + "fmt" + "regexp" + "sort" + "strings" + "time" +) + +// maxCompareCommits is the largest fork range the check accepts. A bigger +// compare is reported as truncated and fails, so no commit is skipped silently. +const maxCompareCommits = 250 + +// forkTagRe splits a fork release tag into the upstream release it names and +// the fork suffix: v0.16.5.vcluster.1 (kine) and v1.96.4-loft.3 (tailscale) +// both name an upstream tag (v0.16.5, v1.96.4). +var forkTagRe = regexp.MustCompile(`^(v\d+\.\d+\.\d+)(\.vcluster\.\d+|-loft\.\d+)$`) + +// githubMergeCommitter is the committer GitHub records on commits it writes +// itself, such as the merge commit of a pull request merged in the UI or API. +const githubMergeCommitter = "web-flow" + +// writePermissions are the collaborator permission values whose approvals +// count toward required reviews. The endpoint reports maintain as write. +var writePermissions = map[string]bool{"admin": true, "write": true} + +// pin is one fork commit a build consumes. +type pin struct { + Repo string // fork repository, owner/name + Ref string // what the build pins: a tag, a Go version or a commit + Tag string // fork release tag; empty for commit and pseudo-version pins + Commit string // S, the commit the build consumes +} + +// upstreamOverride stands in for a missing fork-upstream property so a +// read-only run can still produce per-commit evidence. The missing property +// is reported as a failure either way. +type upstreamOverride struct { + Repo string // upstream repository, owner/name + Ref string // upstream branch for commit pins; empty means its default branch +} + +type baseRecord struct { + Rule string `json:"rule"` // upstream-tag or merge-base + Ref string `json:"ref"` + Commit string `json:"commit"` +} + +type pullRecord struct { + Number int `json:"number"` + URL string `json:"url"` + Author string `json:"author"` + Head string `json:"head"` + MergeCommit string `json:"merge_commit"` + MergedAt time.Time `json:"merged_at"` +} + +type approvalRecord struct { + Approver string `json:"approver"` + SubmittedAt time.Time `json:"submitted_at"` + Commit string `json:"commit"` + Permission string `json:"permission"` // read at check time; the endpoint reports current access only +} + +type commitResult struct { + SHA string `json:"sha"` + Author string `json:"author"` + Subject string `json:"subject"` + Merge bool `json:"merge"` + Pull *pullRecord `json:"pull,omitempty"` + Approval *approvalRecord `json:"approval,omitempty"` + Pass bool `json:"pass"` + Failures []string `json:"failures,omitempty"` +} + +type reviewedTree struct { + Pull *pullRecord `json:"pull,omitempty"` + Via string `json:"via,omitempty"` // head or merge + Pass bool `json:"pass"` +} + +type forkResult struct { + Repo string `json:"repo"` + Ref string `json:"ref"` + Commit string `json:"commit"` + ForkUpstream string `json:"fork_upstream"` + ForkUpstreamSource string `json:"fork_upstream_source"` // property, override or missing + Base *baseRecord `json:"base,omitempty"` + ReviewedTree *reviewedTree `json:"reviewed_tree,omitempty"` + Commits []commitResult `json:"commits"` + Failures []string `json:"failures,omitempty"` + Pass bool `json:"pass"` +} + +// pullData caches what the rules read per pull request. +type pullData struct { + reviews []reviewInfo + lastForcePush time.Time +} + +// checker evaluates the design's rules against live GitHub data. +type checker struct { + api forkAPI + asOf time.Time // rule 3 cutoff; zero skips the rule + overrides map[string]upstreamOverride + + pulls map[string]*pullData + perms map[string]string +} + +func newChecker(api forkAPI, asOf time.Time, overrides map[string]upstreamOverride) *checker { + return &checker{ + api: api, + asOf: asOf, + overrides: overrides, + pulls: map[string]*pullData{}, + perms: map[string]string{}, + } +} + +// checkFork applies the rules to every fork commit between the trusted +// upstream base and p.Commit. +func (c *checker) checkFork(ctx context.Context, p pin) forkResult { + res := forkResult{Repo: p.Repo, Ref: p.Ref, Commit: p.Commit, Commits: []commitResult{}} + + upstream, err := c.api.ForkUpstream(ctx, p.Repo) + if err != nil { + res.Failures = append(res.Failures, fmt.Sprintf("read fork-upstream property: %v", err)) + return res + } + override, hasOverride := c.overrides[p.Repo] + switch { + case upstream != "": + res.ForkUpstream, res.ForkUpstreamSource = upstream, "property" + case hasOverride: + res.ForkUpstream, res.ForkUpstreamSource = override.Repo, "override" + res.Failures = append(res.Failures, "repo has no fork-upstream property; an org owner sets it, and the result below used the override "+override.Repo) + default: + res.ForkUpstreamSource = "missing" + res.Failures = append(res.Failures, "repo has no fork-upstream property; an org owner sets it to the upstream repository, then the check re-runs") + return res + } + + base, cmp, failure := c.resolveBase(ctx, p, res.ForkUpstream, override.Ref) + if failure != "" { + res.Failures = append(res.Failures, failure) + return res + } + res.Base = base + + for _, ci := range cmp.Commits { + res.Commits = append(res.Commits, c.checkCommit(ctx, p.Repo, ci)) + } + + if len(cmp.Commits) > 0 { + res.ReviewedTree = c.checkReviewedTree(ctx, p.Repo, commitOf(cmp.Commits, p.Commit)) + if !res.ReviewedTree.Pass { + res.Failures = append(res.Failures, fmt.Sprintf("%s is not the approved final head or merge result of a merged fork pull request (rule 1)", short(p.Commit))) + } + } + + res.Pass = len(res.Failures) == 0 + for _, cr := range res.Commits { + if !cr.Pass { + res.Pass = false + } + } + return res +} + +// resolveBase finds the trusted upstream base and the fork range above it. A +// tag pin names its base (the tag before the fork suffix), which must be an +// ancestor of S. A commit pin has no tag, so its base is the merge base with +// the upstream branch. +func (c *checker) resolveBase(ctx context.Context, p pin, upstream, upstreamRef string) (*baseRecord, comparison, string) { + var base *baseRecord + var cmp comparison + var err error + + if p.Tag != "" { + m := forkTagRe.FindStringSubmatch(p.Tag) + if m == nil { + return nil, cmp, fmt.Sprintf("fork tag %s has no fork suffix (.vcluster.N or -loft.N), so it names no upstream base; pin a suffixed fork release tag", p.Tag) + } + baseSHA, err := c.api.ResolveTag(ctx, upstream, m[1]) + if err != nil { + return nil, cmp, fmt.Sprintf("resolve upstream tag %s in %s: %v", m[1], upstream, err) + } + if baseSHA == "" { + return nil, cmp, fmt.Sprintf("upstream %s has no tag %s, so the base is ambiguous", upstream, m[1]) + } + base = &baseRecord{Rule: "upstream-tag", Ref: upstream + "@" + m[1], Commit: baseSHA} + cmp, err = c.api.Compare(ctx, p.Repo, baseSHA, p.Commit) + if err != nil { + return nil, cmp, fmt.Sprintf("compare %s...%s: %v", short(baseSHA), short(p.Commit), err) + } + if cmp.Status != "ahead" && cmp.Status != "identical" { + return nil, cmp, fmt.Sprintf("upstream base %s (%s) is not an ancestor of %s (compare status %s)", m[1], short(baseSHA), short(p.Commit), cmp.Status) + } + } else { + ref := upstreamRef + if ref == "" { + info, err := c.api.Repo(ctx, upstream) + if err != nil { + return nil, cmp, fmt.Sprintf("read upstream %s: %v", upstream, err) + } + ref = info.DefaultBranch + } + owner := strings.SplitN(upstream, "/", 2)[0] + cmp, err = c.api.Compare(ctx, p.Repo, owner+":"+ref, p.Commit) + if err != nil { + return nil, cmp, fmt.Sprintf("compare %s:%s...%s: %v", owner, ref, short(p.Commit), err) + } + if cmp.MergeBase == "" { + return nil, cmp, fmt.Sprintf("%s shares no history with %s@%s", short(p.Commit), upstream, ref) + } + base = &baseRecord{Rule: "merge-base", Ref: upstream + "@" + ref, Commit: cmp.MergeBase} + } + + if cmp.Total > maxCompareCommits { + return nil, cmp, fmt.Sprintf("compare from base %s is truncated: %d commits, the limit is %d", short(base.Commit), cmp.Total, maxCompareCommits) + } + if len(cmp.Commits) != cmp.Total { + return nil, cmp, fmt.Sprintf("compare from base %s returned %d of %d commits", short(base.Commit), len(cmp.Commits), cmp.Total) + } + return base, cmp, "" +} + +// checkCommit applies rule 2 to one fork commit: it belongs to a merged fork +// pull request with a qualifying approval. +func (c *checker) checkCommit(ctx context.Context, repo string, ci commitInfo) commitResult { + res := commitResult{SHA: ci.SHA, Author: ci.Author, Subject: ci.Subject, Merge: ci.Merge} + + pulls, err := c.mergedPulls(ctx, repo, ci.SHA) + if err != nil { + res.Failures = []string{fmt.Sprintf("list pull requests for the commit: %v", err)} + return res + } + if len(pulls) == 0 { + res.Failures = []string{"no merged pull request in " + repo + " contains this commit (direct push or bypass)"} + return res + } + + for _, pr := range pulls { + covered, reason := c.commitCoveredByHead(ctx, repo, ci, pr) + if !covered { + res.Failures = append(res.Failures, reason) + continue + } + approval, reasons := c.qualifyingApproval(ctx, repo, pr, effectiveAuthor(ci, pr)) + if approval == nil { + res.Failures = append(res.Failures, reasons...) + continue + } + res.Pull, res.Approval, res.Pass, res.Failures = pullRecordOf(pr), approval, true, nil + return res + } + return res +} + +// commitCoveredByHead proves that an associated commit is in the approved +// final head, or is the exact verified GitHub merge result for that PR. +func (c *checker) commitCoveredByHead(ctx context.Context, repo string, ci commitInfo, pr pullInfo) (bool, string) { + if ci.SHA != "" && ci.SHA == pr.HeadSHA { + return true, "" + } + if ci.SHA != "" && ci.SHA == pr.MergeCommit { + if isGitHubMergeResult(ci, pr) { + return true, "" + } + return false, fmt.Sprintf("#%d: merge result lacks a verified signature or web-flow committer", pr.Number) + } + if ci.SHA == "" || pr.HeadSHA == "" { + return false, fmt.Sprintf("#%d: cannot verify commit ancestry without a commit and final head", pr.Number) + } + cmp, err := c.api.Compare(ctx, repo, ci.SHA, pr.HeadSHA) + if err != nil { + return false, fmt.Sprintf("#%d: compare commit to final head: %v", pr.Number, err) + } + if (cmp.Status != "ahead" && cmp.Status != "identical") || !strings.EqualFold(cmp.MergeBase, ci.SHA) { + return false, fmt.Sprintf("#%d: commit is not an ancestor of the final head (compare status %s, merge base %s)", pr.Number, cmp.Status, short(cmp.MergeBase)) + } + return true, "" +} + +// checkReviewedTree applies rule 1 to S: it is the final head or the merge +// result of a merged fork pull request that carries a qualifying approval. +func (c *checker) checkReviewedTree(ctx context.Context, repo string, ci commitInfo) *reviewedTree { + sha := ci.SHA + pulls, err := c.mergedPulls(ctx, repo, sha) + if err != nil { + return &reviewedTree{} + } + for _, pr := range pulls { + via := "" + switch sha { + case pr.HeadSHA: + via = "head" + case pr.MergeCommit: + if isGitHubMergeResult(ci, pr) { + via = "merge" + } else { + continue + } + default: + continue + } + if approval, _ := c.qualifyingApproval(ctx, repo, pr, effectiveAuthor(ci, pr)); approval != nil { + return &reviewedTree{Pull: pullRecordOf(pr), Via: via, Pass: true} + } + } + return &reviewedTree{} +} + +// qualifyingApproval returns the earliest approval on pr that meets every +// condition, or the reasons each approval fell short. +func (c *checker) qualifyingApproval(ctx context.Context, repo string, pr pullInfo, commitAuthor string) (*approvalRecord, []string) { + data, err := c.pullData(ctx, repo, pr.Number) + if err != nil { + return nil, []string{fmt.Sprintf("#%d: read reviews: %v", pr.Number, err)} + } + + var reasons []string + var approvals []reviewInfo + for _, r := range data.reviews { + if r.State == "APPROVED" { + approvals = append(approvals, r) + } + } + if len(approvals) == 0 { + return nil, []string{fmt.Sprintf("#%d: merged without an approval", pr.Number)} + } + sort.SliceStable(approvals, func(i, j int) bool { return approvals[i].SubmittedAt.Before(approvals[j].SubmittedAt) }) + + for _, r := range approvals { + reason := "" + switch { + case r.CommitID != pr.HeadSHA: + reason = fmt.Sprintf("approval by %s is on %s, not the final head %s", r.User, short(r.CommitID), short(pr.HeadSHA)) + case strings.EqualFold(r.User, pr.Author): + reason = fmt.Sprintf("approval by %s is by the pull request author", r.User) + case commitAuthor == "": + reason = fmt.Sprintf("approval by %s cannot be shown to be by a non-author: the commit author has no GitHub account", r.User) + case strings.EqualFold(r.User, commitAuthor): + reason = fmt.Sprintf("approval by %s is by the commit author", r.User) + case !data.lastForcePush.IsZero() && !r.SubmittedAt.After(data.lastForcePush): + reason = fmt.Sprintf("approval by %s predates the last force push (%s)", r.User, data.lastForcePush.Format(time.RFC3339)) + case !c.asOf.IsZero() && !r.SubmittedAt.Before(c.asOf): + reason = fmt.Sprintf("approval by %s at %s is not before the release (%s) (rule 3)", r.User, r.SubmittedAt.Format(time.RFC3339), c.asOf.Format(time.RFC3339)) + } + if reason != "" { + reasons = append(reasons, fmt.Sprintf("#%d: %s", pr.Number, reason)) + continue + } + + perm, err := c.permission(ctx, repo, r.User) + if err != nil { + reasons = append(reasons, fmt.Sprintf("#%d: read permission of %s: %v", pr.Number, r.User, err)) + continue + } + if !writePermissions[perm] { + reasons = append(reasons, fmt.Sprintf("#%d: approval by %s does not count: %s permission, write or higher needed", pr.Number, r.User, perm)) + continue + } + return &approvalRecord{Approver: r.User, SubmittedAt: r.SubmittedAt, Commit: r.CommitID, Permission: perm}, nil + } + return nil, reasons +} + +// mergedPulls lists the merged pull requests into repo that contain sha. +func (c *checker) mergedPulls(ctx context.Context, repo, sha string) ([]pullInfo, error) { + all, err := c.api.PullsWithCommit(ctx, repo, sha) + if err != nil { + return nil, err + } + var merged []pullInfo + for _, pr := range all { + if !pr.MergedAt.IsZero() && strings.EqualFold(pr.BaseRepo, repo) { + merged = append(merged, pr) + } + } + sort.Slice(merged, func(i, j int) bool { return merged[i].Number < merged[j].Number }) + return merged, nil +} + +func (c *checker) pullData(ctx context.Context, repo string, number int) (*pullData, error) { + key := fmt.Sprintf("%s#%d", repo, number) + if d, ok := c.pulls[key]; ok { + return d, nil + } + reviews, err := c.api.Reviews(ctx, repo, number) + if err != nil { + return nil, err + } + forced, err := c.api.LastForcePush(ctx, repo, number) + if err != nil { + return nil, err + } + d := &pullData{reviews: reviews, lastForcePush: forced} + c.pulls[key] = d + return d, nil +} + +func (c *checker) permission(ctx context.Context, repo, user string) (string, error) { + key := repo + "/" + strings.ToLower(user) + if p, ok := c.perms[key]; ok { + return p, nil + } + p, err := c.api.Permission(ctx, repo, user) + if err != nil { + return "", err + } + c.perms[key] = p + return p, nil +} + +func pullRecordOf(pr pullInfo) *pullRecord { + return &pullRecord{Number: pr.Number, URL: pr.URL, Author: pr.Author, Head: pr.HeadSHA, MergeCommit: pr.MergeCommit, MergedAt: pr.MergedAt} +} + +func commitOf(commits []commitInfo, sha string) commitInfo { + for _, ci := range commits { + if ci.SHA == sha { + return ci + } + } + return commitInfo{SHA: sha} +} + +// effectiveAuthor is the author the non-author rule compares approvers with. +// The merge commit GitHub writes when a pull request merges carries the +// merger as author but no code of its own, so for that commit the code author +// is the pull request author. Any other commit keeps its git author, merge +// commits pushed by hand included. +func effectiveAuthor(ci commitInfo, pr pullInfo) string { + if ci.Merge && isGitHubMergeResult(ci, pr) { + return pr.Author + } + return ci.Author +} + +func isGitHubMergeResult(ci commitInfo, pr pullInfo) bool { + return ci.SHA != "" && ci.SHA == pr.MergeCommit && ci.Committer == githubMergeCommitter && ci.Verified +} + +func short(sha string) string { + if len(sha) > 12 { + return sha[:12] + } + return sha +} diff --git a/.github/actions/fork-review-check/src/check_test.go b/.github/actions/fork-review-check/src/check_test.go new file mode 100644 index 00000000..61ff7c5c --- /dev/null +++ b/.github/actions/fork-review-check/src/check_test.go @@ -0,0 +1,511 @@ +package main + +import ( + "context" + "fmt" + "strings" + "testing" + "time" +) + +// fakeAPI serves canned GitHub data keyed the way forkAPI asks for it. +type fakeAPI struct { + repos map[string]repoInfo + upstreams map[string]string + tags map[string]string // repo@tag -> sha + compares map[string]comparison + pulls map[string][]pullInfo // repo@sha + reviews map[string][]reviewInfo + forcePush map[string]time.Time + perms map[string]string // repo/user + permErr error + permCalled int +} + +func newFake() *fakeAPI { + return &fakeAPI{ + repos: map[string]repoInfo{}, + upstreams: map[string]string{}, + tags: map[string]string{}, + compares: map[string]comparison{}, + pulls: map[string][]pullInfo{}, + reviews: map[string][]reviewInfo{}, + forcePush: map[string]time.Time{}, + perms: map[string]string{}, + } +} + +func (f *fakeAPI) Repo(_ context.Context, repo string) (repoInfo, error) { + r, ok := f.repos[repo] + if !ok { + return repoInfo{}, fmt.Errorf("no repo %s", repo) + } + return r, nil +} + +func (f *fakeAPI) ForkUpstream(_ context.Context, repo string) (string, error) { + return f.upstreams[repo], nil +} + +func (f *fakeAPI) ResolveTag(_ context.Context, repo, tag string) (string, error) { + return f.tags[repo+"@"+tag], nil +} + +func (f *fakeAPI) Compare(_ context.Context, repo, base, head string) (comparison, error) { + c, ok := f.compares[repo+"@"+base+"..."+head] + if !ok { + return comparison{}, fmt.Errorf("no compare %s %s...%s", repo, base, head) + } + return c, nil +} + +func (f *fakeAPI) PullsWithCommit(_ context.Context, repo, sha string) ([]pullInfo, error) { + return f.pulls[repo+"@"+sha], nil +} + +func (f *fakeAPI) Reviews(_ context.Context, repo string, number int) ([]reviewInfo, error) { + return f.reviews[fmt.Sprintf("%s#%d", repo, number)], nil +} + +func (f *fakeAPI) LastForcePush(_ context.Context, repo string, number int) (time.Time, error) { + return f.forcePush[fmt.Sprintf("%s#%d", repo, number)], nil +} + +func (f *fakeAPI) Permission(_ context.Context, repo, user string) (string, error) { + f.permCalled++ + if f.permErr != nil { + return "", f.permErr + } + if p, ok := f.perms[repo+"/"+user]; ok { + return p, nil + } + return "none", nil +} + +const ( + fork = "loft-sh/kine" + upstream = "k3s-io/kine" + baseSHA = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + c1 = "1111111111111111111111111111111111111111" + c2 = "2222222222222222222222222222222222222222" + head = "eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee" + merge = "dddddddddddddddddddddddddddddddddddddddd" + tag = "v0.16.5.vcluster.1" +) + +var t0 = time.Date(2026, 10, 1, 12, 0, 0, 0, time.UTC) + +// reviewedFork is kine at a tag with two fork commits, both from PR #22, +// approved at its final head by bob (write) after alice opened it. The tag +// points at the PR's merge commit, written by GitHub. +func reviewedFork() *fakeAPI { + f := newFake() + f.upstreams[fork] = upstream + f.tags[upstream+"@v0.16.5"] = baseSHA + f.tags[fork+"@"+tag] = merge + f.compares[fork+"@"+baseSHA+"..."+merge] = comparison{ + Status: "ahead", Total: 3, + Commits: []commitInfo{ + {SHA: c1, Author: "alice", Committer: "alice", Subject: "add kerberos"}, + {SHA: head, Author: "alice", Committer: "alice", Subject: "fix review"}, + {SHA: merge, Author: "bob", Committer: githubMergeCommitter, Verified: true, Subject: "Merge pull request #22", Merge: true}, + }, + } + f.compares[fork+"@"+c1+"..."+head] = comparison{Status: "ahead", MergeBase: c1} + pr22 := pullInfo{Number: 22, URL: "https://github.com/loft-sh/kine/pull/22", Author: "alice", BaseRepo: fork, HeadSHA: head, MergeCommit: merge, MergedAt: t0.Add(2 * time.Hour)} + for _, sha := range []string{c1, head, merge} { + f.pulls[fork+"@"+sha] = []pullInfo{pr22} + } + f.reviews[fork+"#22"] = []reviewInfo{{User: "bob", State: "APPROVED", CommitID: head, SubmittedAt: t0.Add(time.Hour)}} + f.perms[fork+"/bob"] = "write" + return f +} + +func tagPin() pin { return pin{Repo: fork, Ref: tag, Tag: tag, Commit: merge} } + +func runCheck(f *fakeAPI, p pin) forkResult { + return newChecker(f, time.Time{}, nil).checkFork(context.Background(), p) +} + +func failuresOf(r forkResult) string { + var all []string + all = append(all, r.Failures...) + for _, c := range r.Commits { + all = append(all, c.Failures...) + } + return strings.Join(all, "\n") +} + +func TestReviewedForkPasses(t *testing.T) { + r := runCheck(reviewedFork(), tagPin()) + if !r.Pass { + t.Fatalf("want pass, got failures:\n%s", failuresOf(r)) + } + if r.Base.Rule != "upstream-tag" || r.Base.Commit != baseSHA { + t.Errorf("base = %+v, want upstream-tag %s", r.Base, baseSHA) + } + if r.ReviewedTree.Via != "merge" || r.ReviewedTree.Pull.Number != 22 { + t.Errorf("reviewed tree = %+v, want merge result of #22", r.ReviewedTree) + } + for _, c := range r.Commits { + if c.Approval == nil || c.Approval.Approver != "bob" || c.Approval.Permission != "write" { + t.Errorf("commit %s approval = %+v, want bob with write", short(c.SHA), c.Approval) + } + } +} + +func TestDirectPushFails(t *testing.T) { + f := reviewedFork() + delete(f.pulls, fork+"@"+c1) + r := runCheck(f, tagPin()) + if r.Pass || r.Commits[0].Pass { + t.Fatal("want the direct push to fail") + } + if !strings.Contains(r.Commits[0].Failures[0], "direct push") { + t.Errorf("failure = %q, want it to name a direct push", r.Commits[0].Failures[0]) + } + if !r.Commits[1].Pass { + t.Error("the reviewed commits should still pass on their own") + } +} + +func TestAssociatedCommitRemovedFromFinalHeadFails(t *testing.T) { + f := reviewedFork() + f.compares[fork+"@"+c1+"..."+head] = comparison{Status: "diverged"} + r := runCheck(f, tagPin()) + if r.Commits[0].Pass { + t.Fatal("a commit associated with the PR but absent from its final head must fail") + } +} + +func TestFinalHeadAncestryLookupErrorFailsClosed(t *testing.T) { + f := reviewedFork() + delete(f.compares, fork+"@"+c1+"..."+head) + r := runCheck(f, tagPin()) + if r.Commits[0].Pass { + t.Fatal("a failed ancestry lookup must not establish that the final head includes the commit") + } +} + +func TestFinalHeadComparisonMustNameCommitAsMergeBase(t *testing.T) { + f := reviewedFork() + f.compares[fork+"@"+c1+"..."+head] = comparison{Status: "ahead", MergeBase: baseSHA} + r := runCheck(f, tagPin()) + if r.Commits[0].Pass { + t.Fatal("a comparison whose merge base differs from the commit must fail") + } +} + +func TestUnsignedWebFlowMergeFails(t *testing.T) { + f := reviewedFork() + f.compares[fork+"@"+baseSHA+"..."+merge].Commits[2].Verified = false + r := runCheck(f, tagPin()) + if r.Commits[2].Pass || r.ReviewedTree.Pass { + t.Fatal("an unsigned merge result must not be treated as a GitHub-generated merge") + } + ci := f.compares[fork+"@"+baseSHA+"..."+merge].Commits[2] + if author := effectiveAuthor(ci, f.pulls[fork+"@"+merge][0]); author != "bob" { + t.Fatalf("unsigned merge author = %q, want git author bob", author) + } +} + +func TestUnmergedPullDoesNotCount(t *testing.T) { + f := reviewedFork() + open := pullInfo{Number: 30, Author: "alice", BaseRepo: fork, HeadSHA: c1} + f.pulls[fork+"@"+c1] = []pullInfo{open} + r := runCheck(f, tagPin()) + if r.Commits[0].Pass { + t.Fatal("an open pull request must not satisfy rule 2") + } +} + +func TestPullIntoAnotherRepoDoesNotCount(t *testing.T) { + f := reviewedFork() + upstreamPR := pullInfo{Number: 22, Author: "alice", BaseRepo: upstream, HeadSHA: head, MergedAt: t0} + f.pulls[fork+"@"+c1] = []pullInfo{upstreamPR} + r := runCheck(f, tagPin()) + if r.Commits[0].Pass { + t.Fatal("a pull request merged into another repository must not count") + } +} + +func TestSelfApprovalFails(t *testing.T) { + f := reviewedFork() + f.reviews[fork+"#22"] = []reviewInfo{{User: "alice", State: "APPROVED", CommitID: head, SubmittedAt: t0.Add(time.Hour)}} + f.perms[fork+"/alice"] = "admin" + r := runCheck(f, tagPin()) + if r.Pass { + t.Fatal("want the PR author's own approval to fail") + } + if !strings.Contains(failuresOf(r), "pull request author") { + t.Errorf("failures do not name the PR author:\n%s", failuresOf(r)) + } +} + +func TestCommitAuthorApprovalFails(t *testing.T) { + // carol opens a PR carrying a commit dave wrote; dave approves it. + f := reviewedFork() + f.compares[fork+"@"+baseSHA+"..."+merge].Commits[0].Author = "dave" + f.reviews[fork+"#22"] = []reviewInfo{{User: "dave", State: "APPROVED", CommitID: head, SubmittedAt: t0.Add(time.Hour)}} + f.perms[fork+"/dave"] = "write" + r := runCheck(f, tagPin()) + if r.Commits[0].Pass { + t.Fatal("want an approval by the commit author to fail") + } + if !strings.Contains(r.Commits[0].Failures[0], "commit author") { + t.Errorf("failure = %q, want it to name the commit author", r.Commits[0].Failures[0]) + } +} + +func TestApproverMayMergeTheirReview(t *testing.T) { + // bob approved and merged; GitHub's merge commit names bob as author but + // carries no code, so bob's approval still counts for it. + r := runCheck(reviewedFork(), tagPin()) + merged := r.Commits[2] + if !merged.Merge || !merged.Pass { + t.Fatalf("GitHub merge commit by the approver should pass: %+v", merged) + } +} + +func TestHandMadeMergeByApproverFails(t *testing.T) { + f := reviewedFork() + f.compares[fork+"@"+baseSHA+"..."+merge].Commits[2].Committer = "bob" + r := runCheck(f, tagPin()) + if r.Commits[2].Pass { + t.Fatal("a merge commit bob pushed himself is his code; his approval cannot cover it") + } +} + +func TestStaleHeadApprovalFails(t *testing.T) { + f := reviewedFork() + f.reviews[fork+"#22"] = []reviewInfo{{User: "bob", State: "APPROVED", CommitID: c1, SubmittedAt: t0.Add(time.Hour)}} + r := runCheck(f, tagPin()) + if r.Pass { + t.Fatal("want an approval on an earlier head to fail") + } + if !strings.Contains(failuresOf(r), "not the final head") { + t.Errorf("failures do not name the stale head:\n%s", failuresOf(r)) + } +} + +func TestApprovalBeforeForcePushFails(t *testing.T) { + f := reviewedFork() + f.forcePush[fork+"#22"] = t0.Add(90 * time.Minute) + r := runCheck(f, tagPin()) + if r.Pass { + t.Fatal("want an approval older than the last force push to fail") + } + if !strings.Contains(failuresOf(r), "force push") { + t.Errorf("failures do not name the force push:\n%s", failuresOf(r)) + } +} + +func TestReadOnlyApproverFails(t *testing.T) { + f := reviewedFork() + f.perms[fork+"/bob"] = "read" + r := runCheck(f, tagPin()) + if r.Pass { + t.Fatal("want an approver with read permission to fail") + } + if !strings.Contains(failuresOf(r), "read permission") { + t.Errorf("failures do not name the permission:\n%s", failuresOf(r)) + } +} + +func TestPermissionLookupErrorFailsClosed(t *testing.T) { + f := reviewedFork() + f.permErr = fmt.Errorf("403 Resource not accessible by integration") + r := runCheck(f, tagPin()) + if r.Pass { + t.Fatal("a permission lookup error must fail the check") + } + if !strings.Contains(failuresOf(r), "403") { + t.Errorf("failures hide the lookup error:\n%s", failuresOf(r)) + } +} + +func TestPermissionIsReadOncePerApprover(t *testing.T) { + f := reviewedFork() + runCheck(f, tagPin()) + if f.permCalled != 1 { + t.Errorf("permission read %d times, want 1 (cached)", f.permCalled) + } +} + +func TestLaterQualifyingApprovalCounts(t *testing.T) { + f := reviewedFork() + f.reviews[fork+"#22"] = []reviewInfo{ + {User: "alice", State: "APPROVED", CommitID: head, SubmittedAt: t0.Add(30 * time.Minute)}, + {User: "carol", State: "CHANGES_REQUESTED", CommitID: head, SubmittedAt: t0.Add(40 * time.Minute)}, + {User: "bob", State: "APPROVED", CommitID: head, SubmittedAt: t0.Add(time.Hour)}, + } + r := runCheck(f, tagPin()) + if !r.Pass { + t.Fatalf("bob's approval qualifies, want pass:\n%s", failuresOf(r)) + } +} + +func TestNoApprovalFails(t *testing.T) { + f := reviewedFork() + f.reviews[fork+"#22"] = nil + r := runCheck(f, tagPin()) + if !strings.Contains(failuresOf(r), "merged without an approval") { + t.Errorf("failures do not say the PR had no approval:\n%s", failuresOf(r)) + } +} + +func TestApprovalAfterReleaseFails(t *testing.T) { + f := reviewedFork() + r := newChecker(f, t0.Add(30*time.Minute), nil).checkFork(context.Background(), tagPin()) + if r.Pass { + t.Fatal("want an approval after the release to fail (rule 3)") + } + if !strings.Contains(failuresOf(r), "rule 3") { + t.Errorf("failures do not cite rule 3:\n%s", failuresOf(r)) + } +} + +func TestIntermediateCommitIsNotAReviewedTree(t *testing.T) { + // Pinning c1, a commit inside PR #22, fails rule 1 even though #22 is approved. + f := reviewedFork() + f.tags[fork+"@v0.16.5.vcluster.0"] = c1 + f.compares[fork+"@"+baseSHA+"..."+c1] = comparison{Status: "ahead", Total: 1, Commits: []commitInfo{{SHA: c1, Author: "alice", Subject: "add kerberos"}}} + r := runCheck(f, pin{Repo: fork, Ref: "v0.16.5.vcluster.0", Tag: "v0.16.5.vcluster.0", Commit: c1}) + if r.Pass { + t.Fatal("want rule 1 to fail on an intermediate PR commit") + } + if r.ReviewedTree.Pass || !strings.Contains(failuresOf(r), "rule 1") { + t.Errorf("rule 1 not reported:\n%s", failuresOf(r)) + } + if !r.Commits[0].Pass { + t.Error("the commit itself is covered by #22; only rule 1 should fail") + } +} + +func TestFinalHeadIsAReviewedTree(t *testing.T) { + f := reviewedFork() + f.tags[fork+"@v0.16.5.vcluster.0"] = head + f.compares[fork+"@"+baseSHA+"..."+head] = comparison{Status: "ahead", Total: 2, Commits: f.compares[fork+"@"+baseSHA+"..."+merge].Commits[:2]} + r := runCheck(f, pin{Repo: fork, Ref: "v0.16.5.vcluster.0", Tag: "v0.16.5.vcluster.0", Commit: head}) + if !r.Pass || r.ReviewedTree.Via != "head" { + t.Fatalf("want the approved final head to pass via head: %+v\n%s", r.ReviewedTree, failuresOf(r)) + } +} + +func TestUnsuffixedTagFails(t *testing.T) { + r := runCheck(reviewedFork(), pin{Repo: fork, Ref: "v0.14.5", Tag: "v0.14.5", Commit: merge}) + if r.Pass || !strings.Contains(failuresOf(r), "no fork suffix") { + t.Fatalf("want an unsuffixed fork tag to fail:\n%s", failuresOf(r)) + } +} + +func TestLoftSuffixNamesUpstreamTag(t *testing.T) { + m := forkTagRe.FindStringSubmatch("v1.96.4-loft.3") + if m == nil || m[1] != "v1.96.4" { + t.Fatalf("v1.96.4-loft.3 split = %v, want base v1.96.4", m) + } + for _, bad := range []string{"v0.14.15-rc.2", "v1.96.4-loft", "v0.16.5.vcluster", "cmd/cigocacher/abc"} { + if forkTagRe.MatchString(bad) { + t.Errorf("%s should not parse as a fork release tag", bad) + } + } +} + +func TestMissingUpstreamTagFails(t *testing.T) { + f := reviewedFork() + delete(f.tags, upstream+"@v0.16.5") + r := runCheck(f, tagPin()) + if r.Pass || !strings.Contains(failuresOf(r), "has no tag v0.16.5") { + t.Fatalf("want a missing upstream tag to fail:\n%s", failuresOf(r)) + } +} + +func TestBaseNotAncestorFails(t *testing.T) { + f := reviewedFork() + cmp := f.compares[fork+"@"+baseSHA+"..."+merge] + cmp.Status = "diverged" + f.compares[fork+"@"+baseSHA+"..."+merge] = cmp + r := runCheck(f, tagPin()) + if r.Pass || !strings.Contains(failuresOf(r), "not an ancestor") { + t.Fatalf("want a diverged base to fail:\n%s", failuresOf(r)) + } +} + +func TestTruncatedCompareFails(t *testing.T) { + f := reviewedFork() + cmp := f.compares[fork+"@"+baseSHA+"..."+merge] + cmp.Total = maxCompareCommits + 1 + f.compares[fork+"@"+baseSHA+"..."+merge] = cmp + r := runCheck(f, tagPin()) + if r.Pass || !strings.Contains(failuresOf(r), "truncated") { + t.Fatalf("want a compare over the limit to fail:\n%s", failuresOf(r)) + } +} + +func TestIdenticalToUpstreamPasses(t *testing.T) { + f := reviewedFork() + f.compares[fork+"@"+baseSHA+"..."+baseSHA] = comparison{Status: "identical"} + r := runCheck(f, pin{Repo: fork, Ref: "v0.16.5.vcluster.0", Tag: "v0.16.5.vcluster.0", Commit: baseSHA}) + if !r.Pass || len(r.Commits) != 0 { + t.Fatalf("no fork commits means nothing to review: %+v\n%s", r, failuresOf(r)) + } +} + +func TestMissingForkUpstreamFails(t *testing.T) { + f := reviewedFork() + delete(f.upstreams, fork) + r := runCheck(f, tagPin()) + if r.Pass || r.ForkUpstreamSource != "missing" || len(r.Commits) != 0 { + t.Fatalf("want a fork without fork-upstream to fail before any lookup: %+v", r) + } +} + +func TestOverrideKeepsEvidenceButStillFails(t *testing.T) { + f := reviewedFork() + delete(f.upstreams, fork) + r := newChecker(f, time.Time{}, map[string]upstreamOverride{fork: {Repo: upstream}}).checkFork(context.Background(), tagPin()) + if r.Pass { + t.Fatal("the missing property must fail even with an override") + } + if r.ForkUpstreamSource != "override" || len(r.Commits) != 3 { + t.Fatalf("want per-commit evidence from the override: %+v", r) + } + for _, c := range r.Commits { + if !c.Pass { + t.Errorf("commit %s should pass on its own: %v", short(c.SHA), c.Failures) + } + } +} + +func TestPropertyWinsOverOverride(t *testing.T) { + f := reviewedFork() + r := newChecker(f, time.Time{}, map[string]upstreamOverride{fork: {Repo: "evil/kine"}}).checkFork(context.Background(), tagPin()) + if r.ForkUpstream != upstream || r.ForkUpstreamSource != "property" || !r.Pass { + t.Fatalf("the property must win: %+v", r) + } +} + +func TestCommitPinUsesMergeBase(t *testing.T) { + // A pseudo-version pin: no tag, so the base is the merge base with the + // upstream default branch. + f := reviewedFork() + f.repos[upstream] = repoInfo{FullName: upstream, DefaultBranch: "master"} + f.compares[fork+"@k3s-io:master..."+merge] = comparison{Status: "diverged", Total: 3, MergeBase: baseSHA, Commits: f.compares[fork+"@"+baseSHA+"..."+merge].Commits} + r := runCheck(f, pin{Repo: fork, Ref: "v0.0.0-20261001120000-dddddddddddd", Commit: merge}) + if !r.Pass { + t.Fatalf("want pass:\n%s", failuresOf(r)) + } + if r.Base.Rule != "merge-base" || r.Base.Commit != baseSHA || r.Base.Ref != upstream+"@master" { + t.Errorf("base = %+v, want merge-base %s on %s@master", r.Base, baseSHA, upstream) + } +} + +func TestCommitPinHonoursUpstreamBranchOverride(t *testing.T) { + f := reviewedFork() + delete(f.upstreams, fork) + f.compares[fork+"@k3s-io:release-1.0..."+merge] = comparison{Status: "diverged", Total: 0, MergeBase: merge} + r := newChecker(f, time.Time{}, map[string]upstreamOverride{fork: {Repo: upstream, Ref: "release-1.0"}}).checkFork(context.Background(), pin{Repo: fork, Ref: merge, Commit: merge}) + if r.Base == nil || r.Base.Ref != upstream+"@release-1.0" { + t.Fatalf("want the override branch as base ref: %+v", r) + } +} diff --git a/.github/actions/fork-review-check/src/github.go b/.github/actions/fork-review-check/src/github.go new file mode 100644 index 00000000..d2a8f502 --- /dev/null +++ b/.github/actions/fork-review-check/src/github.go @@ -0,0 +1,256 @@ +package main + +import ( + "context" + "errors" + "fmt" + "net/http" + "strings" + "time" + + "github.com/google/go-github/v88/github" +) + +// forkUpstreamProperty is the org custom property that names a fork's +// upstream repository. Only org actors can set it, so a repo admin cannot +// point the check at a different upstream. +const forkUpstreamProperty = "fork-upstream" + +type repoInfo struct { + FullName string + DefaultBranch string + Fork bool +} + +type commitInfo struct { + SHA string + Author string // GitHub login of the git author; empty when the email maps to no account + Committer string // GitHub login of the committer + Verified bool // GitHub's verification result for the commit signature + Subject string + Merge bool +} + +type comparison struct { + Status string + Total int + MergeBase string + Commits []commitInfo +} + +type pullInfo struct { + Number int + URL string + Author string + BaseRepo string + HeadSHA string + MergeCommit string + MergedAt time.Time // zero when the pull request is not merged +} + +type reviewInfo struct { + User string + State string + CommitID string + SubmittedAt time.Time +} + +// forkAPI is the read-only GitHub surface the rules need. +type forkAPI interface { + Repo(ctx context.Context, repo string) (repoInfo, error) + // ForkUpstream returns the fork-upstream property, or "" when it is unset. + ForkUpstream(ctx context.Context, repo string) (string, error) + // ResolveTag returns the commit a tag points to, or "" when the tag does not exist. + ResolveTag(ctx context.Context, repo, tag string) (string, error) + Compare(ctx context.Context, repo, base, head string) (comparison, error) + PullsWithCommit(ctx context.Context, repo, sha string) ([]pullInfo, error) + Reviews(ctx context.Context, repo string, number int) ([]reviewInfo, error) + // LastForcePush returns when the pull request head was last force-pushed, or zero. + LastForcePush(ctx context.Context, repo string, number int) (time.Time, error) + // Permission returns the collaborator permission: admin, write, read or none. + Permission(ctx context.Context, repo, user string) (string, error) +} + +type githubAPI struct { + gh *github.Client +} + +func splitRepo(repo string) (string, string) { + owner, name, _ := strings.Cut(repo, "/") + return owner, name +} + +func (a githubAPI) Repo(ctx context.Context, repo string) (repoInfo, error) { + owner, name := splitRepo(repo) + r, _, err := a.gh.Repositories.Get(ctx, owner, name) + if err != nil { + return repoInfo{}, err + } + return repoInfo{FullName: r.GetFullName(), DefaultBranch: r.GetDefaultBranch(), Fork: r.GetFork()}, nil +} + +func (a githubAPI) ForkUpstream(ctx context.Context, repo string) (string, error) { + owner, name := splitRepo(repo) + values, _, err := a.gh.Repositories.GetAllCustomPropertyValues(ctx, owner, name) + if err != nil { + return "", err + } + for _, v := range values { + if v.PropertyName != forkUpstreamProperty { + continue + } + if s, ok := v.Value.(string); ok { + return strings.TrimSpace(s), nil + } + } + return "", nil +} + +func (a githubAPI) ResolveTag(ctx context.Context, repo, tag string) (string, error) { + owner, name := splitRepo(repo) + ref, _, err := a.gh.Git.GetRef(ctx, owner, name, "tags/"+tag) + if isNotFound(err) { + return "", nil + } + if err != nil { + return "", err + } + obj := ref.GetObject() + // Annotated tags point at a tag object; peel until a commit. + for obj.GetType() == "tag" { + t, _, err := a.gh.Git.GetTag(ctx, owner, name, obj.GetSHA()) + if err != nil { + return "", err + } + obj = t.GetObject() + } + if obj.GetType() != "commit" { + return "", fmt.Errorf("tag %s points to a %s, not a commit", tag, obj.GetType()) + } + return obj.GetSHA(), nil +} + +func (a githubAPI) Compare(ctx context.Context, repo, base, head string) (comparison, error) { + owner, name := splitRepo(repo) + var out comparison + opts := &github.ListOptions{PerPage: 100} + for { + cmp, resp, err := a.gh.Repositories.CompareCommits(ctx, owner, name, base, head, opts) + if err != nil { + return comparison{}, err + } + if opts.Page == 0 { + out.Status = cmp.GetStatus() + out.Total = cmp.GetTotalCommits() + out.MergeBase = cmp.GetMergeBaseCommit().GetSHA() + if out.Total > maxCompareCommits { + return out, nil + } + } + for _, rc := range cmp.Commits { + out.Commits = append(out.Commits, commitInfo{ + SHA: rc.GetSHA(), + Author: rc.GetAuthor().GetLogin(), + Committer: rc.GetCommitter().GetLogin(), + Verified: rc.GetCommit().GetVerification().GetVerified(), + Subject: firstLine(rc.GetCommit().GetMessage()), + Merge: len(rc.Parents) > 1, + }) + } + if resp.NextPage == 0 { + return out, nil + } + opts.Page = resp.NextPage + } +} + +func (a githubAPI) PullsWithCommit(ctx context.Context, repo, sha string) ([]pullInfo, error) { + owner, name := splitRepo(repo) + var out []pullInfo + opts := &github.ListOptions{PerPage: 100} + for { + prs, resp, err := a.gh.PullRequests.ListPullRequestsWithCommit(ctx, owner, name, sha, opts) + if err != nil { + return nil, err + } + for _, pr := range prs { + out = append(out, pullInfo{ + Number: pr.GetNumber(), + URL: pr.GetHTMLURL(), + Author: pr.GetUser().GetLogin(), + BaseRepo: pr.GetBase().GetRepo().GetFullName(), + HeadSHA: pr.GetHead().GetSHA(), + MergeCommit: pr.GetMergeCommitSHA(), + MergedAt: pr.GetMergedAt().Time, + }) + } + if resp.NextPage == 0 { + return out, nil + } + opts.Page = resp.NextPage + } +} + +func (a githubAPI) Reviews(ctx context.Context, repo string, number int) ([]reviewInfo, error) { + owner, name := splitRepo(repo) + var out []reviewInfo + opts := &github.ListOptions{PerPage: 100} + for { + reviews, resp, err := a.gh.PullRequests.ListReviews(ctx, owner, name, number, opts) + if err != nil { + return nil, err + } + for _, r := range reviews { + out = append(out, reviewInfo{ + User: r.GetUser().GetLogin(), + State: r.GetState(), + CommitID: r.GetCommitID(), + SubmittedAt: r.GetSubmittedAt().Time, + }) + } + if resp.NextPage == 0 { + return out, nil + } + opts.Page = resp.NextPage + } +} + +func (a githubAPI) LastForcePush(ctx context.Context, repo string, number int) (time.Time, error) { + owner, name := splitRepo(repo) + var last time.Time + opts := &github.ListOptions{PerPage: 100} + for { + events, resp, err := a.gh.Issues.ListIssueTimeline(ctx, owner, name, number, opts) + if err != nil { + return time.Time{}, err + } + for _, e := range events { + if e.GetEvent() == "head_ref_force_pushed" && e.GetCreatedAt().After(last) { + last = e.GetCreatedAt().Time + } + } + if resp.NextPage == 0 { + return last, nil + } + opts.Page = resp.NextPage + } +} + +func (a githubAPI) Permission(ctx context.Context, repo, user string) (string, error) { + owner, name := splitRepo(repo) + p, _, err := a.gh.Repositories.GetPermissionLevel(ctx, owner, name, user) + if err != nil { + return "", err + } + return p.GetPermission(), nil +} + +func isNotFound(err error) bool { + var ghErr *github.ErrorResponse + return errors.As(err, &ghErr) && ghErr.Response != nil && ghErr.Response.StatusCode == http.StatusNotFound +} + +func firstLine(s string) string { + line, _, _ := strings.Cut(s, "\n") + return strings.TrimSpace(line) +} diff --git a/.github/actions/fork-review-check/src/github_test.go b/.github/actions/fork-review-check/src/github_test.go new file mode 100644 index 00000000..fd068f16 --- /dev/null +++ b/.github/actions/fork-review-check/src/github_test.go @@ -0,0 +1,33 @@ +package main + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/google/go-github/v88/github" +) + +func TestCompareReadsCommitSignatureVerification(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, `{"status":"ahead","total_commits":3,"commits":[ + {"sha":"signed","commit":{"verification":{"verified":true}}}, + {"sha":"unsigned","commit":{"verification":{"verified":false}}}, + {"sha":"missing","commit":{}} + ]}`) + })) + defer server.Close() + client, err := github.NewClient(github.WithEnterpriseURLs(server.URL, server.URL)) + if err != nil { + t.Fatal(err) + } + cmp, err := (githubAPI{gh: client}).Compare(context.Background(), "owner/repo", "base", "head") + if err != nil { + t.Fatal(err) + } + if len(cmp.Commits) != 3 || !cmp.Commits[0].Verified || cmp.Commits[1].Verified || cmp.Commits[2].Verified { + t.Fatalf("unexpected verification states: %+v", cmp.Commits) + } +} diff --git a/.github/actions/fork-review-check/src/go.mod b/.github/actions/fork-review-check/src/go.mod new file mode 100644 index 00000000..a3911767 --- /dev/null +++ b/.github/actions/fork-review-check/src/go.mod @@ -0,0 +1,7 @@ +module github.com/loft-sh/github-actions/fork-review-check + +go 1.26.4 + +require github.com/google/go-github/v88 v88.0.0 + +require github.com/google/go-querystring v1.2.0 // indirect diff --git a/.github/actions/fork-review-check/src/go.sum b/.github/actions/fork-review-check/src/go.sum new file mode 100644 index 00000000..06a97fa8 --- /dev/null +++ b/.github/actions/fork-review-check/src/go.sum @@ -0,0 +1,7 @@ +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/go-github/v88 v88.0.0 h1:dZA9IKkPK1eXZj4ypngnpRj5FwdpTv4whix2PrQMP7M= +github.com/google/go-github/v88 v88.0.0/go.mod h1:rufTDgn2N45wjhukLTyxmvc9nilSp3mr3Rgtt6b1MPw= +github.com/google/go-querystring v1.2.0 h1:yhqkPbu2/OH+V9BfpCVPZkNmUXhb2gBxJArfhIxNtP0= +github.com/google/go-querystring v1.2.0/go.mod h1:8IFJqpSRITyJ8QhQ13bmbeMBDfmeEJZD5A0egEOmkqU= diff --git a/.github/actions/fork-review-check/src/gomod.go b/.github/actions/fork-review-check/src/gomod.go new file mode 100644 index 00000000..2611e247 --- /dev/null +++ b/.github/actions/fork-review-check/src/gomod.go @@ -0,0 +1,362 @@ +package main + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" +) + +// forkModulePrefix selects the modules that can carry fork code: after +// replacements, their source lives in a loft-sh repository. +const forkModulePrefix = "github.com/loft-sh/" + +// pseudoVersionRe matches Go pseudo-versions (golang.org/x/mod/module), which +// pin a commit rather than a tag. +var pseudoVersionRe = regexp.MustCompile(`^v[0-9]+\.(0\.0-|\d+\.\d+-([^+]*\.)?0\.)\d{14}-[A-Za-z0-9]+(\+[0-9A-Za-z-]+(\.[0-9A-Za-z-]+)*)?$`) + +type goModule struct { + Path string + Version string + Main bool + Replace *goModule +} + +type goOrigin struct { + VCS string + URL string + Hash string +} + +type goDownload struct { + Path string + Version string + Sum string + Origin *goOrigin + Error string +} + +// goTool runs the go command; tests replace it. +type goTool interface { + ListModules(dir string) ([]goModule, error) + Download(dir, path, version string) (goDownload, error) +} + +// inputRecord is one build input the receipt lists, with its hash. +type inputRecord struct { + Kind string `json:"kind"` + Source string `json:"source"` + Module string `json:"module,omitempty"` + Path string `json:"path"` + Version string `json:"version"` + Repo string `json:"repo"` + Commit string `json:"commit"` + Sum string `json:"sum,omitempty"` + Fork bool `json:"fork"` +} + +// goForkInputs resolves the loft-sh modules in the effective module graph of +// dir to the commits Go downloaded. It reports every loft-sh module; deciding +// which of them are forks needs the GitHub API and happens in the caller. +func goForkInputs(tool goTool, dir string) ([]inputRecord, []string, error) { + mods, err := tool.ListModules(dir) + if err != nil { + return nil, nil, err + } + + var inputs []inputRecord + var failures []string + seen := map[string]bool{} + for _, m := range mods { + if m.Main { + continue + } + eff := m + if m.Replace != nil { + if m.Replace.Version == "" { + // A directory replacement is code in the product repository itself. + continue + } + eff = *m.Replace + } + if !strings.HasPrefix(eff.Path, forkModulePrefix) { + continue + } + key := eff.Path + "@" + eff.Version + if seen[key] { + continue + } + seen[key] = true + + rec := inputRecord{Kind: "go-module", Source: filepath.Join(dir, "go.mod"), Path: eff.Path, Version: eff.Version} + if m.Replace != nil { + rec.Module = m.Path + } + dl, err := tool.Download(dir, eff.Path, eff.Version) + if err != nil { + failures = append(failures, fmt.Sprintf("%s@%s: go mod download: %v", eff.Path, eff.Version, err)) + continue + } + if dl.Origin == nil || dl.Origin.Hash == "" { + failures = append(failures, fmt.Sprintf("%s@%s: Go reported no origin commit; set GOPRIVATE=github.com/loft-sh/* so it fetches from git and records the commit", eff.Path, eff.Version)) + continue + } + rec.Commit = dl.Origin.Hash + rec.Sum = dl.Sum + rec.Repo = repoFromOrigin(dl.Origin.URL, eff.Path) + inputs = append(inputs, rec) + } + return inputs, failures, nil +} + +// pinFromGoInput turns a resolved module into the fork pin the rules check. +// A pseudo-version pins a commit; any other version names a fork tag. +func pinFromGoInput(in inputRecord) pin { + version := strings.TrimSuffix(in.Version, "+incompatible") + p := pin{Repo: in.Repo, Ref: in.Version, Commit: in.Commit} + if !pseudoVersionRe.MatchString(in.Version) { + p.Tag = version + } + return p +} + +// repoFromOrigin names the repository Go fetched from, falling back to the +// first two path elements after github.com. +func repoFromOrigin(url, path string) string { + u := strings.TrimSuffix(strings.TrimPrefix(strings.TrimPrefix(url, "https://"), "http://"), ".git") + if strings.HasPrefix(u, "github.com/") { + path = u + } + parts := strings.Split(strings.TrimPrefix(path, "github.com/"), "/") + if len(parts) < 2 { + return "" + } + return parts[0] + "/" + parts[1] +} + +// verifyVendor checks the committed vendor tree using a caller-provided +// verifier in a temporary clone. The verifier is trusted code, not sandboxed. +func verifyVendor(dir, verifier string) error { + // Workspace vendoring belongs to the workspace root, not an individual + // module. Do not certify it with a module-scoped verifier. + if st, err := os.Stat(dir); err == nil && st.IsDir() { + out, err := runGo(dir, "env", "GOWORK") + if err != nil { + return fmt.Errorf("detect Go workspace: %w", err) + } + workspace := strings.TrimSpace(string(out)) + if workspace != "" && workspace != "off" { + path := filepath.Join(filepath.Dir(workspace), "vendor") + if _, err := os.Lstat(path); err == nil { + return fmt.Errorf("workspace vendor tree %s requires workspace verification, which is not supported", path) + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("inspect workspace vendor tree: %w", err) + } + } + } else if err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("inspect module directory: %w", err) + } + vendor := filepath.Join(dir, "vendor") + info, err := os.Lstat(vendor) + if errors.Is(err, os.ErrNotExist) { + return nil + } + if err != nil { + return fmt.Errorf("inspect vendor tree: %w", err) + } + if !info.IsDir() { + return fmt.Errorf("vendor path %s is not a directory", vendor) + } + if strings.TrimSpace(verifier) == "" { + return fmt.Errorf("%s exists: configure a trusted vendor verifier", vendor) + } + module, err := filepath.EvalSymlinks(dir) + if err != nil { + return fmt.Errorf("resolve module directory: %w", err) + } + module, err = filepath.Abs(module) + if err != nil { + return err + } + runGit := func(at string, args ...string) ([]byte, error) { + cmd := exec.Command("git", args...) + cmd.Dir = at + out, err := cmd.CombinedOutput() + if err != nil { + return nil, fmt.Errorf("git %s: %w: %s", strings.Join(args, " "), err, strings.TrimSpace(string(out))) + } + return out, nil + } + out, err := runGit(module, "rev-parse", "--show-toplevel") + if err != nil { + return fmt.Errorf("vendor verification needs a Git checkout: %w", err) + } + root, err := filepath.EvalSymlinks(strings.TrimSpace(string(out))) + if err != nil { + return err + } + rel, err := filepath.Rel(root, module) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return fmt.Errorf("module directory is outside its Git checkout") + } + // A clean checkout binds the clone to the source the build will consume. + // Include ignored vendor files: Git's normal status hides them, but Go can + // still compile them. Do not follow symlinks outside the checkout. + clean := func() error { + status, err := runGit(root, "status", "--porcelain", "--untracked-files=all") + if err != nil { + return err + } + if len(status) != 0 { + return fmt.Errorf("vendor verification requires a clean checkout") + } + status, err = runGit(root, "status", "--porcelain", "--untracked-files=all", "--ignored", "--", filepath.Join(rel, "vendor")) + if err != nil { + return err + } + if len(status) != 0 { + return fmt.Errorf("vendor tree contains uncommitted or ignored files") + } + return nil + } + if err := clean(); err != nil { + return err + } + err = filepath.WalkDir(filepath.Join(module, "vendor"), func(path string, entry os.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if entry.Type()&os.ModeSymlink == 0 { + return nil + } + target, err := filepath.EvalSymlinks(path) + if err != nil { + return fmt.Errorf("resolve vendor symlink %s: %w", path, err) + } + r, err := filepath.Rel(root, target) + if err != nil || r == ".." || strings.HasPrefix(r, ".."+string(filepath.Separator)) { + return fmt.Errorf("vendor symlink %s points outside the checkout", path) + } + status, err := runGit(root, "status", "--porcelain", "--untracked-files=all", "--ignored", "--", r) + if err != nil { + return err + } + if len(status) != 0 { + return fmt.Errorf("vendor symlink target %s contains uncommitted or ignored files", path) + } + return nil + }) + if err != nil { + return err + } + head, err := runGit(root, "rev-parse", "--verify", "HEAD") + if err != nil { + return err + } + tmp, err := os.MkdirTemp("", "fork-review-vendor-*") + if err != nil { + return err + } + defer os.RemoveAll(tmp) + clone := filepath.Join(tmp, "checkout") + if _, err := runGit(root, "-c", "core.hooksPath=/dev/null", "clone", "--no-hardlinks", "--no-checkout", "--", root, clone); err != nil { + return err + } + if _, err := runGit(clone, "-c", "core.hooksPath=/dev/null", "checkout", "--detach", strings.TrimSpace(string(head))); err != nil { + return err + } + if err := clean(); err != nil { + return err + } + current, err := runGit(root, "rev-parse", "--verify", "HEAD") + if err != nil { + return err + } + if !bytes.Equal(head, current) { + return fmt.Errorf("checkout HEAD changed during vendor verification") + } + cmd := exec.Command("sh", "-c", verifier) + cmd.Dir = filepath.Join(clone, rel) + output, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("vendor verifier failed: %w: %s", err, strings.TrimSpace(string(output))) + } + return nil +} + +type execGo struct{} + +func (execGo) ListModules(dir string) ([]goModule, error) { + args := []string{"list", "-m", "-json"} + // With a vendor directory and no workspace, go list cannot compute "all" + // from vendor/modules.txt. The vendor verifier checks that source separately. + if exists(filepath.Join(dir, "vendor")) { + work, err := runGo(dir, "env", "GOWORK") + if err != nil { + return nil, fmt.Errorf("detect Go workspace: %w", err) + } + if strings.TrimSpace(string(work)) == "" || strings.TrimSpace(string(work)) == "off" { + args = append(args, "-mod=mod") + } + } + args = append(args, "all") + out, err := runGo(dir, args...) + if err != nil { + return nil, err + } + return decodeModules(out) +} + +func (execGo) Download(dir, path, version string) (goDownload, error) { + out, err := runGo(dir, "mod", "download", "-json", path+"@"+version) + var dl goDownload + if jerr := json.Unmarshal(out, &dl); jerr != nil { + if err != nil { + return dl, err + } + return dl, jerr + } + if dl.Error != "" { + return dl, errors.New(dl.Error) + } + return dl, err +} + +func decodeModules(out []byte) ([]goModule, error) { + var mods []goModule + dec := json.NewDecoder(bytes.NewReader(out)) + for { + var m goModule + err := dec.Decode(&m) + if errors.Is(err, io.EOF) { + return mods, nil + } + if err != nil { + return nil, fmt.Errorf("decode go list output: %w", err) + } + mods = append(mods, m) + } +} + +func runGo(dir string, args ...string) ([]byte, error) { + cmd := exec.Command("go", args...) + cmd.Dir = dir + var stderr bytes.Buffer + cmd.Stderr = &stderr + out, err := cmd.Output() + if err != nil { + return out, fmt.Errorf("go %s: %w: %s", strings.Join(args, " "), err, strings.TrimSpace(stderr.String())) + } + return out, nil +} + +func exists(path string) bool { + _, err := os.Stat(path) + return err == nil +} diff --git a/.github/actions/fork-review-check/src/gomod_test.go b/.github/actions/fork-review-check/src/gomod_test.go new file mode 100644 index 00000000..7b346d8b --- /dev/null +++ b/.github/actions/fork-review-check/src/gomod_test.go @@ -0,0 +1,346 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +func vendorFixture(t *testing.T) string { + t.Helper() + dir := t.TempDir() + run := func(args ...string) { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + } + run("init", "-q") + run("config", "user.name", "Test") + run("config", "user.email", "test@example.com") + if err := os.MkdirAll(filepath.Join(dir, "vendor"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "vendor", "source.go"), []byte("original"), 0644); err != nil { + t.Fatal(err) + } + if err := os.Symlink("source.go", filepath.Join(dir, "vendor", "link")); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, ".gitignore"), []byte("vendor/ignored\n"), 0644); err != nil { + t.Fatal(err) + } + run("add", ".") + run("commit", "-qm", "fixture") + return dir +} + +func TestVerifyVendorRequiresVerifier(t *testing.T) { + dir := vendorFixture(t) + if err := verifyVendor(dir, ""); err == nil || !strings.Contains(err.Error(), "verifier") { + t.Fatalf("expected missing verifier error, got %v", err) + } + if err := verifyVendor(t.TempDir(), ""); err != nil { + t.Fatal(err) + } +} + +func TestVerifyVendorRejectsBrokenVendorLink(t *testing.T) { + dir := t.TempDir() + if err := os.Symlink("missing", filepath.Join(dir, "vendor")); err != nil { + t.Fatal(err) + } + if err := verifyVendor(dir, "true"); err == nil { + t.Fatal("expected broken vendor link to fail") + } +} + +func TestVerifyVendorRunsInIsolatedCopy(t *testing.T) { + dir := vendorFixture(t) + command := "test -L vendor/link && test -d .git && test \"$(cat vendor/link)\" = original && printf changed > vendor/source.go" + if err := verifyVendor(dir, command); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(filepath.Join(dir, "vendor", "source.go")) + if err != nil || string(data) != "original" { + t.Fatalf("original changed: %q, %v", data, err) + } + if err := verifyVendor(dir, "exit 7"); err == nil { + t.Fatal("expected verifier failure") + } +} + +func TestVerifyVendorRejectsDirtyAndIgnoredVendor(t *testing.T) { + for _, file := range []string{"source.go", "untracked", "ignored"} { + t.Run(file, func(t *testing.T) { + dir := vendorFixture(t) + if err := os.WriteFile(filepath.Join(dir, "vendor", file), []byte("changed"), 0644); err != nil { + t.Fatal(err) + } + if err := verifyVendor(dir, "true"); err == nil { + t.Fatal("expected dirty vendor failure") + } + }) + } +} + +func TestVerifyVendorRejectsSymlinkOutsideCheckout(t *testing.T) { + dir := vendorFixture(t) + if err := os.Symlink(t.TempDir(), filepath.Join(dir, "vendor", "outside")); err != nil { + t.Fatal(err) + } + cmd := exec.Command("git", "add", "vendor/outside") + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git add: %v: %s", err, out) + } + cmd = exec.Command("git", "commit", "-qm", "outside link") + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git commit: %v: %s", err, out) + } + if err := verifyVendor(dir, "true"); err == nil || !strings.Contains(err.Error(), "outside") { + t.Fatalf("expected outside symlink error, got %v", err) + } +} + +func TestVerifyVendorRejectsIgnoredSymlinkTargetContent(t *testing.T) { + dir := vendorFixture(t) + if err := os.MkdirAll(filepath.Join(dir, "staging"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "staging", "tracked"), []byte("tracked"), 0644); err != nil { + t.Fatal(err) + } + if err := os.Symlink("../staging", filepath.Join(dir, "vendor", "staging")); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, ".gitignore"), []byte("staging/ignored\n"), 0644); err != nil { + t.Fatal(err) + } + cmd := exec.Command("git", "add", "-A") + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git add: %v: %s", err, out) + } + cmd = exec.Command("git", "commit", "-qm", "staging link") + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git commit: %v: %s", err, out) + } + if err := os.WriteFile(filepath.Join(dir, "staging", "ignored"), []byte("uncommitted"), 0644); err != nil { + t.Fatal(err) + } + if err := verifyVendor(dir, "true"); err == nil { + t.Fatal("expected ignored target content to fail") + } +} + +func TestVerifyVendorRunsInModuleDirectory(t *testing.T) { + root := vendorFixture(t) + module := filepath.Join(root, "sub") + if err := os.MkdirAll(module, 0755); err != nil { + t.Fatal(err) + } + if err := os.Rename(filepath.Join(root, "vendor"), filepath.Join(module, "vendor")); err != nil { + t.Fatal(err) + } + cmd := exec.Command("git", "add", "-A") + cmd.Dir = root + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git add: %v: %s", err, out) + } + cmd = exec.Command("git", "commit", "-qm", "nested module") + cmd.Dir = root + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git commit: %v: %s", err, out) + } + if err := verifyVendor(module, "test -L vendor/link && test ! -d .git"); err != nil { + t.Fatal(err) + } +} + +func TestVerifyVendorRejectsWorkspaceVendor(t *testing.T) { + root := t.TempDir() + module := filepath.Join(root, "sub") + if err := os.MkdirAll(module, 0755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(root, "vendor"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "go.work"), []byte("go 1.24\n\nuse ./sub\n"), 0644); err != nil { + t.Fatal(err) + } + t.Setenv("GOWORK", "auto") + if err := verifyVendor(module, "true"); err == nil || !strings.Contains(err.Error(), "workspace") { + t.Fatalf("expected workspace vendor error, got %v", err) + } +} + +func TestListModulesWithVendorAndWorkspaceDisabled(t *testing.T) { + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, "vendor"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "go.mod"), []byte("module example.com/test\n\ngo 1.24\n"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "vendor", "modules.txt"), nil, 0644); err != nil { + t.Fatal(err) + } + t.Setenv("GOWORK", "off") + t.Setenv("GOFLAGS", "-mod=vendor") + mods, err := (execGo{}).ListModules(dir) + if err != nil || len(mods) != 1 || mods[0].Path != "example.com/test" { + t.Fatalf("modules = %+v, err = %v", mods, err) + } +} + +type fakeGo struct { + mods []goModule + downloads map[string]goDownload // path@version +} + +func (f fakeGo) ListModules(string) ([]goModule, error) { return f.mods, nil } + +func (f fakeGo) Download(_, path, version string) (goDownload, error) { + dl, ok := f.downloads[path+"@"+version] + if !ok { + return dl, fmt.Errorf("no download %s@%s", path, version) + } + return dl, nil +} + +const fuseRev = "0b597961e33d5a2b8e3c1f6d1e8e6c4e2c1a0b9f" + +// vnodeGraph mirrors vnode-internal: bazil.org/fuse replaced by the loft-sh +// fork at a pseudo-version, a loft-sh product module, a directory replacement +// and an unrelated module. +func vnodeGraph() fakeGo { + return fakeGo{ + mods: []goModule{ + {Path: "github.com/loft-sh/vnode", Main: true}, + {Path: "bazil.org/fuse", Version: "v0.0.0-20180421153158-65cc252bf669", Replace: &goModule{Path: "github.com/loft-sh/fuse", Version: "v0.0.0-20250416130952-0b597961e33d"}}, + {Path: "github.com/loft-sh/api/v4", Version: "v4.2.1"}, + {Path: "github.com/loft-sh/local", Version: "v0.0.0", Replace: &goModule{Path: "./local"}}, + {Path: "tailscale.com", Version: "v1.96.4", Replace: &goModule{Path: "github.com/loft-sh/tailscale", Version: "v1.96.4-loft.3"}}, + {Path: "github.com/sirupsen/logrus", Version: "v1.9.3"}, + }, + downloads: map[string]goDownload{ + "github.com/loft-sh/fuse@v0.0.0-20250416130952-0b597961e33d": {Sum: "h1:fuse", Origin: &goOrigin{VCS: "git", URL: "https://github.com/loft-sh/fuse", Hash: fuseRev}}, + "github.com/loft-sh/api/v4@v4.2.1": {Sum: "h1:api", Origin: &goOrigin{VCS: "git", URL: "https://github.com/loft-sh/api", Hash: strings.Repeat("a", 40)}}, + "github.com/loft-sh/tailscale@v1.96.4-loft.3": {Sum: "h1:ts", Origin: &goOrigin{VCS: "git", URL: "https://github.com/loft-sh/tailscale", Hash: strings.Repeat("4", 40)}}, + }, + } +} + +func TestGoForkInputsResolvesLoftModules(t *testing.T) { + inputs, failures, err := goForkInputs(vnodeGraph(), "vnode") + if err != nil || len(failures) != 0 { + t.Fatalf("err %v, failures %v", err, failures) + } + got := map[string]inputRecord{} + for _, in := range inputs { + got[in.Path] = in + } + if len(got) != 3 { + t.Fatalf("want fuse, api and tailscale, got %v", inputs) + } + fuse := got["github.com/loft-sh/fuse"] + if fuse.Module != "bazil.org/fuse" || fuse.Repo != "loft-sh/fuse" || fuse.Commit != fuseRev || fuse.Sum != "h1:fuse" { + t.Errorf("fuse input = %+v", fuse) + } + if _, ok := got["github.com/loft-sh/local"]; ok { + t.Error("a directory replacement is product code, not a fork pin") + } +} + +func TestGoForkInputsNeedsOrigin(t *testing.T) { + g := vnodeGraph() + dl := g.downloads["github.com/loft-sh/fuse@v0.0.0-20250416130952-0b597961e33d"] + dl.Origin = nil + g.downloads["github.com/loft-sh/fuse@v0.0.0-20250416130952-0b597961e33d"] = dl + _, failures, _ := goForkInputs(g, "vnode") + if len(failures) != 1 || !strings.Contains(failures[0], "GOPRIVATE") { + t.Fatalf("want one failure naming the GOPRIVATE remedy, got %v", failures) + } +} + +func TestPinFromGoInput(t *testing.T) { + pseudo := pinFromGoInput(inputRecord{Repo: "loft-sh/fuse", Version: "v0.0.0-20250416130952-0b597961e33d", Commit: fuseRev}) + if pseudo.Tag != "" || pseudo.Commit != fuseRev { + t.Errorf("pseudo-version pin = %+v, want a commit pin", pseudo) + } + tagged := pinFromGoInput(inputRecord{Repo: "loft-sh/tailscale", Version: "v1.96.4-loft.3", Commit: "x"}) + if tagged.Tag != "v1.96.4-loft.3" { + t.Errorf("tag pin = %+v, want tag v1.96.4-loft.3", tagged) + } + for _, v := range []string{"v1.2.4-0.20191109021931-daa7c04131f5", "v1.2.3-pre.0.20191109021931-daa7c04131f5", "v2.0.0-20191109021931-daa7c04131f5+incompatible"} { + if !pseudoVersionRe.MatchString(v) { + t.Errorf("%s is a pseudo-version", v) + } + } +} + +func TestRepoFromOrigin(t *testing.T) { + cases := map[[2]string]string{ + {"https://github.com/loft-sh/sysbox-libs", "github.com/loft-sh/sysbox-libs/utils"}: "loft-sh/sysbox-libs", + {"", "github.com/loft-sh/sysbox-libs/utils"}: "loft-sh/sysbox-libs", + {"https://github.com/loft-sh/fuse.git", "github.com/loft-sh/fuse"}: "loft-sh/fuse", + } + for in, want := range cases { + if got := repoFromOrigin(in[0], in[1]); got != want { + t.Errorf("repoFromOrigin(%q, %q) = %q, want %q", in[0], in[1], got, want) + } + } +} + +func TestDecodeModulesStream(t *testing.T) { + var out []byte + for _, m := range vnodeGraph().mods { + b, _ := json.MarshalIndent(m, "", "\t") + out = append(out, b...) + out = append(out, '\n') + } + mods, err := decodeModules(out) + if err != nil || len(mods) != len(vnodeGraph().mods) || mods[1].Replace.Path != "github.com/loft-sh/fuse" { + t.Fatalf("decode = %v, %v", mods, err) + } +} + +// TestGoModeChecksOnlyForks runs the whole go mode: api/v4 is a loft-sh +// product repo and is listed but not checked; fuse and tailscale are forks. +func TestGoModeChecksOnlyForks(t *testing.T) { + f := newFake() + f.repos["loft-sh/fuse"] = repoInfo{FullName: "loft-sh/fuse", Fork: true} + f.repos["loft-sh/api"] = repoInfo{FullName: "loft-sh/api"} + f.repos["loft-sh/tailscale"] = repoInfo{FullName: "loft-sh/tailscale", Fork: true} + f.upstreams["loft-sh/tailscale"] = "tailscale/tailscale" + + rc, err := check(context.Background(), f, vnodeGraph(), options{mode: "go", dirs: multiFlag{"vnode"}}, time.Now()) + if err != nil { + t.Fatal(err) + } + if len(rc.Inputs) != 3 { + t.Fatalf("want 3 inputs listed, got %+v", rc.Inputs) + } + var checked []string + for _, fr := range rc.Forks { + checked = append(checked, fr.Repo) + } + if strings.Join(checked, ",") != "loft-sh/fuse,loft-sh/tailscale" { + t.Fatalf("checked %v, want fuse and tailscale", checked) + } + if rc.Pass { + t.Fatal("fuse has no fork-upstream property, so the run must fail") + } +} diff --git a/.github/actions/fork-review-check/src/main.go b/.github/actions/fork-review-check/src/main.go new file mode 100644 index 00000000..40b4832b --- /dev/null +++ b/.github/actions/fork-review-check/src/main.go @@ -0,0 +1,312 @@ +// Command fork-review-check verifies that every fork commit a product build +// consumes reached its loft-sh fork through a merged pull request approved by +// someone other than its author (loft-sh/architecture, +// engineering-enablement/designs/fork-review-check.md). +// +// For each fork commit S it walks the range from the trusted upstream base to +// S and applies the design's rules: +// +// 1. S is the approved final head, or the merge result, of a merged fork PR. +// 2. Every commit in the range belongs to a merged fork PR approved at its +// final head, after its last force push, by someone with write or higher +// permission on the fork who is neither the PR author nor the commit author. +// 3. With -as-of, every approval predates that time (the product release). +// 4. It fails and names every commit that breaks a rule. +// +// It reads GitHub and the go command. For vendored modules, it runs a trusted +// verifier in a temporary checkout, and it can write a receipt. +// +// fork-review-check ref -repo loft-sh/kine -ref v0.16.5.vcluster.1 +// fork-review-check go -dir . -dir ./sub +// +// Exit codes: 0 pass (or -advisory), 1 a rule failed, 2 the check could not run. +package main + +import ( + "context" + "encoding/json" + "errors" + "flag" + "fmt" + "io" + "log" + "os" + "sort" + "strings" + "time" + + "github.com/google/go-github/v88/github" +) + +// version is stamped into every receipt; the action overrides it with the ref +// it runs from. +var version = "dev" + +const receiptSchema = "fork-review-check/v1" + +type receipt struct { + Schema string `json:"schema"` + CLIVersion string `json:"cli_version"` + Mode string `json:"mode"` + CheckedAt time.Time `json:"checked_at"` + AsOf *time.Time `json:"as_of,omitempty"` + Inputs []inputRecord `json:"inputs"` + Forks []forkResult `json:"forks"` + Failures []string `json:"failures,omitempty"` + Pass bool `json:"pass"` +} + +// multiFlag collects a repeatable string flag. +type multiFlag []string + +func (m *multiFlag) String() string { return strings.Join(*m, ",") } +func (m *multiFlag) Set(v string) error { *m = append(*m, v); return nil } + +type options struct { + mode string + repo string + ref string + dirs multiFlag + vendorVerifier string + upstreams multiFlag + asOf string + receipt string + advisory bool +} + +func main() { + log.SetFlags(0) + os.Exit(run(os.Args[1:], os.Stdout)) +} + +func run(args []string, stdout io.Writer) int { + opts, err := parseArgs(args) + if err != nil { + log.Printf("::error::%v", err) + return 2 + } + + token := os.Getenv("GITHUB_TOKEN") + if token == "" { + log.Print("::error::GITHUB_TOKEN is required") + return 2 + } + gh, err := github.NewClient(github.WithAuthToken(token)) + if err != nil { + log.Printf("::error::create github client: %v", err) + return 2 + } + + rc, err := check(context.Background(), githubAPI{gh: gh}, execGo{}, opts, time.Now().UTC()) + if err != nil { + log.Printf("::error::%v", err) + return 2 + } + if v := os.Getenv("FORK_REVIEW_CHECK_VERSION"); v != "" { + rc.CLIVersion = v + } + + if err := writeReceipt(rc, opts.receipt); err != nil { + log.Printf("::error::write receipt: %v", err) + return 2 + } + report := renderText(rc) + fmt.Fprint(stdout, report) + appendSummary(renderMarkdown(rc)) + + if rc.Pass { + return 0 + } + if opts.advisory { + log.Print("::warning::fork review check failed; advisory mode, not blocking") + return 0 + } + log.Print("::error::fork review check failed") + return 1 +} + +func parseArgs(args []string) (options, error) { + var opts options + if len(args) == 0 { + return opts, errors.New("usage: fork-review-check ref|go [flags]") + } + opts.mode = args[0] + + fs := flag.NewFlagSet("fork-review-check "+opts.mode, flag.ContinueOnError) + fs.StringVar(&opts.repo, "repo", "", "ref mode: fork repository, owner/name") + fs.StringVar(&opts.ref, "ref", "", "ref mode: fork release tag or commit SHA") + fs.Var(&opts.dirs, "dir", "go mode: module directory to resolve (repeatable)") + fs.StringVar(&opts.vendorVerifier, "vendor-verifier", "", "go mode: trusted command to verify vendor in an isolated checkout") + fs.Var(&opts.upstreams, "upstream", "fork=upstream[@branch] used when a fork has no fork-upstream property; in ref mode a bare upstream applies to -repo (repeatable)") + fs.StringVar(&opts.asOf, "as-of", "", "RFC 3339 time every approval must predate (rule 3), normally the product release time") + fs.StringVar(&opts.receipt, "receipt", "", "write the JSON receipt to this path") + fs.BoolVar(&opts.advisory, "advisory", false, "report failures as warnings and exit 0") + if err := fs.Parse(args[1:]); err != nil { + return opts, err + } + + switch opts.mode { + case "ref": + if opts.repo == "" || opts.ref == "" { + return opts, errors.New("ref mode needs -repo and -ref") + } + case "go": + if len(opts.dirs) == 0 { + opts.dirs = multiFlag{"."} + } + default: + return opts, fmt.Errorf("unknown mode %q: want ref or go", opts.mode) + } + return opts, nil +} + +// check resolves the pins for the mode and evaluates every fork. +func check(ctx context.Context, api forkAPI, tool goTool, opts options, now time.Time) (receipt, error) { + rc := receipt{Schema: receiptSchema, CLIVersion: version, Mode: opts.mode, CheckedAt: now, Inputs: []inputRecord{}, Forks: []forkResult{}} + + var asOf time.Time + if opts.asOf != "" { + t, err := time.Parse(time.RFC3339, opts.asOf) + if err != nil { + return rc, fmt.Errorf("parse -as-of: %w", err) + } + asOf = t.UTC() + rc.AsOf = &asOf + } + overrides, err := parseUpstreams(opts.upstreams, opts.repo) + if err != nil { + return rc, err + } + + var pins []pin + switch opts.mode { + case "ref": + p, failure, err := refPin(ctx, api, opts.repo, opts.ref) + if err != nil { + return rc, err + } + if failure != "" { + rc.Failures = append(rc.Failures, failure) + } else { + pins = append(pins, p) + rc.Inputs = append(rc.Inputs, inputRecord{Kind: "fork-ref", Source: "-ref", Path: opts.repo, Version: opts.ref, Repo: opts.repo, Commit: p.Commit, Fork: true}) + } + case "go": + for _, dir := range opts.dirs { + if err := verifyVendor(dir, opts.vendorVerifier); err != nil { + rc.Failures = append(rc.Failures, fmt.Sprintf("%s: verify vendor: %v", dir, err)) + continue + } + inputs, failures, err := goForkInputs(tool, dir) + if err != nil { + return rc, err + } + rc.Failures = append(rc.Failures, failures...) + for _, in := range inputs { + fork, err := isFork(ctx, api, in.Repo) + if err != nil { + return rc, fmt.Errorf("read %s: %w", in.Repo, err) + } + in.Fork = fork + rc.Inputs = append(rc.Inputs, in) + if fork { + pins = append(pins, pinFromGoInput(in)) + } + } + } + } + + c := newChecker(api, asOf, overrides) + seen := map[string]bool{} + for _, p := range pins { + key := p.Repo + "@" + p.Commit + "@" + p.Tag + if seen[key] { + continue + } + seen[key] = true + rc.Forks = append(rc.Forks, c.checkFork(ctx, p)) + } + sort.SliceStable(rc.Forks, func(i, j int) bool { return rc.Forks[i].Repo < rc.Forks[j].Repo }) + + rc.Pass = len(rc.Failures) == 0 + for _, f := range rc.Forks { + if !f.Pass { + rc.Pass = false + } + } + return rc, nil +} + +// refPin resolves a fork ref given by hand: a tag, or a full commit SHA. +func refPin(ctx context.Context, api forkAPI, repo, ref string) (pin, string, error) { + sha, err := api.ResolveTag(ctx, repo, ref) + if err != nil { + return pin{}, "", fmt.Errorf("resolve %s in %s: %w", ref, repo, err) + } + if sha != "" { + return pin{Repo: repo, Ref: ref, Tag: ref, Commit: sha}, "", nil + } + if fullSHARe.MatchString(ref) { + return pin{Repo: repo, Ref: ref, Commit: ref}, "", nil + } + return pin{}, fmt.Sprintf("%s has no tag %s, and it is not a full commit SHA", repo, ref), nil +} + +// isFork reports whether a loft-sh repository holds fork code: GitHub marks +// it a fork, or an org owner gave it a fork-upstream property. +func isFork(ctx context.Context, api forkAPI, repo string) (bool, error) { + info, err := api.Repo(ctx, repo) + if err != nil { + return false, err + } + if info.Fork { + return true, nil + } + upstream, err := api.ForkUpstream(ctx, repo) + return upstream != "", err +} + +func parseUpstreams(values []string, refRepo string) (map[string]upstreamOverride, error) { + out := map[string]upstreamOverride{} + for _, v := range values { + fork, upstream, ok := strings.Cut(v, "=") + if !ok { + if refRepo == "" { + return nil, fmt.Errorf("-upstream %q: want fork=upstream outside ref mode", v) + } + fork, upstream = refRepo, v + } + repo, branch, _ := strings.Cut(upstream, "@") + if !strings.Contains(fork, "/") || !strings.Contains(repo, "/") { + return nil, fmt.Errorf("-upstream %q: repositories are owner/name", v) + } + out[fork] = upstreamOverride{Repo: repo, Ref: branch} + } + return out, nil +} + +func writeReceipt(rc receipt, path string) error { + if path == "" { + return nil + } + data, err := json.MarshalIndent(rc, "", " ") + if err != nil { + return err + } + return os.WriteFile(path, append(data, '\n'), 0o644) +} + +// appendSummary writes to the job summary when running in Actions. +func appendSummary(md string) { + path := os.Getenv("GITHUB_STEP_SUMMARY") + if path == "" { + return + } + f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) + if err != nil { + log.Printf("::warning::write job summary: %v", err) + return + } + defer f.Close() + _, _ = f.WriteString(md) +} diff --git a/.github/actions/fork-review-check/src/main_test.go b/.github/actions/fork-review-check/src/main_test.go new file mode 100644 index 00000000..89303a7c --- /dev/null +++ b/.github/actions/fork-review-check/src/main_test.go @@ -0,0 +1,116 @@ +package main + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +func TestRefModeResolvesTag(t *testing.T) { + rc, err := check(context.Background(), reviewedFork(), nil, options{mode: "ref", repo: fork, ref: tag}, time.Now()) + if err != nil { + t.Fatal(err) + } + if !rc.Pass || len(rc.Forks) != 1 || rc.Forks[0].Commit != merge { + t.Fatalf("want one passing fork at %s: %+v", merge, rc) + } + if rc.Inputs[0].Kind != "fork-ref" || rc.Inputs[0].Commit != merge { + t.Errorf("input = %+v", rc.Inputs[0]) + } +} + +func TestRefModeAcceptsFullSHA(t *testing.T) { + f := reviewedFork() + f.repos[upstream] = repoInfo{FullName: upstream, DefaultBranch: "master"} + f.compares[fork+"@k3s-io:master..."+merge] = comparison{Status: "diverged", Total: 3, MergeBase: baseSHA, Commits: f.compares[fork+"@"+baseSHA+"..."+merge].Commits} + sha := merge + rc, err := check(context.Background(), f, nil, options{mode: "ref", repo: fork, ref: sha}, time.Now()) + if err != nil { + t.Fatal(err) + } + if !rc.Pass || rc.Forks[0].Base.Rule != "merge-base" { + t.Fatalf("want a commit pin on the merge-base rule: %+v", rc.Forks) + } +} + +func TestRefModeUnknownRefFails(t *testing.T) { + rc, err := check(context.Background(), reviewedFork(), nil, options{mode: "ref", repo: fork, ref: "v9.9.9"}, time.Now()) + if err != nil { + t.Fatal(err) + } + if rc.Pass || len(rc.Failures) != 1 { + t.Fatalf("want an unresolvable ref to fail: %+v", rc) + } +} + +func TestAsOfFlagAppliesRule3(t *testing.T) { + rc, err := check(context.Background(), reviewedFork(), nil, options{mode: "ref", repo: fork, ref: tag, asOf: t0.Add(30 * time.Minute).Format(time.RFC3339)}, time.Now()) + if err != nil { + t.Fatal(err) + } + if rc.Pass || rc.AsOf == nil { + t.Fatalf("want rule 3 to fail and as_of recorded: %+v", rc) + } +} + +func TestParseArgs(t *testing.T) { + if _, err := parseArgs([]string{"ref", "-repo", fork}); err == nil { + t.Error("ref mode without -ref must fail") + } + if _, err := parseArgs([]string{"image"}); err == nil { + t.Error("an unknown mode must fail") + } + o, err := parseArgs([]string{"go"}) + if err != nil || len(o.dirs) != 1 || o.dirs[0] != "." { + t.Errorf("go mode defaults to the current directory: %+v %v", o, err) + } +} + +func TestGoModeRequiresVendorVerification(t *testing.T) { + dir := t.TempDir() + if err := os.Mkdir(filepath.Join(dir, "vendor"), 0o755); err != nil { + t.Fatal(err) + } + rc, err := check(context.Background(), newFake(), vnodeGraph(), options{mode: "go", dirs: multiFlag{dir}}, time.Now()) + if err != nil { + t.Fatal(err) + } + if rc.Pass || !strings.Contains(strings.Join(rc.Failures, " "), "vendor") { + t.Fatalf("unverified vendor tree must fail: %+v", rc) + } +} + +func TestParseVendorVerifier(t *testing.T) { + o, err := parseArgs([]string{"go", "-vendor-verifier", "bash hack/verify-vendor.sh"}) + if err != nil || o.vendorVerifier != "bash hack/verify-vendor.sh" { + t.Fatalf("vendor verifier = %q, error %v", o.vendorVerifier, err) + } +} + +func TestParseUpstreams(t *testing.T) { + got, err := parseUpstreams([]string{"k3s-io/kine", "loft-sh/fuse=nestybox/fuse@master"}, fork) + if err != nil { + t.Fatal(err) + } + if got[fork].Repo != upstream || got["loft-sh/fuse"].Ref != "master" { + t.Errorf("overrides = %+v", got) + } + if _, err := parseUpstreams([]string{"kine"}, ""); err == nil { + t.Error("a bare upstream outside ref mode must fail") + } +} + +func TestReportsNameFailures(t *testing.T) { + f := reviewedFork() + delete(f.pulls, fork+"@"+c1) + rc, _ := check(context.Background(), f, nil, options{mode: "ref", repo: fork, ref: tag}, time.Now()) + text, md := renderText(rc), renderMarkdown(rc) + for _, want := range []string{"FAIL", short(c1), "direct push"} { + if !strings.Contains(text, want) || !strings.Contains(md, want) { + t.Errorf("reports miss %q:\n%s\n%s", want, text, md) + } + } +} diff --git a/.github/actions/fork-review-check/src/report.go b/.github/actions/fork-review-check/src/report.go new file mode 100644 index 00000000..d3252b59 --- /dev/null +++ b/.github/actions/fork-review-check/src/report.go @@ -0,0 +1,94 @@ +package main + +import ( + "fmt" + "regexp" + "strings" +) + +var fullSHARe = regexp.MustCompile(`^[0-9a-f]{40}$`) + +func verdict(pass bool) string { + if pass { + return "PASS" + } + return "FAIL" +} + +// renderText is the plain report for logs and local runs. +func renderText(rc receipt) string { + var b strings.Builder + fmt.Fprintf(&b, "fork review check %s: %s (%d forks)\n", rc.CLIVersion, verdict(rc.Pass), len(rc.Forks)) + for _, f := range rc.Failures { + fmt.Fprintf(&b, " FAIL %s\n", f) + } + for _, f := range rc.Forks { + fmt.Fprintf(&b, "\n%s %s @ %s: %s\n", f.Repo, f.Ref, short(f.Commit), verdict(f.Pass)) + fmt.Fprintf(&b, " fork-upstream: %s (%s)\n", orNone(f.ForkUpstream), f.ForkUpstreamSource) + if f.Base != nil { + fmt.Fprintf(&b, " base: %s %s (%s), %d fork commits\n", f.Base.Rule, f.Base.Ref, short(f.Base.Commit), len(f.Commits)) + } + if f.ReviewedTree != nil && f.ReviewedTree.Pass { + fmt.Fprintf(&b, " reviewed tree: #%d (%s)\n", f.ReviewedTree.Pull.Number, f.ReviewedTree.Via) + } + for _, msg := range f.Failures { + fmt.Fprintf(&b, " FAIL %s\n", msg) + } + for _, c := range f.Commits { + fmt.Fprintf(&b, " %s %s %s %q\n", verdict(c.Pass), short(c.SHA), orNone(c.Author), c.Subject) + if c.Pass { + fmt.Fprintf(&b, " #%d approved by %s (%s) at %s\n", c.Pull.Number, c.Approval.Approver, c.Approval.Permission, c.Approval.SubmittedAt.Format("2006-01-02T15:04:05Z")) + } + for _, msg := range c.Failures { + fmt.Fprintf(&b, " %s\n", msg) + } + } + } + return b.String() +} + +// renderMarkdown is the job summary: one table per fork. +func renderMarkdown(rc receipt) string { + var b strings.Builder + fmt.Fprintf(&b, "## Fork review check: %s\n\n", verdict(rc.Pass)) + for _, f := range rc.Failures { + fmt.Fprintf(&b, "- %s\n", f) + } + for _, f := range rc.Forks { + fmt.Fprintf(&b, "\n### %s `%s` (%s): %s\n\n", f.Repo, f.Ref, short(f.Commit), verdict(f.Pass)) + fmt.Fprintf(&b, "fork-upstream: `%s` (%s)", orNone(f.ForkUpstream), f.ForkUpstreamSource) + if f.Base != nil { + fmt.Fprintf(&b, "; base %s `%s`", f.Base.Rule, f.Base.Ref) + } + b.WriteString("\n\n") + for _, msg := range f.Failures { + fmt.Fprintf(&b, "- %s\n", msg) + } + if len(f.Commits) == 0 { + continue + } + b.WriteString("\n| Commit | Author | PR | Approver | Permission | Result |\n| --- | --- | --- | --- | --- | --- |\n") + for _, c := range f.Commits { + pr, approver, perm := "", "", "" + if c.Pull != nil { + pr = fmt.Sprintf("[#%d](%s)", c.Pull.Number, c.Pull.URL) + } + if c.Approval != nil { + approver, perm = c.Approval.Approver, c.Approval.Permission + } + result := verdict(c.Pass) + if !c.Pass { + result += ": " + strings.ReplaceAll(strings.Join(c.Failures, "; "), "|", "\\|") + } + fmt.Fprintf(&b, "| `%s` | %s | %s | %s | %s | %s |\n", short(c.SHA), orNone(c.Author), pr, approver, perm, result) + } + } + return b.String() +} + +func orNone(s string) string { + if s == "" { + return "none" + } + return s +} diff --git a/.github/workflows/test-fork-review-check.yaml b/.github/workflows/test-fork-review-check.yaml new file mode 100644 index 00000000..5bb91efe --- /dev/null +++ b/.github/workflows/test-fork-review-check.yaml @@ -0,0 +1,69 @@ +name: Test fork-review-check + +on: + push: + branches: [main] + paths: + - '.github/actions/fork-review-check/**' + - '.github/workflows/test-fork-review-check.yaml' + pull_request: + paths: + - '.github/actions/fork-review-check/**' + - '.github/workflows/test-fork-review-check.yaml' + workflow_dispatch: + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: .github/actions/fork-review-check/src/go.mod + - run: go test ./... + working-directory: .github/actions/fork-review-check/src + + # Read-only run against the shipped fork pins the design names, with the + # workflow token a required workflow gets. Advisory: it records today's gap + # and whether that token can read approver permission on another repo, and + # never fails the build. + live: + runs-on: ubuntu-latest + permissions: + contents: read + strategy: + fail-fast: false + matrix: + include: + - repo: loft-sh/kine + ref: v0.16.5.vcluster.1 + upstream: k3s-io/kine + - repo: loft-sh/tailscale + ref: v1.96.4-loft.3 + upstream: tailscale/tailscale + # bazil.org/fuse in vnode-internal pins loft-sh/fuse at this pseudo-version commit. + - repo: loft-sh/fuse + ref: 0b597961e33ddd4c9a19e9e3981b79cc113885b5 + upstream: nestybox/fuse + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: .github/actions/fork-review-check/src/go.mod + - name: Check ${{ matrix.repo }} ${{ matrix.ref }} + working-directory: .github/actions/fork-review-check/src + env: + GITHUB_TOKEN: ${{ github.token }} + REPO: ${{ matrix.repo }} + REF: ${{ matrix.ref }} + UPSTREAM: ${{ matrix.upstream }} + run: go run . ref -repo "$REPO" -ref "$REF" -upstream "$UPSTREAM" -advisory -receipt /dev/stdout diff --git a/Makefile b/Makefile index 2a2d55df..fad8fbdb 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: test test-secret-broker-request test-secret-broker-response test-semver-validation test-setup-semstat test-linear-pr-commenter test-link-backport-prs test-select-backport-source-prs test-release-notification test-linear-release-sync test-linear-cve-summary test-aws-test-infra test-cleanup-head-charts test-ci-test-notify test-auto-approve-bot-prs test-ai-pr-review test-ai-step test-release-summary test-publish-helm-chart test-govulncheck test-go-licenses test-run-ginkgo test-sticky-pr-comment test-repository-dispatch test-parse-label-filter test-release-branch-freeze test-prerelease-setup test-vcluster-release test-platform-release test-release-lib test-subtree-mirror test-oss-commit-sync test-oss-mirror-staleness test-backport-legacy-allowlist test-backport-legacy-split test-backport-fix-imports test-promote-release test-wait-for-release test-cve-scan test-commitlint test-resolve-github-release test-comment-triggered-check test-conflict-check build-linear-release-sync lint check-bats-jobs install-auto-doc generate-docs check-docs help +.PHONY: test test-secret-broker-request test-secret-broker-response test-semver-validation test-setup-semstat test-linear-pr-commenter test-link-backport-prs test-fork-review-check test-select-backport-source-prs test-release-notification test-linear-release-sync test-linear-cve-summary test-aws-test-infra test-cleanup-head-charts test-ci-test-notify test-auto-approve-bot-prs test-ai-pr-review test-ai-step test-release-summary test-publish-helm-chart test-govulncheck test-go-licenses test-run-ginkgo test-sticky-pr-comment test-repository-dispatch test-parse-label-filter test-release-branch-freeze test-prerelease-setup test-vcluster-release test-platform-release test-release-lib test-subtree-mirror test-oss-commit-sync test-oss-mirror-staleness test-backport-legacy-allowlist test-backport-legacy-split test-backport-fix-imports test-promote-release test-wait-for-release test-cve-scan test-commitlint test-resolve-github-release test-comment-triggered-check test-conflict-check build-linear-release-sync lint check-bats-jobs install-auto-doc generate-docs check-docs help ACTIONS_DIR := .github/actions WORKFLOWS_DIR := .github/workflows @@ -93,7 +93,7 @@ check-docs: generate-docs ## verify docs are up to date (fails if drift detected help: ## show this help @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {printf " %-30s %s\n", $$1, $$2}' -test: test-secret-broker-request test-secret-broker-response test-semver-validation test-setup-semstat test-linear-pr-commenter test-link-backport-prs test-select-backport-source-prs test-release-notification test-linear-release-sync test-linear-cve-summary test-aws-test-infra test-cleanup-head-charts test-auto-approve-bot-prs test-ai-pr-review test-ai-step test-release-summary test-ci-test-notify test-go-licenses test-publish-helm-chart test-govulncheck test-run-ginkgo test-sticky-pr-comment test-repository-dispatch test-parse-label-filter test-release-branch-freeze test-prerelease-setup test-vcluster-release test-platform-release test-release-lib test-subtree-mirror test-oss-commit-sync test-oss-mirror-staleness test-backport-legacy-allowlist test-backport-legacy-split test-backport-fix-imports test-promote-release test-wait-for-release test-cve-scan test-commitlint test-resolve-github-release test-comment-triggered-check test-conflict-check ## run all action tests +test: test-secret-broker-request test-secret-broker-response test-semver-validation test-setup-semstat test-linear-pr-commenter test-link-backport-prs test-fork-review-check test-select-backport-source-prs test-release-notification test-linear-release-sync test-linear-cve-summary test-aws-test-infra test-cleanup-head-charts test-auto-approve-bot-prs test-ai-pr-review test-ai-step test-release-summary test-ci-test-notify test-go-licenses test-publish-helm-chart test-govulncheck test-run-ginkgo test-sticky-pr-comment test-repository-dispatch test-parse-label-filter test-release-branch-freeze test-prerelease-setup test-vcluster-release test-platform-release test-release-lib test-subtree-mirror test-oss-commit-sync test-oss-mirror-staleness test-backport-legacy-allowlist test-backport-legacy-split test-backport-fix-imports test-promote-release test-wait-for-release test-cve-scan test-commitlint test-resolve-github-release test-comment-triggered-check test-conflict-check ## run all action tests test-secret-broker-request: ## run secret-broker-request pytest tests uv run --with pytest==8.3.3 pytest $(ACTIONS_DIR)/secret-broker-request/test/ -q @@ -113,6 +113,9 @@ test-linear-pr-commenter: ## run linear-pr-commenter unit tests test-link-backport-prs: ## run link-backport-prs unit tests cd $(ACTIONS_DIR)/link-backport-prs/src && go test -v ./... +test-fork-review-check: ## run fork-review-check unit tests + cd $(ACTIONS_DIR)/fork-review-check/src && go test -v ./... + test-select-backport-source-prs: ## run select-backport-source-prs bats tests bats $(ACTIONS_DIR)/select-backport-source-prs/test/*.bats diff --git a/README.md b/README.md index 18033aa8..2565ec46 100644 --- a/README.md +++ b/README.md @@ -157,6 +157,18 @@ Links modern and legacy backport PRs to the matching Linear sub-issue (`[X.Y] Co See [link-backport-prs README](./.github/actions/link-backport-prs/README.md) for detailed documentation. +### Fork Review Check + +CLI that verifies every fork commit a product build consumes reached its loft-sh fork through a merged pull request approved, at its final head, by someone with write access who is not its author. Read-only, build step 1 of the [fork review check design](https://github.com/loft-sh/architecture/blob/main/engineering-enablement/designs/fork-review-check.md); the required workflow and release receipt come next. + +**Location:** `.github/actions/fork-review-check` + +```bash +go run ./.github/actions/fork-review-check/src ref -repo loft-sh/kine -ref v0.16.5.vcluster.1 +``` + +See [fork-review-check README](./.github/actions/fork-review-check/README.md) for the rules, flags and receipt. + ### Select Backport Source PRs Action Selects one explicit source PR or merged source PRs inside a lookback window