hive-forge: validate --forge <label> charset up front
argus flagged (approving) that an unvalidated label could build a path outside the state dir; mara called it out as a usability issue in its own right, not just a low-risk security nit — a typo'd label should give a precise 'not a valid label' error, not a confusing file-not-found or an unexpected traversal. Reject anything outside the plain lowercase+digits+hyphens charset dashboard/extra_forges.rs already enforces on write, before touching the filesystem at all.
This commit is contained in:
parent
8e58793f79
commit
46bdb78466
1 changed files with 25 additions and 1 deletions
|
|
@ -241,13 +241,24 @@ pub fn index(n: u64) -> Result<i64> {
|
||||||
/// the exact same two files `dashboard/extra_forges.rs` writes, so the
|
/// the exact same two files `dashboard/extra_forges.rs` writes, so the
|
||||||
/// read side can't drift from the write side. An unknown label (either
|
/// read side can't drift from the write side. An unknown label (either
|
||||||
/// file missing) is a clear error listing the labels actually found in
|
/// file missing) is a clear error listing the labels actually found in
|
||||||
/// the state dir, not a raw file-not-found.
|
/// the state dir, not a raw file-not-found. A label outside the plain
|
||||||
|
/// identifier charset the dashboard accepts (e.g. a typo containing
|
||||||
|
/// `/` or `..`) is rejected up front with the same charset spelled out,
|
||||||
|
/// rather than silently building a nonsense/traversing path and
|
||||||
|
/// surfacing a confusing file error later.
|
||||||
fn resolve_credentials(forge_label: Option<&str>) -> Result<(String, String)> {
|
fn resolve_credentials(forge_label: Option<&str>) -> Result<(String, String)> {
|
||||||
let Some(label) = forge_label else {
|
let Some(label) = forge_label else {
|
||||||
let base = std::env::var("HIVE_FORGE_URL").unwrap_or_else(|_| DEFAULT_URL.to_owned());
|
let base = std::env::var("HIVE_FORGE_URL").unwrap_or_else(|_| DEFAULT_URL.to_owned());
|
||||||
let token = read_token().context("read forge-token")?;
|
let token = read_token().context("read forge-token")?;
|
||||||
return Ok((base, token));
|
return Ok((base, token));
|
||||||
};
|
};
|
||||||
|
if !is_plain_ident(label) {
|
||||||
|
bail!(
|
||||||
|
"hive-forge: invalid --forge label {label:?} — must be lowercase \
|
||||||
|
letters, digits, and hyphens only (same rule the dashboard's \
|
||||||
|
FORGES tab enforces)"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
let dir = state_dir();
|
let dir = state_dir();
|
||||||
let token_path = dir.join(format!("forge-{label}-token"));
|
let token_path = dir.join(format!("forge-{label}-token"));
|
||||||
|
|
@ -270,6 +281,19 @@ fn resolve_credentials(forge_label: Option<&str>) -> Result<(String, String)> {
|
||||||
bail!("hive-forge: no such forge {label:?} — provisioned forges: {known}");
|
bail!("hive-forge: no such forge {label:?} — provisioned forges: {known}");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Plain-identifier check matching `dashboard/extra_forges.rs`'s
|
||||||
|
/// `is_plain_ident` (itself matching hive-priv's `validate_name_chars`)
|
||||||
|
/// — lowercase ascii + digits + hyphens only. Rejecting anything else
|
||||||
|
/// up front (rather than just letting a weird label fail to resolve a
|
||||||
|
/// file) turns a confusing "no such forge" surprise into a precise
|
||||||
|
/// "that's not a valid label" one, and incidentally means a label like
|
||||||
|
/// `../../etc` can't be used to build a path outside the state dir.
|
||||||
|
fn is_plain_ident(s: &str) -> bool {
|
||||||
|
!s.is_empty()
|
||||||
|
&& s.chars()
|
||||||
|
.all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-')
|
||||||
|
}
|
||||||
|
|
||||||
/// Sidecar shape `dashboard/extra_forges.rs` writes alongside each
|
/// Sidecar shape `dashboard/extra_forges.rs` writes alongside each
|
||||||
/// `forge-<label>-token` file: just the base URL, pinned to the same
|
/// `forge-<label>-token` file: just the base URL, pinned to the same
|
||||||
/// `base_url` JSON key the write side uses.
|
/// `base_url` JSON key the write side uses.
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue