-
Notifications
You must be signed in to change notification settings - Fork 0
Add pykernel support; add testing suite; overhaul documentation #5
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
Conversation
- Updated script and documentation to transition from library to kernel management. - Renamed variables and functions to reflect kernel terminology. - Adjusted user commands and error messages to align with new kernel structure. - Enhanced documentation to guide users on managing R and Python kernels. - Introduced a new hierarchical catalog structure for kernels in the central repository. - Updated Docker setup and workflows to accommodate kernel management features.
- Improved error messages for kernel management commands, providing clearer guidance on usage and parameter requirements. - Updated the `kernels use` command to support a "none" option for both R and Python, with appropriate messaging for each language. - Enhanced the `update_r_libs.sh` script to validate file paths and permissions, ensuring robust error handling. - Expanded test coverage for kernel commands, including new tests for handling "none" parameters and improved validation checks. - Updated documentation to reflect changes in error handling and testing procedures.
- Added functionality for removing and using Python kernels, including checks for installed kernels and user catalog entries. - Enhanced the `kernels use` command to support Python, allowing for installation and activation of Python kernels in Jupyter. - Implemented error handling for invalid characters in kernel names and versions to prevent security vulnerabilities. - Updated documentation to include examples for Python kernel usage and management. - Expanded test coverage for Python kernel operations, ensuring robust validation and error handling.
- corrected exdisting kernel discovery by using complete matches
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.
Bugbot free trial expires on August 11, 2025
Learn more in the Cursor dashboard.
| echo "Error: Invalid characters in kernel name or version. Cannot contain wildcards (*?[]) or path separators (/)" | ||
| exit 1 | ||
| fi | ||
|
|
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.
Bug: Regex Validation Flaws Expose Security Risks
The input validation regex patterns in kernels__get() and kernels__remove() are malformed and inconsistent, creating security vulnerabilities.
- Malformed Syntax: The character class
[\]\/\[\*\?\]is syntactically problematic, potentially leading to unpredictable validation behavior. - Inconsistent Backslash Validation: The
kernels__remove()function's regex[\]\/\[\*\?\]is missing the literal backslash (\\) validation present inkernels__get(). This allows backslashes, which could be used for path traversal attacks, to bypass validation inremove. - Missing Space Validation: The regex patterns in both functions do not reject spaces, which is inconsistent with expected security behavior and could lead to shell parsing issues.
Locations (1)
| ICRN_LIBRARY_REPOSITORY=$(jq -r ".\"icrn_central_catalog_path\"" "${ICRN_MANAGER_CONFIG}") | ||
| ICRN_LIBRARIES=${ICRN_LIBRARY_REPOSITORY}"/"$(jq -r ".\"icrn_r_kernels\"" "${ICRN_MANAGER_CONFIG}") | ||
| ICRN_LIBRARY_CATALOG=${ICRN_LIBRARIES}"/"$(jq -r ".\"icrn_kernel_catalog\"" "${ICRN_MANAGER_CONFIG}") | ||
| ICRN_KERNEL_REPOSITORY=$(jq -r ".\"icrn_central_catalog_path\"" "${ICRN_MANAGER_CONFIG}") |
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.
Bug: Bash Condition Fails to Check Variable Value
The bash conditional if [ -n overwrite ]; then is missing the variable dereference operator ($) and quotes. It should be if [ -n "$overwrite" ]; then. This causes the condition to always evaluate to true, as it checks the literal string "overwrite" instead of the variable's value. Consequently, the central catalog path is always updated in the manager configuration, regardless of whether an overwrite was intended.
Locations (1)
| if [[ "$targetname" =~ [\]\/\[\*\?\\] ]] || [[ "$version" =~ [\]\/\[\*\?\\] ]]; then | ||
| echo "Error: Invalid characters in kernel name or version. Cannot contain wildcards (*?[]) or path separators (/)" | ||
| exit 1 | ||
| fi |
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.
Bug: Malformed Regex Causes Security Vulnerability
The kernels__remove function's input validation regex ([\]\/\[\*\?\]) is malformed, as the final backslash escapes the closing bracket, preventing the character class from properly closing. This regex also inconsistently omits backslash validation (\\) compared to the kernels__get function's regex ([\]\/\[\*\?\\]). This creates a security vulnerability where backslash-based path traversal or escape sequence attacks could bypass validation in kernels__remove.
==========================================
Test Summary
Total Tests: 45
Passed: 42
Failed: 3
Skipped: 0
✗ FAIL: 3 test(s) failed
Failed Tests: