Skip to content

Commit 0f32e17

Browse files
acarl005warp-agentoz-agent
authored andcommitted
fix: cap unbounded file reads in FileModel to prevent multi-GB memory spikes (APP-5343)
FileModel::open and FileModel::read_content_for_file (crates/warp_files) called async_fs::read_to_string with no size check, so opening a pathologically large file (e.g. a multi-gigabyte log or binary opened by mistake) allocated memory proportional to the file's size. A Sentry memory alert captured a single 8 GB allocation from exactly this call stack. Add FileLoadError::TooLarge and a MAX_LOADABLE_FILE_SIZE_BYTES (100 MB) cap enforced via a new FileModel::check_not_too_large helper, applied before every unbounded read_to_string call site (open, read_content_for_file, and the file-watcher auto-reload path). Surface a specific, actionable toast message in the code editor when a file is rejected for being too large. Co-Authored-By: Warp Agent <agent@warp.dev> Co-Authored-By: Oz <oz-agent@warp.dev>
1 parent e2a0802 commit 0f32e17

5 files changed

Lines changed: 149 additions & 8 deletions

File tree

app/src/ai/blocklist/action_model/execute.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,6 +1333,11 @@ async fn read_binary_file_context(
13331333
Ok(content) => content,
13341334
Err(FileLoadError::DoesNotExist) => return Ok(BinaryFileReadResult::NotFound),
13351335
Err(FileLoadError::IOError(e)) => return Err(anyhow::anyhow!(e)),
1336+
// `read_file_as_binary` never checks against `MAX_LOADABLE_FILE_SIZE_BYTES`
1337+
// itself (this function already enforces its own `max_bytes` limit
1338+
// above), so this arm should be unreachable in practice. Handled
1339+
// explicitly so the match stays exhaustive as `FileLoadError` grows.
1340+
Err(err @ FileLoadError::TooLarge { .. }) => return Err(anyhow::anyhow!(err)),
13361341
};
13371342

13381343
let mime_type = from_path(path).first_or_octet_stream().to_string();

app/src/code/view.rs

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,23 @@ pub fn init(app: &mut AppContext) {
127127

128128
const PADDING: f32 = 4.;
129129

130+
/// Renders a byte count in human-readable units (e.g. `8.6 GB`), used for the
131+
/// "file too large to open" toast message.
132+
fn format_file_size(bytes: u64) -> String {
133+
const UNITS: [&str; 5] = ["B", "KB", "MB", "GB", "TB"];
134+
let mut size = bytes as f64;
135+
let mut unit_index = 0;
136+
while size >= 1024.0 && unit_index < UNITS.len() - 1 {
137+
size /= 1024.0;
138+
unit_index += 1;
139+
}
140+
if unit_index == 0 {
141+
format!("{bytes} {}", UNITS[unit_index])
142+
} else {
143+
format!("{size:.1} {}", UNITS[unit_index])
144+
}
145+
}
146+
130147
pub use crate::util::openable_file_type::is_binary_file;
131148
/// Determines the `SavePosition` ID for a draggable tab based on its index.
132149
pub fn tab_position_id(index: usize) -> String {
@@ -516,7 +533,7 @@ impl CodeView {
516533
return;
517534
}
518535
log::warn!("Failed to load file. {err:?}");
519-
CodeView::display_load_failure(ctx.window_id(), ctx);
536+
CodeView::display_load_failure(ctx.window_id(), err, ctx);
520537
}
521538
LocalCodeEditorEvent::SelectionAddedAsContext {
522539
relative_file_path,
@@ -943,10 +960,26 @@ impl CodeView {
943960
}
944961
}
945962

946-
fn display_load_failure(window_id: WindowId, ctx: &mut ViewContext<Self>) {
963+
fn display_load_failure(
964+
window_id: WindowId,
965+
error: &warp_util::file::FileLoadError,
966+
ctx: &mut ViewContext<Self>,
967+
) {
968+
let message = match error {
969+
warp_util::file::FileLoadError::TooLarge {
970+
size_bytes,
971+
limit_bytes,
972+
} => format!(
973+
"File is too large to open ({} > {} limit).",
974+
format_file_size(*size_bytes),
975+
format_file_size(*limit_bytes)
976+
),
977+
warp_util::file::FileLoadError::DoesNotExist
978+
| warp_util::file::FileLoadError::IOError(_) => "Failed to load file.".to_string(),
979+
};
947980
ToastStack::handle(ctx).update(ctx, |toast_stack, ctx| {
948-
let toast = DismissibleToast::error(String::from("Failed to load file."))
949-
.with_object_id("failed_to_load_file".to_string());
981+
let toast =
982+
DismissibleToast::error(message).with_object_id("failed_to_load_file".to_string());
950983
toast_stack.add_ephemeral_toast(toast, window_id, ctx);
951984
});
952985
}

crates/warp_files/src/lib.rs

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use repo_metadata::repository::{RepositorySubscriber, SubscriberId};
2424
use repo_metadata::{CanonicalizedPath, Repository, RepositoryUpdate, RepositoryWatchMode};
2525
use warp_core::HostId;
2626
use warp_util::content_version::ContentVersion;
27-
use warp_util::file::{FileId, FileLoadError, FileSaveError};
27+
use warp_util::file::{FileId, FileLoadError, FileSaveError, MAX_LOADABLE_FILE_SIZE_BYTES};
2828
use warp_util::standardized_path::StandardizedPath;
2929
use warpui_core::r#async::SpawnedFutureHandle;
3030
use warpui_core::{Entity, ModelContext, ModelHandle, SingletonEntity};
@@ -447,9 +447,12 @@ impl FileModel {
447447
let file_path_buf = file_path.to_owned();
448448
let future = ctx.spawn(
449449
async move {
450-
let contents = async_fs::read_to_string(&file_path_buf)
451-
.await
452-
.map_err(FileLoadError::from);
450+
let contents = match Self::check_not_too_large(&file_path_buf).await {
451+
Ok(()) => async_fs::read_to_string(&file_path_buf)
452+
.await
453+
.map_err(FileLoadError::from),
454+
Err(err) => Err(err),
455+
};
453456
(file_id, contents)
454457
},
455458
move |me, (file_id, load_result), ctx| {
@@ -521,11 +524,37 @@ impl FileModel {
521524
if !Self::file_exists(file_path).await {
522525
return Err(FileLoadError::DoesNotExist);
523526
}
527+
Self::check_not_too_large(file_path).await?;
524528
async_fs::read_to_string(file_path)
525529
.await
526530
.map_err(FileLoadError::from)
527531
}
528532

533+
/// Returns `Err(FileLoadError::TooLarge)` if `path`'s on-disk size exceeds
534+
/// [`MAX_LOADABLE_FILE_SIZE_BYTES`]. Reading a file's full contents into a
535+
/// `String` (as `open`/`read_content_for_file` do) can otherwise trigger a
536+
/// single allocation as large as the file itself; an unexpectedly huge
537+
/// file (a multi-gigabyte log, database, or binary opened by mistake) can
538+
/// then spike process memory by that same amount. Checking the size
539+
/// up front avoids ever attempting that allocation.
540+
///
541+
/// If `metadata` fails (e.g. the file does not exist or was removed
542+
/// concurrently), this returns `Ok(())` so the caller's own read attempt
543+
/// surfaces the more specific underlying error.
544+
async fn check_not_too_large(path: &Path) -> Result<(), FileLoadError> {
545+
let Ok(metadata) = async_fs::metadata(path).await else {
546+
return Ok(());
547+
};
548+
let size_bytes = metadata.len();
549+
if size_bytes > MAX_LOADABLE_FILE_SIZE_BYTES {
550+
return Err(FileLoadError::TooLarge {
551+
size_bytes,
552+
limit_bytes: MAX_LOADABLE_FILE_SIZE_BYTES,
553+
});
554+
}
555+
Ok(())
556+
}
557+
529558
/// Asynchronously reads specific lines from a file using BufReader.
530559
///
531560
/// # Arguments
@@ -1145,6 +1174,10 @@ impl FileModel {
11451174
async move {
11461175
let mut res = Vec::new();
11471176
for file_path in matching_files {
1177+
if let Err(err) = Self::check_not_too_large(&file_path).await {
1178+
log::warn!("Skipping auto-reload of {}: {err}", file_path.display());
1179+
continue;
1180+
}
11481181
if let Ok(content) = async_fs::read_to_string(&file_path).await {
11491182
res.push((file_path, content));
11501183
}

crates/warp_files/src/lib_tests.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,4 +356,64 @@ fn test_a_failed_open_registers_no_watcher() {
356356
});
357357
}
358358

359+
/// Opening a file whose on-disk size exceeds `MAX_LOADABLE_FILE_SIZE_BYTES`
360+
/// must fail with `FileLoadError::TooLarge` instead of reading the whole file
361+
/// into memory. A single pathologically large file (a huge log, a binary
362+
/// opened by mistake, etc.) could otherwise trigger a multi-gigabyte
363+
/// allocation. Uses a sparse file (`set_len`) so the test doesn't actually
364+
/// need to write the oversized content to disk.
365+
#[test]
366+
fn test_load_oversized_file_reports_too_large() {
367+
App::test((), |mut app| async move {
368+
let app = &mut app;
369+
let files = app.add_singleton_model(FileModel::new);
370+
let receiver = setup_event_channel(app, &files);
371+
372+
let directory = tempfile::tempdir().expect("temp dir");
373+
let path = directory.path().join("huge.log");
374+
let file = std::fs::File::create(&path).expect("create file");
375+
file.set_len(MAX_LOADABLE_FILE_SIZE_BYTES + 1)
376+
.expect("set sparse length");
377+
drop(file);
378+
379+
files.update(app, |model, ctx| {
380+
model.open(&path, false, ctx);
381+
});
382+
383+
match receiver.recv().await.expect("Could not receive the result") {
384+
TestFileModelEvent::FailedToLoad(err) => {
385+
assert!(
386+
err.contains("TooLarge"),
387+
"expected TooLarge error, got {err}"
388+
);
389+
}
390+
event => panic!("Expected oversized file to fail to load, got {event:?}"),
391+
}
392+
});
393+
}
394+
395+
/// [`FileModel::read_content_for_file`] is used for reload/discard flows and
396+
/// must apply the same size guard as `open`.
397+
#[test]
398+
fn test_read_content_for_file_reports_too_large() {
399+
App::test((), |mut _app| async move {
400+
let directory = tempfile::tempdir().expect("temp dir");
401+
let path = directory.path().join("huge.log");
402+
let file = std::fs::File::create(&path).expect("create file");
403+
file.set_len(MAX_LOADABLE_FILE_SIZE_BYTES + 1)
404+
.expect("set sparse length");
405+
drop(file);
406+
407+
let result = FileModel::read_content_for_file(&path).await;
408+
assert!(
409+
matches!(
410+
result,
411+
Err(FileLoadError::TooLarge { limit_bytes, .. })
412+
if limit_bytes == MAX_LOADABLE_FILE_SIZE_BYTES
413+
),
414+
"expected TooLarge error, got {result:?}"
415+
);
416+
});
417+
}
418+
359419
static TEST_FILE_CONTENT: &[u8] = include_bytes!("../test_data/test_file.rs");

crates/warp_util/src/file.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,22 @@ impl ErrorExt for FileSaveError {
3232
}
3333
register_error!(FileSaveError);
3434

35+
/// Maximum size, in bytes, of a file that can be fully loaded into memory as a
36+
/// `String` (e.g. to populate an editor buffer). Reading larger files whole
37+
/// risks multi-gigabyte allocations for pathologically large files (logs,
38+
/// binaries opened by mistake, etc.); callers should check
39+
/// [`FileLoadError::TooLarge`] and surface a friendly error instead of
40+
/// attempting the read.
41+
pub const MAX_LOADABLE_FILE_SIZE_BYTES: u64 = 100 * 1024 * 1024;
42+
3543
#[derive(thiserror::Error, Debug)]
3644
pub enum FileLoadError {
3745
#[error("File does not exist")]
3846
DoesNotExist,
3947
#[error("IO error when loading file.")]
4048
IOError(#[from] io::Error),
49+
#[error("File is too large to open ({size_bytes} bytes, limit is {limit_bytes} bytes)")]
50+
TooLarge { size_bytes: u64, limit_bytes: u64 },
4151
}
4252

4353
#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]

0 commit comments

Comments
 (0)