Skip to content

Commit 9f6535b

Browse files
Pre-fill issue reports with a redacted log tail. Principle VII.
Fixes #3705 ## Summary - Pre-fill Help → Support → Submit Bug Report with system/radio context and a bounded recent-log tail. - Re-run centralized privacy redaction at the public issue-render boundary and fall back to the clipboard when the encoded GitHub URL would be too long. - Keep SmartLink account names and GPS/location coordinates out of logs at their source, including the WAN certificate-decision path. - Add centralized name/coordinate redaction as a fail-safe so those fields cannot survive into disk logs or the public issue body if a future logging path submits them. - Replace the absolute no-personal-data claim with a concrete privacy notice and a prompt to review before submission. ## Verification - Full macOS RelWithDebInfo build with 8 parallel jobs. - `async_log_writer_test` passes with planted SmartLink names and GPS/location coordinates absent from disk logs. - `issue_report_test` passes with the same planted fields absent from the generated public report. - `tools/check_engine_boundary.py --strict` passes with no new violations. Squashed-from: #4314 Co-authored-by: aethersdr-agent[bot] <273844287+aethersdr-agent[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Pat Jensen <patjensen@gmail.com>
1 parent 6af248b commit 9f6535b

13 files changed

Lines changed: 427 additions & 19 deletions

CMakeLists.txt

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,7 @@ set(CORE_SOURCES
600600
src/core/LogManager.cpp
601601
src/core/ShortcutManager.cpp
602602
src/core/SupportBundle.cpp
603+
src/core/IssueReport.cpp
603604
src/core/DeviceDiagnostics.cpp
604605
src/core/SerialPortController.cpp
605606
src/core/FlexControlManager.cpp
@@ -3453,6 +3454,21 @@ if(UNIX)
34533454
endif()
34543455
set_target_properties(async_log_writer_test PROPERTIES AUTOMOC ON)
34553456

3457+
# Pre-filled GitHub issue body + redaction-at-render guarantee (#3705).
3458+
# IssueReport.cpp depends only on redactPii (AsyncLogWriter.cpp) — no
3459+
# RadioModel — so the redaction contract is unit-testable in isolation.
3460+
add_executable(issue_report_test
3461+
tests/issue_report_test.cpp
3462+
src/core/IssueReport.cpp
3463+
src/core/AsyncLogWriter.cpp
3464+
)
3465+
target_include_directories(issue_report_test PRIVATE src src/core)
3466+
target_link_libraries(issue_report_test PRIVATE Qt6::Core)
3467+
if(UNIX)
3468+
target_link_libraries(issue_report_test PRIVATE pthread)
3469+
endif()
3470+
set_target_properties(issue_report_test PROPERTIES AUTOMOC ON)
3471+
34563472
add_executable(perf_telemetry_test
34573473
tests/perf_telemetry_test.cpp
34583474
src/core/PerfTelemetry.cpp

src/core/AsyncLogWriter.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,26 @@ QString redactPii(const QString& msg)
8383
QRegularExpression::CaseInsensitiveOption);
8484
out.replace(*bearerRe, QStringLiteral("\\1\\2\\3***REDACTED***"));
8585

86+
// SmartLink account-holder names are not useful for diagnostics. Scrub
87+
// the protocol's snake_case fields and common camelCase equivalents as a
88+
// final defense if a raw or parsed user-settings message reaches logging.
89+
static const QRegularExpression* personalNameRe = new QRegularExpression(
90+
R"(\b(first_?name|last_?name|full_?name)(["']?\s*[:=]\s*)(?:"[^"]*"|'[^']*'|[^\s#|,}\]]+))",
91+
QRegularExpression::CaseInsensitiveOption);
92+
out.replace(*personalNameRe, QStringLiteral("\\1\\2***REDACTED***"));
93+
94+
// GPS coordinates likewise have no diagnostic value. Cover the Flex
95+
// lat=/lon= status spelling, long-form/JSON spellings, optional gps_
96+
// prefixes, and a numeric pair carried in a location=/gps= field.
97+
static const QRegularExpression* coordinateFieldRe = new QRegularExpression(
98+
R"(\b((?:gps[_-]?)?(?:lat(?:itude)?|lon(?:gitude)?))(["']?\s*[:=]\s*)(?:"[^"]*"|'[^']*'|[^\s#|,}\]]+))",
99+
QRegularExpression::CaseInsensitiveOption);
100+
out.replace(*coordinateFieldRe, QStringLiteral("\\1\\2***REDACTED***"));
101+
static const QRegularExpression* coordinatePairRe = new QRegularExpression(
102+
R"(\b(gps(?:[_-]?location)?|location)(["']?\s*[:=]\s*)[-+]?\d{1,3}(?:\.\d+)?\s*[,/]\s*[-+]?\d{1,3}(?:\.\d+)?)",
103+
QRegularExpression::CaseInsensitiveOption);
104+
out.replace(*coordinatePairRe, QStringLiteral("\\1\\2***REDACTED***"));
105+
86106
// MAC addresses: 00-1C-2D-05-37-2A -> **-**-**-**-**-2A
87107
// 00:1C:2D:05:37:2A -> **:**:**:**:**:2A
88108
static const QRegularExpression* macRe = new QRegularExpression(

src/core/AsyncLogWriter.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
namespace AetherSDR {
1616

1717
// Apply log-style PII redaction (IPv4 → *.*.*.X, radio serial →
18-
// ****-****-****-XXXX, Auth0 tokens, MAC addresses). Defined in
18+
// ****-****-****-XXXX, Auth0 tokens, SmartLink personal names, GPS
19+
// coordinates, and MAC addresses). Defined in
1920
// AsyncLogWriter.cpp; thread-safe (pure function over local copies of
2021
// static const QRegularExpressions). Used by AsyncLogWriter for every
2122
// log line and by SupportBundle to scrub radio-info.json fields per

src/core/IssueReport.cpp

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
#include "IssueReport.h"
2+
#include "AsyncLogWriter.h" // redactPii — GHSA-ccrg-j8cp-qhc4
3+
4+
namespace AetherSDR {
5+
6+
QString buildIssueReport(const SupportBundle::SystemInfo& sys,
7+
const SupportBundle::RadioInfo& radio,
8+
const QString& logTail)
9+
{
10+
QString body;
11+
12+
// State the concrete privacy guarantees without implying that arbitrary
13+
// user-authored log text can be proven free of every kind of PII.
14+
body += "<!-- Pre-filled by AetherSDR (Help \xE2\x86\x92 File an Issue). "
15+
"Known sensitive fields are redacted: authentication tokens, "
16+
"network/radio identifiers, GPS coordinates, and SmartLink "
17+
"account names. Please review the report, then "
18+
"replace the italic placeholders with your own words. -->\n\n";
19+
20+
body += "### What happened?\n";
21+
body += "_Describe what went wrong (e.g. \"the waterfall freezes after "
22+
"about 10 minutes\")._\n\n";
23+
24+
body += "### What did you expect?\n";
25+
body += "_Describe the expected behavior._\n\n";
26+
27+
body += "### Steps to reproduce\n";
28+
body += "1. _First step_\n";
29+
body += "2. _\xE2\x80\xA6_\n\n";
30+
31+
body += "### Radio model & firmware\n";
32+
if (radio.connected) {
33+
// Serial/IP are PII — redact to the same form used in logs so support
34+
// can correlate without seeing cleartext. Callsign is FCC public
35+
// record; model/firmware/protocol are not sensitive.
36+
body += QString("- Model: %1\n").arg(radio.model);
37+
body += QString("- Firmware: %1\n").arg(radio.firmware);
38+
if (!radio.protocolVersion.isEmpty()) {
39+
body += QString("- Protocol: %1\n").arg(radio.protocolVersion);
40+
}
41+
if (!radio.callsign.isEmpty()) {
42+
body += QString("- Callsign: %1\n").arg(radio.callsign);
43+
}
44+
if (!radio.serial.isEmpty()) {
45+
body += QString("- Serial: %1\n").arg(redactPii(radio.serial));
46+
}
47+
body += "- Connection: connected\n\n";
48+
} else {
49+
body += "- Connection: not connected\n\n";
50+
}
51+
52+
body += "### OS & version\n";
53+
body += QString("- AetherSDR: %1\n").arg(sys.aetherVersion);
54+
body += QString("- Qt: %1\n").arg(sys.qtVersion);
55+
body += QString("- OS: %1 (kernel %2)\n").arg(sys.osName, sys.kernelVersion);
56+
body += QString("- Arch: %1\n").arg(sys.cpuArch);
57+
body += QString("- Build: %1\n\n").arg(sys.buildDate);
58+
59+
body += "### Recent log\n";
60+
if (logTail.isEmpty()) {
61+
body += "_(Recent log omitted from this link to keep it short \xE2\x80\x94 "
62+
"see the attached support bundle, or paste it from your "
63+
"clipboard.)_\n";
64+
} else {
65+
body += QString("Last %1 lines, secret-redacted:\n\n")
66+
.arg(kIssueLogTailLines);
67+
// Redact at the render boundary: even though the on-disk tail is
68+
// already scrubbed by AsyncLogWriter::formatLine, re-run redactPii so
69+
// the guarantee holds regardless of the tail's provenance.
70+
body += "```text\n";
71+
body += redactPii(logTail).trimmed();
72+
body += "\n```\n";
73+
}
74+
75+
return body;
76+
}
77+
78+
} // namespace AetherSDR

src/core/IssueReport.h

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
#pragma once
2+
3+
#include "SupportBundle.h"
4+
5+
#include <QString>
6+
7+
namespace AetherSDR {
8+
9+
// Pre-filled GitHub issue body (#3705).
10+
//
11+
// buildIssueReport() renders the same headed-Markdown sections the AI-prompt
12+
// template uses (What happened / Expected / Steps / Radio / OS) with the
13+
// SupportBundle snapshot filled in and clear placeholders for the user's
14+
// prose. When a log tail is supplied it is embedded as a fenced ```text
15+
// block; when it is empty a short "omitted — see support bundle" note is
16+
// substituted so the "graceful degrade" path still reads sensibly.
17+
//
18+
// Redaction guarantee: the log tail is passed through redactPii() here, at
19+
// the render boundary, before it can reach the body or the clipboard — the
20+
// same scrub applied to every on-disk log line (GHSA-ccrg-j8cp-qhc4). The
21+
// on-disk tail is already redacted at capture; this second pass is
22+
// belt-and-suspenders so the guarantee holds no matter where the tail came
23+
// from. Radio serial/IP are scrubbed too; callsign/model are public and
24+
// left intact. This TU depends only on redactPii (no RadioModel), so the
25+
// redaction contract is unit-testable in isolation (issue_report_test).
26+
27+
// Last N lines of the recent log to include in the issue body.
28+
inline constexpr int kIssueLogTailLines = 100;
29+
30+
// Conservative ceiling on the assembled issues/new URL. A pre-filled URL
31+
// can only carry text in the query string and browsers/GitHub reject or
32+
// truncate over-long URLs; above this the caller re-renders without the log
33+
// block and delivers the full report via the clipboard/bundle instead.
34+
inline constexpr int kIssueUrlMaxBytes = 8000;
35+
36+
// Build the headed-Markdown issue body. logTail empty => log block replaced
37+
// by the omission note. Non-empty => redacted and embedded in a fenced
38+
// block headed "last kIssueLogTailLines lines, secret-redacted".
39+
QString buildIssueReport(const SupportBundle::SystemInfo& sys,
40+
const SupportBundle::RadioInfo& radio,
41+
const QString& logTail);
42+
43+
} // namespace AetherSDR

src/core/RadioConnection.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,9 @@ void RadioConnection::onHeartbeat()
293293

294294
void RadioConnection::processLine(const QString& line)
295295
{
296+
// GPS coordinates are never useful in a support log. Drop the raw status
297+
// at the source; AsyncLogWriter also scrubs coordinate-shaped fields as a
298+
// defense against future logging paths.
296299
const bool isGps = line.contains("|gps ");
297300
bool isPingReply = false;
298301
if (m_lastPingSeq && line.startsWith("R")) {
@@ -305,8 +308,9 @@ void RadioConnection::processLine(const QString& line)
305308
m_lastPingSeq = 0;
306309
}
307310
}
308-
if (!isGps && !isPingReply)
311+
if (!isGps && !isPingReply) {
309312
qCDebug(lcConnection) << "RX:" << line;
313+
}
310314

311315
ParsedMessage msg = CommandParser::parseLine(line);
312316
emit messageReceived(msg);

src/core/SmartLinkClient.cpp

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -402,19 +402,27 @@ void SmartLinkClient::onPingTimer()
402402

403403
void SmartLinkClient::parseMessage(const QString& msg)
404404
{
405-
// Redact any token= values from log output
406-
if (msg.contains("token="))
405+
const bool isUserSettings = msg.startsWith("application user_settings");
406+
407+
// SmartLink account-holder names have no diagnostic value. Do not enqueue
408+
// the raw user-settings message at all; the centralized writer redaction
409+
// remains a final defense for any future logging path.
410+
if (isUserSettings) {
411+
qCDebug(lcSmartLink)
412+
<< "SmartLink RX: application user_settings (personal fields omitted)";
413+
} else if (msg.contains("token=")) {
407414
qCDebug(lcSmartLink) << "SmartLink RX:" << msg.left(msg.indexOf("token=") + 6) + "***REDACTED***";
408-
else
415+
} else {
409416
qCDebug(lcSmartLink) << "SmartLink RX:" << msg.left(120);
417+
}
410418

411419
if (msg.startsWith("radio list ")) {
412420
parseRadioList(msg);
413421
} else if (msg.startsWith("radio connect_ready")) {
414422
parseConnectReady(msg);
415423
} else if (msg.startsWith("application info")) {
416424
parseApplicationInfo(msg);
417-
} else if (msg.startsWith("application user_settings")) {
425+
} else if (isUserSettings) {
418426
parseUserSettings(msg);
419427
} else if (msg.startsWith("application registration_invalid")) {
420428
qCWarning(lcSmartLink) << "SmartLinkClient: registration invalid — token rejected";
@@ -542,8 +550,7 @@ void SmartLinkClient::parseUserSettings(const QString& msg)
542550
else if (key == "first_name") m_userFirstName = val;
543551
else if (key == "last_name") m_userLastName = val;
544552
}
545-
qCDebug(lcSmartLink) << "SmartLinkClient: user:" << m_userFirstName << m_userLastName
546-
<< "callsign:" << m_userCallsign;
553+
qCDebug(lcSmartLink) << "SmartLinkClient: user settings received (personal fields omitted)";
547554
}
548555

549556
void SmartLinkClient::parseTestResults(const QString& msg)

src/core/SupportBundle.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <QJsonDocument>
1515
#include <QJsonObject>
1616
#include <QSaveFile>
17+
#include <QStringList>
1718
#include <QSysInfo>
1819
#include <QTemporaryDir>
1920
#include <QUrl>
@@ -210,6 +211,39 @@ QString SupportBundle::createBundle(const RadioInfo& radio)
210211
return archivePath;
211212
}
212213

214+
QString SupportBundle::recentLogTail(int lines)
215+
{
216+
if (lines <= 0)
217+
return {};
218+
219+
auto& logMgr = LogManager::instance();
220+
logMgr.flushLog();
221+
222+
QFile f(logMgr.logFilePath());
223+
if (!f.open(QIODevice::ReadOnly | QIODevice::Text))
224+
return {};
225+
226+
// Read a bounded tail; ~200KB comfortably holds kIssueLogTailLines worth
227+
// of formatted lines without loading a large log in full.
228+
constexpr qint64 kMaxRead = 200 * 1024;
229+
const qint64 size = f.size();
230+
const bool seeked = size > kMaxRead;
231+
if (seeked)
232+
f.seek(size - kMaxRead);
233+
const QString text = QString::fromUtf8(f.readAll());
234+
f.close();
235+
236+
QStringList all = text.split('\n');
237+
// A mid-line seek can leave a partial first line; drop it so the tail
238+
// starts on a clean line boundary.
239+
if (seeked && !all.isEmpty())
240+
all.removeFirst();
241+
242+
if (all.size() > lines)
243+
all = all.mid(all.size() - lines);
244+
return all.join('\n').trimmed();
245+
}
246+
213247
void SupportBundle::openEmailClient(const QString& bundlePath,
214248
const SystemInfo& sys,
215249
const RadioInfo& radio)

src/core/SupportBundle.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,13 @@ class SupportBundle {
4141
// Returns the full path to the archive, or empty string on failure.
4242
static QString createBundle(const RadioInfo& radio);
4343

44+
// Flush the log pipeline and return the last `lines` lines of the most
45+
// recent on-disk log (#3705). The on-disk log is already secret-redacted
46+
// at capture (AsyncLogWriter::formatLine → redactPii), so this is the
47+
// redacted stream; callers still re-scrub at the render boundary as
48+
// belt-and-suspenders. Reads a bounded tail (~200KB) to stay responsive.
49+
static QString recentLogTail(int lines);
50+
4451
// Open the default email client with pre-filled subject/body.
4552
static void openEmailClient(const QString& bundlePath,
4653
const SystemInfo& sys,

src/core/WanConnection.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,11 @@ void WanConnection::onHeartbeat()
362362

363363
void WanConnection::processLine(const QString& line)
364364
{
365+
// GPS coordinates are never useful in a support log. Compute this before
366+
// the certificate-decision early return so that path cannot bypass the
367+
// normal GPS payload suppression below.
368+
const bool isGps = line.contains("|gps ");
369+
365370
// GHSA-wfx7-w6p8-4jr2 phase 2: while the operator is being prompted
366371
// about a cert mismatch, the TLS channel stays open but we must not
367372
// act on anything that arrives. A MITM on the path (the exact threat
@@ -372,13 +377,17 @@ void WanConnection::processLine(const QString& line)
372377
// to connected() — so we extend the suppression symmetrically:
373378
// nothing gets parsed until the operator's decision lands.
374379
if (m_awaitingCertDecision) {
375-
qCDebug(lcSmartLink)
376-
<< "WAN RX (suppressed during cert-pin decision):" << line;
380+
if (isGps) {
381+
qCDebug(lcSmartLink)
382+
<< "WAN RX (suppressed during cert-pin decision; GPS payload omitted)";
383+
} else {
384+
qCDebug(lcSmartLink)
385+
<< "WAN RX (suppressed during cert-pin decision):" << line;
386+
}
377387
return;
378388
}
379389

380390
// Suppress noisy messages
381-
const bool isGps = line.contains("|gps ");
382391
bool isPingReply = false;
383392
if (m_lastPingSeq && line.startsWith("R")) {
384393
isPingReply = line.startsWith(QString("R%1|").arg(m_lastPingSeq));
@@ -387,8 +396,9 @@ void WanConnection::processLine(const QString& line)
387396
m_lastPingSeq = 0;
388397
}
389398
}
390-
if (!isGps && !isPingReply)
399+
if (!isGps && !isPingReply) {
391400
qCDebug(lcSmartLink) << "WAN RX:" << line;
401+
}
392402

393403
ParsedMessage msg = CommandParser::parseLine(line);
394404
emit messageReceived(msg);

0 commit comments

Comments
 (0)