diff --git a/internal/fsutil/rename.go b/internal/fsutil/rename.go index ace33502a..889a60604 100644 --- a/internal/fsutil/rename.go +++ b/internal/fsutil/rename.go @@ -9,6 +9,23 @@ import ( "time" ) +// ReplaceWithRetry publishes src over dst using the platform's replacement +// primitive, retrying on the same transient Windows lock errors RenameWithRetry +// handles. Prefer it over RenameWithRetry when dst may already exist and is +// expected to keep its identity: on Windows it replaces through ReplaceFileW, +// which is a single operation (os.Rename is documented non-atomic there) and +// carries the destination's security descriptor over to the replacement instead +// of publishing the temporary file's inherited ACL. On Unix it is os.Rename. +// +// replace overrides the platform primitive so tests can exercise the retry path; +// pass nil for the default. +func ReplaceWithRetry(src, dst string, replace func(src, dst string) error) error { + if replace == nil { + replace = replaceExisting + } + return RenameWithRetry(src, dst, replace) +} + // RenameWithRetry renames src to dst, retrying briefly on Windows when the // destination is transiently locked (antivirus scanners, search indexers, or // a concurrent reader holding the file open). rename overrides os.Rename so diff --git a/internal/fsutil/replace_other.go b/internal/fsutil/replace_other.go new file mode 100644 index 000000000..35a985518 --- /dev/null +++ b/internal/fsutil/replace_other.go @@ -0,0 +1,12 @@ +//go:build !windows + +package fsutil + +import "os" + +// replaceExisting publishes src over dst. rename(2) already replaces the +// destination atomically within one filesystem on Unix, and it neither creates +// nor consults an ACL, so there is nothing extra to preserve here. +func replaceExisting(src, dst string) error { + return os.Rename(src, dst) +} diff --git a/internal/fsutil/replace_other_test.go b/internal/fsutil/replace_other_test.go new file mode 100644 index 000000000..9c53c9225 --- /dev/null +++ b/internal/fsutil/replace_other_test.go @@ -0,0 +1,50 @@ +//go:build !windows + +package fsutil + +import ( + "os" + "path/filepath" + "testing" +) + +// On Unix the replacement primitive is rename(2), which already publishes +// atomically within one filesystem and neither creates nor consults an ACL. These +// cover both shapes so the shared helper is exercised on every platform. +func TestReplaceWithRetryPublishesOverExistingAndMissingDestinations(t *testing.T) { + for _, tc := range []struct { + name string + existing bool + }{ + {name: "existing destination", existing: true}, + {name: "missing destination"}, + } { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, ".manifest.tmp") + dst := filepath.Join(dir, "manifest.md") + if err := os.WriteFile(src, []byte("new"), 0o600); err != nil { + t.Fatalf("WriteFile src: %v", err) + } + if tc.existing { + if err := os.WriteFile(dst, []byte("old"), 0o600); err != nil { + t.Fatalf("WriteFile dst: %v", err) + } + } + + if err := ReplaceWithRetry(src, dst, nil); err != nil { + t.Fatalf("ReplaceWithRetry: %v", err) + } + data, err := os.ReadFile(dst) + if err != nil { + t.Fatalf("ReadFile dst: %v", err) + } + if string(data) != "new" { + t.Fatalf("destination content = %q, want the replacement bytes", data) + } + if _, err := os.Lstat(src); !os.IsNotExist(err) { + t.Fatalf("the replacement file should be consumed by the replace: %v", err) + } + }) + } +} diff --git a/internal/fsutil/replace_windows.go b/internal/fsutil/replace_windows.go new file mode 100644 index 000000000..c4aa27a3c --- /dev/null +++ b/internal/fsutil/replace_windows.go @@ -0,0 +1,83 @@ +//go:build windows + +package fsutil + +import ( + "errors" + "fmt" + "os" + "syscall" + "unsafe" +) + +const ( + replaceFileWriteThrough = 0x00000001 + replaceFileIgnoreMergeErrors = 0x00000002 + + // Returned when the volume cannot perform a replace (FAT/exFAT, some network + // redirectors). Such volumes carry no ACL to preserve, so a plain rename is + // an equivalent publish there. + errorInvalidFunction = syscall.Errno(1) + errorNotSupported = syscall.Errno(50) +) + +var ( + replaceKernel32 = syscall.NewLazyDLL("kernel32.dll") + replaceProcReplaceFil = replaceKernel32.NewProc("ReplaceFileW") +) + +// replaceExisting publishes src over dst with ReplaceFileW rather than +// MoveFileEx (what os.Rename uses). Two reasons, both of which os.Rename fails +// to provide on Windows: +// +// - Atomicity. Go documents os.Rename as NOT atomic outside Unix, so an +// interrupted or concurrently observed overwrite can expose a missing or +// intermediate file. ReplaceFileW performs the swap as one operation, and +// REPLACEFILE_WRITE_THROUGH flushes it before returning. +// - Security descriptor. The replacement is a freshly created temporary file, +// so it carries the directory's inherited DACL. Renaming it over the +// destination therefore REPLACES the destination's ACL — silently widening +// access to a file that had been restricted explicitly (os.File.Chmod cannot +// express that on Windows; Go only maps the owner-write bit). ReplaceFileW +// carries the replaced file's security descriptor, attributes, and streams +// over to the replacement instead. +// +// REPLACEFILE_IGNORE_ACL_ERRORS is deliberately NOT passed: silently losing the +// descriptor is the very failure this exists to prevent, so an ACL merge failure +// surfaces as an error, leaving the destination untouched and the caller free to +// clean up its temporary file. +func replaceExisting(src, dst string) error { + if _, err := os.Lstat(dst); err != nil { + if os.IsNotExist(err) { + // Nothing to replace and no descriptor to preserve. + return os.Rename(src, dst) + } + return err + } + replaced, err := syscall.UTF16PtrFromString(dst) + if err != nil { + return err + } + replacement, err := syscall.UTF16PtrFromString(src) + if err != nil { + return err + } + result, _, callErr := replaceProcReplaceFil.Call( + uintptr(unsafe.Pointer(replaced)), + uintptr(unsafe.Pointer(replacement)), + 0, // no backup copy + uintptr(replaceFileWriteThrough|replaceFileIgnoreMergeErrors), + 0, + 0, + ) + if result != 0 { + return nil + } + if callErr == nil || errors.Is(callErr, syscall.Errno(0)) { + return fmt.Errorf("replace %s: ReplaceFileW failed", dst) + } + if errors.Is(callErr, errorInvalidFunction) || errors.Is(callErr, errorNotSupported) { + return os.Rename(src, dst) + } + return callErr +} diff --git a/internal/fsutil/replace_windows_test.go b/internal/fsutil/replace_windows_test.go new file mode 100644 index 000000000..0260fbfcb --- /dev/null +++ b/internal/fsutil/replace_windows_test.go @@ -0,0 +1,180 @@ +//go:build windows + +package fsutil + +import ( + "os" + "path/filepath" + "strings" + "syscall" + "testing" + + "golang.org/x/sys/windows" +) + +// TestReplaceWithRetryKeepsTheDestinationIdentity proves the publish goes through +// ReplaceFileW and not os.Rename: ReplaceFileW gives the replacement the replaced +// file's identity, so the destination's creation time survives. A rename would +// carry the temporary file's creation time over instead, and os.Rename is also +// documented non-atomic on Windows. +func TestReplaceWithRetryKeepsTheDestinationIdentity(t *testing.T) { + dir := t.TempDir() + dst := filepath.Join(dir, "manifest.md") + if err := os.WriteFile(dst, []byte("old"), 0o600); err != nil { + t.Fatalf("WriteFile dst: %v", err) + } + created := creationTime(t, dst) + + src := filepath.Join(dir, ".manifest.tmp") + if err := os.WriteFile(src, []byte("new"), 0o600); err != nil { + t.Fatalf("WriteFile src: %v", err) + } + // Push the replacement's own creation time clearly past the destination's so a + // rename would be visible in the comparison below. + future := windows.NsecToFiletime(created.Nanoseconds() + int64(10*1e9)) + setCreationTime(t, src, future) + + if err := ReplaceWithRetry(src, dst, nil); err != nil { + t.Fatalf("ReplaceWithRetry: %v", err) + } + data, err := os.ReadFile(dst) + if err != nil { + t.Fatalf("ReadFile dst: %v", err) + } + if string(data) != "new" { + t.Fatalf("destination content = %q, want the replacement bytes", data) + } + if _, err := os.Lstat(src); !os.IsNotExist(err) { + t.Fatalf("the replacement file should be consumed by the replace: %v", err) + } + if got := creationTime(t, dst); got != created { + t.Fatalf("creation time = %v, want the replaced file's %v (a rename would not preserve it)", got, created) + } +} + +// TestReplaceWithRetryPreservesDestinationDACL is the regression test for the +// second half of the finding: the replacement is a freshly created temporary file +// carrying the directory's inherited DACL, so publishing it with a rename would +// REPLACE the restrictive descriptor an explicitly locked-down file had. +func TestReplaceWithRetryPreservesDestinationDACL(t *testing.T) { + dir := t.TempDir() + dst := filepath.Join(dir, "manifest.md") + if err := os.WriteFile(dst, []byte("old"), 0o600); err != nil { + t.Fatalf("WriteFile dst: %v", err) + } + // A protected DACL granting only the owner: distinct from whatever the temp + // file inherits from the directory. + restricted, err := windows.SecurityDescriptorFromString("D:P(A;;FA;;;OW)") + if err != nil { + t.Skipf("cannot build a test security descriptor: %v", err) + } + dacl, _, err := restricted.DACL() + if err != nil { + t.Skipf("cannot read the test DACL: %v", err) + } + if err := windows.SetNamedSecurityInfo( + dst, + windows.SE_FILE_OBJECT, + windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION, + nil, nil, dacl, nil, + ); err != nil { + t.Skipf("cannot apply a restrictive DACL on this filesystem: %v", err) + } + want := describeDACL(t, dst) + + src := filepath.Join(dir, ".manifest.tmp") + if err := os.WriteFile(src, []byte("new"), 0o600); err != nil { + t.Fatalf("WriteFile src: %v", err) + } + if inherited := describeDACL(t, src); inherited == want { + t.Skip("the temporary file already carries the same DACL; this filesystem cannot show the difference") + } + + if err := ReplaceWithRetry(src, dst, nil); err != nil { + t.Fatalf("ReplaceWithRetry: %v", err) + } + if got := describeDACL(t, dst); got != want { + t.Fatalf("DACL after replace = %q, want the destination's own %q", got, want) + } +} + +// TestReplaceWithRetryPublishesWhenDestinationIsMissing covers the no-destination +// case: ReplaceFileW requires an existing file to replace, so there is a rename +// fallback (and nothing to preserve). +func TestReplaceWithRetryPublishesWhenDestinationIsMissing(t *testing.T) { + dir := t.TempDir() + src := filepath.Join(dir, ".manifest.tmp") + dst := filepath.Join(dir, "manifest.md") + if err := os.WriteFile(src, []byte("new"), 0o600); err != nil { + t.Fatalf("WriteFile src: %v", err) + } + if err := ReplaceWithRetry(src, dst, nil); err != nil { + t.Fatalf("ReplaceWithRetry: %v", err) + } + if data, err := os.ReadFile(dst); err != nil || string(data) != "new" { + t.Fatalf("destination = %q err=%v, want the replacement bytes", data, err) + } +} + +// TestReplaceWithRetryRetriesTransientLockViolation keeps the retry behavior that +// RenameWithRetry provides for antivirus/indexer holds. +func TestReplaceWithRetryRetriesTransientLockViolation(t *testing.T) { + attempts := 0 + err := ReplaceWithRetry("src", "dst", func(src, dst string) error { + attempts++ + if attempts < 3 { + return &os.PathError{Op: "replace", Path: dst, Err: syscall.Errno(32)} // ERROR_SHARING_VIOLATION + } + return nil + }) + if err != nil { + t.Fatalf("ReplaceWithRetry: %v", err) + } + if attempts != 3 { + t.Fatalf("attempts = %d, want the transient violations retried", attempts) + } +} + +func creationTime(t *testing.T, path string) syscall.Filetime { + t.Helper() + info, err := os.Stat(path) + if err != nil { + t.Fatalf("Stat %s: %v", path, err) + } + data, ok := info.Sys().(*syscall.Win32FileAttributeData) + if !ok { + t.Skipf("no Windows file attributes for %s", path) + } + return data.CreationTime +} + +func setCreationTime(t *testing.T, path string, created windows.Filetime) { + t.Helper() + pathPtr, err := windows.UTF16PtrFromString(path) + if err != nil { + t.Fatalf("UTF16PtrFromString: %v", err) + } + handle, err := windows.CreateFile(pathPtr, windows.FILE_WRITE_ATTRIBUTES, 0, nil, windows.OPEN_EXISTING, windows.FILE_ATTRIBUTE_NORMAL, 0) + if err != nil { + t.Fatalf("CreateFile %s: %v", path, err) + } + defer func() { + _ = windows.CloseHandle(handle) + }() + if err := windows.SetFileTime(handle, &created, nil, nil); err != nil { + t.Fatalf("SetFileTime %s: %v", path, err) + } +} + +func describeDACL(t *testing.T, path string) string { + t.Helper() + sd, err := windows.GetNamedSecurityInfo(path, windows.SE_FILE_OBJECT, windows.DACL_SECURITY_INFORMATION) + if err != nil { + t.Skipf("cannot read the security descriptor of %s: %v", path, err) + } + text := sd.String() + if index := strings.Index(text, "D:"); index >= 0 { + return text[index:] + } + return text +} diff --git a/internal/specialist/storage.go b/internal/specialist/storage.go index 952f4f6f9..719eb8c26 100644 --- a/internal/specialist/storage.go +++ b/internal/specialist/storage.go @@ -4,8 +4,11 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strconv" "strings" + + "github.com/Gitlawb/zero/internal/fsutil" ) type Storage struct { @@ -63,25 +66,17 @@ func (storage *Storage) Create(input CreateInput) (Manifest, error) { return Manifest{}, fmt.Errorf("specialist %q requires a system prompt", manifest.Metadata.Name) } content := FormatMarkdown(manifest) - if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o700); err != nil { return Manifest{}, fmt.Errorf("create specialist directory: %w", err) } if input.Overwrite { - info, err := os.Lstat(path) - if err != nil && !os.IsNotExist(err) { - return Manifest{}, fmt.Errorf("inspect specialist file: %w", err) - } - if err == nil && info.Mode()&os.ModeSymlink != 0 { - return Manifest{}, fmt.Errorf("refusing to overwrite symlink specialist file: %s", path) + if err := writeSpecialistAtomic(path, content); err != nil { + return Manifest{}, err } + return manifest, nil } - flags := os.O_WRONLY | os.O_CREATE - if input.Overwrite { - flags |= os.O_TRUNC - } else { - flags |= os.O_EXCL - } - file, err := os.OpenFile(path, flags, 0o600) + file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) if err != nil { if os.IsExist(err) { return Manifest{}, fmt.Errorf("specialist already exists: %s", manifest.Metadata.Name) @@ -98,6 +93,72 @@ func (storage *Storage) Create(input CreateInput) (Manifest, error) { return manifest, nil } +func writeSpecialistAtomic(path string, content string) error { + return writeSpecialistAtomicWith(path, content, nil, syncSpecialistDir) +} + +func writeSpecialistAtomicWith(path string, content string, rename func(string, string) error, syncDir func(string) error) (err error) { + temp, err := os.CreateTemp(filepath.Dir(path), ".specialist-*.tmp") + if err != nil { + return fmt.Errorf("create temporary specialist file: %w", err) + } + tempPath := temp.Name() + defer func() { + _ = temp.Close() + _ = os.Remove(tempPath) + }() + + if err := temp.Chmod(0o600); err != nil { + return fmt.Errorf("set temporary specialist file permissions: %w", err) + } + if _, err := temp.WriteString(content); err != nil { + return fmt.Errorf("write temporary specialist file: %w", err) + } + if err := temp.Sync(); err != nil { + return fmt.Errorf("sync temporary specialist file: %w", err) + } + if err := temp.Close(); err != nil { + return fmt.Errorf("close temporary specialist file: %w", err) + } + + info, err := os.Lstat(path) + if err != nil && !os.IsNotExist(err) { + return fmt.Errorf("inspect specialist file: %w", err) + } + if err == nil && info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("refusing to overwrite symlink specialist file: %s", path) + } + // ReplaceWithRetry, not RenameWithRetry: os.Rename is documented non-atomic + // outside Unix, and renaming the freshly created temporary file over the + // destination would also publish the directory's inherited DACL in place of + // whatever the destination had — silently widening access to a specialist that + // had been restricted explicitly. On Windows this replaces through + // ReplaceFileW, which is one operation and preserves the destination's + // security descriptor; on Unix it is the same atomic rename as before. + if err := fsutil.ReplaceWithRetry(tempPath, path, rename); err != nil { + return fmt.Errorf("replace specialist file: %w", err) + } + if err := syncDir(filepath.Dir(path)); err != nil { + return fmt.Errorf("sync specialist directory: %w", err) + } + return nil +} + +func syncSpecialistDir(path string) error { + if runtime.GOOS == "windows" { + return nil + } + dir, err := os.Open(path) + if err != nil { + return err + } + if err := dir.Sync(); err != nil { + _ = dir.Close() + return err + } + return dir.Close() +} + func (storage *Storage) Delete(input DeleteInput) (string, error) { location := normalizeWritableLocation(input.Location) path, err := storage.path(input.Name, location) diff --git a/internal/specialist/storage_dacl_windows_test.go b/internal/specialist/storage_dacl_windows_test.go new file mode 100644 index 000000000..7ef53f999 --- /dev/null +++ b/internal/specialist/storage_dacl_windows_test.go @@ -0,0 +1,85 @@ +//go:build windows + +package specialist + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "golang.org/x/sys/windows" +) + +// TestStorageCreateForceKeepsWindowsDACL is the regression test for the Windows +// half of the atomic-overwrite change: the replacement is a freshly created +// temporary file, so publishing it with a plain rename would hand the destination +// the directory's inherited DACL and drop the restrictive one an explicitly +// locked-down specialist had — exposing its system prompt to anyone the directory +// grants access to. temp.Chmod(0o600) cannot express that on Windows (Go only +// maps the owner-write bit there), so the replacement primitive has to carry the +// descriptor over. +func TestStorageCreateForceKeepsWindowsDACL(t *testing.T) { + userDir := t.TempDir() + path := filepath.Join(userDir, "safe.md") + if err := os.WriteFile(path, []byte("old content"), 0o600); err != nil { + t.Fatal(err) + } + // A protected, owner-only DACL: what an operator restricting one specialist + // inside a group-readable directory would end up with. + restricted, err := windows.SecurityDescriptorFromString("D:P(A;;FA;;;OW)") + if err != nil { + t.Skipf("cannot build a test security descriptor: %v", err) + } + dacl, _, err := restricted.DACL() + if err != nil { + t.Skipf("cannot read the test DACL: %v", err) + } + if err := windows.SetNamedSecurityInfo( + path, + windows.SE_FILE_OBJECT, + windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION, + nil, nil, dacl, nil, + ); err != nil { + t.Skipf("cannot apply a restrictive DACL on this filesystem: %v", err) + } + want := specialistDACL(t, path) + if !strings.Contains(want, "(A;;FA;;;OW)") { + t.Skipf("the restrictive DACL did not take effect on this filesystem: %q", want) + } + + storage := NewStorage(Paths{UserDir: userDir}) + manifest, err := storage.Create(CreateInput{ + Name: "safe", + Description: "Safe", + SystemPrompt: "new content", + Overwrite: true, + }) + if err != nil { + t.Fatalf("Create returned error: %v", err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if got, want := string(data), FormatMarkdown(manifest); got != want { + t.Fatalf("file content = %q, want %q", got, want) + } + if got := specialistDACL(t, path); got != want { + t.Fatalf("DACL after overwrite = %q, want the destination's own %q", got, want) + } + assertNoTemporarySpecialistFiles(t, userDir) +} + +func specialistDACL(t *testing.T, path string) string { + t.Helper() + sd, err := windows.GetNamedSecurityInfo(path, windows.SE_FILE_OBJECT, windows.DACL_SECURITY_INFORMATION) + if err != nil { + t.Skipf("cannot read the security descriptor of %s: %v", path, err) + } + text := sd.String() + if index := strings.Index(text, "D:"); index >= 0 { + return text[index:] + } + return text +} diff --git a/internal/specialist/storage_test.go b/internal/specialist/storage_test.go index fe4c41a9c..fb9c5f08f 100644 --- a/internal/specialist/storage_test.go +++ b/internal/specialist/storage_test.go @@ -1,9 +1,12 @@ package specialist import ( + "errors" "os" "path/filepath" + "runtime" "strings" + "syscall" "testing" ) @@ -88,4 +91,99 @@ func TestStorageCreateForceRejectsSymlink(t *testing.T) { if string(data) != "outside" { t.Fatalf("symlink target was modified: %q", string(data)) } + assertNoTemporarySpecialistFiles(t, userDir) +} + +func TestStorageCreateForceAtomicallyReplacesFile(t *testing.T) { + userDir := t.TempDir() + path := filepath.Join(userDir, "safe.md") + if err := os.WriteFile(path, []byte("old content"), 0o644); err != nil { + t.Fatal(err) + } + storage := NewStorage(Paths{UserDir: userDir}) + + manifest, err := storage.Create(CreateInput{ + Name: "safe", + Description: "Safe", + SystemPrompt: "new content", + Overwrite: true, + }) + if err != nil { + t.Fatalf("Create returned error: %v", err) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if got, want := string(data), FormatMarkdown(manifest); got != want { + t.Fatalf("file content = %q, want %q", got, want) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); runtime.GOOS != "windows" && got != 0o600 { + t.Fatalf("file permissions = %o, want 600", got) + } + assertNoTemporarySpecialistFiles(t, userDir) +} + +func TestWriteSpecialistAtomicRetriesTransientWindowsRename(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("rename retries are Windows-specific") + } + dir := t.TempDir() + path := filepath.Join(dir, "safe.md") + if err := os.WriteFile(path, []byte("old content"), 0o600); err != nil { + t.Fatal(err) + } + attempts := 0 + err := writeSpecialistAtomicWith(path, "new content", func(src, dst string) error { + attempts++ + if attempts == 1 { + return syscall.Errno(32) // ERROR_SHARING_VIOLATION + } + return os.Rename(src, dst) + }, func(string) error { return nil }) + if err != nil { + t.Fatalf("writeSpecialistAtomicWith returned error: %v", err) + } + if attempts != 2 { + t.Fatalf("rename attempts = %d, want 2", attempts) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if got := string(data); got != "new content" { + t.Fatalf("file content = %q, want %q", got, "new content") + } + assertNoTemporarySpecialistFiles(t, dir) +} + +func TestWriteSpecialistAtomicPropagatesDirectorySyncError(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "safe.md") + syncErr := errors.New("sync failed") + err := writeSpecialistAtomicWith(path, "new content", nil, func(got string) error { + if got != dir { + t.Fatalf("sync directory = %q, want %q", got, dir) + } + return syncErr + }) + if !errors.Is(err, syncErr) { + t.Fatalf("writeSpecialistAtomicWith error = %v, want %v", err, syncErr) + } + assertNoTemporarySpecialistFiles(t, dir) +} + +func assertNoTemporarySpecialistFiles(t *testing.T, dir string) { + t.Helper() + matches, err := filepath.Glob(filepath.Join(dir, ".specialist-*.tmp")) + if err != nil { + t.Fatal(err) + } + if len(matches) != 0 { + t.Fatalf("temporary specialist files remain: %v", matches) + } }