feat(image-gen): validate image references - #26968
Conversation
This comment was marked as spam.
This comment was marked as spam.
95e2f29 to
8440e89
Compare
|
Addressed review feedback in the updated stack:
|
8440e89 to
6e5fa50
Compare
6e5fa50 to
2bf38b2
Compare
2bf38b2 to
5c1a56b
Compare
5c1a56b to
a8e0afc
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the validation logic and for addressing the encoded-length and header-only-read feedback.
Problems
agent/image_reference.py:50is not reached by the current image-generation path.tools/image_generation_tool.py:1343-1349only normalizes reference strings before callingprovider.generate(). Current providers still load references independently: OpenAI atplugins/image_gen/openai/__init__.py:282-284, xAI atplugins/image_gen/xai/__init__.py:140-151, and OpenRouter atplugins/image_gen/openrouter/__init__.py:99-111. Therefore this PR does not yet enforce its stated validation policy.- Integration needs to cover provider-specific contracts. For example, OpenAI currently reads any non-denied local path at
plugins/image_gen/openai/__init__.py:149-155, whereas Codex has independent local/data validation atplugins/image_gen/openai-codex/__init__.py:179-237.
Suggested changes
- Wire validation through a production dispatch/provider path and add dispatch-level tests proving rejected references never reach a provider.
- Reconcile this helper with the existing unified image resolver in
tools/image_source.py:89-160rather than leaving parallel validation semantics.
Automated hermes-sweeper review.
| """Typed validation error for image references.""" | ||
|
|
||
| def __init__(self, message: str, *, error_type: str = "invalid_argument") -> None: | ||
| super().__init__(message) |
There was a problem hiding this comment.
This validator is not wired into the current image-generation path: tools/image_generation_tool.py:1343-1349 only normalizes strings before dispatch, and providers still load or forward references independently. Please integrate it at a production chokepoint (with dispatch-level coverage); otherwise the cache/type/size policy has no runtime effect.
a8e0afc to
e210a75
Compare
|
Addressed the hermes-sweeper findings in
Focused verification: 174 tests passed; all required GitHub checks are green. Ready for re-review. |
|
Closing this stale implementation after reviewing it against current main. The image-input safety boundary remains valuable, but any future version should be rebuilt as a minimal handler-level pre-dispatch gate that reuses the current |
Summary
http(s)URLs, validdata:image/*;base64,...URLs, and local Hermes image-cache paths with size/type checks.Relationship
mainso CI remains independent.Testing
venv/bin/python -m py_compile agent/image_reference.py tests/agent/test_image_reference.pyvenv/bin/python -m pytest tests/agent/test_image_reference.py -q -o 'addopts='git diff --cached --check