fix: address security review findings (1 CRITICAL, 5 HIGH, 3 MEDIUM)

cmd/audit.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"strings"
 	"sync"
 	"time"
@@ -191,25 +192,17 @@ func writeAuditReports(result engine.ScoreResult, path string) {
 
 	// Set GitHub Actions outputs if in CI
 	if ghOutput := os.Getenv("GITHUB_OUTPUT"); ghOutput != "" {
-		f, err := os.OpenFile(ghOutput, os.O_APPEND|os.O_WRONLY, 0644)
-		if err == nil {
+		if f, err := os.OpenFile(ghOutput, os.O_APPEND|os.O_WRONLY, 0644); err == nil {
+			defer f.Close()
 			fmt.Fprintf(f, "score=%d\n", result.Score)
 			fmt.Fprintf(f, "grade=%s\n", result.Grade)
 			fmt.Fprintf(f, "total-findings=%d\n", len(result.Findings))
 			fmt.Fprintf(f, "critical-findings=%d\n", result.Summary[engine.SevCritical])
 			fmt.Fprintf(f, "high-findings=%d\n", result.Summary[engine.SevHigh])
-			f.Close()
 		}
 	}
 }
 
 func resolveAbsPath(path string) (string, error) {
-	if strings.HasPrefix(path, "/") {
-		return path, nil
-	}
-	cwd, err := os.Getwd()
-	if err != nil {
-		return "", err
-	}
-	return cwd + "/" + path, nil
+	return filepath.Abs(path)
 }

cmd/full.go

@@ -96,7 +96,6 @@ func runFull(cmd *cobra.Command, args []string) {
 		Threads:   fullThreads,
 		Timeout:   time.Duration(fullTimeout) * time.Second,
 		UserAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/125.0.0.0 Safari/537.36",
-		Verbose:   fullVerbose,
 	}
 
 	eng := engine.New(scanCfg)
@@ -107,7 +106,13 @@ func runFull(cmd *cobra.Command, args []string) {
 	eng.Register(&modules.XSS{})
 	eng.Register(&modules.InfoDisclosure{})
 
-	remoteResult := eng.Run()
+	remoteResult, runErr := eng.Run()
+	if runErr != nil {
+		exitError(fmt.Sprintf("scan engine error: %v", runErr))
+	}
+	if len(remoteResult.Errors) > 0 && fullVerbose {
+		fmt.Fprintf(os.Stderr, "  [!] %d module(s) failed — remote scan may be incomplete\n", len(remoteResult.Errors))
+	}
 
 	// --- Phase 2: Local audit ---
 	if !fullJSON {

cmd/scan.go

@@ -69,13 +69,10 @@ func runScan(cmd *cobra.Command, args []string) {
 	target = strings.TrimRight(target, "/")
 
 	cfg := &engine.Config{
-		TargetURL:  target,
-		Threads:    scanThreads,
-		Timeout:    time.Duration(scanTimeout) * time.Second,
-		UserAgent:  "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/125.0.0.0 Safari/537.36",
-		MinScore:   scanMinScore,
-		OutputJSON: scanJSON,
-		Verbose:    scanVerbose,
+		TargetURL: target,
+		Threads:   scanThreads,
+		Timeout:   time.Duration(scanTimeout) * time.Second,
+		UserAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/125.0.0.0 Safari/537.36",
 	}
 
 	if scanModules != "" {
@@ -110,7 +107,14 @@ func runScan(cmd *cobra.Command, args []string) {
 
 	// Run
 	scanStart := time.Now()
-	result := eng.Run()
+	result, runErr := eng.Run()
+	if runErr != nil {
+		fmt.Fprintf(os.Stderr, "Error: %v\n", runErr)
+		os.Exit(1)
+	}
+	if len(result.Errors) > 0 && scanVerbose {
+		fmt.Fprintf(os.Stderr, "  [!] %d module(s) failed — scan may be incomplete\n", len(result.Errors))
+	}
 	scanDuration := time.Since(scanStart)
 
 	// Output
@@ -162,14 +166,13 @@ func runScan(cmd *cobra.Command, args []string) {
 
 	// Set GitHub Actions outputs if in CI
 	if ghOutput := os.Getenv("GITHUB_OUTPUT"); ghOutput != "" {
-		f, err := os.OpenFile(ghOutput, os.O_APPEND|os.O_WRONLY, 0644)
-		if err == nil {
+		if f, err := os.OpenFile(ghOutput, os.O_APPEND|os.O_WRONLY, 0644); err == nil {
+			defer f.Close()
 			fmt.Fprintf(f, "score=%d\n", result.Score)
 			fmt.Fprintf(f, "grade=%s\n", result.Grade)
 			fmt.Fprintf(f, "total-findings=%d\n", len(result.Findings))
 			fmt.Fprintf(f, "critical-findings=%d\n", result.Summary[engine.SevCritical])
 			fmt.Fprintf(f, "high-findings=%d\n", result.Summary[engine.SevHigh])
-			f.Close()
 		}
 	}
 

pkg/engine/config.go

@@ -3,22 +3,9 @@ package engine
 import "time"
 
 type Config struct {
-	TargetURL   string
-	Threads     int
-	Timeout     time.Duration
-	UserAgent   string
-	Modules     []string // empty = all
-	MinScore    int
-	OutputJSON  bool
-	Verbose     bool
-}
-
-func DefaultConfig(target string) *Config {
-	return &Config{
-		TargetURL: target,
-		Threads:   10,
-		Timeout:   15 * time.Second,
-		UserAgent: "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/125.0.0.0 Safari/537.36",
-		MinScore:  70,
-	}
+	TargetURL string
+	Threads   int
+	Timeout   time.Duration
+	UserAgent string
+	Modules   []string // empty = all
 }

pkg/engine/engine.go

@@ -1,45 +1,75 @@
 package engine
 
 import (
+	"context"
 	"fmt"
 	"sync"
 	"time"
 )
 
+// defaultModuleTimeout is the per-module execution timeout.
+const defaultModuleTimeout = 5 * time.Minute
+
+// Module is the interface that all remote scan modules must implement.
 type Module interface {
 	Name() string
 	Description() string
-	Run(cfg *Config) ([]Finding, error)
+	Run(ctx context.Context, cfg *Config) ([]Finding, error)
 }
 
+// Engine holds the configuration and registered modules.
+// Engine is not safe for concurrent use (Register must complete before Run).
 type Engine struct {
 	Config  *Config
+	mu      sync.RWMutex
 	modules []Module
 }
 
 func New(cfg *Config) *Engine {
 	return &Engine{Config: cfg}
 }
 
+// Register adds a module to the engine.
+// Register must not be called concurrently with Run or other Register calls.
 func (e *Engine) Register(m Module) {
+	e.mu.Lock()
+	defer e.mu.Unlock()
 	e.modules = append(e.modules, m)
 }
 
-func (e *Engine) Run() ScoreResult {
+// Run executes all registered modules concurrently and returns the scan result.
+// Module errors are aggregated in ScoreResult.Errors; Run itself only returns an
+// error for fatal configuration problems.
+func (e *Engine) Run() (ScoreResult, error) {
+	if e == nil || e.Config == nil {
+		return ScoreResult{}, fmt.Errorf("engine: nil config")
+	}
+
+	threads := e.Config.Threads
+	if threads < 1 {
+		threads = 1
+	}
+
 	start := time.Now()
 	fmt.Printf("\n  ⚡ VX Security Scanner v0.1.0\n")
 	fmt.Printf("  Target: %s\n", e.Config.TargetURL)
 	fmt.Printf("  Modules: %d loaded\n\n", len(e.modules))
 
 	var (
 		allFindings []Finding
+		allErrors   []ModuleError
 		mu          sync.Mutex
 		wg          sync.WaitGroup
 	)
 
-	sem := make(chan struct{}, e.Config.Threads)
+	sem := make(chan struct{}, threads)
+
+	e.mu.RLock()
+	mods := make([]Module, len(e.modules))
+	copy(mods, e.modules)
+	e.mu.RUnlock()
 
-	for _, mod := range e.modules {
+	for _, mod := range mods {
 		if !e.shouldRun(mod) {
 			continue
 		}
@@ -54,11 +84,17 @@ func (e *Engine) Run() ScoreResult {
 			fmt.Printf("  [~] Running %s...\n", m.Name())
 			modStart := time.Now()
 
-			findings, err := m.Run(e.Config)
+			ctx, cancel := context.WithTimeout(context.Background(), defaultModuleTimeout)
+			defer cancel()
+
+			findings, err := m.Run(ctx, e.Config)
 			elapsed := time.Since(modStart)
 
 			if err != nil {
 				fmt.Printf("  [!] %s failed: %v (%s)\n", m.Name(), err, elapsed.Round(time.Millisecond))
+				mu.Lock()
+				allErrors = append(allErrors, ModuleError{Module: m.Name(), Err: err})
+				mu.Unlock()
 				return
 			}
 
@@ -75,7 +111,9 @@ func (e *Engine) Run() ScoreResult {
 	elapsed := time.Since(start)
 	fmt.Printf("\n  Scan completed in %s\n\n", elapsed.Round(time.Millisecond))
 
-	return ComputeScore(allFindings)
+	result := ComputeScore(allFindings)
+	result.Errors = allErrors
+	return result, nil
 }
 
 func (e *Engine) shouldRun(m Module) bool {

pkg/engine/finding.go

@@ -2,6 +2,12 @@ package engine
 
 import "fmt"
 
+// ModuleError records a module name and its execution error.
+type ModuleError struct {
+	Module string
+	Err    error
+}
+
 type Severity int
 
 const (

pkg/engine/score.go

@@ -11,10 +11,13 @@ const (
 )
 
 type ScoreResult struct {
-	Score    int      `json:"score"`
-	Grade    Grade    `json:"grade"`
-	Findings []Finding `json:"findings"`
+	Score    int              `json:"score"`
+	Grade    Grade            `json:"grade"`
+	Findings []Finding        `json:"findings"`
 	Summary  map[Severity]int `json:"summary"`
+	// Errors holds per-module errors encountered during the scan.
+	// A non-empty Errors slice means the scan is partial.
+	Errors []ModuleError `json:"errors,omitempty"`
 }
 
 func ComputeScore(findings []Finding) ScoreResult {

pkg/history/history.go

@@ -47,12 +47,21 @@ func dir() (string, error) {
 		return "", fmt.Errorf("get home dir: %w", err)
 	}
 	d := filepath.Join(home, ".vx", "scans")
-	if err := os.MkdirAll(d, 0755); err != nil {
+	if err := os.MkdirAll(d, 0700); err != nil {
 		return "", fmt.Errorf("create history dir: %w", err)
 	}
 	return d, nil
 }
 
+// safePath joins filename to dir and rejects paths that escape dir.
+func safePath(dir, filename string) (string, error) {
+	cleaned := filepath.Clean(filename)
+	if filepath.IsAbs(cleaned) || strings.HasPrefix(cleaned, "..") || strings.ContainsRune(cleaned, filepath.Separator) {
+		return "", fmt.Errorf("invalid filename: %q", filename)
+	}
+	return filepath.Join(dir, cleaned), nil
+}
+
 // SaveScan persists a scan result to ~/.vx/scans/.
 func SaveScan(result engine.ScoreResult, target string, duration time.Duration) error {
 	d, err := dir()
@@ -76,7 +85,7 @@ func SaveScan(result engine.ScoreResult, target string, duration time.Duration)
 		return fmt.Errorf("marshal scan: %w", err)
 	}
 
-	return os.WriteFile(filepath.Join(d, filename), data, 0644)
+	return os.WriteFile(filepath.Join(d, filename), data, 0600)
 }
 
 // ListScans returns all saved scans sorted by date (newest first).
@@ -132,7 +141,12 @@ func LoadScan(filename string) (*StoredScan, error) {
 		return nil, err
 	}
 
-	data, err := os.ReadFile(filepath.Join(d, filename))
+	path, err := safePath(d, filename)
+	if err != nil {
+		return nil, err
+	}
+
+	data, err := os.ReadFile(path)
 	if err != nil {
 		return nil, fmt.Errorf("read scan file: %w", err)
 	}