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/3] 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/3] 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): From 372d52a0f8e5d2129e5696ec72b5f81340d35dc8 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:54:28 +0530 Subject: [PATCH 3/3] ci: separate sustained persistence cost from hosted filesystem tails --- docs/performance.md | 12 +++++++++++- scripts/benchmark_runtime.py | 9 +++++++-- tests/test_benchmark_runtime.py | 12 ++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/docs/performance.md b/docs/performance.md index 8195656..96e0797 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -63,7 +63,7 @@ scheduler outlier block a change. | Cold no-op invocation, including startup and dispatch | 2,000 ms | 2,000 ms | 4,000 ms | 4,000 ms | | Base-cli lifecycle increment over Click warm dispatch | 5 ms | 5 ms | 15 ms | 15 ms | | Warm invocation and non-persistence feature scenarios | 50 ms | 50 ms | 100 ms | 100 ms | -| File-persistence-enabled scenario | 50 ms | 50 ms | 250 ms | 50 ms | +| File-persistence-enabled scenario | 125 ms | 125 ms | 250 ms | 50 ms | An initial 31-sample local calibration on macOS (Python 3.14.6, Apple Silicon) measured approximately 101 ms for base-cli cold import, 0.56 ms for warm @@ -76,6 +76,16 @@ budget instead of weakening other warm-scenario gates. These measurements are CI calibration evidence, not adoption claims or release comparisons; review subsequent retained artifacts before tightening platform budgets. +October 2026 hosted recalibration separates sustained persistence cost from +filesystem tails on Unix/macOS: median must remain at most **50 ms** and p95 +at most **125 ms**. The previous 50 ms p95 cap repeatedly rejected otherwise +unchanged runtime code, including the validation-only PR. Observed pairs were +14.66/118.04 ms (Unix median/p95) and 24.93/61.93 and 26.12/87.37 ms (macOS). +Evidence: [Unix run](https://github.com/basefoundry/base-cli/actions/runs/37048785893) +and [macOS validation-only run](https://github.com/basefoundry/base-cli/actions/runs/37052368353). +A sustained slowdown over 50 ms still fails; p95 over 125 ms also fails. +Windows, WSL, parser, import, and non-persistence limits are unchanged. + Each report is versioned as `base-cli.benchmark` schema version 1 and contains the package version, source revision, UTC timestamp, platform profile, Python version/ABI, OS release, architecture, CPU count, sample count, medians, p95, diff --git a/scripts/benchmark_runtime.py b/scripts/benchmark_runtime.py index 24f3e78..f5ade65 100755 --- a/scripts/benchmark_runtime.py +++ b/scripts/benchmark_runtime.py @@ -48,8 +48,8 @@ "wsl": 100.0, } PERSISTENCE_ENABLED_P95_BUDGETS_MS = { - "unix": 50.0, - "macos": 50.0, + "unix": 125.0, + "macos": 125.0, "windows": 250.0, "wsl": 50.0, } @@ -331,6 +331,11 @@ def _check_results(results: dict[str, FrameworkMetrics]) -> list[str]: feature_budget = _feature_budget_for_platform(name, BENCHMARK_PLATFORM) if p95 is not None and p95 > feature_budget: failures.append(f"base-cli {name} p95 exceeded {feature_budget:.0f} ms") + if BENCHMARK_PLATFORM in {"unix", "macos"} and isinstance(features, dict): + persistence = features.get("persistence_enabled_ms", {}) + median = persistence.get("median") if isinstance(persistence, dict) else None + if not isinstance(median, (int, float)) or not 0 <= median <= 50.0: + failures.append("base-cli persistence_enabled_ms median is missing, invalid, or exceeded 50 ms") return failures diff --git a/tests/test_benchmark_runtime.py b/tests/test_benchmark_runtime.py index 8528e5e..7518e06 100644 --- a/tests/test_benchmark_runtime.py +++ b/tests/test_benchmark_runtime.py @@ -125,6 +125,18 @@ def test_windows_persistence_budget_rejects_material_regressions(self) -> None: self.assertTrue(any("persistence_enabled_ms p95 exceeded 250 ms" in failure for failure in failures)) + def test_persistence_budget_separates_sustained_cost_from_filesystem_tails(self) -> None: + for profile in ("unix", "macos"): + for median, p95, fails in ((26.0, 118.0, False), (51.0, 60.0, True), (26.0, 126.0, True)): + with self.subTest(profile=profile, median=median, p95=p95): + metrics = self._complete_results() + sample = self._summary(p95) + sample["median"] = median + metrics["base-cli"]["features"]["persistence_enabled_ms"] = sample + with mock.patch.object(benchmark_runtime, "BENCHMARK_PLATFORM", profile): + failures = benchmark_runtime._check_results(metrics) + self.assertEqual(any("persistence_enabled_ms" in failure for failure in failures), fails) + def test_github_summary_separates_lifecycle_overhead_from_parser(self) -> None: metrics = self._complete_results(lifecycle_p95=4.0, click_p95=1.5) report = {