hardening: fix command-injection in agent check_command + add double-launch guard
- check_agent() custom check_type: replace subprocess shell=True with shlex.split + shell=False (prevents agent-registry config from injecting arbitrary shell). Verified malicious '; touch' no longer executes. - Add startup port guard in main: probe API port before uvicorn.run; exit 1 with clear message if already bound (stops the double-instance collision that caused phantom 'register does not persist' bugs). - start_terminal_server (8082): pre-bind probe + try/except so a taken port logs a warning instead of crashing the whole process.
This commit is contained in:
parent
78eeaa9857
commit
aba5120da2
|
|
@ -13,8 +13,13 @@
|
||||||
- New skills added: firebase-* (10), notion-knowledge-capture, xcode-project-setup, audit-test-plugin, test-plugin.
|
- New skills added: firebase-* (10), notion-knowledge-capture, xcode-project-setup, audit-test-plugin, test-plugin.
|
||||||
- `.gitignore` extended to exclude runtime artifacts (pid, graphify-out, logs).
|
- `.gitignore` extended to exclude runtime artifacts (pid, graphify-out, logs).
|
||||||
- VERIFIED live: server on :8081, all new endpoints return 200; register/unregister persists correctly.
|
- VERIFIED live: server on :8081, all new endpoints return 200; register/unregister persists correctly.
|
||||||
- CAVEAT (not yet fixed): agent `check_command` runs via `subprocess.run(..., shell=True)` — command-injection risk if a malicious agent config is registered. Local-admin-only exposure; flag for later hardening.
|
- CAVEAT (FIXED 2026-07-25): agent `check_command` previously ran via `subprocess.run(..., shell=True)` — command-injection risk. Now uses `shlex.split` + `shell=False`. Verified: malicious `"; touch /tmp/PWNED"` no longer executes.
|
||||||
- CAVEAT: avoid running two `server.py` instances on the same port — registration on one won't show in reads served by the other.
|
- CAVEAT (FIXED 2026-07-25): running two `server.py` on same port caused phantom "register doesn't persist" + 8082 bind crash. Added startup port guard (exits cleanly with message) + terminal websocket now binds gracefully instead of crashing. Verified: 2nd instance bails with exit 1.
|
||||||
|
|
||||||
|
## 2026-07-25 — Security & ops hardening (commit pending)
|
||||||
|
- Fixed command-injection in `check_agent()` custom check_type: `shell=True` → `shlex.split` + `shell=False`.
|
||||||
|
- Added double-launch guard in `main`: probe API port (8081) before `uvicorn.run`; exit 1 with clear message if already bound.
|
||||||
|
- Made `start_terminal_server` (8082) fail gracefully — pre-bind probe + try/except, logs warning instead of crashing the process.
|
||||||
|
|
||||||
## Archived (older than 30 days)
|
## Archived (older than 30 days)
|
||||||
|
|
||||||
|
|
|
||||||
64
server.py
64
server.py
|
|
@ -310,11 +310,20 @@ def check_agent(name: str) -> dict:
|
||||||
except Exception:
|
except Exception:
|
||||||
status = "offline" if not exists else "warning"
|
status = "offline" if not exists else "warning"
|
||||||
elif check_type == "custom":
|
elif check_type == "custom":
|
||||||
# Run a custom check command
|
# Run a custom check command. IMPORTANT: never use shell=True —
|
||||||
|
# check_command comes from agent-registry config and a malicious
|
||||||
|
# value ("; rm -rf ~") would otherwise execute arbitrary shell.
|
||||||
check_cmd = agent.get("check_command", "")
|
check_cmd = agent.get("check_command", "")
|
||||||
if check_cmd:
|
if check_cmd:
|
||||||
result = subprocess.run(check_cmd, shell=True, capture_output=True, timeout=10)
|
import shlex
|
||||||
status = "online" if result.returncode == 0 else "offline"
|
try:
|
||||||
|
cmd = shlex.split(check_cmd)
|
||||||
|
result = subprocess.run(
|
||||||
|
cmd, shell=False, capture_output=True, timeout=10
|
||||||
|
)
|
||||||
|
status = "online" if result.returncode == 0 else "offline"
|
||||||
|
except (ValueError, OSError):
|
||||||
|
status = "offline"
|
||||||
else:
|
else:
|
||||||
status = "online" if exists else "offline"
|
status = "online" if exists else "offline"
|
||||||
else:
|
else:
|
||||||
|
|
@ -1889,7 +1898,25 @@ except ImportError:
|
||||||
|
|
||||||
|
|
||||||
def start_terminal_server(host="0.0.0.0", port=8082):
|
def start_terminal_server(host="0.0.0.0", port=8082):
|
||||||
"""Start the WebSocket terminal server on a separate port."""
|
"""Start the WebSocket terminal server on a separate port.
|
||||||
|
|
||||||
|
If the port is already in use (e.g. a second server.py instance), fail
|
||||||
|
*gracefully* — log and return instead of crashing the whole process.
|
||||||
|
"""
|
||||||
|
import socket
|
||||||
|
|
||||||
|
# Pre-check: is the port already bound by another process?
|
||||||
|
probe = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
|
||||||
|
try:
|
||||||
|
probe.bind((host, port))
|
||||||
|
probe.close()
|
||||||
|
except OSError as e:
|
||||||
|
print(
|
||||||
|
f"[terminal] WARNING: cannot bind {host}:{port} "
|
||||||
|
f"({e}). Terminal websocket disabled; API still running."
|
||||||
|
)
|
||||||
|
return
|
||||||
|
|
||||||
async def serve():
|
async def serve():
|
||||||
server = await websockets.serve(
|
server = await websockets.serve(
|
||||||
terminal_handler,
|
terminal_handler,
|
||||||
|
|
@ -1897,12 +1924,16 @@ def start_terminal_server(host="0.0.0.0", port=8082):
|
||||||
port,
|
port,
|
||||||
max_size=10 * 1024 * 1024,
|
max_size=10 * 1024 * 1024,
|
||||||
)
|
)
|
||||||
|
print(f"[terminal] WebSocket terminal listening on ws://{host}:{port}")
|
||||||
return server
|
return server
|
||||||
|
|
||||||
loop = asyncio.new_event_loop()
|
loop = asyncio.new_event_loop()
|
||||||
asyncio.set_event_loop(loop)
|
asyncio.set_event_loop(loop)
|
||||||
server = loop.run_until_complete(serve())
|
try:
|
||||||
loop.run_forever()
|
server = loop.run_until_complete(serve())
|
||||||
|
loop.run_forever()
|
||||||
|
except OSError as e:
|
||||||
|
print(f"[terminal] WARNING: terminal server failed to start: {e}")
|
||||||
|
|
||||||
|
|
||||||
# Start terminal server in a daemon thread
|
# Start terminal server in a daemon thread
|
||||||
|
|
@ -2025,9 +2056,30 @@ def favicon_svg():
|
||||||
# ─── Main ─────────────────────────────────────────────────────────
|
# ─── Main ─────────────────────────────────────────────────────────
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
|
import argparse
|
||||||
|
import socket
|
||||||
|
|
||||||
import uvicorn
|
import uvicorn
|
||||||
|
|
||||||
parser = argparse.ArgumentParser()
|
parser = argparse.ArgumentParser()
|
||||||
parser.add_argument("--port", type=int, default=8080)
|
parser.add_argument("--port", type=int, default=8080)
|
||||||
parser.add_argument("--host", type=str, default="0.0.0.0")
|
parser.add_argument("--host", type=str, default="0.0.0.0")
|
||||||
args = parser.parse_args()
|
args = parser.parse_args()
|
||||||
|
|
||||||
|
# ── Double-launch guard ───────────────────────────────────────────
|
||||||
|
# Refuse to start if the API port is already bound by another process.
|
||||||
|
# Without this, two server.py instances collide on 8081 (and 8082),
|
||||||
|
# producing phantom "register doesn't persist" bugs and bind crashes.
|
||||||
|
probe = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
|
||||||
|
try:
|
||||||
|
probe.bind((args.host, args.port))
|
||||||
|
probe.close()
|
||||||
|
except OSError:
|
||||||
|
print(
|
||||||
|
f"ERROR: port {args.port} on {args.host} is already in use. "
|
||||||
|
f"Agentic OS appears to be running already — stop it first "
|
||||||
|
f"(./start.sh --stop) before launching another instance."
|
||||||
|
)
|
||||||
|
raise SystemExit(1)
|
||||||
|
|
||||||
uvicorn.run(app, host=args.host, port=args.port)
|
uvicorn.run(app, host=args.host, port=args.port)
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue