Uh oh!
There was an error while loading. Please reload this page.
fix(core,console): an id never travels without its object on the form route (#4292) - #4311
Merged
Merged
Conversation
… route (#4292) `ActionRunner.executeForm` forwarded `/forms/:name?recordId=<id>` for any context record, and that URL names no object — so the form route resolves the id against the FormView's own target object. When the action fired from a record of a DIFFERENT object and ids collide across objects (per-table integer keys), `GET /data/showcase_task/42` answers for the user who was on Account 42. The server cannot flag it: it echoes the path's object back, so the response is self-consistent. Since #4278 gave the route a write side, that stopped being a duplicate INSERT and became a targeted UPDATE of the wrong object's record. Producer (`packages/core/src/actions/ActionRunner.ts`): forward the id only when the firing context record's object matches the target view's object, and send the object with it as `?recordObject=`. Object identity is read from `context.objectName` — the key `ActionProvider` documents, every console surface seeds via `useConsoleActionRuntime`, and `recordFormNavigation` already resolves — not from a new source. On a mismatch neither param is forwarded, so a create-intent view opened from another object's record still creates, which is what that shape did before #4278. Consumer (`apps/console/src/components/FormPage.tsx`): refuse — before any `/data/` request, so no read and no write — when `recordObject` disagrees with the FormView's object. Absent or blank asserts nothing and behaves exactly as before, leaving hand-authored deep links working. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 11, 2026 13:00
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#4292. Builds directly on #4278 (PR #4293), which is the reason this became worth closing now.
The defect
ActionRunner.executeFormforwarded/forms/:name?recordId=for any context record, and that URL carries no object. The form route therefore has to resolve the id against the only object it knows — the FormView's own target — which is right whenever the action fired from a record of that object, and which nothing checked. An action whoselocationsput it on an Account but whosetargetisshowcase_task.editis publishable metadata today.Where primary keys are per-table integers,
GET /api/v1/data/showcase_task/42answers happily for the user who was looking at Account 42. The server cannot flag it either —getDataechoes the path's object back, so the response is perfectly self-consistent, which is exactly why #4278's payload-mismatch guard cannot see this shape.Pre-#4278 the blast radius stopped at a duplicate INSERT. Now that the route honours the param, the same mis-scoped action performs a targeted UPDATE of a different object's record, with no error anywhere and the record the user was actually on left untouched.
The ruling this implements
Per the PM ruling on the card (quoted verbatim):
Both halves land here, in one PR.
Producer —
packages/core/src/actions/ActionRunner.ts(executeForm)The id is forwarded only when the firing context record's object matches the target view's object; on a mismatch neither param is forwarded, so the bare
/forms/:namepreserves create semantics — a "log time" action fired from a Task at another object's create view is a CREATE launched from a record, which is what that shape did before #4278 and is not a broken edit. When the id IS forwarded, the object rides with it as?recordObject=.Object identity is measured, not invented. The source is
context.objectName, which is already this repo's one spelling of that fact:@object-ui/react'sActionProviderdocuments the flat trio (record,user,objectName) and mirrors it into the canonicalctx.*scope;useConsoleActionRuntimeseedscontext: { ...(objectName ? { objectName } : {}), … }, so every console surface that fires record actions (DeclaredActionsBar,ObjectView,RecordDetailView) already publishes it — andRecordDetailViewpasses it explicitly a second time;recordFormNavigationresolver already reads that same key as its last precedence step.action.objectNameis deliberately not consulted: it declares where an action is placed, not which record the id came from, and trusting metadata about the action to describe the record is the conflation that made this defect possible. A test pins that it cannot talk the runner into forwarding across a boundary.The target's object is asked as a prefix match against the object already in hand rather than by parsing an object name out of the view name — view identity is
object+.+key(ADR-0017;viewEnvelopebuilds exactly that,defaultListViewIdcompares exactly this way), so the metadata keeps owning that fact.Consumer —
apps/console/src/components/FormPage.tsxWhen
recordObjectis present and disagrees with the FormView's object, the route takes #4278's existing fail-closed path: the refusal fires before the object-schema fetch and before the record read, so a mismatch costs zero/data/traffic and can never reach a write. When the param is absent or blank it asserts nothing and behaviour is unchanged, so hand-authored deep links and old links keep working — the guard only tightens where identity information is actually present.recordObjectis an assertion, never an override: it cannot change which object the form edits (that comes from the view metadata alone), only make the form refuse. Reading it as a selector would let any hand-authored URL aim a form at an arbitrary object — the same hole re-opened from the other side. The registry'sformObjectis deliberately the other thing (it selects the object the record-form overlay edits); same naming shape, opposite powers, separate surfaces.Why both halves, when the producer already refuses to emit the bad URL
Defence in depth is the ruling's 两端. The producer stops emitting a cross-object id at all, so in a healthy console the consumer guard never fires. It is still not redundant: URLs arrive bookmarked, hand-written, pasted, or from a producer that predates this fix, and the console cannot tell those apart.
Tests
New:
packages/core/src/actions/__tests__/ActionRunner.formObjectIdentity.test.ts(13 cases) andapps/console/src/components/FormPage.recordObject.test.tsx(9 cases), plus #4292 cases inFormPage.test.tsincluding a behavioural round-trip pin — the realActionRunnerproduces a URL and this route's real reader consumes it, so a rename on either side fails rather than silently disarming the guard.Reverse verification per half, directions predicted before running and recorded in each file's docblock (
git checkout origin/main -- FILEafter committing, nevergit stash):9 failed | 4 passed (13), consumer DOM suite9 passed. The headline failure is the defect verbatim:expected '/forms/showcase_task.edit?recordId=42' to be '/forms/showcase_task.edit'(Account 42's id handed to a Task form)4 failed | 18 passed (22), each failure "Unable to find … /Refusing to open it/", i.e. the refusal is gone and the form loaded the other object's recordOne honest gap: with the consumer reverted,
FormPage.test.tscannot be run at all — it importsFORM_RECORD_OBJECT_PARAM, which lives in the reverted file, so it fails at collection rather than producing a verdict. Its round-trip pin was exercised in the producer-reverted direction instead, where it went red on all 3 cases (the only console-side signal that can catch a producer regression).Controls, green both sides on purpose: same-object edit (#4278's main path — load, prefill, PATCH) byte-identical;
recordIdwith norecordObjectunchanged; a host publishing noobjectNamestill gets the plain?recordId=; the public/f/:slugsurface reads neither param; #4278's payload-mismatch guard still fires on its own condition with its own distinct wording.Local verification: the affected suites plus, because
@object-ui/coresits upstream of 36 packages, the full repo suite —1254 files, 15929 passed | 1 skipped, exit 0.type-checkgreen on@object-ui/core,@object-ui/app-shelland@object-ui/console(both tsc commands each, after building the dependency closure).check:action-forward-parity,check:control-bytes,check:changeset-presenceandcheck:changeset-no-majorall green.Scope
ActionDef/actionKeysuntouched — this is behaviour inexecuteForm, not new vocabulary, andcheck:action-forward-parityis green without a whitelist edit. The only app-shell change is one prose row in theurlParamsregistry table so no page repurposes the name; no functional change there.Generated by Claude Code