Potential fix for code scanning alert no. 23: Uncontrolled data used in path expression#19
Conversation
…in path expression Co-authored-by: Arsh Verma <arshverma.dev@gmail.com> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
ArshVermaGit
left a comment
There was a problem hiding this comment.
This looks like a reasonable minimal-risk refinement. Moving away from the realpath(join(...)) + commonpath pattern toward a deterministic check on os.path.join(normalized, ".git"), combined with requiring .git to be a real directory and not a symlink, makes the validation logic simpler and more explicit. I also like that this directly addresses the CodeQL concern without broadening behavior or adding new complexity. The main tradeoff is intentionally disallowing repos that rely on a symlinked .git directory, but from a security perspective that seems like an acceptable constraint for a linking validator. Overall, this feels like a clean hardening change with clear intent and minimal functional impact.
| if not os.path.isdir(git_dir): | ||
| git_dir = os.path.join(normalized, ".git") | ||
| # Require a real .git directory inside the repository path and reject symlinks. | ||
| if not os.path.isdir(git_dir) or os.path.islink(git_dir): |
| if not os.path.isdir(git_dir): | ||
| git_dir = os.path.join(normalized, ".git") | ||
| # Require a real .git directory inside the repository path and reject symlinks. | ||
| if not os.path.isdir(git_dir) or os.path.islink(git_dir): |
Potential fix for https://github.com/ArshVermaGit/SentinelOps-Autonomous-DevOps-AI/security/code-scanning/23
General fix: ensure user-controlled paths are canonicalized and constrained to an allowlisted root, then avoid constructing secondary paths in ways that can be interpreted as path injection. Prefer direct checks on expected repository metadata paths.
Best fix here (minimal behavior change): in
LocalGitService._validate_repo_path_for_linking(insentinelops-backend/app/services/local_git_service.py), replace thegit_dir = realpath(join(normalized, ".git"))+commonpathpattern with a deterministic check ofos.path.join(normalized, ".git")and require it to be a real directory viaos.path.isdirandnot os.path.islink. This prevents accepting repos where.gitis a symlink to somewhere else and removes the taintedrealpath(join(...))flow CodeQL highlights.Edit region: method
_validate_repo_path_for_linking, around current lines 139–146.No new imports or dependencies are required.
Suggested fixes powered by Copilot Autofix. Review carefully before merging.