diff --git a/module/benchmark_iac_state_backend_test.go b/module/benchmark_iac_state_backend_test.go index 1b6aee41..68093414 100644 --- a/module/benchmark_iac_state_backend_test.go +++ b/module/benchmark_iac_state_backend_test.go @@ -2,7 +2,6 @@ package module import ( "context" - "encoding/json" "net" "strconv" "strings" @@ -10,9 +9,7 @@ import ( pb "github.com/GoCodeAlone/workflow/plugin/external/proto" "google.golang.org/grpc" - "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials/insecure" - "google.golang.org/grpc/status" "google.golang.org/grpc/test/bufconn" ) @@ -30,68 +27,6 @@ func oneMBState() *IaCState { } } -// benchStateToProto — local, self-contained IaCState -> pb.IaCState converter. -// Task 7 replaces this with the production iacStateToProto. -func benchStateToProto(s *IaCState) *pb.IaCState { - outJSON, _ := json.Marshal(s.Outputs) - cfgJSON, _ := json.Marshal(s.Config) - return &pb.IaCState{ - ResourceId: s.ResourceID, ResourceType: s.ResourceType, Provider: s.Provider, - Status: s.Status, OutputsJson: outJSON, ConfigJson: cfgJSON, - CreatedAt: s.CreatedAt, UpdatedAt: s.UpdatedAt, - } -} - -// benchStateBackendServer wraps an IaCStateStore behind pb.IaCStateBackendServer. -// Task 7 promotes this to the production iacStateBackendServer. -type benchStateBackendServer struct { - pb.UnimplementedIaCStateBackendServer - store IaCStateStore -} - -func (s *benchStateBackendServer) GetState(_ context.Context, r *pb.GetStateRequest) (*pb.GetStateResponse, error) { - st, err := s.store.GetState(r.ResourceId) - if err != nil { - return nil, err - } - if st == nil { - return &pb.GetStateResponse{Exists: false}, nil - } - return &pb.GetStateResponse{Exists: true, State: benchStateToProto(st)}, nil -} -func (s *benchStateBackendServer) SaveState(_ context.Context, r *pb.SaveStateRequest) (*pb.SaveStateResponse, error) { - if r.State == nil { - return nil, status.Error(codes.InvalidArgument, "SaveState: request State is nil") - } - var outputs, config map[string]any - if len(r.State.OutputsJson) > 0 { - if err := json.Unmarshal(r.State.OutputsJson, &outputs); err != nil { - return nil, status.Errorf(codes.InvalidArgument, "SaveState: invalid OutputsJson: %v", err) - } - } - if len(r.State.ConfigJson) > 0 { - if err := json.Unmarshal(r.State.ConfigJson, &config); err != nil { - return nil, status.Errorf(codes.InvalidArgument, "SaveState: invalid ConfigJson: %v", err) - } - } - return &pb.SaveStateResponse{}, s.store.SaveState(&IaCState{ - ResourceID: r.State.ResourceId, ResourceType: r.State.ResourceType, - Provider: r.State.Provider, Status: r.State.Status, Outputs: outputs, Config: config, - }) -} -func (s *benchStateBackendServer) Lock(_ context.Context, r *pb.LockRequest) (*pb.LockResponse, error) { - return &pb.LockResponse{}, s.store.Lock(r.ResourceId) -} -func (s *benchStateBackendServer) Unlock(_ context.Context, r *pb.UnlockRequest) (*pb.UnlockResponse, error) { - return &pb.UnlockResponse{}, s.store.Unlock(r.ResourceId) -} -func (s *benchStateBackendServer) ListStates(_ context.Context, _ *pb.ListStatesRequest) (*pb.ListStatesResponse, error) { - return &pb.ListStatesResponse{}, nil -} -func (s *benchStateBackendServer) DeleteState(_ context.Context, r *pb.DeleteStateRequest) (*pb.DeleteStateResponse, error) { - return &pb.DeleteStateResponse{}, s.store.DeleteState(r.ResourceId) -} - // BenchmarkIaCStateBackend_InProcess is the baseline: direct IaCStateStore calls. func BenchmarkIaCStateBackend_InProcess(b *testing.B) { store := NewMemoryIaCStateStore() @@ -119,9 +54,9 @@ func BenchmarkIaCStateBackend_GRPC(b *testing.B) { // 4 MiB in-memory listener buffer. Note: this sizes the bufconn pipe only; // gRPC's own max message size is configured separately via dial/server options. lis := bufconn.Listen(4 << 20) - defer lis.Close() + b.Cleanup(func() { _ = lis.Close() }) srv := grpc.NewServer() - pb.RegisterIaCStateBackendServer(srv, &benchStateBackendServer{store: NewMemoryIaCStateStore()}) + pb.RegisterIaCStateBackendServer(srv, &iacStateBackendServer{store: NewMemoryIaCStateStore()}) go func() { _ = srv.Serve(lis) }() defer srv.Stop() @@ -134,7 +69,10 @@ func BenchmarkIaCStateBackend_GRPC(b *testing.B) { defer conn.Close() client := pb.NewIaCStateBackendClient(conn) st := oneMBState() - pbState := benchStateToProto(st) + pbState, err := iacStateToProto(st) + if err != nil { + b.Fatal(err) + } ctx := context.Background() b.ResetTimer() for i := 0; i < b.N; i++ { diff --git a/module/iac_module.go b/module/iac_module.go index b857c9a0..ebc8090f 100644 --- a/module/iac_module.go +++ b/module/iac_module.go @@ -115,7 +115,14 @@ func (m *IaCModule) Init(app modular.Application) error { } m.store = store default: - return fmt.Errorf("iac.state %q: unsupported backend %q (use 'memory', 'filesystem', 'spaces', 'gcs', 'azure_blob', or 'postgres')", m.name, m.backend) + // Not a core in-process backend — consult the plugin-backend registry. + // The engine populates iacStateBackendRegistryInstance at plugin-load + // time; a resolved backend is served over gRPC via grpcIaCStateStore. + if client, ok := iacStateBackendRegistryInstance.resolve(m.backend); ok { + m.store = newGRPCIaCStateStore(client) + break + } + return fmt.Errorf("iac.state %q: unsupported backend %q (use 'memory', 'filesystem', 'spaces', 'gcs', 'azure_blob', or 'postgres', or load the plugin that provides it)", m.name, m.backend) } return app.RegisterService(m.name, m.store) diff --git a/module/iac_state_grpc_client.go b/module/iac_state_grpc_client.go new file mode 100644 index 00000000..999ac6b7 --- /dev/null +++ b/module/iac_state_grpc_client.go @@ -0,0 +1,253 @@ +package module + +import ( + "context" + "encoding/json" + + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" +) + +// ───────────────────────────────────────────────────────────────────────────── +// IaCState ⇄ pb.IaCState converters. +// +// The free-form Outputs / Config map[string]any fields cross the wire as JSON +// bytes — the iac.proto hard invariant (iac.proto:6-10) forbids +// google.protobuf.Struct. The plugin/host owns json.Marshal/Unmarshal directly. +// ───────────────────────────────────────────────────────────────────────────── + +// iacStateToProto converts a module IaCState into its proto wire form. +func iacStateToProto(s *IaCState) (*pb.IaCState, error) { + if s == nil { + return nil, nil + } + outputsJSON, err := json.Marshal(s.Outputs) + if err != nil { + return nil, err + } + configJSON, err := json.Marshal(s.Config) + if err != nil { + return nil, err + } + return &pb.IaCState{ + ResourceId: s.ResourceID, + ResourceType: s.ResourceType, + Provider: s.Provider, + ProviderRef: s.ProviderRef, + ProviderId: s.ProviderID, + ConfigHash: s.ConfigHash, + Status: s.Status, + OutputsJson: outputsJSON, + ConfigJson: configJSON, + Dependencies: s.Dependencies, + CreatedAt: s.CreatedAt, + UpdatedAt: s.UpdatedAt, + Error: s.Error, + }, nil +} + +// iacStateFromProto converts a proto IaCState back into a module IaCState. +// +// Empty / "null" / "{}" JSON byte payloads decode to a nil map (not an empty +// non-nil map) so round-trips through a nil Outputs/Config stay clean. +func iacStateFromProto(p *pb.IaCState) (*IaCState, error) { + if p == nil { + return nil, nil + } + outputs, err := jsonBytesToMap(p.OutputsJson) + if err != nil { + return nil, err + } + config, err := jsonBytesToMap(p.ConfigJson) + if err != nil { + return nil, err + } + return &IaCState{ + ResourceID: p.ResourceId, + ResourceType: p.ResourceType, + Provider: p.Provider, + ProviderRef: p.ProviderRef, + ProviderID: p.ProviderId, + ConfigHash: p.ConfigHash, + Status: p.Status, + Outputs: outputs, + Config: config, + Dependencies: p.Dependencies, + CreatedAt: p.CreatedAt, + UpdatedAt: p.UpdatedAt, + Error: p.Error, + }, nil +} + +// jsonBytesToMap decodes JSON bytes into a map[string]any. Empty, "null" and +// "{}" inputs yield a nil map. +func jsonBytesToMap(b []byte) (map[string]any, error) { + s := string(b) + if len(b) == 0 || s == "null" || s == "{}" { + return nil, nil + } + var m map[string]any + if err := json.Unmarshal(b, &m); err != nil { + return nil, err + } + return m, nil +} + +// ───────────────────────────────────────────────────────────────────────────── +// grpcIaCStateStore — host-side IaCStateStore implemented over an +// IaCStateBackendClient. The host half of the strict IaCStateBackend contract. +// ───────────────────────────────────────────────────────────────────────────── + +// grpcIaCStateStore adapts a pb.IaCStateBackendClient to module.IaCStateStore. +// +// All six methods call the backend with context.Background(): the +// module.IaCStateStore interface has no ctx parameter, so there is no caller +// context to plumb today. Threading a real context through IaCStateStore is a +// known follow-up, out of scope for this extraction. +type grpcIaCStateStore struct { + client pb.IaCStateBackendClient +} + +// newGRPCIaCStateStore wraps an IaCStateBackendClient as an IaCStateStore. +func newGRPCIaCStateStore(c pb.IaCStateBackendClient) *grpcIaCStateStore { + return &grpcIaCStateStore{client: c} +} + +// GetState retrieves a state record by resource ID. Returns nil, nil when the +// backend reports the record does not exist. +func (s *grpcIaCStateStore) GetState(resourceID string) (*IaCState, error) { + resp, err := s.client.GetState(context.Background(), &pb.GetStateRequest{ResourceId: resourceID}) + if err != nil { + return nil, err + } + if !resp.Exists { + return nil, nil + } + return iacStateFromProto(resp.State) +} + +// SaveState inserts or replaces a state record. +func (s *grpcIaCStateStore) SaveState(state *IaCState) error { + pbState, err := iacStateToProto(state) + if err != nil { + return err + } + _, err = s.client.SaveState(context.Background(), &pb.SaveStateRequest{State: pbState}) + return err +} + +// ListStates returns all state records matching the provided key=value filter. +func (s *grpcIaCStateStore) ListStates(filter map[string]string) ([]*IaCState, error) { + resp, err := s.client.ListStates(context.Background(), &pb.ListStatesRequest{Filter: filter}) + if err != nil { + return nil, err + } + states := make([]*IaCState, 0, len(resp.States)) + for _, p := range resp.States { + st, convErr := iacStateFromProto(p) + if convErr != nil { + return nil, convErr + } + states = append(states, st) + } + return states, nil +} + +// DeleteState removes a state record by resource ID. +func (s *grpcIaCStateStore) DeleteState(resourceID string) error { + _, err := s.client.DeleteState(context.Background(), &pb.DeleteStateRequest{ResourceId: resourceID}) + return err +} + +// Lock acquires an exclusive lock for the given resource ID. +func (s *grpcIaCStateStore) Lock(resourceID string) error { + _, err := s.client.Lock(context.Background(), &pb.LockRequest{ResourceId: resourceID}) + return err +} + +// Unlock releases the lock for the given resource ID. +func (s *grpcIaCStateStore) Unlock(resourceID string) error { + _, err := s.client.Unlock(context.Background(), &pb.UnlockRequest{ResourceId: resourceID}) + return err +} + +// ───────────────────────────────────────────────────────────────────────────── +// iacStateBackendServer — production pb.IaCStateBackendServer that delegates to +// any module.IaCStateStore. The plugin-side half of the contract. +// ───────────────────────────────────────────────────────────────────────────── + +// iacStateBackendServer serves an IaCStateStore over the IaCStateBackend gRPC +// contract. +type iacStateBackendServer struct { + pb.UnimplementedIaCStateBackendServer + store IaCStateStore +} + +// GetState delegates to the backing store, mapping a not-found (nil) result to +// GetStateResponse{Exists: false}. +func (s *iacStateBackendServer) GetState(_ context.Context, r *pb.GetStateRequest) (*pb.GetStateResponse, error) { + st, err := s.store.GetState(r.ResourceId) + if err != nil { + return nil, err + } + if st == nil { + return &pb.GetStateResponse{Exists: false}, nil + } + pbState, err := iacStateToProto(st) + if err != nil { + return nil, err + } + return &pb.GetStateResponse{Exists: true, State: pbState}, nil +} + +// SaveState delegates a full-state replace to the backing store. +func (s *iacStateBackendServer) SaveState(_ context.Context, r *pb.SaveStateRequest) (*pb.SaveStateResponse, error) { + st, err := iacStateFromProto(r.State) + if err != nil { + return nil, err + } + if err := s.store.SaveState(st); err != nil { + return nil, err + } + return &pb.SaveStateResponse{}, nil +} + +// ListStates delegates a filtered listing to the backing store. +func (s *iacStateBackendServer) ListStates(_ context.Context, r *pb.ListStatesRequest) (*pb.ListStatesResponse, error) { + states, err := s.store.ListStates(r.Filter) + if err != nil { + return nil, err + } + pbStates := make([]*pb.IaCState, 0, len(states)) + for _, st := range states { + pbState, convErr := iacStateToProto(st) + if convErr != nil { + return nil, convErr + } + pbStates = append(pbStates, pbState) + } + return &pb.ListStatesResponse{States: pbStates}, nil +} + +// DeleteState delegates a delete-by-ID to the backing store. +func (s *iacStateBackendServer) DeleteState(_ context.Context, r *pb.DeleteStateRequest) (*pb.DeleteStateResponse, error) { + if err := s.store.DeleteState(r.ResourceId); err != nil { + return nil, err + } + return &pb.DeleteStateResponse{}, nil +} + +// Lock delegates lock acquisition to the backing store. +func (s *iacStateBackendServer) Lock(_ context.Context, r *pb.LockRequest) (*pb.LockResponse, error) { + if err := s.store.Lock(r.ResourceId); err != nil { + return nil, err + } + return &pb.LockResponse{}, nil +} + +// Unlock delegates lock release to the backing store. +func (s *iacStateBackendServer) Unlock(_ context.Context, r *pb.UnlockRequest) (*pb.UnlockResponse, error) { + if err := s.store.Unlock(r.ResourceId); err != nil { + return nil, err + } + return &pb.UnlockResponse{}, nil +} diff --git a/module/iac_state_grpc_client_test.go b/module/iac_state_grpc_client_test.go new file mode 100644 index 00000000..93af5230 --- /dev/null +++ b/module/iac_state_grpc_client_test.go @@ -0,0 +1,54 @@ +package module + +import ( + "context" + "net" + "testing" + + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/test/bufconn" +) + +func TestGRPCIaCStateStoreRoundTrip(t *testing.T) { + lis := bufconn.Listen(4 << 20) + t.Cleanup(func() { _ = lis.Close() }) + srv := grpc.NewServer() + pb.RegisterIaCStateBackendServer(srv, &iacStateBackendServer{store: NewMemoryIaCStateStore()}) + go func() { _ = srv.Serve(lis) }() + defer srv.Stop() + + conn, err := grpc.NewClient("passthrough:///bufnet", + grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { return lis.DialContext(ctx) }), + grpc.WithTransportCredentials(insecure.NewCredentials())) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + + var store IaCStateStore = newGRPCIaCStateStore(pb.NewIaCStateBackendClient(conn)) + + want := &IaCState{ResourceID: "r1", ResourceType: "kubernetes", Provider: "azure", Status: "active", + Outputs: map[string]any{"endpoint": "https://x"}, Config: map[string]any{"size": "L"}} + if err := store.SaveState(want); err != nil { + t.Fatalf("SaveState: %v", err) + } + got, err := store.GetState("r1") + if err != nil || got == nil { + t.Fatalf("GetState: %v (got=%v)", err, got) + } + if got.ResourceID != "r1" || got.Status != "active" || got.Outputs["endpoint"] != "https://x" { + t.Fatalf("round-trip mismatch: %+v", got) + } + if err := store.Lock("r1"); err != nil { + t.Fatalf("Lock: %v", err) + } + missing, err := store.GetState("nope") + if err != nil || missing != nil { + t.Fatalf("GetState(missing) should be nil,nil — got %v,%v", missing, err) + } + if err := store.Unlock("r1"); err != nil { + t.Fatalf("Unlock: %v", err) + } +} diff --git a/module/iac_state_plugin_registry.go b/module/iac_state_plugin_registry.go new file mode 100644 index 00000000..c1f18685 --- /dev/null +++ b/module/iac_state_plugin_registry.go @@ -0,0 +1,73 @@ +package module + +import ( + "fmt" + "strings" + "sync" + + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" +) + +// ───────────────────────────────────────────────────────────────────────────── +// iacStateBackendRegistry — engine-side registry mapping an iac.state backend +// name to a plugin-served pb.IaCStateBackendClient. +// +// The engine populates the package-level singleton at plugin-load time +// (Task 14); IaCModule.Init consults it for any backend name not handled by an +// in-process core case. Reserved core backend names (memory/filesystem/postgres) +// — the backends that have no cloud SDK and stay in core — cannot be claimed by +// a plugin. +// ───────────────────────────────────────────────────────────────────────────── + +// reservedIaCStateBackends are the core backend names a plugin may never claim. +var reservedIaCStateBackends = map[string]struct{}{ + "memory": {}, + "filesystem": {}, + "postgres": {}, +} + +// iacStateBackendRegistry maps a backend name to a plugin gRPC client. +type iacStateBackendRegistry struct { + mu sync.RWMutex + clients map[string]pb.IaCStateBackendClient +} + +// newIaCStateBackendRegistry constructs an empty registry. +func newIaCStateBackendRegistry() *iacStateBackendRegistry { + return &iacStateBackendRegistry{clients: make(map[string]pb.IaCStateBackendClient)} +} + +// register associates a backend name with a plugin client. The name must be +// non-empty (after trimming) and the client must be non-nil. Reserved core +// backend names are rejected. Re-registering a non-reserved name overwrites the +// previous client (last plugin loaded wins). +func (r *iacStateBackendRegistry) register(name string, client pb.IaCStateBackendClient) error { + name = strings.TrimSpace(name) + if name == "" { + return fmt.Errorf("iac.state backend registration: name must not be empty") + } + if client == nil { + return fmt.Errorf("iac.state backend registration %q: client must not be nil", name) + } + if _, reserved := reservedIaCStateBackends[name]; reserved { + return fmt.Errorf("plugin registered reserved iac.state backend name %q", name) + } + r.mu.Lock() + defer r.mu.Unlock() + r.clients[name] = client + return nil +} + +// resolve returns the plugin client for a backend name, and whether one is +// registered. +func (r *iacStateBackendRegistry) resolve(name string) (pb.IaCStateBackendClient, bool) { + r.mu.RLock() + defer r.mu.RUnlock() + c, ok := r.clients[name] + return c, ok +} + +// iacStateBackendRegistryInstance is the package-level singleton the engine +// populates and IaCModule.Init consults. Task 14 adds an exported +// RegisterIaCStateBackend wrapper around it. +var iacStateBackendRegistryInstance = newIaCStateBackendRegistry() diff --git a/module/iac_state_plugin_registry_test.go b/module/iac_state_plugin_registry_test.go new file mode 100644 index 00000000..38334f82 --- /dev/null +++ b/module/iac_state_plugin_registry_test.go @@ -0,0 +1,93 @@ +package module + +import ( + "context" + "testing" + + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" + "google.golang.org/grpc" +) + +// fakeStateBackendClient is a no-op pb.IaCStateBackendClient stub. The registry +// tests only need it to satisfy the interface; no method is ever called. +type fakeStateBackendClient struct{} + +func (*fakeStateBackendClient) GetState(context.Context, *pb.GetStateRequest, ...grpc.CallOption) (*pb.GetStateResponse, error) { + return nil, nil +} +func (*fakeStateBackendClient) SaveState(context.Context, *pb.SaveStateRequest, ...grpc.CallOption) (*pb.SaveStateResponse, error) { + return nil, nil +} +func (*fakeStateBackendClient) ListStates(context.Context, *pb.ListStatesRequest, ...grpc.CallOption) (*pb.ListStatesResponse, error) { + return nil, nil +} +func (*fakeStateBackendClient) DeleteState(context.Context, *pb.DeleteStateRequest, ...grpc.CallOption) (*pb.DeleteStateResponse, error) { + return nil, nil +} +func (*fakeStateBackendClient) Lock(context.Context, *pb.LockRequest, ...grpc.CallOption) (*pb.LockResponse, error) { + return nil, nil +} +func (*fakeStateBackendClient) Unlock(context.Context, *pb.UnlockRequest, ...grpc.CallOption) (*pb.UnlockResponse, error) { + return nil, nil +} + +func TestIaCStateBackendRegistry(t *testing.T) { + reg := newIaCStateBackendRegistry() + if _, ok := reg.resolve("azure_blob"); ok { + t.Fatal("empty registry should not resolve azure_blob") + } + fake := &fakeStateBackendClient{} + if err := reg.register("azure_blob", fake); err != nil { + t.Fatalf("register: %v", err) + } + got, ok := reg.resolve("azure_blob") + if !ok || got != fake { + t.Fatalf("resolve azure_blob: ok=%v got=%v", ok, got) + } + for _, reserved := range []string{"memory", "filesystem", "postgres"} { + if err := reg.register(reserved, fake); err == nil { + t.Fatalf("register(%q) must fail — reserved core backend name", reserved) + } + } + // Empty / whitespace-only name must be rejected. + for _, bad := range []string{"", " "} { + if err := reg.register(bad, fake); err == nil { + t.Fatalf("register(%q) must fail — empty backend name", bad) + } + } + // Nil client must be rejected. + if err := reg.register("nilclient_backend", nil); err == nil { + t.Fatal("register with nil client must fail") + } + // A name surrounded by whitespace is trimmed and registers under the trimmed key. + if err := reg.register(" spaced_backend ", fake); err != nil { + t.Fatalf("register trimmed name: %v", err) + } + if _, ok := reg.resolve("spaced_backend"); !ok { + t.Fatal("trimmed name must resolve under its trimmed key") + } +} + +// TestIaCModule_PluginBackendDispatch exercises the real IaCModule.Init() path: +// a backend name no in-process switch case matches is resolved from the +// package-level iacStateBackendRegistryInstance, yielding a *grpcIaCStateStore. +func TestIaCModule_PluginBackendDispatch(t *testing.T) { + const backend = "azure_blob_test_only" + fake := &fakeStateBackendClient{} + if err := iacStateBackendRegistryInstance.register(backend, fake); err != nil { + t.Fatalf("register: %v", err) + } + defer func() { + iacStateBackendRegistryInstance.mu.Lock() + delete(iacStateBackendRegistryInstance.clients, backend) + iacStateBackendRegistryInstance.mu.Unlock() + }() + + m := NewIaCModule("iac-plugin", map[string]any{"backend": backend}) + if err := m.Init(NewMockApplication()); err != nil { + t.Fatalf("Init: %v", err) + } + if _, ok := m.store.(*grpcIaCStateStore); !ok { + t.Fatalf("m.store is %T, want *grpcIaCStateStore", m.store) + } +} diff --git a/module/step_output_redactor.go b/module/step_output_redactor.go index aded40b4..7d8562f3 100644 --- a/module/step_output_redactor.go +++ b/module/step_output_redactor.go @@ -24,6 +24,19 @@ const RedactionPlaceholder = "[REDACTED]" // safeFieldSuffix marks a field as explicitly safe and exempt from redaction. const safeFieldSuffix = "_display" +// refFieldSuffix marks a field as a reference (a module/resource name, not a +// secret value). A "_ref" key is exempt from redaction ONLY when its sensitive +// match comes from a structural-reference word ("credential"). A key like +// "bearer_token_ref" still redacts, because "token" is a value-bearing secret +// pattern, not a structural reference — the "_ref" suffix must not be a blanket +// bypass for every sensitive pattern. +const refFieldSuffix = "_ref" + +// refExemptPatterns are the sensitive patterns that a "_ref" suffix is allowed +// to exempt: words that describe a *reference to* a credential-holding module +// (e.g. "credentials_ref"), not words that name a secret value itself. +var refExemptPatterns = []string{"credential"} + // RedactStepOutput recursively scans output and replaces values of sensitive // fields with RedactionPlaceholder. Field names are matched case-insensitively // against SensitiveFieldPatterns. Fields ending with "_display" are never @@ -58,16 +71,44 @@ func redactMap(m map[string]any, patterns []string) map[string]any { } // isSensitiveField returns true when the lowercased field name contains any of -// the patterns and does not have the safe suffix. +// the patterns and is not exempted by a safe/reference suffix. func isSensitiveField(name string, patterns []string) bool { lower := strings.ToLower(name) if strings.HasSuffix(lower, safeFieldSuffix) { return false } + var matched []string for _, p := range patterns { if strings.Contains(lower, p) { - return true + matched = append(matched, p) + } + } + if len(matched) == 0 { + return false + } + // A "_ref" key is exempt ONLY when every sensitive pattern it matched is a + // structural-reference word (e.g. "credentials_ref" → "credential"). A key + // like "bearer_token_ref" still redacts because "token" names a secret + // value, so "_ref" must not blanket-bypass it. + if strings.HasSuffix(lower, refFieldSuffix) && allRefExempt(matched) { + return false + } + return true +} + +// allRefExempt reports whether every matched pattern is in refExemptPatterns. +func allRefExempt(matched []string) bool { + for _, m := range matched { + exempt := false + for _, e := range refExemptPatterns { + if m == e { + exempt = true + break + } + } + if !exempt { + return false } } - return false + return true } diff --git a/module/step_output_redactor_test.go b/module/step_output_redactor_test.go index 1e2d3894..6c82bc23 100644 --- a/module/step_output_redactor_test.go +++ b/module/step_output_redactor_test.go @@ -168,3 +168,50 @@ func TestRedactStepOutput_EmptyMap(t *testing.T) { t.Errorf("empty map should return empty map, got %v", got) } } + +func TestRedactCredentialsBlock(t *testing.T) { + in := map[string]any{ + "credentials": map[string]any{ + "accessKey": "AKIAEXAMPLE", + "secretKey": "supersecret", + }, + "credentials_ref": "aws-creds-module", + "bucket": "public-bucket-name", + } + out := RedactStepOutput(in) + // The credentials: block is redacted WHOLESALE — the existing "credential" + // pattern replaces the whole sub-tree with the placeholder STRING (no + // recursion). That is safe and is the design-sanctioned "already covered". + if out["credentials"] != RedactionPlaceholder { + t.Fatalf("credentials block must be wholesale-redacted, got: %#v", out["credentials"]) + } + // credentials_ref is a module NAME, not a secret — must be PRESERVED. + if out["credentials_ref"] != "aws-creds-module" { + t.Fatalf("credentials_ref must NOT be redacted (it is a module reference): %#v", out["credentials_ref"]) + } + if out["bucket"] != "public-bucket-name" { + t.Fatalf("non-sensitive field wrongly redacted") + } +} + +// TestRedactRefSuffixDoesNotBypassValueSecrets locks in that the "_ref" suffix +// exempts ONLY structural-reference words (credentials_ref) — it must NOT be a +// blanket bypass for value-bearing secret patterns. A key like +// "bearer_token_ref" still matches "token" and must redact. +func TestRedactRefSuffixDoesNotBypassValueSecrets(t *testing.T) { + in := map[string]any{ + "credentials_ref": "aws-creds-module", // structural ref → preserved + "bearer_token_ref": "tok-abc123", // matches "token" → must redact + "api_key_ref": "ak-secret", // matches "api_key" → must redact + "secret_ref": "shhh", // matches "secret" → must redact + } + out := RedactStepOutput(in) + if out["credentials_ref"] != "aws-creds-module" { + t.Errorf("credentials_ref must be preserved, got %#v", out["credentials_ref"]) + } + for _, k := range []string{"bearer_token_ref", "api_key_ref", "secret_ref"} { + if out[k] != RedactionPlaceholder { + t.Errorf("%s matches a value-bearing secret pattern — _ref must not bypass redaction, got %#v", k, out[k]) + } + } +} diff --git a/plugin/external/grpc_logging_guard_test.go b/plugin/external/grpc_logging_guard_test.go new file mode 100644 index 00000000..bc548cde --- /dev/null +++ b/plugin/external/grpc_logging_guard_test.go @@ -0,0 +1,72 @@ +package external + +import ( + "io/fs" + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// interceptorAllowlist is the set of plugin/external/** Go files (path relative +// to plugin/external/) that are permitted to reference a gRPC interceptor +// option. A file lands here only after a reviewer has confirmed it does NOT log +// request bodies — CreateModule requests carry inline credentials: blocks. +// Empty by design: today nothing legitimately installs an interceptor. +var interceptorAllowlist = map[string]struct{}{} + +// isGeneratedProtoFile reports whether path is protoc-generated code. Generated +// *_grpc.pb.go files reference grpc.UnaryServerInterceptor / StreamServerInterceptor +// in their service-registration types — those are type references in generated +// code, not an interceptor being *installed*, so they are not a body-logging risk. +func isGeneratedProtoFile(path string) bool { + base := filepath.Base(path) + if strings.HasSuffix(base, ".pb.go") { + return true + } + // Anything under a proto/ directory is generated wire code. + return strings.Contains(filepath.ToSlash(path), "/proto/") || strings.HasPrefix(filepath.ToSlash(path), "proto/") +} + +// TestNoBodyLoggingInterceptor walks the WHOLE plugin/external/ tree (including +// subpackages like sdk/) and fails if any non-generated, non-test, non-allowlisted +// Go file constructs grpc.NewServer / grpc.NewClient with an *Interceptor option. +// A body-logging interceptor on a credential-carrying RPC leaks inline +// credentials: blocks. Covers Unary AND Stream, Server AND Client, plain AND +// Chain* variants. See the cloud-sdk-extraction design, Security section. +func TestNoBodyLoggingInterceptor(t *testing.T) { + interceptorOpt := regexp.MustCompile(`(Chain)?(Unary|Stream)(Server|Client)?Interceptor`) + + err := filepath.WalkDir(".", func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + return nil + } + rel := filepath.ToSlash(path) + if !strings.HasSuffix(rel, ".go") || strings.HasSuffix(rel, "_test.go") { + return nil + } + if isGeneratedProtoFile(rel) { + return nil + } + if _, ok := interceptorAllowlist[rel]; ok { + return nil + } + b, readErr := os.ReadFile(path) + if readErr != nil { + return readErr + } + if interceptorOpt.Match(b) { + t.Errorf("%s references a gRPC interceptor option — if it logs request "+ + "bodies it can leak inline credentials: blocks. Audit it and, if safe, "+ + "add its plugin/external-relative path to interceptorAllowlist in this test.", rel) + } + return nil + }) + if err != nil { + t.Fatal(err) + } +}