Allowed localhost when making requests

[EvanHahn]

towards https://linear.app/ghost/issue/NY-1465
ref TryGhost/Ghost#28978 (review)

Post scheduling is broken in development because we use a localhost URL. @tryghost/request doesn't consider localhost URLs valid, so scheduling is broken.

This fixes that by allowing localhost URLs.

I was a little nervous about loosening the validation, but it's code could make requests to localhost before this change.

(I considered an alternative where Ghost development used 127.0.0.1 instead, which also fixes the problem, but that's too disruptive to development IMO.)

[coderabbitai]

Walkthrough

The request URL validation now requires a string and accepts URLs supported by the default validator or localhost hosts. Tests cover overlong URLs, URLs with spaces or angle brackets, and successful requests to localhost:2368.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: cmraible

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check	✅ Passed	The title clearly and concisely identifies the main change: allowing localhost URLs in requests.
Description check	✅ Passed	The description directly explains the localhost validation issue, its impact on development scheduling, and the implemented fix.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch allow-localhost-when-making-http-requests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.59%. Comparing base (35474cd) to head (01ed88d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #878      +/-   ##
==========================================
- Coverage   97.76%   96.59%   -1.18%     
==========================================
  Files         105       14      -91     
  Lines        3452      676    -2776     
  Branches      616      118     -498     
==========================================
- Hits         3375      653    -2722     
+ Misses         25        9      -16     
+ Partials       52       14      -38     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cmraible]

Just a heads up that I likely won't have time to review this today. Happy to take a look tomorrow/Thursday. I don't think it's urgent, but let me know if you disagree.

[EvanHahn]

Yeah, not urgent at all. No rush.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[cmraible]

I was a little nervous about loosening the validation, but it's code could make requests to localhost before this change.

question: could you elaborate on "it's code could make requests to localhost before this change"?

[cmraible]

thought: I'm curious what changed that broke post scheduling locally. I'm a little hesitant to loosen this validation before we understand what caused it to break in the first place.

[EvanHahn]

Didn't realize that post scheduling ever worked locally. I'll investigate (probably not urgently).

[9larsons]

From my perspective, looks fine. I think the risk for allowing localhost is SSRF, but we only use request for internal requests. We have request-external for external reqs.

+1 though it'd be nice to know the root cause.

[EvanHahn]

I think newsletter scheduling broke in dev this way: e93c69f updated validator in this repo, and the TryGhost/Ghost@45118e9 picked that up.

The root cause, though, is that we never detected this. (Those commits above are, IMO, flawless.)

Now that I understand this, I'm going to merge, as it's blocking some other work.