[sandbox audit] Keep secret values out of argv in the CLI and fix the env/secrets docs - #4835
Draft
Wauplin wants to merge 1 commit into
Draft
[sandbox audit] Keep secret values out of argv in the CLI and fix the env/secrets docs#4835Wauplin wants to merge 1 commit into
Wauplin wants to merge 1 commit into
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
`--secrets KEY=value` and `--token <value>` put secret material into argv, where it is written to the shell history file and is readable from `/proc/<pid>/cmdline` by any process running as the same user. The CLI already had argv-free alternatives -- bare `--secrets KEY` (resolved from the calling environment), `--secrets-file`, `hf auth login`, `HF_TOKEN` -- but nothing steered users toward them. Separately, the sandbox section of the CLI guide paired `-e` with the secrets channel, while `-e` is `--env` and `-s` is `--secrets`. That is not a cosmetic slip: a user following it puts a secret in the unencrypted env channel, which for a dedicated job is stored in the job metadata instead of the encrypted secrets store. - warn on stderr, once per invocation, when `--secrets` carries an inline value or `--token` is given a value, pointing at the safer forms. `hf auth login --token` is exempt: it is the flow the warning itself recommends. - warn when an env/secrets file is group- or world-readable - support `--secrets-file -` (and `--env-file -`) to read `KEY=value` lines from stdin, so a value need touch neither argv nor the disk - promote the bare-name form in the `--secrets` help text - fix the `-e` / `--secrets` mix-up in `guides/cli.md` and document both channels and their storage properties in a table - explain *why* a pooled sandbox rejects `--secrets` rather than only telling the user to switch to `--env` These stay warnings on purpose: failing on `--secrets KEY=value` would break existing automation. They are silenced by the CLI's existing quiet mode (`-q` / `--format quiet`), so no new knob was introduced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Wauplin
force-pushed
the
security/cli-secrets-hygiene
branch
from
September 8, 2026 15:14
a609837 to
ce05550
Compare
Wauplin
changed the base branch from
security/sandbox-docs-accuracy
to
security/pin-and-verify-binary
September 8, 2026 15:14
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Two related problems with how the CLI handles secret material.
Secret values in
argv.--secrets KEY=valueand--token <value>both take thevalue as a flag argument, so it ends up in
argv. That is not a private channel: theinvocation is appended verbatim to
~/.bash_history/~/.zsh_history, and on Linux/proc/<pid>/cmdlineis readable by every other process running as the same user (and byroot) for as long as the command runs. The CLI already had argv-free paths — bare
--secrets HF_TOKENresolves the value from the calling environment,--secrets-filereads it from disk,
hf auth login/HF_TOKENcover the token — but nothing pointed atthem, and the
--secretshelp text listedKEY=valuefirst.This is local hygiene, not a remote boundary: exploiting it needs access to the machine or
the history file, at which point the stored token in
~/.cache/huggingfaceis usuallyavailable anyway. It is worth fixing because it is cheap and because the bare-name form
already existed.
The env/secrets docs were wrong. The sandbox section of
guides/cli.mdsaid to use"
-e/--secretsfor environment variables". The real bindings are-e/--envand-s/--secrets, and they are different channels with different storage: a user whofollowed that line would put a secret through the unencrypted env channel, which for a
dedicated job means it lands in the job metadata rather than the encrypted secrets store.
What changed
src/huggingface_hub/cli/_cli_utils.pySecretsOptgets an option callback that warns on stderr, once per invocation(not per pair), when any
--secretsvalue contains=. It names the two safer forms.TokenOptgets the same treatment for--token <value>, pointing athf auth loginand
HF_TOKEN.hf auth login --tokenis exempt — that command is the recommendedalternative, so warning there would contradict the advice.
--secrets-file -/--env-file -now readKEY=valuelines from stdin. This is theonly path where a value touches neither
argvnor the disk.mode & 0o077),suggesting
chmod 600. POSIX only: Windows reports a synthetic mode, so the checkwould fire unconditionally there.
--secretshelp text now leads with the bare-name form.src/huggingface_hub/cli/sandbox.py--pool+--secretsrejection now explains the trade-off instead of reading likea downgrade. A pooled sandbox has no encrypted-secrets channel, but its env is
delivered to the host at creation and is not stored in the job metadata — better than
a dedicated job's env, worse than a dedicated job's encrypted secrets. The message says
exactly that.
docs/source/en/guides/cli.md-e/--secretsmix-up.two channels and what each one means for a dedicated vs. a pooled sandbox.
-s MY_SECRET(read from the environment) instead of-s MY_SECRET=psswrd, documents the stdin form, and explains the exposure.docs/source/en/package_reference/cli.mdis regenerated from the changed help strings(
python utils/generate_cli_reference.py --update) —python-qualitychecks it, so ithad to move with this change. Purely mechanical.
Deliberately not done: scrubbing
argvOverwriting the process title at startup would hide the value from
psand/proc/<pid>/cmdline. In Python that needssetproctitle(a third-party dependency), itdoes nothing about the shell history — which is the bigger half of the exposure — and by
the time the CLI runs, anything watching
/procmay already have read the originalargv. The warning plus the bare-name / file / stdin paths cover the real exposure at afraction of the cost. Recording the decision here so it does not get re-litigated.
Behaviour changes
--secrets KEY=value,--token <value>, and loose filepermissions. Nothing becomes an error: a hard failure on
--secrets KEY=valuewouldbreak existing automation, which is not a proportionate response here.
-q/--format quiet) — no newenvironment variable or flag was added. One pre-existing gap: pass-through commands
(
hf jobs run,hf sandbox exec,hf sandbox spawn,hf jobs uv run) forwardunknown flags to the user's command, so the global formatting flags never reach them and
the warning cannot be silenced there. Unchanged by this PR, worth knowing.
--secrets-file -/--env-file -were previously interpreted as a file literallynamed
-. Anyone relying on that (nobody, plausibly) is affected.hf sandbox create --pool <id> --secrets X=Ystill errors, with the new wording.--secretswarning fires during option parsing, so on the--pool+--secretspath it is printed just before the error.
Validation
The three failures are pre-existing on the base branch (verified by stashing this diff and
re-running):
TestRepoListCommand::test_repo_listtrips over a stale localhf_xet(
cannot import name 'XetSession' from 'hf_xet') and the twoTestSkillsMarketplaceCLIcases need network access I don't have. The plainpytest tests/ -k "cli or secret or env"selection pulls intests/test_buckets_cli.py,which hangs against the staging Hub in my environment; the file list above is the same
selection minus those network suites.
New tests, in
tests/test_cli.py::TestSecretHygiene(12, all passing):--secrets KEY=valueand for--token <value>--secrets KEY(and the value is still resolved from theenvironment), for
--secrets-file, for--envonly, or with no flag at allhf auth login --token-qsuppresses it entirely--secrets-file -parsesKEY=valuefrom stdin--pool+--secretsstill errors, with the new explanation🤖 Generated with Claude Code
Note
Low Risk
Changes are mostly warnings and documentation; existing
--secrets KEY=valueand--tokenflows still work, with new stderr output that scripts may need to handle unless-qis used.Overview
Improves local secret hygiene for the
hfCLI: inline--secrets KEY=valueand--token <value>now emit stderr warnings (once per invocation, suppressed with-q) pointing users to env lookup, files, stdin, orhf auth login/HF_TOKEN.--env-file -and--secrets-file -read from stdin; env/secrets files on POSIX warn when group/world-readable. Shared option help text and regeneratedpackage_reference/cli.mdmatch the new guidance.Docs fix the sandbox mix-up of
-evs--secrets, add a jobs/sandbox section on the env vs secrets channels (including pooled sandboxes: no encrypted secrets), and clarify pooled--pool+--secretsrejection with a clearer error message.Reviewed by Cursor Bugbot for commit ce05550. Bugbot is set up for automated code reviews on this repo. Configure here.