From a9516544547666f9e8bac470619fd3e43fa4e50d Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Wed, 30 Sep 2026 19:51:37 +0530 Subject: [PATCH 1/2] security: centralize secret-name redaction heuristics --- docs/json-contracts.md | 5 ++++- docs/security-threat-model.md | 2 +- lib/python/base_cli/json_contracts.py | 6 +++--- lib/python/base_cli/redaction.py | 7 ++++++- tests/test_json_contracts.py | 10 ++++++++++ tests/test_redaction_security.py | 24 ++++++++++++++++++++++++ 6 files changed, 48 insertions(+), 6 deletions(-) diff --git a/docs/json-contracts.md b/docs/json-contracts.md index 81495bb..f593e3e 100644 --- a/docs/json-contracts.md +++ b/docs/json-contracts.md @@ -76,7 +76,10 @@ 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. +URLs are redacted recursively. The heuristic covers token, password/passphrase, +credential, private/access/API key, authorization/bearer, session/cookie, +signature, OTP, salt, SAS, and PEM names; a generic `key` name is not treated +as secret by itself. 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..b840ae0 100644 --- a/docs/security-threat-model.md +++ b/docs/security-threat-model.md @@ -62,7 +62,7 @@ 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 credential, private/access/API key, 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. | | 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 1632015..b1977c4 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 REDACTED, SECRET_KEY_PATTERN, is_secret_key, redact_text_value JSON_CONTRACT_VERSION = 1 JSON_LOG_SCHEMA = "base-cli.log" @@ -29,7 +29,7 @@ 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"(?i)({SECRET_KEY_PATTERN}\s*[:=]\s*)" rf"(\S+?){_SENSITIVE_ASSIGNMENT_BOUNDARY}" ) @@ -161,4 +161,4 @@ def _safe_text(value: str) -> str: 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..e6596fc 100644 --- a/lib/python/base_cli/redaction.py +++ b/lib/python/base_cli/redaction.py @@ -6,7 +6,12 @@ from typing import Any REDACTED = "[REDACTED]" -SECRET_KEY_RE = re.compile(r"(token|password|secret|api[-_]?key|authorization)", re.IGNORECASE) +SECRET_KEY_PATTERN = ( + r"(?[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`` diff --git a/tests/test_json_contracts.py b/tests/test_json_contracts.py index ec0aab2..fdbb11f 100644 --- a/tests/test_json_contracts.py +++ b/tests/test_json_contracts.py @@ -52,6 +52,16 @@ 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", "label": "visible"}, + ) + + self.assertEqual(envelope["details"]["private_key"], "[REDACTED]") + self.assertEqual(envelope["details"]["session_cookie"], "[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..d2fe144 100644 --- a/tests/test_redaction_security.py +++ b/tests/test_redaction_security.py @@ -86,6 +86,30 @@ 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"), + ("--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_bare_key_is_not_treated_as_a_secret_name(self) -> None: + self.assertEqual(redact_argv(["tool", "--key", "visible"], set()), ["tool", "--key", "visible"]) + def test_embedded_secret_segments_are_redacted_without_registration(self) -> None: cases = ( ( From 1f1e142dd7d2b69debf452ebd68b49af7377b844 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:19:50 +0530 Subject: [PATCH 2/2] fix: restore camelCase secret redaction coverage --- docs/json-contracts.md | 14 ++++++++------ docs/security-threat-model.md | 2 +- lib/python/base_cli/redaction.py | 3 ++- tests/test_json_contracts.py | 8 +++++++- tests/test_redaction_security.py | 11 +++++++++++ 5 files changed, 29 insertions(+), 9 deletions(-) diff --git a/docs/json-contracts.md b/docs/json-contracts.md index f593e3e..5409350 100644 --- a/docs/json-contracts.md +++ b/docs/json-contracts.md @@ -74,12 +74,14 @@ 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. The heuristic covers token, password/passphrase, -credential, private/access/API key, authorization/bearer, session/cookie, -signature, OTP, salt, SAS, and PEM names; a generic `key` name is not treated -as secret by itself. +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` and `clientSecret`. A generic `key` name, including +`key-file` and `public-key`, is not treated as secret by itself; explicit +`sensitive=True` remains the authoritative control for domain-specific names. 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 b840ae0..51d2925 100644 --- a/docs/security-threat-model.md +++ b/docs/security-threat-model.md @@ -62,7 +62,7 @@ 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, shared secret-name heuristics (including credential, private/access/API key, 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. | +| 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. | | 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/redaction.py b/lib/python/base_cli/redaction.py index e6596fc..44539f2 100644 --- a/lib/python/base_cli/redaction.py +++ b/lib/python/base_cli/redaction.py @@ -7,7 +7,8 @@ REDACTED = "[REDACTED]" SECRET_KEY_PATTERN = ( - r"(? None: 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", "label": "visible"}, + 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: diff --git a/tests/test_redaction_security.py b/tests/test_redaction_security.py index d2fe144..2408788 100644 --- a/tests/test_redaction_security.py +++ b/tests/test_redaction_security.py @@ -93,6 +93,11 @@ def test_extended_secret_name_heuristics_apply_consistently(self) -> None: ("--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"), @@ -109,6 +114,8 @@ def test_extended_secret_name_heuristics_apply_consistently(self) -> None: 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 = ( @@ -164,6 +171,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):