Skip to content

fix(security): validate repository names to block path traversal - #183

Merged
QaidVoid merged 2 commits into
mainfrom
fix/repo-name-path-traversal
Jul 16, 2026
Merged

fix(security): validate repository names to block path traversal#183
QaidVoid merged 2 commits into
mainfrom
fix/repo-name-path-traversal

Conversation

@QaidVoid

@QaidVoidQaidVoid commented Jul 16, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Bug Fixes
    • Hardened repository name validation to reject unsafe components and traversal-like inputs.
    • Prevented repository removal from deleting outside the intended repository data directory.
    • Improved repository-related error messages by distinguishing “repository not found” from “invalid repository name”.
  • Tests
    • Added unit test coverage for valid and invalid repository name cases (e.g., empty, absolute paths, separators, and traversal patterns).

@coderabbitai

coderabbitaiBot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d352cf15-294e-4dda-b41e-5cbd45bf6d7e

📥 Commits

Reviewing files that changed from the base of the PR and between 13fc1db and 5df0ea3.

📒 Files selected for processing (1)
  • crates/soar-operations/src/repo.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/soar-operations/src/repo.rs

📝 Walkthrough

Walkthrough

Repository names now use shared path-component validation during configuration resolution and repository removal. Configuration errors distinguish missing repositories from invalid names, while repository deletion canonicalizes and bounds its target path before removal.

Changes

Repository safety and diagnostics

Layer / File(s)Summary
Shared safe-component validation
crates/soar-utils/src/path.rs, crates/soar-db/Cargo.toml, crates/soar-db/src/models/types.rs
A shared is_safe_component helper validates repository path components, and the database model imports it instead of maintaining a local implementation.
Configuration validation and error contract
crates/soar-config/src/error.rs, crates/soar-config/src/config.rs
Configuration resolution rejects unsafe repository names, with tests for traversal-like and normal names; repository diagnostics now distinguish missing repositories from invalid names.
Repository operation safeguards
crates/soar-operations/src/repo.rs
Update and removal operations use RepositoryNotFound; removal canonicalizes paths, verifies containment, and rejects unsafe deletion targets.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
participant SoarContext
participant ConfigResolve
participant is_safe_component
participant ConfigError
SoarContext->>ConfigResolve: resolve repository configuration
ConfigResolve->>is_safe_component: validate repository name
is_safe_component-->>ConfigResolve: safe or unsafe result
ConfigResolve->>ConfigError: return InvalidRepositoryName for unsafe name
Loading

Possibly related PRs

  • pkgforge/soar#182: Both changes centralize path-component validation to block traversal and apply it to repository path handling.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title accurately summarizes the main change: hardening repository-name validation to prevent path traversal.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ 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/repo-name-path-traversal

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

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pagesBot commented Jul 16, 2026

Copy link
Copy Markdown

Deploying soar-docs with Cloudflare Pages Cloudflare Pages

Latest commit:5df0ea3
Status: ✅ Deploy successful!
Preview URL:https://3b0e40cf.soar-docs.pages.dev
Branch Preview URL:https://fix-repo-name-path-traversal.soar-docs.pages.dev

View logs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/soar-operations/src/repo.rs`:
- Around line 107-118: Update the repository deletion flow around the
canonicalization and bounds check to remove the configured repo_path rather than
the canonicalized target. Before deletion, inspect repo_path metadata and
explicitly remove a symlink with the appropriate file-removal operation;
otherwise use remove_dir_all on repo_path, preserving the existing validation
and error context.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 79254bd0-72f1-4464-a8b0-886d36abe037

📥 Commits

Reviewing files that changed from the base of the PR and between 034b085 and 13fc1db.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • crates/soar-config/src/config.rs
  • crates/soar-config/src/error.rs
  • crates/soar-db/Cargo.toml
  • crates/soar-db/src/models/types.rs
  • crates/soar-operations/src/repo.rs
  • crates/soar-utils/src/path.rs

Comment threadcrates/soar-operations/src/repo.rs Outdated
@QaidVoid
QaidVoid merged commit c4b34f9 into mainJul 16, 2026
9 of 10 checks passed
@QaidVoidQaidVoid mentioned this pull request Jul 16, 2026
@QaidVoidQaidVoid mentioned this pull request Jul 16, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@QaidVoid