diff --git a/cmd/key.go b/cmd/key.go new file mode 100644 index 0000000..f268a1a --- /dev/null +++ b/cmd/key.go @@ -0,0 +1,39 @@ +package cmd + +import ( + "fmt" + + "github.com/nxssie/nan-cli/internal/session" + "github.com/spf13/cobra" +) + +var keyCmd = &cobra.Command{ + Use: "key", + Short: "Manage your API key", +} + +var keyPrintCmd = &cobra.Command{ + Use: "print", + Short: "Write your API key to stdout for another tool to read", + Args: cobra.NoArgs, + RunE: runKeyPrint, +} + +func init() { + rootCmd.AddCommand(keyCmd) + keyCmd.AddCommand(keyPrintCmd) +} + +func runKeyPrint(cmd *cobra.Command, args []string) error { + sess, err := session.Load() + if err != nil { + return err + } + // Tool configs read this command's stdout, so an empty key printed as an + // empty line would look like a working credential to them. + if sess.APIKey == "" { + return fmt.Errorf("no api key saved") + } + fmt.Fprintln(cmd.OutOrStdout(), sess.APIKey) + return nil +} diff --git a/cmd/key_test.go b/cmd/key_test.go new file mode 100644 index 0000000..7bc39ca --- /dev/null +++ b/cmd/key_test.go @@ -0,0 +1,95 @@ +package cmd + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/nxssie/nan-cli/internal/session" +) + +func withTestHome(t *testing.T) { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) +} + +func executeKeyPrint(t *testing.T) (string, error) { + t.Helper() + var stdout strings.Builder + rootCmd.SetOut(&stdout) + rootCmd.SetArgs([]string{"key", "print"}) + err := rootCmd.Execute() + return stdout.String(), err +} + +func saveSession(t *testing.T, s *session.Session) { + t.Helper() + if err := session.Save(s); err != nil { + t.Fatal(err) + } +} + +func TestKeyPrintWritesOnlyTheKey(t *testing.T) { + withTestHome(t) + saveSession(t, &session.Session{Token: "a-token", APIKey: "an-api-key"}) + + stdout, err := executeKeyPrint(t) + if err != nil { + t.Fatal(err) + } + if stdout != "an-api-key\n" { + t.Errorf("stdout = %q, want %q", stdout, "an-api-key\n") + } +} + +func TestKeyPrintFailsWithoutSession(t *testing.T) { + withTestHome(t) + + stdout, err := executeKeyPrint(t) + if err == nil { + t.Fatal("printed a key with no session") + } + if stdout != "" { + t.Errorf("stdout = %q, want nothing", stdout) + } +} + +func TestKeyPrintFailsOnEmptyKey(t *testing.T) { + withTestHome(t) + saveSession(t, &session.Session{Token: "a-token"}) + + stdout, err := executeKeyPrint(t) + if err == nil { + t.Fatal("printed an empty key as if it were one") + } + if stdout != "" { + t.Errorf("stdout = %q, want nothing", stdout) + } +} + +func TestKeyPrintErrorNeverContainsTheKey(t *testing.T) { + withTestHome(t) + key := "the-secret-key-value" + + dir := filepath.Dir(session.Path()) + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + // A session file the CLI cannot parse is the one error path where a real + // key is in the file, so it is the one where a message could echo it. + raw := []byte(`{"apiKey":"` + key + `"`) + if err := os.WriteFile(session.Path(), raw, 0o600); err != nil { + t.Fatal(err) + } + + _, err := executeKeyPrint(t) + if err == nil { + t.Fatal("expected the unreadable session to fail") + } + if strings.Contains(err.Error(), key) { + t.Errorf("error %q contains the key", err.Error()) + } +} diff --git a/internal/tui/config_test.go b/internal/tui/config_test.go index 1879c7a..633146f 100644 --- a/internal/tui/config_test.go +++ b/internal/tui/config_test.go @@ -232,6 +232,39 @@ func TestPiConfigWritesOnlyTheModalitiesPiAccepts(t *testing.T) { } } +// Pi joins Codex as a tool that never carries the literal key: models.json is +// a file members paste into issues and dotfiles, so the provider points at +// `nan key print` instead. +func TestPiConfigWritesACommandReferenceNotTheKey(t *testing.T) { + restoreNanExecutable(t, "/usr/local/bin/nan") + + path := tempConfig(t, "models.json") + if err := writePiConfig(path, testKey); err != nil { + t.Fatal(err) + } + + // piModels only returns the model list, so the raw provider object is what + // carries the reference. + providers := readJSON(t, path)["providers"].(map[string]any) + nan := providers["nan"].(map[string]any) + if got := nan["apiKey"]; got != "!/usr/local/bin/nan key print" { + t.Errorf("apiKey = %v, want the key command reference", got) + } + data := readFile(t, path) + if strings.Contains(data, testKey) { + t.Error("models.json carries the literal key") + } +} + +// The escapes are Pi's, not a shell's: `$$` reads as a literal `$` and `$!` +// as a literal `!`, so `$` has to be doubled before `!` gets its `$` prefix. +func TestPiKeyReferenceEscapesDollarAndBang(t *testing.T) { + got := piKeyReference("/opt/nan$bin/nan!") + if want := "!/opt/nan$$bin/nan$! key print"; got != want { + t.Errorf("piKeyReference = %q, want %q", got, want) + } +} + func TestPiConfigLeavesOtherProvidersAlone(t *testing.T) { path := tempConfig(t, "models.json") existing := `{"providers":{"openai":{"baseUrl":"https://api.openai.com/v1","models":[]}}}` @@ -329,8 +362,13 @@ wire_api = "chat" if strings.Count(content, "[model_providers.nan]") != 1 { t.Error("the provider was appended a second time instead of repaired") } - if !strings.Contains(content, `experimental_bearer_token = "nan-old"`) { - t.Error("a repair that should touch one line rewrote the member's key") + // The same write also takes the plaintext key an older version of this + // CLI put here, and leaves the command reference in its place. + if strings.Contains(content, "experimental_bearer_token") { + t.Error("the repair left the plaintext key in the file") + } + if !strings.Contains(content, `[model_providers.nan.auth]`) || !strings.Contains(content, `args = ["key", "print"]`) { + t.Errorf("the repair did not write the key command reference:\n%s", content) } // Another provider's wire_api is that provider's business. openai := content[strings.Index(content, "[model_providers.openai]"):strings.Index(content, "[model_providers.nan]")] @@ -408,6 +446,183 @@ func TestCodexConfigDoesNotTouchAnExistingChoice(t *testing.T) { if !strings.Contains(string(data), "[model_providers.nan]") { t.Error("the NaN provider was not appended") } + if !strings.Contains(string(data), `args = ["key", "print"]`) { + t.Errorf("the appended section does not reference nan key print:\n%s", data) + } +} + +// Under `go test` os.Executable() is the test binary, so the tests that +// assert on the written command swap it for a path with a known shape. +func restoreNanExecutable(t *testing.T, path string) { + t.Helper() + original := nanExecutable + nanExecutable = func() string { return path } + t.Cleanup(func() { nanExecutable = original }) +} + +// The key an older version of this CLI wrote here sat in a file members paste +// into issues and, for dotfiles, in a repository. The migration takes it out +// and points the section at `nan key print` instead, in the file that already +// exists - reached on the same early return the wire repair is. +func TestCodexConfigMigratesThePlaintextKeyToTheKeyCommand(t *testing.T) { + path := tempConfig(t, "config.toml") + restoreNanExecutable(t, "/usr/local/bin/nan") + existing := `model = "glm5.3-flash" + +[model_providers.old] +name = "Old" + +[model_providers.nan] +name = "NaN" +base_url = "https://api.nan.builders/v1" +experimental_bearer_token = "nan-old" +wire_api = "responses" + +[projects."/home/member/repo"] +trust_level = "trusted" +` + if err := os.WriteFile(path, []byte(existing), 0o600); err != nil { + t.Fatal(err) + } + if err := writeCodexConfig(path, testKey); err != nil { + t.Fatal(err) + } + content := readFile(t, path) + + if strings.Contains(content, "experimental_bearer_token") || strings.Contains(content, "nan-old") { + t.Errorf("the plaintext key survived the migration:\n%s", content) + } + if strings.Contains(content, testKey) { + t.Error("the migration wrote a literal key of its own") + } + + // Codex runs the command with exec, never a shell: command is the bare + // absolute path to the binary and every argument goes over in args. + if !strings.Contains(content, `[model_providers.nan.auth]`) { + t.Fatalf("no auth sub-table was written:\n%s", content) + } + if !strings.Contains(content, `command = "/usr/local/bin/nan"`) { + t.Errorf("command is not the bare absolute path to nan:\n%s", content) + } + if !strings.Contains(content, `args = ["key", "print"]`) { + t.Errorf("the arguments did not go into args:\n%s", content) + } + + // [model_providers.nan.auth] is a sub-table: every key after it belongs + // to it, the way model_context_window once became a key of the last + // [projects.*] entry. The member's own keys must come out of the + // migration where they went in. + if !strings.Contains(content, `trust_level = "trusted"`) { + t.Error("the member's project entry was lost") + } + if auth := strings.Index(content, "[model_providers.nan.auth]"); strings.Index(content, "trust_level") < auth { + t.Errorf("the auth sub-table swallowed the member's keys:\n%s", content) + } + if strings.Count(content, "[model_providers.nan]") != 1 { + t.Error("the provider was appended a second time instead of migrated") + } +} + +// A config a member also edits by hand is rewritten one line at a time, never +// reflowed: the comments, the spacing and the quoting around the one line +// that changed are theirs, and a round trip through a TOML library takes all +// of them. +func TestCodexMigrationIsLineSurgeryNotARoundTrip(t *testing.T) { + path := tempConfig(t, "config.toml") + restoreNanExecutable(t, "/usr/local/bin/nan") + existing := `# my notes, kept however I wrote them + +model = "gpt-5" # trailing comment +[model_providers.nan] +name = "NaN" +base_url = "https://api.nan.builders/v1" +experimental_bearer_token = "nan-old" +wire_api = "responses" +` + want := `# my notes, kept however I wrote them + +model = "gpt-5" # trailing comment +[model_providers.nan] +name = "NaN" +base_url = "https://api.nan.builders/v1" +wire_api = "responses" + +[model_providers.nan.auth] +command = "/usr/local/bin/nan" +args = ["key", "print"] +` + if err := os.WriteFile(path, []byte(existing), 0o600); err != nil { + t.Fatal(err) + } + if err := writeCodexConfig(path, testKey); err != nil { + t.Fatal(err) + } + if got := readFile(t, path); got != want { + t.Errorf("the file was reflowed, not operated on:\n got: %q\nwant: %q", got, want) + } +} + +// A config already carrying the reference has nothing to migrate, and a +// second run must not rewrite it: the command path it holds may not be +// replaced with whatever this run of the CLI happens to resolve to. +func TestCodexConfigAlreadyUsingTheKeyCommandIsNotRewritten(t *testing.T) { + path := tempConfig(t, "config.toml") + restoreNanExecutable(t, "/elsewhere/nan") + existing := `model = "gpt-5" + +[model_providers.nan] +name = "NaN" +base_url = "https://api.nan.builders/v1" +wire_api = "responses" + +[model_providers.nan.auth] +command = "/usr/local/bin/nan" +args = ["key", "print"] +` + if err := os.WriteFile(path, []byte(existing), 0o600); err != nil { + t.Fatal(err) + } + if err := writeCodexConfig(path, testKey); err != nil { + t.Fatal(err) + } + if got := readFile(t, path); got != existing { + t.Errorf("a config already using auth was rewritten:\n got: %q\nwant: %q", got, existing) + } +} + +// The key reads out of session.json at request time now, so no path through +// this writer may put the literal key into the file any more - the starter +// config and the append included. +func TestCodexConfigWritesACommandReferenceNotTheKey(t *testing.T) { + restoreNanExecutable(t, "/usr/local/bin/nan") + + path := tempConfig(t, "config.toml") + if err := writeCodexConfig(path, testKey); err != nil { + t.Fatal(err) + } + starter := readFile(t, path) + if strings.Contains(starter, testKey) { + t.Errorf("the starter config carries the literal key:\n%s", starter) + } + if !strings.Contains(starter, `[model_providers.nan.auth]`) || + !strings.Contains(starter, `command = "/usr/local/bin/nan"`) || + !strings.Contains(starter, `args = ["key", "print"]`) { + t.Errorf("the starter config does not reference nan key print:\n%s", starter) + } + if strings.Contains(starter, "experimental_bearer_token") { + t.Errorf("the starter config still writes the old bearer token:\n%s", starter) + } + + appended := tempConfig(t, "config.toml") + if err := os.WriteFile(appended, []byte("model = \"gpt-5\"\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := writeCodexConfig(appended, testKey); err != nil { + t.Fatal(err) + } + if content := readFile(t, appended); strings.Contains(content, testKey) { + t.Errorf("the appended section carries the literal key:\n%s", content) + } } // The file this repairs is the one a member actually turns up with: the @@ -916,7 +1131,19 @@ func TestConfigureToolsWritesEveryEnabledTool(t *testing.T) { t.Errorf("%s: written without the NaN base URL", name) } if !strings.Contains(string(data), testKey) { - t.Errorf("%s: written without the API key", name) + // Codex and Pi are the exceptions: their configs carry a reference + // to `nan key print` instead of the key itself. + if name == "Codex" || name == "Pi" { + want := `args = ["key", "print"]` + if name == "Pi" { + want = `key print` + } + if !strings.Contains(string(data), want) { + t.Errorf("%s: written without the key command reference", name) + } + } else { + t.Errorf("%s: written without the API key", name) + } } if !isNaNConfigured(name, p) { t.Errorf("%s: the Setup tab will not show it as configured", name) diff --git a/internal/tui/security_test.go b/internal/tui/security_test.go index 8df170d..864a9b5 100644 --- a/internal/tui/security_test.go +++ b/internal/tui/security_test.go @@ -34,9 +34,28 @@ func TestWritingAKeyTightensAConfigThatWasWideOpen(t *testing.T) { t.Fatalf("tools failed on a clean run: %v", failed) } for name, path := range toolPaths(home) { - if !strings.Contains(readFile(t, path), testKey) { - t.Errorf("%s was not configured at all", name) - continue + content := readFile(t, path) + if name == "Codex" || name == "Pi" { + // The two exceptions: their configs carry a reference to + // `nan key print` instead of the key, and that reference is what + // says they were configured at all. + if strings.Contains(content, testKey) { + t.Errorf("%s carries the literal key", name) + continue + } + want := `args = ["key", "print"]` + if name == "Pi" { + want = `key print` + } + if !strings.Contains(content, want) { + t.Errorf("%s was not configured with the key command reference", name) + continue + } + } else { + if !strings.Contains(content, testKey) { + t.Errorf("%s was not configured at all", name) + continue + } } assertNotWorldReadable(t, path) } @@ -72,7 +91,21 @@ func TestConfiguringTightensAToolThatNeedsNothingWritten(t *testing.T) { t.Fatalf("tools failed on the second run: %v", failed) } for name, path := range paths { - if !strings.Contains(readFile(t, path), testKey) { + content := readFile(t, path) + if name == "Codex" || name == "Pi" { + // The key was never in these files; the command reference is what + // has to survive the second run untouched. + if strings.Contains(content, testKey) { + t.Errorf("%s carries the literal key after the second run", name) + } + want := `args = ["key", "print"]` + if name == "Pi" { + want = `key print` + } + if !strings.Contains(content, want) { + t.Errorf("%s lost its key command reference on the second run", name) + } + } else if !strings.Contains(content, testKey) { t.Errorf("%s lost its key on the second run", name) } assertNotWorldReadable(t, path) @@ -145,6 +178,15 @@ func TestSigningOutTakesTheKeyOutOfTheTools(t *testing.T) { t.Errorf("%s still carries the key after signing out", name) } } + // Codex's config never held the key, but the command reference is ours + // all the same: after signing out it must not survive, or Codex keeps + // resolving a key that no longer exists. + codexContent := readFile(t, toolPaths(home)["Codex"]) + for _, ours := range []string{"[model_providers.nan]", "[model_providers.nan.auth]", `["model_providers.nan"]`} { + if strings.Contains(codexContent, ours) { + t.Errorf("Codex still carries %s after signing out", ours) + } + } if strings.Contains(readFile(t, hermesEnvPath(filepath.Join(home, "hermes"))), testKey) { t.Error("Hermes still carries the key after signing out") } diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 6212f99..54692f5 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -2297,7 +2297,7 @@ func writePiConfig(cfgPath, apiKey string) error { providers["nan"] = map[string]any{ "name": "NaN", "baseUrl": "https://api.nan.builders/v1", - "apiKey": apiKey, + "apiKey": piKeyReference(nanExecutable()), "api": "openai-completions", "compat": map[string]any{"supportsDeveloperRole": true}, "models": models, @@ -2314,6 +2314,18 @@ func writePiConfig(cfgPath, apiKey string) error { return writePiDefaults(piSettingsPath(cfgPath)) } +// Like Codex, Pi gets a reference to `nan key print` instead of the key +// itself: models.json is a file members paste into issues and dotfiles. +// The `!` prefix tells Pi to run the value as a command. +func piKeyReference(exe string) string { + // The order is not interchangeable: `$$` reads as a literal `$` and `$!` + // as a literal `!`, so every `$` has to become `$$` before the second pass + // inserts any new `$`, or the two escapes eat each other. + escaped := strings.ReplaceAll(exe, "$", "$$") + escaped = strings.ReplaceAll(escaped, "!", "$!") + return "!" + escaped + " key print" +} + // Pi reads the provider it calls from a second file, and until this existed // the CLI wrote only the first one. nan.builders/docs/pi marks this step "not // optional" for a reason: with models.json alone Pi goes on calling its @@ -2424,7 +2436,11 @@ func removePiDefaults(settingsPath string) error { return writeConfigFile(settingsPath, out) } -func writeCodexConfig(cfgPath, apiKey string) error { +// The API key no longer enters Codex's config at all: the auth sub-table +// points at `nan key print`, which reads it out of session.json at request +// time. The key parameter stays so every writer in the dispatch keeps one +// shape. +func writeCodexConfig(cfgPath string, _ string) error { data, _ := os.ReadFile(cfgPath) codexModel, _ := catalog.Get(catalog.Coding) @@ -2443,11 +2459,20 @@ func writeCodexConfig(cfgPath, apiKey string) error { // an error dialog and nothing else. Repairing the wire_api alone left // that member exactly as stuck as before, so the pruning runs first // and puts the key back where it is read from. + // + // The same file may still hold the plaintext key an older version of + // this CLI wrote into it - a file members paste into issues, and for + // dotfiles a target of this very repository - and a member who never + // re-runs setup keeps it for as long as this early return passes over + // it, so the migration happens here too. repaired, pruned := codexContextWindowsPruned(data, codexModel.Context) if pruned { repaired = []byte(withCodexContextWindow(string(repaired), codexModel.Context)) } rewired, changed := codexWireAPIRepaired(repaired) + if migrated, migratedNow := codexBearerTokenMigrated(rewired); migratedNow { + rewired, changed = migrated, true + } if pruned || changed { if err := writeConfigFile(cfgPath, rewired); err != nil { return err @@ -2465,9 +2490,12 @@ model_context_window = %d [model_providers.nan] name = "NaN" base_url = "https://api.nan.builders/v1" -experimental_bearer_token = %q wire_api = "responses" -`, codexModel.ID, codexModel.Context, apiKey) + +[model_providers.nan.auth] +command = %q +args = ["key", "print"] +`, codexModel.ID, codexModel.Context, nanExecutable()) if err := writeConfigFile(cfgPath, []byte(content)); err != nil { return err } @@ -2479,9 +2507,12 @@ wire_api = "responses" [model_providers.nan] name = "NaN" base_url = "https://api.nan.builders/v1" -experimental_bearer_token = %q wire_api = "responses" -`, apiKey) + +[model_providers.nan.auth] +command = %q +args = ["key", "print"] +`, nanExecutable()) content := strings.TrimRight(string(data), "\n") + "\n" + section if err := writeConfigFile(cfgPath, []byte(withCodexContextWindow(content, codexModel.Context))); err != nil { return err @@ -2543,6 +2574,21 @@ func removeCodexProfiles(codexHome string) error { return nil } +// What the tools execute for the bearer token: this binary with +// `nan key print`. It runs with exec, never a shell, so command is the bare +// absolute path to the executable and every argument goes over in args - a +// shell string here fails to start on every version measured, from 0.120.0 +// up, and a broken auth command looks exactly like an auth outage. +// +// Swapped in tests, because under `go test` os.Executable() is the test +// binary, not nan. +var nanExecutable = func() string { + if exe, err := os.Executable(); err == nil { + return exe + } + return "nan" +} + // Codex has no metadata for a model on this cluster, so without // model_context_window it compacts against a window it guessed. (It prints // the "metadata not found" warning either way - that fires before it reads @@ -2728,6 +2774,91 @@ func codexWireAPIRepaired(data []byte) ([]byte, bool) { return []byte(strings.Join(lines, "\n")), true } +// Deletes the plaintext experimental_bearer_token an older version of this +// CLI wrote into [model_providers.nan], and puts the auth sub-table in its +// place. Codex forbids combining auth with that key, so the line goes rather +// than the reference being added next to it. +// +// By hand, for the same reason codexWireAPIRepaired is: a round trip through +// a TOML library reflows the member's comments, their spacing, their +// quoting, to satisfy a change of one line in one section. +// +// The sub-table goes in after the last key of our section and before the +// next header. Every key written after a sub-table header belongs to it - +// the same failure class that put model_context_window inside [projects.*], +// where Codex never looked - so nothing of ours may follow it. +func codexBearerTokenMigrated(data []byte) ([]byte, bool) { + lines := strings.Split(string(data), "\n") + inNan, hasAuth, found, insertAt := false, false, false, -1 + var bearer []int + for i, line := range lines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "[") { + if found && insertAt < 0 && !hasAuth { + insertAt = i + } + if isCodexNaNAuthHeader(trimmed) { + hasAuth = true + } + inNan = isCodexNaNProviderHeader(trimmed) + continue + } + if !inNan { + continue + } + if key, _, ok := strings.Cut(trimmed, "="); ok && strings.TrimSpace(key) == "experimental_bearer_token" { + bearer = append(bearer, i) + found = true + } + } + if len(bearer) == 0 { + return data, false + } + + auth := []string(nil) + if !hasAuth { + if insertAt < 0 { + insertAt = len(lines) + } + auth = []string{} + if insertAt > 0 && strings.TrimSpace(lines[insertAt-1]) != "" { + auth = append(auth, "") + } + auth = append(auth, + "[model_providers.nan.auth]", + fmt.Sprintf("command = %q", nanExecutable()), + `args = ["key", "print"]`, + ) + if insertAt < len(lines) { + auth = append(auth, "") + } + } + + dropped := map[int]bool{} + for _, i := range bearer { + dropped[i] = true + } + out := make([]string, 0, len(lines)-len(bearer)+len(auth)) + for i, line := range lines { + if i == insertAt { + out = append(out, auth...) + } + if !dropped[i] { + out = append(out, line) + } + } + if insertAt == len(lines) { + out = append(out, auth...) + } + // The trailing empty element that carried the final newline is only + // still last when the auth block went in mid-file; keep the file + // newline-terminated either way. + if last := len(out) - 1; last >= 0 && out[last] != "" { + out = append(out, "") + } + return []byte(strings.Join(out, "\n")), true +} + // [model_providers.nan], and the quoted spelling of it that TOML also allows. func isCodexNaNProviderHeader(line string) bool { name := strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(line, "["), "]")) @@ -2737,6 +2868,28 @@ func isCodexNaNProviderHeader(line string) bool { return strings.Trim(strings.TrimPrefix(name, "model_providers."), `"'`) == "nan" } +// [model_providers.nan.auth], our sub-table, and the quoted spelling of it. +func isCodexNaNAuthHeader(line string) bool { + name := strings.TrimSpace(strings.TrimSuffix(strings.TrimPrefix(line, "["), "]")) + return strings.Trim(name, `"'`) == "model_providers.nan.auth" +} + +// Whether any of the sections nan-cli owns is in the file: the provider +// table, or the auth sub-table a config can be left holding on its own if the +// member deleted the section around it by hand. +func codexHasNaNSection(content string) bool { + for _, line := range strings.Split(content, "\n") { + trimmed := strings.TrimSpace(line) + if !strings.HasPrefix(trimmed, "[") { + continue + } + if isCodexNaNProviderHeader(trimmed) || isCodexNaNAuthHeader(trimmed) { + return true + } + } + return false +} + // ── Hermes ─────────────────────────────────────────────────────────────────── // // Every other tool here is configured by writing its file. Hermes is not, @@ -2979,7 +3132,7 @@ func removeCodexConfig(cfgPath string) error { // // The error waits until the end. A profile that will not delete - open in // an editor, locked by a running Codex - is a cosmetic failure, and - // returning it here would leave experimental_bearer_token sitting in + // returning it here would leave our provider section sitting in // config.toml after the member asked us to take it out. profileErr := removeCodexProfiles(filepath.Dir(cfgPath)) @@ -2989,7 +3142,7 @@ func removeCodexConfig(cfgPath string) error { } // Nothing of ours in it, so nothing to rewrite: this is called on every // sign-out, against a config.toml that may never have been ours at all. - if !strings.Contains(string(data), "[model_providers.nan]") { + if !codexHasNaNSection(string(data)) { return profileErr } lines := strings.Split(string(data), "\n") @@ -2997,17 +3150,18 @@ func removeCodexConfig(cfgPath string) error { inNanSection := false for _, line := range lines { trimmed := strings.TrimSpace(line) - if trimmed == "[model_providers.nan]" { - inNanSection = true - continue - } - if inNanSection { - // End of the nan section when a new section header appears. - if strings.HasPrefix(trimmed, "[") { - inNanSection = false - } else { + if strings.HasPrefix(trimmed, "[") { + // End of one of our sections when a new section header appears. + // The quoted spelling is the one TOML also allows, and the auth + // sub-table is ours too: it points at `nan key print`, and a + // sign-out that left it behind would leave Codex resolving a key + // that no longer exists. + inNanSection = isCodexNaNProviderHeader(trimmed) || isCodexNaNAuthHeader(trimmed) + if inNanSection { continue } + } else if inNanSection { + continue } out = append(out, line) }