Skip to content

Commit 8a1ebac

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-6881-6762-describe-sweep
2 parents 8e25c89 + 17688fe commit 8a1ebac

25 files changed

Lines changed: 2777 additions & 38 deletions
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
fix(spec): drop the producer-less `batch` row from `DATA_ACTION_TO_API_OPERATION` (#6259)
6+
7+
`DATA_ACTION_TO_API_OPERATION` normalizes the action vocabularies its callers
8+
speak onto canonical `ApiOperation` names. One row, `batch: 'bulk'`, had no
9+
producer on either side, and the table's own TSDoc still taught readers that it
10+
did — calling `batch` a "runtime `callData` action".
11+
12+
Both consumers were re-enumerated at `origin/main` before the row was removed:
13+
14+
- `packages/runtime/src/api-exposure.ts` (`checkApiExposure`) is reached only
15+
from `callData`, which branches on a closed set — `create`/`get`/`update`/
16+
`delete`/`query`/`find`/`aggregate` — and every call site passes one of those
17+
as a string literal. Its `batch` arm was retired in #5856, so no caller has
18+
been able to send the word since.
19+
- `packages/rest/src/rest-server.ts` (`apiAccessDenialFromEnable`) is fed only
20+
canonical literals by `enforceApiAccess`: `import`, `bulk`, `create`,
21+
`update`, `list`, `delete`, `get`, `export`. The cross-object `POST /batch`
22+
route is the trap worth naming — it spells `batch` in the **URL** and gates on
23+
`'bulk'`, so the route is untouched by this change.
24+
25+
FROM → TO: `batch` → `bulk`. If you read this table directly, spell the bulk
26+
surface `bulk`; `DATA_ACTION_TO_API_OPERATION['batch']` is now `undefined`.
27+
28+
**Nothing on a live path changes**, because nothing sent `batch`. What changes
29+
is the answer waiting for anyone who does: the lookup misses, the consumers'
30+
`?? action` pass-through hands `batch` through unmapped, and an unmapped action
31+
is *ungated* by `apiMethods` (it still respects `apiEnabled`) — the same
32+
treatment every custom action gets. That last point is why the row was worth
33+
removing rather than leaving as harmless: while it existed, one unreachable
34+
word silently bought a real `bulk ∧ child` permission verdict, and a reader —
35+
or an AI author — would reasonably conclude `batch` was a supported spelling
36+
and write consumer-side tolerance for it. Prime Directive #12 forbids exactly
37+
that: an alias with no producer belongs at the producer or nowhere.
38+
39+
The export itself is unchanged — same name, same `Record<string, ApiOperation>`
40+
type — so `check:api-surface` records no delta (that snapshot prints type
41+
references, not expanded shapes; #3883 is the precedent for a key-level change
42+
being invisible to it). No authorable metadata key is involved, so there is no
43+
tombstone, no conversion and no liveness-ledger row: nothing parses this table.
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
"@objectstack/rest": minor
3+
---
4+
5+
fix(rest): a repeated `?version=` on `/packages/:id` is refused, not silently resolved (#6307)
6+
7+
`IHttpRequest.query` is declared `Record<string, string | string[]>` — a repeated
8+
query parameter arrives as an **array**. Both `/api/v1/packages/:id` handlers read
9+
it as a string and passed it straight to `PackageService.get/delete`, whose
10+
parameter is `version?: string`. Measured on `main` before the fix:
11+
12+
```
13+
GET /packages/com.acme.crm?version=1.0.0&version=2.0.0
14+
→ packageService.get('com.acme.crm', ['1.0.0','2.0.0'])
15+
DELETE /packages/com.acme.crm?version=1.0.0&version=2.0.0
16+
→ packageService.delete('com.acme.crm', ['1.0.0','2.0.0'])
17+
→ 200 { message: 'Deleted com.acme.crm@1.0.0,2.0.0' }
18+
```
19+
20+
The `DELETE` line is the sharp one. `if (!version && protocol.deletePackage)` is
21+
what gates the **full uninstall** (#2747: the package's metadata rows, the durable
22+
`sys_packages` record, and the registered data-plane cleanups — plugin-security
23+
revoking its permission sets and bindings). Any truthy `version` skips it, so a
24+
repeated parameter silently narrowed the *scope of the operation* on a destructive
25+
verb and still reported success.
26+
27+
**Both verbs now refuse the ambiguity** with `400 VALIDATION_ERROR`
28+
(`The "version" query parameter was supplied 2 times. Supply it at most once — this
29+
endpoint will not choose between conflicting values.`). `?version=a&version=b` is a
30+
well-formed request carrying two conflicting intents; picking one silently is a
31+
wrong answer delivered as a `200`. The rule is identical on both verbs — one
32+
parameter, one answer — and the code comes from ADR-0112's **standard** catalog
33+
rather than a newly registered synonym, because "this request contradicts itself"
34+
is a generic validation condition.
35+
36+
The rule is about **multiplicity, not shape**: the parameter may be supplied at
37+
most once. A one-element array is one occurrence encoded differently by an adapter
38+
and is accepted; an empty array is no occurrence. Two identical values are still
39+
two occurrences and are still refused — "at most one *distinct* value" would be a
40+
de-duplication rule no client can predict, while "supply it at most once" is
41+
checkable client-side.
42+
43+
**Not tolerance for off-spec input.** The contract already declared the array; the
44+
consumer simply never handled a shape it was told to expect.
45+
46+
**Nothing that works today changes.** A single `?version=1.0.0`, no `version` at
47+
all, and an empty `?version=` all behave exactly as before — including the full
48+
uninstall still being reached when no version is supplied. No in-repo caller,
49+
documented example or SDK path repeats the parameter (`client.packages.get` builds
50+
`?version=` from a single `version?: string`), so the new 400 is unreachable from
51+
any supported client. It is `minor` rather than `patch` only because a request
52+
shape that used to answer `200` now answers `400`.
53+
54+
Adapter note, measured over a real socket: the `node:http` adapter
55+
(`NodeHttpServer`) hands `['1.0.0','2.0.0']` to the handler as the contract
56+
declares, while the Hono adapter collapses a repeat to the first value before any
57+
handler sees it. Both are contract-legal (the union permits either), which is
58+
exactly why the consumer must handle the declared shape rather than depend on
59+
which server booted.
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
"@objectstack/plugin-sharing": patch
3+
---
4+
5+
fix(sharing): the by-id write gate defers to an app-authored RLS widener instead of hard-refusing (#5493)
6+
7+
HotCRM's 17.0 GA acceptance sweep declared two RLS update-wideners on one
8+
profile and measured one of them working. On the junction object
9+
`crm_campaign_member` a non-owner PATCH returned 200; on `crm_campaign` the
10+
identical shape returned 403 `FORBIDDEN: insufficient privileges to update
11+
crm_campaign`. That sentence is the sharing middleware's, not the row gate's
12+
`(row-level security)` — so the refusal landed **before** RLS was consulted and
13+
the declared widener was never asked.
14+
15+
The discriminator was never "carries sharing rules". It is whether **record
16+
sharing enforces on the object at all**: `checkEdit` abstains — and `canEdit`
17+
therefore answers `true`, letting the write through to RLS — when the effective
18+
sharing model is `public` (which `controlled_by_parent` maps onto) or when the
19+
schema has no `owner_id` field. A junction lands in that set; an ordinary owned
20+
business object does not. Same declaration, opposite outcome, split by a
21+
property no author writes down.
22+
23+
Row-level write authority is ONE composite determination (maintainer ruling on
24+
#5492), so the middleware no longer ends the decision by itself. Before it
25+
hard-refuses a **by-id** update or delete, it asks the security service's
26+
`ISecurityService.checkAuthoredRowWrite` — the fail-closed verdict landed by
27+
#5493 step 1 (PR #6841) — whether an **app-authored** (non-floor) row-level
28+
policy admits this row for this operation. `admit` retracts this authority's
29+
refusal and hands the row to the security pre-image gate, which composes per
30+
#6684/#5492 and makes the final row decision. It does not authorize anything on
31+
its own.
32+
33+
Everything else is unchanged, deliberately:
34+
35+
- **The guarded surface does not shrink.** A member with no authored policy, no
36+
share and no bypass is still refused; a read-level share still never widens a
37+
write; an `edit`-level share still widens update and not delete (ADR-0111 D3),
38+
and an update-only authored widener does not open delete either — the verb is
39+
threaded through to the verdict, not collapsed.
40+
- **Fail-closed on every non-`admit` outcome.** No security service (a
41+
deployment without `@objectstack/plugin-security`), a service predating the
42+
method, a throwing probe, a principal-less context, an on-behalf-of context
43+
(ADR-0090 D10) and any unrecognised verdict all leave today's refusal
44+
byte-for-byte intact. The probe is reached through the same structural
45+
late-binding this plugin already uses for `hasWriteBypass`; no runtime
46+
dependency on `plugin-security` is introduced.
47+
- **A creator who is no longer the owner gets nothing back.** The platform's own
48+
ownership floor (`created_by == current_user.id`, shipped on the additive
49+
`member_default` baseline) matches a record transferred away from its creator,
50+
so a deferral keyed on "the composed RLS admits this row" would return
51+
transferred records to former creators. The verdict is provenance-aware and
52+
abstains there; the deferral does not widen it.
53+
- **The bulk path is untouched** — it composes a filter rather than a verdict,
54+
and is tracked separately (#6736).
55+
- **Objects with no owner field are untouched** (#6698): sharing abstains, the
56+
gate never refuses, so the deferral is never reached and the platform
57+
`created_by` write floor remains their only row-level write gate.
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
'@objectstack/driver-sql': patch
3+
'@objectstack/driver-sqlite-wasm': patch
4+
'@objectstack/driver-turso': patch
5+
---
6+
7+
drivers: every SQL read door routes through the tenant chokepoint (#6792)
8+
9+
`SqlDriver.applyTenantScope()` owns read-side tenant isolation for the whole SQL family —
10+
the `tenantId` early-out, the "object has no tenant field" early-out, the NULL-org
11+
platform-row rule (#2734) and the ADR-0105 D2 union posture (#3623). Its own docstring
12+
said "every CRUD method routes through it". Nothing ever checked that, and it was false
13+
for as long as it had existed. **Three** read doors built their query through
14+
`getBuilder()` and never arrived:
15+
16+
- **`findWithWindowFunctions()`** — the documented #4286 window door. It returns **rows**,
17+
so on a deployment where the scope would have applied (`options.tenantId` set, object
18+
has a tenant field) it returned rows belonging to **every** tenant. Measured with two
19+
tenants seeded plus one NULL-org platform row: `tenantId: 'org_a'` returned
20+
`[a1, a2, b1, b2, p1]` here against `find()`'s `[a1, a2, p1]` — another tenant's rows,
21+
handed over at the driver layer.
22+
- **`analyzeQuery()` / `explain()`** — returns a **plan**, not rows, so this is a smaller
23+
fix and it is made on its own merits rather than folded into the one above. It is the
24+
same defect #6577 fixed on these two methods one builder line lower: a plan is only
25+
worth reading if it explains the statement `find()` would actually run, and a missing
26+
tenant predicate changes selectivity and therefore which index the planner picks.
27+
Compiled `select * from account` where `find()` sent the `organization_id` clause.
28+
- **`distinct()`** — returns one column's **values** for every tenant. This one was in no
29+
card. #6792 states the opposite, listing `distinct` among the scoped call sites; the
30+
13th read site is `aggregate()`. It was found by measuring the invariant rather than
31+
re-reading it.
32+
33+
All three now call `applyTenantScope()` beside their `getBuilder()` line, the position
34+
`findRows()` uses. They route through the chokepoint rather than re-deriving a predicate:
35+
a local equality would silently drop NULL-org platform rows (#2734) and collapse group
36+
reads to active-org reach (#3623). Both of the chokepoint's early-outs are inherited
37+
unchanged, so an unscoped admin/seed read (no `tenantId`) and any object without a tenant
38+
field behave exactly as before.
39+
40+
**The durable half is a gate, not the three lines.** `pnpm check:tenant-chokepoint`
41+
(`scripts/check-tenant-chokepoint.mjs`, wired into `.github/workflows/lint.yml`) re-derives
42+
the invariant from the AST across the `SqlDriver` family on every run: a method that builds
43+
through `getBuilder(object, options)` must call `applyTenantScope()` on that builder, or
44+
carry a written exemption. Insert builders are exempt structurally — write-side tenancy is
45+
`injectTenantOnInsert` — rather than by a name list. It is keyed on the **builder** and not
46+
on the method signature, because the signature criterion the card sketches ("takes
47+
`(object, …, options)` and returns rows") misses `distinct` (no `query` parameter) and
48+
`analyzeQuery` (returns a plan). Verified red against the pre-fix tree, red against a
49+
newly-added unscoped door, and silent once that door is scoped.
50+
51+
The chokepoint docstring no longer asserts the invariant; it names the gate that proves it.
52+
53+
If you call these doors directly on a multi-tenant deployment, pass `options.tenantId` as
54+
you would to `find()` — that is what now takes effect. Callers that never passed it are
55+
unaffected; that remains the documented unscoped/admin path.

‎.github/workflows/lint.yml‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,44 @@ jobs:
575575
- name: Spec type-alias convention gate (ADR-0122)
576576
run: pnpm check:spec-parsed-alias
577577

578+
# Read-side tenant chokepoint gate (#6792, from #3724 / #6577).
579+
# `SqlDriver.applyTenantScope()` owns read-side tenant isolation for the
580+
# whole SQL family — the tenantId early-out, the no-tenant-field early-out,
581+
# the NULL-org platform-row rule (#2734) and the ADR-0105 D2 union posture
582+
# (#3623). Its own docstring claimed "every CRUD method routes through it".
583+
# Nothing checked that, and it was FALSE for as long as it had existed:
584+
# three doors built through `getBuilder()` and never arrived —
585+
# `findWithWindowFunctions` (ROWS: a caller passing `tenantId` got every
586+
# tenant's rows, measured `[a1,a2,b1,b2,p1]` against `find()`'s
587+
# `[a1,a2,p1]`), `analyzeQuery`/`explain` (a PLAN for a statement `find()`
588+
# would not run — the same defect #6577 fixed on these methods one builder
589+
# line lower), and `distinct` (every tenant's values for one column).
590+
#
591+
# The third is the argument for gating rather than fixing. It was in NO
592+
# card: #6792 asserts the opposite — that `distinct` is among the 13 scoped
593+
# sites — and the triage comment and two rounds of measurement all
594+
# inherited that sentence without re-deriving it. The 13th read site is
595+
# `aggregate()`. Two of the three doors were found by a human reading the
596+
# file for another reason; the third was found only by measuring, which is
597+
# the thing a prose invariant can never do for itself.
598+
#
599+
# Keyed on the BUILDER, not the method signature. #6792 sketches "every
600+
# method taking `(object, …, options)` and returning rows"; that criterion
601+
# is measurably too narrow — `distinct(object, field, filters, options)`
602+
# takes no query and `analyzeQuery` returns a plan, so it misses two of the
603+
# three. `getBuilder()` is the single constructor of every statement this
604+
# driver sends, so every builder is classified and one that cannot be
605+
# classified is an error, never a default (#4690's family). Insert builders
606+
# are exempt structurally, not by name: write-side tenancy is
607+
# `injectTenantOnInsert`.
608+
#
609+
# Static AST over three files, no build needed, so it belongs in this job.
610+
# Runs its own --self-test first, in both directions — the detector can be
611+
# broken while every door is fine, and a scan that stops matching would
612+
# report OK while reading nothing.
613+
- name: Read-side tenant chokepoint gate
614+
run: pnpm check:tenant-chokepoint
615+
578616
typecheck:
579617
name: TypeScript Type Check
580618
runs-on: ubuntu-latest

‎content/docs/data-modeling/queries.mdx‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,17 @@ const ranked = await sqlDriver.findWithWindowFunctions('employee', {
590590
});
591591
```
592592

593+
<Callout type="warn">
594+
**Pass `options.tenantId` on a multi-tenant deployment.** Like `find()`, this door is
595+
tenant-scoped only when the caller supplies it — the example above omits it, so it reads
596+
across every tenant. That is the driver layer's documented contract (seed scripts and
597+
cross-org tooling depend on the unscoped path), but it is a decision to make deliberately.
598+
599+
Until #6792 the door ignored `options.tenantId` even when you *did* pass it and returned
600+
every tenant's rows regardless. It now routes through the driver's `applyTenantScope`
601+
chokepoint like every other read.
602+
</Callout>
603+
593604
For request-level analytics, use `aggregations` + `groupBy`, or model rankings in
594605
report/dashboard metadata.
595606

‎content/docs/protocol/objectql/query-syntax.mdx‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,12 @@ from presentation; Postgres and MySQL hand back their own native temporal value)
826826
then re-deduplicates the presented values, because SQL `DISTINCT` compares the *stored*
827827
form.
828828

829+
It is tenant-scoped on the same terms as `find()` — the `organization_id` predicate is
830+
applied when the call carries `options.tenantId` (the fourth argument), and the example
831+
above omits it, so it returns the column's values across every tenant. Until #6792 the
832+
predicate was dropped even when `tenantId` *was* supplied, which made a scoped call
833+
disclose every other tenant's values for that column.
834+
829835
### Full-Text Search
830836

831837
The `search` parameter does **not** reach a full-text index. The engine expands it into
@@ -959,6 +965,13 @@ projection, and niladic rendering means argument-taking functions (`LAG(field)`)
959965
emit without their argument. For request-level analytics use `aggregations` +
960966
`groupBy` (§5).
961967

968+
Tenancy works exactly as it does on `find()`: the driver applies its
969+
`organization_id` predicate only when the call carries `options.tenantId`. The example
970+
above omits it and therefore reads across every tenant — the intended unscoped/admin
971+
path, but a deliberate choice rather than a default to inherit. (Until #6792 this door
972+
dropped `options.tenantId` even when it was supplied, so a scoped call still returned
973+
every tenant's rows.)
974+
962975
---
963976

964977
## 7. Pagination

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
"check:engine-double-contract": "node scripts/check-engine-double-contract.mjs --self-test && node scripts/check-engine-double-contract.mjs",
8282
"check:resume-authority-declared": "node scripts/check-resume-authority-declared.mjs --self-test && node scripts/check-resume-authority-declared.mjs",
8383
"check:spec-parsed-alias": "node scripts/check-spec-parsed-alias.mjs --self-test && node scripts/check-spec-parsed-alias.mjs",
84+
"check:tenant-chokepoint": "node scripts/check-tenant-chokepoint.mjs --self-test && node scripts/check-tenant-chokepoint.mjs",
8485
"check:stall-guard": "node scripts/run-with-stall-guard.mjs --self-test"
8586
},
8687
"keywords": [

0 commit comments

Comments
 (0)