-
Notifications
You must be signed in to change notification settings - Fork 12
Register webhook and script in create PR command #3998
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -187,7 +187,24 @@ export default class CreateListingPRCommand extends HostBaseCommand< | |||||
|
|
||||||
| log.debug('PR created successfully:', prResult); | ||||||
|
|
||||||
| // Open room and send PR status message with full details | ||||||
| // Register webhook for PR status updates | ||||||
| const webhookData = await this.registerPRWebhook(prResult.prNumber); | ||||||
|
|
||||||
| if (webhookData) { | ||||||
| manifest.webhook = { | ||||||
| id: webhookData.id, | ||||||
| path: webhookData.path, | ||||||
| signingSecret: webhookData.signingSecret, | ||||||
| }; | ||||||
|
|
||||||
| log.debug('Webhook registered for PR:', { | ||||||
| webhookUrl: `${this.realmServer.url.href}_webhooks/${webhookData.path}`, | ||||||
| signingSecret: webhookData.signingSecret, | ||||||
| prNumber: prResult.prNumber, | ||||||
| }); | ||||||
| } | ||||||
|
|
||||||
| // Open room and send PR status message | ||||||
| await new UseAiAssistantCommand(this.commandContext).execute({ | ||||||
| roomId, | ||||||
| prompt: `I just submitted a PR for my listing "${listing.name ?? listing.id}". | ||||||
|
|
@@ -208,6 +225,47 @@ PR Details: | |||||
| return await this.makeResult(manifest); | ||||||
| } | ||||||
|
|
||||||
| private async registerPRWebhook( | ||||||
| prNumber: number, | ||||||
| ): Promise<{ id: string; path: string; signingSecret: string } | null> { | ||||||
| try { | ||||||
| const webhook = await this.realmServer.createIncomingWebhook({ | ||||||
| verificationType: 'HMAC_SHA256_HEADER', | ||||||
| verificationConfig: { | ||||||
| header: 'x-hub-signature-256', | ||||||
|
||||||
| header: 'x-hub-signature-256', | |
| header: 'X-Hub-Signature-256', |
Copilot
AI
Feb 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The registerPRWebhook method always creates a new webhook and webhook command without checking if one already exists for this PR. This could lead to duplicate webhooks being created if the PR creation is retried or if the command is called multiple times. Consider implementing idempotent webhook registration similar to the approach in register-github-webhook.ts (see ensureIncomingWebhook and ensureWebhookCommand functions) to prevent duplicate webhooks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The debug log currently includes
signingSecret, which is the credential used to authenticate webhook payloads. In any environment where debug logging is enabled (staging, incident debugging, or local logs shipped to a shared sink), this leaks enough information to forge validX-Hub-Signature-256headers and spoof GitHub webhook events for the registered path.Useful? React with 👍 / 👎.