Fix .md redirect target for routes.yaml-remapped pages (#30375)

[wakqasahmed]

When a page's URL is remapped by routes.yaml (a collection's `data:` source, or the site root), its `.md` variant redirects to the wrong place. The reporter pointed at the markdown controller, but the actual bug is one level up: `ParentRouter._respectDominantRouter()` builds the dominant-router redirect by appending the unmatched `.md` request path onto the routed directory, so `/contact.md` becomes `/rubrique/contact.md` (404) instead of `/rubrique.md`. For a root-mapped page this produces a redirect to itself instead of a working `/index.md`.

Fixed the dominant-router redirect to use `getMarkdownPath(targetRoute)` when the incoming request is a `.md` path, same as the correct path used for `llms.txt` generation. Also mounted a real `.md` route directly on entry-backed collection/static routes (instead of relying only on the redirect) so the root-page case resolves to a working `/index.md` rather than looping, and threaded the resolved canonical path through so `Content-Location` and the non-llms redirect target stay consistent.

Added e2e coverage with a real routes.yaml fixture covering both cases (collection `data:` remap and root remap). Confirmed both fail on the pre-fix code (`/rubrique/contact.md` 404, self-redirect loop on `/about.md`) and pass after the fix. Ran the broader routing test suite too (55/55 passing) since this touches shared redirect logic.

• I've read and followed the Contributor Guide
• I've explained my change
• I've written an automated test to prove my change works

Closes #30375

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: TryGhost/Ghost/.coderabbit.yaml
• Review profile: QUIET
• Plan: Advanced
• Run ID: dfa88677-9615-4792-8f68-365bee7fba68

📥 Commits

Reviewing files that changed from the base of the PR and between bacec11 and c684b5e.

📒 Files selected for processing (17)

• ghost/core/core/frontend/services/data/fetch-data.js
• ghost/core/core/frontend/services/data/index.js
• ghost/core/core/frontend/services/routing/api-adapter.ts
• ghost/core/core/frontend/services/routing/collection-router.js
• ghost/core/core/frontend/services/routing/controllers/entry.ts
• ghost/core/core/frontend/services/routing/controllers/entry/canonical-url.ts
• ghost/core/core/frontend/services/routing/controllers/entry/markdown.ts
• ghost/core/core/frontend/services/routing/controllers/index.js
• ghost/core/core/frontend/services/routing/parent-router.js
• ghost/core/core/frontend/services/routing/static-routes-router.js
• ghost/core/test/e2e-frontend/markdown-routes.test.js
• ghost/core/test/unit/frontend/services/data/fetch-data.test.js
• ghost/core/test/unit/frontend/services/routing/collection-router.test.js
• ghost/core/test/unit/frontend/services/routing/controllers/entry.test.ts
• ghost/core/test/unit/frontend/services/routing/parent-router.test.js
• ghost/core/test/unit/frontend/services/routing/static-routes-router.test.js
• ghost/core/test/utils/fixtures/settings/markdown-routes.yaml

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

📜 Recent review details

🧰 Additional context used

📚 Code guidelines (2)

docs/contributing/testing.md — configured
docs/codebase/monorepo-structure.md — configured

📓 Path-based instructions (6)

Review whether tests prove changed behaviour, meaningful error/edge paths, and externally observable contracts without coupling to implementation details.

⚙️ CodeRabbit configuration file

Files:

• ghost/core/test/unit/frontend/services/routing/collection-router.test.js
• ghost/core/test/unit/frontend/services/routing/controllers/entry.test.ts
• ghost/core/test/unit/frontend/services/routing/parent-router.test.js
• ghost/core/test/unit/frontend/services/routing/static-routes-router.test.js
• ghost/core/test/unit/frontend/services/data/fetch-data.test.js
• ghost/core/test/e2e-frontend/markdown-routes.test.js

New source files must be TypeScript: flag new JS files as a required change unless exempt (DB migrations, apps/ember-admin/, tool/config files, scripts/, docker/, generated code).

⚙️ CodeRabbit configuration file

Files:

• ghost/core/core/frontend/services/data/index.js
• ghost/core/test/unit/frontend/services/routing/collection-router.test.js
• ghost/core/core/frontend/services/routing/collection-router.js
• ghost/core/core/frontend/services/routing/static-routes-router.js
• ghost/core/test/unit/frontend/services/routing/parent-router.test.js
• ghost/core/test/unit/frontend/services/routing/static-routes-router.test.js
• ghost/core/core/frontend/services/routing/controllers/index.js
• ghost/core/test/unit/frontend/services/data/fetch-data.test.js
• ghost/core/test/e2e-frontend/markdown-routes.test.js
• ghost/core/core/frontend/services/routing/parent-router.js
• ghost/core/core/frontend/services/data/fetch-data.js

