Skip to content

Commit 736390e

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-11757-retire-scim-provider-c
2 parents 222b208 + b1a987e commit 736390e

22 files changed

Lines changed: 2609 additions & 174 deletions
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@objectstack/plugin-security": minor
3+
---
4+
5+
feat(security): the referential FK-clear write is exempt from the object-level CRUD check (#12597)
6+
7+
**This changes which deletes succeed** — an observable behavioural contract
8+
change on the delete path, which is why it ships `minor` rather than as a
9+
patch-grade defect repair.
10+
11+
Deleting a record makes the engine clear every optional lookup that points at it
12+
(`deleteBehavior: 'set_null'`). That cleanup `UPDATE` is engine-owned referential
13+
integrity, and it has carried the server-derived `__referentialFieldClear` marker
14+
since #3023 — but the marker reached only the ownership-anchor guard, so the
15+
write still had to pass the **object-level CRUD check** on the referencing
16+
object. Consequence, measured on a real deployment across 17 role×object pairs: a
17+
role with full delete rights on A and no grant at all on B could delete an A only
18+
while B was **empty**. The moment a real row referenced it, the delete failed with
19+
one generic "you do not have permission", and nothing on any permission screen
20+
showed that deleting A also required write authority on B.
21+
22+
**What is exempt: the object-level CRUD grant check, and nothing else.** A marked
23+
`update` skips that one gate (both the caller's grant and the ADR-0090 D10
24+
delegator half of the same question). Everything else in the security middleware
25+
runs unchanged and is pinned test-by-test:
26+
27+
- field-level security on the FK column still refuses;
28+
- the RLS `using` row scope on the referencing object still refuses;
29+
- the RLS post-image `check` still refuses — so a deployment declaring
30+
`product != null` keeps getting a truthful refusal instead of a silent clear;
31+
- declared validation rules keep firing (they were never in this path);
32+
- a caller without delete rights on the target is still refused;
33+
- an ordinary, unmarked update on the referencing object is untouched.
34+
35+
⛔ Deliberately **not** `isSystem`: that bypass is total (see
36+
`content/docs/permissions/system-context.mdx` — "Elevation is total, and it is not
37+
granular"), and it would have switched off all three guards above. ⛔ The
38+
`cascade` arm — deleting whole referencing rows — is **unchanged** and still
39+
requires the caller's own delete authority on those rows.
40+
41+
The write is not elevated at all, so audit attribution is unchanged: the cleanup
42+
`UPDATE` still runs under the operator's identity and lands in the ledger as that
43+
operator (`user_id` / `actor`, and the `updated_by` stamp).
44+
45+
No authorable surface changes, and no metadata needs migrating: a deployment that
46+
was working around this by granting write access on referencing tables can narrow
47+
those grants, but nothing forces it to.
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
---
2+
"@objectstack/service-storage": minor
3+
---
4+
5+
fix(service-storage): stamp `sys_file` with the acting organization, and backfill the rows that were never stamped (#12745)
6+
7+
`sys_file` is a tenancy-ENABLED object — it declares no `tenancy` key, so
8+
`isTenancyDisabled()` reads `false` and `applySystemFields` provisions
9+
`organization_id` on it. Nothing ever wrote that column:
10+
`StorageMetadataStore.createFile` inserted with **no execution context at all**
11+
(the string `context` appeared 0 times in `metadata-store.ts`), so the SQL
12+
driver's `injectTenantOnInsert` had no `tenantId` to stamp from and every row
13+
landed NULL. Both callers already held the session — `storage-routes.ts` reads
14+
`owner_id: session?.userId` ten lines below each `createFile` — so the
15+
organization was in hand and simply had nowhere in the signature to go.
16+
17+
Maintainer ruling 2026-08-28 on #12745: **A with backfill** — stamp forward AND
18+
repair the existing rows. Both halves ship here.
19+
20+
**Forward stamping.** `createFile(rec, context?)` takes a new optional
21+
`StorageWriteContext` (`{ organizationId }`) and passes it to the engine as
22+
`{ context: { tenantId } }`. The column is deliberately NOT written onto the
23+
payload: whether the object has a tenant column, and whether an explicit value
24+
on the row wins, are the driver's answers (`injectTenantOnInsert`
25+
`resolveTenantField`), and a metadata store re-deciding them one package away
26+
from the schema is how a stamp starts failing on installs that opted the object
27+
out. Both upload doors thread the session's active organization, and the
28+
plugin's session bridge now reports it (`session.session.activeOrganizationId`,
29+
the platform's existing spelling). ⛔ No membership fallback: here the value
30+
becomes a *wall*, and a file stamped from a guessed membership is a file its
31+
uploader can no longer see from the organization they were acting in. A session
32+
with no active organization stamps nothing, exactly as before.
33+
34+
**Why the backfill is not optional.** The SQL driver's tenant predicate is
35+
NULL-tolerant (`organization_id = :tenant OR organization_id IS NULL`), but
36+
Layer 0 AND-composes a strict `organization_id = <active org>` above it and
37+
"the conjunction is the strict equality alone". Forward-only stamping would
38+
therefore split the table: new files org-walled, every existing NULL-org file
39+
invisible to **every** principal. (`single` posture is inert —
40+
`computeTenantLayer0Filter` returns `null` — so single-tenant installs are
41+
unaffected either way.)
42+
43+
**The backfill.** A one-off, idempotent, dry-run-first sweep
44+
(`backfill-sys-file-organizations.ts`), following the tree's own precedent in
45+
`plugin-approvals`. It derives each row's organization from the file's HOLDERS —
46+
the exclusive field reference (`ref_object`/`ref_id`) and every `sys_attachment`
47+
join row — and stamps **only where they all answered and all answered the same
48+
organization**. ⛔ Rows that cannot be derived unambiguously stay NULL and are
49+
REPORTED, never guessed, with the residual-NULL count and its per-reason
50+
breakdown on the report and in the rendered text — for the dry run as well as
51+
the applied run. It is a one-off operational module: not exported from the
52+
package index and not shipped in `dist`.
53+
54+
⛔ Scope: `sys_file` only. The precedent requires a maintainer order per table
55+
and the ruling is that order for this one table — `sys_upload_session` sits in
56+
the same package with the same NULL column and is deliberately not swept.
57+
58+
**Compatibility.** `createFile`'s new parameter is optional and
59+
`StorageRoutesOptions.resolveSession` only WIDENS its return type
60+
(`{ userId? }``{ userId?, organizationId? }`), so existing resolvers and
61+
callers keep compiling and keep their current behaviour.

.github/workflows/lint.yml

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,6 +1037,40 @@ jobs:
10371037
- name: PM ci-failure self-test
10381038
run: node scripts/pm/ci-failure.mjs --self-test
10391039

1040+
# os-regen-merge self-test (#12893). Mirrors the `Verify-lock entry-point
1041+
# self-test` step above 1:1 — same directory, same split between the tool
1042+
# and its self-test. `scripts/pm/os-regen-merge.sh` is the mechanized merge
1043+
# sequence for a branch touching os-regen-driven generated artifacts, and it
1044+
# has NO CI path at all: it is run BY HAND inside a feature branch's
1045+
# worktree, on a merge that exists only there. Its header says so, and that
1046+
# is deliberate.
1047+
#
1048+
# The SELF-TEST is a different animal from the script it tests. It needs no
1049+
# worktree, no remote and no merge — only `git` and a temp dir — and it
1050+
# builds five small fixture repos to pin 23 cases: the PER-FILE merge-side
1051+
# selection and its per-path notices, the staged-diff sentence, the
1052+
# uncommitted-hand-off refusal, and the four pre-existing refusals. A few
1053+
# seconds, no network.
1054+
#
1055+
# Left unwired it rots the way this repo has already recorded and fixed
1056+
# twice (#11514, #6008): the next refactor of the script reds nothing. Two
1057+
# of the 23 cases are SOURCE SCANS of the script's own text — they assert
1058+
# step 2 keeps the non-staging `git restore --source` spelling and never the
1059+
# staging `git checkout` one — which is to say they are precisely the rows a
1060+
# future edit invalidates silently and precisely the rows no reviewer reads.
1061+
# Nothing but this step is an instrument for them.
1062+
#
1063+
# Unconditional and un-`if:`-ed, like every self-test around it — an
1064+
# exemption is precisely what a self-test must not have, or the gap simply
1065+
# moves. One `--self-test` per `run:` block, deliberately: the masking shape
1066+
# `check-step-collectors.mjs` guards is a block driving TWO OR MORE distinct
1067+
# scripts. A discovery collector over `scripts/pm/*.sh --self-test` — which
1068+
# would also catch the next such script arriving unwired — is ruled out of
1069+
# this card: two literal steps do not yet justify the machinery, and a third
1070+
# is when to revisit it.
1071+
- name: os-regen-merge self-test
1072+
run: bash scripts/pm/os-regen-merge.sh --self-test
1073+
10401074
# Claude hook guard self-tests (#11514, objectstack half of
10411075
# objectstack-ai/objectui#5754). `.claude/hooks/` holds the enforcement
10421076
# behind the two rules whose violation is most expensive in this repo —

examples/app-showcase/test/inert-wirings.test.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@
22

33
import { readdirSync, readFileSync } from 'node:fs';
44
import { describe, it, expect } from 'vitest';
5+
// The repo's ONE answer to "is this span a comment, or code?". The naive block
6+
// regex this replaces had no idea what a string literal is: it opened a phantom
7+
// comment at a block-comment opener sitting INSIDE a string and ran to the next
8+
// terminator far below, deleting live code on 5 of this app's 91 sources.
9+
// `stripComments` (not `maskComments`) is the projection this file wants -- the
10+
// one guard below reports bare file paths, never a line or an offset. The
11+
// `.mjs` specifier is deliberate; `scripts/js-comment-mask.d.mts` beside it is a
12+
// hand-written declaration, so this import needs no `allowJs`.
13+
import { stripComments } from '../../../scripts/js-comment-mask.mjs';
514
import stack from '../objectstack.config.js';
615
import { PLATFORM_CAPABILITY_NAMES } from '@objectstack/spec/security';
716
import { FILE_REFERENCE_TYPES, valueSchemaFor } from '@objectstack/spec/data';
@@ -44,15 +53,10 @@ function sourceFiles(dir: string = SRC_ROOT): string[] {
4453
* Source text with comments removed, so a source-scan guard judges CODE.
4554
* Documentation must stay free to name a retired key (this file's own comments
4655
* do, and so do the ones explaining the rename) without tripping the guard that
47-
* bans authoring it. Block comments go first; then whole-line `//` comments —
48-
* never a trailing `//`, which would eat the `//` in a URL inside a string.
56+
* bans authoring it.
4957
*/
5058
function codeOf(file: string): string {
51-
return readFileSync(file, 'utf8')
52-
.replace(/\/\*[\s\S]*?\*\//g, '')
53-
.split('\n')
54-
.filter((line: string) => !line.trimStart().startsWith('//'))
55-
.join('\n');
59+
return stripComments(readFileSync(file, 'utf8'));
5660
}
5761

5862
/** Every `functions` entry, whichever spelling it was authored in. */

packages/cli/src/utils/console-route-ledger.conformance.test.ts

Lines changed: 30 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,19 @@ import { readdirSync, readFileSync, statSync } from 'node:fs';
2424
import { dirname, join, relative, sep } from 'node:path';
2525
import { fileURLToPath } from 'node:url';
2626
import { describe, it, expect } from 'vitest';
27+
// The repo's ONE answer to "is this span a comment, or code?" — its header
28+
// carries the two private-stripper families that drifted apart and the
29+
// parser-differential sweep that measured which way each fails. The private
30+
// scanner this replaces was string-aware but REGEX-BLIND: the doubled slash
31+
// closing `/^https?:\/\//i` read as a line-comment opener and took the rest of
32+
// the line with it, which is the same defect #12398 found live in two sibling
33+
// guards. `stripComments` (not `maskComments`) is the projection this file
34+
// wants: it deletes comment characters but keeps every newline, so the
35+
// `file:line` every finding here reports still points at the real line, and
36+
// nothing in this file reports an offset. The `.mjs` specifier is deliberate;
37+
// `scripts/js-comment-mask.d.mts` beside it is a hand-written declaration, so
38+
// this import needs no `allowJs`.
39+
import { stripComments } from '../../../../scripts/js-comment-mask.mjs';
2740
import { CONSOLE_ROUTE_LEDGER } from './console-route-ledger.js';
2841

2942
/**
@@ -59,49 +72,6 @@ const NON_ROUTE_MEMBERS = new Set(['use', 'notFound', 'onError', 'fire', 'fetch'
5972
// Scanning machinery
6073
// ---------------------------------------------------------------------------
6174

62-
/**
63-
* Strip comments before scanning, PRESERVING newlines inside block comments so
64-
* every finding's `file:line` points at the real line — `console.ts` opens with
65-
* a 35-line header, and reporting a mount 35 lines short makes an accurate
66-
* finding read as a wrong one.
67-
*/
68-
export function stripComments(source: string): string {
69-
let out = '';
70-
let i = 0;
71-
while (i < source.length) {
72-
const c = source[i];
73-
const next = source[i + 1];
74-
if (c === '/' && next === '/') {
75-
while (i < source.length && source[i] !== '\n') i++;
76-
continue;
77-
}
78-
if (c === '/' && next === '*') {
79-
i += 2;
80-
while (i < source.length && !(source[i] === '*' && source[i + 1] === '/')) {
81-
if (source[i] === '\n') out += '\n';
82-
i++;
83-
}
84-
i += 2;
85-
continue;
86-
}
87-
if (c === '\'' || c === '"' || c === '`') {
88-
const quote = c;
89-
out += c;
90-
i++;
91-
while (i < source.length) {
92-
if (source[i] === '\\') { out += source.slice(i, i + 2); i += 2; continue; }
93-
out += source[i];
94-
if (source[i] === quote) { i++; break; }
95-
i++;
96-
}
97-
continue;
98-
}
99-
out += c;
100-
i++;
101-
}
102-
return out;
103-
}
104-
10575
/** Module-scope `const NAME = '<literal>';` bindings, exported or not. */
10676
export function constantBindings(code: string): Map<string, string> {
10777
const out = new Map<string, string>();
@@ -380,13 +350,29 @@ describe('cli console route ledger hygiene', () => {
380350
});
381351

382352
describe('scan machinery, pinned in both directions', () => {
383-
it('the comment stripper drops prose paths, keeps code paths, and preserves line numbers', () => {
353+
it('the shared stripper drops prose paths, keeps code paths, and preserves line numbers', () => {
354+
// Not a re-pin of `js-comment-mask.mjs` -- that module pins its own
355+
// behaviour. This pins the PROPERTY this census rests on: comment
356+
// characters go, every newline stays, so `lineOf()` below still counts
357+
// the real line.
384358
const stripped = stripComments("// app.get('/ghost', h)\n/* a\nb */\napp.get(`/real`, h);\n");
385359
expect(stripped).not.toContain('ghost');
386360
expect(stripped).toContain('/real');
387361
expect(censusOf(['f.ts'], () => "/* a\nb\nc */\napp.get('/x', h);\n").routes[0].line).toBe(4);
388362
});
389363

364+
it('a doubled slash inside a REGEX LITERAL does not swallow the rest of its line', () => {
365+
// The defect the private scanner this file used to carry was measured
366+
// committing on 7 of this package's 110 sources: string-aware but
367+
// regex-blind, it read the `//` that CLOSES `/^https?:\/\//i` as a
368+
// line-comment opener and deleted to end of line. A mount sharing that
369+
// line went with it, and the census reported clean over text it never
370+
// read. Live in `commands/dev.ts`, `commands/serve.ts` and
371+
// `commands/start.ts` at conversion time.
372+
const stripped = stripComments("const ok = /^https?:\\/\\//i.test(u); app.get('/real', h);\n");
373+
expect(stripped).toContain('/real');
374+
});
375+
390376
it('resolves the spellings this package uses, and refuses the rest', () => {
391377
const b = constantBindings("export const CONSOLE_PATH = '/_console';\n");
392378
expect(b.get('CONSOLE_PATH')).toBe('/_console');

packages/drivers/driver-sql/src/live-dialect-matrix.isolation.test.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,14 @@ import { readFileSync, readdirSync } from 'node:fs';
4343
import { SqlDriver } from '../src/index.js';
4444
import { dirname, join } from 'node:path';
4545
import { fileURLToPath } from 'node:url';
46+
// The repo's ONE answer to "is this span a comment, or code?". The naive block
47+
// regex this replaces had no idea what a string literal is: in
48+
// `logger-receiver-detach.test.ts` a fixture STRING quotes a docblock and the
49+
// sweep ate the string. `stripComments` (not `maskComments`) is the projection
50+
// this file wants -- the guard below reports bare file names, never a line or
51+
// an offset. The `.mjs` specifier is deliberate; `scripts/js-comment-mask.d.mts`
52+
// beside it is a hand-written declaration, so this import needs no `allowJs`.
53+
import { stripComments } from '../../../../scripts/js-comment-mask.mjs';
4654
import {
4755
LIVE_SCHEMA_PREFIX,
4856
MYSQL_CELL,
@@ -127,11 +135,9 @@ describe('live-dialect matrix — per-file schema isolation (#9350)', () => {
127135
});
128136

129137
describe('live-dialect matrix — the cell is the only route to a live server (#9350)', () => {
130-
/** Source with line and block comments removed, so prose about the env var is not a hit. */
138+
/** Source with comments removed, so prose about the env var is not a hit. */
131139
const codeOf = (file: string): string =>
132-
readFileSync(join(SRC_DIR, file), 'utf8')
133-
.replace(/\/\*[\s\S]*?\*\//g, '')
134-
.replace(/^[ \t]*\/\/.*$/gm, '');
140+
stripComments(readFileSync(join(SRC_DIR, file), 'utf8'));
135141

136142
/**
137143
* The needle is ASSEMBLED rather than written as a literal.

packages/plugins/plugin-auth/src/rate-limit-storage-isolation.test.ts

Lines changed: 10 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,16 @@
4242
import { describe, it, expect } from 'vitest';
4343
import { readFileSync, existsSync, readdirSync } from 'node:fs';
4444
import { dirname, join, relative, resolve } from 'node:path';
45+
// The repo's ONE answer to "is this span a comment, or code?". The private
46+
// scanner this replaces tracked the three string forms but was REGEX-BLIND, so
47+
// the doubled slash closing a literal like `/^https?:\/\//i` read as a
48+
// line-comment opener and took the rest of the line -- `auth-manager.ts` in this
49+
// very package was losing that line. `stripComments` (not `maskComments`) is the
50+
// projection this file wants: every finding reports a package-relative FILE PATH
51+
// and a specifier, never a line or an offset into the original. The `.mjs`
52+
// specifier is deliberate; `scripts/js-comment-mask.d.mts` beside it is a
53+
// hand-written declaration, so this import needs no `allowJs`.
54+
import { stripComments } from '../../../../scripts/js-comment-mask.mjs';
4555

4656
/**
4757
* Seeded from `__dirname`, not from a `findUp` walk of `process.cwd()`, and not
@@ -109,51 +119,6 @@ const SRC = HERE;
109119
const RUNTIME_SRC = resolve(REPO, 'packages/runtime/src');
110120
const SERVICE_SMS_SRC = resolve(REPO, 'packages/services/service-sms/src');
111121

112-
/**
113-
* Strip comments before scanning. The distinction this file turns on — a
114-
* `import type` versus a value `import` of the same specifier — is invisible to
115-
* a raw-text regex the moment a doc comment quotes an import line, and this
116-
* module's own header quotes several. Handles `//`, block comments and the
117-
* three string forms so a `'http://…'` literal is not mistaken for a comment.
118-
*/
119-
function stripComments(src: string): string {
120-
let out = '';
121-
let i = 0;
122-
while (i < src.length) {
123-
const c = src[i]!;
124-
const next = src[i + 1];
125-
if (c === '/' && next === '/') {
126-
while (i < src.length && src[i] !== '\n') i++;
127-
continue;
128-
}
129-
if (c === '/' && next === '*') {
130-
i += 2;
131-
while (i < src.length && !(src[i] === '*' && src[i + 1] === '/')) i++;
132-
i += 2;
133-
continue;
134-
}
135-
if (c === "'" || c === '"' || c === '`') {
136-
out += c;
137-
i++;
138-
while (i < src.length && src[i] !== c) {
139-
if (src[i] === '\\') {
140-
out += src[i]! + (src[i + 1] ?? '');
141-
i += 2;
142-
continue;
143-
}
144-
out += src[i];
145-
i++;
146-
}
147-
out += c;
148-
i++;
149-
continue;
150-
}
151-
out += c;
152-
i++;
153-
}
154-
return out;
155-
}
156-
157122
interface Ref {
158123
spec: string;
159124
/** `import type … from` / `export type … from` — erased at build, costs nothing at runtime. */

0 commit comments

Comments
 (0)