Adds automatic domain validation and allow-listing #2092
Conversation
|
|
||
| // Validates a domain with the Cloudflare worker endpoint | ||
| async validateDomain(hostname, options = {}) { | ||
| const endpoint = 'https://winter-morning-fa32.shobhit-k.workers.dev/validate-domain'; |
There was a problem hiding this comment.
It should come from API maybe as response of project fetch or build fetch, we don't want CLI release in case we change it or if we move to alternative
| const ABORTED_MESSAGE = 'Request was aborted by browser'; | ||
|
|
||
| // Domain validation constants | ||
| const DOMAIN_VALIDATION_ENDPOINT = 'https://winter-morning-fa32.shobhit-k.workers.dev/validate-domain'; |
There was a problem hiding this comment.
should not be hardcoded
| // Check if validation is already pending for this domain | ||
| if (pending.has(hostname)) { | ||
| network.log.debug(`Domain validation: Waiting for pending validation of ${hostname}`, network.meta); | ||
| return pending.get(hostname); |
There was a problem hiding this comment.
If pending what's the guarantee it will be set by the time it is fetched?
| // Extract hostname for domain validation | ||
| let hostname; | ||
| try { | ||
| hostname = new URL(url).hostname; |
There was a problem hiding this comment.
instead of hostname use host to handle domain with port
| let hostname; | ||
| try { | ||
| hostname = new URL(url).hostname; | ||
| } catch (e) { | ||
| hostname = null; | ||
| } |
There was a problem hiding this comment.
make a method for it
| let hostname; | ||
| try { | ||
| hostname = new URL(url).hostname; | ||
| } catch (e) { | ||
| hostname = null; | ||
| } |
There was a problem hiding this comment.
make a method for it
|
|
||
| // Load domain config early if domain validation is enabled | ||
| // This ensures pre-approved domains are loaded before discovery starts | ||
| if (!this.skipUploads) { |
There was a problem hiding this comment.
We should keep some kill switch via ENV variable to stop this flow completely in case something goes wrong user will have some way to stop this flow
| network.log.debug(`Domain validation: ${hostname} validated as ALLOWED`, network.meta); | ||
| return 'allowed'; | ||
| } else { | ||
| sessionBlocked.add(hostname); |
There was a problem hiding this comment.
this can be risky if some domain took time to load, it will end up adding it in blocked which should not be ideal case as in future all builds will go as disallowedHostname.
| preApproved: new Set(), // Domains from project config | ||
| preBlocked: new Set(), // Domains from project config | ||
| sessionAllowed: new Set(), // Newly discovered allowed domains this session | ||
| sessionBlocked: new Set(), // Newly discovered blocked domains this session |
There was a problem hiding this comment.
Any reason for blocked domains as it can have high impact, allowing is still fine as it can increase our cost but disallow can have negative impact, we can store on our side for reference but should not use this in CLI imo
This pull request introduces a new auto domain allow-listing feature to the Percy client and core, enabling automatic validation and management of external domains accessed during snapshot capture. This helps streamline asset handling by validating domains against project configuration and an external service, and by persisting new discoveries for future builds. The implementation includes changes to both the core logic and the client API, as well as comprehensive tests for the new functionality.
The most important changes are:
Core logic for auto domain allow-listing:
Percyclass, including sets for pre-approved, pre-blocked, session-allowed, and session-blocked domains, as well as statistics tracking. (packages/core/src/percy.js)packages/core/src/network.js) [1] [2]packages/core/src/network.js) [1] [2]Project domain configuration persistence:
Percyclass to load project domain config at startup and save newly discovered allowed/blocked domains at shutdown, integrating with the Percy client API. (packages/core/src/percy.js) [1] [2] [3]Percy client API enhancements:
patchmethod and new methodsupdateProjectDomainConfigandvalidateDomainto thePercyClientclass to support updating project domain configuration and validating domains via an external service. (packages/client/src/client.js) [1] [2]Test coverage for new features:
updateProjectDomainConfigandvalidateDomainmethods, including positive and negative scenarios and request payload validation. (packages/client/test/client.test.js,packages/client/test/helpers.js) [1] [2]Wiring and context propagation:
packages/core/src/discovery.js,packages/core/src/network.js) [1] [2]