Skip to content

fix: redact authorization evidence variants - #1544

Merged
mldangelo-oai merged 6 commits into
mainfrom
mdangelo/codex/fix-evidence-auth-redaction-forms
Jun 9, 2026
Merged

mldangelo-oai merged 6 commits into
mainfrom
mdangelo/codex/fix-evidence-auth-redaction-forms

Conversation

@mldangelo-oai

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

Copy link
Copy Markdown
Contributor

Summary

  • redact proxy, camel-case, mapped, Digest, and SigV4 authorization evidence without hiding benign credential metadata
  • preserve parseable Python expressions, f-string interpolations, shell suffixes, and executable context around redacted values
  • fail closed on unterminated parameterized credentials and multiline Python literal continuations
  • use a bounded linear scanner for quote-aware parameterized authorization values

False-positive / false-negative review

  • closed AWS SigV4 access-key/signature leakage, including reordered SignedHeaders
  • closed Digest response= leakage across quoted delimiters, truncated values, and indented or unindented multiline literals
  • covered ordinary, raw, triple-quoted, bytes, implicit-concatenation, and dynamic f-string forms
  • preserved Token or eval(...), f-string executable expressions, spaced and unspaced shell commands, and source ordering
  • preserved benign endpoint, algorithm, counter, status, enabled, timeout, expiry, type, version, and length metadata
  • independently audited the final live-head tree for false positives, false negatives, and regex performance

All inline review threads are resolved. Synced with main at 6e6ba57bfe0d9fbbef5ab7b8116432353daca8b2.

Validation

  • tests/scanners/test_evidence_redaction.py: 278 passed
  • scoped Ruff check and format check: clean
  • scoped mypy: clean
  • git diff --check: clean
  • remote tree verified byte-for-byte against the tested local tree
  • full suite delegated to CI

Published tested tree: 83b1e562f775ff1eb95a25310067264df1aeae47
Published head: c341dc0110d988ad7c963c1ba8133aa3eb946c18

@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.366s -> 1.358s (-0.6%).

Workload Benchmark Target Size Files Baseline Current Change Status
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_hex] nested_hex 130 B 1 454.5us 474.7us +4.5% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_raw] nested_raw 78 B 1 452.9us 441.3us -2.6% stable
warm-cache-rescan tests/benchmarks/test_scan_benchmarks.py::test_scan_warm_cached_repository_rescan release-candidate 547.3 KiB 32 87.88ms 90.02ms +2.4% stable
chunked-upload-stream tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_chunked_upload_stream chunked_stream 278.2 KiB 1 115.45ms 113.42ms -1.8% stable
padded-multi-stream-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_padded_multi_stream_upload multi_stream_padded 4.1 KiB 1 505.3us 513.8us +1.7% stable
mixed-model-repository tests/benchmarks/test_scan_benchmarks.py::test_scan_release_candidate_repository release-candidate 547.3 KiB 32 470.69ms 465.21ms -1.2% stable
nested-payload-review tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_nested_payload_review[nested_base64] nested_base64 98 B 1 456.8us 452.5us -1.0% stable
direct-malicious-upload tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_direct_malicious_upload malicious_reduce 52 B 1 425.2us 421.1us -0.9% stable
clean-training-checkpoint tests/benchmarks/test_picklescan_benchmarks.py::test_picklescan_clean_training_checkpoint safe_large 278.2 KiB 1 111.09ms 110.45ms -0.6% stable
duplicate-heavy-registry tests/benchmarks/test_scan_benchmarks.py::test_scan_duplicate_registry_snapshot registry-snapshot 915.2 KiB 13 387.69ms 385.49ms -0.6% stable
single-checkpoint-preflight tests/benchmarks/test_scan_benchmarks.py::test_scan_single_checkpoint_before_load single_checkpoint.pkl 183.0 KiB 1 71.09ms 70.81ms -0.4% stable
suspicious-pickle-intake tests/benchmarks/test_scan_benchmarks.py::test_scan_suspicious_pickle_intake suspicious-intake 183.8 KiB 4 120.15ms 120.37ms +0.2% stable

@mldangelo-oai mldangelo-oai left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I found and fixed two boundary issues on the current head:

  1. SENSITIVE_AUTH_SCHEME_ASSIGNMENT_RE accepted only a narrow credential alphabet. Unquoted scheme-bearing API/custom tokens containing valid real-world punctuation such as :, !, or % caused the scheme to be redacted while the credential remained visible. Example: apiKey = ApiKey COLON:SECRET123456 became apiKey = <redacted> COLON:SECRET123456.
  2. The camel-case control guard used a word boundary after Cache|Count|Enabled|Status|Timeout, so obvious benign extensions such as xApiKeyCounter, xApiKeyCount2, myApiKeyTimeoutMs, and sessionTokenEnabledFlag were still falsely redacted.

Local commit 27de41aa reuses the existing bounded unquoted-value grammar for scheme credentials and treats the control terms as suffix prefixes. It adds positive punctuation regressions and expanded near-match negatives.

Focused validation:

  • tests/scanners/test_evidence_redaction.py: 254 passed
  • targeted auth/camel-case slice: 12 passed
  • Ruff check/format: clean
  • scoped mypy: clean
  • changed regex scales linearly: ~0.006s for a one-million-character credential probe
  • git diff --check: clean

I could not push the commit because the available shell GitHub token is invalid, and the source/test files are too large for the connector's whole-file update endpoint. These findings remain on remote head c90c78e1.

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

@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: c90c78e1e2

ℹ️ 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/scanners/_evidence_redaction.py Outdated
Comment thread modelaudit/scanners/_evidence_redaction.py
Comment thread modelaudit/scanners/_evidence_redaction.py Outdated
Comment thread modelaudit/scanners/_evidence_redaction.py

Copy link
Copy Markdown
Contributor Author

Addressed all four review threads and synced current main.

Security/quality fixes:

  • fail closed on complete parameterized AWS4-HMAC-SHA256 and Digest Authorization header values;
  • reuse the bounded unquoted credential grammar so :, !, %, and similar valid token punctuation cannot escape after the auth scheme;
  • narrow camel-case prefixes so benign PascalCase metadata such as ModelAccessTokenEndpoint and RequestSignatureAlgorithm remains visible;
  • treat Algorithm, Counter, Count2, EnabledFlag, Endpoint, and TimeoutMs as benign control/metadata suffixes.

Focused validation:

  • tests/scanners/test_evidence_redaction.py: 255 passed
  • scoped Ruff, format, and mypy: clean
  • changed scheme regex: ~0.004s on a 1,000,000-character credential probe
  • git diff --check: clean

Published tested tree 97bc292ebdeb68dda22ca8969e753fe86d71ff23 at b6bb267bf9ea25b769d16c564e99bd253fb8d4fa. 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: b6bb267bf9

ℹ️ 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/scanners/_evidence_redaction.py Outdated
Comment thread modelaudit/scanners/_evidence_redaction.py Outdated
Comment thread modelaudit/scanners/_evidence_redaction.py Outdated

Copy link
Copy Markdown
Contributor Author

Addressed the three latest review threads and completed another adversarial false-positive/false-negative pass.

Key follow-up fixes:

  • preserve parseable Python expressions instead of source-wide auth-scheme rewriting;
  • redact Digest/SigV4 values inside ordinary, raw, triple-quoted, bytes, concatenated, and f-string literals;
  • keep shell command context at &, &&, |, and ; while treating quoted separators as auth data;
  • handle reordered/final SignedHeaders safely;
  • apply multiple AST literal replacements in descending source order;
  • keep the scanner linear on a 1 MB credential value.

Validation: 273 associated tests passed; scoped Ruff, format, mypy, and git diff --check are clean. Published exact tested tree f167cf9fdf6f47fca75ab07696e78bb93fb6c6b1 at 630fbd7a5217f3da98c0311222b9c9297be1baac. Full suite remains delegated 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: 630fbd7a52

ℹ️ 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/scanners/_evidence_redaction.py
Comment thread modelaudit/scanners/_evidence_redaction.py

mldangelo-oai commented Jun 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Final review pass published through c341dc01.

Additional adversarial QA closed three live-head edge cases:

  • parameterized Digest/SigV4 values under generic sensitive keys now fail closed, including truncated quoted parameters
  • complete Python literals redact indented and unindented multiline authorization continuations while preserving executable Python outside the literal
  • semicolon-delimited SigV4 SignedHeaders stop before recognized unspaced shell commands such as ;curl;, preserving command evidence

Validation on the published tree: 278 associated tests passed; scoped Ruff, format, mypy, and git diff --check are clean. The remote tree matches the tested local tree 83b1e562f775ff1eb95a25310067264df1aeae47 exactly. All inline review threads are resolved.

@mldangelo-oai
mldangelo-oai merged commit f3ad852 into main Jun 9, 2026
27 checks passed
@mldangelo-oai
mldangelo-oai deleted the mdangelo/codex/fix-evidence-auth-redaction-forms branch June 9, 2026 04:22

@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: c341dc0110

ℹ️ 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/scanners/_evidence_redaction.py
Comment thread modelaudit/scanners/_evidence_redaction.py
Comment thread modelaudit/scanners/_evidence_redaction.py

Copy link
Copy Markdown
Contributor Author

The three post-merge authorization-redaction findings are addressed in follow-up PR #1596 (adfc85bdee66bfdfa2d79abfe8bd28a8075fc869). It covers folded header continuations, runs Python-literal redaction before placeholder-producing passes, and applies the sensitive parameterized-scheme scanner to ordinary strings, bytes, and f-strings. Validation: 309 associated tests passed; focused post-main-sync regressions 6 passed; Ruff, format, mypy, and diff checks are clean.

Copy link
Copy Markdown
Contributor Author

The three post-merge findings are addressed in follow-up PR #1596 (adfc85bd). It continues parameterized authorization redaction across indented folded lines while preserving the following unindented context, runs Python literal redaction before placeholder-producing passes, and applies the sensitive Digest/SigV4 parameter scanner to ordinary strings, bytes, and f-strings. Exact regressions plus the full affected module pass (309 passed), with Ruff and mypy clean.

@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