Maintenance: Improved handling of article preferences and attachments.

Approved-by: Dusan Vuckovic <dv@zammad.com>

app/assets/javascripts/app/lib/app_post/utils.coffee

@@ -335,6 +335,25 @@ class App.Utils
       .replace(/"/g, '"')
       .replace(/'/g, ''')
 
+  # safe = App.Utils.safeUrl(url)
+  # Returns the url only if it uses a scheme safe to put in an href, otherwise ''.
+  #   Relative/scheme-less urls are kept; javascript:, data: etc. are dropped.
+  # Intentionally stricter than the shared `safeUrl` of the Vue apps: its only caller
+  #   renders article preference links, which are only ever web links, so an allowlist
+  #   is used here instead of just rejecting the script-executing schemes.
+  @safeUrl: (url) ->
+    return '' if !url
+    return '' if !url.replace
+
+    # Strip control characters (incl. tab/newline) that browsers ignore when
+    #   parsing a scheme, e.g. `java<TAB>script:` would otherwise slip through.
+    stripped = url.replace(/[\x00-\x1F\x7F]/g, '')
+
+    scheme = stripped.match(/^\s*([a-z][a-z0-9+.\-]*):/i)
+    return stripped if !scheme
+    return stripped if _.contains(['http', 'https', 'mailto', 'tel'], scheme[1].toLowerCase())
+    ''
+
   # App.Utils.htmlStrip(element)
   @htmlStrip: (element) ->
     loop

app/assets/javascripts/app/views/ticket_zoom/article_view.jst.eco

@@ -54,7 +54,7 @@
         <div class="horizontal article-meta-row">
           <div class="article-meta-key u-textTruncate"><%- @T('Security') %></div>
           <div class="article-meta-value flex contain-text">
-            <%- securityPreferences.type %>:
+            <%= securityPreferences.type %>:
             <% if encryptionSuccess || encryptionWarning: %>
               <span title="<%- @Ti(encryptionComment) %>">
                 <% if encryptionSuccess: %>
@@ -171,7 +171,7 @@
             <% count = 0 %>
             <% for link in @links: %>
               <% if count > 0: %>|<% end %>
-              <a class="text-muted" href="<%- link.url %>" target="<%- link.target %>"><%- @T(link.name) %></a>
+              <a class="text-muted" href="<%= App.Utils.safeUrl(link.url) %>" target="<%= link.target %>"><%- @T(link.name) %></a>
               <% count =+ 1 %>
             <% end %>
           <% end %>

app/controllers/application_controller/has_download.rb

@@ -6,11 +6,13 @@ module ApplicationController::HasDownload
   def send_data(...)
     super
     set_null_csp
+    set_no_sniff
   end
 
   def send_file(...)
     super
     set_null_csp
+    set_no_sniff
   end
 
   private
@@ -50,6 +52,7 @@ module ApplicationController::HasDownload
       headers['Content-Length'] = bytesize.to_s
       self.response_body = body
       set_null_csp
+      set_no_sniff
     end
   end
 
@@ -72,4 +75,9 @@ module ApplicationController::HasDownload
   def set_null_csp
     request.content_security_policy = ActionDispatch::ContentSecurityPolicy.new.tap { |p| p.default_src :none }
   end
+
+  # Ask the browser to honor the sent content type instead of MIME-sniffing it.
+  def set_no_sniff
+    response.set_header('X-Content-Type-Options', 'nosniff')
+  end
 end

app/controllers/concerns/creates_ticket_articles.rb

@@ -42,6 +42,9 @@ module CreatesTicketArticles # rubocop:disable Metrics/ModuleLength
       end
       clean_params.delete(:type)
       clean_params[:internal] = false
+
+      # Article preferences are internal metadata; only trusted (agent) callers may set them.
+      clean_params.delete(:preferences)
     end
 
     if Ticket::Article::Type.lookup(id: clean_params[:type_id])&.name == 'email' && ticket.group.email_address.blank?

app/frontend/shared/components/CommonLink/CommonLink.vue

@@ -8,6 +8,7 @@ import { getLinkClasses } from '#shared/initializer/initializeLinkClasses.ts'
 import { useApplicationStore } from '#shared/stores/application.ts'
 import type { Link } from '#shared/types/router.ts'
 import stopEvent from '#shared/utils/events.ts'
+import { safeUrl } from '#shared/utils/url.ts'
 
 import type { Sizes } from './types.ts'
 
@@ -87,7 +88,9 @@ const path = computed(() => {
     return `${app.config.api_path}${props.link}`
   }
 
-  return props.link as string
+  // External links may carry caller-supplied urls (e.g. article preferences),
+  //   so drop unsafe schemes like javascript: before they reach the href.
+  return safeUrl(props.link as string)
 })
 
 const onClick = (event: MouseEvent) => {

app/frontend/shared/components/CommonLink/__tests__/CommonLink.spec.ts

@@ -146,4 +146,14 @@ describe('CommonLink.vue', () => {
     })
     expect(getLink()).toHaveAttribute('href', '/api/example')
   })
+
+  it('drops an unsafe javascript: href', () => {
+    const { getLink } = renderCommonLink({
+      props: {
+        link: 'javascript:alert(1)',
+        external: true,
+      },
+    })
+    expect(getLink()).toHaveAttribute('href', '')
+  })
 })

app/frontend/shared/utils/__tests__/url.spec.ts

@@ -0,0 +1,36 @@
+// Copyright (C) 2012-2026 Zammad Foundation, https://zammad-foundation.org/
+
+import { safeUrl } from '#shared/utils/url.ts'
+
+describe('safeUrl', () => {
+  it.each([
+    'https://example.com',
+    'http://example.com',
+    'mailto:test@example.com',
+    'tel:+1234567890',
+    '/relative/path',
+    '#anchor',
+    'data:image/png;base64,',
+    'blob:https://example.com/1234',
+    '',
+  ])('keeps safe url %s', (url) => {
+    expect(safeUrl(url)).toBe(url)
+  })
+
+  it.each([
+    'javascript:alert(1)',
+    'vbscript:msgbox(1)',
+    'JavaScript:alert(1)',
+
+    'java\tscript:alert(1)',
+
+    'java\nscript:alert(1)',
+  ])('drops script-executing url %j', (url) => {
+    expect(safeUrl(url)).toBe('')
+  })
+
+  it('returns empty string for nullish input', () => {
+    expect(safeUrl(undefined)).toBe('')
+    expect(safeUrl(null)).toBe('')
+  })
+})

app/frontend/shared/utils/url.ts

@@ -0,0 +1,25 @@
+// Copyright (C) 2012-2026 Zammad Foundation, https://zammad-foundation.org/
+
+const UNSAFE_URL_SCHEMES = new Set(['javascript', 'vbscript'])
+
+// Neutralize urls whose scheme would execute script when used as an href
+//   (javascript:, vbscript:), otherwise return the url unchanged. Everything else
+//   — relative, http(s), mailto, tel, data:, blob: — is kept, so this stays usable
+//   for general links (e.g. data-URI attachment downloads).
+export const safeUrl = (url?: string | null): string => {
+  if (!url) return ''
+
+  // Strip control characters (incl. tab/newline) that browsers ignore when
+  //   parsing a scheme, e.g. `java\tscript:` would otherwise slip through.
+  const stripped = Array.from(url)
+    .filter((char) => {
+      const code = char.charCodeAt(0)
+      return code > 0x1f && code !== 0x7f
+    })
+    .join('')
+
+  const scheme = stripped.match(/^\s*([a-z][a-z0-9+.-]*):/i)
+  if (scheme && UNSAFE_URL_SCHEMES.has(scheme[1].toLowerCase())) return ''
+
+  return stripped
+}

app/services/service/ticket/article/create.rb

@@ -81,6 +81,9 @@ class Service::Ticket::Article::Create < Service::Base
     article_data.delete :origin_by_id
 
     article_data[:internal] = false
+
+    # Article preferences are internal metadata; only trusted (agent) callers may set them.
+    article_data.delete :preferences
   end
 
   def transform_article(article, attachments_raw, subtype)

config/initializers/content_type_allowed_inline.rb

@@ -5,3 +5,26 @@ Rails.application.config.active_storage.content_types_allowed_inline.delete('app
 
 # Add legacy/invalid content type image/jpg (rather than image/jpeg) to allow showing of legacy avatars.
 Rails.application.config.active_storage.content_types_allowed_inline.push('image/jpg')
+
+# Serve JavaScript attachments as binary, so they are downloaded rather than interpreted.
+#   Covers the full list of JavaScript MIME types from the HTML specification
+#   (https://html.spec.whatwg.org/multipage/scripting.html#javascript-mime-type),
+#   since a browser executes a script response labeled with any of them.
+Rails.application.config.active_storage.content_types_to_serve_as_binary.push(
+  'application/ecmascript',
+  'application/javascript',
+  'application/x-ecmascript',
+  'application/x-javascript',
+  'text/ecmascript',
+  'text/javascript',
+  'text/javascript1.0',
+  'text/javascript1.1',
+  'text/javascript1.2',
+  'text/javascript1.3',
+  'text/javascript1.4',
+  'text/javascript1.5',
+  'text/jscript',
+  'text/livescript',
+  'text/x-ecmascript',
+  'text/x-javascript',
+)

public/assets/tests/qunit/html_utils.js

@@ -3852,3 +3852,33 @@ QUnit.test("#buildEmailAddress", assert => {
     '"Somebody @ \\"Company\\"" <some.body@example.com>'
   )
 })
