diff --git a/README.md b/README.md index 56fcec95..be659ffe 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,10 @@ Contributions are welcome. See [CONTRIBUTING.md](./CONTRIBUTING.md) to get start ## Requirements +Supported targets are `github.com` and GitHub Enterprise Cloud with data +residency (`*.ghe.com`), subject to the [availability note below](#github-enterprise-cloud-with-data-residency). +GitHub Enterprise Server (GHES) is not supported. + Requires the [`gh` CLI](https://cli.github.com/). Install it first, then install the extension: ```bash @@ -48,6 +52,141 @@ lockfile to the new SHA. Suspicious pins whose recorded commit is no longer reachable upstream are left as errors — use `--accept-moved` to re-resolve those as well. +### GitHub Enterprise Cloud with data residency + +> [!IMPORTANT] +> Hostname-aware tenant/public resolution is not yet released. It is being +> developed in [#137](https://github.com/github/gh-actions-lock/pull/137); +> neither v0.1.6 nor v0.1.7-rc.1 includes it. Installing or upgrading the +> published extension does not install this draft implementation. + +With a build that includes this support, authenticate `gh` to your tenant, then +run the extension from your tenant repository checkout: + +```bash +gh auth login --hostname octocorp.ghe.com +# From the repository checkout: +gh actions-lock +``` + +With no conflicting environment overrides, the CLI infers the host from the +repository remote and uses the credentials stored by `gh` for that host. You do +not need to export a token or pass `--hostname` on every run. The account must +have read access to the tenant repositories used by your workflows. + +#### Host and credential overrides + +Host selection and credential selection are separate. Host selection uses the +first available source: + +1. `--hostname`. +2. `GH_HOST`. +3. The current repository from `gh`: `GH_REPO` if set, otherwise a remote on a + host known to `gh`. Among eligible remotes, `upstream` takes precedence over + `github`, then `origin`. +4. `github.com` if the current repository cannot be determined. + +A host-qualified `GH_REPO` such as `octocorp.ghe.com/OWNER/REPO` overrides remote +discovery. An unqualified `OWNER/REPO` uses `gh`'s default host: the sole +configured host if there is one, otherwise `github.com` (unless `GH_HOST` is set). +Authenticate to the tenant before relying on remote discovery. For an +unambiguous host override: + +```bash +gh actions-lock --hostname octocorp.ghe.com --no-interactive +``` + +For `github.com` and `*.ghe.com`, the first **nonempty** credential source wins: +`GH_TOKEN`, then `GITHUB_TOKEN`, then stored credentials for that host. + +Stored credentials come from `gh` configuration or its secure credential store. +`GH_ENTERPRISE_TOKEN` does **not** select credentials for `*.ghe.com`. +Conversely, a dotcom `GH_TOKEN` can override valid stored tenant credentials +and cause a tenant `401`. `--hostname` does not override token environment +variables. For tenant requests, a rejected token is not retried using stored +credentials or anonymous access. + +#### Diagnose authentication without exposing tokens + +Check which overrides are set without printing their values: + +```bash +for name in GH_HOST GH_REPO GH_TOKEN GITHUB_TOKEN; do + if printenv "$name" >/dev/null; then + printf '%s is set\n' "$name" + fi +done +``` + +If the overrides are unintended, test stored tenant credentials with a +command-scoped clean environment. These commands do not change your shell's +environment or print token values: + +```bash +env -u GH_TOKEN -u GITHUB_TOKEN \ + gh auth status --hostname octocorp.ghe.com +env -u GH_TOKEN -u GITHUB_TOKEN \ + gh api --hostname octocorp.ghe.com user --silent +``` + +If needed, sign in without the conflicting token overrides: + +```bash +env -u GH_TOKEN -u GITHUB_TOKEN \ + gh auth login --hostname octocorp.ghe.com +``` + +Then, from the tenant checkout, bypass unintended host, repository, and token +overrides for a read-only remote check: + +```bash +env -u GH_HOST -u GH_REPO -u GH_TOKEN -u GITHUB_TOKEN \ + gh actions-lock --hostname octocorp.ghe.com --rescan --no-fix +``` + +Keep intentional overrides, especially in automation; supply a token valid for +the selected host instead. Do not share token values or use +`gh auth status --show-token` in diagnostic output. A `403` can also mean missing repository +access or an organization policy restriction; changing hosts or retrying +anonymously is not a remedy. + +#### Resolution and lockfile behavior + +New dependencies resolve on the tenant first. Only a repository-level `404` +permits fallback to a **public** repository on `github.com`. A tenant repository +shadows its dotcom namesake even when the requested ref is missing. Authorization +errors, rate limits, and network failures do not trigger fallback. + +Generation and refresh write v0.0.3. An omitted `hostname` binds a pin to the +home host: `github.com` in a dotcom repository, or the selected tenant in a +`*.ghe.com` repository. Tenant-local pins omit `hostname`; public dotcom pins +on a tenant explicitly record `hostname: github.com`. Dotcom-root output omits +`hostname`. No explicit tenant hostname is emitted. + +Proxima execution requires v0.0.3. During migration, legacy v0.0.1/v0.0.2 pins +retain their dotcom binding, with repository IDs verified before writing +explicit `github.com`. Older preview files with an explicit home-tenant hostname +are rewritten to omit it without changing the pinned identity. On Proxima, +recorded repository IDs are checked on the bound host before pins are reused. +An omitted v0.0.3 public pin from an older producer is now tenant-bound: a missing +tenant repository or mismatched IDs fails without falling back or rewriting the +pin. Restore a correctly host-bound lockfile or review the dependencies before +regenerating. Read-only checks do not migrate files or prove that their wire +format is accepted for execution. `--verify-local` checks coverage, not host +identity. + +Existing pins retain their SHA and host binding during ordinary runs. +`--rescan` and `--relock` do not change the bound host. Conflicting host +assignments for the same repository are rejected. + +Public fallback uses unauthenticated dotcom requests, subject to GitHub's +anonymous API rate limit. Tenant tokens and headers are never forwarded to +dotcom. + +If any dependency cannot be resolved, generation exits nonzero without writing +an incomplete lockfile. `--json` reports `valid: false`. `--verify-local` checks +only recorded coverage; it does not prove successful remote resolution. + ### Self repository actions (`$/…`) `uses: $/…` references an action or reusable workflow in the **same repository** as diff --git a/cmd/gh-actions-lock/command_test.go b/cmd/gh-actions-lock/command_test.go index c3ca7c62..fdeba99d 100644 --- a/cmd/gh-actions-lock/command_test.go +++ b/cmd/gh-actions-lock/command_test.go @@ -18,6 +18,24 @@ import ( "github.com/stretchr/testify/require" ) +func TestCheckCommand_HelpExplainsHostAndAuthOverrides(t *testing.T) { + cmd := newRootCmd(nil) + var out strings.Builder + cmd.SetOut(&out) + cmd.SetArgs([]string{"--help"}) + require.NoError(t, cmd.Execute()) + for _, text := range []string{ + "Host selection: --hostname, then GH_HOST", + "GH_REPO or a remote on a host known to gh", + "GH_TOKEN takes precedence over GITHUB_TOKEN", + "GitHub Enterprise Server (GHES) is not supported.", + "--hostname selects the host; it does not override token variables.", + "env -u GH_TOKEN -u GITHUB_TOKEN gh auth status --hostname TENANT.ghe.com", + } { + assert.Contains(t, out.String(), text) + } +} + func TestCheckCommand_JSONWithHTTPMocks(t *testing.T) { reg := &httpmock.Registry{} defer reg.Verify(t) @@ -1030,6 +1048,7 @@ jobs: for _, f := range payload.Findings { if f.Category == "ref-moved" { hasRefMoved = true + assert.Equal(t, "run `gh actions-lock --relock` to refresh the lock entry", f.Remediation) } } assert.True(t, hasRefMoved, diff --git a/cmd/gh-actions-lock/format/json.go b/cmd/gh-actions-lock/format/json.go index 6a4c677b..e186e820 100644 --- a/cmd/gh-actions-lock/format/json.go +++ b/cmd/gh-actions-lock/format/json.go @@ -74,6 +74,7 @@ type Finding struct { // Dependency is the JSON-safe view of a resolved dependency, deduplicated // across workflows in the JSON output. type Dependency struct { + Hostname string `json:"hostname,omitempty"` NWO string `json:"nwo"` Ref string `json:"ref"` SHA string `json:"sha"` @@ -176,6 +177,7 @@ func WriteJSON(w io.Writer, report *checks.Report, valid bool, fieldsCSV, cliVer continue } d := Dependency{ + Hostname: inv.Dep.Hostname, NWO: inv.Dep.NWO, Ref: inv.Dep.Ref, SHA: inv.Dep.SHA, @@ -212,6 +214,7 @@ func WriteJSON(w io.Writer, report *checks.Report, valid bool, fieldsCSV, cliVer } for _, inv := range wr.Inventory { wf.Dependencies = append(wf.Dependencies, Dependency{ + Hostname: inv.Dep.Hostname, NWO: inv.Dep.NWO, Ref: inv.Dep.Ref, SHA: inv.Dep.SHA, diff --git a/cmd/gh-actions-lock/format/terminal.go b/cmd/gh-actions-lock/format/terminal.go index 818bee11..c87d7da2 100644 --- a/cmd/gh-actions-lock/format/terminal.go +++ b/cmd/gh-actions-lock/format/terminal.go @@ -147,7 +147,7 @@ func renderTermFindingDetail(out *ui.UI, f checks.Finding, dep string) { if f.Category == checks.UnreachablePin && f.Dependency != nil { owner, repo := f.Dependency.OwnerRepo() if owner != "" { - out.TermDetail(" ↳ %s", out.TermDim(fmt.Sprintf("https://github.com/%s/%s/releases", owner, repo))) + out.TermDetail(" ↳ %s", out.TermDim(DepReleaseURL(f.Dependency.Hostname, owner+"/"+repo, nil))) } } if IsAlertedCategory(f.Category) && f.Remediation != "" { @@ -278,7 +278,7 @@ func renderFindingDetail(out *ui.UI, f checks.Finding, dep string) { if f.Category == checks.UnreachablePin && f.Dependency != nil { owner, repo := f.Dependency.OwnerRepo() if owner != "" { - out.Detail(" ↳ %s", out.Dim(fmt.Sprintf("https://github.com/%s/%s/releases", owner, repo))) + out.Detail(" ↳ %s", out.Dim(DepReleaseURL(f.Dependency.Hostname, owner+"/"+repo, nil))) } } if IsAlertedCategory(f.Category) && f.Remediation != "" { diff --git a/cmd/gh-actions-lock/format/url.go b/cmd/gh-actions-lock/format/url.go index 883c8794..1a806f68 100644 --- a/cmd/gh-actions-lock/format/url.go +++ b/cmd/gh-actions-lock/format/url.go @@ -17,17 +17,20 @@ type TagObjectCheck func(owner, repo, sha string) bool // /commit/ returns 404 because the tag object is not a // commit. Non-SHA refs link to /releases/tag/. A nil isTagObject // (or one that returns false) falls back to the plain /commit/ path. -func DepReleaseURL(dep string, isTagObject TagObjectCheck) string { +func DepReleaseURL(hostname, dep string, isTagObject TagObjectCheck) string { + if hostname == "" { + hostname = "github.com" + } ar := parserlock.ParseActionRef(dep) if ar == nil { // ParseActionRef rejects refless inputs; fall back to splitting // the bare NWO so links to dep keys without a ref still render. if owner, repo, ok := parserlock.SplitNWO(dep); ok { - return "https://github.com/" + owner + "/" + repo + "/releases" + return "https://" + hostname + "/" + owner + "/" + repo + "/releases" } return "" } - base := "https://github.com/" + ar.Owner + "/" + ar.Repo + base := "https://" + hostname + "/" + ar.Owner + "/" + ar.Repo ref := ar.Ref if isHexSHA(ref) { if isTagObject != nil && isTagObject(ar.Owner, ar.Repo, ref) { diff --git a/cmd/gh-actions-lock/format/url_test.go b/cmd/gh-actions-lock/format/url_test.go index ec09cbd3..0db0055d 100644 --- a/cmd/gh-actions-lock/format/url_test.go +++ b/cmd/gh-actions-lock/format/url_test.go @@ -22,10 +22,17 @@ func TestDepReleaseURL(t *testing.T) { tests := []struct { name string + hostname string dep string isTagObject TagObjectCheck want string }{ + { + name: "tenant release stays on its host", + hostname: "tenant.ghe.com", + dep: "o/r@v1", + want: "https://tenant.ghe.com/o/r/releases/tag/v1", + }, { name: "commit-sha pin → /commit/", dep: "actions/checkout@" + commitSHA, @@ -83,7 +90,7 @@ func TestDepReleaseURL(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.want, DepReleaseURL(tt.dep, tt.isTagObject)) + assert.Equal(t, tt.want, DepReleaseURL(tt.hostname, tt.dep, tt.isTagObject)) }) } } diff --git a/cmd/gh-actions-lock/pin_summary.go b/cmd/gh-actions-lock/pin_summary.go index cdd8375e..de569230 100644 --- a/cmd/gh-actions-lock/pin_summary.go +++ b/cmd/gh-actions-lock/pin_summary.go @@ -380,7 +380,7 @@ func renderInvestigationAlerts(console *ui.UI, investigated []pin.Entry, r *reso ui.Pluralize(len(groups), "requires", "require")) for _, g := range groups { dep := g.NWO + "@" + g.Ref - console.TermDetail(" %s", console.TermLink(console.TermYellow(dep), format.DepReleaseURL(dep, r.IsKnownTagObject))) + console.TermDetail(" %s", console.TermLink(console.TermYellow(dep), format.DepReleaseURL(g.Hostname, dep, r.IsKnownTagObject))) for _, wf := range g.workflows { console.TermDetail(" └─ %s", console.TermDim(wf)) } diff --git a/cmd/gh-actions-lock/proxima_test.go b/cmd/gh-actions-lock/proxima_test.go new file mode 100644 index 00000000..6b7d234e --- /dev/null +++ b/cmd/gh-actions-lock/proxima_test.go @@ -0,0 +1,231 @@ +package main + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "strings" + "testing" + + parserlock "github.com/github/actions-lockfile/go/pkg/lockfile" + "github.com/github/gh-actions-lock/internal/ghapi/httpmock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type proximaTransport func(*http.Request) (*http.Response, error) + +func (f proximaTransport) RoundTrip(req *http.Request) (*http.Response, error) { return f(req) } + +const tenantSHA = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +const publicSHA = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + +func proximaFixture(t *testing.T, publicStatus int) http.RoundTripper { + t.Helper() + return proximaTransport(func(req *http.Request) (*http.Response, error) { + tenant := req.URL.Host == "api.tenant.ghe.com" + if tenant { + assert.NotEmpty(t, req.Header.Get("Authorization")) + } else { + assert.Equal(t, "api.github.com", req.URL.Host) + assert.Empty(t, req.Header.Get("Authorization")) + assert.Empty(t, req.Header.Get("Cookie")) + } + path := req.URL.Path + switch { + case tenant && path == "/repos/tenant/internal": + return httpmock.JSONResponse(map[string]any{ + "visibility": "internal", "id": 111, "owner": map[string]any{"id": 11}, + })(req) + case path == "/repos/actions/public": + if tenant { + return httpmock.StatusResponse(http.StatusNotFound)(req) + } + if publicStatus != 200 { + return httpmock.StatusResponse(publicStatus)(req) + } + return httpmock.JSONResponse(map[string]any{ + "visibility": "public", "id": 222, "owner": map[string]any{"id": 22}, + })(req) + case tenant && path == "/graphql": + var body struct { + Variables map[string]string `json:"variables"` + } + if err := json.NewDecoder(req.Body).Decode(&body); err != nil { + return nil, err + } + assert.Equal(t, "internal", body.Variables["name0"]) + composite := "runs:\n using: composite\n steps:\n - uses: actions/public@v2\n" + return httpmock.JSONResponse(map[string]any{"data": map[string]any{ + "a0": testRepoResponse("tenant/internal", tenantSHA, composite), + }})(req) + case !tenant && path == "/repos/actions/public/commits/v2": + return httpmock.JSONResponse(map[string]any{"sha": publicSHA})(req) + case !tenant && path == "/repos/actions/public/contents/action.yml": + assert.Equal(t, publicSHA, req.URL.Query().Get("ref")) + return &http.Response{StatusCode: 200, Header: http.Header{}, Body: io.NopCloser(strings.NewReader(nodeActionYAML)), Request: req}, nil + case strings.HasSuffix(path, "/releases"): + return httpmock.JSONResponse([]any{})(req) + } + return nil, fmt.Errorf("unexpected request: %s %s", req.Method, req.URL) + }) +} + +func TestProximaGenerationPreservesHosts(t *testing.T) { + path := writeTempWorkflow(t, ` +name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: tenant/internal@v1 + - uses: actions/public@v2 +`) + args := []string{"--hostname", "tenant.ghe.com", "--no-interactive", "--no-narrow", "--no-migrate-local-actions", "--json", path} + _, _, err := runCommandWithHTTP(t, proximaFixture(t, 200), args...) + require.NoError(t, err) + raw := readTempLockfilePins(t) + file, err := parserlock.Parse([]byte(raw)) + require.NoError(t, err) + assert.Equal(t, "v0.0.3", file.Version) + require.Len(t, file.Dependencies, 2) + local := file.Dependencies["tenant/internal@v1"] + assert.Empty(t, local.Hostname) + assert.Equal(t, "sha1-"+tenantSHA, local.Commit) + assert.EqualValues(t, 11, local.OwnerID) + assert.EqualValues(t, 111, local.RepoID) + assert.Equal(t, []string{"actions/public@v2"}, local.Uses) + public := file.Dependencies["actions/public@v2"] + assert.Equal(t, "github.com", public.Hostname) + assert.Equal(t, "sha1-"+publicSHA, public.Commit) + assert.EqualValues(t, 22, public.OwnerID) + assert.EqualValues(t, 222, public.RepoID) + + _, _, err = runCommandWithHTTP(t, proximaFixture(t, 200), args...) + require.NoError(t, err) + assert.Equal(t, raw, readTempLockfilePins(t), "repeat generation must retain hosts and IDs") + + base := proximaFixture(t, 200) + pinnedTransport := proximaTransport(func(req *http.Request) (*http.Response, error) { + assert.False(t, req.URL.Host == "api.tenant.ghe.com" && req.URL.Path == "/repos/actions/public", + "recorded dotcom pins must bypass tenant namesakes") + return base.RoundTrip(req) + }) + _, _, err = runCommandWithHTTP(t, pinnedTransport, + "--hostname", "tenant.ghe.com", "--rescan", "--no-fix", "--json", path) + require.NoError(t, err) + assert.Equal(t, raw, readTempLockfilePins(t)) + + // Convert the previous preview's explicit home host without moving its pin. + explicit := strings.Replace(raw, "'tenant/internal@v1':\n", "'tenant/internal@v1':\n hostname: 'tenant.ghe.com'\n", 1) + require.NotEqual(t, raw, explicit) + require.NoError(t, os.WriteFile(parserlock.Path, []byte(explicit), 0o600)) + _, _, err = runCommandWithHTTP(t, proximaFixture(t, 200), args...) + require.NoError(t, err) + assert.Equal(t, raw, readTempLockfilePins(t)) +} + +func TestProximaOmittedPinsCannotMoveToDotcomOrTenantNamesakes(t *testing.T) { + for _, status := range []int{200, 401, 403, 404} { + t.Run(fmt.Sprint(status), func(t *testing.T) { + path := writeTempWorkflow(t, ` +name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/public@v2 +`, "actions/public@v2=sha1-"+publicSHA) + raw := readTempLockfilePins(t) + raw = strings.ReplaceAll(raw, "owner_id: 1", "owner_id: 22") + raw = strings.ReplaceAll(raw, "repo_id: 1", "repo_id: 222") + require.NoError(t, os.WriteFile(parserlock.Path, []byte(raw), 0o600)) + transport := proximaTransport(func(req *http.Request) (*http.Response, error) { + assert.Equal(t, "api.tenant.ghe.com", req.URL.Host, "omitted pins must not fall back to dotcom") + assert.Equal(t, "/repos/actions/public", req.URL.Path) + assert.NotEmpty(t, req.Header.Get("Authorization")) + if status != 200 { + return httpmock.StatusResponse(status)(req) + } + return httpmock.JSONResponse(map[string]any{ + "visibility": "internal", "id": 333, "owner": map[string]any{"id": 33}, + })(req) + }) + _, _, err := runCommandWithHTTP(t, transport, + "--hostname", "tenant.ghe.com", "--no-narrow", "--no-migrate-local-actions", "--json", path) + require.Error(t, err) + if status == 200 { + assert.ErrorContains(t, err, "does not match its tenant.ghe.com repository IDs") + } else { + assert.ErrorContains(t, err, "verifying repository identity") + } + assert.Equal(t, raw, readTempLockfilePins(t)) + }) + } +} + +func TestProximaIncompleteGenerationDoesNotWrite(t *testing.T) { + for _, status := range []int{404, 403, 429, 500} { + t.Run(fmt.Sprint(status), func(t *testing.T) { + path := writeTempWorkflow(t, ` +name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: tenant/internal@v1 +`) + before, err := os.ReadFile(path) + require.NoError(t, err) + stdout, _, err := runCommandWithHTTP(t, proximaFixture(t, status), + "--hostname", "tenant.ghe.com", "--no-narrow", "--no-migrate-local-actions", "--json", path) + require.ErrorIs(t, err, errSilent) + var result struct { + Valid bool `json:"valid"` + } + require.NoError(t, json.Unmarshal([]byte(stdout), &result)) + assert.False(t, result.Valid) + _, err = os.Stat(parserlock.Path) + assert.True(t, os.IsNotExist(err)) + after, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, before, after) + }) + } +} + +func TestProximaLegacyPinsRequireDotcomIdentity(t *testing.T) { + for _, matches := range []bool{true, false} { + t.Run(fmt.Sprintf("matching IDs=%v", matches), func(t *testing.T) { + path := writeTempWorkflow(t, ` +name: ci +on: push +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/public@v2 +`, "actions/public@v2=sha1-"+publicSHA) + raw := strings.ReplaceAll(readTempLockfilePins(t), "v0.0.3", "v0.0.2") + if matches { + raw = strings.ReplaceAll(raw, "owner_id: 1", "owner_id: 22") + raw = strings.ReplaceAll(raw, "repo_id: 1", "repo_id: 222") + } + require.NoError(t, os.WriteFile(parserlock.Path, []byte(raw), 0o600)) + _, _, err := runCommandWithHTTP(t, proximaFixture(t, 200), + "--hostname", "tenant.ghe.com", "--no-narrow", "--no-migrate-local-actions", "--json", path) + if matches { + require.NoError(t, err) + assert.Contains(t, readTempLockfilePins(t), "hostname: 'github.com'") + } else { + require.ErrorContains(t, err, "regenerate the lockfile") + assert.Equal(t, raw, readTempLockfilePins(t)) + } + }) + } +} diff --git a/cmd/gh-actions-lock/root.go b/cmd/gh-actions-lock/root.go index 7ce32bcb..1d67ec9b 100644 --- a/cmd/gh-actions-lock/root.go +++ b/cmd/gh-actions-lock/root.go @@ -83,6 +83,27 @@ Scans all workflows under .github/workflows/ by default and fixes what it can — pinning every resolvable action and updating the lockfile. Pass --no-fix for a read-only check that writes nothing. +HOST AND AUTHENTICATION + +Supported targets are github.com and GitHub Enterprise Cloud with data +residency (*.ghe.com). GitHub Enterprise Server (GHES) is not supported. + +In a tenant repository checkout, authenticate with +gh auth login --hostname TENANT.ghe.com, then run gh actions-lock. + +Host selection: --hostname, then GH_HOST, then the current repository +(GH_REPO or a remote on a host known to gh), then github.com. +For github.com and *.ghe.com, GH_TOKEN takes precedence over GITHUB_TOKEN +and stored per-host credentials. +--hostname selects the host; it does not override token variables. + +If an unintended token override causes authentication to fail, check +stored tenant credentials without exposing tokens: + env -u GH_TOKEN -u GITHUB_TOKEN gh auth status --hostname TENANT.ghe.com + +For setup and troubleshooting: + https://github.com/github/gh-actions-lock#github-enterprise-cloud-with-data-residency + REF NARROWING When a new workflow is first pinned and uses a partial version ref @@ -208,6 +229,12 @@ func newRun(workflowPaths []string, hostname string, pool *pinpool.Pool, newReso // Now that the resolver is available, set it as the metadata resolver // for the store and re-seed branch hints. store.SetMetadataResolver(r) + if err := store.SetHostname(r.Hostname()); err != nil { + return nil, nil, nil, err + } + if err := r.SeedHosts(store.AllDeps()); err != nil { + return nil, nil, nil, err + } r.SeedBranchHints(store.AllDeps()) return paths, r, store, nil diff --git a/cmd/gh-actions-lock/run.go b/cmd/gh-actions-lock/run.go index 547d5882..1c77a45f 100644 --- a/cmd/gh-actions-lock/run.go +++ b/cmd/gh-actions-lock/run.go @@ -176,6 +176,9 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc) if err != nil { return err } + if err := store.VerifyHosts(ctx); err != nil { + return err + } // Pre-warm resolver caches from the lockfile so repeat runs skip // redundant GraphQL and REST calls. Skipped when --rescan is set: // a full re-verification must hit the network to detect ref movement. @@ -383,8 +386,26 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc) PartialScan: !fullScan, }) endPlan() + if planErr == nil && len(record.Unresolved()) > 0 { + planErr = fmt.Errorf("cannot write an incomplete lockfile: %d unresolved dependencies", len(record.Unresolved())) + } if planErr != nil { console.StopProgress() + if opts.jsonFields != "" { + if err := format.WriteJSON(out, report, false, opts.jsonFields, cliVersion(), store.File().Version); err != nil { + return err + } + } + if len(record.Unresolved()) > 0 { + record.Repo = &pin.RepoInfo{Owner: repoOwner, Name: repoName, Host: r.Hostname()} + if opts.jsonFields == "" { + renderUnresolvedWarnings(console, record.Unresolved()) + } + if path, err := record.WriteJSON(); err == nil && opts.jsonFields == "" { + console.TermDetail("Resolution record: %s", path) + } + return errSilent + } return fmt.Errorf("planning pins: %w", planErr) } diff --git a/cmd/gh-actions-lock/verify.go b/cmd/gh-actions-lock/verify.go index 68bfc100..c5038e1a 100644 --- a/cmd/gh-actions-lock/verify.go +++ b/cmd/gh-actions-lock/verify.go @@ -62,6 +62,9 @@ func runVerifyLocal(opts *checkOptions, out io.Writer, console *ui.UI) error { if err != nil { return fmt.Errorf("opening lockfile: %w", err) } + if err := store.SetHostname(resolveHostname(opts.hostname)); err != nil { + return err + } parsed := pipeline.ParseAll(paths, store) report := pipeline.VerifyLocalCoverage(parsed, store) diff --git a/go.mod b/go.mod index 9e878958..d630b0eb 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( gopkg.in/yaml.v3 v3.0.1 ) -require github.com/github/actions-lockfile/go v0.0.4 +require github.com/github/actions-lockfile/go v0.0.6-rc.1 require ( github.com/AlecAivazis/survey/v2 v2.3.7 // indirect diff --git a/go.sum b/go.sum index c654236f..77411639 100644 --- a/go.sum +++ b/go.sum @@ -33,8 +33,8 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= -github.com/github/actions-lockfile/go v0.0.4 h1:FP2KrJNhBti8rR7+Z8TKycJdWFc0gzuaj0wTlANoFdY= -github.com/github/actions-lockfile/go v0.0.4/go.mod h1:kp8pDNXwrr3fC+6Mgmh/ZODa6AsIEC+bmf1CLQ/7DEs= +github.com/github/actions-lockfile/go v0.0.6-rc.1 h1:IDYDO9NX1MpnOA97+0Nu+tfsbqyhbtNlrgIK5wnepfY= +github.com/github/actions-lockfile/go v0.0.6-rc.1/go.mod h1:kp8pDNXwrr3fC+6Mgmh/ZODa6AsIEC+bmf1CLQ/7DEs= github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 h1:2VTzZjLZBgl62/EtslCrtky5vbi9dd7HrQPQIx6wqiw= github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542/go.mod h1:Ow0tF8D4Kplbc8s8sSb3V2oUCygFHVp8gC3Dn6U4MNI= github.com/henvic/httpretty v0.0.6 h1:JdzGzKZBajBfnvlMALXXMVQWxWMF/ofTy8C3/OSUTxs= diff --git a/internal/dep/dependency.go b/internal/dep/dependency.go index 6fd82fb1..199fba9a 100644 --- a/internal/dep/dependency.go +++ b/internal/dep/dependency.go @@ -15,7 +15,8 @@ import ( // resolver traversal, and lockfile serialization — never persisted on disk // and not part of any public API. type Dependency struct { - NWO string // owner/repo (no path) + Hostname string // owning GitHub instance; empty means github.com + NWO string // owner/repo (no path) // Path is the optional sub-action subpath as written in `uses:` // (e.g. "save" for actions/cache/save). It is preserved on the // in-memory dep so resolver-time graph traversal can fetch the diff --git a/internal/ghapi/client.go b/internal/ghapi/client.go index b007c30c..ae78e948 100644 --- a/internal/ghapi/client.go +++ b/internal/ghapi/client.go @@ -14,6 +14,7 @@ import ( "net/http" "os" "strconv" + "strings" "time" "github.com/cli/go-gh/v2/pkg/api" @@ -29,6 +30,11 @@ type Client struct { rest *api.RESTClient Hostname string restOnly bool + local *Client + public *Client + routes syncmap.Map[Repo, *Client] + routeSF singleflight.Group + pinned map[Repo]string // anonBaseURL overrides the base URL for anonymous REST fallback calls. // Empty uses the default "https://api.". Set in tests. @@ -63,6 +69,7 @@ type clientConfig struct { profile *profile.Session authToken string // non-empty → use explicit token (tests) logIgnore bool // suppress log env vars (tests) + anonymous bool } // WithClientTransport overrides the HTTP transport. Use in tests with httpmock. @@ -83,10 +90,26 @@ func WithClientProfile(p *profile.Session) ClientOption { // ambient gh credential store. Use WithClientTransport for test stubs // and WithClientProfile for profiling. func New(hostname string, opts ...ClientOption) (*Client, error) { + hostname = strings.ToLower(hostname) if hostname == "" { hostname = "github.com" } + local, err := newClient(hostname, opts...) + if err != nil || !IsProxima(hostname) { + return local, err + } + // GH_TOKEN applies to both dotcom and Proxima in go-gh. Public fallback + // must never reuse it across those trust domains. + publicOpts := append([]ClientOption(nil), opts...) + publicOpts = append(publicOpts, func(c *clientConfig) { c.anonymous = true }) + public, err := newClient("github.com", publicOpts...) + if err != nil { + return nil, err + } + return &Client{Hostname: hostname, local: local, public: public, pinned: map[Repo]string{}}, nil +} +func newClient(hostname string, opts ...ClientOption) (*Client, error) { var cfg clientConfig for _, o := range opts { o(&cfg) @@ -118,6 +141,11 @@ func New(hostname string, opts ...ClientOption) (*Client, error) { } apiOpts.Transport = t } + if cfg.anonymous { + apiOpts.AuthToken = "anonymous" + apiOpts.Transport = anonymousTransport{inner: apiOpts.Transport} + c.restOnly = true + } gql, err := api.NewGraphQLClient(apiOpts) if err != nil { @@ -143,10 +171,22 @@ func New(hostname string, opts ...ClientOption) (*Client, error) { } else { c.anonHTTP = http.DefaultClient } + if cfg.anonymous { + c.anonHTTP = &http.Client{Transport: apiOpts.Transport} + } return c, nil } +type anonymousTransport struct{ inner http.RoundTripper } + +func (t anonymousTransport) RoundTrip(req *http.Request) (*http.Response, error) { + req = req.Clone(req.Context()) + req.Header.Del("Authorization") + req.Header.Del("Cookie") + return t.inner.RoundTrip(req) +} + // retryTransport wraps an http.RoundTripper with retry logic for transient // server errors (5xx), explicit rate limits (429), and GitHub secondary // rate limits (403 with Retry-After or X-RateLimit-Reset headers). diff --git a/internal/ghapi/graphql_action_files.go b/internal/ghapi/graphql_action_files.go index f64df865..37ff1a2f 100644 --- a/internal/ghapi/graphql_action_files.go +++ b/internal/ghapi/graphql_action_files.go @@ -27,6 +27,7 @@ func (r ActionFileRequest) NWO() string { return r.Owner + "/" + r.Repo } // for one ActionFileRequest. Err is non-nil when this specific ref could // not be resolved (e.g. not found, SSO required). type ActionFileResult struct { + Hostname string Owner string Repo string Path string @@ -68,6 +69,26 @@ func (c *Client) ResolveActionFiles(ctx context.Context, refs []ActionFileReques if len(refs) == 0 { return nil } + if c.local != nil { + results := make([]ActionFileResult, len(refs)) + groups := make(map[*Client][]ActionFileRequest) + indices := make(map[*Client][]int) + for i, ref := range refs { + client, err := c.ForRepo(ctx, ref.Owner, ref.Repo) + if err != nil { + results[i] = ActionFileResult{Owner: ref.Owner, Repo: ref.Repo, Path: ref.Path, Ref: ref.Ref, Err: err} + continue + } + groups[client] = append(groups[client], ref) + indices[client] = append(indices[client], i) + } + for client, batch := range groups { + for j, result := range client.ResolveActionFiles(ctx, batch) { + results[indices[client][j]] = result + } + } + return results + } if c.restOnly { results := make([]ActionFileResult, len(refs)) for i, ref := range refs { @@ -222,7 +243,7 @@ func buildActionFileQuery(refs []ActionFileRequest) (string, map[string]any, map func parseActionFileResponse(data map[string]json.RawMessage, refs []ActionFileRequest, aliasMap map[string]int, gqlErr *api.GraphQLError, hostname string) []ActionFileResult { results := make([]ActionFileResult, len(refs)) for i, r := range refs { - results[i] = ActionFileResult{Owner: r.Owner, Repo: r.Repo, Path: r.Path, Ref: r.Ref} + results[i] = ActionFileResult{Hostname: hostname, Owner: r.Owner, Repo: r.Repo, Path: r.Path, Ref: r.Ref} } samlOwners := samlBlockedOwners(gqlErr, refs, aliasMap) @@ -254,6 +275,23 @@ func parseActionFileResponse(data map[string]json.RawMessage, refs []ActionFileR } continue } + if gqlErr != nil { + for _, item := range gqlErr.Errors { + if len(item.Path) > 0 && item.Path[0] == alias { + // Both metadata spellings are queried; either may be absent. + // Reusable workflows can have neither. + if item.Type == "NOT_FOUND" && len(item.Path) == 3 && item.Path[1] == "object" && + (item.Path[2] == "file" || item.Path[2] == "fileYaml") { + continue + } + results[idx].Err = fmt.Errorf("%s", item.Message) + break + } + } + if results[idx].Err != nil { + continue + } + } var repo repoResponse if err := json.Unmarshal(raw, &repo); err != nil { diff --git a/internal/ghapi/graphql_action_files_test.go b/internal/ghapi/graphql_action_files_test.go index cdbf9c78..9d1fd47b 100644 --- a/internal/ghapi/graphql_action_files_test.go +++ b/internal/ghapi/graphql_action_files_test.go @@ -12,6 +12,8 @@ import ( "testing" "github.com/cli/go-gh/v2/pkg/api" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestBuildActionFileQuery(t *testing.T) { @@ -77,6 +79,85 @@ func TestParseActionFileResponse(t *testing.T) { } } +func TestParseActionFileResponse_FileErrors(t *testing.T) { + tests := []struct { + name string + file string + errors []api.GraphQLErrorItem + wantErr bool + }{ + { + name: "yml exists and yaml is absent", file: "file", + errors: []api.GraphQLErrorItem{{Type: "NOT_FOUND", Path: []any{"a0", "object", "fileYaml"}}}, + }, + { + name: "yaml exists and yml is absent", file: "fileYaml", + errors: []api.GraphQLErrorItem{{Type: "NOT_FOUND", Path: []any{"a0", "object", "file"}}}, + }, + { + name: "reusable workflow has neither metadata file", + errors: []api.GraphQLErrorItem{ + {Type: "NOT_FOUND", Path: []any{"a0", "object", "file"}}, + {Type: "NOT_FOUND", Path: []any{"a0", "object", "fileYaml"}}, + }, + }, + { + name: "forbidden alternate remains fatal", file: "file", wantErr: true, + errors: []api.GraphQLErrorItem{{Type: "FORBIDDEN", Path: []any{"a0", "object", "fileYaml"}}}, + }, + { + name: "unknown metadata failure remains fatal", wantErr: true, + errors: []api.GraphQLErrorItem{{Path: []any{"a0", "object", "file"}}}, + }, + { + name: "missing commit remains fatal", wantErr: true, + errors: []api.GraphQLErrorItem{{Type: "NOT_FOUND", Path: []any{"a0", "object"}}}, + }, + { + name: "nested object failure remains fatal", wantErr: true, + errors: []api.GraphQLErrorItem{{Type: "NOT_FOUND", Path: []any{"a0", "object", "file", "object"}}}, + }, + { + name: "missing file does not hide later denial", wantErr: true, + errors: []api.GraphQLErrorItem{ + {Type: "NOT_FOUND", Path: []any{"a0", "object", "file"}}, + {Type: "FORBIDDEN", Path: []any{"a0", "object", "fileYaml"}}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + const sha = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + object := map[string]any{"oid": sha} + if tt.file != "" { + object[tt.file] = map[string]any{"object": map[string]any{"text": "runs:\n using: node20\n"}} + } + raw, err := json.Marshal(map[string]any{"object": object}) + require.NoError(t, err) + for i := range tt.errors { + tt.errors[i].Message = "file lookup failed" + } + results := parseActionFileResponse( + map[string]json.RawMessage{"a0": raw}, + []ActionFileRequest{{Owner: "o", Repo: "r", Ref: "v1"}}, + map[string]int{"a0": 0}, + &api.GraphQLError{Errors: tt.errors}, "github.com") + require.Len(t, results, 1) + if tt.wantErr { + require.ErrorContains(t, results[0].Err, "file lookup failed") + return + } + require.NoError(t, results[0].Err) + assert.Equal(t, sha, results[0].CommitOID) + assert.Equal(t, "github.com", results[0].Hostname) + if tt.file == "" { + assert.Empty(t, results[0].ActionYML) + } else { + assert.Equal(t, "runs:\n using: node20\n", results[0].ActionYML) + } + }) + } +} func TestParseActionFileResponse_AnnotatedTagPeeled(t *testing.T) { refs := []ActionFileRequest{ {Owner: "nodeselector", Repo: "actions-test-fixtures", Ref: "annotated-v1"}, diff --git a/internal/ghapi/graphql_peel.go b/internal/ghapi/graphql_peel.go index d0a24b4a..f193cd56 100644 --- a/internal/ghapi/graphql_peel.go +++ b/internal/ghapi/graphql_peel.go @@ -33,6 +33,10 @@ type PeelTagObjectResult struct { // (not an error) when the OID or repo is not accessible — callers decide // how to interpret the negative. func (c *Client) PeelTagObject(ctx context.Context, owner, repo, sha string) (PeelTagObjectResult, error) { + c, err := c.ForRepo(ctx, owner, repo) + if err != nil { + return PeelTagObjectResult{}, err + } if c.restOnly { return c.anonPeelTagObject(ctx, owner, repo, sha) } diff --git a/internal/ghapi/graphql_reachability.go b/internal/ghapi/graphql_reachability.go index c48b112d..7bde399e 100644 --- a/internal/ghapi/graphql_reachability.go +++ b/internal/ghapi/graphql_reachability.go @@ -24,6 +24,10 @@ const batchReachabilitySize = 50 // - anyChecked: true if at least one branch was successfully checked // - err: non-nil only on transport/auth failures (not per-branch misses) func (c *Client) BatchBranchContains(ctx context.Context, owner, repo, sha string, branches []BranchHead) (matchedBranch string, anyChecked bool, err error) { + c, err = c.ForRepo(ctx, owner, repo) + if err != nil { + return "", false, err + } if len(branches) == 0 { return "", false, nil } diff --git a/internal/ghapi/hosts.go b/internal/ghapi/hosts.go new file mode 100644 index 00000000..70ac0aca --- /dev/null +++ b/internal/ghapi/hosts.go @@ -0,0 +1,82 @@ +package ghapi + +import ( + "context" + "fmt" + "net/http" + "regexp" +) + +var proximaHost = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\.ghe\.com$`) + +// IsProxima reports whether hostname is a canonical tenant hostname. +func IsProxima(hostname string) bool { return proximaHost.MatchString(hostname) } + +// PinHost binds a repository to its recorded host before any resolution starts. +// A repository cannot span hosts within a run: caches and ref selection are +// repository-scoped, and guessing would mix identities. +func (c *Client) PinHost(owner, repo, hostname string) error { + if hostname == "" { + hostname = c.Hostname + } + if hostname != c.Hostname && !(c.local != nil && hostname == "github.com") { + return fmt.Errorf("%s/%s is pinned to %s, not the selected host %s", owner, repo, hostname, c.Hostname) + } + if c.local == nil { + return nil + } + key := ForRepo(owner, repo) + if prev, ok := c.pinned[key]; ok && prev != hostname { + return fmt.Errorf("%s has conflicting lockfile hosts %s and %s", key, prev, hostname) + } + c.pinned[key] = hostname + return nil +} + +// ForRepo selects a host once per repository. Only a repository-level 404 +// permits public fallback; missing refs, denied access and transport errors do not. +func (c *Client) ForRepo(ctx context.Context, owner, repo string) (*Client, error) { + if c.local == nil { + return c, nil + } + key := ForRepo(owner, repo) + if selected, ok := c.routes.Get(key); ok { + return selected, nil + } + v, err, _ := c.routeSF.Do(key.String(), func() (any, error) { + if selected, ok := c.routes.Get(key); ok { + return selected, nil + } + selected := c.local + host := c.pinned[key] + if host != "github.com" { + _, err := c.local.repoMetadata(ctx, owner, repo) + if err != nil { + code, _ := StatusCode(err) + if host != "" || code != http.StatusNotFound { + return nil, err + } + selected = c.public + } + } else { + selected = c.public + } + if selected == c.public { + // ponytail: anonymous dotcom rate limit; use host-bound credentials + // if public fallback volume exceeds that limit. + meta, err := c.public.repoMetadata(ctx, owner, repo) + if err != nil { + return nil, err + } + if meta.Visibility != "public" { + return nil, fmt.Errorf("github.com/%s is not a public fallback repository", key) + } + } + c.routes.Put(key, selected) + return selected, nil + }) + if err != nil { + return nil, err + } + return v.(*Client), nil +} diff --git a/internal/ghapi/hosts_test.go b/internal/ghapi/hosts_test.go new file mode 100644 index 00000000..07f2e2a3 --- /dev/null +++ b/internal/ghapi/hosts_test.go @@ -0,0 +1,173 @@ +package ghapi + +import ( + "context" + "errors" + "fmt" + "net/http" + "testing" + + "github.com/cli/go-gh/v2/pkg/api" + "github.com/github/gh-actions-lock/internal/ghapi/httpmock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func hostRequest(host, path string) httpmock.Matcher { + return func(req *http.Request) bool { + return req.URL.Host == host && httpmock.REST("GET", path)(req) + } +} + +func TestProximaRepositorySelection(t *testing.T) { + for _, tt := range []struct { + name string + status int + pin string + visibility string + wantHost string + }{ + {name: "tenant repository shadows dotcom", status: 200, wantHost: "tenant.ghe.com"}, + {name: "repository 404 permits public fallback", status: 404, visibility: "public", wantHost: "github.com"}, + {name: "private dotcom repository rejected", status: 404, visibility: "private"}, + {name: "missing visibility rejected", status: 404}, + {name: "unauthorized does not fall back", status: 401}, + {name: "forbidden does not fall back", status: 403}, + {name: "rate limit does not fall back", status: 429}, + {name: "server failure does not fall back", status: 500}, + {name: "transport failure does not fall back", status: -1}, + {name: "pinned tenant 404 cannot change hosts", status: 404, pin: "tenant.ghe.com"}, + {name: "pinned dotcom bypasses tenant namesake", pin: "github.com", visibility: "public", wantHost: "github.com"}, + } { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + if tt.pin != "github.com" { + response := httpmock.StatusResponse(tt.status) + if tt.status == 200 { + response = httpmock.JSONResponse(map[string]any{"id": 20, "owner": map[string]any{"id": 10}}) + } else if tt.status == -1 { + response = func(*http.Request) (*http.Response, error) { return nil, errors.New("network unavailable") } + } + reg.Register(hostRequest("api.tenant.ghe.com", `repos/o/r$`), response) + } + if tt.pin == "github.com" || tt.status == 404 && tt.pin == "" { + reg.Register(hostRequest("api.github.com", `repos/o/r$`), + httpmock.JSONResponse(map[string]any{"visibility": tt.visibility, "id": 2, "owner": map[string]any{"id": 1}})) + } + c, err := New("tenant.ghe.com", WithClientTransport(reg), func(cfg *clientConfig) { + cfg.authToken = "tenant-only-secret" + }) + require.NoError(t, err) + if tt.pin != "" { + require.NoError(t, c.PinHost("o", "r", tt.pin)) + } + selected, err := c.ForRepo(context.Background(), "o", "r") + if tt.wantHost == "" { + require.Error(t, err) + assert.Nil(t, selected) + } else { + require.NoError(t, err) + assert.Equal(t, tt.wantHost, selected.Hostname) + again, err := c.ForRepo(context.Background(), "O", "R") + require.NoError(t, err) + assert.Same(t, selected, again) + ownerID, repoID, err := c.RepoIDs(context.Background(), "o", "r") + require.NoError(t, err) + if tt.wantHost == "github.com" { + assert.EqualValues(t, 1, ownerID) + assert.EqualValues(t, 2, repoID) + } else { + assert.EqualValues(t, 10, ownerID) + assert.EqualValues(t, 20, repoID) + } + } + for _, req := range reg.Requests { + if req.URL.Host == "api.github.com" { + assert.Empty(t, req.Header.Get("Authorization")) + assert.Empty(t, req.Header.Get("Cookie")) + } else { + assert.Contains(t, req.Header.Get("Authorization"), "tenant-only-secret") + } + } + }) + } +} + +func TestProximaMissingRefDoesNotFallBack(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + reg.Register(hostRequest("api.tenant.ghe.com", `repos/o/r$`), httpmock.JSONResponse(map[string]any{"id": 2, "owner": map[string]any{"id": 1}})) + reg.Register(httpmock.GraphQLForRepo("o", "r"), httpmock.JSONResponse(map[string]any{ + "data": map[string]any{"a0": map[string]any{"nameWithOwner": "o/r", "object": nil}}, + })) + c, err := New("tenant.ghe.com", WithClientTransport(reg)) + require.NoError(t, err) + results := c.ResolveActionFiles(context.Background(), []ActionFileRequest{{Owner: "o", Repo: "r", Ref: "missing"}}) + require.Len(t, results, 1) + require.Error(t, results[0].Err) + for _, req := range reg.Requests { + assert.Equal(t, "api.tenant.ghe.com", req.URL.Host) + } +} + +func TestPinnedHostBoundaries(t *testing.T) { + c, err := New("tenant.ghe.com", WithClientTransport(&httpmock.Registry{})) + require.NoError(t, err) + require.NoError(t, c.PinHost("o", "r", "github.com")) + require.ErrorContains(t, c.PinHost("O", "R", "tenant.ghe.com"), "conflicting") + require.NoError(t, c.PinHost("o", "home", "")) + require.ErrorContains(t, c.PinHost("o", "home", "github.com"), "conflicting") + require.ErrorContains(t, c.PinHost("o", "else", "other.ghe.com"), "not the selected host") + require.ErrorContains(t, c.PinHost("o", "else", "evil.example"), "not the selected host") + + for _, host := range []string{"github.com", "github.example.com", "evilghe.com", "tenant.ghe.com.evil.example"} { + c, err := New(host, WithClientTransport(&httpmock.Registry{})) + require.NoError(t, err) + assert.Nil(t, c.local, host) + selected, err := c.ForRepo(context.Background(), "o", "r") + require.NoError(t, err) + assert.Same(t, c, selected) + } +} + +func TestProximaContentErrorsAreNotLeafActions(t *testing.T) { + for _, status := range []int{403, 429, 500} { + t.Run(fmt.Sprint(status), func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + reg.Register(hostRequest("api.github.com", `repos/o/r$`), + httpmock.JSONResponse(map[string]any{"visibility": "public", "id": 2, "owner": map[string]any{"id": 1}})) + reg.Register(hostRequest("api.github.com", `repos/o/r/commits/v1$`), + httpmock.JSONResponse(map[string]any{"sha": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"})) + reg.Register(hostRequest("api.github.com", `repos/o/r/contents/action.yml$`), httpmock.StatusResponse(status)) + c, err := New("tenant.ghe.com", WithClientTransport(reg)) + require.NoError(t, err) + require.NoError(t, c.PinHost("o", "r", "github.com")) + result := c.ResolveActionFiles(context.Background(), []ActionFileRequest{{Owner: "o", Repo: "r", Ref: "v1"}}) + require.Len(t, result, 1) + var httpErr *api.HTTPError + require.ErrorAs(t, result[0].Err, &httpErr) + assert.Equal(t, status, httpErr.StatusCode) + assert.Empty(t, result[0].ActionYML) + }) + } +} + +func TestProximaPartialGraphQLErrorDoesNotSucceed(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + reg.Register(hostRequest("api.tenant.ghe.com", `repos/o/r$`), httpmock.JSONResponse(map[string]any{"id": 2, "owner": map[string]any{"id": 1}})) + reg.Register(httpmock.GraphQLForRepo("o", "r"), httpmock.JSONResponse(map[string]any{ + "data": map[string]any{"a0": map[string]any{ + "nameWithOwner": "o/r", "object": map[string]any{"oid": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "file": nil}, + }}, + "errors": []any{map[string]any{"type": "FORBIDDEN", "message": "content access denied", "path": []string{"a0", "object", "file"}}}, + })) + c, err := New("tenant.ghe.com", WithClientTransport(reg)) + require.NoError(t, err) + result := c.ResolveActionFiles(context.Background(), []ActionFileRequest{{Owner: "o", Repo: "r", Ref: "v1"}}) + require.Len(t, result, 1) + require.ErrorContains(t, result[0].Err, "content access denied") + assert.Empty(t, result[0].CommitOID) +} diff --git a/internal/ghapi/repos.go b/internal/ghapi/repos.go index cd2c8883..f6703758 100644 --- a/internal/ghapi/repos.go +++ b/internal/ghapi/repos.go @@ -30,6 +30,10 @@ type TagEntry struct { // Paginates up to 3 pages (300 branches). Results are cached per owner/repo // and coalesced via singleflight. func (c *Client) ListBranches(ctx context.Context, owner, repo string) ([]BranchHead, error) { + c, err := c.ForRepo(ctx, owner, repo) + if err != nil { + return nil, err + } key := ForRepo(owner, repo) if cached, ok := c.branchListCache.Get(key); ok { return cached, nil @@ -79,6 +83,10 @@ func (c *Client) ListBranches(ctx context.Context, owner, repo string) ([]Branch // ListTags returns all tags with their commit SHAs for a repo (first page, // up to 100). Results are cached per owner/repo and coalesced via singleflight. func (c *Client) ListTags(ctx context.Context, owner, repo string) ([]TagEntry, error) { + c, err := c.ForRepo(ctx, owner, repo) + if err != nil { + return nil, err + } key := ForRepo(owner, repo) if cached, ok := c.tagListCache.Get(key); ok { return cached, nil @@ -141,6 +149,10 @@ type repoMeta struct { // match/error, and a coalesced caller's cancellation must not abort the shared // fetch for the others waiting on it. func (c *Client) repoMetadata(ctx context.Context, owner, repo string) (repoMeta, error) { + c, err := c.ForRepo(ctx, owner, repo) + if err != nil { + return repoMeta{}, err + } key := ForRepo(owner, repo) if m, ok := c.repoMetaCache.Get(key); ok { return m, nil @@ -199,6 +211,10 @@ func (c *Client) GetDefaultBranch(ctx context.Context, owner, repo string) strin // 300-branch cap. Results (including 404s) are cached and concurrent lookups // are coalesced via singleflight. Returns ok=false on any error. func (c *Client) GetBranchHead(ctx context.Context, owner, repo, name string) (BranchHead, bool) { + c, err := c.ForRepo(ctx, owner, repo) + if err != nil { + return BranchHead{}, false + } if name == "" { return BranchHead{}, false } @@ -243,6 +259,10 @@ func (c *Client) GetBranchHead(ctx context.Context, owner, repo, name string) (B // any error yields whatever was collected so far (possibly empty). Results // are cached per owner/repo and coalesced via singleflight. func (c *Client) ListProtectedBranches(ctx context.Context, owner, repo string) []BranchHead { + c, err := c.ForRepo(ctx, owner, repo) + if err != nil { + return nil + } key := ForRepo(owner, repo) if cached, ok := c.protectedBranchCache.Get(key); ok { return cached @@ -284,6 +304,10 @@ func (c *Client) ListProtectedBranches(ctx context.Context, owner, repo string) // MatchingHeadRefs returns branches whose names start with prefix via the // git/matching-refs endpoint. Best-effort: any error yields nil. func (c *Client) MatchingHeadRefs(ctx context.Context, owner, repo, prefix string) []BranchHead { + c, err := c.ForRepo(ctx, owner, repo) + if err != nil { + return nil + } path := fmt.Sprintf("repos/%s/%s/git/matching-refs/heads/%s", url.PathEscape(owner), url.PathEscape(repo), escapeBranchPath(prefix)) var resp []struct { @@ -335,6 +359,10 @@ type compareResponse struct { // reachability scan cancels siblings on first match) cannot abort the shared // comparison the others are waiting on. func (c *Client) CompareCommits(ctx context.Context, owner, repo, sha, branchHeadSHA string) (bool, error) { + c, err := c.ForRepo(ctx, owner, repo) + if err != nil { + return false, err + } if strings.EqualFold(sha, branchHeadSHA) { return true, nil } @@ -383,6 +411,10 @@ func (c *Client) CompareCommits(ctx context.Context, owner, repo, sha, branchHea // ancestry, forgery, and inconclusive results. Not cached: ancestry checks // key on distinct base/head pairs that rarely repeat within a run. func (c *Client) CompareRefs(ctx context.Context, owner, repo, base, head string) (status, mergeBaseSHA string, err error) { + c, err = c.ForRepo(ctx, owner, repo) + if err != nil { + return "", "", err + } path := fmt.Sprintf("repos/%s/%s/compare/%s...%s", url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(base), url.PathEscape(head)) diff --git a/internal/ghapi/rest_fallback.go b/internal/ghapi/rest_fallback.go index 8fa8f4d1..f2c8ee02 100644 --- a/internal/ghapi/rest_fallback.go +++ b/internal/ghapi/rest_fallback.go @@ -9,6 +9,8 @@ import ( "net/url" "strings" "sync" + + "github.com/cli/go-gh/v2/pkg/api" ) // anonProbeCache caches per-owner results of unauthenticated access probes. @@ -20,6 +22,9 @@ var anonProbeCache sync.Map // map[string]bool // call for an owner, it probes the GitHub API with an unauthenticated // request to determine accessibility, then caches the result. func (c *Client) SSOFallbackEligible(ctx context.Context, owner string) bool { + if IsProxima(c.Hostname) { + return false + } key := c.anonBase() + "/" + owner if v, ok := anonProbeCache.Load(key); ok { return v.(bool) @@ -48,6 +53,9 @@ func (c *Client) SSOFallbackEligible(ctx context.Context, owner string) bool { } func (c *Client) repoFallbackEligible(ctx context.Context, owner, repo string, err error) bool { + if IsProxima(c.Hostname) { + return false + } code, _ := StatusCode(err) if !IsSAMLEnforcement(err) && code != http.StatusUnauthorized { return false @@ -246,10 +254,11 @@ func (c *Client) anonCompareCommits(ctx context.Context, owner, repo, sha, branc // repos and is used as a fallback when SSO blocks the authenticated path. func (c *Client) resolveAnonymous(ctx context.Context, ref ActionFileRequest) ActionFileResult { result := ActionFileResult{ - Owner: ref.Owner, - Repo: ref.Repo, - Path: ref.Path, - Ref: ref.Ref, + Hostname: c.Hostname, + Owner: ref.Owner, + Repo: ref.Repo, + Path: ref.Path, + Ref: ref.Ref, } base := c.anonBase() @@ -278,10 +287,18 @@ func (c *Client) resolveAnonymous(ctx context.Context, ref ActionFileRequest) Ac content, err := c.anonGetFileContent(ctx, base, ref.Owner, ref.Repo, sha, ymlPath) if err != nil { + if code, _ := StatusCode(err); code != http.StatusNotFound { + result.Err = err + return result + } // Try .yaml extension. content, err = c.anonGetFileContent(ctx, base, ref.Owner, ref.Repo, sha, yamlPath) if err != nil { - // Not fatal — some actions don't have action.yml (reusable workflows). + // Reusable workflows have no action metadata; other failures + // must not silently truncate a composite's dependency graph. + if code, _ := StatusCode(err); code != http.StatusNotFound { + result.Err = err + } return result } } @@ -344,7 +361,7 @@ func (c *Client) anonGetFileContent(ctx context.Context, base, owner, repo, ref, defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("HTTP %d fetching %s", resp.StatusCode, path) + return "", &api.HTTPError{StatusCode: resp.StatusCode, Message: fmt.Sprintf("fetching %s", path)} } body, err := io.ReadAll(resp.Body) diff --git a/internal/ghapi/tagsource.go b/internal/ghapi/tagsource.go index b7893db0..1e5c4d38 100644 --- a/internal/ghapi/tagsource.go +++ b/internal/ghapi/tagsource.go @@ -38,6 +38,10 @@ type RepoRelease struct { // Releases lists a repository's most recent releases (up to 30). func (c *Client) Releases(ctx context.Context, owner, repo string) ([]RepoRelease, error) { + c, err := c.ForRepo(ctx, owner, repo) + if err != nil { + return nil, err + } path := fmt.Sprintf("repos/%s/%s/releases?per_page=30", url.PathEscape(owner), url.PathEscape(repo)) @@ -89,6 +93,10 @@ func (c *Client) RepoMetadata(ctx context.Context, owner, repo string) (RepoMeta // CommitSHA resolves a ref (branch, tag, or SHA) to its commit SHA via the // repos/commits endpoint. func (c *Client) CommitSHA(ctx context.Context, owner, repo, ref string) (string, error) { + c, err := c.ForRepo(ctx, owner, repo) + if err != nil { + return "", err + } path := fmt.Sprintf("repos/%s/%s/commits/%s", url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(ref)) diff --git a/internal/lockfile/hosts_test.go b/internal/lockfile/hosts_test.go new file mode 100644 index 00000000..b6294c6a --- /dev/null +++ b/internal/lockfile/hosts_test.go @@ -0,0 +1,189 @@ +package lockfile + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + parserlock "github.com/github/actions-lockfile/go/pkg/lockfile" + "github.com/github/gh-actions-lock/internal/dep" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type hostMetadata struct{} + +func TestDotcomSaveOmitsHostname(t *testing.T) { + for _, version := range []string{"", "v0.0.2", "v0.0.3"} { + t.Run("input version="+version, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, parserlock.Path) + sha := strings.Repeat("a", 40) + if version != "" { + hostname := "" + if version == "v0.0.3" { + hostname = " hostname: github.com\n" + } + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + body := fmt.Sprintf("version: %s\nworkflows:\n .github/workflows/ci.yml:\n - o/r@v1\ndependencies:\n o/r@v1:\n%s ref: v1\n commit: sha1-%s\n owner_id: 1\n repo_id: 2\n", version, hostname, sha) + require.NoError(t, os.WriteFile(path, []byte(body), 0o600)) + } + store, err := LoadState(dir, hostMetadata{}) + require.NoError(t, err) + require.NoError(t, store.SetHostname("github.com")) + if version == "" { + require.NoError(t, store.Set(context.Background(), ".github/workflows/ci.yml", []dep.Dependency{ + {Hostname: "github.com", NWO: "o/r", Ref: "v1", SHA: sha}, + }, nil, nil)) + } + require.NoError(t, store.Save()) + raw, err := os.ReadFile(path) + require.NoError(t, err) + assert.Contains(t, string(raw), "version: 'v0.0.3'") + assert.NotContains(t, string(raw), "hostname:") + reloaded, err := LoadState(dir, nil) + require.NoError(t, err) + require.Len(t, reloaded.AllDeps(), 1) + assert.Equal(t, "github.com", reloaded.AllDeps()[0].Hostname) + assert.Equal(t, sha, reloaded.AllDeps()[0].SHA) + assert.EqualValues(t, 2, reloaded.File().Dependencies["o/r@v1"].RepoID) + require.NoError(t, reloaded.Save()) + after, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, raw, after) + }) + } +} + +func (hostMetadata) RepoIDs(_ context.Context, host, _, _ string) (int64, int64, error) { + if host == "tenant.ghe.com" { + return 10, 20, nil + } + return 1, 2, nil +} + +func TestHostScopedMetadataAndPinCollisions(t *testing.T) { + dir := t.TempDir() + store, err := LoadState(dir, hostMetadata{}) + require.NoError(t, err) + require.NoError(t, store.SetHostname("tenant.ghe.com")) + sha := strings.Repeat("a", 40) + tenant := dep.Dependency{Hostname: "tenant.ghe.com", NWO: "o/r", Ref: "tenant", SHA: sha} + public := dep.Dependency{Hostname: "github.com", NWO: "o/r", Ref: "public", SHA: sha} + require.NoError(t, store.Set(context.Background(), ".github/workflows/ci.yml", []dep.Dependency{tenant, public}, nil, nil)) + require.NoError(t, store.Save()) + reloaded, err := LoadState(dir, nil) + require.NoError(t, err) + require.NoError(t, reloaded.SetHostname("tenant.ghe.com")) + file := reloaded.File() + assert.Empty(t, file.Dependencies["o/r@tenant"].Hostname) + assert.EqualValues(t, 20, file.Dependencies["o/r@tenant"].RepoID) + assert.Equal(t, "github.com", file.Dependencies["o/r@public"].Hostname) + assert.EqualValues(t, 2, file.Dependencies["o/r@public"].RepoID) + deps, err := reloaded.Get(".github/workflows/ci.yml") + require.NoError(t, err) + assert.ElementsMatch(t, []string{"github.com", "tenant.ghe.com"}, []string{deps[0].Hostname, deps[1].Hostname}) + require.NoError(t, reloaded.Set(context.Background(), ".github/workflows/ci.yml", deps, nil, nil)) + assert.EqualValues(t, 20, reloaded.File().Dependencies["o/r@tenant"].RepoID) + assert.EqualValues(t, 2, reloaded.File().Dependencies["o/r@public"].RepoID) + + public.Ref = tenant.Ref + require.ErrorContains(t, store.Set(context.Background(), ".github/workflows/other.yml", []dep.Dependency{public}, nil, nil), "conflicting hosts") + fresh, err := LoadState(t.TempDir(), hostMetadata{}) + require.NoError(t, err) + require.ErrorContains(t, fresh.Set(context.Background(), ".github/workflows/ci.yml", []dep.Dependency{tenant, public}, nil, nil), "conflicting hosts") +} + +func TestLegacyAndOmittedHostnames(t *testing.T) { + for _, version := range []string{"v0.0.1", "v0.0.2", "v0.0.3"} { + t.Run(version, func(t *testing.T) { + key := "o/r@v1" + ref := "ref: v1" + if version == "v0.0.1" { + key += ":sha1-" + strings.Repeat("a", 40) + ref = "tag: v1" + } + path := filepath.Join(t.TempDir(), "actions.lock") + host := "github.com" + ownerID, repoID := 1, 2 + if version == "v0.0.3" { + host = "tenant.ghe.com" + ownerID, repoID = 10, 20 + } + body := fmt.Sprintf("version: %s\nworkflows:\n .github/workflows/ci.yml:\n - %s\ndependencies:\n %s:\n %s\n commit: sha1-%s\n owner_id: %d\n repo_id: %d\n", version, key, key, ref, strings.Repeat("a", 40), ownerID, repoID) + require.NoError(t, os.WriteFile(path, []byte(body), 0o600)) + store, err := LoadStateAt(path, nil) + require.NoError(t, err) + require.NoError(t, store.SetHostname("tenant.ghe.com")) + deps := store.AllDeps() + require.Len(t, deps, 1) + assert.Equal(t, host, deps[0].Hostname) + assert.Equal(t, strings.Repeat("a", 40), deps[0].SHA) + store.SetMetadataResolver(hostMetadata{}) + require.NoError(t, store.VerifyHosts(context.Background())) + require.NoError(t, store.Save()) + raw, err := os.ReadFile(path) + require.NoError(t, err) + assert.Contains(t, string(raw), "version: 'v0.0.3'") + if version == "v0.0.3" { + assert.NotContains(t, string(raw), "hostname:") + } else { + assert.Contains(t, string(raw), "hostname: 'github.com'") + } + action := store.file.Dependencies["o/r@v1"] + for _, field := range []string{"owner", "repo"} { + t.Run("rejects changed "+field+" ID", func(t *testing.T) { + changed := action + if field == "owner" { + changed.OwnerID = 200 + } else { + changed.RepoID = 200 + } + store.file.Dependencies["o/r@v1"] = changed + require.ErrorContains(t, store.VerifyHosts(context.Background()), "regenerate the lockfile") + }) + } + }) + } +} + +func TestSaveRejectsForeignTenant(t *testing.T) { + store, err := LoadState(t.TempDir(), hostMetadata{}) + require.NoError(t, err) + require.NoError(t, store.SetHostname("tenant.ghe.com")) + require.NoError(t, store.Set(context.Background(), ".github/workflows/ci.yml", []dep.Dependency{ + {Hostname: "other.ghe.com", NWO: "o/r", Ref: "v1", SHA: strings.Repeat("a", 40)}, + }, nil, nil)) + require.ErrorContains(t, store.Save(), "not the home host") + _, err = os.Stat(store.lockPath) + assert.True(t, os.IsNotExist(err)) +} + +func TestGHESKeepsHostLocalFormat(t *testing.T) { + dir := t.TempDir() + store, err := LoadState(dir, hostMetadata{}) + require.NoError(t, err) + require.NoError(t, store.SetHostname("github.example.com")) + require.NoError(t, store.Set(context.Background(), ".github/workflows/ci.yml", []dep.Dependency{ + {Hostname: "github.example.com", NWO: "o/r", Ref: "v1", SHA: strings.Repeat("a", 40)}, + }, nil, nil)) + require.NoError(t, store.Save()) + raw, err := os.ReadFile(filepath.Join(dir, parserlock.Path)) + require.NoError(t, err) + assert.Contains(t, string(raw), "version: 'v0.0.2'") + assert.NotContains(t, string(raw), "hostname:") + _, err = parserlock.Parse(raw) + require.NoError(t, err) + + reloaded, err := LoadState(dir, nil) + require.NoError(t, err) + require.NoError(t, reloaded.SetHostname("github.example.com")) + assert.Equal(t, "github.example.com", reloaded.AllDeps()[0].Hostname) + require.NoError(t, reloaded.Save()) + after, err := os.ReadFile(filepath.Join(dir, parserlock.Path)) + require.NoError(t, err) + assert.Equal(t, raw, after) +} diff --git a/internal/lockfile/state.go b/internal/lockfile/state.go index 93860cff..f145d4b5 100644 --- a/internal/lockfile/state.go +++ b/internal/lockfile/state.go @@ -13,6 +13,7 @@ import ( parserlock "github.com/github/actions-lockfile/go/pkg/lockfile" "github.com/github/gh-actions-lock/internal/dep" + "github.com/github/gh-actions-lock/internal/ghapi" "golang.org/x/sync/singleflight" ) @@ -25,7 +26,7 @@ const lockfileHeader = "# This file is machine-generated by `gh actions-lock`.\n // MetadataResolver fetches the owner/repo numeric IDs for a NWO. The store // needs these to populate the dependencies: section on every write. type MetadataResolver interface { - RepoIDs(ctx context.Context, owner, repo string) (ownerID, repoID int64, err error) + RepoIDs(ctx context.Context, hostname, owner, repo string) (ownerID, repoID int64, err error) } // State wraps the on-disk dependency lockfile for the current invocation. @@ -45,6 +46,7 @@ type State struct { meta MetadataResolver idCache map[string][2]int64 idSF singleflight.Group + hostname string } // ErrCorruptLockfile reports that a lockfile exists on disk but cannot be @@ -113,6 +115,7 @@ func LoadStateAt(lockfilePath string, meta MetadataResolver) (*State, error) { originalVersion: originalVersion, meta: meta, idCache: map[string][2]int64{}, + hostname: "github.com", } // Normalize on-disk entries to the canonical (lowercased) pin form so any // legacy mixed-case keys are rewritten on the next Save. @@ -128,7 +131,7 @@ func LoadStateAt(lockfilePath string, meta MetadataResolver) (*State, error) { if action.OwnerID != 0 && action.RepoID != 0 { // idCache is keyed by lowercase owner/repo to match lookupIDs and the // canonical pin reader; mixed-case keys here would silently miss. - s.idCache[strings.ToLower(pin.Owner+"/"+pin.Repo)] = [2]int64{action.OwnerID, action.RepoID} + s.idCache[repoIDKey(action.Hostname, pin.Owner, pin.Repo)] = [2]int64{action.OwnerID, action.RepoID} } } s.file.Dependencies = normalizedDependencies @@ -153,6 +156,36 @@ func LoadStateAt(lockfilePath string, meta MetadataResolver) (*State, error) { return s, nil } +// SetHostname binds omitted v0.0.3 hostnames to the invocation's home host. +// Legacy pins retain the SDK's explicit github.com binding. +func (s *State) SetHostname(hostname string) error { + s.mu.Lock() + defer s.mu.Unlock() + s.hostname = hostname + if hostname == "github.com" || ghapi.IsProxima(hostname) { + s.idCache = map[string][2]int64{} + for key, action := range s.file.Dependencies { + if pin, ok := parserlock.ParsePin(key); ok && action.OwnerID != 0 && action.RepoID != 0 { + s.idCache[repoIDKey(s.hostOrHome(action.Hostname), pin.Owner, pin.Repo)] = [2]int64{action.OwnerID, action.RepoID} + } + } + return nil + } + if s.originalVersion == "v0.0.3" { + return fmt.Errorf("v0.0.3 lockfiles do not support GitHub Enterprise Server host %s", hostname) + } + s.file.Version = "v0.0.2" + s.idCache = map[string][2]int64{} + for key, action := range s.file.Dependencies { + action.Hostname = hostname + s.file.Dependencies[key] = action + if pin, ok := parserlock.ParsePin(key); ok { + s.idCache[repoIDKey(hostname, pin.Owner, pin.Repo)] = [2]int64{action.OwnerID, action.RepoID} + } + } + return nil +} + // File returns the in-memory parser-level lockfile snapshot. Intended for // consumers that drive the workflow-parser diagnostics engine directly and // need the whole file (workflow keys + actions metadata) in one shot. @@ -177,6 +210,34 @@ func (s *State) SetMetadataResolver(meta MetadataResolver) { s.meta = meta } +// VerifyHosts checks recorded identities before trusting pins on Proxima. +// Older producers used omission for dotcom, not the home tenant. +func (s *State) VerifyHosts(ctx context.Context) error { + s.mu.Lock() + defer s.mu.Unlock() + if !ghapi.IsProxima(s.hostname) || s.originalVersion == "" { + return nil + } + for key, action := range s.file.Dependencies { + if s.meta == nil { + return fmt.Errorf("metadata resolver not configured") + } + pin, ok := parserlock.ParsePin(key) + if !ok { + return fmt.Errorf("invalid dependency %q", key) + } + hostname := s.hostOrHome(action.Hostname) + ownerID, repoID, err := s.meta.RepoIDs(ctx, hostname, pin.Owner, pin.Repo) + if err != nil { + return fmt.Errorf("verifying repository identity for %s on %s: %w", key, hostname, err) + } + if ownerID != action.OwnerID || repoID != action.RepoID { + return fmt.Errorf("dependency %s does not match its %s repository IDs; restore a lockfile for this host or regenerate the lockfile after reviewing the dependency", key, hostname) + } + } + return nil +} + // HasWorkflow reports whether the lockfile's workflows{} map already // contains an entry for workflowKey. Used by `upgrade --no-onboard` to // refuse silently onboarding a previously-untracked workflow during a @@ -240,6 +301,7 @@ func (s *State) Get(workflowKey string) ([]dep.Dependency, error) { } d := pinToDep(pin) if action, found := s.file.Dependencies[raw]; found { + d.Hostname = s.hostOrHome(action.Hostname) d.Tag, d.Branch = parserlock.SplitRef(action.Ref) if idx := strings.Index(action.Commit, "-"); idx >= 0 { d.HashAlgo = action.Commit[:idx] @@ -265,6 +327,7 @@ func (s *State) AllDeps() []dep.Dependency { continue } d := pinToDep(pin) + d.Hostname = s.hostOrHome(action.Hostname) d.Tag, d.Branch = parserlock.SplitRef(action.Ref) if idx := strings.Index(action.Commit, "-"); idx >= 0 { d.HashAlgo = action.Commit[:idx] @@ -304,17 +367,27 @@ func (s *State) Set(ctx context.Context, workflowKey string, deps []dep.Dependen // lookupIDs is safe to call without s.mu and dedups in-flight fetches // for the same key via singleflight. seenRepos := make(map[string]struct{}, len(deps)) + pinHosts := make(map[string]string, len(deps)) for _, d := range deps { pin, err := depToPin(d) if err != nil { return err } - k := pin.Owner + "/" + pin.Repo + hostname := d.Hostname + if hostname == "" { + hostname = s.hostname + } + pinKey := pin.Canonical().String() + if host, ok := pinHosts[pinKey]; ok && host != hostname { + return fmt.Errorf("dependency %s has conflicting hosts %s and %s", pinKey, host, hostname) + } + pinHosts[pinKey] = hostname + k := repoIDKey(hostname, pin.Owner, pin.Repo) if _, ok := seenRepos[k]; ok { continue } seenRepos[k] = struct{}{} - if _, err := s.lookupIDs(ctx, pin.Owner, pin.Repo); err != nil { + if _, err := s.lookupIDs(ctx, hostname, pin.Owner, pin.Repo); err != nil { return fmt.Errorf("resolving repo IDs for %s/%s: %w", pin.Owner, pin.Repo, err) } } @@ -332,6 +405,13 @@ func (s *State) Set(ctx context.Context, workflowKey string, deps []dep.Dependen } pin = pin.Canonical() pinKey := pin.String() + hostname := d.Hostname + if hostname == "" { + hostname = s.hostname + } + if existing, ok := s.file.Dependencies[pinKey]; ok && s.hostOrHome(existing.Hostname) != hostname { + return fmt.Errorf("dependency %s has conflicting hosts %s and %s", pinKey, s.hostOrHome(existing.Hostname), hostname) + } keyToPin[d.Key()] = pinKey var isDirect bool if directKeys != nil { @@ -380,7 +460,11 @@ func (s *State) Set(ctx context.Context, workflowKey string, deps []dep.Dependen pinKey := pin.String() // IDs were pre-resolved above (outside the mutex); read from cache // directly so we don't recursively re-acquire s.mu. - ids, ok := s.idCache[strings.ToLower(pin.Owner+"/"+pin.Repo)] + hostname := d.Hostname + if hostname == "" { + hostname = s.hostname + } + ids, ok := s.idCache[repoIDKey(hostname, pin.Owner, pin.Repo)] if !ok { return fmt.Errorf("resolving repo IDs for %s/%s: not in cache after pre-resolve", pin.Owner, pin.Repo) } @@ -437,11 +521,12 @@ func (s *State) Set(ctx context.Context, workflowKey string, deps []dep.Dependen sort.Strings(uses) } s.file.Dependencies[pinKey] = parserlock.Action{ - Ref: ref, - Commit: d.HashAlgoOrDetect() + "-" + d.SHA, - OwnerID: ids[0], - RepoID: ids[1], - Uses: uses, + Hostname: hostname, + Ref: ref, + Commit: d.HashAlgoOrDetect() + "-" + d.SHA, + OwnerID: ids[0], + RepoID: ids[1], + Uses: uses, } } sort.Strings(directPins) @@ -491,7 +576,7 @@ func (s *State) Save() error { return nil } - out, err := marshalDeterministic(s.file) + out, err := marshalDeterministic(s.file, s.hostname) if err != nil { return err } @@ -510,11 +595,11 @@ func (s *State) Save() error { // Safe to call without s.mu held: cache hits are read under a brief lock, // and concurrent misses for the same key are coalesced via singleflight // so we issue at most one network request per repo. -func (s *State) lookupIDs(ctx context.Context, owner, repo string) ([2]int64, error) { +func (s *State) lookupIDs(ctx context.Context, hostname, owner, repo string) ([2]int64, error) { // Cache key is always lowercase: callers mix canonical (lowercased) and // raw NWOs, and we MUST agree across all idCache reads/writes or pins // silently miss the cache after pre-resolve. - key := strings.ToLower(owner + "/" + repo) + key := repoIDKey(hostname, owner, repo) s.mu.Lock() if ids, ok := s.idCache[key]; ok { s.mu.Unlock() @@ -534,7 +619,7 @@ func (s *State) lookupIDs(ctx context.Context, owner, repo string) ([2]int64, er } s.mu.Unlock() - ownerID, repoID, err := s.meta.RepoIDs(ctx, owner, repo) + ownerID, repoID, err := s.meta.RepoIDs(ctx, hostname, owner, repo) if err != nil { return [2]int64{}, err } @@ -553,6 +638,24 @@ func (s *State) lookupIDs(ctx context.Context, owner, repo string) ([2]int64, er return res.([2]int64), nil } +func hostOrDotcom(hostname string) string { + if hostname == "" { + return "github.com" + } + return hostname +} + +func (s *State) hostOrHome(hostname string) string { + if hostname == "" { + return s.hostname + } + return hostname +} + +func repoIDKey(hostname, owner, repo string) string { + return hostOrDotcom(hostname) + "/" + strings.ToLower(owner+"/"+repo) +} + // extractVersion reads the version field from raw lockfile YAML without // a full parse. Returns empty string if not found. func extractVersion(contents []byte) string { diff --git a/internal/lockfile/state_marshal.go b/internal/lockfile/state_marshal.go index 30d152c9..39452638 100644 --- a/internal/lockfile/state_marshal.go +++ b/internal/lockfile/state_marshal.go @@ -18,7 +18,7 @@ import ( // can collide with YAML 1.1 booleans ("y", "no", "on", "off"). Schema // field names (version, dependencies, workflows, ref, …) stay // unquoted because they're hardcoded and trivially safe. -func marshalDeterministic(file parserlock.File) ([]byte, error) { +func marshalDeterministic(file parserlock.File, homeHost string) ([]byte, error) { root := &yaml.Node{Kind: yaml.MappingNode} addQuotedField(root, "version", file.Version) @@ -57,6 +57,12 @@ func marshalDeterministic(file parserlock.File) ([]byte, error) { for _, k := range keys { a := file.Dependencies[k] entry := &yaml.Node{Kind: yaml.MappingNode} + if file.Version == "v0.0.3" && a.Hostname != "" && a.Hostname != homeHost { + if a.Hostname != "github.com" { + return nil, fmt.Errorf("dependency %s is pinned to %s, not the home host %s or github.com", k, a.Hostname, homeHost) + } + addQuotedField(entry, "hostname", a.Hostname) + } if a.Ref != "" { addQuotedField(entry, "ref", a.Ref) } diff --git a/internal/lockfile/state_test.go b/internal/lockfile/state_test.go index af7b70f4..e2d08780 100644 --- a/internal/lockfile/state_test.go +++ b/internal/lockfile/state_test.go @@ -14,7 +14,7 @@ import ( type fakeMetadataResolver struct{} -func (fakeMetadataResolver) RepoIDs(_ context.Context, owner, repo string) (int64, int64, error) { +func (fakeMetadataResolver) RepoIDs(_ context.Context, _, owner, repo string) (int64, int64, error) { return 1, 2, nil } diff --git a/internal/pin/commit.go b/internal/pin/commit.go index bd3e900e..30908ee9 100644 --- a/internal/pin/commit.go +++ b/internal/pin/commit.go @@ -154,6 +154,7 @@ func groupPinnedByWorkflow(rec *Record) map[string][]dep.Dependency { } for _, wf := range e.Workflows { result[wf] = append(result[wf], dep.Dependency{ + Hostname: e.Hostname, NWO: e.NWO, Ref: e.Ref, SHA: e.SHA, diff --git a/internal/pin/plan.go b/internal/pin/plan.go index f9c24603..cf2f8882 100644 --- a/internal/pin/plan.go +++ b/internal/pin/plan.go @@ -105,7 +105,7 @@ func Plan(ctx context.Context, report *checks.Report, opts PlanOptions) (*Record planErr = poolErr } - targetSHAs := make(map[string]string) + targets := make(map[string]string) for _, pr := range results { for _, entry := range pr.entries { if planErr != nil || entry.SHA == "" || @@ -113,11 +113,12 @@ func Plan(ctx context.Context, report *checks.Report, opts PlanOptions) (*Record continue } key := strings.ToLower(entry.NWO) + "@" + entry.Ref - if sha, ok := targetSHAs[key]; ok && !strings.EqualFold(sha, entry.SHA) { - planErr = fmt.Errorf("conflicting planned target %s resolves to both %s and %s", key, sha, entry.SHA) + target := entry.Hostname + "/" + entry.SHA + if previous, ok := targets[key]; ok && !strings.EqualFold(previous, target) { + planErr = fmt.Errorf("conflicting planned target %s resolves to both %s and %s", key, previous, target) continue } - targetSHAs[key] = entry.SHA + targets[key] = target } rec.Entries = append(rec.Entries, pr.entries...) rec.Workflows = append(rec.Workflows, pr.wplans...) @@ -202,12 +203,14 @@ func planWorkflow(ctx context.Context, wr checks.WorkflowReport, opts PlanOption status("resolving " + wr.Path) deps, parentMap, resolveErr := opts.Resolver.ResolveAllRecursive(ctx, unrecordedRefs) if resolveErr != nil { - entries = append(entries, unresolvedEntries(wr, unrecordedRefs, deps, resolveErr)...) - if len(deps) == 0 { - wplans = append(wplans, WorkflowPlan{Path: wr.Path, SelfActionFiles: wr.SelfActionFiles}) - return planResult{entries: entries, wplans: wplans}, nil + // A resolved root is not pinnable when its transitive graph is incomplete. + for _, ref := range unrecordedRefs { + entries = append(entries, Entry{ + NWO: ref.NWO(), Ref: ref.Ref, Resolution: Unresolved, + Reason: "resolution failed: " + resolveErr.Error(), Workflows: []string{wr.Path}, + }) } - // Fall through with partial deps to pin what we can. + return planResult{entries: entries}, nil } // Root refs are recorded directly under the workflow in the lockfile. @@ -328,39 +331,6 @@ func rejectPartialSelfActionRewrites(opts PlanOptions, selfActionRefs []parserlo return nil } -// unresolvedEntries flags findings whose refs were attempted but failed to -// resolve. On a partial failure deps holds the refs that did resolve, so only -// the genuine misses (attempted and not in deps) are marked Unresolved. -func unresolvedEntries(wr checks.WorkflowReport, unrecordedRefs []parserlock.ActionRef, deps []dep.Dependency, resolveErr error) []Entry { - resolved := make(map[string]bool, len(deps)) - for _, d := range deps { - resolved[strings.ToLower(d.NWO+"@"+d.Ref)] = true - } - attempted := make(map[string]bool, len(unrecordedRefs)) - for _, ref := range unrecordedRefs { - attempted[strings.ToLower(ref.Owner+"/"+ref.Repo+"@"+ref.Ref)] = true - } - var out []Entry - for _, f := range wr.Findings { - if f.ActionRef == nil { - continue - } - key := strings.ToLower(f.ActionRef.Owner + "/" + f.ActionRef.Repo + "@" + f.ActionRef.Ref) - if !attempted[key] || resolved[key] { - continue - } - out = append(out, Entry{ - NWO: f.ActionRef.Owner + "/" + f.ActionRef.Repo, - Ref: f.ActionRef.Ref, - Resolution: Unresolved, - Issue: string(f.Category), - Reason: fmt.Sprintf("resolution failed: %s", resolveErr), - Workflows: []string{wr.Path}, - }) - } - return out -} - // narrowDirectDeps rewrites direct partial semver refs to exact patch tags, // leaving bare SHA and transitive refs for reverse lookup. func narrowDirectDeps(ctx context.Context, opts PlanOptions, deps []dep.Dependency, directTracker lockfile.DirectTracker, rewrites map[string]string, preservedDeps map[int]bool) { @@ -508,6 +478,7 @@ func buildPinnedEntries(opts PlanOptions, wr checks.WorkflowReport, deps []dep.D res = Verified } entry := Entry{ + Hostname: dep.Hostname, NWO: dep.NWO, Ref: dep.Ref, SHA: dep.SHA, @@ -546,11 +517,16 @@ func informationalEntries(wr checks.WorkflowReport, opts PlanOptions) []Entry { func informationalEntry(f checks.Finding, path string) Entry { nwo := "" ref := "" + hostname := "" if f.ActionRef != nil { nwo = f.ActionRef.Owner + "/" + f.ActionRef.Repo ref = f.ActionRef.Ref } + if f.Dependency != nil { + hostname = f.Dependency.Hostname + } return Entry{ + Hostname: hostname, NWO: nwo, Ref: ref, ObservedSHA: f.ObservedSHA, @@ -628,6 +604,7 @@ func verifiedEntries(inventory []checks.InventoryEntry, path string) []Entry { NWO: inv.Dep.NWO, Ref: inv.Dep.Ref, SHA: inv.Dep.SHA, + Hostname: inv.Dep.Hostname, Resolution: Verified, OnBranch: inv.Dep.Branch, Tag: inv.Dep.Tag, diff --git a/internal/pin/plan_test.go b/internal/pin/plan_test.go index 3e6d6e6b..131fc52f 100644 --- a/internal/pin/plan_test.go +++ b/internal/pin/plan_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/github/gh-actions-lock/internal/dep" - "github.com/github/gh-actions-lock/internal/lockfile" "github.com/github/gh-actions-lock/internal/pipeline/checks" parserlock "github.com/github/actions-lockfile/go/pkg/lockfile" @@ -113,10 +112,7 @@ func TestNarrowDirectDeps_SameNWOSiblingRefsNormalizeIndependently(t *testing.T) assert.Equal(t, "actions/checkout@v21", reverseRewrites["actions/checkout@"+bareSHA]) } -// TestPlanWorkflow_PartialResolutionFailure verifies that when one ref in a -// workflow fails resolution (e.g. repo not found), only the failed ref is -// marked Unresolved. The successful ref proceeds through reachability and -// pinning. This is the cascade-failure regression test. +// An incomplete graph must not become an apparently complete lockfile. func TestPlanWorkflow_PartialResolutionFailure(t *testing.T) { reg := &httpmock.Registry{} defer reg.Verify(t) @@ -174,33 +170,14 @@ func TestPlanWorkflow_PartialResolutionFailure(t *testing.T) { result, err := planWorkflow(context.Background(), wr, opts, func(string) {}) require.NoError(t, err) - - // Classify entries. - var unresolved, pinned []Entry - for _, e := range result.entries { - switch e.Resolution { - case Unresolved: - unresolved = append(unresolved, e) - case Pinned: - pinned = append(pinned, e) - } + require.Len(t, result.entries, 2) + for _, entry := range result.entries { + assert.Equal(t, Unresolved, entry.Resolution) + assert.Contains(t, entry.Reason, "bad/private@main") } - - // bad/private must be unresolved. - require.Len(t, unresolved, 1, "expected exactly one unresolved entry") - assert.Equal(t, "bad/private", unresolved[0].NWO) - assert.Equal(t, "main", unresolved[0].Ref) - assert.Contains(t, unresolved[0].Reason, "not found") - - // good/action must be pinned (not poisoned by the bad ref). - require.Len(t, pinned, 1, "expected exactly one pinned entry") - assert.Equal(t, "good/action", pinned[0].NWO) - assert.Equal(t, goodSHA, pinned[0].SHA) + assert.Empty(t, result.wplans) } -// TestPlanWorkflow_AllResolutionsFail verifies that when ALL refs in a -// workflow fail resolution, every finding is marked Unresolved and no -// reachability is attempted. func TestPlanWorkflow_AllResolutionsFail(t *testing.T) { reg := &httpmock.Registry{} defer reg.Verify(t) @@ -243,12 +220,12 @@ func TestPlanWorkflow_AllResolutionsFail(t *testing.T) { Pool: pool, }, func(string) {}) require.NoError(t, err) - require.Len(t, result.entries, 2) - for _, e := range result.entries { - assert.Equal(t, Unresolved, e.Resolution, "expected %s to be Unresolved", e.NWO) - assert.Contains(t, e.Reason, "not found") + for _, entry := range result.entries { + assert.Equal(t, Unresolved, entry.Resolution) + assert.Contains(t, entry.Reason, "not found") } + assert.Empty(t, result.wplans) } func newTransitivePlanFixture(t *testing.T, compSHA, transSHA string) (*resolve.Resolver, *pinpool.Pool, *tag.Lister) { @@ -798,7 +775,7 @@ func TestNoNarrow_BareSHA(t *testing.T) { }) t.Run("partial scan rejects unrecorded shared action rewrite", func(t *testing.T) { - resolver, tagger, wr, _ := newSlowPathFixtures(t, false) + resolver, tagger, wr, _ := newSlowPathFixtures(t) wr.SelfActionRefs = append([]parserlock.ActionRef(nil), wr.ActionRefs...) _, err := planWorkflow(context.Background(), wr, PlanOptions{ diff --git a/internal/pin/record.go b/internal/pin/record.go index ad32246a..0811dcd9 100644 --- a/internal/pin/record.go +++ b/internal/pin/record.go @@ -28,6 +28,7 @@ type RepoInfo struct { // Entry records the plan decision for one action dependency. type Entry struct { + Hostname string `json:"hostname,omitempty"` NWO string `json:"nwo"` Ref string `json:"ref"` SHA string `json:"sha,omitempty"` @@ -165,7 +166,7 @@ func dedupActions(entries []Entry) []Entry { seen := map[string]*slot{} var out []Entry for _, e := range entries { - key := e.NWO + "@" + e.Ref + key := e.Hostname + "/" + e.NWO + "@" + e.Ref if s, ok := seen[key]; ok { out[s.idx].Workflows = appendUnique(out[s.idx].Workflows, e.Workflows...) continue diff --git a/internal/pin/retain_impostor_test.go b/internal/pin/retain_impostor_test.go index 97f22458..80aa0997 100644 --- a/internal/pin/retain_impostor_test.go +++ b/internal/pin/retain_impostor_test.go @@ -15,7 +15,7 @@ import ( type fakeMeta struct{} -func (fakeMeta) RepoIDs(_ context.Context, _, _ string) (int64, int64, error) { +func (fakeMeta) RepoIDs(_ context.Context, _, _, _ string) (int64, int64, error) { return 1, 2, nil } diff --git a/internal/pipeline/checks/misleading.go b/internal/pipeline/checks/misleading.go index 19db956e..83a2ffaf 100644 --- a/internal/pipeline/checks/misleading.go +++ b/internal/pipeline/checks/misleading.go @@ -132,7 +132,7 @@ func checkOneRefMoved(ctx context.Context, pw ParsedWorkflow, ref parserlock.Act f.Severity = SeverityWarning f.Confidence = ConfidenceHigh f.Detail = fmt.Sprintf("ref %s now resolves to %s, lockfile pins %s", ref.Ref, parserlock.ShortSHA(sha), parserlock.ShortSHA(pin.SHA())) - f.Remediation = "re-run `gh actions-lock` to refresh the lock entry" + f.Remediation = "run `gh actions-lock --relock` to refresh the lock entry" } return f, true } diff --git a/internal/pipeline/diagnose.go b/internal/pipeline/diagnose.go index ac715eba..da402006 100644 --- a/internal/pipeline/diagnose.go +++ b/internal/pipeline/diagnose.go @@ -137,11 +137,22 @@ func diagnoseOneParsed(ctx context.Context, pw checks.ParsedWorkflow, r *resolve rawFindings := checks.RunChecks(ctx, pw, store.File(), checkR) depByKey := indexDeps(pw.ExistingDeps) + liveByKey := indexDeps(liveDeps) for _, f := range rawFindings { if f.Category == checks.Stale && isTransitivePin(f, depByKey, parentMap) { continue } attachParent(&f, depByKey, directNWOs, parentMap) + if f.Dependency != nil { + d, ok := depByKey[f.Dependency.Key()] + if !ok { + d = liveByKey[f.Dependency.Key()] + } + f.Dependency.Hostname = d.Hostname + if f.Category == checks.ShaAsRef && d.Hostname != "" { + f.Remediation = fmt.Sprintf("pin to a tag instead: https://%s/%s/releases", d.Hostname, d.NWO) + } + } f.DocURL = DocURLFor(f.Category) wr.Findings = append(wr.Findings, f) } diff --git a/internal/pipeline/diagnose_test.go b/internal/pipeline/diagnose_test.go index df2a1999..4a7d8f60 100644 --- a/internal/pipeline/diagnose_test.go +++ b/internal/pipeline/diagnose_test.go @@ -12,7 +12,7 @@ import ( type noopMeta struct{} -func (noopMeta) RepoIDs(context.Context, string, string) (int64, int64, error) { +func (noopMeta) RepoIDs(context.Context, string, string, string) (int64, int64, error) { return 0, 0, nil } diff --git a/internal/resolve/discovery.go b/internal/resolve/discovery.go index 0665d7c9..30b50bb8 100644 --- a/internal/resolve/discovery.go +++ b/internal/resolve/discovery.go @@ -384,10 +384,11 @@ func (r *Resolver) resolveWithActionYMLParallel(ctx context.Context, refs []reso ref := refs[idx].ref if j < len(res) && res[j].Err == nil { d := dep.Dependency{ - NWO: res[j].Owner + "/" + res[j].Repo, - Path: res[j].Path, - Ref: ref.Ref, - SHA: res[j].CommitOID, + Hostname: res[j].Hostname, + NWO: res[j].Owner + "/" + res[j].Repo, + Path: res[j].Path, + Ref: ref.Ref, + SHA: res[j].CommitOID, } r.cache.Put(cacheKey(ref), resolvedEntry{dep: d, actionYML: res[j].ActionYML}) results[idx] = resolveResult{dep: d, yml: res[j].ActionYML, ok: true} diff --git a/internal/resolve/resolver.go b/internal/resolve/resolver.go index 24776e06..929cd36e 100644 --- a/internal/resolve/resolver.go +++ b/internal/resolve/resolver.go @@ -4,6 +4,7 @@ package resolve import ( "context" + "fmt" "net/http" "sync" "time" @@ -107,11 +108,23 @@ func New(hostname string, pool *pinpool.Pool, opts ...Option) (*Resolver, error) return nil, err } r.gh = c + r.hostname = c.Hostname return r, nil } // --- Seeding (post-construction, deps come from lockfile loaded after resolver) --- +// SeedHosts binds recorded repositories before caches or network work begin. +func (r *Resolver) SeedHosts(deps []dep.Dependency) error { + for _, d := range deps { + owner, repo := d.OwnerRepo() + if err := r.gh.PinHost(owner, repo, d.Hostname); err != nil { + return err + } + } + return nil +} + // SeedBranchHints records a branch-of-record for each dep so subsequent // containing-branch scans try that branch first. Hints from a previous // lockfile are advisory: a miss falls through to a full branch scan. @@ -156,8 +169,15 @@ func (r *Resolver) Hostname() string { return r.hostname } func (r *Resolver) GHClient() *ghapi.Client { return r.gh } // RepoIDs returns the numeric owner ID and repo ID for a NWO. -func (r *Resolver) RepoIDs(ctx context.Context, owner, repo string) (int64, int64, error) { - return r.gh.RepoIDs(ctx, owner, repo) +func (r *Resolver) RepoIDs(ctx context.Context, hostname, owner, repo string) (int64, int64, error) { + client, err := r.gh.ForRepo(ctx, owner, repo) + if err != nil { + return 0, 0, err + } + if hostname != client.Hostname { + return 0, 0, fmt.Errorf("%s/%s resolved on %s but metadata requested from %s", owner, repo, client.Hostname, hostname) + } + return client.RepoIDs(ctx, owner, repo) } // branchHint returns the branch previously recorded as containing sha in diff --git a/test/scenarios/catalog.yml b/test/scenarios/catalog.yml index 396407df..f0ea9516 100644 --- a/test/scenarios/catalog.yml +++ b/test/scenarios/catalog.yml @@ -884,10 +884,11 @@ scenarios: needs_token: true tags: [real_repo] live_repo: nodeselector/actions-test-fixtures + flags: [".github/workflows/happy-path.yml", ".github/workflows/deep-self-ref.yml", ".github/workflows/reusable-build.yml"] fixtures: delete_lockfile: true expect: - exit: 1 + exit: 0 lockfile_deps_cover_direct: true lockfile_deps_cover_indirect: true lockfile_contains: @@ -902,10 +903,11 @@ scenarios: needs_token: true tags: [real_repo] live_repo: nodeselector/actions-test-fixtures + flags: [".github/workflows/deep-self-ref.yml"] fixtures: delete_lockfile: true expect: - exit: 1 + exit: 0 lockfile_deps_cover_direct: true lockfile_deps_cover_indirect: true lockfile_contains: @@ -1726,12 +1728,13 @@ scenarios: golden_json: cli_version: (devel) findings: [] - lockfile_version: v0.0.2 + lockfile_version: v0.0.3 valid: true workflows: - dependencies: - direct: true hash_algo: sha1 + hostname: github.com nwo: actions/checkout ref: v4 sha: de0fac2e4500dabe0009e67214ff5f5447ce83dd @@ -1774,7 +1777,7 @@ scenarios: remediation: onboard it first with `gh actions-lock` (without --no-onboard) severity: info workflow: .github/workflows/ci.yml - lockfile_version: v0.0.2 + lockfile_version: v0.0.3 valid: true workflows: - findings: @@ -1821,12 +1824,13 @@ scenarios: remediation: onboard it first with `gh actions-lock` (without --no-onboard) severity: info workflow: .github/workflows/ci.yml - lockfile_version: v0.0.2 + lockfile_version: v0.0.3 valid: true workflows: - dependencies: - direct: true hash_algo: sha1 + hostname: github.com nwo: actions/checkout ref: v4 sha: de0fac2e4500dabe0009e67214ff5f5447ce83dd @@ -1899,12 +1903,13 @@ scenarios: remediation: onboard it first with `gh actions-lock` (without --no-onboard) severity: info workflow: .github/workflows/deploy.yml - lockfile_version: v0.0.2 + lockfile_version: v0.0.3 valid: true workflows: - dependencies: - direct: true hash_algo: sha1 + hostname: github.com nwo: actions/checkout ref: v4 sha: de0fac2e4500dabe0009e67214ff5f5447ce83dd @@ -1952,7 +1957,7 @@ scenarios: golden_json: cli_version: (devel) findings: [] - lockfile_version: v0.0.2 + lockfile_version: v0.0.3 valid: true - name: dbot_transient_403_drops_pin category: dependabot @@ -2092,25 +2097,27 @@ scenarios: - "commit: 'sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'" lockfile_comment_excludes: "de0fac2e4500dabe0009e67214ff5f5447ce83dd" - - name: onboard_roundtrip_v002 + - name: onboard_roundtrip_v003 category: onboarding - description: "Fresh onboard writes a v0.0.2 lockfile with correct key format, ref field, and commit field" + description: "Real-binary dotcom onboarding emits v0.0.3 without hostnames and a complete live transitive dependency graph" needs_token: true tags: [real_repo] live_repo: nodeselector/actions-test-fixtures + flags: [".github/workflows/happy-path.yml"] fixtures: delete_lockfile: true expect: - exit: 1 + exit: 0 lockfile_exists: true lockfile_deps_cover_direct: true lockfile_deps_cover_indirect: true lockfile_contains: - - "version: 'v0.0.2'" + - "version: 'v0.0.3'" - "ref:" - "commit: 'sha1-" - "owner_id:" - "repo_id:" + lockfile_excludes: ["hostname:"] lockfile_comment_excludes: '^\s+tag:' lockfile_comment_matches: "ref: '"