Skip to content

fix: redact malformed delivery report URCs - #249

Draft
Justinabox wants to merge 1 commit into
mainfrom
agent/redact-cdsi-log-20260727
Draft

fix: redact malformed delivery report URCs#249
Justinabox wants to merge 1 commit into
mainfrom
agent/redact-cdsi-log-20260727

Conversation

@Justinabox

Copy link
Copy Markdown
Owner

Summary

  • redact malformed +CDSI delivery-report URCs from dispatcher warning/debug logs
  • redact in-flight executor RX logging for +CDSI URCs
  • add direct-dispatch and executor-path regression tests using non-sensitive sentinel fixtures

Why

Addresses one narrow privacy-hardening slice of #61: malformed delivery-report notifications must not expose raw modem payloads in diagnostic logs.

Verification

  • git diff --check
  • PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/test_executor.py::test_malformed_cdsi_urc_during_command_debug_log_redacts_payload tests/test_delivery_reports.py::TestCDSIDispatch -q → 4 passed
  • PYTHONPATH=. uv run --no-project --with pytest --with pytest-asyncio --with pytest-aiohttp --with pyserial-asyncio --with aiosqlite pytest tests/ -q → 993 passed

Risk

Low. This changes only log representations for malformed delivery-report URCs; parsing and typed event routing are unchanged.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release review: needs-fix (privacy blocker); not ready to land.

URCDispatcher.is_urc() recognizes the +CDSI prefix broadly, but the new redaction and dispatch handling only matches +CDSI:. A malformed no-colon +CDSI line is therefore recognized as a URC yet its raw payload still reaches dispatcher and executor logs, and it falls through to the raw Unhandled URC warning.

Please run a fresh TDD cycle covering a no-colon malformed +CDSI sentinel through both idle/dispatcher and command-response/executor paths, asserting neither raw payload nor sentinel is logged. Make all +CDSI redaction/dispatch matching consistent with the recognized prefix, including the malformed fallback. Rebase on current main, rerun git diff --check plus the required full pytest gate, and request fresh privacy review.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release review: needs-fix (privacy blocker). The change redacts only colon-form +CDSI: values, while URC recognition accepts broad +CDSI input. A malformed no-colon line can still reach executor RX, dispatcher DEBUG, and unhandled-URC logging with its raw payload. Please make the CDSI safe-summary rule cover the accepted prefix forms, ensure the broad dispatcher exception sink uses the safe formatter, and add idle plus in-flight caplog regressions for malformed no-colon input. Rebase on current main and rerun git diff --check plus the full required pytest gate.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release review: needs-fix (privacy blocker), so this is not ready to land.

is_urc() accepts the broad +CDSI prefix, but the new dispatcher/executor redaction checks only +CDSI:. A malformed no-colon +CDSI ... payload therefore reaches both the dispatcher and in-flight executor logs raw.

Please make recognition, fallback handling, and redaction consistent for the whole accepted family (or safely narrow recognition), and add idle + in-flight caplog regressions for malformed no-colon +CDSI input. Rebase, run the full required suite, and request a fresh privacy review.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release review: needs-fix (privacy blocker).

URCDispatcher.is_urc() recognizes the broad +CDSI prefix, but the added redaction paths match only +CDSI:. A parser-invalid no-colon input such as +CDSI<private-sentinel> is therefore routed as a URC and logged verbatim in both idle dispatch and command-in-flight paths.

Please start a fresh TDD cycle: add caplog regressions for that no-colon malformed form in both paths, verify RED on the current head, then make redaction cover the full recognized +CDSI family (preferably centralizing recognition/redaction) without changing routing semantics. Re-run the required full gate after the fix.

Existing tests only cover colon-bearing malformed lines and miss this recognition/redaction mismatch; this PR is not mergeable yet.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release-lane classification: needs fix/revalidation before merge.

This draft was not independently reviewed or integration-tested in this pass and its GitHub mergeability is stale/unknown after current-main advances. Please rebase or otherwise validate it against current main, run the required full local gate, and request a fresh release review with exact head SHA and gate evidence. Do not treat absence of hosted checks as approval.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release classification: needs-fix for this release pass.

The draft has not received a current exact-head review and prescribed local integration gate over the advancing main baseline. Rebase/repackage as needed against current main, resolve any conflicts, then provide a narrow behavioral review plus passing git diff --check and the prescribed full test suite before it can be reconsidered for landing.

@Justinabox

Copy link
Copy Markdown
Owner Author

Release review: needs-fix. This covers malformed +CDSI and in-flight RX logging, but the dispatcher exception path still logs raw URCs and exception text. Do not depend on a separate unmerged PR for this privacy boundary: add an exception-path regression, apply a fail-closed safe dispatcher log, rebase on current origin/main, and rerun required gates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant