mirror of
https://github.com/akitaonrails/ai-memory.git
synced 2026-10-02 03:24:46 +08:00
Merge remote-tracking branch 'origin/main' into issue/776-chunked-v62-migration
# Conflicts: # CHANGELOG.md
This commit is contained in:
@@ -20,6 +20,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
runner now intentionally tolerates a divergent checksum on an already-applied
|
||||
migration (`abort_divergent = false`) so correctly-migrated stores still open;
|
||||
the schema-ahead guard (`abort_missing`) is unchanged (#776).
|
||||
- Page writes now refuse git-reserved and non-portable page paths (a `.git`
|
||||
component or an 8.3 `git~1`..`git~4` alias, Windows-reserved names and
|
||||
characters) on every write funnel, including MCP `memory_write_page` and
|
||||
consolidation `apply_batch`; reads of already-stored pages stay tolerant so
|
||||
a bad row never breaks a listing. The git-reserved check is byte-safe and no
|
||||
longer panics on a 5-byte multibyte path component (#781).
|
||||
- The generated OpenCode and OpenCode 2 plugins now forward a subagent session's
|
||||
`parentID` as the `agent_id` marker, so `[capture] drop_subagent_captures` can
|
||||
recognize and drop OpenCode subagent sessions. Previously both plugins emitted
|
||||
|
||||
@@ -108,33 +108,6 @@ impl PagePath {
|
||||
///
|
||||
/// # Errors
|
||||
/// Returns [`MemoryError::InvalidPagePath`] when the input is empty or
|
||||
/// Reject a path that cannot be materialised and checkpointed on every
|
||||
/// supported platform.
|
||||
///
|
||||
/// Deliberately **not** part of [`PagePath::new`]. Persisted rows are
|
||||
/// reconstructed through that constructor on every read
|
||||
/// (`reader.rs` does so in the recency, search, vector and graph
|
||||
/// queries), so tightening it would make any already-stored
|
||||
/// non-portable page unreadable — and because those are list queries,
|
||||
/// one such page would break a whole listing rather than just itself.
|
||||
/// The rule therefore applies where a *new* path enters the system.
|
||||
///
|
||||
/// The rule is the same on every platform on purpose. A wiki authored
|
||||
/// on Linux is expected to be usable on Windows by the same release;
|
||||
/// making the check platform-conditional would let a Linux session
|
||||
/// create pages a Windows session cannot read, which is the defect
|
||||
/// being fixed rather than a fix for it (#462).
|
||||
///
|
||||
/// # Errors
|
||||
/// Returns [`MemoryError::InvalidPagePath`] naming the offending
|
||||
/// component and the reason.
|
||||
pub fn ensure_portable(&self) -> Result<(), MemoryError> {
|
||||
for component in self.as_str().split('/') {
|
||||
ensure_portable_component(component, self.as_str())?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// contains a path component that would escape or alias the wiki root.
|
||||
pub fn new(raw: impl Into<String>) -> Result<Self, MemoryError> {
|
||||
let raw = raw.into();
|
||||
@@ -184,6 +157,33 @@ impl PagePath {
|
||||
Ok(Self(raw))
|
||||
}
|
||||
|
||||
/// Reject a path that cannot be materialised and checkpointed on every
|
||||
/// supported platform.
|
||||
///
|
||||
/// Deliberately **not** part of [`PagePath::new`]. Persisted rows are
|
||||
/// reconstructed through that constructor on every read
|
||||
/// (`reader.rs` does so in the recency, search, vector and graph
|
||||
/// queries), so tightening it would make any already-stored
|
||||
/// non-portable page unreadable — and because those are list queries,
|
||||
/// one such page would break a whole listing rather than just itself.
|
||||
/// The rule therefore applies where a *new* path enters the system.
|
||||
///
|
||||
/// The rule is the same on every platform on purpose. A wiki authored
|
||||
/// on Linux is expected to be usable on Windows by the same release;
|
||||
/// making the check platform-conditional would let a Linux session
|
||||
/// create pages a Windows session cannot read, which is the defect
|
||||
/// being fixed rather than a fix for it (#462).
|
||||
///
|
||||
/// # Errors
|
||||
/// Returns [`MemoryError::InvalidPagePath`] naming the offending
|
||||
/// component and the reason.
|
||||
pub fn ensure_portable(&self) -> Result<(), MemoryError> {
|
||||
for component in self.as_str().split('/') {
|
||||
ensure_portable_component(component, self.as_str())?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Borrow the inner string.
|
||||
#[must_use]
|
||||
pub fn as_str(&self) -> &str {
|
||||
@@ -704,6 +704,18 @@ const DOS_DEVICE_NAMES: &[&str] = &[
|
||||
/// [`PagePath::new`].
|
||||
const WINDOWS_RESERVED_CHARS: &[char] = &['<', '>', ':', '"', '|', '?', '*'];
|
||||
|
||||
/// Returns true if a path component is reserved by Git (`.git` case-insensitively,
|
||||
/// or an 8.3 short-name alias like `git~1`..`git~4`).
|
||||
#[must_use]
|
||||
pub fn is_git_reserved_component(component: &str) -> bool {
|
||||
// Compare on bytes: a `str` slice at a fixed byte index panics on a
|
||||
// multibyte component (a 5-byte UTF-8 name like "abcé" has no char
|
||||
// boundary at 4), and this runs on untrusted write input.
|
||||
let b = component.as_bytes();
|
||||
component.eq_ignore_ascii_case(".git")
|
||||
|| (b.len() == 5 && b[..4].eq_ignore_ascii_case(b"git~") && (b'1'..=b'4').contains(&b[4]))
|
||||
}
|
||||
|
||||
fn ensure_portable_component(component: &str, full: &str) -> Result<(), MemoryError> {
|
||||
let invalid = |reason: &str| {
|
||||
Err(MemoryError::InvalidPagePath(format!(
|
||||
@@ -738,6 +750,12 @@ fn ensure_portable_component(component: &str, full: &str) -> Result<(), MemoryEr
|
||||
"uses the reserved DOS device name {stem:?}; Windows resolves it to a device, not a file"
|
||||
));
|
||||
}
|
||||
// Git reserves `.git` for repository metadata. Any tree entry named `.git`
|
||||
// or an 8.3 alias is refused by libgit2 with `GIT_EINVALIDPATH` and cannot
|
||||
// be checkpointed.
|
||||
if is_git_reserved_component(component) {
|
||||
return invalid("is reserved by Git for repository metadata and refused in tree entries");
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -778,6 +796,30 @@ mod portable_page_path_tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn git_reserved_paths_are_rejected() {
|
||||
for raw in [
|
||||
".git",
|
||||
".git/config",
|
||||
".git/HEAD",
|
||||
"notes/.git",
|
||||
"notes/.git/sub.md",
|
||||
"notes/.GIT/sub.md",
|
||||
"notes/.Git/sub.md",
|
||||
"notes/git~1",
|
||||
"notes/git~1/foo.md",
|
||||
"notes/GIT~2/bar.md",
|
||||
"notes/git~4/config",
|
||||
"a/b/c/.git/deep.md",
|
||||
] {
|
||||
let path = PagePath::new(raw).expect("still constructible: reads must keep working");
|
||||
assert!(
|
||||
path.ensure_portable().is_err(),
|
||||
"{raw:?} contains a git-reserved component and must be refused at write time"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The rule must not reject ordinary pages. `con` is only reserved as a
|
||||
/// whole component, so `concepts/` and `icon.md` are fine.
|
||||
#[test]
|
||||
@@ -792,6 +834,11 @@ mod portable_page_path_tests {
|
||||
"a/b/c/deep.md",
|
||||
"notes/dot.in.middle.md",
|
||||
"notes/UPPER.MD",
|
||||
"notes/.git.md",
|
||||
"notes/.gitignore",
|
||||
"notes/.gitattributes",
|
||||
"notes/github.md",
|
||||
"git-notes/index.md",
|
||||
] {
|
||||
let path = PagePath::new(raw).expect("valid path");
|
||||
assert!(
|
||||
@@ -801,12 +848,43 @@ mod portable_page_path_tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// A multibyte component whose byte length is 5 must not be mistaken for
|
||||
/// a `git~N` alias, and must not panic: `is_git_reserved_component` once
|
||||
/// sliced the string at byte index 4, which is not a char boundary in a
|
||||
/// name like "abcé" (5 bytes) or "a😀" (5 bytes). Since this runs on the
|
||||
/// write funnel for untrusted input, the panic was a crashable defect.
|
||||
#[test]
|
||||
fn multibyte_components_are_not_git_reserved_and_do_not_panic() {
|
||||
for component in ["abcé", "ab€", "a😀", "éé", "🦀🦀"] {
|
||||
assert!(
|
||||
!super::is_git_reserved_component(component),
|
||||
"{component:?} is an ordinary name, not a git-reserved alias"
|
||||
);
|
||||
}
|
||||
// The full write funnel (construct + ensure_portable) must accept a
|
||||
// page path with a 5-byte multibyte component without panicking.
|
||||
for raw in ["notes/abcé.md", "notes/a😀.md", "notes/ab€.md"] {
|
||||
let path = PagePath::new(raw).expect("valid non-ASCII path");
|
||||
assert!(
|
||||
path.ensure_portable().is_ok(),
|
||||
"{raw:?} is portable and must stay writable"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Reads must keep working for pages already stored under a
|
||||
/// non-portable name: `PagePath::new` stays tolerant so a listing does
|
||||
/// not break on one bad row.
|
||||
#[test]
|
||||
fn existing_non_portable_pages_remain_constructible() {
|
||||
for raw in ["CON.md", "notes/a|b.md", "notes/trailing./x.md"] {
|
||||
for raw in [
|
||||
"CON.md",
|
||||
"notes/a|b.md",
|
||||
"notes/trailing./x.md",
|
||||
".git/config",
|
||||
"notes/.git/sub.md",
|
||||
"notes/git~1/foo.md",
|
||||
] {
|
||||
assert!(
|
||||
PagePath::new(raw).is_ok(),
|
||||
"{raw:?} must still construct so persisted rows stay readable"
|
||||
|
||||
@@ -58,7 +58,7 @@ pub use handoff::{
|
||||
pub use ids::{
|
||||
AgentKind, ApiCredentialId, AutoImproveProposalId, AutoImproveRunId, EntityId, HandoffId,
|
||||
ManagedRunId, MessageId, ObservationId, PageFeedbackId, PageId, PagePath, ProjectId, SessionId,
|
||||
UserId, WorkspaceId, WorkstreamId,
|
||||
UserId, WorkspaceId, WorkstreamId, is_git_reserved_component,
|
||||
};
|
||||
pub use message::{
|
||||
AgentMessage, MessageBox, MessageClaim, MessageOrigin, MessageState, NewAgentMessage,
|
||||
|
||||
@@ -6765,6 +6765,12 @@ async fn handle_write_page(
|
||||
Json(serde_json::json!({ "error": format!("invalid path: {e}") })),
|
||||
)
|
||||
})?;
|
||||
path.ensure_portable().map_err(|e| {
|
||||
(
|
||||
StatusCode::UNPROCESSABLE_ENTITY,
|
||||
Json(serde_json::json!({ "error": format!("invalid path: {e}") })),
|
||||
)
|
||||
})?;
|
||||
|
||||
let (ws, proj) = create_ws_proj(&state, &req.workspace, &req.project).await?;
|
||||
|
||||
@@ -8906,6 +8912,46 @@ mod tests {
|
||||
assert_eq!(resp.status(), StatusCode::OK, "write-page setup failed");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn admin_write_page_refuses_git_reserved_and_non_portable_paths() {
|
||||
let (_tmp, router) = read_page_test_router();
|
||||
for bad in [
|
||||
".git",
|
||||
".git/config",
|
||||
"notes/.git",
|
||||
"notes/.git/sub.md",
|
||||
"notes/git~1",
|
||||
"notes/git~1/foo.md",
|
||||
"CON.md",
|
||||
"notes/aux.md",
|
||||
"notes/a|b.md",
|
||||
] {
|
||||
let req_body = serde_json::json!({
|
||||
"workspace": "default",
|
||||
"project": "audit",
|
||||
"path": bad,
|
||||
"body": "bad path body",
|
||||
});
|
||||
let resp = router
|
||||
.clone()
|
||||
.oneshot(
|
||||
Request::builder()
|
||||
.method("POST")
|
||||
.uri("/admin/write-page")
|
||||
.header("content-type", "application/json")
|
||||
.body(Body::from(serde_json::to_vec(&req_body).unwrap()))
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
resp.status(),
|
||||
StatusCode::UNPROCESSABLE_ENTITY,
|
||||
"expected 422 for {bad:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_page_path_mode_returns_full_body() {
|
||||
let (_tmp, router) = read_page_test_router();
|
||||
|
||||
@@ -3081,6 +3081,8 @@ impl AiMemoryServer {
|
||||
.map_err(|_| McpError::internal_error(format!("unknown tier '{tier_name}'"), None))?;
|
||||
let path = PagePath::new(args.path.clone())
|
||||
.map_err(|e| McpError::internal_error(format!("invalid path: {e}"), None))?;
|
||||
path.ensure_portable()
|
||||
.map_err(|e| McpError::internal_error(format!("invalid path: {e}"), None))?;
|
||||
let path = self.place_slot_write(path, &parts).await?;
|
||||
let (ws, proj) = match args.scope.as_deref().map(str::trim) {
|
||||
None | Some("") => {
|
||||
@@ -9905,10 +9907,66 @@ mod tests {
|
||||
.unwrap();
|
||||
assert!(
|
||||
recent_text.contains("notes/santander-2025.md"),
|
||||
"write-page result must be visible to read tools; got {recent_text}"
|
||||
"got {recent_text}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn memory_write_page_refuses_git_reserved_and_non_portable_paths() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let store = Store::open(tmp.path()).unwrap();
|
||||
let ws = store
|
||||
.writer
|
||||
.get_or_create_workspace("default")
|
||||
.await
|
||||
.unwrap();
|
||||
let proj = store
|
||||
.writer
|
||||
.get_or_create_project(ws, "scratch", None)
|
||||
.await
|
||||
.unwrap();
|
||||
let wiki = Wiki::new(tmp.path(), store.writer.clone()).unwrap();
|
||||
let server = AiMemoryServer::new(store.reader.clone(), store.writer.clone(), ws, proj)
|
||||
.with_wiki(wiki);
|
||||
|
||||
for bad in [
|
||||
".git",
|
||||
".git/config",
|
||||
"notes/.git",
|
||||
"notes/.git/sub.md",
|
||||
"notes/.GIT/sub.md",
|
||||
"notes/git~1",
|
||||
"notes/git~1/foo.md",
|
||||
"notes/GIT~2/bar.md",
|
||||
"CON.md",
|
||||
"notes/aux.md",
|
||||
"notes/a|b.md",
|
||||
] {
|
||||
let err = server
|
||||
.memory_write_page(
|
||||
Parameters(WritePageArgs {
|
||||
path: bad.into(),
|
||||
body: "# Bad\n\nShould be refused.".into(),
|
||||
title: None,
|
||||
tier: None,
|
||||
tags: vec![],
|
||||
pinned: false,
|
||||
project: None,
|
||||
workspace: None,
|
||||
scope: None,
|
||||
expires_at: None,
|
||||
}),
|
||||
OptionalParts(test_parts_default()),
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
err.to_string().contains("invalid path"),
|
||||
"expected invalid path error for {bad:?}, got: {err}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn memory_write_page_rejects_workspace_without_project() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
|
||||
@@ -150,7 +150,12 @@ impl GitAdapter {
|
||||
Some(path)
|
||||
};
|
||||
// The watcher reports git's own writes too; never stage them.
|
||||
if rel.is_some_and(|rel| rel.starts_with(".git")) {
|
||||
// Also ignore any path that contains a git-reserved component.
|
||||
if rel.is_some_and(|rel| {
|
||||
rel.components().any(|c| {
|
||||
ai_memory_core::is_git_reserved_component(&c.as_os_str().to_string_lossy())
|
||||
})
|
||||
}) {
|
||||
return;
|
||||
}
|
||||
let mut written = self.written();
|
||||
@@ -621,6 +626,13 @@ fn stage_paths(
|
||||
paths: &BTreeSet<PathBuf>,
|
||||
) -> Result<usize, git2::Error> {
|
||||
for rel in paths {
|
||||
if rel
|
||||
.components()
|
||||
.any(|c| ai_memory_core::is_git_reserved_component(&c.as_os_str().to_string_lossy()))
|
||||
{
|
||||
warn!(path = %rel.display(), "skipping invalid git path with git-reserved component");
|
||||
continue;
|
||||
}
|
||||
let abs = root.join(rel);
|
||||
if abs.is_dir() {
|
||||
let spec = slash_path(rel);
|
||||
@@ -1048,6 +1060,9 @@ mod tests {
|
||||
adapter.mark_written(Path::new(".git/logs/HEAD"));
|
||||
adapter.mark_written(&root.join(".git/index"));
|
||||
adapter.mark_written(Path::new(".git"));
|
||||
adapter.mark_written(Path::new("ws/proj/.git/config"));
|
||||
adapter.mark_written(&root.join("ws/proj/.git/hooks/pre-commit"));
|
||||
adapter.mark_written(Path::new("ws/proj/git~1/config"));
|
||||
assert!(adapter.written_paths().is_empty());
|
||||
}
|
||||
|
||||
|
||||
@@ -196,10 +196,12 @@ async fn run_loop(
|
||||
}
|
||||
}
|
||||
|
||||
/// Inside the wiki's own git directory: neither indexed nor reported.
|
||||
/// Inside the wiki's own git directory or any nested git metadata: neither indexed nor reported.
|
||||
fn is_git_internal(root: &Path, path: &Path) -> bool {
|
||||
path.strip_prefix(root)
|
||||
.is_ok_and(|rel| rel.starts_with(".git"))
|
||||
path.strip_prefix(root).is_ok_and(|rel| {
|
||||
rel.components()
|
||||
.any(|c| ai_memory_core::is_git_reserved_component(&c.as_os_str().to_string_lossy()))
|
||||
})
|
||||
}
|
||||
|
||||
async fn handle_event(wiki: &Wiki, event: notify_debouncer_full::DebouncedEvent) {
|
||||
@@ -924,13 +926,24 @@ mod tests {
|
||||
let git_log = wiki.root().join(".git/logs/HEAD");
|
||||
std::fs::create_dir_all(git_log.parent().unwrap()).unwrap();
|
||||
std::fs::write(&git_log, "ref\n").unwrap();
|
||||
let nested_git = proj_dir.join(".git/config");
|
||||
std::fs::create_dir_all(nested_git.parent().unwrap()).unwrap();
|
||||
std::fs::write(&nested_git, "config\n").unwrap();
|
||||
let gone = proj_dir.join("gone.md");
|
||||
|
||||
for (kind, path) in [
|
||||
(EventKind::Create(notify::event::CreateKind::File), &ledger),
|
||||
(EventKind::Modify(notify::event::ModifyKind::Any), &git_log),
|
||||
(
|
||||
EventKind::Create(notify::event::CreateKind::File),
|
||||
&nested_git,
|
||||
),
|
||||
(EventKind::Remove(notify::event::RemoveKind::File), &gone),
|
||||
(EventKind::Remove(notify::event::RemoveKind::File), &git_log),
|
||||
(
|
||||
EventKind::Remove(notify::event::RemoveKind::File),
|
||||
&nested_git,
|
||||
),
|
||||
] {
|
||||
let event = notify_debouncer_full::DebouncedEvent::new(
|
||||
notify::Event::new(kind).add_path(path.clone()),
|
||||
@@ -944,7 +957,9 @@ mod tests {
|
||||
assert!(reported.contains(&rel(&ledger)), "{reported:?}");
|
||||
assert!(reported.contains(&rel(&gone)), "{reported:?}");
|
||||
assert!(
|
||||
!reported.iter().any(|p| p.starts_with(".git")),
|
||||
!reported
|
||||
.iter()
|
||||
.any(|p| p.components().any(|c| c.as_os_str() == ".git")),
|
||||
"{reported:?}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1394,6 +1394,7 @@ impl Wiki {
|
||||
.ok_or_else(|| ai_memory_wiki_error("auto-improve proposal not found in scope"))?;
|
||||
|
||||
let path = detail.summary.target_path.clone();
|
||||
path.ensure_portable()?;
|
||||
let mut frontmatter = serde_json::json!({
|
||||
"kind": detail.summary.kind,
|
||||
"title": detail.summary.title,
|
||||
@@ -1810,6 +1811,11 @@ impl Wiki {
|
||||
if requests.is_empty() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
// Reject any path that cannot be materialised and checkpointed on every
|
||||
// supported platform, before anything is written.
|
||||
for req in &requests {
|
||||
req.path.ensure_portable()?;
|
||||
}
|
||||
// Pre-compute markdown for each request. Filesystem work happens only
|
||||
// after the mutation guard + project/workspace validation below.
|
||||
let mut staged: Vec<(
|
||||
@@ -5686,4 +5692,65 @@ mod tests {
|
||||
Some((ws, dst))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn write_page_and_apply_batch_refuse_git_reserved_and_non_portable_paths() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let (_store, wiki, ws, proj) = scoped(&tmp).await;
|
||||
|
||||
for bad in [
|
||||
"CON.md",
|
||||
"notes/aux.md",
|
||||
".git",
|
||||
".git/config",
|
||||
"notes/.git",
|
||||
"notes/.git/sub.md",
|
||||
"notes/.GIT/sub.md",
|
||||
"notes/git~1",
|
||||
"notes/git~1/foo.md",
|
||||
"notes/GIT~2/bar.md",
|
||||
] {
|
||||
let bad_path = PagePath::new(bad).unwrap();
|
||||
let write_err = wiki
|
||||
.write_page(req(
|
||||
ws,
|
||||
proj,
|
||||
bad_path.as_str(),
|
||||
"content",
|
||||
serde_json::json!({}),
|
||||
))
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
matches!(
|
||||
write_err,
|
||||
WikiError::Memory(ai_memory_core::MemoryError::InvalidPagePath(_))
|
||||
),
|
||||
"write_page should refuse {bad:?}, got: {write_err}"
|
||||
);
|
||||
|
||||
let batch_err = wiki
|
||||
.apply_batch(vec![req(
|
||||
ws,
|
||||
proj,
|
||||
bad_path.as_str(),
|
||||
"content",
|
||||
serde_json::json!({}),
|
||||
)])
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
matches!(
|
||||
batch_err,
|
||||
WikiError::Memory(ai_memory_core::MemoryError::InvalidPagePath(_))
|
||||
),
|
||||
"apply_batch should refuse {bad:?}, got: {batch_err}"
|
||||
);
|
||||
|
||||
assert!(
|
||||
!wiki.abs_path(ws, proj, &bad_path).exists(),
|
||||
"file for {bad:?} must not be written to disk"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user