From 135f5e0d197f727a7bff5094f88fe18522ec3491 Mon Sep 17 00:00:00 2001 From: vrubelroman Date: Fri, 3 Jul 2026 20:36:00 +0000 Subject: [PATCH] fix: harden copy/move/delete transfers, add CI, dead code cleanup Audited every copy/move/delete path (local, remote same-host, cross-host) and fixed: - Shell command injection in same-host remote cp/mv (unescaped paths interpolated into exec strings); added remote.ShellQuote. - Missing guard against copying/moving a directory into its own subdirectory, locally and on same-host remote transfers (unbounded recursion / disk fill). New regression tests in ops_test.go. - Remote-to-remote same-path file copy silently truncating the source file via Create() before it finished being read. - Overwrite warning ("N existing target(s)") never shown for remote destinations, only local ones. - Remote same-host server-side cp never advancing the file-done counter (waited on -v output that was never requested). - Removed a dead "plan-then-confirm" subsystem (5 functions, 2 message types, 2 unreachable Update() cases) superseded by the current design. - Remote delete (SFTP) now runs as a cancellable job with the same progress modal as local delete, instead of a single blocking call. Also add a CI workflow that runs build/vet/test on every push and PR, since previously they only ran on release tags. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 32 +++ internal/fs/ops.go | 34 +++ internal/fs/ops_test.go | 50 ++++ internal/fs/remote/client.go | 34 +++ internal/ui/model.go | 439 ++++++++++------------------------- 5 files changed, 268 insertions(+), 321 deletions(-) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..4eeef40 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,32 @@ +name: CI + +on: + push: + branches: + - main + pull_request: + +jobs: + test: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version-file: go.mod + cache: true + + - name: Vendor Go modules + run: go mod vendor + + - name: Build + run: GOFLAGS=-mod=vendor go build ./... + + - name: Vet + run: GOFLAGS=-mod=vendor go vet ./... + + - name: Test + run: GOFLAGS=-mod=vendor go test ./... diff --git a/internal/fs/ops.go b/internal/fs/ops.go index 1898b6e..789d6c1 100644 --- a/internal/fs/ops.go +++ b/internal/fs/ops.go @@ -8,6 +8,7 @@ import ( "io/fs" "os" "path/filepath" + "strings" "syscall" "time" @@ -67,6 +68,13 @@ func CopyPathWithProgressContext(ctx context.Context, srcPath string, dstDir str } else if same { return "", fmt.Errorf("source and target are the same: %s", targetPath) } + if srcInfo.IsDir() { + if within, err := pathWithin(targetPath, srcPath); err != nil { + return "", err + } else if within { + return "", fmt.Errorf("cannot copy %q into itself", srcPath) + } + } if exists, err := PathExists(targetPath); err != nil { return "", err @@ -189,6 +197,13 @@ func MovePathWithProgressContext(ctx context.Context, srcPath string, dstDir str } else if same { return "", fmt.Errorf("source and target are the same: %s", targetPath) } + if srcInfo, statErr := os.Lstat(srcPath); statErr == nil && srcInfo.IsDir() { + if within, err := pathWithin(targetPath, srcPath); err != nil { + return "", err + } else if within { + return "", fmt.Errorf("cannot move %q into itself", srcPath) + } + } if exists, err := PathExists(targetPath); err != nil { return "", err @@ -527,3 +542,22 @@ func samePath(left string, right string) (bool, error) { } return leftAbs == rightAbs, nil } + +// pathWithin reports whether child is parent itself or a path nested inside +// it. Used to reject copying/moving a directory into one of its own +// descendants, which would otherwise make copyDir recurse into the very +// destination it's writing, growing without bound until the disk fills. +func pathWithin(child, parent string) (bool, error) { + childAbs, err := filepath.Abs(child) + if err != nil { + return false, err + } + parentAbs, err := filepath.Abs(parent) + if err != nil { + return false, err + } + if childAbs == parentAbs { + return true, nil + } + return strings.HasPrefix(childAbs, parentAbs+string(filepath.Separator)), nil +} diff --git a/internal/fs/ops_test.go b/internal/fs/ops_test.go index f91bfdf..8fa7071 100644 --- a/internal/fs/ops_test.go +++ b/internal/fs/ops_test.go @@ -87,6 +87,56 @@ func TestMovePathWithProgressContextCancelledBeforeStartKeepsSource(t *testing.T } } +func TestCopyPathWithProgressContextRejectsCopyIntoOwnSubdirectory(t *testing.T) { + t.Parallel() + + root := t.TempDir() + srcDir := filepath.Join(root, "src") + if err := os.MkdirAll(srcDir, 0o755); err != nil { + t.Fatalf("mkdir src: %v", err) + } + nestedDir := filepath.Join(srcDir, "nested") + if err := os.MkdirAll(nestedDir, 0o755); err != nil { + t.Fatalf("mkdir nested: %v", err) + } + + _, err := CopyPathWithProgressContext(context.Background(), srcDir, nestedDir, false, TransferStats{}, nil) + if err == nil { + t.Fatalf("expected error copying directory into its own subdirectory, got nil") + } + + entries, readErr := os.ReadDir(nestedDir) + if readErr != nil { + t.Fatalf("read nested dir: %v", readErr) + } + if len(entries) != 0 { + t.Fatalf("expected nested dir to remain empty, found %d entries", len(entries)) + } +} + +func TestMovePathWithProgressContextRejectsMoveIntoOwnSubdirectory(t *testing.T) { + t.Parallel() + + root := t.TempDir() + srcDir := filepath.Join(root, "src") + if err := os.MkdirAll(srcDir, 0o755); err != nil { + t.Fatalf("mkdir src: %v", err) + } + nestedDir := filepath.Join(srcDir, "nested") + if err := os.MkdirAll(nestedDir, 0o755); err != nil { + t.Fatalf("mkdir nested: %v", err) + } + + _, err := MovePathWithProgressContext(context.Background(), srcDir, nestedDir, false, TransferStats{}, nil) + if err == nil { + t.Fatalf("expected error moving directory into its own subdirectory, got nil") + } + + if _, statErr := os.Stat(srcDir); statErr != nil { + t.Fatalf("expected source to remain in place, stat err=%v", statErr) + } +} + func TestRenamePath(t *testing.T) { t.Parallel() diff --git a/internal/fs/remote/client.go b/internal/fs/remote/client.go index 19478ef..abf41ee 100644 --- a/internal/fs/remote/client.go +++ b/internal/fs/remote/client.go @@ -406,6 +406,15 @@ func (c *SSHClient) IsConnected() bool { return c.sftpCli != nil && c.sshConn != nil } +// ShellQuote wraps s in single quotes so it's safe to interpolate into a +// shell command string passed to Exec/ExecWithProgress, regardless of +// spaces, `$`, backticks, or other shell metacharacters it contains. Any +// single quote in s is escaped by closing the quote, emitting an escaped +// literal quote, and reopening it (the standard POSIX shell idiom). +func ShellQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" +} + // Exec runs a shell command on the remote server and returns combined output. func (c *SSHClient) Exec(cmd string) ([]byte, error) { if c.sshConn == nil { @@ -678,6 +687,12 @@ func CopyFileBetweenRemotes(srcClient, dstClient *SSHClient, srcPath, dstPath st if dstClient.sftpCli == nil { return fmt.Errorf("destination client not connected") } + if srcClient.SameHostAs(dstClient) && path.Clean(srcPath) == path.Clean(dstPath) { + // Same file on the same host: dstClient.Create() below would + // truncate it before srcFile has finished being read, corrupting + // the "copy" into an empty file. + return fmt.Errorf("source and target are the same: %s", dstPath) + } srcFile, err := srcClient.sftpCli.Open(srcPath) if err != nil { @@ -711,6 +726,13 @@ func CopyDirBetweenRemotes(srcClient, dstClient *SSHClient, srcDir, dstDir strin } func copyDirBetweenRemotes(srcClient, dstClient *SSHClient, srcDir, dstDir string, onFile func(path string, done, total int), ctx context.Context) error { + if srcClient.SameHostAs(dstClient) && remotePathWithin(dstDir, srcDir) { + // Same host, and the destination is srcDir itself or nested inside + // it: MkdirAll below would create the destination inside the very + // tree Walk is reading, so the walk would recurse into its own + // output and never terminate on its own. + return fmt.Errorf("cannot copy %q into itself", srcDir) + } done := 0 return srcClient.Walk(srcDir, func(remotePath string, info os.FileInfo, err error) error { if err != nil { @@ -815,6 +837,18 @@ func (c *SSHClient) walk(dirPath string, walkFn walkFunc, info os.FileInfo) erro // filepathSkipDir is used as a return value from Walk to skip a directory. var filepathSkipDir = fmt.Errorf("skip this directory") +// remotePathWithin reports whether child is parent itself or a path nested +// inside it, using POSIX path semantics (remote paths are always "/" +// separated regardless of the local OS). +func remotePathWithin(child, parent string) bool { + childClean := path.Clean(child) + parentClean := path.Clean(parent) + if childClean == parentClean { + return true + } + return strings.HasPrefix(childClean, parentClean+"/") +} + // isSymlinkToDir reports whether the walk entry at remotePath is a symlink // whose target is a directory. Such entries can't be copied with a plain // Open()/Create() file copy (the server rejects opening a directory for diff --git a/internal/ui/model.go b/internal/ui/model.go index d422068..2c94389 100644 --- a/internal/ui/model.go +++ b/internal/ui/model.go @@ -110,35 +110,11 @@ type opMsg struct { err error } -type copyPlanMsg struct { - kind fileOpKind - sourcePaths []string - targetDir string - overwrite bool - existingTargets int - stats vfs.TransferStats - err error - - // Remote operation fields - srcClient *remote.SSHClient - dstClient *remote.SSHClient -} - type copyProgressMsg struct { jobID int progress vfs.CopyProgress } -type deletePlanMsg struct { - kind fileOpKind - sourcePaths []string - stats vfs.TransferStats - err error - - // Remote operation — non-nil when source is remote - srcClient *remote.SSHClient -} - type archivePlanMsg struct { sourcePaths []string targetDir string @@ -570,86 +546,6 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, m.loadPreviewCmd() - case copyPlanMsg: - m.busy = false - if msg.err != nil { - log.Printf("[ERROR] copyPlanMsg: err=%v", msg.err) - m.status = msg.err.Error() - return m, nil - } - - remoteInfo := "" - if msg.srcClient != nil { - remoteInfo = " source=remote" - } - if msg.dstClient != nil { - remoteInfo += " target=remote" - } - log.Printf("[PLAN] copyPlanMsg: kind=%d sources=%d files=%d size=%d targetDir=%s%s", - msg.kind, len(msg.sourcePaths), msg.stats.FilesTotal, msg.stats.BytesTotal, msg.targetDir, remoteInfo) - - verb := operationVerb(msg.kind) - title := fmt.Sprintf("%s selected entry?", strings.Title(verb)) - if msg.srcClient != nil || msg.dstClient != nil { - title = fmt.Sprintf("%s selected entry via SFTP?", strings.Title(verb)) - } - body := strings.Join([]string{ - fmt.Sprintf("Items: %d", len(msg.sourcePaths)), - fmt.Sprintf("Files: %d", msg.stats.FilesTotal), - fmt.Sprintf("Size: %s", formatSize(msg.stats.BytesTotal, true)), - }, "\n") - note := "confirm-actions" - m.openConfirmModal(title, body, note, pendingOperation{ - kind: msg.kind, - sourcePaths: append([]string(nil), msg.sourcePaths...), - targetDir: msg.targetDir, - overwrite: msg.overwrite, - existingTargets: msg.existingTargets, - stats: msg.stats, - srcClient: msg.srcClient, - dstClient: msg.dstClient, - }) - return m, nil - - case deletePlanMsg: - m.busy = false - if msg.err != nil { - log.Printf("[ERROR] deletePlanMsg: err=%v", msg.err) - m.status = msg.err.Error() - return m, nil - } - - log.Printf("[PLAN] deletePlanMsg: kind=%d sources=%d files=%d size=%d remote=%v", - msg.kind, len(msg.sourcePaths), msg.stats.FilesTotal, msg.stats.BytesTotal, msg.srcClient != nil) - - title := "Move selected entr" + pluralSuffix(len(msg.sourcePaths), "y", "ies") + " to trash?" - if msg.srcClient != nil { - title = "Delete selected entr" + pluralSuffix(len(msg.sourcePaths), "y", "ies") + " from remote?" - } else if m.deleteKind == "permanent" { - title = "Permanently delete selected entr" + pluralSuffix(len(msg.sourcePaths), "y", "ies") + "?" - } - bodyLines := []string{ - fmt.Sprintf("Items: %d", len(msg.sourcePaths)), - fmt.Sprintf("Files: %d", msg.stats.FilesTotal), - fmt.Sprintf("Size: %s", formatSize(msg.stats.BytesTotal, true)), - } - note := "confirm-actions" - if msg.srcClient == nil { - note = fmt.Sprintf("Mode: %s (D/d to change)\nEnter / y to confirm, Esc / n to cancel", m.deleteKind) - } - m.openConfirmModal( - title, - strings.Join(bodyLines, "\n"), - note, - pendingOperation{ - kind: msg.kind, - sourcePaths: append([]string(nil), msg.sourcePaths...), - stats: msg.stats, - srcClient: msg.srcClient, - }, - ) - return m, nil - case archivePlanMsg: m.busy = false if msg.err != nil { @@ -1487,11 +1383,14 @@ func (m Model) handleModalKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return m.executeSSHHostDelete(hostName) } - // Remote delete + // Remote delete — same progress modal + cancel support as local delete if pending.srcClient != nil { - m.busy = true + if m.archiveJob != nil { + m.status = "Delete is already running" + return m, nil + } log.Printf("[MODAL] Confirm — remote delete sources=%d", len(pending.sourcePaths)) - return m, m.remoteDeleteCmd(pending.sourcePaths, pending.srcClient) + return m, m.startRemoteDeleteJob(pending.sourcePaths, pending.srcClient) } // Adjust delete kind based on mode toggle @@ -2404,21 +2303,33 @@ func (m *Model) handleTransfer(kind fileOpKind) (tea.Model, tea.Cmd) { log.Printf("[ACTION] Transfer: kind=%d active=%s srcPath=%s dstPath=%s srcRemote=%v dstRemote=%v sources=%v", kind, m.active, srcPane.Path, dstPane.Path, srcHasRemote, dstHasRemote, sources) - // Check for existing targets (fast — one Stat per top-level item) - // Skip when target is remote — local fs check doesn't apply. + // Check for existing targets (fast — one Stat per top-level item), so + // the confirm dialog below can warn about overwrites regardless of + // whether the destination is local or remote. existingTargets := 0 - if !dstHasRemote { - for _, sourcePath := range sources { - targetPath := filepath.Join(targetDir, filepath.Base(sourcePath)) - exists, err := vfs.PathExists(targetPath) + for _, sourcePath := range sources { + targetPath := filepath.Join(targetDir, filepath.Base(sourcePath)) + var exists bool + if dstHasRemote { + _, statErr := dstRemote.Client.Lstat(targetPath) + if statErr == nil { + exists = true + } else if !os.IsNotExist(statErr) { + log.Printf("[ERROR] Transfer: remote path check failed: %s err=%v", targetPath, statErr) + m.status = statErr.Error() + return m, nil + } + } else { + var err error + exists, err = vfs.PathExists(targetPath) if err != nil { log.Printf("[ERROR] Transfer: path check failed: %s err=%v", targetPath, err) m.status = err.Error() return m, nil } - if exists { - existingTargets++ - } + } + if exists { + existingTargets++ } } overwrite := existingTargets > 0 @@ -4764,134 +4675,6 @@ func remoteDirSizeCmd(client *remote.SSHClient, path string) tea.Cmd { } } -func copyPlanCmd(kind fileOpKind, sourcePaths []string, targetDir string, overwrite bool, existingTargets int) tea.Cmd { - return func() tea.Msg { - stats := vfs.TransferStats{} - var err error - for _, sourcePath := range sourcePaths { - part, statErr := vfs.CopyStats(sourcePath) - if statErr != nil { - err = statErr - break - } - stats.FilesTotal += part.FilesTotal - stats.BytesTotal += part.BytesTotal - } - return copyPlanMsg{ - kind: kind, - sourcePaths: append([]string(nil), sourcePaths...), - targetDir: targetDir, - overwrite: overwrite, - existingTargets: existingTargets, - stats: stats, - err: err, - } - } -} - -// remoteCopyPlanCmd computes transfer stats for remote-involved copy/move operations. -// For local sources it uses vfs.CopyStats; for remote sources it walks the SFTP tree. -func (m *Model) remoteCopyPlanCmd(kind fileOpKind, sourcePaths []string, targetDir string, - sourceIsRemote, targetIsRemote bool, srcClient, dstClient *remote.SSHClient) tea.Cmd { - - return func() tea.Msg { - stats := vfs.TransferStats{} - var err error - - for _, sourcePath := range sourcePaths { - if !sourceIsRemote { - // Local source — use existing stats function - part, statErr := vfs.CopyStats(sourcePath) - if statErr != nil { - err = statErr - break - } - stats.FilesTotal += part.FilesTotal - stats.BytesTotal += part.BytesTotal - } else { - // Remote source — walk via SFTP - info, statErr := srcClient.Lstat(sourcePath) - if statErr != nil { - err = statErr - break - } - if !info.IsDir() { - stats.FilesTotal++ - stats.BytesTotal += info.Size() - } else { - walkErr := srcClient.Walk(sourcePath, func(walkPath string, info os.FileInfo, walkErr error) error { - if walkErr != nil { - return walkErr - } - if !info.IsDir() { - stats.FilesTotal++ - stats.BytesTotal += info.Size() - } - return nil - }) - if walkErr != nil { - err = walkErr - break - } - } - } - } - - return copyPlanMsg{ - kind: kind, - sourcePaths: append([]string(nil), sourcePaths...), - targetDir: targetDir, - stats: stats, - err: err, - srcClient: srcClient, - dstClient: dstClient, - } - } -} - -// remoteDeletePlanCmd computes delete stats for remote paths via SFTP. -func (m *Model) remoteDeletePlanCmd(sources []string, client *remote.SSHClient) tea.Cmd { - return func() tea.Msg { - stats := vfs.TransferStats{} - var err error - - for _, sourcePath := range sources { - info, statErr := client.Lstat(sourcePath) - if statErr != nil { - err = statErr - break - } - if !info.IsDir() { - stats.FilesTotal++ - stats.BytesTotal += info.Size() - } else { - walkErr := client.Walk(sourcePath, func(walkPath string, info os.FileInfo, walkErr error) error { - if walkErr != nil { - return walkErr - } - if !info.IsDir() { - stats.FilesTotal++ - stats.BytesTotal += info.Size() - } - return nil - }) - if walkErr != nil { - err = walkErr - break - } - } - } - - return deletePlanMsg{ - kind: opDelete, - sourcePaths: append([]string(nil), sources...), - stats: stats, - err: err, - srcClient: client, - } - } -} - func (m *Model) enterArchive(selected vfs.Entry) error { pane := m.activePane() // Save current path to history before opening the archive. @@ -5302,50 +5085,6 @@ func deletePathsPermanentCmd(paths []string) tea.Cmd { } } -func trashPlanCmd(sourcePaths []string) tea.Cmd { - return func() tea.Msg { - stats := vfs.TransferStats{} - var err error - for _, sourcePath := range sourcePaths { - part, statErr := vfs.CopyStats(sourcePath) - if statErr != nil { - err = statErr - break - } - stats.FilesTotal += part.FilesTotal - stats.BytesTotal += part.BytesTotal - } - return deletePlanMsg{ - kind: opDelete, - sourcePaths: append([]string(nil), sourcePaths...), - stats: stats, - err: err, - } - } -} - -func deletePlanPermanentCmd(sourcePaths []string) tea.Cmd { - return func() tea.Msg { - stats := vfs.TransferStats{} - var err error - for _, sourcePath := range sourcePaths { - part, statErr := vfs.CopyStats(sourcePath) - if statErr != nil { - err = statErr - break - } - stats.FilesTotal += part.FilesTotal - stats.BytesTotal += part.BytesTotal - } - return deletePlanMsg{ - kind: opPermanentDelete, - sourcePaths: append([]string(nil), sourcePaths...), - stats: stats, - err: err, - } - } -} - func (m *Model) mkdirCmd(parent, name string) tea.Cmd { // Remote mkdir via SFTP if mount, ok := m.activePane().CurrentRemote(); ok { @@ -6301,20 +6040,82 @@ func (m *Model) enterRemoteDir(entry vfs.Entry) (tea.Model, tea.Cmd) { return m, m.loadPreviewCmd() } -// remoteDeleteCmd deletes files/directories on a remote host via SFTP. -func (m *Model) remoteDeleteCmd(sources []string, client *remote.SSHClient) tea.Cmd { - log.Printf("[ACTION] remoteDeleteCmd: sources=%v", sources) - return func() tea.Msg { - for _, sourcePath := range sources { - log.Printf("[JOB] remoteDelete: removing path=%s", sourcePath) - if err := client.RemoveRecursive(sourcePath); err != nil { - log.Printf("[ERROR] remoteDelete failed: path=%s err=%v", sourcePath, err) - return opMsg{kind: opDelete, sourcePath: sourcePath, err: err} - } - } - log.Printf("[DONE] remoteDelete: completed %d sources", len(sources)) - return opMsg{kind: opDelete} +// startRemoteDeleteJob deletes files/directories on a remote host via SFTP, +// with the same progress modal and mid-job cancellation support as the local +// delete job (startDeleteJob). There is no remote trash, so this is always a +// permanent delete. +func (m *Model) startRemoteDeleteJob(sources []string, client *remote.SSHClient) tea.Cmd { + m.nextArchiveJob++ + jobID := m.nextArchiveJob + ctx, cancel := context.WithCancel(context.Background()) + + m.archiveJob = &archiveJobState{ + id: jobID, + kind: "delete", + sourcePaths: append([]string(nil), sources...), + targetPath: "", + progress: vfs.CopyProgress{ + FilesDone: 0, + FilesTotal: len(sources), + BytesDone: 0, + BytesTotal: 0, + CurrentPath: sources[0], + Stage: "delete permanently", + }, + cancel: cancel, + startedAt: time.Now(), } + m.modal = modalState{kind: modalArchiveProgress} + m.status = "Deleting started" + + log.Printf("[ACTION] startRemoteDeleteJob: job=%d sources=%v", jobID, sources) + + return tea.Batch( + func() tea.Msg { + go func() { + for i, sourcePath := range sources { + select { + case <-ctx.Done(): + m.archiveProgress <- archiveDoneMsg{ + jobID: jobID, + sourcePaths: append([]string(nil), sources...), + err: context.Canceled, + } + return + default: + } + + log.Printf("[JOB] remoteDelete: job=%d removing path=%s", jobID, sourcePath) + if err := client.RemoveRecursive(sourcePath); err != nil { + log.Printf("[ERROR] remoteDelete failed: job=%d path=%s err=%v", jobID, sourcePath, err) + m.archiveProgress <- archiveDoneMsg{ + jobID: jobID, + sourcePaths: append([]string(nil), sources...), + err: fmt.Errorf("delete %s: %w", sourcePath, err), + } + return + } + + m.archiveProgress <- archiveProgressMsg{ + jobID: jobID, + progress: vfs.CopyProgress{ + FilesDone: i + 1, + FilesTotal: len(sources), + CurrentPath: sourcePath, + Stage: "delete permanently", + }, + } + } + log.Printf("[DONE] remoteDelete: job=%d completed %d sources", jobID, len(sources)) + m.archiveProgress <- archiveDoneMsg{ + jobID: jobID, + sourcePaths: append([]string(nil), sources...), + } + }() + return nil + }, + waitArchiveProgressCmd(m.archiveProgress), + ) } // remoteTransferCmd copies or moves files between local and remote filesystems. @@ -6516,8 +6317,8 @@ func (m *Model) startRemoteCopyJob(kind fileOpKind, sources []string, targetDir if srcClient.SameHostAs(dstClient) { // Same host: use server-side commands, no local streaming - srcEscaped := "'" + sourcePath + "'" - dstEscaped := "'" + targetPath + "'" + srcEscaped := remote.ShellQuote(sourcePath) + dstEscaped := remote.ShellQuote(targetPath) if kind == opMove { log.Printf("[JOB] remoteCopy — same-host move: job=%d src=%s dst=%s", jobID, sourcePath, targetPath) _, err = srcClient.Exec("mv " + srcEscaped + " " + dstEscaped) @@ -6542,20 +6343,13 @@ func (m *Model) startRemoteCopyJob(kind fileOpKind, sources []string, targetDir if !info.IsDir() { cmd = "cp" } - err = srcClient.ExecWithProgress(cmd+" "+srcEscaped+" "+dstEscaped, func(line string) { - // cp -rv outputs one line per copied file - doneFiles++ - log.Printf("[JOB] remoteCopy — same-host progress: job=%d doneFiles=%d line=%s", jobID, doneFiles, line) - m.copyProgress <- copyProgressMsg{ - jobID: jobID, - progress: vfs.CopyProgress{ - FilesDone: doneFiles, - FilesTotal: totalFiles, - CurrentPath: line, - Stage: "Copying files...", - }, - } - }) + // No live per-file progress here: without -v, cp + // prints nothing to parse, and -v's output isn't + // guaranteed to map 1:1 onto our file-only counts + // (it also reports directories). doneFiles is + // credited in one step below once the command + // completes instead. + _, err = srcClient.Exec(cmd + " " + srcEscaped + " " + dstEscaped) if err == nil { serverSide = true } else { @@ -6630,10 +6424,13 @@ func (m *Model) startRemoteCopyJob(kind fileOpKind, sources []string, targetDir log.Printf("[JOB] remoteCopy — source done: job=%d path=%s err=%v doneFiles=%d totalFiles=%d serverSide=%v", jobID, sourcePath, err, doneFiles, totalFiles, serverSide) - if !serverSide || kind == opMove { - if !trackedProgress { - doneFiles += sourceCounts[i] - } + // Credit this source's files once, however the transfer + // happened. Callbacks that already reported per-file + // progress (trackedProgress) are the only case to skip, + // to avoid double-counting; server-side cp has no such + // callback, so it must be credited here too. + if !trackedProgress { + doneFiles += sourceCounts[i] } if info != nil && !info.IsDir() { doneBytes += info.Size()