Skip to content

fix(workbench): bind finding excerpts to selected source paths - #636

Open
mldangelo-oai wants to merge 22 commits into
mdangelo/codex/fix-scan-budget-attributionfrom
mdangelo/codex/fix-finding-excerpt-scope-clean
Open

fix(workbench): bind finding excerpts to selected source paths#636
mldangelo-oai wants to merge 22 commits into
mdangelo/codex/fix-scan-budget-attributionfrom
mdangelo/codex/fix-finding-excerpt-scope-clean

Conversation

@mldangelo-oai

Copy link
Copy Markdown
Collaborator

Summary

Bind optional finding source excerpts to the source paths and exact Git tree selected when a scan starts. Ambiguous normalized paths, linked source ancestry, and replacement-object views now fail closed by omitting the excerpt.

Changes

  • Persist selected source paths and kinds for new scans with append-only migration 33, capturing authority before writer transactions and revalidating Deep Scan targets before persistence.
  • Resolve authorized paths through the captured target tree, require normalized-name uniqueness before exact-name selection, reject linked or reparse-point ancestry, and read validated blob object IDs directly.
  • Detect replacement refs under the validated active GIT_REPLACE_REF_BASE with a bounded one-ref probe, including custom and raw-prefix bases, while retaining fail-closed behavior for malformed bases and Git errors.
  • Build one response-local source index and omit excerpts for legacy, malformed, mutable, or changed scan targets.
  • Advance the bundled plugin identity from 0.1.27 to 0.1.28 so installed caches receive the updated scripts.

Testing

  • Five focused test files: 86 passed, 0 failed, 370 expectations.
  • Full TypeScript package suite with Node 24.14.0, pnpm 11.9.0, Bun 1.3.14, umask 022, system /tmp, and task-scoped caches: 1,785 passed, 28 skipped, 0 failed, 23,582 expectations across 1,813 tests in 94 files.
  • corepack pnpm run generate:models:check
  • corepack pnpm exec tsc --noEmit
  • corepack pnpm run format
  • Recursive Python syntax parsing for all 35 bundled plugin files.
  • git diff --check
  • Real Git replacement matrix for default, custom, raw-prefix, option-like, and malformed bases. A 10,000-warning probe read one combined-output byte, killed and reaped the child, and kept traced parent memory below 68 KiB.
  • corepack pnpm run build
  • corepack pnpm pack
  • corepack pnpm run check:package -- openai-codex-security-0.1.18.tgz: validated 265 entries, public import, NodeNext types, CLI, 113 bundled plugin files, bundled Codex, and a nested worker without global Codex. Tarball SHA-256: de9b7d2d213114110823cd0aac048ff0a19e3042885ba016df23faf8b1f882d1.
  • Four exact-head correctness, security, simplification, and public-disclosure reviews found no actionable issues. Three fresh native review passes plus independent normalization produced zero candidates and a clean high-risk prepublish receipt.

Risk and rollout

The migration only adds nullable metadata. Existing scans continue to work and omit the optional excerpt when authority is absent. Replacement-ref inspection is fail closed, so malformed or unsupported Git state can suppress an optional excerpt but cannot change scan results. The bundled plugin version change refreshes installed caches; no separate rollout step is 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.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 24, 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-08-24T19:08:01.624690Z 566f5c4 Manual request
🔒 Security Review Completed 2026-08-24T19:08:16.145328Z 566f5c4 PR opened
ℹ️ 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.

@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: 566f5c4de7

ℹ️ 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 on lines +153 to +154
decoded_name = read_field(0, name_bytes_limit).decode(
sys.getfilesystemencoding(), errors="surrogateescape"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Stop applying checkout name limits to Git tree entries

When a commit/range scan's selected tree contains any sibling entry longer than the current filesystem's PC_NAME_MAX, read_field raises while resolving an otherwise ordinary finding path, so the exception is caught and source excerpts through that directory silently disappear. Git accepts such immutable tree objects even when the checkout filesystem cannot represent their names; stream or skip unrelated names instead of imposing the checkout limit on the tree data.

AGENTS.md reference: sdk/typescript/AGENTS.md:L22-L24

Useful? React with 👍 / 👎.

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