mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-06 17:38:35 +00:00
fix(webui): harden automation management API
Maintainer edit: redact external channel chat identifiers from the WebUI automation payload and reject malformed or unschedulable automation updates before they mutate cron jobs.
This commit is contained in:
parent
747f0a08c7
commit
8b42d0760e
@ -121,14 +121,17 @@ def _serialize_job(
|
|||||||
payload["delete_after_run"] = job.delete_after_run
|
payload["delete_after_run"] = job.delete_after_run
|
||||||
payload["created_at_ms"] = job.created_at_ms
|
payload["created_at_ms"] = job.created_at_ms
|
||||||
payload["updated_at_ms"] = job.updated_at_ms
|
payload["updated_at_ms"] = job.updated_at_ms
|
||||||
payload["payload"].update(
|
payload["payload"].update({"kind": job.payload.kind})
|
||||||
{
|
if _expose_origin_identifiers(job):
|
||||||
"kind": job.payload.kind,
|
payload["payload"].update(
|
||||||
"session_key": job.payload.session_key,
|
{
|
||||||
"origin_channel": job.payload.origin_channel,
|
"session_key": job.payload.session_key,
|
||||||
"origin_chat_id": job.payload.origin_chat_id,
|
"origin_channel": job.payload.origin_channel,
|
||||||
}
|
"origin_chat_id": job.payload.origin_chat_id,
|
||||||
)
|
}
|
||||||
|
)
|
||||||
|
elif job.payload.origin_channel:
|
||||||
|
payload["payload"]["origin_channel"] = job.payload.origin_channel
|
||||||
payload["state"].update(
|
payload["state"].update(
|
||||||
{
|
{
|
||||||
"last_run_at_ms": job.state.last_run_at_ms,
|
"last_run_at_ms": job.state.last_run_at_ms,
|
||||||
@ -156,11 +159,17 @@ def _origin_payload(
|
|||||||
chat_id = job.payload.origin_chat_id
|
chat_id = job.payload.origin_chat_id
|
||||||
if not channel or not chat_id:
|
if not channel or not chat_id:
|
||||||
return None
|
return None
|
||||||
session_key = f"{channel}:{chat_id}"
|
|
||||||
|
|
||||||
title = ""
|
title = ""
|
||||||
preview = ""
|
preview = ""
|
||||||
if channel == "websocket" and session_manager is not None:
|
if channel != "websocket":
|
||||||
|
return {
|
||||||
|
"channel": channel,
|
||||||
|
"title": title,
|
||||||
|
"preview": preview,
|
||||||
|
}
|
||||||
|
|
||||||
|
session_key = f"{channel}:{chat_id}"
|
||||||
|
if session_manager is not None:
|
||||||
data = session_manager.read_session_file(session_key)
|
data = session_manager.read_session_file(session_key)
|
||||||
if isinstance(data, dict):
|
if isinstance(data, dict):
|
||||||
title = str(data.get("title") or "")
|
title = str(data.get("title") or "")
|
||||||
@ -175,6 +184,11 @@ def _origin_payload(
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _expose_origin_identifiers(job: CronJob) -> bool:
|
||||||
|
channel = job.payload.origin_channel
|
||||||
|
return not channel or channel == "websocket"
|
||||||
|
|
||||||
|
|
||||||
def _session_preview(messages: Any) -> str:
|
def _session_preview(messages: Any) -> str:
|
||||||
if not isinstance(messages, list):
|
if not isinstance(messages, list):
|
||||||
return ""
|
return ""
|
||||||
|
|||||||
@ -789,12 +789,18 @@ def _automation_values_from_request(request: WsRequest) -> dict[str, Any] | None
|
|||||||
def _parse_automation_update(values: dict[str, Any]) -> dict[str, Any] | str:
|
def _parse_automation_update(values: dict[str, Any]) -> dict[str, Any] | str:
|
||||||
update: dict[str, Any] = {}
|
update: dict[str, Any] = {}
|
||||||
if "name" in values:
|
if "name" in values:
|
||||||
name = str(values.get("name") or "").strip()
|
raw_name = values.get("name")
|
||||||
|
if not isinstance(raw_name, str):
|
||||||
|
return "name must be a string"
|
||||||
|
name = raw_name.strip()
|
||||||
if not name:
|
if not name:
|
||||||
return "name cannot be empty"
|
return "name cannot be empty"
|
||||||
update["name"] = name
|
update["name"] = name
|
||||||
if "message" in values:
|
if "message" in values:
|
||||||
message = str(values.get("message") or "").strip()
|
raw_message = values.get("message")
|
||||||
|
if not isinstance(raw_message, str):
|
||||||
|
return "message must be a string"
|
||||||
|
message = raw_message.strip()
|
||||||
if not message:
|
if not message:
|
||||||
return "message cannot be empty"
|
return "message cannot be empty"
|
||||||
update["message"] = message
|
update["message"] = message
|
||||||
@ -805,24 +811,36 @@ def _parse_automation_update(values: dict[str, Any]) -> dict[str, Any] | str:
|
|||||||
parsed_schedule = _parse_automation_schedule(raw_schedule)
|
parsed_schedule = _parse_automation_schedule(raw_schedule)
|
||||||
if isinstance(parsed_schedule, str):
|
if isinstance(parsed_schedule, str):
|
||||||
return parsed_schedule
|
return parsed_schedule
|
||||||
|
schedule_error = _validate_automation_schedule(parsed_schedule)
|
||||||
|
if schedule_error:
|
||||||
|
return schedule_error
|
||||||
update["schedule"] = parsed_schedule
|
update["schedule"] = parsed_schedule
|
||||||
update["delete_after_run"] = parsed_schedule.kind == "at"
|
update["delete_after_run"] = parsed_schedule.kind == "at"
|
||||||
return update
|
return update
|
||||||
|
|
||||||
|
|
||||||
def _parse_automation_schedule(values: dict[str, Any]) -> CronSchedule | str:
|
def _parse_automation_schedule(values: dict[str, Any]) -> CronSchedule | str:
|
||||||
kind = str(values.get("kind") or "").strip()
|
raw_kind = values.get("kind")
|
||||||
|
if not isinstance(raw_kind, str):
|
||||||
|
return "schedule kind must be a string"
|
||||||
|
kind = raw_kind.strip()
|
||||||
if kind == "every":
|
if kind == "every":
|
||||||
every_ms = _positive_int(values.get("every_ms"))
|
every_ms = _positive_int(values.get("every_ms"))
|
||||||
if every_ms is None:
|
if every_ms is None:
|
||||||
return "every schedule requires positive every_ms"
|
return "every schedule requires positive every_ms"
|
||||||
return CronSchedule(kind="every", every_ms=every_ms)
|
return CronSchedule(kind="every", every_ms=every_ms)
|
||||||
if kind == "cron":
|
if kind == "cron":
|
||||||
expr = str(values.get("expr") or "").strip()
|
raw_expr = values.get("expr")
|
||||||
|
if not isinstance(raw_expr, str):
|
||||||
|
return "cron schedule requires expr"
|
||||||
|
expr = raw_expr.strip()
|
||||||
if not expr:
|
if not expr:
|
||||||
return "cron schedule requires expr"
|
return "cron schedule requires expr"
|
||||||
tz = str(values.get("tz") or "").strip() or None
|
raw_tz = values.get("tz")
|
||||||
return CronSchedule(kind="cron", expr=expr, tz=tz)
|
if raw_tz is not None and not isinstance(raw_tz, str):
|
||||||
|
return "cron schedule timezone must be a string"
|
||||||
|
tz = raw_tz.strip() if isinstance(raw_tz, str) else ""
|
||||||
|
return CronSchedule(kind="cron", expr=expr, tz=tz or None)
|
||||||
if kind == "at":
|
if kind == "at":
|
||||||
at_ms = _positive_int(values.get("at_ms"))
|
at_ms = _positive_int(values.get("at_ms"))
|
||||||
if at_ms is None:
|
if at_ms is None:
|
||||||
@ -831,14 +849,32 @@ def _parse_automation_schedule(values: dict[str, Any]) -> CronSchedule | str:
|
|||||||
return "unknown schedule kind"
|
return "unknown schedule kind"
|
||||||
|
|
||||||
|
|
||||||
def _positive_int(value: Any) -> int | None:
|
def _validate_automation_schedule(schedule: CronSchedule) -> str | None:
|
||||||
if isinstance(value, bool):
|
if schedule.kind == "at":
|
||||||
|
if not schedule.at_ms or schedule.at_ms <= int(time.time() * 1000):
|
||||||
|
return "one-time schedule must be in the future"
|
||||||
return None
|
return None
|
||||||
|
if schedule.kind != "cron":
|
||||||
|
return None
|
||||||
|
|
||||||
try:
|
try:
|
||||||
parsed = int(value)
|
from datetime import datetime
|
||||||
except (TypeError, ValueError):
|
from zoneinfo import ZoneInfo
|
||||||
|
|
||||||
|
from croniter import croniter
|
||||||
|
|
||||||
|
tz = ZoneInfo(schedule.tz) if schedule.tz else datetime.now().astimezone().tzinfo
|
||||||
|
base = datetime.now(tz=tz)
|
||||||
|
croniter(schedule.expr, base).get_next(datetime)
|
||||||
|
except Exception:
|
||||||
|
return "cron schedule is invalid"
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def _positive_int(value: Any) -> int | None:
|
||||||
|
if isinstance(value, bool) or not isinstance(value, int):
|
||||||
return None
|
return None
|
||||||
return parsed if parsed > 0 else None
|
return value if value > 0 else None
|
||||||
|
|
||||||
|
|
||||||
def _is_websocket_channel_session_key(key: str) -> bool:
|
def _is_websocket_channel_session_key(key: str) -> bool:
|
||||||
|
|||||||
@ -889,6 +889,7 @@ async def test_webui_automations_route_lists_all_jobs_and_allows_user_actions(
|
|||||||
headers=auth,
|
headers=auth,
|
||||||
)
|
)
|
||||||
assert resp.status_code == 200
|
assert resp.status_code == 200
|
||||||
|
assert "wx-chat" not in resp.text
|
||||||
body = resp.json()
|
body = resp.json()
|
||||||
by_id = {job["id"]: job for job in body["jobs"]}
|
by_id = {job["id"]: job for job in body["jobs"]}
|
||||||
assert by_id[user_job.id]["protected"] is False
|
assert by_id[user_job.id]["protected"] is False
|
||||||
@ -898,7 +899,12 @@ async def test_webui_automations_route_lists_all_jobs_and_allows_user_actions(
|
|||||||
assert by_id[user_job.id]["origin"]["preview"] == "hi"
|
assert by_id[user_job.id]["origin"]["preview"] == "hi"
|
||||||
assert by_id[legacy_job.id]["payload"]["session_key"] == "unified:default"
|
assert by_id[legacy_job.id]["payload"]["session_key"] == "unified:default"
|
||||||
assert by_id[legacy_job.id]["origin"] is None
|
assert by_id[legacy_job.id]["origin"] is None
|
||||||
assert by_id[external_job.id]["origin"]["session_key"] == "weixin:wx-chat"
|
assert by_id[external_job.id]["payload"]["origin_channel"] == "weixin"
|
||||||
|
assert "session_key" not in by_id[external_job.id]["payload"]
|
||||||
|
assert "origin_chat_id" not in by_id[external_job.id]["payload"]
|
||||||
|
assert by_id[external_job.id]["origin"]["channel"] == "weixin"
|
||||||
|
assert "session_key" not in by_id[external_job.id]["origin"]
|
||||||
|
assert "chat_id" not in by_id[external_job.id]["origin"]
|
||||||
assert by_id[external_job.id]["origin"]["preview"] == ""
|
assert by_id[external_job.id]["origin"]["preview"] == ""
|
||||||
assert by_id["heartbeat"]["protected"] is True
|
assert by_id["heartbeat"]["protected"] is True
|
||||||
|
|
||||||
@ -927,6 +933,28 @@ async def test_webui_automations_route_lists_all_jobs_and_allows_user_actions(
|
|||||||
assert by_id[user_job.id]["schedule"]["expr"] == "0 9 * * *"
|
assert by_id[user_job.id]["schedule"]["expr"] == "0 9 * * *"
|
||||||
assert by_id[user_job.id]["schedule"]["tz"] == "UTC"
|
assert by_id[user_job.id]["schedule"]["tz"] == "UTC"
|
||||||
|
|
||||||
|
malformed_update = await _http_get(
|
||||||
|
f"{base_url}/api/webui/automations/update?id={user_job.id}",
|
||||||
|
headers={
|
||||||
|
**auth,
|
||||||
|
"X-Nanobot-Automation-Values": json.dumps({"message": ["bad"]}),
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert malformed_update.status_code == 400
|
||||||
|
assert cron.get_job(user_job.id).payload.message == "Ask the daily quiz"
|
||||||
|
|
||||||
|
invalid_cron_update = await _http_get(
|
||||||
|
f"{base_url}/api/webui/automations/update?id={user_job.id}",
|
||||||
|
headers={
|
||||||
|
**auth,
|
||||||
|
"X-Nanobot-Automation-Values": json.dumps(
|
||||||
|
{"schedule": {"kind": "cron", "expr": "not a cron", "tz": "UTC"}}
|
||||||
|
),
|
||||||
|
},
|
||||||
|
)
|
||||||
|
assert invalid_cron_update.status_code == 400
|
||||||
|
assert cron.get_job(user_job.id).schedule.expr == "0 9 * * *"
|
||||||
|
|
||||||
protected_update = await _http_get(
|
protected_update = await _http_get(
|
||||||
f"{base_url}/api/webui/automations/update?id=heartbeat",
|
f"{base_url}/api/webui/automations/update?id=heartbeat",
|
||||||
headers={
|
headers={
|
||||||
|
|||||||
@ -3583,7 +3583,7 @@ function AutomationRow({
|
|||||||
t(key, { defaultValue: fallback, ...(values ?? {}) });
|
t(key, { defaultValue: fallback, ...(values ?? {}) });
|
||||||
const status = automationStatus(job, tx);
|
const status = automationStatus(job, tx);
|
||||||
const origin = automationOriginLabel(job, tx);
|
const origin = automationOriginLabel(job, tx);
|
||||||
const originHref = job.origin?.channel === "websocket"
|
const originHref = job.origin?.channel === "websocket" && job.origin.session_key
|
||||||
? `#/chat/${encodeURIComponent(job.origin.session_key)}`
|
? `#/chat/${encodeURIComponent(job.origin.session_key)}`
|
||||||
: null;
|
: null;
|
||||||
const history = job.state.run_history ?? [];
|
const history = job.state.run_history ?? [];
|
||||||
@ -4257,7 +4257,7 @@ function automationOriginLabel(
|
|||||||
}
|
}
|
||||||
if (!origin) return tx("settings.automations.origin.unknown", "No linked chat");
|
if (!origin) return tx("settings.automations.origin.unknown", "No linked chat");
|
||||||
if (origin.channel !== "websocket") return automationChannelLabel(origin.channel, tx);
|
if (origin.channel !== "websocket") return automationChannelLabel(origin.channel, tx);
|
||||||
return origin.title || origin.preview || origin.session_key;
|
return origin.title || origin.preview || origin.session_key || automationChannelLabel(origin.channel, tx);
|
||||||
}
|
}
|
||||||
|
|
||||||
function automationChannelLabel(
|
function automationChannelLabel(
|
||||||
|
|||||||
@ -132,9 +132,9 @@ export interface SessionAutomationJob {
|
|||||||
}>;
|
}>;
|
||||||
};
|
};
|
||||||
origin?: {
|
origin?: {
|
||||||
session_key: string;
|
session_key?: string;
|
||||||
channel: string;
|
channel: string;
|
||||||
chat_id: string;
|
chat_id?: string;
|
||||||
title?: string;
|
title?: string;
|
||||||
preview?: string;
|
preview?: string;
|
||||||
} | null;
|
} | null;
|
||||||
|
|||||||
@ -395,9 +395,7 @@ describe("App layout", () => {
|
|||||||
payload: {
|
payload: {
|
||||||
message: "Send a quiz",
|
message: "Send a quiz",
|
||||||
kind: "agent_turn",
|
kind: "agent_turn",
|
||||||
session_key: "weixin:wx-chat",
|
|
||||||
origin_channel: "weixin",
|
origin_channel: "weixin",
|
||||||
origin_chat_id: "wx-chat",
|
|
||||||
},
|
},
|
||||||
state: {
|
state: {
|
||||||
next_run_at_ms: Date.UTC(2026, 3, 17, 11, 30, 0),
|
next_run_at_ms: Date.UTC(2026, 3, 17, 11, 30, 0),
|
||||||
@ -406,11 +404,9 @@ describe("App layout", () => {
|
|||||||
run_history: [],
|
run_history: [],
|
||||||
},
|
},
|
||||||
origin: {
|
origin: {
|
||||||
session_key: "weixin:wx-chat",
|
|
||||||
channel: "weixin",
|
channel: "weixin",
|
||||||
chat_id: "wx-chat",
|
title: "",
|
||||||
title: "Scheduled cron job triggered",
|
preview: "",
|
||||||
preview: "memory with dream state",
|
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user