kyle/kte
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Crash safety: non-regular files, filesystem errors, command exceptions

Browse Source 
- OpenFromFile opens non-blocking, fstat()s and accepts regular files
  only. A directory made tellg() report a huge size and bad_alloc exit
  the editor (reachable by Tab-completing a directory in the open
  prompt); a FIFO blocked open() forever with SIGINT ignored; a device
  node opened as an empty, file-backed buffer that saving then replaced
  with a regular file. Files are read to EOF in chunks; a file too large
  to load is reported, not fatal.
- Saving refuses to replace an existing non-regular file.
- Exceptions escaping a command handler are reported in the status line
  and error log instead of propagating to main() and exiting, which lost
  every unsaved buffer (unnamed buffers' journals are never recovered).
- Commands use non-throwing filesystem queries for existence checks,
  the current directory and Tab completion (ENAMETOOLONG, EACCES or a
  deleted working directory used to throw).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rcVQiiWyTzoNrsvLSaAfB
This commit is contained in:
Claude
2026-09-24 09:16:04 -07:00
committed by Kyle Isom
parent cb9b6bdf45
commit e279945136
3 changed files with 153 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Buffer.cc
+51 -36
View File
@@ -175,6 +175,11 @@ atomic_write_file(const std::string &path_in, const char *data, std::size_t len,
struct stat dst_st{};
const bool dst_exists = ::stat(path.c_str(), &dst_st) == 0;
// Never replace a directory, device node, FIFO or socket with a file.
if (dst_exists && !S_ISREG(dst_st.st_mode)) {
err = "Refusing to overwrite a non-regular file: " + path;
return false;
}
// A file with other hard links must be rewritten in place, or those
// names would keep the old content.
@@ -484,7 +489,15 @@ Buffer::OpenFromFile(const std::string &path, std::string &err)
const std::string norm = normalize_path(path);
// If the file doesn't exist, initialize an empty, non-file-backed buffer
// with the provided filename. Do not touch the filesystem until Save/SaveAs.
if (!std::filesystem::exists(norm)) {
std::error_code exists_ec;
const bool exists = std::filesystem::exists(norm, exists_ec);
if (exists_ec) {
// e.g. name too long, or a parent directory we may not search
err = "Cannot open " + norm + ": " + exists_ec.message();
kte::ErrorHandler::Instance().Error("Buffer", err, norm);
return false;
}
if (!exists) {
rows_.clear();
nrows_ = 0;
filename_ = norm;
@@ -508,50 +521,52 @@ Buffer::OpenFromFile(const std::string &path, std::string &err)
return true;
}
std::ifstream in(norm, std::ios::in | std::ios::binary);
if (!in) {
err = "Failed to open file: " + norm;
// Open non-blocking so a named pipe cannot hang the editor, then accept
// regular files only: a directory reads as garbage (or throws), a FIFO
// blocks, and saving over a device node would replace it with a file.
int oflags = O_RDONLY | O_NONBLOCK;
#ifdef O_CLOEXEC
oflags |= O_CLOEXEC;
#endif
const int fd = kte::syscall::Open(norm.c_str(), oflags);
if (fd < 0) {
err = "Failed to open file: " + norm + ": " + std::strerror(errno);
kte::ErrorHandler::Instance().Error("Buffer", err, norm);
return false;
}
struct stat st{};
if (kte::syscall::Fstat(fd, &st) != 0 || !S_ISREG(st.st_mode)) {
err = S_ISDIR(st.st_mode) ? ("Is a directory: " + norm) : ("Not a regular file: " + norm);
(void) kte::syscall::Close(fd);
kte::ErrorHandler::Instance().Error("Buffer", err, norm);
return false;
}
// Read entire file into PieceTable as-is
// Read to EOF in chunks (the file may change size while we read).
std::string data;
in.seekg(0, std::ios::end);
if (!in) {
err = "Failed to seek to end of file: " + norm;
kte::ErrorHandler::Instance().Error("Buffer", err, norm);
return false;
}
auto sz = in.tellg();
if (sz < 0) {
err = "Failed to get file size: " + norm;
kte::ErrorHandler::Instance().Error("Buffer", err, norm);
return false;
}
if (sz > 0) {
data.resize(static_cast<std::size_t>(sz));
in.seekg(0, std::ios::beg);
if (!in) {
err = "Failed to seek to beginning of file: " + norm;
kte::ErrorHandler::Instance().Error("Buffer", err, norm);
return false;
}
in.read(data.data(), static_cast<std::streamsize>(data.size()));
if (!in && !in.eof()) {
err = "Failed to read file: " + norm;
kte::ErrorHandler::Instance().Error("Buffer", err, norm);
return false;
}
// Validate we read the expected number of bytes
const std::streamsize bytes_read = in.gcount();
if (bytes_read != static_cast<std::streamsize>(data.size())) {
err = "Partial read of file (expected " + std::to_string(data.size()) +
" bytes, got " + std::to_string(bytes_read) + "): " + norm;
try {
data.reserve(static_cast<std::size_t>(st.st_size));
char chunk[1 << 16];
for (;;) {
const ssize_t n = ::read(fd, chunk, sizeof(chunk));
if (n > 0) {
data.append(chunk, static_cast<std::size_t>(n));
} else if (n == 0) {
break;
} else if (errno != EINTR) {
err = "Failed to read file: " + norm + ": " + std::strerror(errno);
(void) kte::syscall::Close(fd);
kte::ErrorHandler::Instance().Error("Buffer", err, norm);
return false;
}
}
} catch (const std::bad_alloc &) {
(void) kte::syscall::Close(fd);
err = "File too large to load: " + norm;
kte::ErrorHandler::Instance().Error("Buffer", err, norm);
return false;
}
(void) kte::syscall::Close(fd);
content_.Clear();
if (!data.empty())
content_.Append(data.data(), data.size());
Command.cc
+52 -9
View File
@@ -11,6 +11,7 @@
#include <string_view>
#include "Command.h"
#include "ErrorHandler.h"
#include "RegexGuard.h"
#include "TermWidth.h"
#include "syntax/HighlighterRegistry.h"
@@ -342,6 +343,26 @@ is_mutating_command(CommandId id)
}
// Non-throwing filesystem queries for command code. The throwing overloads
// fail on ENAMETOOLONG, EACCES or a deleted working directory, which used
// to terminate the editor.
static bool
fs_exists(const std::string &path)
{
std::error_code ec;
return std::filesystem::exists(path, ec) && !ec;
}
static std::filesystem::path
safe_current_path()
{
std::error_code ec;
auto p = std::filesystem::current_path(ec);
return ec ? std::filesystem::path(".") : p;
}
// True if `path` names the file `buf` is backed by (same inode).
static bool
is_buffers_own_file(const Buffer &buf, const std::string &path)
@@ -906,7 +927,7 @@ cmd_save(CommandContext &ctx)
if (!buf->IsFileBacked()) {
if (!buf->Filename().empty()) {
// If first-time save to an existing path, confirm overwrite
if (std::filesystem::exists(buf->Filename())) {
if (fs_exists(buf->Filename())) {
ctx.editor.StartPrompt(Editor::PromptKind::Confirm, "Overwrite", "");
ctx.editor.SetPendingOverwritePath(buf->Filename());
ctx.editor.SetStatus(
@@ -997,7 +1018,7 @@ cmd_save_as(CommandContext &ctx)
return false;
}
// Ask before replacing an existing file other than the buffer's own.
if (std::filesystem::exists(ctx.arg) && !is_buffers_own_file(*buf, ctx.arg)) {
if (fs_exists(ctx.arg) && !is_buffers_own_file(*buf, ctx.arg)) {
ctx.editor.StartPrompt(Editor::PromptKind::Confirm, "Overwrite", "");
ctx.editor.SetPendingOverwritePath(ctx.arg);
ctx.editor.SetStatus(std::string("Overwrite existing file '") + ctx.arg + "'? (y/N)");
@@ -1846,7 +1867,7 @@ cmd_visual_file_picker_toggle(const CommandContext &ctx)
// Initialize directory to current working directory if empty
if (ctx.editor.FilePickerDir().empty()) {
try {
ctx.editor.SetFilePickerDir(std::filesystem::current_path().string());
ctx.editor.SetFilePickerDir(safe_current_path().string());
} catch (...) {
ctx.editor.SetFilePickerDir(".");
}
@@ -2074,17 +2095,18 @@ cmd_insert_text(CommandContext &ctx)
std::filesystem::path p(expanded);
std::filesystem::path dir;
std::string base;
std::error_code dir_ec;
if (expanded.empty()) {
dir = std::filesystem::current_path();
dir = safe_current_path();
base.clear();
} else if (std::filesystem::is_directory(p)) {
} else if (std::filesystem::is_directory(p, dir_ec)) {
dir = p;
base.clear();
} else {
dir = p.parent_path();
base = p.filename().string();
if (dir.empty())
dir = std::filesystem::current_path();
dir = safe_current_path();
}
std::error_code ec;
@@ -2934,7 +2956,7 @@ cmd_newline(CommandContext &ctx)
value = expand_user_path(value);
// Ask before overwriting any existing file other than the
// buffer's own (it used to ask only for unnamed buffers).
if (std::filesystem::exists(value) && !is_buffers_own_file(*buf, value)) {
if (fs_exists(value) && !is_buffers_own_file(*buf, value)) {
ctx.editor.StartPrompt(Editor::PromptKind::Confirm, "Overwrite", "");
ctx.editor.SetPendingOverwritePath(value);
ctx.editor.SetStatus(
@@ -5555,6 +5577,27 @@ InstallDefaultCommands()
}
// Run a command handler. An exception escaping a command (a filesystem
// error, bad_alloc on a huge operation, ...) used to reach main() and exit
// the editor, losing every unsaved buffer; report it and keep running.
static bool
run_handler(Editor &ed, const Command &cmd, CommandContext &ctx)
{
if (!cmd.handler)
return false;
try {
return cmd.handler(ctx);
} catch (const std::exception &e) {
kte::ErrorHandler::Instance().Error("Command", std::string(cmd.name) + ": " + e.what(), "");
ed.SetStatus(std::string("Error in ") + cmd.name + ": " + e.what());
} catch (...) {
kte::ErrorHandler::Instance().Error("Command", std::string(cmd.name) + ": unknown exception", "");
ed.SetStatus(std::string("Error in ") + cmd.name);
}
return false;
}
bool
Execute(Editor &ed, CommandId id, const std::string &arg, int count)
{
@@ -5605,7 +5648,7 @@ Execute(Editor &ed, CommandId id, const std::string &arg, int count)
}
CommandContext ctx{ed, arg, final_count};
return cmd->handler ? cmd->handler(ctx) : false;
return run_handler(ed, *cmd, ctx);
}
@@ -5623,5 +5666,5 @@ Execute(Editor &ed, const std::string &name, const std::string &arg, int count)
}
}
CommandContext ctx{ed, arg, count};
return cmd->handler ? cmd->handler(ctx) : false;
return run_handler(ed, *cmd, ctx);
}
tests/test_audit_regressions.cc
+49
View File
@@ -831,3 +831,52 @@ TEST(Audit_HorizontalScroll_ControlCharsAreTwoCells)
ASSERT_TRUE(h.Exec(CommandId::MoveEnd));
ASSERT_EQ(b.Coloffs(), (std::size_t) (100 - 80 + 1));
}
// Opening a directory or a FIFO fails cleanly (a directory used to throw
// bad_alloc and exit the editor; a FIFO blocked forever).
TEST(Audit_Open_NonRegularFiles_Rejected)
{
TempDir d("open_nonreg");
Buffer b;
std::string err;
ASSERT_TRUE(!b.OpenFromFile(d.path.string(), err));
ASSERT_TRUE(err.find("directory") != std::string::npos);
const std::string fifo = (d.path / "pipe").string();
ASSERT_EQ(::mkfifo(fifo.c_str(), 0600), 0);
err.clear();
ASSERT_TRUE(!b.OpenFromFile(fifo, err));
}
// Saving never replaces a non-regular file (e.g. a device node or FIFO).
TEST(Audit_Save_RefusesNonRegularTarget)
{
TempDir d("save_nonreg");
const std::string fifo = (d.path / "pipe").string();
ASSERT_EQ(::mkfifo(fifo.c_str(), 0600), 0);
Buffer b;
b.insert_text(0, 0, "x\n");
std::string err;
ASSERT_TRUE(!b.SaveAs(fifo, err));
struct stat st{};
ASSERT_EQ(::lstat(fifo.c_str(), &st), 0);
ASSERT_TRUE(S_ISFIFO(st.st_mode));
}
// Filesystem errors in commands (here: a name longer than NAME_MAX) are
// reported, not thrown out of the editor.
TEST(Audit_Commands_FilesystemErrorsDoNotThrow)
{
TestHarness h;
Editor &ed = h.EditorRef();
h.Buf().insert_text(0, 0, "precious\n");
const std::string longname = "/tmp/" + std::string(400, 'n');
ASSERT_TRUE(Execute(ed, "save-as", longname) || true);
ASSERT_TRUE(h.Exec(CommandId::OpenFileStart));
ASSERT_TRUE(h.Exec(CommandId::InsertText, longname));
(void) h.Exec(CommandId::Newline);
(void) ed.ProcessPendingOpens();
ASSERT_EQ(h.Text(), std::string("precious\n"));
}