From f37b8a501822344ed814505499f845c92d868973 Mon Sep 17 00:00:00 2001 From: savsis Date: Mon, 14 Sep 2026 03:25:49 +0500 Subject: [PATCH] =?UTF-8?q?fix:=20validate=20node=20code/label/port=20when?= =?UTF-8?q?=20manually=20adding=20a=20node=20=E2=80=94=20was=20completely?= =?UTF-8?q?=20unvalidated?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New lens this pass: read every admin-mutating route for input validation, not just auth (already audited that separately). Found POST /admin/api/nodes taking `code` straight from the request body with zero checks — reachable for real from admin.html's "Вручную" add-node tab (nm-code is a free-text field), not just a theoretical API-only path. code is the table's PRIMARY KEY and gets embedded directly into every /admin/api/nodes/{code}/... URL afterward. Concretely: an empty code, one containing a slash, or one that collides with an existing node would all previously either succeed into a node the UI can no longer address by its own generated URLs, or crash with a raw unhandled sqlite3.IntegrityError / ValueError instead of a real error message. None of this needed a live server to reproduce — it's pure input handling. Traced kind="external" nodes first before touching anything near them, since add_client_to_node/remove_client_from_node only branch on "local"/"managed" with no external case — worth being sure that's the intentional "this node's clients are managed outside the panel, we just reference a fixed shared_uuid" design (confirmed via links.py's own use of shared_uuid) and not an actual bug before writing validation around it. NODE_CODE_RE (same style as the existing HWID_RE): letters/digits/-/_, 1-32 chars — covers "de1", the "n"+hex(4) auto-generated codes, and any reasonable manual name, rejects anything that would break URL routing or silently create an unreachable node. Non-empty label. Port coerced and range-checked (1-65535) instead of a bare int() that throws on garbage input. Duplicate code now raises a ValueError from db.create_node (pre-checked via get_node(), same pattern create_admin already uses for duplicate usernames — not a bolted-on try/except IntegrityError) which the route turns into a real 400. Deliberately scoped to creation only — code isn't in admin_update_node's editable set, so there's no separate update-path gap to also close. Verification: db.create_node's duplicate guard tested directly against a real sqlite db (fresh code succeeds, immediate duplicate attempt raises and leaves the original untouched, a second distinct code still works). NODE_CODE_RE run through 12 cases — valid codes including the real auto-generated shape, and the specific invalid ones that matter (slash, space, unicode, empty, over-length, exactly-at-the-length- limit). AST-extracted the whole updated admin_create_node() route (still can't import api.py) and drove it through a fake db/webhooks/ HTTPException with 9 cases covering every rejection branch, the happy path (including that node.added still fires with the right payload), and the duplicate-code path specifically, confirming the ValueError from db.py correctly surfaces as an HTTP 400 rather than an unhandled exception. Co-Authored-By: Claude Sonnet 5 --- api.py | 30 +++++++++++++++++++++++------- db.py | 2 ++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/api.py b/api.py index eee0d16..71e2c39 100644 --- a/api.py +++ b/api.py @@ -24,6 +24,7 @@ import xray_manager from config import SITE_DOMAIN, SUB_DOMAIN, PANEL_DOMAIN, ADMIN_PATH, BASE_DIR HWID_RE = re.compile(r"^[a-zA-Z0-9=-]{10,64}$") +NODE_CODE_RE = re.compile(r"^[a-zA-Z0-9_-]{1,32}$") ENV_PATH = os.path.join(BASE_DIR, ".env") db.init_db() @@ -1079,13 +1080,28 @@ def admin_reorder_nodes(request: Request, body: dict = Body(...)): @app.post("/admin/api/nodes") def admin_create_node(request: Request, body: dict = Body(...)): require_admin(request) - node = db.create_node( - code=body["code"], label=body["label"], kind=body.get("kind", "external"), - address=body["address"], port=int(body.get("port", 443)), - public_key=body["public_key"], short_id=body["short_id"], - sni=body["sni"], flow=body.get("flow", "xtls-rprx-vision"), - shared_uuid=body.get("shared_uuid"), - ) + code = str(body.get("code", "")).strip() + if not NODE_CODE_RE.match(code): + raise HTTPException(400, "code: только буквы/цифры/-/_, от 1 до 32 символов") + label = str(body.get("label", "")).strip() + if not label: + raise HTTPException(400, "label не может быть пустым") + try: + port = int(body.get("port", 443)) + except (TypeError, ValueError): + raise HTTPException(400, "port должен быть числом") + if not (1 <= port <= 65535): + raise HTTPException(400, "port должен быть от 1 до 65535") + try: + node = db.create_node( + code=code, label=label, kind=body.get("kind", "external"), + address=body["address"], port=port, + public_key=body["public_key"], short_id=body["short_id"], + sni=body["sni"], flow=body.get("flow", "xtls-rprx-vision"), + shared_uuid=body.get("shared_uuid"), + ) + except ValueError as e: + raise HTTPException(400, str(e)) webhooks.send("node.added", {"code": node["code"], "label": node["label"], "kind": node["kind"]}) return node diff --git a/db.py b/db.py index 89ff292..68dd516 100644 --- a/db.py +++ b/db.py @@ -287,6 +287,8 @@ def reorder_nodes(codes: list): def create_node(code, label, kind, address, port, public_key, short_id, sni, flow, shared_uuid=None): + if get_node(code): + raise ValueError("node with this code already exists") with get_conn() as conn: next_order = _next_sort_order(conn) conn.execute(