Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(dev): use ruff as linter and formatter #991

Draft
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

Pouyanpi
Copy link
Collaborator

No description provided.

Copy link

Documentation preview

https://nvidia.github.io/NeMo-Guardrails/review/pr-991

if not import_path.endswith(".co") and os.path.exists(
os.path.join(root, import_path + ".co")
):
if not import_path.endswith(".co") and os.path.exists(os.path.join(root, import_path + ".co")):

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Copilot Autofix AI 2 days ago

To fix the problem, we need to ensure that the constructed file paths are validated and contained within a safe root directory. This can be achieved by normalizing the path using os.path.normpath and then checking that the normalized path starts with the root directory. This approach will prevent directory traversal attacks by ensuring that the resolved path does not escape the intended directory.

  1. Normalize the import_path using os.path.normpath.
  2. Check that the normalized path starts with one of the allowed root directories in colang_path_dirs.
  3. If the path is valid, proceed with the file operations; otherwise, raise an exception.
Suggested changeset 1
nemoguardrails/rails/llm/config.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/nemoguardrails/rails/llm/config.py b/nemoguardrails/rails/llm/config.py
--- a/nemoguardrails/rails/llm/config.py
+++ b/nemoguardrails/rails/llm/config.py
@@ -730,12 +730,17 @@
             actual_path = None
-            if not os.path.exists(import_path):
-                for root in colang_path_dirs:
-                    if os.path.exists(os.path.join(root, import_path)):
-                        actual_path = os.path.join(root, import_path)
-                        break
+            normalized_import_path = os.path.normpath(import_path)
+            for root in colang_path_dirs:
+                normalized_root = os.path.normpath(root)
+                potential_path = os.path.join(normalized_root, normalized_import_path)
+                if os.path.commonprefix([potential_path, normalized_root]) != normalized_root:
+                    continue
+                if os.path.exists(potential_path):
+                    actual_path = potential_path
+                    break
 
-                    # We also check if we can load it as a file.
-                    if not import_path.endswith(".co") and os.path.exists(os.path.join(root, import_path + ".co")):
-                        actual_path = os.path.join(root, import_path + ".co")
-                        break
+                # We also check if we can load it as a file.
+                potential_path_with_ext = potential_path + ".co"
+                if not import_path.endswith(".co") and os.path.exists(potential_path_with_ext):
+                    actual_path = potential_path_with_ext
+                    break
             else:
EOF
@@ -730,12 +730,17 @@
actual_path = None
if not os.path.exists(import_path):
for root in colang_path_dirs:
if os.path.exists(os.path.join(root, import_path)):
actual_path = os.path.join(root, import_path)
break
normalized_import_path = os.path.normpath(import_path)
for root in colang_path_dirs:
normalized_root = os.path.normpath(root)
potential_path = os.path.join(normalized_root, normalized_import_path)
if os.path.commonprefix([potential_path, normalized_root]) != normalized_root:
continue
if os.path.exists(potential_path):
actual_path = potential_path
break

# We also check if we can load it as a file.
if not import_path.endswith(".co") and os.path.exists(os.path.join(root, import_path + ".co")):
actual_path = os.path.join(root, import_path + ".co")
break
# We also check if we can load it as a file.
potential_path_with_ext = potential_path + ".co"
if not import_path.endswith(".co") and os.path.exists(potential_path_with_ext):
actual_path = potential_path_with_ext
break
else:
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
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