fix(admin): handle symlinked models in delete endpoint

[Bahtya]

Problem

Deleting a symlinked model from the admin dashboard fails with 400: Invalid model name. Symlinked models (e.g. pointing to HuggingFace cache) are valid use cases but the path validation rejects them.

Root Cause

Path.resolve() follows symlinks, so for a symlinked model the resolved path points to the HuggingFace cache directory, which is outside the model directory. The is_relative_to check then fails.

Additionally, shutil.rmtree() would follow the symlink and delete the actual cached files instead of just removing the symlink.

Fix

1. Use absolute() instead of resolve() for path traversal validation — validates the logical path without dereferencing symlinks
2. Handle symlink deletion with unlink() to remove only the symlink, not the target
3. Allow is_symlink() in addition to is_dir() for the directory check

Fixes #646

[jundot]

The symlink deletion part looks good. Using unlink() instead of rmtree() for symlinks is the right call.

The resolve() → absolute() change has a security issue though. absolute() doesn't normalize .., so path traversal passes through:

Path("/models/../../etc").absolute().is_relative_to(Path("/models").absolute())
# True (bad)

Path("/models/../../etc").resolve().is_relative_to(Path("/models").resolve())
# False (safe)

Instead of replacing resolve(), handle symlinks separately in the validation:

if model_path.is_symlink():
    link_parent = model_path.parent.resolve()
    if not link_parent.is_relative_to(parent_model_dir.resolve()):
        raise HTTPException(status_code=400, detail="Invalid model name")
else:
    if not model_path.resolve().is_relative_to(parent_model_dir.resolve()):
        raise HTTPException(status_code=400, detail="Invalid model name")

This validates the link's location without following it, while keeping .. protection.

The is_dir()/is_symlink() check and the unlink() deletion branch are fine as-is. Could you update just the path validation? A test for symlink deletion would also be nice.

[Bahtya]

Thank you for the thorough review! You're absolutely right about the security concern with absolute() — the path traversal would slip through. Your suggested approach of handling symlinks separately in the validation is much cleaner. I'll push an update shortly with the revised path validation and add a test for symlink deletion.

[jundot]

Hey, thanks for working on this. The symlink deletion logic (unlink() vs rmtree() branching) looks correct.

The main issue is that the path traversal validation on line 3051 still uses resolve():

if not model_path.resolve().is_relative_to(parent_model_dir.resolve()):
    raise HTTPException(status_code=400, detail="Invalid model name")

Since resolve() follows symlinks, this check rejects symlinked models before the code ever reaches the is_symlink() check or the unlink() deletion path. The bug from #646 is still reproducible as-is.

The fix I suggested in the previous review (handling symlinks separately in validation) would solve this:

if model_path.is_symlink():
    link_parent = model_path.parent.resolve()
    if not link_parent.is_relative_to(parent_model_dir.resolve()):
        raise HTTPException(status_code=400, detail="Invalid model name")
else:
    if not model_path.resolve().is_relative_to(parent_model_dir.resolve()):
        raise HTTPException(status_code=400, detail="Invalid model name")

Also the PR description mentions using absolute() instead of resolve() but the diff doesn't include that change. Might want to update the description to match.

A test for symlink deletion would be great too.