From 62309d1429b7f88686e4f9b6ac50812f26fd658d Mon Sep 17 00:00:00 2001 From: cnzgray Date: Sat, 28 Feb 2026 17:22:58 +0800 Subject: [PATCH] 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 --- codeagent-wrapper/internal/logger/logger.go | 12 +++- .../internal/logger/logger_test.go | 68 +++++++++++++++++-- 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/codeagent-wrapper/internal/logger/logger.go b/codeagent-wrapper/internal/logger/logger.go index 5cfa66c..695ebc0 100644 --- a/codeagent-wrapper/internal/logger/logger.go +++ b/codeagent-wrapper/internal/logger/logger.go @@ -569,10 +569,16 @@ func isUnsafeFile(path string, tempDir string) (bool, string) { return true, fmt.Sprintf("path resolution failed: %v", err) } - // Get absolute path of tempDir - absTempDir, err := filepath.Abs(tempDir) + // Get canonical path of tempDir, resolving symlinks to match resolvedPath. + // On macOS, os.TempDir() returns /var/folders/... but EvalSymlinks resolves + // files to /private/var/folders/..., causing a spurious "outside tempDir" mismatch. + absTempDir, err := evalSymlinksFn(tempDir) if err != nil { - return true, fmt.Sprintf("tempDir resolution failed: %v", err) + // Fallback to Abs if symlink resolution fails + absTempDir, err = filepath.Abs(tempDir) + if err != nil { + return true, fmt.Sprintf("tempDir resolution failed: %v", err) + } } // Ensure resolved path is within tempDir diff --git a/codeagent-wrapper/internal/logger/logger_test.go b/codeagent-wrapper/internal/logger/logger_test.go index 5fbe217..a9bcab6 100644 --- a/codeagent-wrapper/internal/logger/logger_test.go +++ b/codeagent-wrapper/internal/logger/logger_test.go @@ -515,7 +515,10 @@ func TestLoggerIsUnsafeFileSecurityChecks(t *testing.T) { return fakeFileInfo{}, nil }) outside := filepath.Join(filepath.Dir(absTempDir), "etc", "passwd") - stubEvalSymlinks(t, func(string) (string, error) { + stubEvalSymlinks(t, func(p string) (string, error) { + if p == tempDir { + return absTempDir, nil + } return outside, nil }) unsafe, reason := isUnsafeFile(filepath.Join("..", "..", "etc", "passwd"), tempDir) @@ -529,16 +532,73 @@ func TestLoggerIsUnsafeFileSecurityChecks(t *testing.T) { return fakeFileInfo{}, nil }) otherDir := t.TempDir() - stubEvalSymlinks(t, func(string) (string, error) { - return filepath.Join(otherDir, "codeagent-wrapper-9.log"), nil + outsidePath := filepath.Join(otherDir, "codeagent-wrapper-9.log") + stubEvalSymlinks(t, func(p string) (string, error) { + if p == tempDir { + return absTempDir, nil + } + return outsidePath, nil }) - unsafe, reason := isUnsafeFile(filepath.Join(otherDir, "codeagent-wrapper-9.log"), tempDir) + unsafe, reason := isUnsafeFile(outsidePath, tempDir) if !unsafe || reason != "file is outside tempDir" { t.Fatalf("expected outside file to be rejected, got unsafe=%v reason=%q", unsafe, reason) } }) } +func TestLoggerIsUnsafeFileCanonicalizesTempDir(t *testing.T) { + stubFileStat(t, func(string) (os.FileInfo, error) { + return fakeFileInfo{}, nil + }) + + tempDir := filepath.FromSlash("/var/folders/abc/T") + canonicalTempDir := filepath.FromSlash("/private/var/folders/abc/T") + logPath := filepath.Join(tempDir, "codeagent-wrapper-1.log") + canonicalLogPath := filepath.Join(canonicalTempDir, "codeagent-wrapper-1.log") + + stubEvalSymlinks(t, func(p string) (string, error) { + switch p { + case tempDir: + return canonicalTempDir, nil + case logPath: + return canonicalLogPath, nil + default: + return p, nil + } + }) + + unsafe, reason := isUnsafeFile(logPath, tempDir) + if unsafe { + t.Fatalf("expected canonicalized tempDir to be accepted, got unsafe=%v reason=%q", unsafe, reason) + } +} + +func TestLoggerIsUnsafeFileFallsBackToAbsOnTempDirEvalFailure(t *testing.T) { + stubFileStat(t, func(string) (os.FileInfo, error) { + return fakeFileInfo{}, nil + }) + + tempDir := t.TempDir() + absTempDir, err := filepath.Abs(tempDir) + if err != nil { + t.Fatalf("filepath.Abs() error = %v", err) + } + logPath := filepath.Join(tempDir, "codeagent-wrapper-1.log") + absLogPath := filepath.Join(absTempDir, "codeagent-wrapper-1.log") + + stubEvalSymlinks(t, func(p string) (string, error) { + if p == tempDir { + return "", errors.New("boom") + } + return absLogPath, nil + }) + + unsafe, reason := isUnsafeFile(logPath, tempDir) + if unsafe { + t.Fatalf("expected Abs fallback to allow file, got unsafe=%v reason=%q", unsafe, reason) + } +} + func TestLoggerPathAndRemove(t *testing.T) { setTempDirEnv(t, t.TempDir())