Prevent a staged parent move from crashing on its own descendant

publish_draft! called move_to_child_of before validating the new
parent at all. Staging a node under one of its own descendants
created a real, if transient, cycle in parent_id the instant that
save landed -- and update_unique_names_of_children's after_save
callback, which recurses down through parent_id with no cycle check
and no depth limit, bounced between the two nodes forever, crashing
the whole process with a SystemStackError rather than just producing
bad data.

Now rejected up front with a normal ActiveRecord::RecordInvalid.

2 files changed, 17 insertions, 0 deletions

diff --git a/app/models/node.rb b/app/models/node.rb
index 82d9954..9cb4840 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -227,6 +227,11 @@ class Node < ApplicationRecord
 
     if staged_parent_id && (staged_parent_id != parent_id)
       new_parent = Node.find(staged_parent_id)
+
+      if new_parent == self || self.descendants.include?(new_parent)
+        raise ActiveRecord::RecordInvalid.new(self), "Cannot move a node under itself or one of its own descendants"
+      end
+
       self.staged_parent_id = nil
       self.save!
       self.move_to_child_of(new_parent)
diff --git a/test/models/node_test.rb b/test/models/node_test.rb
index 280614e..5626384 100644
--- a/test/models/node_test.rb
+++ b/test/models/node_test.rb
@@ -499,4 +499,16 @@ class NodeTest < ActiveSupport::TestCase
     node.autosave!({title: "v3"}, user)
     assert_equal [[:head, :draft], [:draft, :autosave]], node.available_layer_pairs # state D
   end
+
+  test "publishing a staged move under one's own descendant is rejected, not allowed to crash" do
+    a = Node.root.children.create!(:slug => "cycle_guard_a")
+    b = a.children.create!(:slug => "cycle_guard_b")
+
+    a.staged_parent_id = b.id
+
+    assert_raises(ActiveRecord::RecordInvalid) { a.publish_draft! }
+
+    a.reload
+    assert_equal Node.root.id, a.parent_id
+  end
 end