Skip to content

Autofix: Code scanning alert #3 #5

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.

Sign up for GitHub

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

Open
Notoriousjayy wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Autofix: Code scanning alert #3 #5

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion scripts/apply_policy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,12 @@

logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(message)s")

cfg = json.loads(Path(args.config).read_text(encoding="utf-8"))
config_path = Path(args.config).expanduser().resolve()

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
Loading
.

Copilot Autofix

AI 4 months ago

General approach: validate or constrain the user-supplied config path before using it to access the file system. Typical strategies are: restrict it to a known base directory, ensure it’s not absolute, and/or enforce a naming pattern or extension; then use resolve() and verify the final path is within the allowed base.

Best fit here with minimal functional change: keep accepting a --config path, but (1) interpret it relative to the current working directory when it’s not absolute, (2) normalize it, and (3) optionally enforce that it ends with .json (since the code expects JSON). We can also log a clear error if the path is suspicious or invalid. We already use Path and .resolve(), so we don’t need new imports or libraries.

Concrete changes in scripts/apply_policy.py:

  • Around line 69, replace the direct config_path = Path(args.config).expanduser().resolve() with a small block that:
    • Builds an initial Path from args.config.
    • Optionally enforces a simple extension check (e.g., .json).
    • Resolves the path.
  • Keep the existing is_file() check and subsequent read_text logic the same, so functionality is preserved except for the added validation.

No new helper functions or external imports are required; everything can be done in place using pathlib.Path.

Suggested changeset 1
scripts/apply_policy.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/scripts/apply_policy.py b/scripts/apply_policy.py
--- a/scripts/apply_policy.py
+++ b/scripts/apply_policy.py
@@ -66,7 +66,13 @@
 
     logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(message)s")
 
-    config_path = Path(args.config).expanduser().resolve()
+    raw_config_path = Path(args.config)
+    # Basic validation for the configuration file path
+    if raw_config_path.suffix and raw_config_path.suffix.lower() != ".json":
+        log.error("Configuration file must have a .json extension: %s", raw_config_path)
+        return 1
+
+    config_path = raw_config_path.expanduser().resolve()
     if not config_path.is_file():
         log.error("Configuration file does not exist or is not a file: %s", config_path)
         return 1
EOF
@@ -66,7 +66,13 @@

logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(message)s")

config_path = Path(args.config).expanduser().resolve()
raw_config_path = Path(args.config)
# Basic validation for the configuration file path
if raw_config_path.suffix and raw_config_path.suffix.lower() != ".json":
log.error("Configuration file must have a .json extension: %s", raw_config_path)
return 1

config_path = raw_config_path.expanduser().resolve()
if not config_path.is_file():
log.error("Configuration file does not exist or is not a file: %s", config_path)
return 1
Copilot is powered by AI and may make mistakes. Always verify output.
if not config_path.is_file():
log.error("Configuration file does not exist or is not a file: %s", config_path)
return 1

cfg = json.loads(config_path.read_text(encoding="utf-8"))

rest, _ = create_clients()
sec = RepoSecurityClient(rest)
Expand Down