fix(cli): support bulk-scan options and flexible CSV ordering - #197
fix(cli): support bulk-scan options and flexible CSV ordering#197mldangelo-oai wants to merge 10 commits into
Conversation
![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: e96d424a08
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe12bfd011
ℹ️ 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".
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
mldangelo-oai
changed the title
…-interactive-bulk-scan-options
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
ianw-oai
left a comment
ianw-oai
left a comment
There was a problem hiding this comment.
The container entrypoint and CLI disagree on option terminators.
| fi | ||
|
|
||
| case "$argument" in | ||
| --) |
There was a problem hiding this comment.
The entrypoint accepts bulk-scan --output-dir out -- repositories.csv, but the pinned incur parser rejects -- with Unknown flag: --. A filename like --help or --schema is treated as metadata instead of input. The tests miss this because their fake CLI only echoes arguments. Handle -- in the real parser and add an end-to-end test, or remove this unsupported branch.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25f4bf6ccb
ℹ️ 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".
| terminatedInput.startsWith("-") | ||
| ? `.${sep}${terminatedInput}` | ||
| : terminatedInput, | ||
| ...argv.slice(optionTerminator + 2), |
There was a problem hiding this comment.
When arguments follow the protected CSV, such as bulk-scan -- repositories.csv --output-dir results, this rewrite removes -- and appends the remaining tokens unchanged, so --output-dir becomes an active option and the scan runs instead of rejecting the extra positional arguments. This also undoes the entrypoint's guarantee that all post-terminator tokens remain positional; preserve that state or reject tokens after the single CSV input.
Useful? React with 👍 / 👎.
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d193660538
ℹ️ 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".
| optionTerminator > 0 && | ||
| argv.slice(0, optionTerminator).includes("bulk-scan") && | ||
| terminatedInput !== undefined |
There was a problem hiding this comment.
This condition matches any pre-terminator argument named bulk-scan, including a positional belonging to another command. For example, validate bulk-scan -- first second is valid because validate accepts variadic findings, but it now returns the bulk-scan-specific positional error; with one option-shaped trailing input, that input is silently rewritten instead. Identify the parsed command rather than using includes("bulk-scan") so literal validation or patch inputs do not trigger this preprocessing.
Useful? React with 👍 / 👎.
| argv = [ | ||
| ...argv.slice(0, optionTerminator), | ||
| terminatedInput.startsWith("-") | ||
| ? `.${sep}${terminatedInput}` | ||
| : terminatedInput, |
There was a problem hiding this comment.
When a value-taking option immediately precedes the terminator, this rewrite lets the protected CSV satisfy that option instead of reporting its missing value. For example, bulk-scan --model -- repositories.csv becomes bulk-scan --model repositories.csv, so repositories.csv is parsed as the model and the CLI starts interactive discovery with no CSV; the container entrypoint also admits this invocation because it records the post-terminator token as an input, bypassing its prohibition on discovery. Fresh evidence beyond the earlier trailing-argument case is that a single post-terminator token is reclassified through a pending option value, so retain the boundary or reject this arrangement.
Useful? React with 👍 / 👎.
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
3 participants
Attribution
Builds directly on #175 by @dipeshbabu and preserves the original authored commit. Thank you, Dipesh, for the interactive bulk-scan option fix.
Summary
--option boundaries so metadata-shaped CSV filenames cannot bypass Landlock, and inject mandatory sandbox options before the terminatorFixes #45.
Verification
--cannot suppress sandbox configurationpnpm run typespnpm run formatpnpm run buildpnpm pack --pack-destination /private/tmp/codex-security-followup-packagenpm_config_cache=/private/tmp/codex-security-followup-npm-cache pnpm run check:package /private/tmp/codex-security-followup-package/openai-codex-security-0.1.5.tgzshellcheck docker/entrypoint.shsh -n docker/entrypoint.shgit diff --check