rusackas commented on code in PR #40593:
URL: https://github.com/apache/superset/pull/40593#discussion_r3342414232


##########
superset-extensions-cli/src/superset_extensions_cli/cli.py:
##########
@@ -239,12 +239,30 @@ def copy_backend_files(cwd: Path) -> None:
 
     # Process include patterns
     for pattern in include_patterns:
+        # Include patterns are only meant to select files within the backend
+        # directory. Reject absolute patterns or ones that walk outside it via
+        # parent ("..") components before handing them to glob().
+        pattern_parts = Path(pattern).parts
+        if Path(pattern).is_absolute() or ".." in pattern_parts:
+            raise ValueError(
+                f"Invalid include pattern {pattern!r}: patterns must be "
+                "relative to the backend directory and may not contain '..'."
+            )

Review Comment:
   Good catch — fixed in 3d21d88. Swapped this to click.ClickException so the 
build/bundle commands surface a clean "Error: ..." message instead of a raw 
traceback, matching the rest of this file.



##########
superset-extensions-cli/src/superset_extensions_cli/cli.py:
##########
@@ -239,12 +239,30 @@ def copy_backend_files(cwd: Path) -> None:
 
     # Process include patterns
     for pattern in include_patterns:
+        # Include patterns are only meant to select files within the backend
+        # directory. Reject absolute patterns or ones that walk outside it via
+        # parent ("..") components before handing them to glob().
+        pattern_parts = Path(pattern).parts
+        if Path(pattern).is_absolute() or ".." in pattern_parts:
+            raise ValueError(
+                f"Invalid include pattern {pattern!r}: patterns must be "
+                "relative to the backend directory and may not contain '..'."
+            )
         for f in backend_dir.glob(pattern):
             if not f.is_file():
                 continue
 
+            # Defense in depth: confirm the matched file resolves to a location
+            # inside the backend directory before copying it into the bundle.
+            resolved = f.resolve()
+            if not resolved.is_relative_to(backend_dir):
+                raise ValueError(
+                    f"Refusing to copy {f}: resolved path is outside the "
+                    f"backend directory {backend_dir}."
+                )

Review Comment:
   Done in 3d21d88 — this defense-in-depth check now raises 
click.ClickException too, so an escaping resolved path fails cleanly instead of 
with a traceback.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to