-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
registerContentScripts misleadingly names page scripts as content scripts #313
Comments
I don't think this is a meaningful change to make at this point. "ContentScript" is slightly inaccurate now, but it is still "content". |
It's completely inaccurate, not slightly. MV3 is still in the early stages, it's bugged all around, its API is evolving, now is the time to fix it. It's a meaningful change that fixes a mistake made by developers who apparently weren't working on the extension API previously and which wasn't caught by a more experienced developer. A script in the main page world is not a content script in the terms of the extension API. Content scripts are processed differently in many aspects e.g. network requests when making a POST/PUT have Origin header, only the isolated world can access Note that the problem is not semantics per se, but the fact that the name "Content" in the methods creates false expectations because for ~10 years it meant a script in the isolated world that has quite a few critical differences to the main world. A content script still means a "script in the isolated world". |
"content scripts" do not mean "isolated world scripts"; they mean "scripts running on the context of a page" (which is true for both isolated world and main world scripts). We should provide better documentation on this, specifically fix the link you mentioned |
It's what this term meant for ~10 years.
This is a generic introduction, not the definition. The problem is not semantics but false expectations and misleading developers into making their extensions unsafe. |
I guess if you fix every piece of the documentation to link to an article that lists dangers of running code in the MAIN world it'll be fine. Primarily the danger of spoofing of standard JS methods and prototypes to intercept and exfiltrate data from such scripts. |
We discussed this topic in the meeting today (https://github.com/w3c/webextensions/blob/main/_minutes/2022-12-08-wecg.md) and did not see value in the churn associated with renaming the API. I agree that there are dangers associated with running code in the page's context, but that issue is not due to the name of the API. The scripting API documentation should call out the danger of running code in the main world. Side note: Renaming the scripts to be more generic, from |
Indeed, looking back I see the underlying problem that I tried to convey is the undocumented change of the behavior not the name per se. Every browser's documentation should clearly and prominently emphasize the dangers of MAIN world injection and do it more convincingly than the current MDN article to frighten the developers properly e.g. show or at least mention how trivial it is to hack into any webpack-produced bundle by overriding Object.defineProperty and Function.prototype.call. |
That page on MDN features the following prominent warning. If that's not sufficiently convincing, then I wonder what you're looking for.
|
This warning isn't frightening at all because it looks like a generic "don't do this do this" proclamation on the same level of urgency as a warning about an experimental API. I guessed this problem may be solved by showing an example that instantly conveys how trivial it is for a web page to hook into almost any JS code. |
With that mindset no amount of documentation is sufficient to stop the development of insecurely implemented extensions.
There are numerous mentions on that page that explicitly describes why working with page objects is unreliable. If someone ignores all these warnings and caveats, then I think that a more likely outcome of an "example" is for an unskilled reader to at most write one counter-defense against that specific example and have a false sense of security while being ignorant about other aspects. There is hardly a way to run code in an untrusted environment and get fully reliable results. |
It's a practical mindset. I'd say it's shared by the majority of programmers. People who solve practical focused tasks and don't work on web platform specifications with caveats. As a practical programmer who has seen thousands of cases where people make mistakes regardless of how many times the documentation warns against it verbally, I hope that adding an example of how spoofing/interception works would help people understand the problem better. An example/MCVE/repro always helps. |
This is what Violentmonkey/Tampemonkey do currently by extracting safe methods from an iframe and any extension can do the same. In the future ShadowRealm might help, assuming its constructor is unforgeable. This is impossible in MV3 though as its CSP for the isolated world doesn't allow creating |
After reading the meeting notes it seems to me everyone considered this problem to be about naming/bikeshedding whereas it's actually about a much bigger and more dangerous change of security guarantees that subverts established behavior expectations of developers for the past 10 years. I'll probably open a new issue... |
This is yet another evidence that the term "content script" has an established meaning of a script in the isolated world. Over the last ~10 years it's been used in like a million of places in code, forums and sites like stackoverflow, apps, UI, so the destructive impact of this change is enormous and the benefits are non-existent. The only benefit of diminishing/disregarding the problem I see is that Chromium team doesn't have to lose face for not thinking this through. |
I just want to point out that I've been confused in some messages from Chrome devs when they've said "content scripts". Coming form Firefox I've always thought that meant "privileged". Apparently that's not the case in the Chrome world as that term can mean "running in the page context / access level" ( Common terminology would be nice.. |
Prior to the introduction of |
FYI: I've submitted a PR to document the |
chrome.scripting.registerContentScripts, getRegisteredContentScripts, updateContentScripts, unregisterContentScripts all misleadingly describe the scripts injected into the MAIN world as content scripts - both in the name of the method and in their synopsis. The API and the documentation misleads developers into thinking that the MAIN-world script has the same security guarantees as a normal content script, which it doesn't, and exposes the same
chrome
API, which it doesn't either.A script in MAIN world is not a content script.
A content script is a script running in the isolated world both historically and by design for the extra layer of security.
Suggesting to drop
Content
from the names and ensure "register" is present in all names (updateContentScripts was missing it). It will also align with the mainexecuteScript
method, which never contained "content" even though it always injected only content scripts (there was noworld
parameter in MV2).The browser should probably print a deprecation message each time the old names are used, and remove them in ~1 year.
Additionally, each browser's documentation repository is encouraged to add a short notice about the distinction and security implications of running code in a possibly poisoned/spoofed environment (see also https://crbug.com/1261964), maybe with a link to a more detailed article.
The text was updated successfully, but these errors were encountered: