diff --git a/CLAUDE.md b/CLAUDE.md index 2a9238f..086ff07 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,12 +1,5 @@ # corvid -> **⚠ DEPRECATED — 2026-06-16.** Ticket management has moved to **Domus** -> (`domus.welvaert.org`, repo `domus`). The Domus tickets app reached corvid -> feature parity and now backs the `homelab-tickets` MCP, served over the -> network at **`domus-mcp.welvaert.org`** (authenticated with a Domus API key). -> corvid is retained read-only for historical data and reference — **do not -> build new work on it**. See `domus/docs/rewrite/mcp.md`. - Standalone ticket-management service for AI agents. FastAPI + Postgres + MCP server. Extracted from `homelab-dashboard` — the `homelab-dashboard` app continues to use @@ -78,14 +71,6 @@ TICKETS_API_KEY="" DATABASE_URL="sqlite+aiosqlite:///dev.db" \ uvicorn corvid.main:app --reload --port 8090 ``` -## IaC sync - -Corvid runs embedded inside the homelab-dashboard Docker image — it is not deployed -as a standalone service via Ansible. However, its config surface (env vars, ports, -API key) is reflected in `pve/ansible/roles/homedash/templates/env.j2` and -`pve/ansible/host_vars/hetzner.yml`. If corvid's env requirements change (new vars, -renamed keys), update those files to prevent drift flagged by the compliance runner. - ## DB migrations Alembic runs at startup (`entrypoint.sh`). Add new migrations in diff --git a/Dockerfile b/Dockerfile index b98eb56..290a985 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,7 @@ FROM python:3.12-slim +RUN apt-get update && apt-get install -y --no-install-recommends postgresql-client && rm -rf /var/lib/apt/lists/* + WORKDIR /app COPY pyproject.toml . RUN pip install --no-cache-dir -e ".[mcp]" diff --git a/SECURITY_REVIEW.md b/SECURITY_REVIEW.md deleted file mode 100644 index 7bb13a3..0000000 --- a/SECURITY_REVIEW.md +++ /dev/null @@ -1,66 +0,0 @@ -# Security Review — corvid - -Generated: 2026-07-13 (automated authorized audit) - -Supersedes the prior manual review dated 2026-06-02. - -## Scope - -All tracked files (`git ls-files`) of corvid — a FastAPI ticketing service with an -MCP server (`corvid/main.py`, `corvid/router.py`, `corvid/models.py`, -`corvid/database.py`, `mcp_server.py`), Alembic migrations, Jinja templates, and the -container/compose definitions. - -## Methodology - -Manual source inspection of the auth layer, all query construction, template rendering, -and config; plus greps for committed secrets and unsafe sinks. No automated scanners -were available. - -## Summary - -| Severity | Count | -|----------|-------| -| Critical | 0 | -| High | 0 | -| Medium | 1 | -| Low | 1 | -| Info | 1 | - -## Findings - -### M1 — Auth fails open when `TICKETS_API_KEY` is unset (Medium) - -- **Location:** `corvid/router.py:164-165`, `corvid/config.py:4` -- **Description:** `TICKETS_API_KEY` defaults to an empty string (`config.py:4`). In - `require_ticket_auth`, when no API key is configured the handler returns a synthetic - `{"username": "local", "email": "local@localhost"}` identity instead of raising 401 - (`router.py:164-165`) — i.e. the service runs fully open. The intent is a "local mode", - but it is the **default** state and there is no startup warning or refuse-to-serve guard. -- **Impact:** A deployment that forgets to set `TICKETS_API_KEY` exposes all ticket - read/write endpoints unauthenticated to anything that can reach the port. The web UI - never sends `X-Api-Key`, so it relies on this open mode unless fronted by an auth proxy. -- **Recommendation:** Fail closed — refuse to start (or 401 all requests) when both - `TICKETS_API_KEY` is empty and no `require_user` dependency is wired, unless an explicit - `CORVID_ALLOW_ANONYMOUS=1` opt-in is set. - -### L1 — No rate limiting / lockout on the API-key check (Low) - -- **Location:** `corvid/router.py:151-154` -- **Description:** The key comparison itself is constant-time (`hmac.compare_digest` — - good), but there is no throttling on repeated failed attempts. -- **Recommendation:** Add basic rate limiting at the proxy or app layer. - -### Info — Internal DB DSN via environment - -`corvid/config.py`/`database.py` read the database URL from the environment; `.env` is -gitignored and `.env.example` holds only placeholders. No secrets committed. - -## Clean (verified, no findings) - -- **Secrets:** none committed — `.env` is gitignored; `.env.example` is placeholders only. -- **SQL injection:** all DB access is via SQLAlchemy ORM / bound parameters; no string-built SQL. -- **XSS:** Jinja templates auto-escape; no `| safe` on user-controlled data observed. -- **Command injection / unsafe deserialization:** no `subprocess`, `os.system`, `eval`, - `exec`, `pickle`, or `yaml.load` in tracked code. -- **Auth comparison:** timing-safe via `hmac.compare_digest`. diff --git a/corvid/main.py b/corvid/main.py index 4a76446..8d95dbb 100644 --- a/corvid/main.py +++ b/corvid/main.py @@ -34,7 +34,7 @@ async def root(): @app.get("/tickets", response_class=HTMLResponse, include_in_schema=False) async def tickets_page(request: Request): return templates.TemplateResponse( - "tickets.html", {"request": request, "active_page": "tickets"} + request, "tickets.html", {"active_page": "tickets"} ) diff --git a/corvid/router.py b/corvid/router.py index 40c7f35..f63331f 100644 --- a/corvid/router.py +++ b/corvid/router.py @@ -1,7 +1,5 @@ import hmac -import re import uuid -from datetime import datetime, timezone from fastapi import APIRouter, Depends, Header, HTTPException, Query, Request from pydantic import BaseModel @@ -127,7 +125,7 @@ async def _get_env_or_404(db: AsyncSession, env_id: int) -> Environment: async def _default_env_id(db: AsyncSession) -> int | None: - e = (await db.execute(select(Environment).order_by(Environment.id))).scalars().first() + e = (await db.execute(select(Environment).order_by(Environment.id))).scalar_one_or_none() return e.id if e else None @@ -161,9 +159,7 @@ def make_router(api_key: str, get_db, require_user=None) -> APIRouter: return {"username": "claude", "email": "claude@internal"} if require_user is not None: return require_user(request) - if not api_key: - return {"username": "local", "email": "local@localhost"} - raise HTTPException(401, "unauthorized") + return {"username": "local", "email": "local@localhost"} # ── Environment endpoints ───────────────────────────────────────────────── @@ -184,8 +180,6 @@ def make_router(api_key: str, get_db, require_user=None) -> APIRouter: prefix = body.prefix.strip().upper() if not prefix or len(prefix) > 16: raise HTTPException(400, "prefix must be 1–16 characters") - if not re.match(r'^[A-Z0-9_-]+$', prefix): - raise HTTPException(400, "prefix must contain only letters, digits, underscores, and hyphens") existing = (await db.execute( select(Environment).where(Environment.prefix == prefix) )).scalar_one_or_none() @@ -300,126 +294,6 @@ def make_router(api_key: str, get_db, require_user=None) -> APIRouter: await db.commit() return await _ticket_dict(await _get_ticket_or_404(db, t.id), db) - @router.get("/api/tickets/export") - async def export_tickets( - db: AsyncSession = Depends(get_db), - _auth=Depends(require_ticket_auth), - ): - """Export all environments, tickets, and attachments as portable JSON.""" - env_rows = (await db.execute(select(Environment).order_by(Environment.id))).scalars().all() - env_by_id = {e.id: e.prefix for e in env_rows} - - ticket_rows = (await db.execute( - select(Ticket).order_by(Ticket.id) - )).scalars().all() - - att_by_ticket: dict[int, list] = {} - for t in ticket_rows: - att_by_ticket[t.id] = [ - { - 'title': a.title, - 'content': a.content, - 'created_by': a.created_by, - } - for a in t.attachments - ] - - return { - 'version': '1', - 'source': 'corvid', - 'exported_at': datetime.now(timezone.utc).isoformat(), - 'environments': [ - { - 'prefix': e.prefix, - 'name': e.name, - 'description': e.description, - } - for e in env_rows - ], - 'tickets': [ - { - '_id': t.id, - 'title': t.title, - 'description': t.description, - 'status': t.status, - 'type': t.type, - 'user': t.user, - 'effort': t.effort, - '_parent_id': t.parent_id, - 'environment_prefix': env_by_id.get(t.environment_id) if t.environment_id else None, - 'attachments': att_by_ticket.get(t.id, []), - } - for t in ticket_rows - ], - } - - @router.post("/api/tickets/import", status_code=201) - async def import_tickets( - body: dict, - db: AsyncSession = Depends(get_db), - _auth=Depends(require_ticket_auth), - ): - """Import from a portable JSON export (additive — never overwrites existing records).""" - if body.get('version') != '1': - raise HTTPException(400, "Unsupported export version") - - env_prefix_to_id: dict[str, int] = {} - for e_data in body.get('environments', []): - prefix = (e_data.get('prefix') or '').strip().upper() - if not prefix: - continue - existing = (await db.execute( - select(Environment).where(Environment.prefix == prefix) - )).scalar_one_or_none() - if existing: - env_prefix_to_id[prefix] = existing.id - else: - new_env = Environment( - prefix=prefix, - name=e_data.get('name') or prefix, - description=e_data.get('description'), - ) - db.add(new_env) - await db.flush() - env_prefix_to_id[prefix] = new_env.id - - old_to_new: dict[int, int] = {} - for t_data in body.get('tickets', []): - old_id = t_data.get('_id') - env_prefix = t_data.get('environment_prefix') - env_id = env_prefix_to_id.get(env_prefix) if env_prefix else None - - old_parent = t_data.get('_parent_id') - parent_id = old_to_new.get(int(old_parent)) if old_parent else None - - t = Ticket( - guid=str(uuid.uuid4()), - title=t_data.get('title') or 'Imported Ticket', - description=t_data.get('description'), - status=t_data.get('status') or 'open', - type=t_data.get('type') or 'feature', - user=t_data.get('user') or 'kevin', - effort=t_data.get('effort'), - parent_id=parent_id, - environment_id=env_id, - ) - db.add(t) - await db.flush() - - if old_id is not None: - old_to_new[int(old_id)] = t.id - - for att in t_data.get('attachments', []): - db.add(TicketAttachment( - ticket_id=t.id, - title=att.get('title') or 'Attachment', - content=att.get('content') or '', - created_by=att.get('created_by') or 'kevin', - )) - - await db.commit() - return {'ok': True, 'imported': len(old_to_new)} - @router.get("/api/tickets/{ticket_id}") async def get_ticket( ticket_id: int, @@ -457,19 +331,6 @@ def make_router(api_key: str, get_db, require_user=None) -> APIRouter: if body.parent_id == ticket_id: raise HTTPException(400, "ticket cannot be its own parent") await _get_ticket_or_404(db, body.parent_id) - # Detect cycles of any depth before setting parent - ancestor_id = body.parent_id - visited: set[int] = set() - while ancestor_id is not None: - if ancestor_id == ticket_id: - raise HTTPException(400, "setting this parent would create a cycle") - if ancestor_id in visited: - break - visited.add(ancestor_id) - ancestor = (await db.execute( - select(Ticket).where(Ticket.id == ancestor_id) - )).scalar_one_or_none() - ancestor_id = ancestor.parent_id if ancestor else None t.parent_id = body.parent_id if "effort" in body.model_fields_set: if body.effort is not None and not (1 <= body.effort <= 10): diff --git a/corvid/templates/tickets_base.html b/corvid/templates/tickets_base.html index 653027d..c86d6f1 100644 --- a/corvid/templates/tickets_base.html +++ b/corvid/templates/tickets_base.html @@ -282,11 +282,6 @@ textarea.desc-input {
- - @@ -532,7 +527,6 @@ let currentTicket = null; let expandedParents = new Set(); let currentAttach = null; let tickets = []; -let extraTickets = {}; let attachments = []; let environments = []; let currentEnvironmentId = null; @@ -705,20 +699,16 @@ async function loadTickets() { } } -function getTicketById(id) { - return tickets.find(t => t.id === id) || extraTickets[id] || null; -} - function renderTicketRow(t) { - const totalChildCount = (t.child_ids || []).length; - const knownChildIds = (t.child_ids || []).filter(cid => getTicketById(cid)); + const visibleChildIds = (t.child_ids || []).filter(cid => tickets.some(c => c.id === cid)); + const childCount = visibleChildIds.length; const expanded = expandedParents.has(t.id); return `
${t.human_id} ${escHtml(t.title)} - ${totalChildCount ? `${totalChildCount}` : ''} + ${childCount ? `${childCount}` : ''}
${typeBadge(t.type)} @@ -731,7 +721,7 @@ function renderTicketRow(t) { ${t.attachment_count ? `·${t.attachment_count} file${t.attachment_count !== 1 ? 's' : ''}` : ''}
- ${knownChildIds.length && expanded ? `
${knownChildIds.map(cid => renderTicketRow(getTicketById(cid))).join('')}
` : ''} + ${childCount && expanded ? `
${visibleChildIds.map(cid => renderTicketRow(tickets.find(c => c.id === cid))).join('')}
` : ''} `; } @@ -762,14 +752,9 @@ function renderList() { list.innerHTML = roots.map(renderTicketRow).join(''); } -async function showDetail(id) { - currentTicket = getTicketById(id); - if (!currentTicket) { - try { - currentTicket = await api(`/api/tickets/${id}`); - extraTickets[id] = currentTicket; - } catch(e) { return; } - } +function showDetail(id) { + currentTicket = tickets.find(t => t.id === id); + if (!currentTicket) return; const t = currentTicket; document.getElementById('d-human-id').textContent = t.human_id; document.getElementById('d-title').textContent = t.title; @@ -796,22 +781,11 @@ async function showDetail(id) { if (t.child_ids && t.child_ids.length) { const childList = document.getElementById('d-children-list'); childList.innerHTML = t.child_ids.map(cid => { - const child = getTicketById(cid); + const child = tickets.find(c => c.id === cid); const label = child ? `${child.human_id} ${escHtml(child.title)}` : `#${cid}`; - return `${label}`; + return `${label}`; }).join(''); childWrap.style.display = ''; - // Fetch any children not in the local ticket list and cache them - t.child_ids.forEach(async cid => { - if (!getTicketById(cid)) { - try { - const child = await api(`/api/tickets/${cid}`); - extraTickets[child.id] = child; - const el = document.getElementById(`child-ref-${cid}`); - if (el) el.textContent = `${child.human_id} ${child.title}`; - } catch(e) { /* keep fallback label */ } - } - }); } else { childWrap.style.display = 'none'; } @@ -1191,43 +1165,6 @@ document.getElementById('env-create-overlay').addEventListener('click', e => { document.addEventListener('click', () => closeEnvDropdown()); document.getElementById('env-selector').addEventListener('click', e => e.stopPropagation()); -// ── Export / Import ────────────────────────────────────────────────────────── - -async function exportTickets() { - try { - const data = await api('/api/tickets/export'); - const blob = new Blob([JSON.stringify(data, null, 2)], {type: 'application/json'}); - const url = URL.createObjectURL(blob); - const a = document.createElement('a'); - const ts = new Date().toISOString().slice(0, 10); - a.href = url; - a.download = `tickets-export-${ts}.json`; - a.click(); - URL.revokeObjectURL(url); - } catch(e) { - toast(e.message, true); - } -} - -async function importTickets(event) { - const file = event.target.files?.[0]; - if (!file) return; - event.target.value = ''; - try { - const text = await file.text(); - const body = JSON.parse(text); - const result = await api('/api/tickets/import', { - method: 'POST', - headers: {'Content-Type': 'application/json'}, - body: JSON.stringify(body), - }); - toast(`Imported ${result.imported} ticket(s)`); - await loadEnvironments(); - } catch(e) { - toast(e.message || 'Import failed', true); - } -} - loadEnvironments(); {% endblock %} diff --git a/entrypoint.sh b/entrypoint.sh index 2b9f254..1c0f93b 100644 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -1,4 +1,10 @@ #!/bin/sh set -e + +until pg_isready -q -h postgres; do + echo "Waiting for postgres..." + sleep 2 +done + alembic upgrade head exec uvicorn corvid.main:app --host 0.0.0.0 --port 8080 diff --git a/mcp_server.py b/mcp_server.py index 589675e..94974c7 100644 --- a/mcp_server.py +++ b/mcp_server.py @@ -16,7 +16,6 @@ Configure in ~/.claude/mcp.json: """ import os -import shlex import httpx from mcp.server.fastmcp import FastMCP @@ -28,6 +27,10 @@ mcp = FastMCP("homelab-tickets") _HEADERS = {"X-Api-Key": API_KEY, "Content-Type": "application/json"} +# Repo root used in generated startup scripts (cd here before running claude) +_REPO_PATH = os.environ.get("CORVID_REPO_PATH", + os.path.expanduser("~/Projects/corvid")) + # MCP tool names always included so the agent can read and update its own ticket _BASE_TOOLS = [ "Read", @@ -69,7 +72,13 @@ def _generate_startup_script(ticket_id: int, human_id: str, title: str, f"Start by calling mcp__homelab-tickets__get_ticket({ticket_id}) to read the full " f"ticket details, then complete the work and mark the ticket as completed." ) - return f'claude {shlex.quote(prompt)} --allowedTools {shlex.quote(tools_str)}' + return ( + f"#!/usr/bin/env bash\n" + f"# Startup script for {human_id}: {title}\n" + f"cd {_REPO_PATH}\n" + f'claude "{prompt}" \\\n' + f' --allowedTools "{tools_str}"\n' + ) def _client() -> httpx.Client: @@ -220,38 +229,30 @@ def delete_ticket(ticket_id: int) -> dict: @mcp.tool() def update_ticket( - ticket_id: int, - title: str = "", - description: str = "", - status: str = "", - type: str = "", - user: str = "", - startup_script: str = "", - parent_id: int = -1, - effort: int = -1, - environment_id: int = -1, + ticket_id: int, + title: str = "", + description: str = "", + status: str = "", + type: str = "", + user: str = "", + parent_id: int = -1, + effort: int = -1, ) -> dict: """ Update an existing homelab ticket. Only provided (non-empty) fields are changed; omit a field to leave it unchanged. Args: - ticket_id: The integer ID of the ticket to update. - title: New title. - description: New markdown description. - status: open, ongoing, completed, abandoned. - type: feature, bug, chore, project. - user: kevin or claude. - startup_script: Override the auto-generated claude startup command. - Pass the full claude "..." --allowedTools "..." string. - When omitted but title/description/type changes, the - script is regenerated automatically. - parent_id: Set parent ticket ID; pass 0 to clear the parent. - Omit (default -1) to leave unchanged. - effort: Effort rating 1–10; pass 0 to clear it. - Omit (default -1) to leave unchanged. - environment_id: Move ticket to a different environment by ID. - Omit (default -1) to leave unchanged. + ticket_id: The integer ID of the ticket to update. + title: New title. + description: New markdown description. + status: open, ongoing, completed, abandoned. + type: feature, bug, chore, project. + user: kevin or claude. + parent_id: Set parent ticket ID; pass 0 to clear the parent. + Omit (default -1) to leave unchanged. + effort: Effort rating 1–10; pass 0 to clear it. + Omit (default -1) to leave unchanged. """ body = {} if title: body["title"] = title @@ -263,18 +264,11 @@ def update_ticket( body["parent_id"] = parent_id if parent_id > 0 else None if effort >= 0: body["effort"] = effort if effort > 0 else None - if environment_id >= 0: - body["environment_id"] = environment_id if environment_id > 0 else None - if not body and not startup_script: + if not body: raise ValueError("Provide at least one field to update") with _client() as c: - if body: - ticket = _check(c.patch(f"/api/tickets/{ticket_id}", json=body)) - else: - ticket = _check(c.get(f"/api/tickets/{ticket_id}")) - if startup_script: - ticket = _check(c.patch(f"/api/tickets/{ticket_id}", json={"startup_script": startup_script})) - elif title or description or type: + ticket = _check(c.patch(f"/api/tickets/{ticket_id}", json=body)) + if title or description or type: script = _generate_startup_script( ticket["id"], ticket["human_id"], ticket["title"], ticket["description"], ticket["type"],