diff --git a/core/package.json b/core/package.json index 678f8c0c9..3d026dc01 100644 --- a/core/package.json +++ b/core/package.json @@ -66,7 +66,6 @@ "js-yaml": "^4.1.1", "jsonpath-plus": "^10.4.0", "lodash-es": "^4.18.1", - "winston": "^3.19.0", "zod": "^4.2.1", "zod-to-json-schema": "^3.25.1" }, diff --git a/core/src/a2a/event_converter_utils.ts b/core/src/a2a/event_converter_utils.ts index 6c7538f16..570d19d21 100644 --- a/core/src/a2a/event_converter_utils.ts +++ b/core/src/a2a/event_converter_utils.ts @@ -252,6 +252,11 @@ function taskToAdkEvent( }; } +// EventActions fields a remote A2A peer may set on the event we emit +// for it. Every other field is dropped: see the comment at the call +// site in createAdkEventFromMetadata for why. +const PEER_SETTABLE_ACTION_FIELDS: ReadonlySet = new Set(['escalate']); + function createAdkEventFromMetadata(a2aEvent: A2AEvent): AdkEvent { const metadata = a2aEvent.metadata || {}; @@ -272,10 +277,21 @@ function createAdkEventFromMetadata(a2aEvent: A2AEvent): AdkEvent { string, unknown >, - actions: createEventActions({ - escalate: !!metadata[A2AMetadataKeys.ESCALATE], - transferToAgent: metadata[A2AMetadataKeys.TRANSFER_TO_AGENT] as string, - }), + // Only fields in PEER_SETTABLE_ACTION_FIELDS may be restored from + // metadata a remote A2A peer controls. Every other action field either + // mutates the caller's own session or drives the caller's own control + // flow (e.g. `transferToAgent`, see llm_agent.ts), so it must never be + // rebuilt from peer-supplied data. Filtering through an allowlist here + // (rather than just omitting the unsafe field) means a future action + // field is unsafe-by-default: adding it to `candidateActions` alone + // does nothing until it's also added to the allowlist. + actions: createEventActions( + Object.fromEntries( + Object.entries({ + escalate: !!metadata[A2AMetadataKeys.ESCALATE], + }).filter(([key]) => PEER_SETTABLE_ACTION_FIELDS.has(key)), + ), + ), }); } diff --git a/core/src/artifacts/in_memory_artifact_service.ts b/core/src/artifacts/in_memory_artifact_service.ts index 5c762de60..c86bf0073 100644 --- a/core/src/artifacts/in_memory_artifact_service.ts +++ b/core/src/artifacts/in_memory_artifact_service.ts @@ -101,17 +101,15 @@ export class InMemoryArtifactService implements BaseArtifactService { userId, sessionId, }: ListArtifactKeysRequest): Promise { - const sessionPrefix = `${appName}/${userId}/${sessionId}/`; - const usernamespacePrefix = `${appName}/${userId}/user/`; + const sessionPrefix = artifactPrefix('session', appName, userId, sessionId); + const userPrefix = artifactPrefix('user', appName, userId); 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); + filenames.push(decodeURIComponent(path.slice(sessionPrefix.length))); + } else if (path.startsWith(userPrefix)) { + filenames.push(decodeURIComponent(path.slice(userPrefix.length))); } } @@ -197,13 +195,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, @@ -212,10 +210,14 @@ function artifactPath( filename: string, ): string { if (fileHasUserNamespace(filename)) { - return `${appName}/${userId}/user/${filename}`; + return `${artifactPrefix('user', appName, userId)}${encodeURIComponent(filename)}`; } - return `${appName}/${userId}/${sessionId}/${filename}`; + return `${artifactPrefix('session', appName, userId, sessionId)}${encodeURIComponent(filename)}`; +} + +function artifactPrefix(scope: string, ...parts: string[]): string { + return `${[scope, ...parts].map(encodeURIComponent).join('/')}/`; } /** diff --git a/core/src/code_executors/unsafe_local_code_executor.ts b/core/src/code_executors/unsafe_local_code_executor.ts index 97061719a..1457d189e 100644 --- a/core/src/code_executors/unsafe_local_code_executor.ts +++ b/core/src/code_executors/unsafe_local_code_executor.ts @@ -65,12 +65,10 @@ async function createTempScriptFile( language: CodeExecutionLanguage, shellCommandPath?: string, ): Promise<{filePath: string; tempDir: string}> { - const tempDir = path.join( - os.tmpdir(), - 'adk_js_unsafe_code_executor', - Date.now().toString() + '_' + Math.random().toString(36).slice(2), + // mkdtemp names the directory itself and creates it exclusively at 0o700. + const tempDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'adk_js_unsafe_code_executor_'), ); - await fs.mkdir(tempDir, {recursive: true}); const ext = getExtensionForLanguage(language, shellCommandPath) || '.js'; const filePath = path.join(tempDir, `script${ext}`); diff --git a/core/src/utils/env_aware_utils.ts b/core/src/utils/env_aware_utils.ts index b20557332..41ea36d66 100644 --- a/core/src/utils/env_aware_utils.ts +++ b/core/src/utils/env_aware_utils.ts @@ -12,29 +12,47 @@ export function isBrowser() { } /** - * Generates a random UUID. + * Generates a random UUID from a cryptographically secure source. + * + * `crypto.randomUUID()` is only exposed in secure contexts, so it is absent on + * plain-HTTP origins even though `crypto` itself is present. + * `crypto.getRandomValues()` carries no such restriction, so it is used as the + * fallback rather than `Math.random()`. + * + * Some callers use this value to make security decisions — the OAuth2 `state` + * parameter in `AuthHandler` and the session identifiers minted by the session + * services — so this function must not silently degrade to a non-cryptographic + * generator. */ -const UUID_MASK = 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'; export function randomUUID(): string { if (globalThis.crypto?.randomUUID) { return globalThis.crypto.randomUUID(); } - let uuid = ''; - - for (let i = 0; i < UUID_MASK.length; i++) { - const randomValue = (Math.random() * 16) | 0; + if (globalThis.crypto?.getRandomValues) { + const bytes = globalThis.crypto.getRandomValues(new Uint8Array(16)); + // RFC 4122 section 4.4: version 4 in the high nibble of octet 6, variant + // 10xx in the two high bits of octet 8. + bytes[6] = (bytes[6] & 0x0f) | 0x40; + bytes[8] = (bytes[8] & 0x3f) | 0x80; - if (UUID_MASK[i] === 'x') { - uuid += randomValue.toString(16); - } else if (UUID_MASK[i] === 'y') { - uuid += ((randomValue & 0x3) | 0x8).toString(16); - } else { - uuid += UUID_MASK[i]; - } + const hex = Array.from(bytes, (b) => b.toString(16).padStart(2, '0')); + return [ + hex.slice(0, 4), + hex.slice(4, 6), + hex.slice(6, 8), + hex.slice(8, 10), + hex.slice(10, 16), + ] + .map((group) => group.join('')) + .join('-'); } - return uuid; + throw new Error( + 'randomUUID: no cryptographically secure source of randomness is ' + + 'available. Neither crypto.randomUUID() nor crypto.getRandomValues() is ' + + 'present in this environment.', + ); } /** diff --git a/core/src/utils/file_utils.ts b/core/src/utils/file_utils.ts index 54dc54a04..d71330482 100644 --- a/core/src/utils/file_utils.ts +++ b/core/src/utils/file_utils.ts @@ -8,6 +8,23 @@ import * as fs from 'node:fs/promises'; import * as path from 'node:path'; import {File} from '../code_executors/code_execution_utils.js'; +/** + * Reports whether resolvedPath is resolvedBaseDir itself, or a path nested + * inside it. + * + * A plain `resolvedPath.startsWith(resolvedBaseDir)` check is a path-separator- + * unaware prefix match: it also accepts sibling directories whose name merely + * starts with the same string, e.g. base dir `/tmp/agent` wrongly "contains" + * `/tmp/agent-evil/x`. Requiring the trailing separator (or exact equality) + * closes that gap. + */ +function isInsideDir(resolvedPath: string, resolvedBaseDir: string): boolean { + return ( + resolvedPath === resolvedBaseDir || + resolvedPath.startsWith(resolvedBaseDir + path.sep) + ); +} + /** * Creates files with the given paths in the current working directory. * @param files The files to materialize. @@ -21,7 +38,7 @@ export async function materializeFiles( for (const file of files) { const fullPath = path.resolve(dir, file.name); - if (!fullPath.startsWith(resolvedBaseDir)) { + if (!isInsideDir(fullPath, resolvedBaseDir)) { throw new Error( `Path traversal detected: ${file.name} resolves outside of ${dir}`, ); @@ -51,7 +68,7 @@ export async function materializeFiles( } } - if (!finalPath.startsWith(resolvedBaseDir)) { + if (!isInsideDir(finalPath, resolvedBaseDir)) { throw new Error( `Path traversal detected: ${file.name} resolves outside of ${dir}`, ); diff --git a/core/src/utils/logger.ts b/core/src/utils/logger.ts index dc854e7ec..f32a49069 100644 --- a/core/src/utils/logger.ts +++ b/core/src/utils/logger.ts @@ -3,7 +3,6 @@ * Copyright 2025 Google LLC * SPDX-License-Identifier: Apache-2.0 */ -import * as winston from 'winston'; /** Log levels for the logger. */ export enum LogLevel { @@ -30,35 +29,21 @@ export interface Logger { setLogLevel(level: LogLevel): void; } +/** The `console` method each level is written with. */ +const CONSOLE_METHOD = { + [LogLevel.DEBUG]: 'debug', + [LogLevel.INFO]: 'info', + [LogLevel.WARN]: 'warn', + [LogLevel.ERROR]: 'error', +} as const; + +/** + * The default logger. Writes through `console` so that it works unchanged in + * Node and in the browser; see https://github.com/google/adk-js/issues/611. + */ class SimpleLogger implements Logger { - private readonly logger: winston.Logger; private logLevel: LogLevel = LogLevel.INFO; - constructor() { - this.logger = winston.createLogger({ - levels: { - 'debug': LogLevel.DEBUG, - 'info': LogLevel.INFO, - 'warn': LogLevel.WARN, - 'error': LogLevel.ERROR, - }, - level: 'error', - format: winston.format.combine( - winston.format.label({label: 'ADK'}), - winston.format((info) => { - info.level = info.level.toUpperCase(); - return info; - })(), - winston.format.colorize(), - winston.format.timestamp(), - winston.format.printf((info) => { - return `${info.level}: [${info.label}] ${info.timestamp} ${info.message}`; - }), - ), - transports: [new winston.transports.Console()], - }); - } - setLogLevel(level: LogLevel): void { this.logLevel = level; } @@ -68,39 +53,26 @@ class SimpleLogger implements Logger { return; } - this.logger.log(level.toString(), messages.join(' ')); + const timestamp = new Date().toISOString(); + const line = `${LogLevel[level]}: [ADK] ${timestamp} ${messages.join(' ')}`; + + console[CONSOLE_METHOD[level]](line); } debug(...messages: unknown[]): void { - if (this.logLevel > LogLevel.DEBUG) { - return; - } - - this.logger.debug(messages.join(' ')); + this.log(LogLevel.DEBUG, ...messages); } info(...messages: unknown[]): void { - if (this.logLevel > LogLevel.INFO) { - return; - } - - this.logger.info(messages.join(' ')); + this.log(LogLevel.INFO, ...messages); } warn(...messages: unknown[]): void { - if (this.logLevel > LogLevel.WARN) { - return; - } - - this.logger.warn(messages.join(' ')); + this.log(LogLevel.WARN, ...messages); } error(...messages: unknown[]): void { - if (this.logLevel > LogLevel.ERROR) { - return; - } - - this.logger.error(messages.join(' ')); + this.log(LogLevel.ERROR, ...messages); } } diff --git a/core/test/a2a/event_converter_utils_test.ts b/core/test/a2a/event_converter_utils_test.ts index 414d63a73..9f88c5460 100644 --- a/core/test/a2a/event_converter_utils_test.ts +++ b/core/test/a2a/event_converter_utils_test.ts @@ -200,7 +200,13 @@ describe('event_converter_utils', () => { ]); expect(event!.turnComplete).toBe(true); expect(event!.actions?.escalate).toBe(true); - expect(event!.actions?.transferToAgent).toBe('agent2'); + // Peer-supplied `adk_transfer_to_agent` in the fixture below must be + // dropped, not restored -- it drives the local orchestrator's own + // control flow and must never come from a remote peer. Asserting + // `toBeUndefined()` here (rather than removing the assertion) means + // a future regression that re-restores it fails loudly instead of + // passing silently. + expect(event!.actions?.transferToAgent).toBeUndefined(); expect(event!.customMetadata).toEqual({ 'a2a:task_id': 'task1', 'a2a:context_id': 'context1', diff --git a/core/test/artifacts/in_memory_artifact_service_test.ts b/core/test/artifacts/in_memory_artifact_service_test.ts index 4fa2f9049..755694cdb 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,94 @@ 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 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('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(); + + 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(keys).toEqual([]); + }); }); diff --git a/core/test/code_executors/unsafe_local_code_executor_test.ts b/core/test/code_executors/unsafe_local_code_executor_test.ts index 82a0d2d01..7a38ed2c0 100644 --- a/core/test/code_executors/unsafe_local_code_executor_test.ts +++ b/core/test/code_executors/unsafe_local_code_executor_test.ts @@ -15,6 +15,7 @@ import { } from '@google/adk'; import {EventEmitter} from 'node:events'; import * as os from 'node:os'; +import * as path from 'node:path'; import {beforeEach, describe, expect, it, vi} from 'vitest'; // Only `spawn` is mocked; it defaults to the real implementation (see @@ -90,6 +91,42 @@ describe('UnsafeLocalCodeExecutor', () => { expect(result.stderr).toBe(''); }); + // The script runs with the temporary directory as its cwd, so it can report + // the name and mode the executor actually created. + it('creates a private, unpredictable temporary directory', async () => { + const params: ExecuteCodeParams = { + invocationContext, + codeExecutionInput: { + code: [ + 'const fs = require("node:fs");', + 'const dir = process.cwd();', + 'const mode = (fs.statSync(dir).mode & 0o777).toString(8);', + 'console.log(JSON.stringify({dir, mode}));', + ].join('\n'), + language: CodeExecutionLanguage.JAVASCRIPT, + inputFiles: [], + }, + }; + + const firstResult = await executor.executeCode(params); + const secondResult = await executor.executeCode(params); + expect(firstResult.stderr).toBe(''); + expect(secondResult.stderr).toBe(''); + + const first = JSON.parse(firstResult.stdout); + const second = JSON.parse(secondResult.stdout); + + // mkdtemp appends six random characters to the prefix it is given. + expect(path.basename(first.dir)).toMatch( + /^adk_js_unsafe_code_executor_.{6}$/, + ); + expect(second.dir).not.toBe(first.dir); + + if (os.platform() !== 'win32') { + expect(first.mode).toBe('700'); + } + }); + it('should capture stderr', async () => { const params: ExecuteCodeParams = { invocationContext, diff --git a/core/test/utils/env_aware_utils_test.ts b/core/test/utils/env_aware_utils_test.ts index bc1a21dc3..38ddbc846 100644 --- a/core/test/utils/env_aware_utils_test.ts +++ b/core/test/utils/env_aware_utils_test.ts @@ -5,7 +5,7 @@ */ import {afterEach, describe, expect, it} from 'vitest'; -import {getBooleanEnvVar} from '../../src/utils/env_aware_utils.js'; +import {getBooleanEnvVar, randomUUID} from '../../src/utils/env_aware_utils.js'; describe('env_aware_utils', () => { describe('getBooleanEnvVar', () => { @@ -50,4 +50,71 @@ describe('env_aware_utils', () => { expect(getBooleanEnvVar('NON_EXISTENT_VAR')).toBe(false); }); }); + + describe('randomUUID', () => { + const originalCrypto = globalThis.crypto; + + afterEach(() => { + Object.defineProperty(globalThis, 'crypto', { + value: originalCrypto, + configurable: true, + writable: true, + }); + }); + + const setCrypto = (value: unknown) => { + Object.defineProperty(globalThis, 'crypto', { + value, + configurable: true, + writable: true, + }); + }; + + const UUID_V4 = + /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/; + + it('uses crypto.randomUUID when it is available', () => { + setCrypto({ + randomUUID: () => '00000000-0000-4000-8000-000000000000', + getRandomValues: () => { + throw new Error('getRandomValues must not be called'); + }, + }); + + expect(randomUUID()).toBe('00000000-0000-4000-8000-000000000000'); + }); + + it('returns a valid v4 UUID on this runtime', () => { + expect(randomUUID()).toMatch(UUID_V4); + }); + + // crypto.randomUUID is a secure-context-only API, so it is absent on + // plain-HTTP origins while crypto.getRandomValues remains available. + it('falls back to crypto.getRandomValues when randomUUID is absent', () => { + const getRandomValues = + originalCrypto.getRandomValues.bind(originalCrypto); + setCrypto({getRandomValues}); + + expect(randomUUID()).toMatch(UUID_V4); + }); + + it('draws every byte from getRandomValues, not Math.random', () => { + const getRandomValues = (array: Uint8Array) => { + array.fill(0xab); + return array; + }; + setCrypto({getRandomValues}); + + // 0xab in every byte, with the RFC 4122 version and variant bits applied. + expect(randomUUID()).toBe('abababab-abab-4bab-abab-abababababab'); + }); + + it('throws instead of degrading when no secure source exists', () => { + setCrypto(undefined); + + expect(() => randomUUID()).toThrow( + /no cryptographically secure source of randomness/, + ); + }); + }); }); diff --git a/core/test/utils/file_utils_test.ts b/core/test/utils/file_utils_test.ts index ba44f8cc3..75b6178fc 100644 --- a/core/test/utils/file_utils_test.ts +++ b/core/test/utils/file_utils_test.ts @@ -69,6 +69,29 @@ describe('file_utils', () => { ); }); + it('should throw an error if file attempts to escape into a sibling directory sharing a name prefix', async () => { + // A plain `resolvedPath.startsWith(resolvedBaseDir)` check is fooled by a + // sibling directory whose name merely starts with the same string as the + // target directory (e.g. target `.../sandbox` vs sibling + // `.../sandbox-evil`), since it never requires a path-separator boundary. + const siblingName = `${path.basename(tempDir)}-evil`; + const files = [ + { + name: `../${siblingName}/escape.txt`, + content: 'dangerous', + contentEncoding: FileContentEncoding.UTF8, + mimeType: 'text/plain', + }, + ]; + + await expect(materializeFiles(files, tempDir)).rejects.toThrow( + /Path traversal detected/, + ); + + const siblingPath = path.join(path.dirname(tempDir), siblingName); + await expect(fs.access(siblingPath)).rejects.toThrow(); + }); + it('should throw an error if file attempts to escape target directory via absolute path', async () => { const outsidePath = path.resolve(tempDir, '../outside.txt'); const files = [ diff --git a/core/test/utils/logger_test.ts b/core/test/utils/logger_test.ts index 2865a9de0..8fb93ccf3 100644 --- a/core/test/utils/logger_test.ts +++ b/core/test/utils/logger_test.ts @@ -5,7 +5,7 @@ */ import {getLogger, Logger, LogLevel, setLogger, setLogLevel} from '@google/adk'; -import {afterEach, beforeEach, describe, expect, it} from 'vitest'; +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; import {resetLogger} from '../../src/utils/logger.js'; describe('setLogger', () => { @@ -142,3 +142,123 @@ describe('setLogger', () => { }); }); }); + +describe('SimpleLogger', () => { + const ISO_TIMESTAMP = String.raw`\d{4}-\d{2}-\d{2}T[\d:.]+Z`; + + beforeEach(() => { + resetLogger(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + resetLogger(); + }); + + it('emits a message at the configured level', () => { + const infoSpy = vi.spyOn(console, 'info').mockImplementation(() => {}); + setLogLevel(LogLevel.INFO); + + getLogger().info('hello'); + + expect(infoSpy).toHaveBeenCalledTimes(1); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringMatching( + new RegExp(`^INFO: \\[ADK\\] ${ISO_TIMESTAMP} hello$`), + ), + ); + }); + + it('suppresses a message below the configured level', () => { + const debugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {}); + const infoSpy = vi.spyOn(console, 'info').mockImplementation(() => {}); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + setLogLevel(LogLevel.WARN); + + getLogger().debug('x'); + getLogger().info('y'); + + expect(debugSpy).not.toHaveBeenCalled(); + expect(infoSpy).not.toHaveBeenCalled(); + + getLogger().warn('z'); + + expect(warnSpy).toHaveBeenCalledTimes(1); + }); + + it('defaults to INFO', () => { + const debugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {}); + const infoSpy = vi.spyOn(console, 'info').mockImplementation(() => {}); + + getLogger().debug('x'); + getLogger().info('y'); + + expect(debugSpy).not.toHaveBeenCalled(); + expect(infoSpy).toHaveBeenCalledTimes(1); + }); + + it('routes each level to its matching console method', () => { + const debugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {}); + const infoSpy = vi.spyOn(console, 'info').mockImplementation(() => {}); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + setLogLevel(LogLevel.DEBUG); + + getLogger().debug('d'); + getLogger().info('i'); + getLogger().warn('w'); + getLogger().error('e'); + + expect(debugSpy).toHaveBeenCalledTimes(1); + expect(debugSpy).toHaveBeenCalledWith( + expect.stringContaining('DEBUG: [ADK] '), + ); + expect(infoSpy).toHaveBeenCalledTimes(1); + expect(infoSpy).toHaveBeenCalledWith( + expect.stringContaining('INFO: [ADK] '), + ); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('WARN: [ADK] '), + ); + expect(errorSpy).toHaveBeenCalledTimes(1); + expect(errorSpy).toHaveBeenCalledWith( + expect.stringContaining('ERROR: [ADK] '), + ); + }); + + it('joins arguments with a single space', () => { + const infoSpy = vi.spyOn(console, 'info').mockImplementation(() => {}); + setLogLevel(LogLevel.INFO); + + getLogger().info('a', 1, true); + + expect(infoSpy).toHaveBeenCalledWith( + expect.stringMatching( + new RegExp(`^INFO: \\[ADK\\] ${ISO_TIMESTAMP} a 1 true$`), + ), + ); + }); + + it('log() emits without throwing', () => { + const infoSpy = vi.spyOn(console, 'info').mockImplementation(() => {}); + setLogLevel(LogLevel.INFO); + + expect(() => getLogger().log(LogLevel.INFO, 'via log')).not.toThrow(); + expect(infoSpy).toHaveBeenCalledTimes(1); + expect(infoSpy).toHaveBeenCalledWith(expect.stringContaining('via log')); + }); + + it('formats the full line for a warning', () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + setLogLevel(LogLevel.WARN); + + getLogger().warn('boom'); + + expect(warnSpy).toHaveBeenCalledWith( + expect.stringMatching( + new RegExp(`^WARN: \\[ADK\\] ${ISO_TIMESTAMP} boom$`), + ), + ); + }); +}); diff --git a/package-lock.json b/package-lock.json index e29a4a190..a323bce13 100644 --- a/package-lock.json +++ b/package-lock.json @@ -70,7 +70,6 @@ "js-yaml": "^4.1.1", "jsonpath-plus": "^10.4.0", "lodash-es": "^4.18.1", - "winston": "^3.19.0", "zod": "^4.2.1", "zod-to-json-schema": "^3.25.1" },