Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
41 changes: 36 additions & 5 deletions internal/components/filemerge/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,13 @@ func WriteFileAtomic(path string, content []byte, perm fs.FileMode) (WriteResult
perm = 0o644
}

writePath, err := resolveExistingSymlink(path)
if err != nil {
return WriteResult{}, err
}

created := false
existing, err := readComparableFile(path)
existing, err := readComparableFile(writePath)
if err == nil {
if bytes.Equal(existing, content) {
return WriteResult{}, nil
Expand All @@ -56,8 +61,8 @@ func WriteFileAtomic(path string, content []byte, perm fs.FileMode) (WriteResult
created = true
}

dir := filepath.Dir(path)
if err := ensureAtomicParentDir(dir, path); err != nil {
dir := filepath.Dir(writePath)
if err := ensureAtomicParentDir(dir, writePath); err != nil {
return WriteResult{}, err
}

Expand Down Expand Up @@ -93,7 +98,7 @@ func WriteFileAtomic(path string, content []byte, perm fs.FileMode) (WriteResult
return WriteResult{}, fmt.Errorf("close temp file for %q: %w", path, err)
}

if err := os.Rename(tmpPath, path); err != nil {
if err := os.Rename(tmpPath, writePath); err != nil {
return WriteResult{}, fmt.Errorf("replace %q atomically: %w", path, err)
}

Expand All @@ -110,13 +115,39 @@ func WriteFileAtomic(path string, content []byte, perm fs.FileMode) (WriteResult
return WriteResult{Changed: true, Created: created}, nil
}

func resolveExistingSymlink(path string) (string, error) {
info, err := os.Lstat(path)
if err != nil {
if os.IsNotExist(err) {
return path, nil
}
return "", fmt.Errorf("stat file %q: %w", path, err)
}
if info.Mode()&os.ModeSymlink == 0 {
return path, nil
}
resolved, err := filepath.EvalSymlinks(path)
if err != nil {
return "", fmt.Errorf("resolve symlink %q: %w", path, err)
}
return resolved, nil
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

func readComparableFile(path string) ([]byte, error) {
info, err := os.Lstat(path)
if err != nil {
return nil, err
}
if info.Mode()&os.ModeSymlink != 0 {
return nil, fmt.Errorf("refusing to read symlink %q", path)
original := path
path, err = filepath.EvalSymlinks(path)
if err != nil {
return nil, fmt.Errorf("resolve symlink %q: %w", original, err)
}
info, err = os.Stat(path)
if err != nil {
return nil, err
}
}
if info.Size() > maxAtomicFileSize {
return nil, fmt.Errorf("file %q exceeds max atomic compare size %d bytes", path, maxAtomicFileSize)
Expand Down
53 changes: 47 additions & 6 deletions internal/components/filemerge/writer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestWriteFileAtomicCreatesAndIsIdempotent(t *testing.T) {
}
}

func TestWriteFileAtomicRejectsExistingSymlink(t *testing.T) {
func TestWriteFileAtomicPreservesExistingSymlink(t *testing.T) {
dir := t.TempDir()
target := filepath.Join(dir, "target.txt")
if err := os.WriteFile(target, []byte("old\n"), 0o644); err != nil {
Expand All @@ -103,12 +103,53 @@ func TestWriteFileAtomicRejectsExistingSymlink(t *testing.T) {
t.Fatalf("Symlink() error = %v", err)
}

_, err := WriteFileAtomic(path, []byte("new\n"), 0o644)
if err == nil || err.Error() == "" {
t.Fatalf("WriteFileAtomic(symlink) error = %v, want rejection", err)
result, err := WriteFileAtomic(path, []byte("new\n"), 0o644)
if err != nil {
t.Fatalf("WriteFileAtomic(symlink) error = %v, want success", err)
}
if !result.Changed || result.Created {
t.Fatalf("WriteFileAtomic(symlink) result = %+v, want Changed=true Created=false", result)
}
info, err := os.Lstat(path)
if err != nil {
t.Fatalf("Lstat(symlink) error = %v", err)
}
if info.Mode()&os.ModeSymlink == 0 {
t.Fatal("WriteFileAtomic replaced symlink, want symlink preserved")
}
if got, readErr := os.ReadFile(target); readErr != nil || string(got) != "new\n" {
t.Fatalf("target content = %q err=%v, want updated target", got, readErr)
}
}

func TestWriteFileAtomicSymlinkNoopPreservesLink(t *testing.T) {
dir := t.TempDir()
target := filepath.Join(dir, "target.txt")
content := []byte("same\n")
if err := os.WriteFile(target, content, 0o644); err != nil {
t.Fatalf("WriteFile(target) error = %v", err)
}
path := filepath.Join(dir, "linked.txt")
if err := os.Symlink(target, path); err != nil {
if isSymlinkPrivilegeError(err) {
t.Skipf("skipping: SeCreateSymbolicLinkPrivilege not held on this Windows build: %v", err)
}
t.Fatalf("Symlink() error = %v", err)
}

result, err := WriteFileAtomic(path, content, 0o644)
if err != nil {
t.Fatalf("WriteFileAtomic(symlink noop) error = %v", err)
}
if result.Changed || result.Created {
t.Fatalf("WriteFileAtomic(symlink noop) result = %+v, want no change", result)
}
info, err := os.Lstat(path)
if err != nil {
t.Fatalf("Lstat(symlink) error = %v", err)
}
if got, readErr := os.ReadFile(target); readErr != nil || string(got) != "old\n" {
t.Fatalf("target content changed through symlink: got %q err=%v", got, readErr)
if info.Mode()&os.ModeSymlink == 0 {
t.Fatal("WriteFileAtomic replaced symlink on noop write")
}
}

Expand Down
11 changes: 10 additions & 1 deletion internal/components/uninstall/cleaners.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io"
"os"
"path/filepath"
"strings"

"github.com/gentleman-programming/gentle-ai/internal/components/filemerge"
Expand Down Expand Up @@ -237,7 +238,15 @@ func readManagedFile(path string) ([]byte, error) {
return nil, fmt.Errorf("stat file %q: %w", path, err)
}
if info.Mode()&os.ModeSymlink != 0 {
return nil, fmt.Errorf("refusing to read symlink %q", path)
original := path
path, err = filepath.EvalSymlinks(path)
if err != nil {
return nil, fmt.Errorf("resolve symlink %q: %w", original, err)
}
info, err = os.Stat(path)
if err != nil {
return nil, fmt.Errorf("stat symlink target %q: %w", path, err)
}
}
if info.Size() > maxManagedFileSize {
return nil, fmt.Errorf("file %q exceeds max managed size %d bytes", path, maxManagedFileSize)
Expand Down
60 changes: 55 additions & 5 deletions internal/components/uninstall/cleaners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,25 @@ package uninstall

import (
"encoding/json"
"errors"
"os"
"path/filepath"
"strings"
"syscall"
"testing"
)

func isSymlinkPrivilegeError(err error) bool {
var le *os.LinkError
if errors.As(err, &le) {
var errno syscall.Errno
if errors.As(le.Err, &errno) {
return errno == 1314 // ERROR_PRIVILEGE_NOT_HELD
}
}
return false
}

func FuzzNormalizeJSON_NoPanic(f *testing.F) {
seeds := [][]byte{
[]byte(`{"mcpServers":{"engram":{"command":"engram"}}}`),
Expand Down Expand Up @@ -270,20 +283,57 @@ func TestRemoveJSONPaths_PreservesCRLF(t *testing.T) {
}
}

func TestReadManagedFile_RejectsSymlink(t *testing.T) {
func TestReadManagedFile_FollowsSymlink(t *testing.T) {
dir := t.TempDir()
target := filepath.Join(dir, "target.json")
if err := os.WriteFile(target, []byte(`{"ok":true}`), 0o644); err != nil {
want := []byte(`{"ok":true}`)
if err := os.WriteFile(target, want, 0o644); err != nil {
t.Fatalf("WriteFile(target) error = %v", err)
}
link := filepath.Join(dir, "link.json")
if err := os.Symlink(target, link); err != nil {
if isSymlinkPrivilegeError(err) {
t.Skipf("skipping: SeCreateSymbolicLinkPrivilege not held on this Windows build: %v", err)
}
t.Fatalf("Symlink() error = %v", err)
}

_, err := readManagedFile(link)
if err == nil || !strings.Contains(err.Error(), "refusing to read symlink") {
t.Fatalf("readManagedFile(symlink) error = %v, want symlink rejection", err)
got, err := readManagedFile(link)
if err != nil {
t.Fatalf("readManagedFile(symlink) error = %v, want success", err)
}
if string(got) != string(want) {
t.Fatalf("readManagedFile(symlink) = %q, want %q", got, want)
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

func TestRemoveFileIfExists_RemovesSymlinkNotTarget(t *testing.T) {
dir := t.TempDir()
target := filepath.Join(dir, "target.md")
content := []byte("keep target\n")
if err := os.WriteFile(target, content, 0o644); err != nil {
t.Fatalf("WriteFile(target) error = %v", err)
}
link := filepath.Join(dir, "link.md")
if err := os.Symlink(target, link); err != nil {
if isSymlinkPrivilegeError(err) {
t.Skipf("skipping: SeCreateSymbolicLinkPrivilege not held on this Windows build: %v", err)
}
t.Fatalf("Symlink() error = %v", err)
}

if err := removeFileIfExists(link); err != nil {
t.Fatalf("removeFileIfExists(symlink) error = %v", err)
}
if _, err := os.Lstat(link); !os.IsNotExist(err) {
t.Fatalf("symlink still exists after remove: %v", err)
}
got, err := os.ReadFile(target)
if err != nil {
t.Fatalf("ReadFile(target) error = %v", err)
}
if string(got) != string(content) {
t.Fatalf("target content = %q, want %q", got, content)
}
}

Expand Down