Skip to content

chore(logging): bring redactPii() coverage and tests up to date with current log sites #5480

Description

@ten9876

Summary

redactPii() in src/core/AsyncLogWriter.cpp has grown one regex at a time as log sites were added. Its coverage and its tests should be brought in line with the fields the app now logs, and the rule set should be easier to extend than hand-written lookbehinds.

Follow-up from the review discussion on #5130 (the exe= path line), which noted that the redactor has no rule for filesystem paths and that the existing path-logging sites predate it.

Scope

Rules to add, each with a case in tests/async_log_writer_test.cpp:

  • Home-directory prefix → ~ (handle both slash styles on Windows, and a /home/<user> or C:\Users\<user> segment when QDir::homePath() differs from the path's actual root, e.g. Flatpak or elevated runs)
  • IPv6 literals, same last-group treatment as the IPv4 rule
  • Email addresses (SmartLinkClient currently truncates by hand at the call site; move the guarantee into the redactor)
  • user <name> / username= style fields (see IcomSession.cpp control-stream login line)
  • Hostnames following connecting to / connected to / pin for keywords (WanConnection, MqttClient, PgxlConnection)
  • Maidenhead grid fields alongside the existing lat/lon rule

Structure:

  • Replace the per-rule regex literals with a small {keyword, keep-prefix} table the patterns are generated from, mirroring how SettingsSanitizer keys off SettingsCredentialPolicy, so a new field is one table row and one test
  • Negative corpus test: feed a fixture of representative status/protocol lines (Flex status, SmartLink WAN frames, MQTT, Icom login) through the redactor and assert none of the fixture values survive
  • Short section in docs/ listing what the redactor does and does not scrub, so reviewers can check a new log site against it

Not in scope

  • Changing any individual log site; the fix belongs in the redactor so existing sites are covered together
  • SettingsSanitizer (already policy-driven)

Verification

Activity

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

Metadata

Metadata

Assignees

Labels

maintainer-reviewRequires maintainer review before any action is takenpriority: lowLow priorityrefactorCode cleanup, restructuring, or consolidation — no user-visible behavior change

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions