Skip to content

chore(cdp): normalize content-type handling in webhook responses#53814

Open
Piccirello wants to merge 2 commits intomasterfrom
tom/hog-functions
Open

chore(cdp): normalize content-type handling in webhook responses#53814
Piccirello wants to merge 2 commits intomasterfrom
tom/hog-functions

Conversation

@Piccirello
Copy link
Copy Markdown
Member

Consolidate content-type resolution for webhook responses behind a shared helper and add standard response headers.

Consolidate content-type resolution for webhook responses behind a
shared helper and add standard response headers.
@assign-reviewers-posthog assign-reviewers-posthog bot requested a review from a team April 9, 2026 03:35
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 9, 2026

Vulnerabilities

  • XSS via content-type injection is addressed by the new sanitizeContentType allowlist and the X-Content-Type-Options: nosniff / Content-Security-Policy: default-src 'none' headers. The allowlist correctly excludes text/html, image/svg+xml, application/javascript, and similar scriptable types.
  • No secrets exposure, SQL injection, or auth boundary issues identified in the changed code.

Comments Outside Diff (1)

  1. nodejs/src/cdp/cdp-api.ts, line 622-645 (link)

    P2 processAndRespondToWebhook duplicates getCustomHttpResponse extraction logic

    The PR exports getCustomHttpResponse as a shared helper and tests it directly, but processAndRespondToWebhook still re-implements the same typeof result.execResult === 'object' && ... && 'httpResponse' in result.execResult check instead of calling it. This violates OnceAndOnlyOnce and means the two extraction paths could diverge.

    This also requires importing getCustomHttpResponse in cdp-api.ts.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: nodejs/src/cdp/cdp-api.ts
    Line: 622-645
    
    Comment:
    **`processAndRespondToWebhook` duplicates `getCustomHttpResponse` extraction logic**
    
    The PR exports `getCustomHttpResponse` as a shared helper and tests it directly, but `processAndRespondToWebhook` still re-implements the same `typeof result.execResult === 'object' && ... && 'httpResponse' in result.execResult` check instead of calling it. This violates OnceAndOnlyOnce and means the two extraction paths could diverge.
    
    
    
    This also requires importing `getCustomHttpResponse` in `cdp-api.ts`.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: nodejs/src/cdp/cdp-api.ts
Line: 622-645

Comment:
**`processAndRespondToWebhook` duplicates `getCustomHttpResponse` extraction logic**

The PR exports `getCustomHttpResponse` as a shared helper and tests it directly, but `processAndRespondToWebhook` still re-implements the same `typeof result.execResult === 'object' && ... && 'httpResponse' in result.execResult` check instead of calling it. This violates OnceAndOnlyOnce and means the two extraction paths could diverge.

```suggestion
    private async processAndRespondToWebhook(
        webhookId: string,
        req: ModifiedRequest,
        res: express.Response,
        onSuccess: (
            result: Awaited<ReturnType<typeof this.cdpSourceWebhooksConsumer.processWebhook>>
        ) => Promise<any> | any
    ): Promise<any> {
        try {
            const result = await this.cdpSourceWebhooksConsumer.processWebhook(webhookId, req)

            const httpResponse = getCustomHttpResponse(result)
            if (httpResponse) {
                // Security headers to prevent XSS via content-type injection
                res.set('X-Content-Type-Options', 'nosniff')
                res.set('Content-Security-Policy', "default-src 'none'")

                if (typeof httpResponse.body === 'string') {
                    const safeContentType = sanitizeContentType(
                        httpResponse.contentType,
                        httpResponse.isBase64Encoded ? 'application/octet-stream' : 'text/plain'
                    )
                    if (httpResponse.isBase64Encoded) {
                        const buffer = Buffer.from(httpResponse.body, 'base64')
                        return res.status(httpResponse.status).type(safeContentType).send(buffer)
                    }
                    return res.status(httpResponse.status).type(safeContentType).send(httpResponse.body)
                } else if (typeof httpResponse.body === 'object') {
                    return res.status(httpResponse.status).json(httpResponse.body)
                }
                return res.status(httpResponse.status).send('')
            }

            return await onSuccess(result)
```

This also requires importing `getCustomHttpResponse` in `cdp-api.ts`.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "chore(cdp): normalize content-type handl..." | Re-trigger Greptile

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