diff --git a/CHANGELOG.md b/CHANGELOG.md index 906d9b6..cb5ef29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Canonical schema `spec/schema/oas-schema-1.6.json`** — 1.5 schema plus root-level and task-level `sandbox` definitions (`tools.allow/deny`, `http.allow_domains`, `file.allow_paths`), and `agent.role` loosened from a closed enum to a free-form string (role is informational; well-known values are RECOMMENDED). Additive: every valid 1.5.x document remains valid. The bundled runtime schema (`oas_cli/schemas/oas-schema.json`) is synced to the canonical 1.6 schema so `oa validate` and the formal spec share one source of truth. - **README repositioned around cost efficiency & effectiveness** — usage/cost observability and reasoning-effort tiers now lead the feature list, "Why This Exists" calls out untracked token spend, and the specification table points at the 1.6 artifacts. - **Multi-agent persona examples** now carry the required `agent.description`, so every bundled example validates against the canonical schema. + +### Security +- **API key no longer leaked in provider error output** — when an API key carried an invalid header character (most commonly a trailing `\r` from a CRLF `.env` on Windows/WSL), the OpenAI and Anthropic providers interpolated the underlying exception — whose message embeds the raw `Authorization` / `x-api-key` value — straight into a user-facing `ProviderError`, printing the key in cleartext to the terminal and any CI logs. Keys are now stripped of surrounding whitespace on read (fixing the common CRLF trigger outright), and any credential header value is redacted from provider error messages as a defence-in-depth backstop for other malformed-key cases. The npm runtime was not affected by this path. - Conformance README notes that cases pin the **minimum** `open_agent_spec` version they require, not the suite version. ### Added (older, pre-1.4 notes) diff --git a/oas_cli/providers/anthropic_http.py b/oas_cli/providers/anthropic_http.py index 91ce775..a3ddfdd 100644 --- a/oas_cli/providers/anthropic_http.py +++ b/oas_cli/providers/anthropic_http.py @@ -12,7 +12,7 @@ from oas_cli.tool_providers.base import InvokeResult, ToolCall from oas_cli.usage import from_anthropic -from .base import IntelligenceProvider, InvokeOutcome, ProviderError +from .base import IntelligenceProvider, InvokeOutcome, ProviderError, scrub_secrets _DEFAULT_ENDPOINT = "https://api.anthropic.com/v1/messages" # TODO: model names go stale — consider requiring specs to always declare @@ -108,7 +108,8 @@ def invoke_verbose( history: list[dict[str, Any]] | None = None, ) -> InvokeOutcome: api_key_env = config.get("api_key_env", "ANTHROPIC_API_KEY") - api_key = os.environ.get(api_key_env) + raw = os.environ.get(api_key_env) + api_key = raw.strip() if raw else raw if not api_key: raise ProviderError( f"Anthropic API key not set — export {api_key_env} or set " @@ -157,7 +158,8 @@ def invoke_verbose( detail = exc.read().decode(errors="replace") raise ProviderError(f"Anthropic HTTP {exc.code}: {detail}") from exc except Exception as exc: - raise ProviderError(f"Anthropic request failed: {exc}") from exc + msg = scrub_secrets(str(exc), headers) + raise ProviderError(f"Anthropic request failed: {msg}") from exc try: text = _extract_text_blocks(data) @@ -178,7 +180,8 @@ def invoke_with_tools( config: dict, ) -> InvokeResult: api_key_env = config.get("api_key_env", "ANTHROPIC_API_KEY") - api_key = os.environ.get(api_key_env) + raw = os.environ.get(api_key_env) + api_key = raw.strip() if raw else raw if not api_key: raise ProviderError( f"Anthropic API key not set — export {api_key_env} or set " @@ -236,7 +239,8 @@ def invoke_with_tools( detail = exc.read().decode(errors="replace") raise ProviderError(f"Anthropic HTTP {exc.code}: {detail}") from exc except Exception as exc: - raise ProviderError(f"Anthropic request failed: {exc}") from exc + msg = scrub_secrets(str(exc), headers) + raise ProviderError(f"Anthropic request failed: {msg}") from exc usage = from_anthropic(data.get("usage")) stop_reason = data.get("stop_reason") diff --git a/oas_cli/providers/base.py b/oas_cli/providers/base.py index 06d1c52..4612d43 100644 --- a/oas_cli/providers/base.py +++ b/oas_cli/providers/base.py @@ -28,6 +28,36 @@ class ProviderError(RuntimeError): """Raised when a provider call fails (HTTP error, bad response shape, etc.).""" +# Header names whose values are credentials and must never reach an error message. +_SENSITIVE_HEADER_NAMES = frozenset({"authorization", "x-api-key", "api-key"}) + + +def scrub_secrets(text: str, headers: dict[str, str] | None = None) -> str: + """Redact credential header values from *text* before it is shown to a user. + + Exceptions raised while building an HTTP request (e.g. an ``Authorization`` + value with a stray CRLF from a Windows/WSL ``.env``) embed the raw header + value in their message. Interpolating that straight into a ``ProviderError`` + leaks the API key to stdout, CI logs, and pasted bug reports. This scrubs any + credential header value — and its whitespace-stripped form, since the + exception ``repr`` escapes the trailing character — out of the message. + """ + scrubbed = text + for name, value in (headers or {}).items(): + if name.lower() not in _SENSITIVE_HEADER_NAMES or not value: + continue + # Redact the whole value and its trimmed form, plus each maximal run of + # non-whitespace characters. The last case matters because an exception + # embeds the header via repr(), which escapes control characters (a + # stray newline becomes a literal "\n"), so the raw value no longer + # matches — but its printable segments still appear verbatim. + variants = {value, value.strip(), *value.split()} + for variant in sorted(variants, key=len, reverse=True): + if len(variant) >= 4: + scrubbed = scrubbed.replace(variant, "***REDACTED***") + return scrubbed + + class EngineNotSupportedError(ProviderError): """Raised when no provider is registered for the requested engine.""" diff --git a/oas_cli/providers/openai_http.py b/oas_cli/providers/openai_http.py index 976b98f..e232631 100644 --- a/oas_cli/providers/openai_http.py +++ b/oas_cli/providers/openai_http.py @@ -12,7 +12,7 @@ from oas_cli.tool_providers.base import InvokeResult, ToolCall from oas_cli.usage import from_openai -from .base import IntelligenceProvider, InvokeOutcome, ProviderError +from .base import IntelligenceProvider, InvokeOutcome, ProviderError, scrub_secrets # Default to the Chat Completions API; set intelligence.endpoint in your spec # to switch to the Responses API (https://api.openai.com/v1/responses) or a @@ -51,7 +51,8 @@ def invoke_verbose( history: list[dict[str, Any]] | None = None, ) -> InvokeOutcome: api_key_env: str | None = config.get("api_key_env", "OPENAI_API_KEY") - api_key: str | None = os.environ.get(api_key_env) if api_key_env else None + raw = os.environ.get(api_key_env) if api_key_env else None + api_key: str | None = raw.strip() if raw else raw # Require a key only when the config explicitly names an env var and it's absent. # Local / anonymous endpoints (api_key_env=None or "") skip this check. @@ -106,7 +107,8 @@ def invoke_with_tools( config: dict, ) -> InvokeResult: api_key_env: str | None = config.get("api_key_env", "OPENAI_API_KEY") - api_key: str | None = os.environ.get(api_key_env) if api_key_env else None + raw = os.environ.get(api_key_env) if api_key_env else None + api_key: str | None = raw.strip() if raw else raw if api_key_env and not api_key: raise ProviderError( f"API key not set — export {api_key_env} or set " @@ -257,7 +259,8 @@ def _http_post_raw( detail = exc.read().decode(errors="replace") raise ProviderError(f"OpenAI HTTP {exc.code}: {detail}") from exc except Exception as exc: - raise ProviderError(f"OpenAI request failed: {exc}") from exc + msg = scrub_secrets(str(exc), all_headers) + raise ProviderError(f"OpenAI request failed: {msg}") from exc def _extract_text(data: dict, url: str) -> str: diff --git a/tests/test_providers.py b/tests/test_providers.py index 85122af..ad7183a 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -210,6 +210,91 @@ def test_error_when_key_env_set_but_missing(self): config={"model": "gpt-4o", "api_key_env": "MISSING_KEY_ENV_XYZ"}, ) + def test_crlf_key_is_stripped_not_leaked(self): + """A CRLF-terminated key (Windows/WSL .env) must be trimmed, not sent raw. + + Regression for the key-leak bug: the trailing ``\\r`` made an invalid + header, and the resulting error echoed the key in cleartext. The key is + now stripped on read, so the header is valid and the value carries no + stray whitespace. + """ + headers = self._make_provider_call( + api_key_env="OPENAI_API_KEY", + env_vars={"OPENAI_API_KEY": "sk-test-crlf\r\n"}, + ) + assert headers["Authorization"] == "Bearer sk-test-crlf" + + +# --------------------------------------------------------------------------- +# Credential redaction in provider errors (issue #91) +# --------------------------------------------------------------------------- + + +class TestCredentialRedaction: + SECRET = "sk-proj-AbCdEf0123456789SecretKeyValue" + + def test_scrub_secrets_redacts_bearer_header(self): + from oas_cli.providers.base import scrub_secrets + + headers = {"Authorization": f"Bearer {self.SECRET}", "Content-Type": "x"} + # Message as an exception repr renders it — trailing char escaped. + msg = f"Invalid header value b'Bearer {self.SECRET}\\r'" + out = scrub_secrets(msg, headers) + assert self.SECRET not in out + assert "***REDACTED***" in out + + def test_scrub_secrets_redacts_x_api_key_header(self): + from oas_cli.providers.base import scrub_secrets + + headers = {"x-api-key": self.SECRET} + out = scrub_secrets(f"bad header b'{self.SECRET}'", headers) + assert self.SECRET not in out + + def test_scrub_secrets_leaves_non_secret_headers_untouched(self): + from oas_cli.providers.base import scrub_secrets + + headers = {"Content-Type": "application/json"} + msg = "some error mentioning application/json" + assert scrub_secrets(msg, headers) == msg + + def test_openai_provider_error_does_not_leak_key(self): + """End-to-end: an un-strippable illegal char in the key must not leak. + + An embedded newline survives ``.strip()``, so header construction still + fails — this exercises the ``except`` redaction path, offline (the error + is raised while building the request, before any socket I/O). + """ + + bad_key = "sk-proj-abc\ndef-embedded-newline" # mid-string \n survives strip() + with patch.dict("os.environ", {"OPENAI_API_KEY": bad_key}, clear=False): + with pytest.raises(ProviderError) as exc_info: + OpenAIProvider().invoke( + system="s", + user="u", + config={ + "model": "gpt-4o", + "endpoint": "https://api.openai.com/v1/chat/completions", + }, + ) + msg = str(exc_info.value) + assert bad_key not in msg + assert "abc" not in msg or "***REDACTED***" in msg + + def test_anthropic_provider_error_does_not_leak_key(self): + + bad_key = "sk-ant-abc\ndef-embedded-newline" + with patch.dict("os.environ", {"ANTHROPIC_API_KEY": bad_key}, clear=False): + with pytest.raises(ProviderError) as exc_info: + AnthropicProvider().invoke( + system="s", + user="u", + config={ + "model": "claude-3-5-sonnet-20241022", + "endpoint": "https://api.anthropic.com/v1/messages", + }, + ) + assert bad_key not in str(exc_info.value) + # --------------------------------------------------------------------------- # CustomProvider