diff --git a/docs/json-contracts.md b/docs/json-contracts.md index 7bdd052..d0cf7e5 100644 --- a/docs/json-contracts.md +++ b/docs/json-contracts.md @@ -74,12 +74,21 @@ errors and diagnostics still use the normal stderr and exit-code boundary. The lower-level `success_envelope()`, `error_envelope()`, `dumps_envelope()`, and `redact_json_value()` helpers are public for commands that need to publish -their own structured `details` records. Secret-looking keys (`token`, -`password`, `secret`, `api_key`, and `authorization`) and credential-bearing -URLs are redacted recursively. Traversal is bounded to 100 container levels; -cyclic or more deeply nested values are replaced with `[TRUNCATED]`, distinct -from the `[REDACTED]` marker used for secrets, so public helpers cannot recurse -indefinitely while preparing a contract. +their own structured `details` records. Secret-looking keys and +credential-bearing URLs are redacted recursively. The heuristic covers +`token`, `password`, `passwd`, `pwd`, `passphrase`, `secret`, `credential`, +`private-key`, `access-key`, `api-key`, `authorization`, `bearer`, `session`, +`cookie`, `signature`, `otp`, `salt`, `sas`, and `pem`, including camelCase +forms such as `accessToken`, `sessionToken`, `dbPassword`, and `bearerToken`. +The `private-key`, `access-key`, and `api-key` compounds are also recognized +when written as `privateKey`, `accessKey`, or `apiKey`. A generic `key` name, +including `key-file`, `sort-key`, and `public-key`, is not treated as secret by +itself; explicit `sensitive=True` remains the authoritative control for +domain-specific names. +Traversal is bounded to 100 container levels; cyclic or more deeply nested +values are replaced with `[TRUNCATED]`, distinct from the `[REDACTED]` marker +used for secrets, so public helpers cannot recurse indefinitely while preparing +a contract. Golden payloads for each public contract live in [`tests/fixtures/contracts`](https://github.com/basefoundry/base-cli/tree/main/tests/fixtures/contracts). diff --git a/docs/security-threat-model.md b/docs/security-threat-model.md index d3293ec..78e479d 100644 --- a/docs/security-threat-model.md +++ b/docs/security-threat-model.md @@ -62,7 +62,14 @@ The boundaries are intentionally explicit: | Threat / asset | Framework controls and tests | Residual risk and consumer action | | --- | --- | --- | -| Secrets in argv, environment-derived values, config, or prompts leak into logs | Sensitive options/arguments, secret-name heuristics, embedded query/list/header segment handling, equals/short-option handling, and redaction before history callbacks; `tests/test_redaction_security.py`, `tests/test_app_security_boundaries.py`, and `tests/test_invocation_parity.py` | A custom secret name or consumer log can still disclose data. Mark domain-specific parameters with `sensitive=True`, do not log `ctx.config`, and review custom formatters/history writers. | +| Secrets in argv, environment-derived values, config, or prompts leak into logs | Sensitive options/arguments, shared secret-name heuristics (including password/passwd/pwd/passphrase, credential, private/access/API key, camelCase access/refresh/id token and client/auth secret forms, bearer/session/cookie, OTP, salt, SAS, and PEM names), embedded query/list/header segment handling, equals/short-option handling, and redaction before history callbacks; `tests/test_redaction_security.py`, `tests/test_app_security_boundaries.py`, and `tests/test_invocation_parity.py` | A custom secret name or consumer log can still disclose data. Mark domain-specific parameters with `sensitive=True`, do not log `ctx.config`, and review custom formatters/history writers. | + +The secret-name backstop tokenizes dash-, underscore-, and camelCase names, so +`sessionToken`, `dbPassword`, and `bearerToken` receive the same protection as +their separated forms. It intentionally does not treat a bare `key` as secret: +`key-file`, `sort-key`, `partition-key`, and `public-key` remain visible unless +a consumer explicitly marks them sensitive. This heuristic is defense in depth, +not a replacement for explicit `sensitive=True` declarations. | Logs, history, JSON, or run metadata expose credentials or unbounded attacker text | Redacted history boundary, bounded JSON log messages, owner-only POSIX modes, atomic metadata writes, and JSON contract tests | Consumer-owned paths and history stores may have weaker permissions. Set private ACLs, avoid copying raw logs, and treat retained diagnostics as sensitive. | | Symlink, traversal, replacement, or mount races redirect cleanup | Exclusive runtime-leaf ownership, retained descriptors, identity checks, no-follow traversal, run-ID containment, and fail-closed cleanup; `tests/test_cleanup_security.py`, `tests/test_app_security_boundaries.py`, and adversarial regression tests | A same-account process with the same filesystem authority can race user-owned paths. Use a private cache root and avoid sharing runtime trees between mutually hostile users. | | Insecure permissions expose runtime files | POSIX `0600`/`0700` modes; Windows uses inherited user-profile ACLs and warns when secure handle operations are unavailable | A custom Windows cache root or network filesystem may not inherit private ACLs. Consumers must provision and verify permissions. | diff --git a/lib/python/base_cli/json_contracts.py b/lib/python/base_cli/json_contracts.py index 779f9a7..6207011 100644 --- a/lib/python/base_cli/json_contracts.py +++ b/lib/python/base_cli/json_contracts.py @@ -16,7 +16,7 @@ from logging import LogRecord from typing import Any -from .redaction import REDACTED, redact_text_value +from .redaction import KEY_NAME_PATTERN, REDACTED, is_secret_key, redact_text_value JSON_CONTRACT_VERSION = 1 JSON_LOG_SCHEMA = "base-cli.log" @@ -29,9 +29,10 @@ r"(?=(?:[&,;]\s*(?=[A-Za-z][A-Za-z0-9_-]*\s*[=:])" r"|\s+[A-Za-z][A-Za-z0-9_-]*\s*[=:])|\s|$)" ) -_SENSITIVE_ASSIGNMENT = re.compile( - r"(?i)(\b(?:token|password|secret|api[-_]?key|authorization)\b\s*[:=]\s*)" - rf"(\S+?){_SENSITIVE_ASSIGNMENT_BOUNDARY}" +_KEY_ASSIGNMENT = re.compile( + rf"(?P{KEY_NAME_PATTERN})(?P\s*[:=]\s*)" + rf"(?P\S+?){_SENSITIVE_ASSIGNMENT_BOUNDARY}", + re.IGNORECASE, ) __all__ = [ @@ -187,8 +188,14 @@ def _timestamp(value: float) -> str: def _safe_text(value: str) -> str: redacted = redact_text_value(value) - return _SENSITIVE_ASSIGNMENT.sub(r"\1" + REDACTED, redacted) + return _KEY_ASSIGNMENT.sub(_redact_assignment, redacted) + + +def _redact_assignment(match: re.Match[str]) -> str: + if not is_secret_key(match.group("key")): + return match.group(0) + return f"{match.group('key')}{match.group('separator')}{REDACTED}" def _is_sensitive_key(value: str) -> bool: - return re.search(r"(?i)(token|password|secret|api[-_]?key|authorization)", value) is not None + return is_secret_key(value) diff --git a/lib/python/base_cli/redaction.py b/lib/python/base_cli/redaction.py index 2a0afa0..bc1212e 100644 --- a/lib/python/base_cli/redaction.py +++ b/lib/python/base_cli/redaction.py @@ -6,7 +6,42 @@ from typing import Any REDACTED = "[REDACTED]" -SECRET_KEY_RE = re.compile(r"(token|password|secret|api[-_]?key|authorization)", re.IGNORECASE) +KEY_NAME_PATTERN = r"[A-Za-z][A-Za-z0-9_-]*" +SECRET_KEY_STEMS = frozenset( + { + "authorization", + "bearer", + "cookie", + "credential", + "passphrase", + "passwd", + "password", + "pem", + "pwd", + "sas", + "salt", + "secret", + "session", + "signature", + "token", + "otp", + } +) +SECRET_KEY_COMPOUNDS = frozenset({("access", "key"), ("api", "key"), ("private", "key")}) +SECRET_KEY_SUBSTRINGS = frozenset( + { + "apikey", + "authorization", + "credential", + "passwd", + "password", + "passphrase", + "secret", + "token", + } +) +_KEY_NAME_RE = re.compile(KEY_NAME_PATTERN) +_CAMEL_TOKEN_RE = re.compile(r"[A-Z]+(?=[A-Z][a-z]|[0-9]|$)|[A-Z]?[a-z]+|[0-9]+") URL_CREDENTIALS_RE = re.compile(r"(?P[a-zA-Z][a-zA-Z0-9+.-]*://)[^/@\s]+@") # Punctuation is part of a value unless it is immediately followed by another # assignment segment. This prevents ``PASSWORD=abc,def`` from exposing ``def`` @@ -16,11 +51,11 @@ r"|\s+[A-Za-z][A-Za-z0-9_-]*\s*[=:])|$)" ) _INLINE_KEY_VALUE_RE = re.compile( - rf"(?P(?=)" + rf"(?P(?=)" rf"(?P[^\n]*?){_INLINE_SEGMENT_END}" ) _INLINE_COLON_VALUE_RE = re.compile( - rf"(?P(?(?\s*:(?!//)\s*)(?P[^\n]*?){_INLINE_SEGMENT_END}" ) @@ -160,7 +195,20 @@ def redact_argv(argv: list[str], sensitive_options: set[str]) -> list[str]: def is_secret_key(value: str) -> bool: - return SECRET_KEY_RE.search(value) is not None + for identifier in _KEY_NAME_RE.findall(value): + compact = identifier.replace("-", "").replace("_", "").casefold() + if any(stem in compact for stem in SECRET_KEY_SUBSTRINGS): + return True + tokens = tuple( + token.casefold() for part in re.split(r"[-_]", identifier) for token in _CAMEL_TOKEN_RE.findall(part) + ) + if any(token in SECRET_KEY_STEMS for token in tokens) or any( + compound == tokens[index : index + len(compound)] + for compound in SECRET_KEY_COMPOUNDS + for index in range(len(tokens) - len(compound) + 1) + ): + return True + return False def redact_text_value(value: str) -> str: @@ -568,7 +616,9 @@ def _redact_without_schema(argv: list[str], sensitive_options: set[str]) -> list option_name, separator, _attached = value.partition("=") normalized = option_name_to_parameter(option_name) explicitly_sensitive = option_name in sensitive or normalized in sensitive - automatically_sensitive = _is_option_alias(option_name) and is_secret_key(normalized) + automatically_sensitive = _is_option_alias(option_name) and ( + is_secret_key(option_name) or is_secret_key(normalized) + ) if explicitly_sensitive: if separator: result[index] = f"{option_name}={REDACTED}" @@ -615,7 +665,7 @@ def _redact_inline_text(value: str) -> str: def _redact_inline_segment(match: re.Match[str]) -> str: key = match.group("key") - if not is_secret_key(option_name_to_parameter(key)): + if not is_secret_key(key): return match.group(0) return f"{key}{match.group('separator')}{REDACTED}" diff --git a/tests/test_json_contracts.py b/tests/test_json_contracts.py index c7e31bb..d6a5dc1 100644 --- a/tests/test_json_contracts.py +++ b/tests/test_json_contracts.py @@ -52,6 +52,22 @@ def test_envelopes_have_stable_fields_and_recursive_redaction(self) -> None: self.assertEqual(failure["message"], "authorization=[REDACTED]") self.assertEqual(json.loads(base_cli.dumps_envelope(failure)), failure) + def test_json_redaction_uses_extended_secret_key_heuristics(self) -> None: + envelope = base_cli.success_envelope( + run_id=None, + details={ + "private_key": "private", + "session_cookie": "cookie", + "accessToken": "camel-case-secret", + "label": "visible", + }, + ) + + self.assertEqual(envelope["details"]["private_key"], "[REDACTED]") + self.assertEqual(envelope["details"]["session_cookie"], "[REDACTED]") + self.assertEqual(envelope["details"]["accessToken"], "[REDACTED]") + self.assertEqual(envelope["details"]["label"], "visible") + def test_json_contract_emitters_reject_nested_non_finite_values(self) -> None: invalid = {"nested": [{"value": float("inf")}]} envelope = base_cli.success_envelope(run_id=None, details=invalid) diff --git a/tests/test_redaction_security.py b/tests/test_redaction_security.py index d8ce2bd..59d8fcb 100644 --- a/tests/test_redaction_security.py +++ b/tests/test_redaction_security.py @@ -4,6 +4,7 @@ import click from base_cli.history import redact_history_argv +from base_cli.json_contracts import redact_json_value from base_cli.redaction import ( REDACTED, RedactionPlan, @@ -86,6 +87,85 @@ def test_secret_name_heuristics_apply_without_registration(self) -> None: self.assertEqual(redact_argv(argv, set()), expected) self.assertEqual(redact_history_argv(argv, set()), expected) + def test_extended_secret_name_heuristics_apply_consistently(self) -> None: + cases = ( + ("--passwd", "old-password"), + ("--passphrase", "phrase"), + ("--credential", "credential-value"), + ("--private-key", "private-key-value"), + ("--access_key", "access-key-value"), + ("--accessToken", "access-token-value"), + ("--refreshToken", "refresh-token-value"), + ("--idToken", "id-token-value"), + ("--clientSecret", "client-secret-value"), + ("--authToken", "auth-token-value"), + ("--bearer", "bearer-value"), + ("--session-cookie", "cookie-value"), + ("--signature", "signature-value"), + ("--otp", "123456"), + ("--salt", "salt-value"), + ("--pem", "pem-value"), + ) + for option, value in cases: + with self.subTest(option=option): + argv = ["tool", option, value] + expected = ["tool", option, REDACTED] + self.assertEqual(redact_argv(argv, set()), expected) + self.assertEqual(redact_history_argv(argv, set()), expected) + + def test_documented_secret_names_cover_argv_inline_and_json(self) -> None: + names = ( + "token", + "password", + "passwd", + "pwd", + "passphrase", + "secret", + "credential", + "private-key", + "access-key", + "api-key", + "authorization", + "bearer", + "session", + "cookie", + "signature", + "otp", + "salt", + "sas", + "pem", + "sessionToken", + "dbPassword", + "bearerToken", + "oauthToken", + "secretKey", + "apikey", + "APIKEY", + "accesstoken", + "ACCESSTOKEN", + "clientsecret", + "csrftoken", + "PGPASSWORD", + "x-apikey", + ) + for name in names: + with self.subTest(name=name): + self.assertEqual(redact_argv(["tool", f"--{name}", "value"], set())[-1], REDACTED) + self.assertEqual(redact_argv(["tool", f"{name}=value"], set())[-1], f"{name}={REDACTED}") + self.assertEqual(redact_json_value({name: "value"}), {name: REDACTED}) + + def test_generic_key_names_remain_visible(self) -> None: + for name in ("key-file", "sort-key", "partition-key", "public-key", "keyFile", "publicKey"): + with self.subTest(name=name): + self.assertEqual(redact_argv(["tool", f"--{name}", "value"], set())[-1], "value") + self.assertEqual(redact_argv(["tool", f"{name}=value"], set())[-1], f"{name}=value") + self.assertEqual(redact_json_value({name: "value"}), {name: "value"}) + + def test_bare_key_is_not_treated_as_a_secret_name(self) -> None: + self.assertEqual(redact_argv(["tool", "--key", "visible"], set()), ["tool", "--key", "visible"]) + self.assertEqual(redact_argv(["tool", "--key-file", "visible"], set()), ["tool", "--key-file", "visible"]) + self.assertEqual(redact_argv(["tool", "--public-key", "visible"], set()), ["tool", "--public-key", "visible"]) + def test_embedded_secret_segments_are_redacted_without_registration(self) -> None: cases = ( ( @@ -140,6 +220,10 @@ def test_embedded_secret_segments_are_redacted_without_registration(self) -> Non ["tool", "PASSWORD=abc,def&LABEL=visible"], ["tool", f"PASSWORD={REDACTED}&LABEL=visible"], ), + ( + ["tool", "accessToken=camel-case-secret"], + ["tool", f"accessToken={REDACTED}"], + ), ) for argv, expected in cases: with self.subTest(argv=argv):