From 6928fd571aa74f3299bc9430fc9d6f057dd407fd Mon Sep 17 00:00:00 2001 From: Tomas Dvorak Date: Fri, 18 Sep 2026 21:32:16 +0200 Subject: [PATCH] feat(fm): source-identity preconditions on move/copy/rename (#3565) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move/copy/rename resolve sources by path; a delayed retried request can act on a different file that reused the path. The services now accept an optional expected-source hashid (expect_id / expect_ids, positional, empty entry skips) which is compared against the resolved file's database ID before mutation — mismatch fails with fs.ErrModified (409). The web app sends the selected files' IDs on every rename and move/copy. WebDAV and API callers omitting the field are unaffected. --- frontend/src/api/explorer.ts | 2 ++ frontend/src/redux/thunks/file.ts | 5 ++- pkg/filemanager/fs/dbfs/dbfs.go | 21 ++++++++++++ pkg/filemanager/fs/dbfs/manage.go | 10 +++++- pkg/filemanager/fs/dbfs/precondition_test.go | 28 ++++++++++++++++ service/explorer/file.go | 35 ++++++++++++++++++++ 6 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 pkg/filemanager/fs/dbfs/precondition_test.go diff --git a/frontend/src/api/explorer.ts b/frontend/src/api/explorer.ts index 5bf62ebc..ca6b84b9 100644 --- a/frontend/src/api/explorer.ts +++ b/frontend/src/api/explorer.ts @@ -264,6 +264,7 @@ export interface UnlockFileService { export interface RenameFileService { uri: string; new_name: string; + expect_id?: string; } export const NavigatorCapability = { @@ -292,6 +293,7 @@ export interface PinFileService { export interface MoveFileService extends MultipleUriService { dst: string; copy?: boolean; + expect_ids?: string[]; } export interface MetadataPatch { diff --git a/frontend/src/redux/thunks/file.ts b/frontend/src/redux/thunks/file.ts index e7865b40..b690a830 100644 --- a/frontend/src/redux/thunks/file.ts +++ b/frontend/src/redux/thunks/file.ts @@ -380,6 +380,7 @@ export function submitRenameFile(index: number, file: FileResponse, newName: str sendRenameFile({ uri: file.path, new_name: newName, + expect_id: file.id, }), ); } catch (e) { @@ -668,7 +669,9 @@ export function moveFiles(index: number, src: FileResponse[], dst: string, isCop let success = true; try { await longRunningTaskWithSnackbar( - dispatch(sendMoveFile({ uris: src.map((f) => f.path), dst, copy: isCopy })), + dispatch( + sendMoveFile({ uris: src.map((f) => f.path), dst, copy: isCopy, expect_ids: src.map((f) => f.id) }), + ), isCopy ? "application:modals.processingCopying" : "application:modals.processingMoving", ); } catch (e) { diff --git a/pkg/filemanager/fs/dbfs/dbfs.go b/pkg/filemanager/fs/dbfs/dbfs.go index ef2d4581..4cff6d5d 100644 --- a/pkg/filemanager/fs/dbfs/dbfs.go +++ b/pkg/filemanager/fs/dbfs/dbfs.go @@ -43,8 +43,29 @@ type ( // IsDownloadCtxKey marks the request as an explicit file download (as // opposed to an inline preview fetch). Navigator hooks consult it. IsDownloadCtxKey struct{} + // ExpectedSourceIDsCtxKey carries source-file identity preconditions + // for move/copy/rename: a []int positionally aligned with the source + // URI list. An entry of 0 disables the check for that position (#3565). + ExpectedSourceIDsCtxKey struct{} ) +// WithExpectedSourceIDs records the expected database IDs of the source +// files so a delayed/retried request cannot silently operate on a new +// file that reused the same path. +func WithExpectedSourceIDs(ctx context.Context, ids []int) context.Context { + return context.WithValue(ctx, ExpectedSourceIDsCtxKey{}, ids) +} + +// sourceIDMismatch reports whether the resolved file violates the +// expected-source precondition at the given position. +func sourceIDMismatch(ctx context.Context, pos int, actual int) bool { + expected, ok := ctx.Value(ExpectedSourceIDsCtxKey{}).([]int) + if !ok || pos >= len(expected) || expected[pos] == 0 { + return false + } + return expected[pos] != actual +} + // writePermitted reports whether the user may mutate file under the given // capability. File owners are always permitted; non-owners (e.g. share // visitors) require the capability in the file's resolved capability set, diff --git a/pkg/filemanager/fs/dbfs/manage.go b/pkg/filemanager/fs/dbfs/manage.go index 43940787..f47a4900 100644 --- a/pkg/filemanager/fs/dbfs/manage.go +++ b/pkg/filemanager/fs/dbfs/manage.go @@ -160,6 +160,9 @@ func (f *DBFS) Rename(ctx context.Context, path *fs.URI, newName string) (fs.Fil if err != nil { return nil, nil, fmt.Errorf("failed to get target file: %w", err) } + if sourceIDMismatch(ctx, 0, target.Model.ID) { + return nil, nil, fs.ErrModified.WithError(fmt.Errorf("source file no longer matches the expected file")) + } oldName := target.Name() if _, ok := ctx.Value(ByPassOwnerCheckCtxKey{}).(bool); !ok && !f.writePermitted(target, NavigatorCapabilityRenameFile) { @@ -561,7 +564,7 @@ func (f *DBFS) MoveOrCopy(ctx context.Context, path []*fs.URI, dst *fs.URI, isCo ctx = context.WithValue(ctx, inventory.LoadFileEntity{}, true) ctx = context.WithValue(ctx, inventory.LoadFileMetadata{}, true) - for _, p := range path { + for i, p := range path { // Get navigator navigator, err := f.getNavigator(ctx, p, NavigatorCapabilityLockFile) if err != nil { @@ -582,6 +585,11 @@ func (f *DBFS) MoveOrCopy(ctx context.Context, path []*fs.URI, dst *fs.URI, isCo continue } + if sourceIDMismatch(ctx, i, target.Model.ID) { + ae.Add(p.String(), fs.ErrModified.WithError(fmt.Errorf("source file no longer matches the expected file"))) + continue + } + // Copy reads the source, move deletes it. requiredSrcCap := NavigatorCapabilityDownloadFile if !isCopy { diff --git a/pkg/filemanager/fs/dbfs/precondition_test.go b/pkg/filemanager/fs/dbfs/precondition_test.go new file mode 100644 index 00000000..115c97b4 --- /dev/null +++ b/pkg/filemanager/fs/dbfs/precondition_test.go @@ -0,0 +1,28 @@ +package dbfs + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSourceIDMismatch(t *testing.T) { + ctx := context.Background() + + // No expectations -> never mismatches. + require.False(t, sourceIDMismatch(ctx, 0, 42)) + require.False(t, sourceIDMismatch(ctx, 5, 42)) + + ctx = WithExpectedSourceIDs(ctx, []int{11, 0, 33}) + + // Matching positions pass. + require.False(t, sourceIDMismatch(ctx, 0, 11)) + // Zero entries disable the check for that position. + require.False(t, sourceIDMismatch(ctx, 1, 999)) + // Positions beyond the expectation list pass. + require.False(t, sourceIDMismatch(ctx, 3, 999)) + // Mismatches are caught per position. + require.True(t, sourceIDMismatch(ctx, 0, 12)) + require.True(t, sourceIDMismatch(ctx, 2, 34)) +} diff --git a/service/explorer/file.go b/service/explorer/file.go index 0552664f..9d85a8c2 100644 --- a/service/explorer/file.go +++ b/service/explorer/file.go @@ -246,6 +246,10 @@ type ( RenameFileService struct { Uri string `json:"uri" binding:"required"` NewName string `json:"new_name" binding:"required,min=1,max=255"` + // ExpectID optionally carries the hashid of the file the caller + // believes sits at Uri; the rename fails with a conflict if the + // resolved file differs (retried request after path reuse, #3565). + ExpectID string `json:"expect_id"` } ) @@ -260,6 +264,14 @@ func (service *RenameFileService) Rename(c *gin.Context) (*FileResponse, error) return nil, serializer.NewError(serializer.CodeParamErr, "unknown uri", err) } + if service.ExpectID != "" { + expectID, err := dep.HashIDEncoder().Decode(service.ExpectID, hashid.FileID) + if err != nil { + return nil, serializer.NewError(serializer.CodeParamErr, "unknown expect_id", err) + } + util.WithValue(c, dbfs.ExpectedSourceIDsCtxKey{}, []int{expectID}) + } + file, err := m.Rename(c, uri, service.NewName) if err != nil { return nil, err @@ -274,6 +286,11 @@ type ( Uris []string `json:"uris" binding:"required,min=1"` Dst string `json:"dst" binding:"required"` Copy bool `json:"copy"` + // ExpectIDs optionally carries the hashids of the source files the + // caller selected, positionally aligned with Uris; a mismatch fails + // that entry with a conflict (#3565). An empty string entry skips + // the check for that position. + ExpectIDs []string `json:"expect_ids"` } ) @@ -297,6 +314,24 @@ func (s *MoveFileService) Move(c *gin.Context) error { return serializer.NewError(serializer.CodeParamErr, "unknown destination uri", err) } + if len(s.ExpectIDs) > 0 { + if len(s.ExpectIDs) != len(s.Uris) { + return serializer.NewError(serializer.CodeParamErr, "expect_ids must align with uris", nil) + } + ids := make([]int, len(s.ExpectIDs)) + for i, e := range s.ExpectIDs { + if e == "" { + continue + } + id, err := dep.HashIDEncoder().Decode(e, hashid.FileID) + if err != nil { + return serializer.NewError(serializer.CodeParamErr, "unknown expect_ids entry", err) + } + ids[i] = id + } + util.WithValue(c, dbfs.ExpectedSourceIDsCtxKey{}, ids) + } + return m.MoveOrCopy(c, uris, dst, s.Copy) }