Skip to content

fix: escape text output controls - #1580

Merged
mldangelo-oai merged 6 commits into
mainfrom
mdangelo/codex/fix-text-output-control-escaping
Jun 9, 2026
Merged

mldangelo-oai merged 6 commits into
mainfrom
mdangelo/codex/fix-text-output-control-escaping

Conversation

@mldangelo-oai

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

Copy link
Copy Markdown
Contributor

Summary

  • escape terminal, Unicode formatting, surrogate, and line-separator controls in human-readable CLI output
  • keep exact local filesystem paths for SBOM/report identity while redacting remote identifiers
  • sanitize progress callbacks, output confirmations, metadata previews, and model-controlled exception sinks
  • suppress raw verbose tracebacks at acquisition and scan boundaries so chained causes cannot reintroduce secrets or controls
  • preserve absent issue locations and tolerate malformed model metadata without weakening output safety
  • integrate current main at 5cf4945de43de7f977cce985410a9bb6bacce4a7, including the shared cross-platform baseline from fix: restore cross-platform scanner baselines #1597

False-positive / false-negative audit

  • Unicode bidi, zero-width, tag, surrogate, terminal, and line-separator controls render visibly at output sinks
  • exact local paths remain unmodified for scanning, report bookkeeping, SBOM component identity, and hashes
  • Hugging Face metadata previews and verbose preflight failures cannot inject controls or leak query credentials
  • remote cloud, JFrog, MLflow, PyTorch Hub, stream, and Hugging Face identifiers use source-aware redaction before terminal escaping
  • raw chained exceptions cannot bypass the sanitized outer message under --verbose
  • current-main shard error bookkeeping is preserved at every resolved conflict
  • cross-platform sink tests avoid illegal Windows filenames while the SBOM identity regression uses a valid bidi-format filename

Focused validation

  • complete associated CLI module: 250 passed, 3 skipped (expected Windows-only semantics)
  • scoped Ruff lint and format checks: clean
  • scoped mypy: clean
  • git diff --check: clean
  • exact published tree: 83ec4ec0507bdbc5a175945ac008386c46ab23aa

Both inline review threads are resolved. No full local suite was run; CI owns broad validation. Published head: 5c0b5333e3fdd6d9b2f5faf52fda9cce8b2af2b0.

@github-actions

github-actions Bot commented Jun 8, 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.334s -> 1.338s (+0.3%).

Workload Benchmark Target Size Files Baseline Current Change Status
padded-multi-stream-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_padded_multi_stream_upload multi_stream_padded 4.1 KiB 1 601.8us 577.2us -4.1% stable
clean-training-checkpoint tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_clean_training_checkpoint safe_large 278.2 KiB 1 104.03ms 106.80ms +2.7% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_raw] nested_raw 78 B 1 518.2us 509.4us -1.7% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_base64] nested_base64 98 B 1 533.6us 524.9us -1.6% stable
single-checkpoint-preflight tests/benchmarks/test_scan_benchmarks.py::test_scan_single_checkpoint_before_load single_checkpoint.pkl 183.0 KiB 1 66.65ms 65.81ms -1.3% stable
duplicate-heavy-registry tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_registry_snapshot registry-snapshot 915.2 KiB 13 366.63ms 371.26ms +1.3% stable
mixed-model-repository tests/benchmarks/test_scan_benchmarks.py::test_scan_release_candidate_repository release-candidate 547.3 KiB 32 468.50ms 462.92ms -1.2% stable
suspicious-pickle-intake tests/benchmarks/test_scan_benchmarks.py::test_scan_suspicious_pickle_intake suspicious-intake 183.8 KiB 4 117.57ms 118.90ms +1.1% stable
warm-cache-rescan tests/benchmarks/test_scan_benchmarks.py::test_scan_warm_cached_repository_rescan release-candidate 547.3 KiB 32 98.38ms 99.41ms +1.0% stable
direct-malicious-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_direct_malicious_upload malicious_reduce 52 B 1 493.6us 489.0us -0.9% stable
chunked-upload-stream tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_chunked_upload_stream chunked_stream 278.2 KiB 1 109.22ms 110.05ms +0.8% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_hex] nested_hex 130 B 1 552.3us 552.9us +0.1% stable

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

