diff --git a/mjr_am_backend/features/workflows/service.py b/mjr_am_backend/features/workflows/service.py index 56463a04..affae9d9 100644 --- a/mjr_am_backend/features/workflows/service.py +++ b/mjr_am_backend/features/workflows/service.py @@ -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: diff --git a/mjr_am_backend/routes/handlers/workflows.py b/mjr_am_backend/routes/handlers/workflows.py index 25535491..e1f726d8 100644 --- a/mjr_am_backend/routes/handlers/workflows.py +++ b/mjr_am_backend/routes/handlers/workflows.py @@ -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") diff --git a/package-lock.json b/package-lock.json index e4f63e6e..f2abc89e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -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": { diff --git a/tests/features/test_workflows_routes.py b/tests/features/test_workflows_routes.py index 531fb388..c4d4dd9e 100644 --- a/tests/features/test_workflows_routes.py +++ b/tests/features/test_workflows_routes.py @@ -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()