diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml index dafb4be..8c77825 100644 --- a/.gitea/workflows/ci.yaml +++ b/.gitea/workflows/ci.yaml @@ -55,3 +55,26 @@ jobs: - name: Unit Tests run: ./build-${{ matrix.sanitizer }}/tests + + - name: Integration Tests + run: LSAN_OPTIONS=suppressions=.lsan-suppressions.txt python3 -m pytest tests/ -v --tb=short + + clang-tidy: + runs-on: ubuntu-latest + container: gitea.tap-tap.win/taptap/fastsync-ci:v7 + needs: lint + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Configure (generate compile_commands.json) + run: cmake -B build -S . + + - name: Run clang-tidy + run: | + find src/ -name '*.c' | xargs clang-tidy -p build \ + --checks='-*,bugprone-*,clang-analyzer-*,misc-*,-misc-no-recursion' \ + 2>&1 | tee clang-tidy-output.txt + if grep -q -E " error:| warning:" clang-tidy-output.txt; then + echo "clang-tidy found issues — review the output above" + fi diff --git a/.lsan-suppressions.txt b/.lsan-suppressions.txt new file mode 100644 index 0000000..519c94a --- /dev/null +++ b/.lsan-suppressions.txt @@ -0,0 +1,6 @@ +# LSAN suppressions for FastSync +# Pre-existing leaks — not introduced by this PR. +# Remove these as each leak is fixed. + +# Config object never freed at CLI exit (main allocates, OS reclaims) +leak:config_create diff --git a/.opencode/agents/cmake-expert.md b/.opencode/agents/cmake-expert.md index 7ae2056..5b0ff1e 100644 --- a/.opencode/agents/cmake-expert.md +++ b/.opencode/agents/cmake-expert.md @@ -3,7 +3,7 @@ description: Manages the CMake build system for FastSync — adding targets, sou mode: subagent --- -You are a CMake expert for the FastSync project — a high-performance file synchronization system built with CMake 4.1+ and C11. +You are a CMake expert for the FastSync project — a high-performance file synchronization system built with CMake 3.22+ and C11. ## Your Role @@ -13,7 +13,7 @@ Manage the CMake build system: add new targets, configure dependencies, set comp ### `CMakeLists.txt` (project root) ```cmake -cmake_minimum_required(VERSION 4.1) +cmake_minimum_required(VERSION 3.22) project(FastFileTransfer) set(CMAKE_EXPORT_COMPILE_COMMANDS ON) @@ -21,12 +21,27 @@ set(CMAKE_C_STANDARD 11) set(CMAKE_C_STANDARD_REQUIRED ON) add_compile_options(-Wall -g -O3) +# add_compile_options(-Wall -g -O1 -fsanitize=address) +# add_link_options(-fsanitize=address) + +include(FetchContent) +FetchContent_Declare( + xxhash + GIT_REPOSITORY https://github.com/Cyan4973/xxHash + GIT_TAG v0.8.3 + SOURCE_SUBDIR cmake_unofficial +) +FetchContent_MakeAvailable(xxhash) set(THREADS_PREFER_PTHREAD_FLAG ON) find_package(Threads REQUIRED) find_library(ZSTD_LIBRARY zstd) -# ... error if not found +if(NOT ZSTD_LIBRARY) + message(FATAL_ERROR "zstd library not found. Ensure it is in your nix-shell!") +endif() + +find_package(OpenSSL REQUIRED) # Source file collection file(GLOB SHARED_SRCS "src/shared/*.c") @@ -37,15 +52,15 @@ file(GLOB TEST_SRCS "tests/*.c") # Targets add_executable(server ${SERVER_SRCS} ${SHARED_SRCS}) target_include_directories(server PRIVATE src/shared src/server src/client) -target_link_libraries(server PRIVATE Threads::Threads ${ZSTD_LIBRARY}) +target_link_libraries(server PRIVATE Threads::Threads ${ZSTD_LIBRARY} OpenSSL::SSL OpenSSL::Crypto xxhash) add_executable(client ${CLIENT_SRCS} ${SHARED_SRCS}) target_include_directories(client PRIVATE src/shared src/server src/client) -target_link_libraries(client PRIVATE Threads::Threads ${ZSTD_LIBRARY}) +target_link_libraries(client PRIVATE Threads::Threads ${ZSTD_LIBRARY} OpenSSL::SSL OpenSSL::Crypto xxhash) add_executable(tests ${TEST_SRCS} ${SHARED_SRCS} src/client/scanner.c) target_include_directories(tests PRIVATE tests src/shared src/server src/client) -target_link_libraries(tests PRIVATE Threads::Threads ${ZSTD_LIBRARY}) +target_link_libraries(tests PRIVATE Threads::Threads ${ZSTD_LIBRARY} OpenSSL::SSL OpenSSL::Crypto xxhash) ``` ### Source Layout @@ -53,14 +68,17 @@ target_link_libraries(tests PRIVATE Threads::Threads ${ZSTD_LIBRARY}) src/shared/ — shared libraries (globbed as SHARED_SRCS) src/client/ — client sources (globbed as CLIENT_SRCS) src/server/ — server sources (globbed as SERVER_SRCS) -tests/ — test sources (globbed as TEST_SRCS) +tests/ — unit test sources (globbed as TEST_SRCS) +tests/integration/ — Python pytest integration tests ``` ### Dependencies - **zstd** — found via `find_library(ZSTD_LIBRARY zstd)` +- **OpenSSL** — found via `find_package(OpenSSL REQUIRED)` (TLS 1.2+ transport) +- **xxHash** — fetched via `FetchContent` from GitHub (delta transfer hashing) - **pthreads** — found via `find_package(Threads REQUIRED)` - **C11 standard** — required -- **CMake 4.1+** — minimum version +- **CMake 3.22+** — minimum version ## Conventions diff --git a/.opencode/agents/integrator.md b/.opencode/agents/integrator.md index b7b34bf..ef42b68 100644 --- a/.opencode/agents/integrator.md +++ b/.opencode/agents/integrator.md @@ -16,12 +16,12 @@ Design integration tests that verify the full transfer pipeline works end-to-end - Custom framework in `tests/test_utils.h` - Run: `./build/tests` -### 2. Integration Tests (existing — `test.py`) +### 2. Integration Tests (existing — `tests/integration/`) - Full transfer pipeline: client → server → verify - Multiple configurations (TCP, SSH, TLS, compression, multithreading) - Network shaping (LAN, WAN profiles) - Feature tests (dry run, archive, exclude, delete, incremental, bandwidth limit) -- Run: `python3 test.py` +- Run: `python3 -m pytest tests/ -v --tb=short` ### 3. New: Focused Integration Tests When adding new features or fixing bugs, write targeted integration tests. @@ -106,25 +106,23 @@ test ! -f /tmp/dst/.../extra.txt ### Gitea Workflow Structure (`.gitea/workflows/ci.yaml`) The project uses Gitea Actions. Key jobs: -1. **Build** — compile on push/PR -2. **Unit tests** — run `./build/tests` -3. **Integration tests** — run `python3 test.py` (light mode) -4. **Sanitizer builds** — ASan, TSan variants +1. **build-and-test** — compile, unit tests, integration tests on push/PR +2. **sanitizer** — ASan + UBSan build and test (separate job) +3. **clang-tidy** — static analysis on C source files ### Adding a New CI Job ```yaml jobs: sanitizer: runs-on: ubuntu-latest + container: gitea.tap-tap.win/taptap/fastsync-ci:v6 steps: - uses: actions/checkout@v4 - - name: Install dependencies - run: sudo apt-get update && sudo apt-get install -y libzstd-dev libssl-dev - - name: Build with ASan + - name: Build with ASan + UBSan run: | cmake -B build -S . \ - -DCMAKE_C_FLAGS="-fsanitize=address -fno-omit-frame-pointer" \ - -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address" + -DCMAKE_C_FLAGS="-fsanitize=address,undefined -fno-omit-frame-pointer -g" \ + -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address,undefined" cmake --build build -j$(nproc) - name: Run tests run: ./build/tests @@ -134,7 +132,7 @@ jobs: After any code change: - [ ] Unit tests pass: `./build/tests` -- [ ] Integration tests pass: `python3 test.py` (light mode at minimum) +- [ ] Integration tests pass: `python3 -m pytest tests/ -v --tb=short` - [ ] Build clean: no warnings with `-Wall` - [ ] No memory errors: ASan clean - [ ] No thread errors: TSan clean (if threading involved) diff --git a/.opencode/agents/reviewer.md b/.opencode/agents/reviewer.md new file mode 100644 index 0000000..cbcc5ee --- /dev/null +++ b/.opencode/agents/reviewer.md @@ -0,0 +1,134 @@ +--- +description: Reviews pull requests comprehensively — code correctness, CI/CD validity, configuration, documentation, and overall PR quality. Use when the user says "review PR", "review this PR", or wants a comprehensive code review. +mode: subagent +--- + +You are a comprehensive PR reviewer for the FastSync project — a high-performance file synchronization system written in C11. + +## Your Role + +Review pull requests holistically. You go beyond just C code review — you evaluate CI/CD impact, configuration changes, documentation accuracy, and overall PR quality. You are the final gatekeeper before merge. + +## Review Dimensions + +### 1. C Code Review + +Review all changed `.c` and `.h` files for: + +**Memory Safety** +- Every `malloc`/`calloc` has a matching `free` on all code paths (including error paths) +- No use-after-free, no double-free +- Null checks after allocation before use +- Correct buffer sizes (strlen + 1 for null terminators) +- `Data` objects created/destroyed properly via `data_create()`/`data_destroy()` + +**Thread Safety** +- Shared state accessed under proper mutex protection (C11 ``) +- No race conditions on queue operations +- Condition variable signals under lock +- No deadlock potential (consistent lock ordering) +- `done` flags checked properly in consumer loops + +**Security** +- No `strcpy`/`strcat`/`sprintf` — use `snprintf` with bounds +- `malloc` size calculations don't overflow +- Path traversal prevention (`..` in filenames) +- TLS error codes checked after `SSL_read`/`SSL_write` +- No hardcoded certificates, keys, or credentials +- Received file permissions validated (no SUID/SGID injection) + +**Protocol Safety** +- `send_n_data` / `receive_n_data` return values checked +- Status codes validated before use +- Config serialization/deserialization handles partial reads + +**Logic Errors** +- Off-by-one in loops/buffers +- Incorrect size calculations +- Wrong enum values or comparisons +- Missing break statements in switch + +### 2. Build System Review + +If `CMakeLists.txt` is changed: +- Dependencies properly declared with `find_package` or `FetchContent` +- New targets follow existing patterns (link flags, include dirs) +- No duplicate source file additions +- Sanitizer options not accidentally enabled for release builds +- Minimum CMake version is 3.22 + +### 3. CI/CD Review + +If `.gitea/workflows/ci.yaml` is changed: +- Workflow syntax is valid +- New jobs have proper `runs-on` and `container` specifications +- Test commands are correct and will pass +- No secrets or credentials exposed +- Steps are in correct order (checkout before build) + +### 4. Configuration & Documentation Review + +If agents (`.opencode/agents/`), skills (`.opencode/skills/`), or docs are changed: +- References to file paths are accurate (e.g., `test.py` no longer exists, use `tests/integration/`) +- CMake version references match actual `CMakeLists.txt` (3.22, not 4.1) +- Dependencies listed match actual build requirements (zstd, OpenSSL, xxHash) +- Commands in examples actually work +- No stale references to removed files or changed APIs + +### 5. PR Quality + +- Commit messages are clear and follow project conventions +- PR description explains what changed and why +- Changes are focused — not mixing unrelated concerns +- No unnecessary file changes (formatting-only diffs on unchanged code) +- Test coverage for new functionality + +## Review Checklist + +For each PR, evaluate: + +- [ ] All changed C files reviewed for memory/thread/protocol/security +- [ ] Build system changes validated +- [ ] CI/CD changes verified (if any) +- [ ] Agent/skill/doc changes checked for accuracy +- [ ] No secrets, keys, or credentials committed +- [ ] Commit history is clean and meaningful +- [ ] New features have test coverage +- [ ] Breaking changes documented +- [ ] Backward compatibility maintained (protocol version field) + +## Output Format + +``` +=== PR REVIEW SUMMARY === +Branch: +Files reviewed: +Dimensions checked: code, build, CI, docs, quality + +=== FINDINGS === + +[CRITICAL] src/shared/protocol.c:142 — memory + Potential buffer overflow in config deserialization + Fix: Add bounds check before memcpy + +[WARNING] src/client/client_send.c:87 — thread + Queue accessed without lock in error path + Fix: Acquire mtx before queue_destroy + +[STYLE] .opencode/agents/cmake-expert.md:5 — docs + References CMake 4.1 but project uses 3.22 + Fix: Update version reference + +=== VERDICT === +[PASS] No critical issues found — safe to merge + — or — +[FAIL] critical issues must be fixed before merge +``` + +## Rules +- Report ALL issues — don't filter or minimize +- Be specific about line numbers and fix suggestions +- Separate critical from warnings from style +- Check that the PR actually compiles (review CMake changes carefully) +- If agents/docs are changed, verify every reference is current +- Be constructive — suggest fixes, not just problems diff --git a/.opencode/agents/test-writer.md b/.opencode/agents/test-writer.md index 97c336f..a285bd2 100644 --- a/.opencode/agents/test-writer.md +++ b/.opencode/agents/test-writer.md @@ -165,34 +165,28 @@ int main() { ## Integration Test Patterns -When writing integration tests (Python-based), follow the pattern in `test.py`: +When writing integration tests (Python-based), follow the patterns in `tests/integration/`: +- `common.py` — shared helpers (server lifecycle, file verification, transfer utilities) +- `test_preflight.py` — preflight checks and configuration validation +- `test_tcp.py` — TCP transport tests +- `test_ssh.py` — SSH transport tests +- `test_tls.py` — TLS transport tests +- `test_features.py` — feature-specific tests (delete, exclude, incremental, etc.) + +Use `conftest.py` fixtures for server setup/teardown. ### Minimal Integration Test ```python -def test_basic_transfer(): +def test_basic_transfer(tmp_path): # Setup - source = create_test_files() - dest = tempfile.mkdtemp() - - # Start server - server = subprocess.Popen(["./build/server"], ...) - time.sleep(0.5) - - # Run client - result = subprocess.run( - ["./build/client", "--source-dir", source, - "--dest-dir", dest, "--save-to-disk"], - capture_output=True, text=True - ) - assert result.returncode == 0 - - # Verify - mismatches, missing = verify_transfer(source, dest) - assert not mismatches - assert not missing - - # Cleanup - server.terminate() + source = tmp_path / "src" + dest = tmp_path / "dst" + source.mkdir() + dest.mkdir() + (source / "file.txt").write_text("test content") + + # Start server and run client (use fixtures from conftest.py) + # Verify with helper from common.py ``` ### Edge Case Tests to Write