diff --git a/docs/configuration.md b/docs/configuration.md index 82823212..b9e77554 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -68,7 +68,7 @@ These are consumed by the wrapper itself, not passed to vLLM: | Variable | Default | Description | | --------------------- | ------- | ---------------------------------------------------------------------- | | `MODEL_NAME` | — | Required. HF repo id or local path of the model. | -| `HF_TOKEN` | — | Hugging Face token for gated/private models. | +| `HF_TOKEN` | — | Hugging Face token for gated/private models. Passed to vLLM via the environment, never as `--hf-token`. | | `BASE_PATH` | `/runpod-volume` | Root for the HF cache (persists on a network volume). | | `MAX_CONCURRENCY` | `30` | Max concurrent jobs per worker (RunPod concurrency modifier). vLLM queues internally beyond this. | | `VLLM_PORT` | `8000` | Loopback port the internal `vllm serve` binds to. | diff --git a/docs/conventions.md b/docs/conventions.md index a06b766f..8aa68d92 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -221,6 +221,9 @@ workers at container start. ## Security & Best Practices - **Build secrets**: `HF_TOKEN` via Docker secrets; never baked into image layers. +- **Runtime secrets**: `HF_TOKEN` reaches `vllm serve` through the inherited environment, not + argv, so it stays out of `ps` output. The `Starting vLLM:` log line masks the value of any + flag whose last word is `token`/`key`/`secret`/`password` (`args_builder.redact_argv`). - **Authentication**: handled by RunPod at the platform level (endpoint API key). The worker intentionally does not wire `VLLM_API_KEY`/`--api-key`; setting it has no effect. The internal vLLM server binds to loopback only and is never diff --git a/src/args_builder.py b/src/args_builder.py index 0228b145..78296259 100644 --- a/src/args_builder.py +++ b/src/args_builder.py @@ -55,8 +55,17 @@ # --host/--port are pinned by main.py, --config comes from the VLLM_CONFIG_FILE # alias, --model has the MODEL_NAME alias. --api-key stays reserved because the # worker intentionally does not support it: the internal server binds to -# loopback only and RunPod authenticates callers at the platform. -RESERVED_ENV_VARS = frozenset({"HOST", "PORT", "API_KEY", "MODEL", "CONFIG", "HELP"}) +# loopback only and RunPod authenticates callers at the platform. HF_TOKEN is +# reserved so the token never lands on the command line (where `ps` and the +# launch log would show it): vLLM's --hf-token defaults to huggingface_hub's +# own lookup, which reads HF_TOKEN from the environment the child inherits. +RESERVED_ENV_VARS = frozenset({"HOST", "PORT", "API_KEY", "MODEL", "CONFIG", "HELP", "HF_TOKEN"}) + +# A flag whose last dash-separated word is one of these carries a credential +# (--hf-token, --api-key). Matching the last word, not a substring, keeps +# --tokenizer, --max-num-batched-tokens and --ssl-keyfile (a path) readable. +SECRET_FLAG_WORDS = frozenset({"token", "key", "secret", "password"}) +REDACTED = "***" # Flags that take a value. vLLM parses int/float/JSON values itself. VALUE_FLAGS = frozenset({ @@ -341,3 +350,32 @@ def build_vllm_args(env: Optional[Mapping[str, str]] = None) -> list[str]: extra = env.get("VLLM_EXTRA_ARGS", "") argv.extend(shlex.split(extra)) return argv + + +def is_secret_flag(flag: str) -> bool: + return flag.startswith("--") and flag.lstrip("-").rsplit("-", 1)[-1].lower() in SECRET_FLAG_WORDS + + +def redact_argv(argv: list[str]) -> list[str]: + """Copy of argv with credential values masked, for logging only. + + Handles both `--hf-token VALUE` and `--hf-token=VALUE`; the second form can + still arrive through VLLM_EXTRA_ARGS. + """ + redacted: list[str] = [] + mask_next = False + for arg in argv: + if mask_next: + redacted.append(REDACTED) + mask_next = False + continue + flag, sep, _ = arg.partition("=") + if is_secret_flag(flag): + if sep: + redacted.append(f"{flag}={REDACTED}") + else: + redacted.append(arg) + mask_next = True + continue + redacted.append(arg) + return redacted diff --git a/src/main.py b/src/main.py index 585cd330..ef623435 100644 --- a/src/main.py +++ b/src/main.py @@ -37,7 +37,7 @@ import model_preflight import startup_errors -from args_builder import TRUE_VALUES, build_vllm_args +from args_builder import TRUE_VALUES, build_vllm_args, redact_argv from download_model import LOCAL_MODEL_ARGS_PATH logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(name)s: %(message)s") @@ -94,7 +94,7 @@ def start_vllm() -> subprocess.Popen: argv = ["vllm", "serve", "--host", VLLM_HOST, "--port", VLLM_PORT] argv += build_vllm_args() - logging.info("Starting vLLM: %s", " ".join(argv)) + logging.info("Starting vLLM: %s", " ".join(redact_argv(argv))) # vLLM's stdout+stderr flow through a pipe so we can both forward them to the # worker logs and keep the tail to classify a startup failure. Unbuffered so # the child's log lines arrive as they are written, not when its buffer fills. diff --git a/tests/test_secret_redaction.py b/tests/test_secret_redaction.py new file mode 100644 index 00000000..9a4a0a00 --- /dev/null +++ b/tests/test_secret_redaction.py @@ -0,0 +1,72 @@ +"""Credentials never reach the `vllm serve` command line or the launch log.""" + +import logging +import sys +from pathlib import Path +from unittest import mock + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "src")) + +import main # noqa: E402 +from args_builder import REDACTED, build_vllm_args, is_secret_flag, redact_argv # noqa: E402 + +# Obviously fake; the tests only check that this exact string never escapes. +FAKE_TOKEN = "hf_FAKEtestTOKENvalue0000000000000000" + + +class TestSecretFlags: + def test_credential_flags_are_secret(self): + for flag in ("--hf-token", "--api-key", "--some-secret", "--db-password"): + assert is_secret_flag(flag), flag + + def test_look_alike_flags_are_not_secret(self): + # Substring matches would hide values operators need when debugging. + for flag in ("--tokenizer", "--max-num-batched-tokens", "--ssl-keyfile", + "--tokenizer-mode", "--dbo-decode-token-threshold", "--model"): + assert not is_secret_flag(flag), flag + + +class TestRedactArgv: + def test_space_separated_value_masked(self): + argv = ["vllm", "serve", "--hf-token", FAKE_TOKEN, "--model", "org/m"] + assert redact_argv(argv) == ["vllm", "serve", "--hf-token", REDACTED, "--model", "org/m"] + + def test_equals_form_masked(self): + assert redact_argv(["--api-key=sk-123", "--model=org/m"]) == [f"--api-key={REDACTED}", "--model=org/m"] + + def test_input_not_mutated(self): + argv = ["--hf-token", FAKE_TOKEN] + redact_argv(argv) + assert argv == ["--hf-token", FAKE_TOKEN] + + def test_non_secret_argv_unchanged(self): + argv = ["--tokenizer", "org/tok", "--max-num-batched-tokens", "8192"] + assert redact_argv(argv) == argv + + +class TestHfTokenStaysInEnv: + def test_hf_token_env_is_not_turned_into_a_flag(self): + args = build_vllm_args({"HF_TOKEN": FAKE_TOKEN, "MODEL_NAME": "org/m"}) + assert "--hf-token" not in args + assert FAKE_TOKEN not in args + + def test_start_vllm_keeps_token_out_of_argv_and_log(self, monkeypatch, caplog): + monkeypatch.setenv("HF_TOKEN", FAKE_TOKEN) + monkeypatch.setenv("MODEL_NAME", "org/m") + # Even if someone passes the flag explicitly, the log must mask it. + monkeypatch.setenv("VLLM_EXTRA_ARGS", f"--api-key=sk-extra --hf-token {FAKE_TOKEN}") + with mock.patch.object(main.subprocess, "Popen") as popen, \ + mock.patch.object(main.threading, "Thread"), \ + caplog.at_level(logging.INFO): + main.start_vllm() + + launch_lines = [r.getMessage() for r in caplog.records if "Starting vLLM" in r.getMessage()] + assert len(launch_lines) == 1 + assert FAKE_TOKEN not in launch_lines[0] + assert "sk-extra" not in launch_lines[0] + assert "--model org/m" in launch_lines[0] + + argv = popen.call_args.args[0] + # HF_TOKEN reaches vLLM through the environment, not the command line. + assert argv.count("--hf-token") == 1 # only the explicit VLLM_EXTRA_ARGS one + assert popen.call_args.kwargs["env"]["HF_TOKEN"] == FAKE_TOKEN