feat(fm): source-identity preconditions on move/copy/rename (#3565)

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.
pull/3582/head
Tomas Dvorak 2 weeks ago
parent 5811af4503
commit 6928fd571a

@ -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 {

@ -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) {

@ -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,

@ -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 {

@ -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))
}

@ -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)
}

Loading…
Cancel
Save