Skip to content

Python: fix(core): refuse SecretString at checkpoint encode with a precise error - #9515

Open
Yufeng He (he-yufeng) wants to merge 1 commit into
microsoft:mainfrom
he-yufeng:fix/checkpoint-secret-string-encode
Open

Yufeng He (he-yufeng) wants to merge 1 commit into
microsoft:mainfrom
he-yufeng:fix/checkpoint-secret-string-encode

Conversation

@he-yufeng

Copy link
Copy Markdown
Contributor

Fixes #9514.

Why

Since #8127, SecretString is an immutable, slots-based, non-str wrapper with no __reduce__/__getstate__, so pickle protocols >= 2 dump it fine and fail at restore when the BUILD step hits the immutability guard. The checkpoint codec pickles every non-JSON-native value, so a credential in executor/session state could not round-trip: FileCheckpointStorage.save() refused it with the save-time restore check from #8214, but as an opaque decode failure (Failed to decode pickled checkpoint data: SecretString is immutable.), and Cosmos / Foundry-hosted storage share the codec without that check, so save() succeeded and only load() failed later. The obvious workaround (unwrap to str first) writes the raw secret into the checkpoint in cleartext.

What changed

One check in the codec's pickle funnel (_encode_pickle): a SecretString now raises a WorkflowCheckpointException at encode time saying why (unpicklable by design; a working pickle would persist the credential in cleartext) and what to do instead (keep credentials out of checkpointed state and re-resolve them from settings or configuration on restore). This matches the durable session-state path, which already rejects the type with a precise error via _validate_durable_state_value (#8874), and because the refusal lives in the shared codec it fires at save time on every backend, closing the Cosmos load-time asymmetry.

Scope notes: a SecretString nested in a plain dict or list is caught by the same recursion; one buried in a custom object still fails the save-time round-trip check with the generic wrapper, unchanged from before. Checkpoints already persisted with an embedded credential still fail at load with the wrapped decode error; they were never loadable, this change only moves the failure earlier and makes it legible.

Tests

  • test_checkpoint_encode.py: direct value, dict-nested, and list-nested SecretString all raise the precise error at encode; all three go red if the check is removed (mutation-verified).
  • test_checkpoint.py: FileCheckpointStorage.save() surfaces the precise error unwrapped, proving the except WorkflowCheckpointException: raise pass-through keeps the message intact.
  • The issue's repro shape run standalone against this tree now produces the precise save-time error instead of the opaque decode one.
  • Full test_checkpoint_encode.py + test_checkpoint.py green (114 passed); the other checkpoint suites green except two test_restricted_decode_allows_openai* failures that reproduce identically on pristine main in this environment (stale openai package in the local venv, unrelated). pyright clean on the touched module.

…ecise error

Since microsoft#8127 SecretString is an immutable, slots-based non-str wrapper with
no __reduce__/__getstate__, so pickle protocols >= 2 dump it fine and fail
at restore on the immutability guard. FileCheckpointStorage's save-time
round-trip check (microsoft#8214) surfaced that as an opaque decode error at save;
Cosmos and Foundry-hosted storage skip that check, so the same state saved
cleanly and only failed on resume. Unwrapping to str first, the obvious
workaround, writes the credential to durable storage in cleartext.

The checkpoint codec now refuses a SecretString at encode time with a
WorkflowCheckpointException that says why (unpicklable by design, and a
successful pickle would persist the secret in cleartext) and what to do
instead (keep credentials out of checkpointed state, re-resolve from
settings or configuration on restore). This matches the durable session
state path, which already rejects the type with a precise error (microsoft#8874),
and because the refusal lives in the shared codec it lands at save time
on every storage backend, closing the Cosmos load-time asymmetry.

Nested SecretString values inside plain dict/list state are caught by the
same recursion. A SecretString buried in a custom object still fails the
round-trip check with the generic wrapper; that edge is unchanged.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Whole-dictionary pickle fallbacks still bypass rejection, allowing credential-bearing checkpoints to be saved by Cosmos but not restored.

1 open issue
What changed in this PR

Addresses #9514 by rejecting SecretString during checkpoint encoding with actionable guidance instead of an opaque restore error.

Changes:

  • Adds a guard in the shared pickle encoder.
  • Tests direct and nested secrets and File storage error propagation.
File Description
python/​packages/​core/​tests/​workflow/​test_checkpoint.py Tests save-time error propagation.
python/​packages/​core/​tests/​workflow/​test_checkpoint_encode.py Tests direct, dict-nested, and list-nested rejection.
python/​packages/​core/​agent_framework/​_workflows/​_checkpoint_encoding.py Adds explicit credential rejection and recovery guidance.

Review effort: Balanced

# credential to durable storage in cleartext. Refuse at encode time with a
# precise error instead of the opaque restore failure (#9514); the session
# state path already rejects it the same way (#8874).
if isinstance(value, SecretString):

This branch was successfully deployed

1 active deployment
github-app-auth — 7b32ee06 Deployed Oct 18, 2026 by he-yufeng via team_check #7034
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: SecretString cannot be pickled, so credential-bearing workflow checkpoints cannot be saved or restored

2 participants