Skip to content

Commit

Permalink
remove ability to pull latest version of resources.txt from remote repo.
Browse files Browse the repository at this point in the history
This is required as per Firefox extension reviewers. Mail exchange:

========

Reviewer:
> Do I read the code correctly that you are executing remote JS by
> downloading/updating from
> https://raw.githubusercontent.com/uBlockOrigin/uAssets/master/filters/resources.txt
> and injecting scripts in contentscripts.js?

Me:
> Yes, resources.txt contains scriptlets or other resources used to:
>
> - Minimize potential page breakage (e.g. google-analytics.com/ga.js);
> - Defuse anti-blockers (e.g. bab-defuser.js);
> - Defuse anti-blockers or minimize page breakage through redirection
> (e.g. 2x2-transparent.png)
>
> This is not a new feature -- this is also part of the legacy version,
> and I consider this is a major feature of uBO. Given how fast things can
> change out there, this allows me to quickly push fixes when a new issue
> is reported for a site without having to go through a full update of the
> extension.

Reviewer:
> I am aware that this is not a new feature. I am unclear why it has been
> allowed in the past, since it violates our policy about remote code
> execution. I assume it was missed due to the fairly complex codebase.
>
> I can approve this version so you are not blocked on the migration, but
> eventually, you cannot use functionality that executes remote code.
> Since we're moving to a more automated review process, you will be able
> to ship new versions without being blocked on a human review.

Me:
> Do I understand correctly that extensions such as TamperMonkey or
> ViolentMonkey won't be allowed on AMO?
>
> Those extensions are even more permissive than uBO given a user can
> import scripts from any source, while with uBO only scriptlets which are
> part of the project are allowed.

Reviewer:
> The key difference between add-ons like Tampermonkey and uBO is that in
> Tampermonkey, users are making an active and conscious decision to
> download and execute that specific code. In uBO, the user did not
> initiate that download/execution, nor are they even aware of it
> happening.

Me:
> So users of TamperMonkey -- tech-savvy or not -- can download & inject
> countless 3rd-party user scripts from countless authors, have them
> update on their own automatically at regular interval with no user
> intervention.
>
> On the other hand, it's not acceptable for me, the author of the
> extension, who users implicitly trusted when installing the extension,
> who is completely controlling and vouching for the content of
> "resources.txt", to have this one 1st-party resource file[1] to be
> updated at regular interval with no user intervention.
>
> So anyways, what is expected from me at this point? Do I need to remove
> scriptlet injection and resource redirection features? Do I need to
> remove only the updating part of resources.txt?
>
> [1] key to core features of uBO (counter anti-blockers + page breakage
> mitigations) and possibly an important factor in installing the
> extension.

========

Now about this commit: the purpose of the code change here is to
prevent "resources.txt" -- which is part of the package -- from being
updated -- this applies only to the Firefox webext[-hybrid] version
of uBO.
  • Loading branch information
gorhill committed Aug 30, 2017
1 parent d165432 commit 126110c
Showing 1 changed file with 20 additions and 0 deletions.
20 changes: 20 additions & 0 deletions src/js/assets.js
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,22 @@ var updateFirst = function() {
updateNext();
};

// Firefox extension reviewers do not want uBO/webext to fetch its *own*
// scriptlets/resources asset from the project's *own* repo (github.com).
var noRemoteResources = false;
(function() {
if (
typeof browser === 'object' &&
browser !== null &&
browser.runtime instanceof Object &&
typeof browser.runtime.getBrowserInfo === 'function'
) {
browser.runtime.getBrowserInfo().then(function(info) {
noRemoteResources = info.vendor === 'Mozilla';
});
}
})();

var updateNext = function() {
var assetDict, cacheDict;

Expand All @@ -1007,6 +1023,10 @@ var updateNext = function() {
if ( cacheEntry && (cacheEntry.writeTime + assetEntry.updateAfter * 86400000) > now ) {
continue;
}
// Update of user scripts/resources forbidden?
if ( assetKey === 'ublock-resources' && noRemoteResources === true ) {
continue;
}
if (
fireNotification(
'before-asset-updated',
Expand Down

10 comments on commit 126110c

@vlaszlo
Copy link

Choose a reason for hiding this comment

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

Can't you make the automatic updating of resources.txt a (well explained) opt-in feature in Firefox? In my understanding that would be comparable to what is possible with Tampermonkey et al.

@uBlock-user
Copy link
Contributor

Choose a reason for hiding this comment

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

This is terrible! So what will happen now that FF extension can no longer use scriptlets from resources.txt ?

@vlaszlo
Copy link

Choose a reason for hiding this comment

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

They can still be used. It's just that for new and changed scriptlets, the extension as a whole has to be updated again, like it was when scriptlets were introduced, IIRC.

The problem is that the Firefox addon reviewers see resources.txt as a part of uBO's code, and its auto-updating as a non-disclosed bypassing of the review process. With an opt-in checkbox, experienced users could make an "active and conscious decision" to use and auto-update that code, to quote the comparison to Tampermonkey by the reviewer in that email exchange above.

@gorhill
Copy link
Owner Author

@gorhill gorhill commented on 126110c Aug 30, 2017

Choose a reason for hiding this comment

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

experienced users could make an "active and conscious decision" to use and auto-update that code

An important point of "resources.txt" is especially for non-experienced users to not be forced to disable their blockers when encountering user-hostile anti-blockers or broken pages due to false positives from filter lists.

@kasper93
Copy link
Contributor

Choose a reason for hiding this comment

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

What if you sign resources.txt to ensure it is provided by you? Couldn't they allow updating? I mean you are trustworthy person, so they could allow it. Unless they really want to strictly stick to their rules.

Autoupdate feature in greasemonkey like addons is way more dangerous.

@gorhill
Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm not special, if they make an exception, then their rules become arbitrary, this defeats the purpose of rules.

What could maybe work is to have a special permission for the specific purpose in the current issue: "inject external scripts directly in web pages", and inform users at install time. Extensions like ViolentMonkey and uBO would use this permission, and users would be able to agree and install or disagree and abort installation.

@uBlock-user
Copy link
Contributor

@uBlock-user uBlock-user commented on 126110c Aug 31, 2017

Choose a reason for hiding this comment

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

Btw speaking of auto updates, why the chrome webstore version is still stuck at 1.13.8 ? It's been over a week now.

@gorhill
Copy link
Owner Author

Choose a reason for hiding this comment

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

If something goes wrong, I can't tell people to fall back to an older version of uBO on Chrome, there is only one version available and it's the latest one. I rather wait and be sure that there is no severe regression before I publish.

@Hrxn
Copy link

@Hrxn Hrxn commented on 126110c Aug 31, 2017

Choose a reason for hiding this comment

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

And this, kids, is how bureaucracy ruins everything..

@sdfasdfgawe456h4356
Copy link

Choose a reason for hiding this comment

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

what if you made the scriptlets into a separate "filter-list-kind-of"
uBlock filters - scriptlets or smth like that
like that it would fit more smoothly into the general way the extension works... and maybe they would then not consider it to be a part of the addon itself anymore and hence not see it as a bypassing of the AMO review process

Please sign in to comment.