Reject booleans for the numeric override fields
Pydantic parses non-strictly and bool subclasses int, so a payload with max_seq_length true was stored as 1, a one-token context, and gpu_ids [true] as [1], an unintended GPU pin. _bounded_int already rejects bools for exactly that reason, but never saw one: coercion happens at the route boundary first, which left that guard unreachable through this path. A mode=before validator rejects only booleans, including inside the gpu_ids list, so every other lax conversion still runs and tensor_parallel, remove and fill_absent_fields keep working.
This commit is contained in:
parent
bd5089d62b
commit
79ddfa2ef4
2 changed files with 67 additions and 1 deletions
|
|
@ -2,7 +2,7 @@
|
|||
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
|
||||
|
||||
import re
|
||||
from typing import Literal, Optional
|
||||
from typing import Any, Literal, Optional
|
||||
from urllib.parse import unquote, urlsplit
|
||||
|
||||
from fastapi import APIRouter, Depends, HTTPException
|
||||
|
|
@ -209,6 +209,29 @@ class ModelOverridePayload(BaseModel):
|
|||
raise ValueError(f"Chat template exceeds the {MAX_CHAT_TEMPLATE_BYTES}-byte limit.")
|
||||
return value
|
||||
|
||||
@field_validator(
|
||||
"max_seq_length",
|
||||
"custom_context_length",
|
||||
"spec_draft_n_max",
|
||||
"n_parallel",
|
||||
"gpu_layers",
|
||||
"n_cpu_moe",
|
||||
"gpu_ids",
|
||||
mode = "before",
|
||||
)
|
||||
@classmethod
|
||||
def _no_booleans(cls, value: Any) -> Any:
|
||||
# bool subclasses int and pydantic parses non-strictly, so `true` arrives
|
||||
# as 1 and `false` as 0: a payload could pin GPU 1 or set a one-token
|
||||
# context. _bounded_int in the normalizer rejects bools for exactly that
|
||||
# reason, but never sees one, because coercion happens here first. Reject
|
||||
# only bools, so every other lax conversion the field relies on still runs.
|
||||
if isinstance(value, bool):
|
||||
raise ValueError("Expected a number, got a boolean.")
|
||||
if isinstance(value, list) and any(isinstance(item, bool) for item in value):
|
||||
raise ValueError("Expected numbers, got a boolean.")
|
||||
return value
|
||||
|
||||
|
||||
class ModelOverridesResponse(BaseModel):
|
||||
overrides: dict[str, dict]
|
||||
|
|
|
|||
|
|
@ -5627,3 +5627,46 @@ def test_a_fill_does_not_replay_a_stored_flag_through_validation(monkeypatch):
|
|||
"tester",
|
||||
)
|
||||
assert excinfo.value.status_code == 400
|
||||
|
||||
|
||||
def test_override_payload_rejects_booleans_for_numeric_fields():
|
||||
"""bool subclasses int and pydantic parses non-strictly, so `true` would
|
||||
arrive as 1: `max_seq_length: true` becomes a one-token context and
|
||||
`gpu_ids: [true]` pins GPU 1. _bounded_int rejects bools for exactly that
|
||||
reason, but never sees one, because coercion happens at the route boundary
|
||||
first. Reject them there so that guard is reachable through this path."""
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
from routes.settings import ModelOverridePayload
|
||||
|
||||
for field, value in (
|
||||
("max_seq_length", True),
|
||||
("custom_context_length", True),
|
||||
("spec_draft_n_max", True),
|
||||
("n_parallel", True),
|
||||
("gpu_layers", False),
|
||||
("n_cpu_moe", True),
|
||||
("gpu_ids", [True]),
|
||||
("gpu_ids", [0, False, 2]),
|
||||
):
|
||||
with pytest.raises(ValidationError):
|
||||
ModelOverridePayload(model_id = "unsloth/x-GGUF:Q4_K_M", **{field: value})
|
||||
|
||||
# Only bools: every real value the picker sends still validates, and the
|
||||
# fields that ARE booleans keep working.
|
||||
ok = ModelOverridePayload(
|
||||
model_id = "unsloth/x-GGUF:Q4_K_M",
|
||||
max_seq_length = 4096,
|
||||
gpu_layers = -1,
|
||||
n_cpu_moe = 0,
|
||||
gpu_ids = [0, 1],
|
||||
tensor_parallel = True,
|
||||
remove = True,
|
||||
fill_absent_fields = True,
|
||||
)
|
||||
assert ok.max_seq_length == 4096
|
||||
assert ok.gpu_ids == [0, 1]
|
||||
assert ok.gpu_layers == -1
|
||||
assert ok.tensor_parallel is True
|
||||
assert ok.remove is True
|
||||
assert ok.fill_absent_fields is True
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue