Skip to content

fix(file): do not collapse missing path components onto ancestor files #67

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
cursor wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(file): do not collapse missing path components onto ancestor files #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 69 additions & 31 deletions src/tools/file.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,22 @@ void tool_file_set_config(const config_t *cfg)
g_file_cfg = cfg;
}

static int resolved_is_under_workspace(const char *resolved)
{
char ws_resolved[PATH_MAX];
const char *workspace;
size_t ws_len;

if (!resolved || !g_file_cfg) return 0;
workspace = config_workspace_path(g_file_cfg);
if (!workspace || workspace[0] == '\0') return 0;
if (realpath(workspace, ws_resolved) == NULL) return 0;
ws_len = strlen(ws_resolved);
if (strncmp(resolved, ws_resolved, ws_len) != 0) return 0;
if (resolved[ws_len] != '\0' && resolved[ws_len] != '/') return 0;
return 1;
}

static int path_within_workspace(const char *path, char *resolved, size_t resolved_size)
{
if (!path || path[0] == '\0') return 0;
Expand All @@ -42,38 +58,68 @@ static int path_within_workspace(const char *path, char *resolved, size_t resolv
if (!workspace || workspace[0] == '\0') {
return 0; /* Deny: cannot validate without workspace path */
}
char ws_resolved[PATH_MAX];
if (realpath(workspace, ws_resolved) == NULL) return 0;
if (realpath(path, resolved) != NULL) {
size_t ws_len = strlen(ws_resolved);
if (strncmp(resolved, ws_resolved, ws_len) != 0) return 0;
if (resolved[ws_len] != '\0' && resolved[ws_len] != '/') return 0;
return 1;
}
if (realpath(workspace, resolved) == NULL) return 0;
if (realpath(path, resolved) != NULL)
return resolved_is_under_workspace(resolved);
char path_copy[PATH_MAX];
snprintf(path_copy, sizeof(path_copy), "%s", path);
for (;;) {
char *dir = dirname(path_copy);
if (!dir || dir[0] == '\0') break;
if (realpath(dir, resolved) != NULL) {
size_t ws_len = strlen(ws_resolved);
if (strncmp(resolved, ws_resolved, ws_len) != 0) return 0;
if (resolved[ws_len] != '\0' && resolved[ws_len] != '/') return 0;
return 1;
}
if (realpath(dir, resolved) != NULL)
return resolved_is_under_workspace(resolved);
if (strcmp(dir, ".") == 0 || strcmp(dir, "/") == 0) break;
snprintf(path_copy, sizeof(path_copy), "%s", dir);
}
return 0;
}

/*
* Map a workspace write to the intended path. Ancestor lookup in
* path_within_workspace is only a membership check: using that ancestor as
* the fopen target truncates an existing file treated as a directory, or
* overwrites a same-named file in a parent when intermediate dirs are missing.
*/
static int resolve_workspace_write_path(const char *path, char *safe_path, size_t cap)
{
char parent[PATH_MAX];
char path_copy[PATH_MAX];
struct stat st;
const char *base;
char *dir;
int n;

if (realpath(path, safe_path) != NULL) {
if (stat(safe_path, &st) != 0 || !S_ISREG(st.st_mode)) return 0;
return resolved_is_under_workspace(safe_path);
}
if (snprintf(path_copy, sizeof(path_copy), "%s", path) >= (int)sizeof(path_copy))
return 0;
dir = dirname(path_copy);
if (!dir || realpath(dir, parent) == NULL) return 0;
if (stat(parent, &st) != 0 || !S_ISDIR(st.st_mode)) return 0;
if (!resolved_is_under_workspace(parent)) return 0;
base = strrchr(path, '/');
base = base ? base + 1 : path;
if (base[0] == '\0' || strcmp(base, ".") == 0 || strcmp(base, "..") == 0)
return 0;
n = snprintf(safe_path, cap, "%s/%s", parent, base);
return n > 0 && (size_t)n < cap;
}

static int file_read(const char *path, char *result_buf, size_t max_len)
{
char resolved[PATH_MAX];
int ws_only;
if (!path_within_workspace(path, resolved, sizeof(resolved))) {
snprintf(result_buf, max_len, "{\"error\":\"path outside workspace\"}");
return -1;
}
ws_only = g_file_cfg ? config_workspace_only(g_file_cfg) : 0;
if (ws_only && realpath(path, resolved) == NULL) {
snprintf(result_buf, max_len, "{\"error\":\"cannot open file\"}");
return -1;
}
FILE *f = fopen(resolved, "rb");
if (!f) {
snprintf(result_buf, max_len, "{\"error\":\"cannot open file\"}");
Expand Down Expand Up @@ -106,23 +152,9 @@ static int file_write(const char *path, const char *content, char *result_buf, s
char safe_path[PATH_MAX];
if (!ws_only) {
snprintf(safe_path, sizeof(safe_path), "%s", path);
} else {
struct stat st;
if (stat(resolved, &st) == 0 && S_ISREG(st.st_mode)) {
snprintf(safe_path, sizeof(safe_path), "%s", resolved);
} else {
const char *base = strrchr(path, '/');
base = base ? base + 1 : path;
size_t res_len = strlen(resolved);
size_t base_len = strlen(base);
if (res_len + 1 + base_len >= sizeof(safe_path)) {
snprintf(result_buf, max_len, "{\"error\":\"path too long\"}");
return -1;
}
memcpy(safe_path, resolved, res_len);
safe_path[res_len] = '/';
memcpy(safe_path + res_len + 1, base, base_len + 1);
}
} else if (!resolve_workspace_write_path(path, safe_path, sizeof(safe_path))) {
snprintf(result_buf, max_len, "{\"error\":\"cannot write file\"}");
return -1;
}
FILE *f = fopen(safe_path, "w");
if (!f) {
Expand All @@ -145,10 +177,16 @@ static int file_write(const char *path, const char *content, char *result_buf, s
static int file_list(const char *path, char *result_buf, size_t max_len)
{
char resolved[PATH_MAX];
int ws_only;
if (!path_within_workspace(path, resolved, sizeof(resolved))) {
snprintf(result_buf, max_len, "{\"error\":\"path outside workspace\"}");
return -1;
}
ws_only = g_file_cfg ? config_workspace_only(g_file_cfg) : 0;
if (ws_only && realpath(path, resolved) == NULL) {
snprintf(result_buf, max_len, "{\"error\":\"cannot list directory\"}");
return -1;
}
DIR *d = opendir(resolved);
if (!d) {
snprintf(result_buf, max_len, "{\"error\":\"cannot list directory\"}");
Expand Down
93 changes: 93 additions & 0 deletions tests/test_file.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,13 +196,106 @@ static void test_symlink_escape_rejected(void)
rmdir(tmpdir);
}

static int slurp_file(const char *path, char *buf, size_t cap)
{
FILE *f = fopen(path, "rb");
size_t n;
if (!f) return -1;
n = fread(buf, 1, cap - 1, f);
fclose(f);
buf[n] = '\0';
return 0;
}

static void test_write_does_not_truncate_file_used_as_directory(void)
{
char tmpdir[PATH_MAX];
char victim[PATH_MAX];
char nested[PATH_MAX];
char config_path[PATH_MAX];
char args[PATH_MAX + 128];
char buf[256];
char kept[64];
config_t *cfg;
const tool_t *t;
FILE *f;
int r;

snprintf(tmpdir, sizeof(tmpdir), "/tmp/sc_test_filedir_%d", (int)getpid());
if (mkdir(tmpdir, 0755) != 0 && errno != EEXIST) return;
snprintf(victim, sizeof(victim), "%s/important.md", tmpdir);
f = fopen(victim, "w");
MU_ASSERT(f != NULL, "create victim file");
fputs("KEEP", f);
fclose(f);
cfg = make_ws_config(tmpdir, config_path, sizeof(config_path));
MU_ASSERT(cfg != NULL, "file-as-dir: load config");
tool_file_set_config(cfg);
t = tool_file_get();
snprintf(nested, sizeof(nested), "%s/important.md/nested.txt", tmpdir);
snprintf(args, sizeof(args),
"{\"operation\":\"write_file\",\"path\":\"%s\",\"content\":\"PWNED\"}", nested);
r = t->execute(args, buf, sizeof(buf));
MU_ASSERT(r == -1, "write through file-as-directory is rejected");
MU_ASSERT(slurp_file(victim, kept, sizeof(kept)) == 0, "victim still readable");
MU_ASSERT(strcmp(kept, "KEEP") == 0, "victim content preserved");
snprintf(args, sizeof(args), "{\"operation\":\"read_file\",\"path\":\"%s\"}", nested);
r = t->execute(args, buf, sizeof(buf));
MU_ASSERT(r == -1, "read through file-as-directory is rejected");
MU_ASSERT(strstr(buf, "KEEP") == NULL, "read does not leak victim contents");
config_free(cfg);
unlink(config_path);
unlink(victim);
rmdir(tmpdir);
}

static void test_write_does_not_collapse_missing_parent_onto_basename(void)
{
char tmpdir[PATH_MAX];
char victim[PATH_MAX];
char nested[PATH_MAX];
char config_path[PATH_MAX];
char args[PATH_MAX + 128];
char buf[256];
char kept[64];
config_t *cfg;
const tool_t *t;
FILE *f;
int r;

snprintf(tmpdir, sizeof(tmpdir), "/tmp/sc_test_missdir_%d", (int)getpid());
if (mkdir(tmpdir, 0755) != 0 && errno != EEXIST) return;
snprintf(victim, sizeof(victim), "%s/notes.md", tmpdir);
f = fopen(victim, "w");
MU_ASSERT(f != NULL, "create same-basename victim");
fputs("KEEP", f);
fclose(f);
cfg = make_ws_config(tmpdir, config_path, sizeof(config_path));
MU_ASSERT(cfg != NULL, "missing-parent: load config");
tool_file_set_config(cfg);
t = tool_file_get();
snprintf(nested, sizeof(nested), "%s/missing_dir/notes.md", tmpdir);
snprintf(args, sizeof(args),
"{\"operation\":\"write_file\",\"path\":\"%s\",\"content\":\"PWNED\"}", nested);
r = t->execute(args, buf, sizeof(buf));
MU_ASSERT(r == -1, "write with missing parent is rejected");
MU_ASSERT(slurp_file(victim, kept, sizeof(kept)) == 0, "workspace notes.md still readable");
MU_ASSERT(strcmp(kept, "KEEP") == 0, "missing parent does not overwrite same basename");
config_free(cfg);
unlink(config_path);
unlink(victim);
rmdir(tmpdir);
}

int main(void)
{
MU_RUN(test_file_read_write_list);
MU_RUN(test_file_empty_workspace_denies_all);
MU_RUN(test_file_outside_workspace_rejected);
MU_RUN(test_path_traversal_rejected);
MU_RUN(test_symlink_escape_rejected);
MU_RUN(test_write_does_not_truncate_file_used_as_directory);
MU_RUN(test_write_does_not_collapse_missing_parent_onto_basename);
printf("%d tests run, %d failed\n", tests_run, tests_failed);
return tests_failed ? 1 : 0;
}
Loading