fix(logging): redact agent registry exception telemetry - #1709
seonghobae wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe registry now uses redacted exception metadata for read and JSON parsing failures. Tests verify that logs retain failure type and fingerprint without exposing exception contents or paths. Documentation records the logging boundary and unchanged missing-file handling. ChangesAgent registry exception telemetry
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The registry continues to recover from read and parse failures without logging raw exception details, payloads, or paths. No actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review Exact review target: |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='2d6800aea110ac04eeedcd5faa91479f62877408'
head='fc56d609045e47093fdf455234121eb7cfe31715'
echo '=== exact changed files ==='
git diff --name-status "$base" "$head"
echo '=== exact diff ==='
git diff --no-ext-diff --unified=80 "$base" "$head" -- \
backend/services/agent_registry.py \
backend/tests/test_agent_registry.py \
docs/doctoring/agent-registry-exception-telemetry.md
echo '=== agent registry outline ==='
ast-grep outline backend/services/agent_registry.py --items all || true
echo '=== redacted_exception_info definition and local call sites ==='
rg -n -A35 -B10 --glob '*.py' '\bdef\s+redacted_exception_info\s*\(|\bredacted_exception_info\s*\(' core backend
echo '=== helper module outline ==='
helper=$(fd -t f 'safe_logging\.py' . | head -n 1)
if [ -n "$helper" ]; then
ast-grep outline "$helper" --items all || true
fiLength of output: 52876 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='2d6800aea110ac04eeedcd5faa91479f62877408'
head='fc56d609045e47093fdf455234121eb7cfe31715'
echo '=== exact diff whitespace validation ==='
git diff --check "$base" "$head" -- \
backend/services/agent_registry.py \
backend/tests/test_agent_registry.py \
docs/doctoring/agent-registry-exception-telemetry.md
echo '=== safe logging module ==='
cat -n backend/core/safe_logging.py
echo '=== test framework configuration candidates ==='
fd -t f -a 'pyproject.toml|pytest.ini|tox.ini|setup.cfg' . -d 3 | while IFS= read -r file; do
echo "--- $file"
rg -n -C2 'pytest|testpaths|pythonpath|addopts' "$file" || true
done
echo '=== exact commit parent relationship ==='
git merge-base --is-ancestor "$base" "$head"
printf 'base_is_ancestor=yes\n'
printf 'merge_base='
git merge-base "$base" "$head"
printf 'changed_file_count='
git diff --name-only "$base" "$head" | wc -lLength of output: 4115
I found no issues in the three-file agent-registry telemetry delta.
The focused tests format real log records and reject the injected secret values and paths. The documentation matches the implementation and the stated sink-specific scope. Static validation passed for the exact diff. I did not execute repository tests. I did not use or transfer parent workflow or approval evidence. ✅ Action performedReview finished.
|
Refs #1698.
Current authority
2d6800aea110ac04eeedcd5faa91479f62877408fc56d609045e47093fdf455234121eb7cfe31715behind_by=0, three commits aheadbackend/services/agent_registry.py,backend/tests/test_agent_registry.py,docs/doctoring/agent-registry-exception-telemetry.mdVerified finding
The agent registry loader still used ordinary
exc_info=TrueforOSErrorandjson.JSONDecodeError, and included the registry path in those exception-bearing log messages. Normal Python logging can render the original exception value and traceback, so provider/OS failures can expose credentials, connection strings or internal path material into the normal log sink. This is one of the remaining sink-by-sink migrations required by #1698; it is not a reason to fork #1700's telemetry contract.The
FileNotFoundErrorbranch is intentionally unchanged because it attaches no exception information and reports the deterministic missing registration file.Minimal causal repair
The two exception-bearing branches now consume #1700's
core.safe_logging.redacted_exception_info(error)helper. The rendered record keeps a bounded operation label, exception class and one-way failure-site fingerprint while dropping raw exception value, raw traceback and the registry path from these failure records. No public/API response behavior or registry parsing semantics change.Focused regressions drive a real
logging.Formatterand inject API-key-shaped data, a PostgreSQL connection string, token-bearing malformed JSON and secret-bearing paths. They require the operation label, exception type andexception_fingerprint=while rejecting the injected values and paths from rendered output.Doctoring records the scope, rejected alternatives and CWE-532 / OWASP Logging Cheat Sheet traceability. No helper copy or second telemetry contract is introduced.
Independent review
CodeRabbit reviewed exactly
2d6800aea110ac04eeedcd5faa91479f62877408 → fc56d609045e47093fdf455234121eb7cfe31715, selected all three effective files, produced no review thread, and submitted formal APPROVED reviewPRR_kwDOSNjZ2s8AAAABN2-vowat2026-09-16T15:51:58Z. This is exact-current-head review evidence only; it does not substitute for hosted required contexts or protected-lineage integration.Evidence boundary
Exact
fc56d609...still has zero PR-triggered repository workflow runs because this PR is stacked on #1700 and the repository-local stacked-PR trigger defect remains owned by #1691. Test source and exact-head independent review are GREEN, but no hosted GREEN is claimed. Parent #1700 workflow/review evidence does not transfer.Keep Draft until the canonical exception-redaction/telemetry lineage reaches protected ancestry and the unchanged final head has every then-live required context terminal-success plus qualifying post-last-push independent review. No temporary
developretarget, copied workflow, dummy/no-op evidence commit, synthetic status, self-approval, force push, destructive rebase, or gate weakening.Summary by CodeRabbit
Bug Fixes
Documentation