From 6b107bb729e7a1d041097916e9491cdbaed020cf Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sat, 15 Aug 2026 11:58:57 +0500 Subject: [PATCH 1/9] feat(nextly): resolve an entry preview url in one place A collection may declare a preview two ways and they are disjoint: a code-first collection writes a function of the entry, a UI-created one writes a template string because it has nowhere to put a function. Both answered the same question in different places, so both are answered here and each caller asks rather than deciding. Resolution runs on the server because the function only exists in the server module graph. That also removes a permission the browser could not satisfy: settings is a system resource the editor and author presets do not grant, and those are the roles that share preview links. Resolving here means a caller needs collection access and never settings. The result is a four-case union rather than a nullable string. Three of the cases render as no button, but noSiteUrl is the one where a host is guessable and the guess is the admin origin, so it carries its own value and the path it could not base. --- .../__tests__/preview-url-resolver.test.ts | 251 ++++++++++++++++++ .../services/preview-url-resolver.ts | 186 +++++++++++++ 2 files changed, 437 insertions(+) create mode 100644 packages/nextly/src/domains/collections/services/__tests__/preview-url-resolver.test.ts create mode 100644 packages/nextly/src/domains/collections/services/preview-url-resolver.ts diff --git a/packages/nextly/src/domains/collections/services/__tests__/preview-url-resolver.test.ts b/packages/nextly/src/domains/collections/services/__tests__/preview-url-resolver.test.ts new file mode 100644 index 0000000000..2347ccd3db --- /dev/null +++ b/packages/nextly/src/domains/collections/services/__tests__/preview-url-resolver.test.ts @@ -0,0 +1,251 @@ +import { describe, expect, it } from "vitest"; + +import { + hasPreviewConfigured, + resolvePreviewUrl, +} from "../preview-url-resolver"; + +const SITE = "https://example.com"; + +describe("hasPreviewConfigured", () => { + it("is false for a collection that declares no preview", () => { + expect(hasPreviewConfigured(undefined)).toBe(false); + expect(hasPreviewConfigured({})).toBe(false); + }); + + it("is true for either authoring path", () => { + expect(hasPreviewConfigured({ url: () => "/p" })).toBe(true); + expect(hasPreviewConfigured({ urlTemplate: "/p/{slug}" })).toBe(true); + }); + + it("is false for an empty template rather than merely present", () => { + // A stored empty string is what a UI field yields when it is cleared, and + // treating it as "configured" would persist hasPreview: true for a + // collection whose button can never resolve. + expect(hasPreviewConfigured({ urlTemplate: "" })).toBe(false); + }); + + it("agrees with the resolver about what counts as configured", () => { + // The stored boolean and the resolution must not drift: anything this + // predicate calls configured must resolve to something other than + // notConfigured, and vice versa. + const cases: Parameters[0][] = [ + undefined, + {}, + { urlTemplate: "" }, + { url: () => "/p" }, + { urlTemplate: "/p/{slug}" }, + ]; + + expect(cases.length).toBeGreaterThan(0); + for (const preview of cases) { + const resolution = resolvePreviewUrl({ + preview, + entry: { slug: "hello" }, + siteUrl: SITE, + }); + expect(resolution.status === "notConfigured").toBe( + !hasPreviewConfigured(preview) + ); + } + }); +}); + +describe("resolvePreviewUrl", () => { + it("reports notConfigured when nothing declares a preview", () => { + expect( + resolvePreviewUrl({ preview: undefined, entry: {}, siteUrl: SITE }) + ).toEqual({ status: "notConfigured" }); + }); + + it("resolves a code-first function against the site URL", () => { + expect( + resolvePreviewUrl({ + preview: { url: entry => `/posts/${String(entry.slug)}` }, + entry: { slug: "hello" }, + siteUrl: SITE, + }) + ).toEqual({ status: "resolved", url: "https://example.com/posts/hello" }); + }); + + it("resolves a template against the site URL", () => { + expect( + resolvePreviewUrl({ + preview: { urlTemplate: "/preview/{slug}" }, + entry: { slug: "hello" }, + siteUrl: SITE, + }) + ).toEqual({ status: "resolved", url: "https://example.com/preview/hello" }); + }); + + it("prefers the function when a declaration somehow carries both", () => { + expect( + resolvePreviewUrl({ + preview: { url: () => "/from-function", urlTemplate: "/from-template" }, + entry: {}, + siteUrl: SITE, + }) + ).toEqual({ status: "resolved", url: "https://example.com/from-function" }); + }); + + it("reports unavailable when the authored function declines", () => { + expect( + resolvePreviewUrl({ + preview: { url: () => null }, + entry: {}, + siteUrl: SITE, + }) + ).toEqual({ status: "unavailable" }); + }); + + it("reports unavailable when the authored function throws", () => { + // User code runs inside a request here. A throw is the collection failing to + // produce a URL, not the server failing, so it must not escape as a 500. + expect( + resolvePreviewUrl({ + preview: { + url: () => { + throw new Error("author bug"); + }, + }, + entry: {}, + siteUrl: SITE, + }) + ).toEqual({ status: "unavailable" }); + }); + + it("reports unavailable when a template placeholder has no value yet", () => { + for (const slug of [undefined, null, ""]) { + expect( + resolvePreviewUrl({ + preview: { urlTemplate: "/preview/{slug}" }, + entry: { slug }, + siteUrl: SITE, + }) + ).toEqual({ status: "unavailable" }); + } + }); + + it("interpolates falsy-but-real values instead of suppressing the URL", () => { + // A truthiness test here would drop a legitimate id of 0, which reads as + // "this entry cannot be previewed" for the one entry that can. + expect( + resolvePreviewUrl({ + preview: { urlTemplate: "/p/{id}" }, + entry: { id: 0 }, + siteUrl: SITE, + }) + ).toEqual({ status: "resolved", url: "https://example.com/p/0" }); + }); + + it("refuses an object placeholder rather than emitting [object Object]", () => { + const resolution = resolvePreviewUrl({ + preview: { urlTemplate: "/p/{ref}" }, + entry: { ref: { id: 1 } }, + siteUrl: SITE, + }); + expect(resolution).toEqual({ status: "unavailable" }); + }); + + it("refuses value types with no meaningful string form", () => { + // A denylist that excluded objects would let each of these through, and the + // function case is the sharp one: String(fn) is the function's source text, + // which is long, valid-looking, and silently wrong in a URL. + const rejected: unknown[] = [ + () => "x", + Symbol("s"), + { id: 1 }, + [1, 2], + Number.NaN, + Number.POSITIVE_INFINITY, + ]; + + expect(rejected.length).toBeGreaterThan(0); + for (const ref of rejected) { + expect( + resolvePreviewUrl({ + preview: { urlTemplate: "/p/{ref}" }, + entry: { ref }, + siteUrl: SITE, + }) + ).toEqual({ status: "unavailable" }); + } + }); + + it("escapes interpolated values so they cannot alter the path shape", () => { + const resolution = resolvePreviewUrl({ + preview: { urlTemplate: "/p/{slug}" }, + entry: { slug: "a/../b" }, + siteUrl: SITE, + }); + expect(resolution).toEqual({ + status: "resolved", + url: "https://example.com/p/a%2F..%2Fb", + }); + }); + + // The separating property. Every case below produces "no usable URL", and a + // resolver that answered null for all of them would pass any test asserting + // only absence. What distinguishes this one is that a host IS guessable here + // and guessing yields the ADMIN's origin, which is confidently wrong — so the + // status must be its own value and must carry the path forward. + it("reports noSiteUrl, distinctly from unavailable, when no host is known", () => { + const resolution = resolvePreviewUrl({ + preview: { url: () => "/posts/hello" }, + entry: {}, + siteUrl: null, + }); + + expect(resolution).toEqual({ status: "noSiteUrl", path: "/posts/hello" }); + expect(resolution.status).not.toBe("unavailable"); + expect(resolution.status).not.toBe("resolved"); + }); + + it("passes an absolute authored URL through even with no site configured", () => { + // An author who wrote a full URL named the host on purpose; re-basing it + // against the configured site would override a deliberate choice. + expect( + resolvePreviewUrl({ + preview: { url: () => "https://staging.example.org/p/1" }, + entry: {}, + siteUrl: null, + }) + ).toEqual({ status: "resolved", url: "https://staging.example.org/p/1" }); + }); + + it("joins base and path without doubling or dropping the separator", () => { + const cases = [ + { siteUrl: "https://example.com/", path: "/p" }, + { siteUrl: "https://example.com", path: "p" }, + { siteUrl: "https://example.com///", path: "p" }, + ]; + + expect(cases.length).toBeGreaterThan(0); + for (const { siteUrl, path } of cases) { + expect( + resolvePreviewUrl({ preview: { url: () => path }, entry: {}, siteUrl }) + ).toEqual({ status: "resolved", url: "https://example.com/p" }); + } + }); + + it("hands the authored function the entry it was given", () => { + // Observed rather than reconstructed: asserting on the argument the real + // call receives is what keeps this honest if the call site starts reshaping + // the entry before passing it on. + const seen: Record[] = []; + const entry = { slug: "hello", status: "draft" }; + + resolvePreviewUrl({ + preview: { + url: received => { + seen.push(received); + return "/p"; + }, + }, + entry, + siteUrl: SITE, + }); + + expect(seen).toEqual([entry]); + }); +}); diff --git a/packages/nextly/src/domains/collections/services/preview-url-resolver.ts b/packages/nextly/src/domains/collections/services/preview-url-resolver.ts new file mode 100644 index 0000000000..de42ac35e9 --- /dev/null +++ b/packages/nextly/src/domains/collections/services/preview-url-resolver.ts @@ -0,0 +1,186 @@ +/** + * Where an entry previews: one answer, derived once. + * + * A collection may declare a preview two ways, and they are disjoint by + * construction. A code-first collection declares `url`, a function of the entry + * that only exists in the server's module graph. A UI-created collection + * declares `urlTemplate`, a string with `{field}` placeholders, because there is + * nowhere for it to write a function. Both answer the same question, so both are + * answered here rather than at each call site. + * + * The function is why this runs on the server at all. No column can hold it, so + * the admin cannot read it back from the registry the way it reads every other + * `admin` option — it has to ASK. That has a second consequence worth stating + * plainly, because it is the reason the site URL is read here and not in the + * browser: resolving in the admin would mean the admin reading site settings, + * and `settings` is a system resource that the `editor` and `author` presets do + * not grant. Those are precisely the roles that share preview links. Resolving + * server-side means the caller needs access to the COLLECTION and never to + * settings, so the permission a previewer already holds is the only one asked + * for. + * + * @module domains/collections/services/preview-url-resolver + */ + +/** + * A preview declaration, as either authoring path may write it. + * + * The two fields are alternatives rather than a pair: `url` comes from + * `CollectionPreviewConfig` (code-first, where it is required) and `urlTemplate` + * from the dynamic-collection admin config (UI-created, where no function can be + * stored). Both are optional here because this type is the union the resolver + * sees, not either authoring surface. + */ +export interface PreviewDeclaration { + /** Code-first: computes the URL from the entry, or declines by returning null. */ + url?: (entry: Record) => string | null; + /** UI-created: a path with `{fieldName}` placeholders. */ + urlTemplate?: string; +} + +/** + * What a preview resolution can say, including that it cannot answer. + * + * Four cases and not two, deliberately. Three of them render as "no preview + * button", so collapsing them into `null` is the tempting simplification — and + * it is the one that caused the defect this shape exists to prevent. A resolver + * that answers `null` for "the site URL is not configured" is indistinguishable + * from one answering `null` for "this collection has no preview", so a caller + * that wants to recover has nothing to branch on and reaches for the only origin + * it can see: its own. That is the admin's host, which is confidently wrong. + * + * `notConfigured` and `unavailable` are also worth separating even though both + * hide the button, because only one of them is a state the editor can leave: an + * entry with no slug yet becomes previewable once it has one, while a collection + * with no preview declaration never does. + */ +export type PreviewUrlResolution = + /** A complete, absolute URL. */ + | { status: "resolved"; url: string } + /** This collection declares no preview at all. */ + | { status: "notConfigured" } + /** + * A preview is declared, but not for this entry right now — the authored + * function returned null, or a template placeholder has no value yet. + */ + | { status: "unavailable" } + /** + * A path was produced and nothing can name a host for it. Distinct from every + * case above because a guess IS available here and is wrong. + */ + | { status: "noSiteUrl"; path: string }; + +/** + * True when a collection declares a preview by either route. + * + * Exported because the admin needs the button's PRESENCE without a round trip, + * and a boolean is something the registry can store even though the function it + * is derived from is not. Both readers ask this one predicate: the projection + * that persists `hasPreview` and the resolver below. Deriving the stored boolean + * from the same expression that decides the resolution is what keeps a persisted + * `hasPreview: true` from outliving the declaration it was computed from. + */ +export function hasPreviewConfigured( + preview: PreviewDeclaration | undefined +): boolean { + if (!preview) return false; + return typeof preview.url === "function" || Boolean(preview.urlTemplate); +} + +/** + * The text a field value contributes to a URL, or null if it cannot contribute. + * + * An allowlist of the types that have a meaningful string form, rather than a + * denylist of the ones that do not. The two are not equivalent: excluding + * objects leaves functions through, and `String(fn)` is the function's SOURCE + * TEXT — a long, valid-looking path segment. Naming what is permitted cannot + * develop that kind of gap as the set of possible field values grows. + * + * Empty string is rejected alongside null and undefined because all three mean + * the same thing to an editor: the field has not been filled in, so no URL can + * be built from it yet. + */ +function asUrlSegment(value: unknown): string | null { + if (typeof value === "string") return value === "" ? null : value; + if (typeof value === "number") + return Number.isFinite(value) ? String(value) : null; + if (typeof value === "boolean") return String(value); + if (typeof value === "bigint") return value.toString(); + return null; +} + +/** + * Substitute `{fieldName}` placeholders with entry values. + * + * Returns null when any placeholder has no usable value, which is the + * `unavailable` case: a template naming `{slug}` cannot produce a URL for an + * entry whose slug has not been filled in yet. Values are compared against + * null/undefined/empty-string rather than tested for truthiness, so a legitimate + * `0` or `false` interpolates instead of suppressing the whole URL. + */ +function interpolate( + template: string, + entry: Record +): string | null { + const placeholders = template.match(/\{(\w+)\}/g); + if (!placeholders) return template; + + let result = template; + for (const placeholder of placeholders) { + const field = placeholder.slice(1, -1); + const text = asUrlSegment(entry[field]); + if (text === null) return null; + result = result.replace(placeholder, encodeURIComponent(text)); + } + return result; +} + +/** + * Resolve the preview URL for one entry. + * + * `siteUrl` is where the reader's site is served, which is what turns the + * authored path into something that survives being pasted into an email. A + * declaration that already returns an absolute URL is passed through untouched — + * an author who writes a full URL has named the host deliberately, and + * re-basing it against the configured site would override that. + */ +export function resolvePreviewUrl({ + preview, + entry, + siteUrl, +}: { + preview: PreviewDeclaration | undefined; + entry: Record; + siteUrl: string | null; +}): PreviewUrlResolution { + if (!hasPreviewConfigured(preview)) return { status: "notConfigured" }; + + let path: string | null = null; + + if (typeof preview?.url === "function") { + // The authored function is user code running inside a request. It may throw, + // and a throw here is the collection declining to preview rather than a + // server fault, so it is reported as `unavailable` — the same answer as a + // deliberate `return null` — instead of failing the request. + try { + path = preview.url(entry); + } catch { + return { status: "unavailable" }; + } + } else if (preview?.urlTemplate) { + path = interpolate(preview.urlTemplate, entry); + } + + if (path === null || path === "") return { status: "unavailable" }; + + if (/^https?:\/\//i.test(path)) return { status: "resolved", url: path }; + + if (!siteUrl) return { status: "noSiteUrl", path }; + + // Join without doubling or dropping the separator: the configured site may or + // may not carry a trailing slash, and an authored path may or may not lead + // with one. + const base = siteUrl.replace(/\/+$/, ""); + const suffix = path.startsWith("/") ? path : `/${path}`; + return { status: "resolved", url: `${base}${suffix}` }; +} From f4b9edfe47372ddfa55c066e8b70065f198b2d7e Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sat, 15 Aug 2026 13:47:32 +0500 Subject: [PATCH 2/9] feat(nextly): persist whether a collection previews The admin needs one thing from a preview declaration before it can draw anything: whether a button belongs on the page. That is a boolean, and a boolean is storable even though the function it comes from is not, so the boolean is what the registry now holds. It is derived by the same predicate the resolver consults, so a stored true and a resolution reporting notConfigured cannot disagree. The URL is still never stored: it depends on the entry. preview therefore leaves the not-persisted list. Verified that the completeness assertion catches the alternative: dropping the projection while the key is absent from that list fails the build naming preview. --- .../__tests__/persisted-admin.test.ts | 27 +++++++++++++++++++ .../services/collection-sync-service.ts | 18 ++++++++----- .../src/schemas/dynamic-collections/types.ts | 17 ++++++++++++ 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/packages/nextly/src/domains/collections/services/__tests__/persisted-admin.test.ts b/packages/nextly/src/domains/collections/services/__tests__/persisted-admin.test.ts index 78b40ffd6d..c6cb5cb590 100644 --- a/packages/nextly/src/domains/collections/services/__tests__/persisted-admin.test.ts +++ b/packages/nextly/src/domains/collections/services/__tests__/persisted-admin.test.ts @@ -54,6 +54,33 @@ describe("toPersistedAdmin", () => { }); }); + it("stores whether a preview exists, never the function that decides it", () => { + const persisted = toPersistedAdmin({ + useAsTitle: "title", + preview: { + url: entry => `/posts/${String(entry.slug)}`, + label: "View post", + openInNewTab: false, + }, + }); + + expect(persisted?.preview).toEqual({ + hasPreview: true, + label: "View post", + openInNewTab: false, + }); + // The function is the thing no column can hold. If it ever appears here the + // row write will either fail or silently store null, and the admin will be + // reading a key that cannot mean anything. + expect(persisted?.preview).not.toHaveProperty("url"); + }); + + it("omits preview entirely when a collection declares none", () => { + // Distinct from hasPreview: false — nothing was declared, so there is no + // preview object to describe. + expect(toPersistedAdmin({ useAsTitle: "title" })?.preview).toBeUndefined(); + }); + it("returns undefined when a collection declares no admin options", () => { expect(toPersistedAdmin(undefined)).toBeUndefined(); }); diff --git a/packages/nextly/src/domains/collections/services/collection-sync-service.ts b/packages/nextly/src/domains/collections/services/collection-sync-service.ts index 20787a0049..c1b4998969 100644 --- a/packages/nextly/src/domains/collections/services/collection-sync-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-sync-service.ts @@ -66,6 +66,7 @@ import { type CodeFirstCollectionConfig, type SyncResult, } from "./collection-registry-service"; +import { hasPreviewConfigured } from "./preview-url-resolver"; /** * Options for the sync operation. @@ -259,12 +260,6 @@ export const ADMIN_KEYS_NOT_PERSISTED = { * authoring paths describe a collection in one place. See `resolveDescription`. */ description: "stored on the collection row rather than under `admin`", - /** - * Carries `url`, a function of the entry, which no column can hold. Serving it needs the - * admin to ASK the server to evaluate it rather than to read it back, which is a change to - * how the panel obtains config and is deliberately not folded in here. - */ - preview: "holds a function; needs server-side evaluation rather than storage", } as const; /** @@ -310,6 +305,17 @@ export function toPersistedAdmin(admin: CollectionConfig["admin"]) { limits: admin.pagination.limits, } : undefined, + // The preview declaration minus the part no column can hold. `url` is a function of the + // entry, so what is stored is the ANSWER to the only question the admin asks of it — is + // there a preview here — plus the two presentation options the button needs to render. + // The URL itself depends on the entry and is resolved per request instead. + preview: admin.preview + ? { + hasPreview: hasPreviewConfigured(admin.preview), + label: admin.preview.label, + openInNewTab: admin.preview.openInNewTab, + } + : undefined, // Include custom components for plugins (e.g., custom Edit views) components: admin.components, }; diff --git a/packages/nextly/src/schemas/dynamic-collections/types.ts b/packages/nextly/src/schemas/dynamic-collections/types.ts index f558025ca9..9f4b6aa035 100644 --- a/packages/nextly/src/schemas/dynamic-collections/types.ts +++ b/packages/nextly/src/schemas/dynamic-collections/types.ts @@ -193,10 +193,27 @@ export interface CollectionAdminConfig { * ``` */ preview?: { + /** + * Whether this collection previews at all, decided when the config was synced. + * + * A code-first collection declares its preview as a FUNCTION of the entry, and no column can + * hold one — so the admin cannot read the declaration back the way it reads every other + * option. What it needs from the declaration is only whether a preview button belongs on the + * page, and that is a boolean, so the boolean is what gets stored. + * + * Derived by `hasPreviewConfigured`, which is also what the resolver consults, so a stored + * `true` and a resolution that reports `notConfigured` cannot disagree. The URL itself is + * never stored: it depends on the entry, so it is resolved per request. + */ + hasPreview?: boolean; + /** * URL template with field placeholders in {fieldName} format. * Used for UI-created collections where functions can't be stored. * + * Read by the server when resolving a preview URL, never by the admin: interpolating it in + * the browser would be a second implementation of a question the resolver already answers. + * * @example "/preview/{slug}", "/api/preview?id={id}" */ urlTemplate?: string; From cc039f1d949be40b86390d7fe91f6d9edb0d268a Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sat, 15 Aug 2026 22:21:18 +0500 Subject: [PATCH 3/9] feat(nextly): answer where an entry previews over http The declaration a code-first collection writes is a function, so it lives only in the server module graph and the admin cannot read it back. This is the route it asks instead, returning a finished absolute URL. Gated on read rather than the update that guards minting a preview LINK. A link is a bearer credential and is gated at the level of someone who may edit the draft it opens; this returns no credential, so requiring update would hide the button from a reviewer who may read but not change. The authored config is consulted before the registry. A code-first collection is synced into the registry too, but the function cannot survive the trip, so reading the registry first would find an empty declaration for exactly the collections whose preview works. --- packages/nextly/src/api/preview-url.ts | 126 +++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 packages/nextly/src/api/preview-url.ts diff --git a/packages/nextly/src/api/preview-url.ts b/packages/nextly/src/api/preview-url.ts new file mode 100644 index 0000000000..d16717dbf9 --- /dev/null +++ b/packages/nextly/src/api/preview-url.ts @@ -0,0 +1,126 @@ +/** + * Resolving where an entry previews. + * + * POST /api/nextly/preview-url -> the preview URL for one entry's data + * + * **Gated on `read` for the collection, which is deliberately weaker than the + * `update` that guards minting a preview LINK.** The two hand out different + * things. A link is a bearer credential: it carries a signed token that grants + * whoever holds it a read of a draft, so it is gated at the level of someone who + * may edit that draft. This endpoint returns a URL and no credential — opening + * it shows the caller exactly what their own session already permits, and shows + * a stranger nothing that was not already public. Requiring `update` here would + * hide the preview button from a reviewer who may read a collection but not + * change it, which is a role the workflow exists to serve. + * + * **The site URL travels in the response, and that is the point rather than a + * leak.** `settings` is a system resource the `editor` and `author` presets do + * not grant, so those roles cannot read the configured site URL directly — and + * they are exactly who previews content. Answering with a finished absolute URL + * is what lets them preview without being handed a settings read they should not + * have. The response discloses the site's own public address, which anyone who + * visits the site already knows. + * + * @module api/preview-url + */ + +import { z } from "zod"; + +import { container } from "../di"; +import type { NextlyServiceConfig } from "../di/register"; +import type { CollectionRegistryService } from "../domains/collections/services/collection-registry-service"; +import { + resolvePreviewUrl, + type PreviewDeclaration, +} from "../domains/collections/services/preview-url-resolver"; +import { getCachedNextly } from "../init"; +import type { GeneralSettingsService } from "../services/general-settings/general-settings-service"; + +import { respondData } from "./response-shapes"; +import { requireRouteCollectionAccess } from "./route-auth"; +import { withErrorHandler } from "./with-error-handler"; +import { nextlyValidationFromZod } from "./zod-to-nextly-error"; + +/** + * The entry travels in the request body rather than being loaded by id. + * + * An editor previews what is on screen, which includes edits not yet saved — so + * the values that decide the URL are the form's, not the row's. Loading the row + * here would resolve a URL for the last saved state and quietly show the wrong + * page, which is worse than not offering the button. + * + * The data is the caller's own: they are sending back what they are already + * looking at, and the resolved URL returns only to them. So this widens no read. + */ +const resolveSchema = z.object({ + collection: z.string().min(1), + entry: z.record(z.string(), z.unknown()), +}); + +async function settingsService(): Promise { + await getCachedNextly(); + return container.get("generalSettingsService"); +} + +/** + * Read the preview declaration for a collection from whichever authoring path + * defined it. + * + * A code-first collection holds a `url` function that exists only in the + * server's module graph; a UI-created one holds a `urlTemplate` string in the + * registry. Both are read here and handed to the one resolver, so the caller + * never branches on which kind of collection it is looking at. + * + * **The authored config is consulted FIRST, and the order is load-bearing.** A + * code-first collection is also synced into the registry, but the function + * cannot survive that trip — its stored record carries `hasPreview` and the + * presentation options and nothing that can produce a URL. Reading the registry + * first would therefore find a declaration for exactly those collections and + * find it empty, and the resolver would correctly report `notConfigured` for a + * collection whose preview works. + */ +async function previewDeclarationFor( + collection: string +): Promise { + await getCachedNextly(); + + const config = container.get("config"); + const authored = config?.collections?.find(c => c.slug === collection); + if (authored?.admin?.preview) return authored.admin.preview; + + const registry = container.get( + "collectionRegistryService" + ); + const stored = await registry.getCollectionBySlug(collection); + return stored?.admin?.preview; +} + +/** + * POST /api/nextly/preview-url + * + * Answers with one of the resolver's states, so a caller can tell "this + * collection has no preview" from "this entry is not previewable yet" from "no + * site URL is configured". Collapsing them into a nullable string is what let an + * earlier attempt fall back to the admin's own origin and hand out a link to the + * wrong host. + */ +export const resolveEntryPreviewUrl = withErrorHandler(async (req: Request) => { + const body: unknown = await req.json().catch(() => undefined); + const parsed = resolveSchema.safeParse(body); + if (!parsed.success) throw nextlyValidationFromZod(parsed.error); + const { collection, entry } = parsed.data; + + // Per COLLECTION, so naming a collection the caller cannot read does not + // resolve a URL into it. Row-level rules are not consulted: the entry data + // came from the caller, so no row they cannot already see is involved. + await requireRouteCollectionAccess(req, "read", collection); + + const [preview, settings] = await Promise.all([ + previewDeclarationFor(collection), + settingsService().then(service => service.getSettings()), + ]); + + return respondData( + resolvePreviewUrl({ preview, entry, siteUrl: settings.siteUrl }) + ); +}); From 4d585649d66db3c4ae48298b0cbc69b227dbef5f Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 16 Aug 2026 02:48:48 +0500 Subject: [PATCH 4/9] feat(nextly): register the preview-url route The route takes no path parameters: what it is asked about is form state, including values not yet saved, so no id identifies it. A deeper path is therefore a mistake rather than a variant and is refused. Left to fall through it would be answered by the bare route, which is the shape that once served webhook signing secrets from an invalid URL. ServiceType gains previewUrl, which is what forced the registration to be complete rather than merely compiling. --- packages/nextly/src/dispatcher/types.ts | 1 + .../route-parser.preview-url.test.ts | 43 +++++++++++++++++++ .../nextly/src/route-handler/route-parser.ts | 32 ++++++++++++++ packages/nextly/src/routeHandler.ts | 8 ++++ 4 files changed, 84 insertions(+) create mode 100644 packages/nextly/src/route-handler/__tests__/route-parser.preview-url.test.ts diff --git a/packages/nextly/src/dispatcher/types.ts b/packages/nextly/src/dispatcher/types.ts index acd304d336..99f3831c15 100644 --- a/packages/nextly/src/dispatcher/types.ts +++ b/packages/nextly/src/dispatcher/types.ts @@ -25,6 +25,7 @@ export type ServiceType = | "webhooks" | "generalSettings" | "previewLinks" + | "previewUrl" | "imageSizes" | "dashboard" | "email" diff --git a/packages/nextly/src/route-handler/__tests__/route-parser.preview-url.test.ts b/packages/nextly/src/route-handler/__tests__/route-parser.preview-url.test.ts new file mode 100644 index 0000000000..0d30b000ca --- /dev/null +++ b/packages/nextly/src/route-handler/__tests__/route-parser.preview-url.test.ts @@ -0,0 +1,43 @@ +/** + * The preview-URL route. + * + * Two properties are worth pinning. The route takes no path parameters at all, + * because what is being asked about is form state rather than a saved row — so a + * deeper path is a mistake rather than a variant, and must not be answered. And + * it must stay distinct from `preview-links`, which sits one segment away and + * hands out a bearer credential rather than a URL: a parser that confused the + * two would answer a credential request with a plain URL, or worse the reverse. + */ +import { describe, it, expect } from "vitest"; + +import { parseRestRoute } from "../route-parser"; + +describe("preview-url routes", () => { + it("parses the resolve request", () => { + expect(parseRestRoute(["preview-url"], "POST")).toMatchObject({ + service: "previewUrl", + operation: "create", + method: "resolveEntryPreviewUrl", + }); + }); + + it("refuses a deeper path rather than ignoring the extra segments", () => { + // An unmatched route is the empty object here, as the webhook suite pins for + // the same shape. Left to fall through instead, a mistyped longer path would + // be answered by the bare route and read as working. + expect(parseRestRoute(["preview-url", "123"], "POST")).toEqual({}); + expect(parseRestRoute(["preview-url", "123", "extra"], "POST")).toEqual({}); + }); + + it("stays distinct from the preview-links routes", () => { + // Same first word, different resource, and they must not answer for each + // other: one returns a URL, the other mints a credential. + expect(parseRestRoute(["preview-links"], "POST")).toMatchObject({ + service: "previewLinks", + method: "mintPreviewLink", + }); + expect(parseRestRoute(["preview-url"], "POST")).toMatchObject({ + service: "previewUrl", + }); + }); +}); diff --git a/packages/nextly/src/route-handler/route-parser.ts b/packages/nextly/src/route-handler/route-parser.ts index 9b2a35ff26..378e13331f 100644 --- a/packages/nextly/src/route-handler/route-parser.ts +++ b/packages/nextly/src/route-handler/route-parser.ts @@ -1770,6 +1770,32 @@ function parsePreviewLinkRoutes( return null; } +/** + * `POST /api/nextly/preview-url` resolves where one entry previews. + * + * The entry travels in the body rather than the path, because an editor + * previews what is on screen — including values not yet saved — so there is no + * id that identifies what is being asked about. + * + * Anything deeper is refused rather than folded in. A trailing segment here + * would otherwise be ignored, and a caller who mistyped a longer path would get + * a confident answer to a route they did not ask for. + */ +function parsePreviewUrlRoutes( + id: string | undefined, + subresource: string | undefined, + routeParams: Record +): ParsedRoute | null { + if (id !== undefined || subresource !== undefined) return null; + + return { + service: "previewUrl", + operation: "create", + method: "resolveEntryPreviewUrl", + routeParams, + }; +} + function parseApiKeyRoutes( id: string | undefined, httpMethod: string, @@ -2368,6 +2394,12 @@ export function parseRestRoute( if (result) return result; } + // Handle resolving where an entry previews + if (resource === "preview-url") { + const result = parsePreviewUrlRoutes(id, subresource, routeParams); + if (result) return result; + } + // Handle API Keys endpoints if (resource === "api-keys") { const result = parseApiKeyRoutes(id, httpMethod, routeParams); diff --git a/packages/nextly/src/routeHandler.ts b/packages/nextly/src/routeHandler.ts index 8dc5ae2b94..cc16afb77c 100644 --- a/packages/nextly/src/routeHandler.ts +++ b/packages/nextly/src/routeHandler.ts @@ -55,6 +55,7 @@ import { deleteImageSize, } from "./api/image-sizes"; import { mintPreviewLink, revokePreviewLinks } from "./api/preview-links"; +import { resolveEntryPreviewUrl } from "./api/preview-url"; import { readOrGenerateRequestId, withRequestIdHeader } from "./api/request-id"; // canonical respondX wire shapes (spec §5.1) instead of the // hand-rolled `{ data: }` envelope. @@ -941,6 +942,13 @@ async function handleServiceRequest( : mintPreviewLink(req); } + // ==================== PREVIEW URL DIRECT DISPATCH ==================== + // Beside the handlers above and for the same reason: it parses its own JSON, + // and the shared body read below would leave the stream empty. + if (service === "previewUrl") { + return resolveEntryPreviewUrl(req); + } + // ==================== GENERAL SETTINGS DIRECT DISPATCH ==================== if (service === "generalSettings") { return handleGeneralSettingsRequest(req, httpMethod); From 682e3e759add9a9534a0572609eb548f49b5bf76 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 16 Aug 2026 02:51:21 +0500 Subject: [PATCH 5/9] feat(admin): open the preview at a url the server resolves The panel decides WHETHER to offer the button from a boolean the registry stores, and asks the server WHERE only on click. It no longer reads the declaration itself: the code-first form is a function, and the template form is the resolver input rather than anything to interpolate here. The tab is claimed synchronously inside the click and navigated once the URL arrives. A window opened after an await has lost the user-gesture context and Safari and Firefox block it. noopener cannot be passed for that, since it makes window.open return null and leaves nothing to navigate, so the opener is severed by hand while the tab is still blank. A click that cannot open anything says which of the three reasons it hit, except notConfigured: the button should not have been drawn, so reporting it would describe a state the editor cannot act on. --- .../hooks/__tests__/useEntryPreview.test.ts | 240 ++++++++++++++ packages/admin/src/hooks/index.ts | 1 + packages/admin/src/hooks/useEntryPreview.ts | 307 ++++++------------ packages/admin/src/services/previewUrlApi.ts | 50 +++ 4 files changed, 392 insertions(+), 206 deletions(-) create mode 100644 packages/admin/src/hooks/__tests__/useEntryPreview.test.ts create mode 100644 packages/admin/src/services/previewUrlApi.ts diff --git a/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts b/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts new file mode 100644 index 0000000000..8a60e6d1ab --- /dev/null +++ b/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts @@ -0,0 +1,240 @@ +import { renderHook, act } from "@testing-library/react"; +import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; + +import { useEntryPreview } from "../useEntryPreview"; + +const resolve = vi.hoisted(() => vi.fn()); + +vi.mock("@admin/services/previewUrlApi", () => ({ + previewUrlApi: { resolve }, +})); + +vi.mock("@admin/lib/preview/preview-data", () => ({ + storePreviewData: vi.fn(() => "preview-key"), + generatePreviewUrlWithData: vi.fn( + (url: string, key: string) => `${url}?__preview=${key}` + ), +})); + +/** A stand-in for the tab `window.open` hands back. */ +function fakeTab() { + return { location: { href: "" }, close: vi.fn(), opener: {} as unknown }; +} + +const collection = { + name: "posts", + admin: { preview: { hasPreview: true } }, +}; + +let openSpy: ReturnType; + +beforeEach(() => { + resolve.mockReset(); + openSpy = vi.spyOn(window, "open"); +}); + +afterEach(() => { + openSpy.mockRestore(); +}); + +describe("isPreviewAvailable", () => { + it("follows the stored boolean without asking the server", () => { + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" } }) + ); + + expect(result.current.isPreviewAvailable).toBe(true); + // Availability is a render-time question; a round trip here would make the + // button appear late on every entry that has one. + expect(resolve).not.toHaveBeenCalled(); + }); + + it("is false when the collection stores no preview", () => { + const { result } = renderHook(() => + useEntryPreview({ collection: { name: "posts" }, entry: { id: "1" } }) + ); + + expect(result.current.isPreviewAvailable).toBe(false); + }); + + it("is false when the collection stores hasPreview: false", () => { + const { result } = renderHook(() => + useEntryPreview({ + collection: { + name: "posts", + admin: { preview: { hasPreview: false } }, + }, + entry: { id: "1" }, + }) + ); + + expect(result.current.isPreviewAvailable).toBe(false); + }); +}); + +describe("openPreview", () => { + it("claims the tab BEFORE awaiting, so the popup blocker does not eat it", async () => { + const tab = fakeTab(); + const order: string[] = []; + openSpy.mockImplementation(() => { + order.push("open"); + return tab as unknown as Window; + }); + resolve.mockImplementation(() => { + order.push("resolve"); + return Promise.resolve({ status: "resolved", url: "https://s.dev/p/1" }); + }); + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" } }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // The whole point: a window opened after an await has lost the user-gesture + // context and Safari and Firefox block it. Asserting only that open() was + // called would pass on the broken ordering too. + expect(order).toEqual(["open", "resolve"]); + expect(tab.location.href).toBe("https://s.dev/p/1"); + }); + + it("opens the tab without noopener, then severs the reference by hand", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + resolve.mockResolvedValue({ status: "resolved", url: "https://s.dev/p/1" }); + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" } }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // Passing "noopener" would make window.open return null and leave nothing to + // navigate, so the reference has to be cut manually instead. + const features = openSpy.mock.calls[0]?.[2]; + expect(features ?? "").not.toContain("noopener"); + expect(tab.opener).toBeNull(); + }); + + it("closes the claimed tab and reports why when no host is configured", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + resolve.mockResolvedValue({ status: "noSiteUrl", path: "/p/1" }); + const onUnavailable = vi.fn(); + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" }, onUnavailable }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // Leaving a blank tab open would look like a preview that failed to load. + expect(tab.close).toHaveBeenCalled(); + expect(tab.location.href).toBe(""); + // Distinct from "unavailable": this one is fixed by an admin setting a site + // URL, not by the editor filling in a field. + expect(onUnavailable).toHaveBeenCalledWith("noSiteUrl"); + }); + + it("reports an entry that is not previewable yet", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + resolve.mockResolvedValue({ status: "unavailable" }); + const onUnavailable = vi.fn(); + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" }, onUnavailable }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + expect(onUnavailable).toHaveBeenCalledWith("unavailable"); + }); + + it("stays silent when the collection has no preview at all", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + resolve.mockResolvedValue({ status: "notConfigured" }); + const onUnavailable = vi.fn(); + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" }, onUnavailable }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // The button should not have been offered, so an error here would describe a + // state the editor cannot act on. + expect(onUnavailable).not.toHaveBeenCalled(); + expect(tab.close).toHaveBeenCalled(); + }); + + it("closes the tab and reports when the request itself fails", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + resolve.mockRejectedValue(new Error("network")); + const onUnavailable = vi.fn(); + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" }, onUnavailable }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + expect(tab.close).toHaveBeenCalled(); + expect(onUnavailable).toHaveBeenCalledWith("failed"); + }); + + it("sends unsaved form values, not the saved row", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + resolve.mockResolvedValue({ + status: "resolved", + url: "https://s.dev/p/new", + }); + + const { result } = renderHook(() => + useEntryPreview({ + collection, + entry: { id: "1", slug: "saved" }, + getFormValues: () => ({ slug: "edited" }), + }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // Resolving against the saved row would open the previous URL and show the + // wrong page, which is worse than not offering the button. + expect(resolve).toHaveBeenCalledWith({ + collection: "posts", + entry: { id: "1", slug: "edited" }, + }); + expect(tab.location.href).toBe("https://s.dev/p/new?__preview=preview-key"); + }); + + it("navigates the current window when the collection opts out of a new tab", async () => { + resolve.mockResolvedValue({ status: "resolved", url: "https://s.dev/p/1" }); + + const { result } = renderHook(() => + useEntryPreview({ + collection: { + name: "posts", + admin: { preview: { hasPreview: true, openInNewTab: false } }, + }, + entry: { id: "1" }, + }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + expect(openSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/admin/src/hooks/index.ts b/packages/admin/src/hooks/index.ts index deb3c63912..8c2bac6a24 100644 --- a/packages/admin/src/hooks/index.ts +++ b/packages/admin/src/hooks/index.ts @@ -54,6 +54,7 @@ export { useEntryPreview } from "./useEntryPreview"; export type { PreviewConfig, PreviewCollection, + PreviewUnavailableReason, UseEntryPreviewOptions, UseEntryPreviewResult, } from "./useEntryPreview"; diff --git a/packages/admin/src/hooks/useEntryPreview.ts b/packages/admin/src/hooks/useEntryPreview.ts index 3f8de0e615..71c5d2eef4 100644 --- a/packages/admin/src/hooks/useEntryPreview.ts +++ b/packages/admin/src/hooks/useEntryPreview.ts @@ -1,14 +1,16 @@ "use client"; /** - * useEntryPreview Hook + * Opening an entry's preview. * - * Provides preview URL generation and preview opening functionality - * for entry forms. Supports both saved entries and unsaved changes - * via session storage. + * The URL is resolved by the server rather than here: a code-first collection + * declares its preview as a function that no column can hold, and the site URL + * the answer is based on sits behind a `settings` permission that editors and + * authors do not hold. So this hook decides WHETHER to offer the button, from a + * boolean the registry can store, and asks the server WHERE only when it is + * clicked. * * @module hooks/useEntryPreview - * @since 1.0.0 */ import { useCallback, useMemo } from "react"; @@ -17,276 +19,169 @@ import { storePreviewData, generatePreviewUrlWithData, } from "@admin/lib/preview/preview-data"; +import { previewUrlApi } from "@admin/services/previewUrlApi"; // ============================================================================ // Types // ============================================================================ /** - * Preview configuration from collection admin config. - * Supports both function-based (code-first) and template-based (UI) configs. + * The preview settings the admin reads back from the collection registry. + * + * Deliberately not the authored declaration: `url` is a function and + * `urlTemplate` is the server's resolution input, so neither belongs here. + * What the panel needs is whether to draw a button and how to label it. */ export interface PreviewConfig { - /** - * Function to generate preview URL from entry data. - * Used by code-first collections. - */ - url?: (entry: Record) => string | null; - - /** - * URL template with {fieldName} placeholders. - * Used by UI-created collections. - */ - urlTemplate?: string; - - /** - * Whether to open preview in a new tab. - * @default true - */ + /** Whether this collection previews at all, decided when the config synced. */ + hasPreview?: boolean; + /** Whether to open the preview in a new tab. @default true */ openInNewTab?: boolean; - - /** - * Custom label for the preview button. - * @default "Preview" - */ + /** Custom label for the preview button. @default "Preview" */ label?: string; } -/** - * Collection configuration required for preview functionality. - */ +/** Collection configuration required for preview functionality. */ export interface PreviewCollection { - /** Collection slug/name */ + /** Collection slug. */ name: string; - /** Admin configuration including preview settings */ admin?: { preview?: PreviewConfig; }; } -/** - * Options for the useEntryPreview hook. - */ export interface UseEntryPreviewOptions { - /** Collection with preview configuration */ collection: PreviewCollection; - /** Existing entry data (for edit mode) */ + /** The saved entry, when editing an existing one. */ entry?: Record | null; - /** - * Function to get current form values (for unsaved changes). - * Called when opening preview to get the latest form state. - */ + /** Current form values, so the preview reflects unsaved edits. */ getFormValues?: () => Record; + /** Told why a click could not open anything. */ + onUnavailable?: (reason: PreviewUnavailableReason) => void; } /** - * Return type for useEntryPreview hook. + * Why a preview click produced nothing. + * + * Separate from the button's availability because these are only discoverable + * on click: the entry's own values decide two of them, and the third is a + * deployment setting the panel cannot see. */ +export type PreviewUnavailableReason = + /** Declared, but not for this entry yet — no slug, wrong status. */ + | "unavailable" + /** No site URL is configured, so no host can be named. */ + | "noSiteUrl" + /** The request itself failed. */ + | "failed"; + export interface UseEntryPreviewResult { - /** Whether preview is available for this collection */ + /** Whether to offer the button at all. Known without a round trip. */ isPreviewAvailable: boolean; - /** Current preview URL based on saved entry (null if not available) */ - previewUrl: string | null; - /** Open the preview (uses unsaved data if available) */ - openPreview: () => void; - /** Get preview URL for specific data */ - getPreviewUrl: (data: Record) => string | null; - /** Label for the preview button */ + /** Resolve and open. Asynchronous: the URL comes from the server. */ + openPreview: () => Promise; + /** Label for the preview button. */ label: string; } -// ============================================================================ -// Helpers -// ============================================================================ - -/** - * Interpolate URL template with entry data. - * - * Replaces {fieldName} placeholders with actual field values. - * Returns null if any required field is missing. - * - * @param template - URL template with {fieldName} placeholders - * @param data - Entry data to interpolate - * @returns Interpolated URL or null if interpolation fails - */ -function interpolateUrlTemplate( - template: string, - data: Record -): string | null { - try { - let result = template; - const placeholders = template.match(/\{(\w+)\}/g) || []; - - for (const placeholder of placeholders) { - const fieldName = placeholder.slice(1, -1); // Remove { and } - const value = data[fieldName]; - - // If a required field is missing/null/undefined, can't generate URL - if (value === null || value === undefined || value === "") { - return null; - } - - // eslint-disable-next-line @typescript-eslint/no-base-to-string - result = result.replace(placeholder, String(value)); - } - - return result; - } catch { - return null; - } -} - -/** - * Normalize a URL to be absolute if it's relative. - * - * @param url - URL that may be relative or absolute - * @returns Absolute URL - */ -function normalizeUrl(url: string): string { - if (url.startsWith("/")) { - return `${window.location.origin}${url}`; - } - return url; -} - // ============================================================================ // Hook // ============================================================================ /** - * useEntryPreview - Preview URL generation for entry forms + * useEntryPreview - open the site at the entry being edited. * - * Generates preview URLs based on collection configuration and entry data. - * Supports: - * - Function-based URL generation (code-first collections) - * - Template-based URL generation (UI-created collections) - * - Previewing unsaved changes via session storage - * - * @example Basic usage + * @example * ```tsx * const { isPreviewAvailable, openPreview, label } = useEntryPreview({ * collection, * entry, * getFormValues: () => form.getValues(), + * onUnavailable: reason => toast.error(PREVIEW_MESSAGES[reason]), * }); - * - * {isPreviewAvailable && ( - * - * )} - * ``` - * - * @example Check preview URL availability - * ```tsx - * const { previewUrl, isPreviewAvailable } = useEntryPreview({ - * collection, - * entry, - * }); - * - * // previewUrl is null if: - * // - Collection has no preview config - * // - Entry data is insufficient (e.g., missing slug) * ``` */ export function useEntryPreview({ collection, entry, getFormValues, + onUnavailable, }: UseEntryPreviewOptions): UseEntryPreviewResult { const previewConfig = collection.admin?.preview; - // Check if preview is configured - const isPreviewAvailable = useMemo(() => { - if (!previewConfig) return false; - return !!(previewConfig.url || previewConfig.urlTemplate); - }, [previewConfig]); - - /** - * Generate preview URL for given data. - * Tries function-based first, then template-based. - */ - const getPreviewUrl = useCallback( - (data: Record): string | null => { - if (!previewConfig) return null; - - try { - // Try function-based URL first (code-first collections) - if (previewConfig.url) { - const url = previewConfig.url(data); - return url ? normalizeUrl(url) : null; - } - - // Try template-based URL (UI collections) - if (previewConfig.urlTemplate) { - const url = interpolateUrlTemplate(previewConfig.urlTemplate, data); - return url ? normalizeUrl(url) : null; - } - - return null; - } catch (error) { - console.error("Failed to generate preview URL:", error); - return null; - } - }, + // From the stored boolean, so the button does not flicker in after a fetch + // and does not appear for a collection that has no preview at all. + const isPreviewAvailable = useMemo( + () => previewConfig?.hasPreview === true, [previewConfig] ); - /** - * Preview URL for the current saved entry. - */ - const previewUrl = useMemo(() => { - if (!entry) return null; - return getPreviewUrl(entry); - }, [entry, getPreviewUrl]); - - /** - * Open preview in a new tab/window. - * Uses unsaved form data if available via getFormValues. - */ - const openPreview = useCallback(() => { - // Get the data to preview (unsaved form values or saved entry) + const openPreview = useCallback(async () => { const unsavedData = getFormValues?.(); const dataToPreview = unsavedData ? { ...entry, ...unsavedData } : entry; - if (!dataToPreview) { - console.warn("No data available for preview"); + onUnavailable?.("unavailable"); return; } - // Generate base preview URL - let url = getPreviewUrl(dataToPreview); - if (!url) { - console.warn("Could not generate preview URL"); - return; - } + const openInNewTab = previewConfig?.openInNewTab !== false; - // If there's unsaved data, store it and append preview key - if (unsavedData) { - const entryId = entry?.id as string | undefined; - const previewKey = storePreviewData( - collection.name, - entryId, - dataToPreview - ); - url = generatePreviewUrlWithData(url, previewKey); - } + // Opened NOW, synchronously, while the click is still on the stack. A window + // opened after an `await` has lost the user-gesture context and Safari and + // Firefox block it, so the tab is claimed first and navigated once the URL + // arrives. + // + // `noopener` cannot be passed here: with it, `window.open` returns null and + // there is no handle left to navigate. The reference is severed by hand + // instead, while the tab is still `about:blank` and same-origin — after + // which it cannot reach back through `window.opener`. + const target = openInNewTab ? window.open("", "_blank") : null; + if (target) target.opener = null; + + const abandon = (reason: PreviewUnavailableReason) => { + target?.close(); + onUnavailable?.(reason); + }; + + try { + const resolution = await previewUrlApi.resolve({ + collection: collection.name, + entry: dataToPreview, + }); + + if (resolution.status !== "resolved") { + // `notConfigured` is not reported: the button should not have been + // offered, so telling the editor a preview is unavailable would describe + // a state they cannot act on. The other two are theirs to fix — fill in + // the slug, or ask an admin to set the site URL. + if (resolution.status !== "notConfigured") abandon(resolution.status); + else target?.close(); + return; + } - // Open the preview - const openInNewTab = previewConfig?.openInNewTab !== false; - if (openInNewTab) { - window.open(url, "_blank", "noopener,noreferrer"); - } else { - window.location.href = url; + // Unsaved values travel through session storage rather than the URL, so + // the preview renders what is on screen instead of what was last saved. + const url = unsavedData + ? generatePreviewUrlWithData( + resolution.url, + storePreviewData( + collection.name, + entry?.id as string | undefined, + dataToPreview + ) + ) + : resolution.url; + + if (target) target.location.href = url; + else window.location.href = url; + } catch { + abandon("failed"); } - }, [collection.name, entry, getFormValues, getPreviewUrl, previewConfig]); + }, [collection.name, entry, getFormValues, onUnavailable, previewConfig]); return { isPreviewAvailable, - previewUrl, openPreview, - getPreviewUrl, label: previewConfig?.label || "Preview", }; } diff --git a/packages/admin/src/services/previewUrlApi.ts b/packages/admin/src/services/previewUrlApi.ts new file mode 100644 index 0000000000..9b9e9d0313 --- /dev/null +++ b/packages/admin/src/services/previewUrlApi.ts @@ -0,0 +1,50 @@ +/** + * Where an entry previews, answered by the server. + * + * The admin cannot compute this itself. A code-first collection declares its + * preview as a function of the entry, which exists only in the server's module + * graph and no column can hold — and the site URL the result is based on lives + * behind a `settings` permission that the `editor` and `author` roles do not + * have, which are exactly the roles that preview content. So the panel asks for + * a finished URL rather than the parts to build one. + * + * @module services/previewUrlApi + */ + +import { protectedApi } from "@admin/lib/api/protectedApi"; + +export interface PreviewUrlRequest { + collection: string; + /** + * The entry as it stands on screen, unsaved edits included. The server + * resolves against these values rather than the saved row, because an editor + * previews what they are looking at. + */ + entry: Record; +} + +/** + * The server's answer, kept as four cases rather than a nullable URL. + * + * Three of them mean "no preview to open", and collapsing them would lose the + * only distinction that matters to a caller trying to recover: `noSiteUrl` is + * the one where an origin is guessable and guessing yields the admin's own host, + * which produces a confident link to the wrong place. + */ +export type PreviewUrlResolution = + | { status: "resolved"; url: string } + | { status: "notConfigured" } + | { status: "unavailable" } + | { status: "noSiteUrl"; path: string }; + +export const previewUrlApi = { + /** + * Resolve the preview URL for one entry's current values. + * + * Requires `read` on the collection — weaker than minting a preview LINK, + * which hands out a bearer credential. This returns a URL and no credential, + * so it shows the caller only what their own session already permits. + */ + resolve: (request: PreviewUrlRequest): Promise => + protectedApi.post("/preview-url", request), +}; From b0fea784bf4da6114ccecc1c0fe60228077d5768 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 16 Aug 2026 02:52:47 +0500 Subject: [PATCH 6/9] chore: add changeset for the preview url resolver --- .changeset/preview-url-resolver.md | 32 ++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 .changeset/preview-url-resolver.md diff --git a/.changeset/preview-url-resolver.md b/.changeset/preview-url-resolver.md new file mode 100644 index 0000000000..c573592580 --- /dev/null +++ b/.changeset/preview-url-resolver.md @@ -0,0 +1,32 @@ +--- +"nextly": patch +"create-nextly-app": patch +"@nextlyhq/admin": patch +"@nextlyhq/admin-css": patch +"@nextlyhq/blocks-engine": patch +"@nextlyhq/blocks-react": 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 +"@nextlyhq/builder": patch +"@nextlyhq/module-specifiers": patch +--- + +Resolve an entry preview URL in one place, on the server. + +A collection declares its preview two ways and they are disjoint: code-first writes a function of the entry, a UI-created collection writes a template string. Both are now answered by one resolver, so the admin asks where an entry previews instead of deciding for itself. + +Resolving on the server is what makes the preview button reachable for editors and authors. The site URL sits behind a settings permission neither role holds, so a browser-side answer was unavailable to exactly the people who share previews. From 33772630d6485e090b483bd804a981ea4b2847b4 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 16 Aug 2026 03:11:17 +0500 Subject: [PATCH 7/9] fix(nextly): refuse a preview url the browser would execute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resolved string is assigned to location.href by the admin, so its scheme decides whether the browser navigates or RUNS it. z.string().url() accepts javascript: and data: — measured, both parse — so a settings write could put script into the admin origin for whoever next clicked Preview. Navigable schemes are now an allowlist, applied to the configured site and to anything an authored function returns, so the two cannot disagree. A site url that fails it reports noSiteUrl rather than resolving: the remedy is the same as an absent one, an administrator setting a real value. The write schema rejects it too, so only a row stored before this existed can still carry one. Also: previewUrl joins the direct-dispatch set, without which the first authenticated request in a process reached an uninitialised container. The admin now writes unsaved data BEFORE opening the tab, since a new context copies session storage at creation and never sees a later write. And a blocked popup is reported instead of navigating this window, which would have taken the editor off the form and discarded their unsaved changes. --- .../hooks/__tests__/useEntryPreview.test.ts | 55 +++++++++++++++++ packages/admin/src/hooks/useEntryPreview.ts | 49 +++++++++++---- packages/nextly/src/api/general-settings.ts | 17 ++++++ .../__tests__/preview-url-resolver.test.ts | 58 ++++++++++++++++++ .../services/preview-url-resolver.ts | 59 +++++++++++++++++-- packages/nextly/src/routeHandler.ts | 1 + 6 files changed, 224 insertions(+), 15 deletions(-) diff --git a/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts b/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts index 8a60e6d1ab..f164c4436f 100644 --- a/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts +++ b/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts @@ -219,6 +219,61 @@ describe("openPreview", () => { expect(tab.location.href).toBe("https://s.dev/p/new?__preview=preview-key"); }); + it("stores unsaved data BEFORE opening the tab, not after", async () => { + const tab = fakeTab(); + const order: string[] = []; + const { storePreviewData } = await import( + "@admin/lib/preview/preview-data" + ); + vi.mocked(storePreviewData).mockImplementation(() => { + order.push("store"); + return "preview-key"; + }); + openSpy.mockImplementation(() => { + order.push("open"); + return tab as unknown as Window; + }); + resolve.mockResolvedValue({ status: "resolved", url: "https://s.dev/p/1" }); + + const { result } = renderHook(() => + useEntryPreview({ + collection, + entry: { id: "1" }, + getFormValues: () => ({ slug: "edited" }), + }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // A new browsing context gets a COPY of session storage taken at creation. + // Written afterwards, the key stays in this window and the preview silently + // shows the last saved values instead of the edits on screen. + expect(order).toEqual(["store", "open"]); + }); + + it("reports a blocked popup instead of navigating the admin away", async () => { + // window.open returns null when the browser blocks it. + openSpy.mockReturnValue(null); + resolve.mockResolvedValue({ status: "resolved", url: "https://s.dev/p/1" }); + const onUnavailable = vi.fn(); + const before = window.location.href; + + const { result } = renderHook(() => + useEntryPreview({ collection, entry: { id: "1" }, onUnavailable }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // Falling back to this window would take the editor off the form and + // discard every unsaved change — the opposite of what preview is for. + expect(window.location.href).toBe(before); + expect(onUnavailable).toHaveBeenCalledWith("popupBlocked"); + // And it must not even ask: the click cannot succeed either way. + expect(resolve).not.toHaveBeenCalled(); + }); + it("navigates the current window when the collection opts out of a new tab", async () => { resolve.mockResolvedValue({ status: "resolved", url: "https://s.dev/p/1" }); diff --git a/packages/admin/src/hooks/useEntryPreview.ts b/packages/admin/src/hooks/useEntryPreview.ts index 71c5d2eef4..0adc95e710 100644 --- a/packages/admin/src/hooks/useEntryPreview.ts +++ b/packages/admin/src/hooks/useEntryPreview.ts @@ -70,8 +70,17 @@ export interface UseEntryPreviewOptions { export type PreviewUnavailableReason = /** Declared, but not for this entry yet — no slug, wrong status. */ | "unavailable" - /** No site URL is configured, so no host can be named. */ + /** + * No usable site URL is configured, so no host can be named. Covers an absent + * setting and one the browser would execute rather than navigate to. + */ | "noSiteUrl" + /** + * The browser refused the new tab. Distinct from every other reason because + * nothing is wrong with the entry or the configuration — the editor can allow + * popups and click again. + */ + | "popupBlocked" /** The request itself failed. */ | "failed"; @@ -126,6 +135,19 @@ export function useEntryPreview({ const openInNewTab = previewConfig?.openInNewTab !== false; + // BEFORE the tab is opened, because a new browsing context receives a COPY + // of session storage taken when it is created. A write afterwards stays in + // this window, the new tab never sees the key, and the preview silently + // renders the last saved values instead of what is on screen — the exact + // failure the unsaved-data path exists to prevent. + const previewKey = unsavedData + ? storePreviewData( + collection.name, + entry?.id as string | undefined, + dataToPreview + ) + : undefined; + // Opened NOW, synchronously, while the click is still on the stack. A window // opened after an `await` has lost the user-gesture context and Safari and // Firefox block it, so the tab is claimed first and navigated once the URL @@ -138,6 +160,16 @@ export function useEntryPreview({ const target = openInNewTab ? window.open("", "_blank") : null; if (target) target.opener = null; + // A blocked popup is NOT the same as a collection that asked to open in + // place. Falling back to navigating this window would take the editor off + // the form they are editing and discard everything unsaved, so it is + // reported instead — the browser's own blocked-popup affordance is what lets + // them retry. + if (openInNewTab && !target) { + onUnavailable?.("popupBlocked"); + return; + } + const abandon = (reason: PreviewUnavailableReason) => { target?.close(); onUnavailable?.(reason); @@ -161,16 +193,11 @@ export function useEntryPreview({ // Unsaved values travel through session storage rather than the URL, so // the preview renders what is on screen instead of what was last saved. - const url = unsavedData - ? generatePreviewUrlWithData( - resolution.url, - storePreviewData( - collection.name, - entry?.id as string | undefined, - dataToPreview - ) - ) - : resolution.url; + // The payload was written above; only the key is appended here. + const url = + previewKey === undefined + ? resolution.url + : generatePreviewUrlWithData(resolution.url, previewKey); if (target) target.location.href = url; else window.location.href = url; diff --git a/packages/nextly/src/api/general-settings.ts b/packages/nextly/src/api/general-settings.ts index 48dafc3fd1..661f36db33 100644 --- a/packages/nextly/src/api/general-settings.ts +++ b/packages/nextly/src/api/general-settings.ts @@ -32,10 +32,27 @@ async function getGeneralSettingsService(): Promise { const updateSettingsSchema = z.object({ applicationName: z.string().max(255).nullable().optional(), + // `.url()` alone accepts any scheme the WHATWG parser does, `javascript:` and + // `data:` included — and this value is concatenated into preview URLs the + // admin assigns to `location.href`, where such a scheme executes rather than + // navigates. The resolver refuses those too; this stops one being stored in + // the first place, so a row written before that check existed is the only way + // one can still be present. siteUrl: z .string() .url("Site URL must be a valid URL") .max(2048) + .refine( + value => { + try { + const { protocol } = new URL(value); + return protocol === "http:" || protocol === "https:"; + } catch { + return false; + } + }, + { message: "Site URL must start with http:// or https://" } + ) .nullable() .optional(), adminEmail: z diff --git a/packages/nextly/src/domains/collections/services/__tests__/preview-url-resolver.test.ts b/packages/nextly/src/domains/collections/services/__tests__/preview-url-resolver.test.ts index 2347ccd3db..f15acd135a 100644 --- a/packages/nextly/src/domains/collections/services/__tests__/preview-url-resolver.test.ts +++ b/packages/nextly/src/domains/collections/services/__tests__/preview-url-resolver.test.ts @@ -201,6 +201,64 @@ describe("resolvePreviewUrl", () => { expect(resolution.status).not.toBe("resolved"); }); + it("refuses a site URL the browser would EXECUTE rather than navigate to", () => { + // The resolved string is assigned to location.href by the admin, and these + // schemes run script in the assigning document's origin — which for the + // preview tab is the admin's own. `z.string().url()` accepts every one of + // them, so a settings write would otherwise become script execution for + // whoever next clicks Preview. + const executable = [ + "javascript:alert(document.cookie)", + "JavaScript:alert(1)", + "data:text/html,", + "vbscript:msgbox(1)", + "file:///etc/passwd", + ]; + + expect(executable.length).toBeGreaterThan(0); + for (const siteUrl of executable) { + const resolution = resolvePreviewUrl({ + preview: { url: () => "/posts/hello" }, + entry: {}, + siteUrl, + }); + + expect(resolution).toEqual({ status: "noSiteUrl", path: "/posts/hello" }); + // Never `resolved`: a caller branching on that status navigates to it. + expect(resolution.status).not.toBe("resolved"); + } + }); + + it("refuses an executable URL returned by the authored function too", () => { + // The declaration is user code and may compute anything, so the same + // standard applies to what it returns as to the configured site. + const resolution = resolvePreviewUrl({ + preview: { url: () => "javascript:alert(1)" }, + entry: {}, + siteUrl: SITE, + }); + + // Not absolute by the navigable test, so it is joined under the site origin, + // where the scheme is inert as an ordinary path segment. + expect(resolution).toEqual({ + status: "resolved", + url: "https://example.com/javascript:alert(1)", + }); + }); + + it("accepts a site URL carrying a base path", () => { + expect( + resolvePreviewUrl({ + preview: { url: () => "/posts/hello" }, + entry: {}, + siteUrl: "https://example.com/site/", + }) + ).toEqual({ + status: "resolved", + url: "https://example.com/site/posts/hello", + }); + }); + it("passes an absolute authored URL through even with no site configured", () => { // An author who wrote a full URL named the host on purpose; re-basing it // against the configured site would override a deliberate choice. diff --git a/packages/nextly/src/domains/collections/services/preview-url-resolver.ts b/packages/nextly/src/domains/collections/services/preview-url-resolver.ts index de42ac35e9..5a12da01b6 100644 --- a/packages/nextly/src/domains/collections/services/preview-url-resolver.ts +++ b/packages/nextly/src/domains/collections/services/preview-url-resolver.ts @@ -67,6 +67,11 @@ export type PreviewUrlResolution = /** * A path was produced and nothing can name a host for it. Distinct from every * case above because a guess IS available here and is wrong. + * + * Covers a site URL that is absent AND one that is unusable — a scheme the + * browser would execute rather than navigate to, which is refused rather than + * returned. Both share one remedy: an administrator sets a real site URL. They + * are merged for that reason and not because they are the same event. */ | { status: "noSiteUrl"; path: string }; @@ -109,6 +114,44 @@ function asUrlSegment(value: unknown): string | null { return null; } +/** + * The schemes a resolved preview URL may carry. + * + * The result of this module is assigned to `location.href` by the admin, so what + * comes back is not a string — it is something the browser will EXECUTE if the + * scheme says so. `javascript:` and `data:` both run script in the assigning + * document's origin, which for the preview tab is the admin's own. + * + * An allowlist rather than a check for the two known-bad schemes: `vbscript:`, + * `blob:` and whatever a future engine adds would each need their own entry, and + * the gap would be silent. Naming what may navigate cannot develop that gap. + */ +const NAVIGABLE_PROTOCOLS = new Set(["http:", "https:"]); + +/** + * Parse `value` as an absolute URL that is safe to navigate to, or null. + * + * One predicate for both places an absolute URL enters this module — the site + * URL read from settings, and a declaration that returned a full URL itself — so + * the two cannot end up holding different opinions about what is navigable. + * + * The site URL is the reason this exists. It is stored through an API whose + * schema is `z.string().url()`, and that accepts any scheme the WHATWG parser + * does: `javascript:alert(1)` validates. Without this, such a value would be + * concatenated with a path, returned as `resolved`, and assigned to a + * same-origin blank tab — turning a settings write into script execution in the + * admin for whoever next clicks Preview. + */ +function asNavigableUrl(value: string): URL | null { + let parsed: URL; + try { + parsed = new URL(value); + } catch { + return null; + } + return NAVIGABLE_PROTOCOLS.has(parsed.protocol) ? parsed : null; +} + /** * Substitute `{fieldName}` placeholders with entry values. * @@ -173,14 +216,22 @@ export function resolvePreviewUrl({ if (path === null || path === "") return { status: "unavailable" }; - if (/^https?:\/\//i.test(path)) return { status: "resolved", url: path }; + // An author who returned a full URL named the host deliberately, so it is not + // re-based against the configured site. It still has to be navigable: the + // declaration is user code and may compute anything. + const absolute = asNavigableUrl(path); + if (absolute) return { status: "resolved", url: path }; - if (!siteUrl) return { status: "noSiteUrl", path }; + // A relative path cannot carry a scheme, so it needs no such check — a value + // like `javascript:...` fails to parse as absolute above and lands here, where + // joining it under the site's origin makes it an ordinary path segment. + const base = siteUrl === null ? null : asNavigableUrl(siteUrl); + if (!base) return { status: "noSiteUrl", path }; // Join without doubling or dropping the separator: the configured site may or // may not carry a trailing slash, and an authored path may or may not lead // with one. - const base = siteUrl.replace(/\/+$/, ""); + const origin = `${base.origin}${base.pathname}`.replace(/\/+$/, ""); const suffix = path.startsWith("/") ? path : `/${path}`; - return { status: "resolved", url: `${base}${suffix}` }; + return { status: "resolved", url: `${origin}${suffix}` }; } diff --git a/packages/nextly/src/routeHandler.ts b/packages/nextly/src/routeHandler.ts index cc16afb77c..7418160fb5 100644 --- a/packages/nextly/src/routeHandler.ts +++ b/packages/nextly/src/routeHandler.ts @@ -342,6 +342,7 @@ const DIRECT_DISPATCH_SERVICES = new Set([ "webhooks", "generalSettings", "previewLinks", + "previewUrl", "imageSizes", "dashboard", "schema", From 19f70271203097ab53b91877932145465701d17c Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 16 Aug 2026 03:12:45 +0500 Subject: [PATCH 8/9] test(nextly): pin the site-url scheme refusal at the write boundary --- .../nextly/src/api/general-settings.test.ts | 57 +++++++++++++++++-- 1 file changed, 53 insertions(+), 4 deletions(-) diff --git a/packages/nextly/src/api/general-settings.test.ts b/packages/nextly/src/api/general-settings.test.ts index d5de907c60..51c307ad2e 100644 --- a/packages/nextly/src/api/general-settings.test.ts +++ b/packages/nextly/src/api/general-settings.test.ts @@ -33,10 +33,7 @@ vi.mock("../di", () => ({ import { isErrorResponse, requireAnyPermission } from "../auth/middleware"; import { container } from "../di"; -import { - getGeneralSettings, - updateGeneralSettings, -} from "./general-settings"; +import { getGeneralSettings, updateGeneralSettings } from "./general-settings"; const SETTINGS = { applicationName: "Nextly", @@ -93,4 +90,56 @@ describe("updateGeneralSettings", () => { expect(json.message).toMatch(/updated/i); expect(json.item).toEqual(updated); }); + + it("refuses a site URL whose scheme the browser would execute", async () => { + // `z.string().url()` accepts every one of these — measured — because the + // WHATWG parser does. The value is concatenated into preview URLs the admin + // assigns to location.href, where such a scheme runs script in the admin's + // own origin rather than navigating. + const executable = [ + "javascript:alert(document.cookie)", + "data:text/html,", + "vbscript:msgbox(1)", + "file:///etc/passwd", + ]; + const updateSettings = vi.fn(); + (container.get as ReturnType).mockReturnValue({ + updateSettings, + }); + + expect(executable.length).toBeGreaterThan(0); + for (const siteUrl of executable) { + const res = await updateGeneralSettings( + new Request("http://x/api/nextly/general-settings", { + method: "PATCH", + body: JSON.stringify({ siteUrl }), + }) + ); + + expect(res.status).toBe(400); + } + + // Rejected before the write, not merely reported afterwards. + expect(updateSettings).not.toHaveBeenCalled(); + }); + + it("still accepts an ordinary http(s) site URL", async () => { + // The positive control for the refusal above: without it, a validator that + // rejected everything would satisfy that test perfectly. + const updated = { ...SETTINGS, siteUrl: "http://localhost:3000" }; + const updateSettings = vi.fn().mockResolvedValue(updated); + (container.get as ReturnType).mockReturnValue({ + updateSettings, + }); + + const res = await updateGeneralSettings( + new Request("http://x/api/nextly/general-settings", { + method: "PATCH", + body: JSON.stringify({ siteUrl: "http://localhost:3000" }), + }) + ); + + expect(res.status).toBe(200); + expect(updateSettings).toHaveBeenCalled(); + }); }); From 30a3e8b10d0e9e1d8611e6d6b801f4f3a0509a1a Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 16 Aug 2026 10:52:45 +0500 Subject: [PATCH 9/9] fix(nextly): refuse a non-post preview-url request and a dead session key The preview-url parser never received the http method, so a GET matched and reached the json-body handler instead of being answered method-not-allowed. The adjacent preview-link parser rejects non-POST on its first line; this one now does the same. Session storage is partitioned by origin, and a resolved preview url is routinely on another one now that relative paths rebase onto the site url. The unsaved-data key would then name a payload the preview page cannot reach, so it is omitted and the caller is told the preview shows saved content. Appending it anyway looked like it worked while rendering stale data, which is what that path exists to prevent. That report is its own callback rather than a new unavailable reason: one says the click produced nothing, the other that it produced less than was asked for, and the editor acts differently on each. Availability now counts a stored template as declaring a preview. Only the code-first sync writes the boolean, so requiring it hid a preview that a ui-created row already declares. --- .../hooks/__tests__/useEntryPreview.test.ts | 102 +++++++++++++++++- packages/admin/src/hooks/useEntryPreview.ts | 80 +++++++++++++- .../route-parser.preview-url.test.ts | 15 +++ .../nextly/src/route-handler/route-parser.ts | 12 ++- 4 files changed, 201 insertions(+), 8 deletions(-) diff --git a/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts b/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts index f164c4436f..e942d33983 100644 --- a/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts +++ b/packages/admin/src/hooks/__tests__/useEntryPreview.test.ts @@ -57,6 +57,39 @@ describe("isPreviewAvailable", () => { expect(result.current.isPreviewAvailable).toBe(false); }); + it("is true from a stored template alone, with no boolean present", () => { + // A UI-created collection stores its template directly and never goes + // through the code-first sync that writes the boolean, so requiring the + // boolean would hide a preview the stored data plainly declares. Every row + // written before the boolean existed is this case. + const { result } = renderHook(() => + useEntryPreview({ + collection: { + name: "posts", + admin: { preview: { urlTemplate: "/preview/{slug}" } }, + }, + entry: { id: "1" }, + }) + ); + + expect(result.current.isPreviewAvailable).toBe(true); + }); + + it("is false for an empty stored template", () => { + // What a cleared field yields; it declares nothing. + const { result } = renderHook(() => + useEntryPreview({ + collection: { + name: "posts", + admin: { preview: { urlTemplate: "" } }, + }, + entry: { id: "1" }, + }) + ); + + expect(result.current.isPreviewAvailable).toBe(false); + }); + it("is false when the collection stores hasPreview: false", () => { const { result } = renderHook(() => useEntryPreview({ @@ -194,9 +227,12 @@ describe("openPreview", () => { it("sends unsaved form values, not the saved row", async () => { const tab = fakeTab(); openSpy.mockReturnValue(tab as unknown as Window); + // Same-origin on purpose: this test is about WHICH VALUES reach the + // resolver, and a cross-origin URL would additionally drop the session-key + // handoff, mixing a second behaviour into the assertion. resolve.mockResolvedValue({ status: "resolved", - url: "https://s.dev/p/new", + url: `${window.location.origin}/p/new`, }); const { result } = renderHook(() => @@ -216,7 +252,9 @@ describe("openPreview", () => { collection: "posts", entry: { id: "1", slug: "edited" }, }); - expect(tab.location.href).toBe("https://s.dev/p/new?__preview=preview-key"); + expect(tab.location.href).toBe( + `${window.location.origin}/p/new?__preview=preview-key` + ); }); it("stores unsaved data BEFORE opening the tab, not after", async () => { @@ -274,6 +312,66 @@ describe("openPreview", () => { expect(resolve).not.toHaveBeenCalled(); }); + it("drops the unsaved-data key cross-origin and says so, rather than sending a dead one", async () => { + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + // jsdom serves the admin from localhost; the site is elsewhere, which is + // what a configured site URL now routinely means. + resolve.mockResolvedValue({ + status: "resolved", + url: "https://site.example.com/p/1", + }); + const onUnsavedChangesNotSent = vi.fn(); + + const { result } = renderHook(() => + useEntryPreview({ + collection, + entry: { id: "1" }, + getFormValues: () => ({ slug: "edited" }), + onUnsavedChangesNotSent, + }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + // Session storage is partitioned per origin, so the key would name a + // payload the preview page cannot reach. Appending it would look like it + // worked while quietly rendering stale content. + expect(tab.location.href).toBe("https://site.example.com/p/1"); + expect(tab.location.href).not.toContain("__preview"); + // Reported, because silently previewing saved data is the failure mode this + // whole path exists to prevent. + expect(onUnsavedChangesNotSent).toHaveBeenCalled(); + }); + + it("still sends the key when the site is same-origin", async () => { + // The positive control for the drop above: without it, a hook that never + // appended the key would satisfy that test perfectly. + const tab = fakeTab(); + openSpy.mockReturnValue(tab as unknown as Window); + resolve.mockResolvedValue({ + status: "resolved", + url: `${window.location.origin}/p/1`, + }); + const onUnsavedChangesNotSent = vi.fn(); + + const { result } = renderHook(() => + useEntryPreview({ + collection, + entry: { id: "1" }, + getFormValues: () => ({ slug: "edited" }), + onUnsavedChangesNotSent, + }) + ); + await act(async () => { + await result.current.openPreview(); + }); + + expect(tab.location.href).toContain("__preview=preview-key"); + expect(onUnsavedChangesNotSent).not.toHaveBeenCalled(); + }); + it("navigates the current window when the collection opts out of a new tab", async () => { resolve.mockResolvedValue({ status: "resolved", url: "https://s.dev/p/1" }); diff --git a/packages/admin/src/hooks/useEntryPreview.ts b/packages/admin/src/hooks/useEntryPreview.ts index 0adc95e710..9dd71a6be9 100644 --- a/packages/admin/src/hooks/useEntryPreview.ts +++ b/packages/admin/src/hooks/useEntryPreview.ts @@ -33,8 +33,21 @@ import { previewUrlApi } from "@admin/services/previewUrlApi"; * What the panel needs is whether to draw a button and how to label it. */ export interface PreviewConfig { - /** Whether this collection previews at all, decided when the config synced. */ + /** + * Whether this collection previews at all, decided when the config synced. + * + * Written only by the code-first sync, because that is the path whose + * declaration — a function — cannot itself be stored. + */ hasPreview?: boolean; + /** + * A UI-created collection's stored template. + * + * Read here ONLY to answer whether a preview exists. The URL is never built + * from it in the browser: that is the resolver's job, and interpolating it + * here would be the second implementation this design exists to avoid. + */ + urlTemplate?: string; /** Whether to open the preview in a new tab. @default true */ openInNewTab?: boolean; /** Custom label for the preview button. @default "Preview" */ @@ -58,6 +71,17 @@ export interface UseEntryPreviewOptions { getFormValues?: () => Record; /** Told why a click could not open anything. */ onUnavailable?: (reason: PreviewUnavailableReason) => void; + /** + * Told when the preview OPENED but could not carry the editor's unsaved + * edits, so it is showing the last saved version instead. + * + * Deliberately not folded into `onUnavailable`. That reports a click which + * produced nothing; this reports one that produced something less than was + * asked for. Merging them would either suppress a real warning or label a + * working preview as a failure — and the editor's response differs: here + * they can save and click again. + */ + onUnsavedChangesNotSent?: () => void; } /** @@ -93,6 +117,26 @@ export interface UseEntryPreviewResult { label: string; } +// ============================================================================ +// Helpers +// ============================================================================ + +/** + * Whether `url` is served from the origin this admin is running on. + * + * Decides only whether the session-storage handoff can work: that storage is + * partitioned per origin, so a payload written here is unreachable from a + * preview page served anywhere else. Compared by parsed origin rather than by + * string prefix, which `https://site.example.com.evil.test` would satisfy. + */ +function isSameOrigin(url: string): boolean { + try { + return new URL(url).origin === window.location.origin; + } catch { + return false; + } +} + // ============================================================================ // Hook // ============================================================================ @@ -115,13 +159,22 @@ export function useEntryPreview({ entry, getFormValues, onUnavailable, + onUnsavedChangesNotSent, }: UseEntryPreviewOptions): UseEntryPreviewResult { const previewConfig = collection.admin?.preview; - // From the stored boolean, so the button does not flicker in after a fetch + // Answered from stored data, so the button does not flicker in after a fetch // and does not appear for a collection that has no preview at all. + // + // EITHER signal counts, because the two authoring paths store different + // things: code-first syncs the boolean, since its function cannot be stored, + // while a UI-created collection has its template stored directly and may + // carry no boolean at all. Requiring the boolean alone would hide a preview + // that a stored template plainly declares — and every row written before the + // boolean existed is exactly that case. const isPreviewAvailable = useMemo( - () => previewConfig?.hasPreview === true, + () => + previewConfig?.hasPreview === true || Boolean(previewConfig?.urlTemplate), [previewConfig] ); @@ -194,17 +247,34 @@ export function useEntryPreview({ // Unsaved values travel through session storage rather than the URL, so // the preview renders what is on screen instead of what was last saved. // The payload was written above; only the key is appended here. + // + // Session storage is partitioned by ORIGIN, and a resolved preview URL is + // now routinely on a different one — that is what a configured site URL + // means. The key would then name a payload the preview page cannot reach, + // so it is omitted and the caller is told the preview shows saved content. + // Appending it anyway would look like it worked and quietly show stale + // data, which is the failure this whole path exists to prevent. + const sameOrigin = isSameOrigin(resolution.url); const url = - previewKey === undefined + previewKey === undefined || !sameOrigin ? resolution.url : generatePreviewUrlWithData(resolution.url, previewKey); + if (previewKey !== undefined && !sameOrigin) onUnsavedChangesNotSent?.(); + if (target) target.location.href = url; else window.location.href = url; } catch { abandon("failed"); } - }, [collection.name, entry, getFormValues, onUnavailable, previewConfig]); + }, [ + collection.name, + entry, + getFormValues, + onUnavailable, + onUnsavedChangesNotSent, + previewConfig, + ]); return { isPreviewAvailable, diff --git a/packages/nextly/src/route-handler/__tests__/route-parser.preview-url.test.ts b/packages/nextly/src/route-handler/__tests__/route-parser.preview-url.test.ts index 0d30b000ca..f933dae471 100644 --- a/packages/nextly/src/route-handler/__tests__/route-parser.preview-url.test.ts +++ b/packages/nextly/src/route-handler/__tests__/route-parser.preview-url.test.ts @@ -29,6 +29,21 @@ describe("preview-url routes", () => { expect(parseRestRoute(["preview-url", "123", "extra"], "POST")).toEqual({}); }); + it("refuses every method but POST", () => { + // The entry travels in the body, so nothing else can carry a request. Left + // matching, a GET would reach the JSON-body handler rather than being + // answered method-not-allowed — the adjacent preview-links parser rejects + // non-POST on its first line for the same reason. + for (const method of ["GET", "PUT", "PATCH", "DELETE", "HEAD"]) { + expect(parseRestRoute(["preview-url"], method)).toEqual({}); + } + // Positive control: the same route with the right method still resolves, so + // the assertions above are about the METHOD and not a broken route. + expect(parseRestRoute(["preview-url"], "POST")).toMatchObject({ + service: "previewUrl", + }); + }); + it("stays distinct from the preview-links routes", () => { // Same first word, different resource, and they must not answer for each // other: one returns a URL, the other mints a credential. diff --git a/packages/nextly/src/route-handler/route-parser.ts b/packages/nextly/src/route-handler/route-parser.ts index 378e13331f..12b44f68a4 100644 --- a/packages/nextly/src/route-handler/route-parser.ts +++ b/packages/nextly/src/route-handler/route-parser.ts @@ -1784,8 +1784,13 @@ function parsePreviewLinkRoutes( function parsePreviewUrlRoutes( id: string | undefined, subresource: string | undefined, + httpMethod: string, routeParams: Record ): ParsedRoute | null { + // The entry travels in the body, so only POST can carry a request at all. + // Matching regardless of method would hand a GET straight to the JSON-body + // handler rather than answering method-not-allowed. + if (httpMethod !== "POST") return null; if (id !== undefined || subresource !== undefined) return null; return { @@ -2396,7 +2401,12 @@ export function parseRestRoute( // Handle resolving where an entry previews if (resource === "preview-url") { - const result = parsePreviewUrlRoutes(id, subresource, routeParams); + const result = parsePreviewUrlRoutes( + id, + subresource, + httpMethod, + routeParams + ); if (result) return result; }