From 4d881b4bc7a213e0b31712563f428dc29d1cf3c7 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Tue, 18 Aug 2026 18:21:03 +0530 Subject: [PATCH] Harden embedded secret redaction --- CHANGELOG.md | 6 +++++ docs/security-threat-model.md | 2 +- lib/python/base_cli/redaction.py | 21 ++++++++++++--- tests/test_redaction_security.py | 44 ++++++++++++++++++++++++++++++++ 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d863fc..45f216c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,12 @@ and versions are tracked in the repo-root `VERSION` file. - Formalize typed extension callback protocols, entry-point capability metadata, and pre-load API-version negotiation. +### Security + +- Redact recognized secret keys embedded in query strings, comma-separated + values, and header-style `key: value` arguments before they reach logs or + persisted history. + ### Added - Add a framework choice guide, five-minute evaluation path, and clearer diff --git a/docs/security-threat-model.md b/docs/security-threat-model.md index cc7cfac..27cb702 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, 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, 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. | | 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 3383318..3e9372b 100644 --- a/lib/python/base_cli/redaction.py +++ b/lib/python/base_cli/redaction.py @@ -8,6 +8,15 @@ REDACTED = "[REDACTED]" SECRET_KEY_RE = re.compile(r"(token|password|secret|api[-_]?key|authorization)", re.IGNORECASE) URL_CREDENTIALS_RE = re.compile(r"(?P[a-zA-Z][a-zA-Z0-9+.-]*://)[^/@\s]+@") +_INLINE_SEGMENT_END = r"(?=(?:[&,;]|\s+[A-Za-z][A-Za-z0-9_-]*\s*[=:])|$)" +_INLINE_KEY_VALUE_RE = re.compile( + rf"(?P(?=)" + rf"(?P[^\n]*?){_INLINE_SEGMENT_END}" +) +_INLINE_COLON_VALUE_RE = re.compile( + rf"(?P(?\s*:(?!//)\s*)(?P[^\n]*?){_INLINE_SEGMENT_END}" +) @dataclass(frozen=True) @@ -593,12 +602,18 @@ def _legacy_short_aliases(sensitive_options: Iterable[str]) -> tuple[str, ...]: def _redact_inline_text(value: str) -> str: - key, separator, _raw_value = value.partition("=") - if separator and is_secret_key(option_name_to_parameter(key)): - value = f"{key}={REDACTED}" + value = _INLINE_KEY_VALUE_RE.sub(_redact_inline_segment, value) + value = _INLINE_COLON_VALUE_RE.sub(_redact_inline_segment, value) return redact_text_value(value) +def _redact_inline_segment(match: re.Match[str]) -> str: + key = match.group("key") + if not is_secret_key(option_name_to_parameter(key)): + return match.group(0) + return f"{key}{match.group('separator')}{REDACTED}" + + def _is_option_alias(value: str) -> bool: return bool(_split_option(value)[0]) diff --git a/tests/test_redaction_security.py b/tests/test_redaction_security.py index 5addb85..20715d7 100644 --- a/tests/test_redaction_security.py +++ b/tests/test_redaction_security.py @@ -86,6 +86,50 @@ 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_embedded_secret_segments_are_redacted_without_registration(self) -> None: + cases = ( + ( + [ + "tool", + "fetch", + "--url", + "https://api.example.test/resource?filter=active&token=SUPERSECRET123", + ], + [ + "tool", + "fetch", + "--url", + "https://api.example.test/resource?filter=active&token=[REDACTED]", + ], + ), + ( + ["tool", "run", "--env", "FOO=bar,SECRET_TOKEN=hunter2"], + ["tool", "run", "--env", "FOO=bar,SECRET_TOKEN=[REDACTED]"], + ), + ( + ["tool", "call", "-H", "Authorization: Bearer sk-supersecrettoken123"], + ["tool", "call", "-H", "Authorization: [REDACTED]"], + ), + ( + [ + "tool", + "call", + "--header", + "X-Api-Key: hunter2,Accept: application/json", + ], + [ + "tool", + "call", + "--header", + "X-Api-Key: [REDACTED],Accept: application/json", + ], + ), + ) + for argv, expected in cases: + with self.subTest(argv=argv): + self.assertEqual(redact_argv(argv, set()), expected) + self.assertEqual(redact_history_argv(argv, set()), expected) + def test_option_looking_values_follow_click_consumption(self) -> None: self.assertEqual( redact_argv(["tool", "--token", "--verbose"], {"token"}),