From 055ed4a55c21190e0f828ea05373621aa4172bda Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Mar 2026 19:20:51 +0000 Subject: [PATCH 1/4] Initial plan From 736307b42e09c91414d1800ce969e86bccc90590 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Mar 2026 19:34:00 +0000 Subject: [PATCH 2/4] feat: add step.secret_fetch pipeline step for tenant-aware secret resolution Implements a new pipeline step type that fetches secrets from secrets.aws or secrets.vault modules at runtime. Secret IDs/ARNs support Go template expressions evaluated against the live pipeline context, enabling per-tenant dynamic secret resolution. Changes: - module/pipeline_step_secret_fetch.go: SecretFetchProvider interface, SecretFetchStep, NewSecretFetchStepFactory - module/pipeline_step_secret_fetch_test.go: 12 tests covering factory validation, single/multi fetch, dynamic/tenant-aware resolution, error handling - plugins/pipelinesteps/plugin.go: register step.secret_fetch - plugins/pipelinesteps/plugin_test.go: update expected step count - DOCUMENTATION.md: table entry + detailed section with examples Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- DOCUMENTATION.md | 74 +++++ mcp/server.go | 2 +- modernize/manifest_rule_test.go | 8 +- module/pipeline_step_secret_fetch.go | 117 ++++++++ module/pipeline_step_secret_fetch_test.go | 313 ++++++++++++++++++++++ plugins/all/all.go | 2 +- plugins/pipelinesteps/plugin.go | 2 + plugins/pipelinesteps/plugin_test.go | 1 + 8 files changed, 513 insertions(+), 6 deletions(-) create mode 100644 module/pipeline_step_secret_fetch.go create mode 100644 module/pipeline_step_secret_fetch_test.go diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 1aa4acf9..f5804fc1 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -183,6 +183,7 @@ flowchart TD | `step.http_proxy` | Proxies an HTTP request to an upstream service | pipelinesteps | | `step.hash` | Computes a cryptographic hash (md5/sha256/sha512) of a template-resolved input | pipelinesteps | | `step.regex_match` | Matches a regular expression against a template-resolved input | pipelinesteps | +| `step.secret_fetch` | Fetches one or more secrets from a secrets module (secrets.aws, secrets.vault) with dynamic tenant-aware secret ID resolution | pipelinesteps | | `step.jq` | Applies a JQ expression to pipeline data for complex transformations | pipelinesteps | | `step.ai_complete` | AI text completion using a configured provider | ai | | `step.ai_classify` | AI text classification into named categories | ai | @@ -1103,6 +1104,79 @@ steps: --- +### `step.secret_fetch` + +Fetches one or more secrets from a named secrets module (`secrets.aws`, `secrets.vault`, etc.) and exposes the resolved values as step outputs. Secret IDs / ARNs are Go template expressions evaluated against the live pipeline context, enabling **per-tenant dynamic secret resolution**. + +**Configuration:** + +| Key | Type | Required | Description | +|-----|------|----------|-------------| +| `module` | string | yes | Service name of the secrets module (the `name` field in the module config). | +| `secrets` | map[string]string | yes | Map of output key → secret ID/ARN. Values support Go template expressions for dynamic resolution. | + +**Output fields:** One field per key in `secrets`, each containing the resolved secret value. `fetched: true` when all secrets were successfully fetched. + +**Examples:** + +Static secret IDs: + +```yaml +steps: + - name: fetch_creds + type: step.secret_fetch + config: + module: aws-secrets + secrets: + token_url: "arn:aws:secretsmanager:us-east-1:123:secret:token-url" + client_id: "arn:aws:secretsmanager:us-east-1:123:secret:client-id" + client_secret: "arn:aws:secretsmanager:us-east-1:123:secret:client-secret" +``` + +Tenant-aware dynamic resolution (ARNs from a previous step's output): + +```yaml +steps: + - name: lookup_integration + type: step.db_query + config: + query: "SELECT * FROM integrations WHERE tenant_id = $1" + params: ["{{.tenant_id}}"] + + - name: fetch_creds + type: step.secret_fetch + config: + module: aws-secrets + secrets: + token_url: "{{.steps.lookup_integration.row.token_url_secret_arn}}" + client_id: "{{.steps.lookup_integration.row.client_id_secret_arn}}" + client_secret: "{{.steps.lookup_integration.row.client_secret_secret_arn}}" + + - name: call_api + type: step.http_call + config: + url: "https://login.example.com/services/oauth2/token" + method: POST + oauth2: + token_url: "{{.steps.fetch_creds.token_url}}" + client_id: "{{.steps.fetch_creds.client_id}}" + client_secret: "{{.steps.fetch_creds.client_secret}}" +``` + +Per-tenant ARN construction using trigger data: + +```yaml +steps: + - name: fetch_tenant_secret + type: step.secret_fetch + config: + module: aws-secrets + secrets: + api_key: "arn:aws:secretsmanager:us-east-1:123:secret:{{.tenant_id}}-api-key" +``` + +--- + ### `step.ai_complete` Invokes an AI provider to produce a text completion. Provider resolution order: explicit `provider` name, then model-based lookup, then first registered provider. diff --git a/mcp/server.go b/mcp/server.go index 61bdf5f4..4bb1c0d9 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -74,7 +74,7 @@ type Server struct { mcpServer *server.MCPServer pluginDir string registryDir string - documentationFile string // optional explicit path to DOCUMENTATION.md + documentationFile string // optional explicit path to DOCUMENTATION.md engine EngineProvider // optional; enables execution tools when set } diff --git a/modernize/manifest_rule_test.go b/modernize/manifest_rule_test.go index 3025ca82..bcc6a8ca 100644 --- a/modernize/manifest_rule_test.go +++ b/modernize/manifest_rule_test.go @@ -453,11 +453,11 @@ modules: // the Fix function does not create duplicate keys. func TestManifestRule_ModuleConfigKeyRename_Collision(t *testing.T) { mr := ManifestRule{ - ID: "test-collision-mod", + ID: "test-collision-mod", Description: "Rename old_key to new_key in my.module", - ModuleType: "my.module", - OldKey: "old_key", - NewKey: "new_key", + ModuleType: "my.module", + OldKey: "old_key", + NewKey: "new_key", } rule, err := mr.ToRule() if err != nil { diff --git a/module/pipeline_step_secret_fetch.go b/module/pipeline_step_secret_fetch.go new file mode 100644 index 00000000..afdd463b --- /dev/null +++ b/module/pipeline_step_secret_fetch.go @@ -0,0 +1,117 @@ +package module + +import ( + "context" + "fmt" + + "github.com/GoCodeAlone/modular" +) + +// SecretFetchProvider is the minimal interface required by SecretFetchStep. +// Both SecretsAWSModule and SecretsVaultModule satisfy this interface. +type SecretFetchProvider interface { + Get(ctx context.Context, key string) (string, error) +} + +// SecretFetchStep fetches one or more secrets from a named secrets module +// (e.g. secrets.aws, secrets.vault) and exposes the resolved values as step +// outputs. Secret IDs / ARNs are Go template expressions evaluated against +// the live PipelineContext, enabling per-tenant dynamic resolution: +// +// config: +// module: aws-secrets +// secrets: +// token_url: "{{.steps.lookup.row.token_url_arn}}" +// client_id: "{{.steps.lookup.row.client_id_arn}}" +type SecretFetchStep struct { + name string + moduleName string // service name registered by the secrets module + secrets map[string]string // output key → secret ID/ARN (may contain templates) + app modular.Application + tmpl *TemplateEngine +} + +// NewSecretFetchStepFactory returns a StepFactory that creates SecretFetchStep instances. +func NewSecretFetchStepFactory() StepFactory { + return func(name string, config map[string]any, app modular.Application) (PipelineStep, error) { + moduleName, _ := config["module"].(string) + if moduleName == "" { + return nil, fmt.Errorf("secret_fetch step %q: 'module' is required", name) + } + + raw, _ := config["secrets"].(map[string]any) + if len(raw) == 0 { + return nil, fmt.Errorf("secret_fetch step %q: 'secrets' map is required and must not be empty", name) + } + + secretMap := make(map[string]string, len(raw)) + for k, v := range raw { + idStr, ok := v.(string) + if !ok { + return nil, fmt.Errorf("secret_fetch step %q: secrets[%q] must be a string (secret ID or ARN)", name, k) + } + secretMap[k] = idStr + } + + return &SecretFetchStep{ + name: name, + moduleName: moduleName, + secrets: secretMap, + app: app, + tmpl: NewTemplateEngine(), + }, nil + } +} + +// Name returns the step name. +func (s *SecretFetchStep) Name() string { return s.name } + +// Execute resolves the secret IDs/ARNs using the pipeline context (enabling +// per-tenant dynamic resolution), fetches each secret from the named secrets +// module, and returns the resolved values as step output. +func (s *SecretFetchStep) Execute(ctx context.Context, pc *PipelineContext) (*StepResult, error) { + if s.app == nil { + return nil, fmt.Errorf("secret_fetch step %q: no application context", s.name) + } + + provider, err := s.resolveProvider() + if err != nil { + return nil, err + } + + output := make(map[string]any, len(s.secrets)+1) + + for outputKey, idTemplate := range s.secrets { + // Resolve the secret ID/ARN template against the current pipeline context. + // This enables tenant-aware ARNs such as: + // "arn:aws:secretsmanager:us-east-1:123:secret:{{.tenant_id}}-creds" + resolvedID, resolveErr := s.tmpl.Resolve(idTemplate, pc) + if resolveErr != nil { + return nil, fmt.Errorf("secret_fetch step %q: failed to resolve secret ID for %q: %w", s.name, outputKey, resolveErr) + } + + value, fetchErr := provider.Get(ctx, resolvedID) + if fetchErr != nil { + return nil, fmt.Errorf("secret_fetch step %q: failed to fetch secret %q (id=%q): %w", s.name, outputKey, resolvedID, fetchErr) + } + + output[outputKey] = value + } + + output["fetched"] = true + return &StepResult{Output: output}, nil +} + +// resolveProvider looks up the SecretFetchProvider from the application service +// registry using the configured module name. +func (s *SecretFetchStep) resolveProvider() (SecretFetchProvider, error) { + svc, ok := s.app.SvcRegistry()[s.moduleName] + if !ok { + return nil, fmt.Errorf("secret_fetch step %q: secrets module %q not found in service registry", s.name, s.moduleName) + } + provider, ok := svc.(SecretFetchProvider) + if !ok { + return nil, fmt.Errorf("secret_fetch step %q: service %q does not implement SecretFetchProvider (Get method)", s.name, s.moduleName) + } + return provider, nil +} diff --git a/module/pipeline_step_secret_fetch_test.go b/module/pipeline_step_secret_fetch_test.go new file mode 100644 index 00000000..3238fc5f --- /dev/null +++ b/module/pipeline_step_secret_fetch_test.go @@ -0,0 +1,313 @@ +package module + +import ( + "context" + "errors" + "testing" +) + +// mockSecretProvider is an in-memory SecretFetchProvider for testing. +type mockSecretProvider struct { + data map[string]string + getErr error +} + +func newMockSecretProvider(data map[string]string) *mockSecretProvider { + return &mockSecretProvider{data: data} +} + +func (m *mockSecretProvider) Get(_ context.Context, key string) (string, error) { + if m.getErr != nil { + return "", m.getErr + } + v, ok := m.data[key] + if !ok { + return "", errors.New("secret not found: " + key) + } + return v, nil +} + +// mockAppWithSecrets creates a MockApplication with a SecretFetchProvider registered. +func mockAppWithSecrets(name string, p SecretFetchProvider) *MockApplication { + app := NewMockApplication() + app.Services[name] = p + return app +} + +// --- factory validation tests --- + +func TestSecretFetchStep_MissingModule(t *testing.T) { + factory := NewSecretFetchStepFactory() + _, err := factory("bad", map[string]any{ + "secrets": map[string]any{"key": "arn:x"}, + }, nil) + if err == nil { + t.Fatal("expected error when 'module' is missing") + } +} + +func TestSecretFetchStep_MissingSecrets(t *testing.T) { + factory := NewSecretFetchStepFactory() + _, err := factory("bad", map[string]any{ + "module": "aws-secrets", + }, nil) + if err == nil { + t.Fatal("expected error when 'secrets' is missing") + } +} + +func TestSecretFetchStep_EmptySecrets(t *testing.T) { + factory := NewSecretFetchStepFactory() + _, err := factory("bad", map[string]any{ + "module": "aws-secrets", + "secrets": map[string]any{}, + }, nil) + if err == nil { + t.Fatal("expected error when 'secrets' map is empty") + } +} + +func TestSecretFetchStep_NonStringSecretID(t *testing.T) { + factory := NewSecretFetchStepFactory() + _, err := factory("bad", map[string]any{ + "module": "aws-secrets", + "secrets": map[string]any{ + "mykey": 42, // not a string + }, + }, nil) + if err == nil { + t.Fatal("expected error when secret ID is not a string") + } +} + +// --- execute tests --- + +func TestSecretFetchStep_FetchSingle(t *testing.T) { + provider := newMockSecretProvider(map[string]string{ + "arn:aws:secretsmanager:us-east-1:123:secret:my-token": "tok-xyz", + }) + app := mockAppWithSecrets("aws-secrets", provider) + + factory := NewSecretFetchStepFactory() + step, err := factory("fetch-creds", map[string]any{ + "module": "aws-secrets", + "secrets": map[string]any{ + "token": "arn:aws:secretsmanager:us-east-1:123:secret:my-token", + }, + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + pc := NewPipelineContext(nil, nil) + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("execute error: %v", err) + } + + if result.Output["token"] != "tok-xyz" { + t.Errorf("expected token=tok-xyz, got %v", result.Output["token"]) + } + if result.Output["fetched"] != true { + t.Errorf("expected fetched=true, got %v", result.Output["fetched"]) + } +} + +func TestSecretFetchStep_FetchMultiple(t *testing.T) { + provider := newMockSecretProvider(map[string]string{ + "arn:secret:token-url": "https://login.example.com/oauth/token", + "arn:secret:client-id": "client-abc", + "arn:secret:client-secret": "super-secret", + }) + app := mockAppWithSecrets("aws-secrets", provider) + + factory := NewSecretFetchStepFactory() + step, err := factory("fetch-creds", map[string]any{ + "module": "aws-secrets", + "secrets": map[string]any{ + "token_url": "arn:secret:token-url", + "client_id": "arn:secret:client-id", + "client_secret": "arn:secret:client-secret", + }, + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + pc := NewPipelineContext(nil, nil) + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("execute error: %v", err) + } + + if result.Output["token_url"] != "https://login.example.com/oauth/token" { + t.Errorf("unexpected token_url: %v", result.Output["token_url"]) + } + if result.Output["client_id"] != "client-abc" { + t.Errorf("unexpected client_id: %v", result.Output["client_id"]) + } + if result.Output["client_secret"] != "super-secret" { + t.Errorf("unexpected client_secret: %v", result.Output["client_secret"]) + } + if result.Output["fetched"] != true { + t.Errorf("expected fetched=true") + } +} + +// TestSecretFetchStep_TenantAwareDynamic verifies that secret IDs support +// Go template expressions so ARNs can be constructed per-tenant at runtime. +func TestSecretFetchStep_TenantAwareDynamic(t *testing.T) { + provider := newMockSecretProvider(map[string]string{ + "arn:aws:secretsmanager:us-east-1:123:secret:tenant-acme-creds": "acme-token", + }) + app := mockAppWithSecrets("aws-secrets", provider) + + factory := NewSecretFetchStepFactory() + step, err := factory("fetch-creds", map[string]any{ + "module": "aws-secrets", + "secrets": map[string]any{ + // Template expression resolved against pipeline context (tenant_id from trigger data). + "token": "arn:aws:secretsmanager:us-east-1:123:secret:tenant-{{.tenant_id}}-creds", + }, + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + // Simulate trigger data containing the tenant ID. + pc := NewPipelineContext(map[string]any{"tenant_id": "acme"}, nil) + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("execute error: %v", err) + } + + if result.Output["token"] != "acme-token" { + t.Errorf("expected token=acme-token, got %v", result.Output["token"]) + } +} + +// TestSecretFetchStep_TenantAwareFromStepOutput verifies resolution from a +// previous step's output (the most common tenant-aware use case in pipelines). +func TestSecretFetchStep_TenantAwareFromStepOutput(t *testing.T) { + provider := newMockSecretProvider(map[string]string{ + "arn:aws:secretsmanager:us-east-1:123:secret:salesforce-client-secret": "sf-secret-xyz", + }) + app := mockAppWithSecrets("aws-secrets", provider) + + factory := NewSecretFetchStepFactory() + step, err := factory("fetch-creds", map[string]any{ + "module": "aws-secrets", + "secrets": map[string]any{ + "client_secret": "{{.steps.lookup_integration.row.client_secret_arn}}", + }, + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + // Simulate a previous step that returned a client_secret_arn. + pc := NewPipelineContext(nil, nil) + pc.StepOutputs["lookup_integration"] = map[string]any{ + "row": map[string]any{ + "client_secret_arn": "arn:aws:secretsmanager:us-east-1:123:secret:salesforce-client-secret", + }, + } + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("execute error: %v", err) + } + + if result.Output["client_secret"] != "sf-secret-xyz" { + t.Errorf("expected client_secret=sf-secret-xyz, got %v", result.Output["client_secret"]) + } +} + +func TestSecretFetchStep_ProviderError(t *testing.T) { + provider := newMockSecretProvider(nil) + provider.getErr = errors.New("access denied") + app := mockAppWithSecrets("aws-secrets", provider) + + factory := NewSecretFetchStepFactory() + step, err := factory("fetch-creds", map[string]any{ + "module": "aws-secrets", + "secrets": map[string]any{ + "token": "arn:secret:token", + }, + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + pc := NewPipelineContext(nil, nil) + _, err = step.Execute(context.Background(), pc) + if err == nil { + t.Fatal("expected error from provider.Get") + } +} + +func TestSecretFetchStep_ModuleNotFound(t *testing.T) { + app := NewMockApplication() + + factory := NewSecretFetchStepFactory() + step, err := factory("fetch-creds", map[string]any{ + "module": "nonexistent-secrets", + "secrets": map[string]any{ + "token": "arn:secret:token", + }, + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + pc := NewPipelineContext(nil, nil) + _, err = step.Execute(context.Background(), pc) + if err == nil { + t.Fatal("expected error when module not found") + } +} + +func TestSecretFetchStep_WrongServiceType(t *testing.T) { + app := NewMockApplication() + app.Services["aws-secrets"] = "not-a-secret-provider" // wrong type + + factory := NewSecretFetchStepFactory() + step, err := factory("fetch-creds", map[string]any{ + "module": "aws-secrets", + "secrets": map[string]any{ + "token": "arn:secret:token", + }, + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + pc := NewPipelineContext(nil, nil) + _, err = step.Execute(context.Background(), pc) + if err == nil { + t.Fatal("expected error for wrong service type") + } +} + +func TestSecretFetchStep_NoAppContext(t *testing.T) { + factory := NewSecretFetchStepFactory() + step, err := factory("fetch-creds", map[string]any{ + "module": "aws-secrets", + "secrets": map[string]any{ + "token": "arn:secret:token", + }, + }, nil) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + // Cast to concrete type to access internal state. + concreteStep := step.(*SecretFetchStep) + concreteStep.app = nil // force nil + + pc := NewPipelineContext(nil, nil) + _, err = step.Execute(context.Background(), pc) + if err == nil { + t.Fatal("expected error when app is nil") + } +} diff --git a/plugins/all/all.go b/plugins/all/all.go index 9056a6d9..408cf5a2 100644 --- a/plugins/all/all.go +++ b/plugins/all/all.go @@ -46,9 +46,9 @@ import ( pluginpipeline "github.com/GoCodeAlone/workflow/plugins/pipelinesteps" pluginplatform "github.com/GoCodeAlone/workflow/plugins/platform" pluginpolicy "github.com/GoCodeAlone/workflow/plugins/policy" + pluginscanner "github.com/GoCodeAlone/workflow/plugins/scanner" pluginscheduler "github.com/GoCodeAlone/workflow/plugins/scheduler" pluginsecrets "github.com/GoCodeAlone/workflow/plugins/secrets" - pluginscanner "github.com/GoCodeAlone/workflow/plugins/scanner" pluginsm "github.com/GoCodeAlone/workflow/plugins/statemachine" pluginstorage "github.com/GoCodeAlone/workflow/plugins/storage" plugintimeline "github.com/GoCodeAlone/workflow/plugins/timeline" diff --git a/plugins/pipelinesteps/plugin.go b/plugins/pipelinesteps/plugin.go index ad40e47b..8893ac93 100644 --- a/plugins/pipelinesteps/plugin.go +++ b/plugins/pipelinesteps/plugin.go @@ -102,6 +102,7 @@ func New() *Plugin { "step.parallel", "step.graphql", "step.event_decrypt", + "step.secret_fetch", }, WorkflowTypes: []string{"pipeline"}, OverridableTypes: []string{"step.authz_check"}, @@ -188,6 +189,7 @@ func (p *Plugin) StepFactories() map[string]plugin.StepFactory { })), "step.graphql": wrapStepFactory(module.NewGraphQLStepFactory()), "step.event_decrypt": wrapStepFactory(module.NewEventDecryptStepFactory()), + "step.secret_fetch": wrapStepFactory(module.NewSecretFetchStepFactory()), } } diff --git a/plugins/pipelinesteps/plugin_test.go b/plugins/pipelinesteps/plugin_test.go index 87d5204a..3e197b47 100644 --- a/plugins/pipelinesteps/plugin_test.go +++ b/plugins/pipelinesteps/plugin_test.go @@ -80,6 +80,7 @@ func TestStepFactories(t *testing.T) { "step.cli_invoke", "step.parallel", "step.graphql", + "step.secret_fetch", } for _, stepType := range expectedSteps { From dad69d6ca2bd9522a5bd9a1f0f3cd46c3cbaa960 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Mar 2026 20:02:57 +0000 Subject: [PATCH 3/4] fix: address review feedback on step.secret_fetch validation - Reject "fetched" as a secrets-map key (reserved for output status) - Validate non-empty/non-whitespace secret IDs at factory time - Guard against empty resolved IDs after template evaluation - Add tests for all three new validation paths Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- module/pipeline_step_secret_fetch.go | 10 ++++ module/pipeline_step_secret_fetch_test.go | 66 +++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/module/pipeline_step_secret_fetch.go b/module/pipeline_step_secret_fetch.go index afdd463b..7ac1f3cd 100644 --- a/module/pipeline_step_secret_fetch.go +++ b/module/pipeline_step_secret_fetch.go @@ -3,6 +3,7 @@ package module import ( "context" "fmt" + "strings" "github.com/GoCodeAlone/modular" ) @@ -46,10 +47,16 @@ func NewSecretFetchStepFactory() StepFactory { secretMap := make(map[string]string, len(raw)) for k, v := range raw { + if k == "fetched" { + return nil, fmt.Errorf("secret_fetch step %q: secrets key %q is reserved for step output status", name, k) + } idStr, ok := v.(string) if !ok { return nil, fmt.Errorf("secret_fetch step %q: secrets[%q] must be a string (secret ID or ARN)", name, k) } + if strings.TrimSpace(idStr) == "" { + return nil, fmt.Errorf("secret_fetch step %q: secrets[%q] must not be empty", name, k) + } secretMap[k] = idStr } @@ -89,6 +96,9 @@ func (s *SecretFetchStep) Execute(ctx context.Context, pc *PipelineContext) (*St if resolveErr != nil { return nil, fmt.Errorf("secret_fetch step %q: failed to resolve secret ID for %q: %w", s.name, outputKey, resolveErr) } + if strings.TrimSpace(resolvedID) == "" { + return nil, fmt.Errorf("secret_fetch step %q: resolved secret ID for %q is empty (check template expression %q)", s.name, outputKey, idTemplate) + } value, fetchErr := provider.Get(ctx, resolvedID) if fetchErr != nil { diff --git a/module/pipeline_step_secret_fetch_test.go b/module/pipeline_step_secret_fetch_test.go index 3238fc5f..418c1893 100644 --- a/module/pipeline_step_secret_fetch_test.go +++ b/module/pipeline_step_secret_fetch_test.go @@ -80,6 +80,45 @@ func TestSecretFetchStep_NonStringSecretID(t *testing.T) { } } +func TestSecretFetchStep_ReservedFetchedKey(t *testing.T) { + factory := NewSecretFetchStepFactory() + _, err := factory("bad", map[string]any{ + "module": "aws-secrets", + "secrets": map[string]any{ + "fetched": "arn:secret:something", // reserved key + }, + }, nil) + if err == nil { + t.Fatal("expected error when secrets key 'fetched' is used (reserved for output status)") + } +} + +func TestSecretFetchStep_EmptySecretID(t *testing.T) { + factory := NewSecretFetchStepFactory() + _, err := factory("bad", map[string]any{ + "module": "aws-secrets", + "secrets": map[string]any{ + "token": "", // empty string + }, + }, nil) + if err == nil { + t.Fatal("expected error when secret ID is an empty string") + } +} + +func TestSecretFetchStep_WhitespaceOnlySecretID(t *testing.T) { + factory := NewSecretFetchStepFactory() + _, err := factory("bad", map[string]any{ + "module": "aws-secrets", + "secrets": map[string]any{ + "token": " ", // whitespace only + }, + }, nil) + if err == nil { + t.Fatal("expected error when secret ID is whitespace-only") + } +} + // --- execute tests --- func TestSecretFetchStep_FetchSingle(t *testing.T) { @@ -246,6 +285,33 @@ func TestSecretFetchStep_ProviderError(t *testing.T) { } } +// TestSecretFetchStep_EmptyResolvedID verifies that a template that resolves to +// an empty string at execute time causes a fast-fail error rather than calling +// provider.Get with an empty key. +func TestSecretFetchStep_EmptyResolvedID(t *testing.T) { + provider := newMockSecretProvider(map[string]string{}) + app := mockAppWithSecrets("aws-secrets", provider) + + factory := NewSecretFetchStepFactory() + step, err := factory("fetch-creds", map[string]any{ + "module": "aws-secrets", + "secrets": map[string]any{ + // Template resolves to empty when tenant_arn is absent from context. + "token": "{{.tenant_arn}}", + }, + }, app) + if err != nil { + t.Fatalf("factory error: %v", err) + } + + // Pipeline context missing the tenant_arn key → resolves to empty string. + pc := NewPipelineContext(nil, nil) + _, err = step.Execute(context.Background(), pc) + if err == nil { + t.Fatal("expected error when resolved secret ID is empty") + } +} + func TestSecretFetchStep_ModuleNotFound(t *testing.T) { app := NewMockApplication() From 068cb3946f44738213a5b9e96e9d6cd22ae54b9c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Mar 2026 21:35:02 +0000 Subject: [PATCH 4/4] fix: add step.secret_fetch to KnownStepTypes in type_registry.go TestKnownStepTypesCoverAllPlugins was failing because step.secret_fetch was registered in the pipelinesteps plugin but missing from the KnownStepTypes() registry used by wfctl. Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- cmd/wfctl/type_registry.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/wfctl/type_registry.go b/cmd/wfctl/type_registry.go index e3e8786f..4472c516 100644 --- a/cmd/wfctl/type_registry.go +++ b/cmd/wfctl/type_registry.go @@ -1428,6 +1428,11 @@ func KnownStepTypes() map[string]StepTypeInfo { Plugin: "secrets", ConfigKeys: []string{"provider", "key", "notify_module"}, }, + "step.secret_fetch": { + Type: "step.secret_fetch", + Plugin: "pipelinesteps", + ConfigKeys: []string{"module", "secrets"}, + }, } // Include any step types registered dynamically (e.g. from external plugins). for _, t := range schema.KnownModuleTypes() {