Skip to content

Commit 8752425

Browse files
committed
fix(skills): prevent path traversal in LocalSkillSource
Add input validation to LocalSkillSource to ensure skill names and resource paths cannot escape the skills base directory via path traversal sequences (e.g. "../../../etc/passwd") or absolute paths (e.g. "/etc/passwd"). The new validatePathWithinBase() helper normalizes and resolves each caller-supplied path component against its parent directory, then checks that the result still starts with that parent. This mirrors the boundary check already present in the Go implementation (filesystem_source.go). Affected methods: findResourcePath, listResources, findSkillMdPath. Corresponding tests added for all traversal and absolute-path cases.
1 parent 1685a4e commit 8752425

43 files changed

Lines changed: 228 additions & 3271 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

core/src/main/java/com/google/adk/agents/BaseAgent.java

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@
2424
import com.google.adk.agents.Callbacks.BeforeAgentCallback;
2525
import com.google.adk.events.Event;
2626
import com.google.adk.plugins.Plugin;
27-
import com.google.adk.telemetry.Instrumentation;
28-
import com.google.adk.telemetry.Instrumentation.AgentInvocation;
27+
import com.google.adk.telemetry.Tracing;
2928
import com.google.adk.utils.AgentEnums.AgentOrigin;
3029
import com.google.common.collect.ImmutableList;
3130
import com.google.errorprone.annotations.CanIgnoreReturnValue;
@@ -323,13 +322,11 @@ public Flowable<Event> runAsync(InvocationContext parentContext) {
323322
private Flowable<Event> run(
324323
InvocationContext parentContext,
325324
Function<InvocationContext, Flowable<Event>> runImplementation) {
326-
Context otelContext = Context.current();
327-
return Flowable.using(
328-
() ->
329-
Instrumentation.recordAgentInvocation(
330-
createInvocationContext(parentContext), this, otelContext),
331-
agentInvocation -> {
332-
InvocationContext invocationContext = agentInvocation.getCtx();
325+
Context parentSpanContext = Context.current();
326+
return Flowable.defer(
327+
() -> {
328+
InvocationContext invocationContext = createInvocationContext(parentContext);
329+
333330
Flowable<Event> mainAndAfterEvents =
334331
Flowable.defer(() -> runImplementation.apply(invocationContext))
335332
.concatWith(
@@ -353,10 +350,14 @@ private Flowable<Event> run(
353350
return Flowable.just(beforeEvent).concatWith(mainAndAfterEvents);
354351
})
355352
.switchIfEmpty(mainAndAfterEvents)
356-
.doOnNext(agentInvocation::addEvent)
357-
.doOnError(agentInvocation::setError);
358-
},
359-
AgentInvocation::close);
353+
.compose(
354+
Tracing.<Event>trace("invoke_agent " + name())
355+
.setParent(parentSpanContext)
356+
.configure(
357+
span ->
358+
Tracing.traceAgentInvocation(
359+
span, name(), description(), invocationContext)));
360+
});
360361
}
361362

362363
/**

core/src/main/java/com/google/adk/agents/LoopAgent.java

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,30 +30,6 @@
3030
*
3131
* <p>The loop continues until a sub-agent escalates, or until the maximum number of iterations is
3232
* reached (if specified).
33-
*
34-
* <p><b>Composition with {@link LlmAgent}s:</b> a {@code LoopAgent} does not transfer control back
35-
* to a parent {@link LlmAgent}. To react to loop results, place the {@code LoopAgent} and the
36-
* follow-up {@link LlmAgent} as siblings inside a {@link SequentialAgent}. Loop sub-agents publish
37-
* via {@code outputKey} and the follow-up reads via {@code {key}} placeholders in its instruction:
38-
*
39-
* <pre>{@code
40-
* var refiner =
41-
* LlmAgent.builder()
42-
* .name("refiner")
43-
* .model("gemini-flash-latest")
44-
* .instruction("Refine: {draft?}")
45-
* .outputKey("draft")
46-
* .build();
47-
* var publisher =
48-
* LlmAgent.builder()
49-
* .name("publisher")
50-
* .model("gemini-flash-latest")
51-
* .instruction("Publish: {draft}")
52-
* .build();
53-
* var loop =
54-
* LoopAgent.builder().name("loop").subAgents(refiner).maxIterations(3).build();
55-
* var root = SequentialAgent.builder().name("root").subAgents(loop, publisher).build();
56-
* }</pre>
5733
*/
5834
public class LoopAgent extends BaseAgent {
5935
private static final Logger logger = LoggerFactory.getLogger(LoopAgent.class);

core/src/main/java/com/google/adk/agents/ParallelAgent.java

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -34,38 +34,6 @@
3434
* <p>This approach is beneficial for scenarios requiring multiple perspectives or attempts on a
3535
* single task, such as running different algorithms simultaneously or generating multiple responses
3636
* for review by a subsequent evaluation agent.
37-
*
38-
* <p><b>Composition with {@link LlmAgent}s:</b> a {@code ParallelAgent} does not transfer control
39-
* back to a parent {@link LlmAgent}. To follow a fan-out with an aggregation step, wrap both in a
40-
* {@link SequentialAgent} (used as the root or transferred-to agent). Each parallel sub-agent
41-
* publishes via {@code outputKey} and the aggregator reads via {@code {key}} placeholders in its
42-
* instruction:
43-
*
44-
* <pre>{@code
45-
* var contacts =
46-
* LlmAgent.builder()
47-
* .name("contacts")
48-
* .model("gemini-flash-latest")
49-
* .instruction("List contacts.")
50-
* .outputKey("contacts")
51-
* .build();
52-
* var schedule =
53-
* LlmAgent.builder()
54-
* .name("schedule")
55-
* .model("gemini-flash-latest")
56-
* .instruction("List schedule.")
57-
* .outputKey("schedule")
58-
* .build();
59-
* var writer =
60-
* LlmAgent.builder()
61-
* .name("writer")
62-
* .model("gemini-flash-latest")
63-
* .instruction("Write: contacts={contacts}, schedule={schedule}")
64-
* .build();
65-
* var gather =
66-
* ParallelAgent.builder().name("gather").subAgents(contacts, schedule).build();
67-
* var root = SequentialAgent.builder().name("root").subAgents(gather, writer).build();
68-
* }</pre>
6937
*/
7038
public class ParallelAgent extends BaseAgent {
7139

core/src/main/java/com/google/adk/agents/SequentialAgent.java

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -22,32 +22,7 @@
2222
import org.slf4j.Logger;
2323
import org.slf4j.LoggerFactory;
2424

25-
/**
26-
* An agent that runs its sub-agents sequentially.
27-
*
28-
* <p><b>Composition with {@link LlmAgent}s:</b> a {@code SequentialAgent} does not transfer control
29-
* back to a parent {@link LlmAgent}. Use it as the root or transferred-to agent and place any
30-
* follow-up {@link LlmAgent} as the next sibling. Upstream publishes via {@code outputKey} and
31-
* downstream reads via {@code {key}} placeholders in its instruction:
32-
*
33-
* <pre>{@code
34-
* var draft =
35-
* LlmAgent.builder()
36-
* .name("draft")
37-
* .model("gemini-flash-latest")
38-
* .instruction("Draft a summary.")
39-
* .outputKey("draft")
40-
* .build();
41-
* var reviewer =
42-
* LlmAgent.builder()
43-
* .name("reviewer")
44-
* .model("gemini-flash-latest")
45-
* .instruction("Polish the draft: {draft}")
46-
* .build();
47-
* var pipeline =
48-
* SequentialAgent.builder().name("pipeline").subAgents(draft, reviewer).build();
49-
* }</pre>
50-
*/
25+
/** An agent that runs its sub-agents sequentially. */
5126
public class SequentialAgent extends BaseAgent {
5227

5328
private static final Logger logger = LoggerFactory.getLogger(SequentialAgent.class);

core/src/main/java/com/google/adk/flows/llmflows/BaseLlmFlow.java

Lines changed: 25 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,7 @@
3838
import com.google.adk.models.LlmRequest;
3939
import com.google.adk.models.LlmResponse;
4040
import com.google.adk.telemetry.Tracing;
41-
import com.google.adk.tools.BaseTool;
42-
import com.google.adk.tools.BaseToolset;
4341
import com.google.adk.tools.ToolContext;
44-
import com.google.common.annotations.VisibleForTesting;
4542
import com.google.common.collect.ImmutableList;
4643
import com.google.common.collect.Iterables;
4744
import com.google.genai.types.FunctionResponse;
@@ -61,7 +58,6 @@
6158
import java.util.Optional;
6259
import java.util.Set;
6360
import java.util.concurrent.atomic.AtomicReference;
64-
import java.util.function.BiFunction;
6561
import org.slf4j.Logger;
6662
import org.slf4j.LoggerFactory;
6763

@@ -100,8 +96,20 @@ private Flowable<Event> preprocess(
10096
Context currentContext = Context.current();
10197
LlmAgent agent = (LlmAgent) context.agent();
10298

99+
RequestProcessor toolsProcessor =
100+
(ctx, req) -> {
101+
LlmRequest.Builder builder = req.toBuilder();
102+
return agent
103+
.canonicalTools(new ReadonlyContext(ctx))
104+
.concatMapCompletable(
105+
tool -> tool.processLlmRequest(builder, ToolContext.builder(ctx).build()))
106+
.andThen(
107+
Single.fromCallable(
108+
() -> RequestProcessingResult.create(builder.build(), ImmutableList.of())));
109+
};
110+
103111
Iterable<RequestProcessor> allProcessors =
104-
Iterables.concat(requestProcessors, ImmutableList.of(getRequestProcessorFromTools(agent)));
112+
Iterables.concat(requestProcessors, ImmutableList.of(toolsProcessor));
105113

106114
return Flowable.fromIterable(allProcessors)
107115
.concatMap(
@@ -113,58 +121,6 @@ private Flowable<Event> preprocess(
113121
result -> result.events() != null ? result.events() : ImmutableList.of()));
114122
}
115123

116-
/**
117-
* Constructs a {@link RequestProcessor} that sequentially applies the {@code processLlmRequest}
118-
* methods of all tools and toolsets associated with this agent to the incoming {@link
119-
* LlmRequest}.
120-
*
121-
* @return A {@link RequestProcessor} that applies tool-specific modifications to LLM requests.
122-
*/
123-
@VisibleForTesting
124-
RequestProcessor getRequestProcessorFromTools(LlmAgent agent) {
125-
return (context, request) -> {
126-
ReadonlyContext readonlyContext = new ReadonlyContext(context);
127-
List<BiFunction<LlmRequest.Builder, ToolContext, Completable>> processors = new ArrayList<>();
128-
129-
for (Object toolOrToolset : agent.toolsUnion()) {
130-
if (toolOrToolset instanceof BaseTool baseTool) {
131-
processors.add(
132-
(builder, ctx) -> {
133-
Completable c = baseTool.processLlmRequest(builder, ctx);
134-
return c == null ? Completable.complete() : c;
135-
});
136-
} else if (toolOrToolset instanceof BaseToolset baseToolset) {
137-
// First apply the toolset's own request processor, then unwrap all tools from the toolset
138-
// and apply each individual tool's request processor sequentially.
139-
processors.add(
140-
(builder, ctx) -> {
141-
Completable c = baseToolset.processLlmRequest(builder, ctx);
142-
Completable toolsetProcessor = c == null ? Completable.complete() : c;
143-
return toolsetProcessor
144-
.andThen(baseToolset.getTools(readonlyContext))
145-
.concatMapCompletable(
146-
b -> {
147-
Completable tc = b.processLlmRequest(builder, ctx);
148-
return tc == null ? Completable.complete() : tc;
149-
});
150-
});
151-
} else {
152-
throw new IllegalArgumentException(
153-
"Object in tools list is not of a supported type: "
154-
+ toolOrToolset.getClass().getName());
155-
}
156-
}
157-
158-
LlmRequest.Builder builder = request.toBuilder();
159-
ToolContext toolContext = ToolContext.builder(context).build();
160-
return Flowable.fromIterable(processors)
161-
.concatMapCompletable(f -> f.apply(builder, toolContext))
162-
.andThen(
163-
Single.fromCallable(
164-
() -> RequestProcessingResult.create(builder.build(), ImmutableList.of())));
165-
};
166-
}
167-
168124
/**
169125
* Post-processes the LLM response after receiving it from the LLM. Executes all registered {@link
170126
* ResponseProcessor} instances. Emits events for the model response and any subsequent function
@@ -479,10 +435,12 @@ private Flowable<Event> runOneStep(Context spanContext, InvocationContext contex
479435
"Agent not found: " + agentToTransfer)));
480436
}
481437
return postProcessedEvents.concatWith(
482-
nextAgent
483-
.get()
484-
.runAsync(context)
485-
.compose(Tracing.withContext(spanContext)));
438+
Flowable.defer(
439+
() -> {
440+
try (Scope s = spanContext.makeCurrent()) {
441+
return nextAgent.get().runAsync(context);
442+
}
443+
}));
486444
}
487445
return postProcessedEvents;
488446
});
@@ -664,10 +622,12 @@ public void onError(Throwable e) {
664622
"Agent not found: " + event.actions().transferToAgent().get());
665623
}
666624
Flowable<Event> nextAgentEvents =
667-
nextAgent
668-
.get()
669-
.runLive(invocationContext)
670-
.compose(Tracing.withContext(spanContext));
625+
Flowable.defer(
626+
() -> {
627+
try (Scope s = spanContext.makeCurrent()) {
628+
return nextAgent.get().runLive(invocationContext);
629+
}
630+
});
671631
events = Flowable.concat(events, nextAgentEvents);
672632
}
673633
return events;

core/src/main/java/com/google/adk/flows/llmflows/Functions.java

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@
2929
import com.google.adk.events.Event;
3030
import com.google.adk.events.EventActions;
3131
import com.google.adk.events.ToolConfirmation;
32-
import com.google.adk.telemetry.Instrumentation;
33-
import com.google.adk.telemetry.Instrumentation.ToolExecution;
3432
import com.google.adk.telemetry.Tracing;
3533
import com.google.adk.tools.BaseTool;
3634
import com.google.adk.tools.FunctionTool;
@@ -432,25 +430,6 @@ private static Maybe<Event> postProcessFunctionResult(
432430
ToolContext toolContext,
433431
boolean isLive,
434432
Context parentContext) {
435-
return Maybe.using(
436-
() ->
437-
Instrumentation.recordToolExecution(
438-
tool, invocationContext.agent(), functionArgs, parentContext),
439-
toolExecution ->
440-
processFunctionResult(
441-
maybeFunctionResult, invocationContext, tool, functionArgs, toolContext, isLive)
442-
.doOnSuccess(event -> toolExecution.context().setFunctionResponseEvent(event))
443-
.doOnError(toolExecution::setError),
444-
ToolExecution::close);
445-
}
446-
447-
private static Maybe<Event> processFunctionResult(
448-
Maybe<Map<String, Object>> maybeFunctionResult,
449-
InvocationContext invocationContext,
450-
BaseTool tool,
451-
Map<String, Object> functionArgs,
452-
ToolContext toolContext,
453-
boolean isLive) {
454433
return maybeFunctionResult
455434
.map(Optional::of)
456435
.defaultIfEmpty(Optional.empty())
@@ -488,7 +467,20 @@ private static Maybe<Event> processFunctionResult(
488467
tool, finalFunctionResult, toolContext, invocationContext);
489468
return Maybe.just(event);
490469
});
491-
});
470+
})
471+
.compose(
472+
Tracing.<Event>trace("execute_tool [" + tool.name() + "]")
473+
.setParent(parentContext)
474+
.onSuccess(
475+
(span, event) ->
476+
Tracing.traceToolExecution(
477+
span,
478+
tool.name(),
479+
tool.description(),
480+
tool.getClass().getSimpleName(),
481+
functionArgs,
482+
event,
483+
null)));
492484
}
493485

494486
private static Optional<Event> mergeParallelFunctionResponseEvents(

core/src/main/java/com/google/adk/plugins/agentanalytics/BigQueryAgentAnalyticsPlugin.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -253,10 +253,7 @@ private Completable logEvent(
253253
parseFuture =
254254
state
255255
.getParser()
256-
.parse(
257-
content,
258-
traceIds.traceId(),
259-
traceIds.spanId() != null ? traceIds.spanId() : "no_span")
256+
.parse(content)
260257
.thenAccept(
261258
parsedContent -> {
262259
row.put(

0 commit comments

Comments
 (0)