fix: Add defense-in-depth check for --no-auto-import flag (bd-4t7)
- autoImportIfNewer() now directly checks noAutoImport flag - Ensures auto-import is blocked even if caller forgets to check autoImportEnabled - Added TestAutoImportIfNewer_NoAutoImportFlag test to verify the fix 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -58,7 +58,15 @@ func findJSONLPath() string {
|
|||||||
// autoImportIfNewer checks if JSONL content changed (via hash) and imports if so
|
// autoImportIfNewer checks if JSONL content changed (via hash) and imports if so
|
||||||
// Fixes bd-84: Hash-based comparison is git-proof (mtime comparison fails after git pull)
|
// Fixes bd-84: Hash-based comparison is git-proof (mtime comparison fails after git pull)
|
||||||
// Fixes bd-228: Now uses collision detection to prevent silently overwriting local changes
|
// Fixes bd-228: Now uses collision detection to prevent silently overwriting local changes
|
||||||
|
// Fixes bd-4t7: Defense-in-depth check to respect --no-auto-import flag
|
||||||
func autoImportIfNewer() {
|
func autoImportIfNewer() {
|
||||||
|
// Defense-in-depth: always check noAutoImport flag directly
|
||||||
|
// This ensures auto-import is disabled even if caller forgot to check autoImportEnabled
|
||||||
|
if noAutoImport {
|
||||||
|
debug.Logf("auto-import skipped (--no-auto-import flag)")
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
// Find JSONL path
|
// Find JSONL path
|
||||||
jsonlPath := findJSONLPath()
|
jsonlPath := findJSONLPath()
|
||||||
|
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package main
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"encoding/json"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -30,6 +31,85 @@ func TestCheckAndAutoImport_NoAutoImportFlag(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestAutoImportIfNewer_NoAutoImportFlag(t *testing.T) {
|
||||||
|
// Test that autoImportIfNewer() respects noAutoImport flag directly (bd-4t7 fix)
|
||||||
|
ctx := context.Background()
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
beadsDir := filepath.Join(tmpDir, ".beads")
|
||||||
|
if err := os.MkdirAll(beadsDir, 0755); err != nil {
|
||||||
|
t.Fatalf("Failed to create beads dir: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
testDBPath := filepath.Join(beadsDir, "bd.db")
|
||||||
|
jsonlPath := filepath.Join(beadsDir, "issues.jsonl")
|
||||||
|
|
||||||
|
// Create database
|
||||||
|
testStore, err := sqlite.New(ctx, testDBPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Failed to create store: %v", err)
|
||||||
|
}
|
||||||
|
defer testStore.Close()
|
||||||
|
|
||||||
|
// Set prefix
|
||||||
|
if err := testStore.SetConfig(ctx, "issue_prefix", "test"); err != nil {
|
||||||
|
t.Fatalf("Failed to set prefix: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create JSONL with an issue that should NOT be imported
|
||||||
|
jsonlIssue := &types.Issue{
|
||||||
|
ID: "test-noimport-bd4t7",
|
||||||
|
Title: "Should Not Import",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
f, err := os.Create(jsonlPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Failed to create JSONL: %v", err)
|
||||||
|
}
|
||||||
|
encoder := json.NewEncoder(f)
|
||||||
|
if err := encoder.Encode(jsonlIssue); err != nil {
|
||||||
|
t.Fatalf("Failed to encode issue: %v", err)
|
||||||
|
}
|
||||||
|
f.Close()
|
||||||
|
|
||||||
|
// Save and set global state
|
||||||
|
oldNoAutoImport := noAutoImport
|
||||||
|
oldAutoImportEnabled := autoImportEnabled
|
||||||
|
oldStore := store
|
||||||
|
oldDbPath := dbPath
|
||||||
|
oldRootCtx := rootCtx
|
||||||
|
oldStoreActive := storeActive
|
||||||
|
|
||||||
|
noAutoImport = true
|
||||||
|
autoImportEnabled = false // Also set this for consistency
|
||||||
|
store = testStore
|
||||||
|
dbPath = testDBPath
|
||||||
|
rootCtx = ctx
|
||||||
|
storeActive = true
|
||||||
|
|
||||||
|
defer func() {
|
||||||
|
noAutoImport = oldNoAutoImport
|
||||||
|
autoImportEnabled = oldAutoImportEnabled
|
||||||
|
store = oldStore
|
||||||
|
dbPath = oldDbPath
|
||||||
|
rootCtx = oldRootCtx
|
||||||
|
storeActive = oldStoreActive
|
||||||
|
}()
|
||||||
|
|
||||||
|
// Call autoImportIfNewer directly - should be blocked by noAutoImport check
|
||||||
|
autoImportIfNewer()
|
||||||
|
|
||||||
|
// Verify issue was NOT imported
|
||||||
|
imported, err := testStore.GetIssue(ctx, "test-noimport-bd4t7")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Failed to check for issue: %v", err)
|
||||||
|
}
|
||||||
|
if imported != nil {
|
||||||
|
t.Error("autoImportIfNewer() imported despite noAutoImport=true - bd-4t7 fix failed")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestCheckAndAutoImport_DatabaseHasIssues(t *testing.T) {
|
func TestCheckAndAutoImport_DatabaseHasIssues(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
tmpDB := t.TempDir() + "/test.db"
|
tmpDB := t.TempDir() + "/test.db"
|
||||||
|
|||||||
Reference in New Issue
Block a user