From d90993cd6fd07ae4e15373c87389072e97e7c5d7 Mon Sep 17 00:00:00 2001 From: colafornia Date: Tue, 10 Nov 2026 23:25:47 +0800 Subject: [PATCH 1/4] fix(agent): preserve grants when permission requests are disabled Only advance the permission boundary for access changes or explicit revocation. Keep the metadata regression with the boundary behavior. Generated-by: Codex --- plugins/agent/native/README.md | 2 + plugins/agent/native/README.zh-CN.md | 2 + plugins/agent/native/src/owner.rs | 2 +- .../agent/native/src/owner/driver/tests.rs | 4 +- .../owner/driver/tests/permission_boundary.rs | 157 ++++++++++++++++++ 5 files changed, 165 insertions(+), 2 deletions(-) create mode 100644 plugins/agent/native/src/owner/driver/tests/permission_boundary.rs diff --git a/plugins/agent/native/README.md b/plugins/agent/native/README.md index 15436815cc..3b72b27ee1 100644 --- a/plugins/agent/native/README.md +++ b/plugins/agent/native/README.md @@ -175,3 +175,5 @@ compaction produces a text summary. Host freezes a single protocol operation; th `NativeCompaction` only at stream completion. Engine and EventLog do not extract provider-options JSON for this result. Checkpoint headers retain the producing-step reference, digest and token estimate; baseline reads derive producer/model facts from that original request. + +Disabling new requests preserves existing valid grants. Changing base access or explicitly revoking grants advances the permission boundary. diff --git a/plugins/agent/native/README.zh-CN.md b/plugins/agent/native/README.zh-CN.md index d691bf09c9..f98f7d9180 100644 --- a/plugins/agent/native/README.zh-CN.md +++ b/plugins/agent/native/README.zh-CN.md @@ -154,3 +154,5 @@ ChatGPT 冷缓存或到期刷新在逻辑请求准备时等待，后台压缩的 订阅 Provider 仅在连接开启该选项时，在解析后的能力中选择原生压缩。关闭后仍可回放已有原生 checkpoint；下一次压缩生成文字摘要。Host 冻结一个协议操作，Responses 适配器负责追加触发命令， 并只在流完成时返回有类型的 `NativeCompaction`。Engine 和 EventLog 无需再从 provider options JSON 提取结果。 检查点头部只保留来源步骤引用、内容摘要和 token 估算，baseline 读取时从原始请求派生 producer/model。 + +关闭新增权限申请会保留已有有效授权；修改基础访问权限或明确撤销授权才会推进权限边界。 diff --git a/plugins/agent/native/src/owner.rs b/plugins/agent/native/src/owner.rs index e44be2b6c6..9a093ea842 100644 --- a/plugins/agent/native/src/owner.rs +++ b/plugins/agent/native/src/owner.rs @@ -1101,7 +1101,7 @@ impl Owner { serde_json::from_value(record.metadata.configuration.clone()) .map_err(|error| format!("agent_session_configuration_invalid: {error}"))?; let policy_changed = input.permissions.as_ref().is_some_and(|change| { - change.settings != configuration.permissions || change.revoke_grants + change.settings.access != configuration.permissions.access || change.revoke_grants }); let guard = if policy_changed { let busy = self.runs.state().await.busy(&input.session_id) diff --git a/plugins/agent/native/src/owner/driver/tests.rs b/plugins/agent/native/src/owner/driver/tests.rs index aaf38ee9a4..0d0461b7fe 100644 --- a/plugins/agent/native/src/owner/driver/tests.rs +++ b/plugins/agent/native/src/owner/driver/tests.rs @@ -414,7 +414,7 @@ impl Rig { let methods: &[&str] = if model_test { &["create", "models", "send", "work"] } else { - &["create", "history", "source", "search", "list", "execution"] + &["create", "history", "source", "search", "list", "execution", "metadata"] }; let mut endpoints = BTreeMap::from([( "agent".into(), @@ -3356,3 +3356,5 @@ mod replay_origin_regressions { } mod tool_history; + +mod permission_boundary; diff --git a/plugins/agent/native/src/owner/driver/tests/permission_boundary.rs b/plugins/agent/native/src/owner/driver/tests/permission_boundary.rs new file mode 100644 index 0000000000..25fbbe40ca --- /dev/null +++ b/plugins/agent/native/src/owner/driver/tests/permission_boundary.rs @@ -0,0 +1,157 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +use super::*; + +async fn update_permissions( + rig: &Rig, + session: &str, + expected_revision: u64, + access: &str, + request_permissions: bool, + revoke_grants: bool, +) -> TestResult<()> { + let mut call = rig + .client + .as_ref() + .unwrap() + .call( + "agent", + "metadata".into(), + json!({ + "sessionId":session, + "expectedRevision":expected_revision, + "title":"Permission metadata regression", + "labels":[], + "permissions":{ + "settings":{"access":access,"requestPermissions":request_permissions}, + "revokeGrants":revoke_grants, + }, + }), + Attachments::new(), + ) + .await?; + call.start().await?; + assert!(matches!(call.receive().await?, Reply::Prepared { .. })); + let Reply::Mutation { reply, cleanup } = + call.confirm(json!({"test":"permission-boundary"})).await? + else { + return Err("metadata did not return a mutation receipt".into()); + }; + cleanup?; + let Receipt::Recorded { + outcome: + OperationOutcome::Returned { + result: OperationResult::Success { value }, + }, + cleanup: OperationCleanup::Confirmed, + } = &reply.receipt + else { + return Err(format!("metadata did not commit: {:?}", reply.receipt).into()); + }; + assert_eq!(value["kind"], "committed", "metadata mutation: {value}"); + call.close().await?; + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn permission_request_toggle_preserves_boundary_but_access_and_revoke_advance_it() { + let mut rig = Rig::new().unwrap(); + let outcome = AssertUnwindSafe(tokio::time::timeout(Duration::from_secs(60), async { + let session = rig.start().await?; + let owner = rig.captured.lock().unwrap().clone().unwrap(); + let initial = owner.mutable_session(&session).await?; + let basis = initial.configuration.permission_basis(); + let boundary = initial.configuration.boundary_revision; + let initial_access = serde_json::to_value(initial.configuration.permissions.access)?; + let initial_access = initial_access + .as_str() + .ok_or("permission access is not a string")?; + let changed_access = if initial_access == "read_only" { + "workspace_write" + } else { + "read_only" + }; + let initial_requests = initial.configuration.permissions.request_permissions; + let mut previous_revision = initial.revision; + for requests in [!initial_requests, initial_requests] { + update_permissions( + &rig, + &session, + previous_revision, + initial_access, + requests, + false, + ) + .await?; + let current = owner.mutable_session(&session).await?; + assert_eq!( + current.configuration.permissions.request_permissions, + requests + ); + assert_eq!(current.configuration.boundary_revision, boundary); + assert_eq!( + current.configuration.permission_basis(), + basis, + "request availability must not change the basis attached to existing grants" + ); + assert!(current.revision > previous_revision); + previous_revision = current.revision; + } + update_permissions( + &rig, + &session, + previous_revision, + changed_access, + false, + false, + ) + .await?; + let changed = owner.mutable_session(&session).await?; + assert_eq!(changed.configuration.boundary_revision, boundary + 1); + assert_ne!(changed.configuration.permission_basis(), basis); + update_permissions( + &rig, + &session, + changed.revision, + changed_access, + false, + true, + ) + .await?; + let revoked = owner.mutable_session(&session).await?; + assert_eq!(revoked.configuration.boundary_revision, boundary + 2); + assert_ne!( + revoked.configuration.permission_basis(), + changed.configuration.permission_basis() + ); + Ok::<_, Box>(()) + })) + .catch_unwind() + .await; + let cleanup = rig.finish().await; + assert!( + matches!(&outcome, Ok(Ok(Ok(())))), + "permission metadata regression: {outcome:?}" + ); + assert!( + cleanup.is_empty(), + "permission metadata fixture cleanup: {cleanup:?}" + ); +} From 95177834065763b9913195eb39240111e2f67010 Mon Sep 17 00:00:00 2001 From: colafornia Date: Tue, 10 Nov 2026 23:54:18 +0800 Subject: [PATCH 2/4] test(agent): simplify permission boundary regression coverage Keep four Host metadata steps covering request toggles, access changes, and explicit revocation. Compare persisted settings and permission basis without redundant revision assertions or string-based access selection. Generated-by: Codex --- .../agent/native/src/owner/driver/tests.rs | 10 ++- .../owner/driver/tests/permission_boundary.rs | 82 ++++++------------- 2 files changed, 32 insertions(+), 60 deletions(-) diff --git a/plugins/agent/native/src/owner/driver/tests.rs b/plugins/agent/native/src/owner/driver/tests.rs index 0d0461b7fe..5b126d154f 100644 --- a/plugins/agent/native/src/owner/driver/tests.rs +++ b/plugins/agent/native/src/owner/driver/tests.rs @@ -414,7 +414,15 @@ impl Rig { let methods: &[&str] = if model_test { &["create", "models", "send", "work"] } else { - &["create", "history", "source", "search", "list", "execution", "metadata"] + &[ + "create", + "history", + "source", + "search", + "list", + "execution", + "metadata", + ] }; let mut endpoints = BTreeMap::from([( "agent".into(), diff --git a/plugins/agent/native/src/owner/driver/tests/permission_boundary.rs b/plugins/agent/native/src/owner/driver/tests/permission_boundary.rs index 25fbbe40ca..005ab5a2c3 100644 --- a/plugins/agent/native/src/owner/driver/tests/permission_boundary.rs +++ b/plugins/agent/native/src/owner/driver/tests/permission_boundary.rs @@ -18,12 +18,13 @@ */ use super::*; +use maka_agent_api::contract::WorkspaceAccess; async fn update_permissions( rig: &Rig, session: &str, expected_revision: u64, - access: &str, + access: WorkspaceAccess, request_permissions: bool, revoke_grants: bool, ) -> TestResult<()> { @@ -76,71 +77,34 @@ async fn permission_request_toggle_preserves_boundary_but_access_and_revoke_adva let outcome = AssertUnwindSafe(tokio::time::timeout(Duration::from_secs(60), async { let session = rig.start().await?; let owner = rig.captured.lock().unwrap().clone().unwrap(); - let initial = owner.mutable_session(&session).await?; - let basis = initial.configuration.permission_basis(); - let boundary = initial.configuration.boundary_revision; - let initial_access = serde_json::to_value(initial.configuration.permissions.access)?; - let initial_access = initial_access - .as_str() - .ok_or("permission access is not a string")?; - let changed_access = if initial_access == "read_only" { - "workspace_write" - } else { - "read_only" + let mut previous = owner.mutable_session(&session).await?; + let initial = previous.configuration.permissions.clone(); + let changed_access = match initial.access { + WorkspaceAccess::ReadOnly => WorkspaceAccess::WorkspaceWrite, + WorkspaceAccess::WorkspaceWrite | WorkspaceAccess::DangerFullAccess => { + WorkspaceAccess::ReadOnly + } }; - let initial_requests = initial.configuration.permissions.request_permissions; - let mut previous_revision = initial.revision; - for requests in [!initial_requests, initial_requests] { - update_permissions( - &rig, - &session, - previous_revision, - initial_access, - requests, - false, - ) - .await?; + for (access, requests, revoke, preserves_basis) in [ + (initial.access, !initial.request_permissions, false, true), + (initial.access, initial.request_permissions, false, true), + (changed_access, false, false, false), + (changed_access, false, true, false), + ] { + update_permissions(&rig, &session, previous.revision, access, requests, revoke).await?; let current = owner.mutable_session(&session).await?; assert_eq!( - current.configuration.permissions.request_permissions, - requests + serde_json::to_value(¤t.configuration.permissions)?, + json!({"access":access,"requestPermissions":requests}), ); - assert_eq!(current.configuration.boundary_revision, boundary); assert_eq!( - current.configuration.permission_basis(), - basis, - "request availability must not change the basis attached to existing grants" + current.configuration.permission_basis() + == previous.configuration.permission_basis(), + preserves_basis, + "permission basis after access={access:?}, requests={requests}, revoke={revoke}", ); - assert!(current.revision > previous_revision); - previous_revision = current.revision; + previous = current; } - update_permissions( - &rig, - &session, - previous_revision, - changed_access, - false, - false, - ) - .await?; - let changed = owner.mutable_session(&session).await?; - assert_eq!(changed.configuration.boundary_revision, boundary + 1); - assert_ne!(changed.configuration.permission_basis(), basis); - update_permissions( - &rig, - &session, - changed.revision, - changed_access, - false, - true, - ) - .await?; - let revoked = owner.mutable_session(&session).await?; - assert_eq!(revoked.configuration.boundary_revision, boundary + 2); - assert_ne!( - revoked.configuration.permission_basis(), - changed.configuration.permission_basis() - ); Ok::<_, Box>(()) })) .catch_unwind() From 6c093537db71a74d2db59a3e5c5d515ad6087811 Mon Sep 17 00:00:00 2001 From: colafornia Date: Wed, 11 Nov 2026 00:48:07 +0800 Subject: [PATCH 3/4] fix(workbench): preserve grants when changing permission requests Remove implicit grant revocation from ordinary permission setting updates. Let the Host metadata owner advance the boundary for access changes and explicit revocation, and cover the UI controller request through its existing control seam. Generated-by: Codex --- .../agent/conversations/ui/SessionSurface.tsx | 4 +- .../maka/workbench/app/sessionPermissions.ts | 2 +- .../tests/session-permissions.test.mjs | 96 +++++++++++++++++++ 3 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 products/maka/workbench/tests/session-permissions.test.mjs diff --git a/plugins/agent/conversations/ui/SessionSurface.tsx b/plugins/agent/conversations/ui/SessionSurface.tsx index c0e2feb5fb..2500ca4e5e 100644 --- a/plugins/agent/conversations/ui/SessionSurface.tsx +++ b/plugins/agent/conversations/ui/SessionSurface.tsx @@ -1136,7 +1136,7 @@ export function createConversationUi( expectedRevision: session.revision, title: session.title, labels: session.labels, - permissions: { settings, revokeGrants: true }, + permissions: { settings, revokeGrants: false }, }); if (changed.kind !== 'committed') throw new UiError('Sandbox settings rejected: {reason}', { @@ -1471,7 +1471,7 @@ export function createConversationUi( access: access ?? session.permissions.access, requestPermissions: requests ?? session.permissions.requestPermissions, }, - revokeGrants: true, + revokeGrants: false, }, }); if (changed.kind !== 'committed') diff --git a/products/maka/workbench/app/sessionPermissions.ts b/products/maka/workbench/app/sessionPermissions.ts index 4ebe276338..c7a3ecdf9b 100644 --- a/products/maka/workbench/app/sessionPermissions.ts +++ b/products/maka/workbench/app/sessionPermissions.ts @@ -65,7 +65,7 @@ export function createSessionPermissions(host: Host, sessionId: string, control: expectedRevision: session.revision, title: session.title, labels: [...session.labels], - permissions: { settings, revokeGrants: true }, + permissions: { settings, revokeGrants: false }, }; pending.set(true); try { diff --git a/products/maka/workbench/tests/session-permissions.test.mjs b/products/maka/workbench/tests/session-permissions.test.mjs new file mode 100644 index 0000000000..d3b1ff2556 --- /dev/null +++ b/products/maka/workbench/tests/session-permissions.test.mjs @@ -0,0 +1,96 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { registerHooks } from 'node:module'; +import test from 'node:test'; + +const appRoot = new URL('../app/', import.meta.url); +const native = { + nsh: `export const signal = initial => { + let value = initial; + const read = () => value; + read.set = next => { value = next; }; + return read; + }; + export const cleanup = () => {};`, + locale: `export const tr = key => key;`, +}; +const hooks = registerHooks({ + resolve(specifier, context, next) { + const key = + context.parentURL?.startsWith(appRoot.href) && specifier === './locale' + ? 'locale' + : specifier; + if (key in native) + return { url: `data:text/javascript,${encodeURIComponent(native[key])}`, shortCircuit: true }; + if ( + context.parentURL?.startsWith(appRoot.href) && + specifier.startsWith('.') && + !/\.\w+$/.test(specifier) + ) + return next(`${specifier}.ts`, context); + return next(specifier, context); + }, +}); +const { createSessionPermissions } = await import(new URL('sessionPermissions.ts', appRoot)); +hooks.deregister(); + +test('changing permission settings leaves grant revocation to the Host', async () => { + let session = { + id: 'session', + revision: 4, + title: 'Permission settings', + labels: [], + permissions: { access: 'read_only', requestPermissions: true }, + }; + const sent = []; + const control = async (method, input) => { + assert.equal(method, 'metadata'); + sent.push(input); + session = { + ...session, + revision: session.revision + 1, + permissions: input.permissions.settings, + }; + return { kind: 'committed', session }; + }; + const permissions = createSessionPermissions({}, session.id, control); + permissions.observe(session); + + await permissions.select({ + session, + settings: { ...session.permissions, requestPermissions: false }, + }); + assert.deepEqual(sent[0].permissions, { + settings: { access: 'read_only', requestPermissions: false }, + revokeGrants: false, + }); + assert.equal(permissions.error(), ''); + + await permissions.select({ + session, + settings: { ...session.permissions, access: 'workspace_write' }, + }); + assert.deepEqual(sent[1].permissions, { + settings: { access: 'workspace_write', requestPermissions: false }, + revokeGrants: false, + }); + assert.equal(permissions.error(), ''); +}); From a2119a1f5863a5f460ebd9063e0b7d6984409345 Mon Sep 17 00:00:00 2001 From: colafornia Date: Wed, 11 Nov 2026 01:00:23 +0800 Subject: [PATCH 4/4] test(workbench): focus permission toggle regression Keep one real-controller request regression for disabling permission requests. Remove the second settings input, simulated session updates, and repeated empty-error assertions; Host coverage retains access-change and explicit-revocation behavior. Generated-by: Codex --- .../tests/session-permissions.test.mjs | 41 +++++++------------ 1 file changed, 14 insertions(+), 27 deletions(-) diff --git a/products/maka/workbench/tests/session-permissions.test.mjs b/products/maka/workbench/tests/session-permissions.test.mjs index d3b1ff2556..800ddbff3c 100644 --- a/products/maka/workbench/tests/session-permissions.test.mjs +++ b/products/maka/workbench/tests/session-permissions.test.mjs @@ -52,8 +52,8 @@ const hooks = registerHooks({ const { createSessionPermissions } = await import(new URL('sessionPermissions.ts', appRoot)); hooks.deregister(); -test('changing permission settings leaves grant revocation to the Host', async () => { - let session = { +test('disabling permission requests does not request grant revocation', async () => { + const session = { id: 'session', revision: 4, title: 'Permission settings', @@ -61,36 +61,23 @@ test('changing permission settings leaves grant revocation to the Host', async ( permissions: { access: 'read_only', requestPermissions: true }, }; const sent = []; - const control = async (method, input) => { - assert.equal(method, 'metadata'); - sent.push(input); - session = { - ...session, - revision: session.revision + 1, - permissions: input.permissions.settings, - }; + const permissions = createSessionPermissions({}, session.id, async (method, input) => { + sent.push([method, input.permissions]); return { kind: 'committed', session }; - }; - const permissions = createSessionPermissions({}, session.id, control); + }); permissions.observe(session); await permissions.select({ session, settings: { ...session.permissions, requestPermissions: false }, }); - assert.deepEqual(sent[0].permissions, { - settings: { access: 'read_only', requestPermissions: false }, - revokeGrants: false, - }); - assert.equal(permissions.error(), ''); - - await permissions.select({ - session, - settings: { ...session.permissions, access: 'workspace_write' }, - }); - assert.deepEqual(sent[1].permissions, { - settings: { access: 'workspace_write', requestPermissions: false }, - revokeGrants: false, - }); - assert.equal(permissions.error(), ''); + assert.deepEqual(sent, [ + [ + 'metadata', + { + settings: { access: 'read_only', requestPermissions: false }, + revokeGrants: false, + }, + ], + ]); });