Skip to content

Commit 653e162

Browse files
committed
Merge branch 'hotfix/dashboard_path' into 'export/v1-2-0'
prevent file traversal based on input model_id (dashboard) See merge request onecomp/onecomp-lab!87
2 parents ad56a19 + 73e6375 commit 653e162

3 files changed

Lines changed: 83 additions & 18 deletions

File tree

dashboard/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ See [troubleshooting in docs/setup-hpc.md](docs/setup-hpc.md#5-troubleshooting)
343343
| SSH tunnel established but API unreachable | Point `LocalForward` at the GPU node name |
344344
| Chat is slow / keeps polling | `ONECOMP_DEVICE=cuda`, then Stop → Deploy |
345345
| Deploy fails: `Could not find nvcc` | `VLLM_USE_FLASHINFER_SAMPLER=0` on the **worker**, restart worker, Stop → Deploy ([#10](docs/setup-hpc.md#10-vllm-deploy-fails--could-not-find-nvcc)) |
346-
| Local model name fails / “not a local folder” on HF | Set the same `LOCAL_MODEL_ROOT` on **worker and API**, restart both; model dir must be `{LOCAL_MODEL_ROOT}/<name>/` |
346+
| Local model name fails / “not a local folder” on HF | Set the same `LOCAL_MODEL_ROOT` on **worker and API**, restart both; model dir must be `{LOCAL_MODEL_ROOT}/<name>/` ([#11](docs/setup-hpc.md#11-local-model-name-fails--not-a-local-folder-on-hugging-face)) |
347347

348348
### Separating the vLLM venv
349349

dashboard/backend/app/services/huggingface.py

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,19 @@ def _local_model_root() -> Path:
2525
).resolve()
2626

2727

28-
def _local_model_dir(model_id: str) -> Path | None:
29-
"""Return the resolved local model directory when it exists under the allowed root."""
28+
def _resolve_local_model_path(identifier: str) -> Path | None:
29+
"""Return a model directory under LOCAL_MODEL_ROOT, or None if not found."""
3030
root = _local_model_root()
31-
candidate = (root / model_id).resolve()
31+
candidate = Path(identifier.strip())
32+
if candidate.is_absolute():
33+
resolved = candidate.resolve()
34+
else:
35+
resolved = (root / identifier).resolve()
3236
try:
33-
candidate.relative_to(root)
37+
resolved.relative_to(root)
3438
except ValueError:
3539
return None
36-
return candidate if candidate.is_dir() else None
40+
return resolved if resolved.is_dir() else None
3741

3842

3943
def resolve_model_identifier(model_id: str) -> str:
@@ -48,11 +52,7 @@ def resolve_model_identifier(model_id: str) -> str:
4852
if not normalized:
4953
raise ValueError("Model name must not be empty.")
5054

51-
absolute = Path(normalized)
52-
if absolute.is_absolute() and absolute.is_dir():
53-
return str(absolute.resolve())
54-
55-
local_dir = _local_model_dir(normalized)
55+
local_dir = _resolve_local_model_path(normalized)
5656
if local_dir is not None:
5757
return str(local_dir)
5858
return normalized
@@ -65,9 +65,11 @@ def check_model_exists(model_id: str) -> None:
6565
treated as a valid local identifier. This matches the deployment layout
6666
where already-downloaded models live on shared storage.
6767
"""
68-
model_id = resolve_model_identifier(model_id)
68+
model_id = model_id.strip()
69+
if not model_id:
70+
raise ValueError("Model name must not be empty.")
6971

70-
if Path(model_id).is_dir():
72+
if _resolve_local_model_path(model_id) is not None:
7173
return
7274

7375
token = os.environ.get("HF_TOKEN") or os.environ.get("HUGGING_FACE_HUB_TOKEN")

dashboard/docs/setup-hpc.md

Lines changed: 68 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ Environment variables with the `ONECOMP_` prefix take precedence over
104104
pkill -f start_worker.py
105105
cd backend && . .venv/bin/activate
106106
export ONECOMP_DEVICE=cuda
107+
export LOCAL_MODEL_ROOT="$(pwd)/models"
107108
# also set this if you did not edit config.py
108109
export ONECOMP_VLLM_PYTHON="$(pwd)/.venv-vllm/bin/python"
109110
nohup uv run python start_worker.py > /tmp/worker.log 2>&1 &
@@ -174,6 +175,9 @@ mkdir -p tmp/redis
174175
cd backend/
175176
. .venv/bin/activate
176177
export ONECOMP_DEVICE=cuda
178+
# Local model root — required on worker **and** API when using short local names
179+
# instead of a Hugging Face repo id. See §4.
180+
export LOCAL_MODEL_ROOT="$(pwd)/models"
177181
# On nodes without nvcc (typical HPC); see Troubleshooting #10
178182
export VLLM_USE_FLASHINFER_SAMPLER=0
179183
nohup uv run python start_worker.py > /tmp/worker.log 2>&1 &
@@ -182,7 +186,11 @@ nohup uv run python start_worker.py > /tmp/worker.log 2>&1 &
182186
### 2.4 Start the backend API
183187

184188
```bash
185-
ONECOMP_DEVICE=cuda python start_backend.py --reload --port 8001
189+
cd backend/
190+
. .venv/bin/activate
191+
export ONECOMP_DEVICE=cuda
192+
export LOCAL_MODEL_ROOT="$(pwd)/models"
193+
uv run python start_backend.py --reload --port 8001
186194
```
187195

188196
Binding to port 8001 lets the frontend reach the GPU-node API.
@@ -348,7 +356,10 @@ old code.
348356

349357
```bash
350358
pkill -f start_worker.py
351-
ONECOMP_DEVICE=cuda nohup uv run python start_worker.py > /tmp/worker.log 2>&1 &
359+
cd backend && . .venv/bin/activate
360+
export ONECOMP_DEVICE=cuda
361+
export LOCAL_MODEL_ROOT="$(pwd)/models"
362+
nohup uv run python start_worker.py > /tmp/worker.log 2>&1 &
352363
tail -f /tmp/worker.log # look for "Starting vLLM" on the next deploy
353364
```
354365

@@ -362,6 +373,7 @@ Everything runs as processes on the GPU node.
362373
| Item | Location | Setting |
363374
|---|---|---|
364375
| Job metadata | `backend/onecomp.db` (SQLite) | `ONECOMP_DATABASE_URL` (default `sqlite:///./onecomp.db`) |
376+
| Pre-downloaded models (local names) | `backend/models/<name>/` | `LOCAL_MODEL_ROOT` (default `/models`; use `$(pwd)/models` from `backend/`) |
365377
| Quantized models | `backend/tmp/quantized/<job-id>/` | `ONECOMP_QUANTIZED_DIR` (default `tmp/quantized`) |
366378

367379
`database.py` sets `check_same_thread=False` on SQLite so that FastAPI
@@ -415,8 +427,33 @@ To work around HPC HTTP proxies, `inference.py` appends
415427

416428
## 4. Environment variables
417429

418-
All use the `ONECOMP_` prefix. Defaults live in
419-
`backend/app/core/config.py` and can be overridden via env vars.
430+
`ONECOMP_*` settings default in `backend/app/core/config.py`. Other variables
431+
below are read directly from the process environment.
432+
433+
### `LOCAL_MODEL_ROOT` (local model directory)
434+
435+
Set this **before starting both the Celery worker and the API** when jobs use
436+
short local directory names (e.g. `gemma-2-2b-it`) rather than a Hugging Face
437+
repo id (`org/model`). The server maps `model_name` to
438+
`{LOCAL_MODEL_ROOT}/<model_name>` for job validation and quantization.
439+
440+
| Variable | Default | Description |
441+
|---|---|---|
442+
| `LOCAL_MODEL_ROOT` | `/models` | Root directory of pre-downloaded models on shared storage |
443+
444+
Example (run from `backend/`; models live in `backend/models/`):
445+
446+
```bash
447+
export LOCAL_MODEL_ROOT="$(pwd)/models"
448+
```
449+
450+
Use the **same value** in the worker terminal (§2.3) and API terminal (§2.4).
451+
If only one process has it, jobs may pass validation but fail during
452+
quantization with “not a local folder” / Hugging Face Hub errors.
453+
454+
After changing `LOCAL_MODEL_ROOT`, restart **both** processes.
455+
456+
### `ONECOMP_*` (application settings)
420457

421458
| Variable | Default | Description |
422459
|---|---|---|
@@ -626,6 +663,7 @@ Either way, restart the worker:
626663
pkill -f start_worker.py
627664
cd backend && . .venv/bin/activate
628665
export ONECOMP_DEVICE=cuda
666+
export LOCAL_MODEL_ROOT="$(pwd)/models"
629667
# only when using a separated venv:
630668
# export ONECOMP_VLLM_PYTHON="$(pwd)/.venv-vllm/bin/python"
631669
nohup uv run python start_worker.py > /tmp/worker.log 2>&1 &
@@ -668,6 +706,7 @@ export VLLM_USE_FLASHINFER_SAMPLER=0
668706
pkill -f start_worker.py
669707
cd backend && . .venv/bin/activate
670708
export ONECOMP_DEVICE=cuda
709+
export LOCAL_MODEL_ROOT="$(pwd)/models"
671710
export VLLM_USE_FLASHINFER_SAMPLER=0
672711
nohup uv run python start_worker.py > /tmp/worker.log 2>&1 &
673712
```
@@ -727,7 +766,7 @@ export ONECOMP_VLLM_PYTHON="$(pwd)/.venv-vllm/bin/python"
727766
```
728767

729768
4. `pkill -f start_worker.py`, then restart the worker with
730-
`ONECOMP_DEVICE=cuda`
769+
`ONECOMP_DEVICE=cuda` and `LOCAL_MODEL_ROOT="$(pwd)/models"` (§4)
731770
5. **Stop → Deploy**. Verify `tail /tmp/worker.log` shows
732771
`.venv-vllm/bin/python` in the command line
733772

@@ -747,3 +786,27 @@ export ONECOMP_VLLM_PYTHON="$(pwd)/.venv-vllm/bin/python"
747786
Removing `vllm>=0.21.0` from `pyproject.toml` and re-running `uv sync`
748787
leaves the main venv with `onecomp` only and the `.venv-vllm` with vLLM
749788
only.
789+
790+
### #11: Local model name fails / “not a local folder” on Hugging Face
791+
792+
**Symptom**:
793+
794+
- Job uses a short local name (e.g. `gemma-2-2b-it`) instead of
795+
`org/model`
796+
- Validation or quantization fails with Hugging Face Hub / “not a local
797+
folder” errors
798+
- Default `LOCAL_MODEL_ROOT` is `/models`, which may not exist on the node
799+
800+
**Fix**:
801+
802+
1. Place the model under `backend/models/<name>/` (or your shared storage
803+
layout)
804+
2. Export the **same** root on **both** worker and API (from `backend/`):
805+
806+
```bash
807+
export LOCAL_MODEL_ROOT="$(pwd)/models"
808+
```
809+
810+
3. Restart worker (§2.3) and API (§2.4)
811+
812+
See **§4** for details.

0 commit comments

Comments
 (0)