Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ require (
)

require (
github.com/modelcontextprotocol/go-sdk v1.1.0
github.com/manifoldco/promptui v0.9.0
github.com/modelcontextprotocol/go-sdk v1.1.0
github.com/stretchr/testify v1.11.1
)

Expand Down
24 changes: 23 additions & 1 deletion internal/adapter/checkstyle/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,29 @@ func (a *Adapter) execute(ctx context.Context, config []byte, files []string) (*
"-c", configFile,
"-f", "xml", // XML output format
}
args = append(args, files...)

// Ensure all file paths are absolute
// Convert relative paths using WorkDir as base
absFiles := make([]string, len(files))
for i, file := range files {
var absPath string
if filepath.IsAbs(file) {
// Already absolute, use as-is
absPath = file
} else {
// Relative path, resolve against WorkDir
absPath = filepath.Join(a.WorkDir, file)
}

// Verify file exists
if _, err := os.Stat(absPath); os.IsNotExist(err) {
return nil, fmt.Errorf("file not found: %s", file)
}

absFiles[i] = absPath
}

args = append(args, absFiles...)

// Execute
start := time.Now()
Expand Down
8 changes: 8 additions & 0 deletions internal/adapter/checkstyle/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ func parseOutput(output *adapter.ToolOutput) ([]adapter.Violation, error) {
return []adapter.Violation{}, nil
}

// If no output but non-zero exit code, something went wrong
if output.Stdout == "" {
if output.Stderr != "" {
return nil, fmt.Errorf("checkstyle failed (exit code %d): %s", output.ExitCode, output.Stderr)
}
return nil, fmt.Errorf("checkstyle failed with exit code %d but produced no output", output.ExitCode)
}

// Parse XML output
var result CheckstyleOutput
if err := xml.Unmarshal([]byte(output.Stdout), &result); err != nil {
Expand Down
1 change: 1 addition & 0 deletions internal/adapter/eslint/adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ func TestNewAdapter(t *testing.T) {
adapter := NewAdapter("", "")
if adapter == nil {
t.Fatal("NewAdapter() returned nil")
return
}

if adapter.ToolsDir == "" {
Expand Down
23 changes: 22 additions & 1 deletion internal/adapter/pmd/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,31 @@ func (a *Adapter) execute(ctx context.Context, config []byte, files []string) (*
// Build command
pmdPath := a.getPMDPath()

// Ensure all file paths are absolute
// Convert relative paths using WorkDir as base
absFiles := make([]string, len(files))
for i, file := range files {
var absPath string
if filepath.IsAbs(file) {
// Already absolute, use as-is
absPath = file
} else {
// Relative path, resolve against WorkDir
absPath = filepath.Join(a.WorkDir, file)
}

// Verify file exists
if _, err := os.Stat(absPath); os.IsNotExist(err) {
return nil, fmt.Errorf("file not found: %s", file)
}

absFiles[i] = absPath
}

// PMD command format: pmd check -d <files> -R <ruleset> -f json
args := []string{
"check",
"-d", strings.Join(files, ","), // Comma-separated file list
"-d", strings.Join(absFiles, ","), // Comma-separated file list
"-R", rulesetFile,
"-f", "json", // JSON output format
"--no-cache", // Disable cache for consistent results
Expand Down
1 change: 1 addition & 0 deletions internal/adapter/prettier/adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ func TestNewAdapter(t *testing.T) {
a := NewAdapter("", "")
if a == nil {
t.Fatal("NewAdapter() returned nil")
return
}

if a.ToolsDir == "" {
Expand Down
1 change: 1 addition & 0 deletions internal/adapter/subprocess_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ func TestNewSubprocessExecutor(t *testing.T) {
executor := NewSubprocessExecutor()
if executor == nil {
t.Fatal("NewSubprocessExecutor() returned nil")
return
}

if executor.Timeout != 2*time.Minute {
Expand Down
1 change: 1 addition & 0 deletions internal/adapter/tsc/adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ func TestNewAdapter(t *testing.T) {
adapter := NewAdapter("", "")
if adapter == nil {
t.Fatal("NewAdapter() returned nil")
return
}

// Should have default ToolsDir
Expand Down
9 changes: 9 additions & 0 deletions internal/git/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package git

import (
"fmt"
"os"
"os/exec"
"regexp"
"strings"
Expand Down Expand Up @@ -56,7 +57,15 @@ func IsGitRepo() bool {
}

// GetCurrentUser returns the current git user name
// Checks GIT_AUTHOR_NAME environment variable first (for testing),
// then falls back to git config user.name
func GetCurrentUser() (string, error) {
// Check environment variable first (for testing)
if user := os.Getenv("GIT_AUTHOR_NAME"); user != "" {
return user, nil
}

// Fallback to git config
cmd := exec.Command("git", "config", "--get", "user.name")
output, err := cmd.Output()
if err != nil {
Expand Down
228 changes: 10 additions & 218 deletions internal/validator/validator.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분 변경이 꼭 필요한 부분이야? 내 PR도 비슷한 수정이 포함되어 잇어

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RBAC 테스트하려다보니 저렇게 넣었는데, 좋은 패턴은 아닌것같아

Original file line number Diff line number Diff line change
Expand Up @@ -65,186 +65,6 @@ type Violation struct {
Column int
}

// Result represents validation result
type Result struct {
Violations []Violation
Passed bool
}

// Validate validates the given path
func (v *Validator) Validate(path string) (*Result, error) {
if v.policy == nil {
return nil, fmt.Errorf("policy is not loaded")
}

result := &Result{
Violations: make([]Violation, 0),
Passed: true,
}

if v.verbose {
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 {
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
}

// Get files that match this rule's selector
files, err := v.selectFilesForRule(path, &rule)
if err != nil {
fmt.Printf("⚠️ Failed to select files for rule %s: %v\n", rule.ID, err)
continue
}

if len(files) == 0 {
if v.verbose {
fmt.Printf(" Rule %s: no matching files\n", rule.ID)
}
continue
}

// 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
if v.verbose {
fmt.Printf(" Rule %s (%s): checking %d file(s)...\n", rule.ID, engineName, len(files))
}

validationResult, err := engine.Validate(v.ctx, coreRule, files)
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 {
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)
}

if v.verbose {
fmt.Printf(" ❌ Found %d violation(s)\n", len(validationResult.Violations))
}
} else if v.verbose {
fmt.Printf(" ✓ Passed\n")
}
}

// Determine overall pass/fail
result.Passed = len(result.Violations) == 0

if v.verbose {
if result.Passed {
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
}

// CanAutoFix checks if violations can be auto-fixed
func (v *Result) CanAutoFix() bool {
for _, violation := range v.Violations {
// Check if rule has autofix enabled
_ = violation
}
return false
}

// AutoFix attempts to automatically fix violations
func (v *Validator) AutoFix(result *Result) error {
// TODO: Implement auto-fix logic
return fmt.Errorf("auto-fix not implemented yet")
}

// Helper functions

// getEngineName extracts the engine name from a rule
Expand All @@ -255,43 +75,6 @@ func getEngineName(rule schema.PolicyRule) string {
return ""
}

// selectFilesForRule selects files that match the rule's selector
func (v *Validator) selectFilesForRule(basePath string, rule *schema.PolicyRule) ([]string, error) {
// If path is a specific file, check if it matches the selector
fileInfo, err := os.Stat(basePath)
if err != nil {
return nil, err
}

if !fileInfo.IsDir() {
// Single file - check if it matches selector
if rule.When != nil {
lang := GetLanguageFromFile(basePath)
if len(rule.When.Languages) > 0 {
matched := false
for _, ruleLang := range rule.When.Languages {
if ruleLang == lang {
matched = true
break
}
}
if !matched {
return []string{}, nil
}
}
}
return []string{basePath}, nil
}

// Directory - use selector to find files
if rule.When == nil {
// No selector, use all files in directory
return v.selector.SelectFiles(nil)
}

return v.selector.SelectFiles(rule.When)
}

// initializeEngine initializes an engine if not already initialized
func (v *Validator) initializeEngine(engine core.Engine) error {
// Create engine config
Expand Down Expand Up @@ -380,10 +163,19 @@ func (v *Validator) ValidateChanges(ctx context.Context, changes []GitChange) (*
}

// Collect all changed files
// Convert absolute paths to relative paths for RBAC checking
changedFiles := make([]string, 0, len(changes))
for _, change := range changes {
if change.Status != "D" { // Skip deleted files
changedFiles = append(changedFiles, change.FilePath)
filePath := change.FilePath
// Convert absolute path to relative path from workDir
if filepath.IsAbs(filePath) {
relPath, err := filepath.Rel(v.workDir, filePath)
if err == nil {
filePath = relPath
}
}
changedFiles = append(changedFiles, filePath)
}
}

Expand Down
Loading