approve_node never rotates an existing key; node update-check endpoint; real Update Node button (node-26#14, #15) #194

Merged
logan merged 1 commits from fix/node-14-15-update-endpoint into main 2026-10-04 01:17:32 -04:00
6 changed files with 352 additions and 16 deletions
+83 -2
View File
@@ -17,11 +17,13 @@ Auth is the token headers checked inline below, not the app-wide
Firebase/service-key dependency the rest of routers/nodes.py uses. Firebase/service-key dependency the rest of routers/nodes.py uses.
""" """
import hashlib import hashlib
import re
import secrets import secrets
import time import time
from typing import Optional from datetime import datetime, timezone
from typing import Literal, Optional
from fastapi import APIRouter, HTTPException, Header, Request from fastapi import APIRouter, HTTPException, Header, Request
from pydantic import BaseModel from pydantic import BaseModel, Field
from app.config import settings from app.config import settings
from app.internal import firestore as fstore from app.internal import firestore as fstore
from app.internal.logger import logger from app.internal.logger import logger
@@ -216,3 +218,82 @@ async def get_node_credentials(node_id: str, x_pickup_secret: Optional[str] = He
return CredentialsResponse(approval_status=approval_status) return CredentialsResponse(approval_status=approval_status)
return CredentialsResponse(approval_status=approval_status, api_key=key_doc["api_key"]) return CredentialsResponse(approval_status=approval_status, api_key=key_doc["api_key"])
# ---------------------------------------------------------------------------
# Host updater check-in (node-26#15)
#
# A node's host-level updater (Client/scripts/drb-update.sh, a systemd timer
# installed by install.sh) calls this over HTTPS on a timer. It is the
# out-of-band channel: it works for a node that is offline from MQTT or
# locked out of the broker entirely, which is exactly when an update or a
# credential fix is needed most.
#
# Auth, checked inline for THIS node_id only (never a fleet credential):
# Authorization: Bearer <api_key> -> must equal node_keys/{id}.api_key
# X-Pickup-Secret: <secret> -> must hash to nodes/{id}.pickup_secret_hash
# The pickup secret is accepted so a node whose api_key is missing/stale can
# still be told to update — it is the same credential that already returns
# the api_key itself, so it grants nothing new.
#
# The response is a single timestamp. The node compares it with the last
# request it applied and, if newer, pulls the repo/registry/ref IT was
# installed with. C2 cannot name a ref, an image or a command here, so a
# compromised C2 or a stolen node credential can at most make a node
# re-pull its own pinned software.
# ---------------------------------------------------------------------------
_SHA_RE = re.compile(r"^[0-9a-f]{7,40}$")
class UpdateCheckRequest(BaseModel):
sha: Optional[str] = Field(None, max_length=40)
applied_request_at: Optional[str] = Field(None, max_length=64)
result: Optional[Literal["ok", "failed"]] = None
class UpdateCheckResponse(BaseModel):
update_requested_at: Optional[str] = None
async def _authenticate_node(
node_id: str, authorization: Optional[str], x_pickup_secret: Optional[str],
) -> dict:
node = await fstore.doc_get("nodes", node_id)
if not node:
# 401, not 404: this route must not double as a node_id oracle.
raise HTTPException(401, "Invalid node credentials")
if authorization and authorization.lower().startswith("bearer "):
presented = authorization[7:].strip()
key_doc = await fstore.doc_get("node_keys", node_id)
stored = (key_doc or {}).get("api_key") or ""
if stored and presented and secrets.compare_digest(stored, presented):
return node
if x_pickup_secret and node.get("pickup_secret_hash"):
if secrets.compare_digest(_hash_secret(x_pickup_secret), node["pickup_secret_hash"]):
return node
raise HTTPException(401, "Invalid node credentials")
@router.post("/{node_id}/update-check", response_model=UpdateCheckResponse)
async def node_update_check(
node_id: str,
body: UpdateCheckRequest,
authorization: Optional[str] = Header(None),
x_pickup_secret: Optional[str] = Header(None),
):
node = await _authenticate_node(node_id, authorization, x_pickup_secret)
now = datetime.now(timezone.utc).isoformat()
updates: dict = {"updater_last_check_at": now}
if body.sha and _SHA_RE.match(body.sha):
updates["updater_sha"] = body.sha
requested = node.get("update_requested_at")
if body.applied_request_at and requested and body.applied_request_at == requested and body.result:
updates["update_applied_at"] = now
updates["update_result"] = body.result
try:
await fstore.doc_update("nodes", node_id, updates)
except Exception as e: # bookkeeping only — never fail the node's check on it
logger.warning(f"update-check: could not record check-in for {node_id!r}: {e}")
return UpdateCheckResponse(update_requested_at=requested)
+41 -3
View File
@@ -1,4 +1,5 @@
import secrets import secrets
from datetime import datetime, timezone
from typing import Dict, List, Optional from typing import Dict, List, Optional
from fastapi import APIRouter, HTTPException, Depends, Query from fastapi import APIRouter, HTTPException, Depends, Query
from pydantic import BaseModel from pydantic import BaseModel
@@ -43,7 +44,17 @@ async def approve_node(node_id: str, _: dict = Depends(require_admin_token)):
if not node: if not node:
raise HTTPException(404, f"Node '{node_id}' not found.") raise HTTPException(404, f"Node '{node_id}' not found.")
api_key = secrets.token_hex(32) # node-26#14: approve must NEVER rotate a key that already exists. The old
# behaviour minted a fresh key on every click and overwrote node_keys, so
# re-clicking Approve on a live node (or approving after a re-run) silently
# invalidated the key already on the device and locked it out of MQTT with
# no remote recovery. Rotation is an explicit, separate action
# (POST /nodes/{id}/reissue-key). An existing key is reused as-is; the
# dynsec upsert below still runs, so a second approve doubles as a safe
# "re-sync this node's broker login" without changing the credential.
key_doc = await fstore.doc_get("node_keys", node_id)
existing_key = (key_doc or {}).get("api_key")
api_key = existing_key or secrets.token_hex(32)
# dynsec FIRST, Firestore second: if the broker rejects/never confirms # dynsec FIRST, Firestore second: if the broker rejects/never confirms
# the new client, we must not tell Firestore (and the admin UI) the # the new client, we must not tell Firestore (and the admin UI) the
@@ -56,13 +67,40 @@ async def approve_node(node_id: str, _: dict = Depends(require_admin_token)):
logger.error(f"Approve {node_id!r}: dynsec upsert failed, NOT writing Firestore: {e}") logger.error(f"Approve {node_id!r}: dynsec upsert failed, NOT writing Firestore: {e}")
raise HTTPException(502, f"Could not provision MQTT credentials for node: {e}") raise HTTPException(502, f"Could not provision MQTT credentials for node: {e}")
await fstore.doc_set("node_keys", node_id, {"node_id": node_id, "api_key": api_key}, merge=False) if not existing_key:
await fstore.doc_set("node_keys", node_id, {"node_id": node_id, "api_key": api_key}, merge=False)
await fstore.doc_update("nodes", node_id, {"approval_status": "approved"}) await fstore.doc_update("nodes", node_id, {"approval_status": "approved"})
# TODO(mqtt-cutover): drop this MQTT push once nodes pull their key via # TODO(mqtt-cutover): drop this MQTT push once nodes pull their key via
# GET /nodes/{id}/credentials (routers/enrollment.py) exclusively — see # GET /nodes/{id}/credentials (routers/enrollment.py) exclusively — see
# MQTT-PUBLIC-AUTH-PLAN.md "Rollout order" step 6. Kept for node-26. # MQTT-PUBLIC-AUTH-PLAN.md "Rollout order" step 6. Kept for node-26.
mqtt_handler.publish_node_key(node_id, api_key) mqtt_handler.publish_node_key(node_id, api_key)
return {"ok": True} if existing_key:
logger.info(f"Approve {node_id!r}: key already exists — reused, not rotated")
return {"ok": True, "key_rotated": False, "already_had_key": bool(existing_key)}
@router.post("/{node_id}/request-update")
async def request_node_update(node_id: str, _: dict = Depends(require_admin_token)):
"""Ask a node to run its host-level updater (node-26#15).
Two channels, so a node that is offline or locked out of MQTT still gets
it: (1) ``update_requested_at`` on the node doc, which the node's host
updater reads over HTTPS on its timer (POST /nodes/{id}/update-check in
routers/enrollment.py); (2) the MQTT ``node_update`` command, best-effort,
which makes an online node's updater run within seconds.
Deliberately carries NO payload the node acts on beyond "update now": no
ref, no image name, no command. The node pulls only the repo/registry and
ref it was installed with — C2 cannot steer what code a node runs.
"""
node = await fstore.doc_get("nodes", node_id)
if not node:
raise HTTPException(404, f"Node '{node_id}' not found.")
requested_at = datetime.now(timezone.utc).isoformat()
await fstore.doc_update("nodes", node_id, {"update_requested_at": requested_at})
mqtt_sent = mqtt_handler.send_command(node_id, {"action": "node_update"})
logger.info(f"Update requested for {node_id!r} at {requested_at} (mqtt_sent={mqtt_sent})")
return {"ok": True, "update_requested_at": requested_at, "mqtt_sent": mqtt_sent}
@router.delete("/{node_id}", status_code=204) @router.delete("/{node_id}", status_code=204)
+171
View File
@@ -0,0 +1,171 @@
"""
node-26#14 / #15 — approve must not rotate an existing key, and the host
updater's HTTPS channel (request-update + update-check).
"""
from unittest.mock import AsyncMock, MagicMock, patch
from fastapi.testclient import TestClient
from app.main import app
from app.internal.auth import require_admin_token, require_service_or_firebase_token
from app.routers import enrollment, nodes
client = TestClient(app)
def setup_function():
app.dependency_overrides[require_admin_token] = lambda: {"admin": True}
app.dependency_overrides[require_service_or_firebase_token] = lambda: {"admin": True}
def teardown_function():
app.dependency_overrides.pop(require_admin_token, None)
app.dependency_overrides.pop(require_service_or_firebase_token, None)
def _store(node, key_doc):
return AsyncMock(side_effect=lambda c, i: {"nodes": node, "node_keys": key_doc}.get(c))
# ---------------------------------------------------------------------------
# approve_node
# ---------------------------------------------------------------------------
def _approve(node, key_doc):
with patch.object(nodes.fstore, "doc_get", _store(node, key_doc)), \
patch.object(nodes.fstore, "doc_set", AsyncMock()) as doc_set, \
patch.object(nodes.fstore, "doc_update", AsyncMock()) as doc_update, \
patch.object(nodes.dynsec, "upsert_node_client", AsyncMock()) as upsert, \
patch.object(nodes.mqtt_handler, "publish_node_key", MagicMock()) as publish:
resp = client.post("/nodes/n1/approve")
return resp, doc_set, doc_update, upsert, publish
def test_approve_already_keyed_node_reuses_key_and_never_overwrites_node_keys():
node = {"node_id": "n1", "approval_status": "approved"}
resp, doc_set, doc_update, upsert, publish = _approve(node, {"node_id": "n1", "api_key": "live-key"})
assert resp.status_code == 200
assert resp.json()["key_rotated"] is False
doc_set.assert_not_awaited()
upsert.assert_awaited_once_with("n1", "live-key")
publish.assert_called_once_with("n1", "live-key")
def test_approve_new_node_mints_a_key():
resp, doc_set, doc_update, upsert, publish = _approve({"node_id": "n1", "approval_status": "pending"}, None)
assert resp.status_code == 200
(_, _, written), kwargs = doc_set.await_args
assert len(written["api_key"]) == 64
assert kwargs == {"merge": False}
upsert.assert_awaited_once_with("n1", written["api_key"])
doc_update.assert_awaited_once_with("nodes", "n1", {"approval_status": "approved"})
def test_approve_twice_keeps_the_same_key():
store = {"nodes": {"node_id": "n1", "approval_status": "pending"}, "node_keys": None}
async def doc_get(c, i):
return store[c]
async def doc_set(c, i, data, merge=False):
store[c] = data
with patch.object(nodes.fstore, "doc_get", doc_get), \
patch.object(nodes.fstore, "doc_set", doc_set), \
patch.object(nodes.fstore, "doc_update", AsyncMock()), \
patch.object(nodes.dynsec, "upsert_node_client", AsyncMock()), \
patch.object(nodes.mqtt_handler, "publish_node_key", MagicMock()):
client.post("/nodes/n1/approve")
first = store["node_keys"]["api_key"]
client.post("/nodes/n1/approve")
assert store["node_keys"]["api_key"] == first
# ---------------------------------------------------------------------------
# request-update (admin)
# ---------------------------------------------------------------------------
def test_request_update_sets_flag_even_when_mqtt_is_down():
with patch.object(nodes.fstore, "doc_get", _store({"node_id": "n1"}, None)), \
patch.object(nodes.fstore, "doc_update", AsyncMock()) as doc_update, \
patch.object(nodes.mqtt_handler, "send_command", MagicMock(return_value=False)) as cmd:
resp = client.post("/nodes/n1/request-update")
assert resp.status_code == 200
body = resp.json()
assert body["mqtt_sent"] is False
doc_update.assert_awaited_once_with("nodes", "n1", {"update_requested_at": body["update_requested_at"]})
cmd.assert_called_once_with("n1", {"action": "node_update"})
def test_request_update_requires_admin():
app.dependency_overrides.pop(require_admin_token, None)
resp = client.post("/nodes/n1/request-update")
assert resp.status_code in (401, 403)
# ---------------------------------------------------------------------------
# update-check (node, HTTPS)
# ---------------------------------------------------------------------------
NODE = {
"node_id": "n1",
"pickup_secret_hash": enrollment._hash_secret("pickup-1"),
"update_requested_at": "2026-10-04T12:00:00+00:00",
}
def _check(headers, body=None, node=NODE, key_doc=None):
key_doc = key_doc if key_doc is not None else {"api_key": "k" * 64}
# A node holds no Firebase/service credential: prove the route needs none.
app.dependency_overrides.clear()
with patch.object(enrollment.fstore, "doc_get", _store(node, key_doc)), \
patch.object(enrollment.fstore, "doc_update", AsyncMock()) as doc_update:
resp = client.post("/nodes/n1/update-check", json=body or {}, headers=headers)
return resp, doc_update
def test_update_check_with_api_key_returns_request():
resp, doc_update = _check({"Authorization": "Bearer " + "k" * 64}, {"sha": "abc1234"})
assert resp.status_code == 200
assert resp.json() == {"update_requested_at": NODE["update_requested_at"]}
(_, _, updates), _ = doc_update.await_args
assert updates["updater_sha"] == "abc1234"
assert "update_applied_at" not in updates
def test_update_check_with_pickup_secret_works_for_a_keyless_node():
resp, _ = _check({"X-Pickup-Secret": "pickup-1"}, key_doc={})
assert resp.status_code == 200
def test_update_check_rejects_wrong_or_missing_credentials():
assert _check({"Authorization": "Bearer wrong"})[0].status_code == 401
assert _check({"X-Pickup-Secret": "wrong"})[0].status_code == 401
assert _check({})[0].status_code == 401
def test_update_check_does_not_reveal_unknown_node_ids():
resp, _ = _check({"X-Pickup-Secret": "pickup-1"}, node=None)
assert resp.status_code == 401
def test_update_check_empty_stored_key_never_matches_empty_bearer():
resp, _ = _check({"Authorization": "Bearer "}, key_doc={"api_key": ""})
assert resp.status_code == 401
def test_update_check_records_applied_request():
resp, doc_update = _check(
{"Authorization": "Bearer " + "k" * 64},
{"applied_request_at": NODE["update_requested_at"], "result": "ok", "sha": "not-a-sha!"},
)
assert resp.status_code == 200
(_, _, updates), _ = doc_update.await_args
assert updates["update_result"] == "ok"
assert "update_applied_at" in updates
assert "updater_sha" not in updates # junk sha is never written
def test_update_check_rejects_unknown_result_value():
resp, _ = _check({"Authorization": "Bearer " + "k" * 64}, {"result": "rm -rf /"})
assert resp.status_code == 422
+44 -11
View File
@@ -146,6 +146,32 @@ export default function NodeDetailPage() {
} }
} }
async function handleRequestUpdate() {
if (
!confirm(
"Update this node's software?\n\n" +
"The node pulls the latest release it was installed to track and recreates its containers " +
"(radio capture pauses for about a minute). An online node starts within seconds; an offline " +
"or locked-out node picks it up on its next check-in (up to ~15 minutes). Nodes installed " +
"before the host updater existed need install.sh re-run once.",
)
)
return;
setSending(true);
try {
const r = await c2api.requestNodeUpdate(id);
alert(
r.mqtt_sent
? "Update requested. The node should restart within a minute."
: "Update requested. The node is not reachable over MQTT right now; it will update on its next HTTPS check-in.",
);
} catch (err) {
alert(err instanceof Error ? err.message : "Update request failed.");
} finally {
setSending(false);
}
}
async function handleApprove() { async function handleApprove() {
setApproving(true); setApproving(true);
try { try {
@@ -251,6 +277,15 @@ export default function NodeDetailPage() {
["System", system?.name ?? "Unassigned"], ["System", system?.name ?? "Unassigned"],
["Location", `${node.lat}, ${node.lon}`], ["Location", `${node.lat}, ${node.lon}`],
["Last Seen", node.last_seen ? new Date(node.last_seen).toLocaleString() : "never"], ["Last Seen", node.last_seen ? new Date(node.last_seen).toLocaleString() : "never"],
["Updater Check-in", node.updater_last_check_at ? new Date(node.updater_last_check_at).toLocaleString() : "never"],
...(node.update_requested_at
? [[
"Last Update",
node.update_applied_at && node.update_applied_at >= node.update_requested_at
? `${node.update_result === "failed" ? "FAILED" : "applied"} ${new Date(node.update_applied_at).toLocaleString()}`
: `requested ${new Date(node.update_requested_at).toLocaleString()} — pending`,
]]
: []),
["Configured", node.configured ? "Yes" : "No"], ["Configured", node.configured ? "Yes" : "No"],
["Node Type", node.node_type ?? "fixed"], ["Node Type", node.node_type ?? "fixed"],
...(node.node_type !== "portable" ? [["Enforce Timeout", node.enforce_override_timeout ? "Yes" : "No"]] : []), ...(node.node_type !== "portable" ? [["Enforce Timeout", node.enforce_override_timeout ? "Yes" : "No"]] : []),
@@ -302,17 +337,15 @@ export default function NodeDetailPage() {
> >
Restart OP25 Restart OP25
</button> </button>
<button {isAdmin && (
disabled={sending} <button
onClick={() => { disabled={sending}
if (confirm("Restart the node container to apply any pulled image updates?")) { onClick={handleRequestUpdate}
sendCommand("node_update"); className="px-4 py-2 bg-gray-800 hover:bg-gray-700 text-gray-300 rounded-lg text-sm font-mono transition-colors disabled:opacity-50"
} >
}} Update Node Software
className="px-4 py-2 bg-gray-800 hover:bg-gray-700 text-gray-300 rounded-lg text-sm font-mono transition-colors disabled:opacity-50" </button>
> )}
Update Node
</button>
<button <button
onClick={() => setShowDiscordJoin(true)} onClick={() => setShowDiscordJoin(true)}
className="px-4 py-2 bg-gray-800 hover:bg-gray-700 text-gray-300 rounded-lg text-sm font-mono transition-colors" className="px-4 py-2 bg-gray-800 hover:bg-gray-700 text-gray-300 rounded-lg text-sm font-mono transition-colors"
+6
View File
@@ -65,6 +65,12 @@ export const c2api = {
// Node approval // Node approval
approveNode: (id: string) => approveNode: (id: string) =>
request(`/nodes/${id}/approve`, { method: "POST" }), request(`/nodes/${id}/approve`, { method: "POST" }),
/** node-26#15: flag the node for its host updater (HTTPS) and nudge it over MQTT. */
requestNodeUpdate: (id: string) =>
request<{ ok: boolean; update_requested_at: string; mqtt_sent: boolean }>(
`/nodes/${id}/request-update`,
{ method: "POST" },
),
rejectNode: (id: string) => rejectNode: (id: string) =>
request(`/nodes/${id}/reject`, { method: "POST" }), request(`/nodes/${id}/reject`, { method: "POST" }),
deleteNode: (id: string) => deleteNode: (id: string) =>
+7
View File
@@ -72,6 +72,13 @@ export interface NodeRecord {
is_overridden?: boolean; is_overridden?: boolean;
override_system_id?: string | null; override_system_id?: string | null;
override_timeout_at?: string | null; override_timeout_at?: string | null;
/** node-26#15 host updater: last "Update Node" request, and what the node did with it. */
update_requested_at?: string | null;
update_applied_at?: string | null;
update_result?: "ok" | "failed" | null;
/** Last HTTPS check-in from the host updater — alive even when MQTT is not. */
updater_last_check_at?: string | null;
updater_sha?: string | null;
} }
export interface SdrDevice { export interface SdrDevice {