diff --git a/packages/tangle-cli/src/tangle_cli/api_cli.py b/packages/tangle-cli/src/tangle_cli/api_cli.py index 4a80863..dafad2e 100644 --- a/packages/tangle-cli/src/tangle_cli/api_cli.py +++ b/packages/tangle-cli/src/tangle_cli/api_cli.py @@ -70,7 +70,12 @@ default_auth_header, default_base_url, default_token, + describe_request_error, + format_http_status_error, + format_request_error, + http_status_line, request_operation, + sanitize_url, ) from .cli_helpers import api_arg_specs, load_args_or_exit from .cli_options import ( @@ -185,14 +190,11 @@ def refresh( include_env_credentials=not base_url_from_config, ) except httpx.HTTPStatusError as exc: - message = f"HTTP {exc.response.status_code} {exc.response.reason_phrase}" - raise SystemExit( - f"Failed to fetch {_openapi_url(normalized_base_url)}: {message}" - ) from exc + # Never echo the /openapi.json response body: an auth failure can + # reflect the credentials we just sent. Status line only. + raise SystemExit(_schema_fetch_error_message(normalized_base_url, exc)) from exc except httpx.RequestError as exc: - raise SystemExit( - f"Failed to fetch {_openapi_url(normalized_base_url)}: {exc}" - ) from exc + raise SystemExit(_schema_fetch_error_message(normalized_base_url, exc)) from exc path_count = len(schema.get("paths", {})) print(f"Cached OpenAPI schema for {normalized_base_url}") print(f"Path: {path}") @@ -452,11 +454,13 @@ def _invoke_operation_once( include_env_credentials=include_env_credentials, ) except httpx.HTTPStatusError as exc: - message = exc.response.text or exc.response.reason_phrase - print(message, file=sys.stderr) - raise SystemExit(exc.response.status_code) from exc + # One-line, credential-safe failure with a non-zero exit. The prior code + # raised the HTTP status as the exit code, but exit codes are 8-bit, so a + # status was truncated (404 -> 148, 500 -> 244) and multiples of 256 + # reported success. A string exit prints to stderr and exits 1. + raise SystemExit(format_http_status_error(exc)) from exc except httpx.RequestError as exc: - raise SystemExit(f"Failed to call {exc.request.url}: {exc}") from exc + raise SystemExit(format_request_error(exc)) from exc except TypeError as exc: raise SystemExit(str(exc)) from exc @@ -697,6 +701,23 @@ def _validate_schema_source(value: str) -> str: return normalized +def _schema_fetch_error_message(base_url: str, exc: Exception) -> str: + """One-line, credential-safe failure for an /openapi.json fetch. + + The response body is deliberately omitted for HTTP status errors so an auth + failure cannot reflect the credentials that were just sent. + """ + + target = sanitize_url(_openapi_url(base_url)) + if isinstance(exc, httpx.HTTPStatusError): + reason = http_status_line(exc) + elif isinstance(exc, httpx.RequestError): + reason = describe_request_error(exc) + else: + reason = exc.__class__.__name__ + return f"Failed to fetch {target}: {reason}" + + def _schema_fetch_failure_message(base_url: str, exc: Exception) -> str: if isinstance(exc, httpx.HTTPStatusError): reason = f"HTTP {exc.response.status_code} {exc.response.reason_phrase}" diff --git a/packages/tangle-cli/src/tangle_cli/api_transport.py b/packages/tangle-cli/src/tangle_cli/api_transport.py index b128070..c34e91f 100644 --- a/packages/tangle-cli/src/tangle_cli/api_transport.py +++ b/packages/tangle-cli/src/tangle_cli/api_transport.py @@ -18,9 +18,23 @@ _MISSING = object() _SENSITIVE_HEADER_NAMES = {"authorization", "cloud-auth", "cookie", "x-api-key"} _SENSITIVE_KEY_RE = re.compile( - r"(authorization|authentication|(^|[-_])auth($|[-_])|cloud[-_]?auth|cookie|x[-_]?api[-_]?key|token|secret|password|credential|pre[-_]?signed[-_]?url|signed[-_]?url)", + r"(authorization|authentication|(^|[-_])auth($|[-_])|cloud[-_]?auth|cookie|(x[-_]?)?api[-_]?key|token|secret|password|credential|pre[-_]?signed[-_]?url|signed[-_]?url)", re.IGNORECASE, ) +# Query parameters that carry the credential portion of a presigned/SAS URL +# (AWS SigV4, GCS, Azure). Redacting the signature neutralizes the grant. +_SIGNED_URL_QUERY_RE = re.compile( + r"^(x-(amz|goog|ms)-.*|sig|signature|awsaccesskeyid|googleaccessid)$", + re.IGNORECASE, +) +# Upper bound on backend-supplied error detail rendered on a single line. +_MAX_BACKEND_DETAIL_CHARS = 500 +# Any ``scheme://...`` run embedded in free-form exception text; each match is +# routed through sanitize_url so userinfo and signed query params are redacted. +_EMBEDDED_URL_RE = re.compile(r"[a-zA-Z][a-zA-Z0-9+.\-]*://[^\s'\"<>]+") +# Bare ``user:pass@host`` userinfo that appears without a scheme (e.g. proxy +# diagnostics). Requires a colon-bearing userinfo terminated by ``@``. +_BARE_USERINFO_RE = re.compile(r"[^\s/@:]+:[^\s/@]+@") _REDACTED = "" _REDACTED_DOCUMENT = "" _OPAQUE_DOCUMENT_KEY_NAMES = { @@ -90,6 +104,152 @@ def _content_to_text(content: bytes | str | None) -> str: return _safe_json_text(parsed) +def _is_sensitive_query_key(key: str) -> bool: + stripped = key.strip() + return bool(_SENSITIVE_KEY_RE.search(stripped) or _SIGNED_URL_QUERY_RE.match(stripped)) + + +def sanitize_url(url: Any) -> str: + """Return *url* with credentials removed so it is safe to display or log. + + Strips any ``user:password@`` userinfo and redacts the values of query + parameters that look like tokens, credentials, or presigned/SAS-URL + signatures. The scheme, host, port, path, and non-sensitive query keys are + preserved so the target stays recognizable. + """ + + text = str(url) + try: + parsed = urllib.parse.urlsplit(text) + except ValueError: + return _REDACTED + if not parsed.scheme and not parsed.netloc: + return text + host = parsed.hostname or "" + # ``hostname`` unwraps IPv6 literals, so re-bracket them before appending an + # optional port; otherwise ``[2001:db8::1]:8443`` becomes ambiguous garbage. + if ":" in host: + host = f"[{host}]" + netloc = f"{_REDACTED}@{host}" if (parsed.username or parsed.password) else host + if parsed.port is not None: + netloc = f"{netloc}:{parsed.port}" + query = parsed.query + if query: + pairs = urllib.parse.parse_qsl(query, keep_blank_values=True) + query = urllib.parse.urlencode( + [ + (key, _REDACTED if _is_sensitive_query_key(key) else value) + for key, value in pairs + ], + safe="<>", + ) + return urllib.parse.urlunsplit((parsed.scheme, netloc, parsed.path, query, parsed.fragment)) + + +def _bounded_detail(text: str | None) -> str: + """Collapse whitespace and cap length of backend-supplied error detail.""" + + if not text: + return "" + collapsed = " ".join(str(text).split()) + if len(collapsed) > _MAX_BACKEND_DETAIL_CHARS: + collapsed = collapsed[:_MAX_BACKEND_DETAIL_CHARS].rstrip() + "…" + return collapsed + + +def http_status_line(exc: httpx.HTTPStatusError) -> str: + """Return the ``HTTP `` summary for a status error.""" + + response = exc.response + return f"HTTP {response.status_code} {response.reason_phrase}".strip() + + +def format_http_status_error(exc: httpx.HTTPStatusError, *, include_detail: bool = True) -> str: + """Build a concise one-line message for an httpx HTTP status error. + + Includes the status, request method, and a credential-safe URL. When + *include_detail* is set, a whitespace-normalized and length-bounded slice of + the response body is appended so backend messages remain visible without + dumping multi-line or oversized payloads. + """ + + request = exc.request + method = request.method if request is not None else "?" + url = sanitize_url(request.url) if request is not None else "?" + message = f"{http_status_line(exc)} for {method} {url}" + if include_detail: + try: + body = exc.response.text + except Exception: # pragma: no cover - defensive: streamed/undecodable body + body = "" + detail = _bounded_detail(body) + if detail: + message = f"{message}: {detail}" + return message + + +def _scrub_secret_text(text: str) -> str: + """Redact URLs and bare userinfo embedded in free-form exception text. + + A crafted or third-party ``httpx`` exception can carry a proxy URL, signed + query, or ``user:pass@host`` inside its message. Never emit that raw: route + every ``scheme://`` run through :func:`sanitize_url` and strip any remaining + schemeless userinfo, while leaving benign diagnostics (errno, TLS reason) + intact. + """ + + def _replace_url(match: re.Match[str]) -> str: + raw = match.group(0) + trailing = "" + while raw and raw[-1] in ").,;'\"": + trailing = raw[-1] + trailing + raw = raw[:-1] + return sanitize_url(raw) + trailing + + scrubbed = _EMBEDDED_URL_RE.sub(_replace_url, text) + return _BARE_USERINFO_RE.sub(f"{_REDACTED}@", scrubbed) + + +def describe_request_error(exc: httpx.RequestError) -> str: + """Return an actionable, credential-safe reason for an httpx request error. + + Connection, timeout, proxy, and TLS failures are labeled so the user knows + what to check; the underlying detail is included when it adds information. + Any URL or userinfo embedded in the exception text is redacted first. + """ + + detail = _scrub_secret_text(" ".join(str(exc).split())) + lowered = detail.lower() + if isinstance(exc, httpx.ProxyError): + return f"proxy error: {detail}" if detail else "proxy error" + if isinstance(exc, httpx.TimeoutException): + label = { + httpx.ConnectTimeout: "connection timed out", + httpx.ReadTimeout: "read timed out", + httpx.WriteTimeout: "write timed out", + httpx.PoolTimeout: "connection pool timed out", + }.get(type(exc), "request timed out") + return f"{label}: {detail}" if detail and label not in lowered else label + if isinstance(exc, httpx.ConnectError): + if any(token in lowered for token in ("ssl", "certificate", "tls", "handshake")): + return f"TLS error: {detail}" if detail else "TLS error" + return f"connection failed: {detail}" if detail else "connection failed" + if detail: + return detail + return exc.__class__.__name__ + + +def format_request_error(exc: httpx.RequestError) -> str: + """Build a concise one-line message for an httpx connection-level error.""" + + request = getattr(exc, "request", None) + reason = describe_request_error(exc) + if request is None: + return f"Failed to reach the backend: {reason}" + url = sanitize_url(request.url) + return f"Failed to reach {request.method} {url}: {reason}" + + def log_http_exchange( logger: Any, *, diff --git a/tests/test_api_cli.py b/tests/test_api_cli.py index c93a575..630ec23 100644 --- a/tests/test_api_cli.py +++ b/tests/test_api_cli.py @@ -1681,6 +1681,37 @@ def fake_get(url, **kwargs): assert "secret-token" not in message +def test_refresh_connection_error_is_clean_and_actionable(monkeypatch): + def fake_get(url, **kwargs): + raise httpx.ConnectError("connection refused", request=httpx.Request("GET", url)) + + monkeypatch.setattr(api_cli.httpx, "get", fake_get) + app = api_cli.build_app(SCHEMA) + + with pytest.raises(SystemExit) as exc_info: + app(["refresh", "--base-url", "http://api.test"]) + + message = str(exc_info.value) + assert "\n" not in message + assert message.startswith("Failed to fetch http://api.test/openapi.json:") + assert "connection failed: connection refused" in message + + +def test_refresh_error_redacts_base_url_credentials(monkeypatch): + def fake_get(url, **kwargs): + raise httpx.ConnectError("connection refused", request=httpx.Request("GET", url)) + + monkeypatch.setattr(api_cli.httpx, "get", fake_get) + app = api_cli.build_app(SCHEMA) + + with pytest.raises(SystemExit) as exc_info: + app(["refresh", "--base-url", "http://alice:hunter2@api.test"]) + + message = str(exc_info.value) + assert "hunter2" not in message + assert "@api.test" in message + + def test_nested_refs_are_resolved_for_simple_array_body_fields(monkeypatch): schema = { "openapi": "3.1.0", @@ -1725,9 +1756,46 @@ def fake_request(method, url, **kwargs): assert json.loads(requests[-1]["content"].decode()) == {"names": ["alice"]} -def test_http_error_prints_body_and_exits_with_status(monkeypatch, capsys): +@pytest.mark.parametrize( + ("status_code", "reason", "body"), + [ + (401, "Unauthorized", "not authorized"), + (404, "Not Found", '{"detail": "missing"}'), + (500, "Internal Server Error", '{"detail": "boom"}'), + (512, "", "unused status multiple of 256"), + ], +) +def test_dynamic_operation_http_error_is_clean_one_line_nonzero( + monkeypatch, capsys, status_code, reason, body +): + def fake_request(method, url, **kwargs): + return text_response(method, url, body, status_code=status_code) + + monkeypatch.setattr(api_cli.httpx, "request", fake_request) + app = api_cli.build_app(SCHEMA) + + with pytest.raises(SystemExit) as exc_info: + app(["pipeline-runs", "list", "--base-url", "http://api.test"]) + + # Non-zero exit that is never the raw HTTP status: exit codes are 8-bit, so a + # status would truncate (404 -> 148) and multiples of 256 would report success. + code = exc_info.value.code + assert code not in (0, None, status_code) + message = str(code) + assert "\n" not in message + assert f"HTTP {status_code}" in message + if reason: + assert reason in message + assert "GET http://api.test/api/pipeline_runs/" in message + assert body in message + assert capsys.readouterr().err == "" + + +def test_dynamic_operation_http_error_bounds_huge_multiline_body(monkeypatch): + body = "detail:\n\n" + "\t".join("x" * 200 for _ in range(50)) + def fake_request(method, url, **kwargs): - return text_response(method, url, "not authorized", status_code=401) + return text_response(method, url, body, status_code=500) monkeypatch.setattr(api_cli.httpx, "request", fake_request) app = api_cli.build_app(SCHEMA) @@ -1735,11 +1803,38 @@ def fake_request(method, url, **kwargs): with pytest.raises(SystemExit) as exc_info: app(["pipeline-runs", "list", "--base-url", "http://api.test"]) - assert exc_info.value.code == 401 - assert "not authorized" in capsys.readouterr().err + message = str(exc_info.value.code) + assert "\n" not in message and "\t" not in message + assert message.endswith("…") + assert len(message) < 700 + + +def test_dynamic_operation_http_error_redacts_url_credentials(monkeypatch): + def fake_request(method, url, **kwargs): + return text_response(method, url, "denied", status_code=403) + + monkeypatch.setattr(api_cli.httpx, "request", fake_request) + app = api_cli.build_app(SCHEMA) + + with pytest.raises(SystemExit) as exc_info: + app( + [ + "pipeline-runs", + "list", + "--filter", + "active", + "--base-url", + "http://alice:hunter2@api.test", + ] + ) + + message = str(exc_info.value.code) + assert "hunter2" not in message + assert "alice" not in message + assert "@api.test" in message -def test_network_error_message_includes_url(monkeypatch): +def test_network_error_message_includes_url_and_redacts_credentials(monkeypatch): def fake_request(method, url, **kwargs): request = httpx.Request(method, url) raise httpx.ConnectError("connection refused", request=request) @@ -1748,11 +1843,51 @@ def fake_request(method, url, **kwargs): app = api_cli.build_app(SCHEMA) with pytest.raises(SystemExit) as exc_info: - app(["pipeline-runs", "list", "--base-url", "http://api.test"]) + app(["pipeline-runs", "list", "--base-url", "http://alice:pw@api.test"]) message = str(exc_info.value) - assert "http://api.test/api/pipeline_runs/" in message + assert "\n" not in message + assert message.startswith("Failed to reach GET ") + assert "/api/pipeline_runs/" in message assert "connection refused" in message + assert "pw" not in message + + +def test_dynamic_operation_connection_refused_real_socket(monkeypatch): + # A refused connection against a closed local port exercises the real httpx + # transport (no monkeypatched request) and must still exit cleanly. + import socket + + probe = socket.socket() + probe.bind(("127.0.0.1", 0)) + refused_port = probe.getsockname()[1] + probe.close() + + app = api_cli.build_app(SCHEMA) + + with pytest.raises(SystemExit) as exc_info: + app(["pipeline-runs", "list", "--base-url", f"http://127.0.0.1:{refused_port}"]) + + message = str(exc_info.value) + assert exc_info.value.code not in (0, None) + assert "\n" not in message + assert message.startswith("Failed to reach GET ") + assert f"127.0.0.1:{refused_port}" in message + + +def test_dynamic_operation_timeout_is_clean(monkeypatch): + def fake_request(method, url, **kwargs): + raise httpx.ConnectTimeout("timed out", request=httpx.Request(method, url)) + + monkeypatch.setattr(api_cli.httpx, "request", fake_request) + app = api_cli.build_app(SCHEMA) + + with pytest.raises(SystemExit) as exc_info: + app(["pipeline-runs", "list", "--base-url", "http://api.test"]) + + message = str(exc_info.value) + assert "\n" not in message + assert "connection timed out" in message def test_custom_headers_apply_to_schema_fetch_and_generated_requests(monkeypatch): diff --git a/tests/test_api_transport.py b/tests/test_api_transport.py index c3ea83a..4b4a84d 100644 --- a/tests/test_api_transport.py +++ b/tests/test_api_transport.py @@ -4,10 +4,15 @@ import pytest from tangle_cli.api_transport import ( + _MAX_BACKEND_DETAIL_CHARS, _redact_headers, build_operation_request, default_base_url, + describe_request_error, + format_http_status_error, + format_request_error, request_operation, + sanitize_url, tangle_verbose_enabled, ) @@ -241,3 +246,191 @@ def test_build_operation_request_allows_relative_paths() -> None: assert url == "https://api.tangle.test/api/components/{id}" assert headers["Authorization"] == "Bearer secret-token" assert content is None + + +def test_sanitize_url_strips_userinfo() -> None: + sanitized = sanitize_url("https://alice:hunter2@api.tangle.test:8443/api/x?limit=5") + + assert "hunter2" not in sanitized + assert "alice" not in sanitized + assert sanitized == "https://@api.tangle.test:8443/api/x?limit=5" + + +@pytest.mark.parametrize( + "param", + ["token", "access_token", "api_key", "signature", "X-Amz-Signature", "sig"], +) +def test_sanitize_url_redacts_credential_query_params(param: str) -> None: + sanitized = sanitize_url(f"https://api.tangle.test/api/x?{param}=SECRETVALUE&limit=5") + + assert "SECRETVALUE" not in sanitized + assert "" in sanitized + assert "limit=5" in sanitized + + +def test_sanitize_url_redacts_presigned_url_signature() -> None: + signed = ( + "https://bucket.s3.amazonaws.com/object?" + "X-Amz-Credential=AKIA_LEAK&X-Amz-Signature=DEADBEEFSIG&X-Amz-Expires=900" + ) + + sanitized = sanitize_url(signed) + + assert "AKIA_LEAK" not in sanitized + assert "DEADBEEFSIG" not in sanitized + assert "bucket.s3.amazonaws.com" in sanitized + + +def test_sanitize_url_preserves_plain_url() -> None: + assert sanitize_url("http://api.test/api/pipeline_runs/") == "http://api.test/api/pipeline_runs/" + + +@pytest.mark.parametrize( + ("url", "expected"), + [ + ("https://[2001:db8::1]/api/x", "https://[2001:db8::1]/api/x"), + ("https://[2001:db8::1]:8443/api/x", "https://[2001:db8::1]:8443/api/x"), + ( + "https://alice:hunter2@[2001:db8::1]:8443/api/x", + "https://@[2001:db8::1]:8443/api/x", + ), + ( + "https://alice:hunter2@[2001:db8::1]:8443/api/x?token=SECRET&limit=5", + "https://@[2001:db8::1]:8443/api/x?token=&limit=5", + ), + ], +) +def test_sanitize_url_rebrackets_ipv6_literals(url: str, expected: str) -> None: + sanitized = sanitize_url(url) + + assert sanitized == expected + assert "hunter2" not in sanitized + assert "SECRET" not in sanitized + + +def test_format_http_status_error_is_one_line_with_status_method_and_url() -> None: + request = httpx.Request("GET", "https://alice:pw@api.tangle.test/api/x") + response = httpx.Response(404, text='{"detail": "missing"}', request=request) + exc = httpx.HTTPStatusError("client error", request=request, response=response) + + message = format_http_status_error(exc) + + assert "\n" not in message + assert "HTTP 404 Not Found for GET" in message + assert "pw" not in message + assert "missing" in message + + +def test_format_http_status_error_bounds_and_normalizes_detail() -> None: + request = httpx.Request("POST", "https://api.tangle.test/api/x") + body = "line one\n\n line two\t" + "A" * 5000 + response = httpx.Response(500, text=body, request=request) + exc = httpx.HTTPStatusError("server error", request=request, response=response) + + message = format_http_status_error(exc) + + assert "\n" not in message and "\t" not in message + assert "line one line two" in message + assert message.endswith("…") + assert len(message) < _MAX_BACKEND_DETAIL_CHARS + 200 + + +def test_format_http_status_error_can_omit_detail() -> None: + request = httpx.Request("GET", "https://api.tangle.test/openapi.json") + response = httpx.Response(401, text="secret-token", request=request) + exc = httpx.HTTPStatusError("unauthorized", request=request, response=response) + + message = format_http_status_error(exc, include_detail=False) + + assert message == "HTTP 401 Unauthorized for GET https://api.tangle.test/openapi.json" + assert "secret-token" not in message + + +@pytest.mark.parametrize( + ("exc", "expected"), + [ + (httpx.ConnectError("connection refused"), "connection failed: connection refused"), + (httpx.ConnectTimeout("timed out"), "connection timed out"), + (httpx.ReadTimeout("slow"), "read timed out"), + (httpx.PoolTimeout("busy"), "connection pool timed out"), + (httpx.ProxyError("bad proxy"), "proxy error: bad proxy"), + ( + httpx.ConnectError("[SSL: CERTIFICATE_VERIFY_FAILED] self-signed certificate"), + "TLS error", + ), + ], +) +def test_describe_request_error_is_actionable(exc: httpx.RequestError, expected: str) -> None: + assert expected in describe_request_error(exc) + + +def test_format_request_error_is_one_line_and_redacts_url() -> None: + request = httpx.Request("GET", "https://alice:pw@api.tangle.test/api/x?token=SECRET") + exc = httpx.ConnectError("connection refused", request=request) + + message = format_request_error(exc) + + assert "\n" not in message + assert message.startswith("Failed to reach GET ") + assert "pw" not in message + assert "SECRET" not in message + assert "connection refused" in message + + +@pytest.mark.parametrize( + ("exc", "secrets"), + [ + ( + httpx.ProxyError( + "unable to connect to proxy http://proxyuser:proxypass@proxy.internal:8080" + ), + ["proxyuser", "proxypass"], + ), + ( + httpx.ConnectError( + "connection failed while fetching " + "https://bucket.s3.amazonaws.com/o?X-Amz-Signature=DEADBEEFSIG&X-Amz-Expires=900" + ), + ["DEADBEEFSIG"], + ), + ( + httpx.ConnectError("refused for user:secretpw@10.0.0.5"), + ["secretpw"], + ), + ], +) +def test_describe_request_error_scrubs_embedded_secrets( + exc: httpx.RequestError, secrets: list[str] +) -> None: + message = describe_request_error(exc) + + assert "\n" not in message + assert "" in message + for secret in secrets: + assert secret not in message + + +def test_describe_request_error_preserves_benign_diagnostics() -> None: + refused = describe_request_error(httpx.ConnectError("[Errno 111] Connection refused")) + assert "connection failed" in refused + assert "[Errno 111] Connection refused" in refused + + tls = describe_request_error( + httpx.ConnectError("[SSL: CERTIFICATE_VERIFY_FAILED] self-signed certificate") + ) + assert "TLS error" in tls + assert "CERTIFICATE_VERIFY_FAILED" in tls + + +def test_format_request_error_scrubs_secrets_in_exception_text() -> None: + request = httpx.Request("POST", "https://api.tangle.test/api/x") + exc = httpx.ProxyError( + "proxy http://proxyuser:proxypass@proxy.internal:8080 rejected", request=request + ) + + message = format_request_error(exc) + + assert "\n" not in message + assert "proxyuser" not in message + assert "proxypass" not in message + assert message.startswith("Failed to reach POST https://api.tangle.test/api/x:")