[pull] main from forem:main

app/controllers/articles_controller.rb

@@ -353,13 +353,10 @@ def article_params_json
                        ]
                      end
 
-    # Allow video_source_url if it's a valid YouTube, Mux, or Twitch URL
-    video_url = params.dig("article", "video_source_url")
-    if video_url.present?
-      youtube_pattern = /\Ahttps?:\/\/(www\.)?(youtube\.com\/watch\?v=|youtu\.be\/)/
-      mux_pattern = /\Ahttps?:\/\/player\.mux\.com\//
-      twitch_pattern = /\Ahttps?:\/\/(www\.)?twitch\.tv\/videos\//
-      allowed_params << :video_source_url if video_url.match?(youtube_pattern) || video_url.match?(mux_pattern) || video_url.match?(twitch_pattern)
+    # Allow video_source_url only for supported hosts; a blank value removes the cover video
+    if params["article"].key?("video_source_url") &&
+        Article.permitted_video_source_url?(params["article"]["video_source_url"])
+      allowed_params << :video_source_url
     end
 
     # NOTE: the organization logic is still a little counter intuitive but this should

app/controllers/concerns/api/articles_controller.rb

@@ -188,13 +188,10 @@ def article_params
       ]
       allowed_params << :ai_disclosure_level if Settings::General.enable_ai_disclosure
       allowed_params << :organization_id if params.dig("article", "organization_id") && allowed_to_change_org_id?
-      # allow if a youtube.com, mux.com, or twitch.tv URL
-      video_url = params.dig("article", "video_source_url")
-      if video_url.present?
-        youtube_pattern = /\Ahttps?:\/\/(www\.)?(youtube\.com\/watch\?v=|youtu\.be\/)/
-        mux_pattern = /\Ahttps?:\/\/player\.mux\.com\//
-        twitch_pattern = /\Ahttps?:\/\/(www\.)?twitch\.tv\/videos\//
-        allowed_params << :video_source_url if video_url.match?(youtube_pattern) || video_url.match?(mux_pattern) || video_url.match?(twitch_pattern)
+      # allow video_source_url only for supported hosts; a blank value removes the cover video
+      if params["article"]&.key?("video_source_url") &&
+          Article.permitted_video_source_url?(params["article"]["video_source_url"])
+        allowed_params << :video_source_url
       end
       if @user.super_admin?
         allowed_params << :clickbait_score

app/models/article.rb

@@ -60,6 +60,15 @@ class Article < ApplicationRecord
 
   MAX_TAG_LIST_SIZE = 4
 
