Skip to content

🐛(backend) prevent moving an item to itself or its descendant#715

Open
maboukerfa wants to merge 1 commit into
suitenumerique:mainfrom
maboukerfa:fix/prevent-move-item-to-descendant
Open

🐛(backend) prevent moving an item to itself or its descendant#715
maboukerfa wants to merge 1 commit into
suitenumerique:mainfrom
maboukerfa:fix/prevent-move-item-to-descendant

Conversation

@maboukerfa

Copy link
Copy Markdown

Problem

Item.move() had no guard against moving a folder into itself or one of its own descendants.

Drive uses django_ltree, which raises no error in this case, it silently runs the RawSQL path rewrite. Because the path__descendants ltree lookup (<@) is inclusive, the moved item matches its own descendant query, so both it and its children get rewritten and the subtree ends up orphaned.

Walkthrough

Screen.Recording.2026-05-22.at.12.04.08.mov

Take a parent P with path P and a child C with path P.C. Moving P into C:

old_path = self.path                      # "P"
self.path = f"{target.path}.{self.id}"     # "P.C.P"
self.save(update_fields=["path"])          # P is now "P.C.P"

# P is a FOLDER, so descendants get rewritten:
Item.objects.filter(path__descendants=old_path).update(   # descendants of "P"
    path=RawSQL("%s || subpath(path, nlevel(%s))", (self.path, old_path))
)

After self.save(), the rows matching "descendant of P" are:

  • C with path P.CP.C <@ P
  • P itself with its new path P.C.PP.C.P <@ P ✅ — P matches its own descendant query

So the bulk UPDATE rewrites both, with self.path = "P.C.P" and nlevel("P") = 1:

Item New path
C "P.C.P"subpath("P.C", 1) = P.C.P.C
P "P.C.P"subpath("P.C.P", 1) = P.C.P.C.P

Result: no item has path P anymore, so neither P nor C is reachable from the real tree. P ends up with the self-referential path P.C.P.C.P — a cycle baked into the materialized paths, leaving the subtree orphaned and unrecoverable through the UI.

Fix

Reject the move in Item.move() when the target is the item itself or one of its descendants, mirroring the style of the existing not-a-folder check:

if target:
    target_path = str(target.path)
    self_path = str(self.path)
    if target_path == self_path or target_path.startswith(f"{self_path}."):
        raise ValidationError(...)  # code="item_move_target_is_descendant"

Moving a folder into itself or one of its own descendants had no
guard. With django_ltree this silently ran the path rewrite, and
because the path__descendants lookup is inclusive the moved item
matched its own descendant query, corrupting paths into a
self-referential cycle and orphaning the whole subtree.

Signed-off-by: Mohamed El Amine BOUKERFA <[email protected]>
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant