google / google/adk-python

Artifact services (in_memory, file, GCS) key storage on raw session_id — padded id creates a session it cannot reach

Abierto
#7,030 2 comentarios 0 reacciones 1 asignado Reclamado por @sanketpatil06 Ver en GitHub
services
Lenguaje dominante
Python
Estrellas
21.5k
Forks
4k
Merge medio
1 d 14 h
PR fusionados (30 d)
37

Descripción

## Required Information

**Describe the Bug:**

`InMemorySessionService` and `SqliteSessionService` normalize `session_id`
(strip surrounding whitespace) before using it as a storage key (#6892,
#6941/#6942). The artifact services never got the same treatment:
`InMemoryArtifactService`, `FileArtifactService`, and `GcsArtifactService`
all key session-scoped artifact storage on the raw, un-normalized
`session_id` string.

The result: a session created with a padded id (e.g. `"order-42\n"`) is
stored, read, and deleted under the trimmed id `"order-42"` — but an
artifact saved against that same padded id lands in a sibling storage
namespace the session's own (trimmed) id can never reach. One logical
session, two artifact namespaces.

A related, narrower issue: `artifact_util.validate_path_segment` only
rejects an *empty* string, so a whitespace-only `session_id` (e.g. `" "`)
previously slipped through as a literal path/key segment instead of being
rejected or treated as "no id".

A third issue: artifact references (the `artifact://...` URI a caller can
pass as `file_data.file_uri` to alias another artifact) embed `session_id`
directly in the URI, and the same-session scope check
(`validate_artifact_reference_scope`) compares the caller's raw
`session_id` against the id parsed back out of that URI. Without
normalizing both sides the same way, a caller consistently using a padded
id can get a spurious `InputValidationError: Session-scoped artifact
references must stay within the same session scope`.

**Steps to Reproduce:**

1. `pip install google-adk`
2. Run:
```python
import asyncio
from google.adk.sessions.in_memory_session_service import InMemorySessionService
from google.adk.artifacts.in_memory_artifact_service import InMemoryArtifactService
from google.genai import types

async def main():
session_service = InMemorySessionService()
artifact_service = InMemoryArtifactService()

await session_service.create_session(
app_name="a", user_id="u", session_id="s1\n"
) # stored under the trimmed id "s1"

await artifact_service.save_artifact(
app_name="a", user_id="u", session_id="s1\n",
filename="f.txt", artifact=types.Part(text="hi"),
)

keys = await artifact_service.list_artifact_keys(
app_name="a", user_id="u", session_id="s1"
)
print("artifact keys visible to the session's own id:", keys)

asyncio.run(main())
```

**Expected Behavior:**

`list_artifact_keys(session_id="s1")` should return `["f.txt"]`, since that
is the id the session itself is stored and reachable under.

**Observed Behavior:**

It returns `[]`. The artifact saved with the padded id `"s1\n"` is invisible
under the session's own (trimmed) id `"s1"`.

**Environment Details:**

- ADK Library Version: `main` (post-#6892/#6941/#6942)
- Desktop OS: N/A — reproduces regardless of OS
- Python Version: 3.11+

**Model Information:**

- Are you using LiteLLM: N/A
- Which model is being used: N/A — no model involved, this is the
session/artifact storage layer alone

---

## 🟡 Optional Information

**Regression:**

Not a regression from a previously-working state; the artifact services
never normalized `session_id`. It became reachable/exploitable in the same
way #6941 was: #6892 made `get_session`/`create_session` succeed under a
normalized id, which makes it easy to reach this artifact-layer split by
simply saving an artifact with the same (un-normalized) id used to create
the session.

**Additional Context:**

This same gap was independently flagged during review of #6942 (see that
PR's review thread) for `InMemoryArtifactService`/`FileArtifactService`,
and confirmed by the PR author as real but out of scope for that change.
While verifying it, `GcsArtifactService` was found to have the identical
gap as well (not previously mentioned).

Fix proposed in #6958: adds `artifact_util.normalize_session_id()` and
applies it at every entry point across all three artifact service
backends that builds a storage key/path or artifact-reference URI from a
caller-supplied `session_id`.

**How often has this issue occurred?:**

- Always (100%) — deterministic, no timing involved

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.