From 126110c9a0a0630cd556f5cb215422296a961029 Mon Sep 17 00:00:00 2001 From: gorhill Date: Wed, 30 Aug 2017 09:15:06 -0400 Subject: [PATCH] remove ability to pull latest version of resources.txt from remote repo. 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. --- src/js/assets.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/js/assets.js b/src/js/assets.js index 09dffbc2f..034eaac5f 100644 --- a/src/js/assets.js +++ b/src/js/assets.js @@ -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; @@ -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',