From d3f752642b998de2a061c693d52aa83a1fcabd98 Mon Sep 17 00:00:00 2001 From: Ghost Scripter Date: Thu, 28 May 2026 03:15:25 +0530 Subject: [PATCH 1/3] fix(observability): classify RPC filesystem path validation errors as expected (Sentry TAURI-RUST-4QH) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `ExpectedErrorKind::FilesystemUserPathInvalid` variant and `is_filesystem_user_path_invalid_message` predicate anchored on the literal phrase `"path is not a directory:"`. Routes the dispatcher's `report_error_or_expected` call (via `src/core/jsonrpc.rs:135-141`) through the demote path so RPC-level user-input validation failures emit a `tracing::warn!` breadcrumb instead of a Sentry error event. Drops Sentry TAURI-RUST-4QH — `vault_create` called with a `root_path` that doesn't resolve to an existing directory. Emission site: `src/openhuman/vault/ops.rs:37`. Observed ~2 events/day on `openhuman@0.56.0`, `domain=rpc method=openhuman.vault_create operation=invoke_method`. Also preempts the symmetric `"hosted path is not a directory:"` shape from `openhuman::http_host::path_utils:23` once it surfaces. Anchor design: trailing colon discriminates user input (path follows the colon, as both wire shapes emit) from invariant violations. The `skills::ops_install:475` SAFETY GUARD (`"{path} is not a directory — refusing to remove"`, em-dash, no colon) stays actionable — it catches an `rm -rf` invariant violation, which IS a code bug. Pinned by the rejection test. Bonus: the user-supplied path embeds OS username via the home-directory prefix (`/Users//...`), so demoting these out of Sentry is a small privacy win on top of the noise reduction. 3 new tests: vault wire shape (verbatim TAURI-RUST-4QH body + the dispatcher's `rpc.invoke_method failed:` re-wrap), http_host wire shape, and a 4-case rejection contract covering the safety guard, a narrative log line, the inverse `Is a directory (os error 21)` EISDIR rendering, and an adjacent vault validation error (`"root_path must be absolute:"`) that's actionable. `cargo test --lib core::observability::tests` → 91 passed. Sentry-Issue: TAURI-RUST-4QH --- src/core/observability.rs | 162 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 162 insertions(+) diff --git a/src/core/observability.rs b/src/core/observability.rs index 4c93b2b02..f487f398e 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -192,6 +192,27 @@ pub enum ExpectedErrorKind { /// state snapshots) — every one of them emits the same canonical errno /// rendering. DiskFull, + /// A user-supplied filesystem path failed an RPC-level validation + /// check — e.g. `openhuman.vault_create` was called with a + /// `root_path` that doesn't exist or points at a file rather than a + /// directory. The UI already shows the typed error to the user, and + /// Sentry has no remediation path (we can't `mkdir -p` a folder the + /// user hasn't actually picked yet). User-supplied paths can also + /// embed PII fragments (the home-directory segment leaks the OS + /// username), so demoting these out of the Sentry event stream is a + /// small privacy win on top of the noise reduction. + /// + /// Drops Sentry TAURI-RUST-4QH (`root_path is not a directory: + /// /Users//Documents/`, observed on + /// `openhuman@0.56.0`) and preempts the symmetric + /// `hosted path is not a directory:` shape from + /// `openhuman::http_host::path_utils` once it starts surfacing. + /// See [`is_filesystem_user_path_invalid_message`] for the polarity + /// contract — the safety-guard variant in `skills::ops_install` + /// (`{path} is not a directory — refusing to remove`) is + /// deliberately not matched because that's an `rm -rf` invariant + /// violation, not user input. + FilesystemUserPathInvalid, /// A memory-store write (document upsert or KV set) was rejected because /// the namespace or key contained what the PII guard classified as a /// personal identifier (national ID, phone number, formatted credential, @@ -343,6 +364,13 @@ pub fn expected_error_kind(message: &str) -> Option { if is_empty_provider_response_message(&lower) { return Some(ExpectedErrorKind::EmptyProviderResponse); } + // RPC-level filesystem path validation — explicit wire-shape anchors + // (root_path / hosted path) prevent accidental demotion of unrelated + // errors. See the variant doc-comment and + // [`is_filesystem_user_path_invalid_message`] polarity contract. + if is_filesystem_user_path_invalid_message(&lower) { + return Some(ExpectedErrorKind::FilesystemUserPathInvalid); + } None } @@ -971,6 +999,42 @@ fn is_prompt_injection_blocked_message(lower: &str) -> bool { || lower.contains("prompt blocked by security policy") } +/// Detect an RPC-level filesystem path validation failure from user input. +/// +/// Anchored on the literal phrase `"path is not a directory:"` (with the +/// trailing colon followed by the user-supplied path). This matches the +/// two known wire shapes — both emitted at the RPC entry boundary when a +/// user typed/picked a path that doesn't resolve to an existing directory: +/// +/// - `"root_path is not a directory: "` — +/// [`crate::openhuman::vault::ops::vault_create`] when the chosen vault +/// folder doesn't exist or points at a file (Sentry TAURI-RUST-4QH). +/// - `"hosted path is not a directory: "` — +/// [`crate::openhuman::http_host::path_utils`] when an HTTP host config +/// references a missing directory. Not yet observed in Sentry but +/// shares the same user-input failure mode; preempts a future ID. +/// +/// Both are deterministic Err returns at the validation gate of an RPC +/// handler, BEFORE any side-effect happens. The UI already surfaces the +/// typed error and Sentry has no remediation path. +/// +/// **Polarity contract** — the trailing colon discriminates user input +/// from invariant violations: +/// +/// - `skills::ops_install` emits `"{path} is not a directory — refusing +/// to remove"` (em-dash separator, no colon after `"directory"`). That +/// is an `rm -rf` safety guard catching an UNEXPECTED state, not user +/// input — it must STAY actionable. +/// - A narrative log line like `"this path is not a directory check +/// passed"` lacks the colon and won't classify. +/// +/// All matches are substring-based against the lower-cased message so +/// the classifier survives caller wrapping (`rpc.invoke_method`, +/// anyhow context chains, …). +fn is_filesystem_user_path_invalid_message(lower: &str) -> bool { + lower.contains("path is not a directory:") +} + /// Detect memory-store writes rejected because the namespace or key contained /// a personal identifier detected by the PII guard. /// @@ -1286,6 +1350,25 @@ fn report_expected_message(kind: ExpectedErrorKind, message: &str, domain: &str, "[observability] {domain}.{operation} skipped expected memory-store circuit-breaker-open error" ); } + ExpectedErrorKind::FilesystemUserPathInvalid => { + // User-input validation failure surfaced at the RPC + // boundary — e.g. `openhuman.vault_create` called with a + // `root_path` that doesn't exist. The typed error is + // already shown to the user; Sentry has no remediation + // path. Demote to `warn!` (same tier as `BackendUserError`) + // so the local trace still pins which RPC + which method + // tripped the gate, but no Sentry event fires. The + // `error = %message` field intentionally retains the + // user-supplied path so the local log can correlate with a + // user bug report — Sentry doesn't see it. + tracing::warn!( + domain = domain, + operation = operation, + kind = "filesystem_user_path_invalid", + error = %message, + "[observability] {domain}.{operation} skipped expected filesystem path validation error: {message}" + ); + } ExpectedErrorKind::MemoryStorePiiRejection => { // PII guard rejected a memory-store write because the namespace or // key was classified as containing a personal identifier. The guard @@ -2034,6 +2117,85 @@ mod tests { } } + // ── FilesystemUserPathInvalid (TAURI-RUST-4QH) ───────────────────────── + + #[test] + fn classifies_vault_create_root_path_not_a_directory_as_filesystem_user_path_invalid() { + // TAURI-RUST-4QH: verbatim wire shape from + // `openhuman::vault::ops::vault_create` line 37 when the + // user-picked vault folder doesn't resolve to an existing + // directory. Bubbles up as the RPC dispatcher's + // `display_message` and reaches `report_error_or_expected` — + // must classify so no Sentry event fires. + assert_eq!( + expected_error_kind( + "root_path is not a directory: /Users/zadam/Documents/SndBrainOpenHuman" + ), + Some(ExpectedErrorKind::FilesystemUserPathInvalid) + ); + + // The same body wrapped by the JSON-RPC dispatcher's `display_message` + // prefix (`rpc.invoke_method` re-emit shape from `src/core/jsonrpc.rs`). + // Must still classify so the dispatch-site re-report doesn't escape + // the matcher even if a future caller layers more context. + assert_eq!( + expected_error_kind( + "rpc.invoke_method failed: root_path is not a directory: /Users/alice/openhuman-data" + ), + Some(ExpectedErrorKind::FilesystemUserPathInvalid) + ); + } + + #[test] + fn classifies_http_host_hosted_path_not_a_directory_as_filesystem_user_path_invalid() { + // Preempt the symmetric shape from + // `openhuman::http_host::path_utils:23` — + // `"hosted path is not a directory: "`. Not yet observed + // in Sentry but shares the same RPC validation polarity as + // vault_create's `root_path` check. Anchoring on + // `"path is not a directory:"` (with trailing colon) covers + // both without two separate matchers. + assert_eq!( + expected_error_kind("hosted path is not a directory: /var/www/static-site"), + Some(ExpectedErrorKind::FilesystemUserPathInvalid) + ); + } + + #[test] + fn does_not_classify_unrelated_path_messages_as_filesystem_user_path_invalid() { + // Polarity contract — the anchor requires a trailing colon + // after `"is not a directory"`, which discriminates user input + // (path follows the colon) from other shapes: + // + // 1. The `skills::ops_install:475` SAFETY GUARD — + // `" is not a directory — refusing to remove"` — must + // stay actionable. It catches an `rm -rf` invariant violation + // (the target should have been a directory but wasn't), + // which is a code bug, not user input. + // 2. A narrative log line that happens to mention the phrase + // without the user-path colon suffix is not a validation + // failure and must not be silenced. + // 3. The dot-prefix variant from POSIX `EISDIR`/`ENOTDIR` + // renderings (`"Is a directory (os error 21)"`) is the + // inverse condition — different code path entirely. + for raw in [ + // Safety guard — must NOT classify. + "/tmp/openhuman-cache is not a directory — refusing to remove", + // Narrative log line — must NOT classify. + "checked that path is not a directory before mkdir", + // Inverse condition (os error 21: EISDIR) — must NOT classify. + "open /etc/passwd failed: Is a directory (os error 21)", + // Bare path with no `directory` mention — must NOT classify. + "root_path must be absolute: ./relative/path", + ] { + assert_eq!( + expected_error_kind(raw), + None, + "polarity contract: must NOT classify as FilesystemUserPathInvalid: {raw}" + ); + } + } + // ── EmptyProviderResponse (TAURI-RUST-4Z1) ───────────────────────────── #[test] From ee3f1c3801f4bbb506f28459da78b0339bc8137b Mon Sep 17 00:00:00 2001 From: Ghost Scripter Date: Thu, 28 May 2026 03:37:39 +0530 Subject: [PATCH 2/3] fix(observability): redact user filesystem path from FilesystemUserPathInvalid breadcrumb (review of #2795) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original commit on this branch included `error = %message` plus `{message}` interpolation in the demoted `tracing::warn!` body. The intent was "warn-level local log for bug-report correlation, Sentry doesn't see it." That intent is wrong: - `src/core/logging.rs:366` maps `Level::WARN` to `EventFilter::Breadcrumb` in the `sentry_tracing_layer`. - Every `tracing::warn!` therefore becomes a Sentry breadcrumb that attaches to any subsequent event captured by the same hub. - The user-supplied path embeds the local filesystem layout (`/Users//Documents//…`) — PII that should never reach Sentry, even when the immediate classifier decision is "skip the event." Mirror the existing `LoopbackUnavailable` arm: drop the `error` structured field and the `{message}` interpolation, keep only `domain` / `operation` / `kind`. Comment block updated to explain the redaction reasoning and point at the `core::logging` layer mapping that makes it load-bearing. Full-path diagnostics for local debugging remain available via `RUST_LOG=openhuman_core::core::observability=debug` since `Level::DEBUG` / `TRACE` are mapped to `EventFilter::Ignore`. Classifier behaviour is unchanged — the three `is_filesystem_user_path_invalid_message` tests already on this branch continue to pass (3/3), and the broader observability suite stays green (91 tests). ## Test plan - [x] `cargo test filesystem_user_path_invalid` — 3 tests pass - [x] `cargo test core::observability` — 91 tests pass, 0 regressions - [x] `cargo check --bin openhuman-core` — passes - [x] `cargo fmt --check` — clean --- src/core/observability.rs | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/core/observability.rs b/src/core/observability.rs index f487f398e..4e994aee1 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -1355,18 +1355,28 @@ fn report_expected_message(kind: ExpectedErrorKind, message: &str, domain: &str, // boundary — e.g. `openhuman.vault_create` called with a // `root_path` that doesn't exist. The typed error is // already shown to the user; Sentry has no remediation - // path. Demote to `warn!` (same tier as `BackendUserError`) - // so the local trace still pins which RPC + which method - // tripped the gate, but no Sentry event fires. The - // `error = %message` field intentionally retains the - // user-supplied path so the local log can correlate with a - // user bug report — Sentry doesn't see it. + // path. Demote to `warn!` so the local trace still pins + // which RPC + which method tripped the gate. + // + // **Do not include the raw `message` here.** The message + // body embeds the user's local filesystem layout (username, + // project name, document directory, …) and + // `sentry_tracing_layer` in `core::logging` maps + // `Level::WARN` to `EventFilter::Breadcrumb` — so any + // formatted body would be attached as a breadcrumb to + // every subsequent Sentry event from this hub, leaking + // user paths into unrelated reports. Log only `domain` / + // `operation` / `kind` (no PII), matching the + // `LoopbackUnavailable` arm above ("metadata over raw text + // for noise demotions", per the #1719 review feedback). + // Full-path diagnostics for local debugging stay available + // via `RUST_LOG=…=debug` since `Level::DEBUG` / `TRACE` + // are mapped to `EventFilter::Ignore`. tracing::warn!( domain = domain, operation = operation, kind = "filesystem_user_path_invalid", - error = %message, - "[observability] {domain}.{operation} skipped expected filesystem path validation error: {message}" + "[observability] {domain}.{operation} skipped expected filesystem path validation error" ); } ExpectedErrorKind::MemoryStorePiiRejection => { From 91f47203278ca35ed56af538bbe36247becbef82 Mon Sep 17 00:00:00 2001 From: M3gA-Mind Date: Fri, 29 May 2026 01:00:20 +0530 Subject: [PATCH 3/3] fix(observability): tighten FilesystemUserPathInvalid anchor and demote level MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Tighten is_filesystem_user_path_invalid_message to explicit wire shapes (root_path / hosted path) instead of generic "path is not a directory:" so future unrelated errors can't accidentally be silenced. - Change tracing::warn! → tracing::info! to match PromptInjectionBlocked tier (both are "user input we already surfaced a typed error for"). - Add polarity test for generic "input config path is not a directory:" confirming it does NOT classify. Addresses major review findings from oxoxDev. --- src/core/observability.rs | 40 +++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/src/core/observability.rs b/src/core/observability.rs index 4e994aee1..c53eabdd7 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -1001,10 +1001,9 @@ fn is_prompt_injection_blocked_message(lower: &str) -> bool { /// Detect an RPC-level filesystem path validation failure from user input. /// -/// Anchored on the literal phrase `"path is not a directory:"` (with the -/// trailing colon followed by the user-supplied path). This matches the -/// two known wire shapes — both emitted at the RPC entry boundary when a -/// user typed/picked a path that doesn't resolve to an existing directory: +/// Anchored on the two known wire shapes — both emitted at the RPC entry +/// boundary when a user typed/picked a path that doesn't resolve to an +/// existing directory: /// /// - `"root_path is not a directory: "` — /// [`crate::openhuman::vault::ops::vault_create`] when the chosen vault @@ -1018,21 +1017,24 @@ fn is_prompt_injection_blocked_message(lower: &str) -> bool { /// handler, BEFORE any side-effect happens. The UI already surfaces the /// typed error and Sentry has no remediation path. /// -/// **Polarity contract** — the trailing colon discriminates user input -/// from invariant violations: +/// **Polarity contract** — explicit wire-shape anchors prevent accidental +/// demotion of future errors whose bodies happen to contain "path is not +/// a directory:" in a different context: /// /// - `skills::ops_install` emits `"{path} is not a directory — refusing -/// to remove"` (em-dash separator, no colon after `"directory"`). That -/// is an `rm -rf` safety guard catching an UNEXPECTED state, not user -/// input — it must STAY actionable. -/// - A narrative log line like `"this path is not a directory check -/// passed"` lacks the colon and won't classify. +/// to remove"` (em-dash separator, no "root_path" or "hosted path" +/// prefix). That is an `rm -rf` safety guard catching an UNEXPECTED +/// state, not user input — it must STAY actionable. +/// - A generic `"input config path is not a directory: /etc/foo"` from a +/// future provider/wallet/storage error would NOT match (no known +/// prefix) and would reach Sentry as intended. /// /// All matches are substring-based against the lower-cased message so /// the classifier survives caller wrapping (`rpc.invoke_method`, /// anyhow context chains, …). fn is_filesystem_user_path_invalid_message(lower: &str) -> bool { - lower.contains("path is not a directory:") + lower.contains("root_path is not a directory:") + || lower.contains("hosted path is not a directory:") } /// Detect memory-store writes rejected because the namespace or key contained @@ -1355,14 +1357,16 @@ fn report_expected_message(kind: ExpectedErrorKind, message: &str, domain: &str, // boundary — e.g. `openhuman.vault_create` called with a // `root_path` that doesn't exist. The typed error is // already shown to the user; Sentry has no remediation - // path. Demote to `warn!` so the local trace still pins - // which RPC + which method tripped the gate. + // path. Demote to `info!` — same tier as + // `PromptInjectionBlocked`, which is the closest severity + // class ("user input we already surfaced a typed error for"; + // not operator-actionable like `DiskFull` / `NetworkUnreachable`). // // **Do not include the raw `message` here.** The message // body embeds the user's local filesystem layout (username, // project name, document directory, …) and // `sentry_tracing_layer` in `core::logging` maps - // `Level::WARN` to `EventFilter::Breadcrumb` — so any + // `Level::INFO` to `EventFilter::Breadcrumb` — so any // formatted body would be attached as a breadcrumb to // every subsequent Sentry event from this hub, leaking // user paths into unrelated reports. Log only `domain` / @@ -1372,7 +1376,7 @@ fn report_expected_message(kind: ExpectedErrorKind, message: &str, domain: &str, // Full-path diagnostics for local debugging stay available // via `RUST_LOG=…=debug` since `Level::DEBUG` / `TRACE` // are mapped to `EventFilter::Ignore`. - tracing::warn!( + tracing::info!( domain = domain, operation = operation, kind = "filesystem_user_path_invalid", @@ -2197,6 +2201,10 @@ mod tests { "open /etc/passwd failed: Is a directory (os error 21)", // Bare path with no `directory` mention — must NOT classify. "root_path must be absolute: ./relative/path", + // Generic body with the trailing colon but no known vault/http_host + // prefix — must NOT classify (future provider/storage errors that + // happen to embed "path is not a directory: ..." should reach Sentry). + "input config path is not a directory: /etc/foo", ] { assert_eq!( expected_error_kind(raw),