From dc3d5d012adff59db0b0c006606073710edebb65 Mon Sep 17 00:00:00 2001 From: os-support-ai Date: Tue, 25 Aug 2026 14:15:09 +0000 Subject: [PATCH 1/2] fix(plugin-form): narrow navigateOnSuccess to relative-only and escape the interpolated id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The admission door (`resolveSuccessNavigate`) accepted any same-origin value, including an ABSOLUTE url the author spelled out in full, and interpolated `{id}` / `{recordId}` raw. Per the 2026-08-17 maintainer ruling this key is the pre-ruling ancestor of the `submitBehavior` family, so as a compat alias it runs under the semantics objectstack#7496 ruled there: relative paths only, with the substituted value URL-escaped when the destination is built. Relative-only and escaping are separate rules and neither implies the other: relative-only says where a destination may START, so it cannot see structure a token injects further along; the escape runs only on record data, so it cannot see an absolute the author wrote. Both are applied, in that order. With every accepted destination relative, the `window.location.assign(nav)` arm at both call sites is unreachable and is deleted — the caller now judges the destination once, at the admission door, rather than twice. The absent-seam fallback inside `useSubmitRedirectNavigation` is untouched. The relative test is spelled locally rather than importing `isAppRelativeDestination`: that predicate answers WHO TRAVELS to an accepted `thankYouPage.redirectUrl`, a key ruled the OPPOSITE way on this very shape, and the two must stay free to diverge. A test pins them agreeing so a divergence is loud rather than silent. --- .../5034-navigateonsuccess-url-contract.md | 41 +++ .../src/ObjectForm.submitRedirect.test.tsx | 8 +- packages/plugin-form/src/ObjectForm.tsx | 58 ++-- .../src/WizardForm.submitRedirect.test.tsx | 4 +- packages/plugin-form/src/WizardForm.tsx | 23 +- .../src/navigateOnSuccess.mountSeam.test.tsx | 146 +++++---- .../navigateOnSuccess.urlContract.test.tsx | 286 ++++++++++++++++++ .../plugin-form/src/submitRedirect.test.ts | 9 +- packages/plugin-form/src/successBehavior.ts | 127 ++++++-- packages/types/src/objectql.ts | 14 +- packages/types/src/zod/objectql.zod.ts | 2 +- 11 files changed, 569 insertions(+), 149 deletions(-) create mode 100644 .changeset/5034-navigateonsuccess-url-contract.md create mode 100644 packages/plugin-form/src/navigateOnSuccess.urlContract.test.tsx diff --git a/.changeset/5034-navigateonsuccess-url-contract.md b/.changeset/5034-navigateonsuccess-url-contract.md new file mode 100644 index 0000000000..567b1f9cf8 --- /dev/null +++ b/.changeset/5034-navigateonsuccess-url-contract.md @@ -0,0 +1,41 @@ +--- +"@object-ui/plugin-form": minor +"@object-ui/types": minor +--- + +`navigateOnSuccess` is relative-only, escapes the interpolated id, and is deprecated in favour of `submitBehavior` + +The url contract for this key was undeclared: it was same-origin-guarded (so a same-origin +ABSOLUTE value was accepted), it interpolated `{id}` / `{recordId}` without escaping the +substituted value, and nothing said which of those was intended. The maintainer ruled it on +2026-08-17: `navigateOnSuccess` is the pre-ruling ancestor of the `submitBehavior` family +rather than a second dialect, so as a compat alias it runs under the semantics +objectstack#7496 ruled for that family. + +**Relative paths only.** A same-origin absolute such as `https://own-host/record/{id}` is +now refused like any other out-of-contract value, rather than accepted and navigated at +browser level. The destination is authored metadata, which is exactly where an address +somebody else chose gets copied in. Cross-origin and protocol-relative values were already +refused and still are; every relative shape that worked before still works. + +**The interpolated id is URL-escaped.** `/r/{id}` with an id of `a/b c` resolved to +`/r/a/b c`, silently growing a path segment, and a template of `{id}` let the id become the +whole destination. The substituted value now goes through `encodeURIComponent`, so a token +is a value in the path and never a way to add path structure. The template is the author's +and is untouched — only the id, which is data read off the written record, is escaped. + +Both halves are needed and neither implies the other: relative-only is a rule about where a +destination starts, so it cannot see structure injected further along; escaping runs only on +the substituted value, so it cannot see an absolute the author wrote out. + +This can only narrow what is reachable. Every destination the key now accepts is a relative +reference, and a relative reference cannot carry an authority, so it was already accepted by +the same-origin guard this replaces — no value that was refused is now followed. With every +accepted destination relative, the browser-level `window.location.assign` fallback at both +call sites became unreachable and was removed; an accepted destination goes to the injected +navigation seam, and the absent-seam fallback inside the shared hook is unchanged. + +**Deprecation.** `navigateOnSuccess` is marked `@deprecated` in favour of `submitBehavior`, +which already takes precedence over it and carries the richer `{{record.field_name}}` +interpolation. The `{id}` / `{recordId}` dialect keeps working for forms that already +declare it — the ruling converges the documentation and the semantics, not the spelling. diff --git a/packages/plugin-form/src/ObjectForm.submitRedirect.test.tsx b/packages/plugin-form/src/ObjectForm.submitRedirect.test.tsx index 57aec8d61e..8138dd324a 100644 --- a/packages/plugin-form/src/ObjectForm.submitRedirect.test.tsx +++ b/packages/plugin-form/src/ObjectForm.submitRedirect.test.tsx @@ -17,8 +17,8 @@ * and it had a real cost: a still-filled form invites a second submit, which * writes a second record. * - **defect 3** — a SAME-ORIGIN ABSOLUTE url is refused. The old guard - * (`isSameOriginUrl`) said yes to it, so this consumer accepted a spelling the - * authoring door refuses. + * (`isSameOriginUrl`, since deleted by objectui#5034) said yes to it, so + * this consumer accepted a spelling the authoring door refuses. * - **defect 5** — `{{record.field_name}}` is substituted from the record the * submit just wrote, URL-escaped. * @@ -32,7 +32,9 @@ * * ## Reverse verification — predicted first, then measured (counts are measured) * - * 1. **Restoring `isSameOriginUrl(behavior.url)` around the old assign** — i.e. + * 1. **Restoring the old same-origin guard around the old assign** + * (`isSameOriginUrl(behavior.url)`; the helper was deleted by objectui#5034, + * so the mutation is now spelled inline) — i.e. * putting the whole pre-ruling consumption back: **6 of the 9 tests here go * RED, 3 stay green** (12 red across both component files; WizardForm's is * 6 of 7). The three survivors are `navigates to a ruled relative path` and diff --git a/packages/plugin-form/src/ObjectForm.tsx b/packages/plugin-form/src/ObjectForm.tsx index 0b4c170f32..379c0a535c 100644 --- a/packages/plugin-form/src/ObjectForm.tsx +++ b/packages/plugin-form/src/ObjectForm.tsx @@ -25,7 +25,6 @@ import { useSubmitRedirectNavigation, type PendingSubmitRedirect, } from './submitRedirectNavigation'; -import { isAppRelativeDestination } from './thankYouRedirectNavigation'; import { usePermissions } from '@object-ui/permissions'; import { TabbedForm } from './TabbedForm'; import { WizardForm, NAVIGATE_ON_SUCCESS_REFUSED_NOTE } from './WizardForm'; @@ -917,9 +916,8 @@ const SimpleObjectForm: React.FC = ({ } else if (!schema.submitHandler) { const nav = resolveSuccessNavigate(schema.navigateOnSuccess, result); if (nav) { - // WHO travels to an ACCEPTED `navigateOnSuccess` destination — - // objectui#5034 point 1, the same mount-blindness class as - // objectui#4989 defect 4 and objectui#5112. + // An ACCEPTED `navigateOnSuccess` destination goes to the host — + // objectui#5034, points 1 and 3. // // A rooted path such as `/apps/x/o/record/r1` handed to // `window.location.assign` resolves against the ORIGIN root, so under a @@ -928,37 +926,31 @@ const SimpleObjectForm: React.FC = ({ // an authored in-app destination left the application. Only the host // knows its mount, and the seam that landed with PR #5111 is already // wired into this component — the state below and the effect that owns - // it are 440 lines up. This arm was the one call site still bypassing - // it. `delayMs: 0` reuses that one mechanism rather than minting a - // second: this key declares no delay, and an unset delay was already a - // zero timer, i.e. "go now". Reuse also hands this arm the property - // objectui#5033 bought for the other one — unmounting cancels the wait, - // so a navigation cannot fire into a form the submitter has left. + // it are 440 lines up. `delayMs: 0` reuses that one mechanism rather + // than minting a second: this key declares no delay, and an unset delay + // was already a zero timer, i.e. "go now". Reuse also hands this arm the + // property objectui#5033 bought for the other one — unmounting cancels + // the wait, so a navigation cannot fire into a form the submitter has + // left. // - // WHICH destinations are accepted is deliberately UNTOUCHED here: - // `resolveSuccessNavigate` is the authority and objectui#5548 is open on - // its contract (same-origin absolutes, the single-brace `{id}` dialect, - // the unescaped interpolation). This edit changes only who travels. + // Handed over UNCONDITIONALLY, which is point 3's consequence rather + // than a relaxation. `resolveSuccessNavigate` now admits relative + // references only (maintainer ruling 2026-08-17: this key runs under the + // objectstack#7496 semantics, so a same-origin ABSOLUTE is refused at + // the door like any other out-of-contract value). `HostNavigationValue` + // declares `to` to be "an already-resolved, application-relative path, + // never an absolute URL … It is the CALLER's job to have judged the + // destination" — and the caller has now judged it, once, at the + // admission door instead of twice. // - // The split is not a conservatism — it is the seam's own declared input - // contract. `HostNavigationValue.navigate` documents `to` as "an - // already-resolved, application-relative path, never an absolute URL … - // It is the CALLER's job to have judged the destination", and this key, - // unlike `submitBehavior.url`, is NOT relative-only: its same-origin - // guard admits an absolute `https://own-host/record/1` too. So the - // shared hook — written for a relative-only key, and correct to hand - // over everything it holds — must not be handed a value its contract - // says it never receives. Routing an absolute through a router would - // also rewrite the author's full address into a path the host then - // places somewhere else; an author who spelled the whole address asked - // for that address. Same judgement, same predicate, as objectui#5112 - // made on `thankYouPage.redirectUrl`, whose acceptance set has exactly - // this shape — reused rather than re-derived. - if (isAppRelativeDestination(nav)) { - setPendingRedirect({ url: nav, delayMs: 0 }); - } else { - window.location.assign(nav); - } + // The `window.location.assign(nav)` arm that used to stand here was + // deleted as unreachable, not as unwanted: nothing can reach it once + // every accepted value is relative. That is proved rather than reasoned + // — `navigateOnSuccess.urlContract.test.tsx` pins, over a corpus, + // that every value this helper accepts satisfies the predicate the arm + // branched on. The absent-seam fallback is unchanged and still + // `window.location.assign`; it lives in `useSubmitRedirectNavigation`. + setPendingRedirect({ url: nav, delayMs: 0 }); return result; } if (schema.navigateOnSuccess) { diff --git a/packages/plugin-form/src/WizardForm.submitRedirect.test.tsx b/packages/plugin-form/src/WizardForm.submitRedirect.test.tsx index 1bbc930bd0..a944c52c72 100644 --- a/packages/plugin-form/src/WizardForm.submitRedirect.test.tsx +++ b/packages/plugin-form/src/WizardForm.submitRedirect.test.tsx @@ -27,7 +27,9 @@ * See `ObjectForm.submitRedirect.test.tsx` for the full account; the numbers for * this file: * - * 1. **Restoring `isSameOriginUrl(behavior.url)` around the old assign**: **6 of + * 1. **Restoring the old same-origin guard around the old assign** + * (`isSameOriginUrl(behavior.url)`; the helper was deleted by objectui#5034, + * so the mutation is now spelled inline): **6 of * the 7 tests go RED**, the survivor being `navigates to a ruled relative * path` — behaviour the old line also had. The same-origin-absolute test fails * on the assign (the old guard answers yes and navigates — defect 3); the diff --git a/packages/plugin-form/src/WizardForm.tsx b/packages/plugin-form/src/WizardForm.tsx index 113b8f4347..a19a5e655f 100644 --- a/packages/plugin-form/src/WizardForm.tsx +++ b/packages/plugin-form/src/WizardForm.tsx @@ -31,7 +31,6 @@ import { useSubmitRedirectNavigation, type PendingSubmitRedirect, } from './submitRedirectNavigation'; -import { isAppRelativeDestination } from './thankYouRedirectNavigation'; import { useOccSave } from './occSave'; import type { FormSectionConfig } from './TabbedForm'; @@ -57,8 +56,8 @@ import type { FormSectionConfig } from './TabbedForm'; * same-origin guard refused) and returns no discriminant, so a reason in this * copy could only be re-derived by reimplementing that helper's internals at the * call site — where it would drift from the helper, and would additionally bake - * today's acceptance rule into user-visible prose while objectui#5548 is still - * open on exactly that rule. The diagnosable detail — the template the author + * an acceptance rule into user-visible prose, which objectui#5034 has since + * narrowed once already. The diagnosable detail — the template the author * actually wrote — goes to `console.warn` at each call site instead. * * Lives here rather than in `successBehavior.ts` (the natural home, but read-only @@ -673,16 +672,14 @@ export const WizardForm: React.FC = ({ if (nav) { // Landing on the saved record is the confirmation — no toast needed. // - // WHO travels is the same split ObjectForm's arm makes; see the long - // comment there (objectui#5034 point 1). An app-relative destination - // goes to the state the seam-owning effect above reads, so a mounted - // host's basename is applied instead of the origin root; anything - // else keeps this synchronous `window.location.assign`. - if (isAppRelativeDestination(nav)) { - setPendingRedirect({ url: nav, delayMs: 0 }); - } else { - window.location.assign(nav); - } + // WHO travels is what ObjectForm's arm does; see the long comment + // there (objectui#5034, points 1 and 3). The destination goes to the + // state the seam-owning effect above reads, so a mounted host's + // basename is applied instead of the origin root. Unconditionally, + // because `resolveSuccessNavigate` now accepts relative references + // only — the `window.location.assign` arm that used to stand here is + // unreachable and was deleted with the ruling that made it so. + setPendingRedirect({ url: nav, delayMs: 0 }); return result; } if (schema.navigateOnSuccess) { diff --git a/packages/plugin-form/src/navigateOnSuccess.mountSeam.test.tsx b/packages/plugin-form/src/navigateOnSuccess.mountSeam.test.tsx index 26cb6f9d13..2921f74c71 100644 --- a/packages/plugin-form/src/navigateOnSuccess.mountSeam.test.tsx +++ b/packages/plugin-form/src/navigateOnSuccess.mountSeam.test.tsx @@ -14,58 +14,62 @@ * `window.location.assign`. So the change under test is a wiring change, and * these tests are about which function ends up receiving the destination. * - * ## What is deliberately NOT touched, and why it is re-measured here + * ## WHICH destinations are accepted, and where that is pinned * - * WHICH destinations are accepted is `resolveSuccessNavigate`'s answer and - * objectui#5548 is open on exactly that contract: the same-origin guard admits - * ABSOLUTE same-origin URLs, the interpolation dialect is single-brace - * `{id}`/`{recordId}`, and the substituted value is not escaped. None of those - * are this card's to settle — answering one here would answer a formally open - * contract question on the maintainer's behalf. The last describe block below - * re-measures those verdicts, unchanged, so that a future edit to WHO travels - * cannot quietly widen or narrow WHAT is accepted. + * That is `resolveSuccessNavigate`'s answer, and it was ruled by the maintainer + * on 2026-08-17 and implemented as this card's point 3: relative-only (a + * same-origin ABSOLUTE is refused like any other out-of-contract value), the + * interpolated id URL-escaped, and the single-brace `{id}` / `{recordId}` + * dialect kept for existing authors of this compat key. The contract's own suite + * is `navigateOnSuccess.urlContract.test.tsx`. The last describe block here + * restates the verdicts next to the navigation arm, so that a future edit to WHO + * travels cannot quietly widen or narrow WHAT is accepted. * - * ## The arm split, and why it is required rather than cautious + * ## Why there is no longer an arm split * - * `submitBehavior.url` is relative-only (objectstack#7496), so the shared hook - * is correct to hand the host everything it ever holds. This key is not - * relative-only — its same-origin guard accepts `https://own-host/record/1` - * too — and `HostNavigationValue.navigate` declares `to` to be "an - * already-resolved, application-relative path, never an absolute URL … It is - * the CALLER's job to have judged the destination". So this call site judges: - * an app-relative destination goes through the seam, anything else keeps the - * browser-level `window.location.assign` it has always had. That is the same - * judgement objectui#5112 made on `thankYouPage.redirectUrl`, whose acceptance - * set has exactly this shape, and its predicate (`isAppRelativeDestination`) is - * reused rather than re-derived. + * Until point 3 landed, this key was NOT relative-only — its same-origin guard + * accepted `https://own-host/record/1` — while `HostNavigationValue.navigate` + * declares `to` to be "an already-resolved, application-relative path, never an + * absolute URL … It is the CALLER's job to have judged the destination". So the + * call sites forked: app-relative through the seam, anything else through the + * browser-level `window.location.assign` they had always had. + * + * Point 3 removed the values that fork existed for. Every accepted destination + * is now a relative reference, so the browser arm became unreachable and was + * deleted; the caller's judgement happens once, at the admission door. What + * survives unchanged is the ABSENT-SEAM fallback inside + * `useSubmitRedirectNavigation` — a host with no router still gets + * `window.location.assign`, and the negative control below pins it. * * ## Reverse verification — direction PREDICTED before running, measured after * * The file holds 17 cases. Predicted counts, written before either was run: * - * Mutation A, replacing the arm split at both call sites with the pre-change + * Mutation A, replacing the seam handoff at both call sites with the pre-seam * body (a bare `window.location.assign(nav)`), leaving point 2 in place: * - RED, expected — **3**: the 2 cases (one per component) asserting a host * navigate RECEIVED an app-relative destination, plus the 1 mounted-host * placement case. The seam would never be reached. - * - GREEN, expected — **14**, and two groups of those are deliberate rather - * than incidental: the 2 absent-seam cases and the 2 same-origin-absolute - * cases describe behaviour that was already correct and is unchanged by - * this card. They are the NEGATIVE CONTROL — without them, an - * implementation that also replaced the no-provider fallback, or that - * laundered an absolute URL through the host router, would pass this file - * just as well. The 8 point-2 cases survive because the refusal note is - * independent of the navigation site: mutation A is not a change detector - * for them, and counting them as one would overstate this file. + * - GREEN, expected — **14**, and one group of those is deliberate rather + * than incidental: the 2 absent-seam cases describe behaviour that was + * already correct and is unchanged. They are the NEGATIVE CONTROL — without + * them, an implementation that also replaced the no-provider fallback would + * pass the cases above just as well. The 8 point-2 cases survive because the + * refusal note is independent of the navigation site, and the 2 same-origin + * absolute cases survive because point 3 refuses those before either + * traveller is chosen; counting either group as a detection would overstate + * this file. * * Mutation B, dropping the `{ description }` argument from both success toasts * and leaving point 1 in place: - * - RED, expected — **7**: the 3 "declared but refused" cases per component - * (6) plus the cross-component agreement case. - * - GREEN, expected — **10**: the 2 "no key declared" cases assert the - * ABSENCE of a note and are unaffected by removing it — they exist to make - * the distinction the defect is about measurable, not to detect this - * mutation — and the 7 point-1 cases plus the verdict table are untouched. + * - RED, expected — **9**: the 3 "declared but refused" cases per component + * (6), the cross-component agreement case, and the 2 same-origin-absolute + * cases, which since point 3 assert the refusal note rather than a + * browser-level navigation. + * - GREEN, expected — **8**: the 2 "no key declared" cases assert the ABSENCE + * of a note and are unaffected by removing it — they exist to make the + * distinction the defect is about measurable, not to detect this mutation — + * and the 5 remaining point-1 cases plus the verdict table are untouched. * * The measured outcome of both is recorded in the PR body. * @@ -242,27 +246,32 @@ describe('objectui#5034 point 1 — a mounted host places the destination inside }); }); -// ─── Point 1, arm 2: a same-origin ABSOLUTE destination ──────────────────── +// ─── A same-origin ABSOLUTE destination: refused at the door (point 3) ───── describe.each([ ['ObjectForm', false], ['WizardForm', true], -] as const)('objectui#5034 point 1 — %s navigateOnSuccess, same-origin absolute', (_name, wizard) => { - it('keeps browser-level navigation even when a host supplied a navigate', async () => { +] as const)('objectui#5034 point 3 — %s navigateOnSuccess, same-origin absolute', (_name, wizard) => { + it('is refused before either traveller: nobody navigates, and the toast says so', async () => { const navigate = vi.fn(); const absolute = `${window.location.origin}/apps/x/o/record/{id}`; - const resolved = `${window.location.origin}${RELATIVE_RESOLVED}`; await submitWith(absolute, makeDS(), { navigate, wizard }); - // The seam's declared input is an application-relative path, and it is the - // caller's job to have judged that. Handing over a full address would mean - // this package rewriting the author's address into a path a mounted router - // then places at a DIFFERENT one. An author who spelled the whole address - // asked for that address. - await waitFor(() => expect(assign).toHaveBeenCalledWith(resolved)); + // MOVED by the 2026-08-17 maintainer ruling. This case used to assert + // browser-level navigation to the resolved absolute — the arm split points + // 1 and 2 deliberately left alone while the acceptance set was unruled. + // Point 3 rules it: as a compat alias this key runs under the + // objectstack#7496 semantics, and a same-origin absolute is refused like any + // other out-of-contract value. So there is no destination for either + // traveller to take, and the refusal note is what the submitter gets. + await waitFor(() => expect(toastSuccess).toHaveBeenCalledTimes(1)); + expect(toastSuccess).toHaveBeenCalledWith( + 'Created', + { description: NAVIGATE_ON_SUCCESS_REFUSED_NOTE }, + ); expect(navigate).not.toHaveBeenCalled(); - expect(assign).toHaveBeenCalledTimes(1); + expect(assign).not.toHaveBeenCalled(); }); }); @@ -353,33 +362,40 @@ describe('objectui#5034 point 2 — the two forms tell the submitter the same th }); }); -// ─── The acceptance set this card does not touch (objectui#5548) ─────────── +// ─── The acceptance set, restated next to the code that consumes it ──────── -describe('objectui#5034 — `resolveSuccessNavigate` verdicts are unchanged', () => { - it('answers exactly what it answered before the arm split', () => { - // Restated next to the arm split so an edit that widens or narrows WHICH - // destinations are accepted cannot pass as an edit to WHO travels. Each line - // here is a shape objectui#5548 is open on; none is this card's to change. +describe('objectui#5034 — `resolveSuccessNavigate` verdicts', () => { + it('answers the ruled acceptance set', () => { + // Restated next to the navigation arm so an edit that widens or narrows + // WHICH destinations are accepted cannot pass as an edit to WHO travels. The + // contract's own suite — corpus properties, the escaping cases, the + // no-widening proof — is `navigateOnSuccess.urlContract.test.tsx`; this is + // the local cross-check, kept small on purpose. const origin = window.location.origin; // Relative, interpolated from `id` — the ordinary case. expect(resolveSuccessNavigate('/r/{id}', { id: 'r1' })).toBe('/r/r1'); - // The single-brace `{recordId}` dialect, and the `recordId` / `_id` fallbacks. + // The single-brace `{recordId}` dialect, and the `recordId` / `_id` + // fallbacks. Ruled to STAY for existing authors of this compat key. expect(resolveSuccessNavigate('/r/{recordId}', { recordId: 'r2' })).toBe('/r/r2'); expect(resolveSuccessNavigate('/r/{id}', { _id: 'r3' })).toBe('/r/r3'); - // A same-origin ABSOLUTE url is still ACCEPTED — it is only navigated - // differently. This is the line that would move if #5548 ruled convergence. - expect(resolveSuccessNavigate('{id}', { id: `${origin}/r` })).toBe(`${origin}/r`); - expect(resolveSuccessNavigate(`${origin}/r/{id}`, { id: 'r1' })).toBe(`${origin}/r/r1`); - // Cross-origin is refused by the same-origin guard. + // MOVED (2026-08-17 ruling, escaping clause). This used to answer the origin + // URL itself — the id becoming the whole destination. Escaped, the id can + // only ever be one opaque segment. + expect(resolveSuccessNavigate('{id}', { id: `${origin}/r` })) + .toBe(encodeURIComponent(`${origin}/r`)); + // MOVED (2026-08-17 ruling, relative-only clause). This used to be ACCEPTED + // and navigated at browser level; a same-origin absolute is now refused like + // any other out-of-contract value. + expect(resolveSuccessNavigate(`${origin}/r/{id}`, { id: 'r1' })).toBeNull(); + // Cross-origin was refused before and is refused now. expect(resolveSuccessNavigate('https://evil.example.com/r/{id}', { id: 'r1' })).toBeNull(); - // No template, and no usable id, are both refusals. + // No template, and no usable id, are both refusals. Unchanged. expect(resolveSuccessNavigate(undefined, { id: 'r1' })).toBeNull(); expect(resolveSuccessNavigate('/r/{id}', {})).toBeNull(); expect(resolveSuccessNavigate('/r/{id}', { id: '' })).toBeNull(); - // The interpolated value is still NOT escaped. Pinned as a fact rather than - // fixed: it is one of the three shapes #5548 exists to rule on, and quietly - // escaping it here would answer that question in a PR that claims not to. - expect(resolveSuccessNavigate('/r/{id}', { id: 'a/b c' })).toBe('/r/a/b c'); + // MOVED (2026-08-17 ruling, escaping clause). This used to answer + // `/r/a/b c` — an id silently growing a path segment. + expect(resolveSuccessNavigate('/r/{id}', { id: 'a/b c' })).toBe('/r/a%2Fb%20c'); }); }); diff --git a/packages/plugin-form/src/navigateOnSuccess.urlContract.test.tsx b/packages/plugin-form/src/navigateOnSuccess.urlContract.test.tsx new file mode 100644 index 0000000000..d9252305db --- /dev/null +++ b/packages/plugin-form/src/navigateOnSuccess.urlContract.test.tsx @@ -0,0 +1,286 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * objectui#5034 point 3 — the url contract for `navigateOnSuccess`. + * + * Points 1 and 2 (mount-blindness, and a refused destination reported as a plain + * success) landed in PR #5558 and are pinned by + * `navigateOnSuccess.mountSeam.test.tsx`. Those changed WHO travels and left + * WHICH destinations are accepted byte-identical, deliberately, because the + * acceptance set was ruled but not yet implemented. This file is that acceptance + * set. + * + * ## The ruling being implemented + * + * Maintainer, 2026-08-17, recorded on the card, verbatim 「同意」: + * + * "As a compat alias it runs under the os#7496 ruled semantics: relative-only + * (same-origin absolutes refused like any out-of-contract value), navigation + * through the injected seam ruled on #4989 once it lands (mount-aware), and the + * interpolated id URL-escaped." + * + * Three admissions follow, and each has its own block below: relative-only, + * escaped interpolation, and the `{id}` / `{recordId}` dialect kept for existing + * authors of this key (the compat half of the same ruling — the docs point new + * authoring at `submitBehavior`, the code keeps honouring the old spelling). + * + * ## Why the two halves are pinned separately + * + * Relative-only is a rule about where a destination STARTS; escaping is a rule + * about what a token may inject further along. Neither implies the other, and + * `submitRedirect.ts` records the same split for the ruled sibling. The block + * `neither half substitutes for the other` measures that directly: it drives one + * value through each half and shows which assertion fires. + * + * ## Two properties asserted over a corpus rather than case by case + * + * 1. **No widening** (the card only ever narrows). Every destination this helper + * accepts must still be same-origin — the exact question the guard it replaces + * asked — so the reachable set can only have shrunk. + * 2. **The deleted browser-navigation arm is unreachable.** Both call sites used + * to fork on `isAppRelativeDestination(nav)` and fall back to + * `window.location.assign`. That arm is deleted, and this is the proof it was + * dead rather than the argument that it should be: the real predicate is + * imported and asked about every accepted value. + * + * The same corpus does double duty as the drift detector for the deliberate + * SECOND spelling of the relative test (`successBehavior.ts` does not import + * `isAppRelativeDestination` — see its docblock for why the two keys must stay + * free to diverge). If the two ever disagree, `the two predicates agree` goes red + * on that day rather than silently moving this key's acceptance set. + * + * ## Reverse verification — direction predicted before running, counts measured + * + * Mutation A, restoring the pre-ruling admission (`isSameOriginUrl(url)` in place + * of the relative-only test), escaping left in place: RED on the same-origin + * absolute refusals and on the corpus properties; the escaping and compat blocks + * stay green because the escape is independent of the admission test. + * + * Mutation B, deleting the `encodeURIComponent` (interpolating the raw value), + * relative-only left in place: RED on the escaping block. Direction worth + * predicting rather than assuming: one of those cases goes red by turning + * ACCEPTED-and-escaped into REFUSED (an id carrying an address, raw-interpolated, + * stops being a relative reference), not by returning a differently-escaped + * string. That is the asymmetry the two halves buy. + * + * Measured counts for both are recorded in the PR body. + */ + +import { describe, it, expect } from 'vitest'; +import { resolveSuccessNavigate } from './successBehavior'; +import { isAppRelativeDestination } from './thankYouRedirectNavigation'; + +/** The origin the test environment serves — never spelled literally. */ +const ORIGIN = window.location.origin; + +describe('objectui#5034 point 3 — relative-only admission', () => { + it('refuses a same-origin ABSOLUTE template, like any other out-of-contract value', () => { + // The line the ruling moves. Before: accepted, and navigated at browser + // level. After: refused at the door, so the submitter gets the success toast + // carrying the refusal note and NOBODY navigates. + expect(resolveSuccessNavigate(`${ORIGIN}/r/{id}`, { id: 'r1' })).toBeNull(); + }); + + it('refuses a same-origin absolute carrying no token at all', () => { + // The template needs no interpolation to be out of contract: the shape is + // refused, not the substitution. + expect(resolveSuccessNavigate(`${ORIGIN}/r`, { id: 'r1' })).toBeNull(); + }); + + it('refuses cross-origin, protocol-relative and scheme-bearing destinations', () => { + expect(resolveSuccessNavigate('https://evil.example.com/r/{id}', { id: 'r1' })).toBeNull(); + expect(resolveSuccessNavigate('//evil.example.com/r/{id}', { id: 'r1' })).toBeNull(); + expect(resolveSuccessNavigate('javascript:alert(1)', { id: 'r1' })).toBeNull(); + }); + + it('keeps admitting every relative shape it admitted before', () => { + // Narrowing is to ABSOLUTES only. This key is a compat alias with authors on + // it, so the shapes that are relative today keep working: rooted, + // document-relative, query-only and fragment-only. + expect(resolveSuccessNavigate('/r/{id}', { id: 'r1' })).toBe('/r/r1'); + expect(resolveSuccessNavigate('r/{id}', { id: 'r1' })).toBe('r/r1'); + expect(resolveSuccessNavigate('?opened={id}', { id: 'r1' })).toBe('?opened=r1'); + expect(resolveSuccessNavigate('#{id}', { id: 'r1' })).toBe('#r1'); + }); +}); + +describe('objectui#5034 point 3 — the interpolated id is URL-escaped', () => { + it('escapes structure out of the substituted value', () => { + // `/` and the space are the two that add path structure. The template is the + // AUTHOR's and is untouched; only the id — data read off the written record — + // is escaped. + expect(resolveSuccessNavigate('/r/{id}', { id: 'a/b c' })).toBe('/r/a%2Fb%20c'); + expect(resolveSuccessNavigate('/r/{recordId}', { recordId: '../../admin' })) + .toBe('/r/..%2F..%2Fadmin'); + }); + + it('cannot let an id become the destination', () => { + // With the raw value this returned the address in the id. Escaped, it is one + // opaque segment of the path the author wrote. + expect(resolveSuccessNavigate('/r/{id}', { id: 'https://evil.example.com/steal' })) + .toBe('/r/https%3A%2F%2Fevil.example.com%2Fsteal'); + }); + + it('does not re-read replacement patterns out of record data', () => { + // `String.prototype.replace` with a STRING replacement re-interprets the + // dollar-sign patterns out of the substituted value. This helper passes a + // function, so an id spelling one of them is data, not an instruction. + expect(resolveSuccessNavigate('/r/{id}', { id: '$&' })).toBe('/r/%24%26'); + expect(resolveSuccessNavigate('/r/{id}', { id: '$1x' })).toBe('/r/%241x'); + }); + + it('escapes every occurrence, not just the first', () => { + expect(resolveSuccessNavigate('/r/{id}/c/{id}', { id: 'a b' })).toBe('/r/a%20b/c/a%20b'); + }); + + it('leaves an id that needs no escaping byte-identical', () => { + // The overwhelmingly common case — an opaque record id — must read exactly as + // it did before, or this ruling would have churned every deployed form. + expect(resolveSuccessNavigate('/r/{id}', { id: 'r1' })).toBe('/r/r1'); + expect(resolveSuccessNavigate('/r/{id}', { id: '65a1f0c3e4b09d7a12345678' })) + .toBe('/r/65a1f0c3e4b09d7a12345678'); + }); +}); + +describe('objectui#5034 point 3 — neither half substitutes for the other', () => { + it('shows which half refuses which value', () => { + // One value per half, driven through both, so the file records WHY there are + // two rules rather than asserting it in prose. + // + // Relative-only cannot see structure injected mid-path: raw-interpolated this + // would be `/r/https://evil.example.com/steal`, which still starts with `/` + // and is a perfectly good relative reference. Only the escape catches it. + expect(resolveSuccessNavigate('/r/{id}', { id: 'https://evil.example.com/steal' })) + .not.toContain('/steal'); + // The escape cannot see an authored absolute, because the template is not + // escaped — only the id is. Only relative-only catches it. + expect(resolveSuccessNavigate(`${ORIGIN}/r/{id}`, { id: 'r1' })).toBeNull(); + }); +}); + +describe('objectui#5034 — the compat half of the ruling is preserved', () => { + it('keeps the single-brace `{id}` / `{recordId}` dialect and the id fallbacks', () => { + // Ruled to STAY for existing authors of this key: the convergence the ruling + // orders is a docs/deprecation pointer at `submitBehavior`, not a silent + // removal of the spelling deployed forms already carry. + expect(resolveSuccessNavigate('/r/{id}', { id: 'r1' })).toBe('/r/r1'); + expect(resolveSuccessNavigate('/r/{recordId}', { recordId: 'r2' })).toBe('/r/r2'); + expect(resolveSuccessNavigate('/r/{id}', { _id: 'r3' })).toBe('/r/r3'); + }); + + it('keeps every refusal that was already a refusal', () => { + expect(resolveSuccessNavigate(undefined, { id: 'r1' })).toBeNull(); + expect(resolveSuccessNavigate('/r/{id}', {})).toBeNull(); + expect(resolveSuccessNavigate('/r/{id}', { id: '' })).toBeNull(); + expect(resolveSuccessNavigate('/r/{id}', { id: null })).toBeNull(); + expect(resolveSuccessNavigate('/r/{id}', undefined)).toBeNull(); + }); + + it('keeps coercing a non-string id rather than refusing it', () => { + // `String(id)` is unchanged: WHICH ids count as usable is not this card's, + // and a numeric id is the ordinary shape for a relational DataSource. + expect(resolveSuccessNavigate('/r/{id}', { id: 42 })).toBe('/r/42'); + expect(resolveSuccessNavigate('/r/{id}', { id: 0 })).toBe('/r/0'); + }); +}); + +/** + * One corpus, three properties. Deliberately mixed: authored absolutes, hostile + * ids, the shapes a URL parser treats specially, and the ordinary case. + */ +const CORPUS: Array<[template: string, id: unknown]> = [ + ['/r/{id}', 'r1'], + ['/r/{id}', 'a/b c'], + ['/r/{id}', 'https://evil.example.com/steal'], + ['/r/{id}', '//evil.example.com/steal'], + ['/r/{id}', '../../admin'], + ['/r/{id}', '?x=1'], + ['/r/{id}', '#frag'], + ['/r/{id}', ''], + ['/r/{id}', 42], + ['{id}', `${ORIGIN}/r`], + ['{id}', 'https://evil.example.com/steal'], + ['r/{id}', 'r1'], + ['?opened={id}', 'r1'], + ['#{id}', 'r1'], + [`${ORIGIN}/r/{id}`, 'r1'], + ['https://evil.example.com/r/{id}', 'r1'], + ['//evil.example.com/r/{id}', 'r1'], + ['javascript:alert({id})', 'r1'], + // Control characters 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. + ['/r/{id}\u0009tab', 'r1'], + ['\\\\evil.example.com/r/{id}', 'r1'], + [' /r/{id}', 'r1'], +]; + +/** + * The shape space, spelled without any `{id}` token so that each string is its + * own resolved destination. Rooted, document-relative, query-only, + * fragment-only, already-escaped, brace-bearing-but-not-a-token, absolute + * same-origin, cross-origin, protocol-relative, backslash-authority, + * scheme-bearing, and leading-whitespace. + */ +const TOKEN_FREE_SHAPES: string[] = [ + '/r/r1', + 'r/r1', + '?opened=r1', + '#r1', + '/r/a%2Fb%20c', + '/r/{unknown}', + `${ORIGIN}/r`, + 'https://evil.example.com/r', + '//evil.example.com/r', + '\\\\evil.example.com/r', + 'javascript:alert(1)', + ' /r/r1', +]; + +describe('objectui#5034 point 3 — properties over a corpus', () => { + it('NO WIDENING: every accepted destination is one the old guard also accepted', () => { + // The guard this ruling replaced asked "does it resolve to this origin?". + // Relative-only is strictly stronger — a relative reference cannot carry an + // authority (RFC 3986) — so this must hold for every accepted value, and it + // is the machine-checkable form of "this card only ever narrows". + let accepted = 0; + for (const [template, id] of CORPUS) { + const result = resolveSuccessNavigate(template, { id }); + if (result === null) continue; + accepted += 1; + expect(new URL(result, window.location.href).origin).toBe(ORIGIN); + } + // A corpus that accepted nothing would satisfy the loop vacuously. + expect(accepted).toBeGreaterThan(0); + }); + + it('THE DELETED ARM IS UNREACHABLE: every accepted destination is app-relative', () => { + // Both call sites used to fork on this exact predicate and fall back to + // `window.location.assign` for anything it refused. The fallback is gone; this + // is the measurement that nothing could have reached it, asked of the real + // function rather than of a restatement of it. + for (const [template, id] of CORPUS) { + const result = resolveSuccessNavigate(template, { id }); + if (result === null) continue; + expect(isAppRelativeDestination(result)).toBe(true); + } + }); + + it('the two predicates agree — the drift detector for the second spelling', () => { + // `successBehavior.ts` spells the relative test itself instead of importing + // this one, because the two keys are ruled in OPPOSITE directions on the + // same-origin-absolute shape (objectui#5112 keeps browser navigation there; + // this card refuses it here) and must stay free to diverge. Two spellings + // that agree today drift silently unless something watches, so this watches. + // + // Its own corpus, TOKEN-FREE on purpose: with no `{id}` in the template the + // resolved url IS the template, which is what makes one function's admission + // comparable to the other function's verdict on the same string. The empty + // template is absent because it is refused one guard earlier, by + // `if (!template)`, and would compare unequal for a reason that is not drift. + for (const shape of TOKEN_FREE_SHAPES) { + expect(resolveSuccessNavigate(shape, { id: 'r1' }) !== null) + .toBe(isAppRelativeDestination(shape)); + } + }); +}); diff --git a/packages/plugin-form/src/submitRedirect.test.ts b/packages/plugin-form/src/submitRedirect.test.ts index 1a6009e3bf..70ed1eb743 100644 --- a/packages/plugin-form/src/submitRedirect.test.ts +++ b/packages/plugin-form/src/submitRedirect.test.ts @@ -50,8 +50,10 @@ * ok/refuse BOOLEAN for every value in this table and still fail these, because * what they check is that the prose came from the schema. * - * Deleting the CALL SITES instead (restoring `isSameOriginUrl` in the two - * components) leaves this entire file GREEN — measured, not assumed: the module + * Deleting the CALL SITES instead (restoring the old same-origin guard in the + * two components — `isSameOriginUrl`, deleted from `successBehavior.ts` by + * objectui#5034 and spelled `new URL(u, location.href).origin === + * location.origin`) leaves this entire file GREEN — measured, not assumed: the module * would simply be correct and unused. That is why the consumption is pinned in the * two rendered-component files and not here. * @@ -88,7 +90,8 @@ const IN_CONTRACT = [ * Values the ruling refuses, one per family the spec's check defends. Named by * what each one would have done had it been followed. * - * The first entry is objectui#4989's defect 3 in one line: `isSameOriginUrl` + * The first entry is objectui#4989's defect 3 in one line: the same-origin + * guard of the day (`isSameOriginUrl`, since deleted by objectui#5034) * answered TRUE for it, so this consumer followed a spelling the authoring door * refuses. */ diff --git a/packages/plugin-form/src/successBehavior.ts b/packages/plugin-form/src/successBehavior.ts index b472b8b4b5..72305c9053 100644 --- a/packages/plugin-form/src/successBehavior.ts +++ b/packages/plugin-form/src/successBehavior.ts @@ -9,45 +9,111 @@ * for another entry (`resetOnSuccess`). * * Kept dependency-free on purpose: importing the redirect guard from - * EmbeddableForm would create a cycle (EmbeddableForm → ObjectForm → WizardForm - * → here), so the same-origin check is inlined. + * EmbeddableForm would create a cycle (EmbeddableForm -> ObjectForm -> + * WizardForm -> here), so the destination test below is local to this module. */ export type { SubmitBehavior } from '@object-ui/types'; /** - * Same-origin guard for declarative navigation (relative or absolute URLs). - * - * **Scope note (objectui#4989).** This guard used to gate `submitBehavior: - * { kind: 'redirect' }` too. It no longer does: objectstack#7496 ruled that key - * RELATIVE-ONLY, and `submitRedirect.ts` gets that verdict from the spec's own - * `FormViewSchema`. Relative-only is strictly stronger than same-origin — a - * rooted relative path is same-origin by construction — so keeping this call in - * that arm as defence-in-depth would add no defence, while its converse (it says - * yes to `https://own-host/thanks`) was exactly the defect: this consumer - * accepted a spelling the authoring door now refuses, which is how a rejected - * spelling stays alive in a corpus. - * - * It survives because {@link resolveSuccessNavigate} still needs it. That serves - * `navigateOnSuccess` — a DIFFERENT declared key with its own `{id}` / - * `{recordId}` dialect, no relative-only ruling, and its own contract question - * still open. Deleting the guard would silently widen that key. + * Two bases that share nothing but their scheme. A relative reference takes its + * authority from whichever base it is resolved against; anything carrying its + * own scheme or its own authority ignores the base and answers the same origin + * for both. `.invalid` is reserved by RFC 2606, so neither can collide with a + * real deployment origin, and comparing against BOTH means an author who + * literally writes one of these sentinels is still classified as absolute. */ -export function isSameOriginUrl(rawUrl: string): boolean { +const PROBE_BASE_A = 'https://a.invalid/'; +const PROBE_BASE_B = 'https://b.invalid/'; + +/** + * Is `rawUrl` a relative reference — a destination that cannot carry its own + * authority, and therefore always resolves inside the application? + * + * True for `/r/1`, `r/1`, `?ok=1`, `#done`. False for `https://x/y`, `//x/y` + * (protocol-relative carries an authority), `javascript:…` — and, since the + * 2026-08-17 ruling this module implements, false for a SAME-ORIGIN absolute + * URL too. + * + * Answered by the URL parser rather than by a hand-written scheme grammar, so + * leading whitespace and C0 controls are handled exactly the way the parser + * handles them. + * + * ## Why this is a second spelling rather than a shared symbol + * + * `thankYouRedirectNavigation.ts` holds a predicate that today asks the same + * question of a string (`isAppRelativeDestination`). It is deliberately NOT + * imported here, and the reason is not tidiness — it is that the two must stay + * free to diverge. That one answers WHO TRAVELS to an already-accepted + * `thankYouPage.redirectUrl`, a key objectui#5112 ruled the opposite way on this + * very shape: a same-origin absolute there keeps browser-level navigation + * because "an author who spelled the whole address asked for that address". This + * one answers WHICH destinations `navigateOnSuccess` accepts at all, where the + * same shape is refused as out-of-contract. Two keys, two rulings, one string + * test in common today; binding them to one symbol would mean the next ruling on + * either key silently moving the other — which is the failure this separation + * exists to prevent. + * + * The drift that separation costs is answered by measurement rather than by + * hope: `navigateOnSuccess.urlContract.test.tsx` pins the two predicates + * agreeing over a shared corpus, so a divergence is loud on the day it happens + * instead of silent. + */ +function isRelativeReference(rawUrl: string): boolean { try { - if (typeof window === 'undefined') return false; - const url = new URL(rawUrl, window.location.href); - return url.origin === window.location.origin; + return ( + new URL(rawUrl, PROBE_BASE_A).origin === 'https://a.invalid' + && new URL(rawUrl, PROBE_BASE_B).origin === 'https://b.invalid' + ); } catch { + // Unparseable against any base is not a destination. Fail closed. return false; } } /** - * Resolve a `navigateOnSuccess` template into a safe URL: interpolate - * `{id}` / `{recordId}` from the created/updated record and same-origin-guard it. - * Returns null when there's no template, no usable id, or the URL fails the - * guard — callers then fall back to a toast. + * Resolve a `navigateOnSuccess` template into the relative destination to + * navigate to, or refuse it. Returns null when there is no template, no usable + * id, or the resolved value is out of contract — callers then show the success + * toast carrying `NAVIGATE_ON_SUCCESS_REFUSED_NOTE`. + * + * ## The contract (maintainer ruling, 2026-08-17, objectui#5034) + * + * `navigateOnSuccess` is the pre-ruling ancestor of the `submitBehavior` family, + * not a second dialect, and it is **deprecated in favour of `submitBehavior`** + * (which already takes precedence over it, pinned). As a compat alias it runs + * under the semantics objectstack#7496 ruled for that family: + * + * 1. **relative paths only.** A same-origin ABSOLUTE value is refused like any + * other out-of-contract value — not routed differently, refused. The + * destination is authored metadata, which is exactly where an address + * somebody else chose gets copied in. + * 2. **the interpolated value is URL-escaped** when the destination is built. + * 3. the `{id}` / `{recordId}` dialect **stays** for existing authors of this + * key. It is the one part deliberately not converged: the ruling keeps the + * spelling working and points new authoring at `submitBehavior` instead. + * + * ## Why both halves are needed, and neither substitutes for the other + * + * The escape is what stops a token from adding path STRUCTURE. Interpolated raw, + * `{id}` with an address in it becomes the whole destination, and `/r/{id}` with + * `a/b` in it silently grows a path segment. Relative-only is a rule about where + * a destination STARTS, so it has nothing to say about structure injected + * further along — the argument `submitRedirect.ts` records for the ruled sibling, + * which holds here verbatim. + * + * Conversely the escape alone would not refuse an authored absolute, because the + * template is the AUTHOR's and only the substituted value passes through + * `encodeURIComponent`. So: escape the data, judge the result. + * + * ## This can only narrow what is reachable + * + * Every destination this returns is a relative reference, and every relative + * reference was already accepted by the same-origin guard it replaces (a + * relative reference cannot carry an authority, RFC 3986, so it always resolves + * to the current origin). Escaping only ever maps an id onto one opaque segment. + * So the set of addresses reachable through this key is a strict subset of the + * set reachable before — there is no value that was refused and is now followed. */ export function resolveSuccessNavigate( template: string | undefined, @@ -56,6 +122,11 @@ export function resolveSuccessNavigate( if (!template) return null; const id = record?.id ?? record?.recordId ?? record?._id; if (id == null || id === '') return null; - const url = template.replace(/\{(?:id|recordId)\}/g, String(id)); - return isSameOriginUrl(url) ? url : null; + // Function replacer, not a string one: a string replacement re-reads `$&`, + // `` $` `` and `$1` out of the substituted value. `encodeURIComponent` escapes + // `$` today so the two forms agree, but that agreement would be a property of + // the escape table rather than of this line, and this line is the one that + // must not re-interpret record data. + const url = template.replace(/\{(?:id|recordId)\}/g, () => encodeURIComponent(String(id))); + return isRelativeReference(url) ? url : null; } diff --git a/packages/types/src/objectql.ts b/packages/types/src/objectql.ts index 84cf73ca10..bd7397df9d 100644 --- a/packages/types/src/objectql.ts +++ b/packages/types/src/objectql.ts @@ -1200,8 +1200,18 @@ export interface ObjectFormSchema extends BaseSchema { /** * Navigate here after a successful create/update (declarative; falls back to - * a toast). Supports `{id}`/`{recordId}` interpolation from the saved record; - * same-origin-guarded. Takes precedence over `successMessage`. + * a toast). Takes precedence over `successMessage`. + * + * The value is a RELATIVE path only — an absolute URL is refused even when it + * is same-origin — and it supports `{id}`/`{recordId}` interpolation from the + * saved record, URL-escaped when the destination is built. A refused + * destination is reported on the success toast rather than silently dropped. + * + * @deprecated Write `submitBehavior` instead — it is the one ruled shape for + * post-submit behaviour, it already takes precedence over this key, and it + * carries the richer `{{record.field_name}}` interpolation. This key keeps + * working for forms that already declare it (maintainer ruling, 2026-08-17, + * objectui#5034). */ navigateOnSuccess?: string; diff --git a/packages/types/src/zod/objectql.zod.ts b/packages/types/src/zod/objectql.zod.ts index 8126944827..e8b9441dec 100644 --- a/packages/types/src/zod/objectql.zod.ts +++ b/packages/types/src/zod/objectql.zod.ts @@ -162,7 +162,7 @@ export const ObjectFormSchema = BaseSchema.extend({ showSubmit: z.boolean().optional().describe('Show submit button'), submitText: z.string().optional().describe('Submit button text'), successMessage: z.string().optional().describe('Success toast text after create/update when no onSuccess handler is given'), - navigateOnSuccess: z.string().optional().describe('Navigate here after success ({id}/{recordId} interpolated, same-origin-guarded); precedes the toast'), + navigateOnSuccess: z.string().optional().describe('DEPRECATED, write submitBehavior instead: navigate here after success (relative path only; {id}/{recordId} interpolated and URL-escaped); precedes the toast'), resetOnSuccess: z.boolean().optional().describe('Reset the form after a successful create for another entry'), submitBehavior: z.union([ z.object({ kind: z.literal('thank-you'), title: z.string().optional(), message: z.string().optional() }), From b96b2ba4a66b789dc9b4db1796247d66101fd1d9 Mon Sep 17 00:00:00 2001 From: os-support-ai Date: Tue, 25 Aug 2026 14:21:59 +0000 Subject: [PATCH 2/2] test(plugin-form): record the measured reverse-verification outcomes Both mount-seam predictions were exact (3 red and 9 red). The url-contract file's prediction about mutation B was right about the phenomenon and wrong about its address: the accepted-turns-refused case lands in the mount-seam verdict table, not in the escaping block, where all five failures are value mismatches. Corrected to what was measured rather than left as written. Also records why the corpus properties split under mutation A: `THE DELETED ARM IS UNREACHABLE` goes red (so the dead-branch proof is a real change detector, not a tautology) while `NO WIDENING` correctly stays green, because it asks about origin and the pre-ruling admission was itself a same-origin test. --- .../src/navigateOnSuccess.mountSeam.test.tsx | 4 +- .../navigateOnSuccess.urlContract.test.tsx | 38 ++++++++++++++----- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/packages/plugin-form/src/navigateOnSuccess.mountSeam.test.tsx b/packages/plugin-form/src/navigateOnSuccess.mountSeam.test.tsx index 2921f74c71..a3fe091966 100644 --- a/packages/plugin-form/src/navigateOnSuccess.mountSeam.test.tsx +++ b/packages/plugin-form/src/navigateOnSuccess.mountSeam.test.tsx @@ -71,7 +71,9 @@ * distinction the defect is about measurable, not to detect this mutation — * and the 5 remaining point-1 cases plus the verdict table are untouched. * - * The measured outcome of both is recorded in the PR body. + * MEASURED, both exactly as predicted: mutation A turns 3 red (the 2 host-navigate + * cases and the mounted-host placement case), mutation B turns 9 red. Full + * output in the PR body. * * ## One property asserted by construction rather than by a case here * diff --git a/packages/plugin-form/src/navigateOnSuccess.urlContract.test.tsx b/packages/plugin-form/src/navigateOnSuccess.urlContract.test.tsx index d9252305db..1ef6dd203a 100644 --- a/packages/plugin-form/src/navigateOnSuccess.urlContract.test.tsx +++ b/packages/plugin-form/src/navigateOnSuccess.urlContract.test.tsx @@ -52,19 +52,37 @@ * * ## Reverse verification — direction predicted before running, counts measured * - * Mutation A, restoring the pre-ruling admission (`isSameOriginUrl(url)` in place - * of the relative-only test), escaping left in place: RED on the same-origin - * absolute refusals and on the corpus properties; the escaping and compat blocks - * stay green because the escape is independent of the admission test. + * Mutation A, restoring the pre-ruling admission (the same-origin guard in place + * of the relative-only test), escaping left in place. PREDICTED: red on the + * same-origin absolute refusals and on the corpus properties, with the escaping + * and compat blocks green because the escape is independent of the admission + * test. MEASURED: **5 red here** — the 2 relative-only refusals, `neither half + * substitutes`, and 2 of the 3 corpus properties. + * + * The corpus half is the one worth reading. `THE DELETED ARM IS UNREACHABLE` + * goes RED, which is what makes it a proof rather than a tautology: widen the + * admission door and the corpus immediately produces an accepted destination + * that is not app-relative — i.e. one that would have reached the browser + * navigation arm this card deleted. `NO WIDENING` stays GREEN, correctly and + * not incidentally: it asks about ORIGIN, and the pre-ruling admission was a + * same-origin test, so it has nothing to detect here. It is a change detector + * for a future widening past same-origin, not for this mutation. * * Mutation B, deleting the `encodeURIComponent` (interpolating the raw value), - * relative-only left in place: RED on the escaping block. Direction worth - * predicting rather than assuming: one of those cases goes red by turning - * ACCEPTED-and-escaped into REFUSED (an id carrying an address, raw-interpolated, - * stops being a relative reference), not by returning a differently-escaped - * string. That is the asymmetry the two halves buy. + * relative-only left in place. PREDICTED: red on the escaping block, with one + * case going red by turning ACCEPTED-and-escaped into REFUSED rather than by + * returning a differently-escaped string. MEASURED: **5 red here**, and the + * prediction was right about the phenomenon and wrong about its address — all + * five failures here are value mismatches (`/r/a/b c` for `/r/a%2Fb%20c`, and + * the address surviving intact in `neither half substitutes`). The + * accepted-turns-refused case exists but lands in the verdict table of + * `navigateOnSuccess.mountSeam.test.tsx`, where a template of `{id}` with an + * origin in the id raw-interpolates into an absolute URL and is then refused by + * the still-present relative-only test: `expected null to be + * 'http%3A%2F%2Flocalhost%3A3000%2Fr'`. The asymmetry is real; this file is + * simply not where it shows. * - * Measured counts for both are recorded in the PR body. + * Full output for both is recorded in the PR body. */ import { describe, it, expect } from 'vitest';