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`. 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..490ba6f 100644 --- a/tests/test_file.c +++ b/tests/test_file.c @@ -563,6 +563,58 @@ 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]; + /* 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; + 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\":\"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"); + 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"); + /* 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); + rmdir(tmpdir); +} + int main(void) { MU_RUN(test_file_read_write_list); @@ -577,6 +629,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; }