Skip to content

Commit e74621a

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-7665-by-id-write-visibility-gate
2 parents 23df0a3 + 098b629 commit e74621a

21 files changed

Lines changed: 3132 additions & 1126 deletions
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
---
2+
"@objectstack/platform-objects": patch
3+
"@objectstack/plugin-auth": patch
4+
---
5+
6+
fix(platform-objects,plugin-auth): let the API-key revoke/restore actions actually run (#7727)
7+
8+
`sys_api_key` contradicted itself. It declared two row actions —
9+
`revoke_api_key` / `restore_api_key` — as `PATCH /api/v1/data/sys_api_key/{id}`
10+
with `bodyExtra: { revoked: true|false }`, while the same object set
11+
`enable.apiMethods = ['get', 'list']`. The declared PATCH was refused at the
12+
ADR-0049 method gate with `405 OBJECT_API_METHOD_NOT_ALLOWED` before any
13+
authorization ran, so **no product route revoked an API key**: the Setup →
14+
API Keys → Revoke button produced an error toast, the row still read
15+
`revoked = false`, and the key kept authenticating. A leaked key could only be
16+
retired by writing the row out of band.
17+
18+
Enforcement of the flag was never the problem — the verifier filters
19+
`revoked: false` and re-checks the row, so a flipped bit takes effect on the
20+
very next `x-api-key` call. The missing piece was purely the write path, and it
21+
had **two** gates, not one:
22+
23+
- **The method gate.** `enable.apiMethods` now carries `update`. `create` and
24+
`delete` stay off: minting is `POST /api/v1/keys` (the only path that ever
25+
returns the raw secret) and keys are retired by revoking, not deleting.
26+
- **The affordance reconciler.** ADR-0103's `reconcileManagedApiMethods` strips
27+
any write verb a `managedBy` object's resolved affordances do not grant —
28+
warning, not failing. So `apiMethods` alone would still have served 405 while
29+
the source read correctly. `userActions: { edit: true }` declares the
30+
affordance, exactly as `sys_user` does under ADR-0092 D4.
31+
32+
**Opening the method does not open the columns.** `sys_api_key` stays
33+
`managedBy: 'better-auth'`, so ADR-0092 D2's identity write guard still
34+
fail-closed rejects user-context writes, and its per-object update whitelist
35+
remains the only opening. `revoked` is registered there and nothing else is:
36+
`key` stays unwritable (a rotated hash would mint a credential nobody holds),
37+
`user_id` stays unwritable (re-owning a key is privilege transfer), and
38+
`expires_at` stays on the mint path. A PATCH carrying only non-whitelisted
39+
columns is refused `403 PERMISSION_DENIED` rather than degrading into a
40+
timestamp touch, and a mixed patch applies `revoked` while stripping the rest.
41+
The guard itself is unchanged — no general weakening, and every other identity
42+
table keeps its default-deny.
43+
44+
Per ADR-0092 D4's form-rendering constraint, the columns outside the whitelist
45+
(`name`, `prefix`, `user_id`, `scopes`, `expires_at`) are now `readonly`, so the
46+
edit form this affordance turns on cannot offer a write the server refuses —
47+
the declared-≠-enforced shape that caused the original defect.
48+
49+
Nothing pinned any of this before: the existing tests exercise key *resolution*
50+
against a pre-revoked row and never call the route the actions declare, which is
51+
how a declared action and a method gate cancelled out unnoticed. The new
52+
`api-key-revoke-lifecycle` dogfood suite drives the real PATCH, asserts `200`,
53+
and then asserts the consequence — the key stops authenticating — because a 200
54+
that leaves the key working is the defect wearing a success code.
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
docs(spec,i18n): `GET /i18n/locales` stops declaring `label` a display name
6+
7+
`GetLocalesResponseSchema` described each locale descriptor's `label` as
8+
"Display name of the locale", and no producer has ever written one. The sole
9+
producer is `toLocaleDescriptors` (`system/i18n-resolver.ts`) — deliberately
10+
shared by the runtime dispatcher's `/i18n` domain and `service-i18n`'s
11+
autonomous route, so there is no second implementation to diverge — and it sets
12+
`label` to the code. `GET /api/v1/i18n/locales` answers `{ code: 'th', label:
13+
'th' }`, never `{ code: 'th', label: 'ไทย' }`. Declared not enforced (ADR-0049),
14+
one field wide, and the describe is what carries the claim into the generated
15+
JSON Schema, the OpenAPI surface and the SDK type — so a client that trusts it
16+
renders locale codes at users and only finds out by looking. objectui#4039 hit
17+
exactly that and routed around the field: the console's language menu reads
18+
`code` alone off this body and names locales from its own built-in table plus
19+
`Intl.DisplayNames`.
20+
21+
Patch, and describe-only. The measurement behind that: **no consumer anywhere
22+
reads `label`**. In this repo every read of the body takes `code` or
23+
`isDefault` (`http-dispatcher.test.ts`, `domain-handler-registry.test.ts`,
24+
`i18n-success-envelope.conformance.test.ts`); the one wire fixture that spells
25+
`label` sets it to the code and asserts only the array length. In objectui the
26+
one real consumer, `apps/console/src/loadLocales.ts`, reads `entry?.code` and
27+
documents in its header that the descriptor's label is not a display name. With
28+
nothing consuming the field, the honest declaration is the whole fix: the
29+
runtime behaviour is unchanged, and only the field's documented meaning moves.
30+
31+
So the declaration now states the convention it ships — `label` equals `code`;
32+
naming a locale for a UI is the client's job, where `Intl.DisplayNames` already
33+
lives and where the choice of *which* language to name it in belongs. The two
34+
alternatives are deliberately not taken here: serving real display names is a
35+
capability addition with no measured pull (CLDR data on the server for
36+
something every client can compute), and retiring the field is a heavier
37+
response-contract action. Both stay open on #7634.
38+
39+
`toLocaleDescriptors`' output and the declaration are now pinned against each
40+
other in `i18n-resolver.test.ts`, on both sides — a producer that starts
41+
inventing display names and a describe that starts promising them each turn it
42+
red separately.
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(engine): a granted child write no longer 500s because the parent's roll-up recompute ran as the caller
6+
7+
Creating a child record returned **HTTP 500 after the row had already been
8+
written** whenever the child fed a roll-up `summary` on a parent the caller may
9+
not edit — the ordinary parent/child shape for tasks, line items, comments and
10+
time entries. A client that retried (or a user who clicked Save again) created a
11+
duplicate row
12+
([#7673](https://github.com/objectstack-ai/objectstack/issues/7673),
13+
[#7719](https://github.com/objectstack-ai/objectstack/issues/7719)).
14+
15+
`recomputeSummaries` issued the parent roll-up write under the **caller's**
16+
execution context, so an engine-derived write was authorized as if the caller had
17+
asked for it. On the showcase app a plain member holding `showcase_task: create +
18+
read` hit it on every `POST` and `PATCH`: the recompute of
19+
`showcase_project.task_count` raised `PERMISSION_DENIED`, the engine recorded it
20+
as a recompute failure, and the call site rethrew it as `SummaryRecomputeError`
21+
(`ERR_SUMMARY_RECOMPUTE`) — which REST maps to a 500. The access matrix and
22+
`/security/explain` both said `create: true`, so a declared-and-granted operation
23+
failed on a permission check about a record the caller never asked to touch.
24+
25+
**The recompute now runs system-elevated**, on all three call sites (insert,
26+
update, delete). A roll-up is engine-derived state, not a caller write: the
27+
permission decision that matters — may this caller write the **child** — has
28+
already been made by the time the recompute runs. The elevation is a
29+
`sudo()`-shaped derivative of the caller's context, so an open transaction
30+
handle, `tenantId` and `timezone` still ride along; it is the same posture the
31+
roll-up's two other writers already held (the insert-time seed and the
32+
`summary-nulls` backfill), so all three writers of a summary column now agree
33+
about who owns it.
34+
35+
Two quieter defects go with it, both of which only showed where the caller
36+
*could* write the parent and the recompute therefore "succeeded":
37+
38+
- the aggregate was computed over the caller's **row-level-visible subset**, so
39+
the parent's column was silently rewritten to one reader's view of the child
40+
collection;
41+
- an author-declared `readonly: true` roll-up column was dropped by the
42+
write-path read-only strip (which runs on `!context.isSystem`), so the summary
43+
never landed at all.
44+
45+
This does not widen what a caller may read or write. The parent's row is still
46+
governed by the caller's grants (a direct update of the parent is refused exactly
47+
as before), the summary column stays subject to the parent's field-level security
48+
on read, and the only value this path can move is the one the author declared as
49+
a function of the child collection. `ERR_SUMMARY_RECOMPUTE` is unchanged and
50+
still surfaces genuinely failed recomputes (a driver or network failure that
51+
outlives its retries), which is what the seed loader and import runner branch on.
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
fix(metadata-protocol): uninstall no longer orphans a package's env-wide `sys_metadata` rows (#7705)
6+
7+
`protocol.deletePackage` selected the rows to remove with a strict
8+
`organization_id` equality:
9+
10+
```ts
11+
const where = { package_id: request.packageId };
12+
if (request.organizationId) where.organization_id = request.organizationId;
13+
```
14+
15+
Against rows stored **env-wide** (`organization_id IS NULL`) that predicate
16+
matches nothing, so an uninstall issued by a session with an active
17+
organization removed only whichever rows happened to be org-scoped and left
18+
every env-wide row behind — while reporting a nonzero `deletedCount` and
19+
`success: true` over the survivors. The package's metadata stayed in
20+
`sys_metadata` after its uninstall "succeeded", and a reinstall then collided
21+
with the rows that were never removed.
22+
23+
Env-wide is where a package's metadata normally lands, which is why this was
24+
the common case rather than a corner: the REST `PUT /meta/:type/:name` save
25+
path does not thread the session's active organization, and AI-authored
26+
metadata is written env-wide too. Measured on a real engine over SQLite, an
27+
org-scoped uninstall of a package holding three env-wide rows and one
28+
org-scoped row deleted **1 of 4** and reported success.
29+
30+
An org-scoped uninstall now matches its own organization **or** env-wide, the
31+
same `$or [{organization_id: oid}, {organization_id: null}]` shape this package
32+
already uses for the #3115 "orphaned draft" fix, and the same shape the SQL
33+
driver's own implicit tenant wall uses (`field = :tenant OR field IS NULL`,
34+
#2734).
35+
36+
Scoping is unchanged in both directions that must not widen: another
37+
organization's rows for the same package are still out of scope for an
38+
org-scoped uninstall, and another package's rows are never touched. An
39+
uninstall issued with **no** organization is also unchanged — it stays
40+
package-wide, because the direct-mount REST door passes no organization at all
41+
and narrowing that branch to env-wide-only would orphan every org-scoped row
42+
instead.

0 commit comments

Comments
 (0)