Redact UnifiedPush identifiers in logs

[juanfradb]

Summary

• replace raw UnifiedPush device_id log values with salted BLAKE3 correlators
• remove endpoint URL prefix from UnifiedPush debug logs
• document UnifiedPush log privacy behavior

Privacy validation

Source/sink/control: UnifiedPush device_id and endpoint URL values reached operator logs in src/push/unifiedpush.rs. Registration and unregistration now log only a salted 8-hex correlator using the same helper as pubkey logging, and send_to_token no longer logs any endpoint prefix.

Original issue no longer reproduces:

• register/unregister log statements no longer format raw device_id
• send_to_token debug log no longer formats any endpoint URL substring
• unit test verifies hashed UnifiedPush identifiers are stable for the same salt and not raw identifiers

Refs #14

Tests

• cargo test unifiedpush_log_identifier_is_stable_and_redacted -- --nocapture
• cargo test
• cargo fmt --check
• cargo clippy --all-targets -- -D warnings
• git diff --check
• rg log macro check in src/push/unifiedpush.rs

Summary by CodeRabbit

Release Notes

• Bug Fixes
  • Device identifiers in server logs are now redacted for improved privacy. Raw UnifiedPush device IDs and endpoint URL prefixes are no longer visible in logs.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Walkthrough

UnifiedPushService now generates a per-instance random salt and uses salted BLAKE3 hashing to redact device identifiers in logs. Registration, unregistration, and notification log messages are updated to use this redaction. Documentation explicitly states the privacy guarantee: raw device IDs and endpoint prefixes are excluded from server logs.

Changes

Device ID Redaction for Privacy

Layer / File(s)	Summary
Redaction mechanism and tests src/push/unifiedpush.rs	Adds rand::RngCore and log_pubkey imports. Introduces log_salt field initialized with random bytes on construction, implements a helper method to compute stable redacted identifiers, and adds unit tests verifying deterministic output, fixed short length, and absence of raw device IDs.
Applied redaction in registration and unregistration logs src/push/unifiedpush.rs	Updates registration and unregistration log statements to call the redaction helper and log masked identifiers instead of raw device_id values.
Notification logging and privacy documentation src/push/unifiedpush.rs, docs/unifiedpush.md	Simplifies notification debug logging to a fixed message. Adds documentation clarifying that server logs exclude raw device IDs and endpoint URL prefixes, using per-process salted BLAKE3 redaction consistent with other sensitive identifiers.

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A salty hash so fine and true,
Device IDs masked from view,
Logs now speak in BLAKE3 tongue,
Privacy sewn, a privacy song sung!
Hops to protect what must not show ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Redact UnifiedPush identifiers in logs' is concise, clear, and directly reflects the main change: replacing raw UnifiedPush device_id values with redacted/salted correlators in logging.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[AndreaDiazCorreia]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/push/unifiedpush.rs (1)

166-176: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Drop or sanitize the remote response body from this error log.

Line 176 still logs response.text() verbatim. Since UnifiedPush endpoints are attacker-controlled, a failing endpoint can echo the endpoint URL or other identifiers back into error_text, which puts the privacy guarantee from this PR back at risk.

Suggested fix

-        let error_text = response.text().await.unwrap_or_default();
-        error!("UnifiedPush error: {} - {}", status, error_text);
+        let _ = response.text().await;
+        error!("UnifiedPush error: {}", status);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/push/unifiedpush.rs` around lines 166 - 176, The error log currently
includes the full remote response body (response.text()/error_text) and may also
echo attacker-controlled identifiers like device_token; update the UnifiedPush
failure path (the code around self.client.post(device_token)... handling the
response variable) to stop logging the raw response body: either omit error_text
entirely from error!("UnifiedPush error: ..."), or replace it with a
sanitized/trimmed excerpt (e.g., limit to N characters, remove URLs/identifiers,
or replace with a fixed "<remote-response-suppressed>" token) and ensure
device_token or payload contents are not logged; keep the status code in the log
for diagnostics.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/push/unifiedpush.rs`:
- Around line 166-176: The error log currently includes the full remote response
body (response.text()/error_text) and may also echo attacker-controlled
identifiers like device_token; update the UnifiedPush failure path (the code
around self.client.post(device_token)... handling the response variable) to stop
logging the raw response body: either omit error_text entirely from
error!("UnifiedPush error: ..."), or replace it with a sanitized/trimmed excerpt
(e.g., limit to N characters, remove URLs/identifiers, or replace with a fixed
"<remote-response-suppressed>" token) and ensure device_token or payload
contents are not logged; keep the status code in the log for diagnostics.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a7220e06-92e7-4d2d-a43e-45406959fafd

📥 Commits

Reviewing files that changed from the base of the PR and between 893e7e1 and 7395664.

📒 Files selected for processing (2)

• docs/unifiedpush.md
• src/push/unifiedpush.rs

[AndreaDiazCorreia]

Nice work on the redaction. One change request before merging: please reuse the existing notify_log_salt from src/main.rs instead of generating a new salt inside UnifiedPushService::new(). You can pass it through the constructor (UnifiedPushService::new(config, client, log_salt)), the same way it is already injected into TokenStore, the Nostr listener and AppState. The original issue asked for the same salt and algorithm as log_pubkey, and sharing the process-wide salt keeps a single source of randomness and matches the pattern already used in the rest of the codebase.