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/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) 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