diff --git a/.github/workflows/builder.yml b/.github/workflows/builder.yml index ae62e9c..ed58f3a 100644 --- a/.github/workflows/builder.yml +++ b/.github/workflows/builder.yml @@ -18,7 +18,7 @@ concurrency: jobs: lint-code: - name: Code Linter ๐Ÿงน + name: Lint Code ๐Ÿงน runs-on: ubuntu-slim permissions: contents: read @@ -39,7 +39,7 @@ jobs: - uses: golangci/golangci-lint-action@v9 lint-actions: - name: Workflow Linter ๐Ÿ“‹ + name: Lint Action โš™๏ธ runs-on: ubuntu-slim permissions: contents: read @@ -87,6 +87,7 @@ jobs: runs-on: ${{ matrix.runner }} permissions: contents: read + pull-requests: write steps: - uses: actions/checkout@v6 - uses: actions/setup-go@v6 @@ -94,6 +95,12 @@ jobs: go-version-file: go.mod - name: Test run: go test ./... + - name: Build tailor + run: go build -o tailor ./cmd/tailor + - name: Baste + run: ./tailor baste + env: + GH_TOKEN: ${{ secrets.TAILOR_TOKEN || secrets.GITHUB_TOKEN }} security: name: Security ๐Ÿ”’ diff --git a/AGENTS.md b/AGENTS.md index 3e0a69a..9189cab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,6 +66,7 @@ tailor/ - `EvaluateTrigger(source string, repo any)` uses reflection to match yaml tags on `RepositorySettings`; `repo` is `any` (not `*config.RepositorySettings`) to avoid a circular import - Five commands: `fit` (bootstrap), `alter` (apply), `baste` (preview), `measure` (inspect), `docket` (inspect) - `fit`, `alter`, and `baste` require a valid GitHub auth token at startup; `measure` and `docket` do not +- GitHub Actions installation tokens (`secrets.GITHUB_TOKEN`) cannot call user-scoped endpoints (e.g. `GET /user`); features hitting such endpoints must check `GITHUB_ACTIONS=true` and fall back to Actions env vars (e.g. `GITHUB_REPOSITORY_OWNER` for the owner name) - see `internal/gh/user.go` for the pattern - `alter` execution order: repository settings, then labels, then licence, then swatches - SHA-256 comparison for `always` and `triggered` swatches; substituted swatches (`.github/FUNDING.yml`, `SECURITY.md`, `.github/ISSUE_TEMPLATE/config.yml`, `.tailor.yml`, `.github/workflows/tailor-automerge.yml`) compare the resolved content hash against the on-disk file - `triggered` swatches deploy when their condition is met (overwrite like `always`), remove the file when the condition becomes false, and skip when the file is absent and condition is false diff --git a/README.md b/README.md index d564bca..4aba336 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,39 @@ The `wimpysworld/tailor` action installs the tailor binary and optionally runs o | `measure` | Run `tailor measure` to check community health files and configuration alignment. | `false` | | `docket` | Run `tailor docket` to display authentication state and repository context. | `false` | +### Token requirements + +`GITHUB_TOKEN` is sufficient for most tailor operations in CI. Two fields are the exception: `default_workflow_permissions` and `can_approve_pull_request_reviews` call the `PUT /repos/{owner}/{repo}/actions/permissions/workflow` endpoint, which requires repository administration access. No `permissions:` block grants `GITHUB_TOKEN` this scope - it is a GitHub platform constraint. + +When `GITHUB_TOKEN` is used, tailor skips those fields and reports: + +``` +would skip (insufficient scope: token missing required scope): default_workflow_permissions +would skip (insufficient scope: token missing required scope): can_approve_pull_request_reviews +``` + +To manage these fields from CI, provide a PAT with the necessary access. + +#### Personal repositories + +Create one of the following: + +- **Classic PAT** at - enable the `repo` scope +- **Fine-grained PAT** at - set "Repository permissions > Administration" to "Read and write" + +#### Organisation repositories + +Use the same PAT creation steps above. The PAT must belong to a user with admin access to the repository. If the organisation enforces SSO, authorise the PAT for the org after creation via the token's "Configure SSO" link. + +#### Storing and using the PAT + +Add the PAT as a repository secret via **Settings > Secrets and variables > Actions**, then pass it as `GH_TOKEN` in the workflow: + +```yaml +env: + GH_TOKEN: ${{ secrets.TAILOR_TOKEN }} +``` + ### Supported platforms Linux (amd64, arm64) and macOS (amd64, arm64). diff --git a/docs/SPECIFICATION.md b/docs/SPECIFICATION.md index f570287..c1ecd7c 100644 --- a/docs/SPECIFICATION.md +++ b/docs/SPECIFICATION.md @@ -94,7 +94,7 @@ Several fields use separate API endpoints rather than the repository PATCH call. **Topics**: The PUT endpoint replaces the entire topics list. The config declares the complete desired set; omitted topics are removed on apply. Topics are project-specific and not included in the default config template. Topic names must start with a lowercase letter or number, contain only lowercase alphanumerics and hyphens, and be 50 characters or fewer. The `topics` field uses `*[]string` semantics: nil (absent) means skip, empty list means clear all topics. -**Actions workflow permissions**: `default_workflow_permissions` accepts `read` or `write`. The PUT endpoint sends both `default_workflow_permissions` and `can_approve_pull_request_reviews` atomically. The tailor defaults (`read` and `false`) follow the principle of least privilege. GitHub defaults vary by context: personal repositories default to restricted `GITHUB_TOKEN` permissions with PR approval disabled, while organisation repositories inherit these settings from organisation-level Actions configuration. +**Actions workflow permissions**: `default_workflow_permissions` accepts `read` or `write`. The PUT endpoint sends both `default_workflow_permissions` and `can_approve_pull_request_reviews` atomically. The tailor defaults (`read` and `false`) follow the principle of least privilege. GitHub defaults vary by context: personal repositories default to restricted `GITHUB_TOKEN` permissions with PR approval disabled, while organisation repositories inherit these settings from organisation-level Actions configuration. Managing these fields from GitHub Actions CI requires a PAT - either a classic PAT with `repo` scope or a fine-grained PAT with the Administration repository permission. `GITHUB_TOKEN` lacks the required scope for the `GET/PUT /repos/{owner}/{repo}/actions/permissions/workflow` endpoint; when tailor runs with `GITHUB_TOKEN`, these two fields are skipped with a `would skip (insufficient scope)` result in `baste` and silently skipped by `alter`. This is graceful degradation, not an error. Settings deliberately excluded due to risk or org-level scope: `visibility`, `default_branch`, `name`, `archived`, `is_template`, `allow_forking`, `security_and_analysis`. Additional API areas considered and deferred: Actions permissions policy (`enabled`, `allowed_actions`), autolinks, Pages configuration, deployment environments, custom properties (org-level), and Dependabot secrets. Branch protection (both classic rules and rulesets) is explicitly out of scope. It requires `Administration: write` - the same permission level needed to delete a repository - which `GITHUB_TOKEN` cannot hold at all; this is a deliberate GitHub security boundary preventing workflows from weakening the rules that govern their own repository. For Tailor's target audience of solo developers and small teams, branch protection is a one-time UI operation that does not drift over time, so the declarative consistency argument that justifies Tailor does not apply. Supporting it would roughly double the PAT privilege requirements for CI users for a setting they configure once, and `gh` CLI handles the setup in a single command, leaving no gap for Tailor to fill. @@ -239,7 +239,7 @@ Behaviour: - For `triggered` swatches: looks up the trigger condition for the swatch source in the trigger condition table. If the condition is met (e.g. `allow_auto_merge: true` in the `repository` section), behaves like `always` - deploys and overwrites when content differs. If the condition is not met and the file exists on disk, removes it. If the condition is not met and the file does not exist, skips silently. Triggered swatches are never overwritten by `--recut` when the trigger condition is false. - For `never` swatches: skips entirely. No file is written, compared, or removed. This mode suppresses any swatch, including triggered swatches whose condition would otherwise be met. - For licences: if `.tailor.yml` contains a `license` key with a value other than `none`, and no `LICENSE` file exists on disk, fetches the licence text via the GitHub REST API (`GET /licenses/{id}`) and writes it to `LICENSE`. The text is written verbatim as returned by GitHub - no token substitution is performed. Always treated as `first-fit`; the on-disk `LICENSE` file is never overwritten. If the licence fetch fails (e.g. unrecognised licence identifier), `alter` exits with the API error. -- For `.github/FUNDING.yml`: substitutes `{{GITHUB_USERNAME}}` before writing. `{{GITHUB_USERNAME}}` is resolved at `alter` time from `GET /user`. The Sponsorships checkbox under Settings > General > Features is not exposed via the GitHub API. After alter places `.github/FUNDING.yml`, enable sponsorships manually in repository settings. +- For `.github/FUNDING.yml`: substitutes `{{GITHUB_USERNAME}}` before writing. `{{GITHUB_USERNAME}}` is resolved at `alter` time: in GitHub Actions (`GITHUB_ACTIONS=true`), it is taken from `GITHUB_REPOSITORY_OWNER` without an API call; otherwise it is resolved via `GET /user`. The Sponsorships checkbox under Settings > General > Features is not exposed via the GitHub API. After alter places `.github/FUNDING.yml`, enable sponsorships manually in repository settings. - For `SECURITY.md`: substitutes `{{ADVISORY_URL}}` before writing. `{{ADVISORY_URL}}` is constructed at `alter` time as `https://github.com///security/advisories/new` from the repository context (owner/name). If no GitHub repository context exists (e.g. a brand-new project with no remote), `{{ADVISORY_URL}}` is left unsubstituted in the written file. The unsubstituted token is intentionally detectable by a future `measure` run; `alter` will resolve and substitute it on a subsequent run once the repository has a remote. - For `.github/ISSUE_TEMPLATE/config.yml`: substitutes `{{SUPPORT_URL}}` before writing. `{{SUPPORT_URL}}` is constructed at `alter` time as `https://github.com///blob/HEAD/SUPPORT.md` from the repository context (owner/name). If no GitHub repository context exists, `{{SUPPORT_URL}}` is left unsubstituted in the written file. - For `.tailor.yml`: substitutes `{{HOMEPAGE_URL}}` before writing. `{{HOMEPAGE_URL}}` is constructed at `alter` time as `https://github.com//` from the repository context (owner/name). If no GitHub repository context exists, `{{HOMEPAGE_URL}}` is left unsubstituted in the written file. @@ -291,7 +291,7 @@ would skip (insufficient role: ): update label "documentation" `would create` - label does not exist on GitHub and would be created. `would update` - label exists on GitHub but colour or description differs from config. `no change` - label exists on GitHub and matches config. -`would skip (insufficient scope: )` / `would skip (insufficient role: )` - label operation could not be applied due to token or role constraints. +`would skip (insufficient scope: )` / `would skip (insufficient role: )` - operation could not be applied due to token or role constraints. For labels, this occurs when the token lacks sufficient scope or the user lacks admin role. For repository settings, this occurs when the token lacks administration scope - notably `default_workflow_permissions` and `can_approve_pull_request_reviews` require a PAT (classic `repo` scope or fine-grained with Administration permission); `GITHUB_TOKEN` is always skipped for these two fields. Label entries are sorted: `would create` first, then `would update`, then `no change`, then `would skip` variants. Within each category, sorted lexicographically by label name. @@ -406,7 +406,7 @@ auth: not authenticated ``` Behaviour: -- `user` is resolved via `GET /user` if authenticated; displays `(none)` if not authenticated. +- `user` is resolved via `GET /user` if authenticated (or from `GITHUB_REPOSITORY_OWNER` in GitHub Actions); displays `(none)` if not authenticated. - `repository` displays the `owner/repo` derived from the GitHub remote in the current directory; displays `(none)` if no GitHub remote exists. - `auth` displays `authenticated` or `not authenticated` based on whether a valid token can be resolved for `github.com`. - Does not read `.tailor.yml` and does not require it to be present. @@ -427,7 +427,7 @@ Behaviour: **Not authenticated**: if no valid authentication token can be resolved for `github.com` (neither `GH_TOKEN`/`GITHUB_TOKEN` environment variable, `gh` config file, nor `gh` keyring), `fit`, `alter`, and `baste` exit with: "tailor requires GitHub authentication. Set the GH_TOKEN or GITHUB_TOKEN environment variable, or run `gh auth login`." -**`{{GITHUB_USERNAME}}` resolution failed**: `{{GITHUB_USERNAME}}` is resolved via the GitHub REST API (`GET /user`). If this call fails (e.g. rate limits, network issues), `alter` exits with the API error. Unlike repo-context tokens, `{{GITHUB_USERNAME}}` depends on the authenticated user, not the repository, so it cannot be deferred. +**`{{GITHUB_USERNAME}}` resolution failed**: outside GitHub Actions, `{{GITHUB_USERNAME}}` is resolved via `GET /user`. If this call fails (e.g. rate limits, network issues), `alter` exits with the API error. In GitHub Actions (`GITHUB_ACTIONS=true`), `GITHUB_REPOSITORY_OWNER` is used instead and no API call is made, so this failure path does not apply. Unlike repo-context tokens, `{{GITHUB_USERNAME}}` depends on the authenticated user, not the repository, so it cannot be deferred. **Repo-context tokens unresolved**: `{{ADVISORY_URL}}`, `{{SUPPORT_URL}}`, and `{{HOMEPAGE_URL}}` require a GitHub repository context. If the project has no GitHub remote (e.g. a brand-new project not yet pushed), these tokens are left unsubstituted silently. For `always` swatches (e.g. `SECURITY.md`), `alter` will resolve and substitute them on a subsequent run once the repository has a remote. For `first-fit` swatches (e.g. `.github/ISSUE_TEMPLATE/config.yml`), delete the file and re-run `alter`, or use `--recut`. @@ -435,6 +435,8 @@ Behaviour: **Repository settings API failure**: if any API call to apply repository settings fails (PATCH, PUT, or DELETE), `alter` exits with the API error. Because repository settings are applied first in the execution order, labels, licence, and swatch operations are not attempted. If licence fetch fails after repository settings and labels have been applied, those changes are not reverted. +**Repository settings with insufficient scope**: `default_workflow_permissions` and `can_approve_pull_request_reviews` require administration access that `GITHUB_TOKEN` (a GitHub Actions installation token) does not have. When tailor detects a 403 response from the `GET/PUT /repos/{owner}/{repo}/actions/permissions/workflow` endpoint due to insufficient token scope, it skips these two fields rather than exiting. `baste` reports them as `would skip (insufficient scope: token missing required scope)`. `alter` skips them silently. Other repository settings continue to be applied. To manage these fields from GitHub Actions CI, use a classic PAT with `repo` scope or a fine-grained PAT with the Administration repository permission as the `GH_TOKEN`/`GITHUB_TOKEN` environment variable. + **Unrecognised repository setting**: if `.tailor.yml` contains a field in the `repository` section that is not in the supported settings list, `alter` exits with an error identifying the unrecognised field and listing all valid repository setting field names. **`fit` repository settings query failed**: if `fit` detects a GitHub remote but the subsequent API call to read repository settings fails (e.g. insufficient permissions, network error), `fit` exits with the API error. The user can re-run `fit` after resolving the issue, or create `.tailor.yml` manually. diff --git a/internal/alter/alter_integration_test.go b/internal/alter/alter_integration_test.go index 0ba35bd..94b1c4f 100644 --- a/internal/alter/alter_integration_test.go +++ b/internal/alter/alter_integration_test.go @@ -143,6 +143,7 @@ func WithPatchError(statusCode int) testOption { // returns an alterTestContext ready for use with alter.Run. func setupAlterTest(t *testing.T, configYAML string, opts ...testOption) *alterTestContext { t.Helper() + t.Setenv("GITHUB_ACTIONS", "") // prevent env-var shortcut in FetchUsername sc := &alterServerConfig{ username: "testuser", diff --git a/internal/alter/settings.go b/internal/alter/settings.go index a709c39..ba419fd 100644 --- a/internal/alter/settings.go +++ b/internal/alter/settings.go @@ -164,8 +164,19 @@ func compareSettings(declared, live *model.RepositorySettings) []RepoSettingResu // readWarningOperationFields maps read-path operation names (from // ErrInsufficientScope/ErrInsufficientRole) to the config field names // (YAML tags) they affect. Workflow permissions covers two fields. +// The installation token entry covers fields that return zero values +// in GitHub Actions. var readWarningOperationFields = map[string][]string{ "fetch workflow permissions": {"default_workflow_permissions", "can_approve_pull_request_reviews"}, + gh.InstallationTokenReadOp: { + "allow_auto_merge", + "allow_rebase_merge", + "allow_squash_merge", + "allow_update_branch", + "delete_branch_on_merge", + "squash_merge_commit_message", + "squash_merge_commit_title", + }, } // readWarningsToResults converts read-path access-error warnings into diff --git a/internal/alter/settings_test.go b/internal/alter/settings_test.go index 9011c4b..20fa387 100644 --- a/internal/alter/settings_test.go +++ b/internal/alter/settings_test.go @@ -11,6 +11,7 @@ import ( "github.com/wimpysworld/tailor/internal/alter" "github.com/wimpysworld/tailor/internal/config" + "github.com/wimpysworld/tailor/internal/gh" "github.com/wimpysworld/tailor/internal/ghfake" "github.com/wimpysworld/tailor/internal/model" "github.com/wimpysworld/tailor/internal/ptr" @@ -41,12 +42,26 @@ type repoJSON struct { } // settingsServer creates an httptest server that responds to repo settings -// GET and PATCH requests. patchCalled is incremented when PATCH is received. +// GET, PATCH, and PUT requests, plus a GET /user endpoint (returning 200 by +// default to simulate a PAT). patchCalled is incremented when PATCH or PUT +// is received. func settingsServer(repo repoJSON, patchCalled *atomic.Int32) *httptest.Server { + return settingsServerWithTokenType(repo, patchCalled, false) +} + +func settingsServerWithTokenType(repo repoJSON, patchCalled *atomic.Int32, installationToken bool) *httptest.Server { return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { path := r.URL.Path switch { + case r.Method == http.MethodGet && path == "/user": + if installationToken { + w.WriteHeader(http.StatusForbidden) + fmt.Fprint(w, `{"message":"Resource not accessible by integration"}`) + } else { + fmt.Fprint(w, `{"login":"testuser"}`) + } + case r.Method == http.MethodGet && path == "/repos/testowner/testrepo": w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(repo) @@ -768,6 +783,119 @@ func TestProcessRepoSettingsReadPath403DoesNotProduceWouldSet(t *testing.T) { } } +func TestProcessRepoSettingsInstallationTokenSkipsUnreliableFields(t *testing.T) { + ghfake.FakeRepo(t, "testowner", "testrepo") + + // Simulate GitHub Actions environment with an installation token. + t.Setenv("GITHUB_ACTIONS", "true") + gh.ResetTokenProbe() + t.Cleanup(gh.ResetTokenProbe) + + // Live repo returns zero values for the unreliable fields (as the + // installation token does in practice). The config declares non-zero + // values. Without the fix these would all be WouldSet. + live := repoJSON{ + AllowAutoMerge: false, + AllowRebaseMerge: false, + AllowSquashMerge: false, + AllowUpdateBranch: false, + DeleteBranchOnMerge: false, + SquashMergeCommitTitle: "", + SquashMergeCommitMessage: "", + // Reliable field that genuinely differs. + HasWiki: true, + } + server := settingsServerWithTokenType(live, nil, true) + t.Cleanup(server.Close) + client := testutil.NewTestClient(t, server) + + cfg := &config.Config{ + Repository: &model.RepositorySettings{ + AllowAutoMerge: ptr.Ptr(true), + AllowRebaseMerge: ptr.Ptr(false), + AllowSquashMerge: ptr.Ptr(true), + AllowUpdateBranch: ptr.Ptr(true), + DeleteBranchOnMerge: ptr.Ptr(true), + SquashMergeCommitTitle: ptr.Ptr("PR_TITLE"), + SquashMergeCommitMessage: ptr.Ptr("COMMIT_MESSAGES"), + HasWiki: ptr.Ptr(false), // reliable field, differs + }, + } + + results, err := alter.ProcessRepoSettings(cfg, alter.DryRun, client, "testowner", "testrepo", true) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + unreliableFields := map[string]bool{ + "allow_auto_merge": true, + "allow_rebase_merge": true, + "allow_squash_merge": true, + "allow_update_branch": true, + "delete_branch_on_merge": true, + "squash_merge_commit_title": true, + "squash_merge_commit_message": true, + } + + for _, r := range results { + if unreliableFields[r.Field] { + if r.Category != alter.WouldSkipScope { + t.Errorf("field %q: category = %q, want %q", r.Field, r.Category, alter.WouldSkipScope) + } + } + if r.Field == "has_wiki" { + if r.Category != alter.WouldSet { + t.Errorf("has_wiki: category = %q, want %q", r.Category, alter.WouldSet) + } + } + } + + // Verify no WouldSet for unreliable fields. + for _, r := range results { + if unreliableFields[r.Field] && r.Category == alter.WouldSet { + t.Errorf("field %q should not be WouldSet under installation token", r.Field) + } + } +} + +func TestProcessRepoSettingsInstallationTokenNotActiveOutsideCI(t *testing.T) { + ghfake.FakeRepo(t, "testowner", "testrepo") + + // Ensure GITHUB_ACTIONS is not set. + t.Setenv("GITHUB_ACTIONS", "") + gh.ResetTokenProbe() + t.Cleanup(gh.ResetTokenProbe) + + live := repoJSON{ + AllowAutoMerge: false, + HasWiki: true, + } + server := settingsServer(live, nil) + t.Cleanup(server.Close) + client := testutil.NewTestClient(t, server) + + cfg := &config.Config{ + Repository: &model.RepositorySettings{ + AllowAutoMerge: ptr.Ptr(true), + }, + } + + results, err := alter.ProcessRepoSettings(cfg, alter.DryRun, client, "testowner", "testrepo", true) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(results) != 1 { + t.Fatalf("got %d results, want 1", len(results)) + } + if results[0].Field != "allow_auto_merge" { + t.Errorf("field = %q, want %q", results[0].Field, "allow_auto_merge") + } + // Outside CI, the field should produce WouldSet as before. + if results[0].Category != alter.WouldSet { + t.Errorf("category = %q, want %q", results[0].Category, alter.WouldSet) + } +} + func TestProcessRepoSettingsCanApprovePullRequestReviewsWouldSet(t *testing.T) { ghfake.FakeRepo(t, "testowner", "testrepo") diff --git a/internal/docket/docket_test.go b/internal/docket/docket_test.go index a37f488..2ee25fe 100644 --- a/internal/docket/docket_test.go +++ b/internal/docket/docket_test.go @@ -25,6 +25,7 @@ type docketTestOpts struct { // httptest server, and returns a *api.RESTClient (nil when token is empty). func setupDocketTest(t *testing.T, opts docketTestOpts) *api.RESTClient { t.Helper() + t.Setenv("GITHUB_ACTIONS", "") // prevent env-var shortcut in FetchUsername ghfake.FakeAuth(t, opts.token) if opts.repoOwner != "" { ghfake.FakeRepo(t, opts.repoOwner, opts.repoName) diff --git a/internal/gh/settings.go b/internal/gh/settings.go index 957bfb6..47bf5c6 100644 --- a/internal/gh/settings.go +++ b/internal/gh/settings.go @@ -12,6 +12,26 @@ import ( "github.com/wimpysworld/tailor/internal/ptr" ) +// installationTokenUnreliableFields lists repo response fields that GitHub +// Actions installation tokens (GITHUB_TOKEN / secrets.GITHUB_TOKEN) return as +// zero values (false / empty string) regardless of the actual repository +// configuration. Comparing these against the user's config produces false +// positives ("would set" when the repo is already correct). +// +// The operation name is used as the key in readWarningOperationFields +// (internal/alter/settings.go) to suppress WouldSet entries for these fields. +const InstallationTokenReadOp = "read repo settings (installation token)" //nolint:gosec // not a credential + +var installationTokenUnreliableFields = map[string]bool{ + "allow_auto_merge": true, + "allow_rebase_merge": true, + "allow_squash_merge": true, + "allow_update_branch": true, + "delete_branch_on_merge": true, + "squash_merge_commit_message": true, + "squash_merge_commit_title": true, +} + // repoResponse holds the subset of GitHub repository fields we read. type repoResponse struct { Description string `json:"description"` @@ -75,10 +95,19 @@ func ReadRepoSettings(client *api.RESTClient, owner, name string) (*model.Reposi WebCommitSignoffRequired: ptr.Ptr(repo.WebCommitSignoffRequired), } - // Each sub-call below uses classifyHTTPError to detect 403 responses. - // On scope/role errors the corresponding field stays nil (unknown), - // and the classified error is appended to warnings for the caller. + // When using a GitHub Actions installation token, the API returns zero + // values for certain fields. Nil them out and emit a synthetic warning + // so the comparison layer skips them instead of producing false diffs. + // IsInstallationToken probes GET /user to distinguish installation + // tokens from PATs; the result is cached per process. var warnings []error + if IsInstallationToken(client) { + nilUnreliableFields(s) + warnings = append(warnings, &ErrInsufficientScope{ + Operation: InstallationTokenReadOp, + Message: "installation token returns unreliable values for merge/branch settings", + }) + } var wfPerms workflowPermissionsResponse if err := client.Get(fmt.Sprintf("repos/%s/%s/actions/permissions/workflow", owner, name), &wfPerms); err != nil { @@ -269,3 +298,22 @@ func buildSettingsPayload(settings *model.RepositorySettings) settingsPayload { return p } + +// nilUnreliableFields sets pointer fields in s to nil when their YAML tag +// matches installationTokenUnreliableFields. This prevents false-positive +// diffs when the API returns zero values instead of the actual configuration. +func nilUnreliableFields(s *model.RepositorySettings) { + rv := reflect.ValueOf(s).Elem() + rt := rv.Type() + for i := range rt.NumField() { + f := rt.Field(i) + tag := f.Tag.Get("yaml") + if tag == "" || tag == ",inline" { + continue + } + key, _, _ := strings.Cut(tag, ",") + if installationTokenUnreliableFields[key] { + rv.Field(i).Set(reflect.Zero(f.Type)) + } + } +} diff --git a/internal/gh/settings_test.go b/internal/gh/settings_test.go index df17371..01c9f9d 100644 --- a/internal/gh/settings_test.go +++ b/internal/gh/settings_test.go @@ -68,6 +68,9 @@ const ( ) func TestReadRepoSettings(t *testing.T) { + ResetTokenProbe() + t.Cleanup(ResetTokenProbe) + tests := []struct { name string repoJSON string @@ -235,6 +238,9 @@ func TestReadRepoSettings(t *testing.T) { } func TestReadRepoSettingsRepoAPIError(t *testing.T) { + ResetTokenProbe() + t.Cleanup(ResetTokenProbe) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) fmt.Fprint(w, `{"message": "Not Found"}`) @@ -249,6 +255,9 @@ func TestReadRepoSettingsRepoAPIError(t *testing.T) { } func TestReadRepoSettingsWFPerms403GracefulDegradation(t *testing.T) { + ResetTokenProbe() + t.Cleanup(ResetTokenProbe) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/repos/testowner/testrepo": @@ -280,8 +289,14 @@ func TestReadRepoSettingsWFPerms403GracefulDegradation(t *testing.T) { } func TestReadRepoSettingsAll403GracefulDegradation(t *testing.T) { + ResetTokenProbe() + t.Cleanup(ResetTokenProbe) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { + case "/user": + // Return 200 so IsInstallationToken detects a PAT. + fmt.Fprint(w, `{"login": "testuser"}`) case "/repos/testowner/testrepo": fmt.Fprint(w, fullRepoJSON) default: @@ -314,6 +329,9 @@ func TestReadRepoSettingsAll403GracefulDegradation(t *testing.T) { } func TestReadRepoSettingsNon403StillFails(t *testing.T) { + ResetTokenProbe() + t.Cleanup(ResetTokenProbe) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/repos/testowner/testrepo": diff --git a/internal/gh/token.go b/internal/gh/token.go new file mode 100644 index 0000000..fadfbb0 --- /dev/null +++ b/internal/gh/token.go @@ -0,0 +1,53 @@ +package gh + +import ( + "errors" + "net/http" + "os" + "sync" + + "github.com/cli/go-gh/v2/pkg/api" +) + +// tokenProbeResult caches the outcome of probing GET /user to distinguish +// installation tokens from PATs. Installation tokens (GITHUB_TOKEN in Actions) +// cannot call user-scoped endpoints and return 403. +type tokenProbeResult struct { + once sync.Once + isInstallation bool +} + +var tokenProbe tokenProbeResult + +// ResetTokenProbe clears the cached probe result. Intended for tests only. +func ResetTokenProbe() { + tokenProbe = tokenProbeResult{} +} + +// IsInstallationToken returns true when the token associated with client +// appears to be a GitHub Actions installation token. Detection works by +// calling GET /user: installation tokens receive 403, PATs succeed. +// +// Outside GitHub Actions (GITHUB_ACTIONS != "true") this always returns false +// without making an API call, preserving local-run behaviour. +// +// The result is cached for the lifetime of the process. +func IsInstallationToken(client *api.RESTClient) bool { + if os.Getenv("GITHUB_ACTIONS") != "true" { + return false + } + + tokenProbe.once.Do(func() { + var resp userResponse + if err := client.Get("user", &resp); err != nil { + var httpErr *api.HTTPError + if errors.As(err, &httpErr) && httpErr.StatusCode == http.StatusForbidden { + tokenProbe.isInstallation = true + } + // Other errors (network, 401, etc.) leave isInstallation false, + // which avoids suppressing fields unnecessarily. + } + }) + + return tokenProbe.isInstallation +} diff --git a/internal/gh/user.go b/internal/gh/user.go index a3949cc..4380b2d 100644 --- a/internal/gh/user.go +++ b/internal/gh/user.go @@ -1,6 +1,8 @@ package gh import ( + "os" + "github.com/cli/go-gh/v2/pkg/api" ) @@ -10,9 +12,18 @@ type userResponse struct { } // FetchUsername returns the authenticated user's login via GET /user. +// When running in GitHub Actions with an installation token (detected by +// probing GET /user for a 403), it falls back to GITHUB_REPOSITORY_OWNER. func FetchUsername(client *api.RESTClient) (string, error) { var resp userResponse if err := client.Get("user", &resp); err != nil { + // In GitHub Actions with an installation token, GET /user returns 403. + // Fall back to the environment variable. + if os.Getenv("GITHUB_ACTIONS") == "true" { + if owner := os.Getenv("GITHUB_REPOSITORY_OWNER"); owner != "" { + return owner, nil + } + } return "", err } return resp.Login, nil diff --git a/internal/gh/user_test.go b/internal/gh/user_test.go index 86804c6..7ae8c03 100644 --- a/internal/gh/user_test.go +++ b/internal/gh/user_test.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "sync/atomic" "testing" ) @@ -29,6 +30,7 @@ func TestFetchUsernameSuccess(t *testing.T) { } func TestFetchUsernameAPIError(t *testing.T) { + t.Setenv("GITHUB_ACTIONS", "") server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusUnauthorized) fmt.Fprint(w, `{"message": "Bad credentials"}`) @@ -41,3 +43,103 @@ func TestFetchUsernameAPIError(t *testing.T) { t.Fatal("FetchUsername() expected error, got nil") } } + +func TestFetchUsernameGitHubActionsFallback(t *testing.T) { + // Simulate installation token: GET /user returns 403. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + fmt.Fprint(w, `{"message": "Resource not accessible by integration"}`) + })) + t.Cleanup(server.Close) + + t.Setenv("GITHUB_ACTIONS", "true") + t.Setenv("GITHUB_REPOSITORY_OWNER", "testowner") + + client := newTestClient(t, server) + username, err := FetchUsername(client) + if err != nil { + t.Fatalf("FetchUsername() error: %v", err) + } + + if username != "testowner" { + t.Errorf("username = %q, want %q", username, "testowner") + } +} + +func TestFetchUsernameGitHubActionsPATUsesAPI(t *testing.T) { + // PAT in GitHub Actions: GET /user succeeds. + var requestCount atomic.Int64 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount.Add(1) + if r.URL.Path != "/user" { + http.NotFound(w, r) + return + } + fmt.Fprint(w, `{"login": "patuser"}`) + })) + t.Cleanup(server.Close) + + t.Setenv("GITHUB_ACTIONS", "true") + t.Setenv("GITHUB_REPOSITORY_OWNER", "testowner") + + client := newTestClient(t, server) + username, err := FetchUsername(client) + if err != nil { + t.Fatalf("FetchUsername() error: %v", err) + } + + if username != "patuser" { + t.Errorf("username = %q, want %q", username, "patuser") + } + + if n := requestCount.Load(); n == 0 { + t.Error("expected at least one HTTP request, got zero") + } +} + +func TestFetchUsernameGitHubActionsNoOwner(t *testing.T) { + // Installation token with no GITHUB_REPOSITORY_OWNER: error propagated. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusForbidden) + fmt.Fprint(w, `{"message": "Resource not accessible by integration"}`) + })) + t.Cleanup(server.Close) + + t.Setenv("GITHUB_ACTIONS", "true") + t.Setenv("GITHUB_REPOSITORY_OWNER", "") + + client := newTestClient(t, server) + _, err := FetchUsername(client) + if err == nil { + t.Fatal("FetchUsername() expected error when GITHUB_REPOSITORY_OWNER is empty, got nil") + } +} + +func TestFetchUsernameNotGitHubActions(t *testing.T) { + var requestCount atomic.Int64 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount.Add(1) + if r.URL.Path != "/user" { + http.NotFound(w, r) + return + } + fmt.Fprint(w, `{"login": "apiuser"}`) + })) + t.Cleanup(server.Close) + + t.Setenv("GITHUB_ACTIONS", "") + + client := newTestClient(t, server) + username, err := FetchUsername(client) + if err != nil { + t.Fatalf("FetchUsername() error: %v", err) + } + + if username != "apiuser" { + t.Errorf("username = %q, want %q", username, "apiuser") + } + + if n := requestCount.Load(); n == 0 { + t.Error("expected at least one HTTP request, got zero") + } +} diff --git a/swatches/.github/workflows/tailor.yml b/swatches/.github/workflows/tailor.yml index 807b98a..d482351 100644 --- a/swatches/.github/workflows/tailor.yml +++ b/swatches/.github/workflows/tailor.yml @@ -14,7 +14,7 @@ jobs: alter: runs-on: ubuntu-slim env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.TAILOR_TOKEN || secrets.GITHUB_TOKEN }} steps: - uses: actions/checkout@v6