-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Description
Just wrote our first constitution for a JS/TS Repos and created a PR.
After putting it up, I noticed that it complained with a lot of security concerns that I'm not finding any explanation for inside the README or any other github issue. I'm not really great at bash scripting so I'm sure it makes sense to other people but for me I just need to at least ask about it.
.specify/scripts/bash/update-agent-context.sh
# Get all paths and variables from common functions
- eval $(get_feature_paths)
+ source <(get_feature_paths)The use of eval $(get_feature_paths) executes shell code that is dynamically constructed from values like CURRENT_BRANCH and SPECIFY_FEATURE without proper escaping. Because branch names or the SPECIFY_FEATURE environment variable can legally contain characters like single quotes and semicolons, a maliciously crafted branch name (e.g., 001-foo'; touch /tmp/pwned #') would cause arbitrary commands to run when this script is executed, resulting in command injection / local RCE. To fix this, avoid eval entirely by having get_feature_paths emit data in a parseable format (e.g., sourcing a file with safe assignments or JSON) or by escaping single quotes robustly, and ensure branch/environment-derived values cannot inject shell syntax into variable assignments.
.vscode/settings.json
"chat.tools.terminal.autoApprove": {
".specify/scripts/bash/": true,
".specify/scripts/powershell/": true
}The chat.tools.terminal.autoApprove setting automatically approves script execution for all scripts in .specify/scripts/bash/ and .specify/scripts/powershell/ directories without user confirmation. This is a potential security risk as it could allow malicious scripts in these directories to execute automatically.
Consider either removing this auto-approval setting or being more selective about which specific scripts are auto-approved, rather than blanket approving entire directories.
Is this just a case of co-pilot during reviews not being correct/intelligent enough to see some nuance? I feel like if I don't write asking about this, someone else eventually will.