From 7d7423419ec10d0060482f7590b651ba1bd08ccf Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Fri, 28 Aug 2026 22:21:12 +0800 Subject: [PATCH 01/17] fix: harden QOpen state and process boundaries --- .github/workflows/ci.yml | 2 +- AGENTS.md | 17 +- BoundedProcess.qml | 108 ++++++ CHANGELOG.md | 30 ++ DESIGN.md | 48 ++- DEVELOPMENT.md | 76 +++- PathPicker.qml | 22 +- QOpen.qml | 129 +++---- README.md | 47 ++- README.zh-CN.md | 39 ++- ResourceEditor.qml | 85 +++-- bin/qopen | 739 ++++++++++++++++++++++++++++++--------- manifest.json | 2 +- tests/test_backend.py | 296 +++++++++++++++- tests/test_docs.py | 3 + 15 files changed, 1286 insertions(+), 357 deletions(-) create mode 100644 BoundedProcess.qml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b45f7d8..1cf64ba 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,7 +44,7 @@ jobs: qopen_qmlcachegen=$(find /usr/lib/qt6 -type f -name qmlcachegen -print -quit) test -n "$qopen_qmlcachegen" qopen_qml_output=$(mktemp -d) - for qopen_qml_file in QOpen.qml ResourceEditor.qml PathPicker.qml BarWidget.qml; do + for qopen_qml_file in BoundedProcess.qml QOpen.qml ResourceEditor.qml PathPicker.qml BarWidget.qml; do "$qopen_qmlcachegen" --only-bytecode \ -o "$qopen_qml_output/$qopen_qml_file.qmlc" "$qopen_qml_file" done diff --git a/AGENTS.md b/AGENTS.md index a890f90..bdc5fbf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -118,6 +118,7 @@ patch release, then propagate the released commit back to `uat` and `dev`. | `QOpen.qml` | Main menu, search, collections, routing, mutations, and status | | `ResourceEditor.qml` | Type-aware add/edit form and validation feedback | | `PathPicker.qml` | Embedded bounded local file/directory browser | +| `BoundedProcess.qml` | One-line bounded backend transport and process deadlines | | `BarWidget.qml` | Optional Omarchy bar entry point | | `bin/qopen` | Python CLI, JSON API, validation, persistence, and launching | | `tests/test_backend.py` | Backend, API, safety, and QML regression tests | @@ -158,6 +159,8 @@ early feedback, but every write and launch must still be validated by Python. ### QML changes - Keep process commands as arrays. +- Route every backend response through `BoundedProcess.qml`; do not use + `StdioCollector` or direct catalog `FileView` access. - Keep all file selection inside `PathPicker.qml`. - Preserve request ids and stale-response rejection in path browsing. - Preserve explicit error feedback for clipboard, target checks, and writes. @@ -171,11 +174,17 @@ early feedback, but every write and launch must still be validated by Python. requirement. - Keep machine API output compact JSON and send human diagnostics to the appropriate stream. +- Keep catalog, backup and recovery operations descriptor-anchored to one + trusted state directory. Do not reintroduce ordinary pathname reads, + `shutil.copy2`, pathname `chmod`, or a pathname lock file. +- Enforce producer-side byte limits and monotonic deadlines before data reaches + QML. Every captured helper subprocess must have bounded output. - Create starter resources only when the catalog is missing; preserve existing catalogs byte-for-byte during reads and upgrades. - Validate the complete catalog before writes, recovery, and rendering. -- Keep file modes private (`0600`) for configuration, backup, lock, and invalid - recovery snapshots. +- Keep the default state directory private (`0700`) and configuration, backup + and invalid recovery snapshots private (`0600`). Never change the parent + directory permissions of a custom `QOPEN_CONFIG`. - Use `subprocess` argv lists and bounded timeouts where a subprocess can wait. - Expand paths only in the backend; do not commit machine-specific absolute paths. @@ -206,7 +215,7 @@ Compile QML with the same strategy as CI: qopen_qmlcachegen=$(find /usr/lib/qt6 -type f -name qmlcachegen -print -quit) test -n "$qopen_qmlcachegen" qopen_qml_output=$(mktemp -d) -for qopen_qml_file in QOpen.qml ResourceEditor.qml PathPicker.qml BarWidget.qml; do +for qopen_qml_file in BoundedProcess.qml QOpen.qml ResourceEditor.qml PathPicker.qml BarWidget.qml; do "$qopen_qmlcachegen" --only-bytecode \ -o "$qopen_qml_output/$qopen_qml_file.qmlc" "$qopen_qml_file" done @@ -246,6 +255,8 @@ At minimum, backend changes must cover: - command argument round trips, including spaces and quotes; - unsafe URL, SSH, and control-character rejection; - permission audit and repair; +- symlink, hard-link, FIFO and non-regular state-file rejection; +- held-lock, oversized-response and concurrent pathname-race behavior; - recovery with valid and invalid backups; - project/file browser filtering and stale request behavior. diff --git a/BoundedProcess.qml b/BoundedProcess.qml new file mode 100644 index 0000000..e2da35c --- /dev/null +++ b/BoundedProcess.qml @@ -0,0 +1,108 @@ +import Quickshell.Io +import QtQuick + +Item { + id: root + + width: 0 + height: 0 + visible: false + + property int timeoutMs: 2000 + property int responseLimit: 1310720 + readonly property bool running: child.running + readonly property var processId: child.processId + + property int activeRequestId: 0 + property string responseText: "" + property string protocolError: "" + property bool responseSeen: false + property bool timedOut: false + property bool canceled: false + + signal finished(string response, int exitCode, int requestId, string error) + + function start(command, requestId) { + if (child.running || !Array.isArray(command) || command.length === 0) return false + root.activeRequestId = Number(requestId || 0) + root.responseText = "" + root.protocolError = "" + root.responseSeen = false + root.timedOut = false + root.canceled = false + deadlineTimer.restart() + child.command = command + child.running = true + return true + } + + function cancel() { + root.canceled = true + deadlineTimer.stop() + if (child.running) { + child.signal(15) + killTimer.restart() + } + } + + function abortProtocol(message) { + if (root.protocolError) return + root.protocolError = String(message || "Invalid backend response") + if (child.running) { + child.signal(15) + killTimer.restart() + } + } + + Process { + id: child + + stdout: SplitParser { + splitMarker: "\n" + onRead: function(data) { + if (root.canceled || root.protocolError) return + var line = String(data || "") + if (root.responseSeen) { + root.abortProtocol("Backend returned more than one response") + return + } + if (line.length > root.responseLimit) { + root.abortProtocol("Backend response exceeded its safety limit") + return + } + root.responseText = line + root.responseSeen = true + } + } + + onExited: function(exitCode) { + deadlineTimer.stop() + killTimer.stop() + var error = root.protocolError + if (root.timedOut) error = "Backend operation timed out" + else if (root.canceled) error = "Backend operation canceled" + else if (!root.responseSeen) error = "Backend returned no response" + root.finished(root.responseText, exitCode, root.activeRequestId, error) + } + } + + Timer { + id: deadlineTimer + interval: root.timeoutMs + onTriggered: { + root.timedOut = true + if (child.running) { + child.signal(15) + killTimer.restart() + } + } + } + + Timer { + id: killTimer + interval: 250 + onTriggered: { + if (child.running) child.signal(9) + } + } +} diff --git a/CHANGELOG.md b/CHANGELOG.md index 4dcf96f..dad16c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,36 @@ All notable user-facing changes to QOpen are documented here. +## [2.5.1] - 2026-08-28 + +### Security + +- Anchored catalog, backup and recovery operations to a trusted, non-symlinked + state-directory descriptor and rejected symlinks, hard links and non-regular + state files. +- Replaced the pathname lock file with a deadline-bound lock on the trusted + state-directory descriptor. +- Added hard limits for catalog reads, API input/output, helper output, + resource counts and directory browsing work. +- Removed direct QML `FileView` catalog access and every unbounded + `StdioCollector`; backend calls now use one-line responses and real process + deadlines with TERM-to-KILL escalation. + +### Changed + +- Backups now reuse the exact bytes from the validated pre-mutation catalog. +- The default state directory is secured to `0700` by the explicit permission + repair command; custom `QOPEN_CONFIG` parent permissions are never changed. +- Direct raw catalog editing through `qopen --edit` is disabled so writes cannot + bypass validation, backup and atomic replacement. +- Path browsing now bounds both returned entries and total scan work. + +### Tests + +- Added adversarial coverage for parent and state-file symlinks, FIFOs, hard + links, oversized catalogs, held locks, unbounded clipboard output and + concurrent pathname replacement. + ## [2.5.0] - 2026-08-28 ### Added diff --git a/DESIGN.md b/DESIGN.md index 85d5323..27046a0 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1,4 +1,4 @@ -# QOpen 2.4 Design +# QOpen 2.5.1 Design ## Product boundary @@ -18,14 +18,21 @@ shell commands well. QOpen owns the resources that do not fit that model: Omarchy menu / bar | v -QOpen.qml <--- watches ---> ~/.config/qopen/config.json +QOpen.qml + | + | bounded argv / one-line JSON + v +BoundedProcess.qml | - | argv only v bin/qopen - |-- validate schema and targets - |-- lock + backup + atomic replace + |-- descriptor-anchored state validation + |-- directory FD lock + backup + atomic replace + |-- bounded reads, responses and helper processes `-- dispatch through Omarchy launch helpers + | + v +~/.config/qopen/config.json ``` The split is deliberate. QML is responsible for presentation, search and @@ -83,14 +90,29 @@ Supported item types are `web`, `file`, `project`, `tui`, `command` and ## Persistence and safety -- A file lock serializes mutations. -- Writes go to a same-directory temporary file and use atomic replacement. -- The previous catalog is copied to `config.json.bak` before mutation. +- Every state-directory component is opened from `/` with directory + descriptors, `O_DIRECTORY` and `O_NOFOLLOW`; the final directory must be + owned by the current user and not writable by another account. +- The trusted state-directory descriptor itself is locked with nonblocking + `flock` and a monotonic deadline; no pathname lock file is needed. +- Configuration and backup files use `O_NOFOLLOW`, `O_NONBLOCK` and descriptor + metadata checks that reject symlinks, hard links and non-regular files. +- Reads are capped at 1 MiB and detect in-place changes before accepting JSON. +- Writes go to a descriptor-relative same-directory temporary file, `fsync` + the file, atomically replace with `src_dir_fd`/`dst_dir_fd`, then `fsync` the + directory. +- The same validated bytes used for a mutation are retained as + `config.json.bak`; the backup is never reopened through an ordinary path. - Backups are validated before recovery, and the replaced invalid catalog is retained as a private timestamped snapshot. -- New state files use mode `0600`; `doctor` audits permissions and repair is - explicit. +- The default state directory uses `0700`; state files use `0600`. Custom + `QOPEN_CONFIG` parent permissions are validated but never changed. - The complete schema is validated before every write. +- API input, producer output, helper output, item count and directory scan work + all have explicit limits. +- QML has no catalog `FileView` and no `StdioCollector`. Backend responses use + one compact JSON line, a second consumer-side limit and real TERM-to-KILL + process deadlines. - Commands are represented as argument arrays and use lossless POSIX quoting when displayed for editing. - File and project paths expand `~` and environment variables only in the @@ -100,6 +122,12 @@ Supported item types are `web`, `file`, `project`, `tui`, `command` and replacing newer navigation state. - Destructive removal always asks for confirmation. +These controls prevent pathname redirection, special-file blocking, unbounded +retention and writes outside the trusted state directory. They do not claim to +prevent a deliberately malicious process running as the same Unix uid from +directly replacing valid user-owned application code or data; that process +already has the user's filesystem authority. + ## Integration The plugin id is `qopen.launcher` and its manifest exposes `menu` and diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 0fa93d0..ce6c807 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -112,6 +112,48 @@ React 富文本、动画、图标、状态、表单和交互能力分组。 该版本没有引入新数据 schema。2.3 的配置可以直接使用，不需要迁移。 +### v2.5：首次目录与市场发布准备 + +2.5 将首次启动资源收敛为六项通用示例，并统一 `main`、`uat`、`dev`、tag、 +双语 README、预览图和 AI 协作流程，为 Omarchy Plugin Marketplace 提交建立精确 +commit 基线。 + +### v2.5.1：市场审核安全加固 + +市场审核在 `bcaf8f047fc581019cecb29f1689752e0303c89f` 指出了两个阻塞项：状态文件 +仍通过普通路径操作，QML 仍直接使用 `FileView` 和无界 `StdioCollector`。2.5.1 +据此重构完整安全边界： + +- 从 `/` 开始逐级使用目录描述符和 `O_NOFOLLOW` 打开状态目录； +- 配置、备份和恢复快照只通过可信目录 FD 与相对名称访问； +- 拒绝 symlink、hardlink、FIFO、设备、Socket、目录和非当前用户文件； +- 取消路径锁文件，直接对可信状态目录 FD 使用带截止时间的非阻塞 `flock`； +- 使用同一次已验证读取的原始 bytes 创建备份，再执行 descriptor-relative 原子替换； +- 配置读取、API 输入输出、helper 输出、目录扫描数量和扫描时间都具有硬上限； +- 删除 QML `FileView` 与全部 `StdioCollector`，统一通过 `BoundedProcess.qml` + 读取单行 JSON，并执行真实 TERM 到 KILL 截止时间； +- 禁用无法参与锁、校验、备份与原子替换协议的原始 `$QOPEN --edit` 入口； +- 增加父目录和文件 symlink、FIFO、hardlink、超限输入、锁超时、无界剪贴板输出和 + 并发路径替换攻击测试。 + +安全保证针对路径重定向、特殊文件阻塞、竞态写出和资源耗尽。相同 Unix uid 的恶意 +进程本身拥有用户文件权限，因此不能声称阻止它直接替换合法普通文件或插件代码。 + +#### v2.5.1 实施与验收计划 + +| 阶段 | 实施内容 | 完成标准 | +| --- | --- | --- | +| 1. 威胁建模 | 固定审核 commit、枚举配置/备份/锁/QML 输出入口与同 uid 边界 | 每个审核意见都有明确代码入口和测试映射 | +| 2. 状态目录加固 | 从 `/` 逐级无跟随打开目录，校验最终目录和状态文件，移除路径锁文件 | symlink、hardlink、FIFO、设备及越权权限全部 fail closed | +| 3. 原子生命周期 | 在同一目录 FD 与锁内完成读取、备份、临时写入、替换和恢复 | 备份来自同一次已验证读取，文件和目录均完成 `fsync` | +| 4. 资源边界 | 限制目录、item、API 输入输出、helper 输出与扫描工作量 | 超限输入快速失败，不进入 QML 或形成无界内存占用 | +| 5. QML 进程协议 | 删除 `FileView`/`StdioCollector`，统一单行 JSON、请求 id、截止时间和 TERM→KILL | 后端挂起、重复响应、无响应和陈旧响应均得到确定处理 | +| 6. 对抗验证 | 覆盖预置路径、并发替换、锁占用、特殊文件、超大输出和超时 helper | 自动测试无挂起、外部哨兵文件不变、真实个人目录哈希不变 | +| 7. 发布门禁 | 校验 manifest、Python、全部 QML、插件规范、双语文档与安装运行时 | CI 与本机门禁通过，UAT 后以精确 HEAD 请求市场复审 | + +版本号、文档和实现一起提升到 `2.5.1`。该修复先通过 hotfix PR 进入 `main`；在用户 +运行验收和市场复审完成前，不创建或移动 release tag，也不覆盖已发布的 `v2.5.0`。 + ## 3. v2.2 文件选择器事故复盘 ### 现象 @@ -164,14 +206,17 @@ QOpen.qml │ └─ PathPicker.qml │ ├─ 路径与列表交互 │ └─ api browse-path - └─ Process argv + └─ BoundedProcess.qml + ├─ 单行 JSON 与响应上限 + └─ 真实进程截止时间 | v bin/qopen ├─ JSON schema 验证 - ├─ 目录枚举 - ├─ 文件锁 - ├─ 备份与原子替换 + ├─ 有界目录枚举 + ├─ 描述符锚定状态目录 + ├─ 目录 FD 锁 + ├─ 有界备份与原子替换 └─ 资源启动分发 ``` @@ -200,6 +245,7 @@ QOpen.qml | `QOpen.qml` | 主界面、搜索、资源列表、路由与 CRUD 协调 | | `ResourceEditor.qml` | 新增/编辑表单 | | `PathPicker.qml` | 安全内嵌文件和目录浏览器 | +| `BoundedProcess.qml` | 有界单行后端协议与进程截止时间 | | `BarWidget.qml` | Omarchy 状态栏按钮 | | `bin/qopen` | Python CLI、机器 API、持久化与启动后端 | | `README.md` | 用户安装、使用与排障文档 | @@ -211,7 +257,7 @@ QOpen.qml ### 首次目录初始化 -`api catalog` 在 `~/.config/qopen/config.json` 不存在时，通过现有锁和原子写入流程创建 +`api catalog` 在 `~/.config/qopen/config.json` 不存在时，通过目录 FD 锁和原子写入流程创建 一个六项通用示例目录。示例覆盖 web、project、file、TUI 和 command，包括 Omarchy、 GitHub、home 目录、Omarchy Shell 配置、btop 与 Fastfetch。它们不包含维护者本机路径、 凭据或网络主机。 @@ -222,17 +268,19 @@ GitHub、home 目录、Omarchy Shell 配置、btop 与 Fastfetch。它们不包 ### 写入流程 1. QML 将编辑结果序列化为 JSON 参数。 -2. 后端读取当前配置并取得独占文件锁。 +2. 后端逐级无跟随打开可信状态目录并取得目录 FD 独占锁。 3. 在副本上执行 mutation。 4. 验证完整配置，而不只验证当前 item。 -5. 将旧配置复制为 `config.json.bak`。 -6. 在配置同目录创建临时文件并 `fsync`。 -7. 使用 `os.replace` 原子替换。 -8. 对配置目录执行 `fsync`。 - -新建的配置、备份和锁文件使用 `0600`。旧文件不会在普通启动过程中被静默改权； -维护者或用户通过 `qopen --doctor` 检查，再显式运行 -`qopen fix-permissions`。恢复流程先验证备份，再保存当前目录快照，最后原子替换， +5. 将同一次安全读取的旧 bytes 写入 `config.json.bak` 临时文件并 `fsync`。 +6. 通过目录 FD 原子替换备份并 `fsync` 目录。 +7. 在同一目录 FD 下创建新配置临时文件并 `fsync`。 +8. 使用带 `src_dir_fd`/`dst_dir_fd` 的 `os.replace` 原子替换。 +9. 对配置目录执行 `fsync`。 + +默认状态目录使用 `0700`，配置、备份和快照使用 `0600`，不再创建锁文件。旧文件 +不会在普通启动过程中被静默改权；维护者或用户通过 `qopen --doctor` 检查，再显式 +运行 `qopen fix-permissions`。自定义 `QOPEN_CONFIG` 的父目录权限只校验、不修改。 +恢复流程先验证备份，再保存当前目录快照，最后原子替换， 因此无效备份不会覆盖现有目录。 ### 命令边界 diff --git a/PathPicker.qml b/PathPicker.qml index b74cc5d..fb58fe8 100644 --- a/PathPicker.qml +++ b/PathPicker.qml @@ -58,7 +58,7 @@ FocusScope { root.requestSerial++ root.pendingPath = "" root.pendingRequestSerial = 0 - if (browseProcess.running) browseProcess.running = false + if (browseProcess.running) browseProcess.cancel() root.canceled() } @@ -85,10 +85,10 @@ FocusScope { root.activeRequestSerial = root.pendingRequestSerial root.pendingPath = "" root.pendingRequestSerial = 0 - browseProcess.command = [root.backendPath, "api", "browse-path", "--path", requested, + var command = [root.backendPath, "api", "browse-path", "--path", requested, "--type", root.resourceType, "--request-id", String(root.activeRequestSerial)] - if (hidden) browseProcess.command.push("--show-hidden") - browseProcess.running = true + if (hidden) command.push("--show-hidden") + browseProcess.start(command, root.activeRequestSerial) } function consumeResponse(raw) { @@ -484,14 +484,16 @@ FocusScope { } } - Process { + BoundedProcess { id: browseProcess - stdout: StdioCollector { - waitForEnd: true - onStreamFinished: root.consumeResponse(text) - } - onExited: function(exitCode) { + timeoutMs: 3000 + onFinished: function(response, exitCode, requestId, error) { if (!root.opened) return + if (requestId === root.requestSerial && !error) root.consumeResponse(response) + else if (requestId === root.requestSerial && error) { + root.loading = false + root.errorText = error + } if (root.pendingPath) { Qt.callLater(root.startPendingRequest) return diff --git a/QOpen.qml b/QOpen.qml index dc3c124..f73153c 100644 --- a/QOpen.qml +++ b/QOpen.qml @@ -16,7 +16,6 @@ Item { readonly property string pluginId: (manifest && manifest.id) || "qopen.launcher" readonly property string pluginDir: (manifest && manifest.__sourceDir) || "" readonly property string backendPath: pluginDir ? pluginDir + "/bin/qopen" : "" - readonly property string configPath: Quickshell.env("HOME") + "/.config/qopen/config.json" onPluginDirChanged: { if (root.pluginDir) root.requestCatalogReload() @@ -46,6 +45,9 @@ Item { property var deleteCandidate: null property bool configMissing: false property string pendingCopyText: "" + property int catalogRequestSerial: 0 + property int mutationRequestSerial: 0 + property int apiRequestSerial: 0 readonly property color background: Color.menu.background readonly property color foreground: Color.menu.text @@ -104,7 +106,7 @@ Item { root.pendingBrowse = payload.browse === true if (root.shell && typeof root.shell.hide === "function") root.shell.hide("omarchy.menu") root.opened = true - configFile.reload() + root.requestCatalogReload() Qt.callLater(function() { searchField.text = root.query if (!root.pendingAction) { @@ -143,7 +145,7 @@ Item { } function refresh() { - configFile.reload() + root.requestCatalogReload() return "ok" } @@ -192,9 +194,8 @@ Item { ].join(" ").toLowerCase() } - function loadConfig(raw) { + function loadConfig(parsed) { try { - var parsed = JSON.parse(raw) if (!parsed || !Array.isArray(parsed.items)) throw new Error("items must be an array") root.catalog = parsed root.configError = "" @@ -219,8 +220,11 @@ Item { return } root.catalogResponseHandled = false - catalogProcess.command = [root.backendPath, "api", "catalog"] - catalogProcess.running = true + root.catalogRequestSerial++ + catalogProcess.start( + [root.backendPath, "api", "catalog"], + root.catalogRequestSerial + ) } function handleCatalogResponse(raw) { @@ -237,7 +241,7 @@ Item { root.rebuildItems() return } - root.loadConfig(JSON.stringify(response.result || ({}))) + root.loadConfig(response.result || ({})) } catch (e) { root.configError = "Invalid catalog response: " + e root.configMissing = false @@ -473,8 +477,8 @@ Item { function runMutation(command) { if (mutationProcess.running) return root.mutationResponseHandled = false - mutationProcess.command = command - mutationProcess.running = true + root.mutationRequestSerial++ + mutationProcess.start(command, root.mutationRequestSerial) } function handleMutationResponse(raw) { @@ -495,7 +499,7 @@ Item { else root.statusText = "Catalog updated" root.deleteCandidate = null - configFile.reload() + root.requestCatalogReload() root.statusError = false statusTimer.restart() } catch (e) { @@ -509,9 +513,14 @@ Item { return } root.apiResponseHandled = false - apiProcess.command = [root.backendPath, "api", action, "--payload", payload] - if (action === "update") apiProcess.command.push("--original", originalPayload) - apiProcess.running = true + if (String(payload || "").length > 32768 || String(originalPayload || "").length > 32768) { + resourceEditor.saveFailed("Resource data exceeds the safety limit") + return + } + var command = [root.backendPath, "api", action, "--payload", payload] + if (action === "update") command.push("--original", originalPayload) + root.apiRequestSerial++ + apiProcess.start(command, root.apiRequestSerial) } function handleApiResponse(raw) { @@ -535,7 +544,7 @@ Item { + (warnings.length ? " · " + warnings[0] : "") root.statusError = false statusTimer.restart() - configFile.reload() + root.requestCatalogReload() Qt.callLater(function() { searchField.forceActiveFocus() }) } catch (e) { resourceEditor.saveFailed("Invalid response from QOpen backend") @@ -545,34 +554,14 @@ Item { ListModel { id: groupModel } ListModel { id: displayModel } - FileView { - id: configFile - path: root.configPath - watchChanges: true - printErrors: false - onLoaded: root.requestCatalogReload() - onLoadFailed: function(error) { - root.configError = "Preparing first-run resources…" - root.configMissing = true - root.catalog = ({ version: 1, defaults: ({}), items: [] }) - root.rebuildGroups() - root.rebuildItems() - root.requestCatalogReload() - } - onFileChanged: reload() - } - - Process { + BoundedProcess { id: catalogProcess - stdout: StdioCollector { - waitForEnd: true - onStreamFinished: { - catalogFallbackTimer.stop() - root.handleCatalogResponse(text) - } - } - onExited: function(exitCode) { - if (!root.catalogResponseHandled) catalogFallbackTimer.restart() + timeoutMs: 3000 + onFinished: function(response, exitCode, requestId, error) { + if (requestId !== root.catalogRequestSerial) return + root.handleCatalogResponse(error + ? JSON.stringify({ ok: false, error: error }) + : response) if (root.catalogReloadPending) { root.catalogReloadPending = false Qt.callLater(root.requestCatalogReload) @@ -580,31 +569,15 @@ Item { } } - Timer { - id: catalogFallbackTimer - interval: 120 - onTriggered: root.handleCatalogResponse('{"ok":false,"error":"QOpen catalog validation failed"}') - } - - Process { + BoundedProcess { id: mutationProcess - stdout: StdioCollector { - waitForEnd: true - onStreamFinished: { - mutationFallbackTimer.stop() - root.handleMutationResponse(text) - } + timeoutMs: 3000 + onFinished: function(response, exitCode, requestId, error) { + if (requestId !== root.mutationRequestSerial) return + root.handleMutationResponse(error + ? JSON.stringify({ ok: false, error: error }) + : response) } - onExited: function(exitCode) { - if (!root.mutationResponseHandled) - mutationFallbackTimer.restart() - } - } - - Timer { - id: mutationFallbackTimer - interval: 120 - onTriggered: root.handleMutationResponse('{"ok":false,"error":"QOpen backend operation failed"}') } Process { @@ -617,27 +590,17 @@ Item { } } - Process { + BoundedProcess { id: apiProcess - stdout: StdioCollector { - waitForEnd: true - onStreamFinished: { - apiFallbackTimer.stop() - root.handleApiResponse(text) - } - } - onExited: function(exitCode) { - if (!root.apiResponseHandled) - apiFallbackTimer.restart() + timeoutMs: 3000 + onFinished: function(response, exitCode, requestId, error) { + if (requestId !== root.apiRequestSerial) return + root.handleApiResponse(error + ? JSON.stringify({ ok: false, error: error }) + : response) } } - Timer { - id: apiFallbackTimer - interval: 120 - onTriggered: root.handleApiResponse('{"ok":false,"error":"QOpen backend failed"}') - } - Timer { id: statusTimer interval: 4000 @@ -762,7 +725,7 @@ Item { root.manage("remove", root.selectedItemId()) event.accepted = true } else if (ctrl && event.key === Qt.Key_R) { - configFile.reload() + root.requestCatalogReload() root.statusText = "Catalog reloaded" root.statusError = false statusTimer.restart() diff --git a/README.md b/README.md index 2482366..9370f96 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ regrouped or removed by you. > 而是把项目目录、文件、常用文档、前端生态网站、TUI、命令和 SSH 目标集中到 > 一个支持搜索、分组、收藏与原生编辑的界面中。 -Current release: **2.5.0** +Current release: **2.5.1** **Documentation:** [简体中文](README.zh-CN.md) · [Development record](DEVELOPMENT.md) @@ -28,7 +28,8 @@ Current release: **2.5.0** - Normal keyboard paste plus an explicit clipboard button for path fields. - Automatic name, id, default group and icon inference where appropriate. - Inline favorite, copy-target, edit and confirmed remove actions. -- Atomic JSON writes with locking and an automatic last-known-good backup. +- Descriptor-anchored JSON writes with directory locking and a last-known-good backup. +- Bounded backend responses and real process deadlines before data reaches QML. - Validated backup recovery and explicit private-permission repair commands. - Lossless editing of command arguments containing spaces or quotes. - Optional bar widget: left-click opens all resources; right-click opens favorites. @@ -97,7 +98,7 @@ environment with: ~/.config/omarchy/plugins/qopen.launcher/bin/qopen --doctor ``` -The 2.5.0 release was developed and verified on Omarchy 4.0.1, Quickshell +The 2.5.1 release was developed and verified on Omarchy 4.0.1, Quickshell 0.3.1 and Qt 6.11.2. These are tested versions, not strict pins. ## Installation @@ -356,20 +357,27 @@ Important guarantees: - ids contain lowercase letters, numbers, `_` or `-` and must be unique; - each mutation validates the complete catalog; -- writes use a lock and same-directory atomic replacement; +- every state-directory component is opened without following symlinks; +- configuration and backup access rejects symlinks, hard links and non-regular files; +- writes use a lock on the trusted state-directory descriptor and same-directory + atomic replacement; - the previous catalog is retained as `config.json.bak`; -- new state files are private (`0600`), and existing permissions can be audited - or repaired explicitly; -- QML never writes the catalog directly; +- the default state directory is private (`0700`) and state files use `0600`; +- catalog reads, API responses, helper output and directory scans are bounded; +- QML never opens or writes the catalog directly and backend processes have deadlines; - personal resource data is not automatically synchronized to this GitHub repository. Set `QOPEN_CONFIG` to use a different catalog with the CLI: ```bash -QOPEN_CONFIG=~/Documents/qopen-work.json \ +QOPEN_CONFIG=~/Documents/qopen-work/config.json \ ~/.config/omarchy/plugins/qopen.launcher/bin/qopen --list ``` +Use a dedicated directory that is owned by your account and is not group- or +world-writable. QOpen validates custom state directories but never changes their +directory permissions. + ## Command-line interface The backend is installed with the plugin: @@ -387,8 +395,7 @@ $QOPEN edit [id] # Guided edit flow $QOPEN remove [id] # Confirmed removal $QOPEN favorite toggle # Toggle favorite state $QOPEN recover # Validate and restore config.json.bak -$QOPEN fix-permissions # Restrict QOpen state files to mode 0600 -$QOPEN --edit # Open raw JSON in the configured editor +$QOPEN fix-permissions # Secure the default state directory and files $QOPEN --doctor # Validate dependencies and every item $QOPEN --version ``` @@ -420,11 +427,15 @@ Omarchy menu / shortcut / bar widget ResourceEditor.qml PathPicker.qml | | +---------+----------+ - | argv only + | bounded argv / one-line JSON + v + BoundedProcess.qml + | v bin/qopen | - config lock + validation + descriptor-anchored validation + + directory FD lock | v ~/.config/qopen/config.json @@ -443,6 +454,16 @@ validation are documented in [DEVELOPMENT.md](DEVELOPMENT.md). Commands are passed as argument arrays. Resource values are never concatenated into a shell command by QML. +QML does not use `FileView` for the catalog and does not retain complete process +streams with `StdioCollector`. The Python producer validates and caps every API +response before writing it, while QML applies a second response limit and a real +deadline with TERM-to-KILL escalation. Catalog, backup and recovery operations +remain anchored to one trusted directory descriptor for their complete lifecycle. + +Direct raw editing through `$QOPEN --edit` is intentionally disabled because an +external editor cannot participate in QOpen's lock, validation, backup and atomic +replacement protocol. Use the native editor or `$QOPEN edit [id]` instead. + ## Updating and removing Update a Git-installed copy: @@ -499,7 +520,7 @@ mutation. Restore it only after validation: Before replacing the catalog, QOpen preserves the current invalid file as a private timestamped `config.json.invalid-*` snapshot. If `--doctor` reports -group- or world-readable state files, repair them explicitly: +insecure default-state permissions, repair them explicitly: ```bash ~/.config/omarchy/plugins/qopen.launcher/bin/qopen fix-permissions diff --git a/README.zh-CN.md b/README.zh-CN.md index 7a230b9..6ac5eea 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -6,7 +6,7 @@ QOpen 是运行在 [Omarchy](https://omarchy.org/) 上的个人资源启动器 > English summary: QOpen is a curated personal resource launcher for Omarchy. It brings projects, files, documentation, web tools, TUI applications, commands and SSH destinations into one searchable interface with grouping, favorites and native editing. -当前版本：**2.5.0** +当前版本：**2.5.1** **文档：** [English](README.md) · [开发记录](DEVELOPMENT.md) @@ -22,7 +22,8 @@ QOpen 是运行在 [Omarchy](https://omarchy.org/) 上的个人资源启动器 - 路径字段支持常规键盘粘贴，并提供明确的剪贴板按钮。 - 在适合时自动推导名称、id、默认分组和图标。 - 资源行内支持收藏、复制目标、编辑和确认后删除。 -- JSON 写入使用文件锁、原子替换和自动的上一个可用版本备份。 +- JSON 写入锚定目录描述符，并使用目录锁、原子替换和上一个可用版本备份。 +- 后端响应在进入 QML 前受字节上限和真实进程截止时间保护。 - 支持经过校验的备份恢复，以及显式的私有权限修复命令。 - 编辑带空格或引号的命令参数时保持无损往返。 - 可选状态栏组件：左键打开全部资源，右键打开收藏。 @@ -73,7 +74,7 @@ QOpen 会在可用时使用 Omarchy 的启动 helper，并可通过以下命令 ~/.config/omarchy/plugins/qopen.launcher/bin/qopen --doctor ``` -2.5.0 发布版本在 Omarchy 4.0.1、Quickshell 0.3.1 和 Qt 6.11.2 上完成开发与验证。这些是已测试版本，并非严格版本锁定。 +2.5.1 发布版本在 Omarchy 4.0.1、Quickshell 0.3.1 和 Qt 6.11.2 上完成开发与验证。这些是已测试版本，并非严格版本锁定。 ## 安装 @@ -312,19 +313,25 @@ Project 模式只列出目录，并通过底部按钮选择当前目录。File - id 只能包含小写字母、数字、`_` 或 `-`，并且必须唯一； - 每次修改都会验证完整目录； -- 写入使用文件锁和同目录原子替换； +- 状态目录的每一级都以不跟随符号链接的方式打开； +- 配置和备份拒绝符号链接、硬链接及非普通文件； +- 写入锁定可信状态目录描述符，并使用同目录原子替换； - 上一个目录版本会保留为 `config.json.bak`； -- 新状态文件使用私有权限（`0600`），已有文件可显式审计和修复权限； -- QML 不会直接写入目录； +- 默认状态目录使用私有权限（`0700`），状态文件使用 `0600`； +- 目录读取、API 响应、helper 输出和目录扫描都有明确上限； +- QML 不会直接打开或写入目录，后端进程也具有真实截止时间； - 个人资源数据不会自动同步到这个 GitHub 仓库。 可以设置 `QOPEN_CONFIG`，让 CLI 使用另一份目录： ```bash -QOPEN_CONFIG=~/Documents/qopen-work.json \ +QOPEN_CONFIG=~/Documents/qopen-work/config.json \ ~/.config/omarchy/plugins/qopen.launcher/bin/qopen --list ``` +请使用由当前账户拥有、且组用户和其他用户不可写的专用目录。QOpen 会验证 +自定义状态目录，但绝不会修改该目录本身的权限。 + ## 命令行界面 后端会随插件一起安装： @@ -342,8 +349,7 @@ $QOPEN edit [id] # 引导式编辑 $QOPEN remove [id] # 确认后删除 $QOPEN favorite toggle # 切换收藏状态 $QOPEN recover # 校验并恢复 config.json.bak -$QOPEN fix-permissions # 将 QOpen 状态文件限制为 0600 -$QOPEN --edit # 使用配置的编辑器打开原始 JSON +$QOPEN fix-permissions # 保护默认状态目录和状态文件 $QOPEN --doctor # 验证依赖和全部条目 $QOPEN --version ``` @@ -374,11 +380,14 @@ Omarchy 菜单 / 快捷键 / 状态栏组件 ResourceEditor.qml PathPicker.qml | | +---------+----------+ - | 仅传 argv + | 有界 argv / 单行 JSON + v + BoundedProcess.qml + | v bin/qopen | - 配置锁 + 完整校验 + 描述符锚定校验 + 目录 FD 锁 | v ~/.config/qopen/config.json @@ -390,6 +399,14 @@ QML 负责展示、焦点和交互。Python 负责目录枚举、规范化、校 命令始终以参数数组传递。QML 不会把资源值拼接成 Shell 命令。 +QML 不使用 `FileView` 读取目录，也不通过 `StdioCollector` 保留完整进程流。 +Python 生产端会在写出前校验并限制每个 API 响应；QML 再执行第二层响应限制， +并通过 TERM 到 KILL 的升级机制落实真实截止时间。配置、备份和恢复在完整生命周期 +内始终锚定到同一个可信目录描述符。 + +`$QOPEN --edit` 原始编辑入口已主动禁用，因为外部编辑器无法参与 QOpen 的锁、 +校验、备份和原子替换协议。请改用原生编辑界面或 `$QOPEN edit [id]`。 + ## 更新和卸载 更新通过 Git 安装的插件： diff --git a/ResourceEditor.qml b/ResourceEditor.qml index bfbfa3d..4cd5174 100644 --- a/ResourceEditor.qml +++ b/ResourceEditor.qml @@ -37,6 +37,8 @@ Item { property string targetLevel: "" property string expandedTarget: "" property string clipboardOutput: "" + property int clipboardRequestSerial: 0 + property int targetRequestSerial: 0 readonly property color foreground: Color.menu.text readonly property color background: Color.menu.background @@ -269,19 +271,30 @@ Item { function checkTarget() { var value = root.primaryValue().trim() if (!value || root.backendPath === "" || targetProcess.running) return + if (value.length > 8192) { + root.targetStatus = "Value exceeds the safety limit" + root.targetLevel = "error" + return + } root.targetStatus = "Checking…" root.targetLevel = "" - targetProcess.command = [root.backendPath, "api", "check-target", "--type", root.selectedType, "--value", value] - targetProcess.running = true + root.targetRequestSerial++ + targetProcess.start( + [root.backendPath, "api", "check-target", "--type", root.selectedType, "--value", value], + root.targetRequestSerial + ) } function useClipboard() { - if (clipboardProcess.running) return + if (clipboardProcess.running || !root.backendPath) return root.clipboardOutput = "" root.targetStatus = "Reading clipboard…" root.targetLevel = "" - clipboardProcess.command = ["wl-paste", "--type", "text", "--no-newline"] - clipboardProcess.running = true + root.clipboardRequestSerial++ + clipboardProcess.start( + [root.backendPath, "api", "clipboard-read"], + root.clipboardRequestSerial + ) } function expandedLocalPath(value) { @@ -340,17 +353,23 @@ Item { pathPicker.openPicker(root.selectedType, root.initialPickerFolder()) } - Process { + BoundedProcess { id: clipboardProcess - stdout: StdioCollector { - waitForEnd: true - onStreamFinished: { - root.clipboardOutput = String(text || "").trim().split("\n")[0] - } - } - onExited: function(exitCode) { - if (exitCode !== 0 || !root.clipboardOutput) { - root.targetStatus = "Clipboard is unavailable or does not contain text" + timeoutMs: 1500 + responseLimit: 16384 + onFinished: function(raw, exitCode, requestId, error) { + if (requestId !== root.clipboardRequestSerial) return + try { + var response = error ? ({ ok: false, error: error }) : JSON.parse(String(raw || "{}")) + var value = response.ok && response.result ? String(response.result.text || "") : "" + if (!value) { + root.targetStatus = String(response.error || "Clipboard is unavailable or does not contain text") + root.targetLevel = "error" + return + } + root.clipboardOutput = value + } catch (e) { + root.targetStatus = "Clipboard returned an invalid response" root.targetLevel = "error" return } @@ -361,27 +380,27 @@ Item { } } - Process { + BoundedProcess { id: targetProcess - stdout: StdioCollector { - waitForEnd: true - onStreamFinished: { - try { - var response = JSON.parse(String(text || "{}")) - if (!response.ok) { - root.targetStatus = String(response.error || "Could not validate this value") - root.targetLevel = "error" - root.expandedTarget = "" - return - } - var result = response.result || ({}) - root.targetStatus = String(result.message || "") - root.targetLevel = String(result.level || "") - root.expandedTarget = String(result.expanded || "") - } catch (e) { - root.targetStatus = "Could not validate this value" + timeoutMs: 1500 + responseLimit: 32768 + onFinished: function(raw, exitCode, requestId, error) { + if (requestId !== root.targetRequestSerial) return + try { + var response = error ? ({ ok: false, error: error }) : JSON.parse(String(raw || "{}")) + if (!response.ok) { + root.targetStatus = String(response.error || "Could not validate this value") root.targetLevel = "error" + root.expandedTarget = "" + return } + var result = response.result || ({}) + root.targetStatus = String(result.message || "") + root.targetLevel = String(result.level || "") + root.expandedTarget = String(result.expanded || "") + } catch (e) { + root.targetStatus = "Could not validate this value" + root.targetLevel = "error" } } } diff --git a/bin/qopen b/bin/qopen index ce8ee7e..52cf6fb 100755 --- a/bin/qopen +++ b/bin/qopen @@ -5,17 +5,19 @@ from __future__ import annotations import argparse import copy +import errno import fcntl import ipaddress import json import os import re +import secrets +import selectors import shlex import shutil import stat import subprocess import sys -import tempfile import time import unicodedata import urllib.parse @@ -24,11 +26,21 @@ from pathlib import Path from typing import Any, Callable -VERSION = "2.5.0" -CONFIG_PATH = Path(os.path.expandvars(os.environ.get("QOPEN_CONFIG", "~/.config/qopen/config.json"))).expanduser() -LOCK_PATH = CONFIG_PATH.with_name(CONFIG_PATH.name + ".lock") +VERSION = "2.5.1" +DEFAULT_CONFIG_PATH = Path("~/.config/qopen/config.json").expanduser() +CONFIG_PATH = Path(os.path.expandvars(os.environ.get( + "QOPEN_CONFIG", str(DEFAULT_CONFIG_PATH) +))).expanduser() BACKUP_PATH = CONFIG_PATH.with_name(CONFIG_PATH.name + ".bak") +MAX_CONFIG_BYTES = 1024 * 1024 +MAX_API_INPUT_BYTES = 32 * 1024 +MAX_API_OUTPUT_BYTES = 1280 * 1024 +MAX_ERROR_CHARS = 4096 +MAX_ITEMS = 2000 +STATE_LOCK_TIMEOUT_SECONDS = 2.0 +STATE_LOCK_RETRY_SECONDS = 0.02 + SUPPORTED_TYPES = {"web", "file", "project", "tui", "command", "ssh"} RESERVED_IDS = {"add", "edit", "remove"} ID_PATTERN = re.compile(r"^[a-z0-9][a-z0-9_-]*$") @@ -111,6 +123,8 @@ MENU_WIDTH = 720 MENU_MAX_HEIGHT = 640 MENU_SETTLE_SECONDS = 0.08 PATH_BROWSE_LIMIT = 1000 +PATH_BROWSE_SCAN_LIMIT = 5000 +PATH_BROWSE_TIMEOUT_SECONDS = 1.5 GROUP_META = { "projects": ("󰉋", "Projects", "Directories opened in a terminal"), "config": ("󰒓", "Configuration", "Frequently edited configuration files"), @@ -138,6 +152,10 @@ class QOpenError(Exception): pass +class StateFileMissing(QOpenError): + pass + + def fail(message: str) -> None: raise QOpenError(message if message.startswith("QOpen:") else f"QOpen: {message}") @@ -308,19 +326,122 @@ def spawn_detached(argv: list[str], cwd: Path | None = None) -> None: fail(f"failed to launch {argv[0]}: {exc}") +def _stop_process(process: subprocess.Popen[bytes]) -> None: + if process.poll() is not None: + return + process.terminate() + try: + process.wait(timeout=0.25) + except subprocess.TimeoutExpired: + process.kill() + try: + process.wait(timeout=0.25) + except subprocess.TimeoutExpired: + pass + + +def run_bounded(command: list[str], *, input_text: str | None = None, + timeout: float, stdout_limit: int, stderr_limit: int = 16 * 1024 + ) -> subprocess.CompletedProcess[str]: + """Run a helper without retaining unbounded output in the QOpen process.""" + input_data = input_text.encode("utf-8") if input_text is not None else None + try: + process = subprocess.Popen( + command, + stdin=subprocess.PIPE if input_data is not None else subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + except OSError as exc: + fail(f"failed to start {command[0]}: {exc}") + + assert process.stdout is not None and process.stderr is not None + selector = selectors.DefaultSelector() + output = {"stdout": bytearray(), "stderr": bytearray()} + input_offset = 0 + deadline = time.monotonic() + timeout + streams: list[Any] = [process.stdout, process.stderr] + try: + os.set_blocking(process.stdout.fileno(), False) + os.set_blocking(process.stderr.fileno(), False) + selector.register(process.stdout, selectors.EVENT_READ, "stdout") + selector.register(process.stderr, selectors.EVENT_READ, "stderr") + if process.stdin is not None: + streams.append(process.stdin) + os.set_blocking(process.stdin.fileno(), False) + selector.register(process.stdin, selectors.EVENT_WRITE, "stdin") + + while selector.get_map(): + remaining = deadline - time.monotonic() + if remaining <= 0: + _stop_process(process) + raise subprocess.TimeoutExpired(command, timeout) + for key, _events in selector.select(min(0.05, remaining)): + stream = key.fileobj + channel = key.data + if channel == "stdin": + assert process.stdin is not None and input_data is not None + try: + count = os.write(process.stdin.fileno(), input_data[input_offset:input_offset + 65536]) + except BrokenPipeError: + count = 0 + input_offset += count + if count == 0 or input_offset >= len(input_data): + selector.unregister(stream) + process.stdin.close() + continue + + try: + chunk = os.read(stream.fileno(), 65536) + except BlockingIOError: + continue + if not chunk: + selector.unregister(stream) + stream.close() + continue + limit = stdout_limit if channel == "stdout" else stderr_limit + output[channel].extend(chunk) + if len(output[channel]) > limit: + _stop_process(process) + fail(f"{command[0]} exceeded its {channel} safety limit") + + remaining = max(0.01, deadline - time.monotonic()) + try: + return_code = process.wait(timeout=remaining) + except subprocess.TimeoutExpired: + _stop_process(process) + raise subprocess.TimeoutExpired(command, timeout) + finally: + selector.close() + for stream in streams: + if not stream.closed: + stream.close() + + return subprocess.CompletedProcess( + command, + return_code, + output["stdout"].decode("utf-8", errors="replace"), + output["stderr"].decode("utf-8", errors="replace"), + ) + + # Omarchy compatibility layer def ui_select(prompt: str, options: list[str], width: int = MENU_WIDTH, max_height: int = MENU_MAX_HEIGHT) -> str | None: if not options: return None require_command("omarchy-menu-select") + command = ["omarchy-menu-select", prompt, "--", "--width", str(width), + "--maxheight", str(max_height)] try: - command = ["omarchy-menu-select", prompt, "--", "--width", str(width), - "--maxheight", str(max_height)] - result = subprocess.run(command, input="\n".join(options) + "\n", - text=True, capture_output=True, check=False) - except OSError as exc: - fail(f"failed to start picker: {exc}") + result = run_bounded( + command, + input_text="\n".join(options) + "\n", + timeout=3600, + stdout_limit=64 * 1024, + ) + except subprocess.TimeoutExpired: + fail("picker timed out") time.sleep(MENU_SETTLE_SECONDS) if result.returncode != 0: return None @@ -331,10 +452,13 @@ def ui_select(prompt: str, options: list[str], width: int = MENU_WIDTH, def ui_input(prompt: str) -> str | None: require_command("omarchy-menu-input") try: - result = subprocess.run(["omarchy-menu-input", prompt], text=True, - capture_output=True, check=False) - except OSError as exc: - fail(f"failed to start input dialog: {exc}") + result = run_bounded( + ["omarchy-menu-input", prompt], + timeout=3600, + stdout_limit=16 * 1024, + ) + except subprocess.TimeoutExpired: + fail("input dialog timed out") time.sleep(MENU_SETTLE_SECONDS) if result.returncode != 0: return None @@ -346,9 +470,12 @@ def read_clipboard_text() -> str | None: if not has_command("wl-paste"): return None try: - result = subprocess.run(["wl-paste", "--type", "text", "--no-newline"], - text=True, capture_output=True, check=False, timeout=1) - except (OSError, subprocess.TimeoutExpired): + result = run_bounded( + ["wl-paste", "--type", "text", "--no-newline"], + timeout=1, + stdout_limit=16 * 1024, + ) + except subprocess.TimeoutExpired: return None if result.returncode != 0: return None @@ -458,6 +585,8 @@ def validate_item(item: Any, defaults: dict[str, Any], seen_ids: set[str], prefi if not isinstance(item_id, str) or not item_id: fail(f"{prefix}.id must be a non-empty string") require_safe_text(item_id, f"{prefix}.id") + if len(item_id) > 128: + fail(f"{prefix}.id exceeds 128 characters") if not ID_PATTERN.fullmatch(item_id): fail(f"invalid id '{item_id}'. Use lowercase letters, numbers, _ or -") if item_id in RESERVED_IDS: @@ -469,6 +598,8 @@ def validate_item(item: Any, defaults: dict[str, Any], seen_ids: set[str], prefi if not isinstance(item.get("name"), str) or not item["name"].strip(): fail(f"{prefix}.name must be a non-empty string") require_safe_text(item["name"], f"{prefix}.name") + if len(item["name"]) > 256: + fail(f"{prefix}.name exceeds 256 characters") item_type = item.get("type") if not isinstance(item_type, str) or item_type not in SUPPORTED_TYPES: fail(f"unsupported type '{item_type}' for {item_id}") @@ -477,6 +608,12 @@ def validate_item(item: Any, defaults: dict[str, Any], seen_ids: set[str], prefi fail(f"{prefix}.{field} must be a string") if field in item: require_safe_text(item[field], f"{prefix}.{field}") + if len(str(item.get("group", ""))) > 128: + fail(f"{prefix}.group exceeds 128 characters") + if len(str(item.get("icon", ""))) > 32: + fail(f"{prefix}.icon exceeds 32 characters") + if len(str(item.get("description", ""))) > 4096: + fail(f"{prefix}.description exceeds 4096 characters") if "group" in item and not item["group"].strip(): fail(f"{prefix}.group must be a non-empty string") if "favorite" in item and not isinstance(item["favorite"], bool): @@ -486,6 +623,8 @@ def validate_item(item: Any, defaults: dict[str, Any], seen_ids: set[str], prefi if not isinstance(item.get("target"), str) or not item["target"].strip(): fail(f"{prefix}.target must be a non-empty string") require_safe_text(item["target"], f"{prefix}.target") + if len(item["target"]) > 8192: + fail(f"{prefix}.target exceeds 8192 characters") if item_type == "web": mode = item.get("mode", defaults.get("webMode", "app")) if not isinstance(mode, str) or mode not in {"app", "browser"}: @@ -504,6 +643,8 @@ def validate_item(item: Any, defaults: dict[str, Any], seen_ids: set[str], prefi if not isinstance(command, list) or not command or any(not isinstance(arg, str) or not arg for arg in command): fail(f"{prefix}.command must be a non-empty string array") command_argv(command, f"{prefix}.command") + if sum(len(argument) for argument in command) > 8192: + fail(f"{prefix}.command exceeds 8192 characters") if item_type == "command" and "terminal" in item and not isinstance(item["terminal"], bool): fail(f"{prefix}.terminal must be boolean") if item_type == "ssh": @@ -526,154 +667,341 @@ def validate_config(config: Any) -> dict[str, Any]: items = config.get("items") if not isinstance(items, list): fail("items must be an array") + if len(items) > MAX_ITEMS: + fail(f"items exceeds the {MAX_ITEMS}-resource safety limit") seen_ids: set[str] = set() for index, item in enumerate(items): validate_item(item, defaults, seen_ids, f"items[{index}]") return config -def read_config_unlocked(create_if_missing: bool = False) -> dict[str, Any]: - if not CONFIG_PATH.exists(): - if create_if_missing: - return default_config() - fail(f"config not found: {CONFIG_PATH}\nRun: qopen add") - try: - with CONFIG_PATH.open("r", encoding="utf-8") as file: - config = json.load(file) - except json.JSONDecodeError as exc: - fail(f"invalid config.json (line {exc.lineno}, column {exc.colno}): {exc.msg}") - except OSError as exc: - fail(f"cannot read config: {exc}") - return validate_config(config) - - -@contextmanager -def config_lock(): - CONFIG_PATH.parent.mkdir(parents=True, exist_ok=True) - try: - descriptor = os.open(LOCK_PATH, os.O_RDWR | os.O_CREAT, 0o600) - with os.fdopen(descriptor, "a+", encoding="utf-8") as lock_file: - fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX) - try: - yield - finally: - fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN) - except OSError as exc: - fail(f"cannot lock config: {exc}") - +class SecureStateStore: + """Descriptor-anchored access to QOpen's complete state lifecycle.""" + + def __init__(self, config_path: Path = CONFIG_PATH, + allow_insecure_directory: bool = False) -> None: + self.config_path = config_path + self.config_name = config_path.name + self.backup_name = f"{self.config_name}.bak" + self.allow_insecure_directory = allow_insecure_directory + self.dir_fd = -1 + self.directory_path = config_path.parent + + def __enter__(self) -> "SecureStateStore": + if not self.config_path.is_absolute(): + fail("QOPEN_CONFIG must expand to an absolute path") + if self.config_name in {"", ".", ".."} or "/" in self.config_name: + fail("QOPEN_CONFIG has an invalid filename") + required_flags = ("O_CLOEXEC", "O_DIRECTORY", "O_NOFOLLOW", "O_NONBLOCK") + if any(not hasattr(os, name) for name in required_flags): + fail("secure state-file operations are not supported on this platform") + + parent = self.config_path.parent + root_fd = os.open("/", os.O_RDONLY | os.O_DIRECTORY | os.O_CLOEXEC) + current_fd = root_fd + try: + for component in parent.parts[1:]: + if component in {"", ".", ".."}: + fail("QOPEN_CONFIG contains an unsafe directory component") + try: + child_fd = os.open( + component, + os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW | os.O_CLOEXEC, + dir_fd=current_fd, + ) + except FileNotFoundError: + try: + os.mkdir(component, 0o700, dir_fd=current_fd) + os.fsync(current_fd) + child_fd = os.open( + component, + os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW | os.O_CLOEXEC, + dir_fd=current_fd, + ) + except OSError as exc: + fail(f"cannot create secure state directory {parent}: {exc}") + except OSError as exc: + fail(f"cannot open secure state directory {parent}: {exc}") + if current_fd != root_fd: + os.close(current_fd) + current_fd = child_fd + + metadata = os.fstat(current_fd) + if not stat.S_ISDIR(metadata.st_mode): + fail(f"state path is not a directory: {parent}") + if metadata.st_uid != os.geteuid(): + fail(f"state directory is not owned by the current user: {parent}") + mode = stat.S_IMODE(metadata.st_mode) + if not self.allow_insecure_directory and mode & 0o022: + fail(f"state directory is writable by another account: {parent} ({mode:04o})") + self.dir_fd = current_fd + self.directory_path = parent + current_fd = -1 + return self + finally: + if current_fd >= 0: + os.close(current_fd) + if root_fd >= 0 and root_fd != current_fd: + try: + os.close(root_fd) + except OSError: + pass + + def __exit__(self, _exc_type: object, _exc: object, _traceback: object) -> None: + if self.dir_fd >= 0: + os.close(self.dir_fd) + self.dir_fd = -1 + + def path_for(self, name: str) -> Path: + return self.directory_path / name + + def _validate_regular(self, fd: int, name: str, label: str, + allow_insecure_mode: bool = False) -> os.stat_result: + metadata = os.fstat(fd) + if not stat.S_ISREG(metadata.st_mode): + fail(f"{label} is not a regular file: {self.path_for(name)}") + if metadata.st_uid != os.geteuid(): + fail(f"{label} is not owned by the current user: {self.path_for(name)}") + if metadata.st_nlink != 1: + fail(f"{label} must not have hard links: {self.path_for(name)}") + mode = stat.S_IMODE(metadata.st_mode) + if not allow_insecure_mode and mode & 0o022: + fail(f"{label} is writable by another account: {self.path_for(name)} ({mode:04o})") + return metadata + + def open_regular(self, name: str, label: str, writable: bool = False, + allow_insecure_mode: bool = False) -> int: + flags = (os.O_RDWR if writable else os.O_RDONLY) | os.O_CLOEXEC + flags |= os.O_NOFOLLOW | os.O_NONBLOCK + try: + fd = os.open(name, flags, dir_fd=self.dir_fd) + except FileNotFoundError: + raise + except OSError as exc: + fail(f"cannot open {label}: {exc}") + try: + self._validate_regular(fd, name, label, allow_insecure_mode) + return fd + except BaseException: + os.close(fd) + raise + + def read_bytes(self, name: str, label: str, + limit: int = MAX_CONFIG_BYTES) -> bytes: + fd = self.open_regular(name, label) + try: + before = self._validate_regular(fd, name, label) + if before.st_size > limit: + fail(f"{label} exceeds the {limit}-byte safety limit") + chunks: list[bytes] = [] + total = 0 + while total <= limit: + chunk = os.read(fd, min(65536, limit + 1 - total)) + if not chunk: + break + chunks.append(chunk) + total += len(chunk) + if total > limit: + fail(f"{label} exceeds the {limit}-byte safety limit") + after = self._validate_regular(fd, name, label) + if (before.st_dev, before.st_ino, before.st_size, + before.st_mtime_ns, before.st_ctime_ns) != ( + after.st_dev, after.st_ino, after.st_size, + after.st_mtime_ns, after.st_ctime_ns): + fail(f"{label} changed while it was being read; retry") + return b"".join(chunks) + except OSError as exc: + fail(f"cannot read {label}: {exc}") + finally: + os.close(fd) -def atomic_write_locked(config: dict[str, Any], backup: bool = True) -> None: - validate_config(config) - CONFIG_PATH.parent.mkdir(parents=True, exist_ok=True) - original_mode = 0o600 - if CONFIG_PATH.exists(): - original_mode = stat.S_IMODE(CONFIG_PATH.stat().st_mode) - if backup: + def read_config(self, name: str | None = None, + label: str = "config") -> tuple[dict[str, Any], bytes]: + target = name or self.config_name + try: + raw = self.read_bytes(target, label) + except FileNotFoundError: + raise StateFileMissing(f"QOpen: {label} not found: {self.path_for(target)}") + try: + text = raw.decode("utf-8") + except UnicodeDecodeError as exc: + fail(f"invalid UTF-8 in {label} at byte {exc.start}") + try: + config = json.loads(text) + except json.JSONDecodeError as exc: + fail(f"invalid {label} (line {exc.lineno}, column {exc.colno}): {exc.msg}") + except RecursionError: + fail(f"invalid {label}: JSON nesting is too deep") + return validate_config(config), raw + + @contextmanager + def lock(self, exclusive: bool = True): + operation = fcntl.LOCK_EX if exclusive else fcntl.LOCK_SH + deadline = time.monotonic() + STATE_LOCK_TIMEOUT_SECONDS + while True: try: - shutil.copy2(CONFIG_PATH, BACKUP_PATH) - if stat.S_IMODE(CONFIG_PATH.stat().st_mode) == 0o600: - os.chmod(BACKUP_PATH, 0o600) + fcntl.flock(self.dir_fd, operation | fcntl.LOCK_NB) + break except OSError as exc: - fail(f"failed to create backup: {exc}") - - fd, temp_name = tempfile.mkstemp(prefix=".qopen-config-", suffix=".tmp", dir=CONFIG_PATH.parent) - temp_path = Path(temp_name) - try: - os.chmod(temp_path, original_mode) - with os.fdopen(fd, "w", encoding="utf-8") as file: - json.dump(config, file, ensure_ascii=False, indent=2) - file.write("\n") - file.flush() - os.fsync(file.fileno()) - os.replace(temp_path, CONFIG_PATH) - directory_fd = os.open(CONFIG_PATH.parent, os.O_DIRECTORY) + if exc.errno not in {errno.EACCES, errno.EAGAIN, errno.EWOULDBLOCK}: + fail(f"cannot lock QOpen state directory: {exc}") + if time.monotonic() >= deadline: + fail("timed out waiting for the QOpen state lock") + time.sleep(STATE_LOCK_RETRY_SECONDS) try: - os.fsync(directory_fd) + yield finally: - os.close(directory_fd) - except OSError as exc: - fail(f"failed to write config: {exc}") - finally: - if temp_path.exists(): - temp_path.unlink(missing_ok=True) + fcntl.flock(self.dir_fd, fcntl.LOCK_UN) - -def read_config_path(path: Path, label: str) -> dict[str, Any]: - try: - with path.open("r", encoding="utf-8") as file: - config = json.load(file) - except FileNotFoundError: - fail(f"{label} not found: {path}") - except json.JSONDecodeError as exc: - fail(f"invalid {label} (line {exc.lineno}, column {exc.colno}): {exc.msg}") - except OSError as exc: - fail(f"cannot read {label}: {exc}") - return validate_config(config) + def _validate_destination(self, name: str, label: str) -> None: + try: + fd = self.open_regular(name, label) + except FileNotFoundError: + return + else: + os.close(fd) + + def write_atomic(self, name: str, data: bytes, label: str) -> None: + if len(data) > MAX_CONFIG_BYTES: + fail(f"{label} exceeds the {MAX_CONFIG_BYTES}-byte safety limit") + self._validate_destination(name, label) + temp_name = f".qopen-{secrets.token_hex(12)}.tmp" + temp_fd = -1 + try: + temp_fd = os.open( + temp_name, + os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW | os.O_CLOEXEC, + 0o600, + dir_fd=self.dir_fd, + ) + written = 0 + while written < len(data): + count = os.write(temp_fd, data[written:]) + if count <= 0: + fail(f"failed to write {label}") + written += count + os.fchmod(temp_fd, 0o600) + os.fsync(temp_fd) + os.close(temp_fd) + temp_fd = -1 + os.replace( + temp_name, + name, + src_dir_fd=self.dir_fd, + dst_dir_fd=self.dir_fd, + ) + os.fsync(self.dir_fd) + except OSError as exc: + fail(f"failed to write {label}: {exc}") + finally: + if temp_fd >= 0: + os.close(temp_fd) + try: + os.unlink(temp_name, dir_fd=self.dir_fd) + except FileNotFoundError: + pass + except OSError: + pass + + def write_config(self, config: dict[str, Any], previous: bytes | None, + backup: bool = True) -> None: + validate_config(config) + serialized = (json.dumps(config, ensure_ascii=False, indent=2) + "\n").encode("utf-8") + if len(serialized) > MAX_CONFIG_BYTES: + fail(f"config exceeds the {MAX_CONFIG_BYTES}-byte safety limit") + if backup and previous is not None: + self.write_atomic(self.backup_name, previous, "backup") + self.write_atomic(self.config_name, serialized, "config") + + def unique_snapshot_name(self) -> str: + timestamp = time.strftime("%Y%m%d-%H%M%S") + for counter in range(1, 1001): + suffix = "" if counter == 1 else f"-{counter}" + candidate = f"{self.config_name}.invalid-{timestamp}{suffix}" + try: + os.stat(candidate, dir_fd=self.dir_fd, follow_symlinks=False) + except FileNotFoundError: + return candidate + except OSError as exc: + fail(f"cannot inspect recovery snapshot: {exc}") + fail("too many recovery snapshots share the same timestamp") -def recovery_snapshot_path() -> Path: - timestamp = time.strftime("%Y%m%d-%H%M%S") - candidate = CONFIG_PATH.with_name(f"{CONFIG_PATH.name}.invalid-{timestamp}") - counter = 2 - while candidate.exists(): - candidate = CONFIG_PATH.with_name(f"{CONFIG_PATH.name}.invalid-{timestamp}-{counter}") - counter += 1 - return candidate +def read_config_unlocked(create_if_missing: bool = False) -> dict[str, Any]: + with SecureStateStore() as store, store.lock(exclusive=False): + try: + config, _raw = store.read_config() + return config + except StateFileMissing: + if create_if_missing: + return default_config() + raise def recover_backup() -> Path | None: """Validate and atomically restore the last-known-good catalog.""" - with config_lock(): - backup_config = read_config_path(BACKUP_PATH, "backup") + with SecureStateStore() as store, store.lock(): + backup_config, backup_raw = store.read_config(store.backup_name, "backup") snapshot: Path | None = None - if CONFIG_PATH.exists(): - snapshot = recovery_snapshot_path() - try: - shutil.copy2(CONFIG_PATH, snapshot) - os.chmod(snapshot, 0o600) - except OSError as exc: - fail(f"failed to preserve the current catalog: {exc}") - atomic_write_locked(backup_config, backup=False) try: - os.chmod(CONFIG_PATH, 0o600) - os.chmod(BACKUP_PATH, 0o600) - os.chmod(LOCK_PATH, 0o600) - except OSError as exc: - fail(f"catalog restored but permissions could not be secured: {exc}") + current_raw = store.read_bytes(store.config_name, "config") + except FileNotFoundError: + current_raw = None + if current_raw is not None: + snapshot_name = store.unique_snapshot_name() + store.write_atomic(snapshot_name, current_raw, "recovery snapshot") + snapshot = store.path_for(snapshot_name) + validate_config(backup_config) + store.write_atomic(store.config_name, backup_raw, "config") return snapshot def fix_permissions() -> list[Path]: - """Restrict QOpen state files without changing their contents.""" + """Restrict QOpen state files without following pathnames.""" changed: list[Path] = [] - with config_lock(): - for path in (CONFIG_PATH, BACKUP_PATH, LOCK_PATH): - if not path.exists(): + with SecureStateStore(allow_insecure_directory=True) as store, store.lock(): + directory_mode = stat.S_IMODE(os.fstat(store.dir_fd).st_mode) + if store.config_path == DEFAULT_CONFIG_PATH and directory_mode != 0o700: + os.fchmod(store.dir_fd, 0o700) + changed.append(store.directory_path) + elif directory_mode & 0o022: + fail("custom QOPEN_CONFIG directory is writable by another account; " + "move the catalog into a private directory") + for name, label in ((store.config_name, "config"), (store.backup_name, "backup")): + try: + fd = store.open_regular(name, label, allow_insecure_mode=True) + except FileNotFoundError: continue try: - if stat.S_IMODE(path.stat().st_mode) != 0o600: - os.chmod(path, 0o600) - changed.append(path) - except OSError as exc: - fail(f"cannot secure {path}: {exc}") + if stat.S_IMODE(os.fstat(fd).st_mode) != 0o600: + os.fchmod(fd, 0o600) + changed.append(store.path_for(name)) + finally: + os.close(fd) return changed -def ensure_config() -> None: - if CONFIG_PATH.exists(): - return - with config_lock(): - if not CONFIG_PATH.exists(): - atomic_write_locked(default_config(), backup=False) +def ensure_config() -> dict[str, Any]: + with SecureStateStore() as store, store.lock(): + try: + config, _raw = store.read_config() + except StateFileMissing: + config = default_config() + store.write_config(config, previous=None, backup=False) + return config def mutate_config(mutator: Callable[[dict[str, Any]], Any]) -> Any: - with config_lock(): - config = read_config_unlocked(create_if_missing=True) + with SecureStateStore() as store, store.lock(): + try: + config, previous = store.read_config() + except StateFileMissing: + config = default_config() + previous = None working = copy.deepcopy(config) result = mutator(working) validate_config(working) - atomic_write_locked(working, backup=CONFIG_PATH.exists()) + store.write_config(working, previous=previous, backup=previous is not None) return result @@ -1185,10 +1513,29 @@ def noninteractive_add(argv: list[str]) -> int: # Machine API used by the native Omarchy plugin editor. It deliberately keeps # all validation and persistence in Python while QML owns only presentation. def api_emit(ok: bool, **payload: Any) -> None: - print(json.dumps({"ok": ok, **payload}, ensure_ascii=False, separators=(",", ":"))) + body = {"ok": ok, **payload} + if "error" in body: + error = "".join( + character if ord(character) >= 32 and ord(character) != 127 else " " + for character in str(body["error"]) + ) + body["error"] = error[:MAX_ERROR_CHARS] + try: + encoded = json.dumps(body, ensure_ascii=False, separators=(",", ":")).encode("utf-8") + except (TypeError, ValueError, RecursionError): + encoded = b'{"ok":false,"error":"QOpen could not serialize the API response"}' + if len(encoded) > MAX_API_OUTPUT_BYTES: + encoded = b'{"ok":false,"error":"QOpen API response exceeded its safety limit"}' + try: + sys.stdout.buffer.write(encoded + b"\n") + sys.stdout.buffer.flush() + except BrokenPipeError: + return def parse_api_payload(value: str, field: str) -> dict[str, Any]: + if len(value.encode("utf-8")) > MAX_API_INPUT_BYTES: + fail(f"{field} exceeds the {MAX_API_INPUT_BYTES}-byte safety limit") try: parsed = json.loads(value) except json.JSONDecodeError as exc: @@ -1305,6 +1652,8 @@ def validate_api_item(config: dict[str, Any], item: dict[str, Any], def api_check_target(item_type: str, value: str) -> dict[str, Any]: + if len(value.encode("utf-8")) > 8192: + fail("value exceeds the 8192-byte safety limit") try: require_safe_text(value, "value") except QOpenError as exc: @@ -1349,7 +1698,13 @@ def api_check_target(item_type: str, value: str) -> dict[str, Any]: def api_browse_path(path_value: str, item_type: str, show_hidden: bool = False, request_id: str = "") -> dict[str, Any]: """List one local directory without loading Qt, GTK, GIO or GVFS.""" + if len(path_value.encode("utf-8")) > 4096: + fail("directory path exceeds the 4096-byte safety limit") + if len(request_id) > 64: + fail("request id exceeds 64 characters") path = expand_path(path_value or str(Path.home())) + deadline = time.monotonic() + PATH_BROWSE_TIMEOUT_SECONDS + scan_truncated = False try: if path.is_file(): path = path.parent @@ -1360,8 +1715,13 @@ def api_browse_path(path_value: str, item_type: str, show_hidden: bool = False, path = path.absolute() entries: list[dict[str, Any]] = [] + scanned = 0 with os.scandir(path) as directory: for entry in directory: + scanned += 1 + if scanned > PATH_BROWSE_SCAN_LIMIT or time.monotonic() >= deadline: + scan_truncated = True + break if not show_hidden and entry.name.startswith("."): continue try: @@ -1380,13 +1740,16 @@ def api_browse_path(path_value: str, item_type: str, show_hidden: bool = False, "hidden": entry.name.startswith("."), "symlink": entry.is_symlink(), }) + if len(entries) > PATH_BROWSE_LIMIT: + scan_truncated = True + break except PermissionError: fail(f"permission denied: {path}") except OSError as exc: fail(f"cannot read directory {path}: {exc.strerror or exc}") entries.sort(key=lambda item: (item["kind"] != "directory", item["name"].casefold())) - truncated = len(entries) > PATH_BROWSE_LIMIT + truncated = scan_truncated or len(entries) > PATH_BROWSE_LIMIT if truncated: entries = entries[:PATH_BROWSE_LIMIT] parent = path.parent @@ -1416,6 +1779,7 @@ def api_cli(argv: list[str]) -> int: subparsers.add_parser("catalog") subparsers.add_parser("recover") subparsers.add_parser("fix-permissions") + subparsers.add_parser("clipboard-read") check_parser = subparsers.add_parser("check-target") check_parser.add_argument("--type", required=True, choices=sorted(SUPPORTED_TYPES)) check_parser.add_argument("--value", required=True) @@ -1427,6 +1791,15 @@ def api_cli(argv: list[str]) -> int: args = parser.parse_args(argv) try: + if args.action == "clipboard-read": + value = read_clipboard_text() + if not value: + fail("clipboard is unavailable or does not contain text") + first_line = value.strip().splitlines()[0] + if len(first_line.encode("utf-8")) > 8192: + fail("clipboard text exceeds the 8192-byte safety limit") + api_emit(True, result={"text": first_line}) + return 0 if args.action == "check-target": api_emit(True, result=api_check_target(args.type, args.value)) return 0 @@ -1435,8 +1808,7 @@ def api_cli(argv: list[str]) -> int: args.request_id)) return 0 if args.action == "catalog": - ensure_config() - api_emit(True, action="catalog", result=read_config_unlocked()) + api_emit(True, action="catalog", result=ensure_config()) return 0 if args.action == "recover": snapshot = recover_backup() @@ -1479,12 +1851,7 @@ def api_cli(argv: list[str]) -> int: def edit_config() -> int: - ensure_config() - config = read_config_unlocked() - editor = command_argv(config.get("defaults", {}).get("editor", os.environ.get("EDITOR", "nvim")), "editor") - require_command(editor[0]) - launch_tui([*editor, str(CONFIG_PATH)], "org.qopen.config") - return 0 + fail("direct catalog-file editing is disabled; use QOpen's editor or 'qopen edit '") def doctor() -> int: @@ -1496,41 +1863,73 @@ def doctor() -> int: if not ok: failed = True - def check_private(path: Path, required: bool = True) -> None: - if not path.exists(): - if required: - check(False, f"missing state file: {path}") - else: - print(f"~ optional state file absent: {path}") - return - mode = stat.S_IMODE(path.stat().st_mode) - check(mode == 0o600, f"private permissions {path} ({mode:04o}; expected 0600)") - - check(CONFIG_PATH.exists(), f"config: {CONFIG_PATH}") - if not CONFIG_PATH.exists(): - return 1 - check_private(CONFIG_PATH) try: - config = read_config_unlocked() - check(True, "config schema") + with SecureStateStore(allow_insecure_directory=True) as store: + directory_mode = stat.S_IMODE(os.fstat(store.dir_fd).st_mode) + expected_directory_mode = directory_mode == 0o700 \ + if store.config_path == DEFAULT_CONFIG_PATH else not (directory_mode & 0o022) + expected_text = "expected 0700" if store.config_path == DEFAULT_CONFIG_PATH \ + else "must not be group- or world-writable" + check(expected_directory_mode, + f"trusted state directory {store.directory_path} " + f"({directory_mode:04o}; {expected_text})") + try: + with store.lock(exclusive=False): + config, _raw = store.read_config() + check(True, f"config: {CONFIG_PATH}") + check(True, "config schema") + except QOpenError as exc: + check(False, str(exc)) + try: + with store.lock(exclusive=False): + store.read_config(store.backup_name, "backup") + check(True, f"recoverable backup: {BACKUP_PATH}") + print("~ run: qopen recover") + except QOpenError as backup_exc: + check(False, str(backup_exc)) + return 1 + + for name, label, required in ( + (store.config_name, "config", True), + (store.backup_name, "backup", False)): + try: + fd = store.open_regular(name, label, allow_insecure_mode=True) + except FileNotFoundError: + if required: + check(False, f"missing state file: {store.path_for(name)}") + else: + print(f"~ optional state file absent: {store.path_for(name)}") + continue + except QOpenError as exc: + check(False, str(exc)) + continue + try: + mode = stat.S_IMODE(os.fstat(fd).st_mode) + check(mode == 0o600, + f"private permissions {store.path_for(name)} " + f"({mode:04o}; expected 0600)") + finally: + os.close(fd) + + try: + writable_fd = store.open_regular( + store.config_name, "config", writable=True, allow_insecure_mode=True + ) + except (FileNotFoundError, QOpenError): + check(False, "config writable") + else: + os.close(writable_fd) + check(True, "config writable") + try: + with store.lock(): + pass + check(True, "state directory lock") + except QOpenError: + check(False, "state directory lock") except QOpenError as exc: check(False, str(exc)) - try: - read_config_path(BACKUP_PATH, "backup") - check(True, f"recoverable backup: {BACKUP_PATH}") - print("~ run: qopen recover") - except QOpenError as backup_exc: - check(False, str(backup_exc)) return 1 - check(os.access(CONFIG_PATH, os.W_OK), "config writable") - try: - with config_lock(): - pass - check(True, "config lock") - except QOpenError: - check(False, "config lock") - check_private(LOCK_PATH) - check_private(BACKUP_PATH, required=False) + check(has_command("omarchy-menu-select"), "omarchy-menu-select") check(has_command("omarchy-menu-input"), "omarchy-menu-input") print(f"{'✓' if has_command('wl-paste') else '~'} wl-paste (clipboard path input)") diff --git a/manifest.json b/manifest.json index a7a5be8..e1539ab 100644 --- a/manifest.json +++ b/manifest.json @@ -2,7 +2,7 @@ "schemaVersion": 1, "id": "qopen.launcher", "name": "QOpen", - "version": "2.5.0", + "version": "2.5.1", "author": "CoderLambert", "description": "Unified launcher for web, files, projects, TUI tools, commands and SSH targets", "kinds": [ diff --git a/tests/test_backend.py b/tests/test_backend.py index 5bd6edb..4abe43f 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -1,11 +1,15 @@ from __future__ import annotations +import fcntl import json import os import shlex import stat import subprocess +import sys import tempfile +import threading +import time import unittest from pathlib import Path @@ -28,12 +32,17 @@ def run_qopen( *arguments: str, config: Path | None = None, check: bool = True, + environment_overrides: dict[str, str] | None = None, + timeout: float = 10, ) -> subprocess.CompletedProcess[str]: environment = os.environ.copy() environment.pop("DISPLAY", None) environment.pop("WAYLAND_DISPLAY", None) + environment.pop("QOPEN_CONFIG", None) if config is not None: environment["QOPEN_CONFIG"] = str(config) + if environment_overrides: + environment.update(environment_overrides) return subprocess.run( [str(QOPEN), *arguments], cwd=REPOSITORY, @@ -41,6 +50,7 @@ def run_qopen( text=True, capture_output=True, check=check, + timeout=timeout, ) def api( @@ -190,6 +200,13 @@ def test_qml_does_not_load_native_file_dialogs(self) -> None: self.assertNotIn("QtQuick.Dialogs", qml_source) self.assertNotIn("FileDialog {", qml_source) self.assertNotIn("FolderDialog {", qml_source) + self.assertNotIn("FileView {", qml_source) + self.assertNotIn("StdioCollector {", qml_source) + bounded_source = (REPOSITORY / "BoundedProcess.qml").read_text(encoding="utf-8") + self.assertIn("stdout: SplitParser", bounded_source) + self.assertIn("deadlineTimer.restart()", bounded_source) + self.assertIn("child.signal(15)", bounded_source) + self.assertIn("child.signal(9)", bounded_source) def test_qml_uses_native_mutations_and_closes_the_stock_menu(self) -> None: qopen_source = (REPOSITORY / "QOpen.qml").read_text(encoding="utf-8") @@ -206,7 +223,8 @@ def test_path_picker_tracks_and_rejects_stale_requests(self) -> None: self.assertIn('"--request-id", String(root.activeRequestSerial)', picker_source) self.assertIn("Number(result.requestId) !== root.requestSerial", picker_source) - self.assertIn("if (browseProcess.running) browseProcess.running = false", picker_source) + self.assertIn("if (browseProcess.running) browseProcess.cancel()", picker_source) + self.assertIn("browseProcess.start(command, root.activeRequestSerial)", picker_source) def test_command_arguments_round_trip_through_the_editor_format(self) -> None: with tempfile.TemporaryDirectory() as temporary_directory: @@ -364,7 +382,8 @@ def test_catalog_api_rejects_semantically_invalid_json(self) -> None: self.assertFalse(response["ok"]) self.assertIn("items[0].id", response["error"]) qopen_source = (REPOSITORY / "QOpen.qml").read_text(encoding="utf-8") - self.assertIn('catalogProcess.command = [root.backendPath, "api", "catalog"]', qopen_source) + self.assertIn('[root.backendPath, "api", "catalog"]', qopen_source) + self.assertIn("catalogProcess.start(", qopen_source) def test_catalog_waits_for_the_host_to_inject_the_plugin_directory(self) -> None: qopen_source = (REPOSITORY / "QOpen.qml").read_text(encoding="utf-8") @@ -380,9 +399,9 @@ def test_catalog_waits_for_the_host_to_inject_the_plugin_directory(self) -> None def test_missing_catalog_requests_first_run_initialization(self) -> None: qopen_source = (REPOSITORY / "QOpen.qml").read_text(encoding="utf-8") - self.assertIn('root.configError = "Preparing first-run resources…"', qopen_source) - self.assertIn("onLoadFailed: function(error)", qopen_source) self.assertIn("root.requestCatalogReload()", qopen_source) + self.assertNotIn("onLoadFailed: function(error)", qopen_source) + self.assertNotIn("configPath:", qopen_source) def test_fix_permissions_secures_all_state_files(self) -> None: with tempfile.TemporaryDirectory() as temporary_directory: @@ -392,16 +411,36 @@ def test_fix_permissions_secures_all_state_files(self) -> None: self.api( "create", "--payload", self.project_payload(root, "second"), config=config ) - lock = config.with_name("config.json.lock") backup = config.with_name("config.json.bak") - for path in (config, lock, backup): + for path in (config, backup): path.chmod(0o644) + root.chmod(0o755) result = self.run_qopen("fix-permissions", config=config) - self.assertIn("3 changed", result.stdout) - for path in (config, lock, backup): + self.assertIn("2 changed", result.stdout) + self.assertEqual(stat.S_IMODE(root.stat().st_mode), 0o755) + for path in (config, backup): self.assertEqual(stat.S_IMODE(path.stat().st_mode), 0o600) + self.assertFalse(config.with_name("config.json.lock").exists()) + + def test_fix_permissions_secures_only_the_default_state_directory(self) -> None: + with tempfile.TemporaryDirectory() as temporary_directory: + home = Path(temporary_directory) + environment = {"HOME": str(home)} + self.run_qopen("api", "catalog", environment_overrides=environment) + state = home / ".config" / "qopen" + config = state / "config.json" + state.chmod(0o755) + config.chmod(0o644) + + result = self.run_qopen( + "fix-permissions", environment_overrides=environment + ) + + self.assertIn("2 changed", result.stdout) + self.assertEqual(stat.S_IMODE(state.stat().st_mode), 0o700) + self.assertEqual(stat.S_IMODE(config.stat().st_mode), 0o600) def test_doctor_reports_insecure_config_permissions(self) -> None: with tempfile.TemporaryDirectory() as temporary_directory: @@ -415,6 +454,247 @@ def test_doctor_reports_insecure_config_permissions(self) -> None: self.assertEqual(result.returncode, 1) self.assertIn("expected 0600", result.stdout) + def test_state_parent_and_files_reject_symlinks_and_special_files(self) -> None: + with tempfile.TemporaryDirectory() as temporary_directory: + root = Path(temporary_directory) + real_state = root / "real-state" + real_state.mkdir(mode=0o700) + linked_state = root / "linked-state" + linked_state.symlink_to(real_state, target_is_directory=True) + + parent_response = self.api( + "catalog", config=linked_state / "config.json", check=False + ) + self.assertFalse(parent_response["ok"]) + self.assertIn("secure state directory", parent_response["error"]) + + sentinel = root / "sentinel" + sentinel.write_text("do-not-change", encoding="utf-8") + config = real_state / "config.json" + config.symlink_to(sentinel) + symlink_response = self.api("catalog", config=config, check=False) + self.assertFalse(symlink_response["ok"]) + self.assertEqual(sentinel.read_text(encoding="utf-8"), "do-not-change") + + config.unlink() + os.mkfifo(config, mode=0o600) + started = time.monotonic() + fifo_response = self.api("catalog", config=config, check=False) + self.assertLess(time.monotonic() - started, 1.0) + self.assertFalse(fifo_response["ok"]) + self.assertIn("regular file", fifo_response["error"]) + + def test_backup_symlink_is_rejected_without_touching_its_target(self) -> None: + with tempfile.TemporaryDirectory() as temporary_directory: + root = Path(temporary_directory) + state = root / "state" + state.mkdir(mode=0o700) + config = state / "config.json" + self.api("create", "--payload", self.project_payload(root), config=config) + sentinel = root / "sentinel" + sentinel.write_text("unchanged", encoding="utf-8") + backup = config.with_name("config.json.bak") + backup.unlink() + backup.symlink_to(sentinel) + + response = self.api( + "create", "--payload", self.project_payload(root, "second"), + config=config, check=False, + ) + + self.assertFalse(response["ok"]) + self.assertEqual(sentinel.read_text(encoding="utf-8"), "unchanged") + self.assertFalse(any(item["id"] == "second" for item in json.loads( + config.read_text(encoding="utf-8") + )["items"])) + + def test_hardlinked_and_oversized_catalogs_are_rejected(self) -> None: + with tempfile.TemporaryDirectory() as temporary_directory: + root = Path(temporary_directory) + state = root / "state" + state.mkdir(mode=0o700) + source = root / "source.json" + source.write_text( + json.dumps({"version": 1, "defaults": {}, "items": []}), + encoding="utf-8", + ) + config = state / "config.json" + os.link(source, config) + + linked = self.api("catalog", config=config, check=False) + self.assertFalse(linked["ok"]) + self.assertIn("hard links", linked["error"]) + + config.unlink() + config.write_bytes(b" " * (1024 * 1024 + 1)) + oversized = self.run_qopen( + "api", "catalog", config=config, check=False + ) + payload = json.loads(oversized.stdout) + self.assertFalse(payload["ok"]) + self.assertLess(len(oversized.stdout.encode("utf-8")), 4096) + self.assertIn("safety limit", payload["error"]) + + def test_group_writable_state_file_is_rejected_and_repairable(self) -> None: + with tempfile.TemporaryDirectory() as temporary_directory: + root = Path(temporary_directory) + config = root / "config.json" + self.api("catalog", config=config) + config.chmod(0o660) + + rejected = self.api("catalog", config=config, check=False) + self.assertFalse(rejected["ok"]) + self.assertIn("writable by another account", rejected["error"]) + + repaired = self.run_qopen("fix-permissions", config=config) + self.assertIn("1 changed", repaired.stdout) + self.assertEqual(stat.S_IMODE(config.stat().st_mode), 0o600) + self.assertTrue(self.api("catalog", config=config)["ok"]) + + def test_state_directory_lock_has_a_real_deadline(self) -> None: + with tempfile.TemporaryDirectory() as temporary_directory: + root = Path(temporary_directory) + config = root / "config.json" + self.api("catalog", config=config) + directory_fd = os.open(root, os.O_RDONLY | os.O_DIRECTORY) + try: + fcntl.flock(directory_fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + started = time.monotonic() + response = self.api("catalog", config=config, check=False) + elapsed = time.monotonic() - started + finally: + fcntl.flock(directory_fd, fcntl.LOCK_UN) + os.close(directory_fd) + + self.assertFalse(response["ok"]) + self.assertIn("timed out", response["error"]) + self.assertGreaterEqual(elapsed, 1.8) + self.assertLess(elapsed, 3.5) + + def test_path_browser_bounds_work_and_response_size(self) -> None: + with tempfile.TemporaryDirectory() as temporary_directory: + root = Path(temporary_directory) + for index in range(1002): + (root / f"entry-{index:04d}").mkdir() + + result = self.run_qopen( + "api", "browse-path", "--path", str(root), "--type", "project" + ) + response = json.loads(result.stdout) + + self.assertTrue(response["ok"]) + self.assertTrue(response["result"]["truncated"]) + self.assertEqual(len(response["result"]["entries"]), 1000) + self.assertLess(len(result.stdout.encode("utf-8")), 1280 * 1024) + + def test_clipboard_helper_output_is_bounded_before_the_api_response(self) -> None: + with tempfile.TemporaryDirectory() as temporary_directory: + root = Path(temporary_directory) + helper = root / "wl-paste" + helper.write_text( + f"#!{sys.executable}\nprint('x' * 20000)\n", + encoding="utf-8", + ) + helper.chmod(0o700) + environment = {"PATH": f"{root}:{os.environ.get('PATH', '')}"} + + result = self.run_qopen( + "api", "clipboard-read", + config=root / "config.json", + check=False, + environment_overrides=environment, + ) + response = json.loads(result.stdout) + + self.assertFalse(response["ok"]) + self.assertIn("safety limit", response["error"]) + self.assertLess(len(result.stdout.encode("utf-8")), 4096) + + def test_clipboard_helper_has_a_real_deadline(self) -> None: + with tempfile.TemporaryDirectory() as temporary_directory: + root = Path(temporary_directory) + helper = root / "wl-paste" + helper.write_text( + f"#!{sys.executable}\nimport time\ntime.sleep(5)\n", + encoding="utf-8", + ) + helper.chmod(0o700) + environment = {"PATH": f"{root}:{os.environ.get('PATH', '')}"} + + started = time.monotonic() + result = self.run_qopen( + "api", "clipboard-read", + config=root / "config.json", + check=False, + environment_overrides=environment, + ) + elapsed = time.monotonic() - started + response = json.loads(result.stdout) + + self.assertFalse(response["ok"]) + self.assertLess(elapsed, 2.0) + self.assertIn("clipboard is unavailable", response["error"]) + + def test_state_lifecycle_has_no_ordinary_pathname_bypass(self) -> None: + source = QOPEN.read_text(encoding="utf-8") + self.assertNotIn("shutil.copy2", source) + self.assertNotIn("CONFIG_PATH.open", source) + self.assertNotIn("CONFIG_PATH.exists", source) + self.assertNotIn("os.chmod(CONFIG_PATH", source) + self.assertNotIn("config.json.lock", source) + self.assertIn("src_dir_fd=self.dir_fd", source) + self.assertIn("os.O_NOFOLLOW | os.O_NONBLOCK", source) + + def test_state_path_race_never_writes_through_an_external_symlink(self) -> None: + with tempfile.TemporaryDirectory() as temporary_directory: + root = Path(temporary_directory) + state = root / "state" + state.mkdir(mode=0o700) + config = state / "config.json" + self.api("catalog", config=config) + valid_catalog = config.read_bytes() + sentinel = root / "sentinel" + sentinel.write_text("external-target", encoding="utf-8") + stopped = threading.Event() + + def race_state_path() -> None: + counter = 0 + while not stopped.is_set(): + temporary = state / f".attacker-{counter % 2}" + counter += 1 + try: + temporary.write_bytes(valid_catalog) + os.replace(temporary, config) + except OSError: + pass + try: + config.unlink(missing_ok=True) + config.symlink_to(sentinel) + except OSError: + pass + + attacker = threading.Thread(target=race_state_path, daemon=True) + attacker.start() + try: + for _index in range(12): + result = self.run_qopen( + "api", "favorite", "--id", "omarchy", "--mode", "toggle", + config=config, check=False, timeout=5, + ) + self.assertLess(len(result.stdout.encode("utf-8")), 4096) + json.loads(result.stdout) + finally: + stopped.set() + attacker.join(timeout=2) + + self.assertFalse(attacker.is_alive()) + self.assertEqual(sentinel.read_text(encoding="utf-8"), "external-target") + + def test_direct_catalog_editing_is_disabled(self) -> None: + result = self.run_qopen("--edit", check=False) + self.assertNotEqual(result.returncode, 0) + self.assertIn("direct catalog-file editing is disabled", result.stderr) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_docs.py b/tests/test_docs.py index dbb063d..ce16561 100644 --- a/tests/test_docs.py +++ b/tests/test_docs.py @@ -137,6 +137,9 @@ def test_ai_collaboration_guide_keeps_critical_contracts(self) -> None: "feature/* -> dev -> uat -> main", "QOPEN_CONFIG", "QtQuick.Dialogs", + "BoundedProcess.qml", + "StdioCollector", + "descriptor-anchored", "omarchy plugin validate .", "omarchy plugin update qopen.launcher", "~/.config/qopen/config.json", From a8e691e66aabdcdf0234fa994428654b50b88ff4 Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Fri, 28 Aug 2026 22:24:20 +0800 Subject: [PATCH 02/17] fix: remove disabled raw editor action --- QOpen.qml | 32 -------------------------------- tests/test_backend.py | 2 ++ 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/QOpen.qml b/QOpen.qml index f73153c..ba64024 100644 --- a/QOpen.qml +++ b/QOpen.qml @@ -420,12 +420,6 @@ Item { Qt.callLater(function() { Quickshell.execDetached(args) }) } - function editRawConfig() { - if (!root.backendPath) return - root.dismiss() - Qt.callLater(function() { Quickshell.execDetached([root.backendPath, "--edit"]) }) - } - function toggleFavorite(itemId) { if (!itemId || mutationProcess.running) return root.statusText = "Updating favorite…" @@ -895,32 +889,6 @@ Item { } } - Rectangle { - id: rawConfigButton - width: parent.width - height: Style.space(36) - radius: root.radius - color: rawMouse.containsMouse ? root.subtle : "transparent" - border.width: 1 - border.color: root.subtle - - Text { - text: " Edit raw config" - color: root.muted - font.family: root.fontFamily - font.pixelSize: Style.font.caption - anchors.centerIn: parent - } - - MouseArea { - id: rawMouse - anchors.fill: parent - enabled: !root.editorOpen - hoverEnabled: true - cursorShape: Qt.PointingHandCursor - onClicked: root.editRawConfig() - } - } } Rectangle { diff --git a/tests/test_backend.py b/tests/test_backend.py index 4abe43f..4d90620 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -215,6 +215,8 @@ def test_qml_uses_native_mutations_and_closes_the_stock_menu(self) -> None: self.assertIn('"api", "favorite"', qopen_source) self.assertIn('"api", "delete"', qopen_source) self.assertIn('"api", "recover"', qopen_source) + self.assertNotIn('"--edit"', qopen_source) + self.assertNotIn("Edit raw config", qopen_source) self.assertNotIn('[root.backendPath, "favorite"', qopen_source) self.assertNotIn('[root.backendPath, "remove"', qopen_source) From bf7a3526e0e7ccafd8b6c25411a58cc21f9757a2 Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Fri, 28 Aug 2026 22:25:41 +0800 Subject: [PATCH 03/17] fix: preserve collection layout after hardening --- QOpen.qml | 2 +- tests/test_backend.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/QOpen.qml b/QOpen.qml index ba64024..5854d1e 100644 --- a/QOpen.qml +++ b/QOpen.qml @@ -814,7 +814,7 @@ Item { ListView { id: groupList width: parent.width - height: parent.height - rawConfigButton.height - parent.spacing * 2 - Style.space(18) + height: parent.height - parent.spacing - Style.space(18) model: groupModel clip: true spacing: Style.space(3) diff --git a/tests/test_backend.py b/tests/test_backend.py index 4d90620..03b52bc 100644 --- a/tests/test_backend.py +++ b/tests/test_backend.py @@ -217,6 +217,7 @@ def test_qml_uses_native_mutations_and_closes_the_stock_menu(self) -> None: self.assertIn('"api", "recover"', qopen_source) self.assertNotIn('"--edit"', qopen_source) self.assertNotIn("Edit raw config", qopen_source) + self.assertNotIn("rawConfigButton", qopen_source) self.assertNotIn('[root.backendPath, "favorite"', qopen_source) self.assertNotIn('[root.backendPath, "remove"', qopen_source) From 97eadd7a91909db3f2571c0c7971315e8173eb4c Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Fri, 28 Aug 2026 22:52:37 +0800 Subject: [PATCH 04/17] test: cover ancestor symlink and backup race --- tests/test_state_boundary_regressions.py | 134 +++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 tests/test_state_boundary_regressions.py diff --git a/tests/test_state_boundary_regressions.py b/tests/test_state_boundary_regressions.py new file mode 100644 index 0000000..fa783ba --- /dev/null +++ b/tests/test_state_boundary_regressions.py @@ -0,0 +1,134 @@ +from __future__ import annotations + +import json +import os +import subprocess +import tempfile +import threading +import unittest +from pathlib import Path + + +REPOSITORY = Path(__file__).resolve().parents[1] +QOPEN = REPOSITORY / "bin" / "qopen" + + +class StateBoundaryRegressionTests(unittest.TestCase): + def run_qopen( + self, + *arguments: str, + config: Path, + check: bool = True, + timeout: float = 10, + ) -> subprocess.CompletedProcess[str]: + environment = os.environ.copy() + environment.pop("DISPLAY", None) + environment.pop("WAYLAND_DISPLAY", None) + environment["QOPEN_CONFIG"] = str(config) + return subprocess.run( + [str(QOPEN), *arguments], + cwd=REPOSITORY, + env=environment, + text=True, + capture_output=True, + check=check, + timeout=timeout, + ) + + def api( + self, + *arguments: str, + config: Path, + check: bool = True, + timeout: float = 10, + ) -> dict[str, object]: + result = self.run_qopen( + "api", + *arguments, + config=config, + check=check, + timeout=timeout, + ) + return json.loads(result.stdout) + + def test_intermediate_parent_symlink_is_rejected(self) -> None: + """Every ancestor component must be opened with O_NOFOLLOW.""" + with tempfile.TemporaryDirectory() as temporary_directory: + root = Path(temporary_directory) + real_parent = root / "real-parent" + real_state = real_parent / "qopen" + real_state.mkdir(parents=True, mode=0o700) + + linked_parent = root / "linked-parent" + linked_parent.symlink_to(real_parent, target_is_directory=True) + config = linked_parent / "qopen" / "config.json" + + response = self.api("catalog", config=config, check=False) + + self.assertFalse(response["ok"]) + self.assertIn("secure state directory", str(response["error"])) + self.assertFalse((real_state / "config.json").exists()) + + def test_backup_path_race_never_writes_through_external_symlink(self) -> None: + """Concurrent replacement of config.json.bak must never modify its target.""" + with tempfile.TemporaryDirectory() as temporary_directory: + root = Path(temporary_directory) + state = root / "state" + state.mkdir(mode=0o700) + config = state / "config.json" + + initial = self.api("catalog", config=config) + self.assertTrue(initial["ok"]) + + sentinel = root / "sentinel" + sentinel.write_text("external-target", encoding="utf-8") + backup = state / "config.json.bak" + stopped = threading.Event() + + def race_backup_path() -> None: + counter = 0 + while not stopped.is_set(): + temporary = state / f".attacker-backup-{counter % 2}" + counter += 1 + try: + temporary.write_text("{}\n", encoding="utf-8") + os.replace(temporary, backup) + except OSError: + pass + try: + backup.unlink(missing_ok=True) + backup.symlink_to(sentinel) + except OSError: + pass + + attacker = threading.Thread(target=race_backup_path, daemon=True) + attacker.start() + try: + for _index in range(16): + result = self.run_qopen( + "api", + "favorite", + "--id", + "omarchy", + "--mode", + "toggle", + config=config, + check=False, + timeout=5, + ) + self.assertLess(len(result.stdout.encode("utf-8")), 4096) + response = json.loads(result.stdout) + self.assertIn("ok", response) + finally: + stopped.set() + attacker.join(timeout=2) + + self.assertFalse(attacker.is_alive()) + self.assertEqual( + sentinel.read_text(encoding="utf-8"), + "external-target", + ) + + +if __name__ == "__main__": + unittest.main() From 2d5348620043840eebd016f92614cc5a8f7d3767 Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Sat, 29 Aug 2026 00:21:11 +0800 Subject: [PATCH 05/17] docs: add Linux file operation security notes --- docs/linux-file-operation-security-notes.md | 2141 +++++++++++++++++++ 1 file changed, 2141 insertions(+) create mode 100644 docs/linux-file-operation-security-notes.md diff --git a/docs/linux-file-operation-security-notes.md b/docs/linux-file-operation-security-notes.md new file mode 100644 index 0000000..789b6fe --- /dev/null +++ b/docs/linux-file-operation-security-notes.md @@ -0,0 +1,2141 @@ +# Linux 文件操作安全：从 Path、inode、FD 到 TOCTOU、Symlink、FIFO 与安全原子写入 + +> 以 QOpen 的一次安全审查为案例，系统整理 Linux 文件系统安全中最容易被业务开发者忽略的基础概念与工程实践。 +> +> 目标不是记住几个 `os.open()` 参数，而是建立一套可复用的文件安全思维模型。 + +--- + +## 1. 背景：为什么普通的文件读写代码会变成安全问题 + +在一般业务开发中，我们很容易把： + +```python +Path("config.json").open() +``` + +理解成： + +> “读取 `config.json` 这个文件。” + +但在 Linux 文件系统中，更准确的说法是： + +> “让内核根据路径字符串重新解析一次文件系统名字空间，并打开当时这个路径所指向的对象。” + +这两种理解之间的差异，就是大量文件系统安全问题的来源。 + +QOpen 最初的实现属于典型的“普通应用代码”模型： + +```python +if CONFIG_PATH.exists(): + with CONFIG_PATH.open("r", encoding="utf-8") as file: + config = json.load(file) +``` + +从业务逻辑看没有问题： + +1. 检查文件是否存在； +2. 打开文件； +3. 解析 JSON； +4. 校验配置。 + +但从安全角度，这段代码隐含了几个并不成立的假设： + +- `CONFIG_PATH.exists()` 和后面的 `CONFIG_PATH.open()` 操作的是同一个文件对象； +- 路径的父目录不会被替换； +- `config.json` 一定是普通文件； +- 文件不是 symlink、FIFO、device、socket 等特殊对象； +- 文件不会在读取过程中被并发修改； +- 文件大小总是合理； +- QML 直接读取文件不会阻塞桌面 Shell； +- subprocess 输出总是有限。 + +安全审查要求我们把这些隐含假设全部显式化。 + +--- + +# 2. Linux 文件系统的核心模型 + +## 2.1 路径不是文件 + +例如： + +```text +/home/lambert/.config/qopen/config.json +``` + +只是一个 **path / pathname（路径名）**。 + +Linux 会逐层解析： + +```text +/ +└── home + └── lambert + └── .config + └── qopen + └── config.json +``` + +目录可以理解成一张： + +```text +名字 → inode +``` + +的映射表。 + +例如： + +```text +qopen/ +├── config.json → inode 91273 +├── foo.txt → inode 91274 +└── backup.json → inode 91275 +``` + +真正代表文件对象的是 inode，而不是文件名。 + +--- + +## 2.2 inode + +inode 可以粗略理解为： + +> Linux 文件对象的身份记录。 + +它包含： + +- 文件类型； +- owner UID； +- permission； +- size； +- hard link 数量； +- 修改时间； +- 数据块位置； +- 其他 metadata。 + +查看 inode： + +```bash +ls -li ~/.config/qopen +``` + +或者： + +```bash +stat ~/.config/qopen/config.json +``` + +例如： + +```text +Inode: 91273 +Links: 1 +``` + +需要注意： + +```text +路径名 ≠ inode +``` + +同一个路径名在不同时间完全可以指向不同 inode。 + +--- + +# 3. File Descriptor：比路径更重要的对象 + +Python： + +```python +f = open("config.json") +``` + +底层最终会让 Linux 执行类似： + +```c +open(...) +``` + +成功后，内核返回一个小整数，例如： + +```text +3 +4 +5 +``` + +这就是 **File Descriptor（FD，文件描述符）**。 + +可以理解为： + +> 当前进程持有的、指向一个已经打开文件对象的句柄。 + +进程可能具有： + +```text +fd 0 → stdin +fd 1 → stdout +fd 2 → stderr +fd 3 → / +fd 4 → ~/.config/qopen +fd 5 → config.json +``` + +可以查看当前 shell 的 FD： + +```bash +ls -l /proc/$$/fd +``` + +--- + +## 3.1 Path 与 FD 的本质区别 + +可以记住一句： + +> **Path 是名字，FD 是已经打开对象的引用。** + +Path： + +```text +/home/lambert/.config/qopen/config.json +``` + +意味着： + +> 请重新根据这个名字查找对象。 + +FD： + +```text +fd 5 +``` + +意味着： + +> 就是我刚刚已经打开的那个对象。 + +例如： + +```text +config.json → inode A +``` + +程序： + +```python +fd = os.open("config.json", ...) +``` + +得到： + +```text +fd 5 → inode A +``` + +随后另一个进程执行： + +```bash +rm config.json +ln -s /tmp/evil config.json +``` + +此时 pathname 已经变成： + +```text +config.json → /tmp/evil → inode B +``` + +但是： + +```text +fd 5 +``` + +仍然指向 inode A。 + +这就是 descriptor-based programming 在安全代码中的重要价值。 + +--- + +# 4. Symlink：路径为什么会“骗人” + +Symbolic Link（符号链接）是一个特殊文件： + +```bash +ln -s ~/.bashrc config.json +``` + +结构： + +```text +config.json + │ + │ symlink + ↓ +~/.bashrc +``` + +普通： + +```python +open("config.json") +``` + +会默认 follow symlink。 + +于是代码看起来打开： + +```text +config.json +``` + +实际上可能读取： + +```text +~/.bashrc +``` + +--- + +## 4.1 `O_NOFOLLOW` + +Linux 提供： + +```python +os.O_NOFOLLOW +``` + +例如： + +```python +fd = os.open( + "config.json", + os.O_RDONLY | os.O_NOFOLLOW, +) +``` + +如果最终路径组件是 symlink，就失败，而不是继续跟随目标。 + +但这只能解决“最终组件”的 symlink 问题。 + +--- + +# 5. 为什么只检查 `config.json` 还不够 + +考虑： + +```text +/home/lambert/.config/qopen/config.json +``` + +即使： + +```text +config.json +``` + +本身不是 symlink， + +父目录： + +```text +qopen +``` + +也可能是： + +```text +qopen -> /tmp/evil +``` + +于是： + +```text +/home/lambert/.config/qopen/config.json +``` + +实际上解析到： + +```text +/tmp/evil/config.json +``` + +因此安全审查中经常会要求： + +> complete parent-directory traversal + +也就是： + +> 整条父目录链都必须受到保护。 + +--- + +# 6. descriptor-relative 目录遍历 + +安全做法不是一次打开整个路径： + +```python +open("/home/lambert/.config/qopen/config.json") +``` + +而是从 `/` 开始，一层一层打开目录： + +```python +root_fd = os.open( + "/", + os.O_RDONLY | os.O_DIRECTORY | os.O_CLOEXEC, +) +``` + +然后： + +```python +home_fd = os.open( + "home", + os.O_RDONLY + | os.O_DIRECTORY + | os.O_NOFOLLOW + | os.O_CLOEXEC, + dir_fd=root_fd, +) +``` + +再： + +```python +user_fd = os.open( + "lambert", + ..., + dir_fd=home_fd, +) +``` + +一直到： + +```text +qopen_dir_fd +``` + +最后： + +```python +config_fd = os.open( + "config.json", + ..., + dir_fd=qopen_dir_fd, +) +``` + +--- + +## 6.1 `dir_fd` + +`dir_fd` 的含义可以理解为： + +> 不要从全局 pathname 重新开始解析，就从我已经打开的这个目录对象下面找。 + +普通： + +```python +os.open( + "/home/lambert/.config/qopen/config.json", + ..., +) +``` + +是： + +```text +name-based lookup +``` + +而： + +```python +os.open( + "config.json", + ..., + dir_fd=qopen_dir_fd, +) +``` + +是： + +```text +descriptor-relative lookup +``` + +--- + +# 7. TOCTOU：最典型的路径竞争问题 + +旧代码： + +```python +if path.exists(): + path.open() +``` + +看起来合理，但存在： + +```text +Time Of Check + ↓ +Time Of Use +``` + +即 TOCTOU。 + +可能出现： + +```text +QOpen 另一个进程 + +path.exists() + ↓ +regular file + + rm path + ln -s target path + +path.open() + ↓ +打开 target +``` + +因此： + +```python +if path.is_symlink(): + reject() + +path.open() +``` + +也不够安全。 + +因为： + +```text +check +``` + +和： + +```text +use +``` + +仍然是两个独立 pathname lookup。 + +--- + +# 8. 安全顺序：open → fstat + +安全敏感代码通常更倾向： + +```text +open +↓ +得到 fd +↓ +fstat(fd) +↓ +验证真实对象 +``` + +而不是： + +```text +stat(path) +↓ +open(path) +``` + +因为： + +```python +os.fstat(fd) +``` + +询问的是： + +> 我已经打开的这个对象到底是什么？ + +而不是： + +> 这个 pathname 此刻又指向什么？ + +--- + +# 9. Linux 的“文件”不仅是普通文件 + +Linux 文件系统对象可能包括： + +| 类型 | `ls -l` 首字符 | 说明 | +|---|---:|---| +| Regular file | `-` | 普通文件 | +| Directory | `d` | 目录 | +| Symlink | `l` | 符号链接 | +| FIFO | `p` | Named Pipe | +| Socket | `s` | Unix Socket | +| Character device | `c` | 字符设备 | +| Block device | `b` | 块设备 | + +文件扩展名对 Linux 内核没有约束。 + +例如： + +```bash +mkfifo config.json +``` + +完全合法。 + +因此： + +```text +文件名叫 config.json +``` + +并不能说明它真的是： + +```text +普通 JSON 文件 +``` + +--- + +# 10. FIFO：为什么普通 `open()` 可能卡住 + +FIFO 又称： + +```text +Named Pipe +``` + +创建： + +```bash +mkfifo /tmp/qopen-learning/config.json +``` + +确认： + +```bash +file /tmp/qopen-learning/config.json +``` + +输出： + +```text +fifo (named pipe) +``` + +普通： + +```python +f = open( + "/tmp/qopen-learning/config.json", + "r", +) +``` + +可能直接阻塞在 `open()`。 + +原因是 FIFO 的语义是： + +```text +Reader + ↓ +open FIFO for read + ↓ +等待 Writer +``` + +直到另一个进程： + +```bash +echo hello > /tmp/qopen-learning/config.json +``` + +reader 才继续。 + +对于普通 CLI，这可能只是一个卡住的命令。 + +但如果发生在： + +```text +Quickshell / Desktop Shell +``` + +就可能拖住整个桌面组件。 + +--- + +# 11. `O_NONBLOCK` + +因此安全打开未知文件对象时，会考虑： + +```python +os.O_NONBLOCK +``` + +例如： + +```python +fd = os.open( + path, + os.O_RDONLY | os.O_NONBLOCK, +) +``` + +对于 FIFO： + +```text +不会为了等待 writer 无限阻塞 +``` + +但： + +> `O_NONBLOCK` 只解决“不要先卡死”，并不能证明对象是安全文件。 + +因此必须继续： + +```python +info = os.fstat(fd) + +if not stat.S_ISREG(info.st_mode): + reject() +``` + +--- + +# 12. `S_ISREG` + +```python +stat.S_ISREG(info.st_mode) +``` + +用于确认： + +> 当前 FD 真正指向的是 regular file。 + +组合： + +```text +O_NOFOLLOW + ↓ +拒绝 symlink + +O_NONBLOCK + ↓ +避免特殊对象在 open 阶段阻塞 + +fstat(fd) + ↓ +检查已经打开的对象 + +S_ISREG + ↓ +只允许普通文件 +``` + +这是安全文件读取中非常重要的一套组合。 + +--- + +# 13. Hard Link：为什么 `O_NOFOLLOW` 还不够 + +Hard Link 与 Symlink 不同。 + +例如： + +```bash +echo hello > a.txt +ln a.txt b.txt +``` + +可能看到： + +```text +12345 a.txt +12345 b.txt +``` + +两个名字指向同一个 inode： + +```text +a.txt ─────┐ + ├── inode 12345 +b.txt ─────┘ +``` + +Hard Link 本身不是 symlink，所以： + +```text +O_NOFOLLOW +``` + +无法发现它。 + +--- + +## 13.1 `st_nlink` + +```python +info = os.fstat(fd) + +print(info.st_nlink) +``` + +可以看到 inode 有多少 hard link。 + +如果： + +```text +st_nlink = 2 +``` + +说明至少有两个路径名指向同一个 inode。 + +对于像 QOpen state file 这样的私有状态文件，可以采用较保守策略： + +```python +if info.st_nlink != 1: + reject() +``` + +即： + +> QOpen 的 state inode 不允许被其他路径名共享。 + +--- + +# 14. Owner 与 Permission + +安全文件不仅需要验证类型，还需要验证 owner： + +```python +if info.st_uid != os.geteuid(): + reject() +``` + +其中： + +```text +st_uid +``` + +是文件 owner UID； + +```python +os.geteuid() +``` + +是当前进程的 effective UID。 + +目录或状态文件也不能允许其他账户写入： + +```python +mode = stat.S_IMODE(info.st_mode) + +if mode & 0o022: + reject() +``` + +`0o022` 代表： + +```text +group write ++ +other write +``` + +--- + +## 14.1 `0600` / `0700` + +典型： + +```text +0600 +``` + +文件： + +```text +owner: rw +group: - +other: - +``` + +典型： + +```text +0700 +``` + +目录： + +```text +owner: rwx +group: --- +other: --- +``` + +但必须明确： + +> `0600` 防的是其他 UID，并不能防同一个 UID 下的其他进程。 + +同 UID 进程理论上仍能直接修改用户自己的文件。 + +--- + +# 15. Same-user attack 到底在防什么 + +审查中常见： + +> A same-user process can race or pre-place these paths. + +这里并不是说： + +> 我们能够完全阻止同 UID 程序修改 QOpen 配置。 + +这是做不到的。 + +真正要防的是： + +> 不要让另一个进程把 QOpen 骗去操作“本来不应该操作的其他对象”。 + +例如： + +```text +config.json.bak -> ~/.bashrc +``` + +QOpen 以为： + +```text +我在写 backup +``` + +实际： + +```text +我在写 ~/.bashrc +``` + +这种问题属于： + +```text +Confused Deputy +``` + +即： + +> 一个本来合法的程序，被攻击者利用成为错误操作其他资源的代理。 + +--- + +# 16. 读取大小为什么必须有限 + +旧代码： + +```python +json.load(file) +``` + +最终需要将输入读入内存并解析。 + +如果输入： + +```text +1 MB +100 MB +1 GB +``` + +程序就可能消耗大量资源。 + +正确思路不是： + +```text +读完 +↓ +发现太大 +↓ +reject +``` + +而是： + +```text +边读 +↓ +边计数 +↓ +超过 limit +↓ +立即停止 +``` + +--- + +## 16.1 metadata size + actual byte count + +可以先检查： + +```python +if info.st_size > MAX_BYTES: + reject() +``` + +这是低成本快速 gate。 + +但还应该实际读取时累计： + +```python +total = 0 + +while True: + chunk = os.read(fd, 65536) + + if not chunk: + break + + total += len(chunk) + + if total > MAX_BYTES: + reject() +``` + +形成： + +```text +metadata bound ++ +runtime streamed-byte bound +``` + +--- + +# 17. 为什么读取前后还要 `fstat` + +即使已经拿到 FD： + +```text +fd → inode A +``` + +另一个同 UID 进程仍可能直接修改： + +```text +inode A +``` + +因此可能出现： + +```text +开始读取 +↓ +另一个进程修改文件 +↓ +继续读取 +``` + +得到一个不一致的数据快照。 + +可以： + +```python +before = os.fstat(fd) + +read(...) + +after = os.fstat(fd) +``` + +比较： + +```text +st_dev +st_ino +st_size +st_mtime_ns +st_ctime_ns +``` + +如果变化： + +```text +reject and retry +``` + +属于典型： + +```text +fail closed +``` + +策略。 + +--- + +# 18. Fail Closed + +Fail Open： + +```text +发生异常或无法确认 +↓ +继续执行 +``` + +Fail Closed： + +```text +发生异常或无法确认 +↓ +停止操作 +``` + +安全敏感文件操作通常倾向： + +```text +symlink? +→ reject + +special file? +→ reject + +wrong owner? +→ reject + +hard link? +→ reject + +too large? +→ reject + +changed while reading? +→ reject +``` + +安全工程中： + +> “不确定”通常不应该自动等于“可以继续”。 + +--- + +# 19. 安全写文件为什么比安全读文件更复杂 + +最简单的写法： + +```python +with open("config.json", "w") as f: + json.dump(config, f) +``` + +会先： + +```text +truncate existing file +``` + +然后逐步写入。 + +如果： + +```text +写到一半进程 crash +``` + +原文件可能只剩： + +```json +{ + "version": 1, + "items": [ + { + "id": +``` + +即半截 JSON。 + +因此配置文件通常应该采用： + +```text +temporary file +↓ +完整写入 +↓ +fsync +↓ +atomic replace +``` + +--- + +# 20. 目录应该理解成“名字 → inode”的映射表 + +例如： + +```text +qopen directory + +config.json → inode 100 +``` + +创建 temp： + +```text +config.json → inode 100 +.qopen-abc.tmp → inode 200 +``` + +完成写入后： + +```text +rename / replace +``` + +变成： + +```text +config.json → inode 200 +``` + +这比“文件夹里装着文件”的模型更适合理解 rename、hard link 与原子写入。 + +--- + +# 21. Atomic Replace + +在同一 filesystem 中： + +```text +rename +``` + +通常可以提供原子的目录项切换。 + +其他进程看到： + +```text +before: +config.json → inode A +``` + +或者： + +```text +after: +config.json → inode B +``` + +而不是看到： + +```text +一半 A + 一半 B +``` + +因此常见配置保存流程： + +```text +write temp +↓ +fsync temp +↓ +rename temp → config +``` + +--- + +# 22. Atomic ≠ Durable + +两个概念必须区分。 + +## Atomic + +回答： + +> 其他进程观察到的是不是完整切换？ + +## Durable + +回答： + +> 系统掉电以后，修改是不是仍然存在？ + +Linux 写入通常先进入： + +```text +kernel page cache +``` + +因此： + +```python +os.write(...) +``` + +成功并不等于 SSD 已经持久化。 + +--- + +# 23. `fsync(file_fd)` + +```python +os.fsync(temp_fd) +``` + +用于要求： + +> 把这个文件的内容及必要 metadata 推进到持久存储。 + +所以： + +```text +write +↓ +fsync(file) +``` + +用于保护新 inode 的内容。 + +--- + +# 24. 为什么还要 `fsync(directory_fd)` + +文件内容和目录项不是同一个东西。 + +例如： + +```text +config.json → inode A +``` + +切换成： + +```text +config.json → inode B +``` + +属于目录 metadata 的变化。 + +因此完整流程： + +```text +write new inode +↓ +fsync(new file) +↓ +rename +↓ +fsync(directory) +``` + +可以理解为： + +```text +保证新文件内容落盘 ++ +保证“名字 → inode”的新映射也落盘 +``` + +--- + +# 25. Backup、Lock、Temp 都属于同一个安全边界 + +一个常见错误是只保护： + +```text +config.json +``` + +但忘记： + +```text +config.json.bak +config.json.lock +temporary file +recovery snapshot +``` + +例如： + +```python +shutil.copy2( + CONFIG_PATH, + BACKUP_PATH, +) +``` + +如果 backup pathname 被攻击者预先构造成危险对象，就可能形成新的攻击入口。 + +所以安全要求通常不是： + +> “把 config.json 修安全。” + +而是： + +> “整个 state lifecycle 都必须使用一致的安全模型。” + +即： + +```text +read +lock +backup +temporary +replace +recovery +permission repair +``` + +全部落在同一个 trust boundary 内。 + +--- + +# 26. 为什么 `chmod(path)` 也有风险 + +普通： + +```python +os.chmod(BACKUP_PATH, 0o600) +``` + +仍然是 pathname operation。 + +安全敏感环境更喜欢： + +```python +fd = safe_open(...) +os.fchmod(fd, 0o600) +``` + +区别： + +```text +chmod(path) +→ 再次解析路径名 + +fchmod(fd) +→ 直接操作已经验证过的 inode +``` + +--- + +# 27. Temp File 为什么要 `O_EXCL` + +创建安全临时文件： + +```python +fd = os.open( + temp_name, + os.O_WRONLY + | os.O_CREAT + | os.O_EXCL + | os.O_NOFOLLOW + | os.O_CLOEXEC, + 0o600, + dir_fd=trusted_dir_fd, +) +``` + +`O_EXCL` 意味着： + +```text +不存在 +→ 创建 + +已经存在 +→ 失败 +``` + +避免： + +```text +攻击者提前创建同名对象 +``` + +配合高随机度名称： + +```text +.qopen-.tmp +``` + +可以显著减少 pathname pre-placement 风险。 + +--- + +# 28. 为什么 temp 应放在同一目录 + +推荐： + +```text +qopen/ +├── config.json +└── .qopen-random.tmp +``` + +而不是： + +```text +/tmp/qopen-random.tmp +``` + +再移动到： + +```text +~/.config/qopen/config.json +``` + +原因： + +- 同目录通常保证同 filesystem； +- rename / replace 的原子语义更可靠； +- descriptor-relative 操作更容易建立一致边界； +- 不需要额外信任 `/tmp`。 + +--- + +# 29. Descriptor-relative `os.replace` + +普通： + +```python +os.replace( + temp_path, + CONFIG_PATH, +) +``` + +需要再次解析完整 pathname。 + +更安全： + +```python +os.replace( + temp_name, + config_name, + src_dir_fd=trusted_dir_fd, + dst_dir_fd=trusted_dir_fd, +) +``` + +意思是： + +> 就在“我已经打开并验证过的这个目录 inode”里做名字替换。 + +避免重新依赖： + +```text +/home/lambert/.config/qopen +``` + +整个字符串路径。 + +--- + +# 30. Lock 的作用 + +如果两个正常 QOpen 进程同时： + +```text +A: +read +modify +write + +B: +read +modify +write +``` + +可能导致 lost update： + +```text +B 覆盖 A +``` + +因此需要 lock。 + +旧模型： + +```text +config.json.lock +``` + +本身又引入了一个 pathname。 + +新版可以： + +```python +fcntl.flock( + trusted_directory_fd, + ... +) +``` + +避免创建新的 lock file pathname。 + +--- + +## 30.1 Advisory Lock + +必须注意： + +```text +flock +``` + +通常属于 advisory lock。 + +也就是说： + +> 只有合作的进程才会遵守它。 + +恶意程序可以完全不理这个锁。 + +所以： + +```text +lock +``` + +主要解决： + +```text +正常并发一致性 +``` + +而不是： + +```text +对抗恶意进程 +``` + +--- + +# 31. QML 直接读取文件为什么有问题 + +旧设计： + +```text +QML +↓ +FileView +↓ +config.json +``` + +意味着未经验证的 filesystem object 直接进入长期运行的 desktop shell。 + +如果 pathname 最终是： + +```text +FIFO +device +symlink +超大文件 +``` + +可能导致： + +```text +阻塞 +内存增长 +桌面 Shell 不稳定 +``` + +更合理的边界： + +```text +QML +↓ +bounded backend process +↓ +filesystem +↓ +类型/owner/size/encoding/schema 验证 +↓ +有界 JSON response +↓ +QML +``` + +--- + +# 32. 为什么 `StdioCollector` / `capture_output=True` 也可能危险 + +简单 subprocess： + +```python +subprocess.run( + command, + capture_output=True, +) +``` + +通常会完整收集： + +```text +stdout +stderr +``` + +如果 producer 异常输出： + +```text +100 MB +1 GB +... +``` + +父进程会持续保存这些数据。 + +即使最终： + +```text +发现 response 太大 +↓ +reject +``` + +也已经太晚： + +```text +内存已经消耗 +``` + +安全原则应该是： + +```text +producer output +↓ +chunked read +↓ +byte counter +↓ +超过 limit +↓ +terminate producer +``` + +也就是： + +> producer-side byte limit + +而不是： + +> consumer 收完之后再检查。 + +--- + +# 33. 真正的 Process Deadline + +仅仅 UI 层： + +```text +3 秒后显示 timeout +``` + +不代表后台进程真的停止。 + +真正 deadline 应该： + +```text +start +↓ +deadline +↓ +超时 +↓ +SIGTERM +↓ +短暂等待 +↓ +仍未退出 +↓ +SIGKILL +``` + +即： + +```text +TERM → KILL +``` + +这样才能真正释放： + +- CPU； +- pipe； +- memory； +- subprocess； +- helper resource。 + +--- + +# 34. Defense in Depth + +QOpen 新设计使用多层保护： + +```text +Python producer +├── filesystem type checks +├── byte limit +├── helper stdout/stderr limit +├── helper deadline +└── API response limit + +QML consumer +├── response-size secondary limit +├── one-response protocol +├── request serial +└── process deadline +``` + +这属于： + +```text +Defense in Depth +纵深防御 +``` + +即： + +> 不把整个安全性依赖在单一层。 + +--- + +# 35. QOpen 案例：旧模型与新模型 + +## 旧模型 + +```text +Path.exists() +↓ +Path.open() +↓ +json.load() +↓ +Path.stat() +↓ +shutil.copy2() +↓ +chmod(path) +↓ +tempfile +↓ +os.replace(path, path) +``` + +核心问题： + +```text +长期依赖 pathname +``` + +--- + +## 新模型 + +```text +absolute state path +↓ +逐层 parent traversal +├── O_DIRECTORY +├── O_NOFOLLOW +└── dir_fd +↓ +trusted state-directory fd +↓ +open config relative to fd +├── O_NOFOLLOW +├── O_NONBLOCK +└── O_CLOEXEC +↓ +fstat(fd) +├── S_ISREG +├── owner +├── nlink +└── mode +↓ +bounded read +↓ +before/after metadata +↓ +UTF-8 +↓ +JSON +↓ +schema +``` + +写入： + +```text +trusted dir fd +↓ +random temp +↓ +O_CREAT | O_EXCL | O_NOFOLLOW +↓ +write +↓ +fchmod(fd) +↓ +fsync(file) +↓ +fd-relative replace +↓ +fsync(directory) +``` + +这已经形成一个完整的安全边界。 + +--- + +# 36. 可复用的文件安全检查清单 + +以后看到安全敏感文件读写，可以先问： + +### Path + +- 这个路径是否来自用户输入？ +- pathname 是否可能被其他进程替换？ +- 是否存在 `check(path) → use(path)`？ +- 父目录是否可能包含 symlink？ + +### File Object + +- 是否使用 `O_NOFOLLOW`？ +- 特殊文件是否可能在 open 阶段阻塞？ +- 是否使用 `O_NONBLOCK`？ +- 是否 `fstat(fd)`？ +- 是否验证 `S_ISREG`？ +- owner 是否正确？ +- permission 是否合理？ +- hard link 是否允许？ + +### Input + +- 是否有最大 byte limit？ +- limit 是否在读取过程中执行？ +- 是否只在“全部读完后”才检查？ +- 文件读取过程中变化怎么办？ + +### Write + +- 是否直接 truncate 正式文件？ +- 是否使用同目录 temp file？ +- temp 是否 `O_EXCL`？ +- temp 是否 random？ +- 是否 `fsync(file)`？ +- replace 是否原子？ +- 是否 `fsync(directory)`？ +- destination / backup / recovery 是否全部使用相同安全边界？ + +### Concurrency + +- 是否存在两个正常实例同时修改？ +- lock 是 advisory 还是 mandatory？ +- lock 是否被误认为安全隔离？ + +### Subprocess + +- stdout 是否有 byte limit？ +- stderr 是否有 byte limit？ +- stdout/stderr 是否同时消费？ +- process 是否有真正 deadline？ +- 超时后是否 TERM → KILL？ + +### UI / Long-running Process + +- UI 是否直接处理未经验证的文件系统对象？ +- 是否应该将 untrusted I/O 下沉到 backend process？ +- consumer 是否还有 secondary limit？ + +--- + +# 37. 最值得记住的 10 条原则 + +1. **Path 不是文件，Path 只是名字。** +2. **每次 pathname operation 都可能重新解析路径。** +3. **FD 是已经打开对象的稳定引用。** +4. **避免 `check(path) → use(path)` 型 TOCTOU。** +5. **安全顺序通常是 `open → fstat → operate by fd`。** +6. **文件扩展名不能证明文件类型。** +7. **特殊文件可能在 open/read 阶段阻塞。** +8. **输入必须在消费过程中有界，而不是读完再判断。** +9. **安全写入需要考虑 atomicity 与 durability 两个维度。** +10. **真正安全的是完整生命周期的一致边界，而不是给某一行 API 打补丁。** + +--- + +# 38. 常用实验命令 + +## 查看 inode + +```bash +ls -li file +stat file +``` + +## 查看文件类型 + +```bash +file file +/usr/bin/ls -l file +``` + +## 创建 symlink + +```bash +ln -s target link +``` + +## 创建 hard link + +```bash +ln source hardlink +``` + +## 创建 FIFO + +```bash +mkfifo pipe +``` + +## 查看当前 shell FD + +```bash +ls -l /proc/$$/fd +``` + +## 查看 UID + +```bash +id -u +``` + +## 查看权限 + +```bash +stat -c '%A %a %U %G %n' file +``` + +--- + +# 39. 建议的学习方向 + +这部分知识继续往下，可以按以下顺序扩展： + +```text +Linux VFS +↓ +inode / dentry +↓ +openat / dirfd +↓ +openat2 +↓ +rename / renameat +↓ +fsync durability +↓ +POSIX file locking +↓ +pipes / sockets / devices +↓ +process signals +↓ +privilege boundary +``` + +其中最值得继续深入的是： + +- `openat()` / `dir_fd` +- Linux VFS 中 pathname resolution 的实际过程 +- inode 与 dentry 的区别 +- `rename()` 原子语义 +- `fsync()` 与 crash consistency +- `openat2()` 的 `RESOLVE_*` 安全能力 +- `/proc//fd` +- `flock()` / `fcntl()` lock 的区别 +- symlink / hard link 的攻击模型 + +--- + +# 40. 总结 + +这次 QOpen 的安全问题，本质上不是： + +```text +“Python 文件 API 用错了” +``` + +而是： + +```text +从普通应用文件模型 +升级到 +安全敏感系统文件模型 +``` + +普通应用关注： + +```text +文件能不能读 +JSON 能不能解析 +配置能不能保存 +``` + +安全模型还必须关注： + +```text +这个路径现在到底指向什么？ +父目录是否可信？ +打开的是不是同一个对象？ +它是不是普通文件？ +会不会阻塞？ +会不会无限输出？ +能不能被 race？ +写入是否原子？ +断电后是否持久？ +整个 state lifecycle 是否都遵守同一个边界？ +``` + +真正值得积累的能力不是记住： + +```text +O_NOFOLLOW +O_NONBLOCK +O_EXCL +``` + +这些参数本身，而是形成下面这套思维： + +```text +Name +↓ +Resolve +↓ +Open +↓ +Verify Object +↓ +Bound Resource +↓ +Operate by Descriptor +↓ +Fail Closed +``` + +一旦这套模型建立起来，再看到文件系统、安全审查、daemon、桌面插件、系统服务等代码，就会知道应该从哪里开始寻找风险。 From 592011f1f6e4fc0a9318552463b44c0ccb54b0b7 Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Sat, 29 Aug 2026 00:41:07 +0800 Subject: [PATCH 06/17] chore: keep security hotfix review-focused --- docs/linux-file-operation-security-notes.md | 2141 ------------------- 1 file changed, 2141 deletions(-) delete mode 100644 docs/linux-file-operation-security-notes.md diff --git a/docs/linux-file-operation-security-notes.md b/docs/linux-file-operation-security-notes.md deleted file mode 100644 index 789b6fe..0000000 --- a/docs/linux-file-operation-security-notes.md +++ /dev/null @@ -1,2141 +0,0 @@ -# Linux 文件操作安全：从 Path、inode、FD 到 TOCTOU、Symlink、FIFO 与安全原子写入 - -> 以 QOpen 的一次安全审查为案例，系统整理 Linux 文件系统安全中最容易被业务开发者忽略的基础概念与工程实践。 -> -> 目标不是记住几个 `os.open()` 参数，而是建立一套可复用的文件安全思维模型。 - ---- - -## 1. 背景：为什么普通的文件读写代码会变成安全问题 - -在一般业务开发中，我们很容易把： - -```python -Path("config.json").open() -``` - -理解成： - -> “读取 `config.json` 这个文件。” - -但在 Linux 文件系统中，更准确的说法是： - -> “让内核根据路径字符串重新解析一次文件系统名字空间，并打开当时这个路径所指向的对象。” - -这两种理解之间的差异，就是大量文件系统安全问题的来源。 - -QOpen 最初的实现属于典型的“普通应用代码”模型： - -```python -if CONFIG_PATH.exists(): - with CONFIG_PATH.open("r", encoding="utf-8") as file: - config = json.load(file) -``` - -从业务逻辑看没有问题： - -1. 检查文件是否存在； -2. 打开文件； -3. 解析 JSON； -4. 校验配置。 - -但从安全角度，这段代码隐含了几个并不成立的假设： - -- `CONFIG_PATH.exists()` 和后面的 `CONFIG_PATH.open()` 操作的是同一个文件对象； -- 路径的父目录不会被替换； -- `config.json` 一定是普通文件； -- 文件不是 symlink、FIFO、device、socket 等特殊对象； -- 文件不会在读取过程中被并发修改； -- 文件大小总是合理； -- QML 直接读取文件不会阻塞桌面 Shell； -- subprocess 输出总是有限。 - -安全审查要求我们把这些隐含假设全部显式化。 - ---- - -# 2. Linux 文件系统的核心模型 - -## 2.1 路径不是文件 - -例如： - -```text -/home/lambert/.config/qopen/config.json -``` - -只是一个 **path / pathname（路径名）**。 - -Linux 会逐层解析： - -```text -/ -└── home - └── lambert - └── .config - └── qopen - └── config.json -``` - -目录可以理解成一张： - -```text -名字 → inode -``` - -的映射表。 - -例如： - -```text -qopen/ -├── config.json → inode 91273 -├── foo.txt → inode 91274 -└── backup.json → inode 91275 -``` - -真正代表文件对象的是 inode，而不是文件名。 - ---- - -## 2.2 inode - -inode 可以粗略理解为： - -> Linux 文件对象的身份记录。 - -它包含： - -- 文件类型； -- owner UID； -- permission； -- size； -- hard link 数量； -- 修改时间； -- 数据块位置； -- 其他 metadata。 - -查看 inode： - -```bash -ls -li ~/.config/qopen -``` - -或者： - -```bash -stat ~/.config/qopen/config.json -``` - -例如： - -```text -Inode: 91273 -Links: 1 -``` - -需要注意： - -```text -路径名 ≠ inode -``` - -同一个路径名在不同时间完全可以指向不同 inode。 - ---- - -# 3. File Descriptor：比路径更重要的对象 - -Python： - -```python -f = open("config.json") -``` - -底层最终会让 Linux 执行类似： - -```c -open(...) -``` - -成功后，内核返回一个小整数，例如： - -```text -3 -4 -5 -``` - -这就是 **File Descriptor（FD，文件描述符）**。 - -可以理解为： - -> 当前进程持有的、指向一个已经打开文件对象的句柄。 - -进程可能具有： - -```text -fd 0 → stdin -fd 1 → stdout -fd 2 → stderr -fd 3 → / -fd 4 → ~/.config/qopen -fd 5 → config.json -``` - -可以查看当前 shell 的 FD： - -```bash -ls -l /proc/$$/fd -``` - ---- - -## 3.1 Path 与 FD 的本质区别 - -可以记住一句： - -> **Path 是名字，FD 是已经打开对象的引用。** - -Path： - -```text -/home/lambert/.config/qopen/config.json -``` - -意味着： - -> 请重新根据这个名字查找对象。 - -FD： - -```text -fd 5 -``` - -意味着： - -> 就是我刚刚已经打开的那个对象。 - -例如： - -```text -config.json → inode A -``` - -程序： - -```python -fd = os.open("config.json", ...) -``` - -得到： - -```text -fd 5 → inode A -``` - -随后另一个进程执行： - -```bash -rm config.json -ln -s /tmp/evil config.json -``` - -此时 pathname 已经变成： - -```text -config.json → /tmp/evil → inode B -``` - -但是： - -```text -fd 5 -``` - -仍然指向 inode A。 - -这就是 descriptor-based programming 在安全代码中的重要价值。 - ---- - -# 4. Symlink：路径为什么会“骗人” - -Symbolic Link（符号链接）是一个特殊文件： - -```bash -ln -s ~/.bashrc config.json -``` - -结构： - -```text -config.json - │ - │ symlink - ↓ -~/.bashrc -``` - -普通： - -```python -open("config.json") -``` - -会默认 follow symlink。 - -于是代码看起来打开： - -```text -config.json -``` - -实际上可能读取： - -```text -~/.bashrc -``` - ---- - -## 4.1 `O_NOFOLLOW` - -Linux 提供： - -```python -os.O_NOFOLLOW -``` - -例如： - -```python -fd = os.open( - "config.json", - os.O_RDONLY | os.O_NOFOLLOW, -) -``` - -如果最终路径组件是 symlink，就失败，而不是继续跟随目标。 - -但这只能解决“最终组件”的 symlink 问题。 - ---- - -# 5. 为什么只检查 `config.json` 还不够 - -考虑： - -```text -/home/lambert/.config/qopen/config.json -``` - -即使： - -```text -config.json -``` - -本身不是 symlink， - -父目录： - -```text -qopen -``` - -也可能是： - -```text -qopen -> /tmp/evil -``` - -于是： - -```text -/home/lambert/.config/qopen/config.json -``` - -实际上解析到： - -```text -/tmp/evil/config.json -``` - -因此安全审查中经常会要求： - -> complete parent-directory traversal - -也就是： - -> 整条父目录链都必须受到保护。 - ---- - -# 6. descriptor-relative 目录遍历 - -安全做法不是一次打开整个路径： - -```python -open("/home/lambert/.config/qopen/config.json") -``` - -而是从 `/` 开始，一层一层打开目录： - -```python -root_fd = os.open( - "/", - os.O_RDONLY | os.O_DIRECTORY | os.O_CLOEXEC, -) -``` - -然后： - -```python -home_fd = os.open( - "home", - os.O_RDONLY - | os.O_DIRECTORY - | os.O_NOFOLLOW - | os.O_CLOEXEC, - dir_fd=root_fd, -) -``` - -再： - -```python -user_fd = os.open( - "lambert", - ..., - dir_fd=home_fd, -) -``` - -一直到： - -```text -qopen_dir_fd -``` - -最后： - -```python -config_fd = os.open( - "config.json", - ..., - dir_fd=qopen_dir_fd, -) -``` - ---- - -## 6.1 `dir_fd` - -`dir_fd` 的含义可以理解为： - -> 不要从全局 pathname 重新开始解析，就从我已经打开的这个目录对象下面找。 - -普通： - -```python -os.open( - "/home/lambert/.config/qopen/config.json", - ..., -) -``` - -是： - -```text -name-based lookup -``` - -而： - -```python -os.open( - "config.json", - ..., - dir_fd=qopen_dir_fd, -) -``` - -是： - -```text -descriptor-relative lookup -``` - ---- - -# 7. TOCTOU：最典型的路径竞争问题 - -旧代码： - -```python -if path.exists(): - path.open() -``` - -看起来合理，但存在： - -```text -Time Of Check - ↓ -Time Of Use -``` - -即 TOCTOU。 - -可能出现： - -```text -QOpen 另一个进程 - -path.exists() - ↓ -regular file - - rm path - ln -s target path - -path.open() - ↓ -打开 target -``` - -因此： - -```python -if path.is_symlink(): - reject() - -path.open() -``` - -也不够安全。 - -因为： - -```text -check -``` - -和： - -```text -use -``` - -仍然是两个独立 pathname lookup。 - ---- - -# 8. 安全顺序：open → fstat - -安全敏感代码通常更倾向： - -```text -open -↓ -得到 fd -↓ -fstat(fd) -↓ -验证真实对象 -``` - -而不是： - -```text -stat(path) -↓ -open(path) -``` - -因为： - -```python -os.fstat(fd) -``` - -询问的是： - -> 我已经打开的这个对象到底是什么？ - -而不是： - -> 这个 pathname 此刻又指向什么？ - ---- - -# 9. Linux 的“文件”不仅是普通文件 - -Linux 文件系统对象可能包括： - -| 类型 | `ls -l` 首字符 | 说明 | -|---|---:|---| -| Regular file | `-` | 普通文件 | -| Directory | `d` | 目录 | -| Symlink | `l` | 符号链接 | -| FIFO | `p` | Named Pipe | -| Socket | `s` | Unix Socket | -| Character device | `c` | 字符设备 | -| Block device | `b` | 块设备 | - -文件扩展名对 Linux 内核没有约束。 - -例如： - -```bash -mkfifo config.json -``` - -完全合法。 - -因此： - -```text -文件名叫 config.json -``` - -并不能说明它真的是： - -```text -普通 JSON 文件 -``` - ---- - -# 10. FIFO：为什么普通 `open()` 可能卡住 - -FIFO 又称： - -```text -Named Pipe -``` - -创建： - -```bash -mkfifo /tmp/qopen-learning/config.json -``` - -确认： - -```bash -file /tmp/qopen-learning/config.json -``` - -输出： - -```text -fifo (named pipe) -``` - -普通： - -```python -f = open( - "/tmp/qopen-learning/config.json", - "r", -) -``` - -可能直接阻塞在 `open()`。 - -原因是 FIFO 的语义是： - -```text -Reader - ↓ -open FIFO for read - ↓ -等待 Writer -``` - -直到另一个进程： - -```bash -echo hello > /tmp/qopen-learning/config.json -``` - -reader 才继续。 - -对于普通 CLI，这可能只是一个卡住的命令。 - -但如果发生在： - -```text -Quickshell / Desktop Shell -``` - -就可能拖住整个桌面组件。 - ---- - -# 11. `O_NONBLOCK` - -因此安全打开未知文件对象时，会考虑： - -```python -os.O_NONBLOCK -``` - -例如： - -```python -fd = os.open( - path, - os.O_RDONLY | os.O_NONBLOCK, -) -``` - -对于 FIFO： - -```text -不会为了等待 writer 无限阻塞 -``` - -但： - -> `O_NONBLOCK` 只解决“不要先卡死”，并不能证明对象是安全文件。 - -因此必须继续： - -```python -info = os.fstat(fd) - -if not stat.S_ISREG(info.st_mode): - reject() -``` - ---- - -# 12. `S_ISREG` - -```python -stat.S_ISREG(info.st_mode) -``` - -用于确认： - -> 当前 FD 真正指向的是 regular file。 - -组合： - -```text -O_NOFOLLOW - ↓ -拒绝 symlink - -O_NONBLOCK - ↓ -避免特殊对象在 open 阶段阻塞 - -fstat(fd) - ↓ -检查已经打开的对象 - -S_ISREG - ↓ -只允许普通文件 -``` - -这是安全文件读取中非常重要的一套组合。 - ---- - -# 13. Hard Link：为什么 `O_NOFOLLOW` 还不够 - -Hard Link 与 Symlink 不同。 - -例如： - -```bash -echo hello > a.txt -ln a.txt b.txt -``` - -可能看到： - -```text -12345 a.txt -12345 b.txt -``` - -两个名字指向同一个 inode： - -```text -a.txt ─────┐ - ├── inode 12345 -b.txt ─────┘ -``` - -Hard Link 本身不是 symlink，所以： - -```text -O_NOFOLLOW -``` - -无法发现它。 - ---- - -## 13.1 `st_nlink` - -```python -info = os.fstat(fd) - -print(info.st_nlink) -``` - -可以看到 inode 有多少 hard link。 - -如果： - -```text -st_nlink = 2 -``` - -说明至少有两个路径名指向同一个 inode。 - -对于像 QOpen state file 这样的私有状态文件，可以采用较保守策略： - -```python -if info.st_nlink != 1: - reject() -``` - -即： - -> QOpen 的 state inode 不允许被其他路径名共享。 - ---- - -# 14. Owner 与 Permission - -安全文件不仅需要验证类型，还需要验证 owner： - -```python -if info.st_uid != os.geteuid(): - reject() -``` - -其中： - -```text -st_uid -``` - -是文件 owner UID； - -```python -os.geteuid() -``` - -是当前进程的 effective UID。 - -目录或状态文件也不能允许其他账户写入： - -```python -mode = stat.S_IMODE(info.st_mode) - -if mode & 0o022: - reject() -``` - -`0o022` 代表： - -```text -group write -+ -other write -``` - ---- - -## 14.1 `0600` / `0700` - -典型： - -```text -0600 -``` - -文件： - -```text -owner: rw -group: - -other: - -``` - -典型： - -```text -0700 -``` - -目录： - -```text -owner: rwx -group: --- -other: --- -``` - -但必须明确： - -> `0600` 防的是其他 UID，并不能防同一个 UID 下的其他进程。 - -同 UID 进程理论上仍能直接修改用户自己的文件。 - ---- - -# 15. Same-user attack 到底在防什么 - -审查中常见： - -> A same-user process can race or pre-place these paths. - -这里并不是说： - -> 我们能够完全阻止同 UID 程序修改 QOpen 配置。 - -这是做不到的。 - -真正要防的是： - -> 不要让另一个进程把 QOpen 骗去操作“本来不应该操作的其他对象”。 - -例如： - -```text -config.json.bak -> ~/.bashrc -``` - -QOpen 以为： - -```text -我在写 backup -``` - -实际： - -```text -我在写 ~/.bashrc -``` - -这种问题属于： - -```text -Confused Deputy -``` - -即： - -> 一个本来合法的程序，被攻击者利用成为错误操作其他资源的代理。 - ---- - -# 16. 读取大小为什么必须有限 - -旧代码： - -```python -json.load(file) -``` - -最终需要将输入读入内存并解析。 - -如果输入： - -```text -1 MB -100 MB -1 GB -``` - -程序就可能消耗大量资源。 - -正确思路不是： - -```text -读完 -↓ -发现太大 -↓ -reject -``` - -而是： - -```text -边读 -↓ -边计数 -↓ -超过 limit -↓ -立即停止 -``` - ---- - -## 16.1 metadata size + actual byte count - -可以先检查： - -```python -if info.st_size > MAX_BYTES: - reject() -``` - -这是低成本快速 gate。 - -但还应该实际读取时累计： - -```python -total = 0 - -while True: - chunk = os.read(fd, 65536) - - if not chunk: - break - - total += len(chunk) - - if total > MAX_BYTES: - reject() -``` - -形成： - -```text -metadata bound -+ -runtime streamed-byte bound -``` - ---- - -# 17. 为什么读取前后还要 `fstat` - -即使已经拿到 FD： - -```text -fd → inode A -``` - -另一个同 UID 进程仍可能直接修改： - -```text -inode A -``` - -因此可能出现： - -```text -开始读取 -↓ -另一个进程修改文件 -↓ -继续读取 -``` - -得到一个不一致的数据快照。 - -可以： - -```python -before = os.fstat(fd) - -read(...) - -after = os.fstat(fd) -``` - -比较： - -```text -st_dev -st_ino -st_size -st_mtime_ns -st_ctime_ns -``` - -如果变化： - -```text -reject and retry -``` - -属于典型： - -```text -fail closed -``` - -策略。 - ---- - -# 18. Fail Closed - -Fail Open： - -```text -发生异常或无法确认 -↓ -继续执行 -``` - -Fail Closed： - -```text -发生异常或无法确认 -↓ -停止操作 -``` - -安全敏感文件操作通常倾向： - -```text -symlink? -→ reject - -special file? -→ reject - -wrong owner? -→ reject - -hard link? -→ reject - -too large? -→ reject - -changed while reading? -→ reject -``` - -安全工程中： - -> “不确定”通常不应该自动等于“可以继续”。 - ---- - -# 19. 安全写文件为什么比安全读文件更复杂 - -最简单的写法： - -```python -with open("config.json", "w") as f: - json.dump(config, f) -``` - -会先： - -```text -truncate existing file -``` - -然后逐步写入。 - -如果： - -```text -写到一半进程 crash -``` - -原文件可能只剩： - -```json -{ - "version": 1, - "items": [ - { - "id": -``` - -即半截 JSON。 - -因此配置文件通常应该采用： - -```text -temporary file -↓ -完整写入 -↓ -fsync -↓ -atomic replace -``` - ---- - -# 20. 目录应该理解成“名字 → inode”的映射表 - -例如： - -```text -qopen directory - -config.json → inode 100 -``` - -创建 temp： - -```text -config.json → inode 100 -.qopen-abc.tmp → inode 200 -``` - -完成写入后： - -```text -rename / replace -``` - -变成： - -```text -config.json → inode 200 -``` - -这比“文件夹里装着文件”的模型更适合理解 rename、hard link 与原子写入。 - ---- - -# 21. Atomic Replace - -在同一 filesystem 中： - -```text -rename -``` - -通常可以提供原子的目录项切换。 - -其他进程看到： - -```text -before: -config.json → inode A -``` - -或者： - -```text -after: -config.json → inode B -``` - -而不是看到： - -```text -一半 A + 一半 B -``` - -因此常见配置保存流程： - -```text -write temp -↓ -fsync temp -↓ -rename temp → config -``` - ---- - -# 22. Atomic ≠ Durable - -两个概念必须区分。 - -## Atomic - -回答： - -> 其他进程观察到的是不是完整切换？ - -## Durable - -回答： - -> 系统掉电以后，修改是不是仍然存在？ - -Linux 写入通常先进入： - -```text -kernel page cache -``` - -因此： - -```python -os.write(...) -``` - -成功并不等于 SSD 已经持久化。 - ---- - -# 23. `fsync(file_fd)` - -```python -os.fsync(temp_fd) -``` - -用于要求： - -> 把这个文件的内容及必要 metadata 推进到持久存储。 - -所以： - -```text -write -↓ -fsync(file) -``` - -用于保护新 inode 的内容。 - ---- - -# 24. 为什么还要 `fsync(directory_fd)` - -文件内容和目录项不是同一个东西。 - -例如： - -```text -config.json → inode A -``` - -切换成： - -```text -config.json → inode B -``` - -属于目录 metadata 的变化。 - -因此完整流程： - -```text -write new inode -↓ -fsync(new file) -↓ -rename -↓ -fsync(directory) -``` - -可以理解为： - -```text -保证新文件内容落盘 -+ -保证“名字 → inode”的新映射也落盘 -``` - ---- - -# 25. Backup、Lock、Temp 都属于同一个安全边界 - -一个常见错误是只保护： - -```text -config.json -``` - -但忘记： - -```text -config.json.bak -config.json.lock -temporary file -recovery snapshot -``` - -例如： - -```python -shutil.copy2( - CONFIG_PATH, - BACKUP_PATH, -) -``` - -如果 backup pathname 被攻击者预先构造成危险对象，就可能形成新的攻击入口。 - -所以安全要求通常不是： - -> “把 config.json 修安全。” - -而是： - -> “整个 state lifecycle 都必须使用一致的安全模型。” - -即： - -```text -read -lock -backup -temporary -replace -recovery -permission repair -``` - -全部落在同一个 trust boundary 内。 - ---- - -# 26. 为什么 `chmod(path)` 也有风险 - -普通： - -```python -os.chmod(BACKUP_PATH, 0o600) -``` - -仍然是 pathname operation。 - -安全敏感环境更喜欢： - -```python -fd = safe_open(...) -os.fchmod(fd, 0o600) -``` - -区别： - -```text -chmod(path) -→ 再次解析路径名 - -fchmod(fd) -→ 直接操作已经验证过的 inode -``` - ---- - -# 27. Temp File 为什么要 `O_EXCL` - -创建安全临时文件： - -```python -fd = os.open( - temp_name, - os.O_WRONLY - | os.O_CREAT - | os.O_EXCL - | os.O_NOFOLLOW - | os.O_CLOEXEC, - 0o600, - dir_fd=trusted_dir_fd, -) -``` - -`O_EXCL` 意味着： - -```text -不存在 -→ 创建 - -已经存在 -→ 失败 -``` - -避免： - -```text -攻击者提前创建同名对象 -``` - -配合高随机度名称： - -```text -.qopen-.tmp -``` - -可以显著减少 pathname pre-placement 风险。 - ---- - -# 28. 为什么 temp 应放在同一目录 - -推荐： - -```text -qopen/ -├── config.json -└── .qopen-random.tmp -``` - -而不是： - -```text -/tmp/qopen-random.tmp -``` - -再移动到： - -```text -~/.config/qopen/config.json -``` - -原因： - -- 同目录通常保证同 filesystem； -- rename / replace 的原子语义更可靠； -- descriptor-relative 操作更容易建立一致边界； -- 不需要额外信任 `/tmp`。 - ---- - -# 29. Descriptor-relative `os.replace` - -普通： - -```python -os.replace( - temp_path, - CONFIG_PATH, -) -``` - -需要再次解析完整 pathname。 - -更安全： - -```python -os.replace( - temp_name, - config_name, - src_dir_fd=trusted_dir_fd, - dst_dir_fd=trusted_dir_fd, -) -``` - -意思是： - -> 就在“我已经打开并验证过的这个目录 inode”里做名字替换。 - -避免重新依赖： - -```text -/home/lambert/.config/qopen -``` - -整个字符串路径。 - ---- - -# 30. Lock 的作用 - -如果两个正常 QOpen 进程同时： - -```text -A: -read -modify -write - -B: -read -modify -write -``` - -可能导致 lost update： - -```text -B 覆盖 A -``` - -因此需要 lock。 - -旧模型： - -```text -config.json.lock -``` - -本身又引入了一个 pathname。 - -新版可以： - -```python -fcntl.flock( - trusted_directory_fd, - ... -) -``` - -避免创建新的 lock file pathname。 - ---- - -## 30.1 Advisory Lock - -必须注意： - -```text -flock -``` - -通常属于 advisory lock。 - -也就是说： - -> 只有合作的进程才会遵守它。 - -恶意程序可以完全不理这个锁。 - -所以： - -```text -lock -``` - -主要解决： - -```text -正常并发一致性 -``` - -而不是： - -```text -对抗恶意进程 -``` - ---- - -# 31. QML 直接读取文件为什么有问题 - -旧设计： - -```text -QML -↓ -FileView -↓ -config.json -``` - -意味着未经验证的 filesystem object 直接进入长期运行的 desktop shell。 - -如果 pathname 最终是： - -```text -FIFO -device -symlink -超大文件 -``` - -可能导致： - -```text -阻塞 -内存增长 -桌面 Shell 不稳定 -``` - -更合理的边界： - -```text -QML -↓ -bounded backend process -↓ -filesystem -↓ -类型/owner/size/encoding/schema 验证 -↓ -有界 JSON response -↓ -QML -``` - ---- - -# 32. 为什么 `StdioCollector` / `capture_output=True` 也可能危险 - -简单 subprocess： - -```python -subprocess.run( - command, - capture_output=True, -) -``` - -通常会完整收集： - -```text -stdout -stderr -``` - -如果 producer 异常输出： - -```text -100 MB -1 GB -... -``` - -父进程会持续保存这些数据。 - -即使最终： - -```text -发现 response 太大 -↓ -reject -``` - -也已经太晚： - -```text -内存已经消耗 -``` - -安全原则应该是： - -```text -producer output -↓ -chunked read -↓ -byte counter -↓ -超过 limit -↓ -terminate producer -``` - -也就是： - -> producer-side byte limit - -而不是： - -> consumer 收完之后再检查。 - ---- - -# 33. 真正的 Process Deadline - -仅仅 UI 层： - -```text -3 秒后显示 timeout -``` - -不代表后台进程真的停止。 - -真正 deadline 应该： - -```text -start -↓ -deadline -↓ -超时 -↓ -SIGTERM -↓ -短暂等待 -↓ -仍未退出 -↓ -SIGKILL -``` - -即： - -```text -TERM → KILL -``` - -这样才能真正释放： - -- CPU； -- pipe； -- memory； -- subprocess； -- helper resource。 - ---- - -# 34. Defense in Depth - -QOpen 新设计使用多层保护： - -```text -Python producer -├── filesystem type checks -├── byte limit -├── helper stdout/stderr limit -├── helper deadline -└── API response limit - -QML consumer -├── response-size secondary limit -├── one-response protocol -├── request serial -└── process deadline -``` - -这属于： - -```text -Defense in Depth -纵深防御 -``` - -即： - -> 不把整个安全性依赖在单一层。 - ---- - -# 35. QOpen 案例：旧模型与新模型 - -## 旧模型 - -```text -Path.exists() -↓ -Path.open() -↓ -json.load() -↓ -Path.stat() -↓ -shutil.copy2() -↓ -chmod(path) -↓ -tempfile -↓ -os.replace(path, path) -``` - -核心问题： - -```text -长期依赖 pathname -``` - ---- - -## 新模型 - -```text -absolute state path -↓ -逐层 parent traversal -├── O_DIRECTORY -├── O_NOFOLLOW -└── dir_fd -↓ -trusted state-directory fd -↓ -open config relative to fd -├── O_NOFOLLOW -├── O_NONBLOCK -└── O_CLOEXEC -↓ -fstat(fd) -├── S_ISREG -├── owner -├── nlink -└── mode -↓ -bounded read -↓ -before/after metadata -↓ -UTF-8 -↓ -JSON -↓ -schema -``` - -写入： - -```text -trusted dir fd -↓ -random temp -↓ -O_CREAT | O_EXCL | O_NOFOLLOW -↓ -write -↓ -fchmod(fd) -↓ -fsync(file) -↓ -fd-relative replace -↓ -fsync(directory) -``` - -这已经形成一个完整的安全边界。 - ---- - -# 36. 可复用的文件安全检查清单 - -以后看到安全敏感文件读写，可以先问： - -### Path - -- 这个路径是否来自用户输入？ -- pathname 是否可能被其他进程替换？ -- 是否存在 `check(path) → use(path)`？ -- 父目录是否可能包含 symlink？ - -### File Object - -- 是否使用 `O_NOFOLLOW`？ -- 特殊文件是否可能在 open 阶段阻塞？ -- 是否使用 `O_NONBLOCK`？ -- 是否 `fstat(fd)`？ -- 是否验证 `S_ISREG`？ -- owner 是否正确？ -- permission 是否合理？ -- hard link 是否允许？ - -### Input - -- 是否有最大 byte limit？ -- limit 是否在读取过程中执行？ -- 是否只在“全部读完后”才检查？ -- 文件读取过程中变化怎么办？ - -### Write - -- 是否直接 truncate 正式文件？ -- 是否使用同目录 temp file？ -- temp 是否 `O_EXCL`？ -- temp 是否 random？ -- 是否 `fsync(file)`？ -- replace 是否原子？ -- 是否 `fsync(directory)`？ -- destination / backup / recovery 是否全部使用相同安全边界？ - -### Concurrency - -- 是否存在两个正常实例同时修改？ -- lock 是 advisory 还是 mandatory？ -- lock 是否被误认为安全隔离？ - -### Subprocess - -- stdout 是否有 byte limit？ -- stderr 是否有 byte limit？ -- stdout/stderr 是否同时消费？ -- process 是否有真正 deadline？ -- 超时后是否 TERM → KILL？ - -### UI / Long-running Process - -- UI 是否直接处理未经验证的文件系统对象？ -- 是否应该将 untrusted I/O 下沉到 backend process？ -- consumer 是否还有 secondary limit？ - ---- - -# 37. 最值得记住的 10 条原则 - -1. **Path 不是文件，Path 只是名字。** -2. **每次 pathname operation 都可能重新解析路径。** -3. **FD 是已经打开对象的稳定引用。** -4. **避免 `check(path) → use(path)` 型 TOCTOU。** -5. **安全顺序通常是 `open → fstat → operate by fd`。** -6. **文件扩展名不能证明文件类型。** -7. **特殊文件可能在 open/read 阶段阻塞。** -8. **输入必须在消费过程中有界，而不是读完再判断。** -9. **安全写入需要考虑 atomicity 与 durability 两个维度。** -10. **真正安全的是完整生命周期的一致边界，而不是给某一行 API 打补丁。** - ---- - -# 38. 常用实验命令 - -## 查看 inode - -```bash -ls -li file -stat file -``` - -## 查看文件类型 - -```bash -file file -/usr/bin/ls -l file -``` - -## 创建 symlink - -```bash -ln -s target link -``` - -## 创建 hard link - -```bash -ln source hardlink -``` - -## 创建 FIFO - -```bash -mkfifo pipe -``` - -## 查看当前 shell FD - -```bash -ls -l /proc/$$/fd -``` - -## 查看 UID - -```bash -id -u -``` - -## 查看权限 - -```bash -stat -c '%A %a %U %G %n' file -``` - ---- - -# 39. 建议的学习方向 - -这部分知识继续往下，可以按以下顺序扩展： - -```text -Linux VFS -↓ -inode / dentry -↓ -openat / dirfd -↓ -openat2 -↓ -rename / renameat -↓ -fsync durability -↓ -POSIX file locking -↓ -pipes / sockets / devices -↓ -process signals -↓ -privilege boundary -``` - -其中最值得继续深入的是： - -- `openat()` / `dir_fd` -- Linux VFS 中 pathname resolution 的实际过程 -- inode 与 dentry 的区别 -- `rename()` 原子语义 -- `fsync()` 与 crash consistency -- `openat2()` 的 `RESOLVE_*` 安全能力 -- `/proc//fd` -- `flock()` / `fcntl()` lock 的区别 -- symlink / hard link 的攻击模型 - ---- - -# 40. 总结 - -这次 QOpen 的安全问题，本质上不是： - -```text -“Python 文件 API 用错了” -``` - -而是： - -```text -从普通应用文件模型 -升级到 -安全敏感系统文件模型 -``` - -普通应用关注： - -```text -文件能不能读 -JSON 能不能解析 -配置能不能保存 -``` - -安全模型还必须关注： - -```text -这个路径现在到底指向什么？ -父目录是否可信？ -打开的是不是同一个对象？ -它是不是普通文件？ -会不会阻塞？ -会不会无限输出？ -能不能被 race？ -写入是否原子？ -断电后是否持久？ -整个 state lifecycle 是否都遵守同一个边界？ -``` - -真正值得积累的能力不是记住： - -```text -O_NOFOLLOW -O_NONBLOCK -O_EXCL -``` - -这些参数本身，而是形成下面这套思维： - -```text -Name -↓ -Resolve -↓ -Open -↓ -Verify Object -↓ -Bound Resource -↓ -Operate by Descriptor -↓ -Fail Closed -``` - -一旦这套模型建立起来，再看到文件系统、安全审查、daemon、桌面插件、系统服务等代码，就会知道应该从哪里开始寻找风险。 From 4417e69c883736054fcaf9976fcf21845b5658b7 Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Sat, 29 Aug 2026 00:44:58 +0800 Subject: [PATCH 07/17] fix: preserve root directory fd ownership --- bin/qopen | 2 ++ tests/test_state_boundary_regressions.py | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/bin/qopen b/bin/qopen index 52cf6fb..495d282 100755 --- a/bin/qopen +++ b/bin/qopen @@ -736,6 +736,8 @@ class SecureStateStore: fail(f"state directory is writable by another account: {parent} ({mode:04o})") self.dir_fd = current_fd self.directory_path = parent + if current_fd == root_fd: + root_fd = -1 current_fd = -1 return self finally: diff --git a/tests/test_state_boundary_regressions.py b/tests/test_state_boundary_regressions.py index fa783ba..e02011a 100644 --- a/tests/test_state_boundary_regressions.py +++ b/tests/test_state_boundary_regressions.py @@ -1,12 +1,16 @@ from __future__ import annotations +import importlib.machinery +import importlib.util import json import os +import stat import subprocess import tempfile import threading import unittest from pathlib import Path +from unittest import mock REPOSITORY = Path(__file__).resolve().parents[1] @@ -51,6 +55,21 @@ def api( ) return json.loads(result.stdout) + def test_root_parent_state_directory_keeps_transferred_descriptor_open(self) -> None: + """The root directory FD must stay owned by the store after transfer.""" + loader = importlib.machinery.SourceFileLoader("qopen_state_boundary", str(QOPEN)) + spec = importlib.util.spec_from_loader(loader.name, loader) + self.assertIsNotNone(spec) + assert spec is not None + module = importlib.util.module_from_spec(spec) + loader.exec_module(module) + + with mock.patch.object(module.os, "geteuid", return_value=0): + with module.SecureStateStore(Path("/config.json")) as store: + metadata = os.fstat(store.dir_fd) + self.assertTrue(stat.S_ISDIR(metadata.st_mode)) + self.assertEqual(store.directory_path, Path("/")) + def test_intermediate_parent_symlink_is_rejected(self) -> None: """Every ancestor component must be opened with O_NOFOLLOW.""" with tempfile.TemporaryDirectory() as temporary_directory: From ed1ef9f79a8029a01d35f46d56f4f08466dbf0bb Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Sat, 29 Aug 2026 00:50:25 +0800 Subject: [PATCH 08/17] fix: terminate bounded helper process groups --- bin/qopen | 17 +++- tests/test_process_boundary_regressions.py | 96 ++++++++++++++++++++++ 2 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 tests/test_process_boundary_regressions.py diff --git a/bin/qopen b/bin/qopen index 495d282..6c104b8 100755 --- a/bin/qopen +++ b/bin/qopen @@ -15,6 +15,7 @@ import secrets import selectors import shlex import shutil +import signal import stat import subprocess import sys @@ -326,14 +327,21 @@ def spawn_detached(argv: list[str], cwd: Path | None = None) -> None: fail(f"failed to launch {argv[0]}: {exc}") +def _signal_process_group(process: subprocess.Popen[bytes], signal_number: int) -> None: + try: + os.killpg(process.pid, signal_number) + except ProcessLookupError: + pass + + def _stop_process(process: subprocess.Popen[bytes]) -> None: - if process.poll() is not None: - return - process.terminate() + _signal_process_group(process, signal.SIGTERM) try: process.wait(timeout=0.25) except subprocess.TimeoutExpired: - process.kill() + pass + _signal_process_group(process, signal.SIGKILL) + if process.poll() is None: try: process.wait(timeout=0.25) except subprocess.TimeoutExpired: @@ -351,6 +359,7 @@ def run_bounded(command: list[str], *, input_text: str | None = None, stdin=subprocess.PIPE if input_data is not None else subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + start_new_session=True, ) except OSError as exc: fail(f"failed to start {command[0]}: {exc}") diff --git a/tests/test_process_boundary_regressions.py b/tests/test_process_boundary_regressions.py new file mode 100644 index 0000000..2100ebf --- /dev/null +++ b/tests/test_process_boundary_regressions.py @@ -0,0 +1,96 @@ +from __future__ import annotations + +import json +import os +import subprocess +import sys +import tempfile +import time +import unittest +from pathlib import Path + + +REPOSITORY = Path(__file__).resolve().parents[1] +QOPEN = REPOSITORY / "bin" / "qopen" + + +class ProcessBoundaryRegressionTests(unittest.TestCase): + def run_qopen( + self, + *arguments: str, + environment_overrides: dict[str, str] | None = None, + timeout: float = 5, + ) -> subprocess.CompletedProcess[str]: + environment = os.environ.copy() + environment.pop("DISPLAY", None) + environment.pop("WAYLAND_DISPLAY", None) + if environment_overrides: + environment.update(environment_overrides) + return subprocess.run( + [str(QOPEN), *arguments], + cwd=REPOSITORY, + env=environment, + text=True, + capture_output=True, + check=False, + timeout=timeout, + ) + + @staticmethod + def process_is_running(pid: int) -> bool: + status = Path(f"/proc/{pid}/stat") + try: + fields = status.read_text(encoding="utf-8").split() + except FileNotFoundError: + return False + return len(fields) > 2 and fields[2] != "Z" + + def test_helper_deadline_terminates_the_whole_process_group(self) -> None: + """A helper child that inherits stdout must not survive its parent deadline.""" + with tempfile.TemporaryDirectory() as temporary_directory: + root = Path(temporary_directory) + child_pid_file = root / "child.pid" + helper = root / "wl-paste" + helper.write_text( + f"#!{sys.executable}\n" + "import os, subprocess, sys, time\n" + "child = subprocess.Popen([sys.executable, '-c', 'import time; time.sleep(60)'])\n" + "with open(os.environ['QOPEN_TEST_CHILD_PID'], 'w', encoding='utf-8') as output:\n" + " output.write(str(child.pid))\n" + " output.flush()\n" + " os.fsync(output.fileno())\n" + "time.sleep(60)\n", + encoding="utf-8", + ) + helper.chmod(0o700) + environment = { + "PATH": f"{root}:{os.environ.get('PATH', '')}", + "QOPEN_TEST_CHILD_PID": str(child_pid_file), + } + + started = time.monotonic() + result = self.run_qopen( + "api", + "clipboard-read", + environment_overrides=environment, + ) + elapsed = time.monotonic() - started + response = json.loads(result.stdout) + + self.assertFalse(response["ok"]) + self.assertIn("clipboard is unavailable", response["error"]) + self.assertLess(elapsed, 2.0) + self.assertTrue(child_pid_file.exists()) + child_pid = int(child_pid_file.read_text(encoding="utf-8")) + + deadline = time.monotonic() + 1.0 + while self.process_is_running(child_pid) and time.monotonic() < deadline: + time.sleep(0.02) + self.assertFalse( + self.process_is_running(child_pid), + "helper descendant survived the bounded-process deadline", + ) + + +if __name__ == "__main__": + unittest.main() From 8db6269d048ccd5cea915eb2537f15edae1418c9 Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Sat, 29 Aug 2026 00:53:18 +0800 Subject: [PATCH 09/17] docs: clarify security boundary guarantees --- README.md | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 9370f96..507a616 100644 --- a/README.md +++ b/README.md @@ -362,7 +362,10 @@ Important guarantees: - writes use a lock on the trusted state-directory descriptor and same-directory atomic replacement; - the previous catalog is retained as `config.json.bak`; -- the default state directory is private (`0700`) and state files use `0600`; +- new default state directories are created with `0700`, and new or rewritten state + files use `0600`; existing state is accepted only when it is not group- or + world-writable, while `--doctor` reports looser private modes and + `fix-permissions` repairs the default state directory and files to `0700`/`0600`; - catalog reads, API responses, helper output and directory scans are bounded; - QML never opens or writes the catalog directly and backend processes have deadlines; - personal resource data is not automatically synchronized to this GitHub repository. @@ -451,14 +454,21 @@ terminating the entire desktop shell. Version 2.2 briefly used native dialogs; 2.3 removed them after reproducible Quickshell crashes. The evidence and release validation are documented in [DEVELOPMENT.md](DEVELOPMENT.md). +User-selected resource browsing intentionally follows symlinks so ordinary +project and file workflows behave like the filesystem the user selected. That +browser path is not used for QOpen persistence: catalog, backup, lock and +replacement operations stay inside the separately validated `SecureStateStore` +trust boundary. + Commands are passed as argument arrays. Resource values are never concatenated into a shell command by QML. QML does not use `FileView` for the catalog and does not retain complete process streams with `StdioCollector`. The Python producer validates and caps every API -response before writing it, while QML applies a second response limit and a real -deadline with TERM-to-KILL escalation. Catalog, backup and recovery operations -remain anchored to one trusted directory descriptor for their complete lifecycle. +response before writing it; QML then applies a secondary protocol-size check. +Bounded helper processes run in their own process groups and have real deadlines +with TERM-to-KILL escalation. Catalog, backup and recovery operations remain +anchored to one trusted directory descriptor for their complete lifecycle. Direct raw editing through `$QOPEN --edit` is intentionally disabled because an external editor cannot participate in QOpen's lock, validation, backup and atomic @@ -553,4 +563,4 @@ python -m unittest discover -s tests -v ## License -[MIT](LICENSE) © 2026 CoderLambert +[MIT](LICENSE) © 2026 CoderLambert \ No newline at end of file From 659cce61cc6c4d5bb11d62edf57730f961981683 Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Sat, 29 Aug 2026 00:54:10 +0800 Subject: [PATCH 10/17] docs: clarify bilingual security guarantees --- README.zh-CN.md | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/README.zh-CN.md b/README.zh-CN.md index 6ac5eea..7d46eb4 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -317,7 +317,9 @@ Project 模式只列出目录，并通过底部按钮选择当前目录。File - 配置和备份拒绝符号链接、硬链接及非普通文件； - 写入锁定可信状态目录描述符，并使用同目录原子替换； - 上一个目录版本会保留为 `config.json.bak`； -- 默认状态目录使用私有权限（`0700`），状态文件使用 `0600`； +- 新建的默认状态目录使用 `0700`，新建或重写的状态文件使用 `0600`；已有状态 + 只有在组用户和其他用户不可写时才会被接受，而 `--doctor` 会报告更宽松的私有 + 权限，`fix-permissions` 可将默认状态目录和状态文件修复为 `0700`/`0600`； - 目录读取、API 响应、helper 输出和目录扫描都有明确上限； - QML 不会直接打开或写入目录，后端进程也具有真实截止时间； - 个人资源数据不会自动同步到这个 GitHub 仓库。 @@ -397,12 +399,16 @@ QML 负责展示、焦点和交互。Python 负责目录枚举、规范化、校 内嵌路径浏览器刻意避开 Qt `FileDialog`、GTK、GIO 和 GVFS。QOpen 运行在共享的 Omarchy Shell 进程中；不在该进程中接入原生文件对话框，可以避免选择器故障终止整个桌面 Shell。2.2 版本曾短暂使用原生对话框；2.3 在复现 Quickshell 崩溃后将其移除。证据和发布验证记录见 [DEVELOPMENT.md](DEVELOPMENT.md)。 +用户主动选择的资源浏览会有意跟随符号链接，使普通 project/file 工作流与用户所选 +文件系统路径保持一致。该浏览路径不会用于 QOpen 自身持久化：配置、备份、锁和替换 +始终由经过独立校验的 `SecureStateStore` 信任边界负责。 + 命令始终以参数数组传递。QML 不会把资源值拼接成 Shell 命令。 QML 不使用 `FileView` 读取目录，也不通过 `StdioCollector` 保留完整进程流。 -Python 生产端会在写出前校验并限制每个 API 响应；QML 再执行第二层响应限制， -并通过 TERM 到 KILL 的升级机制落实真实截止时间。配置、备份和恢复在完整生命周期 -内始终锚定到同一个可信目录描述符。 +Python 生产端会在写出前校验并限制每个 API 响应；QML 随后只执行第二层协议大小检查。 +有界 helper 运行在各自独立的进程组中，并通过 TERM 到 KILL 的升级机制落实真实截止 +时间。配置、备份和恢复在完整生命周期内始终锚定到同一个可信目录描述符。 `$QOPEN --edit` 原始编辑入口已主动禁用，因为外部编辑器无法参与 QOpen 的锁、 校验、备份和原子替换协议。请改用原生编辑界面或 `$QOPEN edit [id]`。 @@ -488,4 +494,4 @@ python -m unittest discover -s tests -v ## 许可证 -[MIT](LICENSE) © 2026 CoderLambert +[MIT](LICENSE) © 2026 CoderLambert \ No newline at end of file From 72a140e51cfae1da0b98c96336dc861bc0ec5c42 Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Sat, 29 Aug 2026 01:15:46 +0800 Subject: [PATCH 11/17] fix: add explicit path picker refresh --- PathPicker.qml | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/PathPicker.qml b/PathPicker.qml index fb58fe8..3b76620 100644 --- a/PathPicker.qml +++ b/PathPicker.qml @@ -78,6 +78,13 @@ FocusScope { if (!browseProcess.running) root.startPendingRequest() } + function refreshCurrentPath() { + if (!root.opened || root.loading) return + var target = root.currentPath + if (!target) target = String(pathField.text || "").trim() + root.loadPath(target || Quickshell.env("HOME")) + } + function startPendingRequest() { if (!root.opened || !root.pendingPath || browseProcess.running) return var requested = root.pendingPath @@ -177,6 +184,11 @@ FocusScope { event.accepted = true return } + if (event.key === Qt.Key_F5) { + root.refreshCurrentPath() + event.accepted = true + return + } if (pathField.activeFocus) return if (event.key === Qt.Key_Down && entryModel.count > 0) { root.selectIndex(Math.min(entryModel.count - 1, entryList.currentIndex + 1)) @@ -269,13 +281,18 @@ FocusScope { } TextField { id: pathField - width: parent.width - Style.space(42 * 4) - parent.spacing * 4 + width: parent.width - Style.space(42 * 5) - parent.spacing * 5 height: parent.height text: root.currentPath placeholderText: "Enter a directory path" foreground: root.foreground onAccepted: root.loadPath(text) Keys.onPressed: function(event) { + if (event.key === Qt.Key_F5) { + root.refreshCurrentPath() + event.accepted = true + return + } if (event.key === Qt.Key_Escape) { root.forceActiveFocus() event.accepted = true @@ -289,6 +306,14 @@ FocusScope { enabled: !root.loading onClicked: root.loadPath(pathField.text) } + Button { + width: Style.space(42); height: parent.height + iconText: "󰑐"; tooltipText: "Refresh current directory · F5" + foreground: root.foreground; bordered: true + enabled: root.currentPath !== "" && !root.loading + opacity: enabled ? 1 : 0.38 + onClicked: root.refreshCurrentPath() + } Button { width: Style.space(42); height: parent.height iconText: root.showHidden ? "󰈈" : "󰈉" From 56f58a0a01cd810b47666e1b782db9a0455c0b0c Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Sat, 29 Aug 2026 01:15:54 +0800 Subject: [PATCH 12/17] test: cover explicit path picker refresh --- tests/test_path_picker_refresh_regressions.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 tests/test_path_picker_refresh_regressions.py diff --git a/tests/test_path_picker_refresh_regressions.py b/tests/test_path_picker_refresh_regressions.py new file mode 100644 index 0000000..9c86530 --- /dev/null +++ b/tests/test_path_picker_refresh_regressions.py @@ -0,0 +1,24 @@ +from __future__ import annotations + +import unittest +from pathlib import Path + + +REPOSITORY = Path(__file__).resolve().parents[1] +PATH_PICKER = REPOSITORY / "PathPicker.qml" + + +class PathPickerRefreshRegressionTests(unittest.TestCase): + def test_refresh_is_explicit_and_does_not_reinterpret_unsubmitted_path_text(self) -> None: + source = PATH_PICKER.read_text(encoding="utf-8") + + self.assertIn("function refreshCurrentPath()", source) + self.assertIn("var target = root.currentPath", source) + self.assertIn('tooltipText: "Refresh current directory · F5"', source) + self.assertGreaterEqual(source.count("event.key === Qt.Key_F5"), 2) + self.assertIn("onClicked: root.refreshCurrentPath()", source) + self.assertIn("onClicked: root.loadPath(pathField.text)", source) + + +if __name__ == "__main__": + unittest.main() From f6466d162534573ca6d65973b9148aba7d27e705 Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Sat, 29 Aug 2026 01:41:44 +0800 Subject: [PATCH 13/17] ui: align primary field with name column --- ResourceEditor.qml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ResourceEditor.qml b/ResourceEditor.qml index 4cd5174..b556886 100644 --- a/ResourceEditor.qml +++ b/ResourceEditor.qml @@ -597,9 +597,7 @@ Item { spacing: Style.space(8) TextField { id: primaryField - width: parent.width - pasteButton.width - checkButton.width - - (browseButton.visible ? browseButton.width + parent.spacing : 0) - - parent.spacing * 2 + width: (parent.width - Style.space(12)) * 0.58 text: root.primaryValue() placeholderText: root.primaryPlaceholder() foreground: root.foreground From 45dd53eb7c87f83cbca821c6dac64db27b97e3d2 Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Sat, 29 Aug 2026 01:55:45 +0800 Subject: [PATCH 14/17] ui: render complete borders for editor fields --- ResourceEditor.qml | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/ResourceEditor.qml b/ResourceEditor.qml index b556886..cb16372 100644 --- a/ResourceEditor.qml +++ b/ResourceEditor.qml @@ -72,6 +72,22 @@ Item { signal cancelRequested() signal submitRequested(string action, string payload, string originalPayload) + component FormTextField: TextField { + foreground: root.foreground + accent: root.accent + background: Rectangle { + color: Style.controlFill( + parent.activeFocus, + parent.hovered || parent.hasCursor, + root.foreground, + root.accent + ) + radius: root.radius + border.width: 1 + border.color: parent.activeFocus ? root.accent : root.borderColor + } + } + function slugify(value) { var slug = String(value || "").toLowerCase() .replace(/[^a-z0-9]+/g, "-").replace(/^-+|-+$/g, "") @@ -540,12 +556,11 @@ Item { width: (parent.width - parent.spacing) * 0.58 spacing: Style.space(6) Text { text: "Name *"; color: root.foreground; font.family: root.fontFamily; font.pixelSize: Style.font.caption } - TextField { + FormTextField { id: nameField width: parent.width text: root.nameValue placeholderText: "Resource name" - foreground: root.foreground onTextChanged: { root.nameValue = text if (!root.idTouched) root.idValue = root.slugify(text) @@ -558,11 +573,10 @@ Item { width: parent.width - parent.spacing - (parent.width - parent.spacing) * 0.58 spacing: Style.space(6) Text { text: "Group *"; color: root.foreground; font.family: root.fontFamily; font.pixelSize: Style.font.caption } - TextField { + FormTextField { width: parent.width text: root.groupValue placeholderText: root.defaultGroups[root.selectedType] - foreground: root.foreground onTextChanged: { root.groupValue = text; root.markDirty() } } } @@ -595,12 +609,13 @@ Item { Row { width: parent.width spacing: Style.space(8) - TextField { + FormTextField { id: primaryField - width: (parent.width - Style.space(12)) * 0.58 + width: parent.width - pasteButton.width - checkButton.width + - (browseButton.visible ? browseButton.width + parent.spacing : 0) + - parent.spacing * 2 text: root.primaryValue() placeholderText: root.primaryPlaceholder() - foreground: root.foreground onTextChanged: { root.setPrimaryValue(text) root.targetStatus = "" @@ -655,11 +670,10 @@ Item { width: parent.width spacing: Style.space(6) Text { text: "Description"; color: root.foreground; font.family: root.fontFamily; font.pixelSize: Style.font.caption } - TextField { + FormTextField { width: parent.width text: root.descriptionValue placeholderText: "Why this resource is useful" - foreground: root.foreground onTextChanged: { root.descriptionValue = text; root.markDirty() } } } @@ -737,8 +751,8 @@ Item { width: (parent.width - parent.spacing) * 0.65 spacing: Style.space(6) Text { text: "Stable ID"; color: root.foreground; font.family: root.fontFamily; font.pixelSize: Style.font.caption } - TextField { - width: parent.width; text: root.idValue; placeholderText: root.slugify(root.nameValue); foreground: root.foreground + FormTextField { + width: parent.width; text: root.idValue; placeholderText: root.slugify(root.nameValue) onTextEdited: root.idTouched = true onTextChanged: { root.idValue = text; root.markDirty() } } @@ -747,7 +761,7 @@ Item { width: parent.width - parent.spacing - (parent.width - parent.spacing) * 0.65 spacing: Style.space(6) Text { text: "Icon"; color: root.foreground; font.family: root.fontFamily; font.pixelSize: Style.font.caption } - TextField { width: parent.width; text: root.iconValue; placeholderText: root.defaultIcons[root.selectedType]; foreground: root.foreground; onTextChanged: { root.iconValue = text; root.markDirty() } } + FormTextField { width: parent.width; text: root.iconValue; placeholderText: root.defaultIcons[root.selectedType]; onTextChanged: { root.iconValue = text; root.markDirty() } } } } @@ -755,14 +769,14 @@ Item { visible: root.selectedType === "web" width: parent.width; spacing: Style.space(6) Text { text: "Window focus pattern"; color: root.foreground; font.family: root.fontFamily; font.pixelSize: Style.font.caption } - TextField { width: parent.width; text: root.focusValue; placeholderText: "Optional app window match"; foreground: root.foreground; onTextChanged: { root.focusValue = text; root.markDirty() } } + FormTextField { width: parent.width; text: root.focusValue; placeholderText: "Optional app window match"; onTextChanged: { root.focusValue = text; root.markDirty() } } } Column { visible: root.selectedType === "file" width: parent.width; spacing: Style.space(6) Text { text: "Editor command"; color: root.foreground; font.family: root.fontFamily; font.pixelSize: Style.font.caption } - TextField { width: parent.width; text: root.editorValue; placeholderText: "Uses the default editor when empty"; foreground: root.foreground; onTextChanged: { root.editorValue = text; root.markDirty() } } + FormTextField { width: parent.width; text: root.editorValue; placeholderText: "Uses the default editor when empty"; onTextChanged: { root.editorValue = text; root.markDirty() } } } } From 77f8122e13234702dc71d226c94dfb7924332863 Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Sat, 29 Aug 2026 01:56:04 +0800 Subject: [PATCH 15/17] test: cover web target validation semantics --- tests/test_url_validation_regressions.py | 100 +++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 tests/test_url_validation_regressions.py diff --git a/tests/test_url_validation_regressions.py b/tests/test_url_validation_regressions.py new file mode 100644 index 0000000..dfbb3cf --- /dev/null +++ b/tests/test_url_validation_regressions.py @@ -0,0 +1,100 @@ +from __future__ import annotations + +import json +import os +import subprocess +import tempfile +import unittest +from pathlib import Path + + +REPOSITORY = Path(__file__).resolve().parents[1] +QOPEN = REPOSITORY / "bin" / "qopen" + + +class UrlValidationRegressionTests(unittest.TestCase): + def run_qopen( + self, + *arguments: str, + config_path: Path | None = None, + ) -> subprocess.CompletedProcess[str]: + environment = os.environ.copy() + environment.pop("DISPLAY", None) + environment.pop("WAYLAND_DISPLAY", None) + if config_path is not None: + environment["QOPEN_CONFIG"] = str(config_path) + return subprocess.run( + [str(QOPEN), *arguments], + cwd=REPOSITORY, + env=environment, + text=True, + capture_output=True, + check=False, + timeout=5, + ) + + def check_web_target(self, value: str) -> dict[str, object]: + result = self.run_qopen( + "api", + "check-target", + "--type", + "web", + "--value", + value, + ) + self.assertEqual(result.returncode, 0, result.stderr) + response = json.loads(result.stdout) + self.assertTrue(response["ok"]) + return response["result"] + + def test_bare_single_label_is_rejected_but_common_local_targets_remain_valid(self) -> None: + invalid = self.check_web_target("asd") + self.assertFalse(invalid["valid"]) + self.assertEqual(invalid["level"], "error") + self.assertIn("domain", str(invalid["message"]).lower()) + + valid_cases = { + "example.com": "https://example.com", + "localhost:3000": "https://localhost:3000", + "127.0.0.1:3000": "https://127.0.0.1:3000", + "[::1]:3000": "https://[::1]:3000", + "https://intranet": "https://intranet", + } + for value, expected in valid_cases.items(): + with self.subTest(value=value): + response = self.check_web_target(value) + self.assertTrue(response["valid"]) + self.assertEqual(response["level"], "ok") + self.assertEqual(response["normalized"], expected) + + def test_editor_validation_uses_the_same_url_rule(self) -> None: + with tempfile.TemporaryDirectory() as temporary_directory: + config_path = Path(temporary_directory) / "qopen" / "config.json" + payload = { + "name": "Invalid URL", + "type": "web", + "group": "web", + "target": "asd", + "mode": "app", + } + result = self.run_qopen( + "api", + "validate", + "--payload", + json.dumps(payload), + config_path=config_path, + ) + self.assertEqual(result.returncode, 1) + response = json.loads(result.stdout) + self.assertFalse(response["ok"]) + self.assertIn("domain", response["error"].lower()) + + def test_embedded_credentials_remain_rejected(self) -> None: + response = self.check_web_target("https://user:pass@example.com") + self.assertFalse(response["valid"]) + self.assertEqual(response["level"], "error") + self.assertIn("credentials", str(response["message"]).lower()) + + +if __name__ == "__main__": + unittest.main() From eb6b55fe62905ce14ae6e7e9617924ace4cfdd0c Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Sat, 29 Aug 2026 01:59:12 +0800 Subject: [PATCH 16/17] fix: reject ambiguous bare web hostnames --- bin/qopen | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/bin/qopen b/bin/qopen index 6c104b8..743aca0 100755 --- a/bin/qopen +++ b/bin/qopen @@ -218,7 +218,9 @@ def normalize_url(url: str) -> str: def validate_http_url(url: str, field: str = "URL") -> str: - normalized = normalize_url(url) + raw = require_safe_text(url, field).strip() + has_explicit_scheme = bool(re.match(r"^https?://", raw, re.IGNORECASE)) + normalized = normalize_url(raw) if any(character.isspace() for character in normalized): fail(f"{field} must not contain whitespace") try: @@ -233,8 +235,10 @@ def validate_http_url(url: str, field: str = "URL") -> str: if port is not None and not 1 <= port <= 65535: fail(f"{field} contains an invalid port") hostname = parsed.hostname.rstrip(".") + is_ip_address = False try: ipaddress.ip_address(hostname) + is_ip_address = True except ValueError: try: ascii_hostname = hostname.encode("idna").decode("ascii") @@ -247,6 +251,10 @@ def validate_http_url(url: str, field: str = "URL") -> str: or not re.fullmatch(r"[A-Za-z0-9-]+", label) for label in labels)): fail(f"{field} contains an invalid host") + if (not has_explicit_scheme and not is_ip_address + and hostname.lower() != "localhost" and "." not in hostname): + fail(f"{field} must be a domain, IP address, localhost, " + "or include http:// / https://") return normalized From b23db5e14c6b867f6c3073cc9b4136879ca857b5 Mon Sep 17 00:00:00 2001 From: CoderLambert Date: Sat, 29 Aug 2026 02:32:37 +0800 Subject: [PATCH 17/17] fix: keep editor form borders inside clip bounds --- ResourceEditor.qml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ResourceEditor.qml b/ResourceEditor.qml index cb16372..cb3a320 100644 --- a/ResourceEditor.qml +++ b/ResourceEditor.qml @@ -499,7 +499,8 @@ Item { Column { id: formColumn - width: formFlick.width + x: Style.space(2) + width: formFlick.width - Style.space(4) topPadding: Style.space(16) bottomPadding: Style.space(16) spacing: Style.space(14)