feat: add HAYSTACK_UNSAFE_DESERIALIZATION env var - #12397
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
topic:tests
topic:core
type:documentation
Contributor
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
bogdankostic
marked this pull request as ready for review
August 18, 2026 15:20
bogdankostic
requested review from
julian-risch
and removed request for
a team
August 18, 2026 15:20
sjrl
reviewed
Aug 19, 2026
Comment on lines
379
to
387
| raise DeserializationError( | ||
| f"Refusing to deserialize a class from module '{module_name}': the module is not on the " | ||
| f"trusted-module allowlist. If you trust the source of this serialized data, you can either:\n" | ||
| f" - extend the allowlist for this call: " | ||
| f"Pipeline.load(..., allowed_modules=['{module_name}']),\n" | ||
| f" - extend it process-wide via haystack.core.serialization.allow_deserialization_module" | ||
| f"('{module_name}') or the {DESERIALIZATION_ALLOWLIST_ENV_VAR} environment variable,\n" | ||
| f" - or bypass the allowlist entirely: Pipeline.load(..., unsafe=True)." | ||
| ) |
Contributor
There was a problem hiding this comment.
Should we update this message to include the other workaround?
The release note framed the switch as affecting Pipeline.load / loads / from_dict, but it is consulted by every deserialization path in the process, including ones with no unsafe argument of their own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reading HAYSTACK_UNSAFE_DESERIALIZATION fresh on every check made os.environ part of the deserialization control plane. os.environ.update is collections.abc.MutableMapping.update, and `collections` is on the default allowlist, so any allowlisted module that binds os.environ at module scope turns into a full bypass: a serialized handle resolves the mutator in safe mode, a Jinja custom_filters call sets the variable while the component is being constructed, and the rest of that same load runs with every check disabled. Read the variable once instead, on the first deserialization check in the process, and freeze the result. The first check necessarily happens while resolving the first handle of a load, before any deserialized data can run, so a hostile pipeline can no longer flip the switch mid-load or stage it for a later one. The read stays lazy rather than moving to import time so that a load_dotenv() before the first load still counts. This also drops the module-level _UNSAFE_ENV_TRUTHY set: a mutable container in this module is the same gadget class (a resolvable .add that could make the empty string truthy), and inlining the values removes the target entirely. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
julian-risch
approved these changes
Aug 19, 2026
julian-risch
left a comment
julian-risch
left a comment
Member
There was a problem hiding this comment.
Looks good to me now after I made the environment variable read-once only now. Without that, there was still a gap where an attacker could potentially change the value and thereby enable unsafe deserialization even if the env variable disabled it.
julian-risch
requested changes
Aug 19, 2026
julian-risch
left a comment
julian-risch
left a comment
Member
There was a problem hiding this comment.
I found an issue with this and discuss with Sebastian how to fix it. Until then, I need to block merging.
julian-risch
approved these changes
Aug 21, 2026
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
3 participants
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.
Related Issues
Proposed Changes:
Adds a process-wide
HAYSTACK_UNSAFE_DESERIALIZATIONenvironment variable that, when set to a truthy value (1ortrue), makes everyPipeline.load/Pipeline.loads/Pipeline.from_dictbehave as if it were called withunsafe=True— i.e. it disables all deserialization safety checks: the module allowlist, the builtin/import-primitive and control-plane denylists, the object-internals traversal guard, and the refusal to honor a component's ownunsafe: trueflag.It's intended for deployments that only ever load fully trusted pipelines and cannot thread
unsafe=Truethrough every call site.Implementation-wise,
_is_unsafe_deserialization()becomes the single source of truth for "are we in unsafe mode" and OR-s in the env var there, so every existing safety check inherits it with no other behavior change; the value is read fresh on each call, and a warning is logged once the first time it takes effect.How did you test it?
TestUnsafeDeserializationEnvVarintest/core/test_serialization_security.pycovering: truthy values (1,true,TRUE,True) bypass the module allowlist and the denied-builtins / import-primitive / control-plane / object-internals-traversal checks; falsey values (0,false,no,off,yes,on, empty) keep safe mode; a serializedOutputAdapterwithunsafe: trueis refused in safe mode but loads with the env var set; and the "safety disabled" warning is logged only once.Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.