Uh oh!
There was an error while loading. Please reload this page.
fix(sharing): enforce the declared publicSharing.eligibility predicate at link creation - #7974
Merged
Merged
Conversation
Evaluate the declared CEL predicate against the candidate record before the share-link insert and refuse with 422, so an ineligible record can no longer mint a token that resolveToken serves anonymously. Refs #7861
…ve siblings Drives the real ShareLinkService over a live SqlDriver (better-sqlite3 :memory:), asserting code+status on every refusal and that sys_share_link takes no row — the harm is the minted token, not the throw. Refs #7861
Adds RECORD_NOT_ELIGIBLE and ELIGIBILITY_UNEVALUABLE to the ADR-0112 ledger under @objectstack/plugin-sharing, with the generated reference docs. Refs #7861
…patch predicate check:engine-double-contract — a double looser than ObjectQL.update is how a dead route once shipped with a green suite. Refs #7861
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 106 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
…erQuery
check:query-options-erasure — the `{} as any` on the sys_share_link sweep
erased the driver-side query type. Typed local instead; ratchet back to 242.
Refs #7861…re-link-eligibility
check:test-source-alias — the eligibility suite drives a real SqlDriver, and without an alias that import resolved to driver-sql's dist, making a security pin a verdict about build state rather than the source in the checkout. Anchored array form so no subpath resolves through a file extension. The package's registry entry is unchanged. Refs #7861
…re-link-eligibility
The 11 content/docs/references/api/*.mdx artifacts are merge=os-regen paths; the driver defers them and pre-commit collects the debt. Regenerated from the merged tree (gen:schema then gen:docs) rather than text-merged, so the result is the union: main's TENANT_SCOPE_REQUIRED alongside this branch's RECORD_NOT_ELIGIBLE and ELIGIBILITY_UNEVALUABLE. Refs #7861
huangyiirene
marked this pull request as ready for review
August 12, 2026 13:40
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 12, 2026
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#7861
Route A per the triage ruling: implement the declared predicate.
packages/spec/src/data/object.zod.tsis untouched — this restores what the key already declares.The defect
publicSharing.eligibilitywas declared with a TSDoc promising the predicate is "evaluated against the candidate record when a link is created … the create call fails with 422", and nothing read it.getPolicy()built its policy from the five sibling keys and skipped this one, socreateLinkran no predicate at all. Re-verified by symbol onorigin/mainat 8d80e12 before implementing — the premise still stood.The harm is not at
createLink.resolveTokenhas no auth check and reads the record underSYSTEM_CTX, so a link that gets minted is served to a caller with no principal. Any staff user who could read a record could publish it to the open internet past a policy written specifically to prevent that, with no error raised.The fix
createLinknow evaluates the predicate against the candidate record before the insert:422 RECORD_NOT_ELIGIBLE422 ELIGIBILITY_UNEVALUABLEBoth fail closed. A restrictive policy that cannot answer must not mint a capability token — the same direction
recordStillExistsalready takes, and the same call the declarative hook-condition evaluator makes. The two codes stay distinct because the fixes differ: a false verdict is about this record, an unevaluable predicate is broken for every record until its author fixes it.When an eligibility predicate is declared, the existing visibility read widens its projection to fetch the record it judges instead of a second query being issued — and it widens only then, so an object without the key keeps the exact
id-only read it always had.The evaluator choice, measured
The TSDoc promises "the same evaluator sharing rules use". A sharing rule's CEL
conditionis lowered bycompileCelToFilter, which is the pushdown path. Measured on this branch:The two answer different questions. A sharing rule asks which records does this rule share and must become a WHERE clause the driver can run, so its vocabulary is bounded by what backends push down. Eligibility asks may THIS record, already fetched, be published — a verdict on one record in hand.
This is not the fork the triage note reserved. Both are the same evaluator in the sense the contract means:
@objectstack/formula's CEL through the one canonical front end (parseCelToAstWithReason) thatcel-to-filter.tswas moved onto in #6132, withcel-to-filter-parse-convergence.test.tspinning that both parse alike. Same dialect, same parse, different lowering — so the declared contract text needed no change, and the key's own.describe()("must evaluate to true on the target record") is honoured exactly. No spec contract text was edited.Declared fields absent from the returned row bind to
nullbefore evaluation, mirroringmaterializeDeclaredFieldsbehind the two server-side CEL gates in@objectstack/objectql. Without it, drivers that omit NULL columns would fault — and since this gate fails closed, that would refuse links for exactly the rows whose field is empty rather than judging them. An undeclared key still faults, because that one really is an author typo.Tests
New
share-link-eligibility.test.ts, 14 cases, driving the realShareLinkServiceover a liveSqlDriveron better-sqlite3:memory:with DDL through the driver's owninitObjectsfrom the realSysShareLinkdefinition.Evidence is end-to-end and about the row, not the throw: every rejection case asserts
codeandstatus(the ADR-0112 envelope) and thatsys_share_linktook no row; the headline case walks mint → resolve on both an eligible and an ineligible record. A baretoThrow()would have passed against the pre-existing 403/404 visibility refusal.The card's sibling table is pinned beside it as the control group —
allowedAudiencesandmaxExpiryDayshad no pin in this package before.Predict-then-mutate ablation
Predictions were written down before the mutation ran, then the
assertEligiblecall was deleted fromcreateLink(enforcement only; the projection widening left in place, isolating enforcement as the variable).Predicted 7 red / 7 green. Observed exactly that, on exactly the predicted cases: all seven eligibility cases flipped red; all five sibling-key pins stayed green, as did "no eligibility key behaves exactly as before" and the deliberate non-flipper (
has(record.owner_id)still mints). Restored from the commit and re-verified green.Gates run locally
@objectstack/plugin-sharingtest (468 passed / 18 files) and typecheck;@objectstack/spectest (10031 passed / 381 files);check:nul-bytes;check:error-code-casing;check:engine-double-contract(caught this PR's harness and it was pinned toassertEngineUpdateDispatch);pnpm --filter @objectstack/spec check:generated— all 13 artifacts up to date after regeneratingcontent/docs/references/for the two new ledger codes; eslint--no-inline-configon the changed files; full workspace build (71/71); and the downstream consumer sweep,--filter '...@objectstack/plugin-sharing'(packages that depend on it), typecheck with zero errors.Note on the spec edit
packages/spec/src/api/error-code-ledger.zod.tsgains the two codes under@objectstack/plugin-sharing.ApiErrorSchema.codevalidates againstStandardErrorCodeunion the ledger, so an unregistered code would itself be a declared-but-invalid surface — the same class this card is about. This is the error-code registration surface, not theobject.zod.tscontract text the ruling placed out of scope.Generated by Claude Code