Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/selfupdate/exec_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ func swapBinary(dst, src string) error {
if cpErr := atomicWriteFromFile(dst, src); cpErr != nil {
// Roll back so we never leave the install without a binary.
if rbErr := os.Rename(old, dst); rbErr != nil {
return fmt.Errorf("installing new binary: %w (copy fallback failed: %v; rollback also failed: %v)", err, cpErr, rbErr)
return fmt.Errorf("installing new binary: %w (copy fallback failed: %w; rollback also failed: %w)", err, cpErr, rbErr)
}
return fmt.Errorf("installing new binary: %w (copy fallback failed: %v)", err, cpErr)
return fmt.Errorf("installing new binary: %w (copy fallback failed: %w)", err, cpErr)
}
_ = os.Remove(src)
}
Expand All @@ -48,7 +48,7 @@ func reExecProcess(path string, args, env []string) error {
childArgs = args[1:]
}

cmd := exec.Command(path, childArgs...) //nolint:gosec // path is our own freshly installed binary
cmd := exec.Command(path, childArgs...) //nolint:noctx // re-exec has no parent context
cmd.Env = env
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
Expand Down
69 changes: 48 additions & 21 deletions pkg/session/session_options_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package session

import (
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -77,11 +78,15 @@ func TestAddAttachedFile(t *testing.T) {
t.Parallel()
t.Run("deduplicates and preserves order", func(t *testing.T) {
t.Parallel()
dir := t.TempDir()
foo := filepath.Join(dir, "foo.go")
bar := filepath.Join(dir, "bar.go")

s := New()
s.AddAttachedFile("/abs/foo.go")
s.AddAttachedFile("/abs/bar.go")
s.AddAttachedFile("/abs/foo.go") // duplicate
assert.Equal(t, []string{"/abs/foo.go", "/abs/bar.go"}, s.AttachedFilesSnapshot())
s.AddAttachedFile(foo)
s.AddAttachedFile(bar)
s.AddAttachedFile(foo) // duplicate
assert.Equal(t, []string{foo, bar}, s.AttachedFilesSnapshot())
})

t.Run("ignores empty paths", func(t *testing.T) {
Expand All @@ -102,55 +107,77 @@ func TestAddAttachedFile(t *testing.T) {

t.Run("snapshot is independent of session storage", func(t *testing.T) {
t.Parallel()
dir := t.TempDir()
foo := filepath.Join(dir, "foo.go")

s := New()
s.AddAttachedFile("/abs/foo.go")
s.AddAttachedFile(foo)
snap := s.AttachedFilesSnapshot()
snap[0] = "mutated"
assert.Equal(t, []string{"/abs/foo.go"}, s.AttachedFilesSnapshot())
assert.Equal(t, []string{foo}, s.AttachedFilesSnapshot())
})
}

func TestRemoveAttachedFile(t *testing.T) {
t.Parallel()
t.Run("removes and reports presence", func(t *testing.T) {
t.Parallel()
dir := t.TempDir()
foo := filepath.Join(dir, "foo.go")
bar := filepath.Join(dir, "bar.go")
baz := filepath.Join(dir, "baz.go")

s := New()
s.AddAttachedFile("/abs/foo.go")
s.AddAttachedFile("/abs/bar.go")
s.AddAttachedFile("/abs/baz.go")
s.AddAttachedFile(foo)
s.AddAttachedFile(bar)
s.AddAttachedFile(baz)

assert.True(t, s.RemoveAttachedFile("/abs/bar.go"))
assert.Equal(t, []string{"/abs/foo.go", "/abs/baz.go"}, s.AttachedFilesSnapshot())
assert.True(t, s.RemoveAttachedFile(bar))
assert.Equal(t, []string{foo, baz}, s.AttachedFilesSnapshot())
})

t.Run("reports absent paths", func(t *testing.T) {
t.Parallel()
dir := t.TempDir()
foo := filepath.Join(dir, "foo.go")
other := filepath.Join(dir, "other.go")

s := New()
s.AddAttachedFile("/abs/foo.go")
assert.False(t, s.RemoveAttachedFile("/abs/other.go"))
s.AddAttachedFile(foo)
assert.False(t, s.RemoveAttachedFile(other))
assert.False(t, s.RemoveAttachedFile(""))
assert.Equal(t, []string{"/abs/foo.go"}, s.AttachedFilesSnapshot())
assert.Equal(t, []string{foo}, s.AttachedFilesSnapshot())
})

t.Run("no-op on empty list", func(t *testing.T) {
t.Parallel()
dir := t.TempDir()
foo := filepath.Join(dir, "foo.go")

s := New()
assert.False(t, s.RemoveAttachedFile("/abs/foo.go"))
assert.False(t, s.RemoveAttachedFile(foo))
assert.Empty(t, s.AttachedFilesSnapshot())
})

t.Run("file can be re-attached after removal", func(t *testing.T) {
t.Parallel()
dir := t.TempDir()
foo := filepath.Join(dir, "foo.go")

s := New()
s.AddAttachedFile("/abs/foo.go")
require.True(t, s.RemoveAttachedFile("/abs/foo.go"))
s.AddAttachedFile("/abs/foo.go")
assert.Equal(t, []string{"/abs/foo.go"}, s.AttachedFilesSnapshot())
s.AddAttachedFile(foo)
require.True(t, s.RemoveAttachedFile(foo))
s.AddAttachedFile(foo)
assert.Equal(t, []string{foo}, s.AttachedFilesSnapshot())
})
}

func TestWithAttachedFiles(t *testing.T) {
t.Parallel()
s := New(WithAttachedFiles([]string{"/abs/foo.go", "", "relative/path.go", "/abs/bar.go", "/abs/foo.go"}))
assert.Equal(t, []string{"/abs/foo.go", "/abs/bar.go"}, s.AttachedFilesSnapshot())
dir := t.TempDir()
foo := filepath.Join(dir, "foo.go")
bar := filepath.Join(dir, "bar.go")

s := New(WithAttachedFiles([]string{foo, "", "relative/path.go", bar, foo}))
assert.Equal(t, []string{foo, bar}, s.AttachedFilesSnapshot())
}
5 changes: 5 additions & 0 deletions pkg/session/sqlitestore/sqlitestore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"database/sql"
"os"
"path/filepath"
"runtime"
"testing"
"time"

Expand All @@ -18,6 +19,10 @@ import (
func TestNew_DirectoryNotWritable(t *testing.T) {
t.Parallel()

if runtime.GOOS == "windows" {
t.Skip("POSIX permission mode bits (0o555) do not prevent directory creation/writes on Windows NTFS")
}

readOnlyDir := filepath.Join(t.TempDir(), "readonly")
err := os.Mkdir(readOnlyDir, 0o555)
require.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/tools/builtin/backgroundjobs/cmd_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ func createProcessGroup(proc *os.Process) (*processGroup, error) {
if _, err := windows.SetInformationJobObject(
job,
windows.JobObjectExtendedLimitInformation,
uintptr(unsafe.Pointer(&info)),
uintptr(unsafe.Pointer(&info)), //nolint:gosec // interacting with Windows API
uint32(unsafe.Sizeof(info))); err != nil {
_ = windows.CloseHandle(job)
return nil, err
}

handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(proc.Pid))
handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(proc.Pid)) //nolint:gosec // PID fits in uint32 on Windows
if err != nil {
_ = windows.CloseHandle(job)
return nil, err
Expand Down
5 changes: 3 additions & 2 deletions pkg/tools/builtin/filesystem/agentsignore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package filesystem
import (
"os"
"path/filepath"
"runtime"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -135,8 +136,8 @@ func TestAgentsIgnoreNegationReIncludes(t *testing.T) {
}

func TestAgentsIgnoreUnreadableFileIsAnError(t *testing.T) {
if os.Geteuid() == 0 {
t.Skip("root bypasses file permissions")
if os.Geteuid() == 0 || runtime.GOOS == "windows" {
t.Skip("root or Windows bypasses file read permissions")
}
dir := t.TempDir()
path := filepath.Join(dir, fsx.AgentsIgnoreFile)
Expand Down
2 changes: 1 addition & 1 deletion pkg/tools/builtin/filesystem/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ func (t *ToolSet) executePostEditCommands(ctx context.Context, filePath string)
if len(t.postEditCommands) == 0 {
return nil
}
return runPostEditCommands(ctx, t.postEditCommands, filePath)
return runPostEditCommands(ctx, t.workingDir, t.postEditCommands, filePath)
}

// resolvePath resolves a path relative to the working directory.
Expand Down
41 changes: 32 additions & 9 deletions pkg/tools/builtin/filesystem/filesystem_paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package filesystem
import (
"os"
"path/filepath"
"runtime"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -26,19 +27,24 @@ func TestFilesystemTool_DefaultIsUnrestricted(t *testing.T) {
// No allow_list, no deny_list: everything resolvable goes through.
resolved, err := tool.resolveAndCheckPath("/etc/hosts")
require.NoError(t, err)
assert.Equal(t, "/etc/hosts", resolved)
want := "/etc/hosts"
if runtime.GOOS == "windows" {
want = filepath.Join(tmpDir, "etc", "hosts")
}
assert.Equal(t, want, resolved)

resolved, err = tool.resolveAndCheckPath("../../some/escape")
require.NoError(t, err)
// Equivalent to filepath.Clean of the joined relative escape.
want := filepath.Clean(filepath.Join(tmpDir, "..", "..", "some", "escape"))
want = filepath.Clean(filepath.Join(tmpDir, "..", "..", "some", "escape"))
assert.Equal(t, want, resolved)
}

func TestFilesystemTool_AllowList_DotMeansWorkingDir(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
tool := New(tmpDir, WithAllowList([]string{"."}))
defer tool.Close()

// Inside working dir is fine.
_, err := tool.resolveAndCheckPath("file.txt")
Expand All @@ -48,7 +54,8 @@ func TestFilesystemTool_AllowList_DotMeansWorkingDir(t *testing.T) {
require.NoError(t, err)

// Outside working dir is rejected.
_, err = tool.resolveAndCheckPath("/etc/hosts")
outsideDir := t.TempDir()
_, err = tool.resolveAndCheckPath(filepath.Join(outsideDir, "hosts"))
require.Error(t, err)
assert.Contains(t, err.Error(), "outside the allowed directories")

Expand All @@ -63,6 +70,7 @@ func TestFilesystemTool_AllowList_TildeMeansHome(t *testing.T) {
wd := t.TempDir()

tool := New(wd, WithAllowList([]string{"~"}))
defer tool.Close()

// A path under $HOME is allowed via ~/...
resolved, err := tool.resolveAndCheckPath(filepath.Join(homeDir, "doc.md"))
Expand All @@ -82,6 +90,7 @@ func TestFilesystemTool_AllowList_TildeSubdirectory(t *testing.T) {
wd := t.TempDir()

tool := New(wd, WithAllowList([]string{"~/projects"}))
defer tool.Close()

// Inside the listed subdir.
_, err := tool.resolveAndCheckPath(filepath.Join(homeDir, "projects", "app", "main.go"))
Expand All @@ -102,14 +111,16 @@ func TestFilesystemTool_AllowList_MultipleRoots(t *testing.T) {
otherDir := t.TempDir()

tool := New(wd, WithAllowList([]string{".", otherDir}))
defer tool.Close()

_, err := tool.resolveAndCheckPath("file.txt")
require.NoError(t, err)

_, err = tool.resolveAndCheckPath(filepath.Join(otherDir, "file.txt"))
require.NoError(t, err)

_, err = tool.resolveAndCheckPath("/etc/hosts")
outsideDir := t.TempDir()
_, err = tool.resolveAndCheckPath(filepath.Join(outsideDir, "hosts"))
require.Error(t, err)
}

Expand All @@ -119,6 +130,7 @@ func TestFilesystemTool_AllowList_AbsolutePath(t *testing.T) {
allowed := t.TempDir()

tool := New(wd, WithAllowList([]string{allowed}))
defer tool.Close()

// Absolute path inside the allowed root is fine.
_, err := tool.resolveAndCheckPath(filepath.Join(allowed, "x", "y.txt"))
Expand All @@ -136,6 +148,7 @@ func TestFilesystemTool_DenyList_RejectsMatchingPaths(t *testing.T) {
require.NoError(t, os.Mkdir(denied, 0o755))

tool := New(wd, WithDenyList([]string{"secret"}))
defer tool.Close()

// Anything under the denied subtree is rejected.
_, err := tool.resolveAndCheckPath("secret/key.pem")
Expand All @@ -161,6 +174,7 @@ func TestFilesystemTool_DenyList_TakesPrecedenceOverAllowList(t *testing.T) {
tool := New(wd,
WithAllowList([]string{"."}),
WithDenyList([]string{"src/vendor"}))
defer tool.Close()

// Allowed by allow-list, not denied.
_, err := tool.resolveAndCheckPath("src/main.go")
Expand All @@ -183,6 +197,7 @@ func TestFilesystemTool_AllowList_SymlinkEscapeRejected(t *testing.T) {
require.NoError(t, os.Symlink(target, link))

tool := New(wd, WithAllowList([]string{"."}))
defer tool.Close()

// Following the symlink escapes the allow-list and must be rejected.
_, err := tool.resolveAndCheckPath("escape/secret.txt")
Expand All @@ -202,6 +217,7 @@ func TestFilesystemTool_DenyList_SymlinkIntoDeniedAreaRejected(t *testing.T) {
require.NoError(t, os.Symlink(denied, link))

tool := New(wd, WithDenyList([]string{"secret"}))
defer tool.Close()

// Reading via the symlink must still trigger the deny-list.
_, err := tool.resolveAndCheckPath("shortcut/key.pem")
Expand All @@ -213,6 +229,7 @@ func TestFilesystemTool_AllowList_NewFilePath(t *testing.T) {
t.Parallel()
wd := t.TempDir()
tool := New(wd, WithAllowList([]string{"."}))
defer tool.Close()

// A path that doesn't exist yet (e.g. about to be created by write_file)
// must still be accepted when its lexical location is inside the allow-list.
Expand Down Expand Up @@ -247,6 +264,7 @@ func TestFilesystemTool_HandlersUseAllowList(t *testing.T) {
require.NoError(t, os.WriteFile(outsideFile, []byte("nope"), 0o644))

tool := New(wd, WithAllowList([]string{"."}))
defer tool.Close()

// read_file: must refuse the outside path.
res, err := tool.handleReadFile(t.Context(), ReadFileArgs{Path: outsideFile})
Expand Down Expand Up @@ -316,6 +334,7 @@ func TestFilesystemTool_HandlersUseDenyList(t *testing.T) {
require.NoError(t, os.WriteFile(filepath.Join(wd, "secrets", "key.pem"), []byte("k"), 0o644))

tool := New(wd, WithDenyList([]string{"secrets"}))
defer tool.Close()

// edit_file: must refuse to read the file in a denied directory.
res, err := tool.handleEditFile(t.Context(), EditFileArgs{
Expand Down Expand Up @@ -356,7 +375,8 @@ func TestExpandPathToken(t *testing.T) {
homeDir := t.TempDir()
resetHomeDir(t, homeDir)
wd := t.TempDir()
t.Setenv("MY_VAR", "/var/data")
srvDir := t.TempDir()
t.Setenv("MY_VAR", "var/data")
t.Setenv("EMPTY_VAR", "")
os.Unsetenv("DEFINITELY_NOT_SET")

Expand All @@ -369,11 +389,11 @@ func TestExpandPathToken(t *testing.T) {
{name: "dot", token: ".", want: wd},
{name: "tilde", token: "~", want: homeDir},
{name: "tilde-subdir", token: "~/projects", want: filepath.Join(homeDir, "projects")},
{name: "absolute", token: "/srv/data", want: "/srv/data"},
{name: "absolute", token: srvDir, want: srvDir},
{name: "relative", token: "src", want: filepath.Join(wd, "src")},
{name: "env-var", token: "$MY_VAR", want: "/var/data"},
{name: "env-var-braces", token: "${MY_VAR}", want: "/var/data"},
{name: "env-var-js-alias", token: "${env.MY_VAR}", want: "/var/data"},
{name: "env-var", token: "$MY_VAR", want: filepath.Join(wd, "var", "data")},
{name: "env-var-braces", token: "${MY_VAR}", want: filepath.Join(wd, "var", "data")},
{name: "env-var-js-alias", token: "${env.MY_VAR}", want: filepath.Join(wd, "var", "data")},
{name: "env-var-js-alias-inside-tilde", token: "~/${env.MY_VAR}", want: filepath.Join(homeDir, "var", "data")},
{name: "env-var-inside-tilde", token: "~/${MY_VAR}", want: filepath.Join(homeDir, "var", "data")},
{name: "empty", token: "", wantErr: "empty"},
Expand Down Expand Up @@ -407,6 +427,7 @@ func TestWithAllowList_RejectsUndefinedEnvVar(t *testing.T) {
os.Unsetenv("DEFINITELY_NOT_SET")
wd := t.TempDir()
tool := New(wd, WithAllowList([]string{"$DEFINITELY_NOT_SET"}))
defer tool.Close()

// The allow-list construction failed, so the toolset is disabled
// (fail-closed). All operations must be rejected.
Expand All @@ -426,6 +447,7 @@ func TestWithAllowList_AcceptsDefinedEnvVar(t *testing.T) {
t.Setenv("ALLOWED_DIR", allowed)

tool := New(wd, WithAllowList([]string{"$ALLOWED_DIR"}))
defer tool.Close()

// Inside the env-var-resolved root.
_, err := tool.resolveAndCheckPath(filepath.Join(allowed, "file.txt"))
Expand All @@ -445,6 +467,7 @@ func TestDenyList_NonExistentPath(t *testing.T) {
wd := t.TempDir()

tool := New(wd, WithDenyList([]string{"~/.ssh"}))
defer tool.Close()

// ~/.ssh does not exist yet — a write to a path inside it must be
// rejected before the directory is even created.
Expand Down
Loading
Loading