Skip to content

Commit 1c42e91

Browse files
committed
Keep credentials off redirected hosts and route integrations through egress
urllib copies every header onto a redirect, so a cross-host redirect handed over the Authorization header; it is dropped when the host changes. The ticket backends and the Slack bot called urllib directly, bypassing the egress policy, and followed a 302 that turned a ticket POST into a GET to another host; they use the shared client with redirects refused. A non-object reply no longer ends the Slack poll loop, and /screenshot keeps only the file name it is given.
1 parent 9ba90a5 commit 1c42e91

15 files changed

Lines changed: 270 additions & 62 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ it shipped into a `## [x.y.z] - date` section of their own; the tag's
2424

2525
### Changed
2626

27+
- **Chat-ops `/screenshot` takes a file name, not a path.** The PNG is
28+
written into the router context's `screenshot_dir` (default: a
29+
`je_auto_control_chatops` folder in the temp directory); anyone in the
30+
channel could previously choose any path. Migration: set `screenshot_dir`.
31+
2732
- **`AC_shell_to_var` on Windows.** A string command is passed to
2833
`CreateProcess` as written; quoted arguments used to arrive with their
2934
quotes still on. Output is decoded with the new `encoding` argument,
@@ -137,6 +142,12 @@ it shipped into a `## [x.y.z] - date` section of their own; the tag's
137142

138143
### Fixed
139144

145+
- **Credentials no longer follow a redirect to another host.** The HTTP
146+
client drops `Authorization` and cookies when a redirect changes host. The
147+
Jira, Linear and GitHub failure-hook backends and the Slack bot now obey
148+
the egress policy and refuse redirects, and a non-object JSON reply is a
149+
failed call instead of an exception that stopped the Slack poll loop.
150+
140151
- **Remote-desktop file transfers cannot leave partial files or destroy the
141152
original.** Both receivers write to a `.part` file and rename it into place
142153
only when exactly the announced number of bytes arrived; excess or missing

‎architecture_explore.md‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ iOS(WebDriverAgent)。核心能力是滑鼠/鍵盤控制、影像辨識、
2020
| 指標 | 數值 |
2121
| --- | ---: |
2222
| Python 模組總數(含周邊子專案) | 1,045 |
23-
| 程式碼總行數 | 142,800 |
23+
| 程式碼總行數 | 142,845 |
2424
| `je_auto_control/utils/` 子套件數 | 310 |
2525
| `AC_*` 動作指令數(`known_commands()` 實測) | 773 |
2626
| 套件門面 `__all__` 公開名稱數 | 1,239 |
@@ -525,17 +525,17 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。
525525

526526
### 5.4.11 伺服器、網路協定與外部整合
527527

528-
> 24 個套件、約 5,965 行。
528+
> 24 個套件、約 6,011 行。
529529
530530
| 模組 | 行數 | 職責 |
531531
| --- | ---: | --- |
532532
| `utils/acme_v2/` | 598 | 完整 ACME v2 用戶端(RFC 8555),不依賴 certbot |
533-
| `utils/chatops/` | 636 | Chat-ops bot:接收 Slack/Discord/webhook 的 slash 指令並路由到動作 |
533+
| `utils/chatops/` | 649 | Chat-ops bot:接收 Slack/Discord/webhook 的 slash 指令並路由到動作 |
534534
| `utils/cookie_jar/` | 103 | RFC 6265 cookie jar |
535535
| `utils/email_send/` | 116 | SMTP 寄信(email 觸發器的發送端搭檔) |
536536
| `utils/events/` | 82 | 對外 CloudEvents 發送(執行生命週期事件) |
537537
| `utils/http_cassette/` | 110 | 錄製/重播 HTTP 互動,做離線決定性 API 測試 |
538-
| `utils/http_client/` | 160 | 零依賴 HTTP(S) 用戶端,供 action 步驟呼叫 API |
538+
| `utils/http_client/` | 193 | 零依賴 HTTP(S) 用戶端,供 action 步驟呼叫 API |
539539
| `utils/http_conditional/` | 87 | 條件式 HTTP 請求與快取驗證器 |
540540
| `utils/http_content/` | 103 | HTTP 內容協商與回應解壓縮 |
541541
| `utils/http_problem/` | 116 | RFC 9457 problem+json 解析 |
@@ -556,7 +556,7 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。
556556

557557
### 5.4.12 報表、可觀測性與測試治理
558558

559-
> 34 個套件、約 6,911 行。
559+
> 34 個套件、約 6,910 行。
560560
561561
| 模組 | 行數 | 職責 |
562562
| --- | ---: | --- |
@@ -567,7 +567,7 @@ socket server 有 8 MiB 讀取上限與 30 秒 handler timeout。
567567
| `utils/canonical_log/` | 90 | canonical log line 與結構化 JSON 日誌 |
568568
| `utils/ci_annotations/` | 62 | 由執行結果輸出 CI 工作流程註記(GitHub Actions) |
569569
| `utils/compliance/` | 136 | 合規:把治理證據對應到 SOC2/ISO 27001 控制項 |
570-
| `utils/failure_hooks/` | 396 | 失敗 → 工單自動化:開 Jira/Linear/GitHub issue |
570+
| `utils/failure_hooks/` | 395 | 失敗 → 工單自動化:開 Jira/Linear/GitHub issue |
571571
| `utils/failure_signature/` | 74 | 把錯誤訊息正規化成穩定的 SHA-256 失敗簽章並分群 |
572572
| `utils/flake_cluster/` | 103 | 以共同失敗 Jaccard 相似度為易碎測試分群 |
573573
| `utils/flakiness/` | 150 | 以執行歷史分析不穩定測試 |
@@ -1079,6 +1079,6 @@ socket 預設綁 `127.0.0.1`;資源一律用 `with`。
10791079
| `osx/` | 17 | 919 |
10801080
| `autocontrol-lsp/` | 8 | 744 |
10811081
| `utils/hotkey/` | 7 | 738 |
1082-
| 其餘模組(約 286 個 `utils/` 子套件 + `android/`/`ios/`/周邊小工具) | 674 | 48,256 |
1083-
| **總計** | **1,039** | **142,735** |
1082+
| 其餘模組(約 286 個 `utils/` 子套件 + `android/`/`ios/`/周邊小工具) | 674 | 48,301 |
1083+
| **總計** | **1,039** | **142,780** |
10841084

‎docs/source/Eng/doc/new_features/v2_features_doc.rst‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,11 @@ Chat-ops bot
236236
Transport-agnostic ``CommandRouter`` plus a polling Slack adapter so
237237
``/run <script>`` over Slack hits the same execution path as the
238238
scheduler. Built-in commands: ``/help``, ``/scripts``, ``/run``,
239-
``/screenshot``, ``/status``. RBAC via the ``required_role``
239+
``/screenshot [name]``, ``/status``. ``/screenshot`` writes into the
240+
context's ``screenshot_dir`` (default: ``je_auto_control_chatops`` in the
241+
temp directory) and keeps only the file name it is given. The Slack
242+
adapter goes through the package HTTP client, so the egress policy applies.
243+
RBAC via the ``required_role``
240244
parameter. GUI: **Chat-Ops** playground tab.
241245

242246

‎docs/source/Zh/doc/new_features/v2_features_doc.rst‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,10 @@ Chat-ops 機器人
229229
傳輸層中立的 ``CommandRouter`` 加上 Slack polling adapter,
230230
``/run <script>`` 經 Slack 進入和 scheduler 相同的執行路徑。
231231
內建命令:``/help``、``/scripts``、``/run``、``/screenshot``、
232-
``/status``。RBAC 透過 ``required_role`` 參數。
232+
``/status``。``/screenshot [name]`` 寫進 context 的 ``screenshot_dir``
233+
(預設為暫存目錄下的 ``je_auto_control_chatops``),給的名稱只保留檔名。
234+
Slack adapter 走套件的 HTTP client,所以出站政策同樣適用。
235+
RBAC 透過 ``required_role`` 參數。
233236
GUI:**Chat-Ops** 試用分頁。
234237

235238

