diff --git a/.changeset/a-default-cannot-read-a-denied-field.md b/.changeset/a-default-cannot-read-a-denied-field.md new file mode 100644 index 0000000000..23c4d63f8b --- /dev/null +++ b/.changeset/a-default-cannot-read-a-denied-field.md @@ -0,0 +1,56 @@ +--- +"@nextlyhq/adapter-drizzle": patch +"@nextlyhq/adapter-mysql": patch +"@nextlyhq/adapter-postgres": patch +"@nextlyhq/adapter-sqlite": patch +"@nextlyhq/admin": patch +"@nextlyhq/admin-css": patch +"@nextlyhq/blocks-engine": patch +"@nextlyhq/blocks-react": patch +"@nextlyhq/builder": patch +"create-nextly-app": patch +"@nextlyhq/eslint-config": patch +"@nextlyhq/eslint-plugin": patch +"@nextlyhq/module-specifiers": patch +"nextly": patch +"@nextlyhq/plugin-form-builder": patch +"@nextlyhq/plugin-page-builder": patch +"@nextlyhq/plugin-sdk": patch +"@nextlyhq/plugin-seo": patch +"@nextlyhq/prettier-config": patch +"@nextlyhq/storage-s3": patch +"@nextlyhq/storage-uploadthing": patch +"@nextlyhq/storage-vercel-blob": patch +"@nextlyhq/telemetry": patch +"@nextlyhq/tsconfig": patch +"@nextlyhq/ui": patch +"@nextlyhq/plugin-mcp": patch +--- + +A function default cannot read a field the caller may not write. A `defaultValue` written as a function receives the data built so far, and that data still held every value the caller sent, including one a field rule denies them, so a default could read the forbidden value and carry it into a field the caller IS allowed to write: the denied field was stripped and its value persisted anyway, one column across. The rules are now applied to a COPY, and the copy is what the functions read. The record itself is left alone until the pass that decides what is stored, because a rule may depend on a sibling the caller omitted precisely BECAUSE it has a default, and judging that rule before the defaults exist would deny it and lose the value for good. + +That copy is now defaulted and judged a second time before the record reads it. A field the caller may not write still takes its own declared default, and the pass that runs before the defaults has no key to judge because nothing supplied one; left in the copy, a later function default read it and carried it into a field the caller CAN write, so the rule was defeated one column across by the field's own default rather than by the caller's input. + +A config reload installs the live config's field functions, replacing them wholesale at the point the reload commits. Replacing rather than adding, so an entity the new config no longer declares stops deciding anything: a collection dropped from the config keeps its registry row and its table so an orphan sweep can find them, so it stays addressable. At the commit point rather than when the config is read, so a reload whose DDL succeeds and whose later sync fails does not leave a rule from a config the process refused deciding writes. The field-level registry holds every field's `access` rules, hooks, `validate` and function `defaultValue`, none of which survive being stored, and a reload never went back through service registration: an edit to any of them kept running the version from process start until the dev server was restarted. That is wrong in the direction that matters most for an access rule, since a rule tightened in the config was not the one being enforced. Applied on the same optimistic terms as the field-type registry, and restored by the same undo when a reload is abandoned. + +An access rule declared inside an unnamed container is captured. The registry keyed only named entries, so every rule, hook and validator inside an unnamed presentational group was dropped. `defineCollection` refuses a field with no name, but a plugin contributing raw config is checked on its field TYPES and not their names, so the shape reaches the live config. + +A Single's first read no longer invents an incomplete group. Filling a group for the sake of one defaulted child, while a required sibling has no default and no value, stored a document the next create or update would refuse; that insert runs no validation pass of its own. The group is left absent instead. + +The documentation no longer claims a field group's children take their defaults before the entry's hooks. They are written in their own pass afterwards, so the parent's hooks see such a child as absent. + +A `defaultValue` written as a function now works on a reusable field group's children. A field group's fields are read from its stored definition on every write, and a function does not survive being stored, so only constants applied there: a function default on a field-group child was silently dropped on every write. Field groups now get the same live-config capture collections and Singles have, so the function form resolves from the config at boot, and it is re-read when the dev server reloads the config. It resolves against the instance being built, so a child may compute from a sibling defaulted before it. + +Only the default is wired. The same capture holds a field's `access` rules, hooks and `validate`, and nothing reads those for a field group, so registering them changes nothing about whether they are enforced. + +The promote gate judges the draft the write actually commits. + +The gate that re-judges a Single's pending change before it is published ran before the write transaction, where it could only judge a copy of the world as it was. It now runs inside that transaction, on the draft the transaction has locked, which closes what a pre-flight check could not: a draft saved by another writer between the check and the commit is now the draft that is judged; a `beforeChange` hook that turns a status-less edit into a publish no longer slips past a check that ran before the hooks; and `publishAllLocales`, which applies every language's snapshot to one row, now judges the combined result rather than each language against its own shared values. Resolving the caller's grants is the one thing that cannot happen inside a transaction, since it queries the pooled connection that transaction holds, so it is resolved beforehand and handed in. + +A field rule on a child of a group or a repeater row is enforced. The check copied the snapshot shallowly, so the rules deleted a denied nested value from the copy and the original alike and the comparison then saw an unchanged container. A denied child is now found at its own depth and named at its own path. + +A publish is no longer refused over a field nobody touched. The comparison read the live document through a read that expands an upload or a relationship into the document behind it, while the snapshot holds the identifier, so an untouched field of either kind looked like an edit. Both sides now go through one conversion. + +A draft older than a newly required field is refused rather than published. The check judged the promoted document as a patch, which skips absent properties by design, so a snapshot written before a field became required reported nothing and published a document that violates the contract. + +An API key is judged on the grants stamped on the key, not on the database roles of whoever owns it. diff --git a/data/nextly.db b/data/nextly.db new file mode 100644 index 0000000000..d86a60c445 Binary files /dev/null and b/data/nextly.db differ diff --git a/docs/configuration/fields.mdx b/docs/configuration/fields.mdx index d84f45d8bb..ba339f56d3 100644 --- a/docs/configuration/fields.mdx +++ b/docs/configuration/fields.mdx @@ -1025,20 +1025,22 @@ Static value or `(data) => value` function. The function form receives the docum Both forms are applied on the server, so a default holds for every write path that applies defaults, not only for the admin form. On an ordinary collection create through the REST or Direct API, an omitted field takes its -default before any hook runs, at every depth: a group's children, each -supplied repeater row's children, and the children of each new field-group -instance. A function default is read from the live config, since a +default before any hook runs, at every depth: a group's children and each +supplied repeater row's children. A new field-group instance also fills its own +children's defaults, but LATER: a field group's rows are written in their own +pass after the entry's hooks have run, so the parent's hooks see such a child as +absent and cannot read or derive from the value that will be stored. A function +default is read from the live config, since a collection's stored definition cannot hold a function, and it receives the data built so far at its own level: the document for a top-level field, the group or the row for a nested one. Rows are never invented; `minRows` is a validation rule, not an instruction to create rows. -Inside a reusable field group, only a constant default applies. A field -group's fields are read from its stored definition on every write, and a -function cannot be stored, so a `defaultValue` written as a function on a -field-group child is not applied anywhere: not on a collection create, not on -a Single's first read, and not in the admin form. Give a field-group child a -constant default, or set the value from the parent's own field. +A field-group child takes its default the same way, constant or function. Its +fields are read from a stored definition, which cannot carry a function, so the +function form is read from the live config exactly as a collection's is. It +resolves against the instance being built, so a child may compute from a sibling +that was defaulted before it. A Single's first read creates its row with the same defaults, top-level and nested, with two exceptions: a default on a `fieldGroup` field itself is diff --git a/packages/nextly/src/collections/__tests__/default-cannot-read-a-denied-field.integration.test.ts b/packages/nextly/src/collections/__tests__/default-cannot-read-a-denied-field.integration.test.ts new file mode 100644 index 0000000000..e63f428d12 --- /dev/null +++ b/packages/nextly/src/collections/__tests__/default-cannot-read-a-denied-field.integration.test.ts @@ -0,0 +1,172 @@ +/** + * A function default is evaluated on what the caller was ALLOWED to send. + * + * Function defaults receive the data built so far. They ran before field write + * access, so a field this caller may not create was still in the record while + * they ran: a default could read the forbidden value and carry it into a field + * the caller IS allowed to write. The denied field was stripped from the + * record and its value was persisted anyway, one column across, which is the + * field rule defeated by proxy. + * + * Write access now runs before the defaults as well as after the hooks, the + * two passes the read path runs for the same reason: the first decides what + * the caller may put in, the second catches a denied key a hook put back. + */ +import { afterEach, describe, expect, it } from "vitest"; + +import { defineCollection, text } from "../../config"; +import { createTestNextly, type TestNextly } from "../../plugins/test-nextly"; +import type { CollectionsHandler } from "../../services/collections-handler"; + +let current: TestNextly | undefined; +afterEach(async () => { + await current?.destroy(); + current = undefined; +}); + +const CALLER = { id: "u1", email: "u@x.test", roles: [] }; + +async function boot() { + current = await createTestNextly({ + collections: [ + defineCollection({ + slug: "leaky", + fields: [ + text({ name: "title" }), + text({ name: "secret", access: { create: () => false } }), + text({ + name: "derived", + defaultValue: d => `saw:${String(d.secret)}`, + }), + text({ name: "open" }), + text({ + name: "fromOpen", + defaultValue: d => `saw:${String(d.open)}`, + }), + ], + }), + ], + }); + return current.getService<"collectionsHandler">("collectionsHandler"); +} + +describe("a function default cannot read a field the caller may not write", () => { + it("evaluates on the authorized record, not the raw request", async () => { + const handler = (await boot()) as unknown as CollectionsHandler; + + const created = await handler.createEntry( + { + collectionName: "leaky", + // The route gate has run; the FIELD rules are the ones under test. + routeAuthorized: true, + user: CALLER, + }, + { title: "t", secret: "forbidden-value", open: "fine" } + ); + + expect(created.success, JSON.stringify(created)).toBe(true); + const row = created.data as Record; + // The denied field is stripped, as it always was. + expect(row.secret ?? null).toBeNull(); + // And its value did not reach the database through the default either. + expect(row.derived).not.toContain("forbidden-value"); + expect(row.derived).toBe("saw:undefined"); + // A default reading an ALLOWED field still sees it: the pass removes what + // the caller may not send, not everything they sent. + expect(row.fromOpen).toBe("saw:fine"); + }); + + it("still lets a trusted caller's default read the field", async () => { + const handler = (await boot()) as unknown as CollectionsHandler; + + const created = await handler.createEntry( + { collectionName: "leaky", overrideAccess: true }, + { title: "t", secret: "system-value", open: "fine" } + ); + + expect(created.success, JSON.stringify(created)).toBe(true); + const row = created.data as Record; + // `overrideAccess` means the field rules do not apply, so the value is + // both stored and visible to the default. + expect(row.secret).toBe("system-value"); + expect(row.derived).toBe("saw:system-value"); + }); + it("hides a denied field's OWN default from a later default", async () => { + current = await createTestNextly({ + collections: [ + defineCollection({ + slug: "defaulted", + fields: [ + text({ name: "title" }), + // Denied AND defaulted. Nothing the caller sends fills it, so the + // pass that runs before the defaults has no key to judge. + text({ + name: "secret", + defaultValue: "s3cret", + access: { create: () => false }, + }), + text({ + name: "copy", + defaultValue: d => `saw:${String(d.secret)}`, + }), + ], + }), + ], + }); + const handler = current.getService<"collectionsHandler">( + "collectionsHandler" + ) as unknown as CollectionsHandler; + + const created = await handler.createEntry( + { collectionName: "defaulted", routeAuthorized: true, user: CALLER }, + { title: "t" } + ); + + expect(created.success, JSON.stringify(created)).toBe(true); + const row = created.data as Record; + // The denied field takes no value, exactly as when the caller sends one. + expect(row.secret ?? null).toBeNull(); + // And the field the caller MAY write did not become a copy of it. Stored + // the other way round, the document would describe a `secret` it does not + // hold, and the rule guarding it would be defeated one column across. + expect(row.copy).toBe("saw:undefined"); + }); + + it("keeps a value whose permission a DEFAULT establishes", async () => { + current = await createTestNextly({ + collections: [ + defineCollection({ + slug: "gated", + fields: [ + text({ name: "title" }), + text({ name: "kind", defaultValue: "public" }), + text({ + name: "note", + access: { + create: ({ data }) => + (data as { kind?: string } | undefined)?.kind === "public", + }, + }), + ], + }), + ], + }); + const handler = current.getService<"collectionsHandler">( + "collectionsHandler" + ) as unknown as CollectionsHandler; + + const created = await handler.createEntry( + { collectionName: "gated", routeAuthorized: true, user: CALLER }, + // `kind` is omitted on purpose: its default is what satisfies the rule + // guarding `note`. + { title: "t", note: "should survive" } + ); + + expect(created.success, JSON.stringify(created)).toBe(true); + const row = created.data as Record; + expect(row.kind).toBe("public"); + // The rules that decide what is STORED run after the defaults, so the + // value the caller sent is still there to be allowed. + expect(row.note).toBe("should survive"); + }); +}); diff --git a/packages/nextly/src/di/register.ts b/packages/nextly/src/di/register.ts index 3b31e4f621..11811d6165 100644 --- a/packages/nextly/src/di/register.ts +++ b/packages/nextly/src/di/register.ts @@ -183,7 +183,7 @@ import type { UserFieldDefinitionService } from "../services/users/user-field-de import type { UserService } from "../services/users/user-service"; import { assertNoLegacyFieldGroupKey } from "../shared/legacy-field-group-key"; import { assertPluginFieldDeclarations } from "../shared/lib/assert-plugin-field-declarations"; -import { registerFieldFunctions } from "../shared/lib/field-level-registry"; +import { replaceFieldFunctions } from "../shared/lib/field-level-registry"; import type { AdminConfig, AuthConfig, @@ -828,6 +828,32 @@ export async function registerServices( } } + // The function-bearing half of the live config, installed the same way a + // reload installs it: one replacement built from the whole config. + // + // Outside the resolver gate above, deliberately. That gate is about runtime + // TABLES, and it has a supported failure path where the registry is absent; + // registering in there meant a caller's `access` rules, hooks, validators + // and function defaults silently did not exist whenever it took that path. + // None of this needs a schema registry to be true. + replaceFieldFunctions([ + ...(transformedConfig.collections ?? []).map(entity => ({ + kind: "collection" as const, + slug: (entity as { slug?: string }).slug ?? "", + fields: (entity as { fields?: unknown[] }).fields ?? [], + })), + ...(transformedConfig.singles ?? []).map(entity => ({ + kind: "single" as const, + slug: (entity as { slug?: string }).slug ?? "", + fields: (entity as { fields?: unknown[] }).fields ?? [], + })), + ...(transformedConfig.fieldGroups ?? []).map(entity => ({ + kind: "fieldGroup" as const, + slug: (entity as { slug?: string }).slug ?? "", + fields: (entity as { fields?: unknown[] }).fields ?? [], + })), + ]); + // Finalize the deferred Builder-lane targets now the DB is reachable (P8/D3). // Builder/UI entities live in the dynamic_* registry tables (loaded by // `loadDynamicTables` above), not in `config`, so their slugs weren't knowable @@ -1736,11 +1762,6 @@ async function registerConfigTablesInResolver( const dbName = (collection as { dbName?: string }).dbName; const fields = (collection as { fields?: unknown[] }).fields ?? []; if (!slug || !Array.isArray(fields) || fields.length === 0) continue; - // Capture function-bearing field configs (validate/access/hooks) - // from the LIVE config: the DB-backed registry serializes fields, - // which drops functions, so the write/read services resolve them - // through the field-level registry instead. - registerFieldFunctions("collection", slug, fields); const baseTableName = dbName ?? slug.replace(/-/g, "_"); const tableName = baseTableName.startsWith("dc_") ? baseTableName @@ -1802,8 +1823,6 @@ async function registerConfigTablesInResolver( const dbName = (single as { dbName?: string }).dbName; const fields = (single as { fields?: unknown[] }).fields ?? []; if (!slug || !Array.isArray(fields) || fields.length === 0) continue; - // Same live-config capture as the collections branch above. - registerFieldFunctions("single", slug, fields); const tableName = resolveSingleTableName({ slug, dbName }); // Why: forward the code-first `status: true` flag for singles too — // mirrors the collection branch above. Same Draft/Published runtime diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index 5bfd04c435..4120f51ad8 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -78,11 +78,13 @@ import { rehydrateSystemTimestamps, SYSTEM_TIMESTAMP_KEYS, } from "../../../shared/lib/case-conversion"; +import { detachData } from "../../../shared/lib/detach"; import { validateEntryData } from "../../../shared/lib/entry-validation"; import { applyFieldDefaults } from "../../../shared/lib/field-defaults"; import { applyFieldReadAccess, applyFieldWriteAccess, + callerAccessGrants, attachFieldValidators, getFieldFunctions, runFieldHooks, @@ -3095,12 +3097,83 @@ export class CollectionMutationService extends BaseService { // of write access matches generation, so a field the caller may not // create is not reintroduced. const seededBody: Record = { ...body }; + // One resolver for both write-access passes over this record, so the + // caller's roles and permissions are read once and both judge with one + // authority. + const writeGrants = callerAccessGrants( + params.user, + params.authenticatedScope + ); + // What a function default is allowed to READ, which is not the same as + // what this write is allowed to STORE. + // + // A function default receives the data built so far, and that data still + // holds every value the caller sent, including one a field rule denies + // them. Left alone, a default could read the forbidden value and carry it + // into a field the caller IS allowed to write: the denied field would be + // stripped below and its value persisted anyway, one column across. + // + // So the rules are applied to a COPY, and the copy is what the functions + // read. Applying them to the record itself would be wrong in the other + // direction: a rule may depend on a sibling the caller omitted BECAUSE it + // has a default (`data.kind === "public"` where `kind` defaults to + // `public`), and judging that rule before the defaults exist denies it. + // Deleting the value then would lose it for good, since the pass that + // decides correctly runs after the defaults and has nothing left to keep. + // A DEEP copy. The rules delete a denied value in place, and a shallow + // copy shares every nested group, repeater row and component with the + // record: the deletion would land on the record too, which is the data + // loss this whole arrangement exists to avoid, one level down. + const readableByDefaults = detachData(seededBody); + const fieldFunctions = getFieldFunctions( + "collection", + params.collectionName + ); + await applyFieldWriteAccess({ + kind: "collection", + slug: params.collectionName, + data: readableByDefaults, + operation: "create", + user: params.user, + authenticatedScope: params.authenticatedScope, + overrideAccess: params.overrideAccess, + grants: writeGrants, + }); + // The view is defaulted BEFORE it is judged a second time, and both + // halves matter. + // + // Defaulted, because a rule may depend on a sibling the caller omitted + // because it has a default, and a view judged without it hides a value + // the caller may in fact write. + // + // Judged again, because a default of its own is a value like any other: + // a field the caller may not write still takes its declared default + // here, and left in the view a later function default would read it and + // carry it into a field they CAN write. The pass below removes the field + // it came from and keeps the copy, so the rule is defeated one column + // across. + // + // These fill the view only. The record is filled from it afterwards, so + // a default the rules removed here is one the record's own pass will not + // put back. + applyFieldDefaults(readableByDefaults, fields, fieldFunctions); + await applyFieldWriteAccess({ + kind: "collection", + slug: params.collectionName, + data: readableByDefaults, + operation: "create", + user: params.user, + authenticatedScope: params.authenticatedScope, + overrideAccess: params.overrideAccess, + grants: writeGrants, + }); // The stored fields carry constant defaults; a function default exists // only in the live config, which the registry captured at boot. applyFieldDefaults( seededBody, fields, - getFieldFunctions("collection", params.collectionName) + fieldFunctions, + readableByDefaults ); const beforeOpArgs = @@ -3187,7 +3260,9 @@ export class CollectionMutationService extends BaseService { data: finalData, operation: "create", user: params.user, + authenticatedScope: params.authenticatedScope, overrideAccess: params.overrideAccess, + grants: writeGrants, }); // Field-level beforeValidate hooks transform values ahead of the diff --git a/packages/nextly/src/domains/field-groups/__tests__/field-group-instance-defaults.integration.test.ts b/packages/nextly/src/domains/field-groups/__tests__/field-group-instance-defaults.integration.test.ts index 07016de12c..ad76c2b8e9 100644 --- a/packages/nextly/src/domains/field-groups/__tests__/field-group-instance-defaults.integration.test.ts +++ b/packages/nextly/src/domains/field-groups/__tests__/field-group-instance-defaults.integration.test.ts @@ -166,4 +166,61 @@ describe("a new field-group instance takes its declared defaults (integration)", { heading: "Given", tone: "calm" }, ]); }); + it("applies a FUNCTION default declared on a field-group child", async () => { + current = await createTestNextly({ + fieldGroups: [ + defineFieldGroup({ + slug: "fnhero", + fields: [ + text({ name: "mode", defaultValue: "dark" }), + // A function cannot be stored, so the stored definition this write + // reads carries nothing for it. It resolves from the live config. + text({ + name: "label", + defaultValue: d => `mode-${String(d.mode)}`, + }), + ], + }), + ], + collections: [ + defineCollection({ + slug: "fnpages", + fields: [ + text({ name: "title" }), + fieldGroup({ name: "hero", component: "fnhero" }), + fieldGroup({ + name: "slides", + component: "fnhero", + repeatable: true, + }), + ], + }), + ], + }); + const handler = current.getService("collectionsHandler"); + + const created = await handler.createEntry( + { collectionName: "fnpages", overrideAccess: true }, + { title: "Home", hero: {}, slides: [{ mode: "light" }] } + ); + expect(created.success, JSON.stringify(created)).toBe(true); + + const read = await handler.getEntry({ + collectionName: "fnpages", + entryId: (created.data as { id: string }).id, + overrideAccess: true, + }); + const data = read.data as { + hero?: { mode?: string; label?: string }; + slides?: Array<{ mode?: string; label?: string }>; + }; + // Resolved against the instance built so far, as it is for a collection + // field: the constant before it is visible to the function after it. + expect(data.hero).toMatchObject({ mode: "dark", label: "mode-dark" }); + // A supplied sibling value is what the function reads, not the default. + expect(data.slides?.[0]).toMatchObject({ + mode: "light", + label: "mode-light", + }); + }); }); diff --git a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts index 6c3af5a0c9..6875fe62c7 100644 --- a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts +++ b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts @@ -19,6 +19,7 @@ import type { FieldGroupRegistryService } from "../../../services/field-groups/f import { BaseService } from "../../../shared/base-service"; import { validateEntryData } from "../../../shared/lib/entry-validation"; import { applyFieldDefaults } from "../../../shared/lib/field-defaults"; +import { getFieldFunctions } from "../../../shared/lib/field-level-registry"; import { coerceDateFieldsToDate, normalizeRelationshipFields, @@ -667,7 +668,8 @@ export class FieldGroupMutationService extends BaseService { data, componentFields, existing.length > 0 ? "update" : "create", - req + req, + componentMeta.slug ); // i18n: split translatable values out of the main comp_ write — they live on the @@ -784,7 +786,8 @@ export class FieldGroupMutationService extends BaseService { data, componentFields, existing.length > 0 ? "update" : "create", - req + req, + componentMeta.slug ); // i18n: split translatable values out of the main comp_ write (companion-owned). @@ -898,7 +901,8 @@ export class FieldGroupMutationService extends BaseService { instance, componentFields, instanceId && existingMap.has(instanceId) ? "update" : "create", - req + req, + componentMeta.slug ); // i18n: split translatable values out per instance (companion-owned). The @@ -1032,7 +1036,8 @@ export class FieldGroupMutationService extends BaseService { instance, componentFields, instanceId && existingMap.has(instanceId) ? "update" : "create", - req + req, + componentMeta.slug ); // i18n: split translatable values out (companion-owned) per instance. @@ -1217,7 +1222,8 @@ export class FieldGroupMutationService extends BaseService { instance, componentFields, instanceId && globalExistingMap.has(instanceId) ? "update" : "create", - req + req, + meta.slug ); // i18n: split translatable values out per instance using its own component meta. @@ -1385,7 +1391,8 @@ export class FieldGroupMutationService extends BaseService { instance, componentFields, instanceId && globalExistingMap.has(instanceId) ? "update" : "create", - req + req, + meta.slug ); // i18n: split translatable values out per instance using its own component meta. @@ -1630,7 +1637,13 @@ export class FieldGroupMutationService extends BaseService { instance: ComponentInstanceData, componentFields: FieldConfig[], mode: "create" | "update", - req: Record = {} + req: Record = {}, + /** + * The field group's slug, so a `defaultValue` written as a FUNCTION can be + * read from the field-level registry. The fields above come from the + * stored definition, which carries constants and cannot carry a function. + */ + slug?: string ): Promise { // A component instance must be a plain object. A primitive (e.g. a bare // string sent for a non-repeatable component field) would make the field @@ -1653,7 +1666,11 @@ export class FieldGroupMutationService extends BaseService { // instance the caller could not have completed. An existing instance is // left alone: its absent keys are the caller's patch, not missing values. if (mode === "create") { - applyFieldDefaults(instance, componentFields); + applyFieldDefaults( + instance, + componentFields, + slug ? getFieldFunctions("fieldGroup", slug) : undefined + ); } // Reduced in place, and before validation, because both halves of the write // depend on it. A validator is written against a field's public value, the diff --git a/packages/nextly/src/domains/singles/__tests__/single-first-read-nested-defaults.integration.test.ts b/packages/nextly/src/domains/singles/__tests__/single-first-read-nested-defaults.integration.test.ts index b3e384269f..1da34d6d0b 100644 --- a/packages/nextly/src/domains/singles/__tests__/single-first-read-nested-defaults.integration.test.ts +++ b/packages/nextly/src/domains/singles/__tests__/single-first-read-nested-defaults.integration.test.ts @@ -168,4 +168,65 @@ describe("a Single's first read fills nested defaults (integration)", () => { expect(messages).toContain("must be a page document"); }); + it("leaves an invented group absent when a required child has no value", async () => { + current = await createTestNextly({ + singles: [ + defineSingle({ + slug: "partial", + fields: [ + group({ + name: "contact", + fields: [ + text({ name: "label", defaultValue: "Support" }), + // Required, with no default and nothing to supply it on a + // first read: a group created for `label` alone would be a + // document the next write refuses. + text({ name: "email", required: true }), + ], + }), + ], + }), + ], + }); + + const doc = (await current.nextly.findSingle({ + slug: "partial", + overrideAccess: true, + })) as { contact?: unknown } | null; + + expect(doc?.contact ?? null).toBeNull(); + }); + + it("still fills a group whose required children are all satisfied", async () => { + current = await createTestNextly({ + singles: [ + defineSingle({ + slug: "complete", + fields: [ + group({ + name: "contact", + fields: [ + text({ name: "label", defaultValue: "Support" }), + text({ + name: "email", + required: true, + defaultValue: "support@example.test", + }), + ], + }), + ], + }), + ], + }); + + const doc = (await current.nextly.findSingle({ + slug: "complete", + overrideAccess: true, + })) as { contact?: { label?: string; email?: string } } | null; + + expect(doc?.contact).toEqual({ + label: "Support", + email: "support@example.test", + }); + }); }); diff --git a/packages/nextly/src/domains/singles/__tests__/single-publish-revalidates-its-draft.integration.test.ts b/packages/nextly/src/domains/singles/__tests__/single-publish-revalidates-its-draft.integration.test.ts index 825ec238c8..77bd875572 100644 --- a/packages/nextly/src/domains/singles/__tests__/single-publish-revalidates-its-draft.integration.test.ts +++ b/packages/nextly/src/domains/singles/__tests__/single-publish-revalidates-its-draft.integration.test.ts @@ -19,6 +19,7 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { defineSingle, group, text } from "../../../config"; +import type { HookContext } from "../../../hooks/types"; import { createAdapter } from "../../../database/factory"; import { createTestNextly, @@ -54,7 +55,12 @@ afterEach(async () => { * database file, once without it and once with, is how a draft held under an * older schema is reproduced: the value was legal when it was saved. */ -async function boot(strict: boolean): Promise { +async function boot( + strict: boolean, + /** Turns any write into a publish, deciding the status after the gate's old + * pre-hook position and before the promotion. */ + publishFromHook = false +): Promise { process.env.DB_DIALECT = "sqlite"; const adapter = await createAdapter({ type: "sqlite", @@ -68,6 +74,18 @@ async function boot(strict: boolean): Promise { status: true, versions: { drafts: true }, access: { read: () => true, update: () => true }, + hooks: { + beforeChange: [ + (ctx: HookContext) => { + const data = ctx.data as Record; + // Only in the case that asks for it, and only on the write under + // test, so every other write here still holds its edit. + return publishFromHook && data.siteName === "Edited" + ? { ...data, status: "published" } + : data; + }, + ], + }, fields: [ text({ name: "siteName" }), // A JSON-backed field, so every case here also proves the gate reads @@ -75,6 +93,19 @@ async function boot(strict: boolean): Promise { // string that was written; judged as-is, the validator calls it an // invalid type and an ordinary publish is refused. group({ name: "seo", fields: [text({ name: "metaTitle" })] }), + // A denied field NESTED in a group. The rules remove it from inside + // the container, which leaves the container itself present. + group({ + name: "ops", + fields: [ + text({ + name: "runbook", + access: { + update: ({ req }) => req.user?.email === BOSS.email, + }, + }), + ], + }), text({ name: "guarded", access: { update: ({ req }) => req.user?.email === BOSS.email }, @@ -327,4 +358,58 @@ describe("a Single's publish re-judges the draft it promotes", () => { }); expect(drafts).toHaveLength(1); }); + it("catches a denied field NESTED inside a group", async () => { + const singles = await boot(false); + await singles.update( + SLUG, + { siteName: "Live", status: "published" }, + { overrideAccess: true } + ); + await singles.update( + SLUG, + { ops: { runbook: "internal" } }, + { routeAuthorized: true, user: BOSS } + ); + + const published = await singles.update( + SLUG, + { status: "published" }, + { routeAuthorized: true, user: CLERK } + ); + + expect(published.success).toBe(false); + const issues = ( + published as { publicData?: { errors?: Array<{ path: string }> } } + ).publicData?.errors; + // Named at its own depth, so the author can see which child is at fault. + expect(issues?.map(i => i.path)).toEqual(["ops.runbook"]); + }); + + it("gates a publish a beforeChange hook decides on", async () => { + // The caller sends no status; the hook turns the write into a publish. A + // gate placed before the hooks would never see this one. + const singles = await boot(false, true); + await singles.update( + SLUG, + { siteName: "Live", status: "published" }, + { overrideAccess: true } + ); + await singles.update( + SLUG, + { guarded: "secret" }, + { routeAuthorized: true, user: BOSS } + ); + + const published = await singles.update( + SLUG, + { siteName: "Edited" }, + { routeAuthorized: true, user: CLERK } + ); + + expect(published.success).toBe(false); + const issues = ( + published as { publicData?: { errors?: Array<{ path: string }> } } + ).publicData?.errors; + expect(issues?.map(i => i.path)).toEqual(["guarded"]); + }); }); diff --git a/packages/nextly/src/domains/singles/services/promote-gate.ts b/packages/nextly/src/domains/singles/services/promote-gate.ts index f2d97bcb62..31998a3fa6 100644 --- a/packages/nextly/src/domains/singles/services/promote-gate.ts +++ b/packages/nextly/src/domains/singles/services/promote-gate.ts @@ -15,24 +15,29 @@ * gates would be two answers to one question, and the second would be the one * nobody remembered to update. * - * Called BEFORE the write transaction opens, never inside it. Both a field's - * `access` rule and its `validate` are user code, and resolving the caller's - * grants issues its own queries on the pooled connection: run inside an open - * transaction on a small pool, those queries wait for a connection the - * transaction is holding and the publish hangs. The collection publish path - * gates its promotion outside the transaction for the same reason. + * Called INSIDE the write transaction, on the draft that transaction has + * locked. Judged before it, the gate can only judge a copy of the world as it + * was: another writer saving a draft in between, a `beforeChange` hook + * rewriting the status, or the fold merging several languages onto one row all + * make the checked document and the written document different documents. The + * one thing that cannot be done inside the transaction is resolve the caller's + * grants, which queries the pooled connection the transaction is holding, so + * that is resolved by the caller beforehand and handed in. * * @module domains/singles/services/promote-gate */ import { isDeepStrictEqual } from "node:util"; +import type { AuthenticatedScope } from "../../../auth/authenticated-scope"; import type { FieldConfig } from "../../../collections/fields/types"; import { NextlyError } from "../../../errors"; +import { detachData } from "../../../shared/lib/detach"; import { validateEntryData } from "../../../shared/lib/entry-validation"; import { applyFieldWriteAccess, attachFieldValidators, + type CallerGrants, } from "../../../shared/lib/field-level-registry"; import { relationshipValidationView } from "../../../shared/lib/field-transform"; @@ -44,13 +49,28 @@ export interface PromotableDraft { snapshot: unknown; } -/** What judging a draft needs from the service that holds it. */ +/** What judging a promotion needs from the service that performs it. */ export interface PromoteGateContext { slug: string; entryId: string; fields: FieldConfig[]; user?: Record; overrideAccess?: boolean; + /** + * The caller's grants, resolved on the pooled connection BEFORE the + * transaction opened. + * + * Resolving them here would issue queries while the transaction holds a + * connection, and on a small pool those queries wait for the connection the + * transaction is holding: the publish hangs rather than fails. + */ + grants?: () => Promise; + /** + * The scope an API key arrived with, so a key is judged on ITS grants rather + * than on the database roles of whoever owns it. There is no request-local + * scope to inherit on the Direct API, so it travels as an argument. + */ + authenticatedScope?: AuthenticatedScope; /** * A stored document in the logical shape the rules are written against. * @@ -61,14 +81,15 @@ export interface PromoteGateContext { */ toLogical: (doc: Record) => Record; /** - * The live document as a reader of that language sees it, used to decide - * whether promoting a denied field would CHANGE anything. + * The live row for one language, as STORED, used to decide whether promoting + * a denied field would change anything. It goes through `toLogical` here, as + * the snapshot does, so both sides of the comparison are one representation. * - * Language-aware by necessity: a localized field's live value is on the - * companion row, so a main-table row alone reports every translation as - * absent and every unchanged translation as an edit. + * A read's expanded document is the wrong source: it turns an upload or a + * relationship into the document behind the identifier, so an untouched + * field reads as an edit and a publish nobody objected to is refused. */ - liveDocumentFor: ( + liveStoredFor: ( locale: string | null ) => Promise | undefined>; /** @@ -82,26 +103,53 @@ export interface PromoteGateContext { } /** - * Refuse the publish unless every pending change may be promoted as it stands. + * Refuse the publish unless the pending changes may be promoted as they stand. + * + * The drafts are judged as ONE outcome, not one at a time. `publishAllLocales` + * applies every language's snapshot to the same main row, so the shared values + * that survive are the last writer's: judged separately, each draft can pass + * against its own shared values while the document that actually lands holds + * another language's. What is judged here is the state the write produces. * - * Throws on the first draft that cannot be, so nothing is written: a publish - * either applies the whole pending change or none of it. + * Throws on the first problem, so nothing is written: a publish applies the + * whole pending change or none of it. */ export async function assertDraftsMayBePromoted( drafts: readonly PromotableDraft[], ctx: PromoteGateContext ): Promise { + if (drafts.length === 0) return; + + // The shared half of what the write produces, in the order the write applies + // it, so the last value to land is the one judged. + let shared: Record = {}; + for (const draft of drafts) { + shared = { ...shared, ...ctx.toLogical(asRecord(draft.snapshot)) }; + } + for (const draft of drafts) { + // This language's own values over the shared outcome, then the caller's + // payload over both: the document this language ends up with. const promoted = { - ...ctx.toLogical((draft.snapshot ?? {}) as Record), + ...shared, + ...ctx.toLogical(asRecord(draft.snapshot)), ...(ctx.callerData ?? {}), }; - - await assertNoDeniedChange(promoted, draft.locale, ctx); + await assertNoDeniedChange( + ctx.toLogical(asRecord(draft.snapshot)), + draft.locale, + ctx + ); await assertSchemaStillAccepts(promoted, ctx); } } +function asRecord(value: unknown): Record { + return value && typeof value === "object" + ? (value as Record) + : {}; +} + /** * Refuse a promotion that would change a field this publisher may not write. * @@ -113,33 +161,45 @@ export async function assertDraftsMayBePromoted( * and take that author's edit with it, reporting success. Refusing keeps the * draft for someone who can write the field. * - * Judged on what would CHANGE, not on what the snapshot holds. A snapshot is a - * full copy of the document, so a denied field appears in every one of them; - * only a value that differs from what is live is a change being made. + * Judged in STORED shape, on both sides, and on what would CHANGE rather than + * on what the snapshot holds: a snapshot is a full copy of the document, so a + * denied field appears in every one of them, and only a value that differs + * from what is live is a change being made. */ async function assertNoDeniedChange( - promoted: Record, + logicalSnapshot: Record, locale: string | null, ctx: PromoteGateContext ): Promise { - const permitted = { ...promoted }; + // A deep copy, because the rules delete a denied value in place and a + // shallow one shares every nested group, repeater row and component with the + // original: the deletion would land on both, and the comparison below would + // then see a container still present and report nothing denied while the + // write persisted the forbidden nested edit. + const permitted = detachData(logicalSnapshot); await applyFieldWriteAccess({ kind: "single", slug: ctx.slug, data: permitted, operation: "update", user: ctx.user, + authenticatedScope: ctx.authenticatedScope, overrideAccess: ctx.overrideAccess, + grants: ctx.grants, id: ctx.entryId, }); - const denied = Object.keys(promoted).filter( - key => !Object.prototype.hasOwnProperty.call(permitted, key) - ); + + const denied = deniedPaths(logicalSnapshot, permitted, ""); if (denied.length === 0) return; - const live = (await ctx.liveDocumentFor(locale)) ?? {}; + // Both sides through the same conversion, so a JSON-backed value is an + // object on both and an upload or a relationship is the identifier it is + // stored as on both. Compared across representations, an untouched field + // reads as an edit and a publish nobody objected to is refused. + const live = ctx.toLogical(asRecord(await ctx.liveStoredFor(locale))); const deniedChanges = denied.filter( - key => !isDeepStrictEqual(promoted[key], live[key]) + path => + !isDeepStrictEqual(valueAt(logicalSnapshot, path), valueAt(live, path)) ); if (deniedChanges.length === 0) return; @@ -159,10 +219,67 @@ async function assertNoDeniedChange( }); } +/** + * Every path the rules removed, at any depth. + * + * A field rule can sit on a child of a group or of a repeater row, and the + * removal there leaves the container in place: comparing only the top level + * reports nothing denied and lets the forbidden nested edit through. + */ +function deniedPaths( + before: unknown, + after: unknown, + prefix: string +): string[] { + if (Array.isArray(before)) { + if (!Array.isArray(after)) return [prefix]; + return before.flatMap((row, index) => + deniedPaths(row, after[index], `${prefix}[${index}]`) + ); + } + if (!isRecord(before)) return []; + if (!isRecord(after)) return [prefix]; + const out: string[] = []; + for (const key of Object.keys(before)) { + const path = prefix ? `${prefix}.${key}` : key; + if (!Object.prototype.hasOwnProperty.call(after, key)) { + out.push(path); + continue; + } + out.push(...deniedPaths(before[key], after[key], path)); + } + return out; +} + +/** Read a dotted/bracketed path the way {@link deniedPaths} writes one. */ +function valueAt(root: unknown, path: string): unknown { + let current: unknown = root; + for (const step of path.split(/\.|\[(\d+)\]/).filter(Boolean)) { + if (Array.isArray(current) && /^\d+$/.test(step)) { + current = current[Number(step)]; + continue; + } + if (!isRecord(current)) return undefined; + current = current[step]; + } + return current; +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + /** * Refuse a promotion the schema no longer accepts, naming every field at * fault so the author can see what to fix. * + * Judged as a CREATE would be, not as a patch. A patch checks the keys it + * carries and skips the ones it does not, which is right for a caller sending + * a few fields and wrong here: a snapshot older than a newly required field + * simply has no property for it, so patch semantics report nothing and publish + * a document that violates the contract. The promoted document is the whole + * document, so it is judged whole. + * * Judged on the snapshot being promoted and never on the live document: a * schema change must not block someone from fixing and republishing content * that has nothing to do with it. @@ -175,7 +292,7 @@ async function assertSchemaStillAccepts( relationshipValidationView(promoted, ctx.fields), attachFieldValidators("single", ctx.slug, ctx.fields), { - mode: "update", + mode: "create", req: ctx.user ? { user: ctx.user } : {}, localizedFieldNames: ctx.localizedFieldNames, enforceLocalizedRequired: ctx.enforceLocalizedRequired, diff --git a/packages/nextly/src/domains/singles/services/single-mutation-service.ts b/packages/nextly/src/domains/singles/services/single-mutation-service.ts index 726c2da567..5fc276fc41 100644 --- a/packages/nextly/src/domains/singles/services/single-mutation-service.ts +++ b/packages/nextly/src/domains/singles/services/single-mutation-service.ts @@ -59,6 +59,7 @@ import { applyFieldWriteAccess, attachFieldValidators, runFieldHooks, + callerAccessGrants, } from "../../../shared/lib/field-level-registry"; import { coerceDateFieldsToDate, @@ -651,75 +652,15 @@ export class SingleMutationService extends BaseService { } // 6.3. A publish promotes the held pending change, and that content has - // been judged only once, when it was SAVED. Judge it again here, against - // the schema and the permissions as they stand now. - // - // Before the transaction, deliberately. Both a field's `access` rule and - // its `validate` are user code, and resolving the caller's grants issues - // its own pooled queries: run inside the open write transaction on a - // small pool, those queries wait for the connection the transaction is - // holding and the publish hangs. The collection publish path gates its - // promotion outside its transaction for the same reason. - // - // Only when this write NAMES `published`. A status-less edit is held as - // a pending change rather than promoting one, and the two are disjoint on - // exactly that, so a held edit is never gated as though it were a - // publish. - if ( - existingDoc !== null && - existingDoc !== undefined && - (singleMeta as { status?: boolean }).status === true && - singleMeta.versions?.drafts?.enabled === true && - currentData.status === "published" - ) { - const promotedInto = existingDoc; - const gateLocale = workingDraftLocale({ - documentLocalized: singleMeta.localized === true, - requestLocale: writeLocale ?? null, - defaultLocale: this.localization?.defaultLocale ?? null, - }); - const held = await new VersionsRepository( - this.adapter - ).findWorkingDraft( - { scopeKind: "single", scopeSlug: slug, entryId: promotedInto.id }, - gateLocale - ); - if (held) { - await assertDraftsMayBePromoted( - [{ locale: gateLocale ?? null, snapshot: held.snapshot }], - { - slug, - entryId: promotedInto.id, - fields: fieldConfigs, - user: options.user, - overrideAccess: options.overrideAccess, - // A snapshot belongs to this document, so its identity is this - // document's. Supplying both ahead of the spread satisfies the - // stored-document shape without asserting one, and lets the - // snapshot's own values win where it carries them. - toLogical: doc => - this.queryService.deserializeJsonFields( - { - id: promotedInto.id, - updatedAt: promotedInto.updatedAt, - ...doc, - }, - singleMeta.fields - ), - liveDocumentFor: async locale => - ( - await this.queryService.get(slug, { - locale: locale ?? undefined, - overrideAccess: true, - }) - ).data, - callerData: currentData, - localizedFieldNames, - enforceLocalizedRequired, - } - ); - } - } + // been judged only once, when it was SAVED. The gate that judges it + // again runs inside the write transaction, on the draft that transaction + // has locked. The one thing it cannot do there is resolve the caller's + // grants, which queries the pooled connection the transaction is + // holding, so that is resolved here and handed in. + const promoteGrants = callerAccessGrants( + options.user, + options.authenticatedScope + ); // 6.25. Single-level beforeChange hooks, on data the validation gate has // just passed. The declaration used to register onto `beforeUpdate`, @@ -1392,10 +1333,55 @@ export class SingleMutationService extends BaseService { singleDraftLocale ); if (pendingDraft) { + // Judged here, on the draft this transaction has LOCKED, and + // after the hooks that decide the status. Before the + // transaction the gate could only judge a copy of the world as + // it was: another writer saving a draft in between, or a + // `beforeChange` hook turning a status-less edit into a + // publish, would both leave the checked document and the + // written document different documents. + // Narrowed once for the gate's arguments; the fold below + // already runs under the same non-null condition. + const promotedInto = existingDoc; + await assertDraftsMayBePromoted( + [ + { + locale: singleDraftLocale ?? null, + snapshot: pendingDraft.snapshot, + }, + ], + { + slug, + entryId: promotedInto.id, + fields: fieldConfigs, + user: options.user, + overrideAccess: options.overrideAccess, + authenticatedScope: options.authenticatedScope, + // Resolved on the pooled connection before this + // transaction opened. + grants: promoteGrants, + // A snapshot belongs to this document, so its identity is + // this document's. Supplying both ahead of the spread + // satisfies the stored-document shape without asserting + // one, and lets the snapshot's own values win. + toLogical: doc => + this.queryService.deserializeJsonFields( + { + id: promotedInto.id, + updatedAt: promotedInto.updatedAt, + ...doc, + }, + singleMeta.fields + ), + liveStoredFor: () => Promise.resolve(promotedInto), + callerData: currentData, + localizedFieldNames, + enforceLocalizedRequired, + } + ); // The caller's own payload wins over the pending change: a // publish that also sets a field is saying something about that - // field now. What may be promoted at all was decided before - // this transaction opened, by the shared promote gate. + // field now. ({ main: mainPayload, companion: companionData } = splitPendingChange( pendingDraft.snapshot, diff --git a/packages/nextly/src/domains/singles/services/single-publish-all.ts b/packages/nextly/src/domains/singles/services/single-publish-all.ts index a3c29b9cae..603d8f5892 100644 --- a/packages/nextly/src/domains/singles/services/single-publish-all.ts +++ b/packages/nextly/src/domains/singles/services/single-publish-all.ts @@ -32,6 +32,10 @@ import type { DynamicSingleRecord } from "../../../schemas/dynamic-singles/types import type { FieldGroupDataService } from "../../../services/field-groups/field-group-data-service"; import { BaseService } from "../../../shared/base-service"; import { convertTimestampsToCamelCase } from "../../../shared/lib/case-conversion"; +import { + callerAccessGrants, + type CallerGrants, +} from "../../../shared/lib/field-level-registry"; import type { Logger } from "../../../shared/types"; import { readComponentSubtrees } from "../../field-groups/read-component-subtrees"; import { readCompanionLocaleStatusAll } from "../../i18n/companion-join"; @@ -165,19 +169,18 @@ export class SinglePublishAllService extends BaseService { const denial = await this.authorizePublish(plan, options); if (denial) return denial; - // Every language's pending change is promoted below, so every one of - // them is judged first, against the schema and the permissions as they - // stand now. Before the transaction, because a field's `access` rule and - // its `validate` are user code and resolving the caller's grants issues - // its own pooled queries: run inside the open transaction on a small - // pool they would wait for the connection it holds. - // - // The same gate the ordinary publish path runs. Two gates would be two - // answers to one question, and this is the path that had none. - await this.assertPendingChangesMayBePromoted(plan, options); + // Resolved here because the gate that uses them runs inside the write + // transaction, and resolving grants queries the pooled connection that + // transaction is holding. + const promoteGrants = callerAccessGrants( + options.user, + options.authenticatedScope + ); const outcome = await withVersionConflictRetry(() => - this.adapter.transaction(tx => this.commitPublish(tx, plan, options)) + this.adapter.transaction(tx => + this.commitPublish(tx, plan, options, promoteGrants) + ) ); // The document was deleted out from under the publish: nothing was @@ -389,7 +392,9 @@ export class SinglePublishAllService extends BaseService { private async commitPublish( tx: TransactionContext, plan: PublishPlan, - options: PublishAllSingleLocalesOptions + options: PublishAllSingleLocalesOptions, + /** Resolved before this transaction opened; see `publishAllLocales`. */ + promoteGrants?: () => Promise ): Promise { const { slug, singleMeta, existingDoc, hasMainStatus } = plan; @@ -423,7 +428,9 @@ export class SinglePublishAllService extends BaseService { tx, plan, lockedRow, - priorCompanionStatuses + priorCompanionStatuses, + options, + promoteGrants ); if (singleMeta.versions?.enabled) { @@ -471,7 +478,10 @@ export class SinglePublishAllService extends BaseService { tx: TransactionContext, plan: PublishPlan, lockedRow: Record, - priorCompanionStatuses: Map + priorCompanionStatuses: Map, + options: PublishAllSingleLocalesOptions, + /** Resolved before this transaction opened; see `publishAllLocales`. */ + promoteGrants?: () => Promise ): Promise<{ publishedRow: Record; mainRowTransitioned: boolean; @@ -506,7 +516,13 @@ export class SinglePublishAllService extends BaseService { // the statuses would all say published while the content each author was // holding stayed unapplied — the document would report itself fully // published and show none of the work being published. - await this.promotePendingChanges(tx, plan, publishNow); + await this.promotePendingChanges( + tx, + plan, + publishNow, + options, + promoteGrants + ); if (companion && plan.companionPublishable) { // Every stored translation moves in ONE statement, through the adapter's @@ -540,12 +556,22 @@ export class SinglePublishAllService extends BaseService { * transaction, so publishing some and refusing others would leave a caller * with no way to say what happened. */ + /** + * Judge every pending change this publish would promote, and refuse the + * whole publish if any of them may not be. + * + * All-or-nothing matches the write it guards: `promotePendingChanges` + * applies every language's change and deletes them all together, so + * publishing some and refusing others would leave a caller with no way to + * say what happened. + */ private async assertPendingChangesMayBePromoted( plan: PublishPlan, - options: PublishAllSingleLocalesOptions + pending: ReadonlyArray<{ locale: string | null; snapshot: unknown }>, + options: PublishAllSingleLocalesOptions, + promoteGrants?: () => Promise ): Promise { const { slug, singleMeta, existingDoc, companion, fieldConfigs } = plan; - if (singleMeta.versions?.drafts?.enabled !== true) return; const reader = this.queryService; // Constructed without a reader only by a caller that built this service // directly. The gate cannot run blind, and silently skipping it would make @@ -561,47 +587,30 @@ export class SinglePublishAllService extends BaseService { }); } - const pending = await new VersionsRepository( - this.adapter - ).findAllWorkingDrafts({ - scopeKind: "single", - scopeSlug: slug, + await assertDraftsMayBePromoted(pending, { + slug, entryId: existingDoc.id, + fields: fieldConfigs, + user: options.user, + overrideAccess: options.overrideAccess, + authenticatedScope: options.authenticatedScope, + grants: promoteGrants, + toLogical: doc => + reader.deserializeJsonFields( + { id: existingDoc.id, updatedAt: existingDoc.updatedAt, ...doc }, + singleMeta.fields + ), + // The stored row, in the shape a snapshot holds, so an untouched upload + // or relationship is compared identifier with identifier rather than + // identifier with the document a read would have expanded it into. + liveStoredFor: () => Promise.resolve(existingDoc), + localizedFieldNames: new Set( + (companion?.localizedFields ?? []).map(f => f.name) + ), + // Every language goes live here, so none of them is falling back to + // another and each one's required values are enforced. + enforceLocalizedRequired: true, }); - if (pending.length === 0) return; - - const localizedFieldNames = new Set( - (companion?.localizedFields ?? []).map(f => f.name) - ); - await assertDraftsMayBePromoted( - pending.map(draft => ({ - locale: draft.locale ?? null, - snapshot: draft.snapshot, - })), - { - slug, - entryId: existingDoc.id, - fields: fieldConfigs, - user: options.user, - overrideAccess: options.overrideAccess, - toLogical: doc => - reader.deserializeJsonFields( - { id: existingDoc.id, updatedAt: existingDoc.updatedAt, ...doc }, - singleMeta.fields - ), - liveDocumentFor: async locale => - ( - await reader.get(slug, { - locale: locale ?? undefined, - overrideAccess: true, - }) - ).data, - localizedFieldNames, - // This publish makes every language live, so every language's required - // values are enforced: none of them is falling back to another. - enforceLocalizedRequired: true, - } - ); } /** @@ -614,7 +623,9 @@ export class SinglePublishAllService extends BaseService { private async promotePendingChanges( tx: TransactionContext, plan: PublishPlan, - publishNow: Date + publishNow: Date, + options: PublishAllSingleLocalesOptions, + promoteGrants?: () => Promise ): Promise { const { slug, singleMeta, existingDoc, companion } = plan; if (singleMeta.versions?.drafts?.enabled !== true) return; @@ -628,6 +639,18 @@ export class SinglePublishAllService extends BaseService { const pending = await repo.findAllWorkingDrafts(ref); if (pending.length === 0) return; + // Judged here, on the drafts this transaction has read under its lock, and + // as ONE outcome: the loop below applies every language's snapshot to the + // same main row, so the shared values that survive are the last writer's. + // Judged one at a time, each draft can pass against its own shared values + // while the document that actually lands holds another language's. + await this.assertPendingChangesMayBePromoted( + plan, + pending, + options, + promoteGrants + ); + for (const draft of pending) { // The pending change carries no lifecycle of its own; the statuses this // publish writes are the ones that count. diff --git a/packages/nextly/src/domains/singles/services/single-query-service.ts b/packages/nextly/src/domains/singles/services/single-query-service.ts index 177f67a110..092fcd488a 100644 --- a/packages/nextly/src/domains/singles/services/single-query-service.ts +++ b/packages/nextly/src/domains/singles/services/single-query-service.ts @@ -66,14 +66,17 @@ import type { CollectionsHandler } from "../../../services/collections-handler"; import type { FieldGroupDataService } from "../../../services/field-groups/field-group-data-service"; import { BaseService } from "../../../shared/base-service"; import { convertTimestampsToCamelCase } from "../../../shared/lib/case-conversion"; -import type { ValidatableField } from "../../../shared/lib/entry-validation"; +import { + isEmptyRequiredValue, + type ValidatableField, +} from "../../../shared/lib/entry-validation"; import { applyFieldDefaults, cloneDefault, } from "../../../shared/lib/field-defaults"; import { applyFieldReadAccess, - readAccessGrants, + callerAccessGrants, runFieldHooks, type ReadAccessRedactions, } from "../../../shared/lib/field-level-registry"; @@ -423,6 +426,41 @@ function hasFieldName( return typeof field.name === "string" && field.name.length > 0; } +/** + * Whether a filled container is missing a child its own declaration requires. + * + * Only asked of a container the default fill created, so "missing" means no + * default reached it and no caller supplied it. A required child inside a + * nested container counts as well: an incomplete group two levels down is + * still a document the write path would refuse. + */ +function missesARequiredChild( + fields: readonly ValidatableField[] | undefined, + filled: unknown +): boolean { + if (!fields) return false; + if (Array.isArray(filled)) { + return filled.some(row => missesARequiredChild(fields, row)); + } + if (!isPlainRecord(filled)) return false; + for (const child of fields) { + if (!child.name) { + // A layout container holds no value; its children are on this object. + if (missesARequiredChild(child.fields, filled)) return true; + continue; + } + const value = filled[child.name]; + // The same question the write validator will ask of this document later. + // A nullish check calls a whitespace string or an empty array present, so + // the group would be stored and the next write would reject it. + if (child.required && isEmptyRequiredValue(value)) return true; + if (value !== undefined && missesARequiredChild(child.fields, value)) { + return true; + } + } + return false; +} + /** * A record the nested walk can read children off: an array is a repeater's * rows and is walked as rows, and a primitive holds no children at all. @@ -1108,7 +1146,7 @@ export class SingleQueryService extends BaseService { overrideAccess: skipFieldRules, // One grants resolver for both passes, so the caller's roles and // permissions are read once and both passes judge with one authority. - grants: readAccessGrants(fieldAccessUser), + grants: callerAccessGrants(fieldAccessUser), }; const sourceRedactions: ReadAccessRedactions = new WeakMap(); await applyFieldReadAccess(fieldAccess, sourceRedactions); @@ -1581,6 +1619,22 @@ export class SingleQueryService extends BaseService { applyFieldDefaults(logicalDefaults, [source]); const after = logicalDefaults[field.name]; if (after === before) return; + // A group the fill INVENTED has to be complete to be stored. This insert + // is direct and runs no validation pass, so a group created for the sake + // of one defaulted child, while a required sibling has no default and no + // value, would persist a document that the next create or update refuses. + // Left absent instead, which is where it stood before anything was + // filled, rather than stored knowing it is invalid. + if ( + before === undefined && + missesARequiredChild( + (source as { fields?: readonly ValidatableField[] }).fields, + after + ) + ) { + delete logicalDefaults[field.name]; + return; + } await assertNestedDefaultsValid( (source as { fields?: readonly ValidatableField[] }).fields, after, diff --git a/packages/nextly/src/init/reload-config.ts b/packages/nextly/src/init/reload-config.ts index 3275bf4034..4854f65227 100644 --- a/packages/nextly/src/init/reload-config.ts +++ b/packages/nextly/src/init/reload-config.ts @@ -1450,6 +1450,15 @@ async function applyReload(opts?: { * registered as that work happens and run together on abandonment. */ const reloadUndo: Array<() => void> = []; + /** + * Installs the field-level registry the newly read config describes. + * + * Set once that config has been read and collected, and called only from + * `commitReload`. A reload abandoned before it is set, or one that never + * commits, leaves the previous registry in force by never having replaced + * it, so no rule from a refused config decides a write. + */ + let commitFieldFunctions: (() => void) | undefined; /** * Put the field-type registry and the config's hooks back when a reload does * not take effect. @@ -1525,6 +1534,45 @@ async function applyReload(opts?: { singles: newConfig?.singles, fieldGroups: newConfig?.fieldGroups, }); + + // The field-level registry holds the function-bearing half of the live + // config: a field's `access` rules, its hooks, its `validate`, and a + // `defaultValue` written as a function. None of it survives being stored, + // so it is captured from the config object at boot and read from here on + // every write and every read. + // + // A reload never goes back through `registerServices`, so that capture was + // never refreshed. An edit to any of it kept running the version from + // process start until the dev server was restarted, which is wrong in the + // direction that matters most for an access rule: a rule TIGHTENED in the + // config was not the one being enforced. + // + // Staged here and installed by `commitReload`, the same boundary the + // config's hooks and the retention policies are published from. A reload + // whose DDL succeeds and whose metadata sync then fails does not commit, + // and a rule from a config the process refused must not be the one + // deciding writes in the meantime. + const { replaceFieldFunctions } = await import( + "../shared/lib/field-level-registry" + ); + const fieldFunctionSources = [ + ...(newConfig?.collections ?? []).map(entity => ({ + kind: "collection" as const, + slug: (entity as { slug?: string }).slug ?? "", + fields: (entity as { fields?: unknown[] }).fields ?? [], + })), + ...(newConfig?.singles ?? []).map(entity => ({ + kind: "single" as const, + slug: (entity as { slug?: string }).slug ?? "", + fields: (entity as { fields?: unknown[] }).fields ?? [], + })), + ...(newConfig?.fieldGroups ?? []).map(entity => ({ + kind: "fieldGroup" as const, + slug: (entity as { slug?: string }).slug ?? "", + fields: (entity as { fields?: unknown[] }).fields ?? [], + })), + ]; + commitFieldFunctions = () => replaceFieldFunctions(fieldFunctionSources); } catch (err) { // The registry was rebuilt from the config that just failed; put the // working set back so the retained config keeps the behavior it was @@ -1584,6 +1632,7 @@ async function applyReload(opts?: { if (committed) return; committed = true; commitConfigHooks(); + commitFieldFunctions?.(); // Published here rather than when the file is read. A reload that is later // refused still parsed a valid config, and publishing early would leave a // policy the process explicitly rejected in force — deleting on windows diff --git a/packages/nextly/src/shared/lib/__tests__/field-functions-replace.test.ts b/packages/nextly/src/shared/lib/__tests__/field-functions-replace.test.ts new file mode 100644 index 0000000000..1490a466f0 --- /dev/null +++ b/packages/nextly/src/shared/lib/__tests__/field-functions-replace.test.ts @@ -0,0 +1,80 @@ +/** + * The field-function registry is REPLACED from the config in force. + * + * A reload re-reads the config and installs what it describes at the reload's + * commit point. Two things have to be true of that install. An entity the new + * config no longer declares must stop deciding anything: a collection dropped + * from `nextly.config.ts` keeps its registry row and its table so an orphan + * sweep can find them, so it stays addressable, and a rule left behind here + * would keep running. And nothing may be installed by a reload that does not + * commit, or a rule from a config the process refused would decide writes + * while every other service ran the previous one. + */ +import { describe, expect, it } from "vitest"; + +import { + getFieldFunctions, + registerFieldFunctions, + replaceFieldFunctions, +} from "../field-level-registry"; + +describe("replaceFieldFunctions", () => { + it("drops an entity the new config no longer declares", () => { + registerFieldFunctions("collection", "kept", [ + { name: "tone", type: "text", defaultValue: () => "before" }, + ]); + registerFieldFunctions("collection", "removed", [ + { name: "tone", type: "text", defaultValue: () => "gone" }, + ]); + + replaceFieldFunctions([ + { + kind: "collection", + slug: "kept", + fields: [{ name: "tone", type: "text", defaultValue: () => "after" }], + }, + ]); + + expect( + getFieldFunctions("collection", "kept")?.tone?.defaultValue?.({}) + ).toBe("after"); + // Not merely stale: absent, so nothing of it runs. + expect(getFieldFunctions("collection", "removed")).toBeUndefined(); + }); + + it("keeps the kinds apart", () => { + replaceFieldFunctions([ + { + kind: "collection", + slug: "shared", + fields: [{ name: "tone", type: "text", defaultValue: () => "coll" }], + }, + { + kind: "single", + slug: "shared", + fields: [{ name: "tone", type: "text", defaultValue: () => "single" }], + }, + ]); + + expect( + getFieldFunctions("collection", "shared")?.tone?.defaultValue?.({}) + ).toBe("coll"); + expect( + getFieldFunctions("single", "shared")?.tone?.defaultValue?.({}) + ).toBe("single"); + }); + + it("installs nothing for an entity that declares no functions", () => { + registerFieldFunctions("collection", "plain", [ + { name: "tone", type: "text", defaultValue: () => "x" }, + ]); + replaceFieldFunctions([ + { + kind: "collection", + slug: "plain", + fields: [{ name: "tone", type: "text" }], + }, + ]); + expect(getFieldFunctions("collection", "plain")).toBeUndefined(); + }); +}); diff --git a/packages/nextly/src/shared/lib/__tests__/field-level-registry.test.ts b/packages/nextly/src/shared/lib/__tests__/field-level-registry.test.ts index 61346feeaf..ff47c655a6 100644 --- a/packages/nextly/src/shared/lib/__tests__/field-level-registry.test.ts +++ b/packages/nextly/src/shared/lib/__tests__/field-level-registry.test.ts @@ -27,7 +27,7 @@ import { attachFieldValidators, clearFieldFunctions, getFieldFunctions, - readAccessGrants, + callerAccessGrants, type ReadAccessRedactions, registerFieldFunctions, runFieldHooks, @@ -138,7 +138,7 @@ describe("field access can ask what the caller is granted", () => { }, }, ]); - const grants = readAccessGrants({ id: "u1" }); + const grants = callerAccessGrants({ id: "u1" }); const entry: Record = { secret: "s" }; await applyFieldReadAccess({ kind: "collection", diff --git a/packages/nextly/src/shared/lib/entry-validation.ts b/packages/nextly/src/shared/lib/entry-validation.ts index 7e5a3148e1..0800b46c1c 100644 --- a/packages/nextly/src/shared/lib/entry-validation.ts +++ b/packages/nextly/src/shared/lib/entry-validation.ts @@ -146,6 +146,19 @@ function requiredIsEnforced( return true; } +/** + * What "no value" means for a required field, in one place. + * + * Exported because a caller deciding whether a document it assembled would + * satisfy the schema has to ask the same question this validator will ask of + * it later. A narrower second implementation (a nullish check, say) calls a + * whitespace string or an empty array present, stores the document, and the + * next write rejects it. + */ +export function isEmptyRequiredValue(value: unknown): boolean { + return isEmpty(value); +} + function isEmpty(value: unknown): boolean { return ( value === undefined || diff --git a/packages/nextly/src/shared/lib/field-defaults.ts b/packages/nextly/src/shared/lib/field-defaults.ts index 96785d49fa..c5e084d9e7 100644 --- a/packages/nextly/src/shared/lib/field-defaults.ts +++ b/packages/nextly/src/shared/lib/field-defaults.ts @@ -62,14 +62,36 @@ const NON_COLUMN_TYPES: ReadonlySet = new Set(fieldGroupFieldTypes); export function applyFieldDefaults( data: Record, fields: readonly ValidatableField[], - functions?: DefaultFunctions + functions?: DefaultFunctions, + /** + * What a FUNCTION default reads, when that is not the record being filled. + * + * A function default receives the document built so far, and on a create + * that document still holds every value the caller sent, including one a + * field rule denies them. The caller passes a filtered view here so a + * default cannot read a value its writer was not allowed to send, while the + * record being filled keeps that value for the authoritative access pass to + * remove. Filtering the record itself instead would delete the value before + * the defaults that decide whether it is allowed have run at all. + * + * The view arrives ALREADY DEFAULTED and already judged: its caller fills it + * with this same function and then runs the field rules over it, so what it + * holds is what this caller may store, defaults included. That is what makes + * a default the rules remove from it stay removed here. + * + * Defaults to the record, which is right wherever there is nothing to hide. + */ + view?: Record ): void { + const readFrom = view ?? data; for (const field of fields) { // A layout container (row, tabs, collapsible) groups fields visually // without holding a value: its children are stored on the parent, so they // are filled against the same object. if (!field.name) { - if (field.fields) applyFieldDefaults(data, field.fields, functions); + if (field.fields) { + applyFieldDefaults(data, field.fields, functions, readFrom); + } continue; } if (NON_COLUMN_TYPES.has(field.type)) continue; @@ -90,11 +112,28 @@ export function applyFieldDefaults( if (declared !== undefined && supplied === undefined) { // Resolved against the data built so far, so a default may read the // values the caller did supply, and earlier defaults in this same pass. - data[field.name] = cloneDefault( + const filled = cloneDefault( typeof declared === "function" - ? (declared as (d: Record) => unknown)(data) + ? (declared as (d: Record) => unknown)(readFrom) : declared ); + data[field.name] = filled; + // The view follows what has been filled, so a later function default + // reading an earlier one sees the value the record will actually store + // rather than `undefined` from a document that visibly holds it. + // + // Only a key the view still HOLDS is refreshed. The view was defaulted + // and judged before this pass, so a default missing from it is one the + // rules removed, and writing the record's copy back would hand a later + // default a value this caller may not write. It would then be carried + // into a field they can, stored there, and survive the access pass that + // removes only the field it came from. + if ( + readFrom !== data && + Object.prototype.hasOwnProperty.call(readFrom, field.name) + ) { + readFrom[field.name] = filled; + } } if (!field.fields) continue; @@ -103,9 +142,12 @@ export function applyFieldDefaults( // before it runs or a required child fails on an entry the caller could // not have satisfied. if (field.type === "group") { - fillGroup(data, field.name, field.fields, own?.fields); + // `view`, not `readFrom`: with no explicit view the recursion must + // read the copy it fills, or it would write defaults into the caller's + // own nested container. + fillGroup(data, field.name, field.fields, own?.fields, view); } else if (field.type === "repeater") { - fillRepeaterRows(data, field.name, field.fields, own?.fields); + fillRepeaterRows(data, field.name, field.fields, own?.fields, view); } } } @@ -143,8 +185,11 @@ function fillGroup( data: Record, name: string, fields: readonly ValidatableField[], - functions: DefaultFunctions | undefined + functions: DefaultFunctions | undefined, + /** The parent's view; a child reads the view's matching container. */ + view?: Record ): void { + const childView = isPlainObject(view?.[name]) ? view[name] : undefined; const existing = Object.prototype.hasOwnProperty.call(data, name) ? data[name] : undefined; @@ -154,7 +199,7 @@ function fillGroup( // A shallow copy per level is enough, because each level down copies again // before it writes. const filled = { ...existing }; - applyFieldDefaults(filled, fields, functions); + applyFieldDefaults(filled, fields, functions, childView); data[name] = filled; return; } @@ -163,7 +208,7 @@ function fillGroup( if (existing !== undefined) return; const seeded: Record = {}; - applyFieldDefaults(seeded, fields, functions); + applyFieldDefaults(seeded, fields, functions, childView); if (Object.keys(seeded).length > 0) data[name] = seeded; } @@ -178,17 +223,26 @@ function fillRepeaterRows( data: Record, name: string, fields: readonly ValidatableField[], - functions: DefaultFunctions | undefined + functions: DefaultFunctions | undefined, + /** The parent's view; each row reads the view's row at the same index. */ + view?: Record ): void { + const viewRows = Array.isArray(view?.[name]) ? view[name] : undefined; const value = data[name]; if (!Array.isArray(value)) return; // Rows are the caller's objects, so each one that gains a default is filled // through a copy and the list is rebuilt from the results. let changed = false; - const rows = value.map(row => { + const rows = value.map((row, index) => { if (!isPlainObject(row)) return row; const filled = { ...row }; - applyFieldDefaults(filled, fields, functions); + const rowView = viewRows?.[index]; + applyFieldDefaults( + filled, + fields, + functions, + isPlainObject(rowView) ? rowView : undefined + ); changed = true; return filled; }); diff --git a/packages/nextly/src/shared/lib/field-level-registry.ts b/packages/nextly/src/shared/lib/field-level-registry.ts index 8c19e69d53..edf5a92b37 100644 --- a/packages/nextly/src/shared/lib/field-level-registry.ts +++ b/packages/nextly/src/shared/lib/field-level-registry.ts @@ -40,6 +40,7 @@ import { listEffectivePermissions, listRoleSlugsForUser, } from "../../services/lib/permissions"; +import { addressableFields } from "../addressable-fields"; import { detachData } from "./detach"; import type { ValidatableField } from "./entry-validation"; @@ -67,7 +68,7 @@ type FieldAccessFn = (args: { * the single source for the string, so this carries what it returns rather * than re-deriving it. */ -interface CallerGrants { +export interface CallerGrants { permissions: string[]; roles: string[]; } @@ -156,7 +157,17 @@ export interface FieldFunctions { fields?: Record; } -type EntityKind = "collection" | "single"; +/** + * What a registration is keyed by. + * + * A field group joins the two entity kinds because it has the same problem and + * only the registry solves it: its fields are read from a stored definition on + * every write, and a function does not survive being stored. What is CONSUMED + * from a field group's entry today is the `defaultValue` alone. Its `access` + * rules and hooks are captured by the same walk but nothing reads them for a + * field group, so registering one does not mean they are enforced. + */ +type EntityKind = "collection" | "single" | "fieldGroup"; type Store = Map>; const GLOBAL_KEY = "__nextlyFieldFunctionRegistry"; @@ -220,14 +231,46 @@ function extractFieldFunctions( return hasAny ? out : undefined; } +/** + * An unnamed GROUP is layout: its children are stored at the level the group + * sits in, which is the level this map is keyed by. An unnamed REPEATER is not: + * its children are stored PER ROW, so flattening them here would file a row's + * field under the parent's name and hand its rules the wrong object. + * + * The same distinction `collection-sources` draws, and it has to be made DURING + * the walk: the walk emits the children themselves, so by the time a predicate + * could read the result, a field reached through an unnamed repeater is + * indistinguishable from one reached through an unnamed group. + */ +function storedAtThisLevel(container: { type?: unknown }): boolean { + return container.type === "group"; +} + +/** + * The function-bearing fields at one level, keyed by name. + * + * Flattened through the shared `addressableFields` walk rather than a loop over + * the array, because an unnamed presentational container stores its children at + * the level it sits in. A loop over named entries drops every rule declared + * inside one: not just a `defaultValue`, but the `access` rule deciding whether + * a caller may read or write that field, and the hooks that shape it. The data + * side already flattens these containers, so the two now agree. + * + * `defineCollection` refuses a field with no name, so the documented code-first + * path cannot build this shape. A plugin contributing raw config can: + * `assertPluginFieldDeclarations` checks each field's TYPE and not its name, so + * an unnamed `group` reaches the live config and this map. Silently dropping + * the access rules inside it is the failure worth spending a walk to avoid. + */ function collectFieldFunctions( fields: unknown[] ): Record | undefined { const map: Record = {}; let hasAny = false; - for (const raw of fields) { - if (raw === null || typeof raw !== "object") continue; - const field = raw as Record; + for (const raw of addressableFields(fields, { + descendInto: storedAtThisLevel, + })) { + const field = raw as unknown as Record; if (typeof field.name !== "string" || !field.name) continue; const fns = extractFieldFunctions(field); if (fns) { @@ -255,6 +298,42 @@ export function registerFieldFunctions( } } +/** One entity's live config, for a wholesale registry replacement. */ +export interface FieldFunctionSource { + kind: EntityKind; + slug: string; + fields: unknown[]; +} + +/** + * Rebuild the whole registry from the config that is now in force. + * + * Replacing rather than adding, because a reload can REMOVE an entity: a + * collection dropped from `nextly.config.ts` keeps its registry row and its + * table so an orphan sweep can find them later, so it stays addressable, and + * an entry left behind here would keep deciding its access, running its hooks + * and filling its defaults from a config that no longer declares it. + * + * Called at the reload's commit point, not when the new config is read. A + * reload that is refused after its DDL has run still parsed a valid config, + * and installing these early would leave a rule the process explicitly + * rejected deciding writes while every other service still ran the previous + * one. + */ +export function replaceFieldFunctions( + sources: readonly FieldFunctionSource[] +): void { + const next: Store = new Map(); + for (const source of sources) { + if (!source.slug || !Array.isArray(source.fields)) continue; + const map = collectFieldFunctions(source.fields); + if (map) next.set(key(source.kind, source.slug), map); + } + const current = store(); + current.clear(); + for (const [k, v] of next) current.set(k, v); +} + export function getFieldFunctions( kind: EntityKind, slug: string @@ -287,7 +366,16 @@ function attachValidators( fns: Record ): ValidatableField[] { return fields.map(field => { - const entry = field.name ? fns[field.name] : undefined; + // An unnamed container holds no entry of its own, and its children are + // registered at THIS level because that is where their values are stored. + // Descending with the same map is what makes the two agree; returning it + // untouched left a validator captured at registration unattached, so a + // value it should have rejected was accepted. + if (!field.name) { + if (!field.fields || !storedAtThisLevel(field)) return field; + return { ...field, fields: attachValidators(field.fields, fns) }; + } + const entry = fns[field.name]; if (!entry) return field; const next: ValidatableField = { ...field }; if (entry.validate) next.validate = entry.validate; @@ -432,6 +520,13 @@ export async function applyFieldWriteAccess(opts: { authenticatedScope?: AuthenticatedScope; overrideAccess?: boolean; id?: string; + /** + * A resolver shared with another pass over the same write, so the caller's + * roles and permissions are read once and both passes judge with one + * authority. Build it with {@link callerAccessGrants}. Omitted, this pass + * builds its own, which is right for a write judged only once. + */ + grants?: () => Promise; }): Promise { // Bypass for a caller that has SAID it is trusted, and only that. The read // side gates on the same single condition, and the two must agree: a rule @@ -458,10 +553,12 @@ export async function applyFieldWriteAccess(opts: { // answers `{ permissions: [], roles: [] }` — so a rule written as // `({ permissions }) => permissions.includes(...)` refuses it, which is // the correct answer rather than an accident of the shape. - grants: grantsResolver( - typeof opts.user?.id === "string" ? opts.user.id : undefined, - opts.authenticatedScope - ), + grants: + opts.grants ?? + grantsResolver( + typeof opts.user?.id === "string" ? opts.user.id : undefined, + opts.authenticatedScope + ), }); } @@ -667,11 +764,16 @@ async function applyReadAccessRec( } /** - * The caller's grants resolver for a read, to hand to every field-access pass - * over one document so roles and permissions are read once. Memoised on first - * use, like the resolver each pass would otherwise build for itself. + * The caller's grants resolver, to hand to every field-access pass over one + * document or record so roles and permissions are read once. + * + * One function for reads and writes, because they ask the same question of the + * same authority. Two constructors of it drifted apart the moment either was + * changed: a fix to how an anonymous caller or an API key resolves would land + * on one side and leave the other judging by different rules, which is the + * shape of an access bug nobody sees until it is exploited. */ -export function readAccessGrants( +export function callerAccessGrants( user: Record | undefined, authenticatedScope?: AuthenticatedScope ): () => Promise { @@ -704,7 +806,7 @@ export async function applyFieldReadAccess( /** * A resolver shared with another pass over the same document, so the two * passes read the caller's roles and permissions once and judge with one - * authority; see {@link readAccessGrants}. + * authority; see {@link callerAccessGrants}. */ grants?: () => Promise; },