feat(mergeAST): add mergeAST utility

[metrue]

This PR is adding utility of merge AST by simply migrating it from graphiql

As first step to resolve this issue https://github.com/graphql/graphql-js/issues/1428#issuecomment-2676341366

[github-actions[bot]]

@github-actions publish-pr-on-npm

@0042834736 The latest changes of this PR are available on NPM as graphql@16.10.0-canary.pr.4359.9fe5229b2fc30d3e5b07a6b00693fac42b649fdd Note: no gurantees provided so please use your own discretion.

Also you can depend on latest version built from this PR: npm install --save graphql@canary-pr-4359

[benjie]

One thing we should very much keep in mind with mergeAST is that it's a potential DOS vector. Having it in the frontend (i.e. GraphiQL) is one thing, but putting it in GraphQL.js means that we will need to have a lot stronger protection against things that could cause explosion of complexity or even infinite loops. If we could require that the document was already validated (and thus had passed all our checks) before calling into mergeAST then that would give us some confidence, but I imagine that people will want to mergeAST separately and we need to protect them from bad inputs. I'm not sure the solution to this, just wanted to surface the concern.

[metrue]

Thanks @benjie .

You're absolutely right: mergeAST introduces complexity risks, especially if used on unvalidated or hostile input. My intention with duplicated the utility from GraphiQL is to support use cases like: dynamically composing selection sets, merging query documents for internal middleware, or some other client side use cases, or Building tooling that operates on statically constructed or pre-validated ASTs. But yeah it might introduce reliability and security risks in server-side GraphQL environments.

Given that context, I'd be happy to explore ways to make the function safer or more clearly scoped, I am thinking,

• We can rename it to mergeValidatedAST, explicitly require/assume that inputs to mergeAST are the result of parse() followed by validate()
• Add shallow complexity safeguards (depth, cost)

It might not be able to prevent all the malicious input I think. Would love your thoughts on whether any of these sound like a reasonable direction. I'm happy to iterate and contribute safeguards in this PR or a follow-up depending on what fits best with the project’s expectations.

[benjie]

It might be best that you break it out into its own library for now. I'd want to carefully test (and probably rewrite) it before incorporating it into GraphQL.js, even with the rename.