From 16edcbdd5b8646460799a9db48e7457d10433bdf Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 24 Mar 2026 14:39:55 +0200 Subject: [PATCH] fix(agent/agentfiles): follow symlinks in write_file and edit_files (#23478) Both write_file and edit_files use atomic writes (write to temp file, then rename). Since rename operates on directory entries, it replaces symlinks with regular files instead of writing through the link to the target. Add resolveSymlink() that uses afero.Lstater/LinkReader to resolve symlink chains (up to 10 levels) before the atomic write. Both writeFile and editFile resolve the path before any filesystem operations, matching the behavior of 'echo content > symlink'. Gracefully no-ops on filesystems that don't support symlinks (e.g. MemMapFs used in existing tests). --- agent/agentfiles/files.go | 61 +++++++++++++++++++- agent/agentfiles/files_test.go | 102 +++++++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 1 deletion(-) 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)) +}