Release/v2.0.0

[WMC001]

No description provided.

In general, to fix uncontrolled-path issues you must (a) strictly validate user-controlled components used in paths (e.g., skill_name), and (b) ensure that any final path derived from user input is normalized and checked to be contained within an expected root directory before use. This prevents directory traversal (..), absolute paths, or other manipulations from writing outside the intended storage area.

For this code, the best minimal fix is:

1. Introduce a helper in backend/services/skill_service.py to validate and normalize skill_name when used as a directory name. This helper should:

   • Reject empty names.
   • Reject names containing path separators (/ or \), drive letters, .., or starting with . or a separator.
   • Optionally enforce a simple regex (e.g., letters, digits, _, -) to preserve current semantics while blocking traversal.

2. Introduce another helper to safely join any relative path under a base directory:

   • Use os.path.join followed by os.path.normpath.
   • Compute the absolute path of both base and candidate.
   • Ensure that the candidate path starts with the base path followed by a separator (or equal), otherwise raise an exception.

3. Apply these helpers within _upload_zip_files:

   • Before using skill_name, call the validation helper and use the sanitized version.
   • When computing relative_path from ZIP entries, use posixpath operations to normalize them (ZIP uses /), remove any leading /, and eliminate ./.. segments; skip entries that resolve outside the intended tree.
   • Replace the plain os.path.join calls for local_dir and local_path with calls that use the secure join helper, rooting everything under self.skill_manager.local_skills_dir. This addresses both tainted skill_name and any malicious relative_path from the ZIP archive.

These changes are all within backend/services/skill_service.py, in the existing helper section (near _normalize_zip_entry_path) and the _upload_zip_files method. You do not need to modify imports beyond adding posixpath and possibly re / os.path usage that is already present.

---

To fix the problem, we need to ensure that any filesystem path derived from skill_name remains under a trusted root directory (self.skill_manager.local_skills_dir) and does not allow traversal or absolute-path tricks. The standard pattern is:

1. Join the user-provided component with the base directory.
2. Normalize / resolve the resulting path (e.g., os.path.realpath or os.path.normpath).
3. Verify that the normalized path is still within the base dir (e.g., using os.path.commonpath or a prefix check after normalization).
4. Only then perform filesystem operations, otherwise raise an error.

In backend/services/skill_service.py, inside delete_skill, we should:

• Compute base_dir = os.path.realpath(self.skill_manager.local_skills_dir).
• Compute candidate_dir = os.path.realpath(os.path.join(base_dir, skill_name)).
• Use os.path.commonpath([base_dir, candidate_dir]) == base_dir to verify containment.
• If the containment check fails, log an error and raise SkillException("Invalid skill name") instead of deleting anything.
• Use candidate_dir (renamed e.g. to skill_dir) for existence check and deletion.

This keeps existing functionality (deleting the corresponding local directory when given a valid skill name) while preventing path traversal or arbitrary directory deletion. No new external dependencies are required; os.path already provides what we need.

---

In general, the fix is to ensure that any user‑supplied component used in a filesystem path is validated and constrained so it cannot escape a designated root directory. For this case, we need to ensure that skill_name cannot cause skill_dir to point outside self.skill_manager.local_skills_dir and ideally restrict skill_name to a simple, safe identifier (e.g., no slashes, no .., no leading . etc.).

The least intrusive, backward‑compatible fix is to normalize the joined path and verify that the resulting real path is still inside self.skill_manager.local_skills_dir. If it is not, we should abort with a controlled exception instead of calling shutil.rmtree. Concretely, in SkillService.delete_skill in backend/services/skill_service.py, we will:

1. Compute base_dir = os.path.realpath(self.skill_manager.local_skills_dir).
2. Compute skill_dir = os.path.realpath(os.path.join(base_dir, skill_name)).
3. Check that os.path.commonpath([base_dir, skill_dir]) == base_dir. If not, raise SkillException("Invalid skill name") (or similar) before any filesystem deletion.
4. Proceed to shutil.rmtree(skill_dir) only if the check passes.

We already import os and SkillException at the top of skill_service.py, and shutil is imported locally in the function, so no new imports are strictly necessary beyond using existing ones. All changes are confined to the delete_skill method in backend/services/skill_service.py around lines 1140–1146.

In general, to fix uncontrolled-path issues you should: (1) define a trusted root directory, (2) construct the candidate path, (3) normalize/resolve it (e.g., with os.path.realpath or os.path.normpath), and (4) verify that the normalized path is still within the trusted root (e.g., with os.path.commonpath or a robust prefix comparison). Only then should you perform filesystem operations on it.

For this specific code, the best fix is to add a small helper inside SkillService to safely resolve a skill file path and then use it in get_skill_file_content (and, by extension, any future methods like delete/update that manipulate skill files) so that both skill_name and file_path cannot escape self.skill_manager.local_skills_dir. We will:

1. Add a private method _resolve_skill_file_path(self, skill_name: str, file_path: str) -> str near the other helper methods in SkillService. This method will:

   • Build local_dir = os.path.join(self.skill_manager.local_skills_dir, skill_name).
   • Compute root = os.path.realpath(local_dir) and candidate = os.path.realpath(os.path.join(local_dir, file_path)).
   • Use os.path.commonpath([root, candidate]) == root to ensure candidate is inside root. If not, raise SkillException("Invalid file path").
   • Return candidate.

2. Update get_skill_file_content to use this method instead of directly joining the paths:

   • Replace:

     local_dir = os.path.join(self.skill_manager.local_skills_dir, skill_name)
     full_path = os.path.join(local_dir, file_path)

     with:

     full_path = self._resolve_skill_file_path(skill_name, file_path)

   • Keep the existing existence check and file reading logic unchanged.

No new imports are required: os and SkillException are already imported in backend/services/skill_service.py. The changes are confined to this file and specifically inside the shown snippets.

---

In general, to fix this kind of issue you must ensure that any path derived from user input is constrained to stay within a known safe root. The usual pattern is: (1) construct a candidate path under a trusted base directory, (2) normalize it with os.path.abspath/os.path.realpath or os.path.normpath, and (3) verify that the normalized path is still within the intended base directory, rejecting it otherwise. You can also separately validate the components (such as skill_name) against an allowlist (e.g. only alphanumeric, dashes, underscores) to rule out weird path characters.

For this code, the best minimal fix without changing existing functionality is:

1. In SkillService.get_skill_file_content, compute a base directory for the skill (local_dir) as before, but then:
   • Resolve both local_dir and the final full_path to absolute, normalized paths using os.path.abspath.
   • Check that full_path is still inside local_dir. We can do this safely with os.path.commonpath([local_dir, full_path]) == local_dir.
   • If the check fails, log a warning and raise a SkillException (or return None) indicating an invalid file path, instead of attempting to open the file.
2. Also ensure file_path is treated as a relative path (it might start with /); we can normalize and strip leading separators before joining, or simply rely on the commonpath check to block absolute paths that escape the base.
3. Keep the rest of the behavior the same: if the file does not exist after validation, log and return None; if it exists and is permitted, open and read it as before.

This change all happens inside backend/services/skill_service.py in the get_skill_file_content method, between the try: and the open(...) call. No changes are needed in backend/apps/skill_app.py for this specific vulnerability. We do not need extra imports because os.path and os are already imported at the top of the file.