Skip to content

Potential fix for code scanning alert no. 13: Uncontrolled command line#2

Merged
ArshVermaGit merged 1 commit intomainfrom
alert-autofix-13
Apr 20, 2026
Merged

Potential fix for code scanning alert no. 13: Uncontrolled command line#2
ArshVermaGit merged 1 commit intomainfrom
alert-autofix-13

Conversation

@ArshVermaGit
Copy link
Copy Markdown
Owner

Potential fix for https://github.com/ArshVermaGit/SentinelOps-Autonomous-DevOps-AI/security/code-scanning/13

Best fix: enforce a strict allowlist so git commands only run for repositories already present in linked_repos.json, with canonicalized path comparison to prevent bypasses (relative-path tricks, symlink-ish variations, ~, etc.). This preserves existing behavior for legitimate linked repos and blocks arbitrary user-provided paths.

Single best implementation in the shown code:

  1. In sentinelops-backend/app/services/local_git_service.py, add helper methods to canonicalize paths and check whether a path is linked.
  2. In _run_git, reject execution unless repo_path is in the linked-repo allowlist.
  3. In get_repo_status, fail early with an "error" response if the requested path is not linked (before calling any git command), and canonicalize the path.
  4. Keep subprocess.run list-style usage as-is (already safer than shell), but add this authorization guard at the service boundary so all three alert variants are addressed centrally.

No router changes are strictly required once service-level allowlisting is enforced.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Welcome to SentinelOps! 🚀 Thanks for your first PR. We'll analyze it for risks and get back to you soon.

@ArshVermaGit ArshVermaGit marked this pull request as ready for review April 20, 2026 19:14
Copy link
Copy Markdown
Owner Author

@ArshVermaGit ArshVermaGit left a comment

Choose a reason for hiding this comment

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

This is a strong and well-targeted security hardening change that addresses the root cause rather than just patching individual call sites. Enforcing a strict allowlist tied to linked_repos.json, combined with canonicalized path comparison, effectively prevents command execution against arbitrary filesystem paths while preserving existing behavior for legitimate repositories. I particularly like that the fix is implemented centrally within _run_git, ensuring consistent enforcement across all git operations without requiring scattered router-level checks. Handling normalization edge cases (~, relative paths, symlink-like variations) shows good defensive thinking and significantly reduces the risk of bypass techniques. Keeping subprocess.run in list form maintains existing safety guarantees while adding a proper authorization boundary at the service layer. Overall, this is a clean, minimal, and high-impact mitigation that meaningfully improves command execution safety without introducing unnecessary complexity.

@ArshVermaGit ArshVermaGit merged commit 02ba6b0 into main Apr 20, 2026
5 of 6 checks passed
Sign up for free to 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