From 97e7f0f4b8266794b0a3e9db584102b6a39b1766 Mon Sep 17 00:00:00 2001 From: mornyx Date: Tue, 6 Oct 2026 22:02:32 +0800 Subject: [PATCH] fix(fs): enforce the no-overwrite contract on companion uploads CopyFile delegates unconditionally to `drive9 fs cp`, which treats every drive9 destination as overwrite-enabled, so `ti fs cp --from-local` (and stdin uploads) silently replaced existing remote targets even with --overwrite at its documented default of false. Probe the destination with `drive9 fs stat` before uploading and fail with the same fs.target_exists error the direct copy path uses; --overwrite, --append, and --resume bypass the probe, and non-not-found probe errors fail closed. Fixes #72 --- internal/fs/drive9_companion.go | 35 ++++++++++++++ internal/fs/drive9_companion_test.go | 71 ++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/internal/fs/drive9_companion.go b/internal/fs/drive9_companion.go index a116c54..11aa725 100644 --- a/internal/fs/drive9_companion.go +++ b/internal/fs/drive9_companion.go @@ -405,6 +405,9 @@ func (s Service) drive9CopyFile(ctx context.Context, opts CopyFileOptions) (File if err != nil { return FileOperationResult{}, err } + if err := s.ensureDrive9UploadTargetAbsent(ctx, opts); err != nil { + return FileOperationResult{}, err + } if opts.CreateParents && strings.TrimSpace(opts.ToLocal) != "" { if err := os.MkdirAll(filepath.Dir(opts.ToLocal), 0o755); err != nil { return FileOperationResult{}, apperr.Wrap("fs.create_local_parent", "runtime", 1, fmt.Sprintf("create parent directories for %q", opts.ToLocal), err) @@ -429,6 +432,38 @@ func (s Service) drive9CopyFile(ctx context.Context, opts CopyFileOptions) (File return FileOperationResult{Operation: "copy_file", SourcePath: source, TargetPath: target, Status: status}, nil } +// ensureDrive9UploadTargetAbsent enforces the documented --overwrite=false +// contract for companion uploads. drive9CopyArgs has no overwrite flag to +// forward because the delegated `drive9 fs cp` treats every drive9 +// destination as overwrite-enabled, so ti must probe the target itself. +func (s Service) ensureDrive9UploadTargetAbsent(ctx context.Context, opts CopyFileOptions) error { + if opts.Overwrite || opts.Append || opts.Resume { + return nil + } + target := "" + switch { + case opts.FromStdin && opts.ToRemote != "": + target = opts.ToRemote + case opts.FromLocal != "" && opts.ToRemote != "": + target = opts.ToRemote + } + if target == "" { + return nil + } + targetPath, err := normalizeRemotePath(target) + if err != nil { + return err + } + _, statErr := s.drive9RunTransientRetry(ctx, opts.Profile, []string{"fs", "stat", "--output", "json", drive9Remote(targetPath)}, true) + if statErr == nil { + return apperr.New("fs.target_exists", "usage", 2, fmt.Sprintf("remote target %q already exists; pass --overwrite to replace it", targetPath)) + } + if isDrive9NotFound(statErr) { + return nil + } + return statErr +} + func (s Service) drive9ReadFile(ctx context.Context, opts ReadFileOptions) ([]byte, error) { remotePath, err := normalizeRemotePath(opts.Path) if err != nil { diff --git a/internal/fs/drive9_companion_test.go b/internal/fs/drive9_companion_test.go index e38ce11..13d0f77 100644 --- a/internal/fs/drive9_companion_test.go +++ b/internal/fs/drive9_companion_test.go @@ -278,6 +278,72 @@ func TestDrive9CopyDoesNotTreatNotFoundAfterTransientFailureAsSuccess(t *testing } } +func TestDrive9CopyUploadWithoutOverwriteRejectsExistingRemoteTarget(t *testing.T) { + companion, recordPath := buildFakeDrive9(t) + t.Setenv("TI_FAKE_DRIVE9_RECORD", recordPath) + // The fake companion's `fs stat` succeeds by default, i.e. the target exists. + + _, err := testCompanionService(t.TempDir(), companion).CopyFile(context.Background(), CopyFileOptions{ + Profile: dataProfile(), + FromLocal: filepath.Join(t.TempDir(), "replacement.txt"), + ToRemote: "/workspace/existing.txt", + }) + if apperr.CodeFor(err) != "fs.target_exists" { + t.Fatalf("copy error = %v, want fs.target_exists", err) + } + for _, call := range readFakeDrive9Calls(t, recordPath) { + if hasArgPrefix(call.Args, []string{"fs", "cp"}) { + t.Fatalf("blocked copy still invoked companion: %#v", call.Args) + } + } +} + +func TestDrive9CopyUploadGuardBypassedForOverwriteAppendAndResume(t *testing.T) { + for _, tc := range []struct { + name string + mutate func(*CopyFileOptions) + }{ + {name: "overwrite", mutate: func(o *CopyFileOptions) { o.Overwrite = true }}, + {name: "append", mutate: func(o *CopyFileOptions) { o.Append = true }}, + {name: "resume", mutate: func(o *CopyFileOptions) { o.Resume = true }}, + } { + t.Run(tc.name, func(t *testing.T) { + companion, recordPath := buildFakeDrive9(t) + t.Setenv("TI_FAKE_DRIVE9_RECORD", recordPath) + opts := CopyFileOptions{ + Profile: dataProfile(), + FromLocal: filepath.Join(t.TempDir(), "input.txt"), + ToRemote: "/workspace/existing.txt", + } + tc.mutate(&opts) + if _, err := testCompanionService(t.TempDir(), companion).CopyFile(context.Background(), opts); err != nil { + t.Fatalf("copy failed: %v", err) + } + requireFakeDrive9Call(t, recordPath, "fs", "cp") + }) + } +} + +func TestDrive9CopyUploadGuardFailsClosedOnStatError(t *testing.T) { + companion, _ := buildFakeDrive9(t) + t.Setenv("TI_FAKE_DRIVE9_STAT_ALWAYS_FAIL", "1") + + _, err := testCompanionService(t.TempDir(), companion).CopyFile(context.Background(), CopyFileOptions{ + Profile: dataProfile(), + FromLocal: filepath.Join(t.TempDir(), "input.txt"), + ToRemote: "/workspace/existing.txt", + }) + if err == nil { + t.Fatal("copy should fail when the target probe cannot determine existence") + } + if isDrive9NotFound(err) { + t.Fatalf("copy error = %v, want non-not-found probe failure", err) + } + if message := apperr.MessageFor(err); !strings.Contains(message, "backend unavailable") { + t.Fatalf("probe failure = %q, want the stat error to propagate", message) + } +} + func TestDrive9CopyDoesNotRetryNonReplayableStreamsOrAppend(t *testing.T) { for _, tc := range []struct { name string @@ -291,6 +357,7 @@ func TestDrive9CopyDoesNotRetryNonReplayableStreamsOrAppend(t *testing.T) { companion, recordPath := buildFakeDrive9(t) t.Setenv("TI_FAKE_DRIVE9_RECORD", recordPath) t.Setenv("TI_FAKE_DRIVE9_CP_FAILURE_SEQUENCE", filepath.Join(t.TempDir(), "copy-attempted")) + t.Setenv("TI_FAKE_DRIVE9_STAT_NOT_FOUND", "1") tc.opts.Profile = dataProfile() if _, err := testCompanionService(t.TempDir(), companion).CopyFile(context.Background(), tc.opts); err == nil { @@ -962,6 +1029,10 @@ func main() { fmt.Fprintln(os.Stderr, "fs stat: storage backend unavailable; resource is still provisioning") os.Exit(1) } + if os.Getenv("TI_FAKE_DRIVE9_STAT_NOT_FOUND") == "1" { + fmt.Fprintln(os.Stderr, "fs stat: remote file not found") + os.Exit(1) + } if sequencePath := os.Getenv("TI_FAKE_DRIVE9_STAT_FAILURE_SEQUENCE"); sequencePath != "" { if _, err := os.Stat(sequencePath); os.IsNotExist(err) { _ = os.WriteFile(sequencePath, []byte("attempted"), 0600)