Skip to content

Commit 30d81c5

Browse files
kvmiloscopybara-github
authored andcommitted
fix: reject cross-session app:/user: state keys from the dev web server API
PiperOrigin-RevId: 928551854
1 parent 0a40557 commit 30d81c5

5 files changed

Lines changed: 223 additions & 1 deletion

File tree

dev/src/main/java/com/google/adk/web/controller/ExecutionController.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.google.adk.events.Event;
2222
import com.google.adk.runner.Runner;
2323
import com.google.adk.web.dto.AgentRunRequest;
24+
import com.google.adk.web.security.CallerStateGuard;
2425
import com.google.adk.web.service.RunnerService;
2526
import com.google.common.collect.Lists;
2627
import io.reactivex.rxjava3.core.Flowable;
@@ -73,6 +74,7 @@ public List<Event> agentRun(@RequestBody AgentRunRequest request) {
7374
throw new ResponseStatusException(
7475
HttpStatus.BAD_REQUEST, "sessionId cannot be null or empty");
7576
}
77+
CallerStateGuard.validateCallerState(request.stateDelta);
7678
log.info("Request received for POST /run for session: {}", request.sessionId);
7779

7880
Runner runner = this.runnerService.getRunner(request.appName);
@@ -124,6 +126,12 @@ public SseEmitter agentRunSse(@RequestBody AgentRunRequest request) {
124126
new ResponseStatusException(HttpStatus.BAD_REQUEST, "sessionId cannot be null or empty"));
125127
return emitter;
126128
}
129+
try {
130+
CallerStateGuard.validateCallerState(request.stateDelta);
131+
} catch (ResponseStatusException e) {
132+
emitter.completeWithError(e);
133+
return emitter;
134+
}
127135

128136
log.info(
129137
"SseEmitter Request received for POST /run_sse_emitter for session: {}", request.sessionId);

dev/src/main/java/com/google/adk/web/controller/SessionController.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.google.adk.sessions.ListSessionsResponse;
2323
import com.google.adk.sessions.Session;
2424
import com.google.adk.web.dto.SessionRequest;
25+
import com.google.adk.web.security.CallerStateGuard;
2526
import io.reactivex.rxjava3.core.Maybe;
2627
import io.reactivex.rxjava3.core.Single;
2728
import java.util.Collections;
@@ -176,6 +177,7 @@ public Session createSessionWithId(
176177
body);
177178

178179
Map<String, Object> initialState = (body != null) ? body.getState() : Collections.emptyMap();
180+
CallerStateGuard.validateCallerState(initialState);
179181

180182
try {
181183
findSessionOrThrow(appName, userId, sessionId);
@@ -237,8 +239,9 @@ public Session createSession(
237239
userId,
238240
body);
239241

242+
Map<String, Object> initialState = (body != null) ? body.getState() : Collections.emptyMap();
243+
CallerStateGuard.validateCallerState(initialState);
240244
try {
241-
Map<String, Object> initialState = (body != null) ? body.getState() : Collections.emptyMap();
242245
Session createdSession =
243246
sessionService
244247
.createSession(appName, userId, new ConcurrentHashMap<>(initialState), null)
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Copyright 2025 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.adk.web.security;
18+
19+
import com.google.adk.sessions.State;
20+
import java.util.Map;
21+
import org.springframework.http.HttpStatus;
22+
import org.springframework.web.server.ResponseStatusException;
23+
24+
/**
25+
* Validates caller-supplied session state received over the development web API.
26+
*
27+
* <p>Keys prefixed with {@link State#APP_PREFIX} or {@link State#USER_PREFIX} are stored in app- or
28+
* user-scoped state that is shared across sessions. Such keys are rejected when supplied by an HTTP
29+
* caller, so the development server cannot be used to write cross-session state from external
30+
* input. Programmatic callers using the session APIs directly are unaffected.
31+
*/
32+
public final class CallerStateGuard {
33+
34+
private CallerStateGuard() {}
35+
36+
/**
37+
* Rejects caller-supplied state containing cross-session ({@code app:}/{@code user:}) keys.
38+
*
39+
* @param state caller-supplied state map (may be {@code null} or empty)
40+
* @throws ResponseStatusException with {@link HttpStatus#BAD_REQUEST} if a disallowed key is
41+
* found
42+
*/
43+
public static void validateCallerState(Map<String, Object> state) {
44+
if (state == null || state.isEmpty()) {
45+
return;
46+
}
47+
for (String key : state.keySet()) {
48+
if (key == null) {
49+
continue;
50+
}
51+
if (key.startsWith(State.APP_PREFIX) || key.startsWith(State.USER_PREFIX)) {
52+
throw new ResponseStatusException(
53+
HttpStatus.BAD_REQUEST,
54+
"Caller-supplied state may not write the key '"
55+
+ key
56+
+ "'. Keys prefixed with '"
57+
+ State.APP_PREFIX
58+
+ "' or '"
59+
+ State.USER_PREFIX
60+
+ "' are shared across sessions and cannot be set via the development web API.");
61+
}
62+
}
63+
}
64+
}

dev/src/test/java/com/google/adk/web/AdkWebServerTest.java

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,4 +139,63 @@ public void listSessions_shouldReturnOk() throws Exception {
139139
mockMvc.perform(delete("/apps/test-app/users/test-user/sessions/test-session-1"));
140140
mockMvc.perform(delete("/apps/test-app/users/test-user/sessions/test-session-2"));
141141
}
142+
143+
@Test
144+
public void createSession_withPlainSessionScopedState_returnsOk() throws Exception {
145+
var result =
146+
mockMvc
147+
.perform(
148+
post("/apps/test-app/users/test-user/sessions")
149+
.contentType(MediaType.APPLICATION_JSON)
150+
.content("{\"state\":{\"topic\":\"weather\"}}"))
151+
.andExpect(status().isOk())
152+
.andReturn();
153+
154+
var sessionId =
155+
com.jayway.jsonpath.JsonPath.read(result.getResponse().getContentAsString(), "$.id");
156+
mockMvc.perform(delete("/apps/test-app/users/test-user/sessions/" + sessionId));
157+
}
158+
159+
@Test
160+
public void createSession_withAppScopedState_returnsBadRequest() throws Exception {
161+
mockMvc
162+
.perform(
163+
post("/apps/test-app/users/test-user/sessions")
164+
.contentType(MediaType.APPLICATION_JSON)
165+
.content("{\"state\":{\"app:operationalScope\":\"everything\"}}"))
166+
.andExpect(status().isBadRequest());
167+
}
168+
169+
@Test
170+
public void createSession_withUserScopedState_returnsBadRequest() throws Exception {
171+
mockMvc
172+
.perform(
173+
post("/apps/test-app/users/test-user/sessions")
174+
.contentType(MediaType.APPLICATION_JSON)
175+
.content("{\"state\":{\"user:role\":\"admin\"}}"))
176+
.andExpect(status().isBadRequest());
177+
}
178+
179+
@Test
180+
public void createSessionWithId_withAppScopedState_returnsBadRequest() throws Exception {
181+
mockMvc
182+
.perform(
183+
post("/apps/test-app/users/test-user/sessions/test-session-scoped")
184+
.contentType(MediaType.APPLICATION_JSON)
185+
.content("{\"state\":{\"app:operationalScope\":\"everything\"}}"))
186+
.andExpect(status().isBadRequest());
187+
}
188+
189+
@Test
190+
public void run_withAppScopedStateDelta_returnsBadRequest() throws Exception {
191+
mockMvc
192+
.perform(
193+
post("/run")
194+
.contentType(MediaType.APPLICATION_JSON)
195+
.content(
196+
"{\"appName\":\"test-app\",\"userId\":\"test-user\","
197+
+ "\"sessionId\":\"test-session\","
198+
+ "\"stateDelta\":{\"app:operationalScope\":\"everything\"}}"))
199+
.andExpect(status().isBadRequest());
200+
}
142201
}
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*
2+
* Copyright 2025 Google LLC
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.google.adk.web.security;
18+
19+
import static com.google.common.truth.Truth.assertThat;
20+
import static org.junit.jupiter.api.Assertions.assertThrows;
21+
22+
import com.google.common.collect.ImmutableMap;
23+
import java.util.HashMap;
24+
import java.util.Map;
25+
import org.junit.jupiter.api.Test;
26+
import org.springframework.http.HttpStatus;
27+
import org.springframework.web.server.ResponseStatusException;
28+
29+
/** Unit tests for {@link CallerStateGuard}. */
30+
public class CallerStateGuardTest {
31+
32+
@Test
33+
public void validateCallerState_nullState_doesNotThrow() {
34+
CallerStateGuard.validateCallerState(null);
35+
}
36+
37+
@Test
38+
public void validateCallerState_emptyState_doesNotThrow() {
39+
CallerStateGuard.validateCallerState(ImmutableMap.of());
40+
}
41+
42+
@Test
43+
public void validateCallerState_plainSessionScopedKey_doesNotThrow() {
44+
CallerStateGuard.validateCallerState(ImmutableMap.of("userContext", "Alice"));
45+
}
46+
47+
@Test
48+
public void validateCallerState_tempPrefix_doesNotThrow() {
49+
CallerStateGuard.validateCallerState(ImmutableMap.of("temp:scratch", "x"));
50+
}
51+
52+
@Test
53+
public void validateCallerState_underscorePrefixedKey_doesNotThrow() {
54+
CallerStateGuard.validateCallerState(ImmutableMap.of("_adk_replay_config", ImmutableMap.of()));
55+
}
56+
57+
@Test
58+
public void validateCallerState_appPrefix_throwsBadRequest() {
59+
ResponseStatusException exception =
60+
assertThrows(
61+
ResponseStatusException.class,
62+
() ->
63+
CallerStateGuard.validateCallerState(ImmutableMap.of("app:operationalScope", "x")));
64+
65+
assertThat(exception.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
66+
assertThat(exception.getReason()).contains("app:operationalScope");
67+
}
68+
69+
@Test
70+
public void validateCallerState_userPrefix_throwsBadRequest() {
71+
ResponseStatusException exception =
72+
assertThrows(
73+
ResponseStatusException.class,
74+
() -> CallerStateGuard.validateCallerState(ImmutableMap.of("user:role", "admin")));
75+
76+
assertThat(exception.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
77+
assertThat(exception.getReason()).contains("user:role");
78+
}
79+
80+
@Test
81+
public void validateCallerState_mixedValidAndScopedKeys_throwsBadRequest() {
82+
Map<String, Object> state = new HashMap<>();
83+
state.put("userContext", "Alice");
84+
state.put("app:scope", "everything");
85+
86+
assertThrows(ResponseStatusException.class, () -> CallerStateGuard.validateCallerState(state));
87+
}
88+
}

0 commit comments

Comments
 (0)