Close owned import roots during panic unwinding

docs/roadmap/implementation.md

@@ -204,6 +204,9 @@ Focused check: `go test ./internal/engine -run 'Callback|Resolution|Validation|V
 Finding: **B02**.
 Owners: `internal/engine/import_acquisition.go`, `filesystem_access.go`, import tests.
 
+Status: **complete** (B02). Prior stage commits: `2736ddf`, `deb9aa5`,
+`ae3408a`, `ea27f17`, `08a8c25`. Validation passed: `GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Import|SourceVersion|Filesystem' -count=1` and `GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -count=1`.
+
 Install deferred fallback closure at the entry of `acquireImportedWithAccess`,
 before operations that may invoke application code. Use a local closed flag or
 one local close-once function so existing explicit close sites preserve their

internal/engine/import_acquisition.go

@@ -61,6 +61,16 @@ func acquireImportedInput(request importAcquisitionRequest, policy *acquisitionP
 }
 
 func acquireImportedWithAccess(request importAcquisitionRequest, policy *acquisitionPolicy, loadState *acquisitionState, tier *tierAcquisitionState, access *filesystemAccess, resolved resolvedFilesystemContext) (*composedDocument, []SourceInfo, []Diagnostic) {
+	closed := false
+	closeAccess := func() error {
+		if closed {
+			return nil
+		}
+		closed = true
+		return access.close()
+	}
+	defer func() { _ = closeAccess() }()
+
 	main := request.main
 	destination := request.input.input.destination
 	limits := policy.importLimits
@@ -73,13 +83,13 @@ func acquireImportedWithAccess(request importAcquisitionRequest, policy *acquisi
 	}
 	visited := tier.visited
 	if !access.contains(basePath) || (mainPath != "" && !access.contains(mainPath)) {
-		_ = access.close()
+		_ = closeAccess()
 		return nil, sourceInfos(main.source), []Diagnostic{importDiagnostic(CodeImportOutsideRoot, "selected source and import base must stay inside the configuration root", nil)}
 	}
 	for _, directory := range []string{access.rootPath, basePath} {
 		info, err := access.stat(directory)
 		if err != nil || !info.IsDir() {
-			_ = access.close()
+			_ = closeAccess()
 			return nil, sourceInfos(main.source), []Diagnostic{importDiagnostic(CodeImportInvalid, "configuration root and import base must be directories", nil)}
 		}
 	}
@@ -95,7 +105,7 @@ func acquireImportedWithAccess(request importAcquisitionRequest, policy *acquisi
 		mainResource = access.syntheticIdentity()
 	}
 	if previous, repeated := visited[mainResource]; repeated {
-		_ = access.close()
+		_ = closeAccess()
 		return nil, sourceInfos(main.source), []Diagnostic{importRepeatedDiagnostic(Location{SourceID: main.source.ID}, previous, importRoute{})}
 	}
 	accounting.sources++
@@ -125,7 +135,7 @@ func acquireImportedWithAccess(request importAcquisitionRequest, policy *acquisi
 	}
 
 	if diagnostic := state.checkInitialLimits(); diagnostic != nil {
-		_ = access.close()
+		_ = closeAccess()
 		return nil, cloneSourceInfos(state.sources), []Diagnostic{*diagnostic}
 	}
 
@@ -181,7 +191,7 @@ func acquireImportedWithAccess(request importAcquisitionRequest, policy *acquisi
 		scheduleAcquisitionScopes(&state.scope, indexImportDependencies(state.importGraph))
 	}
 
-	closeErr := access.close()
+	closeErr := closeAccess()
 	if len(diagnostics) != 0 {
 		if mainIsItem {
 			diagnostics = prependCompositionDiagnosticPaths(diagnostics, destination.Index(0))

internal/engine/import_root_panic_test.go

@@ -0,0 +1,108 @@
+package engine
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+	"testing/fstest"
+)
+
+func TestImportedAcquisitionClosesOwnedHostRootOnEveryExit(t *testing.T) {
+	tests := []struct {
+		name      string
+		main      string
+		child     string
+		version   bool
+		wantError bool
+		wantPanic bool
+	}{
+		{name: "success", main: "configkit: {imports: [{path: child.yml}]}\n", child: "value: child\n"},
+		{name: "error", main: "configkit: {imports: [{path: missing.yml, required: true}]}\n", wantError: true},
+		{name: "version panic", main: "configkit: {imports: [{path: child.yml}]}\n", child: "version: v1\n", version: true, wantPanic: true},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			rootPath := t.TempDir()
+			mainPath := filepath.Join(rootPath, "main.yml")
+			if err := os.WriteFile(mainPath, []byte(test.main), 0o600); err != nil {
+				t.Fatal(err)
+			}
+			if test.child != "" {
+				if err := os.WriteFile(filepath.Join(rootPath, "child.yml"), []byte(test.child), 0o600); err != nil {
+					t.Fatal(err)
+				}
+			}
+			source := File(mainPath)
+			main, diagnostics := parseSource(source, "source-1", defaultParseLimits())
+			if len(diagnostics) != 0 {
+				t.Fatalf("main parse diagnostics = %#v", diagnostics)
+			}
+			access, err := openHostAccess(rootPath)
+			if err != nil {
+				t.Fatal(err)
+			}
+			ownedRoot := access.root
+			sentinel := errors.New("imported version checker panic sentinel")
+			var rule *VersionRule
+			var versionState *sourceVersionState
+			if test.version {
+				rule = &VersionRule{Path: Keys("version"), Check: func(Version, *Validation) error { panic(sentinel) }}
+				versionState = &sourceVersionState{localRule: rule, effectiveRule: rule, mode: sourceVersionModeRequired}
+			}
+			request := testImportRequest(source, nil, main, Path{}, rule, nil)
+			policy := &acquisitionPolicy{importLimits: defaultImportLimits()}
+			state := &acquisitionState{accounting: &importAccounting{}, versionState: versionState}
+			tier := &tierAcquisitionState{}
+			resolved := resolvedFilesystemContext{rootPath: rootPath, basePath: rootPath, mainPath: mainPath}
+			var resultDiagnostics []Diagnostic
+			var recovered any
+			func() {
+				defer func() { recovered = recover() }()
+				_, _, resultDiagnostics = acquireImportedWithAccess(request, policy, state, tier, access, resolved)
+			}()
+			if test.wantPanic {
+				if recovered != sentinel {
+					t.Fatalf("recovered panic = %#v, want original sentinel", recovered)
+				}
+			} else if recovered != nil {
+				t.Fatalf("unexpected panic = %#v", recovered)
+			}
+			if test.wantError != (len(resultDiagnostics) != 0) {
+				t.Fatalf("diagnostics = %#v, want error=%v", resultDiagnostics, test.wantError)
+			}
+			if _, err := ownedRoot.Stat("."); err == nil {
+				t.Fatal("owned host root remained usable after acquisition returned")
+			}
+		})
+	}
+}
+
+func TestImportedAcquisitionClosesChildFileBeforeVersionPanic(t *testing.T) {
+	filesystem := &observedFilesystem{files: fstest.MapFS{
+		"main.yml":  &fstest.MapFile{Data: []byte("configkit: {imports: [{path: child.yml}]}\n")},
+		"child.yml": &fstest.MapFile{Data: []byte("version: v1\n")},
+	}}
+	namespace := newNativeNamespace("bundle", filesystem)
+	source := filesystemFile(namespace, "main.yml")
+	main, diagnostics := parseSource(source, "source-1", defaultParseLimits())
+	if len(diagnostics) != 0 {
+		t.Fatalf("main parse diagnostics = %#v", diagnostics)
+	}
+	sentinel := errors.New("imported version checker panic sentinel")
+	rule := &VersionRule{Path: Keys("version"), Check: func(Version, *Validation) error { panic(sentinel) }}
+	versionState := &sourceVersionState{localRule: rule, effectiveRule: rule, mode: sourceVersionModeRequired}
+	request := testImportRequest(source, nil, main, Path{}, rule, nil)
+	access := newNativeAccess(filesystem, ".", namespace.token)
+	var recovered any
+	func() {
+		defer func() { recovered = recover() }()
+		_, _, _ = acquireImportedWithAccess(request, &acquisitionPolicy{importLimits: defaultImportLimits()},
+			&acquisitionState{accounting: &importAccounting{}, versionState: versionState}, &tierAcquisitionState{}, access,
+			resolvedFilesystemContext{native: namespace, rootPath: ".", basePath: ".", mainPath: source.path})
+	}()
+	if recovered != sentinel {
+		t.Fatalf("recovered panic = %#v, want original sentinel", recovered)
+	}
+	filesystem.assertClosed(t)
+}