diff --git a/codex-rs/skills/src/assets/samples/skill-installer/scripts/install-skill-from-github.py b/codex-rs/skills/src/assets/samples/skill-installer/scripts/install-skill-from-github.py index 1c8ce89d0a..b7b8e6e8d4 100755 --- a/codex-rs/skills/src/assets/samples/skill-installer/scripts/install-skill-from-github.py +++ b/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: diff --git a/codex-rs/skills/tests/test_skill_installer.py b/codex-rs/skills/tests/test_skill_installer.py new file mode 100644 index 0000000000..0265273eb8 --- /dev/null +++ b/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()