diff --git a/cmd/cmd_test.go b/cmd/cmd_test.go index e50c503b..bce75ee0 100644 --- a/cmd/cmd_test.go +++ b/cmd/cmd_test.go @@ -175,13 +175,85 @@ func TestVaultCredentialStoreSubcommandsRegistered(t *testing.T) { if setCmd == nil { t.Fatal("set command not found under credential-store") } - for _, flag := range []string{"kind", "infisical-project-id", "infisical-environment", "infisical-path", "poll-interval-seconds", "yes"} { + for _, flag := range []string{"kind", "infisical-project-id", "infisical-environment", "infisical-path", "infisical-recursive", "poll-interval-seconds", "yes"} { if setCmd.Flags().Lookup(flag) == nil { t.Errorf("expected credential-store set to define --%s flag", flag) } } } +func TestVaultCreateInfisicalFlags(t *testing.T) { + vCmd := findSubcommand(rootCmd, "vault") + if vCmd == nil { + t.Fatal("vault command not found") + } + createCmd := findSubcommand(vCmd, "create") + if createCmd == nil { + t.Fatal("create command not found under vault") + } + for _, flag := range []string{"credential-store", "infisical-project-id", "infisical-environment", "infisical-path", "infisical-recursive", "poll-interval-seconds"} { + if createCmd.Flags().Lookup(flag) == nil { + t.Errorf("expected vault create to define --%s flag", flag) + } + } +} + +// newInfisicalFlagCommand mirrors the Infisical flag set shared by +// `vault create` and `vault credential-store set` without mutating the +// package-level commands. +func newInfisicalFlagCommand() *cobra.Command { + c := &cobra.Command{} + c.Flags().String("infisical-project-id", "", "") + c.Flags().String("infisical-environment", "", "") + c.Flags().String("infisical-path", "/", "") + c.Flags().Bool("infisical-recursive", false, "") + c.Flags().Int("poll-interval-seconds", 60, "") + return c +} + +func TestInfisicalStorePayloadFromFlags_Recursive(t *testing.T) { + c := newInfisicalFlagCommand() + mustSet := func(name, val string) { + t.Helper() + if err := c.Flags().Set(name, val); err != nil { + t.Fatalf("set --%s: %v", name, err) + } + } + mustSet("infisical-project-id", "p") + mustSet("infisical-environment", "dev") + mustSet("infisical-recursive", "true") + + payload, err := infisicalStorePayloadFromFlags(c) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + cfg, ok := payload["config"].(map[string]interface{}) + if !ok { + t.Fatalf("payload has no config map: %+v", payload) + } + if cfg["recursive"] != true { + t.Errorf("config.recursive: want true, got %v", cfg["recursive"]) + } +} + +func TestInfisicalStorePayloadFromFlags_RecursiveDefaultsFalse(t *testing.T) { + c := newInfisicalFlagCommand() + if err := c.Flags().Set("infisical-project-id", "p"); err != nil { + t.Fatal(err) + } + if err := c.Flags().Set("infisical-environment", "dev"); err != nil { + t.Fatal(err) + } + payload, err := infisicalStorePayloadFromFlags(c) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + cfg := payload["config"].(map[string]interface{}) + if cfg["recursive"] != false { + t.Errorf("config.recursive: want false by default, got %v", cfg["recursive"]) + } +} + func TestVaultCredentialStoreSetRequiresKind(t *testing.T) { // Missing --kind should fail before any network call. _, err := executeCommand("vault", "credential-store", "set", "my-app", "--yes") diff --git a/cmd/vaults.go b/cmd/vaults.go index 7c751b83..67f5e1dc 100644 --- a/cmd/vaults.go +++ b/cmd/vaults.go @@ -88,6 +88,7 @@ func infisicalStorePayloadFromFlags(cmd *cobra.Command) (map[string]interface{}, projectID, _ := cmd.Flags().GetString("infisical-project-id") environment, _ := cmd.Flags().GetString("infisical-environment") secretPath, _ := cmd.Flags().GetString("infisical-path") + recursive, _ := cmd.Flags().GetBool("infisical-recursive") pollSecs, _ := cmd.Flags().GetInt("poll-interval-seconds") if projectID == "" || environment == "" { @@ -105,6 +106,7 @@ func infisicalStorePayloadFromFlags(cmd *cobra.Command) (map[string]interface{}, "project_id": projectID, "environment": environment, "secret_path": secretPath, + "recursive": recursive, }, "poll_interval_seconds": pollSecs, }, nil @@ -254,6 +256,10 @@ func printCredentialStore(out io.Writer, cs map[string]interface{}) { fmt.Fprintf(out, " Project: %v\n", cfg["project_id"]) fmt.Fprintf(out, " Environment: %v\n", cfg["environment"]) fmt.Fprintf(out, " Path: %v\n", cfg["secret_path"]) + // Guarded: servers predating the recursive option omit the field. + if v, ok := cfg["recursive"]; ok { + fmt.Fprintf(out, " Recursive: %v\n", v) + } } if v, ok := cs["poll_interval_seconds"]; ok { fmt.Fprintf(out, " Poll: %vs\n", v) @@ -584,6 +590,7 @@ func init() { vaultCreateCmd.Flags().String("infisical-project-id", "", "Infisical project ID (required when --credential-store=infisical)") vaultCreateCmd.Flags().String("infisical-environment", "", "Infisical environment slug, e.g. dev/prod") vaultCreateCmd.Flags().String("infisical-path", "/", "Infisical secret path (default /)") + vaultCreateCmd.Flags().Bool("infisical-recursive", false, "Sync secrets from all subfolders of --infisical-path (secret keys must be unique across the whole folder tree)") vaultCreateCmd.Flags().Int("poll-interval-seconds", 60, "Sync cadence floor for the external store (min 10; server wakes every 10s and refreshes vaults past their interval)") vaultCmd.AddCommand(vaultCreateCmd) @@ -597,6 +604,7 @@ func init() { vaultCredentialStoreSetCmd.Flags().String("infisical-project-id", "", "Infisical project ID (required when --kind=infisical)") vaultCredentialStoreSetCmd.Flags().String("infisical-environment", "", "Infisical environment slug, e.g. dev/prod") vaultCredentialStoreSetCmd.Flags().String("infisical-path", "/", "Infisical secret path (default /)") + vaultCredentialStoreSetCmd.Flags().Bool("infisical-recursive", false, "Sync secrets from all subfolders of --infisical-path (secret keys must be unique across the whole folder tree)") vaultCredentialStoreSetCmd.Flags().Int("poll-interval-seconds", 60, "Sync cadence floor for the external store (min 10)") vaultCredentialStoreSetCmd.Flags().Bool("yes", false, "Skip confirmation prompt") diff --git a/docs/learn/credential-stores.mdx b/docs/learn/credential-stores.mdx index 012a378a..9bebcb90 100644 --- a/docs/learn/credential-stores.mdx +++ b/docs/learn/credential-stores.mdx @@ -33,6 +33,8 @@ agent-vault vault create my-app \ --poll-interval-seconds=60 ``` +Add `--infisical-recursive` to also sync secrets from all subfolders of the secret path (see [Recursive sync](#recursive-sync)). + The create handler probes Infisical first. If the project, environment, or path doesn't resolve, or if the machine identity can't read it, the request fails with a clear error and the vault is **not** created. On success, the vault row, the credential-store config, the initial encrypted secret snapshot, and your admin grant are all committed in a single SQL transaction. To inspect a vault's credential store later: @@ -67,6 +69,18 @@ In the browser, open the vault's **Settings → Edit settings** and change the * **HTTP**: `PATCH {AGENT_VAULT_ADDR}/v1/vaults/{name}/credential-store` with `{"kind":"builtin"}` or `{"kind":"infisical","config":{...},"poll_interval_seconds":N}`. Disconnecting (`builtin`) needs vault admin or instance owner; connecting (`infisical`) needs instance owner. +## Recursive sync + +By default, only secrets directly at the configured secret path are synced — subfolders are ignored. Enable recursive sync to pull secrets from the entire folder tree beneath the path: + +- **CLI**: pass `--infisical-recursive` to `vault create` or `vault credential-store set`. +- **Web UI**: enable the **Recursive sync** toggle in the vault form's Infisical section. +- **HTTP**: set `"recursive": true` inside the `config` object. + +Synced credential keys stay flat — a secret's folder is not encoded into its key. That means secret names must be **unique across the whole folder tree**: if the same key exists in two folders (e.g. `/stripe/TOKEN` and `/github/TOKEN`), the entire sync fails with `external_store_duplicate_key` and the vault keeps serving its previous snapshot until one secret is renamed upstream. Like the key-naming rule below, this is all-or-nothing — there is no partial sync. + +Recursive sync applies to static secrets only; [dynamic secrets](#dynamic-secrets) remain scoped to the configured path and are never discovered recursively. + ## Upstream key naming Agent Vault requires credential keys to match `^[A-Z][A-Z0-9_]*$` (UPPER_SNAKE_CASE). The same rule applies to keys fetched from Infisical: any non-conforming key (e.g., `database-url`, `stripeKey`) fails the whole sync with `external_store_invalid_key` and the vault keeps serving its previous snapshot until the key is renamed upstream. There is no partial sync. Rename the secret in Infisical, then retry. diff --git a/docs/reference/cli.mdx b/docs/reference/cli.mdx index 0d6d1f6b..89422409 100644 --- a/docs/reference/cli.mdx +++ b/docs/reference/cli.mdx @@ -249,6 +249,7 @@ description: "Complete reference for all Agent Vault CLI commands." | `--infisical-project-id` | | Infisical project ID. Required when `--credential-store=infisical`. | | `--infisical-environment` | | Infisical environment slug (e.g., `dev`, `prod`). Required when `--credential-store=infisical`. | | `--infisical-path` | `/` | Infisical secret path (must start with `/`). | + | `--infisical-recursive` | `false` | Sync secrets from all subfolders of `--infisical-path` recursively. Secret keys must be unique across the whole folder tree; a duplicate fails the sync. | | `--poll-interval-seconds` | `60` | Refresh cadence floor for the cached secrets. Minimum 10s. The server wakes every 10s and refreshes any vault past its interval, so actual cadence rounds up to the next 10s boundary. | @@ -319,6 +320,7 @@ description: "Complete reference for all Agent Vault CLI commands." | `--infisical-project-id` | | Infisical project ID. Required when `--kind=infisical`. | | `--infisical-environment` | | Infisical environment slug (e.g., `dev`, `prod`). Required when `--kind=infisical`. | | `--infisical-path` | `/` | Infisical secret path (must start with `/`). | + | `--infisical-recursive` | `false` | Sync secrets from all subfolders of `--infisical-path` recursively. Secret keys must be unique across the whole folder tree; a duplicate fails the sync. | | `--poll-interval-seconds` | `60` | Refresh cadence floor for the cached secrets. Minimum 10s. | | `--yes` | `false` | Skip confirmation prompt | diff --git a/internal/infisical/client.go b/internal/infisical/client.go index cf4e7194..376a8ea6 100644 --- a/internal/infisical/client.go +++ b/internal/infisical/client.go @@ -3,17 +3,20 @@ package infisical import ( "context" "encoding/json" + "errors" "fmt" "io" "log/slog" "net/http" "os" + "sort" "strconv" "strings" "sync" "time" sdk "github.com/infisical/go-sdk" + "github.com/infisical/go-sdk/packages/models" ) // SecretsFetcher is the slice of the SDK the syncer actually uses; tests @@ -119,28 +122,73 @@ func NewClient(ctx context.Context, logger *slog.Logger) (*Client, error) { // AuthMethod returns the detected machine-identity flow this client uses. func (c *Client) AuthMethod() AuthMethod { return c.method } +// ErrDuplicateKey marks a recursive-sync failure: the same secret key exists +// in more than one folder. Surfaced so the operator can rename upstream. +var ErrDuplicateKey = errors.New("infisical: duplicate secret key across folders") + // FetchSecrets honors ctx.Done() via runSDK; on cancel the orphaned SDK call // runs to completion (up to the SDK's internal timeout) and is discarded. func (c *Client) FetchSecrets(ctx context.Context, cfg VaultConfig) ([]Secret, error) { return runSDK(ctx, func() ([]Secret, error) { - res, err := c.sdk.Secrets().ListSecrets(sdk.ListSecretsOptions{ - ProjectID: cfg.ProjectID, - Environment: cfg.Environment, - SecretPath: cfg.SecretPath, - ExpandSecretReferences: true, - AttachToProcessEnv: false, - }) + res, err := c.sdk.Secrets().ListSecrets(listSecretsOptions(cfg)) if err != nil { return nil, err } - out := make([]Secret, len(res.Secrets)) - for i, s := range res.Secrets { - out[i] = Secret{Key: s.SecretKey, Value: s.SecretValue} - } - return out, nil + return secretsFromList(res.Secrets, cfg.Recursive) }) } +// listSecretsOptions maps a VaultConfig onto the SDK request. Recursive mode +// also sets SkipUniqueValidation: without it the SDK collapses cross-folder +// duplicates to one arbitrary winner before we can reject them. +func listSecretsOptions(cfg VaultConfig) sdk.ListSecretsOptions { + return sdk.ListSecretsOptions{ + ProjectID: cfg.ProjectID, + Environment: cfg.Environment, + SecretPath: cfg.SecretPath, + ExpandSecretReferences: true, + AttachToProcessEnv: false, + Recursive: cfg.Recursive, + SkipUniqueValidation: cfg.Recursive, + } +} + +// secretsFromList maps SDK results to broker secrets. In recursive mode the +// flat credential keyspace can collide across folders; any key found at more +// than one path fails the whole fetch (all-or-nothing, like ErrInvalidKey). +// Sorted before formatting: the SDK refills its result slice from a map, so +// input order is nondeterministic. +func secretsFromList(raw []models.Secret, recursive bool) ([]Secret, error) { + if recursive { + pathsByKey := make(map[string][]string, len(raw)) + for _, s := range raw { + pathsByKey[s.SecretKey] = append(pathsByKey[s.SecretKey], s.SecretPath) + } + var dupKeys []string + for key, paths := range pathsByKey { + if len(paths) > 1 { + dupKeys = append(dupKeys, key) + } + } + if len(dupKeys) > 0 { + sort.Strings(dupKeys) + paths := pathsByKey[dupKeys[0]] + sort.Strings(paths) + more := "" + if len(dupKeys) > 1 { + more = fmt.Sprintf(" (and %d more duplicate keys)", len(dupKeys)-1) + } + return nil, fmt.Errorf("%w: %q found at %s (secret keys must be unique across all folders when recursive sync is enabled; rename the secret upstream)%s", + ErrDuplicateKey, dupKeys[0], strings.Join(paths, ", "), more) + } + } + out := make([]Secret, len(raw)) + for i, s := range raw { + out[i] = Secret{Key: s.SecretKey, Value: s.SecretValue} + } + return out, nil +} + // runSDK runs a context-unaware SDK call in a goroutine so ctx.Done() is // honored; on cancel the orphaned call runs to completion and is discarded. func runSDK[T any](ctx context.Context, fn func() (T, error)) (T, error) { diff --git a/internal/infisical/client_recursive_test.go b/internal/infisical/client_recursive_test.go new file mode 100644 index 00000000..9ee0104b --- /dev/null +++ b/internal/infisical/client_recursive_test.go @@ -0,0 +1,90 @@ +package infisical + +import ( + "errors" + "strings" + "testing" + + "github.com/infisical/go-sdk/packages/models" +) + +func TestListSecretsOptions_NonRecursiveUnchanged(t *testing.T) { + opts := listSecretsOptions(VaultConfig{ProjectID: "p", Environment: "dev", SecretPath: "/app"}) + if opts.ProjectID != "p" || opts.Environment != "dev" || opts.SecretPath != "/app" { + t.Fatalf("config fields not passed through: %+v", opts) + } + if !opts.ExpandSecretReferences { + t.Fatalf("ExpandSecretReferences must stay enabled") + } + if opts.Recursive || opts.SkipUniqueValidation { + t.Fatalf("non-recursive config must not set Recursive/SkipUniqueValidation: %+v", opts) + } +} + +// TestListSecretsOptions_RecursiveSetsSkipUniqueValidation: SkipUniqueValidation +// must accompany Recursive — without it the SDK dedups cross-folder duplicates +// by key with an arbitrary winner before we can detect and reject them. +func TestListSecretsOptions_RecursiveSetsSkipUniqueValidation(t *testing.T) { + opts := listSecretsOptions(VaultConfig{ProjectID: "p", Environment: "dev", SecretPath: "/", Recursive: true}) + if !opts.Recursive { + t.Fatalf("Recursive not set") + } + if !opts.SkipUniqueValidation { + t.Fatalf("SkipUniqueValidation must be set when Recursive is") + } +} + +func TestSecretsFromList_MapsKeysAndValues(t *testing.T) { + raw := []models.Secret{ + {SecretKey: "ALPHA", SecretValue: "a", SecretPath: "/"}, + {SecretKey: "BETA", SecretValue: "b", SecretPath: "/sub"}, + } + secs, err := secretsFromList(raw, true) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(secs) != 2 || secs[0] != (Secret{Key: "ALPHA", Value: "a"}) || secs[1] != (Secret{Key: "BETA", Value: "b"}) { + t.Fatalf("bad mapping: %+v", secs) + } +} + +func TestSecretsFromList_DuplicateAcrossFolders(t *testing.T) { + raw := []models.Secret{ + {SecretKey: "TOKEN", SecretValue: "1", SecretPath: "/stripe"}, + {SecretKey: "TOKEN", SecretValue: "2", SecretPath: "/github"}, + {SecretKey: "OTHER", SecretValue: "3", SecretPath: "/"}, + } + _, err := secretsFromList(raw, true) + if !errors.Is(err, ErrDuplicateKey) { + t.Fatalf("expected ErrDuplicateKey, got %v", err) + } + for _, want := range []string{`"TOKEN"`, "/github", "/stripe"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q missing %q", err, want) + } + } + + // The SDK refills its result slice from a map, so input order is + // nondeterministic — the formatted error must not depend on it. + reversed := []models.Secret{raw[2], raw[1], raw[0]} + _, err2 := secretsFromList(reversed, true) + if err2 == nil || err2.Error() != err.Error() { + t.Fatalf("error text must be deterministic:\n%v\n%v", err, err2) + } +} + +// Defensive: in non-recursive mode the SDK already dedups by key, so the +// collision check must not run (and must not reject) there. +func TestSecretsFromList_NonRecursiveSkipsDuplicateCheck(t *testing.T) { + raw := []models.Secret{ + {SecretKey: "TOKEN", SecretValue: "1"}, + {SecretKey: "TOKEN", SecretValue: "2"}, + } + secs, err := secretsFromList(raw, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(secs) != 2 { + t.Fatalf("expected passthrough of 2 secrets, got %d", len(secs)) + } +} diff --git a/internal/infisical/config.go b/internal/infisical/config.go index 77b4558b..e7385f9b 100644 --- a/internal/infisical/config.go +++ b/internal/infisical/config.go @@ -17,6 +17,9 @@ type VaultConfig struct { ProjectID string `json:"project_id"` Environment string `json:"environment"` SecretPath string `json:"secret_path"` + // Recursive syncs secrets from all subfolders of SecretPath. Keys must + // then be unique across the whole folder tree (see ErrDuplicateKey). + Recursive bool `json:"recursive"` } // Validate enforces the structural invariants the SDK and the broker both diff --git a/internal/infisical/config_test.go b/internal/infisical/config_test.go index 61613192..c8b79dfe 100644 --- a/internal/infisical/config_test.go +++ b/internal/infisical/config_test.go @@ -20,3 +20,38 @@ func TestParseConfigJSON_TrimsStringFields(t *testing.T) { t.Errorf("secret_path: want %q, got %q", "/", cfg.SecretPath) } } + +// TestParseConfigJSON_RecursiveDefaultsFalse locks backward compatibility: +// config_json rows written before the recursive option existed must keep +// syncing non-recursively. +func TestParseConfigJSON_RecursiveDefaultsFalse(t *testing.T) { + raw := `{"project_id":"p","environment":"dev","secret_path":"/"}` + cfg, err := ParseConfigJSON(raw) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if cfg.Recursive { + t.Errorf("recursive: want false for legacy config, got true") + } +} + +func TestParseConfigJSON_RecursiveRoundTrip(t *testing.T) { + cfg, err := ParseConfigJSON(`{"project_id":"p","environment":"dev","secret_path":"/","recursive":true}`) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !cfg.Recursive { + t.Fatalf("recursive: want true, got false") + } + out, err := MarshalConfigJSON(cfg) + if err != nil { + t.Fatalf("MarshalConfigJSON: %v", err) + } + back, err := ParseConfigJSON(out) + if err != nil { + t.Fatalf("re-parse: %v", err) + } + if !back.Recursive { + t.Fatalf("recursive lost in round-trip: %s", out) + } +} diff --git a/internal/infisical/sync.go b/internal/infisical/sync.go index a4df4a84..9c96f818 100644 --- a/internal/infisical/sync.go +++ b/internal/infisical/sync.go @@ -240,9 +240,10 @@ func (s *Syncer) recordFailure(ctx context.Context, vaultID string, err error) { s.logger.Warn("infisical sync failed", slog.String("vault_id", vaultID), slog.String("err", err.Error())) - // ErrInvalidKey is caller-supplied topology; surface verbatim. + // ErrInvalidKey and ErrDuplicateKey are caller-supplied topology; + // surface verbatim. publicMsg := syncFailedPublicMessage - if errors.Is(err, ErrInvalidKey) { + if errors.Is(err, ErrInvalidKey) || errors.Is(err, ErrDuplicateKey) { publicMsg = err.Error() } // Bumping last_synced_at on failure makes dueAt act as retry backoff. diff --git a/internal/infisical/sync_test.go b/internal/infisical/sync_test.go index 2b8fa24c..e4a390be 100644 --- a/internal/infisical/sync_test.go +++ b/internal/infisical/sync_test.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "database/sql" "errors" + "fmt" "io" "log/slog" "strings" @@ -406,3 +407,59 @@ func TestSyncerWaitGroupDrainsInflightRefreshes(t *testing.T) { t.Fatalf("wg.Wait did not return after the in-flight refresh finished") } } + +// TestSyncerRefresh_PropagatesRecursiveConfig locks the ParseConfigJSON → +// fetcher plumbing: a stored "recursive":true must reach FetchSecrets. +func TestSyncerRefresh_PropagatesRecursiveConfig(t *testing.T) { + dek := makeDEK(t) + fs := newFakeStore(store.VaultCredentialStore{ + VaultID: "v1", + Kind: store.CredentialStoreInfisical, + ConfigJSON: `{"project_id":"p","environment":"dev","secret_path":"/","recursive":true}`, + PollIntervalSeconds: 60, + }) + ff := &fakeFetcher{secrets: []Secret{{Key: "ALPHA", Value: "a"}}} + s := &Syncer{store: fs, fetcher: ff, dek: dek, logger: newDiscardLogger(), clock: time.Now, inFlight: map[string]struct{}{}} + + s.refresh(context.Background(), fs.rows[0]) + + ff.mu.Lock() + defer ff.mu.Unlock() + if len(ff.callsLog) != 1 { + t.Fatalf("expected 1 fetch, got %d", len(ff.callsLog)) + } + if !ff.callsLog[0].Recursive { + t.Fatalf("recursive flag not propagated to FetchSecrets: %+v", ff.callsLog[0]) + } +} + +// TestSyncerRefresh_DuplicateKeyErrorSurfacedVerbatim: like ErrInvalidKey, +// a cross-folder duplicate is caller-fixable upstream topology and must be +// persisted verbatim in last_sync_error, not scrubbed. +func TestSyncerRefresh_DuplicateKeyErrorSurfacedVerbatim(t *testing.T) { + dek := makeDEK(t) + fs := newFakeStore(store.VaultCredentialStore{ + VaultID: "v1", + Kind: store.CredentialStoreInfisical, + ConfigJSON: `{"project_id":"p","environment":"dev","secret_path":"/","recursive":true}`, + PollIntervalSeconds: 60, + }) + dupErr := fmt.Errorf("%w: %q found at /github, /stripe", ErrDuplicateKey, "TOKEN") + ff := &fakeFetcher{err: dupErr} + s := &Syncer{store: fs, fetcher: ff, dek: dek, logger: newDiscardLogger(), clock: time.Now, inFlight: map[string]struct{}{}} + + s.refresh(context.Background(), fs.rows[0]) + + select { + case got := <-fs.replaceCh: + t.Fatalf("expected no Replace on failure, got %+v", got) + default: + } + h := fs.getHealth("v1") + if h.Status != "error" { + t.Fatalf("expected error health, got %+v", h) + } + if h.Error != dupErr.Error() || !strings.Contains(h.Error, "TOKEN") { + t.Fatalf("expected verbatim duplicate-key message, got %q", h.Error) + } +} diff --git a/internal/server/handle_vaults.go b/internal/server/handle_vaults.go index 83d38c3e..cdf2061c 100644 --- a/internal/server/handle_vaults.go +++ b/internal/server/handle_vaults.go @@ -170,6 +170,18 @@ func (s *Server) handleVaultSyncNow(w http.ResponseWriter, r *http.Request) { jsonCodedError(w, http.StatusBadRequest, "external_store_invalid_key", "Upstream secret key does not match the required UPPER_SNAKE_CASE pattern. See server logs for the offending key.") } + case errors.Is(err, infisical.ErrDuplicateKey): + // Colliding key names and folder paths are topology; redact for + // non-admin/non-owner viewers, mirroring the ErrInvalidKey gate. + if s.callerCanSeeVaultUpstream(ctx, actor, vault.ID) { + jsonCodedError(w, http.StatusBadRequest, "external_store_duplicate_key", err.Error()) + } else { + s.logger.Warn("manual infisical sync rejected duplicate upstream key", + slog.String("vault_id", vault.ID), + slog.String("err", err.Error())) + jsonCodedError(w, http.StatusBadRequest, "external_store_duplicate_key", + "Two folders in the recursive Infisical sync contain a secret with the same name. See server logs for the offending key.") + } case errors.Is(err, context.Canceled): return // caller went away default: @@ -585,6 +597,12 @@ func (s *Server) prepareInfisicalSnapshot(w http.ResponseWriter, ctx context.Con secs, err := s.infisicalClient.FetchSecrets(ctx, cfg) if err != nil { + // Create/connect are owner-only, so duplicate-key topology may be + // surfaced verbatim here (unlike the redaction gate in sync-now). + if errors.Is(err, infisical.ErrDuplicateKey) { + jsonCodedError(w, http.StatusBadRequest, "external_store_duplicate_key", err.Error()) + return infisicalSnapshot{}, false + } // SDK error embeds INFISICAL_URL + upstream rejection body; scrub it. s.logger.Warn("infisical fetch failed", slog.String("vault", logName), diff --git a/internal/server/server_test.go b/internal/server/server_test.go index d36df30e..4d01de5b 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -3695,6 +3695,72 @@ func TestVaultSyncNow_InvalidKeyRedactedForNonAdmin(t *testing.T) { } } +func TestVaultSyncNow_DuplicateKeyReturns400(t *testing.T) { + ms, token := setupMockStoreWithSession(t) + ms.credStores["root-ns-id"] = &store.VaultCredentialStore{ + VaultID: "root-ns-id", Kind: "infisical", + ConfigJSON: `{"project_id":"p","environment":"dev","secret_path":"/","recursive":true}`, + } + srv := newTestServer(withStore(ms)) + dupErr := fmt.Errorf("%w: %q found at /github, /stripe (secret keys must be unique across all folders when recursive sync is enabled; rename the secret upstream)", + infisical.ErrDuplicateKey, "TOKEN") + attachStubSyncer(t, srv, ms, &stubFetcher{err: dupErr}) + + req := httptest.NewRequest(http.MethodPost, "/v1/vaults/default/sync", nil) + req.Header.Set("Authorization", "Bearer "+token) + rec := httptest.NewRecorder() + srv.httpServer.Handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", rec.Code, rec.Body.String()) + } + var resp map[string]string + _ = json.NewDecoder(rec.Body).Decode(&resp) + if resp["code"] != "external_store_duplicate_key" { + t.Fatalf("expected code=external_store_duplicate_key, got %v", resp) + } + // Key and folder paths must appear so the operator can fix them upstream. + for _, want := range []string{"TOKEN", "/github", "/stripe"} { + if !strings.Contains(resp["error"], want) { + t.Fatalf("error must contain %q; got %q", want, resp["error"]) + } + } +} + +// TestVaultSyncNow_DuplicateKeyRedactedForNonAdmin: like ErrInvalidKey, the +// colliding key name and folder paths are upstream topology and must be +// redacted for callers who cannot see the vault's upstream config. +func TestVaultSyncNow_DuplicateKeyRedactedForNonAdmin(t *testing.T) { + ms, _ := setupMockStoreWithSession(t) + memberToken := setupMemberSession(t, ms, "root-ns-id") + ms.credStores["root-ns-id"] = &store.VaultCredentialStore{ + VaultID: "root-ns-id", Kind: "infisical", + ConfigJSON: `{"project_id":"p","environment":"dev","secret_path":"/","recursive":true}`, + } + srv := newTestServer(withStore(ms)) + dupErr := fmt.Errorf("%w: %q found at /github, /stripe", infisical.ErrDuplicateKey, "TOKEN") + attachStubSyncer(t, srv, ms, &stubFetcher{err: dupErr}) + + req := httptest.NewRequest(http.MethodPost, "/v1/vaults/default/sync", nil) + req.Header.Set("Authorization", "Bearer "+memberToken) + rec := httptest.NewRecorder() + srv.httpServer.Handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", rec.Code, rec.Body.String()) + } + var resp map[string]string + _ = json.NewDecoder(rec.Body).Decode(&resp) + if resp["code"] != "external_store_duplicate_key" { + t.Fatalf("expected code=external_store_duplicate_key, got %v", resp) + } + for _, leak := range []string{"TOKEN", "/github", "/stripe"} { + if strings.Contains(resp["error"], leak) { + t.Fatalf("non-admin response must not leak %q; got %q", leak, resp["error"]) + } + } +} + func TestVaultSyncNow_GenericUpstreamFailureReturns502(t *testing.T) { ms, token := setupMockStoreWithSession(t) ms.credStores["root-ns-id"] = &store.VaultCredentialStore{ diff --git a/web/src/components/VaultForm.tsx b/web/src/components/VaultForm.tsx index a6f26499..984c1356 100644 --- a/web/src/components/VaultForm.tsx +++ b/web/src/components/VaultForm.tsx @@ -14,6 +14,7 @@ export type VaultFormValues = { projectID: string; environment: string; secretPath: string; + recursive: boolean; }; export const emptyVaultForm: VaultFormValues = { @@ -23,6 +24,7 @@ export const emptyVaultForm: VaultFormValues = { projectID: "", environment: "", secretPath: "/", + recursive: false, }; // Infisical needs a project + environment; everything else may be blank. @@ -42,6 +44,7 @@ export function buildInfisicalConfig(v: VaultFormValues) { project_id: v.projectID.trim(), environment: v.environment.trim(), secret_path: secretPath, + recursive: v.recursive, }; } @@ -142,6 +145,16 @@ export default function VaultForm({ onChange={(e) => onChange({ secretPath: e.target.value })} /> + + onChange({ recursive: v })} + ariaLabel="Recursive sync" + /> + )} diff --git a/web/src/pages/vault/SettingsTab.tsx b/web/src/pages/vault/SettingsTab.tsx index 085bab8a..d29b1913 100644 --- a/web/src/pages/vault/SettingsTab.tsx +++ b/web/src/pages/vault/SettingsTab.tsx @@ -20,6 +20,7 @@ type InfisicalConfig = { project_id?: string; environment?: string; secret_path?: string; + recursive?: boolean; }; export default function SettingsTab() { @@ -199,6 +200,11 @@ function CredentialStoreDisplay({ store }: { store?: CredentialStoreInfo }) { + )} {isInfisical && store?.last_synced_at && ( @@ -251,6 +257,7 @@ function EditSettingsSheet({ projectID: config.project_id ?? "", environment: config.environment ?? "", secretPath: config.secret_path || "/", + recursive: config.recursive ?? false, }; // Draft state, re-seeded from the live values each time the drawer opens. @@ -276,7 +283,8 @@ function EditSettingsSheet({ switchToInfisical && (values.projectID.trim() !== (config.project_id ?? "") || values.environment.trim() !== (config.environment ?? "") || - (values.secretPath.trim() || "/") !== (config.secret_path || "/")); + (values.secretPath.trim() || "/") !== (config.secret_path || "/") || + values.recursive !== (config.recursive ?? false)); const storeChanged = values.kind !== currentKind || configChanged; // nameChanged already implies a non-empty trimmed name, so no extra guard.