fix: require defusedxml for pmml parsing

[mldangelo-oai]

Summary

• remove the unsafe stdlib XML fallback from PMML parsing and fail closed when the required hardened parser is unavailable
• treat import failures and incomplete defusedxml installations as parser unavailability, including missing or non-callable ElementTree.fromstring
• preserve dangerous XML prechecks before dependency failure so XXE/DOCTYPE findings still determine the security exit code
• key scan caches on hardened PMML parser availability so a previously cached clean result cannot bypass fail-closed behavior
• register the dependency regression in reduced Python CI lanes and keep fixtures deterministic under tmp_path
• synchronize with current main

Security and QA

• missing or broken defusedxml returns an explicit inconclusive result instead of crashing or parsing attacker-controlled XML unsafely
• subprocess regressions scan a real PMML file for exception-raising, missing-parser, and non-callable-parser packages
• clean PMML remains fail-closed when the parser is unavailable; malicious DOCTYPE input still reports the critical preparse finding
• a healthy-to-unavailable dependency transition invalidates the prior cache context and returns exit code 2 instead of stale exit code 0
• all inline review threads and substantive top-level feedback are addressed

Validation

• tests/scanners/test_pmml_scanner.py
• tests/scanners/test_pmml_dependency_handling.py
• result after current-main sync: 52 passed
• two adjacent h5py cache-transition tests were selected but skipped because h5py is not installed in this environment
• scoped Ruff check and format
• scoped mypy
• git diff --check origin/main...HEAD
• exact published tree: cb80854a7b13ae64f2f225b7ac4bbb6acb22e01f

Full pytest remains delegated to CI. Final reviewed head: eb2df8ac7c10aeff9c638e7b175baf4cce0f9aa6, based on main def9169bd2f7ab06d6e094023a52ab6c575bfbc7.

[github-actions]

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.350s -> 1.339s (-0.8%).

Workload	Benchmark	Target	Size	Files	Baseline	Current	Change	Status
warm-cache-rescan	tests/benchmarks/test_scan_benchmarks.py::test_scan_warm_cached_repository_rescan	release-candidate	547.3 KiB	32	98.53ms	93.99ms	-4.6%	stable
nested-payload-review	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_base64]	nested_base64	98 B	1	517.1us	531.4us	+2.8%	stable
single-checkpoint-preflight	tests/benchmarks/test_scan_benchmarks.py::test_scan_single_checkpoint_before_load	single_checkpoint.pkl	183.0 KiB	1	68.13ms	66.93ms	-1.8%	stable
direct-malicious-upload	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_direct_malicious_upload	malicious_reduce	52 B	1	504.3us	497.2us	-1.4%	stable
chunked-upload-stream	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_chunked_upload_stream	chunked_stream	278.2 KiB	1	110.63ms	109.52ms	-1.0%	stable
duplicate-heavy-registry	tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_registry_snapshot	registry-snapshot	915.2 KiB	13	373.15ms	370.01ms	-0.8%	stable
suspicious-pickle-intake	tests/benchmarks/test_scan_benchmarks.py::test_scan_suspicious_pickle_intake	suspicious-intake	183.8 KiB	4	119.41ms	118.85ms	-0.5%	stable
nested-payload-review	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_raw]	nested_raw	78 B	1	518.1us	516.1us	-0.4%	stable
clean-training-checkpoint	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_clean_training_checkpoint	safe_large	278.2 KiB	1	107.74ms	107.33ms	-0.4%	stable
padded-multi-stream-upload	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_padded_multi_stream_upload	multi_stream_padded	4.1 KiB	1	582.9us	581.6us	-0.2%	stable
nested-payload-review	tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_hex]	nested_hex	130 B	1	527.2us	527.4us	+0.1%	stable
mixed-model-repository	tests/benchmarks/test_scan_benchmarks.py::test_scan_release_candidate_repository	release-candidate	547.3 KiB	32	469.72ms	469.69ms	-0.0%	stable

[mldangelo-oai]

Critical review found one dependency-failure gap: a present but broken defusedxml installation could raise a non-ImportError during module import and crash before the scanner's fail-closed path ran.

Fixed in 3ba4438ea7f6110e7409ba40ad45da5d0d584f53 by treating any import-time exception as dependency unavailability and adding a subprocess regression with a deliberately broken defusedxml package.

