fix(extensions): 实例退出时撤销限定范围的包访问控制项

2026-09-09 05:28:42 +08:00
parent 8936974059
commit dfc2643d26
5 changed files with 256 additions and 19 deletions
@@ -531,3 +531,14 @@ Core 的独立数据目录目前不等于已授权 Vault。Python 旧笔记写
 - 集成测试最初以固定 ActiveProcesses=2 判断工具子进程启动，本机在 Ready 时已计入 2 个进程，导致实际取消的是排队请求。已改为同时确认当前 Ticket 处于执行槽、且进程数超过该实例就绪基线，再触发取消；修正后得到进行中工具的 MCP_CANCELLED 并清空 Job。此前仅凭固定计数的证据不能单独用于识别某个具体后代进程。
 - 新增队列满/输入超限/停止后队列不执行，以及已结束但清理失败的注册项不回收测试。67 项扩展回归通过，5 项 ignored 为三个父测试驱动 Job 辅助入口及两个显式长时验收，本轮未重跑长时项。desktop 全目标 Clippy -D warnings 通过，首次检查的回调类型复杂度提示已通过类型别名修正。日志 `.build/extension-instance-tests.log`、`.build/extension-instance-regression.log`、`.build/extension-instance-clippy.log`。
 - 仍需将注册表纳入实际 Host 依赖与 UI/Core 的认证路由，完成活动安装/在线信任钩子、包对象权限回收/残留检查、全部 Host 创建路径继承审计、scratch/CPU 和完整攻击矩阵；其他同步/部署/安装包验收也继续推进。第三方执行能力保持禁用，完整生产化目标尚未完成。
+
+
+## 增量：实例包 ACL 的作用域回收
+
+- PinnedPackage.access 在第一次授权前即创建作用域所有者，正常退出、部分授权失败及栈展开时均尝试撤销该实例 SID 的授权；显式回收失败返回 EXTENSION_CONTAINER_ACL_REVOKE_FAILED，注册表保留隔离项。回收逐个处理所有固定目录和文件，即使单个失败也继续处理其余对象。
+- 后台实例将进程、入口和会话所有者限制在内层运行函数，退出内层后才通过原有持有句柄回收包 ACL，再释放固定包对象和 Profile。Host 内使用全局互斥串行化 ACL 读取、合并和写回，撤销只针对本实例 SID，不把旧 DACL 快照覆盖回去，因此不会主动抹去其他实例的授权。该锁不覆盖外部管理员并发修改。
+- 采用 Windows REVOKE_ACCESS 合并现有 ACL，依据 [ACCESS_MODE](https://learn.microsoft.com/en-us/windows/win32/api/accctrl/ne-accctrl-access_mode) 和 [SetEntriesInAcl](https://learn.microsoft.com/en-us/windows/win32/api/aclapi/nf-aclapi-setentriesinacla) 的语义。测试逐 ACE 比较所有包对象，覆盖另一实例仍获授权、部分授权失败回滚、显式完成和析构回收；真实后台启动/停止、调用取消、策略拒绝和保险库锁定后也检查包根目录与入口 ACL 恢复基线。
+- 首轮扩展回归为 67 通过、1 失败、5 ignored，失败位于旧原生 RPC 的句柄哨兵检查。原检查仅用 GetHandleInformation 判断句柄数值有效，不能排除子进程把该数值复用为其他对象。已改为 Host 私有临时文件哨兵，并比较卷序列号及 128 位文件 ID；查询失败或对象不同不算继承原对象。依据 [FILE_ID_INFO](https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_id_info)。原失败没有记录退出码，不能事后断言其唯一原因；现在异常路径附带子进程退出状态以便诊断。
+- 此回收机制不保证 Host 被强杀后的残留恢复，也尚未验证外部 ACL 修改竞争、清理系统调用故障及全部恶意文件攻击路径。第三方扩展执行能力仍禁用，生产化总目标未完成。
+- 修正后 desktop 全目标测试累计 128 通过、7 ignored：库 113、Host 8，其余集成 7；ignored 包含显式长时验收和由父测试驱动的辅助进程入口。日志 `.build/extension-acl-release-full.log`。其中实际 Sync 服务的有序推送/重复提交及故障恢复集成也通过。
+- 另行执行真实 MCP 60 秒工具超时验收通过，整项原生协议/网络探针测试耗时 76.77 秒，日志 `.build/extension-acl-release-sixty.log`；desktop 全目标 Clippy -D warnings 通过，日志 `.build/extension-acl-release-clippy.log`。这不是完整沙箱攻击矩阵或远端部署验收通过的证据。
@@ -6,6 +6,8 @@ use windows_sys::Win32::Security::{
    PSID,
 };

+static PACKAGE_ACL_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
+
 pub struct Profile {
    name: Vec<u16>,
    sid: PSID,
@@ -60,6 +62,18 @@ impl Profile {
    /// is used: every directory and file must be checked and granted separately.
    /// This adds an ACE; it does not sanitize pre-existing permissions.
    pub fn grant_package_read_execute(&self, object: &std::fs::File) -> Result<()> {
+        self.update_package_access(object, false)
+    }
+    /// Remove only this freshly-created instance's allowed ACEs, using the
+    /// original held object handle. Other principals keep their current ACLs.
+    pub fn revoke_package_access(&self, object: &std::fs::File) -> Result<()> {
+        self.update_package_access(object, true)
+    }
+    fn update_package_access(&self, object: &std::fs::File, revoke: bool) -> Result<()> {
+        // Serialize Host read/merge/write operations across concurrent instances.
+        let _lock = PACKAGE_ACL_LOCK
+            .lock()
+            .map_err(|_| HostError::new("EXTENSION_CONTAINER_ACL_FAILED"))?;
        use std::os::windows::{fs::MetadataExt, io::AsRawHandle};
        use windows_sys::Win32::{
            Foundation::LocalFree,
@@ -82,12 +96,13 @@ impl Profile {
        let metadata = object
            .metadata()
            .map_err(|_| HostError::new("EXTENSION_CONTAINER_ACL_FAILED"))?;
-        if metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0
-            || !(metadata.is_file() || metadata.is_dir())
+        if !revoke
+            && (metadata.file_attributes() & FILE_ATTRIBUTE_REPARSE_POINT != 0
+                || !(metadata.is_file() || metadata.is_dir()))
        {
            return Err(HostError::new("EXTENSION_CONTAINER_ACL_OBJECT_INVALID"));
        }
-        if metadata.is_file() {
+        if metadata.is_file() && !revoke {
            let mut info = BY_HANDLE_FILE_INFORMATION::default();
            if unsafe { GetFileInformationByHandle(object.as_raw_handle(), &mut info) } == 0
                || info.nNumberOfLinks != 1
@@ -117,7 +132,7 @@ impl Profile {
        }
        let entry = EXPLICIT_ACCESS_W {
            grfAccessPermissions: FILE_GENERIC_READ | FILE_GENERIC_EXECUTE,
-            grfAccessMode: GRANT_ACCESS,
+            grfAccessMode: if revoke { REVOKE_ACCESS } else { GRANT_ACCESS },
            grfInheritance: 0,
            Trustee: TRUSTEE_W {
                TrusteeForm: TRUSTEE_IS_SID,
@@ -217,6 +232,52 @@ impl Drop for Profile {
    }
 }

+#[cfg(all(test, feature = "desktop"))]
+pub(crate) fn test_acl_entries(object: &std::fs::File) -> Vec<Vec<u8>> {
+    use std::os::windows::io::AsRawHandle;
+    use windows_sys::Win32::{
+        Foundation::LocalFree,
+        Security::{Authorization::*, *},
+    };
+    struct Allocation(*mut core::ffi::c_void);
+    impl Drop for Allocation {
+        fn drop(&mut self) {
+            unsafe {
+                LocalFree(self.0);
+            }
+        }
+    }
+    let mut acl = std::ptr::null_mut();
+    let mut descriptor = std::ptr::null_mut();
+    assert_eq!(
+        unsafe {
+            GetSecurityInfo(
+                object.as_raw_handle(),
+                SE_FILE_OBJECT,
+                DACL_SECURITY_INFORMATION,
+                std::ptr::null_mut(),
+                std::ptr::null_mut(),
+                &mut acl,
+                std::ptr::null_mut(),
+                &mut descriptor,
+            )
+        },
+        0
+    );
+    let _descriptor = Allocation(descriptor);
+    assert!(!acl.is_null());
+    let mut entries = Vec::new();
+    for index in 0..unsafe { (*acl).AceCount } {
+        let mut ace = std::ptr::null_mut();
+        assert_ne!(unsafe { GetAce(acl, u32::from(index), &mut ace) }, 0);
+        let length = unsafe { (*(ace as *const ACE_HEADER)).AceSize };
+        entries.push(
+            unsafe { std::slice::from_raw_parts(ace.cast::<u8>(), usize::from(length)) }.to_vec(),
+        );
+    }
+    entries
+}
+
 #[cfg(test)]
 mod tests {
    use super::*;
@@ -786,17 +847,39 @@ mod tests {
            // Actual native RPC: the child cannot name an identity or connect to
            // a shared endpoint; only its own stdio pipe reaches this broker.
            {
-                use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle};
-                let sentinel = unsafe {
-                    windows_sys::Win32::System::Threading::CreateEventW(
-                        std::ptr::null(),
-                        1,
-                        0,
-                        std::ptr::null(),
-                    )
+                use std::os::windows::io::AsRawHandle;
+                use windows_sys::Win32::Storage::FileSystem::{
+                    FileIdInfo, GetFileInformationByHandleEx, FILE_ID_INFO,
                };
-                assert!(!sentinel.is_null());
-                let sentinel = unsafe { OwnedHandle::from_raw_handle(sentinel) };
+                let sentinel_dir = tempfile::tempdir().unwrap();
+                let sentinel = std::fs::OpenOptions::new()
+                    .read(true)
+                    .write(true)
+                    .create_new(true)
+                    .open(sentinel_dir.path().join("host-only-sentinel"))
+                    .unwrap();
+                let mut identity: FILE_ID_INFO = unsafe { std::mem::zeroed() };
+                assert_ne!(
+                    unsafe {
+                        GetFileInformationByHandleEx(
+                            sentinel.as_raw_handle(),
+                            FileIdInfo,
+                            (&mut identity as *mut FILE_ID_INFO).cast(),
+                            std::mem::size_of::<FILE_ID_INFO>() as u32,
+                        )
+                    },
+                    0
+                );
+                let sentinel_identity = format!(
+                    "{}:{}",
+                    identity.VolumeSerialNumber,
+                    identity
+                        .FileId
+                        .Identifier
+                        .iter()
+                        .map(|byte| format!("{byte:02x}"))
+                        .collect::<String>()
+                );
                assert_ne!(
                    unsafe {
                        windows_sys::Win32::Foundation::SetHandleInformation(
@@ -817,6 +900,7 @@ mod tests {
                rpc.arguments = vec![
                    "file_rpc".into(),
                    (sentinel.as_raw_handle() as usize).to_string(),
+                    sentinel_identity,
                ];
                rpc.permissions.insert("notes.read".into());
                rpc.expires_at_ms = 10_000;
@@ -840,7 +924,10 @@ mod tests {
                let crate::extension_io::Event::Frame(request) =
                    pump.receive(std::time::Duration::from_secs(5)).unwrap()
                else {
-                    panic!("missing RPC request")
+                    panic!(
+                        "missing RPC request; child exit: {:?}",
+                        running.wait(std::time::Duration::from_secs(1))
+                    )
                };
                let response = files.dispatch(&mut workspace, &request).unwrap();
                pump.send(serde_json::to_vec(&response).unwrap()).unwrap();
@@ -355,6 +355,7 @@ impl Registry {
                            matches!(
                                code,
                                "EXTENSION_CONTAINER_CLEANUP_FAILED"
+                                    | "EXTENSION_CONTAINER_ACL_REVOKE_FAILED"
                                    | "EXTENSION_RESOURCE_TERMINATE_FAILED"
                                    | "EXTENSION_INSTANCE_WORKER_FAILED"
                            )
@@ -398,7 +399,18 @@ fn run_in_profile(
    profile: &Profile,
 ) -> Result<()> {
    let pinned = PinnedPackage::open(&spec.package, &spec.inventory, &spec.claims.tree_sha256)?;
-    pinned.grant_read_execute(profile)?;
+    let access = pinned.access(profile)?;
+    let result = run_with_access(spec, control, receiver, profile, &pinned);
+    access.finish()?;
+    result
+}
+fn run_with_access(
+    spec: LaunchSpec,
+    control: &Control,
+    receiver: Receiver<Command>,
+    profile: &Profile,
+    pinned: &PinnedPackage,
+) -> Result<()> {
    let entry = pinned.bind_entry(&spec.claims.entry)?;
    let folder = profile.folder()?;
    let scratch = folder.join("Temp");
@@ -524,6 +536,10 @@ mod tests {
                .collect();
        let dir =
            cap_std::fs::Dir::open_ambient_dir(&package, cap_std::ambient_authority()).unwrap();
+        let acl_file = std::fs::File::open(&executable).unwrap();
+        let acl_root = dir.try_clone().unwrap().into_std_file();
+        let file_acl = crate::extension_container::test_acl_entries(&acl_file);
+        let root_acl = crate::extension_container::test_acl_entries(&acl_root);
        let inventory = || Inventory {
            files: files.clone(),
            expanded_size: bytes.len() as u64,
@@ -752,6 +768,14 @@ mod tests {
                .unwrap(),
            0
        );
+        assert_eq!(
+            crate::extension_container::test_acl_entries(&acl_file),
+            file_acl
+        );
+        assert_eq!(
+            crate::extension_container::test_acl_entries(&acl_root),
+            root_acl
+        );
    }
    fn channel() -> (Endpoint, Receiver<Command>) {
        let (commands, receiver) = mpsc::sync_channel(4);
@@ -15,6 +15,27 @@ pub struct PinnedPackage {
    files: BTreeMap<String, File>,
    tree_sha256: String,
 }
+/// Scoped ACL ownership, created before any mutation. Release only after all
+/// instance processes/handles have closed; drop retries cleanup on error/unwind.
+pub struct PackageAccess<'a> {
+    package: &'a PinnedPackage,
+    profile: &'a Profile,
+    active: bool,
+}
+impl PackageAccess<'_> {
+    pub fn finish(mut self) -> Result<()> {
+        self.package.revoke_access(self.profile)?;
+        self.active = false;
+        Ok(())
+    }
+}
+impl Drop for PackageAccess<'_> {
+    fn drop(&mut self) {
+        if self.active {
+            let _ = self.package.revoke_access(self.profile);
+        }
+    }
+}
 /// The package borrow and all ancestor handles must outlive the process using
 /// this path. Only files present in the verified package can produce this guard.
 pub struct BoundEntry<'a> {
@@ -244,6 +265,49 @@ impl PinnedPackage {
    pub fn tree_sha256(&self) -> &str {
        &self.tree_sha256
    }
+    pub fn access<'a>(&'a self, profile: &'a Profile) -> Result<PackageAccess<'a>> {
+        self.access_with(profile, || self.grant_read_execute(profile))
+    }
+    fn access_with<'a>(
+        &'a self,
+        profile: &'a Profile,
+        grant: impl FnOnce() -> Result<()>,
+    ) -> Result<PackageAccess<'a>> {
+        let guard = PackageAccess {
+            package: self,
+            profile,
+            active: true,
+        };
+        if let Err(error) = grant() {
+            guard.finish()?;
+            return Err(error);
+        }
+        Ok(guard)
+    }
+    fn revoke_access(&self, profile: &Profile) -> Result<()> {
+        let mut failed = false;
+        for dir in self.directories.values() {
+            if dir
+                .try_clone()
+                .map(|dir| dir.into_std_file())
+                .map_err(HostError::from)
+                .and_then(|file| profile.revoke_package_access(&file))
+                .is_err()
+            {
+                failed = true;
+            }
+        }
+        for file in self.files.values() {
+            if profile.revoke_package_access(file).is_err() {
+                failed = true;
+            }
+        }
+        if failed {
+            Err(HostError::new("EXTENSION_CONTAINER_ACL_REVOKE_FAILED"))
+        } else {
+            Ok(())
+        }
+    }
    pub fn grant_read_execute(&self, profile: &Profile) -> Result<()> {
        for dir in self.directories.values() {
            profile.grant_package_read_execute(&dir.try_clone()?.into_std_file())?;
@@ -347,4 +411,41 @@ mod tests {
            b"verified bytes"
        );
    }
+    #[test]
+    fn scoped_access_restores_all_acl_entries_and_preserves_other_instances() {
+        let (_temp, root, inventory, hash) = fixture();
+        let pinned = PinnedPackage::open(&root, &inventory, &hash).unwrap();
+        let snapshot = || {
+            let mut result = Vec::new();
+            for dir in pinned.directories.values() {
+                result.push(crate::extension_container::test_acl_entries(
+                    &dir.try_clone().unwrap().into_std_file(),
+                ));
+            }
+            for file in pinned.files.values() {
+                result.push(crate::extension_container::test_acl_entries(file));
+            }
+            result
+        };
+        let before = snapshot();
+        let first = Profile::create().unwrap();
+        let second = Profile::create().unwrap();
+        let other = pinned.access(&second).unwrap();
+        let other_acl = snapshot();
+        assert_ne!(other_acl, before);
+        let access = pinned.access(&first).unwrap();
+        assert_ne!(snapshot(), other_acl);
+        access.finish().unwrap();
+        assert_eq!(snapshot(), other_acl);
+        let failed = pinned.access_with(&first, || {
+            first.grant_package_read_execute(pinned.files.values().next().unwrap())?;
+            Err(HostError::new("INJECTED_PARTIAL_GRANT"))
+        });
+        assert_eq!(failed.err().unwrap().code, "INJECTED_PARTIAL_GRANT");
+        assert_eq!(snapshot(), other_acl);
+        drop(other);
+        assert_eq!(snapshot(), before);
+        first.remove().unwrap();
+        second.remove().unwrap();
+    }
 }
@@ -64,11 +64,25 @@ fn main() {
    }
    if args.get(1).is_some_and(|s| s == "file_rpc") {
        use std::io::{Read, Write};
+        #[repr(C)]
+        struct FileIdentity { volume: u64, id: [u8; 16] }
        #[link(name = "kernel32")]
-        extern "system" { fn GetHandleInformation(handle: *mut std::ffi::c_void, flags: *mut u32) -> i32; }
+        extern "system" {
+            fn GetFileInformationByHandleEx(handle: *mut std::ffi::c_void,
+                class: i32, info: *mut std::ffi::c_void, size: u32) -> i32;
+        }
        let sentinel: usize = args[2].parse().unwrap();
-        let mut flags = 0;
-        if unsafe { GetHandleInformation(sentinel as *mut _, &mut flags) } != 0 { std::process::exit(85); }
+        let mut identity = FileIdentity { volume: 0, id: [0; 16] };
+        // Numeric handles may alias unrelated child objects. Compare the actual
+        // file identity without reading from a possibly aliased pipe handle.
+        if unsafe { GetFileInformationByHandleEx(sentinel as *mut _, 18,
+            (&mut identity as *mut FileIdentity).cast(),
+            std::mem::size_of::<FileIdentity>() as u32) } != 0 {
+            let hex: String = identity.id.iter().map(|b| format!("{b:02x}")).collect();
+            if format!("{}:{}", identity.volume, hex) == args[3] {
+                std::process::exit(85);
+            }
+        }
        println!("{{\"method\":\"notes.read\",\"path\":\"fixture.md\"}}");
        std::io::stdout().flush().unwrap();
        let mut response = String::new();