FIX: Allow topic edits when using a hidden tag

Previously, a regular user could not edit the title or category
of a topic if a hidden tag had already been applied.

This also stops hidden tag names from leaking in the error message.

app/controllers/topics_controller.rb

@@ -320,12 +320,26 @@ class TopicsController < ApplicationController
           ).pluck("tags.name")
 
           invalid_tags = topic_tags - allowed_tags
+
+          # Do not raise an error on a topic's hidden tags when not modifying tags
+          if params[:tags].blank?
+            invalid_tags.each do |tag_name|
+              if DiscourseTagging.hidden_tag_names.include?(tag_name)
+                invalid_tags.delete(tag_name)
+              end
+            end
+          end
+
           if !invalid_tags.empty?
+            if (invalid_tags & DiscourseTagging.hidden_tag_names).present?
+              return render_json_error(I18n.t('category.errors.disallowed_tags_generic'))
+            else
               return render_json_error(I18n.t('category.errors.disallowed_topic_tags', tags: invalid_tags.join(", ")))
             end
           end
         end
       end
+    end
 
     changes = {}
 

config/locales/server.en.yml

@@ -609,6 +609,7 @@ en:
       description_incomplete: "The category description post must have at least one paragraph."
       permission_conflict: "Any group that is allowed to access a subcategory must also be allowed to access the parent category. The following groups have access to one of the subcategories, but no access to parent category: %{group_names}."
       disallowed_topic_tags: "This topic has tags not allowed by this category: '%{tags}'"
+      disallowed_tags_generic: "This topic has disallowed tags."
     cannot_delete:
       uncategorized: "This category is special. It is intended as a holding area for topics that have no category; it cannot be deleted."
       has_subcategories: "Can't delete this category because it has sub-categories."

spec/requests/topics_controller_spec.rb

@@ -1138,16 +1138,55 @@ RSpec.describe TopicsController do
             restricted_category.allowed_tags = [tag2.name]
 
             put "/t/#{topic.slug}/#{topic.id}.json", params: {
-              tags: [tag2],
+              tags: [tag2.name],
               category_id: category.id
             }
 
             result = ::JSON.parse(response.body)
             expect(response.status).to eq(422)
             expect(result['errors']).to be_present
+            expect(result['errors'][0]).to include(tag2.name)
             expect(topic.reload.category_id).not_to eq(restricted_category.id)
           end
 
+          it 'allows category change when topic has a hidden tag' do
+            Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [tag1.name])
+
+            put "/t/#{topic.slug}/#{topic.id}.json", params: {
+              category_id: category.id
+            }
+
+            result = ::JSON.parse(response.body)
+            expect(response.status).to eq(200)
+            expect(topic.reload.tags).to include(tag1)
+          end
+
+          it 'allows category change when topic has a read-only tag' do
+            Fabricate(:tag_group, permissions: { "staff" => 1, "everyone" => 3 }, tag_names: [tag1.name])
+
+            put "/t/#{topic.slug}/#{topic.id}.json", params: {
+              category_id: category.id
+            }
+
+            result = ::JSON.parse(response.body)
+            expect(response.status).to eq(200)
+            expect(topic.reload.tags).to include(tag1)
+          end
+
+          it 'does not leak tag name when trying to use a staff tag' do
+            Fabricate(:tag_group, permissions: { "staff" => 1 }, tag_names: [tag2.name])
+
+            put "/t/#{topic.slug}/#{topic.id}.json", params: {
+              tags: [tag2.name],
+              category_id: category.id
+            }
+
+            result = ::JSON.parse(response.body)
+            expect(response.status).to eq(422)
+            expect(result['errors']).to be_present
+            expect(result['errors'][0]).not_to include(tag2.name)
+          end
+
           it 'will clean tag params' do
             restricted_category.allowed_tags = [tag2.name]