diff --git a/iac/admin/handler/apply_resource.go b/iac/admin/handler/apply_resource.go index ff236887..0fadf1d2 100644 --- a/iac/admin/handler/apply_resource.go +++ b/iac/admin/handler/apply_resource.go @@ -46,7 +46,7 @@ func ApplyResource( ) (*adminpb.AdminApplyOutput, error) { // Gate 1: default-deny. if msg := authzError(in.GetEvidence()); msg != "" { - return &adminpb.AdminApplyOutput{Error: msg}, nil + return &adminpb.AdminApplyOutput{Error: msg}, ErrAuthzDenied } // Gate 2: server-side RBAC (NOT the client's evidence.granted_permissions). @@ -56,7 +56,9 @@ func ApplyResource( return &adminpb.AdminApplyOutput{Error: "apply: authz enforce error"}, nil //nolint:nilerr } if !ok { - return &adminpb.AdminApplyOutput{Error: "apply: infra:apply denied for subject " + subject}, nil + // Generic denial — do NOT reflect the authenticated subject in the + // response body. Subject is captured by the module-layer audit log. + return &adminpb.AdminApplyOutput{Error: "apply: infra:apply denied"}, ErrAuthzDenied } } diff --git a/iac/admin/handler/apply_resource_test.go b/iac/admin/handler/apply_resource_test.go index c1554921..6e2debd1 100644 --- a/iac/admin/handler/apply_resource_test.go +++ b/iac/admin/handler/apply_resource_test.go @@ -2,11 +2,11 @@ package handler_test import ( "context" + "errors" "testing" "github.com/GoCodeAlone/workflow/iac/admin/handler" adminpb "github.com/GoCodeAlone/workflow/iac/admin/proto" - "github.com/GoCodeAlone/workflow/iac/stubprovider" "github.com/GoCodeAlone/workflow/interfaces" ) @@ -25,7 +25,7 @@ func (e *testEnforcer) Enforce(sub, obj, act string, _ ...string) (bool, error) // TestApplyResource_DefaultDeny asserts that evidence with checked=false // returns a non-empty error (default-deny). func TestApplyResource_DefaultDeny(t *testing.T) { - prov := stubprovider.New() + prov := &planningProvider{} providers := map[string]interfaces.IaCProvider{"stub": prov} desired := []interfaces.ResourceSpec{ {Name: "vpc1", Type: "infra.vpc"}, @@ -40,8 +40,8 @@ func TestApplyResource_DefaultDeny(t *testing.T) { Evidence: &adminpb.AdminAuthzEvidence{AuthzChecked: false}, } out, err := handler.ApplyResource(context.Background(), nil, providers, nil, "subject", nil, desired, in) - if err != nil { - t.Fatalf("ApplyResource: unexpected Go error: %v", err) + if !errors.Is(err, handler.ErrAuthzDenied) { + t.Fatalf("ApplyResource: want ErrAuthzDenied, got %v (out.Error=%s)", err, out.GetError()) } if out.Error == "" { t.Error("ApplyResource with evidence.checked=false should return non-empty error") @@ -51,7 +51,7 @@ func TestApplyResource_DefaultDeny(t *testing.T) { // TestApplyResource_AuthzDenies asserts that a subject the enforcer // denies infra:apply → 403 even if the client body has valid evidence. func TestApplyResource_AuthzDenies(t *testing.T) { - prov := stubprovider.New() + prov := &planningProvider{} providers := map[string]interfaces.IaCProvider{"stub": prov} desired := []interfaces.ResourceSpec{{Name: "vpc1", Type: "infra.vpc"}} @@ -71,8 +71,8 @@ func TestApplyResource_AuthzDenies(t *testing.T) { }, } out, err := handler.ApplyResource(context.Background(), nil, providers, enforcer, "viewer", nil, desired, in) - if err != nil { - t.Fatalf("ApplyResource: unexpected Go error: %v", err) + if !errors.Is(err, handler.ErrAuthzDenied) { + t.Fatalf("ApplyResource: want ErrAuthzDenied, got %v (out.Error=%s)", err, out.GetError()) } if out.Error == "" { t.Error("ApplyResource should reject subject denied infra:apply by server-side Enforcer") @@ -82,7 +82,7 @@ func TestApplyResource_AuthzDenies(t *testing.T) { // TestApplyResource_HappyPath asserts that a valid evidence + hash + allowed // subject returns applied[] with no errors. func TestApplyResource_HappyPath(t *testing.T) { - prov := stubprovider.New() + prov := &planningProvider{} providers := map[string]interfaces.IaCProvider{"stub": prov} desired := []interfaces.ResourceSpec{ {Name: "vpc1", Type: "infra.vpc", Config: map[string]any{"region": "nyc1"}}, @@ -114,7 +114,7 @@ func TestApplyResource_HappyPath(t *testing.T) { // TestApplyResource_StalePlanHash asserts that a mismatched desired_hash // → "plan is stale" error and no apply. func TestApplyResource_StalePlanHash(t *testing.T) { - prov := stubprovider.New() + prov := &planningProvider{} providers := map[string]interfaces.IaCProvider{"stub": prov} desired := []interfaces.ResourceSpec{{Name: "vpc1", Type: "infra.vpc"}} diff --git a/iac/admin/handler/authz.go b/iac/admin/handler/authz.go index 84ece2ce..8c39e91a 100644 --- a/iac/admin/handler/authz.go +++ b/iac/admin/handler/authz.go @@ -39,7 +39,19 @@ package handler // Not in T5 scope; flagged here so a future contributor sees the // risk before extending the handler family. -import adminpb "github.com/GoCodeAlone/workflow/iac/admin/proto" +import ( + "errors" + + adminpb "github.com/GoCodeAlone/workflow/iac/admin/proto" +) + +// ErrAuthzDenied is the sentinel error returned by handlers when an +// authz check (evidence default-deny or server-side RBAC via Enforcer) +// rejects the request. The HTTP module layer maps this via errors.Is to +// HTTP 403 — NOT via strings.Contains on the error message, which would +// produce false positives when a provider's error message happens to +// contain "denied". +var ErrAuthzDenied = errors.New("authz denied") // authzError returns the operator-facing rejection string when the // supplied evidence does not meet default-deny criteria. Returns "" diff --git a/iac/admin/handler/destroy_resource.go b/iac/admin/handler/destroy_resource.go index 88693016..aa59c960 100644 --- a/iac/admin/handler/destroy_resource.go +++ b/iac/admin/handler/destroy_resource.go @@ -33,7 +33,7 @@ func DestroyResource( ) (*adminpb.AdminDestroyOutput, error) { // Gate 1: default-deny. if msg := authzError(in.GetEvidence()); msg != "" { - return &adminpb.AdminDestroyOutput{Error: msg}, nil + return &adminpb.AdminDestroyOutput{Error: msg}, ErrAuthzDenied } // Gate 2: server-side RBAC. @@ -43,7 +43,9 @@ func DestroyResource( return &adminpb.AdminDestroyOutput{Error: "destroy: authz enforce error"}, nil //nolint:nilerr } if !ok { - return &adminpb.AdminDestroyOutput{Error: "destroy: infra:destroy denied for subject " + subject}, nil + // Generic denial — do NOT reflect the authenticated subject in the + // response body. Subject is captured by the module-layer audit log. + return &adminpb.AdminDestroyOutput{Error: "destroy: infra:destroy denied"}, ErrAuthzDenied } } diff --git a/iac/admin/handler/destroy_resource_test.go b/iac/admin/handler/destroy_resource_test.go index 883c7ae5..cedb936b 100644 --- a/iac/admin/handler/destroy_resource_test.go +++ b/iac/admin/handler/destroy_resource_test.go @@ -2,18 +2,18 @@ package handler_test import ( "context" + "errors" "testing" "github.com/GoCodeAlone/workflow/iac/admin/handler" adminpb "github.com/GoCodeAlone/workflow/iac/admin/proto" - "github.com/GoCodeAlone/workflow/iac/stubprovider" "github.com/GoCodeAlone/workflow/interfaces" ) // TestDestroyResource_DefaultDeny asserts that evidence with checked=false // returns a non-empty error (default-deny). func TestDestroyResource_DefaultDeny(t *testing.T) { - prov := stubprovider.New() + prov := &planningProvider{} providers := map[string]interfaces.IaCProvider{"stub": prov} in := &adminpb.AdminDestroyInput{ Evidence: &adminpb.AdminAuthzEvidence{AuthzChecked: false}, @@ -22,8 +22,8 @@ func TestDestroyResource_DefaultDeny(t *testing.T) { }, } out, err := handler.DestroyResource(context.Background(), providers, nil, "operator", in) - if err != nil { - t.Fatalf("DestroyResource: unexpected Go error: %v", err) + if !errors.Is(err, handler.ErrAuthzDenied) { + t.Fatalf("DestroyResource: want ErrAuthzDenied, got %v (out.Error=%s)", err, out.GetError()) } if out.Error == "" { t.Error("DestroyResource with evidence.checked=false should return non-empty error") @@ -33,7 +33,7 @@ func TestDestroyResource_DefaultDeny(t *testing.T) { // TestDestroyResource_AuthzDenies asserts that a subject denied // infra:destroy by the Enforcer is rejected even with valid evidence. func TestDestroyResource_AuthzDenies(t *testing.T) { - prov := stubprovider.New() + prov := &planningProvider{} providers := map[string]interfaces.IaCProvider{"stub": prov} enforcer := &testEnforcer{allow: map[string]bool{ // viewer is NOT granted infra:destroy @@ -43,8 +43,8 @@ func TestDestroyResource_AuthzDenies(t *testing.T) { Refs: []*adminpb.AdminResourceRef{{Name: "vpc1", Type: "infra.vpc"}}, } out, err := handler.DestroyResource(context.Background(), providers, enforcer, "viewer", in) - if err != nil { - t.Fatalf("DestroyResource: unexpected Go error: %v", err) + if !errors.Is(err, handler.ErrAuthzDenied) { + t.Fatalf("DestroyResource: want ErrAuthzDenied, got %v (out.Error=%s)", err, out.GetError()) } if out.Error == "" { t.Error("DestroyResource should reject subject denied infra:destroy by server-side Enforcer") @@ -54,7 +54,7 @@ func TestDestroyResource_AuthzDenies(t *testing.T) { // TestDestroyResource_HappyPath asserts that a valid subject + refs + correct // confirm_hash → destroyed[] with the ref names. func TestDestroyResource_HappyPath(t *testing.T) { - prov := stubprovider.New() + prov := &planningProvider{} providers := map[string]interfaces.IaCProvider{"stub": prov} refs := []*adminpb.AdminResourceRef{ {Name: "vpc1", Type: "infra.vpc"}, @@ -80,7 +80,7 @@ func TestDestroyResource_HappyPath(t *testing.T) { // TestDestroyResource_MismatchedConfirmHash asserts that a wrong or empty // confirm_hash → TOCTOU error, no destroy operation performed (IMPORTANT-1). func TestDestroyResource_MismatchedConfirmHash(t *testing.T) { - prov := stubprovider.New() + prov := &planningProvider{} providers := map[string]interfaces.IaCProvider{"stub": prov} refs := []*adminpb.AdminResourceRef{ {Name: "vpc1", Type: "infra.vpc"}, diff --git a/iac/admin/handler/drift_check_test.go b/iac/admin/handler/drift_check_test.go index b0def60e..0ea90eb6 100644 --- a/iac/admin/handler/drift_check_test.go +++ b/iac/admin/handler/drift_check_test.go @@ -6,14 +6,13 @@ import ( "github.com/GoCodeAlone/workflow/iac/admin/handler" adminpb "github.com/GoCodeAlone/workflow/iac/admin/proto" - "github.com/GoCodeAlone/workflow/iac/stubprovider" "github.com/GoCodeAlone/workflow/interfaces" ) // TestDriftCheckResource_DefaultDeny asserts that evidence with checked=false // returns a non-empty error and no drift payload. func TestDriftCheckResource_DefaultDeny(t *testing.T) { - prov := stubprovider.New() + prov := &planningProvider{} providers := map[string]interfaces.IaCProvider{"stub": prov} in := &adminpb.AdminDriftInput{ Evidence: &adminpb.AdminAuthzEvidence{AuthzChecked: false}, @@ -36,7 +35,7 @@ func TestDriftCheckResource_DefaultDeny(t *testing.T) { // TestDriftCheckResource_ReturnsNotDrifted asserts that the stub provider's // DetectDrift (Drifted:false) maps to AdminDriftResult with Drifted:false. func TestDriftCheckResource_ReturnsNotDrifted(t *testing.T) { - prov := stubprovider.New() + prov := &planningProvider{} providers := map[string]interfaces.IaCProvider{"stub": prov} in := &adminpb.AdminDriftInput{ Evidence: &adminpb.AdminAuthzEvidence{AuthzChecked: true, AuthzAllowed: true}, diff --git a/iac/admin/handler/get_resource.go b/iac/admin/handler/get_resource.go index 597200b8..624a9531 100644 --- a/iac/admin/handler/get_resource.go +++ b/iac/admin/handler/get_resource.go @@ -38,6 +38,9 @@ func GetResource( if msg := authzError(in.GetEvidence()); msg != "" { return &adminpb.AdminGetResourceOutput{Error: msg}, nil } + if store == nil { + return &adminpb.AdminGetResourceOutput{Error: "get: no state store configured"}, nil + } state, err := store.GetResource(ctx, in.GetName()) if err != nil { diff --git a/iac/admin/handler/list_resources.go b/iac/admin/handler/list_resources.go index bfee1fc3..3915c5cf 100644 --- a/iac/admin/handler/list_resources.go +++ b/iac/admin/handler/list_resources.go @@ -41,6 +41,9 @@ func ListResources( if msg := authzError(in.GetEvidence()); msg != "" { return &adminpb.AdminListResourcesOutput{Error: msg}, nil } + if store == nil { + return &adminpb.AdminListResourcesOutput{Error: "list: no state store configured"}, nil + } states, err := store.ListResources(ctx) if err != nil { diff --git a/iac/admin/handler/list_resources_test.go b/iac/admin/handler/list_resources_test.go index 4cffefc4..24e4f6f7 100644 --- a/iac/admin/handler/list_resources_test.go +++ b/iac/admin/handler/list_resources_test.go @@ -63,6 +63,125 @@ func authzOK() *adminpb.AdminAuthzEvidence { } } +// planningProvider is a minimal interfaces.IaCProvider for handler tests. +// It replaces the deleted iac/stubprovider package — scenario fixtures must +// not live in the workflow engine core. Provides real Plan, Destroy, +// DetectDrift, and ResourceDriver behavior so tests can exercise the full +// dispatch path without an external package dependency. +type planningProvider struct{} + +var _ interfaces.IaCProvider = (*planningProvider)(nil) + +func (p *planningProvider) Name() string { return "test-planning" } +func (p *planningProvider) Version() string { return "0.0.0-test" } +func (p *planningProvider) Initialize(_ context.Context, _ map[string]any) error { + return nil +} +func (p *planningProvider) Capabilities() []interfaces.IaCCapabilityDeclaration { return nil } + +func (p *planningProvider) Plan(_ context.Context, desired []interfaces.ResourceSpec, current []interfaces.ResourceState) (*interfaces.IaCPlan, error) { + currentByName := make(map[string]*interfaces.ResourceState, len(current)) + for i := range current { + currentByName[current[i].Name] = ¤t[i] + } + desiredByName := make(map[string]struct{}, len(desired)) + for _, s := range desired { + desiredByName[s.Name] = struct{}{} + } + plan := &interfaces.IaCPlan{} + for _, spec := range desired { + if _, exists := currentByName[spec.Name]; exists { + plan.Actions = append(plan.Actions, interfaces.PlanAction{Action: "update", Resource: spec, Current: currentByName[spec.Name]}) + } else { + plan.Actions = append(plan.Actions, interfaces.PlanAction{Action: "create", Resource: spec}) + } + } + for i := range current { + st := ¤t[i] + if _, wanted := desiredByName[st.Name]; !wanted { + plan.Actions = append(plan.Actions, interfaces.PlanAction{Action: "delete", Resource: interfaces.ResourceSpec{Name: st.Name, Type: st.Type}, Current: st}) + } + } + return plan, nil +} + +func (p *planningProvider) Destroy(_ context.Context, refs []interfaces.ResourceRef) (*interfaces.DestroyResult, error) { + names := make([]string, 0, len(refs)) + for _, r := range refs { + names = append(names, r.Name) + } + return &interfaces.DestroyResult{Destroyed: names}, nil +} + +func (p *planningProvider) Status(_ context.Context, _ []interfaces.ResourceRef) ([]interfaces.ResourceStatus, error) { + return nil, nil +} + +func (p *planningProvider) DetectDrift(_ context.Context, refs []interfaces.ResourceRef) ([]interfaces.DriftResult, error) { + results := make([]interfaces.DriftResult, 0, len(refs)) + for _, r := range refs { + results = append(results, interfaces.DriftResult{Name: r.Name, Type: r.Type, Drifted: false, Class: interfaces.DriftClassInSync}) + } + return results, nil +} + +func (p *planningProvider) Import(_ context.Context, _ string, _ string) (*interfaces.ResourceState, error) { + return nil, nil +} + +func (p *planningProvider) ResolveSizing(_ string, _ interfaces.Size, _ *interfaces.ResourceHints) (*interfaces.ProviderSizing, error) { + return nil, nil +} + +func (p *planningProvider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { + return &planningDriver{}, nil +} + +func (p *planningProvider) SupportedCanonicalKeys() []string { return nil } + +func (p *planningProvider) BootstrapStateBackend(_ context.Context, _ map[string]any) (*interfaces.BootstrapResult, error) { + return nil, nil +} + +func (p *planningProvider) Close() error { return nil } + +// planningDriver is a minimal interfaces.ResourceDriver for handler tests. +type planningDriver struct{} + +var _ interfaces.ResourceDriver = (*planningDriver)(nil) + +func (d *planningDriver) Create(_ context.Context, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + return &interfaces.ResourceOutput{Name: spec.Name, Type: spec.Type, ProviderID: "test-" + spec.Name}, nil +} + +func (d *planningDriver) Read(_ context.Context, ref interfaces.ResourceRef) (*interfaces.ResourceOutput, error) { + return &interfaces.ResourceOutput{Name: ref.Name, Type: ref.Type, ProviderID: ref.ProviderID}, nil +} + +func (d *planningDriver) Update(_ context.Context, ref interfaces.ResourceRef, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + pid := ref.ProviderID + if pid == "" { + pid = "test-" + spec.Name + } + return &interfaces.ResourceOutput{Name: spec.Name, Type: spec.Type, ProviderID: pid}, nil +} + +func (d *planningDriver) Delete(_ context.Context, _ interfaces.ResourceRef) error { return nil } + +func (d *planningDriver) Diff(_ context.Context, _ interfaces.ResourceSpec, _ *interfaces.ResourceOutput) (*interfaces.DiffResult, error) { + return &interfaces.DiffResult{NeedsUpdate: false, NeedsReplace: false}, nil +} + +func (d *planningDriver) HealthCheck(_ context.Context, _ interfaces.ResourceRef) (*interfaces.HealthResult, error) { + return nil, nil +} + +func (d *planningDriver) Scale(_ context.Context, _ interfaces.ResourceRef, _ int) (*interfaces.ResourceOutput, error) { + return nil, nil +} + +func (d *planningDriver) SensitiveKeys() []string { return nil } + // seedFixture returns a 3-resource store + label-bearing state covering // the filter dimensions: type (infra.vpc vs infra.database), provider // module (do-prod vs do-staging), and app_context (web vs api). diff --git a/iac/admin/handler/plan_resource.go b/iac/admin/handler/plan_resource.go index 06e3ba2c..fede58e8 100644 --- a/iac/admin/handler/plan_resource.go +++ b/iac/admin/handler/plan_resource.go @@ -25,8 +25,10 @@ import ( // The providers map is keyed by module name; the first entry is used for // planning (single-provider per-route model for v1.1). // -// Evidence default-deny: if authzError is non-empty, the handler -// short-circuits with Output.error (HTTP stays 200; consumer sniffs tag-100). +// Evidence default-deny: if authzError is non-empty, the handler returns +// (output, ErrAuthzDenied). The module layer maps this to HTTP 403 via +// writeMutationResponse — NOT 200; the proto tag-100 pattern applies only +// to non-authz errors (provider/backend failures). func PlanResource( ctx context.Context, store interfaces.IaCStateStore, //nolint:revive // nil ok when no state needed (e.g. fresh deploy) @@ -36,7 +38,7 @@ func PlanResource( in *adminpb.AdminPlanInput, ) (*adminpb.AdminPlanOutput, error) { if msg := authzError(in.GetEvidence()); msg != "" { - return &adminpb.AdminPlanOutput{Error: msg}, nil + return &adminpb.AdminPlanOutput{Error: msg}, ErrAuthzDenied } if len(providers) == 0 { return &adminpb.AdminPlanOutput{Error: "plan: no iac.provider registered"}, nil diff --git a/iac/admin/handler/plan_resource_test.go b/iac/admin/handler/plan_resource_test.go index 3f94d017..090c7b17 100644 --- a/iac/admin/handler/plan_resource_test.go +++ b/iac/admin/handler/plan_resource_test.go @@ -2,18 +2,18 @@ package handler_test import ( "context" + "errors" "testing" "github.com/GoCodeAlone/workflow/iac/admin/handler" adminpb "github.com/GoCodeAlone/workflow/iac/admin/proto" - "github.com/GoCodeAlone/workflow/iac/stubprovider" "github.com/GoCodeAlone/workflow/interfaces" ) // TestPlanResource_DefaultDeny asserts that evidence with checked=false // returns a non-empty error and no plan payload. func TestPlanResource_DefaultDeny(t *testing.T) { - prov := stubprovider.New() + prov := &planningProvider{} providers := map[string]interfaces.IaCProvider{"stub": prov} desired := []interfaces.ResourceSpec{ {Name: "vpc1", Type: "infra.vpc"}, @@ -22,8 +22,8 @@ func TestPlanResource_DefaultDeny(t *testing.T) { Evidence: &adminpb.AdminAuthzEvidence{AuthzChecked: false}, } out, err := handler.PlanResource(context.Background(), nil, providers, nil, desired, in) - if err != nil { - t.Fatalf("PlanResource: unexpected error: %v", err) + if !errors.Is(err, handler.ErrAuthzDenied) { + t.Fatalf("PlanResource: want ErrAuthzDenied, got %v (out.Error=%s)", err, out.GetError()) } if out.Error == "" { t.Error("PlanResource with evidence.checked=false should return non-empty error") @@ -36,7 +36,7 @@ func TestPlanResource_DefaultDeny(t *testing.T) { // TestPlanResource_ReturnsActions asserts that a valid evidence returns // a plan_id, non-empty desired_hash, and at least one action. func TestPlanResource_ReturnsActions(t *testing.T) { - prov := stubprovider.New() + prov := &planningProvider{} providers := map[string]interfaces.IaCProvider{"stub": prov} desired := []interfaces.ResourceSpec{ {Name: "vpc1", Type: "infra.vpc", Config: map[string]any{"region": "nyc1"}}, @@ -91,7 +91,7 @@ func TestPlanResource_WithCurrentState(t *testing.T) { {Name: "vpc1", Type: "infra.vpc", ProviderID: "do-vpc-111"}, }, } - prov := stubprovider.New() + prov := &planningProvider{} providers := map[string]interfaces.IaCProvider{"stub": prov} desired := []interfaces.ResourceSpec{ {Name: "vpc1", Type: "infra.vpc"}, // already exists → update diff --git a/iac/stubprovider/provider.go b/iac/stubprovider/provider.go deleted file mode 100644 index f108cbe5..00000000 --- a/iac/stubprovider/provider.go +++ /dev/null @@ -1,208 +0,0 @@ -// Package stubprovider supplies a minimal in-process interfaces.IaCProvider -// for use in integration tests and the scenario-92 demo stack. -// -// The Provider does NOT make real cloud API calls. Every lifecycle method -// returns a deterministic, non-error result: -// -// - Plan: compares desired vs current by name; emits "create" for -// resources absent from current and "delete" for resources -// absent from desired. -// - ResourceDriver: returns a stub driver whose Create/Update/Delete all -// succeed, enabling wfctlhelpers.ApplyPlanWithHooks to run -// end-to-end without any external plugin subprocess. -// - Destroy: returns all supplied refs as Destroyed names (no-op). -// - DetectDrift: returns Drifted:false for every ref. -// -// This package imports only interfaces — no new import cycles. -package stubprovider - -import ( - "context" - - "github.com/GoCodeAlone/workflow/interfaces" -) - -// Provider is the exported stub IaCProvider. Use New() to obtain an instance. -type Provider struct{} - -// Compile-time conformance check. -var _ interfaces.IaCProvider = (*Provider)(nil) - -// New returns an initialized stub Provider. -func New() *Provider { return &Provider{} } - -// Name returns the stub provider identifier. -func (p *Provider) Name() string { return "stub" } - -// Version returns the stub provider version. -func (p *Provider) Version() string { return "0.0.0-stub" } - -// Initialize is a no-op for the stub. -func (p *Provider) Initialize(_ context.Context, _ map[string]any) error { return nil } - -// Capabilities returns nil — the stub does not declare optional capabilities. -func (p *Provider) Capabilities() []interfaces.IaCCapabilityDeclaration { return nil } - -// Plan compares desired specs against current state by name and returns -// a plan with "create" for each new resource and "delete" for each -// resource present in current but absent from desired. Resources present -// in both are returned as "update" actions (no-op at apply time; the -// stub driver's Update returns success). -func (p *Provider) Plan(_ context.Context, desired []interfaces.ResourceSpec, current []interfaces.ResourceState) (*interfaces.IaCPlan, error) { - currentByName := make(map[string]*interfaces.ResourceState, len(current)) - for i := range current { - currentByName[current[i].Name] = ¤t[i] - } - - desiredByName := make(map[string]struct{}, len(desired)) - for _, s := range desired { - desiredByName[s.Name] = struct{}{} - } - - plan := &interfaces.IaCPlan{} - - for _, spec := range desired { - if _, exists := currentByName[spec.Name]; exists { - plan.Actions = append(plan.Actions, interfaces.PlanAction{ - Action: "update", - Resource: spec, - Current: currentByName[spec.Name], - }) - } else { - plan.Actions = append(plan.Actions, interfaces.PlanAction{ - Action: "create", - Resource: spec, - }) - } - } - - for i := range current { - st := ¤t[i] - if _, wanted := desiredByName[st.Name]; !wanted { - plan.Actions = append(plan.Actions, interfaces.PlanAction{ - Action: "delete", - Resource: interfaces.ResourceSpec{Name: st.Name, Type: st.Type}, - Current: st, - }) - } - } - - return plan, nil -} - -// Destroy returns all supplied refs as Destroyed names. -func (p *Provider) Destroy(_ context.Context, refs []interfaces.ResourceRef) (*interfaces.DestroyResult, error) { - destroyed := make([]string, 0, len(refs)) - for _, r := range refs { - destroyed = append(destroyed, r.Name) - } - return &interfaces.DestroyResult{Destroyed: destroyed}, nil -} - -// Status returns nil — the stub does not probe live cloud status. -func (p *Provider) Status(_ context.Context, _ []interfaces.ResourceRef) ([]interfaces.ResourceStatus, error) { - return nil, nil -} - -// DetectDrift returns Drifted:false with DriftClassInSync for every ref. -func (p *Provider) DetectDrift(_ context.Context, refs []interfaces.ResourceRef) ([]interfaces.DriftResult, error) { - results := make([]interfaces.DriftResult, 0, len(refs)) - for _, r := range refs { - results = append(results, interfaces.DriftResult{ - Name: r.Name, - Type: r.Type, - Drifted: false, - Class: interfaces.DriftClassInSync, - }) - } - return results, nil -} - -// Import returns nil — the stub does not support resource import. -func (p *Provider) Import(_ context.Context, _ string, _ string) (*interfaces.ResourceState, error) { - return nil, nil -} - -// ResolveSizing returns nil — the stub does not resolve sizing. -func (p *Provider) ResolveSizing(_ string, _ interfaces.Size, _ *interfaces.ResourceHints) (*interfaces.ProviderSizing, error) { - return nil, nil -} - -// ResourceDriver returns a stub driver for any resource type. -func (p *Provider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { - return &stubDriver{}, nil -} - -// SupportedCanonicalKeys returns nil. -func (p *Provider) SupportedCanonicalKeys() []string { return nil } - -// BootstrapStateBackend returns nil — the stub does not manage state backends. -func (p *Provider) BootstrapStateBackend(_ context.Context, _ map[string]any) (*interfaces.BootstrapResult, error) { - return nil, nil -} - -// Close is a no-op. -func (p *Provider) Close() error { return nil } - -// stubDriver is an in-process ResourceDriver whose lifecycle methods all -// return success with a minimal ResourceOutput. -type stubDriver struct{} - -// Compile-time conformance check. -var _ interfaces.ResourceDriver = (*stubDriver)(nil) - -// Create returns a ResourceOutput with the spec's name and type. -func (d *stubDriver) Create(_ context.Context, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { - return &interfaces.ResourceOutput{ - Name: spec.Name, - Type: spec.Type, - ProviderID: "stub-" + spec.Name, - }, nil -} - -// Read returns a ResourceOutput with the ref's name and type. -func (d *stubDriver) Read(_ context.Context, ref interfaces.ResourceRef) (*interfaces.ResourceOutput, error) { - return &interfaces.ResourceOutput{ - Name: ref.Name, - Type: ref.Type, - ProviderID: ref.ProviderID, - }, nil -} - -// Update returns a ResourceOutput with the spec's name and type. -func (d *stubDriver) Update(_ context.Context, ref interfaces.ResourceRef, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { - pid := ref.ProviderID - if pid == "" { - pid = "stub-" + spec.Name - } - return &interfaces.ResourceOutput{ - Name: spec.Name, - Type: spec.Type, - ProviderID: pid, - }, nil -} - -// Delete is a no-op. -func (d *stubDriver) Delete(_ context.Context, _ interfaces.ResourceRef) error { return nil } - -// Diff returns a DiffResult indicating no changes needed. -func (d *stubDriver) Diff(_ context.Context, _ interfaces.ResourceSpec, _ *interfaces.ResourceOutput) (*interfaces.DiffResult, error) { - return &interfaces.DiffResult{ - NeedsUpdate: false, - NeedsReplace: false, - Changes: nil, - }, nil -} - -// HealthCheck returns nil — the stub does not probe health. -func (d *stubDriver) HealthCheck(_ context.Context, _ interfaces.ResourceRef) (*interfaces.HealthResult, error) { - return nil, nil -} - -// Scale returns nil — the stub does not support scaling. -func (d *stubDriver) Scale(_ context.Context, _ interfaces.ResourceRef, _ int) (*interfaces.ResourceOutput, error) { - return nil, nil -} - -// SensitiveKeys returns nil. -func (d *stubDriver) SensitiveKeys() []string { return nil } diff --git a/iac/stubprovider/provider_test.go b/iac/stubprovider/provider_test.go deleted file mode 100644 index 688deb51..00000000 --- a/iac/stubprovider/provider_test.go +++ /dev/null @@ -1,167 +0,0 @@ -// Package stubprovider_test exercises the stub IaCProvider used by -// scenario 92 and integration tests. The stub must be loadable without -// any external plugin subprocess — it runs entirely in-process. -package stubprovider_test - -import ( - "context" - "testing" - - "github.com/GoCodeAlone/workflow/iac/stubprovider" - "github.com/GoCodeAlone/workflow/iac/wfctlhelpers" - "github.com/GoCodeAlone/workflow/interfaces" -) - -// TestStub_InterfaceConformance asserts that New() returns a non-nil -// provider. The compile-time guard (var _ in provider.go) already verifies -// type satisfaction; this test catches a nil-return regression at runtime. -func TestStub_InterfaceConformance(t *testing.T) { - p := stubprovider.New() - if p == nil { - t.Fatal("New() returned nil") - } -} - -// TestStub_Plan_CreateAction asserts that Plan on a 1-spec desired set -// with no current state returns a plan with 1 "create" action. -func TestStub_Plan_CreateAction(t *testing.T) { - p := stubprovider.New() - desired := []interfaces.ResourceSpec{ - {Name: "my-vpc", Type: "infra.vpc", Config: map[string]any{"region": "nyc1"}}, - } - plan, err := p.Plan(context.Background(), desired, nil) - if err != nil { - t.Fatalf("Plan: unexpected error: %v", err) - } - if plan == nil { - t.Fatal("Plan: returned nil plan") - } - if len(plan.Actions) != 1 { - t.Fatalf("Plan: expected 1 action, got %d", len(plan.Actions)) - } - if plan.Actions[0].Action != "create" { - t.Errorf("Plan: expected action 'create', got %q", plan.Actions[0].Action) - } - if plan.Actions[0].Resource.Name != "my-vpc" { - t.Errorf("Plan: expected resource name 'my-vpc', got %q", plan.Actions[0].Resource.Name) - } -} - -// TestStub_Apply_NoErrors asserts that driving ApplyPlanWithHooks with the -// stub provider on a plan with a create action returns an ApplyResult with -// no errors. -func TestStub_Apply_NoErrors(t *testing.T) { - p := stubprovider.New() - plan := &interfaces.IaCPlan{ - ID: "test-plan", - Actions: []interfaces.PlanAction{ - {Action: "create", Resource: interfaces.ResourceSpec{Name: "my-vpc", Type: "infra.vpc"}}, - }, - } - result, err := wfctlhelpers.ApplyPlanWithHooks(context.Background(), p, plan, wfctlhelpers.ApplyPlanHooks{}) - if err != nil { - t.Fatalf("ApplyPlanWithHooks: unexpected error: %v", err) - } - if result == nil { - t.Fatal("ApplyPlanWithHooks: returned nil result") - } - if len(result.Errors) != 0 { - t.Errorf("ApplyPlanWithHooks: expected no errors, got: %v", result.Errors) - } -} - -// TestStub_Destroy_ReturnsRefs asserts that Destroy returns the refs as -// Destroyed names. -func TestStub_Destroy_ReturnsRefs(t *testing.T) { - p := stubprovider.New() - refs := []interfaces.ResourceRef{ - {Name: "my-vpc", Type: "infra.vpc", ProviderID: "do-vpc-123"}, - {Name: "my-db", Type: "infra.database", ProviderID: "do-db-456"}, - } - result, err := p.Destroy(context.Background(), refs) - if err != nil { - t.Fatalf("Destroy: unexpected error: %v", err) - } - if result == nil { - t.Fatal("Destroy: returned nil result") - } - if len(result.Destroyed) != 2 { - t.Fatalf("Destroy: expected 2 destroyed, got %d", len(result.Destroyed)) - } - names := map[string]bool{} - for _, n := range result.Destroyed { - names[n] = true - } - if !names["my-vpc"] || !names["my-db"] { - t.Errorf("Destroy: expected 'my-vpc' and 'my-db' in destroyed, got %v", result.Destroyed) - } -} - -// TestStub_DetectDrift_NotDrifted asserts that DetectDrift returns results -// with Drifted:false for all refs. -func TestStub_DetectDrift_NotDrifted(t *testing.T) { - p := stubprovider.New() - refs := []interfaces.ResourceRef{ - {Name: "my-vpc", Type: "infra.vpc"}, - {Name: "my-db", Type: "infra.database"}, - } - results, err := p.DetectDrift(context.Background(), refs) - if err != nil { - t.Fatalf("DetectDrift: unexpected error: %v", err) - } - if len(results) != 2 { - t.Fatalf("DetectDrift: expected 2 results, got %d", len(results)) - } - for _, r := range results { - if r.Drifted { - t.Errorf("DetectDrift: expected Drifted:false for %q, got true", r.Name) - } - if r.Class != interfaces.DriftClassInSync { - t.Errorf("DetectDrift: expected Class DriftClassInSync for %q, got %q", r.Name, r.Class) - } - } -} - -// TestStub_Plan_UpdateAction asserts that Plan produces an "update" action -// when a resource is present in both desired and current state. -func TestStub_Plan_UpdateAction(t *testing.T) { - p := stubprovider.New() - desired := []interfaces.ResourceSpec{ - {Name: "my-vpc", Type: "infra.vpc"}, - } - current := []interfaces.ResourceState{ - {Name: "my-vpc", Type: "infra.vpc", ProviderID: "do-vpc-111"}, - } - plan, err := p.Plan(context.Background(), desired, current) - if err != nil { - t.Fatalf("Plan: unexpected error: %v", err) - } - if len(plan.Actions) != 1 { - t.Fatalf("Plan: expected 1 action, got %d", len(plan.Actions)) - } - if plan.Actions[0].Action != "update" { - t.Errorf("Plan: expected action 'update', got %q", plan.Actions[0].Action) - } -} - -// TestStub_Plan_DeleteAction asserts that Plan produces a "delete" action -// when a resource is present in current state but absent from desired. -func TestStub_Plan_DeleteAction(t *testing.T) { - p := stubprovider.New() - current := []interfaces.ResourceState{ - {Name: "old-vpc", Type: "infra.vpc", ProviderID: "do-vpc-999"}, - } - plan, err := p.Plan(context.Background(), nil, current) - if err != nil { - t.Fatalf("Plan: unexpected error: %v", err) - } - if len(plan.Actions) != 1 { - t.Fatalf("Plan: expected 1 action, got %d", len(plan.Actions)) - } - if plan.Actions[0].Action != "delete" { - t.Errorf("Plan: expected action 'delete', got %q", plan.Actions[0].Action) - } - if plan.Actions[0].Resource.Name != "old-vpc" { - t.Errorf("Plan: expected resource name 'old-vpc', got %q", plan.Actions[0].Resource.Name) - } -} diff --git a/module/infra_admin.go b/module/infra_admin.go index aec7cdc9..5240a595 100644 --- a/module/infra_admin.go +++ b/module/infra_admin.go @@ -64,6 +64,14 @@ import ( "google.golang.org/protobuf/proto" ) +// outputError is a type constraint satisfied by all admin output proto +// messages — they all embed an Error string field. Used by +// writeMutationResponse to inspect the output's error without a type switch. +type outputError interface { + proto.Message + GetError() string +} + // InfraAdminConfig is the YAML-config shape the host expects under // the `infra.admin` module entry. Field names use snake_case yaml // tags to match the rest of the workflow config; defaults match the @@ -655,7 +663,7 @@ func (m *InfraAdmin) Start(ctx context.Context) error { "category": "infra", "path": m.config.AssetPrefix + "/resources.html", "render_mode": "iframe", - "permissions": []map[string]any{{ + "permissions": []any{map[string]any{ "resource": "infra", "action": "read", "permission": "infra:read", }}, }, @@ -668,7 +676,7 @@ func (m *InfraAdmin) Start(ctx context.Context) error { "category": "infra", "path": m.config.AssetPrefix + "/resource.html", "render_mode": "iframe", - "permissions": []map[string]any{{ + "permissions": []any{map[string]any{ "resource": "infra", "action": "read", "permission": "infra:read", }}, }, @@ -681,7 +689,7 @@ func (m *InfraAdmin) Start(ctx context.Context) error { "category": "infra", "path": m.config.AssetPrefix + "/new.html", "render_mode": "iframe", - "permissions": []map[string]any{{ + "permissions": []any{map[string]any{ "resource": "infra", "action": "read", "permission": "infra:read", }}, }, @@ -696,7 +704,7 @@ func (m *InfraAdmin) Start(ctx context.Context) error { "category": "infra", "path": m.config.AssetPrefix + "/actions.html", "render_mode": "iframe", - "permissions": []map[string]any{{ + "permissions": []any{map[string]any{ "resource": "infra", "action": "read", "permission": "infra:read", }}, }, @@ -788,15 +796,14 @@ func (m *InfraAdmin) auditAccess(r *http.Request, action string, ev *adminpb.Adm _ = r // r reserved for future targets/app_context extraction } -// auditResultFor maps a handler output's Error field to the -// audit log's `result` value. Empty error → "ok"; non-empty → -// "denied" (the handler library's primary refusal path is -// authz-default-deny, so "denied" is the most informative -// auditResultFor classifies an Output.error string into the +// auditResultFor classifies a read-handler output's Error field into the // three-way audit result: "ok" (no error), "denied" (authz/evidence -// rejection), or "error" (provider/backend failure). The -// discrimination is substring-based on the error prefix, which the -// handler library follows consistently. +// rejection), or "error" (provider/backend failure). Read handlers +// (ListResources, GetResource, etc.) return (output, nil) even on authz +// denial, so the only signal available is the error string. The +// discrimination is substring-based — acceptable here because read +// handlers do not call provider APIs that could return "denied" text. +// Mutation handlers MUST use auditResultFromErr (typed sentinel). func auditResultFor(errMsg string) string { if errMsg == "" { return "ok" @@ -812,6 +819,27 @@ func auditResultFor(errMsg string) string { return "error" } +// auditResultFromErr classifies a mutation handler's outcome into the +// three-way audit result using the TYPED handler error — NOT +// strings.Contains. This eliminates the false-positive where a provider +// error message containing "denied" (e.g. "provider: access denied to +// cloud API") would be mis-logged as result:"denied" by the substring path. +// +// Classification: +// +// errors.Is(err, handler.ErrAuthzDenied) → "denied" +// outError != "" → "error" +// (both empty/nil) → "ok" +func auditResultFromErr(err error, outError string) string { + if errors.Is(err, handler.ErrAuthzDenied) { + return "denied" + } + if outError != "" { + return "error" + } + return "ok" +} + // nowUnix is a package-level var so tests can substitute a fixed // clock without touching time. Default is defaultNowUnix (declared // in infra_admin_clock.go) → time.Now().UTC().Unix(). @@ -1037,6 +1065,41 @@ func writeProtoMsg(w http.ResponseWriter, msg proto.Message) { _, _ = w.Write(data) } +// writeStatusProto marshals a proto message and writes it with the given +// HTTP status code. On marshal failure, falls back to plain-text 500. +func writeStatusProto(w http.ResponseWriter, status int, msg proto.Message) { + data, err := marshalOpts.Marshal(msg) + if err != nil { + http.Error(w, "marshal response: "+err.Error(), http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _, _ = w.Write(data) +} + +// writeMutationResponse writes a mutation handler's output using a typed +// HTTP status discriminator: +// +// - handler.ErrAuthzDenied → HTTP 403 (typed sentinel; avoids strings.Contains +// false-positives when a provider error message happens to contain "denied") +// - non-empty Output.error (provider / backend failure) → HTTP 500 +// - success → HTTP 200 (via writeProtoMsg) +// +// Using this for plan/apply/destroy replaces the naive writeProtoMsg(w, out) +// pattern that silently returned 200 for all outcomes (Bug 3 + Bug 4 fix). +func writeMutationResponse(w http.ResponseWriter, msg outputError, err error) { + if errors.Is(err, handler.ErrAuthzDenied) { + writeStatusProto(w, http.StatusForbidden, msg) + return + } + if msg.GetError() != "" { + writeStatusProto(w, http.StatusInternalServerError, msg) + return + } + writeProtoMsg(w, msg) +} + // ── T8: requireBearerAuth middleware ───────────────────────────────────────── // requireBearerAuthMiddleware is an HTTPMiddleware that rejects requests @@ -1094,10 +1157,30 @@ func (m *InfraAdmin) handlePlanResource(w http.ResponseWriter, r *http.Request) return } } - // handlers never return non-nil error (errors go to out.Error per proto tag-100 pattern) - out, _ := handler.PlanResource(r.Context(), m.state, m.providers, m.wfCfg, m.desiredSpecs, &in) //nolint:errcheck - writeProtoMsg(w, out) - m.auditAccess(r, "plan", in.GetEvidence(), auditResultFor(out.GetError())) + // Server-side RBAC: plan is infra:apply-gated — viewers must not be + // able to probe the desired-state hash or action list (Bug 4 fix). + subject := m.subjectFromRequest(r) + if m.authz != nil { + ok, enforceErr := m.authz.Enforce(subject, "infra:apply", "allow") + if enforceErr != nil { + // Route through writeStatusProto so the 500 body is proto-JSON, + // consistent with all other mutation error responses (Finding 1). + writeStatusProto(w, http.StatusInternalServerError, &adminpb.AdminPlanOutput{Error: "plan: authz enforce error"}) + m.auditAccess(r, "plan", in.GetEvidence(), "error") + return + } + if !ok { + // Generic denial — do NOT reflect the authenticated subject in the + // response body (Finding 2). Subject is captured in the audit log + // separately. Route through writeMutationResponse for proto-JSON body. + writeMutationResponse(w, &adminpb.AdminPlanOutput{Error: "plan: infra:apply denied"}, handler.ErrAuthzDenied) + m.auditAccess(r, "plan", in.GetEvidence(), "denied") + return + } + } + out, handlerErr := handler.PlanResource(r.Context(), m.state, m.providers, m.wfCfg, m.desiredSpecs, &in) + writeMutationResponse(w, out, handlerErr) + m.auditAccess(r, "plan", in.GetEvidence(), auditResultFromErr(handlerErr, out.GetError())) } func (m *InfraAdmin) handleApplyResource(w http.ResponseWriter, r *http.Request) { @@ -1120,9 +1203,9 @@ func (m *InfraAdmin) handleApplyResource(w http.ResponseWriter, r *http.Request) } } subject := m.subjectFromRequest(r) - out, _ := handler.ApplyResource(r.Context(), m.state, m.providers, m.authz, subject, m.wfCfg, m.desiredSpecs, &in) //nolint:errcheck // errors go to out.Error - writeProtoMsg(w, out) - m.auditAccess(r, "apply", in.GetEvidence(), auditResultFor(out.GetError())) + out, handlerErr := handler.ApplyResource(r.Context(), m.state, m.providers, m.authz, subject, m.wfCfg, m.desiredSpecs, &in) + writeMutationResponse(w, out, handlerErr) + m.auditAccess(r, "apply", in.GetEvidence(), auditResultFromErr(handlerErr, out.GetError())) } func (m *InfraAdmin) handleDestroyResource(w http.ResponseWriter, r *http.Request) { @@ -1145,9 +1228,9 @@ func (m *InfraAdmin) handleDestroyResource(w http.ResponseWriter, r *http.Reques } } subject := m.subjectFromRequest(r) - out, _ := handler.DestroyResource(r.Context(), m.providers, m.authz, subject, &in) //nolint:errcheck // errors go to out.Error - writeProtoMsg(w, out) - m.auditAccess(r, "destroy", in.GetEvidence(), auditResultFor(out.GetError())) + out, handlerErr := handler.DestroyResource(r.Context(), m.providers, m.authz, subject, &in) + writeMutationResponse(w, out, handlerErr) + m.auditAccess(r, "destroy", in.GetEvidence(), auditResultFromErr(handlerErr, out.GetError())) } func (m *InfraAdmin) handleDriftCheckResource(w http.ResponseWriter, r *http.Request) { diff --git a/module/infra_admin_mutation_integration_test.go b/module/infra_admin_mutation_integration_test.go index affc254e..d56252d1 100644 --- a/module/infra_admin_mutation_integration_test.go +++ b/module/infra_admin_mutation_integration_test.go @@ -4,7 +4,7 @@ package module // BuildFromConfig (per ADR-0003→v4 lesson that BuildFromConfig makes test // setup brittle — manual wiring is explicit about what's under test). // -// Wiring: auth stub + recordingStateStore + stubprovider.New() (T2) + +// Wiring: auth stub + recordingStateStore + testMutationProvider (T2) + // stubEnforcer (I-2) + infra.admin module + audit log. // // Assertions per T10 spec: @@ -31,7 +31,6 @@ import ( "github.com/GoCodeAlone/workflow/config" "github.com/GoCodeAlone/workflow/iac/admin/handler" adminpb "github.com/GoCodeAlone/workflow/iac/admin/proto" - "github.com/GoCodeAlone/workflow/iac/stubprovider" "github.com/GoCodeAlone/workflow/interfaces" "google.golang.org/protobuf/encoding/protojson" ) @@ -109,7 +108,7 @@ func (e *integrationEnforcer) Enforce(_, _, _ string, _ ...string) (bool, error) // mutationIntegrationApp manually wires the infra.admin module with: // - auth stub + authz stub enforcer (I-2) // - recordingStateStore (C-1 assertion) -// - stub iac.provider (T2: stubprovider.New()) +// - testMutationProvider (replaces the deleted iac/stubprovider; T2) // - WorkflowConfig with one infra.database resource (C-2: non-empty desiredSpecs) func mutationIntegrationApp(t *testing.T, auditPath string) (*withConfigSectionApp, *InfraAdmin, *recordingStateStore, *integrationEnforcer) { t.Helper() @@ -138,7 +137,7 @@ func mutationIntegrationApp(t *testing.T, auditPath string) (*withConfigSectionA mustRegister(t, app, "my-authz", enforcer) // Stub provider registered under its module name (per T2). - mustRegister(t, app, providerName, stubprovider.New()) + mustRegister(t, app, providerName, &testMutationProvider{}) // Recording state store so we can assert assertion (1). store := &recordingStateStore{} @@ -370,5 +369,114 @@ func mustMarshal(t *testing.T, v any) []byte { return data } -// Compile-time: interfaces.IaCProvider satisfied by stubprovider.Provider. -var _ interfaces.IaCProvider = (*stubprovider.Provider)(nil) +// testMutationProvider is an in-package IaCProvider for T10 integration tests. +// It replaces the deleted iac/stubprovider package — scenario test fixtures must +// not live in the workflow engine core. Provides real Plan, Destroy, and +// ResourceDriver so the integration path exercises apply+state persistence. +type testMutationProvider struct{} + +var _ interfaces.IaCProvider = (*testMutationProvider)(nil) + +func (p *testMutationProvider) Name() string { return "test-mutation" } +func (p *testMutationProvider) Version() string { return "0.0.0-test" } +func (p *testMutationProvider) Initialize(_ context.Context, _ map[string]any) error { return nil } +func (p *testMutationProvider) Capabilities() []interfaces.IaCCapabilityDeclaration { return nil } + +func (p *testMutationProvider) Plan(_ context.Context, desired []interfaces.ResourceSpec, current []interfaces.ResourceState) (*interfaces.IaCPlan, error) { + currentByName := make(map[string]*interfaces.ResourceState, len(current)) + for i := range current { + currentByName[current[i].Name] = ¤t[i] + } + desiredByName := make(map[string]struct{}, len(desired)) + for _, s := range desired { + desiredByName[s.Name] = struct{}{} + } + plan := &interfaces.IaCPlan{} + for _, spec := range desired { + if _, exists := currentByName[spec.Name]; exists { + plan.Actions = append(plan.Actions, interfaces.PlanAction{Action: "update", Resource: spec, Current: currentByName[spec.Name]}) + } else { + plan.Actions = append(plan.Actions, interfaces.PlanAction{Action: "create", Resource: spec}) + } + } + for i := range current { + st := ¤t[i] + if _, wanted := desiredByName[st.Name]; !wanted { + plan.Actions = append(plan.Actions, interfaces.PlanAction{Action: "delete", Resource: interfaces.ResourceSpec{Name: st.Name, Type: st.Type}, Current: st}) + } + } + return plan, nil +} + +func (p *testMutationProvider) Destroy(_ context.Context, refs []interfaces.ResourceRef) (*interfaces.DestroyResult, error) { + names := make([]string, 0, len(refs)) + for _, r := range refs { + names = append(names, r.Name) + } + return &interfaces.DestroyResult{Destroyed: names}, nil +} + +func (p *testMutationProvider) Status(_ context.Context, _ []interfaces.ResourceRef) ([]interfaces.ResourceStatus, error) { + return nil, nil +} + +func (p *testMutationProvider) DetectDrift(_ context.Context, _ []interfaces.ResourceRef) ([]interfaces.DriftResult, error) { + return nil, nil +} + +func (p *testMutationProvider) Import(_ context.Context, _ string, _ string) (*interfaces.ResourceState, error) { + return nil, nil +} + +func (p *testMutationProvider) ResolveSizing(_ string, _ interfaces.Size, _ *interfaces.ResourceHints) (*interfaces.ProviderSizing, error) { + return nil, nil +} + +func (p *testMutationProvider) ResourceDriver(_ string) (interfaces.ResourceDriver, error) { + return &testMutationDriver{}, nil +} + +func (p *testMutationProvider) SupportedCanonicalKeys() []string { return nil } + +func (p *testMutationProvider) BootstrapStateBackend(_ context.Context, _ map[string]any) (*interfaces.BootstrapResult, error) { + return nil, nil +} + +func (p *testMutationProvider) Close() error { return nil } + +// testMutationDriver is a minimal ResourceDriver for T10 integration tests. +type testMutationDriver struct{} + +var _ interfaces.ResourceDriver = (*testMutationDriver)(nil) + +func (d *testMutationDriver) Create(_ context.Context, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + return &interfaces.ResourceOutput{Name: spec.Name, Type: spec.Type, ProviderID: "test-" + spec.Name}, nil +} + +func (d *testMutationDriver) Read(_ context.Context, ref interfaces.ResourceRef) (*interfaces.ResourceOutput, error) { + return &interfaces.ResourceOutput{Name: ref.Name, Type: ref.Type, ProviderID: ref.ProviderID}, nil +} + +func (d *testMutationDriver) Update(_ context.Context, ref interfaces.ResourceRef, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + pid := ref.ProviderID + if pid == "" { + pid = "test-" + spec.Name + } + return &interfaces.ResourceOutput{Name: spec.Name, Type: spec.Type, ProviderID: pid}, nil +} + +func (d *testMutationDriver) Delete(_ context.Context, _ interfaces.ResourceRef) error { return nil } + +func (d *testMutationDriver) Diff(_ context.Context, _ interfaces.ResourceSpec, _ *interfaces.ResourceOutput) (*interfaces.DiffResult, error) { + return &interfaces.DiffResult{NeedsUpdate: false, NeedsReplace: false}, nil +} + +func (d *testMutationDriver) HealthCheck(_ context.Context, _ interfaces.ResourceRef) (*interfaces.HealthResult, error) { + return nil, nil +} + +func (d *testMutationDriver) Scale(_ context.Context, _ interfaces.ResourceRef, _ int) (*interfaces.ResourceOutput, error) { + return nil, nil +} + +func (d *testMutationDriver) SensitiveKeys() []string { return nil } diff --git a/module/infra_admin_test.go b/module/infra_admin_test.go index bf2f33d7..16bb3a9a 100644 --- a/module/infra_admin_test.go +++ b/module/infra_admin_test.go @@ -17,6 +17,7 @@ import ( "github.com/GoCodeAlone/modular" "github.com/GoCodeAlone/workflow/config" + "github.com/GoCodeAlone/workflow/iac/admin/handler" "github.com/GoCodeAlone/workflow/interfaces" "google.golang.org/protobuf/encoding/protojson" @@ -1191,7 +1192,7 @@ func TestInfraAdmin_AuditResultFor3Way(t *testing.T) { }{ {"", "ok"}, {"authz evidence missing — admin middleware did not attach", "denied"}, - {"apply: infra:apply denied for subject viewer", "denied"}, + {"apply: infra:apply denied", "denied"}, {"apply: plan is stale (desired_hash mismatch)", "denied"}, {"apply: list state: connection refused", "error"}, {"plan: no iac.provider registered", "error"}, @@ -1237,8 +1238,10 @@ func TestInfraAdmin_ApplyRejectsStalePlanHash(t *testing.T) { rec := httptest.NewRecorder() m.router.ServeHTTP(rec, req) - if rec.Code != http.StatusOK { - t.Fatalf("unexpected status %d; body=%s", rec.Code, rec.Body.String()) + // Stale hash is a provider/backend error → HTTP 500 (Bug 3 fix: writeMutationResponse + // maps non-authz output.Error → 500 so provider-error("denied" text) ≠ 403). + if rec.Code != http.StatusInternalServerError { + t.Fatalf("stale hash: want 500 (non-authz output.Error), got %d; body=%s", rec.Code, rec.Body.String()) } if !strings.Contains(rec.Body.String(), "stale") { t.Errorf("expected stale-hash error in response, got: %s", rec.Body.String()) @@ -1317,14 +1320,14 @@ func TestInfraAdmin_ViewerCannotApply(t *testing.T) { return r.WithContext(ctx) } - // Apply: client claims allowed, server Enforcer denies. + // Apply: client claims allowed, server Enforcer denies → HTTP 403 (Bug 3 fix). applyReq := viewerCtx(httptest.NewRequest(http.MethodPost, "/api/infra-admin/apply", bytes.NewReader([]byte(`{"evidence":{"authz_checked":true,"authz_allowed":true},"desired_hash":"any"}`)))) applyReq.Header.Set("Authorization", "Bearer test-token") applyRec := httptest.NewRecorder() m.router.ServeHTTP(applyRec, applyReq) - if applyRec.Code != http.StatusOK { - t.Fatalf("apply: want 200 with error body, got %d", applyRec.Code) + if applyRec.Code != http.StatusForbidden { + t.Fatalf("apply: want 403 (typed ErrAuthzDenied), got %d; body=%s", applyRec.Code, applyRec.Body.String()) } if !strings.Contains(applyRec.Body.String(), "denied") { t.Errorf("viewer apply should be denied by server-side Enforcer; body=%s", applyRec.Body.String()) @@ -1336,14 +1339,170 @@ func TestInfraAdmin_ViewerCannotApply(t *testing.T) { destroyReq.Header.Set("Authorization", "Bearer test-token") destroyRec := httptest.NewRecorder() m.router.ServeHTTP(destroyRec, destroyReq) - if destroyRec.Code != http.StatusOK { - t.Fatalf("destroy: want 200 with error body, got %d", destroyRec.Code) + if destroyRec.Code != http.StatusForbidden { + t.Fatalf("destroy: want 403 (typed ErrAuthzDenied), got %d; body=%s", destroyRec.Code, destroyRec.Body.String()) } if !strings.Contains(destroyRec.Body.String(), "denied") { t.Errorf("viewer destroy should be denied by server-side Enforcer; body=%s", destroyRec.Body.String()) } } +// deniedPlanProvider wraps infraMockProvider and overrides Plan to return +// an error whose message contains "denied" — used by the provider-error→500 +// discriminator test to verify that strings.Contains("denied") is NOT used +// for HTTP status classification (typed ErrAuthzDenied sentinel instead). +type deniedPlanProvider struct { + infraMockProvider +} + +func (p *deniedPlanProvider) Plan(_ context.Context, _ []interfaces.ResourceSpec, _ []interfaces.ResourceState) (*interfaces.IaCPlan, error) { + return nil, errors.New("provider: access denied to cloud API") +} + +// TestInfraAdmin_TypedAuthzDenied_Returns403 pins the 4 typed HTTP-status +// discriminator behaviors introduced by Bug 3 + Bug 4: +// +// viewer→/plan=403 (handlePlanResource Enforcer gate, Bug 4) +// viewer→/apply=403 (handler.ErrAuthzDenied → writeMutationResponse, Bug 3) +// operator→/apply=200 (happy path unaffected) +// provider-error("denied" text)→500 NOT 403 (strings.Contains FP eliminated) +func TestInfraAdmin_TypedAuthzDenied_Returns403(t *testing.T) { + // subjectCtx injects a JWT "sub" claim into the request context + // so subjectFromRequest() extracts the right principal. + subjectCtx := func(r *http.Request, sub string) *http.Request { + ctx := context.WithValue(r.Context(), authClaimsContextKey, map[string]any{"sub": sub}) + return r.WithContext(ctx) + } + + // startWithConfig boots an InfraAdmin with auth+authz+provider registered. + startWithConfig := func(t *testing.T, enforcer Enforcer, prov interfaces.IaCProvider) *InfraAdmin { + t.Helper() + app, _, _, _ := newAuthEnabledApp(t, "digitalocean") + if enforcer != nil { + if err := app.RegisterService("my-authz", enforcer); err != nil { + t.Fatalf("setup: RegisterService(my-authz): %v", err) + } + } + if prov != nil { + // Override the default do-provider so the module uses our custom one. + if err := app.RegisterService("do-provider", prov); err != nil { + t.Fatalf("setup: RegisterService(do-provider): %v", err) + } + } + cfg := standardAuthCfg() + if enforcer != nil { + cfg.AuthzModule = "my-authz" + } + m := NewInfraAdmin("infra-admin", configToMap(t, cfg)).(*InfraAdmin) + if err := m.Init(app); err != nil { + t.Fatalf("Init: %v", err) + } + if err := m.Start(context.Background()); err != nil { + t.Fatalf("Start: %v", err) + } + if err := m.router.Start(context.Background()); err != nil { + t.Fatalf("router.Start: %v", err) + } + return m + } + + // ── viewer→/plan=403 ───────────────────────────────────────────────────── + // handlePlanResource now calls m.authz.Enforce before handler.PlanResource + // (Bug 4 fix). Viewer subject denied → HTTP 403 directly from module layer. + // Additional assertions (Copilot findings): + // - Body is proto-JSON AdminPlanOutput (not plaintext) — consistent with apply/destroy 403s. + // - Body does NOT contain the subject string "viewer" — no principal leak. + t.Run("viewer/plan=403", func(t *testing.T) { + enforcer := &stubEnforcer{allowed: false} + m := startWithConfig(t, enforcer, nil) + + req := subjectCtx(httptest.NewRequest(http.MethodPost, "/api/infra-admin/plan", + bytes.NewReader([]byte(`{"evidence":{"authz_checked":true,"authz_allowed":true}}`))), "viewer") + req.Header.Set("Authorization", "Bearer test-token") + rec := httptest.NewRecorder() + m.router.ServeHTTP(rec, req) + if rec.Code != http.StatusForbidden { + t.Errorf("viewer/plan: want 403, got %d; body=%s", rec.Code, rec.Body.String()) + } + // Body must be proto-JSON AdminPlanOutput, not plaintext (Copilot finding 1+2). + if ct := rec.Header().Get("Content-Type"); ct != "application/json" { + t.Errorf("viewer/plan 403: Content-Type = %q, want application/json (proto-JSON body)", ct) + } + var planOut adminpb.AdminPlanOutput + if err := protojson.Unmarshal(rec.Body.Bytes(), &planOut); err != nil { + t.Errorf("viewer/plan 403: body is not valid AdminPlanOutput JSON: %v\n%s", err, rec.Body.String()) + } + // Subject must NOT appear in response body — no principal leakage. + if strings.Contains(rec.Body.String(), "viewer") { + t.Errorf("viewer/plan 403: body contains subject 'viewer' — principal leak; body=%s", rec.Body.String()) + } + }) + + // ── viewer→/apply=403 ──────────────────────────────────────────────────── + // handler.ApplyResource Gate 2 returns ErrAuthzDenied → writeMutationResponse → 403. + t.Run("viewer/apply=403", func(t *testing.T) { + enforcer := &stubEnforcer{allowed: false} + m := startWithConfig(t, enforcer, nil) + + // Compute correct desired_hash for empty desiredSpecs + empty state. + hash := handler.DesiredHash(nil, nil, nil) + body := `{"desired_hash":"` + hash + `","evidence":{"authz_checked":true,"authz_allowed":true}}` + req := subjectCtx(httptest.NewRequest(http.MethodPost, "/api/infra-admin/apply", + bytes.NewReader([]byte(body))), "viewer") + req.Header.Set("Authorization", "Bearer test-token") + rec := httptest.NewRecorder() + m.router.ServeHTTP(rec, req) + if rec.Code != http.StatusForbidden { + t.Errorf("viewer/apply: want 403, got %d; body=%s", rec.Code, rec.Body.String()) + } + }) + + // ── operator→/apply=200 ────────────────────────────────────────────────── + // operator is allowed → plan succeeds (empty desired specs) → apply succeeds → 200. + t.Run("operator/apply=200", func(t *testing.T) { + enforcer := &stubEnforcer{allowed: true} + m := startWithConfig(t, enforcer, nil) + + hash := handler.DesiredHash(nil, nil, nil) + body := `{"desired_hash":"` + hash + `","evidence":{"authz_checked":true,"authz_allowed":true}}` + req := subjectCtx(httptest.NewRequest(http.MethodPost, "/api/infra-admin/apply", + bytes.NewReader([]byte(body))), "operator") + req.Header.Set("Authorization", "Bearer test-token") + rec := httptest.NewRecorder() + m.router.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Errorf("operator/apply: want 200, got %d; body=%s", rec.Code, rec.Body.String()) + } + }) + + // ── provider-error("denied" text)→500 NOT 403 ─────────────────────────── + // The provider's Plan returns an error whose message contains "denied". + // writeMutationResponse must use errors.Is(err, ErrAuthzDenied) — NOT + // strings.Contains — so this maps to 500 (backend error), NOT 403 (authz). + t.Run("provider-error-denied-text/apply=500-not-403", func(t *testing.T) { + enforcer := &stubEnforcer{allowed: true} + prov := &deniedPlanProvider{infraMockProvider: infraMockProvider{name: "digitalocean"}} + m := startWithConfig(t, enforcer, prov) + + hash := handler.DesiredHash(nil, nil, nil) + body := `{"desired_hash":"` + hash + `","evidence":{"authz_checked":true,"authz_allowed":true}}` + req := subjectCtx(httptest.NewRequest(http.MethodPost, "/api/infra-admin/apply", + bytes.NewReader([]byte(body))), "operator") + req.Header.Set("Authorization", "Bearer test-token") + rec := httptest.NewRecorder() + m.router.ServeHTTP(rec, req) + if rec.Code == http.StatusForbidden { + t.Errorf("provider-error with 'denied' text: got 403 (strings.Contains FP!) want 500; body=%s", rec.Body.String()) + } + if rec.Code != http.StatusInternalServerError { + t.Errorf("provider-error with 'denied' text: want 500, got %d; body=%s", rec.Code, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), "denied") { + t.Errorf("provider-error body should contain 'denied' (provider error text); got: %s", rec.Body.String()) + } + }) +} + // TestInfraAdmin_AuditDistinguishesDeniedFromError verifies that the // 3-way audit classification correctly distinguishes authz denials from // backend errors (extended from T8's TestInfraAdmin_AuditResultFor3Way). @@ -1351,7 +1510,7 @@ func TestInfraAdmin_AuditDistinguishesDeniedFromError(t *testing.T) { // Denial (authz/evidence/stale markers) → "denied" for _, msg := range []string{ "authz evidence missing", - "infra:apply denied for subject viewer", + "infra:apply denied", "plan is stale (desired_hash mismatch)", } { if got := auditResultFor(msg); got != "denied" { @@ -1369,3 +1528,34 @@ func TestInfraAdmin_AuditDistinguishesDeniedFromError(t *testing.T) { } } } + +// TestInfraAdmin_AuditResultFromErr pins auditResultFromErr — the typed +// mutation-route classifier that replaces strings.Contains in the audit +// path. The key regression: a provider error whose message contains +// "denied" must log as "error", NOT "denied". +func TestInfraAdmin_AuditResultFromErr(t *testing.T) { + cases := []struct { + name string + err error + outErr string + want string + }{ + {"success", nil, "", "ok"}, + {"authz sentinel", handler.ErrAuthzDenied, "apply: infra:apply denied", "denied"}, + // The critical false-positive regression: provider error containing + // "denied" must NOT be classified as "denied" (strings.Contains would + // have done so). Only errors.Is(ErrAuthzDenied) triggers "denied". + {"provider error with denied text", nil, "apply: plan: provider: access denied to cloud API", "error"}, + {"stale hash", nil, "apply: plan is stale (desired_hash mismatch)", "error"}, + {"no provider registered", nil, "plan: no iac.provider registered", "error"}, + {"evidence denial via sentinel", handler.ErrAuthzDenied, "authz evidence missing", "denied"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := auditResultFromErr(tc.err, tc.outErr) + if got != tc.want { + t.Errorf("auditResultFromErr(%v, %q) = %q, want %q", tc.err, tc.outErr, got, tc.want) + } + }) + } +} diff --git a/plugins/all/all.go b/plugins/all/all.go index 512325e5..722f011a 100644 --- a/plugins/all/all.go +++ b/plugins/all/all.go @@ -67,14 +67,7 @@ type PluginLoader interface { LoadPlugin(p plugin.EnginePlugin) error } -// scenarioExtras holds additional plugins that are only linked in when the -// "scenario_stub" build tag is active (see extras_stub.go). The init() -// function in that file appends to this slice; without the tag it remains -// nil and DefaultPlugins() returns only the base set. -var scenarioExtras []plugin.EnginePlugin - -// DefaultPlugins returns the standard set of built-in engine plugins, -// extended by any scenario-specific extras registered via build tags. +// DefaultPlugins returns the standard set of built-in engine plugins. // The slice is freshly allocated on each call so callers may safely append // custom plugins without affecting other callers. func DefaultPlugins() []plugin.EnginePlugin { @@ -112,7 +105,7 @@ func DefaultPlugins() []plugin.EnginePlugin { pluginscanner.New(), pluginactors.New(), } - return append(base, scenarioExtras...) + return base } // LoadAll loads all default built-in plugins into the given engine. diff --git a/plugins/all/extras_base_test.go b/plugins/all/extras_base_test.go deleted file mode 100644 index 0f346670..00000000 --- a/plugins/all/extras_base_test.go +++ /dev/null @@ -1,18 +0,0 @@ -//go:build !scenario_stub - -package all - -import "testing" - -// TestDefaultPlugins_BaseExcludesStub asserts that without the -// "scenario_stub" build tag, DefaultPlugins() does NOT include the -// stub provider plugin. This guards against accidentally shipping the -// stub in production server builds. -func TestDefaultPlugins_BaseExcludesStub(t *testing.T) { - for _, p := range DefaultPlugins() { - if p.Name() == "stubprovider" { - t.Error("DefaultPlugins() contains 'stubprovider' in a non-scenario_stub build — stub must not appear in production") - return - } - } -} diff --git a/plugins/all/extras_stub.go b/plugins/all/extras_stub.go deleted file mode 100644 index ea4f2ce5..00000000 --- a/plugins/all/extras_stub.go +++ /dev/null @@ -1,9 +0,0 @@ -//go:build scenario_stub - -package all - -import pluginstub "github.com/GoCodeAlone/workflow/plugins/stubprovider" - -func init() { - scenarioExtras = append(scenarioExtras, pluginstub.New()) -} diff --git a/plugins/all/extras_stub_test.go b/plugins/all/extras_stub_test.go deleted file mode 100644 index af76a499..00000000 --- a/plugins/all/extras_stub_test.go +++ /dev/null @@ -1,26 +0,0 @@ -//go:build scenario_stub - -package all_test - -import ( - "testing" - - "github.com/GoCodeAlone/workflow/plugins/all" -) - -// TestDefaultPlugins_ContainsStub asserts that when compiled with the -// "scenario_stub" build tag, DefaultPlugins() includes the stub provider -// plugin named "stubprovider". -func TestDefaultPlugins_ContainsStub(t *testing.T) { - plugins := all.DefaultPlugins() - for _, p := range plugins { - if p.Name() == "stubprovider" { - return // found - } - } - names := make([]string, 0, len(plugins)) - for _, p := range plugins { - names = append(names, p.Name()) - } - t.Errorf("DefaultPlugins() does not contain 'stubprovider'; plugins: %v", names) -} diff --git a/plugins/stubprovider/plugin.go b/plugins/stubprovider/plugin.go deleted file mode 100644 index 08c49977..00000000 --- a/plugins/stubprovider/plugin.go +++ /dev/null @@ -1,110 +0,0 @@ -// Package stubprovider provides a build-tagged loadable EnginePlugin that -// registers an in-process "stub" iac.provider module. This plugin is -// intended for scenario testing and integration tests only — it must NOT -// be linked into production server binaries. -// -// Loading is controlled by the "scenario_stub" build tag (see -// plugins/all/extras_stub.go). Without the tag, plugins/all.DefaultPlugins() -// does not include this plugin and cmd/server cannot load it. -// -// The registered module type is "iac.provider". When a config entry declares: -// -// type: iac.provider -// config: -// provider: stub -// -// the module's Init validates the provider field and logs a clear warning -// that no real cloud operations will occur. The module registers a -// stubprovider.Provider as a service under its own name so infra.admin can -// resolve it via app.GetService(, &iacProvider). -package stubprovider - -import ( - "fmt" - - "github.com/GoCodeAlone/modular" - "github.com/GoCodeAlone/workflow/iac/stubprovider" - "github.com/GoCodeAlone/workflow/plugin" -) - -// Plugin is the engine plugin that registers the stub iac.provider factory. -type Plugin struct { - plugin.BaseEnginePlugin -} - -// Compile-time assertion that Plugin implements plugin.EnginePlugin. -var _ plugin.EnginePlugin = (*Plugin)(nil) - -// New creates a new stub provider plugin. -func New() *Plugin { - return &Plugin{ - BaseEnginePlugin: plugin.BaseEnginePlugin{ - BaseNativePlugin: plugin.BaseNativePlugin{ - PluginName: "stubprovider", - PluginVersion: "0.1.0", - PluginDescription: "In-process stub iac.provider for scenario testing — no real cloud operations", - }, - Manifest: plugin.PluginManifest{ - Name: "stubprovider", - Version: "0.1.0", - Author: "GoCodeAlone", - Description: "In-process stub iac.provider for scenario testing — no real cloud operations", - ModuleTypes: []string{"iac.provider"}, - }, - }, - } -} - -// ModuleFactories returns a factory for "iac.provider" that produces a -// stubModule whose ProvidesServices registers a stubprovider.Provider. -func (p *Plugin) ModuleFactories() map[string]plugin.ModuleFactory { - return map[string]plugin.ModuleFactory{ - "iac.provider": func(name string, cfg map[string]any) modular.Module { - return &stubModule{name: name, cfg: cfg} - }, - } -} - -// stubModule is the in-process iac.provider module instantiated by the factory. -type stubModule struct { - name string - cfg map[string]any - provider *stubprovider.Provider -} - -// Name returns the module instance name. -func (m *stubModule) Name() string { return m.name } - -// Init validates config and prepares the stub provider. -// Returns an error when config.provider != "stub" to prevent -// accidentally loading this module as a real cloud provider. -func (m *stubModule) Init(app modular.Application) error { - pt, _ := m.cfg["provider"].(string) - if pt != "stub" { - return fmt.Errorf("iac/stubprovider: module %q: provider must be 'stub', got %q — this plugin cannot proxy real cloud providers", m.name, pt) - } - m.provider = stubprovider.New() - app.Logger().Warn("infra.admin stub provider: NO real cloud operations — demo/test only", "module", m.name) - return nil -} - -// ProvidesServices registers the stub provider under the module name so -// infra.admin can resolve it via app.GetService(m.name, &iacProvider). -func (m *stubModule) ProvidesServices() []modular.ServiceProvider { - if m.provider == nil { - // Not yet initialised; provider is set in Init. - // Return a pre-allocated instance so the DI graph can wire - // services before Init is called. - m.provider = stubprovider.New() - } - return []modular.ServiceProvider{ - { - Name: m.name, - Description: "stub iac.provider — in-process, no real cloud ops", - Instance: m.provider, - }, - } -} - -// RequiresServices returns nil — the stub provider has no service dependencies. -func (m *stubModule) RequiresServices() []modular.ServiceDependency { return nil } diff --git a/plugins/stubprovider/plugin_test.go b/plugins/stubprovider/plugin_test.go deleted file mode 100644 index 3bf40bf9..00000000 --- a/plugins/stubprovider/plugin_test.go +++ /dev/null @@ -1,141 +0,0 @@ -package stubprovider_test - -import ( - "context" - "testing" - - "github.com/GoCodeAlone/modular" - "github.com/GoCodeAlone/workflow/interfaces" - pluginstub "github.com/GoCodeAlone/workflow/plugins/stubprovider" -) - -// TestPlugin_ModuleFactories asserts the plugin registers "iac.provider". -func TestPlugin_ModuleFactories(t *testing.T) { - p := pluginstub.New() - factories := p.ModuleFactories() - if factories == nil { - t.Fatal("ModuleFactories returned nil") - } - factory, ok := factories["iac.provider"] - if !ok { - t.Fatalf("expected 'iac.provider' in ModuleFactories, got keys: %v", keys(factories)) - } - if factory == nil { - t.Fatal("factory for 'iac.provider' is nil") - } -} - -// TestPlugin_Module_ProvidesIaCProvider creates an iac.provider module via the -// factory, calls ProvidesServices(), and asserts one of the entries satisfies -// interfaces.IaCProvider. -func TestPlugin_Module_ProvidesIaCProvider(t *testing.T) { - p := pluginstub.New() - factory := p.ModuleFactories()["iac.provider"] - - mod := factory("stub-provider", map[string]any{"provider": "stub"}) - if mod == nil { - t.Fatal("factory returned nil module") - } - if mod.Name() != "stub-provider" { - t.Errorf("module Name = %q, want 'stub-provider'", mod.Name()) - } - - sa, ok := mod.(modular.ServiceAware) - if !ok { - t.Fatalf("module does not implement modular.ServiceAware; got %T", mod) - } - services := sa.ProvidesServices() - if len(services) == 0 { - t.Fatal("ProvidesServices returned empty slice") - } - - var foundProvider interfaces.IaCProvider - for _, svc := range services { - if p, ok := svc.Instance.(interfaces.IaCProvider); ok { - foundProvider = p - break - } - } - if foundProvider == nil { - t.Fatal("none of ProvidesServices entries implements interfaces.IaCProvider") - } -} - -// TestPlugin_Module_NonStubProviderErrors asserts that a module configured -// with provider != "stub" returns an error from Init. -func TestPlugin_Module_NonStubProviderErrors(t *testing.T) { - p := pluginstub.New() - factory := p.ModuleFactories()["iac.provider"] - mod := factory("bad-provider", map[string]any{"provider": "digitalocean"}) - - app := modular.NewStdApplication(nil, nopLogger{}) - err := mod.Init(app) - if err == nil { - t.Fatal("Init with provider=digitalocean should return an error, got nil") - } -} - -// TestPlugin_Module_StubProviderInits asserts that a module configured -// with provider=stub initialises without error. -func TestPlugin_Module_StubProviderInits(t *testing.T) { - p := pluginstub.New() - factory := p.ModuleFactories()["iac.provider"] - mod := factory("my-stub", map[string]any{"provider": "stub"}) - - app := modular.NewStdApplication(nil, nopLogger{}) - if err := mod.Init(app); err != nil { - t.Fatalf("Init with provider=stub should not error: %v", err) - } -} - -// TestPlugin_Module_StubProviderApply exercises the resolved IaCProvider -// end-to-end via a Plan call to prove the service wire-up actually works. -func TestPlugin_Module_StubProviderApply(t *testing.T) { - p := pluginstub.New() - factory := p.ModuleFactories()["iac.provider"] - mod := factory("my-stub", map[string]any{"provider": "stub"}) - - app := modular.NewStdApplication(nil, nopLogger{}) - if err := mod.Init(app); err != nil { - t.Fatalf("Init: %v", err) - } - - sa := mod.(modular.ServiceAware) - var prov interfaces.IaCProvider - for _, svc := range sa.ProvidesServices() { - if ip, ok := svc.Instance.(interfaces.IaCProvider); ok { - prov = ip - break - } - } - if prov == nil { - t.Fatal("no IaCProvider service after Init") - } - - plan, err := prov.Plan(context.Background(), []interfaces.ResourceSpec{ - {Name: "vpc1", Type: "infra.vpc"}, - }, nil) - if err != nil { - t.Fatalf("Plan: %v", err) - } - if len(plan.Actions) == 0 || plan.Actions[0].Action != "create" { - t.Errorf("expected plan with create action, got %+v", plan.Actions) - } -} - -// keys is a test helper that returns the map keys as a slice for diagnostics. -func keys[V any](m map[string]V) []string { - ks := make([]string, 0, len(m)) - for k := range m { - ks = append(ks, k) - } - return ks -} - -// nopLogger satisfies modular.Logger for tests. -type nopLogger struct{} - -func (nopLogger) Debug(string, ...any) {} -func (nopLogger) Info(string, ...any) {} -func (nopLogger) Warn(string, ...any) {} -func (nopLogger) Error(string, ...any) {}