Skip to content

[Minor] Add App Bridge script to allow for strict CSP policy#1997

Merged
lizkenyon merged 1 commit intomainfrom
add-app-bridge-csp-to-frame-ancestors
Sep 23, 2025
Merged

[Minor] Add App Bridge script to allow for strict CSP policy#1997
lizkenyon merged 1 commit intomainfrom
add-app-bridge-csp-to-frame-ancestors

Conversation

@lizkenyon
Copy link
Contributor

@lizkenyon lizkenyon commented Sep 17, 2025

What this PR does

This gem requires an asset that is hosted on cdn.shopify.com:
https://cdn.shopify.com/shopifycloud/app-bridge.js

For applications that enforce a strict ContentSecurityPolicy, the asset is unlikely to be listed as a permitted script-src .

This PR modifies the FrameAncestors concern to allow loading of the the App Bridge script. This concern is included in the EmbeddedApp concern.
Any time the app attempts to load in an embedded context it will allow the loading of App Bridge

test-ruby-csp-1080.mov

Reviewer's guide to testing

To test:

  1. Update a dev app to use strict CSP policy
    e.g.
Rails.application.configure do
  config.content_security_policy do |policy|
    policy.default_src :self, :https
    policy.font_src    :self, :https, :data
    policy.img_src     :self, :https, :data
    policy.object_src  :none
    # For development: allow unsafe-inline for Vite's inline script and styles
    # In production, this should be removed or use SHA-256 hash
    if Rails.env.development?
      policy.script_src  :self, :unsafe_inline
      policy.style_src   :self, :https, :unsafe_inline
    else
      policy.script_src  :self
      policy.style_src   :self, :https
    end
    # Specify URI for violation reports
    # policy.report_uri "/csp-violation-report-endpoint"
  end
  1. Using the current gem, attempt to install the embedded app.
  2. See console error that app bridge script cannot be loaded
  3. Use local version of this branch instead of published gem
  4. See app is able to install correctly

Things to focus on

  1. Are there any scenarios where we would need to load App Bridge but it is not in an embedded context?
  2. Is there any reason why we would not want to set the CSP config for app bridge script for all apps?

Checklist

Before submitting the PR, please consider if any of the following are needed:

  • Update CHANGELOG.md if the changes would impact users
  • Update README.md, if appropriate.
  • Update any relevant pages in /docs, if necessary

@lizkenyon lizkenyon requested a review from a team as a code owner September 17, 2025 21:52
Copy link
Contributor

@sle-c sle-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lizkenyon lizkenyon merged commit 551948c into main Sep 23, 2025
7 checks passed
@lizkenyon lizkenyon deleted the add-app-bridge-csp-to-frame-ancestors branch September 23, 2025 14:09
lizkenyon added a commit that referenced this pull request Oct 22, 2025
Prevents breaking changes from PR #1997 which would force script-src on all apps
Apps using default template (CSP disabled) must continue working without changes.
Only apps explicitly enabling strict CSP should need App Bridge configuration.

Breaking change avoided: PR #1997 automatically added script-src 'self' to all apps,
blocking inline scripts including Vite HMR in development. This reverts to opt-in.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
lizkenyon added a commit that referenced this pull request Oct 22, 2025
Prevents breaking changes from PR #1997 which would force script-src on all apps
Apps using default template (CSP disabled) must continue working without changes.
Only apps explicitly enabling strict CSP should need App Bridge configuration.

Breaking change avoided: PR #1997 automatically added script-src 'self' to all apps,
blocking inline scripts including Vite HMR in development. This reverts to opt-in.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
lizkenyon added a commit that referenced this pull request Oct 22, 2025
Prevents breaking changes from PR #1997 which would force script-src on all apps
Apps using default template (CSP disabled) must continue working without changes.
Only apps explicitly enabling strict CSP should need App Bridge configuration.

Breaking change avoided: PR #1997 automatically added script-src 'self' to all apps,
blocking inline scripts including Vite HMR in development. This reverts to opt-in.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants