nodes/MajoorWaldi-ComfyUI-Majoor-AssetsManager
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #186 from MajoorWaldi/agent/fix-viewer-paths-and-write-access-docs

Browse Source 
fix(security): harden workflow path containment and bump brace-expansion
This commit is contained in:
Ewald ALOEBOETOE
2026-07-24 16:20:00 +02:00
committed by GitHub
parent fe9bb8300e 17db2fe9d7
commit 07071e7d76
4 changed files with 87 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files
mjr_am_backend/features/workflows/service.py
+52 -21
View File
@@ -1023,6 +1023,35 @@ def is_managed_workflow_json_path(path: Path) -> bool:
return False
def _resolve_managed_workflow_path(path: Path) -> Result[Path]:
"""Resolve *path* while enforcing containment in the managed workflow root.
The lexical prefix check runs before any filesystem access so untrusted
paths never reach ``resolve``/``stat`` (avoids an existence oracle and
keeps the path-injection barrier visible to static analysis), then the
containment is re-validated after symlink resolution.
"""
root = managed_workflow_root(create=False)
if root is None:
return Result.Err("FORBIDDEN", "Workflow path is not allowed")
try:
prefix = os.path.normcase(os.path.normpath(str(root)))
candidate = os.path.normcase(os.path.normpath(os.path.abspath(str(path))))
except Exception:
return Result.Err("FORBIDDEN", "Workflow path is not allowed")
if candidate != prefix and not candidate.startswith(prefix + os.sep):
return Result.Err("FORBIDDEN", "Workflow path is not allowed")
try:
resolved = Path(candidate).resolve(strict=True)
except FileNotFoundError:
return Result.Err("NOT_FOUND", "Workflow not found")
except Exception:
return Result.Err("FORBIDDEN", "Workflow path is not allowed")
if not is_managed_workflow_json_path(resolved):
return Result.Err("FORBIDDEN", "Workflow path is not allowed")
return Result.Ok(resolved)
def read_workflow_content(path: Path) -> Result[dict[str, Any]]:
if not is_workflow_json_path(path):
return Result.Err("FORBIDDEN", "Workflow path is not allowed")
@@ -1291,12 +1320,10 @@ def _snapshot_workflow_version(path: Path, *, reason: str) -> Result[dict[str, A
def list_workflow_versions(path: Path) -> Result[dict[str, Any]]:
if not is_managed_workflow_json_path(path):
return Result.Err("FORBIDDEN", "Workflow path is not allowed")
try:
resolved = path.resolve(strict=True)
except FileNotFoundError:
return Result.Err("NOT_FOUND", "Workflow not found")
resolved_check = _resolve_managed_workflow_path(path)
resolved = resolved_check.data
if not resolved_check.ok or resolved is None:
return Result.Err(resolved_check.code or "FORBIDDEN", resolved_check.error or "Workflow path is not allowed")
history_dir = _workflow_history_dir(resolved)
versions: list[dict[str, Any]] = []
if history_dir.is_dir():
@@ -1333,23 +1360,29 @@ def _flatten_json(value: Any, prefix: str = "") -> dict[str, Any]:
def diff_workflow_versions(path: Path, *, version_filepath: Any = "") -> Result[dict[str, Any]]:
if not is_managed_workflow_json_path(path):
return Result.Err("FORBIDDEN", "Workflow path is not allowed")
try:
current_path = path.resolve(strict=True)
except FileNotFoundError:
return Result.Err("NOT_FOUND", "Workflow not found")
resolved_check = _resolve_managed_workflow_path(path)
current_path = resolved_check.data
if not resolved_check.ok or current_path is None:
return Result.Err(resolved_check.code or "FORBIDDEN", resolved_check.error or "Workflow path is not allowed")
versions = list_workflow_versions(current_path)
if not versions.ok:
return versions
requested = str(version_filepath or "").strip()
if requested:
history_root = _workflow_history_dir(current_path)
try:
version_path = Path(requested).resolve(strict=True)
history_prefix = os.path.normcase(os.path.normpath(str(history_root)))
requested_norm = os.path.normcase(os.path.normpath(os.path.abspath(requested)))
except Exception:
return Result.Err("FORBIDDEN", "Workflow version path is not allowed")
if requested_norm != history_prefix and not requested_norm.startswith(history_prefix + os.sep):
return Result.Err("FORBIDDEN", "Workflow version path is not allowed")
try:
version_path = Path(requested_norm).resolve(strict=True)
except FileNotFoundError:
return Result.Err("NOT_FOUND", "Workflow version not found")
history_root = _workflow_history_dir(current_path).resolve(strict=False)
if version_path != history_root and history_root not in version_path.parents:
history_resolved = history_root.resolve(strict=False)
if version_path != history_resolved and history_resolved not in version_path.parents:
return Result.Err("FORBIDDEN", "Workflow version path is not allowed")
else:
items = (versions.data or {}).get("versions") or []
@@ -1707,12 +1740,10 @@ def set_workflow_info(path: Path, *, info: dict[str, Any] | None = None) -> Resu
def list_workflow_thumbnail_candidates(path: Path, *, limit: int = 12) -> Result[list[dict[str, Any]]]:
if not is_managed_workflow_json_path(path):
return Result.Err("FORBIDDEN", "Workflow path is not allowed")
try:
resolved = path.resolve(strict=True)
except FileNotFoundError:
return Result.Err("NOT_FOUND", "Workflow not found")
resolved_check = _resolve_managed_workflow_path(path)
resolved = resolved_check.data
if not resolved_check.ok or resolved is None:
return Result.Err(resolved_check.code or "FORBIDDEN", resolved_check.error or "Workflow path is not allowed")
workflow = _safe_read_workflow_json(resolved)
if workflow is None:
mjr_am_backend/routes/handlers/workflows.py
+21 -6
View File
@@ -188,20 +188,35 @@ async def _audit_workflow_write(
def _resolve_path_under_roots(user_path: Path, safe_roots: list[Path]) -> Result[Path]:
# Lexical containment check before any filesystem access (no existence
# oracle outside the safe roots), then re-validated after symlink
# resolution.
try:
resolved = user_path.resolve(strict=True)
except FileNotFoundError:
return Result.Err("NOT_FOUND", "Thumbnail not found")
candidate = os.path.normcase(os.path.normpath(os.path.abspath(str(user_path))))
except Exception:
return Result.Err("FORBIDDEN", "Thumbnail path is not allowed")
for root in safe_roots:
try:
root_resolved = root.resolve(strict=False)
prefix = os.path.normcase(os.path.normpath(str(root)))
except Exception:
continue
if resolved == root_resolved or resolved.is_relative_to(root_resolved):
return Result.Ok(resolved)
if candidate != prefix and not candidate.startswith(prefix + os.sep):
continue
try:
resolved = Path(candidate).resolve(strict=True)
except FileNotFoundError:
return Result.Err("NOT_FOUND", "Thumbnail not found")
except Exception:
return Result.Err("FORBIDDEN", "Thumbnail path is not allowed")
for allowed_root in safe_roots:
try:
root_resolved = allowed_root.resolve(strict=False)
except Exception:
continue
if resolved == root_resolved or resolved.is_relative_to(root_resolved):
return Result.Ok(resolved)
return Result.Err("FORBIDDEN", "Thumbnail path is not allowed")
return Result.Err("FORBIDDEN", "Thumbnail path is not allowed")
package-lock.json
Generated
+10 -58
View File
@@ -527,9 +527,6 @@
"arm64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -547,9 +544,6 @@
"arm64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [
@@ -567,9 +561,6 @@
"ppc64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -587,9 +578,6 @@
"riscv64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -607,9 +595,6 @@
"riscv64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [
@@ -627,9 +612,6 @@
"s390x"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -647,9 +629,6 @@
"x64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -667,9 +646,6 @@
"x64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [
@@ -882,9 +858,6 @@
"arm64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -899,9 +872,6 @@
"arm64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [
@@ -916,9 +886,6 @@
"ppc64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -933,9 +900,6 @@
"riscv64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -950,9 +914,6 @@
"riscv64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [
@@ -967,9 +928,6 @@
"s390x"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -984,9 +942,6 @@
"x64"
],
"dev": true,
"libc": [
"glibc"
],
"license": "MIT",
"optional": true,
"os": [
@@ -1001,9 +956,6 @@
"x64"
],
"dev": true,
"libc": [
"musl"
],
"license": "MIT",
"optional": true,
"os": [
@@ -2410,16 +2362,16 @@
}
},
"node_modules/brace-expansion": {
"version": "5.0.6",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.6.tgz",
"integrity": "sha512-kLpxurY4Z4r9sgMsyG0Z9uzsBlgiU/EFKhj/h91/8yHu0edo7XuixOIH3VcJ8kkxs6/jPzoI6U9Vj3WqbMQ94g==",
"version": "5.0.8",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-5.0.8.tgz",
"integrity": "sha512-JZyDyq3D4AUifKTPOB7DELf6XsB3WdPuNxCtob1vFXPsSXhdAiHBWJ/tJ8HAc9aH84BK+5JFZLNkJKx3G9kzQg==",
"dev": true,
"license": "MIT",
"dependencies": {
"balanced-match": "^4.0.2"
},
"engines": {
"node": "18 || 20 || >=22"
"node": "20 || >=22"
}
},
"node_modules/buffer-image-size": {
@@ -2598,9 +2550,9 @@
"license": "MIT"
},
"node_modules/editorconfig/node_modules/brace-expansion": {
"version": "2.0.3",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.3.tgz",
"integrity": "sha512-MCV/fYJEbqx68aE58kv2cA/kiky1G8vux3OR6/jbS+jIMe/6fJWa0DTzJU7dqijOWYwHi1t29FlfYI9uytqlpA==",
"version": "2.1.2",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.2.tgz",
"integrity": "sha512-w5JZcKgdhDOgOwm8H+KgbosopHMuGcl6qbulwjtz3SM7I7P3yW1eAjzMPLrIE+NQ9vjgANKHWeMHnrT0OXW1oA==",
"dev": true,
"license": "MIT",
"dependencies": {
@@ -3041,9 +2993,9 @@
"license": "MIT"
},
"node_modules/glob/node_modules/brace-expansion": {
"version": "2.0.3",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.3.tgz",
"integrity": "sha512-MCV/fYJEbqx68aE58kv2cA/kiky1G8vux3OR6/jbS+jIMe/6fJWa0DTzJU7dqijOWYwHi1t29FlfYI9uytqlpA==",
"version": "2.1.2",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.2.tgz",
"integrity": "sha512-w5JZcKgdhDOgOwm8H+KgbosopHMuGcl6qbulwjtz3SM7I7P3yW1eAjzMPLrIE+NQ9vjgANKHWeMHnrT0OXW1oA==",
"dev": true,
"license": "MIT",
"dependencies": {
tests/features/test_workflows_routes.py
+4 -1
View File
@@ -151,8 +151,11 @@ async def test_workflow_save_forwards_info_fields(monkeypatch):
@pytest.mark.asyncio
async def test_workflow_thumbnail_missing_returns_not_found(monkeypatch, tmp_path):
missing = tmp_path / "workflows" / "missing.png"
workflow_dir = tmp_path / "workflows"
workflow_dir.mkdir()
missing = workflow_dir / "missing.png"
monkeypatch.setenv("MJR_AM_WORKFLOW_DIRECTORY", str(workflow_dir))
monkeypatch.setattr(workflows_routes, "is_workflow_thumbnail_path", lambda _path: True)
app = _build_app()