MALSync / MALSync

Integrates MyAnimeList/AniList/Kitsu/Simkl into various sites, with auto episode tracking.
https://malsync.moe
GNU General Public License v3.0
2.17k stars 247 forks source link

Question about declarativeNetRequest #2427

Closed AliceDTRH closed 3 months ago

AliceDTRH commented 4 months ago

Suggestion

First, thank you for all the hard work on this project—it's greatly appreciated!

I noticed that commit https://github.com/MALSync/MALSync/commit/5cf78948969a5d700fdcc546303a9577628e3520 adds declarativeNetRequest. Firefox indicates that this permission allows the extension to "block content on any page". Is there any way to use a more specific permission instead? It sounds like a pretty broad permission.

Thank you for your time and consideration. Apologies if this is a silly concern.

lolamtisch commented 4 months ago

@AliceDTRH Permissions wise there is no difference to the version prior, it is just called different in manifest v3: Old Version: https://github.com/MALSync/MALSync/blob/0.9.8/webpackConfig/webextension.assets.js#L152-L153 and is not statically defined like in the new version. old version: https://github.com/MALSync/MALSync/blob/0.9.8/src/background.ts#L208C1-L224C3

Permissions are always frased for the worsed case which in 99% is not true. The declarativeNetRequest api is just to edit webrequests. We do not use the api dynamically, only the static defined version you linked.

Explanation:

"action": {
  "type": "modifyHeaders",
  "requestHeaders": [{
    "header": "user-agent",
    "operation": "set",
    "value": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/121.0.0.0 Safari/537.36"
  }]
},

This only overrides the user-agent to apear like it is a desktop. This is necessary because on mobile (android firefox), mal always returns the mobile version of the website, but we need the desktop version to get the necessary infos from it.

"condition": {
  "urlFilter": "https://myanimelist.net/*",
  "resourceTypes": ["xmlhttprequest"]
}

This makes sure that it only hits mal and only when doing requests in the background. So the any page in the permission text is wrong in this case, as it only hits mal and only to change the user-agent.

I hope this somehow makes sense to you, if not ask and I will clarify it more.

Pinefone commented 3 months ago

The new warning does sound a bit terrifying though. I suggest to use declarativeNetRequestWithHostAccess instead, as it won't display the warning but limits the use to sites under host_permissions.

At least in Chrome it won't display the warning, I don't know about Firefox. https://developer.chrome.com/docs/extensions/reference/api/declarativeNetRequest

"declarativeNetRequestWithHostAccess" A permission warning is not shown at install time, but you must request host permissions before you can perform any action on a host. This is appropriate when you want to use declarative net request rules in an extension which already has host permissions without generating additional warnings.

lolamtisch commented 3 months ago

Switched to declarativeNetRequestWithHostAccess lets see if that broke something