fix: address review comments on core library hardening#1303
Merged
scholarsmate merged 2 commits intofix/core-security-and-qualityfrom Mar 6, 2026
Merged
fix: address review comments on core library hardening#1303scholarsmate merged 2 commits intofix/core-security-and-qualityfrom
scholarsmate merged 2 commits intofix/core-security-and-qualityfrom
Conversation
…uards Co-authored-by: scholarsmate <[email protected]>
Copilot
AI
changed the title
[WIP] Fix core C library for security and correctness
fix: address review comments on core library hardening
Mar 6, 2026
scholarsmate
added a commit
that referenced
this pull request
Mar 7, 2026
#1302) * fix: harden core C library for security, correctness, and code quality Security and correctness fixes: - Fix snprintf buffer overflow in edit.cpp (FILENAME_MAX -> FILENAME_MAX + 1) - Convert recursive undo/redo to iterative loops to prevent stack overflow - Add thread_local to static buffers in filesystem.cpp for thread safety - Add odd-length input validation to omega_encode_hex2bin - Guard DEBUG define with #ifndef to support conditional compilation - Remove unsafe mkstemp assert in filesystem.cpp Design and code quality fixes: - Replace assert() with proper error returns in all public API functions across segment, change, search, viewport, session, check, utility, and filesystem modules - Replace magic number 7 with sizeof(omega_data_t) - 1 in segment.cpp - Fix DBG macro parenthesization in macros.h - Rename stl_string_adapter.cpp to stl_string_adaptor.cpp to match header - Add negative capacity guard in segment creation Test coverage: - Add Hex2Bin Odd Length test (7 assertions) - Add Large Transaction Undo/Redo test (114 assertions) - Add Null Pointer Safety test for core APIs (53 assertions) - Add Null Pointer Safety test for utility APIs (14 assertions) - Add Null Pointer Safety test for filesystem APIs (11 assertions) - Add Segment Small Data Optimization test (11 assertions) * fix: address review comments on core library hardening (#1303) * Initial plan * fix: address review comments - null checks, error handling, nullptr guards Co-authored-by: scholarsmate <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: scholarsmate <[email protected]> * applied fixes * applied fixes * optimize undo a bit * optimize transforms a bit, eliminating ~1/3 of the I/O * two low-risk performance optimzations * three additional low-risk performance optimzations * close testing gaps * fix: replace dynamic ESM imports with static imports and fix error type assertions in coverageGaps.spec.ts * fix: use type-agnostic error assertions in coverageGaps.spec.ts Some client functions reject with string errors (getComputedFileSize, destroySession) while others reject with Error objects (insert, del). Use expect(err).to.exist to accept both types. * fix: harden core C library against remaining review-identified defects (round 4) (#1304) * Initial plan * fix: address review comments from PR 1302 review 3907448840 Co-authored-by: scholarsmate <[email protected]> * fix: improve snprintf checks with explicit result variable and dual condition Co-authored-by: scholarsmate <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: scholarsmate <[email protected]> * fix: harden core save and client edit paths * fix: refresh native server for client tests * fix: define O_BINARY for non-Windows builds --------- Co-authored-by: Copilot <[email protected]>
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
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.
Follow-up to #1302 addressing five specific issues raised in code review of the core C/C++ hardening PR.
Changes
encode.c/encode.hassert(src)/assert(dst)with null checks returning0inomega_encode_hex2bindstrequires(src_length / 2) + 1bytes (null-terminated, consistent withbin2hex)search.cppassert(0 < pattern_length)with earlyreturn nullptrwhenpattern_length <= 0, consistent with other guards in the same functionedit.cppomega_edit_undo_last_change: addif (!session_ptr) { return 0; }before accessingsession_ptr->models_—omega_session_changes_paused(nullptr)returns0, so a null session would otherwise enter the loop and crashomega_edit_redo_last_undo: same null guard; break out of the redo loop immediately onupdate_()failure, leaving the failed change inchanges_undoneso the session isn't advanced further in an error statefilesystem.cppomega_util_available_filename, check results ofomega_util_dirname,omega_util_basename, andomega_util_file_extensionfornullptrbefore constructingstd::string— these can returnnullptrfor overlong paths, which previously caused UB✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.