Skip to content

fix(security): migrate webhook auth to HMAC-SHA256 signature header#970

Merged
zmanian merged 1 commit intonearai:stagingfrom
zmanian:security/pr513-webhook-hmac-rebased
Mar 12, 2026
Merged

fix(security): migrate webhook auth to HMAC-SHA256 signature header#970
zmanian merged 1 commit intonearai:stagingfrom
zmanian:security/pr513-webhook-hmac-rebased

Conversation

@zmanian
Copy link
Copy Markdown
Collaborator

@zmanian zmanian commented Mar 11, 2026

Summary

  • migrate HTTP webhook authentication from body secret to X-IronClaw-Signature HMAC-SHA256 verification
  • preserve backward-compatible body-secret fallback with a deprecation warning
  • keep current staging attachment handling and hot secret reload support while adding content-type and header-encoding validation

Why this replacement PR exists

The original approved PR #513 is on a branch I cannot update directly from this checkout, so this branch cleanly reapplies the approved change on top of current staging.

Validation

  • cargo test channels::http::tests::
  • local result: 17 passed, 0 failed

@github-actions github-actions Bot added size: L 200-499 changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Mar 11, 2026
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly improves the security posture of the HTTP webhook channel by upgrading its authentication mechanism. Instead of relying on a secret embedded directly in the request body, it now leverages HMAC-SHA256 signatures provided in a dedicated header. This change enhances the integrity and authenticity of incoming webhook requests, making the system more robust against tampering. A temporary fallback for the old authentication method is included to ease migration, alongside new input validations for request headers and content.

Highlights

  • Enhanced Webhook Authentication: Migrated HTTP webhook authentication from using a secret in the request body to a more secure HMAC-SHA256 signature verification via the X-IronClaw-Signature header.
  • Backward Compatibility with Deprecation: Maintained backward compatibility for the old body-secret authentication method, but it now issues a deprecation warning and will be removed in a future release.
  • Input Validation: Introduced validation for Content-Type header (must be application/json) and X-IronClaw-Signature header encoding to prevent malformed requests.
  • PR Replacement: This pull request is a clean reapplication of the approved changes from the original PR fix(security): migrate webhook auth to HMAC-SHA256 signature header #513, which could not be updated directly.
Changelog
  • .env.example
    • Added detailed documentation and an example for using the new X-IronClaw-Signature header for webhook authentication.
    • Added a deprecation notice for the HTTP_WEBHOOK_SECRET when used in the request body.
  • src/channels/http.rs
    • Added necessary bytes, hmac, sha2, and HeaderMap imports for HMAC-SHA256 signature verification.
    • Defined HmacSha256 type alias for convenience.
    • Updated the WebhookRequest struct to mark the secret field as deprecated.
    • Implemented a new verify_hmac_signature function to validate HMAC-SHA256 signatures against the request body and a provided header.
    • Modified the webhook_handler to accept raw HeaderMap and Bytes for the request body, enabling header-based authentication.
    • Introduced Content-Type validation, requiring application/json.
    • Prioritized authentication via X-IronClaw-Signature header; if present and valid, the request proceeds.
    • Implemented fallback logic to check for the deprecated secret field in the request body if no valid signature header is found, issuing a warning upon use.
    • Refactored the core message processing logic into a new process_authenticated_request function.
    • Updated all error responses to use .into_response() for consistency.
    • Added a compute_signature helper function for testing HMAC signatures.
    • Introduced new test cases for valid and invalid HMAC-SHA256 signatures, malformed signatures, and precedence over body secrets.
    • Renamed existing test cases for clarity to reflect the new authentication methods (e.g., webhook_correct_secret_returns_ok to webhook_deprecated_body_secret_still_works).
    • Added tests for invalid JSON payloads, non-JSON content types, and invalid signature header encoding.
    • Included dedicated unit tests for the verify_hmac_signature function covering various scenarios.
Activity
  • The author, zmanian, created this pull request as a replacement for an earlier approved PR (fix(security): migrate webhook auth to HMAC-SHA256 signature header #513) because the original branch could not be directly updated.
  • Validation was performed by running cargo test channels::http::tests::, which reported 17 tests passed and 0 failed, indicating successful implementation and testing of the new authentication logic.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@zmanian zmanian enabled auto-merge (squash) March 11, 2026 23:31
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request is a solid security enhancement, migrating webhook authentication from a simple secret in the request body to a more robust HMAC-SHA256 signature in the headers. The implementation correctly maintains backward compatibility and includes a comprehensive set of tests for the new logic. My review includes a couple of suggestions to improve code clarity and maintainability.

Comment thread src/channels/http.rs
Comment on lines +215 to +218
let mut mac = match HmacSha256::new_from_slice(secret.as_bytes()) {
Ok(mac) => mac,
Err(_) => return false,
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The HmacSha256::new_from_slice method for Hmac from the hmac crate is guaranteed not to fail (it always returns Ok). Therefore, this match statement contains an unreachable Err arm. You can simplify this to a single line using .unwrap() to make the code more concise and signal that this operation is considered infallible. A panic here would indicate a breaking change in the dependency, which is a condition you'd want to fail fast on.

    let mut mac = HmacSha256::new_from_slice(secret.as_bytes()).unwrap();

Comment thread src/channels/http.rs
Comment on lines +279 to +287
return (
StatusCode::UNAUTHORIZED,
Json(WebhookResponse {
message_id: Uuid::nil(),
status: "error".to_string(),
response: Some("Invalid webhook signature".to_string()),
}),
)
.into_response();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This pattern of creating a Json(WebhookResponse { ... }) for error responses is repeated multiple times in this function for different error conditions (e.g., rate limiting, invalid content type, various auth failures). To improve readability and reduce code duplication, consider extracting this into a helper function. For example, a function like fn unauthorized_response(message: &str) -> axum::response::Response could encapsulate this logic, making the main handler flow easier to follow.

References
  1. This comment aligns with the guideline to refactor duplicated code into a shared function to improve readability and reduce redundancy.

@zmanian zmanian merged commit 195ff44 into nearai:staging Mar 12, 2026
9 checks passed
@ironclaw-ci ironclaw-ci Bot mentioned this pull request Mar 12, 2026
bkutasi pushed a commit to bkutasi/ironclaw that referenced this pull request Mar 28, 2026
drchirag1991 pushed a commit to drchirag1991/ironclaw that referenced this pull request Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules size: L 200-499 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants