From 004c3c2adff0687c56f5801477039e1a66d5b159 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 1 Oct 2026 09:58:58 -0700 Subject: [PATCH 01/10] Resolve data-residency dependencies on their recorded hosts Route new tenant dependencies to public dotcom only on repository absence. Preserve hostname and host-local IDs, isolate public requests from tenant credentials, and refuse incomplete generation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 30 ++++ cmd/gh-actions-lock/format/json.go | 3 + cmd/gh-actions-lock/format/terminal.go | 4 +- cmd/gh-actions-lock/format/url.go | 9 +- cmd/gh-actions-lock/format/url_test.go | 9 +- cmd/gh-actions-lock/pin_summary.go | 2 +- cmd/gh-actions-lock/proxima_test.go | 183 +++++++++++++++++++++++++ cmd/gh-actions-lock/root.go | 6 + cmd/gh-actions-lock/run.go | 21 +++ go.mod | 2 +- go.sum | 4 +- internal/dep/dependency.go | 3 +- internal/ghapi/client.go | 40 ++++++ internal/ghapi/graphql_action_files.go | 34 ++++- internal/ghapi/graphql_peel.go | 4 + internal/ghapi/graphql_reachability.go | 4 + internal/ghapi/hosts.go | 82 +++++++++++ internal/ghapi/hosts_test.go | 171 +++++++++++++++++++++++ internal/ghapi/repos.go | 32 +++++ internal/ghapi/rest_fallback.go | 29 +++- internal/ghapi/tagsource.go | 8 ++ internal/lockfile/hosts_test.go | 111 +++++++++++++++ internal/lockfile/state.go | 116 ++++++++++++++-- internal/lockfile/state_marshal.go | 3 + internal/lockfile/state_test.go | 11 +- internal/pin/commit.go | 1 + internal/pin/plan.go | 61 +++------ internal/pin/plan_test.go | 45 ++---- internal/pin/record.go | 3 +- internal/pin/retain_impostor_test.go | 2 +- internal/pipeline/diagnose.go | 11 ++ internal/pipeline/diagnose_test.go | 2 +- internal/resolve/discovery.go | 9 +- internal/resolve/resolver.go | 24 +++- 34 files changed, 962 insertions(+), 117 deletions(-) create mode 100644 cmd/gh-actions-lock/proxima_test.go create mode 100644 internal/ghapi/hosts.go create mode 100644 internal/ghapi/hosts_test.go create mode 100644 internal/lockfile/hosts_test.go diff --git a/README.md b/README.md index 56fcec95..ef6f7283 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,36 @@ 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 + +Select your tenant with `--hostname` or `GH_HOST`: + +```bash +gh actions-lock --hostname octocorp.ghe.com --no-interactive +``` + +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. + +The v0.0.3 lockfile records each dependency's hostname alongside its commit and +host-specific repository IDs. Existing pins retain their recorded host, including +during `--rescan` and `--relock`. Legacy v0.0.1/v0.0.2 pins and v0.0.3 pins without +a hostname mean `github.com`. Legacy repository IDs are checked against dotcom +before migration on a tenant; tenant pins generated by older CLI versions must +be regenerated rather than relabeled. 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. GitHub Enterprise Server remains host-local and writes v0.0.2, since +the v0.0.3 hostname field does not support GitHub Enterprise Server hosts. + +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/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..81f4cd7f --- /dev/null +++ b/cmd/gh-actions-lock/proxima_test.go @@ -0,0 +1,183 @@ +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.Equal(t, "tenant.ghe.com", 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)) +} + +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..db653ea6 100644 --- a/cmd/gh-actions-lock/root.go +++ b/cmd/gh-actions-lock/root.go @@ -208,6 +208,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..660ae93f 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.VerifyLegacyHosts(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/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..9f528474 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,17 @@ 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 { + 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_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..189a14b9 --- /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 = "github.com" + } + 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..3f77aa93 --- /dev/null +++ b/internal/ghapi/hosts_test.go @@ -0,0 +1,171 @@ +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.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..05995c30 --- /dev/null +++ b/internal/lockfile/hosts_test.go @@ -0,0 +1,111 @@ +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 (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) + 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) + file := reloaded.File() + assert.Equal(t, "tenant.ghe.com", 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}) + + 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") + 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: 1\n repo_id: 2\n", version, key, key, ref, strings.Repeat("a", 40)) + 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, "github.com", deps[0].Hostname) + store.SetMetadataResolver(hostMetadata{}) + require.NoError(t, store.VerifyLegacyHosts(context.Background())) + if version != "v0.0.3" { + action := store.file.Dependencies["o/r@v1"] + action.RepoID = 200 + store.file.Dependencies["o/r@v1"] = action + require.ErrorContains(t, store.VerifyLegacyHosts(context.Background()), "regenerate the lockfile") + } + require.NoError(t, store.Save()) + raw, err := os.ReadFile(path) + require.NoError(t, err) + assert.Contains(t, string(raw), "hostname: 'github.com'") + }) + } +} + +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..bf629511 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,11 +115,13 @@ 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. normalizedDependencies := make(map[string]parserlock.Action, len(file.Dependencies)) for pinKey, action := range file.Dependencies { + action.Hostname = hostOrDotcom(action.Hostname) pin, ok := parserlock.ParsePin(pinKey) if !ok { normalizedDependencies[pinKey] = action @@ -128,7 +132,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 +157,30 @@ func LoadStateAt(lockfilePath string, meta MetadataResolver) (*State, error) { return s, nil } +// SetHostname preserves the legacy host-local format on GHES, whose hostnames +// are not supported by the v0.0.3 schema. +func (s *State) SetHostname(hostname string) error { + s.mu.Lock() + defer s.mu.Unlock() + s.hostname = hostname + if hostname == "github.com" || ghapi.IsProxima(hostname) { + 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 +205,33 @@ func (s *State) SetMetadataResolver(meta MetadataResolver) { s.meta = meta } +// VerifyLegacyHosts prevents old tenant-generated pins from being relabeled as +// dotcom pins during migration. Legacy schemas carry no host provenance. +func (s *State) VerifyLegacyHosts(ctx context.Context) error { + s.mu.Lock() + defer s.mu.Unlock() + if !ghapi.IsProxima(s.hostname) || s.originalVersion == "" || s.originalVersion == "v0.0.3" { + 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) + } + ownerID, repoID, err := s.meta.RepoIDs(ctx, "github.com", pin.Owner, pin.Repo) + if err != nil { + return fmt.Errorf("verifying legacy dotcom identity for %s: %w", key, err) + } + if ownerID != action.OwnerID || repoID != action.RepoID { + return fmt.Errorf("legacy dependency %s does not match its github.com repository IDs; regenerate the lockfile with the selected hostname", key) + } + } + 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 +295,7 @@ func (s *State) Get(workflowKey string) ([]dep.Dependency, error) { } d := pinToDep(pin) if action, found := s.file.Dependencies[raw]; found { + d.Hostname = 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 +321,7 @@ func (s *State) AllDeps() []dep.Dependency { continue } d := pinToDep(pin) + d.Hostname = 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 +361,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 +399,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 && hostOrDotcom(existing.Hostname) != hostname { + return fmt.Errorf("dependency %s has conflicting hosts %s and %s", pinKey, hostOrDotcom(existing.Hostname), hostname) + } keyToPin[d.Key()] = pinKey var isDirect bool if directKeys != nil { @@ -380,7 +454,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 +515,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) @@ -510,11 +589,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 +613,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 +632,17 @@ 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 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..47937141 100644 --- a/internal/lockfile/state_marshal.go +++ b/internal/lockfile/state_marshal.go @@ -57,6 +57,9 @@ 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" { + addQuotedField(entry, "hostname", hostOrDotcom(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..07c75f9a 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 } @@ -761,6 +761,7 @@ func TestState_SaveFormatIsStable(t *testing.T) { " - 'actions/setup-go@v5'\n" + "dependencies:\n" + " 'actions/checkout@v4':\n" + + " hostname: 'github.com'\n" + " ref: 'v4'\n" + " commit: 'sha1-11111111111111111111111111111111111111aa'\n" + " owner_id: 1\n" + @@ -768,11 +769,13 @@ func TestState_SaveFormatIsStable(t *testing.T) { " uses:\n" + " - 'shared/dep@v1'\n" + " 'actions/setup-go@v5':\n" + + " hostname: 'github.com'\n" + " ref: 'v5'\n" + " commit: 'sha1-22222222222222222222222222222222222222bb'\n" + " owner_id: 1\n" + " repo_id: 2\n" + " 'shared/dep@v1':\n" + + " hostname: 'github.com'\n" + " ref: 'v1'\n" + " commit: 'sha1-33333333333333333333333333333333333333cc'\n" + " owner_id: 1\n" + @@ -845,11 +848,13 @@ func TestState_TransitiveClosureGolden(t *testing.T) { " - 'my-org/leaf@main'\n" + "dependencies:\n" + " 'actions/checkout@v4':\n" + + " hostname: 'github.com'\n" + " ref: 'v4'\n" + " commit: 'sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n" + " owner_id: 1\n" + " repo_id: 2\n" + " 'my-org/composite-a@v1':\n" + + " hostname: 'github.com'\n" + " ref: 'v1'\n" + " commit: 'sha1-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'\n" + " owner_id: 1\n" + @@ -858,6 +863,7 @@ func TestState_TransitiveClosureGolden(t *testing.T) { " - 'my-org/composite-b@v2'\n" + " - 'other-org/external@v1'\n" + " 'my-org/composite-b@v2':\n" + + " hostname: 'github.com'\n" + " ref: 'v2'\n" + " commit: 'sha1-cccccccccccccccccccccccccccccccccccccccc'\n" + " owner_id: 1\n" + @@ -865,6 +871,7 @@ func TestState_TransitiveClosureGolden(t *testing.T) { " uses:\n" + " - 'my-org/leaf@main'\n" + " 'my-org/composite-c@v1':\n" + + " hostname: 'github.com'\n" + " ref: 'v1'\n" + " commit: 'sha1-eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee'\n" + " owner_id: 1\n" + @@ -872,11 +879,13 @@ func TestState_TransitiveClosureGolden(t *testing.T) { " uses:\n" + " - 'my-org/composite-b@v2'\n" + " 'my-org/leaf@main':\n" + + " hostname: 'github.com'\n" + " ref: 'main'\n" + " commit: 'sha1-dddddddddddddddddddddddddddddddddddddddd'\n" + " owner_id: 1\n" + " repo_id: 2\n" + " 'other-org/external@v1':\n" + + " hostname: 'github.com'\n" + " ref: 'v1'\n" + " commit: 'sha1-ffffffffffffffffffffffffffffffffffffffff'\n" + " owner_id: 1\n" + 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/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 From e4d5ed093d0975bd2e0770e56a7e9a0b8d7833e8 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 1 Oct 2026 11:29:43 -0700 Subject: [PATCH 02/10] Allow absent alternate action metadata in GraphQL responses Ignore only NOT_FOUND on the queried action metadata fields, while preserving other field failures. Match live integration expectations to the hostname-aware schema. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- internal/ghapi/graphql_action_files.go | 6 ++ internal/ghapi/graphql_action_files_test.go | 81 +++++++++++++++++++++ test/scenarios/catalog.yml | 20 +++-- 3 files changed, 99 insertions(+), 8 deletions(-) diff --git a/internal/ghapi/graphql_action_files.go b/internal/ghapi/graphql_action_files.go index 9f528474..37ff1a2f 100644 --- a/internal/ghapi/graphql_action_files.go +++ b/internal/ghapi/graphql_action_files.go @@ -278,6 +278,12 @@ func parseActionFileResponse(data map[string]json.RawMessage, refs []ActionFileR 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 } 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/test/scenarios/catalog.yml b/test/scenarios/catalog.yml index 396407df..7bc707e4 100644 --- a/test/scenarios/catalog.yml +++ b/test/scenarios/catalog.yml @@ -1726,12 +1726,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 +1775,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 +1822,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 +1901,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 +1955,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,9 +2095,9 @@ 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 onboarding emits v0.0.3 hostnames and live transitive dependencies, then passes offline verification" needs_token: true tags: [real_repo] live_repo: nodeselector/actions-test-fixtures @@ -2106,7 +2109,8 @@ scenarios: lockfile_deps_cover_direct: true lockfile_deps_cover_indirect: true lockfile_contains: - - "version: 'v0.0.2'" + - "version: 'v0.0.3'" + - "hostname: 'github.com'" - "ref:" - "commit: 'sha1-" - "owner_id:" From 7cf4171339f2d4a65565970d1eb0549d406435dd Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 1 Oct 2026 11:33:20 -0700 Subject: [PATCH 03/10] Scope live generation scenarios to valid fixture workflows Positive generation scenarios must not scan the deliberate orphan-commit workflow now that incomplete resolution prevents all writes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- test/scenarios/catalog.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/scenarios/catalog.yml b/test/scenarios/catalog.yml index 7bc707e4..6eddf07f 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: @@ -2097,14 +2099,15 @@ scenarios: - name: onboard_roundtrip_v003 category: onboarding - description: "Real-binary onboarding emits v0.0.3 hostnames and live transitive dependencies, then passes offline verification" + description: "Real-binary onboarding emits v0.0.3 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 From 0188c198c4abea567d2c35c3bd10660d9050efee Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 6 Oct 2026 23:38:37 -0700 Subject: [PATCH 04/10] Point moved-ref remediation to the safe relock command Ordinary generation preserves recorded pins. Recommend the explicit refresh option without accepting unreachable pins. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- cmd/gh-actions-lock/command_test.go | 1 + internal/pipeline/checks/misleading.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/gh-actions-lock/command_test.go b/cmd/gh-actions-lock/command_test.go index c3ca7c62..827023b4 100644 --- a/cmd/gh-actions-lock/command_test.go +++ b/cmd/gh-actions-lock/command_test.go @@ -1030,6 +1030,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/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 } From 70e2b076564b2d3f5a3ca396baca861a0d98084e Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Tue, 6 Oct 2026 23:57:08 -0700 Subject: [PATCH 05/10] Explain tenant host discovery and credential overrides Document plain-command setup, host and token precedence, safe troubleshooting, and unreleased availability. Surface auth guidance in help without changing override or failure behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 100 +++++++++++++++++++++++++++- cmd/gh-actions-lock/command_test.go | 18 +++++ cmd/gh-actions-lock/root.go | 19 ++++++ 3 files changed, 136 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ef6f7283..5ecfef63 100644 --- a/README.md +++ b/README.md @@ -50,12 +50,110 @@ those as well. ### GitHub Enterprise Cloud with data residency -Select your tenant with `--hostname` or `GH_HOST`: +> [!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 the selected host, the first **nonempty** credential source wins: + +| Selected host | Credential precedence | +| --- | --- | +| `github.com` or `*.ghe.com` (GitHub Enterprise Cloud) | `GH_TOKEN`, then `GITHUB_TOKEN`, then stored credentials for that host | +| GitHub Enterprise Server, such as `github.example.com` | `GH_ENTERPRISE_TOKEN`, then `GITHUB_ENTERPRISE_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 GH_ENTERPRISE_TOKEN GITHUB_ENTERPRISE_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. For GitHub Enterprise Server, use the corresponding +`GH_ENTERPRISE_TOKEN` and `GITHUB_ENTERPRISE_TOKEN` variables when diagnosing +credential conflicts. 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 diff --git a/cmd/gh-actions-lock/command_test.go b/cmd/gh-actions-lock/command_test.go index 827023b4..c79902c6 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", + "GH_ENTERPRISE_TOKEN and GITHUB_ENTERPRISE_TOKEN", + "--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) diff --git a/cmd/gh-actions-lock/root.go b/cmd/gh-actions-lock/root.go index db653ea6..056aa408 100644 --- a/cmd/gh-actions-lock/root.go +++ b/cmd/gh-actions-lock/root.go @@ -83,6 +83,25 @@ 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 + +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. For GitHub Enterprise Server, the +equivalent variables are GH_ENTERPRISE_TOKEN and GITHUB_ENTERPRISE_TOKEN. +--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 From c645ac534928bca105611d775a4656b16fda7939 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Wed, 7 Oct 2026 00:01:29 -0700 Subject: [PATCH 06/10] Clarify that GitHub Enterprise Server is unsupported Remove GHES setup guidance and support implications from customer docs and help. Runtime host and authentication behavior is unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 21 +++++++++------------ cmd/gh-actions-lock/command_test.go | 2 +- cmd/gh-actions-lock/root.go | 6 ++++-- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 5ecfef63..1428e3bb 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 @@ -92,12 +96,8 @@ unambiguous host override: gh actions-lock --hostname octocorp.ghe.com --no-interactive ``` -For the selected host, the first **nonempty** credential source wins: - -| Selected host | Credential precedence | -| --- | --- | -| `github.com` or `*.ghe.com` (GitHub Enterprise Cloud) | `GH_TOKEN`, then `GITHUB_TOKEN`, then stored credentials for that host | -| GitHub Enterprise Server, such as `github.example.com` | `GH_ENTERPRISE_TOKEN`, then `GITHUB_ENTERPRISE_TOKEN`, then stored credentials for that host | +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`. @@ -111,7 +111,7 @@ credentials or anonymous access. Check which overrides are set without printing their values: ```bash -for name in GH_HOST GH_REPO GH_TOKEN GITHUB_TOKEN GH_ENTERPRISE_TOKEN GITHUB_ENTERPRISE_TOKEN; do +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 @@ -145,9 +145,7 @@ env -u GH_HOST -u GH_REPO -u GH_TOKEN -u GITHUB_TOKEN \ ``` Keep intentional overrides, especially in automation; supply a token valid for -the selected host instead. For GitHub Enterprise Server, use the corresponding -`GH_ENTERPRISE_TOKEN` and `GITHUB_ENTERPRISE_TOKEN` variables when diagnosing -credential conflicts. Do not share token values or use +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. @@ -169,8 +167,7 @@ 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. GitHub Enterprise Server remains host-local and writes v0.0.2, since -the v0.0.3 hostname field does not support GitHub Enterprise Server hosts. +dotcom. If any dependency cannot be resolved, generation exits nonzero without writing an incomplete lockfile. `--json` reports `valid: false`. `--verify-local` checks diff --git a/cmd/gh-actions-lock/command_test.go b/cmd/gh-actions-lock/command_test.go index c79902c6..fdeba99d 100644 --- a/cmd/gh-actions-lock/command_test.go +++ b/cmd/gh-actions-lock/command_test.go @@ -28,7 +28,7 @@ func TestCheckCommand_HelpExplainsHostAndAuthOverrides(t *testing.T) { "Host selection: --hostname, then GH_HOST", "GH_REPO or a remote on a host known to gh", "GH_TOKEN takes precedence over GITHUB_TOKEN", - "GH_ENTERPRISE_TOKEN and GITHUB_ENTERPRISE_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", } { diff --git a/cmd/gh-actions-lock/root.go b/cmd/gh-actions-lock/root.go index 056aa408..1d67ec9b 100644 --- a/cmd/gh-actions-lock/root.go +++ b/cmd/gh-actions-lock/root.go @@ -85,14 +85,16 @@ 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. For GitHub Enterprise Server, the -equivalent variables are GH_ENTERPRISE_TOKEN and GITHUB_ENTERPRISE_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 From fabfb9b35a14f047c06c1776b9fbb15117ae61a5 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Wed, 7 Oct 2026 10:34:25 -0700 Subject: [PATCH 07/10] Omit default hostname from dotcom-generated lockfiles Keep explicit dotcom and tenant hosts for Proxima generation. Dotcom saves retain v0.0.3 and omit the implicit github.com field without changing dependency identities. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 7 +++-- internal/lockfile/hosts_test.go | 43 ++++++++++++++++++++++++++++++ internal/lockfile/state.go | 2 +- internal/lockfile/state_marshal.go | 4 +-- internal/lockfile/state_test.go | 9 ------- test/scenarios/catalog.yml | 4 +-- 6 files changed, 53 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 1428e3bb..d5373e41 100644 --- a/README.md +++ b/README.md @@ -157,8 +157,11 @@ 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. -The v0.0.3 lockfile records each dependency's hostname alongside its commit and -host-specific repository IDs. Existing pins retain their recorded host, including +When generating from a `*.ghe.com` tenant, the v0.0.3 lockfile records every +dependency's hostname, including `github.com` for public dependencies, alongside +its commit and host-specific repository IDs. Dotcom-root generation and refresh +also write v0.0.3 but omit `hostname` for dotcom dependencies; omission means +`github.com`. Existing pins retain their host identity, including during `--rescan` and `--relock`. Legacy v0.0.1/v0.0.2 pins and v0.0.3 pins without a hostname mean `github.com`. Legacy repository IDs are checked against dotcom before migration on a tenant; tenant pins generated by older CLI versions must diff --git a/internal/lockfile/hosts_test.go b/internal/lockfile/hosts_test.go index 05995c30..c46d8cab 100644 --- a/internal/lockfile/hosts_test.go +++ b/internal/lockfile/hosts_test.go @@ -16,6 +16,48 @@ import ( 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 @@ -27,6 +69,7 @@ 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} diff --git a/internal/lockfile/state.go b/internal/lockfile/state.go index bf629511..c8a8ccd2 100644 --- a/internal/lockfile/state.go +++ b/internal/lockfile/state.go @@ -570,7 +570,7 @@ func (s *State) Save() error { return nil } - out, err := marshalDeterministic(s.file) + out, err := marshalDeterministic(s.file, ghapi.IsProxima(s.hostname)) if err != nil { return err } diff --git a/internal/lockfile/state_marshal.go b/internal/lockfile/state_marshal.go index 47937141..b5613c7e 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, explicitDotcomHost bool) ([]byte, error) { root := &yaml.Node{Kind: yaml.MappingNode} addQuotedField(root, "version", file.Version) @@ -57,7 +57,7 @@ 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" { + if file.Version == "v0.0.3" && (explicitDotcomHost || hostOrDotcom(a.Hostname) != "github.com") { addQuotedField(entry, "hostname", hostOrDotcom(a.Hostname)) } if a.Ref != "" { diff --git a/internal/lockfile/state_test.go b/internal/lockfile/state_test.go index 07c75f9a..e2d08780 100644 --- a/internal/lockfile/state_test.go +++ b/internal/lockfile/state_test.go @@ -761,7 +761,6 @@ func TestState_SaveFormatIsStable(t *testing.T) { " - 'actions/setup-go@v5'\n" + "dependencies:\n" + " 'actions/checkout@v4':\n" + - " hostname: 'github.com'\n" + " ref: 'v4'\n" + " commit: 'sha1-11111111111111111111111111111111111111aa'\n" + " owner_id: 1\n" + @@ -769,13 +768,11 @@ func TestState_SaveFormatIsStable(t *testing.T) { " uses:\n" + " - 'shared/dep@v1'\n" + " 'actions/setup-go@v5':\n" + - " hostname: 'github.com'\n" + " ref: 'v5'\n" + " commit: 'sha1-22222222222222222222222222222222222222bb'\n" + " owner_id: 1\n" + " repo_id: 2\n" + " 'shared/dep@v1':\n" + - " hostname: 'github.com'\n" + " ref: 'v1'\n" + " commit: 'sha1-33333333333333333333333333333333333333cc'\n" + " owner_id: 1\n" + @@ -848,13 +845,11 @@ func TestState_TransitiveClosureGolden(t *testing.T) { " - 'my-org/leaf@main'\n" + "dependencies:\n" + " 'actions/checkout@v4':\n" + - " hostname: 'github.com'\n" + " ref: 'v4'\n" + " commit: 'sha1-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'\n" + " owner_id: 1\n" + " repo_id: 2\n" + " 'my-org/composite-a@v1':\n" + - " hostname: 'github.com'\n" + " ref: 'v1'\n" + " commit: 'sha1-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb'\n" + " owner_id: 1\n" + @@ -863,7 +858,6 @@ func TestState_TransitiveClosureGolden(t *testing.T) { " - 'my-org/composite-b@v2'\n" + " - 'other-org/external@v1'\n" + " 'my-org/composite-b@v2':\n" + - " hostname: 'github.com'\n" + " ref: 'v2'\n" + " commit: 'sha1-cccccccccccccccccccccccccccccccccccccccc'\n" + " owner_id: 1\n" + @@ -871,7 +865,6 @@ func TestState_TransitiveClosureGolden(t *testing.T) { " uses:\n" + " - 'my-org/leaf@main'\n" + " 'my-org/composite-c@v1':\n" + - " hostname: 'github.com'\n" + " ref: 'v1'\n" + " commit: 'sha1-eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee'\n" + " owner_id: 1\n" + @@ -879,13 +872,11 @@ func TestState_TransitiveClosureGolden(t *testing.T) { " uses:\n" + " - 'my-org/composite-b@v2'\n" + " 'my-org/leaf@main':\n" + - " hostname: 'github.com'\n" + " ref: 'main'\n" + " commit: 'sha1-dddddddddddddddddddddddddddddddddddddddd'\n" + " owner_id: 1\n" + " repo_id: 2\n" + " 'other-org/external@v1':\n" + - " hostname: 'github.com'\n" + " ref: 'v1'\n" + " commit: 'sha1-ffffffffffffffffffffffffffffffffffffffff'\n" + " owner_id: 1\n" + diff --git a/test/scenarios/catalog.yml b/test/scenarios/catalog.yml index 6eddf07f..f0ea9516 100644 --- a/test/scenarios/catalog.yml +++ b/test/scenarios/catalog.yml @@ -2099,7 +2099,7 @@ scenarios: - name: onboard_roundtrip_v003 category: onboarding - description: "Real-binary onboarding emits v0.0.3 hostnames and a complete live transitive dependency graph" + 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 @@ -2113,11 +2113,11 @@ scenarios: lockfile_deps_cover_indirect: true lockfile_contains: - "version: 'v0.0.3'" - - "hostname: 'github.com'" - "ref:" - "commit: 'sha1-" - "owner_id:" - "repo_id:" + lockfile_excludes: ["hostname:"] lockfile_comment_excludes: '^\s+tag:' lockfile_comment_matches: "ref: '" From c93e24228bd8325a227efa17689ea6cac96e3bd7 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Wed, 7 Oct 2026 11:36:49 -0700 Subject: [PATCH 08/10] Bind omitted lockfile hostnames to the home host Keep public dotcom pins explicit on Proxima and verify recorded repository IDs before reusing tenant-side pins. Preserve legacy dotcom bindings during migration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 31 +++++++++++----- cmd/gh-actions-lock/proxima_test.go | 50 ++++++++++++++++++++++++- cmd/gh-actions-lock/run.go | 2 +- cmd/gh-actions-lock/verify.go | 3 ++ internal/ghapi/hosts.go | 2 +- internal/ghapi/hosts_test.go | 2 + internal/lockfile/hosts_test.go | 57 +++++++++++++++++++++++------ internal/lockfile/state.go | 43 ++++++++++++++-------- internal/lockfile/state_marshal.go | 9 +++-- 9 files changed, 157 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index d5373e41..be659ffe 100644 --- a/README.md +++ b/README.md @@ -157,16 +157,27 @@ 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. -When generating from a `*.ghe.com` tenant, the v0.0.3 lockfile records every -dependency's hostname, including `github.com` for public dependencies, alongside -its commit and host-specific repository IDs. Dotcom-root generation and refresh -also write v0.0.3 but omit `hostname` for dotcom dependencies; omission means -`github.com`. Existing pins retain their host identity, including -during `--rescan` and `--relock`. Legacy v0.0.1/v0.0.2 pins and v0.0.3 pins without -a hostname mean `github.com`. Legacy repository IDs are checked against dotcom -before migration on a tenant; tenant pins generated by older CLI versions must -be regenerated rather than relabeled. Conflicting host assignments for the same -repository are rejected. +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 diff --git a/cmd/gh-actions-lock/proxima_test.go b/cmd/gh-actions-lock/proxima_test.go index 81f4cd7f..6b7d234e 100644 --- a/cmd/gh-actions-lock/proxima_test.go +++ b/cmd/gh-actions-lock/proxima_test.go @@ -93,7 +93,7 @@ jobs: assert.Equal(t, "v0.0.3", file.Version) require.Len(t, file.Dependencies, 2) local := file.Dependencies["tenant/internal@v1"] - assert.Equal(t, "tenant.ghe.com", local.Hostname) + assert.Empty(t, local.Hostname) assert.Equal(t, "sha1-"+tenantSHA, local.Commit) assert.EqualValues(t, 11, local.OwnerID) assert.EqualValues(t, 111, local.RepoID) @@ -118,6 +118,54 @@ jobs: "--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) { diff --git a/cmd/gh-actions-lock/run.go b/cmd/gh-actions-lock/run.go index 660ae93f..1c77a45f 100644 --- a/cmd/gh-actions-lock/run.go +++ b/cmd/gh-actions-lock/run.go @@ -176,7 +176,7 @@ func runCheck(cmd *cobra.Command, opts *checkOptions, newResolver resolverFunc) if err != nil { return err } - if err := store.VerifyLegacyHosts(ctx); err != nil { + if err := store.VerifyHosts(ctx); err != nil { return err } // Pre-warm resolver caches from the lockfile so repeat runs skip 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/internal/ghapi/hosts.go b/internal/ghapi/hosts.go index 189a14b9..70ac0aca 100644 --- a/internal/ghapi/hosts.go +++ b/internal/ghapi/hosts.go @@ -17,7 +17,7 @@ func IsProxima(hostname string) bool { return proximaHost.MatchString(hostname) // repository-scoped, and guessing would mix identities. func (c *Client) PinHost(owner, repo, hostname string) error { if hostname == "" { - hostname = "github.com" + 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) diff --git a/internal/ghapi/hosts_test.go b/internal/ghapi/hosts_test.go index 3f77aa93..07f2e2a3 100644 --- a/internal/ghapi/hosts_test.go +++ b/internal/ghapi/hosts_test.go @@ -116,6 +116,8 @@ func TestPinnedHostBoundaries(t *testing.T) { 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") diff --git a/internal/lockfile/hosts_test.go b/internal/lockfile/hosts_test.go index c46d8cab..b6294c6a 100644 --- a/internal/lockfile/hosts_test.go +++ b/internal/lockfile/hosts_test.go @@ -77,14 +77,18 @@ func TestHostScopedMetadataAndPinCollisions(t *testing.T) { 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.Equal(t, "tenant.ghe.com", file.Dependencies["o/r@tenant"].Hostname) + 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") @@ -103,30 +107,61 @@ func TestLegacyAndOmittedHostnames(t *testing.T) { ref = "tag: v1" } path := filepath.Join(t.TempDir(), "actions.lock") - 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: 1\n repo_id: 2\n", version, key, key, ref, strings.Repeat("a", 40)) + 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, "github.com", deps[0].Hostname) + assert.Equal(t, host, deps[0].Hostname) + assert.Equal(t, strings.Repeat("a", 40), deps[0].SHA) store.SetMetadataResolver(hostMetadata{}) - require.NoError(t, store.VerifyLegacyHosts(context.Background())) - if version != "v0.0.3" { - action := store.file.Dependencies["o/r@v1"] - action.RepoID = 200 - store.file.Dependencies["o/r@v1"] = action - require.ErrorContains(t, store.VerifyLegacyHosts(context.Background()), "regenerate the lockfile") - } + 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), "hostname: 'github.com'") + 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{}) diff --git a/internal/lockfile/state.go b/internal/lockfile/state.go index c8a8ccd2..f145d4b5 100644 --- a/internal/lockfile/state.go +++ b/internal/lockfile/state.go @@ -121,7 +121,6 @@ func LoadStateAt(lockfilePath string, meta MetadataResolver) (*State, error) { // legacy mixed-case keys are rewritten on the next Save. normalizedDependencies := make(map[string]parserlock.Action, len(file.Dependencies)) for pinKey, action := range file.Dependencies { - action.Hostname = hostOrDotcom(action.Hostname) pin, ok := parserlock.ParsePin(pinKey) if !ok { normalizedDependencies[pinKey] = action @@ -157,13 +156,19 @@ func LoadStateAt(lockfilePath string, meta MetadataResolver) (*State, error) { return s, nil } -// SetHostname preserves the legacy host-local format on GHES, whose hostnames -// are not supported by the v0.0.3 schema. +// 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" { @@ -205,12 +210,12 @@ func (s *State) SetMetadataResolver(meta MetadataResolver) { s.meta = meta } -// VerifyLegacyHosts prevents old tenant-generated pins from being relabeled as -// dotcom pins during migration. Legacy schemas carry no host provenance. -func (s *State) VerifyLegacyHosts(ctx context.Context) error { +// 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 == "" || s.originalVersion == "v0.0.3" { + if !ghapi.IsProxima(s.hostname) || s.originalVersion == "" { return nil } for key, action := range s.file.Dependencies { @@ -221,12 +226,13 @@ func (s *State) VerifyLegacyHosts(ctx context.Context) error { if !ok { return fmt.Errorf("invalid dependency %q", key) } - ownerID, repoID, err := s.meta.RepoIDs(ctx, "github.com", pin.Owner, pin.Repo) + hostname := s.hostOrHome(action.Hostname) + ownerID, repoID, err := s.meta.RepoIDs(ctx, hostname, pin.Owner, pin.Repo) if err != nil { - return fmt.Errorf("verifying legacy dotcom identity for %s: %w", key, err) + return fmt.Errorf("verifying repository identity for %s on %s: %w", key, hostname, err) } if ownerID != action.OwnerID || repoID != action.RepoID { - return fmt.Errorf("legacy dependency %s does not match its github.com repository IDs; regenerate the lockfile with the selected hostname", key) + 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 @@ -295,7 +301,7 @@ func (s *State) Get(workflowKey string) ([]dep.Dependency, error) { } d := pinToDep(pin) if action, found := s.file.Dependencies[raw]; found { - d.Hostname = action.Hostname + 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] @@ -321,7 +327,7 @@ func (s *State) AllDeps() []dep.Dependency { continue } d := pinToDep(pin) - d.Hostname = action.Hostname + 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] @@ -403,8 +409,8 @@ func (s *State) Set(ctx context.Context, workflowKey string, deps []dep.Dependen if hostname == "" { hostname = s.hostname } - if existing, ok := s.file.Dependencies[pinKey]; ok && hostOrDotcom(existing.Hostname) != hostname { - return fmt.Errorf("dependency %s has conflicting hosts %s and %s", pinKey, hostOrDotcom(existing.Hostname), 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 @@ -570,7 +576,7 @@ func (s *State) Save() error { return nil } - out, err := marshalDeterministic(s.file, ghapi.IsProxima(s.hostname)) + out, err := marshalDeterministic(s.file, s.hostname) if err != nil { return err } @@ -639,6 +645,13 @@ func hostOrDotcom(hostname string) string { 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) } diff --git a/internal/lockfile/state_marshal.go b/internal/lockfile/state_marshal.go index b5613c7e..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, explicitDotcomHost bool) ([]byte, error) { +func marshalDeterministic(file parserlock.File, homeHost string) ([]byte, error) { root := &yaml.Node{Kind: yaml.MappingNode} addQuotedField(root, "version", file.Version) @@ -57,8 +57,11 @@ func marshalDeterministic(file parserlock.File, explicitDotcomHost bool) ([]byte for _, k := range keys { a := file.Dependencies[k] entry := &yaml.Node{Kind: yaml.MappingNode} - if file.Version == "v0.0.3" && (explicitDotcomHost || hostOrDotcom(a.Hostname) != "github.com") { - addQuotedField(entry, "hostname", hostOrDotcom(a.Hostname)) + 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) From 75f995bbde231ab4afdbc74803e0a13b2ac1d9fd Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 8 Oct 2026 13:20:21 -0700 Subject: [PATCH 09/10] Restore explicit tenant hostnames for deployed Launch contract Interpret omitted hosts as github.com in all schemas. Keep identity verification before Proxima pin reuse and document migration of tenant pins lacking provenance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 30 ++++++++++------------- cmd/gh-actions-lock/proxima_test.go | 37 ++++++++++++++++------------- internal/ghapi/hosts.go | 2 +- internal/ghapi/hosts_test.go | 5 ++-- internal/lockfile/hosts_test.go | 15 +++--------- internal/lockfile/state.go | 29 +++++++--------------- internal/lockfile/state_marshal.go | 9 ++++--- 7 files changed, 55 insertions(+), 72 deletions(-) diff --git a/README.md b/README.md index be659ffe..b4a55b3a 100644 --- a/README.md +++ b/README.md @@ -157,23 +157,19 @@ 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. +Generation and refresh write v0.0.3. On Proxima, every dependency records its +hostname: the exact `.ghe.com` for tenant-local pins, or `github.com` +for public dotcom pins. Dotcom-root output omits `hostname` for dotcom pins. +An omitted hostname always means `github.com`, never the current tenant. + +Legacy v0.0.1/v0.0.2 pins and omitted-host v0.0.3 pins retain their dotcom +binding. On Proxima, recorded repository IDs are checked on the bound host +before pins are reused or migrated. A missing repository or mismatched IDs +fails without switching hosts or rewriting the pin. Tenant pins generated +without a hostname by an older or preview CLI must not be silently relabeled: +restore a correctly host-bound lockfile or review the dependencies before +regenerating. Read-only checks do not migrate files. `--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 diff --git a/cmd/gh-actions-lock/proxima_test.go b/cmd/gh-actions-lock/proxima_test.go index 6b7d234e..2f206e76 100644 --- a/cmd/gh-actions-lock/proxima_test.go +++ b/cmd/gh-actions-lock/proxima_test.go @@ -82,7 +82,6 @@ jobs: 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...) @@ -93,7 +92,8 @@ jobs: 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, "tenant.ghe.com", local.Hostname) + assert.Equal(t, []string{"tenant/internal@v1"}, file.Workflows[path]) assert.Equal(t, "sha1-"+tenantSHA, local.Commit) assert.EqualValues(t, 11, local.OwnerID) assert.EqualValues(t, 111, local.RepoID) @@ -119,16 +119,20 @@ jobs: 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)) + // Omitted public hostnames retain their dotcom binding. + omitted := strings.Replace(raw, " hostname: 'github.com'\n", "", 1) + require.NotEqual(t, raw, omitted) + require.NoError(t, os.WriteFile(parserlock.Path, []byte(omitted), 0o600)) + _, _, err = runCommandWithHTTP(t, pinnedTransport, + "--hostname", "tenant.ghe.com", "--rescan", "--no-fix", "--json", path) + require.NoError(t, err) + assert.Equal(t, omitted, readTempLockfilePins(t)) _, _, err = runCommandWithHTTP(t, proximaFixture(t, 200), args...) require.NoError(t, err) assert.Equal(t, raw, readTempLockfilePins(t)) } -func TestProximaOmittedPinsCannotMoveToDotcomOrTenantNamesakes(t *testing.T) { +func TestProximaOmittedPinsCannotBecomeTenantPins(t *testing.T) { for _, status := range []int{200, 401, 403, 404} { t.Run(fmt.Sprint(status), func(t *testing.T) { path := writeTempWorkflow(t, ` @@ -138,28 +142,29 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/public@v2 -`, "actions/public@v2=sha1-"+publicSHA) + - uses: tenant/internal@v1 +`, "tenant/internal@v1=sha1-"+tenantSHA) raw := readTempLockfilePins(t) - raw = strings.ReplaceAll(raw, "owner_id: 1", "owner_id: 22") - raw = strings.ReplaceAll(raw, "repo_id: 1", "repo_id: 222") + raw = strings.ReplaceAll(raw, "owner_id: 1", "owner_id: 11") + raw = strings.ReplaceAll(raw, "repo_id: 1", "repo_id: 111") 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")) + assert.Equal(t, "api.github.com", req.URL.Host, "omitted pins must bind to dotcom, never the tenant") + assert.Equal(t, "/repos/tenant/internal", req.URL.Path) + assert.Empty(t, req.Header.Get("Authorization")) + assert.Empty(t, req.Header.Get("Cookie")) if status != 200 { return httpmock.StatusResponse(status)(req) } return httpmock.JSONResponse(map[string]any{ - "visibility": "internal", "id": 333, "owner": map[string]any{"id": 33}, + "visibility": "public", "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") + assert.ErrorContains(t, err, "does not match its github.com repository IDs") } else { assert.ErrorContains(t, err, "verifying repository identity") } diff --git a/internal/ghapi/hosts.go b/internal/ghapi/hosts.go index 70ac0aca..189a14b9 100644 --- a/internal/ghapi/hosts.go +++ b/internal/ghapi/hosts.go @@ -17,7 +17,7 @@ func IsProxima(hostname string) bool { return proximaHost.MatchString(hostname) // repository-scoped, and guessing would mix identities. func (c *Client) PinHost(owner, repo, hostname string) error { if hostname == "" { - hostname = c.Hostname + hostname = "github.com" } 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) diff --git a/internal/ghapi/hosts_test.go b/internal/ghapi/hosts_test.go index 07f2e2a3..c9b42804 100644 --- a/internal/ghapi/hosts_test.go +++ b/internal/ghapi/hosts_test.go @@ -116,8 +116,9 @@ func TestPinnedHostBoundaries(t *testing.T) { 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.NoError(t, c.PinHost("o", "omitted", "")) + require.NoError(t, c.PinHost("o", "omitted", "github.com")) + require.ErrorContains(t, c.PinHost("o", "omitted", "tenant.ghe.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") diff --git a/internal/lockfile/hosts_test.go b/internal/lockfile/hosts_test.go index b6294c6a..3a0f4a0a 100644 --- a/internal/lockfile/hosts_test.go +++ b/internal/lockfile/hosts_test.go @@ -79,7 +79,7 @@ func TestHostScopedMetadataAndPinCollisions(t *testing.T) { 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.Equal(t, "tenant.ghe.com", 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) @@ -107,12 +107,7 @@ func TestLegacyAndOmittedHostnames(t *testing.T) { 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) @@ -120,7 +115,7 @@ func TestLegacyAndOmittedHostnames(t *testing.T) { 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, "github.com", deps[0].Hostname) assert.Equal(t, strings.Repeat("a", 40), deps[0].SHA) store.SetMetadataResolver(hostMetadata{}) require.NoError(t, store.VerifyHosts(context.Background())) @@ -128,11 +123,7 @@ func TestLegacyAndOmittedHostnames(t *testing.T) { 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'") - } + 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) { diff --git a/internal/lockfile/state.go b/internal/lockfile/state.go index f145d4b5..32548a8e 100644 --- a/internal/lockfile/state.go +++ b/internal/lockfile/state.go @@ -121,6 +121,7 @@ func LoadStateAt(lockfilePath string, meta MetadataResolver) (*State, error) { // legacy mixed-case keys are rewritten on the next Save. normalizedDependencies := make(map[string]parserlock.Action, len(file.Dependencies)) for pinKey, action := range file.Dependencies { + action.Hostname = hostOrDotcom(action.Hostname) pin, ok := parserlock.ParsePin(pinKey) if !ok { normalizedDependencies[pinKey] = action @@ -156,19 +157,12 @@ 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. +// SetHostname selects the output host without changing recorded pin bindings. 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" { @@ -211,7 +205,7 @@ func (s *State) SetMetadataResolver(meta MetadataResolver) { } // VerifyHosts checks recorded identities before trusting pins on Proxima. -// Older producers used omission for dotcom, not the home tenant. +// An omitted hostname always binds to dotcom, including in legacy schemas. func (s *State) VerifyHosts(ctx context.Context) error { s.mu.Lock() defer s.mu.Unlock() @@ -226,7 +220,7 @@ func (s *State) VerifyHosts(ctx context.Context) error { if !ok { return fmt.Errorf("invalid dependency %q", key) } - hostname := s.hostOrHome(action.Hostname) + hostname := hostOrDotcom(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) @@ -301,7 +295,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.Hostname = hostOrDotcom(action.Hostname) d.Tag, d.Branch = parserlock.SplitRef(action.Ref) if idx := strings.Index(action.Commit, "-"); idx >= 0 { d.HashAlgo = action.Commit[:idx] @@ -327,7 +321,7 @@ func (s *State) AllDeps() []dep.Dependency { continue } d := pinToDep(pin) - d.Hostname = s.hostOrHome(action.Hostname) + d.Hostname = hostOrDotcom(action.Hostname) d.Tag, d.Branch = parserlock.SplitRef(action.Ref) if idx := strings.Index(action.Commit, "-"); idx >= 0 { d.HashAlgo = action.Commit[:idx] @@ -409,8 +403,8 @@ func (s *State) Set(ctx context.Context, workflowKey string, deps []dep.Dependen 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) + if existing, ok := s.file.Dependencies[pinKey]; ok && hostOrDotcom(existing.Hostname) != hostname { + return fmt.Errorf("dependency %s has conflicting hosts %s and %s", pinKey, hostOrDotcom(existing.Hostname), hostname) } keyToPin[d.Key()] = pinKey var isDirect bool @@ -645,13 +639,6 @@ func hostOrDotcom(hostname string) string { 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) } diff --git a/internal/lockfile/state_marshal.go b/internal/lockfile/state_marshal.go index 39452638..792d4690 100644 --- a/internal/lockfile/state_marshal.go +++ b/internal/lockfile/state_marshal.go @@ -57,11 +57,14 @@ func marshalDeterministic(file parserlock.File, homeHost string) ([]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" { + hostname := hostOrDotcom(a.Hostname) + if file.Version == "v0.0.3" { + if hostname != "github.com" && hostname != homeHost { 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 homeHost != "github.com" || hostname != "github.com" { + addQuotedField(entry, "hostname", hostname) + } } if a.Ref != "" { addQuotedField(entry, "ref", a.Ref) From c5ee15d8f20e3e52d6671851192770b9a2e02152 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 8 Oct 2026 13:35:36 -0700 Subject: [PATCH 10/10] Restore home-host omission contract Undo the explicit-tenant-host change. Only dotcom dependencies resolved from a Proxima root receive a hostname field. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 30 +++++++++++++---------- cmd/gh-actions-lock/proxima_test.go | 37 +++++++++++++---------------- internal/ghapi/hosts.go | 2 +- internal/ghapi/hosts_test.go | 5 ++-- internal/lockfile/hosts_test.go | 15 +++++++++--- internal/lockfile/state.go | 29 +++++++++++++++------- internal/lockfile/state_marshal.go | 9 +++---- 7 files changed, 72 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index b4a55b3a..be659ffe 100644 --- a/README.md +++ b/README.md @@ -157,19 +157,23 @@ 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. On Proxima, every dependency records its -hostname: the exact `.ghe.com` for tenant-local pins, or `github.com` -for public dotcom pins. Dotcom-root output omits `hostname` for dotcom pins. -An omitted hostname always means `github.com`, never the current tenant. - -Legacy v0.0.1/v0.0.2 pins and omitted-host v0.0.3 pins retain their dotcom -binding. On Proxima, recorded repository IDs are checked on the bound host -before pins are reused or migrated. A missing repository or mismatched IDs -fails without switching hosts or rewriting the pin. Tenant pins generated -without a hostname by an older or preview CLI must not be silently relabeled: -restore a correctly host-bound lockfile or review the dependencies before -regenerating. Read-only checks do not migrate files. `--verify-local` checks -coverage, not host identity. +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 diff --git a/cmd/gh-actions-lock/proxima_test.go b/cmd/gh-actions-lock/proxima_test.go index 2f206e76..6b7d234e 100644 --- a/cmd/gh-actions-lock/proxima_test.go +++ b/cmd/gh-actions-lock/proxima_test.go @@ -82,6 +82,7 @@ jobs: 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...) @@ -92,8 +93,7 @@ jobs: assert.Equal(t, "v0.0.3", file.Version) require.Len(t, file.Dependencies, 2) local := file.Dependencies["tenant/internal@v1"] - assert.Equal(t, "tenant.ghe.com", local.Hostname) - assert.Equal(t, []string{"tenant/internal@v1"}, file.Workflows[path]) + assert.Empty(t, local.Hostname) assert.Equal(t, "sha1-"+tenantSHA, local.Commit) assert.EqualValues(t, 11, local.OwnerID) assert.EqualValues(t, 111, local.RepoID) @@ -119,20 +119,16 @@ jobs: require.NoError(t, err) assert.Equal(t, raw, readTempLockfilePins(t)) - // Omitted public hostnames retain their dotcom binding. - omitted := strings.Replace(raw, " hostname: 'github.com'\n", "", 1) - require.NotEqual(t, raw, omitted) - require.NoError(t, os.WriteFile(parserlock.Path, []byte(omitted), 0o600)) - _, _, err = runCommandWithHTTP(t, pinnedTransport, - "--hostname", "tenant.ghe.com", "--rescan", "--no-fix", "--json", path) - require.NoError(t, err) - assert.Equal(t, omitted, 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 TestProximaOmittedPinsCannotBecomeTenantPins(t *testing.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, ` @@ -142,29 +138,28 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: tenant/internal@v1 -`, "tenant/internal@v1=sha1-"+tenantSHA) + - uses: actions/public@v2 +`, "actions/public@v2=sha1-"+publicSHA) raw := readTempLockfilePins(t) - raw = strings.ReplaceAll(raw, "owner_id: 1", "owner_id: 11") - raw = strings.ReplaceAll(raw, "repo_id: 1", "repo_id: 111") + 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.github.com", req.URL.Host, "omitted pins must bind to dotcom, never the tenant") - assert.Equal(t, "/repos/tenant/internal", req.URL.Path) - assert.Empty(t, req.Header.Get("Authorization")) - assert.Empty(t, req.Header.Get("Cookie")) + 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": "public", "id": 333, "owner": map[string]any{"id": 33}, + "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 github.com repository IDs") + assert.ErrorContains(t, err, "does not match its tenant.ghe.com repository IDs") } else { assert.ErrorContains(t, err, "verifying repository identity") } diff --git a/internal/ghapi/hosts.go b/internal/ghapi/hosts.go index 189a14b9..70ac0aca 100644 --- a/internal/ghapi/hosts.go +++ b/internal/ghapi/hosts.go @@ -17,7 +17,7 @@ func IsProxima(hostname string) bool { return proximaHost.MatchString(hostname) // repository-scoped, and guessing would mix identities. func (c *Client) PinHost(owner, repo, hostname string) error { if hostname == "" { - hostname = "github.com" + 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) diff --git a/internal/ghapi/hosts_test.go b/internal/ghapi/hosts_test.go index c9b42804..07f2e2a3 100644 --- a/internal/ghapi/hosts_test.go +++ b/internal/ghapi/hosts_test.go @@ -116,9 +116,8 @@ func TestPinnedHostBoundaries(t *testing.T) { 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", "omitted", "")) - require.NoError(t, c.PinHost("o", "omitted", "github.com")) - require.ErrorContains(t, c.PinHost("o", "omitted", "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") diff --git a/internal/lockfile/hosts_test.go b/internal/lockfile/hosts_test.go index 3a0f4a0a..b6294c6a 100644 --- a/internal/lockfile/hosts_test.go +++ b/internal/lockfile/hosts_test.go @@ -79,7 +79,7 @@ func TestHostScopedMetadataAndPinCollisions(t *testing.T) { require.NoError(t, err) require.NoError(t, reloaded.SetHostname("tenant.ghe.com")) file := reloaded.File() - assert.Equal(t, "tenant.ghe.com", file.Dependencies["o/r@tenant"].Hostname) + 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) @@ -107,7 +107,12 @@ func TestLegacyAndOmittedHostnames(t *testing.T) { 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) @@ -115,7 +120,7 @@ func TestLegacyAndOmittedHostnames(t *testing.T) { require.NoError(t, store.SetHostname("tenant.ghe.com")) deps := store.AllDeps() require.Len(t, deps, 1) - assert.Equal(t, "github.com", deps[0].Hostname) + 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())) @@ -123,7 +128,11 @@ func TestLegacyAndOmittedHostnames(t *testing.T) { raw, err := os.ReadFile(path) require.NoError(t, err) assert.Contains(t, string(raw), "version: 'v0.0.3'") - assert.Contains(t, string(raw), "hostname: 'github.com'") + 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) { diff --git a/internal/lockfile/state.go b/internal/lockfile/state.go index 32548a8e..f145d4b5 100644 --- a/internal/lockfile/state.go +++ b/internal/lockfile/state.go @@ -121,7 +121,6 @@ func LoadStateAt(lockfilePath string, meta MetadataResolver) (*State, error) { // legacy mixed-case keys are rewritten on the next Save. normalizedDependencies := make(map[string]parserlock.Action, len(file.Dependencies)) for pinKey, action := range file.Dependencies { - action.Hostname = hostOrDotcom(action.Hostname) pin, ok := parserlock.ParsePin(pinKey) if !ok { normalizedDependencies[pinKey] = action @@ -157,12 +156,19 @@ func LoadStateAt(lockfilePath string, meta MetadataResolver) (*State, error) { return s, nil } -// SetHostname selects the output host without changing recorded pin bindings. +// 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" { @@ -205,7 +211,7 @@ func (s *State) SetMetadataResolver(meta MetadataResolver) { } // VerifyHosts checks recorded identities before trusting pins on Proxima. -// An omitted hostname always binds to dotcom, including in legacy schemas. +// 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() @@ -220,7 +226,7 @@ func (s *State) VerifyHosts(ctx context.Context) error { if !ok { return fmt.Errorf("invalid dependency %q", key) } - hostname := hostOrDotcom(action.Hostname) + 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) @@ -295,7 +301,7 @@ func (s *State) Get(workflowKey string) ([]dep.Dependency, error) { } d := pinToDep(pin) if action, found := s.file.Dependencies[raw]; found { - d.Hostname = hostOrDotcom(action.Hostname) + 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] @@ -321,7 +327,7 @@ func (s *State) AllDeps() []dep.Dependency { continue } d := pinToDep(pin) - d.Hostname = hostOrDotcom(action.Hostname) + 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] @@ -403,8 +409,8 @@ func (s *State) Set(ctx context.Context, workflowKey string, deps []dep.Dependen if hostname == "" { hostname = s.hostname } - if existing, ok := s.file.Dependencies[pinKey]; ok && hostOrDotcom(existing.Hostname) != hostname { - return fmt.Errorf("dependency %s has conflicting hosts %s and %s", pinKey, hostOrDotcom(existing.Hostname), 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 @@ -639,6 +645,13 @@ func hostOrDotcom(hostname string) string { 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) } diff --git a/internal/lockfile/state_marshal.go b/internal/lockfile/state_marshal.go index 792d4690..39452638 100644 --- a/internal/lockfile/state_marshal.go +++ b/internal/lockfile/state_marshal.go @@ -57,14 +57,11 @@ func marshalDeterministic(file parserlock.File, homeHost string) ([]byte, error) for _, k := range keys { a := file.Dependencies[k] entry := &yaml.Node{Kind: yaml.MappingNode} - hostname := hostOrDotcom(a.Hostname) - if file.Version == "v0.0.3" { - if hostname != "github.com" && hostname != homeHost { + 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) } - if homeHost != "github.com" || hostname != "github.com" { - addQuotedField(entry, "hostname", hostname) - } + addQuotedField(entry, "hostname", a.Hostname) } if a.Ref != "" { addQuotedField(entry, "ref", a.Ref)