diff --git a/.changeset/form-submit-redirect-in-shell-4190.md b/.changeset/form-submit-redirect-in-shell-4190.md new file mode 100644 index 0000000000..6ab4d01c50 --- /dev/null +++ b/.changeset/form-submit-redirect-in-shell-4190.md @@ -0,0 +1,14 @@ +--- +"@object-ui/console": patch +--- + +`FormPage`'s post-submit `redirect` behaviour now consumes the destination the way objectstack#7496 ruled it (objectui#4190): as a **relative in-app path**, navigated to with the router, with `{{record.field}}` interpolation URL-escaped when the redirect is built — and an out-of-contract destination refused on screen instead of followed. + +The url was previously handed to a browser-level, full-page navigation exactly as authored. Two consequences, both fixed here: + +- **A ruled in-app path left the app.** A full-page navigation does not see React Router's `basename`, so on a console served under a mount — which the framework CLI configures for every embedded deployment — an authored `/objects/lead` resolved against the origin root and dropped the submitter out of the SPA. Both mounts of this renderer (`/f/:slug` and `/forms/:name`) live inside the console's router, so the destination is now a router navigation and the mount is applied by the router itself. `withConsoleBase()` is deliberately not used: it prefixes anything not already targeting another absolute SPA mount, so it would have mangled an absolute destination rather than fixing it. +- **`{{record.field}}` tokens were never substituted.** The ruled shape accepts them and assigns the substitution — and the URL-escaping of every interpolated value — to the moment the redirect is built, which is here. The scope is the record the submit just wrote (values as submitted, with whatever the server echoed back layered over them, and the id read by the same one rule the `created-record` behaviour uses). + +The shape verdict is not restated in this app: `resolveSubmitRedirect` asks `@objectstack/spec`'s own `FormViewSchema` at the moment of use, so an absolute URL, a protocol-relative `//host`, a backslash, a control-character smuggle, a malformed token or a document-relative path is refused with the spec's own author-facing prescription, and a later widening of the ruling is followed by the pin rather than by an edit here. A refusal confirms the submit — the write succeeded, only the destination was out of contract — and shows the reason, rather than leaving the submitter watching a redirect that must not happen. + +`delayMs` semantics are unchanged. The wait now lives in an effect tied to the component, so a submitter who navigates away during the delay is no longer yanked back by a timer that outlived the page. diff --git a/apps/console/src/components/FormPage.redirect.test.tsx b/apps/console/src/components/FormPage.redirect.test.tsx new file mode 100644 index 0000000000..701381b5f5 --- /dev/null +++ b/apps/console/src/components/FormPage.redirect.test.tsx @@ -0,0 +1,347 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#4190 — where a declared `submitBehavior: { kind: 'redirect' }` + * actually puts the submitter, rendered. + * + * The ruling this implements is objectstack#7496 (2026-08-11), landed in + * `@objectstack/spec` by objectstack#7657 and live on this repo's 17.0.0 GA pin: + * the url is a RELATIVE in-app path, interpolated from declared record fields + * as `{{record.field}}`, URL-escaped when the redirect is built. The value-level + * consequences are pinned in `submitRedirect.test.ts` against the spec's own + * schema; this file pins the two things only a rendered page can show — WHICH + * navigation mechanism runs, and what the submitter sees when the destination is + * out of contract. + * + * ## Reverse verification — predicted first, then measured + * + * 1. **Restoring the browser-level navigation** on the authored string: 7 of the + * 8 tests here go RED — all five navigation cases (the router's location never + * moves, so the destination route never renders) and both refusals. The one + * that stays green is `documents what the browser-level form would have + * resolved to`, which asserts a fact about URL resolution rather than about + * this component. The whole of `submitRedirect.test.ts` also stays green: that + * mutation kills the CALL SITE, leaving the module correct and unused, which + * is precisely why the mechanism has to be pinned here. + * (jsdom does not actually navigate either — which is how the old line's mount + * bug stayed invisible to every test. The assertion had to be "the shell + * moved", never "a full-page navigation was attempted".) + * 2. **Deleting the refusal** and following the authored value: exactly the two + * refusal tests go RED, the six in-contract cases stay green. A narrow, + * non-overlapping change detector. + * 3. **Deleting the escape** in the helper: `escapes a server-side value into one + * path segment` and `interpolates from the submitted values …` go RED here, + * alongside eight in the unit file. See that file's docblock for why the + * schema oracle alone does not catch every unescaped value. + * + * ## Why the basename is on the harness rather than in one test + * + * A bare `/` mount is where this bug hides: prefixed and unprefixed spellings + * coincide there, which is what kept objectui#4181's sibling defect invisible. + * Every render below is mounted under `/_console` so that a regression to a + * mount-blind navigation cannot pass by coincidence. + */ + +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { MemoryRouter, Route, Routes, useLocation } from 'react-router-dom'; +import { toast } from 'sonner'; +import { FormPage } from './FormPage'; + +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +/** The console mount the framework CLI configures for an embedded deployment. */ +const MOUNT = '/_console'; + +function viewEnvelope(submitBehavior: unknown) { + return { + name: 'showcase_task.edit', + object: 'showcase_task', + viewKind: 'form', + label: 'Log Time', + config: { + type: 'simple', + sections: [{ label: 'Task', fields: ['title'] }], + submitBehavior, + }, + }; +} + +const OBJECT_SCHEMA = { + name: 'showcase_task', + label: 'Task', + fields: { title: { type: 'text', label: 'Title' } }, +}; + +/** `CreateDataResponse = { object, id, record }`, as `packages/rest` serves it. */ +const CREATE_RESPONSE = { + object: 'showcase_task', + id: 'task-42', + record: { id: 'task-42', title: 'Write the report', slug: 'write-the-report' }, +}; + +function publicPayload(submitBehavior: unknown) { + return { + slug: 'contact-us', + object: 'showcase_inquiry', + label: 'Contact us', + form: { type: 'simple', sections: [{ fields: ['title'] }], submitBehavior }, + objectSchema: { + name: 'showcase_inquiry', + fields: { title: { type: 'text', label: 'Title' } }, + }, + }; +} + +let submits: Array<{ url: string; body: unknown }> = []; + +function stubFetch(routes: Record) { + return vi.fn(async (url: string, init?: RequestInit) => { + if (init?.method === 'POST' || init?.method === 'PATCH') { + submits.push({ url, body: init.body ? JSON.parse(String(init.body)) : undefined }); + } + const key = Object.keys(routes).find((k) => String(url).includes(k)); + if (!key) throw new Error(`unstubbed fetch: ${url}`); + return { + ok: true, + status: 200, + statusText: 'OK', + json: async () => routes[key], + text: async () => JSON.stringify(routes[key]), + } as unknown as Response; + }); +} + +/** Where the ROUTER settles — basename-stripped, which is the in-shell path. */ +function LocationProbe() { + const location = useLocation(); + return
{location.pathname + location.search}
; +} + +/** + * Mount the internal form under the console mount, with two plausible redirect + * destinations declared as real routes so "went there" is an assertion about the + * shell and not about a spy. + */ +function renderInternal() { + return render( + + + + } /> + thanks page} /> + slug page} /> + + , + ); +} + +function renderPublic() { + return render( + + + + } /> + thanks page} /> + + , + ); +} + +beforeEach(() => { + submits = []; + vi.mocked(toast.error).mockClear(); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('a ruled relative url is an in-shell route', () => { + it('lands inside the shell, on the route the path names', async () => { + vi.stubGlobal( + 'fetch', + stubFetch({ + '/meta/view/': viewEnvelope({ kind: 'redirect', url: '/thanks' }), + '/meta/object/': OBJECT_SCHEMA, + '/data/showcase_task': CREATE_RESPONSE, + }), + ); + renderInternal(); + + await screen.findByLabelText(/Title/); + await userEvent.type(screen.getByLabelText(/Title/), 'Write the report'); + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + // The shell moved: the destination route is mounted and the router's own + // location is the authored path. A full-page navigation cannot produce + // either — it leaves this router untouched (and, under the mount, resolves + // against the origin root, which is the defect the card filed). + expect(await screen.findByTestId('thanks-page')).toBeInTheDocument(); + expect(screen.getByTestId('location').textContent).toBe('/thanks'); + // And the write really happened before any of that. + expect(submits).toHaveLength(1); + expect(submits[0].url).toContain('/data/showcase_task'); + }); + + /** + * Why the mechanism is load-bearing, in the idiom `consoleBase.test.ts` + * already uses: resolve the authored path the way a full-page navigation + * would, against the document base the framework CLI injects for a mounted + * console. It answers the ORIGIN ROOT, outside the mount — so consuming a + * ruled in-app path that way drops the submitter out of the SPA no matter + * what the path says. The router applies the basename instead, which is what + * the test above observes. + */ + it('documents what the browser-level form would have resolved to', () => { + const baseEl = document.createElement('base'); + baseEl.setAttribute('href', `${MOUNT}/`); + document.head.appendChild(baseEl); + try { + expect(new URL('/thanks', document.baseURI).pathname).toBe('/thanks'); + expect(new URL('/thanks', document.baseURI).pathname).not.toBe(`${MOUNT}/thanks`); + } finally { + baseEl.remove(); + } + }); + + it('substitutes the id of the record the submit just wrote', async () => { + vi.stubGlobal( + 'fetch', + stubFetch({ + '/meta/view/': viewEnvelope({ kind: 'redirect', url: '/thanks?ref={{record.id}}' }), + '/meta/object/': OBJECT_SCHEMA, + '/data/showcase_task': CREATE_RESPONSE, + }), + ); + renderInternal(); + + await screen.findByLabelText(/Title/); + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + expect(await screen.findByTestId('thanks-page')).toBeInTheDocument(); + // `task-42` came off the create response, read by the same one rule the + // `created-record` arm uses — not from the values the user typed. + expect(screen.getByTestId('location').textContent).toBe('/thanks?ref=task-42'); + }); + + it('escapes a server-side value into one path segment', async () => { + vi.stubGlobal( + 'fetch', + stubFetch({ + '/meta/view/': viewEnvelope({ kind: 'redirect', url: '/t/{{record.slug}}' }), + '/meta/object/': OBJECT_SCHEMA, + '/data/showcase_task': { + ...CREATE_RESPONSE, + record: { ...CREATE_RESPONSE.record, slug: 'a/b c' }, + }, + }), + ); + renderInternal(); + + await screen.findByLabelText(/Title/); + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + expect(await screen.findByTestId('slug-page')).toBeInTheDocument(); + expect(screen.getByTestId('location').textContent).toBe('/t/a%2Fb%20c'); + }); + + it('interpolates from the submitted values on the anonymous path', async () => { + vi.stubGlobal( + 'fetch', + stubFetch({ + '/forms/contact-us/submit': { ok: true }, + '/forms/contact-us': publicPayload({ + kind: 'redirect', + url: '/thanks?t={{record.title}}', + }), + }), + ); + renderPublic(); + + await screen.findByLabelText(/Title/); + await userEvent.type(screen.getByLabelText(/Title/), 'Hello there'); + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + // The public submit answers no record, so the scope is what was submitted — + // still "the record just submitted", which is the whole scope the ruling + // gives a post-submit redirect. + expect(await screen.findByTestId('thanks-page')).toBeInTheDocument(); + expect(screen.getByTestId('location').textContent).toBe('/thanks?t=Hello%20there'); + }); + + it('keeps the declared delay: the confirmation shows first', async () => { + vi.stubGlobal( + 'fetch', + stubFetch({ + '/meta/view/': viewEnvelope({ kind: 'redirect', url: '/thanks', delayMs: 60 }), + '/meta/object/': OBJECT_SCHEMA, + '/data/showcase_task': CREATE_RESPONSE, + }), + ); + renderInternal(); + + await screen.findByLabelText(/Title/); + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + // Still on the form route, showing the interstitial the delay exists for. + expect(await screen.findByText('Redirecting…')).toBeInTheDocument(); + expect(screen.getByTestId('location').textContent).toBe('/forms/showcase_task.edit'); + // …and it arrives once the delay elapses. + expect(await screen.findByTestId('thanks-page')).toBeInTheDocument(); + expect(screen.getByTestId('location').textContent).toBe('/thanks'); + }); +}); + +describe('an out-of-contract destination is refused, not followed', () => { + it('refuses an absolute destination and says so on screen', async () => { + vi.stubGlobal( + 'fetch', + stubFetch({ + '/meta/view/': viewEnvelope({ kind: 'redirect', url: 'https://example.com/thanks' }), + '/meta/object/': OBJECT_SCHEMA, + '/data/showcase_task': CREATE_RESPONSE, + }), + ); + renderInternal(); + + await screen.findByLabelText(/Title/); + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + // The refusal is the spec's own prescription, on screen — not a toast that + // scrolls away, and emphatically not silence. + const refusal = await screen.findByText(/accepts a RELATIVE path only/); + expect(refusal).toBeInTheDocument(); + expect(vi.mocked(toast.error)).toHaveBeenCalledWith( + expect.stringContaining('RELATIVE path only'), + ); + + // The write succeeded, so the submitter is told that too. Refusing the + // destination must not read as "your submission failed". + expect(screen.getByText('Your submission has been received.')).toBeInTheDocument(); + expect(submits).toHaveLength(1); + + // Nothing was navigated to, and no interstitial promises otherwise. + expect(screen.getByTestId('location').textContent).toBe('/forms/showcase_task.edit'); + expect(screen.queryByText('Redirecting…')).not.toBeInTheDocument(); + }); + + it('refuses a protocol-relative destination — the leading slash is not enough', async () => { + vi.stubGlobal( + 'fetch', + stubFetch({ + '/forms/contact-us/submit': { ok: true }, + '/forms/contact-us': publicPayload({ kind: 'redirect', url: '//example.com/thanks' }), + }), + ); + renderPublic(); + + await screen.findByLabelText(/Title/); + await userEvent.click(screen.getByRole('button', { name: /Submit/ })); + + expect(await screen.findByText(/protocol-relative/)).toBeInTheDocument(); + expect(screen.getByTestId('location').textContent).toBe('/f/contact-us'); + expect(screen.queryByTestId('thanks-page')).not.toBeInTheDocument(); + }); +}); diff --git a/apps/console/src/components/FormPage.tsx b/apps/console/src/components/FormPage.tsx index 2e8baaa524..7539cd1cb5 100644 --- a/apps/console/src/components/FormPage.tsx +++ b/apps/console/src/components/FormPage.tsx @@ -37,11 +37,23 @@ * {@link FORM_RECORD_OBJECT_PARAM} for why it can only ever refuse, and the * guard in {@link loadInternalForm} for where it fires (before any `/data/` * request, so a mismatch reads nothing and writes nothing). + * + * ## Where a declared `redirect` goes (objectui#4190) + * + * `submitBehavior: { kind: 'redirect' }` was consumed as a browser-level + * navigation on the authored string, which is why the card asked what that + * string even meant. objectstack#7496 ruled it: a RELATIVE in-app path, with + * `{{record.field}}` interpolation, URL-escaped when the redirect is built. + * So the destination is a route in this shell and it is navigated to with the + * router — a full-page navigation ignores the console mount and drops the + * submitter at the origin root — while an out-of-contract absolute is refused + * on screen instead of followed. See `submitRedirect.ts`. */ import { useEffect, useMemo, useState, type FormEvent } from 'react'; import { useNavigate, useParams, useSearchParams } from 'react-router-dom'; import { toast } from 'sonner'; +import { resolveSubmitRedirect } from './submitRedirect'; const API_BASE = (import.meta.env.VITE_SERVER_URL || '') + '/api/v1'; @@ -84,7 +96,15 @@ interface FormViewSpec { submitBehavior?: SubmitBehavior; } -/** Mirrors the spec FormView.submitBehavior union (added in Step 4). */ +/** + * Mirrors the spec FormView.submitBehavior union (added in Step 4). + * + * `redirect.url` stays a plain string here because that is what the contract + * ships: the ruled shape (objectstack#7496) is a refinement ON a string, so the + * key arrives as the author wrote it. What it is ALLOWED to say is not restated + * in this type — `resolveSubmitRedirect` asks the spec's own schema at the + * moment of use (`submitRedirect.ts`). + */ type SubmitBehavior = | { kind: 'thank-you'; title?: string; message?: string } | { kind: 'redirect'; url: string; delayMs?: number } @@ -1010,6 +1030,22 @@ export function FormPage({ mode, recordPath }: FormPageProps) { const [values, setValues] = useState>({}); const [submitting, setSubmitting] = useState(false); const [submitted, setSubmitted] = useState(false); + /** + * The outcome of a `redirect` submit behaviour: the in-app route to go to + * once `delayMs` has elapsed, or the fact that the authored destination was + * refused (objectui#4190). Null until a redirect submit resolves. + * + * A refusal needs its own state rather than riding on `error`, for two + * reasons. This page renders "Redirecting…" for a submitted redirect, and that + * line would be a lie — nothing is going anywhere, and the submitter needs the + * confirmation their record WAS written plus the reason it stopped here. And + * the refusal travels WITH that flag rather than in `error` so one fact has + * one reader: `error` is the page's load/submit failure channel, which a + * refused destination is not (the submit succeeded). + */ + const [redirect, setRedirect] = useState< + { kind: 'pending'; path: string } | { kind: 'refused'; refusal: string } | null + >(null); // Load spec on mount / when identifier or the record it targets changes. useEffect(() => { @@ -1059,6 +1095,24 @@ export function FormPage({ mode, recordPath }: FormPageProps) { loaded?.form?.submitBehavior, ); + /** + * The delayed leg of a `redirect` submit behaviour. + * + * It is an effect rather than a timer armed inside the submit handler so the + * wait is TIED to this component: a submitter who leaves during `delayMs` is + * not yanked back by a timer that outlived the page. `delayMs` semantics are + * otherwise unchanged — the pause is what makes the confirmation readable, + * the spec still declares the key, and an unset one still means "go now" + * (a zero-delay timer, i.e. after this render commits). + */ + const pendingRedirect = redirect?.kind === 'pending' ? redirect.path : null; + const redirectDelayMs = behavior.kind === 'redirect' ? (behavior.delayMs ?? 0) : 0; + useEffect(() => { + if (pendingRedirect === null) return; + const timer = setTimeout(() => navigate(pendingRedirect), redirectDelayMs); + return () => clearTimeout(timer); + }, [pendingRedirect, redirectDelayMs, navigate]); + const handleSubmit = async (e: FormEvent) => { e.preventDefault(); if (!loaded) return; @@ -1098,8 +1152,36 @@ export function FormPage({ mode, recordPath }: FormPageProps) { break; } case 'redirect': { - const delay = behavior.delayMs ?? 0; - setTimeout(() => window.location.assign(behavior.url), delay); + // objectstack#7496 ruled this url a RELATIVE in-app path, so the + // destination is a route in this shell — see `submitRedirect.ts` for + // why the verdict is the spec's own, why a browser-level navigation + // was the objectui#4190 defect rather than the mechanism, and why + // `withConsoleBase` is not the tool. + // + // The token scope is the record this submit just wrote: the values as + // submitted, with whatever the server echoed back layered over them + // (defaults and computed fields are canonical there). The id is read + // exactly as the `created-record` arm above reads it, so ONE rule + // answers "the record this submit wrote" for both arms and a + // `{{record.id}}` token cannot resolve one way here and another there. + const written = unwrapTransportEnvelope(result)?.record; + const writtenId = editingId ?? readCreatedRecordId(result); + const verdict = resolveSubmitRedirect(behavior.url, { + ...values, + ...(written && typeof written === 'object' ? (written as Record) : {}), + ...(writtenId ? { id: writtenId } : {}), + }); + if (!verdict.ok) { + // The write SUCCEEDED and only the destination is out of contract. + // So: confirm the submit, and put the refusal on screen — dropping + // it would leave the submitter watching a redirect that must never + // happen, and following it is the open redirect the ruling closed. + toast.error(verdict.refusal); + setRedirect({ kind: 'refused', refusal: verdict.refusal }); + setSubmitted(true); + break; + } + setRedirect({ kind: 'pending', path: verdict.path }); setSubmitted(true); break; } @@ -1165,6 +1247,24 @@ export function FormPage({ mode, recordPath }: FormPageProps) { ); } if (submitted && behavior.kind === 'redirect') { + // A refused destination (objectui#4190) still confirms the write — it + // happened — and shows why nothing was navigated to. "Redirecting…" is + // reserved for a redirect that is actually pending. + if (redirect?.kind === 'refused') { + return ( +
+
+

Thanks!

+

+ Your submission has been received. +

+
+
+ {redirect.refusal} +
+
+ ); + } return (
Redirecting… diff --git a/apps/console/src/components/submitRedirect.test.ts b/apps/console/src/components/submitRedirect.test.ts new file mode 100644 index 0000000000..ffddf98b69 --- /dev/null +++ b/apps/console/src/components/submitRedirect.test.ts @@ -0,0 +1,229 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `resolveSubmitRedirect` — objectui#4190, against the shape objectstack#7496 + * ruled and objectstack#7657 landed in `@objectstack/spec` (17.0.0 GA). + * + * ## What is pinned here, and what it is pinned AGAINST + * + * The module asks the spec's schema for its shape verdict instead of restating + * the rule, so a table of hand-written expectations would pin this file's + * opinion rather than the contract. Both accept/refuse suites therefore use the + * REAL `FormViewSchema` as the oracle — `specAccepts()` below is the same parse + * an authoring door performs — and assert that the two verdicts agree. That + * makes these tests a drift detector for the contract, not for a copy of it: + * if a later spec release widens or narrows the ruled shape, they follow it, and + * they fail only if the renderer and the door ever disagree about one value. + * + * The strong pin is `emits a path the contract still accepts`: ruling point 2 + * requires every interpolated value to be URL-escaped when the redirect is + * built, so the oracle is turned on the OUTPUT — whatever this module emits, + * hostile record values and all, must still be a value the contract would + * accept. + * + * ## Reverse verification — measured, and NOT what the obvious reading predicts + * + * Deleting the escape (interpolating the raw value) turns 10 tests red across + * this file and `FormPage.redirect.test.tsx`, and WHICH assertion catches each + * value is the part worth writing down, because re-parsing the emitted string is + * necessary and **not sufficient**: + * + * - the schema oracle catches only values whose raw form breaks the ruled shape + * ANYWHERE in the string — a backslash, whitespace, a control character. Those + * checks are not anchored to the start, so they still fire mid-path. + * - an address or a script scheme is NOT caught by it. Raw-interpolated, + * `/t/{{record.slug}}` becomes `/t/https://evil.example/steal`, which starts + * with `/` and carries no leading scheme — a perfectly spec-valid relative + * path that goes somewhere the author did not write. Only + * `toContain(encodeURIComponent(hostile))` sees that: the harm is injected + * path STRUCTURE, and relative-only has nothing to say about it. + * - a brace pair in the record trips the unresolved-interpolation backstop + * instead, refusing rather than emitting a token-bearing URL. + * + * So the two assertions in that block are not belt-and-braces; they cover + * disjoint halves of "escaped enough", and dropping the second one would leave + * the escape pinned only for the values the contract happens to reject twice. + * + * Deleting the whole parse and returning the url verbatim inverts the refusal + * suite: every out-of-contract case goes red at once, which is the change + * detector for this module's reason to exist. Deleting the CALL SITE instead + * (restoring the browser-level navigation in `FormPage`) leaves this entire file + * green — the module would simply be unused — which is why the mechanism is + * pinned next to the rendered page and not here. + * + * Control characters below are written as escape sequences on purpose — a raw + * one makes the whole file read as binary to grep, and this repo has paid for + * that four times. + */ + +import { describe, expect, it } from 'vitest'; +import { FormViewSchema } from '@objectstack/spec/ui'; +import { resolveSubmitRedirect } from './submitRedirect'; + +/** + * The contract's verdict on one authored value — the same minimal parse the + * module performs, spelled out here independently so the test states the + * question rather than borrowing the module's answer. + */ +function specAccepts(url: string): boolean { + return FormViewSchema.safeParse({ submitBehavior: { kind: 'redirect', url } }).success; +} + +/** Values the ruling allows: rooted, relative, optionally interpolated. */ +const IN_CONTRACT = [ + '/thanks', + '/objects/lead', + '/thanks?ref=42', + '/thanks%20you', + '/thanks?ref={{record.id}}', + '/t/{{record.slug}}/done', + '/thanks?a={{record.id}}&b={{record.status}}', +]; + +/** + * Values the ruling refuses, one per family the spec's check defends. Named by + * what each one would have done had it been followed. + */ +const OUT_OF_CONTRACT: Array<[label: string, url: string]> = [ + ['an absolute URL — the open redirect the ruling closed', 'https://example.com/thanks'], + ['a script scheme, the same refusal for a stronger reason', 'javascript:alert(1)'], + ['a data scheme', 'data:text/html,

hi

'], + ['protocol-relative: another origin despite the leading slash', '//example.com/thanks'], + ['a backslash, which browsers normalise to a slash while resolving', '/\\example.com'], + ['whitespace, stripped before resolving and hiding the real start', '/ thanks'], + ['a tab, same smuggle in a form that is easy to miss', '/\u0009thanks'], + ['document-relative, so one form lands in different places', 'thanks'], + ['empty — not a destination at all', ''], + ['a single-brace near-miss of the token spelling', '/thanks?x={record.id}'], + ['a token whose field segment is not field grammar', '/thanks?x={{record.Id}}'], + ['a token with inner spacing', '/thanks?x={{ record.id }}'], +]; + +describe('the shape verdict is the contract’s, for every family', () => { + it.each(IN_CONTRACT)('accepts %j, and so does the schema', (url) => { + expect(specAccepts(url)).toBe(true); + expect(resolveSubmitRedirect(url, {}).ok).toBe(true); + }); + + it.each(OUT_OF_CONTRACT)('refuses %s', (_label, url) => { + // Direction first: the contract itself rejects this value. A fixture that + // the schema accepted would make the refusal below this module's private + // opinion, which is the thing these tests exist to rule out. + expect(specAccepts(url)).toBe(false); + + const verdict = resolveSubmitRedirect(url, { id: 'r1' }); + expect(verdict.ok).toBe(false); + if (verdict.ok) return; + // Refusals are quotable: the author gets the spec's own prescription, which + // names the key and cites the ruling it comes from, so the sentence read + // here is the one the authoring door would have said. An empty or generic + // message would be a silent drop wearing an error's clothes. + expect(verdict.refusal).toMatch(/`(submitBehavior\.)?url`/); + expect(verdict.refusal).toContain('#7496'); + expect(verdict.refusal.length).toBeGreaterThan(40); + }); + + it('points an absolute destination at the surface that IS declared for one', () => { + const verdict = resolveSubmitRedirect('https://example.com/thanks', {}); + expect(verdict.ok).toBe(false); + if (verdict.ok) return; + expect(verdict.refusal).toContain('RELATIVE'); + // The ruling's alternative for a deliberately external destination. + expect(verdict.refusal).toContain("{ type: 'url', url }"); + }); +}); + +describe('interpolation — the consumer’s half of the ruling', () => { + it('substitutes a declared field from the record just written', () => { + expect(resolveSubmitRedirect('/thanks?ref={{record.id}}', { id: 'task-42' })).toEqual({ + ok: true, + path: '/thanks?ref=task-42', + }); + }); + + it('substitutes every occurrence, not just the first', () => { + expect( + resolveSubmitRedirect('/r/{{record.id}}/x/{{record.id}}', { id: '7' }), + ).toEqual({ ok: true, path: '/r/7/x/7' }); + }); + + it('renders numbers and booleans, which JSON records legitimately carry', () => { + expect(resolveSubmitRedirect('/t?n={{record.n}}&b={{record.b}}', { n: 42, b: true })).toEqual({ + ok: true, + path: '/t?n=42&b=true', + }); + }); + + /** + * A blank optional field is DATA, not an authoring defect, and this layer is + * explicitly not the one that judges whether a token names a declared field — + * that needs the object declaration, which `@objectstack/lint`'s + * reference-integrity family holds. Emptying is the honest answer here; + * refusing would be this renderer overreaching on the schema's behalf. + */ + it('leaves an absent or null value empty rather than refusing', () => { + expect(resolveSubmitRedirect('/t?ref={{record.note}}', { note: null })).toEqual({ + ok: true, + path: '/t?ref=', + }); + expect(resolveSubmitRedirect('/t?ref={{record.missing}}', {})).toEqual({ + ok: true, + path: '/t?ref=', + }); + }); + + it('leaves a non-scalar empty — a flat token has no object form to write', () => { + expect(resolveSubmitRedirect('/t?ref={{record.owner}}', { owner: { id: 'u1' } })).toEqual({ + ok: true, + path: '/t?ref=', + }); + }); + + it('escapes the value: a token is a value, never new path structure', () => { + expect(resolveSubmitRedirect('/t/{{record.slug}}/done', { slug: 'a/b' })).toEqual({ + ok: true, + path: '/t/a%2Fb/done', + }); + expect(resolveSubmitRedirect('/t?q={{record.q}}', { q: 'a b&c=d' })).toEqual({ + ok: true, + path: '/t?q=a%20b%26c%3Dd', + }); + }); + + /** + * THE pin for ruling point 2. Every one of these record values is an attempt + * to turn an accepted relative path into something else — an address, a + * traversal onto another origin, a control-character smuggle — and the oracle + * is the contract itself: whatever this module emits must still be a value the + * authoring door would accept. + */ + it.each([ + ['an absolute address', 'https://evil.example/steal'], + ['a protocol-relative address', '//evil.example/steal'], + ['a backslash traversal', '\\\\evil.example'], + ['a script scheme', 'javascript:alert(1)'], + ['whitespace', ' leading space'], + ['a control character', '\u0001'], + ['a brace pair that looks like a token', '{{record.id}}'], + ])('emits a path the contract still accepts, with %s in the record', (_label, hostile) => { + const verdict = resolveSubmitRedirect('/t/{{record.slug}}?ref={{record.slug}}', { + slug: hostile, + }); + expect(verdict.ok).toBe(true); + if (!verdict.ok) return; + expect(specAccepts(verdict.path)).toBe(true); + // And the hostile value is still THERE, escaped — refusing to interpolate + // would be a different bug from interpolating unsafely. + expect(verdict.path).toContain(encodeURIComponent(hostile)); + }); + + it('never emits an unresolved interpolation', () => { + for (const url of IN_CONTRACT) { + const verdict = resolveSubmitRedirect(url, { id: 'x', slug: 's', status: 'open' }); + expect(verdict.ok).toBe(true); + if (!verdict.ok) continue; + expect(verdict.path).not.toContain('{'); + expect(verdict.path).not.toContain('}'); + } + }); +}); diff --git a/apps/console/src/components/submitRedirect.ts b/apps/console/src/components/submitRedirect.ts new file mode 100644 index 0000000000..e3b0d96efb --- /dev/null +++ b/apps/console/src/components/submitRedirect.ts @@ -0,0 +1,229 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Where a form's `submitBehavior: { kind: 'redirect' }` sends the submitter + * (objectui#4190), under the shape objectstack#7496 ruled on 2026-08-11 and + * objectstack#7657 landed in `@objectstack/spec` (live on the 17.0.0 GA pin + * this repo installs). + * + * The ruling has three points, and they divide cleanly into three layers: + * + * 1. **relative paths only** — an absolute or protocol-relative URL is + * refused. This is the open-redirect face: the destination is authored + * metadata, which is exactly where an address somebody else chose gets + * copied in. + * 2. **interpolation only from declared record fields**, spelled + * `{{record.field_name}}`, with every interpolated value **URL-escaped + * when the redirect is built**. + * 3. a **verbatim redirect on the resolved relative path** is the intended + * consumption — the destination is that string with its tokens + * substituted, and nothing else. + * + * Point 1 is a property of the string, so the spec enforces it at the authoring + * door. Whether `{{record.foo}}` names a field the form's object actually + * declares needs both the string and the object, so `@objectstack/lint`'s + * reference-integrity family owns that. Point 2's "when the redirect is built" + * and point 3's consumption are runtime, and this module is where they happen. + * + * ## The shape verdict is the spec's, not a copy of it + * + * `@objectstack/spec` does not export its URL check as a function, but it does + * export the schema the check lives in, so the verdict here is produced by + * PARSING the smallest form view that carries this behavior. That costs one + * parse per submit and buys the property that matters: there is no second + * spelling of a security rule in this repo to drift from the first. When the + * ruling widens — it says an allowlist of absolute origins "waits for measured + * demand" — the console follows the pin with no edit here, which is the same + * lag this card sat blocked on for six days. + * + * The alternative was a local re-implementation of the seven refusal families + * kept honest by a parity table. `scripts/check-spec-symbol-derivation.mjs` + * exists because this repo has paid for that shape four times (objectstack#4115, + * objectui#4074/#4588/#4592): a hand copy passes every value comparison right + * up to the release that moves the original. + * + * Parsing a MINIMAL view is load-bearing, not laziness. A whole stored + * `FormView` refuses on any unrelated key the strict schema does not know, and + * a redirect must not be refused because some other part of the metadata + * drifted. The question asked here is narrow — "is this url a value the + * contract allows?" — so only that value is submitted for judgement, and only + * issues on that value's path are read back. + * + * Neither the import nor the parse is new ground in this repo, which is worth + * knowing before weighing the cost: + * + * - `@object-ui/app-shell` already validates authored metadata drafts on the + * client against these same schemas and surfaces their issue messages + * (`views/metadata-admin/clientValidation.ts`), so "ask the spec at the + * consumer" is the established pattern rather than a new one here. + * - the bundle cost is nil, measured: `@objectstack/spec` is already in the + * console's `vendor-objectstack` chunk (`vite.config.ts` names it a manual + * chunk group) because app-shell — a core console dependency — imports + * `@objectstack/spec/ui` at runtime in several modules. This import adds a + * reference to a module the bundle already carries. That is also why it is a + * static import rather than the lazy one `clientValidation` uses: there is + * no chunk to defer, and making it lazy would only force this function to be + * async on the submit path. + * + * ## Why the accepted value is an in-app route + * + * A ruled-relative path IS a route in this shell, and objectui#4190 was filed + * because the redirect arm handed it to the browser as a full-page navigation + * instead. That form of navigation does not see React Router's basename, so on + * a console served under a mount (the framework CLI configures one for every + * embedded deployment) an authored `/objects/lead` resolves against the ORIGIN + * root and leaves the SPA — the same class objectui#4181 fixed on the auth + * pages. Both mounts of this renderer are inside the console's router + * (`/f/:slug` and `/forms/:name` are siblings under one `BrowserRouter + * basename=…`), so a router navigation is what makes "in-app" true. + * + * `withConsoleBase()` — objectui#4181's answer — is deliberately NOT used, and + * the card's reason still holds: it prefixes anything not already targeting + * another absolute SPA mount, so it would have mangled the absolute case rather + * than fixing it. With absolutes now refused at the door, the helper is not + * needed either: the router applies the basename itself, from the same injected + * `` the helper reads. One mount source, no prefixing arithmetic. + * + * A path that matches no route lands on the shell's own not-found, which is the + * author's error made visible in the app rather than an origin-root 404 with + * the session left behind. + */ + +import { FormViewSchema } from '@objectstack/spec/ui'; + +/** + * The one interpolation the ruled `url` accepts, as a capture of the field + * segment. The grammar (lowercase snake_case) is the one `object.fields` keys + * are declared under, narrowed by the ruling to a FLAT segment under the + * `record.` root — the record just submitted is the whole scope a post-submit + * moment has. + * + * This spelling is the one place the spec's rule is restated rather than asked, + * because substitution is the half of the ruling assigned to the consumer and + * the spec exports no substituter. It is safe in one direction only, and that + * is deliberate: it runs solely on strings the schema has already accepted, and + * anything it fails to consume leaves a brace behind, which + * {@link resolveSubmitRedirect} treats as a refusal. So a future divergence + * from the spec's token grammar can only ever refuse a redirect loudly — never + * navigate to a half-substituted URL. + */ +const RECORD_TOKEN_RE = /\{\{record\.([a-z_][a-z0-9_]*)\}\}/g; + +/** Accepted: `path` is the resolved in-app route to navigate to. */ +interface SubmitRedirectAccepted { + ok: true; + path: string; +} + +/** Refused: `refusal` is author-facing prose explaining what to write instead. */ +interface SubmitRedirectRefused { + ok: false; + refusal: string; +} + +export type SubmitRedirectVerdict = SubmitRedirectAccepted | SubmitRedirectRefused; + +/** + * The string form of a record value inside a URL. + * + * Scalars only, and that is not a shortcut: the ruling accepts a FLAT field + * segment, so a token can only ever name a top-level field, and a field whose + * value is an object or array has no scalar form to put in a path. Absent and + * null read as empty — a blank optional field is data, not an authoring defect, + * and this layer is explicitly not the one that judges whether the field was + * declared (see the module docblock). + * + * The scope is a JSON record as it came off the API, so dates and references + * arrive already stringified. + */ +function urlValue(value: unknown): string { + if (value == null) return ''; + if (typeof value === 'string') return value; + if (typeof value === 'boolean') return String(value); + if (typeof value === 'number') return Number.isFinite(value) ? String(value) : ''; + return ''; +} + +/** + * Resolve an authored `submitBehavior.url` into the in-app route to navigate + * to, or refuse it. + * + * @param url the authored value, exactly as the metadata carries it + * @param record the record this submit just wrote — the token scope + * + * Refusal is fail-closed and quotable: the message is the spec's own + * prescription (it names the key, the rule, and what to write instead, + * including that a deliberately external destination is an app navigation item + * rather than this key), so the author reads the same sentence here that the + * authoring door would have told them. + * + * ## Why substitution cannot widen the destination + * + * Every interpolated value goes through `encodeURIComponent`, which escapes the + * characters that could add structure — `/` and `:`, along with `?`, `#` and the + * rest. So a field carrying an address, a traversal, or a space becomes one + * opaque segment or query value: a token is a VALUE in the path, never a way to + * add path structure. + * + * This escape carries the whole weight of that property, and re-parsing the + * result would NOT be enough on its own — measured, not assumed. Interpolated + * raw, `/t/{{record.slug}}` with an address in `slug` becomes + * `/t/https://evil.example/steal`, which still starts with `/` and carries no + * leading scheme: a spec-VALID relative path pointing somewhere the author never + * wrote. Relative-only is a rule about where a path starts, so it has nothing to + * say about structure injected further along. `submitRedirect.test.ts` therefore + * pins both halves — the emitted string re-parses green, AND the escaped value is + * present in it. + */ +export function resolveSubmitRedirect( + url: string, + record: Record, +): SubmitRedirectVerdict { + const parsed = FormViewSchema.safeParse({ submitBehavior: { kind: 'redirect', url } }); + + if (!parsed.success) { + // Only this value was submitted for judgement, so an issue on its path is + // the answer; the two fallbacks exist so a refusal is never silent, not + // because either is expected to be reached. + const onUrl = parsed.error.issues.find( + (issue) => issue.path[0] === 'submitBehavior' && issue.path[1] === 'url', + ); + return { + ok: false, + refusal: + onUrl?.message + ?? parsed.error.issues[0]?.message + ?? `\`submitBehavior.url\` is not a value this contract accepts: ${JSON.stringify(url)}.`, + }; + } + + // Read the value back off the parse rather than reusing the input: the schema + // is the authority on what it accepted, so if it ever normalises the string + // this follows without a second edit. Today the two are identical — the key + // is a plain string with a refinement, deliberately, so that what reaches + // this renderer stays the string the author wrote. + const behavior = parsed.data.submitBehavior; + const accepted = behavior?.kind === 'redirect' ? behavior.url : url; + + const path = accepted.replace(RECORD_TOKEN_RE, (_token, field: string) => + encodeURIComponent(urlValue(record[field])), + ); + + if (path.includes('{') || path.includes('}')) { + // Unreachable while this module's token grammar matches the schema's — the + // schema refuses a brace that is not a well-formed token. It is a refusal + // rather than an assertion because the failure it guards is a future + // widening upstream, and navigating to a URL with an unsubstituted token in + // it is the one outcome that must not happen. + return { + ok: false, + refusal: + '`submitBehavior.url` carries an interpolation this renderer could not resolve ' + + `(${JSON.stringify(url)}). A token names one declared record field, spelled ` + + '`{{record.field_name}}`, and the redirect is refused rather than followed with the ' + + 'token left in it.', + }; + } + + return { ok: true, path }; +}