Skip to content

fix: select CSP by mounted application - #9408

Open
mvanhorn wants to merge 3 commits into
logto-io:masterfrom
mvanhorn:fix/7456-console-path-csp-selection
Open

fix: select CSP by mounted application#9408
mvanhorn wants to merge 3 commits into
logto-io:masterfrom
mvanhorn:fix/7456-console-path-csp-selection

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

Summary

Update the Console/Welcome CSP branch in koa-security-headers.ts so it applies only when the matching admin application is present in mountedApps; otherwise let the request continue to the existing Experience policy. Keep the Console policy itself strict rather than adding production-wide 'unsafe-inline', and preserve the existing Account Center and generic mounted-app branches. Extend koa-security-headers.test.ts with production-oriented assertions that a mounted Console route retains its Console sources without 'unsafe-inline', while an unmounted /console or /welcome fallback receives the Experience policy with the inline SSR allowance.

In self-hosted deployments behind AWS ALB and Google Cloud Run, requesting /console can fall through to the Experience application while koaSecurityHeaders still selects the Console CSP solely from the request path. The Console policy intentionally omits 'unsafe-inline' in production, so it blocks the Experience HTML bootstrap that initializes window.logtoSsr, leaving a blank page and a ReferenceError. The middleware already receives the tenant's mountedApps, which distinguishes a real mounted Console route from an Experience fallback. The bundled issue history shows no assignee, open competing PR, or cross-referenced closed-unmerged attempt.

Fixes #7456

Testing

Not applicable to this change.

Checklist

  • .changeset
    Not run: no test command resolved in this workspace, so nothing was executed to pass.
  • unit tests
  • integration tests
  • necessary TSDoc comments

@mvanhorn
mvanhorn requested a review from simeng-li as a code owner August 10, 2026 10:28
Copilot AI lite review requested due to automatic review settings August 10, 2026 10:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes CSP selection in koaSecurityHeaders so the strict Admin Console CSP is only applied when the corresponding admin app (console / welcome) is actually present in mountedApps, preventing /console and /welcome requests that fall through to the Experience app from being served with an overly strict CSP that breaks SSR bootstrap.

Changes:

  • Gate the Console/Welcome CSP branch on mountedApps.includes(...) in addition to the request path prefix.
  • Add production-focused unit tests to ensure mounted /console//welcome routes keep the strict Console CSP (no 'unsafe-inline') while unmounted fallbacks receive the Experience CSP (includes 'unsafe-inline' for SSR).
  • Add a patch changeset for @logto/core.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/core/src/middleware/koa-security-headers.ts Adjusts CSP selection logic to avoid applying Console CSP based on path alone when the admin app isn’t mounted.
packages/core/src/middleware/koa-security-headers.test.ts Adds regression tests covering mounted vs. unmounted /console and /welcome CSP behavior in production mode.
.changeset/calm-clouds-load.md Publishes a patch changeset documenting the self-hosted CSP/fallback fix.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +301 to +304
(mountedApps.includes(AdminApps.Console) &&
requestPath.startsWith(`/${AdminApps.Console}`)) ||
(mountedApps.includes(AdminApps.Welcome) &&
requestPath.startsWith(`/${AdminApps.Welcome}`))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you clarify what you actually observed? Specifically: which URL you requested (core endpoint vs. admin endpoint), and what the response looked like — blank page, 404, or the sign-in page. I'd like to make sure we're fixing the same thing.

The reason I ask is that in the scenarios listed in the description, koaSecurityHeaders isn't the last middleware to set the CSP header. When /console is requested on a tenant that doesn't mount the Console (e.g. localhost:3001/console, which resolves to the default tenant), the request continues down to koaExperienceSecurityHeaders in Tenant.ts. That middleware guards on the same mountedApps list, so it doesn't early-return either — it calls helmet again, and since helmet uses res.setHeader, the Console policy is replaced rather than merged.

Stepping through the chain on current master, production mode, mountedApps = the default tenant's list, GET /console:

[1] after koaSecurityHeaders:
    script-src 'self' https://cdn.jsdelivr.net/ blob:

[2] after koaExperienceSecurityHeaders:
    script-src 'self' 'unsafe-inline' 'unsafe-hashes' https://accounts.google.com/gsi/client ...

So the header that reaches the browser already carries 'unsafe-inline', and the inline SSR bootstrap that sets window.logtoSsr isn't blocked. The two conditions are the same predicate over the same list, so this holds by construction: any request where the Console branch matches wrongly is a request the experience middleware rewrites. That was fixed in #8778 when the experience CSP moved into its own middleware.

If you're still hitting the blank page, that would point at something the above doesn't cover, and I'd rather understand that case first before we change the branch here.

One separate note on the original report: forwarding /console to port 3002 isn't enough on its own. Tenant resolution matches on origin, not port, so unless ADMIN_ENDPOINT is set to the externally visible origin, the request falls to the default tenant and gets the sign-in page rather than the Console.

Copilot AI review requested due to automatic review settings August 21, 2026 05:16
@charIeszhao
charIeszhao force-pushed the fix/7456-console-path-csp-selection branch from ea3264c to f92edae Compare August 21, 2026 05:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot removed the size/s label Aug 21, 2026
@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown

COMPARE TO master

Total Size Diff 📈 +1.99 KB

Diff by File
Name Diff
.changeset/calm-clouds-load.md 📈 +119 Bytes
packages/core/src/middleware/koa-security-headers.test.ts 📈 +1.78 KB
packages/core/src/middleware/koa-security-headers.ts 📈 +98 Bytes

Copilot AI review requested due to automatic review settings August 21, 2026 12:43
@github-actions github-actions Bot added size/s and removed size/s labels Aug 21, 2026
@mvanhorn

Copy link
Copy Markdown
Contributor Author

Pushed c4380d5 to clear main-lint. The two Object.assign calls in the production-CSP describe block tripped @silverhand/fp/no-mutating-assign; they now carry targeted eslint-disable comments explaining why the shared env is mutated, and the it.each block is reflowed to the repo's formatting. No test behaviour changed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

packages/core/src/middleware/koa-security-headers.ts:304

  • mountedApps is not the set of actually mounted admin apps in the tenant: Tenant adds all AdminApps for every admin tenant, but skips mounting the Console when isMultiTenancy is enabled (packages/core/src/tenants/Tenant.ts:127-130, 187-203). In that mode a /console request that falls through is still given the strict Console CSP, so this condition does not reliably select the Experience policy for an unmounted route. Build mountedApps from the conditional mounts (or otherwise exclude Console/Welcome in this mode) and add coverage for the multi-tenancy case.
      (mountedApps.includes(AdminApps.Console) &&
        requestPath.startsWith(`/${AdminApps.Console}`)) ||
      (mountedApps.includes(AdminApps.Welcome) &&
        requestPath.startsWith(`/${AdminApps.Welcome}`))

Copilot AI review requested due to automatic review settings August 21, 2026 13:22
@github-actions github-actions Bot added size/s and removed size/s labels Aug 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

packages/core/src/middleware/koa-security-headers.ts:303

  • mountedApps is not the set of routes actually mounted in Tenant: it unconditionally adds all AdminApps for the admin tenant (Tenant.ts:127-130), although the Console mount is skipped in multi-tenancy (Tenant.ts:187-203) and there is no Welcome mount in this composition. Consequently /welcome (and /console in multi-tenancy) still takes this strict branch, while koaExperienceSecurityHeaders also treats it as mounted and will not replace the CSP, so the Experience fallback can still block its SSR bootstrap. Please pass an actual mounted-app list, or exclude these entries whenever their mounts are disabled, before selecting the Console policy.
      (mountedApps.includes(AdminApps.Console) &&
        requestPath.startsWith(`/${AdminApps.Console}`)) ||
      (mountedApps.includes(AdminApps.Welcome) && requestPath.startsWith(`/${AdminApps.Welcome}`))

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

Development

Successfully merging this pull request may close these issues.

bug: Uncaught (in promise) ReferenceError: logtoSsr is not defined

3 participants