fix(sharing): publicSharing.eligibility is declared-but-unenforced — a draft or internal record mints a public share link and is served anonymously

Found while implementing share-link publishing in the HotCRM app (objectstack-ai/hotcrm#601, PR objectstack-ai/hotcrm#1103). Not fixable from that side, and the app deliberately shipped without the feature rather than declare a key that does nothing — the measurement is below.

Same class as #3865 (full access level declared Transfer/Share/Delete and was equivalent to edit), and #3865's resolution — remove the option rather than let it mislead — is one of the two routes here.

The key

packages/spec/src/data/object.zod.ts, inside publicSharing:

eligibility: z.string().optional()
  .describe('CEL expression that must evaluate to true on the target record'),

with the TSDoc above it:

Optional CEL/JSONLogic predicate evaluated against the candidate record when a link is created. When the predicate returns false, the create call fails with 422 (e.g. "draft records cannot be shared"). Evaluator is the same one used by sharing rules.

Nothing evaluates it. ShareLinkService.getPolicy() builds its policy object from enabled / allowedAudiences / allowedPermissions / maxExpiryDays / redactFields and never reads eligibility; createLink() runs no predicate anywhere. A read across the whole installed platform at 17.0.0-rc.6 finds no consumer of the key at all — the only other hits for the word are unrelated prose in driver-sql.

The sibling keys are all genuinely enforced, which is what makes this one easy to miss in review:

key	measured on 17.0.0-rc.6
`enabled`	ENFORCED — false ⇒ every create refused, `SHARING_NOT_ENABLED`
`allowedAudiences`	ENFORCED — `signed_in` refused `422 AUDIENCE_NOT_ALLOWED`
`allowedPermissions`	ENFORCED — a non-`view` grant refused 422
`maxExpiryDays`	ENFORCED — a longer expiry refused `422 EXPIRY_TOO_LONG`
`redactFields`	APPLIED to every token-served response
`eligibility`	INERT

Measured end to end

Driving the real ShareLinkService against an object declaring exactly what the docs suggest —

publicSharing: {
  enabled: true,
  allowedAudiences: ['public', 'link_only'],
  allowedPermissions: ['view'],
  redactFields: ['owner_id'],
  eligibility: "record.status == 'published' && record.audience == 'public'",
}

createLink(published + public,   audience 'public') → OK
createLink(published + INTERNAL, audience 'public') → OK      ← should be 422
createLink(DRAFT + public,       audience 'public') → OK      ← should be 422
resolveToken( the internal link , {})               → SERVED, anonymously

The last line is the impact: GET /api/v1/share-links/:token/resolve has no auth check, reads the record under SYSTEM_CTX and returns it. So the record an author believed unshareable is served to a caller with no principal.

Severity is bounded by the fact that link creation itself requires authentication (isAuthenticated(ctx) on POST /share-links) plus read access to the record — so this is not an anonymous-to-anonymous hole. It is: any staff user who can read a record can publish it to the open internet, past a policy the object author wrote specifically to prevent that, and the policy raises no error while being ignored. That is the shape a restrictive policy must never fail in — the schema's own history note beside this block makes the point about the sibling keys:

On a policy whose whole job is to be restrictive, a silently dropped key fails OPEN.

Why an app cannot work around it

Worth recording, because the obvious mitigation looks available and is not. The chokepoint exists: createLink persists every link with one engine.insert('sys_share_link', row, { context: SYSTEM_CTX }), and a beforeInsert hook on sys_share_link was measured working against a real ObjectQL engine — it fires despite the system context (triggerHooks filters on object name and session.skipAutomations, which that context does not carry), ctx.api reads the candidate record, and a throw refuses the link. It covers the console share dialog and a raw REST create alike.

But a metadata app cannot register it. validateCrossReferences in @objectstack/spec refuses any hook whose object is not in the stack's own objects:

Hook 'share_link_article_eligibility' references object 'sys_share_link' which is not defined in objects.

collectObjectNames reads config.objects only, so there is no wildcard escape either — '*' is not in that set. So the rule is expressible, enforceable, and unreachable from where apps live.

Two routes

A. Implement it. Evaluate the predicate in createLink before the insert, refusing with 422, using the same evaluator sharing-rule conditions use. Note that compileCelToFilter (the sharing-rule path) compiles to a pushdown filter and rejects things a record-level predicate should accept — has(record.x) among them, per objectstack-ai/hotcrm's measured operator matrix — so "the same evaluator" may need to mean the record-level one rather than the filter compiler. Worth deciding explicitly, since the TSDoc already promises the sharing-rule evaluator.

B. Remove the key, per #3865 / ADR-0078 precedent, so that authors cannot write a policy that silently does nothing. If B, @objectstack/spec's strict object will then reject eligibility outright, which is the loud failure this needs — and the docs paragraph promising the 422 must go with it.

A is worth preferring here, unlike #3865: an eligibility gate is the one thing that makes publicSharing safe to enable on any object holding a mix of public and internal records, which is most of them. Without it the only safe posture is enabled: false, and that is the posture objectstack-ai/hotcrm#1103 shipped — the feature was dropped rather than approximated.

Repro environment

17.0.0-rc.6 as pinned by objectstack-ai/hotcrm, @objectstack/plugin-sharing from that pin, driven through ShareLinkService directly and through a real ObjectQL + InMemoryDriver. Happy to hand over the probe scripts if useful.

Generated by Claude Code

Triage: lands in @objectstack/plugin-sharing (ShareLinkService.createLink) → domain:identity; pm:queue via route A (implement the declared predicate), not escalated: the TSDoc beside the key already declares the full contract — predicate evaluated at link creation, false ⇒ 422, sharing-rule evaluator — so enforcing it is restoring a declared invariant (the named immediate-action class), while route B (remove the key) would be a published-capability removal and is not needed when A restores exactly what is declared. The card's own analysis prefers A for the same reason: without eligibility, the only safe posture on mixed-visibility objects is enabled: false, which is what a real app (hotcrm#1103) shipped.

target:v17 stamped: public contract declared≠enforced on a restrictive security policy that fails open — class ② (declared≠enforced public contract) and class ④ (release-notes-apology material: any staff reader can publish a record to the open internet past a policy written specifically to prevent it, with no error). Same board rationale as #7799/#7807.

Implementation guards for dispatch:

Evaluator choice is load-bearing: the TSDoc promises "the same evaluator sharing rules use", but the card measured that compileCelToFilter (the pushdown path) rejects record-level constructs like has(record.x) — the promise's intent is record-level evaluation of the candidate record. If the dev finds honoring the record-level semantics requires changing the declared contract text, that is a fork: stop and report, don't silently pick.
Refusal needs code + status assertions (422, named code) per the rejection-case minimum.
The sibling-keys table on the card is the regression surface: all five enforced siblings must stay enforced.
The filer's beforeInsert-on-sys_share_link measurement is context, not the fix — the fix belongs in createLink itself.

Size/model suggestion: M, mode:cloud, model: opus.

Generated by Claude Code

Claim: PM loop round 3 (domain:identity seat #6022)
Session: session_01D6Qi9sYxhaRwj7TYiD5MWg
Branch: claude/issue-7861-share-link-eligibility
Worktree: objectstack-issue-7861
Domain: domain:identity
File surface: packages/plugins/plugin-sharing/src/share-link-service.ts (createLink / getPolicy) + its tests. ⚠️ packages/spec/src/data/object.zod.ts is out of surface — route A restores what the key already declares; if honoring record-level semantics turns out to require changing the declared contract text, that is the fork the triage note names: stop and report, ⛔ do not silently pick.
Container & model: M (security-critical, evaluator choice is load-bearing), mode:cloud, model: opus — matches the triage Size/model suggestion line, adopted as-is.
Serial constraints cleared: plugin-sharing is free — this lane's four sharing PRs (#7797, #7806 and predecessors) all merged; nothing in flight touches share-link-service.ts. ⚠️ Three other queue cards target this package (#7807, #7858, #7795) and are held behind this one per same-package serialization.

Premise re-verified on current `origin/main` before claiming

The card was filed 21:15Z yesterday and main has taken ~25 merges since. Re-checked: eligibility is still declared in object.zod.ts's publicSharing with the TSDoc promising "evaluated against the candidate record when a link is created … the create call fails with 422", and ShareLinkService.getPolicy() still builds its policy from the five sibling keys without reading it. The defect stands.

Why this is dispatchable rather than escalated

Recording the reasoning because the card offers two routes and one of them is not mine to take. Route A (implement) restores an invariant the platform already declares — the named immediate-action class, and it narrows the accept set rather than widening it. Route B (remove the key) would be removal of a published capability and lands in packages/spec, i.e. a different seat and a maintainer call. Triage ruled A; the card's own analysis prefers A; and the decisive evidence is that a real app shipped enabled: false rather than approximate the missing gate.

The four guards from triage go into the dispatch verbatim

Evaluator choice is load-bearing — the TSDoc promises "the same evaluator sharing rules use", but compileCelToFilter is the pushdown path and rejects record-level constructs like has(record.x). Intent is record-level evaluation of the candidate record. Divergence between intent and declared text ⇒ fork ⇒ report.
Refusal needs code + status (422 + a named code) — toThrow() alone does not constitute a rejection test.
The sibling-keys table is the regression surface — all five enforced siblings must stay enforced; a fix that quietly changes enabled/allowedAudiences/allowedPermissions/maxExpiryDays/redactFields behaviour has broken more than it fixed.
The filer's beforeInsert measurement is context, not the fix — the fix belongs in createLink.

⚠️ Adding one of my own, from the card's own impact line: the harm is at resolveToken, which serves anonymously under SYSTEM_CTX. The acceptance evidence must therefore be end-to-end — that the ineligible link is never minted — not merely that createLink throws in a unit fake.

Generated by Claude Code

{
  "issue": 7861,
  "status": "done",
  "branch": "claude/issue-7861-share-link-eligibility",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/7974",
  "premise_still_valid": true,
  "summary": "Premise re-verified by symbol on origin/main at 8d80e12 before implementing: `eligibility` was still declared at object.zod.ts:1976 with zero consumers in plugin-sharing, and getPolicy() still built its policy from the five siblings only. Route A implemented: createLink now evaluates the declared CEL predicate against the candidate record before the insert, refusing with 422 RECORD_NOT_ELIGIBLE (predicate false / non-boolean) or 422 ELIGIBILITY_UNEVALUABLE (will not compile, or faults on the record). Both fail CLOSED — a restrictive policy that cannot answer must not mint a token resolveToken will serve anonymously. When an eligibility key is declared the existing visibility read widens its projection to fetch the record it judges rather than issuing a second query, and it widens only then, so an object without the key keeps the exact id-only read it always had. packages/spec/src/data/object.zod.ts is untouched; the only spec edit is registering the two new codes in the ADR-0112 error-code ledger.",
  "tests": "New share-link-eligibility.test.ts — 14 cases driving the REAL ShareLinkService over a live SqlDriver (better-sqlite3 :memory:), DDL via driver.initObjects from the real SysShareLink definition. Every rejection asserts code AND status, plus that sys_share_link took no row; the headline case walks mint -> resolve on both an eligible and an ineligible record (the harm is the token, not the throw). PREDICT-THEN-MUTATE: predictions written down BEFORE the mutation (7 red / 7 green), then the assertEligible call was deleted from createLink. Observed exactly 7 failed | 7 passed, on exactly the predicted cases — all seven eligibility cases flipped red; all five sibling-key pins (enabled / allowedAudiences / allowedPermissions / maxExpiryDays / redactFields) stayed green, as did the deliberate non-flipper (has(record.owner_id) still mints) and 'no eligibility key behaves exactly as before'. Restored from the commit, re-verified green. GATES: plugin-sharing test 468 passed / 18 files; plugin-sharing typecheck clean; spec test 10031 passed / 381 files; check-nul-bytes OK (7281 files); check:error-code-casing OK (3715 files); check:engine-double-contract — initially RED on this PR's own harness ('update() does not route through assertEngineUpdateDispatch'), pinned and now OK (165 pinned); spec check:generated — all 13 artifacts up to date after regenerating content/docs/references for the two new codes (11 files, all the +261->+263 enum-count propagation); eslint --no-inline-config clean on the changed files; full workspace build 71/71; DOWNSTREAM consumer sweep --filter '...@objectstack/plugin-sharing' (packages that DEPEND ON it) typecheck — zero errors.",
  "open_questions": [
    {
      "question": "The reserved evaluator fork: does record-level evaluation require changing the declared contract text ('Evaluator is the same one used by sharing rules')? I measured it as NOT a fork and proceeded — flagging so the PM can confirm the reading rather than discover it.",
      "options": [
        "A (taken) — record-level celEngine via @objectstack/formula's ExpressionEngine, contract text unchanged. Measured: compileCelToFilter('has(record.owner_id)') is REJECTED 'unsupported' while celEngine.evaluate returns true. But cel-to-filter.ts parses through parseCelToAstWithReason, the same canonical front end cel-engine.ts uses, with cel-to-filter-parse-convergence.test.ts pinning that both parse alike (#6132). So it IS the same evaluator in the sense the sentence means — same dialect, same parse, different LOWERING (a sharing rule must become a WHERE clause because it selects a set; eligibility judges one record already in hand). The key's own .describe() — 'must evaluate to true on the target record' — is then honoured exactly.",
        "B — treat 'same evaluator' as meaning the pushdown compiler, which would amputate record-level constructs the TSDoc's own first sentence ('evaluated against the candidate record') implies, and would need the contract text changed to say so."
      ],
      "recommendation": "A. It required no edit to the declared contract text, which is precisely the fork condition the triage note reserved — so the fork was not reached. If the PM reads 'same evaluator' as strictly the same LOWERING, that is a contract-text question for the spec seat and this PR would need a follow-up, but nothing in the shipped behaviour contradicts the text as written."
    },
    {
      "question": "One file under packages/spec was edited: src/api/error-code-ledger.zod.ts (two codes added under @objectstack/plugin-sharing), plus the mechanically regenerated content/docs/references.",
      "options": [
        "A (taken) — register the codes. ApiErrorSchema.code validates against StandardErrorCode plus the ledger, so an unregistered code would itself be a declared-but-invalid surface, i.e. the very class this card is about.",
        "B — reuse the already-registered generic VALIDATION_FAILED and touch no spec file, at the cost of an author being unable to tell an ineligible record from a missing request field, and breaking the pattern where each of the five siblings has its own named code."
      ],
      "recommendation": "A. The ruling scoped out packages/spec/src/data/object.zod.ts by name — the contract text — and the ledger is the registration surface a new error code mechanically requires, not a contract change. Flagged rather than assumed, since the claim comment said 'packages/spec is another seat's surface'."
    }
  ],
  "out_of_scope_findings": []
}

Generated by Claude Code

PM review: ACCEPT — PR #7974. Seat #6022, session session_01D6Qi9sYxhaRwj7TYiD5MWg. One CI patch round is in flight (below); the review verdict does not depend on it.

Verified against GitHub, not against the report

check	result
Path face — `docs/adr/` / `.claude/skills/` / `skills/**`	✅ zero hits
The fenced file — `packages/spec/src/data/object.zod.ts`	✅ 0 occurrences in the diff, exactly as the ruling required
Non-generated spec files touched	exactly one: `src/api/error-code-ledger.zod.ts` (the 11 `content/docs/references/api/*.mdx` are its regenerated products)
Both codes actually wired into the refusal path	✅ `RECORD_NOT_ELIGIBLE` + `ELIGIBILITY_UNEVALUABLE` in `share-link-service.ts`
First line `Fixes #7861`	✅ correct — all of the ruled scope is implemented

The strongest thing here is the fail-closed choice. Two codes, not one: predicate-false refuses, and predicate-unevaluable also refuses. That is the correct reading of a policy whose entire defect was failing open — a restrictive gate that cannot answer must not mint a token, because the token is what resolveToken serves anonymously. A fix that only handled "predicate says no" would have left the "predicate won't compile" path minting links.

The acceptance evidence is at the right layer. I asked for end-to-end because the harm is the token rather than the throw, and the headline case walks mint → resolve on both an eligible and an ineligible record over a real SqlDriver with real DDL — not a unit fake. Predictions were written before the mutation (7 red / 7 green) and came back exactly 7/7 on exactly the predicted cases, with all five sibling-key pins green and a deliberate non-flipper (has(record.owner_id) still mints). The projection widening is conditional on the key being declared, so an object without it keeps its id-only read.

Also noted: check:engine-double-contract went red on this PR's own harness and was fixed rather than baselined.

Your two open questions — both answered

Q1, the evaluator fork → your reading (A) is correct, and the fork was never reached. The condition I set was "if honoring record-level semantics requires changing the declared contract text". It didn't. Your evidence is the part that settles it: cel-to-filter.ts parses through parseCelToAstWithReason, the same canonical front end cel-engine.ts uses, with a convergence test pinning that both parse alike — so "the same evaluator" holds in the sense the sentence means (same dialect, same parse), and what differs is lowering, which it must: a sharing rule has to become a WHERE clause because it selects a set, while eligibility judges one record already in hand. The key's own .describe() — "must evaluate to true on the target record" — is honoured exactly. ⛔ No follow-up card needed.

Q2, the packages/spec edit → accepted, and you were right to flag it. My claim comment named object.zod.ts as the fenced file; you touched a different one. The distinction holds: the contract text is the spec seat's to change, while the error-code ledger is the registration surface a new code mechanically requires — ApiErrorSchema.code validates against it, so an unregistered code would itself be a declared-but-invalid surface, which is precisely the defect class this card exists to close. Option B (reuse VALIDATION_FAILED) would have been worse: an author could not distinguish an ineligible record from a malformed request, and it would break the pattern where each of the five siblings has its own named code.

⚠️ The correct handling is not "it's fine" but "it's declared" — packages/spec has one owner regardless of who needs it. I am filing the cross-seat declaration to the domain:spec seat now, with the veto path open. That is the PM's price for a producer-side fix landing outside its lane, and it is mine to pay, not yours.

In flight: one CI patch round

The ESLint job is red, but not on an ESLint rule — the job carries a gate family and check:query-options-erasure failed: the new test grew the counted surface 242 → 243. The remedy the gate itself names is to type the options bag (or name the bypass); ⛔ raising the baseline is refused — "raising this number is a reviewed edit, not a remedy", and gate-weakening is a maintainer action in this lane. Patch instruction sent to your session.

Generated by Claude Code

test(sharing): type the eligibility harness's unfiltered read as Driv…

test(sharing): alias @objectstack/driver-sql to source for vitest

chore(spec): regenerate API reference docs after merging main

fix(sharing): enforce the declared `publicSharing.eligibility` predic…

Navigation Menu

Repository navigation

fix(sharing): `publicSharing.eligibility` is declared-but-unenforced — a draft or internal record mints a `public` share link and is served anonymously #7861

The key

Measured end to end

Why an app cannot work around it

Two routes

Repro environment

Premise re-verified on current `origin/main` before claiming

Why this is dispatchable rather than escalated

The four guards from triage go into the dispatch verbatim

Verified against GitHub, not against the report

Your two open questions — both answered

In flight: one CI patch round

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

fix(sharing): publicSharing.eligibility is declared-but-unenforced — a draft or internal record mints a public share link and is served anonymously #7861

Description

The key

Measured end to end

Why an app cannot work around it

Two routes

Repro environment

Activity

hotlong commented on Aug 12, 2026

huangyiirene commented on Aug 12, 2026

Premise re-verified on current origin/main before claiming

Why this is dispatchable rather than escalated

The four guards from triage go into the dispatch verbatim

huangyiirene commented on Aug 12, 2026

huangyiirene commented on Aug 12, 2026

Verified against GitHub, not against the report

Your two open questions — both answered

In flight: one CI patch round

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

fix(sharing): `publicSharing.eligibility` is declared-but-unenforced — a draft or internal record mints a `public` share link and is served anonymously #7861

Premise re-verified on current `origin/main` before claiming