Focused validation:

• broken/missing dependency and XXE-preservation regressions: passed
• affected PMML scanner tests: passed
• scoped Ruff, format check, mypy, and git diff --check: clean

No actionable review threads were open. Moving on while CI runs.

[mldangelo-oai]

@codex Please publish local commit 8c1c1618 to branch mdangelo/codex/fix-pmml-defusedxml-required.

I re-fetched published head 3ba4438ea7f6110e7409ba40ad45da5d0d584f53 and merged current origin/main (f856f9c341c886501582846f204f806c91d4d971) cleanly. The effective PR delta remains limited to PMML fail-closed behavior: dangerous XML prechecks still run, missing or import-broken defusedxml produces an explicit inconclusive result, and the unsafe stdlib XML fallback is removed.

Focused validation on the merged head:

• tests/scanners/test_pmml_scanner.py + tests/scanners/test_pmml_dependency_handling.py: 44 passed, 1 skipped
• scoped Ruff lint and format: clean
• scoped mypy: clean
• git diff --check origin/main...HEAD: clean
• no review threads are open

Direct HTTPS push failed because this workspace's GitHub credential is invalid. Once 8c1c1618 is published, fresh CI can run and the PR can be rechecked for admin merge.

[chatgpt-codex-connector]

Result

I could not publish commit 8c1c1618 to mdangelo/codex/fix-pmml-defusedxml-required from this workspace.

Key findings:

• The requested commit 8c1c1618 is not present in the local repository.
• The local PR branch currently points at 3ba4438ea7f6110e7409ba40ad45da5d0d584f53.
• I created/updated the local branch mdangelo/codex/fix-pmml-defusedxml-required to point at the available local PR head 3ba4438.
• Publishing to GitHub failed because outbound GitHub access through the configured proxy returned 403 Forbidden.

No code changes were made, so I did not create a commit or call the PR tool.

Checks / Commands

• ✅ pwd && rg --files -g 'AGENTS.md' -g '!**/.venv/**' -g '!**/.git/**' && git status --short --branch
• ✅ cat AGENTS.md
• ✅ git branch --show-current && git log --oneline --decorate -n 20 --graph --all --boundary
• ❌ git rev-parse --verify 8c1c1618^{commit} && git show --stat --oneline --decorate 8c1c1618 — requested commit was not available locally.
• ❌ git remote add origin https://github.com/promptfoo/modelaudit.git && git fetch origin main mdangelo/codex/fix-pmml-defusedxml-required — GitHub fetch failed with CONNECT tunnel failed, response 403.
• ✅ git branch --force mdangelo/codex/fix-pmml-defusedxml-required 3ba4438ea7f6110e7409ba40ad45da5d0d584f53 && git status --short --branch && git log --oneline --decorate -n 5 mdangelo/codex/fix-pmml-defusedxml-required
• ❌ git push origin mdangelo/codex/fix-pmml-defusedxml-required:mdangelo/codex/fix-pmml-defusedxml-required — GitHub push failed with CONNECT tunnel failed, response 403.

View task →

[mldangelo-oai]

Current-main review found and fixed two remaining dependency-coverage gaps locally.

Local reviewed state:

• merge commit 1e30536f merges current main at 1c6978b2
• repair commit 11bb2439 validates that defusedxml.ElementTree.fromstring is actually callable before declaring the safe parser available
• the subprocess regression now scans a real PMML file for both an exception-raising package and an importable package exposing ElementTree = None
• tests/scanners/test_pmml_dependency_handling.py is registered in allowed_test_files so reduced Python lanes execute it
• the XXE preparse regression now uses a deterministic tmp_path URI

Validation: associated PMML files 46 passed; Ruff check clean; Ruff format clean across 410 files; mypy clean across 465 files; git diff --check clean. No full pytest suite was run, per campaign instructions. No review threads are open.

Direct push to mdangelo/codex/fix-pmml-defusedxml-required failed with Invalid username or token.

@codex address that feedback by publishing the equivalent of local commit 11bb2439 on top of current main. Fresh CI and admin-merge review should use the resulting exact published head.

[chatgpt-codex-connector]

Summary

• Validated PMML’s hardened parser availability by requiring defusedxml.ElementTree.fromstring to be callable before setting HAS_DEFUSEDXML; otherwise the scanner fails closed as parser-unavailable. modelaudit/scanners/pmml_scanner.pyL10-L21
• Expanded PMML dependency regressions to scan an actual PMML file in subprocesses for both broken defusedxml imports and importable packages exposing ElementTree = None. tests/scanners/test_pmml_dependency_handling.pyL10-L74
• Registered test_pmml_dependency_handling.py in the reduced Python-lane allowlist so the dependency fail-closed coverage runs there. tests/conftest.pyL139-L140
• Made the XXE preparse regression use a deterministic tmp_path-scoped file URI instead of a host path. tests/scanners/test_pmml_scanner.pyL121-L132
• Committed the changes as 061233a fix: validate pmml safe parser availability and created the PR record with the requested follow-up changes.

Testing

• ✅ uv run pytest tests/scanners/test_pmml_scanner.py tests/scanners/test_pmml_dependency_handling.py — 46 passed.
• ✅ uv run ruff format modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/ — 410 files left unchanged.
• ✅ uv run ruff check --fix modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/ — all checks passed.
• ✅ uv run mypy modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/ — success, no issues in 465 source files.
• ✅ git diff --check — clean.

Full pytest was not run, matching the campaign direction in the triggering review.

View task →

[mldangelo-oai]

Published the requested current-main PMML hardening at 2664ab2fc8cc851f8bd8367a60313944c418e20e.

The published head now:

• validates that defusedxml.ElementTree.fromstring is callable before declaring the safe parser available;
• scans a real PMML file in subprocess regressions for both import-time failure and ElementTree = None;
• registers test_pmml_dependency_handling.py in the reduced Python-lane allowlist;
• uses a deterministic tmp_path URI for the XXE preparse regression;
• preserves dangerous XML prechecks and removes the unsafe stdlib parsing fallback.

Focused validation: 46 associated PMML tests passed; scoped Ruff, format, mypy, and diff checks are clean. The branch is current with main, mergeable, and has no review threads; fresh CI can run while review moves to the next PR.

[mldangelo-oai]

Current-main audit complete and published as d98cd6a0583d227bb5eabe30b988e0643f9db22e (exact tree d0d14c98b2a26b3cdcb5fe655e8101ce773a09cd, parent ff68cb0f8296f582fce3dba1a0a210e71654a99a). Associated PMML tests: 46 passed. Scoped Ruff, format, mypy, and git diff --check are clean. No inline review threads remain; CI is now handling the broader matrix.

[mldangelo-oai]

Current-main audit published at f06049c91a6eff1c4ddb098f9df3843b5fe18c3d.

The review confirms the PMML scanner fails closed when defusedxml is missing, broken, or exposes a non-callable parser; dangerous XML prechecks remain ahead of the dependency failure so malicious entities still receive their specific finding. The real-file subprocess regressions cover both import-time failure and ElementTree=None, and the new reduced-lane test is allowlisted.

Focused validation: PMML scanner and dependency handling 46 passed; scoped Ruff, format, mypy, and git diff --check are clean. Full suite left to CI.

[mldangelo-oai]

Main advanced during publication, so I resynchronized once more. Current head is fab33a7b3f130f10ebdd485d29332c028af3ab17, parented directly on live main ae810485e9339aeeb2c55ff7958996ae82006fea; the reviewed PMML implementation/test blobs are unchanged.

[mldangelo-oai]

Critical follow-up review found a stale-clean-cache bypass in the new fail-closed contract. A PMML result cached while defusedxml was healthy was reused after the parser became unavailable, returning exit code 0 without executing the parser-unavailable branch.

Fixed in dd5326dd20ac8f62e979567f9c2ec22e6d672857 by adding hardened PMML parser availability to the cache version context. The regression first caches a clean scan, disables defusedxml, then verifies the same file misses the old context, reports pmml_safe_xml_parser_unavailable, and exits 2. I also added the exact non-callable ElementTree.fromstring cold-import shape.

After resolving the current-main test import overlap, all 52 associated PMML tests pass; scoped Ruff, format, mypy, and diff checks are clean. Final tree: cb80854a7b13ae64f2f225b7ac4bbb6acb22e01f.