From 464af1625349d557f688da9f845471ef8b80a5f9 Mon Sep 17 00:00:00 2001
From: erdgeist <erdgeist@erdgeist.org>
Date: Fri, 31 Jul 2026 18:14:30 +0200
Subject: Gate live-content changes on restricted surfaces

publish_draft!, trash!, destroy_from_trash!, attach_asset! and
Asset#destroy_witnessed! now refuse unless the acting user holds redaktion,
and only when the subject is on a restricted surface: the front page, the
updates tree that feeds ~100k subscribers, or disclosure. Drafting,
autosaving, tagging and creating stay free everywhere for everyone.

Enforcement is in the models rather than the controllers, since attach_asset!
and the rest are reachable from rake tasks and internal paths. It follows the
errors.add-plus-bare-raise pattern the rest of Node already uses, so every
existing RecordInvalid rescue reports it with a localised message; only
assets_controller#destroy needed a rescue added.

A nil user is treated as a system context and bypasses the gate. The default
nil on three of those verbs is what makes that reachable, and removing those
defaults once every call site passes a user is the next tightening.
---
 app/controllers/assets_controller.rb      |  6 +++
 app/controllers/nodes_controller.rb       |  3 ++
 app/models/asset.rb                       | 12 ++++++
 app/models/node.rb                        | 28 +++++++++++++
 app/models/user.rb                        |  5 +++
 config/locales/de.yml                     |  5 +++
 config/locales/en.yml                     |  1 +
 lib/ccc_conventions.rb                    |  1 +
 test/controllers/nodes_controller_test.rb | 14 +++++++
 test/models/asset_destroy_test.rb         | 13 ++++++
 test/models/node_test.rb                  | 68 +++++++++++++++++++++++++++++++
 test/models/user_test.rb                  | 16 ++++++++
 12 files changed, 172 insertions(+)

diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb
index 988d56d1..03780c69 100644
--- a/app/controllers/assets_controller.rb
+++ b/app/controllers/assets_controller.rb
@@ -93,6 +93,12 @@ class AssetsController < ApplicationController
       format.html { redirect_to(assets_url) }
       format.xml  { head :ok }
     end
+  rescue ActiveRecord::RecordInvalid => e
+    flash[:error] = e.message
+    respond_to do |format|
+      format.html { redirect_to(asset_path(@asset)) }
+      format.xml  { head :forbidden }
+    end
   end
 
   private
diff --git a/app/controllers/nodes_controller.rb b/app/controllers/nodes_controller.rb
index c56fd945..383fb72c 100644
--- a/app/controllers/nodes_controller.rb
+++ b/app/controllers/nodes_controller.rb
@@ -194,6 +194,9 @@ class NodesController < ApplicationController
     @node.publish_draft!(current_user)
     flash[:notice] = t("flash.nodes.published")
     redirect_to node_path(@node)
+  rescue ActiveRecord::RecordInvalid => e
+    flash[:error] = e.message
+    redirect_to node_path(@node)
   end
 
   def unlock
diff --git a/app/models/asset.rb b/app/models/asset.rb
index 8cec4371..b256b929 100644
--- a/app/models/asset.rb
+++ b/app/models/asset.rb
@@ -41,6 +41,13 @@ class Asset < ApplicationRecord
                :ids => page_ids).distinct
   end
 
+  # An asset's reach is the reach of the pages carrying it: destroying one
+  # removes it from every live page at once, so a single restricted
+  # attachment makes the destruction a restricted act.
+  def restricted?
+    attached_nodes.any?(&:restricted?)
+  end
+
   # Witnessed destruction. Destroying an asset is a public-facing act
   # even when unattached. The original and its variants are publicly
   # reachable under /system/uploads, so an entry is always written,
@@ -49,6 +56,11 @@ class Asset < ApplicationRecord
   # participates as the first non-Node subject (its participant row
   # dangles after destroy, by design, the name lives on in metadata).
   def destroy_witnessed! user:
