🛡️ Sentinel: Security Fixes for Publish Flow#115
Conversation
Summary: - Rewrote `_ignore_func` in `git_publish.py` to use `fnmatch.filter` for robust pattern matching, ensuring sensitive directories like `sessions` and `cache` are correctly excluded from publish. - Replaced `$` with `\Z` in skill name validation regexes in `local_source.py` and `external_source.py` to prevent newline injection bypass. Severity: MEDIUM Impact: Prevents accidental publication of private data and prevents malicious skill name injection. Verification: Verified with `pytest` and targeted reproduction scripts. Co-authored-by: renatocaliari <1719651+renatocaliari@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request improves skill name validation by updating regex patterns to use \Z instead of $ to prevent newline injection, and refactors the git ignore function to use fnmatch for robust pattern matching. A review comment suggests optimizing the ignore function by pre-compiling the patterns into regular expressions to avoid redundant checks and intermediate allocations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| def _ignore(path, names): | ||
| ignored = [] | ||
| for name in names: | ||
| for pattern in patterns: | ||
| if pattern.startswith('*.'): | ||
| if name.endswith(pattern[1:]): | ||
| ignored.append(name) | ||
| break | ||
| elif pattern.startswith('.'): | ||
| if name == pattern or name.startswith(pattern.rstrip('/') + '/'): | ||
| ignored.append(name) | ||
| break | ||
| return ignored | ||
| ignored = set() | ||
| for pattern in patterns: | ||
| # Use fnmatch for robust pattern matching (handles *, ?, [seq]) | ||
| ignored.update(fnmatch.filter(names, pattern)) | ||
| return list(ignored) |
There was a problem hiding this comment.
We can optimize the ignore function by pre-compiling the patterns into regular expressions once when _ignore_func is called, rather than repeatedly calling fnmatch.filter for every directory in the tree. This also allows us to short-circuit (break early) as soon as a filename matches any pattern, avoiding redundant checks and intermediate set/list allocations.
| def _ignore(path, names): | |
| ignored = [] | |
| for name in names: | |
| for pattern in patterns: | |
| if pattern.startswith('*.'): | |
| if name.endswith(pattern[1:]): | |
| ignored.append(name) | |
| break | |
| elif pattern.startswith('.'): | |
| if name == pattern or name.startswith(pattern.rstrip('/') + '/'): | |
| ignored.append(name) | |
| break | |
| return ignored | |
| ignored = set() | |
| for pattern in patterns: | |
| # Use fnmatch for robust pattern matching (handles *, ?, [seq]) | |
| ignored.update(fnmatch.filter(names, pattern)) | |
| return list(ignored) | |
| compiled_patterns = [re.compile(fnmatch.translate(p)) for p in patterns] | |
| def _ignore(path, names): | |
| ignored = [] | |
| for name in names: | |
| for regex in compiled_patterns: | |
| if regex.match(name): | |
| ignored.append(name) | |
| break | |
| return ignored |
Identified and fixed two security issues in the
publishflow:_ignore_funconly handled a narrow set of patterns, leading to potential data leakage of sensitive directories likesessionsorcache. Replaced it with a robustfnmatch.filterbased approach._is_valid_skill_namefunction used the$anchor, which matches trailing newlines in Python. This could be bypassed to inject arguments if the name was used in a shell context. Updated to use\Z.All 566 tests passed.
PR created automatically by Jules for task 8705118332549494391 started by @renatocaliari