Skip to content

SafeWebhookUrlTest's "unresolvable hostname" test performs a live DNS lookup, making a security-critical unit test network-dependent #165

Description

@morcen

What: test_unresolvable_hostname_is_unsafe asserts SafeWebhookUrl::isUrlSafe('http://this-host-should-never-resolve.invalid/webhook') is false, but the rule's resolveHost() performs a real network DNS query (@dns_get_record($host, DNS_A | DNS_AAAA)) with no mock/fake resolver injected.

Where:

  • tests/Unit/SafeWebhookUrlTest.php:39-42
  • app/Rules/SafeWebhookUrl.php:149-151 (resolveHost())

Why it matters: This test's outcome is not fully determined by the code under test. If DNS is unreachable in CI/sandbox, dns_get_record fails and the "unresolvable" path returns unsafe — the test passes, but for the wrong reason (it never actually exercised "hostname resolves to nothing," only "network unreachable"). On networks with NXDOMAIN-hijacking (some ISPs, and some corporate/CI egress proxies redirect unknown hostnames to a landing IP), the hostname could actually resolve, and the test's pass/fail would then hinge on whether that landing IP happens to be classified as public — an SSRF-prevention unit test whose result depends on external network conditions rather than the validation logic itself.

Suggested fix: Extract resolveHost()'s DNS lookup behind a swappable resolver (interface/callable) so the test can inject a fake resolver that deterministically returns no records, decoupling this security-critical test from live network behavior.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions