admin/ComfyUI-OpenClaw
1
0
Fork 0
mirror of https://github.com/rookiestar28/ComfyUI-OpenClaw.git synced 2026-08-14 00:48:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: enforce safe oauth egress and notification dismissal

Browse Source
This commit is contained in:
rookiestar28
2026-03-20 00:13:53 +08:00
parent 52cb09e4be
commit 5e51433d7e
5 changed files with 116 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
connector/platforms/slack_installation_manager.py
+33 -23
View File
@@ -21,6 +21,11 @@ try:
InstallationStatus,
get_connector_installation_registry,
)
from services.safe_io import (
STANDARD_OUTBOUND_POLICY,
SafeIOHTTPError,
safe_request_json,
)
from services.secret_store import SecretStore, get_secret_store
from services.state_dir import get_state_dir
except ImportError: # pragma: no cover
@@ -32,6 +37,11 @@ except ImportError: # pragma: no cover
InstallationStatus,
get_connector_installation_registry,
)
from services.safe_io import ( # type: ignore
STANDARD_OUTBOUND_POLICY,
SafeIOHTTPError,
safe_request_json,
)
from services.secret_store import SecretStore, get_secret_store # type: ignore
from services.state_dir import get_state_dir # type: ignore
@@ -47,14 +57,6 @@ _INVALID_TOKEN_ERRORS = frozenset(
_DEGRADED_TOKEN_ERRORS = frozenset({"ratelimited", "request_timeout", "fatal_error"})
def _load_aiohttp():
try:
import aiohttp # type: ignore
except ModuleNotFoundError:
return None
return aiohttp
class SlackInstallationManager:
def __init__(
self,
@@ -150,27 +152,35 @@ class SlackInstallationManager:
return f"{SLACK_AUTHORIZE_URL}?{urlencode(params)}"
async def exchange_code(self, code: str) -> Dict[str, Any]:
aiohttp = _load_aiohttp()
if aiohttp is None:
raise RuntimeError("aiohttp required for Slack OAuth exchange")
payload = {
"client_id": self.config.slack_client_id or "",
"client_secret": self.config.slack_client_secret or "",
"code": str(code or "").strip(),
"redirect_uri": self.resolve_redirect_uri(),
}
async with aiohttp.ClientSession() as session:
async with session.post(
SLACK_OAUTH_ACCESS_URL,
data=payload,
timeout=aiohttp.ClientTimeout(total=15),
) as resp:
data = await resp.json(content_type=None)
if resp.status != 200 or not data.get("ok"):
raise RuntimeError(
f"slack_oauth_exchange_failed:{resp.status}:{data.get('error', 'unknown')}"
)
return data
try:
return safe_request_json(
method="POST",
url=SLACK_OAUTH_ACCESS_URL,
raw_body=urlencode(payload).encode("utf-8"),
headers={"Accept": "application/json"},
content_type="application/x-www-form-urlencoded",
timeout_sec=15,
allow_hosts={"slack.com"},
policy=STANDARD_OUTBOUND_POLICY,
)
except SafeIOHTTPError as exc:
error_code = "unknown"
try:
body = exc.body or ""
parsed = json.loads(body) if body else {}
if isinstance(parsed, dict):
error_code = str(parsed.get("error", "") or error_code)
except Exception:
pass
raise RuntimeError(
f"slack_oauth_exchange_failed:{exc.status_code}:{error_code}"
) from exc
def _normalize_workspace_id(self, payload: Dict[str, Any]) -> str:
workspace_id = (
services/safe_io.py
+9 -2
View File
@@ -610,11 +610,13 @@ def safe_request_json(
url: str,
json_body: Any = None,
*,
raw_body: Optional[bytes] = None,
allow_hosts: Optional[Set[str]] = None,
allow_any_public_host: bool = False,
allow_loopback_hosts: Optional[Set[str]] = None,
allow_insecure_base_url: bool = False,
headers: Optional[dict] = None,
content_type: str = "application/json",
timeout_sec: int = 10,
max_response_bytes: int = 1_000_000,
max_redirects: int = 0,
@@ -629,7 +631,11 @@ def safe_request_json(
current_url = url
current_method = method
current_body = json.dumps(json_body).encode("utf-8") if json_body else None
if json_body is not None and raw_body is not None:
raise ValueError("safe_request_json accepts either json_body or raw_body")
current_body = raw_body
if json_body is not None:
current_body = json.dumps(json_body).encode("utf-8")
redirects_followed = 0
while True:
@@ -659,7 +665,8 @@ def safe_request_json(
PACK_VERSION = "0.0.0"
request.add_header("User-Agent", f"ComfyUI-OpenClaw/{PACK_VERSION}")
request.add_header("Content-Type", "application/json")
if content_type:
request.add_header("Content-Type", content_type)
# Add safe headers
# R106: external control-plane adapter requires Authorization header support.
tests/test_safe_io.py
+34
View File
@@ -296,6 +296,40 @@ class TestURLSafety(unittest.TestCase):
self.assertEqual(header_map.get("x-test"), "ok")
self.assertNotIn("bad-header", header_map)
@patch("services.safe_io._build_pinned_opener")
@patch("services.safe_io.validate_outbound_url")
def test_safe_request_json_supports_form_encoded_raw_body(
self, mock_validate, mock_build
):
"""Non-JSON callers should still use safe_io without direct client sessions."""
mock_validate.return_value = ("https", "slack.com", 443, ["93.184.216.34"])
mock_response = MagicMock()
mock_response.getcode.return_value = 200
mock_response.read.return_value = b'{"ok": true}'
mock_opener = MagicMock()
mock_opener.open.return_value.__enter__.return_value = mock_response
mock_build.return_value = mock_opener
out = safe_request_json(
method="POST",
url="https://slack.com/api/oauth.v2.access",
raw_body=b"code=test&client_id=abc",
content_type="application/x-www-form-urlencoded",
headers={"Accept": "application/json"},
allow_hosts={"slack.com"},
)
self.assertEqual(out["ok"], True)
request_arg = mock_opener.open.call_args.args[0]
self.assertEqual(request_arg.data, b"code=test&client_id=abc")
header_map = {k.lower(): v for k, v in request_arg.header_items()}
self.assertEqual(
header_map.get("content-type"), "application/x-www-form-urlencoded"
)
self.assertEqual(header_map.get("accept"), "application/json")
@patch("services.safe_io._build_pinned_opener")
@patch("services.safe_io.validate_outbound_url")
def test_safe_request_text_stream_accept_header_is_allowed(
web/openclaw_notifications.js
+9
View File
@@ -139,6 +139,15 @@ export class OpenClawNotifications {
existing.action = action;
existing.metadata = metadata;
} else {
const dismissed = this.entries.find((entry) => entry.dedupe_key === dedupeKey && entry.dismissed_at);
if (dismissed && dismissed.message === message && dismissed.severity === severity) {
dismissed.updated_at = nowIso;
dismissed.action = action;
dismissed.metadata = metadata;
this._save();
this._emit();
return { ...dismissed };
}
this.entries.unshift({
id: String(payload.id || `ntf_${Math.random().toString(36).slice(2, 10)}`),
source,
web/tests/unit/openclaw_notifications.test.js
+31
View File
@@ -60,4 +60,35 @@ describe("OpenClawNotifications", () => {
expect(reloaded.getEntries({ includeDismissed: true })[0].dismissed_at).not.toBeNull();
expect(reloaded.getEntries({ includeDismissed: true })[0].acknowledged_at).not.toBeNull();
});
it("does not resurrect an identical dismissed notification on repeated auto-refresh", () => {
let nowValue = Date.parse("2026-03-19T00:00:00Z");
const store = new OpenClawNotifications({
storage: localStorage,
now: () => nowValue,
});
const entry = store.notify({
severity: "error",
source: "model-manager",
message: "search: search_failed",
dedupeKey: "model-manager:refresh",
});
store.dismiss(entry.id);
expect(store.getEntries()).toHaveLength(0);
nowValue += 1_000;
store.notify({
severity: "error",
source: "model-manager",
message: "search: search_failed",
dedupeKey: "model-manager:refresh",
});
expect(store.getEntries()).toHaveLength(0);
const dismissed = store.getEntries({ includeDismissed: true });
expect(dismissed).toHaveLength(1);
expect(dismissed[0].dismissed_at).not.toBeNull();
});
});