fix(ssh): preserve archived mtimes and skip symlinks on extract

The archive/tar extractor regressed two behaviors of the tar xf path
it replaced:

- It wrote files with the current time instead of the archived mtime.
  Remote sync's incremental skip cache keys on (path, mtime) and the
  engine treats it as authoritative, so every sync produced fresh
  mtimes that never matched and nothing was ever skipped (caught by
  the TestSSHSyncIncremental integration test). Restore the archived
  mtime with os.Chtimes after each regular file is written.

- It recreated symlinks, which CodeQL flagged (go/unsafe-unzip-symlink)
  because an extracted symlink can redirect a later write outside the
  extraction dir. Symlinks are not session data and any file they
  alias is extracted on its own, so skip them entirely; this removes
  the write-through risk rather than guarding it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: wesm

internal/ssh/extract.go

@@ -66,7 +66,11 @@ func extractEntry(
 	case tar.TypeReg:
 		return false, writeRegular(target, tr, hdr)
 	case tar.TypeSymlink:
-		return false, writeSymlink(dst, target, hdr)
+		// Symlinks are not session data, and a symlink restored from
+		// an archive can redirect later writes outside the extraction
+		// dir. Any regular file a symlink might alias is extracted on
+		// its own, so skip symlinks entirely.
+		return false, nil
 	case tar.TypeLink:
 		return writeHardlink(dst, target, hdr)
 	default:
@@ -143,30 +147,13 @@ func writeRegular(
 			hdr.Name, n, hdr.Size,
 		)
 	}
-	return nil
-}
-
-// writeSymlink recreates a symlink only if its target stays within
-// dst; a link pointing outside the extraction dir is rejected so it
-// cannot be used to write through to the real filesystem.
-func writeSymlink(dst, target string, hdr *tar.Header) error {
-	link := hdr.Linkname
-	var resolved string
-	if filepath.IsAbs(link) {
-		resolved = filepath.Clean(link)
-	} else {
-		resolved = filepath.Join(filepath.Dir(target), link)
-	}
-	if !within(dst, resolved) {
-		return fmt.Errorf(
-			"symlink %q points outside extraction dir", hdr.Name,
-		)
-	}
-	if err := mkdirAll(filepath.Dir(target), hdr.Name); err != nil {
-		return err
-	}
-	if err := os.Symlink(link, target); err != nil {
-		return fmt.Errorf("symlink %q: %w", hdr.Name, err)
+	// Restore the archived mtime: the incremental skip cache keys on
+	// (path, mtime), so files must keep their remote mtime across
+	// syncs or nothing is ever skipped. Best-effort: a failure only
+	// forces a redundant resync, never data loss, so it must not
+	// discard an otherwise complete extraction.
+	if !hdr.ModTime.IsZero() {
+		_ = os.Chtimes(target, hdr.ModTime, hdr.ModTime)
 	}
 	return nil
 }

internal/ssh/extract_test.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -19,6 +20,7 @@ type tarEntry struct {
 	typeflag byte
 	body     string
 	linkname string
+	modTime  time.Time
 }
 
 // buildTestTar serializes entries into an in-memory tar archive.
@@ -32,6 +34,7 @@ func buildTestTar(t *testing.T, entries []tarEntry) []byte {
 			Typeflag: e.typeflag,
 			Linkname: e.linkname,
 			Mode:     0o644,
+			ModTime:  e.modTime,
 		}
 		if e.typeflag == tar.TypeReg {
 			hdr.Size = int64(len(e.body))
@@ -136,32 +139,47 @@ func TestExtractTarStreamRejectsRelativePathEscape(t *testing.T) {
 	assert.NoFileExists(t, filepath.Join(filepath.Dir(dst), "escape.txt"))
 }
 
-func TestExtractTarStreamRejectsRelativeSymlinkEscape(t *testing.T) {
+func TestExtractTarStreamSkipsSymlinks(t *testing.T) {
 	dst := t.TempDir()
 	data := buildTestTar(t, []tarEntry{
+		{name: "home/target.txt", typeflag: tar.TypeReg, body: "data"},
+		// An in-tree link plus relative and absolute escapes: all
+		// are skipped, never created, so none can redirect a later
+		// write outside the extraction dir.
+		{
+			name:     "home/link.txt",
+			typeflag: tar.TypeSymlink,
+			linkname: "target.txt",
+		},
 		{
-			name:     "home/evil",
+			name:     "home/rel-escape",
 			typeflag: tar.TypeSymlink,
 			linkname: "../../../../etc",
 		},
-	})
-
-	_, err := extract(t, data, dst)
-	require.Error(t, err)
-}
-
-func TestExtractTarStreamRejectsAbsoluteSymlinkEscape(t *testing.T) {
-	dst := t.TempDir()
-	data := buildTestTar(t, []tarEntry{
 		{
-			name:     "home/evil",
+			name:     "home/abs-escape",
 			typeflag: tar.TypeSymlink,
 			linkname: "/etc/passwd",
 		},
 	})
 
 	_, err := extract(t, data, dst)
-	require.Error(t, err)
+	require.NoError(t, err)
+
+	// The regular file is still extracted.
+	body, err := os.ReadFile(filepath.Join(dst, "home/target.txt"))
+	require.NoError(t, err)
+	assert.Equal(t, "data", string(body))
+
+	// No symlink is created anywhere.
+	for _, name := range []string{
+		"home/link.txt", "home/rel-escape", "home/abs-escape",
+	} {
+		_, statErr := os.Lstat(filepath.Join(dst, name))
+		assert.True(
+			t, os.IsNotExist(statErr), "%s should be skipped", name,
+		)
+	}
 }
 
 func TestExtractTarStreamNormalHardlink(t *testing.T) {
@@ -180,24 +198,30 @@ func TestExtractTarStreamNormalHardlink(t *testing.T) {
 	assert.Equal(t, "shared", string(b))
 }
 
-func TestExtractTarStreamSymlinkWithinDst(t *testing.T) {
+func TestExtractTarStreamPreservesModTime(t *testing.T) {
 	dst := t.TempDir()
+	// The incremental skip cache keys on (path, mtime) and the sync
+	// engine treats it as authoritative, so extracted files must keep
+	// their archived mtime across syncs or nothing is ever skipped.
+	want := time.Date(2025, 1, 2, 3, 4, 5, 0, time.UTC)
 	data := buildTestTar(t, []tarEntry{
-		{name: "home/target.txt", typeflag: tar.TypeReg, body: "data"},
 		{
-			name:     "home/link.txt",
-			typeflag: tar.TypeSymlink,
-			linkname: "target.txt",
+			name:     "home/wes/.claude/s.jsonl",
+			typeflag: tar.TypeReg,
+			body:     "session",
+			modTime:  want,
 		},
 	})
 
-	skipped, err := extract(t, data, dst)
+	_, err := extract(t, data, dst)
 	require.NoError(t, err)
-	assert.Equal(t, 0, skipped)
 
-	got, err := os.Readlink(filepath.Join(dst, "home/link.txt"))
+	info, err := os.Stat(filepath.Join(dst, "home/wes/.claude/s.jsonl"))
 	require.NoError(t, err)
-	assert.Equal(t, "target.txt", got)
+	assert.True(
+		t, info.ModTime().Equal(want),
+		"extracted mtime = %s, want %s", info.ModTime(), want,
+	)
 }
 
 func TestExtractTarStreamCreatesDirsAndFiles(t *testing.T) {