google / google/adk-python

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

Ouverte
#7,030 2 commentaires 0 réactions 1 personne assignée Réclamée par @sanketpatil06 Voir sur GitHub
services
Langage dominant
Python
Étoiles
21.5k
Forks
4k
Merge moyen
1 j 14 h
PR mergées (30 j)
37

Description

## 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

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.