Skip to content

Commit

Permalink
include the HTTP Permissions-Policy on non-HTML Content-Types
Browse files Browse the repository at this point in the history
[CVE-2024-28103]
The application configurable Permissions-Policy is only
served on responses with an HTML related Content-Type.

This change allows all Content-Types to serve the
configured Permissions-Policy as there are many non-HTML
Content-Types that would benefit from this header.
(examples include image/svg+xml and application/xml)
  • Loading branch information
fresh-eggs authored and tenderlove committed Jun 4, 2024
1 parent ac87f58 commit b329b26
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 7 deletions.
7 changes: 0 additions & 7 deletions actionpack/lib/action_dispatch/http/permissions_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ def call(env)
request = ActionDispatch::Request.new(env)
_, headers, _ = response = @app.call(env)

return response unless html_response?(headers)
return response if policy_present?(headers)

if policy = request.permissions_policy
Expand All @@ -36,12 +35,6 @@ def call(env)
end

private
def html_response?(headers)
if content_type = headers[CONTENT_TYPE]
/html/.match?(content_type)
end
end

def policy_present?(headers)
headers[POLICY]
end
Expand Down
51 changes: 51 additions & 0 deletions actionpack/test/dispatch/permissions_policy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,57 @@ def test_invalid_directive_source
end
end

class PermissionsPolicyMiddlewareTest < ActionDispatch::IntegrationTest
APP = ->(env) { [200, {}, []] }

POLICY = ActionDispatch::PermissionsPolicy.new do |p|
p.gyroscope :self
end

class PolicyConfigMiddleware
def initialize(app)
@app = app
end

def call(env)
env["action_dispatch.permissions_policy"] = POLICY
env["action_dispatch.show_exceptions"] = :none

@app.call(env)
end
end

test "html requests will set a policy" do
@app = build_app(->(env) { [200, { Rack::CONTENT_TYPE => "text/html" }, []] })
# Dummy CONTENT_TYPE to avoid including backport of the following commit in
# a security-related patch:
# https://github.com/rails/rails/commit/060887d4c55a8b4038dd4662712007d07e74e625
get "/index", headers: { Rack::CONTENT_TYPE => 'cant/be-nil' }

assert_equal "text/html", response.headers['Content-Type']
assert_equal "gyroscope 'self'", response.headers['Feature-Policy']
end

test "non-html requests will set a policy" do
@app = build_app(->(env) { [200, { Rack::CONTENT_TYPE => "application/json" }, []] })
get "/index", headers: { Rack::CONTENT_TYPE => 'cant/be-nil' }

assert_equal "application/json", response.headers['Content-Type']
assert_equal "gyroscope 'self'", response.headers['Feature-Policy']
end

private
def build_app(app)
PolicyConfigMiddleware.new(
Rack::Lint.new(
ActionDispatch::PermissionsPolicy::Middleware.new(
Rack::Lint.new(app),
),
),
)
end
end

class PermissionsPolicyIntegrationTest < ActionDispatch::IntegrationTest
class PolicyController < ActionController::Base
permissions_policy only: :index do |f|
Expand Down

0 comments on commit b329b26

Please sign in to comment.