Promote the experimental architecture to main - #44
Conversation
Assisted-by: Codex-gpt-5.6-sol
Add new minimal core and runtime architecture Add actor endpoint with generics Add negotiation features Assisted-by: Codex:gpt-5.6-sol
Assisted-by: Codex:gpt-5.6-sol
Port RSA actor key generation and persisted PEM validation into ref-feder-core. Redact private key material from debug output and keep actor dispatch as a minimal capability called directly by the server runtime. Assisted-by: Codex:gpt-5.6-sol
Parse and validate local acct resources, resolve actors through the actor dispatcher, and return JRD discovery responses from the experimental server router. Assisted-by: Codex:gpt-5.6-sol
Introduce a stateless receive_follow transition in ref-feder-core. Validate that the Follow actor matches the resolved remote actor and that its object targets the selected local actor, then return the follower facts and Accept delivery through a transient FollowOutcome without retaining protocol state. Extend the portable key module with SHA-256 digest generation and draft-Cavage RSA-SHA256 signing and verification primitives. Add a personal inbox endpoint to ref-feder-runtime-server. Validate ActivityPub content types, body size, request host and date, body digests, signature headers, remote key ownership, and actor identity before invoking the core Follow transition. Persist the follower before sending the generated Accept and return 202 Accepted for successfully handled or irrelevant activities. Represent local actor access and runtime services through FederServer<A, S>, while placing Arc only at the Axum router boundary for concurrent request sharing. Keep signed authentication as the default and temporarily retain an explicit insecure development policy until the reference example can issue signed Follow requests. Update the reference server example with bounded adapters for remote resolution, follower storage, and Accept sending. Document and exercise the actor, WebFinger, and personal inbox endpoints without accumulating an in-memory activity history. Shared inbox and Undo handling remain out of scope for this change. Assisted-by: Codex:gpt-5.6-sol
Define ServerStorage as the application-owned persistence boundary for follower relationships and per-actor signing keys. Allow runtimes to persist Follow outcomes and retrieve the appropriate ActorKeyPair without retaining protocol state inside the core. Assisted-by: Codex:gpt-5.6-sol
Port protected remote actor and key resolution to the reference server runtime. Reject private and special-use destinations by default, disable redirects and proxies, and enforce request timeouts and response size limits. Give FederServer concrete resolver and activity sender components. Verify signed inbox requests, persist Follow relationships through ServerStorage, load per-actor signing keys, and deliver signed Accept activities. Migrate the reference actor-server example to ServerStorage and the fallible server constructor. Use the bundled test key pair and a local recipient inbox to demonstrate signed Accept delivery without generating keys at startup. Assisted-by: Codex:gpt-5.6-sol
Reject the deprecated fec0::/10 site-local range when outbound networking uses the PublicOnly policy, closing an SSRF path through literal URLs and DNS resolution. Assisted-by: Codex:gpt-5.6-sol
Extend ServerStorage with an idempotent follower-removal operation for the upcoming Undo Follow transition. Update the reference actor-server storage adapter to remove its retained follower only when both sides of the relationship match. Assisted-by: Codex:gpt-5.6-sol
Validate that an Undo actor owns its embedded Follow and that the Follow targets the local actor. Return a transient follower-removal outcome without retaining protocol state or performing storage operations inside core. Assisted-by: Codex:gpt-5.6-sol
Dispatch Undo activities through the pure core transition and remove validated follower relationships through ServerStorage. Update the reference actor server example with idempotent follower removal and a documented Follow-to-Undo flow. Assisted-by: Codex:gpt-5.6-sol
Add canonical actor ID lookup to ActorDispatcher so shared inbox activities can be routed without assuming an application URL structure. Route Follow and Undo Follow activities from the shared inbox through the existing authentication, core transition, storage, and delivery flow. Update the reference actor server to advertise and demonstrate the shared inbox endpoint. Assisted-by: Codex:gpt-5.6-sol
Add a follower-listing capability to ServerStorage and expose each local actor's followers as an ActivityStreams OrderedCollection. Advertise the collection from the example actor and demonstrate how Follow and Undo requests update its contents. Assisted-by: Codex:gpt-5.6-sol
Add a pure core operation that constructs an outbound Follow activity and its transient pending relationship. Extend ServerStorage with pending Follow persistence and add FederServer::follow_actor to resolve the remote actor, persist intent, load the local signing key, and deliver the signed activity. Allow applications to retain shared FederServer state alongside the router, and update the reference example with bounded pending storage and an end-to-end outbound Follow demonstration. Assisted-by: Codex:gpt-5.6-sol
Add a pure core transition that validates inbound Accept activities against the authenticated remote actor and the stored pending Follow relationship. Extend ServerStorage with pending Follow lookup and compare-and-set confirmation capabilities. Route linked and embedded Accept Follow activities through personal and shared inboxes, and update the reference example with a bounded pending-to-accepted state transition. Assisted-by: Codex:gpt-5.6-sol
Add a pure core operation that constructs a local Note and its corresponding Create activity from runtime-provided IDs, addressing, and content. Introduce a dedicated NoteStore capability and add FederServer::create_note to load the local actor and persist only the durable Note without retaining protocol history. Update the reference example with bounded Note storage and a local creation flow while leaving Create delivery for the next migration step. Assisted-by: Codex:gpt-5.6-sol
Derive transient delivery intents from Note addressing in core without retaining recipient or activity history. Add follower delivery and shared storage error capabilities, then expand followers, resolve direct actors, deduplicate inboxes, and deliver signed Create activities after Note persistence. Update the reference example to demonstrate signed Create delivery to a stored follower. Assisted-by: Codex:gpt-5.6-sol
Extend NoteStore with typed Note loading and add a pure core check for public ActivityStreams addressing. Expose canonical local post paths through the reference server runtime with ActivityPub content negotiation while hiding missing and non-public Notes. Update the reference example to load and serve its bounded persisted Note. Assisted-by: Codex:gpt-5.6-sol
Require FederServer users to provide the authoritative WebFinger handle host and validate acct resources against it. Stop trusting the client-controlled Host header when deciding whether an account belongs to the local deployment, and update the reference actor server with its configured handle host. Assisted-by: Codex:gpt-5.6-sol
Preserve the inbox endpoint selected during request routing and use its authority when verifying signed requests. Personal inbox requests continue to use the actor inbox, while shared inbox requests use the actor's advertised sharedInbox endpoint. Treat requests for actors without a shared inbox as accepted no-ops. Assisted-by: Codex:gpt-5.6-sol
Restore the http-signatures feature boundary around actor keys, digest generation, and draft-Cavage signing and verification. Keep RSA, Base64, and zeroization out of the portable core dependency tree unless explicitly requested. Enable the feature from the server runtime and reference actor example. Assisted-by: Codex:gpt-5.6-sol
Replace complete stored Actor values in follower fan-out with the minimal addressing facts required for delivery: actor ID, inbox, and optional shared inbox. Update Note delivery and the reference example to use the compact target, keeping the storage contract compatible with the existing SQLite follower schema and avoiding unnecessary actor retention. Assisted-by: Codex:gpt-5.6-sol
Provide file-backed and in-memory SQLite storage implementing the new server, Note, and follower-delivery storage capabilities. Preserve the existing followers, actor keys, and objects schema while adding persistent outbound Follow state. Synchronize connection access for shared Axum state and use an immediate transaction for exact pending-Follow confirmation. Add focused coverage for follower delivery facts, Notes, pending Follow transitions, actor keys, and Send and Sync compatibility. Assisted-by: Codex:gpt-5.6-sol
Add a SQLite-backed load-or-generate operation for actor signing identities. Reuse existing actor keys without invoking the random number generator. When a key is absent, generate it outside the database lock and insert it without replacing an identity established by a concurrent provisioner. Add focused coverage that verifies initial provisioning and stable reuse. Assisted-by: Codex:gpt-5.6-sol
Replace the original RuntimeConfig-based example with the new FederServer, actor dispatcher, and built-in SQLite storage adapter. Provision the actor signing key through SQLite, publish its public key on the actor document, and preserve the signing identity across server restarts. Allow the database path to be configured through FEDER_DATABASE. Update the example documentation with actor and WebFinger requests. Assisted-by: Codex:gpt-5.6-sol
Move protocol transition tests into ref-feder-core/tests, covering Follow, Accept, Undo, Note construction, actor keys, digests, and HTTP signatures. Move the SQLite adapter tests out of the implementation module and into the ref-feder-runtime-server integration test suite. Add runtime coverage for actor and object endpoints, WebFinger, follower collections, actor resolution, outbound delivery, signed personal and shared inboxes, outbound Follow, incoming Undo, and Note persistence and delivery. Keep test keys and shared test infrastructure local to each crate so the reference crates can be tested independently. Assisted-by: Codex:gpt-5.6-sol
Replace the previous stateful feder-core implementation with the portable protocol transition functions and capability traits developed in the reference crate. Publish the standard operating system runtime as feder-server and update its dependencies, imports, tests, and documentation to use the final feder-core package. Remove the superseded feder-runtime-server, temporary ref-* crates, and the redundant custom-storage example. Retain the SQLite single-user server as a non-publishable end-to-end example. Restrict release version stamping to publishable workspace packages and update the project documentation to describe the stateless core and server runtime boundaries. Assisted-by: Codex:gpt-5.6-sol
Refactor architecture
Return 202 Accepted for Accept and Undo activities wrapping unsupported activity types before attempting Follow-specific deserialization. Assisted-by: Codex:gpt-5.6-sol
📝 WalkthroughWalkthroughThe project replaces the stateful ChangesStateless federation architecture
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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: 7
🧹 Nitpick comments (13)
crates/feder-server/src/inbox.rs (4)
103-119: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLog the silent 202 branches in
shared_inbox.Three separate conditions return
202 Acceptedand drop the activity: no routable target, no local actor for the target, and no advertisedsharedInboxendpoint. The third condition indicates a server misconfiguration, not a remote error. Without a log line, an operator cannot distinguish a dropped activity from a processed one.Add a debug or warning log for each branch before returning.
🤖 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-server/src/inbox.rs` around lines 103 - 119, Add a debug or warning log immediately before each 202 Accepted return in shared_inbox: when shared_inbox_target yields no routable target, when get_actor_by_id finds no local actor, and when the actor has no advertised shared_inbox endpoint. Include enough context to distinguish the three dropped-activity cases, treating the missing endpoint as a server configuration issue.
51-59: 🔒 Security & Privacy | 🔵 TrivialTrack removal of
AllowUnsignedInsecureDev.The variant disables all inbox authentication and is part of the public API. The default is
RequireSigned, so the exposure needs an explicit opt-in. The FIXME records the plan to remove the variant after the reference example signs its Follow requests.Do you want me to open an issue to track the removal and the example update?
🤖 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-server/src/inbox.rs` around lines 51 - 59, Track the planned removal of the public InboxAuthPolicy::AllowUnsignedInsecureDev variant and the corresponding reference-example update by opening an issue. Record that the example must send signed Follow requests before removing the variant, while preserving RequireSigned as the default until then.
469-474: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse a core helper instead of duplicating
follow_reference_id.This function repeats the private
follow_reference_idincrates/feder-core/src/follow.rslines 132-137. The same duplication exists for theReference<Actor>variant. Exporting one small accessor fromfeder-core, for example an inherentid()onReference<T>whereThas an id, removes both copies and keeps the two crates aligned ifReferencegains a variant.🤖 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-server/src/inbox.rs` around lines 469 - 474, Remove the local follow_reference_id helper in inbox.rs and reuse a shared accessor from feder-core for both Reference<Follow> and Reference<Actor>. Expose an inherent id() method on Reference<T> in the core implementation, constrained to referenced types with an id, then update callers to use it so both crates share one variant-aware implementation.
315-391: 🔒 Security & Privacy | 🔵 TrivialConsider a replay guard for signed inbox requests.
The date window allows a captured request to be replayed for up to 65 minutes. The digest binds the body, so a replay repeats the identical activity. Follow, Undo, and Accept handling is idempotent today, so the current impact is low. If the inbox later handles non-idempotent activities, a short-lived cache of seen signature values, or of activity IDs, prevents duplicate processing.
🤖 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-server/src/inbox.rs` around lines 315 - 391, Extend verify_signed_request with a short-lived replay guard keyed by each accepted signature value (or activity ID), rejecting entries already seen within the allowed date window while preserving normal validation and idempotent handling. Ensure the guard is concurrency-safe, bounded/expiring, and only records requests after signature verification succeeds.crates/feder-server/tests/cases/inbox.rs (1)
336-363: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the Accept confirmation path.
The suite covers Follow, Undo, and unsupported nested activities. It does not cover a valid Accept that confirms a pending outbound Follow. That path in
crates/feder-server/src/inbox.rslines 187-231 loads the pending relationship, validates it throughreceive_accept_follow, and callsconfirm_pending_follow. It is the newest logic in the handler and has no server-level test.Add one test that stores a pending Follow, posts a signed Accept to the personal inbox, and asserts the relationship became confirmed. Add a second test that posts the same Accept to the shared inbox, to cover the
shared_inbox_targetlookup at lines 445-456.🤖 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-server/tests/cases/inbox.rs` around lines 336 - 363, Add server-level tests for the valid Accept confirmation flow: create and persist a pending outbound Follow, post its signed Accept to the recipient’s personal inbox, and assert the relationship is confirmed; add a second test posting the same Accept to the shared inbox to exercise shared_inbox_target lookup. Reuse existing Follow, signing, inbox, and relationship assertion helpers, anchoring the tests near ignores_accept_and_undo_of_unsupported_activities.crates/feder-core/tests/follow.rs (1)
113-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the remaining
AcceptFollowErrorvariants.The tests assert
WrongActorandWrongFollowonly.WrongLocalActor,WrongFollowActor, andWrongFollowObjectstay untested. Those branches guard cross-actor confirmation of a pending Follow, so a regression there would be silent.🧪 Suggested additional cases
#[test] fn rejects_accept_for_another_local_actor() { let local = actor("https://local.example/users/alice"); let remote = actor("https://remote.example/users/bob"); let pending = PendingFollow { local_actor: iri("https://local.example/users/carol"), remote_actor: remote.clone(), follow_activity: iri("https://local.example/activities/follow/1"), }; let accept = Accept::new( iri("https://remote.example/activities/accept/1"), Reference::id(remote.id.clone()), Reference::id(pending.follow_activity.clone()), ); assert_eq!( receive_accept_follow(&local, &remote, &pending, accept), Err(AcceptFollowError::WrongLocalActor) ); } #[test] fn rejects_accept_whose_embedded_follow_has_wrong_endpoints() { let local = actor("https://local.example/users/alice"); let remote = actor("https://remote.example/users/bob"); let other = actor("https://local.example/users/mallory"); let pending = PendingFollow { local_actor: local.id.clone(), remote_actor: remote.clone(), follow_activity: iri("https://local.example/activities/follow/1"), }; let follow = Follow::new( pending.follow_activity.clone(), Reference::id(other.id.clone()), Reference::id(remote.id.clone()), ); let accept = Accept::new( iri("https://remote.example/activities/accept/1"), Reference::id(remote.id.clone()), Reference::object(follow), ); assert_eq!( receive_accept_follow(&local, &remote, &pending, accept), Err(AcceptFollowError::WrongFollowActor) ); }🤖 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/tests/follow.rs` around lines 113 - 142, Extend the receive_accept_follow test coverage with cases for WrongLocalActor, WrongFollowActor, and WrongFollowObject. Add assertions using mismatched pending local actors and embedded Follow objects with incorrect actor and object endpoints, ensuring each case returns its corresponding AcceptFollowError variant.crates/feder-core/tests/undo.rs (1)
29-70: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a case for a mismatched embedded Follow actor.
The three cases cover
LinkedFollow, the outer-actorWrongActorbranch, andWrongObject. They do not cover the secondWrongActorbranch incrates/feder-core/src/undo.rsat lines 42-44, whereundo.actormatchesremote_actor.idbut the embeddedfollow.actordoes not.That branch carries the authorization guarantee. It stops a verified signer from undoing a Follow that another actor created.
crates/feder-server/src/inbox.rsmaps the result to 401.💚 Proposed additional case
+ let wrong_follow_actor = Undo::new( + iri("https://remote.example/activities/undo/4"), + Reference::id(remote.id.clone()), + Reference::object(Follow::new( + iri("https://remote.example/activities/follow/4"), + Reference::id(other.id.clone()), + Reference::id(local.id.clone()), + )), + ); + assert_eq!( receive_undo_follow(&local, &remote, linked), Err(UndoFollowError::LinkedFollow) ); + assert_eq!( + receive_undo_follow(&local, &remote, wrong_follow_actor), + Err(UndoFollowError::WrongActor) + );Change
Reference::id(other.id)at line 41 toReference::id(other.id.clone())sootherremains usable.🤖 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/tests/undo.rs` around lines 29 - 70, Add a test case in rejects_linked_follow_and_wrong_actor_or_object for an Undo whose outer actor matches remote.id but whose embedded Follow actor is other.id, and assert receive_undo_follow returns UndoFollowError::WrongActor. Clone other.id where needed so the existing wrong_actor fixture remains usable.crates/feder-server/src/lib.rs (1)
73-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicated constructor body.
newandwith_outbound_address_policybuild the same value. Only the policy differs. Letnewdelegate towith_outbound_address_policy. This keeps future field additions in one place.Also consider the naming.
with_inbox_auth_policyon Line 107 is a consuming builder method, butwith_outbound_address_policyis a constructor with a different shape. A name such aswith_configor a builder method that mirrorswith_inbox_auth_policywould make the public API consistent.♻️ Proposed change
pub fn new(actors: A, storage: S, handle_host: impl Into<String>) -> Result<Self, Error> { - let policy = OutboundAddressPolicy::PublicOnly; - let resolver = ActorResolver::new(policy)?; - let sender = ActivitySender::new(policy)?; - Ok(Self { - actors, - storage, - handle_host: handle_host.into(), - resolver, - sender, - inbox_auth_policy: InboxAuthPolicy::RequireSigned, - }) + Self::with_outbound_address_policy( + actors, + storage, + handle_host, + OutboundAddressPolicy::PublicOnly, + ) }🤖 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-server/src/lib.rs` around lines 73 - 104, Remove the duplicated construction logic by making new delegate to with_outbound_address_policy with OutboundAddressPolicy::PublicOnly, preserving the existing default behavior and centralizing field initialization. Rename with_outbound_address_policy to a configuration-oriented or consuming-builder name consistent with with_inbox_auth_policy, and update its callers accordingly.crates/feder-server/Cargo.toml (1)
12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid an exact version pin for
axumin a library crate.
= 0.8resolves to exactly0.8.0. Cargo excludes every later patch release, so the crate cannot receive0.8.xbug and security fixes. An exact pin in a published library also creates conflicts for downstream users who need a newer patch. Use a caret requirement instead.♻️ Proposed change
-axum = "=0.8" +axum = "0.8"🤖 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-server/Cargo.toml` at line 12, Update the axum dependency declaration in the crate manifest from an exact version requirement to a caret-compatible 0.8 requirement, allowing later 0.8.x patch releases while preserving the current minor-version constraint.crates/feder-server/src/follow.rs (1)
73-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGeneric error variants drop the underlying cause in both workflow error types.
FollowActorErrorandCreateNoteErrorcarry the dispatcher and storage errors by value without#[source], while the non-generic variants in the same enums use#[source]. Callers that walk the error chain lose the real reason for a dispatcher or storage failure.
crates/feder-server/src/follow.rs#L73-L83: add#[source]toActorDispatcher(A)andStorage(S), and addA: std::error::Error + 'staticandS: std::error::Error + 'staticbounds toFollowActorError.crates/feder-server/src/note.rs#L141-L148: apply the same#[source]attributes and generic bounds toActorDispatcher(A)andStorage(S)inCreateNoteError.🤖 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-server/src/follow.rs` around lines 73 - 83, Update FollowActorError in crates/feder-server/src/follow.rs lines 73-83 by marking ActorDispatcher(A) and Storage(S) as #[source] and adding A: std::error::Error + 'static and S: std::error::Error + 'static bounds. Apply the same #[source] attributes and generic bounds to ActorDispatcher(A) and Storage(S) in CreateNoteError at crates/feder-server/src/note.rs lines 141-148, preserving the existing non-generic error behavior.crates/feder-server/src/send.rs (1)
57-66: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueThe
Reference::Idbranch skips the key consistency check.The
Reference::Objectbranch verifies the owner and the PEM againstkey_pair. TheReference::Idbranch accepts the key id without any check. An id-only reference carries no PEM, so a local comparison is not possible. The result is thatActorKeyMismatchonly protects the embedded-key case.If the actor document is always expected to embed the key, reject
Reference::IdwithMissingActorKeyinstead. If both forms are supported, document the reduced guarantee at the method level.🤖 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-server/src/send.rs` around lines 57 - 66, Update the key selection logic around local_actor.public_key so Reference::Id cannot bypass key consistency validation. Prefer rejecting id-only references with SendError::MissingActorKey if embedded keys are required; otherwise document the reduced validation guarantee at the enclosing method and preserve support for both reference forms.crates/feder-server/src/note.rs (1)
69-86: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftDelivery failures after the first one are discarded without a trace.
The loop keeps only
first_send_error. Every later failure is dropped. There is no log statement. If delivery to 50 of 100 follower inboxes fails, the caller sees one error and cannot identify the affected inboxes.The loop is also sequential. Each inbox waits for the previous request, and
build_clientsets a request timeout. For a large follower collection the total time grows linearly with the follower count on the caller's task.Two suggestions for the operator-facing path:
- Log each delivery failure with the inbox, or return the full list of failures instead of one error.
- Consider bounded concurrency for the fan-out, for example a
FuturesUnorderedwith a concurrency limit, or hand the deliveries to a background queue so the caller does not block.🤖 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-server/src/note.rs` around lines 69 - 86, Update the inbox delivery loop around send_activity to record every delivery failure with its inbox through the existing logging facility, while preserving the first_send_error result behavior. Replace the sequential fan-out with bounded concurrency, such as a concurrency-limited FuturesUnordered flow, so large follower collections do not block linearly on the caller task. Preserve the existing ActorResolver error precedence when no delivery error occurs.crates/feder-server/tests/cases/operation.rs (1)
82-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert pending Follow persistence.
This test never loads
outbound_follows. It passes iffollow_actorsends the Follow without storing the pending relationship.Assert that
server.storage().load_pending_follow(&follow_id)returns the expected relationship. If delivery ordering is required, block the remote response and inspect storage after the handler receives the request but before it releases the response.🤖 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-server/tests/cases/operation.rs` around lines 82 - 100, Extend outbound_follow_is_persisted_before_signed_delivery to load the pending relationship with server.storage().load_pending_follow(&follow_id) after follow_actor completes, and assert it matches the expected local and remote actors. If persistence must be verified before delivery completes, hold the remote response, inspect storage after receiving the request, then release the response.
🤖 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-core/src/note.rs`:
- Around line 81-104: Add a shared public-address predicate that recognizes both
PUBLIC_COLLECTION and the compact "as:Public" alias, then use it in
is_public_note and note_recipients. Ensure alias recipients are skipped as
public addressing rather than reported as NoteRecipient::Actor targets.
In `@crates/feder-server/src/follow.rs`:
- Around line 56-60: Update follow_actor’s inbox selection to always use
remote_actor.inbox for the targeted Follow, removing the shared_inbox fallback
and any related endpoint preference. Preserve the existing delivery flow while
ensuring no follower endpoint recipients are used.
In `@crates/feder-server/src/inbox.rs`:
- Around line 226-229: Update the confirmation flow in the inbox handler around
confirm_pending_follow to bind its returned bool and detect false. Log an
appropriate diagnostic when the exact pending relationship was not confirmed,
while preserving the existing internal-server-error handling for returned
errors.
- Around line 601-611: The signature parsing and verification flow around
parse_signature_header and verify_signed_request must accept an omitted
algorithm and algorithm="hs2019" for RSA signatures. Treat either case as the
existing RSA-SHA256 verification path while preserving current handling for
explicit rsa-sha256 and rejecting unsupported algorithms.
In `@crates/feder-server/src/lib.rs`:
- Around line 87-93: Add rustdoc to the public constructor
with_outbound_address_policy describing that
OutboundAddressPolicy::AllowPrivateAddress disables SSRF protection and should
be limited to development or tests; replace or supplement the non-rustdoc marker
so published crate consumers see the warning. If the project’s feature structure
supports it, gate this constructor behind an insecure-dev cargo feature to
prevent access in production builds.
In `@crates/feder-server/src/storage/sqlite.rs`:
- Around line 260-264: Update the SQLite upsert for follow activities so a
conflict on follow_activity_id never changes an existing accepted relationship
back to pending. Replace the unconditional state update with DO NOTHING, or
condition updates so they apply only when the stored state is already pending,
while preserving the current insert behavior.
In `@examples/single-user-server/src/main.rs`:
- Around line 98-104: Update the example server construction using
FederServer::with_outbound_address_policy to use
OutboundAddressPolicy::PublicOnly and the default InboxAuthPolicy::RequireSigned
instead of permitting private addresses or unsigned insecure development
requests. Keep private-address access and unsigned local-federation behavior
available only through explicit opt-in configuration.
---
Nitpick comments:
In `@crates/feder-core/tests/follow.rs`:
- Around line 113-142: Extend the receive_accept_follow test coverage with cases
for WrongLocalActor, WrongFollowActor, and WrongFollowObject. Add assertions
using mismatched pending local actors and embedded Follow objects with incorrect
actor and object endpoints, ensuring each case returns its corresponding
AcceptFollowError variant.
In `@crates/feder-core/tests/undo.rs`:
- Around line 29-70: Add a test case in
rejects_linked_follow_and_wrong_actor_or_object for an Undo whose outer actor
matches remote.id but whose embedded Follow actor is other.id, and assert
receive_undo_follow returns UndoFollowError::WrongActor. Clone other.id where
needed so the existing wrong_actor fixture remains usable.
In `@crates/feder-server/Cargo.toml`:
- Line 12: Update the axum dependency declaration in the crate manifest from an
exact version requirement to a caret-compatible 0.8 requirement, allowing later
0.8.x patch releases while preserving the current minor-version constraint.
In `@crates/feder-server/src/follow.rs`:
- Around line 73-83: Update FollowActorError in
crates/feder-server/src/follow.rs lines 73-83 by marking ActorDispatcher(A) and
Storage(S) as #[source] and adding A: std::error::Error + 'static and S:
std::error::Error + 'static bounds. Apply the same #[source] attributes and
generic bounds to ActorDispatcher(A) and Storage(S) in CreateNoteError at
crates/feder-server/src/note.rs lines 141-148, preserving the existing
non-generic error behavior.
In `@crates/feder-server/src/inbox.rs`:
- Around line 103-119: Add a debug or warning log immediately before each 202
Accepted return in shared_inbox: when shared_inbox_target yields no routable
target, when get_actor_by_id finds no local actor, and when the actor has no
advertised shared_inbox endpoint. Include enough context to distinguish the
three dropped-activity cases, treating the missing endpoint as a server
configuration issue.
- Around line 51-59: Track the planned removal of the public
InboxAuthPolicy::AllowUnsignedInsecureDev variant and the corresponding
reference-example update by opening an issue. Record that the example must send
signed Follow requests before removing the variant, while preserving
RequireSigned as the default until then.
- Around line 469-474: Remove the local follow_reference_id helper in inbox.rs
and reuse a shared accessor from feder-core for both Reference<Follow> and
Reference<Actor>. Expose an inherent id() method on Reference<T> in the core
implementation, constrained to referenced types with an id, then update callers
to use it so both crates share one variant-aware implementation.
- Around line 315-391: Extend verify_signed_request with a short-lived replay
guard keyed by each accepted signature value (or activity ID), rejecting entries
already seen within the allowed date window while preserving normal validation
and idempotent handling. Ensure the guard is concurrency-safe, bounded/expiring,
and only records requests after signature verification succeeds.
In `@crates/feder-server/src/lib.rs`:
- Around line 73-104: Remove the duplicated construction logic by making new
delegate to with_outbound_address_policy with OutboundAddressPolicy::PublicOnly,
preserving the existing default behavior and centralizing field initialization.
Rename with_outbound_address_policy to a configuration-oriented or
consuming-builder name consistent with with_inbox_auth_policy, and update its
callers accordingly.
In `@crates/feder-server/src/note.rs`:
- Around line 69-86: Update the inbox delivery loop around send_activity to
record every delivery failure with its inbox through the existing logging
facility, while preserving the first_send_error result behavior. Replace the
sequential fan-out with bounded concurrency, such as a concurrency-limited
FuturesUnordered flow, so large follower collections do not block linearly on
the caller task. Preserve the existing ActorResolver error precedence when no
delivery error occurs.
In `@crates/feder-server/src/send.rs`:
- Around line 57-66: Update the key selection logic around
local_actor.public_key so Reference::Id cannot bypass key consistency
validation. Prefer rejecting id-only references with SendError::MissingActorKey
if embedded keys are required; otherwise document the reduced validation
guarantee at the enclosing method and preserve support for both reference forms.
In `@crates/feder-server/tests/cases/inbox.rs`:
- Around line 336-363: Add server-level tests for the valid Accept confirmation
flow: create and persist a pending outbound Follow, post its signed Accept to
the recipient’s personal inbox, and assert the relationship is confirmed; add a
second test posting the same Accept to the shared inbox to exercise
shared_inbox_target lookup. Reuse existing Follow, signing, inbox, and
relationship assertion helpers, anchoring the tests near
ignores_accept_and_undo_of_unsupported_activities.
In `@crates/feder-server/tests/cases/operation.rs`:
- Around line 82-100: Extend outbound_follow_is_persisted_before_signed_delivery
to load the pending relationship with
server.storage().load_pending_follow(&follow_id) after follow_actor completes,
and assert it matches the expected local and remote actors. If persistence must
be verified before delivery completes, hold the remote response, inspect storage
after receiving the request, then release the response.
🪄 Autofix
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: 3ed53852-d143-46bd-b143-33f6adea91b9
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockcrates/feder-server/tests/fixtures/rsa-private-key.pemis excluded by!**/*.pemcrates/feder-server/tests/fixtures/rsa-public-key.pemis excluded by!**/*.pem
📒 Files selected for processing (63)
CONTRIBUTING.mdCargo.tomlREADME.mdcrates/feder-core/Cargo.tomlcrates/feder-core/src/follow.rscrates/feder-core/src/key.rscrates/feder-core/src/lib.rscrates/feder-core/src/note.rscrates/feder-core/src/storage.rscrates/feder-core/src/undo.rscrates/feder-core/tests/common/mod.rscrates/feder-core/tests/follow.rscrates/feder-core/tests/key.rscrates/feder-core/tests/note.rscrates/feder-core/tests/undo.rscrates/feder-runtime-server/README.mdcrates/feder-runtime-server/src/app.rscrates/feder-runtime-server/src/config.rscrates/feder-runtime-server/src/error.rscrates/feder-runtime-server/src/inbox.rscrates/feder-runtime-server/src/lib.rscrates/feder-runtime-server/src/operation.rscrates/feder-runtime-server/src/storage/mod.rscrates/feder-runtime-server/src/storage/sqlite.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-server/Cargo.tomlcrates/feder-server/src/actor.rscrates/feder-server/src/config.rscrates/feder-server/src/follow.rscrates/feder-server/src/followers.rscrates/feder-server/src/inbox.rscrates/feder-server/src/lib.rscrates/feder-server/src/negotiation.rscrates/feder-server/src/note.rscrates/feder-server/src/object.rscrates/feder-server/src/send.rscrates/feder-server/src/storage/mod.rscrates/feder-server/src/storage/sqlite.rscrates/feder-server/src/url.rscrates/feder-server/src/webfinger.rscrates/feder-server/tests/cases/actor.rscrates/feder-server/tests/cases/followers.rscrates/feder-server/tests/cases/inbox.rscrates/feder-server/tests/cases/object.rscrates/feder-server/tests/cases/operation.rscrates/feder-server/tests/cases/send.rscrates/feder-server/tests/cases/webfinger.rscrates/feder-server/tests/common/mod.rscrates/feder-server/tests/runtime.rscrates/feder-server/tests/storage.rscspell.jsonexamples/single-user-server/Cargo.tomlexamples/single-user-server/README.mdexamples/single-user-server/src/main.rsmise.toml
💤 Files with no reviewable changes (19)
- crates/feder-runtime-server/tests/cases/webfinger.rs
- crates/feder-runtime-server/src/lib.rs
- crates/feder-runtime-server/tests/cases/actor.rs
- crates/feder-runtime-server/src/inbox.rs
- crates/feder-server/src/negotiation.rs
- crates/feder-runtime-server/tests/cases/operation.rs
- crates/feder-runtime-server/tests/cases/inbox.rs
- crates/feder-runtime-server/src/app.rs
- crates/feder-runtime-server/README.md
- crates/feder-runtime-server/tests/cases/send.rs
- crates/feder-runtime-server/src/operation.rs
- crates/feder-runtime-server/src/config.rs
- crates/feder-runtime-server/src/error.rs
- crates/feder-runtime-server/src/storage/mod.rs
- crates/feder-runtime-server/tests/cases/object.rs
- crates/feder-runtime-server/tests/cases/followers.rs
- crates/feder-runtime-server/src/storage/sqlite.rs
- crates/feder-runtime-server/tests/cases/app.rs
- crates/feder-runtime-server/tests/common/mod.rs
| #[must_use] | ||
| pub fn is_public_note(note: &Note) -> bool { | ||
| note.to | ||
| .iter() | ||
| .chain(note.cc.iter()) | ||
| .any(|recipient| recipient.as_str() == PUBLIC_COLLECTION) | ||
| } | ||
|
|
||
| fn note_recipients(local_actor: &Actor, note: &Note) -> Vec<NoteRecipient> { | ||
| let mut recipients = Vec::new(); | ||
|
|
||
| for address in note.to.iter().chain(note.cc.iter()) { | ||
| let recipient = if address.as_str() == PUBLIC_COLLECTION || address == &local_actor.id { | ||
| continue; | ||
| } else if local_actor.followers.as_ref() == Some(address) { | ||
| NoteRecipient::Followers(local_actor.id.clone()) | ||
| } else { | ||
| NoteRecipient::Actor(address.clone()) | ||
| }; | ||
|
|
||
| if !recipients.contains(&recipient) { | ||
| recipients.push(recipient); | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Recognize the as:Public alias for public addressing.
is_public_note and note_recipients compare addresses only against the full IRI https://www.w3.org/ns/activitystreams#Public. ActivityStreams also defines the compact alias as:Public, which parses as a valid IRI and can appear in to or cc.
Two effects follow. is_public_note reports false, so crates/feder-server/src/object.rs returns 404 for a note the application addressed publicly. note_recipients also emits NoteRecipient::Actor("as:Public"), which the server then passes to the actor resolver as a delivery target.
Add a shared predicate so both functions treat the alias as public.
🐛 Proposed fix to recognize the public alias
+const PUBLIC_COLLECTION_ALIAS: &str = "as:Public";
+
+fn is_public_address(address: &Iri) -> bool {
+ matches!(address.as_str(), PUBLIC_COLLECTION | PUBLIC_COLLECTION_ALIAS)
+}
+
#[must_use]
pub fn is_public_note(note: &Note) -> bool {
note.to
.iter()
.chain(note.cc.iter())
- .any(|recipient| recipient.as_str() == PUBLIC_COLLECTION)
+ .any(is_public_address)
}
fn note_recipients(local_actor: &Actor, note: &Note) -> Vec<NoteRecipient> {
let mut recipients = Vec::new();
for address in note.to.iter().chain(note.cc.iter()) {
- let recipient = if address.as_str() == PUBLIC_COLLECTION || address == &local_actor.id {
+ let recipient = if is_public_address(address) || address == &local_actor.id {
continue;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[must_use] | |
| pub fn is_public_note(note: &Note) -> bool { | |
| note.to | |
| .iter() | |
| .chain(note.cc.iter()) | |
| .any(|recipient| recipient.as_str() == PUBLIC_COLLECTION) | |
| } | |
| fn note_recipients(local_actor: &Actor, note: &Note) -> Vec<NoteRecipient> { | |
| let mut recipients = Vec::new(); | |
| for address in note.to.iter().chain(note.cc.iter()) { | |
| let recipient = if address.as_str() == PUBLIC_COLLECTION || address == &local_actor.id { | |
| continue; | |
| } else if local_actor.followers.as_ref() == Some(address) { | |
| NoteRecipient::Followers(local_actor.id.clone()) | |
| } else { | |
| NoteRecipient::Actor(address.clone()) | |
| }; | |
| if !recipients.contains(&recipient) { | |
| recipients.push(recipient); | |
| } | |
| } | |
| const PUBLIC_COLLECTION_ALIAS: &str = "as:Public"; | |
| fn is_public_address(address: &Iri) -> bool { | |
| matches!(address.as_str(), PUBLIC_COLLECTION | PUBLIC_COLLECTION_ALIAS) | |
| } | |
| #[must_use] | |
| pub fn is_public_note(note: &Note) -> bool { | |
| note.to | |
| .iter() | |
| .chain(note.cc.iter()) | |
| .any(is_public_address) | |
| } | |
| fn note_recipients(local_actor: &Actor, note: &Note) -> Vec<NoteRecipient> { | |
| let mut recipients = Vec::new(); | |
| for address in note.to.iter().chain(note.cc.iter()) { | |
| let recipient = if is_public_address(address) || address == &local_actor.id { | |
| continue; | |
| } else if local_actor.followers.as_ref() == Some(address) { | |
| NoteRecipient::Followers(local_actor.id.clone()) | |
| } else { | |
| NoteRecipient::Actor(address.clone()) | |
| }; | |
| if !recipients.contains(&recipient) { | |
| recipients.push(recipient); | |
| } | |
| } |
🤖 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/note.rs` around lines 81 - 104, Add a shared
public-address predicate that recognizes both PUBLIC_COLLECTION and the compact
"as:Public" alias, then use it in is_public_note and note_recipients. Ensure
alias recipients are skipped as public addressing rather than reported as
NoteRecipient::Actor targets.
| let inbox = remote_actor | ||
| .endpoints | ||
| .as_ref() | ||
| .and_then(|endpoints| endpoints.shared_inbox.as_ref()) | ||
| .unwrap_or(&remote_actor.inbox); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
ActivityPub sharedInbox only for public or followers addressed activities, personal inbox for directly addressed Follow
💡 Result:
The ActivityPub specification defines the sharedInbox as an optional endpoint designed for efficient, wide delivery of activities to multiple users on a single server [1][2]. Its intended use cases are limited to specific scenarios rather than direct, personal messaging [1][2]. SharedInbox Usage The sharedInbox is primarily used for two categories of activities: 1. Publicly addressed activities (those addressed to the Public special collection) [1][2]. 2. Activities sent to an actor's followers [1][2]. When an originating server sends these types of activities, it may choose to deliver a single copy of the activity to a recipient server's sharedInbox endpoint, rather than delivering individual copies to the personal inboxes of every follower on that server [1][2]. The receiving server is then responsible for distributing that activity internally to the appropriate users [1][2]. Personal Inbox for Direct Addressing Activities that are directly addressed to a specific actor (such as a direct message or a targeted notification) should be delivered to that actor's personal inbox endpoint [3][4]. Sending directly addressed activities to a sharedInbox is generally considered improper, as the sharedInbox is intended for optimized, wide-scale delivery (broadcasting to many) rather than point-to-point delivery [5][6]. Key Technical Considerations - Actor Model vs. Efficiency: The sharedInbox is an optimization that deviates from the pure Actor model of ActivityPub, as it shifts the responsibility of target identification from the sender to the receiver [5][6]. - Addressing Requirements: Even when using a sharedInbox for efficiency, the origin server remains obligated to deliver the activity to any intended recipients (specified in to, cc, etc.) who do not share that sharedInbox or would otherwise not receive the message [1][2]. - Privacy and Filtering: Because the sharedInbox is a shared endpoint, receiving servers must implement internal routing to ensure that private or followers-only content is only distributed to the authorized recipients [3][6]. The specification warns that sharedInbox endpoints should not be used to bypass these privacy constraints [3]. In practice, the sharedInbox is often criticized for its implementation complexity and the potential to act as a firehose, leading many developers to suggest that it should be reserved strictly for its intended optimizations—public posts and follower-only distribution—while personal interactions must remain directed to individual actor inboxes [7][6].
Citations:
- 1: https://www.w3.org/TR/activitypub/
- 2: https://w3c.github.io/activitypub/
- 3: sharedInbox / siteInbox type endpoint (publicInbox, but not just for public posts) w3c/activitypub#242
- 4: Mastodon delivers activities to inboxes with no addressing mastodon/mastodon#19150
- 5: Support delivery to shared inboxes go-fed/activity#27
- 6: https://socialhub.activitypub.rocks/t/federated-timeline-view/3082
- 7: Delivery of public content to sharedInbox w3c/activitypub#484
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg 'crates/feder-server/src/(follow|note)\.rs|crates/feder-server/src/.+\.rs' | sed -n '1,120p'
echo
echo "follow outline:"
ast-grep outline crates/feder-server/src/follow.rs --view mini || true
echo
echo "follow.rs lines 1-140:"
cat -n crates/feder-server/src/follow.rs | sed -n '1,140p'
echo
echo "note relevant lines:"
cat -n crates/feder-server/src/note.rs | sed -n '80,125p'
echo
echo "Endpoint/shared_inbox definitions/usages:"
rg -n "struct .*Endpoint|shared_inbox|sharedInbox|followers|Follow|followers_url" crates/feder-server src crates -g '*.rs' | sed -n '1,220p'Repository: fedify-dev/feder
Length of output: 26601
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral/static check: identify all ActivityPub delivery targets generated by Follow actor creation
# and whether the shared inbox is selected through Endpoint priority.
python3 - <<'PY'
from pathlib import Path
p = Path('crates/feder-server/src/follow.rs')
text = p.read_text()
for needle in [
"remote_actor.endpoints.as_ref().and_then(|endpoints| endpoints.shared_inbox.as_ref()).unwrap_or(&remote_actor.inbox)",
"follow_request.addresses.push(remote_actor.inbox.clone())",
"follow_request.to.push(remote_actor.id.clone())",
]:
print(f"contains {needle!r}: {needle in text}")
print("\nfollow.rs occurrences:")
for i,line in enumerate(text.splitlines(),1):
if 'endpoints' in line or 'shared_inbox' in line or 'follow_request.addresses' in line or 'follow_request.to' in line:
print(f"{i}: {line}")
PYRepository: fedify-dev/feder
Length of output: 553
Deliver the Follow to the remote actor’s personal inbox.
follow_actor() prefers endpoints.shared_inbox for every Follow, and no recipients are collected from follower endpoints. ActivityPub shared inbox use is for public/follower-addressed activities; a targeted Follow should be sent to the actor’s personal inbox.
🐛 Proposed fix
- let inbox = remote_actor
- .endpoints
- .as_ref()
- .and_then(|endpoints| endpoints.shared_inbox.as_ref())
- .unwrap_or(&remote_actor.inbox);
+ let inbox = &remote_actor.inbox;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let inbox = remote_actor | |
| .endpoints | |
| .as_ref() | |
| .and_then(|endpoints| endpoints.shared_inbox.as_ref()) | |
| .unwrap_or(&remote_actor.inbox); | |
| let inbox = &remote_actor.inbox; |
🤖 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-server/src/follow.rs` around lines 56 - 60, Update
follow_actor’s inbox selection to always use remote_actor.inbox for the targeted
Follow, removing the shared_inbox fallback and any related endpoint preference.
Preserve the existing delivery flow while ensuring no follower endpoint
recipients are used.
| server | ||
| .storage() | ||
| .confirm_pending_follow(&pending) | ||
| .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Use the bool returned by confirm_pending_follow.
confirm_pending_follow returns false when the exact relationship is no longer pending. That happens if a duplicate Accept, or a concurrent Undo, changed the row between load_pending_follow at line 208 and this call. The code discards the value, so a lost confirmation produces the same 202 Accepted as a successful one and leaves no trace.
Bind the result and log the false case.
♻️ Proposed change
- server
- .storage()
- .confirm_pending_follow(&pending)
- .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
+ let confirmed = server
+ .storage()
+ .confirm_pending_follow(&pending)
+ .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
+ if !confirmed {
+ tracing::debug!(
+ follow_activity = %pending.follow_activity,
+ "Accept did not confirm a pending Follow; the relationship changed concurrently"
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| server | |
| .storage() | |
| .confirm_pending_follow(&pending) | |
| .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; | |
| let confirmed = server | |
| .storage() | |
| .confirm_pending_follow(&pending) | |
| .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; | |
| if !confirmed { | |
| tracing::debug!( | |
| follow_activity = %pending.follow_activity, | |
| "Accept did not confirm a pending Follow; the relationship changed concurrently" | |
| ); | |
| } |
🤖 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-server/src/inbox.rs` around lines 226 - 229, Update the
confirmation flow in the inbox handler around confirm_pending_follow to bind its
returned bool and detect false. Log an appropriate diagnostic when the exact
pending relationship was not confirmed, while preserving the existing
internal-server-error handling for returned errors.
| let key_id = parameters.remove("keyid")?; | ||
| let algorithm = parameters.remove("algorithm")?; | ||
| let signed_headers = parameters | ||
| .remove("headers")? | ||
| .split_ascii_whitespace() | ||
| .map(str::to_ascii_lowercase) | ||
| .collect::<Vec<_>>(); | ||
| let signature = parameters.remove("signature")?; | ||
| if key_id.is_empty() || signed_headers.is_empty() || signature.is_empty() { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
HTTP signatures draft-cavage hs2019 algorithm parameter deprecated ActivityPub Mastodon
💡 Result:
The hs2019 algorithm parameter, associated with the draft-cavage-http-signatures specification, is a deprecated, functionally vestigial placeholder that is not part of the finalized HTTP Message Signatures standard, RFC 9421 [1][2][3]. In the original draft-cavage-http-signatures (often referred to as cavage-12), hs2019 was introduced as a generic identifier that deferred the actual algorithm choice to metadata associated with the keyId [1][4]. When the IETF work on HTTP Message Signatures evolved, this approach was superseded. The final standard, RFC 9421, removed the need for such a placeholder because the new specification allows for more flexible, explicit algorithm negotiation that defaults to context-specific behavior, rendering hs2019 obsolete [3][5]. Regarding ActivityPub and Mastodon: 1. Status in Mastodon: Mastodon historically relied on draft-cavage-http-signatures for its security model [6][7]. While Mastodon has included support for hs2019 (often treating it as a fallback or equating it to rsa-sha256) to maintain compatibility with older implementations, it remains tethered to the cavage-12 draft [1][8][9]. Mastodon has been working on implementing support for RFC 9421 (the modern standard), but as of 2026, it maintains cavage-12 for broad compatibility with the wider Fediverse, which has been slow to migrate [10][11]. 2. Fediverse Ecosystem: cavage-12 remains the most widely supported version of HTTP Signatures across the Fediverse [1]. Because many projects have not yet implemented RFC 9421, the hs2019 placeholder is still frequently encountered in the wild [1][8]. However, it is explicitly not recommended for new implementations, which should aim to support the standardized RFC 9421 [1][2]. In summary, hs2019 is a legacy artifact from the drafting phase of HTTP signatures [3]. While it persists in the Fediverse due to the ubiquity of cavage-12 support, it has no place in a modern, standards-compliant implementation [1][2].
Citations:
- 1: https://swicg.github.io/activitypub-http-signature/
- 2: https://datatracker.ietf.org/doc/rfc9421/
- 3: Signatures: hs2019 algorithm gone from spec v3 httpwg/http-extensions#1468
- 4: https://datatracker.ietf.org/doc/html/draft-cavage-http-signatures
- 5: http://httpwg.org/wg-materials/interim-21-06/signatures.pdf
- 6: https://mastodon.gitbook.io/mastodon/spec/security
- 7: Support latest HTTP Signature draft mastodon/mastodon#17004
- 8: https://fedidevs.org/reference/signatures/
- 9: https://github.com/mastodon/mastodon/blob/ead13d48/app/lib/signed_request.rb
- 10: Update HTTP signatures to support IETF draft from October 2021 mastodon/mastodon#21429
- 11: Add support for outgoing HTTP Message Signatures mastodon/mastodon#39756
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching inbox/signature:"
fd -a 'inbox\.rs$' . || true
echo
echo "Relevant function outlines:"
ast-grep outline crates/feder-server/src/inbox.rs --match parse_signature_header --view expanded || true
ast-grep outline crates/feder-server/src/inbox.rs --match verify_signed_request --view expanded || true
echo
echo "Line counts:"
wc -l crates/feder-server/src/inbox.rs
echo
echo "Relevant source slices:"
sed -n '270,350p' crates/feder-server/src/inbox.rs | cat -n
sed -n '540,650p' crates/feder-server/src/inbox.rs | cat -n
echo
echo "Search for algorithm handling in repository:"
rg -n "algorithm|rsa-sha256|hs2019|created|expires|signature" crates/feder-server/src/inbox.rs crates -g '*.rs' | head -n 200Repository: fedify-dev/feder
Length of output: 14975
🌐 Web query:
draft-cavage-http-signatures Signature Input String keyid headers algorithm signature optional parameters
💡 Result:
The draft-cavage-http-signatures specification (now superseded by RFC 9421) defined a method for signing HTTP messages by constructing a canonicalized "signing string" and including parameters in the Signature header [1][2]. Key Signature Parameters: keyId (Required): A string identifier for the key material that the verifier uses to validate the signature [3][4]. signature (Required): The base 64 encoded digital signature [1]. It is created by signing the canonicalized "signing string" using the algorithm specified and the key identified by keyId [1][5]. algorithm (Recommended): Specifies the signature string construction mechanism and the cryptographic algorithm (e.g., rsa-sha256) [1][3]. Implementers are encouraged to derive the specific algorithm from the key metadata associated with the keyId rather than relying solely on this field [1][4]. headers (Optional): A lowercased, quoted list of HTTP header fields to be included in the signature [1][6]. If omitted, implementations act as if it were set to (created) [1][4]. The order of headers in this list is critical because it dictates the concatenation order used in the signing string construction [1][6]. Additional Optional Parameters: created: A Unix timestamp integer indicating when the signature was created [6]. expires: A Unix timestamp integer indicating when the signature ceases to be valid [6]. It is important to note that this draft has been replaced by RFC 9421, which introduced the Signature-Input header to separate signature metadata from the signature bytes themselves [2][7][8].
Citations:
- 1: https://datatracker.ietf.org/doc/html/draft-cavage-http-signatures
- 2: https://datatracker.ietf.org/doc/draft-ietf-httpbis-message-signatures/11/
- 3: https://datatracker.ietf.org/doc/html/draft-cavage-http-signatures-11
- 4: https://www.ietf.org/archive/id/draft-cavage-http-signatures-12.txt
- 5: https://datatracker.ietf.org/doc/draft-cavage-http-signatures/11/
- 6: https://www.potaroo.net/ietf/idref/draft-cavage-http-signatures/
- 7: https://datatracker.ietf.org/doc/html/rfc9421/
- 8: https://www.rfc-editor.org/rfc/rfc9421.txt
🌐 Web query:
draft-cavage-http-signatures hs2019 algorithm parameter generic placeholder required optional
💡 Result:
In the context of the draft-cavage-http-signatures specification (specifically later versions like -11 and -12), the hs2019 algorithm identifier was introduced as an active, recommended mechanism [1][2][3]. Its primary purpose was to indicate that the digital signature algorithm should be derived from the key metadata associated with the keyId, rather than being explicitly defined by the algorithm parameter itself [1][4][5]. Regarding your query on the algorithm parameter and generic placeholder status: 1. Required vs. Optional: In the draft-cavage-http-signatures specification, the algorithm parameter is generally used to specify the signature string construction mechanism [1][3]. While the specification notes it as RECOMMENDED, the introduction of hs2019 was specifically designed to handle cases where the actual cryptographic algorithm is determined by external key metadata [1][5]. 2. Generic Placeholder: hs2019 functioned as a "pseudo-algorithm" or placeholder [6][7]. By using hs2019, an implementation signaled that it would ignore a specific hardcoded algorithm in the signature header and instead determine the correct signing algorithm (such as RSASSA-PSS, ECDSA, or Ed25519) from the associated key metadata [1][4][8]. This was an intentional design to avoid the security risks associated with previous implementations that relied solely on the algorithm parameter for determining the signature type [1][2]. 3. Evolution and Current Status: The draft-cavage-http-signatures work was eventually superseded by the IETF's HTTP Message Signatures work (published as RFC 9421) [9]. In the successor specification, the explicit requirement for an algorithm parameter was removed, making the behavior previously offered by hs2019 (deriving the algorithm from context or key metadata) the new default standard [6]. Consequently, hs2019 was removed as an explicit identifier in later versions of the formal HTTP Message Signatures specification [6].
Citations:
- 1: https://datatracker.ietf.org/doc/html/draft-cavage-http-signatures
- 2: https://datatracker.ietf.org/doc/draft-cavage-http-signatures/11/
- 3: https://datatracker.ietf.org/doc/html/draft-cavage-http-signatures-11
- 4: https://www.ietf.org/archive/id/draft-cavage-http-signatures-12.txt
- 5: https://www.potaroo.net/ietf/idref/draft-cavage-http-signatures/
- 6: Signatures: hs2019 algorithm gone from spec v3 httpwg/http-extensions#1468
- 7: http://httpwg.org/wg-materials/interim-21-06/signatures.pdf
- 8: https://www.ietf.org/archive/id/draft-ietf-httpbis-message-signatures-01.txt
- 9: https://datatracker.ietf.org/doc/draft-ietf-httpbis-message-signatures/19/
Accept missing or hs2019 in algorithm.
verify_signed_request requires algorithm="rsa-sha256" at line 327, and parse_signature_header rejects a missing parameter at line 602. Later draft-cavage makes algorithm recommended, deprecates concrete algorithm tokens, and uses hs2019 to indicate the key determines the algorithm. Accept a missing algorithm as unspecified, or map algorithm="hs2019" to the same RSA-SHA256 verification already used for "rsa-sha256", so valid RSA signatures are not rejected.
🤖 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-server/src/inbox.rs` around lines 601 - 611, The signature
parsing and verification flow around parse_signature_header and
verify_signed_request must accept an omitted algorithm and algorithm="hs2019"
for RSA signatures. Treat either case as the existing RSA-SHA256 verification
path while preserving current handling for explicit rsa-sha256 and rejecting
unsupported algorithms.
| // for development | ||
| pub fn with_outbound_address_policy( | ||
| actors: A, | ||
| storage: S, | ||
| handle_host: impl Into<String>, | ||
| policy: OutboundAddressPolicy, | ||
| ) -> Result<Self, Error> { |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Document the security effect of this public constructor.
with_outbound_address_policy accepts OutboundAddressPolicy::AllowPrivateAddress, which disables the SSRF protection described in config.rs Line 22. The only marker is the bare comment // for development. That comment does not appear in rustdoc, so consumers of the published crate see an unmarked constructor next to new.
Add a doc comment that states the risk. If the constructor is intended only for development and tests, also consider placing it behind a cargo feature such as insecure-dev so production builds cannot reach it.
🛡️ Proposed change
- // for development
+ /// Creates a server with an explicit outbound address policy.
+ ///
+ /// # Security
+ ///
+ /// [`OutboundAddressPolicy::AllowPrivateAddress`] disables SSRF
+ /// protection for outbound requests. Use it only in development and
+ /// tests. Use [`FederServer::new`] in production.
pub fn with_outbound_address_policy(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // for development | |
| pub fn with_outbound_address_policy( | |
| actors: A, | |
| storage: S, | |
| handle_host: impl Into<String>, | |
| policy: OutboundAddressPolicy, | |
| ) -> Result<Self, Error> { | |
| /// Creates a server with an explicit outbound address policy. | |
| /// | |
| /// # Security | |
| /// | |
| /// [`OutboundAddressPolicy::AllowPrivateAddress`] disables SSRF | |
| /// protection for outbound requests. Use it only in development and | |
| /// tests. Use [`FederServer::new`] in production. | |
| pub fn with_outbound_address_policy( | |
| actors: A, | |
| storage: S, | |
| handle_host: impl Into<String>, | |
| policy: OutboundAddressPolicy, | |
| ) -> Result<Self, Error> { |
🤖 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-server/src/lib.rs` around lines 87 - 93, Add rustdoc to the
public constructor with_outbound_address_policy describing that
OutboundAddressPolicy::AllowPrivateAddress disables SSRF protection and should
be limited to development or tests; replace or supplement the non-rustdoc marker
so published crate consumers see the warning. If the project’s feature structure
supports it, gate this constructor behind an insecure-dev cargo feature to
prevent access in production builds.
| ON CONFLICT(follow_activity_id) DO UPDATE SET | ||
| local_actor_id = excluded.local_actor_id, | ||
| remote_actor_json = excluded.remote_actor_json, | ||
| state = 'pending' | ||
| "#, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not reset an accepted Follow to pending.
A retry with the same follow_activity_id enters this conflict branch. Line 263 changes an already accepted relationship back to pending. This permits later Accept processing to treat the old Follow as pending again.
Preserve an existing record on conflict. Use DO NOTHING, or update fields only when the stored state is already pending.
Proposed fix
- ON CONFLICT(follow_activity_id) DO UPDATE SET
- local_actor_id = excluded.local_actor_id,
- remote_actor_json = excluded.remote_actor_json,
- state = 'pending'
+ ON CONFLICT(follow_activity_id) DO NOTHING📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ON CONFLICT(follow_activity_id) DO UPDATE SET | |
| local_actor_id = excluded.local_actor_id, | |
| remote_actor_json = excluded.remote_actor_json, | |
| state = 'pending' | |
| "#, | |
| ON CONFLICT(follow_activity_id) DO NOTHING | |
| "#, |
🤖 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-server/src/storage/sqlite.rs` around lines 260 - 264, Update the
SQLite upsert for follow activities so a conflict on follow_activity_id never
changes an existing accepted relationship back to pending. Replace the
unconditional state update with DO NOTHING, or condition updates so they apply
only when the stored state is already pending, while preserving the current
insert behavior.
| let server = FederServer::with_outbound_address_policy( | ||
| dispatcher, | ||
| storage, | ||
| HANDLE_HOST, | ||
| OutboundAddressPolicy::AllowPrivateAddress, | ||
| )? | ||
| .with_inbox_auth_policy(InboxAuthPolicy::AllowUnsignedInsecureDev); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 \
'AllowPrivateAddress|AllowUnsignedInsecureDev|PublicOnly|RequireSigned' \
examples/single-user-server/src/main.rs \
crates/feder-server/src/config.rs \
crates/feder-server/src/inbox.rsRepository: fedify-dev/feder
Length of output: 2562
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== single-user-server main outline =="
ast-grep outline examples/single-user-server/src/main.rs || true
echo "== relevant single-user-server main =="
cat -n examples/single-user-server/src/main.rs | sed -n '1,140p'
echo "== config.rs =="
cat -n crates/feder-server/src/config.rs | sed -n '1,220p'
echo "== inbox auth policy usage =="
cat -n crates/feder-server/src/inbox.rs | sed -n '150,210p'
echo "== outbound address policy usages =="
rg -n -C 4 'outbound_address_policy|OutboundAddressPolicy|verify_signed_request|allow_unsigned' crates/feder-server examples -g '*.rs'Repository: fedify-dev/feder
Length of output: 29055
Use secure defaults for the example server.
examples/single-user-server/src/main.rs:102-104 disables SSRF protection and HTTP signature verification. An unsigned Follow with a caller-controlled actor IRI can then be resolved through private network addresses when the default policy accepts unsigned local-federation activities.
Use OutboundAddressPolicy::PublicOnly, apply the default InboxAuthPolicy::RequireSigned, and make private-address local-federation settings an explicit opt-in.
🤖 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 `@examples/single-user-server/src/main.rs` around lines 98 - 104, Update the
example server construction using FederServer::with_outbound_address_policy to
use OutboundAddressPolicy::PublicOnly and the default
InboxAuthPolicy::RequireSigned instead of permitting private addresses or
unsigned insecure development requests. Keep private-address access and unsigned
local-federation behavior available only through explicit opt-in configuration.
1 participant
Summary
Promote the architecture developed on
feat/experiment/architectureto themain development line.
feder-coreimplementation with portable,stateless protocol transition functions and capability traits.
feder-runtime-serverwith the publicfeder-serverruntime.SQLite persistence, followers, Follow/Accept/Undo handling, and Note delivery.
202 Acceptedfor unsupported nested Accept and Undo activities.Compatibility
This intentionally replaces the previous public core and runtime APIs.
Consumers of
feder-runtime-servermust migrate tofeder-server.The existing SQLite follower, signing-key, and object tables remain compatible.
The new runtime adds storage for outbound Follow relationships.
The legacy
/healthzroute is intentionally not carried forward.Migration notes
This change replaces the experimental legacy API rather than preserving
backward compatibility.
feder-runtime-serverdependencies and imports withfeder-server.FederCoreworkflow with stateless protocol functionsand runtime-provided capability traits.
FederServer, anActorDispatcher,and storage implementations.
A new table stores outbound Follow relationships.
/healthzendpoint is not included.Validation
mise run checkmise run testmainThis PR is assisted by Codex: gpt-sol-5.6.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes