Add configurable generic_alert APNs payload mode#307
Conversation
Deployments that want a visible iOS notification currently have to choose between silent (invisible without a Notification Service Extension) and nse_prototype_alert (hardcoded White Noise copy plus mutable-content, which also requires shipping an NSE). Add a third payload mode, generic_alert, that shows a plain alert with operator-configured, content-free copy: - apns.alert_title / apns.alert_body configure the alert text (content-free defaults); an empty value omits that key for title-only or body-only alerts, and configuring both empty is rejected at startup. - The payload carries no mutable-content flag, so no Notification Service Extension is needed to display it, and no prototype marker field. - A new optional apns.collapse_id key sets the apns-collapse-id header on every send (any payload mode), so rapid bursts coalesce into a single notification; values over the 64-byte APNs limit are rejected at startup instead of failing per-send with 400 BadCollapseId.
WalkthroughAdds the APNs ChangesAPNs configuration and validation
APNs delivery path
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
Ready to review this PR? Stage has broken it down into 4 individual chapters for you:
Chapters generated by Stage for commit c96d4a8 on Jul 12, 2026 8:23am UTC. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/push/apns.rs (1)
914-944: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a symmetric test for the title-empty / body-only case.
test_generic_alert_mode_omits_empty_body_from_alertcovers title-set/body-empty. The doc comment onApnsGenericAlert::title(Line 145-147) explicitly calls out the mirror case ("body-only alert" when title is empty), but there's no test asserting that shape. As per coding guidelines,**/*.rschanges should "Include tests for new functionality."✅ Suggested additional test
#[test] fn test_generic_alert_mode_omits_empty_title_from_alert() { let mut config = test_config(); config.payload_mode = ApnsPayloadMode::GenericAlert; config.alert_title = String::new(); config.alert_body = "You have a new notification".to_string(); let client = ApnsClient::mock(config, false); let request_parts = ApnsRequestParts::for_config(&client.config); let request = client .build_request( "https://api.push.apple.com/3/device/aabbccdd11223344", "test-token", &request_parts, ) .build() .unwrap(); assert_eq!( request_json(&request), serde_json::json!({ "aps": { "alert": { "body": "You have a new notification" }, "sound": "default" } }) ); }🤖 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/apns.rs` around lines 914 - 944, Add a symmetric test alongside test_generic_alert_mode_omits_empty_body_from_alert for GenericAlert with an empty alert_title and non-empty alert_body. Build the request through ApnsClient::mock and ApnsRequestParts::for_config, then assert the JSON alert contains only the body plus the existing sound field.Source: Coding guidelines
🤖 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.
Inline comments:
In `@src/config.rs`:
- Around line 1300-1307: Extend the startup validation for self.collapse_id
alongside the existing length check to reject values that cannot be used as HTTP
header values, including CR/LF and other invalid bytes. Return a
config::ConfigError::Message with a clear validation error, preserving the
existing maximum-byte-length validation and fail-fast behavior.
In `@src/push/apns.rs`:
- Around line 162-180: Validate non-empty config.collapse_id during
configuration loading and reject values that cannot be represented as an HTTP
header, including control characters such as newlines and tabs. Reuse the
existing HeaderValue validation approach, then keep ApnsRequestParts::for_config
limited to cloning already-validated values before request.header is called.
---
Nitpick comments:
In `@src/push/apns.rs`:
- Around line 914-944: Add a symmetric test alongside
test_generic_alert_mode_omits_empty_body_from_alert for GenericAlert with an
empty alert_title and non-empty alert_body. Build the request through
ApnsClient::mock and ApnsRequestParts::for_config, then assert the JSON alert
contains only the body plus the existing sound field.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 084b0124-0431-48c5-8c20-aec7dcf68d3d
📒 Files selected for processing (9)
CHANGELOG.mdREADME.mdconfig/default.tomlconfig/production.toml.examplesrc/config.rssrc/nostr/events/processor.rssrc/push/apns.rssrc/push/dispatcher/mod.rssrc/server/health.rs
| // APNs rejects `apns-collapse-id` values over 64 bytes with a 400 | ||
| // BadCollapseId on every send; fail at startup instead. | ||
| if self.collapse_id.len() > APNS_COLLAPSE_ID_MAX_BYTES { | ||
| return Err(config::ConfigError::Message(format!( | ||
| "apns.collapse_id must be at most {APNS_COLLAPSE_ID_MAX_BYTES} bytes, got {}", | ||
| self.collapse_id.len() | ||
| ))); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does http crate HeaderValue::from_str reject carriage return and line feed characters?
💡 Result:
Yes, the http crate's HeaderValue::from_str method rejects carriage return (CR, ASCII 13) and line feed (LF, ASCII 10) characters [1][2]. The method validates input to ensure it contains only visible ASCII characters, which are defined as the range 32–127 [3][1]. Since both carriage return (13) and line feed (10) fall outside this permitted range of visible ASCII characters, they are considered invalid, and attempting to create a HeaderValue containing them via from_str will return an error [3][1]. The official documentation explicitly includes examples showing that passing a string with a newline character results in an error [1][2].
Citations:
- 1: https://github.com/hyperium/http/blob/master/src/header/value.rs
- 2: https://docs.rs/reqwest/latest/reqwest/header/struct.HeaderValue.html
- 3: https://docs.rs/http/latest/src/http/header/value.rs.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the relevant config and request-building code.
git ls-files | rg '^(src/config\.rs|src/.*apns.*\.rs|src/.*request.*\.rs|src/.*http.*\.rs)$'
# Show the nearby config validation block.
printf '\n--- src/config.rs (around collapse_id validation) ---\n'
sed -n '1280,1335p' src/config.rs
# Find where apns-collapse-id is attached to requests.
printf '\n--- search for apns-collapse-id / collapse_id usage ---\n'
rg -n 'apns-collapse-id|collapse_id|HeaderValue::from_str|header\(' src
# If a likely builder is found, inspect its definition with line numbers.
printf '\n--- candidate build_request / ApnsRequestParts definitions ---\n'
rg -n 'build_request|ApnsRequestParts' srcRepository: marmot-protocol/transponder
Length of output: 14679
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the APNs request builder and tests around collapse_id handling.
sed -n '430,490p' src/push/apns.rs
printf '\n--- surrounding tests ---\n'
sed -n '930,990p' src/push/apns.rs
# Read the config tests that already cover the max-length validation.
printf '\n--- config tests ---\n'
sed -n '2128,2160p' src/config.rs
# Probe whether any in-repo code treats invalid header values as request-build errors.
rg -n 'build_request\(|RequestBuilder|header\("apns-collapse-id"|from_str\(&self\.collapse_id\)' src/push/apns.rs src/config.rsRepository: marmot-protocol/transponder
Length of output: 6356
🌐 Web query:
reqwest RequestBuilder header invalid HeaderValue returns error build send semantics
💡 Result:
In the reqwest library, the RequestBuilder maintains an internal Result state to handle configuration errors, including those that occur when adding invalid headers [1]. When you call the.header method on a RequestBuilder, it attempts to convert the provided key and value into a HeaderName and HeaderValue using TryFrom [2][1]. If the value provided is invalid (for example, containing restricted characters like CR, LF, or NUL [3]), this conversion returns an InvalidHeaderValue error [4][5]. Because RequestBuilder uses an internal mechanism to accumulate errors, providing an invalid header does not cause an immediate panic or return an error at the exact line where.header is called [1]. Instead, the error is stored within the RequestBuilder [1]. The error is surfaced only when you attempt to finalize or execute the request: 1. build: When you call.build, it consumes the RequestBuilder and returns a Result<Request, Error>. If a previous header configuration failed, this result will contain the error [1]. 2. send: When you call.send, it internally calls build. If the build process fails due to a stored error (such as an InvalidHeaderValue), the send method will return an error, preventing the request from being dispatched [1]. In summary, invalid header values do not interrupt the builder chain immediately; they "poison" the builder's internal state, and the resulting error is surfaced during the final build or send operation [1].
Citations:
- 1: https://deepwiki.com/seanmonstar/reqwest/3.2-request-building
- 2: https://docs.rs/reqwest/latest/reqwest/struct.RequestBuilder.html
- 3: [Security Issues]: Bad Header Parsing in
httpCrate seanmonstar/reqwest#2094 - 4: https://docs.rs/reqwest/latest/reqwest/header/struct.InvalidHeaderValue.html
- 5: https://docs.rs/reqwest/latest/reqwest/header/index.html
Validate apns.collapse_id as a header value too
The length guard is good, but collapse_id is later passed to reqwest::RequestBuilder::header(). If it contains CR/LF or another invalid byte, the builder becomes invalid and build()/send() returns an Err for every APNs notification. Rejecting invalid header values here would fail fast at startup, just like the other config checks.
🤖 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/config.rs` around lines 1300 - 1307, Extend the startup validation for
self.collapse_id alongside the existing length check to reject values that
cannot be used as HTTP header values, including CR/LF and other invalid bytes.
Return a config::ConfigError::Message with a clear validation error, preserving
the existing maximum-byte-length validation and fail-fast behavior.
| struct ApnsRequestParts { | ||
| push_type: &'static str, | ||
| priority: &'static str, | ||
| /// `apns-collapse-id` header value; `None` leaves the header off. | ||
| collapse_id: Option<String>, | ||
| payload: ApnsPayload, | ||
| } | ||
|
|
||
| impl ApnsRequestParts { | ||
| fn for_mode(mode: ApnsPayloadMode) -> Self { | ||
| fn for_config(config: &ApnsConfig) -> Self { | ||
| let mode = config.payload_mode; | ||
| Self { | ||
| push_type: mode.push_type(), | ||
| priority: mode.priority(), | ||
| payload: ApnsPayload::from(mode), | ||
| collapse_id: (!config.collapse_id.is_empty()).then(|| config.collapse_id.clone()), | ||
| payload: ApnsPayload::for_config(config), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does reqwest RequestBuilder::header return an Err instead of panicking when given an invalid header value string, and does that error propagate through send()?
💡 Result:
The reqwest::RequestBuilder::header method does not panic when provided with an invalid header value; instead, it stores the error internally, which then manifests when the request is sent [1][2][3]. The header method is generic and utilizes TryFrom to convert the provided key and value into HeaderName and HeaderValue types [2][3]. If the conversion fails (e.g., due to an invalid header value string), the resulting error is captured within the RequestBuilder [1][3]. When you subsequently call send() (or other methods that finalize the request), that stored error is returned, causing the operation to return an Err rather than panicking [1]. This design allows you to chain header additions and handle potential errors at the point where the request is actually executed [1][4]. If you need to validate header values beforehand, you can manually convert them using HeaderValue::from_str or HeaderValue::try_from, which return a Result<HeaderValue, InvalidHeaderValue> [5][6][7].
Citations:
- 1: https://stackoverflow.com/questions/76558735/rust-reqwesterror-kind-builder-source-httperrorinvalidheadervalue
- 2: https://docs.rs/reqwest/latest/reqwest/struct.RequestBuilder.html
- 3: https://docs.rs/reqwest/0.11.8/reqwest/struct.RequestBuilder.html
- 4: https://stackoverflow.com/questions/47911513/how-do-i-set-the-request-headers-using-reqwest
- 5: https://docs.rs/reqwest_wasi/latest/reqwest/header/struct.HeaderValue.html
- 6: https://docs.rs/reqwest/0.11.10/reqwest/header/struct.HeaderValue.html
- 7: https://docs.rs/reqwest/latest/reqwest/header/struct.HeaderValue.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files and inspect the specific regions mentioned in the review.
git ls-files src/push/apns.rs src/config.rs
echo '--- apns.rs outline ---'
ast-grep outline src/push/apns.rs --view expanded || true
echo '--- config.rs outline ---'
ast-grep outline src/config.rs --view expanded || true
echo '--- apns.rs lines around collapse_id usage ---'
sed -n '150,190p' src/push/apns.rs
echo '--- apns.rs build_request area ---'
sed -n '440,480p' src/push/apns.rs
echo '--- config.rs collapse_id validation/search ---'
rg -n "collapse_id|HeaderValue|length|64|apns" src/config.rs
sed -n '1,260p' src/config.rsRepository: marmot-protocol/transponder
Length of output: 41720
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Probe the local code semantics around collapse_id handling and APNs request building.
python3 - <<'PY'
from pathlib import Path
import re
apns = Path('src/push/apns.rs').read_text()
config = Path('src/config.rs').read_text()
# Extract the relevant snippets for quick inspection.
for label, text, pattern in [
('collapse_id field', apns, r'collapse_id: Option<String>,.*?for_config\(config: &ApnsConfig\) -> Self \{.*?\n \}',),
]:
pass
# Simple checks for validation presence.
print('config contains HeaderValue::from_str:', 'HeaderValue::from_str' in config)
print('config contains collapse_id non-empty check:', '!config.collapse_id.is_empty()' in apns)
print('apns build_request uses .header("apns-collapse-id", collapse_id):', '.header("apns-collapse-id", collapse_id)' in apns)
# Search for tests around collapse_id validation.
for path in ['src/config.rs', 'tests', 'src']:
pass
PYRepository: marmot-protocol/transponder
Length of output: 331
Validate apns.collapse_id as a header value src/config.rs:1300-1305; src/push/apns.rs:176,461
collapse_id is capped at 64 bytes, but it still accepts control characters and is later passed straight into request.header("apns-collapse-id", ...). Reject non-header-safe values at config load time, for example with HeaderValue::from_str, so a pasted newline or tab doesn't make every APNs send for that config fail at runtime.
🤖 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/apns.rs` around lines 162 - 180, Validate non-empty
config.collapse_id during configuration loading and reject values that cannot be
represented as an HTTP header, including control characters such as newlines and
tabs. Reuse the existing HeaderValue validation approach, then keep
ApnsRequestParts::for_config limited to cloning already-validated values before
request.header is called.
1 participant
Motivation
Deployments that want a visible iOS notification currently have to pick between the two existing
payload_modes, and neither fits:silentsendscontent-availableonly, which is invisible unless the app ships a Notification Service Extension or happens to be foregrounded.nse_prototype_alertis visible, but carries hardcoded White Noise copy ("White Noise" / "New encrypted message"), setsmutable-content: 1(so it really wants an NSE), and includes thewn_nse_prototypemarker.Some deployments would rather show a plain, content-free alert ("New activity") without shipping an NSE at all — the transponder is blind by design, so there is nothing app-specific the alert could say anyway.
What's added
A third APNs payload mode,
generic_alert:apns.alert_title/apns.alert_bodykeys (content-free defaults: "New activity" / "You have a new notification"). An empty value omits that key from thealertdictionary, supporting title-only or body-only alerts; configuring both empty is rejected at startup.{"aps":{"alert":{...},"sound":"default"}}— nomutable-content, no prototype marker.apns.collapse_idkey sets theapns-collapse-idheader on every send (any payload mode), so a rapid burst of pushes surfaces as one notification instead of a stack. Values over APNs's 64-byte limit are rejected at startup instead of failing per-send with400 BadCollapseId.Both existing modes are byte-for-byte unchanged (their tests still assert the exact payloads), and
generic_alertreuses the existingalert/10push-type/priority pairing. New keys are wired through theTRANSPONDER_APNS_*env-override allowlist and documented inconfig/default.toml,config/production.toml.example, and the README.Tests
test_generic_alert_mode_builds_alert_headers_and_payload— exact headers + exact payload JSON (proving nomutable-contentand no collapse header when unset)test_generic_alert_mode_omits_empty_body_from_alert— title-only alerttest_generic_alert_mode_sets_collapse_id_header_when_configured/test_collapse_id_header_applies_to_any_payload_modegeneric_alertparses with defaults, custom copy + collapse id, both-empty copy rejected, over-length collapse id rejectedcargo build,cargo test(717 passed),cargo fmt --check, andcargo clippy --all-targetsare all clean.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
generic_alertAPNs notification mode with configurable alert titles and bodies.Documentation