Skip to content

refactor(nextly): decide access with one mechanism, not two - #1659

Merged
faisal-rx merged 1 commit into
mainfrom
feat/nextly/unified-access
Sep 11, 2026
Merged

faisal-rx merged 1 commit into
mainfrom
feat/nextly/unified-access

Conversation

@faisal-rx

@faisal-rx faisal-rx commented Sep 9, 2026 •

Copy link
Copy Markdown
Collaborator

Access is decided by one mechanism, not two

Two independent systems answered "may this caller do this", and only one of them was configurable. The code-defined access on a collection's or Single's own config is reached from every transport. The stored rules were a second engine — five rule types evaluated against an access_rules JSON column — with no way to author them: collections had no code, UI or REST surface for the column at all, and a Single had only an undocumented REST field.

Two evaluators for one question is a divergence waiting to be found in the gap between them, and the half nobody could configure is the half nobody was checking. So the second engine is removed rather than reconciled.

AccessControlService and its five evaluators, the StoredAccessRule / CollectionAccessRules / SingleAccessRules types, the operation constants that only served them, and the access_rules column all go. AccessOperation and ACCESS_OPERATIONS stay — the RBAC gate is keyed on them. ExpressionValidator goes too, having had no importer at all.

Behaviour changes — two narrow, one WIDENS

Stored rules are no longer enforced, and that widens access for some installations. Any value still in access_rules is ignored from this release on. For most installations that changes nothing, because nothing could write the column. It does change behaviour for one that set a Single's rules through the undocumented REST field, or wrote the column directly — and the widest case is an owner-only read. That filter was produced by the stored evaluator alone, so a list, count or by-id read that used to return only the caller's own rows now returns every row the coarse gate admits for the collection. Before upgrading, such an installation should express any rule it still needs as code-defined access on the collection's or Single's config; that is the only place a rule is read now.

An installation whose database still holds rules is told so at boot. The warning states the whole decision order — super-admin bypass, then a code-defined rule for the operations it names, then database role grants for the rest — so nobody reads their role grants as ignored. On the initialized-boot path, beside the existing core-schema drift warning, Nextly introspects the two registry tables, and when the retired column is present and holds a value on any row, logger.warns the tables and row counts, says the rules are no longer enforced, and names nextly migrate / NEXTLY_ALLOW_CORE_DESTRUCTIVE=1 as the way to remove the column. A new database, or one that has dropped the column, never sees it. It is derived from the same live snapshot the core-schema drift check reads, in the same bounded pass (runCoreSchemaChecks, init/bounded-diagnostic.ts) — one introspection, one two-second bound — so a stalled or failing query is a debug line, never a reason the boot waits or fails.

An anonymous publish or unpublish is refused outright. It previously fell through to a rule-less public default unless an explicit stored publish rule denied it — so a collection with no rules, which was every collection, let an unauthenticated caller move a document into the published state.

Populating a relationship judges the TARGET COLLECTION for the caller, once per expansion rather than once per row, by evaluating that collection's own code-defined access.read with the same context a direct read builds. A session caller's rule sees their real roles and effective permissions (via checkAccess, which builds the canonical context); a scoped API key's rule sees the key's own grants. So every row of one target is admitted or withheld together, a rule that reaches out to a service is asked once per batch rather than once per row, and a row the caller can read directly does not vanish from a relationship pointing at it. The RBAC permission gate is deliberately not consulted there, for the reason it never was: requiring a read-<target> grant naming a collection the caller never asked for by name would empty the relationship for every caller whose grants do not list it. An absent rule admits; a throwing rule denies.

The anonymous decision lives in one place and every door asks it. main's #1704 made the collection gate consult a collection's own code-defined rule for a caller with no session (checkAnonymousCodeAccess, handed user: null). Two other doors were still answering that question the old way: the Single gate returned "allowed" at !user before any rule was read, and judgeTarget admitted every anonymous caller on a premise the rebase had made false. Both now ask the same question against the Single or the target and fall through to the public default only when no rule names the operation; an anonymous publish stays refused whatever a rule says.

A key's rule sees one permission spelling and the key's own roles on every path. main's #1772/#1783 landed the same design independently (ReadAccessCaller carries permissions and rulePermissions, code-access.ts as the shared leaf, scope roles first); this branch now sits on that and adds only the translation-worklist read going through readAccessCaller, and the relationship path using it.

A Single's working-draft overlay is read under the live document's access. The overlay replaces the assembled document and re-expands the snapshot's relationships; it used to build its own expansion context with neither enforcement flag, so a relationship in a pending draft exposed target rows the live relationship withheld. One builder (relatedRowAccess) now produces the context for both. The projection also reads the KEY's roles from its scope when it carries them (as apiKeyWriteAllowed does), so a role-deciding rule sees the same set on both doors. And the verdict is cached per target on the expansion (targetVerdicts), so a hasMany field fetching references concurrently asks its target once rather than once per reference — and it is decided BEFORE any target query: a refused caller costs no schema load, lifecycle resolution or select, and the referenced ids are recorded as withheld. What expansion mirrors is the target's code-defined rule only; it deliberately does not require the target's database read-<target> grant (the changeset says so).

The column

A new database never gets access_rules. One that already has it keeps it, and every schema entry point reports it rather than dropping it:

  • nextly migrate refuses and names it — drops column 'dynamic_collections.access_rules'
  • the dev-server reconcile blocks the drop and says so, without degrading
  • NEXTLY_ALLOW_CORE_DESTRUCTIVE=1 is how an operator removes it

Dropping a column that holds configured rules is their decision, not a side effect of upgrading. That machinery already existed; this change only makes access_rules become such a drop.