@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: 8c3daa5820

ℹ️ 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/cli.py Outdated

Copy link
Copy Markdown
Contributor Author

Critical review and focused QA complete. Published audited head 58b3ebf84fa7abc83d4677007c4d4b3bfabc22c2.

Closed false negatives in model metadata, skipped/suppressed paths, source display helpers, Unicode format controls, and the metadata table command; fixed the [None] location false positive and malformed numeric metadata output DoS.

Focused validation: 25 formatter/display/metadata tests and 71 remote-source CLI tests passed. Ruff, format check, mypy, and diff checks are clean. Moving on without waiting for CI, per the review workflow.

@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: 58b3ebf84f

ℹ️ 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/cli.py Outdated

Copy link
Copy Markdown
Contributor Author

@codex address the remaining SBOM path feedback using the verified approach in local commit 825a5378: split path redaction from terminal escaping so local filesystem paths remain exact for SBOM generation, while terminal output stays escaped and remote identifiers stay redacted. Please include the local-control-character bookkeeping and end-to-end hash regression described in the review-thread reply.

@chatgpt-codex-connector

Copy link
Copy Markdown

Summary

Testing

  • ✅ uv run ruff format modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
  • ✅ uv run ruff check --fix modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
  • ✅ uv run mypy modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
  • ✅ PROMPTFOO_DISABLE_TELEMETRY=1 uv run pytest tests/test_cli.py -k "sbom_preserves_local_control_path or display_scan_path_preserves or scan_path_state_preserves_local_control_path or scan_path_state_redacts_stream_fallback or progress_callback_escapes_local_control_path"
  • ✅ git diff --check
  • ❌ PROMPTFOO_DISABLE_TELEMETRY=1 uv run pytest -n auto -m "not slow and not integration" --maxfail=1 — stopped after an extended run; output had already shown unrelated failures before completion.

View task →

Copy link
Copy Markdown
Contributor Author

@codex please publish local review commit 06eb25c5 onto this PR's head branch.

Critical review found one remaining false negative in the published head a5914a2c: the Hugging Face metadata preview printed remote model_id values without terminal-control escaping, and its verbose failure log used the raw URL plus exception, allowing control injection and ?token= credential leakage before the sanitized download path ran.

The commit escapes the preview model ID and routes metadata-preflight logging through display_path / _display_error(). Added regressions cover control/Unicode injection and verbose token redaction.

Validation on the exact current-main merge:

  • PROMPTFOO_DISABLE_TELEMETRY=1 .venv/bin/pytest tests/test_cli.py -> 238 passed, 3 skipped (Windows-only)
  • .venv/bin/ruff check modelaudit/cli.py tests/test_cli.py
  • .venv/bin/ruff format --check modelaudit/cli.py tests/test_cli.py
  • .venv/bin/mypy modelaudit/cli.py tests/test_cli.py
  • git diff --check

Shell GitHub authentication rejected the push, so do not merge a5914a2c without this fix.

@chatgpt-codex-connector

Copy link
Copy Markdown

Summary

  • Escaped the Hugging Face metadata preview model_id before printing, and also safely renders the preview file count. modelaudit/cli.pyL2583-L2584
  • Routed Hugging Face metadata-preflight debug logging through the already-sanitized display_path and _display_error() helpers so verbose logs redact ?token= credentials and render terminal controls visibly. modelaudit/cli.pyL2588-L2593
  • Added regressions for metadata preview control/Unicode escaping and verbose metadata-preflight token redaction. tests/test_cli.pyL2994-L3068
  • The requested local object 06eb25c5 was not present in this checkout, so I reconstructed the described fix on top of PR head a5914a2 and committed it as c7a6707 fix: sanitize hf metadata preview.
  • Created the PR via make_pr with title: fix: sanitize Hugging Face metadata preview output.

Testing

  • ✅ PROMPTFOO_DISABLE_TELEMETRY=1 .venv/bin/pytest tests/test_cli.py -k "huggingface_metadata_preview_escapes_model_id or huggingface_metadata_preflight_verbose_log_is_sanitized"
  • ✅ .venv/bin/ruff check modelaudit/cli.py tests/test_cli.py
  • ✅ .venv/bin/ruff format --check modelaudit/cli.py tests/test_cli.py
  • ✅ .venv/bin/mypy modelaudit/cli.py tests/test_cli.py
  • ✅ uv run ruff format modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
  • ✅ uv run ruff check --fix modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
  • ✅ uv run mypy modelaudit/ packages/modelaudit-picklescan/src packages/modelaudit-picklescan/tests tests/
  • ✅ git diff --check
  • ⚠️ PROMPTFOO_DISABLE_TELEMETRY=1 .venv/bin/pytest tests/test_cli.py — failed in this root container on pre-existing POSIX report-writer symlink/permission tests unrelated to this Hugging Face metadata patch.
  • ⚠️ PROMPTFOO_DISABLE_TELEMETRY=1 uv run pytest -n auto -m "not slow and not integration" --maxfail=1 — stopped after the same unrelated report-writer failures appeared during the extended run.

View task →

Copy link
Copy Markdown
Contributor Author

Published the required Hugging Face metadata-output fix from #issuecomment-4655090863 at 1fbb0522b941d07a8cb3db9215144b839b790c1c. Preview model_id and file-count fields now escape terminal/Unicode controls, and metadata-preflight debug logging uses the redacted display URL plus _display_error() so ?token= credentials and control-bearing exceptions cannot leak under --verbose. The current-main merge conflict preserved both main's MLflow/privacy redaction and this PR's exact local SBOM-path behavior. Complete associated CLI validation: 239 passed, 3 skipped; scoped Ruff/format/mypy/diff checks are clean. Exact tree: 6796abfaea66b8827ac4e7d9aed75c601ed88098. All inline threads remain resolved.

@mldangelo-oai
mldangelo-oai force-pushed the mdangelo/codex/fix-text-output-control-escaping branch from 1fbb052 to e8ca930 Compare June 9, 2026 07:59

Copy link
Copy Markdown
Contributor Author

Critical audit follow-up found and fixed one remaining P1: the prior exc_info=verbose and sanitized_message == str(exc) guard only inspected the outer exception, so a harmless outer error chained from a signed-URL/control-bearing cause still emitted the raw cause traceback. The published head removes raw tracebacks from model-controlled scan/acquisition failure sinks and upgrades the direct Hugging Face failure regression to a chained exception. Associated CLI validation remains 239 passed, 3 Windows-only skips; focused verbose-redaction slice: 4 passed. Exact tree: 2995a247d34b7b6555512b36176e58690df07f06.

@mldangelo-oai
mldangelo-oai marked this pull request as draft June 9, 2026 08:26

Copy link
Copy Markdown
Contributor Author

Return review published at 32f729fbe750800781ab99100956efb62e78c0f6 (tree 8ff2a04812ea454bdc7eaec8ecb4d70b3f448f3a).

I merged current main and fixed both observed CI root causes without weakening production escaping: the output-confirmation test no longer creates an illegal Windows filename, and the Hugging Face preview test explicitly runs in text mode. I also removed the same latent Windows dependency from adjacent sink tests and kept an end-to-end SBOM identity regression using a valid bidi-format filename.

Focused QA: 239 CLI tests passed with 3 existing platform skips, plus scoped Ruff/format/mypy and diff checks. Both inline threads are resolved and all substantive top-level requests are incorporated. Fresh CI is running; moving on without waiting.

@mldangelo-oai
mldangelo-oai marked this pull request as ready for review June 9, 2026 08:54
@mldangelo-oai
mldangelo-oai marked this pull request as draft June 9, 2026 10:24
@mldangelo-oai
mldangelo-oai marked this pull request as ready for review June 9, 2026 13:41
@mldangelo-oai
mldangelo-oai merged commit 164bbb9 into main Jun 9, 2026
26 of 27 checks passed
@mldangelo-oai
mldangelo-oai deleted the mdangelo/codex/fix-text-output-control-escaping branch June 9, 2026 13:41
@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