diff --git a/CHANGELOG.md b/CHANGELOG.md index c329b385..18c0c182 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,21 @@ because it turns other people's test suites red. ## [Unreleased] +### Fixed + +- **The window no longer offers to write into a folder it cannot or should + not write into.** Started from Finder on macOS it offered `/tfg-out`, + which is read-only, so the first run ended in a refusal from the system. + Started by a double click, or from a shortcut, in the folder the program + lives in, it offered a `tfg-out` folder there - under Program Files that + is refused, and in a package manager's folder the files can go with the + next upgrade. Started in the root of a disk or in the program's own + folder, however it was started, the window now offers `tfg-out` in your + home folder. Started in any other folder, from a terminal for example, it + still offers `tfg-out` in that folder, and the command line is unchanged. + A window that remembered one of these folders from an earlier run offers + the home folder too. + ## [0.4.0] - 2026-09-25 ### Changed diff --git a/internal/guard/offereddirectory_test.go b/internal/guard/offereddirectory_test.go new file mode 100644 index 00000000..b327a177 --- /dev/null +++ b/internal/guard/offereddirectory_test.go @@ -0,0 +1,252 @@ +package guard + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/window" +) + +// What this defends. The window never offers to write into a directory it was +// not meant to write into: its own program directory or the root of a disk. +// +// Why it needed a guard of its own. destination_test.go and the first start in +// remembered_test.go stay green whatever this rule does, because under go test +// the working directory is the package directory and the program is a test +// binary somewhere under the temporary directory - the two are never the same +// directory and neither is a root, so those guards never reach the branch this +// is about. Measured on 2026-09-28 (docs/STARTING-DIRECTORY-2026-09-28.md): +// macOS starts an application from Finder in "/", which is read only, and a +// double click or an installer's shortcut starts it in its own directory. +func TestTheWindowOffersTheHomeDirectoryWhenStartedWhereItShouldNotWrite(t *testing.T) { + home := t.TempDir() + program := t.TempDir() + elsewhere := t.TempDir() + atHome := filepath.Join(home, window.OutputFolderName) + + spelled, how := anotherSpelling(t, program) + if spelled == program { + t.Fatalf("the second spelling of %s is the same text, so the case below would test nothing", program) + } + if spelled != "" && !sameDirectoryHere(t, spelled, program) { + t.Fatalf("%s (%s) is not the same directory as %s, so the case below would test the wrong thing", spelled, how, program) + } + root := rootOf(home) + if filepath.Dir(root) != root { + t.Fatalf("%s is not the root of a disk, so the case below would test the wrong thing", root) + } + if sameDirectoryHere(t, elsewhere, program) { + t.Fatalf("%s and %s are one directory, so the ordinary case would test the wrong thing", elsewhere, program) + } + + for _, c := range []struct{ what, working, home, want string }{ + {"started in its own directory, spelled as " + how, spelled, home, atHome}, + {"started in its own directory, spelled the same", program, home, atHome}, + {"started in the root of a disk, as macOS does from Finder", root, home, atHome}, + {"started anywhere else, as from a terminal", elsewhere, home, filepath.Join(elsewhere, window.OutputFolderName)}, + {"no home directory to go to", program, "", filepath.Join(program, window.OutputFolderName)}, + } { + t.Run(c.what, func(t *testing.T) { + if c.working == "" { + t.Skipf("this system allows %s, so there is no second spelling to try", how) + } + got := window.OfferedDirectory(c.working, program, c.home) + if got != c.want { + t.Errorf("started in %s with the program in %s and the home in %q, the window offers %s.\n"+ + "Want %s: a program directory or the root of a disk is not a place to write ten "+ + "thousand files into, and anywhere else is where the person chose to stand.", + c.working, program, c.home, got, c.want) + } + }) + } +} + +// A folder the old offer left in the remembered settings is not offered again. +// +// Closing the window writes down whatever the box held, chosen or not, so a +// window once started from Finder remembers "/tfg-out" and would offer it at +// every start after the offer itself was fixed. The owner decided on +// 2026-09-28 that such a value counts as nothing remembered. +func TestAFolderTheOldOfferLeftBehindIsNotOfferedAgain(t *testing.T) { + program := t.TempDir() + elsewhere := t.TempDir() + spelled, how := anotherSpelling(t, program) + if spelled != "" && !sameDirectoryHere(t, spelled, program) { + t.Fatalf("%s (%s) is not the same directory as %s", spelled, how, program) + } + root := rootOf(program) + + for _, c := range []struct { + what, remembered string + leftBehind bool + spelling bool + }{ + {"the folder under the root of a disk", filepath.Join(root, window.OutputFolderName), true, false}, + {"the folder under this program's directory, spelled as " + how, filepath.Join(spelled, window.OutputFolderName), true, true}, + {"another folder under this program's directory", filepath.Join(program, "results"), false, false}, + {"the folder under a directory somebody chose", filepath.Join(elsewhere, window.OutputFolderName), false, false}, + // What the window offers when it cannot read the working directory. + // It means "here", and the offer made afresh names the same folder in + // full - kept, it would be "/tfg-out" again at a start from Finder. + {"the bare folder name, offered when the working directory could not be read", window.OutputFolderName, true, false}, + } { + t.Run(c.what, func(t *testing.T) { + // Checked here rather than left to the join above: with no second + // spelling that join is the bare name, the last case, and this one + // would pass for its reason. + if c.spelling && spelled == "" { + t.Skipf("this system allows %s, so there is no second spelling to try", how) + } + if got := window.LeftByTheOldOffer(c.remembered, program); got != c.leftBehind { + t.Errorf("LeftByTheOldOffer(%s) = %v, want %v", c.remembered, got, c.leftBehind) + } + }) + } +} + +// And the window really does pass over it at start. The rule above is only +// half of the fix: without the call where the remembered folder is handed to +// the screens, the value left by the old offer comes back regardless. +// +// Both places the old offer could leave: the root of a disk, and the +// directory of the program that is running - here the test binary, so handing +// the rule no program directory at this call turns the second case red. +func TestTheWindowDoesNotOfferTheFolderTheOldOfferLeftBehind(t *testing.T) { + home, err := os.UserHomeDir() + if err != nil { + t.Skipf("this system has no home directory, so the fixed offer has nowhere to go: %v", err) + } + program := testBinaryDirectory(t) + + for _, c := range []struct{ what, stale string }{ + {"under the root of a disk", filepath.Join(rootOf(home), window.OutputFolderName)}, + {"under the program's own directory", filepath.Join(program, window.OutputFolderName)}, + } { + stale := c.stale + t.Run(c.what, func(t *testing.T) { + if !window.LeftByTheOldOffer(stale, program) { + t.Fatalf("%s does not count as left by the old offer, so this would test the wrong thing", stale) + } + host := newFakeHost(t) + host.Remembered().RememberDirectory(stale) + window.Open(host) + if host.content == nil { + t.Fatal("opening the window put no screen in it") + } + for _, tab := range []string{text.TabOneTarget(), text.TabPresets(), text.TabRecipe()} { + screen := selectTab(t, host.content, tab) + box := entryUnder(t, screen, text.FieldOutputDir()) + if box == nil { + t.Fatalf("the %s screen has no output directory box", tab) + } + if box.Text == stale { + t.Errorf("the %s screen offers %s, which the old offer left in the remembered settings "+ + "and which can never be written into", tab, stale) + } + if !hasSuffix(box.Text, window.OutputFolderName) { + t.Errorf("the %s screen offers %q instead of the folder of our own", tab, box.Text) + } + } + }) + } +} + +// And the window really does ask the rule when it starts. The first guard in +// this file hands OfferedDirectory three paths of its own, so a window that +// stopped asking it - or asked it without the program's directory - would +// leave that guard green, and under go test the working directory is the +// package directory, which no rule sends anywhere else. This one stands the +// test binary where a double click and Finder stand a program, with a home +// directory of its own, and reads the box on every screen (outside review of +// #146). +func TestTheWindowStartedWhereItShouldNotWriteOffersTheHomeDirectory(t *testing.T) { + program := testBinaryDirectory(t) + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + if got, err := os.UserHomeDir(); err != nil || got != home { + t.Fatalf("the home directory is %q (%v) after setting it to %s, so the cases below would test the wrong thing", got, err, home) + } + want := filepath.Join(home, window.OutputFolderName) + + for _, c := range []struct{ what, dir string }{ + {"started in its own directory, as a double click or a shortcut starts it", program}, + {"started in the root of a disk, as macOS starts it from Finder", rootOf(program)}, + } { + t.Run(c.what, func(t *testing.T) { + t.Chdir(c.dir) + if here, err := os.Getwd(); err != nil || !sameDirectoryHere(t, here, c.dir) { + t.Fatalf("the working directory is %q (%v) rather than %s, so this would test the wrong thing", here, err, c.dir) + } + host := newFakeHost(t) + window.Open(host) + if host.content == nil { + t.Fatal("opening the window put no screen in it") + } + for _, tab := range []string{text.TabOneTarget(), text.TabPresets(), text.TabRecipe()} { + screen := selectTab(t, host.content, tab) + box := entryUnder(t, screen, text.FieldOutputDir()) + if box == nil { + t.Fatalf("the %s screen has no output directory box", tab) + } + if box.Text != want { + t.Errorf("%s in %s, the %s screen offers %q.\nWant %s: neither is a place to write "+ + "ten thousand files into.", c.what, c.dir, tab, box.Text, want) + } + } + }) + } +} + +// testBinaryDirectory is the directory of the program that is running, which +// under go test is the test binary - the same answer the window gets. +func testBinaryDirectory(t *testing.T) string { + t.Helper() + exe, err := os.Executable() + if err != nil { + t.Fatalf("the test binary cannot say where it lives: %v", err) + } + return filepath.Dir(exe) +} + +// anotherSpelling is the same directory under a different text, and says how +// it was made. A link where the system allows one, letter case where the file +// system ignores it. The guard asks the file system that the two really are one +// directory before it trusts the case - a spelling that turned out to be a +// second directory would make the "own directory" case pass for the wrong +// reason. +// +// Where there is neither it gives back no spelling and says why, and only the +// case that needs one is skipped. Skipping here would skip every case of the +// guard that called it, the root of a disk and the ordinary directory with +// them (outside review of #146). +func anotherSpelling(t *testing.T, dir string) (string, string) { + t.Helper() + link := filepath.Join(t.TempDir(), "same-directory") + if err := os.Symlink(dir, link); err == nil { + return link, "a link" + } + if runtime.GOOS == "windows" || runtime.GOOS == "darwin" { + return strings.ToUpper(dir), "other letter case" + } + return "", "neither a link nor a second letter case" +} + +// sameDirectoryHere is the precondition every case above asserts rather than +// assumes: a guard that only believes it reached a state is green for the +// wrong reason the day something else changes. +func sameDirectoryHere(t *testing.T, a, b string) bool { + t.Helper() + ia, errA := os.Stat(a) + ib, errB := os.Stat(b) + return errA == nil && errB == nil && os.SameFile(ia, ib) +} + +// rootOf is the root of the disk a directory is on: "C:\" or "/". +func rootOf(dir string) string { + return filepath.VolumeName(dir) + string(filepath.Separator) +} diff --git a/internal/gui/window/offered.go b/internal/gui/window/offered.go new file mode 100644 index 00000000..bdddb17f --- /dev/null +++ b/internal/gui/window/offered.go @@ -0,0 +1,108 @@ +package window + +import ( + "os" + "path/filepath" +) + +// OfferedDirectory is the folder the window offers to write into when nobody +// has said anything, from the three things that decide it: the working +// directory, the directory the program itself lives in and the home +// directory. It asks the file system nothing but whether two directories are +// the same one, so a guard can hand it any three paths without a window. +// +// A folder of our own under the working directory, as since O103 - except +// when the working directory is one the program was never meant to write +// into. There are two of those, both measured on 2026-09-28 +// (docs/STARTING-DIRECTORY-2026-09-28.md): +// +// - the program's own directory. A double click in a file manager starts +// there, and so does the Start menu shortcut an installer makes. Under +// Program Files that is a refusal from the system at the first run, and in +// a package manager's folder it is ten thousand files in a directory the +// manager may clear at the next upgrade (O254). +// - the root of a disk. macOS starts an application from Finder in "/", +// which is read only, so the window offered "/tfg-out" and the first run +// ended in the system's refusal. +// +// Either one sends the offer to the home directory instead, however the +// program was started - the rule asks where, not how. Any other directory is +// untouched, which is what keeps a terminal as it was: whoever typed their way +// to a directory knows which one it is. +// +// Without a home directory the offer stays where it always was, because a +// path offered before is better than one made up. +func OfferedDirectory(working, program, home string) string { + if home != "" && notMeantForWriting(working, program) { + return filepath.Join(home, OutputFolderName) + } + return filepath.Join(working, OutputFolderName) +} + +// LeftByTheOldOffer says whether a remembered directory is only what the window +// used to offer from a place it should not write into: the folder of our own +// under the program's directory or under the root of a disk. +// +// Closing the window writes down whatever the box held, chosen or not, so +// everybody who once closed a window started from Finder carries "/tfg-out" +// and would be offered it at every start after the offer itself was fixed. +// That one was never somebody's choice and can never work, so it counts as +// nothing remembered (decided by the owner on 2026-09-28). +// +// A remembered folder under a program that has since moved - an older zip +// unpacked somewhere else - is not under THIS program's directory, so it +// stays. The box shows it before anything runs. +func LeftByTheOldOffer(remembered, program string) bool { + if filepath.Base(remembered) != OutputFolderName { + return false + } + return notMeantForWriting(filepath.Dir(remembered), program) +} + +// notMeantForWriting is the one rule both of the above ask. +func notMeantForWriting(dir, program string) bool { + return isRoot(dir) || sameDirectory(dir, program) +} + +// isRoot says whether a directory is the root of its disk: "/" or "C:\". The +// working directory always comes absolute from the system. +// +// A remembered bare "tfg-out" reaches here as ".", and counts as a root on +// purpose. The bare name is what startingDirectory offers when it cannot read +// the working directory, and it means "here" - which the offer works out +// afresh, spelled in full, or as the home directory when "here" is the root +// of a disk. Asking for an absolute path first (outside review of #146) would +// keep the bare name, and started from Finder it would mean "/tfg-out" again. +func isRoot(dir string) bool { + clean := filepath.Clean(dir) + return filepath.Dir(clean) == clean +} + +// sameDirectory asks the file system rather than compares the text. One +// directory has more than one spelling - letter case on Windows, a short 8.3 +// name, a link - and the texts differ while the directory is one. Anything it +// cannot look at is not the same. +func sameDirectory(a, b string) bool { + if a == "" || b == "" { + return false + } + ia, err := os.Stat(a) + if err != nil { + return false + } + ib, err := os.Stat(b) + if err != nil { + return false + } + return os.SameFile(ia, ib) +} + +// programDirectory is the directory the running program lives in, or nothing +// when the system will not say. +func programDirectory() string { + exe, err := os.Executable() + if err != nil { + return "" + } + return filepath.Dir(exe) +} diff --git a/internal/gui/window/open.go b/internal/gui/window/open.go index 1644dbfe..5601bc99 100644 --- a/internal/gui/window/open.go +++ b/internal/gui/window/open.go @@ -2,7 +2,6 @@ package window import ( "os" - "path/filepath" "fyne.io/fyne/v2" "fyne.io/fyne/v2/container" @@ -201,8 +200,11 @@ func offerWhereItLastWrote(h Host, working map[string]interface { OutDir() string SetOutDir(string) }) { + // A remembered folder the window itself once offered from a place it + // should not write into counts as nothing remembered - see + // LeftByTheOldOffer. last := h.Remembered().Directory() - if last == "" { + if last == "" || LeftByTheOldOffer(last, programDirectory()) { return } for _, screen := range working { @@ -378,16 +380,24 @@ func chooserFor(host Host, box *parts.Entry) fyne.CanvasObject { // A working directory we cannot read leaves the folder name on its own, which // lands in the same place by a shorter route, because a relative name that // means "here" is still better than a path that is wrong. +// +// Which directory the folder goes under is OfferedDirectory's answer, and +// this only asks the system for the three things it needs. func startingDirectory() string { dir, err := os.Getwd() if err != nil { return OutputFolderName } - return filepath.Join(dir, OutputFolderName) + home, err := os.UserHomeDir() + if err != nil { + home = "" + } + return OfferedDirectory(dir, programDirectory(), home) } -// OutputFolderName is the folder the window offers to write into, under -// whatever directory the program was started from. +// OutputFolderName is the folder the window offers to write into, under the +// directory the program was started from - or under the home directory when +// that one is not meant for writing, see OfferedDirectory. // // A folder of our own rather than the working directory itself, and the reason // is what a double click does. Started from a desktop, the working directory is