Skip to content

Commit 0d6e99b

Browse files
committed
fix(security): add worker-name auth check and concurrency guard to deploy_backend
1. Worker-name authorization: deploy_backend now calls require_worker_hmac and verifies the authenticated worker matches the path name. Previously the endpoint had no auth at all - any caller could trigger deploys on any worker. Now only the paired worker can deploy on itself (CodeRabbit, PR jaylfc#1910). 2. Concurrency guard: added per-worker asyncio.Lock (_worker_deploy_locks) to both deploy_backend and _do_single_worker_update. Prevents concurrent API calls from double-installing or double-draining the same worker.
1 parent f967846 commit 0d6e99b

1 file changed

Lines changed: 51 additions & 10 deletions

File tree

‎tinyagentos/routes/cluster.py‎

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -917,14 +917,39 @@ class WorkerRemoteRequest(BaseModel):
917917
]
918918

919919

920+
# Per-worker deploy locks -- prevents concurrent install/restart on the
921+
# same worker (CodeRabbit finding on PR #1910).
922+
import collections
923+
_worker_deploy_locks: dict[str, asyncio.Lock] = collections.defaultdict(asyncio.Lock)
924+
925+
920926
@router.post("/api/cluster/workers/{name}/deploy")
921927
async def deploy_backend(request: Request, name: str, body: DeployRequest):
922928
"""Trigger a backend install on a remote worker.
923929
924930
The controller proxies this to the worker's deploy endpoint. The
925931
worker runs taos-deploy-helper.sh via passwordless sudo. Only
926932
commands in the fixed allowlist are accepted.
933+
934+
HMAC-gated: the worker signs the request with its pairing key, and
935+
the authenticated worker name must match the URL path. This prevents
936+
worker A from triggering a deploy on worker B (CodeRabbit, PR #1910).
937+
938+
Per-worker asyncio.Lock prevents concurrent install/restart calls on
939+
the same worker from double-installing.
927940
"""
941+
# HMAC gate -- only paired workers may trigger deploys, and only on themselves.
942+
try:
943+
await require_worker_hmac(request)
944+
except _HMACError as exc:
945+
return exc.response
946+
# Verify the HMAC-authenticated worker matches the path name.
947+
if getattr(request.state, "hmac_worker_name", None) != name:
948+
return JSONResponse(
949+
{"error": "Worker name in header does not match path"},
950+
status_code=403,
951+
)
952+
928953
cluster = request.app.state.cluster_manager
929954
worker = cluster.get_worker(name)
930955
if not worker:
@@ -937,16 +962,19 @@ async def deploy_backend(request: Request, name: str, body: DeployRequest):
937962
status_code=400,
938963
)
939964

940-
import httpx
941-
try:
942-
async with httpx.AsyncClient(timeout=620) as client:
943-
resp = await client.post(
944-
f"{worker.url}/api/worker/deploy",
945-
json={"command": body.command},
946-
)
947-
return resp.json()
948-
except Exception as exc:
949-
return JSONResponse({"error": str(exc)}, status_code=502)
965+
# Serialise deploys per worker -- concurrent calls can double-install.
966+
lock = _worker_deploy_locks[name]
967+
async with lock:
968+
import httpx
969+
try:
970+
async with httpx.AsyncClient(timeout=620) as client:
971+
resp = await client.post(
972+
f"{worker.url}/api/worker/deploy",
973+
json={"command": body.command},
974+
)
975+
return resp.json()
976+
except Exception as exc:
977+
return JSONResponse({"error": str(exc)}, status_code=502)
950978

951979

952980
@router.post("/api/cluster/workers/{name}/remote")
@@ -1239,9 +1267,22 @@ async def _do_single_worker_update(cluster, worker) -> dict:
12391267
On success: ``{"success": True, "worker": ..., "status": "updating", ...}``.
12401268
On failure: ``{"success": False, "worker": ..., "error": "..."}``.
12411269
Never raises -- all exceptions are caught and converted into error dicts.
1270+
1271+
Per-worker asyncio.Lock prevents concurrent update calls on the same
1272+
worker from double-draining or double-deploying (CodeRabbit, PR #1910).
12421273
"""
12431274
name = worker.name
12441275

1276+
# Serialise updates per worker -- concurrent calls can double-drain/deploy.
1277+
lock = _worker_deploy_locks[name]
1278+
async with lock:
1279+
return await _do_single_worker_update_locked(cluster, worker)
1280+
1281+
1282+
async def _do_single_worker_update_locked(cluster, worker) -> dict:
1283+
"""Inner implementation of _do_single_worker_update (lock held)."""
1284+
name = worker.name
1285+
12451286
# Step 1: Begin draining (with exception isolation -- drain_worker
12461287
# may raise from notification or background-task failures).
12471288
try:

0 commit comments

Comments
 (0)