Skip to content

fix: extend digest verification to sha512 and set sandbox paths#850

Open
ericcurtin wants to merge 1 commit intomainfrom
address-comments
Open

fix: extend digest verification to sha512 and set sandbox paths#850
ericcurtin wants to merge 1 commit intomainfrom
address-comments

Conversation

@ericcurtin
Copy link
Copy Markdown
Contributor

Extend blob digest verification to cover sha512 in addition to sha256, closing a bypass where sha512-addressed blobs were stored without any integrity check. Set correct SandboxPath for Python backends (diffusers, mlx, vllm-metal) so the Darwin sandbox profile resolves UPDATEDLIBPATH to the sibling lib/ directory of the Python bin/ directory.

Extend blob digest verification to cover sha512 in addition to sha256,
closing a bypass where sha512-addressed blobs were stored without any
integrity check. Set correct SandboxPath for Python backends (diffusers,
mlx, vllm-metal) so the Darwin sandbox profile resolves UPDATEDLIBPATH
to the sibling lib/ directory of the Python bin/ directory.
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In WriteBlobWithResume, when diffID.Algorithm is neither sha256 nor sha512 the blob is now written without any integrity check; consider explicitly rejecting unsupported algorithms (or at least logging) instead of silently skipping verification.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `WriteBlobWithResume`, when `diffID.Algorithm` is neither `sha256` nor `sha512` the blob is now written without any integrity check; consider explicitly rejecting unsupported algorithms (or at least logging) instead of silently skipping verification.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for SHA-512 hashing in the blob store and updates the sandbox path configuration for several inference backends (Diffusers, MLX, and vLLM Metal) to ensure correct directory scoping. I have no feedback to provide.

ilopezluna
ilopezluna previously approved these changes Apr 8, 2026
@ilopezluna ilopezluna self-requested a review April 8, 2026 14:06
@ilopezluna
Copy link
Copy Markdown
Contributor

ilopezluna commented Apr 8, 2026

ilopezluna@F2D5QD4D6C model-runner % docker model uninstall-runner --backend vllm     
Uninstalled vllm backend
ilopezluna@F2D5QD4D6C model-runner % docker model install-runner --backend vllm     
Installing vllm backend...
vllm backend installed successfully
ilopezluna@F2D5QD4D6C model-runner % docker model run smollm2-vllm                    
> hello
background model preload failed: preload failed: status=500 body=unable to load runner: error waiting for runner to be ready: vllm-metal terminated unexpectedly: vllm-metal failed: (APIServer pid=92759)     self.input_socket = self.resources.input_socket = make_zmq_socket(
(APIServer pid=92759)                                                       ^^^^^^^^^^^^^^^^
(APIServer pid=92759)   File "/Users/ilopezluna/.docker/model-runner/vllm-metal/lib/python3.12/site-packages/vllm/utils/network_utils.py", line 309, in make_zmq_socket
(APIServer pid=92759)     socket.bind(path)
(APIServer pid=92759)   File "/Users/ilopezluna/.docker/model-runner/vllm-metal/lib/python3.12/site-packages/zmq/sugar/socket.py", line 320, in bind
(APIServer pid=92759)     super().bind(addr)
(APIServer pid=92759)   File "zmq/backend/cython/_zmq.py", line 1009, in zmq.backend.cython._zmq.Socket.bind
(APIServer pid=92759)   File "zmq/backend/cython/_zmq.py", line 190, in zmq.backend.cython._zmq._check_rc
(APIServer pid=92759) zmq.error.ZMQError: Operation not permitted (addr='ipc:///var/folders/xy/2ylpqbs55c7233trwq3lpscm0000gp/T/eca66bed-bc55-41af-9950-e93de87ab5e0')
/v1/engine/core_client.py", line 562, in __init__


Failed to generate a response: error response: status=500 body=unable to load runner: error waiting for runner to be ready: vllm-metal terminated unexpectedly: vllm-metal failed: (APIServer pid=96473)     self.input_socket = self.resources.input_socket = make_zmq_socket(
(APIServer pid=96473)                                                       ^^^^^^^^^^^^^^^^
(APIServer pid=96473)   File "/Users/ilopezluna/.docker/model-runner/vllm-metal/lib/python3.12/site-packages/vllm/utils/network_utils.py", line 309, in make_zmq_socket
(APIServer pid=96473)     socket.bind(path)
(APIServer pid=96473)   File "/Users/ilopezluna/.docker/model-runner/vllm-metal/lib/python3.12/site-packages/zmq/sugar/socket.py", line 320, in bind
(APIServer pid=96473)     super().bind(addr)
(APIServer pid=96473)   File "zmq/backend/cython/_zmq.py", line 1009, in zmq.backend.cython._zmq.Socket.bind
(APIServer pid=96473)   File "zmq/backend/cython/_zmq.py", line 190, in zmq.backend.cython._zmq._check_rc
(APIServer pid=96473) zmq.error.ZMQError: Operation not permitted (addr='ipc:///var/folders/xy/2ylpqbs55c7233trwq3lpscm0000gp/T/53f30cd8-f4af-4372-812a-2ae34c103c08')
/v1/engine/core_client.py", line 562, in __init__

I get this error in vllm-metal

@ilopezluna ilopezluna dismissed their stale review April 8, 2026 17:10

I have realized it fails when using vllm-metal

var hasher hash.Hash
switch diffID.Algorithm {
case "sha256":
hasher = sha256.New()
Copy link
Copy Markdown
Contributor

@sathiraumesh sathiraumesh Apr 9, 2026

Choose a reason for hiding this comment

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

Just a suggestion, It may be better to move this to a separate function like

hasher(string algo) (has.Hash){
...
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants