From 464af1625349d557f688da9f845471ef8b80a5f9 Mon Sep 17 00:00:00 2001 From: erdgeist 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 +++++ 5 files changed, 54 insertions(+) (limited to 'app') 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 -- cgit v1.3