[bhaskargurram-ai]

Summary

Closes #4404.

• Decoding with the numcodecs pickle codec runs pickle.loads on chunk bytes from the store, so reading an untrusted Zarr v2 array can execute arbitrary code. I reproduced this on main: a .zarray with filters=[{"id": "vlen-utf8"}], compressor={"id": "pickle"} opens fine, and indexing it unpickles the chunk (a __reduce__ payload runs).
• New config option array.allow_pickle (default False). It can be enabled with zarr.config.set({"array.allow_pickle": True}) or with the env var ZARR_ARRAY__ALLOW_PICKLE=True (donfig's usual env mapping), matching the opt-in style of array.rectilinear_chunks.
• The check sits in V2Codec._decode_sync and runs before any codec. It matches a compressor or filter whose codec_id == "pickle". This is the only place pickle-encoded chunks get decoded; v3 has no pickle codec registered. Without the opt-in it raises a new zarr.errors.PickleCodecDisabledError (a BaseZarrError/ValueError). The message names the config key and the env var.
• Only True enables it (is True, not a truthiness check), because donfig keeps an env value it can't literal_eval (e.g. ZARR_ARRAY__ALLOW_PICKLE=false) as a non-empty string, which is truthy.
• Opening an array and reading its metadata still work. So does writing with the pickle codec: encoding only calls pickle.dumps on the user's own data, so there is no untrusted input. A partial-chunk write that has to read the existing chunk back will hit the check, which is expected.
• Docs: added a bullet to docs/user-guide/config.md. Changelog: changes/4404.feature.md.

For reviewers

• Is array.allow_pickle the name/namespace you want? It is not an ArrayConfig field, so it can only be set globally, not per array via config=. The same is true of array.rectilinear_chunks.
• Gating at decode time (instead of when get_numcodec resolves the codec from metadata) keeps groups listable and metadata inspectable. Would you rather it fail earlier, at open?
• Should writes with the pickle codec also be gated, or should a warning be emitted?
• Should this be a removal/breaking changelog entry rather than feature, since reads that worked before now raise by default?

Author attestation

• I am a human, these are my changes, and I have reviewed and understood every change and can explain why each is correct.

TODO

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/user-guide/*.md
• Changes documented as a new file in changes/
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

Tests (tests/test_v2.py): default read raises; a crafted malicious chunk is not unpickled; opt-in via zarr.config.set; opt-in via env var (True enables, false does not); a non-pickle v2 array reads without opting in. tests/test_config.py defaults updated.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.71%. Comparing base (f19c19c) to head (d5a3e70).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4465   +/-   ##
=======================================
  Coverage   94.71%   94.71%           
=======================================
  Files          94       94           
  Lines       13663    13672    +9     
=======================================
+ Hits        12941    12950    +9     
  Misses        722      722           

Files with missing lines	Coverage Δ
src/zarr/codecs/_v2.py	94.91% <100.00%> (+0.79%)	⬆️
src/zarr/core/config.py	100.00% <ø> (ø)
src/zarr/errors.py	100.00% <100.00%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[bhaskargurram-ai]

@d-v-b this is ready for a look when you have a moment. The main open question is the option name (array.allow_pickle) and whether writes should be gated too, so I’m happy to adjust either.

[bhaskargurram-ai]

@d-v-b a short follow-up on #4404. This makes reading pickle-codec chunks opt-in through array.allow_pickle (default False), because unpickling chunk data can run arbitrary code. Opening arrays and writing are unchanged. The two open points are the option name and whether writes should be gated too. Could you take a look when there’s time?

[d-v-b]

thanks for this @bhaskargurram-ai, in the interest of speed I pushed some changes on top of yours to leverage the patched version of numcodecs we shipped last night which gates pickle decode behind a flag that defaults to false.

• pyproject.toml now sets the minimum supported version of numcodecs to 0.16.6 (released yesterday with the patch), and excludes 0.17.0 (because that version is unpatched). folks on the 0.17.x series of numcodecs use the patched 0.17.1 version.
• the tests that read pickled data now set the numcodecs flag to enable decoding
• the PickleCodecDisabledError now references the new gate built in to numcodecs.

[bhaskargurram-ai]

Thanks @d-v-b, building on the numcodecs gate makes sense. Looks good to me.

[d-v-b]

I pushed a change that makes zarr use the same name as numcodecs for the "disallow pickle decode" flag. it's a bit longer (array.allow_unsafe_pickle_decode) but more explicit and consistent with numcodecs.