test: Add security and error handling tests for lint warnings

Added comprehensive tests to address gosec and errcheck linter warnings:

1. bd-yxy (P0): Command injection prevention tests for git rm in merge command
   - Added merge_security_test.go with tests for shell metacharacters
   - Verified exec.Command safely passes arguments (no shell interpretation)
   - Added #nosec G204 comment explaining why code is safe

2. bd-nbc (P1): Security tests for file path validation in clean command
   - Added clean_security_test.go with path traversal tests
   - Verified filepath.Join safely constructs paths within .beads directory
   - Added #nosec G304 comment documenting safety guarantees

3. bd-lln (P2): Tests for performFlush error handling in FlushManager
   - Added tests documenting that performFlush intentionally returns nil
   - Errors are handled internally by flushToJSONLWithState
   - Tests verify graceful degradation when store is inactive

4. bd-gra (P2): Error handling test for cmd.Help() in search command
   - Added search_test.go documenting Help() error handling
   - Help() errors intentionally ignored (already in error path, will exit anyway)
   - Added #nosec G104 comment explaining rationale

All new tests pass. The linter warnings are false positives or intentional
design decisions, now documented with tests and #nosec comments.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Steve Yegge
2025-11-21 19:30:17 -05:00
parent d7f4189e3e
commit 74f384444b
7 changed files with 752 additions and 0 deletions

View File

@@ -414,3 +414,75 @@ func teardownTestEnvironment(t *testing.T) {
flushManager = nil
}
}
// TestPerformFlushErrorHandling verifies that performFlush handles errors correctly.
// This test addresses bd-lln: unparam flagged performFlush as always returning nil.
//
// The design is that performFlush calls flushToJSONLWithState, which handles all
// errors internally by:
// - Setting lastFlushError and flushFailureCount
// - Printing warnings to stderr
// - Not propagating errors back to the caller
//
// Therefore, performFlush doesn't return errors - it's a fire-and-forget operation.
// Any error handling is done internally by the flush system.
func TestPerformFlushErrorHandling(t *testing.T) {
setupTestEnvironment(t)
defer teardownTestEnvironment(t)
fm := NewFlushManager(true, 50*time.Millisecond)
defer func() {
if err := fm.Shutdown(); err != nil {
t.Errorf("Shutdown failed: %v", err)
}
}()
// performFlush with inactive store should return nil (graceful degradation)
storeMutex.Lock()
storeActive = false
storeMutex.Unlock()
err := fm.performFlush(false)
if err != nil {
t.Errorf("performFlush should return nil when store inactive, got: %v", err)
}
// Restore store for cleanup
storeMutex.Lock()
storeActive = true
storeMutex.Unlock()
}
// TestPerformFlushStoreInactive verifies performFlush handles inactive store gracefully
func TestPerformFlushStoreInactive(t *testing.T) {
setupTestEnvironment(t)
defer teardownTestEnvironment(t)
fm := NewFlushManager(true, 50*time.Millisecond)
defer func() {
if err := fm.Shutdown(); err != nil {
t.Errorf("Shutdown failed: %v", err)
}
}()
// Deactivate store
storeMutex.Lock()
storeActive = false
storeMutex.Unlock()
// performFlush should handle this gracefully
err := fm.performFlush(false)
if err != nil {
t.Errorf("Expected performFlush to handle inactive store gracefully, got error: %v", err)
}
err = fm.performFlush(true) // Try full export too
if err != nil {
t.Errorf("Expected performFlush (full) to handle inactive store gracefully, got error: %v", err)
}
// Restore store for cleanup
storeMutex.Lock()
storeActive = true
storeMutex.Unlock()
}