Skip to content

Moved the devcontainer and Amp to the host dev flow - #31746

Merged
9larsons merged 4 commits into
mainfrom
slars/intelligent-jepsen-c7b658
Oct 10, 2026
Merged

9larsons merged 4 commits into
mainfrom
slars/intelligent-jepsen-c7b658

Conversation

@9larsons

@9larsons 9larsons commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

no ref

The devcontainer and Amp were the last users of the container flow (pnpm dev:docker, the ghost-dev image and the Caddy gateway), so that flow couldn't be deleted.

  • The devcontainer is now its own compose service on the stock node:22.23.3-bookworm image. It runs pnpm dev against MySQL, Redis and Mailpit by service name, and publishes 2368 because Codespaces forwards ports from the host.
  • Codespaces' forwarded URL rewrites a same-origin Origin to localhost:2368 but leaves the Referer alone, so Admin failed Ghost's origin check whichever url Ghost had. The Admin dev server now undoes that rewrite, and Ghost keeps the forwarded https url.
  • Amp runs pnpm dev on its portal port with url=$PUBLIC_URL.
  • Removed devcontainer-build.yml and compose.dev.orb.yaml. The VS Code dev tasks now run pnpm dev.

Tested in a real Codespace with browser-shaped requests. Login, users/me, post create, image upload and load, the site and HMR all work, and foreign origins are still rejected. VS Code desktop's localhost forward fails the origin check, as it did before. Amp is untested.

no ref

The devcontainer and Amp were the last users of the containerised `pnpm dev:docker` flow. Running the same `pnpm dev` as a laptop lets that flow be deleted later without breaking either, and drops the Caddy gateway and the ghost-dev image build from both.
@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: c5fc9c8e-728f-42f6-b787-cb085ce61e31

📥 Commits

Reviewing files that changed from the base of the PR and between 417e2f9 and baa1cf6.


📒 Files selected for processing (11)
  • .amp/services.yaml
  • .devcontainer/compose.devcontainer.yaml
  • .devcontainer/devcontainer.json
  • .devcontainer/postCreate.sh
  • .devcontainer/start-dev-stack.sh
  • .github/workflows/devcontainer-build.yml
  • .vscode/tasks.json
  • apps/admin/vite-front-door.ts
  • compose.dev.orb.yaml
  • compose.dev.yaml
  • scripts/README.md

💤 Files with no reviewable changes (2)
  • .github/workflows/devcontainer-build.yml
  • compose.dev.orb.yaml

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


📜 Recent review details
⏰ Context from checks skipped due to timeout. (16)
  • GitHub Check: Build Ghost-CLI archive
  • GitHub Check: App Playwright Acceptance Tests (@tryghost/admin 2/3)
  • GitHub Check: App Playwright Acceptance Tests (@tryghost/koenig-lexical 1/1)
  • GitHub Check: App Playwright Acceptance Tests (@tryghost/signup-form 1/1)
  • GitHub Check: App Playwright Acceptance Tests (@tryghost/admin 3/3)
  • GitHub Check: App Playwright Acceptance Tests (@tryghost/kg-unsplash-selector 1/1)
  • GitHub Check: App Playwright Acceptance Tests (@tryghost/admin 1/3)
  • GitHub Check: App Playwright Acceptance Tests (@tryghost/activitypub 1/1)
  • GitHub Check: Unit tests (Node 22.23.3)
  • GitHub Check: Acceptance tests (Node 24.20.0, mysql8)
  • GitHub Check: Build Docker Images
  • GitHub Check: Legacy tests (Node 24.20.0, mysql8)
  • GitHub Check: Legacy tests (Node 22.23.3, mysql8)
  • GitHub Check: Typecheck
  • GitHub Check: Lint
  • GitHub Check: Analyze (javascript-typescript)

🧰 Additional context used
📚 Code guidelines (4)
docs/practices/internationalization.md — configured
docs/codebase/direction.md — auto-discovered
docs/practices/error-handling.md — configured
docs/codebase/monorepo-structure.md — configured

📓 Path-based instructions (7)
Review Admin UI for existing Shade reuse, correct component layer, semantic tokens, and accessible interaction states.

⚙️ CodeRabbit configuration file

Files:

  • apps/admin/vite-front-door.ts

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:

  • apps/admin/vite-front-door.ts

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

⚙️ CodeRabbit configuration file

Files:

  • scripts/README.md
  • compose.dev.yaml
  • apps/admin/vite-front-door.ts

Source excerpt: This extracts source strings, updates all locale files, and synchronizes `packages/i18n/locales/context.json`.

📄 CodeRabbit inference engine (docs/practices/internationalization.md)

Files:

  • apps/admin/vite-front-door.ts

Source excerpt: Build new Admin UI in [`apps/admin/`](../../apps/admin/) with `admin-x-framework` for API access and Shade for UI.

📄 CodeRabbit inference engine (docs/codebase/direction.md)

Files:

  • apps/admin/vite-front-door.ts

Source excerpt: Use that fallback when there is no safe, useful message.

📄 CodeRabbit inference engine (docs/practices/error-handling.md)

Files:

  • apps/admin/vite-front-door.ts

Source excerpt: [`pnpm-workspace.yaml`](../../pnpm-workspace.yaml) is the source of truth for which directories are workspaces.

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

Files:

  • apps/admin/vite-front-door.ts

🪛 ast-grep (0.45.3)
.devcontainer/start-dev-stack.sh

[warning] 24-24: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/ghost-dev.log
Note: [CWE-377] Insecure Temporary File.

(predictable-tmp-file-bash)


[warning] 25-25: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/ghost-dev.log
Note: [CWE-377] Insecure Temporary File.

(predictable-tmp-file-bash)


🔇 Additional comments (1)
apps/admin/vite-front-door.ts (1)

87-88: 🔒 Security & Privacy | 🛡️ Detected with Advanced Tier

Use a trusted source for forwarded origin headers.

apps/admin/vite-front-door.ts copies x-forwarded-host and x-forwarded-proto into Origin when the incoming Origin matches the local Host. Ghost uses that value for admin-origin and session-origin checks. The local Host comparison does not validate the forwarded values. Restrict this rewrite to a documented trusted proxy, or validate the reconstructed origin against the configured public origin before forwarding.



Walkthrough

The development setup now uses a Node-based devcontainer and pnpm dev as its development entry point. The devcontainer startup script and VS Code task no longer launch separate backend and frontend processes. The Admin front-door middleware updates matching request origins using forwarded headers. The devcontainer image workflow and Orb Compose override were removed.

Suggested reviewers: acburdine

Priority: ⬇️ Low

Change: Refactor

Merge Risk | ⚪ Minimal · up to baa1c

Merge Risk: ⚪ Minimal · up to baa1c

This change moves the devcontainer and Amp to the host pnpm dev flow and adds Origin handling for Codespaces forwarding. No concrete merge-blocking risk was found; the change affects development setup only.

Security Architecture Review

Security architecture risk: 🔵 Low · up to baa1c

The change is confined to development environments, with no demonstrated authentication bypass or newly granted host privileges. However, the trustworthiness of forwarded request headers remains unresolved, and the Amp flow has not been tested.

Retained concerns

  • Medium · security · inferred: The new Origin restoration promotes forwarded-header values into identity consumed by Ghost's CSRF checks without locally establishing an authorized forwarding peer. External header sanitization and an effective browser attack path remain unproven. This is an unresolved trust-boundary concern, not a verified authentication bypass.
Security review details

Security Blast Radius

  • inferred — The identified concern affects development Ghost staff-session and Admin operations behind the affected front door. An authenticated CSRF outcome would require a victim session and a browser-reachable way to satisfy the rewrite guard with effective forwarded headers. No path from this candidate to Docker-host control or production environments is established.

Security Findings and Attack Paths

  • inferred — The forwarded-origin candidate remains deferred. Repository source establishes the normalization and downstream enforcement, but not whether Codespaces or other forwarding boundaries replace client-supplied forwarded headers or permit an effective browser attack. The unresolved candidate is not a verified vulnerability, and reported foreign-origin rejection does not resolve that separate provenance question.

Trust Boundaries and Controls

  • observed — The rewrite requires a string forwarded host and an Origin exactly equal to http://Host; it does not normalize ordinary foreign Origins. Ghost still enforces configured and session origins and resolves an active user from the session. HTTPS configurations use Secure, HttpOnly cookies with SameSite=None, so SameSite is not an independent cross-site exclusion for that configuration.

Hardening Proposals

  • proposed — Establish and document the forwarding layer's header-overwrite and endpoint-access guarantees for Codespaces and Amp. Consider restricting Origin restoration to a trusted forwarding path and canonical public origin, with browser-shaped checks for forged, conflicting, and missing forwarded headers. These are boundary-assurance proposals, not observed exploit findings.

Pre-merge checks | Passed 5 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Type-Safe Boundaries Warning The PR adds HTTP-boundary handling in apps/admin/vite-front-door.ts at lines 82-88. It reads x-forwarded-host, x-forwarded-proto, origin, and host, then constructs a new Origin. The code c… Add a boundary schema for the relevant request headers. Validate x-forwarded-host as a valid host and x-forwarded-proto against the allowed protocol values before constructing Origin. Validate the other header values used in the compa…
✅ Passed checks (5 passed)
Check name Status Explanation
Title check Passed The title clearly summarizes the primary change: moving the devcontainer and Amp to the host development flow.
Description check Passed The description directly explains the devcontainer, Amp, Codespaces origin handling, removed files, updated tasks, and testing results.
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.
New Files Are Typescript Passed The pull-request diff adds no files. It only modifies existing files and deletes two files. Therefore, it does not add a new .js, .jsx, .cjs, or .mjs source file.

Full details: Type-Safe Boundaries

Explanation

The PR adds HTTP-boundary handling in apps/admin/vite-front-door.ts at lines 82-88. It reads x-forwarded-host, x-forwarded-proto, origin, and host, then constructs a new Origin. The code checks only that two values are strings. It does not validate that the host and protocol have valid, allowed formats before using them. No Zod schema or equivalent boundary validator is present. The changed code adds no any, unchecked as, @ts-nocheck, or @ts-ignore.

Resolution

Add a boundary schema for the relevant request headers. Validate x-forwarded-host as a valid host and x-forwarded-proto against the allowed protocol values before constructing Origin. Validate the other header values used in the comparison, and skip the rewrite when parsing fails. Use the parsed values only after successful validation.


  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR


🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@nx-cloud

nx-cloud Bot commented Oct 10, 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 baa1cf6

Command Status Duration Result
nx run @tryghost/admin:test:acceptance --shard=3/3 ✅ Succeeded 6m 11s View ↗
nx run @tryghost/admin:test:acceptance --shard=2/3 ✅ Succeeded 5m 41s View ↗
nx run @tryghost/admin:test:acceptance --shard=1/3 ✅ Succeeded 4m 41s View ↗
nx run ghost:test:integration ✅ Succeeded 1m 57s View ↗
nx run ghost:test:ci:unit ✅ Succeeded 1s View ↗
nx run-many -t test:unit -p @tryghost/adapter-b... ✅ Succeeded 3m 7s View ↗
nx run ghost:test:e2e ✅ Succeeded 3m 26s View ↗
nx run @tryghost/koenig-lexical:test:acceptance... ✅ Succeeded 2m 26s View ↗
Additional runs (16) ✅ Succeeded ... View ↗

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


☁️ Nx Cloud last updated this comment at 2026-10-10 18:50:15 UTC

@codecov

codecov Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.15%. Comparing base (5320c67) to head (baa1cf6).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #31746      +/-   ##
==========================================
+ Coverage   85.13%   85.15%   +0.02%     
==========================================
  Files        1365     1365              
  Lines       51276    51276              
  Branches     8809     8809              
==========================================
+ Hits        43652    43665      +13     
+ Misses       6544     6511      -33     
- Partials     1080     1100      +20     
Flag Coverage Δ
e2e-tests 72.35% <ø> (-0.06%) ⬇️
unit-tests 67.43% <ø> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

no ref

Codespaces forwards ports from the VM host, not from inside the devcontainer, so the forwarded 2368 URL returned 502 until the devcontainer service published the port. The gateway used to publish it.
no ref

Codespaces' forwarded URL rewrites Host and same-origin Origin headers to localhost:2368, so Ghost rejected Admin sign-in and every write against the https url the script set. Ghost's default localhost url matches what it receives.
no ref

Codespaces' port forwarding rewrites a same-origin Origin to localhost:2368 but leaves the Referer alone, so whichever url Ghost had, half of Admin's requests failed its origin check. With the https url back and the Admin dev server undoing the Origin rewrite, Admin works through the forwarded URL and Ghost's images and site links point at it.
@9larsons
9larsons marked this pull request as ready for review October 10, 2026 18:43
@9larsons
9larsons enabled auto-merge (squash) October 10, 2026 18:43
@9larsons
9larsons merged commit c9e0cfa into main Oct 10, 2026
68 checks passed
@9larsons
9larsons deleted the slars/intelligent-jepsen-c7b658 branch October 10, 2026 18:53
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