fix: preserve attachment lifecycle boundaries
This commit is contained in:
@@ -17,6 +17,8 @@ thiserror = { workspace = true }
|
||||
protocol = { workspace = true }
|
||||
tracing.workspace = true
|
||||
unicode-normalization = "0.1.25"
|
||||
unicode-properties = { version = "0.1.4", features = ["general-category"] }
|
||||
unicode-security = "0.1.2"
|
||||
|
||||
[dev-dependencies]
|
||||
async-trait = { workspace = true }
|
||||
|
||||
@@ -918,6 +918,36 @@ mod tests {
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn uploaded_file_names_reject_format_mixed_script_and_confusable_forms() {
|
||||
let tmp = tempfile::TempDir::new().unwrap();
|
||||
let store = FsStore::new(tmp.path()).unwrap();
|
||||
let session_id = new_session_id();
|
||||
let limits = UploadedFileLimits::default();
|
||||
|
||||
for file_name in [
|
||||
"safe\u{00ad}name.txt",
|
||||
"safe\u{061c}name.txt",
|
||||
"safe\u{180e}name.txt",
|
||||
"safe\u{e0001}name.txt",
|
||||
"p\u{0430}ypal.txt",
|
||||
"\u{0440}\u{0430}\u{0443}\u{0440}\u{0430}\u{04cf}.txt",
|
||||
"\u{ff26}\u{ff49}\u{ff4c}\u{ff45}.txt",
|
||||
"re\u{0301}sume\u{0301}.txt",
|
||||
] {
|
||||
assert!(matches!(
|
||||
store.write_uploaded_file(session_id, file_name, "text/plain", b"safe", limits),
|
||||
Err(StoreError::InvalidUploadedFileName)
|
||||
));
|
||||
}
|
||||
|
||||
for file_name in ["notes.txt", "résumé.txt", "日本語.txt", "📎.txt"] {
|
||||
store
|
||||
.write_uploaded_file(session_id, file_name, "text/plain", b"safe", limits)
|
||||
.unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn paste_artifact_limits_and_corruption_fail_closed() {
|
||||
let tmp = tempfile::TempDir::new().unwrap();
|
||||
|
||||
@@ -462,11 +462,18 @@ pub fn fork_at(
|
||||
) -> Result<SegmentId, StoreError> {
|
||||
let entries = store.read_all(source_session_id, source_id)?;
|
||||
let cut = if at_turn_index == 0 {
|
||||
// Branch directly after the SegmentStart (or whatever opens the
|
||||
// segment), before any turn completes.
|
||||
// Branch from the seeded state before any new turn completes. A typed
|
||||
// input checkpoint immediately following SegmentStart is part of that
|
||||
// seed and must stay atomic with its annotated history.
|
||||
entries
|
||||
.iter()
|
||||
.position(|e| !matches!(e, LogEntry::AnnotatedSegmentStart { .. }))
|
||||
.position(|entry| {
|
||||
!matches!(
|
||||
entry,
|
||||
LogEntry::AnnotatedSegmentStart { .. }
|
||||
| LogEntry::InputSegmentsCheckpoint { .. }
|
||||
)
|
||||
})
|
||||
.unwrap_or(entries.len())
|
||||
} else {
|
||||
entries
|
||||
@@ -478,8 +485,9 @@ pub fn fork_at(
|
||||
let state = segment_log::collect_state(&entries[..cut]);
|
||||
|
||||
let fork_id = crate::new_segment_id();
|
||||
let ts = segment_log::now_millis();
|
||||
let entry = LogEntry::AnnotatedSegmentStart {
|
||||
ts: segment_log::now_millis(),
|
||||
ts,
|
||||
session_id: source_session_id,
|
||||
system_prompt: state.system_prompt,
|
||||
config: state.config,
|
||||
@@ -490,7 +498,14 @@ pub fn fork_at(
|
||||
}),
|
||||
compacted_from: None,
|
||||
};
|
||||
store.create_segment(source_session_id, fork_id, &[entry])?;
|
||||
let mut fork_entries = vec![entry];
|
||||
if !state.user_segments.is_empty() {
|
||||
fork_entries.push(LogEntry::InputSegmentsCheckpoint {
|
||||
ts,
|
||||
user_segments: state.user_segments,
|
||||
});
|
||||
}
|
||||
store.create_segment(source_session_id, fork_id, &fork_entries)?;
|
||||
Ok(fork_id)
|
||||
}
|
||||
|
||||
|
||||
@@ -10,6 +10,8 @@ use protocol::{UploadedFileAvailability, UploadedFileRef};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use sha2::{Digest, Sha256};
|
||||
use unicode_normalization::UnicodeNormalization;
|
||||
use unicode_properties::general_category::{GeneralCategory, UnicodeGeneralCategory};
|
||||
use unicode_security::{confusable_detection::skeleton, mixed_script::MixedScript};
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::StoreError;
|
||||
@@ -62,18 +64,26 @@ struct StoredUploadedFile {
|
||||
}
|
||||
|
||||
pub(crate) fn validate_file_name(file_name: &str) -> Result<()> {
|
||||
let normalized: String = file_name.nfkc().collect();
|
||||
let stem = file_name
|
||||
.rsplit_once('.')
|
||||
.map_or(file_name, |(stem, _)| stem);
|
||||
let confusable_skeleton: String = skeleton(stem).collect();
|
||||
let ascii_confusable = stem.chars().any(|ch| !ch.is_ascii())
|
||||
&& confusable_skeleton.is_ascii()
|
||||
&& !confusable_skeleton.eq_ignore_ascii_case(stem);
|
||||
|
||||
if file_name.is_empty()
|
||||
|| file_name.chars().count() > MAX_FILE_NAME_CHARS
|
||||
|| file_name == "."
|
||||
|| file_name == ".."
|
||||
|| normalized != file_name
|
||||
|| !stem.is_single_script()
|
||||
|| ascii_confusable
|
||||
|| file_name.chars().any(|ch| {
|
||||
ch.is_control()
|
||||
|| matches!(
|
||||
ch,
|
||||
'/' | '\\' | '\u{200b}' | '\u{200c}' | '\u{200d}' | '\u{2060}' | '\u{feff}'
|
||||
)
|
||||
|| ('\u{202a}'..='\u{202e}').contains(&ch)
|
||||
|| ('\u{2066}'..='\u{2069}').contains(&ch)
|
||||
|| ch.general_category() == GeneralCategory::Format
|
||||
|| matches!(ch, '/' | '\\')
|
||||
})
|
||||
{
|
||||
return Err(StoreError::InvalidUploadedFileName);
|
||||
|
||||
@@ -10,6 +10,7 @@ use agen::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput};
|
||||
use agen::{Engine, History};
|
||||
use async_trait::async_trait;
|
||||
use common::MockLlmClient;
|
||||
use protocol::{Segment, SessionSnapshotEntryData, UploadedFileAvailability, UploadedFileRef};
|
||||
use session_store::{FsStore, LogEntry, SegmentStartState, Store, collect_state};
|
||||
|
||||
// =============================================================================
|
||||
@@ -455,7 +456,11 @@ async fn session_fork_at_truncates_within_session() {
|
||||
let fork_segid = session_store::fork_at(&store, sid, segid, worker.turn_count()).unwrap();
|
||||
|
||||
let fork_entries = store.read_all(sid, fork_segid).unwrap();
|
||||
assert_eq!(fork_entries.len(), 1); // Just the new SegmentStart
|
||||
assert_eq!(fork_entries.len(), 2);
|
||||
assert!(matches!(
|
||||
&fork_entries[1],
|
||||
LogEntry::InputSegmentsCheckpoint { .. }
|
||||
));
|
||||
|
||||
let fork_state = collect_state(&fork_entries);
|
||||
assert_eq!(fork_state.session_id, Some(sid), "fork_at inherits Session");
|
||||
@@ -467,6 +472,7 @@ async fn session_fork_at_truncates_within_session() {
|
||||
.position(|e| matches!(e, LogEntry::TurnEnd { turn_count, .. } if *turn_count == worker.turn_count()))
|
||||
.expect("source segment has the matching TurnEnd");
|
||||
let source_state_at_fork = collect_state(&all_entries[..=turn_end_pos]);
|
||||
assert_eq!(fork_state.user_segments, source_state_at_fork.user_segments);
|
||||
assert_eq!(fork_state.history.len(), source_state_at_fork.history.len());
|
||||
assert_eq!(
|
||||
fork_state.annotated_history, source_state_at_fork.annotated_history,
|
||||
@@ -492,6 +498,58 @@ async fn session_fork_at_truncates_within_session() {
|
||||
assert!(segs.contains(&fork_segid));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rewound_fork_preserves_uploaded_file_segments_in_snapshot() {
|
||||
let (_dir, store) = make_store();
|
||||
let config = RequestConfig::default();
|
||||
let (sid, segid) = session_store::create_segment(
|
||||
&store,
|
||||
SegmentStartState {
|
||||
system_prompt: Some("System prompt"),
|
||||
config: &config,
|
||||
history: Vec::new(),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
let uploaded = UploadedFileRef {
|
||||
artifact_id: "uploaded-file-1".into(),
|
||||
file_name: "notes.txt".into(),
|
||||
media_type: "text/plain".into(),
|
||||
created_at_ms: 123,
|
||||
availability: UploadedFileAvailability::Available,
|
||||
byte_len: 5,
|
||||
sha256: "a".repeat(64),
|
||||
source_entry_id: Some("entry-1".into()),
|
||||
};
|
||||
let segments = vec![Segment::UploadedFile {
|
||||
file: uploaded.clone(),
|
||||
}];
|
||||
session_store::save_user_input(
|
||||
&store,
|
||||
sid,
|
||||
segid,
|
||||
segments.clone(),
|
||||
annotated(&[Item::user_message(Segment::flatten_to_text(&segments))]),
|
||||
)
|
||||
.unwrap();
|
||||
session_store::save_turn_end(&store, sid, segid, 1).unwrap();
|
||||
|
||||
let fork_segid = session_store::fork_at(&store, sid, segid, 1).unwrap();
|
||||
let fork_entries = store.read_all(sid, fork_segid).unwrap();
|
||||
let snapshot = session_store::public_snapshot::project_session_snapshot(sid, &fork_entries);
|
||||
|
||||
assert!(fork_entries.iter().any(|entry| matches!(
|
||||
entry,
|
||||
LogEntry::InputSegmentsCheckpoint { user_segments, .. }
|
||||
if user_segments == &vec![segments.clone()]
|
||||
)));
|
||||
assert!(snapshot.entries.iter().any(|entry| matches!(
|
||||
&entry.data,
|
||||
SessionSnapshotEntryData::UserInput { segments: restored }
|
||||
if restored == &segments
|
||||
)));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn session_config_changed_logged() {
|
||||
let (_dir, store) = make_store();
|
||||
@@ -654,12 +712,19 @@ async fn nested_past_fork_leaves_ancestors_immutable() {
|
||||
let fork1_entries = store.read_all(sid, fork1).unwrap();
|
||||
assert_eq!(
|
||||
fork1_entries.len(),
|
||||
1,
|
||||
"fork1 is just its SegmentStart seed"
|
||||
2,
|
||||
"fork1 stores its SegmentStart and typed input checkpoint"
|
||||
);
|
||||
|
||||
// fork2's lineage points at fork1, not the root.
|
||||
match &store.read_all(sid, fork2).unwrap()[0] {
|
||||
// fork2's lineage points at fork1, not the root, and the typed seed remains
|
||||
// intact across the nested turn-zero fork.
|
||||
let fork2_entries = store.read_all(sid, fork2).unwrap();
|
||||
assert_eq!(fork2_entries.len(), 2);
|
||||
assert_eq!(
|
||||
collect_state(&fork2_entries).user_segments,
|
||||
collect_state(&fork1_entries).user_segments
|
||||
);
|
||||
match &fork2_entries[0] {
|
||||
LogEntry::AnnotatedSegmentStart {
|
||||
forked_from: Some(origin),
|
||||
..
|
||||
|
||||
Reference in New Issue
Block a user