Skip to content

Potential fix for code scanning alert no. 27: Uncontrolled data used in path expression#24

Merged
ArshVermaGit merged 1 commit into
mainfrom
alert-autofix-31
May 21, 2026
Merged

Potential fix for code scanning alert no. 27: Uncontrolled data used in path expression#24
ArshVermaGit merged 1 commit into
mainfrom
alert-autofix-31

Conversation

@ArshVermaGit
Copy link
Copy Markdown
Owner

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

Best fix: keep the existing normalization + containment logic, but make it fail closed unless SENTINELOPS_REPO_ROOT is explicitly configured to a valid absolute existing directory. This prevents uncontrolled probing against an implicit, potentially broad root (os.getcwd()).

Concretely in sentinelops-backend/app/services/local_git_service.py:

  • Replace the global ALLOWED_REPO_ROOT initialization so it does not default to os.getcwd().
  • In _is_within_allowed_root, keep current checks; they will now correctly reject when env is unset.
  • No functional changes to linking flow beyond requiring explicit root configuration (security-hardening behavior).

No new methods or third-party libraries are required; only a constant initialization change in the shown file.

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

…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>
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 reasonable security hardening change, especially if the goal is to avoid unintentionally broad trust boundaries. Removing the implicit os.getcwd() fallback and requiring SENTINELOPS_REPO_ROOT to be explicitly configured as a valid absolute directory makes the allowed-root model much more predictable and fail-closed by default. The main tradeoff is the stricter operational requirement, since deployments that previously relied on the fallback will now need configuration updates, but from a security standpoint that feels justified. As long as the configuration expectation is clearly documented and startup/logging makes misconfiguration obvious, this looks like a solid improvement.

@ArshVermaGit ArshVermaGit marked this pull request as ready for review May 21, 2026 21:01
@ArshVermaGit ArshVermaGit merged commit aac3ee6 into main May 21, 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