Skip to content

InMemoryArtifactService aliases distinct app/user scopes when identifiers contain slashes #7706

Description

@BichengWang

🔴 Required Information

Describe the Bug:

InMemoryArtifactService stores artifacts under {app_name}/{user_id}/{session_id}/{filename}. The shared validator accepts embedded / in app and user identifiers, so distinct (app_name, user_id) pairs can address the same stored artifact. This affects reads, version allocation, and deletion.

Steps to Reproduce:

  1. Run the code below against current main, 6b00a38901e7181801f645e5890da54aa322b6dc.
  2. Save an artifact under app team/project, user alice.
  3. Read, save, and delete under app team, user project/alice, using the same session and filename.

Expected Behavior:

The two app/user scopes have separate artifacts and version histories, or ambiguous identifiers are rejected before storing anything. The second scope cannot read or delete the first scope's artifact.

Observed Behavior:

Both scopes address team/project/alice/s1/notes.txt. The second scope reads the first value, its first save returns version 1, and deleting through it removes the first scope's artifact.

Environment Details:

  • ADK: source checkout reports 2.12.0, main commit 6b00a38901e7181801f645e5890da54aa322b6dc.
  • OS: macOS 27.0.1.
  • Python: 3.11.17.
  • Backend: InMemoryArtifactService.

Model Information:

  • LiteLLM: N/A.
  • Model: N/A; entirely local, no model calls or credentials.

🟡 Optional Information

Regression: Unknown; reproduced on current main.

Minimal Reproduction Code:

import asyncio
from google.adk.artifacts.in_memory_artifact_service import InMemoryArtifactService
from google.genai import types

async def main():
    service = InMemoryArtifactService()
    first = dict(app_name="team/project", user_id="alice",
                 session_id="s1", filename="notes.txt")
    second = dict(app_name="team", user_id="project/alice",
                  session_id="s1", filename="notes.txt")
    await service.save_artifact(**first, artifact=types.Part(text="first scope"))
    other = await service.load_artifact(**second)
    print("second scope before saving:", other.text if other else None)
    version = await service.save_artifact(
        **second, artifact=types.Part(text="second scope"))
    print("second scope first save version:", version)
    print("first scope after second save:",
          (await service.load_artifact(**first)).text)
    await service.delete_artifact(**second)
    print("first scope after second delete:",
          await service.load_artifact(**first))

asyncio.run(main())

Logs:

second scope before saving: first scope
second scope first save version: 1
first scope after second save: second scope
first scope after second delete: None

Additional Context:

Related closed reports #6973 and #7067 describe FileArtifactService collisions involving reserved sessions/artifacts segments. This reproduction uses InMemoryArtifactService and no reserved segments or traversal. I have not tested a real GCS backend. This is a service-level reproduction; it does not establish reachability through the HTTP server.

How often has this issue occurred?: Always in repeated local runs.

Activity

  1. widechaos commented on Oct 23, 2026

    @widechaos

    May I take this on if nobody is already working on it? I reproduced it on current main (ceb0627f). The same component-boundary collisions occur in GcsArtifactService against the upstream fake bucket; FileArtifactService keeps these scopes separate.

    The landed fix for #6973/#7067 preserves ordinary hierarchical IDs, and rejecting all separators breaks the existing namespaced-ID tests. Should a fix preserve those IDs with component-aware storage keys? That would also need an agreed GCS object naming/migration policy. I have only tested a local Memory prototype, with no production patch or real GCS access.

  2. mittalpk commented on Oct 23, 2026

    @mittalpk

    I'd like to work on this. Two ways to fix it: reject / in app_name and user_id in the shared validator, or keep accepting it and escape each segment when building the storage key. Rejecting is simpler but could break callers that already use slashes. Do you have a preference?

  3. mittalpk commented on Oct 23, 2026

    @mittalpk

    widechaos asked first, so I'll step back. Happy to help review.

  4. xxwtiancai commented on Oct 23, 2026

    @xxwtiancai

    I'd like to take this one, with the storage-side option from your expected behavior (separate scopes per identifier tuple, rather than rejecting identifiers — the validator allowing embedded / looks deliberate and rejecting it would affect every backend).

    Approach: key the store by a namespace tag (u//s/) plus percent-encoded segments, so an embedded / can never shift a segment boundary and list_artifact_keys matches exact encoded prefixes instead of raw-string prefixes. The public artifacts field keeps its dict[str, list[_ArtifactEntry]] type; only the key encoding changes (keys were already internal — the existing test suite computes them through _artifact_path). No changes to the shared validator, URIs, or other backends.

    Cross-scope regression tests for both collision directions (read, version allocation, delete, listing); revert control on unmodified main fails both. PR in a moment — @BichengWang thanks for the report, the repro made this quick.

  5. widechaos commented on Oct 24, 2026

    @widechaos

    I'll leave the Memory fix to #7707 and keep my prototype local. My GCS observation used the fake bucket; the GCS object layout and migration policy still need agreement.

  6. surajksharma07 commented on Oct 26, 2026

    @surajksharma07
    Collaborator

    Reproduced the same aliasing on main (2.13.0) @BichengWang. Read, version, list and delete all cross over between the two scopes.

    As a workaround tried percent-encoding each segment with a u/ or s/ namespace tag in the in-memory key (same idea as #7707) and the scopes stay separate with user: artifacts still listed fine.

    @xxwtiancai could you also check list_artifact_keys with a "/" inside the filename and an existing hierarchical app_name just to make sure nothing regresses before we push #7707 for review?

    GCS seems to have the same pattern per @widechaos so would suggest keeping that as a separate follow-up since it needs a migration decision.

  7. xxwtiancai commented on Oct 26, 2026

    @xxwtiancai

    @surajksharma07 thanks for reproducing and for validating the same approach — both requested checks are done, pinned as regression tests in 59513d5 (PR #7707):

    1. list_artifact_keys with a / inside the filename: sub/notes.txt (session scope) and user:preferences/sub/style.json (user namespace) both round-trip exactly — save, listing returns the caller's original spelling, load succeeds. This passes both on the unfixed resolver and with the fix, so it is a pure no-regression guard; the filename segment is percent-encoded like every other segment, so a / in it cannot be mistaken for a scope boundary.

    2. Existing hierarchical app_name: saved under ("team/project", "alice"), ("team", "project/alice"), and ("team/project", "bob"); each list_artifact_keys call returns exactly its own scope's files. Notably this one fails on the unfixed resolver (the aliased scope's file leaks into the listing) and passes with the fix, so the fix actively makes hierarchical-app listings scope-exact rather than merely not regressing them.

    Full artifacts suite with the addition: 1127 passed. #7707 is ready for review whenever you are — and agreed on keeping the GCS pattern as a separate follow-up pending the migration decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions