Security: harden credentials (drop --password, 0700 dirs, atomic 0600 writes, redact bodies, bump cryptography)

pyproject.toml

@@ -25,6 +25,10 @@ dependencies = [
     # Scalable's bot detection serves a captcha-free form (plain requests gets
     # fingerprinted and walled). auth.py falls back to requests if it's absent.
     "curl_cffi>=0.7",
+    # Pin the transitive `cryptography` floor above CVE-2024-12797 (affects
+    # 42.0.0–44.0.0; RFC7250 raw-public-key TLS handshake). Not exploitable
+    # in our standard-TLS usage, but keep the floor clean on venv refresh.
+    "cryptography>=44.0.1",
 ]
 
 [project.optional-dependencies]

src/sc_api/auth.py

@@ -87,6 +87,35 @@ def on_push_pending(mfa_session_id):
 PUSH_POLL_TIMEOUT_SEC = 120.0
 
 
+# ---------------------------------------------------------------------------
+# Secret scrubbing
+# ---------------------------------------------------------------------------
+# Auth0 login HTML / redirect URLs carry a `state` token (and sometimes
+# code/nonce/token params) that can be replayed. Scrub them before a body
+# ever lands in an exception message, a log line, or a debug dump on disk.
+_SENSITIVE_KEYS = "state|code|nonce|token|access_token|id_token|refresh_token|password|session_token"
+_QS_SECRET_RE = re.compile(
+    rf'((?:{_SENSITIVE_KEYS})=)[^&"\'\s<>]+', re.IGNORECASE,
+)
+_FORM_SECRET_RE = re.compile(
+    rf'(name="(?:{_SENSITIVE_KEYS})"[^>]*\bvalue=")[^"]*(")', re.IGNORECASE,
+)
+
+
+def _scrub_sensitive(text: str) -> str:
+    """Redact state/token-style secrets from arbitrary text or HTML."""
+    if not text:
+        return text
+    text = _QS_SECRET_RE.sub(r"\1[REDACTED]", text)
+    text = _FORM_SECRET_RE.sub(r"\1[REDACTED]\2", text)
+    return text
+
+
+def _redact_body(text: str, limit: int = 200) -> str:
+    """Truncate a response body and scrub secrets before it goes in an error."""
+    return _scrub_sensitive((text or "")[:limit])
+
+
 # ---------------------------------------------------------------------------
 # Exceptions
 # ---------------------------------------------------------------------------
@@ -231,13 +260,13 @@ def _post_auth_graphql(
     )
     if r.status_code >= 500:
         raise LoginError(
-            f"/auth/graphql {operation_name} → {r.status_code}: {r.text[:200]}"
+            f"/auth/graphql {operation_name} → {r.status_code}: {_redact_body(r.text)}"
         )
     try:
         payload = r.json()
     except ValueError as e:
         raise LoginError(
-            f"/auth/graphql {operation_name} returned non-JSON: {r.text[:200]}"
+            f"/auth/graphql {operation_name} returned non-JSON: {_redact_body(r.text)}"
         ) from e
     errs = payload.get("errors")
     if errs:
@@ -273,7 +302,7 @@ def _do_auth0_password_login(
     r = session.get(LOGIN_START, allow_redirects=True, timeout=20)
     if r.status_code != 200:
         raise LoginError(
-            f"Initial GET {LOGIN_START} returned {r.status_code}: {r.text[:200]}"
+            f"Initial GET {LOGIN_START} returned {r.status_code}: {_redact_body(r.text)}"
         )
     if "secure.scalable.capital/u/login" not in r.url:
         raise LoginError(
@@ -308,7 +337,7 @@ def _do_auth0_password_login(
         )
     if r2.status_code >= 400:
         raise LoginError(
-            f"POST /u/login → {r2.status_code}: {r2.text[:200]}"
+            f"POST /u/login → {r2.status_code}: {_redact_body(r2.text)}"
         )
 
     # 4. Should be on /auth/mfa-check or /cockpit/ now. Both are OK.
@@ -462,34 +491,57 @@ def _looks_like_nanoid(s: str) -> bool:
 def _dump_failed_extraction(candidates: list[tuple[str, str]], jar) -> str:
     """Save HTML + cookie names of the failed login attempt for offline debugging.
 
-    Returns the directory path so the user can share it. NOT written to a
-    fixed location — picks a place under the sc-api repo's discovery/ dir
-    if we can find it, otherwise /tmp.
+    Returns the directory path so the user can share it. Never written to a
+    world-readable, predictable /tmp path: prefers the sc-api repo's
+    discovery/ dir (dev checkout), else a private `~/.sc-api/discovery/`,
+    else a `tempfile.mkdtemp()` dir (created 0700). The dump dir itself is
+    created mode 0700, and the saved HTML has state/token params scrubbed —
+    the login flow HTML can otherwise carry a replayable `state` token.
     """
     import os
+    import tempfile
     import time
     from pathlib import Path
 
     base = None
-    # Try to write inside the package's discovery dir if we can locate it.
+    # Try to write inside the package's discovery dir if we can locate it
+    # (dev checkout only — the file lives under src/sc_api/ in a repo).
     try:
         pkg_dir = Path(__file__).resolve().parent.parent.parent  # repo root
         candidate = pkg_dir / "discovery"
-        if candidate.is_dir() or pkg_dir.is_dir():
-            candidate.mkdir(exist_ok=True)
+        if (candidate.is_dir() or (pkg_dir / "src").is_dir()):
+            candidate.mkdir(mode=0o700, exist_ok=True)
             base = candidate
     except Exception:
         pass
+    # Otherwise a private per-user dir under ~/.sc-api/ (never /tmp with a
+    # guessable name under the default umask).
     if base is None:
-        base = Path("/tmp")
+        try:
+            candidate = Path.home() / ".sc-api" / "discovery"
+            candidate.mkdir(mode=0o700, parents=True, exist_ok=True)
+            base = candidate
+        except Exception:
+            base = None
 
     stamp = time.strftime("%Y%m%d-%H%M%S")
-    dump_dir = base / f"login-debug-{stamp}"
-    dump_dir.mkdir(parents=True, exist_ok=True)
+    if base is None:
+        # Last resort: mkdtemp() creates a unique 0700 dir, no name to guess.
+        dump_dir = Path(tempfile.mkdtemp(prefix=f"sc-api-login-debug-{stamp}-"))
+    else:
+        dump_dir = base / f"login-debug-{stamp}"
+        dump_dir.mkdir(mode=0o700, parents=True, exist_ok=True)
+    # mkdir()'s mode is masked by the umask; force 0700 explicitly.
+    try:
+        os.chmod(dump_dir, 0o700)
+    except OSError:
+        pass
 
     for i, (url, html_text) in enumerate(candidates):
         safe = re.sub(r"[^a-zA-Z0-9]+", "_", url)[:80]
-        (dump_dir / f"{i:02d}_{safe}.html").write_text(html_text, encoding="utf-8")
+        (dump_dir / f"{i:02d}_{safe}.html").write_text(
+            _scrub_sensitive(html_text), encoding="utf-8",
+        )
 
     # Cookie names only (no values — secrets)
     cookie_summary = sorted({

src/sc_api/cli.py

@@ -82,8 +82,10 @@ def _build_parser() -> argparse.ArgumentParser:
     al = auth_sub.add_parser("login",
                               help="Programmatic login: email+password+push (PRIMARY mode)")
     al.add_argument("--email", required=True, help="Scalable login email")
-    al.add_argument("--password", default=None,
-                    help="Password (prompted if omitted — recommended)")
+    # No --password flag on purpose: a password on argv leaks via `ps aux`,
+    # /proc, and shell history (a real disclosure path on the shared server).
+    # The password comes from the SC_PASSWORD env var or an interactive
+    # getpass prompt instead. See _cmd_auth_login.
     al.add_argument("--name", default=None, help="Friendly profile name (optional)")
     al.add_argument("--set-active", action="store_true",
                     help="Also mark this profile as the active one")
@@ -205,9 +207,13 @@ def _cmd_auth_login(args: argparse.Namespace) -> int:
     push approval per fresh session, cookies persist for hours.
     """
     import getpass
+    import os
     import sys
 
-    password = args.password
+    # Password source, in order: SC_PASSWORD env var → interactive prompt.
+    # Never a CLI flag (would leak via `ps`/history). For unattended
+    # auto-relogin, credentials.json (mode 0600) is consumed elsewhere.
+    password = os.environ.get("SC_PASSWORD")
     if not password:
         password = getpass.getpass("Scalable password: ")
 

src/sc_api/cookies.py

@@ -18,6 +18,8 @@
 from __future__ import annotations
 
 import json
+import os
+import tempfile
 import time
 import urllib.parse
 from http.cookiejar import Cookie, MozillaCookieJar
@@ -143,6 +145,36 @@ def parse_session_cookie(value: str) -> str:
     return user_id
 
 
+# ---------------------------------------------------------------------------
+# Atomic 0600 persistence
+# ---------------------------------------------------------------------------
+def _atomic_save_jar(jar: MozillaCookieJar, path: Path) -> None:
+    """Save a cookie jar to `path` atomically as mode 0600 (no umask race).
+
+    Writes to a temp file in the same dir (created 0600 by mkstemp), then
+    os.replace()s it into place. A chmod-after-write leaves the file briefly
+    world-readable under a permissive umask; this doesn't. The parent dir is
+    created 0700 since cookies are session secrets.
+    """
+    path.parent.mkdir(mode=0o700, parents=True, exist_ok=True)
+    try:
+        path.parent.chmod(0o700)
+    except OSError:
+        pass
+    fd, tmp = tempfile.mkstemp(dir=str(path.parent), prefix=".cookies-", suffix=".txt")
+    os.close(fd)
+    try:
+        jar.save(tmp, ignore_discard=True, ignore_expires=True)
+        os.chmod(tmp, 0o600)
+        os.replace(tmp, str(path))
+    except BaseException:
+        try:
+            os.unlink(tmp)
+        except OSError:
+            pass
+        raise
+
+
 # ---------------------------------------------------------------------------
 # Save / load (Mozilla cookie jar format — compatible with `requests`)
 # ---------------------------------------------------------------------------
@@ -156,7 +188,6 @@ def save_to_file(cookies: dict[str, str], path: Path | str) -> int:
     Returns the number of cookies written.
     """
     path = Path(path)
-    path.parent.mkdir(parents=True, exist_ok=True)
 
     jar = MozillaCookieJar(str(path))
     # Use a long-future expiry — the real expiry is enforced server-side
@@ -186,9 +217,8 @@ def save_to_file(cookies: dict[str, str], path: Path | str) -> int:
             )
         )
 
-    jar.save(ignore_discard=True, ignore_expires=True)
-    # Restrict permissions: cookies are session secrets.
-    path.chmod(0o600)
+    # Atomic 0600 write (see _atomic_save_jar): cookies are session secrets.
+    _atomic_save_jar(jar, path)
     return len(jar)
 
 
@@ -214,7 +244,6 @@ def save_jar_to_file(jar, path: Path | str) -> int:
     `de.scalable.capital` domain. That's a 400-error trap.
     """
     path = Path(path)
-    path.parent.mkdir(parents=True, exist_ok=True)
 
     out = MozillaCookieJar(str(path))
     count = 0
@@ -225,8 +254,8 @@ def save_jar_to_file(jar, path: Path | str) -> int:
         out.set_cookie(c)
         count += 1
 
-    out.save(ignore_discard=True, ignore_expires=True)
-    path.chmod(0o600)
+    # Atomic 0600 write (see _atomic_save_jar): cookies are session secrets.
+    _atomic_save_jar(out, path)
     return count
 
 

src/sc_api/profiles.py

@@ -20,8 +20,10 @@
 from __future__ import annotations
 
 import json
+import os
 import re
 import shutil
+import tempfile
 from dataclasses import asdict, dataclass, field
 from datetime import datetime, timezone
 from pathlib import Path
@@ -90,7 +92,46 @@ def _validate_email(email: str) -> str:
 
 
 def _ensure_dirs() -> None:
-    PROFILES_DIR.mkdir(parents=True, exist_ok=True)
+    # 0700: these dirs hold cookies + credentials (secrets). mkdir()'s mode
+    # is masked by the umask, so chmod explicitly after creating.
+    BASE_DIR.mkdir(mode=0o700, exist_ok=True)
+    PROFILES_DIR.mkdir(mode=0o700, exist_ok=True)
+    for d in (BASE_DIR, PROFILES_DIR):
+        try:
+            d.chmod(0o700)
+        except OSError:
+            pass
+
+
+def _mkdir_private(path: Path) -> None:
+    """Create a directory (and parents) restricted to the owner (0700)."""
+    path.mkdir(mode=0o700, parents=True, exist_ok=True)
+    try:
+        path.chmod(0o700)
+    except OSError:
+        pass
+
+
+def _atomic_write(path: Path, data: str, mode: int = 0o600) -> None:
+    """Write `data` to `path` atomically with `mode`, no umask race.
+
+    Writes to a temp file in the same dir (mkstemp → 0600), sets the final
+    mode, then os.replace()s it into place. A crash mid-write never leaves a
+    truncated or world-readable file at the target path.
+    """
+    _mkdir_private(path.parent)
+    fd, tmp = tempfile.mkstemp(dir=str(path.parent), prefix=".tmp-", suffix=path.suffix)
+    try:
+        with os.fdopen(fd, "w", encoding="utf-8") as fh:
+            fh.write(data)
+        os.chmod(tmp, mode)
+        os.replace(tmp, str(path))
+    except BaseException:
+        try:
+            os.unlink(tmp)
+        except OSError:
+            pass
+        raise
 
 
 def _now_iso() -> str:
@@ -114,7 +155,7 @@ def create(email: str, name: str | None = None) -> Profile:
         return load(email)
 
     prof = Profile(email=email, name=name, created_at=_now_iso(), updated_at=_now_iso())
-    prof.dir.mkdir(parents=True, exist_ok=True)
+    _mkdir_private(prof.dir)
     _write_meta(prof)
     return prof
 
@@ -187,9 +228,10 @@ def update_identity(
 
 
 def _write_meta(profile: Profile) -> None:
-    profile.meta_file.write_text(
+    _atomic_write(
+        profile.meta_file,
         json.dumps(asdict(profile), indent=2, ensure_ascii=False) + "\n",
-        encoding="utf-8",
+        mode=0o600,
     )
 
 
@@ -208,12 +250,16 @@ def save_credentials(profile: Profile, email: str, password: str) -> None:
     For multi-user ownCloud, this file is NOT used — credentials live in
     `oc_preferences` encrypted with `ICrypto` instead.
     """
-    profile.dir.mkdir(parents=True, exist_ok=True)
+    _mkdir_private(profile.dir)
     payload = {"email": email, "password": password}
-    profile.credentials_file.write_text(
-        json.dumps(payload, indent=2) + "\n", encoding="utf-8",
+    # Atomic 0600 write: the temp file is created 0600 and renamed into
+    # place, so the plaintext password is never briefly world-readable
+    # (which a chmod-after-write would allow under a permissive umask).
+    _atomic_write(
+        profile.credentials_file,
+        json.dumps(payload, indent=2) + "\n",
+        mode=0o600,
     )
-    profile.credentials_file.chmod(0o600)
 
 
 def load_credentials(profile: Profile) -> dict | None: