Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions .changeset/single-write-response-related-access.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
---
"nextly": patch
"create-nextly-app": patch
"@nextlyhq/admin": patch
"@nextlyhq/admin-css": patch
"@nextlyhq/blocks-engine": patch
"@nextlyhq/ui": patch
"@nextlyhq/adapter-drizzle": patch
"@nextlyhq/adapter-postgres": patch
"@nextlyhq/adapter-mysql": patch
"@nextlyhq/adapter-sqlite": patch
"@nextlyhq/storage-s3": patch
"@nextlyhq/storage-uploadthing": patch
"@nextlyhq/storage-vercel-blob": patch
"@nextlyhq/plugin-form-builder": patch
"@nextlyhq/plugin-page-builder": patch
"@nextlyhq/plugin-seo": patch
"@nextlyhq/plugin-sdk": patch
"@nextlyhq/eslint-config": patch
"@nextlyhq/prettier-config": patch
"@nextlyhq/telemetry": patch
"@nextlyhq/tsconfig": patch
---

Updating a Single no longer returns related fields the writer is not allowed to read.

The response expands relationships, and those rows belong to another collection carrying its own field-level `access.read` rules. Read paths have evaluated them since the field-access work landed; this path forwarded no caller, so it returned every related field intact — including ones the same caller's `GET` would withhold. That made the write path a way around the rule: write anything, read the response back.

A writer supplied a relationship id, not the related row's protected fields, so "they supplied the data" does not cover them. The rule that applies is the target collection's own, and it is now evaluated against the caller that made the write.

This reaches every hop the response expands, not only the first: a related row's own relationships carry the rules of the collection at the far end, and those are evaluated too.

Enforcement is keyed on there being a caller to judge. A trusted write (`overrideAccess`) and an internal write that forwards no user are unaffected — stripping there would hide fields from a writer nothing denied.
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
/**
* A write response is a read of the rows it expands.
*
* Updating a Single returns the document, relationships expanded. Those related
* rows belong to another collection and carry that collection's own field-level
* `access.read` rules — rules the read paths have evaluated since #335, and
Comment thread
mobeenabdullah marked this conversation as resolved.
Outdated
Comment thread
mobeenabdullah marked this conversation as resolved.
Outdated
* which this path did not, because it forwarded no caller.
*
* The writer supplied a relationship id, not the related row's protected
* fields, so "they just wrote it" does not cover them. Leaving the response
* unredacted makes the write path a way around the rule: write anything, read
* the response, see what a GET would refuse.
*/

import { afterEach, describe, expect, it } from "vitest";

import {
checkbox,
defineCollection,
defineSingle,
relationship,
text,
} from "../../../config";
import {
createTestNextly,
type TestNextly,
} from "../../../plugins/test-nextly";
import type { CollectionsHandler } from "../../../services/collections-handler";
import type { SingleEntryService } from "../services/single-entry-service";

let current: TestNextly | undefined;
afterEach(async () => {
await current?.destroy();
current = undefined;
});

/** A Single pointing at a collection that hides one of its fields. */
async function boot(): Promise<{
entry: SingleEntryService;
authorId: string;
}> {
current = await createTestNextly({
collections: [
defineCollection({
slug: "authors",
fields: [
text({ name: "name" }),
checkbox({ name: "suspended", access: { read: () => false } }),
],
}),
],
singles: [
defineSingle({
slug: "branding",
fields: [
text({ name: "siteName" }),
relationship({ name: "author", relationTo: "authors" }),
],
}),
],
});

const handler = current.getService<CollectionsHandler>("collectionsHandler");
const author = await handler.createEntry(
{ collectionName: "authors", overrideAccess: true },
{ name: "Ada", suspended: true }
);
return {
entry: current.getService<SingleEntryService>("singleEntryService"),
authorId: (author.data as { id: string }).id,
};
}

