From 3d02c5c27d6448cd8e9b55fc1e690a88c3f78cb4 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 10 May 2026 02:22:03 -0400 Subject: [PATCH 1/5] fix: support compute workflow gaps --- plugin/external/adapter_test.go | 42 +++++++++++++++ plugin/external/sdk/grpc_server_test.go | 71 +++++++++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/plugin/external/adapter_test.go b/plugin/external/adapter_test.go index c47afa92..b75e3765 100644 --- a/plugin/external/adapter_test.go +++ b/plugin/external/adapter_test.go @@ -63,6 +63,10 @@ func (c *adapterTestPluginServiceClient) GetModuleTypes(_ context.Context, _ *em return &pb.TypeList{Types: c.moduleTypes}, nil } +func (c *adapterTestPluginServiceClient) GetTriggerTypes(_ context.Context, _ *emptypb.Empty, _ ...grpc.CallOption) (*pb.TypeList, error) { + return &pb.TypeList{Types: c.triggerTypes}, nil +} + func (c *adapterTestPluginServiceClient) CreateModule(_ context.Context, req *pb.CreateModuleRequest, _ ...grpc.CallOption) (*pb.HandleResponse, error) { c.lastCreateModReq = req return &pb.HandleResponse{HandleId: "module-handle"}, nil @@ -73,6 +77,10 @@ func (c *adapterTestPluginServiceClient) CreateStep(_ context.Context, req *pb.C return &pb.HandleResponse{HandleId: "step-handle"}, nil } +func (c *adapterTestPluginServiceClient) InitModule(_ context.Context, _ *pb.HandleRequest, _ ...grpc.CallOption) (*pb.ErrorResponse, error) { + return &pb.ErrorResponse{}, nil +} + func TestIsSamplePlugin_True(t *testing.T) { a := newTestAdapter(&pb.Manifest{ Name: "my-sample", @@ -629,6 +637,40 @@ func TestExternalPluginAdapter_MalformedDescriptorSetRecordsError(t *testing.T) } } +func TestExternalPluginAdapter_RemoteTriggerDelaysCreateUntilConfigure(t *testing.T) { + client := &adapterTestPluginServiceClient{ + manifest: &pb.Manifest{Name: "trigger-plugin"}, + triggerTypes: []string{"compute.completed"}, + } + a, err := NewExternalPluginAdapter("trigger-plugin", &PluginClient{client: client}) + if err != nil { + t.Fatalf("NewExternalPluginAdapter: %v", err) + } + factory := a.TriggerFactories()["compute.completed"] + if factory == nil { + t.Fatal("missing trigger factory") + } + trigger, ok := factory().(*RemoteTrigger) + if !ok { + t.Fatalf("factory type = %T, want *RemoteTrigger", factory()) + } + if client.lastCreateModReq != nil { + t.Fatal("trigger factory should not create remote handle before config is available") + } + if err := trigger.Configure(nil, map[string]any{"task_status": "succeeded"}); err != nil { + t.Fatalf("Configure: %v", err) + } + if client.lastCreateModReq == nil { + t.Fatal("Configure did not create remote trigger handle") + } + if client.lastCreateModReq.Type != "compute.completed" { + t.Fatalf("CreateModule type = %q", client.lastCreateModReq.Type) + } + if got := client.lastCreateModReq.Config.AsMap()["task_status"]; got != "succeeded" { + t.Fatalf("trigger config did not reach CreateModule: %#v", client.lastCreateModReq.Config.AsMap()) + } +} + func TestTypedAnyToMapNormalizesIntegerFields(t *testing.T) { const outputMessage = "workflow.plugins.test.v1.DynamicOutput" registry := &pb.ContractRegistry{ diff --git a/plugin/external/sdk/grpc_server_test.go b/plugin/external/sdk/grpc_server_test.go index 6b442569..aade3744 100644 --- a/plugin/external/sdk/grpc_server_test.go +++ b/plugin/external/sdk/grpc_server_test.go @@ -192,6 +192,41 @@ type typedServiceFactoryProvider struct { TypedModuleProvider } +type triggerProviderForTest struct { + minimalProvider + lastType string + lastConfig map[string]any + lastCB TriggerCallback + trigger *triggerInstanceForTest +} + +func (p *triggerProviderForTest) TriggerTypes() []string { + return []string{"compute.completed"} +} + +func (p *triggerProviderForTest) CreateTrigger(typeName string, config map[string]any, cb TriggerCallback) (TriggerInstance, error) { + p.lastType = typeName + p.lastConfig = config + p.lastCB = cb + p.trigger = &triggerInstanceForTest{} + return p.trigger, nil +} + +type triggerInstanceForTest struct { + started bool + stopped bool +} + +func (t *triggerInstanceForTest) Start(context.Context) error { + t.started = true + return nil +} + +func (t *triggerInstanceForTest) Stop(context.Context) error { + t.stopped = true + return nil +} + type typedServiceModule struct { lastMethod string lastInput *anypb.Any @@ -581,6 +616,42 @@ func TestInvokeService_WithTypedInvoker(t *testing.T) { } } +func TestCreateModule_DispatchesTriggerProviderTypes(t *testing.T) { + provider := &triggerProviderForTest{} + srv := newGRPCServer(provider) + + createResp, err := srv.CreateModule(context.Background(), &pb.CreateModuleRequest{ + Type: "compute.completed", + Name: "compute.completed", + Config: mustMapToStruct(t, map[string]any{"task_status": "succeeded"}), + }) + if err != nil { + t.Fatalf("CreateModule returned rpc error: %v", err) + } + if createResp.Error != "" { + t.Fatalf("unexpected create error: %s", createResp.Error) + } + if provider.lastType != "compute.completed" { + t.Fatalf("CreateTrigger type = %q", provider.lastType) + } + if provider.lastConfig["task_status"] != "succeeded" { + t.Fatalf("CreateTrigger config = %#v", provider.lastConfig) + } + + if _, err := srv.StartModule(context.Background(), &pb.HandleRequest{HandleId: createResp.HandleId}); err != nil { + t.Fatalf("StartModule: %v", err) + } + if provider.trigger == nil || !provider.trigger.started { + t.Fatal("trigger instance was not started through module lifecycle") + } + if _, err := srv.StopModule(context.Background(), &pb.HandleRequest{HandleId: createResp.HandleId}); err != nil { + t.Fatalf("StopModule: %v", err) + } + if !provider.trigger.stopped { + t.Fatal("trigger instance was not stopped through module lifecycle") + } +} + func TestInvokeService_WithTypedModuleFactoryForwardsTypedInvoker(t *testing.T) { module := &typedServiceModule{} srv := newGRPCServer(&typedServiceFactoryProvider{ From 294696f7fae0f0916b188c3e5ea55aad27a3d76d Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 10 May 2026 14:07:01 -0400 Subject: [PATCH 2/5] fix(iac): sanitize routed secret keys --- iac/sensitive/route.go | 46 +++++++++++++++++++-- iac/sensitive/route_test.go | 81 ++++++++++++++++++++++++------------- 2 files changed, 96 insertions(+), 31 deletions(-) diff --git a/iac/sensitive/route.go b/iac/sensitive/route.go index 7d5024f5..806661c4 100644 --- a/iac/sensitive/route.go +++ b/iac/sensitive/route.go @@ -20,6 +20,8 @@ package sensitive import ( "context" + "crypto/sha256" + "encoding/hex" "errors" "fmt" "sort" @@ -36,10 +38,16 @@ import ( const PlaceholderPrefix = "secret_ref://" // SecretKey returns the canonical secrets.Provider key for a resource's -// output: "_". Exported so audit-state-secrets -// and other consumers can reverse-engineer routed-secret names. +// output. The key is provider-safe and collision-resistant for distinct +// (resourceName, outputKey) pairs. Exported so audit-state-secrets and other +// consumers can reverse-engineer routed-secret names. func SecretKey(resourceName, outputKey string) string { - return resourceName + "_" + outputKey + raw := resourceName + "\x00" + outputKey + key := sanitizeSecretKeyPart(resourceName) + "__" + sanitizeSecretKeyPart(outputKey) + "_" + shortHash(raw) + if strings.HasPrefix(strings.ToUpper(key), "GITHUB_") { + key = "WF_" + key + } + return key } // Placeholder returns the canonical "secret_ref://_" @@ -154,6 +162,38 @@ func stringifyOutput(v any) (string, error) { } } +func sanitizeSecretKeyPart(part string) string { + var b strings.Builder + b.Grow(len(part)) + for _, r := range part { + switch { + case r >= 'A' && r <= 'Z': + b.WriteRune(r) + case r >= 'a' && r <= 'z': + b.WriteRune(r) + case r >= '0' && r <= '9': + b.WriteRune(r) + case r == '_': + b.WriteRune(r) + default: + b.WriteByte('_') + } + } + out := b.String() + if out == "" { + out = "SECRET" + } + if out[0] >= '0' && out[0] <= '9' { + out = "_" + out + } + return out +} + +func shortHash(value string) string { + sum := sha256.Sum256([]byte(value)) + return strings.ToUpper(hex.EncodeToString(sum[:4])) +} + // Revoke deletes routed secrets for resourceName. mergedKeys is the union // of placeholder-derived keys (caller extracts from pre-delete // state.Outputs) and any legacy heuristic keys. Errors from diff --git a/iac/sensitive/route_test.go b/iac/sensitive/route_test.go index 5c7ca09b..55809b33 100644 --- a/iac/sensitive/route_test.go +++ b/iac/sensitive/route_test.go @@ -3,6 +3,7 @@ package sensitive import ( "context" "errors" + "regexp" "strings" "testing" @@ -86,22 +87,24 @@ func TestRoute_SensitiveValuePresent_RoutesAndSanitizes(t *testing.T) { if err != nil { t.Fatalf("Route: %v", err) } - if p.values["myres_secret_key"] != "SECRET" { + secretKey := SecretKey("myres", "secret_key") + accessKey := SecretKey("myres", "access_key") + if p.values[secretKey] != "SECRET" { t.Errorf("provider did not receive secret_key; got %v", p.values) } - if p.values["myres_access_key"] != "AK" { + if p.values[accessKey] != "AK" { t.Errorf("provider did not receive access_key; got %v", p.values) } - if sanitized["secret_key"] != "secret_ref://myres_secret_key" { + if sanitized["secret_key"] != Placeholder("myres", "secret_key") { t.Errorf("sanitized[secret_key] = %v, want placeholder", sanitized["secret_key"]) } - if sanitized["access_key"] != "secret_ref://myres_access_key" { + if sanitized["access_key"] != Placeholder("myres", "access_key") { t.Errorf("sanitized[access_key] = %v, want placeholder", sanitized["access_key"]) } if sanitized["bucket"] != "b" { t.Errorf("non-sensitive bucket lost: %v", sanitized["bucket"]) } - if hydrated["myres_secret_key"] != "SECRET" { + if hydrated[secretKey] != "SECRET" { t.Errorf("hydrated missing secret_key: %v", hydrated) } } @@ -119,14 +122,14 @@ func TestRoute_SensitiveKeyAbsentFromOutputs_Skipped(t *testing.T) { if _, ok := sanitized["secret_key"]; ok { t.Errorf("absent sensitive key should NOT yield placeholder, got %v", sanitized["secret_key"]) } - if _, ok := p.values["myres_secret_key"]; ok { + if _, ok := p.values[SecretKey("myres", "secret_key")]; ok { t.Errorf("provider should not have received secret_key (absent value)") } - if hydrated["myres_secret_key"] != "" { + if hydrated[SecretKey("myres", "secret_key")] != "" { t.Errorf("hydrated should not contain absent key") } // access_key was present, should be routed - if sanitized["access_key"] != "secret_ref://myres_access_key" { + if sanitized["access_key"] != Placeholder("myres", "access_key") { t.Errorf("access_key routing failed: %v", sanitized["access_key"]) } } @@ -151,7 +154,7 @@ func TestRoute_SensitiveFalseValue_NotRouted(t *testing.T) { func TestRoute_ProviderSetError_ReturnsError(t *testing.T) { p := newFakeProvider() - p.setErr["myres_secret_key"] = errors.New("boom") + p.setErr[SecretKey("myres", "secret_key")] = errors.New("boom") out := &interfaces.ResourceOutput{ Outputs: map[string]any{"secret_key": "S"}, Sensitive: map[string]bool{"secret_key": true}, @@ -215,7 +218,7 @@ func TestRoute_DeterministicSetOrder(t *testing.T) { if err != nil { t.Fatalf("Route: %v", err) } - want := []string{"r_a_key", "r_b_key", "r_c_key"} + want := []string{SecretKey("r", "a_key"), SecretKey("r", "b_key"), SecretKey("r", "c_key")} if len(p.setLog) != len(want) { t.Fatalf("setLog len: got %v want %v", p.setLog, want) } @@ -250,25 +253,25 @@ func TestRoute_NilOut_Errors(t *testing.T) { func TestRevoke_DeletesAllKeys(t *testing.T) { p := newFakeProvider() - p.values["r_secret_key"] = "S" - p.values["r_access_key"] = "A" + p.values[SecretKey("r", "secret_key")] = "S" + p.values[SecretKey("r", "access_key")] = "A" if err := Revoke(context.Background(), p, "r", []string{"secret_key", "access_key"}); err != nil { t.Fatalf("Revoke: %v", err) } - if _, ok := p.values["r_secret_key"]; ok { + if _, ok := p.values[SecretKey("r", "secret_key")]; ok { t.Errorf("secret_key not deleted") } - if _, ok := p.values["r_access_key"]; ok { + if _, ok := p.values[SecretKey("r", "access_key")]; ok { t.Errorf("access_key not deleted") } } func TestRevoke_AggregatesErrors(t *testing.T) { p := newFakeProvider() - p.values["r_secret_key"] = "S" - p.values["r_access_key"] = "A" - p.delErr["r_secret_key"] = errors.New("boom1") - p.delErr["r_access_key"] = errors.New("boom2") + p.values[SecretKey("r", "secret_key")] = "S" + p.values[SecretKey("r", "access_key")] = "A" + p.delErr[SecretKey("r", "secret_key")] = errors.New("boom1") + p.delErr[SecretKey("r", "access_key")] = errors.New("boom2") err := Revoke(context.Background(), p, "r", []string{"secret_key", "access_key"}) if err == nil { t.Fatal("expected aggregated error") @@ -282,11 +285,11 @@ func TestRevoke_AggregatesErrors(t *testing.T) { func TestRevoke_ContinuesAfterError(t *testing.T) { // First key errors; second key should still be Deleted. p := newFakeProvider() - p.values["r_secret_key"] = "S" - p.values["r_access_key"] = "A" - p.delErr["r_secret_key"] = errors.New("boom") + p.values[SecretKey("r", "secret_key")] = "S" + p.values[SecretKey("r", "access_key")] = "A" + p.delErr[SecretKey("r", "secret_key")] = errors.New("boom") _ = Revoke(context.Background(), p, "r", []string{"secret_key", "access_key"}) - if _, ok := p.values["r_access_key"]; ok { + if _, ok := p.values[SecretKey("r", "access_key")]; ok { t.Error("access_key not deleted (Revoke should continue past first error)") } } @@ -294,8 +297,8 @@ func TestRevoke_ContinuesAfterError(t *testing.T) { func TestRevoke_NotFoundIsSuccess(t *testing.T) { p := newFakeProvider() // Pre-populate one key; Delete on a missing one returns ErrNotFound. - p.values["r_secret_key"] = "S" - p.delErr["r_access_key"] = secrets.ErrNotFound + p.values[SecretKey("r", "secret_key")] = "S" + p.delErr[SecretKey("r", "access_key")] = secrets.ErrNotFound if err := Revoke(context.Background(), p, "r", []string{"secret_key", "access_key"}); err != nil { t.Fatalf("Revoke should swallow ErrNotFound, got %v", err) } @@ -338,7 +341,7 @@ func TestIsPlaceholder(t *testing.T) { func TestPlaceholder(t *testing.T) { got := Placeholder("myres", "secret_key") - want := "secret_ref://myres_secret_key" + want := PlaceholderPrefix + SecretKey("myres", "secret_key") if got != want { t.Errorf("Placeholder = %q, want %q", got, want) } @@ -346,9 +349,31 @@ func TestPlaceholder(t *testing.T) { func TestSecretKey(t *testing.T) { got := SecretKey("myres", "secret_key") - want := "myres_secret_key" - if got != want { - t.Errorf("SecretKey = %q, want %q", got, want) + if ok, _ := regexp.MatchString(`^[A-Za-z_][A-Za-z0-9_]*$`, got); !ok { + t.Fatalf("SecretKey = %q, want provider-safe name", got) + } + if !strings.HasPrefix(got, "myres__secret_key_") { + t.Errorf("SecretKey = %q, want sanitized parts plus hash suffix", got) + } + for _, pair := range [][2]string{ + {"a", "b_c"}, + {"a_b", "c"}, + {"a-b", "c"}, + {"a_b", "c"}, + {"github", "token"}, + } { + if key := SecretKey(pair[0], pair[1]); !regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`).MatchString(key) { + t.Fatalf("SecretKey(%q,%q) = %q, want provider-safe name", pair[0], pair[1], key) + } + } + if SecretKey("a", "b_c") == SecretKey("a_b", "c") { + t.Fatal("SecretKey must not collide for underscore-ambiguous pairs") + } + if SecretKey("a-b", "c") == SecretKey("a_b", "c") { + t.Fatal("SecretKey must not collide for sanitized-equivalent resource names") + } + if key := SecretKey("github", "token"); regexp.MustCompile(`(?i)^github_`).MatchString(key) { + t.Fatalf("SecretKey = %q, must not use GitHub-reserved prefix", key) } } From 39c27f8f916827f66ed6249335a5914669d0b30d Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 10 May 2026 16:09:24 -0400 Subject: [PATCH 3/5] fix: resolve PR 621 feedback --- .../infra_apply_sensitive_routing_test.go | 22 ++++--- iac/sensitive/route.go | 64 +++++++++++++++---- iac/sensitive/route_test.go | 18 +++++- plugin/external/adapter_test.go | 26 ++++---- plugin/external/sdk/grpc_server_test.go | 6 +- 5 files changed, 97 insertions(+), 39 deletions(-) diff --git a/cmd/wfctl/infra_apply_sensitive_routing_test.go b/cmd/wfctl/infra_apply_sensitive_routing_test.go index ef3ab6d1..e4bc833e 100644 --- a/cmd/wfctl/infra_apply_sensitive_routing_test.go +++ b/cmd/wfctl/infra_apply_sensitive_routing_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/GoCodeAlone/workflow/iac/sensitive" "github.com/GoCodeAlone/workflow/interfaces" "github.com/GoCodeAlone/workflow/secrets" ) @@ -121,21 +122,26 @@ func TestPersistResourceWithSecretRouting_RoutesSensitiveAndSanitizesState(t *te t.Fatalf("expected 1 saved, got %d", len(store.saved)) } state := store.saved[0] - if state.Outputs["secret_key"] != "secret_ref://myres_secret_key" { + secretKey := sensitive.SecretKey("myres", "secret_key") + accessKey := sensitive.SecretKey("myres", "access_key") + if state.Outputs["secret_key"] != sensitive.Placeholder("myres", "secret_key") { t.Errorf("state secret_key not sanitized: %v", state.Outputs["secret_key"]) } - if state.Outputs["access_key"] != "secret_ref://myres_access_key" { + if state.Outputs["access_key"] != sensitive.Placeholder("myres", "access_key") { t.Errorf("state access_key not sanitized: %v", state.Outputs["access_key"]) } if state.Outputs["bucket"] != "b" { t.Errorf("state bucket lost: %v", state.Outputs["bucket"]) } - if prov.values["myres_secret_key"] != "SK" { + if prov.values[secretKey] != "SK" { t.Errorf("provider missing secret_key value") } - if hydrated["myres_secret_key"] != "SK" { + if hydrated[secretKey] != "SK" { t.Errorf("hydrated missing secret_key: %v", hydrated) } + if prov.values[accessKey] != "AK" { + t.Errorf("provider missing access_key value") + } } func TestPersistResourceWithSecretRouting_NoProviderHardFails(t *testing.T) { @@ -181,7 +187,7 @@ func TestPersistResourceWithSecretRouting_SaveFailureCompensatesWithDelete(t *te if drv.deleteCalls[0].ProviderID != "AKIA" { t.Errorf("compensating Delete used wrong ProviderID: %v", drv.deleteCalls[0]) } - if _, ok := prov.values["myres_secret_key"]; ok { + if _, ok := prov.values[sensitive.SecretKey("myres", "secret_key")]; ok { t.Errorf("compensating Delete should have removed routed secret; got %v", prov.values) } } @@ -223,7 +229,7 @@ func TestPersistResourceWithSecretRouting_Idempotent(t *testing.T) { t.Fatalf("persist iter %d: %v", i, err) } } - if prov.values["myres_secret_key"] != "SK" { + if prov.values[sensitive.SecretKey("myres", "secret_key")] != "SK" { t.Errorf("provider value lost on re-Apply: %v", prov.values) } if len(store.saved) != 2 { @@ -264,7 +270,7 @@ func TestPersistResourceWithSecretRouting_ReadModeSanitizeOnly_PreservesPriorPla // Pre-existing state has a placeholder store := &stubInfraStore{ saved: []interfaces.ResourceState{ - {Name: "myres", Outputs: map[string]any{"secret_key": "secret_ref://myres_secret_key", "bucket": "b"}}, + {Name: "myres", Outputs: map[string]any{"secret_key": sensitive.Placeholder("myres", "secret_key"), "bucket": "b"}}, }, } out := interfaces.ResourceOutput{ @@ -283,7 +289,7 @@ func TestPersistResourceWithSecretRouting_ReadModeSanitizeOnly_PreservesPriorPla t.Fatalf("expected 2 saves (initial + this), got %d", len(store.saved)) } latest := store.saved[1] - if latest.Outputs["secret_key"] != "secret_ref://myres_secret_key" { + if latest.Outputs["secret_key"] != sensitive.Placeholder("myres", "secret_key") { t.Errorf("Read mode lost prior placeholder: %v", latest.Outputs["secret_key"]) } if latest.Outputs["bucket"] != "b" { diff --git a/iac/sensitive/route.go b/iac/sensitive/route.go index 806661c4..4cbf760b 100644 --- a/iac/sensitive/route.go +++ b/iac/sensitive/route.go @@ -31,27 +31,40 @@ import ( "github.com/GoCodeAlone/workflow/secrets" ) -// PlaceholderPrefix is the URI scheme used in state.Outputs values to -// reference a routed secret stored in the configured secrets.Provider. -// Distinct from secrets.SecretPrefix ("secret://") which is for -// user-supplied config references. -const PlaceholderPrefix = "secret_ref://" +const ( + // PlaceholderPrefix is the URI scheme used in state.Outputs values to + // reference a routed secret stored in the configured secrets.Provider. + // Distinct from secrets.SecretPrefix ("secret://") which is for + // user-supplied config references. + PlaceholderPrefix = "secret_ref://" + + // secretKeyMaxLength follows the lowest common provider limit currently + // targeted by routed output secrets: GitHub Actions secret names. + secretKeyMaxLength = 100 + secretKeyHashBytes = 16 +) // SecretKey returns the canonical secrets.Provider key for a resource's // output. The key is provider-safe and collision-resistant for distinct // (resourceName, outputKey) pairs. Exported so audit-state-secrets and other -// consumers can reverse-engineer routed-secret names. +// consumers can recompute routed-secret names from known resource/output pairs. func SecretKey(resourceName, outputKey string) string { raw := resourceName + "\x00" + outputKey - key := sanitizeSecretKeyPart(resourceName) + "__" + sanitizeSecretKeyPart(outputKey) + "_" + shortHash(raw) - if strings.HasPrefix(strings.ToUpper(key), "GITHUB_") { - key = "WF_" + key + resourcePart := sanitizeSecretKeyPart(resourceName) + outputPart := sanitizeSecretKeyPart(outputKey) + hash := shortHash(raw) + + prefix := "" + if strings.HasPrefix(strings.ToUpper(resourcePart+"__"+outputPart), "GITHUB_") { + prefix = "WF_" } - return key + maxPartsLength := secretKeyMaxLength - len(prefix) - len(hash) - len("__") - len("_") + resourcePart, outputPart = truncateSecretKeyParts(resourcePart, outputPart, maxPartsLength) + return prefix + resourcePart + "__" + outputPart + "_" + hash } -// Placeholder returns the canonical "secret_ref://_" -// string that replaces a routed value in state.Outputs. +// Placeholder returns PlaceholderPrefix + SecretKey(resourceName, outputKey), +// replacing a routed value in state.Outputs. func Placeholder(resourceName, outputKey string) string { return PlaceholderPrefix + SecretKey(resourceName, outputKey) } @@ -67,7 +80,7 @@ func IsPlaceholder(v any) bool { } // Route routes sensitive fields from out through provider, keying each -// secret as "_". Returns: +// secret via SecretKey(resourceName, outputKey). Returns: // // - sanitized: a copy of out.Outputs with sensitive values replaced by // PlaceholderPrefix + SecretKey(resourceName, k). Suitable for @@ -191,7 +204,30 @@ func sanitizeSecretKeyPart(part string) string { func shortHash(value string) string { sum := sha256.Sum256([]byte(value)) - return strings.ToUpper(hex.EncodeToString(sum[:4])) + return strings.ToUpper(hex.EncodeToString(sum[:secretKeyHashBytes])) +} + +func truncateSecretKeyParts(resourcePart, outputPart string, maxLength int) (string, string) { + if len(resourcePart)+len(outputPart) <= maxLength { + return resourcePart, outputPart + } + resourceBudget := maxLength / 2 + outputBudget := maxLength - resourceBudget + if len(resourcePart) < resourceBudget { + outputBudget += resourceBudget - len(resourcePart) + resourceBudget = len(resourcePart) + } + if len(outputPart) < outputBudget { + resourceBudget += outputBudget - len(outputPart) + outputBudget = len(outputPart) + } + if len(resourcePart) > resourceBudget { + resourcePart = resourcePart[:resourceBudget] + } + if len(outputPart) > outputBudget { + outputPart = outputPart[:outputBudget] + } + return resourcePart, outputPart } // Revoke deletes routed secrets for resourceName. mergedKeys is the union diff --git a/iac/sensitive/route_test.go b/iac/sensitive/route_test.go index 55809b33..dbdae75b 100644 --- a/iac/sensitive/route_test.go +++ b/iac/sensitive/route_test.go @@ -352,6 +352,9 @@ func TestSecretKey(t *testing.T) { if ok, _ := regexp.MatchString(`^[A-Za-z_][A-Za-z0-9_]*$`, got); !ok { t.Fatalf("SecretKey = %q, want provider-safe name", got) } + if len(got) > secretKeyMaxLength { + t.Fatalf("SecretKey len = %d, want <= %d", len(got), secretKeyMaxLength) + } if !strings.HasPrefix(got, "myres__secret_key_") { t.Errorf("SecretKey = %q, want sanitized parts plus hash suffix", got) } @@ -359,7 +362,7 @@ func TestSecretKey(t *testing.T) { {"a", "b_c"}, {"a_b", "c"}, {"a-b", "c"}, - {"a_b", "c"}, + {"9resource", "secret-key"}, {"github", "token"}, } { if key := SecretKey(pair[0], pair[1]); !regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`).MatchString(key) { @@ -375,6 +378,19 @@ func TestSecretKey(t *testing.T) { if key := SecretKey("github", "token"); regexp.MustCompile(`(?i)^github_`).MatchString(key) { t.Fatalf("SecretKey = %q, must not use GitHub-reserved prefix", key) } + longA := SecretKey(strings.Repeat("resource", 20), strings.Repeat("output", 20)) + longB := SecretKey(strings.Repeat("resource", 20)+"x", strings.Repeat("output", 20)) + for _, key := range []string{longA, longB} { + if len(key) > secretKeyMaxLength { + t.Fatalf("long SecretKey len = %d, want <= %d: %q", len(key), secretKeyMaxLength, key) + } + if ok, _ := regexp.MatchString(`^[A-Za-z_][A-Za-z0-9_]*$`, key); !ok { + t.Fatalf("long SecretKey = %q, want provider-safe name", key) + } + } + if longA == longB { + t.Fatal("truncated SecretKey values must retain hash collision resistance") + } } func TestMaskSensitiveForDiff_MasksPlaceholdersAndDriverKeys(t *testing.T) { diff --git a/plugin/external/adapter_test.go b/plugin/external/adapter_test.go index b75e3765..6358b153 100644 --- a/plugin/external/adapter_test.go +++ b/plugin/external/adapter_test.go @@ -63,10 +63,6 @@ func (c *adapterTestPluginServiceClient) GetModuleTypes(_ context.Context, _ *em return &pb.TypeList{Types: c.moduleTypes}, nil } -func (c *adapterTestPluginServiceClient) GetTriggerTypes(_ context.Context, _ *emptypb.Empty, _ ...grpc.CallOption) (*pb.TypeList, error) { - return &pb.TypeList{Types: c.triggerTypes}, nil -} - func (c *adapterTestPluginServiceClient) CreateModule(_ context.Context, req *pb.CreateModuleRequest, _ ...grpc.CallOption) (*pb.HandleResponse, error) { c.lastCreateModReq = req return &pb.HandleResponse{HandleId: "module-handle"}, nil @@ -642,7 +638,10 @@ func TestExternalPluginAdapter_RemoteTriggerDelaysCreateUntilConfigure(t *testin manifest: &pb.Manifest{Name: "trigger-plugin"}, triggerTypes: []string{"compute.completed"}, } - a, err := NewExternalPluginAdapter("trigger-plugin", &PluginClient{client: client}) + a, err := NewExternalPluginAdapter("trigger-plugin", &PluginClient{ + client: client, + callbackBrokerID: 42, + }) if err != nil { t.Fatalf("NewExternalPluginAdapter: %v", err) } @@ -650,24 +649,25 @@ func TestExternalPluginAdapter_RemoteTriggerDelaysCreateUntilConfigure(t *testin if factory == nil { t.Fatal("missing trigger factory") } - trigger, ok := factory().(*RemoteTrigger) + instance := factory() + trigger, ok := instance.(*RemoteTrigger) if !ok { - t.Fatalf("factory type = %T, want *RemoteTrigger", factory()) + t.Fatalf("factory type = %T, want *RemoteTrigger", instance) } - if client.lastCreateModReq != nil { + if client.lastCreateTriggerReq != nil { t.Fatal("trigger factory should not create remote handle before config is available") } if err := trigger.Configure(nil, map[string]any{"task_status": "succeeded"}); err != nil { t.Fatalf("Configure: %v", err) } - if client.lastCreateModReq == nil { + if client.lastCreateTriggerReq == nil { t.Fatal("Configure did not create remote trigger handle") } - if client.lastCreateModReq.Type != "compute.completed" { - t.Fatalf("CreateModule type = %q", client.lastCreateModReq.Type) + if client.lastCreateTriggerReq.Type != "compute.completed" { + t.Fatalf("CreateTrigger type = %q", client.lastCreateTriggerReq.Type) } - if got := client.lastCreateModReq.Config.AsMap()["task_status"]; got != "succeeded" { - t.Fatalf("trigger config did not reach CreateModule: %#v", client.lastCreateModReq.Config.AsMap()) + if got := client.lastCreateTriggerReq.Config.AsMap()["task_status"]; got != "succeeded" { + t.Fatalf("trigger config did not reach CreateTrigger: %#v", client.lastCreateTriggerReq.Config.AsMap()) } } diff --git a/plugin/external/sdk/grpc_server_test.go b/plugin/external/sdk/grpc_server_test.go index aade3744..027f6092 100644 --- a/plugin/external/sdk/grpc_server_test.go +++ b/plugin/external/sdk/grpc_server_test.go @@ -616,17 +616,17 @@ func TestInvokeService_WithTypedInvoker(t *testing.T) { } } -func TestCreateModule_DispatchesTriggerProviderTypes(t *testing.T) { +func TestCreateTrigger_DispatchesTriggerProviderTypes(t *testing.T) { provider := &triggerProviderForTest{} srv := newGRPCServer(provider) - createResp, err := srv.CreateModule(context.Background(), &pb.CreateModuleRequest{ + createResp, err := srv.CreateTrigger(context.Background(), &pb.CreateTriggerRequest{ Type: "compute.completed", Name: "compute.completed", Config: mustMapToStruct(t, map[string]any{"task_status": "succeeded"}), }) if err != nil { - t.Fatalf("CreateModule returned rpc error: %v", err) + t.Fatalf("CreateTrigger returned rpc error: %v", err) } if createResp.Error != "" { t.Fatalf("unexpected create error: %s", createResp.Error) From 4c8fe7c697ac40ccfc46438a583554c0660cdfcf Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 10 May 2026 16:17:35 -0400 Subject: [PATCH 4/5] test: avoid repeated regex compile --- iac/sensitive/route_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/iac/sensitive/route_test.go b/iac/sensitive/route_test.go index dbdae75b..67df403f 100644 --- a/iac/sensitive/route_test.go +++ b/iac/sensitive/route_test.go @@ -349,7 +349,8 @@ func TestPlaceholder(t *testing.T) { func TestSecretKey(t *testing.T) { got := SecretKey("myres", "secret_key") - if ok, _ := regexp.MatchString(`^[A-Za-z_][A-Za-z0-9_]*$`, got); !ok { + providerSafeName := regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) + if !providerSafeName.MatchString(got) { t.Fatalf("SecretKey = %q, want provider-safe name", got) } if len(got) > secretKeyMaxLength { @@ -365,7 +366,7 @@ func TestSecretKey(t *testing.T) { {"9resource", "secret-key"}, {"github", "token"}, } { - if key := SecretKey(pair[0], pair[1]); !regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`).MatchString(key) { + if key := SecretKey(pair[0], pair[1]); !providerSafeName.MatchString(key) { t.Fatalf("SecretKey(%q,%q) = %q, want provider-safe name", pair[0], pair[1], key) } } @@ -384,7 +385,7 @@ func TestSecretKey(t *testing.T) { if len(key) > secretKeyMaxLength { t.Fatalf("long SecretKey len = %d, want <= %d: %q", len(key), secretKeyMaxLength, key) } - if ok, _ := regexp.MatchString(`^[A-Za-z_][A-Za-z0-9_]*$`, key); !ok { + if !providerSafeName.MatchString(key) { t.Fatalf("long SecretKey = %q, want provider-safe name", key) } } From 6f2401635c1139394966431095b05ccad65f05d1 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 10 May 2026 16:30:07 -0400 Subject: [PATCH 5/5] fix(wfctl): protect routed secrets in audit --- cmd/wfctl/infra_audit_state_secrets.go | 26 +++++++++++--------- cmd/wfctl/infra_audit_state_secrets_test.go | 27 +++++++++++++++++++++ iac/sensitive/route.go | 5 ++-- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/cmd/wfctl/infra_audit_state_secrets.go b/cmd/wfctl/infra_audit_state_secrets.go index 7d1b9da5..fdcdadb9 100644 --- a/cmd/wfctl/infra_audit_state_secrets.go +++ b/cmd/wfctl/infra_audit_state_secrets.go @@ -21,9 +21,10 @@ import ( // - a plaintext value matching secrets.DefaultSensitiveKeys() → flag legacy. // - a "secret://" string → flag mistaken config-reference in state. // -// Then walks secrets.Provider.List() (when supported) for any -// "_" name whose is NOT in IaCStateStore → -// orphan, candidate for prune. +// Then walks secrets.Provider.List() (when supported) for any name that is +// neither directly referenced by a state placeholder nor recoverable as a +// legacy "_" routed secret for a state resource → orphan, +// candidate for prune. // // Exit codes: // @@ -103,15 +104,12 @@ func runAuditStateSecretsWithPrune(ctx context.Context, w io.Writer, store infra defaultSensitive[k] = struct{}{} } - // Collect the union of sensitive-output key names actually observed in - // state records (any Outputs[k] whose value is a secret_ref:// - // placeholder). Drivers may declare sensitive keys that aren't in - // secrets.DefaultSensitiveKeys() — without harvesting these, orphan - // detection's stripKnownSensitiveSuffix would fail to recover the - // resource-name prefix and misflag valid routed secrets as orphans - // (and --prune would delete them). Defaults remain in the suffix set - // as a fallback for first-run scenarios where state hasn't recorded - // any placeholders yet. + // Collect live placeholder keys plus the union of sensitive-output key + // names observed in state records. New routed secrets are authoritative by + // placeholder value because SecretKey(resource,key) may include hashing and + // truncation. The suffix set remains only as a legacy fallback for older + // "_" routed-secret names. + liveSecretNames := map[string]struct{}{} observedSensitive := map[string]struct{}{} for k := range defaultSensitive { observedSensitive[k] = struct{}{} @@ -120,6 +118,7 @@ func runAuditStateSecretsWithPrune(ctx context.Context, w io.Writer, store infra for k, v := range states[i].Outputs { if sensitive.IsPlaceholder(v) { observedSensitive[k] = struct{}{} + liveSecretNames[strings.TrimPrefix(v.(string), sensitive.PlaceholderPrefix)] = struct{}{} } } } @@ -181,6 +180,9 @@ func runAuditStateSecretsWithPrune(ctx context.Context, w io.Writer, store infra case err == nil: sort.Strings(names) for _, name := range names { + if _, ok := liveSecretNames[name]; ok { + continue + } res := stripSensitiveSuffix(name, observedSensitive) if _, ok := stateNames[res]; ok { continue diff --git a/cmd/wfctl/infra_audit_state_secrets_test.go b/cmd/wfctl/infra_audit_state_secrets_test.go index 3afa34d5..55cfbbe7 100644 --- a/cmd/wfctl/infra_audit_state_secrets_test.go +++ b/cmd/wfctl/infra_audit_state_secrets_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/GoCodeAlone/workflow/iac/sensitive" "github.com/GoCodeAlone/workflow/interfaces" "github.com/GoCodeAlone/workflow/secrets" ) @@ -159,3 +160,29 @@ func TestAuditStateSecrets_ValidPlaceholderWithMatchingProvider_NoFinding(t *tes t.Errorf("rc = %d, want 0 (placeholder + matching provider value); output:\n%s", rc, w.String()) } } + +func TestAuditStateSecrets_NewSecretKeyPlaceholderNotPruned(t *testing.T) { + liveKey := sensitive.SecretKey("live", "secret_key") + store := &stubInfraStore{ + saved: []interfaces.ResourceState{ + {Name: "live", Outputs: map[string]any{"secret_key": sensitive.Placeholder("live", "secret_key")}}, + }, + } + prov := newEnvTestProvider() + prov.values[liveKey] = "VALID" + prov.values["ghost_secret_key"] = "ORPHAN" + w := &bytes.Buffer{} + rc := runAuditStateSecretsWithPrune(context.Background(), w, store, prov, true) + if rc != 0 { + t.Errorf("rc = %d, want 0 after pruning only orphan; output:\n%s", rc, w.String()) + } + if _, ok := prov.values[liveKey]; !ok { + t.Fatalf("live routed secret %q was pruned; remaining values: %v", liveKey, prov.values) + } + if _, ok := prov.values["ghost_secret_key"]; ok { + t.Fatalf("orphan secret was not pruned; remaining values: %v", prov.values) + } + if strings.Contains(w.String(), liveKey) { + t.Errorf("live routed secret should not be reported as orphan; output:\n%s", w.String()) + } +} diff --git a/iac/sensitive/route.go b/iac/sensitive/route.go index 4cbf760b..e078a9f9 100644 --- a/iac/sensitive/route.go +++ b/iac/sensitive/route.go @@ -6,8 +6,9 @@ // - Route is invoked on Create/Update only. Read/Adoption/Refresh paths // use Sanitize-only logic (not in this package — see // cmd/wfctl/infra_apply.go) to prevent cache pollution. -// - The placeholder format "secret_ref://_" is distinct -// from the user-supplied "secret://" config-reference convention. +// - The placeholder format "secret_ref://" is +// distinct from the user-supplied "secret://" config-reference +// convention. // - Routing trigger is exclusively out.Sensitive[k]==true (per-call // dynamic). ResourceDriver.SensitiveKeys() is NOT consulted here; // it remains a display-masking signal.