feat(cli): belt the role panes' git pushes and gh

AGENTS.md

@@ -89,7 +89,7 @@ Never report a step as done before `mise run check` has passed, and say which ga
 - **No model calls in tests.** Unit and integration tests run a stand-in `copilot` (a script on `PATH`, such as `testdata/copilot`, which plays each role in the walking skeleton), never the real one.
 - **tmux on its own socket.** Every tmux server that a test, a smoke run or a live check starts gets a config file of its own (`-f /dev/null`, or a file of the test's), so the user's `~/.tmux.conf` and its plugins never load, and runs without `TMUX` and `TMUX_PANE` (`env -u TMUX -u TMUX_PANE`), so a slip can't reach the user's server. Tests run tmux with `-L baton-test-<n>`, in a `TMUX_TMPDIR` and a `HOME` of their own, and kill only their servers. The walking skeleton (`TestSkeleton`) runs baton itself, so it runs tmux as a smoke run does, on the default socket, in a `TMUX_TMPDIR` (`/tmp/baton-test-skeleton-*`) and a `HOME` of its own. Smoke runs and live checks use a socket of their own, such as `-L baton-live`. baton itself uses tmux's default socket, or `TMUX`'s, so a smoke run of the binary gets a `TMUX_TMPDIR` and a `HOME` of its own, and starts that server first, with `-f /dev/null`; it never runs on the user's default server without the user's OK. The tmux tests' `TestMain` ends every server whose socket is in its `TMUX_TMPDIR`, even one that a test started on another socket, before it removes that folder. A test run that fails or stops early skips its cleanup, so after one, look for leftover servers (`pgrep -lf '^tmux .*(baton-test-|/Test)'`: a test's server shows its `-L baton-test-<n>`, or its config file in the test's temporary folder), end each on its own socket, or with `kill <pid>` once its socket is gone, and remove leftover `baton-tmux-<n>` folders in `/tmp` and `baton-test-skeleton-*` folders in `/tmp` and `$TMPDIR`. Never kill the user's tmux server, and never touch a session or window that baton didn't create.
 - **git without the user's config.** Tests run git, theirs and baton's, with `GIT_CONFIG_GLOBAL=/dev/null`, `GIT_CONFIG_NOSYSTEM=1`, `LC_ALL=C` and a test author, and without the `GIT_*` variables that a hook of git's sets, such as `GIT_DIR`: each package's `TestMain` sets them. A test spells out git's arguments, so it runs no command that it doesn't show.
-- **baton without the user's config.** Tests run baton with a `HOME` of their own and without `XDG_CONFIG_HOME` and `XDG_CACHE_HOME`, so it never reads the user's config file or writes in their cache: the cli tests' `TestMain` gives them an empty `HOME`, a `baton-test-home-*` folder in `$TMPDIR`, and a test that writes there, such as a config file, gives itself one of its own first. `TestMain` fails the run if a test left files in that folder. A run that stops early, such as one whose test panics, leaves the folder, so remove it after one.
+- **baton without the user's config.** Tests run baton with a `HOME` of their own and without `XDG_CONFIG_HOME` and `XDG_CACHE_HOME`, so it never reads the user's config file or writes in their cache: the cli tests' `TestMain` gives them an empty `HOME`, a `baton-test-home-*` folder in `$TMPDIR`, and a test that writes there, such as a config file, gives itself one of its own first. `TestMain` fails the run if a test left files in that folder. A run that stops early, such as one whose test panics, leaves the folder, so remove it after one. That `TestMain` also clears the GitHub tokens (`GH_TOKEN`, `GITHUB_TOKEN`, their enterprise forms and `COPILOT_GITHUB_TOKEN`), so that no test hands the user's token to a stand-in.
 - **Copilot CLI config stays untouched.** Develop with `--plugin-dir`, flags or a throwaway `COPILOT_HOME`, never by changing `~/.copilot/`.
 - **Live checks are rare and small**: a throwaway repo in the spikes folder, the cheapest model that can do the job, and a `--max-ai-credits` cap (30 at least, the CLI's minimum). The PR records each live check's cost. On a Mac, a live check's `copilot` keeps the user's `HOME`, whose login keychain holds its login, and gets a throwaway `COPILOT_HOME`; its tmux server and the commands sent to it run in a clean environment (`env -i`), since a pane takes the `PATH` of the client that makes it (fact 30 in the plan).
 

README.md

@@ -9,7 +9,7 @@ You talk to one agent, the orchestrator. It reads the ticket, plans the work as
 ## Requirements
 
 - macOS, git 2.36 or newer, and tmux 3.5 or newer.
-- [GitHub Copilot CLI](https://docs.github.com/copilot/how-tos/copilot-cli).
+- [GitHub Copilot CLI](https://docs.github.com/copilot/how-tos/copilot-cli), logged in with `copilot login`, or with a token in `COPILOT_GITHUB_TOKEN`, `GH_TOKEN` or `GITHUB_TOKEN`: `gh` doesn't work in the roles' panes, so their `copilot` can't take its login from `gh`.
 - `gh` for GitHub pull requests; a Bitbucket Cloud API token for Bitbucket pull requests; a read-only Jira API token for Jira tickets.
 
 ## Config

internal/cli/belt.go

@@ -0,0 +1,156 @@
+package cli
+
+import (
+	"cmp"
+	"context"
+	"fmt"
+	"os"
+	"slices"
+	"strconv"
+	"strings"
+
+	"github.com/codedogapp/baton/internal/repo"
+)
+
+// The belt is a role's second belt, after baton's guard: settings in the environment of the role's
+// copilot, which every command and script there inherits, so that git's pushes out of the machine
+// and gh fail, even where the guard can't see them (decision 55).
+
+// blocked is the remote helper that git's push rules send a push to: one that doesn't exist, so
+// that git refuses the push, with no network, and names baton's belt (fact 38).
+const blocked = "baton-blocks-push::"
+
+// outward are the characters that start a URL that leaves the machine, such as https://host/repo,
+// host:repo or [::1]:repo, but no absolute path, and no ./ or ../ path (fact 38).
+const outward = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789["
+
+// ghBlocked is gh's config folder in a role's pane: one under a file, which can't be there, so that
+// gh fails before it starts, its auth commands too, and never reaches the user's login (fact 38).
+const ghBlocked = "/dev/null/baton-blocks-gh"
+
+// tokens are the variables that give gh and other tools a GitHub token, which a role's pane goes
+// without (decision 55).
+var tokens = []string{"GH_TOKEN", "GITHUB_TOKEN", "GH_ENTERPRISE_TOKEN", "GITHUB_ENTERPRISE_TOKEN"}
+
+// setting is a setting of git's, a name and its value.
+type setting struct{ key, value string }
+
+// belted returns a role's environment for copilot, from the folder: baton's, with the variables,
+// and with the belt (decision 55).
+func belted(ctx context.Context, dir string, vars ...string) ([]string, error) {
+	count, err := gitCount()
+	if err != nil {
+		return nil, err
+	}
+
+	rules, err := pushRules(ctx, dir)
+	if err != nil {
+		return nil, err
+	}
+
+	vars = slices.Concat(vars, gitVars(count, rules), ghVars())
+
+	return slices.DeleteFunc(environment(vars...), func(variable string) bool {
+		name, _, _ := strings.Cut(variable, "=")
+
+		return slices.Contains(tokens, name)
+	}), nil
+}
+
+// gitCount returns the count of git's settings in baton's environment, which the belt's follow. It
+// refuses one that git reads but Go doesn't, such as " 1"; with a negative one, git fails itself.
+func gitCount() (int, error) {
+	text := os.Getenv("GIT_CONFIG_COUNT")
+	if text == "" {
+		return 0, nil
+	}
+
+	count, err := strconv.Atoi(text)
+	if err != nil {
+		return 0, fmt.Errorf("GIT_CONFIG_COUNT=%q isn't a count of git's settings", text)
+	}
+
+	return count, nil
+}
+
+// pushRules returns git's settings that block a push out of the machine, from the folder: a
+// pushInsteadOf rule for each character that starts such a URL, and one that keeps file:// URLs
+// local; one for each remote's url that leaves, which beats the user's own shorter rules; and for
+// each remote's pushurl that leaves, which those rules skip, an empty pushurl, which clears the
+// remote's pushurls, so that the push goes to its url, or, in a git that can't clear them, an
+// insteadOf rule, which blocks fetches from URLs that start with the pushurl too (fact 38).
+func pushRules(ctx context.Context, dir string) ([]setting, error) {
+	urls, err := repo.RemoteURLs(ctx, dir)
+	if err != nil {
+		return nil, err
+	}
+
+	clears, err := repo.ClearsPushURLs(ctx)
+	if err != nil {
+		return nil, err
+	}
+
+	rules := make([]setting, 0, len(outward)+1+len(urls))
+	for _, char := range outward {
+		rules = append(rules, blockPush(string(char)))
+	}
+
+	rules = append(rules, setting{"url.file://.pushInsteadOf", "file://"})
+
+	for _, url := range urls {
+		switch {
+		case !leaves(url.URL):
+		case !url.Push:
+			rules = append(rules, blockPush(url.URL))
+		case !clears:
+			rules = append(rules, setting{"url." + blocked + url.URL + ".insteadOf", url.URL})
+		case !slices.Contains(rules, setting{"remote." + url.Remote + ".pushurl", ""}):
+			rules = append(rules, setting{"remote." + url.Remote + ".pushurl", ""})
+		}
+	}
+
+	return rules, nil
+}
+
+// blockPush returns git's rule that sends a push to a URL that starts with the prefix to the
+// blocked remote helper.
+func blockPush(prefix string) setting {
+	return setting{"url." + blocked + prefix + ".pushInsteadOf", prefix}
+}
+
+// leaves reports whether git takes the URL for one that leaves the machine: one with a colon before
+// any slash, as git reads it, but no file:// URL (fact 38).
+func leaves(url string) bool {
+	before, _, found := strings.Cut(url, ":")
+
+	return found && !strings.Contains(before, "/") && !strings.HasPrefix(url, "file://")
+}
+
+// gitVars returns the variables that give git the settings, after the count of those in baton's
+// environment.
+func gitVars(count int, settings []setting) []string {
+	vars := make([]string, 0, 2*len(settings)+1)
+	for i, each := range settings {
+		vars = append(vars, fmt.Sprintf("GIT_CONFIG_KEY_%d=%s", count+i, each.key),
+			fmt.Sprintf("GIT_CONFIG_VALUE_%d=%s", count+i, each.value))
+	}
+
+	return append(vars, "GIT_CONFIG_COUNT="+strconv.Itoa(count+len(settings)))
+}
+
+// ghVars returns the variables that block gh, and that give copilot the token that it would have
+// taken from GH_TOKEN or GITHUB_TOKEN, which the role's pane goes without, unless it has its own
+// (fact 38).
+func ghVars() []string {
+	vars := []string{"GH_CONFIG_DIR=" + ghBlocked}
+
+	if os.Getenv("COPILOT_GITHUB_TOKEN") != "" {
+		return vars
+	}
+
+	if token := cmp.Or(os.Getenv("GH_TOKEN"), os.Getenv("GITHUB_TOKEN")); token != "" {
+		vars = append(vars, "COPILOT_GITHUB_TOKEN="+token)
+	}
+
+	return vars
+}

internal/cli/belt_env_test.go

@@ -0,0 +1,176 @@
+package cli_test
+
+import (
+	"fmt"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+	"testing/synctest"
+
+	"github.com/codedogapp/baton/internal/flow"
+	"github.com/codedogapp/baton/internal/state"
+)
+
+// outward are the characters that start a URL that leaves the machine, in baton's push rules
+// (fact 38).
+const outward = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789["
+
+// belt returns the variables that block a role's pushes and gh, after the count of git's settings
+// in baton's environment, with git's settings for the repo's remotes: a push rule for each
+// character that starts a URL that leaves the machine, one that keeps file:// URLs local, then
+// the remotes'; and gh's config folder, which can't be there (decision 55).
+func belt(count int, remotes ...[2]string) []string {
+	rules := make([][2]string, 0, len(outward)+1+len(remotes))
+	for _, char := range outward {
+		rules = append(rules, pushRule(string(char)))
+	}
+
+	rules = append(rules, [2]string{"url.file://.pushInsteadOf", "file://"})
+	rules = append(rules, remotes...)
+
+	vars := make([]string, 0, 2*len(rules)+2)
+	for i, rule := range rules {
+		vars = append(vars, fmt.Sprintf("GIT_CONFIG_KEY_%d=%s", count+i, rule[0]),
+			fmt.Sprintf("GIT_CONFIG_VALUE_%d=%s", count+i, rule[1]))
+	}
+
+	return append(vars, fmt.Sprintf("GIT_CONFIG_COUNT=%d", count+len(rules)),
+		"GH_CONFIG_DIR=/dev/null/baton-blocks-gh")
+}
+
+// pushRule returns git's setting, and its value, that sends a push to a URL that starts with the
+// prefix to baton's missing remote helper.
+func pushRule(prefix string) [2]string {
+	return [2]string{"url.baton-blocks-push::" + prefix + ".pushInsteadOf", prefix}
+}
+
+// TestRolePaneAddsToGitsSettings has the lead's copilot get the belt's settings after git's own in
+// baton's environment, in place of those past their count: a push rule for each remote's url that
+// leaves the machine, and for the pushurls that leave, one empty pushurl for their remote, in a
+// git that clears pushurls, as 2.46 does, or else a rule for each; none for local URLs (decision
+// 55).
+func TestRolePaneAddsToGitsSettings(t *testing.T) {
+	cases := []struct {
+		version string
+		pushURL [][2]string // The belt's settings for origin's pushurls.
+	}{
+		{version: "git version 2.46.0", pushURL: [][2]string{{"remote.origin.pushurl", ""}}},
+		{version: "git version 2.45.2", pushURL: [][2]string{
+			{
+				"url.baton-blocks-push::git@example.invalid:one.git.insteadOf",
+				"git@example.invalid:one.git",
+			},
+			{
+				"url.baton-blocks-push::git@example.invalid:two.git.insteadOf",
+				"git@example.invalid:two.git",
+			},
+		}},
+	}
+
+	for _, test := range cases {
+		t.Run(test.version, func(t *testing.T) {
+			copilot, cache := standIn(t)
+			gitBut(t, "version) echo '"+test.version+"'; exit ;;")
+			setenv(t, "GIT_CONFIG_COUNT=1", "GIT_CONFIG_KEY_0=user.name", "GIT_CONFIG_VALUE_0=user",
+				"GIT_CONFIG_KEY_1=core.editor", "GIT_CONFIG_VALUE_1=vi")
+
+			mainDir, linked := repos(t)
+			write(t, filepath.Join(mainDir, ".git"), "remotes", `[remote "origin"]
+	url = https://example.invalid/demo.git
+	pushurl = git@example.invalid:one.git
+	pushurl = git@example.invalid:two.git
+[remote "local"]
+	url = ../local.git
+	pushurl = /srv/local.git
+[remote "sibling"]
+	url = sibling.git
+`)
+			gitIn(t, mainDir, exec.CommandContext(t.Context(), "git", "config", "include.path",
+				"remotes"))
+
+			store := started(t, mainDir, fresh())
+
+			remotes := append([][2]string{pushRule("https://example.invalid/demo.git")},
+				test.pushURL...)
+
+			synctest.Test(t, func(t *testing.T) {
+				env, _ := inPane(t, linked, paneOf(flow.RoleLead))
+
+				wantCopilot(t, paneFor(t, env), flow.RoleLead, execed{
+					path: copilot,
+					args: copilotLine(flow.RoleLead, pluginIn(t, cache), leadsBrief(store)),
+					env:  roleEnv(belt(1, remotes...)...),
+				})
+			})
+		})
+	}
+}
+
+// TestRolePaneMovesTheGitHubToken has the lead's copilot go without the GitHub tokens that gh and
+// other tools read, and get the one that copilot would have read from them, GH_TOKEN's, or else
+// GITHUB_TOKEN's, as its own, unless it has one (fact 38).
+func TestRolePaneMovesTheGitHubToken(t *testing.T) {
+	cases := []struct {
+		name    string
+		tokens  []string // The user's variables.
+		copilot []string // Copilot's own token, if baton gives it one.
+	}{
+		{
+			name: "every token",
+			tokens: []string{
+				"GH_TOKEN=gh", "GITHUB_TOKEN=github", "GH_ENTERPRISE_TOKEN=ghe",
+				"GITHUB_ENTERPRISE_TOKEN=github-enterprise",
+			},
+			copilot: []string{"COPILOT_GITHUB_TOKEN=gh"},
+		},
+		{
+			name:    "GITHUB_TOKEN's",
+			tokens:  []string{"GH_TOKEN=", "GITHUB_TOKEN=github"},
+			copilot: []string{"COPILOT_GITHUB_TOKEN=github"},
+		},
+		{name: "copilot's own", tokens: []string{"COPILOT_GITHUB_TOKEN=copilot", "GH_TOKEN=gh"}},
+		{
+			name:    "copilot's own, empty",
+			tokens:  []string{"COPILOT_GITHUB_TOKEN=", "GH_TOKEN=gh"},
+			copilot: []string{"COPILOT_GITHUB_TOKEN=gh"},
+		},
+		{name: "an enterprise's", tokens: []string{"GH_ENTERPRISE_TOKEN=ghe"}},
+	}
+
+	for _, test := range cases {
+		t.Run(test.name, func(t *testing.T) {
+			copilot, cache := standIn(t)
+			setenv(t, test.tokens...)
+
+			mainDir, linked := repos(t)
+			store := started(t, mainDir, fresh())
+
+			synctest.Test(t, func(t *testing.T) {
+				env, _ := inPane(t, linked, paneOf(flow.RoleLead))
+
+				wantCopilot(t, paneFor(t, env), flow.RoleLead, execed{
+					path: copilot,
+					args: copilotLine(flow.RoleLead, pluginIn(t, cache), leadsBrief(store)),
+					env:  roleEnv(append(belt(0), test.copilot...)...),
+				})
+			})
+		})
+	}
+}
+
+// setenv sets the variables, each one name=value, for the test.
+func setenv(t *testing.T, vars ...string) {
+	t.Helper()
+
+	for _, variable := range vars {
+		name, value, _ := strings.Cut(variable, "=")
+		t.Setenv(name, value)
+	}
+}
+
+// leadsBrief returns the lead's first prompt in a new feature, with the orchestrator's brief.
+func leadsBrief(store state.Store) string {
+	return "Picked up demo-1 as lead: round 0 of 3, from orchestrator.\nNote: " +
+		notePath(store, "01-orchestrator-to-lead.md") + "\n"
+}