Skip to content

wip: capture for custom domains - #3225

Draft
pory-gone wants to merge 1 commit into
stackernews:masterfrom
pory-gone:customdomainsupport
Draft

pory-gone wants to merge 1 commit into
stackernews:masterfrom
pory-gone:customdomainsupport

Conversation

@pory-gone

Copy link
Copy Markdown
Contributor

Description

A clear and concise description of what you changed and why.

Screenshots

Additional Context

Was anything unclear during your work on this PR? Anything we should definitely take a closer look at?

Checklist

Are your changes backward compatible? Please answer below:

For example, a change is not backward compatible if you removed a GraphQL field or dropped a database column.

On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:

For frontend changes: Tested on mobile, light and dark mode? Please answer below:

Did you introduce any new environment variables? If so, call them out explicitly here:

Did you use AI for this? If so, how much did it assist you?

@gitguardian

gitguardian Bot commented Sep 8, 2026

Copy link
Copy Markdown

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secret in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
- - Generic High Entropy Secret a6a791e .env.development View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secret safely. Learn here the best practices.
  3. Revoke and rotate this secret.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@iliasabk iliasabk left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Security finding: the new allowlist validates only the initial URL returned by resolveCaptureTarget, but captureImage then calls page.goto(url.href) with normal redirect behavior and never revalidates the final/main-frame origin. If an ACTIVE custom-domain owner changes DNS before the domain is marked inactive and serves a valid HTTPS 30x response, the browser can be redirected to an origin that was never allowlisted (for example a localhost/link-local/internal HTTP endpoint), and the capture service will screenshot that destination. That makes the public capture endpoint an SSRF/screenshot primitive despite the initial hostname check.

Please enforce the boundary at browser-navigation time too: reject/abort main-frame navigation (including redirects and subsequent location changes) unless the destination remains the expected main/custom origin, and preferably reject private/link-local resolved addresses as defense in depth. A regression test with an allowlisted origin returning a 302 to a disallowed origin would cover the gap.

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.

2 participants