From 690bf72d7f58e12ddba1fb59e2fed8ea208626f0 Mon Sep 17 00:00:00 2001 From: TapTap Date: Mon, 10 Aug 2026 19:10:02 +0200 Subject: [PATCH] fix: close remaining PR 208 review findings --- README.md | 6 +- src/client/scanner.c | 148 ++++++++++++++++++++++++++++++---- src/client/scanner.h | 2 + src/server/server.c | 34 ++++++-- src/shared/file.c | 61 +++++++++++++- src/shared/file.h | 1 + src/shared/metadata.c | 4 +- src/shared/multiprocessing.c | 11 +++ src/shared/utils.c | 10 ++- src/shared/utils.h | 1 + tests/integration/test_ssh.py | 36 +++++---- 11 files changed, 268 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index f692060..04bc1df 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ A high-performance file synchronization system with SSH and TCP transport, TLS e 5. **Multithreading**: producer-consumer pipeline with thread-safe queues (scanner → loader → sender) 6. **Incremental sync**: skip files unchanged since last transfer (compares size + mtime) 7. **Batch incremental**: send incremental checks in batched groups for reduced round-trips -8. **Metadata preservation**: `mode`, `uid`, `gid`, `mtime` restored on disk when enabled +8. **Metadata preservation**: file mode and mtime are restored when enabled; ownership and atime are intentionally not restored 9. **`sendfile()` zero-copy** on TCP (~2× faster on loopback) 10. **SSH ControlMaster** for connection reuse across repeated invocations 11. **Bandwidth limiting**: token-bucket throttling (`--bwlimit`) @@ -112,7 +112,7 @@ Config negotiation is sender-driven: the client serializes transfer options and | `-m` | Multithreading mode | | `-s` | Chunk serialization (batch all files per chunk) | | `-f, --sendfile` | Sendfile zero-copy. Incompatible with `-c` / `-s`. TCP only. | -| `-M, --preserve` | Preserve file metadata (mode, uid, gid, mtime) | +| `-M, --preserve` | Preserve supported file metadata (mode and mtime; ownership and atime are unsupported) | | `-n, --dry-run` | Scan and print what would be transferred | | `-p ` | SSH port (default: 22) | | `-v, --verbose` | Enable debug logging | @@ -178,7 +178,7 @@ Config negotiation is sender-driven: the client serializes transfer options and ### Data Structures 1. **Chunk** — collection of files (~10 MB total by default) 2. **File** — path, content (`Data`), optional `FileMetadata` pointer -3. **FileMetadata** — `mode`, `uid`, `gid`, `mtime_sec`, `mtime_nsec` +3. **FileMetadata** — `mode`, `uid`, `gid`, `mtime_sec`, `mtime_nsec`; uid/gid are advisory wire fields and are never applied by the receiver; atime is unsupported 4. **Config** — runtime parameters (transported over wire, TLS settings excluded). Includes `timeout`, `contimeout`, `quiet`, `backup`, `backup_dir`, `stats`, `max_depth`, `log_file`, `queue_size`. 5. **Queue** — thread-safe bounded queue with condition variables 6. **DirectoryScanner** — recursive BFS traversal with exclude and include pattern support, max-depth enforcement diff --git a/src/client/scanner.c b/src/client/scanner.c index aef3fef..a2ea713 100644 --- a/src/client/scanner.c +++ b/src/client/scanner.c @@ -11,6 +11,7 @@ #include #include #include +#include typedef struct { char* path; @@ -38,6 +39,19 @@ static DirEntry* dir_entry_create(const char* path, int depth) { return de; } +static bool safe_relative_link(const char* source_root, const char* containing_dir, + const char* link_target) { + char root[PATH_MAX]; + if (!realpath(source_root, root)) + return false; + char* joined = path_cat(containing_dir, link_target); + char resolved[PATH_MAX]; + bool safe = joined && realpath(joined, resolved) && strncmp(root, resolved, strlen(root)) == 0 && + (resolved[strlen(root)] == '\0' || resolved[strlen(root)] == '/'); + free(joined); + return safe; +} + DirectoryScanner* directory_scanner_create(const char* root_directory, bool use_metadata, unsigned long long chunk_size, char** exclude_patterns, int exclude_count, char** include_patterns, @@ -45,10 +59,14 @@ DirectoryScanner* directory_scanner_create(const char* root_directory, bool use_ unsigned long long min_size, int max_depth, bool follow_symlinks, bool copy_links, bool safe_links, bool copy_unsafe_links, bool checksum) { - DirectoryScanner* scanner = malloc(sizeof(DirectoryScanner)); + DirectoryScanner* scanner = calloc(1, sizeof(DirectoryScanner)); if (scanner == NULL) return NULL; scanner->directories = queue_create(100, dir_entry_destroy); + if (!scanner->directories) { + free(scanner); + return NULL; + } scanner->current_dir = NULL; scanner->current_path = NULL; scanner->use_metadata = use_metadata; @@ -66,6 +84,7 @@ DirectoryScanner* directory_scanner_create(const char* root_directory, bool use_ scanner->safe_links = safe_links; scanner->copy_unsafe_links = copy_unsafe_links; scanner->checksum = checksum; + scanner->failed = false; DirEntry* root = dir_entry_create(root_directory, 0); if (!root) { queue_destroy(scanner->directories); @@ -95,8 +114,12 @@ void directory_scanner_destroy(DirectoryScanner* scanner) { static Chunk* chunk_data_to_chunk(ArrayList* chunk_data) { void** chunk_items = array_list_to_array(chunk_data); + if (!chunk_items) + return NULL; Chunk* chunk = chunk_create((File**)chunk_items, chunk_data->size); free(chunk_items); + if (!chunk) + return NULL; chunk_data->item_destroyer = NULL; array_list_delete(chunk_data); return chunk; @@ -157,6 +180,10 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { continue; char* cur_path = path_cat(scanner->current_path, entry->d_name); + if (!cur_path) { + scanner->failed = true; + break; + } struct stat stats; struct stat lstats; bool is_symlink = false; @@ -180,7 +207,8 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { continue; } link_target[len] = '\0'; - if (link_target[0] == '/') { + if (link_target[0] == '/' || + !safe_relative_link(scanner->current_path, scanner->current_path, link_target)) { free(cur_path); continue; } @@ -215,8 +243,10 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { int next_depth = scanner->current_depth + 1; if (scanner->max_depth <= 0 || next_depth < scanner->max_depth) { DirEntry* de = dir_entry_create(cur_path, next_depth); - if (!queue_enqueue(scanner->directories, de)) + if (!de || !queue_enqueue(scanner->directories, de)) { dir_entry_destroy(de); + scanner->failed = true; + } } free(cur_path); } else { @@ -259,11 +289,18 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { File* file = file_create(cur_path); if (file == NULL) { free(cur_path); + scanner->failed = true; continue; } file->data->size = stats.st_size; if (scanner->use_metadata) file->metadata = file_metadata_create(&stats); + if (scanner->use_metadata && !file->metadata) { + file_destroy(file); + free(cur_path); + scanner->failed = true; + break; + } if (!array_list_add(chunk_data, file)) { file_destroy(file); scanner->failed = true; @@ -272,14 +309,21 @@ Chunk* directory_scanner_next(DirectoryScanner* scanner) { chunk_data_size += file->data->size; if (chunk_data_size > scanner->chunk_size) { free(cur_path); - return chunk_data_to_chunk(chunk_data); + Chunk* result = chunk_data_to_chunk(chunk_data); + if (!result) + scanner->failed = true; + return result; } free(cur_path); } } - if (chunk_data->size > 0) - return chunk_data_to_chunk(chunk_data); + if (chunk_data->size > 0) { + Chunk* result = chunk_data_to_chunk(chunk_data); + if (!result) + scanner->failed = true; + return result; + } array_list_delete(chunk_data); return NULL; } @@ -349,7 +393,7 @@ static int parallel_worker_thread(void* arg) { free(wa); mtx_lock(&ps->result_mutex); ps->completed++; - if (ps->completed >= ps->num_threads) { + if (ps->completed >= ps->expected_threads) { ps->done = true; cnd_signal(&ps->result_not_empty); } @@ -409,6 +453,13 @@ ParallelScanner* parallel_scanner_create(char* root_directory, bool use_metadata ArrayList* root_files = array_list_create(file_destroy); ArrayList* subdirs = array_list_create(free); + if (!root_files || !subdirs) { + array_list_delete(root_files); + array_list_delete(subdirs); + closedir(dir); + parallel_scanner_destroy(ps); + return NULL; + } struct dirent* entry; while ((entry = readdir(dir)) != NULL) { if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) @@ -438,7 +489,8 @@ ParallelScanner* parallel_scanner_create(char* root_directory, bool use_metadata continue; } link_target[len] = 0; - if (link_target[0] == '/') { + if (link_target[0] == '/' || + !safe_relative_link(root_directory, root_directory, link_target)) { free(cur_path); continue; } @@ -473,7 +525,10 @@ ParallelScanner* parallel_scanner_create(char* root_directory, bool use_metadata } if (S_ISDIR(st.st_mode)) { - array_list_add(subdirs, cur_path); + if (!array_list_add(subdirs, cur_path)) { + free(cur_path); + ps->failed = true; + } } else { bool excluded = false; for (int i = 0; i < exclude_count; i++) { @@ -506,12 +561,22 @@ ParallelScanner* parallel_scanner_create(char* root_directory, bool use_metadata } File* file = file_create(cur_path); free(cur_path); - if (!file) + if (!file) { + ps->failed = true; continue; + } file->data->size = st.st_size; if (use_metadata) file->metadata = file_metadata_create(&st); - array_list_add(root_files, file); + if (use_metadata && !file->metadata) { + file_destroy(file); + ps->failed = true; + continue; + } + if (!array_list_add(root_files, file)) { + file_destroy(file); + ps->failed = true; + } } } closedir(dir); @@ -519,16 +584,40 @@ ParallelScanner* parallel_scanner_create(char* root_directory, bool use_metadata unsigned long long cs = chunk_size > 0 ? chunk_size : DESIRED_CHUNK_SIZE; if (root_files->size > 0) { ArrayList* batch = array_list_create(NULL); + if (!batch) { + ps->failed = true; + array_list_delete(root_files); + array_list_delete(subdirs); + parallel_scanner_destroy(ps); + return NULL; + } unsigned long long batch_size = 0; Chunk* first = NULL; for (int i = 0; i < root_files->size; i++) { File* f = (File*)root_files->items[i]; - array_list_add(batch, f); + if (!array_list_add(batch, f)) { + ps->failed = true; + break; + } batch_size += f->data->size; if (batch_size >= cs || i == root_files->size - 1) { void** items = array_list_to_array(batch); + if (!items) { + ps->failed = true; + batch->item_destroyer = file_destroy; + array_list_delete(batch); + batch = NULL; + break; + } Chunk* c = chunk_create((File**)items, batch->size); free(items); + if (!c) { + ps->failed = true; + batch->item_destroyer = file_destroy; + array_list_delete(batch); + batch = NULL; + break; + } batch->item_destroyer = NULL; array_list_delete(batch); batch = NULL; @@ -542,6 +631,10 @@ ParallelScanner* parallel_scanner_create(char* root_directory, bool use_metadata } if (i < root_files->size - 1) { batch = array_list_create(NULL); + if (!batch) { + ps->failed = true; + break; + } batch_size = 0; } } @@ -561,6 +654,7 @@ ParallelScanner* parallel_scanner_create(char* root_directory, bool use_metadata if (subdirs->size > 0) { ps->num_threads = n; + ps->expected_threads = n; ps->threads = calloc(n, sizeof(thrd_t)); if (!ps->threads) { array_list_delete(subdirs); @@ -570,21 +664,37 @@ ParallelScanner* parallel_scanner_create(char* root_directory, bool use_metadata int dirs_per_thread = subdirs->size / n; int remainder = subdirs->size % n; int start = 0; + ps->num_threads = 0; for (int t = 0; t < n; t++) { int count = dirs_per_thread + (t < remainder ? 1 : 0); if (count == 0) break; ParallelWorkerArg* wa = calloc(1, sizeof(ParallelWorkerArg)); - if (!wa) + if (!wa) { + ps->failed = true; break; + } wa->ps = ps; wa->dirs = calloc(count, sizeof(char*)); if (!wa->dirs) { free(wa); + ps->failed = true; break; } - for (int j = 0; j < count; j++) + bool dup_ok = true; + for (int j = 0; j < count; j++) { wa->dirs[j] = str_dup((char*)subdirs->items[start + j]); + if (!wa->dirs[j]) + dup_ok = false; + } + if (!dup_ok) { + for (int j = 0; j < count; j++) + free(wa->dirs[j]); + free(wa->dirs); + free(wa); + ps->failed = true; + break; + } wa->dir_count = count; wa->use_metadata = use_metadata; wa->chunk_size = cs; @@ -606,9 +716,17 @@ ParallelScanner* parallel_scanner_create(char* root_directory, bool use_metadata free(wa->dirs[j]); free(wa->dirs); free(wa); - ps->num_threads = t; + ps->failed = true; + atomic_store(&ps->cancelled, true); + ps->expected_threads = ps->created_threads; + mtx_lock(&ps->result_mutex); + cnd_broadcast(&ps->result_not_empty); + cnd_broadcast(&ps->result_not_full); + mtx_unlock(&ps->result_mutex); break; } + ps->num_threads++; + ps->created_threads++; } } array_list_delete(subdirs); diff --git a/src/client/scanner.h b/src/client/scanner.h index 982b3d2..0bff53c 100644 --- a/src/client/scanner.h +++ b/src/client/scanner.h @@ -36,6 +36,8 @@ typedef struct { cnd_t result_not_empty; cnd_t result_not_full; int num_threads; + int expected_threads; + int created_threads; thrd_t* threads; bool done; bool failed; diff --git a/src/server/server.c b/src/server/server.c index 2cc98f0..ee34957 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -16,10 +16,12 @@ #include #include #include +#include #include #include static char* authorized_root; +static int authorized_root_fd = -1; static bool allow_delete; static bool path_is_within(const char* root, const char* path) { @@ -27,12 +29,27 @@ static bool path_is_within(const char* root, const char* path) { return strncmp(root, path, n) == 0 && (path[n] == '\0' || path[n] == '/'); } +static bool valid_batch_path(const char* path) { + return path && path[0] != '\0' && path[0] != '/' && !has_path_traversal(path) && + strchr(path, '\0') == path + strlen(path); +} + static bool __attribute__((unused)) configure_authorization(const char* root) { char resolved[PATH_MAX]; if (!root || !realpath(root, resolved)) return false; authorized_root = str_dup(resolved); - return authorized_root != NULL; + if (!authorized_root) + return false; + authorized_root_fd = open(resolved, O_RDONLY | O_DIRECTORY | O_CLOEXEC); + if (authorized_root_fd < 0) { + free(authorized_root); + authorized_root = NULL; + return false; + } + file_set_authorized_root(authorized_root_fd, authorized_root); + utils_set_authorized_root_fd(authorized_root_fd); + return true; } int receive_files(Config* config, int fd) { @@ -96,17 +113,21 @@ int receive_files(Config* config, int fd) { free(check_path); return -1; } + if (!valid_batch_path(check_path)) { + free(check_path); + send_status(fd, STATUS_ERROR); + return -1; + } char* full_path = path_cat(config->receive_root_directory, check_path); struct stat st; bool has_old = full_path && lstat(full_path, &st) == 0; bool match = has_old && (unsigned long long)st.st_size == check_size && (long long)st.st_mtime == check_mtime; - if (match) - send_status(fd, STATUS_OK); - else - send_status(fd, STATUS_NEXT); + bool sent = send_status(fd, match ? STATUS_OK : STATUS_NEXT); free(full_path); free(check_path); + if (!sent) + return -1; } goto next; } else { @@ -324,6 +345,9 @@ int main(int argc, char* argv[]) { if (stdio_mode) { io_set_fds(STDIN_FILENO, STDOUT_FILENO); handler(STDIN_FILENO); + file_set_authorized_root(-1, NULL); + utils_set_authorized_root_fd(-1); + close(authorized_root_fd); free(authorized_root); return 0; } diff --git a/src/shared/file.c b/src/shared/file.c index de330e4..b333ce1 100644 --- a/src/shared/file.c +++ b/src/shared/file.c @@ -37,6 +37,8 @@ bool file_checksum(File* file, uint64_t* checksum) { } File* file_create(const char* path) { + if (!path) + return NULL; File* file = (File*)malloc(sizeof(File)); if (file == NULL) { perror("ERROR: Could not allocate memory for file struct"); @@ -102,6 +104,8 @@ bool file_load_data(File* file) { if (file == NULL) return false; if (file->data->data == NULL) { + if (file->data->size == 0) + return true; file->data->data = malloc(file->data->size); if (file->data->data == NULL) { perror("Could not allocate memory for file data"); @@ -121,6 +125,8 @@ bool file_load_data(File* file) { bool file_send_single_calls(File* file, int file_descriptor, bool use_metadata, int compression_level, bool send_path) { + if (!file || !file->path || !file->data) + return false; const Data* data_to_send = file->data; Data* compressed_data = NULL; if (compression_level > 0 && !compression_should_skip(file->path)) { @@ -151,6 +157,14 @@ static bool to_disk_secure(const char* path, const void* data, unsigned long lon bool inplace, bool sparse, const FileMetadata* metadata); static int open_secure_parent(const char* path, char** leaf_out); static bool rename_secure(const char* old_path, const char* new_path); +static int authorized_root_fd = -1; +static char* authorized_root_path; + +void file_set_authorized_root(int fd, const char* canonical_path) { + authorized_root_fd = fd; + free(authorized_root_path); + authorized_root_path = canonical_path ? str_dup(canonical_path) : NULL; +} static bool path_is_within_root(const char* root, const char* path) { size_t n = strlen(root); @@ -648,8 +662,25 @@ static int open_secure_parent(const char* path, char** leaf_out) { free(copy); return -1; } - int fd = (parent[0] == '/') ? open("/", O_RDONLY | O_DIRECTORY | O_CLOEXEC) - : open(".", O_RDONLY | O_DIRECTORY | O_CLOEXEC); + int fd; + if (authorized_root_fd >= 0 && authorized_root_path && path[0] == '/' && + path_is_within_root(authorized_root_path, path)) { + fd = dup(authorized_root_fd); + size_t root_len = strlen(authorized_root_path); + char* relative = str_dup(path + root_len); + if (!relative) { + free(copy); + free(leaf); + close(fd); + return -1; + } + free(copy); + copy = relative; + parent = dirname(copy); + } else { + fd = (parent[0] == '/') ? open("/", O_RDONLY | O_DIRECTORY | O_CLOEXEC) + : open(".", O_RDONLY | O_DIRECTORY | O_CLOEXEC); + } if (fd < 0) { free(copy); free(leaf); @@ -861,6 +892,8 @@ done: bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int compression_level, bool send_path) { + if (!file || !file->path || !file->data) + return false; if (compression_level > 0) return file_send_single_calls(file, file_descriptor, use_metadata, compression_level, send_path); @@ -877,6 +910,12 @@ bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int } unsigned long long file_size = file->data->size; + struct stat source_stat; + if (fstat(fd, &source_stat) != 0 || !S_ISREG(source_stat.st_mode) || + (unsigned long long)source_stat.st_size < file_size) { + close(fd); + return false; + } if (!send_n_data(file_descriptor, &file_size, sizeof(unsigned long long))) { close(fd); return false; @@ -885,8 +924,18 @@ bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int /* sendfile cannot encrypt TLS records. Keep the framing identical but route encrypted transfers through the deadline-aware IO layer. */ if (io_get_ssl() != NULL) { - bool loaded = file->data->data != NULL || file_load_data(file); - bool ok = loaded && send_n_data(file_descriptor, file->data->data, (size_t)file_size); + unsigned char buffer[64 * 1024]; + unsigned long long remaining = file_size; + bool ok = true; + while (remaining > 0) { + size_t want = remaining > sizeof(buffer) ? sizeof(buffer) : (size_t)remaining; + ssize_t got = read(fd, buffer, want); + if (got <= 0 || !send_n_data(file_descriptor, buffer, (size_t)got)) { + ok = false; + break; + } + remaining -= (unsigned long long)got; + } close(fd); return ok; } @@ -919,6 +968,10 @@ bool file_send_sendfile(File* file, int file_descriptor, bool use_metadata, int close(fd); return false; } + if (sent == 0) { + close(fd); + return false; + } } close(fd); diff --git a/src/shared/file.h b/src/shared/file.h index eb280c3..e763f91 100644 --- a/src/shared/file.h +++ b/src/shared/file.h @@ -38,6 +38,7 @@ void file_metadata_destroy(void* metadata); bool to_disk(const char* path, const void* data, unsigned long long data_size, bool inplace, bool sparse); bool file_save_to_disk(const char* root_directory, const File* file, const Config* config); +void file_set_authorized_root(int fd, const char* canonical_path); File* receive_incremental_check(int fd, const Config* config, bool* skipped); int receive_manifest(int fd, const Config* config, int* next_status); diff --git a/src/shared/metadata.c b/src/shared/metadata.c index 98da17e..9260591 100644 --- a/src/shared/metadata.c +++ b/src/shared/metadata.c @@ -177,8 +177,8 @@ void file_restore_metadata(const char* path, FileMetadata* metadata) { return; if (chmod(path, metadata->mode & 07777 & ~(S_ISUID | S_ISGID)) != 0) log_message(LOG_LEVEL_WARNING, "Failed to chmod %s: %s", path, strerror(errno)); - if (chown(path, metadata->uid, metadata->gid) != 0) - log_message(LOG_LEVEL_WARNING, "Failed to chown %s: %s", path, strerror(errno)); + /* Never apply client-supplied ownership. The descriptor API below is the + receiver write path; retain this legacy API only for compatibility. */ struct timespec times[2]; times[0].tv_sec = 0; times[0].tv_nsec = UTIME_OMIT; diff --git a/src/shared/multiprocessing.c b/src/shared/multiprocessing.c index 1a5b0d4..5edbec2 100644 --- a/src/shared/multiprocessing.c +++ b/src/shared/multiprocessing.c @@ -1,4 +1,5 @@ #include "multiprocessing.h" + #include "array_list.h" #include "chunk.h" #include "config.h" @@ -13,6 +14,10 @@ #include #include +static bool valid_batch_path(const char* path) { + return path && path[0] != '\0' && path[0] != '/' && !has_path_traversal(path); +} + PipelineContextSender* pipeline_context_sender_create(Config* config, Queue* queue_scanner, Queue* queue_loader) { PipelineContextSender* context = malloc(sizeof(PipelineContextSender)); @@ -221,6 +226,12 @@ int receive_thread(void* pipeline_context) { free(check_path); RECEIVE_THREAD_FAIL(); } + if (!valid_batch_path(check_path)) { + free(check_path); + if (!send_status(file_descriptor, STATUS_ERROR)) + RECEIVE_THREAD_FAIL(); + RECEIVE_THREAD_FAIL(); + } char* full_path = path_cat(config->receive_root_directory, check_path); struct stat st; bool has_old = full_path && lstat(full_path, &st) == 0; diff --git a/src/shared/utils.c b/src/shared/utils.c index c1cedd0..9e11988 100644 --- a/src/shared/utils.c +++ b/src/shared/utils.c @@ -10,6 +10,12 @@ #include #include +static int authorized_root_fd = -1; + +void utils_set_authorized_root_fd(int fd) { + authorized_root_fd = fd; +} + bool mkdir_r(const char* path) { size_t path_len = strlen(path); char* path_duplicate = malloc(path_len + 1); @@ -201,7 +207,9 @@ static bool delete_extras_fd(int dirfd, const char* rel_path, ArrayList* manifes } bool delete_extras(const char* dest_root, ArrayList* manifest) { - int rootfd = open(dest_root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); + int rootfd = authorized_root_fd >= 0 + ? dup(authorized_root_fd) + : open(dest_root, O_RDONLY | O_DIRECTORY | O_NOFOLLOW | O_CLOEXEC); if (rootfd < 0) return false; bool ok = delete_extras_fd(rootfd, "", manifest); diff --git a/src/shared/utils.h b/src/shared/utils.h index ce01d33..d04daf5 100644 --- a/src/shared/utils.h +++ b/src/shared/utils.h @@ -9,6 +9,7 @@ char* str_dup(const char* string); char* path_cat(const char* path1, const char* path2); bool glob_match(const char* pattern, const char* str); bool delete_extras(const char* dest_root, ArrayList* manifest); +void utils_set_authorized_root_fd(int fd); bool has_path_traversal(const char* path); #endif diff --git a/tests/integration/test_ssh.py b/tests/integration/test_ssh.py index fa1234d..7cf69b9 100644 --- a/tests/integration/test_ssh.py +++ b/tests/integration/test_ssh.py @@ -4,6 +4,9 @@ import shutil import subprocess import sys import pytest +import shlex +import tempfile +import shutil sys.path.insert(0, os.path.dirname(__file__)) from common import (PROJECT_ROOT, BUILD_DIR, TEST_DATA_DIR, CLIENT_CMD, @@ -14,37 +17,38 @@ SOURCE_DIR = os.path.join(TEST_DATA_DIR, "ssh_source") DEST_DIR = os.path.join(TEST_DATA_DIR, "ssh_dest") SSH_AVAILABLE = False SSH_SKIP_REASON = "SSH localhost probe was not run" +SSH_PROBE_DIR = None def _check_ssh(): - global SSH_AVAILABLE, SSH_SKIP_REASON + global SSH_AVAILABLE, SSH_SKIP_REASON, SSH_PROBE_DIR server_path = os.path.join(BUILD_DIR, "server") if not os.path.isfile(server_path): SSH_SKIP_REASON = f"current server binary is missing: {server_path}" return try: - path = subprocess.run(["ssh", "-o", "BatchMode=yes", "localhost", "echo", "$PATH"], + SSH_PROBE_DIR = tempfile.mkdtemp(prefix="fastsync-ssh-probe-") + probe_server = os.path.join(SSH_PROBE_DIR, "fastsync-server") + os.symlink(server_path, probe_server) + command = f"{shlex.quote(probe_server)} --help" + path = subprocess.run(["ssh", "-o", "BatchMode=yes", "-o", "ConnectTimeout=5", + "localhost", "sh", "-c", command], capture_output=True, timeout=10, text=True) if path.returncode != 0: - SSH_SKIP_REASON = "SSH to localhost is unavailable" + SSH_SKIP_REASON = "SSH to localhost is unavailable or current server probe failed" return - for directory in path.stdout.strip().split(":"): - if not directory or "wrappers" in directory: - continue - probe = subprocess.run( - ["ssh", "-o", "BatchMode=yes", "localhost", - f'test -w "{directory}" && ln -sf "{server_path}" ' - f'"{directory}/fastsync-server" && test -x "{directory}/fastsync-server" ' - f'&& "{directory}/fastsync-server" --help'], - capture_output=True, timeout=10) - if probe.returncode == 0 and b"FastSync Server" in probe.stdout: - SSH_AVAILABLE = True - return - SSH_SKIP_REASON = "SSH setup could not install and validate the current server binary" + if "FastSync Server" in path.stdout: + SSH_AVAILABLE = True + return + SSH_SKIP_REASON = "SSH probe did not execute the current server binary" except FileNotFoundError: SSH_SKIP_REASON = "ssh executable is unavailable" except (OSError, subprocess.TimeoutExpired) as exc: SSH_SKIP_REASON = f"SSH setup failed: {exc}" + finally: + if SSH_PROBE_DIR: + shutil.rmtree(SSH_PROBE_DIR, ignore_errors=True) + SSH_PROBE_DIR = None _check_ssh()