Skip to content

fix(plugin-security): a required master-detail parent omitted on insert answers 400 VALIDATION_FAILED with fields[] (#8688) - #8879

Merged
qq9340100 merged 2 commits into
mainfrom
claude/issue-8688-master-detail-missing-parent-envelope
Aug 15, 2026
Merged

fix(plugin-security): a required master-detail parent omitted on insert answers 400 VALIDATION_FAILED with fields[] (#8688)#8879
qq9340100 merged 2 commits into
mainfrom
claude/issue-8688-master-detail-missing-parent-envelope

Conversation

@qq9340100

Copy link
Copy Markdown
Collaborator

Fixes#8688

Implements the maintainer ruling of 2026-08-15 01:42Z (conditional stand-down, ruled as a pair with #8772).

What changed

assertControlledByParentWrite runs in the security middleware chain, outside the executor that calls validateRecord, so on an insert it short-circuited required-field validation on the one field they share. The same omission that every other required field answers 400 VALIDATION_FAILED with fields[] came back 422 MISSING_REQUIRED_FIELD with no fields[] and a [Security] prefix — on the very field whose present-but-unresolvable case already answers the wanted envelope.

The gate's insert leg now stands down where required-field validation provably covers the omission, and the caller gets the platform's ordinary envelope instead.

The stand-down is conditional, and that is the whole design

declared master referenceinsert with the FK absent
master_detail + required: trueNEW: gate stands down → 400 VALIDATION_FAILED + fields[]
master_detail, no requiredunchanged — 422 MISSING_REQUIRED_FIELD
master_detail + required + readonlyunchanged — 422 MISSING_REQUIRED_FIELD
master_detail + required + systemunchanged — 422 MISSING_REQUIRED_FIELD
required-lookup fallback relationunchanged — 422 MISSING_REQUIRED_FIELD
master present but not writableunchanged — 403 PERMISSION_DENIED
stored row's FK is null (by-id write)unchanged — 422 MISSING_REQUIRED_FIELD

validateRecord skips provenance-flagged fields before its required check (if (def.system || def.readonly) continue;) and never fires on a field that is not required. For those shapes this gate is the only refusal there is, and an unconditional hand-over was measured to mint a detail row with a null master FK — unmatchable by the controlled_by_parent read filter, readable by nobody. The residual envelope asymmetry is confined to exactly the shapes #8772's lint refuses at publish; the changeset states that residue.

The predicate is a deliberate mirror of the validator's own insert-mode gate, resolved once alongside the relation itself so the two can never describe different fields.

The pin that moved

Exactly one, rewritten deliberately: the [#7474] six-envelope truth table's insert leg in controlled-by-parent-sharing.test.ts. Its successor asserts both sides of the condition rather than the new behaviour alone:

  • covered shape → the executor is reached (the hand-over, not merely "no throw"), and the real validateRecord refuses the same payload with VALIDATION_FAILED + fields[{ field: 'account', code: 'required' }];
  • each uncovered shape → the gate still answers 422 with its wording, and the real validator raises nothing on the same payload — which is why the gate must stay.

validateRecord is imported from @objectstack/objectql (already a devDep, already aliased to packages/objectql/src by this package's vitest config, so the verdict is about source in the checkout, not a prebuilt dist). A hand-mirrored copy of the predicate would have asserted the belief instead of the behaviour.

The truth table's other legs — the ct_orphan case and the four-envelope toEqual — are update-path and did not move, as the ruling predicted. The master-access verdict is newly pinned as surviving on the insert path.

Reverse verification

Direction predicted in writing before each run.

RV1 — the successor against the old behaviour. Restored the unconditional throw, keeping the new tests. Predicted: red, exactly 1 failure, the covered-shape case; everything pinning surviving behaviour stays green. Measured: Test Files 1 failed | 4 passed (5), Tests 1 failed | 107 passed (108)

AssertionError: promise rejected "MasterReferenceMissingError: [Security] M… " instead of resolving
❯ src/controlled-by-parent-sharing.test.ts:859 await expect(h.insertContact(payload)).resolves.toBe(true);

RV2 — ablate the condition. Made the stand-down unconditional (if (operation === 'insert') return;) — the route both prior runs measured fail-open. Predicted: red, exactly 4 failures, the three unsafe shapes plus the required-lookup case. Measured: Tests 4 failed | 104 passed (108), all four expected the write to be refused, but it resolved. This is what proves the new cases can fail.

Both restores came out of the commit (git checkout HEAD -- …), never the shared stash; git hash-object afterwards matched the committed blob exactly.

Verification

All heavy runs serialized under flock /tmp/os-heavy-verify.lock. Gate union re-derived from the actual changed paths with scripts/pm/dispatch-gates.mjs and run at final HEAD = b115bfede, tree clean.

  • pnpm --filter @objectstack/plugin-security testTest Files 65 passed (65), Tests 1235 passed (1235); typecheck clean.
  • dogfood consumer (resolves from built dist/, and the dist was verified to carry the change) → Test Files 2 passed (2), Tests 9 passed (9).
  • 16 gate families green: nul-bytes, cross-package-test-inputs (both invocations), test-source-alias, type-source-resolution, i18n, changeset-gate-self-tests, objectui-changeset, query-options-erasure, error-code-casing, engine-double-contract, type-check-coverage, type-check-debt, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset.

Two of those were red first and are worth naming: check:i18n refused to run on an unbuilt workspace (PREREQUISITE NOT MET — not measured, not passed) until the closure was built, and check-adr-0087-registration requires a declared-breaking changeset to answer the ledger question in writing. Both green at the final head.

Changeset

minor + BREAKING annotation, per the ruling's post-cut convention. It states the residue, records that one pin was rewritten deliberately, and carries the ADR-0087 disposition not-required (no-migration-prescription) — nothing authorable moves.

#8772 is not addressed here (its lint change is a separate card), and #8865 remains open — this change neither helps nor hinders that leg.


Generated by Claude Code

…rt answers the validation envelope (#8688)
`assertControlledByParentWrite` runs outside the executor that calls
`validateRecord`, so on an insert it short-circuited required-field validation
on the master FK: the same omission every other required field answers `400
VALIDATION_FAILED` with `fields[]` came back `422 MISSING_REQUIRED_FIELD` with
no `fields[]` and a `[Security]` prefix.
Per the maintainer ruling of 2026-08-15 the gate now STANDS DOWN on that leg —
conditionally. Only where `validateRecord` provably refuses the omission
(`master_detail` + `required: true`, not `readonly`/`system`) does it hand over;
for the three shapes the validator skips it keeps answering exactly as today,
because an unconditional hand-over mints a detail row with a null master FK that
the controlled_by_parent read filter can never match.
A master that is present but not writable keeps its 403, pinned as surviving.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn
…ope change
`check-adr-0087-registration` requires a declared-breaking changeset to answer
the ledger question in writing. Nothing authorable moves here — no spec
property, object or field is renamed, retired or tombstoned — so the honest
disposition is `not-required (no-migration-prescription)`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn
@vercel

vercelBot commented Aug 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 15, 2026 1:29pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security.

12 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx(via @objectstack/plugin-security)
  • content/docs/kernel/runtime-services/sharing-service.mdx(via @objectstack/plugin-security)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/plugin-security)
  • content/docs/permissions/access-recipes.mdx(via packages/plugins/plugin-security)
  • content/docs/permissions/authorization.mdx(via @objectstack/plugin-security)
  • content/docs/permissions/explain.mdx(via @objectstack/plugin-security)
  • content/docs/permissions/permissions-matrix.mdx(via packages/plugins/plugin-security)
  • content/docs/permissions/sharing-rules.mdx(via @objectstack/plugin-security)
  • content/docs/plugins/index.mdx(via @objectstack/plugin-security)
  • content/docs/plugins/packages.mdx(via @objectstack/plugin-security)
  • content/docs/ui/audience-based-interfaces.mdx(via packages/plugins/plugin-security)
  • content/docs/ui/dashboards.mdx(via @objectstack/plugin-security)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/plugin-security)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@qq9340100@claude