nucypher / taco-web

🌮 A TypeScript client for TACo (Threshold Access Control)
https://docs.threshold.network/app-development/threshold-access-control-tac
GNU General Public License v3.0
14 stars 22 forks source link

Syntactic validation of JSONPath queries while creating conditions #559

Closed KPrasch closed 2 weeks ago

KPrasch commented 1 month ago

Originally identified here https://github.com/nucypher/taco-web/pull/550/files#r1696888086

derekpierre commented 1 month ago

zod.refine and a jsonpath library will be your friends.

Analogous to the SIWE message string validation - https://github.com/nucypher/taco-web/blob/main/packages/taco-auth/src/providers/eip4361.ts#L20, https://github.com/nucypher/taco-web/blob/main/packages/taco-auth/src/providers/eip4361.ts#L9

KPrasch commented 1 month ago

After some experimentation it appears that the grammar parsing of jsonpath-plus does not enforce syntactically invalid expressions.

Even the expression ...store.. ..book[**** is parsed (incorrectly) into the array Array(8) ["", .., store, .., "", .., book, ****]. This same expression will not be usable by nodes at decryption-time.

Ultimately this is both a developer experience (early error detection/prevention) and a compatibility concern (syntactically invalid expressions will not be rejected by taco nodes).

Also see this open issue from 2022 https://github.com/JSONPath-Plus/JSONPath/issues/134

KPrasch commented 1 month ago

It also appears that neither the jsonpath or jsonpath-plus repositories are being actively maintained. (https://github.com/JSONPath-Plus/JSONPath)

Screenshot 2024-07-31 at 14 42 17

This messaging also appears to be reiterated in this comment on May 15, 2024 https://github.com/JSONPath-Plus/JSONPath/issues/173#issuecomment-2112352687

As mentioned on the main page of the README, I am not actively maintaining this project, though I am accepting PRs. Feel free to submit a well-documented PR if you find an issue and your question is not answered by the README.

This may not be an issue if the implementation is stable enough though.

derekpierre commented 2 weeks ago

Closed via #561