fix(err-06): return IpcError from every command that can emit a conflict code

.planning/milestones/v1.0-phases/03-typed-ipc-error-contract/03-SECURITY.md

@@ -0,0 +1,66 @@
+---
+phase: 03
+slug: typed-ipc-error-contract
+status: verified
+threats_open: 0
+asvs_level: 1
+created: 2026-08-28
+---
+
+# Phase 3 Security
+
+> Retroactive verification of the Phase 3 threat register after the ERR-06
+> post-close hardening.
+
+## Trust Boundaries
+
+| Boundary | Description | Data Crossing |
+|----------|-------------|---------------|
+| Rust command to Tauri bridge | `IpcError` is serialized instead of flattening reserved conflict codes into text | Conflict code and human-readable message |
+| Tauri rejection to frontend normalizer | `todayInvoke`, `saveDocument`, and `updateFrontmatterField` normalize unknown rejection values before branch sites read them | Untrusted rejection object |
+| Source inventory to regression guard | The ERR-06 test reads every Rust source file and checks code-emitting paths for typed-error flattening | Repository source text only |
+
+## Threat Register
+
+| Threat ID | Category | Component | Severity | Disposition | Mitigation | Status |
+|-----------|----------|-----------|----------|-------------|------------|--------|
+| T-03-01 | Tampering / Spoofing | `normalizeIpcError` and its consumers | medium | mitigate | String-shape checks and closed union membership reject forged codes; `ipcError.test.ts`, `today.test.ts`, and `api.test.ts` pin fail-safe normalization | closed |
+| T-03-02 | Information Disclosure | `IpcError` wire shape | low | accept | Conflict codes were already present in legacy error strings and the binary; the structured field adds no new sensitive detail | closed |
+| T-03-03 | Information Disclosure | `From<String>` and `Display` | low | mitigate | Legacy text remains the complete message for empty-code errors; Rust Display tests and the frontmatter API test pin unchanged user-visible text | closed |
+| T-03-04 | Tampering | Conflict-emitting command paths | low | mitigate | ERR-06 removes string flattening from three commands; the recursive source guard detects new Rust modules, while the sole internal `apply_receipt` exception is function-scoped and documented | closed |
+| T-03-05 | Information Disclosure | E2E fixture messages | low | accept | Fixtures carry a separate code and prefix-free message; normalization restores the same display text without adding internal data | closed |
+| T-03-06 | Tampering | Rename and source-discovery drills | low | mitigate | Drill edits are reverted before commit; the source-discovery probe failed on the injected offender and passed after removal | closed |
+| T-03-SC | Tampering | Dependency supply chain | low | accept | No dependency was added; the recursive guard uses the repository's existing `walkdir` dependency | closed |
+
+## Accepted Risks Log
+
+| Risk ID | Threat Ref | Rationale | Accepted By | Date |
+|---------|------------|-----------|-------------|------|
+| AR-03-01 | T-03-02 | The structured code exposes no information beyond the previous string prefix | Phase 3 design decision | 2026-08-28 |
+| AR-03-02 | T-03-05 | Test fixtures preserve the production display surface and contain no secrets | Phase 3 design decision | 2026-08-28 |
+| AR-03-03 | T-03-SC | No new package or version was introduced | Phase 3 design decision | 2026-08-28 |
+
+## Security Audit Trail
+
+| Audit Date | Threats Total | Closed | Open | Run By |
+|------------|---------------|--------|------|--------|
+| 2026-08-28 | 7 | 7 | 0 | Codex inline audit |
+
+## Verification Evidence
+
+- `make verify`: passed, including 1,945 frontend tests, 1,235 Rust tests,
+  fmt, clippy with warnings denied, typecheck, lint, and production build.
+- ERR-06 recursive guard: passed on the final tree.
+- Red probe: a newly added unregistered Rust module was detected as
+  `err06_guard_probe.rs:8`, then removed and the guard returned green.
+- `Result<_, String>` measurement: 1,146 before ERR-06 and 1,142 after the
+  four intended signature changes.
+
+## Sign-Off
+
+- [x] All threats have a disposition.
+- [x] Accepted risks are documented.
+- [x] `threats_open: 0` confirmed.
+- [x] `status: verified` set in frontmatter.
+
+**Approval:** verified 2026-08-28

src-tauri/src/document.rs

@@ -208,21 +208,23 @@ pub fn update_frontmatter_field(
     key: String,
     value: Option<FieldInput>,
     expected_revision: Option<String>,
-) -> Result<DocumentPayload, String> {
+) -> Result<DocumentPayload, IpcError> {
     let path = resolve_inside_vault(&vault_path, &document_path)?;
     let is_html = path
         .extension()
         .and_then(|value| value.to_str())
         .map(|value| matches!(value.to_ascii_lowercase().as_str(), "html" | "htm"))
         .unwrap_or(false);
     if is_html {
-        return Err("frontmatter editing is not supported for HTML documents".to_string());
+        return Err("frontmatter editing is not supported for HTML documents"
+            .to_string()
+            .into());
     }
     assert_document_owner(&vault_path, &path)?;
     assert_maru_can_write(&vault_path, WorkspaceWriteAction::Modify)?;
     let original =
         fs::read_to_string(&path).map_err(|err| format!("Cannot read document: {err}"))?;
-    assert_expected_revision(&original, expected_revision.as_deref()).map_err(|e| e.to_string())?;
+    assert_expected_revision(&original, expected_revision.as_deref())?;
     let mapped = value.map(FrontmatterValue::from);
     let updated = update_frontmatter_content(&original, &key, mapped)?;
     if updated != original {
@@ -242,7 +244,7 @@ pub fn update_frontmatter_field(
         }
         write_atomic(&path, updated.as_bytes())?;
     }
-    read_document(vault_path, path.to_string_lossy().to_string())
+    read_document(vault_path, path.to_string_lossy().to_string()).map_err(Into::into)
 }
 
 /// Optional Hub-driven prefill values. When the user picks a template +
@@ -1211,7 +1213,7 @@ mod tests {
         .unwrap_err();
 
         assert_eq!(
-            error,
+            error.to_string(),
             "frontmatter editing is not supported for HTML documents"
         );
     }

src-tauri/src/ipc_error.rs

@@ -55,6 +55,138 @@ impl From<String> for IpcError {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::fs;
+    use std::path::{Path, PathBuf};
+    use walkdir::WalkDir;
+
+    /// Name of the function a line sits in, found by walking back to the
+    /// nearest `fn` declaration. Used both to match the allowlist precisely and
+    /// to name the offender in the failure message.
+    fn enclosing_fn_name<'a>(lines: &[&'a str], index: usize) -> Option<&'a str> {
+        lines[..=index].iter().rev().find_map(|line| {
+            let trimmed = line.trim_start();
+            let rest = trimmed
+                .strip_prefix("pub ")
+                .unwrap_or(trimmed)
+                .strip_prefix("pub(crate) ")
+                .unwrap_or_else(|| trimmed.strip_prefix("pub ").unwrap_or(trimmed));
+            let rest = rest.strip_prefix("async ").unwrap_or(rest);
+            let rest = rest.strip_prefix("fn ")?;
+            Some(rest.split(['(', '<']).next().unwrap_or(rest).trim())
+        })
+    }
+
+    /// True only for `map_err(|x| x.to_string())`, where the closure's own
+    /// argument is what gets stringified. That is the shape that takes a typed
+    /// error and throws the code away. It deliberately does not match
+    /// `map_err(|_| "literal".to_string())`, which mints a fresh error rather
+    /// than flattening one, nor an unrelated `.to_string()` elsewhere on the line.
+    fn flattens_the_closure_argument(line: &str) -> bool {
+        let Some((_, rest)) = line.split_once("map_err(|") else {
+            return false;
+        };
+        let Some((param, body)) = rest.split_once('|') else {
+            return false;
+        };
+        let param = param.trim();
+        if param.is_empty() || param == "_" {
+            return false;
+        }
+        body.trim_start()
+            .starts_with(&format!("{param}.to_string())"))
+    }
+
+    /// Read every Rust source under this crate's `src/` tree. Keeping source
+    /// discovery automatic means a new module that calls an existing emitter
+    /// is covered without a second, easy-to-forget inventory edit.
+    fn rust_sources() -> Vec<(String, String)> {
+        let root = Path::new(env!("CARGO_MANIFEST_DIR")).join("src");
+        let mut paths: Vec<PathBuf> = WalkDir::new(&root)
+            .into_iter()
+            .map(|entry| entry.expect("Rust source tree must be readable"))
+            .filter(|entry| {
+                entry.file_type().is_file()
+                    && entry.path().extension().and_then(|value| value.to_str()) == Some("rs")
+            })
+            .map(|entry| entry.into_path())
+            .collect();
+        paths.sort();
+
+        paths
+            .into_iter()
+            .map(|path| {
+                let name = path
+                    .strip_prefix(&root)
+                    .expect("walked source must stay below the crate src directory")
+                    .to_string_lossy()
+                    .replace('\\', "/");
+                let source = fs::read_to_string(&path)
+                    .unwrap_or_else(|err| panic!("Cannot read {}: {err}", path.display()));
+                (name, source)
+            })
+            .collect()
+    }
+
+    /// ERR-06: a path that reaches a contract-code emitter must not flatten its
+    /// error back to `String`. `.map_err(|e| e.to_string())` compiles fine and
+    /// silently strips the code, so the frontend's `err instanceof IpcError`
+    /// recovery branch stops matching while every build stays green. The
+    /// compiler cannot catch that; only an inventory can.
+    ///
+    /// Extend EMITTERS when a new code-producing helper lands. Add to ALLOWED
+    /// only with a reason, and only when the error genuinely never crosses IPC.
+    #[test]
+    fn no_code_emitting_path_flattens_its_error_to_string() {
+        const EMITTERS: &[&str] = &[
+            "today_mutate(",
+            "task_transition(",
+            "task_trash(",
+            "save_document(",
+            "evidence_binder_mutate(",
+            "assert_expected_revision(",
+            "check_revision(",
+            "load_context(",
+        ];
+        // (file, line) pairs that flatten deliberately, each with its reason.
+        const ALLOWED: &[(&str, &str)] = &[(
+            "web_actions.rs",
+            // apply_receipt is an internal helper, not a command: web_actions_apply
+            // consumes its Err as the reason string on a retry marker, so the value
+            // never crosses the IPC boundary and no frontend branch can read a code.
+            "apply_receipt",
+        )];
+        let mut offenders = Vec::new();
+        for (name, source) in rust_sources() {
+            let lines: Vec<&str> = source.lines().collect();
+            for (index, line) in lines.iter().enumerate() {
+                if !flattens_the_closure_argument(line) {
+                    continue;
+                }
+                // Look back a short window for the emitter call this flattens.
+                let start = index.saturating_sub(15);
+                let window = lines[start..=index].join("\n");
+                let Some(emitter) = EMITTERS.iter().find(|e| window.contains(**e)) else {
+                    continue;
+                };
+                let enclosing = enclosing_fn_name(&lines, index).unwrap_or("<unknown>");
+                let allowed = ALLOWED
+                    .iter()
+                    .any(|(file, func)| *file == name && *func == enclosing);
+                if !allowed {
+                    offenders.push(format!(
+                        "{name}:{} in {enclosing}() flattens {emitter}",
+                        index + 1
+                    ));
+                }
+            }
+        }
+
+        assert!(
+            offenders.is_empty(),
+            "these paths reach a contract-code emitter and flatten its error to String, \
+             stripping the code the frontend branches on: {offenders:#?}"
+        );
+    }
 
     #[test]
     fn ipc_error_codes_are_stable() {

src-tauri/src/today_ai.rs

@@ -7,6 +7,7 @@
 // own revision/day/plan checks remain the final authority.
 
 use crate::agent_host::contracts::{TODAY_CAPTURE_SCHEMA_VERSION, TODAY_PLAN_SCHEMA_VERSION};
+use crate::ipc_error::IpcError;
 use crate::today::{
     block_crosses_sleep, parse_sleep_start, parse_timezone, CalendarSyncStatus, CapacitySummary,
     CaptureCandidate, DailyPlanV1, PlanItemRef, TodayMutation, TodaySnapshot, TOP_LANE_MAX,
@@ -274,7 +275,7 @@ pub fn today_apply_plan_result(
     output_json: String,
     valid_refs: Vec<PlanItemRef>,
     sleep_start: String,
-) -> Result<TodaySnapshot, String> {
+) -> Result<TodaySnapshot, IpcError> {
     let raw: JsonValue = serde_json::from_str(&output_json)
         .map_err(|err| format!("today_ai_invalid_payload: {err}"))?;
     let valid: HashSet<PlanItemRef> = valid_refs.into_iter().collect();
@@ -296,7 +297,6 @@ pub fn today_apply_plan_result(
         expected_revision,
         TodayMutation::SetPlan { plan },
     )
-    .map_err(|e| e.to_string())
 }
 
 #[cfg(test)]
@@ -542,7 +542,7 @@ mod tests {
             SLEEP.to_string(),
         )
         .unwrap_err();
-        assert!(err.starts_with("today_conflict"));
+        assert_eq!(err.code, crate::ipc_error::TODAY_CONFLICT);
         // A draft computed against the newer revision but handed in with the
         // old expected_revision is caught by validation itself.
         let fresh = plan_output_json(&applied.revision, vec![top_item("a")], vec![]);
@@ -555,7 +555,7 @@ mod tests {
             SLEEP.to_string(),
         )
         .unwrap_err();
-        assert!(err.starts_with("today_ai_stale_revision"));
+        assert!(err.to_string().starts_with("today_ai_stale_revision"));
     }
 
     fn plan_validation_err(output: String, snapshot: &TodaySnapshot, refs: &[&str]) -> String {
@@ -801,7 +801,7 @@ mod tests {
             SLEEP.to_string(),
         )
         .unwrap_err();
-        assert!(err.starts_with("today_ai_invalid_payload"));
+        assert!(err.to_string().starts_with("today_ai_invalid_payload"));
         // deny_unknown_fields on the output wrapper: unexpected top-level
         // key is an invalid payload, not silently ignored.
         let mut raw: JsonValue = serde_json::from_str(&plan_output_json(

src-tauri/src/today_calendar.rs

@@ -293,7 +293,7 @@ pub fn task_calendar_set_sync(
     item_ref: PlanItemRef,
     selected: bool,
     destination: Option<String>,
-) -> Result<TodaySnapshot, String> {
+) -> Result<TodaySnapshot, IpcError> {
     crate::today_store::today_mutate(
         work_path,
         logical_day,
@@ -304,7 +304,6 @@ pub fn task_calendar_set_sync(
             destination,
         },
     )
-    .map_err(|e| e.to_string())
 }
 
 // --- Publish ------------------------------------------------------------------
@@ -648,7 +647,7 @@ mod tests {
         snapshot: &TodaySnapshot,
         task_id: &str,
         selected: bool,
-    ) -> Result<TodaySnapshot, String> {
+    ) -> Result<TodaySnapshot, IpcError> {
         task_calendar_set_sync(
             work(tmp),
             snapshot.logical_day.clone(),
@@ -787,9 +786,9 @@ mod tests {
 
         // Stale revision conflicts; unknown item errors.
         let err = select(&tmp, &snapshot, "a", true).unwrap_err();
-        assert!(err.starts_with("today_conflict"));
+        assert_eq!(err.code, crate::ipc_error::TODAY_CONFLICT);
         let err = select(&tmp, &cleared, "nope", true).unwrap_err();
-        assert!(err.starts_with("today_plan_item_missing"));
+        assert!(err.to_string().starts_with("today_plan_item_missing"));
     }
 
     // --- Publish -----------------------------------------------------------------