fixes

.gitignore

@@ -43,7 +43,7 @@ config.yaml
 
 # Cache
 /var/cache/gohoarder/
-cache/
+/cache/
 
 # Logs
 *.log

pkg/app/app.go

@@ -249,7 +249,7 @@ func (a *App) setupServer() error {
 	a.app.Get("/api/info", a.handleInfo)
 
 	// Admin endpoints (bypass management)
-	a.app.All("/api/admin/bypasses/:id?", a.handleAdminBypasses)
+	a.app.All("/api/admin/bypasses/:id?", a.requireAdmin, a.handleAdminBypasses)
 
 	// Proxy handlers (adapted from net/http)
 	// Load git credentials if configured

pkg/cache/cache.go

@@ -0,0 +1,572 @@
+package cache
+
+import (
+	"bytes"
+	"context"
+	"crypto/sha256"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"sync"
+	"time"
+
+	"github.com/lukaszraczylo/gohoarder/pkg/errors"
+	"github.com/lukaszraczylo/gohoarder/pkg/metadata"
+	"github.com/lukaszraczylo/gohoarder/pkg/metrics"
+	"github.com/lukaszraczylo/gohoarder/pkg/storage"
+	"github.com/lukaszraczylo/gohoarder/pkg/uuid"
+	"github.com/rs/zerolog/log"
+	"golang.org/x/sync/singleflight"
+)
+
+// ScannerInterface defines the interface for security scanners
+// Defined here to avoid circular dependency with scanner package
+type ScannerInterface interface {
+	ScanPackage(ctx context.Context, registry, packageName, version string, filePath string) error
+	CheckVulnerabilities(ctx context.Context, registry, packageName, version string) (blocked bool, reason string, err error)
+}
+
+// Manager coordinates caching operations between storage and metadata
+type Manager struct {
+	storage  storage.StorageBackend
+	metadata metadata.MetadataStore
+	scanner  ScannerInterface
+	config   Config
+	sf       singleflight.Group
+	mu       sync.RWMutex
+	evicting bool
+}
+
+// Config holds cache manager configuration
+type Config struct {
+	DefaultTTL        time.Duration // Default TTL for cached packages
+	CleanupInterval   time.Duration // How often to run cleanup
+	EvictionThreshold float64       // Trigger eviction when usage > threshold (0.0-1.0)
+	MaxConcurrent     int           // Max concurrent upstream fetches
+}
+
+// CacheEntry represents a cached package
+type CacheEntry struct {
+	Package      *metadata.Package
+	Data         io.ReadCloser
+	FromCache    bool
+	UpstreamURL  string
+	CacheControl string
+}
+
+// New creates a new cache manager
+func New(storage storage.StorageBackend, metadata metadata.MetadataStore, scanner ScannerInterface, config Config) (*Manager, error) {
+	if storage == nil {
+		return nil, errors.New(errors.ErrCodeInvalidConfig, "storage backend is required")
+	}
+
+	if metadata == nil {
+		return nil, errors.New(errors.ErrCodeInvalidConfig, "metadata store is required")
+	}
+
+	// Scanner is optional - can be nil if security scanning is disabled
+	if scanner != nil {
+		log.Info().Msg("Cache manager initialized with security scanning enabled")
+	}
+
+	if config.DefaultTTL == 0 {
+		config.DefaultTTL = 7 * 24 * time.Hour // 7 days default
+	}
+
+	if config.CleanupInterval == 0 {
+		config.CleanupInterval = 1 * time.Hour
+	}
+
+	if config.EvictionThreshold == 0 {
+		config.EvictionThreshold = 0.9 // 90% full
+	}
+
+	if config.MaxConcurrent == 0 {
+		config.MaxConcurrent = 100
+	}
+
+	manager := &Manager{
+		storage:  storage,
+		metadata: metadata,
+		scanner:  scanner,
+		config:   config,
+	}
+
+	// Start background cleanup worker
+	go manager.cleanupWorker()
+
+	return manager, nil
+}
+
+// Get retrieves a package from cache or upstream
+func (m *Manager) Get(ctx context.Context, registry, name, version string, fetchFunc func(context.Context) (io.ReadCloser, string, error)) (*CacheEntry, error) {
+	// Use singleflight to deduplicate concurrent requests
+	key := fmt.Sprintf("%s/%s/%s", registry, name, version)
+
+	result, err, _ := m.sf.Do(key, func() (interface{}, error) {
+		return m.getOrFetch(ctx, registry, name, version, fetchFunc)
+	})
+
+	if err != nil {
+		return nil, err
+	}
+
+	return result.(*CacheEntry), nil
+}
+
+// getOrFetch implements the actual get-or-fetch logic
+func (m *Manager) getOrFetch(ctx context.Context, registry, name, version string, fetchFunc func(context.Context) (io.ReadCloser, string, error)) (*CacheEntry, error) {
+	// Check metadata first
+	pkg, err := m.metadata.GetPackage(ctx, registry, name, version)
+	if err == nil {
+		// Package found in metadata, check if expired
+		if pkg.ExpiresAt != nil && time.Now().After(*pkg.ExpiresAt) {
+			log.Debug().Str("package", name).Str("version", version).Msg("Package expired, re-fetching")
+			metrics.RecordCacheEviction("ttl")
+			// Delete expired package
+			m.deletePackage(ctx, pkg)
+		} else {
+			// Try to get from storage
+			data, err := m.storage.Get(ctx, pkg.StorageKey)
+			if err == nil {
+				// Cache hit!
+				metrics.RecordCacheHit(registry)
+				m.metadata.UpdateDownloadCount(ctx, registry, name, version)
+
+				// Check for vulnerabilities if scanner is enabled
+				if m.scanner != nil {
+					blocked, reason, err := m.scanner.CheckVulnerabilities(ctx, registry, name, version)
+					if err != nil {
+						log.Warn().Err(err).Str("package", name).Msg("Failed to check vulnerabilities")
+					}
+					if blocked {
+						metrics.RecordCacheHit(registry) // Record as blocked
+						data.Close()                     // Close the data reader
+						return nil, errors.New(errors.ErrCodeSecurityViolation, reason)
+					}
+				}
+
+				return &CacheEntry{
+					Package:   pkg,
+					Data:      data,
+					FromCache: true,
+				}, nil
+			}
+
+			// Storage miss but metadata exists - inconsistency, clean up
+			log.Warn().Str("package", name).Str("version", version).Msg("Metadata exists but storage missing")
+			m.metadata.DeletePackage(ctx, registry, name, version)
+		}
+	}
+
+	// Cache miss - fetch from upstream
+	metrics.RecordCacheMiss(registry)
+
+	if fetchFunc == nil {
+		return nil, errors.NotFound(fmt.Sprintf("package not found and no fetch function provided: %s/%s@%s", registry, name, version))
+	}
+
+	log.Debug().Str("package", name).Str("version", version).Msg("Fetching from upstream")
+
+	// Fetch from upstream
+	data, upstreamURL, err := fetchFunc(ctx)
+	if err != nil {
+		metrics.RecordUpstreamRequest(registry, "error")
+		return nil, errors.Wrap(err, errors.ErrCodeUpstreamFailure, "failed to fetch from upstream")
+	}
+	defer data.Close()
+
+	metrics.RecordUpstreamRequest(registry, "success")
+
+	// Store in cache (this will also trigger background scan)
+	storedPkg, err := m.store(ctx, registry, name, version, data, upstreamURL)
+	if err != nil {
+		return nil, err
+	}
+
+	// Wait briefly for initial scan to complete if scanner is enabled
+	// This prevents serving vulnerable packages on first request
+	if m.scanner != nil {
+		// Wait up to 30 seconds for scan to complete
+		scanCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
+		defer cancel()
+
+		ticker := time.NewTicker(100 * time.Millisecond)
+		defer ticker.Stop()
+
+		for {
+			select {
+			case <-scanCtx.Done():
+				// Timeout or context cancelled - proceed anyway
+				// Package is cached, will be blocked on next request if vulnerable
+				log.Warn().
+					Str("package", name).
+					Str("version", version).
+					Msg("Scan timeout - allowing first download, will block on subsequent requests if vulnerable")
+				goto servePkg
+
+			case <-ticker.C:
+				// First check if scan has completed by checking the SecurityScanned flag
+				// This prevents race condition where CheckVulnerabilities() returns "clean"
+				// before all scanners have finished
+				pkg, err := m.metadata.GetPackage(scanCtx, registry, name, version)
+				if err != nil {
+					// Failed to get package metadata - continue waiting
+					log.Debug().
+						Str("package", name).
+						Str("version", version).
+						Err(err).
+						Msg("Failed to get package metadata, waiting...")
+					continue
+				}
+
+				if !pkg.SecurityScanned {
+					// Scan still in progress - continue waiting
+					log.Debug().
+						Str("package", name).
+						Str("version", version).
+						Msg("Scan in progress, waiting...")
+					continue
+				}
+
+				// Scan completed - now check if package should be blocked
+				blocked, reason, err := m.scanner.CheckVulnerabilities(scanCtx, registry, name, version)
+				if err != nil {
+					// Unexpected error after scan complete - log and continue waiting
+					log.Warn().
+						Str("package", name).
+						Str("version", version).
+						Err(err).
+						Msg("Error checking vulnerabilities, waiting...")
+					continue
+				}
+
+				// Scan completed - check if blocked
+				if blocked {
+					log.Info().
+						Str("package", name).
+						Str("version", version).
+						Str("reason", reason).
+						Msg("Package cached but blocked due to vulnerabilities")
+					return nil, errors.New(errors.ErrCodeSecurityViolation, reason)
+				}
+
+				// Package is clean - proceed to serve
+				log.Info().
+					Str("package", name).
+					Str("version", version).
+					Msg("Scan completed, package is clean")
+				goto servePkg
+			}
+		}
+	}
+
+servePkg:
+	// Re-open from storage for consistency
+	storedData, err := m.storage.Get(ctx, storedPkg.StorageKey)
+	if err != nil {
+		return nil, errors.Wrap(err, errors.ErrCodeStorageFailure, "failed to retrieve just-stored package")
+	}
+
+	return &CacheEntry{
+		Package:     storedPkg,
+		Data:        storedData,
+		FromCache:   false,
+		UpstreamURL: upstreamURL,
+	}, nil
+}
+
+// store stores a package in cache
+func (m *Manager) store(ctx context.Context, registry, name, version string, data io.ReadCloser, upstreamURL string) (*metadata.Package, error) {
+	// Generate storage key
+	storageKey := m.generateStorageKey(registry, name, version)
+
+	// Calculate checksums while storing
+	// We need to read the data, calculate checksums, and store it
+	// This requires buffering the data
+	var buf []byte
+	var err error
+
+	// Read all data
+	buf, err = io.ReadAll(data)
+	if err != nil {
+		return nil, errors.Wrap(err, errors.ErrCodeUpstreamFailure, "failed to read upstream data")
+	}
+
+	// Calculate checksums
+	h := sha256.New()
+	h.Write(buf)
+	checksumSHA256 := fmt.Sprintf("%x", h.Sum(nil))
+
+	size := int64(len(buf))
+
+	// Check quota before storing
+	quota, err := m.storage.GetQuota(ctx)
+	if err == nil && quota.Limit > 0 {
+		if quota.Used+size > quota.Limit {
+			// Trigger eviction
+			if err := m.evict(ctx, size); err != nil {
+				return nil, errors.QuotaExceeded(quota.Limit)
+			}
+		}
+	}
+
+	// Store in storage backend
+	opts := &storage.PutOptions{
+		ChecksumSHA256: checksumSHA256,
+	}
+
+	err = m.storage.Put(ctx, storageKey, io.NopCloser(bytes.NewReader(buf)), opts)
+	if err != nil {
+		return nil, err
+	}
+
+	// Create metadata entry
+	now := time.Now()
+	expiresAt := now.Add(m.config.DefaultTTL)
+
+	pkg := &metadata.Package{
+		ID:             uuid.New().String(),
+		Registry:       registry,
+		Name:           name,
+		Version:        version,
+		StorageKey:     storageKey,
+		Size:           size,
+		ChecksumSHA256: checksumSHA256,
+		UpstreamURL:    upstreamURL,
+		CachedAt:       now,
+		LastAccessed:   now,
+		ExpiresAt:      &expiresAt,
+		DownloadCount:  0,
+		Metadata:       make(map[string]string),
+	}
+
+	// Save metadata
+	if err := m.metadata.SavePackage(ctx, pkg); err != nil {
+		// Clean up storage if metadata save fails
+		m.storage.Delete(ctx, storageKey)
+		return nil, err
+	}
+
+	// Scan package if scanner is enabled (run in background to not block cache operations)
+	if m.scanner != nil {
+		go func() {
+			scanCtx := context.Background()
+			var filePath string
+			var cleanupFunc func()
+
+			// Check if storage backend supports local paths
+			if localProvider, ok := m.storage.(interface {
+				GetLocalPath(ctx context.Context, key string) (string, error)
+			}); ok {
+				// Use direct file path from storage (avoid double download)
+				path, err := localProvider.GetLocalPath(scanCtx, storageKey)
+				if err != nil {
+					log.Error().Err(err).Str("package", name).Msg("Failed to get local path for scanning")
+					return
+				}
+				filePath = path
+				cleanupFunc = func() {} // No cleanup needed for direct path
+				log.Debug().Str("package", name).Str("path", filePath).Msg("Scanning package from storage path")
+			} else {
+				// Fallback: Create temp file for remote storage (S3, SMB, etc.)
+				tempFilePath := filepath.Join(os.TempDir(), storageKey)
+
+				// Create parent directories if they don't exist
+				if err := os.MkdirAll(filepath.Dir(tempFilePath), 0755); err != nil {
+					log.Error().Err(err).Str("package", name).Msg("Failed to create temp directory for scanning")
+					return
+				}
+
+				tempFile, err := os.Create(tempFilePath)
+				if err != nil {
+					log.Error().Err(err).Str("package", name).Msg("Failed to create temp file for scanning")
+					return
+				}
+
+				// Write package data to temp file
+				if _, err := tempFile.Write(buf); err != nil {
+					tempFile.Close()
+					os.Remove(tempFilePath)
+					log.Error().Err(err).Str("package", name).Msg("Failed to write temp file for scanning")
+					return
+				}
+				tempFile.Close()
+
+				filePath = tempFilePath
+				cleanupFunc = func() { os.Remove(tempFilePath) }
+				log.Debug().Str("package", name).Str("path", filePath).Msg("Scanning package from temp file")
+			}
+
+			defer cleanupFunc()
+
+			// Scan package
+			if err := m.scanner.ScanPackage(scanCtx, registry, name, version, filePath); err != nil {
+				log.Error().Err(err).Str("package", name).Msg("Failed to scan package")
+			}
+		}()
+	}
+
+	return pkg, nil
+}
+
+// Delete removes a package from cache
+func (m *Manager) Delete(ctx context.Context, registry, name, version string) error {
+	pkg, err := m.metadata.GetPackage(ctx, registry, name, version)
+	if err != nil {
+		return err
+	}
+
+	return m.deletePackage(ctx, pkg)
+}
+
+// deletePackage deletes a package from both storage and metadata
+func (m *Manager) deletePackage(ctx context.Context, pkg *metadata.Package) error {
+	// Delete from storage
+	if err := m.storage.Delete(ctx, pkg.StorageKey); err != nil {
+		log.Warn().Err(err).Str("key", pkg.StorageKey).Msg("Failed to delete from storage")
+	}
+
+	// Delete from metadata
+	return m.metadata.DeletePackage(ctx, pkg.Registry, pkg.Name, pkg.Version)
+}
+
+// evict implements LRU eviction
+func (m *Manager) evict(ctx context.Context, needed int64) error {
+	m.mu.Lock()
+	if m.evicting {
+		m.mu.Unlock()
+		return errors.New(errors.ErrCodeStorageFailure, "eviction already in progress")
+	}
+	m.evicting = true
+	m.mu.Unlock()
+
+	defer func() {
+		m.mu.Lock()
+		m.evicting = false
+		m.mu.Unlock()
+	}()
+
+	log.Info().Int64("needed", needed).Msg("Starting LRU eviction")
+
+	// List packages sorted by last accessed (oldest first)
+	opts := &metadata.ListOptions{
+		SortBy:   "last_accessed",
+		SortDesc: false,
+		Limit:    100,
+	}
+
+	var freed int64
+	for freed < needed {
+		packages, err := m.metadata.ListPackages(ctx, opts)
+		if err != nil || len(packages) == 0 {
+			break
+		}
+
+		for _, pkg := range packages {
+			if err := m.deletePackage(ctx, pkg); err != nil {
+				log.Warn().Err(err).Str("package", pkg.Name).Msg("Failed to evict package")
+				continue
+			}
+
+			freed += pkg.Size
+			metrics.RecordCacheEviction("lru")
+
+			if freed >= needed {
+				break
+			}
+		}
+
+		if len(packages) < opts.Limit {
+			break // No more packages
+		}
+	}
+
+	log.Info().Int64("freed", freed).Msg("Eviction completed")
+	return nil
+}
+
+// cleanupWorker runs periodic cleanup of expired packages
+func (m *Manager) cleanupWorker() {
+	ticker := time.NewTicker(m.config.CleanupInterval)
+	defer ticker.Stop()
+
+	for range ticker.C {
+		ctx := context.Background()
+		m.cleanup(ctx)
+	}
+}
+
+// cleanup removes expired packages
+func (m *Manager) cleanup(ctx context.Context) {
+	log.Debug().Msg("Starting cleanup worker")
+
+	// List all packages
+	packages, err := m.metadata.ListPackages(ctx, &metadata.ListOptions{})
+	if err != nil {
+		log.Error().Err(err).Msg("Failed to list packages for cleanup")
+		return
+	}
+
+	now := time.Now()
+	var cleaned int
+
+	for _, pkg := range packages {
+		if pkg.ExpiresAt != nil && now.After(*pkg.ExpiresAt) {
+			if err := m.deletePackage(ctx, pkg); err != nil {
+				log.Warn().Err(err).Str("package", pkg.Name).Msg("Failed to clean up expired package")
+				continue
+			}
+			cleaned++
+		}
+	}
+
+	if cleaned > 0 {
+		log.Info().Int("count", cleaned).Msg("Cleanup completed")
+	}
+}
+
+// generateStorageKey generates a storage key for a package
+func (m *Manager) generateStorageKey(registry, name, version string) string {
+	return fmt.Sprintf("%s/%s/%s", registry, name, version)
+}
+
+// GetStats returns cache statistics
+func (m *Manager) GetStats(ctx context.Context, registry string) (*metadata.Stats, error) {
+	return m.metadata.GetStats(ctx, registry)
+}
+
+// Health checks cache manager health
+func (m *Manager) Health(ctx context.Context) error {
+	// Check storage health
+	if err := m.storage.Health(ctx); err != nil {
+		return errors.Wrap(err, errors.ErrCodeStorageFailure, "storage health check failed")
+	}
+
+	// Check metadata health
+	if err := m.metadata.Health(ctx); err != nil {
+		return errors.Wrap(err, errors.ErrCodeDatabaseFailure, "metadata health check failed")
+	}
+
+	return nil
+}
+
+// Close closes the cache manager
+func (m *Manager) Close() error {
+	var err error
+
+	if closeErr := m.storage.Close(); closeErr != nil {
+		err = closeErr
+	}
+
+	if closeErr := m.metadata.Close(); closeErr != nil {
+		if err != nil {
+			err = fmt.Errorf("%w; %w", err, closeErr)
+		} else {
+			err = closeErr
+		}
+	}
+
+	return err
+}

pkg/cache/cache_test.go

@@ -0,0 +1,980 @@
+package cache
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"io"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/lukaszraczylo/gohoarder/pkg/metadata"
+	"github.com/lukaszraczylo/gohoarder/pkg/storage"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+// MockStorageBackend is a mock for storage.StorageBackend
+type MockStorageBackend struct {
+	mock.Mock
+}
+
+func (m *MockStorageBackend) Get(ctx context.Context, key string) (io.ReadCloser, error) {
+	args := m.Called(ctx, key)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(io.ReadCloser), args.Error(1)
+}
+
+func (m *MockStorageBackend) Put(ctx context.Context, key string, data io.Reader, opts *storage.PutOptions) error {
+	args := m.Called(ctx, key, data, opts)
+	return args.Error(0)
+}
+
+func (m *MockStorageBackend) Delete(ctx context.Context, key string) error {
+	args := m.Called(ctx, key)
+	return args.Error(0)
+}
+
+func (m *MockStorageBackend) Exists(ctx context.Context, key string) (bool, error) {
+	args := m.Called(ctx, key)
+	return args.Bool(0), args.Error(1)
+}
+
+func (m *MockStorageBackend) List(ctx context.Context, prefix string, opts *storage.ListOptions) ([]storage.StorageObject, error) {
+	args := m.Called(ctx, prefix, opts)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).([]storage.StorageObject), args.Error(1)
+}
+
+func (m *MockStorageBackend) Stat(ctx context.Context, key string) (*storage.StorageInfo, error) {
+	args := m.Called(ctx, key)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(*storage.StorageInfo), args.Error(1)
+}
+
+func (m *MockStorageBackend) GetQuota(ctx context.Context) (*storage.QuotaInfo, error) {
+	args := m.Called(ctx)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(*storage.QuotaInfo), args.Error(1)
+}
+
+func (m *MockStorageBackend) Health(ctx context.Context) error {
+	args := m.Called(ctx)
+	return args.Error(0)
+}
+
+func (m *MockStorageBackend) Close() error {
+	args := m.Called()
+	return args.Error(0)
+}
+
+// MockMetadataStore is a mock for metadata.MetadataStore
+type MockMetadataStore struct {
+	mock.Mock
+}
+
+func (m *MockMetadataStore) SavePackage(ctx context.Context, pkg *metadata.Package) error {
+	args := m.Called(ctx, pkg)
+	return args.Error(0)
+}
+
+func (m *MockMetadataStore) GetPackage(ctx context.Context, registry, name, version string) (*metadata.Package, error) {
+	args := m.Called(ctx, registry, name, version)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(*metadata.Package), args.Error(1)
+}
+
+func (m *MockMetadataStore) DeletePackage(ctx context.Context, registry, name, version string) error {
+	args := m.Called(ctx, registry, name, version)
+	return args.Error(0)
+}
+
+func (m *MockMetadataStore) ListPackages(ctx context.Context, opts *metadata.ListOptions) ([]*metadata.Package, error) {
+	args := m.Called(ctx, opts)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).([]*metadata.Package), args.Error(1)
+}
+
+func (m *MockMetadataStore) UpdateDownloadCount(ctx context.Context, registry, name, version string) error {
+	args := m.Called(ctx, registry, name, version)
+	return args.Error(0)
+}
+
+func (m *MockMetadataStore) GetStats(ctx context.Context, registry string) (*metadata.Stats, error) {
+	args := m.Called(ctx, registry)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(*metadata.Stats), args.Error(1)
+}
+
+func (m *MockMetadataStore) SaveScanResult(ctx context.Context, result *metadata.ScanResult) error {
+	args := m.Called(ctx, result)
+	return args.Error(0)
+}
+
+func (m *MockMetadataStore) GetScanResult(ctx context.Context, registry, name, version string) (*metadata.ScanResult, error) {
+	args := m.Called(ctx, registry, name, version)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(*metadata.ScanResult), args.Error(1)
+}
+
+func (m *MockMetadataStore) Count(ctx context.Context) (int, error) {
+	args := m.Called(ctx)
+	return args.Int(0), args.Error(1)
+}
+
+func (m *MockMetadataStore) Health(ctx context.Context) error {
+	args := m.Called(ctx)
+	return args.Error(0)
+}
+
+func (m *MockMetadataStore) Close() error {
+	args := m.Called()
+	return args.Error(0)
+}
+
+func (m *MockMetadataStore) SaveCVEBypass(ctx context.Context, bypass *metadata.CVEBypass) error {
+	args := m.Called(ctx, bypass)
+	return args.Error(0)
+}
+
+func (m *MockMetadataStore) GetActiveCVEBypasses(ctx context.Context) ([]*metadata.CVEBypass, error) {
+	args := m.Called(ctx)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).([]*metadata.CVEBypass), args.Error(1)
+}
+
+func (m *MockMetadataStore) ListCVEBypasses(ctx context.Context, opts *metadata.BypassListOptions) ([]*metadata.CVEBypass, error) {
+	args := m.Called(ctx, opts)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).([]*metadata.CVEBypass), args.Error(1)
+}
+
+func (m *MockMetadataStore) DeleteCVEBypass(ctx context.Context, id string) error {
+	args := m.Called(ctx, id)
+	return args.Error(0)
+}
+
+func (m *MockMetadataStore) CleanupExpiredBypasses(ctx context.Context) (int, error) {
+	args := m.Called(ctx)
+	return args.Int(0), args.Error(1)
+}
+
+func (m *MockMetadataStore) GetTimeSeriesStats(ctx context.Context, period string, registry string) (*metadata.TimeSeriesStats, error) {
+	args := m.Called(ctx, period, registry)
+	if args.Get(0) == nil {
+		return nil, args.Error(1)
+	}
+	return args.Get(0).(*metadata.TimeSeriesStats), args.Error(1)
+}
+
+func (m *MockMetadataStore) AggregateDownloadData(ctx context.Context) error {
+	args := m.Called(ctx)
+	return args.Error(0)
+}
+
+// TestNew tests cache manager creation
+func TestNew(t *testing.T) {
+	tests := []struct {
+		name        string
+		storage     storage.StorageBackend
+		metadata    metadata.MetadataStore
+		config      Config
+		wantErr     bool
+		errContains string
+	}{
+		// GOOD: Valid configuration
+		{
+			name:     "valid config with defaults",
+			storage:  &MockStorageBackend{},
+			metadata: &MockMetadataStore{},
+			config:   Config{},
+			wantErr:  false,
+		},
+		{
+			name:     "valid config with custom settings",
+			storage:  &MockStorageBackend{},
+			metadata: &MockMetadataStore{},
+			config: Config{
+				DefaultTTL:        24 * time.Hour,
+				CleanupInterval:   30 * time.Minute,
+				EvictionThreshold: 0.8,
+				MaxConcurrent:     50,
+			},
+			wantErr: false,
+		},
+		// WRONG: Missing required components
+		{
+			name:        "nil storage",
+			storage:     nil,
+			metadata:    &MockMetadataStore{},
+			config:      Config{},
+			wantErr:     true,
+			errContains: "storage backend is required",
+		},
+		{
+			name:        "nil metadata",
+			storage:     &MockStorageBackend{},
+			metadata:    nil,
+			config:      Config{},
+			wantErr:     true,
+			errContains: "metadata store is required",
+		},
+		// EDGE: Both nil
+		{
+			name:        "both nil",
+			storage:     nil,
+			metadata:    nil,
+			config:      Config{},
+			wantErr:     true,
+			errContains: "storage backend is required",
+		},
+		// EDGE: Zero values get defaults
+		{
+			name:     "zero config gets defaults",
+			storage:  &MockStorageBackend{},
+			metadata: &MockMetadataStore{},
+			config:   Config{},
+			wantErr:  false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			manager, err := New(tt.storage, tt.metadata, nil, tt.config)
+
+			if tt.wantErr {
+				require.Error(t, err)
+				if tt.errContains != "" {
+					assert.Contains(t, err.Error(), tt.errContains)
+				}
+				assert.Nil(t, manager)
+			} else {
+				require.NoError(t, err)
+				require.NotNil(t, manager)
+
+				// Verify defaults were set
+				if tt.config.DefaultTTL == 0 {
+					assert.Equal(t, 7*24*time.Hour, manager.config.DefaultTTL)
+				}
+				if tt.config.CleanupInterval == 0 {
+					assert.Equal(t, 1*time.Hour, manager.config.CleanupInterval)
+				}
+				if tt.config.EvictionThreshold == 0 {
+					assert.Equal(t, 0.9, manager.config.EvictionThreshold)
+				}
+				if tt.config.MaxConcurrent == 0 {
+					assert.Equal(t, 100, manager.config.MaxConcurrent)
+				}
+			}
+		})
+	}
+}
+
+// TestGet tests cache retrieval with various scenarios
+func TestGet(t *testing.T) {
+	tests := []struct {
+		name          string
+		registry      string
+		packageName   string
+		version       string
+		setupMock     func(*MockStorageBackend, *MockMetadataStore)
+		fetchFunc     func(context.Context) (io.ReadCloser, string, error)
+		wantFromCache bool
+		wantErr       bool
+		errContains   string
+	}{
+		// GOOD: Cache hit
+		{
+			name:        "cache hit - package exists and valid",
+			registry:    "npm",
+			packageName: "react",
+			version:     "18.2.0",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				now := time.Now()
+				expiresAt := now.Add(24 * time.Hour)
+				pkg := &metadata.Package{
+					ID:           "test-id",
+					Registry:     "npm",
+					Name:         "react",
+					Version:      "18.2.0",
+					StorageKey:   "npm/react/18.2.0",
+					CachedAt:     now,
+					LastAccessed: now,
+					ExpiresAt:    &expiresAt,
+				}
+				m.On("GetPackage", mock.Anything, "npm", "react", "18.2.0").Return(pkg, nil)
+				s.On("Get", mock.Anything, "npm/react/18.2.0").Return(io.NopCloser(strings.NewReader("cached data")), nil)
+				m.On("UpdateDownloadCount", mock.Anything, "npm", "react", "18.2.0").Return(nil)
+			},
+			wantFromCache: true,
+			wantErr:       false,
+		},
+		// GOOD: Cache miss - fetch from upstream
+		{
+			name:        "cache miss - fetch from upstream",
+			registry:    "npm",
+			packageName: "lodash",
+			version:     "4.17.21",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				m.On("GetPackage", mock.Anything, "npm", "lodash", "4.17.21").Return(nil, errors.New("not found"))
+				s.On("GetQuota", mock.Anything).Return(&storage.QuotaInfo{Used: 100, Available: 900, Limit: 1000}, nil)
+				s.On("Put", mock.Anything, "npm/lodash/4.17.21", mock.Anything, mock.Anything).Return(nil)
+				m.On("SavePackage", mock.Anything, mock.Anything).Return(nil)
+				s.On("Get", mock.Anything, "npm/lodash/4.17.21").Return(io.NopCloser(strings.NewReader("upstream data")), nil)
+			},
+			fetchFunc: func(ctx context.Context) (io.ReadCloser, string, error) {
+				return io.NopCloser(strings.NewReader("upstream data")), "https://registry.npmjs.org/lodash", nil
+			},
+			wantFromCache: false,
+			wantErr:       false,
+		},
+		// WRONG: Expired package
+		{
+			name:        "expired package - re-fetch",
+			registry:    "npm",
+			packageName: "expired-pkg",
+			version:     "1.0.0",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				now := time.Now()
+				expiresAt := now.Add(-1 * time.Hour) // Expired 1 hour ago
+				pkg := &metadata.Package{
+					ID:         "test-id",
+					Registry:   "npm",
+					Name:       "expired-pkg",
+					Version:    "1.0.0",
+					StorageKey: "npm/expired-pkg/1.0.0",
+					ExpiresAt:  &expiresAt,
+				}
+				m.On("GetPackage", mock.Anything, "npm", "expired-pkg", "1.0.0").Return(pkg, nil)
+				m.On("DeletePackage", mock.Anything, "npm", "expired-pkg", "1.0.0").Return(nil)
+				s.On("Delete", mock.Anything, "npm/expired-pkg/1.0.0").Return(nil)
+				s.On("GetQuota", mock.Anything).Return(&storage.QuotaInfo{Used: 100, Available: 900, Limit: 1000}, nil)
+				s.On("Put", mock.Anything, "npm/expired-pkg/1.0.0", mock.Anything, mock.Anything).Return(nil)
+				m.On("SavePackage", mock.Anything, mock.Anything).Return(nil)
+				s.On("Get", mock.Anything, "npm/expired-pkg/1.0.0").Return(io.NopCloser(strings.NewReader("refreshed data")), nil)
+			},
+			fetchFunc: func(ctx context.Context) (io.ReadCloser, string, error) {
+				return io.NopCloser(strings.NewReader("refreshed data")), "https://registry.npmjs.org/expired-pkg", nil
+			},
+			wantFromCache: false,
+			wantErr:       false,
+		},
+		// BAD: Fetch function is nil and package not cached
+		{
+			name:        "nil fetch function and not cached",
+			registry:    "npm",
+			packageName: "missing",
+			version:     "1.0.0",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				m.On("GetPackage", mock.Anything, "npm", "missing", "1.0.0").Return(nil, errors.New("not found"))
+			},
+			fetchFunc:   nil,
+			wantErr:     true,
+			errContains: "package not found and no fetch function provided",
+		},
+		// BAD: Upstream fetch fails
+		{
+			name:        "upstream fetch error",
+			registry:    "npm",
+			packageName: "fail-pkg",
+			version:     "1.0.0",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				m.On("GetPackage", mock.Anything, "npm", "fail-pkg", "1.0.0").Return(nil, errors.New("not found"))
+			},
+			fetchFunc: func(ctx context.Context) (io.ReadCloser, string, error) {
+				return nil, "", errors.New("upstream error")
+			},
+			wantErr:     true,
+			errContains: "failed to fetch from upstream",
+		},
+		// EDGE: Metadata exists but storage missing
+		{
+			name:        "metadata exists but storage missing - inconsistency",
+			registry:    "npm",
+			packageName: "inconsistent",
+			version:     "1.0.0",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				now := time.Now()
+				expiresAt := now.Add(24 * time.Hour)
+				pkg := &metadata.Package{
+					ID:         "test-id",
+					Registry:   "npm",
+					Name:       "inconsistent",
+					Version:    "1.0.0",
+					StorageKey: "npm/inconsistent/1.0.0",
+					ExpiresAt:  &expiresAt,
+				}
+				m.On("GetPackage", mock.Anything, "npm", "inconsistent", "1.0.0").Return(pkg, nil)
+				// First Get fails (storage missing)
+				s.On("Get", mock.Anything, "npm/inconsistent/1.0.0").Return(nil, errors.New("not found")).Once()
+				m.On("DeletePackage", mock.Anything, "npm", "inconsistent", "1.0.0").Return(nil)
+				s.On("GetQuota", mock.Anything).Return(&storage.QuotaInfo{Used: 100, Available: 900, Limit: 1000}, nil)
+				s.On("Put", mock.Anything, "npm/inconsistent/1.0.0", mock.Anything, mock.Anything).Return(nil)
+				m.On("SavePackage", mock.Anything, mock.Anything).Return(nil)
+				// Second Get succeeds (after re-storing)
+				s.On("Get", mock.Anything, "npm/inconsistent/1.0.0").Return(io.NopCloser(strings.NewReader("recovered data")), nil).Once()
+			},
+			fetchFunc: func(ctx context.Context) (io.ReadCloser, string, error) {
+				return io.NopCloser(strings.NewReader("recovered data")), "https://registry.npmjs.org/inconsistent", nil
+			},
+			wantFromCache: false,
+			wantErr:       false,
+		},
+		// EDGE: Storage save fails
+		{
+			name:        "storage save fails",
+			registry:    "npm",
+			packageName: "save-fail",
+			version:     "1.0.0",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				m.On("GetPackage", mock.Anything, "npm", "save-fail", "1.0.0").Return(nil, errors.New("not found"))
+				s.On("GetQuota", mock.Anything).Return(&storage.QuotaInfo{Used: 100, Available: 900, Limit: 1000}, nil)
+				s.On("Put", mock.Anything, "npm/save-fail/1.0.0", mock.Anything, mock.Anything).Return(errors.New("storage error"))
+			},
+			fetchFunc: func(ctx context.Context) (io.ReadCloser, string, error) {
+				return io.NopCloser(strings.NewReader("data")), "https://registry.npmjs.org/save-fail", nil
+			},
+			wantErr:     true,
+			errContains: "storage error",
+		},
+		// EDGE: Metadata save fails (should cleanup storage)
+		{
+			name:        "metadata save fails - storage cleanup",
+			registry:    "npm",
+			packageName: "meta-fail",
+			version:     "1.0.0",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				m.On("GetPackage", mock.Anything, "npm", "meta-fail", "1.0.0").Return(nil, errors.New("not found"))
+				s.On("GetQuota", mock.Anything).Return(&storage.QuotaInfo{Used: 100, Available: 900, Limit: 1000}, nil)
+				s.On("Put", mock.Anything, "npm/meta-fail/1.0.0", mock.Anything, mock.Anything).Return(nil)
+				m.On("SavePackage", mock.Anything, mock.Anything).Return(errors.New("metadata error"))
+				s.On("Delete", mock.Anything, "npm/meta-fail/1.0.0").Return(nil)
+			},
+			fetchFunc: func(ctx context.Context) (io.ReadCloser, string, error) {
+				return io.NopCloser(strings.NewReader("data")), "https://registry.npmjs.org/meta-fail", nil
+			},
+			wantErr:     true,
+			errContains: "metadata error",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockStorage := &MockStorageBackend{}
+			mockMetadata := &MockMetadataStore{}
+
+			if tt.setupMock != nil {
+				tt.setupMock(mockStorage, mockMetadata)
+			}
+
+			manager, err := New(mockStorage, mockMetadata, nil, Config{
+				DefaultTTL:      24 * time.Hour,
+				CleanupInterval: 1 * time.Hour,
+			})
+			require.NoError(t, err)
+
+			ctx := context.Background()
+			entry, err := manager.Get(ctx, tt.registry, tt.packageName, tt.version, tt.fetchFunc)
+
+			if tt.wantErr {
+				require.Error(t, err)
+				if tt.errContains != "" {
+					assert.Contains(t, err.Error(), tt.errContains)
+				}
+				assert.Nil(t, entry)
+			} else {
+				require.NoError(t, err)
+				require.NotNil(t, entry)
+				assert.Equal(t, tt.wantFromCache, entry.FromCache)
+				assert.NotNil(t, entry.Data)
+				// Read and verify data exists
+				data, _ := io.ReadAll(entry.Data)
+				assert.NotEmpty(t, data)
+			}
+
+			mockStorage.AssertExpectations(t)
+			mockMetadata.AssertExpectations(t)
+		})
+	}
+}
+
+// TestDelete tests package deletion
+func TestDelete(t *testing.T) {
+	tests := []struct {
+		name        string
+		registry    string
+		packageName string
+		version     string
+		setupMock   func(*MockStorageBackend, *MockMetadataStore)
+		wantErr     bool
+		errContains string
+	}{
+		// GOOD: Successful deletion
+		{
+			name:        "successful deletion",
+			registry:    "npm",
+			packageName: "react",
+			version:     "18.2.0",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				pkg := &metadata.Package{
+					ID:         "test-id",
+					Registry:   "npm",
+					Name:       "react",
+					Version:    "18.2.0",
+					StorageKey: "npm/react/18.2.0",
+				}
+				m.On("GetPackage", mock.Anything, "npm", "react", "18.2.0").Return(pkg, nil)
+				s.On("Delete", mock.Anything, "npm/react/18.2.0").Return(nil)
+				m.On("DeletePackage", mock.Anything, "npm", "react", "18.2.0").Return(nil)
+			},
+			wantErr: false,
+		},
+		// WRONG: Package not found
+		{
+			name:        "package not found",
+			registry:    "npm",
+			packageName: "missing",
+			version:     "1.0.0",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				m.On("GetPackage", mock.Anything, "npm", "missing", "1.0.0").Return(nil, errors.New("not found"))
+			},
+			wantErr:     true,
+			errContains: "not found",
+		},
+		// EDGE: Storage delete fails but metadata succeeds
+		{
+			name:        "storage delete fails",
+			registry:    "npm",
+			packageName: "storage-fail",
+			version:     "1.0.0",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				pkg := &metadata.Package{
+					ID:         "test-id",
+					Registry:   "npm",
+					Name:       "storage-fail",
+					Version:    "1.0.0",
+					StorageKey: "npm/storage-fail/1.0.0",
+				}
+				m.On("GetPackage", mock.Anything, "npm", "storage-fail", "1.0.0").Return(pkg, nil)
+				s.On("Delete", mock.Anything, "npm/storage-fail/1.0.0").Return(errors.New("storage error"))
+				m.On("DeletePackage", mock.Anything, "npm", "storage-fail", "1.0.0").Return(nil)
+			},
+			wantErr: false, // Metadata delete still succeeds
+		},
+		// EDGE: Metadata delete fails
+		{
+			name:        "metadata delete fails",
+			registry:    "npm",
+			packageName: "meta-fail",
+			version:     "1.0.0",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				pkg := &metadata.Package{
+					ID:         "test-id",
+					Registry:   "npm",
+					Name:       "meta-fail",
+					Version:    "1.0.0",
+					StorageKey: "npm/meta-fail/1.0.0",
+				}
+				m.On("GetPackage", mock.Anything, "npm", "meta-fail", "1.0.0").Return(pkg, nil)
+				s.On("Delete", mock.Anything, "npm/meta-fail/1.0.0").Return(nil)
+				m.On("DeletePackage", mock.Anything, "npm", "meta-fail", "1.0.0").Return(errors.New("metadata error"))
+			},
+			wantErr:     true,
+			errContains: "metadata error",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockStorage := &MockStorageBackend{}
+			mockMetadata := &MockMetadataStore{}
+
+			if tt.setupMock != nil {
+				tt.setupMock(mockStorage, mockMetadata)
+			}
+
+			manager, err := New(mockStorage, mockMetadata, nil, Config{})
+			require.NoError(t, err)
+
+			ctx := context.Background()
+			err = manager.Delete(ctx, tt.registry, tt.packageName, tt.version)
+
+			if tt.wantErr {
+				require.Error(t, err)
+				if tt.errContains != "" {
+					assert.Contains(t, err.Error(), tt.errContains)
+				}
+			} else {
+				require.NoError(t, err)
+			}
+
+			mockStorage.AssertExpectations(t)
+			mockMetadata.AssertExpectations(t)
+		})
+	}
+}
+
+// TestHealth tests health check functionality
+func TestHealth(t *testing.T) {
+	tests := []struct {
+		name        string
+		setupMock   func(*MockStorageBackend, *MockMetadataStore)
+		wantErr     bool
+		errContains string
+	}{
+		// GOOD: Both healthy
+		{
+			name: "both storage and metadata healthy",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				s.On("Health", mock.Anything).Return(nil)
+				m.On("Health", mock.Anything).Return(nil)
+			},
+			wantErr: false,
+		},
+		// WRONG: Storage unhealthy
+		{
+			name: "storage unhealthy",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				s.On("Health", mock.Anything).Return(errors.New("storage error"))
+			},
+			wantErr:     true,
+			errContains: "storage health check failed",
+		},
+		// WRONG: Metadata unhealthy
+		{
+			name: "metadata unhealthy",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				s.On("Health", mock.Anything).Return(nil)
+				m.On("Health", mock.Anything).Return(errors.New("metadata error"))
+			},
+			wantErr:     true,
+			errContains: "metadata health check failed",
+		},
+		// BAD: Both unhealthy
+		{
+			name: "both unhealthy",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				s.On("Health", mock.Anything).Return(errors.New("storage error"))
+			},
+			wantErr:     true,
+			errContains: "storage health check failed",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockStorage := &MockStorageBackend{}
+			mockMetadata := &MockMetadataStore{}
+
+			if tt.setupMock != nil {
+				tt.setupMock(mockStorage, mockMetadata)
+			}
+
+			manager, err := New(mockStorage, mockMetadata, nil, Config{})
+			require.NoError(t, err)
+
+			ctx := context.Background()
+			err = manager.Health(ctx)
+
+			if tt.wantErr {
+				require.Error(t, err)
+				if tt.errContains != "" {
+					assert.Contains(t, err.Error(), tt.errContains)
+				}
+			} else {
+				require.NoError(t, err)
+			}
+
+			mockStorage.AssertExpectations(t)
+			mockMetadata.AssertExpectations(t)
+		})
+	}
+}
+
+// TestGetStats tests statistics retrieval
+func TestGetStats(t *testing.T) {
+	mockStorage := &MockStorageBackend{}
+	mockMetadata := &MockMetadataStore{}
+
+	expectedStats := &metadata.Stats{
+		Registry:       "npm",
+		TotalPackages:  100,
+		TotalSize:      1024 * 1024 * 100,
+		TotalDownloads: 5000,
+	}
+
+	mockMetadata.On("GetStats", mock.Anything, "npm").Return(expectedStats, nil)
+
+	manager, err := New(mockStorage, mockMetadata, nil, Config{})
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	stats, err := manager.GetStats(ctx, "npm")
+
+	require.NoError(t, err)
+	assert.Equal(t, expectedStats, stats)
+	mockMetadata.AssertExpectations(t)
+}
+
+// TestClose tests manager cleanup
+func TestClose(t *testing.T) {
+	tests := []struct {
+		name      string
+		setupMock func(*MockStorageBackend, *MockMetadataStore)
+		wantErr   bool
+	}{
+		// GOOD: Clean close
+		{
+			name: "both close successfully",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				s.On("Close").Return(nil)
+				m.On("Close").Return(nil)
+			},
+			wantErr: false,
+		},
+		// WRONG: Storage close fails
+		{
+			name: "storage close fails",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				s.On("Close").Return(errors.New("storage error"))
+				m.On("Close").Return(nil)
+			},
+			wantErr: true,
+		},
+		// WRONG: Metadata close fails
+		{
+			name: "metadata close fails",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				s.On("Close").Return(nil)
+				m.On("Close").Return(errors.New("metadata error"))
+			},
+			wantErr: true,
+		},
+		// BAD: Both close fail
+		{
+			name: "both close fail",
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				s.On("Close").Return(errors.New("storage error"))
+				m.On("Close").Return(errors.New("metadata error"))
+			},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockStorage := &MockStorageBackend{}
+			mockMetadata := &MockMetadataStore{}
+
+			if tt.setupMock != nil {
+				tt.setupMock(mockStorage, mockMetadata)
+			}
+
+			manager, err := New(mockStorage, mockMetadata, nil, Config{})
+			require.NoError(t, err)
+
+			err = manager.Close()
+
+			if tt.wantErr {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+			}
+
+			mockStorage.AssertExpectations(t)
+			mockMetadata.AssertExpectations(t)
+		})
+	}
+}
+
+// TestEvict tests LRU eviction
+func TestEvict(t *testing.T) {
+	tests := []struct {
+		name        string
+		needed      int64
+		setupMock   func(*MockStorageBackend, *MockMetadataStore)
+		wantErr     bool
+		errContains string
+	}{
+		// GOOD: Successful eviction
+		{
+			name:   "evict enough to free space",
+			needed: 200,
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				packages := []*metadata.Package{
+					{
+						ID:         "1",
+						Name:       "old-pkg-1",
+						Version:    "1.0.0",
+						Registry:   "npm",
+						StorageKey: "npm/old-pkg-1/1.0.0",
+						Size:       100,
+					},
+					{
+						ID:         "2",
+						Name:       "old-pkg-2",
+						Version:    "1.0.0",
+						Registry:   "npm",
+						StorageKey: "npm/old-pkg-2/1.0.0",
+						Size:       150,
+					},
+				}
+				m.On("ListPackages", mock.Anything, mock.MatchedBy(func(opts *metadata.ListOptions) bool {
+					return opts.SortBy == "last_accessed" && !opts.SortDesc
+				})).Return(packages, nil).Once()
+
+				s.On("Delete", mock.Anything, "npm/old-pkg-1/1.0.0").Return(nil)
+				m.On("DeletePackage", mock.Anything, "npm", "old-pkg-1", "1.0.0").Return(nil)
+				s.On("Delete", mock.Anything, "npm/old-pkg-2/1.0.0").Return(nil)
+				m.On("DeletePackage", mock.Anything, "npm", "old-pkg-2", "1.0.0").Return(nil)
+			},
+			wantErr: false,
+		},
+		// EDGE: No packages to evict
+		{
+			name:   "no packages available to evict",
+			needed: 100,
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				m.On("ListPackages", mock.Anything, mock.Anything).Return([]*metadata.Package{}, nil)
+			},
+			wantErr: false, // Doesn't error, just can't free enough
+		},
+		// EDGE: Eviction list error
+		{
+			name:   "list packages fails",
+			needed: 100,
+			setupMock: func(s *MockStorageBackend, m *MockMetadataStore) {
+				m.On("ListPackages", mock.Anything, mock.Anything).Return(nil, errors.New("list error"))
+			},
+			wantErr: false, // Doesn't error, just can't complete
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockStorage := &MockStorageBackend{}
+			mockMetadata := &MockMetadataStore{}
+
+			if tt.setupMock != nil {
+				tt.setupMock(mockStorage, mockMetadata)
+			}
+
+			manager, err := New(mockStorage, mockMetadata, nil, Config{})
+			require.NoError(t, err)
+
+			ctx := context.Background()
+			err = manager.evict(ctx, tt.needed)
+
+			if tt.wantErr {
+				require.Error(t, err)
+				if tt.errContains != "" {
+					assert.Contains(t, err.Error(), tt.errContains)
+				}
+			} else {
+				require.NoError(t, err)
+			}
+
+			mockStorage.AssertExpectations(t)
+			mockMetadata.AssertExpectations(t)
+		})
+	}
+}
+
+// TestGenerateStorageKey tests storage key generation
+func TestGenerateStorageKey(t *testing.T) {
+	mockStorage := &MockStorageBackend{}
+	mockMetadata := &MockMetadataStore{}
+
+	manager, err := New(mockStorage, mockMetadata, nil, Config{})
+	require.NoError(t, err)
+
+	tests := []struct {
+		registry string
+		name     string
+		version  string
+		expected string
+	}{
+		{"npm", "react", "18.2.0", "npm/react/18.2.0"},
+		{"pypi", "requests", "2.28.0", "pypi/requests/2.28.0"},
+		{"go", "github.com/gin-gonic/gin", "v1.9.0", "go/github.com/gin-gonic/gin/v1.9.0"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.expected, func(t *testing.T) {
+			key := manager.generateStorageKey(tt.registry, tt.name, tt.version)
+			assert.Equal(t, tt.expected, key)
+		})
+	}
+}
+
+// TestConcurrentGet tests concurrent access doesn't cause data races
+func TestConcurrentGet(t *testing.T) {
+	mockStorage := &MockStorageBackend{}
+	mockMetadata := &MockMetadataStore{}
+
+	// Setup mocks for concurrent access
+	now := time.Now()
+	expiresAt := now.Add(24 * time.Hour)
+	pkg := &metadata.Package{
+		ID:           "test-id",
+		Registry:     "npm",
+		Name:         "concurrent",
+		Version:      "1.0.0",
+		StorageKey:   "npm/concurrent/1.0.0",
+		CachedAt:     now,
+		LastAccessed: now,
+		ExpiresAt:    &expiresAt,
+	}
+
+	// Use Maybe() to allow variable number of calls due to singleflight deduplication
+	mockMetadata.On("GetPackage", mock.Anything, "npm", "concurrent", "1.0.0").Return(pkg, nil).Maybe()
+	mockStorage.On("Get", mock.Anything, "npm/concurrent/1.0.0").Return(
+		io.NopCloser(bytes.NewReader([]byte("test data"))), nil).Maybe()
+	mockMetadata.On("UpdateDownloadCount", mock.Anything, "npm", "concurrent", "1.0.0").Return(nil).Maybe()
+
+	manager, err := New(mockStorage, mockMetadata, nil, Config{})
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	const numGoroutines = 10
+
+	// Run concurrent gets
+	errs := make(chan error, numGoroutines)
+	for i := 0; i < numGoroutines; i++ {
+		go func() {
+			_, err := manager.Get(ctx, "npm", "concurrent", "1.0.0", nil)
+			errs <- err
+		}()
+	}
+
+	// Collect results
+	for i := 0; i < numGoroutines; i++ {
+		err := <-errs
+		assert.NoError(t, err)
+	}
+
+	// Verify at least one call was made (singleflight may deduplicate others)
+	mockMetadata.AssertCalled(t, "GetPackage", mock.Anything, "npm", "concurrent", "1.0.0")
+}

pkg/scanner/ghsa/ghsa.go

@@ -163,15 +163,11 @@ func (s *Scanner) queryAdvisories(ctx context.Context, ecosystem, packageName st
 
 // filterAffectedAdvisories filters advisories that affect the given version
 func (s *Scanner) filterAffectedAdvisories(advisories []GHSAAdvisory, version string) []GHSAAdvisory {
-	affected := make([]GHSAAdvisory, 0)
-
-	for _, advisory := range advisories {
 	// Check if this version is affected
 	// GitHub API already filters by package, but we need to check version ranges
 	// For now, we'll include all advisories that match the package
 	// A more sophisticated implementation would parse version ranges
-		affected = append(affected, advisory)
+	affected := append([]GHSAAdvisory(nil), advisories...)
-	}
 
 	return affected
 }

pkg/scanner/govulncheck/govulncheck.go

@@ -75,7 +75,7 @@ func (s *Scanner) Scan(ctx context.Context, registry, packageName, version strin
 
 	// Run govulncheck
 	cmd := exec.CommandContext(ctx, "govulncheck", "-json", "-mode=binary", tmpDir)
-	output, err := cmd.CombinedOutput()
+	output, _ := cmd.CombinedOutput()
 
 	// govulncheck returns non-zero when vulnerabilities are found
 	// Parse output regardless of error

pkg/scanner/scanner.go

@@ -431,20 +431,20 @@ func (m *Manager) CheckVulnerabilities(ctx context.Context, registry, packageNam
 		switch severity {
 		case "CRITICAL":
 			if severityCounts["CRITICAL"] > 0 {
-				return true, fmt.Sprintf("Package has CRITICAL vulnerabilities"), nil
+				return true, "Package has CRITICAL vulnerabilities", nil
 			}
 		case "HIGH":
 			if severityCounts["CRITICAL"] > 0 || severityCounts["HIGH"] > 0 {
-				return true, fmt.Sprintf("Package has HIGH or CRITICAL vulnerabilities"), nil
+				return true, "Package has HIGH or CRITICAL vulnerabilities", nil
 			}
 		case "MODERATE", "MEDIUM":
 			moderateCount := severityCounts["MODERATE"] + severityCounts["MEDIUM"]
 			if severityCounts["CRITICAL"] > 0 || severityCounts["HIGH"] > 0 || moderateCount > 0 {
-				return true, fmt.Sprintf("Package has MODERATE, HIGH, or CRITICAL vulnerabilities"), nil
+				return true, "Package has MODERATE, HIGH, or CRITICAL vulnerabilities", nil
 			}
 		case "LOW":
 			if len(result.Vulnerabilities) > 0 {
-				return true, fmt.Sprintf("Package has vulnerabilities"), nil
+				return true, "Package has vulnerabilities", nil
 			}
 		}
 	}

pkg/vcs/git.go

@@ -98,6 +98,7 @@ func (g *GitFetcher) FetchModule(ctx context.Context, modulePath, version, crede
 
 // modulePathToRepoURL converts a Go module path to a git repository URL
 // Examples:
+//
 //	github.com/user/repo → https://github.com/user/repo.git
 //	gitlab.com/group/project → https://gitlab.com/group/project.git
 func (g *GitFetcher) modulePathToRepoURL(modulePath string) (string, error) {
@@ -116,10 +117,6 @@ func (g *GitFetcher) modulePathToRepoURL(modulePath string) (string, error) {
 
 	// Remove version suffix if present (e.g., /v2, /v3)
 	repo = strings.TrimPrefix(repo, "v")
-	if strings.HasPrefix(repo, "2") || strings.HasPrefix(repo, "3") {
-		// This might be a version suffix, but we need to be careful
-		// For now, keep it as-is
-	}
 
 	repoURL := fmt.Sprintf("https://%s/%s/%s.git", host, owner, repo)
 	return repoURL, nil