+    if user && !user.may_change_live?(self)
+      errors.add(:base, :not_permitted)
+      raise ActiveRecord::RecordInvalid.new(self)
+    end
+
     ActiveRecord::Base.transaction do
       affected = attached_nodes.to_a
       headline_losses = affected.select do |node|
diff --git a/app/models/node.rb b/app/models/node.rb
index 1823daa4..ac3a6160 100644
--- a/app/models/node.rb
+++ b/app/models/node.rb
@@ -236,6 +236,8 @@ class Node < ApplicationRecord
     # Return nil if nothing to publish and no staged changes
     return nil unless self.draft || staged_slug || staged_parent_id
 
+    guard_live_change!(current_user)
+
     if in_trash? || trash_node?
       errors.add(:base, :publish_in_trash)
       raise ActiveRecord::RecordInvalid.new(self)
@@ -319,6 +321,9 @@ class Node < ApplicationRecord
   # at the root, carrying the leaving-public-view snapshot.
   def trash! current_user = nil
     return nil if in_trash?
+
+    guard_live_change!(current_user)
+
     if trash_node?
       errors.add(:base, :trash_the_trash)
       raise ActiveRecord::RecordInvalid.new(self)
@@ -391,6 +396,8 @@ class Node < ApplicationRecord
   # One log entry at the root, per the subtree rule, written before the
   # rows die.
   def destroy_from_trash! current_user = nil
+    guard_live_change!(current_user)
+
     unless in_trash?
       errors.add(:base, :destroy_outside_trash)
       raise ActiveRecord::RecordInvalid.new(self)
@@ -485,6 +492,8 @@ class Node < ApplicationRecord
   # Returns { :attached => n, :already => n,
   #           :headline => nil | :set | :kept_existing | :not_eligible }
   def attach_asset! asset, user:, headline: false
+    guard_live_change!(user)
+
     if in_trash? || trash_node?
       errors.add(:base, :attach_in_trash)
       raise ActiveRecord::RecordInvalid.new(self)
@@ -565,6 +574,17 @@ class Node < ApplicationRecord
     false
   end
 
+  def restricted?
+    return true if root?
+
+    name = unique_name.to_s
+    return false if name.empty?
+
+    CccConventions::RESTRICTED_SUBTREES.any? do |prefix|
+      name == prefix || name.start_with?("#{prefix}/")
+    end
+  end
+
   # Returns immutable node id for all new nodes so that the atom feed entry ids
   # stay the same eventhough the slug or positions changes.
   # Can be removed after a year or so ;)
@@ -677,6 +697,14 @@ class Node < ApplicationRecord
 
   private
 
+    def guard_live_change! user
+      return if user.nil?
+      return if user.may_change_live?(self)
+
+      errors.add(:base, :not_permitted)
+      raise ActiveRecord::RecordInvalid.new(self)
+    end
+
     def reserved_slug_stays_reserved
       if parent&.root? && !trash_node_already_me?
         errors.add(:slug, :reserved_for_trash) if slug == CccConventions::TRASH_SLUG
diff --git a/app/models/user.rb b/app/models/user.rb
index 1728521a..e8c3b9bb 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -105,6 +105,11 @@ class User < ApplicationRecord
     roles.map { |r| I18n.t("users.roles.#{r}", :default => r) }
   end
 
+  def may_change_live?(subject)
+    return true unless subject.restricted?
+    redaktion?
+  end
+
   def deactivate!(actor:)
     return false if alumni?
     transaction do
diff --git a/config/locales/de.yml b/config/locales/de.yml
index 8aae7ca5..a7ad0f26 100644
--- a/config/locales/de.yml
+++ b/config/locales/de.yml
@@ -145,6 +145,7 @@ de:
               restore_target_invalid: "Das Wiederherstellungsziel muss ein lebender Node sein"
               destroy_outside_trash: "Nodes können nur aus dem Papierkorb gelöscht werden"
               attach_in_trash: "An einen Node im Papierkorb können keine Assets angehängt werden"
+              not_permitted: "In diesem Bereich dürfen nur Mitglieder der Redaktion veröffentlichte Inhalte ändern"
         user:
           attributes:
             roles:
@@ -153,6 +154,10 @@ de:
           attributes:
             headline:
               images_and_pdfs_only: "kann nur auf Bildern oder PDFs gesetzt werden"
+        asset:
+          attributes:
+            base:
+              not_permitted: "Dieses Asset ist an geschützte Seiten angehängt; nur die Redaktion darf es löschen"
 
   tags:
     index:
diff --git a/config/locales/en.yml b/config/locales/en.yml
index e434538f..7e830325 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -92,6 +92,7 @@ en:
               restore_target_invalid: "Restore target must be a living node"
               destroy_outside_trash: "Nodes are only destroyed from the Trash"
               attach_in_trash: "Cannot attach assets to a node in the Trash"
+              not_permitted: "Only Redaktion members may change published content in this section"
         user:
           attributes:
             roles:
diff --git a/lib/ccc_conventions.rb b/lib/ccc_conventions.rb
index 7ff5a3ab..b840dee2 100644
--- a/lib/ccc_conventions.rb
+++ b/lib/ccc_conventions.rb
@@ -3,6 +3,7 @@ module CccConventions
   ERFA_PARENT_NAME        = "club/erfas"
   CHAOSTREFF_PARENT_NAME  = "club/chaostreffs"
   SITEMAP_COLLAPSED_PATHS = %w[updates club/erfas club/chaostreffs disclosure].freeze
+  RESTRICTED_SUBTREES     = %w[updates disclosure].freeze
 
   NODE_KINDS = {
     "top_level" => {
diff --git a/test/controllers/nodes_controller_test.rb b/test/controllers/nodes_controller_test.rb
index 0b6e65ee..f15be067 100644
--- a/test/controllers/nodes_controller_test.rb
+++ b/test/controllers/nodes_controller_test.rb
@@ -770,6 +770,20 @@ class NodesControllerTest < ActionController::TestCase
     assert_select "form[action=?]", node_path(node), count: 1
   end
 
+  test "publishing a restricted node without the redaktion role flashes and does not publish" do
+    login_as :quentin
+    updates = Node.root.children.create!(:slug => "updates")
+    node    = updates.children.create!(:slug => "controller-gated")
+    node.reload.draft.update!(:title => "Entwurf")
+
+    put :publish, params: { :id => node.id }
+
+    assert_redirected_to node_path(node)
+    assert_match I18n.t("activerecord.errors.models.node.attributes.base.not_permitted"),
+                 flash[:error]
+    assert_nil node.reload.head
+  end
+
 test "show annotates history rows with their lifecycle" do
     login_as :quentin
     node = Node.root.children.create!(:slug => "history_annotation_test")
diff --git a/test/models/asset_destroy_test.rb b/test/models/asset_destroy_test.rb
index 5583f685..2f38d692 100644
--- a/test/models/asset_destroy_test.rb
+++ b/test/models/asset_destroy_test.rb
@@ -48,4 +48,17 @@ class AssetDestroyTest < ActiveSupport::TestCase
     assert_equal "Doomed asset", action.metadata["asset_name"]
     assert_nil action.action_participants.first.subject
   end
+
+  test "destroying an asset attached to a restricted node needs the redaktion role" do
+    editor = User.create!(:login => "asset_gate", :email => "ag@example.com",
+                          :password => "secret", :password_confirmation => "secret")
+    updates = Node.root.children.create!(:slug => "updates")
+    node    = updates.children.create!(:slug => "gated-attachment")
+    node.reload.attach_asset!(@asset, :user => nil)
+
+    error = assert_raises(ActiveRecord::RecordInvalid) { @asset.destroy_witnessed!(:user => editor) }
+    assert_includes error.message,
+                    I18n.t("activerecord.errors.models.asset.attributes.base.not_permitted")
+    assert Asset.exists?(@asset.id)
+  end
 end
diff --git a/test/models/node_test.rb b/test/models/node_test.rb
index c1316ea6..f57f83bf 100644
--- a/test/models/node_test.rb
+++ b/test/models/node_test.rb
@@ -936,4 +936,72 @@ class NodeTest < ActiveSupport::TestCase
                       I18n.t("activerecord.errors.models.node.attributes.base.trash_the_trash")
     end
   end
+
+  test "restricted? covers the root node, the restricted subtrees and their descendants" do
+    assert Node.root.restricted?, "the front page aggregates the feed"
+
+    updates = Node.root.children.create!(:slug => "updates")
+    assert updates.restricted?
+    year = updates.children.create!(:slug => "2026")
+    assert year.reload.restricted?
+    post = year.children.create!(:slug => "some-post")
+    assert post.reload.restricted?
+
+    disclosure = Node.root.children.create!(:slug => "disclosure")
+    assert disclosure.restricted?
+
+    plain = Node.root.children.create!(:slug => "club")
+    assert_not plain.restricted?
+    child = plain.children.create!(:slug => "erfas")
+    assert_not child.reload.restricted?
+  end
+
+  test "restricted? does not match a prefix that is merely a substring" do
+    decoy = Node.root.children.create!(:slug => "updatesomething")
+    assert_not decoy.restricted?
+  end
+
+  test "publishing a restricted node is refused without the redaktion role" do
+    editor = User.create!(:login => "guard_editor", :email => "gd@example.com",
+                          :password => "secret", :password_confirmation => "secret")
+    updates = Node.root.children.create!(:slug => "updates")
+    node    = updates.children.create!(:slug => "guarded-post")
+    node.reload.draft.update!(:title => "Entwurf")
+
+    error = assert_raises(ActiveRecord::RecordInvalid) { node.publish_draft!(editor) }
+    assert_includes error.message,
+                    I18n.t("activerecord.errors.models.node.attributes.base.not_permitted")
+    assert_nil node.reload.head
+  end
+
+  test "publishing a restricted node succeeds with the redaktion role" do
+    red = User.create!(:login => "guard_red", :email => "gr2@example.com",
+                       :password => "secret", :password_confirmation => "secret",
+                       :roles => ["redaktion"])
+    updates = Node.root.children.create!(:slug => "updates")
+    node    = updates.children.create!(:slug => "allowed-post")
+    node.reload.draft.update!(:title => "Entwurf")
+
+    node.publish_draft!(red)
+    assert_not_nil node.reload.head
+  end
+
+  test "publishing outside the restricted subtrees needs no role" do
+    editor = User.create!(:login => "guard_free", :email => "gf@example.com",
+                          :password => "secret", :password_confirmation => "secret")
+    node = Node.root.children.create!(:slug => "guard-free-post")
+    node.reload.draft.update!(:title => "Entwurf")
+
+    node.publish_draft!(editor)
+    assert_not_nil node.reload.head
+  end
+
+  test "a nil user is a system context and bypasses the gate" do
+    updates = Node.root.children.create!(:slug => "updates")
+    node    = updates.children.create!(:slug => "system-post")
+    node.reload.draft.update!(:title => "Entwurf")
+
+    node.publish_draft!
+    assert_not_nil node.reload.head
+  end
 end
diff --git a/test/models/user_test.rb b/test/models/user_test.rb
index feccce25..9942385c 100644
--- a/test/models/user_test.rb
+++ b/test/models/user_test.rb
@@ -126,6 +126,22 @@ class UserTest < ActiveSupport::TestCase
 
     assert user.update(:email => "quentin@example.org")
   end
+
+  test "may_change_live? gates restricted subjects on the redaktion role" do
+    editor    = User.create!(:login => "gate_editor", :email => "ge@example.com",
+                             :password => "secret", :password_confirmation => "secret")
+    redaktion = User.create!(:login => "gate_red", :email => "gr@example.com",
+                             :password => "secret", :password_confirmation => "secret",
+                             :roles => ["redaktion"])
+
+    restricted = Node.root
+    plain = Node.root.children.create!(:slug => "gate_plain")
+
+    assert     editor.may_change_live?(plain)
+    assert_not editor.may_change_live?(restricted)
+    assert     redaktion.may_change_live?(plain)
+    assert     redaktion.may_change_live?(restricted)
+  end
   
 protected
   def create_user(options = {})
-- 
cgit v1.3