feat: add FieldType/FieldDefault AST validators with schema-aware allowlists#1153
Merged
Merged
Conversation
…owlists - Add FieldType and FieldDefault TypeScript types for structured JSONB models - Add converters: FieldType → TypeName AST, FieldDefault → expression AST - Add fieldTypeToSql/fieldDefaultToSql for canonical SQL text output - Add validateFieldType with identifier checks, forbidden types, schema allowlists - Add validateFieldDefault with recursive validation, schema-function maps, depth limits, and full expression AST validation via existing validator - Add allowedSchemaFunctions option for per-schema function allowlists - Fix: add missing 'isnull' to ALLOWED_NODE_TYPES (NULL literal support) - 155 new tests covering all type categories, real-world defaults, SQL injection prevention, and invalid structure rejection
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Adds a new
field-typesmodule tographile-sql-expression-validatorthat validates structured FieldType/FieldDefault JSONB models — the precursor to migrating the metaschema API from raw SQL strings to safe, structured objects (tracked in constructive-planning#786).New module (
src/field-types.ts):FieldType/FieldDefaultTypeScript interfaces matching the JSONB model designfieldTypeToAst,fieldDefaultToAst)fieldTypeToSql,fieldDefaultToSql)validateFieldType()— structural checks, forbidden OID-alias type rejection, schema allowlists, interval range validationvalidateFieldDefault()— structural checks → AST conversion → full expression validation via existingvalidateAst()machinery (function/schema allowlists, forbidden type casts, recursive depth limiting)allowedSchemaFunctionsoption for per-schema function allowlists (e.g.{ jwt_public: ['current_user_id', 'current_origin'] })Bug fix (
src/validator.ts):'isnull'toALLOWED_NODE_TYPES— was missing alongsideival,sval,fval,boolval,bsval, causing NULL literals ({ A_Const: { isnull: true } }) to be rejected by the existing validator.Test suite (
__tests__/field-types.test.ts):Review & Testing Checklist for Human
allowedSchemaFunctionsmerging logic (security-critical): Functions from the schema map are merged into the flatallowedFunctionslist before passing tovalidateAst(). This means e.g.current_user_idbecomes globally allowed by function name — the schema gate invalidateAstprovides the second check, but verify this can't create unintended cross-schema allowances.isnulladdition toALLOWED_NODE_TYPES: Confirmisnullonly appears as a property insideA_Constnodes in pgsql-parser and cannot be used as a standalone node type to bypass validation.geometry(Point, 4326),encode(gen_random_bytes(16), 'hex'),'{}'::jsonb) against actual pgsql-parser output to verify structural match.IDENTIFIER_PATTERN(/^[a-zA-Z_][a-zA-Z0-9_]*$/): Verify this isn't too restrictive for legitimate type/function identifiers (e.g. hyphenated names, unicode) or too permissive for security purposes.Suggested test plan: Run
npx jest --no-coverageingraphile/graphile-sql-expression-validator/and review the real-world defaults test block to confirm coverage of all patterns from constructive-db generators.Notes
Link to Devin session: https://app.devin.ai/sessions/1c5e3fccccaa40f1b327a07423ab947d
Requested by: @pyramation