Review lens: "where does this data become trusted?" Boundary data (HTTP input, external API/SDK responses, env/config, DB/filesystem reads, queue/webhook/event payloads) is `unknown` until validated — Zod by default.

⚙️ CodeRabbit configuration file

Files:

• ghost/core/core/frontend/services/routing/api-adapter.ts
• ghost/core/test/unit/frontend/services/routing/controllers/entry.test.ts
• ghost/core/core/frontend/services/routing/controllers/entry/canonical-url.ts
• ghost/core/core/frontend/services/routing/controllers/entry/markdown.ts
• ghost/core/core/frontend/services/routing/controllers/entry.ts

Prioritise concrete correctness, security, data-integrity, compatibility, and regression risks.

⚙️ CodeRabbit configuration file

Files:

• ghost/core/core/frontend/services/data/index.js
• ghost/core/test/unit/frontend/services/routing/collection-router.test.js
• ghost/core/core/frontend/services/routing/api-adapter.ts
• ghost/core/test/unit/frontend/services/routing/controllers/entry.test.ts
• ghost/core/core/frontend/services/routing/collection-router.js
• ghost/core/core/frontend/services/routing/static-routes-router.js
• ghost/core/core/frontend/services/routing/controllers/entry/canonical-url.ts
• ghost/core/test/unit/frontend/services/routing/parent-router.test.js
• ghost/core/test/unit/frontend/services/routing/static-routes-router.test.js
• ghost/core/core/frontend/services/routing/controllers/index.js
• ghost/core/test/unit/frontend/services/data/fetch-data.test.js
• ghost/core/core/frontend/services/routing/controllers/entry/markdown.ts
• ghost/core/test/e2e-frontend/markdown-routes.test.js
• ghost/core/core/frontend/services/routing/controllers/entry.ts
• ghost/core/test/utils/fixtures/settings/markdown-routes.yaml
• ghost/core/core/frontend/services/routing/parent-router.js
• ghost/core/core/frontend/services/data/fetch-data.js

Source excerpt: Ghost has several test suites across the monorepo.

📄 CodeRabbit inference engine (docs/contributing/testing.md)

Files:

• ghost/core/test/unit/frontend/services/routing/collection-router.test.js
• ghost/core/test/unit/frontend/services/routing/controllers/entry.test.ts
• ghost/core/test/unit/frontend/services/routing/parent-router.test.js
• ghost/core/test/unit/frontend/services/routing/static-routes-router.test.js
• ghost/core/test/unit/frontend/services/data/fetch-data.test.js
• ghost/core/test/e2e-frontend/markdown-routes.test.js
• ghost/core/test/utils/fixtures/settings/markdown-routes.yaml

Source excerpt: Built Admin assets are copied into `ghost/core/core/built/admin/` for the Ghost release.

📄 CodeRabbit inference engine (docs/codebase/monorepo-structure.md)

Files:

• ghost/core/core/frontend/services/data/index.js
• ghost/core/test/unit/frontend/services/routing/collection-router.test.js
• ghost/core/core/frontend/services/routing/api-adapter.ts
• ghost/core/test/unit/frontend/services/routing/controllers/entry.test.ts
• ghost/core/core/frontend/services/routing/collection-router.js
• ghost/core/core/frontend/services/routing/static-routes-router.js
• ghost/core/core/frontend/services/routing/controllers/entry/canonical-url.ts
• ghost/core/test/unit/frontend/services/routing/parent-router.test.js
• ghost/core/test/unit/frontend/services/routing/static-routes-router.test.js
• ghost/core/core/frontend/services/routing/controllers/index.js
• ghost/core/test/unit/frontend/services/data/fetch-data.test.js
• ghost/core/core/frontend/services/routing/controllers/entry/markdown.ts
• ghost/core/test/e2e-frontend/markdown-routes.test.js
• ghost/core/core/frontend/services/routing/controllers/entry.ts
• ghost/core/test/utils/fixtures/settings/markdown-routes.yaml
• ghost/core/core/frontend/services/routing/parent-router.js
• ghost/core/core/frontend/services/data/fetch-data.js

🪛 ast-grep (0.45.3)

ghost/core/core/frontend/services/routing/parent-router.js

[warning] 117-117: Detects non-literal values in regular expressions
Context: new RegExp(matchPath)
Note: [CWE-1333] Inefficient Regular Expression Complexity (ReDoS via non-literal RegExp).

(detect-non-literal-regexp)

---

[warning] 117-117: Do not use variable for regular expressions
Context: new RegExp(matchPath)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.

(regexp-non-literal)

🔇 Additional comments (17)

ghost/core/core/frontend/services/routing/api-adapter.ts (1)

157-167: LGTM!

ghost/core/core/frontend/services/data/fetch-data.js (1)

124-137: LGTM!

ghost/core/core/frontend/services/data/index.js (1)

5-5: LGTM!

ghost/core/test/unit/frontend/services/data/fetch-data.test.js (1)

226-241: LGTM!

ghost/core/core/frontend/services/routing/controllers/entry.ts (1)

131-155: LGTM!

ghost/core/core/frontend/services/routing/collection-router.js (1)

74-75: LGTM!

ghost/core/core/frontend/services/routing/static-routes-router.js (1)

61-61: LGTM!

Also applies to: 106-106

ghost/core/core/frontend/services/routing/controllers/index.js (1)

30-33: LGTM!

ghost/core/core/frontend/services/routing/controllers/entry/canonical-url.ts (1)

10-16: LGTM!

ghost/core/core/frontend/services/routing/controllers/entry/markdown.ts (1)

56-62: LGTM!

ghost/core/test/unit/frontend/services/routing/controllers/entry.test.ts (1)

400-460: LGTM!

ghost/core/test/unit/frontend/services/routing/static-routes-router.test.js (1)

120-143: LGTM!

ghost/core/test/utils/fixtures/settings/markdown-routes.yaml (1)

1-20: LGTM!

ghost/core/core/frontend/services/routing/parent-router.js (1)

105-124: LGTM!

ghost/core/test/unit/frontend/services/routing/parent-router.test.js (1)

318-427: LGTM!

ghost/core/test/unit/frontend/services/routing/collection-router.test.js (1)

96-109: LGTM!

ghost/core/test/e2e-frontend/markdown-routes.test.js (1)

1-102: LGTM!

---

Walkthrough

Custom collection and static routes now register .md endpoints for routes backed by a single page or post. The Markdown controller resolves routed entries and uses route metadata for Content-Location and canonical redirects. Dominant-router redirects use matching Markdown routes when available. Unit and end-to-end tests cover route registration, redirects, response metadata, and Markdown content.

Suggested reviewers: vershwal, evanhahn

Priority: ➖ Normal

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to c684b

No merge-blocking issue is established for the custom Markdown routes. Complete normal checks before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to c684b

The new Markdown URLs reuse existing gift, subscription and payment checks. No introduced access bypass was established, but complete verification of protected-content serialization and all response-cache behavior remains incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed exposure is additional public Markdown URLs for a site’s qualifying configured posts and pages, including protected entries. The inspected flow carries the resolved entry ID and resource type into payment handling; it does not introduce a caller-selected tenant or arbitrary privileged resource lookup.

Trust Boundaries and Controls

• observed — The new controller rejects gift requests before fetching content. Both fetchEntry and the existing entry lookup propagate member identity into API context. The shared Markdown handler refuses full members-only rendering and sends paid or tiered entries through payment handling; entries already carrying full HTML do not receive the unpaid preview callback.
• inferred — The inspected payment adapters derive payment scope from the requested pathname, while fulfillment reloads the entry by its supplied ID and resource type. Route-specific Content-Location therefore does not itself grant payment authority or select another entry. Cross-alias credential reuse was assessed from source, not an inspected end-to-end alias test.

Resilience and Maintainability Implications

• observed — The existing fulfillment sequence checks deliverability, verifies payment, records replay state and then renders full Markdown. The inspected production factory injects the replay repository, whose duplicate and unique-constraint race handling reject reused credentials. Ledger failures and replay return errors before full rendering. Paid responses use private, no-store; payment challenges and problem responses use no-store. Settlement, recording and response delivery are not one atomic transaction, and that ordering is unchanged by this PR.

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
New Files Are Typescript	❌ Error	The PR adds ghost/core/test/e2e-frontend/markdown-routes.test.js as a new JavaScript test source file. The diff marks it as added, and it is not under any listed exception path. The PR adds no exemp…	Convert the new end-to-end test to TypeScript (for example, markdown-routes.test.ts) and use TypeScript-compatible imports and types, or place it under an explicitly exempt category if that is appropriate.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the fix for incorrect Markdown redirect targets on routes.yaml-remapped pages.
Description check	✅ Passed	The description explains the routing bug, the fix, and the added test coverage. It is directly related to the changeset.
Linked Issues check	✅ Passed	Issue #30375 requires working Markdown URLs for pages remapped through routes.yaml, including collection data routes and the site root. ParentRouter computes routed .md paths with `getMarkdownPa…
Out of Scope Changes check	✅ Passed	The data lookup helper, route-entry resolution, canonical URL handling, route registration, and tests support Markdown serving for routes.yaml remaps. The changes stay within issue #30375. No unrela…
Type-Safe Boundaries	✅ Passed	The PR introduces no any, unchecked as, @ts-nocheck, or @ts-ignore in changed production code. It types route data with the existing RouteData type. Route settings pass through `parseRouteSe…

Full details: New Files Are Typescript

Explanation

The PR adds ghost/core/test/e2e-frontend/markdown-routes.test.js as a new JavaScript test source file. The diff marks it as added, and it is not under any listed exception path. The PR adds no exemption for test files.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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