Skip to content

test(rest): drive the tenancy axis of GET /ui/view/:object/:type — an anonymous caller can name another environment and receive its UI view - #13258

Open
os-trump wants to merge 3 commits into
mainfrom
claude/issue-13214-ui-view-tenancy-exposure
Open

test(rest): drive the tenancy axis of GET /ui/view/:object/:type — an anonymous caller can name another environment and receive its UI view#13258
os-trump wants to merge 3 commits into
mainfrom
claude/issue-13214-ui-view-tenancy-exposure

Conversation

@os-trump

Copy link
Copy Markdown
Collaborator

Part of #13214

⛔ Deliberately notFixes: #13214 is a decision awaiting the maintainer, and a measurement must not auto-close it.

⛔ MEASUREMENT ONLY — nothing is repaired

The change set is one new test file. packages/rest/src/rest-server.ts is byte-identical to the base — blob 3eca151d4b118f0a84c10958cd3c5e8b43b79406 on both sides, verifiable with git rev-parse HEAD:packages/rest/src/rest-server.ts. Access control is a human floor in this repo: reporting the measurement is the complete delivery.

packages/core/src/security/auth-gate.ts (#7898's on-hold trigger file, H17) was not read or edited on this branch — the previous run already settled that question by driving the predicate.

What this extends, and what it does NOT re-measure

PR #13244 settled the single-tenant half: this route resolves no identity at the REST seam, and getUiView applies no authorization downstream because the seam hands it exactly { object, type }. ⛔ None of that is re-measured here.

What #13244 explicitly did not assert is its own UNMEASURED 1. Its harness had no envRegistry and no kernelManager, so the environment-resolution chain was read from source and never driven. This PR drives it for the first time, treating the earlier reading as a hypothesis rather than a fact.

⭐ The instrument is proved bidirectional before any reading is trusted

Four controls run first (section 0), because a harness that always answers "environment B" is no evidence of a crossing and one that can only answer "environment A" is no evidence of its absence:

ControlWhat it establishes
C1The two environments' producers, called directly, answer observably differently — distinct object label, disjoint field sets, in both directions.
C2With no header and an unbound hostname the route answers with environment A's view. The "would otherwise resolve to A" baseline is measured, not assumed.
C3The scoped mount (/environments/:environmentId/ui/view/...), where naming an environment is declared and URL-visible, answers with environment B's view. So the route CAN deliver B's body on this instrument, and a negative would have been a real negative.
C4The kernel-acquisition recorder distinguishes which environment was acquired, so acquired is an observable and not a constant.

A third distinct answer (Control Plane Accounts) is wired at the boot protocol argument, so "fell through to the control plane" cannot collapse into "resolved to A".

① and ② — the answers, per channel

Both channels cross. Each is a separate reading and each carries its own controls.

Channel 1 — the X-Environment-Id header

An anonymous request (no execution context at all) on the unscoped URL, whose hostname resolves to nothing, carrying only X-Environment-Id naming environment B, receives environment B's UI view. Three independent observations, not one:

  • the body is B's (Beta Environment Accounts, beta_only_field), not the A baseline C2 measured;
  • B's kernel was acquired by that anonymous request (acquired === ['env_beta']);
  • resolveExecCtx was called zero times, on every site the request reached.

An entitled caller naming B receives byte-identical bytes, so the answer does not depend on the caller.

Negative control: a header naming an id the registry does not know does not cross — it falls to the default environment — and the registry was consulted (idLookups contains the probed id), so that green is a decision rather than a header nobody read.

Channel 2 — the request hostname

An anonymous request whose hostname is bound to environment B receives environment B's view, again with resolveExecCtx at zero and B's kernel acquired.

  • Control: the same channel answers with A when the hostname is bound to A, so it is bidirectional.
  • Negative control: an unbound hostname does not cross; it falls to the default.
  • Precedence, measured: hostname is consulted before the header, so a bound host wins and the header is never even looked up. On a hostname-routed deployment the header is not an additional lever; on a non-hostname deployment it is the lever.

③ Blast radius on the cross-environment path — measured, ⛔ not inherited

Every reading below is pinned to the crossed body (it asserts B's own label first), so none of it is an envelope statement true of any environment.

  • Object metadata only. The list body is exactly { object, list } with list = { type, label, columns, sort, searchableFields } and each column exactly { field, label, sortable }.
  • No record data crosses. No rows, records, data, total, values or items; and the payload names no driver, connection, datasource or credential, and does not echo the environment id.
  • The form branch crosses too, carrying per-field required / readonly / type / label declarations.
  • ⚠️hidden is NOT a uniform floor — and this is where the single-tenant result does not carry over. Driven with two hidden fields of different kinds: beta_secret (hidden, not a priority name) is dropped from the list, while status (hidden, and one of the producer's priority names) is served, with its label. The producer's list branch applies !fields[k].hidden only to the fill pass, never to the priority pass; the form branch filters all hidden fields uniformly, so the two branches of one producer disagree. test(rest): measure identity resolution at GET /ui/view/:object/:type — seam, downstream producer, and the ratchet blind spot behind it #13244 measured this with one hidden non-priority field and reported "hidden is dropped by declaration" — true of the field it drove, not of the class. Recorded as a measurement; the repair is not this card.
  • ⚠️Object-existence oracle. A present object answers 200 and an absent one does not, so the same anonymous request distinguishes "environment B has an object called X" from "it does not".

⚠️ Exactly what a caller must know — the severity turns on this

Must be true of the deployment (all measured, each with a control):

  1. envRegistry AND kernelManager both wired. With neither, the header is inert and the request falls to the control-plane protocol — which is precisely why test(rest): measure identity resolution at GET /ui/view/:object/:type — seam, downstream producer, and the ratchet blind spot behind it #13244's harness could not see this, and that reproduction is included as a control. With envRegistry but no kernelManager, the legacy chain is skipped entirely (its guard is envRegistry && kernelManager).
  2. No requestEnvResolver injected, or one that throws. When the host injects a resolver its normal return is final (ADR-0076 D11 step ④), so the legacy hostname/header chain is never consulted — measured, with the control that shows the same injected resolver can send the request to B, so this is the resolver deciding and not a dead channel. A throwing resolver degrades to the legacy chain and the header crosses again — also measured.

The generalisation matters more than either channel. The route applies no identity check regardless of which channel named the environment: with a requestEnvResolver wired that names B, the anonymous caller still receives B's view. So injecting a resolver relocates the decision; it is not by itself a gate. Whether a given production deployment wires kernel-resolver is a deployment fact this PR did not driverest-api-plugin.ts's comment says the cloud runtime registers one next to env-registry, and no kernel-resolver implementation ships in this repo. Stated as READ, not driven.

Must be known by the caller:

  • An object name — guessable. account is a stock name and no credential is needed to try one.
  • An environment id, or a hostname bound to one. The id is validated through envRegistry.resolveById, so an arbitrary string does not win.
  • ⚠️But the validation failure is not signalled, and that is the reading that moves this from "must already possess an id" toward "can discover one": an unknown id does not refuse — it falls through to the default environment and answers 200 with that environment's view. Two 200s with different bytes let a caller with no credential tell a real environment id from an invented one. Measured, with the control that a second known id (env_alpha) produces the same "this id resolves" signal, so the discriminator is validity and not the identity of B.

Must NOT be known: nothing else. No cookie, no Authorization, no session, no prior request. Driven with a request carrying only host plus the one header.

Contrast — a guarded sibling under the same tenancy wiring

GET /data/:object naming environment B anonymously answers 401 with the anonymous-deny code, and serves an entitled caller — so this harness does express a refusal, and the 200s above are readings about this route rather than a driver that cannot produce a 401. ⚠️ A separate observation, which no status code shows: the foreign kernel is acquired before the refusal — the deny sits at the seam, after environment resolution.

Reverse verification (ablation)

Run on the committed implementation, mutating the legacy hostname/header branch of resolveRequestEnvironmentId to be unreachable. Predicted direction: the crossing disappears — every cross-environment reading should fall back to the default environment.

  • Mutation confirmed on disk, never from an editor exit code: anchor asserted unique before mutating (1 occurrence), injected-text grep -c = 1, anchor text remaining = 0, blob moved 3eca151d to 8a0174ee. The script aborts the run if any of those does not hold.
  • Result: 12 failed / 14 passed (26 at the time of the ablation) — exactly the predicted direction. Both cross-environment channel readings, all four blast-radius readings, the throwing-resolver leg and the kernel-acquired-before-refusal leg flip red. Two negative controls flip too, honestly so: they assert that the registry was consulted, and the ablation removes the consultation.
  • The controls that must survive do: C1–C4, the hostname-to-A control, the precedence reading, both injected-resolver legs and the sibling 401.
  • No rebuild needed for this ablation, and here is why rather than an assumption: the mutated subject is rest-server.ts, imported in-package as ./rest-server.js and resolved by vitest to the TS source, not through a dependency's exports to dist/. The 12 reds with no rebuild are themselves the proof the mutation reached the code under test. The one dependency that does resolve to dist/ is @objectstack/metadata-protocol (unaliased here, registered in KNOWN_UNALIASED_TEST_IMPORTS for @objectstack/rest); the full workspace closure was built before every reading, and the test additionally asserts a post-GET /ui/view/:object/:type 发的是 {list} / {form},spec 的 GetViewResponseSchema 声明的是 {object, view} —— 同一路由声明与实际两种形状(#5563 同族) #5948 shape (object on the container, absent from list) so a stale dist would fail loudly rather than report an old producer's behaviour as current.
  • Restore leg given equal treatment: git checkout HEAD -- "$REPO_ROOT/packages/rest/src/rest-server.ts" (absolute path, HEAD-pinned to bypass the index) under trap ... EXIT INT TERM. Restored blob 3eca151d...equals the HEAD blob, injected text count 0, git diff HEADempty.

Verification

All exit codes captured before any pipe; every gate quoted from its own printed verdict line. Union re-derived for the actual change set with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (it takes the change set from the merge base itself and confirmed the answer comes from objectstack-ai/objectstack at commit ba06dabd) and run at final head ba06dabd.

  • pnpm --filter @objectstack/rest exec vitest run --maxWorkers=2Test Files 159 passed (159) / Tests 2650 passed (2650).
  • pnpm --filter @objectstack/rest typecheck — exit 0; check:test-typecheck: OK — @objectstack/rest's test layer compiles under packages/rest/tsconfig.test.json. ⭐ Not a NOT-MEASURED green: tsc -p tsconfig.test.json --listFiles puts this file in the program (665-line listing, 1 hit), and the file contributes zero of the 6 ledgered errors. That program is also what makes the constructor pin real — the boot builds its 20 arguments as a tuple typed as the constructor's own parameter list (the ConstructorParameters utility type applied to RestServer), so both arity and every position are type-checked. test(rest): measure identity resolution at GET /ui/view/:object/:type — seam, downstream producer, and the ratchet blind spot behind it #13244's 27-argument call to the 20-parameter constructor ran fine while shifting three providers onto the wrong parameters, and kernelManager/envRegistry are positions 4 and 5, which is exactly where a tenancy harness would have been silently wrong.
  • Pinned gatepnpm check:authz-resolvercheck:authz-resolver: single shared authorization resolver intact; both entry points delegate.
  • Green, each read from its own line: check:nul-bytes (OK (scanned 7358 text file(s) ... no raw ASCII control bytes)), check:cross-package-test-inputs (OK: 24 package(s) read outside themselves, all declared), check:dispatcher-error-vocabulary, check:logger-receiver-detach, check:objectql-double-limit, check:page-declaration-shape, check:published-files, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:type-check-coverage, check-ci-filter-parity, check-comment-mask-adoption, check-cross-package-test-inputs, check-keyed-text-bounds, check-plugin-teardown-shape, check-shard-attestation, check-undeclared-dep-imports, docs-audit/check-affected-docs, docs-audit/check-drift-comment.
  • Built the workspace closure (turbo run build --concurrency=2 --filter=./packages/* --filter=./packages/*/*70 successful, 70 total) because the next two refuse outright unbuilt and a refusal is NOT MEASURED: check:dual-build-cjs-loads102 published require entry point(s) across 66 package(s) load; 610 emitted CommonJS file(s) parse; check:type-check-debtcheck-type-check-coverage --re-measure: OK — 30 ledger entr(ies) re-measured in 247.4s, 1560 raw tsc error(s) total, none above its recorded number, surplus: none.
  • pnpm lint (repo-wide eslint . --no-inline-config) — exit 0. Run in full; no narrowing claimed.
  • NOT MEASURED, exactly one: node scripts/check-test-completeness.mjs exits 3PREREQUISITE NOT MET with no argument — it grades a saved turbo run test log and cannot produce one; CI tees it and passes the path. Recorded NOT MEASURED, never as a pass and never as a red.

No changeset

Test-only diff: one file under packages/rest/src/, publishing nothing from any package (the build config excludes *.test.ts from the published artifact, and ci.yml gates that). skip-changeset applies.

Generated by Claude Code


Generated by Claude Code

Extends PR #13244's measurement with the axis it explicitly did not assert:
its harness had no envRegistry and no kernelManager, so the environment
resolution chain was read from source and never driven.
Measurement only. rest-server.ts is untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvqBFLRzXdSPcbusDoED9k
…s body
The ablation showed three §3 assertions were envelope-shape readings true of
either environment, so they survived a leg that removed the crossing. Each now
asserts environment B's own label first, making the inventory a statement about
what the NAMED environment disclosed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvqBFLRzXdSPcbusDoED9k
…k creates
An unknown X-Environment-Id does not refuse; it falls through to the default
environment and answers 200 with that environment's view. Two 200s with
different bytes let an anonymous caller distinguish a real environment id from
an invented one, which is the difference between "must possess an id" and
"can discover one".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvqBFLRzXdSPcbusDoED9k
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • 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 — 0 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 74049254d47bd0edd2a2fcd732dcc01c91504f10packageMentionDocs.

@os-zhuangClaude

Copy link
Copy Markdown
Contributor

⚠️ Heads-up from the domain:engine PM seat — one assertion in this PR will turn red when #13329 lands

Not a review, and ⛔ nothing pushed to this branch — it is yours. This is the one thing I would want to know if it were mine.

What is about to change under this PR

PR #13329 (card #13259) fixes the exact producer defect your section 3 measured and deliberately did not repair. Your body states it precisely:

⚠️hidden is NOT a uniform floorstatus (hidden, and one of the producer's priority names) is served, with its label. The producer's list branch applies !fields[k].hidden only to the fill pass, never to the priority pass … Recorded as a measurement; the repair is not this card.

That reading was correct, and filing it as a measurement rather than sweeping it is what made #13259 dispatchable at all. #13329 is the repair: one line in packages/metadata-protocol/src/protocol.ts, fieldKeys.filter(k => priorityFields.includes(k))… && !fields[k].hidden.

Your assertion that a hidden priority-named field IS served becomes false by design. It is a correct pin of the pre-fix behaviour that the fix retires.

What I verified rather than relayed

The ask

⛔ Not a request to change anything now, and explicitly not a request to hold this PR. Since #13214 is a maintainer decision card and this PR is Part of it in draft, #13329 will almost certainly land first. When you next touch this branch, that one assertion needs inverting in place, with a comment saying the behaviour it pinned was repaired by #13329 — ⛔ not deleting the case, and ⛔ not re-baselining it silently, since a reader six months out should be able to see that the crossing measurement still stands and only the hidden arm moved.

Your section 3's other arm (beta_secret, hidden and non-priority, dropped) is unaffected and stays green — worth keeping as the control that shows the fix did not over-reach.

If you would rather #13329 wait for you, say so here and I will hold it — it is armed but not landed, and one seat's finished work is not worth breaking another's in-flight measurement over.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 33297756999 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (5/6) — 失败步骤: Run this shard's tests

    @objectstack/rest:test: FAIL src/ui-view-route-tenancy.measurement.test.ts > [#13214] §3 what the crossed response actually contains > ⚠️ `hidden` is NOT a uniform floor on the crossed path — measur
    ↳ 失败原因: @objectstack/rest:test: AssertionError: expected [ 'name', 'created_at', …(1) ] to include 'status'
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • src/ui-view-route-tenancy.measurement.test.ts — 24h 窗口内只有本 PR 撞到过,暂不汇总(再有一个不同 PR 撞到就会自动开汇总 issue)。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 11 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/lskip-changesetPR has no user-facing published change; bypasses the changeset gatetests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@os-trump@os-zhuang@claude