‎docs/updates/2026-09.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,3 +336,12 @@ Index and query commands: [README.md](README.md). New entries go at the end.
336336
- **Tests**: `test_file_transfer_audit.py` (new, 24; 21 fail on the previous commit, the other 3 guard the success paths and a name Windows already refused).
337337
- **Files**: `je_auto_control/utils/remote_desktop/{file_transfer,webrtc_files}.py`, `je_auto_control/utils/admin/admin_client.py`, `je_auto_control/utils/package_manager/package_manager_class.py`, `test/unit_test/headless/test_file_transfer_audit.py`, `Progress.md`, `CHANGELOG.md`, `architecture_explore.md` (figures).
338338
- **Open items**: the viewer-side path decision in `Progress.md`.
339+
340+
## U-20260923-24 · 2026-09-23 · Outbound audit: credentials across redirects, egress bypass, chat-ops screenshot path · #incident #security #chatops
341+
342+
- **What**: An audit of the outbound integrations (`utils/failure_hooks`, `utils/chatops`, `utils/notify_channels`, `utils/events`, `utils/email_send`, `utils/observability`); every finding reproduced against servers on 127.0.0.1.
343+
- **Fixes**: (1) urllib copies every request header onto a redirect, so after U-20260923-22's redirect check a cross-host redirect still handed the `Authorization` header to the other host. `_CheckedRedirectHandler` now drops `Authorization`, `Cookie` and `Proxy-Authorization` when the host changes. (2) The Jira / Linear / GitHub backends (`failure_hooks/backends.py`) and the Slack bot (`chatops/slack_bot.py`) called `urllib.request.urlopen` themselves, so the egress policy did not apply to them. Both go through the new `http_client.perform_call`, the egress check plus transport that `http_request` now uses too, with redirects refused (`call["follow_redirects"] = False`): urllib turned a ticket POST's 302 into a GET that carried the credentials to the other host, and counted that reply as a created ticket. A non-2xx ticket reply is a failure with its status. (3) A reply that was valid JSON but not an object raised `AttributeError`: out of `FailureHookManager.fire()`, and out of `SlackBot.run_forever`, which catches only `SlackError`, ending the poll loop. Both treat it as a failed call; non-object message items are skipped. (4) `/screenshot <path>` wrote the PNG to any path in the chat message, and no default command requires a role. The argument now keeps only its file name, written into `context['screenshot_dir']` (default `je_auto_control_chatops` in the temp directory). (5) `SlackError` and `ChatOpsError` join the `AutoControlException` family, keeping their old bases.
344+
- **Checked and fine**: `notify_channels` and `events.post_cloudevent` already used `http_request`; e-mail rejects CR/LF in headers and verifies STARTTLS; every direct call had a timeout; Slack backoff is capped; `/run` is held to `script_root`.
345+
- **Tests**: `test_outbound_audit.py` (new, 9; 8 fail on the previous commit, the same-host redirect one is a guard). `test_failure_hooks.py` stubbed the global `urlopen`, which the backends no longer call; it stubs `http_client.urllib_transport`. `test_chatops_bot.py`'s screenshot test passes a name and a `screenshot_dir`.
346+
- **Files**: `je_auto_control/utils/http_client/{http_client,__init__}.py`, `je_auto_control/utils/failure_hooks/backends.py`, `je_auto_control/utils/chatops/{slack_bot,router,handlers}.py`, `test/unit_test/headless/{test_outbound_audit,test_failure_hooks,test_chatops_bot}.py`, `docs/source/{Eng,Zh}/doc/new_features/v2_features_doc.rst`, `CHANGELOG.md`, `architecture_explore.md` (figures).
347+
- **Open items**: none.

‎docs/updates/README.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ In the same commit: delete the item from `Progress.md`, add a `#done` entry here
5858

5959
| ID | Date | Title | Tags | Batch |
6060
|---|---|---|---|---|
61+
| U-20260923-24 | 2026-09-23 | Outbound audit: credentials across redirects, egress bypass, chat-ops screenshot path | #incident #security #chatops | [2026-09](2026-09.md) |
6162
| U-20260923-23 | 2026-09-23 | File transfer / admin audit: partial files, size limits, device names, poll crash | #incident #security #remote-desktop | [2026-09](2026-09.md) |
6263
| U-20260923-22 | 2026-09-23 | Data-command audit: escaping errors, redirect egress bypass, Windows shell quoting | #incident #security #executor | [2026-09](2026-09.md) |
6364
| U-20260923-21 | 2026-09-23 | Report audit: control characters, silent write failures | #incident #reports | [2026-09](2026-09.md) |
@@ -128,7 +129,7 @@ In the same commit: delete the item from `Progress.md`, add a `#done` entry here
128129

