Skip to content

fix(migration): restrict unpickling of v0 actions blobs#5866

Open
White-Mouse wants to merge 3 commits into
google:mainfrom
White-Mouse:codex/restrict-migration-unpickler
Open

fix(migration): restrict unpickling of v0 actions blobs#5866
White-Mouse wants to merge 3 commits into
google:mainfrom
White-Mouse:codex/restrict-migration-unpickler

Conversation

@White-Mouse
Copy link
Copy Markdown

@White-Mouse White-Mouse commented May 27, 2026

What

The v0 session schema stored event actions as pickled blobs. The migration helper reads raw bytes via SELECT * FROM events and previously used pickle.loads(...) directly.

This PR replaces that load with a small restricted unpickler allowlist (builtin containers/primitives + google.adk.events.event_actions.EventActions) and adds regression coverage for both safe and unsafe legacy actions payloads.

Why

pickle is not safe for untrusted inputs. Migration tooling often runs against restored/backed-up DB files or shared storage; failing closed here reduces the blast radius if the source DB contents are compromised.

Associated Issue / Background

No existing GitHub issue is linked. This was found while reviewing the v0-to-v1 migration path for unsafe deserialization risks in legacy session data.

Compatibility / fail-closed boundary

Normal v0 EventActions payloads made from primitive/container fields continue to migrate. The tests cover state_delta and artifact_delta round-tripping through migration.

This PR intentionally does not allow arbitrary nested model globals while unpickling. If a legacy actions blob contains nested objects that require additional globals (for example AuthConfig, ToolConfirmation, EventCompaction, or their transitive model types), the restricted unpickler rejects that blob; the migration logs a warning and falls back to empty EventActions() for that event. That preserves migration availability without importing lower-trust pickle globals.

Verification

  • uv run pytest -q tests/unittests/sessions/migration/test_migration.py
    • 20 passed, 2 warnings
  • uv run --extra dev pyink --check tests/unittests/sessions/migration/test_migration.py

@google-cla
Copy link
Copy Markdown

google-cla Bot commented May 27, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@adk-bot adk-bot added the services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc label May 27, 2026
@adk-bot
Copy link
Copy Markdown
Collaborator

adk-bot commented May 27, 2026

Response from ADK Triaging Agent

Hello @White-Mouse, thank you for submitting this pull request!

To help us review and process your contribution more efficiently, please address the following items in accordance with our contribution guidelines:

  1. Contributor License Agreement (CLA): It looks like the CLA check has failed. Please make sure you (or your employer) have signed the Google CLA so we can proceed with the review.
  2. Testing / Verification Plan: The Verification section in your PR description is currently empty. Please update this section with your testing plan or a summary of your test results (e.g., confirming unit tests under tests/unittests/sessions/migration/test_migration.py passed).
  3. Associated Issue: If there is an existing GitHub issue related to this bug/fix, please link it in your PR description. If not, consider creating one or briefly describing the background context.

Thank you for your cooperation!

@White-Mouse White-Mouse force-pushed the codex/restrict-migration-unpickler branch from d09e22e to 71c4053 Compare May 27, 2026 14:45
@rohityan rohityan self-assigned this May 27, 2026
@wyf7107 wyf7107 requested a review from DeanChensj May 27, 2026 19:00
@rohityan
Copy link
Copy Markdown
Collaborator

Hi @White-Mouse , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Can you please fix the failing mypy-diff tests before we can proceed with the review.

@rohityan rohityan added the request clarification [Status] The maintainer need clarification or more information from the author label May 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

request clarification [Status] The maintainer need clarification or more information from the author services [Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants