Skip to content

Commit 17e3dc1

Browse files
authored
Merge pull request #3713 from dgageot/remove-keychain-pass-env-providers
feat(env): remove macOS keychain and pass secret providers
2 parents ad9f237 + 73027ed commit 17e3dc1

22 files changed

Lines changed: 112 additions & 323 deletions

File tree

cmd/root/doctor.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,8 @@ type doctorFlags struct {
9898

9999
// Test seams: sourcesForTests replaces the env-file + default secret-source
100100
// chain, dmrLister replaces dmr.ListModels, and loadUserConfig replaces
101-
// userconfig.Load so tests never exec `pass`/`security`/`docker model` or
102-
// read the developer's real configuration.
101+
// userconfig.Load so tests never exec `docker model` or credential helpers,
102+
// and never read the developer's real configuration.
103103
sourcesForTests []environment.Source
104104
dmrLister config.DMRModelLister
105105
loadUserConfig userConfigLoader

cmd/root/doctor_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import (
2121

2222
// withDoctorTestEnv wires a hermetic doctor command: a map-backed secret
2323
// source chain, a stubbed DMR lister, and an empty user config, so tests
24-
// never exec `pass`/`security`/`docker model` and never read the developer's
24+
// never exec `docker model` or credential helpers and never read the developer's
2525
// real configuration.
2626
func withDoctorTestEnv(env map[string]string, dmrModels []string, dmrErr error) doctorCmdOption {
2727
return func(f *doctorFlags) {

cmd/root/models.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ type modelRow struct {
5858
// modelsCmdOption pre-seeds the RuntimeConfig backing the models command
5959
// before its flags are wired. It is the seam tests use to inject a hermetic
6060
// env provider and an in-memory models.dev store, so listing models never
61-
// shells out to the OS keychain or reaches the network. Production calls
61+
// shells out to secret helper CLIs or reaches the network. Production calls
6262
// newModelsCmd with no options, leaving behavior unchanged.
6363
type modelsCmdOption func(*config.RuntimeConfig)
6464

cmd/root/models_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func testCatalog() *modelsdev.Database {
5353

5454
// withTestConfig injects a hermetic env provider and an in-memory models.dev
5555
// store into the models command. It keeps listing side-effect-free: without it
56-
// the real env provider chain shells out to the OS keychain / pass / 1Password
56+
// the real env provider chain consults Docker Desktop / 1Password
5757
// for every missing API key and the store fetches https://models.dev, making
5858
// the tests slow and non-parallelizable.
5959
func withTestConfig(env map[string]string) modelsCmdOption {

cmd/root/setup.go

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ type setupResult struct {
5353
}
5454

5555
// setupWizard drives the interactive model setup. The function fields are
56-
// seams: production wiring talks to the terminal, the OS secret stores, and
56+
// seams: production wiring talks to the terminal, the secret stores, and
5757
// Docker Model Runner, while tests inject scripted answers and fakes.
5858
//
5959
// in is buffered once at construction: a fresh bufio.Reader per prompt would
@@ -78,18 +78,17 @@ func newSetupCmd() *cobra.Command {
7878
Long: `Set up a model for docker agent, interactively.
7979
8080
Three paths:
81-
- Cloud provider: pick a provider, paste its API key, and choose where to
82-
store it (OS keychain, pass, or the docker agent env file). Picking
83-
chatgpt signs in with your ChatGPT account in the browser instead of
84-
asking for an API key.
81+
- Cloud provider: pick a provider, paste its API key, and store it in the
82+
docker agent env file (~/.config/cagent/.env). Picking chatgpt signs in
83+
with your ChatGPT account in the browser instead of asking for an API key.
8584
- Local model: check Docker Model Runner and pull a model. No API key needed.
8685
- OpenAI-compatible provider: register a custom endpoint (vLLM, LiteLLM,
8786
a corporate gateway, ...) with its API format and API key variable. The
8887
provider is saved to your user configuration and its models become
8988
usable everywhere via --model <name>/<model>.
9089
91-
Ends with the exact command to start chatting. Secret values are stored where
92-
you choose and never printed. Check the result anytime with 'docker agent doctor'.`,
90+
Ends with the exact command to start chatting. Secret values are never
91+
printed. Check the result anytime with 'docker agent doctor'.`,
9392
Example: ` docker-agent setup`,
9493
GroupID: "core",
9594
Args: cobra.NoArgs,
@@ -181,7 +180,7 @@ func (w *setupWizard) run(ctx context.Context) (*setupResult, error) {
181180
}
182181

183182
// setupCloudProvider walks the cloud path: pick a provider, paste its key,
184-
// pick a store, and persist the key there.
183+
// and persist it in a secret store.
185184
func (w *setupWizard) setupCloudProvider(ctx context.Context) (*setupResult, error) {
186185
providers := config.CloudProviderEnvVars()
187186

@@ -247,24 +246,33 @@ func (w *setupWizard) promptSecret(ctx context.Context, prompt string) (string,
247246
}
248247
}
249248

250-
// storeSecret asks where to store the key and persists it, re-asking when a
251-
// store fails (e.g. an uninitialized pass store) so the pasted key is not
252-
// lost to a storage hiccup.
249+
// storeSecret persists the key in a secret store. When several stores are
250+
// available it asks which one to use, re-asking when a store fails so the
251+
// pasted key is not lost to a storage hiccup.
253252
func (w *setupWizard) storeSecret(ctx context.Context, envVar, key string) error {
253+
if len(w.stores) == 0 {
254+
return errors.New("no secret store is available to save the key")
255+
}
254256
for {
255-
fmt.Fprintln(w.out)
256-
fmt.Fprintf(w.out, "Where should %s be stored?\n", envVar)
257-
for i, store := range w.stores {
258-
fmt.Fprintf(w.out, " %d. %s\n", i+1, store.Description())
259-
}
257+
store := w.stores[0]
258+
if len(w.stores) > 1 {
259+
fmt.Fprintln(w.out)
260+
fmt.Fprintf(w.out, "Where should %s be stored?\n", envVar)
261+
for i, s := range w.stores {
262+
fmt.Fprintf(w.out, " %d. %s\n", i+1, s.Description())
263+
}
260264

261-
choice, err := w.promptChoice(ctx, len(w.stores), 1)
262-
if err != nil {
263-
return err
265+
choice, err := w.promptChoice(ctx, len(w.stores), 1)
266+
if err != nil {
267+
return err
268+
}
269+
store = w.stores[choice-1]
264270
}
265-
store := w.stores[choice-1]
266271

267272
if err := store.Store(ctx, envVar, key); err != nil {
273+
if len(w.stores) == 1 {
274+
return fmt.Errorf("storing the key: %w", err)
275+
}
268276
fmt.Fprintf(w.out, "Could not store the key: %v\nPick another location, or press Ctrl+C to cancel.\n", err)
269277
continue
270278
}
@@ -654,9 +662,8 @@ func (f *runExecFlags) offerSetupOnNoModel(ctx context.Context, cmd *cobra.Comma
654662
}
655663

656664
// The run's env provider chain was built before the wizard stored the key,
657-
// so bridge it into the process environment for the retry. Keychain and
658-
// pass lookups are live either way; the config env file is not, when it
659-
// did not exist at chain construction.
665+
// so bridge it into the process environment for the retry: the config env
666+
// file is not live when it did not exist at chain construction.
660667
if result.EnvVar != "" {
661668
if err := os.Setenv(result.EnvVar, result.Value); err != nil {
662669
slog.WarnContext(ctx, "Failed to export the stored key for the retry", "env_var", result.EnvVar, "error", err)

cmd/root/setup_test.go

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,9 @@ func newTestWizard(answers string, keys []string, stores []environment.SecretSto
7777
func TestSetupWizard_CloudPathStoresKey(t *testing.T) {
7878
t.Parallel()
7979

80-
store := &fakeSecretStore{name: "keychain"}
81-
// cloud -> provider 1 (anthropic) -> store 1
82-
wizard, out, _ := newTestWizard("1\n1\n1\n", []string{"sk-cloud-key"}, []environment.SecretStore{store}, nil, nil)
80+
store := &fakeSecretStore{name: "config-env-file"}
81+
// cloud -> provider 1 (anthropic); a single store is used without a prompt
82+
wizard, out, _ := newTestWizard("1\n1\n", []string{"sk-cloud-key"}, []environment.SecretStore{store}, nil, nil)
8383

8484
result, err := wizard.run(t.Context())
8585
require.NoError(t, err)
@@ -90,7 +90,8 @@ func TestSetupWizard_CloudPathStoresKey(t *testing.T) {
9090
assert.Equal(t, "anthropic/claude-sonnet-4-6", result.Model)
9191

9292
output := out.String()
93-
assert.Contains(t, output, "Stored ANTHROPIC_API_KEY in the keychain (fake).")
93+
assert.NotContains(t, output, "Where should", "a single store must not prompt for a location")
94+
assert.Contains(t, output, "Stored ANTHROPIC_API_KEY in the config-env-file (fake).")
9495
assert.Contains(t, output, "docker agent run")
9596
assert.Contains(t, output, "--model anthropic/claude-sonnet-4-6")
9697
assert.Contains(t, output, "docker agent doctor")
@@ -100,9 +101,9 @@ func TestSetupWizard_CloudPathStoresKey(t *testing.T) {
100101
func TestSetupWizard_DefaultsSelectCloudAndFirstEntries(t *testing.T) {
101102
t.Parallel()
102103

103-
store := &fakeSecretStore{name: "keychain"}
104-
// Empty answers take every default: cloud, first provider, first store.
105-
wizard, _, _ := newTestWizard("\n\n\n", []string{"sk-key"}, []environment.SecretStore{store}, nil, nil)
104+
store := &fakeSecretStore{name: "config-env-file"}
105+
// Empty answers take every default: cloud, first provider.
106+
wizard, _, _ := newTestWizard("\n\n", []string{"sk-key"}, []environment.SecretStore{store}, nil, nil)
106107

107108
result, err := wizard.run(t.Context())
108109
require.NoError(t, err)
@@ -115,7 +116,7 @@ func TestSetupWizard_DefaultsSelectCloudAndFirstEntries(t *testing.T) {
115116
func TestSetupWizard_CloudPathRetriesFailedStore(t *testing.T) {
116117
t.Parallel()
117118

118-
broken := &fakeSecretStore{name: "pass", err: errors.New("password store is empty")}
119+
broken := &fakeSecretStore{name: "broken-store", err: errors.New("store is unavailable")}
119120
working := &fakeSecretStore{name: "config-env-file"}
120121
// cloud -> provider 1 -> store 1 (fails) -> store 2 (succeeds)
121122
wizard, out, _ := newTestWizard("1\n1\n1\n2\n", []string{"sk-key"},
@@ -124,15 +125,37 @@ func TestSetupWizard_CloudPathRetriesFailedStore(t *testing.T) {
124125
result, err := wizard.run(t.Context())
125126
require.NoError(t, err)
126127

127-
assert.Contains(t, out.String(), "Could not store the key: password store is empty")
128+
assert.Contains(t, out.String(), "Could not store the key: store is unavailable")
128129
assert.Equal(t, "sk-key", working.stored[result.EnvVar])
129130
}
130131

132+
func TestSetupWizard_CloudPathSingleStoreFailureIsFatal(t *testing.T) {
133+
t.Parallel()
134+
135+
broken := &fakeSecretStore{name: "config-env-file", err: errors.New("disk full")}
136+
wizard, _, _ := newTestWizard("1\n1\n", []string{"sk-key"}, []environment.SecretStore{broken}, nil, nil)
137+
138+
_, err := wizard.run(t.Context())
139+
require.Error(t, err)
140+
assert.Contains(t, err.Error(), "storing the key")
141+
assert.Contains(t, err.Error(), "disk full")
142+
}
143+
144+
func TestSetupWizard_CloudPathNoStoresIsFatal(t *testing.T) {
145+
t.Parallel()
146+
147+
wizard, _, _ := newTestWizard("1\n1\n", []string{"sk-key"}, nil, nil, nil)
148+
149+
_, err := wizard.run(t.Context())
150+
require.Error(t, err)
151+
assert.Contains(t, err.Error(), "no secret store is available")
152+
}
153+
131154
func TestSetupWizard_CloudPathReasksOnEmptyKey(t *testing.T) {
132155
t.Parallel()
133156

134-
store := &fakeSecretStore{name: "keychain"}
135-
wizard, out, _ := newTestWizard("1\n1\n1\n", []string{" ", "sk-key"}, []environment.SecretStore{store}, nil, nil)
157+
store := &fakeSecretStore{name: "config-env-file"}
158+
wizard, out, _ := newTestWizard("1\n1\n", []string{" ", "sk-key"}, []environment.SecretStore{store}, nil, nil)
136159

137160
_, err := wizard.run(t.Context())
138161
require.NoError(t, err)
@@ -148,8 +171,8 @@ func TestSetupWizard_ChatGPTPathRunsBrowserSignIn(t *testing.T) {
148171
idx := slices.IndexFunc(providers, func(p config.ProviderEnvVars) bool { return p.Provider == "chatgpt" })
149172
require.GreaterOrEqual(t, idx, 0, "chatgpt must be offered by the wizard")
150173

151-
store := &fakeSecretStore{name: "keychain"}
152-
// cloud -> chatgpt: no key prompt, no store prompt.
174+
store := &fakeSecretStore{name: "config-env-file"}
175+
// cloud -> chatgpt: no key prompt, no store involved.
153176
wizard, out, _ := newTestWizard(fmt.Sprintf("1\n%d\n", idx+1), nil, []environment.SecretStore{store}, nil, nil)
154177
loginCalled := false
155178
wizard.chatgptLogin = func(_ context.Context, _ io.Writer) (*chatgpt.LoginResult, error) {
@@ -274,10 +297,10 @@ func (r *customProviderRecorder) save(name string, provider latest.ProviderConfi
274297
func TestSetupWizard_CustomPathSavesProviderAndKey(t *testing.T) {
275298
t.Parallel()
276299

277-
store := &fakeSecretStore{name: "keychain"}
300+
store := &fakeSecretStore{name: "config-env-file"}
278301
recorder := &customProviderRecorder{}
279-
// custom -> name -> base URL -> format 2 (responses) -> env var -> store 1 -> model (default)
280-
wizard, out, _ := newTestWizard("3\nmyprovider\nhttps://llm.corp.example.com/v1\n2\nMYPROVIDER_API_KEY\n1\n\n",
302+
// custom -> name -> base URL -> format 2 (responses) -> env var -> model (default)
303+
wizard, out, _ := newTestWizard("3\nmyprovider\nhttps://llm.corp.example.com/v1\n2\nMYPROVIDER_API_KEY\n\n",
281304
[]string{"sk-custom-key"}, []environment.SecretStore{store}, nil, nil)
282305
wizard.saveProvider = recorder.save
283306
wizard.listModels = func(_ context.Context, baseURL, token string) []string {
@@ -362,9 +385,9 @@ func TestSetupWizard_CustomPathValidatesNameAndURL(t *testing.T) {
362385
func TestSetupWizard_CustomPathReasksOnInvalidEnvVarName(t *testing.T) {
363386
t.Parallel()
364387

365-
store := &fakeSecretStore{name: "keychain"}
388+
store := &fakeSecretStore{name: "config-env-file"}
366389
recorder := &customProviderRecorder{}
367-
wizard, out, _ := newTestWizard("3\nmyprovider\nhttps://ok.example.com/v1\n1\nMY BAD VAR\nMY_KEY\n1\nm1\n",
390+
wizard, out, _ := newTestWizard("3\nmyprovider\nhttps://ok.example.com/v1\n1\nMY BAD VAR\nMY_KEY\nm1\n",
368391
[]string{"sk-key"}, []environment.SecretStore{store}, nil, nil)
369392
wizard.saveProvider = recorder.save
370393

docs/community/troubleshooting/index.md

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,9 @@ The following environment variables must be set:
6060
- ANTHROPIC_API_KEY
6161
6262
Provide them using any of these sources:
63-
- Shell environment: export ANTHROPIC_API_KEY=<value>
64-
- Env file: docker agent run --env-from-file <file> ...
65-
- pass: pass insert ANTHROPIC_API_KEY
66-
- macOS Keychain: security add-generic-password -a "$USER" -s ANTHROPIC_API_KEY -w
63+
- Shell environment: export ANTHROPIC_API_KEY=<value>
64+
- Env file: docker agent run --env-from-file <file> ...
65+
- Docker Agent env file: docker agent setup (stores the key in ~/.config/cagent/.env)
6766
6867
See https://docs.docker.com/ai/docker-agent/guides/secrets/ for details.
6968
```

docs/configuration/overview/index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ Models can be referenced inline or defined in the `models` section:
129129

130130
## Environment Variables
131131

132-
API keys and secrets are read from environment variables — never stored in config files. See [Managing Secrets](../../guides/secrets/index.md) for all the ways to provide credentials (env files, Docker Compose secrets, macOS Keychain, `pass`):
132+
API keys and secrets are read from environment variables — never stored in config files. See [Managing Secrets](../../guides/secrets/index.md) for all the ways to provide credentials (env files, Docker Compose secrets, the Docker Agent env file):
133133

134134
| Variable | Provider |
135135
| -------------------------- | --------------------------------------------------- |

docs/features/cli/index.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ $ docker agent toolsets --format json | jq # machine-readable (type,
229229

230230
### `docker agent setup`
231231

232-
Set up a model interactively. Three paths: pick a cloud provider, paste its API key, and choose where to store it (macOS Keychain, `pass`, or the Docker Agent env file `~/.config/cagent/.env`); check Docker Model Runner and pull a local model (no API key needed); or register a custom OpenAI-compatible provider (endpoint URL, API format, and API key variable) saved to your [user configuration](../../providers/custom/index.md#global-providers-user-configuration) so its models work everywhere via `--model <name>/<model>`. Ends with the exact command to start chatting. Secret values are never printed.
232+
Set up a model interactively. Three paths: pick a cloud provider, paste its API key, and store it in the Docker Agent env file `~/.config/cagent/.env`; check Docker Model Runner and pull a local model (no API key needed); or register a custom OpenAI-compatible provider (endpoint URL, API format, and API key variable) saved to your [user configuration](../../providers/custom/index.md#global-providers-user-configuration) so its models work everywhere via `--model <name>/<model>`. Ends with the exact command to start chatting. Secret values are never printed.
233233

234234
The wizard is also offered automatically when an interactive run finds no usable model (decline-able; set `DOCKER_AGENT_NO_SETUP=1` to suppress the offer).
235235

@@ -239,7 +239,7 @@ $ docker agent setup
239239

240240
### `docker agent doctor`
241241

242-
Diagnose the model and credential setup. Reports which model providers have credentials and where each credential comes from (shell environment, env file, pass, keychain, …), whether Docker Model Runner is reachable and which models are pulled, and which model the `auto` selection would pick. Secret values are never printed. Exits with a non-zero status when an issue would prevent an agent from running, which makes it usable in scripts and CI.
242+
Diagnose the model and credential setup. Reports which model providers have credentials and where each credential comes from (shell environment, env file, Docker Desktop, …), whether Docker Model Runner is reachable and which models are pulled, and which model the `auto` selection would pick. Secret values are never printed. Exits with a non-zero status when an issue would prevent an agent from running, which makes it usable in scripts and CI.
243243

244244
```bash
245245
$ docker agent doctor [agent-file|registry-ref] [flags]

docs/getting-started/set-up-a-model/index.md

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,10 @@ That lasts for the current shell session. To set a key up once, use any other bu
5353
$ echo 'ANTHROPIC_API_KEY=sk-ant-...' > .env
5454
$ docker agent run --env-from-file .env
5555

56-
# pass password manager (Linux, macOS)
57-
$ pass insert ANTHROPIC_API_KEY
58-
59-
# macOS Keychain
60-
$ security add-generic-password -a "$USER" -s ANTHROPIC_API_KEY -w
56+
# Docker Agent env file, read automatically on every run
57+
# (`docker agent setup` writes it for you with owner-only permissions)
58+
$ echo 'ANTHROPIC_API_KEY=sk-ant-...' >> ~/.config/cagent/.env
59+
$ chmod 600 ~/.config/cagent/.env
6160
```
6261

6362
The entry name must match the environment variable the provider expects. [Managing Secrets](../../guides/secrets/index.md) covers every source (Docker Compose secrets, credential helpers, 1Password references) and the order they are checked in.

0 commit comments

Comments
 (0)