fix: security hardening, null array fix, and test coverage

[eddieran]

Summary

• Security: Add path traversal validation in findSkillDir and listSkillsInDir — skill names containing .. are rejected to prevent directory escape from cache root
• Bug fix: Initialize enabledAdapters and inject results as empty slices so JSON output emits [] instead of null
• Tests: Add 16 new tests covering v2.3.0 features (status cmd, isJSONMode, ExitCoder, git quiet mode, path traversal, detectCurrentBranch edge cases, isGitRepo, ScanPathError)

Test plan

• go vet ./... — clean
• go test ./... -count=1 — 29/29 packages pass
• Overall coverage: 70.2%
• gofmt — clean
• go build — clean

🤖 Generated with Claude Code

Summary by CodeRabbit

• Security

  • Added validation to reject invalid skill names containing path traversal sequences.

• Bug Fixes

  • Improved error handling and validation for skill resolution and status command output.

• Tests

  • Enhanced test coverage for status command with JSON and text output formats, and git provider functionality.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a9c35f7 and ff9ca56.

📒 Files selected for processing (4)

• cmd/skillpm/main.go
• cmd/skillpm/main_test.go
• internal/source/git_provider.go
• internal/source/git_provider_test.go

---

📝 Walkthrough

Walkthrough

This PR introduces path traversal protections in git provider skill resolution functions and comprehensive test coverage for status command functionality and git provider behavior. Slice initialization in main.go is refactored to use non-nil empty slices.

Changes

Cohort / File(s)	Summary
Slice initialization refactoring cmd/skillpm/main.go	Changes results and enabledAdapters slice declarations from var (nil) to make() with zero capacity, ensuring non-nil empty slices instead of nil.
Status command and JSON tests cmd/skillpm/main_test.go	Adds 133 lines of test coverage for status command argument validation, text and JSON output structure validation, JSON mode flag behavior, and ExitCoder interface compliance.
Path traversal protection guards internal/source/git_provider.go	Adds validation guards in findSkillDir and listSkillsInDir to reject skill names and prefixes containing ".." sequences, returning error and nil respectively.
Git provider integration tests internal/source/git_provider_test.go	Adds 164 lines of test coverage for git provider initialization, path traversal protection, skill resolution behavior, auto-clone logic, constraint handling, and search functionality.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• fix(resolve): expand scan-path directory URLs into individual skill installs #12: Directly related—modifies listSkillsInDir behavior by adding path traversal protection guards to reject ".." prefixes.
• feat: CLI agent-friendly optimizations (P0+P1) #16: Related through the slice initialization changes ensuring non-nil JSON arrays (like enabledAdapters) in status JSON output.
• cleanup: remove dead code, invalid test references, and aliases #7: Related through concurrent modifications to slice initialization and command wiring in cmd/skillpm/main.go.

Poem

🐰 A rabbit hops through git repos with glee,
No ".." tricks can fool thee!
Tests abound from skill to skill,
Non-nil slices fit the bill! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 24.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the three main changes: security hardening (path traversal protection), null array fix (slice initialization), and expanded test coverage.
Description check	✅ Passed	The description covers all required template sections: summary of changes with detail, type of change (fix), validation steps completed, and security impact explanation.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/hardening-tests-security

---

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