RSSHub-Radar icon indicating copy to clipboard operation
RSSHub-Radar copied to clipboard

feat: 增加自定义radar-rules.js文件链接的功能

Open yindaheng98 opened this issue 3 years ago • 4 comments

Close #634 Close #460

yindaheng98 avatar Jun 20 '21 14:06 yindaheng98

Firefox目前不支持远程更新,radar-rules文件也可以在本地进行修改

NeverBehave avatar Sep 27 '21 00:09 NeverBehave

Firefox目前不支持远程更新,radar-rules文件也可以在本地进行修改

In fact, if remote rules are disabled, any local modifications will be ignored in the meantime.

https://github.com/DIYgod/RSSHub-Radar/blob/e015163e536d4b6596481ecd484c25e5f07e02f8/src/js/common/rules.js#L29-L43

Though @DIYgod never describes the reason why local modifications should be ignored, there are still some clues.

https://github.com/DIYgod/RSSHub-Radar/blob/e015163e536d4b6596481ecd484c25e5f07e02f8/src/js/sandbox/utils.js#L44-L46

Here we see an eval(), which is strictly prohibited by Firefox and AMO (addons.mozilla.org). Wrapping the code containing eval() in a sandbox (a196e495e5e9bf2d9d05a1aa3a1c4acad7f9eaf5) won't help on the recent versions of Firefox: eval() and its friends will still be prohibited.

Will a CSP containing 'unsafe-eval' help? The answer is "yes". But here is another bad news: 'unsafe-eval' effectively takes away the possibility to publish to AMO.

extensions with 'unsafe-eval', remote script, blob, or remote sources in their CSP are not allowed for Firefox extensions as per the add-on policies and due to major security issues.

If 'unsafe-eval' is not enabled and rules are modified or updated, parseRules() will immediately throw an EvalError and break everything. I consider that's the real reason why e9a8ffeb03421540a7188f9a41dc48df12d24d21 also made any local modifications on the rules be ignored.


The conclusion is obvious, this PR will NOT help Firefox users any (but that's not the aim of this PR, lol). The prohibition of remote scripts is not the real reason why remote rules should be completely disabled on Firefox, but the prohibition of eval().

So, are there any other alternative solutions? The best solution is completely deprecating the usage of eval() (or its friends). I consider Firefox does better than Chrome to this extent and probably Chrome will follow, who knows? An addon using eval() to execute remote scripts (even if it is considered to be trusted and executed in a sandbox) is neither cool nor modern. For now, most target functions are just matching URL query strings / page elements / inline script variables or filtering/mapping matches, fairly limited use cases, isn't it? Another solution is self-distributing the Firefox addon. It is believed that self-distributing addons will face looser reviews and here's an example with eval() that passes the self-distributing review. Here is a toturial on how to self-distributing Firefox addons.

Rongronggg9 avatar Apr 16 '22 17:04 Rongronggg9

@Rongronggg9 The first step could be disable/remove eval() function and sandbox target and start from there. I agree that DOM read is pretty bad as well so maybe we could add it back once we have a better solution.

However, I don't think self-distributing rules will be helpful: it opens up attack surface for misuse, and taking control over rules is necessary. We may consider it after we have proper sandbox mechanism. An example will be tampermonkey, it disable scripts for website like paypal.com. I get the idea that "user should be responsible for their action" but as developers, we should also avoid to make user take unnecessary risks.

sandbox PR is welcome :), if you have a specs in hand. There are other project e.g. RSSBud also using radar rules so backward-compatibility is also something needs to be done.

@Cay-Zhang @LeetaoGoooo

NeverBehave avatar Apr 16 '22 23:04 NeverBehave

However, I don't think self-distributing rules will be helpful

I've told the same opinion... The conclusion is obvious, this PR will NOT help Firefox users any

A self-distributed add-on is just an add-on chosen not to be listed on AMO, not an add-on containing self-distributing rules. I showed no interest in customizable remote rules URL. I just took a place to describe the current situation of Firefox add-ons.

sandbox PR is welcome :)

Loading remote rules, and everything related to parameter matching are already executed in a sandbox. The real problem is that loading remote rules should not need remote code execution. So what we need to do is to draft a new specification to describe the parameter matchers, making radar rules a valid JSON instead of Javascript.

We may consider it after we have proper sandbox mechanism. An example will be tampermonkey, it disable scripts for website like paypal.com.

That's not a sandbox issue, it's just a filter. And user script is working in another mechanism, I don't think it can be used to load remote rules or do parameter matching.


I do have some drafts in my mind, but I consider if we drop the support for querySelector, we do even not need a sandbox. (But I suppose you are opposed to doing so, lol)

Generally speaking, a target function can be described in such a workflow, in which each step is optional:

match (URL path/URL qs/regex/querySelector) -> post process (string operation) -> filter -> remap

Supporting all sorts of querySelector operations (and chain operations) will make things too complex, maybe a subset will be fine enough? There are still some target functions that cannot be described in such a workflow (e.g. if they include conditional branches), maybe we could only drop them.

Later I may draft a PR / issue.

Rongronggg9 avatar Apr 17 '22 00:04 Rongronggg9

Thanks to everyone for participating. This feature has been implemented in v2. Firefox now also offers the same updates as Chrome.

DIYgod avatar Dec 17 '23 11:12 DIYgod