Corrige validação de URL para permitir localhost e endereços IP#1290
Corrige validação de URL para permitir localhost e endereços IP#1290DavidsonGomes merged 1 commit intoEvolutionAPI:developfrom
Conversation
Reviewer's Guide by SourceryThe pull request replaces the Sequence diagram for webhook URL validationsequenceDiagram
participant User
participant WebhookController
participant CustomRegex
User->>WebhookController: Sends webhook data with URL
WebhookController->>CustomRegex: Validates URL using regex
alt URL is valid
WebhookController->>WebhookController: Processes webhook
else URL is invalid
WebhookController-->>User: Returns error
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey @jrCleber - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider defining the regex in a single place and reusing it to avoid inconsistencies.
- The new regex only checks for
https?at the beginning, which is less strict than the originalisURLvalidation.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| override async set(instanceName: string, data: EventDto): Promise<wa.LocalWebHook> { | ||
| if (!isURL(data.webhook.url, { require_tld: false })) { | ||
| if (!/^(https?:\/\/)/.test(data.webhook.url)) { |
There was a problem hiding this comment.
question (bug_risk): Review the new URL validation logic using regex.
The previous implementation used isURL with specific options, which may have been more comprehensive. This regex only checks for the presence of a protocol prefix, so please ensure that this simplified check meets all your validation requirements.
| const we = event.replace(/[.-]/gm, '_').toUpperCase(); | ||
| const transformedWe = we.replace(/_/gm, '-').toLowerCase(); | ||
| const enabledLog = configService.get<Log>('LOG').LEVEL.includes('WEBHOOKS'); | ||
| const regex = /^(https?:\/\/)/; |
There was a problem hiding this comment.
suggestion: Consolidate URL validation logic.
Consider declaring and using a shared, well-documented constant for the URL validation regex across all methods. This will help avoid inconsistent usage, as the set method uses a literal regex while other parts rely on the declared variable.
Suggested implementation:
/**
* Regex pattern for validating URLs starting with "http://" or "https://"
*/
const URL_VALIDATION_REGEX = /^(https?:\/\/)/;
// (Existing imports remain unchanged) if (!URL_VALIDATION_REGEX.test(data.webhook.url)) { // Removed local regex constant in favor of shared URL_VALIDATION_REGEX if (instance?.enabled && URL_VALIDATION_REGEX.test(instance.url)) {Make sure that the new constant declaration is positioned appropriately (e.g. at the top of the file after the imports) and that no other part of the file uses a hard-coded regex. Adjust the location if your project's conventions require constants to be declared in a separate file.
O isUrl do class-validator não valida URLs com localhost porque ele segue a especificação da WHATWG URL, que considera localhost um nome de host especial e pode não tratá-lo como um domínio válido.
Se você quiser uma validação mais rigorosa, que permita localhost, IPs e domínios válidos, pode usar esta regex aprimorada:
Este PR substitui a validação de URL que utilizava
isUrldoclass-validatorpor uma regex personalizada. A motivação para essa mudança é queisUrl, mesmo com a opçãoprotocols: ['http', 'https'], não validava URLs que usamlocalhost.A nova regex implementada agora permite:
httpehttpslocalhostcom ou sem porta (http://localhost,http://localhost:3000)http://192.168.1.1,https://10.0.0.1:8080)https://example.com,http://sub.example.org)https://example.com/api)Essa alteração garante maior flexibilidade na configuração de webhooks, permitindo URLs locais e ambientes de desenvolvimento sem comprometer a segurança ou funcionalidade.
Alterações principais:
isUrlpela regex/^(https?:\/\/)(localhost|(\d{1,3}\.){3}\d{1,3}|([a-zA-Z0-9-]+\.)+[a-zA-Z]{2,})(:\d+)?(\/.*)?$/Testes realizados:
✅ Testado com URLs locais (
http://localhost,http://localhost:3000)✅ Testado com IPs (
http://127.0.0.1,https://192.168.0.1:8000)✅ Testado com domínios (
https://example.com,http://api.test.net)✅ Testado com portas e caminhos opcionais
Impacto:
Esta mudança melhora a compatibilidade com ambientes locais e configurações personalizadas de webhook, garantindo uma validação mais robusta sem restringir casos de uso comuns
Summary by Sourcery
Replaces the isUrl validation with a custom regex to allow localhost and IP addresses in webhook URLs.
Bug Fixes: