Skip to content
1 change: 0 additions & 1 deletion core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
24 changes: 20 additions & 4 deletions core/src/a2a/event_converter_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> = new Set(['escalate']);

function createAdkEventFromMetadata(a2aEvent: A2AEvent): AdkEvent {
const metadata = a2aEvent.metadata || {};

Expand All @@ -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)),
),
),
});
}

Expand Down
24 changes: 13 additions & 11 deletions core/src/artifacts/in_memory_artifact_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,17 +101,15 @@ export class InMemoryArtifactService implements BaseArtifactService {
userId,
sessionId,
}: ListArtifactKeysRequest): Promise<string[]> {
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)));
}
}

Expand Down Expand Up @@ -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,
Expand All @@ -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('/')}/`;
}

/**
Expand Down
8 changes: 3 additions & 5 deletions core/src/code_executors/unsafe_local_code_executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand Down
46 changes: 32 additions & 14 deletions core/src/utils/env_aware_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
);
}

/**
Expand Down
21 changes: 19 additions & 2 deletions core/src/utils/file_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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}`,
);
Expand Down Expand Up @@ -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}`,
);
Expand Down
68 changes: 20 additions & 48 deletions core/src/utils/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
}
Expand All @@ -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);
}
}

Expand Down
8 changes: 7 additions & 1 deletion core/test/a2a/event_converter_utils_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Loading
Loading