From 27052754d6fa911d9fdff81b3b01f92a6af4023f Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Sun, 27 Sep 2026 01:37:58 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=96=20fix:=20allow=20read-only=20admin?= =?UTF-8?q?=20policy=20in=20the=20VS=20Code=20webview,=20redacting=20gatew?= =?UTF-8?q?ay=20URLs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- vscode/src/extension.ts | 8 +++-- vscode/src/orpcAllowlist.test.ts | 53 +++++++++++++++++++++++++++++++- vscode/src/orpcAllowlist.ts | 40 ++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 3 deletions(-) diff --git a/vscode/src/extension.ts b/vscode/src/extension.ts index 3d383e157e..34170e70eb 100644 --- a/vscode/src/extension.ts +++ b/vscode/src/extension.ts @@ -23,7 +23,11 @@ import type { UiConnectionStatus, UiWorkspace, } from "./webview/protocol"; -import { isAllowedOrpcPath, sanitizeWebviewOrpcInput } from "./orpcAllowlist"; +import { + isAllowedOrpcPath, + redactWebviewOrpcResult, + sanitizeWebviewOrpcInput, +} from "./orpcAllowlist"; import { parseWebviewToExtensionMessage } from "./parseWebviewToExtensionMessage"; import { openWorkspace } from "./workspaceOpener"; @@ -1667,7 +1671,7 @@ class XumChatViewProvider implements vscode.WebviewViewProvider, vscode.Disposab requestId: args.requestId, ok: true, kind: "value", - value: result, + value: redactWebviewOrpcResult(args.path, result), }); } catch (error) { if (controller.signal.aborted) { diff --git a/vscode/src/orpcAllowlist.test.ts b/vscode/src/orpcAllowlist.test.ts index 6128186a2f..f028ba81a2 100644 --- a/vscode/src/orpcAllowlist.test.ts +++ b/vscode/src/orpcAllowlist.test.ts @@ -1,6 +1,10 @@ import { describe, expect, test } from "bun:test"; -import { isAllowedOrpcPath, sanitizeWebviewOrpcInput } from "./orpcAllowlist"; +import { + isAllowedOrpcPath, + redactWebviewOrpcResult, + sanitizeWebviewOrpcInput, +} from "./orpcAllowlist"; describe("isAllowedOrpcPath", () => { test("allows known procedures", () => { @@ -79,3 +83,50 @@ describe("agents.list (#4751)", () => { }); }); }); + +describe("policy (#4739)", () => { + test("allows reading the effective policy and its change signal only", () => { + expect(isAllowedOrpcPath(["policy", "get"])).toBe(true); + expect(isAllowedOrpcPath(["policy", "onChanged"])).toBe(true); + expect(isAllowedOrpcPath(["policy", "refresh"])).toBe(false); + }); + + test("strips provider forcedBaseUrl from policy.get and keeps everything else", () => { + const response = { + source: "governor", + status: { state: "enforced" }, + policy: { + policyFormatVersion: "0.1", + providerAccess: [ + { + id: "openai", + forcedBaseUrl: "https://user:token@gateway.corp.example/v1", + allowedModels: ["gpt-5.6-terra"], + }, + { id: "anthropic", allowedModels: null }, + ], + mcp: { allowUserDefined: { stdio: false, remote: true } }, + runtimes: ["worktree"], + }, + }; + expect(redactWebviewOrpcResult(["policy", "get"], response)).toEqual({ + ...response, + policy: { + ...response.policy, + providerAccess: [ + { id: "openai", allowedModels: ["gpt-5.6-terra"] }, + { id: "anthropic", allowedModels: null }, + ], + }, + }); + // The input object is not mutated. + expect(response.policy.providerAccess[0].forcedBaseUrl).toBeDefined(); + }); + + test("passes other results and policy-less responses through unchanged", () => { + const noPolicy = { source: "none", status: { state: "disabled" }, policy: null }; + expect(redactWebviewOrpcResult(["policy", "get"], noPolicy)).toEqual(noPolicy); + const other = { forcedBaseUrl: "kept" }; + expect(redactWebviewOrpcResult(["providers", "getConfig"], other)).toBe(other); + }); +}); diff --git a/vscode/src/orpcAllowlist.ts b/vscode/src/orpcAllowlist.ts index a7cd1de041..a2fecbaa88 100644 --- a/vscode/src/orpcAllowlist.ts +++ b/vscode/src/orpcAllowlist.ts @@ -35,6 +35,10 @@ const ALLOWED_PROCEDURES = { // the agent picker and agent-cycle shortcut (#4751). agents.get (full prompt bodies) stays // blocked, and sanitizeWebviewOrpcInput limits the input to workspaces the webview is shown. agents: new Set(["list"]), + // Read-only admin policy (provider/model allowlists, runtime and MCP flags) so the model list + // matches what the backend enforces (#4739); onChanged only emits empty change signals. + // redactWebviewOrpcResult strips provider forcedBaseUrl before policy.get reaches the webview. + policy: new Set(["get", "onChanged"]), } as const; export function isAllowedOrpcPath(path: string[]): boolean { @@ -61,11 +65,47 @@ export function isAllowedOrpcPath(path: string[]): boolean { return ALLOWED_PROCEDURES.providers.has(procedure); case "agents": return ALLOWED_PROCEDURES.agents.has(procedure); + case "policy": + return ALLOWED_PROCEDURES.policy.has(procedure); default: return false; } } +/** + * Removes fields the webview does not need from results before they cross the bridge. + * + * policy.get: a provider's forcedBaseUrl is an internal gateway URL that could embed credentials, + * and no webview code reads it; only the allowlists and flags are forwarded. The input is not + * mutated. Every other result passes through unchanged. + */ +export function redactWebviewOrpcResult(path: string[], value: unknown): unknown { + if (path[0] !== "policy" || path[1] !== "get") { + return value; + } + if (typeof value !== "object" || value === null) { + return value; + } + const response = value as { policy?: unknown }; + const policy = response.policy as { providerAccess?: unknown } | null | undefined; + if (typeof policy !== "object" || policy === null || !Array.isArray(policy.providerAccess)) { + return value; + } + return { + ...response, + policy: { + ...policy, + providerAccess: policy.providerAccess.map((entry: unknown) => { + if (typeof entry !== "object" || entry === null) { + return entry; + } + const { forcedBaseUrl: _forcedBaseUrl, ...rest } = entry as Record; + return rest; + }), + }, + }; +} + export type SanitizedOrpcInput = { ok: true; input: unknown } | { ok: false; error: string }; /**