From 43959a0c71bc6b9e0e74218c12213dc7350226e3 Mon Sep 17 00:00:00 2001 From: JeffreyChen Date: Fri, 9 Oct 2026 11:23:47 +0800 Subject: [PATCH 1/7] Record two maintainer decisions: coverage stays on coverage run, releasing the Remote Desktop tab ends WebRTC sessions --- CLAUDE.md | 4 +++- Progress.md | 4 ---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b49796a7..295991e0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -42,7 +42,9 @@ executed: measured, that was 11,962 statements and ~24 percentage points module `je_auto_control_pytest`, which imports only pytest (`test_pytest_entrypoint_light.py` keeps it so), but `coverage run` remains the rule: it does not depend on what a plugin imports or on which build is -installed, and `pytest --cov` has not been re-measured since. +installed. Re-measured on 2026-10-09 the two agreed (87.35% against 87.32%), +and the maintainer decided the same day to keep `coverage run` as the only +spelling. `test/unit_test/headless/test_coverage_measurement.py` holds CI to the correct spelling. After changing the entry point, reinstall (`pip install -e .`) for it to take effect. diff --git a/Progress.md b/Progress.md index faa458e5..2a48450c 100644 --- a/Progress.md +++ b/Progress.md @@ -308,8 +308,6 @@ U-20261009-18 的每一個裝置面呼叫都只對假的 ADB／uiautomator2／WD 新轉成可停止的等待裡，`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 的時間與名稱衝突行為只對假物件測過。 -- **`DECIDE`：釋放 Remote Desktop 分頁時，WebRTC 的 host 與 viewer session 現在會被結束**（TCP／WebSocket 的仍留在 registry）。理由：WebRTC host 只屬於面板， - 面板釋放後沒有東西能停它，會在看不到的情況下繼續分享畫面。要改回去是拿掉兩個 `dispose()` 裡的 `self._stop_host_if_any` 與 `self._end_session_for_good`。 - **切換主題後那幾個事件迴圈回合內**，以被暫存的頁面為 parent 的 `QMessageBox` 會相對於隱藏的暫存 widget 定位（只影響位置）。 系統匣路徑在 offscreen 下沒有 tray，`_on_tray_open` 是直接呼叫測的。基準（`benchmarks/gui_*.py`）CI 仍不設門檻。 - **把 Python 建的 widget 放進分頁列在結束時崩潰**：已查明觸發條件——分頁列按鈕上接了 Python callable、又有排隊中的 `deleteLater()`、而行程經 `os._exit` 離開； @@ -326,8 +324,6 @@ U-20261009-18 的每一個裝置面呼叫都只對假的 ADB／uiautomator2／WD `TODO` — 小項目，各自獨立 -- **`DECIDE`：`pytest --cov` 可以放行了嗎**。進入點搬到 `je_auto_control_pytest` 之後重新量過（2026-10-09）：`coverage run -m pytest` 87.35%，`pytest --cov` 87.32%。 - `CLAUDE.md` 與 `test_coverage_measurement.py` 仍規定只能用 `coverage run`；要不要放寬由維護者決定。 - **本機的共用 `.venv` 裝著 `je_auto_control_dev 0.0.136`**，它的 `pytest11` 進入點還是舊的重量級路徑；重裝即可，不影響 CI。 - **free-threaded**：呼叫者身分、綁定的裝置、自愈的執行標記與 stop token 都改成只在設定它的執行緒上有效（`utils/thread_bound.py`）。 這台機器沒有 free-threaded 直譯器：驗證用的是模擬繼承與真的 `-X thread_inherit_context=1`（3.14.7，GIL 開著）；`free-threaded-scope` job（3.14t）是第一次真的跑。 From 9c2aa61df39cd5ceec855bcba3de7db98a62875a Mon Sep 17 00:00:00 2001 From: JeffreyChen Date: Fri, 9 Oct 2026 11:36:33 +0800 Subject: [PATCH 2/7] Let the Script Builder reveal a freshly issued token once AC_user_add and AC_user_rotate_token answer with a token that exists nowhere else, and the builder masked it, so whoever ran the step there had to repeat it from another surface. 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. Nothing opens by itself, and nothing writes a value anywhere. AC_jwt_encode mints a credential the same way and is marked too. The executor's result log printed such a result as it was (a signed JWT, a lease token); it now masks secret-named fields, as it already did for arguments. --- .../Eng/doc/new_features/v61_features_doc.rst | 6 +- .../operations_layer/operations_layer_doc.rst | 17 +- .../Zh/doc/new_features/v61_features_doc.rst | 4 +- .../operations_layer/operations_layer_doc.rst | 10 +- .../gui/language_wrapper/english.py | 13 + .../gui/language_wrapper/japanese.py | 13 + .../language_wrapper/simplified_chinese.py | 13 + .../language_wrapper/traditional_chinese.py | 13 + .../gui/script_builder/builder_tab.py | 65 ++- .../gui/script_builder/command_schema.py | 9 +- .../gui/script_builder/one_time_dialog.py | 102 +++++ .../gui/script_builder/step_model.py | 103 ++++- .../utils/executor/action_executor.py | 3 +- pyproject.toml | 1 + .../test_script_builder_one_time_reveal.py | 400 ++++++++++++++++++ .../test_script_builder_user_commands.py | 3 +- 16 files changed, 754 insertions(+), 21 deletions(-) create mode 100644 je_auto_control/gui/script_builder/one_time_dialog.py create mode 100644 test/unit_test/headless/test_script_builder_one_time_reveal.py 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 2e6e0376..1cc6c193 100644 --- a/docs/source/Eng/doc/new_features/v61_features_doc.rst +++ b/docs/source/Eng/doc/new_features/v61_features_doc.rst @@ -67,4 +67,8 @@ optional ``alg``; it returns ``{token}``. ``AC_jwt_decode`` takes ``token``, ``key`` and optional ``algorithms`` / ``audience`` / ``leeway``; it returns ``{ok, claims}`` (or ``{ok: false, error}`` so a flow can branch without raising). Both are exposed as the MCP tools ``ac_jwt_encode`` / ``ac_jwt_decode`` -and as Script Builder commands under **Security**. +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. 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 bc608012..439282f0 100644 --- a/docs/source/Eng/doc/operations_layer/operations_layer_doc.rst +++ b/docs/source/Eng/doc/operations_layer/operations_layer_doc.rst @@ -173,8 +173,21 @@ surface, each of which needs the ``manage_users`` capability: ``AC_user_remove``, ``AC_user_set_role``, ``AC_user_rotate_token`` and ``AC_user_list`` in an action list -- so also through ``POST /execute``. The Script Builder lists all five under **Security**; a run's result pane - shows ``AC_user_add`` / ``AC_user_rotate_token`` with the token masked, so - read a new token from the Users group, the CLI or a script instead; + shows ``AC_user_add`` / ``AC_user_rotate_token`` with the token masked and + says that a one-time value is available. **One-time values** on the + builder's toolbar then shows each token of that run once, labelled by its + step (``Step 1 · User: Add · alice · token``), with a Copy button per value. + Nothing opens by itself, so an unattended run is not interrupted. The + values are held in memory only: opening the dialog takes them from the + 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; - 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 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 beab7cbf..2ab32370 100644 --- a/docs/source/Zh/doc/new_features/v61_features_doc.rst +++ b/docs/source/Zh/doc/new_features/v61_features_doc.rst @@ -59,4 +59,6 @@ token 可直接接上 HTTP 用戶端: ``{token}``。``AC_jwt_decode`` 接受 ``token``、``key`` 與選用的 ``algorithms`` / ``audience`` / ``leeway``,回傳 ``{ok, claims}``(或 ``{ok: false, error}``,讓流程可在不拋出 例外的情況下分支)。兩者皆以 MCP 工具 ``ac_jwt_encode`` / ``ac_jwt_decode`` 以及 Script Builder -中 **Security** 分類下的命令提供。 +中 **Security** 分類下的命令提供。Script Builder 的結果面板會遮蔽 ``AC_jwt_encode`` 回傳的 +token;執行後按 **一次性的值** 可看一次(腳本照舊收到 token)。執行器的結果 log 會遮蔽每個命令 +結果中名稱屬於機密的欄位(``token``、``password``…)。 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 9b7c43f0..f2e28a7b 100644 --- a/docs/source/Zh/doc/operations_layer/operations_layer_doc.rst +++ b/docs/source/Zh/doc/operations_layer/operations_layer_doc.rst @@ -158,7 +158,15 @@ CLI:: ``AC_user_remove``、``AC_user_set_role``、``AC_user_rotate_token`` 與 ``AC_user_list``——所以 ``POST /execute`` 也可以。Script Builder 在 **Security** 分類列出這五個指令；執行結果面板顯示 ``AC_user_add``／``AC_user_rotate_token`` 時 - 會遮蔽 token，所以新 token 請從使用者群組、CLI 或腳本取得； + 會遮蔽 token，並註明有一次性的值可看。接著按工具列的 **一次性的值**，就會把該次 + 執行的每個 token 顯示一次，各自標上步驟（``步驟 1 · User: Add · alice · token``）， + 每個值有自己的「複製」按鈕。對話框不會自動開啟，所以無人值守的執行不會被打斷。 + 這些值只存在記憶體裡：開啟對話框時就從分頁取走，關閉對話框即清空，開始下一次 + 執行或關閉分頁則丟棄沒人看過的值。它們不會寫進結果面板、log、測試紀錄、執行歷史、 + 動作日誌、儲存的腳本或 GUI 設定；「複製」按鈕會把值放到剪貼簿。在區塊 + （``AC_loop``、``AC_try``…）裡產生的 token 不在任何紀錄中——區塊只記錄自己的 + 摘要——所以結果面板會註明無法取得；請把這種步驟放在最上層，或改從腳本、CLI、 + 使用者群組執行。``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 選單），該分頁也可以用這個 diff --git a/je_auto_control/gui/language_wrapper/english.py b/je_auto_control/gui/language_wrapper/english.py index 43c07661..893dea8e 100644 --- a/je_auto_control/gui/language_wrapper/english.py +++ b/je_auto_control/gui/language_wrapper/english.py @@ -981,6 +981,19 @@ "rh_journal_link": "Journal run {run} in {path}", "re_import_journal": "Import journal run…", "sb_import_journal": "Import journal", + "sb_one_time_show": "One-time values", + "sb_one_time_title": "One-time values of the last run", + "sb_one_time_notice": ( + "These values exist nowhere else and are shown this once. Copy what you need now: " + "closing this window, or starting another run, forgets them." + ), + "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." + ), "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 82711540..9035c560 100644 --- a/je_auto_control/gui/language_wrapper/japanese.py +++ b/je_auto_control/gui/language_wrapper/japanese.py @@ -871,6 +871,19 @@ "rh_journal_link": "ジャーナル実行 {run}({path})", "re_import_journal": "ジャーナルの実行を取り込む…", "sb_import_journal": "ジャーナル取込", + "sb_one_time_show": "一度だけの値", + "sb_one_time_title": "前回の実行の一度だけの値", + "sb_one_time_notice": ( + "これらの値は他のどこにも存在せず、今回だけ表示されます。必要な値を今コピーしてください。" + "このウィンドウを閉じるか次の実行を開始すると破棄されます。" + ), + "sb_one_time_step": "ステップ {step}", + "sb_one_time_copy": "コピー", + "sb_one_time_available": "一度だけの値が {count} 件あります。「一度だけの値」を押すと一度だけ確認できます。", + "sb_one_time_nested": ( + "ここでは取得できません：ブロック内で発行された値（{commands}）。" + "ブロックは内部のコマンドの結果を記録しません。" + ), "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 679add91..6b6acb21 100644 --- a/je_auto_control/gui/language_wrapper/simplified_chinese.py +++ b/je_auto_control/gui/language_wrapper/simplified_chinese.py @@ -841,6 +841,19 @@ "rh_journal_link": "日志运行 {run},位于 {path}", "re_import_journal": "导入日志的运行…", "sb_import_journal": "导入日志", + "sb_one_time_show": "一次性的值", + "sb_one_time_title": "上次执行的一次性的值", + "sb_one_time_notice": ( + "这些值不存在于其他任何地方，而且只显示这一次。请现在复制需要的值：" + "关闭这个窗口或开始下一次执行后就会被丢弃。" + ), + "sb_one_time_step": "步骤 {step}", + "sb_one_time_copy": "复制", + "sb_one_time_available": "有 {count} 个一次性的值：按「一次性的值」查看，只能看一次。", + "sb_one_time_nested": ( + "这里无法取得：在区块内产生的值（{commands}）。" + "区块不会记录其内部指令的结果。" + ), "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 d150d24e..356f6eb3 100644 --- a/je_auto_control/gui/language_wrapper/traditional_chinese.py +++ b/je_auto_control/gui/language_wrapper/traditional_chinese.py @@ -848,6 +848,19 @@ "rh_journal_link": "日誌執行 {run},位於 {path}", "re_import_journal": "匯入日誌的執行…", "sb_import_journal": "匯入日誌", + "sb_one_time_show": "一次性的值", + "sb_one_time_title": "上次執行的一次性的值", + "sb_one_time_notice": ( + "這些值不存在於其他任何地方，而且只顯示這一次。請現在複製需要的值：" + "關閉這個視窗或開始下一次執行後就會被捨棄。" + ), + "sb_one_time_step": "步驟 {step}", + "sb_one_time_copy": "複製", + "sb_one_time_available": "有 {count} 個一次性的值：按「一次性的值」查看，只能看一次。", + "sb_one_time_nested": ( + "這裡無法取得：在區塊內產生的值（{commands}）。" + "區塊不會記錄其內部指令的結果。" + ), "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 91818698..0a380500 100644 --- a/je_auto_control/gui/script_builder/builder_tab.py +++ b/je_auto_control/gui/script_builder/builder_tab.py @@ -2,10 +2,16 @@ 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. """ import functools import json -from typing import Optional +from typing import List, Optional from PySide6.QtCore import Qt from PySide6.QtGui import QAction @@ -24,11 +30,12 @@ from je_auto_control.gui.script_builder.command_schema import ( CATEGORIES, COMMAND_SPECS, specs_in_category, ) +from je_auto_control.gui.script_builder.one_time_dialog import OneTimeValueDialog 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 ( - Step, actions_to_steps, displayable_record, load_action_file, save_action_file, - steps_to_actions, + OneTimeValue, Step, actions_to_steps, displayable_record, load_action_file, + nested_sensitive_commands, one_time_values, 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 @@ -58,6 +65,11 @@ def __init__(self, parent: Optional[QWidget] = None) -> None: # The other top-level keys of a loaded {"auto_control": [...]} file, # written back on Save; None for a bare list. 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] = [] + self._reveal_btn = QPushButton() + self._reveal_dialog: Optional[OneTimeValueDialog] = None self._runs = TabTask(self) self._runs.result.connect(self._show_run_result) self._runs.error.connect(self._show_run_error) @@ -70,7 +82,7 @@ def dispose(self) -> None: A script run is stopped; other work cannot be interrupted, so it runs to its end and its result is dropped. Called by ``close_tab(key, release=True)``; safe to call twice. """ - release_resources(self) + release_resources(self, self._forget_one_time) def retranslate(self) -> None: TranslatableMixin.retranslate(self) @@ -111,6 +123,10 @@ def _build_toolbar(self) -> QHBoxLayout: btn = self._tr(QPushButton(), key) btn.clicked.connect(handler) bar.addWidget(btn) + self._tr(self._reveal_btn, "sb_one_time_show") + self._reveal_btn.setEnabled(False) + self._reveal_btn.clicked.connect(self._on_reveal) + bar.addWidget(self._reveal_btn) return bar def _add_button(self) -> QToolButton: @@ -205,6 +221,8 @@ def _on_run(self) -> None: if not self._runs.start_script(functools.partial(_run_actions, actions)): self._result.setPlainText(_t("task_busy")) return + self._forget_one_time() + self._nested_marked = nested_sensitive_commands(actions) self._result.setPlainText(_t("task_running")) def _on_stop(self) -> None: @@ -213,8 +231,43 @@ def _on_stop(self) -> None: def _show_run_result(self, result: object) -> None: # Masked first: AC_user_add / AC_user_rotate_token answer with a token. - self._result.setPlainText(json.dumps( - displayable_record(result), indent=2, default=str, ensure_ascii=False)) + lines = [json.dumps(displayable_record(result), indent=2, default=str, ensure_ascii=False)] + self._one_time = one_time_values(result) + 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._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._reveal_btn.setEnabled(False) + dialog, self._reveal_dialog = self._reveal_dialog, None + if dialog is not None: + dialog.forget() + dialog.done(0) # emits ``finished`` even when it was never shown + + def _take_one_time_dialog(self) -> Optional[OneTimeValueDialog]: + """Build the reveal dialog for the pending values and forget them here; not shown yet.""" + if not self._one_time: + return None + values, self._one_time = self._one_time, [] + self._reveal_btn.setEnabled(False) + dialog = OneTimeValueDialog(values, self) + dialog.finished.connect(dialog.deleteLater) + dialog.finished.connect(self._on_reveal_closed) + self._reveal_dialog = dialog + return dialog + + def _on_reveal(self) -> None: + dialog = self._take_one_time_dialog() + if dialog is not None: + dialog.open() # window-modal without a nested event loop + + def _on_reveal_closed(self, _result: int = 0) -> None: + self._reveal_dialog = None def _show_run_error(self, error: object) -> None: if was_stopped(error): diff --git a/je_auto_control/gui/script_builder/command_schema.py b/je_auto_control/gui/script_builder/command_schema.py index 6b1cfb15..adaa7624 100644 --- a/je_auto_control/gui/script_builder/command_schema.py +++ b/je_auto_control/gui/script_builder/command_schema.py @@ -42,7 +42,8 @@ class CommandSpec: 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. + #: freshly issued token). The builder masks it before displaying a run and + #: offers it once through its one-time reveal. sensitive_result: bool = False @@ -2872,7 +2873,9 @@ def _add_misc_specs(specs: List[CommandSpec]) -> None: placeholder="HS256", choices=("HS256", "HS384", "HS512")), ), - description="Sign a compact JWT (HMAC) from claims; returns {token}.", + description="Sign a compact JWT (HMAC) from claims; returns {token}. This builder masks " + "the token -- read it with 'One-time values' after the run.", + sensitive_result=True, )) specs.append(CommandSpec( "AC_jwt_decode", "Security", "JWT: Verify Token", @@ -5344,7 +5347,7 @@ def _add_user_specs(specs: List[CommandSpec]) -> None: placeholder="default: JE_AUTOCONTROL_RBAC_USERS") roles = ("viewer", "operator", "admin") token_note = (" The token is in the run's record once; this builder masks it -- " - "run the step from a script, REST or the CLI to read it.") + "read it with 'One-time values' after the run (top-level steps only).") specs.append(CommandSpec( "AC_user_add", "Security", "User: Add", fields=( diff --git a/je_auto_control/gui/script_builder/one_time_dialog.py b/je_auto_control/gui/script_builder/one_time_dialog.py new file mode 100644 index 00000000..66d43ed4 --- /dev/null +++ b/je_auto_control/gui/script_builder/one_time_dialog.py @@ -0,0 +1,102 @@ +"""The Script Builder's one-time reveal of the secrets a run produced. + +``AC_user_add`` and ``AC_user_rotate_token`` answer with a bearer token that +exists nowhere else, and the builder masks it in its result pane. This dialog +is where the person who ran the step reads and copies it, once: each value is +in a read-only field labelled by its step -- the presentation the Users (RBAC) +group uses for a new token -- and closing the dialog empties the fields and +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. + +The tab opens it with ``open()`` (window-modal, not a nested event loop), only +when its "one-time values" button is pressed. +""" +from typing import List, Optional, Sequence + +from PySide6.QtGui import QGuiApplication +from PySide6.QtWidgets import ( + QDialog, QDialogButtonBox, QGridLayout, QLabel, QLineEdit, QPushButton, + QVBoxLayout, QWidget, +) + +from je_auto_control.gui.language_wrapper.multi_language_wrapper import ( + language_wrapper, +) +from je_auto_control.gui.script_builder.command_schema import COMMAND_SPECS +from je_auto_control.gui.script_builder.step_model import OneTimeValue + + +def _t(key: str) -> str: + return str(language_wrapper.translate(key, key)) + + +def _copy_to_clipboard(text: str) -> None: + """Put ``text`` on the clipboard; the one place a value leaves the dialog.""" + QGuiApplication.clipboard().setText(text) + + +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] + return " · ".join(part for part in parts if part) + + +class OneTimeValueDialog(QDialog): + """Shows each value of a run once; forgets them all when it closes.""" + + def __init__(self, values: Sequence[OneTimeValue], + parent: Optional[QWidget] = None) -> None: + super().__init__(parent) + self.setWindowTitle(_t("sb_one_time_title")) + self._values: List[OneTimeValue] = list(values) + self._fields: List[QLineEdit] = [] + root = QVBoxLayout(self) + notice = QLabel(_t("sb_one_time_notice")) + notice.setWordWrap(True) + root.addWidget(notice) + grid = QGridLayout() + for row, value in enumerate(self._values): + self._add_row(grid, row, value) + root.addLayout(grid) + buttons = QDialogButtonBox(QDialogButtonBox.StandardButton.Close) + buttons.rejected.connect(self.reject) + root.addWidget(buttons) + self.setMinimumWidth(560) + self.finished.connect(self.forget) + + def _add_row(self, grid: QGridLayout, row: int, value: OneTimeValue) -> None: + field = QLineEdit(value.value) + field.setReadOnly(True) + copy = QPushButton(_t("sb_one_time_copy")) + copy.clicked.connect(lambda _checked=False, index=row: self.copy_value(index)) + grid.addWidget(QLabel(value_label(value)), row, 0) + grid.addWidget(field, row, 1) + grid.addWidget(copy, row, 2) + self._fields.append(field) + + def labels(self) -> List[str]: + """The label of each value still held, in the order shown.""" + return [value_label(value) for value in self._values] + + def shown_values(self) -> List[str]: + """The text of each value field, in the order shown.""" + return [field.text() for field in self._fields] + + 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) + return True + + 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 = [] + + +__all__ = ["OneTimeValueDialog", "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 f4d43bd0..091be160 100644 --- a/je_auto_control/gui/script_builder/step_model.py +++ b/je_auto_control/gui/script_builder/step_model.py @@ -1,11 +1,14 @@ """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``. +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. """ +import json import re from dataclasses import dataclass, field -from typing import Any, Dict, List, Mapping, Optional, Tuple +from typing import Any, Dict, Iterator, List, Mapping, Optional, Set, 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 @@ -63,15 +66,105 @@ def displayable_record(record: Any) -> Any: return record shown: Dict[Any, Any] = {} for key, value in record.items(): - match = _RECORD_COMMAND.match(key) if isinstance(key, str) else None - spec = COMMAND_SPECS.get(match.group(1)) if match is not None else None - if spec is not None and spec.sensitive_result: + if _marked_command(key) is not None: shown[key] = _without_secrets(value) else: shown[key] = displayable_record(value) return shown +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 + + +@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. + """ + step: int + command: str + subject: str + name: str + value: str = field(repr=False) + + +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. + """ + 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): diff --git a/je_auto_control/utils/executor/action_executor.py b/je_auto_control/utils/executor/action_executor.py index bdb93746..25468817 100644 --- a/je_auto_control/utils/executor/action_executor.py +++ b/je_auto_control/utils/executor/action_executor.py @@ -8517,7 +8517,8 @@ def _execute_list(self, action_list: Union[list, dict], raise_on_error: bool, signal, execute_record_dict, raise_on_error, key) for key, value in execute_record_dict.items(): - autocontrol_logger.info("%s -> %s", key, value) + # 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)) return execute_record_dict @staticmethod diff --git a/pyproject.toml b/pyproject.toml index a531e43e..ce4e523b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -364,6 +364,7 @@ module = [ "je_auto_control.gui.remote_desktop.webrtc_viewer_files", "je_auto_control.gui.remote_desktop.webrtc_viewer_media", "je_auto_control.gui.remote_desktop.webrtc_viewer_ui", + "je_auto_control.gui.script_builder.one_time_dialog", "je_auto_control.gui.tab_registry", "je_auto_control.gui.task_controller", "je_auto_control.gui.theme", 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 new file mode 100644 index 00000000..43c35b04 --- /dev/null +++ b/test/unit_test/headless/test_script_builder_one_time_reveal.py @@ -0,0 +1,400 @@ +"""The Script Builder reveals a run's freshly issued token once, and only on screen. + +``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. + +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. +""" +import json +import logging +from pathlib import Path + +import pytest + +from je_auto_control.gui.script_builder.step_model import ( + OneTimeValue, Step, displayable_record, nested_sensitive_commands, one_time_values, + save_action_file, steps_to_actions, +) +from je_auto_control.utils.executor.action_executor import execute_action +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'}]" + + +def _user_steps(users: str) -> list: + return [ + Step("AC_user_add", {"user_id": "ann", "role": "operator", "users_path": users}), + Step("AC_user_list", {"users_path": users}), + Step("AC_user_rotate_token", {"user_id": "ann", "users_path": users}), + ] + + +# --- 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) + 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" + + +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") + assert "tok-in-memory-only" not in repr(value) + str(value) + f"{[value]}" + + +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"]}) + 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_everything_revealed_is_exactly_what_the_display_masks(tmp_path): + users = str(tmp_path / "users.json") + record = execute_action(steps_to_actions(_user_steps(users))) + 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 + + +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_block_really_records_no_nested_token(tmp_path): + """Why the note exists: the value issued in a body is in no record to reveal.""" + 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) == [] + + +def test_a_signed_jwt_is_masked_revealed_once_and_kept_out_of_the_log(caplog): + """The schema flag was missing: the builder printed the credential it had just minted.""" + 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}]]) + 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)] + logged = "\n".join(entry.getMessage() for entry in caplog.records) + assert "AC_jwt_encode" in logged and token not in logged + + +def test_a_result_is_logged_with_its_secret_named_fields_masked_at_any_depth(caplog): + """The result log printed a lease token; the record a script receives is untouched.""" + autocontrol_logger.addHandler(caplog.handler) + try: + with caplog.at_level(logging.DEBUG, logger=autocontrol_logger.name): + record = execute_action([["AC_lease_secret", {"name": "db", "ttl": 5}], ["AC_lease_active"]]) + finally: + autocontrol_logger.removeHandler(caplog.handler) + issued, active = record.values() + assert issued["token"] in [lease["token"] for lease in active["leases"]] + logged = "\n".join(entry.getMessage() for entry in caplog.records) + assert "AC_lease_active" in logged and "'name': 'db'" in logged + assert issued["token"] not in logged + execute_action([["AC_revoke_lease", {"token": issued["token"]}]]) + + +def test_the_extraction_is_free_of_qt(): + import je_auto_control.gui.script_builder.step_model as step_model + assert "PySide6" not in Path(step_model.__file__).read_text(encoding="utf-8") + + +def test_the_strings_exist_in_every_catalogue_with_the_same_placeholders(): + import re + + from je_auto_control.gui.language_wrapper.english import english_word_dict + from je_auto_control.gui.language_wrapper.japanese import japanese_word_dict + from je_auto_control.gui.language_wrapper.simplified_chinese import simplified_chinese_word_dict + from je_auto_control.gui.language_wrapper.traditional_chinese import traditional_chinese_word_dict + + 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): + for key in _KEYS: + assert catalogue[key].strip() + assert placeholders(catalogue[key]) == placeholders(english_word_dict[key]), key + + +# --- the tab, offscreen ------------------------------------------------------ + + +@pytest.fixture() +def builder(monkeypatch): + """A Script Builder tab, its module, and the list standing in for the clipboard.""" + pytest.importorskip("PySide6.QtWidgets", exc_type=ImportError) + from PySide6.QtWidgets import QApplication + + from je_auto_control.gui.script_builder import builder_tab, one_time_dialog + + app = QApplication.instance() + if app is None: + app = QApplication([]) + copied: list = [] + monkeypatch.setattr(one_time_dialog, "_copy_to_clipboard", copied.append) + + 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 + finally: + tab.dispose() + tab.deleteLater() + app.processEvents() + + +def _run(tab, steps) -> None: + from headless._qt_settle import settle + + tab._tree.load_steps(steps) + tab._on_run() + assert settle(tab._runs, "task") + + +def _translated(key: str, **fields) -> str: + from je_auto_control.gui.language_wrapper.multi_language_wrapper import language_wrapper + return language_wrapper.translate(key, key).format(**fields) + + +def test_the_result_pane_masks_and_says_values_are_available(builder, tmp_path): + tab, _module, _copied = 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() + tokens = [value.value for value in tab._one_time] + assert len(tokens) == 2 and all(len(token) > 20 for token in tokens) + assert not any(token in text for token in tokens) + assert '"token": "***"' in text and "ann" in text, "the masked record is still shown" + assert _translated("sb_one_time_available", count=2) in text + assert tab._reveal_btn.isEnabled() + assert tab._reveal_dialog is None, "nothing opens by itself" + + +def test_the_dialog_shows_each_value_labelled_by_its_step_and_only_once(builder, tmp_path): + tab, _module, copied = builder + users = str(tmp_path / "users.json") + _run(tab, _user_steps(users)) + tokens = [value.value for value in tab._one_time] + dialog = tab._take_one_time_dialog() + assert dialog.shown_values() == tokens + assert dialog.labels() == [ + " · ".join((_translated("sb_one_time_step", step=1), "User: Add", "ann", "token")), + " · ".join((_translated("sb_one_time_step", step=3), "User: Rotate Token", "ann", "token")), + ] + 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 not dialog.copy_value(5) + + +def test_closing_the_dialog_forgets_every_value(builder, tmp_path): + tab, _module, copied = 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 tab._reveal_dialog is None and tab._one_time == [] + assert tab._take_one_time_dialog() is None + + +def test_the_button_opens_the_dialog_without_a_nested_event_loop(builder, tmp_path, monkeypatch): + tab, _module, _copied = builder + from je_auto_control.gui.script_builder.one_time_dialog import OneTimeValueDialog + + opened: list = [] + monkeypatch.setattr(OneTimeValueDialog, "open", lambda dialog: opened.append(dialog)) + tab._reveal_btn.click() + assert opened == [], "a click with nothing pending opens nothing" + _run(tab, _user_steps(str(tmp_path / "users.json"))) + tab._reveal_btn.click() + assert opened == [tab._reveal_dialog] and len(opened[0].shown_values()) == 2 + opened[0].reject() + + +def test_the_next_run_drops_values_nobody_revealed(builder, tmp_path): + tab, _module, _copied = builder + users = str(tmp_path / "users.json") + _run(tab, _user_steps(users)) + assert len(tab._one_time) == 2 + _run(tab, [Step("AC_user_list", {"users_path": users})]) + assert tab._one_time == [] and not tab._reveal_btn.isEnabled() + assert _translated("sb_one_time_available", count=2) not in tab._result.toPlainText() + + +def test_the_next_run_empties_a_dialog_still_holding_values(builder, tmp_path): + tab, _module, copied = 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 tab._reveal_dialog is None + + +def test_disposing_the_tab_forgets_pending_values_and_the_dialog(builder, tmp_path): + tab, _module, _copied = builder + users = str(tmp_path / "users.json") + _run(tab, _user_steps(users)) + tab.dispose() + assert tab._one_time == [] and not tab._reveal_btn.isEnabled() + _run(tab, _user_steps(users)[2:]) + dialog = tab._take_one_time_dialog() + tab.dispose() + assert dialog.shown_values() == [""] + + +def test_a_failed_step_offers_nothing(builder, tmp_path): + tab, _module, _copied = 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})]) + assert tab._one_time == [] and not tab._reveal_btn.isEnabled() + 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() + + +# --- the value reaches nothing that is kept ---------------------------------- + + +class _Writes: + """Everything handed to a store that persists, as text.""" + + def __init__(self) -> None: + self.seen: list = [] + + def recorder(self, name: str, result=True): + def record(_self, *args, **kwargs): + self.seen.append(f"{name} {args!r} {kwargs!r} {json.dumps([args, kwargs], default=str)}") + return result + return record + + +def test_the_token_reaches_no_log_record_journal_history_script_or_setting( + builder, tmp_path, monkeypatch, caplog): + from je_auto_control.gui.window_settings import WindowSettings + from je_auto_control.utils.action_journal.recorder import ( + start_action_journal, stop_action_journal, + ) + 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 + users = str(tmp_path / "users.json") + journal = tmp_path / "journal.jsonl" + saved = tmp_path / "saved.json" + writes = _Writes() + for name in ("start_run", "finish_run", "attach_artifact", "link_journal"): + monkeypatch.setattr(HistoryStore, name, writes.recorder(f"history.{name}", result=1)) + for name in ("save", "save_form"): + 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", []) + 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)) + 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() + 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" + + outputs = { + "result pane before the reveal": pane_before, + "result pane after the reveal": tab._result.toPlainText(), + "log": "\n".join(entry.getMessage() for entry in caplog.records), + "test record": repr(test_record_instance.test_record_list), + "run history and GUI settings writes": "\n".join(writes.seen), + "the tab's steps": json.dumps(steps_to_actions(tab._tree.root_steps())), + } + for path in sorted(tmp_path.rglob("*")): + if path.is_file(): + outputs[f"file {path.name}"] = path.read_bytes().decode("utf-8", errors="replace") + assert "AC_user_add" in outputs["log"], "the run was logged" + 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" + for name, text in outputs.items(): + for token in tokens: + assert token not in text, f"the token reached: {name}" + + +def test_the_reveal_code_names_no_store_that_persists(): + """The dialog and the tab hold the values; neither module can write them anywhere.""" + from je_auto_control.gui.script_builder import builder_tab, one_time_dialog + + dialog_source = Path(one_time_dialog.__file__).read_text(encoding="utf-8") + for word in ("autocontrol_logger", "QSettings", "window_settings", "run_history", + "test_record", "action_journal", "open(", "write"): + assert word not in dialog_source.replace("dialog.open()", "").replace("``open()``", ""), word + tab_source = Path(builder_tab.__file__).read_text(encoding="utf-8") + for word in ("autocontrol_logger", "QSettings", "window_settings", "run_history", "test_record"): + assert word not in tab_source, word 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 0c72ece6..8a426396 100644 --- a/test/unit_test/headless/test_script_builder_user_commands.py +++ b/test/unit_test/headless/test_script_builder_user_commands.py @@ -45,7 +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} - assert marked == _TOKEN_COMMANDS + # AC_jwt_encode mints a bearer credential the same way: it exists only in the reply. + assert marked == _TOKEN_COMMANDS | {"AC_jwt_encode"} def test_a_run_record_is_shown_without_the_token(tmp_path): From 641919e40428f60e9d3cf52a7bbd61860f69a6c7 Mon Sep 17 00:00:00 2001 From: JeffreyChen Date: Fri, 9 Oct 2026 11:42:20 +0800 Subject: [PATCH 3/7] Let a plug-in declare a tool read-only and require read_data for stored records A read-only MCP server ran no plug-in tool, because every plug-in tool was registered as mutating. A callable with mcp_read_only = True is now registered read-only; the declaration is trusted, not verified, so it is logged at registration and anything but the boolean True stays mutating. DATA_TOOLS follows one rule: records or configuration the host keeps need read_data, the live screen needs read_screen. Run history, traces, the cost ledger, journals, healing logs and the USB ACL move to read_data, so a viewer no longer reads them over MCP. --- .../Eng/doc/mcp_server/mcp_server_doc.rst | 85 +++++- .../Zh/doc/mcp_server/mcp_server_doc.rst | 62 ++++- .../utils/mcp_server/plugin_watcher.py | 3 +- .../utils/mcp_server/tools/plugin_tools.py | 68 ++++- je_auto_control/utils/rbac/policy.py | 26 +- .../test_mcp_plugin_read_only_declaration.py | 258 ++++++++++++++++++ .../headless/test_rbac_user_admin.py | 57 +++- 7 files changed, 545 insertions(+), 14 deletions(-) create mode 100644 test/unit_test/headless/test_mcp_plugin_read_only_declaration.py 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 a570cb62..b83b7e6e 100644 --- a/docs/source/Eng/doc/mcp_server/mcp_server_doc.rst +++ b/docs/source/Eng/doc/mcp_server/mcp_server_doc.rst @@ -337,6 +337,38 @@ bearer token and is never subject to RBAC. ``DATA_TOOLS`` in ``je_auto_control.utils.rbac.policy``. A ``viewer`` keeps every other read-only tool: screen size, windows, pixels, image and text location, accessibility reads, waits. +- The rule behind that list: a tool that returns **records or configuration + the host keeps** needs ``read_data``; a tool that only observes the live + screen needs ``read_screen``. So ``read_data`` also covers run history and + what is computed from it (``ac_list_run_history``, ``ac_flaky_report``, + ``ac_rank_tests``, ``ac_select_tests``, ``ac_shard_suite``); agent traces + and the cost ledger (``ac_trace_export``, ``ac_trace_summary``, + ``ac_costs_list``, ``ac_costs_summary``); action journals + (``ac_journal_read``, ``ac_journal_runs``); the self-healing log, its + statistics, stored revisions and datasets (``ac_self_heal_log_list``, + ``ac_heal_stats``, ``ac_self_heal_revision_list``, + ``ac_self_heal_evaluate``); locator strategy history, repairs and saved + libraries (``ac_ab_report``, ``ac_ab_best_strategy``, ``ac_repair_pending``, + ``ac_repair_resolved``, ``ac_element_list``, ``ac_skill_list``, + ``ac_skill_search``); stored policy and state (``ac_usb_acl_list``, + ``ac_lease_active``, ``ac_quarantine_list``, ``ac_pending_artifacts``, + ``ac_config_sync_status``); and a connected device's clipboard + (``ac_android_get_clipboard``, ``ac_ios_get_clipboard``). These were + ``read_screen`` before, so **a viewer token can no longer call them**; + ``operator`` and ``admin`` are unaffected. +- What a call costs is not a capability. ``ac_vlm_locate`` and + ``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. +- 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``, + ``ac_watchdog_list``, ``ac_journal_status``, the USB device lists -- stay + ``read_screen``. - ``tools/list`` returns only the tools the caller may call, and ``tools/call`` on any other answers JSON-RPC error ``-32003`` (``Forbidden: ...``, ``data.required_capability``) without running it. @@ -669,11 +701,12 @@ that is not marked read-only and is registered on a running read-only server out of ``tools/list`` and a call to it is answered ``-32602`` and recorded in the audit log as ``denied``. In the default ``full`` mode the filter used to run only when the registry was built, so such a tool was listed and ran. -Plugin tools are registered as destructive, so **a read-only server runs no -plugin tool**; a tool your own code registers with ``readOnlyHint`` true is -still offered. ``MCPServer(tools=[...])`` with the variable set is filtered -the same way -- pass ``read_only=False`` to an embedded server that must -ignore the variable. +Plugin tools are registered as destructive unless the plugin declares +otherwise, so **a read-only server runs no plugin tool that has not declared +itself read-only** (see `Plugin hot-reload`_); a tool your own code registers +with ``readOnlyHint`` true is still offered. ``MCPServer(tools=[...])`` with +the variable set is filtered the same way -- pass ``read_only=False`` to an +embedded server that must ignore the variable. Confining file arguments to root directories ============================================ @@ -882,6 +915,48 @@ Each register / unregister fires ``notifications/tools/list_changed`` so the client refreshes its cached catalogue automatically. +**Declaring a plugin tool read-only.** A plugin tool is registered as +mutating: it is absent from a read-only server and needs ``drive_input`` +under RBAC. A plugin opts one of its callables out by setting the attribute +``mcp_read_only`` to ``True`` on it: + +.. code-block:: python + + def AC_queue_depth(): + """How many jobs are waiting.""" + return {"depth": len(_pending)} + + AC_queue_depth.mcp_read_only = True + +The tool is then registered with ``readOnlyHint`` true. It is listed and +runs on a read-only server in all three tool modes (``full``, +``progressive`` -- once enabled -- and ``static``, when the profile names +it), and under RBAC it needs only ``read_screen``, so a ``viewer`` may call +it. Only the read-only hint is taken from the plugin; the tool is not +marked idempotent. + +.. warning:: + + The declaration is **trusted, not verified**. It is the plugin author's + word: the server cannot tell what arbitrary Python does, and a callable + that declares itself read-only and then writes files or sends input will + do so on a read-only server and for a viewer. Declaring it does not + sandbox it. Review a plugin before giving it a directory a read-only or + viewer-facing server watches. Each time such a tool is registered the + server logs a warning naming it (``plugin tool 'plugin_ac_queue_depth' is + registered read-only because its plugin declared mcp_read_only=True; the + declaration is trusted, not verified``). + +Anything other than the boolean ``True`` is not a declaration: ``"true"``, +``1``, a callable, an attribute that raises. Such a value is logged and the +tool stays mutating, as does a tool with no attribute or with ``False``. +The same rule applies to tools registered through +``register_plugin_tools(server, commands)`` and ``make_plugin_tool``; the +attribute's name is ``PLUGIN_READ_ONLY_ATTRIBUTE`` in +``je_auto_control.utils.mcp_server.tools.plugin_tools``. The declaration +says nothing to the ``AC_*`` executor command of the same name, and the path +roots still do not reach a plugin tool's arguments. + CI smoke tests with the fake backend ==================================== 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 060b2830..b1d13db4 100644 --- a/docs/source/Zh/doc/mcp_server/mcp_server_doc.rst +++ b/docs/source/Zh/doc/mcp_server/mcp_server_doc.rst @@ -306,6 +306,31 @@ Bearer token 也可從 ``JE_AUTOCONTROL_MCP_TOKEN`` 環境變數讀取。 ``ac_assert_audio``）。完整清單是 ``je_auto_control.utils.rbac.policy`` 的 ``DATA_TOOLS``。其餘唯讀工具 ``viewer`` 都保留：螢幕尺寸、視窗、像素、影像與文字 定位、無障礙讀取、等待。 +- 這份清單背後的規則：回傳\ **主機保存的紀錄或設定**\ 的工具需要 ``read_data``； + 只觀察目前畫面的工具需要 ``read_screen``。所以 ``read_data`` 也包含執行歷史與由它 + 算出來的結果（``ac_list_run_history``、``ac_flaky_report``、``ac_rank_tests``、 + ``ac_select_tests``、``ac_shard_suite``）；agent trace 與費用帳本 + （``ac_trace_export``、``ac_trace_summary``、``ac_costs_list``、 + ``ac_costs_summary``）；動作日誌（``ac_journal_read``、``ac_journal_runs``）； + 自愈紀錄、其統計、保存的 revision 與資料集（``ac_self_heal_log_list``、 + ``ac_heal_stats``、``ac_self_heal_revision_list``、``ac_self_heal_evaluate``）； + 定位策略歷史、修復與保存的函式庫（``ac_ab_report``、``ac_ab_best_strategy``、 + ``ac_repair_pending``、``ac_repair_resolved``、``ac_element_list``、 + ``ac_skill_list``、``ac_skill_search``）；保存的政策與狀態（``ac_usb_acl_list``、 + ``ac_lease_active``、``ac_quarantine_list``、``ac_pending_artifacts``、 + ``ac_config_sync_status``）；以及連線裝置的剪貼簿（``ac_android_get_clipboard``、 + ``ac_ios_get_clipboard``）。這些以前是 ``read_screen``，所以\ **viewer 的 token + 現在不能呼叫它們**\ ；``operator`` 與 ``admin`` 不受影響。 +- 一次呼叫要花多少錢不是一種能力。``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。 +- 回報這個行程目前狀態、而不是保存紀錄的工具——``ac_list_executions``、 + ``ac_scheduler_list_jobs``、``ac_trigger_list``、``ac_hotkey_list``、 + ``ac_observe_list``、``ac_watchdog_list``、``ac_journal_status``、USB 裝置清單—— + 維持 ``read_screen``。 - ``tools/list`` 只回呼叫者可以呼叫的工具；對其他工具 ``tools/call`` 會回 JSON-RPC 錯誤 ``-32003``\ （``Forbidden: ...``、``data.required_capability``），且不會執行。 - 接受動作清單的工具（``ac_execute_actions`` 等）在清單含有呼叫者角色沒有的指令時 @@ -572,7 +597,8 @@ process 內量測兩種模式。以 680 個工具的 registry 量測(2026-10-09, 標為唯讀的工具(透過 ``register_tool``、``ac_load_plugins`` 或 plugin watcher),不會 出現在 ``tools/list``,呼叫它會得到 ``-32602`` 並在稽核 log 記為 ``denied``。預設的 ``full`` 模式以前只在建立 registry 時過濾,所以這種工具會被列出、也會執行。Plugin -工具一律註冊為破壞性,因此\ **唯讀伺服器不會執行任何 plugin 工具**;你自己的程式以 +工具除非自己宣告,否則註冊為破壞性,因此\ **唯讀伺服器不會執行沒有宣告自己唯讀的 +plugin 工具**\ (見下方「Plugin Hot-Reload」);你自己的程式以 ``readOnlyHint`` 為 true 註冊的工具仍然會提供。設了該環境變數時, ``MCPServer(tools=[...])`` 也會被同樣過濾——內嵌的伺服器若必須忽略該變數,請傳 ``read_only=False``。 @@ -750,6 +776,40 @@ Plugin Hot-Reload 每次 register / unregister 都會送出 ``notifications/tools/list_changed``,client 會自動更新工具目錄。 +**宣告 plugin 工具為唯讀。** Plugin 工具預設註冊為會變更狀態的工具:唯讀伺服器不提供它, +在 RBAC 下需要 ``drive_input``。Plugin 可以在某個 callable 上把屬性 ``mcp_read_only`` +設為 ``True``,讓它例外: + +.. code-block:: python + + def AC_queue_depth(): + """有多少工作在等待。""" + return {"depth": len(_pending)} + + AC_queue_depth.mcp_read_only = True + +這個工具就會以 ``readOnlyHint`` 為 true 註冊。唯讀伺服器在三種工具模式下都會列出並執行它 +(``full``、``progressive`` —— 啟用之後 —— 以及 profile 有列它的 ``static``),在 RBAC 下 +只需要 ``read_screen``,所以 ``viewer`` 可以呼叫。從 plugin 取用的只有唯讀這一項; +工具不會被標成 idempotent。 + +.. warning:: + + 這個宣告是\ **被信任的,不是被驗證的**。它只是 plugin 作者的說法:伺服器無從得知 + 任意的 Python 程式做了什麼,一個宣告自己唯讀、實際上卻寫檔或送出輸入的 callable, + 在唯讀伺服器上、對 viewer 也照樣會那樣做。宣告不會把它關進沙箱。把 plugin 放進 + 唯讀或給 viewer 用的伺服器所監看的目錄之前,請先審查它。每次註冊這樣的工具, + 伺服器都會記一筆指名它的 warning(``plugin tool 'plugin_ac_queue_depth' is + registered read-only because its plugin declared mcp_read_only=True; the + declaration is trusted, not verified``)。 + +布林值 ``True`` 以外的任何值都不算宣告:``"true"``、``1``、callable、讀取時丟例外的屬性。 +這樣的值會被記錄,工具維持會變更狀態;沒有這個屬性或值為 ``False`` 的工具也一樣。 +透過 ``register_plugin_tools(server, commands)`` 與 ``make_plugin_tool`` 註冊的工具適用 +同一條規則;屬性名稱是 ``je_auto_control.utils.mcp_server.tools.plugin_tools`` 的 +``PLUGIN_READ_ONLY_ATTRIBUTE``。這個宣告不影響同名的 ``AC_*`` executor 指令, +路徑根目錄也仍然管不到 plugin 工具的參數。 + CI 煙霧測試 (Fake Backend) ========================== diff --git a/je_auto_control/utils/mcp_server/plugin_watcher.py b/je_auto_control/utils/mcp_server/plugin_watcher.py index 780b3521..1f95efa3 100644 --- a/je_auto_control/utils/mcp_server/plugin_watcher.py +++ b/je_auto_control/utils/mcp_server/plugin_watcher.py @@ -12,7 +12,7 @@ from je_auto_control.utils.logging.logging_instance import autocontrol_logger from je_auto_control.utils.mcp_server.tools.plugin_tools import ( - make_plugin_tool, + log_read_only_declaration, make_plugin_tool, ) @@ -107,6 +107,7 @@ def _reload_file(self, path: str, mtime: float) -> None: registered: List[str] = [] for tool in tools: self._server.register_tool(tool) + log_read_only_declaration(tool) registered.append(tool.name) self._known[path] = (mtime, registered) autocontrol_logger.info( diff --git a/je_auto_control/utils/mcp_server/tools/plugin_tools.py b/je_auto_control/utils/mcp_server/tools/plugin_tools.py index 7d350b31..1d42b3c3 100644 --- a/je_auto_control/utils/mcp_server/tools/plugin_tools.py +++ b/je_auto_control/utils/mcp_server/tools/plugin_tools.py @@ -5,14 +5,66 @@ dynamic catalogue into the live MCP server so a plugin a user drops into their plugin directory shows up as a tool the model can call, and the client gets notified to refresh its tool list. + +A plugin tool is registered as mutating (``DESTRUCTIVE``) unless the plugin +says otherwise: a callable whose ``mcp_read_only`` attribute is exactly +``True`` is registered read-only, which is what lets it run on a read-only +server and makes it need only ``read_screen`` under RBAC. That is the plugin +author's word and nothing checks it -- the server cannot tell what arbitrary +Python does -- so each declaration is logged when the tool is registered. +Anything other than the boolean ``True`` (``"yes"``, ``1``, a callable, an +attribute that raises) leaves the tool mutating. """ import inspect from typing import Any, Callable, Dict, List +from je_auto_control.utils.logging.logging_instance import autocontrol_logger from je_auto_control.utils.mcp_server.tools._base import ( - DESTRUCTIVE, MCPTool, schema, + DESTRUCTIVE, MCPTool, MCPToolAnnotations, schema, ) +#: Attribute a plugin sets to ``True`` on a callable to declare it read-only. +PLUGIN_READ_ONLY_ATTRIBUTE = "mcp_read_only" + +#: Annotations of a plugin tool that declared itself read-only. Only the +#: read-only hint is the plugin's claim; idempotence was not declared. +PLUGIN_READ_ONLY = MCPToolAnnotations(read_only=True) + +_UNSET = object() + + +def plugin_declares_read_only(handler: Callable[..., Any]) -> bool: + """Whether ``handler`` carries ``mcp_read_only = True``, and nothing looser. + + The declaration is trusted, not verified. A value that is present but is + not a boolean is logged and treated as no declaration, so a typo fails + towards the tool staying mutating. + """ + try: + declared = getattr(handler, PLUGIN_READ_ONLY_ATTRIBUTE, _UNSET) + except Exception as error: # noqa: BLE001 # reason: untrusted plugin object; raising means undeclared + autocontrol_logger.warning( + "plugin %r: reading %s raised %r; the tool stays mutating", + getattr(handler, "__name__", handler), PLUGIN_READ_ONLY_ATTRIBUTE, error) + return False + if declared is True: + return True + if declared is not _UNSET and declared is not False: + autocontrol_logger.warning( + "plugin %r: %s must be the boolean True, not %s; the tool stays mutating", + getattr(handler, "__name__", handler), PLUGIN_READ_ONLY_ATTRIBUTE, + type(declared).__name__) + return False + + +def log_read_only_declaration(tool: MCPTool) -> None: + """Log that the plugin tool ``tool`` is registered on its own read-only claim.""" + if tool.annotations.read_only: + autocontrol_logger.warning( + "plugin tool %r is registered read-only because its plugin declared " + "%s=True; the declaration is trusted, not verified", + tool.name, PLUGIN_READ_ONLY_ATTRIBUTE) + def make_plugin_tool(name: str, handler: Callable[..., Any], @@ -22,7 +74,8 @@ def make_plugin_tool(name: str, The schema is derived from ``inspect.signature(handler)``: every parameter becomes a property, parameters without defaults are marked required, and a parameter named ``ctx`` is excluded so - progress / cancellation context plumbing keeps working. + progress / cancellation context plumbing keeps working. The tool is + mutating unless :func:`plugin_declares_read_only` says otherwise. """ properties, required = _properties_from_signature(handler) tool_name = f"plugin_{name.lower()}" if not name.lower().startswith( @@ -37,7 +90,7 @@ def make_plugin_tool(name: str, description=desc, input_schema=schema(properties, required=required or None), handler=handler, - annotations=DESTRUCTIVE, + annotations=PLUGIN_READ_ONLY if plugin_declares_read_only(handler) else DESTRUCTIVE, ) @@ -45,12 +98,14 @@ def register_plugin_tools(server, commands: Dict[str, Callable[..., Any]] ) -> List[str]: """Wrap each entry in ``commands`` and add it to ``server``. - Returns the list of MCP tool names that were registered. + Returns the list of MCP tool names that were registered. A tool whose + plugin declared it read-only is logged by name. """ registered: List[str] = [] for raw_name, handler in commands.items(): tool = make_plugin_tool(raw_name, handler) server.register_tool(tool) + log_read_only_declaration(tool) registered.append(tool.name) return registered @@ -86,4 +141,7 @@ def _properties_from_signature(handler: Callable[..., Any] return properties, required -__all__ = ["make_plugin_tool", "register_plugin_tools"] +__all__ = [ + "PLUGIN_READ_ONLY", "PLUGIN_READ_ONLY_ATTRIBUTE", "log_read_only_declaration", + "make_plugin_tool", "plugin_declares_read_only", "register_plugin_tools", +] diff --git a/je_auto_control/utils/rbac/policy.py b/je_auto_control/utils/rbac/policy.py index e836bb55..1f214d55 100644 --- a/je_auto_control/utils/rbac/policy.py +++ b/je_auto_control/utils/rbac/policy.py @@ -127,12 +127,36 @@ #: Read-only MCP tools that expose data rather than screen state. "Read-only" #: says a tool changes nothing; it does not say that what it returns is #: something a viewer of the desktop was meant to see. +#: +#: The rule: a tool that returns records or configuration the host keeps -- +#: history, traces, cost ledgers, healing logs, journals, saved libraries, ACL +#: contents -- is listed here; a tool that only observes the live screen (or +#: computes on what the caller sent) is not. What a tool costs to run is a +#: separate question this table does not answer: ``ac_vlm_locate`` and +#: ``ac_self_heal_locate`` read the screen and stay ``read_screen`` although a +#: call may be billed by a model provider. DATA_TOOLS = frozenset({ - # the clipboard and its history + # the clipboard and its history, a connected device's included "ac_get_clipboard", "ac_get_clipboard_csv", "ac_get_clipboard_files", "ac_get_clipboard_html", "ac_get_clipboard_image", "ac_get_clipboard_rtf", "ac_clipboard_formats", "ac_assert_clipboard", "ac_clip_history_list", "ac_clip_history_search", + "ac_android_get_clipboard", "ac_ios_get_clipboard", + # records the host keeps of what ran: run history and what is derived + # from it, agent traces, the cost ledger, action journals + "ac_list_run_history", "ac_flaky_report", "ac_rank_tests", "ac_select_tests", + "ac_shard_suite", "ac_trace_export", "ac_trace_summary", + "ac_costs_list", "ac_costs_summary", "ac_journal_read", "ac_journal_runs", + # the self-healing log, its statistics, stored revisions and datasets; + # locator strategy history and repairs; saved locators and skills + "ac_self_heal_log_list", "ac_heal_stats", "ac_self_heal_revision_list", + "ac_self_heal_evaluate", "ac_ab_report", "ac_ab_best_strategy", + "ac_repair_pending", "ac_repair_resolved", "ac_element_list", + "ac_skill_list", "ac_skill_search", + # stored policy and state: the USB ACL, lease tokens, quarantine, + # artifacts awaiting approval, the recorded config-sync state + "ac_usb_acl_list", "ac_lease_active", "ac_quarantine_list", + "ac_pending_artifacts", "ac_config_sync_status", # files: their content, or an oracle on it (substring, digest) "ac_load_dotenv", "ac_load_data", "ac_read_action_file", "ac_read_document", "ac_read_presentation", "ac_read_workbook", "ac_extract_pdf_text", diff --git a/test/unit_test/headless/test_mcp_plugin_read_only_declaration.py b/test/unit_test/headless/test_mcp_plugin_read_only_declaration.py new file mode 100644 index 00000000..d63667bb --- /dev/null +++ b/test/unit_test/headless/test_mcp_plugin_read_only_declaration.py @@ -0,0 +1,258 @@ +"""A plugin may declare a tool read-only; nothing else makes a plugin tool read-only. + +Read-only mode is enforced in every tool mode and plugin tools were always +registered as mutating, so a read-only MCP server ran no plugin tool at all. +``mcp_read_only = True`` on the plugin's callable is the opt-in. It is the +plugin author's word -- trusted, not verified -- so it is logged at +registration, and anything but the boolean ``True`` leaves the tool mutating. +""" +import json +import logging + +import pytest + +from je_auto_control.utils.logging.logging_instance import autocontrol_logger +from je_auto_control.utils.mcp_server._authz import visible_tools +from je_auto_control.utils.mcp_server.plugin_watcher import PluginWatcher +from je_auto_control.utils.mcp_server.server import MCPServer +from je_auto_control.utils.mcp_server.tools.plugin_tools import ( + PLUGIN_READ_ONLY_ATTRIBUTE, make_plugin_tool, plugin_declares_read_only, + register_plugin_tools, +) +from je_auto_control.utils.rbac import ( + AuthorizationContext, Capability, Role, authorization_scope, capability_for_tool, +) + +_MODES = ["full", "progressive", "static"] +_PEEK = "plugin_ac_peek" +_POKE = "plugin_ac_poke" + + +@pytest.fixture(autouse=True) +def _no_ambient_configuration(monkeypatch): + for name in ("JE_AUTOCONTROL_MCP_READONLY", "JE_AUTOCONTROL_MCP_TOOL_MODE", + "JE_AUTOCONTROL_MCP_AUDIT"): + monkeypatch.delenv(name, raising=False) + # The static mode offers a fixed profile; name the two plugin tools in it. + monkeypatch.setenv("JE_AUTOCONTROL_MCP_TOOL_PROFILE", f"{_PEEK},{_POKE}") + + +@pytest.fixture() +def log_records(): + """Records the framework logger emits (it does not propagate to caplog).""" + records = [] + + class _Collect(logging.Handler): + def emit(self, record): + records.append(record) + + handler = _Collect(level=logging.DEBUG) + autocontrol_logger.addHandler(handler) + try: + yield records + finally: + autocontrol_logger.removeHandler(handler) + + +class _Plugin: + """Two plugin callables that record their calls instead of doing anything.""" + + def __init__(self, declaration=True): + self.ran = [] + ran = self.ran + + def AC_peek(): + """Report something without changing it.""" + ran.append("peek") + return {"seen": True} + + def AC_poke(): + """Change something.""" + ran.append("poke") + return {"poked": True} + + setattr(AC_peek, PLUGIN_READ_ONLY_ATTRIBUTE, declaration) + self.commands = {"AC_peek": AC_peek, "AC_poke": AC_poke} + + +def _rpc(server, method, params=None): + reply = server.handle_line(json.dumps( + {"jsonrpc": "2.0", "id": 1, "method": method, "params": params or {}})) + return json.loads(reply) + + +def _listed(server): + return {tool["name"] for tool in _rpc(server, "tools/list")["result"]["tools"]} + + +def _call(server, name, arguments=None): + return _rpc(server, "tools/call", {"name": name, "arguments": arguments or {}}) + + +def _enable(server, mode, *names): + if mode == "progressive": + reply = _call(server, "ac_tools_enable", {"names": list(names)}) + assert "result" in reply, reply + + +def test_the_attribute_name_is_the_documented_one(): + assert PLUGIN_READ_ONLY_ATTRIBUTE == "mcp_read_only" + + +def test_a_plugin_tool_is_mutating_by_default(): + def AC_plain(): + return 1 + + tool = make_plugin_tool("AC_plain", AC_plain) + assert tool.annotations.read_only is False + assert tool.annotations.to_dict()["destructiveHint"] is True + assert capability_for_tool(tool.name, tool.annotations.read_only) == Capability.DRIVE_INPUT + + +def test_a_declared_tool_is_read_only_and_claims_nothing_else(): + plugin = _Plugin() + hints = make_plugin_tool("AC_peek", plugin.commands["AC_peek"]).annotations.to_dict() + assert hints["readOnlyHint"] is True and hints["destructiveHint"] is False + assert hints["idempotentHint"] is False, "only read-only was declared" + assert make_plugin_tool("AC_poke", plugin.commands["AC_poke"]).annotations.read_only is False + + +@pytest.mark.parametrize("declaration", [ + False, None, 1, 1.0, "true", "True", "yes", ["True"], {"read_only": True}, lambda: True, +]) +def test_anything_but_the_boolean_true_is_mutating(declaration): + plugin = _Plugin(declaration) + assert plugin_declares_read_only(plugin.commands["AC_peek"]) is False + assert make_plugin_tool("AC_peek", plugin.commands["AC_peek"]).annotations.read_only is False + + +def test_a_malformed_declaration_is_logged(log_records): + plugin = _Plugin("yes") + make_plugin_tool("AC_peek", plugin.commands["AC_peek"]) + assert any("mcp_read_only" in record.getMessage() and "stays mutating" in record.getMessage() + for record in log_records) + + +def test_a_declaration_that_raises_is_mutating(log_records): + class _Raises: + @property + def mcp_read_only(self): + raise RuntimeError("no") + + def __call__(self): + return 1 + + handler = _Raises() + assert plugin_declares_read_only(handler) is False + assert make_plugin_tool("AC_odd", handler).annotations.read_only is False + assert any("stays mutating" in record.getMessage() for record in log_records) + + +def test_a_bound_method_carries_its_functions_declaration(): + class _Owner: + def AC_look(self): + return 1 + AC_look.mcp_read_only = True + + assert make_plugin_tool("AC_look", _Owner().AC_look).annotations.read_only is True + + +@pytest.mark.parametrize("mode", _MODES) +def test_a_declared_tool_runs_on_a_read_only_server(mode): + plugin = _Plugin() + server = MCPServer(tools=[], read_only=True, tool_mode=mode) + assert register_plugin_tools(server, plugin.commands) == [_PEEK, _POKE] + _enable(server, mode, _PEEK) + listed = _listed(server) + assert _PEEK in listed and _POKE not in listed + assert _call(server, _PEEK)["result"]["isError"] is False + assert _call(server, _POKE)["error"]["code"] == -32602 + assert plugin.ran == ["peek"] + + +@pytest.mark.parametrize("mode", _MODES) +def test_an_undeclared_plugin_tool_still_does_not_run_read_only(mode): + plugin = _Plugin(declaration=False) + server = MCPServer(tools=[], read_only=True, tool_mode=mode) + register_plugin_tools(server, plugin.commands) + assert not {_PEEK, _POKE} & _listed(server) + assert "error" in _call(server, _PEEK) and "error" in _call(server, _POKE) + assert plugin.ran == [] + + +@pytest.mark.parametrize("mode", _MODES) +def test_a_malformed_declaration_does_not_run_read_only(mode): + plugin = _Plugin(declaration="true") + server = MCPServer(tools=[], read_only=True, tool_mode=mode) + register_plugin_tools(server, plugin.commands) + assert _PEEK not in _listed(server) + assert _call(server, _PEEK)["error"]["code"] == -32602 + assert plugin.ran == [] + + +def test_a_server_that_is_not_read_only_offers_both(): + plugin = _Plugin() + server = MCPServer(tools=[]) + register_plugin_tools(server, plugin.commands) + assert _listed(server) == {_PEEK, _POKE} + assert _call(server, _POKE)["result"]["isError"] is False + + +@pytest.mark.parametrize("mode", _MODES) +def test_a_viewer_may_call_the_declared_tool_and_only_that_one(mode): + plugin = _Plugin() + server = MCPServer(tools=[], tool_mode=mode) + register_plugin_tools(server, plugin.commands) + with authorization_scope(AuthorizationContext("viewer-user", Role.VIEWER)): + _enable(server, mode, _PEEK) + listed = _listed(server) + peek, poke = _call(server, _PEEK), _call(server, _POKE) + assert _PEEK in listed and _POKE not in listed + assert peek["result"]["isError"] is False + assert "error" in poke + assert plugin.ran == ["peek"] + + +def test_the_declared_tool_needs_read_screen_and_a_role_without_it_is_refused(): + plugin = _Plugin() + tools = [make_plugin_tool(name, handler) for name, handler in plugin.commands.items()] + peek = tools[0] + assert capability_for_tool(peek.name, peek.annotations.read_only) == Capability.READ_SCREEN + with authorization_scope(AuthorizationContext("nobody", "no-such-role")): + assert visible_tools(tools) == [] + with authorization_scope(AuthorizationContext("op", Role.OPERATOR)): + assert {tool.name for tool in visible_tools(tools)} == {_PEEK, _POKE} + server = MCPServer(tools=tools) + with authorization_scope(AuthorizationContext("nobody", "no-such-role")): + refused = _call(server, _PEEK) + assert "error" in refused and plugin.ran == [] + + +def test_registration_logs_which_tools_declared_it(log_records): + plugin = _Plugin() + register_plugin_tools(MCPServer(tools=[]), plugin.commands) + declared = [record.getMessage() for record in log_records + if "trusted, not verified" in record.getMessage()] + assert len(declared) == 1 and _PEEK in declared[0] and _POKE not in declared[0] + + +def test_the_watcher_honours_and_logs_the_declaration(tmp_path, log_records): + marker = tmp_path / "ran.txt" + plugins = tmp_path / "plugins" + plugins.mkdir() + (plugins / "tools.py").write_text( + "def AC_peek():\n" + f" open({str(marker)!r}, 'w').close()\n" + " return 'seen'\n" + "AC_peek.mcp_read_only = True\n" + "def AC_poke():\n" + " raise AssertionError('a mutating plugin tool ran on a read-only server')\n", + encoding="utf-8") + server = MCPServer(tools=[], read_only=True) + PluginWatcher(server, str(plugins)).poll_once() + assert _listed(server) == {_PEEK} + assert _call(server, _PEEK)["result"]["isError"] is False + assert marker.exists() + assert "error" in _call(server, _POKE) + assert any("trusted, not verified" in record.getMessage() and _PEEK in record.getMessage() + for record in log_records) diff --git a/test/unit_test/headless/test_rbac_user_admin.py b/test/unit_test/headless/test_rbac_user_admin.py index ffd7b86e..c3e731b0 100644 --- a/test/unit_test/headless/test_rbac_user_admin.py +++ b/test/unit_test/headless/test_rbac_user_admin.py @@ -23,7 +23,7 @@ from je_auto_control.utils.mcp_server.audit import AuditLogger from je_auto_control.utils.mcp_server.http_transport import DEFAULT_PATH, HttpMCPServer from je_auto_control.utils.mcp_server.server import MCPServer -from je_auto_control.utils.mcp_server.tools import build_default_tool_registry +from je_auto_control.utils.mcp_server.tools import MCPTool, build_default_tool_registry from je_auto_control.utils.rbac import ( USERS_ENV, AuthorizationContext, AuthorizationError, Capability, IssuedToken, Role, UserAuthError, UserStore, admin, authorization_scope, can, @@ -394,6 +394,61 @@ def test_data_tools_need_more_than_read_screen(users): "screenshot"}, "screen-state tools stay; the screenshot tool is not read-only" +_STORED_RECORD_TOOLS = ( + # the ones the maintainer named + "ac_list_run_history", "ac_costs_list", "ac_costs_summary", "ac_trace_export", + "ac_self_heal_log_list", "ac_usb_acl_list", + # the rest of the registry that meets the same rule + "ac_trace_summary", "ac_heal_stats", "ac_self_heal_revision_list", + "ac_self_heal_evaluate", "ac_flaky_report", "ac_rank_tests", "ac_select_tests", + "ac_shard_suite", "ac_ab_report", "ac_ab_best_strategy", "ac_journal_read", + "ac_journal_runs", "ac_lease_active", "ac_element_list", "ac_skill_list", + "ac_skill_search", "ac_repair_pending", "ac_repair_resolved", "ac_quarantine_list", + "ac_pending_artifacts", "ac_config_sync_status", + "ac_android_get_clipboard", "ac_ios_get_clipboard", +) +_LIVE_SCREEN_TOOLS = ( + "ac_vlm_locate", "ac_self_heal_locate", # they read the screen; cost is not a capability + "ac_locate_text", "ac_locate_image_center", "ac_a11y_list", "ac_element_find", + "ac_list_executions", "ac_scheduler_list_jobs", "ac_journal_status", +) + + +def test_stored_records_need_read_data_and_the_live_screen_only_read_screen(): + registry = {tool.name: tool for tool in build_default_tool_registry(read_only=False)} + for name in _STORED_RECORD_TOOLS: + assert registry[name].annotations.read_only, name + assert capability_for_tool(name, True) == Capability.READ_DATA, name + for name in _LIVE_SCREEN_TOOLS: + assert registry[name].annotations.read_only, name + assert capability_for_tool(name, True) == Capability.READ_SCREEN, name + assert len(DATA_TOOLS) == 71 + + +def test_a_viewer_is_refused_a_stored_record_tool_and_an_operator_is_not(): + ran = [] + + def handler(): + ran.append("history") + return [] + + registry = [tool for tool in build_default_tool_registry(read_only=False) + if tool.name == "ac_list_run_history"] + server = MCPServer(tools=[MCPTool( + name=registry[0].name, description="history", input_schema={"type": "object"}, + handler=handler, annotations=registry[0].annotations)]) + call = json.dumps({"jsonrpc": "2.0", "id": 1, "method": "tools/call", + "params": {"name": "ac_list_run_history", "arguments": {}}}) + with authorization_scope(AuthorizationContext("viewer-user", Role.VIEWER)): + refused = json.loads(server.handle_line(call)) + assert refused["error"]["code"] == -32003 + assert refused["error"]["data"]["required_capability"] == Capability.READ_DATA + assert ran == [] + with authorization_scope(AuthorizationContext("op", Role.OPERATOR)): + allowed = json.loads(server.handle_line(call)) + assert allowed["result"]["isError"] is False and ran == ["history"] + + def test_an_alias_needs_what_its_tool_needs(): from je_auto_control.utils.mcp_server.tools import _DEFAULT_ALIASES registry = {tool.name: tool for tool in build_default_tool_registry(read_only=False)} From 43bf298b1e5ceef626ca11a7626b6f99424743ee Mon Sep 17 00:00:00 2001 From: JeffreyChen Date: Fri, 9 Oct 2026 11:42:20 +0800 Subject: [PATCH 4/7] Make a ChatOps /run a stoppable run and add /stop /run could only be stopped when the script itself carried AC_run_stoppable, so AC_stop_execution, the MCP stop tool and the GUI's stop-all had nothing to address. It now runs as chatops-