129130
| File | Period | Entries |
130131
|---|---|---:|
131-
| [2026-09.md](2026-09.md) | 2026-09 | 39 |
132+
| [2026-09.md](2026-09.md) | 2026-09 | 40 |
132133
| [2026-08-f.md](2026-08-f.md) | 2026-08 | 1 |
133134
| [2026-08-e.md](2026-08-e.md) | 2026-08 | 2 |
134135
| [2026-08-d.md](2026-08-d.md) | 2026-08 | 2 |

‎je_auto_control/utils/chatops/handlers.py‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -107,13 +107,27 @@ def _format_duration(duration_seconds: Any) -> str:
107107

108108

109109
def cmd_screenshot(argv: List[str],
110-
_context: Dict[str, Any]) -> CommandResult:
111-
"""``/screenshot [path]`` — capture the screen and return the path."""
112-
target = Path(argv[0]).expanduser().resolve() if argv else Path(
113-
tempfile.NamedTemporaryFile(
114-
prefix="chatops_", suffix=".png", delete=False,
115-
).name,
116-
)
110+
context: Dict[str, Any]) -> CommandResult:
111+
"""``/screenshot [name]`` — capture the screen and return the path.
112+
113+
The file goes into ``context['screenshot_dir']`` (default: a
114+
``je_auto_control_chatops`` folder in the temp directory); a given name
115+
keeps only its last component. The argument used to be a full path taken
116+
from the chat message, so anyone in the channel could write a PNG to any
117+
location the bot's account can write.
118+
"""
119+
directory = Path(context.get("screenshot_dir")
120+
or Path(tempfile.gettempdir()) / "je_auto_control_chatops")
121+
directory.mkdir(parents=True, exist_ok=True)
122+
if argv:
123+
name = Path(argv[0]).name
124+
if name in ("", ".", ".."):
125+
raise ChatOpsError(f"invalid screenshot name: {argv[0]!r}")
126+
target = directory / name
127+
else:
128+
target = Path(tempfile.NamedTemporaryFile(
129+
dir=directory, prefix="chatops_", suffix=".png", delete=False,
130+
).name)
117131
from je_auto_control.wrapper.auto_control_screen import screenshot
118132
screenshot(file_path=str(target))
119133
return CommandResult(

‎je_auto_control/utils/chatops/router.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
_BUILTIN_HELP = "help"
3636

3737

38-
class ChatOpsError(ValueError):
38+
class ChatOpsError(AutoControlException, ValueError):
3939
"""Raised for malformed commands the user can fix in their next message."""
4040

4141

‎je_auto_control/utils/chatops/slack_bot.py‎

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
"""
1919
from __future__ import annotations
2020

21-
import json
2221
import threading
2322
import urllib.error
2423
import urllib.parse
@@ -28,6 +27,7 @@
2827

2928
from je_auto_control.utils.chatops.router import CommandResult, CommandRouter
3029
from je_auto_control.utils.exception.exceptions import AutoControlException
30+
from je_auto_control.utils.http_client.http_client import build_call, perform_call
3131
from je_auto_control.utils.logging.logging_instance import autocontrol_logger
3232

3333

@@ -37,7 +37,7 @@
3737
_MAX_BACKOFF = 60.0
3838

3939

40-
class SlackError(RuntimeError):
40+
class SlackError(AutoControlException, RuntimeError):
4141
"""Raised when the Slack API returns ``ok: false`` or HTTP fails."""
4242

4343

@@ -151,7 +151,8 @@ def _fetch_messages(self) -> list:
151151
if self.last_seen_ts:
152152
params["oldest"] = self.last_seen_ts
153153
body = self._api_get("conversations.history", params)
154-
return list(body.get("messages") or [])
154+
messages = body.get("messages") or []
155+
return [message for message in messages if isinstance(message, dict)]
155156

156157
def _is_self(self, message: Dict[str, Any]) -> bool:
157158
if message.get("subtype") == "bot_message":
@@ -188,23 +189,21 @@ def _request(self, url: str, *, method: str,
188189
) -> Dict[str, Any]:
189190
if not url.startswith("https://slack.com/api/"):
190191
raise SlackError(f"refusing to call non-Slack URL: {url}")
191-
headers = {"Authorization": f"Bearer {self.token}"}
192-
data: Optional[bytes] = None
193-
if payload is not None:
194-
data = json.dumps(payload).encode("utf-8")
195-
headers["Content-Type"] = "application/json"
196-
request = urllib.request.Request( # nosec B310 # reason: scheme allow-listed above
197-
url, data=data, method=method, headers=headers,
198-
)
192+
# Through http_client, so the egress policy applies to Slack too; a
193+
# Slack API call never redirects, so a 3xx is an error, not followed.
194+
call = build_call(url, method, headers={"Authorization": f"Bearer {self.token}"},
195+
json_body=payload, timeout=_HTTP_TIMEOUT)
196+
call["follow_redirects"] = False
199197
try:
200-
with urllib.request.urlopen( # nosec B310
201-
request, timeout=_HTTP_TIMEOUT,
202-
) as response:
203-
body = json.loads(response.read().decode("utf-8"))
204-
except urllib.error.URLError as error:
198+
response = perform_call(call)
199+
except (OSError, ValueError) as error: # URLError, EgressBlocked
205200
raise SlackError(f"HTTP failure: {error}") from error
206-
except ValueError as error:
207-
raise SlackError(f"non-JSON response: {error}") from error
201+
body = response["json"]
202+
if not isinstance(body, dict):
203+
# A list or string body raised AttributeError, which run_forever
204+
# does not catch: one bad reply ended the poll loop for good.
205+
raise SlackError(f"Slack {url} returned HTTP {response['status']} "
206+
"without a JSON object")
208207
if not body.get("ok"):
209208
raise SlackError(
210209
f"Slack {url} returned {body.get('error', 'unknown')}",

‎je_auto_control/utils/failure_hooks/backends.py‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,13 @@
77
from __future__ import annotations
88

99
import base64
10-
import json
11-
import urllib.error
12-
import urllib.request
1310
from dataclasses import dataclass
1411
from typing import Any, Dict, Optional, Protocol
1512

1613
from je_auto_control.utils.failure_hooks.report import (
1714
FailureReport, TicketResult,
1815
)
16+
from je_auto_control.utils.http_client.http_client import build_call, perform_call
1917

2018

2119
_HTTP_TIMEOUT = 15.0
@@ -130,24 +128,25 @@ def _post_json(backend_name: str, url: str, body: Dict[str, Any], *,
130128
backend=backend_name, succeeded=False,
131129
error=f"refusing to call non-HTTP(S) URL: {url}",
132130
)
133-
data = json.dumps(body).encode("utf-8")
134-
request = urllib.request.Request( # nosec B310 # reason: scheme guard above
135-
url, data=data, method="POST",
136-
headers={**headers, "Content-Type": "application/json"},
137-
)
131+
# Through http_client so the egress policy applies, and with redirects
132+
# refused: urllib turned a 302 into a GET that carried the credentials to
133+
# the other host, and counted that reply as a created ticket.
134+
call = build_call(url, "POST", headers=headers, json_body=body, timeout=_HTTP_TIMEOUT)
135+
call["follow_redirects"] = False
138136
try:
139-
with urllib.request.urlopen( # nosec B310
140-
request, timeout=_HTTP_TIMEOUT,
141-
) as response:
142-
payload = json.loads(response.read().decode("utf-8"))
143-
except urllib.error.URLError as error:
137+
response = perform_call(call)
138+
except (OSError, ValueError) as error: # URLError, EgressBlocked
139+
return TicketResult(backend=backend_name, succeeded=False, error=str(error))
140+
if not 200 <= response["status"] < 300:
144141
return TicketResult(
145-
backend=backend_name, succeeded=False, error=str(error),
142+
backend=backend_name, succeeded=False,
143+
error=f"HTTP {response['status']}: {response['text'][:200]}",
146144
)
147-
except ValueError as error:
145+
payload = response["json"]
146+
if not isinstance(payload, dict):
148147
return TicketResult(
149148
backend=backend_name, succeeded=False,
150-
error=f"non-JSON response: {error}",
149+
error="response is not a JSON object",
151150
)
152151
if response_extractor is not None:
153152
return response_extractor(backend_name, payload)

0 commit comments

Comments
 (0)