)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"c3e2861888889441fb242998c3b2fb6b11c507b8","unresolved":true,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Now, harden our pickle unmarshalling: only allow a subset of imports"},{"line_number":16,"context_line":"which should all be safe. Apply this to all places where we unpickle,"},{"line_number":17,"context_line":"not just hashes."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Partial-Bug: #2152384"},{"line_number":20,"context_line":"Change-Id: I00e27a402466cb24c94b22e5bde6d707a05c3609"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5d2672b8_5f73e186","line":17,"updated":"2026-05-18 09:49:58.000000000","message":"Should we mention that the ring/builder.py is an exception here?","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e71313ffcc1871bc639dea320ead7b556a0d5100","unresolved":true,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Now, harden our pickle unmarshalling: only allow a subset of imports"},{"line_number":16,"context_line":"which should all be safe. Apply this to all places where we unpickle,"},{"line_number":17,"context_line":"not just hashes."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Partial-Bug: #2152384"},{"line_number":20,"context_line":"Change-Id: I00e27a402466cb24c94b22e5bde6d707a05c3609"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"b6d2b424_e5458ef8","line":17,"in_reply_to":"5d2672b8_5f73e186","updated":"2026-05-18 21:37:08.000000000","message":"Good point -- I think I mostly hoped that we could just drop support for that Real Soon™️","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d938b3d856bff9c423a30409d18ba07a19ccbe3f","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Now, harden our pickle unmarshalling: only allow a subset of imports"},{"line_number":16,"context_line":"which should all be safe. Apply this to all places where we unpickle,"},{"line_number":17,"context_line":"not just hashes."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Partial-Bug: #2152384"},{"line_number":20,"context_line":"Change-Id: I00e27a402466cb24c94b22e5bde6d707a05c3609"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"863a22f2_e96b3664","line":17,"in_reply_to":"b6d2b424_e5458ef8","updated":"2026-05-26 23:37:13.000000000","message":"Done","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d938b3d856bff9c423a30409d18ba07a19ccbe3f","unresolved":true,"context_lines":[{"line_number":18,"context_line":"security boundaries and required-supported data types are radically"},{"line_number":19,"context_line":"different from other pickle usage."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"Partial-Bug: #2152384"},{"line_number":22,"context_line":"Co-Authored-By: Christian Schwede \u003ccschwede@mailbox.org\u003e"},{"line_number":23,"context_line":"Co-Authored-By: Alistair Coles \u003calistairncoles@gmail.com\u003e"},{"line_number":24,"context_line":"Assisted-by: Claude:claude-4.7-opus"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"aa6a7537_99d2a518","line":21,"updated":"2026-05-26 23:37:13.000000000","message":"I\u0027m torn about whether to call this a `Partial-Bug` or `Closes-Bug`. On the one hand, we\u0027re still using `pickle`, and will continue until we can follow-up https://review.opendev.org/c/openstack/swift/+/988827 to first change the default and second get rid of the option.\n\nOn the other hand, this should be sufficient to resolve any security concerns about the continued use of `pickle`.","commit_id":"6258659a76f6d80a8c8be47603f0482746f0f4fb"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"c3e2861888889441fb242998c3b2fb6b11c507b8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5ea1f6be_e36e45a0","updated":"2026-05-18 09:49:58.000000000","message":"I did test this on my SAIO with a modified hashes.pkl, and with this patch applied it prevents calls that are not allowed. I do have a few comments inline, pls have a look.\n\nWe probably also want some unit tests?\n\nI\u0027m close to a +2 b/c this fixes the issue in my tests, and we could improve in a follow up as well.","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"8ae6e92b7da2d9ec0c285741f3da196ea82ab87e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"acef2fdc_74052e14","updated":"2026-05-19 16:02:49.000000000","message":"I think there is no real blocker, a couple of things can be fixed in a follow up.","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"f06ac07afe84f5b461b5e288c2919dc00cf59f59","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e7bcf12e_5a9341ab","updated":"2026-05-21 11:45:30.000000000","message":"Follow up tests in https://review.opendev.org/c/openstack/swift/+/989513/1 - can be also squashed into this if wanted.","commit_id":"8d63fbf96945b79868724e246cc38a6a0177e887"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"b14143d4b1373bbd736872637c1a2ac4fdc7a050","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"83c3fdf4_0d2d91d9","updated":"2026-05-21 11:44:40.000000000","message":"I\u0027m fine with this as it is.\n\nI rebased this patch on top of master (which now has Alistairs Timestap test patches merged) and removed the TimeStamp allowance as well as the \"??\" comment.","commit_id":"8d63fbf96945b79868724e246cc38a6a0177e887"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f2f1c0cf742563386435dc0775742be813eb99f6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"8a838583_8173cca1","updated":"2026-05-22 15:28:09.000000000","message":"I asked an LLM to analyse what types are been passed to write_pickle in diskfile.py:\n```\nThe complete set of value types across both calls is exactly str, float, bool, dict, HeaderKeyDict, and None — no others.\n\n  Summary of where each type comes from:\n\n  Call 1 (write_hashes → hashes.pkl):\n  - bool — \u0027valid\u0027 key\n  - float — \u0027updated\u0027 key (always time.time(), even if read_hashes returned the -1 int sentinel)\n  - None — invalidated suffix keys\n  - str — replication-policy suffix hash (md5 hexdigest)\n  - dict — EC-policy suffix hash ({int|None → str} mapping fragment index to md5 hexdigest)\n\n  Call 2 (pickle_async_update → async_pending pickle):\n  - str — \u0027op\u0027, \u0027account\u0027, \u0027container\u0027, \u0027obj\u0027, optional \u0027container_path\u0027\n  - HeaderKeyDict — \u0027headers\u0027 (all stored values are str due to __setitem__ coercion; int values like int(policy) are converted to str on insertion)\n  - str | None — \u0027db_state\u0027 (from headers_in.get(\u0027X-Container-Root-Db-State\u0027))\n  ```\n  \nand to analyse if that had changed over all swift releases:\n\n```\n---\n  Of the 159 tags, 34 have no write_pickle in diskfile.py and are skipped. Of the remaining 125 tags, 44 differ from the expected set {str, float, bool, dict,\n  HeaderKeyDict, None}:\n\n  ---\n  {str, None} — 5 tags (old-style hashes only; no pickle_async_update):\n  - 1.9.1, 1.9.2, 1.10.0, 1.10.0.rc1, havana-eol\n\n  {str, None, HeaderKeyDict} — 21 tags (pickle_async_update present with HeaderKeyDict, but no EC and no write_hashes so no bool/float/dict):\n  - 1.11.0, 1.12.0, 1.13.0, 1.13.1, 1.13.1.rc1, 1.13.1.rc2, 2.0.0, 2.0.0.rc1, 2.0.0.rc2, 2.1.0, 2.1.0.rc1, 2.2.0, 2.2.0.rc1, 2.2.1, 2.2.1.rc1, 2.2.1c1, 2.2.2, 2.2.2rc1,\n  icehouse-eol, juno-eol, storage-policy-historical\n\n  {str, None, HeaderKeyDict, dict} — 18 tags (EC added dict suffix-hash values, but write_hashes not yet present so still no bool/float):\n  - 2.3.0, 2.3.0rc1, 2.3.0rc2, 2.4.0, 2.5.0, 2.6.0, 2.7.0, 2.7.1, 2.8.0, 2.9.0, 2.10.0, 2.10.1, 2.11.0, 2.12.0, erasure_code_dev_history, kilo-eol, liberty-eol, mitaka-eol\n\n  ---\n  The three transitions that explain every change:\n  1. 1.11.0 — pickle_async_update added with HeaderKeyDict headers → HeaderKeyDict joins the set\n  2. 2.3.0 — EC policy added; ECDiskFileManager._hash_suffix returns dict → dict joins the set\n  3. 2.10.2 — write_hashes added; introduces hashes[\u0027valid\u0027] \u003d bool and hashes[\u0027updated\u0027] \u003d time.time() → bool and float join the set; full expected set reached for the\n  first time\n  ```\n  \n  Similarly for the db.py usage:\n  ```\n  \u003e for the make_tuple_for_pickle function in @swift/common/db.py . What types are passed to that function\u0027s record arg?\n  \n  ---\n  The record arg is always a dict, constructed just before the put_record call. There are two concrete implementations:\n\n  ---\n  ContainerBroker.make_tuple_for_pickle — called from put_object (container/backend.py:784)\n\n  The record is built in put_object and the values in it come from the container server (container/server.py:536):\n\n  ┌──────────────────────┬────────────┬─────────────────────────────────────────────────────────────┐\n  │        Field         │    Type    │                           Source                            │\n  ├──────────────────────┼────────────┼─────────────────────────────────────────────────────────────┤\n  │ name                 │ str        │ object name                                                 │\n  ├──────────────────────┼────────────┼─────────────────────────────────────────────────────────────┤\n  │ created_at           │ str        │ req_timestamp.internal                                      │\n  ├──────────────────────┼────────────┼─────────────────────────────────────────────────────────────┤\n  │ size                 │ int        │ int(req.headers[\u0027x-size\u0027])                                  │\n  ├──────────────────────┼────────────┼─────────────────────────────────────────────────────────────┤\n  │ content_type         │ str        │ wsgi_to_str(req.headers[\u0027x-content-type\u0027])                  │\n  ├──────────────────────┼────────────┼─────────────────────────────────────────────────────────────┤\n  │ etag                 │ str        │ wsgi_to_str(req.headers[\u0027x-etag\u0027])                          │\n  ├──────────────────────┼────────────┼─────────────────────────────────────────────────────────────┤\n  │ deleted              │ int        │ 0 or 1 literal                                              │\n  ├──────────────────────┼────────────┼─────────────────────────────────────────────────────────────┤\n  │ storage_policy_index │ int        │ obj_policy_index — either int(header) or 0                  │\n  ├──────────────────────┼────────────┼─────────────────────────────────────────────────────────────┤\n  │ ctype_timestamp      │ str | None │ wsgi_to_str(req.headers.get(...)) → None when header absent │\n  ├──────────────────────┼────────────┼─────────────────────────────────────────────────────────────┤\n  │ meta_timestamp       │ str | None │ same                                                        │\n  └──────────────────────┴────────────┴─────────────────────────────────────────────────────────────┘\n\n  Tuple value types: str, int, None\n\n  ---\n  AccountBroker.make_tuple_for_pickle — called from put_container (account/backend.py:245)\n\n  The record is built in put_container and the values come from the account server (account/server.py:183):\n\n  ┌──────────────────────┬───────────┬────────────────────────────────────────────────────────────────────────────────────────┐\n  │        Field         │   Type    │                                         Source                                         │\n  ├──────────────────────┼───────────┼────────────────────────────────────────────────────────────────────────────────────────┤\n  │ name                 │ str       │ container name                                                                         │\n  ├──────────────────────┼───────────┼────────────────────────────────────────────────────────────────────────────────────────┤\n  │ put_timestamp        │ str       │ req.headers[\u0027x-put-timestamp\u0027] — raw header string                                     │\n  ├──────────────────────┼───────────┼────────────────────────────────────────────────────────────────────────────────────────┤\n  │ delete_timestamp     │ str       │ req.headers[\u0027x-delete-timestamp\u0027] — raw header string                                  │\n  ├──────────────────────┼───────────┼────────────────────────────────────────────────────────────────────────────────────────┤\n  │ object_count         │ str       │ req.headers[\u0027x-object-count\u0027] — raw header string                                      │\n  ├──────────────────────┼───────────┼────────────────────────────────────────────────────────────────────────────────────────┤\n  │ bytes_used           │ str       │ req.headers[\u0027x-bytes-used\u0027] — raw header string                                        │\n  ├──────────────────────┼───────────┼────────────────────────────────────────────────────────────────────────────────────────┤\n  │ deleted              │ int       │ computed in put_container, 0 or 1                                                      │\n  ├──────────────────────┼───────────┼────────────────────────────────────────────────────────────────────────────────────────┤\n  │ storage_policy_index │ str | int │ req.headers.get(\u0027X-Backend-Storage-Policy-Index\u0027, 0) — header string, or default int 0 │\n  └──────────────────────┴───────────┴────────────────────────────────────────────────────────────────────────────────────────┘\n\n  Tuple value types: str, int\n\n  ---\n  Combined across both implementations: str, int, None\n\n  Two things worth noting that differ from the write_pickle analysis:\n  - The pickling here is of a tuple (not a dict) — make_tuple_for_pickle extracts specific fields in a fixed order. The record dict is just a staging structure.\n  - Unlike write_pickle\u0027s async-update path, there is no HeaderKeyDict, float, bool, or dict anywhere here. The object_count and bytes_used in the account case arrive as\n  raw header strings (not integers), since the account server does not call int() on them before passing to put_container.\n  \n  Tags 2.2.0 through 2.6.0 (plus erasure_code_dev_history, juno-eol, kilo-eol, liberty-eol) have type set {str, int} — None is absent because ctype_timestamp and\n  meta_timestamp did not yet exist in ContainerBroker.make_tuple_for_pickle. The tuple at those tags was:\n\n  (record[\u0027name\u0027], record[\u0027created_at\u0027], record[\u0027size\u0027],\n   record[\u0027content_type\u0027], record[\u0027etag\u0027], record[\u0027deleted\u0027],\n   record[\u0027storage_policy_index\u0027])\n\n  None joined the set at 2.7.0 when put_object gained the ctype_timestamp/meta_timestamp parameters (defaulting to None) and the tuple was extended to include them.\n\n  52 tags pre-date make_tuple_for_pickle entirely and were skipped. No tag ever introduced bool, float, dict, or any other unexpected type.\n  ```","commit_id":"89161efd633131ee3e5266e29203cb06d089fdb2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d938b3d856bff9c423a30409d18ba07a19ccbe3f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e3d4e5ab_d6b2c7c3","updated":"2026-05-26 23:37:13.000000000","message":"I should probably still go address some of those outstanding comments...","commit_id":"6258659a76f6d80a8c8be47603f0482746f0f4fb"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"24b50086e886cfa6939d49511ead14d744f12637","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"72effd68_11ee2c8d","updated":"2026-05-27 14:41:02.000000000","message":"recheck\n\nThe py3.10 test is fixed; recheck in case we want to merge this as it is.","commit_id":"6258659a76f6d80a8c8be47603f0482746f0f4fb"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"1fbabaeb93c643d79d733a91756d2d30a28eb520","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"f7c808b2_5e338d4a","updated":"2026-05-28 12:14:07.000000000","message":"...otherwise LGTM","commit_id":"bdf83f09271647df72cf218635c61dd1b5e0b131"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"138f2c8df2acb8ceb778e2f3402859e488573ccc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"04a1332b_6aac613d","updated":"2026-05-28 12:13:52.000000000","message":"I suggest not logging the potentially compromised async update file content, see: https://review.opendev.org/c/openstack/swift/+/990453\n\nor, at least truncating the content","commit_id":"bdf83f09271647df72cf218635c61dd1b5e0b131"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"16bb1f1d9ecac5fb17077151c288e2f3e2ef639d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"a3af3521_f89abca9","updated":"2026-05-28 05:36:19.000000000","message":"I think this is nice and clean, LGTM. Thx Tim!","commit_id":"bdf83f09271647df72cf218635c61dd1b5e0b131"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"73889eb44045412b85c91f39ea71ee2870ad5be4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"202ae213_bc595313","updated":"2026-05-28 02:13:17.000000000","message":"This is looking great and tests seem to be passing. Love the extra security. I did add a follow up because we weren\u0027t using the new unpickle in other tests (https://review.opendev.org/c/openstack/swift/+/990406). If use unpickle in tests then we\u0027ll hopefully get an early indication when new code breaks the hardening.\n\nAll the tests in the follow are passing, so using the unpickle in tests is working as expected, which is nice. So happy for either the follow up to be squashed or this landed and the follow up landed later.\n\nRunning swift-bench and I don\u0027t see issues. I\u0027ll run everything including the consistency engine under load for a while just to make sure.. but so far looking great!","commit_id":"bdf83f09271647df72cf218635c61dd1b5e0b131"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"0b229c80482ad24f72c952234b638c81ccc3c249","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"02865f22_9feb9f03","updated":"2026-05-29 12:23:23.000000000","message":"I think with the latest squashed in changes we\u0027re good on this one?","commit_id":"b7fdd9743bf6697114526f972a3cd1618bb0f542"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"d4f77b965389577baf4f41455237ea7821b9b750","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"d7aebd11_aea435c2","updated":"2026-05-28 19:24:14.000000000","message":"recheck\n\nFailure in `tempest.api.volume.admin.test_volume_manage.VolumeManageAdminTest.test_unmanage_manage_volume` -- nothing to do with us.","commit_id":"b7fdd9743bf6697114526f972a3cd1618bb0f542"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"60c3edee0d765cfa371bf0c328ee200676a69d29","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"ff4b9ae8_49620fa6","updated":"2026-05-29 19:34:44.000000000","message":"this is gREAT!\n\n```\nvagrant@vsaio:~$ python3 -c \u0027print(repr(open(\"vulnderable.pkl\", \"rb\").read()))\u0027\nb\u0027\\x80\\x04\\x957\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x8c\\x05posix\\x94\\x8c\\x06system\\x94\\x93\\x94\\x8c\\x1cecho Arbitrary_Code_Executed\\x94\\x85\\x94R\\x94.\u0027\nvagrant@vsaio:~$ strings vulnderable.pkl \nposix\nsystem\necho Arbitrary_Code_Executed\nvagrant@vsaio:~$ python3 -c \u0027import pickle; pickle.load(open(\"vulnderable.pkl\", \"rb\"))\u0027\nArbitrary_Code_Executed\nvagrant@vsaio:~$ python3 -c \u0027from swift.common.utils import pickle; pickle.unpickle(open(\"vulnderable.pkl\", \"rb\"))\u0027\n/vagrant/swift/swift/common/concurrency.py:22: EventletDeprecationWarning: \nEventlet is deprecated. It is currently being maintained in bugfix mode, and\nwe strongly recommend against using it for new projects.\n\nIf you are already using Eventlet, we recommend migrating to a different\nframework.  For more detail see\nhttps://eventlet.readthedocs.io/en/latest/asyncio/migration.html\n\n  import eventlet\nTraceback (most recent call last):\n  File \"/vagrant/swift/swift/common/utils/pickle.py\", line 66, in find_class\n    return _ALLOWED_GLOBALS[(module, name)]\n           ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^\nKeyError: (\u0027posix\u0027, \u0027system\u0027)\n\nDuring handling of the above exception, another exception occurred:\n\nTraceback (most recent call last):\n  File \"\u003cstring\u003e\", line 1, in \u003cmodule\u003e\n  File \"/vagrant/swift/swift/common/utils/pickle.py\", line 77, in unpickle\n    return RestrictedUnpickler(source, encoding\u003dencoding).load()\n           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n  File \"/vagrant/swift/swift/common/utils/pickle.py\", line 69, in find_class\n    raise pickle.UnpicklingError(\n_pickle.UnpicklingError: global \u0027posix.system\u0027 is forbidden\n```\n\n```\n• Findings\n\n  - P2: write_pickle was moved to instances/vsaio/swift/swift/common/utils/pickle.py:27, but it is no longer importable from swift.common.utils. That old import path existed before this patch and is likely used by out-of-tree Swift code or backend\n    integrations. I confirmed from swift.common.utils import write_pickle now raises ImportError. A small compatibility wrapper or re-export in swift/common/utils/__init__.py would avoid breaking existing callers.\n```\n\nTLDR: do we care about an import alias for write_pickle?\n\nSince most of this change is mostly `s/import pickle # insecure/import common.unpickle/` I\u0027m not sure I see any evidence it would be any easier to backport with an import alias for `write_pickle` *specifically*, but sure enough:\n\n```\ncgerrard@NVStation:~/Workspace/nv-swift-gizmos$ ag write_pickle\nbin/manually-redirect.py\n19:from swift.common.utils import find_namespace, write_pickle, Namespace, ContextPool, drop_privileges, renamer\n72:    write_pickle(update, update_path, tmpdir)\n```\n\nMy gut leans towards merging it WITHOUT the alias to sort of highlight:\n\n\u003e you should probably audit swift tools that were using pickle!\n\n```\ncgerrard@NVStation:~/Workspace/nv-swift-gizmos$ grep -R \"import pickle\" ~/Workspace/nv-swift-gizmos/ | wc -l\n6\n```","commit_id":"b7fdd9743bf6697114526f972a3cd1618bb0f542"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"72cb67c6e8596caef87a5646a42ef2cbcba2dc3e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"fb6c2a9b_07372c2c","in_reply_to":"ff4b9ae8_49620fa6","updated":"2026-05-29 23:12:54.000000000","message":"\u003e do we care about an import alias for write_pickle?\n\nTried it; led to circular import errors:\n\n```\nswift/common/utils/__init__.py:151: in \u003cmodule\u003e\n    from swift.common.utils.pickle import write_pickle  # noqa\nswift/common/utils/pickle.py:24: in \u003cmodule\u003e\n    from swift.common.utils import mkdirs, renamer\nE   ImportError: cannot import name \u0027mkdirs\u0027 from partially initialized module \u0027swift.common.utils\u0027 (most likely due to a circular import)\n```\n\nMaybe there\u0027s still space to argue that I shouldn\u0027t have moved it, but I think long-term I\u0027d prefer to move all of our pickle handling to the one module (and then hopefuly find a way to get rid of it over time).","commit_id":"b7fdd9743bf6697114526f972a3cd1618bb0f542"}],"swift/common/utils/__init__.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f2f1c0cf742563386435dc0775742be813eb99f6","unresolved":true,"context_lines":[{"line_number":1739,"context_line":"        return data"},{"line_number":1740,"context_line":""},{"line_number":1741,"context_line":""},{"line_number":1742,"context_line":"def write_pickle(obj, dest, tmp\u003dNone, pickle_protocol\u003d0):"},{"line_number":1743,"context_line":"    \"\"\""},{"line_number":1744,"context_line":"    Ensure that a pickle file gets written to disk.  The file"},{"line_number":1745,"context_line":"    is first written to a tmp location, ensure it is synced to disk, then"}],"source_content_type":"text/x-python","patch_set":4,"id":"6f5a3912_6f043552","side":"PARENT","line":1742,"updated":"2026-05-22 15:28:09.000000000","message":"previously when we have relocated functions from utils we have left an import in this file for out-of-tree compatibility e.g. timestamp, logs","commit_id":"ca8749f344168ad1c4e07fdfd24e68ca04c57eae"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"be64c4b374d05921a4edfbdc716852e4329177ee","unresolved":true,"context_lines":[{"line_number":1739,"context_line":"        return data"},{"line_number":1740,"context_line":""},{"line_number":1741,"context_line":""},{"line_number":1742,"context_line":"def write_pickle(obj, dest, tmp\u003dNone, pickle_protocol\u003d0):"},{"line_number":1743,"context_line":"    \"\"\""},{"line_number":1744,"context_line":"    Ensure that a pickle file gets written to disk.  The file"},{"line_number":1745,"context_line":"    is first written to a tmp location, ensure it is synced to disk, then"}],"source_content_type":"text/x-python","patch_set":4,"id":"b9967476_a5366427","side":"PARENT","line":1742,"in_reply_to":"6f5a3912_6f043552","updated":"2026-05-27 16:12:22.000000000","message":"Leads to circular import errors:\n```\nswift/common/utils/__init__.py:151: in \u003cmodule\u003e\n    from swift.common.utils.pickle import write_pickle  # noqa\nswift/common/utils/pickle.py:24: in \u003cmodule\u003e\n    from swift.common.utils import mkdirs, renamer\nE   ImportError: cannot import name \u0027mkdirs\u0027 from partially initialized module \u0027swift.common.utils\u0027 (most likely due to a circular import)\n```","commit_id":"ca8749f344168ad1c4e07fdfd24e68ca04c57eae"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"138f2c8df2acb8ceb778e2f3402859e488573ccc","unresolved":false,"context_lines":[{"line_number":1739,"context_line":"        return data"},{"line_number":1740,"context_line":""},{"line_number":1741,"context_line":""},{"line_number":1742,"context_line":"def write_pickle(obj, dest, tmp\u003dNone, pickle_protocol\u003d0):"},{"line_number":1743,"context_line":"    \"\"\""},{"line_number":1744,"context_line":"    Ensure that a pickle file gets written to disk.  The file"},{"line_number":1745,"context_line":"    is first written to a tmp location, ensure it is synced to disk, then"}],"source_content_type":"text/x-python","patch_set":4,"id":"467fac17_f325687f","side":"PARENT","line":1742,"in_reply_to":"b9967476_a5366427","updated":"2026-05-28 12:13:52.000000000","message":"Acknowledged","commit_id":"ca8749f344168ad1c4e07fdfd24e68ca04c57eae"}],"swift/common/utils/pickle.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0898ce5a1eb36e65a0a7cafe290acf1265248363","unresolved":true,"context_lines":[{"line_number":24,"context_line":"from swift.common.utils import mkdirs, renamer, timestamp"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def write_pickle(obj, dest, tmp\u003dNone, pickle_protocol\u003d0):"},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":"    Ensure that a pickle file gets written to disk.  The file"},{"line_number":30,"context_line":"    is first written to a tmp location, ensure it is synced to disk, then"}],"source_content_type":"text/x-python","patch_set":2,"id":"e3a932e4_e4b730f9","line":27,"updated":"2026-05-18 11:31:02.000000000","message":"I wonder how hard it would be to add some enforcement on the types that are allowed to be pickled so that we don\u0027t accidentally start picking a type that might change?","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f2f1c0cf742563386435dc0775742be813eb99f6","unresolved":false,"context_lines":[{"line_number":24,"context_line":"from swift.common.utils import mkdirs, renamer, timestamp"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def write_pickle(obj, dest, tmp\u003dNone, pickle_protocol\u003d0):"},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":"    Ensure that a pickle file gets written to disk.  The file"},{"line_number":30,"context_line":"    is first written to a tmp location, ensure it is synced to disk, then"}],"source_content_type":"text/x-python","patch_set":2,"id":"e8923559_1aacb5b6","line":27,"in_reply_to":"01bca0bc_101c06ed","updated":"2026-05-22 15:28:09.000000000","message":"Acknowledged","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e71313ffcc1871bc639dea320ead7b556a0d5100","unresolved":true,"context_lines":[{"line_number":24,"context_line":"from swift.common.utils import mkdirs, renamer, timestamp"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"def write_pickle(obj, dest, tmp\u003dNone, pickle_protocol\u003d0):"},{"line_number":28,"context_line":"    \"\"\""},{"line_number":29,"context_line":"    Ensure that a pickle file gets written to disk.  The file"},{"line_number":30,"context_line":"    is first written to a tmp location, ensure it is synced to disk, then"}],"source_content_type":"text/x-python","patch_set":2,"id":"01bca0bc_101c06ed","line":27,"in_reply_to":"e3a932e4_e4b730f9","updated":"2026-05-18 21:37:08.000000000","message":"Should be doable, but I think as long as we\u0027re good about using the new `unpickle` it should tend to be pretty obvious when we start pickling new types. My biggest concern is about what random stuff we may have written down ages ago, then only later realized we should use primitive types...","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"c3e2861888889441fb242998c3b2fb6b11c507b8","unresolved":true,"context_lines":[{"line_number":46,"context_line":"        renamer(tmppath, dest)"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"class RestrictedUnpickler(pickle.Unpickler):"},{"line_number":50,"context_line":"    def find_class(self, module, name):"},{"line_number":51,"context_line":"        if module \u003d\u003d \u0027_codecs\u0027 and name \u003d\u003d \u0027encode\u0027:"},{"line_number":52,"context_line":"            return _codecs.encode"}],"source_content_type":"text/x-python","patch_set":2,"id":"e584224e_238dbeb8","line":49,"updated":"2026-05-18 09:49:58.000000000","message":"I asked Claude to refactor this a bit for readability. WDYT?\n\n    _ALLOWED \u003d {\n        (\u0027_codecs\u0027, \u0027encode\u0027): _codecs.encode,\n        (\u0027copyreg\u0027, \u0027_reconstructor\u0027): copyreg._reconstructor,\n        (\u0027copy_reg\u0027, \u0027_reconstructor\u0027): copyreg._reconstructor,\n        (\u0027swift.common.header_key_dict\u0027, \u0027HeaderKeyDict\u0027):\n            header_key_dict.HeaderKeyDict,\n        (\u0027swift.common.utils\u0027, \u0027Timestamp\u0027): timestamp.Timestamp,\n        (\u0027swift.common.utils.timestamp\u0027, \u0027Timestamp\u0027): timestamp.Timestamp,\n        (\u0027builtins\u0027, \u0027dict\u0027): dict,\n        (\u0027__builtins__\u0027, \u0027dict\u0027): dict,\n        (\u0027__builtin__\u0027, \u0027dict\u0027): dict,\n        (\u0027builtins\u0027, \u0027bytes\u0027): bytes,\n        (\u0027__builtins__\u0027, \u0027bytes\u0027): bytesm\n        (\u0027__builtin__\u0027, \u0027bytes\u0027): bytes,\n    }\n \n    def find_class(self, module, name):\n        try:\n            return _ALLOWED[(module, name)]\n        except KeyError:\n            raise pickle.UnpicklingError(\n            f\"global \u0027{module}.{name}\u0027 is forbidden\")","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"be64c4b374d05921a4edfbdc716852e4329177ee","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        renamer(tmppath, dest)"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"class RestrictedUnpickler(pickle.Unpickler):"},{"line_number":50,"context_line":"    def find_class(self, module, name):"},{"line_number":51,"context_line":"        if module \u003d\u003d \u0027_codecs\u0027 and name \u003d\u003d \u0027encode\u0027:"},{"line_number":52,"context_line":"            return _codecs.encode"}],"source_content_type":"text/x-python","patch_set":2,"id":"b43f5f43_e6d8cecc","line":49,"in_reply_to":"24ef8dea_cdebb62d","updated":"2026-05-27 16:12:22.000000000","message":"Done","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e6f742cc20f1cbb0fd680fa62e4710aa028202d4","unresolved":true,"context_lines":[{"line_number":46,"context_line":"        renamer(tmppath, dest)"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"class RestrictedUnpickler(pickle.Unpickler):"},{"line_number":50,"context_line":"    def find_class(self, module, name):"},{"line_number":51,"context_line":"        if module \u003d\u003d \u0027_codecs\u0027 and name \u003d\u003d \u0027encode\u0027:"},{"line_number":52,"context_line":"            return _codecs.encode"}],"source_content_type":"text/x-python","patch_set":2,"id":"24ef8dea_cdebb62d","line":49,"in_reply_to":"90dbd8c3_6b6702df","updated":"2026-05-26 17:33:21.000000000","message":"Maybe `__builtins__` is unnecessary, though? Looking at that note on https://docs.python.org/2/library/__builtin__.html","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e71313ffcc1871bc639dea320ead7b556a0d5100","unresolved":true,"context_lines":[{"line_number":46,"context_line":"        renamer(tmppath, dest)"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"class RestrictedUnpickler(pickle.Unpickler):"},{"line_number":50,"context_line":"    def find_class(self, module, name):"},{"line_number":51,"context_line":"        if module \u003d\u003d \u0027_codecs\u0027 and name \u003d\u003d \u0027encode\u0027:"},{"line_number":52,"context_line":"            return _codecs.encode"}],"source_content_type":"text/x-python","patch_set":2,"id":"90dbd8c3_6b6702df","line":49,"in_reply_to":"e584224e_238dbeb8","updated":"2026-05-18 21:37:08.000000000","message":"Sure -- the `builtins`/`__builtins__`/`__builtin__` thing is still unfortunate, sorry.","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"c3e2861888889441fb242998c3b2fb6b11c507b8","unresolved":true,"context_lines":[{"line_number":51,"context_line":"        if module \u003d\u003d \u0027_codecs\u0027 and name \u003d\u003d \u0027encode\u0027:"},{"line_number":52,"context_line":"            return _codecs.encode"},{"line_number":53,"context_line":"        if module in (\u0027copy_reg\u0027, \u0027copyreg\u0027) and name \u003d\u003d \u0027_reconstructor\u0027:"},{"line_number":54,"context_line":"            return copyreg._reconstructor  # Needed for HeaderKeyDict??"},{"line_number":55,"context_line":"        if module \u003d\u003d \u0027swift.common.header_key_dict\u0027 \\"},{"line_number":56,"context_line":"                and name \u003d\u003d \u0027HeaderKeyDict\u0027:"},{"line_number":57,"context_line":"            return header_key_dict.HeaderKeyDict"}],"source_content_type":"text/x-python","patch_set":2,"id":"2d5c2722_453a812c","line":54,"updated":"2026-05-18 09:49:58.000000000","message":"I think this is needed for pending asyncs?","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e71313ffcc1871bc639dea320ead7b556a0d5100","unresolved":true,"context_lines":[{"line_number":51,"context_line":"        if module \u003d\u003d \u0027_codecs\u0027 and name \u003d\u003d \u0027encode\u0027:"},{"line_number":52,"context_line":"            return _codecs.encode"},{"line_number":53,"context_line":"        if module in (\u0027copy_reg\u0027, \u0027copyreg\u0027) and name \u003d\u003d \u0027_reconstructor\u0027:"},{"line_number":54,"context_line":"            return copyreg._reconstructor  # Needed for HeaderKeyDict??"},{"line_number":55,"context_line":"        if module \u003d\u003d \u0027swift.common.header_key_dict\u0027 \\"},{"line_number":56,"context_line":"                and name \u003d\u003d \u0027HeaderKeyDict\u0027:"},{"line_number":57,"context_line":"            return header_key_dict.HeaderKeyDict"}],"source_content_type":"text/x-python","patch_set":2,"id":"7b7db924_67c54636","line":54,"in_reply_to":"2d5c2722_453a812c","updated":"2026-05-18 21:37:08.000000000","message":"It\u0027s for whatever brings in `HeaderKeyDict` -- as Alistair\u0027s investigation into `Timestamp` shows, it might be worth double-checking that we really need it.","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"8ae6e92b7da2d9ec0c285741f3da196ea82ab87e","unresolved":true,"context_lines":[{"line_number":51,"context_line":"        if module \u003d\u003d \u0027_codecs\u0027 and name \u003d\u003d \u0027encode\u0027:"},{"line_number":52,"context_line":"            return _codecs.encode"},{"line_number":53,"context_line":"        if module in (\u0027copy_reg\u0027, \u0027copyreg\u0027) and name \u003d\u003d \u0027_reconstructor\u0027:"},{"line_number":54,"context_line":"            return copyreg._reconstructor  # Needed for HeaderKeyDict??"},{"line_number":55,"context_line":"        if module \u003d\u003d \u0027swift.common.header_key_dict\u0027 \\"},{"line_number":56,"context_line":"                and name \u003d\u003d \u0027HeaderKeyDict\u0027:"},{"line_number":57,"context_line":"            return header_key_dict.HeaderKeyDict"}],"source_content_type":"text/x-python","patch_set":2,"id":"9720ccb5_6d755953","line":54,"in_reply_to":"7b7db924_67c54636","updated":"2026-05-19 16:02:49.000000000","message":"The following unittests are failing if I remove this:\n\n    obj/test_server.py::TestObjectController::test_PUT_redirected_async_pending\n    obj/test_server.py::TestObjectController::test_PUT_redirected_async_pending_with_container_path \n    obj/test_server.py::TestObjectController::test_PUT_redirected_async_pending_with_old_style_container_path \n    obj/test_server.py::TestObjectController::test_PUT_then_POST_async_pendings_with_EC_policy \n    obj/test_server.py::TestObjectController::test_PUT_then_POST_async_pendings_with_repl_policy","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"0898ce5a1eb36e65a0a7cafe290acf1265248363","unresolved":true,"context_lines":[{"line_number":58,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":59,"context_line":"                and name \u003d\u003d \u0027dict\u0027:"},{"line_number":60,"context_line":"            return dict"},{"line_number":61,"context_line":"        if module in (\u0027swift.common.utils\u0027, \u0027swift.common.utils.timestamp\u0027) \\"},{"line_number":62,"context_line":"                and name \u003d\u003d \u0027Timestamp\u0027:"},{"line_number":63,"context_line":"            return timestamp.Timestamp"},{"line_number":64,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":65,"context_line":"                and name \u003d\u003d \u0027bytes\u0027:"},{"line_number":66,"context_line":"            return bytes"}],"source_content_type":"text/x-python","patch_set":2,"id":"6db76607_f45cf884","line":63,"range":{"start_line":61,"start_character":8,"end_line":63,"end_character":38},"updated":"2026-05-18 11:31:02.000000000","message":"AFAICT this is not needed (thank goodness!)\n\nI hope that what you ran into were three failing tests in test_sharder.py that were incorrectly passing Timestamp instances to ``put_object()``. See https://review.opendev.org/c/openstack/swift/+/988949 for a fix for those tests.","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f2f1c0cf742563386435dc0775742be813eb99f6","unresolved":false,"context_lines":[{"line_number":58,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":59,"context_line":"                and name \u003d\u003d \u0027dict\u0027:"},{"line_number":60,"context_line":"            return dict"},{"line_number":61,"context_line":"        if module in (\u0027swift.common.utils\u0027, \u0027swift.common.utils.timestamp\u0027) \\"},{"line_number":62,"context_line":"                and name \u003d\u003d \u0027Timestamp\u0027:"},{"line_number":63,"context_line":"            return timestamp.Timestamp"},{"line_number":64,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":65,"context_line":"                and name \u003d\u003d \u0027bytes\u0027:"},{"line_number":66,"context_line":"            return bytes"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ec83498_35f6bbe5","line":63,"range":{"start_line":61,"start_character":8,"end_line":63,"end_character":38},"in_reply_to":"31482ed8_c13dafea","updated":"2026-05-22 15:28:09.000000000","message":"Done","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e71313ffcc1871bc639dea320ead7b556a0d5100","unresolved":true,"context_lines":[{"line_number":58,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":59,"context_line":"                and name \u003d\u003d \u0027dict\u0027:"},{"line_number":60,"context_line":"            return dict"},{"line_number":61,"context_line":"        if module in (\u0027swift.common.utils\u0027, \u0027swift.common.utils.timestamp\u0027) \\"},{"line_number":62,"context_line":"                and name \u003d\u003d \u0027Timestamp\u0027:"},{"line_number":63,"context_line":"            return timestamp.Timestamp"},{"line_number":64,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":65,"context_line":"                and name \u003d\u003d \u0027bytes\u0027:"},{"line_number":66,"context_line":"            return bytes"}],"source_content_type":"text/x-python","patch_set":2,"id":"d0762453_96d3ad67","line":63,"range":{"start_line":61,"start_character":8,"end_line":63,"end_character":38},"in_reply_to":"6db76607_f45cf884","updated":"2026-05-18 21:37:08.000000000","message":"Thanks! We should do a similar investigation for `HeaderKeyDict` -- I\u0027d *love* to drop that (and the `copyreg` stuff) if we can get away with it.","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"d2974ce065b96d0c00b24d798564f378b2611985","unresolved":true,"context_lines":[{"line_number":58,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":59,"context_line":"                and name \u003d\u003d \u0027dict\u0027:"},{"line_number":60,"context_line":"            return dict"},{"line_number":61,"context_line":"        if module in (\u0027swift.common.utils\u0027, \u0027swift.common.utils.timestamp\u0027) \\"},{"line_number":62,"context_line":"                and name \u003d\u003d \u0027Timestamp\u0027:"},{"line_number":63,"context_line":"            return timestamp.Timestamp"},{"line_number":64,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":65,"context_line":"                and name \u003d\u003d \u0027bytes\u0027:"},{"line_number":66,"context_line":"            return bytes"}],"source_content_type":"text/x-python","patch_set":2,"id":"31482ed8_c13dafea","line":63,"range":{"start_line":61,"start_character":8,"end_line":63,"end_character":38},"in_reply_to":"d0762453_96d3ad67","updated":"2026-05-19 16:18:19.000000000","message":"Just rebased this patch on top of Alistairs patch, and these failing tests are now passing (after removing the three highlighted lines for Timestamp). \n\nSo I think we can get rid of Timestamp and keep HeaderKeyDict.","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":6968,"name":"Christian Schwede","email":"cschwede@nvidia.com","username":"cschwede"},"change_message_id":"c3e2861888889441fb242998c3b2fb6b11c507b8","unresolved":true,"context_lines":[{"line_number":64,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":65,"context_line":"                and name \u003d\u003d \u0027bytes\u0027:"},{"line_number":66,"context_line":"            return bytes"},{"line_number":67,"context_line":"        # Forbid any other loading"},{"line_number":68,"context_line":"        raise pickle.UnpicklingError(f\"global \u0027{module}.{name}\u0027 is forbidden\")"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"7fdc7658_be6ad897","line":67,"updated":"2026-05-18 09:49:58.000000000","message":"What do you think about logging at this point to leave a trace in the logs? Hopefully no one ever sees them in production, but if so it would be a strong signal to investigate further?","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"be64c4b374d05921a4edfbdc716852e4329177ee","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":65,"context_line":"                and name \u003d\u003d \u0027bytes\u0027:"},{"line_number":66,"context_line":"            return bytes"},{"line_number":67,"context_line":"        # Forbid any other loading"},{"line_number":68,"context_line":"        raise pickle.UnpicklingError(f\"global \u0027{module}.{name}\u0027 is forbidden\")"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"14a2fe2e_dc91d9a7","line":67,"in_reply_to":"46e27e75_c3c46fdc","updated":"2026-05-27 16:12:22.000000000","message":"Done","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e71313ffcc1871bc639dea320ead7b556a0d5100","unresolved":true,"context_lines":[{"line_number":64,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":65,"context_line":"                and name \u003d\u003d \u0027bytes\u0027:"},{"line_number":66,"context_line":"            return bytes"},{"line_number":67,"context_line":"        # Forbid any other loading"},{"line_number":68,"context_line":"        raise pickle.UnpicklingError(f\"global \u0027{module}.{name}\u0027 is forbidden\")"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"46e27e75_c3c46fdc","line":67,"in_reply_to":"7fdc7658_be6ad897","updated":"2026-05-18 21:37:08.000000000","message":"Should get some logging about it already, though -- see https://github.com/openstack/swift/blob/2.37.1/swift/common/db.py#L840-L843 for example. Might be good to log the actual pickled data around https://github.com/openstack/swift/blob/2.37.1/swift/obj/updater.py#L654-L655","commit_id":"667aed0d7bc5eda0ba83ca145b6ec3f010166df1"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"f2f1c0cf742563386435dc0775742be813eb99f6","unresolved":true,"context_lines":[{"line_number":60,"context_line":"            return dict"},{"line_number":61,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":62,"context_line":"                and name \u003d\u003d \u0027bytes\u0027:"},{"line_number":63,"context_line":"            return bytes"},{"line_number":64,"context_line":"        # Forbid any other loading"},{"line_number":65,"context_line":"        raise pickle.UnpicklingError(f\"global \u0027{module}.{name}\u0027 is forbidden\")"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"f2e22684_9558ede6","line":63,"updated":"2026-05-22 15:28:09.000000000","message":"I\u0027m unsure why bytes must be explicitly allowed but not str, int, float","commit_id":"89161efd633131ee3e5266e29203cb06d089fdb2"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"e6f742cc20f1cbb0fd680fa62e4710aa028202d4","unresolved":true,"context_lines":[{"line_number":60,"context_line":"            return dict"},{"line_number":61,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":62,"context_line":"                and name \u003d\u003d \u0027bytes\u0027:"},{"line_number":63,"context_line":"            return bytes"},{"line_number":64,"context_line":"        # Forbid any other loading"},{"line_number":65,"context_line":"        raise pickle.UnpicklingError(f\"global \u0027{module}.{name}\u0027 is forbidden\")"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"fe736366_a0abc8f0","line":63,"in_reply_to":"f2e22684_9558ede6","updated":"2026-05-26 17:33:21.000000000","message":"Seems to do with pickling empty bytes with `protocol\u003d2`:\n```\n\u003e\u003e\u003e pickletools.dis(pickle.dumps(b\u0027x\u0027, protocol\u003d2))\n    0: \\x80 PROTO      2\n    2: c    GLOBAL     \u0027_codecs encode\u0027\n   18: q    BINPUT     0\n   20: X    BINUNICODE \u0027x\u0027\n   26: q    BINPUT     1\n   28: X    BINUNICODE \u0027latin1\u0027\n   39: q    BINPUT     2\n   41: \\x86 TUPLE2\n   42: q    BINPUT     3\n   44: R    REDUCE\n   45: q    BINPUT     4\n   47: .    STOP\nhighest protocol among opcodes \u003d 2\n\u003e\u003e\u003e pickletools.dis(pickle.dumps(b\u0027\u0027, protocol\u003d2))\n    0: \\x80 PROTO      2\n    2: c    GLOBAL     \u0027__builtin__ bytes\u0027\n   21: q    BINPUT     0\n   23: )    EMPTY_TUPLE\n   24: R    REDUCE\n   25: q    BINPUT     1\n   27: .    STOP\nhighest protocol among opcodes \u003d 2\n```\nIf we\u0027d been using `protocol\u003d3` the protocol would have a primitive for it:\n```\n\u003e\u003e\u003e pickletools.dis(pickle.dumps(b\u0027x\u0027, protocol\u003d3))\n    0: \\x80 PROTO      3\n    2: C    SHORT_BINBYTES b\u0027x\u0027\n    5: q    BINPUT     0\n    7: .    STOP\nhighest protocol among opcodes \u003d 3\n\u003e\u003e\u003e pickletools.dis(pickle.dumps(b\u0027\u0027, protocol\u003d3))\n    0: \\x80 PROTO      3\n    2: C    SHORT_BINBYTES b\u0027\u0027\n    4: q    BINPUT     0\n    6: .    STOP\nhighest protocol among opcodes \u003d 3\n```","commit_id":"89161efd633131ee3e5266e29203cb06d089fdb2"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"138f2c8df2acb8ceb778e2f3402859e488573ccc","unresolved":false,"context_lines":[{"line_number":60,"context_line":"            return dict"},{"line_number":61,"context_line":"        if module in (\u0027builtins\u0027, \u0027__builtins__\u0027, \u0027__builtin__\u0027) \\"},{"line_number":62,"context_line":"                and name \u003d\u003d \u0027bytes\u0027:"},{"line_number":63,"context_line":"            return bytes"},{"line_number":64,"context_line":"        # Forbid any other loading"},{"line_number":65,"context_line":"        raise pickle.UnpicklingError(f\"global \u0027{module}.{name}\u0027 is forbidden\")"},{"line_number":66,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ca06e699_a398ad5e","line":63,"in_reply_to":"fe736366_a0abc8f0","updated":"2026-05-28 12:13:52.000000000","message":"Acknowledged","commit_id":"89161efd633131ee3e5266e29203cb06d089fdb2"}],"swift/obj/updater.py":[{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"138f2c8df2acb8ceb778e2f3402859e488573ccc","unresolved":true,"context_lines":[{"line_number":659,"context_line":"                    \u0027ERROR Pickle problem, quarantining %s\u0027, update_path)"},{"line_number":660,"context_line":"            else:"},{"line_number":661,"context_line":"                self.logger.exception("},{"line_number":662,"context_line":"                    \u0027ERROR Pickle problem loading %r, quarantining %s\u0027,"},{"line_number":663,"context_line":"                    pickle_data, update_path)"},{"line_number":664,"context_line":"            self.stats.quarantines +\u003d 1"},{"line_number":665,"context_line":"            self.logger.increment(\u0027quarantines\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"cb793289_689f0a5e","line":662,"range":{"start_line":662,"start_character":50,"end_line":662,"end_character":52},"updated":"2026-05-28 12:13:52.000000000","message":"I\u0027m not sure we want to log the content of a potentially compromised pickle file of unknown size. It will be available for inspection in the quarantined files.","commit_id":"bdf83f09271647df72cf218635c61dd1b5e0b131"},{"author":{"_account_id":15343,"name":"Tim Burke","email":"tburke@nvidia.com","username":"tburke"},"change_message_id":"8c456194cee91e8a11e5defac542db4865fa2bfc","unresolved":true,"context_lines":[{"line_number":659,"context_line":"                    \u0027ERROR Pickle problem, quarantining %s\u0027, update_path)"},{"line_number":660,"context_line":"            else:"},{"line_number":661,"context_line":"                self.logger.exception("},{"line_number":662,"context_line":"                    \u0027ERROR Pickle problem loading %r, quarantining %s\u0027,"},{"line_number":663,"context_line":"                    pickle_data, update_path)"},{"line_number":664,"context_line":"            self.stats.quarantines +\u003d 1"},{"line_number":665,"context_line":"            self.logger.increment(\u0027quarantines\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3aed5b7e_e4bd3f60","line":662,"range":{"start_line":662,"start_character":50,"end_line":662,"end_character":52},"in_reply_to":"cb793289_689f0a5e","updated":"2026-05-28 15:26:29.000000000","message":"FWIW, there\u0027s precedent in https://github.com/openstack/swift/blob/master/swift/common/db.py#L840-L843 (though there, it\u0027s our last chance to see the data; after failing to load, we drop it on the floor).\n\nI did this more out of concern to the possibility of clusters of indeterminate age having some sort of weird asyncs lying around. We still haven\u0027t redrawn our security boundaries -- we don\u0027t do any service-to-service authentication -- I think it\u0027s still reasonable to assume that the pickles are *not* compromised and treat as a reasonable concern that this change may cause us to fail to load legitimate pickles. If that happens, I kinda want all the help I can get.\n\nBut I could be swayed otherwise. I\u0027ll do a straw poll at today\u0027s stand-up.","commit_id":"bdf83f09271647df72cf218635c61dd1b5e0b131"}]}