Skip to content

fix(installer): stop leaking the client password on curl's argv (CWE-214) - #325

Merged
saadqbal merged 2 commits into
developfrom
fix/cred-leak-curl-argv
Jul 10, 2026
Merged

fix(installer): stop leaking the client password on curl's argv (CWE-214)#325
saadqbal merged 2 commits into
developfrom
fix/cred-leak-curl-argv

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

verify_credentials() passed the machine-credential password as a curl argv element (--data-urlencode "password=${client_password}"). For the request's lifetime (up to the -m 60 timeout, and up to on the interactive retry loop) it was world-readable via ps aux / /proc/<pid>/cmdline to any unprivileged local user — and tracebloc deploys on shared institutional/on-prem compute (HPC, universities, hospitals) where a co-tenant could scrape it and impersonate the client against the backend. CWE-214. Fires on every install path (minted #838 credential, env-supplied creds, interactive entry).

(Found by an adversarial bug-hunt of the installer + CLI.)

Fix

Feed the password via stdin: --data-urlencode "password@-" reads the value from stdin and URL-encodes it, so it never appears in the process table. printf '%s' is a bash builtin (no fork → no argv exposure) and emits no trailing newline (a here-string <<< would append one and corrupt the password). The username (client_id, a UUID) isn't secret and stays inline.

Verification

  • Byte-identical POST body vs the old argv form — captured both with nc using a tricky password (p@ss w/ +&=é); both send username=uid-123&password=p%40ss+w%2F+%2B%26%3D%C3%A9. Auth behavior unchanged.
  • shellcheck clean.
  • The password reaches helm via the values file (only the non-secret clientId is --set on argv), so verify_credentials was the sole argv leak.
  • Behavior-level bats tests (mock curl → http code) are unaffected.

Type

Security fix (CWE-214) · installer · HIGH.


Note

Low Risk
Small, localized security fix to the install script with the same POST semantics; manifest hash update only.

Overview
verify_credentials() no longer passes the client password as a curl command-line argument. The secret is piped from a printf '%s' builtin into --data-urlencode password@-, so it stays out of /proc/ps while the auth check runs (including retries). Client ID remains inline on the argv; HTTP status handling is unchanged.

scripts/manifest.sha256 is updated for the modified installer script.

Reviewed by Cursor Bugbot for commit 3627b3d. Bugbot is set up for automated code reviews on this repo. Configure here.

…214)
verify_credentials() passed the machine-credential password as a curl argv
element (`--data-urlencode "password=${client_password}"`). For the request's
lifetime (up to the -m 60 timeout, and up to 5× on the interactive retry loop)
the secret was world-readable via `ps aux` / /proc/<pid>/cmdline to any
unprivileged local user — and tracebloc deploys on shared institutional/on-prem
compute (HPC, universities, hospitals) where a co-tenant could scrape it and
impersonate the client against the backend. Fires on every install path
(minted #838 credential, env-supplied creds, interactive entry).
Feed the password via stdin instead: `--data-urlencode "password@-"` reads the
value from stdin and URL-encodes it, so it never appears in the process table.
`printf '%s'` is a bash builtin (no fork → no argv exposure) and emits no
trailing newline (a here-string would append one and corrupt the password). The
username (client_id, a UUID) isn't secret and stays inline.
Verified byte-identical POST body vs the old argv form (incl. spaces / + / & /
= / unicode → same URL-encoding), so auth behavior is unchanged. shellcheck
clean. The password reaches helm via the values FILE (not --set), so this was
the only argv leak.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@LukasWodka
LukasWodka requested a review from saadqbal as a code ownerJuly 9, 2026 18:03
scripts/manifest.sha256 pins each installer sub-script's sha256 (R8 supply-chain
hardening); the verify_credentials change moved install-client-helm.sh's hash.
Regenerated via scripts/gen-manifest.sh — only that one hash changed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@saadqbal

Copy link
Copy Markdown
Contributor

Clean, well-scoped fix 👍 Verified the stdin form produces a byte-identical POST body, the || code=000 still catches curl failures, and the manifest is in sync (gen-manifest.sh --check passes). Confirmed verify_credentials was the only argv leak — Windows Test-Credentials already passes the password via Invoke-WebRequest -Body, so CWE-214 is closed on both paths.

@saadqbal
saadqbal merged commit 77f2af3 into developJul 10, 2026
31 checks passed
saadqbal added a commit that referenced this pull request Jul 10, 2026
Publishes everything unreleased since v1.9.0:
- #323 ingestor default tag → 0.6 (landed as 1.9.1, never released)
- #325 stop leaking client password on curl's argv (CWE-214)
- #326 survive curl|bash: retry name prompt, read creds from terminal,
guard pkg-index refresh (customer-reported)
Refs #328
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal added a commit that referenced this pull request Jul 10, 2026
… + curl|bash survival) (#327)
* Merge pull request #325 from tracebloc/fix/cred-leak-curl-argv
fix(installer): stop leaking the client password on curl's argv (CWE-214)
* fix(installer): survive curl|bash — retry name prompt (customer-reported), read creds from terminal, guard pkg-index refresh (#326)
* fix(installer): don't abort Linux install when the package-index refresh flakes
install_system_deps ran `spin_cmd "Updating package index…" $PM_UPDATE`
unguarded — under set -e a transient mirror/network failure there aborted
the whole install. Yet the per-package installs right below are already
guarded (|| log), so a flaky refresh was MORE fatal than a failed install,
which is backwards: a stale index usually still installs from cache. Guard
it with || warn so we continue to the (guarded) installs, which surface a
genuinely missing package with an actionable message.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(installer): read credential prompts from the terminal so curl|bash doesn't abort
The Step-5 credential prompt read from stdin. Under `curl … | bash` stdin
is the piped script, not the terminal, so each `read` hit EOF and (under
set -e) aborted the installer with an opaque failure the moment it reached
the prompt — the dual-mode env-var path (TRACEBLOC_CLIENT_ID/PASSWORD) was
the only way through, but nothing told the user that.
Read prompts from TB_TTY (the controlling terminal, /dev/tty) instead, the
same mechanism provision.sh already uses. When no terminal is available and
no env creds were supplied, fail with an actionable message pointing at
TRACEBLOC_CLIENT_ID/PASSWORD rather than the set -e abort. TB_TTY is
overridable so the bats suite can feed canned input on stdin.
Regenerated scripts/manifest.sha256 (R8 supply-chain: any scripts/ change
must re-pin, alongside the setup-linux.sh guard in the previous commit).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(installer): retry the client-name prompt so tty type-ahead can't abort provisioning
Customer-reported 2026-07-09 (external onboarding): the installer signed in,
printed the name + location prompts, captured NEITHER, and died with 'A name
for this client is required to provision it.'
provision_client read the name with `IFS= read -r client_name </dev/tty || true`
— a single shot whose status was swallowed by `|| true`, with no retry and no
fallback (location silently defaults to the detected zone, which is why only the
name failed hard). Any empty/failed read on the name → the fatal error. The most
likely trigger is tty type-ahead: during the ~minute browser-approval wait the
CLI reads nothing, so a stray newline queued in the terminal is consumed by the
read as an empty name. (An adversarial pass ruled out the background-process-group
and login-drains-the-tty theories; the reader is foreground and login never
touches the tty — the empty read is an environmental dead/queued-input condition.)
Fix: read the name in a bounded retry loop that RE-PROMPTS on an empty line
(so a queued blank is skipped, not accepted) and BREAKS on a failed read
(rc!=0 = EOF / no live input, which re-prompting can't fix) so the actionable
'set TRACEBLOC_CLIENT_NAME' error still fires. Reads route through TB_TTY
(defaults to /dev/tty; overridable so the bats suite can feed stdin), matching
the install-client-helm.sh credential reads in this branch; prompt WRITES stay
on /dev/tty but are guarded so a test without a real terminal doesn't abort.
The location reads adopt TB_TTY too (their empty->fallback behavior is unchanged).
2 new provision.bats tests: type-ahead blanks are re-prompted then the real name
is captured; a dead-input tty (EOF) fails fast with the guidance. Regenerated
scripts/manifest.sha256 (R8). NOTE: this is Failure 1 of the report; the existing
install-client-helm.sh fix on this branch does NOT cover these provision.sh reads.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(installer): guard credential reads against a dead-input tty (EOF), not just a missing one
Review feedback on #326 (Asad + Bugbot): the credential reads route through
$TB_TTY but had no EOF guard, unlike the provision.sh name read (which breaks on
rc!=0). _tty_available only checks `-r`, so on a readable-but-dead-input tty
(non-PTY ssh, an IDE terminal, a drained/queued tty — the same class this PR
documents for provision.sh) it returns true, the actionable no-creds error is
skipped, and the first `read <"$TB_TTY"` hits EOF and aborts under set -e —
the exact opaque failure this PR set out to remove, left in place for creds.
Factor the actionable env-var guidance into _no_interactive_creds_die and call
it from BOTH the `! _tty_available` check AND a per-read `|| _no_interactive_creds_die`
guard on all five prompts (the Use-previous read + the ID/password reads). A
dead-input tty now fails fast with the same guidance as no-tty instead of
aborting mid-read. Happy path (input present) is unchanged — the guard only
fires on EOF; all existing cred tests (re-prompt, reuse-defaults, max-attempts)
consume their fed input exactly and still pass.
+1 regression test (readable /dev/stdin backed by /dev/null → EOF → actionable
error, no helm). Regenerated scripts/manifest.sha256 (R8).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* chore(chart): bump client 1.9.1 → 1.9.2 (version + appVersion) (#329)
Publishes everything unreleased since v1.9.0:
- #323 ingestor default tag → 0.6 (landed as 1.9.1, never released)
- #325 stop leaking client password on curl's argv (CWE-214)
- #326 survive curl|bash: retry name prompt, read creds from terminal,
guard pkg-index refresh (customer-reported)
Refs #328
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: lukasWuttke <54042461+LukasWodka@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka
LukasWodka deleted the fix/cred-leak-curl-argv branch August 14, 2026 13:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@LukasWodka@saadqbal