fix(core): guard reconcile skill deletion paths

[AkaraChen]

Motivation

• Reconcile previously trusted adapter-discovered skills directories and deleted dir/sanitize_name(skill) unconditionally, which allowed deletions via symlinked skills roots and could remove directories outside intended skill roots.
• The change prevents an attacker-controlled marketplace/plugin or symlinked project tree from causing remove_dir_all() to remove arbitrary user-writable directories.

Description

• Add has_symlink_component() to detect symlinked path components and valid_skill_delete_location() to validate candidate deletion targets with multiple checks (safe name, non-symlink directory, presence of SKILL.md, parse name match, canonical containment, and canonical-path deduplication).
• Update find_skill_locations_in_agents() to only return validated, deduplicated deletion candidates instead of trusting every adapter-provided skills directory.
• Revalidate each candidate immediately before calling fs::remove_dir_all() and refuse deletion when a candidate becomes unsafe.
• Add a Unix-only regression test reconcile_skill_skips_symlinked_skills_root_delete() that asserts a symlinked .claude/skills pointing outside the project is skipped and the outside directory is not removed.

Testing

• Ran formatting with cargo fmt -p aghub-core, which completed successfully.
• Ran a repo health check with git diff --check, which reported no whitespace issues.
• Attempted to run the new unit test via cargo test -p aghub-core reconcile_skill_skips_symlinked_skills_root_delete and variants, but test execution was blocked by the environment (missing sccache wrapper and network crate-download failures), so the test could not be executed here.
• just fmt was not available in the execution environment, so frontend/monorepo format steps were not run.

---

Codex Task

[coderabbitai]

Warning

Review limit reached

@AkaraChen, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 27 minutes and 9 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c0b01514-16ee-43e0-a6af-d75cb12eea35

📥 Commits

Reviewing files that changed from the base of the PR and between 88dc33e and 6425c27.

📒 Files selected for processing (1)

• crates/core/src/transfer.rs

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/fix-path-trust-issue-in-skill-deletion

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6425c27b4d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Allow deleting symlinked skill entries

When the skills root itself is a real directory but skills/<safe-name> is a symlink, this check filters the location out before reconcile_skill can remove anything. That layout is supported elsewhere in the repo: skill discovery follows symlinked skill directories and records canonical_path, and ConfigManager::remove_skill removes just the symlink parent rather than the target. In that supported setup, removing an agent through reconcile returns no delete result and leaves the symlinked skill installed.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Do not reject symlinked project ancestors

This rejects any symlink in the entire skills_dir path, not just a symlinked skills root. When a project is opened through a symlinked checkout path, or a home/tmp ancestor is a symlink, a normal .claude/skills/<skill> directory is skipped before the later canonical containment check can prove it is still inside the real skills directory, so reconcile leaves the skill installed and reports no delete result for that agent.

Useful? React with 👍 / 👎.