diff --git a/README.md b/README.md index 574f16c..3089aab 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,41 @@ as intuitive as Dockge, as capable as Portainer for Compose workflows. > (Auto-update) + Phase 23 (Secrets & configs) + Phase 24 (Design System v2) > complete. +## Upgrading to 0.56.0 — private registries, and a silent bug fixed + +**Update checks on private images were lying.** StackPilot asks the registry for +a tag's current digest itself, and it could only do that anonymously. A private +repository answers `401`, the check gave up, and the result was indistinguishable +from a network blip — so the Images page said nothing and a stack sitting on a +six-month-old image looked up to date. It now says +`ghcr.io needs credentials`, or `ghcr.io rejected the stored credentials` when +there are some and they are wrong. + +**Settings → Private registries** takes a login per registry (Docker Hub, +ghcr.io, or your own), with a *Test* button that actually asks the registry. Add +one and the update check starts working for those images. + +The same credentials also reach `docker compose pull`. StackPilot writes a +Docker CLI config into `${DATA_DIR}/docker/config.json` (mode 0600, regenerated +from the database on every change) and runs compose with `DOCKER_CONFIG` pointed +at it — so pulling a private image works without anyone running `docker login` +inside the container, and removing a registry in the UI actually revokes the +CLI's access instead of leaving a stale login behind. + +Passwords are encrypted at rest with the same key as backup destinations, and +are never sent to the browser — not even masked. Editing a registry with the +password field left blank keeps the stored one. A registry whose password +cannot be decrypted (a changed `SECRET_KEY`) is skipped with a warning rather +than taking the others down with it. + +Host spellings are normalized, which is the join that makes the whole thing +work: `docker.io`, `index.docker.io`, `https://index.docker.io/v1/` and +`registry-1.docker.io` are one registry, because a bare `nginx:alpine` resolves +to the last of those while nobody types it that way. Prefer an access token over +your account password — read scope is enough. + +Nothing to do if you only use public images. + ## Upgrading to 0.55.0 — nothing to do **Images and Networks are grouped by stack too**, the same way Volumes were in @@ -318,6 +353,12 @@ it is what your saved destination credentials are encrypted with. compose** converter. - **Dashboard** — system resource bar, stack grid with quick actions, and a recent-activity audit feed. +- **Private registries** — a login per registry (Settings → Private registries) + used both by StackPilot's own update checks and by `docker compose pull`, + which it reaches through a generated `DOCKER_CONFIG`. Passwords are encrypted + at rest and never leave the server. Without one, an image whose registry + demands auth now reports *needs credentials* instead of quietly looking up to + date. - **Auto-discovery** — stacks created outside the UI (any folder under the stacks dir containing a compose file) are picked up automatically. - **Dark / light theme.** @@ -863,6 +904,16 @@ GET /api/dashboard/funnel[?refresh=true] (stack-health funnel, 30s TTL cache) GET /api/dashboard/summary (containers, uptime series, ops activity) ``` +### Private registry endpoints + +``` +GET /api/registries (admin; passwords are never returned) +POST /api/registries ({"host","username","password","name"?}) +PUT /api/registries/{id} (omit "password" to keep the stored one) +DELETE /api/registries/{id} (also drops the Docker CLI login) +POST /api/registries/test (verify credentials; omit "password" to test a saved one) +``` + ### Stack icon endpoints ``` @@ -878,6 +929,9 @@ GET /api/stacks/icons/logo/{slug} (one catalog logo, served from our cache) - The Docker socket is only ever touched by the backend process; it is never proxied to the browser. +- Registry passwords and backup-destination credentials are encrypted at rest + (Fernet, key derived from `SECRET_KEY`). The generated Docker CLI config that + carries them for `compose pull` is written 0600 inside the data volume. - Login is rate-limited (10/min/IP). - Compose files are backed up to `*.bak` before every overwrite. - Generated YAML never includes the obsolete `version:` field and uses Compose v2 diff --git a/backend/main.py b/backend/main.py index 7cb587d..5a71bd7 100644 --- a/backend/main.py +++ b/backend/main.py @@ -26,6 +26,7 @@ from routers import ( images, networks, ports, + registries, schedules, secrets, settings as settings_router, @@ -39,6 +40,7 @@ from services import ( backup_destination_service, image_status_store, logo_service, + registry_service, schedule_service, stack_lock_service, template_service, @@ -86,6 +88,15 @@ async def lifespan(app: FastAPI): logger.info("Restored %d cached image update status(es)", restored) except Exception as exc: # noqa: BLE001 logger.warning("Could not restore the image update cache: %s", exc) + # Private registry credentials: into the in-memory cache the update checker + # reads, and into the config.json the Docker CLI reads. + try: + with Session(engine) as session: + known = registry_service.reload(session) + if known: + logger.info("Loaded credentials for %d registr%s", known, "y" if known == 1 else "ies") + except Exception as exc: # noqa: BLE001 + logger.warning("Could not load registry credentials: %s", exc) update_task = asyncio.create_task(update_service.background_loop()) schedule_task = asyncio.create_task(schedule_service.scheduler_loop()) @@ -121,6 +132,7 @@ async def docker_error_handler(_request: Request, exc: DockerError): app.include_router(auth.router) app.include_router(stacks.router) +app.include_router(registries.router) app.include_router(secrets.router) app.include_router(containers.router) app.include_router(dashboard.router) diff --git a/backend/models/__init__.py b/backend/models/__init__.py index 149cddc..5cb23be 100644 --- a/backend/models/__init__.py +++ b/backend/models/__init__.py @@ -3,6 +3,7 @@ from models.audit import AuditLog from models.auto_update import AutoUpdate from models.backup_destination import BackupDestination from models.backup_schedule import BackupSchedule +from models.registry import Registry from models.runtime_state import ImageStatus, LoginAttempt, StackLock from models.setting import Setting, Webhook from models.stack import Stack @@ -11,5 +12,5 @@ from models.user import User __all__ = [ "User", "Stack", "AuditLog", "Setting", "Webhook", "BackupDestination", "BackupSchedule", "AutoUpdate", - "StackLock", "ImageStatus", "LoginAttempt", + "StackLock", "ImageStatus", "LoginAttempt", "Registry", ] diff --git a/backend/models/registry.py b/backend/models/registry.py new file mode 100644 index 0000000..10eb42c --- /dev/null +++ b/backend/models/registry.py @@ -0,0 +1,68 @@ +from __future__ import annotations + +from datetime import datetime, timezone +from typing import Optional + +from sqlmodel import Field, SQLModel + + +def _now() -> datetime: + return datetime.now(timezone.utc) + + +class Registry(SQLModel, table=True): + """Credentials for one container registry. + + ``host`` is the canonical registry hostname as + :func:`services.registry_service.canonical_host` produces it, so the lookup + from an image reference is a dict hit and Docker Hub's several spellings all + land on one row. + + The password is encrypted at rest (see ``services/crypto_service.py``) and + never leaves the API — reads return it masked. + """ + + id: Optional[int] = Field(default=None, primary_key=True) + name: str + host: str = Field(index=True) + username: str + password: str # encrypted + created_at: datetime = Field(default_factory=_now) + updated_at: datetime = Field(default_factory=_now) + + +# --- API schemas --- + + +class RegistryCreate(SQLModel): + name: Optional[str] = None + host: str + username: str + password: str + + +class RegistryUpdate(SQLModel): + name: Optional[str] = None + host: Optional[str] = None + username: Optional[str] = None + # Omitted leaves the stored password alone, so the UI can save a row it only + # ever received masked. + password: Optional[str] = None + + +class RegistryRead(SQLModel): + id: int + name: str + host: str + username: str + has_password: bool + created_at: datetime + updated_at: datetime + + +class RegistryTestRequest(SQLModel): + """An unsaved set of credentials to try, for the "Test" button.""" + + host: str + username: str + password: Optional[str] = None diff --git a/backend/routers/registries.py b/backend/routers/registries.py new file mode 100644 index 0000000..4bdb925 --- /dev/null +++ b/backend/routers/registries.py @@ -0,0 +1,186 @@ +"""Private registry credentials. + +Admin-only throughout, including the reads: even masked, the rows say which +registries this install talks to and under what account. +""" +from __future__ import annotations + +from datetime import datetime, timezone + +from fastapi import APIRouter, Depends, HTTPException, Request +from sqlmodel import Session, select + +from auth import require_admin +from database import get_session +from models.registry import ( + Registry, + RegistryCreate, + RegistryRead, + RegistryTestRequest, + RegistryUpdate, +) +from models.user import User +from services import audit_service, crypto_service, registry_service + +router = APIRouter(prefix="/api/registries", tags=["registries"]) + + +def _ip(request: Request) -> str: + return request.client.host if request.client else "unknown" + + +def _to_read(row: Registry) -> RegistryRead: + # The password never leaves the server, not even masked — the UI only needs + # to know whether one is stored, so it can leave the field blank on edit. + return RegistryRead( + id=row.id, + name=row.name, + host=row.host, + username=row.username, + has_password=bool(row.password), + created_at=row.created_at, + updated_at=row.updated_at, + ) + + +def _get_or_404(session: Session, registry_id: int) -> Registry: + row = session.get(Registry, registry_id) + if not row: + raise HTTPException(status_code=404, detail=f"Registry {registry_id} not found") + return row + + +def _canonical(host: str) -> str: + try: + return registry_service.canonical_host(host) + except registry_service.RegistryError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc + + +@router.get("", response_model=list[RegistryRead]) +def list_registries( + session: Session = Depends(get_session), + _user: User = Depends(require_admin), +) -> list[RegistryRead]: + rows = session.exec(select(Registry).order_by(Registry.host)).all() + return [_to_read(r) for r in rows] + + +@router.post("", response_model=RegistryRead, status_code=201) +def create_registry( + body: RegistryCreate, + request: Request, + session: Session = Depends(get_session), + user: User = Depends(require_admin), +) -> RegistryRead: + host = _canonical(body.host) + if session.exec(select(Registry).where(Registry.host == host)).first(): + # One set of credentials per registry: two rows for the same host would + # make "which account are we using" unanswerable. + raise HTTPException( + status_code=409, detail=f"Credentials for '{host}' already exist" + ) + if not body.username or not body.password: + raise HTTPException(status_code=400, detail="Username and password are required") + row = Registry( + name=body.name or host, + host=host, + username=body.username, + password=crypto_service.encrypt(body.password), + ) + session.add(row) + session.commit() + session.refresh(row) + registry_service.reload(session) + audit_service.record( + session, user=user.username, action="registry.create", target=host, + detail=f"as {body.username}", ip=_ip(request), + ) + return _to_read(row) + + +@router.put("/{registry_id}", response_model=RegistryRead) +def update_registry( + registry_id: int, + body: RegistryUpdate, + request: Request, + session: Session = Depends(get_session), + user: User = Depends(require_admin), +) -> RegistryRead: + row = _get_or_404(session, registry_id) + if body.host is not None: + host = _canonical(body.host) + clash = session.exec(select(Registry).where(Registry.host == host)).first() + if clash and clash.id != row.id: + raise HTTPException( + status_code=409, detail=f"Credentials for '{host}' already exist" + ) + row.host = host + if body.name is not None: + row.name = body.name + if body.username is not None: + row.username = body.username + # An omitted password keeps the stored one: the UI never received it, so it + # cannot send it back. + if body.password: + row.password = crypto_service.encrypt(body.password) + row.updated_at = datetime.now(timezone.utc) + session.add(row) + session.commit() + session.refresh(row) + registry_service.reload(session) + audit_service.record( + session, user=user.username, action="registry.update", target=row.host, + ip=_ip(request), + ) + return _to_read(row) + + +@router.delete("/{registry_id}") +def delete_registry( + registry_id: int, + request: Request, + session: Session = Depends(get_session), + user: User = Depends(require_admin), +) -> dict: + row = _get_or_404(session, registry_id) + host = row.host + session.delete(row) + session.commit() + # Rewrites config.json without this host, so the CLI loses the login too. + registry_service.reload(session) + audit_service.record( + session, user=user.username, action="registry.delete", target=host, + ip=_ip(request), + ) + return {"ok": True} + + +@router.post("/test") +async def test_credentials( + body: RegistryTestRequest, + session: Session = Depends(get_session), + _user: User = Depends(require_admin), +) -> dict: + """Try a set of credentials against the registry. + + With no password in the body, the stored one for that host is used — that is + how the UI can re-test a saved registry it never received the password for. + """ + host = _canonical(body.host) + password = body.password + username = body.username + if not password: + stored = session.exec(select(Registry).where(Registry.host == host)).first() + if not stored: + raise HTTPException(status_code=400, detail="A password is required") + try: + password = crypto_service.decrypt(stored.password) + except crypto_service.DecryptError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc + username = username or stored.username + try: + await registry_service.verify(host, username, password) + except registry_service.RegistryError as exc: + raise HTTPException(status_code=400, detail=str(exc)) from exc + return {"ok": True, "host": host} diff --git a/backend/services/compose_service.py b/backend/services/compose_service.py index 0369267..dcb2933 100644 --- a/backend/services/compose_service.py +++ b/backend/services/compose_service.py @@ -16,6 +16,7 @@ from typing import Optional from config import settings from docker_client import DockerError, get_client, safe_call +from services import registry_service COMPOSE_FILENAMES = ("compose.yaml", "compose.yml", "docker-compose.yml", "docker-compose.yaml") DEFAULT_COMPOSE_NAME = "compose.yaml" @@ -285,6 +286,7 @@ async def run_compose( cmd = _compose_base_cmd(stack_id, override) + args proc = await asyncio.create_subprocess_exec( *cmd, + env=registry_service.cli_env(), stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, ) @@ -322,6 +324,7 @@ async def validate_yaml(content: str, env_content: str = "") -> dict: try: proc = await asyncio.create_subprocess_exec( *cmd, + env=registry_service.cli_env(), stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, ) @@ -339,6 +342,7 @@ async def stream_compose( cmd = _compose_base_cmd(stack_id, override) + args proc = await asyncio.create_subprocess_exec( *cmd, + env=registry_service.cli_env(), stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.STDOUT, ) @@ -364,6 +368,7 @@ async def supports_json_progress() -> bool: try: proc = await asyncio.create_subprocess_exec( "docker", "compose", "--progress", "json", "version", + env=registry_service.cli_env(), stdout=asyncio.subprocess.DEVNULL, stderr=asyncio.subprocess.DEVNULL, ) @@ -386,6 +391,7 @@ async def _stream_phase( cmd += args proc = await asyncio.create_subprocess_exec( *cmd, + env=registry_service.cli_env(), stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.STDOUT, ) diff --git a/backend/services/registry_service.py b/backend/services/registry_service.py new file mode 100644 index 0000000..f5c8c56 --- /dev/null +++ b/backend/services/registry_service.py @@ -0,0 +1,249 @@ +"""Credentials for private container registries. + +Two very different consumers need these, which is why this module exists rather +than the credentials living next to either of them: + +* **StackPilot's own update checker** (``services/update_service.py``) talks to + the registry v2 API over HTTP itself. Without credentials a private repository + answers 401, the check gave up, and the UI said nothing at all — a stack could + sit on a stale image for months and look up to date. That was the actual bug + here, not a missing feature. +* **The Docker CLI**, which runs ``docker compose pull``. It reads its own + ``config.json``, so this module writes one into ``${DATA_DIR}/docker`` and + compose runs with ``DOCKER_CONFIG`` pointed at it. + +Lookups happen inside async registry calls that have no database session, so +the rows are mirrored into a small in-memory cache. :func:`reload` refills it and +rewrites the CLI config; every write path calls it, and so does startup. + +Passwords are encrypted at rest and only ever decrypted into this cache and the +CLI config file (0600, in the data volume). They are never returned by the API. +""" +from __future__ import annotations + +import base64 +import json +import logging +import os +import threading +from typing import Optional + +import httpx +from sqlmodel import Session, select + +from config import settings +from models.registry import Registry +from services import crypto_service + +logger = logging.getLogger("stackpilot.registries") + +#: What `parse_ref` calls Docker Hub, and what the CLI calls it. Every spelling +#: users type — docker.io, index.docker.io, the v1 URL — normalizes to the +#: first; the second is what has to appear in config.json for `docker pull`. +DOCKER_HUB = "registry-1.docker.io" +DOCKER_HUB_CONFIG_KEY = "https://index.docker.io/v1/" +_DOCKER_HUB_ALIASES = { + "docker.io", + "index.docker.io", + "registry.docker.io", + "registry-1.docker.io", + "https://index.docker.io/v1/", + "index.docker.io/v1/", +} + +TIMEOUT = httpx.Timeout(15.0) + +_lock = threading.Lock() +_credentials: dict[str, tuple[str, str]] = {} + + +class RegistryError(Exception): + """A registry that cannot be reached, or credentials it rejects.""" + + +# --------------------------------------------------------------------------- # +# Host normalization +# --------------------------------------------------------------------------- # + + +def canonical_host(value: str) -> str: + """The registry host as an image reference would name it. + + Accepts what people actually paste: a bare host, a URL with a scheme, a + trailing slash, Docker Hub under any of its names. Without this, credentials + entered as ``docker.io`` would never be found for an image that parses as + ``registry-1.docker.io``. + """ + host = (value or "").strip().lower() + if not host: + raise RegistryError("Registry host is required") + if host in _DOCKER_HUB_ALIASES: + return DOCKER_HUB + for scheme in ("https://", "http://"): + if host.startswith(scheme): + host = host[len(scheme) :] + break + host = host.split("/", 1)[0].rstrip("/") + if host in _DOCKER_HUB_ALIASES: + return DOCKER_HUB + if not host: + raise RegistryError("Registry host is required") + return host + + +def is_docker_hub(host: str) -> bool: + return canonical_host(host) == DOCKER_HUB + + +# --------------------------------------------------------------------------- # +# The cache the async paths read +# --------------------------------------------------------------------------- # + + +def reload(session: Session) -> int: + """Refill the credential cache from the database and rewrite config.json. + + Called at startup and after every write. Returns how many registries are + configured. + """ + fresh: dict[str, tuple[str, str]] = {} + for row in session.exec(select(Registry)).all(): + try: + password = crypto_service.decrypt(row.password) + except crypto_service.DecryptError as exc: + # One unreadable row must not take the others down with it. + logger.warning("Ignoring credentials for %s: %s", row.host, exc) + continue + fresh[row.host] = (row.username, password) + with _lock: + _credentials.clear() + _credentials.update(fresh) + _write_docker_config(fresh) + return len(fresh) + + +def credentials_for(host: str) -> Optional[tuple[str, str]]: + """(username, password) for a registry host, or None.""" + try: + key = canonical_host(host) + except RegistryError: + return None + with _lock: + return _credentials.get(key) + + +def credentials_for_image(image: str) -> Optional[tuple[str, str]]: + """Credentials for whichever registry an image reference points at.""" + from services import update_service + + registry, _repo, _tag = update_service.parse_ref(image) + return credentials_for(registry) + + +def configured_hosts() -> list[str]: + with _lock: + return sorted(_credentials) + + +# --------------------------------------------------------------------------- # +# The Docker CLI's config.json +# --------------------------------------------------------------------------- # + + +def docker_config_dir() -> str: + return os.path.join(settings.DATA_DIR, "docker") + + +def _write_docker_config(creds: dict[str, tuple[str, str]]) -> None: + """Write the auths file ``docker compose pull`` reads. + + The file is rewritten from the database every time, so removing a registry + in the UI actually revokes the CLI's access rather than leaving a stale + login behind. + """ + directory = docker_config_dir() + path = os.path.join(directory, "config.json") + auths = {} + for host, (username, password) in creds.items(): + key = DOCKER_HUB_CONFIG_KEY if host == DOCKER_HUB else host + token = base64.b64encode(f"{username}:{password}".encode("utf-8")).decode("ascii") + auths[key] = {"auth": token} + try: + os.makedirs(directory, mode=0o700, exist_ok=True) + tmp = f"{path}.tmp" + # Written 0600 before it is put in place, so the credentials are never + # briefly world-readable. + fd = os.open(tmp, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as fh: + json.dump({"auths": auths}, fh) + os.replace(tmp, path) + except OSError as exc: + logger.warning("Could not write the Docker CLI credentials file: %s", exc) + + +def cli_env() -> dict: + """Environment for a ``docker``/``docker compose`` subprocess. + + Points DOCKER_CONFIG at our generated file rather than writing into + ``~/.docker``, so what StackPilot manages stays separate from anything the + image ships or an operator put there by hand. + """ + return {**os.environ, "DOCKER_CONFIG": docker_config_dir()} + + +# --------------------------------------------------------------------------- # +# Verifying credentials +# --------------------------------------------------------------------------- # + + +async def verify(host: str, username: str, password: str) -> None: + """Check that a registry accepts these credentials. Raises RegistryError. + + Asks for a pull-scoped token the way a client would, and treats only an + outright 401 as "wrong credentials" — a registry that answers anything else + is reachable and talking, which is as much as a credentials check can + honestly claim. + """ + registry = canonical_host(host) + url = f"https://{registry}/v2/" + auth = (username, password) + try: + async with httpx.AsyncClient(follow_redirects=True, timeout=TIMEOUT) as client: + response = await client.get(url, auth=auth) + if response.status_code == 401: + challenge = response.headers.get("WWW-Authenticate", "") + if challenge.lower().startswith("bearer"): + token = await _token(client, challenge, auth) + if not token: + raise RegistryError("The registry rejected these credentials") + return + raise RegistryError("The registry rejected these credentials") + if response.status_code >= 500: + raise RegistryError( + f"The registry answered {response.status_code}; try again later" + ) + except httpx.HTTPError as exc: + raise RegistryError(f"Could not reach {registry}: {exc}") from exc + + +async def _token( + client: httpx.AsyncClient, challenge: str, auth: tuple[str, str] +) -> Optional[str]: + """Follow a Bearer challenge with credentials attached.""" + params = {} + for part in challenge[len("Bearer ") :].split(","): + if "=" in part: + key, value = part.split("=", 1) + params[key.strip()] = value.strip().strip('"') + realm = params.pop("realm", None) + if not realm: + return None + try: + response = await client.get(realm, params=params, auth=auth, timeout=TIMEOUT) + if response.status_code == 401: + return None + response.raise_for_status() + data = response.json() + return data.get("token") or data.get("access_token") + except (httpx.HTTPError, ValueError): + return None diff --git a/backend/services/update_service.py b/backend/services/update_service.py index d78ca6d..34ac461 100644 --- a/backend/services/update_service.py +++ b/backend/services/update_service.py @@ -17,7 +17,7 @@ import httpx from docker_client import DockerError, get_client, safe_call from models.setting import EVENT_UPDATE_AVAILABLE -from services import notify_service, settings_service +from services import notify_service, registry_service, settings_service logger = logging.getLogger("stackpilot.update") @@ -145,7 +145,16 @@ def _local_digest(image: str) -> Optional[str]: # --------------------------------------------------------------------------- # -async def _get_token(client: httpx.AsyncClient, www_auth: str) -> Optional[str]: +async def _get_token( + client: httpx.AsyncClient, + www_auth: str, + auth: Optional[tuple[str, str]] = None, +) -> Optional[str]: + """Follow a Bearer challenge, with credentials when we have them. + + A public image gets an anonymous token; a private one only gets a token at + all if the request to the token realm is authenticated. + """ # Parse: Bearer realm="...",service="...",scope="..." params = {} if not www_auth.lower().startswith("bearer"): @@ -158,7 +167,7 @@ async def _get_token(client: httpx.AsyncClient, www_auth: str) -> Optional[str]: if not realm: return None try: - resp = await client.get(realm, params=params, timeout=10) + resp = await client.get(realm, params=params, auth=auth, timeout=10) resp.raise_for_status() data = resp.json() return data.get("token") or data.get("access_token") @@ -166,25 +175,54 @@ async def _get_token(client: httpx.AsyncClient, www_auth: str) -> Optional[str]: return None +class AuthRequired(Exception): + """The registry wants credentials we do not have (or rejected ours). + + Distinct from "could not reach the registry" on purpose: a private image + with no configured credentials used to be indistinguishable from a network + blip, so the UI said nothing and the stack looked up to date forever. + """ + + async def remote_digest(image: str) -> Optional[str]: + """The digest the registry currently serves for this tag. + + Raises :class:`AuthRequired` when the registry refuses us; returns None when + it could not be reached or answered without a digest. + """ registry, repo, tag = parse_ref(image) if tag.startswith("sha256:"): return tag scheme = "https" url = f"{scheme}://{registry}/v2/{repo}/manifests/{tag}" headers = {"Accept": _MANIFEST_ACCEPT} + auth = registry_service.credentials_for(registry) async with httpx.AsyncClient(follow_redirects=True) as client: try: resp = await client.head(url, headers=headers, timeout=10) if resp.status_code == 401: - token = await _get_token(client, resp.headers.get("WWW-Authenticate", "")) - if not token: - return None - headers["Authorization"] = f"Bearer {token}" - resp = await client.head(url, headers=headers, timeout=10) + challenge = resp.headers.get("WWW-Authenticate", "") + if challenge.lower().startswith("basic"): + # A plain htpasswd-protected registry: no token dance. + if not auth: + raise AuthRequired(registry) + resp = await client.head(url, headers=headers, auth=auth, timeout=10) + else: + token = await _get_token(client, challenge, auth) + if not token: + raise AuthRequired(registry) + headers["Authorization"] = f"Bearer {token}" + resp = await client.head(url, headers=headers, timeout=10) + if resp.status_code in (401, 403): + raise AuthRequired(registry) if resp.status_code == 405 or "Docker-Content-Digest" not in resp.headers: # Some registries don't support HEAD; fall back to GET. - resp = await client.get(url, headers=headers, timeout=10) + resp = await client.get( + url, headers=headers, auth=auth if "Authorization" not in headers else None, + timeout=10, + ) + if resp.status_code in (401, 403): + raise AuthRequired(registry) digest = resp.headers.get("Docker-Content-Digest") return digest except httpx.HTTPError as exc: @@ -199,9 +237,18 @@ async def remote_digest(image: str) -> Optional[str]: async def check_image(image: str) -> UpdateStatus: local = _local_digest(image) - remote = await remote_digest(image) error = None - if remote is None: + try: + remote = await remote_digest(image) + except AuthRequired as exc: + # Say which registry, because the fix is to add credentials for it. + remote = None + error = ( + f"{exc} needs credentials" + if not registry_service.credentials_for(str(exc)) + else f"{exc} rejected the stored credentials" + ) + if remote is None and error is None: error = "could not reach registry" update_available = bool(local and remote and local != remote) status = UpdateStatus( diff --git a/backend/tests/test_registries.py b/backend/tests/test_registries.py new file mode 100644 index 0000000..095e67a --- /dev/null +++ b/backend/tests/test_registries.py @@ -0,0 +1,360 @@ +"""Private registry credentials. + +Three things have to hold, and the first one is a bug fix rather than a feature: + +* a private image whose registry refuses us must *say so*. Before this, a 401 + was indistinguishable from a network blip, so the UI showed nothing and a + stack sitting on a months-old image looked up to date. +* the host people type has to reach the host an image reference parses to. + Docker Hub has five spellings and `parse_ref` only ever produces one of them, + so credentials entered as "docker.io" would otherwise never be found. +* the password must be encrypted at rest and must never come back out of the + API — not even masked, because the UI has no use for it. +""" +from __future__ import annotations + +import base64 +import json +import os +import stat + +import pytest + + +@pytest.fixture +def svc(db): + from services import registry_service + + return registry_service + + +@pytest.fixture(autouse=True) +def clean_registries(db): + """Credentials are global state; don't let one test leak into the next.""" + from sqlmodel import Session, delete + + from database import engine + from models.registry import Registry + from services import registry_service + + def wipe(): + with Session(engine) as session: + session.exec(delete(Registry)) + session.commit() + registry_service.reload(session) + + wipe() + yield + wipe() + + +def _add(host: str, username: str = "bob", password: str = "hunter2"): + """Insert a registry the way the API would, and refresh the cache.""" + from sqlmodel import Session + + from database import engine + from models.registry import Registry + from services import crypto_service, registry_service + + with Session(engine) as session: + session.add( + Registry( + name=host, + host=registry_service.canonical_host(host), + username=username, + password=crypto_service.encrypt(password), + ) + ) + session.commit() + registry_service.reload(session) + + +# --------------------------------------------------------------------------- # +# Host normalization +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + "written", + [ + "docker.io", + "index.docker.io", + "registry-1.docker.io", + "https://index.docker.io/v1/", + "https://docker.io", + "DOCKER.IO", + " docker.io/ ", + ], +) +def test_every_spelling_of_docker_hub_lands_on_one_host(svc, written): + """`parse_ref` only ever says registry-1.docker.io; the lookup has to agree.""" + assert svc.canonical_host(written) == svc.DOCKER_HUB + + +@pytest.mark.parametrize( + "written,expected", + [ + ("ghcr.io", "ghcr.io"), + ("https://ghcr.io/", "ghcr.io"), + ("registry.example.com:5000", "registry.example.com:5000"), + ("http://registry.lan:5000/v2/", "registry.lan:5000"), + ("GHCR.IO", "ghcr.io"), + ], +) +def test_other_registries_keep_their_host_and_port(svc, written, expected): + assert svc.canonical_host(written) == expected + + +def test_an_empty_host_is_refused(svc): + for value in ("", " ", "https://"): + with pytest.raises(svc.RegistryError): + svc.canonical_host(value) + + +# --------------------------------------------------------------------------- # +# The lookup the update checker uses +# --------------------------------------------------------------------------- # + + +def test_credentials_are_found_for_the_registry_an_image_names(svc): + _add("ghcr.io", "bob", "hunter2") + assert svc.credentials_for("ghcr.io") == ("bob", "hunter2") + assert svc.credentials_for_image("ghcr.io/acme/app:1.2") == ("bob", "hunter2") + + +def test_docker_hub_credentials_are_found_for_a_bare_image_name(svc): + # "nginx:alpine" parses to registry-1.docker.io; the row was typed as + # "docker.io". This is the join that makes the whole feature work. + _add("docker.io", "bob", "hunter2") + assert svc.credentials_for_image("nginx:alpine") == ("bob", "hunter2") + assert svc.credentials_for_image("acme/private:1.0") == ("bob", "hunter2") + + +def test_an_unconfigured_registry_has_no_credentials(svc): + _add("ghcr.io") + assert svc.credentials_for("quay.io") is None + assert svc.credentials_for_image("quay.io/acme/app") is None + + +def test_a_row_that_cannot_be_decrypted_does_not_break_the_others(svc, monkeypatch): + """A changed SECRET_KEY must cost you one registry, not all of them.""" + from sqlmodel import Session + + from database import engine + from models.registry import Registry + + _add("ghcr.io", "bob", "hunter2") + with Session(engine) as session: + session.add(Registry(name="broken", host="quay.io", username="x", password="enc:v1:nonsense")) + session.commit() + svc.reload(session) + assert svc.credentials_for("ghcr.io") == ("bob", "hunter2") + assert svc.credentials_for("quay.io") is None + + +# --------------------------------------------------------------------------- # +# The file the Docker CLI reads +# --------------------------------------------------------------------------- # + + +def _config(svc) -> dict: + with open(os.path.join(svc.docker_config_dir(), "config.json"), encoding="utf-8") as fh: + return json.load(fh) + + +def test_the_cli_config_carries_a_usable_auth_entry(svc): + _add("ghcr.io", "bob", "hunter2") + auths = _config(svc)["auths"] + token = auths["ghcr.io"]["auth"] + assert base64.b64decode(token).decode() == "bob:hunter2" + + +def test_docker_hub_is_written_under_the_key_the_cli_expects(svc): + # The CLI looks Docker Hub up as https://index.docker.io/v1/, not as the + # host an image reference parses to. + _add("docker.io", "bob", "hunter2") + assert svc.DOCKER_HUB_CONFIG_KEY in _config(svc)["auths"] + + +def test_the_cli_config_is_not_readable_by_anyone_else(svc): + _add("ghcr.io") + path = os.path.join(svc.docker_config_dir(), "config.json") + assert stat.S_IMODE(os.stat(path).st_mode) == 0o600 + + +def test_removing_a_registry_revokes_the_cli_login_too(svc): + _add("ghcr.io") + assert "ghcr.io" in _config(svc)["auths"] + # clean_registries' wipe() is exactly what deleting the last row does. + from sqlmodel import Session, delete + + from database import engine + from models.registry import Registry + + with Session(engine) as session: + session.exec(delete(Registry)) + session.commit() + svc.reload(session) + assert _config(svc)["auths"] == {} + + +def test_compose_runs_against_our_config_not_the_home_directory(svc): + assert svc.cli_env()["DOCKER_CONFIG"] == svc.docker_config_dir() + + +# --------------------------------------------------------------------------- # +# Through the API +# --------------------------------------------------------------------------- # + + +def test_create_list_and_delete(as_admin): + created = as_admin.post( + "/api/registries", + json={"name": "GHCR", "host": "https://ghcr.io/", "username": "bob", "password": "hunter2"}, + ) + assert created.status_code == 201, created.text + body = created.json() + assert body["host"] == "ghcr.io" # normalized on the way in + assert body["has_password"] is True + + rows = as_admin.get("/api/registries").json() + assert [r["host"] for r in rows] == ["ghcr.io"] + + assert as_admin.delete(f"/api/registries/{body['id']}").status_code == 200 + assert as_admin.get("/api/registries").json() == [] + + +def test_the_password_never_comes_back_out(as_admin): + as_admin.post( + "/api/registries", + json={"host": "ghcr.io", "username": "bob", "password": "hunter2"}, + ) + payload = as_admin.get("/api/registries").text + assert "hunter2" not in payload + # No password field at all — only the has_password flag the UI needs to + # decide whether to show the input as "leave blank to keep". + assert '"password"' not in payload + assert '"has_password":true' in payload.replace(" ", "") + + +def test_the_password_is_encrypted_at_rest(as_admin): + from sqlmodel import Session, select + + from database import engine + from models.registry import Registry + from services import crypto_service + + as_admin.post( + "/api/registries", + json={"host": "ghcr.io", "username": "bob", "password": "hunter2"}, + ) + with Session(engine) as session: + row = session.exec(select(Registry)).one() + assert "hunter2" not in row.password + assert crypto_service.is_encrypted(row.password) + assert crypto_service.decrypt(row.password) == "hunter2" + + +def test_two_rows_for_the_same_registry_are_refused(as_admin): + first = {"host": "ghcr.io", "username": "bob", "password": "x"} + assert as_admin.post("/api/registries", json=first).status_code == 201 + # The same registry under a different spelling is still the same registry. + second = {"host": "https://ghcr.io", "username": "alice", "password": "y"} + assert as_admin.post("/api/registries", json=second).status_code == 409 + + +def test_editing_without_a_password_keeps_the_stored_one(as_admin, svc): + created = as_admin.post( + "/api/registries", + json={"host": "ghcr.io", "username": "bob", "password": "hunter2"}, + ).json() + updated = as_admin.put( + f"/api/registries/{created['id']}", json={"username": "alice"} + ) + assert updated.status_code == 200, updated.text + # The UI never received the password, so it cannot send it back — and must + # not have to. + assert svc.credentials_for("ghcr.io") == ("alice", "hunter2") + + +def test_a_saved_password_is_replaced_when_one_is_given(as_admin, svc): + created = as_admin.post( + "/api/registries", + json={"host": "ghcr.io", "username": "bob", "password": "hunter2"}, + ).json() + as_admin.put(f"/api/registries/{created['id']}", json={"password": "correcthorse"}) + assert svc.credentials_for("ghcr.io") == ("bob", "correcthorse") + + +def test_a_nonsense_host_is_a_400(as_admin): + response = as_admin.post( + "/api/registries", json={"host": " ", "username": "b", "password": "p"} + ) + assert response.status_code == 400 + + +def test_the_read_only_role_cannot_see_or_touch_registries(as_user): + assert as_user.get("/api/registries").status_code == 403 + assert ( + as_user.post( + "/api/registries", json={"host": "ghcr.io", "username": "b", "password": "p"} + ).status_code + == 403 + ) + + +# --------------------------------------------------------------------------- # +# The bug this was really about +# --------------------------------------------------------------------------- # + + +def _refusing(monkeypatch): + """Point check_image at a registry that answers 401.""" + from services import update_service + + async def refuse(image: str): + raise update_service.AuthRequired("ghcr.io") + + monkeypatch.setattr(update_service, "remote_digest", refuse) + monkeypatch.setattr(update_service, "_local_digest", lambda image: "sha256:local") + return update_service + + +def test_a_registry_that_refuses_us_is_reported_not_swallowed(monkeypatch): + """The whole point: 401 must not look like "up to date".""" + import asyncio + + update_service = _refusing(monkeypatch) + result = asyncio.run(update_service.check_image("ghcr.io/acme/private:1.0")) + + assert result.update_available is False + # Names the registry, because the fix is "add credentials for that one". + assert "ghcr.io" in result.error + assert "needs credentials" in result.error + + +def test_stored_credentials_that_are_rejected_say_so(monkeypatch): + import asyncio + + _add("ghcr.io", "bob", "hunter2") + update_service = _refusing(monkeypatch) + result = asyncio.run(update_service.check_image("ghcr.io/acme/private:1.0")) + + # Different wording, because the fix is different: the credentials are there + # and wrong, rather than missing. + assert "rejected" in result.error + + +def test_a_registry_we_simply_cannot_reach_still_says_that(monkeypatch): + """The old message has to survive — not every failure is an auth failure.""" + import asyncio + + from services import update_service + + async def unreachable(image: str): + return None + + monkeypatch.setattr(update_service, "remote_digest", unreachable) + monkeypatch.setattr(update_service, "_local_digest", lambda image: "sha256:local") + result = asyncio.run(update_service.check_image("ghcr.io/acme/app:1.0")) + assert result.error == "could not reach registry" diff --git a/backend/version.py b/backend/version.py index 4ff25f9..e185534 100644 --- a/backend/version.py +++ b/backend/version.py @@ -1,3 +1,3 @@ """Single source of truth for the StackPilot release version.""" -APP_VERSION = "0.55.0" +APP_VERSION = "0.56.0" diff --git a/frontend/package.json b/frontend/package.json index a00cfe5..5c19988 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "stackpilot-frontend", "private": true, - "version": "0.55.0", + "version": "0.56.0", "type": "module", "scripts": { "dev": "vite", diff --git a/frontend/src/api/registries.ts b/frontend/src/api/registries.ts new file mode 100644 index 0000000..dbf6837 --- /dev/null +++ b/frontend/src/api/registries.ts @@ -0,0 +1,36 @@ +import api from "./client"; + +const base = "/api/registries"; + +export interface RegistryCredentials { + id: number; + name: string; + /** Canonical host, as the server normalized it (e.g. registry-1.docker.io). */ + host: string; + username: string; + /** The password itself is never sent to the browser — only whether one is set. */ + has_password: boolean; + created_at: string; + updated_at: string; +} + +export interface RegistryInput { + name?: string; + host?: string; + username?: string; + /** Omit when editing to keep the stored password. */ + password?: string; +} + +export const registriesApi = { + list: () => api.get(base).then((r) => r.data), + create: (body: RegistryInput) => + api.post(base, body).then((r) => r.data), + update: (id: number, body: RegistryInput) => + api.put(`${base}/${id}`, body).then((r) => r.data), + remove: (id: number) => api.delete(`${base}/${id}`).then((r) => r.data), + /** Try credentials against the registry. Without a password, the stored one + * is used — which is how a saved row can be re-tested. */ + test: (body: { host: string; username: string; password?: string }) => + api.post<{ ok: boolean; host: string }>(`${base}/test`, body).then((r) => r.data), +}; diff --git a/frontend/src/pages/Settings.tsx b/frontend/src/pages/Settings.tsx index 17643f4..3b6cea9 100644 --- a/frontend/src/pages/Settings.tsx +++ b/frontend/src/pages/Settings.tsx @@ -13,6 +13,7 @@ import { HardDrive, CalendarClock, Play, + KeyRound, } from "lucide-react"; import { toast } from "sonner"; import { Badge, Button, Card, Input, Spinner } from "@/components/ui"; @@ -23,6 +24,11 @@ import { type WebhookInput, } from "@/api/settings"; import { destinationsApi, type BackupDestination } from "@/api/backups"; +import { + registriesApi, + type RegistryCredentials, + type RegistryInput, +} from "@/api/registries"; import { schedulesApi, type BackupSchedule } from "@/api/schedules"; import { stacksApi } from "@/api/stacks"; import { apiErrorMessage } from "@/api/client"; @@ -48,6 +54,7 @@ export function Settings() { return (
+ @@ -318,6 +325,243 @@ const FIELDS: Record { + qc.invalidateQueries({ queryKey: ["registries"] }); + // Update availability is computed from these, so a new login can change + // what the Images page has to say. + qc.invalidateQueries({ queryKey: ["images"] }); + qc.invalidateQueries({ queryKey: ["stack-updates"] }); + }; + + return ( +
+ }>Private registries +
+ {isLoading ? ( + + ) : ( + data?.map((r) => ) + )} + {data?.length === 0 && !adding && ( + +

+ No registry logins. Add one for Docker Hub, ghcr.io or your own + registry so StackPilot can pull private images — and so update + checks stop reporting “needs credentials” for them. +

+
+ )} + {adding ? ( + { + setAdding(false); + invalidate(); + }} + onCancel={() => setAdding(false)} + /> + ) : ( + + )} +
+
+ ); +} + +function RegistryRow({ + registry, + onChange, +}: { + registry: RegistryCredentials; + onChange: () => void; +}) { + const [editing, setEditing] = useState(false); + const test = useMutation({ + // No password: the server falls back to the stored one, which the browser + // has never seen. + mutationFn: () => registriesApi.test({ host: registry.host, username: registry.username }), + onSuccess: () => toast.success(`${registry.host} accepted the credentials`), + onError: (e) => toast.error(apiErrorMessage(e)), + }); + const remove = useMutation({ + mutationFn: () => registriesApi.remove(registry.id), + onSuccess: () => { + toast.success("Registry removed"); + onChange(); + }, + onError: (e) => toast.error(apiErrorMessage(e)), + }); + + if (editing) { + return ( + { + setEditing(false); + onChange(); + }} + onCancel={() => setEditing(false)} + /> + ); + } + + return ( + +
+
+ {registry.name} + {registry.host} +
+

+ {registry.username} · password stored +

+
+
+ + + +
+
+ ); +} + +function RegistryForm({ + registry, + onDone, + onCancel, +}: { + registry?: RegistryCredentials; + onDone: () => void; + onCancel: () => void; +}) { + const editing = Boolean(registry); + const [name, setName] = useState(registry?.name ?? ""); + const [host, setHost] = useState(registry?.host ?? ""); + const [username, setUsername] = useState(registry?.username ?? ""); + const [password, setPassword] = useState(""); + const [testing, setTesting] = useState(false); + + const body = (): RegistryInput => ({ + name: name.trim() || undefined, + host: host.trim(), + username: username.trim(), + // Editing with the field left blank keeps whatever is stored. + password: password || undefined, + }); + + const save = useMutation({ + mutationFn: () => + registry ? registriesApi.update(registry.id, body()) : registriesApi.create(body()), + onSuccess: () => { + toast.success(editing ? "Registry updated" : "Registry added"); + onDone(); + }, + onError: (e) => toast.error(apiErrorMessage(e)), + }); + + const test = async () => { + setTesting(true); + try { + await registriesApi.test({ + host: host.trim(), + username: username.trim(), + password: password || undefined, + }); + toast.success("The registry accepted these credentials"); + } catch (e) { + toast.error(apiErrorMessage(e)); + } finally { + setTesting(false); + } + }; + + const ready = host.trim() && username.trim() && (editing || password); + + return ( + +
+ + + + +
+
+ + + +
+
+ ); +} + +/* -------------------------------------------------------------------------- */ +/* Backup destinations */ +/* -------------------------------------------------------------------------- */ + function DestinationsSection() { const qc = useQueryClient(); const { data, isLoading } = useQuery({ queryKey: ["destinations"], queryFn: destinationsApi.list });