From dc394c7754db822582faaeffd3ea1e57b422ab65 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Wed, 30 Sep 2026 19:46:20 +0530 Subject: [PATCH 1/4] fix: bound JSON redaction traversal --- docs/json-contracts.md | 4 ++- lib/python/base_cli/__init__.py | 2 ++ lib/python/base_cli/json_contracts.py | 43 ++++++++++++++++++++++----- tests/test_json_contracts.py | 27 ++++++++++++++++- 4 files changed, 66 insertions(+), 10 deletions(-) diff --git a/docs/json-contracts.md b/docs/json-contracts.md index 81495bb..0e822f3 100644 --- a/docs/json-contracts.md +++ b/docs/json-contracts.md @@ -76,7 +76,9 @@ 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. Traversal is bounded to 100 container levels; +cyclic or more deeply nested values are replaced with `[REDACTED]` 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/lib/python/base_cli/__init__.py b/lib/python/base_cli/__init__.py index 5b7774b..c9efec4 100644 --- a/lib/python/base_cli/__init__.py +++ b/lib/python/base_cli/__init__.py @@ -114,6 +114,7 @@ def _resolve_version() -> str: JSON_LOG_SCHEMA, JSON_OUTPUT_SCHEMA, MAX_JSON_LOG_MESSAGE_LENGTH, + MAX_JSON_REDACTION_DEPTH, JsonLogFormatter, dumps_envelope, error_envelope, @@ -218,6 +219,7 @@ def _resolve_version() -> str: "JSON_OUTPUT_SCHEMA", "JsonLogFormatter", "MAX_JSON_LOG_MESSAGE_LENGTH", + "MAX_JSON_REDACTION_DEPTH", "NDJSON_SCHEMA", "NDJSON_SCHEMA_VERSION", "NdjsonWriter", diff --git a/lib/python/base_cli/json_contracts.py b/lib/python/base_cli/json_contracts.py index 1632015..ed2219f 100644 --- a/lib/python/base_cli/json_contracts.py +++ b/lib/python/base_cli/json_contracts.py @@ -23,6 +23,7 @@ JSON_OUTPUT_SCHEMA = "base-cli.output" JSON_ERROR_SCHEMA = "base-cli.error" MAX_JSON_LOG_MESSAGE_LENGTH = 8192 +MAX_JSON_REDACTION_DEPTH = 100 _SENSITIVE_ASSIGNMENT_BOUNDARY = ( r"(?=(?:[&,;]\s*(?=[A-Za-z][A-Za-z0-9_-]*\s*[=:])" @@ -40,6 +41,7 @@ "JSON_OUTPUT_SCHEMA", "JsonLogFormatter", "MAX_JSON_LOG_MESSAGE_LENGTH", + "MAX_JSON_REDACTION_DEPTH", "error_envelope", "success_envelope", "dumps_envelope", @@ -108,17 +110,42 @@ def dumps_strict_json(value: Any, **kwargs: Any) -> str: return json.dumps(value, **kwargs) -def redact_json_value(value: Any, *, _key: str | None = None) -> Any: - """Recursively redact secret-looking JSON keys and text values.""" +def redact_json_value( + value: Any, + *, + _key: str | None = None, + _depth: int = 0, + _seen: set[int] | None = None, +) -> Any: + """Recursively redact JSON values with bounded depth and cycle handling. + + The private traversal arguments let recursive calls reject cycles and + pathological nesting before Python's recursion limit or an unbounded + serializer can be reached. A fresh branch-local set is used for each + child so shared, acyclic values are not mistaken for cycles. + """ if _key is not None and _is_sensitive_key(_key): return REDACTED - if isinstance(value, Mapping): - return {str(key): redact_json_value(item, _key=str(key)) for key, item in value.items()} - if isinstance(value, list): - return [redact_json_value(item) for item in value] - if isinstance(value, tuple): - return [redact_json_value(item) for item in value] + if isinstance(value, (Mapping, list, tuple)): + if _depth >= MAX_JSON_REDACTION_DEPTH: + return REDACTED + seen = set() if _seen is None else _seen + identity = id(value) + if identity in seen: + return REDACTED + branch_seen = seen | {identity} + if isinstance(value, Mapping): + return { + str(key): redact_json_value( + item, + _key=str(key), + _depth=_depth + 1, + _seen=branch_seen, + ) + for key, item in value.items() + } + return [redact_json_value(item, _depth=_depth + 1, _seen=branch_seen) for item in value] if isinstance(value, str): return _safe_text(value) return value diff --git a/tests/test_json_contracts.py b/tests/test_json_contracts.py index ec0aab2..4c64c3d 100644 --- a/tests/test_json_contracts.py +++ b/tests/test_json_contracts.py @@ -11,7 +11,7 @@ import base_cli from base_cli._run import JsonCaptureLimitError, _BoundedJsonCapture, _json_requested -from base_cli.json_contracts import MAX_JSON_LOG_MESSAGE_LENGTH +from base_cli.json_contracts import MAX_JSON_LOG_MESSAGE_LENGTH, MAX_JSON_REDACTION_DEPTH @unittest.skipUnless(importlib.util.find_spec("click"), "Click is not installed") @@ -58,6 +58,31 @@ def test_json_contract_emitters_reject_nested_non_finite_values(self) -> None: with self.assertRaises(ValueError): base_cli.dumps_envelope(envelope) + def test_json_redaction_bounds_deep_nesting(self) -> None: + value: dict[str, object] = {} + root = value + for _ in range(MAX_JSON_REDACTION_DEPTH + 5): + child: dict[str, object] = {} + root["nested"] = child + root = child + + redacted = base_cli.redact_json_value(value) + current: object = redacted + for _ in range(MAX_JSON_REDACTION_DEPTH): + self.assertIsInstance(current, dict) + current = current["nested"] # type: ignore[index] + self.assertEqual(current, "[REDACTED]") + json.dumps(redacted) + + def test_json_redaction_replaces_cycles(self) -> None: + value: dict[str, object] = {} + value["self"] = value + + redacted = base_cli.redact_json_value(value) + + self.assertEqual(redacted, {"self": "[REDACTED]"}) + json.dumps(redacted) + def test_inline_secret_redaction_keeps_delimiters_inside_values(self) -> None: for value in ("abc,def", "abc;def"): with self.subTest(value=value): From 93eb00b9010bd7d5c76e4ba5e016b5b3d863b38e Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Wed, 30 Sep 2026 20:05:15 +0530 Subject: [PATCH 2/4] fix: keep redaction limit private --- lib/python/base_cli/__init__.py | 2 -- lib/python/base_cli/json_contracts.py | 1 - 2 files changed, 3 deletions(-) diff --git a/lib/python/base_cli/__init__.py b/lib/python/base_cli/__init__.py index c9efec4..5b7774b 100644 --- a/lib/python/base_cli/__init__.py +++ b/lib/python/base_cli/__init__.py @@ -114,7 +114,6 @@ def _resolve_version() -> str: JSON_LOG_SCHEMA, JSON_OUTPUT_SCHEMA, MAX_JSON_LOG_MESSAGE_LENGTH, - MAX_JSON_REDACTION_DEPTH, JsonLogFormatter, dumps_envelope, error_envelope, @@ -219,7 +218,6 @@ def _resolve_version() -> str: "JSON_OUTPUT_SCHEMA", "JsonLogFormatter", "MAX_JSON_LOG_MESSAGE_LENGTH", - "MAX_JSON_REDACTION_DEPTH", "NDJSON_SCHEMA", "NDJSON_SCHEMA_VERSION", "NdjsonWriter", diff --git a/lib/python/base_cli/json_contracts.py b/lib/python/base_cli/json_contracts.py index ed2219f..6088f23 100644 --- a/lib/python/base_cli/json_contracts.py +++ b/lib/python/base_cli/json_contracts.py @@ -41,7 +41,6 @@ "JSON_OUTPUT_SCHEMA", "JsonLogFormatter", "MAX_JSON_LOG_MESSAGE_LENGTH", - "MAX_JSON_REDACTION_DEPTH", "error_envelope", "success_envelope", "dumps_envelope", From a78ec55e142ae451f07642ca2238e2675a3e0e00 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Thu, 1 Oct 2026 00:19:49 +0530 Subject: [PATCH 3/4] fix: address JSON redaction review feedback --- docs/json-contracts.md | 5 ++-- lib/python/base_cli/json_contracts.py | 36 +++++++++++++++------------ tests/test_json_contracts.py | 11 ++++++-- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/docs/json-contracts.md b/docs/json-contracts.md index 0e822f3..7bdd052 100644 --- a/docs/json-contracts.md +++ b/docs/json-contracts.md @@ -77,8 +77,9 @@ 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 `[REDACTED]` so public -helpers cannot recurse indefinitely while preparing a contract. +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/lib/python/base_cli/json_contracts.py b/lib/python/base_cli/json_contracts.py index 6088f23..779f9a7 100644 --- a/lib/python/base_cli/json_contracts.py +++ b/lib/python/base_cli/json_contracts.py @@ -120,31 +120,35 @@ def redact_json_value( The private traversal arguments let recursive calls reject cycles and pathological nesting before Python's recursion limit or an unbounded - serializer can be reached. A fresh branch-local set is used for each - child so shared, acyclic values are not mistaken for cycles. + serializer can be reached. The active-path set is mutated with + backtracking so shared, acyclic values are not mistaken for cycles and + recursive traversal does not copy the full ancestor set at every node. """ if _key is not None and _is_sensitive_key(_key): return REDACTED if isinstance(value, (Mapping, list, tuple)): if _depth >= MAX_JSON_REDACTION_DEPTH: - return REDACTED + return "[TRUNCATED]" seen = set() if _seen is None else _seen identity = id(value) if identity in seen: - return REDACTED - branch_seen = seen | {identity} - if isinstance(value, Mapping): - return { - str(key): redact_json_value( - item, - _key=str(key), - _depth=_depth + 1, - _seen=branch_seen, - ) - for key, item in value.items() - } - return [redact_json_value(item, _depth=_depth + 1, _seen=branch_seen) for item in value] + return "[TRUNCATED]" + seen.add(identity) + try: + if isinstance(value, Mapping): + return { + str(key): redact_json_value( + item, + _key=str(key), + _depth=_depth + 1, + _seen=seen, + ) + for key, item in value.items() + } + return [redact_json_value(item, _depth=_depth + 1, _seen=seen) for item in value] + finally: + seen.remove(identity) if isinstance(value, str): return _safe_text(value) return value diff --git a/tests/test_json_contracts.py b/tests/test_json_contracts.py index 4c64c3d..c7e31bb 100644 --- a/tests/test_json_contracts.py +++ b/tests/test_json_contracts.py @@ -71,7 +71,7 @@ def test_json_redaction_bounds_deep_nesting(self) -> None: for _ in range(MAX_JSON_REDACTION_DEPTH): self.assertIsInstance(current, dict) current = current["nested"] # type: ignore[index] - self.assertEqual(current, "[REDACTED]") + self.assertEqual(current, "[TRUNCATED]") json.dumps(redacted) def test_json_redaction_replaces_cycles(self) -> None: @@ -80,9 +80,16 @@ def test_json_redaction_replaces_cycles(self) -> None: redacted = base_cli.redact_json_value(value) - self.assertEqual(redacted, {"self": "[REDACTED]"}) + self.assertEqual(redacted, {"self": "[TRUNCATED]"}) json.dumps(redacted) + def test_json_redaction_allows_shared_acyclic_values_on_each_branch(self) -> None: + shared = {"value": "visible"} + + redacted = base_cli.redact_json_value({"first": shared, "second": shared}) + + self.assertEqual(redacted, {"first": shared, "second": shared}) + def test_inline_secret_redaction_keeps_delimiters_inside_values(self) -> None: for value in ("abc,def", "abc;def"): with self.subTest(value=value): From be268f4c202cf63a4013ff7f97c9ce6d356f7576 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Thu, 1 Oct 2026 06:42:12 +0530 Subject: [PATCH 4/4] docs: refresh generated JSON redaction reference --- docs/api-reference.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api-reference.md b/docs/api-reference.md index 56abbd1..8f36643 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -1366,9 +1366,9 @@ base_cli.register_record_schema(...) ### `redact_json_value` **Kind:** function -**Signature:** `redact_json_value(value: 'Any', *, _key: 'str | None' = None) -> 'Any'` +**Signature:** `redact_json_value(value: 'Any', *, _key: 'str | None' = None, _depth: 'int' = 0, _seen: 'set[int] | None' = None) -> 'Any'` -**Behavior:** Recursively redact secret-looking JSON keys and text values. +**Behavior:** Recursively redact JSON values with bounded depth and cycle handling. **Errors and compatibility:** Follow the contract documentation linked in the description. Callers should handle the documented exception types and pin a compatible minor release.