+
+QUnit.test("safeUrl", assert => {
+  // Urls safe to render into an href are returned unchanged.
+  assert.equal(App.Utils.safeUrl('https://example.com'), 'https://example.com')
+  assert.equal(App.Utils.safeUrl('http://example.com'), 'http://example.com')
+  assert.equal(App.Utils.safeUrl('mailto:john.doe@example.com'), 'mailto:john.doe@example.com')
+  assert.equal(App.Utils.safeUrl('tel:+1234567890'), 'tel:+1234567890')
+  assert.equal(App.Utils.safeUrl('/api/v1/ticket_article_plain/1'), '/api/v1/ticket_article_plain/1')
+  assert.equal(App.Utils.safeUrl('#anchor'), '#anchor')
+
+  // Schemes which would execute script are dropped, regardless of case.
+  assert.equal(App.Utils.safeUrl('javascript:alert(1)'), '')
+  assert.equal(App.Utils.safeUrl('JavaScript:alert(1)'), '')
+  assert.equal(App.Utils.safeUrl('vbscript:msgbox(1)'), '')
+
+  // This one only renders web links, so data:/blob: are dropped here as well.
+  assert.equal(App.Utils.safeUrl('data:text/html,<script>alert(1)</script>'), '')
+  assert.equal(App.Utils.safeUrl('blob:https://example.com/1234'), '')
+
+  // Control characters are ignored by browsers when parsing a scheme, so they
+  //   must not be usable to smuggle one past the allowlist.
+  assert.equal(App.Utils.safeUrl('java\tscript:alert(1)'), '')
+  assert.equal(App.Utils.safeUrl('java\nscript:alert(1)'), '')
+  assert.equal(App.Utils.safeUrl('  javascript:alert(1)'), '')
+
+  // Blank input yields a blank href.
+  assert.equal(App.Utils.safeUrl(''), '')
+  assert.equal(App.Utils.safeUrl(undefined), '')
+  assert.equal(App.Utils.safeUrl(null), '')
+})

spec/controllers/application_controller/has_download/download_file_spec.rb

@@ -3,6 +3,27 @@
 require 'rails_helper'
 
 RSpec.describe ApplicationController::HasDownload::DownloadFile do
+  # The JavaScript MIME types from the HTML specification, all of which a browser
+  #   executes when a script response is labeled with them.
+  javascript_content_types = %w[
+    application/ecmascript
+    application/javascript
+    application/x-ecmascript
+    application/x-javascript
+    text/ecmascript
+    text/javascript
+    text/javascript1.0
+    text/javascript1.1
+    text/javascript1.2
+    text/javascript1.3
+    text/javascript1.4
+    text/javascript1.5
+    text/jscript
+    text/livescript
+    text/x-ecmascript
+    text/x-javascript
+  ]
+
   subject(:download_file) { described_class.new(stored_file.id, disposition: 'inline') }
 
   let(:file_content_type) { 'image/jpeg' }
@@ -46,6 +67,18 @@ RSpec.describe ApplicationController::HasDownload::DownloadFile do
 
         it_behaves_like 'forcing disposition to attachment'
       end
+
+      javascript_content_types.each do |javascript_content_type|
+        context "with JavaScript content type #{javascript_content_type}" do
+          let(:file_content_type) { javascript_content_type }
+
+          it_behaves_like 'forcing disposition to attachment'
+
+          it 'serves the file as binary' do
+            expect(download_file.content_type).to eq('application/octet-stream')
+          end
+        end
+      end
     end
 
     context "with given object dispostion 'attachment'" do

spec/requests/attachments_controller_spec.rb

@@ -151,6 +151,11 @@ RSpec.describe AttachmentsController, type: :request do
         include_examples 'responding with not found'
       end
     end
+
+    it 'sends X-Content-Type-Options: nosniff on download' do
+      get "/api/v1/attachments/#{public_store.id}"
+      expect(response.headers['X-Content-Type-Options']).to eq('nosniff')
+    end
   end
 
   describe '#destroy' do

spec/requests/ticket/article_spec.rb

@@ -924,6 +924,44 @@ AAAFCAYAAACNbyblAAAAHElEQVQI12P4//8/w38GIAXDIBKE0DHxgljNBAAO
       end
     end
 
+    describe 'article preferences write filtering' do
+      let(:preferences) do
+        {
+          links:    [{ url: 'javascript:alert(1)//', target: '_blank', name: 'x' }],
+          security: { type: '<img src=x onerror=alert(1)>' },
+        }
+      end
+      let(:params) do
+        {
+          ticket_id:   ticket.id,
+          body:        'some body',
+          type:        'note',
+          preferences:,
+        }
+      end
+
+      context 'when user is customer' do
+        let(:ticket) { create(:ticket, customer: user, group:) }
+        let(:user)   { create(:customer) }
+
+        it 'drops the client-supplied preferences' do
+          post '/api/v1/ticket_articles', params: params, as: :json
+          expect(response).to have_http_status(:created)
+          expect(ticket.articles.last.preferences).not_to include('links', 'security')
+        end
+      end
+
+      context 'when user is agent' do
+        let(:user) { create(:agent, groups: [group]) }
+
+        it 'keeps the preferences' do
+          post '/api/v1/ticket_articles', params: params, as: :json
+          expect(response).to have_http_status(:created)
+          expect(ticket.articles.last.preferences).to include('links', 'security')
+        end
+      end
+    end
+
   end
 
   describe 'POST /api/v1/ticket_articles (full response)', authenticated_as: :customer do

spec/services/service/ticket/article/create_spec.rb

@@ -120,6 +120,12 @@ RSpec.describe Service::Ticket::Article::Create, current_user_id: -> { user.id }
 
           expect(service_result.type.name).to eq('note')
         end
+
+        it 'drops client-supplied preferences' do
+          payload[:preferences] = { 'security' => { 'type' => '<img src=x>' } }
+
+          expect(service_result.preferences).not_to include('security')
+        end
       end
 
       # Agent-Customer is incorrectly detected as Agent in a group he has no access to
@@ -152,6 +158,12 @@ RSpec.describe Service::Ticket::Article::Create, current_user_id: -> { user.id }
 
           expect(service_result.type.name).to eq('phone')
         end
+
+        it 'keeps client-supplied preferences' do
+          payload[:preferences] = { 'security' => { 'type' => 'S/MIME' } }
+
+          expect(service_result.preferences).to include('security')
+        end
       end
     end
 

spec/system/ticket/zoom/article_preferences_spec.rb

@@ -0,0 +1,56 @@
+# Copyright (C) 2012-2026 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+
+# The preferences are planted directly via the factory, deliberately bypassing the
+#   write-path filtering, so the rendering is verified independently.
+RSpec.describe 'Ticket zoom > Article preferences rendering', authenticated_as: :agent, type: :system do
+  let(:group)  { create(:group) }
+  let(:agent)  { create(:agent, groups: [group]) }
+  let(:ticket) { create(:ticket, group: group) }
+
+  context 'with an unexpected security.type value' do
+    let(:article) do
+      create(:ticket_article, ticket: ticket, preferences: {
+               security: {
+                 type:       '<img src=x onerror="window.xssFired=true" id="xss-security">',
+                 encryption: { success: true, comment: 'c' },
+                 sign:       { success: false, comment: nil },
+               },
+             })
+    end
+
+    it 'renders the type as text and does not execute markup' do
+      visit "#ticket/zoom/#{article.ticket.id}"
+
+      open_article_meta
+
+      within :active_ticket_article, article do
+        # The security row must actually be rendered, so the check below is not vacuous.
+        expect(page).to have_css('.article-meta-value', text: 'onerror', visible: :all)
+        expect(page).to have_no_css('#xss-security', visible: :all)
+      end
+
+      expect(page.evaluate_script('window.xssFired')).to be_nil
+    end
+  end
+
+  context 'with an unsafe link url' do
+    let(:article) do
+      create(:ticket_article, ticket: ticket, preferences: {
+               links: [{ url: 'javascript:window.xssFired=true//', target: '_blank', name: 'evil' }],
+             })
+    end
+
+    it 'neutralizes the unsafe href' do
+      visit "#ticket/zoom/#{article.ticket.id}"
+
+      open_article_meta
+
+      within :active_ticket_article, article do
+        link = find('.article-meta-links a', text: 'evil')
+        expect(link[:href].to_s).not_to start_with('javascript:')
+      end
+    end
+  end
+end