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. diff --git a/docs/json-contracts.md b/docs/json-contracts.md index 81495bb..7bdd052 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. 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/lib/python/base_cli/json_contracts.py b/lib/python/base_cli/json_contracts.py index 1632015..779f9a7 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*[=:])" @@ -108,17 +109,46 @@ 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. 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): - 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 "[TRUNCATED]" + seen = set() if _seen is None else _seen + identity = id(value) + if identity in seen: + 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 ec0aab2..c7e31bb 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,38 @@ 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, "[TRUNCATED]") + 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": "[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):