Conversation
New filter that loads pubkeys from a NIP-05 nostr.json file (local path or HTTP/HTTPS URL) into an in-memory HashSet and rejects notes from pubkeys not in the set. The whitelist is refreshed in a background thread on a configurable interval (default 60s), with failed reloads logging an error and preserving the previous set. Uses ureq for lightweight sync HTTP fetching.
📝 WalkthroughWalkthroughAdds a new NIP-05 whitelist note filter that loads allowed pubkeys from a configured JSON source (HTTP(S) URL or local file), caches them, spawns a periodic reload thread, and filters incoming notes by pubkey membership with a configurable rejection message. Changes
Sequence Diagram(s)sequenceDiagram
participant Noteguard as Noteguard System
participant Filter as Nip05Whitelist Filter
participant Source as JSON Source (URL or File)
participant Thread as Reload Thread
participant Notes as Incoming Notes
Noteguard->>Filter: first initialization (on first note)
activate Filter
Filter->>Source: fetch NIP-05 JSON (names map)
Source-->>Filter: JSON payload
Filter->>Filter: parse & populate HashSet (RwLock)
Filter-->>Noteguard: init complete (log)
deactivate Filter
loop periodic reload (reload_interval_secs)
Thread->>Source: refetch JSON
Source-->>Thread: JSON payload
Thread->>Filter: atomically replace HashSet (RwLock)
Thread-->>Thread: log success/error
end
loop incoming notes
Notes->>Filter: deliver note
Filter->>Filter: check msg.event.pubkey in HashSet
alt pubkey present
Filter-->>Notes: accept
else pubkey absent
Filter-->>Notes: reject (custom/default message)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main.rs (1)
1-3: Add the new builtin to the registration test.The filter is registered correctly, but
test_register_builtin_filtersstill does not assert"nip05_whitelist", so this wire-up can regress silently.✅ Proposed test assertion
assert!(noteguard.registered_filters.contains_key("kinds")); + assert!(noteguard + .registered_filters + .contains_key("nip05_whitelist"));Also applies to: 56-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 1 - 3, The test test_register_builtin_filters is missing an assertion for the newly-registered filter "nip05_whitelist", so registration can regress silently; update test_register_builtin_filters to include an assertion that the registered builtin filter names (or the list it inspects) contains "nip05_whitelist" alongside the existing entries (e.g., assert presence in whatever collection is returned by the registration function), referencing the test function name test_register_builtin_filters and the filter identifier "nip05_whitelist" to ensure the new builtin is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@noteguard.toml`:
- Around line 26-29: The nip05_whitelist filter block (filters.nip05_whitelist)
is inert unless the top-level pipeline includes "nip05_whitelist"; add a
commented example entry to the pipeline configuration showing how to enable it
(e.g., add "nip05_whitelist" to the pipeline array or sequence), so users know
they must include the filter name in the pipeline to activate
filters.nip05_whitelist.
In `@src/filters/nip05_whitelist.rs`:
- Around line 88-89: The reload interval must be clamped to avoid a zero-second
tight loop: when building the Duration for spawn_reload_thread, check
self.reload_interval_secs (and its unwrap_or default) and ensure it is at least
1 (or another sane minimum) before calling Duration::from_secs; update the code
around the Duration::from_secs(self.reload_interval_secs.unwrap_or(60)) so it
uses something like max(1, value) or returns an early error/uses a safe default,
and then call spawn_reload_thread(self.source.clone(), interval, pubkeys) with
the clamped interval to prevent a busy loop.
- Around line 29-38: The current read logic for `body` can block or OOM on
slow/large HTTP responses; change the HTTP path to use a ureq::Agent with
explicit connect/read timeouts (e.g.,
Agent::new().timeout_read(...).timeout_connect(...)) and fetch via
agent.get(source).call()?.into_reader(), then read into a buffer but limit bytes
using std::io::Read::take with a MAX_BYTES constant and return an error if more
data exists; similarly replace std::fs::read_to_string(source) with opening a
File and reading via file.take(MAX_BYTES).read_to_end(...) (or read_to_string on
the taken reader) so local files are also capped, and keep the same error
mapping messages; reference the existing `is_url(source)` check, the `body`
variable, and introduce a MAX_BYTES constant near this code.
---
Nitpick comments:
In `@src/main.rs`:
- Around line 1-3: The test test_register_builtin_filters is missing an
assertion for the newly-registered filter "nip05_whitelist", so registration can
regress silently; update test_register_builtin_filters to include an assertion
that the registered builtin filter names (or the list it inspects) contains
"nip05_whitelist" alongside the existing entries (e.g., assert presence in
whatever collection is returned by the registration function), referencing the
test function name test_register_builtin_filters and the filter identifier
"nip05_whitelist" to ensure the new builtin is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 097037a6-6133-4989-b48e-0bc7276dc6a5
📒 Files selected for processing (5)
Cargo.tomlnoteguard.tomlsrc/filters/mod.rssrc/filters/nip05_whitelist.rssrc/main.rs
Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/filters/nip05_whitelist.rs`:
- Around line 66-76: The code is logging raw `source` (which may contain
secrets); create and use a sanitizer (e.g., `redact_source` or
`sanitize_source`) that strips URL userinfo (username:password@) and replaces
sensitive filesystem paths with a safe placeholder or basename, then pass the
sanitized value into all error/log messages and calls that only need a display
string; update the `map_err` closures around `agent.get(source).call()` and
`std::fs::File::open(source)` and change the second argument to
`read_capped(reader, sanitized, "...")` / `read_capped(file, sanitized, "...")`
(keep the original `source` for actual I/O but use `sanitized` for any
logging/display).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 38fe8f1a-4c2d-47f4-b447-727164d11136
📒 Files selected for processing (2)
noteguard.tomlsrc/filters/nip05_whitelist.rs
✅ Files skipped from review due to trivial changes (1)
- noteguard.toml
Comment on lines
+66
to
+76
| let reader = agent | ||
| .get(source) | ||
| .call() | ||
| .map_err(|e| format!("HTTP request failed for {}: {}", source, e))? | ||
| .into_reader(); | ||
|
|
||
| read_capped(reader, source, "response body from")? | ||
| } else { | ||
| let file = std::fs::File::open(source) | ||
| .map_err(|e| format!("failed to read file {}: {}", source, e))?; | ||
| read_capped(file, source, "file")? |
There was a problem hiding this comment.
Redact source before logging it.
source can be an authenticated URL or an internal filesystem path, and the current success/error logs emit it verbatim. That leaks secrets and deployment details into logs; log a sanitized display value instead.
🛡️ Suggested direction
+use std::path::Path;
+
+fn source_for_log(source: &str) -> String {
+ if is_url(source) {
+ let without_query = source.split('?').next().unwrap_or(source);
+ match without_query.rsplit_once('@') {
+ Some((prefix, tail)) if prefix.contains("://") => {
+ let scheme = prefix.split("://").next().unwrap_or("https");
+ format!("{scheme}://{tail}")
+ }
+ _ => without_query.to_string(),
+ }
+ } else {
+ Path::new(source)
+ .file_name()
+ .and_then(|name| name.to_str())
+ .unwrap_or("<local file>")
+ .to_string()
+ }
+}
+
fn fetch_pubkeys(source: &str) -> Result<HashSet<String>, String> {
+ let log_source = source_for_log(source);
let body = if is_url(source) {
let agent = ureq::AgentBuilder::new()
.timeout_connect(HTTP_CONNECT_TIMEOUT)
.timeout_read(HTTP_READ_TIMEOUT)
.build();
let reader = agent
.get(source)
.call()
- .map_err(|e| format!("HTTP request failed for {}: {}", source, e))?
+ .map_err(|e| format!("HTTP request failed for {}: {}", log_source, e))?
.into_reader();
- read_capped(reader, source, "response body from")?
+ read_capped(reader, &log_source, "response body from")?
} else {
let file = std::fs::File::open(source)
- .map_err(|e| format!("failed to read file {}: {}", source, e))?;
- read_capped(file, source, "file")?
+ .map_err(|e| format!("failed to read file {}: {}", log_source, e))?;
+ read_capped(file, &log_source, "file")?
};
@@
info!(
"nip05_whitelist: reloaded {} pubkeys from {}",
- count, source
+ count, source_for_log(&source)
);
@@
info!(
"nip05_whitelist: loaded {} pubkeys from {}",
set.len(),
- self.source
+ source_for_log(&self.source)
);Also applies to: 95-101, 109-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/filters/nip05_whitelist.rs` around lines 66 - 76, The code is logging raw
`source` (which may contain secrets); create and use a sanitizer (e.g.,
`redact_source` or `sanitize_source`) that strips URL userinfo
(username:password@) and replaces sensitive filesystem paths with a safe
placeholder or basename, then pass the sanitized value into all error/log
messages and calls that only need a display string; update the `map_err`
closures around `agent.get(source).call()` and `std::fs::File::open(source)` and
change the second argument to `read_capped(reader, sanitized, "...")` /
`read_capped(file, sanitized, "...")` (keep the original `source` for actual I/O
but use `sanitized` for any logging/display).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New filter that loads pubkeys from a NIP-05 nostr.json file (local path or HTTP/HTTPS URL) into an in-memory HashSet and rejects notes from pubkeys not in the set. The whitelist is refreshed in a background thread on a configurable interval (default 60s), with failed reloads logging an error and preserving the previous set. Uses ureq for lightweight sync HTTP fetching.
Summary by CodeRabbit
New Features
Documentation
Chores