Skip to content

fix: route renamed R workspace artifacts#1322

Open
mldangelo-oai wants to merge 4 commits into
mdangelo/codex/fix-r-serialized-incomplete-analysisfrom
mdangelo/codex/fix-renamed-r-serialized-routing
Open

fix: route renamed R workspace artifacts#1322
mldangelo-oai wants to merge 4 commits into
mdangelo/codex/fix-r-serialized-incomplete-analysisfrom
mdangelo/codex/fix-renamed-r-serialized-routing

Conversation

@mldangelo-oai
Copy link
Copy Markdown
Contributor

@mldangelo-oai mldangelo-oai commented May 25, 2026

Summary

  • Route misleading-suffix R workspace artifacts only when they carry a complete workspace serialization prefix (RDX*/RDA* plus X/A/B marker).
  • Keep weak raw stream markers and incomplete workspace-like prefixes extension-scoped so ordinary text near-matches are not promoted into R scans.
  • Make scanner acceptance agree with routing, document renamed-workspace support, and cover direct, filtered-directory, and nested archive behavior.

Stack

Stacked on #1312 (mdangelo/codex/fix-r-serialized-incomplete-analysis), which owns R incomplete-analysis and cache semantics and is green in CI.

Root Cause

detect_file_format_from_magic() and the directory prefilter recognized R workspace content, but full detect_file_format() only returned r_serialized for an R suffix. Core therefore retained a renamed .jpg workspace artifact yet dispatched it as unknown, losing actionable findings.

During review of the initial patch, an additional false-positive boundary appeared: promoting either a bare raw stream marker (X\n) or an incomplete workspace-like prefix (RDX3\nQ\n) under a misleading suffix causes ordinary text-like files to be retained as inconclusive R scans. The final routing predicate requires the complete distinctive renamed-workspace prefix and is shared with scanner acceptance.

Behavior

Before this change, an RDX3\nX\n payload named payload.rds reported critical executable/payload findings and exited 1, while identical bytes named payload.jpg produced only an unknown/inconclusive notice with exit 2. After this change, renamed complete-header payloads report the critical findings in direct, directory, and nested ZIP scans; a benign complete-header workspace remains clean; and both X\nordinary exported table and RDX3\nQ\nordinary exported table near-matches remain unknown/skipped.

Validation

  • env -u UV_EXCLUDE_NEWER -u VIRTUAL_ENV uv --no-config run --locked ruff format modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
  • env -u UV_EXCLUDE_NEWER -u VIRTUAL_ENV uv --no-config run --locked ruff check --fix modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
  • env -u UV_EXCLUDE_NEWER -u VIRTUAL_ENV uv --no-config run --locked mypy modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/ (Success: no issues found in 451 source files)
  • env -u UV_EXCLUDE_NEWER -u VIRTUAL_ENV PROMPTFOO_DISABLE_TELEMETRY=1 uv --no-config run --locked pytest tests/scanners/test_r_serialized_scanner.py tests/utils/file/test_filetype.py tests/utils/file/test_file_filter.py tests/test_core.py tests/scanners/test_zip_scanner.py --maxfail=1 -q (371 passed)
  • env -u UV_EXCLUDE_NEWER -u VIRTUAL_ENV PROMPTFOO_DISABLE_TELEMETRY=1 uv --no-config run --locked pytest -n auto -m "not slow and not integration" --maxfail=1 (4751 passed, 971 skipped)
  • npx prettier --check CHANGELOG.md README.md docs/user/compatibility-matrix.md
  • git diff --check

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 25, 2026

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: 623.14ms -> 628.52ms (+0.9%).

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 398.9us 389.8us -2.3% stable
direct-malicious-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_direct_malicious_upload malicious_reduce 52 B 1 1.30ms 1.27ms -2.2% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_base64] nested_base64 98 B 1 404.1us 395.7us -2.1% stable
clean-training-checkpoint tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_clean_training_checkpoint safe_large 278.2 KiB 1 11.27ms 11.07ms -1.9% stable
suspicious-pickle-intake tests/benchmarks/test_scan_benchmarks.py::test_scan_suspicious_pickle_intake suspicious-intake 183.8 KiB 4 85.50ms 86.82ms +1.5% stable
mixed-model-repository tests/benchmarks/test_scan_benchmarks.py::test_scan_release_candidate_repository release-candidate 547.3 KiB 32 244.72ms 248.48ms +1.5% stable
chunked-upload-stream tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_chunked_upload_stream chunked_stream 278.2 KiB 1 14.44ms 14.58ms +0.9% stable
padded-multi-stream-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_padded_multi_stream_upload multi_stream_padded 4.1 KiB 1 1.36ms 1.35ms -0.8% stable
single-checkpoint-preflight tests/benchmarks/test_scan_benchmarks.py::test_scan_single_checkpoint_before_load single_checkpoint.pkl 183.0 KiB 1 34.03ms 34.27ms +0.7% stable
warm-cache-rescan tests/benchmarks/test_scan_benchmarks.py::test_scan_warm_cached_repository_rescan release-candidate 547.3 KiB 32 48.64ms 48.79ms +0.3% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_hex] nested_hex 130 B 1 411.6us 410.4us -0.3% stable
duplicate-heavy-registry tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_registry_snapshot registry-snapshot 915.2 KiB 13 180.66ms 180.69ms +0.0% stable

…incomplete-analysis' into mdangelo/codex/review-pr1322
@mldangelo-oai mldangelo-oai marked this pull request as ready for review May 25, 2026 03:24
@mldangelo-oai mldangelo-oai requested a review from Copilot May 25, 2026 04:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Routes renamed R workspace artifacts (e.g., .jpg carrying real R workspace serialization bytes) to the r_serialized scanner only when the header is a strong/complete match, preventing weak near-matches from being promoted into R scans and aligning scanner acceptance with routing.

Changes:

  • Tighten magic-byte detection to require RDX*/RDA* + X/A/B marker for renamed R workspace artifacts, while keeping weaker markers extension-scoped.
  • Update RSerializedScanner.can_handle() to match the same strong-header acceptance rule for renamed artifacts.
  • Add/extend tests and docs to cover direct scans, directory skip-filter behavior, and nested archive routing.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
modelaudit/utils/file/detection.py Adds strong-header predicate for renamed R workspace routing and uses it in magic/full detection paths.
modelaudit/scanners/r_serialized_scanner.py Aligns scanner acceptance with strong-header routing for renamed artifacts.
tests/utils/file/test_filetype.py Adds detection tests for renamed workspace headers and weak-marker near-matches.
tests/utils/file/test_file_filter.py Adds skip-filter coverage ensuring renamed workspaces are preserved without promoting near-matches.
tests/scanners/test_r_serialized_scanner.py Adds core and archive routing tests to ensure critical findings are preserved for renamed artifacts.
README.md Documents support for signature-valid renamed workspace artifacts.
docs/user/compatibility-matrix.md Documents renamed-workspace support and clarifies weak lookalikes aren’t promoted.
CHANGELOG.md Records the renamed-workspace routing behavior change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread modelaudit/utils/file/detection.py Outdated
Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

ℹ️ 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".

Copy link
Copy Markdown
Contributor

@ianw-oai ianw-oai left a comment

Choose a reason for hiding this comment

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

Reviewed for the mldangelo-oai approval sweep. Focused change, green checks, and no blocking concerns.

…incomplete-analysis' into mdangelo/codex/fix-renamed-r-serialized-routing

# Conflicts:
#	README.md
#	docs/user/compatibility-matrix.md
#	modelaudit/utils/file/detection.py
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.

3 participants