fix(security): harden skill ID validation and installation paths

[yacosta738]

User's Goal

Harden AgentSync against path traversal vulnerabilities by strictly validating skill IDs during installation and updates.

Severity

CRITICAL

Vulnerability

Path traversal during skill installation. Malicious configurations could provide skill IDs like ../escape, allowing the tool to create directories or files outside the intended .agents/skills/ directory.

Impact

An attacker with control over agentsync.toml or providing malicious skill IDs to the CLI could potentially overwrite sensitive files or create unauthorized directories on the user's filesystem.

Fix

Implemented a two-layered defense:

1. Strict Whitelisting: At the CLI entry point (src/commands/skill.rs), skill IDs are now restricted to a safe set of characters (alphanumeric, hyphens, and underscores).
2. Defense-in-Depth Validation: At the core installation layer (src/skills/install.rs), the install_from_dir function now validates that the provided skill_id resolves to exactly one "Normal" path component, preventing any traversal attempts even if the CLI whitelist were bypassed.

Source

• src/commands/skill.rs: validate_skill_id
• src/skills/install.rs: install_from_dir

Verification

• New unit tests in src/commands/skill.rs verify the hardened validate_skill_id logic.
• A new regression test test_skill_install_id_traversal in tests/test_security.rs confirms that traversal attempts in skill IDs are correctly rejected during installation.
• All existing tests pass.
• Verified code quality with cargo clippy and cargo fmt.

---

PR created automatically by Jules for task 10869251960065621527 started by @yacosta738

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 08e7d68b-9f51-4a3f-9fd5-db2ca4465073

📥 Commits

Reviewing files that changed from the base of the PR and between ffc0dfd and 517e39d.

📒 Files selected for processing (3)

• src/commands/skill.rs
• src/skills/install.rs
• tests/test_security.rs

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Strengthened skill ID validation with stricter character restrictions (alphanumeric characters, hyphens, and underscores only) to prevent path traversal attacks and directory escape vulnerabilities.
  • Added defense-in-depth security validation to ensure skill IDs cannot access directories outside their intended scope.

• Tests

  • Added security regression tests to validate protection against path traversal attacks and malicious skill ID inputs.

Walkthrough

This PR strengthens skill ID validation to prevent path traversal attacks. The validate_skill_id function now enforces a strict character whitelist and uses path component analysis as a second defense layer. These stricter rules are then integrated into the install flow, with a regression test confirming that traversal attempts are properly rejected.

Changes

Skill ID Path Traversal Prevention

Layer / File(s)	Summary
Skill ID Validation Hardening src/commands/skill.rs	validate_skill_id replaces previous validation with a strict allowed-character whitelist (alphanumeric, -, _) and adds Path::components() verification to ensure exactly one safe "normal" segment. Unit tests extend coverage to reject "..." and special characters such as @, !, and whitespace.
Install-Time Validation and Regression Test src/skills/install.rs, tests/test_security.rs	install_from_dir adds a pre-install security check that validates skill_id before computing the destination directory, returning SkillInstallError::Validation on invalid input. A new regression test test_skill_install_id_traversal verifies that traversal attempts (e.g., "../outside_skill") fail with an "invalid skill id" error and do not create directories outside the intended agents folder.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 A hare guards the path with watchful eyes,
No ../ tricks can slip on by,
With whitelist checks and component guards so tight,
Traversals bounce back to the light.
Safe skillIDs install with secure delight! 🔒

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(security): harden skill ID validation and installation paths' directly and clearly summarizes the main change: hardening security by improving skill ID validation.
Description check	✅ Passed	The description comprehensively covers the vulnerability, fix, and verification steps, clearly relating to the changeset that hardens skill ID validation and prevents path traversal attacks.
Docstring Coverage	✅ Passed	Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/harden-skill-id-validation-10869251960065621527

---

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.}

[sentry]

Codecov Report

❌ Patch coverage is 85.71429% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/commands/skill.rs	80.95%	4 Missing ⚠️

📢 Thoughts on this report? Let us know!

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
85.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud