feat(auth): reuse existing docker login (OAT) instead of requiring credential.json

docs/configuration.md

@@ -697,6 +697,42 @@ export MAD_DOCKERHUB_PASSWORD=mytoken
 export MAD_DOCKERHUB_REPO=myorg
 ```
 
+### Registry Authentication
+
+madengine reuses an existing `docker login` — including an organization access
+token (OAT) — rather than requiring credentials to be duplicated into
+`credential.json`. Ambient credentials are read from
+`${DOCKER_CONFIG:-~/.docker}/config.json` exactly as the Docker CLI reads them,
+covering `auths` entries, `credHelpers`, and `credsStore`.
+
+Blank values are treated as **not configured**, not as credentials. A placeholder
+entry such as `{"username": "", "password": ""}` will never override or break a
+working `docker login`.
+
+| Existing `docker login` | Credentials in `credential.json` / env | Behavior |
+|---|---|---|
+| no  | yes (non-blank) | `docker login` with the configured credentials |
+| yes | yes (non-blank) | `docker login` with the configured credentials (explicit wins) |
+| yes | absent or blank | Reuse the existing login; no `docker login` is run |
+| no  | absent or blank | Push fails with an actionable error; pull warns and continues |
+
+Before `docker build`, madengine logs in to the base image's registry only when
+that registry has no existing login and usable credentials are configured, so a
+node authenticated with an OAT is never re-authenticated.
+
+Relevant environment variables:
+
+- `DOCKER_CONFIG` — directory holding `config.json` (default `~/.docker`)
+- `MAD_SKIP_DOCKER_LOGIN=1` — never run `docker login`; always defer to the
+  credentials the machine already has
+
+If a base image pull is denied, madengine distinguishes the two causes:
+credentials were rejected (authentication — supply credentials or run
+`docker login`), versus credentials were accepted but the registry granted no
+pull scope for that repository (`insufficient_scope` — an authorization problem,
+where the access token needs to be scoped to the repository and re-running
+`docker login` will not help).
+
 ## Configuration Priority
 
 For Kubernetes/SLURM deployments:

src/madengine/core/auth.py

@@ -11,14 +11,28 @@
 import json
 import os
 import shlex
-from typing import Dict, Optional
+from pathlib import Path
+from typing import Dict, Optional, Tuple
 
 from madengine.core.errors import (
     ConfigurationError,
     create_error_context,
     handle_error,
 )
 
+# Keys under which the Docker CLI stores Docker Hub credentials in config.json.
+# Docker has used several spellings over the years and any of them means
+# "this machine is authenticated to Docker Hub".
+_DOCKERHUB_CONFIG_KEYS: Tuple[str, ...] = (
+    "https://index.docker.io/v1/",
+    "index.docker.io",
+    "registry-1.docker.io",
+    "docker.io",
+)
+
+# Registry values that madengine treats as "Docker Hub" rather than a host.
+_DOCKERHUB_ALIASES = ("docker.io", "dockerhub")
+
 
 def load_credentials() -> Optional[Dict]:
     """Load credentials from credential.json and environment variables.
@@ -81,6 +95,174 @@ def load_credentials() -> Optional[Dict]:
     return credentials
 
 
+def _registry_config_keys(registry: Optional[str]) -> Tuple[str, ...]:
+    """Map a madengine registry value to the keys Docker uses in config.json.
+
+    Args:
+        registry: Registry URL (e.g. ``"localhost:5000"``, ``"docker.io/rocm"``),
+            or ``None``/empty string for Docker Hub.
+
+    Returns:
+        The config.json ``auths`` keys that would hold credentials for it.
+    """
+    if not registry or registry.lower() in _DOCKERHUB_ALIASES:
+        return _DOCKERHUB_CONFIG_KEYS
+    # Downstream code derives the registry host the same way (docker login <host>).
+    host = registry.split("/")[0]
+    if host.lower() in _DOCKERHUB_ALIASES:
+        return _DOCKERHUB_CONFIG_KEYS
+    return (host,)
+
+
+def has_ambient_docker_auth(registry: Optional[str]) -> bool:
+    """Report whether the local Docker CLI is already authenticated to ``registry``.
+
+    Reads ``${DOCKER_CONFIG:-~/.docker}/config.json`` the same way the Docker CLI
+    does, so an existing ``docker login`` (e.g. an organisation access token) is
+    honoured instead of being overridden or reported as "no credentials".
+
+    Args:
+        registry: Registry URL, or ``None``/empty string for Docker Hub.
+
+    Returns:
+        ``True`` if a usable credential entry exists for the registry. Any read
+        or parse problem yields ``False``; this function never raises and never
+        logs credential material.
+    """
+    config_dir = os.environ.get("DOCKER_CONFIG") or os.path.join(
+        os.path.expanduser("~"), ".docker"
+    )
+    try:
+        config = json.loads(Path(config_dir, "config.json").read_text(encoding="utf-8"))
+    except (OSError, ValueError):
+        return False
+    if not isinstance(config, dict):
+        return False
+
+    keys = _registry_config_keys(registry)
+    auths = config.get("auths") or {}
+    cred_helpers = config.get("credHelpers") or {}
+    creds_store = config.get("credsStore")
+
+    if isinstance(cred_helpers, dict) and any(key in cred_helpers for key in keys):
+        return True
+    if not isinstance(auths, dict):
+        return False
+    for key in keys:
+        entry = auths.get(key)
+        if not isinstance(entry, dict):
+            continue
+        if entry.get("auth") or entry.get("identitytoken") or entry.get("username"):
+            return True
+        # Credential-store-managed entries are persisted as an empty object;
+        # the secret itself lives in the external store.
+        if creds_store:
+            return True
+    return False
+
+
+def _usable_credentials(creds: object) -> bool:
+    """Report whether a credential entry carries a non-blank username and password.
+
+    Placeholder entries such as ``{"username": "", "password": ""}`` are treated
+    as "not configured" rather than as credentials, matching
+    :func:`madengine.deployment.k8s_secrets.build_registry_secret_data`.
+    """
+    if not isinstance(creds, dict):
+        return False
+    return bool(str(creds.get("username") or "").strip()) and bool(
+        str(creds.get("password") or "").strip()
+    )
+
+
+def _registry_from_image(image: Optional[str]) -> Optional[str]:
+    """Extract the registry host from an image reference.
+
+    Args:
+        image: Image reference (e.g. ``"ghcr.io/org/app:tag"``), or ``None``.
+
+    Returns:
+        The registry host, or ``None`` when the reference targets Docker Hub,
+        is malformed, or was not supplied.
+    """
+    if not image or not image.strip():
+        return None
+    ref = image.strip()
+    if "/" not in ref:
+        return None
+    # Docker treats the first component as a registry only when it looks like a
+    # host; otherwise it is a Docker Hub namespace (e.g. "rocm/private").
+    host = ref.split("/")[0]
+    if not ("." in host or ":" in host or host == "localhost"):
+        return None
+    if host.lower() in _DOCKERHUB_ALIASES:
+        return None
+    return host
+
+
+def explain_registry_denial(
+    log_text: str, image: Optional[str] = None
+) -> Optional[str]:
+    """Turn a registry denial in Docker output into an actionable explanation.
+
+    Distinguishes "authenticated but not authorized for this repository" from
+    "not authenticated at all", because the two need completely different fixes.
+
+    Args:
+        log_text: Docker build/pull output to inspect.
+        image: Optional image reference the denial refers to, for the message.
+
+    Returns:
+        A multi-line hint, or ``None`` if the output shows no registry denial.
+    """
+    lowered = (log_text or "").lower()
+    subject = image.strip() if image and image.strip() else "the base image"
+
+    if "insufficient_scope" in lowered or "authorization failed" in lowered:
+        return (
+            f"Base image pull was denied: {subject}\n"
+            "   The registry ACCEPTED the credentials but granted no pull scope "
+            "for this repository.\n"
+            "   This is an authorization problem, not a login problem:\n"
+            "     - the access token is not scoped to this repository, or\n"
+            "     - the repository/tag does not exist under that namespace.\n"
+            "   Re-running `docker login` will not fix it; widen the token's "
+            "repository scope instead."
+        )
+
+    denied = (
+        "pull access denied" in lowered
+        or "requested access to the resource is denied" in lowered
+        or "authentication required" in lowered
+        or "unauthorized" in lowered
+    )
+    if not denied:
+        return None
+
+    registry = _registry_from_image(image)
+    if registry is None:
+        fixes = (
+            "     - `docker login` on this machine (madengine reuses an existing "
+            "login)\n"
+            '     - add {"dockerhub": {"username": "...", "password": "..."}} to '
+            "credential.json\n"
+            "     - export MAD_DOCKERHUB_USER and MAD_DOCKERHUB_PASSWORD"
+        )
+    else:
+        fixes = (
+            f"     - `docker login {registry}` on this machine (madengine reuses "
+            "an existing login)\n"
+            f'     - add {{"{registry}": {{"username": "...", "password": "..."}}}} '
+            "to credential.json"
+        )
+
+    return (
+        f"Base image pull was denied: {subject}\n"
+        "   No usable credentials were presented to the registry.\n"
+        "   Fix with any one of:\n" + fixes
+    )
+
+
 def login_to_registry(
     registry: Optional[str],
     credentials: Optional[Dict],
@@ -103,10 +285,17 @@ def login_to_registry(
             failure (missing key, invalid format, or docker login error).
             Set to ``False`` to log and return instead, allowing the caller
             to fall back to pulling public images.
+
+    Precedence: explicit credentials (``credential.json`` / ``MAD_DOCKERHUB_*``)
+    win when they carry a non-blank username and password. Otherwise an existing
+    ``docker login`` on this machine is reused and no login is attempted, so a
+    placeholder credential entry never overrides or breaks working ambient auth.
+    Set ``MAD_SKIP_DOCKER_LOGIN=1`` to always defer to ambient credentials.
     """
-    if not credentials:
+    if os.environ.get("MAD_SKIP_DOCKER_LOGIN") == "1":
         rich_console.print(
-            "[yellow]No credentials provided for registry login[/yellow]"
+            "[yellow]MAD_SKIP_DOCKER_LOGIN=1 - using existing docker login for "
+            f"{registry or 'DockerHub'}[/yellow]"
         )
         return
 
@@ -116,8 +305,35 @@ def login_to_registry(
     if registry and registry.lower() == "docker.io":
         registry_key = "dockerhub"
 
-    if registry_key not in credentials:
-        error_msg = f"No credentials found for registry: {registry_key}"
+    entry = (credentials or {}).get(registry_key)
+    creds: Dict = entry if isinstance(entry, dict) else {}
+
+    if not _usable_credentials(creds):
+        # No explicit credentials configured for this registry. If the machine
+        # is already logged in (e.g. an organisation access token), reuse that
+        # instead of failing or clobbering it.
+        if has_ambient_docker_auth(registry):
+            rich_console.print(
+                f"[green]Using existing docker login for "
+                f"{registry or 'DockerHub'} (no explicit credentials "
+                f"configured)[/green]"
+            )
+            return
+
+        if not credentials:
+            rich_console.print(
+                "[yellow]No credentials provided for registry login[/yellow]"
+            )
+            return
+
+        if registry_key not in credentials:
+            error_msg = f"No credentials found for registry: {registry_key}"
+        else:
+            error_msg = (
+                f"Invalid credentials format for registry: {registry_key}"
+                f"\nCredentials must contain non-empty 'username' and "
+                f"'password' fields"
+            )
         if registry_key == "dockerhub":
             error_msg += (
                 f"\nPlease add dockerhub credentials to credential.json:\n"
@@ -140,18 +356,7 @@ def login_to_registry(
                 "  }\n"
                 "}"
             )
-        rich_console.print(f"[red]{error_msg}[/red]")
-        if raise_on_failure:
-            raise RuntimeError(error_msg)
-        return
-
-    creds = credentials[registry_key]
-
-    if "username" not in creds or "password" not in creds:
-        error_msg = (
-            f"Invalid credentials format for registry: {registry_key}"
-            f"\nCredentials must contain 'username' and 'password' fields"
-        )
+        error_msg += "\nAlternatively, run `docker login` on this machine."
         rich_console.print(f"[red]{error_msg}[/red]")
         if raise_on_failure:
             raise RuntimeError(error_msg)

src/madengine/core/console.py

@@ -229,6 +229,12 @@ def sh(
         # Check for failure
         success = proc.returncode == 0
 
+        # When output is captured rather than streamed it is discarded on
+        # failure, and the RuntimeError below carries only the command and the
+        # exit code. Echo it so the log records why the command actually failed.
+        if not success and not canFail and not secret and not self.live_output and outs:
+            print(redact_secrets(outs), flush=True)
+
         # Show docker operation completion status
         if not secret:
             self._show_docker_completion(command, success)