ampproject / ampproject/amphtml
FR: `sandbox` iframes in `amp-facebook` components
- Dominant language
- JavaScript
- Stars
- 14.9k
- Forks
- 4.1k
- PR merge metrics
- No merged PRs in 30d
Description
We currently exclude `amp-facebook`, `amp-facebook-comments`, `amp-facebook-like`, and `amp-facebook-page` from applying `sandbox` attributes to their generated `iframe` element:
https://github.com/ampproject/amphtml/blob/36e818946a3231685e407297b8180a48f869f626/src/3p-frame.js#L158-L161
This is a tracking issue to introduce `sandbox` attributes for these components, which _may_* no longer be critical.
*Our manual tests show that embedded Facebook posts of all the above types don't appear to be affected aside from one side affect: an alarming error to the console, likely due to the 3p script altering `document.domain`, which is a `sandbox`-non-compliant operation. For all other intents and purposes the component appears to continue to work despite throwing this error.

It is possible that we were not able to detect potential breakages due to confirmation bias from manually testing our examples exclusively, and that [Domain Verification](https://developers.facebook.com/docs/sharing/domain-verification/) may have something to do with the `document.domain` access that [historically caused `sandbox` FB iframes to break](https://github.com/ampproject/amphtml/blob/36e818946a3231685e407297b8180a48f869f626/extensions/amp-facebook/0.1/test/test-amp-facebook.js#L72-L74).
@alanorozco suggests to verify if we can safely introduce `sandbox` by accessing facebook analytics from embedding comments configured to a certain domain or facebook page.
[Related discussion](https://github.com/ampproject/amphtml/pull/33852#discussion_r617587842)
Contributor guide
Research direction
Start in src/3p-frame.js at the sandbox exclusion and review extensions/amp-facebook/0.1/test/test-amp-facebook.js around the historical document.domain failure. Check the related discussion in PR #33852, then verify sandboxed amp-facebook, comments, like, and page embeds, including comments configured for a domain or Facebook page. Done means the components and relevant analytics continue working without unacceptable errors or regressions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- frontend, security
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100