Skip to content

Commit b70a55d

Browse files
claude[bot]claude
andauthored
fix(plugin-sharing): per-grant catch in both reconcile loops so one refused grant no longer aborts the pass (#14754) (#14930)
* fix(plugin-sharing): per-grant catch in both reconcile loops so a refused grant no longer aborts the pass After #14484 `sys_record_share` is tenant-scoped in the #13491 ledger, so an organization-less system insert on it is refused loudly with `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` on a walled install. A platform-global sharing rule materialising a grant onto an organization-less record resolves no organization and meets that refusal; with no per-grant catch it propagated and that rule's reconcile pass aborted mid-loop, taking the pass's stale-row revocations with it. Those revocations are the security-relevant half: a stale over-grant persisted across every later pass, which met the same record and died in the same place. Both loops now attempt each grant individually. A refusal is logged with the rule, object, record, recipient and the engine's code, counted in the pass result, and the pass continues. The catch is narrow — only `ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` is absorbed. `record-share-organization-stamp.test.ts` deliberately pins the abort on the other error a pass can meet here (the scoped update half answering `RECORD_NOT_FOUND`), a shape the 2026-09-02 contract review left standing; a catch-all would retire that decision as a side effect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 * docs(permissions): re-anchor the system-context census to the two shifted elevation reads `check:check-system-context-census` failed with pure line rot: the per-grant catch added 37 lines above `sharing-rule-service.ts`'s two `context?.isSystem` elevation reads, so the census page's anchors pointed at :157/:382 while the reads now sit at :194/:419. Re-anchored with the gate's own `--fix`; no prose and no behaviour changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 * docs(permissions): re-derive the row-39 census anchors on the merged tree The merge with origin/main moved `sharing-rule-service.ts` — main added the `warnOnEmptyUnitExpansion` limb above both elevation reads — so the anchors this branch re-anchored before the merge point at the pre-merge lines. Machine re-derived with `pnpm gen:system-context-census`; nothing hand-edited. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8 * docs(permissions): re-derive the system-context census anchors on the merged tree The os-regen merge driver does not text-merge this file. Regenerated with `pnpm gen:system-context-census` on the committed merge, as its own commit: main's line rot (rows 21, 56, 63 and the misconception table) is re-anchored, and row 39's branch anchors are re-derived unchanged. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WXyGTWPbbreqXow7Z2pZCk * chore(spec): record the plugin-sharing organization-refusal MATCH as a provenance waiver (#14937) `check:error-code-provenance` scans for `constdef` stamp sites and cannot tell a package that RECOGNISES a registered code from one that EMITS it. The per-grant catch added for #14754 spells the engine's refusal code once, as `ENGINE_ORGANIZATION_REFUSAL_CODE`, and compares an incoming `err.code` against it — the gate reads that as an unlisted stamp site under `@objectstack/plugin-sharing`. Adjudicated on #14937 (maintainer ruling A, 2026-09-04): record it as a `PROVENANCE_WAIVERS` row naming `@objectstack/objectql` — the real emitter, whose owner key already carries the code (#8844) — rather than widening the gate or evading it with a bare inline literal. The row comes out together with the stamp site when #14936 lands and objectql publishes a recognizer. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WXyGTWPbbreqXow7Z2pZCk * docs(changeset): declare the wire surface `grantsRefused` reaches, and the client-type lag The evaluate route is a ledgered SDK route whose handler passes the service return value through unfiltered, so the seventh key is on the response body every caller already receives. The SDK method's declared resolved type is the spec's six-field `SharingRuleEvaluationResult`, which cannot name it — a client-type lag, additive on the wire, tracked as #14969. Declared here rather than lifted, because the spec type is a `domain:spec` single-owner file. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WXyGTWPbbreqXow7Z2pZCk --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 96b4af8 commit b70a55d

6 files changed

Lines changed: 700 additions & 66 deletions

File tree

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
---
2+
"@objectstack/plugin-sharing": minor
3+
---
4+
5+
fix(plugin-sharing): one refused grant no longer aborts a sharing rule's reconcile pass — its stale-row revocations still run (#14754)
6+
7+
After #14484 `sys_record_share` is `tenant-scoped` in the #13491 ledger, so on a
8+
walled install an organization-less system insert on it is refused loudly with
9+
`ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` (#8844). `SharingService.grant`
10+
resolves the organization on every path that can; a platform-global sharing
11+
rule (`organization_id = null`, its sweep unscoped) materialising a grant onto
12+
an organization-LESS record resolves none, and meets that refusal.
13+
14+
`SharingRuleService.reconcile` / `reconcileForRecord` had no per-grant catch, so
15+
the refusal propagated and **that rule's pass aborted mid-loop**. Two things
16+
were lost, and they are not equally serious:
17+
18+
- the remaining grants — recoverable, the next pass writes them;
19+
- **the stale-row revocations of that pass** — not recoverable by waiting,
20+
because every subsequent pass meets the same organization-less record and
21+
dies in the same place. A stale over-grant of that rule therefore persisted
22+
indefinitely, and the record kept aborting the pass until it was repaired by
23+
hand. That is the security-relevant half.
24+
25+
Measured while pinning this, and it sharpens the point: the engine returns
26+
organization-less rows **last** in a rule's criteria sweep (the driver's
27+
NULL-org compatibility arm is appended to the scoped arm). So a refused grant
28+
is nearly always one of the final attempts of a pass, and what an abort
29+
destroyed was hardly ever "the remaining grants" — it was almost entirely the
30+
revoke loop that runs after the whole upsert loop.
31+
32+
Both loops now attempt each grant individually. A refusal is logged with the
33+
rule, object, record, recipient and the engine's own code, counted, and the
34+
pass **continues** — the remaining grants and, above all, the stale-row
35+
revocations still run.
36+
37+
**The catch is deliberately narrow.** Only
38+
`ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED` is absorbed; every other error
39+
rethrows unchanged. A catch-all would swallow real defects and report a pass
40+
that "completed" having written nothing. It would also silently retire a
41+
reviewed decision: `record-share-organization-stamp.test.ts` deliberately pins
42+
the abort on the OTHER error a reconcile pass can meet here — the scoped update
43+
half answering `RECORD_NOT_FOUND` for a row stamped with a different
44+
organization — which the 2026-09-02 contract review left standing on "loud
45+
beats a wrong count". Those three pins are unchanged and still green.
46+
47+
**Why `minor` rather than `patch`.** The repair is a bug fix, but it reports
48+
through a new key. `reconcile` / `reconcileForRecord` / `evaluateRule` /
49+
`evaluateAllForRecord` now return `SharingRuleReconcilePassResult` — the spec's
50+
`SharingRuleEvaluationResult` plus `grantsRefused: number` — and that type is
51+
newly exported from the package index. Purely additive: the contract in
52+
`@objectstack/spec` is untouched, its six declared fields are unchanged, and a
53+
consumer typed against `ISharingRuleService` keeps compiling as it did. Same
54+
shape as `fix(runtime): tell an action handler when its caller-scope record load
55+
was refused` (#14143), which shipped `minor` for the same reason.
56+
57+
`grantsRefused > 0` does **not** mean the pass failed. It means the pass met a
58+
record it cannot grant on and carried on — which is the whole point.
59+
60+
**Wire surface — declared, not lifted.** `grantsRefused` reaches the wire.
61+
`POST /api/v1/sharing/rules/:idOrName/evaluate` is a ledgered **SDK** route —
62+
`packages/rest/src/rest-route-ledger.ts:390`, the row carrying
63+
`disposition: 'sdk'` and `client: 'shares.rules.evaluate'` — and its REST handler
64+
passes the service return value through **unfiltered**
65+
(`packages/rest/src/rest-server.ts:11108``:11109`:
66+
`const result = await svc.evaluateRule(req.params.idOrName, context ?? {})`
67+
followed by `res.json(result)`). So the seventh key is on the response body every
68+
caller of that route already receives. The SDK method declares
69+
`SharingRuleEvaluationResult` as its resolved type
70+
(`packages/client/src/index.ts:4766`, unwrapped at `:4771` through
71+
`unwrapResponse` parameterised on that same type), and that type is the spec's
72+
six-field contract — so the **declared client type cannot name the seventh key**.
73+
That is a client-type **lag**, not a contract break: the key is additive on the
74+
wire, every declared field is unchanged, and a consumer typed against
75+
`SharingRuleEvaluationResult` keeps compiling exactly as before. Lifting the type
76+
is not this PR's to do — `SharingRuleEvaluationResult` lives in
77+
`@objectstack/spec`, a `domain:spec` single-owner file — so the lag is declared
78+
here and tracked as the follow-up #14969, which lifts `grantsRefused?: number`
79+
(optional) into `SharingRuleEvaluationResult`.

content/docs/permissions/system-context.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ The largest single consumer — **17 of the 106 sites**.
137137
| 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1088` |
138138
| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `plugin-sharing/src/share-link-service.ts:469`, `:523`, `:527`, `:600`, `:630` |
139139
| 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` |
140-
| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:165`, `:390` |
140+
| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:202`, `:427` |
141141

142142
### 4. Approvals, reports, attachments, comments, knowledge
143143

packages/plugins/plugin-sharing/src/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ export {
2323
export {
2424
SharingRuleService,
2525
type SharingRuleServiceOptions,
26+
// [#14754] The spec's `SharingRuleEvaluationResult` plus the pass's
27+
// `grantsRefused` count. Additive: the six declared fields are unchanged, and
28+
// a consumer typed against the spec contract keeps compiling untouched.
29+
type SharingRuleReconcilePassResult,
2630
} from './sharing-rule-service.js';
2731
export {
2832
ShareLinkService,

0 commit comments

Comments
 (0)