feat(cli): run standalone validation after scan - #1007
mldangelo-oai wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5943f48930
ℹ️ 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".
| ...providerOptions, | ||
| safetyIdentifier: arguments_.safetyIdentifier, | ||
| auth, | ||
| directory: result.scanDir, |
There was a problem hiding this comment.
For every nonempty --validate scan, passing result.scanDir as directory makes runSkill launch Codex with the entire completed scan directory as its workspace-write root. A validator influenced by repository/finding content can therefore overwrite findings.json, coverage.json, scan-manifest.json, or other sealed artifacts; because scanData was captured before validation and the seal is not rechecked afterward, the command can report success while leaving scan history and later exports corrupted. Run validation in a separate writable workspace or verify/protect every sealed artifact across the model call.
AGENTS.md reference: sdk/typescript/AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
| safetyIdentifier: arguments_.safetyIdentifier, | ||
| auth, | ||
| directory: result.scanDir, | ||
| validationRepository: repository, |
There was a problem hiding this comment.
If the checkout changes after security.run() returns or while the second model call is running, validation reads the live repository even though the scan's target observer has already been closed and targetWarnings can no longer be updated. The resulting report is then marked complete against code that may differ from the code which produced the supplied findings. Preserve the scanned snapshot for this pass or recheck the target digest before and after validation.
AGENTS.md reference: sdk/typescript/AGENTS.md:L23-L23
Useful? React with 👍 / 👎.
| ), | ||
| ).toBe(0); | ||
| expect(validationPrompt).toContain("Example finding"); | ||
| expect(validationPrompt).toContain("Leave the repository unchanged."); |
There was a problem hiding this comment.
This assertion makes the test depend on one exact English sentence in the generated prompt, so a harmless wording change breaks the suite without changing observable validation behavior. Assert a stable contract or behavioral boundary instead of the Markdown prose, as required by the scoped test guidance.
AGENTS.md reference: sdk/typescript/AGENTS.md:L35-L35
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Summary
Add opt-in
scan --validateto run the existing standalone finding validation workflow after a completed scan. The separatevalidatecommand remains available.Changes
validation.md, include its status and path in JSON output, and retain the completed scan if validation fails.Testing
main(162 tests). The full SDK suite passed with both fixed and random seeds (3,179 passed, 50 skipped, 0 failed).--validate; the incompatible dry-run combination returned the expected error.Risk and rollout
The new pass is opt-in and makes additional model calls. It does not rewrite sealed scan findings or change the severity failure policy. Validation failure exits with status 2 while preserving the completed scan. No migration is required.
Public disclosure review