fix(nextly): a Single read hides a denied field from its hooks - #1800
Conversation
The Single read ran afterRead before field read access, so a denied field's own hook ran with its value and a hook on an allowed sibling could read it. It now runs the two passes a collection read runs, one before the hooks and one after, sharing a redactions store; the document-level rule judges a snapshot with the removed values restored as evidence, so a rule that inspects a denied field still sees it.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
|
Warning Review limit reachedNext included review available in 8 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Docs note: #1777 documents the Single read's current ordering (hooks first, redaction after) in docs/configuration/fields.mdx. That sentence lives only on #1777's branch, so it cannot be updated here without carrying that PR. Whichever of the two merges second, a one-line follow-up makes the docs say both read paths redact before their field hooks; I will open it then. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddaaaf5918
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ook may replace A hook that returns a fresh container leaves the redaction store's row keys pointing at objects the document no longer holds, so the judge lost the evidence for a denied nested value. The removed values are captured by path right after the first pass and put back onto the judge's detached copy by path; the response object is never touched.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 667d1a694e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92b832d151
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… ids, and into the second pass Capture reads the rows of a container the pass removed whole from the removed value, so a denied child's evidence survives its parent's removal. Rows sharing an id get an occurrence suffix. The second field-access pass restores the path-keyed evidence first and removes again what it put back, so a container a hook rebuilt is judged against what the first pass removed from it.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 249b0a3fea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…s a rule's writes A row the capture saw is restored from the path it had then, so a hook that reorders id-less rows does not move their evidence; only a repeater rebuilt row by row and reordered is left to position. Evidence values are detached on capture and on every restore, so what a rule writes into its argument never reaches the pass after it.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b399573487
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
@nextlyhq/adapter-drizzle
@nextlyhq/adapter-mysql
@nextlyhq/adapter-postgres
@nextlyhq/adapter-sqlite
@nextlyhq/admin
@nextlyhq/admin-css
@nextlyhq/blocks-engine
@nextlyhq/blocks-react
@nextlyhq/builder
create-nextly-app
@nextlyhq/eslint-plugin
nextly
@nextlyhq/plugin-form-builder
@nextlyhq/plugin-page-builder
@nextlyhq/plugin-sdk
@nextlyhq/plugin-seo
@nextlyhq/storage-s3
@nextlyhq/storage-uploadthing
@nextlyhq/storage-vercel-blob
@nextlyhq/ui
commit: |
…th one grants lookup The pass records what it removes by path while the rows it parsed still exist, so a container it serialised back to a JSON string keeps its nested evidence; the walk taken afterwards is gone. Row segments encode the id, so an id that looks like an occurrence marker cannot collide. Both passes over a Single share one grants resolver, so the caller's roles and permissions are read once and judged with one authority.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 375c15d7c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ts-before-field-hooks
…p the evidence layer Access decides on what is stored; hooks shape what is returned. The custom document-level read rule is judged on the assembled stored document before any afterRead hook and before field access, so it sees denied values without reconstructing them onto whatever the hooks returned. The path-keyed evidence machinery that reconstruction needed is removed. The two field-access passes around the field hooks stay, sharing one redactions store and one grants resolver, as on the collection read.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 274db505d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ts-before-field-hooks # Conflicts: # packages/nextly/src/domains/collections/__tests__/_fixtures/single-read-rule.ts # packages/nextly/src/domains/singles/services/single-query-service.ts
|
Merged #1659 ( So the merge keeps the half that still has a mechanism and drops the half that does not:
The founder's decision (access decides on what is stored, hooks shape what is returned) is unchanged and still the reason the two field-access passes sit where they do; there is simply no document-level rule left for it to govern here. Verification: the three remaining tests pass; against @codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
What
A Single read now applies field-level
access.readrules before its field-levelafterReadhooks and again after them, sharing one redactions store and one grants resolver: the two passes a collection read has always run.The defect
single-query-service.tsranrunFieldHookson the assembled document andapplyFieldReadAccessonly afterwards. So a denied field's ownafterReadran and was handed its value, and a hook on an allowed sibling could read the denied value fromdataand copy it onto its own field, where the later redaction would not look. The response was redacted, but app code had already seen the value. The collection read had redacted before and after its hooks since it existed. Surfaced while documenting the hook order for #1777.The principle
Access decides on what is stored; hooks shape what is returned.
The first pass decides every field on the stored document. The second pass, after the hooks, exists to catch a denied key a hook put back, which it does by name: the key's own rule runs on the value the hook wrote. It does not re-judge a value the first pass allowed against a condition a hook changed, which would be a hook influencing access; the collection read has always behaved this way. This is how Payload orders read access and
afterRead.Both passes share one
ReadAccessRedactionsstore and onereadAccessGrantslookup, so the caller's roles and permissions are read once and both passes judge with one authority.The document-rule half, and why it is gone
This PR also moved a Single's custom document-level read rule to before the hooks (founder decision, option A, after five review rounds showed that reconstructing redacted values for a post-hook rule could not be done soundly).
While this PR was open, #1659 (
decide access with one mechanism, not two) removed the stored access-rule engine outright:AccessControlService, theStoredAccessRuletypes, and theaccess_rulescolumn ondynamic_collectionsanddynamic_singles. A Single's document-level rule was that engine's only authored surface, so there is no longer a rule for this step to judge. The merge ofmaintherefore drops that half: the 7.5 judgement, the two tests that wroteaccess_rulesdirectly, and its sentence in the docs and the changeset. The fixture andcustom-read-constraint.integration.test.tswere deleted by #1659 itself.The principle above is unchanged and is still why the two field-access passes sit where they do.
Tests
single-read-redacts-before-field-hooks.integration.test.ts, throughfindSingle: a denied field's hook does not run and its sibling's hook seesundefined; a hook that reintroduces the denied value is stripped; the positive control, a user the rule allows, sees the hook run with the value. A unit case pins one grants lookup across both passes.Control run against
origin/main'ssingle-query-service.ts: the mechanism test fails. All 15 Singles integration suites pass and the package builds.Patch changeset, every package.
Closes the ledger's
single-read-redacts-after-field-hooks.