From 0d5f8817ec0261ec3ad4b4f9aa05d5aa12473ae5 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Thu, 15 Oct 2026 23:57:03 -0300 Subject: [PATCH 1/3] fix(file): reject reads that do not fit the result buffer file_read returned success after clipping to the caller buffer. The agent passes a 4096-byte slot, so a later write_file could replace the file with that prefix and delete the rest. Supersedes #114. --- src/tools/file.c | 9 ++++++++- tests/test_file.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/tools/file.c b/src/tools/file.c index 4e07d83..cef427a 100644 --- a/src/tools/file.c +++ b/src/tools/file.c @@ -259,7 +259,14 @@ static int file_read(const char *path, char *result_buf, size_t max_len) return -1; } size_t to_read = (size_t)sz; - if (to_read >= max_len - 1) to_read = max_len - 1; + /* A prefix with return 0 is the whole file to the model. write_file would + * then replace the original and drop the unread tail. */ + if (to_read + 1 > max_len) { + fclose(f); + snprintf(result_buf, max_len, + "{\"error\":\"file exceeds result buffer\",\"bytes\":%ld}", sz); + return -1; + } size_t n = fread(result_buf, 1, to_read, f); fclose(f); result_buf[n] = '\0'; diff --git a/tests/test_file.c b/tests/test_file.c index 0907d27..a97221e 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -563,6 +563,48 @@ static void test_write_in_workspace_symlink_alias_rejected(void) rmdir(tmpdir); } +static void test_read_rejects_file_larger_than_result_buffer(void) +{ + char tmpdir[64]; + char notes_path[PATH_MAX]; + char config_path[PATH_MAX]; + char args[PATH_MAX + 64]; + char buf[32]; + char exact[8]; + FILE *f; + config_t *cfg; + const tool_t *t; + int i; + + snprintf(tmpdir, sizeof(tmpdir), "/tmp/sc_test_readcap_%d", (int)getpid()); + if (mkdir(tmpdir, 0755) != 0 && errno != EEXIST) + return; + snprintf(notes_path, sizeof(notes_path), "%s/notes.md", tmpdir); + f = fopen(notes_path, "w"); + MU_ASSERT(f != NULL, "create oversized notes.md"); + for (i = 0; i < 64; i++) + fputc('A', f); + fclose(f); + cfg = make_ws_config(tmpdir, config_path, sizeof(config_path)); + MU_ASSERT(cfg != NULL, "readcap: load config"); + tool_file_set_config(cfg); + t = tool_file_get(); + snprintf(args, sizeof(args), "{\"operation\":\"read_file\",\"path\":\"%s\"}", notes_path); + MU_ASSERT(t->execute(args, buf, sizeof(buf)) == -1, "oversize read fails"); + MU_ASSERT(strstr(buf, "error") != NULL, "oversize read reports an error"); + MU_ASSERT(strstr(buf, "AAAAAAAAAAAAAAAA") == NULL, "oversize read is not a raw prefix"); + f = fopen(notes_path, "w"); + MU_ASSERT(f != NULL, "rewrite exact-fit notes.md"); + fputs("0123456", f); + fclose(f); + MU_ASSERT(t->execute(args, exact, sizeof(exact)) == 0, "exact-fit read succeeds"); + MU_ASSERT(strcmp(exact, "0123456") == 0, "exact-fit read returns the whole file"); + config_free(cfg); + unlink(config_path); + unlink(notes_path); + rmdir(tmpdir); +} + int main(void) { MU_RUN(test_file_read_write_list); @@ -577,6 +619,7 @@ int main(void) MU_RUN(test_file_write_failure_preserves_existing); MU_RUN(test_write_does_not_clobber_sibling_tmp); MU_RUN(test_write_in_workspace_symlink_alias_rejected); + MU_RUN(test_read_rejects_file_larger_than_result_buffer); printf("%d tests run, %d failed\n", tests_run, tests_failed); return tests_failed ? 1 : 0; } From 0104890f31466361a278a44fc15d9d467d1e8771 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sat, 17 Oct 2026 02:20:04 -0300 Subject: [PATCH 2/3] docs(changelog): record read_file refuse-vs-truncate fix Add the Unreleased Fixed bullet for the file read bounds fix so the behavior change is changelogged like the other refuse-vs-truncate fixes in this tree. Review of #120 flagged the missing entry; docs only, no behavior change. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ce4ee7..9a8111b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to ShellClaw are documented here. Format follows [Keep a Cha ## [Unreleased] ### Fixed +- `read_file` reads that do not fit the result buffer now fail with a JSON error instead of returning a clipped prefix, so a later `write_file` cannot persist a silently truncated file. - Cron re-offer waits the full timeout when a due id could not be stored in the 16-slot table, and an ack frees that slot so the next due job can be delivered and then backed off. - Landlock device grants stay `/dev/null`, `/dev/zero`, `/dev/urandom`, and `/dev/tty` without `IOCTL_DEV`. A failed `landlock_add_rule` fails the ruleset closed. - Dashboard config save and JSON patch validation use a unique temp plus `fsync`, and the live gateway config pointer is published once under `agent_lock`. From fd3a0980ae8d0b90387dd365d3994c971a42db46 Mon Sep 17 00:00:00 2001 From: Adrianno Esnarriaga Sereno Date: Sat, 17 Oct 2026 02:20:05 -0300 Subject: [PATCH 3/3] test(file): assert full oversize error and one-over boundary The oversize regression used a 32-byte result buffer, smaller than the ~49-byte error JSON, so snprintf truncated the message and the assertions only proved a bare "error" substring. Give the oversize case a 56-byte buffer (still below the 64-byte fixture) so the exact refuse-to-truncate error and the "bytes" field can be asserted, and cover the last boundary: a read of exactly max_len bytes fails where max_len - 1 already succeeds. Test-only change per review of #120. --- tests/test_file.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/test_file.c b/tests/test_file.c index a97221e..490ba6f 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -569,7 +569,8 @@ static void test_read_rejects_file_larger_than_result_buffer(void) char notes_path[PATH_MAX]; char config_path[PATH_MAX]; char args[PATH_MAX + 64]; - char buf[32]; + /* Large enough for the ~49-byte error JSON, still smaller than the 64-byte file. */ + char buf[56]; char exact[8]; FILE *f; config_t *cfg; @@ -591,7 +592,9 @@ static void test_read_rejects_file_larger_than_result_buffer(void) t = tool_file_get(); snprintf(args, sizeof(args), "{\"operation\":\"read_file\",\"path\":\"%s\"}", notes_path); MU_ASSERT(t->execute(args, buf, sizeof(buf)) == -1, "oversize read fails"); - MU_ASSERT(strstr(buf, "error") != NULL, "oversize read reports an error"); + MU_ASSERT(strstr(buf, "\"error\":\"file exceeds result buffer\"") != NULL, + "oversize read reports the refuse-to-truncate error"); + MU_ASSERT(strstr(buf, "\"bytes\":64") != NULL, "error reports the file size"); MU_ASSERT(strstr(buf, "AAAAAAAAAAAAAAAA") == NULL, "oversize read is not a raw prefix"); f = fopen(notes_path, "w"); MU_ASSERT(f != NULL, "rewrite exact-fit notes.md"); @@ -599,6 +602,13 @@ static void test_read_rejects_file_larger_than_result_buffer(void) fclose(f); MU_ASSERT(t->execute(args, exact, sizeof(exact)) == 0, "exact-fit read succeeds"); MU_ASSERT(strcmp(exact, "0123456") == 0, "exact-fit read returns the whole file"); + /* One over the boundary: size == max_len must fail, not clip (max_len - 1 covered above). */ + f = fopen(notes_path, "w"); + MU_ASSERT(f != NULL, "rewrite one-over notes.md"); + fputs("01234567", f); + fclose(f); + MU_ASSERT(t->execute(args, exact, sizeof(exact)) == -1, "one-over read fails"); + MU_ASSERT(strstr(exact, "error") != NULL, "one-over read reports an error, not a prefix"); config_free(cfg); unlink(config_path); unlink(notes_path);