Skip to content

Comments

Fix: webhook's s3's fileUrl link when when using custom S3 endpoint#2446

Open
EduBalbino wants to merge 3 commits intowppconnect-team:mainfrom
EduBalbino:fix/endpoint_s3_compatible
Open

Fix: webhook's s3's fileUrl link when when using custom S3 endpoint#2446
EduBalbino wants to merge 3 commits intowppconnect-team:mainfrom
EduBalbino:fix/endpoint_s3_compatible

Conversation

@EduBalbino
Copy link

Webhook always sent a hardcoded aws S3, even when a custom endpoint was set aws_s3.endpoint (e.g., for MinIO, DigitalOcean Spaces). The fileUrl now correctly uses the custom endpoint (if it exists) in forced path-style format (more compatible with s3 endpoints) instead of the hardcoded AWS S3 URL.

Thanks!

When aws_s3.endpoint is set (e.g., for MinIO, DigitalOcean Spaces),
the fileUrl now correctly uses the custom endpoint in path-style format
instead of the hardcoded AWS S3 URL.

Closes issue where webhook received incorrect S3 links pointing to AWS
when using S3-compatible services.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where webhook S3 file URLs were always constructed using the hardcoded AWS S3 endpoint (https://{bucket}.s3.amazonaws.com/{key}), even when users configured custom S3-compatible endpoints (e.g., MinIO, DigitalOcean Spaces). The fix ensures that custom endpoints are respected when constructing file URLs in webhook payloads.

Changes:

  • Added conditional logic to construct S3 file URLs using custom endpoints when configured
  • Implemented support for both path-style and virtual-hosted-style URL formats based on the forcePathStyle configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

message.fileUrl = `https://${bucketName}.s3.amazonaws.com/${fileName}`;
if (config?.aws_s3?.endpoint) {
const url = new URL(config.aws_s3.endpoint);
const forcePathStyle = config?.aws_s3?.forcePathStyle ?? true;
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

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

There's an inconsistency in how forcePathStyle defaults are handled. On line 180, the S3Client configuration uses the OR operator (||), which converts false to undefined. This means if a user explicitly sets forcePathStyle: false, the S3Client will receive undefined instead of false.

However, on line 232, the URL construction uses the nullish coalescing operator (??) which correctly preserves explicit false values and only defaults to true for null or undefined.

This creates a mismatch: when forcePathStyle is explicitly set to false, the S3Client behavior may differ from the generated fileUrl format. The S3Client would use its default behavior (virtual-hosted style for AWS, varies for other providers), while the fileUrl would be generated using virtual-hosted style.

For consistency with line 232's logic and to allow users to explicitly control the path style, consider changing line 180 to use the same pattern. Note that this is a pre-existing issue, not introduced by this PR, but addressing it would improve the overall fix.

Copilot uses AI. Check for mistakes.
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