Code Review

This pull request introduces a robust mechanism for handling unverified inbound activities by adding the Federatable.onUnverifiedActivity() hook. This allows applications to define custom logic for activities that fail signature verification, which is particularly useful for handling Delete activities from actors whose keys are no longer accessible. The changes are well-supported by a more detailed signature verification API (verifyRequestDetailed) that provides structured failure reasons, and enhanced observability through new OpenTelemetry attributes. The implementation is clean, well-tested across multiple packages, and the documentation has been updated accordingly. Overall, this is an excellent enhancement to the library's flexibility and debugging capabilities.

_{Note: Security Review did not run due to the size of the PR.}

Code Review

This pull request introduces a new onUnverifiedActivity() handler for inspecting inbound activities that fail signature verification, allowing for custom response handling instead of the default 401 Unauthorized. The implementation also adds verifyRequestDetailed() to provide more granular information about signature verification failures, such as unsigned requests or key fetch errors. These details are also exposed through new OpenTelemetry attributes for improved tracing. The associated documentation and tests have been updated to cover these new features. The changes are logically sound and consistently applied throughout the codebase.

_{Note: Security Review did not run due to the size of the PR.}

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 192e8cbca2

Pull request overview

This PR addresses #472 by enabling Fedify applications to inspect inbound activities whose HTTP signatures could not be verified. Instead of always returning 401 Unauthorized, applications can now register a callback to handle unverified activities (e.g., accepting Delete activities from actors whose keys return 410 Gone).

Changes:

• Added InboxListenerSetters.onUnverifiedActivity() with supporting types UnverifiedActivityHandler and UnverifiedActivityReason, and wired the callback through the federation builder, middleware, and inbox handler so unverified activities can be intercepted before the default 401 response.
• Introduced verifyRequestDetailed() and fetchKeyDetailed() with structured failure result types (VerifyRequestDetailedResult, VerifyRequestFailureReason, FetchKeyErrorResult, FetchKeyDetailedResult) to distinguish unsigned requests, invalid signatures, and key-fetch failures, and extended FetchError in @fedify/vocab-runtime with an optional response field.
• Added OpenTelemetry attributes for HTTP signature failure reasons and key-fetch details, updated FedifySpanExporter and documentation accordingly.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 342e5d0521

Code Review

This pull request introduces a valuable feature for handling unverified inbound activities by adding the onUnverifiedActivity handler and providing detailed signature verification results. The changes are well-implemented across the signature verification, request handling, and OpenTelemetry exporting layers. The related documentation and tests have been updated thoroughly to reflect these new capabilities.

_{Note: Security Review did not run due to the size of the PR.}

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.

Code Review

This pull request introduces a robust mechanism for handling unverified inbound activities, a valuable addition for dealing with real-world federation scenarios like deleted actors. The changes are well-structured, introducing onUnverifiedActivity() for custom handling, verifyRequestDetailed() for granular failure reasons, and extending FetchError to include the original HTTP response. The implementation is consistent across the codebase, with corresponding updates to tests, documentation, and OpenTelemetry instrumentation. The refactoring in the signature verification and key fetching logic is clean and effectively isolates the new functionality. Overall, this is a high-quality contribution that enhances Fedify's error handling and observability.

_{Note: Security Review did not run due to the size of the PR.}

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 991f18e02a

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.

Code Review

This pull request introduces a valuable feature for handling unverified inbound activities by adding the onUnverifiedActivity hook. This provides applications with more granular control over federation behavior, especially in real-world scenarios like handling deleted actors. The introduction of verifyRequestDetailed and the propagation of detailed failure reasons, including HTTP status codes from key fetches, significantly improve the observability and debugging capabilities of the library. The changes are well-structured, thoroughly tested, and clearly documented.

_{Note: Security Review did not run due to the size of the PR.}

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.