From 63ce8e619b4061f6e58821439f482387b5f67ad1 Mon Sep 17 00:00:00 2001 From: Wilber Carrascal Date: Mon, 21 Sep 2026 10:21:43 -0500 Subject: [PATCH 1/3] feat(key): imprimir la key para que la lea el config de otra herramienta Los configs de Codex y de Pi guardan hoy la key en texto plano. La salida de este cambio es que esos dos ficheros referencien un comando en vez de llevarla dentro, y para eso hace falta un comando que la saque de donde ya vive: ~/.config/nan/session.json, que esta en 0600 y que nan auth logout ya borra. Lo que importa del subcomando es como falla. Quien lo invoca lee su stdout y autentica con lo que salga: si sin sesion imprimiera una linea vacia y saliera con cero, la herramienta mandaria "Bearer " y el fallo se veria como una caida de autenticacion en el otro programa, no como esto. Por eso una sesion sin key se trata igual que no tener sesion - nada por stdout y salida distinta de cero - y por eso la key no entra en ningun mensaje de error. --- cmd/key.go | 39 ++++++++++++++++++++ cmd/key_test.go | 95 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+) create mode 100644 cmd/key.go create mode 100644 cmd/key_test.go 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()) + } +} From 4499b285b7dc4917875bf1f425a4eb4a914b3733 Mon Sep 17 00:00:00 2001 From: Wilber Carrascal Date: Mon, 21 Sep 2026 10:51:02 -0500 Subject: [PATCH 2/3] fix(codex): leer la key de un comando en vez de escribirla en el config El config.toml guardaba la key en texto plano con experimental_bearer_token. Es un fichero que el miembro pega en un issue cuando pide ayuda, y para quien gestiona sus dotfiles es ademas un destino de este mismo repo: writeConfigFile sigue los symlinks a proposito. La key pasa a estar solo en ~/.config/nan/session.json, y Codex la lee por stdout de `nan key print`. Medido contra Codex de verdad antes de escribirlo, porque la forma importa: auth.command se ejecuta con exec, nunca con un shell. Una cadena tipo "cat ~/.codex/nan/credentials" no arranca - "failed to start: No such file or directory" - y cuando falla Codex manda igual la peticion, sin cabecera Authorization, y reintenta. Un comando roto se ve como una caida de autenticacion, no como un config mal escrito. Por eso command es la ruta absoluta pelada al binario y los argumentos van en args. Verificado de 0.120.0 a 0.155.1. La doc de Codex prohibe combinar auth con experimental_bearer_token, asi que la migracion borra la linea en vez de anadir al lado. Va en el camino de salida temprana que ya existia, el mismo por el que pasa la reparacion del wire_api: un miembro que no vuelva a pasar por Setup se quedaria con la key en texto plano para siempre. Se hace linea a linea, no con una vuelta por una libreria TOML, por el mismo motivo que la reparacion del wire_api: la vuelta reflowea los comentarios, el espaciado y las comillas del miembro para cambiar una linea. Y la sub-tabla va justo antes de la siguiente cabecera, nunca antes de claves nuestras: toda clave escrita despues de una sub-tabla le pertenece, que es como model_context_window acabo dentro de [projects.*]. De paso, removeCodexConfig cortaba la seccion en la primera cabecera pero se quedaba con ella, asi que un [model_providers.nan.auth] sobrevivia al sign-out y Codex seguia resolviendo una key que ya no existe. Ahora reconoce las dos cabeceras, y tambien la forma entrecomillada que el resto del fichero ya aceptaba. --- internal/tui/config_test.go | 196 +++++++++++++++++++++++++++++++++- internal/tui/security_test.go | 39 ++++++- internal/tui/tui.go | 174 +++++++++++++++++++++++++++--- 3 files changed, 386 insertions(+), 23 deletions(-) diff --git a/internal/tui/config_test.go b/internal/tui/config_test.go index 1879c7a..4866931 100644 --- a/internal/tui/config_test.go +++ b/internal/tui/config_test.go @@ -329,8 +329,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 +413,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 restoreCodexAuthCommand(t *testing.T, path string) { + t.Helper() + original := codexAuthCommand + codexAuthCommand = func() string { return path } + t.Cleanup(func() { codexAuthCommand = 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") + restoreCodexAuthCommand(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") + restoreCodexAuthCommand(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") + restoreCodexAuthCommand(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) { + restoreCodexAuthCommand(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 +1098,15 @@ 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 is the one exception: its config carries a reference to + // `nan key print` instead of the key itself. + if name == "Codex" { + if !strings.Contains(string(data), `args = ["key", "print"]`) { + 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..082b1c9 100644 --- a/internal/tui/security_test.go +++ b/internal/tui/security_test.go @@ -34,9 +34,24 @@ 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" { + // The one exception: Codex's config carries a reference to + // `nan key print` instead of the key, and that reference is what + // says it was configured at all. + if strings.Contains(content, testKey) { + t.Errorf("%s carries the literal key", name) + continue + } + if !strings.Contains(content, `args = ["key", "print"]`) { + 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 +87,14 @@ 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" { + // The key was never in this file; the command reference is what + // has to survive the second run untouched. + if !strings.Contains(content, `args = ["key", "print"]`) { + 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 +167,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..a4114fc 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -2424,7 +2424,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 +2447,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 +2478,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, codexAuthCommand()) if err := writeConfigFile(cfgPath, []byte(content)); err != nil { return err } @@ -2479,9 +2495,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"] +`, codexAuthCommand()) content := strings.TrimRight(string(data), "\n") + "\n" + section if err := writeConfigFile(cfgPath, []byte(withCodexContextWindow(content, codexModel.Context))); err != nil { return err @@ -2543,6 +2562,21 @@ func removeCodexProfiles(codexHome string) error { return nil } +// What Codex executes 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 codexAuthCommand = 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 +2762,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", codexAuthCommand()), + `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 +2856,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 +3120,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 +3130,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 +3138,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) } From 36e7380aa3e11c858fe4b894c6f2230b853fc74c Mon Sep 17 00:00:00 2001 From: Wilber Carrascal Date: Mon, 21 Sep 2026 10:56:27 -0500 Subject: [PATCH 3/3] fix(pi): leer la key de un comando en vez de escribirla en el config models.json guardaba la key en texto plano dentro de providers.nan.apiKey, con el mismo problema que el config de Codex: es un fichero que el miembro pega en un issue y que para quien gestiona dotfiles vive en un repo. Pi acepta un valor que empieza por "!" y ejecuta el resto como comando, asi que el provider apunta a `nan key print` y la key se queda solo en ~/.config/nan/session.json. De la doc de Pi, el valor admite ademas $$ como "$" literal y $! como "!" literal, y el escapado se hace en ese orden a proposito: primero todos los $ a $$, y despues los ! a $!. Al reves, el $ que inserta el segundo paso se comeria el escape del primero. nanExecutable pasa a llamarse asi porque ya no lo usa solo Codex. Queda un limite conocido: una ruta con espacios no se entrecomilla. Pi resuelve el valor con variantes de shell segun la plataforma y no hay comillas que sirvan en todas, asi que se deja escrito en vez de inventar una que funcione en una sola. Una instalacion en /usr/local/bin no lo toca. --- internal/tui/config_test.go | 61 ++++++++++++++++++++++++++++------- internal/tui/security_test.go | 25 ++++++++++---- internal/tui/tui.go | 32 ++++++++++++------ 3 files changed, 89 insertions(+), 29 deletions(-) diff --git a/internal/tui/config_test.go b/internal/tui/config_test.go index 4866931..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":[]}}}` @@ -420,11 +453,11 @@ func TestCodexConfigDoesNotTouchAnExistingChoice(t *testing.T) { // 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 restoreCodexAuthCommand(t *testing.T, path string) { +func restoreNanExecutable(t *testing.T, path string) { t.Helper() - original := codexAuthCommand - codexAuthCommand = func() string { return path } - t.Cleanup(func() { codexAuthCommand = original }) + 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 @@ -433,7 +466,7 @@ func restoreCodexAuthCommand(t *testing.T, path string) { // exists - reached on the same early return the wire repair is. func TestCodexConfigMigratesThePlaintextKeyToTheKeyCommand(t *testing.T) { path := tempConfig(t, "config.toml") - restoreCodexAuthCommand(t, "/usr/local/bin/nan") + restoreNanExecutable(t, "/usr/local/bin/nan") existing := `model = "glm5.3-flash" [model_providers.old] @@ -496,7 +529,7 @@ trust_level = "trusted" // of them. func TestCodexMigrationIsLineSurgeryNotARoundTrip(t *testing.T) { path := tempConfig(t, "config.toml") - restoreCodexAuthCommand(t, "/usr/local/bin/nan") + restoreNanExecutable(t, "/usr/local/bin/nan") existing := `# my notes, kept however I wrote them model = "gpt-5" # trailing comment @@ -534,7 +567,7 @@ args = ["key", "print"] // replaced with whatever this run of the CLI happens to resolve to. func TestCodexConfigAlreadyUsingTheKeyCommandIsNotRewritten(t *testing.T) { path := tempConfig(t, "config.toml") - restoreCodexAuthCommand(t, "/elsewhere/nan") + restoreNanExecutable(t, "/elsewhere/nan") existing := `model = "gpt-5" [model_providers.nan] @@ -561,7 +594,7 @@ args = ["key", "print"] // this writer may put the literal key into the file any more - the starter // config and the append included. func TestCodexConfigWritesACommandReferenceNotTheKey(t *testing.T) { - restoreCodexAuthCommand(t, "/usr/local/bin/nan") + restoreNanExecutable(t, "/usr/local/bin/nan") path := tempConfig(t, "config.toml") if err := writeCodexConfig(path, testKey); err != nil { @@ -1098,10 +1131,14 @@ func TestConfigureToolsWritesEveryEnabledTool(t *testing.T) { t.Errorf("%s: written without the NaN base URL", name) } if !strings.Contains(string(data), testKey) { - // Codex is the one exception: its config carries a reference to - // `nan key print` instead of the key itself. - if name == "Codex" { - if !strings.Contains(string(data), `args = ["key", "print"]`) { + // 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 { diff --git a/internal/tui/security_test.go b/internal/tui/security_test.go index 082b1c9..864a9b5 100644 --- a/internal/tui/security_test.go +++ b/internal/tui/security_test.go @@ -35,15 +35,19 @@ func TestWritingAKeyTightensAConfigThatWasWideOpen(t *testing.T) { } for name, path := range toolPaths(home) { content := readFile(t, path) - if name == "Codex" { - // The one exception: Codex's config carries a reference to + 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 it was configured at all. + // says they were configured at all. if strings.Contains(content, testKey) { t.Errorf("%s carries the literal key", name) continue } - if !strings.Contains(content, `args = ["key", "print"]`) { + 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 } @@ -88,10 +92,17 @@ func TestConfiguringTightensAToolThatNeedsNothingWritten(t *testing.T) { } for name, path := range paths { content := readFile(t, path) - if name == "Codex" { - // The key was never in this file; the command reference is what + 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, `args = ["key", "print"]`) { + 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) { diff --git a/internal/tui/tui.go b/internal/tui/tui.go index a4114fc..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 @@ -2483,7 +2495,7 @@ wire_api = "responses" [model_providers.nan.auth] command = %q args = ["key", "print"] -`, codexModel.ID, codexModel.Context, codexAuthCommand()) +`, codexModel.ID, codexModel.Context, nanExecutable()) if err := writeConfigFile(cfgPath, []byte(content)); err != nil { return err } @@ -2500,7 +2512,7 @@ wire_api = "responses" [model_providers.nan.auth] command = %q args = ["key", "print"] -`, codexAuthCommand()) +`, nanExecutable()) content := strings.TrimRight(string(data), "\n") + "\n" + section if err := writeConfigFile(cfgPath, []byte(withCodexContextWindow(content, codexModel.Context))); err != nil { return err @@ -2562,15 +2574,15 @@ func removeCodexProfiles(codexHome string) error { return nil } -// What Codex executes 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. +// 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 codexAuthCommand = func() string { +var nanExecutable = func() string { if exe, err := os.Executable(); err == nil { return exe } @@ -2814,7 +2826,7 @@ func codexBearerTokenMigrated(data []byte) ([]byte, bool) { } auth = append(auth, "[model_providers.nan.auth]", - fmt.Sprintf("command = %q", codexAuthCommand()), + fmt.Sprintf("command = %q", nanExecutable()), `args = ["key", "print"]`, ) if insertAt < len(lines) {