diff --git a/internal/cmd/validate.go b/internal/cmd/validate.go index 547f185..54ece77 100644 --- a/internal/cmd/validate.go +++ b/internal/cmd/validate.go @@ -106,12 +106,18 @@ func runValidate(cmd *cobra.Command, args []string) error { fmt.Printf("Found %d changed file(s)\n", len(changes)) - // Create validator - v := validator.NewLLMValidator(llmClient, &policy) + // Create unified validator that handles all engines + RBAC + v := validator.NewValidator(&policy, true) // verbose=true for CLI + v.SetLLMClient(llmClient) + defer func() { + if err := v.Close(); err != nil { + fmt.Printf("Warning: failed to close validator: %v\n", err) + } + }() // Validate changes ctx := context.Background() - result, err := v.Validate(ctx, changes) + result, err := v.ValidateChanges(ctx, changes) if err != nil { return fmt.Errorf("validation failed: %w", err) } diff --git a/internal/git/repo.go b/internal/git/repo.go index 30ebc59..ae458b2 100644 --- a/internal/git/repo.go +++ b/internal/git/repo.go @@ -54,3 +54,13 @@ func IsGitRepo() bool { err := cmd.Run() return err == nil } + +// GetCurrentUser returns the current git user name +func GetCurrentUser() (string, error) { + cmd := exec.Command("git", "config", "--get", "user.name") + output, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("failed to get git user.name: %w", err) + } + return strings.TrimSpace(string(output)), nil +} diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 80f9e45..312fa86 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -15,6 +15,7 @@ import ( "github.com/DevSymphony/sym-cli/internal/git" "github.com/DevSymphony/sym-cli/internal/llm" "github.com/DevSymphony/sym-cli/internal/policy" + "github.com/DevSymphony/sym-cli/internal/roles" "github.com/DevSymphony/sym-cli/internal/validator" "github.com/DevSymphony/sym-cli/pkg/schema" sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" @@ -446,9 +447,16 @@ func (s *Server) handleQueryConventions(params map[string]interface{}) (interfac } textContent += "\n" } - textContent += "\nāœ“ Next Step: Implement your code following these conventions. After completion, MUST call validate_code to verify compliance." } + // Add RBAC information if available + rbacInfo := s.getRBACInfo() + if rbacInfo != "" { + textContent += "\n\n" + rbacInfo + } + + textContent += "\nāœ“ Next Step: Implement your code following these conventions. After completion, MUST call validate_code to verify compliance." + // Return MCP-compliant response with content array return map[string]interface{}{ "content": []map[string]interface{}{ @@ -635,11 +643,17 @@ func (s *Server) handleValidateCode(params map[string]interface{}) (interface{}, } llmClient := llm.NewClient(apiKey) - llmValidator := validator.NewLLMValidator(llmClient, validationPolicy) - // Validate git changes + // Create unified validator that handles all engines + RBAC + v := validator.NewValidator(validationPolicy, false) // verbose=false for MCP + v.SetLLMClient(llmClient) + defer func() { + _ = v.Close() // Ignore close error in MCP context + }() + + // Validate git changes using unified validator ctx := context.Background() - result, err := llmValidator.Validate(ctx, changes) + result, err := v.ValidateChanges(ctx, changes) if err != nil { return nil, &RPCError{ Code: -32000, @@ -904,3 +918,70 @@ func (s *Server) needsConversion(codePolicyPath string) bool { func (s *Server) convertUserPolicy(userPolicyPath, codePolicyPath string) error { return ConvertPolicyWithLLM(userPolicyPath, codePolicyPath) } + +// getRBACInfo returns RBAC information for the current user +func (s *Server) getRBACInfo() string { + // Try to get current user + username, err := git.GetCurrentUser() + if err != nil { + // Not in a git environment or user not configured + return "" + } + + // Get user's role + userRole, err := roles.GetUserRole(username) + if err != nil { + // Roles not configured + return "" + } + + if userRole == "none" { + return fmt.Sprintf("āš ļø RBAC: User '%s' has no assigned role. You may not have permission to modify files.", username) + } + + // Load user policy to get RBAC details + userPolicy, err := roles.LoadUserPolicyFromRepo() + if err != nil { + // User policy not available + return fmt.Sprintf("šŸ” RBAC: Current user '%s' has role '%s'", username, userRole) + } + + // Check if RBAC is defined + if userPolicy.RBAC == nil || userPolicy.RBAC.Roles == nil { + return fmt.Sprintf("šŸ” RBAC: Current user '%s' has role '%s' (no restrictions defined)", username, userRole) + } + + // Get role configuration + roleConfig, exists := userPolicy.RBAC.Roles[userRole] + if !exists { + return fmt.Sprintf("āš ļø RBAC: User '%s' has role '%s', but role is not defined in policy", username, userRole) + } + + // Build RBAC info message + var rbacMsg strings.Builder + rbacMsg.WriteString("šŸ” RBAC Information:\n") + rbacMsg.WriteString(fmt.Sprintf(" User: %s\n", username)) + rbacMsg.WriteString(fmt.Sprintf(" Role: %s\n", userRole)) + + if len(roleConfig.AllowWrite) > 0 { + rbacMsg.WriteString(fmt.Sprintf(" Allowed paths: %s\n", strings.Join(roleConfig.AllowWrite, ", "))) + } else { + rbacMsg.WriteString(" Allowed paths: All files (no restrictions)\n") + } + + if len(roleConfig.DenyWrite) > 0 { + rbacMsg.WriteString(fmt.Sprintf(" Denied paths: %s\n", strings.Join(roleConfig.DenyWrite, ", "))) + } + + if roleConfig.CanEditPolicy { + rbacMsg.WriteString(" Can edit policy: Yes\n") + } + + if roleConfig.CanEditRoles { + rbacMsg.WriteString(" Can edit roles: Yes\n") + } + + rbacMsg.WriteString("\nāš ļø Note: Modifications to denied paths will be blocked during validation.") + + return rbacMsg.String() +} diff --git a/internal/validator/llm_validator.go b/internal/validator/llm_validator.go index b3a39be..c77d698 100644 --- a/internal/validator/llm_validator.go +++ b/internal/validator/llm_validator.go @@ -69,7 +69,7 @@ func (v *LLMValidator) Validate(ctx context.Context, changes []GitChange) (*Vali for _, rule := range llmRules { result.Checked++ - violation, err := v.checkRule(ctx, change, addedLines, rule) + violation, err := v.CheckRule(ctx, change, addedLines, rule) if err != nil { // Log error but continue fmt.Printf("Warning: failed to check rule %s: %v\n", rule.ID, err) @@ -106,8 +106,9 @@ func (v *LLMValidator) filterLLMRules() []schema.PolicyRule { return llmRules } -// checkRule checks if code violates a specific rule using LLM -func (v *LLMValidator) checkRule(ctx context.Context, change GitChange, addedLines []string, rule schema.PolicyRule) (*Violation, error) { +// CheckRule checks if code violates a specific rule using LLM +// This is the single source of truth for LLM-based validation logic +func (v *LLMValidator) CheckRule(ctx context.Context, change GitChange, addedLines []string, rule schema.PolicyRule) (*Violation, error) { // Build prompt for LLM systemPrompt := `You are a code reviewer. Check if the code changes violate the given coding convention. diff --git a/internal/validator/validator.go b/internal/validator/validator.go index 0523424..72a9885 100644 --- a/internal/validator/validator.go +++ b/internal/validator/validator.go @@ -5,22 +5,27 @@ import ( "fmt" "os" "path/filepath" + "strings" "time" "github.com/DevSymphony/sym-cli/internal/engine/core" "github.com/DevSymphony/sym-cli/internal/engine/registry" + "github.com/DevSymphony/sym-cli/internal/git" + "github.com/DevSymphony/sym-cli/internal/llm" + "github.com/DevSymphony/sym-cli/internal/roles" "github.com/DevSymphony/sym-cli/pkg/schema" ) // Validator validates code against policy type Validator struct { - policy *schema.CodePolicy - verbose bool - registry *registry.Registry - workDir string - selector *FileSelector - ctx context.Context - ctxCancel context.CancelFunc + policy *schema.CodePolicy + verbose bool + registry *registry.Registry + workDir string + selector *FileSelector + ctx context.Context + ctxCancel context.CancelFunc + llmClient *llm.Client // Optional: for LLM-based validation } // NewValidator creates a new validator @@ -34,16 +39,22 @@ func NewValidator(policy *schema.CodePolicy, verbose bool) *Validator { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) return &Validator{ - policy: policy, - verbose: verbose, - registry: registry.Global(), - workDir: workDir, - selector: NewFileSelector(workDir), - ctx: ctx, - ctxCancel: cancel, + policy: policy, + verbose: verbose, + registry: registry.Global(), + workDir: workDir, + selector: NewFileSelector(workDir), + ctx: ctx, + ctxCancel: cancel, + llmClient: nil, // Will be set via SetLLMClient if needed } } +// SetLLMClient sets the LLM client for this validator +func (v *Validator) SetLLMClient(client *llm.Client) { + v.llmClient = client +} + // Violation represents a policy violation type Violation struct { RuleID string @@ -75,6 +86,52 @@ func (v *Validator) Validate(path string) (*Result, error) { fmt.Printf("šŸ” Validating %s against %d rule(s)...\n", path, len(v.policy.Rules)) } + // Check RBAC permissions if enabled + if v.policy.Enforce.RBACConfig != nil && v.policy.Enforce.RBACConfig.Enabled { + // Get current git user + username, err := git.GetCurrentUser() + if err != nil { + if v.verbose { + fmt.Printf("āš ļø RBAC check skipped: %v\n", err) + } + } else { + if v.verbose { + fmt.Printf("šŸ” Checking RBAC permissions for user: %s\n", username) + } + + // Get files to check + fileInfo, err := os.Stat(path) + if err == nil && !fileInfo.IsDir() { + // Validate file permissions + rbacResult, err := roles.ValidateFilePermissions(username, []string{path}) + if err != nil { + if v.verbose { + fmt.Printf("āš ļø RBAC validation failed: %v\n", err) + } + } else if !rbacResult.Allowed { + // Add RBAC violations + for _, deniedFile := range rbacResult.DeniedFiles { + result.Violations = append(result.Violations, Violation{ + RuleID: "rbac-permission-denied", + Severity: "error", + Message: fmt.Sprintf("User '%s' does not have permission to modify this file", username), + File: deniedFile, + Line: 0, + Column: 0, + }) + } + result.Passed = false + + if v.verbose { + fmt.Printf("āŒ RBAC: %d file(s) denied for user %s\n", len(rbacResult.DeniedFiles), username) + } + } else if v.verbose { + fmt.Printf("āœ“ RBAC: User %s has permission to modify all files\n", username) + } + } + } + } + // For each enabled rule, execute validation for _, rule := range v.policy.Rules { if !rule.Enabled { @@ -294,3 +351,239 @@ func (v *Validator) Close() error { } return nil } + +// ValidateChanges validates git changes against all enabled rules +// This is the unified entry point for diff-based validation used by both CLI and MCP +func (v *Validator) ValidateChanges(ctx context.Context, changes []GitChange) (*ValidationResult, error) { + if v.policy == nil { + return nil, fmt.Errorf("policy is not loaded") + } + + result := &ValidationResult{ + Violations: make([]Violation, 0), + Checked: 0, + Passed: 0, + Failed: 0, + } + + // Check RBAC permissions first if enabled + if v.policy.Enforce.RBACConfig != nil && v.policy.Enforce.RBACConfig.Enabled { + // Get current git user + username, err := git.GetCurrentUser() + if err != nil { + if v.verbose { + fmt.Printf("āš ļø RBAC check skipped: %v\n", err) + } + } else { + if v.verbose { + fmt.Printf("šŸ” Checking RBAC permissions for user: %s\n", username) + } + + // Collect all changed files + changedFiles := make([]string, 0, len(changes)) + for _, change := range changes { + if change.Status != "D" { // Skip deleted files + changedFiles = append(changedFiles, change.FilePath) + } + } + + if len(changedFiles) > 0 { + // Validate file permissions + rbacResult, err := roles.ValidateFilePermissions(username, changedFiles) + if err != nil { + if v.verbose { + fmt.Printf("āš ļø RBAC validation failed: %v\n", err) + } + } else if !rbacResult.Allowed { + // Add RBAC violations + for _, deniedFile := range rbacResult.DeniedFiles { + result.Violations = append(result.Violations, Violation{ + RuleID: "rbac-permission-denied", + Severity: "error", + Message: fmt.Sprintf("User '%s' does not have permission to modify this file", username), + File: deniedFile, + Line: 0, + Column: 0, + }) + result.Failed++ + } + + if v.verbose { + fmt.Printf("āŒ RBAC: %d file(s) denied for user %s\n", len(rbacResult.DeniedFiles), username) + } + } else if v.verbose { + fmt.Printf("āœ“ RBAC: User %s has permission to modify all files\n", username) + } + } + } + } + + if v.verbose { + fmt.Printf("šŸ” Validating %d change(s) against %d rule(s)...\n", len(changes), len(v.policy.Rules)) + } + + // For each enabled rule, execute validation + for _, rule := range v.policy.Rules { + if !rule.Enabled { + continue + } + + // Determine which engine to use + engineName := getEngineName(rule) + if engineName == "" { + if v.verbose { + fmt.Printf("āš ļø Rule %s has no engine specified, skipping\n", rule.ID) + } + continue + } + + // Filter changes that match this rule's selector + relevantChanges := v.filterChangesForRule(changes, &rule) + if len(relevantChanges) == 0 { + if v.verbose { + fmt.Printf(" Rule %s: no matching changes\n", rule.ID) + } + continue + } + + if v.verbose { + fmt.Printf(" Rule %s (%s): checking %d change(s)...\n", rule.ID, engineName, len(relevantChanges)) + } + + // Get or create engine + engine, err := v.registry.Get(engineName) + if err != nil { + fmt.Printf("āš ļø Engine %s not found for rule %s: %v\n", engineName, rule.ID, err) + continue + } + + // Initialize engine if needed + if err := v.initializeEngine(engine); err != nil { + fmt.Printf("āš ļø Failed to initialize engine %s: %v\n", engineName, err) + continue + } + + // Convert schema.PolicyRule to core.Rule + coreRule := convertToCoreRule(rule) + + // Execute validation on each change + for _, change := range relevantChanges { + if change.Status == "D" { + continue // Skip deleted files + } + + result.Checked++ + + // For LLM engine, validate the diff using LLMValidator + if engineName == "llm-validator" { + if v.llmClient == nil { + fmt.Printf("āš ļø LLM client not configured for rule %s\n", rule.ID) + continue + } + + // Create LLMValidator to use its CheckRule method + llmValidator := NewLLMValidator(v.llmClient, v.policy) + + // Extract added lines from diff + addedLines := ExtractAddedLines(change.Diff) + if len(addedLines) == 0 && strings.TrimSpace(change.Diff) != "" { + addedLines = strings.Split(change.Diff, "\n") + } + + if len(addedLines) == 0 { + result.Passed++ + continue + } + + violation, err := llmValidator.CheckRule(ctx, change, addedLines, rule) + if err != nil { + fmt.Printf("āš ļø Validation failed for rule %s: %v\n", rule.ID, err) + continue + } + if violation != nil { + result.Failed++ + result.Violations = append(result.Violations, *violation) + } else { + result.Passed++ + } + } else { + // For other engines, validate the file + validationResult, err := engine.Validate(ctx, coreRule, []string{change.FilePath}) + if err != nil { + fmt.Printf("āš ļø Validation failed for rule %s: %v\n", rule.ID, err) + continue + } + + // Collect violations + if validationResult != nil && len(validationResult.Violations) > 0 { + result.Failed++ + for _, coreViolation := range validationResult.Violations { + violation := Violation{ + RuleID: rule.ID, + Severity: rule.Severity, + Message: coreViolation.Message, + File: coreViolation.File, + Line: coreViolation.Line, + Column: coreViolation.Column, + } + + // Use custom message if provided + if rule.Message != "" { + violation.Message = rule.Message + } + + result.Violations = append(result.Violations, violation) + } + } else { + result.Passed++ + } + } + } + + if v.verbose && len(result.Violations) > 0 { + fmt.Printf(" āŒ Found %d violation(s)\n", len(result.Violations)) + } else if v.verbose { + fmt.Printf(" āœ“ Passed\n") + } + } + + if v.verbose { + if len(result.Violations) == 0 { + fmt.Printf("\nāœ… Validation passed: no violations found\n") + } else { + fmt.Printf("\nāŒ Validation failed: %d violation(s) found\n", len(result.Violations)) + } + } + + return result, nil +} + +// filterChangesForRule filters git changes that match the rule's selector +func (v *Validator) filterChangesForRule(changes []GitChange, rule *schema.PolicyRule) []GitChange { + if rule.When == nil { + return changes // No selector, all changes match + } + + var filtered []GitChange + for _, change := range changes { + // Check language match + if len(rule.When.Languages) > 0 { + lang := GetLanguageFromFile(change.FilePath) + matched := false + for _, ruleLang := range rule.When.Languages { + if ruleLang == lang { + matched = true + break + } + } + if !matched { + continue + } + } + + // TODO: Check include/exclude patterns + filtered = append(filtered, change) + } + + return filtered +}