diff --git a/file/AGENTS.md b/file/AGENTS.md index b932e03..1416bc9 100644 --- a/file/AGENTS.md +++ b/file/AGENTS.md @@ -39,13 +39,32 @@ way POSIX tools expect, without that newline leaking into the mod content and ap spurious character in TTS. The two halves are a matched pair — changing one without the other adds or eats a newline on every round trip. -## `Clear` and its safety check - -`DirOps.Clear()` deletes and recreates a directory. Because it is destructive, `preClearCheck` -first walks the tree and refuses if it finds a file whose extension is not in `allowedExtensions` -(`.json`, `.gmnotes`, `.luascriptstate`, `.ttslua`, `.xml`). The intent is that the objects -directory should only ever contain files this tool generated, so anything unrecognized means the -path is wrong and deleting would destroy someone's work. +## `Clear` and its safety checks + +`DirOps.Clear()` deletes and recreates a directory. Because it is destructive it runs two guards +before touching anything. + +**Ownership marker.** After a successful clear, `Clear` drops a hidden sentinel file +(`.ttsmm-managed`, the `managedMarker` constant) into the directory. On the next run that marker is +proof the tool created the directory, so deleting it is safe. `Clear` refuses unless one of: + +- the directory does not exist, +- the directory is empty, +- the directory already contains the marker, or +- the directory has no marker but every file passes the legacy extension allowlist + (`allowedExtensions`: `.json`, `.gmnotes`, `.luascriptstate`, `.ttslua`, `.xml`) — backward + compatibility for trees written before the marker existed; they clear once and gain a marker + going forward. + +A non-empty directory with no marker and unrecognized content (a mistargeted `--moddir` pointed at, +say, `$HOME`) is refused with an error explaining how to proceed. This also resolves the older +`.DS_Store`/`.gitkeep` false-positive, since a tool-created directory carries a marker and the +extension check no longer gates it. + +**Path guard.** Independent of contents, `pathGuard` resolves the target to an absolute, cleaned +path and refuses the filesystem root, the user's home directory, any ancestor of home, and +suspiciously shallow single-segment paths — targets that are almost always a typo rather than a mod +tree. ## `conversions.go` diff --git a/file/dirops.go b/file/dirops.go index 455bef2..902bc04 100644 --- a/file/dirops.go +++ b/file/dirops.go @@ -6,6 +6,7 @@ import ( "os" "path" "path/filepath" + "strings" "time" ) @@ -18,6 +19,12 @@ var allowedExtensions = map[string]struct{}{ ".xml": {}, } +// managedMarker is the hidden ownership marker (sentinel) this tool drops into any +// directory it manages. Its presence is proof that the directory was created by +// TTSModManager, which is what makes deleting the directory's contents safe. See the +// "Clear and its safety check" section of AGENTS.md. +const managedMarker = ".ttsmm-managed" + // DirCreator abstracts folder creation type DirCreator interface { CreateDir(relpath string, suggestion string) (string, error) @@ -89,12 +96,119 @@ func (d *DirOps) preClearCheck() error { return walkErr } +// pathGuard refuses to operate on paths that are almost certainly a mistargeted +// --moddir rather than a real objects directory: the filesystem root, the user's home +// directory (or any ancestor of it), or any suspiciously shallow path. Deleting any of +// these would destroy unrelated user data. +func pathGuard(target string) error { + abs, err := filepath.Abs(target) + if err != nil { + return fmt.Errorf("could not resolve absolute path of %s: %w", target, err) + } + abs = filepath.Clean(abs) + + // Refuse the filesystem root. + if abs == filepath.Clean(string(filepath.Separator)) { + return fmt.Errorf("refusing to clear filesystem root %q", abs) + } + + // Refuse the home directory and any ancestor of it. + if home, herr := os.UserHomeDir(); herr == nil && home != "" { + home = filepath.Clean(home) + if abs == home { + return fmt.Errorf("refusing to clear home directory %q", abs) + } + // An ancestor of home (e.g. /home, / on some systems) is even more dangerous. + // abs is a strict ancestor when the path from abs down to home never climbs out. + if rel, rerr := filepath.Rel(abs, home); rerr == nil && rel != "." && !startsWithParent(rel) { + return fmt.Errorf("refusing to clear %q, an ancestor of your home directory %q", abs, home) + } + } + + // Refuse suspiciously shallow paths: a single segment below the root such as + // "/objects" is far more likely a typo than a real mod tree. + trimmed := strings.Trim(abs, string(filepath.Separator)) + if trimmed == "" { + return fmt.Errorf("refusing to clear filesystem root %q", abs) + } + if !strings.ContainsRune(trimmed, filepath.Separator) { + return fmt.Errorf("refusing to clear suspiciously shallow path %q; pass a --moddir at least two levels deep", abs) + } + + return nil +} + +// startsWithParent reports whether a filepath.Rel result indicates the target is an +// ancestor of home (the relative path from target to home does not need to climb out). +func startsWithParent(rel string) bool { + return rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} + +// isClearable decides whether it is safe to delete the contents of d.base. Deletion is +// allowed when the directory: +// - does not exist, or +// - is empty, or +// - already contains our ownership marker (proof this tool created it), or +// - passes the legacy extension allowlist (backward compatibility for trees written +// by versions predating the marker). +// +// Anything else - a non-empty directory of unrecognized files with no marker, such as a +// mistargeted $HOME - is refused. +func (d *DirOps) isClearable() error { + entries, err := os.ReadDir(d.base) + if err != nil { + if os.IsNotExist(err) { + return nil // absent: nothing to destroy + } + return fmt.Errorf("reading directory %s: %w", d.base, err) + } + + if len(entries) == 0 { + return nil // empty: nothing to destroy + } + + // Marker present: this tool created the directory, so clearing is safe. + for _, e := range entries { + if !e.IsDir() && e.Name() == managedMarker { + return nil + } + } + + // Backward compatibility: a directory that predates the marker but contains only + // recognized content is still ours to clear (and gains a marker going forward). + if err := d.preClearCheck(); err != nil { + return fmt.Errorf( + "%s is not empty, has no %s ownership marker, and contains unrecognized content: %w; "+ + "refusing to delete it in case --moddir is pointed at the wrong place. "+ + "If this really is a mod objects directory, remove the offending file or the whole directory by hand and re-run", + d.base, managedMarker, err) + } + return nil +} + +// writeMarker drops the ownership marker into d.base so future Clear calls recognize the +// directory as one this tool manages. +func (d *DirOps) writeMarker() error { + marker := filepath.Join(d.base, managedMarker) + content := []byte("This directory is managed by TTSModManager. It may be deleted and recreated by reverse mode.\n") + if err := os.WriteFile(marker, content, 0644); err != nil { + return fmt.Errorf("error writing ownership marker %s: %w", marker, err) + } + return nil +} + // Clear removes all contents from the base directory and recreates it. func (d *DirOps) Clear() error { log.Println("Performing safety check...") startTime := time.Now() - if err := d.preClearCheck(); err != nil { + // Path guard: refuse obviously dangerous targets regardless of contents. + if err := pathGuard(d.base); err != nil { + return fmt.Errorf("pre-clear safety check failed, operation aborted: %w", err) + } + + // Ownership guard: refuse to delete a directory this tool does not appear to own. + if err := d.isClearable(); err != nil { return fmt.Errorf("pre-clear safety check failed, operation aborted: %w", err) } @@ -111,6 +225,11 @@ func (d *DirOps) Clear() error { return fmt.Errorf("error recreating directory %s: %w", d.base, err) } + // Drop the ownership marker so this directory is recognized as ours next time. + if err := d.writeMarker(); err != nil { + return err + } + log.Printf("Cleared and recreated directory: %s", d.base) return nil } @@ -128,6 +247,11 @@ func (d *DirOps) ListFilesAndFolders(relpath string) ([]string, []string, error) if entry.IsDir() { folnames = append(folnames, filepath.Join(relpath, entry.Name())) } else { + // Hide the ownership marker: it is an internal file/ bookkeeping detail and + // must not be seen as mod content by the rest of the pipeline. + if entry.Name() == managedMarker { + continue + } fnames = append(fnames, filepath.Join(relpath, entry.Name())) } } diff --git a/file/dirops_test.go b/file/dirops_test.go index 3ae9b80..40a8481 100644 --- a/file/dirops_test.go +++ b/file/dirops_test.go @@ -114,8 +114,12 @@ func TestClearRemovesAndRecreates(t *testing.T) { if err != nil { t.Fatalf("ReadDir() after Clear: %v", err) } - if len(entries) != 0 { - t.Errorf("expected empty directory after Clear, found %d entries", len(entries)) + // Clear now drops the ownership marker into the recreated dir (#96), + // so the only permitted entry is that marker. + for _, e := range entries { + if e.Name() != managedMarker { + t.Errorf("expected only the ownership marker after Clear, found %q", e.Name()) + } } } @@ -162,3 +166,124 @@ func TestListFilesAndFolders(t *testing.T) { t.Errorf("folders want != got:\n%v\n", diff) } } + +// --- ownership-marker + path-guard tests (#96) --- + +func writeFile(t *testing.T, p, content string) { + t.Helper() + if err := os.WriteFile(p, []byte(content), 0644); err != nil { + t.Fatalf("could not write %s: %v", p, err) + } +} + +func markerExists(dir string) bool { + _, err := os.Stat(filepath.Join(dir, managedMarker)) + return err == nil +} + +// (a) An unmarked, non-empty directory containing a disallowed file is refused, and its +// contents are left untouched. +func TestClearRefusesUnmarkedForeignContent(t *testing.T) { + dir := t.TempDir() + victim := filepath.Join(dir, "important.txt") + writeFile(t, victim, "the user's real data") + + d := NewDirOps(dir) + if err := d.Clear(); err == nil { + t.Fatalf("expected Clear to refuse an unmarked directory with foreign content, got nil") + } + + if _, err := os.Stat(victim); err != nil { + t.Fatalf("Clear deleted the user's file despite refusing: %v", err) + } +} + +// (b) A directory that already carries the ownership marker clears successfully, and the +// marker is present afterwards. +func TestClearAllowsMarkedDirectory(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, managedMarker), "managed") + // Even foreign content is fine once the marker proves we own the directory. + writeFile(t, filepath.Join(dir, "leftover.bin"), "stale") + + d := NewDirOps(dir) + if err := d.Clear(); err != nil { + t.Fatalf("expected Clear to succeed on a marked directory, got %v", err) + } + + if _, err := os.Stat(filepath.Join(dir, "leftover.bin")); !os.IsNotExist(err) { + t.Fatalf("expected old content to be removed, stat err = %v", err) + } + if !markerExists(dir) { + t.Fatalf("expected marker to be re-written after Clear") + } +} + +// (c) An empty directory, and an absent directory, both clear and gain a marker. +func TestClearEmptyAndAbsentGetMarker(t *testing.T) { + t.Run("empty", func(t *testing.T) { + dir := t.TempDir() + d := NewDirOps(dir) + if err := d.Clear(); err != nil { + t.Fatalf("expected Clear to succeed on empty directory, got %v", err) + } + if !markerExists(dir) { + t.Fatalf("expected marker after clearing empty directory") + } + }) + + t.Run("absent", func(t *testing.T) { + dir := filepath.Join(t.TempDir(), "does-not-exist-yet") + d := NewDirOps(dir) + if err := d.Clear(); err != nil { + t.Fatalf("expected Clear to succeed on absent directory, got %v", err) + } + if !markerExists(dir) { + t.Fatalf("expected marker after creating absent directory") + } + }) +} + +// Backward compatibility: an unmarked directory whose files all pass the legacy +// extension allowlist still clears (and gains a marker going forward). +func TestClearAllowsLegacyRecognizedContent(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "obj.json"), "{}") + writeFile(t, filepath.Join(dir, "script.ttslua"), "-- lua") + + d := NewDirOps(dir) + if err := d.Clear(); err != nil { + t.Fatalf("expected Clear to succeed on legacy recognized content, got %v", err) + } + if !markerExists(dir) { + t.Fatalf("expected marker after clearing legacy directory") + } +} + +// (d) The path guard refuses the filesystem root and the home directory outright. +func TestPathGuardRefusesDangerousTargets(t *testing.T) { + if err := pathGuard(string(filepath.Separator)); err == nil { + t.Fatalf("expected path guard to refuse filesystem root") + } + + home, err := os.UserHomeDir() + if err != nil || home == "" { + t.Skip("no home directory available on this platform") + } + if err := pathGuard(home); err == nil { + t.Fatalf("expected path guard to refuse home directory %q", home) + } + + // Clear must also refuse the home directory, not just the raw guard. + d := NewDirOps(home) + if err := d.Clear(); err == nil { + t.Fatalf("expected Clear to refuse home directory %q", home) + } +} + +// The path guard refuses suspiciously shallow single-segment paths. +func TestPathGuardRefusesShallowPaths(t *testing.T) { + if err := pathGuard(string(filepath.Separator) + "objects"); err == nil { + t.Fatalf("expected path guard to refuse a single-segment path") + } +}