feat(AF-940): query-shape routing condition and denied_shapes grants - #1101
Merged
Merged
Conversation
Detect joins, set operations, subqueries, CTEs, GROUP BY, HAVING, aggregates and window functions anywhere in a statement's AST, unioned across a transactional batch. SqlParseResult gains shapes + shapesAnalyzed; plugin engines keep the old constructors and report not-analyzed, so the new query_shape routing leaf fails closed on them.
Add denied_shapes to user and group datasource grants (V191), enforced at submission, the recurring recheck, break-glass, dry-run, request-group members and the access simulator with a 403. Denials union across grants, carry over a JIT replacement, and fail closed when the shape was not analyzed; engine-managed datasources refuse the field at grant time (422). Docs, website, help corpus and e2e specs cover both halves of #940.
Flag parenthesised joins, MERGE, function-argument subqueries such as ARRAY(SELECT ...), JSON_ARRAYAGG/OBJECTAGG and more vendor aggregates; check OTHER request-group members; an unknown stored shape denies all. Scope the e2e routing policy to its datasource; correct docs scope.
This was referenced Sep 25, 2026
Contributor
Contributor
Coverage Report for Frontend Coverage (frontend)
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
Contributor
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
1 participant
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.
Closes #940
Summary
Lets an admin constrain the shape of a query, not just its type and tables. Both deliverables from the issue, in the order it proposed:
query_shaperouting condition:{"type":"query_shape","any_of":["JOIN","SUBQUERY"]}matches a query that has any listed shape. It works with ESCALATE, REQUIRE_APPROVALS or AUTO_REJECT.denied_shapesgrant capability: a hard deny-list on user and group datasource grants (V191). A query with a denied shape is rejected with 403 before it is persisted.Shapes:
JOIN,UNION(any set operation),SUBQUERY,CTE,GROUP_BY,HAVING,AGGREGATE,WINDOW_FUNCTION. They are detected anywhere in the statement by a new JSqlParser AST walker (proxy.internal.QueryShapeDetector) and unioned across aBEGIN…COMMITbatch.Resolving the issue's open questions
SqlParseResultgains a third state,shapesAnalyzed, which mirrors the existingcolumnsAnalyzed. Plugin engines keep the pre-workflow: query-shape routing condition and grammar restriction #940 constructors and report "not analyzed". The deny-list fails closed on an unanalysed parse, and a non-emptydenied_shapesis refused at grant time for an engine-managed datasource (422DENIED_SHAPES_NOT_SUPPORTED). No engine re-pins are needed, because the old constructors are kept.AGGREGATEcovers the engines' built-in aggregate set, matched by name, plus anyWITHIN GROUPorFILTERed call. User-defined aggregates are not detected; this is documented.Where the issue's premise was wrong
Step 4 ("persist shape flags on the request") is not needed. Routing does not read persisted clause flags.
ConditionContextFactoryre-parsessql_textboth live and in the AF-630 replay, exactly ashas_wherealready does. So there is noquery_requestsmigration.Enforcement (denied_shapes)
DatasourcePermissionVerifier), break-glass, dry-run, request-group members (includingOTHERmembers such asMERGE), and the access simulator (denied_shapesdetail).Also in this PR
DatasourceControllernow answers an unreadable request body with 400VALIDATION_ERROR, where it used to return a 500 via the catch-all. This follows the existing per-controller precedent. It also changes an unknowndb_typeon create/update from 500 to 400.Docs / website
docs/03-data-model.md,docs/04-api-spec.md,docs/05-backend.md,docs/06-frontend.md,docs/07-security.mdwebsite/docs/configuration/{datasources,review-workflows,users-roles},website/docs/guides/team,website/sitemap.xml(all three dates bumped to 2026-09-25)README.md, and a regeneratedhelp-corpus/Tests
QueryShapeDetectorTest(56 cases),DeniedShapesTest, parser, routing-leaf, verifier, checker, merge, admin-service, handler, break-glass, request-group, dry-run and simulator tests, andDeniedShapesEnforcementIntegrationTest.deniedShapesutil, and settings-page tests.e2e/tests/datasource-denied-shapes.spec.ts(UI grant plus 403/202), and aquery_shapecase inadmin-routing-policies.spec.ts.Verification (local)
mvn verify -Pcoveragegreen: 10,489 tests, includingApplicationModulesTest,ApiPackageDependencyTestandMessagesParityTest. After the review fixes, the affected classes plus the architecture gates were re-run and are green.check-engine-pins.mjsverifies all 10 pins.af-verifierwas not dispatched: I ran every gate it would run myself (listed above).Review notes
The reviewers ran before the final fix commit. I fixed their blockers in
fix(AF-940): close shape-detector gaps found in review:FROM (a JOIN b …),MERGE, function-argument subqueries (ARRAY(SELECT …)),JSON_ARRAYAGG/JSON_OBJECTAGGand several vendor built-in aggregates.OTHERrequest-group members also skipped the check. All of these now have regression tests.shapes_analyzed. Also documented who is exempt (query-admins at submission, but not at break-glass), and stated that shapes do not make a grant read-only.query_shapepolicy is now scoped to its datasource with a run-unique priority, so it cannot auto-reject other specs' joins. Also added a group-row test.query_shapeeditor is not rendered in a component or e2e test. Its form mapping is unit-tested, and it mirrors theday_of_week/risk_leveleditors exactly.DeniedColumns.rejectedhas the sameOTHERshort-circuit, so request-groupOTHERmembers skip denied-column checks. I'll file it as a follow-up._one/_other, matching the existing deny-list keys.Screenshots