Skip to content

fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it#13590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-steve merged 2 commits into from
Aug 31, 2026
Merged

fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it #13590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a72fcf3
fix(plugin-sharing): guard the record-share $in against nullish recor…
claude Aug 31, 2026
374de16
docs(permissions): re-anchor the system-context census rows onto thei…
claude Aug 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions .changeset/sharing-granted-ids-nullish-guard.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
---
"@objectstack/plugin-sharing": patch
---

fix(plugin-sharing): the record-share `$in` guard now tests `record_id` before `String()` coerces it (#13551)

`buildReadFilter` and the bulk-write half of `buildWriteFilter` each turned the
`sys_record_share` rows granted to the caller into the members of a security
predicate, `{ id: { $in: [...] } }`, with the same expression:

```ts
grants.map((g: any) => String(g.record_id)).filter(Boolean)
```

`.filter(Boolean)` reads as "drop rows whose `record_id` is nullish". It cannot:
`String(null)` is `'null'` and `String(undefined)` is `'undefined'`, and both are
truthy. The only value that spelling could drop was the empty string, so the
guard was dead for exactly the case its spelling advertised, and a
`sys_record_share` row with a nullish `record_id` put the literal string
`'null'` into the emitted `$in`.

**Direction — this was not an open bypass, and the repair is not a bypass fix.**
The emitted member is a bogus id that matches no row on any backend, and both
sites are positive polarity (an OR-ed branch beside the owner match, never
negated), so a corrupt row lost its grant rather than widening anyone's scope.
It also took an already-corrupt row to reach at all. What was actually broken is
the guard's honesty: a reader — or an audit asking which security paths already
handle nullish ids — would have counted these two sites as covered when they
provably were not.

Both sites now share one module-private helper that tests the raw column value
first and coerces after, the shape the sibling id-list guards already use
(`plugin-sharing`'s own `sharing-rule-service.ts` and `primary-bu-projection.ts`,
`core`'s `resolve-authz-context.ts`, `plugin-security`'s controlled-by-parent
`masterIds`, `objectql`'s master-detail parent resolution). Factoring it into one
helper is deliberate: the expression stood in two places, and repairing one would
have left the other advertising a guarantee it does not keep.

The non-null path is unchanged. Every non-nullish value still stringifies exactly
as it did — a driver-numeric primary key still becomes its decimal string — and
the empty string, the one value the old spelling really did drop, is still
dropped. The only behavioural difference is that rows with a nullish `record_id`
now contribute no member at all; when they were the *only* grants, the filter
collapses to the plain owner match instead of OR-ing in a branch that matched
nothing.
10 changes: 5 additions & 5 deletions content/docs/permissions/system-context.mdx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -129,11 +129,11 @@ The largest single consumer — **20 of the 109 sites**.
| # | Behaviour when `isSystem` | What you get / what you lose | Anchor |
|:--|:---|:---|:---|
| 30 | **Sharing-rule grant materialisation is skipped on all four record-write hooks** | Lose: **no `sys_record_share` rows are created**. A fully configured sharing rule grants **nothing** on seeded data until a rule is re-evaluated or the boot backfill runs. This is the behaviour that motivated #4707. Since #6783 the skip is no longer silent — it emits an INFO notice (rough edge 2) | `rule-hooks.ts:250`, `:274`, `:293`, `:322` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:625` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:891`, `:978`, `:1568` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1179` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1257` (guard at `:1282`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1309` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:654` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:920`, `:1007`, `:1597` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1208` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1286` (guard at `:1311`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1338` |
| 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` |
| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:413`, `:467`, `:471`, `:544`, `:574` |
| 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` |
Expand Down
82 changes: 82 additions & 0 deletions packages/plugins/plugin-sharing/src/sharing-service.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1704,3 +1704,85 @@ describe('[#6428] the boolean projection does not drift (compatibility clause)',
expect(await svc.checkEdit('account', 'a1', { isSystem: true })).toBe('allow');
});
});

// ─────────────────────────────────────────────────────────────────────

describe('[#13551] the record-share `$in` drops nullish `record_id` rows', () => {
// The guard standing in front of BOTH `$in` constructions used to read
// `.map((g) => String(g.record_id)).filter(Boolean)`, which cannot drop a
// nullish `record_id`: `String(null)` is `'null'`, `String(undefined)` is
// `'undefined'`, and both are truthy. What follows pins the MECHANISM — a
// row with no `record_id` contributes no member — and asserts nothing about
// whether such a row exists in the wild.
//
// The rows are seeded straight into the fake table on purpose: `grant()`
// refuses a nullish `recordId` at the front door, so writing the row
// directly is the only way to stand up the already-corrupt state the guard
// exists for — the shape a bad backfill or an out-of-band
// `sys_record_share` write would leave behind.
let engine: ReturnType<typeof makeFakeEngine>;
let svc: SharingService;

const shareRow = (record_id: unknown) => ({
id: `shr_${String(record_id)}`,
object_name: 'account',
record_id,
recipient_type: 'user',
recipient_id: 'alice',
access_level: 'edit', // in WRITE_ACCESS_LEVELS, so the write filter reads it too
});

beforeEach(() => {
engine = makeFakeEngine({
account: ACCOUNT_SCHEMA,
sys_record_share: { name: 'sys_record_share' },
});
svc = new SharingService({ engine });
});

it('read filter: a null / undefined `record_id` contributes NO member, and the real grant survives', async () => {
engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
expect(f.$or[1].id.$in).toEqual(['a1']);
// Named literally: these are the two members the dead guard used to emit.
expect(f.$or[1].id.$in).not.toContain('null');
expect(f.$or[1].id.$in).not.toContain('undefined');
});

it('write filter: the same rows, the same outcome — both construction sites are repaired', async () => {
engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
const f: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
expect(f.$or[1].id.$in).toEqual(['a1']);
expect(f.$or[1].id.$in).not.toContain('null');
expect(f.$or[1].id.$in).not.toContain('undefined');
});

it('when EVERY grant is nullish the share branch disappears, on both filters', async () => {
engine._tables.sys_record_share = [shareRow(null), shareRow(undefined)];
// Not an `$or` carrying a member that matches nothing: zero usable grants
// collapses to the owner match, which is what "no grants" already meant.
expect(await svc.buildReadFilter('account', { userId: 'alice' }))
.toEqual({ owner_id: 'alice' });
expect(await svc.buildWriteFilter('account', { userId: 'alice' }, 'update'))
.toEqual({ owner_id: 'alice' });
});

it('over-denial control: an ordinary grant set still produces exactly its ids, on both filters', async () => {
engine._tables.sys_record_share = [shareRow('a1'), shareRow('a2'), shareRow('a3')];
const read: any = await svc.buildReadFilter('account', { userId: 'alice' });
const write: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
expect(read.$or[0]).toEqual({ owner_id: 'alice' });
expect(write.$or[0]).toEqual({ owner_id: 'alice' });
expect(read.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
expect(write.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
});

it('the non-null path is unchanged: a driver-numeric id still stringifies, an empty string is still dropped', async () => {
engine._tables.sys_record_share = [shareRow(42), shareRow(''), shareRow('a1')];
const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
// `42` becomes `'42'` exactly as `String()` always made it, and `''` — the
// one value the old `.filter(Boolean)` could actually drop — is still
// dropped. Only the nullish rows are newly excluded.
expect(f.$or[1].id.$in).toEqual(['42', 'a1']);
});
});
41 changes: 35 additions & 6 deletions packages/plugins/plugin-sharing/src/sharing-service.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -143,6 +143,39 @@ function hasOwnerField(schema: any): boolean {
return Boolean(schema?.fields && OWNER_FIELD in schema.fields);
}

/**
* The `record_id` column of a `sys_record_share` read, as the members of a
* security `$in`. A row whose `record_id` is nullish or empty contributes
* NOTHING — the nullish test runs on the RAW column value, BEFORE `String()`.
*
* The order is the whole point. The previous spelling coerced first and
* filtered after — `grants.map((g) => String(g.record_id)).filter(Boolean)` —
* which cannot drop a nullish `record_id` at all: `String(null)` is `'null'`
* and `String(undefined)` is `'undefined'`, and both are truthy. The only
* value it could drop was the empty string, so the guard was dead for exactly
* the case its spelling advertised, and a corrupt row put the literal string
* `'null'` into `{ id: { $in: [...] } }`. Both call sites are positive
* polarity (an OR-ed branch, never negated) and no real record id matches that
* member, so the effect was a silently DROPPED grant rather than a widened
* scope — but an audit asking which security paths already handle nullish ids
* would have counted these two as covered when they provably were not.
*
* `String()` is kept for the surviving values: a driver may hand back a
* numeric primary key, and the members must compare against the string ids the
* rest of the filter is built from. Every non-nullish value therefore
* stringifies exactly as it did before, and the trailing `!== ''` drops
* precisely what `filter(Boolean)` used to drop — so the non-null path is
* unchanged and only the nullish rows are newly excluded.
*/
function grantedRecordIds(grants: unknown): string[] {
if (!Array.isArray(grants)) return [];
return grants
.map((g: any) => g?.record_id)
.filter((recordId: unknown) => recordId != null)
.map((recordId: unknown) => String(recordId))
.filter((recordId: string) => recordId !== '');
}

/**
* [#8418] The one WARN line a write gate emits when it refuses because the
* ownership fast-path was defeated by a FEDERATED object's phantom `owner_id`
Expand DownExpand Up@@ -416,9 +449,7 @@ export class SharingService implements ISharingService {
context: SYSTEM_CTX,
});

const grantedIds: string[] = Array.isArray(grants)
? grants.map((g: any) => String(g.record_id)).filter(Boolean)
: [];
const grantedIds: string[] = grantedRecordIds(grants);

if (grantedIds.length === 0) {
return ownerMatch;
Expand DownExpand Up@@ -494,9 +525,7 @@ export class SharingService implements ISharingService {
limit: 5000,
context: SYSTEM_CTX,
});
const grantedIds: string[] = Array.isArray(grants)
? grants.map((g: any) => String(g.record_id)).filter(Boolean)
: [];
const grantedIds: string[] = grantedRecordIds(grants);

if (grantedIds.length === 0) return ownerMatch;
return { $or: [ownerMatch, { id: { $in: grantedIds } }] };
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix(plugin-sharing): guard the record-share `$in` against a nullish `record_id` before `String()` coerces it by os-steve · Pull Request #13590 · objectstack-ai/objectstack · GitHub
Skip to content
fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it#13590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-steve
merged 2 commits into

from

Aug 31, 2026
 Merged


fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it
#13590
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
a72fcf3

fix(plugin-sharing): guard the record-share $in against nullish recor…


claude Aug 31, 2026
374de16

docs(permissions): re-anchor the system-context census rows onto thei…


claude Aug 31, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


45 changes: 45 additions & 0 deletions
.changeset/sharing-granted-ids-nullish-guard.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
---
"@objectstack/plugin-sharing": patch
---

fix(plugin-sharing): the record-share `$in` guard now tests `record_id` before `String()` coerces it (#13551)

`buildReadFilter` and the bulk-write half of `buildWriteFilter` each turned the
`sys_record_share` rows granted to the caller into the members of a security
predicate, `{ id: { $in: [...] } }`, with the same expression:

```ts
grants.map((g: any) => String(g.record_id)).filter(Boolean)
```

`.filter(Boolean)` reads as "drop rows whose `record_id` is nullish". It cannot:
`String(null)` is `'null'` and `String(undefined)` is `'undefined'`, and both are
truthy. The only value that spelling could drop was the empty string, so the
guard was dead for exactly the case its spelling advertised, and a
`sys_record_share` row with a nullish `record_id` put the literal string
`'null'` into the emitted `$in`.

**Direction — this was not an open bypass, and the repair is not a bypass fix.**
The emitted member is a bogus id that matches no row on any backend, and both
sites are positive polarity (an OR-ed branch beside the owner match, never
negated), so a corrupt row lost its grant rather than widening anyone's scope.
It also took an already-corrupt row to reach at all. What was actually broken is
the guard's honesty: a reader — or an audit asking which security paths already
handle nullish ids — would have counted these two sites as covered when they
provably were not.

Both sites now share one module-private helper that tests the raw column value
first and coerces after, the shape the sibling id-list guards already use
(`plugin-sharing`'s own `sharing-rule-service.ts` and `primary-bu-projection.ts`,
`core`'s `resolve-authz-context.ts`, `plugin-security`'s controlled-by-parent
`masterIds`, `objectql`'s master-detail parent resolution). Factoring it into one
helper is deliberate: the expression stood in two places, and repairing one would
have left the other advertising a guarantee it does not keep.

The non-null path is unchanged. Every non-nullish value still stringifies exactly
as it did — a driver-numeric primary key still becomes its decimal string — and
the empty string, the one value the old spelling really did drop, is still
dropped. The only behavioural difference is that rows with a nullish `record_id`
now contribute no member at all; when they were the *only* grants, the filter
collapses to the plain owner match instead of OR-ing in a branch that matched
nothing.

10 changes: 5 additions & 5 deletions
content/docs/permissions/system-context.mdx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -129,11 +129,11 @@ The largest single consumer — **20 of the 109 sites**.
| # | Behaviour when `isSystem` | What you get / what you lose | Anchor |
|:--|:---|:---|:---|
| 30 | **Sharing-rule grant materialisation is skipped on all four record-write hooks** | Lose: **no `sys_record_share` rows are created**. A fully configured sharing rule grants **nothing** on seeded data until a rule is re-evaluated or the boot backfill runs. This is the behaviour that motivated #4707. Since #6783 the skip is no longer silent — it emits an INFO notice (rough edge 2) | `rule-hooks.ts:250`, `:274`, `:293`, `:322` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:625` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:891`, `:978`, `:1568` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1179` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1257` (guard at `:1282`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1309` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:654` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:920`, `:1007`, `:1597` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1208` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1286` (guard at `:1311`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1338` |
| 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` |
| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:413`, `:467`, `:471`, `:544`, `:574` |
| 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` |
Expand Down

82 changes: 82 additions & 0 deletions
packages/plugins/plugin-sharing/src/sharing-service.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1704,3 +1704,85 @@ describe('[#6428] the boolean projection does not drift (compatibility clause)',
 expect(await svc.checkEdit('account', 'a1', { isSystem: true })).toBe('allow');
 });
});

// ─────────────────────────────────────────────────────────────────────

describe('[#13551] the record-share `$in` drops nullish `record_id` rows', () => {
 // The guard standing in front of BOTH `$in` constructions used to read
 // `.map((g) => String(g.record_id)).filter(Boolean)`, which cannot drop a
 // nullish `record_id`: `String(null)` is `'null'`, `String(undefined)` is
 // `'undefined'`, and both are truthy. What follows pins the MECHANISM — a
 // row with no `record_id` contributes no member — and asserts nothing about
 // whether such a row exists in the wild.
 //
 // The rows are seeded straight into the fake table on purpose: `grant()`
 // refuses a nullish `recordId` at the front door, so writing the row
 // directly is the only way to stand up the already-corrupt state the guard
 // exists for — the shape a bad backfill or an out-of-band
 // `sys_record_share` write would leave behind.
 let engine: ReturnType<typeof makeFakeEngine>;
 let svc: SharingService;

 const shareRow = (record_id: unknown) => ({
 id: `shr_${String(record_id)}`,
 object_name: 'account',
 record_id,
 recipient_type: 'user',
 recipient_id: 'alice',
 access_level: 'edit', // in WRITE_ACCESS_LEVELS, so the write filter reads it too
 });

 beforeEach(() => {
 engine = makeFakeEngine({
 account: ACCOUNT_SCHEMA,
 sys_record_share: { name: 'sys_record_share' },
 });
 svc = new SharingService({ engine });
 });

 it('read filter: a null / undefined `record_id` contributes NO member, and the real grant survives', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
 const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
 expect(f.$or[1].id.$in).toEqual(['a1']);
 // Named literally: these are the two members the dead guard used to emit.
 expect(f.$or[1].id.$in).not.toContain('null');
 expect(f.$or[1].id.$in).not.toContain('undefined');
 });

 it('write filter: the same rows, the same outcome — both construction sites are repaired', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
 const f: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
 expect(f.$or[1].id.$in).toEqual(['a1']);
 expect(f.$or[1].id.$in).not.toContain('null');
 expect(f.$or[1].id.$in).not.toContain('undefined');
 });

 it('when EVERY grant is nullish the share branch disappears, on both filters', async () => {
 engine._tables.sys_record_share = [shareRow(null), shareRow(undefined)];
 // Not an `$or` carrying a member that matches nothing: zero usable grants
 // collapses to the owner match, which is what "no grants" already meant.
 expect(await svc.buildReadFilter('account', { userId: 'alice' }))
 .toEqual({ owner_id: 'alice' });
 expect(await svc.buildWriteFilter('account', { userId: 'alice' }, 'update'))
 .toEqual({ owner_id: 'alice' });
 });

 it('over-denial control: an ordinary grant set still produces exactly its ids, on both filters', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow('a2'), shareRow('a3')];
 const read: any = await svc.buildReadFilter('account', { userId: 'alice' });
 const write: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
 expect(read.$or[0]).toEqual({ owner_id: 'alice' });
 expect(write.$or[0]).toEqual({ owner_id: 'alice' });
 expect(read.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
 expect(write.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
 });

 it('the non-null path is unchanged: a driver-numeric id still stringifies, an empty string is still dropped', async () => {
 engine._tables.sys_record_share = [shareRow(42), shareRow(''), shareRow('a1')];
 const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
 // `42` becomes `'42'` exactly as `String()` always made it, and `''` — the
 // one value the old `.filter(Boolean)` could actually drop — is still
 // dropped. Only the nullish rows are newly excluded.
 expect(f.$or[1].id.$in).toEqual(['42', 'a1']);
 });
});

41 changes: 35 additions & 6 deletions
packages/plugins/plugin-sharing/src/sharing-service.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -143,6 +143,39 @@ function hasOwnerField(schema: any): boolean {
 return Boolean(schema?.fields && OWNER_FIELD in schema.fields);
}

/**
 * The `record_id` column of a `sys_record_share` read, as the members of a
 * security `$in`. A row whose `record_id` is nullish or empty contributes
 * NOTHING — the nullish test runs on the RAW column value, BEFORE `String()`.
 *
 * The order is the whole point. The previous spelling coerced first and
 * filtered after — `grants.map((g) => String(g.record_id)).filter(Boolean)` —
 * which cannot drop a nullish `record_id` at all: `String(null)` is `'null'`
 * and `String(undefined)` is `'undefined'`, and both are truthy. The only
 * value it could drop was the empty string, so the guard was dead for exactly
 * the case its spelling advertised, and a corrupt row put the literal string
 * `'null'` into `{ id: { $in: [...] } }`. Both call sites are positive
 * polarity (an OR-ed branch, never negated) and no real record id matches that
 * member, so the effect was a silently DROPPED grant rather than a widened
 * scope — but an audit asking which security paths already handle nullish ids
 * would have counted these two as covered when they provably were not.
 *
 * `String()` is kept for the surviving values: a driver may hand back a
 * numeric primary key, and the members must compare against the string ids the
 * rest of the filter is built from. Every non-nullish value therefore
 * stringifies exactly as it did before, and the trailing `!== ''` drops
 * precisely what `filter(Boolean)` used to drop — so the non-null path is
 * unchanged and only the nullish rows are newly excluded.
 */
