From 18004a457b62ba60a8c6b75ff929955be0506988 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 30 Jul 2026 09:48:33 +0200 Subject: [PATCH 1/3] fix(artifacts): isolate in-memory composite keys --- .../artifacts/in_memory_artifact_service.ts | 26 +++++++++------- .../in_memory_artifact_service_test.ts | 30 ++++++++++++++++++- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/core/src/artifacts/in_memory_artifact_service.ts b/core/src/artifacts/in_memory_artifact_service.ts index 5c762de60..7225319d1 100644 --- a/core/src/artifacts/in_memory_artifact_service.ts +++ b/core/src/artifacts/in_memory_artifact_service.ts @@ -101,17 +101,19 @@ export class InMemoryArtifactService implements BaseArtifactService { userId, sessionId, }: ListArtifactKeysRequest): Promise { - const sessionPrefix = `${appName}/${userId}/${sessionId}/`; - const usernamespacePrefix = `${appName}/${userId}/user/`; const filenames: string[] = []; for (const path in this.artifacts) { - if (path.startsWith(sessionPrefix)) { - const filename = path.replace(sessionPrefix, ''); - filenames.push(filename); - } else if (path.startsWith(usernamespacePrefix)) { - const filename = path.replace(usernamespacePrefix, ''); - filenames.push(filename); + const key = JSON.parse(path) as ArtifactStorageKey; + if ( + key[0] === 'session' && + key[1] === appName && + key[2] === userId && + key[3] === sessionId + ) { + filenames.push(key[4]); + } else if (key[0] === 'user' && key[1] === appName && key[2] === userId) { + filenames.push(key[3]); } } @@ -212,12 +214,16 @@ function artifactPath( filename: string, ): string { if (fileHasUserNamespace(filename)) { - return `${appName}/${userId}/user/${filename}`; + return JSON.stringify(['user', appName, userId, filename]); } - return `${appName}/${userId}/${sessionId}/${filename}`; + return JSON.stringify(['session', appName, userId, sessionId, filename]); } +type ArtifactStorageKey = + | ['session', string, string, string, string] + | ['user', string, string, string]; + /** * Checks if the filename has a user namespace prefix. * diff --git a/core/test/artifacts/in_memory_artifact_service_test.ts b/core/test/artifacts/in_memory_artifact_service_test.ts index 4fa2f9049..d46a94394 100644 --- a/core/test/artifacts/in_memory_artifact_service_test.ts +++ b/core/test/artifacts/in_memory_artifact_service_test.ts @@ -5,7 +5,7 @@ */ import {InMemoryArtifactService} from '@google/adk'; -import {describe} from 'vitest'; +import {describe, expect, it} from 'vitest'; import {runArtifactServiceTests} from './artifact_service_test_utils.js'; describe('InMemoryArtifactService', () => { @@ -13,4 +13,32 @@ describe('InMemoryArtifactService', () => { async () => new InMemoryArtifactService(), async () => {}, ); + + it('keeps artifacts with ambiguous path components isolated', async () => { + const service = new InMemoryArtifactService(); + + await service.saveArtifact({ + appName: 'app', + userId: 'user', + sessionId: 'session', + filename: 'nested/report.txt', + artifact: {text: 'artifact-a'}, + }); + await service.saveArtifact({ + appName: 'app', + userId: 'user', + sessionId: 'session/nested', + filename: 'report.txt', + artifact: {text: 'artifact-b'}, + }); + + const artifact = await service.loadArtifact({ + appName: 'app', + userId: 'user', + sessionId: 'session', + filename: 'nested/report.txt', + }); + + expect(artifact?.text).toBe('artifact-a'); + }); }); From a025180555e2f70efeeaa36026aaec868ebfe2fb Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 30 Jul 2026 20:10:10 +0200 Subject: [PATCH 2/3] test(artifacts): cover storage key boundaries --- .../artifacts/in_memory_artifact_service.ts | 14 ++++++--- .../in_memory_artifact_service_test.ts | 31 +++++++++++++++++-- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/core/src/artifacts/in_memory_artifact_service.ts b/core/src/artifacts/in_memory_artifact_service.ts index 7225319d1..55b919a88 100644 --- a/core/src/artifacts/in_memory_artifact_service.ts +++ b/core/src/artifacts/in_memory_artifact_service.ts @@ -199,13 +199,13 @@ export class InMemoryArtifactService implements BaseArtifactService { } /** - * Constructs the path to the artifact. + * Constructs the storage key for the artifact. * * @param appName The app name. * @param userId The user ID. * @param sessionId The session ID. * @param filename The filename. - * @return The path to the artifact. + * @return The encoded storage key for the artifact. */ function artifactPath( appName: string, @@ -221,8 +221,14 @@ function artifactPath( } type ArtifactStorageKey = - | ['session', string, string, string, string] - | ['user', string, string, string]; + | [ + 'session', + appName: string, + userId: string, + sessionId: string, + filename: string, + ] + | ['user', appName: string, userId: string, filename: string]; /** * Checks if the filename has a user namespace prefix. diff --git a/core/test/artifacts/in_memory_artifact_service_test.ts b/core/test/artifacts/in_memory_artifact_service_test.ts index d46a94394..973c6ed6f 100644 --- a/core/test/artifacts/in_memory_artifact_service_test.ts +++ b/core/test/artifacts/in_memory_artifact_service_test.ts @@ -32,13 +32,40 @@ describe('InMemoryArtifactService', () => { artifact: {text: 'artifact-b'}, }); - const artifact = await service.loadArtifact({ + const artifactA = await service.loadArtifact({ appName: 'app', userId: 'user', sessionId: 'session', filename: 'nested/report.txt', }); + const artifactB = await service.loadArtifact({ + appName: 'app', + userId: 'user', + sessionId: 'session/nested', + filename: 'report.txt', + }); + + expect(artifactA?.text).toBe('artifact-a'); + expect(artifactB?.text).toBe('artifact-b'); + }); + + it('does not leak a session named user into other sessions', async () => { + const service = new InMemoryArtifactService(); + + await service.saveArtifact({ + appName: 'app', + userId: 'user', + sessionId: 'user', + filename: 'foo.txt', + artifact: {text: 'session-scoped'}, + }); + + const keys = await service.listArtifactKeys({ + appName: 'app', + userId: 'user', + sessionId: 'other', + }); - expect(artifact?.text).toBe('artifact-a'); + expect(keys).toEqual([]); }); }); From d0ff10871efae13df091da433368001b65802e49 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Fri, 31 Jul 2026 00:07:29 +0200 Subject: [PATCH 3/3] refactor(artifacts): encode in-memory key segments --- .../artifacts/in_memory_artifact_service.ts | 32 ++++++----------- .../in_memory_artifact_service_test.ts | 35 +++++++++++++++++++ 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/core/src/artifacts/in_memory_artifact_service.ts b/core/src/artifacts/in_memory_artifact_service.ts index 55b919a88..c86bf0073 100644 --- a/core/src/artifacts/in_memory_artifact_service.ts +++ b/core/src/artifacts/in_memory_artifact_service.ts @@ -101,19 +101,15 @@ export class InMemoryArtifactService implements BaseArtifactService { userId, sessionId, }: ListArtifactKeysRequest): Promise { + const sessionPrefix = artifactPrefix('session', appName, userId, sessionId); + const userPrefix = artifactPrefix('user', appName, userId); const filenames: string[] = []; for (const path in this.artifacts) { - const key = JSON.parse(path) as ArtifactStorageKey; - if ( - key[0] === 'session' && - key[1] === appName && - key[2] === userId && - key[3] === sessionId - ) { - filenames.push(key[4]); - } else if (key[0] === 'user' && key[1] === appName && key[2] === userId) { - filenames.push(key[3]); + if (path.startsWith(sessionPrefix)) { + filenames.push(decodeURIComponent(path.slice(sessionPrefix.length))); + } else if (path.startsWith(userPrefix)) { + filenames.push(decodeURIComponent(path.slice(userPrefix.length))); } } @@ -214,21 +210,15 @@ function artifactPath( filename: string, ): string { if (fileHasUserNamespace(filename)) { - return JSON.stringify(['user', appName, userId, filename]); + return `${artifactPrefix('user', appName, userId)}${encodeURIComponent(filename)}`; } - return JSON.stringify(['session', appName, userId, sessionId, filename]); + return `${artifactPrefix('session', appName, userId, sessionId)}${encodeURIComponent(filename)}`; } -type ArtifactStorageKey = - | [ - 'session', - appName: string, - userId: string, - sessionId: string, - filename: string, - ] - | ['user', appName: string, userId: string, filename: string]; +function artifactPrefix(scope: string, ...parts: string[]): string { + return `${[scope, ...parts].map(encodeURIComponent).join('/')}/`; +} /** * Checks if the filename has a user namespace prefix. diff --git a/core/test/artifacts/in_memory_artifact_service_test.ts b/core/test/artifacts/in_memory_artifact_service_test.ts index 973c6ed6f..755694cdb 100644 --- a/core/test/artifacts/in_memory_artifact_service_test.ts +++ b/core/test/artifacts/in_memory_artifact_service_test.ts @@ -49,6 +49,41 @@ describe('InMemoryArtifactService', () => { expect(artifactB?.text).toBe('artifact-b'); }); + it('keeps artifacts with ambiguous app and user components isolated', async () => { + const service = new InMemoryArtifactService(); + + await service.saveArtifact({ + appName: 'app', + userId: 'nested/user', + sessionId: 'session', + filename: 'report.txt', + artifact: {text: 'artifact-a'}, + }); + await service.saveArtifact({ + appName: 'app/nested', + userId: 'user', + sessionId: 'session', + filename: 'report.txt', + artifact: {text: 'artifact-b'}, + }); + + const artifactA = await service.loadArtifact({ + appName: 'app', + userId: 'nested/user', + sessionId: 'session', + filename: 'report.txt', + }); + const artifactB = await service.loadArtifact({ + appName: 'app/nested', + userId: 'user', + sessionId: 'session', + filename: 'report.txt', + }); + + expect(artifactA?.text).toBe('artifact-a'); + expect(artifactB?.text).toBe('artifact-b'); + }); + it('does not leak a session named user into other sessions', async () => { const service = new InMemoryArtifactService();