diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 884b69525..d384288fc 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -2406,6 +2406,97 @@ The full normalization contract, which the property tests in `parse_pathext(None)` and `parse_pathext(Some("; ;"))` both yield `DEFAULT_PATHEXT`. +### Home-directory resolution ladders + +`stdlib::path::path_utils` resolves the user's home through two precedence +ladders — POSIX (`HOME`, then `USERPROFILE`) and Windows (those two, then the +`HOMEDRIVE`/`HOMEPATH` pair, then `HOMESHARE`). Both take an injected +`read_env` closure. `home_from_env` remains the sole platform-*selection* +point, and each ladder is gated to its own platform plus `test` +(`posix_home_from` is `#[cfg(any(not(windows), test))]`, `windows_home_from` +is `#[cfg(any(windows, test))]`), so a release build compiles only the ladder +it uses while the `test` arm keeps both reachable from any host. + +#### Ladder ownership and call sites + +- The ladders are `pub(super)` and owned by `stdlib::path`. They are not a + general home-directory utility: callers elsewhere use `expanduser`. +- `expanduser` resolves the home through the injected `HomeDirectory` value + and an injected `read_env` reader: `Explicit` and `Missing` never touch the + environment, and `Ambient` drives `home_from_env` with whatever reader the + caller supplied. The composition root lives at the registration boundary — + filter registration in `stdlib::path::filters` captures the process-backed + reader once, carrying the sanctioned site-level expectation — so + `path_utils` holds no process access at all. Tests inject their own reader, + covering the `Ambient` path without touching the process environment. + +#### Ladder composition rules + +- Keep each ladder free of platform *selection logic*, leaving that to + `home_from_env`. Gating decides only whether a ladder compiles, never which + one applies — that separation is what lets the `test` arm expose both + ladders to the CI host. +- Gate each ladder `#[cfg(any(windows, test))]` or its inverse, and have + `home_from_env` name only the ladder it selects. Compiling both + unconditionally would leave the inapplicable one dead in a release build, + which `-D warnings` rejects; the previous workaround — binding both as a + `(posix, windows)` function-pointer pair so the unused one counted as + referenced — was an artificial dead-code anchor and has been removed. + Bounded constants used by only one ladder (`HOME_SOURCE_DRIVE_PATH` and + `HOME_SOURCE_HOMESHARE`, both Windows-only) carry the same gate as the + ladder that reads them. +- The ladders report what the environment says. An empty value is passed + through rather than treated as unset for the single-variable readings + (`HOME`, `USERPROFILE`, `HOMESHARE`), and interpreting that is + `expanduser`'s concern, not theirs. The `HOMEDRIVE`/`HOMEPATH` pair is the + exception: it counts only when both halves are non-empty, since a bare + drive or a bare relative path is not a home directory; an incomplete pair + falls through to `HOMESHARE`. + +#### Home-resolution telemetry + +The ladders stay pure: each *returns* the resolved home paired with a bounded +`&'static str` label naming the rung that supplied it, and emits nothing. +`resolve_home` is the sole telemetry boundary, emitting a +`tracing::debug!` event for every resolution, plus an additional +`tracing::debug!` failure event when no home was available, with these +fields: + +Table: Home-resolution telemetry fields. + +| Field | Meaning | +| --- | --- | +| `event` | Always `stdlib.expanduser.home`, so the events are filterable. | +| `source` | The bounded label naming what supplied the home. | +| `found` | Whether a home was resolved at all. | +| `outcome` | Present only on the failure event: `home_unavailable`. | + +`source` is drawn from a closed set and is never derived from a value: + +- `home` — `HOME`. +- `userprofile` — `USERPROFILE`. +- `drive_path` — the `HOMEDRIVE`/`HOMEPATH` pair, both halves non-empty. +- `homeshare` — `HOMESHARE`. +- `explicit` — a configured `HomeDirectory::Explicit` value. +- `missing` — no source supplied a home. + +`resolve_home` also increments a counter, +`netsuke_stdlib_expanduser_home_total`, described once per process via a +`Once`-guarded `describe_counter!`, matching the pattern in +`stdlib::which::cache`. It carries two labels, both drawn from closed sets so +the series count is fixed by the code, never by the environment: `outcome` is +`found` or `home_unavailable`; `source` is the same bounded label set listed +above. It increments exactly once per resolution whatever the outcome, so the +counter totals resolutions rather than events — the failure path emits a +second *debug event* but no second sample. Both the success and failure cases +are pinned by tests in `src/stdlib/path/home_tests.rs`, which capture samples +through a local `metrics_util` `DebuggingRecorder` rather than the global one. + +The events carry no paths and no environment values: neither the resolved +home, nor a variable's contents, nor the expanded result. Adding a rung means +adding a label to the closed set above and pinning it in the ladder tests, not +recording the value that distinguished it. + ### Configuration discovery module layout `src/cli/discovery.rs` attaches several small `#[path = "..."]` modules that diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index a2867ee6f..c13fd8a03 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1218,6 +1218,13 @@ Implementation notes: - `expanduser` mirrors shell semantics by inspecting `HOME`, `USERPROFILE`, and on Windows the `HOMEDRIVE`/`HOMEPATH` or `HOMESHARE` fallbacks. Platform-specific forms such as `~user` remain unsupported. +- The sole process-environment read for home resolution lives at the + registration boundary: `register_expanduser` in `stdlib::path::filters` + captures a process-backed reader closure once and injects it into the pure + resolution ladders in `path_utils`. Those ladders never touch the ambient + environment directly; they only consult whatever reader they are given. + Tests supply their own readers, exercising the ladders without reaching the + process environment. - `with_suffix` removes dotted suffix segments (default `n = 1`) before appending the provided suffix. diff --git a/docs/stdlib-yaml-and-jinja-guide.md b/docs/stdlib-yaml-and-jinja-guide.md index 435312262..ef074745d 100644 --- a/docs/stdlib-yaml-and-jinja-guide.md +++ b/docs/stdlib-yaml-and-jinja-guide.md @@ -123,7 +123,19 @@ split the same input identically. `{{ 'input-link' | realpath }}`. - `path | expanduser` is host-observing because it reads the home-directory environment. It expands `~` and `~/...`; named-user forms such as `~alice` - are unsupported. For example, `{{ '~/cache' | expanduser }}`. + are unsupported. The filter tries a platform-specific ladder of variables + and uses the first one that resolves, treating an empty value as set (not + skipped) except in the `HOMEDRIVE`/`HOMEPATH` case noted below: + - POSIX hosts: `HOME`, then `USERPROFILE`. + - Windows hosts: `HOME`, then `USERPROFILE`, then `HOMEDRIVE` joined with + `HOMEPATH`, then `HOMESHARE`. The joined pair is used only when both + halves are non-empty because an empty `HOMEPATH` would yield a bare + drive letter (`C:`) and an empty `HOMEDRIVE` would yield a + current-drive path (`\me`); an incomplete pair falls through to + `HOMESHARE`. + - If none of these resolve, the filter fails with an error. + + For example, `{{ '~/cache' | expanduser }}`. ## Read and identify files diff --git a/docs/users-guide.md b/docs/users-guide.md index f14d2e86e..5c9e5610d 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -584,6 +584,15 @@ every helper's signature, defaults, purity, platform caveats, and executable examples. Host-observing helpers belong only in trusted manifests: Netsuke bounds command and network output, but does not sandbox template evaluation. +One helper deserves a note here because its result depends on the host's +environment. `path | expanduser` expands a leading `~` against the home +directory, resolved from `HOME` then `USERPROFILE` on POSIX hosts, and from +`HOME`, `USERPROFILE`, the `HOMEDRIVE`/`HOMEPATH` pair, then `HOMESHARE` on +Windows. The Windows pair counts only when both halves are non-empty; an +incomplete pair falls through to `HOMESHARE`. Named-user forms such as +`~alice` are unsupported, and when no home directory resolves at all, the +filter fails rather than passing the `~` through silently. + ## Use the command-line interface The top-level command shape is: diff --git a/proptest-regressions/stdlib/path/home_tests.txt b/proptest-regressions/stdlib/path/home_tests.txt new file mode 100644 index 000000000..b1a04fd06 --- /dev/null +++ b/proptest-regressions/stdlib/path/home_tests.txt @@ -0,0 +1,10 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +# Recorded while mutation-testing the property (the HOMEDRIVE emptiness +# check was disabled to prove the test detects it), not from a defect in +# the ladder itself. +cc be405399e1c660af7820653ee114f838dcaba6702e5fe1b47efc3a0a4953002c # shrinks to home = None, userprofile = None, homedrive = Some(""), homepath = Some("a"), homeshare = None diff --git a/src/stdlib/path/filters.rs b/src/stdlib/path/filters.rs index 31d3381a0..b5a9b097a 100644 --- a/src/stdlib/path/filters.rs +++ b/src/stdlib/path/filters.rs @@ -10,6 +10,23 @@ use super::{fs_utils, hash_utils, path_utils}; use crate::localization::{self, keys}; use crate::stdlib::config_types::HomeDirectory; +/// Register the `expanduser` filter. +/// +/// Composition root: the one sanctioned ambient environment read. The +/// process-backed reader is captured here, at the registration boundary, and +/// injected into `expanduser`, so `path_utils` holds no process access and +/// `HomeDirectory::Ambient` consults whatever reader registration supplies. +fn register_expanduser(env: &mut Environment<'_>, home_directory: HomeDirectory) { + #[expect( + clippy::disallowed_methods, + reason = "composition root: registration captures the process-backed reader once and injects it into the home ladders" + )] + let read_env = |key: &str| std::env::var(key).ok(); + env.add_filter("expanduser", move |raw: String| -> Result { + path_utils::expanduser(&raw, &home_directory, read_env) + }); +} + pub(crate) fn register_filters(env: &mut Environment<'_>, home_directory: HomeDirectory) { env.add_filter("basename", |raw: String| -> Result { Ok(path_utils::basename(Utf8Path::new(&raw))) @@ -39,9 +56,7 @@ pub(crate) fn register_filters(env: &mut Environment<'_>, home_directory: HomeDi env.add_filter("realpath", |raw: String| -> Result { path_utils::canonicalize_any(Utf8Path::new(&raw)).map(camino::Utf8PathBuf::into_string) }); - env.add_filter("expanduser", move |raw: String| -> Result { - path_utils::expanduser(&raw, &home_directory) - }); + register_expanduser(env, home_directory); env.add_filter("size", |raw: String| -> Result { fs_utils::file_size(Utf8Path::new(&raw)) }); diff --git a/src/stdlib/path/home_metrics_tests.rs b/src/stdlib/path/home_metrics_tests.rs new file mode 100644 index 000000000..4e17ba869 --- /dev/null +++ b/src/stdlib/path/home_metrics_tests.rs @@ -0,0 +1,79 @@ +//! Tests for the bounded home-resolution counter. +//! +//! Split from `home_tests`, which covers the ladders and the tracing events; +//! these cases pin the metric alone. A local `DebuggingRecorder` captures the +//! samples without touching the global recorder, so they stay as isolated as +//! the ladder cases. Both labels are drawn from closed sets in `path_utils`, +//! so the assertions below pin the series a dashboard would group by. + +use super::path_utils::{EXPANDUSER_HOME_TOTAL, expanduser}; +use crate::stdlib::config_types::HomeDirectory; +use metrics_util::MetricKind; +use metrics_util::debugging::{DebugValue, DebuggingRecorder}; +use rstest::rstest; + +/// Resolve one path under a local recorder and return the counter samples +/// as `(outcome, source, value)` triples. +fn samples_for( + home: &HomeDirectory, + read_env: impl Fn(&str) -> Option, +) -> Vec<(String, String, u64)> { + let recorder = DebuggingRecorder::new(); + let snapshotter = recorder.snapshotter(); + metrics::with_local_recorder(&recorder, || { + drop(expanduser("~/x", home, read_env)); + }); + snapshotter + .snapshot() + .into_vec() + .into_iter() + .filter_map(|(key, _unit, _description, value)| { + if key.kind() != MetricKind::Counter || key.key().name() != EXPANDUSER_HOME_TOTAL { + return None; + } + let label = |name: &str| { + key.key() + .labels() + .find(|label| label.key() == name) + .map(|label| label.value().to_owned()) + }; + let DebugValue::Counter(count) = value else { + return None; + }; + Some((label("outcome")?, label("source")?, count)) + }) + .collect() +} + +/// A resolution that finds a home records `found` against the rung that +/// supplied it — one sample, whichever branch resolved it. +#[rstest] +#[case::explicit(HomeDirectory::Explicit("/home/a".to_owned()), "explicit")] +#[case::ambient(HomeDirectory::Ambient, "home")] +fn a_resolved_home_is_counted_against_its_source( + #[case] home: HomeDirectory, + #[case] expected_source: &str, +) { + let read_env = |key: &str| (key == "HOME").then(|| "/home/a".to_owned()); + let samples = samples_for(&home, read_env); + assert_eq!( + samples, + vec![("found".to_owned(), expected_source.to_owned(), 1)], + "one sample labelled by outcome and source", + ); +} + +/// A resolution that finds nothing records the failure outcome against the +/// `missing` source, and still counts exactly once despite the second debug +/// event the failure path emits. +#[rstest] +#[case::missing(HomeDirectory::Missing)] +#[case::ambient_with_an_empty_reader(HomeDirectory::Ambient)] +fn an_unresolvable_home_is_counted_once(#[case] home: HomeDirectory) { + let samples = samples_for(&home, |_| None); + assert_eq!( + samples, + vec![("home_unavailable".to_owned(), "missing".to_owned(), 1)], + "the failure path adds an event, not a second sample", + ); +} diff --git a/src/stdlib/path/home_tests.rs b/src/stdlib/path/home_tests.rs new file mode 100644 index 000000000..30b54ca22 --- /dev/null +++ b/src/stdlib/path/home_tests.rs @@ -0,0 +1,329 @@ +//! Tests for the home-directory precedence ladders. +//! +//! The ladders are deliberately tested at this seam rather than through the +//! `expanduser` `MiniJinja` filter. Driving the filter with real environment +//! combinations would require mutating the process environment in-process, +//! which AGENTS.md forbids; the seam exists precisely so the precedence +//! logic is reachable without that (#486). The filter's own concerns — `~` +//! recognition, the named-user rejection, and the no-home error — are +//! exercised below on `expanduser` itself through the injected +//! [`HomeDirectory`] value the filter registration receives; the +//! filter-level suite in `tests/std_filter_tests` is dark pending #520. +//! +//! Both ladders are driven through their `read_env` closure, so nothing here +//! mutates the process environment and the cases run concurrently. Each helper +//! is gated to its own platform *plus* `test`, so both are in scope for this +//! module on every host while a release build compiles only the one it uses. +//! That `test` arm matters most for the Windows ladder, the more intricate of +//! the two, which would otherwise be unreachable from the Unix CI host. +//! +//! The bounded counter `resolve_home` increments is covered separately, in +//! `home_metrics_tests`. + +use super::path_utils::{HomeSource, posix_home_from, windows_home_from}; +use crate::stdlib::config_types::HomeDirectory; +use rstest::rstest; + +/// Borrow a resolution as `(home, source)` so cases can be written as literals. +fn as_pair(resolved: Option<&HomeSource>) -> Option<(&str, &str)> { + resolved.map(|(home, source)| (home.as_str(), *source)) +} + +mod expanduser_behaviour { + //! `expanduser` resolves the home through the injected [`HomeDirectory`] + //! value and `read_env` reader the filter registration supplies, so its + //! behaviour is testable here without touching the process environment. + //! Every branch below is drivable that way: `Explicit` supplies a home, + //! `Missing` models a host without one, and `Ambient` walks the platform + //! ladder through the injected reader — the isolation the seam exists to + //! provide. Cases whose branch never consults the environment pass + //! `|_| None` to prove they do not. + + use super::super::path_utils::expanduser; + use super::HomeDirectory; + use crate::test_tracing_capture::with_test_subscriber; + use rstest::rstest; + use tracing_subscriber::filter::LevelFilter; + + #[rstest] + #[case::tilde_alone("~", "/home/a")] + #[case::tilde_with_path("~/notes", "/home/a/notes")] + fn expands_against_the_resolved_home(#[case] raw: &str, #[case] expected: &str) { + let home = HomeDirectory::Explicit("/home/a".to_owned()); + let expanded = expanduser(raw, &home, |_| None).expect("expansion should succeed"); + assert_eq!(expanded, expected); + } + + /// `Ambient` resolves through the injected reader, not process state: + /// a reader supplying HOME drives the expansion without any process + /// environment involvement. + #[test] + fn ambient_resolves_through_the_injected_reader() { + let read_env = |key: &str| (key == "HOME").then(|| "/injected/home".to_owned()); + let expanded = expanduser("~/x", &HomeDirectory::Ambient, read_env) + .expect("the injected reader should supply the home"); + assert_eq!(expanded, "/injected/home/x"); + } + + /// `Ambient` with a reader that finds nothing is the no-home error. + #[test] + fn ambient_with_an_empty_reader_is_an_error() { + let error = expanduser("~/x", &HomeDirectory::Ambient, |_| None) + .expect_err("an empty reader should leave no home"); + assert_eq!(error.kind(), minijinja::ErrorKind::InvalidOperation); + } + + /// Capture the home-resolution events emitted by one `expanduser` call. + fn events_for( + raw: &str, + home: &HomeDirectory, + read_env: impl Fn(&str) -> Option, + ) -> Vec { + with_test_subscriber(LevelFilter::DEBUG, |captured| { + drop(expanduser(raw, home, read_env)); + captured.snapshot() + }) + .into_iter() + .filter(|event| event.contains("stdlib.expanduser.home")) + .collect() + } + + /// A successful ambient resolution reports the rung that supplied the home, + /// and nothing else: no path, no environment value. + #[test] + fn resolution_reports_its_bounded_source() { + let read_env = |key: &str| (key == "USERPROFILE").then(|| "/injected/home".to_owned()); + let events = events_for("~/x", &HomeDirectory::Ambient, read_env); + let event = events.first().expect("one home-resolution event"); + assert!( + event.contains("source=\"userprofile\"") && event.contains("found=true"), + "the event should name the rung and the outcome: {event}" + ); + assert!( + !events.iter().any(|other| other.contains("/injected/home")), + "no event may carry the resolved home: {events:?}" + ); + } + + /// A resolution that finds nothing records the bounded failure category + /// alongside the `missing` source. + #[test] + fn an_unresolvable_home_reports_a_bounded_outcome() { + let events = events_for("~/x", &HomeDirectory::Missing, |_| None); + assert!( + events.iter().any(|event| { + event.contains("outcome=\"home_unavailable\"") + && event.contains("source=\"missing\"") + }), + "the failure event should carry the bounded outcome: {events:?}" + ); + } + + /// A path without a leading `~` passes through unchanged, and the home + /// source is never consulted: `Missing` would otherwise make this an + /// error. + #[test] + fn a_non_tilde_path_passes_through() { + let expanded = expanduser("/etc/hosts", &HomeDirectory::Missing, |_| None) + .expect("a non-tilde path should pass through"); + assert_eq!(expanded, "/etc/hosts"); + } + + /// Named-user expansion is rejected before the home source is consulted. + #[test] + fn named_user_forms_are_rejected() { + let error = expanduser("~alice/notes", &HomeDirectory::Missing, |_| None) + .expect_err("~alice should be rejected"); + assert_eq!(error.kind(), minijinja::ErrorKind::InvalidOperation); + } + + /// A tilde with no resolvable home is an error, not a passthrough. + #[test] + fn a_missing_home_is_an_error() { + let error = expanduser("~/notes", &HomeDirectory::Missing, |_| None) + .expect_err("no home should be an error"); + assert_eq!(error.kind(), minijinja::ErrorKind::InvalidOperation); + } +} + +/// Build a lookup over a fixed set of pairs; anything absent reads as unset. +fn env_of<'a>(pairs: &'a [(&'a str, &'a str)]) -> impl Fn(&str) -> Option + 'a { + move |key| { + pairs + .iter() + .find(|(name, _)| *name == key) + .map(|(_, value)| (*value).to_owned()) + } +} + +/// Each case pins the resolved home *and* the bounded label naming the rung +/// that supplied it, since the label is what home-resolution telemetry reports. +#[rstest] +#[case::home_wins(&[("HOME", "/home/a"), ("USERPROFILE", "/users/b")], Some(("/home/a", "home")))] +#[case::falls_back_to_userprofile(&[("USERPROFILE", "/users/b")], Some(("/users/b", "userprofile")))] +#[case::nothing_set(&[], None)] +// An empty value is passed through rather than treated as unset: the ladder +// reports what the environment says, and `expanduser` decides what an empty +// home means. Pinned so the behaviour cannot drift silently. +#[case::empty_home_is_passed_through(&[("HOME", "")], Some(("", "home")))] +#[case::empty_userprofile_is_passed_through(&[("USERPROFILE", "")], Some(("", "userprofile")))] +fn posix_ladder(#[case] pairs: &[(&str, &str)], #[case] expected: Option<(&str, &str)>) { + let resolved = posix_home_from(env_of(pairs)); + assert_eq!(as_pair(resolved.as_ref()), expected); +} + +/// As above: every Windows rung is pinned by both value and source label. +#[rstest] +#[case::home_wins( + &[("HOME", "H:\\home"), ("USERPROFILE", "U:\\users"), ("HOMEDRIVE", "C:"), ("HOMEPATH", "\\me")], + Some(("H:\\home", "home")) +)] +#[case::userprofile_before_the_pair( + &[("USERPROFILE", "U:\\users"), ("HOMEDRIVE", "C:"), ("HOMEPATH", "\\me")], + Some(("U:\\users", "userprofile")) +)] +#[case::drive_and_path_pair( + &[("HOMEDRIVE", "C:"), ("HOMEPATH", "\\me")], + Some(("C:\\me", "drive_path")) +)] +#[case::homeshare_last( + &[("HOMESHARE", "\\\\server\\share")], + Some(("\\\\server\\share", "homeshare")) +)] +#[case::nothing_set(&[], None)] +fn windows_ladder(#[case] pairs: &[(&str, &str)], #[case] expected: Option<(&str, &str)>) { + let resolved = windows_home_from(env_of(pairs)); + assert_eq!(as_pair(resolved.as_ref()), expected); +} + +/// An incomplete `HOMEDRIVE`/`HOMEPATH` pair must not be combined. +/// +/// Joining them would yield a bare `C:`, which names a drive rather than a home +/// directory and would silently expand `~` to the drive root. The ladder falls +/// through to `HOMESHARE` instead. +#[rstest] +#[case::falls_through_to_homeshare( + &[("HOMEDRIVE", "C:"), ("HOMEPATH", ""), ("HOMESHARE", "\\\\server\\share")], + Some(("\\\\server\\share", "homeshare")) +)] +#[case::no_homeshare_yields_none(&[("HOMEDRIVE", "C:"), ("HOMEPATH", "")], None)] +// An empty HOMEDRIVE is equally incomplete: combining it would yield a bare +// `\me`, naming a path on the current drive rather than a home directory. +#[case::empty_homedrive_falls_through_to_homeshare( + &[("HOMEDRIVE", ""), ("HOMEPATH", "\\me"), ("HOMESHARE", "\\\\server\\share")], + Some(("\\\\server\\share", "homeshare")) +)] +#[case::empty_homedrive_without_homeshare_yields_none( + &[("HOMEDRIVE", ""), ("HOMEPATH", "\\me")], + None +)] +#[case::both_halves_empty_yields_none(&[("HOMEDRIVE", ""), ("HOMEPATH", "")], None)] +fn windows_incomplete_drive_pair_is_treated_as_unset( + #[case] pairs: &[(&str, &str)], + #[case] expected: Option<(&str, &str)>, +) { + let resolved = windows_home_from(env_of(pairs)); + assert_eq!(as_pair(resolved.as_ref()), expected); +} + +/// A `HOMEDRIVE` without `HOMEPATH` is incomplete and must not be used alone. +#[test] +fn windows_homedrive_without_homepath_falls_through() { + let pairs = [("HOMEDRIVE", "C:")]; + assert_eq!(windows_home_from(env_of(&pairs)), None); +} + +mod properties { + //! Property coverage for the precedence ladders. + //! + //! The fixed cases above pin named rungs; these state the whole contract + //! at once over every combination of set, unset, and empty variables, so + //! a drift in one rung — a dropped emptiness check, a reordered + //! fallback — fails here even if no enumerated case names it. + + use super::{posix_home_from, windows_home_from}; + use proptest::option; + use proptest::prelude::*; + + /// A variable's value; deliberately often empty, so the incomplete-pair + /// rule is generated rather than only enumerated. + fn value() -> impl Strategy { + prop_oneof![ + 1 => Just(String::new()), + 3 => "[a-z]{1,3}".prop_map(|s| s), + ] + } + + /// Lookup over the five optional variables. + fn reader<'a>( + pairs: &'a [(&'a str, &'a Option)], + ) -> impl Fn(&str) -> Option + 'a { + move |key| { + pairs + .iter() + .find(|(name, _)| *name == key) + .and_then(|(_, value)| (*value).clone()) + } + } + + proptest! { + /// The Windows ladder matches its documented precedence exactly. + /// + /// The expectation is computed from the documented contract — `HOME`, + /// then `USERPROFILE`, then a non-empty `HOMEDRIVE`/`HOMEPATH` pair, + /// then `HOMESHARE` — not by calling the ladder, so an implementation + /// change that departs from the contract cannot agree with itself. + #[test] + fn windows_ladder_matches_its_documented_precedence( + home in option::of(value()), + userprofile in option::of(value()), + homedrive in option::of(value()), + homepath in option::of(value()), + homeshare in option::of(value()), + ) { + let expected = home.clone().map(|value| (value, "home")) + .or_else(|| userprofile.clone().map(|value| (value, "userprofile"))) + .or_else(|| { + match (&homedrive, &homepath) { + (Some(drive), Some(path)) if !drive.is_empty() && !path.is_empty() => { + Some((format!("{drive}{path}"), "drive_path")) + } + _ => homeshare.clone().map(|value| (value, "homeshare")), + } + }); + let pairs = [ + ("HOME", &home), + ("USERPROFILE", &userprofile), + ("HOMEDRIVE", &homedrive), + ("HOMEPATH", &homepath), + ("HOMESHARE", &homeshare), + ]; + prop_assert_eq!(windows_home_from(reader(&pairs)), expected); + } + + /// The POSIX ladder reads only `HOME` and `USERPROFILE`. + /// + /// Its result must equal the two-variable precedence and must not + /// change when the Windows-only variables are present, whatever their + /// values. + #[test] + fn posix_ladder_reads_only_home_and_userprofile( + home in option::of(value()), + userprofile in option::of(value()), + homedrive in option::of(value()), + homepath in option::of(value()), + homeshare in option::of(value()), + ) { + let expected = home.clone().map(|value| (value, "home")) + .or_else(|| userprofile.clone().map(|value| (value, "userprofile"))); + let all = [ + ("HOME", &home), + ("USERPROFILE", &userprofile), + ("HOMEDRIVE", &homedrive), + ("HOMEPATH", &homepath), + ("HOMESHARE", &homeshare), + ]; + prop_assert_eq!(posix_home_from(reader(&all)), expected); + } + } +} diff --git a/src/stdlib/path/mod.rs b/src/stdlib/path/mod.rs index dda638be4..5c82addc9 100644 --- a/src/stdlib/path/mod.rs +++ b/src/stdlib/path/mod.rs @@ -7,5 +7,10 @@ mod fs_utils; mod hash_utils; mod path_utils; +#[cfg(test)] +mod home_metrics_tests; +#[cfg(test)] +mod home_tests; + pub(crate) use filters::register_filters; pub(crate) use fs_utils::file_type_matches; diff --git a/src/stdlib/path/path_utils.rs b/src/stdlib/path/path_utils.rs index ece668a21..652675949 100644 --- a/src/stdlib/path/path_utils.rs +++ b/src/stdlib/path/path_utils.rs @@ -1,11 +1,12 @@ //! Path utilities backing stdlib filters for UTF-8 paths: basename/dirname, `with_suffix`, //! `relative_to`, canonicalize/realpath, and expanduser with Windows HOME fallbacks. Uses cap-std //! directory handles and consistent error mapping for template errors. -use std::{env, io}; +use std::{io, sync::Once}; use cap_std::{ambient_authority, fs_utf8::Dir}; use camino::{Utf8Path, Utf8PathBuf}; +use metrics::{counter, describe_counter}; use minijinja::{Error, ErrorKind}; use super::fs_utils::{ParentDir, open_parent_dir}; @@ -113,7 +114,14 @@ pub(super) fn is_user_specific_expansion(stripped: &str) -> bool { ) } -pub(super) fn expanduser(raw: &str, home_directory: &HomeDirectory) -> Result { +pub(super) fn expanduser( + raw: &str, + home_directory: &HomeDirectory, + read_env: F, +) -> Result +where + F: Fn(&str) -> Option, +{ if let Some(stripped) = raw.strip_prefix('~') { if is_user_specific_expansion(stripped) { return Err(Error::new( @@ -121,7 +129,7 @@ pub(super) fn expanduser(raw: &str, home_directory: &HomeDirectory) -> Result) -> Utf8PathBuf { .map_or_else(|| Utf8PathBuf::from("."), Utf8Path::to_path_buf) } -fn resolve_home(home_directory: &HomeDirectory) -> Result { - let home = match home_directory { - HomeDirectory::Ambient => home_from_env(), +/// Resolve the home and report which source supplied it. +/// +/// This is the telemetry boundary for home resolution: the ladders stay pure +/// and merely *return* their bounded source label, and this function is the +/// only place that emits an event or increments a counter. Only the label is +/// recorded — never the resolved home, an environment value, or a variable's +/// contents. +fn resolve_home(home_directory: &HomeDirectory, read_env: F) -> Result +where + F: Fn(&str) -> Option, +{ + describe_home_metrics(); + let resolved = match home_directory { + HomeDirectory::Ambient => home_from_env(read_env), HomeDirectory::Missing => None, - HomeDirectory::Explicit(home) => Some(home.clone()), + HomeDirectory::Explicit(home) => Some((home.clone(), HOME_SOURCE_EXPLICIT)), }; - home.ok_or_else(|| { + let source = resolved + .as_ref() + .map_or(HOME_SOURCE_MISSING, |(_, source)| *source); + let outcome = if resolved.is_some() { + HOME_OUTCOME_FOUND + } else { + HOME_OUTCOME_UNAVAILABLE + }; + tracing::debug!( + event = EXPANDUSER_HOME_EVENT, + source, + found = resolved.is_some(), + "resolved the home directory for expanduser", + ); + // Exactly one increment per resolution, whatever the outcome, so the + // counter totals resolutions rather than events: the failure path below + // adds a second *event* but no second sample. + counter!( + EXPANDUSER_HOME_TOTAL, + "outcome" => outcome, + "source" => source, + ) + .increment(1); + resolved.map(|(home, _)| home).ok_or_else(|| { + tracing::debug!( + event = EXPANDUSER_HOME_EVENT, + source, + outcome = HOME_OUTCOME_UNAVAILABLE, + "expanduser found no home directory", + ); Error::new( ErrorKind::InvalidOperation, localization::message(keys::STDLIB_PATH_EXPANDUSER_NO_HOME).to_string(), @@ -157,28 +205,154 @@ fn current_dir_utf8() -> Result { dir.canonicalize(Utf8Path::new(".")) } -#[cfg(windows)] -#[expect( - clippy::disallowed_methods, - reason = "composition root: home resolution is the path stdlib's ambient boundary; the injected ladders are tracked in the environment meta issue" -)] -fn home_from_env() -> Option { - env::var("HOME") - .or_else(|_| env::var("USERPROFILE")) - .ok() - .or_else( - || match (env::var("HOMEDRIVE").ok(), env::var("HOMEPATH").ok()) { - (Some(drive), Some(path)) if !path.is_empty() => Some(format!("{drive}{path}")), - _ => env::var("HOMESHARE").ok(), - }, - ) +/// The `event` field naming every home-resolution telemetry event. +pub(super) const EXPANDUSER_HOME_EVENT: &str = "stdlib.expanduser.home"; + +/// Counts home resolutions by bounded `outcome` and `source`. +/// +/// Both labels are drawn from the closed sets below, so the series count is +/// fixed by the code rather than by anything the environment supplies. +pub(super) const EXPANDUSER_HOME_TOTAL: &str = "netsuke_stdlib_expanduser_home_total"; + +/// The bounded `outcome` recorded when a source supplied a home. +pub(super) const HOME_OUTCOME_FOUND: &str = "found"; + +/// The bounded `outcome` recorded when no source supplied a home. +pub(super) const HOME_OUTCOME_UNAVAILABLE: &str = "home_unavailable"; + +/// Describe the home-resolution counter once per process. +fn describe_home_metrics() { + static DESCRIBE: Once = Once::new(); + DESCRIBE.call_once(|| { + describe_counter!( + EXPANDUSER_HOME_TOTAL, + "Counts expanduser home resolutions labelled by outcome (found or \ + home_unavailable) and by the bounded source that supplied the home." + ); + }); +} + +/// `HOME` supplied the home directory. +pub(super) const HOME_SOURCE_HOME: &str = "home"; +/// `USERPROFILE` supplied the home directory. +pub(super) const HOME_SOURCE_USERPROFILE: &str = "userprofile"; +/// The `HOMEDRIVE`/`HOMEPATH` pair supplied the home directory. +/// +/// Gated with [`windows_home_from`], its only reader: the rungs it labels +/// exist solely on that ladder. +#[cfg(any(windows, test))] +pub(super) const HOME_SOURCE_DRIVE_PATH: &str = "drive_path"; +/// `HOMESHARE` supplied the home directory. +/// +/// Gated with [`windows_home_from`], its only reader. +#[cfg(any(windows, test))] +pub(super) const HOME_SOURCE_HOMESHARE: &str = "homeshare"; +/// A configured [`HomeDirectory::Explicit`] value supplied the home directory. +pub(super) const HOME_SOURCE_EXPLICIT: &str = "explicit"; +/// No source supplied a home directory. +pub(super) const HOME_SOURCE_MISSING: &str = "missing"; + +/// A resolved home paired with the bounded label naming what supplied it. +/// +/// The label is a `&'static str` drawn from the closed set above, so it can be +/// recorded as telemetry without ever exposing a path or an environment value. +pub(super) type HomeSource = (String, &'static str); + +/// Select the platform ladder and drive it with the injected reader. +/// +/// This is the module's only platform selection, and it names just the ladder +/// it selects. Each ladder is gated to its own platform plus `test`, so a +/// release build compiles exactly one of them and the other is absent rather +/// than dead — no reference exists solely to keep the compiler quiet. +/// +/// The reader is injected all the way from the filter-registration boundary, +/// so this module holds no process access of its own: whoever registers the +/// `expanduser` filter decides what [`HomeDirectory::Ambient`] consults. +fn home_from_env(read_env: F) -> Option +where + F: Fn(&str) -> Option, +{ + #[cfg(windows)] + { + windows_home_from(read_env) + } + #[cfg(not(windows))] + { + posix_home_from(read_env) + } } -#[cfg(not(windows))] -#[expect( - clippy::disallowed_methods, - reason = "composition root: home resolution is the path stdlib's ambient boundary; the injected ladders are tracked in the environment meta issue" -)] -fn home_from_env() -> Option { - env::var("HOME").or_else(|_| env::var("USERPROFILE")).ok() +/// Resolve the home directory using the POSIX precedence ladder. +/// +/// Returns the home alongside the bounded label naming the rung that supplied +/// it, so the caller can report the source without inspecting any value. +/// +/// Holds no platform-selection logic of its own — that lives solely in +/// [`home_from_env`] — so the `test` arm of the gate below makes it reachable +/// from any host. +/// +/// # Examples +/// +/// ```rust,ignore +/// let env = |key: &str| (key == "HOME").then(|| String::from("/home/a")); +/// assert_eq!(posix_home_from(env), Some((String::from("/home/a"), "home"))); +/// ``` +#[cfg(any(not(windows), test))] +pub(super) fn posix_home_from(read_env: F) -> Option +where + F: Fn(&str) -> Option, +{ + read_env("HOME") + .map(|home| (home, HOME_SOURCE_HOME)) + .or_else(|| read_env("USERPROFILE").map(|home| (home, HOME_SOURCE_USERPROFILE))) +} + +/// Resolve the home directory using the Windows precedence ladder. +/// +/// `HOME` and `USERPROFILE` first, then the `HOMEDRIVE`/`HOMEPATH` pair, and +/// finally `HOMESHARE`. An empty `HOMEPATH` is treated as unset, because +/// joining it to `HOMEDRIVE` would yield a bare drive letter rather than a home +/// directory. +/// +/// Returns the home alongside the bounded label naming the rung that supplied +/// it, so the caller can report the source without inspecting any value. +/// +/// Holds no platform-selection logic of its own — that lives solely in +/// [`home_from_env`] — so the `test` arm of the gate below makes it reachable +/// from any host. That arm is what lets the Unix CI host exercise this ladder, +/// the more intricate of the two. +/// +/// # Examples +/// +/// ```rust,ignore +/// // The drive and path pair combine when HOMEPATH is non-empty. +/// let env = |key: &str| match key { +/// "HOMEDRIVE" => Some(String::from("C:")), +/// "HOMEPATH" => Some(String::from("\\me")), +/// _ => None, +/// }; +/// assert_eq!( +/// windows_home_from(env), +/// Some((String::from("C:\\me"), "drive_path")), +/// ); +/// ``` +#[cfg(any(windows, test))] +pub(super) fn windows_home_from(read_env: F) -> Option +where + F: Fn(&str) -> Option, +{ + read_env("HOME") + .map(|home| (home, HOME_SOURCE_HOME)) + .or_else(|| read_env("USERPROFILE").map(|home| (home, HOME_SOURCE_USERPROFILE))) + .or_else(|| match (read_env("HOMEDRIVE"), read_env("HOMEPATH")) { + // Both halves must be non-empty. An empty HOMEDRIVE would yield a + // bare `\me`, which names a path on the current drive rather than + // a home directory; an empty HOMEPATH would yield a bare `C:`, + // which names a drive. Either way the pair is incomplete, so fall + // through to HOMESHARE. + (Some(drive), Some(path)) if !drive.is_empty() && !path.is_empty() => { + Some((format!("{drive}{path}"), HOME_SOURCE_DRIVE_PATH)) + } + _ => read_env("HOMESHARE").map(|home| (home, HOME_SOURCE_HOMESHARE)), + }) } diff --git a/tests/stdlib_expanduser_filter_tests.rs b/tests/stdlib_expanduser_filter_tests.rs new file mode 100644 index 000000000..95b2d9892 --- /dev/null +++ b/tests/stdlib_expanduser_filter_tests.rs @@ -0,0 +1,115 @@ +//! Integration tests for the registered `expanduser` `MiniJinja` filter. +//! +//! The precedence ladders are covered exhaustively at their injected-reader +//! seam (`src/stdlib/path/home_tests.rs`); these cases prove the *registered +//! filter* — the template-visible boundary — behaves per contract. The +//! successful `~/...` cases run against a home configured through +//! `with_home_override`, so no test consults or mutates the process +//! environment, which the AGENTS.md testing mandate forbids; the `Ambient` +//! ladders behind the same registration closure are the seam tests' concern. + +use anyhow::{Context, Result, anyhow, ensure}; +use camino::{Utf8Path, Utf8PathBuf}; +use cap_std::{ambient_authority, fs_utf8::Dir}; +use minijinja::{Environment, context}; +use netsuke::stdlib::{self, StdlibConfig}; +use rstest::{fixture, rstest}; + +struct StdlibWorkspace { + _temp: tempfile::TempDir, + root: Utf8PathBuf, +} + +#[fixture] +fn stdlib_workspace() -> Result { + let temp = tempfile::tempdir().context("create temp workspace")?; + let root = Utf8PathBuf::from_path_buf(temp.path().to_path_buf()) + .map_err(|path| anyhow!("temp path should be UTF-8: {path:?}"))?; + Ok(StdlibWorkspace { _temp: temp, root }) +} + +fn stdlib_env(root: &Utf8Path) -> Result> { + let workspace = Dir::open_ambient_dir(root, ambient_authority()) + .with_context(|| format!("open workspace {root}"))?; + let config = StdlibConfig::new(workspace)?.with_workspace_root_path(root.to_path_buf())?; + let mut env = Environment::new(); + stdlib::register_with_config(&mut env, config)?; + Ok(env) +} + +fn render(env: &Environment<'static>, template: &str) -> Result { + env.render_str(template, context! {}) +} + +/// A tilde path renders to the configured home through the registered filter. +/// +/// This is the end-to-end success path at the template-visible boundary: the +/// home comes from `with_home_override`, so the ladder consults no ambient +/// environment, and the assertion covers the full route — template, filter +/// registration, `expanduser`, and home resolution. +#[rstest] +#[case::bare_tilde("{{ '~' | expanduser }}", "")] +#[case::tilde_subpath("{{ '~/notes/todo.txt' | expanduser }}", "/notes/todo.txt")] +fn a_tilde_path_expands_against_the_configured_home( + stdlib_workspace: Result, + #[case] template: &str, + #[case] suffix: &str, +) -> Result<()> { + let workspace = stdlib_workspace?; + let home = workspace.root.join("home/dweller"); + let dir = Dir::open_ambient_dir(&workspace.root, ambient_authority()) + .with_context(|| format!("open workspace {}", workspace.root))?; + let config = StdlibConfig::new(dir)? + .with_workspace_root_path(&workspace.root)? + .with_home_override(Some(home.to_string())); + let mut env = Environment::new(); + stdlib::register_with_config(&mut env, config)?; + + let rendered = render(&env, template).context("a tilde path should render")?; + let expected = format!("{home}{suffix}"); + ensure!( + rendered == expected, + "expected {expected:?}, got {rendered:?}" + ); + Ok(()) +} + +/// A path without a leading tilde passes through the filter unchanged. +/// +/// This branch reads no environment at all, so it proves the registration — +/// name, argument plumbing, and return path — with no ambient dependency. +#[rstest] +fn a_non_tilde_path_passes_through_unchanged( + stdlib_workspace: Result, +) -> Result<()> { + let workspace = stdlib_workspace?; + let env = stdlib_env(&workspace.root)?; + let rendered = render(&env, "{{ '/etc/hosts' | expanduser }}") + .context("a non-tilde path should render")?; + ensure!( + rendered == "/etc/hosts", + "expected passthrough, got {rendered:?}" + ); + Ok(()) +} + +/// Named-user expansion is rejected at the filter boundary. +/// +/// The rejection happens before any environment read, so the error is +/// deterministic on every host; asserting on the rendered error proves the +/// filter surfaces it to templates rather than swallowing it. +#[rstest] +fn named_user_forms_are_rejected_at_the_filter( + stdlib_workspace: Result, +) -> Result<()> { + let workspace = stdlib_workspace?; + let env = stdlib_env(&workspace.root)?; + let error = render(&env, "{{ '~alice/notes' | expanduser }}") + .err() + .context("~alice should be rejected")?; + ensure!( + error.to_string().contains("expansion is unsupported"), + "the error should state the unsupported expansion, got {error}" + ); + Ok(()) +}