Skip to content

fix(signature): validate signing_secret is a non-empty string#1884

Open
WilliamBergamin wants to merge 1 commit into
mainfrom
empty-signing-secret
Open

fix(signature): validate signing_secret is a non-empty string#1884
WilliamBergamin wants to merge 1 commit into
mainfrom
empty-signing-secret

Conversation

@WilliamBergamin
Copy link
Copy Markdown
Contributor

@WilliamBergamin WilliamBergamin commented Jun 2, 2026

Summary

Adds input validation to SignatureVerifier.signing_secret to fail fast with a clear error when an empty or non-string value is provided. Previously, passing "", None, or a non-string type would silently produce incorrect HMAC signatures, making debugging difficult for users who misconfigured their app.

  • Introduces a property getter/setter on signing_secret that raises
    ValueError for non-string or blank values
  • Validation applies both at construction time and on reassignment

Note: this is a similar fix as slackapi/bolt-js#2946

Testing

CI tests should be sufficient

Category

  • slack_sdk.web.WebClient (sync/async) (Web API client)
  • slack_sdk.webhook.WebhookClient (sync/async) (Incoming Webhook, response_url sender)
  • slack_sdk.socket_mode (Socket Mode client)
  • slack_sdk.signature (Request Signature Verifier)
  • slack_sdk.oauth (OAuth Flow Utilities)
  • slack_sdk.models (UI component builders)
  • slack_sdk.scim (SCIM API client)
  • slack_sdk.audit_logs (Audit Logs API client)
  • slack_sdk.rtm_v2 (RTM client)
  • /docs (Documents)
  • /tutorial (PythOnBoardingBot tutorial)
  • tests/integration_tests (Automated tests for this library)

Requirements

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh after making the changes.

@WilliamBergamin WilliamBergamin added this to the 3.43.0 milestone Jun 2, 2026
@WilliamBergamin WilliamBergamin requested review from srtaalej and zimeg June 2, 2026 18:45
@WilliamBergamin WilliamBergamin self-assigned this Jun 2, 2026
@WilliamBergamin WilliamBergamin requested a review from a team as a code owner June 2, 2026 18:45
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.14%. Comparing base (be8be5e) to head (73fefdb).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1884      +/-   ##
==========================================
+ Coverage   84.09%   84.14%   +0.04%     
==========================================
  Files         117      117              
  Lines       13346    13356      +10     
==========================================
+ Hits        11223    11238      +15     
+ Misses       2123     2118       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Contributor

@srtaalej srtaalej left a comment

Choose a reason for hiding this comment

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

LGTM! ⭐

Copy link
Copy Markdown
Member

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

@WilliamBergamin LGTM! Tested with a bolt app to find a new error 👾 ✨

ValueError: signing_secret must not be empty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants