fix: validate node code/label/port when manually adding a node — was completely unvalidated
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 <noreply@anthropic.com>
This commit is contained in:
parent
bbbab9998e
commit
f37b8a5018
2 changed files with 25 additions and 7 deletions
30
api.py
30
api.py
|
|
@ -24,6 +24,7 @@ import xray_manager
|
||||||
from config import SITE_DOMAIN, SUB_DOMAIN, PANEL_DOMAIN, ADMIN_PATH, BASE_DIR
|
from config import SITE_DOMAIN, SUB_DOMAIN, PANEL_DOMAIN, ADMIN_PATH, BASE_DIR
|
||||||
|
|
||||||
HWID_RE = re.compile(r"^[a-zA-Z0-9=-]{10,64}$")
|
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")
|
ENV_PATH = os.path.join(BASE_DIR, ".env")
|
||||||
|
|
||||||
db.init_db()
|
db.init_db()
|
||||||
|
|
@ -1079,13 +1080,28 @@ def admin_reorder_nodes(request: Request, body: dict = Body(...)):
|
||||||
@app.post("/admin/api/nodes")
|
@app.post("/admin/api/nodes")
|
||||||
def admin_create_node(request: Request, body: dict = Body(...)):
|
def admin_create_node(request: Request, body: dict = Body(...)):
|
||||||
require_admin(request)
|
require_admin(request)
|
||||||
node = db.create_node(
|
code = str(body.get("code", "")).strip()
|
||||||
code=body["code"], label=body["label"], kind=body.get("kind", "external"),
|
if not NODE_CODE_RE.match(code):
|
||||||
address=body["address"], port=int(body.get("port", 443)),
|
raise HTTPException(400, "code: только буквы/цифры/-/_, от 1 до 32 символов")
|
||||||
public_key=body["public_key"], short_id=body["short_id"],
|
label = str(body.get("label", "")).strip()
|
||||||
sni=body["sni"], flow=body.get("flow", "xtls-rprx-vision"),
|
if not label:
|
||||||
shared_uuid=body.get("shared_uuid"),
|
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"]})
|
webhooks.send("node.added", {"code": node["code"], "label": node["label"], "kind": node["kind"]})
|
||||||
return node
|
return node
|
||||||
|
|
||||||
|
|
|
||||||
2
db.py
2
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):
|
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:
|
with get_conn() as conn:
|
||||||
next_order = _next_sort_order(conn)
|
next_order = _next_sort_order(conn)
|
||||||
conn.execute(
|
conn.execute(
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue