Security: Unsafe pickle.loads() in v0 schema runtime path and migration tool

@daridor9

Summary

Two code paths in ADK Python call pickle.loads() on data read from the session database with no type restriction, integrity check, or opt-in gate. A single poisoned row in events.actions (BLOB column) achieves arbitrary code execution in any ADK process that reads the session — including the official migration tool.

Affected Code

Sink 1 — Runtime read path
src/google/adk/sessions/schemas/v0.py:97 — DynamicPickleType.process_result_value()

if dialect.name in ("spanner+spanner", "mysql"):
    return pickle.loads(value)   # no guards

SQLite falls through to PickleType.result_processor() which calls self.pickler.loads(value) — same effect. All three supported dialects (SQLite, MySQL, Cloud Spanner) are affected.

Sink 2 — Migration path
src/google/adk/sessions/migration/migrate_from_sqlalchemy_pickle.py:62

if isinstance(actions_val, bytes):
    actions = pickle.loads(actions_val)   # no guards

This runs on every existing row during the recommended v0→v1 migration (adk migrate session). The vendor remediation path is itself an exploit trigger on a poisoned database.

Affected Versions

All google-adk releases through current (v1.31.1).

< 1.22.0 — v0 (pickle) was the only schema
1.22.0+ — v1 (JSON) is the default for new databases; existing v0 databases continue using the pickle path (docs confirm)

The pickle code path is not deprecated, not warning-gated, and not behind a feature flag.

Impact

RCE in any ADK process reading events from a v0-schema database
Blast radius: one poisoned row compromises every reader until the row is manually deleted
Persistence: payload survives process restarts, container rebuilds, and ADK version upgrades
Cross-authority: write-authority (e.g., dev environment, adjacent service with DB access) and execution-authority (production ADK process) can be distinct principals — the database is treated as a trust boundary that pickle collapses
Migration trap: running adk migrate session on a poisoned DB executes every planted payload under the production service account

Suggested Remediation

Fail-closed v0 reads (preferred): refuse to load v0-schema databases unless explicitly opted in via ADK_ALLOW_UNSAFE_V0_PICKLE=1. Forces operators to acknowledge risk rather than inheriting it silently.
RestrictedUnpickler: if v0 must remain loadable, allowlist only EventActions and known field types. Apply to both Sink 1 and Sink 2.
HMAC on serialized data before storage as defense-in-depth.
Long-term: deprecate v0, remove v0.py.

CVSS 3.1

9.9 Critical — AV:N/AC:L/PR:L/UI:N/S:C/C:H/I:H/A:H

(S:U fallback: 8.8 High)

Disclosure

Reported to Google VRP: 2026-04-24
GHSA filed
90-day coordinated disclosure deadline: 2026-07-23
This issue is filed per Google VRP guidance to work with maintainers via public issues

Discovered by SPR{K}3 Security Research (Dan Aridor — @daridor9)

security: add RestrictedUnpickler for safe v0 pickle deserialization

security: use RestrictedUnpickler in v0 runtime read path

security: use RestrictedUnpickler in v0 migration path

@daridor9

@daridor9 the detail here made it straightforward to trace both sinks.

Looking at #5635, the RestrictedUnpickler approach seems like the right direction.
Could you verify on your end that the find_class() allowlist covers all types your existing pickled actions rows actually contain — particularly anything stored in state_delta or agent_state fields?
Those are dict[str, Any] and could carry types beyond the core EventActions hierarchy, so it's worth making sure nothing legitimate gets blocked.

If it holds up across your test cases and both patched paths behave correctly end-to-end, that'd be a solid signal to move the PR into formal review!

security: add enum types to RestrictedUnpickler allowlist

@surajksharma07

@surajksharma07 Verified. 27/27 tests pass.

Ran end-to-end against google-adk with the RestrictedUnpickler allowlist covering all EventActions field types:

Legitimate types — all deserialize correctly:

Primitives in state_delta/agent_state (dict[str, Any]): str, int, float, bool, None, list, dict, bytes, tuple ✓
Collections: OrderedDict, defaultdict ✓
Datetime types: datetime, date, time, timedelta, timezone ✓
Full EventActions round-trip: minimal, with state_delta + agent_state, with transfer_to_agent + escalate, with artifact_delta ✓

Malicious payloads — all blocked:

os.system, subprocess.Popen, posix.system, builtins.__import__, nt.system, eval ✓
Crafted STACK_GLOBAL + REDUCE opcode payload ✓

Also added enum types to the allowlist (commit d9a0f7a) as safety margin for any ADK/genai enums in Any-typed dict values.

Full test output:

============================================================
RestrictedUnpickler E2E Test — v0 EventActions Compatibility
============================================================

1. Primitive types (state_delta / agent_state values)
  [PASS] str_val: str
  [PASS] int_val: int
  [PASS] float_val: float
  [PASS] bool_val: bool
  [PASS] none_val: NoneType
  [PASS] list_val: list
  [PASS] dict_val: dict
  [PASS] bytes_val: bytes
  [PASS] tuple_val: tuple

2. Collections (OrderedDict, defaultdict)
  [PASS] ordered_dict
  [PASS] defaultdict_int

3. Datetime types
  [PASS] datetime
  [PASS] date
  [PASS] time
  [PASS] timedelta
  [PASS] timezone_utc

4. Full EventActions round-trip
  [PASS] EventActions (minimal)
  [PASS] EventActions (state_delta + agent_state)
  [PASS] EventActions (transfer + escalate)
  [PASS] EventActions (artifact_delta)

5. Malicious payloads (must be blocked)
  [PASS] BLOCKED os.system
  [PASS] BLOCKED subprocess.Popen
  [PASS] BLOCKED posix.system
  [PASS] BLOCKED builtins.__import__
  [PASS] BLOCKED nt.system (Windows)
  [PASS] BLOCKED eval

6. Crafted REDUCE opcode payload
  [PASS] BLOCKED crafted STACK_GLOBAL+REDUCE

============================================================
Results: 27/27 passed, 0 failed
ALL TESTS PASSED
============================================================

Both patched paths (Sink 1 runtime + Sink 2 migration) use the same safe_loads() entry point. Escape hatch via ADK_ALLOW_UNSAFE_V0_PICKLE=1 is there if anyone hits an edge case.

Ready for formal review whenever you are.

style: run pyink + isort on _safe_unpickle.py

style: run pyink + isort on v0.py

style: run pyink + isort on migrate_from_sqlalchemy_pickle.py

@daridor9

@daridor9 The 27/27 coverage is exactly the signal needed — enumerating both the legitimate round-trips and the explicit payload categories (os/subprocess/builtins/crafted opcodes) leaves no ambiguity about what the allowlist stops.
The enum addition and the ADK_ALLOW_UNSAFE_V0_PICKLE=1 escape hatch with the deprecation warning are both the right calls.

Things to address before #5635 can land:

1. SQLite path is still unguarded.

The PR patches process_result_value(), which handles MySQL and Spanner because load_dialect_impl() returns LONGBLOB/SpannerPickleType for those dialects — raw bytes reach process_result_value() directly. For SQLite, load_dialect_impl() returns PickleType (the impl), and SQLAlchemy's TypeDecorator calls PickleType.result_processor() → self.pickler.loads() before process_result_value() is invoked.
By the time process_result_value() runs for SQLite, the data is already deserialized.
To cover SQLite, DynamicPickleType needs to also override result_processor() for non-MySQL/Spanner dialects, replacing self.pickler.loads with safe_loads.

2. Unit tests in CI.
The E2E script confirms the fix works, but the PR has no test files.
A tests/unittests/sessions/test_safe_unpickle.py covering at minimum — blocked payloads raise UnpicklingError, all four EventActions round-trips succeed, and ADK_ALLOW_UNSAFE_V0_PICKLE=1 triggers the fallback — would prevent this from silently regressing in a future refactor.

Once those are in, the PR is in good shape for review!

@surajksharma07

@surajksharma07 Both addressed — added result_processor() override for the SQLite path and test_safe_unpickle.py with coverage for blocked payloads, EventActions round-trips, and env var fallback.

H, Both Addressed Best Dan From: Suraj Sharma ***@***.***> Date: Monday, 11 May 2026 at 16:50 To: google/adk-python ***@***.***> Cc: Dan Aridor ***@***.***>, Mention ***@***.***> Subject: Re: [google/adk-python] Security: Unsafe pickle.loads() in v0 schema runtime path and migration tool (Issue #5634) [https://avatars.githubusercontent.com/u/228440606?s=20&v=4]surajksharma07 left a comment (google/adk-python#5634)<#5634 (comment)> @daridor9<https://github.com/daridor9> The 27/27 coverage is exactly the signal needed — enumerating both the legitimate round-trips and the explicit payload categories (os/subprocess/builtins/crafted opcodes) leaves no ambiguity about what the allowlist stops. The enum addition and the ADK_ALLOW_UNSAFE_V0_PICKLE=1 escape hatch with the deprecation warning are both the right calls. Things to address before #5635<#5635> can land: 1. SQLite path is still unguarded. The PR patches process_result_value(), which handles MySQL and Spanner because load_dialect_impl() returns LONGBLOB/SpannerPickleType for those dialects — raw bytes reach process_result_value() directly. For SQLite, load_dialect_impl() returns PickleType (the impl), and SQLAlchemy's TypeDecorator calls PickleType.result_processor() → self.pickler.loads() before process_result_value() is invoked. By the time process_result_value() runs for SQLite, the data is already deserialized. To cover SQLite, DynamicPickleType needs to also override result_processor() for non-MySQL/Spanner dialects, replacing self.pickler.loads with safe_loads. 2. Unit tests in CI. The E2E script confirms the fix works, but the PR has no test files. A tests/unittests/sessions/test_safe_unpickle.py covering at minimum — blocked payloads raise UnpicklingError, all four EventActions round-trips succeed, and ADK_ALLOW_UNSAFE_V0_PICKLE=1 triggers the fallback — would prevent this from silently regressing in a future refactor. Once those are in, the PR is in good shape for review! — Reply to this email directly, view it on GitHub<#5634 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BIC7DI4ZCN4NZMIAKYWZIW342HLATAVCNFSM6AAAAACYU64TTWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DIMRRGMYTQNBWHA>. You are receiving this because you were mentioned.Message ID: ***@***.***>

@daridor9

@daridor9 Both gaps are closed — the result_processor() override is the right fix for SQLite: it intercepts the value before PickleType.result_processor() can call self.pickler.loads(), the memoryview → bytes coercion handles SQLite's raw BLOB return correctly, and MySQL/Spanner continue through super().result_processor() → process_result_value() → safe_loads unchanged.

The test file covers all the critical paths cleanly — blocked payloads, env-var fallback, and round-trip cases.

One small gap worth noting: TestEventActionsRoundTrip exercises plain dicts rather than actual EventActions instances.
It's low risk since all google.adk.* prefixes are allowed, but a quick smoke test with a real EventActions() object would close that loop entirely.

@surajksharma07

@surajksharma07 Added — TestRealEventActionsRoundTrip with four cases using real EventActions() instances: minimal (empty defaults), populated state_delta + artifact_delta, transfer_to_agent + escalate + skip_summarization, and nested dict[str, Any] values in state_delta. All assert isinstance(result, EventActions) after the round-trip

@daridor9

That closes the last gap, @daridor9 — four real EventActions() instances with isinstance(result, EventActions) on each round-trip is the right form of proof that the full object hierarchy (not just its field types in isolation) survives safe_loads end-to-end, including the nested dict[str, Any] case that's most likely to surface edge-case types in practice.

Every item flagged across the review is now addressed: the allowlist and enum coverage, the SQLite result_processor() override, the CI test file with blocked payloads and env-var fallback, and now the real object round-trips.
The test suite is thorough enough to catch any future regression from an allowlist edit.

Team is looking more into it.
Thanks!

security: add RestrictedUnpickler for safe v0 pickle deserialization

security: use RestrictedUnpickler in v0 runtime read path

security: add enum types to RestrictedUnpickler allowlist

style: run pyink + isort on _safe_unpickle.py

style: run pyink + isort on v0.py

style: run pyink + isort on migrate_from_sqlalchemy_pickle.py

fix: restrict unpickling of legacy v0 session event actions

Navigation Menu

Repository navigation

Security: Unsafe pickle.loads() in v0 schema runtime path and migration tool #5634

Summary

Affected Code

Affected Versions

Impact

Suggested Remediation

CVSS 3.1

Disclosure

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Security: Unsafe pickle.loads() in v0 schema runtime path and migration tool #5634

Description

Summary

Affected Code

Affected Versions

Impact

Suggested Remediation

CVSS 3.1

Disclosure

Activity

surajksharma07 commented on May 7, 2026

daridor9 commented on May 7, 2026

surajksharma07 commented on May 11, 2026

daridor9 commented on May 11, 2026

daridor9 commented on May 13, 2026

surajksharma07 commented on May 18, 2026

daridor9 commented on May 18, 2026

surajksharma07 commented on May 25, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions