Improve UUID handling in SQLite functions and error reporting - #22233

1 issue found across 5 files

Confidence score: 4/5

• This PR is likely safe to merge, but there is a mild logic-risk issue (severity 4/10, high confidence) that keeps it from a full low-risk score.
• In src/database/sqlite/sqlite_health.c, sql_find_alert_transition() may return false even after valid rows were processed, because later invalid rows can reset ok to false, which could suppress expected transition detection.
• Pay close attention to src/database/sqlite/sqlite_health.c - ensure ok is not overwritten by invalid-row handling after a successful match.

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/database/sqlite/sqlite_health.c">

<violation number="1" location="src/database/sqlite/sqlite_health.c:1391">
P2: `sql_find_alert_transition()` can incorrectly return false after successfully processing valid rows because invalid rows reset `ok` back to false.</violation>
</file>

sequenceDiagram
    participant App as Subsystems (Health, Context, Metadata)
    participant Help as NEW: SQLite UUID Helpers
    participant SQL as SQLite Statement (Result Row)
    participant Log as Error Reporting

    Note over App, SQL: Data Retrieval Loop
    
    App->>SQL: sqlite3_step()
    SQL-->>App: SQLITE_ROW
    
    rect rgb(240, 240, 240)
        Note over App, Help: NEW: Validation Flow
        App->>Help: sqlite3_column_uuid_copy / ptr / unparse
        Help->>SQL: sqlite3_column_type(iCol)
        SQL-->>Help: column type (BLOB vs other)
        
        alt Column is BLOB
            Help->>SQL: sqlite3_column_bytes(iCol)
            SQL-->>Help: size in bytes
            
            alt Size == sizeof(nd_uuid_t)
                Help->>SQL: sqlite3_column_blob(iCol)
                SQL-->>Help: raw pointer
                Help-->>App: SUCCESS: (Valid UUID data)
            else Invalid Size or NULL
                Help-->>App: FAILURE: NULL / false
            end
        else Not a BLOB
            Help-->>App: FAILURE: NULL / false
        end
    end

    alt SUCCESS
        App->>App: Process row with valid UUID
    else FAILURE (NEW Handling)
        App->>Log: NEW: error_report(Invalid UUID)
        Note right of App: CHANGED: Skip row or return empty string
    end
    
    App->>SQL: Continue to next row

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

Pull request overview

This PR hardens UUID handling when reading UUIDs stored as SQLite BLOBs, replacing direct sqlite3_column_blob() casts with validated helpers to prevent crashes/undefined behavior from malformed rows. It centralizes safe UUID extraction/copying/unparsing and updates several SQLite-backed flows (metadata, health, contexts, ACLK alerts) to skip invalid UUID rows while emitting clearer errors.

Changes:

• Introduces sqlite3_column_uuid_ptr(), sqlite3_column_uuid_copy(), and sqlite3_column_uuid_unparse_lower() helpers for validated UUID BLOB access.
• Updates metadata cleanup/loading paths to copy/validate UUIDs before passing them to callbacks and other logic.
• Updates health/context/ACLK alert code paths to validate UUIDs, skip malformed rows, and improve error reporting.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

Improves robustness of UUID handling when reading from SQLite by introducing centralized helpers that validate UUID BLOB sizes/types before copying or formatting, and updates multiple SQLite call sites to skip malformed UUID rows with clearer logging.

Changes:

• Added sqlite3_column_uuid_ptr, sqlite3_column_uuid_copy, and sqlite3_column_uuid_unparse_lower helpers for validating/reading UUID BLOB columns.
• Replaced direct sqlite3_column_blob UUID casts across health, context, ACLK alert, and metadata paths with validated copy/unparse logic.
• Improved error reporting and prevented propagation of invalid UUIDs by skipping malformed rows.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Pull request overview

This PR improves robustness of UUID handling when reading SQLite result columns by introducing centralized UUID validation/copy/unparse helpers and applying them across SQLite health, context, ACLK alert, and metadata code paths to safely skip malformed rows and emit clearer error logs.

Changes:

• Added sqlite3_column_uuid_ptr(), sqlite3_column_uuid_copy(), and sqlite3_column_uuid_unparse_lower() helpers for strict UUID BLOB validation and safe use.
• Replaced direct sqlite3_column_blob() UUID casts with validated UUID reads in health log load/export, alert transitions/config loading, context chart/dimension listing, and metadata cleanup/metric population.
• Improved error reporting and resilience by skipping invalid rows and logging aggregated counts where relevant.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

After few hours running, everything is working as expected. LGTM!