Skip to content

fix: harden JFrog redirect targets - #1520

Merged
mldangelo-oai merged 1 commit into
mainfrom
mdangelo/codex/fix-jfrog-redirect-ssrf-publish
Jun 9, 2026
Merged

mldangelo-oai merged 1 commit into
mainfrom
mdangelo/codex/fix-jfrog-redirect-ssrf-publish

Conversation

@mldangelo-oai

@mldangelo-oai mldangelo-oai commented Jun 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • integrate JFrog redirect validation directly into the source module and remove the import-time monkeypatch
  • prevent DNS-rebinding and proxy split-DNS bypasses by rejecting arbitrary off-origin redirect hostnames unless they are trusted JFrog hosts or explicitly listed in MODELAUDIT_JFROG_ALLOWED_REDIRECT_HOSTS
  • allow canonical public IP redirect targets while rejecting private, loopback, link-local, CGNAT, multicast, reserved, site-local, IPv4-mapped, NAT64, Teredo, 6to4, ISATAP, scoped IPv6, and legacy mixed-radix IPv4 destinations
  • suppress ambient .netrc authentication so Requests cannot add credentials to allowlisted redirect targets or replace explicit Bearer auth
  • keep credentials only on the original effective origin, including normalized default ports; alternate-port and untrusted redirects receive no authorization headers
  • permanently drop credentials and trusted session state after an untrusted hop, and isolate redirect cookies by effective origin
  • document custom JFrog and redirect-host configuration and register the regression suite for reduced CI lanes
  • synchronize with current main at dc0051c5bd1fa22f3779b94e9624a78f46da37d1

False-positive / false-negative audit

  • reject IPv4-mapped IPv6 in its original form before any unwrapping
  • reject short, octal, hexadecimal, and mixed-radix IPv4 spellings
  • reject globally routed IPv6 literals carrying a scope identifier
  • reject both RFC 5214 ISATAP interface-identifier variants
  • preserve credentials and cookies across same-origin/default-port redirects
  • strip credentials across alternate-port or untrusted redirects and never restore them after returning
  • keep cookies isolated between distinct untrusted origins
  • allow explicitly configured private redirect destinations without forwarding JFrog credentials
  • found no additional high-confidence bypass after an independent current-main parser/redirect audit

Validation

  • associated JFrog modules: 211 passed, 1 skipped (optional ONNX dependency unavailable)
  • scoped Ruff check and format: passed
  • scoped mypy: passed
  • Prettier check for CHANGELOG.md and README.md: passed
  • git diff --check: passed
  • full local suite intentionally not run per PR-audit workflow

Published commit: 67bcc37159f10b8a9faadcacc3ba8d1d608676b2
Verified tree: f74e9b07ff5e40b21555d11c408431b63d58229e

@github-actions

github-actions Bot commented Jun 6, 2026 •

Copy link
Copy Markdown
Contributor

Workflow run and artifacts

Performance Benchmarks

Compared 12 shared benchmarks with a regression threshold of 15%.
Status: 0 regressions, 0 improved, 12 stable, 0 new, 0 missing.
Aggregate shared-benchmark median: 1.259s -> 1.261s (+0.1%).

Workload Benchmark Target Size Files Baseline Current Change Status
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_raw] nested_raw 78 B 1 488.2us 461.8us -5.4% stable
warm-cache-rescan tests/benchmarks/test_scan_benchmarks.py::test_scan_warm_cached_repository_rescan release-candidate 547.3 KiB 32 80.69ms 83.54ms +3.5% stable
padded-multi-stream-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_padded_multi_stream_upload multi_stream_padded 4.1 KiB 1 541.9us 528.6us -2.5% stable
direct-malicious-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_direct_malicious_upload malicious_reduce 52 B 1 438.0us 431.1us -1.6% stable
clean-training-checkpoint tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_clean_training_checkpoint safe_large 278.2 KiB 1 108.27ms 107.18ms -1.0% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_hex] nested_hex 130 B 1 473.8us 469.5us -0.9% stable
chunked-upload-stream tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_chunked_upload_stream chunked_stream 278.2 KiB 1 110.45ms 110.89ms +0.4% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_base64] nested_base64 98 B 1 474.7us 476.5us +0.4% stable
mixed-model-repository tests/benchmarks/test_scan_benchmarks.py::test_scan_release_candidate_repository release-candidate 547.3 KiB 32 425.60ms 424.00ms -0.4% stable
single-checkpoint-preflight tests/benchmarks/test_scan_benchmarks.py::test_scan_single_checkpoint_before_load single_checkpoint.pkl 183.0 KiB 1 61.53ms 61.75ms +0.4% stable
duplicate-heavy-registry tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_registry_snapshot registry-snapshot 915.2 KiB 13 361.31ms 361.65ms +0.1% stable
suspicious-pickle-intake tests/benchmarks/test_scan_benchmarks.py::test_scan_suspicious_pickle_intake suspicious-intake 183.8 KiB 4 109.14ms 109.24ms +0.1% stable

@mldangelo-oai mldangelo-oai changed the title [codex] fix JFrog private redirect SSRF fix: harden JFrog redirect targets Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Security review follow-up pushed in 7162241e: removed the DNS preflight TOCTOU, default-denied arbitrary redirect hostnames, blocked IPv6 transition/site-local targets, preserved public literal-IP and explicit-host redirects without credentials, added reduced-lane coverage, and documented the opt-in. Focused result: 185 passed, 1 optional-ONNX skip; scoped Ruff and mypy clean.

@mldangelo-oai
mldangelo-oai marked this pull request as ready for review June 6, 2026 16:20

Copy link
Copy Markdown
Contributor Author

Critical security review complete. The original hostname acceptance was vulnerable to DNS rebinding/proxy split-DNS behavior, so the final implementation defaults off-origin hostnames to denied unless explicitly allowlisted, preserves no-credential/no-trusted-cookie behavior for those targets, and blocks IP-literal evasions including IPv4-mapped, NAT64, 6to4, Teredo, scoped IPv6, integer/octal/hex, shorthand, and mixed-radix forms. Final associated QA: 189 passed, 1 optional-ONNX skip; scoped Ruff and mypy clean. Full suite intentionally left to CI.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e4419f7c5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread modelaudit/utils/sources/jfrog.py Outdated
Comment thread modelaudit/utils/sources/jfrog.py Outdated
Comment thread modelaudit/utils/sources/jfrog.py

Copy link
Copy Markdown
Contributor Author

Follow-up review found and fixed one additional SSRF false negative locally in c4736a25: globally routed ISATAP literals can embed an IPv4 destination in the interface identifier and were accepted by the public-IP check. The local commit now covers all three existing review findings plus ISATAP, with canonical-only IPv4 parsing, original-address IPv4-mapped checks, unconditional scoped-IPv6 rejection, and ISATAP rejection. Focused validation: 199 passed; scoped Ruff, format check, mypy, and git diff --check are clean. The workspace Git credential still fails authentication, so this commit is not yet on the PR branch and the three review threads intentionally remain unresolved.

Copy link
Copy Markdown
Contributor Author

Published the review fixes at ca6844aa: canonical-only IPv4 parsing, original-form IPv4-mapped rejection, unconditional scoped-IPv6 rejection, and both RFC 5214 ISATAP interface-identifier variants. Focused result: 198 passed, 1 optional-ONNX skip; scoped Ruff, mypy, and diff checks are clean. The three previously blocked review threads are now resolved.

@mldangelo-oai
mldangelo-oai force-pushed the mdangelo/codex/fix-jfrog-redirect-ssrf-publish branch from ca6844a to 434cc83 Compare June 8, 2026 20:24

Copy link
Copy Markdown
Contributor Author

@codex please sync the current main branch into this PR branch and push the merge. I re-reviewed exact head 434cc83d, found no additional high-confidence redirect/auth false positive or false negative, and merged current main (f856f9c3) cleanly locally as ad5bbd44. Post-merge focused validation is clean: 211 passed across tests/integrations/test_jfrog.py and tests/integrations/test_jfrog_redirect_security.py; scoped Ruff, format, mypy, and diff checks also pass. The workspace HTTPS credential cannot publish the merge commit.

@chatgpt-codex-connector

Copy link
Copy Markdown

Summary

  • Applied the current-main CVE-2025-12060 truthiness follow-up locally on commit b4de642 because this workspace could not fetch or push GitHub refs through the HTTPS proxy.
  • Updated the CVE-2025-12060 explanation text to describe truthy untar and truthy extract arguments enabling remote tar extraction.
  • Updated Keras ZIP scanner detection details and argument handling so direct, kwargs, and positional untar/extract values use Keras/Python truthiness semantics.
  • Added regression coverage confirming truthy kwargs/positional extraction arguments are detected and false-like values are not reported as CVE-2025-12060.

Testing

  • ⚠️ git fetch origin main — blocked by environment proxy: CONNECT tunnel failed, response 403.
  • ✅ uv run ruff format modelaudit/config/explanations.py modelaudit/scanners/keras_zip_scanner.py tests/scanners/test_keras_zip_scanner.py
  • ✅ uv run ruff check modelaudit/config/explanations.py modelaudit/scanners/keras_zip_scanner.py tests/scanners/test_keras_zip_scanner.py
  • ✅ uv run mypy modelaudit/config/explanations.py modelaudit/scanners/keras_zip_scanner.py tests/scanners/test_keras_zip_scanner.py
  • ✅ PROMPTFOO_DISABLE_TELEMETRY=1 uv run pytest -q tests/scanners/test_keras_zip_scanner.py -k "get_file" --maxfail=1
  • ✅ git diff --check
  • ⚠️ git push origin HEAD:work — blocked by environment proxy: CONNECT tunnel failed, response 403.

I also recorded a PR message via the PR tool, but the branch itself was not published because git push is blocked in this workspace.

View task →

@mldangelo-oai
mldangelo-oai force-pushed the mdangelo/codex/fix-jfrog-redirect-ssrf-publish branch from 434cc83 to 67bcc37 Compare June 9, 2026 07:41

Copy link
Copy Markdown
Contributor Author

Re-reviewed the exact live PR on current main after #1526 and #1581 merged. The combined tree preserves the three prior SSRF fixes plus the ISATAP follow-up, and an independent parser/redirect audit found no additional high-confidence credential, cookie, URL-parser, IP-classification, or response-lifecycle bypass.

The associated JFrog modules pass 211 passed, 1 optional-ONNX skip; scoped Ruff, mypy, Prettier, and diff checks are clean. Published as the single current-main commit 67bcc37159f10b8a9faadcacc3ba8d1d608676b2 with verified tree f74e9b07ff5e40b21555d11c408431b63d58229e.

@mldangelo-oai
mldangelo-oai merged commit ea54e39 into main Jun 9, 2026
26 of 30 checks passed
@mldangelo-oai
mldangelo-oai deleted the mdangelo/codex/fix-jfrog-redirect-ssrf-publish branch June 9, 2026 08:13
@github-actions github-actions Bot mentioned this pull request Jun 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant