You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/06/01 22:32:38 UTC
[PR] fix(extensions-cli): constrain backend include patterns to the backend directory [superset]
rusackas opened a new pull request, #40593:
URL: https://github.com/apache/superset/pull/40593
### SUMMARY
The Superset Extensions CLI `build`/`bundle` step copies backend files into the distributable bundle based on the `include` glob patterns declared in an extension's `backend/pyproject.toml` (`[tool.apache_superset_extensions.build]`).
Previously those patterns were passed directly to `Path.glob()` and every match was copied without confirming it actually lived under the `backend/` directory. Because `Path.glob()` happily evaluates patterns with parent (`..`) components and `Path.relative_to()` compares lexically (it does not resolve `..`), a pattern could select files from outside the intended `backend/` root and stage them into the build output.
This change tightens the input handling in `copy_backend_files`:
- **Reject** absolute include patterns and any pattern containing `..` path components before they reach `glob()`.
- **Boundary-check** every matched file: resolve it and confirm it is inside the (resolved) backend directory before copying. This is defense-in-depth that also covers symlink-based escapes.
Both checks raise a clear `ValueError` so a misconfigured or hostile build config fails loudly instead of silently bundling unintended files.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A β CLI behavior change.
### TESTING INSTRUCTIONS
```
cd superset-extensions-cli
pip install -e ".[test]"
pytest tests/test_cli_build.py
```
New tests:
- `test_copy_backend_files_rejects_patterns_escaping_backend_dir` β parametrized over patterns that try to escape the backend dir; asserts a `ValueError` and that nothing was staged into `dist/`.
- `test_copy_backend_files_supports_legitimate_nested_patterns` β confirms ordinary recursive globs (e.g. `src/**/*.py`) still resolve and copy deeply nested files.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
π€ Generated with [Claude Code](https://claude.com/claude-code)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(extensions-cli): constrain backend include patterns to the backend directory [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
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 '..'."
+ )
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.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(extensions-cli): constrain backend include patterns to the backend directory [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #40593:
URL: https://github.com/apache/superset/pull/40593
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(extensions-cli): constrain backend include patterns to the backend directory [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40593:
URL: https://github.com/apache/superset/pull/40593#issuecomment-4597047694
### <span aria-hidden="true">β
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">π¨</span> Latest commit | 2431c499bf7bca1c785986132963b2d5aac5a967 |
|<span aria-hidden="true">π</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a1e0886bac43800081a4ebf |
|<span aria-hidden="true">π</span> Deploy Preview | [https://deploy-preview-40593--superset-docs-preview.netlify.app](https://deploy-preview-40593--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">π±</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">π€</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DYzBDcIwDAB38RurqaiqiA0Yw4ld1SI4UWpQEWJ38rvH3X2h9fpsDjeAC6ROlvfBm56TnC52aLUDc1FMlB9ijGq5vFiwkbt0wzcVZfLhjQFLK_Vz57FYaZYQ4zq65RpDCHGmRdIGvz8) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-40593--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(extensions-cli): constrain backend include patterns to the backend directory [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #40593:
URL: https://github.com/apache/superset/pull/40593#discussion_r3337854919
##########
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:
Raising a bare ValueError from a Click-based CLI results in an unfriendly traceback in normal CLI usage. Use click.ClickException (or UsageError) so the command fails with a clean, user-facing error message consistent with the rest of this fileβs error handling.
##########
superset-extensions-cli/tests/test_cli_build.py:
##########
@@ -625,6 +625,109 @@ def test_copy_backend_files_handles_various_glob_patterns(isolated_filesystem):
)
+@pytest.mark.unit
+def test_copy_backend_files_supports_legitimate_nested_patterns(isolated_filesystem):
+ """Test copy_backend_files copies deeply nested files via recursive globs."""
+ backend_dir = isolated_filesystem / "backend"
+ nested = backend_dir / "src" / "test_org" / "test_ext" / "deep" / "deeper"
+ nested.mkdir(parents=True)
+ (nested / "module.py").write_text("# nested module")
+
+ pyproject_content = """[project]
+name = "test_org-test_ext"
+version = "1.0.0"
+license = "Apache-2.0"
+
+[tool.apache_superset_extensions.build]
+include = [
+ "src/test_org/test_ext/**/*.py",
+]
+exclude = []
+"""
+ (backend_dir / "pyproject.toml").write_text(pyproject_content)
+
+ extension_data = {
+ "publisher": "test-org",
+ "name": "test-ext",
+ "displayName": "Test Extension",
+ "version": "1.0.0",
+ "permissions": [],
+ }
+ (isolated_filesystem / "extension.json").write_text(json.dumps(extension_data))
+
+ clean_dist(isolated_filesystem)
+ copy_backend_files(isolated_filesystem)
+
+ dist_dir = isolated_filesystem / "dist"
+ assert_file_exists(
+ dist_dir
+ / "backend"
+ / "src"
+ / "test_org"
+ / "test_ext"
+ / "deep"
+ / "deeper"
+ / "module.py"
+ )
+
+
+@pytest.mark.unit
+@pytest.mark.parametrize(
+ "bad_pattern",
+ [
+ "../../.ssh/*",
+ "../config",
+ "src/../../secret.txt",
+ "/etc/passwd",
+ ],
+)
+def test_copy_backend_files_rejects_patterns_escaping_backend_dir(
+ isolated_filesystem, bad_pattern
+):
+ """Test copy_backend_files refuses include patterns that escape backend_dir."""
+ # Create a sensitive file outside the backend directory.
+ (isolated_filesystem / "secret.txt").write_text("SECRET")
+ (isolated_filesystem / "config").write_text("SECRET")
+
+ backend_dir = isolated_filesystem / "backend"
+ backend_src = backend_dir / "src" / "test_org" / "test_ext"
+ backend_src.mkdir(parents=True)
+ (backend_src / "__init__.py").write_text("# init")
+
+ pyproject_content = f"""[project]
+name = "test_org-test_ext"
+version = "1.0.0"
+license = "Apache-2.0"
+
+[tool.apache_superset_extensions.build]
+include = [
+ "{bad_pattern}",
+]
+exclude = []
+"""
+ (backend_dir / "pyproject.toml").write_text(pyproject_content)
+
+ extension_data = {
+ "publisher": "test-org",
+ "name": "test-ext",
+ "displayName": "Test Extension",
+ "version": "1.0.0",
+ "permissions": [],
+ }
+ (isolated_filesystem / "extension.json").write_text(json.dumps(extension_data))
+
+ clean_dist(isolated_filesystem)
+
+ with pytest.raises(ValueError):
+ copy_backend_files(isolated_filesystem)
+
Review Comment:
If copy_backend_files is switched to raise click.ClickException for user-friendly CLI errors, this assertion should be updated accordingly so the test matches the intended behavior.
##########
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:
This should raise a ClickException (not ValueError) so the CLI exits with a clean error message rather than a Python traceback when a matched file resolves outside the backend directory.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(extensions-cli): constrain backend include patterns to the backend directory [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #40593:
URL: https://github.com/apache/superset/pull/40593#discussion_r3342417973
##########
superset-extensions-cli/tests/test_cli_build.py:
##########
@@ -625,6 +625,109 @@ def test_copy_backend_files_handles_various_glob_patterns(isolated_filesystem):
)
+@pytest.mark.unit
+def test_copy_backend_files_supports_legitimate_nested_patterns(isolated_filesystem):
+ """Test copy_backend_files copies deeply nested files via recursive globs."""
+ backend_dir = isolated_filesystem / "backend"
+ nested = backend_dir / "src" / "test_org" / "test_ext" / "deep" / "deeper"
+ nested.mkdir(parents=True)
+ (nested / "module.py").write_text("# nested module")
+
+ pyproject_content = """[project]
+name = "test_org-test_ext"
+version = "1.0.0"
+license = "Apache-2.0"
+
+[tool.apache_superset_extensions.build]
+include = [
+ "src/test_org/test_ext/**/*.py",
+]
+exclude = []
+"""
+ (backend_dir / "pyproject.toml").write_text(pyproject_content)
+
+ extension_data = {
+ "publisher": "test-org",
+ "name": "test-ext",
+ "displayName": "Test Extension",
+ "version": "1.0.0",
+ "permissions": [],
+ }
+ (isolated_filesystem / "extension.json").write_text(json.dumps(extension_data))
+
+ clean_dist(isolated_filesystem)
+ copy_backend_files(isolated_filesystem)
+
+ dist_dir = isolated_filesystem / "dist"
+ assert_file_exists(
+ dist_dir
+ / "backend"
+ / "src"
+ / "test_org"
+ / "test_ext"
+ / "deep"
+ / "deeper"
+ / "module.py"
+ )
+
+
+@pytest.mark.unit
+@pytest.mark.parametrize(
+ "bad_pattern",
+ [
+ "../../.ssh/*",
+ "../config",
+ "src/../../secret.txt",
+ "/etc/passwd",
+ ],
+)
+def test_copy_backend_files_rejects_patterns_escaping_backend_dir(
+ isolated_filesystem, bad_pattern
+):
+ """Test copy_backend_files refuses include patterns that escape backend_dir."""
+ # Create a sensitive file outside the backend directory.
+ (isolated_filesystem / "secret.txt").write_text("SECRET")
+ (isolated_filesystem / "config").write_text("SECRET")
+
+ backend_dir = isolated_filesystem / "backend"
+ backend_src = backend_dir / "src" / "test_org" / "test_ext"
+ backend_src.mkdir(parents=True)
+ (backend_src / "__init__.py").write_text("# init")
+
+ pyproject_content = f"""[project]
+name = "test_org-test_ext"
+version = "1.0.0"
+license = "Apache-2.0"
+
+[tool.apache_superset_extensions.build]
+include = [
+ "{bad_pattern}",
+]
+exclude = []
+"""
+ (backend_dir / "pyproject.toml").write_text(pyproject_content)
+
+ extension_data = {
+ "publisher": "test-org",
+ "name": "test-ext",
+ "displayName": "Test Extension",
+ "version": "1.0.0",
+ "permissions": [],
+ }
+ (isolated_filesystem / "extension.json").write_text(json.dumps(extension_data))
+
+ clean_dist(isolated_filesystem)
+
+ with pytest.raises(ValueError):
+ copy_backend_files(isolated_filesystem)
+
Review Comment:
Updated in 3d21d88 β the assertion now expects click.ClickException to match the new behavior, and the parametrized cases still pass.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(extensions-cli): constrain backend include patterns to the backend directory [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40593:
URL: https://github.com/apache/superset/pull/40593#discussion_r3374820879
##########
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 click.ClickException(
+ 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 click.ClickException(
+ f"Refusing to copy {f}: resolved path is outside the "
+ f"backend directory {backend_dir}."
+ )
+
# Check exclude patterns
- relative_path = f.relative_to(backend_dir)
+ relative_path = resolved.relative_to(backend_dir)
Review Comment:
**Suggestion:** Using the resolved filesystem path as the bundle-relative path changes behavior for symlinked files inside `backend/`: excludes are evaluated against the target path instead of the configured symlink path, and copied files are staged under the resolved target location rather than the matched path. This can cause unexpected inclusions and incorrect bundle layout (and potential overwrites when multiple symlinks point to the same target). Keep the boundary check on `resolved`, but compute the staged/exclude path from the matched path (`f.relative_to(backend_dir)`). [logic error]
<details>
<summary><b>Severity Level:</b> Critical π¨</summary>
```mdx
- β Backend build copies symlinked files to wrong bundle paths.
- β Symlinked shared modules may be missing at expected locations.
- β οΈ Exclude patterns apply to targets not configured symlink paths.
- β οΈ Multiple symlinks to one target overwrite bundle files.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. In an extension project with a backend directory, create a symlinked module under
`backend/src` (for example `backend/src/test_org/test_ext/common/module.py` as a symlink
to `backend/src/common/module.py`). The build logic that will process this lives in
`superset-extensions-cli/src/superset_extensions_cli/cli.py:226-275`
(`copy_backend_files`).
2. Configure `backend/pyproject.toml` so that
`[tool.apache_superset_extensions.build].include` contains a pattern that matches the
symlinked path (for example `include = ["src/test_org/test_ext/**/*.py"]`) and an
`exclude` that does NOT exclude the symlink path but may exclude the target path (for
example `exclude = ["src/common/**"]`), matching how `include_patterns` and
`exclude_patterns` are read at `cli.py:231-238`.
3. Run the extensions CLI build command from the project root (entrypoint `build` is
defined at `superset-extensions-cli/src/superset_extensions_cli/cli.py:553-580` and calls
`rebuild_backend` at `570-574`, which in turn calls `copy_backend_files` at `291-295`).
This causes `copy_backend_files` to iterate include patterns, glob files under
`backend_dir.glob(pattern)` at `cli.py:251`, and pick up the symlinked file as `f`.
4. Inside `copy_backend_files`, for the symlinked file `f`, the code resolves it
(`resolved = f.resolve()` at `cli.py:257`) and then computes `relative_path =
resolved.relative_to(backend_dir)` at `cli.py:265`. For a symlink
`backend/src/test_org/test_ext/common/module.py` pointing to
`backend/src/common/module.py`, `relative_path` becomes `src/common/module.py`. Exclude
checks and the target path `tgt = dist_dir / "backend" / relative_path` at `cli.py:272`
now operate on the resolved target path rather than the matched symlink path, so: (a) the
exclude pattern `src/common/**` will unexpectedly exclude the symlinked file even though
the configured include path is under `src/test_org/test_ext/**`, and (b) if not excluded,
the file is copied into `dist/backend/src/common/module.py` instead of
`dist/backend/src/test_org/test_ext/common/module.py`, changing the bundle layout relative
to the backend tree and breaking symlink path semantics.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=dfab9a0071974771a315ce41907db7ab&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=dfab9a0071974771a315ce41907db7ab&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-extensions-cli/src/superset_extensions_cli/cli.py
**Line:** 265:265
**Comment:**
*Logic Error: Using the resolved filesystem path as the bundle-relative path changes behavior for symlinked files inside `backend/`: excludes are evaluated against the target path instead of the configured symlink path, and copied files are staged under the resolved target location rather than the matched path. This can cause unexpected inclusions and incorrect bundle layout (and potential overwrites when multiple symlinks point to the same target). Keep the boundary check on `resolved`, but compute the staged/exclude path from the matched path (`f.relative_to(backend_dir)`).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40593&comment_hash=6bdbe14a6457df49c42de5157c77f237e8d90505e2a6c8404dff8de1687d39bb&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40593&comment_hash=6bdbe14a6457df49c42de5157c77f237e8d90505e2a6c8404dff8de1687d39bb&reaction=dislike'>π</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(extensions-cli): constrain backend include patterns to the backend directory [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #40593:
URL: https://github.com/apache/superset/pull/40593#discussion_r3374889283
##########
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 click.ClickException(
+ 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 click.ClickException(
+ f"Refusing to copy {f}: resolved path is outside the "
+ f"backend directory {backend_dir}."
+ )
+
# Check exclude patterns
- relative_path = f.relative_to(backend_dir)
+ relative_path = resolved.relative_to(backend_dir)
Review Comment:
Good catch β fixed in 8687268. The boundary check still runs against the resolved path, but the bundle-relative path and exclude evaluation now use the matched path (`f.relative_to(backend_dir)`) instead of the resolved symlink target. So symlinked files inside `backend/` are staged at their configured path rather than under the target location, and excludes apply to the configured path. Added a test covering the symlink case.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(extensions-cli): constrain backend include patterns to the backend directory [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40593:
URL: https://github.com/apache/superset/pull/40593#issuecomment-4605850217
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(extensions-cli): constrain backend include patterns to the backend directory [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40593:
URL: https://github.com/apache/superset/pull/40593#issuecomment-4597033381
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(extensions-cli): constrain backend include patterns to the backend directory [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40593:
URL: https://github.com/apache/superset/pull/40593#issuecomment-4652658944
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(extensions-cli): constrain backend include patterns to the backend directory [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org