Web UI #634

hasan7n · 2024-12-21T21:09:43Z

Comments/discussions to check:

To be merged before merging to main: #618, which depends on #615

… not updated)

github-actions · 2024-12-21T21:09:57Z

MLCommons CLA bot:
Thank you very much for your submission, we really appreciate it. Before we can accept your contribution, we ask that you sign the MLCommons CLA (Apache 2). Please use this [Google form] (https://forms.gle/Ew1KkBVpyeJDuRw67) to initiate authorization. If you are from an MLCommons member organization, we will request that you be added to the CLA. If you are not from a member organization, we will email you a CLA to sign. For any questions, please contact support@mlcommons.org.
3 out of 4 committers have signed the MLCommons CLA.
✅ @aristizabal95
✅ @mhmdk0
✅ @hasan7n
❌ @VukW
_{You can retrigger this bot by commenting recheck in this Pull Request}

cli/medperf/web_ui/api/routes.py

+    full_path = os.path.join(BASE_DIR, path)
+    os.path.join(BASE_DIR, path)
+
+    if not os.path.exists(full_path) or not os.path.isdir(full_path):


To fix the problem, we need to ensure that the full_path is normalized before performing any checks. This will remove any ".." segments and ensure that the path is correctly contained within the BASE_DIR. We will use os.path.normpath to normalize the path and then check if the normalized path starts with the BASE_DIR.

cli/medperf/web_ui/api/routes.py

+    full_path = os.path.join(BASE_DIR, path)
+    os.path.join(BASE_DIR, path)
+
+    if not os.path.exists(full_path) or not os.path.isdir(full_path):


To fix the problem, we need to ensure that the full_path is normalized before checking if it is within the BASE_DIR. This can be done using os.path.normpath to remove any ".." segments that could allow directory traversal. After normalizing the path, we should check if it starts with the BASE_DIR.

Normalize the full_path using os.path.normpath.

Check if the normalized full_path starts with BASE_DIR to ensure it is within the base directory.

Update the existing checks to use the normalized path.

cli/medperf/web_ui/api/routes.py

+
+    # List directories inside the path
+    folders = []
+    for item in os.listdir(full_path):


To fix the problem, we need to ensure that the full_path is properly validated and sanitized before it is used in any file system operations. The best way to do this is to normalize the path using os.path.normpath and then check that the normalized path starts with the BASE_DIR. This will prevent any path traversal attacks and ensure that the path is within the intended directory.

cli/medperf/web_ui/api/routes.py

+    folders = []
+    for item in os.listdir(full_path):
+        item_path = os.path.join(full_path, item)
+        if os.path.isdir(item_path):


To fix the problem, we need to ensure that the full_path is normalized before checking if it is within the BASE_DIR. This can be done using os.path.normpath to remove any ".." segments and ensure the path is safe. Additionally, we should ensure that the item_path is also validated properly.

Normalize the full_path using os.path.normpath.

Check if the normalized full_path starts with BASE_DIR.

Ensure that item_path is also validated properly.

cli/medperf/web_ui/login.py

+    # Check if user is already authenticated
+    if token == security_token:
+        # User is already authenticated, redirect to original URL
+        return RedirectResponse(url=redirect_url, status_code=status.HTTP_302_FOUND)


To fix the problem, we need to validate the redirect_url parameter to ensure it does not contain an explicit host name, which could lead to an open redirect vulnerability. We can use the urlparse function from the Python standard library to parse the URL and check that the netloc attribute is empty. Additionally, we should replace backslashes with forward slashes to handle cases where browsers accept backslashes as equivalent to forward slashes.

We will make changes to the login_form and login functions to include this validation.

cli/medperf/web_ui/login.py

+        redirect_url: str = Form("/"),
+):
+    if token == security_token:
+        response = RedirectResponse(url=redirect_url, status_code=status.HTTP_302_FOUND)


To fix the problem, we need to validate the redirect_url parameter to ensure it does not contain an explicit host name, which could lead to open redirect vulnerabilities. We can use the urlparse function from the Python standard library to parse the URL and check that the netloc attribute is empty. Additionally, we should replace backslashes with forward slashes to handle browser quirks.

Add the necessary import for urlparse.

Modify the login function to validate the redirect_url parameter before using it in the redirect response.

cli/medperf/web_ui/login.py

+):
+    if token == security_token:
+        response = RedirectResponse(url=redirect_url, status_code=status.HTTP_302_FOUND)
+        response.set_cookie(key=AUTH_COOKIE_NAME, value=token)


cli/medperf/web_ui/login.py

+):
+    if token == security_token:
+        response = RedirectResponse(url=redirect_url, status_code=status.HTTP_302_FOUND)
+        response.set_cookie(key=AUTH_COOKIE_NAME, value=token)


VukW added 30 commits July 11, 2024 10:34

added fastapi as web ui backend

040d446

Added cube + benchmark basic listing

97e6bd7

Adds navigation

0382684

Aded mlcube detailed page

55fe60e

Improved mlcubes detailed layout

fb1bca3

Improved mlcube layout

64cf53e

yaml displaying

36611e1

yaml: spinner

56fa5c4

yaml panel improvement

8563887

yaml panel layout improvement

07ce4ab

layout fixes

b260401

Added benchmark detailed page

b7980a8

added links to mlcube

ca356cc

benchmark page: added owner

6efd724

Colors refactoring

319b1bf

Dataset detailed page

58008f3

Forgot to add js file

375d89e

Unified data format for all data fields automatically

c6d8a56

(mlcube-detailed) Display image tarball and additional files always

74f7743

Fixed scrolling and reinvented basic page layout

b312882

Fix navbar is hiding

0e282cb

Make templates & static files independent of user's workdir

6b28ebb

Added error handling

881b281

Display invalid entities correctly

e28107b

Added invalid entities highlighting + badges

5b718eb

Added benchmark associations

0f95027

Improved association panel style

444786e

Added association card

e273577

Sorted associations by status / timestamp

eea1e77

Sorted mlcubes and datasets: mine first

7b68911

VukW and others added 15 commits October 15, 2024 19:28

Restyled model list

4926f07

Updated models list layout

4b45f49

Restyled models list

35ded73

Restyled the run buttons

2ab585d

Added "Running" state

a7fdd52

"Run all" button

d9d2932

removed unused code

397aed4

minor bugfixes

7780bc0

Result submission

7d20e31

bugfix: status was passed wrongly if result is submitted (as draft is…

2a1d55d

… not updated)

Auth by security token

48e388e

Restyled dataset pipeline buttons

23908f5

Merge remote-tracking branch 'origin/main' into webui-dataset

020da3e

Merge remote-tracking branch 'upstream/main' into web-ui

e59d877

Merge remote-tracking branch 'upstream/web-ui' into webui-dataset

88c5eb0

hasan7n requested a review from a team as a code owner December 21, 2024 21:09

hasan7n had a problem deploying to testing-external-code December 21, 2024 21:09 — with GitHub Actions Failure

github-advanced-security bot found potential problems Dec 21, 2024

View reviewed changes

mhmdk0 added 10 commits January 7, 2025 00:06

temp

e265382

temp 1

d4e7151

fix dataset submission and preparation

e636cdd

set operation temp

79e25e6

refactoring dataset - temp

72b2eef

temp

2bdd9f4

finalize dataset

638fc5e

include bootstrap 5, update old ignored 'non commited' files

37b5106

Finalize Dataset

bb7a5ae

finalize model owner

a21dea0

mhmdk0 requested a deployment to testing-external-code January 28, 2025 00:29 — with GitHub Actions Waiting

@@ -18,11 +18,10 @@
             ):
-                full_path = os.path.join(BASE_DIR, path)
-                os.path.join(BASE_DIR, path)
-                if not os.path.exists(full_path) or not os.path.isdir(full_path):
-                    raise HTTPException(status_code=404, detail="Directory not found")
-                # Ensure path is within the base directory
-                if not os.path.commonpath([BASE_DIR, full_path]).startswith(BASE_DIR):
-                    raise HTTPException(status_code=403, detail="Access denied")
+                full_path = os.path.normpath(os.path.join(BASE_DIR, path))
+                # Ensure path is within the base directory
+                if not full_path.startswith(BASE_DIR):
+                    raise HTTPException(status_code=403, detail="Access denied")
+                if not os.path.exists(full_path) or not os.path.isdir(full_path):
+                    raise HTTPException(status_code=404, detail="Directory not found")

@@ -18,11 +18,9 @@
             ):
-                full_path = os.path.join(BASE_DIR, path)
-                os.path.join(BASE_DIR, path)
-                if not os.path.exists(full_path) or not os.path.isdir(full_path):
-                    raise HTTPException(status_code=404, detail="Directory not found")
-                # Ensure path is within the base directory
-                if not os.path.commonpath([BASE_DIR, full_path]).startswith(BASE_DIR):
-                    raise HTTPException(status_code=403, detail="Access denied")
+                full_path = os.path.normpath(os.path.join(BASE_DIR, path))
+                if not full_path.startswith(BASE_DIR):
+                    raise HTTPException(status_code=403, detail="Access denied")
+                if not os.path.exists(full_path) or not os.path.isdir(full_path):
+                    raise HTTPException(status_code=404, detail="Directory not found")

@@ -18,11 +18,10 @@
             ):
-                full_path = os.path.join(BASE_DIR, path)
-                os.path.join(BASE_DIR, path)
-                if not os.path.exists(full_path) or not os.path.isdir(full_path):
-                    raise HTTPException(status_code=404, detail="Directory not found")
-                # Ensure path is within the base directory
-                if not os.path.commonpath([BASE_DIR, full_path]).startswith(BASE_DIR):
-                    raise HTTPException(status_code=403, detail="Access denied")
+                full_path = os.path.normpath(os.path.join(BASE_DIR, path))
+                # Ensure path is within the base directory
+                if not full_path.startswith(BASE_DIR):
+                    raise HTTPException(status_code=403, detail="Access denied")
+                if not os.path.exists(full_path) or not os.path.isdir(full_path):
+                    raise HTTPException(status_code=404, detail="Directory not found")

@@ -18,18 +18,17 @@
             ):
-                full_path = os.path.join(BASE_DIR, path)
-                os.path.join(BASE_DIR, path)
-                if not os.path.exists(full_path) or not os.path.isdir(full_path):
-                    raise HTTPException(status_code=404, detail="Directory not found")
-                # Ensure path is within the base directory
-                if not os.path.commonpath([BASE_DIR, full_path]).startswith(BASE_DIR):
-                    raise HTTPException(status_code=403, detail="Access denied")
-                # List directories inside the path
-                folders = []
-                for item in os.listdir(full_path):
-                    item_path = os.path.join(full_path, item)
-                    if os.path.isdir(item_path):
-                        folders.append({"name": item, "path": os.path.relpath(item_path, BASE_DIR)})
+                full_path = os.path.normpath(os.path.join(BASE_DIR, path))
+                if not os.path.exists(full_path) or not os.path.isdir(full_path):
+                    raise HTTPException(status_code=404, detail="Directory not found")
+                # Ensure path is within the base directory
+                if not full_path.startswith(BASE_DIR):
+                    raise HTTPException(status_code=403, detail="Access denied")
+                # List directories inside the path
+                folders = []
+                for item in os.listdir(full_path):
+                    item_path = os.path.normpath(os.path.join(full_path, item))
+                    if item_path.startswith(full_path) and os.path.isdir(item_path):
+                        folders.append({"name": item, "path": os.path.relpath(item_path, BASE_DIR)})

@@ -1,3 +1,4 @@
-            from fastapi import Request, Form, APIRouter, status, Security
-            from fastapi.responses import HTMLResponse, RedirectResponse
+            from fastapi import Request, Form, APIRouter, status, Security
+            from fastapi.responses import HTMLResponse, RedirectResponse
+            from urllib.parse import urlparse
@@ -11,14 +12,20 @@
             @router.get("/login", response_class=HTMLResponse)
-            def login_form(
-                request: Request, redirect_url: str = "/", token: str = Security(api_key_cookie)
-            ):
-                # Check if user is already authenticated
-                if token == security_token:
-                    # User is already authenticated, redirect to original URL
-                    return RedirectResponse(url=redirect_url, status_code=status.HTTP_302_FOUND)
-                else:
-                    # User is not authenticated, show login form
-                    return templates.TemplateResponse(
-                        "login.html", {"request": request, "redirect_url": redirect_url}
-                    )
+            def login_form(
+                request: Request, redirect_url: str = "/", token: str = Security(api_key_cookie)
+            ):
+                # Validate redirect_url
+                redirect_url = redirect_url.replace('\\', '/')
+                if not urlparse(redirect_url).netloc and not urlparse(redirect_url).scheme:
+                    valid_redirect_url = redirect_url
+                else:
+                    valid_redirect_url = '/'
+                # Check if user is already authenticated
+                if token == security_token:
+                    # User is already authenticated, redirect to original URL
+                    return RedirectResponse(url=valid_redirect_url, status_code=status.HTTP_302_FOUND)
+                else:
+                    # User is not authenticated, show login form
+                    return templates.TemplateResponse(
+                        "login.html", {"request": request, "redirect_url": valid_redirect_url}
+                    )
@@ -27,19 +34,25 @@
             @router.post("/login")
-            def login(
-                request: Request,
-                token: str = Form(...),
-                redirect_url: str = Form("/"),
-            ):
-                if token == security_token:
-                    response = RedirectResponse(url=redirect_url, status_code=status.HTTP_302_FOUND)
-                    response.set_cookie(key=AUTH_COOKIE_NAME, value=token)
-                    return response
-                else:
-                    return templates.TemplateResponse(
-                        "login.html",
-                        {
-                            "request": request,
-                            "redirect_url": redirect_url,
-                            "error": "Invalid token",
-                        },
-                    )
+            def login(
+                request: Request,
+                token: str = Form(...),
+                redirect_url: str = Form("/"),
+            ):
+                # Validate redirect_url
+                redirect_url = redirect_url.replace('\\', '/')
+                if not urlparse(redirect_url).netloc and not urlparse(redirect_url).scheme:
+                    valid_redirect_url = redirect_url
+                else:
+                    valid_redirect_url = '/'
+                if token == security_token:
+                    response = RedirectResponse(url=valid_redirect_url, status_code=status.HTTP_302_FOUND)
+                    response.set_cookie(key=AUTH_COOKIE_NAME, value=token)
+                    return response
+                else:
+                    return templates.TemplateResponse(
+                        "login.html",
+                        {
+                            "request": request,
+                            "redirect_url": valid_redirect_url,
+                            "error": "Invalid token",
+                        },
+                    )

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Web UI #634

Web UI #634

hasan7n commented Dec 21, 2024 •

edited

Loading

github-actions bot commented Dec 21, 2024

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Web UI #634

Are you sure you want to change the base?

Web UI #634

Conversation

hasan7n commented Dec 21, 2024 • edited Loading

github-actions bot commented Dec 21, 2024

hasan7n commented Dec 21, 2024 •

edited

Loading