Repository navigation
Python: fix(core): refuse SecretString at checkpoint encode with a precise error - #9515
Open
Yufeng He (he-yufeng) wants to merge 1 commit into
Open
Yufeng He (he-yufeng) wants to merge 1 commit into
Yufeng He (he-yufeng) wants to merge 1 commit into
Conversation
…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.
Yufeng He (he-yufeng)
requested review from
SergeyMenshykh,
Tao Chen (TaoChenOSU),
Eduard van Valkenburg (eavanvalkenburg),
Giles Odigwe (giles17),
Jose Alvarez (jpalvarezl),
Evan Mattson (moonbox3),
Roger Barreto (rogerbarreto),
sophia-ramsey and
westey (westey-m)
as code owners
October 18, 2026 21:26
Yufeng He (he-yufeng)
deployed
to
github-app-auth
October 18, 2026 21:26 — with
GitHub Actions
Active
Yufeng He (he-yufeng)
deployed
to
github-app-auth
October 18, 2026 21:26 — with
GitHub Actions
Active
Contributor
There was a problem hiding this comment.
🟡 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Fixes #9514.
Why
Since #8127,
SecretStringis an immutable, slots-based, non-strwrapper with no__reduce__/__getstate__, so pickle protocols >= 2 dump it fine and fail at restore when theBUILDstep 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, sosave()succeeded and onlyload()failed later. The obvious workaround (unwrap tostrfirst) writes the raw secret into the checkpoint in cleartext.What changed
One check in the codec's pickle funnel (
_encode_pickle): aSecretStringnow raises aWorkflowCheckpointExceptionat 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
SecretStringnested 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-nestedSecretStringall 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 theexcept WorkflowCheckpointException: raisepass-through keeps the message intact.test_checkpoint_encode.py+test_checkpoint.pygreen (114 passed); the other checkpoint suites green except twotest_restricted_decode_allows_openai*failures that reproduce identically on pristinemainin this environment (stale openai package in the local venv, unrelated). pyright clean on the touched module.