Skip to content

fix(plugin): include text files in scans regardless of extension - #1347

Merged
mldangelo-oai merged 5 commits into
mainfrom
mdangelo/codex/text-inventory-coverage
Oct 7, 2026
Merged

mldangelo-oai merged 5 commits into
mainfrom
mdangelo/codex/text-inventory-coverage

Conversation

@mldangelo-oai

@mldangelo-oai mldangelo-oai commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Before reviewing code, Codex Security builds lists of files to inspect. Some of those lists currently accept only known file extensions, so they can miss files such as a script named scripts/entrypoint or code saved with an unfamiliar extension.

This change checks the file contents to decide whether a file is text. Those files can then be included without adding each extension to a maintained list.

Changes

  • Include text files with unfamiliar or no extension when choosing files for review and preparing short previews of their contents. Existing file exclusions still apply.
  • Classify complete working-tree files in bounded chunks, so files with binary bytes beyond the preview window are excluded consistently across local and committed scans. Keep previews limited to their existing read budget.
  • Check the previous contents of deleted files so deleted text is still included and deleted binary files are skipped.
  • Stream file contents from Git while retaining only short previews in memory. Drain rejected binary blobs in bounded chunks so large binary assets do not require a temporary disk copy.

Testing

After rebasing onto main 3a1a51e, all 296 focused inventory, preview, and Git snapshot tests passed. They cover extensionless and unfamiliar text, selected revisions, staged and unstaged changes, deleted text and binary files, gitlinks, bounded blob reads, UTF-16 classification, late binary bytes, and preview byte limits. Large two-way Git pipe traffic, malformed framing, partial output, and Git failures are covered. A discarded 4 MiB binary no longer prevents text inventory generation under a 1 MiB temporary-file size limit.

Ruff checks and formatting, the TypeScript CI build, and portable plugin source compatibility checks passed on this head.

Repeat the focused checks from the repository root:

python -m pytest -q plugins/codex-security/tests/test_generate_in_scope_files.py plugins/codex-security/tests/test_generate_rank_input.py plugins/codex-security/tests/test_rank_preview.py plugins/codex-security/tests/test_workbench_target.py

Risk and rollout

Repositories with previously skipped text files may have more files to review. Existing file exclusions and preview-size limits still apply. Classifying working-tree files now reads through the file until binary content or EOF, which adds I/O for large text files while keeping memory bounded. Reading file versions stored in Git uses bounded chunks and retains only preview samples. Unrecognized formats get short text samples for previews. No command-line changes or migration are required.

Public disclosure review

  • No customer, partner, prospect, or user identities, data, or identifying details are included.
  • No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
  • I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.

@github-actions github-actions Bot added the bug Something isn't working label Oct 7, 2026
@mldangelo-oai
mldangelo-oai marked this pull request as ready for review October 7, 2026 02:08
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-07T08:59:17.241321Z 3dd9a07 New commits
🔒 Security Review ✅ Completed 2026-10-07T09:01:29.816980Z 3dd9a07 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@mldangelo-oai mldangelo-oai changed the title fix(plugin): select scan text independently of file extensions fix(plugin): stop skipping text files with unknown or no extension Oct 7, 2026
@mldangelo-oai mldangelo-oai changed the title fix(plugin): stop skipping text files with unknown or no extension fix(plugin): include text files in scans regardless of extension Oct 7, 2026

@alandelong-oai alandelong-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the current diff; no actionable issues found.

@kmbroai kmbroai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the full inventory/preview change, Git blob framing, deleted-file classification, bounded reads, UTF-16 handling, path checks, and shared regression cases. Local Python validation: 286 passed; six temporary-directory-related failures reproduced on base and all six passed in clean fixtures. Hosted CI inspected. No actionable findings at this commit.

@zcrab-oai zcrab-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the diff and relevant surrounding code at 9abc6d7. No obvious correctness or regression issues found. 292 focused tests passed locally. Hosted CI remains incomplete because runs were canceled; the failed aggregate checks enforce completion of those canceled jobs.

@mldangelo-oai
mldangelo-oai force-pushed the mdangelo/codex/text-inventory-coverage branch from 9abc6d7 to 736e787 Compare October 7, 2026 08:01

@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: 736e7870fd

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

@mldangelo-oai
mldangelo-oai force-pushed the mdangelo/codex/text-inventory-coverage branch from 736e787 to 3dd9a07 Compare October 7, 2026 08:55
@mldangelo-oai
mldangelo-oai merged commit 7ac5a8b into main Oct 7, 2026
64 checks passed
@mldangelo-oai
mldangelo-oai deleted the mdangelo/codex/text-inventory-coverage branch October 7, 2026 14:42
@github-actions github-actions Bot mentioned this pull request Oct 7, 2026
3 tasks done
mldangelo-oai pushed a commit that referenced this pull request Oct 28, 2026
…es (#2109)

The extension allowlist that silently dropped `.cgi`, `.pl`, `.pm`, and `.t`
files was removed in #1347, and #1432 moved inventory generation to Node. The
move left no coverage for the reported Perl/mixed-encoding scenario: the
Python `source_cases` matrix that carried unfamiliar and extensionless sources
is no longer referenced, so a future suffix filter would regress silently.

Add a diff-inventory regression that includes `.cgi`, `.pl`, `.pm`, `.t`, an
extensionless Perl shebang script, and a file stored as EUC-JP rather than
UTF-8. A `.pm` containing NUL bytes stays excluded, pinning selection to file
content rather than suffix.

Refs #2078.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants