diff --git a/CHANGELOG.md b/CHANGELOG.md index 02ba21d8a..040ac351d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -238,6 +238,21 @@ it shipped into a `## [x.y.z] - date` section of their own; the tag's ### Changed +- RBAC: REST `GET /history` and `GET /usb/acl` need `read_data`; a `viewer` + token gets 403 there. MCP `resources/read`, `resources/list` and + `resources/subscribe` are gated the same way: `autocontrol://history` and + `autocontrol://files/...` need `read_data`, and a resource URI the server + does not recognise is treated as stored data. +- Slack bot: `/run` runs on a worker thread and its reply arrives when the + script ends; a second `/run` while one is going is refused instead of queued. + `SlackBot.stop()` also stops the running command and returns `bool`. +- `execute_action` takes `result_callback=`; it is called after every command + that returned, nested ones included. +- Script Builder: every secret-named field of a command's result is masked at + any depth, flagged or not (`AC_lease_secret`, `AC_lease_active`, + `AC_rest_api_start`, `AC_rest_api_status` were shown in plain text). The + executor's result log leaves a secret-named field that is `None` or empty + as it is. - RBAC: 29 MCP tools that return stored records or configuration (run history, costs, traces, heal logs, journals, the USB ACL, lease tokens, mobile clipboards) now need `read_data` instead of `read_screen`. The built-in diff --git a/Progress.md b/Progress.md index 81f1fce0a..4fb9183ca 100644 --- a/Progress.md +++ b/Progress.md @@ -2,7 +2,7 @@ ## 跨平台與 GUI 全面改版 -`WIP` — 計畫 A–H 的程式都已交付(A 在 U-20261009-01…14,B–H 在 U-20261009-15…26,留下的缺口在 U-20261009-27…38);還沒做到的是**實機驗證**、要維護者決定的幾件事,與下面各節列的少數缺口 +`WIP` — 計畫 A–H 的程式都已交付(A 在 U-20261009-01…14,B–H 在 U-20261009-15…26,留下的缺口在 U-20261009-27…40);還沒做到的是**實機驗證**、要維護者決定的幾件事,與下面各節列的少數缺口 核准設計:[跨平台自動化與 GUI 改版](docs/superpowers/specs/2026-10-02-platform-gui-modernization-design.md)。 實作計畫:[分階段交付計畫](docs/superpowers/plans/2026-10-02-modernization-index.md)。 @@ -226,14 +226,7 @@ U-20261009-06 與 -04 的 macOS 部分是在 Windows 上對假的 pyobjc/Quart ## 安全與伺服器:留下的缺口 -`TODO` — U-20261009-29、-30、-34、-35、-37 之後還開著的 - -**要維護者過目或決定的:** - -- **`DECIDE`:REST 的 `GET /history` 與 `GET /usb/acl` 仍只要 `read_screen`**(`REST_ROUTE_CAPABILITIES`;REST 文件寫 viewer 可以用「除了兩個以外的所有 GET」)。 - MCP 那邊同樣的資料已改成要 `read_data`(`DATA_TOOLS` 42 → 71),所以 viewer 的 token 經 MCP 讀不到執行歷史與 USB ACL,經 REST 仍讀得到。 -- **`DECIDE`:`AC_lease_secret`/`AC_lease_active` 回傳的租約 token 在 Script Builder 仍以明文顯示**(後者巢狀在 `leases` 底下,遮蔽與一次性顯示都只看最上層欄位)。 - `AC_rest_api_start`/`AC_rest_api_status` 回傳共用的 REST bearer token,沒有 Script Builder schema,從 raw JSON 檢視執行時明文顯示。 +`TODO` — U-20261009-29、-30、-34、-35、-37、-39 之後還開著的 **還沒做的:** @@ -246,6 +239,8 @@ U-20261009-06 與 -04 的 macOS 部分是在 Windows 上對假的 pyobjc/Quart - **MCP 路徑根目錄**管不到的:執行動作清單的工具、自由格式物件裡的路徑、沒標註的外掛工具、`ac_launch_process` 的 `argv` 與 `ac_shell` 的 `command`(刻意不管)。 `path-or-other` 有已知的誤判:開了根目錄時,像 `/help` 這種文字目標或剛好對到根目錄外既有檔案的值會被拒絕。 檔案 symlink 的跳脫在這台機器上建不出來;POSIX 的 `file:` URL、`:` 分隔與 `~` 沒在 Linux/macOS 跑。 +- **REST 與 dashboard**:`GET /jobs`(腳本路徑與間隔)與 `GET /usb/passthrough/status` 可以被看成「設定」,目前和對應的 MCP 工具一樣留在 `read_screen`;要搬就要兩邊一起搬。 + REST 與 MCP 的一致性測試只配對 `GET` 路由與唯讀工具,經由不同呼叫讀同一份資料的(例如 `/commands`)配不到。dashboard 的 403 訊息(`app.js`)沒有在任何 JS runtime 或瀏覽器裡執行過。 - **MCP 漸進揭露**:沒對任何真的 client 跑過;`tools/list` 的分頁游標與 `-32602` 是憑對規格的記憶寫的。 - **USB passthrough**:沒對真的舊版 host、真的 WebRTC 通道或真的 USB 裝置跑過。`usb_acl.json` 改成單一檔案(簽章內嵌)之後,**舊版讀不了新檔、會全部拒絕**; 沒有對真的舊版安裝做過降版測試。 @@ -297,18 +292,19 @@ U-20261009-18 的每一個裝置面呼叫都只對假的 ADB/uiautomator2/WD ## GUI:留下的缺口 -`TODO` — U-20261009-27、-28、-34、-36、-38 之後還開著的 +`TODO` — U-20261009-27、-28、-34、-36、-38、-40 之後還開著的 - **Windows UIA 後端的執行緒模型完全沒對真的 COM 驗過**:現在每個呼叫執行緒各有一個自動化物件(`accessibility/backends/windows_automation.py`; 主執行緒 STA、其他執行緒進 MTA),Accessibility 分頁與 A11y Audit 已移出 GUI 執行緒。開發環境沒裝 `comtypes`,CI 也沒有同時有 `comtypes` 與桌面的 job, 測試全是假 COM 物件。未驗證的假設:MTA 工作執行緒上的 UIA 呼叫可用;焦點事件 callback 不需要 message pump; `import comtypes` 第一次發生在已經是 MTA 的執行緒上會丟 `OSError`(憑記憶,沒處理)。`AC_wait_for_focus_change` 的原生 `Queue.get` 仍不會被 stop 叫醒。 -- **Script Builder 的一次性值**:在區塊指令(`AC_loop`、`AC_try`、`AC_if_*`)裡產生的 token 拿不到——區塊只記錄自己的摘要,巢狀指令的結果不在任何紀錄裡(結果畫面會說明並列出指令)。 - 要補需要 executor 的結果掛鉤(`utils/executor/action_executor.py` 的 `_run_one_action` 與 `gui/script_builder/step_model.py::nested_sensitive_commands`)。 - 對話框開過之後結果畫面仍顯示「有值可看」那一行;複製到系統剪貼簿的 token 關閉時不清除;對話框從未實際顯示過。 +- **Script Builder 的一次性值**:一次執行最多保留 200 個(`MAX_ONE_TIME_VALUES`,留前面的);`AC_parallel` 分支裡產生的值不經結果掛鉤回報, + 只能從區塊的結果裡取得(標成 `results[0].…`)。結果裡內嵌的動作清單以「參數」規則遮蔽,那些欄位不提供一次性顯示。 + 對話框從未在真的螢幕上顯示過,真的系統剪貼簿(`QClipboard.clear()`/`text()`)在測試裡是假物件。 + `AC_approval_request` 的 `token`(請求編號)在結果畫面保持可見,但在 log 裡仍被遮蔽。 - **stop 還叫不醒的**:已經進到後端的單一指令(一次影像搜尋、OCR、HTTP 請求、shell 指令)、單一手勢內的節奏 sleep、`AC_wait_for_focus_change`; MCP 自己的輪詢(`ac_wait_for_image`/`ac_wait_for_pixel`)不在可停止的執行裡。非腳本的分頁工作在後端無法取消,只是結果被丟掉。 - ChatOps:`SlackBot.poll_once` 在輪詢執行緒上依序分派,所以同一個 bot 自己的 `/run` 還在跑時貼的 `/stop` 要等它結束才會被讀到(不帶參數的 `/stop` 因此只停 `chatops-` 開頭的執行);沒對真的 Slack workspace 跑過。 + ChatOps 的 Slack bot:`/run` 現在在工作執行緒上跑,`/stop` 的反應受輪詢間隔限制(預設 5 秒);卡在不可中斷的後端呼叫裡的執行仍停不掉,`stop()` 逾時後回 `False`;在輪詢執行緒上的指令 handler 裡呼叫 `stop()` 也會等到逾時。沒對真的 Slack workspace 跑過。 新轉成可停止的等待裡,`AC_handle_file_dialog`、`AC_wait_until_app_idle`、`AC_wait_for_composition_commit`、`AC_wait_for_unlock` 與行動裝置的 `wait_for_app` 只對假探針測過。 - **WebRTC**:`StatsPoller.start()` 會在 GUI 執行緒上最多等 2 秒(`future.result(timeout=2.0)`,是啟動路徑,沒動)。LAN 廣播的註冊與取消已移出 GUI 執行緒, 但真的 zeroconf 的時間與名稱衝突行為只對假物件測過。 diff --git a/architecture_explore.md b/architecture_explore.md index 8a889ea22..111f26abd 100644 --- a/architecture_explore.md +++ b/architecture_explore.md @@ -6,7 +6,7 @@ > 擷取每個模組的 docstring 與頂層公開名稱;統計數字取自實際檔案,非估算。 > 指令數與公開 API 數以 `executor.known_commands()` 與 `je_auto_control.__all__` 在工作樹上實測取得。 > -> **掃描時間**:2026-10-09 **版本**:`pyproject.toml` version `0.0.232` **分支**:`feat/maintainer-decisions` +> **掃描時間**:2026-10-09 **版本**:`pyproject.toml` version `0.0.233` **分支**:`feat/decisions-followup` --- @@ -19,8 +19,8 @@ iOS(WebDriverAgent)。核心能力是滑鼠/鍵盤控制、影像辨識、 | 指標 | 數值 | | --- | ---: | -| Python 模組總數(含周邊子專案) | 1,176 | -| 程式碼總行數 | 188,115 | +| Python 模組總數(含周邊子專案) | 1,177 | +| 程式碼總行數 | 188,730 | | `je_auto_control/utils/` 子套件數 | 311 | | `AC_*` 動作指令數(`known_commands()` 實測) | 852 | | 套件門面 `__all__` 公開名稱數 | 1,244 | @@ -272,7 +272,7 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。 ### 5.4.1 執行引擎與腳本資產 -> 25 個套件、約 18,210 行。 +> 25 個套件、約 18,499 行。 | 模組 | 行數 | 職責 | | --- | ---: | --- | @@ -283,7 +283,7 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。 | `utils/dag/` | 564 | 跨主機 DAG 編排器(圖模型 + runner) | | `utils/decision_table/` | 112 | DMN 風格決策表:規則 + 命中策略,把分支外部化 | | `utils/deterministic/` | 116 | 決定性執行控制:固定亂數種子 + 凍結時鐘 | -| `utils/executor/` | 10,369 | **核心**。`Executor` 指令分派表(852 個 `AC_*`)、參數插值、乾跑、逐步 callback;`flow_control` 提供 34 個區塊指令(迴圈/分支/try/巨集/變數) | +| `utils/executor/` | 10,658 | **核心**。`Executor` 指令分派表(852 個 `AC_*`)、參數插值、乾跑、逐步 callback;`flow_control` 提供 34 個區塊指令(迴圈/分支/try/巨集/變數) | | `utils/action_journal/` | 1,001 | 選用的動作日誌:執行邊界記錄 typed 輸入/結果與 parent id,先遮蔽再寫入 JSONL;`codegen.journal_import` 由它產生候選腳本 | | `utils/flow_debugger/` | 166 | action list 的單步除錯器與追蹤器 | | `utils/input_macro/` | 469 | 定時輸入事件:錄製結果的整形(`timeline`/`InputRecorder`,Windows 與 macOS 共用)、重播與宣告式輸入序列 DSL | @@ -495,7 +495,7 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。 ### 5.4.9 AI / Agent / LLM -> 13 個套件、約 25,860 行。 +> 13 個套件、約 25,906 行。 | 模組 | 行數 | 職責 | | --- | ---: | --- | @@ -508,7 +508,7 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。 | `utils/cua_action/` | 204 | 標準化 computer-use 動作結構(Anthropic/OpenAI → `AC_*`) | | `utils/llm/` | 387 | 自然語言 → action list 規劃器 + Anthropic/null 後端 | | `utils/mcp_registry/` | 97 | MCP registry `server.json` 資訊清單產生(可被發現) | -| `utils/mcp_server/` | 21,172 | **無頭 MCP 伺服器**(16K LOC,預設註冊 757 個工具=738 個 `ac_*` + 19 個別名):stdio + HTTP 傳輸、工具工廠與處理器、資源、prompt、稽核、限流、外掛熱重載 | +| `utils/mcp_server/` | 21,218 | **無頭 MCP 伺服器**(16K LOC,預設註冊 757 個工具=738 個 `ac_*` + 19 個別名):stdio + HTTP 傳輸、工具工廠與處理器、資源、prompt、稽核、限流、外掛熱重載 | | `utils/tool_use_schema/` | 195 | 把 `AC_*` 指令匯出成 Claude/OpenAI 的 tool-use schema | | `utils/trajectory_eval/` | 132 | agent 軌跡評估:依評分規準為一次執行打分 | | `utils/vision/` | 580 | VLM 元素定位器(依描述找元素)+ Anthropic/OpenAI/null 後端 | @@ -536,15 +536,16 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。 > `utils/accessibility/backends/windows_automation.py`(每個執行緒的 COM apartment 與 UIAutomation 物件)、 > `gui/_weak_call.py`(弱參照持有的工作回呼)、`gui/_qt_typed.py`(對真實 Qt 型別檢查用的小工具)。 > 之後新增:`gui/script_builder/one_time_dialog.py`(Script Builder 的一次性值對話框,值只留在記憶體)。 +> `utils/executor/result_hook.py`(每次執行、每個執行緒的指令結果掛鉤,巢狀指令也回報)。 ### 5.4.11 伺服器、網路協定與外部整合 -> 24 個套件、約 7,241 行。 +> 24 個套件、約 7,411 行。 | 模組 | 行數 | 職責 | | --- | ---: | --- | | `utils/acme_v2/` | 617 | 完整 ACME v2 用戶端(RFC 8555),不依賴 certbot | -| `utils/chatops/` | 758 | Chat-ops bot:接收 Slack/Discord/webhook 的 slash 指令並路由到動作 | +| `utils/chatops/` | 928 | Chat-ops bot:接收 Slack/Discord/webhook 的 slash 指令並路由到動作 | | `utils/cookie_jar/` | 191 | RFC 6265 cookie jar | | `utils/email_send/` | 118 | SMTP 寄信(email 觸發器的發送端搭檔) | | `utils/events/` | 106 | 對外 CloudEvents 發送(執行生命週期事件) | @@ -642,7 +643,7 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。 ### 5.4.14 安全、機密與合規 -> 13 個套件、約 3,931 行。 +> 13 個套件、約 3,940 行。 | 模組 | 行數 | 職責 | | --- | ---: | --- | @@ -651,7 +652,7 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。 | `utils/governance/` | 242 | 治理:maker-checker 核准閘門與即時憑證租約 | | `utils/license_policy/` | 240 | 以 SBOM 元件評估 SPDX 授權允許/拒絕政策 | | `utils/provenance/` | 126 | SLSA 建置來源證明(in-toto v1) | -| `utils/rbac/` | 1,185 | 角色型存取控制:使用者、角色與權杖驗證(尚未接到 REST/MCP) | +| `utils/rbac/` | 1,194 | 角色型存取控制:使用者、角色與權杖驗證(尚未接到 REST/MCP) | | `utils/redaction/` | 508 | 截圖遮蔽層:規則偵測 + 政策 + 協調器(上傳 VLM 前先遮) | | `utils/sbom/` | 170 | SBOM(CycloneDX)產生 | | `utils/secret_ref/` | 242 | URI scheme 形式的值參照解析 | @@ -708,19 +709,19 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。 上表以子套件為單位;以下把行數最大的幾個子系統展開到檔案層。 -#### `utils/executor/`(10,369 行)— 執行核心 +#### `utils/executor/`(10,658 行)— 執行核心 | 檔案 | 行數 | 職責 | | --- | ---: | --- | -| `action_executor.py` | 8,731 | `Executor` 類別與 `event_dict` 分派表(852 個指令),另含數百個把 utils 能力接成指令的 adapter 函式;全域單例 `executor` 與 `add_command_to_executor()` 擴充點。 | +| `action_executor.py` | 8,760 | `Executor` 類別與 `event_dict` 分派表(852 個指令),另含數百個把 utils 能力接成指令的 adapter 函式;全域單例 `executor` 與 `add_command_to_executor()` 擴充點。 | | `flow_control.py` | 693 | 真正的流程控制:`AC_loop`/`AC_for_each`/`AC_while_*`/`AC_if_*`/`AC_try`/`AC_retry`/`AC_parallel`/`AC_define_macro`/`AC_call_macro`/變數指令(`AC_set_var`/`AC_get_var`/`AC_inc_var`)。`LoopBreak`/`LoopContinue` 以例外實作。34 個區塊指令的分派表 `BLOCK_COMMANDS` 也在這裡,含下一列匯入的資料來源指令。 | | `flow_data_commands.py` | 272 | `AC_*_to_var` 資料來源與轉換指令:shell、時鐘、亂數、PDF、TOTP、SQL、檔案、HTTP、OCR,加上 `AC_assert_var`/`AC_assert_db`/`AC_assert_duration`/`AC_transform_var`。都不執行巢狀 action list,所以沒有迴圈/分支語意。 | | `action_schema.py` | 223 | action list 的結構驗證:形狀、參數型別、未知指令拒絕。單一走訪同時支援兩種消費方式:`validate_actions()` 遇到第一個問題就拋、`unknown_command_names()` 收齊全部不認得的名字(REST `/execute` 用它回 400)。 | -| `action_redaction.py` | 86 | 記錄與紀錄鍵用的遮蔽:`AC_secret_*` 的參數(金庫通行碼、機密值)在寫進 log、當成結果紀錄的鍵之前換成 `***`,巢狀在區塊指令裡的也一樣。 | +| `action_redaction.py` | 196 | 記錄與紀錄鍵用的遮蔽:`AC_secret_*` 的參數(金庫通行碼、機密值)在寫進 log、當成結果紀錄的鍵之前換成 `***`,巢狀在區塊指令裡的也一樣。 | | `mouse_aliases.py` | 39 | 單鍵點擊別名(`AC_click_left` 等),executor 與 callback executor 共用。 | | `flags.py` | 19 | 旗標讀取 `as_bool`:依拼法(`"true"`/`"yes"`/`"on"`/`"1"`)而非 Python 真值判斷,所以 JSON、CLI、MCP 傳來的 `"false"` 是關;所有 adapter 與流程指令共用。 | -#### `utils/mcp_server/`(21,172 行,757 個工具)— 最大子系統 +#### `utils/mcp_server/`(21,218 行,757 個工具)— 最大子系統 | 檔案 | 行數 | 職責 | | --- | ---: | --- | @@ -736,14 +737,14 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。 | `tools/_handlers_executor_bridge.py` | 1,429 | 252 個純委派(中位數 3 行,最長的 16 行全是參數簽章):每個都是 `from action_executor import _x` 再 `return _x(...)`,沒有分支邏輯。超過 750 行,理由記在 `Progress.md` 的豁免表(再切只能照 MCP 工廠領域分,會把同一種委派散進十幾個沒有語意邊界的檔)。 | | `tools/_handlers_locators.py` | 483 | 同一種 adapter,定位主題:無障礙樹、智慧等待、自我修復、螢幕觀察、座標空間、視覺與 OCR、影像去重、元件倉庫、A/B 定位。 | | `tools/_handlers_operations.py` | 654 | 同一種 adapter,營運主題:agent 與其記憶/追蹤、治理與合規、成本與遙測、失敗掛鉤、看門狗、速率限制、檢查點、核可、產物與資產、測試選擇與分片、佇列與 saga。 | -| `server.py` | 644 | JSON-RPC 2.0 over stdio 的最小 MCP 伺服器:連線範圍狀態、行內/併發分派、工具與 resource/prompt 處理器;握手時代的方法表(`_run_method`),兩個協定時代的逐請求分派在 `_stateless.py`。 | +| `server.py` | 647 | JSON-RPC 2.0 over stdio 的最小 MCP 伺服器:連線範圍狀態、行內/併發分派、工具與 resource/prompt 處理器;握手時代的方法表(`_run_method`),兩個協定時代的逐請求分派在 `_stateless.py`。 | | `http_transport.py` | 655 | MCP 的 HTTP 傳輸;宣告 2026-07-28 的請求走 `_http_stateless.py` 的標頭規則,不發 session。 | | `_http_stateless.py` | 185 | MCP 2026-07-28 在 Streamable HTTP 上的規則:`MCP-Protocol-Version`/`Mcp-Method`/`Mcp-Name` 必須與 body 相符(`=?base64?…?=` 先解碼),不符是 400+`HeaderMismatch`;版本與中繼資料錯誤 400、未知方法 404。純函式,由 `http_transport.py` 回覆。 | | `http_sessions.py` | 290 | MCP 的 HTTP 傳輸用的 session 身分:`Mcp-Session-Id` 註冊表,以及每個 session 那條常駐的 server→client SSE 串流。 | | `_client_requests.py` | 274 | 伺服器主動送出的請求:`roots/list`/`elicitation/create`/`sampling/createMessage`,對應表與回應路由,以及破壞性工具的確認交握。只屬於握手時代:無狀態請求裡送出會丟例外。 | | `_stateless.py` | 267 | MCP 2026-07-28 無狀態版本,與以 `initialize` 握手的版本並存:逐請求的 `_meta`(版本、client 能力、`logLevel`)、`server/discover`、結果的 `resultType`/`serverInfo`/快取提示、`-32020`~`-32022` 錯誤碼,以及兩個時代逐請求分派的 mixin。 | | `_input_required.py` | 160 | 多輪往返請求(MRTR):`input_required` 結果,與 HMAC 簽章、會過期、只兌換一次的 `requestState`;破壞性工具確認在無狀態請求裡的形式。 | -| `_subscriptions.py` | 281 | 變更通知:握手時代的 `resources/subscribe`/`unsubscribe` 與未經訂閱的 `resources/updated`、`tools/list_changed`(不送給無狀態的對端);2026-07-28 的 `subscriptions/listen`:確認、以訂閱 id 標記的通知、取消與伺服器結束時的完成回覆。 | +| `_subscriptions.py` | 285 | 變更通知:握手時代的 `resources/subscribe`/`unsubscribe` 與未經訂閱的 `resources/updated`、`tools/list_changed`(不送給無狀態的對端);2026-07-28 的 `subscriptions/listen`:確認、以訂閱 id 標記的通知、取消與伺服器結束時的完成回覆。 | | `_protocol.py` | 243 | JSON-RPC 線路格式:版本與識別常數、`_MCPError`、決定失敗工具行為的錯誤 tuple、envelope 產生器、工具回傳值轉 `content` 區塊。不碰伺服器狀態。 | | `resources.py` | 309 | MCP resource 提供者。 | | `prompts.py` | 220 | MCP prompt 目錄。 | @@ -1099,10 +1100,10 @@ socket 預設綁 `127.0.0.1`;資源一律用 `with`。 | 層/子系統 | 檔案數 | 行數 | | --- | ---: | ---: | -| `gui/` | 126 | 34,987 | -| `utils/mcp_server/` | 43 | 21,172 | +| `gui/` | 126 | 35,088 | +| `utils/mcp_server/` | 43 | 21,218 | | `utils/remote_desktop/` | 57 | 14,179 | -| `utils/executor/` | 9 | 10,369 | +| `utils/executor/` | 10 | 10,658 | | `utils/usb/` | 20 | 5,057 | | `je_auto_control/`(頂層 3 檔) | 3 | 2,676 | | `utils/accessibility/` | 15 | 3,290 | @@ -1119,6 +1120,6 @@ socket 預設綁 `127.0.0.1`;資源一律用 `with`。 | `osx/` | 17 | 981 | | `autocontrol-lsp/` | 8 | 744 | | `utils/hotkey/` | 7 | 861 | -| 其餘模組(約 286 個 `utils/` 子套件 + `android/`/`ios/`/周邊小工具) | 724 | 68,664 | -| **總計** | **1,164** | **187,275** | +| 其餘模組(約 286 個 `utils/` 子套件 + `android/`/`ios/`/周邊小工具) | 724 | 68,843 | +| **總計** | **1,165** | **187,890** | diff --git a/docs/API_LIFECYCLE.md b/docs/API_LIFECYCLE.md index be849bf82..874388f61 100644 --- a/docs/API_LIFECYCLE.md +++ b/docs/API_LIFECYCLE.md @@ -59,9 +59,9 @@ repository has driven a real phone, not because the code is unfinished. ## Compatibility notes for callers -- `je_auto_control.execute_action(actions)` takes the action list only. - `dry_run`, `raise_on_error` and `step_callback` are parameters of - `je_auto_control.executor.execute_action`. +- `je_auto_control.execute_action(actions, *, raise_on_error=False, + dry_run=False, step_callback=None, result_callback=None)` takes the same + keyword-only options as `je_auto_control.executor.execute_action`. - A refused or revoked Wayland input consent raises `WaylandAuthorisationError` and no longer falls back to `ydotool`. It is deliberately not a `RuntimeError`. diff --git a/docs/source/Eng/doc/mcp_server/mcp_server_doc.rst b/docs/source/Eng/doc/mcp_server/mcp_server_doc.rst index b83b7e6e1..feea88d35 100644 --- a/docs/source/Eng/doc/mcp_server/mcp_server_doc.rst +++ b/docs/source/Eng/doc/mcp_server/mcp_server_doc.rst @@ -360,10 +360,17 @@ bearer token and is never subject to RBAC. ``ac_self_heal_locate`` read the screen and stay ``read_screen`` although a call may be billed by a model provider; limit spend with the rate limiter or by not configuring a VLM backend for a server viewers reach. -- The classification covers MCP tools only. The REST routes ``GET /history`` - and ``GET /usb/acl`` still need ``read_screen``, as the REST table in the - operations chapter says, so a viewer token reads the same run history and - USB ACL there. +- The REST API follows the same rule: ``GET /history`` and ``GET /usb/acl`` + need ``read_data`` like ``ac_list_run_history`` and ``ac_usb_acl_list``, so + a viewer token reads neither the run history nor the USB ACL there either + (see the REST table in the operations chapter). +- Resources follow the same rule. ``autocontrol://history`` (the last 100 + run-history rows) and the action files under ``autocontrol://files/`` need + ``read_data``; ``autocontrol://commands`` and the live screen feed need + ``read_screen``. ``resources/list`` shows a caller only what it may read, + and ``resources/read`` and ``resources/subscribe`` refuse the rest with + ``-32003``. A resource URI the server does not recognise (one from a custom + provider) is treated as stored data and needs ``read_data``. - Tools that report the live state of this process rather than a stored record -- ``ac_list_executions``, ``ac_scheduler_list_jobs``, ``ac_trigger_list``, ``ac_hotkey_list``, ``ac_observe_list``, diff --git a/docs/source/Eng/doc/new_features/new_features_doc.rst b/docs/source/Eng/doc/new_features/new_features_doc.rst index ab8d5900c..6fb55e9ca 100644 --- a/docs/source/Eng/doc/new_features/new_features_doc.rst +++ b/docs/source/Eng/doc/new_features/new_features_doc.rst @@ -44,6 +44,25 @@ Run an action list through the executor without invoking any side effects executor.execute_action(actions, step_callback=lambda a: print(a)) +``result_callback`` is told the result of every command that returned, +including the ones inside a block (``AC_loop``, ``AC_try``, ``AC_if_*``...), +whose results are in no execution record:: + + def seen(command, arguments, result, path): + # path: one (run, position, command) per enclosing action list, + # e.g. step 2 (AC_loop) > iteration 3, step 1 + ... + + executor.execute_action(actions, result_callback=seen) + +The hook lasts for that run on the calling thread only (a thread the run +starts, such as an ``AC_parallel`` branch, is not followed), a failed command +and a ``dry_run`` report nothing, and a callback that raises is logged by its +exception type and ignored. The run returns and logs the same with or without +it. ``result`` is the recorded object: do not change it. To observe several +runs, use ``with observe_results(seen):`` from +``je_auto_control.utils.executor.result_hook``. + From the CLI:: python -m je_auto_control.cli run script.json --dry-run diff --git a/docs/source/Eng/doc/new_features/v2_features_doc.rst b/docs/source/Eng/doc/new_features/v2_features_doc.rst index dfa225e1e..54d362f90 100644 --- a/docs/source/Eng/doc/new_features/v2_features_doc.rst +++ b/docs/source/Eng/doc/new_features/v2_features_doc.rst @@ -495,10 +495,29 @@ reply's ``metadata["run_id"]`` and in ``AC_list_executions`` while it lasts): ``AC_stop_execution``, the MCP ``ac_stop_execution`` tool and the GUI's stop-all end it, **including a stop-all that names no run**, and the reply is ``run stopped.``. ``/stop`` asks the chat-started runs to stop; -``/stop `` asks the one named, wherever it was started. The Slack -adapter handles one message at a time, so a ``/stop`` posted while its own -``/run`` is still going is read after that run has ended -- stop such a run -from the GUI, an action list or MCP. ``/screenshot`` writes into the +``/stop `` asks the one named, wherever it was started. + +The Slack adapter (``SlackBot``) runs ``/run`` on a worker thread and keeps +polling while it lasts, so **a** ``/stop`` **posted to the channel ends the +bot's own run**: it is read at the next poll (``poll_interval_s``, 5 s by +default, 1 s at least), answered ``stop requested: ``, and the run +then answers in its own thread ``run stopped. (stopped from chat by )`` +-- always in that order. The reply to ``/run`` therefore arrives when the +script ends, not before the next message is read. One ``/run`` executes at a +time per bot: a second one while it is going is not queued but answered +``run: already running (); wait for it to end or /stop it first.`` +(before, the two ran one after the other). Every other command is still +dispatched and answered on the poll thread. Which commands run on the worker +is ``SlackBot(background_commands=frozenset({"run"}))``; a custom long +command listed there becomes a stoppable run named ``chatops--`` the same way. ``bot.stop(timeout=10.0)`` ends the poll loop, +stops the running command, waits up to ``timeout`` seconds for its worker and +returns whether none is left; ``bot.wait_idle(timeout)`` waits without +stopping anything and ``bot.running_run_id`` names the run in progress. When +the reply of a worker cannot be posted the failure is logged (the poll thread +is not there to raise to) and the command is not run again. + +``/screenshot`` writes into the context's ``screenshot_dir`` (default: ``je_auto_control_chatops`` in the temp directory) and keeps only the file name it is given. The Slack adapter goes through the package HTTP client, so the egress policy applies. diff --git a/docs/source/Eng/doc/new_features/v61_features_doc.rst b/docs/source/Eng/doc/new_features/v61_features_doc.rst index 1cc6c1933..eafc3c128 100644 --- a/docs/source/Eng/doc/new_features/v61_features_doc.rst +++ b/docs/source/Eng/doc/new_features/v61_features_doc.rst @@ -69,6 +69,8 @@ optional ``alg``; it returns ``{token}``. ``AC_jwt_decode`` takes ``token``, raising). Both are exposed as the MCP tools ``ac_jwt_encode`` / ``ac_jwt_decode`` and as Script Builder commands under **Security**. The builder masks the token ``AC_jwt_encode`` returns in its result pane; read it once with -**One-time values** after the run (a script still receives it). The executor's -result log masks secret-named fields (``token``, ``password``...) of every -command's result. +**One-time values** after the run (a script still receives it), also when the +step ran inside a block such as ``AC_loop``. The executor's result log masks +secret-named fields (``token``, ``password``...) of every command's result at +any depth, and the builder's result pane applies the same rule to every +command. diff --git a/docs/source/Eng/doc/operations_layer/operations_layer_doc.rst b/docs/source/Eng/doc/operations_layer/operations_layer_doc.rst index 439282f02..ebaa46ed0 100644 --- a/docs/source/Eng/doc/operations_layer/operations_layer_doc.rst +++ b/docs/source/Eng/doc/operations_layer/operations_layer_doc.rst @@ -182,12 +182,29 @@ surface, each of which needs the ``manage_users`` capability: tab, closing it empties it, and starting the next run or closing the tab drops what nobody read. They are never written to the result pane, the log, the test record, the run history, the action journal, a saved script - or the GUI settings; the Copy button puts one on the clipboard. A token - issued inside a block (``AC_loop``, ``AC_try``...) is in no record -- a - block records its own summary -- so the result pane says it is not - available; keep such a step at the top level, or run it from a script, the - CLI or the Users group. ``execute_action`` still returns the token to a - script, as before; + or the GUI settings; the Copy button puts one on the clipboard. Once a + value was copied, **Close and clear clipboard** closes the dialog and + empties the clipboard -- only while the clipboard still holds a value + copied from that dialog; anything copied since is left alone. After the + dialog was opened the result pane says the values were shown. A token + issued inside a block (``AC_loop``, ``AC_try``, ``AC_if_*``...) is offered + too, although a block records only its own summary: the run reports each + command's result to the builder through the executor's result hook, and + the value is labelled by its path (``Step 2 › Loop run 3, step 1 · User: + Rotate Token · alice · token``). At most 200 values are kept for one run + -- the first ones; the result pane says when a run issued more. A run + that was stopped or failed still offers what it had issued by then. The + hook follows the run's own thread, so a command in an ``AC_parallel`` + branch is offered from the block's result instead. The pane masks every + secret-named field (``token``, ``password``, ``api_key``...) of every + command's result at any depth -- a lease token from ``AC_lease_secret`` / + ``AC_lease_active``, the shared bearer token ``AC_rest_api_start`` / + ``AC_rest_api_status`` answer with -- by the rule the executor's result log + uses, and **One-time values** offers exactly the fields that were masked. + The one exemption is an identifier the person has to read: + ``AC_approval_request`` answers ``{token}``, the request id the maker hands + to the checker, which the pane shows (the log still masks it). + ``execute_action`` still returns every token to a script, as before; - the MCP tools ``ac_user_add`` / ``ac_user_remove`` / ``ac_user_set_role`` / ``ac_user_rotate_token`` / ``ac_user_list``; - the **Users (RBAC)** group of the REST API tab (commands in the Actions @@ -213,11 +230,16 @@ rotated token or a changed role applies to the next request. - Endpoints * - ``read_screen`` - viewer, operator, admin - - every ``GET`` except the two below, ``/metrics`` included + - the ``GET`` routes that observe live state: ``/screenshot``, + ``/mouse_position``, ``/screen_size``, ``/windows``, ``/sessions``, + ``/commands``, ``/jobs``, ``/inspector/recent``, ``/inspector/summary``, + ``/usb/devices``, ``/usb/events``, ``/usb/passthrough/status``, + ``/usb/loopback/devices``, ``/usb/remote/devices``, ``/diagnose``, + ``/metrics``, ``/openapi.json`` * - ``read_data`` - operator, admin - - no REST route; the MCP tools that return the host's data rather than - its screen (see the MCP chapter) + - ``GET /history``, ``GET /usb/acl``; the MCP tools that return the + host's data rather than its screen (see the MCP chapter) * - ``drive_input`` - operator, admin - ``POST /execute``, ``/execute_file``, ``/usb/loopback/open``, @@ -242,6 +264,25 @@ A role that lacks the capability gets ``403`` The same value is published per operation as ``x-required-capability`` in ``/openapi.json``. +**Stored records need** ``read_data`` **on REST as on MCP.** A route that +returns records or configuration the host keeps needs ``read_data``; a route +that observes live state needs ``read_screen``. ``GET /history`` (the run +history, as the MCP tool ``ac_list_run_history``) and ``GET /usb/acl`` (the +stored USB ACL, as ``ac_usb_acl_list``) needed only ``read_screen`` before, so +**a viewer token is now answered 403 on those two routes**; ``operator`` and +``admin`` are unaffected, and so is a server without a user store (the shared +token is not restricted by any role). Every other ``GET`` keeps its +capability: each one that has an MCP counterpart (``/jobs`` and +``ac_scheduler_list_jobs``, ``/sessions`` and ``ac_remote_host_status``, +``/usb/passthrough/status``, the USB device lists...) already needed what its +tool needs. A test pairs each route with the read-only MCP tools whose +handler reads the same source and fails when the two disagree. + +The built-in dashboard calls neither route. With a token whose role lacks a +panel's capability -- the audit panel needs ``read_audit`` -- that panel now +says ``not available to role '': needs the '' capability`` +instead of ``HTTP 403 on ``, and the other panels keep refreshing. + Being allowed to ``POST /execute`` is being allowed to run actions, not every command. Signing an action file (``sign_actions``), reading the audit log (``read_audit``), and the commands that administer the host itself -- diff --git a/docs/source/Zh/doc/mcp_server/mcp_server_doc.rst b/docs/source/Zh/doc/mcp_server/mcp_server_doc.rst index b1d13db4f..0ebc26f8b 100644 --- a/docs/source/Zh/doc/mcp_server/mcp_server_doc.rst +++ b/docs/source/Zh/doc/mcp_server/mcp_server_doc.rst @@ -324,9 +324,14 @@ Bearer token 也可從 ``JE_AUTOCONTROL_MCP_TOKEN`` 環境變數讀取。 - 一次呼叫要花多少錢不是一種能力。``ac_vlm_locate`` 與 ``ac_self_heal_locate`` 讀的是 畫面,雖然呼叫可能被模型供應商計費,仍然是 ``read_screen``;要限制花費,請用 rate limiter,或不要在 viewer 連得到的伺服器上設定 VLM 後端。 -- 這個分類只涵蓋 MCP 工具。REST 的 ``GET /history`` 與 ``GET /usb/acl`` 仍然只需要 - ``read_screen``\ (如維運章節的 REST 表格所列),所以 viewer 的 token 在那裡讀得到 - 同樣的執行歷史與 USB ACL。 +- REST API 依同一條規則:``GET /history`` 與 ``GET /usb/acl`` 和 + ``ac_list_run_history``、``ac_usb_acl_list`` 一樣需要 ``read_data``,所以 viewer 的 + token 在那裡也讀不到執行歷史與 USB ACL(見維運章節的 REST 表格)。 +- resource 適用同一條規則。\ ``autocontrol://history``\ (最近 100 筆執行歷史)與 + ``autocontrol://files/`` 底下的動作檔需要 ``read_data``\ ;\ ``autocontrol://commands`` + 與即時畫面需要 ``read_screen``\ 。\ ``resources/list`` 只列出呼叫者讀得到的, + ``resources/read`` 與 ``resources/subscribe`` 對其餘的回 ``-32003``\ 。伺服器不認得的 + resource URI(自訂 provider 的)視為已儲存的資料,需要 ``read_data``\ 。 - 回報這個行程目前狀態、而不是保存紀錄的工具——``ac_list_executions``、 ``ac_scheduler_list_jobs``、``ac_trigger_list``、``ac_hotkey_list``、 ``ac_observe_list``、``ac_watchdog_list``、``ac_journal_status``、USB 裝置清單—— diff --git a/docs/source/Zh/doc/new_features/new_features_doc.rst b/docs/source/Zh/doc/new_features/new_features_doc.rst index 63aeee909..c407eb515 100644 --- a/docs/source/Zh/doc/new_features/new_features_doc.rst +++ b/docs/source/Zh/doc/new_features/new_features_doc.rst @@ -42,6 +42,22 @@ Linux (``xclip`` 或 ``xsel``)。若無可用後端會拋出 ``RuntimeError``。 executor.execute_action(actions, step_callback=lambda a: print(a)) +``result_callback`` 會收到每一個成功回傳的指令的結果,包含區塊(``AC_loop``、 +``AC_try``、``AC_if_*``…)裡的指令——這些結果不在任何執行紀錄裡:: + + def seen(command, arguments, result, path): + # path:每一層 action 列表一個 (run, position, command), + # 例如 步驟 2(AC_loop)> 第 3 次、步驟 1 + ... + + executor.execute_action(actions, result_callback=seen) + +掛鉤只在該次執行、呼叫它的執行緒上有效(執行中啟動的執行緒,例如 ``AC_parallel`` +的分支,不會被跟到);失敗的指令與 ``dry_run`` 不會回報;callback 丟出例外時只記錄 +例外型別並忽略。有沒有掛鉤,執行的回傳值與 log 都相同。``result`` 就是紀錄裡的那個 +物件,請勿修改。要觀察多次執行,可用 +``je_auto_control.utils.executor.result_hook`` 的 ``with observe_results(seen):``。 + CLI:: python -m je_auto_control.cli run script.json --dry-run diff --git a/docs/source/Zh/doc/new_features/v2_features_doc.rst b/docs/source/Zh/doc/new_features/v2_features_doc.rst index d1ab4e94e..fbe911deb 100644 --- a/docs/source/Zh/doc/new_features/v2_features_doc.rst +++ b/docs/source/Zh/doc/new_features/v2_features_doc.rst @@ -439,9 +439,24 @@ Chat-ops 機器人 (id 在回覆的 ``metadata["run_id"]`` 裡,執行期間也列在 ``AC_list_executions``): ``AC_stop_execution``、MCP 的 ``ac_stop_execution`` 工具與 GUI 的「全部停止」都能結束它, **包含不指名的全部停止**,回覆是 ``run stopped.``。``/stop`` 要求由聊天啟動的執行停止; -``/stop `` 要求指名的那一個停止,不論它從哪裡啟動。Slack adapter 一次處理一則訊息, -所以在自己的 ``/run`` 還在跑時送出的 ``/stop`` 要等那次執行結束後才會被讀到—— -這種執行請從 GUI、action list 或 MCP 停止。``/screenshot [name]`` 寫進 context 的 ``screenshot_dir`` +``/stop `` 要求指名的那一個停止,不論它從哪裡啟動。 + +Slack adapter(``SlackBot``)把 ``/run`` 放到工作執行緒上執行,執行期間繼續輪詢,所以 +**貼到頻道的** ``/stop`` **停得掉 bot 自己的執行**:它在下一次輪詢被讀到 +(``poll_interval_s``,預設 5 秒、最少 1 秒),回覆 ``stop requested: ``, +接著那次執行在自己的討論串回覆 ``run stopped. (stopped from chat by <使用者>)``—— +一定是這個順序。``/run`` 的回覆因此是在腳本結束時送出,而不是在讀下一則訊息之前。 +每個 bot 同時只執行一個 ``/run``:執行中再送一個不會排隊,而是回覆 +``run: already running (); wait for it to end or /stop it first.``\ +(以前兩個會依序執行)。其他命令仍在輪詢執行緒上分派與回覆。哪些命令放到工作執行緒 +由 ``SlackBot(background_commands=frozenset({"run"}))`` 決定;列在裡面的自訂長時間命令 +同樣成為名稱為 ``chatops-<第一個參數>-`` 的可停止執行。``bot.stop(timeout=10.0)`` +結束輪詢、停止執行中的命令、最多等 ``timeout`` 秒讓工作執行緒結束,並回傳是否已沒有 +工作執行緒;``bot.wait_idle(timeout)`` 只等待、不停止任何東西;``bot.running_run_id`` +是執行中的 run id。工作執行緒的回覆送不出去時只記錄在 log(沒有輪詢執行緒可以往上拋), +命令不會被重新執行。 + +``/screenshot [name]`` 寫進 context 的 ``screenshot_dir`` (預設為暫存目錄下的 ``je_auto_control_chatops``),給的名稱只保留檔名。 Slack adapter 走套件的 HTTP client,所以出站政策同樣適用。 RBAC 透過 ``required_role`` 參數。 diff --git a/docs/source/Zh/doc/new_features/v61_features_doc.rst b/docs/source/Zh/doc/new_features/v61_features_doc.rst index 2ab323703..fa3b54634 100644 --- a/docs/source/Zh/doc/new_features/v61_features_doc.rst +++ b/docs/source/Zh/doc/new_features/v61_features_doc.rst @@ -60,5 +60,6 @@ token 可直接接上 HTTP 用戶端: ``audience`` / ``leeway``,回傳 ``{ok, claims}``(或 ``{ok: false, error}``,讓流程可在不拋出 例外的情況下分支)。兩者皆以 MCP 工具 ``ac_jwt_encode`` / ``ac_jwt_decode`` 以及 Script Builder 中 **Security** 分類下的命令提供。Script Builder 的結果面板會遮蔽 ``AC_jwt_encode`` 回傳的 -token;執行後按 **一次性的值** 可看一次(腳本照舊收到 token)。執行器的結果 log 會遮蔽每個命令 -結果中名稱屬於機密的欄位(``token``、``password``…)。 +token;執行後按 **一次性的值** 可看一次(腳本照舊收到 token),步驟位於 ``AC_loop`` 之類的區塊 +裡時也一樣。執行器的結果 log 會遮蔽每個命令結果中、任何深度、名稱屬於機密的欄位(``token``、 +``password``…),Script Builder 的結果面板對每個命令套用同一條規則。 diff --git a/docs/source/Zh/doc/operations_layer/operations_layer_doc.rst b/docs/source/Zh/doc/operations_layer/operations_layer_doc.rst index f2e28a7ba..6633e005c 100644 --- a/docs/source/Zh/doc/operations_layer/operations_layer_doc.rst +++ b/docs/source/Zh/doc/operations_layer/operations_layer_doc.rst @@ -163,10 +163,22 @@ CLI:: 每個值有自己的「複製」按鈕。對話框不會自動開啟,所以無人值守的執行不會被打斷。 這些值只存在記憶體裡:開啟對話框時就從分頁取走,關閉對話框即清空,開始下一次 執行或關閉分頁則丟棄沒人看過的值。它們不會寫進結果面板、log、測試紀錄、執行歷史、 - 動作日誌、儲存的腳本或 GUI 設定;「複製」按鈕會把值放到剪貼簿。在區塊 - (``AC_loop``、``AC_try``…)裡產生的 token 不在任何紀錄中——區塊只記錄自己的 - 摘要——所以結果面板會註明無法取得;請把這種步驟放在最上層,或改從腳本、CLI、 - 使用者群組執行。``execute_action`` 照舊把 token 回傳給腳本; + 動作日誌、儲存的腳本或 GUI 設定;「複製」按鈕會把值放到剪貼簿。只要複製過值, + **關閉並清除剪貼簿** 就會關閉對話框並清空剪貼簿——僅在剪貼簿仍是從這個對話框 + 複製的值時才清;之後複製的其他內容不會被動到。對話框開過之後,結果面板會改成 + 註明這些值已經顯示過。在區塊(``AC_loop``、``AC_try``、``AC_if_*``…)裡產生的 + token 同樣可以看,雖然區塊只記錄自己的摘要:執行時每個指令的結果會透過執行器的 + 結果掛鉤回報給 Script Builder,值則以路徑標示(``步驟 2 › Loop 第 3 次,步驟 1 · + User: Rotate Token · alice · token``)。一次執行最多保留 200 個值——保留最先 + 產生的;超過時結果面板會註明。被停止或失敗的執行,仍可看到在那之前已經產生的值。 + 掛鉤只跟著執行本身的執行緒,所以 ``AC_parallel`` 分支裡的指令改由區塊的結果取得。 + 結果面板會遮蔽每個指令結果中、任何深度、名稱屬於機密的欄位(``token``、 + ``password``、``api_key``…)——``AC_lease_secret``/``AC_lease_active`` 的租約 + token、``AC_rest_api_start``/``AC_rest_api_status`` 回傳的共用 bearer token—— + 用的是執行器結果 log 的同一條規則,而 **一次性的值** 提供的正好就是被遮蔽的欄位。 + 唯一的例外是使用者必須看到的識別碼:``AC_approval_request`` 回傳 ``{token}``, + 那是 maker 交給 checker 的申請編號,結果面板會照樣顯示(log 仍然遮蔽)。 + ``execute_action`` 照舊把每個 token 回傳給腳本; - MCP 工具 ``ac_user_add``/``ac_user_remove``/``ac_user_set_role``/ ``ac_user_rotate_token``/``ac_user_list``; - REST API 分頁的 **使用者(RBAC)** 群組(指令在 Actions 選單),該分頁也可以用這個 @@ -190,10 +202,15 @@ CLI:: - 端點 * - ``read_screen`` - viewer、operator、admin - - 除下列兩個之外的所有 ``GET``,包含 ``/metrics`` + - 觀察目前狀態的 ``GET`` 路由:``/screenshot``、``/mouse_position``、 + ``/screen_size``、``/windows``、``/sessions``、``/commands``、``/jobs``、 + ``/inspector/recent``、``/inspector/summary``、``/usb/devices``、 + ``/usb/events``、``/usb/passthrough/status``、``/usb/loopback/devices``、 + ``/usb/remote/devices``、``/diagnose``、``/metrics``、``/openapi.json`` * - ``read_data`` - operator、admin - - 沒有 REST 路由;回傳主機資料而非畫面的 MCP 工具(見 MCP 章節) + - ``GET /history``、``GET /usb/acl``;以及回傳主機資料而非畫面的 MCP 工具 + (見 MCP 章節) * - ``drive_input`` - operator、admin - ``POST /execute``、``/execute_file``、``/usb/loopback/open``、 @@ -217,6 +234,21 @@ CLI:: ``{"error": "forbidden", "required_capability": "...", "role": "..."}``。 同一個值也以 ``x-required-capability`` 列在 ``/openapi.json`` 的每個 operation 上。 +**保存的紀錄在 REST 與 MCP 都需要** ``read_data``\ **。** 回傳主機保存的紀錄或設定的 +路由需要 ``read_data``;觀察目前狀態的路由需要 ``read_screen``。``GET /history``\ +(執行歷史,與 MCP 工具 ``ac_list_run_history`` 相同)與 ``GET /usb/acl``\ (保存的 +USB ACL,與 ``ac_usb_acl_list`` 相同)以前只需要 ``read_screen``,所以\ **viewer 的 +token 現在在這兩個路由會得到 403**\ ;``operator`` 與 ``admin`` 不受影響,沒有使用者 +存放檔的伺服器也不受影響(共用 token 不受任何角色限制)。其餘 ``GET`` 的能力不變: +有對應 MCP 工具的路由(``/jobs`` 與 ``ac_scheduler_list_jobs``、``/sessions`` 與 +``ac_remote_host_status``、``/usb/passthrough/status``、USB 裝置清單……)原本就與工具 +一致。有一個測試會把每個路由與讀取同一個來源的唯讀 MCP 工具配對,兩者不一致時失敗。 + +內建的 dashboard 不會呼叫這兩個路由。token 的角色缺少某個面板需要的能力時——稽核 +面板需要 ``read_audit``——該面板現在顯示 +``not available to role '<角色>': needs the '<能力>' capability``,而不是 +``HTTP 403 on <路徑>``;其他面板照常更新。 + 可以 ``POST /execute`` 代表可以執行動作,不代表可以執行每一個指令。替動作檔簽章 (``sign_actions``)、讀取稽核紀錄(``read_audit``),以及管理主機本身的指令—— ``AC_admin_*``、啟動或停止 REST/MCP/遠端桌面/webhook 伺服器、``AC_usb_acl_*``、 diff --git a/docs/updates/2026-10.md b/docs/updates/2026-10.md index 9a2c1f646..0d221bfbd 100644 --- a/docs/updates/2026-10.md +++ b/docs/updates/2026-10.md @@ -446,3 +446,15 @@ Jeffrey_RPA kept its own `KEY_ALIASES` (40 friendly names) and `_EXTRA_KEY_CODES - **What**: After a run the Script Builder keeps the masked record and offers **One-time values**: each value produced by a `sensitive_result` step, labelled by its step, with a copy button, held in memory until the dialog closes or the next run starts. `AC_jwt_encode` is now a `sensitive_result` command. Fixed: the executor logged every command result as it was, so a signed JWT and a lease token appeared in the log in plain text; results now pass through `redact_actions` before logging (the returned record is unchanged). - **Tests**: `test_script_builder_one_time_reveal.py` (23). - **Open items**: a token issued inside a block command cannot be revealed. `Progress.md` › GUI. + +## U-20261009-39 · 2026-10-09 · REST and MCP resources follow the read_data rule; a Slack /stop no longer waits for the run it stops · #done #security #rbac #mcp #chatops + +- **What**: REST `GET /history` and `GET /usb/acl` need `read_data`; a test pairs each REST GET with the read-only MCP tool that reads the same source and fails on a disagreement. MCP `resources/read` was checked against no capability, so a viewer read `autocontrol://history` and the action files under `autocontrol://files/`: `resources/list`, `resources/read` and `resources/subscribe` now apply the same rule (an unrecognised URI needs `read_data`) and record a refusal. The dashboard names the role and the missing capability on a 403. The Slack bot runs `/run` on a worker thread, one at a time, so a `/stop` is read while it runs; `SlackBot.stop()` stops the run, joins the worker and returns `bool`. +- **Tests**: `test_rest_read_data_routes.py` (15), `test_mcp_resource_authorization.py` (29), `test_chatops_slack_background_run.py` (15). +- **Open items**: fake Slack only; the dashboard script was not executed. `Progress.md` › 安全與伺服器. + +## U-20261009-40 · 2026-10-09 · A result hook reports nested command results; secret-named result fields are masked at any depth and offered once · #done #executor #gui #security + +- **What**: `execute_action(..., result_callback=fn)` (and `observe_results`) calls `fn(command, arguments, result, path)` after every command that returned, nested ones included; it is per run and per thread, changes neither the return value nor the log, and a callback that raises is logged by type and ignored (`utils/executor/result_hook.py`). The Script Builder collects its one-time values through it, so a token issued inside a loop or a try block is offered with its path; at most 200 per run. The result pane masks every secret-named field of every command's result at any depth, which closes the plain-text lease and REST bearer tokens; `AC_approval_request`'s request id stays visible through an explicit allow-list. The dialog can clear the clipboard when it still holds a value copied from it. +- **Tests**: `test_executor_result_hook.py` (14), `test_result_redaction.py` (13), `test_script_builder_one_time_reveal.py` (33). +- **Open items**: `AC_parallel` branches are not followed by the hook. `Progress.md` › GUI. diff --git a/docs/updates/README.md b/docs/updates/README.md index 8dbaa3508..5bd3d800a 100644 --- a/docs/updates/README.md +++ b/docs/updates/README.md @@ -58,6 +58,8 @@ In the same commit: delete the item from `Progress.md`, add a `#done` entry here | ID | Date | Title | Tags | Batch | |---|---|---|---|---| +| U-20261009-40 | 2026-10-09 | A result hook reports nested command results; secret-named result fields are masked at any depth and offered once | #done #executor #gui #security | [2026-10](2026-10.md) | +| U-20261009-39 | 2026-10-09 | REST and MCP resources follow the read_data rule; a Slack /stop no longer waits for the run it stops | #done #security #rbac #mcp #chatops | [2026-10](2026-10.md) | | U-20261009-38 | 2026-10-09 | Script Builder shows a freshly issued token once; command results are masked before they are logged | #done #gui #security | [2026-10](2026-10.md) | | U-20261009-37 | 2026-10-09 | Plug-in tools can declare themselves read-only; stored records need read_data; a chat /run can be stopped; joining a bucket commits nothing | #done #mcp #rbac #chatops #sync | [2026-10](2026-10.md) | | U-20261009-36 | 2026-10-09 | GUI: task callbacks held weakly, dispose() on every panel and tab with background work, per-thread UI Automation, 52 fewer typing exemptions | #done #gui #accessibility #typing | [2026-10](2026-10.md) | @@ -387,7 +389,7 @@ In the same commit: delete the item from `Progress.md`, add a `#done` entry here | File | Period | Entries | |---|---|---:| -| [2026-10.md](2026-10.md) | 2026-10 | 51 | +| [2026-10.md](2026-10.md) | 2026-10 | 53 | | [2026-09-e.md](2026-09-e.md) | 2026-09 | 7 | | [2026-09-d.md](2026-09-d.md) | 2026-09 | 55 | | [2026-09-c.md](2026-09-c.md) | 2026-09 | 38 | diff --git a/examples/25_chatops_bot.py b/examples/25_chatops_bot.py index 714d01631..22ed973e7 100644 --- a/examples/25_chatops_bot.py +++ b/examples/25_chatops_bot.py @@ -5,8 +5,9 @@ /help list every registered command /scripts list .json scripts under ``script_root`` - /run execute one of those scripts - /screenshot [path] capture the screen, reply with the saved path + /run execute one of those scripts (one at a time) + /stop [run-id] stop the run /run started, or the one named + /screenshot [name] capture the screen, reply with the saved path /status show the last 5 runs from the run-history store Register your own commands by calling ``router.register("deploy", …)`` @@ -60,7 +61,7 @@ def main() -> None: try: bot.run_forever() except KeyboardInterrupt: - bot.stop() + bot.stop() # also stops a /run still going and waits for it if __name__ == "__main__": diff --git a/je_auto_control/actions.pyi b/je_auto_control/actions.pyi index 18473d4c1..e4a5a8545 100644 --- a/je_auto_control/actions.pyi +++ b/je_auto_control/actions.pyi @@ -1284,6 +1284,7 @@ def AC_execute_action( _validated: bool = ..., dry_run: bool = ..., step_callback: Callable[[list], None] | None = ..., + result_callback: Any = ..., ) -> Dict[str, str]: """執行 action list""" diff --git a/je_auto_control/gui/language_wrapper/english.py b/je_auto_control/gui/language_wrapper/english.py index 893dea8ee..606825fa4 100644 --- a/je_auto_control/gui/language_wrapper/english.py +++ b/je_auto_control/gui/language_wrapper/english.py @@ -990,10 +990,10 @@ "sb_one_time_step": "Step {step}", "sb_one_time_copy": "Copy", "sb_one_time_available": "{count} one-time value(s) available: press 'One-time values' to read them, once.", - "sb_one_time_nested": ( - "Not available here: values issued inside a block ({commands}). " - "A block records no result for the commands in its body." - ), + "sb_one_time_inside": "{block} run {run}, step {step}", + "sb_one_time_shown": "The one-time values were shown and are no longer kept.", + "sb_one_time_limit": "Only the first {limit} one-time values of this run were kept; later ones cannot be shown.", + "sb_one_time_close_clear": "Close and clear clipboard", "jr_dialog_open": "Open action journal", "jr_dialog_start": "Journal file to append to", "jr_dialog_save_code": "Save candidate script", diff --git a/je_auto_control/gui/language_wrapper/japanese.py b/je_auto_control/gui/language_wrapper/japanese.py index 9035c5602..45c29fd9e 100644 --- a/je_auto_control/gui/language_wrapper/japanese.py +++ b/je_auto_control/gui/language_wrapper/japanese.py @@ -880,10 +880,10 @@ "sb_one_time_step": "ステップ {step}", "sb_one_time_copy": "コピー", "sb_one_time_available": "一度だけの値が {count} 件あります。「一度だけの値」を押すと一度だけ確認できます。", - "sb_one_time_nested": ( - "ここでは取得できません:ブロック内で発行された値({commands})。" - "ブロックは内部のコマンドの結果を記録しません。" - ), + "sb_one_time_inside": "{block} {run} 回目、ステップ {step}", + "sb_one_time_shown": "一度だけの値は表示済みで、もう保持されていません。", + "sb_one_time_limit": "この実行の一度だけの値は最初の {limit} 件だけ保持されました。それ以降の値は表示できません。", + "sb_one_time_close_clear": "閉じてクリップボードを消去", "jr_dialog_open": "アクションジャーナルを開く", "jr_dialog_start": "追記するジャーナルファイル", "jr_dialog_save_code": "候補スクリプトを保存", diff --git a/je_auto_control/gui/language_wrapper/simplified_chinese.py b/je_auto_control/gui/language_wrapper/simplified_chinese.py index 6b6acb212..36524d712 100644 --- a/je_auto_control/gui/language_wrapper/simplified_chinese.py +++ b/je_auto_control/gui/language_wrapper/simplified_chinese.py @@ -850,10 +850,10 @@ "sb_one_time_step": "步骤 {step}", "sb_one_time_copy": "复制", "sb_one_time_available": "有 {count} 个一次性的值:按「一次性的值」查看,只能看一次。", - "sb_one_time_nested": ( - "这里无法取得:在区块内产生的值({commands})。" - "区块不会记录其内部指令的结果。" - ), + "sb_one_time_inside": "{block} 第 {run} 次,步骤 {step}", + "sb_one_time_shown": "一次性的值已经显示过,不再保留。", + "sb_one_time_limit": "这次执行只保留了前 {limit} 个一次性的值;之后的值无法显示。", + "sb_one_time_close_clear": "关闭并清除剪贴板", "jr_dialog_open": "打开动作日志", "jr_dialog_start": "要追加写入的日志文件", "jr_dialog_save_code": "保存候选脚本", diff --git a/je_auto_control/gui/language_wrapper/traditional_chinese.py b/je_auto_control/gui/language_wrapper/traditional_chinese.py index 356f6eb3b..918b9ab13 100644 --- a/je_auto_control/gui/language_wrapper/traditional_chinese.py +++ b/je_auto_control/gui/language_wrapper/traditional_chinese.py @@ -857,10 +857,10 @@ "sb_one_time_step": "步驟 {step}", "sb_one_time_copy": "複製", "sb_one_time_available": "有 {count} 個一次性的值:按「一次性的值」查看,只能看一次。", - "sb_one_time_nested": ( - "這裡無法取得:在區塊內產生的值({commands})。" - "區塊不會記錄其內部指令的結果。" - ), + "sb_one_time_inside": "{block} 第 {run} 次,步驟 {step}", + "sb_one_time_shown": "一次性的值已經顯示過,不再保留。", + "sb_one_time_limit": "這次執行只保留了前 {limit} 個一次性的值;之後的值無法顯示。", + "sb_one_time_close_clear": "關閉並清除剪貼簿", "jr_dialog_open": "開啟動作日誌", "jr_dialog_start": "要附加寫入的日誌檔", "jr_dialog_save_code": "儲存候選腳本", diff --git a/je_auto_control/gui/script_builder/builder_tab.py b/je_auto_control/gui/script_builder/builder_tab.py index 0a380500b..786f1eb98 100644 --- a/je_auto_control/gui/script_builder/builder_tab.py +++ b/je_auto_control/gui/script_builder/builder_tab.py @@ -3,15 +3,18 @@ Run executes the steps off the GUI thread as a stoppable executor run, one at a time; Stop ends it at its next checkpoint. -A step whose result carries a secret (``sensitive_result`` in the schema: a -freshly issued token) is masked in the result pane. The values of the last -run are kept in memory, and the "one-time values" button hands them to -:class:`OneTimeValueDialog` -- once: the tab forgets them as it opens the -dialog, when the next run starts and when it is disposed. +Every secret-named field of a result (a freshly issued token, at any depth, +whatever the command) is masked in the result pane. The run reports each +command's result -- the ones inside a block too -- to a +:class:`OneTimeCollector`, which keeps what the pane masks, in memory. The +"one-time values" button hands those values to :class:`OneTimeValueDialog` -- +once: the tab forgets them as it opens the dialog, when the next run starts +and when it is disposed. A run that was stopped or failed still offers what +it had issued by then. """ import functools import json -from typing import List, Optional +from typing import List, Optional, Sequence from PySide6.QtCore import Qt from PySide6.QtGui import QAction @@ -34,8 +37,8 @@ from je_auto_control.gui.script_builder.step_form_view import StepFormView from je_auto_control.gui.script_builder.step_list_view import StepTreeView from je_auto_control.gui.script_builder.step_model import ( - OneTimeValue, Step, actions_to_steps, displayable_record, load_action_file, - nested_sensitive_commands, one_time_values, save_action_file, steps_to_actions, + OneTimeCollector, OneTimeValue, Step, actions_to_steps, displayable_record, + load_action_file, save_action_file, steps_to_actions, ) from je_auto_control.utils.exception.exceptions import AutoControlException from je_auto_control.utils.executor.action_executor import execute_action @@ -45,9 +48,9 @@ def _t(key: str) -> str: return language_wrapper.translate(key, key) -def _run_actions(actions: list) -> object: - """Worker thread: execute the built action list.""" - return execute_action(actions) +def _run_actions(actions: list, collector: OneTimeCollector) -> object: + """Worker thread: execute the built action list, reporting each result to ``collector``.""" + return execute_action(actions, result_callback=collector.note) class ScriptBuilderTab(TranslatableMixin, QWidget): @@ -67,7 +70,10 @@ def __init__(self, parent: Optional[QWidget] = None) -> None: self._file_extras: Optional[dict] = None # The secrets of the last run, until they are revealed or the next run starts. self._one_time: List[OneTimeValue] = [] - self._nested_marked: List[str] = [] + # What the running (or last) run reported; emptied when its values are claimed. + self._collector: Optional[OneTimeCollector] = None + # The line of the result pane that says values are waiting, to replace once they are shown. + self._available_line = "" self._reveal_btn = QPushButton() self._reveal_dialog: Optional[OneTimeValueDialog] = None self._runs = TabTask(self) @@ -218,11 +224,12 @@ def _on_run(self) -> None: if not actions: QMessageBox.information(self, "Info", "No steps to run") return - if not self._runs.start_script(functools.partial(_run_actions, actions)): + collector = OneTimeCollector() + if not self._runs.start_script(functools.partial(_run_actions, actions, collector)): self._result.setPlainText(_t("task_busy")) return self._forget_one_time() - self._nested_marked = nested_sensitive_commands(actions) + self._collector = collector self._result.setPlainText(_t("task_running")) def _on_stop(self) -> None: @@ -230,19 +237,29 @@ def _on_stop(self) -> None: self._result.setPlainText(_t("task_stopping")) def _show_run_result(self, result: object) -> None: - # Masked first: AC_user_add / AC_user_rotate_token answer with a token. - lines = [json.dumps(displayable_record(result), indent=2, default=str, ensure_ascii=False)] - self._one_time = one_time_values(result) + # Masked first: a result may carry a token (AC_user_add, AC_jwt_encode, a lease...). + self._show_with_one_time( + [json.dumps(displayable_record(result), indent=2, default=str, ensure_ascii=False)]) + + def _show_with_one_time(self, lines: Sequence[str]) -> None: + """Fill the result pane with ``lines`` and say what the ended run left to reveal.""" + shown = [line for line in lines if line] + collector, self._collector = self._collector, None + self._one_time, truncated = collector.take() if collector is not None else ([], False) + self._available_line = "" if self._one_time: - lines.append(_t("sb_one_time_available").format(count=len(self._one_time))) - if self._nested_marked: - lines.append(_t("sb_one_time_nested").format(commands=", ".join(self._nested_marked))) - self._result.setPlainText("\n\n".join(lines)) + self._available_line = _t("sb_one_time_available").format(count=len(self._one_time)) + shown.append(self._available_line) + if truncated and collector is not None: + shown.append(_t("sb_one_time_limit").format(limit=collector.limit)) + self._result.setPlainText("\n\n".join(shown)) self._reveal_btn.setEnabled(bool(self._one_time)) def _forget_one_time(self) -> None: """Drop the last run's secrets and close the dialog still showing them.""" self._one_time = [] + self._collector = None + self._available_line = "" self._reveal_btn.setEnabled(False) dialog, self._reveal_dialog = self._reveal_dialog, None if dialog is not None: @@ -255,12 +272,20 @@ def _take_one_time_dialog(self) -> Optional[OneTimeValueDialog]: return None values, self._one_time = self._one_time, [] self._reveal_btn.setEnabled(False) + self._mark_one_time_shown() dialog = OneTimeValueDialog(values, self) dialog.finished.connect(dialog.deleteLater) dialog.finished.connect(self._on_reveal_closed) self._reveal_dialog = dialog return dialog + def _mark_one_time_shown(self) -> None: + """Replace the pane's "values are available" line: they were shown and are gone.""" + line, self._available_line = self._available_line, "" + text = self._result.toPlainText() + if line and line in text: + self._result.setPlainText(text.replace(line, _t("sb_one_time_shown"))) + def _on_reveal(self) -> None: dialog = self._take_one_time_dialog() if dialog is not None: @@ -270,8 +295,9 @@ def _on_reveal_closed(self, _result: int = 0) -> None: self._reveal_dialog = None def _show_run_error(self, error: object) -> None: + # A token issued before the stop or the failure exists; it is still offered. if was_stopped(error): - self._result.setPlainText(_t("task_stopped")) + self._show_with_one_time([_t("task_stopped")]) return - self._result.setPlainText("") + self._show_with_one_time([]) QMessageBox.warning(self, "Error", str(error)) diff --git a/je_auto_control/gui/script_builder/command_schema.py b/je_auto_control/gui/script_builder/command_schema.py index adaa76243..232f4ae32 100644 --- a/je_auto_control/gui/script_builder/command_schema.py +++ b/je_auto_control/gui/script_builder/command_schema.py @@ -41,9 +41,10 @@ class CommandSpec: fields: Tuple[FieldSpec, ...] = () body_keys: Tuple[str, ...] = () description: str = "" - #: The command's result carries a secret that exists nowhere else (a - #: freshly issued token). The builder masks it before displaying a run and - #: offers it once through its one-time reveal. + #: The command's result carries a secret (a freshly issued token). The + #: builder masks secret-named fields in every command's result and offers + #: them once through its one-time reveal, flagged or not; the flag says so + #: in the schema and also hides a result that has no named fields at all. sensitive_result: bool = False @@ -2164,7 +2165,8 @@ def _add_misc_specs(specs: List[CommandSpec]) -> None: FieldSpec("requester", FieldType.STRING, optional=True), FieldSpec("db", FieldType.STRING, optional=True), ), - description="Maker-checker: file a high-risk action for approval.", + description="Maker-checker: file a high-risk action for approval; returns {token}, the " + "request id to hand to the checker (shown, not masked).", )) specs.append(CommandSpec( "AC_approval_approve", "Tools", "Approval: Approve", @@ -2198,7 +2200,9 @@ def _add_misc_specs(specs: List[CommandSpec]) -> None: FieldSpec("name", FieldType.STRING), FieldSpec("ttl", FieldType.FLOAT, optional=True, default=300.0), ), - description="Issue a short-lived JIT lease for a secret (no value).", + description="Issue a short-lived JIT lease for a secret (no value); returns {token, ttl}. " + "This builder masks the token -- read it with 'One-time values' after the run.", + sensitive_result=True, )) specs.append(CommandSpec( "AC_lease_valid", "Tools", "Lease: Valid?", @@ -2213,7 +2217,9 @@ def _add_misc_specs(specs: List[CommandSpec]) -> None: specs.append(CommandSpec( "AC_lease_active", "Tools", "Lease: List Active", fields=(), - description="List active leases (token, name, ttl_remaining).", + description="List active leases (token, name, ttl_remaining). This builder masks each " + "token -- read them with 'One-time values' after the run.", + sensitive_result=True, )) specs.append(CommandSpec( "AC_egress_allow", "Tools", "Egress: Set Allowlist", diff --git a/je_auto_control/gui/script_builder/one_time_dialog.py b/je_auto_control/gui/script_builder/one_time_dialog.py index 66d43ed45..4a7de206f 100644 --- a/je_auto_control/gui/script_builder/one_time_dialog.py +++ b/je_auto_control/gui/script_builder/one_time_dialog.py @@ -8,6 +8,11 @@ drops the values. Nothing here logs, saves or records a value; the only way one leaves the dialog is the Copy button, onto the clipboard. +A value copied here stays on the clipboard after the dialog is gone, so once +one was copied the dialog offers a second way out, "Close and clear +clipboard". It empties the clipboard only while the clipboard still holds a +value copied from this dialog; anything put there since is left alone. + The tab opens it with ``open()`` (window-modal, not a nested event loop), only when its "one-time values" button is pressed. """ @@ -25,6 +30,8 @@ from je_auto_control.gui.script_builder.command_schema import COMMAND_SPECS from je_auto_control.gui.script_builder.step_model import OneTimeValue +_PATH_SEPARATOR = " › " + def _t(key: str) -> str: return str(language_wrapper.translate(key, key)) @@ -35,12 +42,35 @@ def _copy_to_clipboard(text: str) -> None: QGuiApplication.clipboard().setText(text) +def _clipboard_text() -> str: + """What the clipboard holds now, as text.""" + return str(QGuiApplication.clipboard().text()) + + +def _clear_clipboard() -> None: + """Empty the clipboard.""" + QGuiApplication.clipboard().clear() + + +def _command_label(command: str) -> str: + spec = COMMAND_SPECS.get(command) + return spec.label if spec is not None else command + + +def path_label(value: OneTimeValue) -> str: + """Where ``value``'s step ran: ``Step 2 › Loop run 3, step 1`` for a step inside a block.""" + if not value.path: + return "" + parts = [_t("sb_one_time_step").format(step=value.path[0].position)] + for block, inner in zip(value.path, value.path[1:]): + parts.append(_t("sb_one_time_inside").format( + block=_command_label(block.command), run=inner.run, step=inner.position)) + return _PATH_SEPARATOR.join(parts) + + def value_label(value: OneTimeValue) -> str: - """What names ``value`` in the dialog: its step, its command and whom it is for.""" - spec = COMMAND_SPECS.get(value.command) - parts = [_t("sb_one_time_step").format(step=value.step), - spec.label if spec is not None else value.command, - value.subject, value.name] + """What names ``value`` in the dialog: where its step ran, its command and whom it is for.""" + parts = [path_label(value), _command_label(value.command), value.subject, value.name] return " · ".join(part for part in parts if part) @@ -53,6 +83,7 @@ def __init__(self, values: Sequence[OneTimeValue], self.setWindowTitle(_t("sb_one_time_title")) self._values: List[OneTimeValue] = list(values) self._fields: List[QLineEdit] = [] + self._copied: List[str] = [] root = QVBoxLayout(self) notice = QLabel(_t("sb_one_time_notice")) notice.setWordWrap(True) @@ -63,6 +94,10 @@ def __init__(self, values: Sequence[OneTimeValue], root.addLayout(grid) buttons = QDialogButtonBox(QDialogButtonBox.StandardButton.Close) buttons.rejected.connect(self.reject) + self._clear_btn = buttons.addButton(_t("sb_one_time_close_clear"), + QDialogButtonBox.ButtonRole.ActionRole) + self._clear_btn.setEnabled(False) + self._clear_btn.clicked.connect(self.close_and_clear) root.addWidget(buttons) self.setMinimumWidth(560) self.finished.connect(self.forget) @@ -89,14 +124,37 @@ def copy_value(self, index: int) -> bool: """Copy value ``index`` to the clipboard; ``False`` once it is forgotten.""" if not 0 <= index < len(self._values): return False - _copy_to_clipboard(self._values[index].value) + text = self._values[index].value + _copy_to_clipboard(text) + self._copied.append(text) + self._clear_btn.setEnabled(True) return True + def can_clear_clipboard(self) -> bool: + """Whether a value was copied from this dialog, so clearing is on offer.""" + return self._clear_btn.isEnabled() + + def clear_clipboard(self) -> bool: + """Empty the clipboard if it still holds a value copied here; say whether it did.""" + copied, self._copied = self._copied, [] + self._clear_btn.setEnabled(False) + if not copied or _clipboard_text() not in copied: + return False + _clear_clipboard() + return True + + def close_and_clear(self) -> None: + """The "Close and clear clipboard" button: clear what was copied here, then close.""" + self.clear_clipboard() + self.reject() + def forget(self, _result: int = 0) -> None: """Empty every field and drop the values; runs when the dialog closes.""" for field in self._fields: field.setText("") # also resets the field's undo history self._values = [] + self._copied = [] + self._clear_btn.setEnabled(False) -__all__ = ["OneTimeValueDialog", "value_label"] +__all__ = ["OneTimeValueDialog", "path_label", "value_label"] diff --git a/je_auto_control/gui/script_builder/step_model.py b/je_auto_control/gui/script_builder/step_model.py index 091be160c..d051423f5 100644 --- a/je_auto_control/gui/script_builder/step_model.py +++ b/je_auto_control/gui/script_builder/step_model.py @@ -1,17 +1,21 @@ """Step data model and (de)serialisation between the tree view and AC JSON. -Also what the builder may show of a run: :func:`displayable_record` masks the -results of commands whose schema entry is marked ``sensitive_result``, and -:func:`one_time_values` lists what it masked, for the builder's one-time -reveal. Neither writes anything anywhere; both are free of Qt. +Also what the builder may show of a run: :func:`displayable_record` masks +every secret-named field of every command's result, and +:class:`OneTimeCollector` gathers what it masked while the run reports each +result, for the builder's one-time reveal. Neither writes anything anywhere; +both are free of Qt. """ import json -import re +import threading from dataclasses import dataclass, field -from typing import Any, Dict, Iterator, List, Mapping, Optional, Set, Tuple +from typing import Any, Dict, List, Mapping, Optional, Tuple from je_auto_control.gui.script_builder.command_schema import COMMAND_SPECS -from je_auto_control.utils.executor.action_redaction import SENSITIVE_ARGUMENT_NAMES +from je_auto_control.utils.executor.action_redaction import ( + FieldPath, MaskedField, masked_fields, record_command, redact_result, +) +from je_auto_control.utils.executor.result_hook import StepPath from je_auto_control.utils.json.json_file import read_action_json, write_action_json @@ -46,133 +50,140 @@ def label(self) -> str: #: Shown in place of a marked command's result that is not a mapping. HIDDEN_RESULT = "(hidden: this command's result carries a secret)" -_MASK = "***" -# A record key is "execute: " + str(action); the command is its first element. -_RECORD_COMMAND = re.compile(r"^execute: \['(AC_\w+)'") +#: The most one-time values kept for one run; a loop issuing more keeps the first ones. +MAX_ONE_TIME_VALUES = 200 def displayable_record(record: Any) -> Any: """A copy of an execution record that is safe to print in the builder. - The result of a command marked ``sensitive_result`` -- at the top level or - inside a block's nested record -- keeps its shape with every secret-named - field masked, so the row still says which user it was. A failure is a - string (the error's ``repr``) and is shown as it is. The record itself is - not changed. + Every command's result keeps its shape with every secret-named field + masked, at any depth, by the rule the executor's result log uses + (:func:`~je_auto_control.utils.executor.action_redaction.redact_result`); + the identifiers that rule lists (an approval request's id) stay readable. + A failure is a string (the error's ``repr``) and is shown as it is. The + result of a command marked ``sensitive_result`` that has no fields to mask + by name is hidden whole. The record itself is not changed. """ - if isinstance(record, list): - return [displayable_record(item) for item in record] if not isinstance(record, dict): - return record - shown: Dict[Any, Any] = {} - for key, value in record.items(): - if _marked_command(key) is not None: - shown[key] = _without_secrets(value) - else: - shown[key] = displayable_record(value) - return shown + return redact_result(None, record, show_identifiers=True) + return {key: _shown_result(record_command(key), value) for key, value in record.items()} + + +def _shown_result(command: Optional[str], value: Any) -> Any: + if _hidden_whole(command, value): + return HIDDEN_RESULT + return redact_result(command, value, show_identifiers=True) -def _marked_command(key: Any) -> Optional[str]: - """The command of record key ``key`` when its result is marked sensitive.""" - match = _RECORD_COMMAND.match(key) if isinstance(key, str) else None - if match is None: - return None - spec = COMMAND_SPECS.get(match.group(1)) - return match.group(1) if spec is not None and spec.sensitive_result else None +def _hidden_whole(command: Optional[str], value: Any) -> bool: + """Whether ``value`` is a marked command's result with no named fields (a string is a failure).""" + spec = COMMAND_SPECS.get(command or "") + return (spec is not None and spec.sensitive_result + and value is not None and not isinstance(value, (dict, str))) @dataclass(frozen=True) class OneTimeValue: """One secret a run produced, with what identifies the step that produced it. - ``step`` is the 1-based position of the top-level step in the run, - ``subject`` what the command acted on (the user id) and ``name`` the - result field that held the value. ``value`` is left out of the text form, - so formatting or logging the object never prints it. + ``path`` is where the step ran (one + :class:`~je_auto_control.utils.executor.result_hook.StepPosition` per + enclosing action list, outermost first), ``subject`` what the command + acted on (the user id, the secret's name) and ``name`` the result field + that held the value. ``value`` is left out of the text form, so formatting + or logging the object never prints it. """ - step: int + path: StepPath command: str subject: str name: str value: str = field(repr=False) + @property + def step(self) -> int: + """The 1-based position of the top-level step the value came from.""" + return self.path[0].position if self.path else 0 -def one_time_values(record: Any) -> List[OneTimeValue]: - """Every secret :func:`displayable_record` would mask in ``record``, in run order. - A command that failed (its result is the error's text) produced none. The - record is only read. - """ - found: List[OneTimeValue] = [] - if isinstance(record, dict): - for position, (key, value) in enumerate(record.items(), start=1): - _collect_secrets(key, value, position, found) - return found - - -def _collect_secrets(key: Any, value: Any, step: int, found: List[OneTimeValue]) -> None: - command = _marked_command(key) - if command is not None: - found.extend(_secrets_of(step, command, value)) - return - for nested_key, nested in _entries(value): - _collect_secrets(nested_key, nested, step, found) - - -def _entries(value: Any) -> Iterator[Tuple[Any, Any]]: - """The ``(key, item)`` pairs of a container; a list's items have no key.""" - if isinstance(value, dict): - yield from value.items() - elif isinstance(value, list): - for item in value: - yield None, item - - -def _secrets_of(step: int, command: str, value: Any) -> List[OneTimeValue]: - """What a marked command's result holds that the display hides.""" - if value is None or isinstance(value, str): - return [] - if not isinstance(value, dict): - text = json.dumps(value, default=str, ensure_ascii=False) - return [OneTimeValue(step, command, "", "", text)] - subject = str(value.get("user_id", "")) - return [OneTimeValue(step, command, subject, str(name), str(item)) - for name, item in value.items() - if str(name).lower() in SENSITIVE_ARGUMENT_NAMES and item not in (None, "")] - - -def nested_sensitive_commands(actions: Any) -> List[str]: - """The marked commands that sit inside a block of ``actions``, sorted. - - A block (``AC_loop``, ``AC_if_*``, ``AC_try``...) records its own summary, - not the results of the commands in its body, so a secret issued there is in - no record and cannot be revealed afterwards. +class OneTimeCollector: + """Gathers the secrets of one run as the executor reports each command's result. + + :meth:`note` is the run's result hook + (``execute_action(actions, result_callback=collector.note)``), so a + command in the body of a block is seen as well as a top-level one. What + it keeps of a result is exactly what :func:`displayable_record` masks in + it. At most ``limit`` values are kept -- the first ones; :meth:`take` + says whether any were left out. Nothing is stored outside this object. """ - names: Set[str] = set() - for action in actions if isinstance(actions, list) else []: - for argument in action[1:] if isinstance(action, list) else []: - _nested_marked(argument, names) - return sorted(names) - - -def _nested_marked(value: Any, names: Set[str]) -> None: - if isinstance(value, list) and value and isinstance(value[0], str) and value[0] in COMMAND_SPECS: - if COMMAND_SPECS[value[0]].sensitive_result: - names.add(value[0]) - value = value[1:] - for _key, item in _entries(value): - _nested_marked(item, names) - - -def _without_secrets(value: Any) -> Any: - """``value`` with its secret-named fields masked; hidden whole when it has no fields.""" - if isinstance(value, str): - return value - if not isinstance(value, dict): - return HIDDEN_RESULT - return {key: _MASK if str(key).lower() in SENSITIVE_ARGUMENT_NAMES else item - for key, item in value.items()} + + def __init__(self, limit: int = MAX_ONE_TIME_VALUES) -> None: + #: The most values kept for the run. + self.limit = limit + self._values: List[OneTimeValue] = [] + self._truncated = False + self._lock = threading.Lock() + + def note(self, command: str, arguments: Any, result: Any, path: StepPath) -> None: + """Keep the secrets of ``result``, what ``command`` at ``path`` returned.""" + if _hidden_whole(command, result): + fields = [MaskedField((), result)] + else: + fields = masked_fields(command, result, show_identifiers=True) + if not fields: + return + with self._lock: + # A block that returns its body's record repeats what the body's steps reported. + reported = {value.value for value in self._values + if len(value.path) > len(path) and value.path[:len(path)] == path} + for found in fields: + text = _as_text(found.value) + if text not in reported: + self._keep(OneTimeValue(path, command, _subject(result, arguments, found.path), + _field_name(found.path), text)) + + def _keep(self, value: OneTimeValue) -> None: + if len(self._values) < self.limit: + self._values.append(value) + else: + self._truncated = True + + def take(self) -> Tuple[List[OneTimeValue], bool]: + """The values kept, in run order, and whether the limit left some out; then forget them.""" + with self._lock: + values, self._values = self._values, [] + truncated, self._truncated = self._truncated, False + return values, truncated + + +def _as_text(value: Any) -> str: + return value if isinstance(value, str) else json.dumps(value, default=str, ensure_ascii=False) + + +def _field_name(path: FieldPath) -> str: + """``leases[0].token`` for ``("leases", 0, "token")``; a record key is named by its command.""" + name = "" + for part in path: + if isinstance(part, int): + name += f"[{part}]" + else: + name += ("." if name else "") + (record_command(part) or str(part)) + return name + + +_SUBJECT_KEYS = ("user_id", "name") + + +def _subject(result: Any, arguments: Any, path: FieldPath) -> str: + """Whom or what a value is for: named beside it in the result, else in the arguments.""" + holder = result + for part in path[:-1]: + holder = holder[part] + for source in (holder, arguments): + for key in _SUBJECT_KEYS if isinstance(source, dict) else (): + if isinstance(source.get(key), (str, int)) and source[key] != "": + return str(source[key]) + return "" def step_to_action(step: Step) -> list: diff --git a/je_auto_control/utils/chatops/slack_bot.py b/je_auto_control/utils/chatops/slack_bot.py index e123fd88d..384f85712 100644 --- a/je_auto_control/utils/chatops/slack_bot.py +++ b/je_auto_control/utils/chatops/slack_bot.py @@ -7,6 +7,13 @@ infrastructure where the bot lives on a private network and pulls work rather than receiving pushes. +A command named in ``background_commands`` (``/run`` by default) executes on +a worker thread and is answered when it ends, so the poll loop keeps reading +while it lasts -- that is what lets a ``/stop`` posted to the channel reach +the bot's own run. One such command runs at a time per bot; a second is +refused with a reply. Every other command is dispatched and answered on the +poll thread, as before. + Three pieces of state are tracked per channel: * ``last_seen_ts`` — the Slack timestamp of the most-recent message we @@ -23,10 +30,14 @@ import urllib.parse import urllib.request from dataclasses import dataclass, field -from typing import Any, Dict, Optional +from typing import Any, Dict, FrozenSet, List, Optional -from je_auto_control.utils.chatops.router import CommandResult, CommandRouter +from je_auto_control.utils.chatops.handlers import chatops_run_id +from je_auto_control.utils.chatops.router import ( + ChatOpsError, CommandResult, CommandRouter, +) from je_auto_control.utils.exception.exceptions import AutoControlException +from je_auto_control.utils.executor.run_control import StopToken, stoppable_run from je_auto_control.utils.http_client.http_client import build_call, perform_call from je_auto_control.utils.logging.logging_instance import autocontrol_logger @@ -36,6 +47,17 @@ _MIN_POLL_INTERVAL = 1.0 _MAX_PAGES = 20 # 1000 messages per poll _MAX_BACKOFF = 60.0 +#: How long the poll thread waits for a started command to be listed as a run. +_START_WAIT = 10.0 +#: How long :meth:`SlackBot.stop` waits for the running command by default. +_JOIN_TIMEOUT = 10.0 +#: What the name of every worker thread begins with. +_WORKER_PREFIX = "chatops-slack-" +#: What a dispatch may fail with beyond what the router already contains: +#: defence in depth, as wide as the router's own boundary. An ImportError +#: from a handler's lazy import ended ``run_forever`` for good. +_ROUTE_ERRORS = (RuntimeError, ValueError, TypeError, LookupError, AttributeError, + ImportError, ArithmeticError, OSError, AutoControlException) class SlackError(AutoControlException, RuntimeError): @@ -44,7 +66,11 @@ class SlackError(AutoControlException, RuntimeError): @dataclass class SlackBot: - """Polling Slack adapter wrapped around a :class:`CommandRouter`.""" + """Polling Slack adapter wrapped around a :class:`CommandRouter`. + + ``background_commands`` names the commands that run on a worker thread + (case-insensitive, without the prefix); see the module docstring. + """ token: str channel_id: str @@ -53,6 +79,16 @@ class SlackBot: last_seen_ts: Optional[str] = None _bot_user_id: Optional[str] = None _stop: threading.Event = field(default_factory=threading.Event) + background_commands: FrozenSet[str] = frozenset({"run"}) + # The one background command in progress: its thread and its stop token. + _worker: Optional[threading.Thread] = field(default=None, init=False, repr=False) + _run_token: Optional[StopToken] = field(default=None, init=False, repr=False) + _state_lock: threading.Lock = field( + default_factory=threading.Lock, init=False, repr=False) + # Held while one message is dispatched and answered, and by the worker + # while it posts its reply: the answer to a /stop is posted before the + # "stopped" reply of the run it ended. + _turn: threading.Lock = field(default_factory=threading.Lock, init=False, repr=False) def __post_init__(self) -> None: if not self.token or not self.token.startswith("xox"): @@ -63,11 +99,19 @@ def __post_init__(self) -> None: raise SlackError( f"poll_interval_s must be >= {_MIN_POLL_INTERVAL}", ) + if isinstance(self.background_commands, str): + raise SlackError("background_commands must be a set of command names") + self.background_commands = frozenset( + str(name).strip().lower() for name in self.background_commands) # --- polling ------------------------------------------------- def poll_once(self) -> int: - """Pull new messages, dispatch each through the router. Returns count.""" + """Pull new messages, dispatch each through the router. Returns count. + + A background command counts once it is started (or refused); the call + returns without waiting for it. + """ messages = self._fetch_messages() if not messages: return 0 @@ -87,7 +131,7 @@ def poll_once(self) -> int: if self._is_self(msg): continue text = str(msg.get("text") or "") - if self._route_one(text, msg) is not None: + if self._handle(text, msg): dispatched += 1 return dispatched @@ -111,26 +155,80 @@ def run_forever(self, *, max_iterations: Optional[int] = None) -> None: return stop.wait(self.poll_interval_s + backoff) - def stop(self) -> None: + def stop(self, timeout: float = _JOIN_TIMEOUT) -> bool: + """End the poll loop, stop the running background command and join it. + + The command is asked to stop the way ``/stop`` asks, and its worker is + waited for up to ``timeout`` seconds; it still posts its reply. Returns + whether no worker is left. ``False`` means the command is inside a + call that cannot be interrupted (the worker is a daemon thread and + ends with the process), or that this was called from a command + handler, which holds the turn the worker needs to post its reply. + Until :meth:`run_forever` is called again a background command is + refused rather than started. + """ self._stop.set() + with self._state_lock: + worker, token = self._worker, self._run_token + if worker is None or not worker.is_alive(): + return True + if token is not None: + token.stop("the bot was stopped") + return self._join(worker, timeout) + + def wait_idle(self, timeout: float = _JOIN_TIMEOUT) -> bool: + """Wait up to ``timeout`` seconds for the running background command to end. + + Nothing is stopped. Returns whether the bot has no command running. + """ + with self._state_lock: + worker = self._worker + if worker is None or not worker.is_alive(): + return True + return self._join(worker, timeout) + + @property + def running_run_id(self) -> Optional[str]: + """The run id of the background command in progress, or ``None``.""" + with self._state_lock: + if self._worker is None or not self._worker.is_alive(): + return None + return self._run_token.run_id if self._run_token is not None else None + + @staticmethod + def _join(worker: threading.Thread, timeout: float) -> bool: + """Join ``worker`` for at most ``timeout`` seconds; whether it has ended.""" + if worker is threading.current_thread(): + return False # called from the command itself: it cannot wait for itself + worker.join(max(0.0, float(timeout))) + if worker.is_alive(): + autocontrol_logger.warning( + f"chatops slack: {worker.name} is still running after {timeout}s") + return False + return True # --- routing ------------------------------------------------- + def _handle(self, text: str, message: Dict[str, Any]) -> bool: + """Dispatch one message; whether it was a command.""" + with self._turn: + argv = self._background_argv(text) + if argv is None: + return self._route_one(text, message) is not None + return self._start_background(argv, text, message) + + def _context(self, message: Dict[str, Any]) -> Dict[str, Any]: + return {"slack_user": message.get("user"), "slack_ts": message.get("ts"), + "slack_channel": self.channel_id} + def _route_one(self, text: str, message: Dict[str, Any]) -> Optional[CommandResult]: # The router already converts handler failures into CommandResults; # this is defence in depth so a framework error surfaces as a chat # reply rather than escaping poll_once and stopping run_forever. try: - result = self.router.dispatch( - text, context={"slack_user": message.get("user"), - "slack_ts": message.get("ts"), - "slack_channel": self.channel_id}, - ) - # Defence in depth, as wide as the router's own boundary: an - # ImportError from a handler's lazy import ended run_forever for good. - except (RuntimeError, ValueError, TypeError, LookupError, AttributeError, - ImportError, ArithmeticError, OSError, AutoControlException) as error: + result = self.router.dispatch(text, context=self._context(message)) + except _ROUTE_ERRORS as error: self.post_message(f"router error: {error}") return None if result is None: @@ -138,6 +236,78 @@ def _route_one(self, text: str, self.post_message(result.text, thread_ts=str(message.get("ts") or "")) return result + # --- background commands ------------------------------------- + + def _background_argv(self, text: str) -> Optional[List[str]]: + """The parsed command when ``text`` is one that runs in the background.""" + try: + argv = self.router.parse(text) + except ChatOpsError: + return None # the synchronous path answers with the parse error + if not argv or argv[0].lower() not in self.background_commands: + return None + return argv + + def _start_background(self, argv: List[str], text: str, + message: Dict[str, Any]) -> bool: + """Start ``text`` on a worker, or say why not; always ``True`` (it was a command).""" + name = argv[0].lower() + listed = threading.Event() + refusal = "" + with self._state_lock: + if self._worker is not None and self._worker.is_alive(): + running = self._run_token.run_id if self._run_token is not None else "?" + refusal = (f"{name}: already running ({running}); wait for it to end " + f"or {self.router.prefix}stop it first.") + elif self._stop.is_set(): + refusal = f"{name}: the bot is stopping; not started." + else: + token = StopToken(chatops_run_id(argv[1] if len(argv) > 1 else name)) + self._run_token = token + self._worker = threading.Thread( + target=self._run_background, args=(text, message, token, listed), + name=f"{_WORKER_PREFIX}{token.run_id}", daemon=True) + self._worker.start() + if refusal: + self.post_message(refusal, thread_ts=str(message.get("ts") or "")) + return True + # The next message may be the /stop for this run: read it only once + # the run is listed (or has already ended). + listed.wait(_START_WAIT) + return True + + def _run_background(self, text: str, message: Dict[str, Any], + token: StopToken, listed: threading.Event) -> None: + """Worker thread: run one command as a stoppable run, then post its reply.""" + try: + reply = self._dispatch_stoppable(text, message, token, listed) + finally: + listed.set() + if not reply: + return + with self._turn: + try: + self.post_message(reply, thread_ts=str(message.get("ts") or "")) + except SlackError as error: + # Nobody to raise to: the command has run and is not retried. + autocontrol_logger.warning( + f"chatops slack: reply of {token.run_id} not posted: {error}") + + def _dispatch_stoppable(self, text: str, message: Dict[str, Any], + token: StopToken, listed: threading.Event) -> str: + """Dispatch ``text`` inside the run named by ``token``; the reply text. + + ``/run`` joins the run it finds itself in, so ``token`` is the one + ``/stop`` and :meth:`stop` address, from before the script is opened. + """ + try: + with stoppable_run(token=token): + listed.set() + result = self.router.dispatch(text, context=self._context(message)) + except _ROUTE_ERRORS as error: + return f"router error: {error}" + return result.text if result is not None else "" + # --- HTTP wrappers ------------------------------------------- def post_message(self, text: str, diff --git a/je_auto_control/utils/executor/action_executor.py b/je_auto_control/utils/executor/action_executor.py index 254688178..0ad0568ea 100644 --- a/je_auto_control/utils/executor/action_executor.py +++ b/je_auto_control/utils/executor/action_executor.py @@ -32,7 +32,12 @@ from je_auto_control.utils.executor.flow_control import ( BLOCK_COMMANDS, LoopBreak, LoopContinue, MacroDepthExceeded, ) -from je_auto_control.utils.executor.action_redaction import describe_action, redact_actions +from je_auto_control.utils.executor.action_redaction import ( + describe_action, record_command, redact_actions, redact_result, +) +from je_auto_control.utils.executor.result_hook import ( + ResultHook, enter_list, leave_list, observe_results, report_result, +) from je_auto_control.utils.action_journal.recorder import step as _journal_step from je_auto_control.utils.executor.mouse_aliases import MOUSE_BUTTON_COMMANDS from je_auto_control.utils.executor.run_control import ( @@ -8457,6 +8462,7 @@ def execute_action(self, action_list: Union[list, dict], _validated: bool = False, dry_run: bool = False, step_callback: Optional[Callable[[list], None]] = None, + result_callback: Optional[ResultHook] = None, ) -> Dict[str, str]: """ 執行 action list @@ -8467,8 +8473,15 @@ def execute_action(self, action_list: Union[list, dict], :param _validated: 內部用;子呼叫已驗證過時避免重複驗證 :param dry_run: 若為 True,只記錄將執行的動作,不實際呼叫。 :param step_callback: 每個 action 開始前呼叫此 hook(偵錯用)。 + :param result_callback: called with ``(command, arguments, result, path)`` after every + command that returned, nested ones included, for this run on this thread only + (:mod:`je_auto_control.utils.executor.result_hook`). :return: 執行紀錄字典 """ + if result_callback is not None: + with observe_results(result_callback): + return self.execute_action(action_list, raise_on_error, _validated, + dry_run, step_callback) autocontrol_logger.info(f"execute_action, action_list: {redact_actions(action_list)}") # A nested body inherits the strictness of the list that runs it, so # a failure inside an AC_loop / AC_if_* / macro under raise_on_error @@ -8499,28 +8512,39 @@ def _execute_list(self, action_list: Union[list, dict], raise_on_error: bool, validate_actions(action_list, self.known_commands(), self._self_loadable()) execute_record_dict: Dict[str, Any] = {} - for action in action_list: - checkpoint() # a stop requested for this run ends it between actions - if step_callback is not None: - step_callback(action) - if dry_run: - key = _unique_key(execute_record_dict, "dry-run: " + describe_action(action)) - execute_record_dict[key] = "(not executed)" - continue - key = _unique_key(execute_record_dict, "execute: " + describe_action(action)) - try: - self._run_one_action(action, execute_record_dict, raise_on_error, key) - except (LoopBreak, LoopContinue, MacroDepthExceeded) as signal: - if _validated: - raise # a nested body: the enclosing block handles it - self._record_unwound_signal( - signal, execute_record_dict, raise_on_error, key) + frame = enter_list() # None unless a result hook watches this run + try: + for position, action in enumerate(action_list, start=1): + checkpoint() # a stop requested for this run ends it between actions + if frame is not None: + frame.advance(position, action) + if step_callback is not None: + step_callback(action) + self._run_listed_action(action, execute_record_dict, + (raise_on_error, _validated, dry_run)) + finally: + leave_list(frame) for key, value in execute_record_dict.items(): # Masked by field name: a minted token (AC_jwt_encode, a lease) is a result, not an argument. - autocontrol_logger.info("%s -> %s", key, redact_actions(value)) + autocontrol_logger.info("%s -> %s", key, redact_result(record_command(key), value)) return execute_record_dict + def _run_listed_action(self, action: list, record: Dict[str, Any], + modes: Tuple[bool, bool, bool]) -> None: + """Run one action of a list under ``modes`` (raise_on_error, _validated, dry_run).""" + raise_on_error, _validated, dry_run = modes + if dry_run: + record[_unique_key(record, "dry-run: " + describe_action(action))] = "(not executed)" + return + key = _unique_key(record, "execute: " + describe_action(action)) + try: + self._run_one_action(action, record, raise_on_error, key) + except (LoopBreak, LoopContinue, MacroDepthExceeded) as signal: + if _validated: + raise # a nested body: the enclosing block handles it + self._record_unwound_signal(signal, record, raise_on_error, key) + @staticmethod def _record_unwound_signal(signal: Exception, record: Dict[str, Any], raise_on_error: bool, key: str) -> None: @@ -8570,6 +8594,7 @@ def _run_one_action(self, action: list, record: Dict[str, Any], with default_profiler.measure(action_name), _journal_step(action) as step: record[key] = self._execute_event(action) step.outcome(record[key]) + report_result(action, record[key]) # a no-op unless a result hook watches this run _observe_executor_metrics(action_name, started, error=None) except (LoopBreak, LoopContinue, ExecutionStopped): raise # a stop is never recorded-and-continued, whatever raise_on_error says @@ -8681,17 +8706,21 @@ def add_command_to_executor(command_dict: dict) -> None: def execute_action(action_list: Union[list, dict], *, raise_on_error: bool = False, dry_run: bool = False, step_callback: Optional[Callable[[list], None]] = None, + result_callback: Optional[ResultHook] = None, ) -> Dict[str, str]: """Run ``action_list`` on the shared executor; return its execution record. The keywords are the ones :meth:`Executor.execute_action` takes: ``raise_on_error`` raises at the first failed action instead of recording - it, ``dry_run`` lists what would run without calling anything, and - ``step_callback`` is called with each action before it starts. + it, ``dry_run`` lists what would run without calling anything, + ``step_callback`` is called with each top-level action before it starts, + and ``result_callback`` with ``(command, arguments, result, path)`` after + every command that returned, the ones nested in a block included + (:mod:`je_auto_control.utils.executor.result_hook`). """ return executor.execute_action( action_list, raise_on_error=raise_on_error, dry_run=dry_run, - step_callback=step_callback) + step_callback=step_callback, result_callback=result_callback) def execute_files(execute_files_list: list) -> List[Dict[str, str]]: diff --git a/je_auto_control/utils/executor/action_redaction.py b/je_auto_control/utils/executor/action_redaction.py index 5a3346509..d2e941a13 100644 --- a/je_auto_control/utils/executor/action_redaction.py +++ b/je_auto_control/utils/executor/action_redaction.py @@ -11,8 +11,30 @@ socket server, run history) in plain text. The walk is recursive because a command nested in a block (``AC_loop``, ``AC_if``...) is logged with its parent. + +A *result* is masked by :func:`redact_result`, the one rule for what a +command answered: a field whose name is in :data:`SENSITIVE_ARGUMENT_NAMES` +is masked wherever it sits in the result, whatever the command. The executor +uses it for its result log and the Script Builder for its result pane, and +:func:`masked_fields` lists exactly what it masked, which is what the builder +offers once. + +Generated secret or identifier. A value is a secret when holding it is enough +to act: a bearer token (``AC_user_add``, ``AC_user_rotate_token``, +``AC_rest_api_start`` / ``AC_rest_api_status``), a signed JWT +(``AC_jwt_encode``), a lease token (``AC_lease_secret``, ``AC_lease_active``). +It is an identifier when the person who ran the command has to read it to go +on and it names a thing rather than opening it. :data:`RESULT_IDENTIFIERS` +lists those, per command and field, and it is the only exemption: +``AC_approval_request`` answers ``{"token": ...}``, the request id the maker +hands to the checker. The exemption is honoured only where a person is +reading their own run (``show_identifiers=True``, the builder's result pane); +the log masks an identifier like any other ``token``. """ -from typing import Any, FrozenSet +import re +from dataclasses import dataclass, field +from types import MappingProxyType +from typing import Any, FrozenSet, List, Mapping, Optional, Tuple, Union _VAULT_COMMAND_PREFIX = "AC_secret_" #: Commands outside the vault whose every argument is a secret. @@ -56,6 +78,94 @@ def redact_actions(value: Any) -> Any: return [redact_actions(item) for item in value] +#: Result fields with a secret-looking name that are identifiers: command -> field names. +RESULT_IDENTIFIERS: Mapping[str, FrozenSet[str]] = MappingProxyType({ + # The request id the maker gives the checker, who must be somebody else. + "AC_approval_request": frozenset({"token"}), +}) +_NO_FIELDS: FrozenSet[str] = frozenset() +# A record key is "execute: " + str(action); the command is its first element. +_RECORD_COMMAND = re.compile(r"^(?:execute|dry-run): \['(AC_\w+)'") + +FieldPath = Tuple[Union[str, int], ...] + + +@dataclass(frozen=True) +class MaskedField: + """One value :func:`redact_result` masked: where it sits in the result, and the value. + + ``path`` is the keys and list indexes that lead to it. ``value`` is left + out of the text form, so formatting or logging the object never prints it. + """ + path: FieldPath + value: Any = field(repr=False) + + +def record_command(key: Any) -> Optional[str]: + """The command an execution-record key was written for, or ``None``.""" + match = _RECORD_COMMAND.match(key) if isinstance(key, str) else None + return match.group(1) if match is not None else None + + +def redact_result(command: Optional[str], result: Any, *, show_identifiers: bool = False, + found: Optional[List[MaskedField]] = None) -> Any: + """A copy of ``result``, what ``command`` answered, with every secret-named field masked. + + At any depth, for any command. A field holding ``None`` or ``""`` has + nothing to hide and is kept. An execution record nested in the result is + masked per command, and an action list in it as :func:`redact_actions` + masks one. ``show_identifiers`` keeps the fields :data:`RESULT_IDENTIFIERS` + lists for the command readable. Each value masked is appended to ``found`` + when one is given. ``result`` is not changed. + """ + visible = RESULT_IDENTIFIERS.get(command or "", _NO_FIELDS) if show_identifiers else _NO_FIELDS + return _redact_value(result, visible, (), _Walk(show_identifiers, found)) + + +def masked_fields(command: Optional[str], result: Any, *, + show_identifiers: bool = False) -> List[MaskedField]: + """Exactly the values :func:`redact_result` masks in ``result``, in order.""" + found: List[MaskedField] = [] + redact_result(command, result, show_identifiers=show_identifiers, found=found) + return found + + +class _Walk: + """What one :func:`redact_result` call carries down the result.""" + + __slots__ = ("show_identifiers", "found") + + def __init__(self, show_identifiers: bool, found: Optional[List[MaskedField]]) -> None: + self.show_identifiers = show_identifiers + self.found = found + + +def _redact_value(value: Any, visible: FrozenSet[str], path: FieldPath, walk: _Walk) -> Any: + if isinstance(value, dict): + return {key: _redact_field(key, item, visible, path + (key,), walk) + for key, item in value.items()} + if not isinstance(value, (list, tuple)): + return value + if isinstance(value, list) and value and isinstance(value[0], str) and value[0].startswith("AC_"): + return redact_actions(value) # an action: its arguments, which are not result fields + return [_redact_value(item, _NO_FIELDS, path + (index,), walk) + for index, item in enumerate(value)] + + +def _redact_field(key: Any, item: Any, visible: FrozenSet[str], path: FieldPath, walk: _Walk) -> Any: + nested = record_command(key) + if nested is not None: # a record inside the result: its own command's rule applies + shown = RESULT_IDENTIFIERS.get(nested, _NO_FIELDS) if walk.show_identifiers else _NO_FIELDS + return _redact_value(item, shown, path, walk) + name = str(key).lower() + empty = item is None or (isinstance(item, str) and not item) + if name not in SENSITIVE_ARGUMENT_NAMES or name in visible or empty: + return _redact_value(item, _NO_FIELDS, path, walk) + if walk.found is not None: + walk.found.append(MaskedField(path, item)) + return _MASK + + def is_sensitive_argument(command: str, name: str) -> bool: """Whether argument ``name`` of ``command`` holds a secret.""" lowered = str(name).lower() diff --git a/je_auto_control/utils/executor/result_hook.py b/je_auto_control/utils/executor/result_hook.py new file mode 100644 index 000000000..c0db63801 --- /dev/null +++ b/je_auto_control/utils/executor/result_hook.py @@ -0,0 +1,150 @@ +"""An opt-in hook that is told the result of every command one run executes. + +A block command (``AC_loop``, ``AC_try``, ``AC_if_*``...) records its own +summary, so the result of a command in its body is in no execution record. +Whoever needs those results -- the Script Builder, to show a token issued +inside a loop once -- registers a callback for the run:: + + with observe_results(callback): + executor.execute_action(actions) + +or ``execute_action(actions, result_callback=callback)``. The callback is +called after each command that returned, nested ones included, with +``(command, arguments, result, path)``: + +* ``arguments`` is the action's second element as written (``None`` when it + has none): placeholders such as ``${secrets.db}`` are not expanded. +* ``result`` is the object the command returned -- the same object the record + holds, so the callback must not change it. +* ``path`` says where the step is: one :class:`StepPosition` per enclosing + action list, outermost first. + +A command that failed has no result and is not reported; neither is a +``dry_run``. The hook belongs to the thread that registered it and ends with +the ``with`` block: a thread started during the run (``AC_parallel``, a +scheduler) does not inherit it, and its commands are not reported. Nothing +about the run changes: what it returns, records and logs is the same with and +without a hook, and a callback that raises is logged by its exception type and +otherwise ignored. With no hook registered the executor pays one lookup per +action list and one per command. + +``step_callback`` is a different contract and stays as it is: it is called +*before* each action of the list it was passed with, with that action alone, +and is not handed down to nested bodies -- its callers count top-level steps. +""" +from contextlib import contextmanager +from typing import Any, Callable, Iterator, List, NamedTuple, Optional, Tuple + +from je_auto_control.utils.exception.exceptions import AutoControlActionException +from je_auto_control.utils.logging.logging_instance import autocontrol_logger +from je_auto_control.utils.thread_bound import ThreadBoundVar + + +class StepPosition(NamedTuple): + """Where a step is inside one action list. + + ``run`` counts the action lists the enclosing step has started so far (the + iteration of a loop, the attempt of a retry; 1 for a body that runs once), + ``position`` is the 1-based index of the step in its list and ``command`` + its name. + """ + run: int + position: int + command: str + + +StepPath = Tuple[StepPosition, ...] +ResultHook = Callable[[str, Any, Any, StepPath], None] + + +class _Frame: + """The action list being run: which of its steps is running, and how many lists that step started.""" + + __slots__ = ("run", "position", "command", "started") + + def __init__(self, run: int) -> None: + self.run = run + self.position = 0 + self.command = "" + self.started = 0 + + def advance(self, position: int, action: Any) -> None: + """Note that step ``position`` of this list is about to run.""" + self.position = position + self.command = _command_of(action) + self.started = 0 + + +class _Observation: + """One registered callback and the stack of action lists running under it.""" + + __slots__ = ("callback", "frames", "started") + + def __init__(self, callback: ResultHook) -> None: + self.callback = callback + self.frames: List[_Frame] = [] + self.started = 0 + + +_ACTIVE: "ThreadBoundVar[Optional[_Observation]]" = ThreadBoundVar("autocontrol_result_hook", None) + + +def _command_of(action: Any) -> str: + if isinstance(action, list) and action and isinstance(action[0], str): + return action[0] + return "" + + +@contextmanager +def observe_results(callback: ResultHook) -> Iterator[None]: + """Report every command result of the runs inside the block to ``callback``. + + For the calling thread only, and only until the block ends. Raises + ``AutoControlActionException`` when ``callback`` cannot be called. + """ + if not callable(callback): + raise AutoControlActionException("result_callback must be callable") + token = _ACTIVE.set(_Observation(callback)) + try: + yield + finally: + _ACTIVE.reset(token) + + +def enter_list() -> Optional[_Frame]: + """Executor: an action list starts; ``None`` when no hook is registered.""" + observation = _ACTIVE.get() + if observation is None: + return None + parent = observation.frames[-1] if observation.frames else observation + parent.started += 1 + frame = _Frame(parent.started) + observation.frames.append(frame) + return frame + + +def leave_list(frame: Optional[_Frame]) -> None: + """Executor: the action list of ``frame`` ended, however it ended.""" + if frame is None: + return + observation = _ACTIVE.get() + if observation is not None and frame in observation.frames: + del observation.frames[observation.frames.index(frame):] + + +def report_result(action: Any, result: Any) -> None: + """Executor: ``action`` returned ``result``; tell the hook, if there is one.""" + observation = _ACTIVE.get() + if observation is None or not observation.frames: + return + path = tuple(StepPosition(frame.run, frame.position, frame.command) + for frame in observation.frames) + arguments = action[1] if isinstance(action, list) and len(action) > 1 else None + try: + observation.callback(_command_of(action), arguments, result, path) + except Exception as error: # noqa: BLE001 # reason: an observer must not fail the run it watches + # The type only: the message could quote the result it was handed. + autocontrol_logger.warning("result hook raised %s; ignored", type(error).__name__) + + +__all__ = ["ResultHook", "StepPath", "StepPosition", "observe_results"] diff --git a/je_auto_control/utils/mcp_server/_authz.py b/je_auto_control/utils/mcp_server/_authz.py index 2ff4c145a..a3da48212 100644 --- a/je_auto_control/utils/mcp_server/_authz.py +++ b/je_auto_control/utils/mcp_server/_authz.py @@ -5,7 +5,9 @@ store is configured -- the caller's identity. :func:`visible_tools` and :func:`authorize_tool_call` are the dispatcher's: ``tools/list`` shows only what the caller may call, and ``tools/call`` refuses the rest, so the two -answers cannot disagree. +answers cannot disagree. :func:`visible_resources` and +:func:`authorize_resource` do the same for ``resources/list``, +``resources/read`` and ``resources/subscribe``. Without a user store nothing here identifies anyone: the transport keeps its optional shared token and the dispatcher offers every tool, as before. @@ -103,6 +105,43 @@ def authorize_tool_call(tool: MCPTool, arguments: Dict[str, Any], raise _MCPError(FORBIDDEN_CODE, message, {"required_capability": needed}) +def resource_capability(uri: str) -> str: + """The capability reading resource ``uri`` needs. + + The same rule as for tools: what observes the live host (the command + catalogue, the screen feed) needs ``read_screen``; stored records and + files (run history, action files) need ``read_data``. A URI this does not + recognise is treated as stored data, so a provider added later is closed + to a viewer until it is classified here. + """ + _scheme, _sep, path = uri.partition("://") + if path == "commands" or path == "screen" or path.startswith("screen/"): + return Capability.READ_SCREEN + return Capability.READ_DATA + + +def visible_resources(resources: List[Any]) -> List[Any]: + """The resources the current caller may read; all of them outside an RBAC scope.""" + caller = current_authorization() + if caller is None: + return resources + return [resource for resource in resources if caller.allows(resource_capability(resource.uri))] + + +def authorize_resource(uri: str, audit: AuditLogger, method: str = "resources/read") -> None: + """Refuse, and record, a resource the current caller's role does not grant.""" + caller = current_authorization() + needed = resource_capability(uri) + if caller is None or caller.allows(needed): + return + message = (f"Forbidden: resource {uri!r} needs the {needed!r} capability; " + f"role {caller.role!r} does not grant it") + audit.record(tool=method, arguments={"uri": uri}, status="denied", + duration_seconds=0.0, error_text=message) + raise _MCPError(FORBIDDEN_CODE, message, {"required_capability": needed}) + + __all__ = [ - "FORBIDDEN_CODE", "authorize_tool_call", "check_bearer", "visible_tools", + "FORBIDDEN_CODE", "authorize_resource", "authorize_tool_call", "check_bearer", + "resource_capability", "visible_resources", "visible_tools", ] diff --git a/je_auto_control/utils/mcp_server/_subscriptions.py b/je_auto_control/utils/mcp_server/_subscriptions.py index 4997adeea..32c4f9e99 100644 --- a/je_auto_control/utils/mcp_server/_subscriptions.py +++ b/je_auto_control/utils/mcp_server/_subscriptions.py @@ -21,6 +21,7 @@ from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Tuple from je_auto_control.utils.logging.logging_instance import autocontrol_logger +from je_auto_control.utils.mcp_server._authz import authorize_resource from je_auto_control.utils.mcp_server._protocol import ( _is_hashable, _MCPError, _notification_message, _result_response, ) @@ -108,6 +109,7 @@ class SubscriptionMixin: if TYPE_CHECKING: _resources: Any + _audit: Any _resource_subscriptions: Dict[str, Any] _subscriptions_lock: threading.Lock _listeners: Dict[Any, Listener] @@ -243,6 +245,8 @@ def _handle_resources_subscribe(self, uri = params.get("uri") if not isinstance(uri, str) or not uri: raise _MCPError(-32602, "resources/subscribe requires 'uri'") + # A subscription delivers the resource's updates: the same right as reading it. + authorize_resource(uri, self._audit, "resources/subscribe") # Hold the lock across the check *and* the subscribe so two concurrent # requests for the same uri cannot both create a provider handle and # leak the loser (a TOCTOU that left an orphaned subscription running). diff --git a/je_auto_control/utils/mcp_server/server.py b/je_auto_control/utils/mcp_server/server.py index 45fb01f8a..9ec7cb814 100644 --- a/je_auto_control/utils/mcp_server/server.py +++ b/je_auto_control/utils/mcp_server/server.py @@ -17,7 +17,9 @@ from je_auto_control.utils.cli_output import utf8_stream from je_auto_control.utils.exception.exceptions import AutoControlException from je_auto_control.utils.logging.logging_instance import autocontrol_logger -from je_auto_control.utils.mcp_server._authz import authorize_tool_call +from je_auto_control.utils.mcp_server._authz import ( + authorize_resource, authorize_tool_call, visible_resources, +) from je_auto_control.utils.mcp_server._connection_state import ConnectionStateMixin from je_auto_control.utils.mcp_server._core_tools import build_core_tools from je_auto_control.utils.mcp_server.audit import AuditLogger @@ -440,7 +442,7 @@ def _handle_ping(self) -> Dict[str, Any]: def _handle_resources_list(self) -> Dict[str, Any]: """List descriptors for every registered resource.""" return {"resources": [resource.to_descriptor() - for resource in self._resources.list()]} + for resource in visible_resources(self._resources.list())]} def _handle_prompts_list(self) -> Dict[str, Any]: """List descriptors for every registered prompt.""" @@ -490,6 +492,7 @@ def _handle_resources_read(self, uri = params.get("uri") if not isinstance(uri, str) or not uri: raise _MCPError(-32602, "resources/read requires string 'uri'") + authorize_resource(uri, self._audit) # RBAC; a no-op without a user content = self._resources.read(uri) if content is None: raise _MCPError(-32602, f"Unknown resource: {uri}") diff --git a/je_auto_control/utils/rbac/policy.py b/je_auto_control/utils/rbac/policy.py index 1f214d550..30b93fb25 100644 --- a/je_auto_control/utils/rbac/policy.py +++ b/je_auto_control/utils/rbac/policy.py @@ -4,7 +4,9 @@ * **REST routes** are listed one by one. A route missing from the list needs ``manage_hosts``, so a route added without a decision is admin-only rather - than open. + than open. A ``GET`` route that returns records or configuration the host + keeps needs ``read_data`` like the MCP tool reading the same thing; one + that observes live state needs ``read_screen``. * **MCP tools** follow their own annotation -- a tool marked read-only needs ``read_screen``, any other ``drive_input`` -- except the ones named here: those that change how the host itself is exposed, those that manage users, @@ -41,7 +43,7 @@ REST_ROUTE_CAPABILITIES: Dict[Tuple[str, str], str] = { ("GET", "/metrics"): _READ, ("GET", "/jobs"): _READ, - ("GET", "/history"): _READ, + ("GET", "/history"): _DATA, # run history, as ac_list_run_history ("GET", "/screenshot"): _READ, ("GET", "/mouse_position"): _READ, ("GET", "/screen_size"): _READ, @@ -53,7 +55,7 @@ ("GET", "/usb/devices"): _READ, ("GET", "/usb/events"): _READ, ("GET", "/usb/passthrough/status"): _READ, - ("GET", "/usb/acl"): _READ, + ("GET", "/usb/acl"): _DATA, # the stored ACL, as ac_usb_acl_list ("GET", "/usb/loopback/devices"): _READ, ("GET", "/usb/remote/devices"): _READ, ("GET", "/diagnose"): _READ, @@ -72,6 +74,12 @@ ("POST", "/usb/acl/default"): _HOSTS, } +#: The routes that need ``read_data``: the ones returning records or +#: configuration the host keeps, by the rule :data:`DATA_TOOLS` states. Each +#: has an MCP tool reading the same thing, and the two must agree. +DATA_ROUTES = frozenset( + route for route, needed in REST_ROUTE_CAPABILITIES.items() if needed == _DATA) + #: Commands that need more than ``drive_input``. Every other command is #: covered by the capability of the surface that submitted the action list. COMMAND_CAPABILITIES: Dict[str, str] = { @@ -267,7 +275,8 @@ def _leading_names(node: Any, depth: int) -> Iterator[str]: __all__ = [ - "COMMAND_CAPABILITIES", "DATA_TOOLS", "REST_ROUTE_CAPABILITIES", "TOOL_CAPABILITIES", + "COMMAND_CAPABILITIES", "DATA_ROUTES", "DATA_TOOLS", "REST_ROUTE_CAPABILITIES", + "TOOL_CAPABILITIES", "authorize_command", "capability_for_command", "capability_for_route", "capability_for_tool", "denied_command_in", ] diff --git a/je_auto_control/utils/rest_api/dashboard/app.js b/je_auto_control/utils/rest_api/dashboard/app.js index f1b71729f..b3427eb2b 100644 --- a/je_auto_control/utils/rest_api/dashboard/app.js +++ b/je_auto_control/utils/rest_api/dashboard/app.js @@ -40,12 +40,32 @@ async function fetchJson(path) { const resp = await fetch(path, { headers: { Authorization: `Bearer ${token}` }, }); + if (resp.status === 403) { + throw new Error(await forbiddenMessage(resp, path)); + } if (!resp.ok) { throw new Error(`HTTP ${resp.status} on ${path}`); } return resp.json(); } +// A 403 is the server saying this token's role may not read the panel, not +// that the panel is broken: say which capability is missing. The body is +// {"error": "forbidden", "required_capability": "...", "role": "..."}. +async function forbiddenMessage(resp, path) { + let body = null; + try { + body = await resp.json(); + } catch (_error) { + body = null; + } + if (!body || !body.required_capability) { + return `HTTP 403 on ${path}`; + } + const role = body.role ? `role '${body.role}'` : "this token"; + return `not available to ${role}: needs the '${body.required_capability}' capability`; +} + function panelEl(name) { return document.querySelector(`section[data-panel="${name}"]`); } diff --git a/pyproject.toml b/pyproject.toml index cec988b00..4f527536e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -442,6 +442,7 @@ module = [ "je_auto_control.gui._tab_task", "je_auto_control.gui.remote_desktop.connection_recent", "je_auto_control.utils.executor.run_control", + "je_auto_control.utils.executor.result_hook", "je_auto_control.utils.thread_bound", "je_auto_control.gui._dispose", "je_auto_control.gui._slow_op", diff --git a/test/unit_test/headless/test_chatops_slack_background_run.py b/test/unit_test/headless/test_chatops_slack_background_run.py new file mode 100644 index 000000000..6de3a90bb --- /dev/null +++ b/test/unit_test/headless/test_chatops_slack_background_run.py @@ -0,0 +1,382 @@ +"""A ``/stop`` posted to Slack is read while the bot's own ``/run`` is going. + +``SlackBot.poll_once`` dispatched every command on the poll thread, so the +``/stop`` meant for a run was fetched only after that run had ended. ``/run`` +now executes on a worker thread, one at a time per bot, and the poll loop keeps +reading. + +Slack is a fake held in memory -- no network. The scripts run fake commands +that wait on the stop-aware ``pause``: no input, no screen. Every wait is +bounded and none is asserted to be short. +""" +import json +import threading + +import pytest + +from je_auto_control.utils.chatops import ( + CHATOPS_RUN_PREFIX, CommandResult, CommandRouter, SlackBot, SlackError, + register_chatops_default_commands, +) +from je_auto_control.utils.executor.action_executor import executor +from je_auto_control.utils.executor.run_control import ( + active_executions, pause, stop_execution, +) + +_WAIT = 10.0 +_LONG = 120.0 +_WORKER_PREFIX = "chatops-slack-" + + +class _FakeSlack: + """The two Slack calls the bot makes, answered from memory.""" + + def __init__(self): + self._changed = threading.Condition() + self._pending = [] + self.posts = [] + self.post_threads = [] + self.fail_posts_from_workers = False + + def say(self, ts, text, user="U1"): + with self._changed: + self._pending.append({"ts": str(ts), "user": user, "text": text}) + + def api_get(self, method, _params): + if method == "auth.test": + return {"ok": True, "user_id": "U_BOT"} + with self._changed: + batch = list(reversed(self._pending)) # Slack answers newest first + self._pending.clear() + return {"ok": True, "messages": batch} + + def api_post(self, _method, payload): + thread = threading.current_thread() + if self.fail_posts_from_workers and thread.name.startswith(_WORKER_PREFIX): + raise SlackError("reply failed") + with self._changed: + self.posts.append(payload) + self.post_threads.append(thread) + self._changed.notify_all() + return {"ok": True} + + def wait_for_posts(self, count): + with self._changed: + return self._changed.wait_for(lambda: len(self.posts) >= count, _WAIT) + + def texts(self): + with self._changed: + return [post["text"] for post in self.posts] + + +class _Script: + """Fake commands: one that waits until it is stopped, one that only marks.""" + + def __init__(self): + self.entered = threading.Event() + self.threads = [] + self.after = [] + + def held(self): + self.threads.append(threading.current_thread()) + self.entered.set() + pause(_LONG) + + def mark(self): + self.threads.append(threading.current_thread()) + self.after.append("ran") + + +def _workers(): + return [thread for thread in threading.enumerate() + if thread.name.startswith(_WORKER_PREFIX) and thread.is_alive()] + + +@pytest.fixture() +def script(monkeypatch): + fake = _Script() + monkeypatch.setitem(executor.event_dict, "AC_fake_slack_held", fake.held) + monkeypatch.setitem(executor.event_dict, "AC_fake_slack_mark", fake.mark) + yield fake + stop_execution() + + +@pytest.fixture() +def root(tmp_path, monkeypatch): + (tmp_path / "long job.json").write_text( + json.dumps([["AC_fake_slack_held"], ["AC_fake_slack_mark"]]), encoding="utf-8") + (tmp_path / "quick.json").write_text(json.dumps([["AC_fake_slack_mark"]]), encoding="utf-8") + monkeypatch.setenv("JE_AUTOCONTROL_CHATOPS_SCRIPT_ROOT", str(tmp_path)) + return tmp_path + + +@pytest.fixture() +def slack(): + return _FakeSlack() + + +@pytest.fixture() +def bot(slack, script, root, monkeypatch): + """A bot with the default commands whose Slack calls go to the fake.""" + made = _make_bot(slack, monkeypatch, register_chatops_default_commands(CommandRouter())) + yield made + made.stop(timeout=_WAIT) + stop_execution() + for worker in _workers(): + worker.join(_WAIT) + assert _workers() == [], "a worker outlived its test" + assert active_executions() == [], "a chat run outlived its test" + + +def _make_bot(slack, monkeypatch, router, **options): + made = SlackBot(token="xoxb-fake", channel_id="C1", router=router, + poll_interval_s=1.0, **options) + monkeypatch.setattr(made, "_api_get", slack.api_get) + monkeypatch.setattr(made, "_api_post", slack.api_post) + return made + + +class _Poll: + """One ``poll_once`` on its own thread, so a poll that blocks fails instead of hanging.""" + + def __init__(self, bot): + self.count = None + self.error = None + self.thread = threading.Thread(target=self._run, args=(bot,), daemon=True) + self.thread.start() + self.thread.join(_WAIT) + assert not self.thread.is_alive(), "poll_once waited for the command it dispatched" + if self.error is not None: + raise self.error + + def _run(self, bot): + try: + self.count = bot.poll_once() + except SlackError as error: + self.error = error + + +def _start_long_run(bot, slack, script, ts=1): + slack.say(ts, '/run "long job.json"') + poll = _Poll(bot) + assert poll.count == 1 + assert script.entered.wait(_WAIT) + run_id = bot.running_run_id + assert run_id is not None and run_id.startswith(f"{CHATOPS_RUN_PREFIX}long_job-") + return poll, run_id + + +# --- the run is on another thread ------------------------------------------ + +def test_a_run_executes_on_a_worker_and_the_poll_returns(bot, slack, script): + poll, run_id = _start_long_run(bot, slack, script) + assert script.threads[0] is not poll.thread + assert script.threads[0].name == f"{_WORKER_PREFIX}{run_id}" + assert [run["run_id"] for run in active_executions()] == [run_id] + assert slack.texts() == [], "the reply comes when the run ends, not when it starts" + assert bot.last_seen_ts == "1" + + +def test_a_quick_run_replies_from_the_worker_in_the_message_thread(bot, slack, script): + slack.say(5, "/run quick.json") + assert _Poll(bot).count == 1 + assert slack.wait_for_posts(1) + assert slack.posts == [{"channel": "C1", "thread_ts": "5", + "text": "ran quick.json: 1 action(s) executed"}] + assert slack.post_threads[0].name.startswith(_WORKER_PREFIX) + assert script.after == ["ran"] + + +# --- /stop reaches it -------------------------------------------------------- + +def test_stop_arrives_while_the_run_is_blocked_and_ends_it(bot, slack, script): + _poll, run_id = _start_long_run(bot, slack, script) + slack.say(2, "/stop", user="U7") + stopping = _Poll(bot) + assert stopping.count == 1 + assert slack.wait_for_posts(2) + assert slack.posts == [ + {"channel": "C1", "thread_ts": "2", "text": f"stop requested: {run_id}"}, + {"channel": "C1", "thread_ts": "1", + "text": "run stopped. (stopped from chat by U7)"}, + ] + assert slack.post_threads[0] is stopping.thread, "/stop is answered on the poll thread" + assert slack.post_threads[1].name == f"{_WORKER_PREFIX}{run_id}" + assert script.after == [], "the action after the stop still ran" + assert bot.stop(timeout=_WAIT) is True + assert bot.running_run_id is None + + +def test_a_stop_in_the_same_batch_as_its_run_still_finds_it(bot, slack, script): + """``/run`` is registered before the next message is read, however the threads fall.""" + slack.say(1, '/run "long job.json"') + slack.say(2, "/stop") + assert _Poll(bot).count == 2 + assert slack.wait_for_posts(2) + texts = slack.texts() + assert texts[0].startswith(f"stop requested: {CHATOPS_RUN_PREFIX}long_job-") + assert texts[1] == "run stopped. (stopped from chat by U1)" + assert script.after == [] + + +def test_stop_by_id_and_stop_of_an_unknown_run_answer_at_once(bot, slack, script): + _poll, run_id = _start_long_run(bot, slack, script) + slack.say(2, "/stop nope") + _Poll(bot) + assert slack.texts() == [f"no run named 'nope' is in progress. " + f"Other runs in progress: {run_id} (/stop )."] + assert bot.running_run_id == run_id + slack.say(3, f"/stop {run_id}") + _Poll(bot) + assert slack.wait_for_posts(3) + assert slack.texts()[1:] == [f"stop requested: {run_id}", + "run stopped. (stopped from chat by U1)"] + + +# --- one run per bot --------------------------------------------------------- + +def test_a_second_run_is_refused_while_one_is_going(bot, slack, script): + _poll, run_id = _start_long_run(bot, slack, script) + slack.say(2, "/run quick.json") + refused = _Poll(bot) + assert refused.count == 1 + assert slack.posts == [{ + "channel": "C1", "thread_ts": "2", + "text": f"run: already running ({run_id}); wait for it to end or /stop it first.", + }] + assert slack.post_threads[0] is refused.thread + assert script.after == [], "the refused run executed" + assert len(_workers()) == 1 + assert bot.running_run_id == run_id + + +def test_a_run_is_accepted_again_once_the_first_has_ended(bot, slack, script): + _start_long_run(bot, slack, script) + slack.say(2, "/stop") + _Poll(bot) + assert slack.wait_for_posts(2) + assert bot.wait_idle(timeout=_WAIT) is True + assert bot.running_run_id is None + slack.say(3, "/run quick.json") + assert _Poll(bot).count == 1 + assert slack.wait_for_posts(3) + assert slack.texts()[2] == "ran quick.json: 1 action(s) executed" + assert script.after == ["ran"] + + +def test_wait_idle_does_not_stop_the_run(bot, slack, script): + _poll, run_id = _start_long_run(bot, slack, script) + assert bot.wait_idle(timeout=0.2) is False + assert bot.running_run_id == run_id and slack.texts() == [] + + +# --- the other commands stay where they were --------------------------------- + +def test_other_commands_are_answered_on_the_poll_thread_during_a_run(bot, slack, script): + _poll, run_id = _start_long_run(bot, slack, script) + slack.say(2, "/help") + slack.say(3, "/scripts") + asked = _Poll(bot) + assert asked.count == 2 + texts = slack.texts() + assert len(texts) == 2, "both replies were posted before poll_once returned" + assert texts[0].startswith("Available commands:") and texts[1].startswith("scripts under") + assert all(thread is asked.thread for thread in slack.post_threads) + assert bot.running_run_id == run_id + + +def test_a_command_that_cannot_be_parsed_is_still_answered(bot, slack): + slack.say(1, '/run "unclosed') + assert _Poll(bot).count == 0 + assert slack.texts()[0].startswith("router error: could not parse command") + assert _workers() == [] + + +def test_only_the_listed_commands_run_in_the_background(slack, script, root, monkeypatch): + seen = [] + router = CommandRouter() + + def deploy(_argv, _context): + seen.append(threading.current_thread()) + script.entered.set() + pause(_LONG) + return CommandResult(text="deployed") + + def ping(_argv, _context): + seen.append(threading.current_thread()) + return CommandResult(text="pong") + + router.register("deploy", deploy) + router.register("ping", ping) + register_chatops_default_commands(router) + made = _make_bot(slack, monkeypatch, router, background_commands=frozenset({"Deploy"})) + try: + slack.say(1, "/deploy site") + slack.say(2, "/ping") + poll = _Poll(made) + assert poll.count == 2 + assert script.entered.wait(_WAIT) + assert made.background_commands == frozenset({"deploy"}) + # The two handlers run on different threads, so their order is not fixed. + assert len(seen) == 2 and poll.thread in seen + assert [thread.name.startswith(_WORKER_PREFIX) for thread in seen].count(True) == 1 + assert (made.running_run_id or "").startswith(f"{CHATOPS_RUN_PREFIX}site-") + slack.say(3, "/stop") + _Poll(made) + assert slack.wait_for_posts(3) + assert slack.texts()[2] == "deploy stopped. (stopped from chat by U1)" + finally: + assert made.stop(timeout=_WAIT) is True + + +# --- shutdown ---------------------------------------------------------------- + +def test_stopping_the_bot_stops_the_run_and_joins_the_worker(bot, slack, script): + _poll, run_id = _start_long_run(bot, slack, script) + worker = script.threads[0] + assert bot.stop(timeout=_WAIT) is True + assert not worker.is_alive() + assert _workers() == [] + assert slack.posts == [{"channel": "C1", "thread_ts": "1", + "text": "run stopped. (the bot was stopped)"}] + assert script.after == [] + assert active_executions() == [] + assert bot.running_run_id is None + assert bot.stop(timeout=_WAIT) is True, "stopping twice is harmless" + + +def test_a_stopped_bot_refuses_a_run_instead_of_starting_a_worker(bot, slack, script): + bot.stop(timeout=_WAIT) + slack.say(1, "/run quick.json") + assert _Poll(bot).count == 1 + assert slack.texts() == ["run: the bot is stopping; not started."] + assert _workers() == [] and script.after == [] + + +def test_run_forever_keeps_reading_during_a_run_and_stop_ends_both(bot, slack, script): + slack.say(1, '/run "long job.json"') + loop = threading.Thread(target=bot.run_forever, daemon=True) + loop.start() + assert script.entered.wait(_WAIT) + slack.say(2, "/stop") + assert slack.wait_for_posts(2), "the loop did not read /stop while the run was going" + assert slack.texts()[1] == "run stopped. (stopped from chat by U1)" + assert bot.stop(timeout=_WAIT) is True + loop.join(_WAIT) + assert not loop.is_alive() + + +# --- a reply that cannot be posted ------------------------------------------- + +def test_a_failed_reply_from_the_worker_does_not_rerun_or_end_the_bot(bot, slack, script): + slack.fail_posts_from_workers = True + slack.say(1, "/run quick.json") + assert _Poll(bot).count == 1 + assert bot.wait_idle(timeout=_WAIT) is True + assert script.after == ["ran"], "the command ran exactly once" + assert slack.texts() == [] + slack.fail_posts_from_workers = False + slack.say(2, "/help") + assert _Poll(bot).count == 1 + assert bot.last_seen_ts == "2" + assert script.after == ["ran"] diff --git a/test/unit_test/headless/test_executor_result_hook.py b/test/unit_test/headless/test_executor_result_hook.py new file mode 100644 index 000000000..589941318 --- /dev/null +++ b/test/unit_test/headless/test_executor_result_hook.py @@ -0,0 +1,232 @@ +"""A caller can be told the result of every command one run executes, nested ones included. + +A block command records its own summary, so the result of a command in its +body is in no execution record. ``execute_action(..., result_callback=...)`` +/ ``observe_results`` report it, for one run on one thread, without changing +what the run returns or logs. + +Every command here is a fake registered on a private executor; nothing drives +an input device. +""" +import logging +import threading + +import pytest + +from je_auto_control.utils.exception.exceptions import AutoControlActionException +from je_auto_control.utils.executor import action_executor, result_hook +from je_auto_control.utils.executor.action_executor import Executor, execute_action +from je_auto_control.utils.executor.result_hook import StepPosition, observe_results +from je_auto_control.utils.logging.logging_instance import autocontrol_logger + + +@pytest.fixture() +def runner(): + """An executor with three fake commands: one answers, one fails, one hands back its argument.""" + executor = Executor() + executor.event_dict["AC_fake_issue"] = lambda name="n": {"token": f"issued-{name}"} + executor.event_dict["AC_fake_echo"] = lambda value=None: value + + def fail(): + raise AutoControlActionException("nope") + executor.event_dict["AC_fake_fail"] = fail + return executor + + +def _recorder(): + seen = [] + + def hook(command, arguments, result, path): + seen.append((command, arguments, result, path)) + return seen, hook + + +def test_a_top_level_command_is_reported_with_its_arguments_result_and_position(runner): + seen, hook = _recorder() + record = runner.execute_action( + [["AC_fake_echo", {"value": 1}], ["AC_fake_issue", {"name": "a"}], ["AC_fake_echo"]], + result_callback=hook) + assert seen == [ + ("AC_fake_echo", {"value": 1}, 1, (StepPosition(1, 1, "AC_fake_echo"),)), + ("AC_fake_issue", {"name": "a"}, {"token": "issued-a"}, (StepPosition(1, 2, "AC_fake_issue"),)), + ("AC_fake_echo", None, None, (StepPosition(1, 3, "AC_fake_echo"),)), + ] + assert seen[1][2] is list(record.values())[1], "the hook is handed the recorded object, not a copy" + + +def test_a_command_in_a_loop_is_reported_once_per_iteration_with_its_path(runner): + seen, hook = _recorder() + record = runner.execute_action([ + ["AC_fake_echo", {"value": 0}], + ["AC_loop", {"times": 3, "body": [["AC_fake_echo", {"value": 9}], ["AC_fake_issue"]]}], + ], result_callback=hook) + issued = [(result, path) for command, _arguments, result, path in seen if command == "AC_fake_issue"] + assert issued == [ + ({"token": "issued-n"}, (StepPosition(1, 2, "AC_loop"), StepPosition(run, 2, "AC_fake_issue"))) + for run in (1, 2, 3)] + assert seen[-1][0] == "AC_loop" and seen[-1][3] == (StepPosition(1, 2, "AC_loop"),), \ + "the block itself is reported last, at its own position" + assert "issued-n" not in repr(record), "the block still records only its summary" + + +def test_the_path_names_every_enclosing_block(runner): + seen, hook = _recorder() + runner.execute_action([["AC_loop", {"times": 2, "body": [ + ["AC_try", {"body": [["AC_fake_issue", {"name": "deep"}]], "catch": []}]]}]], + result_callback=hook) + paths = [path for command, _arguments, _result, path in seen if command == "AC_fake_issue"] + assert paths == [ + (StepPosition(1, 1, "AC_loop"), StepPosition(run, 1, "AC_try"), StepPosition(1, 1, "AC_fake_issue")) + for run in (1, 2)] + + +def test_a_failed_command_and_a_dry_run_report_nothing(runner): + seen, hook = _recorder() + record = runner.execute_action([["AC_fake_fail"], ["AC_fake_issue"]], result_callback=hook) + assert [entry[0] for entry in seen] == ["AC_fake_issue"] + assert seen[0][3] == (StepPosition(1, 2, "AC_fake_issue"),), "a failure still counts as a position" + assert "nope" in list(record.values())[0] + del seen[:] + runner.execute_action([["AC_fake_issue"]], dry_run=True, result_callback=hook) + assert seen == [] + + +def test_the_run_returns_and_logs_the_same_with_and_without_a_hook(runner, caplog): + actions = [["AC_fake_issue", {"name": "a"}], + ["AC_loop", {"times": 2, "body": [["AC_fake_issue", {"name": "b"}]]}], + ["AC_fake_fail"]] + + def run(**keywords): + caplog.clear() + autocontrol_logger.addHandler(caplog.handler) + try: + with caplog.at_level(logging.DEBUG, logger=autocontrol_logger.name): + record = runner.execute_action(actions, **keywords) + finally: + autocontrol_logger.removeHandler(caplog.handler) + return record, [entry.getMessage() for entry in caplog.records] + + plain_record, plain_log = run() + seen, hook = _recorder() + hooked_record, hooked_log = run(result_callback=hook) + assert hooked_record == plain_record and hooked_log == plain_log + assert len(seen) == 4 and "issued-" not in "\n".join(hooked_log) + + +def test_a_hook_that_raises_does_not_fail_the_run_and_only_its_type_is_logged(runner, caplog): + def hook(_command, _arguments, result, _path): + raise ValueError(f"saw {result}") + + autocontrol_logger.addHandler(caplog.handler) + try: + with caplog.at_level(logging.DEBUG, logger=autocontrol_logger.name): + record = runner.execute_action([["AC_fake_issue"], ["AC_fake_echo", {"value": 2}]], + raise_on_error=True, result_callback=hook) + finally: + autocontrol_logger.removeHandler(caplog.handler) + assert list(record.values()) == [{"token": "issued-n"}, 2] + logged = "\n".join(entry.getMessage() for entry in caplog.records) + assert "result hook raised ValueError" in logged + assert "issued-n" not in logged and "saw" not in logged + + +def test_the_hook_ends_with_its_run_and_with_its_block(runner): + seen, hook = _recorder() + runner.execute_action([["AC_fake_issue"]], result_callback=hook) + runner.execute_action([["AC_fake_issue"]]) + assert len(seen) == 1, "the next run of the same thread is not observed" + with observe_results(hook): + runner.execute_action([["AC_fake_issue"]]) + runner.execute_action([["AC_fake_issue"]]) + assert [entry[3] for entry in seen[1:]] == [ + (StepPosition(1, 1, "AC_fake_issue"),), (StepPosition(2, 1, "AC_fake_issue"),)], \ + "inside one block the second list is run 2" + runner.execute_action([["AC_fake_issue"]]) + assert len(seen) == 3 + assert result_hook.enter_list() is None, "nothing is left registered" + + +def test_the_hook_ends_when_the_run_raises(runner): + seen, hook = _recorder() + with pytest.raises(AutoControlActionException): + runner.execute_action([["AC_loop", {"times": 1, "body": [["AC_fake_fail"]]}]], + raise_on_error=True, result_callback=hook) + runner.execute_action([["AC_fake_issue"]]) + assert seen == [] and result_hook.enter_list() is None + + +def test_another_thread_and_a_thread_started_by_the_run_are_not_observed(runner): + seen, hook = _recorder() + started = threading.Event() + release = threading.Event() + other: list = [] + + def elsewhere(): + started.wait(10) + other.append(runner.execute_action([["AC_fake_issue", {"name": "other"}]])) + release.set() + + def spawn(value=None): + child = threading.Thread( + target=lambda: other.append(runner.execute_action([["AC_fake_issue", {"name": "child"}]]))) + child.start() + child.join(10) + started.set() + release.wait(10) + return value + + runner.event_dict["AC_fake_spawn"] = spawn + thread = threading.Thread(target=elsewhere) + thread.start() + runner.execute_action([["AC_fake_spawn", {"value": 1}]], result_callback=hook) + thread.join(10) + assert len(other) == 2, "both other threads really ran a command during the observed run" + assert [entry[0] for entry in seen] == ["AC_fake_spawn"] + + +def test_a_parallel_branch_runs_on_its_own_thread_and_is_not_reported(runner): + seen, hook = _recorder() + record = execute_action( + [["AC_parallel", {"branches": [[["AC_get_var", {"name": "missing", "default": 5}]]]}]], + result_callback=hook) + assert [entry[0] for entry in seen] == ["AC_parallel"] + assert seen[0][2] is list(record.values())[0], "what the branch answered is in the block's result" + + +def test_no_hook_means_no_frame_and_no_call(runner, monkeypatch): + def refuse(*_args, **_kwargs): + raise AssertionError("no path is built when nothing is registered") + monkeypatch.setattr(result_hook, "StepPosition", refuse) + assert result_hook.enter_list() is None + result_hook.leave_list(None) + result_hook.report_result(["AC_fake_issue"], {"token": "x"}) + assert list(runner.execute_action([["AC_fake_issue"]]).values()) == [{"token": "issued-n"}] + + +def test_a_callback_that_cannot_be_called_is_refused_before_anything_runs(runner): + ran: list = [] + runner.event_dict["AC_fake_note"] = lambda: ran.append(1) + with pytest.raises(AutoControlActionException): + runner.execute_action([["AC_fake_note"]], result_callback="not a function") + assert ran == [] + with pytest.raises(AutoControlActionException): + with observe_results(None): + ran.append(2) + assert ran == [] + + +def test_step_callback_keeps_its_contract_beside_the_result_hook(runner): + """Before each top-level action, with the action alone; not handed to nested bodies.""" + before: list = [] + seen, hook = _recorder() + actions = [["AC_loop", {"times": 2, "body": [["AC_fake_issue"]]}], ["AC_fake_echo"]] + runner.execute_action(actions, step_callback=before.append, result_callback=hook) + assert before == actions + assert [entry[0] for entry in seen] == ["AC_fake_issue", "AC_fake_issue", "AC_loop", "AC_fake_echo"] + + +def test_the_module_function_forwards_the_keyword(): + seen, hook = _recorder() + execute_action([["AC_get_var", {"name": "missing", "default": 3}]], result_callback=hook) + assert [(entry[0], entry[2]) for entry in seen] == [("AC_get_var", 3)] + assert "result_callback" in action_executor.execute_action.__kwdefaults__ diff --git a/test/unit_test/headless/test_mcp_resource_authorization.py b/test/unit_test/headless/test_mcp_resource_authorization.py new file mode 100644 index 000000000..3624ba173 --- /dev/null +++ b/test/unit_test/headless/test_mcp_resource_authorization.py @@ -0,0 +1,133 @@ +"""MCP resources are gated by role like the tools that return the same data. + +``resources/read`` was not checked against any capability: a viewer, who may +not call ``ac_list_run_history`` or ``ac_read_action_file``, read the same +rows at ``autocontrol://history`` and the same files under +``autocontrol://files/``. +""" +import json + +import pytest + +from je_auto_control.utils.mcp_server import _authz +from je_auto_control.utils.mcp_server.resources import MCPResource, ResourceProvider +from je_auto_control.utils.mcp_server.server import MCPServer +from je_auto_control.utils.rbac import AuthorizationContext, Role, authorization_scope +from je_auto_control.utils.rbac.policy import capability_for_tool +from je_auto_control.utils.rbac.users import Capability + +_HISTORY = "autocontrol://history" +_FILE = "autocontrol://files/flow.json" +_COMMANDS = "autocontrol://commands" +_SCREEN = "autocontrol://screen/live" +_ALL = (_HISTORY, _FILE, _COMMANDS, _SCREEN) + + +class _Provider(ResourceProvider): + """Every URI answers, and says which ones were read or subscribed to.""" + + def __init__(self): + self.read_uris, self.subscribed = [], [] + + def list(self): + return [MCPResource(uri=uri, name=uri, description="", mime_type="application/json") + for uri in _ALL] + + def read(self, uri): + self.read_uris.append(uri) + return {"uri": uri, "mimeType": "application/json", "text": "{}"} + + def subscribe(self, uri, on_update): + self.subscribed.append(uri) + return object() + + def unsubscribe(self, uri, handle): + """Nothing to release.""" + + +@pytest.fixture() +def provider(): + return _Provider() + + +def _ask(provider, method, params=None, role=None): + server = MCPServer(tools=[], resource_provider=provider) + message = json.dumps({"jsonrpc": "2.0", "id": 1, "method": method, "params": params or {}}) + if role is None: + return json.loads(server.handle_line(message)) + with authorization_scope(AuthorizationContext("someone", role)): + return json.loads(server.handle_line(message)) + + +@pytest.mark.parametrize("uri, capability", [ + (_HISTORY, Capability.READ_DATA), (_FILE, Capability.READ_DATA), + (_COMMANDS, Capability.READ_SCREEN), (_SCREEN, Capability.READ_SCREEN), + ("other://history", Capability.READ_DATA), ("other://commands", Capability.READ_SCREEN), + ("autocontrol://something-new", Capability.READ_DATA), ("no-scheme", Capability.READ_DATA), + ("autocontrol://screenshots/archive", Capability.READ_DATA), +]) +def test_what_each_resource_needs(uri, capability): + assert _authz.resource_capability(uri) == capability + + +def test_resources_need_what_the_tools_returning_the_same_data_need(): + assert _authz.resource_capability(_HISTORY) == capability_for_tool("ac_list_run_history", True) + assert _authz.resource_capability(_FILE) == capability_for_tool("ac_read_action_file", True) + assert _authz.resource_capability(_COMMANDS) == capability_for_tool("ac_list_action_commands", True) + + +@pytest.mark.parametrize("uri", [_HISTORY, _FILE]) +def test_a_viewer_cannot_read_stored_records(provider, uri): + answer = _ask(provider, "resources/read", {"uri": uri}, Role.VIEWER) + assert answer["error"]["code"] == _authz.FORBIDDEN_CODE + assert answer["error"]["data"] == {"required_capability": Capability.READ_DATA} + assert provider.read_uris == [], "refused before the provider is asked" + + +@pytest.mark.parametrize("uri", [_COMMANDS, _SCREEN]) +def test_a_viewer_still_reads_what_observes_the_host(provider, uri): + answer = _ask(provider, "resources/read", {"uri": uri}, Role.VIEWER) + assert answer["result"]["contents"][0]["uri"] == uri + + +@pytest.mark.parametrize("role", [Role.OPERATOR, Role.ADMIN]) +@pytest.mark.parametrize("uri", _ALL) +def test_roles_with_read_data_read_everything(provider, role, uri): + assert "result" in _ask(provider, "resources/read", {"uri": uri}, role) + + +@pytest.mark.parametrize("uri", _ALL) +def test_without_a_user_store_nothing_changes(provider, uri): + assert "result" in _ask(provider, "resources/read", {"uri": uri}) + listed = _ask(provider, "resources/list")["result"]["resources"] + assert [resource["uri"] for resource in listed] == list(_ALL) + + +def test_a_viewer_is_listed_only_what_it_may_read(provider): + listed = _ask(provider, "resources/list", role=Role.VIEWER)["result"]["resources"] + assert [resource["uri"] for resource in listed] == [_COMMANDS, _SCREEN] + + +def test_a_viewer_cannot_subscribe_to_what_it_cannot_read(provider): + answer = _ask(provider, "resources/subscribe", {"uri": _HISTORY}, Role.VIEWER) + assert answer["error"]["code"] == _authz.FORBIDDEN_CODE + assert provider.subscribed == [] + assert "result" in _ask(provider, "resources/subscribe", {"uri": _SCREEN}, Role.VIEWER) + assert provider.subscribed == [_SCREEN] + + +def test_a_refusal_is_recorded_in_the_audit_log(provider): + records = [] + + class _Audit: + def record(self, **entry): + records.append(entry) + + server = MCPServer(tools=[], resource_provider=provider) + server._audit = _Audit() + with authorization_scope(AuthorizationContext("vera", Role.VIEWER)): + server.handle_line(json.dumps({"jsonrpc": "2.0", "id": 1, "method": "resources/read", + "params": {"uri": _HISTORY}})) + assert len(records) == 1 + assert records[0]["status"] == "denied" and records[0]["arguments"] == {"uri": _HISTORY} + assert records[0]["tool"] == "resources/read" diff --git a/test/unit_test/headless/test_r3_net_chatops.py b/test/unit_test/headless/test_r3_net_chatops.py index 40c90089d..50bda0a8c 100644 --- a/test/unit_test/headless/test_r3_net_chatops.py +++ b/test/unit_test/headless/test_r3_net_chatops.py @@ -77,14 +77,17 @@ def do_run(_argv, _ctx): executed.append(1) return CommandResult(text="ran") - router.register("run", do_run) + # A command answered on the poll thread: /run itself now executes on a + # worker, whose failed reply is logged rather than raised here (covered + # by test_chatops_slack_background_run.py). + router.register("job", do_run) bot = SlackBot(token="xoxb-test", channel_id="C1", router=router) # Slack returns newest-first; only the older message ("1") is reached # before the reply fails. monkeypatch.setattr(bot, "_fetch_messages", lambda: [ - {"ts": "2", "text": "/run a", "user": "U1"}, - {"ts": "1", "text": "/run b", "user": "U1"}, + {"ts": "2", "text": "/job a", "user": "U1"}, + {"ts": "1", "text": "/job b", "user": "U1"}, ]) monkeypatch.setattr(bot, "_is_self", lambda msg: False) diff --git a/test/unit_test/headless/test_rest_read_data_routes.py b/test/unit_test/headless/test_rest_read_data_routes.py new file mode 100644 index 000000000..b6bf16ae6 --- /dev/null +++ b/test/unit_test/headless/test_rest_read_data_routes.py @@ -0,0 +1,358 @@ +"""REST routes that return stored records need ``read_data``, as the MCP tools do. + +A viewer token could read the run history and the USB ACL over REST after the +same data was closed to it over MCP. The capability of a route now follows the +MCP classification, and the pairing is not a hand-written list: a route and a +read-only tool are paired when their handlers read the same source in the +package, found by parsing both. + +Nothing here touches the screen, the mouse, the run-history database or the +USB ACL file: the two routes that would are replaced by recorders. +""" +import ast +import inspect +import json +import re +import textwrap +import urllib.error +import urllib.request +from pathlib import Path +from typing import Any, Callable, Dict, Iterable, Optional, Set, Tuple + +import pytest + +from je_auto_control.utils.mcp_server.tools import build_default_tool_registry +from je_auto_control.utils.rbac import ( + USERS_ENV, Capability, Role, UserStore, capability_for_route, capability_for_tool, +) +from je_auto_control.utils.rbac.policy import DATA_ROUTES, REST_ROUTE_CAPABILITIES +from je_auto_control.utils.rest_api import rest_server +from je_auto_control.utils.rest_api.rest_openapi import build_openapi_spec +from je_auto_control.utils.rest_api.rest_server import RestApiServer + +_SCHEME = "http" # NOSONAR localhost-only ephemeral test server; TLS out of scope + +_READ, _DATA, _DRIVE = Capability.READ_SCREEN, Capability.READ_DATA, Capability.DRIVE_INPUT +_AUDIT, _HOSTS = Capability.READ_AUDIT, Capability.MANAGE_HOSTS + +#: The whole table, written out: a change to any route has to be made here too. +_EXPECTED_ROUTES = { + ("GET", "/metrics"): _READ, + ("GET", "/jobs"): _READ, + ("GET", "/history"): _DATA, + ("GET", "/screenshot"): _READ, + ("GET", "/mouse_position"): _READ, + ("GET", "/screen_size"): _READ, + ("GET", "/windows"): _READ, + ("GET", "/sessions"): _READ, + ("GET", "/commands"): _READ, + ("GET", "/inspector/recent"): _READ, + ("GET", "/inspector/summary"): _READ, + ("GET", "/usb/devices"): _READ, + ("GET", "/usb/events"): _READ, + ("GET", "/usb/passthrough/status"): _READ, + ("GET", "/usb/acl"): _DATA, + ("GET", "/usb/loopback/devices"): _READ, + ("GET", "/usb/remote/devices"): _READ, + ("GET", "/diagnose"): _READ, + ("GET", "/openapi.json"): _READ, + ("GET", "/audit/list"): _AUDIT, + ("GET", "/audit/verify"): _AUDIT, + ("POST", "/execute"): _DRIVE, + ("POST", "/execute_file"): _DRIVE, + ("POST", "/usb/loopback/open"): _DRIVE, + ("POST", "/usb/remote/open"): _DRIVE, + ("POST", "/config/export"): _HOSTS, + ("POST", "/config/import"): _HOSTS, + ("POST", "/usb/passthrough/enable"): _HOSTS, + ("POST", "/usb/acl/add"): _HOSTS, + ("POST", "/usb/acl/remove"): _HOSTS, + ("POST", "/usb/acl/default"): _HOSTS, +} + + +@pytest.fixture(autouse=True) +def _no_ambient_rbac(monkeypatch): + """The developer's own environment must not switch RBAC on for these tests.""" + monkeypatch.delenv(USERS_ENV, raising=False) + + +@pytest.fixture() +def users(tmp_path): + """A store with one user per role; ``tokens`` maps the role to its token.""" + store = UserStore(tmp_path / "users.json") + store.tokens = { + role: store.add_user(user_id=f"{role}-user", display_name=role, role=role) + for role in Role.all() + } + return store + + +class _FakeAudit: + """Stands in for the tamper-evident audit log; keeps what it was given.""" + + def __init__(self): + self.rows = [] + + def log(self, event_type, **fields): + self.rows.append({"event_type": event_type, **fields}) + + +@pytest.fixture() +def rest(monkeypatch): + """Start REST servers on ephemeral ports whose data routes only record the call.""" + served = [] + + def recorder(name: str, payload: Dict[str, Any]) -> Callable[[Any], Tuple[int, Dict[str, Any]]]: + def handle(_ctx): + served.append(name) + return 200, payload + return handle + + monkeypatch.setitem(rest_server._GET_ROUTES, "/history", + recorder("/history", {"runs": [{"id": 7}]})) + monkeypatch.setitem(rest_server._GET_ROUTES, "/usb/acl", + recorder("/usb/acl", {"default": "deny", "rules": []})) + monkeypatch.setitem(rest_server._GET_ROUTES, "/jobs", recorder("/jobs", {"jobs": []})) + started = [] + + def start(**kwargs): + server = RestApiServer(host="127.0.0.1", port=0, enable_audit=False, **kwargs) + server._audit_log = _FakeAudit() + server.start() + server.served = served + started.append(server) + return server + + yield start + for server in started: + server.stop(timeout=10.0) + + +def _get(server, path: str, token: str): + """``(status, decoded JSON)`` of one GET; an error status is returned, not raised.""" + host, port = server.address + request = urllib.request.Request( + f"{_SCHEME}://{host}:{port}{path}", method="GET", + headers={"Authorization": f"Bearer {token}"}) + try: + with urllib.request.urlopen(request, timeout=10) as response: # nosec B310 # reason: loopback test server + return response.status, json.loads(response.read().decode("utf-8")) + except urllib.error.HTTPError as error: + return error.code, json.loads(error.read().decode("utf-8")) + + +# --- what a role may read ----------------------------------------------------- + +@pytest.mark.parametrize("path", ["/history", "/usb/acl"]) +def test_a_viewer_is_refused_a_stored_record_route(rest, users, path): + server = rest(user_store=users) + status, body = _get(server, path, users.tokens[Role.VIEWER]) + assert status == 403 + assert body == {"error": "forbidden", "role": Role.VIEWER, + "required_capability": Capability.READ_DATA} + assert server.served == [], "the handler must not run for a refused caller" + refusals = [row for row in server._audit_log.rows + if f"forbidden:{Capability.READ_DATA}" in json.dumps(row)] + assert refusals, "the refusal is in the audit log with the capability it lacked" + + +@pytest.mark.parametrize("role", [Role.OPERATOR, Role.ADMIN]) +@pytest.mark.parametrize("path", ["/history", "/usb/acl"]) +def test_an_operator_and_an_admin_still_read_them(rest, users, role, path): + server = rest(user_store=users) + status, _body = _get(server, path, users.tokens[role]) + assert status == 200 + assert server.served == [path] + + +def test_a_viewer_keeps_the_routes_that_observe_live_state(rest, users): + server = rest(user_store=users) + viewer = users.tokens[Role.VIEWER] + assert _get(server, "/jobs", viewer)[0] == 200 + assert _get(server, "/commands", viewer)[0] == 200 + assert _get(server, "/openapi.json", viewer)[0] == 200 + + +def test_the_shared_token_is_not_restricted(rest): + server = rest(token="shared-secret") + assert _get(server, "/history", "shared-secret")[0] == 200 + assert _get(server, "/usb/acl", "shared-secret")[0] == 200 + assert server.served == ["/history", "/usb/acl"] + + +# --- the table --------------------------------------------------------------- + +def test_the_route_table_is_the_one_documented(): + assert REST_ROUTE_CAPABILITIES == _EXPECTED_ROUTES + assert DATA_ROUTES == {route for route, needed in _EXPECTED_ROUTES.items() + if needed == _DATA} + for (method, path), needed in _EXPECTED_ROUTES.items(): + assert capability_for_route(method, path) == needed + + +def test_openapi_publishes_the_capability_of_the_data_routes(): + paths = build_openapi_spec()["paths"] + assert paths["/history"]["get"]["x-required-capability"] == Capability.READ_DATA + assert paths["/usb/acl"]["get"]["x-required-capability"] == Capability.READ_DATA + assert paths["/jobs"]["get"]["x-required-capability"] == Capability.READ_SCREEN + + +# --- the same data needs the same capability on both surfaces ------------------- +# +# A REST handler and an MCP tool handler are "the same data" when both read the +# same thing in the package: ``default_history_store.list_runs``, +# ``commands.acl_list`` ... Those are found by parsing each handler for what it +# uses of the names it imports from ``je_auto_control``, so a new route or tool +# is covered without being added to a list here. + +Source = Tuple[str, str, str] + +#: Imports every handler shares; reading the same logger is not reading the same data. +_PLUMBING = ("je_auto_control.utils.logging", "je_auto_control.utils.exception") + + +def _imported_names(nodes: Iterable[ast.AST]) -> Dict[str, Tuple[str, str]]: + """``local name -> (module, imported name)`` for every ``from ... import`` below ``nodes``.""" + found: Dict[str, Tuple[str, str]] = {} + for top in nodes: + for node in ast.walk(top): + if isinstance(node, ast.ImportFrom) and node.module: + for alias in node.names: + found[alias.asname or alias.name] = (node.module, alias.name) + return found + + +def _root_and_chain(node: ast.AST) -> Tuple[Optional[str], Tuple[str, ...]]: + """``(name, attributes)`` of ``name.a.b`` / ``name().a``; the name is ``None`` otherwise.""" + chain = [] + while True: + if isinstance(node, ast.Attribute): + chain.append(node.attr) + node = node.value + elif isinstance(node, ast.Call): + node = node.func + else: + break + name = node.id if isinstance(node, ast.Name) else None + return name, tuple(reversed(chain)) + + +def _sources(function: Callable[..., Any]) -> Set[Source]: + """What ``function`` reads from the package: ``(module, name, attribute path)``. + + Only the longest path per use is kept, so ``default_audit_log().query`` + is one source and does not also count as "anything from the audit log". + """ + try: + function = inspect.unwrap(function) + tree = ast.parse(textwrap.dedent(inspect.getsource(function))) + module_tree = ast.parse(inspect.getsource(inspect.getmodule(function))) + except (OSError, TypeError, SyntaxError): + return set() + names = _imported_names(module_tree.body) + names.update(_imported_names([tree])) + uses: Set[Tuple[str, Tuple[str, ...]]] = set() + for node in ast.walk(tree): + if isinstance(node, (ast.Attribute, ast.Name)): + name, chain = _root_and_chain(node) + if name in names and names[name][0].startswith("je_auto_control"): + uses.add((name, chain)) + longest = {(name, chain) for name, chain in uses + if not any(other_name == name and len(other) > len(chain) + and other[:len(chain)] == chain for other_name, other in uses)} + return {(*names[name], ".".join(chain)) for name, chain in longest + if not names[name][0].startswith(_PLUMBING)} + + +def _pairs() -> Dict[Tuple[str, str], Set[str]]: + """``(method, path) -> names of the MCP tools reading a source that route reads``. + + Only ``GET`` routes and read-only tools are paired: a tool that lists + windows and then focuses one reads what ``GET /windows`` reads without + being the same operation, and the question here is who may *read*. + """ + tools = build_default_tool_registry(read_only=False) + tool_sources = {tool.name: _sources(tool.handler) for tool in tools} + read_only = {tool.name: bool(tool.annotations.read_only) for tool in tools} + paired: Dict[Tuple[str, str], Set[str]] = {} + for path, handler in rest_server._GET_ROUTES.items(): + wanted = _sources(handler) + paired[("GET", path)] = {name for name, sources in tool_sources.items() + if read_only[name] and sources & wanted} + return paired + + +#: Pairs the parser has to keep finding. Without this floor a refactor that +#: hides the shared source (a wrapper, a string lookup) would leave the test +#: comparing nothing and passing. +_KNOWN_PAIRS = { + ("GET", "/history"): "ac_list_run_history", + ("GET", "/usb/acl"): "ac_usb_acl_list", + ("GET", "/jobs"): "ac_scheduler_list_jobs", + ("GET", "/windows"): "ac_list_windows", + ("GET", "/screen_size"): "ac_screen_size", + ("GET", "/mouse_position"): "ac_get_mouse_position", + ("GET", "/sessions"): "ac_remote_host_status", + ("GET", "/usb/passthrough/status"): "ac_usb_passthrough_status", + ("GET", "/usb/loopback/devices"): "ac_usb_loopback_list", + ("GET", "/usb/remote/devices"): "ac_usb_remote_list", +} + + +def test_the_pairing_still_finds_the_routes_known_to_share_a_source(): + paired = _pairs() + for route, tool in _KNOWN_PAIRS.items(): + assert tool in paired[route], f"{route} is no longer paired with {tool}" + + +def test_a_route_and_a_tool_serving_the_same_data_need_the_same_capability(): + disagreements = [] + for (method, path), tools in sorted(_pairs().items()): + if path in rest_server._PUBLIC_PATHS: + continue + route_needs = capability_for_route(method, path) + for tool in sorted(tools): + tool_needs = capability_for_tool(tool, True) + if tool_needs != route_needs: + disagreements.append( + f"{method} {path} needs {route_needs}, {tool} needs {tool_needs}") + assert disagreements == [] + + +def test_the_parser_reads_a_source_passed_as_a_reference(): + """``_usb_command(commands.acl_list)`` hands the function over without calling it.""" + sources = _sources(rest_server._GET_ROUTES["/usb/acl"]) + assert ("je_auto_control.utils.usb.passthrough", "commands", "acl_list") in sources + history = _sources(rest_server._GET_ROUTES["/history"]) + assert ("je_auto_control.utils.run_history.history_store", + "default_history_store", "list_runs") in history + assert not any(module.startswith(_PLUMBING) for module, _name, _path in history) + + +# --- the built-in dashboard --------------------------------------------------- + +def _dashboard_script() -> str: + path = Path(rest_server.__file__).parent / "dashboard" / "app.js" + return path.read_text(encoding="utf-8") + + +def test_the_dashboard_calls_no_route_that_needs_read_data(): + """The page is read with whatever token is pasted in, a viewer's included.""" + fetched = {match.split("?")[0] + for match in re.findall(r'fetchJson\("([^"]+)"\)', _dashboard_script())} + assert fetched, "the script no longer fetches with a literal path this test can read" + for path in fetched: + assert ("GET", path) in REST_ROUTE_CAPABILITIES, path + assert not {("GET", path) for path in fetched} & DATA_ROUTES + needs = {path: capability_for_route("GET", path) for path in fetched} + assert needs["/audit/list"] == Capability.READ_AUDIT, "the one panel a viewer is refused" + assert {needed for path, needed in needs.items() if path != "/audit/list"} == { + Capability.READ_SCREEN} + + +def test_the_dashboard_names_the_missing_capability_on_a_403(): + """Read from the source: there is no JavaScript runtime in the test environment.""" + script = _dashboard_script() + assert "resp.status === 403" in script + assert "required_capability" in script and "not available to" in script diff --git a/test/unit_test/headless/test_result_redaction.py b/test/unit_test/headless/test_result_redaction.py new file mode 100644 index 000000000..041afaf9b --- /dev/null +++ b/test/unit_test/headless/test_result_redaction.py @@ -0,0 +1,235 @@ +"""Secret-named fields of a command's result are masked at any depth, by one rule. + +The Script Builder masked only the top-level fields of the commands its schema +marks ``sensitive_result``: ``AC_lease_secret`` and ``AC_lease_active`` (lease +tokens, the latter nested under ``leases``) and ``AC_rest_api_start`` / +``AC_rest_api_status`` (the shared REST bearer token; no schema entry) were +printed in plain text. ``redact_result`` is now the rule for the executor's +result log and for the builder's result pane alike, and ``masked_fields`` +lists exactly what it masks -- which is what the builder offers once. + +An identifier with a secret-looking name stays readable where a person reads +their own run: ``AC_approval_request``'s ``token`` is the request id the maker +hands to the checker. + +No server is started: the REST registry is replaced by a fake. +""" +import copy +import json +import logging + +import pytest + +from je_auto_control.gui.script_builder.command_schema import COMMAND_SPECS +from je_auto_control.gui.script_builder.step_model import OneTimeCollector, displayable_record +from je_auto_control.utils.executor import action_executor +from je_auto_control.utils.executor.action_executor import execute_action +from je_auto_control.utils.executor.action_redaction import ( + RESULT_IDENTIFIERS, SENSITIVE_ARGUMENT_NAMES, MaskedField, masked_fields, record_command, + redact_actions, redact_result, +) +from je_auto_control.utils.logging.logging_instance import autocontrol_logger + +_BEARER = "shared-rest-bearer-0123456789" +_DEEP = { + "running": True, + "token": "t-top", + "leases": [{"token": "t-a", "name": "db"}, {"Token": "t-b", "name": "api"}], + "http": {"headers": {"Set-Cookie": "sid=1", "Content-Type": "text/plain"}}, + "pairs": ({"password": "p-1"}, {"note": "plain"}), + "empty": {"token": None, "secret": ""}, + "secret": {"whole": ["mapping", 1]}, +} + + +def _collected(actions: list): + collector = OneTimeCollector() + record = execute_action(actions, result_callback=collector.note) + return record, collector.take()[0] + + +def _logged_run(caplog, actions: list): + autocontrol_logger.addHandler(caplog.handler) + try: + with caplog.at_level(logging.DEBUG, logger=autocontrol_logger.name): + record = execute_action(actions) + finally: + autocontrol_logger.removeHandler(caplog.handler) + return record, "\n".join(entry.getMessage() for entry in caplog.records) + + +# --- the rule ---------------------------------------------------------------- + + +def test_a_secret_named_field_is_masked_at_any_depth_for_any_command(): + before = copy.deepcopy(_DEEP) + shown = redact_result("AC_anything", _DEEP) + assert _DEEP == before, "the result is not changed" + assert shown["token"] == "***" and shown["running"] is True + assert shown["leases"] == [{"token": "***", "name": "db"}, {"Token": "***", "name": "api"}] + assert shown["http"]["headers"] == {"Set-Cookie": "***", "Content-Type": "text/plain"} + assert shown["pairs"] == [{"password": "***"}, {"note": "plain"}] + assert shown["secret"] == "***", "a secret-named container is masked whole" + assert shown["empty"] == {"token": None, "secret": ""}, "an empty field has nothing to hide" + assert redact_result(None, _DEEP) == shown, "the command plays no part without an identifier list" + for plain in ("just text", 5, None, ["a", 1]): + assert redact_result("AC_anything", plain) == plain + + +def test_masked_fields_are_exactly_what_the_rule_masks(): + found = masked_fields("AC_anything", _DEEP) + assert [(field.path, field.value) for field in found] == [ + (("token",), "t-top"), + (("leases", 0, "token"), "t-a"), + (("leases", 1, "Token"), "t-b"), + (("http", "headers", "Set-Cookie"), "sid=1"), + (("pairs", 0, "password"), "p-1"), + (("secret",), {"whole": ["mapping", 1]}), + ] + shown = redact_result("AC_anything", _DEEP) + assert json.dumps(shown).count('"***"') == len(found) + for field in found: + holder = shown + for part in field.path[:-1]: + holder = holder[part] + assert holder[field.path[-1]] == "***" + assert "t-top" not in repr(found) + str(found[0]), "a masked field never prints its value" + assert isinstance(found[0], MaskedField) + + +def test_a_record_nested_in_a_result_is_masked_per_command_and_an_action_as_an_action(): + result = { + "execute: ['AC_approval_request', {'action': 'x'}]": {"token": "req-1"}, + "execute: ['AC_user_add', {'user_id': 'a'}]": {"user_id": "a", "token": "tok-1"}, + "plan": ["AC_secret_set", {"name": "db", "value": "hunter2"}], + } + assert redact_result("AC_execute_action", result) == { + "execute: ['AC_approval_request', {'action': 'x'}]": {"token": "***"}, + "execute: ['AC_user_add', {'user_id': 'a'}]": {"user_id": "a", "token": "***"}, + "plan": redact_actions(result["plan"]), + } + shown = redact_result("AC_execute_action", result, show_identifiers=True) + assert shown["execute: ['AC_approval_request', {'action': 'x'}]"] == {"token": "req-1"} + assert shown["execute: ['AC_user_add', {'user_id': 'a'}]"]["token"] == "***" + assert "hunter2" not in json.dumps(shown) + assert [field.value for field in masked_fields( + "AC_execute_action", result, show_identifiers=True)] == ["tok-1"] + + +def test_record_command_reads_the_command_of_a_record_key(): + assert record_command("execute: ['AC_lease_active']") == "AC_lease_active" + assert record_command("dry-run: ['AC_user_add', {'user_id': 'a'}]") == "AC_user_add" + for key in ("token", "execute: nothing", 5, None): + assert record_command(key) is None + + +# --- identifiers ------------------------------------------------------------- + + +def test_the_identifier_list_is_explicit_and_holds_only_the_approval_request_id(): + assert dict(RESULT_IDENTIFIERS) == {"AC_approval_request": frozenset({"token"})} + with pytest.raises(TypeError): + RESULT_IDENTIFIERS["AC_user_add"] = frozenset({"token"}) # type: ignore[index] + for names in RESULT_IDENTIFIERS.values(): + assert names <= SENSITIVE_ARGUMENT_NAMES, "an entry only exempts a name the rule would mask" + + +def test_an_identifier_is_shown_only_at_the_top_of_its_own_command_result(): + result = {"token": "req-1", "detail": {"token": "inner"}} + assert redact_result("AC_approval_request", result, show_identifiers=True) == { + "token": "req-1", "detail": {"token": "***"}} + assert redact_result("AC_approval_request", result) == {"token": "***", "detail": {"token": "***"}} + assert redact_result("AC_lease_secret", {"token": "lease"}, show_identifiers=True) == {"token": "***"} + assert masked_fields("AC_approval_request", {"token": "req-1"}, show_identifiers=True) == [] + + +def test_the_approval_request_id_is_readable_in_the_builder_and_not_offered(tmp_path, caplog): + db = str(tmp_path / "approvals.json") + actions = [["AC_approval_request", {"action": "wire transfer", "requester": "maker", "db": db}]] + record, values = _collected(actions) + request_id = list(record.values())[0]["token"] + assert request_id in json.dumps(displayable_record(record)), "the maker can read the id" + assert values == [], "it is not a one-time value" + approved = execute_action([["AC_approval_approve", {"token": request_id, "approver": "checker", "db": db}]]) + assert list(approved.values())[0] == {"approved": True}, "the id shown is the one the checker uses" + record, logged = _logged_run(caplog, actions) + assert "AC_approval_request" in logged and list(record.values())[0]["token"] not in logged, \ + "the log keeps masking it: the exemption is for the person reading their own run" + + +# --- the commands that were printed in plain text ---------------------------- + + +def test_a_lease_token_is_masked_in_the_builder_and_offered_once(): + record, values = _collected([["AC_lease_secret", {"name": "redaction-db", "ttl": 5}], + ["AC_lease_active"]]) + issued, active = record.values() + try: + shown = displayable_record(record) + text = json.dumps(shown, default=str) + assert issued["token"] not in text and "redaction-db" in text + assert list(shown.values())[0] == {"token": "***", "ttl": 5.0} + mine = [lease for lease in list(shown.values())[1]["leases"] if lease["name"] == "redaction-db"] + assert mine and all(lease["token"] == "***" for lease in mine) + assert (values[0].command, values[0].subject, values[0].name, values[0].value) == ( + "AC_lease_secret", "redaction-db", "token", issued["token"]) + listed = [value for value in values[1:] if value.subject == "redaction-db"] + assert [(value.command, value.value) for value in listed] == [("AC_lease_active", issued["token"])] + assert listed[0].name.startswith("leases[") and listed[0].name.endswith("].token") + assert text.count('"***"') == len(values), "what is offered is exactly what is masked" + finally: + execute_action([["AC_revoke_lease", {"token": issued["token"]}]]) + + +@pytest.mark.parametrize("command", ["AC_rest_api_start", "AC_rest_api_status"]) +def test_the_shared_rest_bearer_token_is_masked_although_the_command_has_no_schema_entry( + monkeypatch, caplog, command): + status = {"running": True, "host": "127.0.0.1", "port": 9939, "token": _BEARER, + "url": "http://127.0.0.1:9939"} + monkeypatch.setattr(action_executor.rest_api_registry, "start", lambda **_kwargs: dict(status)) + monkeypatch.setattr(action_executor.rest_api_registry, "status", lambda: dict(status)) + assert command not in COMMAND_SPECS, "it is run from a loaded action file, like every server command" + autocontrol_logger.addHandler(caplog.handler) + try: + with caplog.at_level(logging.DEBUG, logger=autocontrol_logger.name): + record, values = _collected([[command]]) + finally: + autocontrol_logger.removeHandler(caplog.handler) + assert list(record.values())[0]["token"] == _BEARER, "the script still receives it" + shown = list(displayable_record(record).values())[0] + assert shown == {**status, "token": "***"} + assert [(value.command, value.name, value.value) for value in values] == [(command, "token", _BEARER)] + assert _BEARER not in "\n".join(entry.getMessage() for entry in caplog.records) + + +def test_a_stopped_rest_server_has_no_token_to_mask_or_offer(monkeypatch): + monkeypatch.setattr(action_executor.rest_api_registry, "status", + lambda: {"running": False, "token": None, "url": None}) + record, values = _collected([["AC_rest_api_status"]]) + assert list(displayable_record(record).values())[0] == {"running": False, "token": None, "url": None} + assert values == [] + + +def test_the_commands_that_answer_with_a_generated_secret_are_marked_in_the_schema(): + marked = {command for command, spec in COMMAND_SPECS.items() if spec.sensitive_result} + assert marked == {"AC_user_add", "AC_user_rotate_token", "AC_jwt_encode", + "AC_lease_secret", "AC_lease_active"} + assert not marked & set(RESULT_IDENTIFIERS), "a command is one or the other" + for command in ("AC_lease_secret", "AC_lease_active"): + assert "One-time values" in COMMAND_SPECS[command].description + assert "not masked" in COMMAND_SPECS["AC_approval_request"].description + + +def test_the_display_of_a_record_does_not_depend_on_the_schema_flag(): + record = { + "execute: ['AC_made_up_command']": {"items": [{"api_key": "k-1", "id": 7}]}, + "execute: ['AC_get_var', {'name': 'a'}]": "AutoControlActionException('token missing')", + "execute: ['AC_loop', {'times': 2}]": 2, + } + assert displayable_record(record) == { + "execute: ['AC_made_up_command']": {"items": [{"api_key": "***", "id": 7}]}, + "execute: ['AC_get_var', {'name': 'a'}]": "AutoControlActionException('token missing')", + "execute: ['AC_loop', {'times': 2}]": 2, + } + assert displayable_record("not a record") == "not a record" + assert displayable_record([{"token": "t"}]) == [{"token": "***"}] diff --git a/test/unit_test/headless/test_script_builder_one_time_reveal.py b/test/unit_test/headless/test_script_builder_one_time_reveal.py index 1b96f3af6..d112b3a9c 100644 --- a/test/unit_test/headless/test_script_builder_one_time_reveal.py +++ b/test/unit_test/headless/test_script_builder_one_time_reveal.py @@ -2,14 +2,18 @@ ``AC_user_add`` / ``AC_user_rotate_token`` answer with a bearer token, and the builder masks it in its result pane -- so whoever ran the step there never saw -it. The values of the last run are now held in memory until the "one-time -values" button hands them to a dialog; the tab forgets them at that moment, -when the next run starts and when it is disposed, and the dialog forgets them -when it closes. +it. The values of the last run are held in memory until the "one-time values" +button hands them to a dialog; the tab forgets them at that moment, when the +next run starts and when it is disposed, and the dialog forgets them when it +closes. + +The values are gathered from the executor's result hook, so a token issued in +the body of a block (``AC_loop``, ``AC_try``...) is offered too, labelled by +its path; at most ``MAX_ONE_TIME_VALUES`` are kept for one run. The first tests are headless (what the run produced, how each value is named). The rest build the tab offscreen; the dialog is built and inspected but never -shown or executed, and the clipboard is replaced by a list. +shown or executed, and the clipboard is a fake. """ import json import logging @@ -18,16 +22,20 @@ import pytest from je_auto_control.gui.script_builder.step_model import ( - OneTimeValue, Step, displayable_record, nested_sensitive_commands, one_time_values, + MAX_ONE_TIME_VALUES, OneTimeCollector, OneTimeValue, Step, displayable_record, save_action_file, steps_to_actions, ) +from je_auto_control.utils.exception.exceptions import AutoControlActionException from je_auto_control.utils.executor.action_executor import execute_action +from je_auto_control.utils.executor.result_hook import StepPosition +from je_auto_control.utils.executor.run_control import ExecutionStopped from je_auto_control.utils.logging.logging_instance import autocontrol_logger from je_auto_control.utils.rbac import UserStore _KEYS = ("sb_one_time_show", "sb_one_time_title", "sb_one_time_notice", "sb_one_time_step", - "sb_one_time_copy", "sb_one_time_available", "sb_one_time_nested") -_ADD_KEY = "execute: ['AC_user_add', {'user_id': 'x'}]" + "sb_one_time_copy", "sb_one_time_available", "sb_one_time_inside", "sb_one_time_shown", + "sb_one_time_limit", "sb_one_time_close_clear") +_TOP = (StepPosition(1, 1, "AC_user_add"),) def _user_steps(users: str) -> list: @@ -38,72 +46,124 @@ def _user_steps(users: str) -> list: ] +def _loop_step(users: str, times: int = 2) -> Step: + """A loop whose body lists the users, then issues a token inside an ``AC_try``.""" + return Step("AC_loop", {"times": times}, bodies={"body": [ + Step("AC_user_list", {"users_path": users}), + Step("AC_try", bodies={"body": [ + Step("AC_user_rotate_token", {"user_id": "ann", "users_path": users})], "catch": []}), + ]}) + + +def _collected(actions: list, limit: int = MAX_ONE_TIME_VALUES): + """Run ``actions`` as the builder does: the record, the values kept, whether some were left out.""" + collector = OneTimeCollector(limit) + record = execute_action(actions, result_callback=collector.note) + values, truncated = collector.take() + return record, values, truncated + + # --- headless: what a run produced ------------------------------------------- def test_the_headless_run_still_returns_the_token_and_names_each_by_its_step(tmp_path): users = str(tmp_path / "users.json") - record = execute_action(steps_to_actions(_user_steps(users))) - values = one_time_values(record) + record, values, truncated = _collected(steps_to_actions(_user_steps(users))) assert [(value.step, value.command, value.subject, value.name) for value in values] == [ (1, "AC_user_add", "ann", "token"), (3, "AC_user_rotate_token", "ann", "token")] assert values[0].value == list(record.values())[0]["token"], "the record is not changed" assert UserStore(users).authenticate(values[1].value).user_id == "ann" + assert not truncated def test_a_value_is_not_in_the_text_form_of_its_holder(): - value = OneTimeValue(1, "AC_user_add", "ann", "token", "tok-in-memory-only") + value = OneTimeValue(_TOP, "AC_user_add", "ann", "token", "tok-in-memory-only") assert "tok-in-memory-only" not in repr(value) + str(value) + f"{[value]}" + assert value.step == 1 -def test_a_failed_or_empty_marked_command_has_nothing_to_reveal(): - assert one_time_values({_ADD_KEY: "UserAuthError('user exists')"}) == [] - assert one_time_values({_ADD_KEY: None}) == [] - assert one_time_values({_ADD_KEY: {"user_id": "x", "token": ""}}) == [] - assert one_time_values("not a record") == [] - - -def test_a_marked_result_without_fields_is_revealed_whole(): - values = one_time_values({_ADD_KEY: ["opaque", "values"]}) +def test_a_failed_or_empty_command_has_nothing_to_reveal(tmp_path): + users = str(tmp_path / "users.json") + UserStore(users).add_user(user_id="root", display_name="root", role="admin") + record, values, _truncated = _collected( + [["AC_user_rotate_token", {"user_id": "nobody", "users_path": users}]]) + assert "nobody" in list(record.values())[0] and values == [] + collector = OneTimeCollector() + collector.note("AC_user_add", {"user_id": "x"}, None, _TOP) + collector.note("AC_user_add", {"user_id": "x"}, {"user_id": "x", "token": ""}, _TOP) + collector.note("AC_user_add", {"user_id": "x"}, {"user_id": "x", "token": None}, _TOP) + assert collector.take() == ([], False) + + +def test_a_marked_result_without_fields_is_hidden_and_revealed_whole(): + key = "execute: ['AC_user_add', {'user_id': 'x'}]" + assert "opaque" not in json.dumps(displayable_record({key: ["opaque", "values"]})) + collector = OneTimeCollector() + collector.note("AC_user_add", {"user_id": "x"}, ["opaque", "values"], _TOP) + values, _truncated = collector.take() assert [(value.step, value.name, value.value) for value in values] == [ (1, "", json.dumps(["opaque", "values"]))] -def test_a_marked_result_in_a_nested_record_keeps_the_top_level_step_number(): - record = {"execute: ['AC_get_var', {'name': 'a'}]": 1, - "execute: ['AC_try', {}]": {"body": [{_ADD_KEY: {"user_id": "x", "token": "tok-nested"}}]}} - values = one_time_values(record) - assert [(value.step, value.subject, value.value) for value in values] == [(2, "x", "tok-nested")] +def test_a_token_issued_inside_a_block_is_collected_with_its_path(tmp_path): + """The gap: a block records only its summary, so the value was in no record to reveal.""" + users = str(tmp_path / "users.json") + actions = steps_to_actions([_user_steps(users)[0], _loop_step(users, times=3)]) + record, values, _truncated = _collected(actions) + nested = values[1:] + assert [value.path for value in nested] == [ + (StepPosition(1, 2, "AC_loop"), StepPosition(run, 2, "AC_try"), + StepPosition(1, 1, "AC_user_rotate_token")) for run in (1, 2, 3)] + assert {(value.step, value.command, value.subject, value.name) for value in nested} == { + (2, "AC_user_rotate_token", "ann", "token")} + assert len({value.value for value in values}) == 4 + assert UserStore(users).authenticate(nested[-1].value).user_id == "ann", "the last one is the live token" + raw = json.dumps(record, default=str) + assert not any(value.value in raw for value in nested), "the record still holds only the summary" -def test_everything_revealed_is_exactly_what_the_display_masks(tmp_path): +def test_a_block_that_returns_its_body_record_does_not_offer_a_value_twice(tmp_path): users = str(tmp_path / "users.json") - record = execute_action(steps_to_actions(_user_steps(users))) + record, values, _truncated = _collected([["AC_execute_action", {"action_list": [ + ["AC_user_add", {"user_id": "bob", "users_path": users}]]}]]) + assert [(value.command, len(value.path), value.subject) for value in values] == [ + ("AC_user_add", 2, "bob")] shown = json.dumps(displayable_record(record), default=str) - raw = json.dumps(record, default=str) - for value in one_time_values(record): - assert value.value in raw and value.value not in shown + assert values[0].value in json.dumps(record, default=str) and values[0].value not in shown -def test_marked_commands_inside_a_block_are_named_and_top_level_ones_are_not(): - actions = [ - ["AC_user_add", {"user_id": "top"}], - ["AC_loop", {"times": 2, "body": [ - ["AC_user_rotate_token", {"user_id": "a"}], - ["AC_try", {"body": [["AC_user_add", {"user_id": "b"}]], "catch": []}]]}], - ["AC_user_list"], - ] - assert nested_sensitive_commands(actions) == ["AC_user_add", "AC_user_rotate_token"] - assert nested_sensitive_commands(actions[:1] + actions[2:]) == [] - assert nested_sensitive_commands({"not": "a list"}) == [] +def test_a_value_only_a_parallel_branch_produced_is_taken_from_the_block_result(): + """A branch runs on its own thread, which the hook does not follow; its record is in the result.""" + record, values, _truncated = _collected( + [["AC_parallel", {"branches": [[["AC_lease_secret", {"name": "one-time-par", "ttl": 5}]]]}]]) + try: + assert [(value.command, value.name) for value in values] == [ + ("AC_parallel", "results[0].AC_lease_secret.token")] + assert values[0].value not in json.dumps(displayable_record(record), default=str) + finally: + execute_action([["AC_revoke_lease", {"token": values[0].value}]]) -def test_a_block_really_records_no_nested_token(tmp_path): - """Why the note exists: the value issued in a body is in no record to reveal.""" +def test_the_number_of_values_kept_for_one_run_is_bounded(tmp_path): users = str(tmp_path / "users.json") - record = execute_action([["AC_loop", {"times": 1, "body": [ - ["AC_user_add", {"user_id": "bob", "users_path": users}]]}]]) - assert UserStore(users).get("bob") is not None and one_time_values(record) == [] + actions = steps_to_actions([_user_steps(users)[0], _loop_step(users, times=6)]) + _record, values, truncated = _collected(actions, limit=4) + assert len(values) == 4 and truncated + assert [value.step for value in values] == [1, 2, 2, 2], "the first ones are the ones kept" + _record, values, truncated = _collected( + [["AC_user_rotate_token", {"user_id": "ann", "users_path": users}]], limit=1) + assert len(values) == 1 and not truncated, "exactly at the limit nothing was left out" + assert MAX_ONE_TIME_VALUES == 200 and OneTimeCollector().limit == 200 + + +def test_everything_revealed_is_exactly_what_the_display_masks(tmp_path): + users = str(tmp_path / "users.json") + record, values, _truncated = _collected(steps_to_actions(_user_steps(users))) + shown = json.dumps(displayable_record(record), default=str) + raw = json.dumps(record, default=str) + assert shown.count('"***"') == len(values) == 2 + for value in values: + assert value.value in raw and value.value not in shown def test_a_signed_jwt_is_masked_revealed_once_and_kept_out_of_the_log(caplog): @@ -111,14 +171,14 @@ def test_a_signed_jwt_is_masked_revealed_once_and_kept_out_of_the_log(caplog): autocontrol_logger.addHandler(caplog.handler) try: with caplog.at_level(logging.DEBUG, logger=autocontrol_logger.name): - record = execute_action([["AC_jwt_encode", {"claims": {"sub": "ann"}, "key": "k" * 40}]]) + record, values, _truncated = _collected( + [["AC_jwt_encode", {"claims": {"sub": "ann"}, "key": "k" * 40}]]) finally: autocontrol_logger.removeHandler(caplog.handler) token = list(record.values())[0]["token"] assert token.count(".") == 2, "the script still gets the signed token" assert token not in json.dumps(displayable_record(record)) - assert [(value.step, value.name, value.value) for value in one_time_values(record)] == [ - (1, "token", token)] + assert [(value.step, value.name, value.value) for value in values] == [(1, "token", token)] logged = "\n".join(entry.getMessage() for entry in caplog.records) assert "AC_jwt_encode" in logged and token not in logged @@ -155,7 +215,10 @@ def test_the_strings_exist_in_every_catalogue_with_the_same_placeholders(): def placeholders(text: str) -> list: return sorted(re.findall(r"\{(\w+)\}", text)) - for catalogue in (japanese_word_dict, simplified_chinese_word_dict, traditional_chinese_word_dict): + catalogues = (english_word_dict, japanese_word_dict, simplified_chinese_word_dict, + traditional_chinese_word_dict) + for catalogue in catalogues: + assert "sb_one_time_nested" not in catalogue, "the 'not available in a block' line is gone" for key in _KEYS: assert catalogue[key].strip() assert placeholders(catalogue[key]) == placeholders(english_word_dict[key]), key @@ -164,9 +227,26 @@ def placeholders(text: str) -> list: # --- the tab, offscreen ------------------------------------------------------ +class _FakeClipboard: + """Stands in for the system clipboard: what it holds, and every value copied onto it.""" + + def __init__(self) -> None: + self.text = "somebody else's note" + self.copied: list = [] + self.cleared = 0 + + def copy(self, text: str) -> None: + self.copied.append(text) + self.text = text + + def clear(self) -> None: + self.cleared += 1 + self.text = "" + + @pytest.fixture() def builder(monkeypatch): - """A Script Builder tab, its module, and the list standing in for the clipboard.""" + """A Script Builder tab, its module, and the fake standing in for the clipboard.""" pytest.importorskip("PySide6.QtWidgets", exc_type=ImportError) from PySide6.QtWidgets import QApplication @@ -175,15 +255,17 @@ def builder(monkeypatch): app = QApplication.instance() if app is None: app = QApplication([]) - copied: list = [] - monkeypatch.setattr(one_time_dialog, "_copy_to_clipboard", copied.append) + clipboard = _FakeClipboard() + monkeypatch.setattr(one_time_dialog, "_copy_to_clipboard", clipboard.copy) + monkeypatch.setattr(one_time_dialog, "_clipboard_text", lambda: clipboard.text) + monkeypatch.setattr(one_time_dialog, "_clear_clipboard", clipboard.clear) def refuse(*_args, **_kwargs): raise AssertionError("the reveal dialog must never run a nested event loop") monkeypatch.setattr(one_time_dialog.OneTimeValueDialog, "exec", refuse) tab = builder_tab.ScriptBuilderTab() try: - yield tab, builder_tab, copied + yield tab, builder_tab, clipboard finally: tab.dispose() tab.deleteLater() @@ -203,8 +285,14 @@ def _translated(key: str, **fields) -> str: return language_wrapper.translate(key, key).format(**fields) +def _label(command: str) -> str: + from je_auto_control.gui.script_builder.command_schema import COMMAND_SPECS + spec = COMMAND_SPECS.get(command) + return spec.label if spec is not None else command # a command without a form shows its name + + def test_the_result_pane_masks_and_says_values_are_available(builder, tmp_path): - tab, _module, _copied = builder + tab, _module, _clipboard = builder assert not tab._reveal_btn.isEnabled(), "nothing to reveal before a run" _run(tab, _user_steps(str(tmp_path / "users.json"))) text = tab._result.toPlainText() @@ -218,7 +306,7 @@ def test_the_result_pane_masks_and_says_values_are_available(builder, tmp_path): def test_the_dialog_shows_each_value_labelled_by_its_step_and_only_once(builder, tmp_path): - tab, _module, copied = builder + tab, _module, clipboard = builder users = str(tmp_path / "users.json") _run(tab, _user_steps(users)) tokens = [value.value for value in tab._one_time] @@ -231,25 +319,138 @@ def test_the_dialog_shows_each_value_labelled_by_its_step_and_only_once(builder, assert not dialog.isVisible() and not dialog.isModal(), "built, not shown" assert tab._one_time == [] and not tab._reveal_btn.isEnabled() assert tab._take_one_time_dialog() is None, "a second reveal has nothing left" - assert dialog.copy_value(1) and copied == [tokens[1]] - assert UserStore(users).authenticate(copied[0]).user_id == "ann", "what is copied is the live token" + assert dialog.copy_value(1) and clipboard.copied == [tokens[1]] + assert UserStore(users).authenticate(clipboard.copied[0]).user_id == "ann", \ + "what is copied is the live token" assert not dialog.copy_value(5) +def test_a_token_issued_inside_a_block_is_offered_and_labelled_by_its_path(builder, tmp_path): + tab, _module, _clipboard = builder + users = str(tmp_path / "users.json") + _run(tab, [_user_steps(users)[0], _loop_step(users, times=2)]) + text = tab._result.toPlainText() + assert _translated("sb_one_time_available", count=3) in text + assert "block" not in text.lower(), "nothing is reported as unavailable any more" + dialog = tab._take_one_time_dialog() + tokens = dialog.shown_values() + assert len(set(tokens)) == 3 and not any(token in text for token in tokens) + + def nested(run: int) -> str: + path = " › ".join(( + _translated("sb_one_time_step", step=2), + _translated("sb_one_time_inside", block=_label("AC_loop"), run=run, step=2), + _translated("sb_one_time_inside", block=_label("AC_try"), run=1, step=1))) + return " · ".join((path, "User: Rotate Token", "ann", "token")) + assert dialog.labels()[1:] == [nested(1), nested(2)] + assert "Step 2 › " + _label("AC_loop") + " run 2, step 2 › " in dialog.labels()[2] + assert UserStore(users).authenticate(tokens[2]).user_id == "ann" + dialog.reject() + + +def test_the_pane_says_when_a_run_issued_more_values_than_are_kept(builder, tmp_path, monkeypatch): + import functools + + tab, module, _clipboard = builder + monkeypatch.setattr(module, "OneTimeCollector", functools.partial(OneTimeCollector, 2)) + users = str(tmp_path / "users.json") + _run(tab, [_user_steps(users)[0], _loop_step(users, times=5)]) + text = tab._result.toPlainText() + assert len(tab._one_time) == 2 + assert _translated("sb_one_time_available", count=2) in text + assert _translated("sb_one_time_limit", limit=2) in text + _run(tab, _user_steps(users)[2:]) + assert _translated("sb_one_time_limit", limit=2) not in tab._result.toPlainText() + + +def test_the_pane_says_the_values_were_shown_once_the_dialog_was_opened(builder, tmp_path): + """It kept saying values were available after they had been handed over and dropped.""" + tab, _module, _clipboard = builder + _run(tab, _user_steps(str(tmp_path / "users.json"))) + available = _translated("sb_one_time_available", count=2) + assert available in tab._result.toPlainText() + dialog = tab._take_one_time_dialog() + text = tab._result.toPlainText() + assert available not in text and _translated("sb_one_time_shown") in text + assert '"token": "***"' in text, "the masked record stays" + dialog.reject() + assert _translated("sb_one_time_shown") in tab._result.toPlainText() + + +def test_marking_values_shown_leaves_another_message_in_the_pane_alone(builder, tmp_path): + tab, _module, _clipboard = builder + _run(tab, _user_steps(str(tmp_path / "users.json"))) + tab._result.setPlainText("Saved: somewhere.json") + tab._take_one_time_dialog().reject() + assert tab._result.toPlainText() == "Saved: somewhere.json" + + def test_closing_the_dialog_forgets_every_value(builder, tmp_path): - tab, _module, copied = builder + tab, _module, clipboard = builder _run(tab, _user_steps(str(tmp_path / "users.json"))) dialog = tab._take_one_time_dialog() assert tab._reveal_dialog is dialog dialog.reject() # what the Close button and the window's close box do assert dialog.shown_values() == ["", ""] and dialog.labels() == [] - assert not dialog.copy_value(0) and copied == [] + assert not dialog.copy_value(0) and clipboard.copied == [] assert tab._reveal_dialog is None and tab._one_time == [] assert tab._take_one_time_dialog() is None + assert clipboard.cleared == 0 and clipboard.text == "somebody else's note" + + +def test_clearing_is_offered_only_after_a_copy_and_clears_a_value_still_on_the_clipboard( + builder, tmp_path): + tab, _module, clipboard = builder + _run(tab, _user_steps(str(tmp_path / "users.json"))) + dialog = tab._take_one_time_dialog() + tokens = dialog.shown_values() + assert not dialog.can_clear_clipboard() and not dialog._clear_btn.isEnabled() + assert not dialog.clear_clipboard() and clipboard.cleared == 0, "nothing was copied from here" + dialog.copy_value(0) + dialog.copy_value(1) + assert dialog.can_clear_clipboard() and clipboard.text == tokens[1] + dialog._clear_btn.click() # "Close and clear clipboard" + assert clipboard.cleared == 1 and clipboard.text == "" + assert dialog.shown_values() == ["", ""] and tab._reveal_dialog is None, "it also closed" + assert not dialog.can_clear_clipboard() + + +def test_clearing_leaves_content_somebody_else_put_on_the_clipboard(builder, tmp_path): + tab, _module, clipboard = builder + _run(tab, _user_steps(str(tmp_path / "users.json"))) + dialog = tab._take_one_time_dialog() + dialog.copy_value(0) + clipboard.text = "a paragraph copied in another window" + dialog.close_and_clear() + assert clipboard.cleared == 0 and clipboard.text == "a paragraph copied in another window" + assert dialog.shown_values() == ["", ""], "the dialog still closed and forgot" + + +def test_an_earlier_copy_from_the_dialog_still_on_the_clipboard_is_cleared(builder, tmp_path): + tab, _module, clipboard = builder + _run(tab, _user_steps(str(tmp_path / "users.json"))) + dialog = tab._take_one_time_dialog() + first = dialog.shown_values()[0] + dialog.copy_value(1) + dialog.copy_value(0) + assert clipboard.text == first and dialog.clear_clipboard() and clipboard.text == "" + assert not dialog.clear_clipboard(), "a second clear has nothing of this dialog's to remove" + dialog.reject() + + +def test_a_plain_close_after_a_copy_leaves_the_clipboard_for_pasting(builder, tmp_path): + tab, _module, clipboard = builder + _run(tab, _user_steps(str(tmp_path / "users.json"))) + dialog = tab._take_one_time_dialog() + token = dialog.shown_values()[0] + dialog.copy_value(0) + dialog.reject() + assert clipboard.cleared == 0 and clipboard.text == token + assert not dialog.clear_clipboard(), "once closed the dialog no longer knows the value" def test_the_button_opens_the_dialog_without_a_nested_event_loop(builder, tmp_path, monkeypatch): - tab, _module, _copied = builder + tab, _module, _clipboard = builder from je_auto_control.gui.script_builder.one_time_dialog import OneTimeValueDialog opened: list = [] @@ -263,7 +464,7 @@ def test_the_button_opens_the_dialog_without_a_nested_event_loop(builder, tmp_pa def test_the_next_run_drops_values_nobody_revealed(builder, tmp_path): - tab, _module, _copied = builder + tab, _module, _clipboard = builder users = str(tmp_path / "users.json") _run(tab, _user_steps(users)) assert len(tab._one_time) == 2 @@ -273,17 +474,17 @@ def test_the_next_run_drops_values_nobody_revealed(builder, tmp_path): def test_the_next_run_empties_a_dialog_still_holding_values(builder, tmp_path): - tab, _module, copied = builder + tab, _module, clipboard = builder users = str(tmp_path / "users.json") _run(tab, _user_steps(users)) dialog = tab._take_one_time_dialog() _run(tab, [Step("AC_user_list", {"users_path": users})]) - assert dialog.shown_values() == ["", ""] and not dialog.copy_value(0) and copied == [] + assert dialog.shown_values() == ["", ""] and not dialog.copy_value(0) and clipboard.copied == [] assert tab._reveal_dialog is None def test_disposing_the_tab_forgets_pending_values_and_the_dialog(builder, tmp_path): - tab, _module, _copied = builder + tab, _module, _clipboard = builder users = str(tmp_path / "users.json") _run(tab, _user_steps(users)) tab.dispose() @@ -295,7 +496,7 @@ def test_disposing_the_tab_forgets_pending_values_and_the_dialog(builder, tmp_pa def test_a_failed_step_offers_nothing(builder, tmp_path): - tab, _module, _copied = builder + tab, _module, _clipboard = builder users = str(tmp_path / "users.json") UserStore(users).add_user(user_id="root", display_name="root", role="admin") _run(tab, [Step("AC_user_rotate_token", {"user_id": "nobody", "users_path": users})]) @@ -303,14 +504,19 @@ def test_a_failed_step_offers_nothing(builder, tmp_path): assert "nobody" in tab._result.toPlainText() -def test_a_token_issued_inside_a_block_is_reported_as_unavailable(builder, tmp_path): - tab, _module, _copied = builder - users = str(tmp_path / "users.json") - _run(tab, [Step("AC_loop", {"times": 1}, bodies={"body": [ - Step("AC_user_add", {"user_id": "bob", "users_path": users})]})]) - assert UserStore(users).get("bob") is not None - assert _translated("sb_one_time_nested", commands="AC_user_add") in tab._result.toPlainText() - assert tab._one_time == [] and not tab._reveal_btn.isEnabled() +def test_a_run_that_was_stopped_or_failed_still_offers_what_it_had_issued(builder, monkeypatch): + tab, module, _clipboard = builder + warned: list = [] + monkeypatch.setattr(module.QMessageBox, "warning", lambda *args: warned.append(args[-1])) + for error in (ExecutionStopped("stopped"), AutoControlActionException("the run broke")): + tab._collector = OneTimeCollector() + tab._collector.note("AC_user_add", {"user_id": "ann"}, {"user_id": "ann", "token": "tok-early"}, _TOP) + tab._show_run_error(error) + text = tab._result.toPlainText() + assert _translated("sb_one_time_available", count=1) in text and "tok-early" not in text + assert [value.value for value in tab._one_time] == ["tok-early"] and tab._reveal_btn.isEnabled() + tab._forget_one_time() + assert warned == ["the run broke"] # --- the value reaches nothing that is kept ---------------------------------- @@ -329,8 +535,9 @@ def record(_self, *args, **kwargs): return record +@pytest.mark.parametrize("nested", [False, True], ids=["top-level", "inside a block"]) def test_the_token_reaches_no_log_record_journal_history_script_or_setting( - builder, tmp_path, monkeypatch, caplog): + builder, tmp_path, monkeypatch, caplog, nested): from je_auto_control.gui.window_settings import WindowSettings from je_auto_control.utils.action_journal.recorder import ( start_action_journal, stop_action_journal, @@ -338,7 +545,7 @@ def test_the_token_reaches_no_log_record_journal_history_script_or_setting( from je_auto_control.utils.run_history.history_store import HistoryStore from je_auto_control.utils.test_record.record_test_class import test_record_instance - tab, _module, copied = builder + tab, _module, clipboard = builder users = str(tmp_path / "users.json") journal = tmp_path / "journal.jsonl" saved = tmp_path / "saved.json" @@ -349,23 +556,26 @@ def test_the_token_reaches_no_log_record_journal_history_script_or_setting( monkeypatch.setattr(WindowSettings, name, writes.recorder(f"settings.{name}")) monkeypatch.setattr(test_record_instance, "init_record", True) monkeypatch.setattr(test_record_instance, "test_record_list", []) + steps = _user_steps(users) + ([_loop_step(users, times=2)] if nested else []) autocontrol_logger.addHandler(caplog.handler) start_action_journal(journal) try: with caplog.at_level(logging.DEBUG, logger=autocontrol_logger.name): - _run(tab, _user_steps(users)) + _run(tab, steps) pane_before = tab._result.toPlainText() dialog = tab._take_one_time_dialog() tokens = dialog.shown_values() - dialog.copy_value(0) - dialog.copy_value(1) - dialog.reject() + for index in range(len(tokens)): + dialog.copy_value(index) + dialog.close_and_clear() save_action_file(str(saved), tab._tree.root_steps(), tab._file_extras) finally: stop_action_journal() autocontrol_logger.removeHandler(caplog.handler) - assert len(tokens) == 2 and copied == tokens, "the values were really revealed and copied" - assert UserStore(users).authenticate(tokens[1]).user_id == "ann" + assert len(tokens) == (4 if nested else 2), "the nested tokens were collected too" + assert clipboard.copied == tokens, "the values were really revealed and copied" + assert clipboard.text == "", "and the last one was taken off the clipboard again" + assert UserStore(users).authenticate(tokens[-1]).user_id == "ann" outputs = { "result pane before the reveal": pane_before, @@ -382,6 +592,9 @@ def test_the_token_reaches_no_log_record_journal_history_script_or_setting( assert "AC_user_add" in outputs["file journal.jsonl"], "the run was journalled" assert "AC_user_add" in outputs["file saved.json"], "the script was saved" assert "file users.json" in outputs, "the store (hashes only) is searched too" + if nested: + assert outputs["file journal.jsonl"].count("AC_user_rotate_token") >= 3, \ + "the nested steps were journalled" for name, text in outputs.items(): for token in tokens: assert token not in text, f"the token reached: {name}" diff --git a/test/unit_test/headless/test_script_builder_user_commands.py b/test/unit_test/headless/test_script_builder_user_commands.py index 8a4263969..e3af3daeb 100644 --- a/test/unit_test/headless/test_script_builder_user_commands.py +++ b/test/unit_test/headless/test_script_builder_user_commands.py @@ -45,8 +45,8 @@ def test_role_fields_offer_exactly_the_three_roles(): def test_only_the_token_returning_commands_are_marked(): marked = {command for command, spec in COMMAND_SPECS.items() if spec.sensitive_result} - # AC_jwt_encode mints a bearer credential the same way: it exists only in the reply. - assert marked == _TOKEN_COMMANDS | {"AC_jwt_encode"} + # AC_jwt_encode and the lease commands answer with a credential the same way. + assert marked == _TOKEN_COMMANDS | {"AC_jwt_encode", "AC_lease_secret", "AC_lease_active"} def test_a_run_record_is_shown_without_the_token(tmp_path): @@ -96,9 +96,13 @@ def test_a_failed_marked_command_still_shows_its_error(): assert displayable_record({key: "UserAuthError('unknown user')"})[key].startswith("UserAuthError") -def test_other_commands_are_displayed_unchanged(): - record = {"execute: ['AC_get_var', {'name': 'token'}]": {"token": "kept: not a marked command"}} - assert displayable_record(record) == record +def test_a_secret_named_field_is_masked_for_a_command_that_is_not_marked(): + """Only marked commands were masked, so a lease or REST bearer token was printed.""" + key = "execute: ['AC_get_var', {'name': 'token'}]" + record = {key: {"token": "masked whatever the command", "count": 3}} + assert displayable_record(record) == {key: {"token": "***", "count": 3}} + plain = {"execute: ['AC_get_var', {'name': 'a'}]": {"count": 3, "items": ["a"]}} + assert displayable_record(plain) == plain def test_the_builder_tab_does_not_put_the_token_in_its_result_pane(monkeypatch, tmp_path): @@ -114,8 +118,8 @@ def test_the_builder_tab_does_not_put_the_token_in_its_result_pane(monkeypatch, tab._tree.load_steps([Step("AC_user_add", {"user_id": "eve", "users_path": users})]) seen = [] - def run(actions): - record = execute_action(actions) + def run(actions, **keywords): + record = execute_action(actions, **keywords) seen.extend(value["token"] for value in record.values()) return record monkeypatch.setattr(builder_tab, "execute_action", run)