From c5876cfb15b3c4e8a0b98863f997d58be00ca0ec Mon Sep 17 00:00:00 2001 From: Arseniy Dugin Date: Mon, 13 Jul 2026 14:05:04 +0000 Subject: [PATCH] feat(pipeline-run): surface API HTTP failures as clean errors Pipeline-run SDK commands leaked raw requests tracebacks whenever the API answered non-2xx. Manager and annotation methods that call the client now run under a shared error-surfacing context manager that re-raises requests.HTTPError as PipelineRunError with the status, reason, method, URL and a single-line, trimmed response body, so sdk pipeline-runs commands (including submit and annotations) exit non-zero with a one-line message. Client-internal recovery such as the 404 run-id to execution-id fallback and post-submit run recovery keeps handling the statuses it can; per-run graph-state failures reuse the formatted message in each result's error field. on_poll_error and on_submit_error hooks now receive PipelineRunError with the original HTTPError chained as __cause__. Harden the shared redaction so an echoed URL or body cannot leak a credential. Field names are now judged by their tokens rather than by substring containment, so affixed and camelCase credentials (access_token, refresh_token, id_token, sessionToken, accessToken, client_secret, user_credential, AwsAccessKeyId) lose their values while max_tokens, tokenizer, token_count, function_signature, private_key_path and secretaryEmail keep theirs. URL sanitization strips userinfo, redacts presigned/SAS signature parameters while keeping the non-credential SigV4 set (X-Amz-Algorithm, X-Amz-Date, X-Amz-Expires, X-Amz-SignedHeaders) readable, descends exactly one level into a nested URL, sanitizes the fragment the OAuth implicit flow delivers a token in, and fails closed on a malformed URL -- including an unparsable port, which urlsplit only rejects once the port is read. Non-JSON bodies (form-encoded, plain text, HTML, truncated JSON) have credential assignments and auth-scheme credentials cut while the scheme name and the surrounding prose survive; a run of auth scheme names (Bearer, Basic, Digest, Negotiate, NTLM, SSWS, JWT, ApiKey, GoogleLogin, AWS4-HMAC-SHA256, Token, OAuth), matched case-insensitively across whitespace including newlines and tabs, is consumed as one bounded chain, and a chain carrying at least one explicit scheme always loses the first non-scheme token after it regardless of that token's shape or vocabulary -- an ambiguous word such as token inside the chain is scheme vocabulary and cannot shield the token behind it, so adding scheme context can only tighten redaction, never weaken it. A chain of only the ambiguous words Token and OAuth keeps the length heuristic, and WWW-Authenticate challenge parameters (realm, error, nonce) stay readable by grammar. A sensitive field's value is redacted regardless of its shape, including values beginning with slashes (password=//hunter2); a scheme:// URL start is declined by the field-name judgment rather than by a grammar exemption, so benign URL and path assignments stay readable. A URL or bare user:pass@host reflected in a body is scrubbed structurally so its host and path stay diagnosable. Parsed JSON bodies are walked iteratively under a depth bound that fails closed, so a hostile body cannot exhaust the interpreter stack, and every scan is anchored rather than backtracking, so cost stays linear in the body length. All redaction runs before the body is collapsed to one line and truncated. --- .../src/tangle_cli/api_transport.py | 763 +++++++++++++- .../tangle-cli/src/tangle_cli/cli_helpers.py | 58 ++ packages/tangle-cli/src/tangle_cli/handler.py | 12 + .../tangle_cli/pipeline_run_annotations.py | 12 +- .../src/tangle_cli/pipeline_run_details.py | 5 + .../src/tangle_cli/pipeline_run_manager.py | 100 +- .../src/tangle_cli/pipeline_runs_cli.py | 5 +- tests/test_sdk_http_errors.py | 960 ++++++++++++++++++ 8 files changed, 1855 insertions(+), 60 deletions(-) create mode 100644 tests/test_sdk_http_errors.py diff --git a/packages/tangle-cli/src/tangle_cli/api_transport.py b/packages/tangle-cli/src/tangle_cli/api_transport.py index b128070..2dc17a4 100644 --- a/packages/tangle-cli/src/tangle_cli/api_transport.py +++ b/packages/tangle-cli/src/tangle_cli/api_transport.py @@ -5,6 +5,7 @@ import json import os import re +import string import sys import urllib.parse from pathlib import Path @@ -17,12 +18,209 @@ _HEADER_NAME_RE = re.compile(r"^[!#$%&'*+.^_`|~0-9A-Za-z-]+$") _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)", +# A field name is judged by its *tokens*, not by substring containment, so +# ``max_tokens``, ``token_count``, ``function_signature``, and ``secretary_email`` +# keep their values while ``access_token`` and ``my_api_key`` lose theirs. +_FIELD_NAME_TOKEN_SPLIT_RE = re.compile(r"[^A-Za-z0-9]+") +# camelCase/PascalCase word boundaries, so ``accessToken`` tokenizes the same way +# ``access_token`` does. The second alternative splits an acronym from the word +# that follows it (``APIKey`` -> ``API`` ``Key``) without splitting the acronym +# itself, which is what keeps ``XApiKey`` and ``oauth2Token`` recognizable. +_FIELD_NAME_CAMEL_BOUNDARY_RE = re.compile(r"(?<=[a-z0-9])(?=[A-Z])|(?<=[A-Z])(?=[A-Z][a-z])") +# Tokens that name a credential on their own, as the whole field name or as its +# final token. Deliberately singular: ``tokens`` counts usage, ``token`` is one. +_CREDENTIAL_WORDS = frozenset( + { + "auth", + "authentication", + "authorization", + "cookie", + "credential", + "credentials", + "passphrase", + "passwd", + "password", + "pwd", + "secret", + "token", + # Credential names that arrive as one unbroken lowercase run and so offer + # no boundary to tokenize on. Listed in full rather than matched as + # substrings, which is what keeps ``accesskeyidformat``, ``privatekeypath``, + # ``maxtokens``, ``sessionid``, ``oauthlib``, and ``tokenizer`` readable. + "accessid", + "accesskey", + "accesskeyid", + "accesstoken", + "apikey", + "apisecret", + "apitoken", + "authtoken", + "awsaccesskeyid", + "bearertoken", + "clientsecret", + "googleaccessid", + "idtoken", + "oauth", + "privatekey", + "refreshtoken", + "secretkey", + "sessionkey", + "sessiontoken", + } +) +# Trailing words that name an identifier. They do not make a field safe: an +# access-key ID is one half of a credential pair and is issued and revoked with +# it, so the suffix is stripped and what it was attached to is judged again. +# ``access_key_id_format`` is unaffected -- its trailing word is ``format``. +_IDENTIFIER_SUFFIX_WORDS = frozenset({"id", "ident", "identifier"}) +# Words that name key material only once an identifier suffix is stripped. +# ``access`` alone is ordinary; ``access_id`` and ``GoogleAccessId`` are not. +_CREDENTIAL_ID_WORDS = frozenset({"access"}) +# Credential names that only read as one across two tokens. ``key`` and ``url`` +# are far too common alone, so they are sensitive only in these pairings. +_CREDENTIAL_PHRASES = frozenset( + { + ("access", "key"), + ("api", "key"), + ("presigned", "url"), + ("private", "key"), + ("secret", "key"), + ("session", "key"), + ("signed", "url"), + } +) +# Credential fields whose value is a URL. Blanket redaction throws away the host +# and path -- the part that separates a wrong bucket from an expired grant -- so +# inside a parsed body these go through the URL scrubber instead, which strips +# the signature and keeps the rest. A value with no URL in it has no such +# structure to lean on and is still redacted whole, as are header and query +# occurrences, whose values are never scrubbed. +_URL_VALUED_CREDENTIAL_PHRASES = frozenset({("presigned", "url"), ("signed", "url")}) +# Query parameters that carry the credential portion of a presigned/SAS URL +# (AWS SigV4, GCS, Azure). Redacting the signature neutralizes the grant, so the +# rest of the SigV4 parameter set (``X-Amz-Algorithm``, ``X-Amz-Date``, +# ``X-Amz-Expires``, ``X-Amz-SignedHeaders``) stays readable and remains useful +# for diagnosing an expired or malformed link. These names are matched in full, +# so ``function_signature`` is untouched. +_SIGNED_URL_QUERY_RE = re.compile( + r"^(x-(amz|goog|ms)-(signature|credential|security-token)" + r"|sig|signature|awsaccesskeyid|googleaccessid)$", + re.IGNORECASE, +) +# HTTP authentication schemes that prefix the credential rather than being one. +# The scheme name is diagnostic and kept; only the credential after it is cut. +# ``Token`` and ``OAuth`` are also ordinary English words, so whether a matched +# run of scheme words is trusted to carry a credential is judged per match +# against ``_AMBIGUOUS_BARE_SCHEMES`` rather than encoded in the pattern. +_AUTH_SCHEME = ( + r"Bearer|Basic|Digest|Token|JWT|OAuth|ApiKey|SSWS|Negotiate|NTLM" + r"|GoogleLogin|AWS4-HMAC-SHA256" +) +# The ``=``/``:`` separator of an assignment, on its own. Scanning for +# separators rather than for a fixed list of key names is what makes the field +# name open-ended: the name is recovered by a bounded lookbehind and judged by +# :func:`_is_sensitive_query_key`, so affixed names (``access_token``, +# ``my_api_key``) are covered without an alternation having to enumerate them. +# Free text is judged by the query predicate rather than the body one because a +# reflected ``X-Amz-Signature=...`` reads as a credential wherever it appears. +# The pattern deliberately stops at the separator so a declined match consumes +# nothing of the value -- a nested assignment under a harmless outer key +# (``{"detail": "token=..."``) is still reached. Optional quotes are absorbed so +# truncated JSON is scrubbed too. A ``scheme://`` URL start needs no grammar +# exemption here: its would-be field name (``https``, ``x``) is judged +# non-sensitive and declined, while an explicitly sensitive name always loses +# its value -- a credential is not exempted by beginning with slashes +# (``password=//hunter2``). Everything after the separator character is only +# scanned once a separator matched, so scanning is linear in the text length. +# The separator absorbs any whitespace, newlines included: display collapses +# whitespace only after redaction, so a credential on the line below its field +# name must be caught here, before collapsing makes the two adjacent. +_ASSIGNMENT_SEPARATOR_RE = re.compile(r"[:=]\s*[\"']?") +# Characters that may sit between a field name and its separator. +_FIELD_NAME_PADDING = " \t\"'" +# Characters a field name (or its padding) can end with, for an O(1) pre-check +# that skips the lookbehind for separators no name could precede. +_FIELD_NAME_TAIL_CHARS = _FIELD_NAME_PADDING + "_.-" +# The value of an assignment, anchored immediately after its separator. The +# value stops at whitespace, a form/list delimiter, a quote, or ``?`` so +# form-encoded, HTML-embedded, and query-string values stay bounded. The scheme +# prefix is a chain (``Authorization: Basic Bearer sk``): consuming the run of +# scheme names in one match is what keeps the credential behind a doubled +# scheme from surviving as the next word after a non-overlapping match. +_ASSIGNMENT_VALUE_RE = re.compile( + rf"(?:(?P(?:{_AUTH_SCHEME})(?:\s+(?:{_AUTH_SCHEME}))*)\s+)?" + r"(?P[^\s&;,<>\"'?]+)", re.IGNORECASE, ) +# Trailing field name immediately preceding an assignment separator. +_FIELD_NAME_RE = re.compile(r"[A-Za-z][A-Za-z0-9_.\-]*$") +# Longest field name considered; bounds the per-separator lookbehind. +_MAX_FIELD_NAME_CHARS = 64 _REDACTED = "" _REDACTED_DOCUMENT = "" +_REDACTED_DEEP = "" +# An auth scheme carrying its credential without a preceding field name (e.g. +# ``rejected header: Bearer sk-1``, where the name is not itself sensitive). +# Matched case-insensitively: a proxy or logging layer may lowercase the header +# it reflects, so ``bearer``/``BASIC`` spellings must redact the same. An +# unambiguous scheme always cuts the value that follows -- a credential can be +# short and word-like, so neither its shape nor its vocabulary is trusted, and +# prose after a scheme name is conservatively lost rather than risked. The +# chain absorbs a whole run of scheme names (``Basic Bearer sk``) in one match, +# so a non-overlapping scan cannot stop at a doubled scheme and leave the +# credential behind it as unmatched text. Whitespace between the words includes +# newlines and tabs: display collapses whitespace only after redaction, so a +# credential on its own line must be caught before collapsing makes it adjacent +# to the scheme. Each chain repetition consumes at least one word and at most +# one word is given back when no value follows, so matching stays linear. The +# redaction marker is accepted as a value so a chain whose credential the +# assignment pass already cut (``Authorization: Basic Bearer sk``) is +# recognized as done rather than re-matched with a scheme word as the value. +_BARE_AUTH_SCHEME_RE = re.compile( + rf"\b(?P(?:(?:{_AUTH_SCHEME})\s+)+)" + rf"(?P{re.escape(_REDACTED)}|[A-Za-z0-9\-._~+/=]+)", + re.IGNORECASE, +) +_SCHEME_CHAIN_WORD_RE = re.compile(r"\S+") +# Scheme names that are also ordinary English words, compared case-insensitively: +# a bare occurrence needs a credential long enough that "Token 12345 expired" and +# "token count: 42" cannot trip it. +_AMBIGUOUS_BARE_SCHEMES = frozenset({"token", "oauth"}) +_MIN_AMBIGUOUS_SCHEME_CREDENTIAL_CHARS = 16 +# Challenge parameters (RFC 7235, plus the Digest and Bearer sets) follow a +# scheme name in a ``WWW-Authenticate`` challenge (``Basic realm="api"``) as +# directives, not as its credential. Matched as a prefix of the captured value +# so quoted (``realm="api"``, where the capture stops at the quote) and +# unquoted (``realm=api``) spellings are both kept. A credential cannot +# collide: base64 and JWT values never carry ``=`` before their final padding. +_AUTH_CHALLENGE_PARAM_RE = re.compile( + r"(?:realm|charset|error|error_description|error_uri|scope|nonce|opaque" + r"|qop|algorithm|stale|domain|userhash)=", + re.IGNORECASE, +) +# Character classes for locating an embedded ``scheme://...`` run. The scan +# anchors on the ``://`` literal and walks outward, because a regex of the form +# ``[a-zA-Z][a-zA-Z0-9+.\-]*://`` re-scans every alphanumeric run once per +# starting offset and so costs O(n^2) on a body that is one long run. +_URL_SCHEME_CHARS = frozenset(string.ascii_letters + string.digits + "+.-") +_URL_SCHEME_START_CHARS = frozenset(string.ascii_letters) +_URL_SEPARATOR = "://" +_URL_STOP_CHARS = frozenset("'\"<>") +# Punctuation that ordinarily terminates a sentence rather than a URL. +_URL_TRAILING_PUNCTUATION = ").,;'\"" +# ``user:pass@host`` userinfo carries the credential before the ``@``; the ``@`` +# is the anchor, and the host side is never part of the secret. +_USERINFO_STOP_CHARS = frozenset("/@") +# Percent-encoding layers peeled off a displayed leaf while looking for a hidden +# credential assignment. Encoding ``access_token=x`` again hides the separator +# from a scanner that decodes once, so one layer is not enough; the cap is what +# keeps the walk linear, since each peel is O(len) and there is a fixed number of +# them. A leaf still changing under decode when the cap runs out is dropped. +_MAX_DECODE_LAYERS = 4 +# Structure nesting kept when redacting a parsed JSON body. Error detail is +# collapsed onto one bounded line anyway, so nothing legible is lost past this +# point, and a hostile body cannot make an always-on error path recurse. +_MAX_JSON_DEPTH = 32 _OPAQUE_DOCUMENT_KEY_NAMES = { "component_yaml", "dockerfile", @@ -40,38 +238,234 @@ def tangle_verbose_enabled() -> bool: return value.strip().lower() in {"1", "true", "yes", "on"} +def _field_name_tokens(name: str) -> list[str]: + """Split *name* on non-alphanumerics and camelCase/PascalCase boundaries.""" + + split = _FIELD_NAME_CAMEL_BOUNDARY_RE.sub(" ", name).lower() + return [token for token in _FIELD_NAME_TOKEN_SPLIT_RE.split(split) if token] + + +def _names_credential(tokens: list[str]) -> bool: + """Do *tokens* end in a word, or pair of words, that names a credential?""" + + if tokens[-1] in _CREDENTIAL_WORDS: + return True + return len(tokens) >= 2 and tuple(tokens[-2:]) in _CREDENTIAL_PHRASES + + +def _is_url_valued_credential_field(name: str) -> bool: + """Is *name* a credential field whose value is expected to be a URL?""" + + tokens = _field_name_tokens(name) + return len(tokens) >= 2 and tuple(tokens[-2:]) in _URL_VALUED_CREDENTIAL_PHRASES + + +def _is_sensitive_field_name(name: str) -> bool: + """Is *name* a header or body field whose value must never be displayed? + + The name is split into tokens on any non-alphanumeric character and on + camelCase/PascalCase word boundaries, then judged by its final token (or + final two), so an affixed credential such as ``access_token``, + ``accessToken``, ``client_secret``, or ``myApiKey`` is caught without the + predicate having to enumerate prefixes or spellings. A trailing identifier + word is stripped and the remainder judged again, which is what catches + ``access_key_id``, ``AwsAccessKeyId``, and ``GoogleAccessId``. + + Judging tokens rather than substrings is what keeps ordinary fields + readable: ``max_tokens``, ``tokenCount``, ``function_signature``, + ``signatureVersion``, ``password_policy``, ``private_key_path``, and + ``secretaryEmail`` all end in a token that names something other than a + credential. ``access_key_id_format`` is readable for the same reason -- its + identifier word is not trailing, so nothing is stripped. + """ + + tokens = _field_name_tokens(name) + if not tokens: + return False + if _names_credential(tokens): + return True + while tokens[-1] in _IDENTIFIER_SUFFIX_WORDS: + tokens = tokens[:-1] + if not tokens: + return False + if tokens[-1] in _CREDENTIAL_ID_WORDS or _names_credential(tokens): + return True + return False + + def _redact_headers(headers: dict[str, Any] | None) -> dict[str, Any]: redacted: dict[str, Any] = {} for name, value in (headers or {}).items(): normalized_name = name.lower() redacted[name] = ( _REDACTED - if normalized_name in _SENSITIVE_HEADER_NAMES or _SENSITIVE_KEY_RE.search(name) + if normalized_name in _SENSITIVE_HEADER_NAMES or _is_sensitive_field_name(name) else value ) return redacted def _redact_sensitive_values(value: Any, key: str | None = None) -> Any: - if key and _SENSITIVE_KEY_RE.search(key): - return _REDACTED - if key and key.lower() in _OPAQUE_DOCUMENT_KEY_NAMES and isinstance(value, str) and value: - return _REDACTED_DOCUMENT - if isinstance(value, dict): - return {str(k): _redact_sensitive_values(v, str(k)) for k, v in value.items()} - if isinstance(value, list): - return [_redact_sensitive_values(item) for item in value] - return value + """Rebuild *value* with credential-bearing fields and string leaves redacted. + + Iterative rather than recursive, and bounded by :data:`_MAX_JSON_DEPTH`: this + runs on every failed request, on a body an untrusted backend controls, and a + few kilobytes of ``[[[[...]]]]`` is enough to exhaust the interpreter stack. + Anything nested deeper than the bound is replaced wholesale, so the bound + fails closed -- an unexamined subtree is never emitted. + """ + + holder: list[Any] = [None] + # (container, slot, node, node_key, depth); the result is written into the + # slot the parent reserved, which keeps dict insertion order intact. + pending: list[tuple[Any, Any, Any, str | None, int]] = [(holder, 0, value, key, 0)] + while pending: + container, slot, node, node_key, depth = pending.pop() + if ( + node_key + and isinstance(node, str) + and _URL_SEPARATOR in node + and _is_url_valued_credential_field(node_key) + ): + container[slot] = _redact_text_secrets(node) + elif node_key and _is_sensitive_field_name(node_key): + container[slot] = _REDACTED + elif ( + node_key + and node_key.lower() in _OPAQUE_DOCUMENT_KEY_NAMES + and isinstance(node, str) + and node + ): + container[slot] = _REDACTED_DOCUMENT + elif isinstance(node, dict): + if depth >= _MAX_JSON_DEPTH: + container[slot] = _REDACTED_DEEP + continue + branch: dict[str, Any] = {} + container[slot] = branch + for child_key in node: + branch[str(child_key)] = None + # Reversed so the stack pops in source order and a duplicated + # ``str(key)`` resolves to the last occurrence, as a dict would. + for child_key, child in reversed(list(node.items())): + name = str(child_key) + pending.append((branch, name, child, name, depth + 1)) + elif isinstance(node, list): + if depth >= _MAX_JSON_DEPTH: + container[slot] = _REDACTED_DEEP + continue + items: list[Any] = [None] * len(node) + container[slot] = items + for index, child in enumerate(node): + pending.append((items, index, child, None, depth + 1)) + elif isinstance(node, str): + # A non-sensitive field can still quote a sensitive assignment back at + # us (``{"detail": "invalid token=... supplied"}``), so string leaves + # get the same free-text scrub as an unparseable body. + container[slot] = _redact_text_secrets(node) + else: + container[slot] = node + return holder[0] def _safe_json_text(value: Any) -> str: redacted = _redact_sensitive_values(value) try: return json.dumps(redacted, indent=2, sort_keys=True, default=str) - except TypeError: + except (TypeError, ValueError, RecursionError): return str(redacted) +def _redact_assignments(text: str) -> str: + """Redact credential assignments and auth-scheme credentials in *text*. + + Each ``=``/``:`` separator is found once, the field name preceding it is + recovered by a bounded lookbehind, and :func:`_is_sensitive_query_key` + decides -- so any spelling of a credential field is covered, including + affixed names such as ``access_token`` or ``my_api_key`` that no fixed + alternation would list. Only the value is replaced, so non-sensitive + assignments (``page=2``) and the surrounding diagnostic prose survive. An + auth scheme keeps its name (``Bearer ``) because the scheme is + useful and the credential is not; once an unambiguous scheme matches, in any + letter case, the word that follows is cut regardless of shape or vocabulary, + and a chained run of scheme names (``Basic Bearer sk``) keeps its scheme + words while losing the first non-scheme token after the last of them -- an + ambiguous word inside a chain that carries an explicit scheme + (``Bearer token s3cret``) is scheme vocabulary, not the chain's end, so it + cannot shield the token behind it. Only + ``WWW-Authenticate`` challenge parameters (``realm=``, ``error=``, + ``nonce=``) are exempt, by grammar rather than by value judgment, and the + length heuristic remains only for the ambiguous scheme words ``Token`` and + ``OAuth``. + + Work is bounded per separator and per match, with no nested quantifier, so + cost stays linear in ``len(text)``. Nothing here parses a URL or re-enters + :func:`sanitize_url`, which is what lets the URL sanitizer reuse it as a leaf. + """ + + def _replace_bare_scheme(match: re.Match[str]) -> str: + """Redact the token after a bare, possibly chained, auth scheme. + + A chain carrying at least one explicit scheme name always loses the + first non-scheme token after it, whatever that token's shape: an + ambiguous word such as ``token`` inside the chain is scheme vocabulary, + and trusting it to end the chain would let ``Bearer token `` + keep the secret. A chain of only ambiguous words ("token count") needs + a value long enough to be opaque, and a value shaped like a challenge + parameter (``realm="api"``) is a directive rather than a credential. + """ + + chain, value = match.group("chain"), match.group("value") + if value == _REDACTED or _AUTH_CHALLENGE_PARAM_RE.match(value): + return match.group(0) + has_explicit = any( + word.lower() not in _AMBIGUOUS_BARE_SCHEMES + for word in _SCHEME_CHAIN_WORD_RE.findall(chain) + ) + if not has_explicit and len(value) < _MIN_AMBIGUOUS_SCHEME_CREDENTIAL_CHARS: + return match.group(0) + return f"{chain}{_REDACTED}" + + chunks: list[str] = [] + cursor = 0 + for separator in _ASSIGNMENT_SEPARATOR_RE.finditer(text): + start = separator.start() + if start < cursor: + # Inside a value that was already redacted. + continue + if start == 0 or not ( + text[start - 1].isalnum() or text[start - 1] in _FIELD_NAME_TAIL_CHARS + ): + # Nothing that could end a field name; skip the lookbehind entirely. + continue + preceding = text[max(0, start - _MAX_FIELD_NAME_CHARS) : start] + name = _FIELD_NAME_RE.search(preceding.rstrip(_FIELD_NAME_PADDING)) + if name is None or not _is_sensitive_query_key(name.group(0)): + continue + value = _ASSIGNMENT_VALUE_RE.match(text, separator.end()) + if value is None: + continue + chunks.append(text[cursor : value.start("value")]) + chunks.append(_REDACTED) + cursor = value.end("value") + chunks.append(text[cursor:]) + return _BARE_AUTH_SCHEME_RE.sub(_replace_bare_scheme, "".join(chunks)) + + +def _redact_text_secrets(text: str) -> str: + """Redact credentials in free text. + + Handles every representation the structured JSON path cannot: form-encoded + bodies, plain text, HTML error pages, truncated/unparseable JSON, and string + leaves inside otherwise well-formed JSON. Credentials carried in a URL or in + bare ``user:pass@host`` userinfo are stripped first by + :func:`_scrub_secret_text`, the same primitive used on exception text; what + survives as a plain assignment is then caught by :func:`_redact_assignments`. + """ + + return _redact_assignments(_scrub_secret_text(text)) + + def _content_to_text(content: bytes | str | None) -> str: if content is None: return "" @@ -86,10 +480,351 @@ def _content_to_text(content: bytes | str | None) -> str: try: parsed = json.loads(text) except Exception: - return text + return _redact_text_secrets(text) return _safe_json_text(parsed) +def _is_sensitive_query_key(key: str) -> bool: + """Is *key* a query parameter whose value must never be displayed? + + Stricter than the header/body predicate: a query string is also where a + presigned-URL grant lives, and those parameter names (``sig``, ``signature``, + ``X-Amz-*``) are credentials in a way the same word is not when it names a + body field such as ``function_signature``. + """ + + stripped = key.strip() + return _is_sensitive_field_name(stripped) or bool( + _SIGNED_URL_QUERY_RE.fullmatch(stripped) + ) + + +def _redact_parameter_credentials(query: str) -> str: + """Redact credential-named parameters in *query*, or drop it wholesale. + + The leaf of the sanitizer, and where the descent stops. It judges parameter + names and strips userinfo, but it will not parse a value as a URL in turn — + so a value that still looks like one is redacted whole rather than emitted + unexamined. That is what makes the depth bound safe instead of merely finite: + nesting a credential one level deeper than the sanitizer looks buries it + rather than smuggling it out. + """ + + if not query: + return "" + try: + pairs = urllib.parse.parse_qsl(query, keep_blank_values=True) + except ValueError: + return _REDACTED + if not pairs or urllib.parse.urlencode(pairs) != query: + return _REDACTED + return urllib.parse.urlencode( + [ + (_redact_bare_userinfo(key), _redact_leaf_value(key, value)) + for key, value in pairs + ], + safe="<>", + ) + + +def _redact_leaf_value(key: str, value: str) -> str: + """Redact *value* if its name is sensitive or it hides a further URL.""" + + if _is_sensitive_query_key(key) or _URL_SEPARATOR in value: + return _REDACTED + return _redact_assignments(_redact_bare_userinfo(value)) + + +def _redact_nested_url_credentials(value: str) -> str: + """Redact credential parameters inside a nested absolute URL. + + A return URL is routinely carried inside another URL + (``?next=https%3A%2F%2Fhost%2Fcb%3Faccess_token%3D...``), and once + :func:`urllib.parse.parse_qsl` has decoded it the inner credential is plainly + visible. Its scheme, host, and path are kept so the destination stays + diagnosable; only its own parameters are judged, by + :func:`_redact_parameter_credentials`, which does not descend again. Going + exactly one level deep is deliberate: :func:`sanitize_url` must not be + re-entered here, or a URL nested inside a URL inside a URL would drive the + recursion as deep as an untrusted body cared to nest it. + """ + + if _URL_SEPARATOR not in value: + return value + try: + parsed = urllib.parse.urlsplit(value) + except ValueError: + return _REDACTED + if not parsed.scheme or not (parsed.query or parsed.fragment): + return value + return urllib.parse.urlunsplit( + ( + parsed.scheme, + parsed.netloc, + parsed.path, + _redact_parameter_credentials(parsed.query), + _redact_parameter_credentials(parsed.fragment), + ) + ) + + +def _hides_credential(value: str) -> bool: + """Report whether bounded percent-decoding of *value* reveals a credential. + + ``state=access_token%3D...`` is legible after the single decode + :func:`urllib.parse.parse_qsl` already performed. Encoding it again hides the + separator from :func:`_redact_assignments`, and encoding it a third time hides + it from any check that decodes only once more. Layers are therefore peeled up + to ``_MAX_DECODE_LAYERS`` and every intermediate form is scanned. + + Every shape the caller redacts at the surface is looked for at each layer, not + just assignments: ``alice%253Apw%2540host`` is userinfo one decode further + down, and a scan for ``=`` alone walks straight past it. + + The walk is bounded rather than run to a fixpoint: an attacker supplies the + value, so the number of layers must not be theirs to choose. When the cap runs + out on a value that is still changing under decode, encoding remains that was + never looked behind, and that is reported as hiding a credential -- burying one + deeper than the sanitizer looks drops the value instead of publishing it. + """ + + for _ in range(_MAX_DECODE_LAYERS): + decoded = urllib.parse.unquote(value) + if decoded == value: + return False + if ( + _redact_assignments(decoded) != decoded + or _redact_bare_userinfo(decoded) != decoded + ): + return True + value = decoded + return urllib.parse.unquote(value) != value + + +def _scrub_parameter_value(value: str) -> str: + """Scrub a decoded parameter value in every shape a credential arrives in. + + An absolute URL is sanitized structurally, so its host and path survive. Any + other shape -- a relative return path (``/cb?access_token=...``), an opaque + ``mailto:``/``data:`` URI, or a bare ``access_token=...`` pair that a caller + round-trips through ``state`` -- has no structure worth preserving, so it is + scanned for credential assignments instead. The scan runs last either way: + ahead of :func:`_redact_nested_url_credentials` it would rewrite the nested + query out of its exact-round-trip form and cost that URL its host and path. + """ + + value = _redact_bare_userinfo(value) + if _URL_SEPARATOR in value: + value = _redact_nested_url_credentials(value) + value = _redact_assignments(value) + if _hides_credential(value): + # Encoded past what the scans above can read (``state=access_token%253D...``, + # ``next=alice%253Apw%2540host``). Emitting it would publish a value never + # examined, so fail closed. + return _REDACTED + return value + + +def _sanitize_query_pairs(pairs: list[tuple[str, str]]) -> str: + """Re-encode *pairs*, redacting credential names and scrubbing what remains. + + :func:`urllib.parse.parse_qsl` has already percent-decoded both halves of a + pair, so a credential smuggled through a harmless-looking name is legible + here: as userinfo (``next=https%3A%2F%2Fu%3Apw%40host``), as a parameter of a + nested URL (``next=...%3Faccess_token%3D...``), as a bare assignment + (``state=access_token%3D...``), or in the parameter name itself, either as + userinfo (``alice%3Apw%40host=1``) or as a whole assignment encoded into the + name (``access_token%3D...=1``). A name is displayed just as a value is, so + both halves go through the same leaf scrub. + + Sensitivity is judged on the name as parsed, before that scrub, so rewriting + a name cannot change the verdict on its value. + + Only non-recursive primitives are used. :func:`_scrub_secret_text` in + particular is not, because it routes embedded URLs back through + :func:`sanitize_url`, which arrives here again. + """ + + return urllib.parse.urlencode( + [ + ( + _scrub_parameter_value(key), + _REDACTED + if _is_sensitive_query_key(key) + else _scrub_parameter_value(value), + ) + for key, value in pairs + ], + safe="<>", + ) + + +def _sanitize_fragment(fragment: str) -> str: + """Return *fragment* with credentials removed, or drop it wholesale. + + A fragment is not merely an anchor: the OAuth implicit flow returns + ``#access_token=...&token_type=Bearer`` there precisely so the credential + stays out of the query, and it reaches this function on the always-on error + display path. + + A fragment carrying neither ``=`` nor ``&`` is an anchor. Its userinfo is + still stripped (``#u:pw@host`` is a valid anchor and a leak), and if it + embeds a whole URL it is dropped instead, because sanitizing that properly + would mean re-entering :func:`sanitize_url` from inside itself. Otherwise + the fragment is parsed as a query string and re-encoded from the parsed + pairs, so every value emitted is one that was examined. If the re-encoding + does not reproduce the original exactly, some separator this does not model + (``;``, a stray encoding) could be hiding a value that was never examined, + so the fragment is dropped rather than guessed at. + """ + + if not fragment: + return "" + if "=" not in fragment and "&" not in fragment: + if _URL_SEPARATOR in fragment: + return _REDACTED + anchor = _redact_assignments(_redact_bare_userinfo(fragment)) + if _hides_credential(anchor): + return _REDACTED + return anchor + try: + pairs = urllib.parse.parse_qsl(fragment, keep_blank_values=True) + except ValueError: + return _REDACTED + if not pairs or urllib.parse.urlencode(pairs) != fragment: + return _REDACTED + return _sanitize_query_pairs(pairs) + + +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. A parameter kept under a non-sensitive name is scrubbed in turn, + since a credential rides through one either as userinfo + (``next=https%3A%2F%2Fu%3Apw%40host``) or as a parameter of the URL nested + inside it (``next=...%3Faccess_token%3D...``); the nested URL is descended + exactly one level. The scheme, host, port, path, and non-sensitive parameter + names are preserved so the target stays recognizable. The fragment is + sanitized on the same terms by :func:`_sanitize_fragment`, because the OAuth + implicit flow delivers its access token there. + """ + + text = str(url) + try: + parsed = urllib.parse.urlsplit(text) + except ValueError: + return _REDACTED + if not parsed.scheme and not parsed.netloc: + return text + try: + host = parsed.hostname or "" + port = parsed.port + except ValueError: + # ``urlsplit`` defers authority validation until ``hostname``/``port`` is + # read, so a bad port (``host:99999``, ``host:bad``) raises here rather + # than above. Once the authority does not parse, the userinfo boundary + # inside it cannot be trusted either, so the whole authority is dropped. + # The scheme, path, and query are still sanitized and still diagnostic. + netloc = _REDACTED + else: + # ``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 port is not None: + netloc = f"{netloc}:{port}" + query = parsed.query + if query: + query = _sanitize_query_pairs( + urllib.parse.parse_qsl(query, keep_blank_values=True) + ) + fragment = _sanitize_fragment(parsed.fragment) + return urllib.parse.urlunsplit((parsed.scheme, netloc, parsed.path, query, fragment)) + + +def _redact_embedded_urls(text: str) -> str: + """Route every ``scheme://...`` run in *text* through :func:`sanitize_url`. + + Anchored on the ``://`` literal: the scheme is recovered by walking back over + the scheme characters and the target by walking forward to the first + delimiter. Consecutive runs cannot overlap, so every character is visited a + bounded number of times whatever shape the input has. + """ + + chunks: list[str] = [] + cursor = 0 + separator = text.find(_URL_SEPARATOR) + while separator != -1: + start = separator + while start > cursor and text[start - 1] in _URL_SCHEME_CHARS: + start -= 1 + while start < separator and text[start] not in _URL_SCHEME_START_CHARS: + start += 1 + if start < separator: + end = separator + len(_URL_SEPARATOR) + while end < len(text) and not ( + text[end].isspace() or text[end] in _URL_STOP_CHARS + ): + end += 1 + raw = text[start:end] + trailing = "" + while raw and raw[-1] in _URL_TRAILING_PUNCTUATION: + trailing = raw[-1] + trailing + raw = raw[:-1] + chunks.append(text[cursor:start]) + chunks.append(sanitize_url(raw) + trailing) + cursor = end + separator = text.find(_URL_SEPARATOR, max(separator + 1, cursor)) + chunks.append(text[cursor:]) + return "".join(chunks) + + +def _redact_bare_userinfo(text: str) -> str: + """Redact schemeless ``user:pass@host`` userinfo, keeping the host. + + Anchored on ``@`` and walking back only to the nearest delimiter, so the cost + is linear even when the text is one unbroken run. + """ + + chunks: list[str] = [] + cursor = 0 + at = text.find("@") + while at != -1: + start = at + while start > cursor and not ( + text[start - 1].isspace() or text[start - 1] in _USERINFO_STOP_CHARS + ): + start -= 1 + userinfo = text[start:at] + colon = userinfo.find(":") + # An empty username is legal userinfo (``:password@host``), so the colon + # may sit at offset zero; it may not sit last, or there is no credential. + if 0 <= colon < len(userinfo) - 1: + chunks.append(text[cursor:start]) + chunks.append(_REDACTED) + cursor = at + at = text.find("@", at + 1) + chunks.append(text[cursor:]) + return "".join(chunks) + + +def _scrub_secret_text(text: str) -> str: + """Redact URLs and bare userinfo embedded in free-form text. + + A crafted or third-party ``httpx`` exception -- and equally a reflected + response body -- can carry a proxy URL, a 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. + """ + + return _redact_bare_userinfo(_redact_embedded_urls(text)) + + def log_http_exchange( logger: Any, *, diff --git a/packages/tangle-cli/src/tangle_cli/cli_helpers.py b/packages/tangle-cli/src/tangle_cli/cli_helpers.py index 7868fb4..e1b608b 100644 --- a/packages/tangle-cli/src/tangle_cli/cli_helpers.py +++ b/packages/tangle-cli/src/tangle_cli/cli_helpers.py @@ -4,10 +4,68 @@ import json import pathlib +from collections.abc import Iterator +from contextlib import contextmanager from typing import Any +import requests + +from .api_transport import _content_to_text, sanitize_url from .args_container import ArgsContainer, ConfigFileError +_HTTP_ERROR_BODY_LIMIT = 2000 + + +def format_http_error(exc: requests.HTTPError) -> str: + """Render an HTTP status failure as a concise CLI message for SDK commands. + + SDK/static client calls raise ``requests.HTTPError`` for non-2xx responses + (via ``raise_for_status``). Client-internal helpers handle the statuses they + can recover from (e.g. the 404 run-id -> execution-id fallback) and re-raise + the rest, so the command layer surfaces the remaining errors here instead of + letting a raw traceback reach the interpreter. The response status, reason, + attempted method/URL, and body are preserved as that context is what a caller + needs to act on. The attempted URL goes through :func:`sanitize_url` and the + body through :func:`_content_to_text`, the same transport-neutral redaction + the diagnostic log path uses, so userinfo, credential query parameters, and + credentials reflected in the body are removed *before* the body is collapsed + to a single line and truncated -- truncating first would leave a secret in + the surviving prefix. Only HTTP status failures are formatted here; + connection/timeout errors carry no response and propagate unchanged. + """ + + response = exc.response + if response is None: + return f"Tangle API request failed: {exc}" + request = response.request + if request is not None and request.url: + target = f"{request.method} {sanitize_url(request.url)}" + else: + target = (sanitize_url(response.url) if response.url else "") or "Tangle API" + reason = f" {response.reason}" if response.reason else "" + summary = f"Tangle API request failed ({response.status_code}{reason}) for {target}" + body = " ".join(_content_to_text(response.content).split()) + if not body or body == "": + return summary + if len(body) > _HTTP_ERROR_BODY_LIMIT: + body = f"{body[:_HTTP_ERROR_BODY_LIMIT]}... (truncated)" + return f"{summary}: {body}" + + +@contextmanager +def surface_http_errors(error_type: type[Exception]) -> Iterator[None]: + """Re-raise ``requests.HTTPError`` as ``error_type`` with a formatted message. + + Client-internal recovery runs first and re-raises only the statuses it + cannot handle, so errors reaching here are the ones the command layer must + surface as a clean nonzero exit rather than a raw traceback. + """ + + try: + yield + except requests.HTTPError as exc: + raise error_type(format_http_error(exc)) from exc + def load_args_or_exit(config: str | None, **kwargs: Any) -> list[ArgsContainer]: """Load ArgsContainer values from CLI/config specs, exiting with CLI errors.""" diff --git a/packages/tangle-cli/src/tangle_cli/handler.py b/packages/tangle-cli/src/tangle_cli/handler.py index 03b7a59..00a0d1b 100644 --- a/packages/tangle-cli/src/tangle_cli/handler.py +++ b/packages/tangle-cli/src/tangle_cli/handler.py @@ -3,9 +3,11 @@ from __future__ import annotations from collections.abc import Callable, Mapping +from contextlib import AbstractContextManager from typing import Any from .api_transport import default_base_url +from .cli_helpers import surface_http_errors from .logger import Logger, get_default_logger @@ -94,3 +96,13 @@ def _require_client(self) -> Any: if client is None: raise self._required_client_error_type(self._required_client_error_message) return client + + def _http_error_type(self) -> type[Exception]: + """Exception type used to re-raise a formatted API HTTP failure.""" + + return self._required_client_error_type + + def _surface_http_errors(self) -> AbstractContextManager[None]: + """Context manager re-raising API ``HTTPError`` as concise, formatted failures.""" + + return surface_http_errors(self._http_error_type()) diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_run_annotations.py b/packages/tangle-cli/src/tangle_cli/pipeline_run_annotations.py index 2539b7a..469b0a8 100644 --- a/packages/tangle-cli/src/tangle_cli/pipeline_run_annotations.py +++ b/packages/tangle-cli/src/tangle_cli/pipeline_run_annotations.py @@ -5,11 +5,14 @@ from typing import Any from .handler import TangleCliHandler +from .pipeline_run_manager import PipelineRunError class AnnotationManager(TangleCliHandler): """Manage annotations on Tangle pipeline runs.""" + _required_client_error_type = PipelineRunError + @staticmethod def to_plain(value: Any) -> Any: if hasattr(value, "to_dict"): @@ -19,7 +22,8 @@ def to_plain(value: Any) -> Any: return value def list_annotations(self, run_id: str) -> dict[str, Any]: - annotations = self.to_plain(self._require_client().pipeline_runs_annotations(run_id)) or {} + with self._surface_http_errors(): + annotations = self.to_plain(self._require_client().pipeline_runs_annotations(run_id)) or {} if not isinstance(annotations, dict): annotations = dict(annotations) return { @@ -30,11 +34,13 @@ def list_annotations(self, run_id: str) -> dict[str, Any]: } def set_annotation(self, run_id: str, key: str, value: Any = None) -> dict[str, Any]: - self._require_client().pipeline_runs_put_annotations(run_id, key, value=value) + with self._surface_http_errors(): + self._require_client().pipeline_runs_put_annotations(run_id, key, value=value) return {"status": "success", "run_id": run_id, "key": key, "value": value} def delete_annotation(self, run_id: str, key: str) -> dict[str, Any]: - self._require_client().pipeline_runs_delete_annotations(run_id, key) + with self._surface_http_errors(): + self._require_client().pipeline_runs_delete_annotations(run_id, key) return {"status": "success", "run_id": run_id, "key": key} diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_run_details.py b/packages/tangle-cli/src/tangle_cli/pipeline_run_details.py index 3591b23..fb85493 100644 --- a/packages/tangle-cli/src/tangle_cli/pipeline_run_details.py +++ b/packages/tangle-cli/src/tangle_cli/pipeline_run_details.py @@ -11,6 +11,9 @@ from concurrent.futures import TimeoutError as FutureTimeoutError from typing import Any +import requests + +from .cli_helpers import format_http_error from .handler import TangleCliHandler @@ -164,6 +167,8 @@ def get_graph_state_output( results.append(future.result(timeout=timeout)) except FutureTimeoutError: results.append(_error_result(run_id, f"timeout after {timeout}s")) + except requests.HTTPError as exc: + results.append(_error_result(run_id, format_http_error(exc))) except Exception as exc: results.append(_error_result(run_id, str(exc))) finally: diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_run_manager.py b/packages/tangle-cli/src/tangle_cli/pipeline_run_manager.py index 24742fa..1649409 100644 --- a/packages/tangle-cli/src/tangle_cli/pipeline_run_manager.py +++ b/packages/tangle-cli/src/tangle_cli/pipeline_run_manager.py @@ -617,6 +617,9 @@ def __post_init__(self) -> None: if self.hooks is not self: setattr(self.hooks, "client", self.client) + def _http_error_type(self) -> type[Exception]: + return PipelineRunError + @staticmethod def to_plain(value: Any) -> Any: if isinstance(value, Mapping): @@ -1137,7 +1140,8 @@ def submit_prepared_body( self.hooks.before_submit_context(submit_context) client = self._require_client() try: - response = self.to_plain(client.pipeline_runs_create(body=body)) + with self._surface_http_errors(): + response = self.to_plain(client.pipeline_runs_create(body=body)) except Exception as exc: if notify_submit_error: self.hooks.on_submit_error(exc, context=submit_context) @@ -1224,12 +1228,13 @@ def submit_pipeline( return self.submit_prepared_payload(payload, pipeline_path=pipeline_path, attempt=attempt) def get_run(self, run_id: str, *, include_execution_stats: bool = True) -> dict[str, Any]: - return self.to_plain( - self.client.pipeline_runs_get( - run_id, - include_execution_stats=include_execution_stats, + with self._surface_http_errors(): + return self.to_plain( + self.client.pipeline_runs_get( + run_id, + include_execution_stats=include_execution_stats, + ) ) - ) def get_run_details( self, @@ -1240,26 +1245,33 @@ def get_run_details( include_implementations: bool = False, execution_id: str | None = None, ) -> dict[str, Any]: - return PipelineRunDetails(client=self.client).get_run_details_output( - run_id, - include_implementations=include_implementations, - include_annotations=include_annotations, - include_execution_state=include_execution_state, - execution_id=execution_id, - ) + with self._surface_http_errors(): + return PipelineRunDetails(client=self.client).get_run_details_output( + run_id, + include_implementations=include_implementations, + include_annotations=include_annotations, + include_execution_state=include_execution_state, + execution_id=execution_id, + ) def cancel_run(self, run_id: str) -> dict[str, Any]: - return self.to_plain(self.client.pipeline_runs_cancel(run_id)) or {"id": run_id, "cancelled": True} + with self._surface_http_errors(): + cancelled = self.to_plain(self.client.pipeline_runs_cancel(run_id)) + return cancelled or {"id": run_id, "cancelled": True} def graph_state(self, execution_id: str) -> Mapping[str, Any] | Any: - graph_state = self.client.executions_graph_execution_state(execution_id) + with self._surface_http_errors(): + graph_state = self.client.executions_graph_execution_state(execution_id) return self.to_plain(graph_state) def graph_state_output(self, run_ids: list[str], *, timeout: float = 30.0) -> dict[str, Any]: + # Per-run failures are reported in each result's "error" field rather + # than raised, so no HTTP-error surfacing is needed at this boundary. return PipelineRunDetails(client=self.client).get_graph_state_output(run_ids, timeout=timeout) def logs(self, execution_id: str) -> dict[str, Any]: - return self.to_plain(self.hooks.fetch_logs(self.client, execution_id)) + with self._surface_http_errors(): + return self.to_plain(self.hooks.fetch_logs(self.client, execution_id)) def search_runs( self, @@ -1270,15 +1282,16 @@ def search_runs( include_pipeline_names: bool | None = None, include_execution_stats: bool | None = True, ) -> dict[str, Any]: - return self.to_plain( - self.client.pipeline_runs_list( - page_token=page_token, - filter=filter, - filter_query=filter_query, - include_pipeline_names=include_pipeline_names, - include_execution_stats=include_execution_stats, + with self._surface_http_errors(): + return self.to_plain( + self.client.pipeline_runs_list( + page_token=page_token, + filter=filter, + filter_query=filter_query, + include_pipeline_names=include_pipeline_names, + include_execution_stats=include_execution_stats, + ) ) - ) def search_pipeline_runs( self, @@ -1293,17 +1306,18 @@ def search_pipeline_runs( limit: int = 10, page_token: str | None = None, ) -> dict[str, Any]: - return PipelineRunSearch(client=self.client, logger=self.logger).search( - name=name, - created_by=created_by, - annotations=annotations, - start_date=start_date, - end_date=end_date, - local_time=local_time, - query=query, - limit=limit, - page_token=page_token, - ) + with self._surface_http_errors(): + return PipelineRunSearch(client=self.client, logger=self.logger).search( + name=name, + created_by=created_by, + annotations=annotations, + start_date=start_date, + end_date=end_date, + local_time=local_time, + query=query, + limit=limit, + page_token=page_token, + ) def export_run( self, @@ -1312,7 +1326,8 @@ def export_run( *, dehydrate: bool = False, ) -> dict[str, Any]: - task_spec = self.client.get_run_pipeline_spec(run_id) + with self._surface_http_errors(): + task_spec = self.client.get_run_pipeline_spec(run_id) if task_spec is None: raise PipelineRunError(f"No pipeline spec found for run {run_id}") raw = getattr(task_spec, "raw", None) @@ -1328,12 +1343,13 @@ def export_run( if dehydrate and output is None: raise PipelineRunError("--dehydrate requires --output") if dehydrate: - spec = PipelineDehydrator( - remembered_choices={"": DehydrateChoice.AUTO}, - output_file=output, - client=self.client, - logger=self.logger, - ).dehydrate(spec) + with self._surface_http_errors(): + spec = PipelineDehydrator( + remembered_choices={"": DehydrateChoice.AUTO}, + output_file=output, + client=self.client, + logger=self.logger, + ).dehydrate(spec) content = dump_yaml(spec) if output is None: return {"run_id": run_id, "pipeline": spec, "yaml": content, "dehydrated": dehydrate} diff --git a/packages/tangle-cli/src/tangle_cli/pipeline_runs_cli.py b/packages/tangle-cli/src/tangle_cli/pipeline_runs_cli.py index 2da0eb5..a8cd831 100644 --- a/packages/tangle-cli/src/tangle_cli/pipeline_runs_cli.py +++ b/packages/tangle-cli/src/tangle_cli/pipeline_runs_cli.py @@ -120,7 +120,10 @@ def _run_annotation_action(config: str | None, cli_base_url: str | None, specs: client=_api_client(args, cli_base_url=cli_base_url, command_name="pipeline-run annotation commands"), logger=logger, ) - print_json(fn(manager, args)) + try: + print_json(fn(manager, args)) + except PipelineRunError as exc: + raise SystemExit(str(exc)) from exc finally: finalize_logs() diff --git a/tests/test_sdk_http_errors.py b/tests/test_sdk_http_errors.py new file mode 100644 index 0000000..f327e68 --- /dev/null +++ b/tests/test_sdk_http_errors.py @@ -0,0 +1,960 @@ +"""SDK-layer HTTP status error handling. + +SDK commands raise ``requests.HTTPError`` on non-2xx responses. These tests +cover the shared formatter and confirm the pipeline-runs dispatch points +(read/query, submit, and annotation commands) render a clean nonzero error +instead of a raw traceback, while client-internal recovery (the 404 run-id -> +execution-id fallback and post-submit run recovery) is preserved end to end. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +import pytest +import requests +import yaml + +from tangle_cli import cli, pipeline_runs_cli +from tangle_cli.cli_helpers import _HTTP_ERROR_BODY_LIMIT, format_http_error +from tangle_cli.pipeline_run_details import PipelineRunDetails +from tangle_cli.pipeline_run_manager import PipelineRunError, PipelineRunHooks, PipelineRunManager + + +def _http_error( + *, + status_code: int = 500, + reason: str = "Internal Server Error", + method: str = "GET", + url: str = "https://api.test/api/pipeline_runs/missing", + body: str = "boom", +) -> requests.HTTPError: + resp = requests.Response() + resp.status_code = status_code + resp.reason = reason + resp._content = body.encode("utf-8") + resp.request = requests.Request(method, url).prepare() + return requests.HTTPError(f"{status_code} error", response=resp) + + +# -------------------------------------------------------------------------- +# Formatter +# -------------------------------------------------------------------------- + + +def test_format_http_error_includes_status_reason_method_url_and_body() -> None: + message = format_http_error( + _http_error(status_code=404, reason="Not Found", method="GET", url="https://api.test/x", body="missing run") + ) + assert message == "Tangle API request failed (404 Not Found) for GET https://api.test/x: missing run" + + +def test_format_http_error_omits_body_when_empty() -> None: + message = format_http_error(_http_error(status_code=500, reason="Server Error", body=" ")) + assert message == "Tangle API request failed (500 Server Error) for GET https://api.test/api/pipeline_runs/missing" + + +def test_format_http_error_truncates_long_body() -> None: + message = format_http_error(_http_error(body="x" * 5000)) + _, _, rendered_body = message.partition(": ") + assert rendered_body == "x" * _HTTP_ERROR_BODY_LIMIT + "... (truncated)" + + +def test_format_http_error_collapses_body_to_one_line() -> None: + message = format_http_error(_http_error(body='{\n "error": "bad\r\nrequest",\n\t"detail": "x"\n}')) + assert "\n" not in message + assert "\r" not in message + assert "\t" not in message + assert message.endswith(': { "error": "bad request", "detail": "x" }') + + +def test_format_http_error_without_response_falls_back_to_str() -> None: + exc = requests.HTTPError("opaque failure") + assert format_http_error(exc) == "Tangle API request failed: opaque failure" + + +def test_format_http_error_redacts_url_userinfo_and_credential_query() -> None: + message = format_http_error( + _http_error( + status_code=401, + reason="Unauthorized", + method="GET", + url="https://user:s3cret@api.test/x?access_token=abc&page=2", + body="nope", + ) + ) + assert "s3cret" not in message + assert "user:" not in message + assert "abc" not in message + assert "access_token=" in message + assert "page=2" in message + # The userinfo is replaced rather than silently dropped, so the message still + # says a credential was in the URL -- which is what a caller must fix. + assert message.startswith( + "Tangle API request failed (401 Unauthorized) for GET https://@api.test/x?" + ) + + +def test_format_http_error_redacts_sensitive_keys_in_json_body_before_truncation() -> None: + payload = { + "detail": "boom", + "token": "super-secret-token", + "nested": {"password": "hunter2", "ok": "keep"}, + } + message = format_http_error( + _http_error(status_code=500, reason="Server Error", body=json.dumps(payload)) + ) + assert "super-secret-token" not in message + assert "hunter2" not in message + assert "" in message + assert "boom" in message + assert "keep" in message + + +@pytest.mark.parametrize( + "url, leaked, expected_redacted", + [ + ( + "https://api.test/o?X-Amz-Credential=AKIALEAK&X-Amz-Signature=DEADBEEFSIG&X-Amz-Expires=900", + ["DEADBEEFSIG", "AKIALEAK"], + ["X-Amz-Signature=", "X-Amz-Credential="], + ), + ( + "https://api.test/x?sig=SECRETSIG&signature=SECRET2&page=2", + ["SECRETSIG", "SECRET2"], + ["sig=", "signature="], + ), + ( + "https://api.test/y?api_key=APIKEYLEAK&oauth_token=OAUTHLEAK&keep=1", + ["APIKEYLEAK", "OAUTHLEAK"], + ["api_key=", "oauth_token="], + ), + ( + "https://api.test/z?awsaccesskeyid=AKIAX&googleaccessid=GOOGLEID&next=5", + ["AKIAX", "GOOGLEID"], + ["awsaccesskeyid=", "googleaccessid="], + ), + ], +) +def test_format_http_error_redacts_signed_url_query_keys( + url: str, leaked: list[str], expected_redacted: list[str] +) -> None: + message = format_http_error(_http_error(status_code=401, reason="Unauthorized", url=url, body="nope")) + for secret in leaked: + assert secret not in message + for fragment in expected_redacted: + assert fragment in message + + +def test_format_http_error_preserves_non_sensitive_query_keys() -> None: + message = format_http_error( + _http_error(url="https://api.test/p?page=2&design=cool&assignment=1", body="nope") + ) + assert "page=2" in message + assert "design=cool" in message + assert "assignment=1" in message + + +@pytest.mark.parametrize( + "body, leaked, expected_redacted", + [ + ("credential=BODYSECRET&foo=bar", "BODYSECRET", "credential="), + ("password=hunter2; note=ok", "hunter2", "password="), + ("token: sk-live-0123456789ABCDEF", "sk-live-0123456789ABCDEF", "token: "), + ("oauth_token=OAUTHPLAIN api_key=APIPLAIN", "OAUTHPLAIN", "oauth_token="), + ("Signature: PLAINSIG12345", "PLAINSIG12345", "Signature: "), + ], +) +def test_format_http_error_redacts_secrets_in_non_json_body( + body: str, leaked: str, expected_redacted: str +) -> None: + message = format_http_error(_http_error(status_code=400, reason="Bad Request", body=body)) + assert leaked not in message + assert expected_redacted in message + + +def test_format_http_error_preserves_non_sensitive_non_json_body() -> None: + # Non-sensitive assignments and surrounding prose stay intact so diagnostics + # remain useful; only the value of a credential-named field is cut. + message = format_http_error( + _http_error( + status_code=502, + reason="Bad Gateway", + body="Upstream rejected the request; status=failed detail=useful page=2", + ) + ) + assert message.endswith( + ": Upstream rejected the request; status=failed detail=useful page=2" + ) + + +def test_format_http_error_redacts_credential_assignment_regardless_of_value() -> None: + # A field explicitly named ``credential`` loses its value whether or not the + # value looks opaque: gating on "does this look like a secret?" would let a + # short or wordlike credential (``token: hunter2``) through. The field name is + # kept, so the message still says which field the backend objected to. + message = format_http_error( + _http_error(status_code=502, reason="Bad Gateway", body="Invalid credential: invalid") + ) + assert message.endswith(": Invalid credential: ") + + +def test_format_http_error_fails_closed_on_malformed_url() -> None: + # A URL that urlsplit rejects (invalid IPv6) must fall closed to + # rather than leaking the raw value. Set it directly on the response since + # requests refuses to prepare such a URL. + resp = requests.Response() + resp.status_code = 500 + resp.reason = "Server Error" + resp._content = b"boom" + resp.url = "https://[oops/x" + resp.request = None + message = format_http_error(requests.HTTPError("500 error", response=resp)) + assert "[oops" not in message + assert "" in message + + +@pytest.mark.parametrize( + "url", + [ + # ``urlsplit`` defers authority validation until the port is read, so + # these reach the formatter as a live ValueError rather than a parse + # failure. The authority is dropped; the path stays diagnostic. + "https://api.test:bad/x", + "https://user:pw@api.test:99999/x", + "https://api.test:-1/x", + "https://[::1]:nope/x", + ], +) +def test_format_http_error_fails_closed_on_unparsable_port(url: str) -> None: + resp = requests.Response() + resp.status_code = 502 + resp.reason = "Bad Gateway" + resp._content = b"boom" + resp.url = url + resp.request = None + message = format_http_error(requests.HTTPError("502 error", response=resp)) + assert "pw" not in message + assert "" in message + + +def test_format_http_error_redacts_userinfo_query_and_body_credentials_together() -> None: + message = format_http_error( + _http_error( + status_code=401, + reason="Unauthorized", + method="POST", + url="https://alice:hunter2@api.test/x?access_token=QUERYSECRET", + body="credential=BODYSECRET", + ) + ) + assert "alice" not in message + assert "hunter2" not in message + assert "QUERYSECRET" not in message + assert "BODYSECRET" not in message + assert message == ( + "Tangle API request failed (401 Unauthorized) for " + "POST https://@api.test/x?access_token=: credential=" + ) + + +@pytest.mark.parametrize( + "field", + [ + "access_token", + "refresh_token", + "id_token", + "sessionToken", + "accessToken", + "client_secret", + "user_credential", + "tangle_access_token", + "X-Access-Token", + "myApiKey", + "aws_secret_access_key", + "AwsAccessKeyId", + ], +) +@pytest.mark.parametrize("separator", ["=", ": ", ":", " = ", '":"']) +def test_format_http_error_redacts_affixed_credential_fields_in_non_json_body( + field: str, separator: str +) -> None: + """Prefixed, snake_case, camelCase, and quoted spellings are all covered. + + The field name is recovered by a bounded lookbehind and judged by its trailing + tokens, so no alternation has to enumerate ``tangle_access_token`` or + ``sessionToken`` for their values to be cut. + """ + + secret = "s3cretOpaqueValue123" + message = format_http_error( + _http_error(status_code=400, reason="Bad Request", body=f"rejected {field}{separator}{secret}") + ) + assert secret not in message + assert "" in message + assert field in message + + +@pytest.mark.parametrize( + "body", + [ + "Authorization: Bearer s3cretOpaqueValue123", + "Authorization: Basic YWxpY2U6aHVudGVyMg==", + "authorization: bearer s3cretOpaqueValue123", + "Proxy-Authorization: Bearer s3cretOpaqueValue123", + "rejected header: Bearer s3cretOpaqueValue123", + "Digest s3cretOpaqueValue123 was rejected", + ], +) +def test_format_http_error_redacts_auth_scheme_credentials(body: str) -> None: + """The scheme name is diagnostic and kept; the credential after it is cut.""" + + message = format_http_error(_http_error(status_code=401, reason="Unauthorized", body=body)) + assert "s3cretOpaqueValue123" not in message + assert "YWxpY2U6aHVudGVyMg==" not in message + assert "" in message + + +@pytest.mark.parametrize( + ("body", "expected"), + [ + ("Bearer abc", "Bearer "), + ("Bearer a", "Bearer "), + ("Bearer token", "Bearer "), + ("Bearer TOKEN", "Bearer "), + ("Basic secret", "Basic "), + ("Basic Access", "Basic "), + ("Basic credentials", "Basic "), + ("Basic hunter", "Basic "), + ("bearer abc", "bearer "), + ("BASIC secret", "BASIC "), + ("bEaReR x", "bEaReR "), + ("digest word was rejected", "digest was rejected"), + ("Digest word was rejected", "Digest was rejected"), + ("rejected header: Bearer abc", "rejected header: Bearer "), + ("Bearer abc.", "Bearer "), + ("server rejected Basic c2VjcmV0, retry later", "server rejected Basic , retry later"), + ("Negotiate opaquevalue", "Negotiate "), + ("Basic authentication failed", "Basic failed"), + ("Bearer token expired", "Bearer token "), + ("Bearer token", "Bearer "), + ("

Authorization: bearer token

", "

Authorization: bearer

"), + ], +) +def test_format_http_error_redacts_short_word_like_scheme_credentials( + body: str, expected: str +) -> None: + """An explicit scheme always redacts what follows; value shape is not trusted.""" + + message = format_http_error(_http_error(status_code=401, reason="Unauthorized", body=body)) + assert message.endswith(f": {expected}") + + +@pytest.mark.parametrize( + ("body", "expected"), + [ + ("Bearer Bearer sk", "Bearer Bearer "), + ("Basic Bearer sk", "Basic Bearer "), + ("bearer BASIC sk", "bearer BASIC "), + ("Bearer Bearer Bearer abc", "Bearer Bearer Bearer "), + ("Bearer Basic Digest Negotiate abc", "Bearer Basic Digest Negotiate "), + ("Token Bearer sk", "Token Bearer "), + ("Bearer Token sk", "Bearer Token "), + ("Bearer token s3cretOpaqueValue123", "Bearer token "), + ("Bearer token a", "Bearer token "), + ("Bearer token count", "Bearer token "), + ("bearer TOKEN s3cret", "bearer TOKEN "), + ("oauth Bearer sk", "oauth Bearer "), + ("Basic Bearer jwt", "Basic Bearer "), + ("Basic Bearer jwt eyJhbGciOiJIUzI1NiJ9.p.s", "Basic Bearer jwt "), + ("

Bearer token s3cret

", "

Bearer token

"), + ( + "Authorization: Bearer token s3cretOpaqueValue123", + "Authorization: Bearer token ", + ), + ("Token Token s3cretOpaqueValue123", "Token Token "), + ("Authorization: Basic Bearer sk", "Authorization: Basic Bearer "), + ("rejected: bEaReR bAsIc sk", "rejected: bEaReR bAsIc "), + ("

Bearer Bearer sk

", "

Bearer Bearer

"), + ( + "server said Basic Bearer sk, retry later", + "server said Basic Bearer , retry later", + ), + ], +) +def test_format_http_error_redacts_credentials_after_chained_schemes( + body: str, expected: str +) -> None: + """A doubled scheme cannot shield the credential from a non-overlapping scan.""" + + message = format_http_error(_http_error(status_code=401, reason="Unauthorized", body=body)) + assert message.endswith(f": {expected}") + + +@pytest.mark.parametrize("depth", [1, 2, 3, 5, 8, 13, 20, 50, 100]) +@pytest.mark.parametrize("link", ["Bearer ", "Bearer token ", "bAsIc BEARER "]) +def test_format_http_error_redacts_credential_after_any_chain_depth( + depth: int, link: str +) -> None: + chain = link * depth + message = format_http_error( + _http_error(status_code=401, reason="Unauthorized", body=f"{chain}s3cretOpaqueValue123") + ) + assert "s3cretOpaqueValue123" not in message + assert message.endswith(f": {chain}") + + +@pytest.mark.parametrize( + ("leaf", "expected"), + [ + ("Basic Bearer sk", "Basic Bearer "), + ("Bearer token s3cretOpaqueValue123", "Bearer token "), + ], +) +def test_format_http_error_redacts_chained_schemes_in_json_leaves( + leaf: str, expected: str +) -> None: + message = format_http_error( + _http_error(status_code=401, reason="Unauthorized", body=json.dumps({"detail": leaf})) + ) + assert expected in message + assert " sk" not in message + assert "s3cretOpaqueValue123" not in message + + +@pytest.mark.parametrize( + ("body", "expected"), + [ + ("Bearer\ns3cretOpaqueValue123", "Bearer "), + ("Bearer\ts3cretOpaqueValue123", "Bearer "), + ("Bearer\r\ns3cretOpaqueValue123", "Bearer "), + ("Bearer\nBearer\ns3cretOpaqueValue123", "Bearer Bearer "), + ("Bearer\ntoken\ns3cretOpaqueValue123", "Bearer token "), + ("Authorization:\nBearer\ns3cretOpaqueValue123", "Authorization: Bearer "), + ( + "Authorization:\nBearer\ntoken\ns3cretOpaqueValue123", + "Authorization: Bearer token ", + ), + ("password:\nhunter2secret", "password: "), + ("access_token =\n s3cretOpaqueValue123", "access_token = "), + ], +) +def test_format_http_error_redacts_across_newline_separators(body: str, expected: str) -> None: + """Display collapses whitespace after redaction; a newline must not hide a value.""" + + message = format_http_error(_http_error(status_code=401, reason="Unauthorized", body=body)) + assert "s3cretOpaqueValue123" not in message + assert "hunter2secret" not in message + assert message.endswith(f": {expected}") + + +@pytest.mark.parametrize( + "payload", + [ + {"detail": "Bearer token"}, + {"detail": "basic Access"}, + {"errors": [{"message": "rejected: Bearer TOKEN"}]}, + ], +) +def test_format_http_error_redacts_word_like_scheme_values_in_json_leaves( + payload: dict[str, Any], +) -> None: + message = format_http_error( + _http_error(status_code=401, reason="Unauthorized", body=json.dumps(payload)) + ) + assert "" in message + assert "token" not in message.lower().split(": ", 1)[1] + assert "access" not in message.lower() + + +@pytest.mark.parametrize( + "body", + [ + 'Basic realm="api"', + "Bearer realm=api", + 'basic realm="api"', + 'BEARER realm="api"', + 'Bearer realm="example", error="invalid_token", error_description="expired"', + 'Digest realm="tangle", qop="auth", algorithm=MD5, nonce="f2a9"', + 'OAuth realm="Example"', + "Bearer Bearer realm=api", + "Bearer token realm=api", + 'Basic Digest realm="api"', + ], +) +def test_format_http_error_keeps_auth_challenge_parameters(body: str) -> None: + """A ``WWW-Authenticate`` challenge carries directives, not a credential.""" + + message = format_http_error(_http_error(status_code=401, reason="Unauthorized", body=body)) + assert message.endswith(f": {body}") + assert "" not in message + + +@pytest.mark.parametrize( + "payload", + [ + {"detail": "invalid token=s3cretOpaqueValue123 supplied"}, + {"message": "Authorization: Bearer s3cretOpaqueValue123"}, + {"detail": ["access_token=s3cretOpaqueValue123"]}, + {"error": {"message": "password: s3cretOpaqueValue123"}}, + {"errors": [{"detail": "client_secret=s3cretOpaqueValue123"}]}, + {"detail": "password=//s3cretOpaqueValue123"}, + {"error": {"message": "token: //s3cretOpaqueValue123"}}, + ], +) +def test_format_http_error_redacts_sensitive_assignments_in_json_string_leaves( + payload: dict[str, Any], +) -> None: + """A harmless outer key can still quote a credential back at us.""" + + message = format_http_error( + _http_error(status_code=400, reason="Bad Request", body=json.dumps(payload)) + ) + assert "s3cretOpaqueValue123" not in message + assert "" in message + + +@pytest.mark.parametrize( + "body", + [ + "password=//s3cretOpaqueValue123", + "client_secret=//czNjcmV0T3BhcXVlYjY0dg==", + "token: //s3cretOpaqueValue123", + "password=/s3cretOpaqueValue123", + '{"password": "//s3cretOpaqueValue123', + "a=1&password=//s3cretOpaqueValue123&b=2", + "

password=//s3cretOpaqueValue123

", + "token://s3cretOpaqueValue123", + ], +) +def test_format_http_error_redacts_slash_prefixed_credential_values(body: str) -> None: + """A sensitive field's value is not exempted by beginning with slashes.""" + + message = format_http_error(_http_error(status_code=400, reason="Bad Request", body=body)) + assert "s3cretOpaqueValue123" not in message + assert "czNjcmV0" not in message + assert "" in message + + +@pytest.mark.parametrize( + "body", + [ + "see https://api.test/callback ok", + "endpoint=https://api.test/path retry", + "docs: https://api.test/help", + "path: /var/log/app.log", + "ratio: 1//2 of requests", + ], +) +def test_format_http_error_keeps_non_sensitive_slash_assignments(body: str) -> None: + message = format_http_error(_http_error(status_code=400, reason="Bad Request", body=body)) + assert message.endswith(f": {body}") + assert "" not in message + + +@pytest.mark.parametrize( + "body, kept", + [ + ("see https://api.test/cb?access_token=s3cretOpaqueValue123", "api.test/cb"), + ("see https://alice:s3cretOpaqueValue123@api.test/cb", "api.test/cb"), + ('{"detail": "https://api.test/cb?api_key=s3cretOpaqueValue123"}', "api.test/cb"), + ('{"detail": "https://alice:s3cretOpaqueValue123@api.test/cb"}', "api.test/cb"), + ('x', "api.test/c"), + ("redirect to /cb#access_token=s3cretOpaqueValue123&token_type=Bearer", "/cb"), + ("connect alice:s3cretOpaqueValue123@db.internal failed", "db.internal"), + ], +) +def test_format_http_error_redacts_urls_reflected_in_body(body: str, kept: str) -> None: + """A credential reflected inside a URL in the body loses only the credential. + + The scheme, host, and path survive, because "wrong host" and "expired grant" + are different failures and the message has to tell them apart. + """ + + message = format_http_error(_http_error(status_code=400, reason="Bad Request", body=body)) + assert "s3cretOpaqueValue123" not in message + assert kept in message + + +def test_format_http_error_keeps_host_and_path_of_presigned_url_field() -> None: + """A ``signed_url`` field is scrubbed structurally rather than dropped whole.""" + + payload = { + "presigned_url": ( + "https://bucket.s3.amazonaws.com/obj?X-Amz-Signature=DEADBEEFSIG" + "&X-Amz-Expires=900&X-Amz-Date=20240101T000000Z" + ) + } + message = format_http_error( + _http_error(status_code=403, reason="Forbidden", body=json.dumps(payload)) + ) + assert "DEADBEEFSIG" not in message + assert "bucket.s3.amazonaws.com/obj" in message + assert "X-Amz-Expires=900" in message + assert "X-Amz-Date=20240101T000000Z" in message + + +@pytest.mark.parametrize( + "query", + [ + # SigV4 parameters that are not the credential stay readable, because an + # expired or misdated link is diagnosed from exactly these. + "X-Amz-Date=20240101T000000Z", + "X-Amz-Expires=900", + "X-Amz-Algorithm=AWS4-HMAC-SHA256", + "X-Amz-SignedHeaders=host", + # Field names that merely contain a credential word as a non-final token. + "max_tokens=500", + "tokenizer=gpt2", + "token_count=17", + "tokens_used=42", + "function_signature=fn", + "signatureVersion=4", + "access_key_id_format=hex", + "password_policy=strict", + "private_key_path=etc-k.pem", + "secretaryEmail=alice", + "session_id=abc123", + "requestId=r-1", + "keyboard=qwerty", + ], +) +def test_format_http_error_does_not_over_redact_diagnostic_query_keys(query: str) -> None: + message = format_http_error( + _http_error(status_code=400, reason="Bad Request", url=f"https://api.test/x?{query}", body="nope") + ) + assert query in message + assert "" not in message + + +@pytest.mark.parametrize( + "body", + [ + "max_tokens=500 exceeds the model limit", + "tokenizer=gpt2 is unsupported", + "function_signature=fn(a,b) is invalid", + "token_count=17 below minimum", + "Invalid operation id", + "Token count exceeded", + "token count: 42 exceeds the limit", + "Token 12345 expired", + "Supported schemes: Bearer, Basic, and Digest", + "max tokens exceeded", + "Token Token count", + "token oauth flow enabled", + ], +) +def test_format_http_error_does_not_over_redact_diagnostic_prose(body: str) -> None: + message = format_http_error(_http_error(status_code=400, reason="Bad Request", body=body)) + assert message.endswith(f": {body}") + + +@pytest.mark.parametrize( + "body_factory", + [ + pytest.param( + lambda pad, secret: json.dumps({"access_token": secret, "pad": pad}), id="json" + ), + pytest.param(lambda pad, secret: f"{pad}&access_token={secret}", id="form"), + pytest.param(lambda pad, secret: f"{pad} access_token={secret}", id="text"), + ], +) +def test_format_http_error_redacts_before_truncation(body_factory: Any) -> None: + """Redaction runs on the whole body, not on the surviving prefix. + + The secret is long enough that truncating first would keep several hundred of + its characters, and the pad is sized so that once the assignment collapses to + ```` the message no longer needs truncating at all. Truncate-first + therefore leaks and also loses the placeholder. + """ + + secret = "S3cret" + "x" * 400 + pad = "detail=useful " * 135 + body = body_factory(pad, secret) + assert len(body) > _HTTP_ERROR_BODY_LIMIT + message = format_http_error(_http_error(status_code=500, reason="Server Error", body=body)) + assert secret not in message + assert "S3cret" not in message + assert "" in message + + +def test_format_http_error_survives_deeply_nested_json_body() -> None: + """A few kilobytes of ``[[[[...]]]]`` must not exhaust the interpreter stack. + + Two safe renderings are possible, and which one appears is decided by the + interpreter rather than by this formatter: CPython's JSON scanner carries its + own recursion limit, so on some supported versions the body parses and the + depth-bounded walk replaces the unexamined subtree with its sentinel, while on + others ``json.loads`` gives up first and the body is scrubbed as text instead. + Both satisfy the contract that matters -- the call returns, the secret is gone, + and the message stays one bounded line -- so accepting either keeps this test + portable across every declared Python version rather than pinning the parser's + recursion limit. + """ + + body = "[" * 9000 + '"access_token=s3cretOpaqueValue123"' + "]" * 9000 + message = format_http_error(_http_error(status_code=500, reason="Server Error", body=body)) + assert "s3cretOpaqueValue123" not in message + assert "\n" not in message + assert "\r" not in message + _, _, rendered_body = message.partition(": ") + assert len(rendered_body) <= _HTTP_ERROR_BODY_LIMIT + len("... (truncated)") + depth_bounded = "nesting too deep" in rendered_body + text_fallback = rendered_body == "[" * _HTTP_ERROR_BODY_LIMIT + "... (truncated)" + assert depth_bounded or text_fallback + + +def test_format_http_error_redacts_every_key_of_a_wide_json_body() -> None: + payload = {f"tenant{index}_access_token": "s3cretOpaqueValue123" for index in range(5000)} + message = format_http_error( + _http_error(status_code=500, reason="Server Error", body=json.dumps(payload)) + ) + assert "s3cretOpaqueValue123" not in message + + +@pytest.mark.parametrize( + "body", + [ + # One unbroken run per shape the scanners anchor on: ``=``/``:`` + # separators, ``@`` userinfo, and ``://`` URL starts. A quadratic scan + # would not return on these. + ("token=" + "a" * 40 + " ") * 20000 + "password: s3cretOpaqueValue123", + "

" + "a@" * 100000 + "

api_key=s3cretOpaqueValue123

", + "x://" * 50000 + " api_key=s3cretOpaqueValue123", + "a" * 200000 + ":" + "b" * 200000, + "%" * 100000 + "state=access_token%253Ds3cretOpaqueValue123", + "Bearer " * 60000 + "s3cretOpaqueValue123", + "Bearer x " * 30000 + "api_key=s3cretOpaqueValue123", + "Bearer token " * 30000 + "s3cretOpaqueValue123", + "token " * 100000 + "api_key=s3cretOpaqueValue123", + ], +) +def test_format_http_error_stays_linear_on_adversarial_bodies(body: str) -> None: + import time + + start = time.perf_counter() + message = format_http_error(_http_error(status_code=500, reason="Server Error", body=body)) + elapsed = time.perf_counter() - start + # A generous ceiling: these complete in well under a second, so anything + # near it means a scan went superlinear. + assert elapsed < 10.0 + assert "s3cretOpaqueValue123" not in message + assert len(message) < _HTTP_ERROR_BODY_LIMIT + 200 + + +@pytest.mark.parametrize( + "url", + [ + # A credential buried under extra percent-encoding layers is not legible + # to a scan that decodes once, so the parameter fails closed instead. + "https://api.test/x?state=access_token%253Ds3cretOpaqueValue123", + "https://api.test/x?next=alice%253As3cretOpaqueValue123%2540host", + "https://api.test/x?next=https%3A%2F%2Fh%2Fcb%3Faccess_token%3Ds3cretOpaqueValue123", + "https://api.test/x#access_token=s3cretOpaqueValue123&token_type=Bearer", + ], +) +def test_format_http_error_redacts_credentials_hidden_in_url_parameters(url: str) -> None: + message = format_http_error( + _http_error(status_code=401, reason="Unauthorized", url=url, body="nope") + ) + assert "s3cretOpaqueValue123" not in message + assert "api.test/x" in message + + +# -------------------------------------------------------------------------- +# pipeline-runs commands +# -------------------------------------------------------------------------- + + +def test_pipeline_runs_status_renders_http_error_without_traceback(monkeypatch) -> None: + class RaisingClient: + base_url = "https://api.test" + + def pipeline_runs_get(self, *args: Any, **kwargs: Any) -> Any: + raise _http_error(status_code=500, reason="Internal Server Error", body="kaboom") + + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: RaisingClient()) + app = cli.build_app() + + with pytest.raises(SystemExit) as exc_info: + app(["sdk", "pipeline-runs", "status", "missing-run"]) + + assert exc_info.value.code == ( + "Tangle API request failed (500 Internal Server Error) for " + "GET https://api.test/api/pipeline_runs/missing: kaboom" + ) + + +def test_pipeline_runs_details_preserves_404_execution_fallback(monkeypatch, capsys) -> None: + """A 404 the client recovers from must not be intercepted by the new catch.""" + + from tangle_cli.client import TangleApiClient + + def make_response(payload: Any, status_code: int) -> requests.Response: + resp = requests.Response() + resp.status_code = status_code + resp.reason = "Not Found" if status_code == 404 else "OK" + resp._content = b"" if payload is None else json.dumps(payload).encode("utf-8") + if payload is not None: + resp.headers["Content-Type"] = "application/json" + resp.request = requests.Request("GET", "https://api.test/x").prepare() + return resp + + execution_payload = { + "id": "missing-run", + "task_spec": {"componentRef": {"spec": {"name": "pipeline"}}}, + "child_task_execution_ids": {}, + "input_artifacts": {}, + "output_artifacts": {}, + } + + class FakeSession: + def __init__(self) -> None: + self.responses = [make_response(None, 404), make_response(execution_payload, 200)] + + def request(self, *args: Any, **kwargs: Any) -> requests.Response: + return self.responses.pop(0) + + real_client = TangleApiClient("https://api.test", session=FakeSession()) + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: real_client) + app = cli.build_app() + + with pytest.raises(SystemExit) as exc_info: + app(["sdk", "pipeline-runs", "details", "missing-run"]) + + assert exc_info.value.code in (0, None) + payload = json.loads(capsys.readouterr().out) + assert payload["run"]["id"] == "missing-run" + + +def test_pipeline_runs_submit_renders_http_error_without_traceback(monkeypatch, tmp_path: Path) -> None: + pipeline_path = tmp_path / "pipeline.yaml" + pipeline_path.write_text( + yaml.safe_dump({"name": "Demo", "implementation": {"graph": {"tasks": {}}}}), + encoding="utf-8", + ) + + class RaisingClient: + base_url = "https://api.test" + + def pipeline_runs_create(self, body: Any = None) -> Any: + raise _http_error( + status_code=403, + reason="Forbidden", + method="POST", + url="https://api.test/api/pipeline_runs", + body="denied", + ) + + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: RaisingClient()) + app = cli.build_app() + + with pytest.raises(SystemExit) as exc_info: + app( + [ + "sdk", + "pipeline-runs", + "submit", + str(pipeline_path), + "--no-hydrate", + "--submit-recovery-attempts", + "0", + ] + ) + + assert exc_info.value.code == ( + "Tangle API request failed (403 Forbidden) for POST https://api.test/api/pipeline_runs: denied" + ) + + +def test_submit_error_hook_receives_pipeline_run_error_with_http_cause() -> None: + class RaisingClient: + def pipeline_runs_create(self, body: Any = None) -> Any: + raise _http_error(status_code=500, reason="Internal Server Error", body="kaboom") + + errors: list[Exception] = [] + + class Hooks(PipelineRunHooks): + def on_submit_error(self, error: Exception, *, context: Any) -> None: + errors.append(error) + + manager = PipelineRunManager(client=RaisingClient(), hooks=Hooks()) + + with pytest.raises(PipelineRunError, match="kaboom"): + manager.submit_pipeline_spec( + {"name": "Explodes", "implementation": {"graph": {"tasks": {}}}}, + hydrate=False, + ) + + assert len(errors) == 1 + assert isinstance(errors[0], PipelineRunError) + assert isinstance(errors[0].__cause__, requests.HTTPError) + + +def test_pipeline_runs_annotations_list_renders_http_error_without_traceback(monkeypatch) -> None: + class RaisingClient: + base_url = "https://api.test" + + def pipeline_runs_annotations(self, id: str) -> Any: + raise _http_error( + status_code=500, + reason="Internal Server Error", + url="https://api.test/api/pipeline_runs/run-1/annotations", + body="kaboom", + ) + + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: RaisingClient()) + app = cli.build_app() + + with pytest.raises(SystemExit) as exc_info: + app(["sdk", "pipeline-runs", "annotations", "list", "run-1"]) + + assert exc_info.value.code == ( + "Tangle API request failed (500 Internal Server Error) for " + "GET https://api.test/api/pipeline_runs/run-1/annotations: kaboom" + ) + + +def test_pipeline_runs_annotations_set_renders_http_error_without_traceback(monkeypatch) -> None: + class RaisingClient: + base_url = "https://api.test" + + def pipeline_runs_put_annotations(self, id: str, key: str, value: Any = None) -> None: + raise _http_error( + status_code=409, + reason="Conflict", + method="PUT", + url="https://api.test/api/pipeline_runs/run-1/annotations/owner", + body="conflict", + ) + + monkeypatch.setattr(pipeline_runs_cli, "LazyTangleApiClient", lambda **kwargs: RaisingClient()) + app = cli.build_app() + + with pytest.raises(SystemExit) as exc_info: + app(["sdk", "pipeline-runs", "annotations", "set", "run-1", "owner", "bob"]) + + assert exc_info.value.code == ( + "Tangle API request failed (409 Conflict) for " + "PUT https://api.test/api/pipeline_runs/run-1/annotations/owner: conflict" + ) + + +def test_graph_state_output_reports_formatted_http_error_per_run() -> None: + class RaisingClient: + def pipeline_runs_get(self, run_id: str) -> Any: + raise _http_error( + status_code=500, + reason="Internal Server Error", + url="https://api.test/api/pipeline_runs/run-1", + body="kaboom", + ) + + result = PipelineRunDetails(client=RaisingClient()).get_graph_state_output(["run-1"]) + + assert result["results"][0]["error"] == ( + "Tangle API request failed (500 Internal Server Error) for " + "GET https://api.test/api/pipeline_runs/run-1: kaboom" + )