Skip to content

fix(service-settings): supply the effective tenancy posture at the manifest gate - #16015

Merged
zhuangjianguo merged 4 commits into
mainfrom
claude/issue-15351-settings-tenancy-posture
Sep 5, 2026
Merged

fix(service-settings): supply the effective tenancy posture at the manifest gate#16015
zhuangjianguo merged 4 commits into
mainfrom
claude/issue-15351-settings-tenancy-posture

Conversation

@claude

@claude claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #15351

SettingsServicePlugin's verifiedContextFromRequest — the manifest gate behind every /api/settings door — called resolveAuthzContext with no tenancyPosture. Both posture-conditional API-key refusals are gated on a posture the caller supplies (organization_required in packages/core/src/security/api-key.ts, organization_membership_ended in packages/core/src/security/resolve-authz-context.ts), so neither ran here. An API key's tenantId is sys_api_key.active_organization_id copied verbatim — the caller's own stored claim, never vetted against current membership — so under a wall-enforcing posture a key stamped with an organization its owner had left was admitted, and this seam then handed that organization onward as SettingsContext.tenantId, the resolved tenant the settings read/write path acts on.

The posture is now derived at this door through the kernel's async registry, carrying decision-1-option-A's classification (#13906): a tenancy service that was never registered resolves quietly to undefined, one that was registered and failed to build raises AuthzStoreUnavailableError. The requested posture (OS_TENANCY_POSTURE, via resolveTenancyPosture()) is deliberately not reused — under ADR-0093 D4/D5 a deployment that asks for isolated without the enterprise organizations runtime is single in force, and the requested value would refuse legitimate automation over a wall that is not there.

Measured, at d49ec41f7

Driven through the real SettingsServicePlugin (initstartkernel:ready), a real ObjectQL engine over a memory driver, the real routes, and a tenancy service on a REAL ObjectKernel so the never-registered / registered-and-broken split comes from the registry's own rejections rather than a hand-branded stub. sys_setting / sys_setting_audit rows are real and are read back out of the driver's store, never off a response body.

door before after
PUT /api/settings/:ns, ex-member's org-stamped key 200, the row lands 403, zero rows
GET /api/settings/:ns, same key 200, values served 403
GET /api/settings (the listing) 200, the namespace enumerated 200, empty list
organization-less key, same posture 200, the row lands 403, zero rows
CURRENT member's key (control) 200, row lands 200, row lands — unchanged
no credential (control) 403 403 — unchanged

The wiring ablation, held permanently in the test

Three mount() shapes differ in nothing but whether this seam can resolve a posture. no-async-registry and unregistered re-admit the very caller the isolated case refuses, so the refusal is attributable to the posture supply and to nothing else; the CURRENT-member control proves the door still authenticates somebody. Both re-admitting shapes are ruled behaviours, not regressions: a host with no tenancy service has no wall, and a KernelBase/LiteKernel host has no async registry at all (#15997, already filed — untouched here).

The returned tenantId, the half of the card that is not admission

A browser session whose stored activeOrganizationId is no longer backed by a sys_member row now has that claim dropped here (#15409's ruled option B) instead of returned as the settings tenant. Pinned with its own ablation (no posture resolvable ⇒ the stale org_A comes back) and a current-member control in both the session and the API-key direction.

Red-then-green, twice

Both ablations ran in a single shell with the mutation proven on disk (HEAD-blob hash comparison plus injected/removed marker counts) BEFORE any measurement, and the restore proven after (git diff HEAD empty, restored hash equal to the HEAD blob). No dist/ is involved: the suite imports the seam through a relative source specifier, so there is no built artifact to go stale.

  1. Remove the posture from the call site (the fix's one line): 7 failed / 11 passed — exactly the six refusal cases plus the session claim-drop. The controls, both re-admitting ablation shapes, and the 503 arm stay green.
  2. Replace the classification with the forbidden catch { undefined } ([finding] two more computeExecCtx seams read "failed" and "not wired" as one value, and both feed authorization inputs — tenancy posture and the ADR-0069 auth gate #13906's permissive-on-failure defect): 3 failed / 15 passed — exactly the three ADR-0112 outage cases.

Commands

  • pnpm --filter '@objectstack/service-settings^...' build — green (the closure, built before anything was judged).
  • pnpm --filter @objectstack/service-settings typecheck — green. It really does cover the new test file: an earlier run failed with settings-admission-tenancy-posture.test.ts(395,87): error TS2339 before that line was fixed.
  • pnpm --filter @objectstack/service-settings test — 31 files, 551 passed.
  • The full derived gate family for this diff (node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, 54 families) re-run at d49ec41f7: 52 green. check:objectql-double-limit found the new fixture driver twice — limit-blind, then touching rows outside the bound — and both were repaired rather than baselined; it is green now.

NOT MEASURED, declared

check:dual-build-cjs-loads and check:type-check-debt both exit 3 — PREREQUISITE NOT MET: they read built output and 54 packages have no dist/ in this worktree. That is not a pass and not a failure; CI runs both after its own workspace build. Neither can plausibly move on this diff — no package.json, no exports, no ledgered-package boundary is touched, and check:type-check-coverage (which counts hidden tests) is green.

Contract tier

Clause-②: no — re-derived from this diff, not inherited from the claim.

api-surface/ artifacts exist only for packages/spec, so the published surface of this service package is its files[] + types, i.e. dist/**. Measured by building dist/index.d.ts on this branch and again with the one edited source file swapped back to f7db8f4fd, then diffing. Ignoring doc comments, the delta is exactly one line:

+    private resolveAdmissionTenancyPosture;

Name-only, type-erased, not callable from outside. SettingsServicePlugin already ships four private members and this file's shipped classes carry over forty; the byte-identical line already ships in a published sibling, @objectstack/cloud-connection (MarketplaceInstallLocalPlugin is exported from its index and declares the same private member). No exported type, signature or accept surface is added or widened. Behaviourally the accept set narrows — a declared guard returns to enforced — which is the card's Bug grading, not a contract widening.

Deliberately not done

Scope

Enterprise-surface: the wall-enforcing posture needs the org-scoping service, and nothing in this repository registers it (sole registrar is cloud-private packages/organizations). That shapes urgency, not correctness. The card body's census ("exactly two callers supply a posture") was already stale when it was written — measured on this tree it is four: packages/rest/src/rest-server.ts, packages/runtime/src/security/resolve-execution-context.ts, packages/mcp/src/plugin.ts and packages/cloud-connection/src/marketplace-install-local-plugin.ts. With this PR it is five of the eight non-test callers; the three left are packages/plugins/plugin-sharing/src/sharing-plugin.ts:847, packages/services/service-datasource/src/admin-routes.ts:387 and packages/services/service-storage/src/storage-service-plugin.ts:841.


Generated by Claude Code

…nifest gate

`SettingsServicePlugin`'s `verifiedContextFromRequest` called
`resolveAuthzContext` with no `tenancyPosture`, so both posture-conditional
API-key refusals stayed unreachable at the settings doors: under a
wall-enforcing posture an ex-member's org-stamped key and an organization-less
key were both admitted. This seam also returns `authz.tenantId` onward as the
resolved settings tenant, so the key's own unvetted `active_organization_id`
became the verdict the read/write path acted on.

The posture is derived here through the kernel's async registry, carrying
decision-1-option-A's classification: a `tenancy` service that was never
registered resolves quietly to "no posture", one that was registered and failed
to build raises `AuthzStoreUnavailableError`. The requested posture
(`OS_TENANCY_POSTURE`) is deliberately not reused — under ADR-0093 D4/D5 it
would refuse legitimate automation over a wall that is not in force.

Deliberately not extracted into a shared helper: sibling repairs are in flight
on this same seam, and this copies the precedent set by
`@objectstack/cloud-connection`'s install-local door.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…on its void return

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
…'s find

`check:objectql-double-limit` graded the new fixture driver twice: first
limit-blind, then touching rows outside the bound because the copy ran before
the slice. The bound is now applied after the filter, by presence, and before
any row-touching stage; `count` no longer derives a population size from a page.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions github-actions Bot added the size/l label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-settings, touching 3 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/protocol/kernel/index.mdx (via SettingsServicePlugin (symbol, a top-level class))
What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 7 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json f50c394da55846da8d38f1e1efadbc24faa26ce2packageMentionDocs.

Which tree this was computed on

This run read content/docs from acd91daba9ea6a4665abd351bcb14f9d3beb2cae — the merge of head bc53dad48d3ef37d16dc06fba97bd967e278bced into base f50c394da55846da8d38f1e1efadbc24faa26ce2, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin acd91daba9ea6a4665abd351bcb14f9d3beb2cae && git checkout acd91daba9ea6a4665abd351bcb14f9d3beb2cae
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f50c394da55846da8d38f1e1efadbc24faa26ce2 bc53dad48d3ef37d16dc06fba97bd967e278bced && git checkout -B drift-repro f50c394da55846da8d38f1e1efadbc24faa26ce2 && git merge --no-ff bc53dad48d3ef37d16dc06fba97bd967e278bced

node scripts/docs-audit/affected-docs.mjs --json f50c394da55846da8d38f1e1efadbc24faa26ce2

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs f50c394da55846da8d38f1e1efadbc24faa26ce2 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026
@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Docs Drift Check — verdict on the one page it named

content/docs/protocol/kernel/index.mdxnot falsified. No edit owed in this PR, and no card owed either. Read per-file, not by a lane-wide grep.

Why it was named: the detector attributes a page to a top-level exported symbol, and SettingsServicePlugin is a top-level class this PR edits. That is a true positive for the instrument and a null for the page.

What the page actually claims about that symbol. One mention, line 180, and it names the class only as the registrar of the settings service. The claim around it is about the RESOLVER'S CASCADE reached through ctx.getService('settings'):

const { value, source, locked } = await settings.get('stripe', 'apiKey', { tenantId, userId });
// Cascade: OS_STRIPE_APIKEY (env) > global > tenant > user > registered default.

That is the in-process, trusted path. This PR touches only verifiedContextFromRequest, the resolver the plugin hands to registerSettingsRoutes for the HTTP boundary (enforced: true). An in-process caller leaves enforced unset, assertPermitted returns early for it, and no posture is consulted on that path at all — the trusted/enforced split this snippet sits on the trusted side of. SettingsService.get, the cascade, source, locked and SettingsLockedError are all untouched.

The tenantId in that snippet is an argument the caller supplies, not a value the plugin derives. What this PR changes is what SettingsContext.tenantId is when the service is reached over HTTP. The page never describes the HTTP route surface, never mentions /api/settings, and never says where an HTTP caller's tenant comes from.

The one admission passage on the page is about a different seam. Lines 293-307 describe resolveExecutionContext() — the runtime dispatcher — and already carry the post-#13906 language: "AuthzStoreUnavailableError (503) only when an authorization input exists and could not be read — a failed permission-store read, or a tenancy service that is registered and failed to build." Still true, and now true of one seam more than when it was written. It names resolveExecutionContext, so nothing there needs amending.

Measured, with a positive control on the same command and scope. Over that one file:

  • grep -niE 'api[- ]?key|resolveAuthzContext|tenancyPosture|tenancy|posture|isolated|admission|admit|authoriz|authenticat|activeOrganization|organization|member|session|401|403|enforced' — 13 lines, none attributing an admission behaviour to SettingsServicePlugin.
  • CONTROL, same command form and same scope: grep -niE 'tenantId' — 3 hits (lines 183, 302, 477). The scan is live, so the null above is a reading and not a silently empty one.

No page under content/docs/releases/** was read or touched.


Generated by Claude Code

@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Standing down on the red Lint & Repo Gates: not this PR's — the sixth PR today hit by the gate defect filed as #15990 / #15992.

check:merge-driverscripts/check-regen-pending.mjs --self-test, job 101358654781, on the current head d49ec41f7.

Mechanism (measured; my first summary of it was wrong and is corrected on #15978/#15998): the launcher does not fail to resolve. .github/actions/setup-pnpm materialises only the pinned pnpm 10.31.0 and writes no lastKnownGood.json; the fixture's tmpdir pins nothing, so Corepack resolves the registry's latest — now pnpm 12.3.4, whose Rust CLI rejects -s with exit 2. Every stub then reads as "gate exited non-zero" ⇒ stale, reddening exactly the cases that expect a non-stale verdict. Corepack writes a sticky lastKnownGood.json and actions/cache@v6 saves COREPACK_HOME back, so a job restores a store holding either a -s-accepting pnpm or a pnpm 12.

Not tree-dependent, and the cleanest proof is on main itself: commit f7db8f4fd carries two Lint & Repo Gates runs — one failure, one success. Same tree, opposite verdicts. origin/main's own push build (run 33981169123) concluded failure at 17:51Z.

Severity bound: false-RED only, never false-green — a poisoned store makes every artifact look stale, and stale is the refusing verdict.

Fix: PR #16002 (from the domain:spec seat, pinning the fixture to the root's packageManager) is queued and is the one landing. Nothing to port before it does.

⛔ No test skipped, disabled or quarantined; ⛔ no empty commit. Spending this PR's one permitted re-run, unspent.
⚠️ All four earlier re-runs reproduced, 4 for 4 — so a pass here would be luck, not a verdict, and a failure is expected rather than new information. Either way this PR waits on #16002.


Generated by Claude Code

@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Update, one minute later: ⛔ the re-run is NOT being spent. The fix has landed, so the remedy changed.

#16002 merged as f50c394da"fix(devx): pin the regen self-test fixture to the root's packageManager (#16002)". Verified by content on origin/main, not by the API's merged field: packageManager now matches in scripts/check-regen-pending.mjs (control on the same command and scope: fixtureSelfTest, which also matches, so the reading is live).

The comment above said this PR's one permitted re-run was being spent. It is not, and the reason is worth stating rather than quietly skipping:

  • A re-run replays the original merge ref, which was computed against the old base. It would re-measure the same poisoned condition and tell us nothing new — 4 for 4 of the earlier re-runs reproduced.
  • With the fix on main, the CI rule's own preferred path applies instead: "port the same change into this PR now and push: it no-ops once the base carries it". The base carries it, so the port is simply merging main.

So this PR's re-run stays unspent, and the next action here is a main merge to re-run CI against the fixed base. ⛔ Still no test skipped, disabled or quarantined; ⛔ still no empty commit.


Generated by Claude Code

Picks up #16002 (f50c394), which pins the regen self-test fixture to the
root's packageManager and clears the Corepack-store defect that reddened
check:merge-driver on this PR and five others.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 5, 2026 20:02
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit 2024eca Sep 5, 2026
35 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-15351-settings-tenancy-posture branch September 5, 2026 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

service-settings: the manifest gate supplies no tenancyPosture to resolveAuthzContext — and returns the unvetted tenantId to its callers

2 participants