Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

webhook request validation doc update #739

Merged
merged 1 commit into from
Aug 27, 2024
Merged

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Aug 27, 2024

🚀 This description was created by Ellipsis for commit 1365662

Summary:

Updated docs/running-tasks/webhooks-faq.mdx with Python and JavaScript examples for webhook request validation using SKYVERN_API_KEY.

Key points:

  • Updated docs/running-tasks/webhooks-faq.mdx to include code examples for webhook request validation.
  • Added Python example using hmac and fastapi to validate request headers.
  • Added JavaScript example using crypto module to validate request headers.
  • Both examples demonstrate signature verification using SKYVERN_API_KEY.

Generated with ❤️ by ellipsis.dev

@wintonzheng wintonzheng merged commit 32fe08b into main Aug 27, 2024
2 checks passed
@wintonzheng wintonzheng deleted the shu/update_webhook_faq branch August 27, 2024 01:07
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 1365662 in 11 seconds

More details
  • Looked at 46 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. docs/running-tasks/webhooks-faq.mdx:31
  • Draft comment:
    Ensure req.body is a Buffer or string before using it. Consider using middleware to parse the body appropriately.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is speculative and suggests a verification step, which violates the rule against asking the PR author to confirm or ensure something. It does not point out a definite issue with the code. The code assumes req.body is a Buffer or string, and the comment does not indicate a clear code change required.
    The comment might be useful if there's a known issue with req.body not being a Buffer or string, but this is not indicated in the comment. It could be seen as a proactive suggestion rather than a necessary change.
    The rules clearly state not to make speculative comments or ask for verification. The comment does not indicate a definite issue, so it should be removed.
    The comment should be removed as it is speculative and does not indicate a definite issue with the code.

Workflow ID: wflow_7E1NDdfNR2CpFDBz


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

payload = request.body() # this is a bytes
hash_obj = hmac.new(SKYVERN_API_KEY.encode("utf-8"), msg=payload, digestmod=hashlib.sha256)
header_skyvern_signature = request.headers["x-skyvern-signature"]
payload = request.body() # this is a bytes
Copy link
Contributor

Choose a reason for hiding this comment

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

request.body() should be awaited in FastAPI to get the payload. Use payload = await request.body() if inside an async function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant