From c01ff6da67db76dca8fe2cacdd43d3a7d34be677 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Tue, 9 Jun 2026 17:02:48 +0200 Subject: [PATCH 1/2] feat: add 1Password CLI integration for secret resolution --- pkg/environment/default.go | 6 +- pkg/environment/onepassword.go | 60 ++++++++++++++++++ pkg/environment/onepassword_test.go | 97 +++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 pkg/environment/onepassword.go create mode 100644 pkg/environment/onepassword_test.go diff --git a/pkg/environment/default.go b/pkg/environment/default.go index 25737cff4..e94c1cf6c 100644 --- a/pkg/environment/default.go +++ b/pkg/environment/default.go @@ -7,6 +7,8 @@ import ( // NewDefaultProvider creates a provider chain with OS env, run secrets, // credential helper (if configured), Docker Desktop, pass, and keychain providers. +// The whole chain is wrapped so that values shaped like "op://..." are resolved +// as 1Password secret references through the `op` CLI. // // When running inside a Docker sandbox (detected via SANDBOX_VM_ID), a // [SandboxTokenProvider] is prepended so that DOCKER_TOKEN is read from the @@ -48,5 +50,7 @@ func NewDefaultProvider() Provider { providers = append(providers, keychainProvider) } - return NewMultiProvider(providers...) + // Resolve any "op://" secret references through the 1Password CLI, + // regardless of which provider returned the value. + return NewOnePasswordProvider(NewMultiProvider(providers...)) } diff --git a/pkg/environment/onepassword.go b/pkg/environment/onepassword.go new file mode 100644 index 000000000..330a2d2e8 --- /dev/null +++ b/pkg/environment/onepassword.go @@ -0,0 +1,60 @@ +package environment + +import ( + "context" + "log/slog" + "strings" +) + +// onePasswordPrefix marks an environment value as a 1Password secret reference +// (e.g. "op://vault/item/field") that must be resolved with the `op` CLI. +const onePasswordPrefix = "op://" + +// OnePasswordProvider decorates another Provider and resolves 1Password secret +// references. When the wrapped provider returns a value starting with "op://", +// the value is treated as a secret reference and resolved using the `op read` +// CLI command. All other values are passed through unchanged. +type OnePasswordProvider struct { + provider Provider + // resolve turns a "op://..." reference into its secret value. It is a field + // so tests can inject a fake resolver without relying on the `op` binary. + resolve func(ctx context.Context, reference string) (string, bool) +} + +type OnePasswordNotAvailableError struct{} + +func (OnePasswordNotAvailableError) Error() string { + return "op (1Password CLI) is not installed" +} + +// NewOnePasswordProvider wraps provider so that "op://" references are resolved +// with the `op` CLI. If the `op` binary is not installed, provider is returned +// unchanged so that values are passed through untouched. +func NewOnePasswordProvider(provider Provider) Provider { + path, err := lookupBinary("op", OnePasswordNotAvailableError{}) + if err != nil { + return provider + } + + return &OnePasswordProvider{ + provider: provider, + resolve: func(ctx context.Context, reference string) (string, bool) { + return runCommand(ctx, "1password", path, "read", reference) + }, + } +} + +func (p *OnePasswordProvider) Get(ctx context.Context, name string) (string, bool) { + value, found := p.provider.Get(ctx, name) + if !found || !strings.HasPrefix(value, onePasswordPrefix) { + return value, found + } + + resolved, ok := p.resolve(ctx, value) + if !ok { + slog.WarnContext(ctx, "Failed to resolve 1Password secret reference", "name", name) + return "", false + } + + return resolved, true +} diff --git a/pkg/environment/onepassword_test.go b/pkg/environment/onepassword_test.go new file mode 100644 index 000000000..2e37c8226 --- /dev/null +++ b/pkg/environment/onepassword_test.go @@ -0,0 +1,97 @@ +package environment + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestOnePasswordProvider_Get(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + stored map[string]string + resolve func(ctx context.Context, reference string) (string, bool) + lookup string + wantValue string + wantFound bool + wantRefSeen string + }{ + { + name: "plain value is passed through", + stored: map[string]string{"API_KEY": "plain-secret"}, + lookup: "API_KEY", + wantValue: "plain-secret", + wantFound: true, + }, + { + name: "op reference is resolved", + stored: map[string]string{"API_KEY": "op://vault/item/field"}, + lookup: "API_KEY", + wantValue: "resolved-secret", + wantFound: true, + wantRefSeen: "op://vault/item/field", + }, + { + name: "missing variable is not resolved", + stored: map[string]string{}, + lookup: "API_KEY", + wantFound: false, + }, + { + name: "failed resolution reports not found", + stored: map[string]string{"API_KEY": "op://vault/item/field"}, + resolve: func(context.Context, string) (string, bool) { + return "", false + }, + lookup: "API_KEY", + wantFound: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var refSeen string + resolve := tt.resolve + if resolve == nil { + resolve = func(_ context.Context, reference string) (string, bool) { + refSeen = reference + return "resolved-secret", true + } + } + + provider := &OnePasswordProvider{ + provider: NewMapEnvProvider(tt.stored), + resolve: resolve, + } + + value, found := provider.Get(t.Context(), tt.lookup) + assert.Equal(t, tt.wantFound, found) + assert.Equal(t, tt.wantValue, value) + if tt.wantRefSeen != "" { + assert.Equal(t, tt.wantRefSeen, refSeen) + } + }) + } +} + +func TestNewOnePasswordProvider_NoBinaryPassesThrough(t *testing.T) { + t.Parallel() + + // On a system without `op`, the wrapped provider is returned untouched and + // "op://" values are passed through unchanged. + if _, err := lookupBinary("op", OnePasswordNotAvailableError{}); err == nil { + t.Skip("op binary is installed; skipping pass-through test") + } + + base := NewMapEnvProvider(map[string]string{"API_KEY": "op://vault/item/field"}) + provider := NewOnePasswordProvider(base) + + value, found := provider.Get(t.Context(), "API_KEY") + assert.True(t, found) + assert.Equal(t, "op://vault/item/field", value) +} From 5d3e82b3f7d833e8dd53de743cf37fe3a8bb561a Mon Sep 17 00:00:00 2001 From: David Gageot Date: Tue, 9 Jun 2026 17:08:25 +0200 Subject: [PATCH 2/2] harden 1Password provider: prevent silent pass-through and PATH hijacking --- docs/guides/secrets/index.md | 23 ++++++++++++++++++++++- pkg/environment/cmd_provider.go | 15 ++++++++++++--- pkg/environment/onepassword.go | 22 +++++++++++++--------- pkg/environment/onepassword_test.go | 18 +++++++----------- 4 files changed, 54 insertions(+), 24 deletions(-) diff --git a/docs/guides/secrets/index.md b/docs/guides/secrets/index.md index 5d40e8063..28ac98eb3 100644 --- a/docs/guides/secrets/index.md +++ b/docs/guides/secrets/index.md @@ -1,6 +1,6 @@ --- title: "Managing Secrets" -description: "How to securely provide API keys and credentials to docker-agent using environment variables, env files, Docker Compose secrets, macOS Keychain, and pass." +description: "How to securely provide API keys and credentials to docker-agent using environment variables, env files, Docker Compose secrets, macOS Keychain, pass, and 1Password references." permalink: /guides/secrets/ --- @@ -23,6 +23,8 @@ docker-agent needs API keys to talk to model providers (OpenAI, Anthropic, etc.) The first provider that has a value wins. You can mix and match — for example, use environment variables for one key and Keychain for another. +Whatever provider returns the value, if that value looks like a [1Password secret reference](#1password-references) (it starts with `op://`), docker-agent resolves it through the `op` CLI before handing it to a model provider or tool. + When docker-agent runs inside a Docker sandbox (detected via `SANDBOX_VM_ID`), a sandbox token provider is prepended to the chain so that `DOCKER_TOKEN` is read from a continuously-refreshed file instead of a stale environment variable. ## Environment Variables @@ -216,6 +218,24 @@ security delete-generic-password -s ANTHROPIC_API_KEY Once stored, docker-agent finds the secret automatically — no flags or config needed. +## 1Password References + +Any secret value resolved through the chain above can be a **1Password secret reference** instead of the literal secret. If the value starts with `op://`, docker-agent resolves it by invoking the [1Password CLI](https://developer.1password.com/docs/cli/) (`op read `) and uses the result. + +This works with every provider — most commonly an environment variable or env file: + +```bash +export OPENAI_API_KEY="op://Personal/OpenAI/api-key" +docker agent run agent.yaml +``` + +References follow the `op:////` format. Make sure the `op` CLI is installed and you are signed in (`op signin`) so that non-interactive reads succeed. + +
+
Behaviour when resolution fails
+

If the value starts with op:// but the op CLI is not installed, or the reference cannot be read (not signed in, wrong path, locked vault), docker-agent logs a warning and treats the variable as unset — it never forwards the raw op:// reference to a model provider or tool.

+
+ ## Choosing a Method | Method | Best for | Setup effort | @@ -225,6 +245,7 @@ Once stored, docker-agent finds the secret automatically — no flags or config | Docker Compose secrets | Containerized deployments, CI/CD | Medium | | `pass` | Linux/macOS, GPG-based workflows | Medium | | macOS Keychain | macOS local development | Low | +| 1Password references (`op://`) | Teams already using 1Password | Low | You can combine methods. For example, store long-lived provider keys in macOS Keychain and pass project-specific MCP tokens via env files. diff --git a/pkg/environment/cmd_provider.go b/pkg/environment/cmd_provider.go index 9f72b7b7d..11c967652 100644 --- a/pkg/environment/cmd_provider.go +++ b/pkg/environment/cmd_provider.go @@ -6,6 +6,7 @@ import ( "errors" "log/slog" "os/exec" + "path/filepath" "strings" ) @@ -34,10 +35,18 @@ func runCommand(ctx context.Context, logLabel, name string, args ...string) (str // to avoid TOCTOU races and PATH hijacking. func lookupBinary(name string, notFoundErr error) (string, error) { path, err := exec.LookPath(name) - if err != nil && !errors.Is(err, exec.ErrNotFound) { - slog.Warn("failed to lookup `"+name+"` binary", "error", err) + if err != nil { + // exec.ErrDot means the binary was only found via an unsafe relative + // PATH entry (or "."). Treat it like "not found" so we never run an + // attacker-controlled binary from the working directory (CWE-426). + if !errors.Is(err, exec.ErrNotFound) && !errors.Is(err, exec.ErrDot) { + slog.Warn("failed to lookup `"+name+"` binary", "error", err) + } + return "", notFoundErr } - if path == "" { + // Defensively require an absolute path so the resolved binary cannot be + // hijacked via PATH or the current working directory. + if !filepath.IsAbs(path) { return "", notFoundErr } return path, nil diff --git a/pkg/environment/onepassword.go b/pkg/environment/onepassword.go index 330a2d2e8..b4f096bde 100644 --- a/pkg/environment/onepassword.go +++ b/pkg/environment/onepassword.go @@ -28,20 +28,24 @@ func (OnePasswordNotAvailableError) Error() string { } // NewOnePasswordProvider wraps provider so that "op://" references are resolved -// with the `op` CLI. If the `op` binary is not installed, provider is returned -// unchanged so that values are passed through untouched. +// with the `op` CLI. The `op` binary is looked up lazily, only when a reference +// is actually encountered, so an "op://" value is never silently passed through +// as if it were a real secret. func NewOnePasswordProvider(provider Provider) Provider { + return &OnePasswordProvider{ + provider: provider, + resolve: resolveOnePasswordReference, + } +} + +func resolveOnePasswordReference(ctx context.Context, reference string) (string, bool) { path, err := lookupBinary("op", OnePasswordNotAvailableError{}) if err != nil { - return provider + slog.WarnContext(ctx, "Cannot resolve 1Password secret reference: op (1Password CLI) is not installed") + return "", false } - return &OnePasswordProvider{ - provider: provider, - resolve: func(ctx context.Context, reference string) (string, bool) { - return runCommand(ctx, "1password", path, "read", reference) - }, - } + return runCommand(ctx, "1password", path, "read", reference) } func (p *OnePasswordProvider) Get(ctx context.Context, name string) (string, bool) { diff --git a/pkg/environment/onepassword_test.go b/pkg/environment/onepassword_test.go index 2e37c8226..b56c09a51 100644 --- a/pkg/environment/onepassword_test.go +++ b/pkg/environment/onepassword_test.go @@ -79,19 +79,15 @@ func TestOnePasswordProvider_Get(t *testing.T) { } } -func TestNewOnePasswordProvider_NoBinaryPassesThrough(t *testing.T) { +func TestNewOnePasswordProvider_AlwaysWraps(t *testing.T) { t.Parallel() - // On a system without `op`, the wrapped provider is returned untouched and - // "op://" values are passed through unchanged. - if _, err := lookupBinary("op", OnePasswordNotAvailableError{}); err == nil { - t.Skip("op binary is installed; skipping pass-through test") - } - - base := NewMapEnvProvider(map[string]string{"API_KEY": "op://vault/item/field"}) + // The constructor must always wrap so that "op://" references are never + // silently passed through as if they were real secrets, regardless of + // whether the `op` binary is installed on the host. + base := NewMapEnvProvider(map[string]string{"API_KEY": "plain"}) provider := NewOnePasswordProvider(base) - value, found := provider.Get(t.Context(), "API_KEY") - assert.True(t, found) - assert.Equal(t, "op://vault/item/field", value) + _, ok := provider.(*OnePasswordProvider) + assert.True(t, ok) }