🛡️ Sentinel: [HIGH] Harden publish flow against data leakage and injection

.jules/sentinel.md

@@ -2,3 +2,13 @@
 **Vulnerability:** Regex patterns using `$` instead of `\Z` allowed trailing newlines to bypass validation.
 **Learning:** In Python's `re` module, `$` matches at the end of the string OR just before a newline at the end of the string. This can be exploited to inject arguments if the validated string is passed to a shell command.
 **Prevention:** Always use `\Z` for absolute end-of-string matching in security-critical regex validators.
+
+## 2025-05-15 - Symlink Content Leakage in Publish Flow
+**Vulnerability:** `shutil.copytree` and `shutil.copy2` followed symlinks by default, copying their target content into the published repository.
+**Learning:** In security-sensitive copy operations (like publishing user content), symlinks must be preserved as links to prevent accidental disclosure of sensitive files (e.g., `/etc/passwd`) that might be linked.
+**Prevention:** Use `symlinks=True` in `shutil.copytree` and `follow_symlinks=False` in `shutil.copy2`.
+
+## 2025-05-15 - Brittle File Exclusion Logic
+**Vulnerability:** Hand-rolled string matching for file exclusions failed to catch many intended patterns (e.g., 'sessions', 'models.json').
+**Learning:** Simple string prefix/suffix checks are insufficient for robust file filtering. `fnmatch.filter` provides a standard, reliable way to apply shell-style globbing.
+**Prevention:** Use `fnmatch.filter` for file and directory name filtering against security exclusion lists.

src/agent_sync/publish/external_source.py

@@ -199,12 +199,13 @@ def _find_skills_in_repo(cache_path: Path, source: SourceConfig) -> list[SkillSo
 
 def _is_valid_skill_name(name: str) -> bool:
     """Check if skill name is valid.
-    
+
     Valid: lowercase, numbers, hyphens. No leading/trailing/consecutive hyphens.
     """
     import re
     # Match: starts with letter, then letters/numbers/hyphens, ends with letter/number
-    pattern = r'^[a-z][a-z0-9]*(-[a-z0-9]+)*$'
+    # Use \Z instead of $ to prevent newline injection
+    pattern = r'^[a-z][a-z0-9]*(-[a-z0-9]+)*\Z'
     return bool(re.match(pattern, name))
 
 

src/agent_sync/publish/git_publish.py

@@ -30,19 +30,16 @@
 
 def _ignore_func(*patterns):
     """Create a callable that returns a list of filenames to ignore."""
+    import fnmatch
+
     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
+        for pattern in patterns:
+            # Use fnmatch.filter for robust pattern matching
+            matches = fnmatch.filter(names, pattern)
+            ignored.extend(matches)
+        return list(set(ignored))
+
     return _ignore
 
 from rich.console import Console
@@ -91,11 +88,18 @@ def do_git_publish(
         for src_path, dest_name in items:
             dest = items_dir / dest_name
             if src_path.is_dir():
-                shutil.copytree(src_path, dest, dirs_exist_ok=True,
-                               ignore=_ignore_func(*DEFAULT_IGNORE_PATTERNS))
+                # Use symlinks=True to prevent content leakage of linked files
+                shutil.copytree(
+                    src_path,
+                    dest,
+                    dirs_exist_ok=True,
+                    symlinks=True,
+                    ignore=_ignore_func(*DEFAULT_IGNORE_PATTERNS),
+                )
             else:
                 dest.parent.mkdir(parents=True, exist_ok=True)
-                shutil.copy2(src_path, dest)
+                # Use follow_symlinks=False to prevent content leakage of linked files
+                shutil.copy2(src_path, dest, follow_symlinks=False)
 
         # Generate README
         readme_generator(items_dir, items, repo)
@@ -162,11 +166,18 @@ def publish_all(
                             dest_name = f"{source_id}/{skill_name}"
                             dest = skills_dir / dest_name
                             if skill.path.is_dir():
-                                shutil.copytree(skill.path, dest, dirs_exist_ok=True,
-                                               ignore=_ignore_func(*DEFAULT_IGNORE_PATTERNS))
+                                # Use symlinks=True to prevent content leakage of linked files
+                                shutil.copytree(
+                                    skill.path,
+                                    dest,
+                                    dirs_exist_ok=True,
+                                    symlinks=True,
+                                    ignore=_ignore_func(*DEFAULT_IGNORE_PATTERNS),
+                                )
                             else:
                                 dest.parent.mkdir(parents=True, exist_ok=True)
-                                shutil.copy2(skill.path, dest)
+                                # Use follow_symlinks=False to prevent content leakage of linked files
+                                shutil.copy2(skill.path, dest, follow_symlinks=False)
                             skills_to_publish.append((skill.path, dest_name))
                             break
 

src/agent_sync/publish/local_source.py

@@ -82,12 +82,13 @@ def _create_skill_source(path: Path) -> SkillSource | None:
 
 def _is_valid_skill_name(name: str) -> bool:
     """Check if skill name is valid.
-    
+
     Valid: lowercase, numbers, hyphens. No leading/trailing/consecutive hyphens.
     """
     import re
     # Match: starts with letter, then letters/numbers/hyphens, ends with letter/number
-    pattern = r'^[a-z][a-z0-9]*(-[a-z0-9]+)*$'
+    # Use \Z instead of $ to prevent newline injection
+    pattern = r'^[a-z][a-z0-9]*(-[a-z0-9]+)*\Z'
     return bool(re.match(pattern, name))