Skip to content

Commit c425f4f

Browse files
meymchenclaude
andcommitted
fix(kimi): reject symlinked target dir during legacy skills migration
When the migration destination already exists, guard against a symlinked (or non-directory) target_dir before comparing SKILL.md bytes, so the comparison never follows a link outside the project root. Also skip a missing/non-file target SKILL.md explicitly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3df99c4 commit c425f4f

2 files changed

Lines changed: 47 additions & 15 deletions

File tree

src/specify_cli/integrations/kimi/__init__.py

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -276,23 +276,25 @@ def _migrate_legacy_kimi_skills_dir(
276276
continue
277277

278278
# Target exists — only remove legacy if SKILL.md is identical.
279-
# Skip when the target SKILL.md is a symlink so the byte comparison
280-
# never follows it outside the project. (legacy_skill is already
281-
# guaranteed to be a real file by the guard above.)
279+
# Skip when the target dir or its SKILL.md is a symlink (or the dir is
280+
# not a real directory) so the byte comparison never follows a link
281+
# outside the project. (legacy_skill is already guaranteed to be a real
282+
# file by the guard above.)
283+
if target_dir.is_symlink() or not target_dir.is_dir():
284+
continue
282285
target_skill = target_dir / "SKILL.md"
283-
if target_skill.is_symlink():
286+
if target_skill.is_symlink() or not target_skill.is_file():
284287
continue
285-
if target_skill.is_file():
286-
try:
287-
if target_skill.read_bytes() == legacy_skill.read_bytes():
288-
has_extra = any(
289-
child.name != "SKILL.md" for child in legacy_dir.iterdir()
290-
)
291-
if not has_extra:
292-
shutil.rmtree(legacy_dir)
293-
removed_count += 1
294-
except OSError:
295-
pass
288+
try:
289+
if target_skill.read_bytes() == legacy_skill.read_bytes():
290+
has_extra = any(
291+
child.name != "SKILL.md" for child in legacy_dir.iterdir()
292+
)
293+
if not has_extra:
294+
shutil.rmtree(legacy_dir)
295+
removed_count += 1
296+
except OSError:
297+
pass
296298

297299
# Remove the legacy skills directory if it is now empty.
298300
try:

tests/integrations/test_integration_kimi.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from specify_cli.integrations.kimi import (
99
_migrate_legacy_kimi_context_file,
1010
_migrate_legacy_kimi_dotted_skills,
11+
_migrate_legacy_kimi_skills_dir,
1112
)
1213
from specify_cli.integrations.manifest import IntegrationManifest
1314

@@ -492,6 +493,35 @@ def test_setup_rejects_symlinked_destination_before_writing(self, tmp_path):
492493
# Nothing was written into the unintended `./skills` location.
493494
assert not (project / "skills").exists()
494495

496+
def test_migrate_skips_symlinked_target_dir(self, tmp_path):
497+
# The destination `.kimi-code/skills/speckit-foo` already exists but is
498+
# a symlink to a directory outside the project. Migration compares
499+
# SKILL.md bytes to decide whether to drop the legacy copy; it must not
500+
# follow the symlinked target dir to read SKILL.md from outside.
501+
outside = tmp_path / "outside"
502+
outside.mkdir()
503+
(outside / "SKILL.md").write_text("# shared\n")
504+
505+
project = tmp_path / "project"
506+
legacy = project / ".kimi" / "skills" / "speckit-foo"
507+
legacy.mkdir(parents=True)
508+
# Identical bytes: without the symlink guard the legacy dir would be
509+
# removed after following the link out of the project.
510+
(legacy / "SKILL.md").write_text("# shared\n")
511+
512+
target = project / ".kimi-code" / "skills" / "speckit-foo"
513+
target.parent.mkdir(parents=True)
514+
_symlink_or_skip(target, outside, target_is_directory=True)
515+
516+
_migrate_legacy_kimi_skills_dir(
517+
project / ".kimi" / "skills", project / ".kimi-code" / "skills"
518+
)
519+
520+
# Legacy copy is preserved (migration refused to follow the symlink),
521+
# and the outside target is untouched.
522+
assert (legacy / "SKILL.md").exists()
523+
assert (outside / "SKILL.md").exists()
524+
495525
def test_context_migration_does_not_write_through_symlinked_agents_md(
496526
self, tmp_path
497527
):

0 commit comments

Comments
 (0)