One upgrade is not supported and is called out rather than papered over. A database old enough to be missing core columns added since — a 0.45-era install — reconciles a drop and several adds on one table, which drizzle-kit pairs to ask whether the drop is a rename; pushSchema builds that resolver internally with no way to supply a hints handler, so it throws and the boot degrades to its additive-tables-only baseline. Such a database should run nextly migrate before taking this release. upgrade-sim-045 is skipped for that reason, with the fix that restores it named in its header.

Not a breaking change

  • Public API — none of the removed names were exported from any public entry point
  • Code-first config — accessRules was never accepted by defineCollection or defineSingle
  • Singles REST — the accessRules body field is now ignored rather than rejected, so no new 400s
  • Existing databases — the column is retained, never dropped without the operator asking

Docs

packages/nextly/AGENTS.md, docs/guides/routing-and-seo.mdx, docs/guides/isr-caching.mdx and the scaffold's templates/base/AGENTS.md.template described the two-layer model and stored rules for anonymous readers; all three now describe the single mechanism, including the anonymous code-rule branch main gained in #1704.

Tests

All break-verified against code mutations rather than test edits. The two integration tests below that guard the retained column run on every dialect with a database in the run — SQLite in-memory always, Postgres and MySQL through a per-run database (src/__tests__/helpers/fresh-database.ts), skipped without a URL. Verified locally on all three: 15/15. Break-verified per dialect: a Postgres count branch that answers 0 fails exactly the two Postgres loud cases while SQLite and MySQL stay green; a neutered destructive guard fails the upgrade case on all three.

  • access-rules-orphan-column.test.ts — 5 cases pinning the refusal, the operator override, the dev-path prompt and the additive skip. 4 of 5 fail when the column is restored to the desired schema.
  • access-rules-orphan-column-upgrade.integration.test.ts (×3 dialects) — a current database still carrying the column upgrades cleanly, with the drop blocked and no degraded pass. Fails for the intended reason when the destructive guard is neutered.
  • init/__tests__/retired-access-rules.test.ts (7), retired-access-rules-boot.integration.test.ts (4 × 3 dialects, a real database through ensureFirstRunSetup) — the boot warning names each table and its count, stays quiet when the column is absent or every value is NULL, and never blocks boot. Breaks: dropping the wiring fails both loud cases; counting all rows instead of non-NULL rows fails three; counting without first checking the column exists fails the never-had-it case.

related-row-collection-access.integration.test.ts is rewritten onto code-defined access; both leak cases fail when the target verdict admits everything, while the mirrors keep passing. Round 3 adds an anonymous pair (a read: ({ user }) => !!user target is withheld from a visitor exactly as the direct read refuses them; a rule-less target stays populated) and an api-key spelling case (a permissions.includes("pages:read") target admits a key scoped read-pages through the relationship as by name) — each fails when its early return or the stored copy is put back. single-query-service.read-access.test.ts gains the Single gate's anonymous cases (rule refuses → 403; no rule → public default; anonymous publish refused even when the rule admits), break-verified the same way. Two cases added in round 2: a rule that reads permissions admits a role-holding user on expansion exactly as on a direct read (fails when the session branch is reverted to the narrow context), and three parents expanding one target in a list read invoke the rule once (fails with 3 ≠ 1 when the per-row judgment is restored).

Obsolete suites were deleted rather than left green: they drove getAccessQueryConstraint, the stored-rule evaluator, or the strict authorization view, none of which exist now.

Verification

Run against this branch rebased onto main:

Gate Result
Build — 19 published packages 19/19
tsc product + tests clean
eslint packages/nextly/src clean
Unit suite 13 failures / 7 files — pre-existing: five files identical on main with none of this branch present; slug-param-is-a-leaf (#1715) and block-document (#1784) are Windows backslash-vs-slash path assertions in code this branch does not touch
Integration — Postgres 17 33 failures — exact baseline, zero regressions (run before the latest rebase)
Integration — MySQL 33 failures — exact baseline, zero regressions (run before the latest rebase)
fallow audit --base origin/main verdict warn (exit 0) with the health baseline; dead code introduced: 0 (the acorn / acorn-walk devDependencies that expression-validator.ts alone imported are removed). The verdict still reads fail on CRAP scores for updateEntry, createEntry, checkCollectionAccess and neighbours — pre-existing functions this diff shrinks, flagged because changed lines sit inside them — and on near-duplicate blocks between the bulk/mutation/dispatcher paths that predate this change. Neither is refactored here.

The pre-existing unit failures are config-loader-registry-restore, config-loader-watch-serialization, block-manifest, ndjson, local-read-cap, slug-param-is-a-leaf and the integration ones db-sync-localized-companion, ensure-core-tables-registry, draft-published-split. The first five were confirmed on a detached origin/main checkout.

Rebase

Rebased onto current main twice; the branch is one commit. The second rebase (over #1642's read-path refactor and #1772/#1783's caller work) took main's versions of the auth modules and the query service wholesale and re-applied the stored-rule removal on top — resolveRowScope no longer resolves an access constraint, accessConstraintCondition is gone, and collection-read-access-parity.test.ts (a main suite that tests the stored predicate on all three read paths) is deleted as obsolete rather than left green: it tested a mechanism that no longer exists. The first rebase (85 commits) Carried #1695's ambient scope pinning and #1704's anonymous code-rule branch (checkAnonymousCodeAccess) into checkCollectionAccess; deleted owner-safety-net.ts and its test, which only served the stored owner-only rule; renamed the batch row-gate mode from "owner-predicate" to "batch-hoisted", since no predicate remains; bare-Error allowlist ratchet 95 → 93.

Integration was run against freshly recreated databases: nextly_widget_layout is a fixed-name system table that cannot be per-file prefixed, so state survives between whole-suite runs and produced a false primary-key failure until the databases were reset.

@greptile-apps

greptile-apps Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Too many files changed for review (159 files, 100 file limit).

Bypass the limit by tagging @greptile-apps to review.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Too many files!

This PR contains 157 files, which is 57 over the limit of 100.

To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch.

Upgrade to a paid plan to raise the limit.

This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 5a02efc1-3a82-41f5-8160-20ae1d27498f

📥 Commits

Reviewing files that changed from the base of the PR and between 746a714 and 80db282.

⛔ Files ignored due to path filters (2)
  • .changeset/one-access-mechanism-not-two.md is excluded by !.changeset/**
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/pnpm-lock.yaml
📒 Files selected for processing (157)
  • .fallowrc.jsonc
  • docs/guides/isr-caching.mdx
  • docs/guides/routing-and-seo.mdx
  • fallow-health-baseline.json
  • packages/nextly/AGENTS.md
  • packages/nextly/eslint-bare-error-allowlist.json
  • packages/nextly/package.json
  • packages/nextly/src/__tests__/database/_fixture-schema/unified.ts
  • packages/nextly/src/__tests__/helpers/fresh-database.ts
  • packages/nextly/src/__tests__/helpers/seed-helpers.ts
  • packages/nextly/src/api/__tests__/document-lock-route.test.ts
  • packages/nextly/src/api/__tests__/versions-access.test.ts
  • packages/nextly/src/api/dashboard.ts
  • packages/nextly/src/api/document-lock.ts
  • packages/nextly/src/api/preview-access.ts
  • packages/nextly/src/api/preview-links.test.ts
  • packages/nextly/src/api/preview-links.ts
  • packages/nextly/src/api/singles-schema-detail.ts
  • packages/nextly/src/api/translations.ts
  • packages/nextly/src/api/versions-access.ts
  • packages/nextly/src/api/versions-detail.ts
  • packages/nextly/src/auth/__tests__/entity-read-access.test.ts
  • packages/nextly/src/auth/caller-scope.ts
  • packages/nextly/src/auth/resource-readable.ts
  • packages/nextly/src/database/sqlite-core-tables.ts
  • packages/nextly/src/di/registrations/register-auth.ts
  • packages/nextly/src/di/registrations/register-collections.ts
  • packages/nextly/src/dispatcher/handlers/__tests__/collection-dispatcher-read-access.test.ts
  • packages/nextly/src/dispatcher/handlers/__tests__/format-toast-summary.test.ts
  • packages/nextly/src/dispatcher/handlers/__tests__/should-log-f14-hint-receipt.test.ts
  • packages/nextly/src/dispatcher/handlers/__tests__/single-dispatcher-shapes.test.ts
  • packages/nextly/src/dispatcher/handlers/collection-dispatcher.ts
  • packages/nextly/src/dispatcher/handlers/single-dispatcher.ts
  • packages/nextly/src/dispatcher/helpers/authenticated-user.ts
  • packages/nextly/src/dispatcher/helpers/validation.ts
  • packages/nextly/src/domains/auth/__tests__/expression-validator-boundaries.test.ts
  • packages/nextly/src/domains/auth/__tests__/expression-validator.test.ts
  • packages/nextly/src/domains/auth/services/anonymous-code-access.test.ts
  • packages/nextly/src/domains/auth/services/expression-validator.ts
  • packages/nextly/src/domains/auth/services/rbac-access-control-service.ts
  • packages/nextly/src/domains/collections/__tests__/_fixtures/localized-target-read-rule.ts
  • packages/nextly/src/domains/collections/__tests__/_fixtures/related-target-read-rule.ts
  • packages/nextly/src/domains/collections/__tests__/_fixtures/single-read-rule.ts
  • packages/nextly/src/domains/collections/__tests__/_fixtures/tenant-read-rule.ts
  • packages/nextly/src/domains/collections/__tests__/collection-access.test.ts
  • packages/nextly/src/domains/collections/__tests__/collection-bulk.test.ts
  • packages/nextly/src/domains/collections/__tests__/collection-hooks.test.ts
  • packages/nextly/src/domains/collections/__tests__/collection-mutation.test.ts
  • packages/nextly/src/domains/collections/__tests__/collection-query.test.ts
  • packages/nextly/src/domains/collections/__tests__/collection-read-access-parity.test.ts
  • packages/nextly/src/domains/collections/__tests__/collection-test-helpers.ts
  • packages/nextly/src/domains/collections/__tests__/created-by-stamp.integration.test.ts
  • packages/nextly/src/domains/collections/__tests__/localized-target-predicate.integration.test.ts
  • packages/nextly/src/domains/collections/__tests__/publish-enforcement-owner-only.integration.test.ts
  • packages/nextly/src/domains/collections/__tests__/publish-enforcement-pool-reentry.integration.test.ts
  • packages/nextly/src/domains/collections/__tests__/publish-enforcement-rbac.integration.test.ts
  • packages/nextly/src/domains/collections/__tests__/read-constraint-predicate.integration.test.ts
  • packages/nextly/src/domains/collections/__tests__/related-row-collection-access.integration.test.ts
  • packages/nextly/src/domains/collections/__tests__/related-row-status.integration.test.ts
  • packages/nextly/src/domains/collections/services/__tests__/lifecycle-vs-trust-seam.test.ts
  • packages/nextly/src/domains/collections/services/__tests__/owner-safety-net.test.ts
  • packages/nextly/src/domains/collections/services/__tests__/related-row-fetch-seam.test.ts
  • packages/nextly/src/domains/collections/services/all-locales-lifecycle.ts
  • packages/nextly/src/domains/collections/services/collection-access-anonymous.test.ts
  • packages/nextly/src/domains/collections/services/collection-access-constraint.test.ts
  • packages/nextly/src/domains/collections/services/collection-access-route-authorized.test.ts
  • packages/nextly/src/domains/collections/services/collection-access-service.ts
  • packages/nextly/src/domains/collections/services/collection-bulk-service.ts
  • packages/nextly/src/domains/collections/services/collection-mutation-service.ts
  • packages/nextly/src/domains/collections/services/collection-query-service.ts
  • packages/nextly/src/domains/collections/services/collection-relationship-service.ts
  • packages/nextly/src/domains/collections/services/owner-safety-net.ts
  • packages/nextly/src/domains/dynamic-collections/__tests__/dynamic-collection-registry-service.not-found.integration.test.ts
  • packages/nextly/src/domains/dynamic-collections/services/dynamic-collection-registry-service.ts
  • packages/nextly/src/domains/dynamic-collections/services/dynamic-collection-schema-service.ts
  • packages/nextly/src/domains/schema/migrate/access-rules-orphan-column-upgrade.integration.test.ts
  • packages/nextly/src/domains/schema/migrate/access-rules-orphan-column.test.ts
  • packages/nextly/src/domains/schema/migrate/metadata-register.ts
  • packages/nextly/src/domains/schema/pipeline/__tests__/v1-golden/upgrade-sim-045.integration.test.ts
  • packages/nextly/src/domains/schema/services/field-column-descriptor.ts
  • packages/nextly/src/domains/singles/__tests__/custom-read-constraint.integration.test.ts
  • packages/nextly/src/domains/singles/__tests__/draft-overlay-related-access.integration.test.ts
  • packages/nextly/src/domains/singles/__tests__/publish-enforcement.integration.test.ts
  • packages/nextly/src/domains/singles/__tests__/single-entry-service.test.ts
  • packages/nextly/src/domains/singles/__tests__/single-query-service.authorization-completeness.test.ts
  • packages/nextly/src/domains/singles/__tests__/single-query-service.read-access.test.ts
  • packages/nextly/src/domains/singles/__tests__/single-registry-service.status-persist.test.ts
  • packages/nextly/src/domains/singles/__tests__/single-registry-service.test.ts
  • packages/nextly/src/domains/singles/__tests__/single-registry-service.versions-persist.test.ts
  • packages/nextly/src/domains/singles/__tests__/single-test-helpers.ts
  • packages/nextly/src/domains/singles/services/__tests__/single-document-access.test.ts
  • packages/nextly/src/domains/singles/services/single-access-stored-rules.test.ts
  • packages/nextly/src/domains/singles/services/single-document-access.ts
  • packages/nextly/src/domains/singles/services/single-entry-service.ts
  • packages/nextly/src/domains/singles/services/single-mutation-service.ts
  • packages/nextly/src/domains/singles/services/single-publish-all.ts
  • packages/nextly/src/domains/singles/services/single-query-service.ts
  • packages/nextly/src/domains/singles/services/single-registry-service.ts
  • packages/nextly/src/domains/versions/__tests__/pending-edits-document-rules.integration.test.ts
  • packages/nextly/src/domains/versions/__tests__/versions-widget-source.test.ts
  • packages/nextly/src/domains/versions/db-api.ts
  • packages/nextly/src/domains/versions/draft-overlay.ts
  • packages/nextly/src/domains/versions/pending-edit-visibility.ts
  • packages/nextly/src/domains/versions/restore-version.ts
  • packages/nextly/src/domains/versions/versions-widget-source.ts
  • packages/nextly/src/domains/widgets/result.ts
  • packages/nextly/src/errors/__tests__/bare-error-allowlist.test.ts
  • packages/nextly/src/init/__tests__/retired-access-rules-boot.integration.test.ts
  • packages/nextly/src/init/__tests__/retired-access-rules.test.ts
  • packages/nextly/src/init/bounded-diagnostic.ts
  • packages/nextly/src/init/first-run.ts
  • packages/nextly/src/init/retired-access-rules.ts
  • packages/nextly/src/lib/__tests__/system-columns.test.ts
  • packages/nextly/src/lib/immutable-system-fields.ts
  • packages/nextly/src/lib/system-columns.ts
  • packages/nextly/src/plugins/test-nextly.ts
  • packages/nextly/src/routeHandler.ts
  • packages/nextly/src/runtime/cache/__tests__/read-helpers.test.ts
  • packages/nextly/src/runtime/cache/cached-find.ts
  • packages/nextly/src/runtime/preview/__tests__/preview-draft-gate.test.ts
  • packages/nextly/src/runtime/preview/__tests__/preview-identity.test.ts
  • packages/nextly/src/runtime/preview/preview-draft-gate.ts
  • packages/nextly/src/runtime/preview/preview-single-draft-gate.ts
  • packages/nextly/src/runtime/routing/resolve-content.ts
  • packages/nextly/src/schemas/dynamic-collections/mysql.ts
  • packages/nextly/src/schemas/dynamic-collections/postgres.ts
  • packages/nextly/src/schemas/dynamic-collections/sqlite.ts
  • packages/nextly/src/schemas/dynamic-collections/types.ts
  • packages/nextly/src/schemas/dynamic-field-groups/mysql.ts
  • packages/nextly/src/schemas/dynamic-field-groups/postgres.ts
  • packages/nextly/src/schemas/dynamic-field-groups/sqlite.ts
  • packages/nextly/src/schemas/dynamic-field-groups/types.ts
  • packages/nextly/src/schemas/dynamic-singles/mysql.ts
  • packages/nextly/src/schemas/dynamic-singles/postgres.ts
  • packages/nextly/src/schemas/dynamic-singles/sqlite.ts
  • packages/nextly/src/schemas/dynamic-singles/types.ts
  • packages/nextly/src/services/access/access-control-service.test.ts
  • packages/nextly/src/services/access/access-control-service.ts
  • packages/nextly/src/services/access/constraint-shape.ts
  • packages/nextly/src/services/access/index.ts
  • packages/nextly/src/services/access/super-admin.ts
  • packages/nextly/src/services/access/types.ts
  • packages/nextly/src/services/collections-handler.ts
  • packages/nextly/src/services/collections/collection-entry-service.ts
  • packages/nextly/src/services/collections/related-row-read-context.ts
  • packages/nextly/src/services/collections/trust-bound.ts
  • packages/nextly/src/services/dashboard/__tests__/activity-document-rules.integration.test.ts
  • packages/nextly/src/services/dashboard/__tests__/collection-counts.test.ts
  • packages/nextly/src/services/dashboard/__tests__/status-breakdown.test.ts
  • packages/nextly/src/services/dashboard/activity-log-service.ts
  • packages/nextly/src/services/dashboard/dashboard-service.ts
  • packages/nextly/src/services/index.ts
  • packages/nextly/src/services/lib/document-visibility.ts
  • packages/nextly/src/services/lib/readable-documents.ts
  • packages/nextly/src/services/system/system-table-service.ts
  • packages/nextly/tsconfig.tests.json
  • templates/base/AGENTS.md.template

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

@nextlyhq/adapter-drizzle

npm i https://pkg.pr.new/@nextlyhq/adapter-drizzle@bed0dac

@nextlyhq/adapter-mysql

npm i https://pkg.pr.new/@nextlyhq/adapter-mysql@bed0dac

@nextlyhq/adapter-postgres

npm i https://pkg.pr.new/@nextlyhq/adapter-postgres@bed0dac

@nextlyhq/adapter-sqlite

npm i https://pkg.pr.new/@nextlyhq/adapter-sqlite@bed0dac

@nextlyhq/admin

npm i https://pkg.pr.new/@nextlyhq/admin@bed0dac

@nextlyhq/admin-css

npm i https://pkg.pr.new/@nextlyhq/admin-css@bed0dac

@nextlyhq/blocks-engine

npm i https://pkg.pr.new/@nextlyhq/blocks-engine@bed0dac

@nextlyhq/blocks-react

npm i https://pkg.pr.new/@nextlyhq/blocks-react@bed0dac

@nextlyhq/builder

npm i https://pkg.pr.new/@nextlyhq/builder@bed0dac

create-nextly-app

npm i https://pkg.pr.new/create-nextly-app@bed0dac

@nextlyhq/eslint-plugin

npm i https://pkg.pr.new/@nextlyhq/eslint-plugin@bed0dac

nextly

npm i https://pkg.pr.new/nextly@bed0dac

@nextlyhq/plugin-form-builder

npm i https://pkg.pr.new/@nextlyhq/plugin-form-builder@bed0dac

@nextlyhq/plugin-page-builder

npm i https://pkg.pr.new/@nextlyhq/plugin-page-builder@bed0dac

@nextlyhq/plugin-sdk

npm i https://pkg.pr.new/@nextlyhq/plugin-sdk@bed0dac

@nextlyhq/plugin-seo

npm i https://pkg.pr.new/@nextlyhq/plugin-seo@bed0dac

@nextlyhq/storage-s3

npm i https://pkg.pr.new/@nextlyhq/storage-s3@bed0dac

@nextlyhq/storage-uploadthing

npm i https://pkg.pr.new/@nextlyhq/storage-uploadthing@bed0dac

@nextlyhq/storage-vercel-blob

npm i https://pkg.pr.new/@nextlyhq/storage-vercel-blob@bed0dac

@nextlyhq/ui

npm i https://pkg.pr.new/@nextlyhq/ui@bed0dac

commit: bed0dac

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 68692b3216

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread .changeset/one-access-mechanism-not-two.md Outdated
@github-actions

github-actions Bot commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Whole-Repository Code Hygiene Summary

Full dead-code, duplication, and complexity report for the PR branch as it stands now. Playground is excluded. Quality gate enforcement on introduced issues is performed by the Changed files job.

🌿 Fallow

Warning

Review needed

⚠️ 73 code issues · ⚠️ 685 clone groups · ⚠️ 1031 health findings

See inline review comments for per-finding details.

Code issues (73)
Category Count
Unused files 2
Unused exports 5
Unused dependencies 19
Unused devDependencies 6
Unresolved imports 2
Unlisted dependencies 1
Circular dependencies 38
Duplication (685 groups · 28738 lines · 4.1%)
Locations Lines Tokens
schemas/_dialect-bundles/mysql.relations.ts:40-134
schemas/_dialect-bundles/postgres.relations.ts:40-134
schemas/_dialect-bundles/sqlite.relations.ts:40-134
95 593
cli/commands/db-sync-demote.ts:70-75
cli/commands/db-sync-promote.ts:38-43
cli/commands/dev-build.ts:100-105
cli/commands/dev-build.ts:179-184
cli/commands/dev-build.ts:299-304
cli/commands/dev-build.ts:411-416
cli/commands/dev-build.ts:552-557
cli/commands/dev-server.ts:575-580
cli/commands/dev-server.ts:840-845
cli/commands/dev-server.ts:1143-1148
cli/commands/migrate-field-groups.ts:110-115
6 70
entries/EntryList/EntryTableSkeleton.tsx:74-98
collection/components/CollectionTableSkeleton.tsx:94-118
field-group/components/FieldGroupTableSkeleton.tsx:90-114
plugins/components/PluginsTableSkeleton.tsx:86-110
singles/components/SinglesTableSkeleton.tsx:77-101
src/components/table-skeleton.tsx:100-124
25 89
collections/config/validate-config.ts:380-433
field-groups/config/validate-field-group.ts:185-238
singles/config/validate-single.ts:190-243
54 152
dispatcher/handlers/collection-dispatcher.ts:925-967
field-groups/services/field-group-table-provisioning.ts:186-236
singles/services/reconcile-single-companion.ts:110-160
51 149

… and 680 more groups.

Across 422 files.

Complexity (1031 functions above threshold)
File Function Severity Cyclomatic Cognitive CRAP Lines
singles/services/single-mutation-service.ts:937 <arrow> critical 245 ! 307 ! 13210.4 ! 1588
collections/services/collection-mutation-service.ts:6110 <arrow> critical 169 ! 160 ! 6338.2 ! 1261
src/init/reload-config.ts:1319 applyReload critical 144 ! 228 ! 4623 ! 1433
shared/lib/entry-validation.ts:223 validateFieldValue critical 109 ! 157 ! 2675.3 ! 432
blocks-engine/src/measure-bytes.ts:646 surveyDocument critical 102 ! 250 ! 137.1 ! 658

5016 files, 76700 functions analyzed (thresholds: cyclomatic > 20, cognitive > 15, CRAP >= 30)

Codebase health

Metric Value
Maintainability 91.7 / 100
Avg complexity 1.8

Tip

Run fallow fix --dry-run to preview auto-fixes.
Add /** @public */ above exports to preserve them.

@mobeenabdullah

Copy link
Copy Markdown
Collaborator

Hi Faisal — I read this one closely. The idea is right and most of the execution is good, so I want to be specific about what to keep and what to change, and why.


First, what you got right

I checked your central claim instead of taking your word for it, and it holds: collections really do have no way to author stored rules — no config option, no admin UI, no REST field. The only mention anywhere in the admin package is an old changelog line. So "two engines, and nobody can configure one of them" is a fair description, and deleting the unreachable one is the right call.

Three things I'd single out:

  • You were careful with the database. The column stays, nextly migrate refuses to drop it and names it, and you wrote upgrade tests for all three dialects. This is the part that usually goes wrong in a change like this, and you got it right.
  • You found a real security hole on the way. An unauthenticated caller could publish or unpublish a document. That's now refused. Nobody asked you to find that.
  • You disclosed the awkward exception — that a Single's rules could be set through an undocumented REST field — instead of quietly leaving it out. That honesty is what let me review this properly.

The one thing that has to change before this merges

Your release note opens with:

Three behaviours change, all of them narrowing

and the second bullet under it is:

An owner-only read no longer narrows a result set.

Those two statements contradict each other, and the second one is the important one. Let me spell out what it means for a real site.

Picture a customer who set an owner-only read rule on a collection (they could, via that undocumented Single REST field). Today, when Ana lists documents, she sees only Ana's. After this change, the per-row filter is gone, so the decision is made once for the whole collection — and if Ana is allowed to read the collection at all, she now sees Bob's documents too.

That is not narrowing. That is widening, and widening access is the one kind of change that has to be announced loudly, because an operator reading "all narrowing" will upgrade without checking anything.

Nothing about your code is wrong here — removing the filter follows necessarily from removing the engine. The problem is only that the release note tells operators the opposite of what will happen.

What to do: rewrite that section to say plainly that one change widens access, name who it affects (installations whose retained access_rules column holds an owner-only rule), and say what they should do before upgrading.


The gap right next to it

Follow that scenario one step further.

You keep the access_rules column and its data — good, that's the safe choice. But after this PR nothing reads it any more. So that customer's rules are still sitting in their database, doing nothing, and nothing anywhere tells them. No log line, no warning at boot. I searched your whole diff for one; there isn't any.

From their side: they set up access rules, the rules are still visibly there in the database, and they have silently stopped working.

What to do: when Nextly starts, if that column contains any data, log a warning saying these rules are no longer enforced and pointing at the release note. There's a pattern for exactly this already — see init/drift-check.ts and init/first-run.ts, which both use logger.warn for "your database has something you should know about". Reuse that shape rather than inventing one.

This is the single change that turns a risky upgrade into a safe one, so I'd treat it as part of the same piece of work as the release note.


Three smaller things

1. The docs still describe the engine you deleted. The scoped access-control guide still tells readers that overrideAccess: false has two layers and that stored rules run for anonymous callers. The next person — or the next agent — will write code against a mechanism that is gone. Update the guide in this PR, not later.

2. A row can vanish from a relationship that the user can read directly. When expanding a relationship you evaluate the target's access.read through a path that hands it an empty permissions list, while a direct read gives it the user's real permissions. Same user, same row, two different answers depending on how they arrived. Worth fixing, because it will be reported as a ghost bug.

3. Access is evaluated once per row. A relationship with 500 rows makes 500 authorization calls for what is one collection-level verdict, all fired at once. If someone's access.read calls an external service, that's 500 concurrent calls. Evaluate once per target collection and reuse the answer.

(Items 2 and 3 are the two P2s Codex left — I'm repeating them because they're still open, not to pile on.)


Also

The branch now conflicts with main and is about a day behind. Rebase before the next round, or the conflicts will grow.

Your test deletions look right, by the way — I checked the scariest-looking one (single-query-service.authorization-completeness.test.ts) and it tests a custom stored rule, so it goes with the engine. No concern there.


The checklist

  1. Rewrite the release note: say clearly that one change widens access, and who it affects.
  2. Add a boot warning when access_rules still holds data.
  3. Update the scoped access-control guide.
  4. Give the relationship path the caller's real permissions.
  5. Evaluate target access once per collection, not once per row.
  6. Rebase on main.

The one thing worth taking away

Everything above comes back to a single habit.

A release note is a claim about behaviour, and it needs checking against the diff the same way a test does. "All narrowing" was written once at the top and then contradicted three lines later — not because you didn't understand the change, but because nothing forced the summary to be re-read against the list underneath it.

So before you push a change with a release note: read the note and the diff side by side, and for each sentence ask "which line of code makes this true?" If you can't point at one, it isn't verified yet.

The engineering here is solid. It's the description that needs to catch up with it — and that's a much easier fix than the other way round.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 391bf29ef0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@mobeenabdullah

Copy link
Copy Markdown
Collaborator

Hi Faisal — I re-read the new head (391bf29ef) against the checklist from my last comment, and I want to start with what you fixed, because I checked each one rather than assuming:

  • Release note — now says "Two narrow; one WIDENS, and it is the one to read". Exactly right.
  • Boot warning for retained access_rules data — init/bounded-diagnostic.ts + first-run.ts, with unit and integration tests. That's the change that makes this upgrade safe. Good.
  • Package guide (packages/nextly/AGENTS.md) now says there is no stored-rule layer. Good.
  • Relationship expansion — one verdict per target instead of one per row, and the relationship path builds its caller through the same readAccessCaller the direct read uses. Both right.
  • Rebased, no conflicts.

So the two blocking items from my review are done. Nice work.


Codex found three more on this head, and they are one problem wearing three coats

I verified each against the code — they're real. Here's the shape in plain words.

Your refactor made the collection gate ask a new question for anonymous callers: "does this collection's own access.read rule allow someone with no user?" (checkAnonymousCodeAccess, collection-access-service.ts). That's correct, and it's what makes read: false mean something for a visitor.

But three other places answer the same question, and none of them were told about the new rule:

1. The Single gate (P1) — single-query-service.ts, the if (!user) return null; right after the publish/unpublish guard. null means allowed. So an anonymous visitor asking for a Single with access.read: false gets it — and may even auto-create it. Meanwhile the same rule on a collection refuses them. Same rule, two answers.

2. Relationship population (P1) — collection-relationship-service.ts, judgeTarget: if (!user) return true;. The comment above it says "Anonymous admits, because a DIRECT read of the target admits". That was true before your PR and is false after it — a direct anonymous read of that target now consults its rule and can refuse. So a public parent document can expose rows from a target declaring read: false, through the relationship, that a visitor could not read directly. This is the same "comment describes the old code" trap as the docs one, just in a source comment.

3. API-key vocabulary (P2) — readAccessCaller copies authenticatedScope.permissions as stored (read-articles), but a rule callback is documented to receive the rule-facing spelling (articles:read) — the direct gate converts with ruleFacingPermissions(scope) (see auth/middleware/index.ts and authenticated-scope.ts). So a rule like permissions.includes("articles:read") passes on a direct read and fails when the same row arrives through a relationship. Two conversions of one caller.

Why these matter more than they look: every one of them is a case where a visitor sees more through one door than another — the widening class, which is the whole reason this PR needed the release-note change. And they're all the same shape as the finding you already fixed: the same question answered by more than one piece of code, and the copies drifted.

What to do

Make the anonymous decision live in one place and have all three doors call it:

  1. In the Single gate, replace if (!user) return null; with the same checkAnonymousCodeAccess call the collection gate makes (hand it the Single's registered access, user: null).
  2. In judgeTarget, replace if (!user) return true; with that same call against the target collection — and rewrite the comment above it, because its reason is gone.
  3. In readAccessCaller, build permissions with ruleFacingPermissions(caller.authenticatedScope) for an API key instead of copying the stored list — that's the conversion the rest of the codebase already uses.

Then, the test that proves it: for each door, one test where a target/Single declares read: false (or read: ({ user }) => !!user) and an anonymous read is refused — and the control, where the rule is absent and the read is admitted. Break each by putting the old early-return back and confirm the refusal test goes red. Without the break you can't tell the test reached the door.

That's the whole remaining list from my side. Once these three are in, I'd be comfortable with this merging.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c30dc6d5e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread .changeset/one-access-mechanism-not-two.md
Comment thread packages/nextly/src/init/retired-access-rules.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 691b2ea78e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4a530f5724

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread .changeset/one-access-mechanism-not-two.md Outdated
Comment thread templates/base/AGENTS.md.template Outdated
Comment thread packages/nextly/src/init/first-run.ts Outdated
@mobeenabdullah

Copy link
Copy Markdown
Collaborator

Checked 0d88b2c7c against the three items from my 07:22Z follow-up, by body rather than by symbol:

  1. Single gate — single-query-service.ts:326 now asks rbacAccessControlService.checkAnonymousCodeAccess for an anonymous caller, and single-query-service.read-access.test.ts pins both verdicts (refused on false, admitted on true) plus the not-called control for a session caller.
  2. Relationship population — judgeTarget no longer admits !user; it asks the same checkAnonymousCodeAccess question a direct anonymous read asks, undefined falls through to the public default, false refuses.
  3. API-key rule callbacks — readAccessCaller hands rules rulePermissions: ruleFacingPermissions(scope), the same spelling the direct api-key gate shows them, beside the stored permissions list the permission half reads.

All three are in. As I said at 07:22Z, I am comfortable with this merging on that basis. What I did not gate on: CI for this head — as of 11:20Z several jobs still read queued/in-progress, so read the jobs before merging rather than the rollup.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1a51478e36

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@faisal-rx
faisal-rx force-pushed the feat/nextly/unified-access branch from 1a51478 to 3f16e87 Compare September 11, 2026 14:46

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f16e87f4b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

targetCompanions: options.targetCompanions ?? new Map(),
// And the access verdicts, likewise: the caller is the same on every hop,
// so a target already judged for it is not asked again.
targetVerdicts: options.targetVerdicts ?? new Map(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Forward target verdicts through component expansions

When a list or detail read populates repeatable components, the caller supplies one shared targetVerdicts map, but field-group-query-service.ts forwards targetPolicies and targetCompanions while omitting this new cache. Every component instance therefore reaches this fallback with options.targetVerdicts undefined and allocates its own map; hundreds of components referencing the same target can launch hundreds of identical asynchronous access.read callbacks concurrently, and a stateful or transient callback can produce inconsistent admission within one response. Forward access.targetVerdicts alongside the other two caches.

AGENTS.md reference: packages/nextly/AGENTS.md:L31-L37

Useful? React with 👍 / 👎.

@faisal-rx
faisal-rx force-pushed the feat/nextly/unified-access branch from 3f16e87 to bed0dac Compare September 11, 2026 16:45
Two independent systems answered "may this caller do this", and only one of
them was configurable. The code-defined `access` on a collection's or Single's
own config is reached from every transport. The stored rules were a second
engine -- five rule types evaluated against an `access_rules` JSON column --
with no way to author them: collections had no code, UI or REST surface for the
column at all, and a Single had only an undocumented REST field. Two evaluators
for one question is a divergence waiting to be found in the gap between them,
and the half nobody could configure is the half nobody was checking.

So the second engine is removed rather than reconciled. `AccessControlService`
and its five evaluators, the `StoredAccessRule` / `CollectionAccessRules` /
`SingleAccessRules` types, the operation constants that only served them, and
the `access_rules` column on `dynamic_collections` and `dynamic_singles` all
go. `AccessOperation` and `ACCESS_OPERATIONS` stay: the RBAC gate is keyed on
them. `ExpressionValidator` goes with them, having had no importer at all.

Three behaviours change. Two narrow; one widens, and it is the one to read
before upgrading.

Stored rules are no longer enforced. Any value still in `access_rules` is
ignored from this release on. That changes nothing for an installation that
never wrote the column, which is most of them; it does change behaviour for
one that set a Single's rules through the undocumented REST field or wrote
the column directly, and the widest case is an `owner-only` read. That filter
was produced by the stored evaluator alone, so a list, count or by-id read
that returned only the caller's own rows now returns every row the coarse
gate admits for the collection. Any rule still needed is expressed as
code-defined `access` before upgrading. An installation whose database still
holds rules is told so at startup, with the tables and row counts.

An anonymous publish or unpublish is refused outright. It previously fell
through to a rule-less public default unless an explicit stored `publish` rule
denied it, so a collection with no rules -- which was every collection -- let
an unauthenticated caller move a document into the published state.

Populating a relationship judges the TARGET COLLECTION for the caller, once
per expansion rather than once per row, by evaluating that collection's own
code-defined `access.read` with the context a direct read builds: a session
caller's rule sees their real roles and effective permissions, a scoped API
key's rule sees the key's own grants. The RBAC permission gate is
deliberately not consulted there, for the reason it never was: requiring a
`read-<target>` grant naming a collection the caller never asked for by name
would empty the relationship for every caller whose grants do not list it.

A new database never gets the `access_rules` column. One that already has it
keeps it, and every schema entry point reports it rather than dropping it:
`nextly migrate` refuses and names it, the dev-server reconcile blocks the drop
and says so, and `NEXTLY_ALLOW_CORE_DESTRUCTIVE=1` is how an operator removes
it. Dropping a column that holds configured rules is their decision, not a side
effect of upgrading.

One upgrade is not supported and is called out rather than papered over. A
database old enough to be missing core columns added since -- a 0.45-era
install -- reconciles a drop and several adds on one table, which drizzle-kit
pairs to ask whether the drop is a rename; `pushSchema` builds that resolver
internally with no way to supply a hints handler, so it throws and the boot
degrades to its additive-tables-only baseline. Such a database should run
`nextly migrate` before taking this release. `upgrade-sim-045` is skipped for
that reason, with the fix that restores it named in its header.

Review rounds folded in:

The upgrade and boot-warning integration tests run on every dialect with a
database in the run, through one fresh-database helper. The Single gate and
relationship expansion ask the collection's own code-defined rule for a
caller with no session, as the collection gate does. A key's rule sees its
grants in the spelling a rule reads and the key's own roles, on every path.
The target verdict is decided once per expansion, before any target query.
The boot warning states the whole RBAC decision order and is derived from the
drift check's snapshot in one bounded pass. A Single's working-draft overlay
is read under the live document's related-row access. The scaffold's agent
guide and the changeset describe the model that remains. A health baseline
generated from `main` keeps the changed-files gate from refusing a change
that shrinks a function `main` already carries above threshold.
@faisal-rx
faisal-rx force-pushed the feat/nextly/unified-access branch from bed0dac to 80db282 Compare September 11, 2026 18:17
@faisal-rx
faisal-rx merged commit c1fdd9b into main Sep 11, 2026
19 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 80db282580

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +460 to +461
// 1.5. Load the current document first (no auto-create yet), then run the
// access check before any hooks/DB writes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Check Single update access before reading its row

When a Direct API or plugin call uses overrideAccess: false without prior route authorization and its update rule or RBAC denies, this still selects the live Single row before calling checkSingleAccess. Since this change removes every document-dependent stored rule, the access verdict no longer needs existingDoc; move the gate ahead of selectOne so denied callers do not query the protected table or tie up a slow database before rejection.

AGENTS.md reference: AGENTS.md:L308-L311

Useful? React with 👍 / 👎.

// caller above does not lift this — the operation has no identity to stamp
// — so an unauthenticated caller is refused outright rather than falling
// through to the permission-less default below.
if (!user && (operation === "publish" || operation === "unpublish")) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject anonymous transitions before invoking access callbacks

When a caller with no user attempts publish or unpublish under overrideAccess: false, the earlier anonymous branch invokes the collection's arbitrary code-defined callback before this unconditional 403. An async rule may perform external side effects or expensive work even though its result can never admit the operation; move this guard ahead of the RBAC/code-access branch.

AGENTS.md reference: AGENTS.md:L308-L311

Useful? React with 👍 / 👎.

// Quiet because the check ran and found nothing, not because it
// failed on the way.
expect(logger.debug).not.toHaveBeenCalledWith(
expect.stringContaining("Could not check for stored access rules")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Assert against the diagnostic's actual failure message

When either boot diagnostic fails, runBoundedDiagnostic logs [nextly] Could not check core schema state: ..., not the substring asserted here. A failure before the retired-rule count therefore produces no warning and still satisfies both not.toHaveBeenCalledWith assertions, so these tests labeled as distinguishing a healthy quiet result from a failed check go green on broken wiring; assert the actual failure prefix or add a positive probe that proves the diagnostic ran.

AGENTS.md reference: AGENTS.md:L228-L233

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Dependency updates (label applied by Dependabot) scope: core nextly type: docs Documentation only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants