fix(image-gen): guard local provider inputs against credential reads - #57698
fix(image-gen): guard local provider inputs against credential reads#57698necoweb3 wants to merge 1 commit into
Conversation
Related to #57695 (same credential-read guard applied to the xAI image/video providers). This PR extends the same |
|
looks mergeable The local image input paths for OpenAI and OpenRouter now call Signed: GPT-5.5-xhigh in Codex |
|
Superseded by #57726, which salvages this fix (your commit cherry-picked, authorship preserved) and extends it:
Verified: 225 tests pass, mutation-checked, and E2E against a real profile-scoped Thanks for the fix and the clean diagnosis — credit preserved in #57726's history. Closing as superseded. |
… chokepoint + cover xai (#57698) Follow-up to the per-provider guards. Three improvements from review: 1. Extract agent.file_safety.raise_if_read_blocked() as a single shared chokepoint and route the OpenAI, OpenRouter, and (newly) xAI image providers through it, replacing the 3x-duplicated inline try/except. Fixes the whole bug class: xai/_xai_image_field read a model-supplied local path via open() with no guard — the same vulnerability the PR fixed for OpenAI/OpenRouter, in a sibling provider it missed. 2. Strengthen the regression tests from pass-on-any-ValueError to true security invariants: spy open()/read_bytes() and assert the blocked credential is NEVER read; add negative controls (legit local image still loads; remote/data: URIs pass through unguarded) so a block-everything regression can't pass. 3. Guard is best-effort by design (defense-in-depth, not a security boundary) — documented on the shared helper. - agent/file_safety.py: raise_if_read_blocked() - plugins/image_gen/{openai,openrouter,xai}: route through helper - tests: no-read spies + negative controls across all three providers
|
Merged via #57726 — your commit is cherry-picked with authorship preserved ( Thanks for catching this credential-read gap. On top of your openai + openrouter fix, the salvage extended it to the whole bug class: covered the xAI image provider (which had the identical unguarded |
…al-read guard video_analyze_tool's local-path branch read raw bytes via _detect_video_mime_type (extension-only, no magic-byte check) with no call to agent.file_safety.raise_if_read_blocked, unlike the image-gen and video-gen provider plugins that already route local inputs through that shared chokepoint (#57698). A model could point video_url at a credential store (e.g. .env, auth.json) renamed or symlinked to a video-like extension and have its raw bytes base64-encoded and sent to the vision provider. vision_analyze_tool and its native fast path (_vision_analyze_native) had the same gap in their local-file branches; they were only incidentally protected by the image magic-byte sniff rejecting non-image content, not by the intended read guard. Add raise_if_read_blocked() to all three local-file branches, mirroring the existing plugins/image_gen and plugins/video_gen call sites.
… chokepoint + cover xai (NousResearch#57698) Follow-up to the per-provider guards. Three improvements from review: 1. Extract agent.file_safety.raise_if_read_blocked() as a single shared chokepoint and route the OpenAI, OpenRouter, and (newly) xAI image providers through it, replacing the 3x-duplicated inline try/except. Fixes the whole bug class: xai/_xai_image_field read a model-supplied local path via open() with no guard — the same vulnerability the PR fixed for OpenAI/OpenRouter, in a sibling provider it missed. 2. Strengthen the regression tests from pass-on-any-ValueError to true security invariants: spy open()/read_bytes() and assert the blocked credential is NEVER read; add negative controls (legit local image still loads; remote/data: URIs pass through unguarded) so a block-everything regression can't pass. 3. Guard is best-effort by design (defense-in-depth, not a security boundary) — documented on the shared helper. - agent/file_safety.py: raise_if_read_blocked() - plugins/image_gen/{openai,openrouter,xai}: route through helper - tests: no-read spies + negative controls across all three providers
…al-read guard video_analyze_tool's local-path branch read raw bytes via _detect_video_mime_type (extension-only, no magic-byte check) with no call to agent.file_safety.raise_if_read_blocked, unlike the image-gen and video-gen provider plugins that already route local inputs through that shared chokepoint (NousResearch#57698). A model could point video_url at a credential store (e.g. .env, auth.json) renamed or symlinked to a video-like extension and have its raw bytes base64-encoded and sent to the vision provider. vision_analyze_tool and its native fast path (_vision_analyze_native) had the same gap in their local-file branches; they were only incidentally protected by the image magic-byte sniff rejecting non-image content, not by the intended read guard. Add raise_if_read_blocked() to all three local-file branches, mirroring the existing plugins/image_gen and plugins/video_gen call sites.
… chokepoint + cover xai (NousResearch#57698) Follow-up to the per-provider guards. Three improvements from review: 1. Extract agent.file_safety.raise_if_read_blocked() as a single shared chokepoint and route the OpenAI, OpenRouter, and (newly) xAI image providers through it, replacing the 3x-duplicated inline try/except. Fixes the whole bug class: xai/_xai_image_field read a model-supplied local path via open() with no guard — the same vulnerability the PR fixed for OpenAI/OpenRouter, in a sibling provider it missed. 2. Strengthen the regression tests from pass-on-any-ValueError to true security invariants: spy open()/read_bytes() and assert the blocked credential is NEVER read; add negative controls (legit local image still loads; remote/data: URIs pass through unguarded) so a block-everything regression can't pass. 3. Guard is best-effort by design (defense-in-depth, not a security boundary) — documented on the shared helper. - agent/file_safety.py: raise_if_read_blocked() - plugins/image_gen/{openai,openrouter,xai}: route through helper - tests: no-read spies + negative controls across all three providers
…al-read guard video_analyze_tool's local-path branch read raw bytes via _detect_video_mime_type (extension-only, no magic-byte check) with no call to agent.file_safety.raise_if_read_blocked, unlike the image-gen and video-gen provider plugins that already route local inputs through that shared chokepoint (NousResearch#57698). A model could point video_url at a credential store (e.g. .env, auth.json) renamed or symlinked to a video-like extension and have its raw bytes base64-encoded and sent to the vision provider. vision_analyze_tool and its native fast path (_vision_analyze_native) had the same gap in their local-file branches; they were only incidentally protected by the image magic-byte sniff rejecting non-image content, not by the intended read guard. Add raise_if_read_blocked() to all three local-file branches, mirroring the existing plugins/image_gen and plugins/video_gen call sites.
… chokepoint + cover xai (NousResearch#57698) Follow-up to the per-provider guards. Three improvements from review: 1. Extract agent.file_safety.raise_if_read_blocked() as a single shared chokepoint and route the OpenAI, OpenRouter, and (newly) xAI image providers through it, replacing the 3x-duplicated inline try/except. Fixes the whole bug class: xai/_xai_image_field read a model-supplied local path via open() with no guard — the same vulnerability the PR fixed for OpenAI/OpenRouter, in a sibling provider it missed. 2. Strengthen the regression tests from pass-on-any-ValueError to true security invariants: spy open()/read_bytes() and assert the blocked credential is NEVER read; add negative controls (legit local image still loads; remote/data: URIs pass through unguarded) so a block-everything regression can't pass. 3. Guard is best-effort by design (defense-in-depth, not a security boundary) — documented on the shared helper. - agent/file_safety.py: raise_if_read_blocked() - plugins/image_gen/{openai,openrouter,xai}: route through helper - tests: no-read spies + negative controls across all three providers
…al-read guard video_analyze_tool's local-path branch read raw bytes via _detect_video_mime_type (extension-only, no magic-byte check) with no call to agent.file_safety.raise_if_read_blocked, unlike the image-gen and video-gen provider plugins that already route local inputs through that shared chokepoint (NousResearch#57698). A model could point video_url at a credential store (e.g. .env, auth.json) renamed or symlinked to a video-like extension and have its raw bytes base64-encoded and sent to the vision provider. vision_analyze_tool and its native fast path (_vision_analyze_native) had the same gap in their local-file branches; they were only incidentally protected by the image magic-byte sniff rejecting non-image content, not by the intended read guard. Add raise_if_read_blocked() to all three local-file branches, mirroring the existing plugins/image_gen and plugins/video_gen call sites.
… chokepoint + cover xai (NousResearch#57698) Follow-up to the per-provider guards. Three improvements from review: 1. Extract agent.file_safety.raise_if_read_blocked() as a single shared chokepoint and route the OpenAI, OpenRouter, and (newly) xAI image providers through it, replacing the 3x-duplicated inline try/except. Fixes the whole bug class: xai/_xai_image_field read a model-supplied local path via open() with no guard — the same vulnerability the PR fixed for OpenAI/OpenRouter, in a sibling provider it missed. 2. Strengthen the regression tests from pass-on-any-ValueError to true security invariants: spy open()/read_bytes() and assert the blocked credential is NEVER read; add negative controls (legit local image still loads; remote/data: URIs pass through unguarded) so a block-everything regression can't pass. 3. Guard is best-effort by design (defense-in-depth, not a security boundary) — documented on the shared helper. - agent/file_safety.py: raise_if_read_blocked() - plugins/image_gen/{openai,openrouter,xai}: route through helper - tests: no-read spies + negative controls across all three providers
…al-read guard video_analyze_tool's local-path branch read raw bytes via _detect_video_mime_type (extension-only, no magic-byte check) with no call to agent.file_safety.raise_if_read_blocked, unlike the image-gen and video-gen provider plugins that already route local inputs through that shared chokepoint (NousResearch#57698). A model could point video_url at a credential store (e.g. .env, auth.json) renamed or symlinked to a video-like extension and have its raw bytes base64-encoded and sent to the vision provider. vision_analyze_tool and its native fast path (_vision_analyze_native) had the same gap in their local-file branches; they were only incidentally protected by the image magic-byte sniff rejecting non-image content, not by the intended read guard. Add raise_if_read_blocked() to all three local-file branches, mirroring the existing plugins/image_gen and plugins/video_gen call sites.
Summary
OpenAI and OpenRouter image-generation providers accepted local
image_url/reference_image_urlsinputs and read those paths before sending the bytes to the upstream provider.Those local input paths did not use Hermes' existing
agent.file_safety.get_read_block_error()guard. As a result, a model/tool-supplied path to a Hermes credential store could be read as source image input and leave the machine in the provider request.The OpenAI Codex image provider already guards this same local-image path; this applies the same read boundary to the OpenAI and OpenRouter image providers.
Changes
get_read_block_error().get_read_block_error().~/.hermes/auth.jsonbeing rejected before local bytes are read/inlined.Security impact
This prevents image-generation reference inputs from bypassing Hermes' credential-file read denylist and sending local credential stores to external image providers.
Tests