[M2/M4] mxaccess-rpc: NtlmClientContext::from_env + local_hostname (resolves F1)
Reduces open followups from 11 → 10 (back at the soft threshold).
Step 0 triage flagged F1 as resolvable now: M4's connect-path
example will need a from_env constructor anyway, and the hostname
lookup is portable enough not to need a native-libc dep.
New
- NtlmClientContext::from_env() -> Result<Self, NtlmError>: reads
MX_RPC_USER / MX_RPC_PASSWORD / MX_RPC_DOMAIN env vars. Empty
MX_RPC_DOMAIN is permitted (workgroup auth). Mirrors the .NET
ManagedNtlmClientContext.FromEnvironment() at cs:41-49.
- local_hostname() -> String public helper: checks COMPUTERNAME
(Windows) then HOSTNAME (POSIX) and returns the empty string when
neither is set — same "unavailable" semantics as
Environment.MachineName returning null. No gethostname(2) call,
no unsafe, no native-libc dep. Callers needing reliable POSIX
hostnames can pass workstation explicitly.
- NtlmError::MissingEnvVar { name: &'static str } variant.
Tests (8 new in ntlm; total 27)
- from_env three-var happy path
- from_env missing each of the three vars (3 tests)
- from_env accepts empty MX_RPC_DOMAIN
- local_hostname prefers COMPUTERNAME over HOSTNAME
- local_hostname falls back to HOSTNAME
- local_hostname returns empty when neither set
- All env-mutating tests serialize via a static ENV_LOCK Mutex inside
EnvScope, since std::env::set_var touches process-global state and
cargo runs #[test]s in parallel by default.
design/followups.md: F1 moved to Resolved.
Open followups: 11 → 10 (back at soft threshold).
Test count delta: 498 -> 506 (+8). All four DoD gates green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -205,6 +205,32 @@ pub enum NtlmError {
|
||||
/// expected bytes.
|
||||
#[error("NTLM signature mismatch")]
|
||||
SignatureMismatch,
|
||||
|
||||
/// [`NtlmClientContext::from_env`] could not find a required env var.
|
||||
/// Mirrors the .NET `ArgumentNullException` paths in
|
||||
/// `ManagedNtlmClientContext.FromEnvironment()` (`cs:43-48`).
|
||||
#[error("missing environment variable: {name}")]
|
||||
MissingEnvVar { name: &'static str },
|
||||
}
|
||||
|
||||
/// Look up the local machine name. Mirrors `Environment.MachineName`
|
||||
/// (`ManagedNtlmClientContext.cs:38`).
|
||||
///
|
||||
/// Cross-platform: checks `COMPUTERNAME` (Windows) then `HOSTNAME`
|
||||
/// (POSIX). Returns the empty string when neither is set — same
|
||||
/// "unavailable" semantics as `Environment.MachineName` returning
|
||||
/// `null` on platforms where it can't read the host name.
|
||||
///
|
||||
/// This deliberately doesn't call `gethostname(2)` to keep
|
||||
/// `mxaccess-rpc` free of native-libc dependencies and `unsafe`. For
|
||||
/// reliable hostname discovery on POSIX where `HOSTNAME` isn't
|
||||
/// exported (the common case in Bash subshells), callers can pass
|
||||
/// `Some(&hostname_string)` to [`NtlmClientContext::new`] explicitly.
|
||||
#[must_use]
|
||||
pub fn local_hostname() -> String {
|
||||
std::env::var("COMPUTERNAME")
|
||||
.or_else(|_| std::env::var("HOSTNAME"))
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
/// Trait for supplying the random/clock inputs the .NET reference reads from
|
||||
@@ -339,6 +365,39 @@ impl NtlmClientContext {
|
||||
}
|
||||
}
|
||||
|
||||
/// Create an unauthenticated context from environment variables.
|
||||
/// Mirrors `ManagedNtlmClientContext.FromEnvironment()`
|
||||
/// (`ManagedNtlmClientContext.cs:41-49`).
|
||||
///
|
||||
/// Required env vars (all three must be set; empty values are
|
||||
/// permitted for `MX_RPC_DOMAIN`):
|
||||
///
|
||||
/// - `MX_RPC_USER` — user name
|
||||
/// - `MX_RPC_PASSWORD` — plaintext password
|
||||
/// - `MX_RPC_DOMAIN` — NT domain (or workgroup name; empty is fine)
|
||||
///
|
||||
/// `workstation` defaults to [`local_hostname`] (the .NET reference
|
||||
/// uses `Environment.MachineName` per `cs:38`). When the env-var
|
||||
/// reads succeed but the local hostname is unavailable on this
|
||||
/// platform, `workstation` is the empty string — same as
|
||||
/// `Environment.MachineName` returning `null` on .NET.
|
||||
///
|
||||
/// # Errors
|
||||
/// [`NtlmError::MissingEnvVar`] if any of the three env vars is unset.
|
||||
pub fn from_env() -> Result<Self, NtlmError> {
|
||||
let user = std::env::var("MX_RPC_USER").map_err(|_| NtlmError::MissingEnvVar {
|
||||
name: "MX_RPC_USER",
|
||||
})?;
|
||||
let password = std::env::var("MX_RPC_PASSWORD").map_err(|_| NtlmError::MissingEnvVar {
|
||||
name: "MX_RPC_PASSWORD",
|
||||
})?;
|
||||
let domain = std::env::var("MX_RPC_DOMAIN").map_err(|_| NtlmError::MissingEnvVar {
|
||||
name: "MX_RPC_DOMAIN",
|
||||
})?;
|
||||
let hostname = local_hostname();
|
||||
Ok(Self::new(&user, &password, &domain, Some(&hostname)))
|
||||
}
|
||||
|
||||
/// Build the 32-byte Type1 (NEGOTIATE) message. Mirrors `cs:51-70`.
|
||||
pub fn create_type1(&mut self) -> [u8; TYPE1_LEN] {
|
||||
self.flags = TYPE1_FLAGS;
|
||||
@@ -811,7 +870,12 @@ const _: () = assert!(SIGNATURE_LEN == 16);
|
||||
const _: () = assert!(NTLMSSP_SIGNATURE[7] == 0);
|
||||
|
||||
#[cfg(test)]
|
||||
#[allow(clippy::unwrap_used, clippy::expect_used, clippy::indexing_slicing)]
|
||||
#[allow(
|
||||
clippy::unwrap_used,
|
||||
clippy::expect_used,
|
||||
clippy::indexing_slicing,
|
||||
clippy::panic
|
||||
)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
@@ -1167,4 +1231,181 @@ mod tests {
|
||||
assert_eq!(&msg[16..19], &[0xaa, 0xbb, 0xcc]);
|
||||
assert_eq!(off, 19);
|
||||
}
|
||||
|
||||
// ---- F1: from_env + local_hostname ------------------------------
|
||||
|
||||
/// All env-mutating tests in this mod serialize on this mutex.
|
||||
/// `#[test]`s run in parallel by default, but `std::env::set_var`
|
||||
/// touches process-global state, so concurrent env-var tests race.
|
||||
/// Holding `ENV_LOCK` for the duration of each `EnvScope` makes
|
||||
/// snapshot-and-restore safe.
|
||||
static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
|
||||
|
||||
/// Snapshot the listed env vars on creation, restore them on Drop.
|
||||
/// Holds [`ENV_LOCK`] for the scope's lifetime so tests don't race
|
||||
/// on process-global env state.
|
||||
struct EnvScope {
|
||||
snapshots: Vec<(&'static str, Option<String>)>,
|
||||
_guard: std::sync::MutexGuard<'static, ()>,
|
||||
}
|
||||
|
||||
impl EnvScope {
|
||||
fn capture(names: &[&'static str]) -> Self {
|
||||
// Acquire before snapshotting so a concurrent test can't
|
||||
// mutate vars between our snapshot and restore.
|
||||
let guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
|
||||
let snapshots = names
|
||||
.iter()
|
||||
.map(|n| (*n, std::env::var(n).ok()))
|
||||
.collect();
|
||||
Self {
|
||||
snapshots,
|
||||
_guard: guard,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for EnvScope {
|
||||
fn drop(&mut self) {
|
||||
for (name, prev) in &self.snapshots {
|
||||
// SAFETY: ENV_LOCK serializes all env-mutating tests in
|
||||
// this mod, so concurrent set_var/remove_var calls from
|
||||
// tests-in-this-file are not possible. Other Cargo test
|
||||
// binaries run in separate processes.
|
||||
unsafe {
|
||||
match prev {
|
||||
Some(v) => std::env::set_var(name, v),
|
||||
None => std::env::remove_var(name),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn set_env(name: &str, value: &str) {
|
||||
// SAFETY: see EnvScope::drop note. Caller must hold ENV_LOCK
|
||||
// (acquired via EnvScope::capture).
|
||||
unsafe {
|
||||
std::env::set_var(name, value);
|
||||
}
|
||||
}
|
||||
|
||||
fn unset_env(name: &str) {
|
||||
// SAFETY: see EnvScope::drop note.
|
||||
unsafe {
|
||||
std::env::remove_var(name);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn from_env_reads_three_mx_rpc_vars() {
|
||||
let _scope = EnvScope::capture(&[
|
||||
"MX_RPC_USER",
|
||||
"MX_RPC_PASSWORD",
|
||||
"MX_RPC_DOMAIN",
|
||||
"COMPUTERNAME",
|
||||
"HOSTNAME",
|
||||
]);
|
||||
set_env("MX_RPC_USER", "alice");
|
||||
set_env("MX_RPC_PASSWORD", "secret");
|
||||
set_env("MX_RPC_DOMAIN", "TESTDOMAIN");
|
||||
set_env("COMPUTERNAME", "TESTHOST");
|
||||
unset_env("HOSTNAME");
|
||||
|
||||
let ctx = NtlmClientContext::from_env().unwrap();
|
||||
// Use the Debug projection (which exposes user/domain/workstation
|
||||
// but redacts the password) to assert the fields landed.
|
||||
let dbg = format!("{ctx:?}");
|
||||
assert!(dbg.contains("user: \"alice\""));
|
||||
assert!(dbg.contains("domain: \"TESTDOMAIN\""));
|
||||
assert!(dbg.contains("workstation: \"TESTHOST\""));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn from_env_missing_user_errors() {
|
||||
let _scope =
|
||||
EnvScope::capture(&["MX_RPC_USER", "MX_RPC_PASSWORD", "MX_RPC_DOMAIN"]);
|
||||
unset_env("MX_RPC_USER");
|
||||
set_env("MX_RPC_PASSWORD", "p");
|
||||
set_env("MX_RPC_DOMAIN", "d");
|
||||
let err = NtlmClientContext::from_env().unwrap_err();
|
||||
match err {
|
||||
NtlmError::MissingEnvVar { name } => assert_eq!(name, "MX_RPC_USER"),
|
||||
other => panic!("expected MissingEnvVar(MX_RPC_USER), got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn from_env_missing_password_errors() {
|
||||
let _scope =
|
||||
EnvScope::capture(&["MX_RPC_USER", "MX_RPC_PASSWORD", "MX_RPC_DOMAIN"]);
|
||||
set_env("MX_RPC_USER", "u");
|
||||
unset_env("MX_RPC_PASSWORD");
|
||||
set_env("MX_RPC_DOMAIN", "d");
|
||||
let err = NtlmClientContext::from_env().unwrap_err();
|
||||
assert!(matches!(
|
||||
err,
|
||||
NtlmError::MissingEnvVar {
|
||||
name: "MX_RPC_PASSWORD"
|
||||
}
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn from_env_missing_domain_errors() {
|
||||
let _scope =
|
||||
EnvScope::capture(&["MX_RPC_USER", "MX_RPC_PASSWORD", "MX_RPC_DOMAIN"]);
|
||||
set_env("MX_RPC_USER", "u");
|
||||
set_env("MX_RPC_PASSWORD", "p");
|
||||
unset_env("MX_RPC_DOMAIN");
|
||||
let err = NtlmClientContext::from_env().unwrap_err();
|
||||
assert!(matches!(
|
||||
err,
|
||||
NtlmError::MissingEnvVar {
|
||||
name: "MX_RPC_DOMAIN"
|
||||
}
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn from_env_accepts_empty_domain() {
|
||||
let _scope = EnvScope::capture(&[
|
||||
"MX_RPC_USER",
|
||||
"MX_RPC_PASSWORD",
|
||||
"MX_RPC_DOMAIN",
|
||||
"COMPUTERNAME",
|
||||
"HOSTNAME",
|
||||
]);
|
||||
set_env("MX_RPC_USER", "u");
|
||||
set_env("MX_RPC_PASSWORD", "p");
|
||||
set_env("MX_RPC_DOMAIN", "");
|
||||
set_env("COMPUTERNAME", "X");
|
||||
let ctx = NtlmClientContext::from_env().unwrap();
|
||||
let dbg = format!("{ctx:?}");
|
||||
assert!(dbg.contains("domain: \"\""));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_hostname_prefers_computername_over_hostname() {
|
||||
let _scope = EnvScope::capture(&["COMPUTERNAME", "HOSTNAME"]);
|
||||
set_env("COMPUTERNAME", "WIN_HOST");
|
||||
set_env("HOSTNAME", "POSIX_HOST");
|
||||
assert_eq!(local_hostname(), "WIN_HOST");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_hostname_falls_back_to_hostname_when_computername_unset() {
|
||||
let _scope = EnvScope::capture(&["COMPUTERNAME", "HOSTNAME"]);
|
||||
unset_env("COMPUTERNAME");
|
||||
set_env("HOSTNAME", "POSIX_HOST");
|
||||
assert_eq!(local_hostname(), "POSIX_HOST");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_hostname_returns_empty_when_neither_set() {
|
||||
let _scope = EnvScope::capture(&["COMPUTERNAME", "HOSTNAME"]);
|
||||
unset_env("COMPUTERNAME");
|
||||
unset_env("HOSTNAME");
|
||||
assert_eq!(local_hostname(), "");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user