diff --git a/agent/agentfiles/files.go b/agent/agentfiles/files.go index b82b57ce6d..6144771601 100644 --- a/agent/agentfiles/files.go +++ b/agent/agentfiles/files.go @@ -14,6 +14,7 @@ import ( "syscall" "github.com/google/uuid" + "github.com/spf13/afero" "golang.org/x/xerrors" "cdr.dev/slog/v3" @@ -319,8 +320,14 @@ func (api *API) writeFile(ctx context.Context, r *http.Request, path string) (HT return http.StatusBadRequest, xerrors.Errorf("file path must be absolute: %q", path) } + resolved, err := api.resolveSymlink(path) + if err != nil { + return http.StatusInternalServerError, xerrors.Errorf("resolve symlink %q: %w", path, err) + } + path = resolved + dir := filepath.Dir(path) - err := api.filesystem.MkdirAll(dir, 0o755) + err = api.filesystem.MkdirAll(dir, 0o755) if err != nil { status := http.StatusInternalServerError switch { @@ -410,6 +417,12 @@ func (api *API) editFile(ctx context.Context, path string, edits []workspacesdk. return http.StatusBadRequest, xerrors.New("must specify at least one edit") } + resolved, err := api.resolveSymlink(path) + if err != nil { + return http.StatusInternalServerError, xerrors.Errorf("resolve symlink %q: %w", path, err) + } + path = resolved + f, err := api.filesystem.Open(path) if err != nil { status := http.StatusInternalServerError @@ -510,6 +523,52 @@ func (api *API) atomicWrite(ctx context.Context, path string, mode *os.FileMode, return 0, nil } +// resolveSymlink resolves a path through any symlinks so that +// subsequent operations (such as atomic rename) target the real +// file instead of replacing the symlink itself. +// +// The filesystem must implement afero.Lstater and afero.LinkReader +// for resolution to occur; if it does not (e.g. MemMapFs), the +// path is returned unchanged. +func (api *API) resolveSymlink(path string) (string, error) { + const maxDepth = 10 + + lstater, hasLstat := api.filesystem.(afero.Lstater) + if !hasLstat { + return path, nil + } + reader, hasReadlink := api.filesystem.(afero.LinkReader) + if !hasReadlink { + return path, nil + } + + for range maxDepth { + info, _, err := lstater.LstatIfPossible(path) + if err != nil { + // If the file does not exist yet (new file write), + // there is nothing to resolve. + if errors.Is(err, os.ErrNotExist) { + return path, nil + } + return "", err + } + if info.Mode()&os.ModeSymlink == 0 { + return path, nil + } + + target, err := reader.ReadlinkIfPossible(path) + if err != nil { + return "", err + } + if !filepath.IsAbs(target) { + target = filepath.Join(filepath.Dir(path), target) + } + path = target + } + + return "", xerrors.Errorf("too many levels of symlinks resolving %q", path) +} + // fuzzyReplace attempts to find `search` inside `content` and replace it // with `replace`. It uses a cascading match strategy inspired by // openai/codex's apply_patch: diff --git a/agent/agentfiles/files_test.go b/agent/agentfiles/files_test.go index d962b80da0..cf4a7aa7ee 100644 --- a/agent/agentfiles/files_test.go +++ b/agent/agentfiles/files_test.go @@ -1395,3 +1395,105 @@ func TestReadFileLines(t *testing.T) { }) } } + +func TestWriteFile_FollowsSymlinks(t *testing.T) { + t.Parallel() + + if runtime.GOOS == "windows" { + t.Skip("symlinks are not reliably supported on Windows") + } + + dir := t.TempDir() + logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug) + osFs := afero.NewOsFs() + api := agentfiles.NewAPI(logger, osFs, nil) + + // Create a real file and a symlink pointing to it. + realPath := filepath.Join(dir, "real.txt") + err := afero.WriteFile(osFs, realPath, []byte("original"), 0o644) + require.NoError(t, err) + + linkPath := filepath.Join(dir, "link.txt") + err = os.Symlink(realPath, linkPath) + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + // Write through the symlink. + w := httptest.NewRecorder() + r := httptest.NewRequestWithContext(ctx, http.MethodPost, + fmt.Sprintf("/write-file?path=%s", linkPath), + bytes.NewReader([]byte("updated"))) + api.Routes().ServeHTTP(w, r) + require.Equal(t, http.StatusOK, w.Code) + + // The symlink must still be a symlink. + fi, err := os.Lstat(linkPath) + require.NoError(t, err) + require.NotZero(t, fi.Mode()&os.ModeSymlink, "symlink was replaced") + + // The real file must have the new content. + data, err := os.ReadFile(realPath) + require.NoError(t, err) + require.Equal(t, "updated", string(data)) +} + +func TestEditFiles_FollowsSymlinks(t *testing.T) { + t.Parallel() + + if runtime.GOOS == "windows" { + t.Skip("symlinks are not reliably supported on Windows") + } + + dir := t.TempDir() + logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug) + osFs := afero.NewOsFs() + api := agentfiles.NewAPI(logger, osFs, nil) + + // Create a real file and a symlink pointing to it. + realPath := filepath.Join(dir, "real.txt") + err := afero.WriteFile(osFs, realPath, []byte("hello world"), 0o644) + require.NoError(t, err) + + linkPath := filepath.Join(dir, "link.txt") + err = os.Symlink(realPath, linkPath) + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + body := workspacesdk.FileEditRequest{ + Files: []workspacesdk.FileEdits{ + { + Path: linkPath, + Edits: []workspacesdk.FileEdit{ + { + Search: "hello", + Replace: "goodbye", + }, + }, + }, + }, + } + buf := bytes.NewBuffer(nil) + enc := json.NewEncoder(buf) + enc.SetEscapeHTML(false) + err = enc.Encode(body) + require.NoError(t, err) + + w := httptest.NewRecorder() + r := httptest.NewRequestWithContext(ctx, http.MethodPost, "/edit-files", buf) + api.Routes().ServeHTTP(w, r) + require.Equal(t, http.StatusOK, w.Code) + + // The symlink must still be a symlink. + fi, err := os.Lstat(linkPath) + require.NoError(t, err) + require.NotZero(t, fi.Mode()&os.ModeSymlink, "symlink was replaced") + + // The real file must have the edited content. + data, err := os.ReadFile(realPath) + require.NoError(t, err) + require.Equal(t, "goodbye world", string(data)) +}