🛡️ Sentinel: [HIGH] Fix regex newline injection and improve publish exclusion logic#107
🛡️ Sentinel: [HIGH] Fix regex newline injection and improve publish exclusion logic#107calionauta wants to merge 1 commit into
Conversation
…xclusion logic This PR addresses two security/robustness issues: 1. Regex newline injection: Replaced vulnerable `$` anchors with `\Z` in skill name validation to prevent trailing newline bypasses. 2. Publish exclusion logic: Refactored `_ignore_func` to use `fnmatch.fnmatchcase`, ensuring that sensitive directories like `sessions` and files like `models.json` are correctly excluded from public publishing. Verified with reproduction scripts and existing test suites. 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. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves security and reliability by updating the regex pattern in _is_valid_skill_name to use \Z instead of $ to prevent trailing newline bypasses, and refactoring the file exclusion logic in git_publish.py to use fnmatch.fnmatchcase. The review feedback suggests enhancing the file exclusion logic to be case-insensitive to prevent bypasses via case variations, and centralizing the duplicated _is_valid_skill_name function into a shared module to improve maintainability.
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.
| if name == pattern or name.startswith(pattern.rstrip('/') + '/'): | ||
| ignored.append(name) | ||
| break | ||
| if fnmatch.fnmatchcase(name, pattern): |
There was a problem hiding this comment.
Using fnmatch.fnmatchcase performs a case-sensitive match. If a user has sensitive files or directories with different casing (e.g., .ENV, Models.json, or SESSIONS), they will bypass this ignore filter and potentially be published to a public repository.
To prevent accidental leakage of sensitive data due to case variations, it is highly recommended to perform case-insensitive matching by converting both the filename and the pattern to lowercase.
| if fnmatch.fnmatchcase(name, pattern): | |
| if fnmatch.fnmatchcase(name.lower(), pattern.lower()): |
| import re | ||
| # Match: starts with letter, then letters/numbers/hyphens, ends with letter/number | ||
| pattern = r'^[a-z][a-z0-9]*(-[a-z0-9]+)*$' | ||
| pattern = r'^[a-z][a-z0-9]*(-[a-z0-9]+)*\Z' |
There was a problem hiding this comment.
The _is_valid_skill_name function is duplicated exactly between external_source.py (line 200) and local_source.py (line 83). To improve maintainability and ensure that any future updates to the skill name validation logic are applied consistently, consider centralizing this function in src/agent_sync/validators.py (for example, as validate_publish_skill_name) and importing it in both files.
1 participant
🛡️ Sentinel: [HIGH] Fix regex newline injection and improve publish exclusion logic
🚨 Severity: HIGH
💡 Vulnerability:
$anchor, which in Python matches before a trailing newline. This could allow malicious skill names to pass validation and potentially lead to argument or path injection if used in shell/file operations._ignore_funcused during thepublishflow had a brittle implementation that failed to match exact directory names or filenames (likesessionsormodels.json), potentially leading to sensitive data leakage in public repositories.🎯 Impact:
🔧 Fix:
$with\Zin all regex patterns inlocal_source.pyandexternal_source.py.git_publish.py's_ignore_functo usefnmatch.fnmatchcasefor robust pattern matching againstDEFAULT_IGNORE_PATTERNS.✅ Verification:
skill-name\nis now rejected.sessionsandmodels.jsonare now correctly ignored by the publish filter.pyteston relevant security and publish tests; all passed.PR created automatically by Jules for task 4670584271098205781 started by @renatocaliari