🐛 Added config sanitization to strip /ghost from admin:url

[ErisDS]

Problem

admin:url should be a base URL like https://admin.example.com — Ghost appends /ghost/ automatically via urlFor('admin', true). If the config value already contains /ghost, every caller of urlFor('admin', true) produces a doubled path like /ghost/ghost/.

There are 22 call sites across the codebase that use urlFor('admin', true), including password reset links, staff notification emails, invite links, CORS config, and comment notification URLs. All would produce broken links with a misconfigured admin:url.

Fix

Added sanitizeAdminUrl to the config loader (alongside the existing sanitizeDatabaseProperties and checkUrlProtocol). At boot, if admin:url contains /ghost in its path:

1. Strips it
2. Logs a warning: admin:url should not contain /ghost — Ghost adds this automatically. Corrected to ...

Also reverted a workaround in update-local-template-options.js that was masking the issue by using getAdminUrl() instead of urlFor.

Testing

4 unit tests added to config/utils.test.js.

[coderabbitai]

Walkthrough

The pull request modifies admin URL handling across multiple files. It changes the theme engine middleware to derive the admin URL directly from urlFor('admin', true) instead of using a conditional fallback. Additionally, it introduces a new sanitizeAdminUrl utility function that normalizes admin URLs by removing trailing /ghost segments from admin:url when present, with corresponding logging and configuration updates. The sanitization is integrated into the config loader's initialization flow and is accompanied by comprehensive test coverage.

🚥 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 clearly describes the main change: adding config sanitization to remove /ghost from admin:url, which directly matches the primary purpose of the PR.
Description check	✅ Passed	The description is comprehensive and directly related to the changeset, detailing the problem, fix approach, and testing added.

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

✨ 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 sanitize-admin-url

📝 Coding Plan

• Generate coding plan for human review comments

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@ghost/core/core/shared/config/utils.js`:
- Around line 99-101: The catch block swallowing errors for admin:url is
misleading because checkUrlProtocol only validates the main url, not admin:url;
update the comment in the catch block for the try that parses admin:url
(referencing checkUrlProtocol and urlFor('admin', true)) to state that invalid
admin:url values will be ignored here but not validated by checkUrlProtocol and
may cause downstream failures (e.g., urlFor('admin', true)), and optionally log
or rethrow the parse error if you want explicit validation rather than silent
ignore.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6a620a58-be5f-475b-b60a-82280617b02a

📥 Commits

Reviewing files that changed from the base of the PR and between 832456d and a5faba0.

📒 Files selected for processing (4)

• ghost/core/core/frontend/services/theme-engine/middleware/update-local-template-options.js
• ghost/core/core/shared/config/loader.js
• ghost/core/core/shared/config/utils.js
• ghost/core/test/unit/shared/config/utils.test.js

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Misleading comment: checkUrlProtocol validates main URL, not admin:url.

The comment suggests checkUrlProtocol will catch invalid admin:url values, but it only validates the main url config. An invalid admin:url (e.g., missing protocol) will fail silently here and may cause issues downstream when urlFor('admin', true) uses it.

Consider updating the comment to clarify:

     } catch (e) {
-        // invalid URL — checkUrlProtocol will catch this separately
+        // invalid URL — silently skip; urlFor will fail later if admin:url is malformed
     }

🤖 Prompt for AI Agents

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

In `@ghost/core/core/shared/config/utils.js` around lines 99 - 101, The catch
block swallowing errors for admin:url is misleading because checkUrlProtocol
only validates the main url, not admin:url; update the comment in the catch
block for the try that parses admin:url (referencing checkUrlProtocol and
urlFor('admin', true)) to state that invalid admin:url values will be ignored
here but not validated by checkUrlProtocol and may cause downstream failures
(e.g., urlFor('admin', true)), and optionally log or rethrow the parse error if
you want explicit validation rather than silent ignore.