Add signed ActivityPub server federation flows - #42
Conversation
Add the security JSON-LD context and model embedded CryptographicKey values on ActivityPub actors. Cover the publicKey representation with serialization tests. Assisted-by: Codex:gpt-5.6-sol
…r Core. Accept runtime-provided entropy, validate persisted pairs, and zeroize private PEM material. Use fixed fixtures for behavioral tests and exercise all workspace features and targets in CI. Assisted-by: Codex:gpt-5.6-sol
Enable HTTP signature keys in the server runtime and store validated per-actor key pairs in the keys table. Reject accidental replacement and test schema creation, round trips, malformed keys, missing actors, and persistence across reopen. Assisted-by: Codex:gpt-5.6-sol
Move HTTP endpoint and startup tests under the tests directory, with shared fixtures kept private to the integration test crate. Assisted-by: Codex:gpt-5.6-sol
Generate and persist a 4096-bit RSA key pair with operating system entropy when an actor has no stored key, and reuse the persisted pair on restart. Assisted-by: Codex:gpt-5.6-sol
Attach the persisted RSA public key to the local actor as an embedded CryptographicKey using the #main-key identifier and security context. Assisted-by: Codex:gpt-5.6-sol
Deliver SendActivity actions synchronously to recipient inboxes as ActivityPub JSON, report remote failures, and continue attempting later recipients after individual delivery failures. Assisted-by: Codex:gpt-5.6-sol
Create draft-Cavage RSA signatures in the portable core and apply them to outgoing ActivityPub requests using the persisted actor key and matching #main-key identifier. Assisted-by: Codex:gpt-5.6-sol
Sign the complete path and query for outgoing ActivityPub requests, and add regression coverage for inbox URLs containing query parameters. Assisted-by: Codex:gpt-5.6-sol
Block private and special-use inbox destinations using validated DNS resolution, with an explicit option for local development. Configure connection and total request timeouts to prevent stalled deliveries. Assisted-by: Codex:gpt-5.6-sol
…vered inbox, and deliver the signed Accept activity. Reuse outbound network protections and enforce response type, size, and actor ID validation. Add an end-to-end test covering Mastodon-style actor resolution and Accept delivery. Assisted-by: Codex:gpt-5.6-sol
Validate Cavage RSA-SHA256 signatures, signed headers, request dates, body digests, actor ownership, and public-key identity before processing inbox activities. Add tests for valid, tampered, unsigned, and mismatched-actor requests. Assisted-by: Codex:gpt-5.6-sol
Fetch signature key IDs directly and support standalone key documents as well as keys embedded in actor documents. Verify that the activity actor owns and advertises the resolved key before processing the request. Add coverage for signed Follows using an independent key URL. Assisted-by: Codex:gpt-5.6-sol
Default omitted public key types to CryptographicKey for compatibility with Mastodon actor documents. Cover the representation in vocabulary and signed Follow integration tests. Assisted-by: Codex:gpt-5.6-sol
Add Undo vocabulary and core handling for follower removal. Validate that the Undo actor owns the embedded Follow, remove matching in-memory and SQLite follower state, and cover signed unfollow and forged Undo requests. Assisted-by: Codex:gpt-5.6-sol
Add the OrderedCollection vocabulary type and followers property for actors. Advertise the local actor's followers collection URI and preserve the property when resolving remote actors. Assisted-by: Codex:gpt-5.6-sol
Add a storage-backed followers handler and expose it through the runtime router. Return an ActivityPub OrderedCollection with current follower IDs and counts, and cover collection updates, response headers, and unknown users. Assisted-by: Codex:gpt-5.6-sol
Require an explicit ActivityPub-compatible Accept header for actor and followers collection requests. Return 406 for HTML, wildcard-only, missing, or unsupported Accept headers, and mark successful responses with Vary: Accept. Add coverage for supported media types, quality preferences, HTML requests, and requests without an ActivityPub Accept header. Assisted-by: Codex:gpt-5.6-sol
Add Vary: Accept to actor and followers responses that reject requests without an ActivityPub-compatible Accept header, preventing caches from reusing those responses across representations. Assisted-by: Codex:gpt-5.6-sol
Ignore unsupported Accept media types when choosing between HTML and ActivityPub. Compare the best quality in each supported representation group and use header order to resolve equal preferences. Assisted-by: Codex:gpt-5.6-sol
Add to, cc, url, and mediaType fields to the Note vocabulary model. Preserve ActivityStreams scalar-or-array recipient serialization and omit empty or absent values. Assisted-by: Codex:gpt-5.6-sol
Store Note objects emitted through StoreObject actions in SQLite using their IRI, object type, and serialized payload. Add typed object loading, idempotent replacement, and persistence across database reopen. Assisted-by: Codex:gpt-5.6-sol
Add the local post object route with ActivityPub content negotiation, SQLite-backed lookup, and coverage for persistence and error responses. Assisted-by: Codex:gpt-5.6-sol
Add a runtime operation for local Note creation that applies storage actions before synchronously delivering Create activities. Reuse the same action pipeline for inbox processing and cover successful and failed delivery. Assisted-by: Codex:gpt-5.6-sol
Remove the in-memory delivery target cache and resolve current follower inboxes from SQLite when delivering Create activities. Keep delivery synchronous, deduplicate shared inboxes, and refresh inbox data through repeated Follow persistence. Assisted-by: Codex:gpt-5.6-sol
Populate Note addressing and metadata, mirror to and cc onto Create, and use one SendActivity action with typed inbox or follower recipients. Resolve follower recipients into concrete inbox deliveries at runtime. Assisted-by: Codex:gpt-5.6-sol
Assisted-by: Codex:gpt-5.6-sol
Assisted-by: Codex:gpt-5.6-sol
Assisted-by: Codex:gpt-5.6-sol
Assisted-by: Codex:gpt-5.6-sol
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds ActivityPub vocabulary and HTTP Signature support, replaces delivery-target state with recipient routing, adds actor resolution and signed delivery, verifies signed inbox requests, persists objects and keys in SQLite, and introduces followers/object endpoints with comprehensive integration tests. ChangesAuthenticated ActivityPub runtime
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant RemoteActor
participant Inbox
participant ActorResolver
participant FederCore
participant ActivitySender
RemoteActor->>Inbox: POST signed Follow or Undo
Inbox->>ActorResolver: resolve actor and signing key
ActorResolver-->>Inbox: verified actor data
Inbox->>FederCore: handle federation input
FederCore-->>ActivitySender: Accept or removal action
ActivitySender->>RemoteActor: signed ActivityPub POST
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cbc265a0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Create SQLite database files with owner-only permissions and restrict existing files before opening them. Document Linux as the currently supported runtime target. Assisted-by: Codex:gpt-5.6-sol
Resolve direct recipients independently so an unavailable actor does not prevent delivery to recipients that resolve successfully. Assisted-by: Codex:gpt-5.6-sol
Track resolved inboxes across follower, direct actor, and pre-resolved recipients so each inbox receives an activity only once. Assisted-by: Codex:gpt-5.6-sol
![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 (16)
crates/feder-runtime-server/src/storage/sqlite.rs (3)
157-171: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMatch
Action::SendActivityexplicitly instead of a wildcard arm.The
_ => {}arm silently swallows anyActionvariant added later, which for a persistence layer means silent data loss rather than a compile error.Actionis not#[non_exhaustive], so an explicit arm gives you exhaustiveness checking for free.♻️ Suggested change
- _ => {} + // Delivery is performed by the runtime; nothing to persist. + Action::SendActivity(_) => {}🤖 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 `@crates/feder-runtime-server/src/storage/sqlite.rs` around lines 157 - 171, Replace the wildcard arm in the action match with an explicit Action::SendActivity arm that performs no operation. Keep the existing Action::StoreObject persistence logic unchanged and rely on exhaustive matching so newly added Action variants require deliberate handling.
257-275: 🔒 Security & Privacy | 🔵 TrivialActor private keys are persisted unencrypted; document the operational expectations.
private_key_pemis written as plaintext into the SQLite file. The0o600mode on the database is the only protection, and it doesn't cover backups, snapshots, or the surrounding directory. For the preview this is a reasonable trade-off, but consider documenting it (README / deployment notes) and planning for: encryption at rest or an external key store, key rotation, and restricting the parent directory to0o700.🤖 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 `@crates/feder-runtime-server/src/storage/sqlite.rs` around lines 257 - 275, The SQLite storage path around insert_actor_key_pair persists private keys in plaintext; document this operational expectation in the relevant README or deployment notes, including protection of backups and snapshots, parent-directory permissions of 0o700, and future plans for encryption at rest or an external key store and key rotation.
537-567: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
tempfileover hand-rolled temp paths in these tests.Both reopen tests build a path from pid + nanos in the shared temp dir and clean up with a trailing
remove_file, which is skipped whenever an assertion panics — leaving stray SQLite files behind across runs.tempfile::TempDir(dev-dependency) gives RAII cleanup and a non-predictable directory.Also applies to: 647-677
🤖 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 `@crates/feder-runtime-server/src/storage/sqlite.rs` around lines 537 - 567, Update the reopen persistence tests, including stored_note_persists_across_store_reopen and the other reopen test, to use tempfile::TempDir instead of manually constructing paths in the system temporary directory. Derive the SQLite path within the TempDir and remove the explicit remove_file cleanup, preserving the existing open, reopen, and assertion behavior.crates/feder-core/src/http_signatures.rs (1)
123-145: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winEscape or validate
key_idbefore embedding it in the Signature header.
key_idis interpolated into a quoted header parameter with no escaping. A"(or\) in the value produces a malformed header and lets a caller smuggle extra signature parameters. Today's only caller derives it from the local actor IRI, so this isn't exploitable, but this is a public library API — cheap to harden here rather than relying on every caller.🛡️ Suggested hardening
pub fn sign_draft_cavage( key_pair: &ActorKeyPair, key_id: &str, method: &str, request_target: &str, headers: &[(&str, &str)], ) -> Result<String, HttpSignatureError> { + if key_id.contains(['"', '\\']) || key_id.chars().any(char::is_control) { + return Err(HttpSignatureError::InvalidKeyId); + } let signature_base = draft_cavage_signature_base(method, request_target, headers);Add the matching variant to
HttpSignatureErrorand itsDisplayarm.🤖 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 `@crates/feder-core/src/http_signatures.rs` around lines 123 - 145, Harden sign_draft_cavage by validating key_id before interpolating it into the quoted Signature header parameter, rejecting values containing quotes or backslashes. Add the corresponding HttpSignatureError variant and handle it in the error’s Display implementation, returning that error for invalid key identifiers while preserving valid key_id behavior.crates/feder-vocab/tests/activitypub_serialization.rs (1)
46-76: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a deserialization test for a real-world mixed
@context.The new tests only cover contexts this crate produces. Given
Actoris now parsed from remote documents, a test that deserializes"@context": ["https://www.w3.org/ns/activitystreams", "https://w3id.org/security/v1", {"manuallyApprovesFollowers": "as:manuallyApprovesFollowers"}]would pin the interop behavior flagged onContextincrates/feder-vocab/src/lib.rs.🤖 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 `@crates/feder-vocab/tests/activitypub_serialization.rs` around lines 46 - 76, The activitypub serialization tests lack coverage for deserializing a real-world mixed `@context`. Add a deserialization test alongside actor_serializes_embedded_cryptographic_key using the ActivityStreams URL, security URL, and manuallyApprovesFollowers mapping, then assert the document parses into the expected Actor representation and preserves the flagged Context interoperability behavior.crates/feder-core/src/lib.rs (1)
255-275: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
note_recipientstreats a remote followers-collection address as a resolvable actor.Any
to/ccentry that isn't Public, the local followers IRI, or the local actor becomesRecipients::Actor. Addressing e.g.https://remote.example/users/bob/followerswill fail actor resolution, andresolve_outbound_deliveriesincrates/feder-runtime-server/src/operation.rssurfaces the first resolve error, so the wholecreate_notereturnsErrdespite other deliveries succeeding. Acceptable for the preview, but worth a follow-up (or a doc comment stating that only actor IRIs are addressable).🤖 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 `@crates/feder-core/src/lib.rs` around lines 255 - 275, The note_recipients function currently converts remote followers-collection addresses into Recipients::Actor, causing outbound resolution failures. Update recipient classification to handle only supported actor-addressable recipients, and explicitly exclude or otherwise safely handle remote followers collection IRIs so create_note does not fail while resolving them; preserve existing handling for PUBLIC_COLLECTION, the local followers collection, the local actor, and ordinary actor IRIs.crates/feder-runtime-server/src/send.rs (1)
57-69: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftNo retry/backoff for transient delivery failures.
Issue
#26lists "bounded retry/backoff and durable status"; delivery here is single-shot and the failure is only surfaced as an error. Fine for a preview, but worth tracking. Want me to open an issue?🤖 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 `@crates/feder-runtime-server/src/send.rs` around lines 57 - 69, Update send_actions and its delivery flow to retry transient send failures with bounded backoff before recording the final error. Preserve processing of all actions and first-error reporting, while leaving non-transient failures non-retriable.crates/feder-runtime-server/src/app.rs (1)
55-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFollowers IRI is built by string concatenation while
object.rsbuilds IDs withUrlpath manipulation.If
config.actor_idever carries a query or fragment, this produces.../alice?x=1/followers(still parses, silently wrong), whereasnote_idincrates/feder-runtime-server/src/object.rsclears query/fragment first. Consider reusing the same URL-based construction for consistency.🤖 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 `@crates/feder-runtime-server/src/app.rs` around lines 55 - 59, Update the followers IRI construction in the actor setup to use URL path manipulation, matching the approach used for note IDs in object.rs: clear any query and fragment from the actor URL, append the followers path segment, and preserve the existing valid-IRI expectation.crates/feder-runtime-server/src/object.rs (2)
49-59: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueNegotiation runs after the store lookup.
Cheap request-shape checks are usually done before I/O; moving
accepts_activitypubabove theload_objectcall avoids a DB hit for requests that will get 406, and keeps the 404/406 distinction independent of stored state.🤖 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 `@crates/feder-runtime-server/src/object.rs` around lines 49 - 59, Move the accepts_activitypub(&headers) check before the note store lookup and any stored-state validation in the object handler. Return the existing 406 response immediately for unsupported requests, while preserving the current 404 handling and successful response flow for accepted requests.
71-84: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
post_idis pushed as a raw path segment.Values such as
..or.are appended without normalization, producing an odd object IRI. It only results in a lookup miss today, but rejecting non-[A-Za-z0-9_-]ids up front would be more predictable.🤖 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 `@crates/feder-runtime-server/src/object.rs` around lines 71 - 84, Update note_id to validate post_id before modifying the URL, accepting only non-empty identifiers containing exclusively ASCII letters, digits, underscores, or hyphens; return an appropriate client-error StatusCode for invalid values, and preserve the existing URL construction for valid IDs.crates/feder-runtime-server/src/inbox.rs (1)
182-200: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueWide acceptance window.
65 minutes past / 60 minutes future is quite permissive for signature freshness; most implementations use a few minutes of skew. Combined with the absent replay cache this widens the replay surface.
🤖 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 `@crates/feder-runtime-server/src/inbox.rs` around lines 182 - 200, Reduce the signature freshness window enforced by verify_request_date: replace the current MAX_SIGNATURE_AGE and MAX_CLOCK_SKEW values with a few-minute limit appropriate for the service, while preserving rejection of requests older than the age limit or farther ahead than the skew limit.crates/feder-runtime-server/src/operation.rs (2)
64-108: 🩺 Stability & Availability | 🔵 TrivialFan-out delivery is fully synchronous with the inbound request.
Every follower inbox (and every remote actor resolution) is awaited sequentially inside the HTTP handler, so request latency scales linearly with follower count and with the slowest remote host. A background delivery queue with per-target status would be the natural next step beyond the preview.
🤖 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 `@crates/feder-runtime-server/src/operation.rs` around lines 64 - 108, Refactor the action fan-out flow around the SendActivity handling loop so follower expansion and remote actor resolution are dispatched through a background delivery queue instead of being awaited synchronously by the inbound request. Preserve inbox deduplication and capture per-target delivery status/errors in the queued work, allowing the handler to return without waiting on every recipient or actor resolver.
45-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueResolution error is discarded when delivery also fails.
send_actions(...).await?returns early, soactor_resolve_erroris never reported when both fail. Given the doc comment promises "returns an error" for any failure, consider preferring the resolve error or wrapping both.🤖 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 `@crates/feder-runtime-server/src/operation.rs` around lines 45 - 51, Update the error handling in the operation flow around resolve_outbound_deliveries and activity_sender.send_actions so actor_resolve_error is still reported when delivery sending also fails. Preserve the documented contract of returning an error for either failure, preferring the actor resolution error or combining both instead of allowing the send_actions await? to return early.crates/feder-runtime-server/src/url.rs (1)
100-135: 🔒 Security & Privacy | 🔵 TrivialDomain-based SSRF blocks lose structured error context compared to literal-IP blocks.
validate_literal_hostproduces a specificPrivateInboxAddress/PrivateResourceAddress { address, .. }for literal-IP targets, but when a hostname resolves to a non-public address, the block happens here via a plainio::Errorthat reqwest surfaces as an opaque transport error (SendError::Request/ActorResolveError::Request). Operators can't distinguish "blocked for SSRF" from "genuine DNS/network failure" from the error type alone — confirmed byblocks_hostname_resolving_to_private_addressintests/cases/send.rsexpectingSendError::Request(_)rather than a private-address variant.Consider surfacing the blocked address distinctly, e.g. by having the resolver return a typed error the caller can inspect (or logging the blocked address at the point of rejection) so delivery-failure metrics/logs can separate policy blocks from real outages.
🤖 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 `@crates/feder-runtime-server/src/url.rs` around lines 100 - 135, Update PublicDnsResolver::resolve to preserve structured context when validate_public_address rejects a resolved address, rather than converting the block directly into a plain io::Error. Propagate a typed or otherwise inspectable policy-block error containing the rejected address, and ensure the caller can distinguish SSRF policy blocks from ordinary DNS or transport failures while preserving existing rejection behavior.crates/feder-runtime-server/tests/cases/send.rs (1)
222-234: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a regression test for the IPv4-mapped IPv6 bypass path.
validate_public_address(url.rs) recursively unwraps::ffff:a.b.c.dIPv4-mapped IPv6 addresses before checking them against the IPv4 CIDR list — but no test here exercises that path (e.g.http://[::ffff:127.0.0.1]/inbox). Since this mapping is the one case where correctness depends on the unwrap-then-recheck logic rather than a direct CIDR containment check, it's worth a dedicated test alongside the existing special-use IPv6 cases.Suggested addition
for address in ["100:0:0:1::1", "2001:2::1", "5f00::1"] { let inbox = format!("http://[{address}]/inbox"); let actions = [create_note_send_action(&inbox)]; let result = sender.send_actions(&actions).await; assert!(matches!(result, Err(SendError::PrivateInboxAddress { .. }))); } + + let inbox = "http://[::ffff:127.0.0.1]/inbox"; + let actions = [create_note_send_action(inbox)]; + let result = sender.send_actions(&actions).await; + assert!(matches!(result, Err(SendError::PrivateInboxAddress { .. }))); }🤖 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 `@crates/feder-runtime-server/tests/cases/send.rs` around lines 222 - 234, Add a dedicated regression test alongside blocks_special_use_ipv6_inbox_addresses that sends an inbox URL using an IPv4-mapped IPv6 address such as ::ffff:127.0.0.1, and assert it returns SendError::PrivateInboxAddress under OutboundAddressPolicy::PublicOnly. Exercise the mapped-address unwrap and IPv4 CIDR recheck path without changing the existing special-use IPv6 test.crates/feder-runtime-server/tests/cases/followers.rs (1)
28-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated
iri()test helper into the shared test module. The samefn iri(value: &str) -> Iri { value.parse().expect("valid test IRI") }is copy-pasted verbatim in three test files; each already imports fromcrate::common, so this belongs there instead.
crates/feder-runtime-server/tests/cases/followers.rs#L28-L30: remove the localirifn and import it fromcrate::common.crates/feder-runtime-server/tests/cases/object.rs#L29-L31: remove the localirifn and import it fromcrate::common.crates/feder-runtime-server/tests/cases/operation.rs#L30-L32: remove the localirifn and import it fromcrate::common.♻️ Proposed consolidation
// tests/common/mod.rs pub fn iri(value: &str) -> feder_vocab::Iri { value.parse().expect("valid test IRI") }🤖 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 `@crates/feder-runtime-server/tests/cases/followers.rs` around lines 28 - 30, Move the duplicated iri helper into the shared test module as a public function returning feder_vocab::Iri, preserving its existing parsing behavior. Remove the local iri definitions and import the shared helper in crates/feder-runtime-server/tests/cases/followers.rs:28-30, crates/feder-runtime-server/tests/cases/object.rs:29-31, and crates/feder-runtime-server/tests/cases/operation.rs:30-32.
🤖 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 `@crates/feder-runtime-server/src/inbox.rs`:
- Around line 110-118: Update the inbound request validation around
verify_request_date and verify_request_digest to validate the signed host header
against the configured local authority before signature verification. In
crates/feder-runtime-server/src/inbox.rs:110-118, add this authority check; in
crates/feder-runtime-server/src/inbox.rs:182-200, tighten
MAX_SIGNATURE_AGE/MAX_CLOCK_SKEW and/or add a short-lived cache keyed by
signature value to reject previously seen signatures within the acceptance
window.
In `@crates/feder-vocab/src/lib.rs`:
- Around line 180-209: The Context enum only accepts IRI-shaped values, so
inline object entries cause Actor deserialization to fail. In
crates/feder-vocab/src/lib.rs lines 180-209, add a catch-all context variant or
otherwise deserialize unknown shapes leniently while keeping include restricted
to Iri and Iris. In crates/feder-vocab/tests/activitypub_serialization.rs lines
46-76, add a deserialization test using the specified two IRIs plus the
manuallyApprovesFollowers object to verify tolerant handling.
- Around line 426-436: The Undo handling in the inbox currently deserializes
every undo object as Reference<Follow>, causing non-Follow undos to return
BAD_REQUEST. Update the undo processing in the inbox flow to inspect object.type
before deserializing as Follow, and return the existing 202 response for Undo
objects such as Like, Announce, or Accept while preserving Follow handling.
---
Nitpick comments:
In `@crates/feder-core/src/http_signatures.rs`:
- Around line 123-145: Harden sign_draft_cavage by validating key_id before
interpolating it into the quoted Signature header parameter, rejecting values
containing quotes or backslashes. Add the corresponding HttpSignatureError
variant and handle it in the error’s Display implementation, returning that
error for invalid key identifiers while preserving valid key_id behavior.
In `@crates/feder-core/src/lib.rs`:
- Around line 255-275: The note_recipients function currently converts remote
followers-collection addresses into Recipients::Actor, causing outbound
resolution failures. Update recipient classification to handle only supported
actor-addressable recipients, and explicitly exclude or otherwise safely handle
remote followers collection IRIs so create_note does not fail while resolving
them; preserve existing handling for PUBLIC_COLLECTION, the local followers
collection, the local actor, and ordinary actor IRIs.
In `@crates/feder-runtime-server/src/app.rs`:
- Around line 55-59: Update the followers IRI construction in the actor setup to
use URL path manipulation, matching the approach used for note IDs in object.rs:
clear any query and fragment from the actor URL, append the followers path
segment, and preserve the existing valid-IRI expectation.
In `@crates/feder-runtime-server/src/inbox.rs`:
- Around line 182-200: Reduce the signature freshness window enforced by
verify_request_date: replace the current MAX_SIGNATURE_AGE and MAX_CLOCK_SKEW
values with a few-minute limit appropriate for the service, while preserving
rejection of requests older than the age limit or farther ahead than the skew
limit.
In `@crates/feder-runtime-server/src/object.rs`:
- Around line 49-59: Move the accepts_activitypub(&headers) check before the
note store lookup and any stored-state validation in the object handler. Return
the existing 406 response immediately for unsupported requests, while preserving
the current 404 handling and successful response flow for accepted requests.
- Around line 71-84: Update note_id to validate post_id before modifying the
URL, accepting only non-empty identifiers containing exclusively ASCII letters,
digits, underscores, or hyphens; return an appropriate client-error StatusCode
for invalid values, and preserve the existing URL construction for valid IDs.
In `@crates/feder-runtime-server/src/operation.rs`:
- Around line 64-108: Refactor the action fan-out flow around the SendActivity
handling loop so follower expansion and remote actor resolution are dispatched
through a background delivery queue instead of being awaited synchronously by
the inbound request. Preserve inbox deduplication and capture per-target
delivery status/errors in the queued work, allowing the handler to return
without waiting on every recipient or actor resolver.
- Around line 45-51: Update the error handling in the operation flow around
resolve_outbound_deliveries and activity_sender.send_actions so
actor_resolve_error is still reported when delivery sending also fails. Preserve
the documented contract of returning an error for either failure, preferring the
actor resolution error or combining both instead of allowing the send_actions
await? to return early.
In `@crates/feder-runtime-server/src/send.rs`:
- Around line 57-69: Update send_actions and its delivery flow to retry
transient send failures with bounded backoff before recording the final error.
Preserve processing of all actions and first-error reporting, while leaving
non-transient failures non-retriable.
In `@crates/feder-runtime-server/src/storage/sqlite.rs`:
- Around line 157-171: Replace the wildcard arm in the action match with an
explicit Action::SendActivity arm that performs no operation. Keep the existing
Action::StoreObject persistence logic unchanged and rely on exhaustive matching
so newly added Action variants require deliberate handling.
- Around line 257-275: The SQLite storage path around insert_actor_key_pair
persists private keys in plaintext; document this operational expectation in the
relevant README or deployment notes, including protection of backups and
snapshots, parent-directory permissions of 0o700, and future plans for
encryption at rest or an external key store and key rotation.
- Around line 537-567: Update the reopen persistence tests, including
stored_note_persists_across_store_reopen and the other reopen test, to use
tempfile::TempDir instead of manually constructing paths in the system temporary
directory. Derive the SQLite path within the TempDir and remove the explicit
remove_file cleanup, preserving the existing open, reopen, and assertion
behavior.
In `@crates/feder-runtime-server/src/url.rs`:
- Around line 100-135: Update PublicDnsResolver::resolve to preserve structured
context when validate_public_address rejects a resolved address, rather than
converting the block directly into a plain io::Error. Propagate a typed or
otherwise inspectable policy-block error containing the rejected address, and
ensure the caller can distinguish SSRF policy blocks from ordinary DNS or
transport failures while preserving existing rejection behavior.
In `@crates/feder-runtime-server/tests/cases/followers.rs`:
- Around line 28-30: Move the duplicated iri helper into the shared test module
as a public function returning feder_vocab::Iri, preserving its existing parsing
behavior. Remove the local iri definitions and import the shared helper in
crates/feder-runtime-server/tests/cases/followers.rs:28-30,
crates/feder-runtime-server/tests/cases/object.rs:29-31, and
crates/feder-runtime-server/tests/cases/operation.rs:30-32.
In `@crates/feder-runtime-server/tests/cases/send.rs`:
- Around line 222-234: Add a dedicated regression test alongside
blocks_special_use_ipv6_inbox_addresses that sends an inbox URL using an
IPv4-mapped IPv6 address such as ::ffff:127.0.0.1, and assert it returns
SendError::PrivateInboxAddress under OutboundAddressPolicy::PublicOnly. Exercise
the mapped-address unwrap and IPv4 CIDR recheck path without changing the
existing special-use IPv6 test.
In `@crates/feder-vocab/tests/activitypub_serialization.rs`:
- Around line 46-76: The activitypub serialization tests lack coverage for
deserializing a real-world mixed `@context`. Add a deserialization test alongside
actor_serializes_embedded_cryptographic_key using the ActivityStreams URL,
security URL, and manuallyApprovesFollowers mapping, then assert the document
parses into the expected Actor representation and preserves the flagged Context
interoperability behavior.
🪄 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 Plus
Run ID: f396ada4-c33a-4a86-993d-7aaa8831b279
⛔ Files ignored due to path filters (6)
Cargo.lockis excluded by!**/*.lockcrates/feder-core/tests/fixtures/rsa-other-public-key.pemis excluded by!**/*.pemcrates/feder-core/tests/fixtures/rsa-private-key.pemis excluded by!**/*.pemcrates/feder-core/tests/fixtures/rsa-public-key.pemis excluded by!**/*.pemcrates/feder-runtime-server/tests/fixtures/rsa-private-key.pemis excluded by!**/*.pemcrates/feder-runtime-server/tests/fixtures/rsa-public-key.pemis excluded by!**/*.pem
📒 Files selected for processing (36)
Cargo.tomlcrates/feder-core/Cargo.tomlcrates/feder-core/src/http_signatures.rscrates/feder-core/src/lib.rscrates/feder-runtime-server/Cargo.tomlcrates/feder-runtime-server/README.mdcrates/feder-runtime-server/src/actor.rscrates/feder-runtime-server/src/app.rscrates/feder-runtime-server/src/config.rscrates/feder-runtime-server/src/error.rscrates/feder-runtime-server/src/followers.rscrates/feder-runtime-server/src/inbox.rscrates/feder-runtime-server/src/lib.rscrates/feder-runtime-server/src/negotiation.rscrates/feder-runtime-server/src/object.rscrates/feder-runtime-server/src/operation.rscrates/feder-runtime-server/src/send.rscrates/feder-runtime-server/src/storage/mod.rscrates/feder-runtime-server/src/storage/sqlite.rscrates/feder-runtime-server/src/url.rscrates/feder-runtime-server/src/webfinger.rscrates/feder-runtime-server/tests/cases/actor.rscrates/feder-runtime-server/tests/cases/app.rscrates/feder-runtime-server/tests/cases/followers.rscrates/feder-runtime-server/tests/cases/inbox.rscrates/feder-runtime-server/tests/cases/object.rscrates/feder-runtime-server/tests/cases/operation.rscrates/feder-runtime-server/tests/cases/send.rscrates/feder-runtime-server/tests/cases/webfinger.rscrates/feder-runtime-server/tests/common/mod.rscrates/feder-runtime-server/tests/runtime.rscrates/feder-vocab/src/lib.rscrates/feder-vocab/tests/activitypub_serialization.rsexamples/single-user-server/README.mdexamples/single-user-server/src/main.rsmise.toml
💤 Files with no reviewable changes (1)
- crates/feder-runtime-server/src/webfinger.rs
| if !["host", "date", "digest"] | ||
| .iter() | ||
| .all(|required| seen_headers.contains(required)) | ||
| { | ||
| return Err(StatusCode::UNAUTHORIZED); | ||
| } | ||
|
|
||
| verify_request_date(&req.headers)?; | ||
| verify_request_digest(&req.headers, &req.body)?; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Inbound signature verification has no replay protection. Signed requests are accepted purely on freshness, with no binding to this server's authority and no record of already-seen signatures, so a captured request stays usable for the full acceptance window.
crates/feder-runtime-server/src/inbox.rs#L110-L118: validate the signedhostheader value against the configured local authority before verifying the signature.crates/feder-runtime-server/src/inbox.rs#L182-L200: tightenMAX_SIGNATURE_AGE/MAX_CLOCK_SKEWand/or add a short-lived seen-signature cache keyed by the signature value.
📍 Affects 1 file
crates/feder-runtime-server/src/inbox.rs#L110-L118(this comment)crates/feder-runtime-server/src/inbox.rs#L182-L200
🤖 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 `@crates/feder-runtime-server/src/inbox.rs` around lines 110 - 118, Update the
inbound request validation around verify_request_date and verify_request_digest
to validate the signed host header against the configured local authority before
signature verification. In crates/feder-runtime-server/src/inbox.rs:110-118, add
this authority check; in crates/feder-runtime-server/src/inbox.rs:182-200,
tighten MAX_SIGNATURE_AGE/MAX_CLOCK_SKEW and/or add a short-lived cache keyed by
signature value to reject previously seen signatures within the acceptance
window.
| /// A JSON-LD context represented by one or more IRIs. | ||
| #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] | ||
| #[serde(untagged)] | ||
| pub enum Context { | ||
| Iri(Iri), | ||
| Iris(Vec<Iri>), | ||
| } | ||
|
|
||
| impl Context { | ||
| #[must_use] | ||
| pub fn one(context: Iri) -> Self { | ||
| Self::Iri(context) | ||
| } | ||
|
|
||
| #[must_use] | ||
| pub fn many(contexts: impl Into<Vec<Iri>>) -> Self { | ||
| Self::Iris(contexts.into()) | ||
| } | ||
|
|
||
| fn include(&mut self, context: Iri) { | ||
| match self { | ||
| Self::Iri(existing) if existing == &context => {} | ||
| Self::Iri(existing) => { | ||
| *self = Self::Iris(Vec::from([existing.clone(), context])); | ||
| } | ||
| Self::Iris(existing) if !existing.contains(&context) => existing.push(context), | ||
| Self::Iris(_) => {} | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Context only models IRI-shaped @context values, and nothing tests otherwise. Most Fediverse actor documents embed an inline object in the @context array, which matches neither Context::Iri nor Context::Iris; with #[serde(untagged)] that makes Actor deserialization fail for the remote actors this PR now resolves.
crates/feder-vocab/src/lib.rs#L180-L209: add a catch-all variant (or deserialize@contextleniently) so unknown context shapes are preserved rather than rejected, and keepincludeoperating only on the IRI variants.crates/feder-vocab/tests/activitypub_serialization.rs#L46-L76: add a deserialization case for"@context": [ACTIVITYSTREAMS_CONTEXT, SECURITY_CONTEXT, {"manuallyApprovesFollowers": "as:manuallyApprovesFollowers"}]to lock in the tolerant behavior.
📍 Affects 2 files
crates/feder-vocab/src/lib.rs#L180-L209(this comment)crates/feder-vocab/tests/activitypub_serialization.rs#L46-L76
🤖 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 `@crates/feder-vocab/src/lib.rs` around lines 180 - 209, The Context enum only
accepts IRI-shaped values, so inline object entries cause Actor deserialization
to fail. In crates/feder-vocab/src/lib.rs lines 180-209, add a catch-all context
variant or otherwise deserialize unknown shapes leniently while keeping include
restricted to Iri and Iris. In
crates/feder-vocab/tests/activitypub_serialization.rs lines 46-76, add a
deserialization test using the specified two IRIs plus the
manuallyApprovesFollowers object to verify tolerant handling.
| /// A minimal Undo activity for a Follow. | ||
| #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] | ||
| pub struct Undo { | ||
| #[serde(rename = "@context", skip_serializing_if = "Option::is_none")] | ||
| pub context: Option<Iri>, | ||
| #[serde(rename = "type")] | ||
| pub kind: UndoType, | ||
| pub id: Iri, | ||
| pub actor: Reference<Actor>, | ||
| pub object: Reference<Follow>, | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C6 'Some\("Undo"\)' crates/feder-runtime-server/src/inbox.rs
rg -nP -C3 'undo' crates/feder-runtime-server/tests/cases/inbox.rs | head -60Repository: fedify-dev/feder
Length of output: 2914
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== lib Undo/Follow definitions =="
sed -n '380,450p' crates/feder-vocab/src/lib.rs
printf '\n'
echo "== inbox handler Undo branch =="
sed -n '340,385p' crates/feder-runtime-server/src/inbox.rs
printf '\n'
echo "== other activity dispatch =="
rg -n '=> \{|Some\("' crates/feder-runtime-server/src/inbox.rs -A2 -B2 | sed -n '1,220p'
echo "== Activity enum / types in vocab =="
rg -n 'enum .*Activity|pub enum .*Activity|struct Undo|enum UndoType|struct Follow' crates/feder-vocab/src/lib.rs crates/feder-runtime-server/src -A3 -B3
echo "== tests around unsupported/accept/inbox statuses =="
fd -p '*inbox*' crates/feder-runtime-server/tests | xargs ls -la
rg -n 'BAD_REQUEST|ACCEPTED|202|BAD_REQUEST|Bad Request|200 OK' crates/feder-runtime-server/tests -A4 -B4 | sed -n '1,240p'Repository: fedify-dev/feder
Length of output: 8152
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
lib = open('crates/feder-vocab/src/lib.rs', encoding='utf-8').read()
inbox = open('crates/feder-runtime-server/src/inbox.rs', encoding='utf-8').read()
# Extract Undo struct and object type annotation from lib.rs
m = re.search(r'/// A minimal Undo activity for a Follow\.\n#\[derive.*?\npub struct Undo \{(?P<body>.*?)\n\}', lib, re.S)
if m:
print("Undo struct body:")
print(m.group('body'))
else:
print("Undo struct not found")
# Check whether inbox handles Other activities as 202 before Undo deserialization.
if 'InboxContext<' in inbox:
idx = inbox.find('InboxContext<')
section = inbox[idx:inbox.find('}', idx)+1] if idx != -1 else inbox
print("\ninbox handler handles unsupported activities as ACCEPTED:")
print('StatusCode::ACCEPTED.into_response()' in section or 'return Ok(StatusCode::ACCEPTED.into_response())' in section)
print('from_value(value).map_err(|_| StatusCode::BAD_REQUEST)? in Some("Undo") branch:')
print(re.search(r'Some\("Undo"\) => \{(?P<body>.*?)\n(\})', inbox, re.S).group('body') if re.search(r'Some\("Undo"\) => \{(?P<body>.*?)\n(\})', inbox, re.S) else "not found")
print("\nPotential references to other Undo object types in tests/docs:")
for path in ['crates/feder-runtime-server/tests/cases/inbox.rs', 'crates/feder-runtime-server/src/inbox.rs']:
try:
txt = open(path, encoding='utf-8').read()
for needle in ['UndoLike', 'UndoLike"', 'Like"', '"type": "Like"', 'type": "Like"']:
if needle in txt:
print(f"{path}: contains {needle!r}")
except Exception as e:
print(f"{path}: error {e}")
PYRepository: fedify-dev/feder
Length of output: 481
Don’t return 400 BAD_REQUEST for non-Follow Undo activities.
crates/feder-runtime-server/src/inbox.rs deserializes every Undo as Undo<Follow> before checking the object type, so Undo(Like), Undo(Announce), and Undo(Accept) are mapped to BAD_REQUEST while other unsupported activity types return 202. This makes ActivityPub delivery for peers sending non-Follow undos fail instead of being ignoreable, likely causing retries. Inspect object.type before deserializing object as Follow, or return 202 when the undo object is not a Follow.
🤖 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 `@crates/feder-vocab/src/lib.rs` around lines 426 - 436, The Undo handling in
the inbox currently deserializes every undo object as Reference<Follow>, causing
non-Follow undos to return BAD_REQUEST. Update the undo processing in the inbox
flow to inspect object.type before deserializing as Follow, and return the
existing 202 response for Undo objects such as Like, Announce, or Accept while
preserving Follow handling.
Expand follower recipients first and track their actor IDs so directly addressed followers are not also sent to their personal inboxes. Assisted-by: Codex:gpt-5.6-sol
Assisted-by: Codex:GPT-5.6-sol
1 participant
Summary
Build out Feder's server runtime into a functional single-user ActivityPub federation proof of concept. The example project is a microblog called Federog. I follow fedify's microblog tutorial as a guideline.
This adds signed inbox (draft-cavage-http-signature way) and outbound delivery, follower management, persisted Notes, recipient-aware delivery, and ActivityPub object-serving safeguards while keeping protocol decisions in
feder-coreand platform concerns in the runtime.Closes #26
Changes
timeouts, and mismatched actor IDs.
Followand embeddedUndo(Follow)activities.Acceptactivities.to ∪ cc.404for non-public Notes instead of exposing them anonymously.Architecture
feder-coreemits storage and delivery actions without performing network or database operations. The Axum/Tokio runtime resolves those actions using SQLite, remote actor discovery, HTTP signatures(draft-cavage-http-signature), and outbound delivery.AI assistance is disclosed through the 'Assisted-by` trailers on the relevant commits. Changes section's contents are summarised by Codex:gpt-5.6-sol.
Summary by CodeRabbit