Improved Vite Env Configs and Formatted #30
Conversation
muhammad-fiaz
commented
Mar 3, 2025
- Improved Vite Env Configs and Formatted
…rror handling in the App component
There was a problem hiding this comment.
PR Overview
This PR refactors the asset injection process, updates the Vite configuration for production environments, and cleans up API route definitions and type definitions in both the frontend and backend. Key changes include:
- Refactored asset injection and logging in the frontend hooks.
- Updated FastAPI endpoints and route definitions in the webflow modules.
- Modified Vite configuration to use the react-swc plugin along with production-specific settings.
Reviewed Changes
| File | Description |
|---|---|
| packages/frontend/src/styles.ts | Added new style imports. |
| packages/frontend/src/hooks/assets.ts | Refactored to use a single async asset injection function with improved logging. |
| webflow/modules/webflow_api.py | Introduced new API endpoints and initialization logic for serving static assets and HTML. |
| webflow/modules/routes.py | Updated route definitions to leverage the WebFlow_API class directly and simplify responses. |
| packages/frontend/src/Layout.tsx | Added fallback logic for configuration values and enhanced error handling when fetching config. |
| packages/frontend/src/App.tsx | Added loading and error states on sidebar API calls. |
| packages/frontend/src/components/InjectedHtml.tsx | Switched from dangerouslySetInnerHTML to sanitized HTML parsing using DOMPurify and html-react-parser. |
| packages/frontend/src/Root.tsx | Updated imports to remove duplicate style imports and adjusted config fetching logic. |
| packages/frontend/vite.config.ts | Changed to react-swc and added production-specific proxy and minification options. |
| webflow/modules/types.py | Adjusted type definitions for sidebar-related models to be more flexible. |
Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
webflow/modules/webflow_api.py:83
- Consider renaming the 'sidebar' method to 'setSidebar' to avoid confusion with the 'sidebar' property and improve clarity.
def sidebar(visible: bool, label: str, default_open: bool, items: List[Dict[str, str]]):
packages/frontend/vite.config.ts:26
- The 'cssMinify' option is not a standard Vite configuration option and might be ignored; verify its validity or remove it if unnecessary.
cssMinify: 'esbuild',
| logly.warn("Static directory not set.") | ||
| raise HTTPException(status_code=500, detail="Static directory not set") | ||
| file_path = Path(cls.static_dir) / filename | ||
| if file_path.exists(): |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 months ago
To fix the problem, we need to validate and sanitize the user-provided filename before using it to construct file paths. The best way to do this is to normalize the path and ensure it is contained within the static directory. This can be achieved by using os.path.normpath to remove any ".." segments and then checking that the normalized path starts with the static directory.
- Normalize the
filenameusingos.path.normpath. - Ensure that the normalized path starts with the static directory.
- If the path is valid, proceed to serve the file; otherwise, raise an HTTP exception.
| @@ -1,2 +1,3 @@ | ||
| import datetime | ||
| import os | ||
| from pathlib import Path | ||
| @@ -155,3 +156,7 @@ | ||
| raise HTTPException(status_code=500, detail="Static directory not set") | ||
| file_path = Path(cls.static_dir) / filename | ||
| normalized_path = Path(os.path.normpath(filename)) | ||
| file_path = Path(cls.static_dir) / normalized_path | ||
| if not str(file_path.resolve()).startswith(str(Path(cls.static_dir).resolve())): | ||
| logly.warn(f"Attempt to access file outside static directory: {filename}") | ||
| raise HTTPException(status_code=400, detail="Invalid file path") | ||
| if file_path.exists(): |
| elif file_path.suffix == ".html": | ||
| media_type = "text/html" | ||
| headers = {"Cache-Control": "no-cache, no-store, must-revalidate"} | ||
| return FileResponse(str(file_path), media_type=media_type, headers=headers) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 months ago
To fix the problem, we need to ensure that the constructed file path is contained within a safe root directory. This can be achieved by normalizing the path using os.path.normpath and then checking that the normalized path starts with the root directory. This will prevent path traversal attacks.
- Normalize the path using
os.path.normpath. - Check that the normalized path starts with the root directory (
cls.static_dir). - If the check fails, raise an HTTP exception.
| @@ -156,12 +156,16 @@ | ||
| file_path = Path(cls.static_dir) / filename | ||
| if file_path.exists(): | ||
| normalized_path = file_path.resolve() | ||
| if not str(normalized_path).startswith(str(Path(cls.static_dir).resolve())): | ||
| logly.warn(f"Attempt to access file outside of static directory: {filename}") | ||
| raise HTTPException(status_code=403, detail="Access to the requested file is forbidden") | ||
| if normalized_path.exists(): | ||
| media_type = "application/octet-stream" | ||
| if file_path.suffix == ".css": | ||
| if normalized_path.suffix == ".css": | ||
| media_type = "text/css" | ||
| elif file_path.suffix == ".js": | ||
| elif normalized_path.suffix == ".js": | ||
| media_type = "application/javascript" | ||
| elif file_path.suffix == ".html": | ||
| elif normalized_path.suffix == ".html": | ||
| media_type = "text/html" | ||
| headers = {"Cache-Control": "no-cache, no-store, must-revalidate"} | ||
| return FileResponse(str(file_path), media_type=media_type, headers=headers) | ||
| return FileResponse(str(normalized_path), media_type=media_type, headers=headers) | ||
| else: |