fix(sessions): validate session ID format before using as storage key (#17718)

Add UUID_REGEX validation in #ensureSessionID() so non-UUID cookie values
are rejected before reaching storage.get(). Previously, a crafted cookie
value matching a non-session key in shared storage could trigger destroy()
and delete that data via storage.removeItem().

Update existing test mocks to use valid UUIDs and add two new tests
verifying that non-UUID values are rejected and valid UUIDs are preserved.

Author: astrobot-houston

.changeset/honest-ears-argue.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes session cookie values not being validated against the expected UUID format before being used as storage keys

packages/astro/src/core/session/runtime.ts

@@ -12,6 +12,7 @@ export const PERSIST_SYMBOL = Symbol();
 
 const DEFAULT_COOKIE_NAME = 'astro-session';
 const VALID_COOKIE_REGEX = /^[\w-]+$/;
+const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
 
 interface SessionEntry {
 	data: any;
@@ -432,7 +433,7 @@ export class AstroSession {
 	#ensureSessionID() {
 		if (!this.#sessionID) {
 			const cookieValue = this.#cookies.get(this.#cookieName)?.value;
-			if (cookieValue) {
+			if (cookieValue && UUID_REGEX.test(cookieValue)) {
 				this.#sessionID = cookieValue;
 				this.#sessionIDFromCookie = true;
 			} else {

packages/astro/test/units/sessions/astro-session.test.ts

@@ -25,7 +25,7 @@ interface MockCookies {
 const defaultMockCookies: MockCookies = {
 	set: () => {},
 	delete: () => {},
-	get: () => ({ value: 'sessionid' }),
+	get: () => ({ value: '00000000-0000-4000-8000-000000000000' }),
 };
 
 const stringify = (data: unknown) => JSON.parse(devalueStringify(data));
@@ -400,7 +400,7 @@ describe('AstroSession - Sparse Data Operations', () => {
 
 	it('should persist delete as the first mutation (no prior get/set)', async () => {
 		const store = new Map<string, string>();
-		const sessionId = 'sessionid';
+		const sessionId = '00000000-0000-4000-8000-000000000000';
 		store.set(sessionId, devalueStringify(new Map([['token', { data: 'secret' }]])));
 
 		const mockStorage = {
@@ -631,7 +631,7 @@ describe('AstroSession - No-Cookie Short Circuit', () => {
 			set: () => {},
 			delete: () => {},
 			get: (name: string) => {
-				if (name === 'test-session') return { value: 'existing-session-id' };
+				if (name === 'test-session') return { value: '00000000-0000-4000-8000-000000000002' };
 				return undefined;
 			},
 		};
@@ -664,18 +664,19 @@ describe('AstroSession - No-Cookie Short Circuit', () => {
 describe('AstroSession - regenerate() error path', () => {
 	it('should route errors to logger and reset #partial flag', async () => {
 		let storageGetCount = 0;
-		// The cookie mock returns 'old-session' so that ensureData() will try to
-		// load from storage using that key and hit the corrupt data path.
+		// Use a valid UUID so #ensureSessionID() accepts it and ensureData() tries
+		// to load from storage, hitting the corrupt data path.
+		const oldSessionUUID = '00000000-0000-4000-8000-000000000001';
 		const cookies: MockCookies = {
 			set: () => {},
 			delete: () => {},
-			get: (name: string) => (name === 'test-session' ? { value: 'old-session' } : undefined),
+			get: (name: string) => (name === 'test-session' ? { value: oldSessionUUID } : undefined),
 		};
 		const spyLogger = new SpyLogger();
 		const mockStorage = {
 			async get(key: string) {
 				storageGetCount++;
-				if (key === 'old-session') {
+				if (key === oldSessionUUID) {
 					// Return a string that unflatten() will parse into an Array, not a Map.
 					// This causes unflatten(raw) instanceof Map to be false, throwing an AstroError.
 					return '[1,2,3]';
@@ -734,3 +735,42 @@ describe('AstroSession - regenerate() error path', () => {
 	});
 });
 // #endregion
+
+// #region Session ID Validation
+describe('AstroSession - Session ID Validation', () => {
+	it('rejects a non-UUID session cookie value and generates a new ID', async () => {
+		const cookies: MockCookies = {
+			set: () => {},
+			delete: () => {},
+			get: (name: string) =>
+				name === 'test-session' ? { value: 'not-a-uuid-at-all' } : undefined,
+		};
+
+		const session = createSession(defaultConfig, cookies);
+		session.set('key', 'value');
+
+		const id = session.sessionID;
+		assert.ok(id, 'Session ID should be set');
+		assert.notEqual(id, 'not-a-uuid-at-all', 'Should not accept non-UUID cookie value');
+		assert.match(
+			id!,
+			/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i,
+			'Should generate a valid UUID',
+		);
+	});
+
+	it('accepts a valid UUID session cookie value', async () => {
+		const validUUID = '550e8400-e29b-41d4-a716-446655440000';
+		const cookies: MockCookies = {
+			set: () => {},
+			delete: () => {},
+			get: (name: string) => (name === 'test-session' ? { value: validUUID } : undefined),
+		};
+
+		const session = createSession(defaultConfig, cookies);
+		session.set('key', 'value');
+
+		assert.equal(session.sessionID, validUUID, 'Should preserve valid UUID from cookie');
+	});
+});
+// #endregion