fix: fence attachment grants and lifecycle cleanup
This commit is contained in:
@@ -20,10 +20,13 @@ use crate::paste_artifact::{read_from_dir, write_to_dir};
|
||||
use crate::segment_log::LogEntry;
|
||||
use crate::store::{Store, StoreError};
|
||||
use crate::uploaded_file::{
|
||||
bind_uploaded_file, delete_uncommitted_uploaded_files, delete_uploaded_file,
|
||||
bind_uploaded_file, clear_uploaded_file_binding, copy_committed_uploaded_files,
|
||||
delete_uncommitted_uploaded_files, delete_uploaded_file, list_uploaded_file_refs,
|
||||
read_uploaded_file, read_uploaded_file_by_id, write_uploaded_file,
|
||||
};
|
||||
use crate::{PasteArtifactLimits, SegmentId, SessionId, UploadedFileLimits};
|
||||
use crate::{
|
||||
PasteArtifactLimits, SegmentId, SessionId, UploadedFileLimits, UploadedFileUploadContext,
|
||||
};
|
||||
use protocol::{PasteArtifactRef, UploadedFileRef};
|
||||
use std::fs;
|
||||
use std::io::{Read, Seek, SeekFrom, Write};
|
||||
@@ -119,6 +122,40 @@ impl FsStore {
|
||||
self.session_dir(session_id).join("artifacts").join("paste")
|
||||
}
|
||||
|
||||
fn uploaded_file_is_referenced(
|
||||
&self,
|
||||
session_id: SessionId,
|
||||
artifact_id: &str,
|
||||
) -> Result<bool, StoreError> {
|
||||
fn segments_contain(segments: &[protocol::Segment], artifact_id: &str) -> bool {
|
||||
segments.iter().any(|segment| {
|
||||
matches!(
|
||||
segment,
|
||||
protocol::Segment::UploadedFile { file }
|
||||
if file.artifact_id == artifact_id
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
for segment_id in self.list_segments(session_id)? {
|
||||
for entry in self.read_all(session_id, segment_id)? {
|
||||
let referenced = match entry {
|
||||
LogEntry::AnnotatedUserInput { segments, .. } => {
|
||||
segments_contain(&segments, artifact_id)
|
||||
}
|
||||
LogEntry::InputSegmentsCheckpoint { user_segments, .. } => user_segments
|
||||
.iter()
|
||||
.any(|segments| segments_contain(segments, artifact_id)),
|
||||
_ => false,
|
||||
};
|
||||
if referenced {
|
||||
return Ok(true);
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(false)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
fn paste_artifact_path(&self, session_id: SessionId, artifact_id: &str) -> PathBuf {
|
||||
self.paste_artifact_dir(session_id)
|
||||
@@ -410,6 +447,30 @@ impl Store for FsStore {
|
||||
file_name,
|
||||
media_type,
|
||||
content,
|
||||
None,
|
||||
limits,
|
||||
)
|
||||
}
|
||||
|
||||
fn write_uploaded_file_with_context(
|
||||
&self,
|
||||
session_id: SessionId,
|
||||
file_name: &str,
|
||||
media_type: &str,
|
||||
content: &[u8],
|
||||
context: &UploadedFileUploadContext,
|
||||
limits: UploadedFileLimits,
|
||||
) -> Result<UploadedFileRef, StoreError> {
|
||||
let _guard = self
|
||||
.append_lock
|
||||
.lock()
|
||||
.map_err(|_| std::io::Error::other("session store append lock was poisoned"))?;
|
||||
write_uploaded_file(
|
||||
&self.paste_artifact_dir(session_id),
|
||||
file_name,
|
||||
media_type,
|
||||
content,
|
||||
Some(context),
|
||||
limits,
|
||||
)
|
||||
}
|
||||
@@ -440,11 +501,21 @@ impl Store for FsStore {
|
||||
.append_lock
|
||||
.lock()
|
||||
.map_err(|_| std::io::Error::other("session store append lock was poisoned"))?;
|
||||
bind_uploaded_file(
|
||||
&self.paste_artifact_dir(session_id),
|
||||
reference,
|
||||
source_entry_id,
|
||||
)
|
||||
let dir = self.paste_artifact_dir(session_id);
|
||||
match bind_uploaded_file(&dir, reference, source_entry_id) {
|
||||
Err(StoreError::ArtifactAlreadyCommitted) => {
|
||||
let (stored, _) = read_uploaded_file_by_id(&dir, &reference.artifact_id)?;
|
||||
let previous_source = stored
|
||||
.source_entry_id
|
||||
.ok_or(StoreError::ArtifactIntegrityMismatch)?;
|
||||
if self.uploaded_file_is_referenced(session_id, &reference.artifact_id)? {
|
||||
return Err(StoreError::ArtifactAlreadyCommitted);
|
||||
}
|
||||
clear_uploaded_file_binding(&dir, &reference.artifact_id, &previous_source)?;
|
||||
bind_uploaded_file(&dir, reference, source_entry_id)
|
||||
}
|
||||
result => result,
|
||||
}
|
||||
}
|
||||
|
||||
fn delete_uploaded_file(
|
||||
@@ -464,7 +535,37 @@ impl Store for FsStore {
|
||||
.append_lock
|
||||
.lock()
|
||||
.map_err(|_| std::io::Error::other("session store append lock was poisoned"))?;
|
||||
delete_uncommitted_uploaded_files(&self.paste_artifact_dir(session_id))
|
||||
let dir = self.paste_artifact_dir(session_id);
|
||||
let mut removed = delete_uncommitted_uploaded_files(&dir)?;
|
||||
for reference in list_uploaded_file_refs(&dir)? {
|
||||
let Some(source_entry_id) = reference.source_entry_id.as_deref() else {
|
||||
continue;
|
||||
};
|
||||
if !self.uploaded_file_is_referenced(session_id, &reference.artifact_id)? {
|
||||
clear_uploaded_file_binding(&dir, &reference.artifact_id, source_entry_id)?;
|
||||
if delete_uploaded_file(&dir, &reference.artifact_id)? {
|
||||
removed = removed
|
||||
.checked_add(1)
|
||||
.ok_or(StoreError::ArtifactQuotaExceeded)?;
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(removed)
|
||||
}
|
||||
|
||||
fn copy_committed_uploaded_files(
|
||||
&self,
|
||||
source_session_id: SessionId,
|
||||
target_session_id: SessionId,
|
||||
) -> Result<u64, StoreError> {
|
||||
let _guard = self
|
||||
.append_lock
|
||||
.lock()
|
||||
.map_err(|_| std::io::Error::other("session store append lock was poisoned"))?;
|
||||
copy_committed_uploaded_files(
|
||||
&self.paste_artifact_dir(source_session_id),
|
||||
&self.paste_artifact_dir(target_session_id),
|
||||
)
|
||||
}
|
||||
|
||||
fn append_trace(
|
||||
@@ -626,6 +727,45 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn uploaded_file_persists_trusted_upload_context_without_projecting_it() {
|
||||
let tmp = tempfile::TempDir::new().unwrap();
|
||||
let store = FsStore::new(tmp.path()).unwrap();
|
||||
let session_id = new_session_id();
|
||||
let context = UploadedFileUploadContext {
|
||||
upload_id: "upload-1".into(),
|
||||
principal_id: "account-1".into(),
|
||||
workspace_id: "workspace-1".into(),
|
||||
runtime_id: "runtime-1".into(),
|
||||
worker_id: "worker-1".into(),
|
||||
};
|
||||
let reference = store
|
||||
.write_uploaded_file_with_context(
|
||||
session_id,
|
||||
"notes.txt",
|
||||
"text/plain",
|
||||
b"hello",
|
||||
&context,
|
||||
UploadedFileLimits::default(),
|
||||
)
|
||||
.unwrap();
|
||||
let raw = fs::read_to_string(
|
||||
store
|
||||
.paste_artifact_dir(session_id)
|
||||
.join(format!("{}.file.json", reference.artifact_id)),
|
||||
)
|
||||
.unwrap();
|
||||
assert!(raw.contains("account-1"));
|
||||
assert!(raw.contains("workspace-1"));
|
||||
assert!(raw.contains("runtime-1"));
|
||||
assert!(raw.contains("worker-1"));
|
||||
assert!(
|
||||
!serde_json::to_string(&reference)
|
||||
.unwrap()
|
||||
.contains("account-1")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn uploaded_files_are_session_scoped_integrity_checked_and_removable() {
|
||||
let tmp = tempfile::TempDir::new().unwrap();
|
||||
@@ -715,18 +855,51 @@ mod tests {
|
||||
store.write_uploaded_file(session_id, "README.txt", "text/plain", b"y", limits),
|
||||
Err(StoreError::InvalidUploadedFileName)
|
||||
));
|
||||
store
|
||||
.bind_uploaded_file(session_id, &pending, "entry-from-failed-submit")
|
||||
.unwrap();
|
||||
let bound = store
|
||||
.bind_uploaded_file(session_id, &pending, "entry-upload")
|
||||
.unwrap();
|
||||
store
|
||||
.create_segment(
|
||||
session_id,
|
||||
new_segment_id(),
|
||||
&[LogEntry::InputSegmentsCheckpoint {
|
||||
ts: 1,
|
||||
user_segments: vec![vec![protocol::Segment::UploadedFile {
|
||||
file: bound.clone(),
|
||||
}]],
|
||||
}],
|
||||
)
|
||||
.unwrap();
|
||||
let other = store
|
||||
.write_uploaded_file(session_id, "other.txt", "text/plain", b"z", limits)
|
||||
.unwrap();
|
||||
let stale = store
|
||||
.write_uploaded_file(session_id, "stale.txt", "text/plain", b"s", limits)
|
||||
.unwrap();
|
||||
store
|
||||
.bind_uploaded_file(session_id, &stale, "entry-never-committed")
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
store.delete_uncommitted_uploaded_files(session_id).unwrap(),
|
||||
1
|
||||
2
|
||||
);
|
||||
assert!(store.read_uploaded_file(session_id, &other).is_err());
|
||||
assert!(store.read_uploaded_file(session_id, &stale).is_err());
|
||||
assert_eq!(store.read_uploaded_file(session_id, &bound).unwrap(), b"x");
|
||||
let fork_session_id = new_session_id();
|
||||
assert_eq!(
|
||||
store
|
||||
.copy_committed_uploaded_files(session_id, fork_session_id)
|
||||
.unwrap(),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
store.read_uploaded_file(fork_session_id, &bound).unwrap(),
|
||||
b"x"
|
||||
);
|
||||
store
|
||||
.write_paste_artifact(
|
||||
session_id,
|
||||
|
||||
@@ -70,6 +70,7 @@ pub use system_item::{
|
||||
pub use uploaded_file::{
|
||||
DEFAULT_MAX_FILES_PER_SUBMISSION, DEFAULT_MAX_SESSION_ARTIFACT_BYTES,
|
||||
DEFAULT_MAX_SESSION_UPLOADED_FILES, DEFAULT_MAX_UPLOADED_FILE_BYTES, UploadedFileLimits,
|
||||
UploadedFileUploadContext,
|
||||
};
|
||||
pub use worker_metadata::{
|
||||
CombinedStore, FsWorkerStore, WorkerActiveSegmentRef, WorkerAggregateStore, WorkerMetadata,
|
||||
|
||||
@@ -420,6 +420,7 @@ pub fn save_config_changed(
|
||||
/// [`fork_at`] or [`ensure_head_or_fork`] instead.
|
||||
pub fn fork(
|
||||
store: &impl Store,
|
||||
source_session_id: SessionId,
|
||||
state: SegmentStartState<'_>,
|
||||
) -> Result<(SessionId, SegmentId), StoreError> {
|
||||
let session_id = crate::new_session_id();
|
||||
@@ -434,6 +435,7 @@ pub fn fork(
|
||||
compacted_from: None,
|
||||
};
|
||||
store.create_segment(session_id, fork_id, &[entry])?;
|
||||
store.copy_committed_uploaded_files(source_session_id, session_id)?;
|
||||
Ok((session_id, fork_id))
|
||||
}
|
||||
|
||||
|
||||
@@ -13,7 +13,9 @@
|
||||
|
||||
use crate::event_trace::TraceEntry;
|
||||
use crate::segment_log::LogEntry;
|
||||
use crate::{PasteArtifactLimits, SegmentId, SessionId, UploadedFileLimits};
|
||||
use crate::{
|
||||
PasteArtifactLimits, SegmentId, SessionId, UploadedFileLimits, UploadedFileUploadContext,
|
||||
};
|
||||
use protocol::{PasteArtifactRef, UploadedFileRef};
|
||||
|
||||
/// Errors from the persistence store.
|
||||
@@ -186,6 +188,18 @@ pub trait Store: Send + Sync {
|
||||
Err(StoreError::PasteArtifactUnsupported)
|
||||
}
|
||||
|
||||
fn write_uploaded_file_with_context(
|
||||
&self,
|
||||
session_id: SessionId,
|
||||
file_name: &str,
|
||||
media_type: &str,
|
||||
content: &[u8],
|
||||
_context: &UploadedFileUploadContext,
|
||||
limits: UploadedFileLimits,
|
||||
) -> Result<UploadedFileRef, StoreError> {
|
||||
self.write_uploaded_file(session_id, file_name, media_type, content, limits)
|
||||
}
|
||||
|
||||
/// Read and integrity-check an uploaded file owned by `session_id`.
|
||||
fn read_uploaded_file(
|
||||
&self,
|
||||
@@ -225,6 +239,14 @@ pub trait Store: Send + Sync {
|
||||
Ok(0)
|
||||
}
|
||||
|
||||
fn copy_committed_uploaded_files(
|
||||
&self,
|
||||
_source_session_id: SessionId,
|
||||
_target_session_id: SessionId,
|
||||
) -> Result<u64, StoreError> {
|
||||
Ok(0)
|
||||
}
|
||||
|
||||
/// Append a trace entry to the debug event trace file.
|
||||
fn append_trace(
|
||||
&self,
|
||||
|
||||
@@ -38,6 +38,15 @@ impl Default for UploadedFileLimits {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct UploadedFileUploadContext {
|
||||
pub upload_id: String,
|
||||
pub principal_id: String,
|
||||
pub workspace_id: String,
|
||||
pub runtime_id: String,
|
||||
pub worker_id: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize, Deserialize)]
|
||||
struct StoredUploadedFile {
|
||||
file_name: String,
|
||||
@@ -47,6 +56,8 @@ struct StoredUploadedFile {
|
||||
sha256: String,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
source_entry_id: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
upload_context: Option<UploadedFileUploadContext>,
|
||||
content_base64: String,
|
||||
}
|
||||
|
||||
@@ -186,6 +197,7 @@ pub(crate) fn write_uploaded_file(
|
||||
file_name: &str,
|
||||
media_type: &str,
|
||||
content: &[u8],
|
||||
context: Option<&UploadedFileUploadContext>,
|
||||
limits: UploadedFileLimits,
|
||||
) -> Result<UploadedFileRef> {
|
||||
validate_file_name(file_name)?;
|
||||
@@ -226,6 +238,7 @@ pub(crate) fn write_uploaded_file(
|
||||
if stored.media_type == media_type
|
||||
&& stored.byte_len == byte_len
|
||||
&& stored.sha256 == sha256
|
||||
&& stored.upload_context.as_ref() == context
|
||||
{
|
||||
let artifact_id = path
|
||||
.file_name()
|
||||
@@ -264,6 +277,7 @@ pub(crate) fn write_uploaded_file(
|
||||
byte_len,
|
||||
sha256: sha256.clone(),
|
||||
source_entry_id: None,
|
||||
upload_context: context.cloned(),
|
||||
content_base64: BASE64.encode(content),
|
||||
};
|
||||
let path = record_path(dir, &artifact_id)?;
|
||||
@@ -324,6 +338,30 @@ pub(crate) fn read_uploaded_file(dir: &Path, reference: &UploadedFileRef) -> Res
|
||||
Ok(content)
|
||||
}
|
||||
|
||||
pub(crate) fn clear_uploaded_file_binding(
|
||||
dir: &Path,
|
||||
artifact_id: &str,
|
||||
expected_source_entry_id: &str,
|
||||
) -> Result<()> {
|
||||
fs::create_dir_all(dir)?;
|
||||
let aggregate_lock = fs::OpenOptions::new()
|
||||
.create(true)
|
||||
.read(true)
|
||||
.write(true)
|
||||
.open(dir.join(".aggregate.lock"))?;
|
||||
FileExt::lock_exclusive(&aggregate_lock)?;
|
||||
let path = record_path(dir, artifact_id)?;
|
||||
let mut stored: StoredUploadedFile = serde_json::from_slice(&fs::read(&path)?)?;
|
||||
if stored.source_entry_id.as_deref() != Some(expected_source_entry_id) {
|
||||
return Err(StoreError::ArtifactIntegrityMismatch);
|
||||
}
|
||||
stored.source_entry_id = None;
|
||||
let temp = dir.join(format!(".{artifact_id}.file.unbind.tmp"));
|
||||
fs::write(&temp, serde_json::to_vec(&stored)?)?;
|
||||
fs::rename(temp, path)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(crate) fn bind_uploaded_file(
|
||||
dir: &Path,
|
||||
reference: &UploadedFileRef,
|
||||
@@ -338,7 +376,15 @@ pub(crate) fn bind_uploaded_file(
|
||||
.write(true)
|
||||
.open(dir.join(".aggregate.lock"))?;
|
||||
FileExt::lock_exclusive(&aggregate_lock)?;
|
||||
read_uploaded_file(dir, reference)?;
|
||||
let (stored_reference, _) = read_uploaded_file_by_id(dir, &reference.artifact_id)?;
|
||||
if stored_reference.file_name != reference.file_name
|
||||
|| stored_reference.media_type != reference.media_type
|
||||
|| stored_reference.created_at_ms != reference.created_at_ms
|
||||
|| stored_reference.byte_len != reference.byte_len
|
||||
|| stored_reference.sha256 != reference.sha256
|
||||
{
|
||||
return Err(StoreError::ArtifactIntegrityMismatch);
|
||||
}
|
||||
let path = record_path(dir, &reference.artifact_id)?;
|
||||
let mut stored: StoredUploadedFile = serde_json::from_slice(&fs::read(&path)?)?;
|
||||
if stored.source_entry_id.is_some() {
|
||||
@@ -353,6 +399,72 @@ pub(crate) fn bind_uploaded_file(
|
||||
Ok(bound)
|
||||
}
|
||||
|
||||
pub(crate) fn list_uploaded_file_refs(dir: &Path) -> Result<Vec<UploadedFileRef>> {
|
||||
if !dir.exists() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
let mut refs = Vec::new();
|
||||
for entry in fs::read_dir(dir)? {
|
||||
let path = entry?.path();
|
||||
let Some(artifact_id) = path
|
||||
.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.and_then(|name| name.strip_suffix(".file.json"))
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
refs.push(read_uploaded_file_by_id(dir, artifact_id)?.0);
|
||||
}
|
||||
Ok(refs)
|
||||
}
|
||||
|
||||
pub(crate) fn copy_committed_uploaded_files(source_dir: &Path, target_dir: &Path) -> Result<u64> {
|
||||
if !source_dir.exists() {
|
||||
return Ok(0);
|
||||
}
|
||||
fs::create_dir_all(target_dir)?;
|
||||
let target_lock = fs::OpenOptions::new()
|
||||
.create(true)
|
||||
.read(true)
|
||||
.write(true)
|
||||
.open(target_dir.join(".aggregate.lock"))?;
|
||||
FileExt::lock_exclusive(&target_lock)?;
|
||||
let mut copied = 0_u64;
|
||||
for entry in fs::read_dir(source_dir)? {
|
||||
let entry = entry?;
|
||||
let path = entry.path();
|
||||
let Some(name) = path.file_name().and_then(|name| name.to_str()) else {
|
||||
continue;
|
||||
};
|
||||
if !name.ends_with(".file.json") {
|
||||
continue;
|
||||
}
|
||||
let bytes = fs::read(&path)?;
|
||||
let stored: StoredUploadedFile = serde_json::from_slice(&bytes)?;
|
||||
if stored.source_entry_id.is_none() {
|
||||
continue;
|
||||
}
|
||||
let target = target_dir.join(name);
|
||||
if target.exists() {
|
||||
let existing: StoredUploadedFile = serde_json::from_slice(&fs::read(&target)?)?;
|
||||
if existing.sha256 != stored.sha256
|
||||
|| existing.file_name != stored.file_name
|
||||
|| existing.source_entry_id != stored.source_entry_id
|
||||
{
|
||||
return Err(StoreError::ArtifactIntegrityMismatch);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
let temp = target_dir.join(format!(".{name}.copy.tmp"));
|
||||
fs::write(&temp, &bytes)?;
|
||||
fs::rename(temp, target)?;
|
||||
copied = copied
|
||||
.checked_add(1)
|
||||
.ok_or(StoreError::ArtifactQuotaExceeded)?;
|
||||
}
|
||||
Ok(copied)
|
||||
}
|
||||
|
||||
pub(crate) fn delete_uncommitted_uploaded_files(dir: &Path) -> Result<u64> {
|
||||
fs::create_dir_all(dir)?;
|
||||
let aggregate_lock = fs::OpenOptions::new()
|
||||
|
||||
@@ -406,6 +406,7 @@ async fn session_fork_creates_new_session() {
|
||||
let original_history_len = worker.history().len();
|
||||
let (fork_sid, fork_segid) = session_store::fork(
|
||||
&store,
|
||||
sid,
|
||||
SegmentStartState {
|
||||
system_prompt: worker.get_system_prompt(),
|
||||
config: worker.request_config(),
|
||||
|
||||
Reference in New Issue
Block a user