From 5d9f026e553c8ca950862f86f77606c4f38ef082 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Wed, 12 Aug 2026 21:35:21 +0800 Subject: [PATCH] fix(settings): correct the exposure analysis and satisfy the coverage and lint gates The Agent Note claimed the plugin inventory page already exposed every mounted plugin's effective configuration to the same browser, so the removed allowlist blocked nothing a caller could not already read. That is false: PluginInventoryEntry carries entryId, moduleName, enabled, and fiberPhase, and the page's configuration row renders an enabled tag, not a stored value. The allowlist did keep an unlisted namespace's resolved, base, and user values off the wire; the note now says so and rests the decision on the loopback pin, secret redaction, the user's own document, and the fact that permission and agent-presets were already served. The post-await disposal check reads through an opaque method, mirroring the settings seam's isStopped(): control flow narrowed the field to false across the await, so the lint gate saw the guard as dead. --- .../2026-08-12-plugin-owned-settings-surface.i18n.yaml | 4 ++-- .../2026-08-12-plugin-owned-settings-surface.md | 6 ++++-- .../2026-08-12-plugin-owned-settings-surface.zh.md | 6 ++++-- .../client/ui-plugin-config/src/client/section-store.ts | 9 +++++++-- .../client/ui-plugin-config/tests/stores.client.spec.ts | 2 +- 5 files changed, 18 insertions(+), 9 deletions(-) diff --git a/.agents/notes/implemented/architecture/2026-08-12-plugin-owned-settings-surface.i18n.yaml b/.agents/notes/implemented/architecture/2026-08-12-plugin-owned-settings-surface.i18n.yaml index 6abe3f25e7..9b3c3a967c 100644 --- a/.agents/notes/implemented/architecture/2026-08-12-plugin-owned-settings-surface.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-08-12-plugin-owned-settings-surface.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/architecture/2026-08-12-plugin-owned-settings-surface.md -2026-08-12-plugin-owned-settings-surface.md: 3e6b75e8516312dc72313541b05e3dfb9f57140f -2026-08-12-plugin-owned-settings-surface.zh.md: ad06a25c5cb9023f15ca39d6302049c30fa36ce3 +2026-08-12-plugin-owned-settings-surface.md: dd044659bb336a5a8add19650b968c069ce24ecc +2026-08-12-plugin-owned-settings-surface.zh.md: ebe7dc87cb595628b5a072ec923768a07797eff0 diff --git a/.agents/notes/implemented/architecture/2026-08-12-plugin-owned-settings-surface.md b/.agents/notes/implemented/architecture/2026-08-12-plugin-owned-settings-surface.md index 3e6b75e851..dd044659bb 100644 --- a/.agents/notes/implemented/architecture/2026-08-12-plugin-owned-settings-surface.md +++ b/.agents/notes/implemented/architecture/2026-08-12-plugin-owned-settings-surface.md @@ -30,9 +30,11 @@ Keying makes absence the signal, and that is what removes the bookkeeping the pr ## What the allowlist protected -The removed gate was not the boundary it read as. Every `settings.*` method sits in `PRIVILEGED_METHODS` (`packages/client/connection`), so a non-loopback or cross-origin request is refused with 403 before reaching this code; `role('secret')` fields are structurally stripped from every layer of every response; and the document the plane edits is the user's own `settings.yaml`. The read the gate blocked was already available to the same browser through the plugin inventory page, which lists every mounted plugin with its effective configuration. The writes it blocked were the least consequential ones on the plane: `permission` (which can widen the approval preset) and `agent-presets` (which decides what a session mounts) were both already served. +The gate did keep one thing off the wire, and this note states it plainly because the decision has to survive the accurate version: a registered namespace the list did not name never had its resolved, `base`, or `user` values reach the browser at all. The plugin inventory page is not a substitute — `PluginInventoryEntry` carries `entryId`, `moduleName`, `enabled`, and `fiberPhase`, and its "configuration" row renders an enabled/disabled tag, never a stored value. -The one namespace whose exposure actually changes is `agent-default-model`. It has no browser half, so nothing renders it. +What the gate was not is the boundary its position suggested. Every `settings.*` method sits in `PRIVILEGED_METHODS` (`packages/client/connection`), so a non-loopback or cross-origin request is refused with 403 before reaching this code; `role('secret')` fields are structurally stripped from every layer of every response; and the document the plane edits is the user's own `settings.yaml`, which the same settings page offers to open. The writes it did not block were also the consequential ones: `permission` (which can widen the approval preset) and `agent-presets` (which decides what a session mounts) were both already served. + +So the exposure this change actually adds, in this repository, is one namespace: `agent-default-model`, whose two fields name a provider and a model and which no browser half renders. A future namespace whose values genuinely must not cross the wire is answered per field by `role('secret')` — finer than a namespace switch, and already enforced. ## Alternatives considered diff --git a/.agents/notes/implemented/architecture/2026-08-12-plugin-owned-settings-surface.zh.md b/.agents/notes/implemented/architecture/2026-08-12-plugin-owned-settings-surface.zh.md index ad06a25c5c..ebe7dc87cb 100644 --- a/.agents/notes/implemented/architecture/2026-08-12-plugin-owned-settings-surface.zh.md +++ b/.agents/notes/implemented/architecture/2026-08-12-plugin-owned-settings-surface.zh.md @@ -30,9 +30,11 @@ Status: implemented ## 白名单实际护住了什么 -被删掉的这道门并不是它读起来的那种边界。每个 `settings.*` 方法都在 `PRIVILEGED_METHODS` 里（`packages/client/connection`），非回环或跨源请求在到达这段代码之前就以 403 被拒；`role('secret')` 字段在每种响应的每一层都被结构性剥离；而这个面所编辑的文档，本就是用户自己的 `settings.yaml`。这道门挡住的读取，同一个浏览器早已能从插件清单页拿到——那一页列出每个已挂载插件及其 effective configuration。它挡住的写入，则是整个面上最无关紧要的那些：`permission`（能放宽审批预设）与 `agent-presets`（决定一个会话挂载什么）本来就已被服务。 +这道门确实挡住了一样东西，本 note 如实写出，因为这个决策必须在准确版本下也站得住：不在名单上的已注册命名空间，其 resolved、`base` 与 `user` 值根本不会抵达浏览器。插件清单页不能替代它——`PluginInventoryEntry` 携带的是 `entryId`、`moduleName`、`enabled` 与 `fiberPhase`，它那一行「configuration」渲染的是启用／停用标签，从不是任何已存值。 -暴露状况真正发生变化的只有 `agent-default-model` 一个命名空间。它没有浏览器半侧，因此没有任何界面渲染它。 +这道门不是的，是它所处位置暗示的那种边界。每个 `settings.*` 方法都在 `PRIVILEGED_METHODS` 里（`packages/client/connection`），非回环或跨源请求在到达这段代码之前就以 403 被拒；`role('secret')` 字段在每种响应的每一层都被结构性剥离；而这个面所编辑的文档，本就是用户自己的 `settings.yaml`，同一个设置页还提供了打开它的入口。它没有挡住的写入，恰恰是有分量的那些：`permission`（能放宽审批预设）与 `agent-presets`（决定一个会话挂载什么）本来就已被服务。 + +因此本次改动在本仓库实际新增的暴露面是一个命名空间：`agent-default-model`——它的两个字段指明一个提供方与一个模型，且没有任何浏览器半侧渲染它。将来若某个命名空间的值确实不该跨越协议，由 `role('secret')` 逐字段作答：比整命名空间开关更精细，而且已经在执行。 ## Alternatives considered diff --git a/packages/client/ui-plugin-config/src/client/section-store.ts b/packages/client/ui-plugin-config/src/client/section-store.ts index b12c014a0d..367243822e 100644 --- a/packages/client/ui-plugin-config/src/client/section-store.ts +++ b/packages/client/ui-plugin-config/src/client/section-store.ts @@ -60,12 +60,17 @@ export class PluginConfigSectionController { private readonly entries: () => readonly StoredEntry[], ) {} + /** Opaque read of {@link disposed}: control flow cannot narrow it across awaits. */ + private isDisposed(): boolean { + return this.disposed + } + /** * Re-read the served namespaces from the Host and republish. * @returns settlement after the read, or immediately once disposed. */ async load(): Promise { - if (this.disposed) return + if (this.isDisposed()) return const generation = ++this.generation let response: Awaited> try { @@ -75,7 +80,7 @@ export class PluginConfigSectionController { // or reconnect reads again. return } - if (this.disposed || generation !== this.generation || !response.result.ok) return + if (this.isDisposed() || generation !== this.generation || !response.result.ok) return this.served = response.result.value.namespaces.map(view => view.ns) this.loaded = true this.publish() diff --git a/packages/client/ui-plugin-config/tests/stores.client.spec.ts b/packages/client/ui-plugin-config/tests/stores.client.spec.ts index 30a537a38f..54257f74fc 100644 --- a/packages/client/ui-plugin-config/tests/stores.client.spec.ts +++ b/packages/client/ui-plugin-config/tests/stores.client.spec.ts @@ -603,7 +603,7 @@ describe('PluginConfigSectionController', () => { const settings = settingsApi(['bash']) const controller = new PluginConfigSectionController(settings.api, () => ledger('bash')) await controller.load() - settings.describe.mockRejectedValueOnce(new Error('offline') as never) + settings.describe.mockRejectedValueOnce(new Error('offline')) await controller.load()