Regression: query not supported

[theofficialgman]

Are you experiencing an issue with...

shields.io

🐞 Description

https://img.shields.io/badge/dynamic/json?color=c51a4a&label=Pi-Apps&prefix=Prism%20Launcher%20&query=%24..%5B%3F%28%40.Name%3D%3D%22Minecraft%20Java%20Prism%20Launcher%22%29%5D.Version&url=https%3A%2F%2Fraw.githubusercontent.com%2FBotspot%2Fpi-apps-analytics%2Fmain%2Fpackage_data.json

which is decoded as

https://img.shields.io/badge/dynamic/json?color=c51a4a&label=Pi-Apps&prefix=Prism Launcher &query=$..[?(@.Name=="Minecraft Java Prism Launcher")].Version&url=https://raw.githubusercontent.com/Botspot/pi-apps-analytics/main/package_data.json

the above query was previously supported for over a year but stopped working sometime recently (in the past few months)

its purpose was to query https://raw.githubusercontent.com/Botspot/pi-apps-analytics/main/package_data.json for a Name with Minecraft Java Prism Launcher and return the corresponding Version string in the json.

The above query continues to work in https://jsonpath.com/ so this is a regression in shields or the JSONPath evaluator is uses.

🔗 Link to the badge

💡 Possible Solution

No response

[theofficialgman]

Regression caused by ec1b6c8

CC: @chris48s

linking #10553

No possible solution to my knowledge.
This change has made shield unusable for all my usecases which rely on parsing a JSON file that contains data for all projects for pi-apps.

[chris48s]

Hi. As noted in #10553 (which you've presumably read) we've disabled script expressions in jsonpath queries to resolve an important security issue. This does remove some functionality that was previously available (e.g: filters using [?(expr)] ), but it was a conscious decision. In some cases it may be possible to rewrite queries relying on script expressions. In some cases, it will not.

In terms of other ways to achieve this.. having a really quick look at https://github.com/Botspot/pi-apps-analytics/ it looks like it is a repo you contribute to and the json file you are parsing is the output of a GitHub workflow.

• One option would be to have the workflow also output JSON in a format that can be parsed using JSONPath selectors that don't involve script expressions (for example if https://raw.githubusercontent.com/Botspot/pi-apps-analytics/main/package_data.json were a key:value mapping instead of an array).
• Another option would be to have the workflow output JSON files conforming to the endpoint schema and switch to using the endpoint badge https://shields.io/badges/endpoint-badge

[theofficialgman]

The latter option is not a solution we can work with.

I assume for the first you mean something like this?

{
  "AbiWord": {
    "Version": "",
    "Description": "A free and open-source alternative to Microsoft Word.  ",
    "URL": "http://www.abisource.com/",
    "Architecture": "package",
    "Users": "5238"
  },
  "Alacritty Terminal": {
    "Version": "",
    "Description": "A fast, cross-platform, OpenGL terminal emulator",
    "URL": "https://github.com/alacritty/alacritty",
    "Architecture": "ARM32/ARM64",
    "Users": "3672"
  }
}

ofc this format is different and I'll have to update all dependent projects (or create another file in the repo for this format and also still update any dependent projects that use the shield to use it).

[chris48s]

I assume for the first you mean something like this?
...

Yep. If you made a data file like that, then you could query it using

• $.AbiWord.Version
• `$.['Alacritty Terminal'].Version``
• etc

which would still work as they don't use script expressions.

[PRO-2684]

Why not set eval to safe though? Thoroughly disabling scripting would kill so much possibilities... I can't even count the length of an Array and display that in the badge.

[chris48s]

I did have a look at the eval: 'safe' option when we originally made this change.

Here's a section from the JSONPath-Plus docs (I've made some small formatting changes for clarity):

eval (default: "safe") - Script evaluation method.

• safe: In browser, it will use a minimal scripting engine which doesn't use eval or Function and satisfies Content Security Policy. In NodeJS, it has no effect and is equivalent to native as scripting is safe there.
• native: uses the native scripting capabilities. i.e. unsafe eval or Function in browser and vm.Script in nodejs
  ...

so the first thing to note here is that safe does one thing in-browser, but a different thing in node. In node it is equivilent to native scripting. The JSONPath-Plus docs go on to imply that enabling scripting in Node JS is "safe" because it uses the vm.Script module. However, looking at the docs for vm.Script

https://nodejs.org/docs/latest-v20.x/api/vm.html#vm-executing-javascript

There's a big red warning at the top saying

The node:vm module is not a security mechanism. Do not use it to run untrusted code.

My reading of this is that we should not use eval: safe / vm.Script to run arbitrary expressions from untrusted users under node.

[PRO-2684]

I did have a look at the eval: 'safe' option when we originally made this change.

Here's a section from the JSONPath-Plus docs (I've made some small formatting changes for clarity):

eval (default: "safe") - Script evaluation method.

• safe: In browser, it will use a minimal scripting engine which doesn't use eval or Function and satisfies Content Security Policy. In NodeJS, it has no effect and is equivalent to native as scripting is safe there.
• native: uses the native scripting capabilities. i.e. unsafe eval or Function in browser and vm.Script in nodejs
  ...

so the first thing to note here is that safe does one thing in-browser, but a different thing in node. In node it is equivilent to native scripting. The JSONPath-Plus docs go on to imply that enabling scripting in Node JS is "safe" because it uses the vm.Script module. However, looking at the docs for vm.Script

https://nodejs.org/docs/latest-v20.x/api/vm.html#vm-executing-javascript

There's a big red warning at the top saying

The node:vm module is not a security mechanism. Do not use it to run untrusted code.

My reading of this is that we should not use eval: safe / vm.Script to run arbitrary expressions from untrusted users under node.

Oh I see your point. Since JSONPath-Plus is no longer actively maintained, maybe we could consider switch to another implementation? After a little digging around, I've found a Rust implementation that looks promising, which also has a Python binding.

[chris48s]

So there's a few things to dig into here.

Since JSONPath-Plus is no longer actively maintained,

JSONPath-Plus isn't actively developed, but they do accept PRs for bugfixes and address security issues. For example, since we switched to version 9.0.0 in September there have been 10 releases including multiple security-related patches https://github.com/JSONPath-Plus/JSONPath/blob/main/CHANGES.md . So although they describe it as "not actively maintained" it is not completely abandoned.

I've found a Rust implementation that looks promising, which also has a Python binding.

Shields is a javascript application. Very decisively a javascript application, in fact.

In the interests of keeping the deployment simple and the codebase maintainable by the current team, we don't really have the capacity to have bits of the application implemented in other languages.

maybe we could consider switch to...

Every JSONPath implementation has quirks and idiosyncrasies. I link to this a lot, but this table https://cburgmer.github.io/json-path-comparison/ does a very good job of demonstrating that there are many subtle implementation differences between every JSONPath implementation.

This means that switching JSONPath implementations entails some level of switching cost for users. The main thing under discussion in this particular thread is the fact that we disabled script expressions, and that is a big impact. However, when we ditched https://github.com/dchester/jsonpath and migrated to https://github.com/JSONPath-Plus/JSONPath we also switched from one quirky implementation to another quirky implementation. So as well as losing script expressions, there was also this other class of backwards-compatibility breaks that users had to deal with. To be honest, one of the reasons we stuck with https://github.com/dchester/jsonpath for so long was to maintain compatibility and avoid that backwards compatibility break. That turned out not to be the right decision from a security perspective, but hey-ho.

Anyway, my point is: Switching to another JSONPath implementation today will be another BC break for users. It isn't a zero cost switch.

That said..

One interesting thing that has happened in the world of JSONPath lately is RFC 9535 https://www.rfc-editor.org/rfc/rfc9535 which is an attempt to standardise JSONPath. In principle, if everyone implements that standard it will get rid of most of those quirks and make implementations much more interchangeable. However the reality is basically none of the widely-used libraries actually implement it yet. As part of this mitigation we did look at whether it would be possible to migrate to something that is RFC9535-compliant. However it is our assessment that the JavaScript community does not yet have a sufficiently mature RFC9535-compliant JSONPath implementation.

I think if and when we do swap out JSONPath-Plus for another implementation, I would only really want to so that if we can move to a RFC9535-compliant implementation instead of another legacy quirky implementation. There's a few advantages to that:

1. One of the goals of RFC9535 is to define script expressions in a way that they can be implemented without executing arbitrary JS. That would allow us to re-enable them in a secure way.
2. The payoff of that second backwards compatibility break would be to get us to a place where the API exposed to the user conforms to a standard and in theory we would have more flexibility to switch implementations on the backend in future with minimal impact on users.

However right now today the javascript ecosystem sadly does not yet have a mature RFC9535-compliant JSONPath implementation in my view. The project I have my eye on is https://github.com/jg-rp/json-p3 I think this is probably the most promising. It is not yet in a place where I would bet the farm on it, but it is something I'm monitoring as a possible replacement for JSONPath-Plus at some point in future. However, I have no defined timeline for this.