feat(safegres): read PostGraphile behaviors as a declared access-path signal - #1596
Closed
pyramation wants to merge 3 commits into
Closed
feat(safegres): read PostGraphile behaviors as a declared access-path signal#1596pyramation wants to merge 3 commits into
pyramation wants to merge 3 commits into
Conversation
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:
|
Contributor
|
Superseded: the exposure-planes work in #1598/main makes this the wrong shape. Behaviors should arrive through the |
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
paths.tssaid, in its own header, that the signal it needed was "whether the generated GraphQL surface still contains the field". PostGraphile v5 behaviors are that signal, and unlike everything else X1 could reach for they are a declaration rather than a measurement —reltuplesandidx_scanboth read zero on the ephemeral CI database safegres grades, at exactly the moment they would have to mean something.Read from
pg_description, not from a running Graphile instance or a project's metadata tables:@behaviorin a comment is the v5 convention, so this works on any Graphile database, the comment already holds the composed value, and no API needs to be up.Only an explicit denial counts. Presets grant most behaviors by default, so absence of
+listis not evidence — a scanner reading silence as denial recommends dropping an index a live API is using. Concretely,deniesAll(behavior, ['list', 'connection', 'single']): every one of the three, last fragment wins (-* +listgrantslist), scope paths matched on their final segment (-resource:connectiondeniesconnection). A table-level-selecthides every key of the table.This is signal-only — no finding, severity or score moves. Two places had to be held still deliberately:
checkUnindexedForeignKeysnow testsonWriteOncePointeragainst theconfig-recordsignal rather than the assessment, because a path that is both write-once shaped and declared hidden now reports the stronger assessment, anddemote/suppressmust keep meaning what they meant.report.perf.paths.declaredHiddenis reported in markdown/pretty and counted nowhere in the score.behavior-hiddenis also not a claim that the index is unwanted: the referential-integrity scan on a parentDELETEruns whatever the API exposes. That caveat is why the gating of X1/X7/X8 is a separate change, made after measuring this signal against constructive-db.Design and rollout: https://github.com/constructive-io/constructive-planning/issues/1356
Tests
__tests__/behaviors.test.tscovers tag extraction (including a tag followed by prose, and prose followed by a would-be tag), fragment parsing, and the denied/granted/undeclared three-way distinction.paths-behaviors.sql+ thedeclared surfaceblock inperf.test.tscover the catalog path: an explicit denial, a partial denial, a retracted denial, silence, a table-level-select, and a denial that loses to apolicy-read. 198 passed.Link to Devin session: https://app.devin.ai/sessions/ec06ef6eabae4872ae5ec3926f037c85
Requested by: @pyramation