diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 2f8b80d56..da60d0c1f 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -283,6 +283,7 @@ flowchart TD | `step.iac_provider_apply` | Applies a plan after recomputing + validating `desired_hash` (stateless two-phase TOCTOU guard) | platform | | `step.iac_provider_destroy` | Destroys resources via an `iac.provider` plugin | platform | | `step.iac_provider_drift` | Detects drift via an `iac.provider` (optional `IaCProviderDriftDetector`; `supported:false` fallback) | platform | +| `step.iac_secret_reachability` | Pre-flight gate: checks whether `secret://` refs in plan specs are reachable from the chosen exec-env. The verdict is provider-level (one `CheckAccess` probe; the same result is reported per distinct ref); returns `all_reachable` bool. Fail-safe for remote exec-envs (host-local backends unverifiable per ADR 0017, unknown backends, and probe failure → unreachable) | platform | | `step.tofu_init` | Initializes an OpenTofu working directory | platform | | `step.tofu_plan` | Creates an OpenTofu execution plan | platform | | `step.tofu_apply` | Applies OpenTofu changes to infrastructure | platform | diff --git a/module/pipeline_step_iac_secret_reachability.go b/module/pipeline_step_iac_secret_reachability.go new file mode 100644 index 000000000..3b178bd9f --- /dev/null +++ b/module/pipeline_step_iac_secret_reachability.go @@ -0,0 +1,218 @@ +package module + +import ( + "context" + "fmt" + "sort" + "strings" + + "github.com/GoCodeAlone/modular" + "github.com/GoCodeAlone/workflow/iac/specparse" + "github.com/GoCodeAlone/workflow/interfaces" + "github.com/GoCodeAlone/workflow/secrets" +) + +// ─── step.iac_secret_reachability ──────────────────────────────────────────── + +// IaCSecretReachabilityStep pre-flights whether the secrets referenced in a set +// of IaC resource specs can be read from the chosen execution environment. +// +// It resolves the secrets provider from the app service registry (mirrors the +// resolveProvider pattern in SecretSetStep), gathers all distinct secret:// +// references from the spec configs (including nested maps and slices), calls +// secrets.Reachability once for the provider, and reports per-ref results. +// +// Output shape: +// +// { +// "secrets": [ {ref, reachable, reason}, ... ], +// "all_reachable": bool, +// } +// +// When there are zero secret:// refs, all_reachable is true with an empty list. +type IaCSecretReachabilityStep struct { + name string + provider string // secrets service name in the app registry + execEnv string + specs []interfaces.ResourceSpec + specsFrom string // dotted context path; mutually exclusive with specs + app modular.Application +} + +// NewIaCSecretReachabilityStepFactory returns a StepFactory for +// step.iac_secret_reachability. +func NewIaCSecretReachabilityStepFactory() StepFactory { + return func(name string, cfg map[string]any, app modular.Application) (PipelineStep, error) { + providerName, _ := cfg["provider"].(string) + if providerName == "" { + return nil, fmt.Errorf("iac_secret_reachability step %q: 'provider' is required", name) + } + + execEnv, _ := cfg["exec_env"].(string) + + specsFrom, _ := cfg["specs_from"].(string) + _, hasStaticSpecs := cfg["specs"] + if specsFrom != "" && hasStaticSpecs { + return nil, fmt.Errorf("iac_secret_reachability step %q: 'specs' and 'specs_from' are mutually exclusive", name) + } + + var specs []interfaces.ResourceSpec + if hasStaticSpecs { + var err error + specs, err = specparse.ParseResourceSpecs(cfg["specs"]) + if err != nil { + return nil, fmt.Errorf("iac_secret_reachability step %q: parse specs: %w", name, err) + } + } + + return &IaCSecretReachabilityStep{ + name: name, + provider: providerName, + execEnv: execEnv, + specs: specs, + specsFrom: specsFrom, + app: app, + }, nil + } +} + +func (s *IaCSecretReachabilityStep) Name() string { return s.name } + +func (s *IaCSecretReachabilityStep) Execute(ctx context.Context, pc *PipelineContext) (*StepResult, error) { + // Resolve specs: dynamic path takes precedence when specsFrom is configured. + specs := s.specs + if s.specsFrom != "" { + raw := resolveBodyFrom(s.specsFrom, pc) + var err error + specs, err = specparse.ParseResourceSpecs(raw) + if err != nil { + return nil, fmt.Errorf("iac_secret_reachability step %q: resolve specs_from %q: %w", s.name, s.specsFrom, err) + } + // Fail-safe: a specs_from that resolves to nil/empty (missing/misspelled + // context path, or a body lacking specs) must NOT silently bypass the + // gate with all_reachable=true. Error, matching iac_provider_plan/apply. + if len(specs) == 0 { + return nil, fmt.Errorf("iac_secret_reachability step %q: specs_from %q resolved to empty/zero specs", s.name, s.specsFrom) + } + } + + // Resolve the secrets provider from the service registry. + p, err := resolveSecretsProvider(s.app, s.provider, s.name) + if err != nil { + return nil, err + } + + // Gather distinct secret:// refs across all spec configs. + refs := collectSecretRefs(specs) + + // Short-path: no secret refs → trivially reachable. + if len(refs) == 0 { + return &StepResult{Output: map[string]any{ + "secrets": []map[string]any{}, + "all_reachable": true, + }}, nil + } + + // Call Reachability once — the verdict is provider-level, not per-ref. + // Propagate the Execute ctx so a slow/unreachable backend probe is bounded + // by the pipeline/route deadline rather than hanging the pre-flight. + verdict := secrets.Reachability(ctx, p, s.execEnv) + + secretEntries := make([]map[string]any, 0, len(refs)) + for _, ref := range refs { + entry := map[string]any{ + "ref": ref, + "reachable": verdict.Reachable, + } + if !verdict.Reachable { + entry["reason"] = verdict.Reason + } + secretEntries = append(secretEntries, entry) + } + + allReachable := verdict.Reachable + + return &StepResult{Output: map[string]any{ + "secrets": secretEntries, + "all_reachable": allReachable, + }}, nil +} + +// resolveSecretsProvider looks up a secrets.Provider from the application +// service registry by name. It mirrors the resolveProvider pattern in +// SecretSetStep: first checks if the service directly implements secrets.Provider; +// if not, checks for a Provider() accessor (used by SecretsAWSModule, +// SecretsVaultModule, etc.). +func resolveSecretsProvider(app modular.Application, providerName, stepName string) (secrets.Provider, error) { + if app == nil { + return nil, fmt.Errorf("iac_secret_reachability step %q: no application context", stepName) + } + svc, ok := app.SvcRegistry()[providerName] + if !ok { + return nil, fmt.Errorf("iac_secret_reachability step %q: secrets service %q not found in registry", stepName, providerName) + } + + // Direct: service itself implements secrets.Provider. + if p, ok := svc.(secrets.Provider); ok { + return p, nil + } + + // Indirect: service exposes a Provider() accessor (e.g. SecretsAWSModule, + // SecretsVaultModule). + type providerAccessor interface { + Provider() secrets.Provider + } + if acc, ok := svc.(providerAccessor); ok { + p := acc.Provider() + if p == nil { + return nil, fmt.Errorf("iac_secret_reachability step %q: service %q exposes Provider() accessor but returned nil; module may not be started", stepName, providerName) + } + return p, nil + } + + return nil, fmt.Errorf("iac_secret_reachability step %q: service %q does not implement secrets.Provider directly or via Provider() accessor", stepName, providerName) +} + +// collectSecretRefs walks the Config map of each ResourceSpec and returns a +// sorted, deduplicated slice of all string values that start with secrets.SecretPrefix. +// It recurses into nested map[string]any, []any, and typed []string values so +// both YAML-decoded and programmatically-built spec configs are fully scanned. +func collectSecretRefs(specs []interfaces.ResourceSpec) []string { + seen := make(map[string]struct{}) + for _, spec := range specs { + collectFromValue(spec.Config, seen) + } + refs := make([]string, 0, len(seen)) + for ref := range seen { + refs = append(refs, ref) + } + sort.Strings(refs) + return refs +} + +// collectFromValue recursively extracts secret:// refs from an arbitrary value. +// It handles both YAML-decoded shapes (map[string]any, []any) and typed Go +// shapes built programmatically (notably []string), since ResourceSpec.Config +// may carry either when specs are constructed in code rather than parsed. +func collectFromValue(v any, seen map[string]struct{}) { + switch val := v.(type) { + case string: + if strings.HasPrefix(val, secrets.SecretPrefix) { + seen[val] = struct{}{} + } + case []string: + for _, item := range val { + if strings.HasPrefix(item, secrets.SecretPrefix) { + seen[item] = struct{}{} + } + } + case map[string]any: + for _, child := range val { + collectFromValue(child, seen) + } + case []any: + for _, item := range val { + collectFromValue(item, seen) + } + } +} diff --git a/module/pipeline_step_iac_secret_reachability_internal_test.go b/module/pipeline_step_iac_secret_reachability_internal_test.go new file mode 100644 index 000000000..03a1bb37a --- /dev/null +++ b/module/pipeline_step_iac_secret_reachability_internal_test.go @@ -0,0 +1,93 @@ +package module + +import ( + "reflect" + "testing" + + "github.com/GoCodeAlone/workflow/interfaces" +) + +// TestCollectSecretRefs exercises collectSecretRefs/collectFromValue across every +// container shape a ResourceSpec.Config can carry: a flat string ref, a ref +// nested inside a map[string]any, a ref inside an []any of strings, a ref inside +// an []any of maps, a ref inside a typed []string (programmatically-built specs), +// and double-nesting. It also asserts dedup + sorted order. +func TestCollectSecretRefs(t *testing.T) { + specs := []interfaces.ResourceSpec{ + { + Name: "r1", + Type: "infra.database", + Config: map[string]any{ + // flat string ref + "password": "secret://vault/db-pass", + // non-ref string (ignored) + "region": "us-east-1", + // ref nested in a map[string]any + "nested": map[string]any{ + "token": "secret://vault/nested-token", + }, + // ref in an []any of strings + "list_any_strings": []any{ + "plain", + "secret://vault/from-anylist", + }, + // ref in an []any of maps + "list_any_maps": []any{ + map[string]any{"key": "secret://vault/from-anymap"}, + }, + // ref in a typed []string (programmatically-built) + "list_typed_strings": []string{ + "not-a-ref", + "secret://vault/from-typedlist", + }, + // double-nesting: map → slice → map → string + "deep": map[string]any{ + "items": []any{ + map[string]any{ + "deeper": map[string]any{ + "v": "secret://vault/deep-ref", + }, + }, + }, + }, + }, + }, + { + Name: "r2", + Type: "infra.database", + Config: map[string]any{ + // duplicate of r1's password — must be deduped + "password": "secret://vault/db-pass", + // a unique ref in r2 + "api": "secret://vault/r2-api", + }, + }, + } + + got := collectSecretRefs(specs) + + want := []string{ + "secret://vault/db-pass", + "secret://vault/deep-ref", + "secret://vault/from-anylist", + "secret://vault/from-anymap", + "secret://vault/from-typedlist", + "secret://vault/nested-token", + "secret://vault/r2-api", + } + + if !reflect.DeepEqual(got, want) { + t.Errorf("collectSecretRefs mismatch:\n got: %#v\n want: %#v", got, want) + } +} + +// TestCollectSecretRefs_NoRefs asserts an empty (non-nil) slice when no refs exist. +func TestCollectSecretRefs_NoRefs(t *testing.T) { + specs := []interfaces.ResourceSpec{ + {Name: "r1", Type: "infra.database", Config: map[string]any{"size": "small"}}, + } + got := collectSecretRefs(specs) + if len(got) != 0 { + t.Errorf("expected no refs, got %#v", got) + } +} diff --git a/module/pipeline_step_iac_secret_reachability_test.go b/module/pipeline_step_iac_secret_reachability_test.go new file mode 100644 index 000000000..5a3e5af93 --- /dev/null +++ b/module/pipeline_step_iac_secret_reachability_test.go @@ -0,0 +1,503 @@ +package module_test + +import ( + "context" + "errors" + "testing" + + "github.com/GoCodeAlone/workflow/module" + "github.com/GoCodeAlone/workflow/secrets" +) + +// ─── secrets provider stubs for reachability step tests ────────────────────── + +// stubSecretsProviderWithAccess implements secrets.Provider + secrets.AccessChecker. +// It simulates a reachable (or unreachable, via checkErr) remote backend. +type stubSecretsProviderWithAccess struct { + checkErr error + checkCalled bool + checkCount int +} + +func (s *stubSecretsProviderWithAccess) Name() string { return "stub-vault" } +func (s *stubSecretsProviderWithAccess) Get(_ context.Context, _ string) (string, error) { + return "", nil +} +func (s *stubSecretsProviderWithAccess) Set(_ context.Context, _, _ string) error { return nil } +func (s *stubSecretsProviderWithAccess) Delete(_ context.Context, _ string) error { return nil } +func (s *stubSecretsProviderWithAccess) List(_ context.Context) ([]string, error) { return nil, nil } +func (s *stubSecretsProviderWithAccess) CheckAccess(_ context.Context) error { + s.checkCalled = true + s.checkCount++ + return s.checkErr +} + +// stubSecretsProviderNoAccess implements secrets.Provider only (no AccessChecker). +type stubSecretsProviderNoAccess struct{} + +func (s *stubSecretsProviderNoAccess) Name() string { return "stub-no-access" } +func (s *stubSecretsProviderNoAccess) Get(_ context.Context, _ string) (string, error) { + return "", nil +} +func (s *stubSecretsProviderNoAccess) Set(_ context.Context, _, _ string) error { return nil } +func (s *stubSecretsProviderNoAccess) Delete(_ context.Context, _ string) error { return nil } +func (s *stubSecretsProviderNoAccess) List(_ context.Context) ([]string, error) { return nil, nil } + +// stubSecretsModuleAccessor wraps a secrets.Provider behind a Provider() accessor, +// mirroring how SecretsVaultModule / SecretsAWSModule work in production. +type stubSecretsModuleAccessor struct { + underlying secrets.Provider +} + +func (m *stubSecretsModuleAccessor) Provider() secrets.Provider { return m.underlying } + +// compile-time assertions +var _ secrets.Provider = (*stubSecretsProviderWithAccess)(nil) +var _ secrets.AccessChecker = (*stubSecretsProviderWithAccess)(nil) +var _ secrets.Provider = (*stubSecretsProviderNoAccess)(nil) + +// ─── TestSecretReachabilityStep ─────────────────────────────────────────────── + +// TestSecretReachabilityStep_VaultWithAccess verifies case (a): +// specs with a vault secret:// ref + stub vault provider whose CheckAccess→nil +// + execEnv "remote" → all_reachable true. +func TestSecretReachabilityStep_VaultWithAccess(t *testing.T) { + app := module.NewMockApplication() + stub := &stubSecretsProviderWithAccess{checkErr: nil} + // Register via Provider() accessor (mirrors production SecretsVaultModule wiring). + if err := app.RegisterService("my-vault", &stubSecretsModuleAccessor{underlying: stub}); err != nil { + t.Fatal(err) + } + + cfg := map[string]any{ + "provider": "my-vault", + "exec_env": "remote", + "specs": []any{ + map[string]any{ + "name": "my-db", + "type": "infra.database", + "config": map[string]any{ + "password": "secret://vault/my-db-password", + }, + }, + }, + } + + factory := module.NewIaCSecretReachabilityStepFactory() + step, err := factory("reach-step", cfg, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + result, err := step.Execute(context.Background(), &module.PipelineContext{}) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + + // The AccessChecker probe must have been consulted (remote backend path). + if !stub.checkCalled { + t.Error("expected CheckAccess to be called on the vault provider") + } + + allReachable, ok := result.Output["all_reachable"].(bool) + if !ok { + t.Fatalf("expected bool all_reachable, got %T: %v", result.Output["all_reachable"], result.Output["all_reachable"]) + } + if !allReachable { + t.Errorf("expected all_reachable=true, got false") + } + + secretsList, ok := result.Output["secrets"].([]map[string]any) + if !ok { + t.Fatalf("expected []map[string]any secrets, got %T", result.Output["secrets"]) + } + if len(secretsList) != 1 { + t.Errorf("expected 1 secret entry, got %d", len(secretsList)) + } + if secretsList[0]["ref"] != "secret://vault/my-db-password" { + t.Errorf("expected ref=secret://vault/my-db-password, got %v", secretsList[0]["ref"]) + } + if secretsList[0]["reachable"] != true { + t.Errorf("expected reachable=true for secret entry, got %v", secretsList[0]["reachable"]) + } +} + +// TestSecretReachabilityStep_GitHubProvider verifies case (b): +// a github-backed provider → all_reachable false with the write-only reason. +func TestSecretReachabilityStep_GitHubProvider(t *testing.T) { + app := module.NewMockApplication() + t.Setenv("WORKFLOW_TEST_GH_TOKEN2", "ghp_fake_token_step_test") + ghProvider, err := secrets.NewGitHubSecretsProvider("owner/repo", "WORKFLOW_TEST_GH_TOKEN2") + if err != nil { + t.Fatalf("NewGitHubSecretsProvider: %v", err) + } + if err := app.RegisterService("my-gh", &stubSecretsModuleAccessor{underlying: ghProvider}); err != nil { + t.Fatal(err) + } + + cfg := map[string]any{ + "provider": "my-gh", + "exec_env": "remote", + "specs": []any{ + map[string]any{ + "name": "deploy-key", + "type": "infra.secret", + "config": map[string]any{ + "token": "secret://gh/DEPLOY_TOKEN", + }, + }, + }, + } + + factory := module.NewIaCSecretReachabilityStepFactory() + step, err := factory("reach-step", cfg, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + result, err := step.Execute(context.Background(), &module.PipelineContext{}) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + + allReachable, _ := result.Output["all_reachable"].(bool) + if allReachable { + t.Error("expected all_reachable=false for github-backed provider") + } + + secretsList, ok := result.Output["secrets"].([]map[string]any) + if !ok { + t.Fatalf("expected []map[string]any secrets, got %T", result.Output["secrets"]) + } + if len(secretsList) != 1 { + t.Errorf("expected 1 secret entry, got %d", len(secretsList)) + } + reason, _ := secretsList[0]["reason"].(string) + if !containsString(reason, "write-only") { + t.Errorf("expected reason to contain 'write-only', got %q", reason) + } +} + +// TestSecretReachabilityStep_VaultNoAccessChecker_Remote verifies case (c): +// a vault provider without AccessChecker + execEnv "remote" → all_reachable false (fail-safe). +func TestSecretReachabilityStep_VaultNoAccessChecker_Remote(t *testing.T) { + app := module.NewMockApplication() + stub := &stubSecretsProviderNoAccess{} + if err := app.RegisterService("my-vault-noaccess", &stubSecretsModuleAccessor{underlying: stub}); err != nil { + t.Fatal(err) + } + + cfg := map[string]any{ + "provider": "my-vault-noaccess", + "exec_env": "remote", + "specs": []any{ + map[string]any{ + "name": "infra-key", + "type": "infra.database", + "config": map[string]any{ + "key": "secret://unknown/backend-key", + }, + }, + }, + } + + factory := module.NewIaCSecretReachabilityStepFactory() + step, err := factory("reach-step", cfg, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + result, err := step.Execute(context.Background(), &module.PipelineContext{}) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + + allReachable, _ := result.Output["all_reachable"].(bool) + if allReachable { + t.Error("expected all_reachable=false for fail-safe (no AccessChecker + remote env)") + } + + secretsList, ok := result.Output["secrets"].([]map[string]any) + if !ok { + t.Fatalf("expected []map[string]any secrets, got %T", result.Output["secrets"]) + } + if len(secretsList) != 1 { + t.Errorf("expected 1 secret entry, got %d", len(secretsList)) + } + reason, _ := secretsList[0]["reason"].(string) + if !containsString(reason, "reachability unknown") { + t.Errorf("expected reason to contain 'reachability unknown', got %q", reason) + } +} + +// TestSecretReachabilityStep_NoSecretRefs verifies case (d): +// specs with NO secret refs → all_reachable true, empty list. +func TestSecretReachabilityStep_NoSecretRefs(t *testing.T) { + app := module.NewMockApplication() + stub := &stubSecretsProviderWithAccess{checkErr: nil} + if err := app.RegisterService("my-vault", &stubSecretsModuleAccessor{underlying: stub}); err != nil { + t.Fatal(err) + } + + cfg := map[string]any{ + "provider": "my-vault", + "exec_env": "remote", + "specs": []any{ + map[string]any{ + "name": "plain-resource", + "type": "infra.database", + "config": map[string]any{ + "size": "small", + "region": "us-east-1", + }, + }, + }, + } + + factory := module.NewIaCSecretReachabilityStepFactory() + step, err := factory("reach-step", cfg, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + result, err := step.Execute(context.Background(), &module.PipelineContext{}) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + + allReachable, ok := result.Output["all_reachable"].(bool) + if !ok { + t.Fatalf("expected bool all_reachable, got %T", result.Output["all_reachable"]) + } + if !allReachable { + t.Error("expected all_reachable=true when no secret refs present") + } + + secretsList, ok := result.Output["secrets"].([]map[string]any) + if !ok { + t.Fatalf("expected []map[string]any secrets, got %T", result.Output["secrets"]) + } + if len(secretsList) != 0 { + t.Errorf("expected empty secrets list, got %d entries", len(secretsList)) + } +} + +// TestSecretReachabilityStep_MultipleRefs_SameProvider verifies that multiple +// distinct secret:// refs in specs are all reported individually but the provider +// check fires only once (provider-level verdict). +func TestSecretReachabilityStep_MultipleRefs_SameProvider(t *testing.T) { + app := module.NewMockApplication() + stub := &stubSecretsProviderWithAccess{checkErr: errors.New("unreachable vault")} + if err := app.RegisterService("my-vault", &stubSecretsModuleAccessor{underlying: stub}); err != nil { + t.Fatal(err) + } + + cfg := map[string]any{ + "provider": "my-vault", + "exec_env": "remote", + "specs": []any{ + map[string]any{ + "name": "db", + "type": "infra.database", + "config": map[string]any{ + "password": "secret://vault/db-pass", + "username": "secret://vault/db-user", + }, + }, + }, + } + + factory := module.NewIaCSecretReachabilityStepFactory() + step, err := factory("reach-step", cfg, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + result, err := step.Execute(context.Background(), &module.PipelineContext{}) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + + allReachable, _ := result.Output["all_reachable"].(bool) + if allReachable { + t.Error("expected all_reachable=false when vault is unreachable") + } + + // The verdict is provider-level: CheckAccess must fire EXACTLY ONCE even + // though two distinct refs are reported. + if stub.checkCount != 1 { + t.Errorf("expected CheckAccess called exactly once (provider-level verdict), got %d", stub.checkCount) + } + + secretsList, ok := result.Output["secrets"].([]map[string]any) + if !ok { + t.Fatalf("expected []map[string]any secrets, got %T", result.Output["secrets"]) + } + if len(secretsList) != 2 { + t.Errorf("expected 2 secret entries (one per distinct ref), got %d", len(secretsList)) + } + for _, entry := range secretsList { + if entry["reachable"] != false { + t.Errorf("expected all entries unreachable when vault is down, got %v", entry["reachable"]) + } + } +} + +// TestSecretReachabilityStep_Factory_RequiresProvider verifies factory validation. +func TestSecretReachabilityStep_Factory_RequiresProvider(t *testing.T) { + factory := module.NewIaCSecretReachabilityStepFactory() + _, err := factory("reach-step", map[string]any{}, nil) + if err == nil { + t.Fatal("expected factory error when 'provider' missing") + } + if !containsString(err.Error(), "provider") { + t.Errorf("expected error to mention 'provider', got: %v", err) + } +} + +// TestSecretReachabilityStep_DirectProvider verifies that the step works when +// the service directly implements secrets.Provider (not via Provider() accessor). +func TestSecretReachabilityStep_DirectProvider(t *testing.T) { + app := module.NewMockApplication() + stub := &stubSecretsProviderWithAccess{checkErr: nil} + // Register the provider directly (not wrapped). + if err := app.RegisterService("direct-provider", stub); err != nil { + t.Fatal(err) + } + + cfg := map[string]any{ + "provider": "direct-provider", + "exec_env": "remote", + "specs": []any{ + map[string]any{ + "name": "my-service", + "type": "infra.service", + "config": map[string]any{ + "api_key": "secret://direct/api-key", + }, + }, + }, + } + + factory := module.NewIaCSecretReachabilityStepFactory() + step, err := factory("reach-step", cfg, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + result, err := step.Execute(context.Background(), &module.PipelineContext{}) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + + allReachable, _ := result.Output["all_reachable"].(bool) + if !allReachable { + t.Error("expected all_reachable=true for direct provider with CheckAccess→nil") + } +} + +// TestSecretReachabilityStep_SpecsFromContext verifies the dynamic specs_from +// path: specs are resolved from the pipeline context at Execute time, and their +// secret:// refs are collected and checked. +func TestSecretReachabilityStep_SpecsFromContext(t *testing.T) { + app := module.NewMockApplication() + stub := &stubSecretsProviderWithAccess{checkErr: nil} + if err := app.RegisterService("my-vault", &stubSecretsModuleAccessor{underlying: stub}); err != nil { + t.Fatal(err) + } + + factory := module.NewIaCSecretReachabilityStepFactory() + step, err := factory("reach-step", map[string]any{ + "provider": "my-vault", + "exec_env": "remote", + "specs_from": "steps.parse-request.body.specs", + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + pc := &module.PipelineContext{ + StepOutputs: map[string]map[string]any{ + "parse-request": { + "body": map[string]any{ + "specs": []any{ + map[string]any{ + "name": "vault-db", + "type": "infra.database", + "config": map[string]any{ + "password": "secret://vault/dynamic-pass", + }, + }, + }, + }, + }, + }, + } + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("Execute error: %v", err) + } + + if !stub.checkCalled { + t.Error("expected CheckAccess to be called for specs_from path") + } + allReachable, _ := result.Output["all_reachable"].(bool) + if !allReachable { + t.Error("expected all_reachable=true for reachable vault via specs_from") + } + secretsList, ok := result.Output["secrets"].([]map[string]any) + if !ok { + t.Fatalf("expected []map[string]any secrets, got %T", result.Output["secrets"]) + } + if len(secretsList) != 1 || secretsList[0]["ref"] != "secret://vault/dynamic-pass" { + t.Errorf("expected one ref secret://vault/dynamic-pass from specs_from, got %#v", secretsList) + } +} + +// TestSecretReachabilityStep_Factory_SpecsAndSpecsFromMutualExclusion verifies +// that setting both 'specs' and 'specs_from' is rejected at factory time. +func TestSecretReachabilityStep_Factory_SpecsAndSpecsFromMutualExclusion(t *testing.T) { + factory := module.NewIaCSecretReachabilityStepFactory() + _, err := factory("reach-step", map[string]any{ + "provider": "my-vault", + "specs_from": "steps.parse-request.body.specs", + "specs": []any{ + map[string]any{"name": "x", "type": "infra.database"}, + }, + }, nil) + if err == nil { + t.Fatal("expected factory error when both specs and specs_from are set") + } + if !containsString(err.Error(), "mutually exclusive") { + t.Errorf("expected 'mutually exclusive' error, got: %v", err) + } +} + +// TestSecretReachabilityStep_SpecsFromEmpty_Errors verifies the fail-safe: a +// specs_from that resolves to a missing/empty context path errors instead of +// silently returning all_reachable=true (which would bypass the gate). +func TestSecretReachabilityStep_SpecsFromEmpty_Errors(t *testing.T) { + app := module.NewMockApplication() + stub := &stubSecretsProviderWithAccess{checkErr: nil} + if err := app.RegisterService("my-vault", &stubSecretsModuleAccessor{underlying: stub}); err != nil { + t.Fatal(err) + } + + factory := module.NewIaCSecretReachabilityStepFactory() + step, err := factory("reach-step", map[string]any{ + "provider": "my-vault", + "exec_env": "remote", + "specs_from": "steps.parse-request.body.specs", + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + // PipelineContext WITHOUT the parse-request output → specs_from resolves nil. + pc := &module.PipelineContext{StepOutputs: map[string]map[string]any{}} + + if _, err := step.Execute(context.Background(), pc); err == nil { + t.Fatal("expected an error when specs_from resolves to empty/missing, got nil (fail-open!)") + } +} diff --git a/plugins/platform/plugin.go b/plugins/platform/plugin.go index 072c1fb5e..9884ea015 100644 --- a/plugins/platform/plugin.go +++ b/plugins/platform/plugin.go @@ -44,7 +44,7 @@ func New() *Plugin { Description: "Platform infrastructure modules, workflow handler, reconciliation trigger, and template step", Tier: plugin.TierCore, ModuleTypes: []string{"platform.provider", "platform.resource", "platform.context", "platform.kubernetes", "platform.dns", "platform.region", "platform.region_router", "iac.state", "app.container", "argo.workflows"}, - StepTypes: []string{"step.platform_template", "step.k8s_plan", "step.k8s_apply", "step.k8s_status", "step.k8s_destroy", "step.iac_plan", "step.iac_apply", "step.iac_status", "step.iac_destroy", "step.iac_drift_detect", "step.iac_provider_list", "step.iac_provider_catalog", "step.iac_provider_plan", "step.iac_provider_apply", "step.iac_provider_destroy", "step.iac_provider_drift", "step.dns_plan", "step.dns_apply", "step.dns_status", "step.app_deploy", "step.app_status", "step.app_rollback", "step.region_deploy", "step.region_promote", "step.region_failover", "step.region_status", "step.region_weight", "step.region_sync", "step.argo_submit", "step.argo_status", "step.argo_logs", "step.argo_delete", "step.argo_list"}, + StepTypes: []string{"step.platform_template", "step.k8s_plan", "step.k8s_apply", "step.k8s_status", "step.k8s_destroy", "step.iac_plan", "step.iac_apply", "step.iac_status", "step.iac_destroy", "step.iac_drift_detect", "step.iac_provider_list", "step.iac_provider_catalog", "step.iac_provider_plan", "step.iac_provider_apply", "step.iac_provider_destroy", "step.iac_provider_drift", "step.iac_secret_reachability", "step.dns_plan", "step.dns_apply", "step.dns_status", "step.app_deploy", "step.app_status", "step.app_rollback", "step.region_deploy", "step.region_promote", "step.region_failover", "step.region_status", "step.region_weight", "step.region_sync", "step.argo_submit", "step.argo_status", "step.argo_logs", "step.argo_delete", "step.argo_list"}, TriggerTypes: []string{"reconciliation"}, WorkflowTypes: []string{"platform"}, }, @@ -152,6 +152,9 @@ func (p *Plugin) StepFactories() map[string]plugin.StepFactory { "step.iac_provider_drift": func(name string, cfg map[string]any, app modular.Application) (any, error) { return module.NewIaCProviderDriftStepFactory()(name, cfg, app) }, + "step.iac_secret_reachability": func(name string, cfg map[string]any, app modular.Application) (any, error) { + return module.NewIaCSecretReachabilityStepFactory()(name, cfg, app) + }, "step.dns_plan": func(name string, cfg map[string]any, app modular.Application) (any, error) { return module.NewDNSPlanStepFactory()(name, cfg, app) }, diff --git a/plugins/platform/plugin_test.go b/plugins/platform/plugin_test.go index 8a6182a1c..8d29559b5 100644 --- a/plugins/platform/plugin_test.go +++ b/plugins/platform/plugin_test.go @@ -47,6 +47,7 @@ func TestStepFactories(t *testing.T) { "step.iac_provider_apply", "step.iac_provider_destroy", "step.iac_provider_drift", + "step.iac_secret_reachability", "step.dns_plan", "step.dns_apply", "step.dns_status", diff --git a/schema/module_schema.go b/schema/module_schema.go index 80120ed52..cabdd1c4d 100644 --- a/schema/module_schema.go +++ b/schema/module_schema.go @@ -2858,6 +2858,7 @@ func (r *ModuleSchemaRegistry) registerBuiltins() { {"step.iac_provider_drift", "IaC Provider Drift", "Detects drift via IaCProvider service"}, {"step.iac_provider_list", "IaC Provider List", "Lists resource statuses from IaCProvider service"}, {"step.iac_provider_plan", "IaC Provider Plan", "Plans infrastructure changes via IaCProvider service"}, + {"step.iac_secret_reachability", "IaC Secret Reachability", "Pre-flight gate: reports whether a plan's secret:// refs are reachable from the chosen exec-env"}, {"step.iac_status", "IaC Status", "Gets IaC provisioning status"}, {"step.k8s_apply", "K8s Apply", "Applies Kubernetes manifests"}, {"step.k8s_destroy", "K8s Destroy", "Deletes Kubernetes resources"}, diff --git a/schema/schema.go b/schema/schema.go index 0ef8316af..06a7f7af0 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -318,6 +318,7 @@ var coreModuleTypes = []string{ "step.iac_provider_drift", "step.iac_provider_list", "step.iac_provider_plan", + "step.iac_secret_reachability", "step.iac_status", "step.jq", "step.json_parse", diff --git a/schema/step_schema_builtins.go b/schema/step_schema_builtins.go index 45b23a677..7689e228c 100644 --- a/schema/step_schema_builtins.go +++ b/schema/step_schema_builtins.go @@ -2215,6 +2215,24 @@ func (r *StepSchemaRegistry) registerBuiltins() { }, }) + // ---- IaC Secret Reachability ---- + + r.Register(&StepSchema{ + Type: "step.iac_secret_reachability", + Plugin: "platform", + Description: "Pre-flight gate: collects the secret:// refs in a plan's specs and reports whether they are reachable from the chosen exec-env (fail-safe; the direct-apply route 409s when any are unreachable). The verdict is provider-level (one AccessChecker probe), reported per ref.", + ConfigFields: []ConfigFieldDef{ + {Key: "provider", Type: FieldTypeString, Description: "Name of the registered secrets.Provider service", Required: true}, + {Key: "exec_env", Type: FieldTypeString, Description: "Execution environment the apply will run in (empty/local/local-docker = local; anything else = remote)"}, + {Key: "specs", Type: FieldTypeArray, Description: "Static resource specs (mutually exclusive with specs_from)"}, + {Key: "specs_from", Type: FieldTypeString, Description: "Pipeline-context path to the specs (e.g. steps.parse-request.body.specs); mutually exclusive with specs"}, + }, + Outputs: []StepOutputDef{ + {Key: "secrets", Type: "[]any", Description: "Per-ref verdicts: {ref, reachable, reason}"}, + {Key: "all_reachable", Type: "boolean", Description: "True iff every referenced secret is reachable from the exec-env"}, + }, + }) + // ---- Kubernetes Apply ---- r.Register(&StepSchema{ diff --git a/schema/testdata/editor-schemas.golden.json b/schema/testdata/editor-schemas.golden.json index 10fb3c71d..fab7b3e06 100644 --- a/schema/testdata/editor-schemas.golden.json +++ b/schema/testdata/editor-schemas.golden.json @@ -6055,6 +6055,13 @@ "description": "Plans infrastructure changes via IaCProvider service", "configFields": [] }, + "step.iac_secret_reachability": { + "type": "step.iac_secret_reachability", + "label": "IaC Secret Reachability", + "category": "pipeline", + "description": "Pre-flight gate: reports whether a plan's secret:// refs are reachable from the chosen exec-env", + "configFields": [] + }, "step.iac_status": { "type": "step.iac_status", "label": "IaC Status", diff --git a/secrets/reachability.go b/secrets/reachability.go new file mode 100644 index 000000000..73fe1f1e6 --- /dev/null +++ b/secrets/reachability.go @@ -0,0 +1,151 @@ +package secrets + +import ( + "context" + "fmt" + "reflect" +) + +// Result holds the outcome of a Reachability check. +// +// Reachable is true when the secrets provider can be reached from the given +// execution environment. Reason is empty when Reachable is true, and contains a +// human-readable (non-credential-leaking) explanation when Reachable is false. +type Result struct { + Reachable bool + Reason string +} + +// isLocalExecEnv returns true for execution environments that are considered +// local — i.e., the same host where the engine (and its configured secrets +// providers) run. Host-local secrets backends (env, file, keychain) are only +// verifiably reachable from these environments. +func isLocalExecEnv(execEnv string) bool { + return execEnv == "" || execEnv == "local" || execEnv == "local-docker" +} + +// hostLocalKind returns a short label for a host-local backend type, or "" if p +// is not a host-local backend. Used to classify and to build the fail-safe +// reason string. +func hostLocalKind(p Provider) string { + switch p.(type) { + case *EnvProvider: + return "env" + case *FileProvider: + return "file" + case *KeychainProvider: + return "keychain" + default: + return "" + } +} + +// Reachability determines whether the secrets referenced by a plan can be read +// from the given execution environment (execEnv). It is designed to be +// fail-safe: when the answer is uncertain and the exec-env is remote, it +// returns unreachable rather than failing open. +// +// ctx bounds any backend probe (AccessChecker.CheckAccess). Callers MUST pass a +// deadline-bearing context (e.g. the pipeline/route ctx) so a slow or +// unreachable remote backend cannot hang the pre-flight (vault default ~60s, +// aws ~10s) past the request deadline. +// +// Classification uses a concrete type-switch rather than Name() string-matching. +// Name() is a config-assigned identifier that operators may override; the +// concrete type is the authoritative signal for capability detection. +// +// Rules (in order): +// +// 1. *EnvProvider, *FileProvider, *KeychainProvider — host-local backends. +// Reachable ONLY when execEnv is local ("" | "local" | "local-docker"). +// For a REMOTE exec-env these are fail-safe UNREACHABLE: under the +// agent-side-resolution model (ADR 0017), a remote exec-env resolves +// secrets with the remote agent's OWN provider, so the engine cannot vouch +// that the engine-host's env vars / files / OS keychain exist on the remote +// runner (e.g. a macOS keychain entry is not present on a remote Linux +// runner). This is intentionally conservative: it is the safe default until +// the agent-probe hardening filed in ADR 0017 lands. +// +// 2. *GitHubSecretsProvider — short-circuited to unreachable BEFORE any +// AccessChecker call. GitHub secrets are write-only: Get() returns +// ErrUnsupported and values are only injected at CI-job startup. No +// exec-env can read them at runtime. CheckAccess on the GitHub provider +// would succeed (it probes the public key endpoint, not Get), so probing +// it would give a false "reachable" verdict. +// +// 3. All other providers that implement AccessChecker — call CheckAccess(ctx). +// Reachable iff CheckAccess returns nil. +// +// 4. Providers that do NOT implement AccessChecker + remote execEnv — fail-safe +// unreachable ("reachability unknown for remote exec-env; assuming unreachable"). +// We never fail open for an unknown/unclassified backend in a remote context. +// +// 5. Providers that do NOT implement AccessChecker + local execEnv — treat as +// reachable (local operation; the runtime will surface access errors if any). +func Reachability(ctx context.Context, p Provider, execEnv string) Result { + // Fail-safe: a nil provider — including a typed-nil pointer stored in the + // Provider interface (e.g. (*VaultProvider)(nil)) — is never reachable, and + // calling into it could panic on a nil receiver. Reject before any dispatch. + if isNilProvider(p) { + return Result{Reachable: false, Reason: "no secrets provider configured (nil provider)"} + } + if kind := hostLocalKind(p); kind != "" { + // Host-local backend — verifiable only from a local exec-env. + if isLocalExecEnv(execEnv) { + return Result{Reachable: true} + } + return Result{ + Reachable: false, + Reason: fmt.Sprintf("host-local backend (%s) is not verifiable from a remote exec-env; the remote agent resolves its own secrets (ADR 0017)", kind), + } + } + + switch p.(type) { + case *GitHubSecretsProvider: + // Write-only short-circuit: GitHub secrets inject at CI time only. + // CheckAccess is intentionally NOT called — it would return a false + // positive (probing the public key endpoint, not read capability). + return Result{ + Reachable: false, + Reason: "write-only — secrets inject at CI time only; no exec-env can read them", + } + + default: + // For all remaining providers: check whether they implement AccessChecker. + if ac, ok := p.(AccessChecker); ok { + if err := ac.CheckAccess(ctx); err != nil { + return Result{ + Reachable: false, + Reason: fmt.Sprintf("access check failed: %s", err.Error()), + } + } + return Result{Reachable: true} + } + + // No AccessChecker implementation: + // - Local exec-env: treat as reachable (the runtime will catch errors). + // - Remote exec-env: fail-safe unreachable. + if isLocalExecEnv(execEnv) { + return Result{Reachable: true} + } + return Result{ + Reachable: false, + Reason: "reachability unknown for remote exec-env; assuming unreachable", + } + } +} + +// isNilProvider reports whether p is nil or a typed-nil pointer/interface stored +// in the Provider interface — both of which must be treated as unreachable. +func isNilProvider(p Provider) bool { + if p == nil { + return true + } + v := reflect.ValueOf(p) + switch v.Kind() { + case reflect.Ptr, reflect.Interface, reflect.Map, reflect.Slice, reflect.Func, reflect.Chan: + return v.IsNil() + default: + return false + } +} diff --git a/secrets/reachability_test.go b/secrets/reachability_test.go new file mode 100644 index 000000000..b99a67116 --- /dev/null +++ b/secrets/reachability_test.go @@ -0,0 +1,379 @@ +package secrets_test + +import ( + "context" + "errors" + "testing" + + "github.com/GoCodeAlone/workflow/secrets" +) + +// ─── stub types for Reachability tests ─────────────────────────────────────── + +// stubRemoteWithAccess implements Provider + AccessChecker and returns nil from +// CheckAccess (simulates a reachable remote backend). +type stubRemoteWithAccess struct { + checkAccessErr error + checkCalled bool +} + +func (s *stubRemoteWithAccess) Name() string { return "stub-remote-access" } +func (s *stubRemoteWithAccess) Get(_ context.Context, _ string) (string, error) { return "", nil } +func (s *stubRemoteWithAccess) Set(_ context.Context, _, _ string) error { return nil } +func (s *stubRemoteWithAccess) Delete(_ context.Context, _ string) error { return nil } +func (s *stubRemoteWithAccess) List(_ context.Context) ([]string, error) { return nil, nil } +func (s *stubRemoteWithAccess) CheckAccess(_ context.Context) error { + s.checkCalled = true + return s.checkAccessErr +} + +// stubRemoteNoAccess implements Provider only (no AccessChecker). +type stubRemoteNoAccess struct{} + +func (s *stubRemoteNoAccess) Name() string { return "stub-remote-no-access" } +func (s *stubRemoteNoAccess) Get(_ context.Context, _ string) (string, error) { return "", nil } +func (s *stubRemoteNoAccess) Set(_ context.Context, _, _ string) error { return nil } +func (s *stubRemoteNoAccess) Delete(_ context.Context, _ string) error { return nil } +func (s *stubRemoteNoAccess) List(_ context.Context) ([]string, error) { return nil, nil } + +// stubGitHubLikeWithAccess is a provider that behaves like GitHub: +// Get returns ErrUnsupported but ALSO implements AccessChecker (returns nil). +// This tests that the GitHub short-circuit fires before CheckAccess is consulted. +type stubGitHubLikeWithAccess struct { + checkCalled bool +} + +func (s *stubGitHubLikeWithAccess) Name() string { return "stub-github-like" } +func (s *stubGitHubLikeWithAccess) Get(_ context.Context, _ string) (string, error) { + return "", secrets.ErrUnsupported +} +func (s *stubGitHubLikeWithAccess) Set(_ context.Context, _, _ string) error { return nil } +func (s *stubGitHubLikeWithAccess) Delete(_ context.Context, _ string) error { return nil } +func (s *stubGitHubLikeWithAccess) List(_ context.Context) ([]string, error) { return nil, nil } +func (s *stubGitHubLikeWithAccess) CheckAccess(_ context.Context) error { + s.checkCalled = true + return nil +} + +// compile-time interface assertions +var _ secrets.Provider = (*stubRemoteWithAccess)(nil) +var _ secrets.AccessChecker = (*stubRemoteWithAccess)(nil) +var _ secrets.Provider = (*stubRemoteNoAccess)(nil) +var _ secrets.Provider = (*stubGitHubLikeWithAccess)(nil) +var _ secrets.AccessChecker = (*stubGitHubLikeWithAccess)(nil) + +// ─── TestReachability ───────────────────────────────────────────────────────── + +func TestReachability(t *testing.T) { + cases := []struct { + name string + provider secrets.Provider + execEnv string + wantReachable bool + wantReasonSub string // non-empty: the reason must contain this substring + }{ + // ── Host-local backends + LOCAL exec-env — reachable ────────────────── + { + name: "EnvProvider local-exec", + provider: secrets.NewEnvProvider(""), + execEnv: "local", + wantReachable: true, + }, + { + name: "EnvProvider empty (local) exec", + provider: secrets.NewEnvProvider("APP_"), + execEnv: "", + wantReachable: true, + }, + { + name: "FileProvider local-docker exec", + provider: secrets.NewFileProvider("/tmp"), + execEnv: "local-docker", + wantReachable: true, + }, + { + name: "KeychainProvider local exec", + provider: newTestKeychainProvider(t), + execEnv: "local", + wantReachable: true, + }, + + // ── Host-local backends + REMOTE exec-env — fail-safe UNREACHABLE ────── + // Under ADR 0017 the remote agent resolves its own secrets; the engine + // cannot vouch for engine-host env/file/keychain on a remote runner. + { + name: "EnvProvider remote-exec → fail-safe unreachable", + provider: secrets.NewEnvProvider("APP_"), + execEnv: "remote", + wantReachable: false, + wantReasonSub: "host-local backend (env)", + }, + { + name: "FileProvider remote-exec → fail-safe unreachable", + provider: secrets.NewFileProvider("/tmp"), + execEnv: "k8s-prod", + wantReachable: false, + wantReasonSub: "host-local backend (file)", + }, + { + name: "KeychainProvider remote-exec → fail-safe unreachable", + provider: newTestKeychainProvider(t), + execEnv: "remote", + wantReachable: false, + wantReasonSub: "host-local backend (keychain)", + }, + + // ── GitHubSecretsProvider — always unreachable (write-only short-circuit) ─ + // The stub has CheckAccess returning nil to prove it is NOT consulted. + { + name: "GitHubSecretsProvider write-only short-circuit (local exec)", + provider: newTestGitHubProvider(t), + execEnv: "local", + wantReachable: false, + wantReasonSub: "write-only", + }, + { + name: "GitHubSecretsProvider write-only short-circuit (remote exec)", + provider: newTestGitHubProvider(t), + execEnv: "remote", + wantReachable: false, + wantReasonSub: "write-only", + }, + + // ── Remote backends WITH AccessChecker ──────────────────────────────── + { + name: "remote with AccessChecker nil → reachable", + provider: &stubRemoteWithAccess{checkAccessErr: nil}, + execEnv: "remote", + wantReachable: true, + }, + { + name: "remote with AccessChecker err → unreachable", + provider: &stubRemoteWithAccess{checkAccessErr: errors.New("connection refused")}, + execEnv: "remote", + wantReachable: false, + wantReasonSub: "connection refused", + }, + + // ── Remote backends WITHOUT AccessChecker ───────────────────────────── + { + name: "remote WITHOUT AccessChecker + remote execEnv → fail-safe unreachable", + provider: &stubRemoteNoAccess{}, + execEnv: "remote", + wantReachable: false, + wantReasonSub: "reachability unknown", + }, + { + name: "remote WITHOUT AccessChecker + empty execEnv → reachable (local assumed)", + provider: &stubRemoteNoAccess{}, + execEnv: "", + wantReachable: true, + }, + { + name: "remote WITHOUT AccessChecker + 'local' execEnv → reachable", + provider: &stubRemoteNoAccess{}, + execEnv: "local", + wantReachable: true, + }, + { + name: "remote WITHOUT AccessChecker + 'local-docker' execEnv → reachable", + provider: &stubRemoteNoAccess{}, + execEnv: "local-docker", + wantReachable: true, + }, + + // ── Unknown provider type + remote execEnv → fail-safe ──────────────── + { + name: "unknown provider + remote execEnv → fail-safe unreachable", + provider: &stubRemoteNoAccess{}, + execEnv: "prod-k8s", + wantReachable: false, + wantReasonSub: "reachability unknown", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + result := secrets.Reachability(context.Background(), tc.provider, tc.execEnv) + if result.Reachable != tc.wantReachable { + t.Errorf("Reachable = %v, want %v (reason: %q)", result.Reachable, tc.wantReachable, result.Reason) + } + if tc.wantReasonSub != "" { + if result.Reason == "" { + t.Errorf("expected Reason containing %q, got empty", tc.wantReasonSub) + } else if !containsSubstr(result.Reason, tc.wantReasonSub) { + t.Errorf("expected Reason containing %q, got %q", tc.wantReasonSub, result.Reason) + } + } + if tc.wantReachable && result.Reason != "" { + t.Errorf("expected empty Reason when reachable, got %q", result.Reason) + } + }) + } +} + +// TestReachability_GitHubShortCircuit_CheckAccessNotConsulted proves that the +// GitHub write-only short-circuit fires BEFORE any AccessChecker call. We use a +// real *secrets.GitHubSecretsProvider for the type-switch, but we also verify the +// pattern with our stub whose CheckAccess is instrumented. +// +// For the stub: the key invariant is that a provider whose Get returns ErrUnsupported +// should be short-circuited — the stub is NOT a *GitHubSecretsProvider, so this +// test specifically covers the *GitHubSecretsProvider path via the real type. +func TestReachability_GitHubShortCircuit_CheckAccessNotConsulted(t *testing.T) { + p := newTestGitHubProvider(t) + + result := secrets.Reachability(context.Background(), p, "remote") + if result.Reachable { + t.Error("GitHubSecretsProvider must be unreachable (write-only)") + } + if !containsSubstr(result.Reason, "write-only") { + t.Errorf("expected reason to mention write-only, got %q", result.Reason) + } + // The short-circuit must fire without making any network call. + // We can't instrument the real GitHubSecretsProvider's CheckAccess, but we + // verify the stubGitHubLikeWithAccess (same shape) is NOT consulted: + stub := &stubGitHubLikeWithAccess{} + // stubGitHubLikeWithAccess is NOT *GitHubSecretsProvider, so the type-switch + // won't catch it — it will fall through to the generic remote path. + // This is intentional: the type-switch is explicitly for *GitHubSecretsProvider. + // Verify the stub does get its CheckAccess called (it's a regular remote-with-access). + result2 := secrets.Reachability(context.Background(), stub, "remote") + if !result2.Reachable { + t.Errorf("stub with CheckAccess→nil should be reachable, got reason: %q", result2.Reason) + } + if !stub.checkCalled { + t.Error("expected CheckAccess to be called on stub remote provider") + } +} + +// TestReachability_VaultAndAWS_WithAccessChecker covers the AccessChecker branch +// that *VaultProvider and *AWSSecretsManagerProvider hit (rule 3). Those concrete +// providers require live clients, so this test uses stubRemoteWithAccess — a stub +// implementing AccessChecker — to exercise the same default-case probe path. It +// does NOT instantiate the real provider types. +func TestReachability_VaultAndAWS_WithAccessChecker(t *testing.T) { + // stubRemoteWithAccess covers the same code path as *VaultProvider and + // *AWSSecretsManagerProvider: it implements AccessChecker. + stub := &stubRemoteWithAccess{checkAccessErr: nil} + result := secrets.Reachability(context.Background(), stub, "prod-env") + if !result.Reachable { + t.Errorf("remote with CheckAccess→nil should be reachable, got: %q", result.Reason) + } + if !stub.checkCalled { + t.Error("expected CheckAccess to be called") + } + + stubErr := &stubRemoteWithAccess{checkAccessErr: errors.New("vault: token expired")} + result2 := secrets.Reachability(context.Background(), stubErr, "prod-env") + if result2.Reachable { + t.Error("remote with CheckAccess returning err should be unreachable") + } + if !containsSubstr(result2.Reason, "token expired") { + t.Errorf("expected reason to contain error text, got %q", result2.Reason) + } +} + +// stubCtxObservingProvider records the context it received in CheckAccess so a +// test can prove ctx is propagated (not context.Background()). +type stubCtxObservingProvider struct { + gotCtx context.Context +} + +func (s *stubCtxObservingProvider) Name() string { return "stub-ctx" } +func (s *stubCtxObservingProvider) Get(_ context.Context, _ string) (string, error) { return "", nil } +func (s *stubCtxObservingProvider) Set(_ context.Context, _, _ string) error { return nil } +func (s *stubCtxObservingProvider) Delete(_ context.Context, _ string) error { return nil } +func (s *stubCtxObservingProvider) List(_ context.Context) ([]string, error) { return nil, nil } +func (s *stubCtxObservingProvider) CheckAccess(ctx context.Context) error { + s.gotCtx = ctx + return ctx.Err() +} + +var _ secrets.AccessChecker = (*stubCtxObservingProvider)(nil) + +// TestReachability_PropagatesContext proves Reachability passes the caller's ctx +// (not a fresh context.Background) into AccessChecker.CheckAccess, so a route / +// pipeline deadline bounds the probe. An already-cancelled context must surface +// as an unreachable verdict. +func TestReachability_PropagatesContext(t *testing.T) { + stub := &stubCtxObservingProvider{} + + type ctxKey string + const k ctxKey = "marker" + ctx, cancel := context.WithCancel(context.WithValue(context.Background(), k, "v")) + cancel() // already cancelled — CheckAccess returns ctx.Err() + + result := secrets.Reachability(ctx, stub, "remote") + + if stub.gotCtx == nil { + t.Fatal("CheckAccess did not receive a context") + } + if stub.gotCtx.Value(k) != "v" { + t.Error("Reachability did not propagate the caller's context into CheckAccess (got a different context)") + } + if result.Reachable { + t.Error("expected unreachable when the propagated context is already cancelled") + } + if !containsSubstr(result.Reason, "access check failed") { + t.Errorf("expected access-check-failed reason, got %q", result.Reason) + } +} + +// ─── helpers ────────────────────────────────────────────────────────────────── + +// newTestGitHubProvider creates a real *secrets.GitHubSecretsProvider using a +// fake token so the type-switch in Reachability fires correctly. +// It sets the WORKFLOW_TEST_GH_TOKEN env var temporarily (the name passed into +// NewGitHubSecretsProvider) so the provider doesn't reject an empty token. +func newTestGitHubProvider(t *testing.T) *secrets.GitHubSecretsProvider { + t.Helper() + t.Setenv("WORKFLOW_TEST_GH_TOKEN", "ghp_fake_token_for_type_switch_test") + p, err := secrets.NewGitHubSecretsProvider("owner/repo", "WORKFLOW_TEST_GH_TOKEN") + if err != nil { + t.Fatalf("NewGitHubSecretsProvider: %v", err) + } + return p +} + +// newTestKeychainProvider creates a real *secrets.KeychainProvider so the +// host-local type-switch in Reachability fires correctly. It performs no OS +// keychain I/O (Reachability classifies by type, it does not probe keychain). +func newTestKeychainProvider(t *testing.T) *secrets.KeychainProvider { + t.Helper() + p, err := secrets.NewKeychainProvider("workflow-test-reachability") + if err != nil { + t.Fatalf("NewKeychainProvider: %v", err) + } + return p +} + +func containsSubstr(s, sub string) bool { + if len(sub) == 0 { + return true + } + for i := 0; i <= len(s)-len(sub); i++ { + if s[i:i+len(sub)] == sub { + return true + } + } + return false +} + +// TestReachability_NilProvider verifies a nil or typed-nil provider is fail-safe +// unreachable (never panics, never reports reachable). +func TestReachability_NilProvider(t *testing.T) { + // Untyped nil. + if r := secrets.Reachability(context.Background(), nil, "local"); r.Reachable { + t.Errorf("nil provider should be unreachable, got reachable") + } + // Typed-nil pointer stored in the Provider interface. + var typedNil *secrets.VaultProvider + r := secrets.Reachability(context.Background(), typedNil, "local") + if r.Reachable { + t.Errorf("typed-nil *VaultProvider should be unreachable, got reachable") + } + if r.Reason == "" { + t.Errorf("expected a non-empty reason for nil provider") + } +}