Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drafting a new specification of radar rules #692

Open
Rongronggg9 opened this issue Apr 17, 2022 · 16 comments
Open

Drafting a new specification of radar rules #692

Rongronggg9 opened this issue Apr 17, 2022 · 16 comments

Comments

@Rongronggg9
Copy link
Contributor

Rongronggg9 commented Apr 17, 2022

Context

#635

Motivation

Currently, the target field can be either 1. a route string; or 2. a function. The second use case could potentially lead to some security worry. What's more, it effectively makes radar rules unable to be converted to a JSON but remains to be a Javascript. That is, to load remote or local-modified rules cannot avoid the usage of eval(), which is strictly prohibited by some modern browsers (e.g. Firefox and Safari, total market share is ~18%) by default. We have already seen there are two distributions (AMO, Mac App Store) that have to disable remote rules and ignore local modifications. And no one can promise that Chrome Web Store would not ban the usage of eval() in the future too.

Another reason is most radar rules use target function never need to access the page content, but just match URL query strings / do some filtering/remapping/regex replacement. These rules should have been compatible with RSSBud/RSSAid, but with the current specification, they can only use a target function which is incompatible with RSSBud/RSSAid. The new specification should change these embarrassing situations.

Last but not least, some users indeed have their demand to use their own online rule list instead of the official one. Though the user is the only one who is responsible for their action, we should still worry about their data security since using eval() to load remote rules is typical remote code execution (RCE). As a result, such a feature could not be added before we completely deprecated the usage of eval(), and the prerequisite is this issue.

Core Idea

A typical target function can be described in these three workflows or there combination:

  1. 1st positional parameter param -> post-process (string operations) -> filter -> remap
  2. 2nd positional parameter URL -> extract qs -> ...
  3. 3rd positional parameter document -> DOM operations -> ...

Thus, I constructed a 4-stage workflow:

  • match -> post-process -> filter -> remap

But after deep diving, I think there's no need to distinguish these stages, we just need to define some actions and let rule authors chain them:

matcher: {
  paramName: [
    {
      action: ...,
      ...: ...,
    },
    {...},
    {...}, 
    ...
  ]
}

Actions

Universal parameters

  • action: action name
  • stopIfEmpty: whether to stop the chain immediately if the output of the current action is an empty string? (optional, default: true)

Input (must be placed at the beginning of the chain)

  • URL path: { action: 'path' }, { action: 'path', key: '...' } (formerly param[paramName])
  • URL qs: { action: 'qs' }, { action: 'qs', key: '...' }
  • raw URL: { action: 'URL' }
  • raw document as a string: { action: 'raw' }
  • querySelector: { action: 'DOM', ...(To be determined) } (maybe we could implement some workarounds to make it chainable?)
  • More? Share your ideas ☺️

Modify

  • regex matching: { action: 'regex', match: '...' }, { action: 'regex', match: '...', matchGroup: 1 }
  • regex replacement: { action: 'regex', match: '...', replace: '...' }
  • string operation (regex matching/replacement should be enough)
  • More? Share your ideas ☺️

Filter

Hmm, regex replacement should be enough, but that's OK to provide a convenient action.

Remap

{
  action: 'remap',
  map: {
    key1: 'value1',
    key2: { value: 'value2', overrideTitle: 'title2' }
  }
  default: 'defaultValue', // optional, default: ''
}

A Quick Look at the New Specification

Need more discussion to determine more details.

Due to the support for optional parameters, #693 is a prerequisite of the new specification.

Under the new specification, the radar rule list should be exported both in .js and .json. The former, which contains all fields, is preserved for backward compatibility; the latter, in which target functions are filtered out and old target route strings are remapped to route, will be used by new versions of RSSHub Radar.

{
    '12306.cn': {
        _name: '12306',
        kyfw: [
            {
                title: '售票信息',
                docs: 'https://docs.rsshub.app/travel.html#_12306',
                source: ['/', '/otn/leftTicket/init'],
                target: (params, url) => { // for backward compatibility, will be dropped in JSON output
                    const searchParams = new URL(url).searchParams;
                    const from = searchParams.get('fs').split(',')[0];
                    const to = searchParams.get('ts').split(',')[0];
                    const date = searchParams.get('date');
                    return `/12306/${date}/${from}/${to}`;
                },
                route: '/12306/:date/:from/:to/:type?', // `/:type?` can be omitted since here we don't match it
                matcher: {
                    from: [
                        { action: 'qs', key: 'fs' },
                        { action: 'regex', match: '^([^,]+)' },
                    ],
                    to: [
                        { action: 'qs', key: 'ts' },
                        { action: 'regex', match: '^([^,]+)' },
                    ],
                    date: [
                        { action: 'qs', key: 'date' }
                    ],
                },
            }
        ],
    },
}

A use case of remap and overrideTitle

Old rule

{
    '423down.com': {
        _name: '423down',
        www: [
            {
                title: '安卓软件',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'apk') {
                        return '/423down/android/apk';
                    }
                }
            },
            {
                title: '原创软件',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'zd423') {
                        return '/423down/computer/originalsoft';
                    }
                }
            },
            {
                title: '媒体播放',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'multimedia') {
                        return '/423down/computer/multimedia';
                    }
                }
            },
            {
                title: '网页浏览',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'browser') {
                        return '/423down/computer/browser';
                    }
                }
            },
            {
                title: '图形图像',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'image') {
                        return '/423down/computer/image';
                    }
                }
            },
            {
                title: '聊天软件',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'im') {
                        return '/423down/computer/im';
                    }
                }
            },
            {
                title: '办公软件',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'work') {
                        return '/423down/computer/work';
                    }
                }
            },
            {
                title: '上传下载',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'down') {
                        return '/423down/computer/down';
                    }
                }
            },
            {
                title: '系统辅助',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'systemsoft') {
                        return '/423down/computer/systemsoft';
                    }
                }
            },
            {
                title: '系统必备',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'systemplus') {
                        return '/423down/computer/systemplus';
                    }
                }
            },
            {
                title: '安全软件',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'security') {
                        return '/423down/computer/security';
                    }
                }
            },
            {
                title: '补丁相关',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'patch') {
                        return '/423down/computer/patch';
                    }
                }
            },
            {
                title: '硬件相关',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'hardwork') {
                        return '/423down/computer/hardware';
                    }
                }
            },
            {
                title: 'windows 11',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'win11') {
                        return '/423down/os/win11';
                    }
                }
            },
            {
                title: 'windows 10',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'win10') {
                        return '/423down/os/win10';
                    }
                }
            },
            {
                title: 'windows 7',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'win7') {
                        return '/423down/os/win7';
                    }
                }
            },
            {
                title: 'windows xp',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'winxp') {
                        return '/423down/os/winxp';
                    }
                }
            },
            {
                title: 'windows pe',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                target: (params) => {
                    if (params.type === 'winpe') {
                        return '/423down/os/winpe';
                    }
                }
            }
        ]
    },
}

New rule

{
    '423down.com': {
        _name: '423down',
        www: [
            {
                title: '分类',
                docs: 'https://docs.rsshub.app/bbs.html#_423down',
                source: '/:type',
                route: '/423down/:category_and_type', // original route is `/423down/:category/:type`
                matcher: {
                    category_and_type: [
                        { action: 'path', key: 'type', stopIfEmpty: false },
                        {
                            action: 'remap',
                            map: {
                                '': { value: 'index/all', overrideTitle: '首页' },
                                apk: { value: 'android/apk', overrideTitle: '安卓软件' },
                                zd423: { value: 'computer/originalsoft', overrideTitle: '原创软件' },
                                multimedia: { value: 'computer/multimedia', overrideTitle: '媒体播放' },
                                browser: { value: 'computer/browser', overrideTitle: '网页浏览' },
                                image: { value: 'computer/image', overrideTitle: '图形图像' },
                                im: { value: 'computer/im', overrideTitle: '聊天软件' },
                                work: { value: 'computer/work', overrideTitle: '办公软件' },
                                down: { value: 'computer/down', overrideTitle: '上传下载' },
                                systemsoft: { value: 'computer/systemsoft', overrideTitle: '系统辅助' },
                                systemplus: { value: 'computer/systemplus', overrideTitle: '系统必备' },
                                security: { value: 'computer/security', overrideTitle: '安全软件' },
                                patch: { value: 'computer/patch', overrideTitle: '补丁相关' },
                                hardwork: { value: 'computer/hardware', overrideTitle: '硬件相关' },
                                win11: { value: 'os/win11', overrideTitle: 'windows 11' },
                                win10: { value: 'os/win10', overrideTitle: 'windows 10' },
                                win7: { value: 'os/win7', overrideTitle: 'windows 7' },
                                winxp: { value: 'os/winxp', overrideTitle: 'windows xp' },
                                winpe: { value: 'os/winpe', overrideTitle: 'windows pe' },
                            }
                        }
                    ]
                }
            }
        ]
    },
}

