feat: terminal fullscreen, asyncpg fix, CI improvements #59
@@ -70,6 +70,13 @@ CC_CONNECT_URL = os.environ.get("CC_CONNECT_URL", "")
|
|||||||
# OpenClaw
|
# OpenClaw
|
||||||
OPENCLAW_GATEWAY_TOKEN = os.environ.get("OPENCLAW_GATEWAY_TOKEN", "")
|
OPENCLAW_GATEWAY_TOKEN = os.environ.get("OPENCLAW_GATEWAY_TOKEN", "")
|
||||||
|
|
||||||
|
# Transmission RPC
|
||||||
|
TRANSMISSION_URL = os.environ.get("TRANSMISSION_URL", "http://host.docker.internal:9091/transmission/rpc")
|
||||||
|
TRANSMISSION_USER = os.environ.get("TRANSMISSION_USER", "")
|
||||||
|
TRANSMISSION_PASS = os.environ.get("TRANSMISSION_PASS", "")
|
||||||
|
if not TRANSMISSION_USER or not TRANSMISSION_PASS:
|
||||||
|
raise RuntimeError("TRANSMISSION_USER and TRANSMISSION_PASS must be set")
|
||||||
|
|
||||||
# Networking configs
|
# Networking configs
|
||||||
LAN_IP_RANGES = os.environ.get("LAN_IP_RANGES", "192.168.31.0/24").split(",")
|
LAN_IP_RANGES = os.environ.get("LAN_IP_RANGES", "192.168.31.0/24").split(",")
|
||||||
TRUSTED_PROXIES = os.environ.get("TRUSTED_PROXIES", "127.0.0.1,::1,172.21.0.1").split(",")
|
TRUSTED_PROXIES = os.environ.get("TRUSTED_PROXIES", "127.0.0.1,::1,172.21.0.1").split(",")
|
||||||
|
|||||||
@@ -6,15 +6,14 @@ import logging
|
|||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
from fastapi import APIRouter, WebSocket, WebSocketDisconnect
|
from fastapi import APIRouter, HTTPException, Query, WebSocket, WebSocketDisconnect, status
|
||||||
|
|
||||||
|
import auth_service as auth
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
router = APIRouter()
|
router = APIRouter()
|
||||||
|
|
||||||
# Active WebSocket connections
|
|
||||||
active_connections: set[WebSocket] = set()
|
|
||||||
|
|
||||||
|
|
||||||
def serialize_datetime(obj: Any) -> Any:
|
def serialize_datetime(obj: Any) -> Any:
|
||||||
"""Recursively serialize datetime objects to ISO format strings"""
|
"""Recursively serialize datetime objects to ISO format strings"""
|
||||||
@@ -29,21 +28,21 @@ def serialize_datetime(obj: Any) -> Any:
|
|||||||
|
|
||||||
|
|
||||||
class ConnectionManager:
|
class ConnectionManager:
|
||||||
"""Manages WebSocket connections"""
|
"""Manages WebSocket connections with per-user tracking"""
|
||||||
|
|
||||||
def __init__(self):
|
def __init__(self):
|
||||||
self.active_connections: set[WebSocket] = set()
|
self.active_connections: dict[WebSocket, str] = {}
|
||||||
|
|
||||||
async def connect(self, websocket: WebSocket):
|
async def connect(self, websocket: WebSocket, username: str):
|
||||||
"""Accept and register a new connection"""
|
"""Accept and register a new connection"""
|
||||||
await websocket.accept()
|
await websocket.accept()
|
||||||
self.active_connections.add(websocket)
|
self.active_connections[websocket] = username
|
||||||
logger.info(f"WebSocket connected. Total connections: {len(self.active_connections)}")
|
logger.info(f"WebSocket connected: {username}. Total connections: {len(self.active_connections)}")
|
||||||
|
|
||||||
def disconnect(self, websocket: WebSocket):
|
def disconnect(self, websocket: WebSocket):
|
||||||
"""Remove a connection"""
|
"""Remove a connection"""
|
||||||
self.active_connections.discard(websocket)
|
username = self.active_connections.pop(websocket, "unknown")
|
||||||
logger.info(f"WebSocket disconnected. Total connections: {len(self.active_connections)}")
|
logger.info(f"WebSocket disconnected: {username}. Total connections: {len(self.active_connections)}")
|
||||||
|
|
||||||
async def broadcast(self, message: dict):
|
async def broadcast(self, message: dict):
|
||||||
"""Broadcast message to all connected clients"""
|
"""Broadcast message to all connected clients"""
|
||||||
@@ -64,9 +63,19 @@ manager = ConnectionManager()
|
|||||||
|
|
||||||
|
|
||||||
@router.websocket("/ws")
|
@router.websocket("/ws")
|
||||||
async def websocket_endpoint(websocket: WebSocket):
|
async def websocket_endpoint(websocket: WebSocket, token: str = Query(None)):
|
||||||
"""WebSocket endpoint for OPC real-time updates"""
|
"""WebSocket endpoint for OPC real-time updates. Requires JWT token via ?token= query param."""
|
||||||
await manager.connect(websocket)
|
if not token:
|
||||||
|
await websocket.close(code=status.WS_1008_POLICY_VIOLATION, reason="Missing authentication token")
|
||||||
|
return
|
||||||
|
|
||||||
|
try:
|
||||||
|
user = auth.user_from_access_token(token, include_auth_header=False)
|
||||||
|
except HTTPException:
|
||||||
|
await websocket.close(code=status.WS_1008_POLICY_VIOLATION, reason="Invalid authentication token")
|
||||||
|
return
|
||||||
|
|
||||||
|
await manager.connect(websocket, user.username)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
while True:
|
while True:
|
||||||
|
|||||||
@@ -56,7 +56,8 @@ def _get_challenge(client_data_b64: str) -> bytes:
|
|||||||
|
|
||||||
|
|
||||||
@router.post("/passkey/register/options")
|
@router.post("/passkey/register/options")
|
||||||
async def passkey_register_options(current_user=Depends(auth.get_current_user)):
|
@limiter.limit("5/minute")
|
||||||
|
async def passkey_register_options(request: Request, current_user=Depends(auth.get_current_user)):
|
||||||
"""Generate WebAuthn registration options for adding a new passkey."""
|
"""Generate WebAuthn registration options for adding a new passkey."""
|
||||||
existing = auth.load_passkey_credentials()
|
existing = auth.load_passkey_credentials()
|
||||||
exclude = [PublicKeyCredentialDescriptor(id=base64url_to_bytes(c["credential_id"])) for c in existing]
|
exclude = [PublicKeyCredentialDescriptor(id=base64url_to_bytes(c["credential_id"])) for c in existing]
|
||||||
|
|||||||
@@ -4,6 +4,8 @@ from typing import Any, Dict, List
|
|||||||
import httpx
|
import httpx
|
||||||
from fastapi import APIRouter, HTTPException
|
from fastapi import APIRouter, HTTPException
|
||||||
|
|
||||||
|
import config
|
||||||
|
|
||||||
router = APIRouter(prefix="/api/transmission", tags=["transmission"])
|
router = APIRouter(prefix="/api/transmission", tags=["transmission"])
|
||||||
|
|
||||||
|
|
||||||
@@ -12,8 +14,8 @@ def get_session_id() -> str:
|
|||||||
try:
|
try:
|
||||||
with httpx.Client(timeout=5.0) as client:
|
with httpx.Client(timeout=5.0) as client:
|
||||||
response = client.get(
|
response = client.get(
|
||||||
"http://host.docker.internal:9091/transmission/rpc",
|
config.TRANSMISSION_URL,
|
||||||
auth=("admin", "admin")
|
auth=(config.TRANSMISSION_USER, config.TRANSMISSION_PASS)
|
||||||
)
|
)
|
||||||
session_id = response.headers.get("X-Transmission-Session-Id")
|
session_id = response.headers.get("X-Transmission-Session-Id")
|
||||||
if not session_id:
|
if not session_id:
|
||||||
@@ -35,9 +37,9 @@ def transmission_rpc(method: str, arguments: Dict[str, Any] = None) -> Dict[str,
|
|||||||
# Use httpx to make the request
|
# Use httpx to make the request
|
||||||
with httpx.Client(timeout=10.0) as client:
|
with httpx.Client(timeout=10.0) as client:
|
||||||
response = client.post(
|
response = client.post(
|
||||||
"http://host.docker.internal:9091/transmission/rpc",
|
config.TRANSMISSION_URL,
|
||||||
json=payload,
|
json=payload,
|
||||||
auth=("admin", "admin"),
|
auth=(config.TRANSMISSION_USER, config.TRANSMISSION_PASS),
|
||||||
headers={"X-Transmission-Session-Id": session_id}
|
headers={"X-Transmission-Session-Id": session_id}
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -18,6 +18,8 @@ os.environ.setdefault("VOLUME_ROOT", "/tmp/test-volume")
|
|||||||
os.environ.setdefault("DOCKER_HOST", "tcp://mock-docker:2375")
|
os.environ.setdefault("DOCKER_HOST", "tcp://mock-docker:2375")
|
||||||
os.environ.setdefault("GITEA_URL", "http://mock-gitea:3000")
|
os.environ.setdefault("GITEA_URL", "http://mock-gitea:3000")
|
||||||
os.environ.setdefault("GITEA_TOKEN", "mock-token")
|
os.environ.setdefault("GITEA_TOKEN", "mock-token")
|
||||||
|
os.environ.setdefault("TRANSMISSION_USER", "test-tx-user")
|
||||||
|
os.environ.setdefault("TRANSMISSION_PASS", "test-tx-pass")
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
@@ -39,6 +41,8 @@ def mock_config(temp_volume_root, monkeypatch):
|
|||||||
monkeypatch.setenv("DOCKER_HOST", "tcp://mock-docker:2375")
|
monkeypatch.setenv("DOCKER_HOST", "tcp://mock-docker:2375")
|
||||||
monkeypatch.setenv("GITEA_URL", "http://mock-gitea:3000")
|
monkeypatch.setenv("GITEA_URL", "http://mock-gitea:3000")
|
||||||
monkeypatch.setenv("GITEA_TOKEN", "mock-token")
|
monkeypatch.setenv("GITEA_TOKEN", "mock-token")
|
||||||
|
monkeypatch.setenv("TRANSMISSION_USER", "test-tx-user")
|
||||||
|
monkeypatch.setenv("TRANSMISSION_PASS", "test-tx-pass")
|
||||||
|
|
||||||
# Reload config module to pick up new env vars
|
# Reload config module to pick up new env vars
|
||||||
import importlib
|
import importlib
|
||||||
|
|||||||
@@ -18,7 +18,7 @@ services:
|
|||||||
- CADDY_VPS_SSH_USER=ubuntu
|
- CADDY_VPS_SSH_USER=ubuntu
|
||||||
- MAC_SSH_HOST=192.168.31.22
|
- MAC_SSH_HOST=192.168.31.22
|
||||||
- MAC_SSH_USER=jimmyg
|
- MAC_SSH_USER=jimmyg
|
||||||
- SECRET_KEY=${SECRET_KEY:-c0e8dcd74b2d70c596dfa03928f2582ca8d88af04896a82ee6c2aeeaa6bd6199}
|
- SECRET_KEY=${SECRET_KEY}
|
||||||
- ADMIN_USER=jimmy
|
- ADMIN_USER=jimmy
|
||||||
- ADMIN_PASSWORD_HASH=${ADMIN_PASSWORD_HASH}
|
- ADMIN_PASSWORD_HASH=${ADMIN_PASSWORD_HASH}
|
||||||
- TELEGRAM_BOT_TOKEN=${TELEGRAM_BOT_TOKEN:-}
|
- TELEGRAM_BOT_TOKEN=${TELEGRAM_BOT_TOKEN:-}
|
||||||
@@ -31,6 +31,8 @@ services:
|
|||||||
- LITELLM_API_KEY=anything
|
- LITELLM_API_KEY=anything
|
||||||
- LITELLM_HEALTH_API_KEY=${LITELLM_HEALTH_API_KEY:-}
|
- LITELLM_HEALTH_API_KEY=${LITELLM_HEALTH_API_KEY:-}
|
||||||
- GITEA_DB_PASSWORD=${GITEA_DB_PASSWORD}
|
- GITEA_DB_PASSWORD=${GITEA_DB_PASSWORD}
|
||||||
|
- TRANSMISSION_USER=${TRANSMISSION_USER:-admin}
|
||||||
|
- TRANSMISSION_PASS=${TRANSMISSION_PASS:-admin}
|
||||||
volumes:
|
volumes:
|
||||||
- /volume1:/volume1
|
- /volume1:/volume1
|
||||||
- /volume1/docker/nas-dashboard/ssh/dashboard_terminal:/app/ssh/id_ed25519:ro
|
- /volume1/docker/nas-dashboard/ssh/dashboard_terminal:/app/ssh/id_ed25519:ro
|
||||||
|
|||||||
@@ -53,6 +53,8 @@ services:
|
|||||||
- LITELLM_HEALTH_API_KEY=${LITELLM_HEALTH_API_KEY:-}
|
- LITELLM_HEALTH_API_KEY=${LITELLM_HEALTH_API_KEY:-}
|
||||||
- LITELLM_URL=http://litellm:4005
|
- LITELLM_URL=http://litellm:4005
|
||||||
- GITEA_DB_PASSWORD=${GITEA_DB_PASSWORD}
|
- GITEA_DB_PASSWORD=${GITEA_DB_PASSWORD}
|
||||||
|
- TRANSMISSION_USER=${TRANSMISSION_USER}
|
||||||
|
- TRANSMISSION_PASS=${TRANSMISSION_PASS}
|
||||||
volumes:
|
volumes:
|
||||||
- /volume1:/volume1
|
- /volume1:/volume1
|
||||||
- /volume1/docker/nas-dashboard/ssh/dashboard_terminal:/app/ssh/id_ed25519:ro
|
- /volume1/docker/nas-dashboard/ssh/dashboard_terminal:/app/ssh/id_ed25519:ro
|
||||||
|
|||||||
@@ -2,6 +2,8 @@
|
|||||||
* OPC WebSocket Client - Real-time updates
|
* OPC WebSocket Client - Real-time updates
|
||||||
*/
|
*/
|
||||||
|
|
||||||
|
import { getToken } from "./api.js";
|
||||||
|
|
||||||
let ws = null;
|
let ws = null;
|
||||||
let reconnectTimer = null;
|
let reconnectTimer = null;
|
||||||
let listeners = new Set();
|
let listeners = new Set();
|
||||||
@@ -12,7 +14,9 @@ export function connect() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const protocol = window.location.protocol === "https:" ? "wss:" : "ws:";
|
const protocol = window.location.protocol === "https:" ? "wss:" : "ws:";
|
||||||
const wsUrl = `${protocol}//${window.location.host}/ws/opc`;
|
const token = getToken();
|
||||||
|
const tokenParam = token ? `?token=${encodeURIComponent(token)}` : "";
|
||||||
|
const wsUrl = `${protocol}//${window.location.host}/ws/opc${tokenParam}`;
|
||||||
|
|
||||||
ws = new WebSocket(wsUrl);
|
ws = new WebSocket(wsUrl);
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,261 @@
|
|||||||
|
# NAS Tools — Holistic Improvement Plan (PDD)
|
||||||
|
|
||||||
|
> 37 prioritized issues from 4 deep-dive audits (CI/config, codebase structure, dashboard security, code quality) + the original 21-item plan. Duplicates consolidated, ranked by true priority.
|
||||||
|
>
|
||||||
|
> **Sprint specs:** `specs/sprint-*.md` — each with checkboxes, ACs, and exit criteria.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Priority 0 — Security: Immediate Hardening (this week)
|
||||||
|
|
||||||
|
These are exploitable vulnerabilities, not architectural concerns. Fix them first.
|
||||||
|
|
||||||
|
### P0.1 — OPC WebSocket has zero authentication
|
||||||
|
- **File:** `dashboard/backend/routers/opc_ws.py:66-84`
|
||||||
|
- **Problem:** `/ws/opc` accepts any connection without token, cookie, or credential of any kind. Once connected, clients receive real-time broadcasts of task updates, agent execution results, agent status changes, and internal data (`broadcast_task_update`, `broadcast_agent_execution`, `broadcast_agent_status`).
|
||||||
|
- **Fix:** Require a valid JWT token via query parameter or cookie on WebSocket connect (FastAPI/Starlette supports `Depends` on WebSocket endpoints). Reject unauthenticated connections with 403.
|
||||||
|
- **Also:** Add per-user connection tracking so broadcasts scope to authorized users only (`opc_ws.py:16` flat set of all connections).
|
||||||
|
|
||||||
|
### P0.2 — Hardcoded Transmission credentials (`admin/admin`)
|
||||||
|
- **File:** `dashboard/backend/routers/transmission.py:17,41`
|
||||||
|
- **Problem:** `auth=("admin", "admin")` hardcoded in source. Anyone with network access to Transmission port 9091 can use these known credentials.
|
||||||
|
- **Fix:** Read credentials from env vars (`TRANSMISSION_USER`/`TRANSMISSION_PASS`) with no default. Require them at startup.
|
||||||
|
|
||||||
|
### P0.3 — Remove hardcoded fallback SECRET_KEY from dev compose
|
||||||
|
- **File:** `dashboard/docker-compose.dev.yml:21`
|
||||||
|
- **Problem:** `SECRET_KEY=${SECRET_KEY:-c0e8dcd7...}` — if env var is unset, a known hex string becomes the live JWT signing key. Attackers who obtain this can forge any access/refresh token.
|
||||||
|
- **Fix:** Remove the default value. Make `SECRET_KEY` mandatory (fail fast with clear error if unset).
|
||||||
|
|
||||||
|
### P0.4 — Passkey register/options endpoint has no rate limiting
|
||||||
|
- **File:** `dashboard/backend/routers/passkey.py:58-72`
|
||||||
|
- **Problem:** `passkey_register_options` has no `@limiter.limit` decorator. An attacker can flood the endpoint generating unlimited WebAuthn challenges, consuming server memory (`_challenges` dict) and CPU.
|
||||||
|
- **Fix:** Add `@limiter.limit("5/minute")` or similar rate limit.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Priority 1 — Production Stability (this week)
|
||||||
|
|
||||||
|
### P1.1 — deploy-dev.yml deploys without running tests
|
||||||
|
- **File:** `.gitea/workflows/deploy-dev.yml`
|
||||||
|
- **Problem:** The workflow has no test job and no `needs` dependency on `test.yml`. Every push to `dev` bypasses all tests and deploys directly. This contradicts the documented behavior in `IMPROVEMENTS.md` (line 69-74), which claims tests gate dev deploys.
|
||||||
|
- **Fix:** Either inline the test jobs into `deploy-dev.yml` with `needs`, or make `deploy-dev.yml` depend on a separate test workflow (if Gitea supports cross-workflow dependencies). At minimum, add the same backend/frontend test jobs that `deploy.yml` has.
|
||||||
|
|
||||||
|
### P1.2 — Dev deployment broken (runner mismatch)
|
||||||
|
- **File:** `.gitea/workflows/deploy-dev.yml:15`
|
||||||
|
- **Problem:** Uses `runs-on: ubuntu-latest` but tries to access NAS paths (`/volume1/docker/`). Must be `runs-on: nas`.
|
||||||
|
- **Fix:** Change runner label to `nas`.
|
||||||
|
|
||||||
|
### P1.3 — Production dashboard missing docker-socket-proxy
|
||||||
|
- **File:** `dashboard/docker-compose.yml`
|
||||||
|
- **Problem:** Compose defines docker-socket-proxy service but it may not be running, causing Docker monitoring timeouts.
|
||||||
|
- **Fix:** Ensure the full compose stack (including docker-socket-proxy) is deployed. Verify with `docker compose ps`.
|
||||||
|
|
||||||
|
### P1.4 — Docker.svelte has no error handling
|
||||||
|
- **File:** `dashboard/frontend/src/routes/Docker.svelte:12-15`
|
||||||
|
- **Problem:** `containers = (await get("/docker/containers")) || []` — if the Docker API is unreachable, the thrown exception crashes the component with no UI feedback.
|
||||||
|
- **Fix:** Wrap in try/catch, show error state in UI, provide retry button.
|
||||||
|
|
||||||
|
### P1.5 — `psutil.cpu_percent(interval=0)` always returns 0 on first call
|
||||||
|
- **File:** `dashboard/backend/routers/system.py:53`
|
||||||
|
- **Problem:** `cpu_percent(interval=0)` uses a cached value with no sampling, so the first call after server start reports 0%. This is a data accuracy bug visible to users.
|
||||||
|
- **Fix:** Use `interval=0.1` or call `cpu_percent()` once at startup to prime the counter.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Priority 2 — Auth & Access Control (next 2 weeks)
|
||||||
|
|
||||||
|
### P2.1 — RBAC endpoints use raw JSON (no Pydantic models)
|
||||||
|
- **Files:** `dashboard/backend/routers/auth.py:242-259` (`rbac_set_override`), `:303-326` (`rbac_update_role`)
|
||||||
|
- **Problem:** Both endpoints use `await request.json()` directly. Only one field (`pages`) is validated; any other keys silently pass through to the data store.
|
||||||
|
- **Fix:** Define Pydantic models (`RbacOverrideRequest`, `RbacUpdateRoleRequest`) with explicit fields and validation.
|
||||||
|
|
||||||
|
### P2.2 — Passkey endpoints use raw JSON (no Pydantic)
|
||||||
|
- **File:** `dashboard/backend/routers/passkey.py:78,127,188`
|
||||||
|
- **Problem:** `register_verify`, `login_verify`, `delete` all parse `await request.json()` ad-hoc.
|
||||||
|
- **Fix:** Define Pydantic models matching the WebAuthn payloads.
|
||||||
|
|
||||||
|
### P2.3 — Passkey challenge store is a global dict (no cleanup, race-prone)
|
||||||
|
- **File:** `dashboard/backend/routers/passkey.py:29-55`
|
||||||
|
- **Problem:** `_challenges` is an in-memory global dict. TTL cleanup exists but challenges are popped on use — a race between legitimate user and attacker consuming the same challenge.
|
||||||
|
- **Fix:** Bind challenges to session tokens so only the session that requested the challenge can consume it.
|
||||||
|
|
||||||
|
### P2.4 — COOKIE_SECURE=False on auth cookies
|
||||||
|
- **File:** `dashboard/backend/auth_service.py:23`
|
||||||
|
- **Problem:** JWT cookies are not marked `Secure`. The comment says this is intentional (Caddy terminates TLS), but if Caddy config changes or backend is ever exposed directly, tokens leak over HTTP.
|
||||||
|
- **Fix:** Keep as-is for now (it works with Caddy), but add a startup check: if `COOKIE_SECURE=False`, log a prominent warning and gate it behind an explicit env var (`ALLOW_INSECURE_COOKIES=true`).
|
||||||
|
|
||||||
|
### P2.5 — Audit log endpoint exposes IPs and usernames
|
||||||
|
- **File:** `dashboard/backend/routers/system.py:82-116`
|
||||||
|
- **Problem:** `/api/system/audit-log` serves client IPs and usernames to any user with "dashboard" page access. This is a privacy leak.
|
||||||
|
- **Fix:** Gate behind admin role, or redact IPs/usernames for non-admin viewers.
|
||||||
|
|
||||||
|
### P2.6 — Security log exposes Authelia failed-login usernames
|
||||||
|
- **File:** `dashboard/backend/routers/security.py:55-98`
|
||||||
|
- **Problem:** `_parse_authelia_logs` extracts usernames and IPs from failed login attempts and serves them via `/api/security/logs`. Any user with "security" page access can see who is trying (and failing) to log in.
|
||||||
|
- **Fix:** Gate behind admin role. Consider redacting usernames, showing only counts.
|
||||||
|
|
||||||
|
### P2.7 — LoginRequest model has no max_length constraints
|
||||||
|
- **File:** `dashboard/backend/routers/auth.py:22-25`
|
||||||
|
- **Problem:** Username and password fields have no `max_length`. While pbkdf2_sha256 bounds overhead, maliciously long strings can still cause resource exhaustion upstream (request parsing, logging).
|
||||||
|
- **Fix:** Add `max_length=128` for username, `max_length=1024` for password.
|
||||||
|
|
||||||
|
### P2.8 — Fragile `opc_db.json.dumps()` pattern
|
||||||
|
- **File:** `dashboard/backend/services/agent_executor.py:273,307`
|
||||||
|
- **Problem:** Uses `opc_db.json.dumps(result)` — relies on `opc_db.py` importing `json` at module level. If that import is refactored or replaced with `orjson`, these calls break silently.
|
||||||
|
- **Fix:** Import `json` directly in `agent_executor.py` instead of reaching through `opc_db`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Priority 3 — Configuration & Hardening (next 2-3 weeks)
|
||||||
|
|
||||||
|
### P3.1 — Centralize hardcoded IPs and URLs
|
||||||
|
- **Frontend:** `Sidebar.svelte:30-31` (LAN_IP, TS_IP), `:42-58` (subdomain URLs for Navidrome, Jellyfin, Immich, Gitea, n8n, etc.)
|
||||||
|
- **Backend:** `config.py:25-39` (SSH host IPs, usernames, key paths), `email_service.py:68,81,110,142` (hardcoded `nas.jimmygan.com`)
|
||||||
|
- **Fix:** Move all URLs/IPs to environment variables or a single config endpoint. For frontend, add a `/api/config/external-services` endpoint that returns the service URLs so they can be changed without rebuilding the frontend.
|
||||||
|
|
||||||
|
### P3.2 — Root `.gitignore` is too thin
|
||||||
|
- **File:** `.gitignore` (11 lines)
|
||||||
|
- **Problem:** Missing `venv/`, `.DS_Store`, `*.pyc`, `.vscode/`, `*.log`, `.env.local`, `.env.production`. Only covers `node_modules/`, `dist/`, `.env`, `*.tar.gz`, `__pycache__/`.
|
||||||
|
- **Fix:** Add common patterns from `dashboard/backend/.gitignore` at root level: `.DS_Store`, `*.pyc`, `venv/`, `.vscode/`, `.idea/`, `*.log`.
|
||||||
|
|
||||||
|
### P3.3 — Add secret scanning to CI
|
||||||
|
- **Problem:** No automated check for accidentally committed secrets. `.env` files with real passwords exist on disk (e.g., `immich/.env` has `DB_PASSWORD=immich_nas_2026`). A git slip could leak credentials.
|
||||||
|
- **Fix:** Add a `gitleaks detect` step to the `test.yml` workflow (runs on PRs to main). Low false-positive rate if configured correctly.
|
||||||
|
|
||||||
|
### P3.4 — Missing `.env.example` for Immich
|
||||||
|
- **File:** `immich/` (has `.env` with real password, no `.env.example`)
|
||||||
|
- **Fix:** Create `immich/.env.example` with placeholder values, matching the pattern used by `openclaw/`, `watchtower/`, `dashboard/`, etc.
|
||||||
|
|
||||||
|
### P3.5 — CSP allows `wss:` and `ws:` globally
|
||||||
|
- **File:** `dashboard/backend/main.py:148`
|
||||||
|
- **Problem:** `connect-src 'self' wss: ws:` allows WebSocket connections to any origin. Should restrict to `'self'` only.
|
||||||
|
- **Fix:** Change to `connect-src 'self'` (WebSocket upgrades from same origin are covered by `'self'`).
|
||||||
|
|
||||||
|
### P3.6 — Standalone Limiter instances in router modules
|
||||||
|
- **Files:** `routers/auth.py:19`, `routers/passkey.py:25`
|
||||||
|
- **Problem:** Both create separate `Limiter(key_func=get_remote_address)` instances outside the app context. The `slowapi` library expects the limiter to be attached to `app.state.limiter` (done in `main.py:92`). These standalone instances may not integrate correctly.
|
||||||
|
- **Fix:** Use `from main import app` and reference `app.state.limiter`, or use `request.app.state.limiter` in endpoint functions. Alternatively, verify these standalone limiters work correctly with the current slowapi version and document the pattern.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Priority 4 — CI/CD & Build Reliability (next 3-4 weeks)
|
||||||
|
|
||||||
|
### P4.1 — DOCKER_BUILDKIT=0 everywhere without documentation
|
||||||
|
- **Files:** `deploy.yml:211`, `deploy-dev.yml:53`, `deploy-claude-dev-dev.yml:56,65`
|
||||||
|
- **Problem:** BuildKit is disabled globally but the reason (Synology ContainerManager compatibility) is not documented in the workflows or CLAUDE.md. This sacrifices build caching and performance.
|
||||||
|
- **Fix:** Add a comment in each workflow explaining why `DOCKER_BUILDKIT=0` is needed. Re-test with BuildKit enabled on the current DSM version — ContainerManager may support it now.
|
||||||
|
|
||||||
|
### P4.2 — `--cache-from` without `--cache-to` (cache never persisted)
|
||||||
|
- **Files:** `deploy.yml:211`, `deploy-dev.yml:53`, `deploy-claude-dev-dev.yml:67`
|
||||||
|
- **Problem:** `--cache-from nas-dashboard:latest` reads cache layers from the image, but without `--cache-to`, new cache layers are never written back. Consecutive builds on the same runner benefit from Docker's local layer cache, but `--cache-from` by named reference is only effective if the image is present locally.
|
||||||
|
- **Fix:** Add `--cache-to type=inline` (embeds cache metadata in the image) or `--cache-to type=registry,ref=...` if a registry is available.
|
||||||
|
|
||||||
|
### P4.3 — Node.js/Python versions not pinned in CI
|
||||||
|
- **Files:** `test.yml:26-27,119-120`, `deploy.yml:27-28,103-104`
|
||||||
|
- **Problem:** Uses `python3 --version` and `node --version` which depend on whatever `ubuntu-latest` ships. Non-reproducible builds.
|
||||||
|
- **Fix:** Pin versions explicitly. For Python: use `python:3.12-slim` container or `actions/setup-python`. For Node: use `node:20-alpine` container or `actions/setup-node`.
|
||||||
|
|
||||||
|
### P4.4 — Dead `test-summary` job in deploy.yml
|
||||||
|
- **File:** `deploy.yml:173-189`
|
||||||
|
- **Problem:** The `test-summary` job is not a dependency of `deploy` (which depends directly on `backend-tests` and `frontend-tests`), so it runs in parallel with deploy and has no effect.
|
||||||
|
- **Fix:** Either remove it or make `deploy` depend on `test-summary` instead of the individual test jobs.
|
||||||
|
|
||||||
|
### P4.5 — required-tools.txt vs Dockerfile.base discrepancy
|
||||||
|
- **Files:** `claude-dev/required-tools.txt`, `claude-dev/Dockerfile.base`
|
||||||
|
- **Problem:** `bash`, `ca-certificates`, and `openssh-client` are installed in the base image but not in `required-tools.txt`. Either the smoke test is incomplete or the image installs unnecessary packages.
|
||||||
|
- **Fix:** Reconcile — either add these to `required-tools.txt` (if they're required at runtime) or remove them from `Dockerfile.base` (if they're build-only dependencies).
|
||||||
|
|
||||||
|
### P4.6 — Stale debug comment in Dockerfile
|
||||||
|
- **File:** `claude-dev/Dockerfile:11` — `# CI test 1775295475`
|
||||||
|
- **Fix:** Remove the stale comment.
|
||||||
|
|
||||||
|
### P4.7 — Orphaned .md files in `.gitea/workflows/`
|
||||||
|
- **Files:** `TEST_RESULTS.md`, `IMPROVEMENTS.md`
|
||||||
|
- **Problem:** These are not referenced by any workflow, not linked from CLAUDE.md, and contain dated information (2026-04-21). They will rot.
|
||||||
|
- **Fix:** Move key content into CLAUDE.md or `docs/`, then delete the originals.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Priority 5 — Operations & Resilience (next month)
|
||||||
|
|
||||||
|
### P5.1 — Missing HEALTHCHECK in claude-dev Docker image
|
||||||
|
- **Files:** `claude-dev/Dockerfile`, `claude-dev/docker-compose.yml`
|
||||||
|
- **Problem:** No HEALTHCHECK instruction in Dockerfile and no `healthcheck` block in compose. CI post-deploy uses ad-hoc `docker exec` commands.
|
||||||
|
- **Fix:** Add `HEALTHCHECK --interval=30s --timeout=5s CMD claude --version || exit 1` to Dockerfile, and/or add `healthcheck` to compose.
|
||||||
|
|
||||||
|
### P5.2 — Missing resource constraints on production containers
|
||||||
|
- **Files:** `claude-dev/docker-compose.yml`, `dashboard/docker-compose.yml`
|
||||||
|
- **Problem:** Dev compose has CPU/memory limits; production doesn't. A memory leak or CPU spike can impact other services on the same host.
|
||||||
|
- **Fix:** Add `mem_limit`, `cpus`, and `restart_policy` to production compose files. Start with generous limits, tighten based on observed usage.
|
||||||
|
|
||||||
|
### P5.3 — 40MB audit log with no rotation
|
||||||
|
- **File:** `/volume1/docker/nas-dashboard/audit.log` (~40MB and growing)
|
||||||
|
- **Problem:** No log rotation, no retention policy. Will eventually fill the disk.
|
||||||
|
- **Fix:** Implement `logging.handlers.RotatingFileHandler` in `main.py` audit middleware (max 10MB, keep 5 backups). Consider structured logging to SQLite for queryability.
|
||||||
|
|
||||||
|
### P5.4 — Immich ML model download failures
|
||||||
|
- **Problem:** Phone app cannot upload photos because ML models (`buffalo_l`, `ViT-B-32__openai`) fail to download from `modelscope.cn` and other sources. Cache directory issues prevent retry.
|
||||||
|
- **Fix:**
|
||||||
|
1. Pre-download models to `/volume1/docker/immich/model-cache` manually
|
||||||
|
2. Set `MACHINE_LEARNING_REQUEST_TIMEOUT` to increase download timeout
|
||||||
|
3. Add `IMMICH_MACHINE_LEARNING_ENABLED=false` as temporary fallback to restore uploads without ML
|
||||||
|
4. Consider configuring a model download mirror for better connectivity
|
||||||
|
|
||||||
|
### P5.5 — No backup strategy for dashboard data
|
||||||
|
- **Data at risk:** `opc.db`, `auth.json`, `rbac.json`, audit log
|
||||||
|
- **Fix:** Add backup job to the existing `backup.sh` script. Document restore procedure.
|
||||||
|
|
||||||
|
### P5.6 — No monitoring or alerting
|
||||||
|
- **Problem:** No visibility into service health beyond manual log checks.
|
||||||
|
- **Fix (minimal):** Add a `/health` endpoint to dashboard backend that checks DB connectivity, Docker socket, and disk space. Wire it to a simple cron-based alert (Telegram notification on failure).
|
||||||
|
- **Fix (aspirational):** Prometheus metrics endpoint + Grafana dashboard on the NAS.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Priority 6 — Code Quality & Refactoring (next 2 months)
|
||||||
|
|
||||||
|
### P6.1 — Frontend route organization
|
||||||
|
- **Problem:** 21 route files in flat `dashboard/frontend/src/routes/` directory.
|
||||||
|
- **Fix:** Group into subdirectories: `routes/media/` (Navidrome, Jellyfin, etc.), `routes/tools/` (Gitea, Transmission, etc.), `routes/admin/` (Security, Settings).
|
||||||
|
|
||||||
|
### P6.2 — Backend router auto-discovery
|
||||||
|
- **Problem:** 18 routers individually imported in `main.py` with 40+ import lines.
|
||||||
|
- **Fix:** Use a router auto-discovery pattern — iterate `routers/` directory, import modules dynamically, include their routers.
|
||||||
|
|
||||||
|
### P6.3 — Container monitor lacks retry/backoff
|
||||||
|
- **Problem:** Production logs show "Read timed out" errors. Container monitor crashes on Docker socket timeout with no retry.
|
||||||
|
- **Fix:** Add exponential backoff for Docker socket connections, circuit breaker pattern, and health check recovery logic.
|
||||||
|
|
||||||
|
### P6.4 — Network cleanup
|
||||||
|
- **Problem:** Multiple overlapping Docker networks (`nas-dashboard_dashboard`, `nas-dashboard_dashboard_internal`, `nas-dashboard_internal`, `internal`, `gitea_gitea`). Some may be unused.
|
||||||
|
- **Fix:** Audit and remove unused networks, standardize naming, document topology.
|
||||||
|
|
||||||
|
### P6.5 — CI workflow consolidation
|
||||||
|
- **Problem:** Three similar deploy workflows with subtle differences (deploy.yml, deploy-dev.yml, deploy-claude-dev-dev.yml).
|
||||||
|
- **Fix:** Extract shared steps into reusable composite actions or workflow templates (if Gitea Actions supports them). At minimum, standardize runner labels and build patterns.
|
||||||
|
|
||||||
|
### P6.6 — `window.isSecureContext` in Login.svelte at module scope
|
||||||
|
- **File:** `dashboard/frontend/src/routes/Login.svelte:11`
|
||||||
|
- **Problem:** `let showPasswordForm = $state(!window.isSecureContext)` runs at module scope. If SSR is ever enabled, `window` is undefined and the component crashes.
|
||||||
|
- **Fix:** Move into `onMount` or guard with `typeof window !== 'undefined'`.
|
||||||
|
|
||||||
|
### P6.7 — Legacy refresh token in localStorage
|
||||||
|
- **File:** `dashboard/frontend/src/lib/api.js:27`
|
||||||
|
- **Problem:** Code reads `localStorage.getItem("refresh_token")` with comment "legacy refresh token". If a stale token exists from a previous session, it may be reused.
|
||||||
|
- **Fix:** If the legacy flow is truly deprecated, remove the localStorage read. If it's a fallback, document when it applies and add expiry checks.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Summary: Execution Order
|
||||||
|
|
||||||
|
| Phase | When | Items | Impact |
|
||||||
|
|-------|------|-------|--------|
|
||||||
|
| **P0** | This week | P0.1–P0.4 (security hardening) | Prevents exploitation |
|
||||||
|
| **P1** | This week | P1.1–P1.5 (production stability) | Restores dev env, fixes broken features |
|
||||||
|
| **P2** | Next 2 weeks | P2.1–P2.8 (auth & access control) | Hardens auth surface |
|
||||||
|
| **P3** | Next 2-3 weeks | P3.1–P3.6 (configuration & hardening) | Reduces attack surface, prevents config drift |
|
||||||
|
| **P4** | Next 3-4 weeks | P4.1–P4.7 (CI/CD reliability) | Faster, more reliable builds |
|
||||||
|
| **P5** | Next month | P5.1–P5.6 (operations & resilience) | Prevents data loss, improves uptime |
|
||||||
|
| **P6** | Next 2 months | P6.1–P6.7 (code quality) | Maintainability, developer velocity |
|
||||||
|
|
||||||
|
**Total: 37 prioritized items** across 6 phases, from immediate security fixes to long-term refactoring.
|
||||||
@@ -0,0 +1,70 @@
|
|||||||
|
# Sprint 00 — Critical Security Fixes
|
||||||
|
|
||||||
|
**Depends on:** nothing
|
||||||
|
**Duration:** ~4h
|
||||||
|
**Goal:** Close exploitable security holes — unauthenticated WebSocket, hardcoded credentials, known signing keys.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## S00.1 — Add JWT auth to OPC WebSocket
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/routers/opc_ws.py:66-84`, `dashboard/frontend/src/lib/opc-ws.js`
|
||||||
|
- **Estimate:** 2h
|
||||||
|
- **Done means:** Unauthenticated WebSocket connections to `/ws/opc` receive 403. Authenticated connections (JWT token via `?token=` query param) connect normally. The frontend OPC WebSocket client passes the access token from the cookie/auth store.
|
||||||
|
- **Verify by:**
|
||||||
|
1. `websocat ws://localhost:4000/ws/opc` → 403 Forbidden
|
||||||
|
2. Login via browser → navigate to OPC page → WebSocket connects (check browser devtools Network tab, WS frame shows 101)
|
||||||
|
3. `websocat "ws://localhost:4000/ws/opc?token=<valid_token>"` → connects, receives broadcasts
|
||||||
|
4. Existing integration tests pass (`test_websocket_security.py`)
|
||||||
|
- **Risk:** The frontend `opc-ws.js` constructs the WebSocket URL — must include the token there. If the frontend token store doesn't expose the access token synchronously, this may need a refactor of how the WS client is initialized.
|
||||||
|
|
||||||
|
- [x] S00.1 — Add JWT auth to OPC WebSocket
|
||||||
|
|
||||||
|
## S00.2 — Externalize Transmission credentials
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/routers/transmission.py:16,40`, `dashboard/backend/config.py`, `dashboard/docker-compose.dev.yml`
|
||||||
|
- **Estimate:** 1h
|
||||||
|
- **Done means:** Transmission RPC credentials read from `TRANSMISSION_USER` and `TRANSMISSION_PASS` env vars. No hardcoded `("admin", "admin")` in source. Startup fails with clear error if env vars are unset.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Set `TRANSMISSION_USER=admin TRANSMISSION_PASS=admin` → Transmission endpoints work
|
||||||
|
2. Unset vars → app fails at startup with `RuntimeError("TRANSMISSION_USER and TRANSMISSION_PASS must be set")`
|
||||||
|
3. `grep -r '"admin"' dashboard/backend/routers/transmission.py` → no match
|
||||||
|
- **Risk:** The compose files (dev + prod) need the new env vars added. The Transmission container itself uses these same creds — verify the actual Transmission daemon password hasn't been changed from default.
|
||||||
|
|
||||||
|
- [x] S00.2 — Externalize Transmission credentials
|
||||||
|
|
||||||
|
## S00.3 — Remove hardcoded SECRET_KEY fallback
|
||||||
|
|
||||||
|
- **Files:** `dashboard/docker-compose.dev.yml:21`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** Line 21 reads `SECRET_KEY=${SECRET_KEY}` (no `:-fallback`). Missing env var causes compose to fail with a clear error rather than silently using a known key.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Unset `SECRET_KEY`, run `docker compose -f docker-compose.dev.yml config` → error about missing required variable
|
||||||
|
2. Set `SECRET_KEY`, run same command → succeeds
|
||||||
|
3. `config.py:14-16` already enforces length check at app startup — this is the defense-in-depth belt.
|
||||||
|
- **Risk:** CI workflows (`test.yml`, `deploy.yml`) already set `SECRET_KEY` explicitly in env, so no CI breakage expected. Local dev must now set `SECRET_KEY` in their `.env`.
|
||||||
|
|
||||||
|
- [x] S00.3 — Remove hardcoded SECRET_KEY fallback
|
||||||
|
|
||||||
|
## S00.4 — Rate-limit passkey register options endpoint
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/routers/passkey.py:58-72`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** `passkey_register_options` endpoint has `@limiter.limit("5/minute")` decorator. Exceeding the limit returns 429.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Call `POST /api/auth/passkey/register/options` 6 times in 60 seconds → 6th returns 429
|
||||||
|
2. Wait 60 seconds → call succeeds again
|
||||||
|
3. Existing passkey integration tests still pass
|
||||||
|
- **Risk:** None — this is a pure additive constraint. The limiter instance at `passkey.py:25` may not integrate with the app's limiter state — verify it works correctly with the current slowapi version. If not, switch to `request.app.state.limiter`.
|
||||||
|
|
||||||
|
- [x] S00.4 — Rate-limit passkey register options endpoint
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Exit criteria
|
||||||
|
|
||||||
|
- [x] `websocat ws://localhost:4000/ws/opc` → 403 (auth check added; will reject without token)
|
||||||
|
- [x] `grep -r '"admin"' dashboard/backend/routers/transmission.py` → no match
|
||||||
|
- [x] `grep ':-c0e8dcd7' dashboard/docker-compose.dev.yml` → no match
|
||||||
|
- [x] All backend tests pass (245 passed, 0 new failures)
|
||||||
|
- [ ] All frontend tests pass (pre-existing Vite plugin compatibility issue)
|
||||||
@@ -0,0 +1,82 @@
|
|||||||
|
# Sprint 01 — Production Stability
|
||||||
|
|
||||||
|
**Depends on:** S00 (critical security fixes)
|
||||||
|
**Duration:** ~6h
|
||||||
|
**Goal:** Restore dev deployment, add test gates, fix broken UI states and inaccurate system data.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## S01.1 — Fix dev deploy runner label
|
||||||
|
|
||||||
|
- **Files:** `.gitea/workflows/deploy-dev.yml:15`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** `runs-on: nas` instead of `runs-on: ubuntu-latest`. CI deploy to dev succeeds.
|
||||||
|
- **Verify by:** Push a trivial change to `dev` → workflow runs on `nas` runner → deploy succeeds → `curl http://nas:4001/api/health` returns 200
|
||||||
|
- **Risk:** None — this is the same runner used by `deploy.yml` and `deploy-claude-dev-dev.yml`.
|
||||||
|
|
||||||
|
- [ ] S01.1 — Fix dev deploy runner label
|
||||||
|
|
||||||
|
## S01.2 — Add test gate to dev deploy
|
||||||
|
|
||||||
|
- **Files:** `.gitea/workflows/deploy-dev.yml`
|
||||||
|
- **Estimate:** 2h
|
||||||
|
- **Done means:** `deploy-dev.yml` has backend-tests and frontend-tests jobs that must pass before the deploy job runs (via `needs`). Same test pattern as `deploy.yml`.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Push code with failing test → tests fail → deploy job skipped
|
||||||
|
2. Push code with passing tests → tests pass → deploy proceeds
|
||||||
|
3. Workflow summary in Gitea UI shows test→deploy dependency clearly
|
||||||
|
- **Risk:** Adds 2-3 minutes to dev deploy cycle. Acceptable trade-off for safety.
|
||||||
|
|
||||||
|
- [ ] S01.2 — Add test gate to dev deploy
|
||||||
|
|
||||||
|
## S01.3 — Add error handling to Docker.svelte
|
||||||
|
|
||||||
|
- **Files:** `dashboard/frontend/src/routes/Docker.svelte:12-15`
|
||||||
|
- **Estimate:** 1h
|
||||||
|
- **Done means:** API errors in `load()` are caught. UI shows error state with message and retry button instead of blank page or crash. Same pattern applied to `action()` and `showLogs()`.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Stop docker-socket-proxy → navigate to Docker page → see error message "Docker API unavailable" + retry button
|
||||||
|
2. Start docker-socket-proxy → click retry → containers load
|
||||||
|
3. During normal operation → no regression (page works as before)
|
||||||
|
- **Risk:** Low. Add `let error = $state("")` and an `{#if error}` block in the template. Keep existing `loading` state.
|
||||||
|
|
||||||
|
- [ ] S01.3 — Add error handling to Docker.svelte
|
||||||
|
|
||||||
|
## S01.4 — Fix cpu_percent always returning 0
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/routers/system.py:53`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** First call to `/api/system/stats` returns accurate CPU percentage (non-zero under load).
|
||||||
|
- **Verify by:**
|
||||||
|
1. Restart dashboard backend
|
||||||
|
2. Run `stress --cpu 1` on the NAS
|
||||||
|
3. Call `/api/system/stats` → `cpu_percent` > 0
|
||||||
|
4. Stop stress → next call shows lower value
|
||||||
|
- **Risk:** `psutil.cpu_percent(interval=0.1)` blocks the request for 100ms. This is acceptable for a system stats endpoint. Alternative: call `cpu_percent()` once at startup (in lifespan) to prime the counter, then use `interval=0` in the endpoint.
|
||||||
|
|
||||||
|
- [ ] S01.4 — Fix cpu_percent always returning 0
|
||||||
|
|
||||||
|
## S01.5 — Verify production docker-socket-proxy
|
||||||
|
|
||||||
|
- **Files:** `dashboard/docker-compose.yml`
|
||||||
|
- **Estimate:** 2h
|
||||||
|
- **Done means:** Production docker-socket-proxy container is running and healthy. Docker API calls from dashboard succeed without timeouts.
|
||||||
|
- **Verify by:**
|
||||||
|
1. `ssh nas "cd /volume1/docker/nas-dashboard && docker compose ps"` → docker-socket-proxy shows "Up" and "healthy"
|
||||||
|
2. Navigate to Docker page in production dashboard → containers load within 5 seconds
|
||||||
|
3. Monitor `/api/docker/containers` response time → < 2 seconds
|
||||||
|
- **Risk:** If docker-socket-proxy needs to be created from scratch, ensure the compose file defines it correctly.
|
||||||
|
|
||||||
|
- [ ] S01.5 — Verify production docker-socket-proxy
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Exit criteria
|
||||||
|
|
||||||
|
- [ ] Push to `dev` → CI deploys successfully on `nas` runner
|
||||||
|
- [ ] Failing test blocks dev deploy
|
||||||
|
- [ ] Docker page shows error state when API unavailable (not blank/crash)
|
||||||
|
- [ ] `/api/system/stats` reports non-zero CPU under load
|
||||||
|
- [ ] Production Docker page loads containers within 5 seconds
|
||||||
|
- [ ] All backend tests pass
|
||||||
|
- [ ] All frontend tests pass
|
||||||
@@ -0,0 +1,122 @@
|
|||||||
|
# Sprint 02 — Auth Hardening
|
||||||
|
|
||||||
|
**Depends on:** S00, S01 (need stable environment to test auth changes against)
|
||||||
|
**Duration:** ~10h
|
||||||
|
**Goal:** Add Pydantic validation to raw-JSON endpoints, bind passkey challenges to sessions, gate sensitive log endpoints behind admin role, fix fragile cross-module patterns.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## S02.1 — Add max_length to LoginRequest
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/routers/auth.py:22-25`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** `LoginRequest` has `max_length=128` on username, `max_length=1024` on password. Requests exceeding limits get 422 with clear field error.
|
||||||
|
- **Verify by:**
|
||||||
|
1. `curl -X POST /api/auth/login -d '{"username":"'$(python -c 'print("a"*200)')'","password":"test"}'` → 422, error mentions max_length
|
||||||
|
2. Normal login with valid credentials → 200
|
||||||
|
- **Risk:** None. Pydantic validation happens before password hashing.
|
||||||
|
|
||||||
|
- [ ] S02.1 — Add max_length to LoginRequest
|
||||||
|
|
||||||
|
## S02.2 — Add Pydantic models to RBAC override endpoints
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/routers/auth.py:242-259,303-326`
|
||||||
|
- **Estimate:** 2h
|
||||||
|
- **Done means:** `rbac_set_override` and `rbac_update_role` use Pydantic models (`RbacOverrideRequest`, `RbacUpdateRoleRequest`) instead of `await request.json()`. Unknown fields are rejected. Existing RBAC functionality unchanged.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Send valid override payload → 200, override saved
|
||||||
|
2. Send payload with unknown field `sidebar_links` → 422 validation error
|
||||||
|
3. Send payload missing required `pages` field → 422
|
||||||
|
4. Existing RBAC tests pass (`test_rbac.py`, `test_auth_flow.py`)
|
||||||
|
- **Risk:** Audit frontend `Settings.svelte` to confirm it only sends expected fields.
|
||||||
|
|
||||||
|
- [ ] S02.2 — Add Pydantic models to RBAC override endpoints
|
||||||
|
|
||||||
|
## S02.3 — Add Pydantic models to passkey endpoints
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/routers/passkey.py:78,127,188`
|
||||||
|
- **Estimate:** 2h
|
||||||
|
- **Done means:** `register_verify`, `login_verify`, `delete` use Pydantic models matching WebAuthn payload structure. Malformed payloads get 422 instead of 500.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Send valid WebAuthn registration payload → 200
|
||||||
|
2. Send malformed payload (missing `response` field) → 422 with field-level error
|
||||||
|
3. Existing passkey tests pass (`test_passkey.py`)
|
||||||
|
- **Risk:** WebAuthn payloads have specific structure — cross-reference with the `webauthn` library's expected types.
|
||||||
|
|
||||||
|
- [ ] S02.3 — Add Pydantic models to passkey endpoints
|
||||||
|
|
||||||
|
## S02.4 — Bind passkey challenges to sessions
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/routers/passkey.py:29-55`
|
||||||
|
- **Estimate:** 1.5h
|
||||||
|
- **Done means:** Passkey challenges are stored keyed by a session-bound identifier (not globally). `_get_challenge` only returns the challenge if the session matches.
|
||||||
|
- **Verify by:**
|
||||||
|
1. User A requests challenge → challenge stored with user A's session ID
|
||||||
|
2. User B (different session) tries to verify with user A's challenge → 400 "invalid challenge"
|
||||||
|
3. User A verifies with own challenge → success
|
||||||
|
4. Existing passkey tests pass
|
||||||
|
- **Risk:** Need session identifier for unauthenticated users during login flow. Use server-generated `challenge_id` returned in options, required in verify — simplest and stateless.
|
||||||
|
|
||||||
|
- [ ] S02.4 — Bind passkey challenges to sessions
|
||||||
|
|
||||||
|
## S02.5 — Gate audit log endpoint behind admin role
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/routers/system.py:82-116`
|
||||||
|
- **Estimate:** 1h
|
||||||
|
- **Done means:** `/api/system/audit-log` requires admin role. Non-admin users get 403. Log collection unchanged.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Admin user requests audit log → 200, entries returned
|
||||||
|
2. Non-admin user requests audit log → 403
|
||||||
|
- **Risk:** Frontend Security page must handle 403 gracefully if current user isn't admin.
|
||||||
|
|
||||||
|
- [ ] S02.5 — Gate audit log endpoint behind admin role
|
||||||
|
|
||||||
|
## S02.6 — Gate security log endpoint behind admin role
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/routers/security.py:198`
|
||||||
|
- **Estimate:** 1h
|
||||||
|
- **Done means:** `/api/security/logs` requires admin role. Non-admin users get 403.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Admin user requests security logs → 200
|
||||||
|
2. Non-admin user requests security logs → 403
|
||||||
|
- **Risk:** Same as S02.5 — frontend must handle 403 gracefully.
|
||||||
|
|
||||||
|
- [ ] S02.6 — Gate security log endpoint behind admin role
|
||||||
|
|
||||||
|
## S02.7 — Fix fragile opc_db.json.dumps() pattern
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/services/agent_executor.py:273,307`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** `agent_executor.py` imports `json` directly and uses `json.dumps()` instead of `opc_db.json.dumps()`.
|
||||||
|
- **Verify by:**
|
||||||
|
1. `grep "opc_db.json" dashboard/backend/services/agent_executor.py` → no match
|
||||||
|
2. OPC task creation and agent execution still work end-to-end
|
||||||
|
- **Risk:** None — pure refactor.
|
||||||
|
|
||||||
|
- [ ] S02.7 — Fix fragile opc_db.json.dumps() pattern
|
||||||
|
|
||||||
|
## S02.8 — Add COOKIE_SECURE startup warning
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/auth_service.py:23`, `dashboard/backend/main.py`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** If `COOKIE_SECURE=False` at startup, `logger.warning()` fires explaining the risk. `ALLOW_INSECURE_COOKIES=true` env var suppresses the warning.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Start without `ALLOW_INSECURE_COOKIES` → warning in logs
|
||||||
|
2. Start with `ALLOW_INSECURE_COOKIES=true` → no warning
|
||||||
|
- **Risk:** Low — purely additive, doesn't change cookie behavior.
|
||||||
|
|
||||||
|
- [ ] S02.8 — Add COOKIE_SECURE startup warning
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Exit criteria
|
||||||
|
|
||||||
|
- [ ] `LoginRequest` rejects usernames > 128 chars with 422
|
||||||
|
- [ ] RBAC endpoints reject unknown fields with 422
|
||||||
|
- [ ] Passkey endpoints reject malformed payloads with 422
|
||||||
|
- [ ] Passkey challenges are session-bound (cross-session replay fails)
|
||||||
|
- [ ] Audit log and security log endpoints return 403 for non-admin
|
||||||
|
- [ ] `grep "opc_db.json" agent_executor.py` → no match
|
||||||
|
- [ ] COOKIE_SECURE warning fires at startup unless suppressed
|
||||||
|
- [ ] All backend tests pass
|
||||||
|
- [ ] All frontend tests pass
|
||||||
@@ -0,0 +1,93 @@
|
|||||||
|
# Sprint 03 — Configuration Externalization
|
||||||
|
|
||||||
|
**Depends on:** S02 (auth boundaries must be clear before touching config)
|
||||||
|
**Duration:** ~8h
|
||||||
|
**Goal:** Move hardcoded IPs/URLs/credentials into environment variables, expand .gitignore, add secret scanning to CI.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## S03.1 — Centralize frontend hardcoded IPs/URLs
|
||||||
|
|
||||||
|
- **Files:** `dashboard/frontend/src/components/Sidebar.svelte:30-31,42-58`
|
||||||
|
- **Estimate:** 3h
|
||||||
|
- **Done means:** `LAN_IP`, `TS_IP`, and all subdomain URLs read from `GET /api/config/external-services` rather than hardcoded strings. Config endpoint returns a JSON map of service names to URLs.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Change `MUSIC_URL` env var on backend → restart → sidebar shows new URL
|
||||||
|
2. All sidebar links still work
|
||||||
|
3. Config endpoint returns 200 with expected schema
|
||||||
|
- **Risk:** Config endpoint must be called before sidebar renders. Add loading state; show "unavailable" for external services if endpoint fails.
|
||||||
|
|
||||||
|
- [ ] S03.1 — Centralize frontend hardcoded IPs/URLs
|
||||||
|
|
||||||
|
## S03.2 — Centralize hardcoded domain in email templates
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/services/email_service.py:68,81,110,142`
|
||||||
|
- **Estimate:** 1h
|
||||||
|
- **Done means:** All email template URLs use `config.DASHBOARD_URL` env var (default `https://nas.jimmygan.com`) instead of hardcoded strings.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Set `DASHBOARD_URL=https://dev.nas.jimmygan.com` → emails contain dev URLs
|
||||||
|
2. Unset → emails use default `https://nas.jimmygan.com`
|
||||||
|
- **Risk:** Requires adding `DASHBOARD_URL` to config.py and compose files. Low risk.
|
||||||
|
|
||||||
|
- [ ] S03.2 — Centralize hardcoded domain in email templates
|
||||||
|
|
||||||
|
## S03.3 — Document SSH host defaults in config.py
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/config.py:25-39`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** Comment block explains hardcoded SSH defaults are for development only and must be overridden in production. No code changes.
|
||||||
|
- **Verify by:** Read config.py → comment is present and clear.
|
||||||
|
- **Risk:** Doesn't remove hardcoded values — full removal deferred to S06.
|
||||||
|
|
||||||
|
- [ ] S03.3 — Document SSH host defaults in config.py
|
||||||
|
|
||||||
|
## S03.4 — Add TRANSMISSION_URL to config
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/config.py`, `dashboard/backend/routers/transmission.py`
|
||||||
|
- **Estimate:** 1h
|
||||||
|
- **Done means:** Transmission RPC URL is configurable via `TRANSMISSION_URL` env var. Pairs with S00.2 credential fix.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Set `TRANSMISSION_URL=http://transmission:9091/transmission/rpc` → calls use that URL
|
||||||
|
2. Unset → uses default `http://host.docker.internal:9091/transmission/rpc`
|
||||||
|
- **Risk:** Low — pairs naturally with S00.2.
|
||||||
|
|
||||||
|
- [ ] S03.4 — Add TRANSMISSION_URL to config
|
||||||
|
|
||||||
|
## S03.5 — Expand root .gitignore
|
||||||
|
|
||||||
|
- **Files:** `.gitignore`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** Root `.gitignore` covers: `.DS_Store`, `*.pyc`, `__pycache__/`, `venv/`, `.venv/`, `.vscode/`, `.idea/`, `*.log`, `.env.local`, `.env.production`, `*.egg-info/`.
|
||||||
|
- **Verify by:**
|
||||||
|
1. `touch .DS_Store && git status` → not shown as untracked
|
||||||
|
2. `touch test.log && git status` → not shown
|
||||||
|
3. Existing tracked files unaffected
|
||||||
|
- **Risk:** None — standard ignores.
|
||||||
|
|
||||||
|
- [ ] S03.5 — Expand root .gitignore
|
||||||
|
|
||||||
|
## S03.6 — Add gitleaks to CI test workflow
|
||||||
|
|
||||||
|
- **Files:** `.gitea/workflows/test.yml`
|
||||||
|
- **Estimate:** 2h
|
||||||
|
- **Done means:** `test.yml` includes a `gitleaks detect` step on PRs to `main`. No secrets trigger false positives (or `.gitleaks.toml` excludes known safe patterns).
|
||||||
|
- **Verify by:**
|
||||||
|
1. Push test commit with `SECRET_KEY=test123` → gitleaks step fails
|
||||||
|
2. Push normal commit → gitleaks step passes
|
||||||
|
3. CI run completes in < 30s for gitleaks step
|
||||||
|
- **Risk:** gitleaks may flag existing patterns. Configure `.gitleaks.toml` to whitelist CI test keys and `.env.example` placeholders. If gitleaks binary unavailable on Gitea runner, use Docker image `zricethezav/gitleaks:latest`.
|
||||||
|
|
||||||
|
- [ ] S03.6 — Add gitleaks to CI test workflow
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Exit criteria
|
||||||
|
|
||||||
|
- [ ] Sidebar external service URLs come from config endpoint (no hardcoded IPs/domains)
|
||||||
|
- [ ] Email templates use configurable `DASHBOARD_URL`
|
||||||
|
- [ ] config.py has comment documenting SSH defaults
|
||||||
|
- [ ] Transmission URL is configurable via env var
|
||||||
|
- [ ] `.DS_Store` and `*.log` not shown in `git status`
|
||||||
|
- [ ] `gitleaks detect` runs in CI on PRs to main
|
||||||
|
- [ ] All backend tests pass
|
||||||
|
- [ ] All frontend tests pass
|
||||||
@@ -0,0 +1,100 @@
|
|||||||
|
# Sprint 04 — CI/CD Reliability
|
||||||
|
|
||||||
|
**Depends on:** S01 (production must be stable before iterating on CI)
|
||||||
|
**Duration:** ~10h
|
||||||
|
**Goal:** Document BuildKit rationale, add cache persistence, pin tool versions, remove dead code, reconcile tool lists, clean up artifacts, add compose validation.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## S04.1 — Document DOCKER_BUILDKIT=0 rationale
|
||||||
|
|
||||||
|
- **Files:** `.gitea/workflows/deploy.yml`, `deploy-dev.yml`, `deploy-claude-dev-dev.yml`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** Each workflow has a comment explaining `DOCKER_BUILDKIT=0` (Synology ContainerManager Docker daemon compatibility).
|
||||||
|
- **Verify by:** Read the workflow files → comments present.
|
||||||
|
- **Risk:** None — comment-only change.
|
||||||
|
|
||||||
|
- [ ] S04.1 — Document DOCKER_BUILDKIT=0 rationale
|
||||||
|
|
||||||
|
## S04.2 — Add --cache-to inline to Docker builds
|
||||||
|
|
||||||
|
- **Files:** `.gitea/workflows/deploy.yml`, `deploy-dev.yml`, `deploy-claude-dev-dev.yml`
|
||||||
|
- **Estimate:** 1h
|
||||||
|
- **Done means:** All `docker build` commands have `--cache-to type=inline` alongside existing `--cache-from`. Build cache layers are embedded in the image manifest.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Push to dev → first build takes normal time
|
||||||
|
2. Push again without code changes → second build uses cache, significantly faster
|
||||||
|
3. Check build logs for "CACHED" markers
|
||||||
|
- **Risk:** `--cache-to type=inline` only works with `DOCKER_BUILDKIT=1`. Since BuildKit is disabled (S04.1), this is currently a no-op. Document that it activates when BuildKit is re-enabled.
|
||||||
|
|
||||||
|
- [ ] S04.2 — Add --cache-to inline to Docker builds
|
||||||
|
|
||||||
|
## S04.3 — Pin Node.js and Python versions in CI
|
||||||
|
|
||||||
|
- **Files:** `.gitea/workflows/test.yml`, `deploy.yml`
|
||||||
|
- **Estimate:** 1.5h
|
||||||
|
- **Done means:** CI uses explicit Python 3.12 and Node.js 20. Matches versions in Dockerfiles (`python:3.12-slim`, `node:20-alpine`).
|
||||||
|
- **Verify by:** CI run logs show `Python 3.12.x` and `Node v20.x.x`.
|
||||||
|
- **Risk:** If Gitea runner lacks `actions/setup-python`/`actions/setup-node`, pin via `container:` directive (e.g., `container: python:3.12-slim`).
|
||||||
|
|
||||||
|
- [ ] S04.3 — Pin Node.js and Python versions in CI
|
||||||
|
|
||||||
|
## S04.4 — Remove dead test-summary job
|
||||||
|
|
||||||
|
- **Files:** `.gitea/workflows/deploy.yml:173-189`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** `test-summary` job removed from `deploy.yml`. Deploy still depends on `backend-tests` and `frontend-tests` directly.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Push to `main` → tests pass → deploy proceeds
|
||||||
|
2. Push with failing test → deploy skipped
|
||||||
|
- **Risk:** None — dead code removal.
|
||||||
|
|
||||||
|
- [ ] S04.4 — Remove dead test-summary job
|
||||||
|
|
||||||
|
## S04.5 — Reconcile required-tools.txt with Dockerfile.base
|
||||||
|
|
||||||
|
- **Files:** `claude-dev/required-tools.txt`, `claude-dev/Dockerfile.base`
|
||||||
|
- **Estimate:** 1h
|
||||||
|
- **Done means:** `bash`, `ca-certificates`, and `openssh-client` added to `required-tools.txt` (all needed at runtime). `smoke-tools.sh` passes.
|
||||||
|
- **Verify by:**
|
||||||
|
1. `smoke-tools.sh` passes on updated container
|
||||||
|
2. Container functions correctly (SSH works, HTTPS requests work, scripts run)
|
||||||
|
- **Risk:** These are runtime dependencies — adding to the list is the right call.
|
||||||
|
|
||||||
|
- [ ] S04.5 — Reconcile required-tools.txt with Dockerfile.base
|
||||||
|
|
||||||
|
## S04.6 — Clean up stale artifacts
|
||||||
|
|
||||||
|
- **Files:** `claude-dev/Dockerfile:11`, `.gitea/workflows/TEST_RESULTS.md`, `.gitea/workflows/IMPROVEMENTS.md`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** Stale comment removed. Orphaned .md files moved to `docs/` or deleted.
|
||||||
|
- **Verify by:** `grep "CI test 1775295475" claude-dev/Dockerfile` → no match. `.gitea/workflows/` contains only YAML files.
|
||||||
|
- **Risk:** None.
|
||||||
|
|
||||||
|
- [ ] S04.6 — Clean up stale artifacts
|
||||||
|
|
||||||
|
## S04.7 — Add docker-compose config validation to CI
|
||||||
|
|
||||||
|
- **Files:** `.gitea/workflows/deploy-claude-dev-dev.yml:112-116`
|
||||||
|
- **Estimate:** 1h
|
||||||
|
- **Done means:** Before `docker compose up`, CI validates compose file with `docker compose config`. Invalid files block deploy.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Normal deploy → config validation passes → deploy proceeds
|
||||||
|
2. Corrupted compose file → config validation fails → deploy blocked
|
||||||
|
- **Risk:** `docker compose config` requires Docker daemon running. Networks referenced but not existing produce warnings, not errors — acceptable.
|
||||||
|
|
||||||
|
- [ ] S04.7 — Add docker-compose config validation to CI
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Exit criteria
|
||||||
|
|
||||||
|
- [ ] All workflows have comments explaining `DOCKER_BUILDKIT=0`
|
||||||
|
- [ ] All `docker build` commands include `--cache-to type=inline`
|
||||||
|
- [ ] CI logs show pinned Python 3.12 and Node 20 versions
|
||||||
|
- [ ] `deploy.yml` has no `test-summary` job
|
||||||
|
- [ ] `required-tools.txt` includes bash, ca-certificates, openssh-client
|
||||||
|
- [ ] No stale debug comment in claude-dev/Dockerfile
|
||||||
|
- [ ] `.gitea/workflows/` contains only YAML files
|
||||||
|
- [ ] Deploy includes compose config validation step
|
||||||
|
- [ ] CI workflows pass on push to dev
|
||||||
@@ -0,0 +1,96 @@
|
|||||||
|
# Sprint 05 — Operations & Resilience
|
||||||
|
|
||||||
|
**Depends on:** S04 (CI must be reliable before automating ops)
|
||||||
|
**Duration:** ~12h
|
||||||
|
**Goal:** Add health checks, resource limits, log rotation, fix Immich uploads, back up dashboard data, add monitoring endpoint.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## S05.1 — Add HEALTHCHECK to claude-dev image
|
||||||
|
|
||||||
|
- **Files:** `claude-dev/Dockerfile`, `claude-dev/docker-compose.yml`
|
||||||
|
- **Estimate:** 1h
|
||||||
|
- **Done means:** Dockerfile has `HEALTHCHECK --interval=30s --timeout=5s --retries=3 CMD claude --version || exit 1`. `docker ps` shows healthy.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Deploy claude-dev → `docker ps` shows "(healthy)"
|
||||||
|
2. Break `claude` binary → container shows "(unhealthy)" after 3 failures
|
||||||
|
- **Risk:** `claude --version` may require network access. Test on NAS first. Alternative: `pgrep -f claude` or simple `curl localhost:<port>`.
|
||||||
|
|
||||||
|
- [ ] S05.1 — Add HEALTHCHECK to claude-dev image
|
||||||
|
|
||||||
|
## S05.2 — Add resource limits to production containers
|
||||||
|
|
||||||
|
- **Files:** `claude-dev/docker-compose.yml`, `dashboard/docker-compose.yml`
|
||||||
|
- **Estimate:** 1.5h
|
||||||
|
- **Done means:** Production compose files have `mem_limit`, `cpus`, and `restart_policy`. Limits are generous: 2GB dashboard, 1GB claude-dev, 0.5GB sidecars.
|
||||||
|
- **Verify by:**
|
||||||
|
1. `docker stats` shows containers respecting limits
|
||||||
|
2. Normal operation unaffected
|
||||||
|
3. `docker compose up -d` applies limits without restarting
|
||||||
|
- **Risk:** Limits too tight → OOM kills. Start generous, adjust after a week of monitoring.
|
||||||
|
|
||||||
|
- [ ] S05.2 — Add resource limits to production containers
|
||||||
|
|
||||||
|
## S05.3 — Implement audit log rotation
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/main.py:161,171-188`
|
||||||
|
- **Estimate:** 2h
|
||||||
|
- **Done means:** Audit logging uses `RotatingFileHandler` with max 10MB and 5 backups.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Write 11MB of audit entries → file rotates, `audit.log.1` created
|
||||||
|
2. Original `audit.log` starts fresh
|
||||||
|
3. Old backups capped at 5 files
|
||||||
|
- **Risk:** Log format change may affect parsing in `system.py:82-116`. Test the log reader with rotated files.
|
||||||
|
|
||||||
|
- [ ] S05.3 — Implement audit log rotation
|
||||||
|
|
||||||
|
## S05.4 — Fix Immich ML model downloads
|
||||||
|
|
||||||
|
- **Files:** `immich/docker-compose.yml`, `immich/.env`
|
||||||
|
- **Estimate:** 3h (investigation + fix)
|
||||||
|
- **Done means:** Immich mobile upload works. ML models download successfully or ML is gracefully disabled.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Upload photo from phone → succeeds (no timeout)
|
||||||
|
2. `ssh nas "docker logs immich_machine_learning"` → no "Failed to load detection model" errors
|
||||||
|
3. If ML disabled: photo upload works, smart search unavailable (acceptable)
|
||||||
|
- **Risk:** Most complex item. May require pre-downloading models, configuring mirror, increasing timeouts, or disabling ML temporarily. Environment-specific (China network to modelscope.cn).
|
||||||
|
|
||||||
|
- [ ] S05.4 — Fix Immich ML model downloads
|
||||||
|
|
||||||
|
## S05.5 — Add dashboard data to backup script
|
||||||
|
|
||||||
|
- **Files:** `backup.sh`
|
||||||
|
- **Estimate:** 1.5h
|
||||||
|
- **Done means:** `backup.sh` copies `opc.db`, `auth.json`, `rbac.json`, and audit log to backup tarball. Restore procedure documented.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Run `backup.sh` → tarball contains dashboard data files
|
||||||
|
2. Extract tarball → files are valid (SQLite DB opens, JSON parses)
|
||||||
|
- **Risk:** `auth.json` contains passkey data — ensure backup stored securely.
|
||||||
|
|
||||||
|
- [ ] S05.5 — Add dashboard data to backup script
|
||||||
|
|
||||||
|
## S05.6 — Add minimal health check endpoint
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/main.py` or `routers/health.py`
|
||||||
|
- **Estimate:** 1h
|
||||||
|
- **Done means:** `GET /api/health` returns `{"status":"ok","db":true,"docker":true,"disk":{"free_gb":123}}`. Non-200 triggers optional Telegram notification.
|
||||||
|
- **Verify by:**
|
||||||
|
1. All services healthy → `/api/health` returns 200
|
||||||
|
2. Docker socket unreachable → returns 503 with `docker: false`
|
||||||
|
3. Cron job calls `/api/health` every 5 minutes
|
||||||
|
- **Risk:** Health endpoint must be lightweight. Cache results for 30 seconds.
|
||||||
|
|
||||||
|
- [ ] S05.6 — Add minimal health check endpoint
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Exit criteria
|
||||||
|
|
||||||
|
- [ ] `docker ps` shows claude-dev as "(healthy)"
|
||||||
|
- [ ] Production containers have CPU/memory limits in compose
|
||||||
|
- [ ] Audit log rotates at 10MB with 5 backups
|
||||||
|
- [ ] Photo upload from phone succeeds
|
||||||
|
- [ ] `backup.sh` includes dashboard data files
|
||||||
|
- [ ] `GET /api/health` returns component statuses
|
||||||
|
- [ ] All backend tests pass
|
||||||
|
- [ ] All frontend tests pass
|
||||||
@@ -0,0 +1,111 @@
|
|||||||
|
# Sprint 06 — Code Quality & Refactoring
|
||||||
|
|
||||||
|
**Depends on:** S05 (system must be stable before large refactors)
|
||||||
|
**Duration:** ongoing (~12h total)
|
||||||
|
**Goal:** Reorganize frontend/backend structure, add retry logic, clean up networks, consolidate CI, fix SSR safety, remove legacy code.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## S06.1 — Reorganize frontend routes into subdirectories
|
||||||
|
|
||||||
|
- **Files:** `dashboard/frontend/src/routes/` (21 files), `dashboard/frontend/src/App.svelte`
|
||||||
|
- **Estimate:** 3h
|
||||||
|
- **Done means:** Routes grouped: `routes/media/`, `routes/tools/`, `routes/admin/`. `App.svelte` imports updated. No functional changes.
|
||||||
|
- **Verify by:**
|
||||||
|
1. `npm run build` succeeds
|
||||||
|
2. Navigate to every page → all pages load
|
||||||
|
3. Frontend tests pass
|
||||||
|
- **Risk:** Update import paths in `App.svelte` and cross-route references. Do incrementally, one group at a time.
|
||||||
|
|
||||||
|
- [ ] S06.1 — Reorganize frontend routes into subdirectories
|
||||||
|
|
||||||
|
## S06.2 — Implement backend router auto-discovery
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/main.py:9-50`
|
||||||
|
- **Estimate:** 2h
|
||||||
|
- **Done means:** `main.py` uses auto-discovery to load routers from `routers/` directory instead of 40+ individual imports.
|
||||||
|
- **Verify by:**
|
||||||
|
1. All 18 routers still registered (check `/docs` OpenAPI page)
|
||||||
|
2. All integration tests pass
|
||||||
|
3. Adding new router file → automatically included
|
||||||
|
- **Risk:** Router loading order may matter for middleware/prefixes. Preserve existing order or make explicit via `__init__.py` manifest.
|
||||||
|
|
||||||
|
- [ ] S06.2 — Implement backend router auto-discovery
|
||||||
|
|
||||||
|
## S06.3 — Add retry/backoff to Docker container monitor
|
||||||
|
|
||||||
|
- **Files:** `dashboard/backend/routers/docker_router.py`
|
||||||
|
- **Estimate:** 2h
|
||||||
|
- **Done means:** Docker API calls have exponential backoff (1s, 2s, 4s, max 30s) with 3 retries. Timeouts no longer crash the monitor.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Temporarily pause docker-socket-proxy → Docker page shows "reconnecting..." not error
|
||||||
|
2. Resume proxy → containers load automatically
|
||||||
|
3. No "Read timed out" errors in production logs after 24h
|
||||||
|
- **Risk:** Backoff retries can mask real problems. Log each retry at WARNING level.
|
||||||
|
|
||||||
|
- [ ] S06.3 — Add retry/backoff to Docker container monitor
|
||||||
|
|
||||||
|
## S06.4 — Clean up Docker networks
|
||||||
|
|
||||||
|
- **Files:** `dashboard/docker-compose.yml`, `dashboard/docker-compose.dev.yml`
|
||||||
|
- **Estimate:** 1.5h
|
||||||
|
- **Done means:** Unused networks removed. Remaining networks documented. Network naming standardized.
|
||||||
|
- **Verify by:**
|
||||||
|
1. `docker network ls` on NAS → only in-use networks exist
|
||||||
|
2. `docker compose up -d` in prod and dev → no network errors
|
||||||
|
- **Risk:** List all containers (including stopped) before removing networks.
|
||||||
|
|
||||||
|
- [ ] S06.4 — Clean up Docker networks
|
||||||
|
|
||||||
|
## S06.5 — Consolidate CI workflows (DRY)
|
||||||
|
|
||||||
|
- **Files:** `.gitea/workflows/deploy.yml`, `deploy-dev.yml`, `deploy-claude-dev-dev.yml`
|
||||||
|
- **Estimate:** 3h
|
||||||
|
- **Done means:** Shared steps extracted. If Gitea Actions lacks composite actions, at minimum standardize patterns across all three files.
|
||||||
|
- **Verify by:**
|
||||||
|
1. All three workflows still run correctly
|
||||||
|
2. Changing a shared step updates all workflows
|
||||||
|
3. Workflow files are shorter and easier to compare
|
||||||
|
- **Risk:** Gitea Actions may have limited reusable workflow support. Verify before investing time.
|
||||||
|
|
||||||
|
- [ ] S06.5 — Consolidate CI workflows (DRY)
|
||||||
|
|
||||||
|
## S06.6 — Guard window.isSecureContext in Login.svelte
|
||||||
|
|
||||||
|
- **Files:** `dashboard/frontend/src/routes/Login.svelte:11`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** `window.isSecureContext` check inside `onMount` or guarded with `typeof window !== 'undefined'`. SSR-safe.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Build with SSR enabled → no crash
|
||||||
|
2. Normal SPA build → login page works as before
|
||||||
|
- **Risk:** None. `onMount` only runs in browser.
|
||||||
|
|
||||||
|
- [ ] S06.6 — Guard window.isSecureContext in Login.svelte
|
||||||
|
|
||||||
|
## S06.7 — Remove legacy localStorage refresh token read
|
||||||
|
|
||||||
|
- **Files:** `dashboard/frontend/src/lib/api.js:27`
|
||||||
|
- **Estimate:** 0.5h
|
||||||
|
- **Done means:** Legacy `localStorage.getItem("refresh_token")` removed or documented with explicit comment.
|
||||||
|
- **Verify by:**
|
||||||
|
1. Login → token refresh works via cookies only
|
||||||
|
2. Clear cookies → redirect to login (no localStorage fallback)
|
||||||
|
3. Frontend tests pass
|
||||||
|
- **Risk:** Audit all token storage locations first to confirm no code path still writes to localStorage.
|
||||||
|
|
||||||
|
- [ ] S06.7 — Remove legacy localStorage refresh token read
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Exit criteria
|
||||||
|
|
||||||
|
- [ ] Frontend routes organized in subdirectories
|
||||||
|
- [ ] Backend uses router auto-discovery
|
||||||
|
- [ ] Docker monitor retries with backoff (no crashes on timeout)
|
||||||
|
- [ ] Only in-use Docker networks remain
|
||||||
|
- [ ] CI workflows are DRY (or documented why they can't be)
|
||||||
|
- [ ] Login.svelte is SSR-safe
|
||||||
|
- [ ] No legacy localStorage refresh token logic (or clearly documented)
|
||||||
|
- [ ] All backend tests pass
|
||||||
|
- [ ] All frontend tests pass
|
||||||
|
- [ ] `npm run build` succeeds
|
||||||
Reference in New Issue
Block a user