diff --git a/.changeset/fix-cloudflare-static-output.md b/.changeset/fix-cloudflare-static-output.md new file mode 100644 index 000000000000..5b6b635d3e78 --- /dev/null +++ b/.changeset/fix-cloudflare-static-output.md @@ -0,0 +1,7 @@ +--- +'@astrojs/cloudflare': patch +--- + +Fixes deployment of static sites with the Cloudflare adapter + +Fixes an issue with detecting and building fully static sites that caused deployment errors when using `output: 'static'` with the Cloudflare adapter \ No newline at end of file diff --git a/.changeset/harden-merge-responses-cookies.md b/.changeset/harden-merge-responses-cookies.md new file mode 100644 index 000000000000..4b9bd521ff92 --- /dev/null +++ b/.changeset/harden-merge-responses-cookies.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes cookie handling during error page rendering to ensure cookies set by middleware are consistently included in the response diff --git a/.changeset/harden-xff-allowed-domains.md b/.changeset/harden-xff-allowed-domains.md new file mode 100644 index 000000000000..655133d29546 --- /dev/null +++ b/.changeset/harden-xff-allowed-domains.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Hardens `clientAddress` resolution to respect `security.allowedDomains` for `X-Forwarded-For`, consistent with the existing handling of `X-Forwarded-Host`, `X-Forwarded-Proto`, and `X-Forwarded-Port`. The `X-Forwarded-For` header is now only used to determine `Astro.clientAddress` when the request's host has been validated against an `allowedDomains` entry. Without a matching domain, `clientAddress` falls back to the socket's remote address. diff --git a/.changeset/preserve-directory-structure.md b/.changeset/preserve-directory-structure.md new file mode 100644 index 000000000000..1999a26d9f66 --- /dev/null +++ b/.changeset/preserve-directory-structure.md @@ -0,0 +1,29 @@ +--- +'astro': minor +--- + +Adds `preserveBuildClientDir` option to adapter features + +Adapters can now opt in to preserving the client/server directory structure for static builds by setting `preserveBuildClientDir: true` in their adapter features. When enabled, static builds will output files to `build.client` instead of directly to `outDir`. + +This is useful for adapters that require a consistent directory structure regardless of the build output type, such as deploying to platforms with specific file organization requirements. + +```js +// my-adapter/index.js +export default function myAdapter() { + return { + name: 'my-adapter', + hooks: { + 'astro:config:done': ({ setAdapter }) => { + setAdapter({ + name: 'my-adapter', + adapterFeatures: { + buildOutput: 'static', + preserveBuildClientDir: true + } + }); + } + } + }; +} +``` \ No newline at end of file diff --git a/.changeset/red-quail-bite.md b/.changeset/red-quail-bite.md new file mode 100644 index 000000000000..0dca2db5d325 --- /dev/null +++ b/.changeset/red-quail-bite.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes an issue where a session ID from a cookie with no matching server-side data was accepted as-is. The session now generates a new ID when the cookie value has no corresponding storage entry. diff --git a/.changeset/redirect-catch-all-hardening.md b/.changeset/redirect-catch-all-hardening.md new file mode 100644 index 000000000000..4566f24c0d61 --- /dev/null +++ b/.changeset/redirect-catch-all-hardening.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Hardens config-based redirects with catch-all parameters to prevent producing protocol-relative URLs (e.g. `//example.com`) in the `Location` header diff --git a/.changeset/soft-vans-ask.md b/.changeset/soft-vans-ask.md new file mode 100644 index 000000000000..5f91de4aa7df --- /dev/null +++ b/.changeset/soft-vans-ask.md @@ -0,0 +1,5 @@ +--- +'@astrojs/node': patch +--- + +Hardens static file handler path resolution to ensure resolved paths stay within the client directory diff --git a/packages/astro/src/core/app/base.ts b/packages/astro/src/core/app/base.ts index 2782b0c52f2e..9c8300b07218 100644 --- a/packages/astro/src/core/app/base.ts +++ b/packages/astro/src/core/app/base.ts @@ -23,7 +23,12 @@ import { REWRITE_DIRECTIVE_HEADER_KEY, ROUTE_TYPE_HEADER, } from '../constants.js'; -import { getSetCookiesFromResponse } from '../cookies/index.js'; +import { + AstroCookies, + attachCookiesToResponse, + getSetCookiesFromResponse, +} from '../cookies/index.js'; +import { getCookiesFromResponse } from '../cookies/response.js'; import { AstroError, AstroErrorData } from '../errors/index.js'; import { consoleLogDestination } from '../logger/console.js'; import { AstroIntegrationLogger, Logger } from '../logger/core.js'; @@ -726,16 +731,24 @@ export abstract class BaseApp

{ // this function could throw an error... originalResponse.headers.delete('Content-type'); } catch {} - // we use a map to remove duplicates - const mergedHeaders = new Map([ - ...Array.from(newResponseHeaders), - ...Array.from(originalResponse.headers), - ]); + // Build merged headers using append() to preserve multi-value headers (e.g. Set-Cookie). + // Headers from the original response take priority over new response headers for + // single-value headers, but we use append to avoid collapsing multi-value entries. const newHeaders = new Headers(); - for (const [name, value] of mergedHeaders) { - newHeaders.set(name, value); + const seen = new Set(); + // Add original response headers first (they take priority) + for (const [name, value] of originalResponse.headers) { + newHeaders.append(name, value); + seen.add(name.toLowerCase()); + } + // Add new response headers that weren't already set by the original response, + // but skip content-type since the error page must return text/html + for (const [name, value] of newResponseHeaders) { + if (!seen.has(name.toLowerCase())) { + newHeaders.append(name, value); + } } - return new Response(newResponse.body, { + const mergedResponse = new Response(newResponse.body, { status, statusText: status === 200 ? newResponse.statusText : originalResponse.statusText, // If you're looking at here for possible bugs, it means that it's not a bug. @@ -745,6 +758,24 @@ export abstract class BaseApp

{ // Although, we don't want it to replace the content-type, because the error page must return `text/html` headers: newHeaders, }); + + // Transfer AstroCookies from the original or new response so that + // #prepareResponse can read them when addCookieHeader is true. + const originalCookies = getCookiesFromResponse(originalResponse); + const newCookies = getCookiesFromResponse(newResponse); + if (originalCookies) { + // If both responses have cookies, merge new response cookies into original + if (newCookies) { + for (const cookieValue of AstroCookies.consume(newCookies)) { + originalResponse.headers.append('set-cookie', cookieValue); + } + } + attachCookiesToResponse(mergedResponse, originalCookies); + } else if (newCookies) { + attachCookiesToResponse(mergedResponse, newCookies); + } + + return mergedResponse; } getDefaultStatusCode(routeData: RouteData, pathname: string): number { diff --git a/packages/astro/src/core/app/node.ts b/packages/astro/src/core/app/node.ts index 716ec5205f76..b58db09ec2eb 100644 --- a/packages/astro/src/core/app/node.ts +++ b/packages/astro/src/core/app/node.ts @@ -141,8 +141,14 @@ export function createRequest( } // Get the IP of end client behind the proxy. + // Only trust X-Forwarded-For when the request's host was validated against allowedDomains, + // meaning it arrived through a trusted proxy. Without this check, any client can spoof + // their IP via this header. // @example "1.1.1.1,8.8.8.8" => "1.1.1.1" - const forwardedClientIp = getFirstForwardedValue(req.headers['x-forwarded-for']); + const hostValidated = validated.host !== undefined || validatedHostname !== undefined; + const forwardedClientIp = hostValidated + ? getFirstForwardedValue(req.headers['x-forwarded-for']) + : undefined; const clientIp = forwardedClientIp || req.socket?.remoteAddress; if (clientIp) { Reflect.set(request, clientAddressSymbol, clientIp); diff --git a/packages/astro/src/core/build/common.ts b/packages/astro/src/core/build/common.ts index 326450b1ba7f..18a3041e6229 100644 --- a/packages/astro/src/core/build/common.ts +++ b/packages/astro/src/core/build/common.ts @@ -9,7 +9,9 @@ const STATUS_CODE_PAGES = new Set(['/404', '/500']); const FALLBACK_OUT_DIR_NAME = './.astro/'; function getOutRoot(astroSettings: AstroSettings): URL { - if (astroSettings.buildOutput === 'static') { + const preserveStructure = astroSettings.adapter?.adapterFeatures?.preserveBuildClientDir; + + if (astroSettings.buildOutput === 'static' && !preserveStructure) { return new URL('./', astroSettings.config.outDir); } else { return new URL('./', astroSettings.config.build.client); diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 8d160bcfe356..4a8253a63668 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -544,7 +544,9 @@ function checkPublicConflict( ): boolean { const outFilePath = fileURLToPath(outFile); const outRoot = fileURLToPath( - settings.buildOutput === 'static' ? settings.config.outDir : settings.config.build.client, + settings.buildOutput === 'static' && !settings.adapter?.adapterFeatures?.preserveBuildClientDir + ? settings.config.outDir + : settings.config.build.client, ); const relativePath = outFilePath.slice(outRoot.length); const publicFilePath = new URL(relativePath, settings.config.publicDir); diff --git a/packages/astro/src/core/build/static-build.ts b/packages/astro/src/core/build/static-build.ts index 01b4e8f77b25..97627be4f953 100644 --- a/packages/astro/src/core/build/static-build.ts +++ b/packages/astro/src/core/build/static-build.ts @@ -533,10 +533,12 @@ async function ssrMoveAssets( ) { opts.logger.info('build', 'Rearranging server assets...'); const isFullyStaticSite = opts.settings.buildOutput === 'static'; + const preserveStructure = opts.settings.adapter?.adapterFeatures?.preserveBuildClientDir; const serverRoot = opts.settings.config.build.server; - const clientRoot = isFullyStaticSite - ? opts.settings.config.outDir - : opts.settings.config.build.client; + const clientRoot = + isFullyStaticSite && !preserveStructure + ? opts.settings.config.outDir + : opts.settings.config.build.client; // Move prerender assets const prerenderAssetsToMove = getSSRAssets(internals, ASTRO_VITE_ENVIRONMENT_NAMES.prerender); diff --git a/packages/astro/src/core/redirects/render.ts b/packages/astro/src/core/redirects/render.ts index eba4395b36ca..00a4543c109b 100644 --- a/packages/astro/src/core/redirects/render.ts +++ b/packages/astro/src/core/redirects/render.ts @@ -2,13 +2,15 @@ import type { RedirectConfig } from '../../types/public/index.js'; import type { RenderContext } from '../render-context.js'; import { getRouteGenerator } from '../routing/generator.js'; +function isExternalURL(url: string): boolean { + return url.startsWith('http://') || url.startsWith('https://') || url.startsWith('//'); +} + export function redirectIsExternal(redirect: RedirectConfig): boolean { if (typeof redirect === 'string') { - return redirect.startsWith('http://') || redirect.startsWith('https://'); + return isExternalURL(redirect); } else { - return ( - redirect.destination.startsWith('http://') || redirect.destination.startsWith('https://') - ); + return isExternalURL(redirect.destination); } } diff --git a/packages/astro/src/core/routing/generator.ts b/packages/astro/src/core/routing/generator.ts index 94dcf4324465..7ee9d8806aac 100644 --- a/packages/astro/src/core/routing/generator.ts +++ b/packages/astro/src/core/routing/generator.ts @@ -1,3 +1,4 @@ +import { collapseDuplicateLeadingSlashes } from '@astrojs/internal-helpers/path'; import type { AstroConfig } from '../../types/public/config.js'; import type { RoutePart } from '../../types/public/internal.js'; @@ -41,7 +42,7 @@ function getParameter(part: RoutePart, params: Record): function getSegment(segment: RoutePart[], params: Record): string { const segmentPath = segment.map((part) => getParameter(part, params)).join(''); - return segmentPath ? '/' + segmentPath : ''; + return segmentPath ? collapseDuplicateLeadingSlashes('/' + segmentPath) : ''; } type RouteGenerator = (data?: any) => string; diff --git a/packages/astro/src/core/session/runtime.ts b/packages/astro/src/core/session/runtime.ts index d814ca95e528..a72f12ffa232 100644 --- a/packages/astro/src/core/session/runtime.ts +++ b/packages/astro/src/core/session/runtime.ts @@ -53,6 +53,8 @@ export class AstroSession { #dirty = false; // Whether the session cookie has been set. #cookieSet = false; + // Whether the session ID was sourced from a client cookie rather than freshly generated. + #sessionIDFromCookie = false; // The local data is "partial" if it has not been loaded from storage yet and only // contains values that have been set or deleted in-memory locally. // We do this to avoid the need to block on loading data when it is only being set. @@ -242,6 +244,7 @@ export class AstroSession { // Create new session this.#sessionID = crypto.randomUUID(); + this.#sessionIDFromCookie = false; this.#data = data; this.#dirty = true; await this.#setCookie(); @@ -349,6 +352,16 @@ export class AstroSession { // We stored this as a devalue string, but unstorage will have parsed it as JSON const raw = await storage.get(this.#ensureSessionID()); if (!raw) { + if (this.#sessionIDFromCookie) { + // The session ID was supplied by the client cookie but has no corresponding + // server-side data. Generate a new server-controlled ID rather than + // accepting an unrecognized value from the client. + this.#sessionID = crypto.randomUUID(); + this.#sessionIDFromCookie = false; + if (this.#cookieSet) { + await this.#setCookie(); + } + } // If there is no existing data in storage we don't need to merge anything // and can just return the existing local data. return this.#data; @@ -404,7 +417,15 @@ export class AstroSession { * Returns the session ID, generating a new one if it does not exist. */ #ensureSessionID() { - this.#sessionID ??= this.#cookies.get(this.#cookieName)?.value ?? crypto.randomUUID(); + if (!this.#sessionID) { + const cookieValue = this.#cookies.get(this.#cookieName)?.value; + if (cookieValue) { + this.#sessionID = cookieValue; + this.#sessionIDFromCookie = true; + } else { + this.#sessionID = crypto.randomUUID(); + } + } return this.#sessionID; } diff --git a/packages/astro/src/integrations/hooks.ts b/packages/astro/src/integrations/hooks.ts index 776ab92ba3e1..7fdf9a4ea2e7 100644 --- a/packages/astro/src/integrations/hooks.ts +++ b/packages/astro/src/integrations/hooks.ts @@ -585,8 +585,11 @@ export async function runHookBuildGenerated({ logger: Logger; routeToHeaders: RouteToHeaders; }) { + const preserveStructure = settings.adapter?.adapterFeatures?.preserveBuildClientDir; const dir = - settings.buildOutput === 'server' ? settings.config.build.client : settings.config.outDir; + settings.buildOutput === 'server' || preserveStructure + ? settings.config.build.client + : settings.config.outDir; for (const integration of settings.config.integrations) { await runHookInternal({ diff --git a/packages/astro/src/prerender/utils.ts b/packages/astro/src/prerender/utils.ts index 7c2dd8fcfeb0..eedbdd47543a 100644 --- a/packages/astro/src/prerender/utils.ts +++ b/packages/astro/src/prerender/utils.ts @@ -19,5 +19,10 @@ export function getServerOutputDirectory(settings: AstroSettings): URL { * Returns the correct output directory of the client build based on the configuration */ export function getClientOutputDirectory(settings: AstroSettings): URL { - return settings.buildOutput === 'server' ? settings.config.build.client : settings.config.outDir; + const preserveStructure = settings.adapter?.adapterFeatures?.preserveBuildClientDir; + + if (settings.buildOutput === 'server' || preserveStructure) { + return settings.config.build.client; + } + return settings.config.outDir; } diff --git a/packages/astro/src/types/public/integrations.ts b/packages/astro/src/types/public/integrations.ts index 66883143572b..ae5c2392db3e 100644 --- a/packages/astro/src/types/public/integrations.ts +++ b/packages/astro/src/types/public/integrations.ts @@ -114,6 +114,15 @@ export interface AstroAdapterFeatures { * for example, to create a `_headers` file for platforms that support it. */ staticHeaders?: boolean; + + /** + * When true, static builds will preserve the client/server directory structure + * instead of outputting directly to outDir. This ensures static builds use + * build.client for assets, maintaining consistency with server builds. + * Useful for adapters that require a specific directory structure regardless + * of the build output type. + */ + preserveBuildClientDir?: boolean; } /** diff --git a/packages/astro/test/astro-attrs.test.js b/packages/astro/test/astro-attrs.test.js deleted file mode 100644 index 23d852d37e33..000000000000 --- a/packages/astro/test/astro-attrs.test.js +++ /dev/null @@ -1,116 +0,0 @@ -import assert from 'node:assert/strict'; -import { before, describe, it } from 'node:test'; -import * as cheerio from 'cheerio'; -import { loadFixture } from './test-utils.js'; - -describe('Attributes', async () => { - let fixture; - - before(async () => { - fixture = await loadFixture({ root: './fixtures/astro-attrs/' }); - await fixture.build(); - }); - - it('Passes attributes to elements as expected', async () => { - const html = await fixture.readFile('/index.html'); - const $ = cheerio.load(html); - - const attrs = { - 'download-true': { attribute: 'download', value: '' }, - 'download-false': { attribute: 'download', value: undefined }, - 'download-undefined': { attribute: 'download', value: undefined }, - 'download-string-empty': { attribute: 'download', value: '' }, - 'download-string': { attribute: 'download', value: 'my-document.pdf' }, - 'popover-auto': { attribute: 'popover', value: 'auto' }, - 'popover-true': { attribute: 'popover', value: '' }, - 'popover-false': { attribute: 'popover', value: undefined }, - 'popover-string-empty': { attribute: 'popover', value: '' }, - // Note: cheerio normalizes boolean `hidden` to the string "hidden", - // so we use "hidden" as the expected value instead of "" - 'hidden-true': { attribute: 'hidden', value: 'hidden' }, - 'hidden-false': { attribute: 'hidden', value: undefined }, - 'hidden-string-empty': { attribute: 'hidden', value: 'hidden' }, - 'boolean-attr-true': { attribute: 'allowfullscreen', value: '' }, - 'boolean-attr-false': { attribute: 'allowfullscreen', value: undefined }, - 'boolean-attr-string-truthy': { attribute: 'allowfullscreen', value: '' }, - 'boolean-attr-string-falsy': { attribute: 'allowfullscreen', value: undefined }, - 'boolean-attr-number-truthy': { attribute: 'allowfullscreen', value: '' }, - 'boolean-attr-number-falsy': { attribute: 'allowfullscreen', value: undefined }, - 'data-attr-true': { attribute: 'data-foobar', value: 'true' }, - 'data-attr-false': { attribute: 'data-foobar', value: 'false' }, - 'data-attr-string-truthy': { attribute: 'data-foobar', value: 'foo' }, - 'data-attr-string-falsy': { attribute: 'data-foobar', value: '' }, - 'data-attr-number-truthy': { attribute: 'data-foobar', value: '1' }, - 'data-attr-number-falsy': { attribute: 'data-foobar', value: '0' }, - 'normal-attr-true': { attribute: 'foobar', value: 'true' }, - 'normal-attr-false': { attribute: 'foobar', value: 'false' }, - 'normal-attr-string-truthy': { attribute: 'foobar', value: 'foo' }, - 'normal-attr-string-falsy': { attribute: 'foobar', value: '' }, - 'normal-attr-number-truthy': { attribute: 'foobar', value: '1' }, - 'normal-attr-number-falsy': { attribute: 'foobar', value: '0' }, - null: { attribute: 'attr', value: undefined }, - undefined: { attribute: 'attr', value: undefined }, - 'html-enum': { attribute: 'draggable', value: 'true' }, - 'html-enum-true': { attribute: 'draggable', value: 'true' }, - 'html-enum-false': { attribute: 'draggable', value: 'false' }, - }; - - // cheerio normalizes hidden="until-found" to just hidden, so we check the raw HTML - assert.ok( - html.includes('id="hidden-until-found" hidden="until-found"'), - 'hidden="until-found" should preserve the attribute value', - ); - assert.ok(!/allowfullscreen=/.test(html), 'boolean attributes should not have values'); - assert.ok( - !/id="data-attr-string-falsy"\s+data-foobar=/.test(html), - "data attributes should not have values if it's an empty string", - ); - assert.ok( - !/id="normal-attr-string-falsy"\s+data-foobar=/.test(html), - "normal attributes should not have values if it's an empty string", - ); - - // cheerio will unescape the values, so checking that the url rendered escaped has to be done manually - assert.equal( - html.includes('https://example.com/api/og?title=hello&description=somedescription'), - true, - ); - - // cheerio will unescape the values, so checking that the url rendered unescaped to begin with has to be done manually - assert.equal( - html.includes('cmd: echo "foo" && echo "bar" > /tmp/hello.txt'), - true, - ); - - for (const id of Object.keys(attrs)) { - const { attribute, value } = attrs[id]; - const attr = $(`#${id}`).attr(attribute); - assert.equal(attr, value, `Expected ${attribute} to be ${value} for #${id}`); - } - }); - - it('Passes boolean attributes to components as expected', async () => { - const html = await fixture.readFile('/component/index.html'); - const $ = cheerio.load(html); - - assert.equal($('#true').attr('attr'), 'attr-true'); - assert.equal($('#true').attr('type'), 'boolean'); - assert.equal($('#false').attr('attr'), 'attr-false'); - assert.equal($('#false').attr('type'), 'boolean'); - }); - - it('Passes namespaced attributes as expected', async () => { - const html = await fixture.readFile('/namespaced/index.html'); - const $ = cheerio.load(html); - - assert.equal($('div').attr('xmlns:happy'), 'https://example.com/schemas/happy'); - assert.equal($('img').attr('happy:smile'), 'sweet'); - }); - - it('Passes namespaced attributes to components as expected', async () => { - const html = await fixture.readFile('/namespaced-component/index.html'); - const $ = cheerio.load(html); - - assert.deepEqual($('span').attr('on:click'), '(event) => console.log(event)'); - }); -}); diff --git a/packages/astro/test/fixtures/astro-attrs/astro.config.mjs b/packages/astro/test/fixtures/astro-attrs/astro.config.mjs deleted file mode 100644 index e7ce274c003a..000000000000 --- a/packages/astro/test/fixtures/astro-attrs/astro.config.mjs +++ /dev/null @@ -1,7 +0,0 @@ -import react from '@astrojs/react'; -import { defineConfig } from 'astro/config'; - -// https://astro.build/config -export default defineConfig({ - integrations: [react()], -}); diff --git a/packages/astro/test/fixtures/astro-attrs/package.json b/packages/astro/test/fixtures/astro-attrs/package.json deleted file mode 100644 index d0085726d054..000000000000 --- a/packages/astro/test/fixtures/astro-attrs/package.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "name": "@test/astro-attrs", - "version": "0.0.0", - "private": true, - "dependencies": { - "@astrojs/react": "workspace:*", - "astro": "workspace:*", - "react": "^18.3.1", - "react-dom": "^18.3.1" - } -} diff --git a/packages/astro/test/fixtures/astro-attrs/src/components/NamespacedSpan.astro b/packages/astro/test/fixtures/astro-attrs/src/components/NamespacedSpan.astro deleted file mode 100644 index bfadf035c518..000000000000 --- a/packages/astro/test/fixtures/astro-attrs/src/components/NamespacedSpan.astro +++ /dev/null @@ -1 +0,0 @@ - diff --git a/packages/astro/test/fixtures/astro-attrs/src/pages/component.astro b/packages/astro/test/fixtures/astro-attrs/src/pages/component.astro deleted file mode 100644 index cfdc636c804e..000000000000 --- a/packages/astro/test/fixtures/astro-attrs/src/pages/component.astro +++ /dev/null @@ -1,8 +0,0 @@ ---- -import Span from '../components/Span.jsx'; ---- - - - - - diff --git a/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro b/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro deleted file mode 100644 index 55503e094d82..000000000000 --- a/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro +++ /dev/null @@ -1,51 +0,0 @@ - - - - - - - -

- - - - -