function grantedRecordIds(grants: unknown): string[] {
 if (!Array.isArray(grants)) return [];
 return grants
 .map((g: any) => g?.record_id)
 .filter((recordId: unknown) => recordId != null)
 .map((recordId: unknown) => String(recordId))
 .filter((recordId: string) => recordId !== '');
}

/**
 * [#8418] The one WARN line a write gate emits when it refuses because the
 * ownership fast-path was defeated by a FEDERATED object's phantom `owner_id`
Expand DownExpand Up@@ -416,9 +449,7 @@ export class SharingService implements ISharingService {
 context: SYSTEM_CTX,
 });

 const grantedIds: string[] = Array.isArray(grants)
 ? grants.map((g: any) => String(g.record_id)).filter(Boolean)
 : [];
 const grantedIds: string[] = grantedRecordIds(grants);

 if (grantedIds.length === 0) {
 return ownerMatch;
Expand DownExpand Up@@ -494,9 +525,7 @@ export class SharingService implements ISharingService {
 limit: 5000,
 context: SYSTEM_CTX,
 });
 const grantedIds: string[] = Array.isArray(grants)
 ? grants.map((g: any) => String(g.record_id)).filter(Boolean)
 : [];
 const grantedIds: string[] = grantedRecordIds(grants);

 if (grantedIds.length === 0) return ownerMatch;
 return { $or: [ownerMatch, { id: { $in: grantedIds } }] };
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
fix(plugin-sharing): guard the record-share `$in` against a nullish `record_id` before `String()` coerces it by os-steve · Pull Request #13590 · objectstack-ai/objectstack · GitHub
Skip to content
fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it#13590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-steve
merged 2 commits into

from

Aug 31, 2026
 Merged


fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it
#13590
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
a72fcf3

fix(plugin-sharing): guard the record-share $in against nullish recor…


claude Aug 31, 2026
374de16

docs(permissions): re-anchor the system-context census rows onto thei…


claude Aug 31, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


45 changes: 45 additions & 0 deletions
.changeset/sharing-granted-ids-nullish-guard.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
---
"@objectstack/plugin-sharing": patch
---

fix(plugin-sharing): the record-share `$in` guard now tests `record_id` before `String()` coerces it (#13551)

`buildReadFilter` and the bulk-write half of `buildWriteFilter` each turned the
`sys_record_share` rows granted to the caller into the members of a security
predicate, `{ id: { $in: [...] } }`, with the same expression:

```ts
grants.map((g: any) => String(g.record_id)).filter(Boolean)
```

`.filter(Boolean)` reads as "drop rows whose `record_id` is nullish". It cannot:
`String(null)` is `'null'` and `String(undefined)` is `'undefined'`, and both are
truthy. The only value that spelling could drop was the empty string, so the
guard was dead for exactly the case its spelling advertised, and a
`sys_record_share` row with a nullish `record_id` put the literal string
`'null'` into the emitted `$in`.

**Direction — this was not an open bypass, and the repair is not a bypass fix.**
The emitted member is a bogus id that matches no row on any backend, and both
sites are positive polarity (an OR-ed branch beside the owner match, never
negated), so a corrupt row lost its grant rather than widening anyone's scope.
It also took an already-corrupt row to reach at all. What was actually broken is
the guard's honesty: a reader — or an audit asking which security paths already
handle nullish ids — would have counted these two sites as covered when they
provably were not.

Both sites now share one module-private helper that tests the raw column value
first and coerces after, the shape the sibling id-list guards already use
(`plugin-sharing`'s own `sharing-rule-service.ts` and `primary-bu-projection.ts`,
`core`'s `resolve-authz-context.ts`, `plugin-security`'s controlled-by-parent
`masterIds`, `objectql`'s master-detail parent resolution). Factoring it into one
helper is deliberate: the expression stood in two places, and repairing one would
have left the other advertising a guarantee it does not keep.

The non-null path is unchanged. Every non-nullish value still stringifies exactly
as it did — a driver-numeric primary key still becomes its decimal string — and
the empty string, the one value the old spelling really did drop, is still
dropped. The only behavioural difference is that rows with a nullish `record_id`
now contribute no member at all; when they were the *only* grants, the filter
collapses to the plain owner match instead of OR-ing in a branch that matched
nothing.

10 changes: 5 additions & 5 deletions
content/docs/permissions/system-context.mdx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -129,11 +129,11 @@ The largest single consumer — **20 of the 109 sites**.
| # | Behaviour when `isSystem` | What you get / what you lose | Anchor |
|:--|:---|:---|:---|
| 30 | **Sharing-rule grant materialisation is skipped on all four record-write hooks** | Lose: **no `sys_record_share` rows are created**. A fully configured sharing rule grants **nothing** on seeded data until a rule is re-evaluated or the boot backfill runs. This is the behaviour that motivated #4707. Since #6783 the skip is no longer silent — it emits an INFO notice (rough edge 2) | `rule-hooks.ts:250`, `:274`, `:293`, `:322` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:625` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:891`, `:978`, `:1568` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1179` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1257` (guard at `:1282`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1309` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:654` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:920`, `:1007`, `:1597` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1208` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1286` (guard at `:1311`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1338` |
| 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` |
| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:413`, `:467`, `:471`, `:544`, `:574` |
| 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` |
Expand Down

82 changes: 82 additions & 0 deletions
packages/plugins/plugin-sharing/src/sharing-service.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1704,3 +1704,85 @@ describe('[#6428] the boolean projection does not drift (compatibility clause)',
 expect(await svc.checkEdit('account', 'a1', { isSystem: true })).toBe('allow');
 });
});

// ─────────────────────────────────────────────────────────────────────

describe('[#13551] the record-share `$in` drops nullish `record_id` rows', () => {
 // The guard standing in front of BOTH `$in` constructions used to read
 // `.map((g) => String(g.record_id)).filter(Boolean)`, which cannot drop a
 // nullish `record_id`: `String(null)` is `'null'`, `String(undefined)` is
 // `'undefined'`, and both are truthy. What follows pins the MECHANISM — a
 // row with no `record_id` contributes no member — and asserts nothing about
 // whether such a row exists in the wild.
 //
 // The rows are seeded straight into the fake table on purpose: `grant()`
 // refuses a nullish `recordId` at the front door, so writing the row
 // directly is the only way to stand up the already-corrupt state the guard
 // exists for — the shape a bad backfill or an out-of-band
 // `sys_record_share` write would leave behind.
 let engine: ReturnType<typeof makeFakeEngine>;
 let svc: SharingService;

 const shareRow = (record_id: unknown) => ({
 id: `shr_${String(record_id)}`,
 object_name: 'account',
 record_id,
 recipient_type: 'user',
 recipient_id: 'alice',
 access_level: 'edit', // in WRITE_ACCESS_LEVELS, so the write filter reads it too
 });

 beforeEach(() => {
 engine = makeFakeEngine({
 account: ACCOUNT_SCHEMA,
 sys_record_share: { name: 'sys_record_share' },
 });
 svc = new SharingService({ engine });
 });

 it('read filter: a null / undefined `record_id` contributes NO member, and the real grant survives', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
 const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
 expect(f.$or[1].id.$in).toEqual(['a1']);
 // Named literally: these are the two members the dead guard used to emit.
 expect(f.$or[1].id.$in).not.toContain('null');
 expect(f.$or[1].id.$in).not.toContain('undefined');
 });

 it('write filter: the same rows, the same outcome — both construction sites are repaired', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
 const f: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
 expect(f.$or[1].id.$in).toEqual(['a1']);
 expect(f.$or[1].id.$in).not.toContain('null');
 expect(f.$or[1].id.$in).not.toContain('undefined');
 });

 it('when EVERY grant is nullish the share branch disappears, on both filters', async () => {
 engine._tables.sys_record_share = [shareRow(null), shareRow(undefined)];
 // Not an `$or` carrying a member that matches nothing: zero usable grants
 // collapses to the owner match, which is what "no grants" already meant.
 expect(await svc.buildReadFilter('account', { userId: 'alice' }))
 .toEqual({ owner_id: 'alice' });
 expect(await svc.buildWriteFilter('account', { userId: 'alice' }, 'update'))
 .toEqual({ owner_id: 'alice' });
 });

 it('over-denial control: an ordinary grant set still produces exactly its ids, on both filters', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow('a2'), shareRow('a3')];
 const read: any = await svc.buildReadFilter('account', { userId: 'alice' });
 const write: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
 expect(read.$or[0]).toEqual({ owner_id: 'alice' });
 expect(write.$or[0]).toEqual({ owner_id: 'alice' });
 expect(read.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
 expect(write.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
 });

 it('the non-null path is unchanged: a driver-numeric id still stringifies, an empty string is still dropped', async () => {
 engine._tables.sys_record_share = [shareRow(42), shareRow(''), shareRow('a1')];
 const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
 // `42` becomes `'42'` exactly as `String()` always made it, and `''` — the
 // one value the old `.filter(Boolean)` could actually drop — is still
 // dropped. Only the nullish rows are newly excluded.
 expect(f.$or[1].id.$in).toEqual(['42', 'a1']);
 });
});

41 changes: 35 additions & 6 deletions
packages/plugins/plugin-sharing/src/sharing-service.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -143,6 +143,39 @@ function hasOwnerField(schema: any): boolean {
 return Boolean(schema?.fields && OWNER_FIELD in schema.fields);
}

/**
 * The `record_id` column of a `sys_record_share` read, as the members of a
 * security `$in`. A row whose `record_id` is nullish or empty contributes
 * NOTHING — the nullish test runs on the RAW column value, BEFORE `String()`.
 *
 * The order is the whole point. The previous spelling coerced first and
 * filtered after — `grants.map((g) => String(g.record_id)).filter(Boolean)` —
 * which cannot drop a nullish `record_id` at all: `String(null)` is `'null'`
 * and `String(undefined)` is `'undefined'`, and both are truthy. The only
 * value it could drop was the empty string, so the guard was dead for exactly
 * the case its spelling advertised, and a corrupt row put the literal string
 * `'null'` into `{ id: { $in: [...] } }`. Both call sites are positive
 * polarity (an OR-ed branch, never negated) and no real record id matches that
 * member, so the effect was a silently DROPPED grant rather than a widened
 * scope — but an audit asking which security paths already handle nullish ids
 * would have counted these two as covered when they provably were not.
 *
 * `String()` is kept for the surviving values: a driver may hand back a
 * numeric primary key, and the members must compare against the string ids the
 * rest of the filter is built from. Every non-nullish value therefore
 * stringifies exactly as it did before, and the trailing `!== ''` drops
 * precisely what `filter(Boolean)` used to drop — so the non-null path is
 * unchanged and only the nullish rows are newly excluded.
 */
function grantedRecordIds(grants: unknown): string[] {
 if (!Array.isArray(grants)) return [];
 return grants
 .map((g: any) => g?.record_id)
 .filter((recordId: unknown) => recordId != null)
 .map((recordId: unknown) => String(recordId))
 .filter((recordId: string) => recordId !== '');
}

/**
 * [#8418] The one WARN line a write gate emits when it refuses because the
 * ownership fast-path was defeated by a FEDERATED object's phantom `owner_id`
Expand DownExpand Up@@ -416,9 +449,7 @@ export class SharingService implements ISharingService {
 context: SYSTEM_CTX,
 });

 const grantedIds: string[] = Array.isArray(grants)
 ? grants.map((g: any) => String(g.record_id)).filter(Boolean)
 : [];
 const grantedIds: string[] = grantedRecordIds(grants);

 if (grantedIds.length === 0) {
 return ownerMatch;
Expand DownExpand Up@@ -494,9 +525,7 @@ export class SharingService implements ISharingService {
 limit: 5000,
 context: SYSTEM_CTX,
 });
 const grantedIds: string[] = Array.isArray(grants)
 ? grants.map((g: any) => String(g.record_id)).filter(Boolean)
 : [];
 const grantedIds: string[] = grantedRecordIds(grants);

 if (grantedIds.length === 0) return ownerMatch;
 return { $or: [ownerMatch, { id: { $in: grantedIds } }] };
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
fix(plugin-sharing): guard the record-share `$in` against a nullish `record_id` before `String()` coerces it by os-steve · Pull Request #13590 · objectstack-ai/objectstack · GitHub
Skip to content
fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it#13590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-steve
merged 2 commits into

from

Aug 31, 2026
 Merged


fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it
#13590
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
a72fcf3

fix(plugin-sharing): guard the record-share $in against nullish recor…


claude Aug 31, 2026
374de16

docs(permissions): re-anchor the system-context census rows onto thei…


claude Aug 31, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


45 changes: 45 additions & 0 deletions
.changeset/sharing-granted-ids-nullish-guard.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
---
"@objectstack/plugin-sharing": patch
---

fix(plugin-sharing): the record-share `$in` guard now tests `record_id` before `String()` coerces it (#13551)

`buildReadFilter` and the bulk-write half of `buildWriteFilter` each turned the
`sys_record_share` rows granted to the caller into the members of a security
predicate, `{ id: { $in: [...] } }`, with the same expression:

```ts
grants.map((g: any) => String(g.record_id)).filter(Boolean)
```

`.filter(Boolean)` reads as "drop rows whose `record_id` is nullish". It cannot:
`String(null)` is `'null'` and `String(undefined)` is `'undefined'`, and both are
truthy. The only value that spelling could drop was the empty string, so the
guard was dead for exactly the case its spelling advertised, and a
`sys_record_share` row with a nullish `record_id` put the literal string
`'null'` into the emitted `$in`.

**Direction — this was not an open bypass, and the repair is not a bypass fix.**
The emitted member is a bogus id that matches no row on any backend, and both
sites are positive polarity (an OR-ed branch beside the owner match, never
negated), so a corrupt row lost its grant rather than widening anyone's scope.
It also took an already-corrupt row to reach at all. What was actually broken is
the guard's honesty: a reader — or an audit asking which security paths already
handle nullish ids — would have counted these two sites as covered when they
provably were not.

Both sites now share one module-private helper that tests the raw column value
first and coerces after, the shape the sibling id-list guards already use
(`plugin-sharing`'s own `sharing-rule-service.ts` and `primary-bu-projection.ts`,
`core`'s `resolve-authz-context.ts`, `plugin-security`'s controlled-by-parent
`masterIds`, `objectql`'s master-detail parent resolution). Factoring it into one
helper is deliberate: the expression stood in two places, and repairing one would
have left the other advertising a guarantee it does not keep.

The non-null path is unchanged. Every non-nullish value still stringifies exactly
as it did — a driver-numeric primary key still becomes its decimal string — and
the empty string, the one value the old spelling really did drop, is still
dropped. The only behavioural difference is that rows with a nullish `record_id`
now contribute no member at all; when they were the *only* grants, the filter
collapses to the plain owner match instead of OR-ing in a branch that matched
nothing.

10 changes: 5 additions & 5 deletions
content/docs/permissions/system-context.mdx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -129,11 +129,11 @@ The largest single consumer — **20 of the 109 sites**.
| # | Behaviour when `isSystem` | What you get / what you lose | Anchor |
|:--|:---|:---|:---|
| 30 | **Sharing-rule grant materialisation is skipped on all four record-write hooks** | Lose: **no `sys_record_share` rows are created**. A fully configured sharing rule grants **nothing** on seeded data until a rule is re-evaluated or the boot backfill runs. This is the behaviour that motivated #4707. Since #6783 the skip is no longer silent — it emits an INFO notice (rough edge 2) | `rule-hooks.ts:250`, `:274`, `:293`, `:322` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:625` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:891`, `:978`, `:1568` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1179` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1257` (guard at `:1282`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1309` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:654` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:920`, `:1007`, `:1597` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1208` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1286` (guard at `:1311`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1338` |
| 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` |
| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:413`, `:467`, `:471`, `:544`, `:574` |
| 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` |
Expand Down

82 changes: 82 additions & 0 deletions
packages/plugins/plugin-sharing/src/sharing-service.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1704,3 +1704,85 @@ describe('[#6428] the boolean projection does not drift (compatibility clause)',
 expect(await svc.checkEdit('account', 'a1', { isSystem: true })).toBe('allow');
 });
});

// ─────────────────────────────────────────────────────────────────────

describe('[#13551] the record-share `$in` drops nullish `record_id` rows', () => {
 // The guard standing in front of BOTH `$in` constructions used to read
 // `.map((g) => String(g.record_id)).filter(Boolean)`, which cannot drop a
 // nullish `record_id`: `String(null)` is `'null'`, `String(undefined)` is
 // `'undefined'`, and both are truthy. What follows pins the MECHANISM — a
 // row with no `record_id` contributes no member — and asserts nothing about
 // whether such a row exists in the wild.
 //
 // The rows are seeded straight into the fake table on purpose: `grant()`
 // refuses a nullish `recordId` at the front door, so writing the row
 // directly is the only way to stand up the already-corrupt state the guard
 // exists for — the shape a bad backfill or an out-of-band
 // `sys_record_share` write would leave behind.
 let engine: ReturnType<typeof makeFakeEngine>;
 let svc: SharingService;

 const shareRow = (record_id: unknown) => ({
 id: `shr_${String(record_id)}`,
 object_name: 'account',
 record_id,
 recipient_type: 'user',
 recipient_id: 'alice',
 access_level: 'edit', // in WRITE_ACCESS_LEVELS, so the write filter reads it too
 });

 beforeEach(() => {
 engine = makeFakeEngine({
 account: ACCOUNT_SCHEMA,
 sys_record_share: { name: 'sys_record_share' },
 });
 svc = new SharingService({ engine });
 });

 it('read filter: a null / undefined `record_id` contributes NO member, and the real grant survives', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
 const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
 expect(f.$or[1].id.$in).toEqual(['a1']);
 // Named literally: these are the two members the dead guard used to emit.
 expect(f.$or[1].id.$in).not.toContain('null');
 expect(f.$or[1].id.$in).not.toContain('undefined');
 });

 it('write filter: the same rows, the same outcome — both construction sites are repaired', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
 const f: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
 expect(f.$or[1].id.$in).toEqual(['a1']);
 expect(f.$or[1].id.$in).not.toContain('null');
 expect(f.$or[1].id.$in).not.toContain('undefined');
 });

 it('when EVERY grant is nullish the share branch disappears, on both filters', async () => {
 engine._tables.sys_record_share = [shareRow(null), shareRow(undefined)];
 // Not an `$or` carrying a member that matches nothing: zero usable grants
 // collapses to the owner match, which is what "no grants" already meant.
 expect(await svc.buildReadFilter('account', { userId: 'alice' }))
 .toEqual({ owner_id: 'alice' });
 expect(await svc.buildWriteFilter('account', { userId: 'alice' }, 'update'))
 .toEqual({ owner_id: 'alice' });
 });

 it('over-denial control: an ordinary grant set still produces exactly its ids, on both filters', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow('a2'), shareRow('a3')];
 const read: any = await svc.buildReadFilter('account', { userId: 'alice' });
 const write: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
 expect(read.$or[0]).toEqual({ owner_id: 'alice' });
 expect(write.$or[0]).toEqual({ owner_id: 'alice' });
 expect(read.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
 expect(write.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
 });

 it('the non-null path is unchanged: a driver-numeric id still stringifies, an empty string is still dropped', async () => {
 engine._tables.sys_record_share = [shareRow(42), shareRow(''), shareRow('a1')];
 const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
 // `42` becomes `'42'` exactly as `String()` always made it, and `''` — the
 // one value the old `.filter(Boolean)` could actually drop — is still
 // dropped. Only the nullish rows are newly excluded.
 expect(f.$or[1].id.$in).toEqual(['42', 'a1']);
 });
});

41 changes: 35 additions & 6 deletions
packages/plugins/plugin-sharing/src/sharing-service.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -143,6 +143,39 @@ function hasOwnerField(schema: any): boolean {
 return Boolean(schema?.fields && OWNER_FIELD in schema.fields);
}

/**
 * The `record_id` column of a `sys_record_share` read, as the members of a
 * security `$in`. A row whose `record_id` is nullish or empty contributes
 * NOTHING — the nullish test runs on the RAW column value, BEFORE `String()`.
 *
 * The order is the whole point. The previous spelling coerced first and
 * filtered after — `grants.map((g) => String(g.record_id)).filter(Boolean)` —
 * which cannot drop a nullish `record_id` at all: `String(null)` is `'null'`
 * and `String(undefined)` is `'undefined'`, and both are truthy. The only
 * value it could drop was the empty string, so the guard was dead for exactly
 * the case its spelling advertised, and a corrupt row put the literal string
 * `'null'` into `{ id: { $in: [...] } }`. Both call sites are positive
 * polarity (an OR-ed branch, never negated) and no real record id matches that
 * member, so the effect was a silently DROPPED grant rather than a widened
 * scope — but an audit asking which security paths already handle nullish ids
 * would have counted these two as covered when they provably were not.
 *
 * `String()` is kept for the surviving values: a driver may hand back a
 * numeric primary key, and the members must compare against the string ids the
 * rest of the filter is built from. Every non-nullish value therefore
 * stringifies exactly as it did before, and the trailing `!== ''` drops
 * precisely what `filter(Boolean)` used to drop — so the non-null path is
 * unchanged and only the nullish rows are newly excluded.
 */
function grantedRecordIds(grants: unknown): string[] {
 if (!Array.isArray(grants)) return [];
 return grants
 .map((g: any) => g?.record_id)
 .filter((recordId: unknown) => recordId != null)
 .map((recordId: unknown) => String(recordId))
 .filter((recordId: string) => recordId !== '');
}

/**
 * [#8418] The one WARN line a write gate emits when it refuses because the
 * ownership fast-path was defeated by a FEDERATED object's phantom `owner_id`
Expand DownExpand Up@@ -416,9 +449,7 @@ export class SharingService implements ISharingService {
 context: SYSTEM_CTX,
 });

 const grantedIds: string[] = Array.isArray(grants)
 ? grants.map((g: any) => String(g.record_id)).filter(Boolean)
 : [];
 const grantedIds: string[] = grantedRecordIds(grants);

 if (grantedIds.length === 0) {
 return ownerMatch;
Expand DownExpand Up@@ -494,9 +525,7 @@ export class SharingService implements ISharingService {
 limit: 5000,
 context: SYSTEM_CTX,
 });
 const grantedIds: string[] = Array.isArray(grants)
 ? grants.map((g: any) => String(g.record_id)).filter(Boolean)
 : [];
 const grantedIds: string[] = grantedRecordIds(grants);

 if (grantedIds.length === 0) return ownerMatch;
 return { $or: [ownerMatch, { id: { $in: grantedIds } }] };
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
fix(plugin-sharing): guard the record-share `$in` against a nullish `record_id` before `String()` coerces it by os-steve · Pull Request #13590 · objectstack-ai/objectstack · GitHub
Skip to content
fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it#13590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-steve
merged 2 commits into

from

Aug 31, 2026
 Merged


fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it
#13590
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
a72fcf3

fix(plugin-sharing): guard the record-share $in against nullish recor…


claude Aug 31, 2026
374de16

docs(permissions): re-anchor the system-context census rows onto thei…


claude Aug 31, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


45 changes: 45 additions & 0 deletions
.changeset/sharing-granted-ids-nullish-guard.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
---
"@objectstack/plugin-sharing": patch
---

fix(plugin-sharing): the record-share `$in` guard now tests `record_id` before `String()` coerces it (#13551)

`buildReadFilter` and the bulk-write half of `buildWriteFilter` each turned the
`sys_record_share` rows granted to the caller into the members of a security
predicate, `{ id: { $in: [...] } }`, with the same expression:

```ts
grants.map((g: any) => String(g.record_id)).filter(Boolean)
```

`.filter(Boolean)` reads as "drop rows whose `record_id` is nullish". It cannot:
`String(null)` is `'null'` and `String(undefined)` is `'undefined'`, and both are
truthy. The only value that spelling could drop was the empty string, so the
guard was dead for exactly the case its spelling advertised, and a
`sys_record_share` row with a nullish `record_id` put the literal string
`'null'` into the emitted `$in`.

**Direction — this was not an open bypass, and the repair is not a bypass fix.**
The emitted member is a bogus id that matches no row on any backend, and both
sites are positive polarity (an OR-ed branch beside the owner match, never
negated), so a corrupt row lost its grant rather than widening anyone's scope.
It also took an already-corrupt row to reach at all. What was actually broken is
the guard's honesty: a reader — or an audit asking which security paths already
handle nullish ids — would have counted these two sites as covered when they
provably were not.

Both sites now share one module-private helper that tests the raw column value
first and coerces after, the shape the sibling id-list guards already use
(`plugin-sharing`'s own `sharing-rule-service.ts` and `primary-bu-projection.ts`,
`core`'s `resolve-authz-context.ts`, `plugin-security`'s controlled-by-parent
`masterIds`, `objectql`'s master-detail parent resolution). Factoring it into one
helper is deliberate: the expression stood in two places, and repairing one would
have left the other advertising a guarantee it does not keep.

The non-null path is unchanged. Every non-nullish value still stringifies exactly
as it did — a driver-numeric primary key still becomes its decimal string — and
the empty string, the one value the old spelling really did drop, is still
dropped. The only behavioural difference is that rows with a nullish `record_id`
now contribute no member at all; when they were the *only* grants, the filter
collapses to the plain owner match instead of OR-ing in a branch that matched
nothing.

10 changes: 5 additions & 5 deletions
content/docs/permissions/system-context.mdx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -129,11 +129,11 @@ The largest single consumer — **20 of the 109 sites**.
| # | Behaviour when `isSystem` | What you get / what you lose | Anchor |
|:--|:---|:---|:---|
| 30 | **Sharing-rule grant materialisation is skipped on all four record-write hooks** | Lose: **no `sys_record_share` rows are created**. A fully configured sharing rule grants **nothing** on seeded data until a rule is re-evaluated or the boot backfill runs. This is the behaviour that motivated #4707. Since #6783 the skip is no longer silent — it emits an INFO notice (rough edge 2) | `rule-hooks.ts:250`, `:274`, `:293`, `:322` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:625` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:891`, `:978`, `:1568` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1179` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1257` (guard at `:1282`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1309` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:654` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:920`, `:1007`, `:1597` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1208` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1286` (guard at `:1311`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1338` |
| 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` |
| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:413`, `:467`, `:471`, `:544`, `:574` |
| 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` |
Expand Down

82 changes: 82 additions & 0 deletions
packages/plugins/plugin-sharing/src/sharing-service.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1704,3 +1704,85 @@ describe('[#6428] the boolean projection does not drift (compatibility clause)',
 expect(await svc.checkEdit('account', 'a1', { isSystem: true })).toBe('allow');
 });
});

// ─────────────────────────────────────────────────────────────────────

describe('[#13551] the record-share `$in` drops nullish `record_id` rows', () => {
 // The guard standing in front of BOTH `$in` constructions used to read
 // `.map((g) => String(g.record_id)).filter(Boolean)`, which cannot drop a
 // nullish `record_id`: `String(null)` is `'null'`, `String(undefined)` is
 // `'undefined'`, and both are truthy. What follows pins the MECHANISM — a
 // row with no `record_id` contributes no member — and asserts nothing about
 // whether such a row exists in the wild.
 //
 // The rows are seeded straight into the fake table on purpose: `grant()`
 // refuses a nullish `recordId` at the front door, so writing the row
 // directly is the only way to stand up the already-corrupt state the guard
 // exists for — the shape a bad backfill or an out-of-band
 // `sys_record_share` write would leave behind.
 let engine: ReturnType<typeof makeFakeEngine>;
 let svc: SharingService;

 const shareRow = (record_id: unknown) => ({
 id: `shr_${String(record_id)}`,
 object_name: 'account',
 record_id,
 recipient_type: 'user',
 recipient_id: 'alice',
 access_level: 'edit', // in WRITE_ACCESS_LEVELS, so the write filter reads it too
 });

 beforeEach(() => {
 engine = makeFakeEngine({
 account: ACCOUNT_SCHEMA,
 sys_record_share: { name: 'sys_record_share' },
 });
 svc = new SharingService({ engine });
 });

 it('read filter: a null / undefined `record_id` contributes NO member, and the real grant survives', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
 const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
 expect(f.$or[1].id.$in).toEqual(['a1']);
 // Named literally: these are the two members the dead guard used to emit.
 expect(f.$or[1].id.$in).not.toContain('null');
 expect(f.$or[1].id.$in).not.toContain('undefined');
 });

 it('write filter: the same rows, the same outcome — both construction sites are repaired', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
 const f: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
 expect(f.$or[1].id.$in).toEqual(['a1']);
 expect(f.$or[1].id.$in).not.toContain('null');
 expect(f.$or[1].id.$in).not.toContain('undefined');
 });

 it('when EVERY grant is nullish the share branch disappears, on both filters', async () => {
 engine._tables.sys_record_share = [shareRow(null), shareRow(undefined)];
 // Not an `$or` carrying a member that matches nothing: zero usable grants
 // collapses to the owner match, which is what "no grants" already meant.
 expect(await svc.buildReadFilter('account', { userId: 'alice' }))
 .toEqual({ owner_id: 'alice' });
 expect(await svc.buildWriteFilter('account', { userId: 'alice' }, 'update'))
 .toEqual({ owner_id: 'alice' });
 });

 it('over-denial control: an ordinary grant set still produces exactly its ids, on both filters', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow('a2'), shareRow('a3')];
 const read: any = await svc.buildReadFilter('account', { userId: 'alice' });
 const write: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
 expect(read.$or[0]).toEqual({ owner_id: 'alice' });
 expect(write.$or[0]).toEqual({ owner_id: 'alice' });
 expect(read.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
 expect(write.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
 });

 it('the non-null path is unchanged: a driver-numeric id still stringifies, an empty string is still dropped', async () => {
 engine._tables.sys_record_share = [shareRow(42), shareRow(''), shareRow('a1')];
 const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
 // `42` becomes `'42'` exactly as `String()` always made it, and `''` — the
 // one value the old `.filter(Boolean)` could actually drop — is still
 // dropped. Only the nullish rows are newly excluded.
 expect(f.$or[1].id.$in).toEqual(['42', 'a1']);
 });
});

41 changes: 35 additions & 6 deletions
packages/plugins/plugin-sharing/src/sharing-service.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -143,6 +143,39 @@ function hasOwnerField(schema: any): boolean {
 return Boolean(schema?.fields && OWNER_FIELD in schema.fields);
}

/**
 * The `record_id` column of a `sys_record_share` read, as the members of a
 * security `$in`. A row whose `record_id` is nullish or empty contributes
 * NOTHING — the nullish test runs on the RAW column value, BEFORE `String()`.
 *
 * The order is the whole point. The previous spelling coerced first and
 * filtered after — `grants.map((g) => String(g.record_id)).filter(Boolean)` —
 * which cannot drop a nullish `record_id` at all: `String(null)` is `'null'`
 * and `String(undefined)` is `'undefined'`, and both are truthy. The only
 * value it could drop was the empty string, so the guard was dead for exactly
 * the case its spelling advertised, and a corrupt row put the literal string
 * `'null'` into `{ id: { $in: [...] } }`. Both call sites are positive
 * polarity (an OR-ed branch, never negated) and no real record id matches that
 * member, so the effect was a silently DROPPED grant rather than a widened
 * scope — but an audit asking which security paths already handle nullish ids
 * would have counted these two as covered when they provably were not.
 *
 * `String()` is kept for the surviving values: a driver may hand back a
 * numeric primary key, and the members must compare against the string ids the
 * rest of the filter is built from. Every non-nullish value therefore
 * stringifies exactly as it did before, and the trailing `!== ''` drops
 * precisely what `filter(Boolean)` used to drop — so the non-null path is
 * unchanged and only the nullish rows are newly excluded.
 */
function grantedRecordIds(grants: unknown): string[] {
 if (!Array.isArray(grants)) return [];
 return grants
 .map((g: any) => g?.record_id)
 .filter((recordId: unknown) => recordId != null)
 .map((recordId: unknown) => String(recordId))
 .filter((recordId: string) => recordId !== '');
}

/**
 * [#8418] The one WARN line a write gate emits when it refuses because the
 * ownership fast-path was defeated by a FEDERATED object's phantom `owner_id`
Expand DownExpand Up@@ -416,9 +449,7 @@ export class SharingService implements ISharingService {
 context: SYSTEM_CTX,
 });

 const grantedIds: string[] = Array.isArray(grants)
 ? grants.map((g: any) => String(g.record_id)).filter(Boolean)
 : [];
 const grantedIds: string[] = grantedRecordIds(grants);

 if (grantedIds.length === 0) {
 return ownerMatch;
Expand DownExpand Up@@ -494,9 +525,7 @@ export class SharingService implements ISharingService {
 limit: 5000,
 context: SYSTEM_CTX,
 });
 const grantedIds: string[] = Array.isArray(grants)
 ? grants.map((g: any) => String(g.record_id)).filter(Boolean)
 : [];
 const grantedIds: string[] = grantedRecordIds(grants);

 if (grantedIds.length === 0) return ownerMatch;
 return { $or: [ownerMatch, { id: { $in: grantedIds } }] };
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
fix(plugin-sharing): guard the record-share `$in` against a nullish `record_id` before `String()` coerces it by os-steve · Pull Request #13590 · objectstack-ai/objectstack · GitHub
Skip to content
fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it#13590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-steve
merged 2 commits into

from

Aug 31, 2026
 Merged


fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it
#13590
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
a72fcf3

fix(plugin-sharing): guard the record-share $in against nullish recor…


claude Aug 31, 2026
374de16

docs(permissions): re-anchor the system-context census rows onto thei…


claude Aug 31, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


45 changes: 45 additions & 0 deletions
.changeset/sharing-granted-ids-nullish-guard.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
---
"@objectstack/plugin-sharing": patch
---

fix(plugin-sharing): the record-share `$in` guard now tests `record_id` before `String()` coerces it (#13551)

`buildReadFilter` and the bulk-write half of `buildWriteFilter` each turned the
`sys_record_share` rows granted to the caller into the members of a security
predicate, `{ id: { $in: [...] } }`, with the same expression:

```ts
grants.map((g: any) => String(g.record_id)).filter(Boolean)
```

`.filter(Boolean)` reads as "drop rows whose `record_id` is nullish". It cannot:
`String(null)` is `'null'` and `String(undefined)` is `'undefined'`, and both are
truthy. The only value that spelling could drop was the empty string, so the
guard was dead for exactly the case its spelling advertised, and a
`sys_record_share` row with a nullish `record_id` put the literal string
`'null'` into the emitted `$in`.

**Direction — this was not an open bypass, and the repair is not a bypass fix.**
The emitted member is a bogus id that matches no row on any backend, and both
sites are positive polarity (an OR-ed branch beside the owner match, never
negated), so a corrupt row lost its grant rather than widening anyone's scope.
It also took an already-corrupt row to reach at all. What was actually broken is
the guard's honesty: a reader — or an audit asking which security paths already
handle nullish ids — would have counted these two sites as covered when they
provably were not.

Both sites now share one module-private helper that tests the raw column value
first and coerces after, the shape the sibling id-list guards already use
(`plugin-sharing`'s own `sharing-rule-service.ts` and `primary-bu-projection.ts`,
`core`'s `resolve-authz-context.ts`, `plugin-security`'s controlled-by-parent
`masterIds`, `objectql`'s master-detail parent resolution). Factoring it into one
helper is deliberate: the expression stood in two places, and repairing one would
have left the other advertising a guarantee it does not keep.

The non-null path is unchanged. Every non-nullish value still stringifies exactly
as it did — a driver-numeric primary key still becomes its decimal string — and
the empty string, the one value the old spelling really did drop, is still
dropped. The only behavioural difference is that rows with a nullish `record_id`
now contribute no member at all; when they were the *only* grants, the filter
collapses to the plain owner match instead of OR-ing in a branch that matched
nothing.

10 changes: 5 additions & 5 deletions
content/docs/permissions/system-context.mdx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -129,11 +129,11 @@ The largest single consumer — **20 of the 109 sites**.
| # | Behaviour when `isSystem` | What you get / what you lose | Anchor |
|:--|:---|:---|:---|
| 30 | **Sharing-rule grant materialisation is skipped on all four record-write hooks** | Lose: **no `sys_record_share` rows are created**. A fully configured sharing rule grants **nothing** on seeded data until a rule is re-evaluated or the boot backfill runs. This is the behaviour that motivated #4707. Since #6783 the skip is no longer silent — it emits an INFO notice (rough edge 2) | `rule-hooks.ts:250`, `:274`, `:293`, `:322` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:625` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:891`, `:978`, `:1568` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1179` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1257` (guard at `:1282`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1309` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:654` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:920`, `:1007`, `:1597` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1208` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1286` (guard at `:1311`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1338` |
| 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` |
| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:413`, `:467`, `:471`, `:544`, `:574` |
| 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` |
Expand Down

82 changes: 82 additions & 0 deletions
packages/plugins/plugin-sharing/src/sharing-service.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1704,3 +1704,85 @@ describe('[#6428] the boolean projection does not drift (compatibility clause)',
 expect(await svc.checkEdit('account', 'a1', { isSystem: true })).toBe('allow');
 });
});

// ─────────────────────────────────────────────────────────────────────

describe('[#13551] the record-share `$in` drops nullish `record_id` rows', () => {
 // The guard standing in front of BOTH `$in` constructions used to read
 // `.map((g) => String(g.record_id)).filter(Boolean)`, which cannot drop a
 // nullish `record_id`: `String(null)` is `'null'`, `String(undefined)` is
 // `'undefined'`, and both are truthy. What follows pins the MECHANISM — a
 // row with no `record_id` contributes no member — and asserts nothing about
 // whether such a row exists in the wild.
 //
 // The rows are seeded straight into the fake table on purpose: `grant()`
 // refuses a nullish `recordId` at the front door, so writing the row
 // directly is the only way to stand up the already-corrupt state the guard
 // exists for — the shape a bad backfill or an out-of-band
 // `sys_record_share` write would leave behind.
 let engine: ReturnType<typeof makeFakeEngine>;
 let svc: SharingService;

 const shareRow = (record_id: unknown) => ({
 id: `shr_${String(record_id)}`,
 object_name: 'account',
 record_id,
 recipient_type: 'user',
 recipient_id: 'alice',
 access_level: 'edit', // in WRITE_ACCESS_LEVELS, so the write filter reads it too
 });

 beforeEach(() => {
 engine = makeFakeEngine({
 account: ACCOUNT_SCHEMA,
 sys_record_share: { name: 'sys_record_share' },
 });
 svc = new SharingService({ engine });
 });

 it('read filter: a null / undefined `record_id` contributes NO member, and the real grant survives', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
 const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
 expect(f.$or[1].id.$in).toEqual(['a1']);
 // Named literally: these are the two members the dead guard used to emit.
 expect(f.$or[1].id.$in).not.toContain('null');
 expect(f.$or[1].id.$in).not.toContain('undefined');
 });

 it('write filter: the same rows, the same outcome — both construction sites are repaired', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
 const f: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
 expect(f.$or[1].id.$in).toEqual(['a1']);
 expect(f.$or[1].id.$in).not.toContain('null');
 expect(f.$or[1].id.$in).not.toContain('undefined');
 });

 it('when EVERY grant is nullish the share branch disappears, on both filters', async () => {
 engine._tables.sys_record_share = [shareRow(null), shareRow(undefined)];
 // Not an `$or` carrying a member that matches nothing: zero usable grants
 // collapses to the owner match, which is what "no grants" already meant.
 expect(await svc.buildReadFilter('account', { userId: 'alice' }))
 .toEqual({ owner_id: 'alice' });
 expect(await svc.buildWriteFilter('account', { userId: 'alice' }, 'update'))
 .toEqual({ owner_id: 'alice' });
 });

 it('over-denial control: an ordinary grant set still produces exactly its ids, on both filters', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow('a2'), shareRow('a3')];
 const read: any = await svc.buildReadFilter('account', { userId: 'alice' });
 const write: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
 expect(read.$or[0]).toEqual({ owner_id: 'alice' });
 expect(write.$or[0]).toEqual({ owner_id: 'alice' });
 expect(read.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
 expect(write.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
 });

 it('the non-null path is unchanged: a driver-numeric id still stringifies, an empty string is still dropped', async () => {
 engine._tables.sys_record_share = [shareRow(42), shareRow(''), shareRow('a1')];
 const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
 // `42` becomes `'42'` exactly as `String()` always made it, and `''` — the
 // one value the old `.filter(Boolean)` could actually drop — is still
 // dropped. Only the nullish rows are newly excluded.
 expect(f.$or[1].id.$in).toEqual(['42', 'a1']);
 });
});

41 changes: 35 additions & 6 deletions
packages/plugins/plugin-sharing/src/sharing-service.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -143,6 +143,39 @@ function hasOwnerField(schema: any): boolean {
 return Boolean(schema?.fields && OWNER_FIELD in schema.fields);
}

/**
 * The `record_id` column of a `sys_record_share` read, as the members of a
 * security `$in`. A row whose `record_id` is nullish or empty contributes
 * NOTHING — the nullish test runs on the RAW column value, BEFORE `String()`.
 *
 * The order is the whole point. The previous spelling coerced first and
 * filtered after — `grants.map((g) => String(g.record_id)).filter(Boolean)` —
 * which cannot drop a nullish `record_id` at all: `String(null)` is `'null'`
 * and `String(undefined)` is `'undefined'`, and both are truthy. The only
 * value it could drop was the empty string, so the guard was dead for exactly
 * the case its spelling advertised, and a corrupt row put the literal string
 * `'null'` into `{ id: { $in: [...] } }`. Both call sites are positive
 * polarity (an OR-ed branch, never negated) and no real record id matches that
 * member, so the effect was a silently DROPPED grant rather than a widened
 * scope — but an audit asking which security paths already handle nullish ids
 * would have counted these two as covered when they provably were not.
 *
 * `String()` is kept for the surviving values: a driver may hand back a
 * numeric primary key, and the members must compare against the string ids the
 * rest of the filter is built from. Every non-nullish value therefore
 * stringifies exactly as it did before, and the trailing `!== ''` drops
 * precisely what `filter(Boolean)` used to drop — so the non-null path is
 * unchanged and only the nullish rows are newly excluded.
 */
function grantedRecordIds(grants: unknown): string[] {
 if (!Array.isArray(grants)) return [];
 return grants
 .map((g: any) => g?.record_id)
 .filter((recordId: unknown) => recordId != null)
 .map((recordId: unknown) => String(recordId))
 .filter((recordId: string) => recordId !== '');
}

/**
 * [#8418] The one WARN line a write gate emits when it refuses because the
 * ownership fast-path was defeated by a FEDERATED object's phantom `owner_id`
Expand DownExpand Up@@ -416,9 +449,7 @@ export class SharingService implements ISharingService {
 context: SYSTEM_CTX,
 });

 const grantedIds: string[] = Array.isArray(grants)
 ? grants.map((g: any) => String(g.record_id)).filter(Boolean)
 : [];
 const grantedIds: string[] = grantedRecordIds(grants);

 if (grantedIds.length === 0) {
 return ownerMatch;
Expand DownExpand Up@@ -494,9 +525,7 @@ export class SharingService implements ISharingService {
 limit: 5000,
 context: SYSTEM_CTX,
 });
 const grantedIds: string[] = Array.isArray(grants)
 ? grants.map((g: any) => String(g.record_id)).filter(Boolean)
 : [];
 const grantedIds: string[] = grantedRecordIds(grants);

 if (grantedIds.length === 0) return ownerMatch;
 return { $or: [ownerMatch, { id: { $in: grantedIds } }] };
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
})();
fix(plugin-sharing): guard the record-share `$in` against a nullish `record_id` before `String()` coerces it by os-steve · Pull Request #13590 · objectstack-ai/objectstack · GitHub
Skip to content
fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it#13590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

os-steve
merged 2 commits into

from

Aug 31, 2026
 Merged


fix(plugin-sharing): guard the record-share $in against a nullish record_id before String() coerces it
#13590
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
a72fcf3

fix(plugin-sharing): guard the record-share $in against nullish recor…


claude Aug 31, 2026
374de16

docs(permissions): re-anchor the system-context census rows onto thei…


claude Aug 31, 2026

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


45 changes: 45 additions & 0 deletions
.changeset/sharing-granted-ids-nullish-guard.md

View file


Open in desktop

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
---
"@objectstack/plugin-sharing": patch
---

fix(plugin-sharing): the record-share `$in` guard now tests `record_id` before `String()` coerces it (#13551)

`buildReadFilter` and the bulk-write half of `buildWriteFilter` each turned the
`sys_record_share` rows granted to the caller into the members of a security
predicate, `{ id: { $in: [...] } }`, with the same expression:

```ts
grants.map((g: any) => String(g.record_id)).filter(Boolean)
```

`.filter(Boolean)` reads as "drop rows whose `record_id` is nullish". It cannot:
`String(null)` is `'null'` and `String(undefined)` is `'undefined'`, and both are
truthy. The only value that spelling could drop was the empty string, so the
guard was dead for exactly the case its spelling advertised, and a
`sys_record_share` row with a nullish `record_id` put the literal string
`'null'` into the emitted `$in`.

**Direction — this was not an open bypass, and the repair is not a bypass fix.**
The emitted member is a bogus id that matches no row on any backend, and both
sites are positive polarity (an OR-ed branch beside the owner match, never
negated), so a corrupt row lost its grant rather than widening anyone's scope.
It also took an already-corrupt row to reach at all. What was actually broken is
the guard's honesty: a reader — or an audit asking which security paths already
handle nullish ids — would have counted these two sites as covered when they
provably were not.

Both sites now share one module-private helper that tests the raw column value
first and coerces after, the shape the sibling id-list guards already use
(`plugin-sharing`'s own `sharing-rule-service.ts` and `primary-bu-projection.ts`,
`core`'s `resolve-authz-context.ts`, `plugin-security`'s controlled-by-parent
`masterIds`, `objectql`'s master-detail parent resolution). Factoring it into one
helper is deliberate: the expression stood in two places, and repairing one would
have left the other advertising a guarantee it does not keep.

The non-null path is unchanged. Every non-nullish value still stringifies exactly
as it did — a driver-numeric primary key still becomes its decimal string — and
the empty string, the one value the old spelling really did drop, is still
dropped. The only behavioural difference is that rows with a nullish `record_id`
now contribute no member at all; when they were the *only* grants, the filter
collapses to the plain owner match instead of OR-ing in a branch that matched
nothing.

10 changes: 5 additions & 5 deletions
content/docs/permissions/system-context.mdx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -129,11 +129,11 @@ The largest single consumer — **20 of the 109 sites**.
| # | Behaviour when `isSystem` | What you get / what you lose | Anchor |
|:--|:---|:---|:---|
| 30 | **Sharing-rule grant materialisation is skipped on all four record-write hooks** | Lose: **no `sys_record_share` rows are created**. A fully configured sharing rule grants **nothing** on seeded data until a rule is re-evaluated or the boot backfill runs. This is the behaviour that motivated #4707. Since #6783 the skip is no longer silent — it emits an INFO notice (rough edge 2) | `rule-hooks.ts:250`, `:274`, `:293`, `:322` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:625` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:891`, `:978`, `:1568` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1179` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1257` (guard at `:1282`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1309` |
| 31 | Sharing write verdict short-circuits to `allow` | Get: writes pass the sharing gate unconditionally | `plugin-sharing/src/sharing-service.ts:654` |
| 32 | Record visibility / manage-shares checks return true | Get: no ownership or Modify-All requirement | `plugin-sharing/src/sharing-service.ts:920`, `:1007`, `:1597` |
| 33 | `grant()` skips the enforcement + manage-shares assertions | Get: the rule evaluator can materialise through the public API. Note it is **not** a bare skip: the system branch asserts the grant is not *inert* instead (a grant on an object no verdict can consult is refused) | `plugin-sharing/src/sharing-service.ts:1208` |
| 34 | `revoke()` deletes directly, **before** the non-manual-source guard | Get: the evaluator can revoke its own grants. Lose: the `CONFLICT` guard that warns a rule-materialised grant will be silently re-granted on the next reconcile | `plugin-sharing/src/sharing-service.ts:1286` (guard at `:1311`) |
| 35 | `listShares()` skips the management gate | Get: full enumeration of who can see a record | `plugin-sharing/src/sharing-service.ts:1338` |
| 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1077` |
| 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link creation/resolution while the policy is off | `plugin-sharing/src/share-link-service.ts:413`, `:467`, `:471`, `:544`, `:574` |
| 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` |
Expand Down

82 changes: 82 additions & 0 deletions
packages/plugins/plugin-sharing/src/sharing-service.test.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1704,3 +1704,85 @@ describe('[#6428] the boolean projection does not drift (compatibility clause)',
 expect(await svc.checkEdit('account', 'a1', { isSystem: true })).toBe('allow');
 });
});

// ─────────────────────────────────────────────────────────────────────

describe('[#13551] the record-share `$in` drops nullish `record_id` rows', () => {
 // The guard standing in front of BOTH `$in` constructions used to read
 // `.map((g) => String(g.record_id)).filter(Boolean)`, which cannot drop a
 // nullish `record_id`: `String(null)` is `'null'`, `String(undefined)` is
 // `'undefined'`, and both are truthy. What follows pins the MECHANISM — a
 // row with no `record_id` contributes no member — and asserts nothing about
 // whether such a row exists in the wild.
 //
 // The rows are seeded straight into the fake table on purpose: `grant()`
 // refuses a nullish `recordId` at the front door, so writing the row
 // directly is the only way to stand up the already-corrupt state the guard
 // exists for — the shape a bad backfill or an out-of-band
 // `sys_record_share` write would leave behind.
 let engine: ReturnType<typeof makeFakeEngine>;
 let svc: SharingService;

 const shareRow = (record_id: unknown) => ({
 id: `shr_${String(record_id)}`,
 object_name: 'account',
 record_id,
 recipient_type: 'user',
 recipient_id: 'alice',
 access_level: 'edit', // in WRITE_ACCESS_LEVELS, so the write filter reads it too
 });

 beforeEach(() => {
 engine = makeFakeEngine({
 account: ACCOUNT_SCHEMA,
 sys_record_share: { name: 'sys_record_share' },
 });
 svc = new SharingService({ engine });
 });

 it('read filter: a null / undefined `record_id` contributes NO member, and the real grant survives', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
 const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
 expect(f.$or[1].id.$in).toEqual(['a1']);
 // Named literally: these are the two members the dead guard used to emit.
 expect(f.$or[1].id.$in).not.toContain('null');
 expect(f.$or[1].id.$in).not.toContain('undefined');
 });

 it('write filter: the same rows, the same outcome — both construction sites are repaired', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow(null), shareRow(undefined)];
 const f: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
 expect(f.$or[1].id.$in).toEqual(['a1']);
 expect(f.$or[1].id.$in).not.toContain('null');
 expect(f.$or[1].id.$in).not.toContain('undefined');
 });

 it('when EVERY grant is nullish the share branch disappears, on both filters', async () => {
 engine._tables.sys_record_share = [shareRow(null), shareRow(undefined)];
 // Not an `$or` carrying a member that matches nothing: zero usable grants
 // collapses to the owner match, which is what "no grants" already meant.
 expect(await svc.buildReadFilter('account', { userId: 'alice' }))
 .toEqual({ owner_id: 'alice' });
 expect(await svc.buildWriteFilter('account', { userId: 'alice' }, 'update'))
 .toEqual({ owner_id: 'alice' });
 });

 it('over-denial control: an ordinary grant set still produces exactly its ids, on both filters', async () => {
 engine._tables.sys_record_share = [shareRow('a1'), shareRow('a2'), shareRow('a3')];
 const read: any = await svc.buildReadFilter('account', { userId: 'alice' });
 const write: any = await svc.buildWriteFilter('account', { userId: 'alice' }, 'update');
 expect(read.$or[0]).toEqual({ owner_id: 'alice' });
 expect(write.$or[0]).toEqual({ owner_id: 'alice' });
 expect(read.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
 expect(write.$or[1].id.$in).toEqual(['a1', 'a2', 'a3']);
 });

 it('the non-null path is unchanged: a driver-numeric id still stringifies, an empty string is still dropped', async () => {
 engine._tables.sys_record_share = [shareRow(42), shareRow(''), shareRow('a1')];
 const f: any = await svc.buildReadFilter('account', { userId: 'alice' });
 // `42` becomes `'42'` exactly as `String()` always made it, and `''` — the
 // one value the old `.filter(Boolean)` could actually drop — is still
 // dropped. Only the nullish rows are newly excluded.
 expect(f.$or[1].id.$in).toEqual(['42', 'a1']);
 });
});

41 changes: 35 additions & 6 deletions
packages/plugins/plugin-sharing/src/sharing-service.ts

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -143,6 +143,39 @@ function hasOwnerField(schema: any): boolean {
 return Boolean(schema?.fields && OWNER_FIELD in schema.fields);
}

/**
 * The `record_id` column of a `sys_record_share` read, as the members of a
 * security `$in`. A row whose `record_id` is nullish or empty contributes
 * NOTHING — the nullish test runs on the RAW column value, BEFORE `String()`.
 *
 * The order is the whole point. The previous spelling coerced first and
 * filtered after — `grants.map((g) => String(g.record_id)).filter(Boolean)` —
 * which cannot drop a nullish `record_id` at all: `String(null)` is `'null'`
 * and `String(undefined)` is `'undefined'`, and both are truthy. The only
 * value it could drop was the empty string, so the guard was dead for exactly
 * the case its spelling advertised, and a corrupt row put the literal string
 * `'null'` into `{ id: { $in: [...] } }`. Both call sites are positive
 * polarity (an OR-ed branch, never negated) and no real record id matches that
 * member, so the effect was a silently DROPPED grant rather than a widened
 * scope — but an audit asking which security paths already handle nullish ids
 * would have counted these two as covered when they provably were not.
 *
 * `String()` is kept for the surviving values: a driver may hand back a
 * numeric primary key, and the members must compare against the string ids the
 * rest of the filter is built from. Every non-nullish value therefore
 * stringifies exactly as it did before, and the trailing `!== ''` drops
 * precisely what `filter(Boolean)` used to drop — so the non-null path is
 * unchanged and only the nullish rows are newly excluded.
 */
function grantedRecordIds(grants: unknown): string[] {
 if (!Array.isArray(grants)) return [];
 return grants
 .map((g: any) => g?.record_id)
 .filter((recordId: unknown) => recordId != null)
 .map((recordId: unknown) => String(recordId))
 .filter((recordId: string) => recordId !== '');
}

/**
 * [#8418] The one WARN line a write gate emits when it refuses because the
 * ownership fast-path was defeated by a FEDERATED object's phantom `owner_id`
Expand DownExpand Up@@ -416,9 +449,7 @@ export class SharingService implements ISharingService {
 context: SYSTEM_CTX,
 });

 const grantedIds: string[] = Array.isArray(grants)
 ? grants.map((g: any) => String(g.record_id)).filter(Boolean)
 : [];
 const grantedIds: string[] = grantedRecordIds(grants);

 if (grantedIds.length === 0) {
 return ownerMatch;
Expand DownExpand Up@@ -494,9 +525,7 @@ export class SharingService implements ISharingService {
 limit: 5000,
 context: SYSTEM_CTX,
 });
 const grantedIds: string[] = Array.isArray(grants)
 ? grants.map((g: any) => String(g.record_id)).filter(Boolean)
 : [];
 const grantedIds: string[] = grantedRecordIds(grants);

 if (grantedIds.length === 0) return ownerMatch;
 return { $or: [ownerMatch, { id: { $in: grantedIds } }] };
Expand Down
Loading