From 75e64c4095a0c0f9b18f34854d9f5d5ebb8d4860 Mon Sep 17 00:00:00 2001 From: Carlos Damken Date: Wed, 8 Jul 2026 00:13:13 +0200 Subject: [PATCH] Fix #2: drop --password, 0700 dump/profile dirs, atomic 0600 writes, redact bodies, bump cryptography Security hardening from the read-only credential/CLI review: - M8: remove `--password` CLI flag (leaked via ps/proc/shell history); password now comes from SC_PASSWORD env var or interactive getpass only. - M9: failed-login HTML dump now uses ~/.sc-api/discovery (or mkdtemp), never a guessable /tmp path; dir created mode 0700; state/token params scrubbed from saved HTML. - L5/L6: profiles.py + cookies.py now write credentials/cookies/meta via atomic tmp->chmod 0600->rename (no umask race); profile/base dirs 0700. - L7: raw response bodies routed through _redact_body/_scrub_sensitive before landing in exception messages. - M10: pin cryptography>=44.0.1 (CVE-2024-12797). Co-Authored-By: Claude Opus 4.8 (1M context) --- pyproject.toml | 4 +++ src/sc_api/auth.py | 80 ++++++++++++++++++++++++++++++++++-------- src/sc_api/cli.py | 12 +++++-- src/sc_api/cookies.py | 43 +++++++++++++++++++---- src/sc_api/profiles.py | 62 +++++++++++++++++++++++++++----- 5 files changed, 169 insertions(+), 32 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 5c776ad..1c8a62c 100644 --- a/pyproject.toml +++ b/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] diff --git a/src/sc_api/auth.py b/src/sc_api/auth.py index bf6a0ff..a7cb933 100644 --- a/src/sc_api/auth.py +++ b/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({ diff --git a/src/sc_api/cli.py b/src/sc_api/cli.py index 7b94844..40b10d9 100644 --- a/src/sc_api/cli.py +++ b/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: ") diff --git a/src/sc_api/cookies.py b/src/sc_api/cookies.py index 89581a9..c4e3b02 100644 --- a/src/sc_api/cookies.py +++ b/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 diff --git a/src/sc_api/profiles.py b/src/sc_api/profiles.py index 306261d..7713149 100644 --- a/src/sc_api/profiles.py +++ b/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: