Skip to content

Commit f31eb52

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-7918-currency-precision-iso4217
2 parents dc27f33 + a0151e9 commit f31eb52

17 files changed

Lines changed: 1749 additions & 31 deletions

‎.changeset/console-6d77acfe3125.md‎

Lines changed: 127 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
"@objectstack/plugin-security": patch
3+
---
4+
5+
fix(plugin-security): a member can revoke their OWN API key — owner-scoped `update` on `sys_api_key` for `member_default` (#8053)
6+
7+
An ordinary member who minted a personal API key could not revoke it.
8+
`PATCH /api/v1/data/sys_api_key/{their own id} {"revoked": true}` answered **403
9+
`PERMISSION_DENIED`**, the row stayed `revoked: false`, and the key kept
10+
authenticating. The `revoke_api_key` / `restore_api_key` row actions rendered in
11+
that member's own **My Keys** grid the whole time — a dead affordance on the
12+
persona the surface is built for.
13+
14+
A personal API key acts as its owner ("treat it like a password", per the
15+
console's own mint screen), and the owner is the person who discovers it leaked.
16+
Their only remedy was to find an admin.
17+
18+
This is the residual of #7727, one layer down. That fix was correct as far as it
19+
went: the method gate opened (`enable.apiMethods` gained `update`) and ADR-0092
20+
D2's column whitelist registered `revoked`. But the **object-CRUD** layer was
21+
untouched — the platform `member_default` set granted only `allowRead` across the
22+
better-auth-managed identity tables, so `update` on `sys_api_key` resolved for
23+
`admin_full_access` and nobody else. `GET /api/v1/security/explain` said so
24+
outright, as that member: *"No resolved permission set grants update on
25+
sys_api_key"*. Because #7727's tests all drove the admin, the member half stayed
26+
hidden behind its fix.
27+
28+
`member_default` now carries an explicit `sys_api_key` entry with `allowEdit`.
29+
Two pre-existing mechanisms bound it, and the grant is deliberately not bounded
30+
by the permission-set boolean alone:
31+
32+
- **which rows** — the `sys_api_key_self` RLS carve-out
33+
(`user_id == current_user.id`), which already made the row owner-*visible*;
34+
there was simply no `allowEdit` to go with it. A member PATCHing another
35+
user's key is still refused **403**, row unchanged.
36+
- **which fields** — ADR-0092 D2's identity write guard, whose per-object update
37+
whitelist for this table lists `revoked` alone. `key` stays unwritable (a
38+
rotated hash would mint a credential nobody holds) and `user_id` stays
39+
unwritable (re-owning a key is privilege transfer) — both are stripped even
40+
when smuggled alongside a legal `revoked`.
41+
42+
**Unaffected, and pinned as such:** cross-owner revocation stays 403; a
43+
non-`revoked` column stays refused for the owner too; `create` / `delete` stay
44+
**405** at the method gate (minting remains `POST /api/v1/keys`, the only path
45+
that returns the raw secret once, and rows retire by revoking, not deleting);
46+
show-once semantics are intact. Every other better-auth-managed identity table
47+
stays write-denied — `sys_api_key` is the one exception, and it is one because
48+
that table is hand-rolled ObjectStack rather than better-auth-owned, with a
49+
registered whitelist already governing its single platform-owned column.
50+
51+
The regression pin runs as the key's **owner**, not as an admin — the persona
52+
gap that let this survive #7727's own test suite.
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
'@objectstack/rest': patch
3+
---
4+
5+
`GET /api/v1/meta/app/<name>`: report a permission denial instead of absence
6+
7+
An app the session lacks the `requiredPermissions` for used to answer the same
8+
404-equivalent as an app that does not exist, so the two were byte-identical on
9+
the wire. A console has nothing to branch on and renders its only copy for an
10+
absent app — "it may still be publishing" — over a permanent authorization
11+
denial.
12+
13+
The by-name route now answers `403` with the ADR-0112 standard catalog code
14+
`PERMISSION_DENIED`, in the declared envelope
15+
(`{ success: false, error: { code, message } }`), when the app EXISTS and the
16+
session lacks its `requiredPermissions`.
17+
18+
Deliberately unchanged, because the disclosure is licensed only for the case
19+
above:
20+
21+
- a **nonexistent** app name keeps answering absence — converting it too would
22+
make every app name on the platform enumerable;
23+
- an **unpublished** app keeps answering `404` (ADR-0045 §3 makes it externally
24+
unobservable, and a 403 confirms existence);
25+
- an app withheld by an absent optional service (ADR-0057 D10) keeps answering
26+
`404` — nothing was denied to the caller;
27+
- the **list** route `GET /meta/apps` stays filtered exactly as before, with no
28+
`authorized: false` flag, so the enumeration surface is not widened past what a
29+
direct by-name probe already implies.
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@objectstack/rest": patch
3+
---
4+
5+
fix(rest): the direct-mount package door withholds a leaky 5xx message, like its two siblings already do (#8086)
6+
7+
A driver failure under `/api/v1/packages` returned the driver's own line to the
8+
API client. Reproduced end to end through a real `ObjectQL` engine and a real
9+
`ObjectStackProtocolImplementation`, with the driver failing the `sys_metadata`
10+
read the way a missing table does — `DELETE /api/v1/packages/:id` answered:
11+
12+
```
13+
HTTP 500
14+
{"success":false,"error":{"code":"INTERNAL_ERROR",
15+
"message":"SQLITE_ERROR: no such table: sys_metadata"}}
16+
```
17+
18+
That path is not exotic: a full uninstall (no `?version=`) routes to
19+
`protocol.deletePackage`, whose first database touch sits outside its own
20+
per-item `try`, so the driver line propagates whole into this registrar's
21+
catch-all and onto the wire.
22+
23+
**Not a new rule — the rule this surface already followed, at the door that was
24+
missed.** Both siblings sanitize: the dispatcher twin (`HttpDispatcher.error`)
25+
has replaced a leaky 5xx message with the generic text since #3867, and
26+
`rest-server.ts` runs the same predicate at three call sites. The earlier fix
27+
for this class (#5437) reached neither this registrar nor could it, because
28+
this door does not go through `resolveErrorResponse` at all. `/api/v1/packages`
29+
has **two** HTTP doors and this one mounts first in the production stack, so
30+
the unfiltered answer was the live one — one deployment, two different answers
31+
to the same failure.
32+
33+
**Only the prose is withheld.** `status`, `code` and `details` are untouched,
34+
so a client can still branch on the code and the coded-refusal mapping added in
35+
#8016 still answers. The full text still reaches the server log and the error
36+
reporter.
37+
38+
**4xx is deliberately untouched.** A refusal's message is caller-facing by
39+
design — `[tenant_scope_required]` names the exact parameter to pass, a `409
40+
DESTRUCTIVE_CHANGE` names the remedy — and it is disclosed where disclosure
41+
costs nothing, because the caller supplied the input. Withholding those would
42+
delete the self-correcting sentence that makes the refusal actionable.
43+
44+
**Known ceiling, stated so a green suite is not mistaken for full coverage.**
45+
The shared predicate is a heuristic over the message and does not recognise
46+
Postgres's `relation "…" does not exist` phrasing, so that dialect's line still
47+
travels — here and through the dispatcher twin alike, since both run the same
48+
predicate. Widening it at one door would re-create the divergence this closes.
49+
The cure is to stop interpolating driver text into client-facing messages at
50+
the producer, which is tracked separately.

‎.objectui-sha‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
6314e87f2d49b1ff3b158c296f1b2a52d14dff68
1+
6d77acfe3125949796918ea12b926d3460bc933d

‎packages/plugins/plugin-security/src/member-default-explicit-allow.test.ts‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,35 @@ describe('[#5491] what the baseline still declares, it still enforces', () => {
114114
it('better-auth identity tables stay WRITE-DENIED (the door is better-auth, not CRUD)', () => {
115115
for (const object of BETTER_AUTH_MANAGED_OBJECTS) {
116116
expect(allows('insert', [MEMBER_DEFAULT], object), `${object} insert`).toBe(false);
117-
expect(allows('update', [MEMBER_DEFAULT], object), `${object} update`).toBe(false);
117+
// [#8053] `sys_api_key` is the ONE update exception: a member revokes
118+
// their own personal key. It is not a hole in the "door is better-auth"
119+
// rule — that table is hand-rolled ObjectStack (better-auth's `apiKey`
120+
// plugin is not loaded), and the write is narrowed to the owner's rows by
121+
// the `sys_api_key_self` RLS policy and to `revoked` by ADR-0092 D2's
122+
// column whitelist. Neither narrowing is expressible as a permission-set
123+
// boolean, which is why this axis has to be asserted per object here.
124+
expect(allows('update', [MEMBER_DEFAULT], object), `${object} update`).toBe(
125+
object === 'sys_api_key',
126+
);
118127
expect(allows('delete', [MEMBER_DEFAULT], object), `${object} delete`).toBe(false);
119128
}
120129
});
121130

131+
it('[#8053] the update exception is `sys_api_key` alone, and it does not leak onto the other axes', () => {
132+
// Stated positively and separately so the loop above cannot be "fixed" by
133+
// widening the condition: every other managed table must still refuse
134+
// update, and `sys_api_key` itself must still refuse insert and delete.
135+
const alsoUpdatable = BETTER_AUTH_MANAGED_OBJECTS.filter(
136+
(o) => o !== 'sys_api_key' && allows('update', [MEMBER_DEFAULT], o),
137+
);
138+
expect(alsoUpdatable, 'no other managed identity table may become updatable').toEqual([]);
139+
140+
expect(allows('update', [MEMBER_DEFAULT], 'sys_api_key')).toBe(true);
141+
expect(allows('insert', [MEMBER_DEFAULT], 'sys_api_key'), 'minting stays POST /keys').toBe(false);
142+
expect(allows('delete', [MEMBER_DEFAULT], 'sys_api_key'), 'rows retire by revoking').toBe(false);
143+
expect(allows('find', [MEMBER_DEFAULT], 'sys_api_key')).toBe(true);
144+
});
145+
122146
it('self-service preferences survive the wildcard removal as an EXPLICIT grant', () => {
123147
// `sys_user_preference` is not a better-auth table, so the managed-deny
124148
// block does not cover it, and its `sys_user_preference_self` RLS policy

‎packages/plugins/plugin-security/src/objects/default-permission-sets.test.ts‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,19 @@ describe('BETTER_AUTH_MANAGED_OBJECTS ↔ schemas (drift pin, #3325)', () => {
3838
});
3939
});
4040

41+
/**
42+
* [#8053] The single, deliberate exception to the blanket managed-object edit
43+
* deny: `member_default` may EDIT `sys_api_key`, so a member can revoke their
44+
* own personal key. Bounded elsewhere and not by the permission-set boolean —
45+
* the `sys_api_key_self` RLS carve-out decides which rows, ADR-0092 D2's column
46+
* whitelist (`revoked` alone) decides which fields.
47+
*
48+
* Encoded as an exact (set, object) pair rather than by loosening the loop, so
49+
* a second entry — or the same one on another set — still fails this pin. The
50+
* create/delete/read axes are NOT excepted and are still asserted below.
51+
*/
52+
const EDIT_EXCEPTIONS = new Set(['member_default::sys_api_key']);
53+
4154
describe('default permission sets carry the managed denies (static baseline)', () => {
4255
it('each write-granting target set denies create/edit/delete on every managed object', () => {
4356
for (const setName of MANAGED_DENY_TARGET_SETS) {
@@ -47,13 +60,34 @@ describe('default permission sets carry the managed denies (static baseline)', (
4760
const entry = set.objects[obj];
4861
expect(entry, `${setName} has entry for ${obj}`).toBeTruthy();
4962
expect(entry.allowCreate).toBe(false);
50-
expect(entry.allowEdit).toBe(false);
63+
expect(entry.allowEdit, `${setName}.${obj} allowEdit`).toBe(
64+
EDIT_EXCEPTIONS.has(`${setName}::${obj}`),
65+
);
5166
expect(entry.allowDelete).toBe(false);
5267
expect(entry.allowRead).toBe(true);
5368
}
5469
}
5570
});
5671

72+
it('the edit exception is exactly one (set, object) pair, and it is the API-key one', () => {
73+
// The exception list is itself pinned: a future widening has to edit THIS
74+
// assertion, which is the moment someone is asked whether the new pair
75+
// really rides an owner-scoping RLS policy and a column whitelist the way
76+
// `sys_api_key` does. Without this, `EDIT_EXCEPTIONS` could grow silently.
77+
expect([...EDIT_EXCEPTIONS]).toEqual(['member_default::sys_api_key']);
78+
79+
const member = setByName('member_default');
80+
expect(member.objects.sys_api_key.allowEdit).toBe(true);
81+
// The owner scoping the grant leans on must exist, or the edit bit is
82+
// table-wide on a credential table.
83+
const selfPolicy = (member.rowLevelSecurity ?? []).find(
84+
(p: any) => p.object === 'sys_api_key' && p.name === 'sys_api_key_self',
85+
);
86+
expect(selfPolicy, 'member_default must keep the sys_api_key_self RLS carve-out').toBeTruthy();
87+
expect(selfPolicy.using).toBe('user_id == current_user.id');
88+
expect(['all', 'update']).toContain(selfPolicy.operation);
89+
});
90+
5791
it('admin_full_access keeps its bare wildcard (zero per-object entries) — admin rescue path', () => {
5892
const admin = setByName('admin_full_access');
5993
expect(admin).toBeTruthy();

‎packages/plugins/plugin-security/src/objects/default-permission-sets.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,52 @@ const baseDefaultPermissionSets: PermissionSet[] = [
354354
// but the grant itself: it is what keeps `/auth/me`, the org switcher and
355355
// the Account app working for a member with no application profile.
356356
...denyWritesOnManagedObjects(),
357+
// [#8053] The ONE override of the block just above: a member may revoke
358+
// their OWN API key. #7727 opened the method gate and registered the
359+
// ADR-0092 D2 column whitelist, but left this object-CRUD layer
360+
// untouched, so `update` on `sys_api_key` resolved for
361+
// `admin_full_access` alone — the `revoke_api_key` row action rendered in
362+
// the member's own My Keys grid and answered 403 for them. A personal key
363+
// "acts as you — treat it like a password", and the owner is the person
364+
// who discovers it leaked; their only remedy was to find an admin.
365+
//
366+
// This is a restoration of declared-≠-enforced intent, not a new grant:
367+
// the row action, the checklist persona and the `sys_api_key_self` policy
368+
// below (which already makes the row owner-VISIBLE) all say the owner
369+
// path was intended — there was simply no `allowEdit` to go with it.
370+
//
371+
// The opening is bounded by TWO pre-existing mechanisms, and it is
372+
// deliberately not bounded by this line alone:
373+
// - WHICH ROWS: the `sys_api_key_self` RLS carve-out below
374+
// (`user_id == current_user.id`, `operation: 'all'`), enforced on
375+
// by-id writes through the security middleware's pre-image check. A
376+
// member PATCHing another user's key is still refused.
377+
// - WHICH FIELDS: ADR-0092 D2's identity write guard, whose per-object
378+
// update whitelist for this table lists `revoked` alone
379+
// (plugin-auth `MANAGED_EXTENSION_EDITABLE_FIELDS`). `key` stays
380+
// unwritable (a rotated hash mints a credential nobody holds) and
381+
// `user_id` stays unwritable (re-owning a key is privilege transfer).
382+
//
383+
// `allowCreate` / `allowDelete` stay false and are NOT an oversight:
384+
// minting is `POST /api/v1/keys` (the only path that returns the raw
385+
// secret once) and rows are retired by revoking, not deleting, so history
386+
// survives. `allowDelete` also stays false because this set is bound to
387+
// the `everyone` anchor and must remain anchor-safe (ADR-0090 D5).
388+
//
389+
// ⚠️ Not a pattern to copy across the managed list. Every OTHER
390+
// better-auth table here stays write-denied because its mutations must
391+
// flow through an auth endpoint; this one is a hand-rolled ObjectStack
392+
// table (`packages/core/src/security/api-key.ts` mints and verifies it,
393+
// better-auth's `apiKey` plugin is not loaded) whose one platform-owned
394+
// column already has a registered whitelist. Widening `update` on
395+
// `sys_api_key` beyond owner-scoped-plus-one-column would close #8053 and
396+
// open a worse defect on a table whose rows act as the user.
397+
//
398+
// Being an EXPLICIT entry is what makes it survive `kernel:ready`:
399+
// `applyManagedWriteDenies` injects its deny only for managed objects a
400+
// target set does not already name (`name in objects` → skip), so this
401+
// line is preserved rather than overwritten.
402+
sys_api_key: { allowRead: true, allowCreate: false, allowEdit: true, allowDelete: false },
357403
// Self-service preferences. NOT a better-auth table, so it is not covered
358404
// by the block above, and its `sys_user_preference_self` RLS policy below
359405
// (`operation: 'all'`) declares exactly this intent: a member reads and

0 commit comments

Comments
 (0)