Conversation
Add GitHub Actions workflows adapted from ml-odyssey for C++ development: Workflows: - pre-commit.yml: C++ formatting (clang-format), linting (clang-tidy, cppcheck) - unit-tests.yml: Google Test execution with coverage reporting (gcovr) - build-validation.yml: Docker multi-stage builds and native CMake builds - integration-tests.yml: E2E tests for all 4 phases (L0-L3 hierarchy) - security-scan.yml: Gitleaks, Semgrep, CodeQL, dependency scanning Configuration: - CODEOWNERS: Code ownership rules for ProjectKeystone - dependabot.yml: Automated dependency updates for Actions and Docker - .pre-commit-config.yaml: Pre-commit hooks for C++/CMake/Docker - .clang-format: Google C++ Style with ProjectKeystone customizations - .yamllint.yaml: YAML linting configuration Key adaptations from ml-odyssey: - Replaced Pixi with Docker/CMake build system - Replaced Mojo tests with Google Test (C++) - Added C++-specific tools: clang-format, clang-tidy, cppcheck, CodeQL - Adapted security scanning for C++ dependencies - Added Docker-based test execution for consistency Documentation: - Comprehensive workflows/README.md with usage, troubleshooting, and maintenance All workflows support PR comments, artifact uploads, and graceful handling of missing tests during development phases.
Add comprehensive sanitizer support and update all documentation to use Catch2 v3 instead of Google Test. Sanitizers: - Add sanitizers.yml workflow with ASan, UBSan, TSan, MSan - ASan + UBSan run together (compatible) - TSan runs separately (incompatible with ASan) - MSan only runs on manual workflow dispatch - Sanitizer results uploaded as artifacts - PR comments with sanitizer status - Workflow fails if sanitizers detect issues Catch2 v3 Migration: - Update CLAUDE.md: Replace Google Test with Catch2 v3 - Update testing-strategy.md: All test examples use Catch2 syntax - TEST_CASE instead of TEST/TEST_F - SECTION for test subsections - REQUIRE/CHECK assertions - Fixtures using TEST_CASE_METHOD - BDD-style SCENARIO/GIVEN/WHEN/THEN examples - Update integration-tests.yml: Use Catch2 test filtering ([e2e] tags) - Update workflows README: Reference Catch2 v3 documentation Sanitizer Documentation: - Add detailed sanitizer section to testing-strategy.md - Document sanitizer options (ASAN_OPTIONS, TSAN_OPTIONS, etc.) - Add CMake build commands for each sanitizer - Document suppression files for false positives - Add CI integration notes - Update workflows README with sanitizer details - Add local testing instructions - Update PR timeline to include sanitizers (~25 min total) Testing Framework Updates: - All test naming conventions now use Catch2 tags - Test examples show Catch2 sections and fixtures - Documentation includes both TEST_CASE and SCENARIO styles - Unit test references updated from GTest to Catch2 - Integration test examples use Catch2 syntax - References section updated with Catch2 documentation link This ensures all runtime errors (memory, undefined behavior, data races) are caught during CI, while using the modern Catch2 v3 framework for better test readability and BDD-style testing.
mvillmow
deleted the
claude/add-github-workflows-01Sijee4T4cgCi4ZYTMLSk2c
branch
Security Scan Results
Recommendations
Workflow: Security Scanning |
Integration Test Results
Workflow: Integration Tests |
🔍 Sanitizer Results
Important Notes
Workflow: Sanitizers |
Build Validation ReportDocker Builds
Native Build
Workflow: Build Validation |
🧪 Unit Test ResultsUpdated: 2025-11-18T01:02:26.197Z |
mvillmow
added a commit
that referenced
this pull request
Nov 27, 2025
This PR fixes 9 security vulnerabilities identified during comprehensive code review: ## CRITICAL Vulnerabilities Fixed 1. **Use-after-free in ProfilingSession** (profiling.cpp:83-121) - Root cause: Map pointer captured before lock release, invalidated by rehashing - Fix: Hold shared_lock during entire section access to prevent map modifications - Impact: Prevents crash in multi-threaded profiling scenarios - File: src/core/profiling.cpp 2. **Integer overflow in LeadAgentBase** (lead_agent_base_impl.hpp:215-225) - Root cause: size_t to int cast without bounds checking (size_t can be 2^64-1, int max is 2^31-1) - Fix: Check subtasks.size() > INT_MAX before cast, return error if exceeded - Impact: Prevents overflow causing negative coordination count - File: include/agents/lead_agent_base_impl.hpp 3. **Null pointer dereference + TOCTOU race in PullOrSteal** (pull_or_steal.cpp:43-65) - Root cause: queues.size() captured before loop, vector could shrink; null pointers not checked - Fix: Capture num_workers once, add combined bounds+null check before access - Impact: Prevents segfault in work-stealing scheduler - File: src/concurrency/pull_or_steal.cpp ## HIGH Priority Vulnerabilities Fixed 4. **Agent ID space exhaustion** (agent_id_interning.cpp:30-36) - Root cause: uint32_t wraps to 0 after 4,294,967,295, causing ID collisions - Fix: Check next_id_ == UINT32_MAX before increment, throw overflow_error - Impact: Prevents silent ID collision catastrophic failure - File: src/core/agent_id_interning.cpp 5. **Configuration validation missing** (config.hpp:147-160) - Root cause: Float percentage truncated to size_t without validation - Fix: Add static_assert compile-time checks for watermark percentage and result - Impact: Catches invalid configuration at compile time - File: include/core/config.hpp ## MEDIUM Priority Vulnerabilities Fixed 6. **Memory cleanup failure** (metrics.cpp:46-70) - Root cause: Time-based cleanup might not remove enough entries under flood - Fix: Add forced removal of oldest entries if time-based cleanup insufficient - Impact: Prevents unbounded memory growth under message flood - File: src/core/metrics.cpp 7. **Modulo by zero** (pull_or_steal.cpp:43-49) - Root cause: num_workers could be 0, causing (worker_index + i) % 0 - Fix: Check num_workers == 0, return nullopt early - Impact: Prevents FPE crash (fixed with CRITICAL #3) - File: src/concurrency/pull_or_steal.cpp ## Testing - Added comprehensive security regression test suite (test_security_regression.cpp) - 10 new test cases covering all 7 vulnerabilities - Stress tests: 100 threads × 100 sections × 10 records (ProfilingSession) - Memory flood test: 20,000 messages (2x limit) for metrics cleanup - All 476 tests pass with AddressSanitizer + UndefinedBehaviorSanitizer - Test runtime: 66.33 seconds ## Files Changed - CMakeLists.txt: Added security regression test file - src/core/profiling.cpp: Fixed use-after-free with shared_lock - include/agents/lead_agent_base_impl.hpp: Added integer overflow check - src/concurrency/pull_or_steal.cpp: Fixed null pointer + TOCTOU + modulo by zero - src/core/agent_id_interning.cpp: Added agent ID overflow check - include/core/config.hpp: Added compile-time validation with static_assert - src/core/metrics.cpp: Added forced cleanup for memory leak prevention - tests/unit/test_security_regression.cpp: NEW - Comprehensive regression tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
8 tasks
mvillmow
added a commit
that referenced
this pull request
Nov 27, 2025
This PR fixes 9 security vulnerabilities identified during comprehensive code review: ## CRITICAL Vulnerabilities Fixed 1. **Use-after-free in ProfilingSession** (profiling.cpp:83-121) - Root cause: Map pointer captured before lock release, invalidated by rehashing - Fix: Hold shared_lock during entire section access to prevent map modifications - Impact: Prevents crash in multi-threaded profiling scenarios - File: src/core/profiling.cpp 2. **Integer overflow in LeadAgentBase** (lead_agent_base_impl.hpp:215-225) - Root cause: size_t to int cast without bounds checking (size_t can be 2^64-1, int max is 2^31-1) - Fix: Check subtasks.size() > INT_MAX before cast, return error if exceeded - Impact: Prevents overflow causing negative coordination count - File: include/agents/lead_agent_base_impl.hpp 3. **Null pointer dereference + TOCTOU race in PullOrSteal** (pull_or_steal.cpp:43-65) - Root cause: queues.size() captured before loop, vector could shrink; null pointers not checked - Fix: Capture num_workers once, add combined bounds+null check before access - Impact: Prevents segfault in work-stealing scheduler - File: src/concurrency/pull_or_steal.cpp ## HIGH Priority Vulnerabilities Fixed 4. **Agent ID space exhaustion** (agent_id_interning.cpp:30-36) - Root cause: uint32_t wraps to 0 after 4,294,967,295, causing ID collisions - Fix: Check next_id_ == UINT32_MAX before increment, throw overflow_error - Impact: Prevents silent ID collision catastrophic failure - File: src/core/agent_id_interning.cpp 5. **Configuration validation missing** (config.hpp:147-160) - Root cause: Float percentage truncated to size_t without validation - Fix: Add static_assert compile-time checks for watermark percentage and result - Impact: Catches invalid configuration at compile time - File: include/core/config.hpp ## MEDIUM Priority Vulnerabilities Fixed 6. **Memory cleanup failure** (metrics.cpp:46-70) - Root cause: Time-based cleanup might not remove enough entries under flood - Fix: Add forced removal of oldest entries if time-based cleanup insufficient - Impact: Prevents unbounded memory growth under message flood - File: src/core/metrics.cpp 7. **Modulo by zero** (pull_or_steal.cpp:43-49) - Root cause: num_workers could be 0, causing (worker_index + i) % 0 - Fix: Check num_workers == 0, return nullopt early - Impact: Prevents FPE crash (fixed with CRITICAL #3) - File: src/concurrency/pull_or_steal.cpp ## Testing - Added comprehensive security regression test suite (test_security_regression.cpp) - 10 new test cases covering all 7 vulnerabilities - Stress tests: 100 threads × 100 sections × 10 records (ProfilingSession) - Memory flood test: 20,000 messages (2x limit) for metrics cleanup - All 476 tests pass with AddressSanitizer + UndefinedBehaviorSanitizer - Test runtime: 66.33 seconds ## Files Changed - CMakeLists.txt: Added security regression test file - src/core/profiling.cpp: Fixed use-after-free with shared_lock - include/agents/lead_agent_base_impl.hpp: Added integer overflow check - src/concurrency/pull_or_steal.cpp: Fixed null pointer + TOCTOU + modulo by zero - src/core/agent_id_interning.cpp: Added agent ID overflow check - include/core/config.hpp: Added compile-time validation with static_assert - src/core/metrics.cpp: Added forced cleanup for memory leak prevention - tests/unit/test_security_regression.cpp: NEW - Comprehensive regression tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description Summary:
All workflows will run automatically once the PR is created!