Studio: Keep API keys from training with the server's Hugging Face login - #10809
Studio: Keep API keys from training with the server's Hugging Face login#10809NilayYadav wants to merge 8 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Review + GitHub Actions evidencePinned review: Findings
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.
At Tests
|
for more information, see https://pre-commit.ci
|
@codex review |
There was a problem hiding this comment.
💡 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) |
There was a problem hiding this comment.
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 👍 / 👎.
There was a problem hiding this comment.
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.
|
@codex review |
Follow-up review fixesFour more review rounds after the evidence above. Each accepted finding was fixed and has a regression test:
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.
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
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. |
for more information, see https://pre-commit.ci
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
UI evidence: Train page after a tokenless API-key startThis compares the merge base Both sides get the same setup:
Then an API key with no token calls The difference is in the Train page sidebar. The crop below is enlarged 2x:
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. |
|
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. |


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.