Skip to content

🐛 Fixed custom routes with :param paths being rejected on upload - #29523

Merged
vershwal merged 1 commit into
mainfrom
princi-hkg-1940-accept-param-notation-in-route-keys
Jul 22, 2026
Merged

🐛 Fixed custom routes with :param paths being rejected on upload#29523
vershwal merged 1 commit into
mainfrom
princi-hkg-1940-accept-param-notation-in-route-keys

Conversation

@vershwal

Copy link
Copy Markdown
Member

ref https://linear.app/ghost/issue/HKG-1940/

Problem

The new route-settings validator rejects :param notation in every path. That's correct for collection permalinks and taxonomy values — but wrong for route and collection keys.

Keys are mounted on Express verbatim. Nothing ever rewrites {param}:param for them, the way activation-bridge does for permalinks and taxonomies. So:

key today
/author/:slug/ real Express param — matches /author/anything/, works
/author/{slug}/ matches the literal /author/%7Bslug%7D/dead route

The error even suggested the rewrite that breaks it:

"/author/:slug/" uses the :param notation. Please use "/author/{slug}/".

Following that advice would silently kill a working route. On one affected site the :param route mounts before taxonomies and shadows Ghost's built-in author router, so any edit there changes every author page.

Legacy validate.js only ever checked /:\w+ on collection permalinks (:316) and taxonomy values (:390) — never on keys. These files have always been valid; only the new validator rejects them.

Change

  • validatePath takes allowParamNotation, set for route and collection keys only. Permalinks and taxonomies still reject :param, matching legacy.
  • The data shorthand accepts %s as the slug. fetch-data substitutes it with the request's slug param on channel routes and collections, which is what makes a wildcard route serve a different resource per request — there is no static slug that could replace it. Exactly one file in the fleet uses it, for per-language author/tag pages.

Both are relaxations of a gate that is stricter than the validator it replaced, so no configuration that validates today stops validating.

Why not fix {} to work instead?

Considered and rejected. Making {param} behave as a wildcard in keys would bring 45 currently-dead routes across 31 live sites to life at once — 22 of which pass validation today. One has /tag/{slug}/ alongside a tag taxonomy (the route would start shadowing it); another has a collection at /{primary_tag}/ that would become a root-level wildcard swallowing the site's entire URL space. Those owners wrote {slug}, got an inert route, and built around the result. Not something to change silently under them.

Impact

  • 10 routes.yaml files in the Pro fleet validate without any edit. Fleet failures: 51 → 41.
  • Uploads accept these forms again, as they did before the migration.
  • No runtime change: the path string is passed through untouched, so routes mount exactly as before. Verified by expanding the affected files through the legacy validator and through the new gate and diffing — identical output.

Testing

  • 211 route-settings unit tests pass.
  • Two tests flipped from throws to accepts; new coverage for %s, for %s rejected when embedded mid-slug, for :param still rejected in a permalink even when the collection key uses it, and a serialize round-trip with a :param key (a YAML mapping key containing a colon, only expressible since this change).

ref https://linear.app/ghost/issue/HKG-1940/

- the new route-settings validator rejected `:param` in every path, but route
  and collection keys are mounted on Express verbatim — nothing rewrites
  `{param}` to `:param` for them the way it does for permalinks and taxonomies
- so `/author/:slug/` is a working wildcard route on live sites, and the error's
  suggested `/author/{slug}/` would mount a literal, unreachable path — the
  "fix" it recommended would have silently killed the route
- legacy validate.js only ever checked `/:\w+` on collection permalinks and
  taxonomy values, so this restores parity there and keeps the tightening where
  legacy also tightened; permalinks and taxonomies still reject :param
- also accepts `%s` as a data shorthand slug: fetch-data substitutes it with the
  request's slug param on channel routes and collections, which is what makes a
  wildcard route serve a different resource per request. There is no static slug
  that could replace it
- 10 files in the Pro fleet depend on these two forms and now validate without
  needing edits; uploads accept them again, as they did before the migration
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: be1a19c8-3c13-475e-b7ef-dfe2535b39fe

📥 Commits

Reviewing files that changed from the base of the PR and between ee01433 and cceb9e6.

📒 Files selected for processing (3)
  • ghost/core/core/server/services/route-settings/route-settings-parser.ts
  • ghost/core/test/unit/server/services/route-settings/route-settings-parser.test.ts
  • ghost/core/test/unit/server/services/route-settings/validate-route-settings.test.ts

Walkthrough

Route settings validation now accepts /:param notation for route and collection path keys while retaining rejection for permalink and taxonomy paths. Shortform route data accepts %s as a complete slug placeholder and rejects embedded placeholders. Tests cover parsing, serialization round trips, valid parameterized paths, invalid permalink notation, and updated error cases.

Suggested reviewers: acburdine, allouis

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: custom routes with :param paths were being rejected on upload.
Description check ✅ Passed The description is directly related to the changeset and accurately explains the validator and %s wildcard route updates.
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.
✨ 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 princi-hkg-1940-accept-param-notation-in-route-keys

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

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

@nx-cloud

nx-cloud Bot commented Jul 22, 2026

Copy link
Copy Markdown

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit cceb9e6

Command Status Duration Result
nx run ghost:test:ci:integration ✅ Succeeded 2m 59s View ↗
nx run ghost:test:integration ✅ Succeeded 3m 16s View ↗
nx run ghost:test:legacy ✅ Succeeded 2m 57s View ↗
nx run ghost:test:e2e ✅ Succeeded 2m 43s View ↗
nx run ghost-monorepo:lint:boundaries ✅ Succeeded 20s View ↗
nx run-many -t test:unit -p ghost ✅ Succeeded 33s View ↗
nx run-many -t lint -p ghost,ghost-monorepo ✅ Succeeded 21s View ↗
nx run-many --target=build --projects=tag:publi... ✅ Succeeded <1s View ↗
nx run @tryghost/admin:build ✅ Succeeded 5s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-07-22 11:54:56 UTC

@vershwal
vershwal merged commit 2ab159a into main Jul 22, 2026
47 checks passed
@vershwal
vershwal deleted the princi-hkg-1940-accept-param-notation-in-route-keys branch July 22, 2026 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant