Skip to content

Studio: Keep API keys from training with the server's Hugging Face login - #10809

Open
NilayYadav wants to merge 8 commits into
unslothai:mainfrom
NilayYadav:train-api-key-hf-token
Open

Studio: Keep API keys from training with the server's Hugging Face login#10809
NilayYadav wants to merge 8 commits into
unslothai:mainfrom
NilayYadav:train-api-key-hf-token

Conversation

@NilayYadav

Copy link
Copy Markdown
Collaborator

An API key without its own Hugging Face token could start training with the server owner's Hugging Face login. The model check before training and the training process both fell back to the login saved on the server. That let anyone with an API key train on private models the key was never given access to.

This follows #10142, which closed the same gap for uploads and exports. Now training started with an API key only uses the token sent with that request. If no token is sent, the server's saved login is removed from the training process before it talks to Hugging Face.

Training started from the Studio app still uses the saved login, and an API key that sends its own token still works. Tested with a stand-in Hugging Face server that records which token each request uses. Before, an API key starting training on a private model sent the server's token on 18 requests. After, the request is refused and the server's token is never sent.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-11T22:25:54.921112Z 079def1 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@NilayYadav

Copy link
Copy Markdown
Collaborator Author

Review + GitHub Actions evidence

Pinned review: 89ca288c4c plus review fix 0ce539671a.

Findings

  • Codex P1 (cached snapshot pinned without caller authorization) and P2 (parent GPU probe used the saved login): both fixed in 89ca288.
  • P2, fixed in 0ce5396: an API key without a token could still train on a private dataset that was already in the server's Hugging Face cache. The dataset preflight returned early for cached or offline starts, and the worker then loaded the cache with local_files_only. Training starts now apply the same cached_read_refused check that the dataset preview already uses, and refuse with hf_dataset_access_denied.
  • CI, fixed in 0ce5396: allow_ambient = not via_api_key treated a call that leaves via_api_key unresolved as an API key. That broke 4 tests in test_training_cached_start.py (the red Backend CI job). The check is now via_api_key is not True, which matches the existing check in the same route.

Live reproduction on GitHub Actions

Both branches run a real Studio backend against a stand-in Hugging Face server that records which credential each request carries. The server has a saved login, and the model and dataset are private unless a request uses the server's login or the caller's own token. The workflow and probe are identical on both branches; only the Studio code differs.

scenario HTTP status/code server-login requests caller-token requests
API key, no token, private model 200 queued 2 0
API key, no token, private model cached on server 200 queued 2 0
API key, no token, private dataset cached on server 200 queued 2 0
API key with its own token 200 queued 0 7
Studio app session 200 queued 2 0
scenario HTTP status/code server-login requests caller-token requests
API key, no token, private model 404 hf_model_verification_failed 0 0
API key, no token, private model cached on server 422 hf_model_access_denied 0 0
API key, no token, private dataset cached on server 422 hf_dataset_access_denied 0 0
API key with its own token 200 queued 0 7
Studio app session 200 queued 2 0

At 89ca288, before 0ce5396, the cached private dataset case was still 200 queued.

Tests

  • test_training_ambient_hf_token.py on the merge base: 22 of 25 fail. On 89ca288: 25 of 25 pass.
  • With 0ce5396: all 587 tests pass across test_training_ambient_hf_token.py, test_training_cached_start.py, the training preflight, streaming, idempotency and offload suites, and test_hub_token_caller_identity.py. The same files against 89ca288 fail 8 tests: the 4 cached-dataset refusal cases and the 4 test_training_cached_start.py tests.
  • The other red Backend CI test, test_pr5624_regressions.py::test_deepseek_r1_huge_fenceless_body_is_linear, is a timing bound. It is unrelated to this change.

@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 11, 2026
chatgpt-codex-connector[bot]

This comment was marked as resolved.

@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 11, 2026
@NilayYadav

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ae371d2153

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if request.hf_dataset:
await asyncio.to_thread(_preflight_hf_dataset_request, request)
# After the preflight: a cache it pinned is still on disk for this scan to refuse.
await asyncio.to_thread(_refuse_unauthorized_cached_dataset, request, hf_token)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Pin and authorize the same dataset snapshot

When a tokenless API-key request claims a cached dataset, the snapshot found by preflight can disappear before this authorization scan, causing cached_read_refused to observe no cache and allow the request; if another request restores the private dataset before TrainingBackend._apply_cache_pins calls training_dataset_cache_pin again, the backend pins and trains on that unauthorized cache. Fresh evidence in the current tree is that this check still does not return or persist the authorized path, while _apply_cache_pins independently resolves the dataset afterward; carry the checked pin into training_kwargs and validate that exact snapshot instead.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing this one. The bypass needs the cached private dataset to disappear in the milliseconds between two adjacent preflight steps and then come back before the backend pins it. A tokenless API-key caller cannot restore a private dataset, so this only happens if the operator deletes and re-downloads that exact dataset inside that window. If the cache is present at either scan the start is refused, and if it is absent at preflight the anonymous dataset_info call (or the offline 409) refuses it. Authorization is per repository, so the snapshot the backend later selects does not change who may read it.

@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 11, 2026
chatgpt-codex-connector[bot]

This comment was marked as resolved.

@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 11, 2026
chatgpt-codex-connector[bot]

This comment was marked as resolved.

@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 11, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 11, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 11, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 11, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 11, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 11, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Sep 11, 2026
@NilayYadav

Copy link
Copy Markdown
Collaborator Author

@codex review

@NilayYadav

Copy link
Copy Markdown
Collaborator Author

Follow-up review fixes

Four more review rounds after the evidence above. Each accepted finding was fixed and has a regression test:

  • ae371d2: a private model snapshot that finishes caching during the metadata request is now checked for access before it is pinned. The cached-dataset check now runs after the dataset preflight, so it scans the same cache that preflight pins.
  • 3d541b5: the route stores the trimmed Hugging Face token. Before, a token with surrounding whitespace passed preflight, and the worker then sent it unchanged to the Hub and failed authentication.
  • 079def1: an absolute snapshot path inside the Hugging Face cache is now checked against the repository that owns it. Before, an API key could pass that path as model_name and train from cached private weights without an access check. /models/cached-model-path returns these paths to API keys.

Live probe for the cache-path start

Same stand-in Hugging Face server as above: a tokenless API key starts training with the absolute snapshot path of a private model that is cached on the server.

tree HTTP status/code server-login requests
main + 3d541b5 200 queued 0
main + this PR 422 hf_model_access_denied 0

The other scenarios are unchanged: tokenless API-key starts are refused and never send the server's login, an API key with its own token is accepted, and a Studio app session still uses the saved login. That is 19 of 19 assertions passing.

Tests

  • On 3d541b5 with the new tests, the targeted suites fail exactly the 3 new cases. With 079def1, all 835 tests in those suites pass.
  • The full backend suite on 079def1 has 25 failures, and the same 25 fail without this change. They are in the RAG, web-rank and llama.cpp update tests, and each is an environment failure on the test container. The serial-only suites pass, 2,224 of 2,224.

Out of scope

The model config, remote-code scan and inference load routes still treat any local path as outside the Hub cache, so they skip the cache authorization. /models/cached-model-path also returns cache paths to API keys. These are separate from training and are better handled in their own PR.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 079def1859

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@NilayYadav

NilayYadav commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

UI evidence: Train page after a tokenless API-key start

This compares the merge base 8ee07d6ae with the PR head a242187. Each side is a separate Studio install with its own frontend build. Both sides run on an NVIDIA L4, so training is available and /studio loads on both.

Both sides get the same setup:

  • a saved Hugging Face login on the server
  • the private model unsloth-probe/private-llama already cached
  • a stand-in Hugging Face server that only serves that repo to the saved login or a caller's own token

Then an API key with no token calls POST /api/train/start for that model, and the screenshot shows the owner's Train page.

The difference is in the Train page sidebar. The crop below is enlarged 2x:

PR 10809 Train page sidebar before/after

Full Train page on both sides

PR 10809 Train page before/after

merge base 8ee07d6ae PR head a242187
API-key start 200, queued 422, hf_model_access_denied
Runs recorded for the private model 1 (error) 0
Requests that used the server's saved login 6 0
Recents row for unsloth-probe/private-llama shown none

Before, the API key's run on the private model shows in the owner's Recents, and the server's login was used to fetch it. After, the start is refused, nothing is recorded, and the saved login is never sent.

@danielhanchen

Copy link
Copy Markdown
Member

Confirmed this hits studio/backend/routes/training.py, where the start path still passes the token as or None and falls back to the server's saved Hugging Face login for an API-key caller, the same gap #10142 closed for uploads and exports. Will get this reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants