Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .changeset/public-forms-redirect-authoring-door-4990.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
---
"@object-ui/console": patch
---

The Public Forms dialog now refuses an out-of-contract `submitBehavior.url` at the moment it is authored, with the contract's own prescription shown next to the field (objectui#4990).

The redirect branch validated one thing — that the field was not empty — and wrote whatever else was typed into the view metadata. objectstack#7496 rules this key **relative-only** and refuses seven families of value; this door enforced the first. An admin could type `https://example.com/thanks`, or `javascript:alert(1)`, and be told nothing by the surface that had just taught them the value was acceptable — the field was `type="url"`, whose own notion of valid is an absolute URL, under a `https://example.com/thanks` placeholder.

What changed:

- **The save is refused, with the spec's sentence.** The verdict comes from `checkSubmitRedirectUrl`, the same `@objectstack/spec` `FormViewSchema` parse the renderer already asks at submit time — now exported from `submitRedirect` and called by the door. An absolute URL, a script or data scheme, a protocol-relative `//host`, a backslash, whitespace or a control character, a malformed `{{record.field_name}}` token, a document-relative path and an empty value each get their own author-facing prescription, naming the rule and what to write instead. The rule is not restated here: a second copy in the dialog would pass every value comparison right up to the release that moved the original, so a later widening of the ruling is followed by the pin rather than by an edit.
- **`Redirect URL is required` is gone.** Empty is one of the seven families, so it routes through the contract too and the author reads a sentence that says what a destination looks like.
- **The field no longer teaches the wrong value.** It is a plain text input with a `/thanks` placeholder, and a hint stating the rule — an in-app path, `{{record.field_name}}` interpolation, and the app navigation item that is declared for a deliberately external destination.

The saved value is the one the schema accepted, read back off the parse, so the door and the renderer cannot hold different opinions about a destination. `thank-you`'s `title` and `message` stay unvalidated deliberately: the spec declares both as free-form strings, so there is no contract for a door to state about them.

The server's own metadata gate already refused these bodies (`422 invalid_metadata` on `submitBehavior.url`, from the same schema), so this closes an error path rather than a silent-save hole: the correction now arrives in the field the admin can fix instead of as a failed round-trip.
54 changes: 53 additions & 1 deletion apps/console/src/components/submitRedirect.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -58,7 +58,7 @@

import { describe, expect, it } from 'vitest';
import { FormViewSchema } from '@objectstack/spec/ui';
import { resolveSubmitRedirect } from './submitRedirect';
import { checkSubmitRedirectUrl, resolveSubmitRedirect } from './submitRedirect';

/**
* The contract's verdict on one authored value — the same minimal parse the
Expand All@@ -69,6 +69,21 @@ function specAccepts(url: string): boolean {
return FormViewSchema.safeParse({ submitBehavior: { kind: 'redirect', url } }).success;
}

/**
* The contract's own prescription for a value it refuses, read off the same
* parse. Used to pin message PROVENANCE rather than message wording: a reworded
* spec keeps these tests green, a hand-written copy in either caller does not.
*/
function specRefusalMessage(url: string): string {
const parsed = FormViewSchema.safeParse({ submitBehavior: { kind: 'redirect', url } });
if (parsed.success) throw new Error(`fixture is IN contract, not out of it: ${url}`);
const onUrl = parsed.error.issues.find(
(issue) => issue.path[0] === 'submitBehavior' && issue.path[1] === 'url',
);
if (!onUrl) throw new Error(`the schema refused ${url} on some other path`);
return onUrl.message;
}

/** Values the ruling allows: rooted, relative, optionally interpolated. */
const IN_CONTRACT = [
'/thanks',
Expand DownExpand Up@@ -227,3 +242,40 @@ describe('interpolation — the consumer’s half of the ruling', () => {
}
});
});

/**
* `checkSubmitRedirectUrl` — the shape half of the ruling, exported for the
* authoring door (objectui#4990).
*
* The console's Public Forms dialog validated one of the seven families and
* saved the rest into view metadata unexamined. It now calls this, so the
* property under test is not "the door has a rule" but "the door and the
* renderer cannot hold different opinions about a value, because there is one
* parse". That is what a second hand-written copy in the dialog would take away
* while passing every value comparison until the spec moved.
*/
describe('the authoring door asks the same question (#4990)', () => {
it.each(IN_CONTRACT)('accepts %j, handing back the value the schema accepted', (url) => {
expect(checkSubmitRedirectUrl(url)).toEqual({ ok: true, url });
});

it.each(OUT_OF_CONTRACT)('refuses %s with the spec’s own prescription', (_label, url) => {
// Direction first: the contract itself rejects this value, so the refusal
// below is the contract's and not this module's private opinion.
expect(specAccepts(url)).toBe(false);

const verdict = checkSubmitRedirectUrl(url);
expect(verdict.ok).toBe(false);
if (verdict.ok) return;
expect(verdict.refusal).toBe(specRefusalMessage(url));
});

it('never disagrees with the renderer about a value', () => {
for (const url of [...IN_CONTRACT, ...OUT_OF_CONTRACT.map(([, u]) => u)]) {
const door = checkSubmitRedirectUrl(url);
const renderer = resolveSubmitRedirect(url, { id: 'x', slug: 's', status: 'open' });
expect(door.ok).toBe(renderer.ok);
if (!door.ok && !renderer.ok) expect(door.refusal).toBe(renderer.refusal);
}
});
});
104 changes: 71 additions & 33 deletions apps/console/src/components/submitRedirect.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -25,12 +25,27 @@
* 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.
*
* ## Both layers ask the same question here (objectui#4990)
*
* "The spec enforces point 1 at the authoring door" is only true of a door that
* ASKS it. This repo ships one — the console's Public Forms dialog — and it
* enforced exactly one of the seven refusal families (empty), writing the other
* six into view metadata unexamined. So the parse below is exported as
* {@link checkSubmitRedirectUrl} and called there at save time: one parse, two
* callers, one spelling of a security rule. The author now reads the spec's
* prescription at the moment they can still fix the value, and the submitter
* never meets a destination the door let past.
*
* The door needs the verdict on the string alone — there is no record at
* authoring time — which is why the shape check and the substitution are
* separate exports rather than one function with an optional scope.
*
* ## 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
* parse per submit or save 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
Expand All@@ -42,12 +57,8 @@
* 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.
* That the parsed view is a MINIMAL one is load-bearing, not laziness — see
* {@link checkSubmitRedirectUrl}, which is where the parse lives.
*
* Neither the import nor the parse is new ground in this repo, which is worth
* knowing before weighing the cost:
Expand DownExpand Up@@ -123,6 +134,56 @@ interface SubmitRedirectRefused {

export type SubmitRedirectVerdict = SubmitRedirectAccepted | SubmitRedirectRefused;

/**
* The contract's verdict on one authored `url`, before any substitution: `url`
* is the value the schema accepted, and the refused arm is the same
* author-facing prose the renderer quotes.
*/
export type SubmitRedirectUrlVerdict = { ok: true; url: string } | SubmitRedirectRefused;

/**
* Ask the contract whether an authored `submitBehavior.url` is a value it
* accepts, and get its own prescription back when it is not.
*
* This is the shape half of the ruling — the half that needs only the string —
* so it is what an authoring door calls before writing the value, and what
* {@link resolveSubmitRedirect} calls before substituting into it.
*
* 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
* neither a redirect nor a save dialog must 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.
*/
export function checkSubmitRedirectUrl(url: string): SubmitRedirectUrlVerdict {
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
// both callers follow without a second edit. Today the two are identical —
// the key is a plain string with a refinement, deliberately, so that what is
// saved and what reaches the renderer stay the string the author wrote.
const behavior = parsed.data.submitBehavior;
return { ok: true, url: behavior?.kind === 'redirect' ? behavior.url : url };
}

/**
* The string form of a record value inside a URL.
*
Expand DownExpand Up@@ -179,33 +240,10 @@ export function resolveSubmitRedirect(
url: string,
record: Record<string, unknown>,
): 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 verdict = checkSubmitRedirectUrl(url);
if (!verdict.ok) return verdict;

const path = accepted.replace(RECORD_TOKEN_RE, (_token, field: string) =>
const path = verdict.url.replace(RECORD_TOKEN_RE, (_token, field: string) =>
encodeURIComponent(urlValue(record[field])),
);

Expand Down
Loading
Loading