Missing docs for LRU mercurius caching mechanism

[rodlukas]

Hi, we've recently found a bug in our app that was caused by misunderstanding of caching mechanism in mercurius. According to the preValidation hook docs (added in https://github.com/mercurius-js/mercurius/pull/680), the hook is not fired for cached queries:

By the time the preValidation hook triggers, the query string has been parsed into a GraphQL Document AST. The hook will not be triggered for cached queries, as they are not validated.

It might look like this refers to jit compilation (which is disabled by default), but apparently it refers to default LRU caching mechanism driven by cache mercurius option (https://github.com/mercurius-js/mercurius/blob/master/index.js#L55).

This mechanism is (probably) also mentioned in mercurius homepage https://mercurius.dev/#/:

Caching of query parsing and validation.

However, there is no additional docs refering to this LRU cache, which I've found a little bit confusing as this feature is active by default. Some users may also confuse this cache with mercurius-cache plugin or with persisted queries. This may lead to some serious security issues (ex. user auth logic can be placed in preValidation hook because jit: 0 - "so we're safe", but that's not true and allows the attacker to make in certain cases successful unauthorized requests).

I think it would be nice to add docs for this cache and explicitly explain it's logic, advantages and drawbacks.

[mcollina]

⚠️ NEVER SEND POTENTIAL SECURITY ISSUES PUBLICLY.

Writing in a public issue that there is a security risk creates Fear, Uncertainty, and Doubt. If the security issue is concrete, it also poses all the users at risk because the maintainers might not have time to act immediately. I've edited the title of your issue.

I noticed we did not add a SECURITY.md where we state how to contact us privately. I'll take an action to add a SECURITY.md file so that there are details on how to contact us privately.

If you find potential security issues in other repositories without a SECURITY.md file, please contact the maintainers directly.

---

It might look like this refers to jit compilation (which is disabled by default), but apparently it refers to default LRU caching mechanism driven by cache mercurius option (https://github.com/mercurius-js/mercurius/blob/master/index.js#L55).

This mechanism is (probably) also mentioned in mercurius homepage https://mercurius.dev/#/.

Indeed, that's precisely what that is. Good spot that we did not document this up correctly. I'll open a PR to document this.

This may lead to some serious security issues (ex. user auth logic can be placed in preValidation hook because jit: 0 - "so we're safe", but that's not true and allows the attacker to make in certain cases successful unauthorized requests).

I'm not sure where you concluded that placing the user auth logic should be placed in preValidation given the hook could be skipped.

This feature is essentially harmless and implementation detail. As much as your JavaScript runtime does not parse the code every time you execute a function, Mercurius does not parse the same query repeatedly. It'd be a waste of resources that you can safely avoid.

[rodlukas]

ok, thanks for your response and for clarification on security reporting, lesson learned :)

I'm not sure where you concluded that placing the user auth logic should be placed in preValidation given the hook could be skipped.

That's our mistake since we've somehow missed this sentence during a recent migration to mercurius and found it later. Afterwards, this sentence led us to place the auth logic to preParsing which is executed every time, but then we were curious what cache is responsible for this behaviour since the jit was disabled and it resulted in this issue with missing docs.

So, thanks for your work and for the PR! :)

[lbittencurt]

In my understanding this issue can be closed, because according to the authContext documentation of the mercurius-auth plugin it is self explanatory that the logic layer occurs in the preExecution hook instead of preValidation, so security prevails and there is no need to cache in preValidation keeping the sole responsibility of schema validation.

AuthContext https://github.com/mercurius-js/auth/blob/main/docs/auth-context.md#usage-with-authcontext

[mcollina]

thanks!