Harden skill installation against unsafe symlinks (#39608)

## What changed

- Require selected skill paths to resolve within the cloned repository and reject
  symlinked path components.
- Reject special files and symlinks that escape the selected skill or resolve to
  anything other than a regular file.
- Add regression tests showing that escaping symlinks are rejected while links
  to regular files within the skill are installed as file contents.

GitOrigin-RevId: d4c3f09fb630dea96b34ea5552de571b36f1ceca

codex-rs/skills/src/assets/samples/skill-installer/scripts/install-skill-from-github.py

@@ -161,13 +161,49 @@ def _git_sparse_checkout(repo_url: str, ref: str, paths: list[str], dest_dir: st
     return repo_dir
 
 
-def _validate_skill(path: str) -> None:
+def _validate_skill(path: str, repo_root: str) -> None:
+    resolved_repo_root = os.path.realpath(repo_root)
+    resolved_path = os.path.realpath(path)
+    try:
+        inside_repo = os.path.commonpath([resolved_repo_root, resolved_path]) == resolved_repo_root
+    except ValueError:
+        inside_repo = False
+    if not inside_repo:
+        raise InstallError("Skill path must be inside the repo.")
+
+    relative_path = os.path.relpath(path, repo_root)
+    current_path = repo_root
+    for component in relative_path.split(os.path.sep):
+        current_path = os.path.join(current_path, component)
+        if os.path.islink(current_path):
+            raise InstallError(
+                f"Symbolic links are not allowed in skills: {os.path.relpath(current_path, repo_root)}"
+            )
+
     if not os.path.isdir(path):
         raise InstallError(f"Skill path not found: {path}")
     skill_md = os.path.join(path, "SKILL.md")
     if not os.path.isfile(skill_md):
         raise InstallError("SKILL.md not found in selected skill directory.")
 
+    for root, directories, files in os.walk(path):
+        for name in directories + files:
+            entry_path = os.path.join(root, name)
+            relative_entry = os.path.relpath(entry_path, repo_root)
+            if os.path.islink(entry_path):
+                resolved_entry = os.path.realpath(entry_path)
+                try:
+                    inside_skill = (
+                        os.path.commonpath([resolved_path, resolved_entry]) == resolved_path
+                    )
+                except ValueError:
+                    inside_skill = False
+                if not inside_skill or not os.path.isfile(resolved_entry):
+                    raise InstallError(f"Unsupported symbolic link in skill: {relative_entry}")
+                continue
+            if not os.path.isdir(entry_path) and not os.path.isfile(entry_path):
+                raise InstallError(f"Unsupported file type in skill: {relative_entry}")
+
 
 def _copy_skill(src: str, dest_dir: str) -> None:
     os.makedirs(os.path.dirname(dest_dir), exist_ok=True)
@@ -290,7 +326,7 @@ def main(argv: list[str]) -> int:
                 if os.path.exists(dest_dir):
                     raise InstallError(f"Destination already exists: {dest_dir}")
                 skill_src = os.path.join(repo_root, path)
-                _validate_skill(skill_src)
+                _validate_skill(skill_src, repo_root)
                 _copy_skill(skill_src, dest_dir)
                 installed.append((skill_name, dest_dir))
         finally:

codex-rs/skills/tests/test_skill_installer.py

@@ -0,0 +1,137 @@
+"""Local-only regression tests for the bundled skill-installer script.
+
+Run manually; these tests are not wired into CI:
+    python3 -B -m unittest discover -s codex-rs/skills/tests -p 'test_*.py'
+"""
+
+import os
+from pathlib import Path
+import subprocess
+import sys
+import tempfile
+import unittest
+
+
+INSTALLER = (
+    Path(__file__).resolve().parents[1]
+    / "src"
+    / "assets"
+    / "samples"
+    / "skill-installer"
+    / "scripts"
+    / "install-skill-from-github.py"
+)
+
+
+class SkillInstallerSymlinkTests(unittest.TestCase):
+    def setUp(self) -> None:
+        self.directory = tempfile.TemporaryDirectory()
+        self.addCleanup(self.directory.cleanup)
+        self.root = Path(self.directory.name)
+        self.repository = self.root / "repository"
+        self.skill = self.repository / "skill"
+        self.skill.mkdir(parents=True)
+        (self.skill / "SKILL.md").write_text("Synthetic test skill\n", encoding="utf-8")
+        self.destination = self.root / "installed"
+
+    def create_symlink(self, link: Path, target: Path | str) -> None:
+        try:
+            link.symlink_to(target)
+        except OSError as error:
+            self.skipTest(f"symlinks are unavailable: {error}")
+
+    def run_installer(self) -> subprocess.CompletedProcess[str]:
+        subprocess.run(
+            ["git", "init", "--initial-branch=main", str(self.repository)],
+            check=True,
+            capture_output=True,
+            text=True,
+        )
+        subprocess.run(
+            ["git", "-C", str(self.repository), "add", "."],
+            check=True,
+            capture_output=True,
+            text=True,
+        )
+        subprocess.run(
+            [
+                "git",
+                "-C",
+                str(self.repository),
+                "-c",
+                "user.name=Skill Installer Test",
+                "-c",
+                "user.email=skill-installer@example.invalid",
+                "commit",
+                "-m",
+                "synthetic skill fixture",
+            ],
+            check=True,
+            capture_output=True,
+            text=True,
+        )
+
+        environment = os.environ.copy()
+        python_path = [str(INSTALLER.parent)]
+        if environment.get("PYTHONPATH"):
+            python_path.append(environment["PYTHONPATH"])
+        environment.update(
+            {
+                "GIT_CONFIG_COUNT": "2",
+                "GIT_CONFIG_KEY_0": f"url.{self.repository.as_uri()}.insteadOf",
+                "GIT_CONFIG_VALUE_0": "https://github.com/synthetic/fixture.git",
+                "GIT_CONFIG_KEY_1": "core.symlinks",
+                "GIT_CONFIG_VALUE_1": "true",
+                "GIT_TERMINAL_PROMPT": "0",
+                "PYTHONPATH": os.pathsep.join(python_path),
+            }
+        )
+        return subprocess.run(
+            [
+                sys.executable,
+                "-B",
+                str(INSTALLER),
+                "--repo",
+                "synthetic/fixture",
+                "--path",
+                "skill",
+                "--method",
+                "git",
+                "--dest",
+                str(self.destination),
+            ],
+            capture_output=True,
+            text=True,
+            check=False,
+            env=environment,
+        )
+
+    def test_rejects_symlink_to_file_outside_skill(self) -> None:
+        outside_file = self.root / "synthetic-secret.txt"
+        outside_file.write_text("synthetic secret\n", encoding="utf-8")
+        self.create_symlink(self.skill / "outside.txt", outside_file)
+
+        result = self.run_installer()
+
+        self.assertNotEqual(result.returncode, 0)
+        self.assertIn("Unsupported symbolic link", result.stderr)
+        self.assertFalse((self.destination / "skill").exists())
+
+    def test_installs_symlink_to_regular_file_inside_skill(self) -> None:
+        (self.skill / "actual.txt").write_text(
+            "safe skill contents\n", encoding="utf-8"
+        )
+        self.create_symlink(self.skill / "alias.txt", "actual.txt")
+
+        result = self.run_installer()
+
+        self.assertEqual(result.returncode, 0, result.stderr)
+        installed_alias = self.destination / "skill" / "alias.txt"
+        self.assertFalse(installed_alias.is_symlink())
+        self.assertEqual(
+            installed_alias.read_text(encoding="utf-8"), "safe skill contents\n"
+        )
+
+
+if __name__ == "__main__":
+    unittest.main()