More Information

Here is a filtered radar-rules.js containing only those rules using target function (based on https://github.com/DIYgod/RSSHub/blob/780031a5e3e6a43691bb877e759a152cc4c8779f/build/radar-rules.js): radar-rules-filtered.tar.gz (.js ext name is banned by GitHub, so I compressed it)

You can filter it by yourself, here's some vim magic:

:%s/\v\{\_[^{}()]+title:\_[^{}()]+},?//g
:%s/\v['"][^"']+['"]: ?\{\_[^{}]+},?//g
:%s/\v\w+: ?\[\_\s*],?//g
:%g/^\s*$/d

cc @NeverBehave @Cay-Zhang @LeetaoGoooo @TonyRL

@Cay-Zhang
Copy link

Please allow me to shine some light on the issue, especially in relation to RSSBud.

These rules should have been compatible with RSSBud/RSSAid, but with the current specification, they can only use a target function which is incompatible with RSSBud/RSSAid. The new specification should change these embarrassing situations.

Just to clarify, RSSBud gained the ability to evaluate target functions in its v1.2 release (1y ago), and as far as I know, RSSAid got the same feature around that time too.

RSSBud uses JavaScriptCore to load and evaluate rules dynamically. JavaScriptCore is extremely limited (especially on iOS, where safety is a top priority): merely the bare bones of JavaScript are accessible so I had to "polyfill" URL and DOM myself. In my opinion, it would be rather safe and future-proof to provide users the ability to load rules from a remote source given sufficient warning.

Based on the above reasoning, RSSBud will continue supporting target functions. However, I fully understand that it might be an unavoidable trend for major browsers to prohibit remote code execution so I am willing to integrate any changes made to the specification into RSSBud.

In addition, I'm planning to expand the specification to allow targeting of non-RSSHub feeds. Feel free to check it out.

@Rongronggg9
Copy link
Contributor Author

@Cay-Zhang
Happy to see and thanks for your "light"!

Sorry for my unawareness that RSSBud/RSSAid has gained the ability to evaluate target functions. I am neither an RSSBud user nor an RSSAid user so I misunderstood the documentation of RSSHub, which says that "RSSBud is not a browser extension, it only fetches and analyzes the URL of a website". Apologize for my misunderstanding again.
I consider if we figured out a new specification to describe most parameter matchers, we could probably give a much weaker warning when a user requires to load a custom remote rule list. Safer is always better, though the major worry is still the prohibition on RCE, lol.

Whether to make target functions deprecated will be up to the result of the discussion. Personally, I consider that deprecation can hardly be avoided. On the one hand, rule authors might not be willing to write both an old-style target function and a new-style matcher simultaneously, but the latter might probably be prioritized so a rule with only the former might not be accepted. On the other hand, only Chromium users can fetch remote rules containing target functions, that's not so equitable to a certain extent (though that's not RSSHub-Radar's fault). Last but not least, if the goal is the removal of eval(), target functions will be sentenced to death sooner or later. However, probably target functions still have a long way to go before being cleared from the rule list, if not a lot of route maintainers updating their rules according to the new specification.

I've checked RSSBudRules. Extracting official feeds is a nice idea, personally, I am glad to see such a feature, but we need more approval to get it in the new specification. BTW, I've updated the drafting specification (especially adding raw URL as an input choice) to make it possible to be compatible with your current rules. However, targetType is needed to make it fully compatible (as mentioned, need more approval to get it in the new specification). Would you please check the draft and give some feedback? Appreciate it.

@NeverBehave
Copy link
Contributor

NeverBehave commented Apr 20, 2022

Just wondering, what if we embed the rule into each release? Could we save the hassle doing these workaround? For the new route v2 we could definitely generate and pack function.

@Rongronggg9
Copy link
Contributor Author

Rongronggg9 commented Apr 20, 2022

Just wondering, what if we embed the rule into each release? Could we save the hassle doing these workaround? For the new route v2 we could definitely generate and pack function.

If this is a solution, shouldn't RSSHub-Radar have already been actively releasing minor versions along with the latest rules since v1.6.0? I feel sorry to mention that until #667, the default rule list was even not imported properly and was unusable, and no new version was released to include this PR so far. What's more, what about debugging new rules? Forcing contributors to install Chromium before they are able to debug, or forcing them to apply some patches and build their own version? I think neither is a good idea.
I don't think route standard v2 could do any to change the embarrassing situations on RSSHub-Radar. Radar rules from v2 routes are just aggregated together, they show nothing different from the ones from v1 routes.

off-topic

I, again, feel sorry to mention that whether remote rules along with local-customized rules are enabled is totally up to UA; however, whether the update button to display is up to a synced variable. That is, if someone builds their own add-on without patches and installs, that variable will be set to false, applying patches then installing again will not update that variable to enable the update button. If you do clean your synced variables and build the master branch and install it, you will face exactly the same embarrassing situation (#690). I wasted a lot of time figuring out this.

Updating a public add-on needs reviews from the add-on store which consumes some time. Now that RSSHub-Radar has 4 distributions on different add-on stores, of which 3 (Chrome, Edge, Firefox) are owned by @DIYgod, 1 (Safari) is owned by @Edd-Gao, so the problem is, would they willing to manually update their distributions often or setup some CI to automatically push updates to the add-on stores (is each add-on stores has such an API?)? I fully understand they are busy, so I draft this new specification to completely solve these problems (it hurts me that you call my work workaround, shouldn't the usage of eval() or embed rules into each release be workarounds and dirty-hacks?). Just to mention, the distribution on Edge Addons is still on v1.6.0, while the latest version is v1.7.0.

@NeverBehave
Copy link
Contributor

NeverBehave commented Apr 20, 2022

Gotcha, do you have a pipeline of this kind so that I may take a look and work with @DIYgod working on auto publish? We may have a variance for remote update but I would like to focus on having a working pipeline first.
The idea of target function and the ecosystem around it is much more powerful that the draft and I think if we could solve this pipeline issue we may keep it. Really appreciate your effort though!

@Rongronggg9
Copy link
Contributor Author

Rongronggg9 commented Apr 20, 2022

Still, I must argue that auto publishing would not change the fact that Firefox and Safari users are unable to debug their new rules. Honestly speaking, if not for debugging new rules, there was no Chromium on my PC.

I may help set up a pipeline to automatically publish to AMO. I do not have and am not willing to register Chrome Web Store / Edge Add-ons accounts, thus, I have nothing to help with the distributions on these two stores. What's more, it seems that there is no such API for publishing Safari extensions, thus, the distribution for Safari is still unable to get the latest rules.

Let us figure out what an embarrassing situation to which we are facing. Basically, depends on the goal and the browser, here's what we need:

remove the usage of eval()? latest rules? rules debugging?
Firefox auto-publishing pipeline / new specification of radar rules self-hosted add-on / auto-publishing pipeline / new specification of radar rules self-hosted add-on / new specification of radar rules
Safari new specification of radar rules new specification of radar rules new specification of radar rules
Chrome auto-publishing pipeline / new specification of radar rules
Edge auto-publishing pipeline / new specification of radar rules

@NeverBehave
Copy link
Contributor

NeverBehave commented Apr 22, 2022

For official plugins update I don't think it would be a great idea to do any remote fetch. A monthly release should be good enough. We could also have an emergency version disable API if any malicious script was published so we could stop any execution until users have the extension updated.

For debugging purpose, could we set up scripts like tampermonkey? We pack our rules in form of JS instead of JSON and user could override this file for testing purposes, I believe this debug version could also be publish via github release instead of official channel as only developer or advance user could make use of it.

@Rongronggg9
Copy link
Contributor Author

Rongronggg9 commented Apr 22, 2022

For official plugins update I don't think it would be a great idea to do any remote fetch. A monthly release should be good enough.

I consider there is nothing different between fetching remote rules and monthly releasing as long as the usage of eval() is removed. I mean, no additional audit would happen between rule approving and add-on releasing.

We could also have an emergency version disable API if any malicious script was published so we could stop any execution until users have the extension updated.

I consider it relies on users to open issues to report malicious scripts. Still, nothing different between fetching remote rules and monthly releasing.

For debugging purpose, could we set up scripts like tampermonkey?

The answer is no. User scripts are registered and executed on webpages in situ. That's much more dangerous than loading rules using eval() in a sandbox (current behavior). And what's more, a bare rule JS cannot be directly executed on a webpage and another much more difficult problem is how to get its output.
If you would like to learn about the mechanism of how a user script works, turn to MDN.

user could override this file for testing purposes

If what you exactly mean is "override file", that's impossible unless users rebuild/repack their own add-on each time they modify it. If it is "override rules", eval() is still needed as long as rules are in form of JS. That's the only way RSSHub-Radar could load target functions since the user-script way is impracticable.

I believe this debug version could also be publish via github release

Chromium users can get their sideloaded extensions preserved after a browser relaunch, but that's not true for Firefox and Safari users. The good news is that a self-hosted (a.k.a. not-listed-on-AMO) Firefox add-on will help, while the bad news is Safari does not provide such a solution.

@NeverBehave
Copy link
Contributor

NeverBehave commented Apr 22, 2022

So if I understand this correctly:

Extensions could load remote script(js) without any limitation? If that's true we could simply publish rules online and load it anyway. If not true how do you update without publishing it officially or packed it? If we embed the rules into the extension why do we need eval? They are just part of the extension and we don't need eval to execute it -- just run it as part of the function already there. I don't consider there are auditing anyway but it is the process. Remote fetch open more attack surface and I would rather avoid that in official release. Developer extension -- please take it away with all your ideas.

Also,even though security issues might be reported by third-party, we are already choosing the way of doing it "officially". Then we need to have some thought over these bad situation and how we could stop them. The issue with monthly release is that user may not be able to have latest update and keep the malicious script running for a month is not a responsible behavior imo. At least we have to do something to stop such behavior at the time we know it.

@Rongronggg9
Copy link
Contributor Author

Extensions could load remote script(js) without any limitation?

Nope. As I have already described, such remote scripts need to be registered and executed on certain web pages (properly speaking, URL patterns) in situ. So the limitations are:

  1. There must be an opening web page, which matches the URL pattern.
  2. The script will only be executed once the runAt condition is met, which means that the web page must be loaded successfully first.
  3. The URL pattern must not match any page of an add-on or any internal page of the browser.
  4. The remote script must be executed on the web page in situ (properly speaking, it is a sandbox in which window and document are exposed in particular, and modifications on them will influence the web page itself).
  5. That triggers the execution of the script is the browser, not the add-on.
  6. If the add-on needs to communicate with the user script, there is no side-channel but only the web page itself.
  7. Such a script must be registered before the webpage is loaded. If registered after that, a reload is needed.
  8. Of what the add-on can take control is only (un-)registering.
  9. If such a script is loaded from an URL pre-built into the add-on, probably it is unable to pass the review since it is no longer a "user" script.
  10. User script manager add-ons face stricter reviews, of course. If you create an add-on that is not a user script manager but allows users to load remote scripts, reviews may be even much stricter since it is definitely not the intended usage of the user script API and probably just abusing.

If that's true we could simply publish rules online and load it anyway.

Publishing rules isn't enough since they cannot be executed directly on web pages. The script must modify something on the web page, either window or document to communicate with RSSHub-Radar. That's not acceptable since it is even more dangerous than the current behavior.

If not true how do you update without publishing it officially or packed it?

It is the user script manager add-on that re-registers the user script.

If we embed the rules into the extension why do we need eval? They are just part of the extension and we don't need eval to execute it -- just run it as part of the function already there.

When debugging new rules or fetching remote rules, "debug version published via GitHub release" cannot get rid of it. I am not so sure why you asked me such questions since my previous reply has described the precondition (If it is "override rules") clearly and that words are exactly commenting debug versions.

Remote fetch open more attack surface and I would rather avoid that in official release.

I agree that that's a significant point. However, that's true only if target function is something that could never be dropped. I didn't see any attack surface if the usage of eval() is removed and rules can use only limited operations (just like the new specification drafted in this issue).

Also,even though security issues might be reported by third-party, we are already choosing the way of doing it "officially".

So, do you mean that there will be several developers who are auditing rules day by day? If not, no additional audit would happen between rule approving and add-on releasing. Thus, any security issues, not "might be", but would only be reported by third-party (users) before any official actions could be done. Even if there is an official API to remotely disable the extension to avoid further damage, it still relies on users to discover and report security issues, doesn't it? So there is nothing different between fetching remote rules and monthly releasing as long as the usage of eval() is removed. That's exactly what I mean in my previous reply. It is just a repeat in case you misunderstood it.

Then we need to have some thought over these bad situation and how we could stop them. The issue with monthly release is that user may not be able to have latest update and keep the malicious script running for a month is not a responsible behavior imo. At least we have to do something to stop such behavior at the time we know it.

In the case of monthly releases, if a security issue is reported and such API effectively disables users' extensions, they will be unable to use their extension until a newer version is released. In the case of loading remote rules, just a forced-rule-updating API is enough to protect users and can avoid such long downtime. Even if there's no such API, RSSHub-Radar already has forced updates per 5h.


To be honest, I am not really opposed to target functions so that's OK to deny my work and never adopt this draft. But we would better figure out the answer to this question:

  • Would most Chromium users be glad to see remote rules were disabled and they could only rely on monthly releases, even if they knew that's for security concerns?

At least we are depriving them of something they already have.

The reason why I drafted this issue is that it is the only solution to make all Chromium, Safari, and Firefox users happy, and meanwhile, is secure and does not require the add-on publisher to publish new versions frequently.

@NeverBehave
Copy link
Contributor

NeverBehave commented Apr 22, 2022

I believe the current proposal 1)cannot replace the target function and 2)breaking other parts of the ecosystem. I see the point where most of the rules won't make use of the power and we are giving too much, but it is also important to realize that currently, we don't have enough hands to get the adoption, especially without support from other systems. That's why I am trying to find an easy way to get around it:

  1. Normal user will be okay with the monthly release and we could make it faster if we want to (could check marketplace rules for details). Our code review at least have some cover but unlimited rule fetch is not what we want on the official extensions. For self-hosted they could do it manually of course, but what if we have plugin updates, users have to update manually. IMO this is bad experience. Even worse, how would you notify normal user with security updates? We could rollout new version and rely on browser update if we make it "officially" on the marketplace.

  2. For auditing part, I believe it is not really the case here. tbh I am just trying to get away from it. What I am thinking is that:

    1. For each pull request touches rules, we should have more or less audited the target function, and we are okay with it. Just like the RSSHub itself. We may not have reviewed everything but at least we try to avoid code execution from outside with default settings.
    2. If we cannot do remote rule fetch and execution, lets just keep what we have, make the rules part of the extension and some ways to prevent vulnerable version being used.

And for those power user who don't satisfied with monthly update and have enough security knowledge, they still have the option to use the developer version, where we could simply expose the rules and use eval. I don't want to give normal user too much power when they have no idea what they are doing (like pasting stuff in developer console, raise the bar by asking manually install from github imo is a good start). Like I said, unless we have a safe way to cover the case where website content is necessary from RSS link generation, we would rather stay what we have for now.

Still, this proposal is a good start for anyone who wanna do the sandbox. I believe it is the first time someone work this issue with some specs, and I really appreciate it.

@Rongronggg9
Copy link
Contributor Author

we could make it faster if we want to

As I have already mentioned, there seems to be no add-on publishing API for Safari, so manual actions are needed. If the releasing interval is up to the publisher's manual actions instead of automatical pipelines, it is considered unreliable.

For self-hosted they could do it manually of course, but what if we have plugin updates, users have to update manually. IMO this is bad experience.

Just turn to the doc. Self-hosted addons are able to update automatically as long as the manifest.json contains an update_url which points to a JSON containing necessary info. That JSON could be easily hosted on GitHub Pages and the add-on's signed xpi file could be easily distributed via GitHub Release with a permanent URL or via a repository with raw.githubusercontent.com permanent URL.

Even worse, how would you notify normal user with security updates?

That's up to how the add-on fetches remote warnings, not how it is distributed.

We could rollout new version and rely on browser update if we make it "officially" on the marketplace.

So do self-hosted ones as I have already explained.

For auditing part, I believe it is not really the case here.

I am not arguing on the topic of auditing. What I had said were just examples to show there would be nothing different between remote fetching and monthly releasing as long as the usage of eval() is removed. But since you are radically opposed to deprecating the target function, those examples become nothing. So you don't need to prove anything about this topic since I can accept your opinion on the necessity of target functions.

And for those power user who don't satisfied with monthly update and have enough security knowledge, they still have the option to use the developer version, where we could simply expose the rules and use eval.

They have the option, but Firefox and Safari effectively make this option nothing because sideloaded add-ons are uninstalled after a browser relaunch. So at least for Firefox, if we still need the official add-on to be listed on AMO, the developer version should be distributed as a self-hosted addon. As for Safari, I could just support the opinion of GNU (just joking, lol).
Meanwhile, I am not sure if Chrome and Edge uninstall sideloaded unpacked addons after a relaunch since I merely tested Chromium. At least packed sideloaded add-ons seem unable to survive.


That's pretty OK not to adopt my draft. Willing to comment on my work is already a kind of treasure. I am always here to see if there is something I may help with.

@NeverBehave
Copy link
Contributor

NeverBehave commented Apr 23, 2022

Okay, so for self-distributed extension, firefox could do update, good to know. I get the idea that since we are doing something new, we should consider everything as perfect as possible. As I said, currently, it does not cover some important cases we are using right now and also break the ecosystem without an alternative, so instead of limits by a preset of rules, could we just make target function won't be able to do anything but compute a result from the input for URL generation.

From the project's perspective this is the next step I am looking for (While I am not sure if this is what we already have), and as a stricter set of rules I am more than happy to see if community could come up with better ideas to cover the cases here. So the next step:

  • Pack rules into js instead of eval
  • Self-distribute firefox extension
  • Safari user may use third-party extension for now, as main focus will be chrome and firefox in the near future (until we have time implement partial solution or better solution evolves).

If that's sound good to you, I could go ahead and move onto #690

@Rongronggg9
Copy link
Contributor Author

Rongronggg9 commented Apr 23, 2022

Sounds good.


I consider there is another simple solution to partially enable remote rules and local debugging. That is, only in such two use cases, deny target functions to avoid eval(). We could distribute a JSON-version rule list in which target functions are filtered out, thus, fetching and parsing such a rule list is totally safe. Meanwhile, the JS-version, without target functions filtered out, is packed into the add-on itself. Every time a rule update is performed, these two lists get merged together. This is pretty significant since most rules do not use target functions, and even for those new rules that use target functions, just dropping their target functions in remote rules isn't a big deal since RSSHub-Radar could still prompt users to read to the doc and finally, next add-on release will come with the target functions of new rules.

If so, the official Firefox add-on does not need to be self-distributed, only the developer version needs. And could greatly improve the user experience of the Safari add-on.

What's your opinion about that?

@dvikan
Copy link

dvikan commented May 1, 2022

My two cents: please produce a json file so that I can use it to resolve a url to a rsshub route. Example:

https://gab.com/:user/ => /gab/user/:user

I need this because I am making an app which resolves urls to feed urls.

My app is not written in js and it does not run in a browser. I need json.

@NeverBehave
Copy link
Contributor

@Rongronggg9 I have your changes merged, sorry I don't have access to this repo and diygod seems still busy right now. I will nudge again later.

cc @dvikan https://github.com/DIYgod/RSSHub/blob/gh-pages/build/radar-rules.json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants