Improved DNS validation in external request handling

[kevinansfield]

ref https://linear.app/ghost/issue/ONC-1533/

Ensured that the IP address validated during the DNS check is the same
one used for the actual TCP connection by installing a custom dnsLookup
on request options. Added tests for the new validation layer.

[coderabbitai]

Walkthrough

This change introduces DNS rebinding attack protection by adding a new installSafeDnsLookup function to the request-external module. The function enforces private IP address blocking at connection time, integrating into both initial requests and redirects via beforeRequest and beforeRedirect hooks. The implementation bypasses checks in development environments and for same-host requests. The export naming was changed from _installSafeDnsLookup to installSafeDnsLookup. Comprehensive test coverage was added with DNS module stubbing and lifecycle management to verify blocking of private IPs, allowance of public IPs, and proper error propagation.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Improved DNS validation in external request handling' clearly summarizes the main change: enhancing DNS validation in the external request module to prevent DNS rebinding attacks.
Description check	✅ Passed	The description is directly related to the changeset, explaining the mechanism (custom dnsLookup on request options) and mentioning the addition of tests, which aligns with the actual changes.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch dns-validation-external-requests

---

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

[coderabbitai]

🧹 Nitpick comments (1)

ghost/core/test/unit/server/lib/request-external.test.js (1)

527-535: Unnecessary await on synchronous function.

installSafeDnsLookup is a synchronous function that doesn't return a promise. The await is harmless but unnecessary.

🔧 Suggested fix

-            await installSafeDnsLookup(options);
+            installSafeDnsLookup(options);

This applies to all test cases in this suite (lines 532, 544, 556, 568, 589, 619, 649, 681, 714, 743).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@ghost/core/test/unit/server/lib/request-external.test.js` around lines 527 -
535, The tests call the synchronous function installSafeDnsLookup with an
unnecessary await; remove the await operator from all test invocations of
installSafeDnsLookup (e.g., in the "installs a lookup function on request
options" test and the other cases in the same suite) so the function is invoked
directly (installSafeDnsLookup(options)) and assertions remain unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@ghost/core/test/unit/server/lib/request-external.test.js`:
- Around line 527-535: The tests call the synchronous function
installSafeDnsLookup with an unnecessary await; remove the await operator from
all test invocations of installSafeDnsLookup (e.g., in the "installs a lookup
function on request options" test and the other cases in the same suite) so the
function is invoked directly (installSafeDnsLookup(options)) and assertions
remain unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f28fbf46-5ecc-4036-beca-4e1d85128855

📥 Commits

Reviewing files that changed from the base of the PR and between 292bdc3 and 1eda49d.

📒 Files selected for processing (2)

• ghost/core/core/server/lib/request-external.js
• ghost/core/test/unit/server/lib/request-external.test.js