Skip to content

fix(security,verify): keep the security reference-alias read tolerant but LOUD, narrow verify with a named finding - #13541

Merged
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-13250-tolerant-alias-readers
Aug 30, 2026
Merged

fix(security,verify): keep the security reference-alias read tolerant but LOUD, narrow verify with a named finding#13541
zhuangjianguo merged 3 commits into
mainfrom
claude/issue-13250-tolerant-alias-readers

Conversation

@claude

@claude claude Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Fixes #13250

The last two tolerant readers of the rejected reference aliases, given
different dispositions — because their failure modes are different in kind.
Maintainer ruling, 2026-08-30, verbatim 「同意」; the ruling is recorded on the
card at comment 5468957088. The lint reader was already narrowed and merged
separately and is not touched here.

Q1 — plugin-security: the tolerance STAYS, and is now LOUD

resolveCbpRelation still reads reference ?? reference_to ?? referenceTo.
Narrowing it is the one thing the ruling forbids, and the reason is measured
rather than stylistic:

  • The alias reaches this reader. It consumes ql.getSchema(), i.e. the
    SchemaRegistry, and a raw registerObject skips Zod by design. Re-measured
    on this branch through the real registry: a raw round-trip serves the field
    back as ["name","type","required","reference_to"] — canonical absent, alias
    verbatim — with the canonical spelling surviving as a positive control.
  • A miss here is a DENIAL, not a wrong answer. resolveCbpRelation
    returning null is fail-closed: the read leg answers RLS_DENY_FILTER (zero
    rows for every non-admin caller) and the write leg throws
    MasterDetailRelationMissingError. Narrowing would take a raw-registered,
    alias-spelled controlled_by_parent object from "access derived from its
    master" to "everything denied, and writes throw" — an availability outage on
    a population that provably exists.

So the card's defect — a tolerant consumer hiding an authoring mistake — is
answered by making the tolerance loud. When the alias is what resolved the
relation, the plugin now reports it through its own SecurityReportSink, the
same warn channel and console-backed default as every other report site in
the file. No new diagnostic channel was invented.

Granularity: once per object, per cache generation. The report sits inside
resolveCbpRelation's resolution body, which runs only on a cbpRelCache
miss, so 25 reads of one object produce one report — measured. It is
deliberately not per-read: this is under the per-request RLS path, and a noisy
channel is one operators filter out, which would make the tolerance silent
again by a longer route. It re-arms when the metadata.watch('*') subscription
installed in start() clears that cache — exactly when a Studio / AI-authoring
author is listening. A hand-rolled "already warned" set would have to reproduce
that invalidation to stay correct.

refKey was introduced so the diagnostic and the resolution cannot disagree
about which key answered: ref is derived from it rather than spelling the
?? chain twice. The fall-through is pinned, including the present-but-null
canonical case.

Q2 — verify: narrowed, and the finding says WHY

relationTarget now reads the canonical reference only. The alias is
reachable here too — loadConfig() does no validation of its own ("the gate
lives in the loaded module"), so it arrives through two unparsed doors.
Re-measured on this branch with isolated controls: the same fixture parses
clean with reference, is refused by the default strict parse with
"Unrecognized key(s) on this field: reference_to", and survives verbatim
through both defineStack(cfg, { strict: false }) and a plain-object config.

Unlike Q1, this consumer's failure mode is a report line rather than a
refusal, so narrowing costs coverage and not availability. But narrowing alone
would have traded one silent seam for another, so the reason ships with it:

required lookup field "company_id" spells the rejected alias reference_to
instead of referencereference is the only relationship spelling
@objectstack/spec declares, so this app's target "company" was not derived;
rename the key

instead of degrading to the generic "has no reference target"; an optional
relation is skipped under relation-rejected-reference-alias:KEY rather than
the generic relation-missing-reference. A field carrying both a canonical
reference and a stale alias derives from the canonical key and reports
nothing.

Clause 2, answered with evidence: this widens no published surface

The named finding lands in the existing free-form strings
CrudCase.blocked (typed string) and skippedFields[].reason (typed
string). @objectstack/verify is a published package, and CrudCase is
published as a type from its index — but neither field is an enum, no value set
is enumerated anywhere, and no gate, doc or test pins the reason vocabulary
(the repo-wide sweep finds the reason literals only in derive.ts itself). The
published union that does exist, ObjectVerifyResult.status, is untouched: a
blocked case still arrives as status: 'skipped'. No new exported
identifier, no changed type, no new status.
The content limb does not trip.

Scope

  • Narrowing Q1 for real, behind a migration that sweeps stored / raw-registered
    metadata first, is out of scope by the ruling's own words and is filed
    separately rather than ridden in here.
  • A fourth tolerant reader observed in passing is filed separately too; it is
    not touched by this PR.

One extra fix, declared

packages/verify/src/derive.test.ts imported ./derive without the .js
extension. Under moduleResolution: NodeNext that specifier does not resolve,
so every symbol it named was any — and the package's TEST_DEBT ratchet
reads this layer. The new cases would have drifted the entry from 8 raw tsc
errors to 19; with the extension the whole file is clean and the entry measures
3. Lowered with the gate's own --lower and the note re-tallied to the
composition that is actually left (three harness test files with the same
missing extension), because a note describing the larger pile reads as "nearly
graduated" to the next author. That is why scripts/check-type-check-coverage.mjs
is in this diff.

Verification — at 5c467362cd

Dependency closures and then the whole workspace built first (70/70 turbo
tasks), so nothing below is a verdict about a stale dist/.

check verdict line
@objectstack/plugin-security suite Test Files 90 passed (90) / Tests 1665 passed (1665)
@objectstack/verify suite Test Files 9 passed (9) / Tests 48 passed (48)
typecheck: plugin-security, verify, cli, driver-turso, dogfood exit 0, each script name echoed
pnpm lint (whole repo, --no-inline-config) exit 0, silent — full scan, not narrowed
pnpm check:type-check-debt 29 ledger entr(ies) re-measured … none above its recorded number · surplus: none
pnpm check:doc-authoring 831 pinned site(s) … no growth, no burn-down unrecorded
node packages/lint/scripts/check-reference-carrier-shape.mjs exit 0 (package-local, outside the root check:* namespace — not derived, run anyway)
pnpm check:verify-stand-in 2 stand-in check(s) guarded … 0 asserted driver arguments
pnpm check:nul-bytes scanned 7515 text file(s) … no raw ASCII control bytes

39 derived gate families were run in total (30 derived from the first change
set, 9 more after the gate-script edit changed it), plus the convention-triggered
ones. Every exit code was captured before any pipe. Two gates reported
NOT MEASURED by their own refusal text rather than a finding, and neither is a
red: check-test-completeness (PREREQUISITE NOT MET — it grades a saved
turbo test log; CI tees one) and scripts/pm/check-half-states.mjs (needs a
real GitHub credential; the --self-test form CI runs, check:pm-half-states,
exits 0).

One gate found a real defect in the first draft and it was fixed as prescribed:
check:doc-authoring refused the tracker id inside the diagnostic's runtime
string — operators cannot resolve #NNNN — so the tag is now a resolvable name
and the anchor moved to an adjacent comment.

Ablation — three legs, each red in the predicted direction

Run at the final head, implementation committed first, every mutation proven on
disk by grepping both the injected and the deleted text, every restore proven
by a git hash-object match against the HEAD blob plus an empty whole-tree
git status --porcelain, under an EXIT INT TERM trap with an absolute repo
root. Both suites read their subject through a same-package relative specifier,
so the mutation is live in the source the run consumes without a dist/
rebuild.

ablation result
narrow the Q1 tolerant read to canonical-only vitest exit=1 — 8 failed, including reference_to resolves the relation
delete the Q1 diagnostic call vitest exit=1Tests 5 failed | 6 passed (11)
restore Q2's ?? chain vitest exit=1Tests 4 failed | 8 passed (12)

Baseline before each: 11 passed (11) and 12 passed (12).

Generated by Claude Code


Generated by Claude Code

claude added 3 commits August 30, 2026 18:39
…verify's with a named finding

Two remaining tolerant readers of the rejected `reference` aliases, given
different dispositions because their failure modes differ:

- plugin-security `resolveCbpRelation` KEEPS the tolerance (a miss there is
  fail-closed: RLS_DENY_FILTER on read, MasterDetailRelationMissingError on
  write) and now reports the alias once per object through the plugin's own
  report sink.
- verify `relationTarget` narrows to canonical-only and names the cause, so an
  alias-spelled relation reports "this app spells a rejected alias" rather than
  the generic "could not be derived".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
…the changeset

check:doc-authoring refuses an internal issue id inside a runtime string: it
reaches operators and generated surfaces, none of which can resolve #NNNN. The
tag is now a resolvable name and the ruling anchor moved to an adjacent comment.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
…hecks, and press the ratchet down

Under moduleResolution NodeNext a relative import without .js does not resolve,
so every symbol it names is any. That one bare specifier was carrying 1x TS2835
plus every TS7006 in the file — the new cases would have drifted the package's
TEST_DEBT entry from 8 to 19. With the extension the whole file is clean and the
entry measures 3, lowered via the gate's own --lower and re-tallied to the
composition that is actually left.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3jdziLbAPGeceVNmSox5L
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-security, @objectstack/verify, touching 9 documentable anchor(s).

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/permissions/field-level-security.mdx (via SecurityPlugin (symbol))
  • content/docs/permissions/index.mdx (via SecurityPlugin (symbol))
  • content/docs/plugins/packages.mdx (via SecurityPlugin (symbol))
  • content/docs/protocol/objectql/types.mdx (via reference_to (literal))
  • content/docs/ui/forms.mdx (via SecurityPlugin (symbol), referenceTo (literal))

3 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx (via SecurityPlugin (symbol))
  • content/docs/releases/v15.mdx (via reference_to (literal))
  • content/docs/releases/v17.mdx (via referenceTo (literal))

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.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 3959a394d6e028a4fd5048f96b5498af18a9ecdfpackageMentionDocs.

Which tree this was computed on

This run read content/docs from fe0e70787277860109ebda17e7370bb278284950 — the merge of head 5c467362cdd4214d278dbbc5aea32707e4f66dff into base 3959a394d6e028a4fd5048f96b5498af18a9ecdf, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin fe0e70787277860109ebda17e7370bb278284950 && git checkout fe0e70787277860109ebda17e7370bb278284950
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 3959a394d6e028a4fd5048f96b5498af18a9ecdf 5c467362cdd4214d278dbbc5aea32707e4f66dff && git checkout -B drift-repro 3959a394d6e028a4fd5048f96b5498af18a9ecdf && git merge --no-ff 5c467362cdd4214d278dbbc5aea32707e4f66dff

node scripts/docs-audit/affected-docs.mjs --json 3959a394d6e028a4fd5048f96b5498af18a9ecdf

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 3959a394d6e028a4fd5048f96b5498af18a9ecdf → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator

PM review — ACCEPT, landing held until CI converges (22 success · 2 skipped · 6 running · 0 red at time of writing). My bar is every check green. ⛔ I do not merge it.

The clause ② question is answered by MEASUREMENT, and that matters more than the answer

I flagged the content limb as open at dispatch: a named verify finding might widen a published surface. It does not — and the seat measured it rather than reasoning from the fact that it compiles:

@objectstack/verify is published (publishConfig.access: public, single . export) and CrudCase is a published type. But the finding lands in blocked?: string and skippedFields[].reason: string, both already free-form. A repo-wide sweep for the existing reason literals (relation-missing-reference, relation-target-external, unsynthesizable-optional) finds them only inside derive.ts itself — no gate, doc or test pins the vocabulary; the README documents the skipped verdict, not reason strings. The one published union that does exist, ObjectVerifyResult.status, is untouched. No new exported identifier, no changed type, no .d.ts change.

⭐ And it ran the check that settles it rather than arguing: check:verify-stand-in, which reconciles verify's exported set in both directions, exits 0 — and that gate was not in the derived family. ⇒ Internal, with evidence. I had an inference pointing the same way and deliberately did not act on it; an inference is not a measurement, and this is what the difference looks like.

⭐ The refinement in Zone 2 C is the sentence worth keeping: "the shape is a free-form string, not a typed finding union, so 'a named finding' means a distinct STRING VALUE. That is what makes the clause-2 answer come out internal; had it been an exported union, the same ruling would have tripped the content limb."

Both rulings honoured, including the one that forbids acting

Q1 = A. resolveCbpRelation still reads reference ?? reference_to ?? referenceTo — the tolerance is intact, and pinned by a test that reddens if anyone narrows it. Ablation leg 1 (narrowing to canonical-only) turns 8 tests red including "reference_to resolves the relation — narrowing it would deny access". ⇒ The one thing the ruling forbids cannot land silently.

It is loud through this.logger.warn?.() — the plugin's existing SecurityReportSink, same tag shape and console-backed default as every other report site in the file. No new channel. refKey exists so the report and the resolution cannot name different keys, and the present-but-null fall-through is pinned.

Granularity (my Zone 2 D): once per object per cache generation, inheriting cbpRelCache's own lifecycle rather than inventing a dedup. Both halves measured: 25 resolutions of one object → exactly 1 report; and it re-arms on a metadata change, pinned by driving the real metadata.watch('*') callback the plugin installs (2 → 1 report, change, 3rd → 2). ⭐ The reasoning is the good part: a process-lifetime "already warned" set "would go silent at the one moment it matters" — when a Studio or AI-authoring author is actually listening.

Q2 = C. Narrowed to canonical-only with the named finding. The blocked text names the alias, the underived target, and the remedy; the skipped reason is relation-rejected-reference-alias:<alias>. ⭐ The load-bearing pin is a negative: the blocked text must not match /has no \reference` target/`, so a later degradation back to the generic sentence reddens rather than passing quietly. A field with no target at all keeps the generic reason.

The gate-script edit is a tightening, and one gate found a real defect

scripts/check-type-check-coverage.mjs lowers @objectstack/verify 8 → 3 via the gate's own --lower. That is the ratchet doing its job: appending tests first drifted it 8 → 19 and went red, because verify's ratchet measurement lifts the tsconfig test exclusion and one relative import missing its .js under NodeNext resolved to nothing — making every symbol it named any, so it carried 1×TS2835 plus every TS7006 in the file. With the extension the file is clean, and the ceiling came down rather than banking headroom. check:type-check-debt verdict: "none above its recorded number" and "surplus: none — every entry sits exactly at its measurement, so any new error is red."

⭐ And check:doc-authoring refused the tracker id inside the diagnostic's runtime string"a runtime string reaches authors, operators and generated surfaces — none of whom can resolve #NNNN." The tag became a resolvable name and the anchor moved to an adjacent comment. ⛔ No baseline entry was added (that path is maintainer-only, and the gate says so).

Ablation was run twice — at an intermediate head and again at the final head — because either the tag rename or the import fix could have made an assertion vacuous. That is the right instinct.

My docs pointer was resolved by reading, not by pattern-match

I relayed content/docs/ui/forms.mdx (naming referenceTo) as a possible #13539-class teaching site. It is not, and the seat read it: :260 is the publicPicker.object row, naming reference first and the aliases as "a legacy referenceTo / target / options.objectName on a pre-fold stored row" — an accurate description of the still-tolerant picker reader at rest-server.ts:8934. types.mdx:736-747 is the opposite of a teaching site. It also read the three release-owned pages rather than assuming, found none made false, and edited no docs. ⇒ Nothing owed, and my pointer correctly cost one read instead of a wrong card.

Routed — ⛔ grading is triage's

#13542 (the deferred Q1 narrowing behind a stored-metadata migration — the ruling's own separate card, with the population measurement it must start from) and #13543 (a fourth tolerant reader at packages/objectql/src/master-detail.ts:58, which unlike the three on this card declares its tolerance in its own type with a stated population — asserted but never measured, so filed as an observation, not a defect claim) → both pm:retriage.

Declared and left alone, correctly

@objectstack/verify's own typecheck script does not cover its test files (tsconfig.json excludes **/*.test.ts; --listFiles counts the new suite 0 times) — so that script's green says nothing about them. The TEST_DEBT ratchet does read that layer, which is what caught the drift. ⛔ The seat did not fix the exclusion: different defect class from this card, so it fails the bounded in-place exemption. Restraint in the right direction.

⇒ On full green I flip ready and enqueue.


Generated by Claude Code

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

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants