Skip to content

fix: route all logging through kDebugMode-gated logger (L1) - #579

Merged
n13 merged 3 commits into
mainfrom
fix/l1-gated-logging
Jul 28, 2026
Merged

fix: route all logging through kDebugMode-gated logger (L1)#579
n13 merged 3 commits into
mainfrom
fix/l1-gated-logging

Conversation

@n13

@n13 n13 commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Release-build print() / ungated debugPrint() logs are not stripped in release and land in Android logcat, leaking sensitive data. Most critically, taskmaster_service.dart logged print('verify response: ${r.body}') — the response body contains the 24h Taskmaster access token (JWT). Session IDs, balances, and addresses were also logged across the SDK and app.

This PR routes every log statement in quantus_sdk/lib and mobile-app/lib through a kDebugMode-gated quantusDebugPrint:

  • mobile-app already had the gated helper at lib/shared/utils/print.dart; all raw print() and ungated debugPrint() call sites now use it.
  • quantus_sdk gets an identical helper at lib/src/utils/print.dart; all raw print() and ungated debugPrint() call sites (including the _log wrappers in the wormhole/encrypted-account services) now use it.
  • Purely mechanical: log content unchanged, no rewording, no behavior change in debug builds. In release builds these statements now compile to no-ops.
  • Also removed now-stale // ignore: avoid_print comments and newly-unused flutter/foundation / flutter/material imports.

No mnemonic/seed material was found in any log statement (per the audit).

Sites changed: 91 in quantus_sdk (17 files), 41 in mobile-app (19 files).

Verification: dart analyze clean in both packages; flutter test test/unit (mobile-app, 222 tests) and SDK parser/history tests pass.

Addresses finding L1 of the 2026-07-22 mobile wallet security audit.

Raw print() and ungated debugPrint() calls are not stripped in release
builds and land in Android logcat, leaking the Taskmaster JWT (via the
verify response body in taskmaster_service.dart), session IDs, balances,
and addresses.

Route every log statement in quantus_sdk/lib and mobile-app/lib through
a kDebugMode-gated quantusDebugPrint. Mobile-app already had the helper
in lib/shared/utils/print.dart; the SDK gets an identical one at
lib/src/utils/print.dart. Purely mechanical: log content unchanged.

Addresses finding L1 of the 2026-07-22 mobile wallet security audit.
@n13

n13 commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

🔍 Review — L1 (kDebugMode-gated logging)

Verdict: 🟡 Approve with non-blocking comments

A clean, genuinely mechanical conversion. The gating is correct, completeness is thorough (no functional print/debugPrint survivors on the PR branch, including the critical Taskmaster token log), and I found no regressions. The only items below are non-blocking (debug-build secret exposure that is out of L1's release-scope, a trivial count nit, and one out-of-scope logging path).

What it does

  • Adds an identical kDebugMode-gated helper to each package: quantus_sdk/lib/src/utils/print.dart (new) and the pre-existing mobile-app/lib/shared/utils/print.dart.
  • Routes every raw print() and ungated debugPrint() in quantus_sdk/lib and mobile-app/lib through quantusDebugPrint(...), so in release/profile builds these compile out (dead-code-eliminated via the compile-time-const kDebugMode).
  • Removes now-stale // ignore: avoid_print comments and newly-unused flutter/material / flutter/foundation imports.
  • 132 total call sites converted across 17 SDK files (+1 new helper) and 19 mobile-app files.

Strengths

  • Both helpers are byte-for-byte equivalent and correctly gated. void quantusDebugPrint(String message) { if (kDebugMode) { debugPrint(message); } }. kDebugMode is const (!kReleaseMode), so in release AOT the if body is tree-shaken — the statements truly become no-ops. Note debugPrint alone does not no-op in release; the kDebugMode wrapper is what makes this work, and it's present. (quantus_sdk/lib/src/utils/print.dart:1-7, mobile-app/lib/shared/utils/print.dart:1-7)
  • The critical finding is fixed. taskmaster_service.dart:93 print('verify response: ${r.body}') — the response body carrying the 24h JWT — is now quantusDebugPrint(...) (diff lines 1190–1191). It no longer reaches release logcat.
  • Completeness verified against the actual PR head (b22c784, fetched read-only; the local fix/lock-lifecycle-m1 checkout predates this PR so a local grep would be misleading). git grep on the PR ref finds zero functional survivors: the only remaining raw print( is a doc-comment example (quantus_sdk/lib/src/quantus_payload_parser.dart:18 /// print(...)), and the only debugPrint( calls left are the two helper definitions themselves.
  • No rewording. A mechanical diff of every removed log line vs. its replacement shows 120 verbatim message-preserving renames plus exactly 5 necessary object→String adaptations (the helper takes String): print(signedExtensionMap)'$signedExtensionMap' and print(stackTrace)'$stackTrace' (×4 in taskmaster). All are output-equivalent (toString()). No message text changed.
  • Import hygiene is correct. Every file that calls quantusDebugPrint imports a print.dart helper on the PR branch (verified programmatically — zero missing imports). Every removed material/foundation import is confirmed dead (the only required hits are the language keyword, not @required); encrypted_send_provider.dart correctly keeps foundation because it still uses @immutable.

Findings

  1. [non-blocking] Debug builds still print secrets — gated, not redacted. quantusDebugPrint only suppresses in release; in a debug build it still emits the full JWT (taskmaster_service.dart:93 verify response: ${r.body}), the login challenge/msg (:111,:113), balances (substrate_service.dart:62,84), addresses/guardians (reversible_transfers_service.dart, high_security_service.dart:88), referral codes, and eth/X handles. This is acceptable under L1's stated scope (release logcat is the threat), but the JWT-in-debug is worth a follow-up (redact or drop the body) — anyone running/attaching to a debug build sees a live token.

  2. [non-blocking / out of scope] Logging surface isn't fully unified. Four developer.log sites remain ungated: mobile-app/lib/services/secure_clipboard_service.dart:58,75, quantus_sdk/lib/src/models/multisig_proposal.dart:150, quantus_sdk/lib/src/services/chain_history_service.dart:39. dart:developer log() does not surface to Android logcat in a release build (no VM service attached), so it isn't the same leak class as print, and it's outside this PR's print/debugPrint remit — noting only so the audit item isn't assumed to cover every log call.

  3. [nit] Per-package counts in the PR body are off by one. Mechanically I count 92 SDK / 40 mobile-app converted call sites vs. the body's "91 / 41". The total (132) and the file counts (17 SDK call-site files + new helper; 19 mobile-app files) are exact — the split difference is just a counting-method nuance (e.g., the helper declaration line). Immaterial.

  4. [nit] "No behavior change in debug builds" is slightly imprecise. Former print() sites now route through debugPrint, which throttles/rewraps long lines (debugPrintThrottled). Debug-only and harmless, but not literally "no change."

Verification

Adequate and, I'd argue, thorough. I fetched the PR head ref read-only (no checkout) and ran the completeness greps against b22c784 rather than the local branch — this matters, since the local fix/lock-lifecycle-m1 tree still contains all the un-converted prints and would have produced 80+ false "survivors." Confirmed: no functional print/debugPrint survivors, both helpers identical and correctly gated, the critical token log converted, no message rewording (mechanical old-vs-new comparison), no removed-import regressions, and no file using the helper without importing it. I did not compile/dart analyze (would require a checkout), but the import analysis corroborates the PR's "analyze clean" claim.

@n13

n13 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

Not sure we want debugprint - not really - we want print() but gated by debug flag

- Resolve conflicts with main (sendError simplification, push payload
  resolution, multisig call_raw decoding), keeping gated logging
- Rename quantusDebugPrint -> quantusPrint; helper now calls print()
  under kDebugMode (debugPrint throttles/drops output unreliably)
- Gate new debugPrint sites that arrived from main
@n13

n13 commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator Author

🔍 Re-review — L1 (print() gated by kDebugMode)

Verdict: ✅ Approve

The delta since the previous review does exactly what was asked — print() gated by kDebugMode instead of debugPrint — and remains a purely mechanical conversion with no survivors, no regressions, and green CI on the new head (477fa22c).

What changed since the last review

  • Helper switched and renamed. Both packages now define quantusPrint(String) = if (kDebugMode) { print(message); } (was quantusDebugPrint wrapping debugPrint). The rename left zero stale quantusDebugPrint references. Release safety is unchanged: kDebugMode is compile-time const, so the body is still tree-shaken out of release builds — raw print inside the gate is just as safe as debugPrint was.
  • Two merges of origin/main. The PR head now contains the latest main (a49be71b). Main had meanwhile picked up the mobile-app quantusDebugPrint conversion via the stacked audit PRs, so the current PR diff is effectively: the SDK conversion + the helper rename/switch across both packages.

Verified on the new head

  • Completeness: zero raw print(/debugPrint( in quantus_sdk/lib and mobile-app/lib outside the two helper definitions — the only survivor is the doc-comment example in quantus_payload_parser.dart:17. This includes code newly merged from main.
  • Mechanically pure: every added line in the entire 3,576-line diff is a quantusPrint call, an import, or the 7-line helper body. A normalized removed-vs-added comparison matches exactly, except the new SDK helper file and the same five object→'$…' string adaptations noted last time (output-equivalent).
  • Merge hygiene: no conflict markers, no accidental reverts of main's changes, no non-logging code touched.
  • Analyzer/lint: raw print in the helpers is fine because both analysis_options.yaml files already set avoid_print: ignore (pre-existing, untouched by this PR). Every file calling quantusPrint imports a print.dart, the SDK helper is not exported from the quantus_sdk.dart barrel (no ambiguous-symbol risk), and no file imports both helpers. CI Analyze passes on the new head.

Notes (non-blocking, unchanged from last review)

  1. Debug builds still print secrets — the Taskmaster JWT (taskmaster_service.dart verify response), balances, addresses. Gated, not redacted; fine for L1's release-logcat scope, still worth an eventual follow-up.
  2. print vs debugPrint trade-off, now inverted: unthrottled, unwrapped output as intended, at the cost that Android logcat can truncate single lines over ~1–4KB and drop lines under heavy burst in debug builds. Debug-only and intentional — just noting it.
  3. Four ungated developer.log sites remain (secure_clipboard_service.dart:58,75, multisig_proposal.dart:165, chain_history_service.dart:39) — different mechanism, doesn't reach release logcat, out of L1 scope.

@n13
n13 merged commit b1ff47d into main Jul 28, 2026
1 check passed
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