search

AdguardTeam / AdguardForWindows

AdGuard for Windows open bug tracker
https://adguard.com/
691 stars 72 forks source link

Allow to trust built-in third-party filters #4386

Open contribucious opened 2 years ago

contribucious commented 2 years ago

Intro

Hi. :wave: Let's take a concrete example. For reasons I'll explain in the Context section — sorry in advance for the length —, I wanted to switch from the built-in third-party Fanboy's Annoyances filter (with its trustLevel set to "low") to the original Fanboy's Annoyance List (not having this limitation). Impossible unfortunately without a workaround (i.e. to use another URL) in AdGuard for Windows — unlike in AdGuard Browser extension.

Proposed solutions

    View the implications … - The reported issues by the users should automatically mention this state for these filters. - The AdGuard Filters team will need to ensure that they use these same conditions for their reproducibility tests. - :bulb: If AdGuard does not want to take the risk of hosting third-party filter versions with risky rules inside, simply use the original source when the user checks "trusted filter" on an otherwise "low" one (AdGuard knows how to handle `+js` rules and stuff like that anyway).

:mag: About this hijack

:heavy_multiplication_x: AdGuard for Windows current problematic state:

View it … - **Initial state** — Built-in third-party `Fanboy's Annoyances` filter added: [screenshot](https://user-images.githubusercontent.com/4764956/185612901-727bb247-c97b-43e3-a91c-aaa99670aa21.png) :arrow_right_hook: Observe: 57551 rules. - When removing it then adding manually a new filter with its official URL (which is: `https://secure.fanboy.co.nz/fanboy-annoyance_ubo.txt`): [screenshot](https://user-images.githubusercontent.com/4764956/185613173-c1d872a8-9bc4-48e2-b9c4-591f49277609.png) :arrow_right_hook: Observe: the name is not `Fanboy's Annoyance List` but has become `Fanboy's Annoyances` with `v0.0.0.0`. **#hijack** - Once added: [screenshot](https://user-images.githubusercontent.com/4764956/185613478-6472335e-effa-4dfc-ba74-b6260104be35.png) :arrow_right_hook: Observe: 57551 rules _only_, name of the AG version and auto put in "Annoyances" section. - NOW, when adding manually a new filter with _another_ URL to the same location **as a workaround** (using the following URL instead: `https://www.fanboy.co.nz/fanboy-annoyance_ubo.txt`): [screenshot](https://user-images.githubusercontent.com/4764956/185613877-8ba65101-63db-4879-8586-9ee5d3289f13.png) :arrow_right_hook: Observe: the name **is** correctly `Fanboy's Annoyance List` with the current version of the moment. - Once added: [screenshot](https://user-images.githubusercontent.com/4764956/185613918-e3d0cdcb-864a-4b2d-b4d0-cafe7ec779b0.png) :arrow_right_hook: Observe: **59016** rules _as expected_, original name kept and put in the top section ("User" section).  

:heavy_check_mark: AdGuard Browser extension current okay state:

View it … - Adding manually a new filter with its official URL (which is: `https://secure.fanboy.co.nz/fanboy-annoyance_ubo.txt`): [screenshot](https://user-images.githubusercontent.com/4764956/185619346-436b3cca-1d41-4f58-8987-fb23542f2f99.png) :arrow_right_hook: Observe: **59015** rules _as expected_`*`, original name kept. NOT hijacked in this case unlike `AdGuard for Windows`.   _`*` (So, all the 59000+ rules. No need to worry about the 1 rule of difference versus `AdGuard for Windows` here, the `sha256sum` returned value for this file both accessed via "www." and "secure." is always identical for information.)_ - Once added: [screenshot](https://user-images.githubusercontent.com/4764956/185619640-f776cece-5f01-4ff5-8684-27d4a77d6234.png) :arrow_right_hook: Observe: original name kept, correct version of the moment. Tested and … additional rules are correctly present. :heavy_check_mark:
contribucious commented 2 years ago

Context / Why it's desired initially

Read it … ### Excluding all `#$#` rules (as currently in "trust level: low" filters, like `Fanboy's Annoyances`) IS problematic for "cookies" box. For context, I needed to use the original version _(in order not to have the limitation of trust level low)_ because many websites are broken by adding this (popular) filter in a built-in way (because of trust level low) due to this general action: `! [RULE-HERE] is excluded by "#$#" in exclusions-low.txt` Excluding `#$#` completely for security (and more) is understandable, but a lot of "annoyances" (the purpose of this filter in particular) of the "accept or not our cookies" type consist of the following: 1) Hide the cookies box — using basic CSS rule(s); 2) Play on the `overflow` CSS property of the body and/or html element (otherwise the site is _very_ often not scrollable and therefore becomes "broken"). It's the second point that is problematic, because we have for example 316 matches (and many slight variants of style in fact) just for: `##body,html:style(height: auto !important; overflow: auto !important)` Like in: `! Rule "[DOMAIN-HERE]##body,html:style(height: auto !important; overflow: auto !important)" converted to: "[DOMAIN-HERE]#$#body,html { height: auto !important; overflow: auto !important }"` :eyes: Converted but then … excluded: **Search:** `is excluded by "#$#" in exclusions-low.txt` **In:** `https://raw.githubusercontent.com/AdguardTeam/FiltersRegistry/master/filters/ThirdParty/filter_122_FanboysAnnoyances/diff.txt` :arrow_right: You'll see "More than 1000 matches"! Since a rule often has multiple domain names, that's a lot of potentially broken websites when using this popular filter in its AdGuard built-in version (I have seen this a few times myself). Instead of improving the user experience (thanks to the hard work of @ryanbr mainly regarding this list), it instead makes the latter _worse_ on many websites (and this in a way unrelated to Ryan's work so). And currently, without signaling this to the user (for example via a ribbon like [this](https://github.blog/wp-content/uploads/2008/12/forkme_left_red_aa0000.png?resize=149%2C149) or like [that](https://user-images.githubusercontent.com/4764956/185623051-819544d2-2117-43f3-8fe8-970879f7f990.png) on the filter, but signaling "low" concerning its current trust level instead). Because otherwise, you might think that it's the filter itself (created by its author) that breaks a website, and possibly even report a detected issue on the author's GitHub repository for nothing (waste of time and energy), when it's actually … "the limited version of this filter in AdGuard" the problem here. Because of the trust level set on low for this one (which is _not_ mentioned to the user when he add it at the very least, thinking to add "the one everyone uses", not a "partially broken" version of it :wink:). ### _Possible_ solutions. - Do not exclude rules — therefore strongly ensuring that they are OK on the security side — related to `overflow` CSS property for body and/or html elements (and some more complex ones for these elements unfortunately, search for example `body,html:style`) for the "low" level; - Add a new "low+" level for … "Annoyances" type filters that deal with cookie related dialogs; - Increase the trust level (`trustLevel`) of some built-in third-party (popular ones at least) "Annoyances" filters that deal with cookie related dialogs; - Inform, at least, the user via a ribbon "low" on the "trust level: low" filters (with possibly a tooltip regarding its potential effects); - Allow the user to set any built-in third-party filter as trusted by himself; - In any case, do not hijack — to a more limited version — the original URLs of the third-party filters if added manually via URL by the user when the latter has explicitly checked "trusted filter" at least. _Or if hijacked, provide him with an AdGuard version of the same level of trust (i.e. converted/optimized but \*not\* more limited), although in this particular case hosting these risky versions of the filters can be problematic._ Or, at the very least, let him know about this hijack and why that, in this case. Finally, it should be noted that `AdGuard Browser extension` is OK in this regard, as detailed in my issue. - … Yours? :relaxed: _I'm all ears!_   ##### Sorry again for the length!
Aydinv13 commented 2 years ago

@contribucious thank you for the clear design and very detailed description of your request, we considered it and decided to redo the principle of work in this place.

contribucious commented 2 years ago

@Aydinv13 Great! Thank you! :thumbsup: