fix(chat-bash): inherit tools.exec security settings#41598
Conversation
Greptile SummaryThis PR fixes The three new fields are correctly typed — In the chat-bash flow, The Confidence Score: 5/5
Last reviewed commit: 0ae5876 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6246b6e75b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const bashSlashRequested = | ||
| command.commandBodyNormalized === "/bash" || command.commandBodyNormalized.startsWith("/bash "); | ||
| const bashBangRequested = command.commandBodyNormalized.startsWith("!"); | ||
| const bashBangRequested = /^!(?:\s|:|[a-z0-9]|$)/i.test(command.commandBodyNormalized); |
There was a problem hiding this comment.
Preserve bang alias for punctuation-prefixed commands
The new bashBangRequested regex only treats ! as a bash alias when the next character is whitespace, :, or [a-z0-9], so valid commands like !/bin/ls, !./script.sh, or !$SHELL -lc ... are no longer routed to handleBashChatCommand. This is a regression from the previous startsWith("!") behavior and conflicts with parseBashRequest, which still supports arbitrary command text after !; users now silently lose the ! alias for a class of real shell commands unless they switch to /bash ....
Useful? React with 👍 / 👎.
|
This pull request has been automatically marked as stale due to inactivity. |
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close as superseded: the useful config-inheritance part is now carried forward by #73982, which is the active broader implementation candidate and explicitly preserves this contributor’s attribution. This PR should not be merged as-is because its bang-command router change still regresses valid punctuation-prefixed shell commands. So I’m closing this here and keeping the remaining discussion on the canonical linked item. Review detailsBest possible solution: Use #73982 as the canonical follow-up: land a focused fix that makes chat-bash inherit the same effective exec defaults as the normal exec tool, preserves broad bang-command routing, and resolves the remaining maintainer/security review questions there. Security review: Security review cleared: The PR touches host-exec policy but only forwards existing config defaults and tests command routing; it does not add dependencies, workflows, package scripts, secret handling, or new code-download/execution paths. What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 7c51cd2baf1c. |
No description provided.