Restricted image size fetching to internal images only

[kevinansfield]

ref https://linear.app/ghost/issue/ONC-1525/credential-leak-vulnerability

Restricted image size fetching to only process internal images.

[coderabbitai]

Walkthrough

The changes refactor image size handling in the mobiledoc library. In the main source file, the conditional logic for processing different image types is made more explicit by converting a fallback else block into a specific else-if statement for internal images. The test suite is updated to remove a logging dependency and to verify that external images are only sized when explicitly configured. Additional test cases are added to cover edge scenarios, including handling of non-standard URLs and validating that arbitrary external URLs are not processed for sizing information.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: restricting image size fetching to internal images only, which directly matches the primary code modification in mobiledoc.js.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check	✅ Passed	The PR description accurately describes the changeset: restricting image size fetching to internal images, which is reflected in the code changes replacing a generic else with else-if for internal image checks.

_{✏️ 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 restrict-image-size-fetching

---

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

[coderabbitai]

🧹 Nitpick comments (1)

ghost/core/test/unit/server/lib/mobiledoc.test.js (1)

355-367: Consider adding explicit verification that no HTTP request was made.

This is an excellent SSRF prevention test using the AWS metadata endpoint. However, it only verifies the result has no dimensions—it doesn't explicitly confirm that no HTTP request was attempted. Consider adding a nock assertion to strengthen the test.

🔒 Suggested improvement to verify no request was made

 it('skips sizing for arbitrary external URLs', async function () {
+    const metadataMock = nock('http://169.254.169.254')
+        .get('/latest/meta-data/')
+        .reply(200, 'test');
+
     let mobiledoc = {
         cards: [
             ['image', {src: 'http://169.254.169.254/latest/meta-data/'}]
         ]
     };

     const transformedMobiledoc = await mobiledocLib.populateImageSizes(JSON.stringify(mobiledoc));
     const transformed = JSON.parse(transformedMobiledoc);

+    // Verify no request was made to the metadata endpoint
+    assert.equal(metadataMock.isDone(), false);
     assert.equal(transformed.cards[0][1].width, undefined);
     assert.equal(transformed.cards[0][1].height, undefined);
 });

🤖 Prompt for AI Agents

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

In `@ghost/core/test/unit/server/lib/mobiledoc.test.js` around lines 355 - 367,
Set up a nock scope for the AWS metadata URL before calling populateImageSizes
and assert after the call that the scope was not invoked; specifically, in the
"skips sizing for arbitrary external URLs" test create a nock for
'http://169.254.169.254' with .get('/latest/meta-data/') and a dummy
.reply(...), call mobiledocLib.populateImageSizes(JSON.stringify(mobiledoc)),
then assert that scope.isDone() (or nock.isDone()) is false to prove no HTTP
request was made, and finally clean up nock with nock.cleanAll() / scope.done
handling.

🤖 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/mobiledoc.test.js`:
- Around line 355-367: Set up a nock scope for the AWS metadata URL before
calling populateImageSizes and assert after the call that the scope was not
invoked; specifically, in the "skips sizing for arbitrary external URLs" test
create a nock for 'http://169.254.169.254' with .get('/latest/meta-data/') and a
dummy .reply(...), call
mobiledocLib.populateImageSizes(JSON.stringify(mobiledoc)), then assert that
scope.isDone() (or nock.isDone()) is false to prove no HTTP request was made,
and finally clean up nock with nock.cleanAll() / scope.done handling.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0d134cdb-8671-4a5d-b24c-1ff577ba0f45

📥 Commits

Reviewing files that changed from the base of the PR and between 292bdc3 and 01bedf0.

📒 Files selected for processing (2)

• ghost/core/core/server/lib/mobiledoc.js
• ghost/core/test/unit/server/lib/mobiledoc.test.js