From d04df03d7ab9cfdd859d20a9a5cfdbbbf35373b6 Mon Sep 17 00:00:00 2001 From: Tori Date: Mon, 27 Jul 2026 00:08:27 -0500 Subject: [PATCH 1/7] fix: scrub Nostr keys from 14 more log lines + add a CI check (#836) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AGENTS.md:48 says to scrub logs that might leak invoices or Nostr keys. found 5 more scattered across the daemon (issue #836) but no mechanism to stop the pattern from recurring. Adds scripts/check_log_redaction.py, wired into ci.yml, which fails the build on any tracing::{trace,debug,info,warn,error}! call that interpolates a Nostr key/identity-shaped argument. Running it against the current codebase surfaced 9 more instances beyond the 5 already documented, including util::send_dm logging both sender and receiver on every outbound protocol message. Fixes all 14 with the same one-line-per-site treatment #834/#835 used, so the new check ships green. cancel.rs: removing taker_pubkey from one log line left the parameter fully unused across cancel_order_by_taker_inner and its only caller, cancel_order_by_taker — dropped from both signatures and their call sites rather than silenced. --- .github/workflows/ci.yml | 9 ++- scripts/check_log_redaction.py | 124 +++++++++++++++++++++++++++++++++ src/app.rs | 11 ++- src/app/admin_take_dispute.rs | 12 ++-- src/app/bond/payout.rs | 6 +- src/app/cancel.rs | 41 ++--------- src/app/last_trade_index.rs | 8 +-- src/db.rs | 7 +- src/rpc/service.rs | 6 +- src/scheduler.rs | 7 +- src/util.rs | 17 +++-- 11 files changed, 167 insertions(+), 81 deletions(-) create mode 100644 scripts/check_log_redaction.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 000a4cc8..2f8186fd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,6 +15,13 @@ jobs: - name: cargo fmt -- --check run: cargo fmt --all -- --check + log-redaction: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - name: Check for Nostr key/identity leaks in tracing calls + run: python3 scripts/check_log_redaction.py + clippy: runs-on: ubuntu-latest steps: @@ -32,7 +39,7 @@ jobs: test: runs-on: ubuntu-latest - needs: [fmt, clippy] + needs: [fmt, clippy, log-redaction] steps: - uses: actions/checkout@v6 - uses: dtolnay/rust-toolchain@stable diff --git a/scripts/check_log_redaction.py b/scripts/check_log_redaction.py new file mode 100644 index 00000000..08a8a575 --- /dev/null +++ b/scripts/check_log_redaction.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +"""CI gate for AGENTS.md:48 ("Scrub logs that might leak invoices or Nostr +keys"). Flags any `tracing::{trace,debug,info,warn,error}!(...)` call whose +argument list interpolates an identifier that looks like a Nostr +key/identity, so a new log-scrubbing regression (issue #836's pattern) fails +CI instead of shipping quietly. + +Not a Rust parser: string literals are skipped so a key-shaped *word* inside +a log message's own text doesn't trigger a false positive, but the paren +matching is a plain depth counter — a macro call containing a raw string +literal with unbalanced parens would confuse it. None of this codebase's +tracing calls do that today; if one ever needs to, exempt it inline (see +ALLOW_COMMENT below) rather than fighting the matcher. +""" + +import re +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +SRC_ROOT = REPO_ROOT / "src" + +MACRO_RE = re.compile(r"\b(?:tracing::)?(trace|debug|info|warn|error)!\s*\(") + +# Identifiers that name a Nostr key/identity in this codebase. Extend this +# list, don't loosen it to a bare `key` — that also matches innocuous things +# like HashMap iteration variables. +SUSPICIOUS_RE = re.compile( + r"\b(" + r"\w*pubkey\w*" + r"|identity" + r"|sender" + r"|master_key" + r"|trade_key" + r"|nsec\w*" + r"|priv(?:ate)?_?key\w*" + r")\b" +) + +# A `// pubkey-log-allow: ` comment on the line right before a +# flagged macro call exempts it — for a documented, deliberate exception +# (e.g. an already-redacted/truncated value) rather than a silent miss. +ALLOW_COMMENT = "pubkey-log-allow:" + + +def find_call_span(text: str, open_paren: int) -> tuple[int, str]: + """Return (index just past the `)` matching `text[open_paren] == '('`, + the call's source with string-literal *contents* blanked out). + + Blanking string contents (not just skipping them for paren-matching) + matters: a format string's own English prose can contain a key-shaped + word ("...pubkey in order...") that isn't an interpolated argument at + all — only the blanked version should be searched for suspicious + identifiers, or every message that merely *mentions* a pubkey false- + positives. + """ + depth = 0 + i = open_paren + n = len(text) + out = [] + while i < n: + c = text[i] + if c == '"': + start = i + i += 1 + while i < n and text[i] != '"': + i += 2 if text[i] == "\\" else 1 + i += 1 + out.append('"' * (i - start)) + continue + out.append(c) + if c == "(": + depth += 1 + elif c == ")": + depth -= 1 + if depth == 0: + return i + 1, "".join(out) + i += 1 + return n, "".join(out) # unbalanced — best effort + + +def line_before(text: str, index: int) -> str: + line_start = text.rfind("\n", 0, index) + prev_start = text.rfind("\n", 0, line_start) + 1 if line_start != -1 else 0 + return text[prev_start:line_start] if line_start != -1 else "" + + +def check_file(path: Path) -> list[tuple[int, str]]: + text = path.read_text(encoding="utf-8") + violations = [] + for m in MACRO_RE.finditer(text): + open_paren = text.index("(", m.end() - 1) + _end, code_only = find_call_span(text, open_paren) + found = SUSPICIOUS_RE.search(code_only) + if not found: + continue + if ALLOW_COMMENT in line_before(text, m.start()): + continue + line_no = text.count("\n", 0, m.start()) + 1 + violations.append((line_no, found.group(0))) + return violations + + +def main() -> int: + total = 0 + for path in sorted(SRC_ROOT.rglob("*.rs")): + for line_no, ident in check_file(path): + rel = path.relative_to(REPO_ROOT) + print( + f"{rel}:{line_no}: tracing call interpolates `{ident}` — " + f"looks like a Nostr key/identity (AGENTS.md:48). Drop it from " + f"the log line, or mark a deliberate exception with a " + f"`// {ALLOW_COMMENT} ` comment on the line above." + ) + total += 1 + if total: + print(f"\n{total} log-redaction violation(s) found.", file=sys.stderr) + return 1 + print("check_log_redaction: clean.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/src/app.rs b/src/app.rs index fd9a9b38..bd097ea4 100644 --- a/src/app.rs +++ b/src/app.rs @@ -341,9 +341,9 @@ async fn accept_event( // we decrypt. New orders/takes legitimately arrive // here — so does spam, hence the PoW toll. if !gate.is_known(&event.pubkey.to_string()) && !event.check_pow(pow_first_contact) { + // No key in the log line — sender pubkey (AGENTS.md:48). tracing::info!( - "Dropping first-contact kind-14 event from unknown key {} below pow_first_contact ({} bits)", - event.pubkey, + "Dropping first-contact kind-14 event below pow_first_contact ({} bits)", pow_first_contact ); return None; @@ -386,11 +386,8 @@ async fn accept_event( // signature — unwrap_message already verified it, so if identity // and sender differ here without a signature we bail out. if unwrapped.identity != unwrapped.sender && unwrapped.signature.is_none() { - tracing::warn!( - "Missing inner signature: identity {} differs from trade key {}", - unwrapped.identity, - unwrapped.sender - ); + // No keys in the log line — identity/trade key (AGENTS.md:48). + tracing::warn!("Missing inner signature: identity differs from trade key"); return None; } diff --git a/src/app/admin_take_dispute.rs b/src/app/admin_take_dispute.rs index 6d442e87..f37f72c6 100644 --- a/src/app/admin_take_dispute.rs +++ b/src/app/admin_take_dispute.rs @@ -92,12 +92,9 @@ pub async fn pubkey_event_can_solve( ) -> bool { let sender_pubkey = ev_pubkey.to_string(); - // Is mostro admin taking dispute? - info!( - "admin pubkey {} -event pubkey {} ", - my_keys.public_key().to_string(), - sender_pubkey - ); + // Is mostro admin taking dispute? No keys in the log line — admin/event + // pubkeys (AGENTS.md:48). + info!("Checking whether the dispute event was sent by the mostro admin"); if sender_pubkey == my_keys.public_key().to_string() && matches!(status, DisputeStatus::InProgress | DisputeStatus::Initiated) { @@ -192,7 +189,8 @@ pub async fn admin_take_dispute_action( dispute.solver_pubkey = Some(event.identity.to_string()); dispute.taken_at = Timestamp::now().as_secs() as i64; - info!("Dispute {} taken by {}", dispute.id, event.identity); + // No key in the log line — solver identity (AGENTS.md:48). + info!("Dispute {} taken by a solver", dispute.id); // Save it to DB dispute diff --git a/src/app/bond/payout.rs b/src/app/bond/payout.rs index 05cde252..7f41fa6e 100644 --- a/src/app/bond/payout.rs +++ b/src/app/bond/payout.rs @@ -454,11 +454,11 @@ async fn request_payout_invoice( return Ok(()); } + // No key in the log fields — recipient pubkey (AGENTS.md:48). info!( bond_id = %bond.id, order_id = %bond.order_id, amount_sats = counterparty_share, - recipient = %recipient_pubkey, slashed_at, attempt = bond.invoice_request_attempts + 1, "bond payout: requesting invoice from counterparty" @@ -1349,18 +1349,18 @@ pub async fn add_bond_invoice_action( match apply_payout_invoice(pool, &bond, &payment_request, now, claim_window_seconds).await? { InvoiceApplyOutcome::Persisted => { + // No key in the log fields — sender pubkey (AGENTS.md:48). info!( bond_id = %bond.id, order_id = %bond.order_id, - sender = %sender, "bond payout: invoice accepted; awaiting scheduler tick for payout" ); } InvoiceApplyOutcome::Resurrected => { + // No key in the log fields — sender pubkey (AGENTS.md:48). info!( bond_id = %bond.id, order_id = %bond.order_id, - sender = %sender, "bond payout: Failed -> PendingPayout (user submitted fresh invoice within claim window); payout_attempts reset, awaiting scheduler tick for payout" ); } diff --git a/src/app/cancel.rs b/src/app/cancel.rs index ab22976d..66ed15e8 100644 --- a/src/app/cancel.rs +++ b/src/app/cancel.rs @@ -216,7 +216,6 @@ async fn cancel_order_by_taker( my_keys: &Keys, request_id: Option, ln_client: &mut L, - taker_pubkey: PublicKey, ) -> Result<(), MostroError> { let order_id = order.id; let sender_str = event.sender.to_string(); @@ -263,16 +262,7 @@ async fn cancel_order_by_taker( // No surviving bonds: run the full reset-and-republish path so // the order goes back into the book exactly as before. - cancel_order_by_taker_inner( - pool, - event, - order, - my_keys, - request_id, - ln_client, - taker_pubkey, - ) - .await + cancel_order_by_taker_inner(pool, event, order, my_keys, request_id, ln_client).await } async fn cancel_order_by_taker_inner( @@ -282,7 +272,6 @@ async fn cancel_order_by_taker_inner( my_keys: &Keys, request_id: Option, ln_client: &mut L, - taker_pubkey: PublicKey, ) -> Result<(), MostroError> { // Cancel hold invoice if present if let Some(hash) = &order.hash { @@ -318,10 +307,8 @@ async fn cancel_order_by_taker_inner( .await .map_err(|e| MostroInternalErr(ServiceError::NostrError(e.to_string())))?; - info!( - "{}: Canceled order Id {} republishing order", - taker_pubkey, order.id - ); + // No key in the log line — taker pubkey (AGENTS.md:48). + info!("Canceled order Id {} republishing order", order.id); // Notify the creator about the republished order after the taker-side cancellation flow completes notify_creator(&order_updated, request_id).await?; @@ -568,16 +555,7 @@ async fn cancel_action_generic( .as_deref() .is_some_and(|p| p == sender_str && p != order.creator_pubkey); if bond_match || order_taker_match { - cancel_order_by_taker( - pool, - event, - order, - my_keys, - request_id, - ln_client, - event.sender, - ) - .await?; + cancel_order_by_taker(pool, event, order, my_keys, request_id, ln_client).await?; return Ok(()); } return Err(MostroCantDo(CantDoReason::IsNotYourOrder)); @@ -698,16 +676,7 @@ async fn cancel_not_active_order( ) .await?; } else if event.sender == taker_pubkey { - cancel_order_by_taker( - pool, - event, - order, - my_keys, - request_id, - ln_client, - taker_pubkey, - ) - .await?; + cancel_order_by_taker(pool, event, order, my_keys, request_id, ln_client).await?; } else { return Err(MostroCantDo(CantDoReason::InvalidPubkey)); } diff --git a/src/app/last_trade_index.rs b/src/app/last_trade_index.rs index d634ad2d..e559b38a 100644 --- a/src/app/last_trade_index.rs +++ b/src/app/last_trade_index.rs @@ -82,11 +82,9 @@ pub async fn last_trade_index( .as_json() .map_err(|_| MostroError::MostroInternalErr(ServiceError::MessageSerializationError))?; - // Print the last trade index message - tracing::info!( - "User with pubkey: {} requested last trade index", - user.pubkey - ); + // Print the last trade index message. No key in the log line — user + // pubkey (AGENTS.md:48). + tracing::info!("User requested last trade index"); tracing::info!("Last trade index: {}", user.last_trade_index); // Send message back to the requester diff --git a/src/db.rs b/src/db.rs index 6c9bdfd1..a468c9a5 100644 --- a/src/db.rs +++ b/src/db.rs @@ -1543,11 +1543,8 @@ pub async fn is_assigned_solver( solver_pubkey: &str, order_id: Uuid, ) -> Result { - tracing::info!( - "Solver_pubkey: {} assigned to order {}", - solver_pubkey, - order_id - ); + // No key in the log line — solver pubkey (AGENTS.md:48). + tracing::info!("Solver assigned to order {}", order_id); let result = sqlx::query( "SELECT EXISTS(SELECT 1 FROM disputes WHERE solver_pubkey = ? AND order_id = ?)", ) diff --git a/src/rpc/service.rs b/src/rpc/service.rs index 854230a0..fff19b59 100644 --- a/src/rpc/service.rs +++ b/src/rpc/service.rs @@ -318,10 +318,8 @@ impl AdminService for AdminServiceImpl { request: Request, ) -> Result, Status> { let req = request.into_inner(); - info!( - "Received add solver request for pubkey: {}", - req.solver_pubkey - ); + // No key in the log line — solver pubkey (AGENTS.md:48). + info!("Received add solver request"); match self .call_admin_add_solver(req.solver_pubkey, req.request_id) diff --git a/src/scheduler.rs b/src/scheduler.rs index 7b006a8c..e37015c7 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -310,11 +310,10 @@ pub(crate) async fn notify_users_canceled_order( // Neutral wording on purpose: this helper serves several closure paths // (waiting-state timeout, hold-invoice cancel, actual cancels), so the - // specific cause is logged by each caller, not here. + // specific cause is logged by each caller, not here. No keys in the log + // line (AGENTS.md, Security & Configuration Tips). tracing::info!( - "Notifying maker {} and taker {} that order {} was not completed", - maker_pubkey.to_string(), - taker_pubkey.to_string(), + "Notifying maker and taker that order {} was not completed", old_order.id ); diff --git a/src/util.rs b/src/util.rs index 8ead10e4..d30c4d46 100644 --- a/src/util.rs +++ b/src/util.rs @@ -670,11 +670,11 @@ pub async fn send_dm( payload: &str, expiration: Option, ) -> Result<(), MostroError> { - info!( - "sender key {} - receiver key {}", - sender_keys.public_key().to_hex(), - receiver_pubkey.to_hex() - ); + // No keys in the log line — sender/receiver pubkeys (AGENTS.md:48). This + // is the highest-frequency send path in the daemon (every outbound + // protocol message), so it's also the highest-blast-radius instance of + // this pattern. + info!("Sending DM"); let mut message = Message::from_json(payload) .map_err(|_| MostroInternalErr(ServiceError::MessageSerializationError))?; @@ -717,11 +717,10 @@ pub async fn send_dm( ) .await?; + // No key in the log line — receiver pubkey (AGENTS.md:48). info!( - "Sending message, Event ID: {} to {} with payload: {:#?}", - event.id, - receiver_pubkey.to_hex(), - payload + "Sending message, Event ID: {} with payload: {:#?}", + event.id, payload ); if let Ok(client) = get_nostr_client() { From 322c6ae67f114da5fe6bc5a99bbc8bfdd1cf9fd4 Mon Sep 17 00:00:00 2001 From: Tori Date: Tue, 28 Jul 2026 22:58:55 -0500 Subject: [PATCH 2/7] fix(util): drop cleartext payload from send_dm log line admin_take_dispute_action sends a Payload::Peer{pubkey} to both parties via send_dm; the trailing info! logged that payload in full, leaking the solver's Nostr pubkey (AGENTS.md:48). --- src/util.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/util.rs b/src/util.rs index d30c4d46..7bb34190 100644 --- a/src/util.rs +++ b/src/util.rs @@ -717,11 +717,9 @@ pub async fn send_dm( ) .await?; - // No key in the log line — receiver pubkey (AGENTS.md:48). - info!( - "Sending message, Event ID: {} with payload: {:#?}", - event.id, payload - ); + // No key in the log line — payload can carry a Nostr pubkey (e.g. + // Payload::Peer sent by admin_take_dispute_action) (AGENTS.md:48). + info!("Sending message, Event ID: {}", event.id); if let Ok(client) = get_nostr_client() { client From 0f96b74224254fbdbc7e0f5e9863024c88a19fe7 Mon Sep 17 00:00:00 2001 From: Tori Date: Tue, 28 Jul 2026 22:59:06 -0500 Subject: [PATCH 3/7] fix(scheduler): remove debug println! leaking order pubkeys job_cancel_orders printed the full edited Order via println!, which carries buyer/seller/master pubkeys. tracing:: macros go through the new log-redaction CI gate; bare println! doesn't, so this slipped past it (AGENTS.md:48). --- src/scheduler.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/scheduler.rs b/src/scheduler.rs index e37015c7..9da5ff83 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -602,7 +602,6 @@ async fn job_cancel_orders(ctx: AppContext) { // Get edited order to use for update_order_event let edited_order = if let Ok(edited_order) = edited_order { - println!("Edited order: {:?}", edited_order); edited_order } else { tracing::warn!("Error editing pubkeys in order {} cancel", order.id); From ea08a7e75f491aa805df8f0b7005b110e725913d Mon Sep 17 00:00:00 2001 From: Tori Date: Tue, 28 Jul 2026 22:59:16 -0500 Subject: [PATCH 4/7] test(check_log_redaction): match brace/bracket macros and identity_key/sender_key variants The scanner only matched name!(...) and bare identity/sender, missing trace! {..}/trace![..] call forms and identity_key/sender_key-style identifiers. Extend both, add regression tests for every delimiter and identifier form, and wire the tests into the log-redaction CI job. --- .github/workflows/ci.yml | 2 + .gitignore | 2 + scripts/check_log_redaction.py | 32 +++++++++------ scripts/check_log_redaction_test.py | 62 +++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 12 deletions(-) create mode 100644 scripts/check_log_redaction_test.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f8186fd..92cf44b2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,6 +19,8 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 + - name: Regression-test the log-redaction checker itself + run: python3 scripts/check_log_redaction_test.py - name: Check for Nostr key/identity leaks in tracing calls run: python3 scripts/check_log_redaction.py diff --git a/.gitignore b/.gitignore index fd5316fb..24eb37dd 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,5 @@ CLAUDE.md # Mutation testing output mutants.out/ mutants.out.old/ + +__pycache__/ diff --git a/scripts/check_log_redaction.py b/scripts/check_log_redaction.py index 08a8a575..0bf371df 100644 --- a/scripts/check_log_redaction.py +++ b/scripts/check_log_redaction.py @@ -20,7 +20,11 @@ REPO_ROOT = Path(__file__).resolve().parent.parent SRC_ROOT = REPO_ROOT / "src" -MACRO_RE = re.compile(r"\b(?:tracing::)?(trace|debug|info|warn|error)!\s*\(") +MACRO_RE = re.compile(r"\b(?:tracing::)?(?:trace|debug|info|warn|error)!\s*([({\[])") + +# Rust macros accept any of these three delimiter pairs; the matcher must +# track whichever one was actually opened. +DELIMITER_PAIRS = {"(": ")", "{": "}", "[": "]"} # Identifiers that name a Nostr key/identity in this codebase. Extend this # list, don't loosen it to a bare `key` — that also matches innocuous things @@ -28,8 +32,8 @@ SUSPICIOUS_RE = re.compile( r"\b(" r"\w*pubkey\w*" - r"|identity" - r"|sender" + r"|identity\w*" + r"|sender\w*" r"|master_key" r"|trade_key" r"|nsec\w*" @@ -43,19 +47,23 @@ ALLOW_COMMENT = "pubkey-log-allow:" -def find_call_span(text: str, open_paren: int) -> tuple[int, str]: - """Return (index just past the `)` matching `text[open_paren] == '('`, - the call's source with string-literal *contents* blanked out). +def find_call_span(text: str, open_delim: int) -> tuple[int, str]: + """Return (index just past the delimiter matching + `text[open_delim]`, the call's source with string-literal *contents* + blanked out). Handles all three Rust macro delimiter pairs: `()`, + `{}`, `[]`. - Blanking string contents (not just skipping them for paren-matching) + Blanking string contents (not just skipping them for delimiter-matching) matters: a format string's own English prose can contain a key-shaped word ("...pubkey in order...") that isn't an interpolated argument at all — only the blanked version should be searched for suspicious identifiers, or every message that merely *mentions* a pubkey false- positives. """ + open_ch = text[open_delim] + close_ch = DELIMITER_PAIRS[open_ch] depth = 0 - i = open_paren + i = open_delim n = len(text) out = [] while i < n: @@ -69,9 +77,9 @@ def find_call_span(text: str, open_paren: int) -> tuple[int, str]: out.append('"' * (i - start)) continue out.append(c) - if c == "(": + if c == open_ch: depth += 1 - elif c == ")": + elif c == close_ch: depth -= 1 if depth == 0: return i + 1, "".join(out) @@ -89,8 +97,8 @@ def check_file(path: Path) -> list[tuple[int, str]]: text = path.read_text(encoding="utf-8") violations = [] for m in MACRO_RE.finditer(text): - open_paren = text.index("(", m.end() - 1) - _end, code_only = find_call_span(text, open_paren) + open_delim = m.end() - 1 + _end, code_only = find_call_span(text, open_delim) found = SUSPICIOUS_RE.search(code_only) if not found: continue diff --git a/scripts/check_log_redaction_test.py b/scripts/check_log_redaction_test.py new file mode 100644 index 00000000..c10ce591 --- /dev/null +++ b/scripts/check_log_redaction_test.py @@ -0,0 +1,62 @@ +#!/usr/bin/env python3 +"""Regression coverage for check_log_redaction.py's macro/identifier +matching (delimiter forms and suspicious-identifier variants). Run via +`python3 scripts/check_log_redaction_test.py` — wired into the +`log-redaction` CI job alongside the checker itself. +""" + +import tempfile +import unittest +from pathlib import Path + +from check_log_redaction import check_file + + +class CheckLogRedactionTest(unittest.TestCase): + def _violations(self, rust_src: str) -> list[tuple[int, str]]: + with tempfile.NamedTemporaryFile( + "w", suffix=".rs", delete=False, encoding="utf-8" + ) as f: + f.write(rust_src) + path = Path(f.name) + try: + return check_file(path) + finally: + path.unlink() + + def test_paren_call_flags_pubkey(self): + violations = self._violations('fn x() { tracing::info!("{}", pubkey); }') + self.assertEqual(len(violations), 1) + + def test_brace_call_flags_pubkey(self): + violations = self._violations("fn x() { trace! {pubkey} }") + self.assertEqual(len(violations), 1) + + def test_bracket_call_flags_pubkey(self): + violations = self._violations("fn x() { trace![pubkey] }") + self.assertEqual(len(violations), 1) + + def test_identity_key_variant_is_flagged(self): + violations = self._violations('fn x() { info!("{}", identity_key); }') + self.assertEqual(len(violations), 1) + + def test_sender_key_variant_is_flagged(self): + violations = self._violations('fn x() { info!("{}", sender_key); }') + self.assertEqual(len(violations), 1) + + def test_prose_mention_is_not_flagged(self): + violations = self._violations('fn x() { info!("logging pubkey redaction"); }') + self.assertEqual(violations, []) + + def test_allow_comment_exempts_call(self): + violations = self._violations( + "fn x() {\n" + "// pubkey-log-allow: already truncated\n" + 'info!("{}", pubkey);\n' + "}" + ) + self.assertEqual(violations, []) + + +if __name__ == "__main__": + unittest.main() From 54c276b4ec53bc168a5f87aec7f8954523fe61d9 Mon Sep 17 00:00:00 2001 From: Tori Date: Sat, 1 Aug 2026 09:40:44 -0500 Subject: [PATCH 5/7] test(check_log_redaction): assert the reported line and identifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The positive cases only checked the violation count, so the scanner could report the wrong key name or the wrong source line and every one of them would still pass. Assert the exact `(line, identifier)` tuples instead. The existing cases are all one-liners, so their line `1` would hold even if the number were never computed — add a case with the call further down the file so that arithmetic is actually exercised. --- scripts/check_log_redaction_test.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/scripts/check_log_redaction_test.py b/scripts/check_log_redaction_test.py index c10ce591..ec7b7957 100644 --- a/scripts/check_log_redaction_test.py +++ b/scripts/check_log_redaction_test.py @@ -26,23 +26,32 @@ def _violations(self, rust_src: str) -> list[tuple[int, str]]: def test_paren_call_flags_pubkey(self): violations = self._violations('fn x() { tracing::info!("{}", pubkey); }') - self.assertEqual(len(violations), 1) + self.assertEqual(violations, [(1, "pubkey")]) def test_brace_call_flags_pubkey(self): violations = self._violations("fn x() { trace! {pubkey} }") - self.assertEqual(len(violations), 1) + self.assertEqual(violations, [(1, "pubkey")]) def test_bracket_call_flags_pubkey(self): violations = self._violations("fn x() { trace![pubkey] }") - self.assertEqual(len(violations), 1) + self.assertEqual(violations, [(1, "pubkey")]) def test_identity_key_variant_is_flagged(self): violations = self._violations('fn x() { info!("{}", identity_key); }') - self.assertEqual(len(violations), 1) + self.assertEqual(violations, [(1, "identity_key")]) def test_sender_key_variant_is_flagged(self): violations = self._violations('fn x() { info!("{}", sender_key); }') - self.assertEqual(len(violations), 1) + self.assertEqual(violations, [(1, "sender_key")]) + + def test_reported_line_is_the_macro_line_not_the_first(self): + # Every other positive case is a one-liner, so its `1` would hold + # even if the line number were never computed. Put the call further + # down so the assertion actually exercises that arithmetic. + violations = self._violations( + "fn x() {\n let a = 1;\n info!(\"{}\", pubkey);\n}" + ) + self.assertEqual(violations, [(3, "pubkey")]) def test_prose_mention_is_not_flagged(self): violations = self._violations('fn x() { info!("logging pubkey redaction"); }') From 0c83de865a210988f27e446c78d9ccfeae812eba Mon Sep 17 00:00:00 2001 From: Tori Date: Mon, 17 Aug 2026 11:55:04 -0500 Subject: [PATCH 6/7] fix(check_log_redaction): catch Rust 2021 inline format captures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate blanked string-literal contents before scanning for suspicious identifiers, which hid captured args living inside the format string itself (`info!("pubkey {pubkey}")`) — a style used throughout the tree, so the gate green-lit exactly what it exists to stop. Capture names are now pulled out before blanking and checked alongside the call's other arguments. Also, per review on #842: - scrub the solver pubkey leaking out of admin_add_solver_action on the success path (it was scrubbed going in via the RPC entrypoint, logged going out here) - stop dumping full Nostr events with {event:#?} (pubkey/tags/content/sig in clear) across six call sites; log a scoped identifier instead - move the is_assigned_solver log after the query it was asserting unconditionally before - give send_dm's log a request_id for correlation now that logging moved past the point where message is already parsed - cite "AGENTS.md, Security & Configuration Tips" instead of a line number that drifts under edits --- scripts/check_log_redaction.py | 39 +++++++++++++++++++---------- scripts/check_log_redaction_test.py | 23 +++++++++++++++++ src/app.rs | 4 +-- src/app/admin_add_solver.rs | 3 ++- src/app/admin_cancel.rs | 5 ++-- src/app/admin_settle.rs | 5 ++-- src/app/admin_take_dispute.rs | 8 +++--- src/app/bond/payout.rs | 6 ++--- src/app/cancel.rs | 2 +- src/app/last_trade_index.rs | 2 +- src/db.rs | 9 ++++--- src/rpc/service.rs | 2 +- src/util.rs | 28 ++++++++++++++------- 13 files changed, 95 insertions(+), 41 deletions(-) diff --git a/scripts/check_log_redaction.py b/scripts/check_log_redaction.py index 0bf371df..ba08bff9 100644 --- a/scripts/check_log_redaction.py +++ b/scripts/check_log_redaction.py @@ -1,6 +1,7 @@ #!/usr/bin/env python3 -"""CI gate for AGENTS.md:48 ("Scrub logs that might leak invoices or Nostr -keys"). Flags any `tracing::{trace,debug,info,warn,error}!(...)` call whose +"""CI gate for AGENTS.md's Security & Configuration Tips ("Scrub logs that +might leak invoices or Nostr keys"). Flags any +`tracing::{trace,debug,info,warn,error}!(...)` call whose argument list interpolates an identifier that looks like a Nostr key/identity, so a new log-scrubbing regression (issue #836's pattern) fails CI instead of shipping quietly. @@ -22,6 +23,12 @@ MACRO_RE = re.compile(r"\b(?:tracing::)?(?:trace|debug|info|warn|error)!\s*([({\[])") +# Rust 2021 captured identifiers in a format string, e.g. `"pubkey {pubkey}"`. +# Only bare identifiers are captures (`{}`/`{:?}`/`{0}` are positional/empty +# and can't name a variable), and `{{`/`}}` are the escaped-brace literals — +# excluded so `"{{not_a_capture}}"` isn't misread as one. +FORMAT_CAPTURE_RE = re.compile(r"(? tuple[int, str]: - """Return (index just past the delimiter matching - `text[open_delim]`, the call's source with string-literal *contents* - blanked out). Handles all three Rust macro delimiter pairs: `()`, - `{}`, `[]`. +def find_call_span(text: str, open_delim: int) -> tuple[int, str, str]: + """Return (index just past the delimiter matching `text[open_delim]`, + the call's source with string-literal *contents* blanked out, and the + format captures pulled from those strings before blanking). Handles all + three Rust macro delimiter pairs: `()`, `{}`, `[]`. Blanking string contents (not just skipping them for delimiter-matching) matters: a format string's own English prose can contain a key-shaped word ("...pubkey in order...") that isn't an interpolated argument at all — only the blanked version should be searched for suspicious identifiers, or every message that merely *mentions* a pubkey false- - positives. + positives. But a Rust 2021 captured identifier (`"pubkey {pubkey}"`) IS + an interpolated argument living inside that same string, so its capture + names are extracted first and returned alongside, not lost to blanking. """ open_ch = text[open_delim] close_ch = DELIMITER_PAIRS[open_ch] @@ -66,6 +75,7 @@ def find_call_span(text: str, open_delim: int) -> tuple[int, str]: i = open_delim n = len(text) out = [] + captures = [] while i < n: c = text[i] if c == '"': @@ -74,6 +84,8 @@ def find_call_span(text: str, open_delim: int) -> tuple[int, str]: while i < n and text[i] != '"': i += 2 if text[i] == "\\" else 1 i += 1 + literal = text[start:i] + captures.extend(m.group(1) for m in FORMAT_CAPTURE_RE.finditer(literal)) out.append('"' * (i - start)) continue out.append(c) @@ -82,9 +94,9 @@ def find_call_span(text: str, open_delim: int) -> tuple[int, str]: elif c == close_ch: depth -= 1 if depth == 0: - return i + 1, "".join(out) + return i + 1, "".join(out), " ".join(captures) i += 1 - return n, "".join(out) # unbalanced — best effort + return n, "".join(out), " ".join(captures) # unbalanced — best effort def line_before(text: str, index: int) -> str: @@ -98,8 +110,8 @@ def check_file(path: Path) -> list[tuple[int, str]]: violations = [] for m in MACRO_RE.finditer(text): open_delim = m.end() - 1 - _end, code_only = find_call_span(text, open_delim) - found = SUSPICIOUS_RE.search(code_only) + _end, code_only, captures = find_call_span(text, open_delim) + found = SUSPICIOUS_RE.search(code_only) or SUSPICIOUS_RE.search(captures) if not found: continue if ALLOW_COMMENT in line_before(text, m.start()): @@ -116,7 +128,8 @@ def main() -> int: rel = path.relative_to(REPO_ROOT) print( f"{rel}:{line_no}: tracing call interpolates `{ident}` — " - f"looks like a Nostr key/identity (AGENTS.md:48). Drop it from " + f"looks like a Nostr key/identity (AGENTS.md, Security & " + f"Configuration Tips). Drop it from " f"the log line, or mark a deliberate exception with a " f"`// {ALLOW_COMMENT} ` comment on the line above." ) diff --git a/scripts/check_log_redaction_test.py b/scripts/check_log_redaction_test.py index ec7b7957..4a06315f 100644 --- a/scripts/check_log_redaction_test.py +++ b/scripts/check_log_redaction_test.py @@ -57,6 +57,29 @@ def test_prose_mention_is_not_flagged(self): violations = self._violations('fn x() { info!("logging pubkey redaction"); }') self.assertEqual(violations, []) + def test_inline_capture_flags_pubkey(self): + # Rust 2021 captured identifiers live inside the string literal + # itself — the checker must pull them out before blanking, not lose + # them along with the surrounding prose. + violations = self._violations( + 'fn x(pubkey: &str) { tracing::info!("User with pubkey {pubkey} did X"); }' + ) + self.assertEqual(violations, [(1, "pubkey")]) + + def test_inline_capture_with_format_spec_flags_pubkey(self): + violations = self._violations( + 'fn x(pubkey: &str) { info!("pubkey={pubkey:?}"); }' + ) + self.assertEqual(violations, [(1, "pubkey")]) + + def test_positional_placeholder_is_not_a_capture(self): + violations = self._violations('fn x() { info!("{} {:?}", pubkey, 1); }') + self.assertEqual(violations, [(1, "pubkey")]) + + def test_escaped_braces_are_not_a_capture(self): + violations = self._violations('fn x() { info!("{{pubkey}}"); }') + self.assertEqual(violations, []) + def test_allow_comment_exempts_call(self): violations = self._violations( "fn x() {\n" diff --git a/src/app.rs b/src/app.rs index bd097ea4..81642c3e 100644 --- a/src/app.rs +++ b/src/app.rs @@ -341,7 +341,7 @@ async fn accept_event( // we decrypt. New orders/takes legitimately arrive // here — so does spam, hence the PoW toll. if !gate.is_known(&event.pubkey.to_string()) && !event.check_pow(pow_first_contact) { - // No key in the log line — sender pubkey (AGENTS.md:48). + // No key in the log line — sender pubkey (AGENTS.md, Security & Configuration Tips). tracing::info!( "Dropping first-contact kind-14 event below pow_first_contact ({} bits)", pow_first_contact @@ -386,7 +386,7 @@ async fn accept_event( // signature — unwrap_message already verified it, so if identity // and sender differ here without a signature we bail out. if unwrapped.identity != unwrapped.sender && unwrapped.signature.is_none() { - // No keys in the log line — identity/trade key (AGENTS.md:48). + // No keys in the log line — identity/trade key (AGENTS.md, Security & Configuration Tips). tracing::warn!("Missing inner signature: identity differs from trade key"); return None; } diff --git a/src/app/admin_add_solver.rs b/src/app/admin_add_solver.rs index 25fba544..8bc7f9c0 100644 --- a/src/app/admin_add_solver.rs +++ b/src/app/admin_add_solver.rs @@ -70,7 +70,8 @@ pub async fn admin_add_solver_action( let user = User::new(public_key.to_string(), 0, 1, 0, category, trade_index); match add_new_user(pool, user).await { - Ok(r) => info!("Solver added: {} with category {}", r, category), + // No key in the log line — solver pubkey (AGENTS.md, Security & Configuration Tips). + Ok(_) => info!("Solver added with category {}", category), Err(ee) => { error!("Error creating solver: {:#?}", ee); return Err(MostroInternalErr(ServiceError::DbAccessError( diff --git a/src/app/admin_cancel.rs b/src/app/admin_cancel.rs index 7cf8e8be..00062242 100644 --- a/src/app/admin_cancel.rs +++ b/src/app/admin_cancel.rs @@ -151,8 +151,9 @@ pub async fn admin_cancel_action( let event = new_dispute_event(my_keys, "", dispute_id.to_string(), tags) .map_err(|e| MostroInternalErr(ServiceError::NostrError(e.to_string())))?; - // Publish dispute event with update - info!("Dispute event to be published: {event:#?}"); + // Publish dispute event with update. No full event dump — it carries + // the pubkey, tags and content in clear (AGENTS.md, Security & Configuration Tips). + info!("Dispute event to be published for dispute {dispute_id}"); let client = ctx.nostr_client(); if let Err(e) = client.send_event(&event).await { diff --git a/src/app/admin_settle.rs b/src/app/admin_settle.rs index 8a519c35..ec456dfc 100644 --- a/src/app/admin_settle.rs +++ b/src/app/admin_settle.rs @@ -150,8 +150,9 @@ pub async fn admin_settle_action( let event = new_dispute_event(my_keys, "", dispute_id.to_string(), tags) .map_err(|e| MostroInternalErr(ServiceError::NostrError(e.to_string())))?; - // Print event dispute with update - tracing::info!("Dispute event to be published: {event:#?}"); + // No full event dump — it carries the pubkey, tags and content in + // clear (AGENTS.md, Security & Configuration Tips). + tracing::info!("Dispute event to be published for dispute {dispute_id}"); let client = ctx.nostr_client(); if let Err(e) = client.send_event(&event).await { diff --git a/src/app/admin_take_dispute.rs b/src/app/admin_take_dispute.rs index f37f72c6..300cf62b 100644 --- a/src/app/admin_take_dispute.rs +++ b/src/app/admin_take_dispute.rs @@ -93,7 +93,7 @@ pub async fn pubkey_event_can_solve( let sender_pubkey = ev_pubkey.to_string(); // Is mostro admin taking dispute? No keys in the log line — admin/event - // pubkeys (AGENTS.md:48). + // pubkeys (AGENTS.md, Security & Configuration Tips). info!("Checking whether the dispute event was sent by the mostro admin"); if sender_pubkey == my_keys.public_key().to_string() && matches!(status, DisputeStatus::InProgress | DisputeStatus::Initiated) @@ -189,7 +189,7 @@ pub async fn admin_take_dispute_action( dispute.solver_pubkey = Some(event.identity.to_string()); dispute.taken_at = Timestamp::now().as_secs() as i64; - // No key in the log line — solver identity (AGENTS.md:48). + // No key in the log line — solver identity (AGENTS.md, Security & Configuration Tips). info!("Dispute {} taken by a solver", dispute.id); // Save it to DB @@ -270,7 +270,9 @@ pub async fn admin_take_dispute_action( // nip33 kind with dispute id as identifier (kind 38386 for disputes) let event = new_dispute_event(mostro_keys, "", dispute.id.to_string(), tags) .map_err(|e| MostroInternalErr(ServiceError::NostrError(e.to_string())))?; - info!("Dispute event to be published: {event:#?}"); + // No full event dump — it carries the pubkey, tags and content in clear + // (AGENTS.md, Security & Configuration Tips). + info!("Dispute event to be published for dispute {}", dispute.id); let client = ctx.nostr_client(); client diff --git a/src/app/bond/payout.rs b/src/app/bond/payout.rs index 7f41fa6e..2f487a93 100644 --- a/src/app/bond/payout.rs +++ b/src/app/bond/payout.rs @@ -454,7 +454,7 @@ async fn request_payout_invoice( return Ok(()); } - // No key in the log fields — recipient pubkey (AGENTS.md:48). + // No key in the log fields — recipient pubkey (AGENTS.md, Security & Configuration Tips). info!( bond_id = %bond.id, order_id = %bond.order_id, @@ -1349,7 +1349,7 @@ pub async fn add_bond_invoice_action( match apply_payout_invoice(pool, &bond, &payment_request, now, claim_window_seconds).await? { InvoiceApplyOutcome::Persisted => { - // No key in the log fields — sender pubkey (AGENTS.md:48). + // No key in the log fields — sender pubkey (AGENTS.md, Security & Configuration Tips). info!( bond_id = %bond.id, order_id = %bond.order_id, @@ -1357,7 +1357,7 @@ pub async fn add_bond_invoice_action( ); } InvoiceApplyOutcome::Resurrected => { - // No key in the log fields — sender pubkey (AGENTS.md:48). + // No key in the log fields — sender pubkey (AGENTS.md, Security & Configuration Tips). info!( bond_id = %bond.id, order_id = %bond.order_id, diff --git a/src/app/cancel.rs b/src/app/cancel.rs index 66ed15e8..ae957558 100644 --- a/src/app/cancel.rs +++ b/src/app/cancel.rs @@ -307,7 +307,7 @@ async fn cancel_order_by_taker_inner( .await .map_err(|e| MostroInternalErr(ServiceError::NostrError(e.to_string())))?; - // No key in the log line — taker pubkey (AGENTS.md:48). + // No key in the log line — taker pubkey (AGENTS.md, Security & Configuration Tips). info!("Canceled order Id {} republishing order", order.id); // Notify the creator about the republished order after the taker-side cancellation flow completes diff --git a/src/app/last_trade_index.rs b/src/app/last_trade_index.rs index e559b38a..a3afab72 100644 --- a/src/app/last_trade_index.rs +++ b/src/app/last_trade_index.rs @@ -83,7 +83,7 @@ pub async fn last_trade_index( .map_err(|_| MostroError::MostroInternalErr(ServiceError::MessageSerializationError))?; // Print the last trade index message. No key in the log line — user - // pubkey (AGENTS.md:48). + // pubkey (AGENTS.md, Security & Configuration Tips). tracing::info!("User requested last trade index"); tracing::info!("Last trade index: {}", user.last_trade_index); diff --git a/src/db.rs b/src/db.rs index a468c9a5..c094fb1a 100644 --- a/src/db.rs +++ b/src/db.rs @@ -1543,9 +1543,7 @@ pub async fn is_assigned_solver( solver_pubkey: &str, order_id: Uuid, ) -> Result { - // No key in the log line — solver pubkey (AGENTS.md:48). - tracing::info!("Solver assigned to order {}", order_id); - let result = sqlx::query( + let result: bool = sqlx::query( "SELECT EXISTS(SELECT 1 FROM disputes WHERE solver_pubkey = ? AND order_id = ?)", ) .bind(solver_pubkey) @@ -1555,6 +1553,11 @@ pub async fn is_assigned_solver( .await .map_err(|e| MostroInternalErr(ServiceError::DbAccessError(e.to_string())))?; + // No key in the log line — solver pubkey (AGENTS.md, Security & Configuration Tips). + if result { + tracing::info!("Solver assigned to order {}", order_id); + } + Ok(result) } diff --git a/src/rpc/service.rs b/src/rpc/service.rs index fff19b59..052db2c3 100644 --- a/src/rpc/service.rs +++ b/src/rpc/service.rs @@ -318,7 +318,7 @@ impl AdminService for AdminServiceImpl { request: Request, ) -> Result, Status> { let req = request.into_inner(); - // No key in the log line — solver pubkey (AGENTS.md:48). + // No key in the log line — solver pubkey (AGENTS.md, Security & Configuration Tips). info!("Received add solver request"); match self diff --git a/src/util.rs b/src/util.rs index 7bb34190..a831ac78 100644 --- a/src/util.rs +++ b/src/util.rs @@ -469,7 +469,8 @@ async fn finalize_order_publication( return Err(MostroInternalErr(ServiceError::InvalidPubkey)); }; - info!("Order event to be published: {event:#?}"); + // No full event dump — it carries the pubkey, tags and content in clear + // (AGENTS.md, Security & Configuration Tips); the id below is enough. let event_id = event.id.to_string(); info!("Publishing Event Id: {event_id} for Order Id: {order_id}"); // We update the order with the new event_id (and Pending status) @@ -670,14 +671,19 @@ pub async fn send_dm( payload: &str, expiration: Option, ) -> Result<(), MostroError> { - // No keys in the log line — sender/receiver pubkeys (AGENTS.md:48). This - // is the highest-frequency send path in the daemon (every outbound - // protocol message), so it's also the highest-blast-radius instance of - // this pattern. - info!("Sending DM"); let mut message = Message::from_json(payload) .map_err(|_| MostroInternalErr(ServiceError::MessageSerializationError))?; + // No keys in the log line — sender/receiver pubkeys (AGENTS.md, Security + // & Configuration Tips). This is the highest-frequency send path in the + // daemon (every outbound protocol message), so it's also the highest- + // blast-radius instance of this pattern. request_id gives a correlation + // handle at no extra parsing cost, since `message` is already parsed. + info!( + "Sending DM, request_id: {:?}", + message.get_inner_message_kind().request_id + ); + // Non-panicking accessor: send_dm sits on every reply path and is // exercised by unit tests that don't initialize the global config. // DEPRECATED(v0.19.0, #786): both calls below go away with the @@ -718,7 +724,7 @@ pub async fn send_dm( .await?; // No key in the log line — payload can carry a Nostr pubkey (e.g. - // Payload::Peer sent by admin_take_dispute_action) (AGENTS.md:48). + // Payload::Peer sent by admin_take_dispute_action) (AGENTS.md, Security & Configuration Tips). info!("Sending message, Event ID: {}", event.id); if let Ok(client) = get_nostr_client() { @@ -838,7 +844,9 @@ pub async fn update_user_rating_event( ) -> Result<(), MostroError> { let event = new_rating_event(keys, "", user.to_string(), tags) .map_err(|e| MostroInternalErr(ServiceError::NostrError(e.to_string())))?; - info!("Sending replaceable event: {event:#?}"); + // No full event dump — it carries the rated user's pubkey, tags and + // content in clear (AGENTS.md, Security & Configuration Tips). + info!("Sending replaceable rating event"); MESSAGE_QUEUES.queue_order_rate.write().await.push(event); Ok(()) } @@ -1228,7 +1236,9 @@ async fn update_order_event_stamped( new_order_event_with_created_at(keys, "", order.id.to_string(), tags, event_created_at) .map_err(|e| MostroInternalErr(ServiceError::NostrError(e.to_string())))?; - info!("Sending replaceable event: {event:#?}"); + // No full event dump — it carries the pubkey, tags and content in + // clear (AGENTS.md, Security & Configuration Tips). + info!("Sending replaceable event for order {}", order.id); // We update the order with the new event_id order_updated.event_id = event.id.to_string(); From c87d664f5f9bc6417a2e2c4b8dffcae5b86e7ebc Mon Sep 17 00:00:00 2001 From: Tori Date: Tue, 18 Aug 2026 12:10:02 -0500 Subject: [PATCH 7/7] fix(check_log_redaction): restrict allow-comment exemption to real // comments --- scripts/check_log_redaction.py | 3 ++- scripts/check_log_redaction_test.py | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/scripts/check_log_redaction.py b/scripts/check_log_redaction.py index ba08bff9..32034ed7 100644 --- a/scripts/check_log_redaction.py +++ b/scripts/check_log_redaction.py @@ -114,7 +114,8 @@ def check_file(path: Path) -> list[tuple[int, str]]: found = SUSPICIOUS_RE.search(code_only) or SUSPICIOUS_RE.search(captures) if not found: continue - if ALLOW_COMMENT in line_before(text, m.start()): + previous_line = line_before(text, m.start()).lstrip() + if previous_line.startswith(f"// {ALLOW_COMMENT}"): continue line_no = text.count("\n", 0, m.start()) + 1 violations.append((line_no, found.group(0))) diff --git a/scripts/check_log_redaction_test.py b/scripts/check_log_redaction_test.py index 4a06315f..a3776d25 100644 --- a/scripts/check_log_redaction_test.py +++ b/scripts/check_log_redaction_test.py @@ -89,6 +89,17 @@ def test_allow_comment_exempts_call(self): ) self.assertEqual(violations, []) + def test_allow_marker_inside_string_literal_does_not_exempt(self): + # The marker must sit in an actual `//` comment on the line above — + # not just appear anywhere on that line, e.g. inside another string. + violations = self._violations( + "fn x() {\n" + 'let note = "pubkey-log-allow: not a real exemption";\n' + 'info!("{}", pubkey);\n' + "}" + ) + self.assertEqual(violations, [(3, "pubkey")]) + if __name__ == "__main__": unittest.main()