+  # Hosts a user may link as a cover video via `video_source_url`. Used by the
+  # web and API controllers' param whitelists; mirrored on the frontend in
+  # app/javascript/article-form/components/CoverVideoLink.jsx.
+  LINKED_VIDEO_SOURCE_PATTERNS = [
+    %r{\Ahttps?://(www\.)?(youtube\.com/watch\?v=|youtu\.be/)},
+    %r{\Ahttps?://player\.mux\.com/},
+    %r{\Ahttps?://(www\.)?twitch\.tv/videos/},
+  ].freeze
+
   # Author-visible edits, for the article_updated CDP event. Rows churn on score
   # recalcs, counter caches and last_comment_at. Mirrors User::SYNC_TRIGGER_KEYS.
   TRACKABLE_UPDATE_KEYS = %w[
@@ -133,6 +142,14 @@ def self.unique_url_error
     I18n.t("models.article.unique_url", email: ForemInstance.contact_email)
   end
 
+  # Whether a user-submitted `video_source_url` may be mass-assigned.
+  # A blank value is allowed so the cover video can be removed.
+  def self.permitted_video_source_url?(url)
+    return true if url.blank?
+
+    LINKED_VIDEO_SOURCE_PATTERNS.any? { |pattern| url.to_s.match?(pattern) }
+  end
+
   enum :type_of, {
     full_post: 0,
     status: 1,
@@ -1209,7 +1226,17 @@ def set_default_subforem_id
   end
 
   def generate_video_embed_url
-    return if video_source_url.blank?
+    if video_source_url.blank?
+      # The cover video was removed: drop the embed derived from it. Only Mux
+      # derives the thumbnail from the source URL, so user-provided thumbnails
+      # are kept.
+      if video_source_url_was.present?
+        self.video = nil
+        self.video_thumbnail_url = nil if video_thumbnail_url&.include?("image.mux.com")
+      end
+      self.video_source_url = nil
+      return
+    end
 
     if video_source_url.include?("youtube.com") || video_source_url.include?("youtu.be")
       begin

spec/models/article_spec.rb

@@ -1664,6 +1664,88 @@ def build_and_validate_article(*args)
         expect(article.video).to be_nil
       end
     end
+
+    context "when clearing video_source_url" do
+      it "clears video and video_source_url when set to blank" do
+        saved_article = create(:article, user: user, video_source_url: "https://www.youtube.com/watch?v=dQw4w9WgXcQ")
+        expect(saved_article.video).to eq("https://www.youtube.com/embed/dQw4w9WgXcQ")
+
+        saved_article.video_source_url = ""
+        saved_article.valid?
+        expect(saved_article.video_source_url).to be_nil
+        expect(saved_article.video).to be_nil
+      end
+
+      it "clears video_thumbnail_url for Mux video when cleared" do
+        saved_article = create(:article, user: user, video_source_url: "https://player.mux.com/nw5QrgIQS02FEx5BJEQH8CdcLmXXRvCNACZKQ01kLoKEI")
+        expect(saved_article.video_thumbnail_url).to be_present
+
+        saved_article.video_source_url = nil
+        saved_article.valid?
+        expect(saved_article.video_source_url).to be_nil
+        expect(saved_article.video).to be_nil
+        expect(saved_article.video_thumbnail_url).to be_nil
+      end
+
+      it "does not clear an existing video embed on unrelated updates when video_source_url is untouched" do
+        saved_article = create(:article, user: user, video_source_url: nil)
+        saved_article.update_column(:video, "https://www.youtube.com/embed/dQw4w9WgXcQ")
+
+        saved_article.title = "Updated title"
+        saved_article.valid?
+        expect(saved_article.video).to eq("https://www.youtube.com/embed/dQw4w9WgXcQ")
+      end
+
+      it "does not clear a legacy uploaded video on unrelated updates" do
+        saved_article = create(:article, :video, user: user)
+        expect(saved_article.video).to be_present
+        expect(saved_article.video_source_url).to include(".m3u8")
+
+        saved_article.title = "Updated title"
+        saved_article.valid?
+        expect(saved_article.video).to be_present
+        expect(saved_article.video_thumbnail_url).to be_present
+      end
+
+      it "keeps a user-provided thumbnail when a non-Mux video is cleared" do
+        saved_article = create(:article, user: user,
+                                         video_source_url: "https://www.youtube.com/watch?v=dQw4w9WgXcQ",
+                                         video_thumbnail_url: "https://i.imgur.com/HPiu7N4.jpg")
+
+        saved_article.video_source_url = ""
+        saved_article.valid?
+        expect(saved_article.video).to be_nil
+        expect(saved_article.video_thumbnail_url).to eq("https://i.imgur.com/HPiu7N4.jpg")
+      end
+
+      it "does not clear video on a new record with a blank video_source_url" do
+        new_article = build(:article, user: user, video: "https://www.youtube.com/embed/dQw4w9WgXcQ",
+                                      video_source_url: "")
+        new_article.valid?
+        expect(new_article.video).to eq("https://www.youtube.com/embed/dQw4w9WgXcQ")
+        expect(new_article.video_source_url).to be_nil
+      end
+    end
+  end
+
+  describe ".permitted_video_source_url?" do
+    it "permits blank values so the cover video can be removed" do
+      expect(described_class.permitted_video_source_url?(nil)).to be(true)
+      expect(described_class.permitted_video_source_url?("")).to be(true)
+    end
+
+    it "permits YouTube, Mux and Twitch video URLs" do
+      expect(described_class.permitted_video_source_url?("https://www.youtube.com/watch?v=dQw4w9WgXcQ")).to be(true)
+      expect(described_class.permitted_video_source_url?("https://youtu.be/dQw4w9WgXcQ")).to be(true)
+      expect(described_class.permitted_video_source_url?("https://player.mux.com/abc123")).to be(true)
+      expect(described_class.permitted_video_source_url?("https://www.twitch.tv/videos/1234567890")).to be(true)
+    end
+
+    it "rejects other URLs" do
+      expect(described_class.permitted_video_source_url?("https://example.com/video")).to be(false)
+      expect(described_class.permitted_video_source_url?("https://www.twitch.tv/somechannel")).to be(false)
+      expect(described_class.permitted_video_source_url?("javascript:alert(1)")).to be(false)
+    end
   end
 
   describe "#fetch_video_duration" do

spec/requests/api/v1/articles_spec.rb

@@ -1209,6 +1209,38 @@ def post_article(**params)
         expect(article.reload.main_image).to eq("https://dummyimage.com/100x100")
       end
 
+      it "removes video_source_url when given an empty string" do
+        article.update!(video_source_url: "https://www.youtube.com/watch?v=dQw4w9WgXcQ")
+        expect(article.reload.video).to be_present
+
+        put_article(video_source_url: "")
+        expect(response).to have_http_status(:ok)
+        article.reload
+        expect(article.video_source_url).to be_nil
+        expect(article.video).to be_nil
+      end
+
+      it "removes video_source_url when given nil" do
+        article.update!(video_source_url: "https://www.youtube.com/watch?v=dQw4w9WgXcQ")
+        expect(article.reload.video).to be_present
+
+        put_article(video_source_url: nil)
+        expect(response).to have_http_status(:ok)
+        article.reload
+        expect(article.video_source_url).to be_nil
+        expect(article.video).to be_nil
+      end
+
+      it "does not touch video_source_url when the key is absent" do
+        article.update!(video_source_url: "https://www.youtube.com/watch?v=dQw4w9WgXcQ")
+
+        put_article(title: "New title")
+        expect(response).to have_http_status(:ok)
+        article.reload
+        expect(article.video_source_url).to eq("https://www.youtube.com/watch?v=dQw4w9WgXcQ")
+        expect(article.video).to be_present
+      end
+
       it "updates the tags" do
         expect do
           put_article(

spec/requests/articles/articles_update_spec.rb

@@ -194,6 +194,57 @@
     expect(article.reload.video_thumbnail_url).to include "https://i.imgur.com/HPiu7N4.jpg"
   end
 
+  it "removes video_source_url and video embed when passed empty string" do
+    article.update!(video_source_url: "https://www.youtube.com/watch?v=dQw4w9WgXcQ")
+    expect(article.reload.video).to be_present
+
+    put "/articles/#{article.id}", params: {
+      article: { video_source_url: "" }
+    }
+    expect(response).to have_http_status(:ok)
+    article.reload
+    expect(article.video_source_url).to be_nil
+    expect(article.video).to be_nil
+  end
+
+  it "removes video_source_url and video embed when passed null via JSON" do
+    article.update!(video_source_url: "https://www.youtube.com/watch?v=dQw4w9WgXcQ")
+    expect(article.reload.video).to be_present
+
+    put "/articles/#{article.id}",
+        params: { article: { video_source_url: nil } }.to_json,
+        headers: { "Content-Type" => "application/json" }
+
+    expect(response).to have_http_status(:ok)
+    article.reload
+    expect(article.video_source_url).to be_nil
+    expect(article.video).to be_nil
+  end
+
+  it "keeps video_source_url when the key is absent from the payload" do
+    article.update!(video_source_url: "https://www.youtube.com/watch?v=dQw4w9WgXcQ")
+
+    put "/articles/#{article.id}",
+        params: { article: { title: "New title" } }.to_json,
+        headers: { "Content-Type" => "application/json" }
+
+    expect(response).to have_http_status(:ok)
+    article.reload
+    expect(article.video_source_url).to eq("https://www.youtube.com/watch?v=dQw4w9WgXcQ")
+    expect(article.video).to be_present
+  end
+
+  it "ignores an unsupported video_source_url" do
+    article.update!(video_source_url: "https://www.youtube.com/watch?v=dQw4w9WgXcQ")
+
+    put "/articles/#{article.id}",
+        params: { article: { video_source_url: "https://vimeo.com/123456" } }.to_json,
+        headers: { "Content-Type" => "application/json" }
+
+    expect(response).to have_http_status(:ok)
+    expect(article.reload.video_source_url).to eq("https://www.youtube.com/watch?v=dQw4w9WgXcQ")
+  end
+
   context "when setting published_at in editor v2" do
     let(:tomorrow) { 1.day.from_now }
     let(:published_at) { "#{tomorrow.strftime('%d.%m.%Y')} 18:00" }