From 4601ccf062f620b122150f99b67ca5c7fcd5458e Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Fri, 29 May 2026 21:18:18 -0400 Subject: [PATCH] chore(plugin): remove orphaned dead code plugin/community + plugin/rbac MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both packages have ZERO importers anywhere in the workflow ecosystem (verified across all workflow* repos + ratchet — only documentation mentions, no code): - plugin/community (SubmissionValidator/ReviewChecklist): plugin-submission validation tooling that was never wired. The workflow-registry repo does its own manifest validation (scripts/validate-manifests.sh + jsonschema), so this Go validator is redundant. - plugin/rbac (BuiltinProvider): an auth.PermissionProvider bridge over auth/rbac.PolicyEngine that was never registered with auth.PermissionManager (AddProvider is never called with a builtin in non-test code). Per the plugin/ consolidation design (#795) these were deferred from deletion pending an ecosystem check; that check found no home or use. Recoverable from git history if a builtin permission provider or submission validator is wanted later. Also drops the two stale rows in docs/API_STABILITY.md and the stale plugin/rbac references in scripts/audit-cloud-symbols.sh comments (the plugin/rbac/aws.go they named never existed). Historical docs/plans left intact. go build ./... + tests for plugin/plugins/cmd-server/auth pass; audit script still passes (plugin/rbac never imported aws-sdk-go-v2). --- docs/API_STABILITY.md | 2 - plugin/community/validator.go | 278 ----------------------------- plugin/community/validator_test.go | 268 --------------------------- plugin/rbac/builtin.go | 72 -------- plugin/rbac/builtin_test.go | 225 ----------------------- scripts/audit-cloud-symbols.sh | 6 +- 6 files changed, 3 insertions(+), 848 deletions(-) delete mode 100644 plugin/community/validator.go delete mode 100644 plugin/community/validator_test.go delete mode 100644 plugin/rbac/builtin.go delete mode 100644 plugin/rbac/builtin_test.go diff --git a/docs/API_STABILITY.md b/docs/API_STABILITY.md index f89ec519..e8f444ca 100644 --- a/docs/API_STABILITY.md +++ b/docs/API_STABILITY.md @@ -321,9 +321,7 @@ The following packages are **not part of the stable public API**. They may chang | `github.com/GoCodeAlone/workflow/mock` | Test helpers. Not stable; only for use in the repo's own test suite. | | `github.com/GoCodeAlone/workflow/secrets` | Secrets resolver internals. Accessed via `StdEngine.SecretsResolver()` only. Note: `StdEngine.SecretsResolver()` returns `*secrets.MultiResolver`. Callers should treat this type as opaque and not depend on its internal methods. A future version may replace this with a public interface. | | `github.com/GoCodeAlone/workflow/plugin/external` | gRPC-based external plugin protocol. Wire format may change. | -| `github.com/GoCodeAlone/workflow/plugin/rbac` | RBAC plugin internals. Use the `plugin.EnginePlugin` interface instead. | | `github.com/GoCodeAlone/workflow/plugin/sdk` | Plugin scaffolding/generation tools. Internal CLI tooling only. | -| `github.com/GoCodeAlone/workflow/plugin/community` | Community plugin validator internals. | --- diff --git a/plugin/community/validator.go b/plugin/community/validator.go deleted file mode 100644 index 8d2fe639..00000000 --- a/plugin/community/validator.go +++ /dev/null @@ -1,278 +0,0 @@ -package community - -import ( - "fmt" - "os" - "path/filepath" - "strings" - - "github.com/GoCodeAlone/workflow/dynamic" - "github.com/GoCodeAlone/workflow/plugin" -) - -// CheckResult represents the outcome of a single validation check. -type CheckResult struct { - Name string `json:"name"` - Passed bool `json:"passed"` - Message string `json:"message,omitempty"` -} - -// ValidationResult holds the full outcome of submission validation. -type ValidationResult struct { - Valid bool `json:"valid"` - Checks []CheckResult `json:"checks"` -} - -// SubmissionValidator validates plugin submissions for community contributions. -type SubmissionValidator struct{} - -// NewSubmissionValidator creates a new SubmissionValidator. -func NewSubmissionValidator() *SubmissionValidator { - return &SubmissionValidator{} -} - -// ValidateDirectory validates a plugin directory for community submission. -func (v *SubmissionValidator) ValidateDirectory(dir string) (*ValidationResult, error) { - result := &ValidationResult{Valid: true} - - // Check 1: Directory exists - info, err := os.Stat(dir) - if err != nil { - result.Valid = false - result.Checks = append(result.Checks, CheckResult{ - Name: "directory_exists", - Passed: false, - Message: fmt.Sprintf("directory %s does not exist: %v", dir, err), - }) - return result, nil - } - if !info.IsDir() { - result.Valid = false - result.Checks = append(result.Checks, CheckResult{ - Name: "directory_exists", - Passed: false, - Message: fmt.Sprintf("%s is not a directory", dir), - }) - return result, nil - } - result.Checks = append(result.Checks, CheckResult{ - Name: "directory_exists", - Passed: true, - }) - - // Check 2: Manifest exists and is valid - manifestPath := filepath.Join(dir, "plugin.json") - manifest, manifestErr := plugin.LoadManifest(manifestPath) - if manifestErr != nil { - result.Valid = false - result.Checks = append(result.Checks, CheckResult{ - Name: "manifest_exists", - Passed: false, - Message: fmt.Sprintf("failed to load manifest: %v", manifestErr), - }) - return result, nil - } - result.Checks = append(result.Checks, CheckResult{ - Name: "manifest_exists", - Passed: true, - }) - - // Check 3: Manifest validation - if valErr := manifest.Validate(); valErr != nil { - result.Valid = false - result.Checks = append(result.Checks, CheckResult{ - Name: "manifest_valid", - Passed: false, - Message: valErr.Error(), - }) - } else { - result.Checks = append(result.Checks, CheckResult{ - Name: "manifest_valid", - Passed: true, - }) - } - - // Check 4: Source file exists - sourceFiles, _ := filepath.Glob(filepath.Join(dir, "*.go")) - var nonTestSources []string - for _, sf := range sourceFiles { - if !strings.HasSuffix(sf, "_test.go") { - nonTestSources = append(nonTestSources, sf) - } - } - if len(nonTestSources) == 0 { - result.Valid = false - result.Checks = append(result.Checks, CheckResult{ - Name: "source_exists", - Passed: false, - Message: "no .go source files found", - }) - } else { - result.Checks = append(result.Checks, CheckResult{ - Name: "source_exists", - Passed: true, - }) - } - - // Check 5: Source syntax validation - for _, sf := range nonTestSources { - data, readErr := os.ReadFile(sf) - if readErr != nil { - result.Valid = false - result.Checks = append(result.Checks, CheckResult{ - Name: "source_valid", - Passed: false, - Message: fmt.Sprintf("failed to read %s: %v", filepath.Base(sf), readErr), - }) - continue - } - if valErr := dynamic.ValidateSource(string(data)); valErr != nil { - result.Valid = false - result.Checks = append(result.Checks, CheckResult{ - Name: "source_valid", - Passed: false, - Message: fmt.Sprintf("%s: %v", filepath.Base(sf), valErr), - }) - } else { - result.Checks = append(result.Checks, CheckResult{ - Name: "source_valid", - Passed: true, - }) - } - } - - // Check 6: Has test files - var testFiles []string - for _, sf := range sourceFiles { - if strings.HasSuffix(sf, "_test.go") { - testFiles = append(testFiles, sf) - } - } - if len(testFiles) == 0 { - result.Valid = false - result.Checks = append(result.Checks, CheckResult{ - Name: "tests_exist", - Passed: false, - Message: "no test files found (expected *_test.go)", - }) - } else { - result.Checks = append(result.Checks, CheckResult{ - Name: "tests_exist", - Passed: true, - }) - } - - // Check 7: Has description in manifest - if manifest.Description == "" { - result.Valid = false - result.Checks = append(result.Checks, CheckResult{ - Name: "has_description", - Passed: false, - Message: "plugin description is empty", - }) - } else { - result.Checks = append(result.Checks, CheckResult{ - Name: "has_description", - Passed: true, - }) - } - - // Check 8: Has license - if manifest.License == "" { - // Warning, not a failure - result.Checks = append(result.Checks, CheckResult{ - Name: "has_license", - Passed: false, - Message: "no license specified (recommended)", - }) - } else { - result.Checks = append(result.Checks, CheckResult{ - Name: "has_license", - Passed: true, - }) - } - - return result, nil -} - -// ReviewChecklist is a structured checklist for plugin code review. -type ReviewChecklist struct { - PluginName string `json:"plugin_name"` - Version string `json:"version"` - Items []ReviewItem `json:"items"` -} - -// ReviewItem is a single item in the review checklist. -type ReviewItem struct { - Category string `json:"category"` - Description string `json:"description"` - Required bool `json:"required"` - Passed bool `json:"passed"` - Notes string `json:"notes,omitempty"` -} - -// NewReviewChecklist creates a standard review checklist for a plugin. -func NewReviewChecklist(manifest *plugin.PluginManifest) *ReviewChecklist { - return &ReviewChecklist{ - PluginName: manifest.Name, - Version: manifest.Version, - Items: []ReviewItem{ - {Category: "manifest", Description: "Manifest contains valid name, version, author, description", Required: true}, - {Category: "manifest", Description: "Version follows semantic versioning (major.minor.patch)", Required: true}, - {Category: "manifest", Description: "Dependencies use valid semver constraints", Required: true}, - {Category: "code", Description: "Source passes sandbox validation (stdlib-only imports)", Required: true}, - {Category: "code", Description: "Component implements Execute function", Required: true}, - {Category: "code", Description: "Component declares Contract() for input/output documentation", Required: false}, - {Category: "code", Description: "No hardcoded secrets or credentials", Required: true}, - {Category: "testing", Description: "Test files exist and cover primary functionality", Required: true}, - {Category: "testing", Description: "Edge cases and error handling are tested", Required: false}, - {Category: "documentation", Description: "Description clearly explains plugin purpose", Required: true}, - {Category: "documentation", Description: "License is specified", Required: false}, - {Category: "security", Description: "Input validation is present for user-supplied data", Required: true}, - {Category: "security", Description: "No unbounded resource consumption (memory, goroutines)", Required: true}, - }, - } -} - -// PassedRequired returns true if all required items are marked as passed. -func (rc *ReviewChecklist) PassedRequired() bool { - for _, item := range rc.Items { - if item.Required && !item.Passed { - return false - } - } - return true -} - -// Summary returns a textual summary of the review. -func (rc *ReviewChecklist) Summary() string { - var b strings.Builder - fmt.Fprintf(&b, "Review Checklist for %s v%s\n", rc.PluginName, rc.Version) - b.WriteString(strings.Repeat("=", 50) + "\n\n") - - passed := 0 - total := 0 - for _, item := range rc.Items { - total++ - status := "[ ]" - if item.Passed { - status = "[x]" - passed++ - } - req := "" - if item.Required { - req = " (required)" - } - fmt.Fprintf(&b, "%s [%s] %s%s\n", status, item.Category, item.Description, req) - if item.Notes != "" { - fmt.Fprintf(&b, " Notes: %s\n", item.Notes) - } - } - fmt.Fprintf(&b, "\nResult: %d/%d checks passed\n", passed, total) - if rc.PassedRequired() { - b.WriteString("Status: APPROVED (all required checks passed)\n") - } else { - b.WriteString("Status: NEEDS REVISION (some required checks failed)\n") - } - return b.String() -} diff --git a/plugin/community/validator_test.go b/plugin/community/validator_test.go deleted file mode 100644 index 8589a638..00000000 --- a/plugin/community/validator_test.go +++ /dev/null @@ -1,268 +0,0 @@ -package community - -import ( - "encoding/json" - "os" - "path/filepath" - "testing" - - "github.com/GoCodeAlone/workflow/plugin" -) - -func validManifest() *plugin.PluginManifest { - return &plugin.PluginManifest{ - Name: "test-plugin", - Version: "1.0.0", - Author: "Test Author", - Description: "A test plugin", - License: "MIT", - } -} - -func setupValidPluginDir(t *testing.T) string { - t.Helper() - dir := t.TempDir() - - // Write manifest - m := validManifest() - data, _ := json.MarshalIndent(m, "", " ") - if err := os.WriteFile(filepath.Join(dir, "plugin.json"), data, 0644); err != nil { - t.Fatal(err) - } - - // Write source file (valid Go with only stdlib imports) - source := `package component - -import "context" - -func Name() string { return "test-plugin" } - -func Execute(ctx context.Context, params map[string]interface{}) (map[string]interface{}, error) { - return map[string]interface{}{"status": "ok"}, nil -} -` - if err := os.WriteFile(filepath.Join(dir, "test-plugin.go"), []byte(source), 0644); err != nil { - t.Fatal(err) - } - - // Write test file - testSource := `package component_test - -import "testing" - -func TestPlugin(t *testing.T) { - // placeholder -} -` - if err := os.WriteFile(filepath.Join(dir, "test-plugin_test.go"), []byte(testSource), 0644); err != nil { - t.Fatal(err) - } - - return dir -} - -func TestSubmissionValidatorValidDirectory(t *testing.T) { - dir := setupValidPluginDir(t) - v := NewSubmissionValidator() - - result, err := v.ValidateDirectory(dir) - if err != nil { - t.Fatalf("ValidateDirectory error: %v", err) - } - - for _, check := range result.Checks { - if !check.Passed && check.Name != "has_license" { - t.Errorf("check %q failed: %s", check.Name, check.Message) - } - } -} - -func TestSubmissionValidatorNonexistentDir(t *testing.T) { - v := NewSubmissionValidator() - result, err := v.ValidateDirectory("/nonexistent/path") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if result.Valid { - t.Error("expected invalid result for nonexistent directory") - } -} - -func TestSubmissionValidatorNotADirectory(t *testing.T) { - tmpFile := filepath.Join(t.TempDir(), "file.txt") - if err := os.WriteFile(tmpFile, []byte("test"), 0644); err != nil { - t.Fatal(err) - } - - v := NewSubmissionValidator() - result, err := v.ValidateDirectory(tmpFile) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if result.Valid { - t.Error("expected invalid result for file") - } -} - -func TestSubmissionValidatorNoManifest(t *testing.T) { - dir := t.TempDir() - v := NewSubmissionValidator() - - result, err := v.ValidateDirectory(dir) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if result.Valid { - t.Error("expected invalid result for missing manifest") - } -} - -func TestSubmissionValidatorInvalidManifest(t *testing.T) { - dir := t.TempDir() - // Invalid manifest (missing fields) - m := &plugin.PluginManifest{Name: "x"} // Missing version, author, etc. - data, _ := json.MarshalIndent(m, "", " ") - if err := os.WriteFile(filepath.Join(dir, "plugin.json"), data, 0644); err != nil { - t.Fatal(err) - } - - v := NewSubmissionValidator() - result, err := v.ValidateDirectory(dir) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if result.Valid { - t.Error("expected invalid result for invalid manifest") - } -} - -func TestSubmissionValidatorNoSourceFiles(t *testing.T) { - dir := t.TempDir() - m := validManifest() - data, _ := json.MarshalIndent(m, "", " ") - if err := os.WriteFile(filepath.Join(dir, "plugin.json"), data, 0644); err != nil { - t.Fatal(err) - } - - v := NewSubmissionValidator() - result, err := v.ValidateDirectory(dir) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if result.Valid { - t.Error("expected invalid result for no source files") - } -} - -func TestSubmissionValidatorNoTestFiles(t *testing.T) { - dir := t.TempDir() - m := validManifest() - data, _ := json.MarshalIndent(m, "", " ") - if err := os.WriteFile(filepath.Join(dir, "plugin.json"), data, 0644); err != nil { - t.Fatal(err) - } - source := `package component - -func Name() string { return "test" } -` - if err := os.WriteFile(filepath.Join(dir, "plugin.go"), []byte(source), 0644); err != nil { - t.Fatal(err) - } - - v := NewSubmissionValidator() - result, err := v.ValidateDirectory(dir) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if result.Valid { - t.Error("expected invalid result for no test files") - } - - // Verify the tests_exist check specifically failed - for _, check := range result.Checks { - if check.Name == "tests_exist" && check.Passed { - t.Error("tests_exist check should have failed") - } - } -} - -func TestReviewChecklist(t *testing.T) { - m := validManifest() - rc := NewReviewChecklist(m) - - if rc.PluginName != "test-plugin" { - t.Errorf("PluginName = %q, want %q", rc.PluginName, "test-plugin") - } - if rc.Version != "1.0.0" { - t.Errorf("Version = %q, want %q", rc.Version, "1.0.0") - } - if len(rc.Items) == 0 { - t.Error("expected non-empty items") - } - - // Initially nothing is passed - if rc.PassedRequired() { - t.Error("expected PassedRequired to be false initially") - } - - // Pass all required items - for i := range rc.Items { - if rc.Items[i].Required { - rc.Items[i].Passed = true - } - } - if !rc.PassedRequired() { - t.Error("expected PassedRequired to be true after passing all required") - } -} - -func TestReviewChecklistSummary(t *testing.T) { - m := validManifest() - rc := NewReviewChecklist(m) - - summary := rc.Summary() - if summary == "" { - t.Error("expected non-empty summary") - } - - // Mark all required as passed and check APPROVED - for i := range rc.Items { - if rc.Items[i].Required { - rc.Items[i].Passed = true - } - } - summary = rc.Summary() - if !contains(summary, "APPROVED") { - t.Error("expected APPROVED in summary when all required pass") - } - - // Add a note to one item - rc.Items[0].Notes = "looks good" - summary = rc.Summary() - if !contains(summary, "looks good") { - t.Error("expected notes in summary") - } -} - -func TestReviewChecklistNeedsRevision(t *testing.T) { - m := validManifest() - rc := NewReviewChecklist(m) - - summary := rc.Summary() - if !contains(summary, "NEEDS REVISION") { - t.Error("expected NEEDS REVISION in summary when required checks fail") - } -} - -func contains(s, substr string) bool { - return len(s) >= len(substr) && searchString(s, substr) -} - -func searchString(s, substr string) bool { - for i := range len(s) - len(substr) + 1 { - if s[i:i+len(substr)] == substr { - return true - } - } - return false -} diff --git a/plugin/rbac/builtin.go b/plugin/rbac/builtin.go deleted file mode 100644 index dff1d48d..00000000 --- a/plugin/rbac/builtin.go +++ /dev/null @@ -1,72 +0,0 @@ -package rbac - -import ( - "context" - "sync" - - "github.com/GoCodeAlone/workflow/auth" - coreRBAC "github.com/GoCodeAlone/workflow/auth/rbac" -) - -// BuiltinProvider wraps the existing PolicyEngine to implement PermissionProvider. -type BuiltinProvider struct { - engine *coreRBAC.PolicyEngine - mu sync.RWMutex -} - -// NewBuiltinProvider creates a BuiltinProvider backed by the given PolicyEngine. -func NewBuiltinProvider(engine *coreRBAC.PolicyEngine) *BuiltinProvider { - return &BuiltinProvider{engine: engine} -} - -// Name returns the provider identifier. -func (b *BuiltinProvider) Name() string { return "builtin" } - -// CheckPermission maps the PermissionProvider interface to PolicyEngine.Allowed. -// The subject is treated as a role name. -func (b *BuiltinProvider) CheckPermission(_ context.Context, subject, resource, action string) (bool, error) { - b.mu.RLock() - defer b.mu.RUnlock() - return b.engine.Allowed(subject, coreRBAC.Resource(resource), coreRBAC.Action(action)), nil -} - -// ListPermissions returns all permissions for the given role. -func (b *BuiltinProvider) ListPermissions(_ context.Context, subject string) ([]auth.Permission, error) { - b.mu.RLock() - defer b.mu.RUnlock() - role, ok := b.engine.GetRole(subject) - if !ok { - return nil, nil - } - perms := make([]auth.Permission, 0, len(role.Permissions)) - for _, p := range role.Permissions { - perms = append(perms, auth.Permission{ - Resource: string(p.Resource), - Action: string(p.Action), - Effect: "allow", - }) - } - return perms, nil -} - -// SyncRoles registers role definitions in the underlying PolicyEngine. -// This allows dynamic role creation beyond the 4 built-in roles. -func (b *BuiltinProvider) SyncRoles(_ context.Context, roles []auth.RoleDefinition) error { - b.mu.Lock() - defer b.mu.Unlock() - for _, rd := range roles { - perms := make([]coreRBAC.Permission, 0, len(rd.Permissions)) - for _, p := range rd.Permissions { - perms = append(perms, coreRBAC.Permission{ - Resource: coreRBAC.Resource(p.Resource), - Action: coreRBAC.Action(p.Action), - }) - } - b.engine.RegisterRole(&coreRBAC.Role{ - Name: rd.Name, - Description: rd.Description, - Permissions: perms, - }) - } - return nil -} diff --git a/plugin/rbac/builtin_test.go b/plugin/rbac/builtin_test.go deleted file mode 100644 index 500146f9..00000000 --- a/plugin/rbac/builtin_test.go +++ /dev/null @@ -1,225 +0,0 @@ -package rbac - -import ( - "context" - "testing" - - "github.com/GoCodeAlone/workflow/auth" - coreRBAC "github.com/GoCodeAlone/workflow/auth/rbac" -) - -func TestBuiltinProvider_Name(t *testing.T) { - p := NewBuiltinProvider(coreRBAC.NewPolicyEngine()) - if p.Name() != "builtin" { - t.Errorf("expected name 'builtin', got %q", p.Name()) - } -} - -func TestBuiltinProvider_CheckPermission_BuiltinRoles(t *testing.T) { - p := NewBuiltinProvider(coreRBAC.NewPolicyEngine()) - ctx := context.Background() - - tests := []struct { - subject string - resource string - action string - want bool - }{ - {"admin", "workflows", "read", true}, - {"admin", "users", "admin", true}, - {"viewer", "workflows", "read", true}, - {"viewer", "workflows", "write", false}, - {"editor", "workflows", "write", true}, - {"editor", "workflows", "delete", false}, - {"operator", "workflows", "delete", true}, - {"operator", "users", "write", false}, - {"nonexistent", "workflows", "read", false}, - } - - for _, tt := range tests { - got, err := p.CheckPermission(ctx, tt.subject, tt.resource, tt.action) - if err != nil { - t.Errorf("CheckPermission(%q, %q, %q): unexpected error: %v", tt.subject, tt.resource, tt.action, err) - continue - } - if got != tt.want { - t.Errorf("CheckPermission(%q, %q, %q) = %v, want %v", tt.subject, tt.resource, tt.action, got, tt.want) - } - } -} - -func TestBuiltinProvider_ListPermissions(t *testing.T) { - p := NewBuiltinProvider(coreRBAC.NewPolicyEngine()) - ctx := context.Background() - - perms, err := p.ListPermissions(ctx, "viewer") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(perms) != 3 { - t.Fatalf("expected 3 permissions for viewer, got %d", len(perms)) - } - for _, perm := range perms { - if perm.Effect != "allow" { - t.Errorf("expected effect 'allow', got %q", perm.Effect) - } - if perm.Action != "read" { - t.Errorf("expected action 'read' for viewer, got %q", perm.Action) - } - } -} - -func TestBuiltinProvider_ListPermissions_Unknown(t *testing.T) { - p := NewBuiltinProvider(coreRBAC.NewPolicyEngine()) - ctx := context.Background() - - perms, err := p.ListPermissions(ctx, "nonexistent") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if perms != nil { - t.Errorf("expected nil for unknown role, got %v", perms) - } -} - -func TestBuiltinProvider_SyncRoles(t *testing.T) { - engine := coreRBAC.NewPolicyEngine() - p := NewBuiltinProvider(engine) - ctx := context.Background() - - roles := []auth.RoleDefinition{ - { - Name: "plugin-manager", - Description: "Can manage plugins", - Permissions: []auth.Permission{ - {Resource: "plugins", Action: "read", Effect: "allow"}, - {Resource: "plugins", Action: "write", Effect: "allow"}, - {Resource: "plugins", Action: "delete", Effect: "allow"}, - }, - }, - } - - if err := p.SyncRoles(ctx, roles); err != nil { - t.Fatalf("SyncRoles: %v", err) - } - - // Verify the role was registered in the engine - allowed, err := p.CheckPermission(ctx, "plugin-manager", "plugins", "write") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !allowed { - t.Error("expected plugin-manager to have write access to plugins") - } - - // Verify the role doesn't have permissions not granted - allowed, err = p.CheckPermission(ctx, "plugin-manager", "workflows", "read") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if allowed { - t.Error("expected plugin-manager to not have access to workflows") - } -} - -func TestBuiltinProvider_SyncRoles_OverridesExisting(t *testing.T) { - engine := coreRBAC.NewPolicyEngine() - p := NewBuiltinProvider(engine) - ctx := context.Background() - - // Viewer normally has read-only. Override with write access. - roles := []auth.RoleDefinition{ - { - Name: "viewer", - Description: "Upgraded viewer with write", - Permissions: []auth.Permission{ - {Resource: "workflows", Action: "read", Effect: "allow"}, - {Resource: "workflows", Action: "write", Effect: "allow"}, - }, - }, - } - - if err := p.SyncRoles(ctx, roles); err != nil { - t.Fatalf("SyncRoles: %v", err) - } - - allowed, err := p.CheckPermission(ctx, "viewer", "workflows", "write") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !allowed { - t.Error("expected overridden viewer to have write access") - } -} - -func TestBuiltinProvider_CustomPermissionPatterns(t *testing.T) { - engine := coreRBAC.NewPolicyEngine() - p := NewBuiltinProvider(engine) - ctx := context.Background() - - roles := []auth.RoleDefinition{ - { - Name: "workflow-admin", - Description: "Custom workflow permissions", - Permissions: []auth.Permission{ - {Resource: "plugins", Action: "manage", Effect: "allow"}, - {Resource: "workflows", Action: "create", Effect: "allow"}, - {Resource: "environments", Action: "deploy", Effect: "allow"}, - }, - }, - } - - if err := p.SyncRoles(ctx, roles); err != nil { - t.Fatalf("SyncRoles: %v", err) - } - - tests := []struct { - resource string - action string - want bool - }{ - {"plugins", "manage", true}, - {"workflows", "create", true}, - {"environments", "deploy", true}, - {"plugins", "delete", false}, - } - - for _, tt := range tests { - got, err := p.CheckPermission(ctx, "workflow-admin", tt.resource, tt.action) - if err != nil { - t.Errorf("CheckPermission(workflow-admin, %q, %q): %v", tt.resource, tt.action, err) - continue - } - if got != tt.want { - t.Errorf("CheckPermission(workflow-admin, %q, %q) = %v, want %v", tt.resource, tt.action, got, tt.want) - } - } -} - -// --- Integration: BuiltinProvider with PermissionManager --- - -func TestBuiltinProvider_WithPermissionManager(t *testing.T) { - engine := coreRBAC.NewPolicyEngine() - bp := NewBuiltinProvider(engine) - - pm := auth.NewPermissionManager() - pm.AddProvider(bp) - - ctx := context.Background() - - allowed, err := pm.Check(ctx, "admin", "workflows", "delete") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !allowed { - t.Error("expected admin to be allowed through PermissionManager") - } - - perms, err := pm.ListAll(ctx, "editor") - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if len(perms) == 0 { - t.Error("expected non-empty permissions for editor") - } -} diff --git a/scripts/audit-cloud-symbols.sh b/scripts/audit-cloud-symbols.sh index 042afb08..12cabbe3 100755 --- a/scripts/audit-cloud-symbols.sh +++ b/scripts/audit-cloud-symbols.sh @@ -7,7 +7,7 @@ # extended in that design's Phase 0. It exists because review cycles kept # finding hand-maintained inventory claims wrong — first grep matching SDK # names inside doc comments (cycle 9), then a survey scoped to module/ that -# missed aws-sdk importers in provider/, plugin/rbac/, iam/, artifact/ +# missed aws-sdk importers in provider/, iam/, artifact/ # (cycle 10). This script is comment-immune AND whole-repo by construction. # # This script answers, mechanically: @@ -60,7 +60,7 @@ FAIL=0 echo "== Cloud-SDK real-import map (WHOLE REPO, *_test.go excluded) ==" echo " module/ = this design's IaC-state/platform/standalone scope" echo " elsewhere = out-of-scope surface (see design Non-Goals): provider/," -echo " plugin/rbac/, iam/, artifact/ — the #653 'RBAC/secrets/artifact stay'" +echo " iam/, artifact/ — the #653 'RBAC/secrets/artifact stay'" echo " surface, parallel to godo." for sdk in "${SDK_TREES[@]}"; do echo @@ -128,7 +128,7 @@ echo "== Invariant: build graph has no unexpected gcp/azure/api transitive deps # 2026-05-15 migration doc; legitimate transitive, not a GCP SDK # client). # Asymmetric vs aws-sdk-go-v2 (Phase B): aws-sdk-go-v2 STAYS for out-of-scope -# provider/aws/ + plugin/rbac/aws.go + iam/aws.go + artifact/s3.go. +# provider/aws/ + iam/aws.go + artifact/s3.go. if [[ $CHECK -eq 1 && -f .phase-c-complete ]]; then # Capture exit code separately so a failed `go list` cannot be swallowed # by `|| true` (which would also mask legitimate gate violations).