perf(security): reuse the canonical-workspace cache in the sync path checks

[mysma-9403]

Summary

• The synchronous path validators is_path_string_allowed and is_resolved_path_allowed_for re-ran workspace_dir.canonicalize() — a stat(2) + symlink walk on the same immutable input — on every file/shell tool call.
• The async validate_path already memoizes that resolution in the canonical_workspace OnceCell; the sync validators bypassed it and paid the syscall each time.
• Route both sync checks through a new workspace_root_sync() that hydrates the same cell via tokio::sync::OnceCell's synchronous get/set. No new field, no second cache.

Problem

A single agent turn performs many file reads/edits/greps and shell path validations. validate_path calls both is_path_string_allowed (string + symlink-containment) and is_resolved_path_allowed_for (resolved-path containment), and each independently did:

let workspace_root = self.workspace_dir.canonicalize().unwrap_or_else(|_| self.workspace_dir.clone());

workspace_dir is immutable for a given policy, so this is the same stat(2) + symlink walk repeated per call. The struct's own canonical_workspace doc comment already makes this argument for the async path — the sync validators simply never adopted the cache (they can't .await it).

Solution

Add a sync workspace_root_sync() that reads/populates the existing canonical_workspace cell through OnceCell's sync get/set, and call it from the two sync validators.

• One cache, both paths agree. std::fs::canonicalize (sync) and tokio::fs::canonicalize (async) resolve the same input to the same canonical form, so whichever hydrates the cell first, the sync and async paths converge on one value. On the rare race where the async initializer wins, set fails and the helper returns its locally-resolved value — equal to what was stored.
• Correctness is preserved. Both call sites previously used the identical canonicalize().unwrap_or_else(clone) expression, so the cached value is byte-identical to what they computed inline. workspace_dir never mutates on a live policy: live_policy rebuilds a fresh policy on reload (*guard = Arc::new(rebuilt)), and security_for_tool_context only overrides action_dir/trusted_roots.
• is_workspace_internal_path is deliberately left unchanged — its workspace_dir.canonicalize() is coupled all-or-nothing with a per-call path.canonicalize() at a fail-closed security boundary, so caching only the workspace half would change its comparison semantics.

Submission Checklist

• Tests added or updated (happy path + at least one failure / edge case) — new workspace_root_sync_hydrates_and_shares_the_async_cache pins hydrate-once, reuse, and sync/async agreement; the existing validate_path_* and containment suites cover the behavior-preservation edges.
• Diff coverage ≥ 80% — the new helper and its two call sites are exercised by the new test plus the existing is_resolved_path_allowed/is_path_string_allowed suites.
• Coverage matrix updated — N/A: behaviour-preserving performance change, no feature row added/removed/renamed.
• All affected feature IDs from the matrix are listed under ## Related — N/A: no feature-level change.
• No new external network dependencies introduced.
• Manual smoke checklist updated if this touches release-cut surfaces — N/A: internal caching only, no user-visible surface.
• Linked issue closed via Closes #NNN — N/A: no tracking issue (self-identified hot-path micro-optimization).

Impact

• Runtime: desktop/CLI/core — every file/shell tool call. Removes one stat(2) + symlink walk from each of the two sync path checks after the first (they now share the async path's single cached canonicalization).
• Security: none — behavior-preserving; the fail-closed is_workspace_internal_path check is untouched.
• Migration/compat: none.

Related

• Closes: N/A
• Follow-up PR(s)/TODOs: is_within_trusted_root re-canonicalizes the trusted roots + turn workspace per call — a sibling caching opportunity, intentionally out of scope here to keep this single-concern.

---

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

• Key: N/A
• URL: N/A

Commit & Branch

• Branch: perf/security-cache-sync-workspace-canonicalize

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5a516249-44d6-4c68-94b4-7f622b394518

📥 Commits

Reviewing files that changed from the base of the PR and between 2826259 and 863ead1.

📒 Files selected for processing (2)

• src/openhuman/security/policy/path_checks.rs
• src/openhuman/security/policy/policy_tests.rs

---

📝 Walkthrough

Walkthrough

The change adds synchronous workspace-root resolution with shared canonical caching. Synchronous symlink and resolved-path checks now use this resolver. A regression test verifies cache sharing with asynchronous validation.

Changes

Workspace cache resolution

Layer / File(s)	Summary
Synchronous workspace-root resolution src/openhuman/security/policy/path_checks.rs	Adds workspace_root_sync with cache reuse, synchronous canonicalization on cache miss, and raw-path fallback. Symlink and resolved-path checks use the shared resolver.
Cache sharing regression coverage src/openhuman/security/policy/policy_tests.rs	Verifies cache hydration, repeated synchronous results, asynchronous reuse, and workspace path allowance.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: al629176, codeghost21

Poem

A rabbit hops through cached roots,
No path is checked twice.
Sync and async share one burrow,
Canonical and nice.
Inside the workspace, all is right.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: reusing the canonical workspace cache in synchronous security path checks.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

Improves performance in the Rust core’s SecurityPolicy path-validation hot path by making the synchronous validators reuse the already-established canonical_workspace memoization, avoiding repeated workspace_dir.canonicalize() syscalls on every file/shell tool call.

Changes:

• Added workspace_root_sync() to synchronously hydrate/read the existing canonical_workspace OnceCell.
• Routed sync validators to use workspace_root_sync() instead of re-canonicalizing workspace_dir each time.
• Added a regression test ensuring sync/async paths share the same cache and return identical results.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
src/openhuman/security/policy/path_checks.rs	Introduces workspace_root_sync() and updates sync validation call sites to reuse the shared canonical-workspace cache.
src/openhuman/security/policy/policy_tests.rs	Adds a test that pins cache sharing and sync/async agreement for workspace canonicalization.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[tinysweeper]

tinysweeper found nothing blocking. Approving.

             $0.0261 · 17,726 in / 7,617 out · 8,839 cached (50%) · z-ai/glm-5.2
critique:    $0.0144 · 5,071 in  / 4,633 out · 832 cached (16%)   · z-ai/glm-5.2
security:    $0.0051 · 5,029 in  / 1,043 out · 1,870 cached (37%) · z-ai/glm-5.2
tests:       $0.0036 · 3,300 in  / 1,186 out · 2,795 cached (85%) · z-ai/glm-5.2
description: $0.0030 · 4,326 in  / 755 out   · 3,342 cached (77%) · z-ai/glm-5.2

[mysma-9403]

CI note — the two red Rust lanes are pre-existing on main, not from this PR

• Rust Feature-Gate Smoke (gates off) — its scoped cargo test --no-default-features --lib step fails on two tests in config/migration_helpers (migrate_hermes_apply_imports_markdown_entries, migrate_openclaw_apply_imports_markdown_entries_into_target_workspace), which panic with no EmbeddingHost installed — the host must call memory::embedding_host::set_embedding_host during startup wiring. Those tests are untouched by this PR. I reproduced them on a clean checkout of main with cargo test --no-default-features --lib config::migration_helpers::ops (2/8 fail) — it's fallout from the tinymemory v0.3.0 extraction, surfacing only in the gates-off test run (a plain cargo check --no-default-features on main compiles clean). Older-base PRs branched before it merged still pass this lane.
• Rust Core Coverage (cargo-llvm-cov) — the chronic pre-existing red on Rust PRs.

Rust Quality (fmt, clippy) passes, which is the signal that this PR itself compiles and lints clean under the product feature set. The change here is confined to its own module and unrelated to either red lane.