From d420abfa6ac1a80f7ebd426913d503447b66786e Mon Sep 17 00:00:00 2001 From: Nige Date: Thu, 12 Mar 2026 18:28:57 +0000 Subject: [PATCH] fix(memory): reject absolute filesystem paths with corrective routing (#934) * ci(staging): use default branch instead of hardcoded main * fix(memory): route absolute paths to filesystem tools --- .github/workflows/staging-ci.yml | 23 ++++++----- src/tools/builtin/memory.rs | 65 +++++++++++++++++++++++++++++++- 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/.github/workflows/staging-ci.yml b/.github/workflows/staging-ci.yml index c8a39c80..f89fa05a 100644 --- a/.github/workflows/staging-ci.yml +++ b/.github/workflows/staging-ci.yml @@ -44,6 +44,7 @@ jobs: id: check env: FORCE_RUN: ${{ inputs.force }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} run: | CURRENT_HEAD=$(git rev-parse HEAD) echo "current_head=${CURRENT_HEAD}" >> "$GITHUB_OUTPUT" @@ -65,8 +66,8 @@ jobs: echo "Found ${COMMIT_COUNT} new commit(s) since last tested" DIFF_RANGE="${LAST_TESTED}..${CURRENT_HEAD}" else - git fetch origin main - MERGE_BASE=$(git merge-base origin/main HEAD) + git fetch origin "${DEFAULT_BRANCH}" + MERGE_BASE=$(git merge-base "origin/${DEFAULT_BRANCH}" HEAD) echo "First run -- reviewing from merge-base ${MERGE_BASE}" DIFF_RANGE="${MERGE_BASE}..${CURRENT_HEAD}" fi @@ -129,18 +130,19 @@ jobs: echo "token=${{ github.token }}" >> "$GITHUB_OUTPUT" fi - - name: Check if staging is ahead of main + - name: Check if staging is ahead of target branch id: ahead-check env: GH_TOKEN: ${{ steps.token.outputs.token }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} run: | - git fetch origin main - AHEAD=$(git rev-list --count origin/main..origin/staging) + git fetch origin "${DEFAULT_BRANCH}" + AHEAD=$(git rev-list --count "origin/${DEFAULT_BRANCH}..origin/staging") echo "commits_ahead=${AHEAD}" >> "$GITHUB_OUTPUT" if [ "$AHEAD" -eq 0 ]; then - echo "Staging is not ahead of main. Nothing to promote." + echo "Staging is not ahead of ${DEFAULT_BRANCH}. Nothing to promote." else - echo "Staging is ${AHEAD} commits ahead of main." + echo "Staging is ${AHEAD} commits ahead of ${DEFAULT_BRANCH}." fi - name: Create promotion branch @@ -159,6 +161,7 @@ jobs: if: steps.ahead-check.outputs.commits_ahead != '0' env: GH_TOKEN: ${{ steps.token.outputs.token }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} run: | # Find the newest open promotion PR with a staging-promote/* head branch LATEST=$(gh pr list --label staging-promotion --state open \ @@ -168,8 +171,8 @@ jobs: echo "base=${LATEST}" >> "$GITHUB_OUTPUT" echo "Chaining onto existing promotion branch: ${LATEST}" else - echo "base=main" >> "$GITHUB_OUTPUT" - echo "No existing promotion PR — targeting main" + echo "base=${DEFAULT_BRANCH}" >> "$GITHUB_OUTPUT" + echo "No existing promotion PR — targeting ${DEFAULT_BRANCH}" fi - name: Create promotion PR @@ -186,7 +189,7 @@ jobs: PR_URL=$(gh pr create \ --base "$BASE" \ --head "$BRANCH" \ - --title "chore: promote staging to main (${TIMESTAMP})" \ + --title "chore: promote staging to ${BASE} (${TIMESTAMP})" \ --body "## Auto-promotion from staging CI **Batch range:** \`${RANGE}\` diff --git a/src/tools/builtin/memory.rs b/src/tools/builtin/memory.rs index 71fe8a3b..87ae7fa5 100644 --- a/src/tools/builtin/memory.rs +++ b/src/tools/builtin/memory.rs @@ -12,6 +12,7 @@ //! Use `memory_write` to persist important facts that should be remembered //! across sessions. +use std::path::Path; use std::sync::Arc; use async_trait::async_trait; @@ -26,6 +27,28 @@ use crate::workspace::{Workspace, paths}; const PROTECTED_IDENTITY_FILES: &[&str] = &[paths::IDENTITY, paths::SOUL, paths::AGENTS, paths::USER]; +/// Detect paths that are clearly local filesystem references, not workspace-memory docs. +/// +/// Examples: +/// - `/Users/.../file.md` (Unix absolute) +/// - `C:\Users\...` or `D:/work/...` (Windows absolute) +/// - `~/notes.md` (home expansion shorthand) +fn looks_like_filesystem_path(path: &str) -> bool { + if path.is_empty() { + return false; + } + + if Path::new(path).is_absolute() || path.starts_with("~/") { + return true; + } + + let bytes = path.as_bytes(); + bytes.len() >= 3 + && bytes[0].is_ascii_alphabetic() + && bytes[1] == b':' + && (bytes[2] == b'\\' || bytes[2] == b'/') +} + /// Tool for searching workspace memory. /// /// Performs hybrid search (FTS + semantic) across all memory documents. @@ -143,7 +166,8 @@ impl Tool for MemoryWriteTool { be remembered across sessions. Targets: 'memory' for curated long-term facts, \ 'daily_log' for timestamped session notes, 'heartbeat' for the periodic \ checklist (HEARTBEAT.md), 'bootstrap' to clear the first-run ritual file, \ - or provide a custom path for arbitrary file creation." + or provide a custom workspace path for arbitrary file creation. \ + Never pass absolute filesystem paths like '/Users/...' or 'C:\\...'." } fn parameters_schema(&self) -> serde_json::Value { @@ -183,6 +207,14 @@ impl Tool for MemoryWriteTool { .and_then(|v| v.as_str()) .unwrap_or("daily_log"); + if looks_like_filesystem_path(target) { + return Err(ToolError::InvalidParameters(format!( + "'{}' looks like a local filesystem path. memory_write only works with workspace-memory paths. \ + Use write_file for filesystem writes. For opening files in an editor, use shell with: open \"\".", + target + ))); + } + // Bootstrap target: clear BOOTSTRAP.md to mark first-run ritual complete. // Handled early because it accepts empty content (unlike other targets). if target == "bootstrap" { @@ -332,7 +364,8 @@ impl Tool for MemoryReadTool { fn description(&self) -> &str { "Read a file from the workspace memory (database-backed storage). \ Use this to read files shown by memory_tree. NOT for local filesystem files \ - (use read_file for those). Works with identity files, heartbeat checklist, \ + (use read_file for those). Do not pass absolute paths like '/Users/...' or 'C:\\...'. \ + Works with identity files, heartbeat checklist, \ memory, daily logs, or any custom workspace path." } @@ -358,6 +391,14 @@ impl Tool for MemoryReadTool { let path = require_str(¶ms, "path")?; + if looks_like_filesystem_path(path) { + return Err(ToolError::InvalidParameters(format!( + "'{}' looks like a local filesystem path. memory_read only works with workspace-memory paths. \ + Use read_file for filesystem reads. For opening files in an editor, use shell with: open \"\".", + path + ))); + } + let doc = self .workspace .read(path) @@ -379,6 +420,26 @@ impl Tool for MemoryReadTool { } } +#[cfg(test)] +mod path_routing_tests { + use super::looks_like_filesystem_path; + + #[test] + fn detects_filesystem_paths() { + assert!(looks_like_filesystem_path("/Users/nige/file.md")); + assert!(looks_like_filesystem_path("C:\\Users\\nige\\file.md")); + assert!(looks_like_filesystem_path("D:/work/file.md")); + assert!(looks_like_filesystem_path("~/notes.md")); + } + + #[test] + fn allows_workspace_memory_paths() { + assert!(!looks_like_filesystem_path("MEMORY.md")); + assert!(!looks_like_filesystem_path("daily/2026-03-11.md")); + assert!(!looks_like_filesystem_path("projects/alpha/notes.md")); + } +} + /// Tool for viewing workspace structure as a tree. /// /// Returns a hierarchical view of files and directories with configurable depth.