From 8d68f2d305e0f156cbdfcf9dc36d2b06b01fc4b8 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Thu, 22 Oct 2026 09:44:33 -0400 Subject: [PATCH 1/2] Add failing tests for Codex rules that let reads leave the sandbox Codex runs every hey command in a sandbox that blocks the keyring and the network, so each one fails and the skill sends the user to log in again. These tests pin the fix: hey setup codex installs a managed hey.rules of read-only prefix rules, refuses a file it did not write, doctor reports it, removal deletes it, and the skill stops advising a login loop. --- internal/cmd/setup_codex_rules_test.go | 305 +++++++++++++++++++++++++ skills/embed_test.go | 32 ++- 2 files changed, 328 insertions(+), 9 deletions(-) create mode 100644 internal/cmd/setup_codex_rules_test.go diff --git a/internal/cmd/setup_codex_rules_test.go b/internal/cmd/setup_codex_rules_test.go new file mode 100644 index 00000000..6dbb2327 --- /dev/null +++ b/internal/cmd/setup_codex_rules_test.go @@ -0,0 +1,305 @@ +package cmd + +import ( + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "github.com/basecamp/hey-cli/internal/apierr" + "github.com/basecamp/hey-cli/internal/output" +) + +// codexRulesEnv isolates a home with Codex installed at its own CODEX_HOME and +// returns the home, the rules file Codex would load, and a server that must not +// be called. +func codexRulesEnv(t *testing.T) (home, rulesPath, serverURL string) { + t.Helper() + isolateAgents(t) + home = t.TempDir() + codexHome := t.TempDir() + t.Setenv("CODEX_HOME", codexHome) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected HTTP request: %s %s", r.Method, r.URL.Path) + })) + t.Cleanup(server.Close) + return home, filepath.Join(codexHome, "rules", "hey.rules"), server.URL +} + +func setupCodex(t *testing.T, home, serverURL string) (output.Response, error) { + t.Helper() + _, response, err := runAuthCommand(t, home, serverURL, "", true, "setup", "codex") + return response, err +} + +func writeRulesFixture(t *testing.T, path, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +var allowRuleLine = regexp.MustCompile(`^prefix_rule\(pattern = \[("[a-z-]+"(?:, "[a-z-]+")*)\], decision = "allow"\)$`) + +// parseAllowRules reads the rules file the way this test needs Codex to: a +// managed-marker first line, comments, and one allow rule per line. Anything +// else fails, since a parse error in any rules file makes Codex drop them all. +func parseAllowRules(t *testing.T, content string) [][]string { + t.Helper() + lines := strings.Split(strings.TrimSuffix(content, "\n"), "\n") + if !strings.HasPrefix(lines[0], "# Managed by hey-cli.") { + t.Fatalf("first line is not the managed marker: %q", lines[0]) + } + var rules [][]string + for _, line := range lines { + if line == "" || strings.HasPrefix(line, "#") { + continue + } + match := allowRuleLine.FindStringSubmatch(line) + if match == nil { + t.Fatalf("unexpected line in hey.rules: %q", line) + } + var tokens []string + for _, quoted := range strings.Split(match[1], ", ") { + tokens = append(tokens, strings.Trim(quoted, `"`)) + } + rules = append(rules, tokens) + } + if len(rules) == 0 { + t.Fatal("hey.rules allows nothing") + } + return rules +} + +// allowed applies the rules as Codex does: positionally, on argv tokens. +func rulesAllow(rules [][]string, argv ...string) bool { + for _, rule := range rules { + if len(argv) >= len(rule) && strings.Join(argv[:len(rule)], "\x00") == strings.Join(rule, "\x00") { + return true + } + } + return false +} + +func installedRules(t *testing.T) [][]string { + t.Helper() + home, rulesPath, serverURL := codexRulesEnv(t) + response, err := setupCodex(t, home, serverURL) + if err != nil { + t.Fatalf("setup codex: %v", err) + } + if response.Summary != "Codex connected" { + t.Errorf("summary = %q", response.Summary) + } + info, err := os.Lstat(rulesPath) + if err != nil { + t.Fatalf("rules not installed: %v", err) + } + if !info.Mode().IsRegular() || info.Mode().Perm() != 0o644 { + t.Errorf("rules mode = %v, want a regular 0644 file", info.Mode()) + } + return parseAllowRules(t, string(readFile(t, rulesPath))) +} + +func TestSetupCodexInstallsReadOnlyRules(t *testing.T) { + rules := installedRules(t) + + for _, argv := range [][]string{ + {"hey", "search", "invoice", "--json"}, + {"hey", "box", "imbox"}, + {"hey", "thread", "read", "12345", "--json"}, + {"hey", "contact", "note", "show", "12345"}, + {"hey", "auth", "status", "--json"}, + } { + if !rulesAllow(rules, argv...) { + t.Errorf("read command not allowed: %v", argv) + } + } + for _, argv := range [][]string{ + {"hey", "compose", "--to", "alice@example.com"}, + {"hey", "reply", "12345", "-m", "Thanks"}, + {"hey", "draft", "send", "12345"}, + {"hey", "attachment", "save", "67890:1"}, + {"hey", "contact", "show-again", "12345"}, + {"hey", "auth", "login"}, + {"hey", "timetrack", "export"}, + {"hey", "watch"}, + {"hey", "--json", "search", "invoice"}, + } { + if rulesAllow(rules, argv...) { + t.Errorf("rules allow a command that must ask: %v", argv) + } + } +} + +// Every allowed prefix must name a runnable command, so a renamed or removed +// command fails here instead of silently losing (or misdirecting) its rule. +func TestCodexRulesNameRunnableCommands(t *testing.T) { + rules := installedRules(t) + root := newRootCmd() + for _, rule := range rules { + if rule[0] != "hey" || len(rule) < 2 { + t.Errorf("rule %v does not name a hey subcommand", rule) + continue + } + found, rest, err := root.Find(rule[1:]) + path := strings.Join(rule, " ") + if err != nil || len(rest) != 0 || found.CommandPath() != path { + t.Errorf("rule %q resolves to %q (rest %v, err %v)", path, found.CommandPath(), rest, err) + continue + } + if !found.Runnable() { + t.Errorf("rule %q names a command group, not a command", path) + } + } +} + +func TestSetupCodexReplacesManagedRules(t *testing.T) { + home, rulesPath, serverURL := codexRulesEnv(t) + stale := "# Managed by hey-cli. An older release.\nprefix_rule(pattern = [\"hey\"], decision = \"allow\")\n" + writeRulesFixture(t, rulesPath, stale) + + if _, err := setupCodex(t, home, serverURL); err != nil { + t.Fatalf("setup codex: %v", err) + } + rules := parseAllowRules(t, string(readFile(t, rulesPath))) + if rulesAllow(rules, "hey", "compose") { + t.Error("the stale blanket rule survived") + } + entries, err := os.ReadDir(filepath.Dir(rulesPath)) + if err != nil { + t.Fatal(err) + } + if len(entries) != 1 { + t.Errorf("rules directory holds %d entries, want only hey.rules", len(entries)) + } +} + +func TestSetupCodexRefusesUnmanagedRules(t *testing.T) { + home, rulesPath, serverURL := codexRulesEnv(t) + custom := "# My own rules for hey.\nprefix_rule(pattern = [\"hey\"], decision = \"allow\")\n" + writeRulesFixture(t, rulesPath, custom) + + _, err := setupCodex(t, home, serverURL) + var cliErr *apierr.Error + if !errors.As(err, &cliErr) || cliErr.Code != "setup_incomplete" { + t.Fatalf("error = %v, want setup_incomplete", err) + } + if errs, _ := cliErr.Meta["errors"].([]string); !strings.Contains(strings.Join(errs, "|"), rulesPath) { + t.Errorf("error does not name %s: %+v", rulesPath, cliErr) + } + if got := string(readFile(t, rulesPath)); got != custom { + t.Errorf("user rules changed: %q", got) + } +} + +func TestSetupCodexRefusesSymlinkedRules(t *testing.T) { + home, rulesPath, serverURL := codexRulesEnv(t) + target := filepath.Join(t.TempDir(), "shared.rules") + managedLooking := "# Managed by hey-cli.\nprefix_rule(pattern = [\"hey\"], decision = \"allow\")\n" + writeRulesFixture(t, target, managedLooking) + if err := os.MkdirAll(filepath.Dir(rulesPath), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, rulesPath); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + + _, err := setupCodex(t, home, serverURL) + var cliErr *apierr.Error + if !errors.As(err, &cliErr) || cliErr.Code != "setup_incomplete" { + t.Fatalf("error = %v, want setup_incomplete", err) + } + if info, err := os.Lstat(rulesPath); err != nil || info.Mode()&os.ModeSymlink == 0 { + t.Errorf("symlink replaced: %v", err) + } + if got := string(readFile(t, target)); got != managedLooking { + t.Errorf("symlink target changed: %q", got) + } +} + +func TestSetupAgentsRemoveDeletesManagedCodexRules(t *testing.T) { + home, rulesPath, serverURL := codexRulesEnv(t) + if _, err := setupCodex(t, home, serverURL); err != nil { + t.Fatalf("setup codex: %v", err) + } + if _, err := os.Stat(rulesPath); err != nil { + t.Fatalf("rules not installed: %v", err) + } + + _, response, err := runAuthCommand(t, home, serverURL, "", true, "setup", "agents", "--remove") + if err != nil { + t.Fatalf("setup agents --remove: %v", err) + } + removed := stringList(t, response.Data.(map[string]any)["removed"]) + if !strings.Contains(strings.Join(removed, "|"), "Codex rules") { + t.Errorf("removed = %v", removed) + } + if _, err := os.Lstat(rulesPath); !os.IsNotExist(err) { + t.Errorf("managed rules remain: %v", err) + } +} + +func TestSetupAgentsRemovePreservesUnmanagedCodexRules(t *testing.T) { + home, rulesPath, serverURL := codexRulesEnv(t) + custom := "prefix_rule(pattern = [\"hey\"], decision = \"allow\")\n" + writeRulesFixture(t, rulesPath, custom) + + _, response, err := runAuthCommand(t, home, serverURL, "", true, "setup", "agents", "--remove") + if err != nil { + t.Fatalf("setup agents --remove: %v", err) + } + if removed := stringList(t, response.Data.(map[string]any)["removed"]); strings.Contains(strings.Join(removed, "|"), "Codex rules") { + t.Errorf("removed = %v", removed) + } + if got := string(readFile(t, rulesPath)); got != custom { + t.Errorf("user rules changed: %q", got) + } +} + +func codexRulesDoctorCheck(t *testing.T, home, serverURL string) map[string]any { + t.Helper() + _, response, err := runAuthCommand(t, home, serverURL, "", true, "doctor") + if err != nil { + t.Fatalf("doctor: %v", err) + } + for _, raw := range response.Data.([]any) { + if check := raw.(map[string]any); check["name"] == "Codex Rules" { + return check + } + } + t.Fatalf("doctor has no Codex Rules check: %v", response.Data) + return nil +} + +func TestDoctorChecksCodexRules(t *testing.T) { + home, rulesPath, serverURL := codexRulesEnv(t) + + if check := codexRulesDoctorCheck(t, home, serverURL); check["status"] != "error" || check["hint"] != "Run: hey setup codex" { + t.Errorf("missing rules: %v", check) + } + + if _, err := setupCodex(t, home, serverURL); err != nil { + t.Fatalf("setup codex: %v", err) + } + if check := codexRulesDoctorCheck(t, home, serverURL); check["status"] != "ok" { + t.Errorf("installed rules: %v", check) + } + + writeRulesFixture(t, rulesPath, "# Managed by hey-cli. An older release.\nprefix_rule(pattern = [\"hey\", \"search\"], decision = \"allow\")\n") + if check := codexRulesDoctorCheck(t, home, serverURL); check["status"] != "error" || check["hint"] != "Run: hey setup codex" { + t.Errorf("stale rules: %v", check) + } + + writeRulesFixture(t, rulesPath, "prefix_rule(pattern = [\"hey\"], decision = \"allow\")\n") + if check := codexRulesDoctorCheck(t, home, serverURL); check["status"] != "error" || check["hint"] != "Move it aside, then run: hey setup codex" { + t.Errorf("unmanaged rules: %v", check) + } +} diff --git a/skills/embed_test.go b/skills/embed_test.go index d5451a0a..bfc47262 100644 --- a/skills/embed_test.go +++ b/skills/embed_test.go @@ -34,7 +34,10 @@ func TestHeySkillReusesAuthenticationForUnattendedAgents(t *testing.T) { } } -func TestHeySkillRetriesMacOSKeychainAccessWithoutBroadEscalation(t *testing.T) { +// Under Codex the sandbox hides the keyring and the network. The skill must +// send reads through the rules hey setup codex installs, send everything else +// through the user's approval, and not answer a sandboxed read with a login loop. +func TestHeySkillRunsUnderTheCodexSandbox(t *testing.T) { data, err := FS.ReadFile("hey/SKILL.md") if err != nil { t.Fatal(err) @@ -42,14 +45,16 @@ func TestHeySkillRetriesMacOSKeychainAccessWithoutBroadEscalation(t *testing.T) content := string(data) for _, want := range []string{ - "On macOS under Codex", - "retry `hey auth status --json` once with elevated sandbox permission", - "one read-only command", - "normal approval flow", - "rerun only the exact `hey` command the user requested", - "separate one-command approval", - "`data.authenticated` is `true`", - "`data.authenticated` is `false` or the status command fails", + "Under Codex", + "blocks the system keyring and the network", + "`hey setup codex` installs rules", + "no `VAR=value` prefix, no pipe, no redirect", + "global flags after the subcommand", + "`--jq` instead of piping to `jq`", + "with escalated sandbox permissions so the user approves it", + "run `hey setup codex` in a terminal and restart Codex", + "Do not tell them to log in again unless `hey auth status --json`", + "`data.authenticated` is `false`", "Never run the macOS `security` command", "print or copy credentials", "move credentials into a file", @@ -60,6 +65,15 @@ func TestHeySkillRetriesMacOSKeychainAccessWithoutBroadEscalation(t *testing.T) t.Errorf("embedded HEY skill does not contain %q", want) } } + + for _, forbidden := range []string{ + "retry `hey auth status --json` once with elevated sandbox permission", + "On macOS under Codex, the sandbox can hide credentials", + } { + if strings.Contains(content, forbidden) { + t.Errorf("embedded HEY skill still has the escalate-and-retry advice %q", forbidden) + } + } } // Setting a note replaces it, so the skill's recipe for adding to one must not write From eda8cf965937c4a105b1af2a991a532ac6e95366 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Thu, 22 Oct 2026 09:49:10 -0400 Subject: [PATCH 2/2] Install Codex rules that run hey's read commands outside the sandbox Codex runs shell commands in a sandbox that blocks the system keyring and the network, so every hey command it ran failed and the skill sent users back to hey auth login in a loop. hey setup codex now writes a managed $CODEX_HOME/rules/hey.rules whose prefix rules let the read-only commands run outside the sandbox; everything that sends, changes or saves still goes through Codex's approval prompt. The file is written only when absent or carrying hey-cli's marker, doctor reports it missing, stale or foreign, and setup agents --remove deletes it. The skill now tells Codex to run reads as plain commands and to send the user to hey setup codex, not to log in again. --- docs/agents.md | 16 +- docs/architecture/auth-and-setup.md | 5 +- internal/cmd/setup_agent.go | 67 +++++++- internal/cmd/setup_agents_remove.go | 26 +++ internal/cmd/setup_codex_rules_test.go | 13 ++ internal/harness/codex.go | 9 +- internal/harness/codex_rules.go | 149 ++++++++++++++++++ .../skill-coverage/ledger/f-journal-auth.tsv | 11 +- skills/hey/SKILL.md | 8 +- 9 files changed, 286 insertions(+), 18 deletions(-) create mode 100644 internal/harness/codex_rules.go diff --git a/docs/agents.md b/docs/agents.md index 5fc31f34..78d4f706 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -14,10 +14,10 @@ manage the integrations on their own: ```bash hey setup claude # install the skill and the hey@37signals plugin for Claude Code -hey setup codex # install the shared skill for Codex +hey setup codex # install the shared skill and sandbox rules for Codex hey skill install # install the skill only (~/.agents/skills/hey, linked for detected agents) hey setup agents # non-interactive: skill + a single detected agent (the installer uses this) -hey setup agents --remove # remove HEY's managed skills and Claude Code plugin +hey setup agents --remove # remove HEY's managed skills, Codex rules and Claude Code plugin hey doctor # check skill and plugin health per detected agent ``` @@ -33,6 +33,18 @@ skill twice. Setup and version refresh remove that legacy path only when its ownership marker proves an older hey-cli created it; user-authored files are left untouched. +Codex runs every shell command in a sandbox that blocks the system keyring and the +network, so a `hey` command run there can neither read your credentials nor reach HEY. +`hey setup codex` therefore also writes `~/.codex/rules/hey.rules` (under `$CODEX_HOME` +when that is set): prefix rules that let Codex run hey's read-only commands, such as +`hey search`, `hey box`, `hey thread read` and the list, show and view subcommands, +outside the sandbox without asking. Every other hey command, including anything that +sends, changes or saves, still goes through Codex's approval prompt each time. Codex +reads rules only when a session starts, so restart Codex after setup. `hey doctor` +reports the rules as missing or out of date after an upgrade that changes them; rerun +`hey setup codex` to refresh them. `hey setup agents --remove` deletes the file. Setup +never overwrites a `hey.rules` it did not write: move your own aside first. + hey only ever writes skill directories it owns: each one it creates carries a `.managed-by-hey-cli` marker, and install, replacement and automatic refresh all refuse a `hey` skill directory (or symlink) without it — a hand-authored skill at one of those paths diff --git a/docs/architecture/auth-and-setup.md b/docs/architecture/auth-and-setup.md index 88758b4b..ab54f2f1 100644 --- a/docs/architecture/auth-and-setup.md +++ b/docs/architecture/auth-and-setup.md @@ -47,7 +47,10 @@ version change. Ownership is explicit: every skill write goes through `claimSkil marks a directory it creates (`.managed-by-hey-cli`) and refuses a populated one without the marker; removal (`removeExistingSkillLink`) touches only our canonical symlink or a marked copy; refresh writes only marked, non-symlinked files. Do not add a write path that bypasses -the gate. +the gate. Codex's `$CODEX_HOME/rules/hey.rules`, which lets the read-only commands in +`harness.CodexReadCommands` run outside Codex's sandbox, is the other hey-cli-managed write: its +first line is the marker, and setup, removal and `hey doctor` all go through +`harness.InspectCodexRules`, which treats an unmarked file or a symlink as the user's. New top-level commands and subcommands need the `.surface` snapshot updated (`go test ./internal/cmd -run TestSurfaceSnapshot -update-surface`), and anything listed in diff --git a/internal/cmd/setup_agent.go b/internal/cmd/setup_agent.go index 55c46baf..a118411a 100644 --- a/internal/cmd/setup_agent.go +++ b/internal/cmd/setup_agent.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "strings" "time" @@ -73,6 +74,7 @@ var agentSetupHandlers = map[string]agentSetupHandler{ "codex": { Labels: []string{ "Install the shared HEY skill for Codex", + "Let Codex run read-only hey commands outside its sandbox", }, Run: runCodexSetup, RunNonInteractive: runCodexSetupNonInteractive, @@ -238,7 +240,11 @@ func runSetupAgent(cmd *cobra.Command, agent harness.AgentInfo, handler agentSet if len(manualCommands) > 0 { hint = strings.Join(manualCommands, "; ") } - return &apierr.Error{Code: "setup_incomplete", Message: summary, Hint: hint} + setupErr := &apierr.Error{Code: "setup_incomplete", Message: summary, Hint: hint} + if len(setupErrors) > 0 { + setupErr.Meta = map[string]any{"errors": setupErrors} + } + return setupErr } breadcrumbs := []output.Breadcrumb{ @@ -393,7 +399,8 @@ func agentCommandFailure(out []byte, err error) string { // --- Codex --- -// runCodexSetup connects Codex to the shared agent skill. +// runCodexSetup connects Codex to the shared agent skill and installs the +// rules that let its reads leave the sandbox. func runCodexSetup(cmd *cobra.Command) error { w := cmd.OutOrStdout() path, err := installCodexSkill() @@ -403,11 +410,22 @@ func runCodexSetup(cmd *cobra.Command) error { return nil //nolint:nilerr // warn and continue; the post-setup snapshot reports the failure } fmt.Fprintln(w, statusLine(true, "Codex skill installed ("+path+")")) + + rulesPath, err := installCodexRules() + if err != nil { + fmt.Fprintln(w, warning.format("Codex rules install failed: "+err.Error())) + fmt.Fprintln(w, "Then verify with: hey doctor") + return nil //nolint:nilerr // warn and continue; the post-setup snapshot reports the failure + } + fmt.Fprintln(w, statusLine(true, "Codex rules installed ("+rulesPath+"); restart Codex to load them")) return nil } func runCodexSetupNonInteractive(*cobra.Command) error { - _, err := installCodexSkill() + if _, err := installCodexSkill(); err != nil { + return err + } + _, err := installCodexRules() return err } @@ -434,6 +452,49 @@ func installCodexSkill() (string, error) { return path, nil } +// installCodexRules writes hey's rules file, which lets Codex run the read-only +// commands outside its sandbox. Like claimSkillDir, it replaces only a file +// hey-cli wrote. It runs after installCodexSkill, so it never fabricates the +// Codex home on a machine without Codex. +func installCodexRules() (string, error) { + path := harness.CodexRulesPath() + if path == "" { + return "", fmt.Errorf("cannot determine Codex home directory") + } + state, err := harness.InspectCodexRules(path) + if err != nil { + return "", fmt.Errorf("inspecting %s: %w", path, err) + } + if state == harness.CodexRulesForeign { + return "", fmt.Errorf("%s exists but was not written by hey-cli; move it aside to let hey install its Codex rules there", path) + } + if err := writeFileAtomic(path, harness.CodexRules()); err != nil { + return "", fmt.Errorf("writing %s: %w", path, err) + } + return path, nil +} + +// writeFileAtomic replaces path through a rename, so Codex never loads a +// half-written rules file. The temporary name does not end in .rules, which +// keeps Codex from loading it either. +func writeFileAtomic(path string, data []byte) error { + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o755); err != nil { // #nosec G301 -- Codex's own rules directory + return err + } + tmp, err := os.CreateTemp(dir, ".hey-rules-*.tmp") + if err != nil { + return err + } + defer os.Remove(tmp.Name()) // a no-op once the rename has moved it + _, writeErr := tmp.Write(data) + chmodErr := tmp.Chmod(0o644) // #nosec G302 -- rules are configuration, not secrets + if err := errors.Join(writeErr, chmodErr, tmp.Close()); err != nil { + return err + } + return os.Rename(tmp.Name(), path) +} + // --- Shared helpers --- // statusLine renders a ✓/✗ checklist line. diff --git a/internal/cmd/setup_agents_remove.go b/internal/cmd/setup_agents_remove.go index 36d54d34..1b2545af 100644 --- a/internal/cmd/setup_agents_remove.go +++ b/internal/cmd/setup_agents_remove.go @@ -55,6 +55,12 @@ func runRemoveAgentSetup(cmd *cobra.Command) error { } } + if didRemove, removeErr := removeCodexRules(); removeErr != nil { + failures = append(failures, "Codex rules: "+removeErr.Error()) + } else if didRemove { + removed = append(removed, "Codex rules") + } + baseline := filepath.Join(home, ".agents", "skills", "hey") if didRemove, removeErr := removeOwnedSkillFiles(baseline); removeErr != nil { failures = append(failures, "agent skill: "+removeErr.Error()) @@ -123,6 +129,26 @@ func removeOwnedSkillFiles(dir string) (bool, error) { return true, nil } +// removeCodexRules deletes hey's Codex rules file, and only a regular file +// that carries hey-cli's marker: a user's hey.rules or symlink stays put. +func removeCodexRules() (bool, error) { + path := harness.CodexRulesPath() + if path == "" { + return false, nil + } + state, err := harness.InspectCodexRules(path) + if err != nil { + return false, fmt.Errorf("inspecting %s: %w", path, err) + } + if state != harness.CodexRulesCurrent && state != harness.CodexRulesStale { + return false, nil + } + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + return false, fmt.Errorf("removing %s: %w", path, err) + } + return true, nil +} + func exists(path string) bool { _, err := os.Lstat(path) return err == nil diff --git a/internal/cmd/setup_codex_rules_test.go b/internal/cmd/setup_codex_rules_test.go index 6dbb2327..292dbd4d 100644 --- a/internal/cmd/setup_codex_rules_test.go +++ b/internal/cmd/setup_codex_rules_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "regexp" + "slices" "strings" "testing" @@ -144,6 +145,7 @@ func TestSetupCodexInstallsReadOnlyRules(t *testing.T) { func TestCodexRulesNameRunnableCommands(t *testing.T) { rules := installedRules(t) root := newRootCmd() + var covered []string for _, rule := range rules { if rule[0] != "hey" || len(rule) < 2 { t.Errorf("rule %v does not name a hey subcommand", rule) @@ -158,6 +160,17 @@ func TestCodexRulesNameRunnableCommands(t *testing.T) { if !found.Runnable() { t.Errorf("rule %q names a command group, not a command", path) } + for _, sub := range found.Commands() { + covered = append(covered, sub.CommandPath()) + } + } + // A rule is a prefix, so it also lets a subcommand of the command it names + // leave the sandbox. These were reviewed as read-only; a new one fails here + // until someone decides whether it may run without asking. + reviewed := "hey box list|hey box view|hey bundle view|hey search filters" + slices.Sort(covered) + if got := strings.Join(covered, "|"); got != reviewed { + t.Errorf("subcommands covered by a rule = %q, reviewed %q", got, reviewed) } } diff --git a/internal/harness/codex.go b/internal/harness/codex.go index 92d4fc9e..ad945ba1 100644 --- a/internal/harness/codex.go +++ b/internal/harness/codex.go @@ -13,14 +13,13 @@ func init() { Name: "Codex", ID: "codex", Detect: DetectCodex, - // Codex discovers the shared ~/.agents skill directly, so health is - // skill-presence only. When a native plugin lands, this grows the - // plugin/version checks basecamp-cli has. + // Codex discovers the shared ~/.agents skill directly; the rules are + // what let its commands reach the keyring and HEY from the sandbox. Checks: func() []*StatusCheck { - return []*StatusCheck{CheckCodexSkill()} + return []*StatusCheck{CheckCodexSkill(), CheckCodexRules()} }, Diagnostics: func(_ context.Context) []*StatusCheck { - return []*StatusCheck{CheckCodexSkill()} + return []*StatusCheck{CheckCodexSkill(), CheckCodexRules()} }, }) } diff --git a/internal/harness/codex_rules.go b/internal/harness/codex_rules.go new file mode 100644 index 00000000..81f8c323 --- /dev/null +++ b/internal/harness/codex_rules.go @@ -0,0 +1,149 @@ +package harness + +import ( + "bytes" + "fmt" + "os" + "path/filepath" + "strings" +) + +// codexRulesMarker opens every rules file hey-cli writes. A hey.rules without +// it is the user's, and nothing in hey-cli rewrites, removes or vouches for it. +const codexRulesMarker = "# Managed by hey-cli." + +// CodexReadCommands are the hey commands Codex may run outside its sandbox +// without asking. Each reads HEY or local state and changes neither: no +// command here sends, edits, files, saves to disk or signs in. Codex matches a +// rule as a prefix of the command's words, so "box" also covers "box view". +var CodexReadCommands = []string{ + "search", + "box", + "bundle", + "version", + "doctor", + "commands", + "auth status", + "account list", + "account senders", + "attachment list", + "bubble list", + "bulk-reply preview", + "calendar list", + "clip list", + "collection list", + "collection view", + "contact list", + "contact show", + "contact threads", + "contact note show", + "draft list", + "draft show", + "event day", + "event list", + "event week", + "habit list", + "journal list", + "journal read", + "label list", + "label view", + "screener list", + "screener history", + "set-aside view", + "set-aside group list", + "set-aside group view", + "snippet list", + "thread read", + "timetrack categories", + "timetrack current", + "timetrack list", + "todo list", + "workflow list", + "workflow view", +} + +// CodexRulesState is what occupies the path Codex loads hey's rules from. +type CodexRulesState int + +const ( + CodexRulesAbsent CodexRulesState = iota + CodexRulesCurrent // hey-cli's, matching this release + CodexRulesStale // hey-cli's, from another release + CodexRulesForeign // anything hey-cli did not write, symlinks included +) + +// CodexRulesPath returns $CODEX_HOME/rules/hey.rules, or "" without a Codex home. +func CodexRulesPath() string { + codexHome := CodexHome() + if codexHome == "" { + return "" + } + return filepath.Join(codexHome, "rules", "hey.rules") +} + +// CodexRules renders the rules file. Codex drops every user rule when any +// rules file fails to parse, so this emits one fixed line shape only. +func CodexRules() []byte { + var b strings.Builder + b.WriteString(codexRulesMarker + " hey setup codex rewrites this file; hey setup agents --remove deletes it.\n") + b.WriteString("# Lets Codex run these read-only hey commands outside its sandbox, where they can\n") + b.WriteString("# reach the system keyring and HEY. Every other hey command still asks first.\n") + for _, command := range CodexReadCommands { + tokens := append([]string{"hey"}, strings.Fields(command)...) + fmt.Fprintf(&b, "prefix_rule(pattern = [\"%s\"], decision = \"allow\")\n", strings.Join(tokens, `", "`)) + } + return []byte(b.String()) +} + +// InspectCodexRules reports what occupies path. A symlink is foreign whatever +// it points at: its target was never ours to inspect. +func InspectCodexRules(path string) (CodexRulesState, error) { + info, err := os.Lstat(path) + if os.IsNotExist(err) { + return CodexRulesAbsent, nil + } + if err != nil { + return CodexRulesAbsent, err + } + if !info.Mode().IsRegular() { + return CodexRulesForeign, nil + } + data, err := os.ReadFile(path) // #nosec G304 -- the fixed rules path under the Codex home + if err != nil { + return CodexRulesAbsent, err + } + switch { + case !bytes.HasPrefix(data, []byte(codexRulesMarker)): + return CodexRulesForeign, nil + case bytes.Equal(data, CodexRules()): + return CodexRulesCurrent, nil + default: + return CodexRulesStale, nil + } +} + +// CheckCodexRules checks whether hey's current Codex rules are installed. +func CheckCodexRules() *StatusCheck { + check := &StatusCheck{Name: "Codex Rules"} + path := CodexRulesPath() + if path == "" { + check.Status, check.Message = "warn", "Cannot determine Codex home directory" + return check + } + state, err := InspectCodexRules(path) + if err != nil { + check.Status, check.Message, check.Hint = "warn", "Cannot check Codex rules", err.Error() + return check + } + switch state { + case CodexRulesCurrent: + check.Status, check.Message = "pass", "Installed" + case CodexRulesStale: + check.Status, check.Message, check.Hint = "fail", "Out of date at "+path, "Run: hey setup codex" + case CodexRulesForeign: + check.Status, check.Message, check.Hint = "fail", "A rules file not written by hey-cli occupies "+path, "Move it aside, then run: hey setup codex" + default: + check.Status, check.Message, check.Hint = "fail", "Not installed; Codex runs hey reads in its sandbox, without the keyring", "Run: hey setup codex" + } + return check +} diff --git a/scripts/skill-coverage/ledger/f-journal-auth.tsv b/scripts/skill-coverage/ledger/f-journal-auth.tsv index 48fb165c..676dec90 100644 --- a/scripts/skill-coverage/ledger/f-journal-auth.tsv +++ b/scripts/skill-coverage/ledger/f-journal-auth.tsv @@ -26,11 +26,12 @@ authentication/2 skill Run the requested data command without a login preflight. authentication/3 help:hey auth status Show authentication status authentication/3 help:hey auth status Use it when the user asks for authentication status or when an explicit check helps diagnose a failed command, not as a preflight. It reports stored credentials without contacting HEY and changes nothing authentication/4 skill If a data command returns exit code 3 with `"code": "auth"`, or fails with `could not read stored credentials`, authentication is unavailable to that process. -authentication/5 skill On macOS under Codex, the sandbox can hide credentials that the HEY CLI stores in Keychain. -authentication/6 skill Before reporting the task as blocked, use the harness's normal approval flow to retry `hey auth status --json` once with elevated sandbox permission. -authentication/7 skill Limit the escalation to this one read-only command. -authentication/8 skill If the response's `data.authenticated` is `true`, rerun only the exact `hey` command the user requested through a separate one-command approval. -authentication/9 skill If `data.authenticated` is `false` or the status command fails, report the task as blocked and tell the user to run `hey auth login`; do not run it for them unattended. +authentication/5 retired The Codex sandbox blocks the keyring and the network on every platform, not only macOS; the rules hey setup codex installs now run reads outside it. +authentication/6 retired Escalating one status check and then one command per task left users looping back to hey auth login; reads now leave the sandbox through hey setup codex's rules, and other commands go through Codex's own approval. +authentication/7 retired Escalating one status check and then one command per task left users looping back to hey auth login; reads now leave the sandbox through hey setup codex's rules, and other commands go through Codex's own approval. +authentication/8 retired Escalating one status check and then one command per task left users looping back to hey auth login; reads now leave the sandbox through hey setup codex's rules, and other commands go through Codex's own approval. +authentication/9 skill Outside Codex, report the task as blocked and tell the user to run `hey auth login`; do not run it for them unattended. +authentication/9 skill Do not tell them to log in again unless `hey auth status --json`, run with escalated sandbox permissions, reports that `data.authenticated` is `false`. authentication/10 skill Never run the macOS `security` command to read Keychain contents, print or copy credentials, or move credentials into a file; never disable the sandbox globally; never set `HEY_NO_KEYRING=1` as an authentication workaround. authentication/11 skill Piped, machine-output and non-TTY commands do not prompt for sign-in. authentication/12 skill When an agent harness runs commands under a PTY, set `HEY_NONINTERACTIVE=1` so a missing login returns the same actionable auth error instead of opening an interactive prompt. diff --git a/skills/hey/SKILL.md b/skills/hey/SKILL.md index f5767d5a..d8d67cd5 100644 --- a/skills/hey/SKILL.md +++ b/skills/hey/SKILL.md @@ -241,8 +241,12 @@ entry=$(hey journal read 2026-03-15 --jq '.data.content // error("no journal ent Data commands use the credentials HEY already stores and refresh expiring OAuth tokens automatically. Run the requested data command without a login preflight. -If a data command returns exit code 3 with `"code": "auth"`, or fails with `could not read stored credentials`, authentication is unavailable to that process. On macOS under Codex, the sandbox can hide credentials that the HEY CLI stores in Keychain. Before reporting the task as blocked, use the harness's normal approval flow to retry `hey auth status --json` once with elevated sandbox permission. Limit the escalation to this one read-only command. If the response's `data.authenticated` is `true`, rerun only the exact `hey` command the user requested through a separate one-command approval. If `data.authenticated` is `false` or the status command fails, report the task as blocked and tell the user to run `hey auth login`; do not run it for them unattended. +If a data command returns exit code 3 with `"code": "auth"`, or fails with `could not read stored credentials`, authentication is unavailable to that process. Outside Codex, report the task as blocked and tell the user to run `hey auth login`; do not run it for them unattended. + +Under Codex, the sandbox blocks the system keyring and the network, so a `hey` command run inside it can neither read the stored credentials nor reach HEY. `hey setup codex` installs rules that run the read commands (`hey search`, `hey box`, `hey thread read`, `hey contact show`, `hey calendar list`, and the other list, show and view commands) outside the sandbox without asking. A rule matches only a command that begins with its words, so run a read as a plain `hey ...`: no `VAR=value` prefix, no pipe, no redirect, no `cd` before it, global flags after the subcommand (`hey search invoice --json`, not `hey --json search invoice`), and `--jq` instead of piping to `jq`. Run any other `hey` command, including everything that sends, changes or saves, with escalated sandbox permissions so the user approves it. + +If a read still fails with a keyring or credentials error under Codex, the rules are missing or Codex has not been restarted since they were installed. Report the task as blocked, and tell the user to run `hey setup codex` in a terminal and restart Codex. Do not tell them to log in again unless `hey auth status --json`, run with escalated sandbox permissions, reports that `data.authenticated` is `false`. Never run the macOS `security` command to read Keychain contents, print or copy credentials, or move credentials into a file; never disable the sandbox globally; never set `HEY_NO_KEYRING=1` as an authentication workaround. -Piped, machine-output and non-TTY commands do not prompt for sign-in. When an agent harness runs commands under a PTY, set `HEY_NONINTERACTIVE=1` so a missing login returns the same actionable auth error instead of opening an interactive prompt. +Piped, machine-output and non-TTY commands do not prompt for sign-in. When an agent harness runs commands under a PTY, set `HEY_NONINTERACTIVE=1` so a missing login returns the same actionable auth error instead of opening an interactive prompt. Under Codex, pass `--json` instead: a prefix stops the rules from matching, and machine output never prompts.