From adbfcf59c4ab3eac9d02d1602f08cc3439e9607d Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 09:50:59 -0400 Subject: [PATCH 01/13] Say which folder the agent works in, and warn when it's home Workers run in the folder the connector is started from and may change files there without asking. Nothing said so, and starting the connector from a terminal that happened to be in the home directory put the whole of it in reach. `connect setup` and `connect doctor` now report a "Work folder" check: the folder they run in, which is where the connector will work if it's started from there. The home directory, or the filesystem root, is a warning with a hint to start from a folder made for the agent or the project it should work on. Symlinks to home are seen through. --- internal/commands/connect.go | 1 + internal/commands/connect_doctor.go | 1 + internal/commands/connect_workfolder.go | 42 +++++++++++++++++ internal/commands/connect_workfolder_test.go | 49 ++++++++++++++++++++ 4 files changed, 93 insertions(+) create mode 100644 internal/commands/connect_workfolder.go create mode 100644 internal/commands/connect_workfolder_test.go diff --git a/internal/commands/connect.go b/internal/commands/connect.go index 8926e9a19..d668f5a79 100644 --- a/internal/commands/connect.go +++ b/internal/commands/connect.go @@ -620,6 +620,7 @@ func runConnectSetup(cmd *cobra.Command, app *appctx.App, f *connectSetupFlags) report.Add(checks...) report.Add(setup.TicketCheck(ctx, reader, kind)) report.Add(setup.ProjectChecks(ctx, reader, next, !exists)...) + report.Add(workFolderCheck()) // A command the person stopped did not find the connector unready: it // found nothing, and says so as an interruption. if err := ctx.Err(); err != nil { diff --git a/internal/commands/connect_doctor.go b/internal/commands/connect_doctor.go index 9953e8408..3778cbb83 100644 --- a/internal/commands/connect_doctor.go +++ b/internal/commands/connect_doctor.go @@ -81,6 +81,7 @@ func runConnectDoctor(cmd *cobra.Command, _ []string) error { } checks = append(checks, ledgerChecks(ctx, p)...) checks = append(checks, workerChecks(ctx, p.file)...) + checks = append(checks, workFolderCheck()) checks = append(checks, mcpHandshakeCheck(ctx, p.name)) result := summarizeChecks(asDoctorChecks(checks)) diff --git a/internal/commands/connect_workfolder.go b/internal/commands/connect_workfolder.go new file mode 100644 index 000000000..62a2319b3 --- /dev/null +++ b/internal/commands/connect_workfolder.go @@ -0,0 +1,42 @@ +package commands + +import ( + "fmt" + "os" + "path/filepath" + + "github.com/basecamp/basecamp-cli/internal/connector/setup" +) + +// workFolderCheck says which folder workers run in: the one the connector is +// started from (connector.DispatcherOptions.WorkDir), which setup and doctor +// take to be the one they run in. Workers may change files there without +// asking, so the home directory, or the filesystem's root, is a warning. +func workFolderCheck() setup.Check { + c := setup.Check{Name: "Work folder"} + dir, err := os.Getwd() + if err != nil { + c.Status = setup.StatusWarn + c.Message = "Couldn't tell which folder this is: " + err.Error() + return c + } + if home, err := os.UserHomeDir(); (err == nil && samePath(dir, home)) || samePath(dir, string(filepath.Separator)) { + c.Status = setup.StatusWarn + c.Message = fmt.Sprintf("Started from here, the agent works in %s, your home folder, and may change files anywhere in it without asking", dir) + c.Hint = "Start the connector from a folder made for the agent, or from the project it should work on." + return c + } + c.Status = setup.StatusPass + c.Message = fmt.Sprintf("Started from here, the agent works in %s and may change files in it without asking", dir) + return c +} + +// samePath reports whether two paths name the same folder, through symlinks. +func samePath(a, b string) bool { + ra, errA := filepath.EvalSymlinks(a) + rb, errB := filepath.EvalSymlinks(b) + if errA != nil || errB != nil { + return filepath.Clean(a) == filepath.Clean(b) + } + return ra == rb +} diff --git a/internal/commands/connect_workfolder_test.go b/internal/commands/connect_workfolder_test.go new file mode 100644 index 000000000..167b06e46 --- /dev/null +++ b/internal/commands/connect_workfolder_test.go @@ -0,0 +1,49 @@ +package commands + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/connector/setup" +) + +// Workers run in the folder the connector starts in and may change files +// there without asking, so starting from the home folder is a warning. +func TestWorkFolderCheckWarnsInTheHomeFolder(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Chdir(home) + + c := workFolderCheck() + assert.Equal(t, setup.StatusWarn, c.Status) + assert.Contains(t, c.Message, "your home folder") + assert.NotEmpty(t, c.Hint) +} + +func TestWorkFolderCheckPassesInAFolderOfItsOwn(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + work := filepath.Join(home, "agent") + require.NoError(t, os.Mkdir(work, 0o700)) + t.Chdir(work) + + c := workFolderCheck() + assert.Equal(t, setup.StatusPass, c.Status) + assert.Contains(t, c.Message, "agent") + assert.NotContains(t, c.Message, "home folder") +} + +// A symlink to the home folder is still the home folder. +func TestWorkFolderCheckSeesThroughASymlinkToHome(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + link := filepath.Join(t.TempDir(), "home-link") + require.NoError(t, os.Symlink(home, link)) + t.Chdir(link) + + assert.Equal(t, setup.StatusWarn, workFolderCheck().Status) +} From 03bb62a08e8061c68d255e1abafdd99b3593d04c Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 10:14:58 -0400 Subject: [PATCH 02/13] Show the work folder on one line, and tell a filesystem root from home The work-folder check printed the raw path, and check messages are printed as they are, so a folder name carrying a newline or a terminal escape could forge lines or clear the screen. The path is now shown through SanitizeSingleLine; comparisons still use the raw path. A filesystem root was only recognised as "/", so on Windows a volume or UNC root passed, and "/" was called "your home folder". A root is now any path that is its own parent, and its warning names it as the root of the filesystem. --- internal/commands/connect_workfolder.go | 30 ++++++++++++++----- internal/commands/connect_workfolder_test.go | 31 ++++++++++++++++++++ 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/internal/commands/connect_workfolder.go b/internal/commands/connect_workfolder.go index 62a2319b3..1454e76d4 100644 --- a/internal/commands/connect_workfolder.go +++ b/internal/commands/connect_workfolder.go @@ -6,31 +6,47 @@ import ( "path/filepath" "github.com/basecamp/basecamp-cli/internal/connector/setup" + "github.com/basecamp/basecamp-cli/internal/richtext" ) // workFolderCheck says which folder workers run in: the one the connector is // started from (connector.DispatcherOptions.WorkDir), which setup and doctor // take to be the one they run in. Workers may change files there without -// asking, so the home directory, or the filesystem's root, is a warning. +// asking, so the home directory, or a filesystem root, is a warning. func workFolderCheck() setup.Check { c := setup.Check{Name: "Work folder"} dir, err := os.Getwd() if err != nil { c.Status = setup.StatusWarn - c.Message = "Couldn't tell which folder this is: " + err.Error() + c.Message = "Couldn't tell which folder this is: " + richtext.SanitizeSingleLine(err.Error()) return c } - if home, err := os.UserHomeDir(); (err == nil && samePath(dir, home)) || samePath(dir, string(filepath.Separator)) { + // The path is shown on one line: a folder name can carry newlines or + // terminal escapes, and check messages are printed as they are. + shown := richtext.SanitizeSingleLine(dir) + switch home, err := os.UserHomeDir(); { + case err == nil && samePath(dir, home): c.Status = setup.StatusWarn - c.Message = fmt.Sprintf("Started from here, the agent works in %s, your home folder, and may change files anywhere in it without asking", dir) - c.Hint = "Start the connector from a folder made for the agent, or from the project it should work on." + c.Message = fmt.Sprintf("Started from here, the agent works in %s, your home folder, and may change files anywhere in it without asking", shown) + case isFilesystemRoot(dir): + c.Status = setup.StatusWarn + c.Message = fmt.Sprintf("Started from here, the agent works in %s, the root of the filesystem, and may change any file it can reach without asking", shown) + default: + c.Status = setup.StatusPass + c.Message = fmt.Sprintf("Started from here, the agent works in %s and may change files in it without asking", shown) return c } - c.Status = setup.StatusPass - c.Message = fmt.Sprintf("Started from here, the agent works in %s and may change files in it without asking", dir) + c.Hint = "Start the connector from a folder made for the agent, or from the project it should work on." return c } +// isFilesystemRoot reports whether dir is a root: / on Unix, a volume root +// such as C:\ or a UNC share on Windows. A root is its own parent. +func isFilesystemRoot(dir string) bool { + clean := filepath.Clean(dir) + return filepath.Dir(clean) == clean +} + // samePath reports whether two paths name the same folder, through symlinks. func samePath(a, b string) bool { ra, errA := filepath.EvalSymlinks(a) diff --git a/internal/commands/connect_workfolder_test.go b/internal/commands/connect_workfolder_test.go index 167b06e46..c71e77e76 100644 --- a/internal/commands/connect_workfolder_test.go +++ b/internal/commands/connect_workfolder_test.go @@ -47,3 +47,34 @@ func TestWorkFolderCheckSeesThroughASymlinkToHome(t *testing.T) { assert.Equal(t, setup.StatusWarn, workFolderCheck().Status) } + +// The filesystem root is a warning too, and says so rather than calling it +// the home folder. +func TestWorkFolderCheckWarnsAtTheFilesystemRoot(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Chdir(string(filepath.Separator)) + + c := workFolderCheck() + assert.Equal(t, setup.StatusWarn, c.Status) + assert.Contains(t, c.Message, "the root of the filesystem") + assert.NotContains(t, c.Message, "home folder") +} + +func TestIsFilesystemRoot(t *testing.T) { + assert.True(t, isFilesystemRoot(string(filepath.Separator))) + assert.False(t, isFilesystemRoot(t.TempDir())) +} + +// A folder name can carry newlines and terminal escapes. The check shows the +// path on one line, escaped, and still recognises the folder. +func TestWorkFolderCheckShowsTheFolderOnOneLine(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + odd := filepath.Join(t.TempDir(), "evil\n\x1b[2J✓ fake") + require.NoError(t, os.Mkdir(odd, 0o700)) + t.Chdir(odd) + + c := workFolderCheck() + assert.Equal(t, setup.StatusPass, c.Status) + assert.NotContains(t, c.Message, "\n") + assert.NotContains(t, c.Message, "\x1b") +} From 7876faeec929b90d6f0829a7aacf59201db83a09 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 10:26:17 -0400 Subject: [PATCH 03/13] See through a symlink to the root, and keep the tests portable `cd /proc/self/root` keeps that path as the working directory, and it isn't its own parent until resolved, so the root went unwarned. The root check now resolves symlinks first. The tests set USERPROFILE as well as HOME, since os.UserHomeDir reads it on Windows, and the folder-name test skips on Windows, which forbids newlines and escapes in names. --- internal/commands/connect_workfolder.go | 6 +++- internal/commands/connect_workfolder_test.go | 31 ++++++++++++++++---- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/internal/commands/connect_workfolder.go b/internal/commands/connect_workfolder.go index 1454e76d4..fa5090794 100644 --- a/internal/commands/connect_workfolder.go +++ b/internal/commands/connect_workfolder.go @@ -41,9 +41,13 @@ func workFolderCheck() setup.Check { } // isFilesystemRoot reports whether dir is a root: / on Unix, a volume root -// such as C:\ or a UNC share on Windows. A root is its own parent. +// such as C:\ or a UNC share on Windows. A root is its own parent, once +// symlinks are resolved: /proc/self/root is / by another name. func isFilesystemRoot(dir string) bool { clean := filepath.Clean(dir) + if resolved, err := filepath.EvalSymlinks(clean); err == nil { + clean = resolved + } return filepath.Dir(clean) == clean } diff --git a/internal/commands/connect_workfolder_test.go b/internal/commands/connect_workfolder_test.go index c71e77e76..fbbbc08cc 100644 --- a/internal/commands/connect_workfolder_test.go +++ b/internal/commands/connect_workfolder_test.go @@ -3,6 +3,7 @@ package commands import ( "os" "path/filepath" + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -15,7 +16,7 @@ import ( // there without asking, so starting from the home folder is a warning. func TestWorkFolderCheckWarnsInTheHomeFolder(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + setHome(t, home) t.Chdir(home) c := workFolderCheck() @@ -26,7 +27,7 @@ func TestWorkFolderCheckWarnsInTheHomeFolder(t *testing.T) { func TestWorkFolderCheckPassesInAFolderOfItsOwn(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + setHome(t, home) work := filepath.Join(home, "agent") require.NoError(t, os.Mkdir(work, 0o700)) t.Chdir(work) @@ -40,7 +41,7 @@ func TestWorkFolderCheckPassesInAFolderOfItsOwn(t *testing.T) { // A symlink to the home folder is still the home folder. func TestWorkFolderCheckSeesThroughASymlinkToHome(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + setHome(t, home) link := filepath.Join(t.TempDir(), "home-link") require.NoError(t, os.Symlink(home, link)) t.Chdir(link) @@ -51,7 +52,7 @@ func TestWorkFolderCheckSeesThroughASymlinkToHome(t *testing.T) { // The filesystem root is a warning too, and says so rather than calling it // the home folder. func TestWorkFolderCheckWarnsAtTheFilesystemRoot(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + setHome(t, t.TempDir()) t.Chdir(string(filepath.Separator)) c := workFolderCheck() @@ -65,10 +66,30 @@ func TestIsFilesystemRoot(t *testing.T) { assert.False(t, isFilesystemRoot(t.TempDir())) } +// A symlink to the root is the root. +func TestIsFilesystemRootSeesThroughASymlink(t *testing.T) { + link := filepath.Join(t.TempDir(), "root-link") + if err := os.Symlink(string(filepath.Separator), link); err != nil { + t.Skipf("can't make a symlink here: %v", err) + } + assert.True(t, isFilesystemRoot(link)) +} + +// setHome points the home folder at dir, where os.UserHomeDir looks for it on +// every platform: HOME on Unix, USERPROFILE on Windows. +func setHome(t *testing.T, dir string) { + t.Helper() + t.Setenv("HOME", dir) + t.Setenv("USERPROFILE", dir) +} + // A folder name can carry newlines and terminal escapes. The check shows the // path on one line, escaped, and still recognises the folder. func TestWorkFolderCheckShowsTheFolderOnOneLine(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + if runtime.GOOS == "windows" { + t.Skip("Windows forbids newlines and escapes in folder names") + } + setHome(t, t.TempDir()) odd := filepath.Join(t.TempDir(), "evil\n\x1b[2J✓ fake") require.NoError(t, os.Mkdir(odd, 0o700)) t.Chdir(odd) From 2b4f1ed7f77a2398dc4b49994e24c4de0833e9ea Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 10:37:07 -0400 Subject: [PATCH 04/13] Fail the work-folder check when the folder can't be read The connector can't start in a folder it can't read: the dispatcher returns os.Getwd's error. The check only warned, and readiness accepts warnings, so setup could write ready:true and doctor pass for a shell whose folder had been removed. It is now a failure, with a hint to change to a folder that exists. --- internal/commands/connect_workfolder.go | 6 ++++-- internal/commands/connect_workfolder_test.go | 21 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/internal/commands/connect_workfolder.go b/internal/commands/connect_workfolder.go index fa5090794..e4bf9c817 100644 --- a/internal/commands/connect_workfolder.go +++ b/internal/commands/connect_workfolder.go @@ -17,8 +17,10 @@ func workFolderCheck() setup.Check { c := setup.Check{Name: "Work folder"} dir, err := os.Getwd() if err != nil { - c.Status = setup.StatusWarn - c.Message = "Couldn't tell which folder this is: " + richtext.SanitizeSingleLine(err.Error()) + // The connector can't start without it (connector.NewDispatcher). + c.Status = setup.StatusFail + c.Message = "Couldn't read which folder this is: " + richtext.SanitizeSingleLine(err.Error()) + c.Hint = "Change to a folder that exists, then run this again." return c } // The path is shown on one line: a folder name can carry newlines or diff --git a/internal/commands/connect_workfolder_test.go b/internal/commands/connect_workfolder_test.go index fbbbc08cc..150d96f19 100644 --- a/internal/commands/connect_workfolder_test.go +++ b/internal/commands/connect_workfolder_test.go @@ -99,3 +99,24 @@ func TestWorkFolderCheckShowsTheFolderOnOneLine(t *testing.T) { assert.NotContains(t, c.Message, "\n") assert.NotContains(t, c.Message, "\x1b") } + +// The connector can't start in a folder it can't read, so neither setup nor +// doctor may call that ready. +func TestWorkFolderCheckFailsWhenTheFolderIsGone(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows won't remove the current directory") + } + setHome(t, t.TempDir()) + gone := filepath.Join(t.TempDir(), "gone") + require.NoError(t, os.Mkdir(gone, 0o700)) + t.Chdir(gone) + t.Setenv("PWD", gone) + require.NoError(t, os.Remove(gone)) + if _, err := os.Getwd(); err == nil { + t.Skip("this platform still reports a removed working directory") + } + + c := workFolderCheck() + assert.Equal(t, setup.StatusFail, c.Status) + assert.NotEmpty(t, c.Hint) +} From b09a6f6b7528b7a7b8f3e19fa32f571e63fc5d11 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 10:46:26 -0400 Subject: [PATCH 05/13] Resolve the path as given, and check the reported folder in tests Cleaning before resolving symlinks turned a working directory like /proc/self/root/.. into /proc/self, which isn't where it leads, so the root went unwarned. The root check now resolves the path as given and cleans only when it can't. The tests now check the message names the folder, not just a phrase around it. --- internal/commands/connect_workfolder.go | 4 +++- internal/commands/connect_workfolder_test.go | 25 ++++++++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/internal/commands/connect_workfolder.go b/internal/commands/connect_workfolder.go index e4bf9c817..3fbb7a07a 100644 --- a/internal/commands/connect_workfolder.go +++ b/internal/commands/connect_workfolder.go @@ -46,8 +46,10 @@ func workFolderCheck() setup.Check { // such as C:\ or a UNC share on Windows. A root is its own parent, once // symlinks are resolved: /proc/self/root is / by another name. func isFilesystemRoot(dir string) bool { + // Resolve the path as given: cleaning first would turn a path like + // /proc/self/root/.. into /proc/self, which is not where it leads. clean := filepath.Clean(dir) - if resolved, err := filepath.EvalSymlinks(clean); err == nil { + if resolved, err := filepath.EvalSymlinks(dir); err == nil { clean = resolved } return filepath.Dir(clean) == clean diff --git a/internal/commands/connect_workfolder_test.go b/internal/commands/connect_workfolder_test.go index 150d96f19..352cb9702 100644 --- a/internal/commands/connect_workfolder_test.go +++ b/internal/commands/connect_workfolder_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "github.com/basecamp/basecamp-cli/internal/connector/setup" + "github.com/basecamp/basecamp-cli/internal/richtext" ) // Workers run in the folder the connector starts in and may change files @@ -21,7 +22,7 @@ func TestWorkFolderCheckWarnsInTheHomeFolder(t *testing.T) { c := workFolderCheck() assert.Equal(t, setup.StatusWarn, c.Status) - assert.Contains(t, c.Message, "your home folder") + assert.Contains(t, c.Message, currentFolder(t)+", your home folder") assert.NotEmpty(t, c.Hint) } @@ -34,7 +35,7 @@ func TestWorkFolderCheckPassesInAFolderOfItsOwn(t *testing.T) { c := workFolderCheck() assert.Equal(t, setup.StatusPass, c.Status) - assert.Contains(t, c.Message, "agent") + assert.Contains(t, c.Message, currentFolder(t)) assert.NotContains(t, c.Message, "home folder") } @@ -120,3 +121,23 @@ func TestWorkFolderCheckFailsWhenTheFolderIsGone(t *testing.T) { assert.Equal(t, setup.StatusFail, c.Status) assert.NotEmpty(t, c.Hint) } + +// A symlink to the root, given with a trailing .., is still resolved as +// given: /proc/self/root/.. leads to /, though it cleans to /proc/self. +func TestIsFilesystemRootResolvesThePathAsGiven(t *testing.T) { + base := t.TempDir() + link := filepath.Join(base, "root-link") + if err := os.Symlink(string(filepath.Separator), link); err != nil { + t.Skipf("can't make a symlink here: %v", err) + } + require.NoError(t, os.Mkdir(filepath.Join(base, "sibling"), 0o700)) + assert.True(t, isFilesystemRoot(link+string(filepath.Separator)+"..")) +} + +// currentFolder is the working directory as the check reports it. +func currentFolder(t *testing.T) string { + t.Helper() + dir, err := os.Getwd() + require.NoError(t, err) + return richtext.SanitizeSingleLine(dir) +} From 968a09b8741010dd14295891bd09c924f37ae6f0 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 10:56:05 -0400 Subject: [PATCH 06/13] Compare folders by identity, not by their path's spelling The home check compared resolved paths as text, so on a filesystem that ignores letter case, as a Mac's does by default, /Users/Alice and /users/alice were different folders and the home warning was missed. samePath now asks whether the two are the same file on disk. --- internal/commands/connect_workfolder.go | 10 ++++++---- internal/commands/connect_workfolder_test.go | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/commands/connect_workfolder.go b/internal/commands/connect_workfolder.go index 3fbb7a07a..695864f53 100644 --- a/internal/commands/connect_workfolder.go +++ b/internal/commands/connect_workfolder.go @@ -55,12 +55,14 @@ func isFilesystemRoot(dir string) bool { return filepath.Dir(clean) == clean } -// samePath reports whether two paths name the same folder, through symlinks. +// samePath reports whether two paths name the same folder: the same file on +// disk, whatever the path's spelling, symlinks or letter case (a Mac's +// filesystem ignores case). Paths that can't be read compare as cleaned text. func samePath(a, b string) bool { - ra, errA := filepath.EvalSymlinks(a) - rb, errB := filepath.EvalSymlinks(b) + ia, errA := os.Stat(a) + ib, errB := os.Stat(b) if errA != nil || errB != nil { return filepath.Clean(a) == filepath.Clean(b) } - return ra == rb + return os.SameFile(ia, ib) } diff --git a/internal/commands/connect_workfolder_test.go b/internal/commands/connect_workfolder_test.go index 352cb9702..c5ecc4555 100644 --- a/internal/commands/connect_workfolder_test.go +++ b/internal/commands/connect_workfolder_test.go @@ -85,7 +85,7 @@ func setHome(t *testing.T, dir string) { } // A folder name can carry newlines and terminal escapes. The check shows the -// path on one line, escaped, and still recognises the folder. +// path on one line, escaped, and still recognizes the folder. func TestWorkFolderCheckShowsTheFolderOnOneLine(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("Windows forbids newlines and escapes in folder names") From cb281deb1cb4b617e9e479a4d9bd956220720109 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 11:17:34 -0400 Subject: [PATCH 07/13] Find home without HOME, and warn when it can't be found If os.UserHomeDir failed, HOME unset on Unix for instance, the check had no home to compare with and passed every folder but the root, the home folder included. It now falls back to the account's own record, and if that says nothing either, it warns that it couldn't check rather than passing. --- internal/commands/connect_workfolder.go | 31 ++++++++++++++++-- internal/commands/connect_workfolder_test.go | 33 ++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/internal/commands/connect_workfolder.go b/internal/commands/connect_workfolder.go index 695864f53..0ae5f647e 100644 --- a/internal/commands/connect_workfolder.go +++ b/internal/commands/connect_workfolder.go @@ -3,6 +3,7 @@ package commands import ( "fmt" "os" + "os/user" "path/filepath" "github.com/basecamp/basecamp-cli/internal/connector/setup" @@ -26,13 +27,17 @@ func workFolderCheck() setup.Check { // The path is shown on one line: a folder name can carry newlines or // terminal escapes, and check messages are printed as they are. shown := richtext.SanitizeSingleLine(dir) - switch home, err := os.UserHomeDir(); { - case err == nil && samePath(dir, home): + home := homeFolder() + switch { + case home != "" && samePath(dir, home): c.Status = setup.StatusWarn c.Message = fmt.Sprintf("Started from here, the agent works in %s, your home folder, and may change files anywhere in it without asking", shown) case isFilesystemRoot(dir): c.Status = setup.StatusWarn c.Message = fmt.Sprintf("Started from here, the agent works in %s, the root of the filesystem, and may change any file it can reach without asking", shown) + case home == "": + c.Status = setup.StatusWarn + c.Message = fmt.Sprintf("Started from here, the agent works in %s and may change files in it without asking. Couldn't tell where your home folder is, so couldn't check this isn't it", shown) default: c.Status = setup.StatusPass c.Message = fmt.Sprintf("Started from here, the agent works in %s and may change files in it without asking", shown) @@ -42,6 +47,28 @@ func workFolderCheck() setup.Check { return c } +// homeFolder is where the person's home folder is: $HOME (USERPROFILE on +// Windows), or else the account's own record, or "" when neither says. +func homeFolder() string { + if home, err := os.UserHomeDir(); err == nil { + return home + } + if home, err := accountHomeFolder(); err == nil && home != "" { + return home + } + return "" +} + +// accountHomeFolder reads the home folder from the account's record. A +// variable so a test can stand in for the system's user database. +var accountHomeFolder = func() (string, error) { + u, err := user.Current() + if err != nil { + return "", err + } + return u.HomeDir, nil +} + // isFilesystemRoot reports whether dir is a root: / on Unix, a volume root // such as C:\ or a UNC share on Windows. A root is its own parent, once // symlinks are resolved: /proc/self/root is / by another name. diff --git a/internal/commands/connect_workfolder_test.go b/internal/commands/connect_workfolder_test.go index c5ecc4555..e1e06014b 100644 --- a/internal/commands/connect_workfolder_test.go +++ b/internal/commands/connect_workfolder_test.go @@ -1,6 +1,7 @@ package commands import ( + "errors" "os" "path/filepath" "runtime" @@ -141,3 +142,35 @@ func currentFolder(t *testing.T) string { require.NoError(t, err) return richtext.SanitizeSingleLine(dir) } + +// With no HOME, the account's own record still says where home is. +func TestWorkFolderCheckFindsHomeWithoutHOME(t *testing.T) { + home := t.TempDir() + setHome(t, "") + accountHome(t, home, nil) + t.Chdir(home) + + c := workFolderCheck() + assert.Equal(t, setup.StatusWarn, c.Status) + assert.Contains(t, c.Message, currentFolder(t)+", your home folder") +} + +// When nothing says where home is, the check can't rule it out, so it warns +// rather than passing. +func TestWorkFolderCheckWarnsWhenHomeIsUnknown(t *testing.T) { + setHome(t, "") + accountHome(t, "", errors.New("no such user")) + t.Chdir(t.TempDir()) + + c := workFolderCheck() + assert.Equal(t, setup.StatusWarn, c.Status) + assert.Contains(t, c.Message, "Couldn't tell where your home folder is") +} + +// accountHome stands in for the account's record of its home folder. +func accountHome(t *testing.T, home string, err error) { + t.Helper() + prev := accountHomeFolder + accountHomeFolder = func() (string, error) { return home, err } + t.Cleanup(func() { accountHomeFolder = prev }) +} From 70454ca906376c9f70a522d5070f4afb5589aa2d Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 11:27:17 -0400 Subject: [PATCH 08/13] Fail the work-folder check when its path can't be reached getcwd can hand back a path that no longer resolves, for instance when a parent folder lost its search permission after the shell went in. The check passed, but workers open their folder by that path and would fail to start. It now checks the path can be reached, and fails if not. --- internal/commands/connect_workfolder.go | 6 ++++++ internal/commands/connect_workfolder_test.go | 21 ++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/internal/commands/connect_workfolder.go b/internal/commands/connect_workfolder.go index 0ae5f647e..e44b09c57 100644 --- a/internal/commands/connect_workfolder.go +++ b/internal/commands/connect_workfolder.go @@ -17,6 +17,12 @@ import ( func workFolderCheck() setup.Check { c := setup.Check{Name: "Work folder"} dir, err := os.Getwd() + if err == nil { + // Workers open the folder by its path (cmd.Dir), and a path can come + // back from getcwd that can no longer be reached, say when a parent + // lost its search permission after the shell went in. + _, err = os.Stat(dir) + } if err != nil { // The connector can't start without it (connector.NewDispatcher). c.Status = setup.StatusFail diff --git a/internal/commands/connect_workfolder_test.go b/internal/commands/connect_workfolder_test.go index e1e06014b..68ca004f4 100644 --- a/internal/commands/connect_workfolder_test.go +++ b/internal/commands/connect_workfolder_test.go @@ -174,3 +174,24 @@ func accountHome(t *testing.T, home string, err error) { accountHomeFolder = func() (string, error) { return home, err } t.Cleanup(func() { accountHomeFolder = prev }) } + +// A folder whose path can't be reached is one no worker can start in, even +// if the shell is still inside it. +func TestWorkFolderCheckFailsWhenThePathCantBeReached(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX permissions") + } + setHome(t, t.TempDir()) + parent := filepath.Join(t.TempDir(), "shared") + work := filepath.Join(parent, "work") + require.NoError(t, os.MkdirAll(work, 0o700)) + t.Chdir(work) + require.NoError(t, os.Chmod(parent, 0)) + t.Cleanup(func() { _ = os.Chmod(parent, 0o700) }) + if _, err := os.Stat(work); err == nil { + t.Skip("permissions aren't enforced here (running as root?)") + } + + c := workFolderCheck() + assert.Equal(t, setup.StatusFail, c.Status) +} From 40a230da2e0bf01a4a1a249339e2e9399ed7052e Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 11:37:38 -0400 Subject: [PATCH 09/13] Check the work folder can be entered, not just seen os.Stat on a folder succeeds without search permission on the folder itself, so a folder whose own permissions stopped anyone entering it still passed, while every worker, which enters it as cmd.Dir, would fail to start. The check now stats "folder/.", which only resolves through a folder that can be entered. --- internal/commands/connect_workfolder.go | 9 +++++---- internal/commands/connect_workfolder_test.go | 21 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/internal/commands/connect_workfolder.go b/internal/commands/connect_workfolder.go index e44b09c57..eb45ee3e7 100644 --- a/internal/commands/connect_workfolder.go +++ b/internal/commands/connect_workfolder.go @@ -18,10 +18,11 @@ func workFolderCheck() setup.Check { c := setup.Check{Name: "Work folder"} dir, err := os.Getwd() if err == nil { - // Workers open the folder by its path (cmd.Dir), and a path can come - // back from getcwd that can no longer be reached, say when a parent - // lost its search permission after the shell went in. - _, err = os.Stat(dir) + // Workers enter the folder by its path (cmd.Dir), and getcwd can + // return a path that can no longer be entered: a parent, or the + // folder itself, lost its search permission after the shell went in. + // "dir/." is only reachable through a folder that can be entered. + _, err = os.Stat(dir + string(filepath.Separator) + ".") } if err != nil { // The connector can't start without it (connector.NewDispatcher). diff --git a/internal/commands/connect_workfolder_test.go b/internal/commands/connect_workfolder_test.go index 68ca004f4..f9014f239 100644 --- a/internal/commands/connect_workfolder_test.go +++ b/internal/commands/connect_workfolder_test.go @@ -195,3 +195,24 @@ func TestWorkFolderCheckFailsWhenThePathCantBeReached(t *testing.T) { c := workFolderCheck() assert.Equal(t, setup.StatusFail, c.Status) } + +// A folder that can no longer be entered is one no worker can start in, even +// if the shell is still inside it. +func TestWorkFolderCheckFailsWhenTheFolderCantBeEntered(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX permissions") + } + setHome(t, t.TempDir()) + work := filepath.Join(t.TempDir(), "work") + require.NoError(t, os.Mkdir(work, 0o700)) + t.Chdir(work) + t.Setenv("PWD", "") + require.NoError(t, os.Chmod(work, 0o600)) + t.Cleanup(func() { _ = os.Chmod(work, 0o700) }) + if _, err := os.Stat(work + "/."); err == nil { + t.Skip("permissions aren't enforced here (running as root?)") + } + + c := workFolderCheck() + assert.Equal(t, setup.StatusFail, c.Status) +} From 3e9b3050a15b462904e589756fca862303cf682f Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 11:52:56 -0400 Subject: [PATCH 10/13] Count an unreachable home as unknown, and probe long paths as they are A $HOME that can't be looked at (a symlink through a folder the process can't search, say) was compared as text and never matched, so the home folder passed. A home that can't be looked at now counts as unknown, and the check warns that it couldn't tell. Appending "/." to a working directory already at the length limit made the probe fail with ENAMETOOLONG though the folder was fine; it now falls back to the path as given. --- internal/commands/connect_workfolder.go | 19 +++++++++-- internal/commands/connect_workfolder_test.go | 34 ++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/internal/commands/connect_workfolder.go b/internal/commands/connect_workfolder.go index eb45ee3e7..369280cf4 100644 --- a/internal/commands/connect_workfolder.go +++ b/internal/commands/connect_workfolder.go @@ -1,10 +1,12 @@ package commands import ( + "errors" "fmt" "os" "os/user" "path/filepath" + "syscall" "github.com/basecamp/basecamp-cli/internal/connector/setup" "github.com/basecamp/basecamp-cli/internal/richtext" @@ -23,6 +25,10 @@ func workFolderCheck() setup.Check { // folder itself, lost its search permission after the shell went in. // "dir/." is only reachable through a folder that can be entered. _, err = os.Stat(dir + string(filepath.Separator) + ".") + if errors.Is(err, syscall.ENAMETOOLONG) { + // No room for "/.": a path at the limit is checked as it is. + _, err = os.Stat(dir) + } } if err != nil { // The connector can't start without it (connector.NewDispatcher). @@ -55,17 +61,24 @@ func workFolderCheck() setup.Check { } // homeFolder is where the person's home folder is: $HOME (USERPROFILE on -// Windows), or else the account's own record, or "" when neither says. +// Windows), or else the account's own record, or "" when neither names a +// folder that can be looked at. A home that can't be looked at can't be +// compared with, so it counts as unknown rather than as somewhere else. func homeFolder() string { - if home, err := os.UserHomeDir(); err == nil { + if home, err := os.UserHomeDir(); err == nil && reachable(home) { return home } - if home, err := accountHomeFolder(); err == nil && home != "" { + if home, err := accountHomeFolder(); err == nil && home != "" && reachable(home) { return home } return "" } +func reachable(path string) bool { + _, err := os.Stat(path) + return err == nil +} + // accountHomeFolder reads the home folder from the account's record. A // variable so a test can stand in for the system's user database. var accountHomeFolder = func() (string, error) { diff --git a/internal/commands/connect_workfolder_test.go b/internal/commands/connect_workfolder_test.go index f9014f239..652844911 100644 --- a/internal/commands/connect_workfolder_test.go +++ b/internal/commands/connect_workfolder_test.go @@ -216,3 +216,37 @@ func TestWorkFolderCheckFailsWhenTheFolderCantBeEntered(t *testing.T) { c := workFolderCheck() assert.Equal(t, setup.StatusFail, c.Status) } + +// A $HOME that can't be looked at can't rule home out: with nothing else to +// go on, the check warns rather than passing. +func TestWorkFolderCheckWarnsWhenHomeCantBeLookedAt(t *testing.T) { + setHome(t, filepath.Join(t.TempDir(), "missing")) + accountHome(t, "", errors.New("no such user")) + t.Chdir(t.TempDir()) + + c := workFolderCheck() + assert.Equal(t, setup.StatusWarn, c.Status) + assert.Contains(t, c.Message, "Couldn't tell where your home folder is") +} + +// A working directory spelled right at the path-length limit is checked as it +// is: adding "/." to probe it would only make it too long. +func TestWorkFolderCheckAcceptsAPathAtTheLengthLimit(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("Linux's 4096-byte path limit") + } + setHome(t, t.TempDir()) + work := t.TempDir() + t.Chdir(work) + long := work + for len(long)+2 < 4095 { + long += "/." + } + t.Setenv("PWD", long) + if dir, err := os.Getwd(); err != nil || dir != long { + t.Skip("Getwd doesn't report PWD as given here") + } + + c := workFolderCheck() + assert.Equal(t, setup.StatusPass, c.Status, c.Message) +} From d47cfe2c6e341497a78eae82528f0d6050d73a4d Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 12:07:44 -0400 Subject: [PATCH 11/13] Check the path and the folder separately, without lengthening the path The "/." probe fell back to a plain stat at the length limit, and a plain stat doesn't need search permission on the folder itself, so a folder at the limit that couldn't be entered passed. The check now stats the path, which needs its parents searchable, and then ".", which needs the folder itself searchable, and never lengthens the path. --- internal/commands/connect_workfolder.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/internal/commands/connect_workfolder.go b/internal/commands/connect_workfolder.go index 369280cf4..a1bb0eec3 100644 --- a/internal/commands/connect_workfolder.go +++ b/internal/commands/connect_workfolder.go @@ -1,12 +1,10 @@ package commands import ( - "errors" "fmt" "os" "os/user" "path/filepath" - "syscall" "github.com/basecamp/basecamp-cli/internal/connector/setup" "github.com/basecamp/basecamp-cli/internal/richtext" @@ -23,11 +21,10 @@ func workFolderCheck() setup.Check { // Workers enter the folder by its path (cmd.Dir), and getcwd can // return a path that can no longer be entered: a parent, or the // folder itself, lost its search permission after the shell went in. - // "dir/." is only reachable through a folder that can be entered. - _, err = os.Stat(dir + string(filepath.Separator) + ".") - if errors.Is(err, syscall.ENAMETOOLONG) { - // No room for "/.": a path at the limit is checked as it is. - _, err = os.Stat(dir) + // The path resolving says the parents can be searched; "." resolving + // says the folder itself can, without lengthening the path. + if _, err = os.Stat(dir); err == nil { + _, err = os.Stat(".") } } if err != nil { From 57a27d5dd279bea7d926a11df8aadc8aefb45282 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 12:24:21 -0400 Subject: [PATCH 12/13] Recognise the root by identity before resolving symlinks A working directory can reach / through more symlinks than EvalSymlinks can expand within the path-length limit, leaving only the lexical check, which then misses the root. The root check now asks first whether the folder is / itself, the same way home is recognised. --- internal/commands/connect_workfolder.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/commands/connect_workfolder.go b/internal/commands/connect_workfolder.go index a1bb0eec3..d59e5c5ec 100644 --- a/internal/commands/connect_workfolder.go +++ b/internal/commands/connect_workfolder.go @@ -90,6 +90,11 @@ var accountHomeFolder = func() (string, error) { // such as C:\ or a UNC share on Windows. A root is its own parent, once // symlinks are resolved: /proc/self/root is / by another name. func isFilesystemRoot(dir string) bool { + // Identity first: a path can reach the root through more symlinks than + // EvalSymlinks can expand within the path-length limit. + if samePath(dir, string(filepath.Separator)) { + return true + } // Resolve the path as given: cleaning first would turn a path like // /proc/self/root/.. into /proc/self, which is not where it leads. clean := filepath.Clean(dir) From 321f72c9bb17054d9af33bcffff179f938333e8b Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Wed, 30 Sep 2026 12:33:35 -0400 Subject: [PATCH 13/13] Only a folder can be the home folder A HOME naming a file passed the reachability test, so the account's own record was never consulted and the real home folder passed. A home must now be a folder that can be looked at. --- internal/commands/connect_workfolder.go | 6 ++++-- internal/commands/connect_workfolder_test.go | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/internal/commands/connect_workfolder.go b/internal/commands/connect_workfolder.go index d59e5c5ec..a7743d156 100644 --- a/internal/commands/connect_workfolder.go +++ b/internal/commands/connect_workfolder.go @@ -71,9 +71,11 @@ func homeFolder() string { return "" } +// reachable reports whether path is a folder that can be looked at: a home +// that is a file, or can't be read, can't be compared with. func reachable(path string) bool { - _, err := os.Stat(path) - return err == nil + info, err := os.Stat(path) + return err == nil && info.IsDir() } // accountHomeFolder reads the home folder from the account's record. A diff --git a/internal/commands/connect_workfolder_test.go b/internal/commands/connect_workfolder_test.go index 652844911..371bd6865 100644 --- a/internal/commands/connect_workfolder_test.go +++ b/internal/commands/connect_workfolder_test.go @@ -250,3 +250,17 @@ func TestWorkFolderCheckAcceptsAPathAtTheLengthLimit(t *testing.T) { c := workFolderCheck() assert.Equal(t, setup.StatusPass, c.Status, c.Message) } + +// A HOME that names a file isn't a home folder: the account's record decides. +func TestWorkFolderCheckIgnoresAHomeThatIsAFile(t *testing.T) { + home := t.TempDir() + file := filepath.Join(t.TempDir(), "not-a-folder") + require.NoError(t, os.WriteFile(file, nil, 0o600)) + setHome(t, file) + accountHome(t, home, nil) + t.Chdir(home) + + c := workFolderCheck() + assert.Equal(t, setup.StatusWarn, c.Status) + assert.Contains(t, c.Message, currentFolder(t)+", your home folder") +}