describe("single write response — related-row field access (integration)", () => {
it("withholds a related field the writer may not read", async () => {
const { entry, authorId } = await boot();

const result = await entry.update(
"branding",
{ siteName: "Acme", author: authorId },
{ user: { id: "writer-1" }, routeAuthorized: true }
);

expect(result.success).toBe(true);
const author = (result.data as { author?: Record<string, unknown> })
?.author;
// The relationship is still expanded — only the protected field is gone.
expect(author).toMatchObject({ name: "Ada" });
expect(author).not.toHaveProperty("suspended");
});

it("withholds a protected field a hop further out", async () => {
// Nested expansion reaches the related row's own relationships, so the
// rules of the collection at the far end apply too. Threading the caller
// into the first hop is only half the answer if it stops there.
current = await createTestNextly({
collections: [
defineCollection({
slug: "orgs",
fields: [
text({ name: "name" }),
checkbox({ name: "blocked", access: { read: () => false } }),
],
}),
defineCollection({
slug: "authors",
fields: [
text({ name: "name" }),
relationship({ name: "org", relationTo: "orgs" }),
],
}),
],
singles: [
defineSingle({
slug: "branding",
fields: [
text({ name: "siteName" }),
relationship({ name: "author", relationTo: "authors" }),
],
}),
],
});
const handler =
current.getService<CollectionsHandler>("collectionsHandler");
const org = await handler.createEntry(
{ collectionName: "orgs", overrideAccess: true },
{ name: "Acme", blocked: true }
);
const author = await handler.createEntry(
{ collectionName: "authors", overrideAccess: true },
{ name: "Ada", org: (org.data as { id: string }).id }
);
const entry = current.getService<SingleEntryService>("singleEntryService");

const result = await entry.update(
"branding",
{ siteName: "Acme", author: (author.data as { id: string }).id },
{ user: { id: "writer-1" }, routeAuthorized: true }
);

expect(result.success).toBe(true);
const nested = (
result.data as { author?: { org?: Record<string, unknown> } }
)?.author?.org;
expect(nested).toMatchObject({ name: "Acme" });
expect(nested).not.toHaveProperty("blocked");
});

it("leaves a trusted write's response whole", async () => {
// The mirror, and the reason enforcement is not simply switched on
// everywhere: a trusted write supplies no caller to judge, and stripping
// there would hide fields from an internal writer that nothing denied.
const { entry, authorId } = await boot();

const result = await entry.update(
"branding",
{ siteName: "Acme", author: authorId },
{ overrideAccess: true }
);

expect(result.success).toBe(true);
const author = (result.data as { author?: Record<string, unknown> })
?.author;
expect(author).toMatchObject({ name: "Ada", suspended: true });
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -1883,10 +1883,28 @@ export class SingleMutationService extends BaseService {
fieldConfigs
);

// 10.6. Expand relationship fields with full related entry data
// 10.6. Expand relationship fields with full related entry data.
//
// The rows this pulls in belong to another collection and carry that
// collection's field rules. A writer supplied a relationship id, not the
// related row's protected fields, so returning them here would answer a
// question the same caller's GET refuses — the write path is not a way
// around the rule.
//
// Enforcement is keyed on there being a caller to judge. A trusted or
// system write forwards none, and stripping for it would hide fields from
// an internal writer that nothing denied.
updatedDoc = await this.queryService.expandRelationshipFields(
updatedDoc,
fieldConfigs
fieldConfigs,
// The write response has no depth option of its own; expansion applies
// its own default.
undefined,
{
enforceFieldAccess: Boolean(options.user),
Comment thread
mobeenabdullah marked this conversation as resolved.
Outdated
Comment thread
mobeenabdullah marked this conversation as resolved.
Outdated
user: options.user,
overrideAccess: options.overrideAccess,
Comment thread
mobeenabdullah marked this conversation as resolved.
Outdated
}
);

// 11. Execute afterChange hooks (afterUpdate equivalent for Singles)
Expand Down
Loading