fix(audit-2026-06-11/H2): reject inline CLI secrets; true no-echo prompt
pic admins create/set --password and pic login --token accepted secrets on argv, where they leak into shell history, ps aux, and /proc/<pid>/cmdline. Now only --password -/--token - (stdin) is allowed; an inline value is rejected with guidance. PICLOUD_TOKEN env still covers CI for login. Also (M2): the interactive admins password prompt used read_line, which echoes despite a 'no echo' claim — switched to rpassword::prompt_password (already a dep via login). Removed the now-dead ReadLine trait + Write import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -3,7 +3,7 @@
|
|||||||
//! All endpoints require `Capability::InstanceManageUsers` (owner /
|
//! All endpoints require `Capability::InstanceManageUsers` (owner /
|
||||||
//! admin). Member accounts get a 403.
|
//! admin). Member accounts get a 403.
|
||||||
|
|
||||||
use std::io::{IsTerminal, Read, Write};
|
use std::io::{IsTerminal, Read};
|
||||||
|
|
||||||
use anyhow::{anyhow, Context, Result};
|
use anyhow::{anyhow, Context, Result};
|
||||||
use picloud_shared::InstanceRole;
|
use picloud_shared::InstanceRole;
|
||||||
@@ -44,33 +44,32 @@ pub async fn ls(mode: OutputMode) -> Result<()> {
|
|||||||
|
|
||||||
/// Resolve the password the user wants to set:
|
/// Resolve the password the user wants to set:
|
||||||
/// * `Some("-")` → read from stdin (no echo when interactive).
|
/// * `Some("-")` → read from stdin (no echo when interactive).
|
||||||
/// * `Some(other)` → use the inline value (handy for non-interactive
|
/// * `Some(other)` → **rejected**. Audit 2026-06-11 (H2) — an inline
|
||||||
/// callers, but logs to shell history; warn when interactive).
|
/// password lands in shell history, `ps aux`, and
|
||||||
|
/// `/proc/<pid>/cmdline`, readable by any other UID on the host
|
||||||
|
/// while the process runs.
|
||||||
/// * `None` → require explicit `--password`.
|
/// * `None` → require explicit `--password`.
|
||||||
fn resolve_password(password_arg: Option<&str>) -> Result<String> {
|
fn resolve_password(password_arg: Option<&str>) -> Result<String> {
|
||||||
match password_arg {
|
match password_arg {
|
||||||
None => Err(anyhow!(
|
None => Err(anyhow!(
|
||||||
"missing --password; pass --password - to read from stdin"
|
"missing --password; pass --password - to read the password from stdin"
|
||||||
)),
|
)),
|
||||||
Some("-") => read_password_from_stdin(),
|
Some("-") => read_password_from_stdin(),
|
||||||
Some(inline) => Ok(inline.to_string()),
|
Some(_) => Err(anyhow!(
|
||||||
|
"passing the password inline is not allowed — it leaks into shell history, \
|
||||||
|
`ps aux`, and /proc/<pid>/cmdline. Use `--password -` to read it from stdin \
|
||||||
|
(interactive prompt, or piped: `printf %s \"$PW\" | pic admins create … --password -`)."
|
||||||
|
)),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn read_password_from_stdin() -> Result<String> {
|
fn read_password_from_stdin() -> Result<String> {
|
||||||
let stdin = std::io::stdin();
|
let stdin = std::io::stdin();
|
||||||
if stdin.is_terminal() {
|
if stdin.is_terminal() {
|
||||||
// Prompt without echoing — rpassword would be cleaner, but the
|
// True no-echo prompt. Audit 2026-06-11 (M2) — the previous
|
||||||
// CLI already avoids extra crates. Print a one-line prompt so
|
// `read_line` path echoed the password despite a "no echo"
|
||||||
// the user knows we're waiting for input.
|
// claim; `rpassword` is already a dependency via the login flow.
|
||||||
eprint!("Password: ");
|
rpassword::prompt_password("Password: ").context("read password from stdin")
|
||||||
std::io::stderr().flush().ok();
|
|
||||||
let mut buf = String::new();
|
|
||||||
stdin
|
|
||||||
.lock()
|
|
||||||
.read_line(&mut buf)
|
|
||||||
.context("read password from stdin")?;
|
|
||||||
Ok(buf.trim_end_matches(['\n', '\r']).to_string())
|
|
||||||
} else {
|
} else {
|
||||||
// Piped input — read everything, strip trailing newline.
|
// Piped input — read everything, strip trailing newline.
|
||||||
let mut buf = String::new();
|
let mut buf = String::new();
|
||||||
@@ -82,16 +81,6 @@ fn read_password_from_stdin() -> Result<String> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
trait ReadLine {
|
|
||||||
fn read_line(&mut self, buf: &mut String) -> std::io::Result<usize>;
|
|
||||||
}
|
|
||||||
|
|
||||||
impl<T: std::io::BufRead> ReadLine for T {
|
|
||||||
fn read_line(&mut self, buf: &mut String) -> std::io::Result<usize> {
|
|
||||||
std::io::BufRead::read_line(self, buf)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
pub async fn create(
|
pub async fn create(
|
||||||
username: &str,
|
username: &str,
|
||||||
password: Option<&str>,
|
password: Option<&str>,
|
||||||
|
|||||||
@@ -26,7 +26,21 @@ pub async fn run(url_arg: Option<&str>, token_arg: Option<&str>) -> Result<()> {
|
|||||||
let token_from_env = std::env::var("PICLOUD_TOKEN")
|
let token_from_env = std::env::var("PICLOUD_TOKEN")
|
||||||
.ok()
|
.ok()
|
||||||
.filter(|s| !s.is_empty());
|
.filter(|s| !s.is_empty());
|
||||||
let bearer_token = token_arg.map(str::to_string).or(token_from_env);
|
// Audit 2026-06-11 (H2) — an inline `--token <T>` lands in shell
|
||||||
|
// history, `ps aux`, and /proc/<pid>/cmdline. Accept only `--token -`
|
||||||
|
// (stdin) or the PICLOUD_TOKEN env var (the CI path); reject inline.
|
||||||
|
let token_from_arg = match token_arg {
|
||||||
|
Some("-") => Some(read_token_from_stdin()?),
|
||||||
|
Some(_) => {
|
||||||
|
anyhow::bail!(
|
||||||
|
"passing the token inline is not allowed — it leaks into shell history, \
|
||||||
|
`ps aux`, and /proc/<pid>/cmdline. Use `--token -` to read it from stdin, \
|
||||||
|
or set the PICLOUD_TOKEN environment variable."
|
||||||
|
)
|
||||||
|
}
|
||||||
|
None => None,
|
||||||
|
};
|
||||||
|
let bearer_token = token_from_arg.or(token_from_env);
|
||||||
|
|
||||||
let (token, username, role) = match bearer_token {
|
let (token, username, role) = match bearer_token {
|
||||||
Some(t) => login_with_bearer(&url, &t).await?,
|
Some(t) => login_with_bearer(&url, &t).await?,
|
||||||
@@ -46,6 +60,24 @@ pub async fn run(url_arg: Option<&str>, token_arg: Option<&str>) -> Result<()> {
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Read a bearer token from stdin. Interactive sessions get a no-echo
|
||||||
|
/// prompt (a token is a secret); piped input reads one line. Audit
|
||||||
|
/// 2026-06-11 (H2) — backs `pic login --token -`.
|
||||||
|
fn read_token_from_stdin() -> Result<String> {
|
||||||
|
use std::io::IsTerminal;
|
||||||
|
if io::stdin().is_terminal() {
|
||||||
|
let t = rpassword::prompt_password("Token: ").context("read token from stdin")?;
|
||||||
|
Ok(t.trim().to_string())
|
||||||
|
} else {
|
||||||
|
let mut buf = String::new();
|
||||||
|
io::stdin()
|
||||||
|
.lock()
|
||||||
|
.read_line(&mut buf)
|
||||||
|
.context("read token from stdin")?;
|
||||||
|
Ok(buf.trim().to_string())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async fn login_with_password(url: &str) -> Result<(String, String, InstanceRole)> {
|
async fn login_with_password(url: &str) -> Result<(String, String, InstanceRole)> {
|
||||||
let username = prompt_line("Username: ")?;
|
let username = prompt_line("Username: ")?;
|
||||||
if username.is_empty() {
|
if username.is_empty() {
|
||||||
|
|||||||
@@ -119,9 +119,10 @@ struct LoginArgs {
|
|||||||
#[arg(long)]
|
#[arg(long)]
|
||||||
url: Option<String>,
|
url: Option<String>,
|
||||||
|
|
||||||
/// Skip the username + password exchange and persist this bearer
|
/// Skip the username + password exchange and persist a bearer
|
||||||
/// directly (validated against `/auth/me` first). Also reads
|
/// directly (validated against `/auth/me` first). Pass `--token -`
|
||||||
/// `PICLOUD_TOKEN`.
|
/// to read it from stdin; an inline value is rejected (it leaks into
|
||||||
|
/// shell history / `ps`). For CI, set `PICLOUD_TOKEN` instead.
|
||||||
#[arg(long)]
|
#[arg(long)]
|
||||||
token: Option<String>,
|
token: Option<String>,
|
||||||
}
|
}
|
||||||
@@ -551,9 +552,9 @@ enum AdminsCmd {
|
|||||||
/// `--instance-role`, defaults to `admin`.
|
/// `--instance-role`, defaults to `admin`.
|
||||||
Create {
|
Create {
|
||||||
username: String,
|
username: String,
|
||||||
/// Password. `-` reads from stdin (one line interactive, or the
|
/// Password. Use `--password -` to read from stdin (interactive
|
||||||
/// full input piped). Passing the password inline puts it in
|
/// no-echo prompt, or piped). An inline value is rejected — it
|
||||||
/// shell history — pipe instead when scripting.
|
/// would leak into shell history, `ps`, and /proc.
|
||||||
#[arg(long)]
|
#[arg(long)]
|
||||||
password: Option<String>,
|
password: Option<String>,
|
||||||
/// `owner`, `admin`, or `member`. Defaults to `admin`.
|
/// `owner`, `admin`, or `member`. Defaults to `admin`.
|
||||||
@@ -571,7 +572,8 @@ enum AdminsCmd {
|
|||||||
id: String,
|
id: String,
|
||||||
#[arg(long)]
|
#[arg(long)]
|
||||||
username: Option<String>,
|
username: Option<String>,
|
||||||
/// `-` reads from stdin. Inline value lands in shell history.
|
/// Use `--password -` to read from stdin. An inline value is
|
||||||
|
/// rejected (it leaks into shell history, `ps`, and /proc).
|
||||||
#[arg(long)]
|
#[arg(long)]
|
||||||
password: Option<String>,
|
password: Option<String>,
|
||||||
/// `true` to (re)activate, `false` to deactivate (deactivation
|
/// `true` to (re)activate, `false` to deactivate (deactivation
|
||||||
|
|||||||
@@ -119,6 +119,24 @@ fn create_without_password_errors() {
|
|||||||
.stderr(predicate::str::contains("missing --password"));
|
.stderr(predicate::str::contains("missing --password"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
|
||||||
|
#[test]
|
||||||
|
fn create_with_inline_password_is_rejected() {
|
||||||
|
// Audit 2026-06-11 (H2) — an inline password on argv leaks into
|
||||||
|
// shell history / ps / proc; only `--password -` (stdin) is allowed.
|
||||||
|
let Some(fx) = common::fixture_or_skip() else {
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
let env = common::admin_env(fx);
|
||||||
|
let username = common::unique_username("inlinepw");
|
||||||
|
|
||||||
|
common::pic_as(&env)
|
||||||
|
.args(["admins", "create", &username, "--password", "hunter2"])
|
||||||
|
.assert()
|
||||||
|
.failure()
|
||||||
|
.stderr(predicate::str::contains("inline is not allowed"));
|
||||||
|
}
|
||||||
|
|
||||||
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
|
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
|
||||||
#[test]
|
#[test]
|
||||||
fn member_cannot_list_admins() {
|
fn member_cannot_list_admins() {
|
||||||
|
|||||||
Reference in New Issue
Block a user