Skip to content

[codex] Harden public outbound URL validation#2199

Merged
riderx merged 3 commits into
mainfrom
codex/harden-outbound-url-validation
May 11, 2026
Merged

[codex] Harden public outbound URL validation#2199
riderx merged 3 commits into
mainfrom
codex/harden-outbound-url-validation

Conversation

@riderx
Copy link
Copy Markdown
Member

@riderx riderx commented May 11, 2026

Summary (AI generated)

  • Added a shared public outbound URL validator for syntax checks, DNS-over-HTTPS preflight, private IP blocking, and manual redirect revalidation.
  • Reused that validator for website preview fetches and webhook URL create/update/test/retry/dispatch/delivery paths.
  • Added unit coverage for private DNS answers, redirect-to-private blocking, and webhook delivery preflight behavior.

Motivation (AI generated)

DNS-based SSRF advisories kept recurring because webhook targets only had syntactic URL checks. Centralizing public outbound validation gives the advisory scanners a concrete preflight path while keeping the existing Cloudflare serverless assumptions documented in code.

Business Impact (AI generated)

This should reduce repeated advisory noise around website preview and webhooks without blocking normal customer webhooks when DNS preflight is unavailable. Explicit private DNS answers are blocked before outbound delivery.

Test Plan (AI generated)

  • bun lint:backend
  • bunx vitest run tests/public-url-validation.unit.test.ts tests/webhook-delivery-security.unit.test.ts tests/webhook-delivery-redirect.unit.test.ts
  • bun typecheck

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Admin dashboard now requires proper user authentication for replication status checks.
    • Enhanced webhook security validation to prevent delivery to private network addresses.
    • Improved website preview URL validation with centralized error messaging.
  • Style

    • Updated sidebar navigation button styling and admin dashboard loading UI appearance.
  • Tests

    • Added comprehensive public URL validation test coverage.
    • Enhanced webhook delivery test assertions for better accuracy.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

Warning

Rate limit exceeded

@riderx has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 3 minutes and 2 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f6b057b9-2fbe-4790-b353-94c53c5ff4c1

📥 Commits

Reviewing files that changed from the base of the PR and between c7da8d7 and 86df672.

📒 Files selected for processing (2)
  • src/pages/admin/dashboard/replication.vue
  • supabase/functions/_backend/utils/publicUrl.ts
📝 Walkthrough

Walkthrough

The PR refactors public URL validation into a centralized module with DNS and private-IP detection, then migrates webhook and website preview URL validation to use this system. Webhook handlers now perform async hostname validation before delivery. Tests verify syntax validation, DNS-based blocking of private IPs, and redirect revalidation.

Changes

Public URL Validation & Webhook Security

Layer / File(s) Summary
Public URL Validation Module
supabase/functions/_backend/utils/publicUrl.ts
New utility exports PublicUrlValidationMessages, PublicUrlValidationOptions, FetchPublicUrlOptions; implements normalizePublicHostname(), getPublicUrlSyntaxValidationError() (checks localhost/IP-literals/HTTPS), getPublicHostnameValidationError() (async DNS resolution + private-IP detection), and fetchPublicUrl() (fetch with redirect revalidation up to configurable max).
Webhook Validation Refactor
supabase/functions/_backend/utils/webhook.ts
Imports public URL validators; defines WEBHOOK_URL_VALIDATION_MESSAGES; refactors getWebhookUrlValidationError() to delegate to syntax validator; adds new async getWebhookPublicUrlValidationError() for hostname validation with optional DNS; updates deliverWebhook() to await async validation before marking delivery failed.
Webhook POST Handler
supabase/functions/_backend/public/webhooks/post.ts
Imports and awaits getWebhookPublicUrlValidationError instead of synchronous getWebhookUrlValidationError.
Webhook PUT Handler
supabase/functions/_backend/public/webhooks/put.ts
Switches import and validation call to getWebhookPublicUrlValidationError when validating optional url field.
Webhook Test Handler
supabase/functions/_backend/public/webhooks/test.ts
Updates import and validation call to use getWebhookPublicUrlValidationError for test ping URL validation.
Webhook Retry (Deliveries)
supabase/functions/_backend/public/webhooks/deliveries.ts
Updates import and validation call in retryDelivery() to use getWebhookPublicUrlValidationError.
Webhook Dispatcher
supabase/functions/_backend/triggers/webhook_dispatcher.ts
Updates import and awaits getWebhookPublicUrlValidationError before updating delivery record on validation failure.
Website Preview Refactor
supabase/functions/_backend/private/website_preview.ts
Removes Context-based validation; adds WEBSITE_URL_VALIDATION_MESSAGES; introduces getWebsitePublicHostnameValidationError() delegating to public hostname validator with DNS required; replaces custom fetchValidatedUrl(c, ...) with fetchValidatedUrl(urlString, init?) using fetchPublicUrl(); updates icon fetching and response canonicalization to use finalUrl.
Replication Auth & UI
src/pages/admin/dashboard/replication.vue, src/components/Sidebar.vue
loadReplicationStatus() now requires Supabase session access token (removed fallback to VITE_REPLICATION_API_SECRET); spinner min-height changed to min-h-75; Sidebar button height optimized to min-h-11.
Public URL Validation Tests
tests/public-url-validation.unit.test.ts
New test suite verifying: URL syntax validation rejects localhost/IP-literals/non-HTTPS before DNS; DNS A/AAAA resolution blocks private IPv4/IPv6; missing DNS answers configurable via requireDnsResolution; redirect targets revalidated to prevent fetching unsafe redirected URLs.
Webhook Delivery Tests
tests/webhook-delivery-redirect.unit.test.ts, tests/webhook-delivery-security.unit.test.ts
Redirect handling tests refactored to filter mock calls by target URL and assert redirect: 'manual'; new security test verifies delivery blocked when webhook host resolves to private address via mocked Cloudflare DoH.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Cap-go/capgo#2090: Both PRs modify webhook URL validation logic in supabase/functions/_backend/utils/webhook.ts, with this PR refactoring the validators and #2090 adding inline documentation to the same validation functions.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.17% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: hardening public outbound URL validation to address security concerns.
Description check ✅ Passed The pull request description provides a comprehensive summary, motivation, business impact, and test plan, covering the key aspects of the changes, though it lacks step-by-step test reproduction steps and screenshots.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/harden-outbound-url-validation

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread supabase/functions/_backend/utils/publicUrl.ts Fixed
@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq Bot commented May 11, 2026

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing codex/harden-outbound-url-validation (86df672) with main (e7952d0)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@riderx riderx marked this pull request as ready for review May 11, 2026 10:52
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/pages/admin/dashboard/replication.vue`:
- Around line 111-112: The error thrown when session?.access_token is missing
still references a removed "replication secret" fallback; update the check in
the replication code to throw a clear message that only session-based
authentication is supported by changing the Error thrown in the
session?.access_token guard (the if (!session?.access_token) branch) to
something like "No session available; replication requires session-based
authentication" so it no longer mentions the removed VITE_REPLICATION_API_SECRET
fallback.
- Line 205: The class on the loading container uses an invalid Tailwind utility
`min-h-75`; update the div with v-else-if="isLoading && !data" to use a valid
Tailwind min-height such as `min-h-72` or `min-h-80`, or use arbitrary value
syntax like `min-h-[18.75rem]` so the spinner container gets the intended
minimum height.

In `@supabase/functions/_backend/utils/publicUrl.ts`:
- Around line 154-157: The two sequential awaits building ips cause A and AAAA
lookups to be serialized; change to run them in parallel by using Promise.all to
await both resolveHostnameIps(url.hostname, 'A') and
resolveHostnameIps(url.hostname, 'AAAA') concurrently and then merge the results
into ips (so keep the same variable name and downstream logic, e.g. const [aIps,
aaaaIps] = await Promise.all([...]) or const results = await Promise.all([...])
and spread results into ips). Ensure this preserves behavior when
requireDnsResolution is false and retains the same fallback semantics.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8b396577-ea16-4ed3-96ad-93947eb24c9e

📥 Commits

Reviewing files that changed from the base of the PR and between 84fa1f2 and c7da8d7.

📒 Files selected for processing (13)
  • src/components/Sidebar.vue
  • src/pages/admin/dashboard/replication.vue
  • supabase/functions/_backend/private/website_preview.ts
  • supabase/functions/_backend/public/webhooks/deliveries.ts
  • supabase/functions/_backend/public/webhooks/post.ts
  • supabase/functions/_backend/public/webhooks/put.ts
  • supabase/functions/_backend/public/webhooks/test.ts
  • supabase/functions/_backend/triggers/webhook_dispatcher.ts
  • supabase/functions/_backend/utils/publicUrl.ts
  • supabase/functions/_backend/utils/webhook.ts
  • tests/public-url-validation.unit.test.ts
  • tests/webhook-delivery-redirect.unit.test.ts
  • tests/webhook-delivery-security.unit.test.ts

Comment thread src/pages/admin/dashboard/replication.vue Outdated
Comment thread src/pages/admin/dashboard/replication.vue Outdated
Comment thread supabase/functions/_backend/utils/publicUrl.ts Outdated
@riderx
Copy link
Copy Markdown
Member Author

riderx commented May 11, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@sonarqubecloud
Copy link
Copy Markdown

@riderx riderx merged commit 0fab75e into main May 11, 2026
52 of 53 checks passed
@riderx riderx deleted the codex/harden-outbound-url-validation branch May 11, 2026 11:32
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