feat: add graphile-sql-expression-validator plugin #543
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat: add graphile-sql-expression-validator plugin
Summary
Adds a new Graphile plugin that validates SQL expressions at the GraphQL layer before they reach the database. The plugin parses SQL expressions to AST, validates against allowed node types and functions, and deparses back to canonical SQL text.
This is part 2 of a 2-part change for constructive-io/constructive-planning#476. Part 1 is the schema change in constructive-db (PR #178) that adds the
default_value_astcolumn.Key features:
@sqlExpressionsmart comment*_astcolumns (e.g.,default_value→default_value_ast)Updates since last revision
Latest: Tests added to CI workflow
graphile/graphile-sql-expression-validatorto the CI test matrix in.github/workflows/run-tests.yamlPrevious updates:
graphile-settingsto avoid loadingpgsql-parser/pgsql-deparserdependencies in servers that don't need SQL expression validation. Apps should add the plugin directly:@rawSqlAstFieldsmart comment: Override the default<column>_astnaming conventionallowOwnedSchemasoption: Auto-allows schema-qualified functions from schemas owned by the current databasegetAdditionalAllowedSchemashook: Custom async function for dynamic schema resolutionpgsql-parserandpgsql-deparserupdated to v17.xReview & Testing Checklist for Human
This is a medium-risk PR - unit tests exist but integration testing is still needed:
GraphQLObjectType:fields:fieldhook withisRootMutationandpgFieldIntrospectionmay not correctly intercept mutations. Test with actual GraphQL mutations againstcollections_public.fieldallowOwnedSchemasDB query: The query silently returns empty array on error. Verify the GraphQL role has SELECT access tocollections_public.schemaand thatjwt_private.current_database_id()returns correctlyALLOWED_NODE_TYPESset is defined but onlyFORBIDDEN_NODE_TYPESis checked - confirm this is intended behaviorRecommended test plan:
SqlExpressionValidatorPluginto your app's PostGraphile pluginsdefaultValue: "uuid_generate_v4()"default_value_astcolumn is populated with the parsed ASTdefault_valuetext is the canonical deparsed formdefaultValue: "SELECT * FROM users"and verify it's rejectedNotes
graphile-settings- it must be explicitly added to apps that need itallowedSchemasoption defaults to empty, meaning schema-qualified function calls will be rejected unless explicitly configured orallowOwnedSchemasis enabledallowOwnedSchemasfeature fails silently (returns empty array) if the DB query fails - this is intentional for fail-closed securityLink to Devin run: https://app.devin.ai/sessions/7655646c517d4c74b0f7b827a102d494
Requested by: Dan Lynch (@pyramation)