Skip to content

Commit 45f428d

Browse files
objectstack-fleet[bot]hotlongclaude
authored
fix(runtime): the dispatcher's /packages doors scope to the vetted organization, not the raw session claim (#20477) (#20491)
Fixes #20477 Clause-②: no ## What was wrong (H0, measured before any edit) The runtime dispatcher's nine organization-scoped `/packages` doors take the caller's organization from one source, `HttpDispatcher.resolveActiveOrganizationId`. That source returned the auth session's `activeOrganizationId` as stored. `resolveAuthzContext` vets that claim onto the execution context as `tenantId`, and under a wall-enforcing posture it drops a claim that no membership backs (maintainer ruling B on #15409, implemented by PR #15794). The dispatcher never read the vetted value. Measured on unmodified `origin/main` source (`851af0c27`), with real identity resolution (`dispatch()` → `resolveRequestScope` → `resolveExecutionContext` → `resolveAuthzContext`) under an `isolated` posture. The subject is a member removed from `org_alpha` whose session still names it. They hold the same permission set as a current member, so RBAC cannot separate the arms. | Door | Transport | What it did with `org_alpha`'s rows | |---|---|---| | `GET /packages/:id/commits` (read) | `dispatch()`, and the plugin's explicit mount over a real socket | 200, served `org_alpha`'s commit `cmt_alpha` | | `POST /packages/:id/discard-drafts` (write) | both | 200, deleted `org_alpha`'s draft | | `DELETE /packages/:id` (write) | both | 200, deleted `org_alpha`'s row | All nine doors handed the protocol `org_alpha` on both transports: 20 of 20 subject cases red, 56 of 56 controls green. So reach is served, and the p0 grade stands. Every effect stayed inside the test's own in-memory stack. ## The fix The fix is in the one source, not at the nine sites. `resolveActiveOrganizationId` now returns `context.executionContext.tenantId` through `metaCallerOrganizationId` from `@objectstack/rest`. That is the helper `RestServer` and the dispatcher's `/meta` doors already share (#20408). - `@objectstack/rest` is already a dependency of `@objectstack/runtime` (`domains/meta.ts` imports from it), so no dependency edge is added. - There is no second vetting path: `packages/core` is untouched. - `domains/packages.ts` is unchanged. The `domain-handler-registry.ts` edit is only the dep's contract comment. ## Hypotheses - **H1 (every caller has the vetted context): holds.** - The dep's only caller is `domains/packages.ts`, at 9 sites (`git grep` at HEAD). - Both HTTP entries reach the domain through `dispatch()`: the `createHonoApp` catch-all and `createDispatcherPlugin`'s explicit mounts. `dispatch()` runs `resolveRequestScope`, which writes `executionContext` before any domain handler. Only the declared liveness route (`/health`) skips it, and that route reads no organization. - The domain's first statement is the anonymous-deny floor. It refuses a context with no resolved principal before any of the nine sites is reached. - So no call site runs without a resolved context. - **H2 (controls unchanged): holds.** - A current member reaches their own organization on every door, on both transports. - The ex-member, switched to an organization they belong to, reaches that one and never the one they left. - An anonymous caller gets `401 ANONYMOUS_DENY` before any protocol call. - A control asserts the resolver really dropped the ex-member's claim: its `Session organization claim dropped` line names `org_alpha`. So the green subject cannot come from a rig that never presented the stale claim. - **H3 (ablation): holds, in the direction predicted.** - Starting from the committed fix, `scripts/ablation-replace.mjs` put the original raw-session body back, verbatim from `BASE` (anchor 1 → 0, blob `d6ad749b8ced` → `0414fe725ad2`). - Predicted before the run: exactly the 20 left-organization cases red and the 56 controls green, plus the migrated seed-apply §0 control red. - Measured: 21 failed and 65 passed of 86, and the red set is exactly those 21 names. - The tool proved the restore: the blob after restore equals the blob at HEAD (`d6ad749b8ced`), and `git diff HEAD` is empty. ## The pins `packages/runtime/src/domains/packages-vetted-org-source.test.ts` has 76 cases. It covers each of the nine doors × both transports × four callers: a current member, the ex-member switched to a real membership, an anonymous caller, and the ex-member with the stale claim. It adds the claim-drop control and the uninstall refusal. - The subject cases assert two things: the protocol is handed no organization, and the left organization's partition is neither read (no `alpha` in the answer) nor written (its partition is byte-identical afterwards). - The ex-member's uninstall gets the protocol's refusal, `400 TENANT_SCOPE_REQUIRED`, and nothing is deleted. The protocol double copies that refusal from `metadata-protocol`'s `deletePackage`. ## One existing test migrated `packages-seed-apply-org-scope.test.ts` passed its organization in through a stubbed `getSession`, which is the raw source this PR retires. After the fix, every measurement in that file had silently moved to its one-rung branch, and only its §0 control went red. The organization now lands on the execution context's `tenantId`, where `dispatch()` puts it, and the dead session stub is removed. ## Behaviour notes - **API-key callers.** The doors now use the organization the key is bound to. The raw read found no session for a key, because API keys resolve in `@objectstack/core`, not in better-auth. So these doors used to get no organization for a key caller. This matches `RestServer` and the landed `/meta` doors. It is reasoned from source, not measured with a key. - **The ex-member after the fix** is exactly a session with no active organization, so the doors reach the env-wide package state. The landed `/meta` sibling does the same: its pin lands the ex-member's `PUT` env-wide on both transports. ## Verification Run on the final head `1e0553b290`, after merging `origin/main` at `3062e5001`. - **Build:** `pnpm --workspace-concurrency=2 --filter '@objectstack/runtime^...' --filter @objectstack/runtime build` exited 0. - **Tests:** the `@objectstack/runtime` `local` project ran 285 files: 4160 passed, 1 skipped. `test:repo` ran 3 files: 575 passed. - **Typecheck:** exited 0, and `check:test-typecheck` is OK. `tsc --listFiles` confirms both touched test files are in the test program. - **Lint:** `pnpm lint` exited 0. - **Citations:** `node scripts/check-issue-citations.mjs --base origin/main` reports that every added citation resolves. - **Derived gates:** `dispatch-gates --commands` derived 61 commands, and 60 of them ran with exit 0. The `--ran` reconciliation reads 61 derived, 60 run, 1 NOT-MEASURED, 0 UNRUN. - The NOT-MEASURED one is `pnpm check:dual-build-cjs-loads`, which exited 3 with `PREREQUISITE NOT MET` because the whole workspace's `dist/` is not built in this worktree. It is not a pass. This diff touches no manifest, export or build config. **Declared narrowing — verification ran UNLOCKED.** `scripts/pm/os-verify-lock.sh` could not take the shared verify lock on this host: no usable `flock`. The shared verify lock is declared Linux-only (`flock` is util-linux, and a stock macOS does not ship it), so the command below was run directly, without the lock — a declared narrowing, not a silent one. No serialization guarantee held for this run, nor for any sibling agent in this container while it ran. pnpm --filter @objectstack/runtime exec vitest run --project local --maxWorkers=2 Every build and test command listed above ran under the same declaration, and each printed this block with its own command. ## Acceptance notes - **Out of scope, found while measuring, reported for the seat to file (class a).** `DELETE /packages/:id` runs `registry.uninstallPackage(id)` before the protocol's org-scoped persistence call. - The protocol refuses any caller with no active organization (`400 TENANT_SCOPE_REQUIRED`, "Refusing to uninstall"). By then the package has already been removed from the live registry. - Measured through `dispatch()` with a real `SchemaRegistry` and the real `ObjectStackProtocolImplementation`: `GET /packages/:id` answered 200 before the refused DELETE and 404 after it, while the stored rows were kept. - It is not fixed here: it is a different defect class from this card, in code this diff does not touch. - **ADR-0123 boundary, noted only.** ADR-0123 D2 refuses a caller with no organization when they write tenant-scoped data through the security middleware. The package verbs run as protocol calls, so that caller's publish-drafts, discard-drafts, revert, rollback and adopt-orphans reach the env-wide package state. - That is the existing behaviour for every session with no active organization, and the landed `/meta` sibling behaves the same way. - Whether ADR-0123's write refusal should also cover metadata-package verbs is a question for the maintainer. It is not a finding here. --- _Generated by [Claude Code](https://claude.ai/code/session_local_1d2a197c-c20e-4e90-9be8-413d4d432289)_ --------- Co-authored-by: Jack Zhuang <50353452+hotlong@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent fc0db22 commit 45f428d

5 files changed

Lines changed: 479 additions & 56 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/runtime': patch
3+
---
4+
5+
fix(runtime): the dispatcher's `/packages` doors scope a caller to the organization the identity step vetted, not the session's stored claim (#20477)
6+
7+
Clause-②: no
8+
9+
The runtime dispatcher's nine organization-scoped `/packages` doors took the caller's organization from the auth session's `activeOrganizationId` as stored. Under a wall-enforcing tenancy posture (`isolated` or `group`), the identity step drops that claim when no membership backs it any more, and the request resolves with no active organization (the maintainer's ruling B on #15409). The doors never saw the drop. So a member removed from an organization kept that organization's packages for the rest of the session: its commit history and whole-package export were served to them, and their publish-drafts, discard-drafts, commit revert, rollback, adopt-orphans, duplicate and uninstall ran inside it.
10+
11+
The doors now read the vetted organization on the request's execution context, the value `RestServer` scopes by and the dispatcher's `/meta` doors already read. The fix is in the one source the nine doors share (`HttpDispatcher`'s `resolveActiveOrganizationId`), so every door changes together, on both HTTP entries (the `createHonoApp` catch-all and the dispatcher plugin's explicit package routes).
12+
13+
- **A removed member whose session still names the organization they left:** every door is handed no organization. Reads and writes reach only the env-wide package state, as for any session with no active organization. An uninstall is refused `400 TENANT_SCOPE_REQUIRED` and deletes nothing. The left organization's rows are neither read nor written.
14+
- **A caller authenticated by an API key:** the doors now use the organization the key is bound to. The session read found no session for a key, so these doors used to get no organization for it.
15+
- **Unchanged:** a current member reaches their own organization exactly as before, a member who switched to an organization they belong to reaches that one, and an anonymous caller is refused `401` before any package operation. Single-posture deployments are unchanged, because no claim is dropped there.

‎packages/runtime/src/domain-handler-registry.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,13 @@ export interface DomainHandlerDeps {
320320
* surface relies on field-anchored 422s).
321321
*/
322322
errorFromThrown(e: any, fallbackStatus?: number): { status: number; body: any };
323-
/** Active organization id from the request session (undefined if anonymous / no auth). */
323+
/**
324+
* The caller's VETTED active organization — the execution context's
325+
* `tenantId`, never the session's stored `activeOrganizationId`, which the
326+
* identity step drops under a walled posture when no membership backs it
327+
* (#20477). `undefined` for anonymous, no active organization, or a dropped
328+
* claim.
329+
*/
324330
resolveActiveOrganizationId(context: HttpProtocolContext): Promise<string | undefined>;
325331
/**
326332
* Fire a kernel-context event on the request's resolved kernel (no-op

‎packages/runtime/src/domains/packages-seed-apply-org-scope.test.ts‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
* would double-insert"). So the second call presents a `publishPackageDrafts`
5151
* that reports the published seed without a `seedApplied` field — the exact
5252
* population this fallback documents itself as existing for. It also records
53-
* the request it received, which is §0's positive control that the session's
53+
* the request it received, which is §0's positive control that the caller's
5454
* organization really reached this request.
5555
*
5656
* ## Sections, and which are evidence vs. which are the bound
@@ -217,18 +217,24 @@ function makeEngine() {
217217
return engine;
218218
}
219219

220-
/** An authenticated package admin — the route's anonymous-deny + capability floor. */
221-
const PKG_ADMIN = (): any => ({
220+
/**
221+
* An authenticated package admin — the route's anonymous-deny + capability
222+
* floor. `organizationId` lands where `dispatch()`'s identity step puts the
223+
* caller's VETTED organization, the execution context's `tenantId`: the one
224+
* source the route reads (#20477), never the session claim as stored.
225+
*/
226+
const PKG_ADMIN = (organizationId?: string): any => ({
222227
request: { headers: {} },
223228
environmentId: 'env_1',
224229
executionContext: {
225230
userId: 'u_pkg_admin',
226231
systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'],
232+
...(organizationId ? { tenantId: organizationId } : {}),
227233
},
228234
});
229235

230236
interface DriveOptions {
231-
/** Session's active organization; `undefined` drives the one-rung branch. */
237+
/** The caller's vetted active organization; `undefined` drives the one-rung branch. */
232238
activeOrganizationId?: string;
233239
/** Injection: the read-back throws this instead of answering. */
234240
readBackError?: () => Error;
@@ -297,13 +303,6 @@ async function publishThenRead(opts: DriveOptions = {}) {
297303
fields: { name: { type: 'text' }, status: { type: 'select' } },
298304
}),
299305
},
300-
auth: {
301-
api: {
302-
getSession: async () => (opts.activeOrganizationId
303-
? { session: { activeOrganizationId: opts.activeOrganizationId } }
304-
: { session: {} }),
305-
},
306-
},
307306
};
308307
const kernel: any = {
309308
getServiceAsync: async (name: string) => services[name] ?? null,
@@ -312,7 +311,7 @@ async function publishThenRead(opts: DriveOptions = {}) {
312311
};
313312

314313
const result = await new HttpDispatcher(kernel).handlePackages(
315-
`/${PKG}/publish-drafts`, 'POST', {}, {}, PKG_ADMIN(),
314+
`/${PKG}/publish-drafts`, 'POST', {}, {}, PKG_ADMIN(opts.activeOrganizationId),
316315
);
317316
expect(result.response?.status).toBe(200);
318317
const body: any = (result.response as any)?.body;
@@ -388,7 +387,7 @@ describe('#15068 · 0 · the publish-then-read path really runs', () => {
388387
expect(served?.item?.records).toHaveLength(2);
389388
});
390389

391-
it('the session organization really reaches this request', async () => {
390+
it('the caller\'s organization really reaches this request', async () => {
392391
const { publishRequest } = await publishThenRead({ activeOrganizationId: ORG });
393392

394393
// `applyPublishedSeeds` receives the SAME binding this route handed

0 commit comments

Comments
 (0)