-
Notifications
You must be signed in to change notification settings - Fork 1
π‘οΈ Sentinel: [MEDIUM] Fix shell injection vulnerability in fix_env.py #112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,12 +11,18 @@ | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # Helper to clean quotes (curly or straight) | ||||||||||||||||||||||||||||||||
| def clean_val(val): | ||||||||||||||||||||||||||||||||
| if not val: return "" | ||||||||||||||||||||||||||||||||
Check warningCode scanning / Pylintpython3 (reported by Codacy) More than one statement on a single line Warning
More than one statement on a single line
|
||||||||||||||||||||||||||||||||
| # Remove surrounding quotes of any kind | ||||||||||||||||||||||||||||||||
| val = val.strip() | ||||||||||||||||||||||||||||||||
| val = re.sub(r'^[\"\u201c\u201d\']|[\"\u201c\u201d\']$', '', val) | ||||||||||||||||||||||||||||||||
| return val | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # Helper to escape value for shell | ||||||||||||||||||||||||||||||||
| def escape_val(val): | ||||||||||||||||||||||||||||||||
| if not val: return "" | ||||||||||||||||||||||||||||||||
Check warningCode scanning / Pylint (reported by Codacy) More than one statement on a single line Warning
More than one statement on a single line
|
||||||||||||||||||||||||||||||||
| # Escape backslashes first, then double quotes | ||||||||||||||||||||||||||||||||
| return val.replace('\\', '\\\\').replace('"', '\\"') | ||||||||||||||||||||||||||||||||
|
Comment on lines
+23
to
+24
|
||||||||||||||||||||||||||||||||
| # Escape backslashes first, then double quotes | |
| return val.replace('\\', '\\\\').replace('"', '\\"') | |
| # Escape backslashes first, then double quotes, dollar signs, and backticks | |
| return ( | |
| val.replace('\\', '\\\\') | |
| .replace('"', '\\"') | |
| .replace('$', '\\$') | |
| .replace('`', '\\`') | |
| ) |
Copilot
AI
Jan 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The escape_val function and its security-critical escaping logic lack test coverage. Given that this fix addresses a shell injection vulnerability, it would be valuable to add tests that verify the escaping works correctly for various edge cases including values with double quotes, backslashes, dollar signs, backticks, and newlines. Consider adding a test file like test_fix_env.py with test cases for the escape_val function.
Copilot
AI
Jan 18, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The escape_val function does not handle newline characters or other control characters. If real_token or real_profiles contain newlines (e.g., from malicious input or data corruption), the generated .env file will be malformed. Consider escaping newlines as \n, carriage returns as \r, and tabs as \t to ensure the .env file remains valid.
| # Escape backslashes first, then double quotes | |
| return val.replace('\\', '\\\\').replace('"', '\\"') | |
| # Escape backslashes first, then control characters, then double quotes | |
| val = val.replace('\\', '\\\\') | |
| val = val.replace('\n', '\\n').replace('\r', '\\r').replace('\t', '\\t') | |
| return val.replace('"', '\\"') |
Check warning
Code scanning / Pylint (reported by Codacy)
Missing function docstring Warning