Skip to content

🛡️ Sentinel: Security Fixes for Publish Flow#115

Open
calionauta wants to merge 1 commit into
mainfrom
fix/security-enhancements-publish-8705118332549494391
Open

🛡️ Sentinel: Security Fixes for Publish Flow#115
calionauta wants to merge 1 commit into
mainfrom
fix/security-enhancements-publish-8705118332549494391

Conversation

@calionauta

Copy link
Copy Markdown
Owner

Identified and fixed two security issues in the publish flow:

  1. Brittle File Exclusion: The previous implementation of _ignore_func only handled a narrow set of patterns, leading to potential data leakage of sensitive directories like sessions or cache. Replaced it with a robust fnmatch.filter based approach.
  2. Regex Newline Injection: The _is_valid_skill_name function 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

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>
@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +34 to +40

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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant