Skip to content

Commit 62309d1

Browse files
cnzgraycexll
andauthored
fix(cleanup): resolve macOS symlink mismatch causing all log files to be kept (#155)
* fix(cleanup): resolve macOS symlink mismatch causing all log files to be kept On macOS, os.TempDir() returns /var/folders/... while filepath.EvalSymlinks resolves to /private/var/folders/... (since /var is a symlink to /private/var). isUnsafeFile was comparing filepath.Abs(tempDir) against EvalSymlinks(file), causing filepath.Rel to produce a path starting with "../../../../../private/..." which triggered the "file is outside tempDir" guard. As a result, --cleanup kept all 1367 log files instead of deleting any. Fix: use evalSymlinksFn on tempDir as well, so both sides of the comparison are resolved consistently. Falls back to filepath.Abs if symlink resolution fails. * test(logger): fix isUnsafeFile eval symlinks stubs --------- Co-authored-by: cexll <evanxian9@gmail.com>
1 parent 33a94d2 commit 62309d1

2 files changed

Lines changed: 73 additions & 7 deletions

File tree

codeagent-wrapper/internal/logger/logger.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -569,10 +569,16 @@ func isUnsafeFile(path string, tempDir string) (bool, string) {
569569
return true, fmt.Sprintf("path resolution failed: %v", err)
570570
}
571571

572-
// Get absolute path of tempDir
573-
absTempDir, err := filepath.Abs(tempDir)
572+
// Get canonical path of tempDir, resolving symlinks to match resolvedPath.
573+
// On macOS, os.TempDir() returns /var/folders/... but EvalSymlinks resolves
574+
// files to /private/var/folders/..., causing a spurious "outside tempDir" mismatch.
575+
absTempDir, err := evalSymlinksFn(tempDir)
574576
if err != nil {
575-
return true, fmt.Sprintf("tempDir resolution failed: %v", err)
577+
// Fallback to Abs if symlink resolution fails
578+
absTempDir, err = filepath.Abs(tempDir)
579+
if err != nil {
580+
return true, fmt.Sprintf("tempDir resolution failed: %v", err)
581+
}
576582
}
577583

578584
// Ensure resolved path is within tempDir

codeagent-wrapper/internal/logger/logger_test.go

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,10 @@ func TestLoggerIsUnsafeFileSecurityChecks(t *testing.T) {
515515
return fakeFileInfo{}, nil
516516
})
517517
outside := filepath.Join(filepath.Dir(absTempDir), "etc", "passwd")
518-
stubEvalSymlinks(t, func(string) (string, error) {
518+
stubEvalSymlinks(t, func(p string) (string, error) {
519+
if p == tempDir {
520+
return absTempDir, nil
521+
}
519522
return outside, nil
520523
})
521524
unsafe, reason := isUnsafeFile(filepath.Join("..", "..", "etc", "passwd"), tempDir)
@@ -529,16 +532,73 @@ func TestLoggerIsUnsafeFileSecurityChecks(t *testing.T) {
529532
return fakeFileInfo{}, nil
530533
})
531534
otherDir := t.TempDir()
532-
stubEvalSymlinks(t, func(string) (string, error) {
533-
return filepath.Join(otherDir, "codeagent-wrapper-9.log"), nil
535+
outsidePath := filepath.Join(otherDir, "codeagent-wrapper-9.log")
536+
stubEvalSymlinks(t, func(p string) (string, error) {
537+
if p == tempDir {
538+
return absTempDir, nil
539+
}
540+
return outsidePath, nil
534541
})
535-
unsafe, reason := isUnsafeFile(filepath.Join(otherDir, "codeagent-wrapper-9.log"), tempDir)
542+
unsafe, reason := isUnsafeFile(outsidePath, tempDir)
536543
if !unsafe || reason != "file is outside tempDir" {
537544
t.Fatalf("expected outside file to be rejected, got unsafe=%v reason=%q", unsafe, reason)
538545
}
539546
})
540547
}
541548

549+
func TestLoggerIsUnsafeFileCanonicalizesTempDir(t *testing.T) {
550+
stubFileStat(t, func(string) (os.FileInfo, error) {
551+
return fakeFileInfo{}, nil
552+
})
553+
554+
tempDir := filepath.FromSlash("/var/folders/abc/T")
555+
canonicalTempDir := filepath.FromSlash("/private/var/folders/abc/T")
556+
logPath := filepath.Join(tempDir, "codeagent-wrapper-1.log")
557+
canonicalLogPath := filepath.Join(canonicalTempDir, "codeagent-wrapper-1.log")
558+
559+
stubEvalSymlinks(t, func(p string) (string, error) {
560+
switch p {
561+
case tempDir:
562+
return canonicalTempDir, nil
563+
case logPath:
564+
return canonicalLogPath, nil
565+
default:
566+
return p, nil
567+
}
568+
})
569+
570+
unsafe, reason := isUnsafeFile(logPath, tempDir)
571+
if unsafe {
572+
t.Fatalf("expected canonicalized tempDir to be accepted, got unsafe=%v reason=%q", unsafe, reason)
573+
}
574+
}
575+
576+
func TestLoggerIsUnsafeFileFallsBackToAbsOnTempDirEvalFailure(t *testing.T) {
577+
stubFileStat(t, func(string) (os.FileInfo, error) {
578+
return fakeFileInfo{}, nil
579+
})
580+
581+
tempDir := t.TempDir()
582+
absTempDir, err := filepath.Abs(tempDir)
583+
if err != nil {
584+
t.Fatalf("filepath.Abs() error = %v", err)
585+
}
586+
logPath := filepath.Join(tempDir, "codeagent-wrapper-1.log")
587+
absLogPath := filepath.Join(absTempDir, "codeagent-wrapper-1.log")
588+
589+
stubEvalSymlinks(t, func(p string) (string, error) {
590+
if p == tempDir {
591+
return "", errors.New("boom")
592+
}
593+
return absLogPath, nil
594+
})
595+
596+
unsafe, reason := isUnsafeFile(logPath, tempDir)
597+
if unsafe {
598+
t.Fatalf("expected Abs fallback to allow file, got unsafe=%v reason=%q", unsafe, reason)
599+
}
600+
}
601+
542602
func TestLoggerPathAndRemove(t *testing.T) {
543603
setTempDirEnv(t, t.TempDir())
544604

0 commit comments

Comments
 (0)