feat(runtime-host): bind capability providers to Client owners - #4187
Conversation
4f4b159 to
799c343
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for keeping the association at the Host access authority instead of accepting a caller-supplied owner tuple. The overall direction is sound: the Host derives and persists the provider owner, the capability coordinator consumes that fact, unrelated providers remain invisible, and multiple companions fail closed.
I reviewed exact head 799c343d79f57b4af7492ea70cdebb240beea136. The exact-head checks are green, but I found one P1 trust-boundary issue that must be fixed before merge.
P1 — an unbound remote-owner credential can impersonate another Client for provider selection (category ③).
The handshake validates hello.clientInstanceId only when the access authority already contains a credential-bound Client ID. An unbound remote_owner credential can therefore declare an arbitrary Client ID. The new coordinator then matches companion providers using only { principalId, clientInstanceId }.
A reachable path is:
- a provider is associated with
(owner, client-A); - another unbound remote-owner credential exists for the same principal;
- its holder connects while declaring
clientInstanceId=client-A; - the Host accepts that identity and selects client-A companion provider for the impersonating Client.
This crosses an authenticated Client boundary and can expose or invoke another Client MCP capabilities, so P1 is appropriate. The smallest fix is to let only a Host-authenticated, credential-bound initiating Client identity participate in companion matching. A Client ID supplied only in hello must fail closed for this association. Please add an authenticated-WebSocket regression proving that an unbound credential with the same principal cannot claim a bound Client ID or obtain its provider.
I also found one non-blocking P2 recovery issue. createAccessCredentialFile() always writes schema 2, including ordinary credential issue, replace, or revoke operations where no capabilityOwner has ever been stored. An older Host then rejects the file after any such mutation, even though there is no owner association to protect. This is broader than the PR description that says the downgrade fence begins once an association is written. Either retain schema 1 until a credential actually carries capabilityOwner, or explicitly make this a global schema fence and cover ordinary mutation plus rollback behavior.
A small P3 cleanup: ResolvedRuntimeHostAccessIssue.capabilityOwner is imported and declared but never populated by the resolver, so it can be removed.
The production authority shape should otherwise remain as it is; there is no need for a second association registry or lifecycle manager.
Review analysis was assisted by Codex and independent @reviewer agents. Astro-Han verified the exact head, authenticated handshake and Session selection path, schema persistence behavior, reachability, CI evidence, and severity judgment, and owns this review.
中文对照
谢谢你把关联关系放在 Host access authority 中,而不是接受调用方自报的 owner tuple。整体方向是正确的:Host 派生并持久化 provider owner,capability coordinator 只消费这个事实,无关 provider 不可见,多个 companion 也会 fail closed。
我审查了精确 head 799c343d79f57b4af7492ea70cdebb240beea136。当前 exact-head checks 全绿,但有一个必须在合并前修复的 P1 信任边界问题。
P1 — 未绑定的 remote-owner credential 可以在 provider 选择时冒充另一个 Client(类别③)。
只有 access authority 已经包含 credential-bound Client ID 时,握手才会校验 hello.clientInstanceId。因此,未绑定的 remote_owner credential 可以声明任意 Client ID。新的 coordinator 随后只用 { principalId, clientInstanceId } 匹配 companion provider。
可达路径是:
- provider 已关联到
(owner, client-A); - 同一 principal 下还存在另一枚未绑定的 remote-owner credential;
- 持有者连接时声明
clientInstanceId=client-A; - Host 接受该身份,并为冒充者选择 client-A 的 companion provider。
这跨越了已认证 Client 边界,可能暴露或调用另一个 Client 的 MCP capability,因此 P1 合理。最小修复是:只有经过 Host 验证、由 credential 绑定的 initiating Client identity 才能参与 companion matching;只有 hello 自报 ID 的 Client 必须 fail closed。请补一条 authenticated-WebSocket 回归测试,证明同 principal 的未绑定 credential 不能冒用已绑定 Client ID,也不能获得对应 provider。
另有一个不阻塞的 P2 恢复问题。createAccessCredentialFile() 会始终写 schema 2,包括从未存储过 capabilityOwner 的普通 issue、replace 或 revoke。这样即使没有任何 owner association,旧 Host 也会在一次普通凭据变更后拒绝该文件。这比 PR 正文所说的“写入 association 后才启用 downgrade fence”更宽。可以在确实出现 capabilityOwner 前继续写 schema 1;或者明确把它定义为全局 schema fence,并覆盖普通 mutation 和 rollback 行为。
还有一个很小的 P3 清理项:ResolvedRuntimeHostAccessIssue.capabilityOwner 被导入并声明,但 resolver 从未填充,可以删除。
除此之外,生产 authority 结构应继续保持现状,不需要再增加第二套 association registry 或 lifecycle manager。
本次审查分析由 Codex 和独立的 @reviewer 子代理协助;Astro-Han 核验了精确 head、认证握手与 Session 选择路径、schema 持久化行为、可达性、CI 证据和问题分级,并对本次 Review 负责。
799c343 to
0170c25
Compare
Resolve provider ownership from an active Client-bound owner credential and persist only the Host-verified identity. Fence the durable schema and protocol epoch so older Hosts cannot silently drop the association. Generated-by: Codex
Prefer the initiating Client's direct provider, then one exact Host-bound companion. Keep unrelated providers invisible to remote Clients and fail closed when more than one companion claims the same owner. Generated-by: Codex
Require the initiating Client identity to come from its access credential before it can match an associated capability provider. Reject an unbound credential that claims an already-bound Client ID over the authenticated handshake. Keep ordinary access files on schema 3 and publish schema 4 only after a capability-owner association commits, so failed or unrelated mutations do not widen the downgrade fence. Generated-by: Codex <noreply@openai.com>
0170c25 to
84cf0fc
Compare
|
Thanks — addressed on
The final-stack focused Runtime Host/CLI suites pass 144/144; the full Runtime Host run passed 1,411 with 9 skips and no failures before the final non-overlapping main rebase. |
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks—the current head closes the earlier trust-boundary and compatibility findings without introducing another association authority.
The initiating Client can now select a companion only when its handshake identity matches the Client identity authenticated by its credential. An unbound same-principal credential therefore cannot claim another Client ID and inherit its provider. The access-file writer also remains on schema 3 for ordinary credentials and advances to schema 4 only after a real capabilityOwner association exists; the unused resolved-issue field was removed.
The Host access authority remains the single owner of the association, while the capability coordinator only consumes the authenticated fact. I found no remaining P0–P2 issue on exact head 84cf0fcbd1b3e4042fe478faa6a06f78cff78ded. All current hosted checks are green.
中文对照
谢谢,当前 head 已经闭合此前的信任边界与兼容性问题:只有 credential 实际绑定的 Client identity 才能选择 companion;普通凭据文件继续保持 schema 3,真正写入 owner association 后才升级到 schema 4。关联关系仍只有 Host access authority 一个权威。当前没有剩余 P0–P2,CI 全绿。
AI-assisted review: Codex traced the authenticated identity and persistence paths and drafted this review; the maintainer verified the fixes and severity.
Summary
Add the Runtime Host association contract needed for a remote TUI to use a separate Client Capability provider without exposing that provider to unrelated Clients.
{ principalId, clientInstanceId }.Refs #3838
Review focus
This keeps one authority for the association. The access authority derives the Client identity from an existing bound credential; the capability coordinator only consumes that derived fact. There is no caller-supplied identity tuple, name-based matching, association registry, or second lifecycle manager.
Client-bound credential rotation preserves the binding requirement so a rotated owner can remain the trusted source for companion credentials.
This slice intentionally does not launch or supervise a TUI companion process and does not add TUI credential storage or UI. Those lifecycle and end-to-end changes remain in the follow-up PR stacked on this contract.
Verification
git diff --check origin/main...HEADAI use
Select exactly one:
Tool(s) and scope: Codex traced the existing access, Client identity, Session binding, and provider lifecycle boundaries; implemented the credential association and exact provider selection; repaired the authenticated identity and conditional schema fences from review; added regression coverage; and performed architecture and simplification reviews. The commits carry a
Generated-by: Codextrailer.Checklist
Does this PR entail a change in behavior?