diff --git a/cmd/wfctl/secrets_detect.go b/cmd/wfctl/secrets_detect.go index dfdb97cf..bbbc1d55 100644 --- a/cmd/wfctl/secrets_detect.go +++ b/cmd/wfctl/secrets_detect.go @@ -2,6 +2,7 @@ package main import ( "context" + "encoding/json" "errors" "flag" "fmt" @@ -9,6 +10,7 @@ import ( "os" "regexp" "strings" + "time" "github.com/GoCodeAlone/workflow/config" "github.com/GoCodeAlone/workflow/secrets" @@ -328,12 +330,22 @@ func stdinFileDescriptor() (int, error) { return int(fd), nil //nolint:gosec // fd is range-checked before conversion. } +// secretListJSONEntry is the JSON output shape for a single secret in --json mode. +type secretListJSONEntry struct { + Name string `json:"name"` + Store string `json:"store,omitempty"` + State string `json:"state"` + Exists bool `json:"exists"` + UpdatedAt string `json:"updatedAt,omitempty"` +} + func runSecretsList(args []string) error { fs := flag.NewFlagSet("secrets list", flag.ContinueOnError) configFile := fs.String("config", "app.yaml", "Workflow config file") envName := fs.String("env", "", "Environment name for store resolution (optional)") providerName := fs.String("provider", "", "Ad-hoc provider override (keychain|env|aws); bypasses app.yaml") service := fs.String("service", "", "Service name for keychain provider") + asJSON := fs.Bool("json", false, "Output as JSON array") fs.Usage = func() { fmt.Fprintf(fs.Output(), "Usage: wfctl secrets list [options]\n\nList declared secrets and their status.\n\nOptions:\n") fs.PrintDefaults() @@ -379,10 +391,14 @@ func runSecretsList(args []string) error { if err != nil { return err } - fmt.Printf("%-40s %-12s %-10s\n", "NAME", "STORE", "STATUS") - fmt.Printf("%-40s %-12s %-10s\n", strings.Repeat("-", 40), strings.Repeat("-", 12), strings.Repeat("-", 10)) + if *asJSON { + return printSecretsJSON(statuses) + } + fmt.Printf("%-40s %-12s %-10s %-20s\n", "NAME", "STORE", "STATUS", "UPDATED") + fmt.Printf("%-40s %-12s %-10s %-20s\n", strings.Repeat("-", 40), strings.Repeat("-", 12), strings.Repeat("-", 10), strings.Repeat("-", 20)) for _, s := range statuses { - fmt.Printf("%-40s %-12s %-10s\n", s.Name, s.Store, secretStateLabel(s.State)) + updatedAt := formatUpdatedAt(s.LastRotated) + fmt.Printf("%-40s %-12s %-10s %-20s\n", s.Name, s.Store, secretStateLabel(s.State), updatedAt) } return nil } @@ -397,25 +413,99 @@ func runSecretsList(args []string) error { return err } - fmt.Printf("Provider: %s\n\n", cmp(secretsCfg.Provider, "env")) - fmt.Printf("%-40s %-6s\n", "NAME", "STATUS") - fmt.Printf("%-40s %-6s\n", strings.Repeat("-", 40), "------") + // Check access if the provider supports it (only print in text mode). + if !*asJSON { + if adapter, ok := provider.(secretsProviderAdapter); ok { + if accessErr := adapter.checkAccess(ctx); accessErr != nil { + fmt.Printf("Store access: ✗ %s\n", accessErr.Error()) + } else { + fmt.Printf("Store access: ✓\n") + } + } + } + // Build statuses for all declared entries so we can use them for --json or UPDATED column. + // Use Check (not Get) so the adapter's StatAll→Get→List precedence applies — this is + // essential for write-only stores like github where Get returns ErrUnsupported. + var statuses []SecretStatus for _, entry := range secretsCfg.Entries { - val, _ := provider.Get(ctx, entry.Name) - status := "unset" - if val != "" { - status = "set" + state, _ := provider.Check(ctx, entry.Name) + statuses = append(statuses, SecretStatus{ + Name: entry.Name, + Store: cmp(secretsCfg.Provider, "env"), + State: state, + IsSet: state == SecretSet, + }) + } + + // Enrich with metadata if supported. + if adapter, ok := provider.(secretsProviderAdapter); ok { + if mp, ok2 := adapter.p.(secrets.MetadataProvider); ok2 { + if metas, metaErr := mp.StatAll(ctx); metaErr == nil { + metaByName := make(map[string]secrets.SecretMeta, len(metas)) + for _, m := range metas { + metaByName[m.Name] = m + } + for i, s := range statuses { + if m, found := metaByName[s.Name]; found { + statuses[i].LastRotated = m.UpdatedAt + } + } + } } + } + + if *asJSON { + return printSecretsJSON(statuses) + } + + fmt.Printf("Provider: %s\n\n", cmp(secretsCfg.Provider, "env")) + fmt.Printf("%-40s %-6s %-20s\n", "NAME", "STATUS", "UPDATED") + fmt.Printf("%-40s %-6s %-20s\n", strings.Repeat("-", 40), "------", strings.Repeat("-", 20)) + + for i, entry := range secretsCfg.Entries { desc := "" if entry.Description != "" { desc = " # " + entry.Description } - fmt.Printf("%-40s %-6s%s\n", entry.Name, status, desc) + updatedAt := "—" + if i < len(statuses) { + updatedAt = formatUpdatedAt(statuses[i].LastRotated) + } + fmt.Printf("%-40s %-6s %-20s%s\n", entry.Name, secretStateLabel(statuses[i].State), updatedAt, desc) } return nil } +// printSecretsJSON marshals statuses to a JSON array and writes to stdout. +func printSecretsJSON(statuses []SecretStatus) error { + entries := make([]secretListJSONEntry, len(statuses)) + for i, s := range statuses { + entry := secretListJSONEntry{ + Name: s.Name, + Store: s.Store, + State: secretStateLabel(s.State), + Exists: s.IsSet, + } + if !s.LastRotated.IsZero() { + entry.UpdatedAt = s.LastRotated.UTC().Format(time.RFC3339) + } + entries[i] = entry + } + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + return enc.Encode(entries) +} + +// formatUpdatedAt returns a human-readable string for a LastRotated timestamp. +// Returns "—" when the timestamp is zero. +func formatUpdatedAt(t time.Time) string { + if t.IsZero() { + return "—" + } + return t.UTC().Format("2006-01-02 15:04") +} + // secretStateLabel returns a human-readable label for a SecretState. func secretStateLabel(state SecretState) string { switch state { @@ -510,7 +600,20 @@ func runSecretsInit(args []string) error { envSuffix = " for environment " + *envName } fmt.Printf("Initialized secrets provider %q%s\n", *providerName, envSuffix) - fmt.Printf("Provider %q uses OS environment variables — no additional setup required.\n", *providerName) + switch *providerName { + case "env", "": + fmt.Printf("Provider %q uses OS environment variables — no additional setup required.\n", *providerName) + case "github": + fmt.Printf("Provider %q reads from GitHub Actions secrets — ensure GITHUB_TOKEN is set.\n", *providerName) + case "vault": + fmt.Printf("Provider %q reads from HashiCorp Vault — configure address and token in secrets.config.\n", *providerName) + case "aws": + fmt.Printf("Provider %q reads from AWS Secrets Manager — ensure AWS credentials are available.\n", *providerName) + case "keychain": + fmt.Printf("Provider %q reads from the OS keychain — no additional setup required.\n", *providerName) + default: + fmt.Printf("Provider %q initialized — check provider documentation for setup.\n", *providerName) + } return nil } diff --git a/cmd/wfctl/secrets_providers.go b/cmd/wfctl/secrets_providers.go index f0c0bd49..9326308d 100644 --- a/cmd/wfctl/secrets_providers.go +++ b/cmd/wfctl/secrets_providers.go @@ -2,9 +2,11 @@ package main import ( "context" - "fmt" + "errors" "os" "time" + + "github.com/GoCodeAlone/workflow/secrets" ) // SecretState describes the accessibility of a secret in its backing store. @@ -45,14 +47,153 @@ type SecretStatus struct { IsSet bool } +// secretsProviderAdapter wraps a secrets.Provider and implements SecretsProvider. +// It upgrades to MetadataProvider/AccessChecker when the underlying provider +// supports them. +type secretsProviderAdapter struct { + p secrets.Provider +} + +func (a secretsProviderAdapter) Get(ctx context.Context, name string) (string, error) { + return a.p.Get(ctx, name) +} + +func (a secretsProviderAdapter) Set(ctx context.Context, name, value string) error { + return a.p.Set(ctx, name, value) +} + +func (a secretsProviderAdapter) Delete(ctx context.Context, name string) error { + return a.p.Delete(ctx, name) +} + +// Check returns the SecretState for the named secret. +// +// Resolution order, most-precise first: +// 1. MetadataProvider.StatAll — when it succeeds, membership + presence is authoritative. +// 2. Get(name)-presence — when StatAll is unavailable/errored but Get works +// (env, file, vault, aws). A non-empty value is Set; an empty value or +// ErrNotFound is NotSet. +// 3. List()-membership — only when Get itself reports ErrUnsupported +// (write-only stores like github, where reading a value is impossible). +func (a secretsProviderAdapter) Check(ctx context.Context, name string) (SecretState, error) { + if mp, ok := a.p.(secrets.MetadataProvider); ok { + metas, err := mp.StatAll(ctx) + if err == nil { + for _, m := range metas { + if m.Name == name { + if m.Exists { + return SecretSet, nil + } + return SecretNotSet, nil + } + } + return SecretNotSet, nil + } + // StatAll failed (including ErrUnsupported) — fall through to Get/List. + } + + // Get-presence check. + v, err := a.p.Get(ctx, name) + if err == nil { + if v != "" { + return SecretSet, nil + } + return SecretNotSet, nil + } + if errors.Is(err, secrets.ErrNotFound) { + return SecretNotSet, nil + } + if !errors.Is(err, secrets.ErrUnsupported) { + // Unexpected Get error (e.g. permission denied) — surface as fetch error. + return SecretFetchError, err + } + + // Get is unsupported (write-only store) — fall back to List() membership. + names, listErr := a.p.List(ctx) + if listErr != nil { + if errors.Is(listErr, secrets.ErrUnsupported) { + return SecretFetchError, nil + } + return SecretFetchError, listErr + } + for _, n := range names { + if n == name { + return SecretSet, nil + } + } + return SecretNotSet, nil +} + +// List returns SecretStatus entries from the provider. +// +// It prefers MetadataProvider.StatAll (which carries LastRotated from +// SecretMeta.UpdatedAt). When StatAll is unavailable or errors, it falls back to +// the plain List() names with presence-only statuses. A store that supports +// neither (e.g. env with no prefix) yields an empty list rather than an error, +// so callers that only need per-entry Check semantics are unaffected. +func (a secretsProviderAdapter) List(ctx context.Context) ([]SecretStatus, error) { + if mp, ok := a.p.(secrets.MetadataProvider); ok { + metas, err := mp.StatAll(ctx) + if err == nil { + statuses := make([]SecretStatus, len(metas)) + for i, m := range metas { + state := SecretNotSet + if m.Exists { + state = SecretSet + } + statuses[i] = SecretStatus{ + Name: m.Name, + State: state, + IsSet: m.Exists, + LastRotated: m.UpdatedAt, + } + } + return statuses, nil + } + // StatAll failed (including ErrUnsupported) — fall through to List() below. + } + // Fall back to plain List() with presence-only statuses. + names, err := a.p.List(ctx) + if err != nil { + if errors.Is(err, secrets.ErrUnsupported) { + // Store cannot enumerate — return empty rather than erroring. + return nil, nil + } + return nil, err + } + statuses := make([]SecretStatus, len(names)) + for i, n := range names { + statuses[i] = SecretStatus{ + Name: n, + State: SecretSet, + IsSet: true, + } + } + return statuses, nil +} + +// checkAccess calls CheckAccess on the underlying provider if it implements +// AccessChecker. Returns nil when the provider does not implement the interface. +func (a secretsProviderAdapter) checkAccess(ctx context.Context) error { + if ac, ok := a.p.(secrets.AccessChecker); ok { + return ac.CheckAccess(ctx) + } + return nil +} + // newSecretsProvider constructs the provider matching the given name. +// It now supports all 5 backends (env, github, vault, aws, keychain) by +// delegating to resolveSecretsProvider, then wrapping the result in the adapter. func newSecretsProvider(providerName string) (SecretsProvider, error) { - switch providerName { - case "env", "": - return &envProvider{}, nil - default: - return nil, fmt.Errorf("unknown secrets provider %q (supported: env)", providerName) + name := providerName + if name == "" { + name = "env" + } + p, err := resolveSecretsProvider(&SecretsConfig{Provider: name}) + if err != nil { + return nil, err } + return secretsProviderAdapter{p}, nil } // envProvider reads/writes secrets as OS environment variables. diff --git a/cmd/wfctl/secrets_providers_test.go b/cmd/wfctl/secrets_providers_test.go new file mode 100644 index 00000000..3f0c1af9 --- /dev/null +++ b/cmd/wfctl/secrets_providers_test.go @@ -0,0 +1,148 @@ +package main + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/GoCodeAlone/workflow/secrets" +) + +// plainSecretsProvider is a minimal secrets.Provider (no metadata). +type plainSecretsProvider struct { + stored map[string]string +} + +func (f *plainSecretsProvider) Name() string { return "plain" } +func (f *plainSecretsProvider) Get(_ context.Context, key string) (string, error) { + v, ok := f.stored[key] + if !ok { + return "", secrets.ErrNotFound + } + return v, nil +} +func (f *plainSecretsProvider) Set(_ context.Context, key, value string) error { + f.stored[key] = value + return nil +} +func (f *plainSecretsProvider) Delete(_ context.Context, key string) error { + delete(f.stored, key) + return nil +} +func (f *plainSecretsProvider) List(_ context.Context) ([]string, error) { + keys := make([]string, 0, len(f.stored)) + for k := range f.stored { + keys = append(keys, k) + } + return keys, nil +} + +// metadataSecretsProvider extends plainSecretsProvider with MetadataProvider support. +type metadataSecretsProvider struct { + *plainSecretsProvider + metas []secrets.SecretMeta +} + +func (f *metadataSecretsProvider) StatAll(_ context.Context) ([]secrets.SecretMeta, error) { + return f.metas, nil +} + +// --------------------------------------------------------------------------- +// Adapter tests +// --------------------------------------------------------------------------- + +func TestSecretsProviderAdapter_Check_PlainProvider_Set(t *testing.T) { + fp := &plainSecretsProvider{stored: map[string]string{"MY_KEY": "some-value"}} + adapter := secretsProviderAdapter{fp} + + state, err := adapter.Check(context.Background(), "MY_KEY") + if err != nil { + t.Fatalf("Check: %v", err) + } + if state != SecretSet { + t.Errorf("Check(set key) = %v, want SecretSet", state) + } +} + +func TestSecretsProviderAdapter_Check_PlainProvider_NotSet(t *testing.T) { + fp := &plainSecretsProvider{stored: map[string]string{}} + adapter := secretsProviderAdapter{fp} + + state, err := adapter.Check(context.Background(), "MISSING_KEY") + if err != nil { + t.Fatalf("Check: %v", err) + } + if state != SecretNotSet { + t.Errorf("Check(missing key) = %v, want SecretNotSet", state) + } +} + +func TestSecretsProviderAdapter_List_PlainProvider_PresenceOnly(t *testing.T) { + fp := &plainSecretsProvider{stored: map[string]string{"KEY_A": "v1", "KEY_B": "v2"}} + adapter := secretsProviderAdapter{fp} + + statuses, err := adapter.List(context.Background()) + if err != nil { + t.Fatalf("List: %v", err) + } + if len(statuses) != 2 { + t.Fatalf("expected 2 statuses, got %d", len(statuses)) + } + // Plain provider: LastRotated should be zero. + for _, s := range statuses { + if !s.LastRotated.IsZero() { + t.Errorf("key %q: expected zero LastRotated for plain provider, got %v", s.Name, s.LastRotated) + } + } +} + +func TestSecretsProviderAdapter_List_MetadataProvider_UpdatedAt(t *testing.T) { + ts := time.Date(2026, 5, 20, 0, 0, 0, 0, time.UTC) + fp := &plainSecretsProvider{stored: map[string]string{"KEY_A": "v1"}} + metas := []secrets.SecretMeta{ + {Name: "KEY_A", Exists: true, UpdatedAt: ts}, + } + mp := &metadataSecretsProvider{ + plainSecretsProvider: fp, + metas: metas, + } + adapter := secretsProviderAdapter{mp} + + statuses, err := adapter.List(context.Background()) + if err != nil { + t.Fatalf("List: %v", err) + } + if len(statuses) != 1 { + t.Fatalf("expected 1 status, got %d", len(statuses)) + } + if statuses[0].LastRotated != ts { + t.Errorf("LastRotated = %v, want %v", statuses[0].LastRotated, ts) + } +} + +func TestNewSecretsProvider_NotUnknownProviderError(t *testing.T) { + // Clear GITHUB_TOKEN so the github branch is actually reached even when the + // ambient environment has a token set — otherwise the test would short-circuit. + t.Setenv("GITHUB_TOKEN", "") + + // newSecretsProvider("github") should NOT return the old "unknown secrets provider" error. + // It may fail with another error (e.g. missing token), but not the old sentinel. + _, err := newSecretsProvider("github") + if err == nil { + t.Fatal("expected an error (missing GITHUB_TOKEN), got nil") + } + if strings.Contains(err.Error(), "unknown secrets provider") { + t.Errorf("newSecretsProvider(\"github\") returned the old unknown-provider error: %v", err) + } +} + +func TestNewSecretsProvider_Env_Success(t *testing.T) { + p, err := newSecretsProvider("env") + if err != nil { + t.Fatalf("newSecretsProvider(\"env\"): %v", err) + } + if p == nil { + t.Fatal("expected non-nil provider") + } +} diff --git a/cmd/wfctl/secrets_resolve.go b/cmd/wfctl/secrets_resolve.go index 3be0ccbf..11b8ad26 100644 --- a/cmd/wfctl/secrets_resolve.go +++ b/cmd/wfctl/secrets_resolve.go @@ -55,7 +55,12 @@ func ResolveSecretStore(secretName string, envName string, cfg *config.WorkflowC func getProviderForStore(storeName string, cfg *config.WorkflowConfig) (SecretsProvider, error) { if cfg != nil { if store, ok := cfg.SecretStores[storeName]; ok && store != nil { - return newSecretsProvider(store.Provider) + // Named store found — build via resolveSecretsProvider and wrap in adapter. + p, err := resolveSecretsProvider(secretsConfigFromStore(store)) + if err != nil { + return nil, err + } + return secretsProviderAdapter{p}, nil } } // Store name not in SecretStores map — treat as a direct provider name. diff --git a/cmd/wfctl/secrets_test.go b/cmd/wfctl/secrets_test.go index 8e0fd98b..39ee4caa 100644 --- a/cmd/wfctl/secrets_test.go +++ b/cmd/wfctl/secrets_test.go @@ -3,6 +3,7 @@ package main import ( "bytes" "context" + "encoding/json" "io" "os" "strings" @@ -360,3 +361,73 @@ func TestSecretsExport_ShellExportFormat(t *testing.T) { t.Errorf("missing export line:\n%s", buf.String()) } } + +// --------------------------------------------------------------------------- +// runSecretsList --json and UPDATED column tests +// --------------------------------------------------------------------------- + +func TestSecretsList_JSONFlag(t *testing.T) { + // Write a temp config with env provider and a declared entry. + dir := t.TempDir() + cfgFile := dir + "/app.yaml" + if err := os.WriteFile(cfgFile, []byte(`secrets: + provider: env + entries: + - name: TEST_LIST_JSON_KEY +`), 0600); err != nil { + t.Fatal(err) + } + + // Capture stdout. + old := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + err := runSecretsList([]string{"--config", cfgFile, "--json"}) + + w.Close() + out, _ := io.ReadAll(r) + os.Stdout = old + + if err != nil { + t.Fatalf("runSecretsList --json: %v", err) + } + + // Output must be valid JSON. + var parsed []map[string]any + if jsonErr := json.Unmarshal(out, &parsed); jsonErr != nil { + t.Fatalf("output is not valid JSON: %v\noutput: %s", jsonErr, out) + } +} + +func TestSecretsList_TextMode_HasUpdatedColumn(t *testing.T) { + // Write a temp config with env provider. + dir := t.TempDir() + cfgFile := dir + "/app.yaml" + if err := os.WriteFile(cfgFile, []byte(`secrets: + provider: env + entries: + - name: TEST_LIST_TEXT_KEY +`), 0600); err != nil { + t.Fatal(err) + } + + // Capture stdout. + old := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + err := runSecretsList([]string{"--config", cfgFile}) + + w.Close() + out, _ := io.ReadAll(r) + os.Stdout = old + + if err != nil { + t.Fatalf("runSecretsList text mode: %v", err) + } + + if !strings.Contains(string(out), "UPDATED") { + t.Errorf("text mode output should contain UPDATED column header:\n%s", out) + } +} diff --git a/secrets/aws_provider.go b/secrets/aws_provider.go index 5b7994bd..1dc89dfd 100644 --- a/secrets/aws_provider.go +++ b/secrets/aws_provider.go @@ -307,6 +307,32 @@ func extractJSONField(jsonStr, field string) (string, error) { return fmt.Sprintf("%v", val), nil } +// StatAll implements MetadataProvider on a best-effort basis. It lists secret +// names from AWS Secrets Manager and returns them with Exists=true and zero +// UpdatedAt (LastChangedDate is not fetched to avoid N+1 API calls). Returns +// ErrUnsupported when listing is not available. +func (p *AWSSecretsManagerProvider) StatAll(ctx context.Context) ([]SecretMeta, error) { + keys, err := p.List(ctx) + if err != nil { + return nil, fmt.Errorf("%w: aws StatAll: %v", ErrUnsupported, err) + } + metas := make([]SecretMeta, len(keys)) + for i, k := range keys { + metas[i] = SecretMeta{Name: k, Exists: true} + } + return metas, nil +} + +// CheckAccess implements AccessChecker. It performs a lightweight check by +// attempting to list secrets. Errors never contain credential material. +func (p *AWSSecretsManagerProvider) CheckAccess(ctx context.Context) error { + _, err := p.List(ctx) + if err != nil { + return fmt.Errorf("aws store access: check failed (creds redacted)") + } + return nil +} + // sha256Hex computes hex-encoded SHA-256 hash. func sha256Hex(data []byte) string { h := sha256.Sum256(data) diff --git a/secrets/github_provider.go b/secrets/github_provider.go index bdd0ddff..e2241ba6 100644 --- a/secrets/github_provider.go +++ b/secrets/github_provider.go @@ -12,6 +12,7 @@ import ( "net/url" "os" "strings" + "time" "golang.org/x/crypto/blake2b" "golang.org/x/crypto/nacl/box" @@ -56,6 +57,15 @@ type GitHubSecretsProvider struct { selectedRepoIDs []int64 // required iff scope=org && visibility=selected token string client *http.Client + baseURL string // overridden in tests to point at an httptest.Server +} + +// base returns the API base URL, using baseURL when set (for tests). +func (p *GitHubSecretsProvider) base() string { + if p.baseURL != "" { + return p.baseURL + } + return githubAPIBase } // NewGitHubSecretsProvider creates a repo-scoped provider for the given @@ -205,8 +215,15 @@ func (p *GitHubSecretsProvider) Delete(ctx context.Context, key string) error { return nil } -// List returns the names of all GitHub Actions secrets for the repo. -func (p *GitHubSecretsProvider) List(ctx context.Context) ([]string, error) { +// ghSecretEntry is the JSON shape returned by GitHub's list-secrets endpoints. +type ghSecretEntry struct { + Name string `json:"name"` + UpdatedAt time.Time `json:"updated_at"` + CreatedAt time.Time `json:"created_at"` +} + +// listSecretEntries fetches and decodes all secret entries (name + timestamps). +func (p *GitHubSecretsProvider) listSecretEntries(ctx context.Context) ([]ghSecretEntry, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, p.secretsURL(), nil) if err != nil { return nil, err @@ -221,20 +238,70 @@ func (p *GitHubSecretsProvider) List(ctx context.Context) ([]string, error) { return nil, fmt.Errorf("secrets: github list secrets: HTTP %d%s", resp.StatusCode, readErrorBody(resp)) } var result struct { - Secrets []struct { - Name string `json:"name"` - } `json:"secrets"` + Secrets []ghSecretEntry `json:"secrets"` } if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { return nil, fmt.Errorf("secrets: github list decode: %w", err) } - names := make([]string, len(result.Secrets)) - for i, s := range result.Secrets { + return result.Secrets, nil +} + +// List returns the names of all GitHub Actions secrets for the repo. +func (p *GitHubSecretsProvider) List(ctx context.Context) ([]string, error) { + entries, err := p.listSecretEntries(ctx) + if err != nil { + return nil, err + } + names := make([]string, len(entries)) + for i, s := range entries { names[i] = s.Name } return names, nil } +// StatAll implements MetadataProvider. It returns presence + timestamp for every +// secret visible to the configured token. UpdatedAt is the updated_at field from +// GitHub, falling back to created_at when updated_at is zero. +func (p *GitHubSecretsProvider) StatAll(ctx context.Context) ([]SecretMeta, error) { + entries, err := p.listSecretEntries(ctx) + if err != nil { + return nil, err + } + metas := make([]SecretMeta, len(entries)) + for i, e := range entries { + ts := e.UpdatedAt + if ts.IsZero() { + ts = e.CreatedAt + } + metas[i] = SecretMeta{ + Name: e.Name, + Exists: true, + UpdatedAt: ts, + } + } + return metas, nil +} + +// CheckAccess implements AccessChecker. It verifies the configured credentials +// have at least read access by fetching the public key. Errors never contain +// credential material. +func (p *GitHubSecretsProvider) CheckAccess(ctx context.Context) error { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, p.publicKeyURL(), nil) + if err != nil { + return fmt.Errorf("github store access: request build: %w", err) + } + p.setHeaders(req) + resp, err := p.client.Do(req) + if err != nil { + return fmt.Errorf("github store access: %w (creds redacted)", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("github store access: HTTP %d (creds redacted)", resp.StatusCode) + } + return nil +} + // readErrorBody reads up to 512 bytes from resp.Body and returns them as a // trimmed string prefixed with ": " for appending to an error message. // Returns "" when the body is empty, so callers don't emit a trailing ": ". @@ -258,11 +325,11 @@ func (p *GitHubSecretsProvider) setHeaders(req *http.Request) { func (p *GitHubSecretsProvider) secretsURL() string { switch p.scope { case GitHubScopeOrg: - return fmt.Sprintf("%s/orgs/%s/actions/secrets", githubAPIBase, p.org) + return fmt.Sprintf("%s/orgs/%s/actions/secrets", p.base(), p.org) case GitHubScopeEnv: - return fmt.Sprintf("%s/repos/%s/%s/environments/%s/secrets", githubAPIBase, p.owner, p.repo, url.PathEscape(p.env)) + return fmt.Sprintf("%s/repos/%s/%s/environments/%s/secrets", p.base(), p.owner, p.repo, url.PathEscape(p.env)) default: // GitHubScopeRepo - return fmt.Sprintf("%s/repos/%s/%s/actions/secrets", githubAPIBase, p.owner, p.repo) + return fmt.Sprintf("%s/repos/%s/%s/actions/secrets", p.base(), p.owner, p.repo) } } diff --git a/secrets/github_provider_test.go b/secrets/github_provider_test.go index ff547366..4db5b3b6 100644 --- a/secrets/github_provider_test.go +++ b/secrets/github_provider_test.go @@ -516,6 +516,98 @@ func TestGitHubProvider_RepoScope_NoVisibility(t *testing.T) { } } +// --------------------------------------------------------------------------- +// StatAll + CheckAccess tests +// --------------------------------------------------------------------------- + +func TestGitHubProvider_StatAll(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/repos/owner/repo/actions/secrets": + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]any{ //nolint:errcheck + "secrets": []map[string]any{ + { + "name": "A", + "created_at": "2026-05-01T00:00:00Z", + "updated_at": "2026-05-20T00:00:00Z", + }, + }, + }) + case "/repos/owner/repo/actions/secrets/public-key": + // 32 zero bytes encoded as base64 + key := make([]byte, 32) + json.NewEncoder(w).Encode(map[string]string{ //nolint:errcheck + "key_id": "1", + "key": base64.StdEncoding.EncodeToString(key), + }) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + p := newTestGitHubProvider(t, srv) + + metas, err := p.StatAll(context.Background()) + if err != nil { + t.Fatalf("StatAll: %v", err) + } + if len(metas) != 1 { + t.Fatalf("expected 1 meta, got %d", len(metas)) + } + if metas[0].Name != "A" { + t.Errorf("Name = %q, want A", metas[0].Name) + } + if !metas[0].Exists { + t.Error("expected Exists=true") + } + // UpdatedAt should be 2026-05-20 + if metas[0].UpdatedAt.Year() != 2026 || metas[0].UpdatedAt.Month() != 5 || metas[0].UpdatedAt.Day() != 20 { + t.Errorf("UpdatedAt = %v, want 2026-05-20", metas[0].UpdatedAt) + } +} + +func TestGitHubProvider_CheckAccess_Success(t *testing.T) { + key := make([]byte, 32) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]string{ //nolint:errcheck + "key_id": "1", + "key": base64.StdEncoding.EncodeToString(key), + }) + })) + defer srv.Close() + + p := newTestGitHubProvider(t, srv) + if err := p.CheckAccess(context.Background()); err != nil { + t.Errorf("CheckAccess expected nil, got %v", err) + } +} + +func TestGitHubProvider_CheckAccess_Forbidden(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + w.Write([]byte(`{"message":"must have admin rights"}`)) //nolint:errcheck + })) + defer srv.Close() + + p := newTestGitHubProvider(t, srv) + err := p.CheckAccess(context.Background()) + if err == nil { + t.Fatal("expected non-nil error for 403") + } + // Must NOT contain the token string. + if strings.Contains(err.Error(), "test-token") { + t.Errorf("error leaks token: %v", err) + } + if !strings.Contains(err.Error(), "403") { + t.Errorf("error should mention 403: %v", err) + } + if !strings.Contains(err.Error(), "creds redacted") { + t.Errorf("error should mention 'creds redacted': %v", err) + } +} + func TestGitHubProvider_ScopeReporter(t *testing.T) { t.Setenv("GITHUB_TOKEN", "x") p, _ := NewGitHubSecretsProvider("o/r", "GITHUB_TOKEN") diff --git a/secrets/metadata_test.go b/secrets/metadata_test.go new file mode 100644 index 00000000..233eab21 --- /dev/null +++ b/secrets/metadata_test.go @@ -0,0 +1,119 @@ +package secrets + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" +) + +// TestMetadataProvider_InterfaceShape verifies that: +// - EnvProvider satisfies Provider +// - SecretMeta zero-value has zero UpdatedAt +func TestMetadataProvider_InterfaceShape(t *testing.T) { + // Compile-time check: EnvProvider must satisfy Provider. + var _ Provider = (*EnvProvider)(nil) + + m := SecretMeta{Name: "X", Exists: true} + if !m.UpdatedAt.IsZero() { + t.Fatal("expected zero UpdatedAt for new SecretMeta") + } +} + +// --------------------------------------------------------------------------- +// FileProvider StatAll + CheckAccess +// --------------------------------------------------------------------------- + +func TestFileProvider_StatAll(t *testing.T) { + dir := t.TempDir() + p := NewFileProvider(dir) + ctx := context.Background() + + // Write two key files. + if err := os.WriteFile(filepath.Join(dir, "KEY_A"), []byte("valA"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "KEY_B"), []byte("valB"), 0600); err != nil { + t.Fatal(err) + } + + metas, err := p.StatAll(ctx) + if err != nil { + t.Fatalf("StatAll: %v", err) + } + if len(metas) != 2 { + t.Fatalf("expected 2 metas, got %d", len(metas)) + } + // All should exist; mtime should be non-zero. + for _, m := range metas { + if !m.Exists { + t.Errorf("key %q: expected Exists=true", m.Name) + } + if m.UpdatedAt.IsZero() { + t.Errorf("key %q: expected non-zero UpdatedAt (file mtime)", m.Name) + } + } +} + +func TestFileProvider_CheckAccess_Success(t *testing.T) { + dir := t.TempDir() + p := NewFileProvider(dir) + if err := p.CheckAccess(context.Background()); err != nil { + t.Errorf("expected nil error for writable dir, got %v", err) + } +} + +func TestFileProvider_CheckAccess_MissingDir(t *testing.T) { + p := NewFileProvider("/nonexistent/path/xyz123") + err := p.CheckAccess(context.Background()) + if err == nil { + t.Error("expected error for missing directory") + } +} + +// --------------------------------------------------------------------------- +// EnvProvider StatAll + CheckAccess +// --------------------------------------------------------------------------- + +func TestEnvProvider_StatAll(t *testing.T) { + prefix := "WFTEST_META_" + t.Setenv(prefix+"KEY1", "v1") + t.Setenv(prefix+"KEY2", "v2") + + p := NewEnvProvider(prefix) + ctx := context.Background() + + metas, err := p.StatAll(ctx) + if err != nil { + t.Fatalf("StatAll: %v", err) + } + if len(metas) < 2 { + t.Fatalf("expected at least 2 metas, got %d", len(metas)) + } + for _, m := range metas { + // Env provider can't know mtime — UpdatedAt is always zero. + if !m.UpdatedAt.IsZero() { + t.Errorf("key %q: expected zero UpdatedAt for env provider, got %v", m.Name, m.UpdatedAt) + } + if !m.Exists { + t.Errorf("key %q: expected Exists=true", m.Name) + } + } +} + +func TestEnvProvider_StatAll_NoPrefix(t *testing.T) { + // Without prefix, StatAll should return ErrUnsupported. + p := NewEnvProvider("") + _, err := p.StatAll(context.Background()) + if !errors.Is(err, ErrUnsupported) { + t.Errorf("expected ErrUnsupported from StatAll with no prefix, got %v", err) + } +} + +func TestEnvProvider_CheckAccess(t *testing.T) { + p := NewEnvProvider("") + if err := p.CheckAccess(context.Background()); err != nil { + t.Errorf("CheckAccess on EnvProvider: %v", err) + } +} diff --git a/secrets/secrets.go b/secrets/secrets.go index 84532b78..62e67de7 100644 --- a/secrets/secrets.go +++ b/secrets/secrets.go @@ -7,6 +7,7 @@ import ( "os" "strings" "sync" + "time" ) // SecretPrefix is the URI scheme used in config values to reference secrets. @@ -34,6 +35,25 @@ type Provider interface { List(ctx context.Context) ([]string, error) } +// SecretMeta is presence + freshness for one key. Never carries a value. +type SecretMeta struct { + Name string + Exists bool + UpdatedAt time.Time // zero when the store doesn't expose it +} + +// MetadataProvider is optional: stores that can report which keys exist and when they changed. +type MetadataProvider interface { + Provider + StatAll(ctx context.Context) ([]SecretMeta, error) +} + +// AccessChecker is optional: verify the store is reachable + usable for setup. +// CheckAccess MUST NOT leak credential material in its error. +type AccessChecker interface { + CheckAccess(ctx context.Context) error +} + // RotationProvider extends Provider with key rotation capabilities. type RotationProvider interface { Provider @@ -112,6 +132,36 @@ func (p *EnvProvider) envKey(key string) string { return k } +// StatAll implements MetadataProvider. It lists env vars that match the prefix +// (same logic as List) and returns SecretMeta with Exists=true and zero UpdatedAt +// (env vars have no last-modified timestamp). +func (p *EnvProvider) StatAll(_ context.Context) ([]SecretMeta, error) { + prefix := strings.ToUpper(p.prefix) + if prefix == "" { + return nil, ErrUnsupported + } + var metas []SecretMeta + for _, env := range os.Environ() { + parts := strings.SplitN(env, "=", 2) + if len(parts) != 2 { + continue + } + if strings.HasPrefix(parts[0], prefix) { + metas = append(metas, SecretMeta{ + Name: parts[0], + Exists: true, + // UpdatedAt intentionally zero — env vars carry no mtime. + }) + } + } + return metas, nil +} + +// CheckAccess implements AccessChecker. For EnvProvider, access is always available. +func (p *EnvProvider) CheckAccess(_ context.Context) error { + return nil +} + // --- File Provider --- // FileProvider reads secrets from files in a directory. @@ -177,6 +227,44 @@ func (p *FileProvider) List(_ context.Context) ([]string, error) { return keys, nil } +// StatAll implements MetadataProvider. It returns SecretMeta for every file in +// the directory, using the file's modification time as UpdatedAt. +func (p *FileProvider) StatAll(_ context.Context) ([]SecretMeta, error) { + entries, err := os.ReadDir(p.dir) + if err != nil { + return nil, fmt.Errorf("secrets: failed to list directory: %w", err) + } + var metas []SecretMeta + for _, e := range entries { + if e.IsDir() { + continue + } + info, err := e.Info() + if err != nil { + continue + } + metas = append(metas, SecretMeta{ + Name: e.Name(), + Exists: true, + UpdatedAt: info.ModTime(), + }) + } + return metas, nil +} + +// CheckAccess implements AccessChecker. It verifies the directory exists and is +// writable by attempting to create (then remove) a probe file. +func (p *FileProvider) CheckAccess(_ context.Context) error { + probe := p.dir + "/.wfctl_probe" + f, err := os.OpenFile(probe, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600) + if err != nil { + return fmt.Errorf("secrets: file store not accessible: %w", err) + } + f.Close() + os.Remove(probe) //nolint:errcheck + return nil +} + // --- Vault Configuration --- // VaultConfig holds configuration for HashiCorp Vault. diff --git a/secrets/vault_provider.go b/secrets/vault_provider.go index b0c3fa98..3e1dae13 100644 --- a/secrets/vault_provider.go +++ b/secrets/vault_provider.go @@ -300,6 +300,31 @@ func parseVaultKey(key string) (path, field string) { return key, "" } +// StatAll implements MetadataProvider on a best-effort basis. If the Vault +// client is wired and the mount is reachable, it returns entries from the KV v2 +// metadata list. Otherwise it returns ErrUnsupported so callers can fall back. +func (p *VaultProvider) StatAll(ctx context.Context) ([]SecretMeta, error) { + keys, err := p.List(ctx) + if err != nil { + return nil, fmt.Errorf("%w: vault StatAll: %v", ErrUnsupported, err) + } + metas := make([]SecretMeta, len(keys)) + for i, k := range keys { + metas[i] = SecretMeta{Name: k, Exists: true} + } + return metas, nil +} + +// CheckAccess implements AccessChecker. It performs a lightweight check by +// attempting to list the mount's metadata root. The returned error never wraps +// the raw vault error, which may carry the address or secret path. +func (p *VaultProvider) CheckAccess(ctx context.Context) error { + if _, err := p.List(ctx); err != nil { + return fmt.Errorf("vault store access: check failed (creds redacted)") + } + return nil +} + // isVaultNotFound checks if a vault error is a 404 / not found. func isVaultNotFound(err error) bool { if err == nil {