Skip to content

Conversation

@mike182uk
Copy link
Member

ref TryGhost/ActivityPub#1487

Added ActivityPub routing and HMR support for external domains

  • Added dev-gateway routing for ActivityPub API (/.ghost/activitypub/*) and federation endpoints (/.well-known/webfinger, /.well-known/nodeinfo)
  • Fixed HMR WebSocket connections through reverse proxies by updating vite-deep-links.ts to skip WebSocket upgrades and root path requests
  • Changed allowedHosts to true to support external domains (Tailscale, etc.)
  • Added /ghost/.well-known/* route to Ghost backend for JWKS endpoint
  • Simplified admin reverse proxy config (Caddy handles WebSocket automatically)
ref TryGhost/ActivityPub#1487

Added ActivityPub routing and HMR support for external domains
- Added `dev-gateway` routing for ActivityPub API (`/.ghost/activitypub/*`)
  and federation endpoints (`/.well-known/webfinger`, `/.well-known/nodeinfo`)
- Fixed HMR WebSocket connections through reverse proxies by updating
  `vite-deep-links.ts` to skip WebSocket upgrades and root path requests
- Changed `allowedHosts` to `true` to support external domains (Tailscale, etc.)
- Added `/ghost/.well-known/*` route to Ghost backend for JWKS endpoint
- Simplified admin reverse proxy config (Caddy handles WebSocket automatically)
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 18, 2025

Walkthrough

This pull request introduces ActivityPub integration to the development gateway infrastructure. Changes include modifying the Vite admin server configuration to use permissive host allowlisting, adding ActivityPub proxy routing rules to the Caddyfile for federation endpoints (/.ghost/activitypub/*, /.well-known/webfinger, /.well-known/nodeinfo), configuring the ActivityPub proxy target in the Dockerfile, and updating documentation with the new routing rules and environment variable requirements.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • vite.config.ts host allowlist change: Verify the security implications of switching from explicit host whitelist to permissive mode (true) and confirm this is intentional for development environments
  • Caddyfile proxy routing: Review the new ActivityPub proxy blocks and routing order; verify proxy headers (Host, X-Real-IP, X-Forwarded-For, X-Forwarded-Proto) are correctly configured for federation
  • Admin ghost routing modification: Confirm the /ghost* pattern change (from /ghost/*) and HMR header adjustments don't break existing admin functionality
  • Proxy ordering dependencies: Verify that positioning JWKS and ActivityPub blocks before general /ghost/assets/* handling maintains expected precedence and doesn't introduce routing conflicts

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding ActivityPub support and improving the dev workflow, which aligns with the actual changes across the modified files.
Description check ✅ Passed The description is directly related to the changeset, providing specific details about ActivityPub routing, HMR fixes, vite configuration changes, and Gateway modifications.
✨ Finishing touches
�� Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ap-ghost-local-dev

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 and usage tips.

Copy link
Member

@jonatansberg jonatansberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this looks really good. I have a couple of questions that would be relevant to go over before merging.

"::1",
"host.docker.internal",
]
allowedHosts: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for changing this? I went for the more restrictive list based on the recommendations in the Vite docs. In practice, this should probably be fine as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was to allow support for external domains (i.e when funneling via tailscale)

Comment on lines 19 to 30
// Skip WebSocket upgrade requests (used for HMR)
if (req.headers.upgrade?.toLowerCase() === 'websocket') {
next();
return;
}

// Skip root path with query string only (e.g. /ghost/?token=xxx for Vite HMR)
const urlPath = req.originalUrl?.split('?')[0];
if (urlPath === `${base}/` || urlPath === base) {
next();
return;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need to handle the token case in the gateway rather than this middleware? I think the same goes for the WebSocket thing. If that has made it all the way here, then something's off, no? 🤔

Copy link
Member Author

@mike182uk mike182uk Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh this ain't needed at all 😅

I think I added it because I couldn't get the Caddy changes working at one point, but works fine without this so all good

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
docker/dev-gateway/Caddyfile (1)

172-180: JWKS endpoint routing is correctly positioned.

The handler is properly placed before the general /ghost* catch-all to ensure it's matched correctly. The configuration follows the same pattern as other Ghost backend routes.

Optional: Consider clarifying the comment to reflect the wildcard pattern

The comment mentions "JWKS endpoint" (singular) but the path pattern /ghost/.well-known/* suggests it may serve multiple well-known endpoints. Consider updating the comment if multiple endpoints are expected:

-    # JWKS endpoint - must go to Ghost backend for JWT verification
+    # Well-known endpoints (e.g., JWKS) - must go to Ghost backend for JWT verification
📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d3e136 and e29f1cb.

📒 Files selected for processing (1)
  • docker/dev-gateway/Caddyfile (3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use React with Vite for Admin app development (admin-x-settings, admin-x-activitypub, posts, stats)
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Ghost admin serves admin-x apps from `/ghost/assets/{app-name}/{app-name}.js` URLs

Applied to files:

  • docker/dev-gateway/Caddyfile
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Frontend dev servers and foundation libraries run on host machine during `yarn dev:forward`, while Ghost Core backend, MySQL, Redis, Mailpit, and Caddy run in Docker

Applied to files:

  • docker/dev-gateway/Caddyfile
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.

Applied to files:

  • docker/dev-gateway/Caddyfile
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: E2E Tests (Ember 2/4)
  • GitHub Check: [Optional] E2E Tests (React 2/4)
  • GitHub Check: E2E Tests (Ember 4/4)
  • GitHub Check: [Optional] E2E Tests (React 1/4)
  • GitHub Check: E2E Tests (Ember 1/4)
  • GitHub Check: [Optional] E2E Tests (React 4/4)
  • GitHub Check: E2E Tests (Ember 3/4)
  • GitHub Check: [Optional] E2E Tests (React 3/4)
🔇 Additional comments (2)
docker/dev-gateway/Caddyfile (2)

184-189: Admin handler simplification aligns with PR objectives.

The path change from /ghost/* to /ghost* now matches requests to /ghost without a trailing slash, and the simplified header configuration relies on Caddy's built-in WebSocket support for HMR.

These changes align with the PR description: "Simplified admin reverse proxy configuration, relying on Caddy to handle WebSocket automatically" and "Fixed HMR WebSocket connections through reverse proxies."


49-78: ActivityPub routing is correctly configured.

The three ActivityPub endpoints (API, WebFinger, NodeInfo) are properly set up with appropriate headers for federation. The ACTIVITYPUB_PROXY_TARGET environment variable is already defined in the Dockerfile with a sensible default (host.docker.internal:8080), and positioning of these handlers ensures correct route matching before the general /ghost* handler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants