Skip to content

Commit 8fc1949

Browse files
committed
Add cbm_replace_binary: unlink-before-write for update command
TDD: two tests (overwrite read-only file, create new file) written first, then cbm_replace_binary() implemented and wired into update. The update command failed when the existing binary had no write permission (e.g., 0500). unlink() only requires write permission on the parent directory, not the file itself, so removing first then creating with O_CREAT | 0755 handles all permission combinations. Fixes #114.
1 parent 7e78e30 commit 8fc1949

File tree

3 files changed

+125
-18
lines changed

3 files changed

+125
-18
lines changed

src/cli/cli.c

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,40 @@ int cbm_copy_file(const char *src, const char *dst) {
258258
return rc == 0 ? 0 : -1;
259259
}
260260

261+
/* Replace a binary file: unlink first (handles read-only existing files),
262+
* then create with the given data and permissions. */
263+
int cbm_replace_binary(const char *path, const unsigned char *data, int len, int mode) {
264+
if (!path || !data || len <= 0) {
265+
return -1;
266+
}
267+
268+
/* Remove existing file first — handles the case where the old binary
269+
* has no write permission (e.g., 0500). unlink() only requires write
270+
* permission on the parent directory, not the file itself. */
271+
(void)cbm_unlink(path);
272+
273+
#ifndef _WIN32
274+
int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, (mode_t)mode);
275+
if (fd < 0) {
276+
return -1;
277+
}
278+
FILE *f = fdopen(fd, "wb");
279+
if (!f) {
280+
close(fd);
281+
return -1;
282+
}
283+
#else
284+
FILE *f = fopen(path, "wb");
285+
if (!f) {
286+
return -1;
287+
}
288+
#endif
289+
290+
size_t written = fwrite(data, 1, (size_t)len, f);
291+
(void)fclose(f);
292+
return written == (size_t)len ? 0 : -1;
293+
}
294+
261295
/* ── Skill file content (embedded) ────────────────────────────── */
262296

263297
static const char skill_exploring_content[] =
@@ -2822,28 +2856,13 @@ int cbm_cmd_update(int argc, char **argv) {
28222856
return 1;
28232857
}
28242858

2825-
/* Open with final permissions atomically (no TOCTOU between write and chmod) */
2826-
#ifndef _WIN32
2827-
int fd = open(bin_dest, O_WRONLY | O_CREAT | O_TRUNC, 0755);
2828-
if (fd < 0) {
2829-
fprintf(stderr, "error: cannot write to %s\n", bin_dest);
2830-
free(bin_data);
2831-
return 1;
2832-
}
2833-
FILE *out = fdopen(fd, "wb");
2834-
#else
2835-
FILE *out = fopen(bin_dest, "wb");
2836-
#endif
2837-
if (!out) {
2859+
/* Replace binary: unlink first (handles read-only existing file),
2860+
* then create with 0755 permissions. Fixes #114. */
2861+
if (cbm_replace_binary(bin_dest, bin_data, bin_len, 0755) != 0) {
28382862
fprintf(stderr, "error: cannot write to %s\n", bin_dest);
28392863
free(bin_data);
2840-
#ifndef _WIN32
2841-
close(fd);
2842-
#endif
28432864
return 1;
28442865
}
2845-
fwrite(bin_data, 1, (size_t)bin_len, out);
2846-
fclose(out);
28472866
free(bin_data);
28482867
} else {
28492868
/* Zip extraction: exec unzip directly without shell interpretation */

src/cli/cli.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@ const char *cbm_find_cli(const char *name, const char *home_dir);
4545
/* Copy a file from src to dst. Returns 0 on success, -1 on error. */
4646
int cbm_copy_file(const char *src, const char *dst);
4747

48+
/* Replace a binary file: unlinks the existing file first (handles read-only),
49+
* then creates a new file with the given data and permissions.
50+
* Returns 0 on success, -1 on error. */
51+
int cbm_replace_binary(const char *path, const unsigned char *data, int len, int mode);
52+
4853
/* ── Skill file management ────────────────────────────────────── */
4954

5055
/* Number of skill files. */

tests/test_cli.c

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2012,6 +2012,83 @@ TEST(cli_config_persists) {
20122012
PASS();
20132013
}
20142014

2015+
/* ═══════════════════════════════════════════════════════════════════
2016+
* Group H: cbm_replace_binary (update command helper)
2017+
* ═══════════════════════════════════════════════════════════════════ */
2018+
2019+
#ifndef _WIN32
2020+
2021+
TEST(replace_binary_overwrites_readonly) {
2022+
/* Simulate #114: existing binary has mode 0500 (no write permission).
2023+
* cbm_replace_binary must unlink first, then create with 0755. */
2024+
char tmpdir[256];
2025+
snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-replace-XXXXXX");
2026+
if (!cbm_mkdtemp(tmpdir)) {
2027+
SKIP("cbm_mkdtemp failed");
2028+
}
2029+
2030+
char path[512];
2031+
snprintf(path, sizeof(path), "%s/test-binary", tmpdir);
2032+
2033+
/* Create a read-only file (simulating an installed binary with 0500) */
2034+
FILE *f = fopen(path, "w");
2035+
ASSERT_NOT_NULL(f);
2036+
fputs("old-content", f);
2037+
fclose(f);
2038+
chmod(path, 0500); /* r-x------ */
2039+
2040+
/* Replace it with new content */
2041+
const unsigned char new_data[] = "new-content-replaced";
2042+
int rc = cbm_replace_binary(path, new_data, (int)sizeof(new_data) - 1, 0755);
2043+
ASSERT_EQ(rc, 0);
2044+
2045+
/* Verify new content was written */
2046+
FILE *check = fopen(path, "r");
2047+
ASSERT_NOT_NULL(check);
2048+
char buf[64] = {0};
2049+
fread(buf, 1, sizeof(buf) - 1, check);
2050+
fclose(check);
2051+
ASSERT_STR_EQ(buf, "new-content-replaced");
2052+
2053+
/* Verify permissions are 0755 */
2054+
struct stat st;
2055+
ASSERT_EQ(stat(path, &st), 0);
2056+
ASSERT_EQ(st.st_mode & 0777, 0755);
2057+
2058+
remove(path);
2059+
rmdir(tmpdir);
2060+
PASS();
2061+
}
2062+
2063+
TEST(replace_binary_creates_new_file) {
2064+
/* If no existing file, cbm_replace_binary should create it. */
2065+
char tmpdir[256];
2066+
snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-replace2-XXXXXX");
2067+
if (!cbm_mkdtemp(tmpdir)) {
2068+
SKIP("cbm_mkdtemp failed");
2069+
}
2070+
2071+
char path[512];
2072+
snprintf(path, sizeof(path), "%s/new-binary", tmpdir);
2073+
2074+
const unsigned char data[] = "brand-new";
2075+
int rc = cbm_replace_binary(path, data, (int)sizeof(data) - 1, 0755);
2076+
ASSERT_EQ(rc, 0);
2077+
2078+
FILE *check = fopen(path, "r");
2079+
ASSERT_NOT_NULL(check);
2080+
char buf[64] = {0};
2081+
fread(buf, 1, sizeof(buf) - 1, check);
2082+
fclose(check);
2083+
ASSERT_STR_EQ(buf, "brand-new");
2084+
2085+
remove(path);
2086+
rmdir(tmpdir);
2087+
PASS();
2088+
}
2089+
2090+
#endif /* _WIN32 */
2091+
20152092
/* ═══════════════════════════════════════════════════════════════════
20162093
* Suite definition
20172094
* ═══════════════════════════════════════════════════════════════════ */
@@ -2148,4 +2225,10 @@ SUITE(cli) {
21482225
RUN_TEST(cli_config_get_int);
21492226
RUN_TEST(cli_config_delete);
21502227
RUN_TEST(cli_config_persists);
2228+
2229+
/* Replace binary (update command helper — group H) */
2230+
#ifndef _WIN32
2231+
RUN_TEST(replace_binary_overwrites_readonly);
2232+
RUN_TEST(replace_binary_creates_new_file);
2233+
#endif
21512234
}

0 commit comments

Comments
 (0)