From 09b63033e4e01b305cd8f269aadea60202c544fc Mon Sep 17 00:00:00 2001 From: root Date: Tue, 28 Jul 2026 23:35:45 +0000 Subject: [PATCH 1/2] feat(platform): make credential resolvers pluggable, remove the 1Password default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every printed CLI shipped a 1Password dependency whether or not its operator used 1Password. platform_profile.go.tmpl emitted an OnePasswordResolver that shells out to `1password-pp-cli secrets read --reveal`; ValidateCredentialReference rejected any reference not beginning with op://; and platform_cli.go.tmpl's platformResolverFactory returned that resolver as the only option. The net effect was that the entire client-profile mechanism was unusable without 1Password installed, and every SourceProfile in every printed CLI was implicitly a 1Password profile. The CredentialResolver interface already existed and was already the right abstraction — it was simply bypassed by a hardcoded validator and a single hardcoded implementation. This change puts a registry behind it. - CredentialScheme declares a reference syntax; RegisterCredentialResolver takes a scheme AND its resolver together, so a scheme the validator accepts but no resolver handles cannot exist. That combination is a reference that passes validation and then fails at use, which is the worst time to find out. - ValidateCredentialReference is registry-driven. No scheme is built in. With none registered, every reference is rejected and the error says exactly that — correct for a CLI never configured to reach a secret manager, and strictly better than silently binding to whichever vendor was hardcoded. - RegistryResolver dispatches by scheme and is what platformResolverFactory now returns. No manager is privileged. - auth.credential_resolvers selects which resolvers compile in. Omitted, it defaults to [file] — the one resolver that depends on nothing installed. 1Password remains available as an explicit opt-in for anyone who does use it. - Three resolvers ship: file (file://), bitwarden (bws://), onepassword (op://). Adding a fourth is a template plus a catalog entry; the validator, registry and dispatcher do not change. Two things fell out of building it that are worth keeping: CredentialScheme.Validate exists because a filesystem path is not a sequence of opaque identifiers. "file:///etc/key" splits to a leading EMPTY component, so the generic component rules rejected every absolute path — that is, every valid file reference. Path-shaped schemes supply their own rules rather than contorting the generic ones. Test fixtures are now generated via a credentialRefFixture template function rather than hardcoding op:// strings. Hardcoding one vendor's syntax in the test templates is part of how the original assumption got baked in and stayed invisible: the conformance tests could only pass under 1Password. Subprocess-backed resolvers share runCredentialCommand, which enforces the two rules that were previously restated per vendor: no credential value ever enters argv, and the provider's stderr never enters an error (a secret manager may echo the resolved value while reporting an unrelated failure, and error strings reach logs and run receipts). Behaviour change: a reprint of an existing CLI that used op:// references will reject them unless its spec opts into `onepassword`. In practice nothing is affected today — registerPlatformSource is never called by any printed CLI, so this entire path is dead code everywhere, which is also why the 1Password dependency went unnoticed. Verified by printing a real spec three ways: default (emits resolver_file.go only, zero 1Password references anywhere in the tree), credential_resolvers: [bitwarden, file], and [onepassword]. All three pass the generator's own build, vet, test and bundle gates. An unknown resolver name is rejected at spec validation with the catalog listed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PieLzEBrxVUbR4HauaTG3u --- docs/SPEC-EXTENSIONS.md | 55 ++++++ internal/generator/generator.go | 16 ++ internal/generator/generator_test.go | 10 +- .../client_platform_rate_limit_test.go.tmpl | 2 +- .../generator/templates/platform_cli.go.tmpl | 12 +- .../templates/platform_cli_test.go.tmpl | 20 +- .../platform_conformance_test.go.tmpl | 22 ++- .../templates/platform_profile.go.tmpl | 172 +++++++++++++++--- .../platform_resolver_bitwarden.go.tmpl | 119 ++++++++++++ .../templates/platform_resolver_exec.go.tmpl | 39 ++++ .../templates/platform_resolver_file.go.tmpl | 126 +++++++++++++ .../platform_resolver_onepassword.go.tmpl | 58 ++++++ .../templates/platform_window_test.go.tmpl | 2 +- internal/spec/credential_resolvers.go | 141 ++++++++++++++ internal/spec/credential_resolvers_test.go | 128 +++++++++++++ internal/spec/spec.go | 21 +++ 16 files changed, 886 insertions(+), 57 deletions(-) create mode 100644 internal/generator/templates/platform_resolver_bitwarden.go.tmpl create mode 100644 internal/generator/templates/platform_resolver_exec.go.tmpl create mode 100644 internal/generator/templates/platform_resolver_file.go.tmpl create mode 100644 internal/generator/templates/platform_resolver_onepassword.go.tmpl create mode 100644 internal/spec/credential_resolvers.go create mode 100644 internal/spec/credential_resolvers_test.go diff --git a/docs/SPEC-EXTENSIONS.md b/docs/SPEC-EXTENSIONS.md index 484603ee1..58a7196e2 100644 --- a/docs/SPEC-EXTENSIONS.md +++ b/docs/SPEC-EXTENSIONS.md @@ -1693,3 +1693,58 @@ x-streaming: statuses: [live, pending] primary_key: event_id ``` + +## `auth.credential_resolvers` + +Selects which secret-manager resolvers a printed CLI compiles in for the +client-profile mechanism (`platform.SourceProfile`, `--credential-ref`). + +```yaml +auth: + type: bearer_token + credential_resolvers: [bitwarden, file] +``` + +| Name | Scheme | Reference form | Needs | +|---|---|---|---| +| `file` | `file://` | `file:///absolute/path/to/secret` | nothing | +| `bitwarden` | `bws://` | `bws://` or `bws:///` | `bws` on PATH | +| `onepassword` | `op://` | `op:////` | `1password-pp-cli` on PATH | + +**Omitted, this defaults to `[file]`** — the one resolver that depends on +nothing installed. No secret manager is built in. + +Before this field existed, every printed CLI shipped a 1Password resolver and a +`ValidateCredentialReference` that rejected anything not starting with `op://`. +That made the whole client-profile mechanism unusable without 1Password, and +left a subprocess call to a password manager in the tree of every CLI whose +operator had never heard of it. Choosing a resolver is now a deliberate spec +decision, and choosing none but `file` is a perfectly good answer. + +An unknown name is a spec validation error, not a silent skip: a spec that asks +for a resolver and does not get it produces a CLI that fails at credential-read +time, which is the worst moment to discover a typo. + +### Adding a secret manager + +The validator, the registry and the dispatcher are vendor-agnostic. Adding one is: + +1. A template `internal/generator/templates/platform_resolver_.go.tmpl` + that declares a `CredentialScheme`, implements `CredentialResolver`, and + registers both together from `init()` via `RegisterCredentialResolver`. +2. An entry in `credentialResolverTemplates` in + `internal/spec/credential_resolvers.go`. + +Nothing else changes. Two rules for the resolver itself: + +- **Never put a credential value in argv.** Only the non-secret reference and + the operator-chosen binary name. Subprocess-backed resolvers should call the + shared `runCredentialCommand` helper, which enforces this. +- **Never include the provider's stderr in an error.** A secret manager may echo + the resolved value while reporting an unrelated failure, and error strings + travel into logs and run receipts. + +If a scheme's reference is not a sequence of opaque identifiers — a filesystem +path, say — set `CredentialScheme.Validate` and supply its own rules. The +generic `MinParts`/`MaxParts` component checks reject a leading empty component, +which every absolute path has. diff --git a/internal/generator/generator.go b/internal/generator/generator.go index 9af910d9c..61c961030 100644 --- a/internal/generator/generator.go +++ b/internal/generator/generator.go @@ -233,6 +233,13 @@ func New(s *spec.APISpec, outputDir string) *Generator { templates: make(map[string]*template.Template), } g.funcs = template.FuncMap{ + // credentialRefFixture yields a synthetic credential reference valid + // under the resolvers this spec selected, so generated conformance + // tests do not hardcode one vendor's syntax. Takes a name so distinct + // fixtures stay distinct. + "credentialRefFixture": func(name string) string { + return spec.SyntheticCredentialReference(g.Spec.Auth, name) + }, "title": cases.Title(language.English).String, "lower": strings.ToLower, "upper": strings.ToUpper, @@ -2440,6 +2447,15 @@ func (g *Generator) renderSingleFiles() error { } maps.Copy(singleFiles, cobratreeWalkerTemplateFiles()) + // Credential resolvers are emitted per spec (auth.credential_resolvers), so a + // printed CLI carries only the secret managers its operator actually uses. + // Before this, every CLI shipped a 1Password resolver and an op://-only + // reference validator whether or not 1Password was in play. + for _, tmplName := range g.Spec.Auth.CredentialResolverTemplates() { + outName := strings.TrimSuffix(strings.TrimPrefix(tmplName, "platform_"), ".tmpl") + singleFiles[tmplName] = filepath.Join("internal", "platform", outName) + } + for tmplName, outPath := range singleFiles { if tmplName == "types.go.tmpl" && g.shouldPreserveExistingTypesFile(outPath) { continue diff --git a/internal/generator/generator_test.go b/internal/generator/generator_test.go index f79d4a8bc..6c3e72d11 100644 --- a/internal/generator/generator_test.go +++ b/internal/generator/generator_test.go @@ -48,6 +48,10 @@ func TestGenerateProjectsCompile(t *testing.T) { mustInclude := []string{ "go.mod", "Makefile", + // The default credential resolver. A spec selecting other resolvers + // (auth.credential_resolvers) adds files beside this one, but `file` is + // what a spec that says nothing gets, so every fixture ships it. + filepath.Join("internal", "platform", "resolver_file.go"), "AGENTS.md", "CLAUDE.md", "README.md", @@ -139,9 +143,9 @@ func TestGenerateProjectsCompile(t *testing.T) { // +2: command and MCP platform-window adoption plus conformance tests. // +1: internal/cliutil/testenv, the sandbox helper every emitted test // routes its HOME/USERPROFILE isolation through. - {name: "stytch", specPath: filepath.Join("..", "..", "testdata", "stytch.yaml"), expectedFiles: 167}, - {name: "clerk", specPath: filepath.Join("..", "..", "testdata", "clerk.yaml"), expectedFiles: 171}, - {name: "loops", specPath: filepath.Join("..", "..", "testdata", "loops.yaml"), expectedFiles: 169}, + {name: "stytch", specPath: filepath.Join("..", "..", "testdata", "stytch.yaml"), expectedFiles: 168}, + {name: "clerk", specPath: filepath.Join("..", "..", "testdata", "clerk.yaml"), expectedFiles: 172}, + {name: "loops", specPath: filepath.Join("..", "..", "testdata", "loops.yaml"), expectedFiles: 170}, } for _, tt := range tests { diff --git a/internal/generator/templates/client_platform_rate_limit_test.go.tmpl b/internal/generator/templates/client_platform_rate_limit_test.go.tmpl index 02f1251fb..3d9b28196 100644 --- a/internal/generator/templates/client_platform_rate_limit_test.go.tmpl +++ b/internal/generator/templates/client_platform_rate_limit_test.go.tmpl @@ -74,7 +74,7 @@ func platformRateLimitTestClient(t *testing.T, server *httptest.Server) (*Client SchemaVersion: platform.ProfileSchemaVersion, Name: "tenant-a", Sources: map[string]platform.SourceProfile{ - "{{.Name}}": {CredentialRef: "op://Test Vault/Synthetic/token", ExpectedAccountID: "acct-a", ExpectedOrgName: "Tenant A", ExpectedStoreID: "store-a", ExpectedStoreDomain: "tenant-a.example.test", ExpectedPropertyID: "123", ExpectedBaseURL: "https://tenant-a.example.test"}, + "{{.Name}}": {CredentialRef: "{{credentialRefFixture "token"}}", ExpectedAccountID: "acct-a", ExpectedOrgName: "Tenant A", ExpectedStoreID: "store-a", ExpectedStoreDomain: "tenant-a.example.test", ExpectedPropertyID: "123", ExpectedBaseURL: "https://tenant-a.example.test"}, }, }, "{{.Name}}-pp-cli", "{{.Name}}") if err != nil { diff --git a/internal/generator/templates/platform_cli.go.tmpl b/internal/generator/templates/platform_cli.go.tmpl index f9371b8db..a786611ed 100644 --- a/internal/generator/templates/platform_cli.go.tmpl +++ b/internal/generator/templates/platform_cli.go.tmpl @@ -48,7 +48,11 @@ func registerPlatformSource(registration platformSourceRegistration) { } var platformResolverFactory = func() platform.CredentialResolver { - return platform.OnePasswordResolver{Binary: strings.TrimSpace(os.Getenv("PRINTING_PRESS_ONEPASSWORD_CLI"))} + // Dispatches to whichever resolvers this CLI was built with; see + // auth.credential_resolvers in the spec. No secret manager is privileged, + // and a CLI built with none rejects every credential reference with an + // error saying so. + return platform.RegistryResolver{} } func preparePlatformSession(flags *rootFlags) error { @@ -363,9 +367,9 @@ type platformSourceFlags struct { } func (values *platformSourceFlags) addFlags(command *cobra.Command) { - command.Flags().StringVar(&values.credentialRef, "credential-ref", "", "Exact op:// credential reference") - command.Flags().StringVar(&values.usernameRef, "username-ref", "", "Exact op:// username reference") - command.Flags().StringToStringVar(&values.additionalCredentialRef, "additional-credential-ref", nil, "Named exact op:// credential references (name=reference)") + command.Flags().StringVar(&values.credentialRef, "credential-ref", "", "Exact credential reference, e.g. "+platform.CredentialReferenceHint()) + command.Flags().StringVar(&values.usernameRef, "username-ref", "", "Exact username reference, e.g. "+platform.CredentialReferenceHint()) + command.Flags().StringToStringVar(&values.additionalCredentialRef, "additional-credential-ref", nil, "Named exact credential references (name=reference), e.g. "+platform.CredentialReferenceHint()) command.Flags().StringVar(&values.expectedAccountID, "expected-account-id", "", "Trusted immutable provider account ID") command.Flags().StringVar(&values.expectedOrgName, "expected-org-name", "", "Exact expected organization name") command.Flags().StringVar(&values.expectedStoreID, "expected-store-id", "", "Trusted immutable Shopify store ID") diff --git a/internal/generator/templates/platform_cli_test.go.tmpl b/internal/generator/templates/platform_cli_test.go.tmpl index 641987b46..904ea2213 100644 --- a/internal/generator/templates/platform_cli_test.go.tmpl +++ b/internal/generator/templates/platform_cli_test.go.tmpl @@ -82,10 +82,10 @@ func TestPlatformSourceFlagsPreserveNamedExactReferences(t *testing.T) { t.Fatalf("empty named references must stay nil for migration idempotence: %#v", source.AdditionalCredentialRefs) } values := platformSourceFlags{ - credentialRef: "op://Test Vault/Test Item/token", + credentialRef: "{{credentialRefFixture "token"}}", additionalCredentialRef: map[string]string{ - "client_id": "op://Test Vault/Test Item/client-id", - "secret": "op://Test Vault/Test Item/secret", + "client_id": "{{credentialRefFixture "client-id"}}", + "secret": "{{credentialRefFixture "secret"}}", }, } source := values.sourceProfile() @@ -136,7 +136,7 @@ func TestPlatformCLIConformanceSurfaceAndGate(t *testing.T) { SchemaVersion: platform.ProfileSchemaVersion, Name: "tenant-a", Sources: map[string]platform.SourceProfile{ - "test-source": {CredentialRef: "op://Test Vault/Test Item/token", ExpectedAccountID: "acct-a"}, + "test-source": {CredentialRef: "{{credentialRefFixture "token"}}", ExpectedAccountID: "acct-a"}, }, } if err := platform.SaveProfile(profile); err != nil { @@ -210,7 +210,7 @@ func TestPlatformCLIConformanceOutputMetadataAndAnalytics(t *testing.T) { SchemaVersion: platform.ProfileSchemaVersion, Name: "tenant-a", Sources: map[string]platform.SourceProfile{ - "test-source": {CredentialRef: "op://Test Vault/Test Item/token", ExpectedAccountID: "acct-a"}, + "test-source": {CredentialRef: "{{credentialRefFixture "token"}}", ExpectedAccountID: "acct-a"}, }, } session, err := platform.PrepareProfileSourceFromConfig(profile, "sample-pp-cli", "test-source") @@ -365,7 +365,7 @@ func TestPlatformCLIConformanceMismatchFailsBeforeCommand(t *testing.T) { t.Setenv("PRINTING_PRESS_CLIENT_PROFILE", "") if err := platform.SaveProfile(&platform.Profile{ SchemaVersion: platform.ProfileSchemaVersion, Name: "tenant-a", - Sources: map[string]platform.SourceProfile{"test-source": {CredentialRef: "op://Test Vault/Test Item/token", ExpectedAccountID: "acct-a"}}, + Sources: map[string]platform.SourceProfile{"test-source": {CredentialRef: "{{credentialRefFixture "token"}}", ExpectedAccountID: "acct-a"}}, }); err != nil { t.Fatal(err) } @@ -411,7 +411,7 @@ func TestPlatformMigrationIsVerifiedIdempotentAndQuarantinesLegacyState(t *testi return conformanceResolver{value: []byte("migration-secret-never-print")} } - args := []string{"client", "migrate", "tenant-a", "--credential-ref", "op://Test Vault/Test Item/token", "--expected-account-id", "acct-a", "--legacy-db", legacyDB, "--receipt-file", receiptPath, "--json"} + args := []string{"client", "migrate", "tenant-a", "--credential-ref", "{{credentialRefFixture "token"}}", "--expected-account-id", "acct-a", "--legacy-db", legacyDB, "--receipt-file", receiptPath, "--json"} root := RootCmd() var output bytes.Buffer root.SetOut(&output) @@ -458,7 +458,7 @@ func TestPlatformMigrationIsVerifiedIdempotentAndQuarantinesLegacyState(t *testi } conflict := RootCmd() - conflict.SetArgs([]string{"client", "migrate", "tenant-a", "--credential-ref", "op://Test Vault/Test Item/token", "--expected-account-id", "acct-b"}) + conflict.SetArgs([]string{"client", "migrate", "tenant-a", "--credential-ref", "{{credentialRefFixture "token"}}", "--expected-account-id", "acct-b"}) if err := conflict.Execute(); err == nil || !strings.Contains(err.Error(), "refusing to overwrite") { t.Fatalf("conflicting migration error = %v", err) } @@ -501,7 +501,7 @@ func TestPlatformMigrationAdoptsOnlyVerifiedTenantDatabase(t *testing.T) { var output bytes.Buffer root.SetOut(&output) root.SetErr(&output) - migrationArgs := []string{"client", "migrate", "tenant-adopt", "--credential-ref", "op://Test Vault/Test Item/token", "--expected-account-id", "acct-a", "--legacy-db", legacyDB, "--json"} + migrationArgs := []string{"client", "migrate", "tenant-adopt", "--credential-ref", "{{credentialRefFixture "token"}}", "--expected-account-id", "acct-a", "--legacy-db", legacyDB, "--json"} root.SetArgs(migrationArgs) if err := root.Execute(); err != nil { t.Fatal(err) @@ -574,7 +574,7 @@ func TestPlatformMigrationWrongTenantCannotCreateProfile(t *testing.T) { root := RootCmd() root.SetOut(&bytes.Buffer{}) root.SetErr(&bytes.Buffer{}) - root.SetArgs([]string{"client", "migrate", "tenant-rejected", "--credential-ref", "op://Test Vault/Test Item/token", "--expected-account-id", "acct-a", "--json"}) + root.SetArgs([]string{"client", "migrate", "tenant-rejected", "--credential-ref", "{{credentialRefFixture "token"}}", "--expected-account-id", "acct-a", "--json"}) if err := root.Execute(); err == nil || !strings.Contains(err.Error(), "tenant gate mismatch") { t.Fatalf("wrong-tenant migration error = %v", err) } diff --git a/internal/generator/templates/platform_conformance_test.go.tmpl b/internal/generator/templates/platform_conformance_test.go.tmpl index 1fa53f113..606bafe8f 100644 --- a/internal/generator/templates/platform_conformance_test.go.tmpl +++ b/internal/generator/templates/platform_conformance_test.go.tmpl @@ -61,7 +61,7 @@ func validKlaviyoProfile(name string) *Profile { Name: name, Sources: map[string]SourceProfile{ "klaviyo": { - CredentialRef: "op://Synthetic/Klaviyo/private-api-key", + CredentialRef: "{{credentialRefFixture "private-api-key"}}", ExpectedAccountID: "account-one", ExpectedOrgName: "Synthetic Org", }, @@ -87,7 +87,11 @@ func TestProfileParserAndSelectionConformance(t *testing.T) { t.Fatalf("profile mode = %v, err = %v", info.Mode().Perm(), err) } - for _, bad := range []string{"literal-secret", "$TOKEN", "${TOKEN}", "file:///tmp/key", "op://vault/item", "op://vault//field"} { + // Unsafe or unregistered in every resolver configuration: a bare literal, a + // shell interpolation, and references in a scheme this CLI was not built + // with. Do not add a well-formed reference in a scheme that MIGHT be + // selected — what is valid here depends on auth.credential_resolvers. + for _, bad := range []string{"literal-secret", "$TOKEN", "${TOKEN}", "made-up://vault/item", " {{credentialRefFixture "leading-space"}}"} { if err := ValidateCredentialReference(bad); err == nil { t.Fatalf("unsafe credential reference accepted: %q", bad) } @@ -118,7 +122,7 @@ func TestProfileRejectsUnknownAndDuplicateNormalizedSources(t *testing.T) { if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { t.Fatal(err) } - unknown := []byte("schema_version = 1\nname = \"synthetic\"\nunknown = true\n[sources.klaviyo]\ncredential_ref = \"op://Synthetic/Klaviyo/key\"\nexpected_account_id = \"a\"\n") + unknown := []byte("schema_version = 1\nname = \"synthetic\"\nunknown = true\n[sources.klaviyo]\ncredential_ref = \"{{credentialRefFixture "klaviyo-key"}}\"\nexpected_account_id = \"a\"\n") if err := os.WriteFile(path, unknown, 0o600); err != nil { t.Fatal(err) } @@ -244,8 +248,8 @@ func TestGorgiasUsesDocumentedBaseURLIdentityOnly(t *testing.T) { func TestProviderMismatchAndDatabaseOwnershipConformance(t *testing.T) { setPlatformRoots(t) profile := &Profile{SchemaVersion: ProfileSchemaVersion, Name: "tenant-a", Sources: map[string]SourceProfile{ - "shopify": {CredentialRef: "op://Synthetic/Shopify/token", ExpectedStoreID: "gid://shopify/Shop/1", ExpectedStoreDomain: "tenant-a.myshopify.com"}, - "gorgias": {CredentialRef: "op://Synthetic/Gorgias/key", UsernameRef: "op://Synthetic/Gorgias/user", ExpectedBaseURL: "https://tenant-a.gorgias.com/api"}, + "shopify": {CredentialRef: "{{credentialRefFixture "token"}}", ExpectedStoreID: "gid://shopify/Shop/1", ExpectedStoreDomain: "tenant-a.myshopify.com"}, + "gorgias": {CredentialRef: "{{credentialRefFixture "key"}}", UsernameRef: "{{credentialRefFixture "user"}}", ExpectedBaseURL: "https://tenant-a.gorgias.com/api"}, }} if err := SaveProfile(profile); err != nil { t.Fatal(err) @@ -288,8 +292,8 @@ func TestCrossSourceDatabaseRequiresSelectedProfileOwnership(t *testing.T) { SchemaVersion: ProfileSchemaVersion, Name: "tenant-a", Sources: map[string]SourceProfile{ - "meta-ads": {CredentialRef: "op://Synthetic/Meta/token", ExpectedAccountID: "meta-one"}, - "shopify": {CredentialRef: "op://Synthetic/Shopify/token", ExpectedStoreID: "shop-one", ExpectedStoreDomain: "tenant-a.myshopify.com"}, + "meta-ads": {CredentialRef: "{{credentialRefFixture "token"}}", ExpectedAccountID: "meta-one"}, + "shopify": {CredentialRef: "{{credentialRefFixture "token"}}", ExpectedStoreID: "shop-one", ExpectedStoreDomain: "tenant-a.myshopify.com"}, }, } primary, err := PrepareProfileSourceFromConfig(profile, "meta-ads-pp-cli", "meta-ads") @@ -331,7 +335,7 @@ func TestCrossSourceDatabaseRequiresSelectedProfileOwnership(t *testing.T) { tamperedProfile := *profile tamperedProfile.Sources = map[string]SourceProfile{ "meta-ads": profile.Sources["meta-ads"], - "shopify": {CredentialRef: "op://Synthetic/Shopify/token", ExpectedStoreID: "shop-two", ExpectedStoreDomain: "tenant-b.myshopify.com"}, + "shopify": {CredentialRef: "{{credentialRefFixture "token"}}", ExpectedStoreID: "shop-two", ExpectedStoreDomain: "tenant-b.myshopify.com"}, } tampered.Profile = &tamperedProfile if err := VerifyCrossSourceTenantMetadataReadOnly(context.Background(), db, &tampered, "shopify"); err == nil { @@ -496,7 +500,7 @@ func TestContextCarriesOutputMetadataRecorder(t *testing.T) { SchemaVersion: ProfileSchemaVersion, Name: "synthetic", Sources: map[string]SourceProfile{ - "sample": {CredentialRef: "op://Test Vault/Test Item/token", ExpectedAccountID: "one"}, + "sample": {CredentialRef: "{{credentialRefFixture "token"}}", ExpectedAccountID: "one"}, }, } session, err := PrepareProfileSourceFromConfig(profile, "sample-pp-cli", "sample") diff --git a/internal/generator/templates/platform_profile.go.tmpl b/internal/generator/templates/platform_profile.go.tmpl index 4918b4e10..8ebd77e33 100644 --- a/internal/generator/templates/platform_profile.go.tmpl +++ b/internal/generator/templates/platform_profile.go.tmpl @@ -4,7 +4,6 @@ package platform import ( - "bytes" "context" "crypto/hmac" "crypto/rand" @@ -14,11 +13,11 @@ import ( "fmt" "io" "os" - "os/exec" "path/filepath" "regexp" "sort" "strings" + "sync" "github.com/pelletier/go-toml/v2" ) @@ -64,33 +63,130 @@ type CredentialResolver interface { Resolve(context.Context, string) ([]byte, error) } -// OnePasswordResolver delegates exact-reference reads to the existing -// 1password-pp-cli policy surface. Only the non-secret reference enters argv. -type OnePasswordResolver struct { - Binary string +// CredentialScheme declares one secret manager's reference syntax. +// +// A reference is not itself a secret — that is the entire point of the +// indirection — but it IS interpolated into a subprocess argument list or a +// filesystem path, so the bounds here are load-bearing rather than cosmetic. +type CredentialScheme struct { + // Prefix is the URI scheme including "://", e.g. "bws://". + Prefix string + // MinParts and MaxParts bound how many slash-separated components may + // follow the prefix. Both are inclusive and both must be set: an unbounded + // component count is not "flexible", it is an unvalidated string on its way + // to a subprocess. + MinParts int + MaxParts int + // Syntax is the human-readable form, used in error messages. A reference is + // written by hand into a profile file, so "that is wrong" without "here is + // the shape" is a dead end. + Syntax string + // Validate optionally replaces the default slash-component checks for + // schemes whose reference is not a sequence of opaque identifiers. + // + // A filesystem path is the motivating case: "file:///etc/key" splits to a + // leading EMPTY component, which the default rules reject — so every + // absolute path, i.e. every valid file reference, would fail. Such a scheme + // supplies its own rules rather than contorting the generic ones. When set, + // MinParts/MaxParts are not applied. + Validate func(ref string) error +} + +// registeredCredentialResolver pairs a scheme with the resolver that handles it. +// +// They are registered TOGETHER, and deliberately cannot be registered apart: a +// scheme the validator accepts but no resolver handles is a reference that +// passes validation and then fails at use, which is the worst time to find out. +type registeredCredentialResolver struct { + Scheme CredentialScheme + Resolver CredentialResolver +} + +var ( + credentialRegistryMu sync.RWMutex + credentialRegistry []registeredCredentialResolver +) + +// RegisterCredentialResolver adds a scheme and the resolver that handles it. +// Registering a prefix that is already present replaces it, so a host +// application can tighten or substitute a built-in rather than accumulating two +// rules for one prefix. +// +// Printed CLIs call this from init() in the resolver files their spec selected; +// see the auth.credential_resolvers spec field. +func RegisterCredentialResolver(scheme CredentialScheme, resolver CredentialResolver) { + scheme.Prefix = strings.TrimSpace(scheme.Prefix) + if scheme.Prefix == "" || resolver == nil { + return + } + credentialRegistryMu.Lock() + defer credentialRegistryMu.Unlock() + for i := range credentialRegistry { + if credentialRegistry[i].Scheme.Prefix == scheme.Prefix { + credentialRegistry[i] = registeredCredentialResolver{Scheme: scheme, Resolver: resolver} + return + } + } + credentialRegistry = append(credentialRegistry, registeredCredentialResolver{Scheme: scheme, Resolver: resolver}) } -func (r OnePasswordResolver) Resolve(ctx context.Context, ref string) ([]byte, error) { - if err := ValidateCredentialReference(ref); err != nil { - return nil, err +// CredentialSchemes returns the registered schemes, for diagnostics and help +// text. Order is registration order. +func CredentialSchemes() []CredentialScheme { + credentialRegistryMu.RLock() + defer credentialRegistryMu.RUnlock() + schemes := make([]CredentialScheme, 0, len(credentialRegistry)) + for _, entry := range credentialRegistry { + schemes = append(schemes, entry.Scheme) + } + return schemes +} + +// CredentialReferenceHint returns one example reference syntax, for flag help. +// Help text is rendered before any command runs, so it reflects the resolvers +// this binary was compiled with rather than a hardcoded vendor. +func CredentialReferenceHint() string { + syntaxes := CredentialSyntaxes() + if len(syntaxes) == 0 { + return "(no credential resolvers are configured for this CLI)" + } + return strings.Join(syntaxes, " | ") +} + +// CredentialSyntaxes returns the human-readable reference forms, for errors. +func CredentialSyntaxes() []string { + syntaxes := []string{} + for _, scheme := range CredentialSchemes() { + syntaxes = append(syntaxes, scheme.Syntax) } - binary := strings.TrimSpace(r.Binary) - if binary == "" { - binary = "1password-pp-cli" + return syntaxes +} + +func lookupCredentialResolver(ref string) (registeredCredentialResolver, bool) { + credentialRegistryMu.RLock() + defer credentialRegistryMu.RUnlock() + for _, entry := range credentialRegistry { + if strings.HasPrefix(ref, entry.Scheme.Prefix) { + return entry, true + } } - cmd := exec.CommandContext(ctx, binary, "secrets", "read", ref, "--reveal") // #nosec G204 -- binary is operator-selected; credential value is never an argument. - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - // Provider stderr is intentionally suppressed: an external resolver may - // echo the resolved field while reporting an error. - return nil, fmt.Errorf("resolve exact 1Password reference: %w", err) + return registeredCredentialResolver{}, false +} + +// RegistryResolver dispatches a reference to whichever registered resolver +// handles its scheme. It is the CredentialResolver a printed CLI uses by +// default; no single secret manager is privileged. +type RegistryResolver struct{} + +func (RegistryResolver) Resolve(ctx context.Context, ref string) ([]byte, error) { + if err := ValidateCredentialReference(ref); err != nil { + return nil, err } - if stdout.Len() == 0 { - return nil, errors.New("resolve exact 1Password reference: empty value") + entry, ok := lookupCredentialResolver(ref) + if !ok { + return nil, fmt.Errorf("no credential resolver registered for reference scheme") } - return bytes.Clone(stdout.Bytes()), nil + return entry.Resolver.Resolve(ctx, ref) } // ResolvedCredentials holds credential bytes only for the current process. @@ -113,17 +209,35 @@ func ValidateProfileName(name string) error { return nil } -// ValidateCredentialReference accepts only complete exact op:// references. +// ValidateCredentialReference accepts a complete, exact reference in any +// registered scheme. +// +// No scheme is built in. A printed CLI registers the resolvers its spec asked +// for (auth.credential_resolvers); with none registered, every reference is +// rejected and the error says so — which is the correct behaviour for a CLI +// that was never configured to reach a secret manager, and is strictly better +// than silently binding to whichever vendor happened to be hardcoded. func ValidateCredentialReference(ref string) error { - if strings.TrimSpace(ref) != ref || !strings.HasPrefix(ref, "op://") { - return errors.New("credential reference must be an exact op://vault/item/field-or-document reference") + if strings.TrimSpace(ref) != ref { + return errors.New("credential reference must not have leading or trailing whitespace") + } + entry, ok := lookupCredentialResolver(ref) + if !ok { + syntaxes := CredentialSyntaxes() + if len(syntaxes) == 0 { + return errors.New("no credential resolvers are configured for this CLI; credential references cannot be used") + } + return fmt.Errorf("credential reference must be an exact reference in a known scheme (%s)", strings.Join(syntaxes, ", ")) } if strings.ContainsAny(ref, "$\n\r\t") { return errors.New("credential reference may not contain interpolation or control characters") } - parts := strings.Split(strings.TrimPrefix(ref, "op://"), "/") - if len(parts) != 3 { - return errors.New("credential reference must contain vault, item, and field or document") + if entry.Scheme.Validate != nil { + return entry.Scheme.Validate(ref) + } + parts := strings.Split(strings.TrimPrefix(ref, entry.Scheme.Prefix), "/") + if len(parts) < entry.Scheme.MinParts || len(parts) > entry.Scheme.MaxParts { + return fmt.Errorf("credential reference must match %s", entry.Scheme.Syntax) } for _, part := range parts { if strings.TrimSpace(part) == "" || part == "." || part == ".." { diff --git a/internal/generator/templates/platform_resolver_bitwarden.go.tmpl b/internal/generator/templates/platform_resolver_bitwarden.go.tmpl new file mode 100644 index 000000000..702ed9dda --- /dev/null +++ b/internal/generator/templates/platform_resolver_bitwarden.go.tmpl @@ -0,0 +1,119 @@ +// Copyright {{currentYear}} {{copyrightHolder}}. Licensed under Apache-2.0. See LICENSE. +// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT. + +package platform + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os" + "strings" +) + +// BitwardenResolver reads secrets from Bitwarden Secrets Manager via the `bws` +// CLI. Only the non-secret reference enters argv; the access token is supplied +// to `bws` through its own configured mechanism, never by this process. +// +// Two reference forms are accepted: +// +// bws:// a secret UUID, resolved directly +// bws:/// a secret looked up by key within a project +// +// The two-part form exists because a UUID is unreadable in a config file a +// human has to maintain, and the value of a reference is that it can sit in +// version control and be understood. It costs one extra list call. +type BitwardenResolver struct { + // Binary overrides the `bws` executable name or path. + Binary string +} + +// BitwardenScheme is the reference syntax BitwardenResolver handles. +var BitwardenScheme = CredentialScheme{ + Prefix: "bws://", + MinParts: 1, + MaxParts: 2, + Syntax: "bws://secret-id or bws://project-id/secret-key", +} + +func init() { + RegisterCredentialResolver(BitwardenScheme, BitwardenResolver{ + Binary: strings.TrimSpace(os.Getenv("PRINTING_PRESS_BITWARDEN_CLI")), + }) +} + +func (r BitwardenResolver) binary() string { + if binary := strings.TrimSpace(r.Binary); binary != "" { + return binary + } + return "bws" +} + +func (r BitwardenResolver) Resolve(ctx context.Context, ref string) ([]byte, error) { + if err := ValidateCredentialReference(ref); err != nil { + return nil, err + } + parts := strings.Split(strings.TrimPrefix(ref, BitwardenScheme.Prefix), "/") + switch len(parts) { + case 1: + return r.resolveByID(ctx, parts[0]) + case 2: + return r.resolveByKey(ctx, parts[0], parts[1]) + default: + return nil, fmt.Errorf("bitwarden reference must match %s", BitwardenScheme.Syntax) + } +} + +// bwsSecret is the subset of `bws secret` output this resolver reads. +type bwsSecret struct { + ID string `json:"id"` + Key string `json:"key"` + Value string `json:"value"` +} + +func (r BitwardenResolver) resolveByID(ctx context.Context, id string) ([]byte, error) { + out, err := runCredentialCommand(ctx, "bitwarden", r.binary(), "secret", "get", id, "--output", "json") + if err != nil { + return nil, err + } + var secret bwsSecret + if err := json.Unmarshal(out, &secret); err != nil { + return nil, fmt.Errorf("resolve bitwarden secret: unexpected output shape: %w", err) + } + if secret.Value == "" { + return nil, errors.New("resolve bitwarden secret: empty value") + } + return []byte(secret.Value), nil +} + +func (r BitwardenResolver) resolveByKey(ctx context.Context, projectID, key string) ([]byte, error) { + out, err := runCredentialCommand(ctx, "bitwarden", r.binary(), "secret", "list", projectID, "--output", "json") + if err != nil { + return nil, err + } + var secrets []bwsSecret + if err := json.Unmarshal(out, &secrets); err != nil { + return nil, fmt.Errorf("resolve bitwarden secret: unexpected output shape: %w", err) + } + // An exact, case-sensitive match, and an error on more than one. Picking the + // first of several would bind a credential to whichever the API happened to + // return first, which for a credential is the worst kind of nondeterminism. + var found *bwsSecret + for i := range secrets { + if secrets[i].Key != key { + continue + } + if found != nil { + return nil, fmt.Errorf("resolve bitwarden secret: project contains more than one secret with key %q", key) + } + found = &secrets[i] + } + if found == nil { + return nil, fmt.Errorf("resolve bitwarden secret: no secret with key %q in project", key) + } + if found.Value == "" { + return nil, errors.New("resolve bitwarden secret: empty value") + } + return []byte(found.Value), nil +} diff --git a/internal/generator/templates/platform_resolver_exec.go.tmpl b/internal/generator/templates/platform_resolver_exec.go.tmpl new file mode 100644 index 000000000..77700824b --- /dev/null +++ b/internal/generator/templates/platform_resolver_exec.go.tmpl @@ -0,0 +1,39 @@ +// Copyright {{currentYear}} {{copyrightHolder}}. Licensed under Apache-2.0. See LICENSE. +// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT. + +package platform + +import ( + "bytes" + "context" + "fmt" + "os/exec" +) + +// runCredentialCommand invokes a secret manager's CLI and returns its stdout. +// +// Shared by every subprocess-backed resolver so the two rules below hold in one +// place rather than being re-derived per vendor: +// +// 1. THE PROVIDER'S STDERR NEVER ENTERS THE ERROR. A secret manager may echo +// the resolved value while reporting an unrelated failure, and error +// strings travel into logs, run receipts and terminal scrollback. Losing +// the provider's diagnostic is the accepted cost; leaking a credential into +// a log is not. +// +// 2. NO CREDENTIAL VALUE IS EVER AN ARGUMENT. Only the non-secret reference +// and operator-chosen binary name reach argv, where `ps` and +// /proc//cmdline can see them. +func runCredentialCommand(ctx context.Context, provider, binary string, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, binary, args...) // #nosec G204 -- binary is operator-selected; every argument is a validated reference component, never a credential value. + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return nil, fmt.Errorf("resolve %s reference: %w", provider, err) + } + if stdout.Len() == 0 { + return nil, fmt.Errorf("resolve %s reference: empty output", provider) + } + return bytes.Clone(stdout.Bytes()), nil +} diff --git a/internal/generator/templates/platform_resolver_file.go.tmpl b/internal/generator/templates/platform_resolver_file.go.tmpl new file mode 100644 index 000000000..80fee805e --- /dev/null +++ b/internal/generator/templates/platform_resolver_file.go.tmpl @@ -0,0 +1,126 @@ +// Copyright {{currentYear}} {{copyrightHolder}}. Licensed under Apache-2.0. See LICENSE. +// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT. + +package platform + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" +) + +// FileResolver reads a credential from a local file. +// +// This is the default resolver because it depends on nothing: no secret manager +// installed, no daemon running, no network. It is also the shape the rest of +// this CLI already uses for credential intake — a path is not sensitive, only +// its contents are — so a profile that points at files is consistent with how +// `auth` commands take credentials everywhere else. +// +// Reference form is file:///absolute/path. The path must be absolute: a +// relative path would resolve against whatever directory the CLI happened to be +// invoked from, which makes the same profile mean different things in different +// shells. +type FileResolver struct { + // MaxBytes bounds a credential file. Zero uses defaultCredentialFileBytes. + MaxBytes int64 +} + +const defaultCredentialFileBytes = 64 * 1024 + +// FileScheme is the reference syntax FileResolver handles. +// +// It supplies its own Validate because a filesystem path is not a sequence of +// opaque identifiers: "file:///etc/key" splits to a leading EMPTY component, so +// the generic component rules would reject every absolute path — that is, +// every valid file reference. +const fileSchemeSyntax = "file:///absolute/path/to/secret" + +var FileScheme = CredentialScheme{ + Prefix: "file://", + Syntax: fileSchemeSyntax, + Validate: validateFileReference, +} + +func validateFileReference(ref string) error { + path := strings.TrimPrefix(ref, "file://") + if !filepath.IsAbs(path) { + return fmt.Errorf("file credential reference must use an absolute path: %s", fileSchemeSyntax) + } + // Reject traversal on the raw reference rather than trusting Clean to + // normalize it away: the point is that the operator wrote a definite path, + // not that some path can be derived from what they wrote. + for _, part := range strings.Split(path, "/") { + if part == ".." { + return errors.New("file credential reference may not contain a .. component") + } + } + if strings.Contains(path, "\x00") { + return errors.New("file credential reference may not contain a null byte") + } + return nil +} + +func init() { + RegisterCredentialResolver(FileScheme, FileResolver{}) +} + +func (r FileResolver) Resolve(_ context.Context, ref string) ([]byte, error) { + if err := ValidateCredentialReference(ref); err != nil { + return nil, err + } + path := strings.TrimPrefix(ref, "file://") + if !filepath.IsAbs(path) { + return nil, fmt.Errorf("file credential reference must use an absolute path: %s", fileSchemeSyntax) + } + limit := r.MaxBytes + if limit <= 0 { + limit = defaultCredentialFileBytes + } + file, err := os.Open(filepath.Clean(path)) // #nosec G304 -- the operator named this path in their own profile. + if err != nil { + // The path is not a secret, so naming it is safe and is the only way to + // make a typo diagnosable. + return nil, fmt.Errorf("reading credential file %s: %w", path, err) + } + defer file.Close() + info, err := file.Stat() + if err != nil { + return nil, fmt.Errorf("reading credential file %s metadata: %w", path, err) + } + if !info.Mode().IsRegular() { + return nil, fmt.Errorf("credential file %s is not a regular file", path) + } + if info.Size() > limit { + return nil, fmt.Errorf("credential file %s exceeds %d bytes", path, limit) + } + // Same check LoadProfile applies to the profile itself: a file holding a + // credential must not be readable by anyone else. On Windows the mode bits + // are not an ACL and this is effectively a no-op, which is a known + // limitation of the whole package rather than of this resolver. + if info.Mode().Perm()&0o077 != 0 { + return nil, fmt.Errorf("%w: %s has mode %04o; run: chmod 600 %s", ErrCredentialFilePerms, path, info.Mode().Perm(), path) + } + data, err := io.ReadAll(io.LimitReader(file, limit+1)) + if err != nil { + return nil, fmt.Errorf("reading credential file %s: %w", path, err) + } + if int64(len(data)) > limit { + return nil, fmt.Errorf("credential file %s exceeds %d bytes", path, limit) + } + // Surrounding whitespace is stripped so a file written with a trailing + // newline — which every editor and every `echo` produces — works. + value := strings.TrimSpace(string(data)) + if value == "" { + return nil, fmt.Errorf("credential file %s is empty", path) + } + return []byte(value), nil +} + +// ErrCredentialFilePerms reports a credential file readable by more than its +// owner. +var ErrCredentialFilePerms = errors.New("credential file is group/world-accessible") diff --git a/internal/generator/templates/platform_resolver_onepassword.go.tmpl b/internal/generator/templates/platform_resolver_onepassword.go.tmpl new file mode 100644 index 000000000..1b83c2278 --- /dev/null +++ b/internal/generator/templates/platform_resolver_onepassword.go.tmpl @@ -0,0 +1,58 @@ +// Copyright {{currentYear}} {{copyrightHolder}}. Licensed under Apache-2.0. See LICENSE. +// Generated by CLI Printing Press (https://github.com/mvanhorn/cli-printing-press). DO NOT EDIT. + +package platform + +import ( + "context" + "errors" + "os" + "strings" +) + +// OnePasswordResolver delegates exact-reference reads to the 1password-pp-cli +// policy surface. Only the non-secret reference enters argv. +// +// This is emitted only when a spec asks for it via +// auth.credential_resolvers: [onepassword]. It used to be the ONLY resolver and +// the only accepted reference syntax, which made every printed CLI depend on +// 1Password whether or not its operator used it. +type OnePasswordResolver struct { + // Binary overrides the `1password-pp-cli` executable name or path. + Binary string +} + +// OnePasswordScheme is the reference syntax OnePasswordResolver handles. +var OnePasswordScheme = CredentialScheme{ + Prefix: "op://", + MinParts: 3, + MaxParts: 3, + Syntax: "op://vault/item/field-or-document", +} + +func init() { + RegisterCredentialResolver(OnePasswordScheme, OnePasswordResolver{ + Binary: strings.TrimSpace(os.Getenv("PRINTING_PRESS_ONEPASSWORD_CLI")), + }) +} + +func (r OnePasswordResolver) binary() string { + if binary := strings.TrimSpace(r.Binary); binary != "" { + return binary + } + return "1password-pp-cli" +} + +func (r OnePasswordResolver) Resolve(ctx context.Context, ref string) ([]byte, error) { + if err := ValidateCredentialReference(ref); err != nil { + return nil, err + } + out, err := runCredentialCommand(ctx, "1Password", r.binary(), "secrets", "read", ref, "--reveal") + if err != nil { + return nil, err + } + if len(out) == 0 { + return nil, errors.New("resolve 1Password reference: empty value") + } + return out, nil +} diff --git a/internal/generator/templates/platform_window_test.go.tmpl b/internal/generator/templates/platform_window_test.go.tmpl index 33602986b..f8620b500 100644 --- a/internal/generator/templates/platform_window_test.go.tmpl +++ b/internal/generator/templates/platform_window_test.go.tmpl @@ -63,7 +63,7 @@ func TestPlatformCommandWindowPreservesCalendarInputs(t *testing.T) { } func TestPlatformMCPWindowPreservesCalendarMonthInput(t *testing.T) { - profile := &platform.Profile{SchemaVersion: platform.ProfileSchemaVersion, Name: "tenant-a", Sources: map[string]platform.SourceProfile{"test-source": {CredentialRef: "op://Test Vault/Test Item/token", ExpectedAccountID: "acct-a"}}} + profile := &platform.Profile{SchemaVersion: platform.ProfileSchemaVersion, Name: "tenant-a", Sources: map[string]platform.SourceProfile{"test-source": {CredentialRef: "{{credentialRefFixture "token"}}", ExpectedAccountID: "acct-a"}}} session, err := platform.PrepareProfileSourceFromConfig(profile, "fixture-pp-cli", "test-source") if err != nil { t.Fatal(err) diff --git a/internal/spec/credential_resolvers.go b/internal/spec/credential_resolvers.go new file mode 100644 index 000000000..8b3a552e6 --- /dev/null +++ b/internal/spec/credential_resolvers.go @@ -0,0 +1,141 @@ +package spec + +import ( + "fmt" + "sort" + "strings" +) + +// Credential resolver catalog names, selectable via auth.credential_resolvers. +const ( + CredentialResolverFile = "file" + CredentialResolverBitwarden = "bitwarden" + CredentialResolverOnePassword = "onepassword" +) + +// credentialResolverTemplates maps a catalog name to the template that emits it. +// Adding a secret manager is an entry here plus one template file — the +// validator, the registry and the dispatcher are vendor-agnostic and do not +// change. +var credentialResolverTemplates = map[string]string{ + CredentialResolverFile: "platform_resolver_file.go.tmpl", + CredentialResolverBitwarden: "platform_resolver_bitwarden.go.tmpl", + CredentialResolverOnePassword: "platform_resolver_onepassword.go.tmpl", +} + +// defaultCredentialResolvers is what a spec that says nothing gets. +// +// Deliberately the one resolver that depends on nothing installed. A vendor +// default is what this field exists to remove: it makes every printed CLI carry +// a dependency its operator never chose, and leaves a subprocess call to a +// password manager in trees that will never use it. +var defaultCredentialResolvers = []string{CredentialResolverFile} + +// KnownCredentialResolvers returns the catalog, sorted. +func KnownCredentialResolvers() []string { + names := make([]string, 0, len(credentialResolverTemplates)) + for name := range credentialResolverTemplates { + names = append(names, name) + } + sort.Strings(names) + return names +} + +// EffectiveCredentialResolvers returns the resolvers to compile in, normalized +// and de-duplicated. +func (a AuthConfig) EffectiveCredentialResolvers() []string { + if len(a.CredentialResolvers) == 0 { + return append([]string(nil), defaultCredentialResolvers...) + } + seen := map[string]struct{}{} + names := make([]string, 0, len(a.CredentialResolvers)) + for _, raw := range a.CredentialResolvers { + name := strings.ToLower(strings.TrimSpace(raw)) + if name == "" { + continue + } + if _, dup := seen[name]; dup { + continue + } + seen[name] = struct{}{} + names = append(names, name) + } + if len(names) == 0 { + return append([]string(nil), defaultCredentialResolvers...) + } + return names +} + +// CredentialResolverTemplates returns the template files for the selected +// resolvers, plus the shared subprocess helper when any resolver needs it. +func (a AuthConfig) CredentialResolverTemplates() []string { + templates := []string{} + needsExec := false + for _, name := range a.EffectiveCredentialResolvers() { + tmpl, ok := credentialResolverTemplates[name] + if !ok { + continue + } + templates = append(templates, tmpl) + if name != CredentialResolverFile { + needsExec = true + } + } + if needsExec { + templates = append(templates, "platform_resolver_exec.go.tmpl") + } + sort.Strings(templates) + return templates +} + +// ValidateCredentialResolvers rejects unknown catalog names. An unknown name is +// an error rather than a silent skip: a spec asking for "vault" and getting a +// CLI that cannot resolve vault:// references would fail at credential-read +// time, which is the worst moment to discover a typo. +func ValidateCredentialResolvers(names []string) error { + for _, raw := range names { + name := strings.ToLower(strings.TrimSpace(raw)) + if name == "" { + continue + } + if _, ok := credentialResolverTemplates[name]; !ok { + return fmt.Errorf("auth.credential_resolvers contains unknown resolver %q (known: %s)", + raw, strings.Join(KnownCredentialResolvers(), ", ")) + } + } + return nil +} + +// SyntheticCredentialReference builds a reference valid under the first +// resolver a spec selected, for use as a test fixture. +// +// Generated conformance tests need a reference that PASSES validation, and what +// passes now depends on which resolvers the spec asked for. Hardcoding one +// vendor's syntax in a test template is how the whole 1Password assumption got +// baked in originally. +func SyntheticCredentialReference(auth AuthConfig, name string) string { + name = strings.ToLower(strings.TrimSpace(name)) + if name == "" { + name = "synthetic" + } + name = strings.Map(func(r rune) rune { + switch { + case r >= 'a' && r <= 'z', r >= '0' && r <= '9', r == '-', r == '_': + return r + default: + return '-' + } + }, name) + resolvers := auth.EffectiveCredentialResolvers() + if len(resolvers) == 0 { + return "file:///tmp/printing-press-synthetic/" + name + } + switch resolvers[0] { + case CredentialResolverBitwarden: + return "bws://synthetic-project/" + name + case CredentialResolverOnePassword: + return "op://Synthetic/Printing Press/" + name + default: + return "file:///tmp/printing-press-synthetic/" + name + } +} diff --git a/internal/spec/credential_resolvers_test.go b/internal/spec/credential_resolvers_test.go new file mode 100644 index 000000000..27e57dcbf --- /dev/null +++ b/internal/spec/credential_resolvers_test.go @@ -0,0 +1,128 @@ +package spec + +import ( + "strings" + "testing" +) + +// The default must depend on nothing installed and must name no vendor. This is +// the whole point of the field: before it, every printed CLI shipped a +// 1Password resolver and an op://-only reference validator whether or not its +// operator used 1Password. +func TestDefaultCredentialResolversNameNoVendor(t *testing.T) { + got := AuthConfig{}.EffectiveCredentialResolvers() + if len(got) != 1 || got[0] != CredentialResolverFile { + t.Fatalf("default resolvers = %v, want [%s]", got, CredentialResolverFile) + } + for _, tmpl := range (AuthConfig{}).CredentialResolverTemplates() { + if strings.Contains(tmpl, "onepassword") || strings.Contains(tmpl, "bitwarden") { + t.Errorf("a default print emits vendor resolver %s", tmpl) + } + } +} + +// The file resolver needs no subprocess, so a default print must not carry the +// exec helper either. +func TestDefaultPrintOmitsExecHelper(t *testing.T) { + for _, tmpl := range (AuthConfig{}).CredentialResolverTemplates() { + if strings.Contains(tmpl, "resolver_exec") { + t.Errorf("default print emits %s, which only subprocess-backed resolvers need", tmpl) + } + } +} + +func TestSelectedResolversEmitTheirTemplates(t *testing.T) { + cases := []struct { + name string + selected []string + want []string + }{ + { + name: "bitwarden pulls in the exec helper", + selected: []string{CredentialResolverBitwarden}, + want: []string{"platform_resolver_bitwarden.go.tmpl", "platform_resolver_exec.go.tmpl"}, + }, + { + name: "file alone stays subprocess-free", + selected: []string{CredentialResolverFile}, + want: []string{"platform_resolver_file.go.tmpl"}, + }, + { + name: "several resolvers coexist", + selected: []string{CredentialResolverFile, CredentialResolverBitwarden}, + want: []string{ + "platform_resolver_bitwarden.go.tmpl", + "platform_resolver_exec.go.tmpl", + "platform_resolver_file.go.tmpl", + }, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := AuthConfig{CredentialResolvers: c.selected}.CredentialResolverTemplates() + if strings.Join(got, ",") != strings.Join(c.want, ",") { + t.Errorf("templates = %v, want %v", got, c.want) + } + }) + } +} + +func TestCredentialResolversNormalizeAndDeduplicate(t *testing.T) { + got := AuthConfig{CredentialResolvers: []string{" File ", "FILE", "bitwarden", ""}}.EffectiveCredentialResolvers() + if len(got) != 2 || got[0] != CredentialResolverFile || got[1] != CredentialResolverBitwarden { + t.Fatalf("resolvers = %v, want [file bitwarden]", got) + } +} + +// An unknown name must be an error rather than a silent skip: a spec asking for +// a resolver it does not get produces a CLI that fails at credential-read time, +// which is the worst moment to discover a typo. +func TestUnknownResolverIsRejectedAndNamesTheCatalog(t *testing.T) { + err := ValidateCredentialResolvers([]string{"vault"}) + if err == nil { + t.Fatal("unknown resolver accepted") + } + for _, known := range KnownCredentialResolvers() { + if !strings.Contains(err.Error(), known) { + t.Errorf("error does not name known resolver %q: %v", known, err) + } + } +} + +func TestKnownResolversAreAccepted(t *testing.T) { + if err := ValidateCredentialResolvers(KnownCredentialResolvers()); err != nil { + t.Fatalf("catalog rejected its own names: %v", err) + } +} + +// Generated conformance tests need a fixture that PASSES validation, and what +// passes depends on which resolvers were selected. Hardcoding one vendor's +// syntax in a test template is how the 1Password assumption got baked in. +func TestSyntheticReferenceMatchesSelectedResolver(t *testing.T) { + cases := []struct { + selected []string + prefix string + }{ + {nil, "file://"}, + {[]string{CredentialResolverFile}, "file://"}, + {[]string{CredentialResolverBitwarden}, "bws://"}, + {[]string{CredentialResolverOnePassword}, "op://"}, + } + for _, c := range cases { + got := SyntheticCredentialReference(AuthConfig{CredentialResolvers: c.selected}, "token") + if !strings.HasPrefix(got, c.prefix) { + t.Errorf("fixture for %v = %q, want prefix %q", c.selected, got, c.prefix) + } + } +} + +// Distinct names must stay distinct, or two sources in a conformance fixture +// collapse onto one credential. +func TestSyntheticReferencesAreDistinctPerName(t *testing.T) { + auth := AuthConfig{CredentialResolvers: []string{CredentialResolverBitwarden}} + a := SyntheticCredentialReference(auth, "client-id") + b := SyntheticCredentialReference(auth, "client-secret") + if a == b { + t.Fatalf("distinct names produced the same fixture %q", a) + } +} diff --git a/internal/spec/spec.go b/internal/spec/spec.go index 599c34d7b..d466a985c 100644 --- a/internal/spec/spec.go +++ b/internal/spec/spec.go @@ -1222,6 +1222,17 @@ type AuthConfig struct { // Used by the authorization_code flow only; ignored for other grants. RefreshTokenMechanism string `yaml:"refresh_token_mechanism,omitempty" json:"refresh_token_mechanism,omitempty"` + // CredentialResolvers selects which secret-manager resolvers the printed + // CLI compiles in for the client-profile mechanism. Values are catalog + // names: "file", "bitwarden", "onepassword". + // + // Omitted, this defaults to ["file"], which depends on nothing installed. No + // vendor is built in: before this field existed, every printed CLI shipped a + // 1Password resolver and an op://-only reference validator, so the whole + // mechanism was unusable without 1Password whether or not the operator used + // it. Selecting a resolver is now a deliberate spec decision. + CredentialResolvers []string `yaml:"credential_resolvers,omitempty" json:"credential_resolvers,omitempty"` + // AdditionalHeaders carries per-call credentials from non-winning sibling // security schemes. Composed apiKey + OAuth (or apiKey + bearer) shapes // declare both schemes in components.securitySchemes; selectSecurityScheme @@ -1765,6 +1776,9 @@ func validateAuthConfig(context string, auth AuthConfig) error { } } } + if err := validateCredentialResolvers(context, auth); err != nil { + return err + } return validateAuthFormat(context, auth) } @@ -1792,6 +1806,13 @@ func isHeaderCarriedCookieAuth(auth AuthConfig) bool { return false } +func validateCredentialResolvers(context string, auth AuthConfig) error { + if err := ValidateCredentialResolvers(auth.CredentialResolvers); err != nil { + return fmt.Errorf("%s: %w", context, err) + } + return nil +} + func validateAuthFormat(context string, auth AuthConfig) error { if auth.Format == "" { return nil From dc4119103765fb87e8343e8398d5d9606f6fde8c Mon Sep 17 00:00:00 2001 From: root Date: Wed, 29 Jul 2026 04:33:04 +0000 Subject: [PATCH 2/2] test(generator): skip the read-only-dir save test when running as root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TestConfigSaveFailureLeavesOriginalParseable makes save() fail by chmod-ing the config directory to 0500 and asserting the write is refused. Root ignores that: CAP_DAC_OVERRIDE bypasses the DAC check, so the temp file is created anyway, save() returns nil, and the test fails for a reason unrelated to the code it covers. Verified rather than assumed, in both directions: as uid 0 a write into a 0500 directory succeeds, and as a normal user the same write is refused. So the premise holds everywhere except root, and the skip is environment-specific rather than a blanket disable — the assertion still runs in CI and on developer machines. This mirrors the Windows skip already at the top of the same test: the mechanism the test depends on does not exist in this environment. The alternative — loosening the assertion to tolerate a nil error — would delete the coverage for everyone, which is the wrong trade for an environment quirk. Pre-existing on main; unrelated to the credential-resolver change in the preceding commit, and kept separate for that reason. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01PieLzEBrxVUbR4HauaTG3u --- internal/generator/config_paths_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/internal/generator/config_paths_test.go b/internal/generator/config_paths_test.go index a7ef5727f..967ee12a2 100644 --- a/internal/generator/config_paths_test.go +++ b/internal/generator/config_paths_test.go @@ -189,6 +189,17 @@ func TestConfigSaveFailureLeavesOriginalParseable(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("directory permission failure is POSIX-specific") } + // This test makes save() fail by removing write permission from the config + // directory. Root ignores that: CAP_DAC_OVERRIDE bypasses the DAC check, so + // the temp file is created anyway, save() succeeds, and the assertion below + // fails for a reason that has nothing to do with the code under test. + // + // Same category as the Windows skip above — the mechanism the test depends + // on does not exist in this environment. Skipping is honest; weakening the + // assertion to accept a nil error would delete the coverage everywhere. + if os.Geteuid() == 0 { + t.Skip("running as root: a read-only directory cannot make the write fail") + } home, _ := resetPathTestEnv(t) configPath := filepath.Join(home, "atomic", "config.toml") configDir := filepath.Dir(configPath)