fix(cypher): bound OPTIONAL fallback in expand_pattern_rels to a single ceiling

[SEPURI-SAI-KRISHNA]

What does this PR do?

Fixes a heap buffer overflow (CWE-787) in expand_pattern_rels
(src/cypher/cypher.c) when expanding an OPTIONAL MATCH relationship pattern.
The query text is agent-controlled via the MCP query tool.

The bug

The per-hop output buffer was sized bind_cap * 10 + 1:

size_t alloc_n = (size_t)*bind_cap * (size_t)CYP_GROWTH_10 + SKIP_ONE;

The expansion helpers (expand_fixed_length / expand_var_length, via
process_edges) correctly stop at max_new = bind_cap * 10. But the OPTIONAL
fallback that keeps a row with the target left unbound was not bounded:

if (is_optional && match_count == 0) {
    ...
    new_bindings[new_count++] = nb;   // no ceiling check
}

The + 1 left room for a single fallback row after a saturated expansion.
When one source binding saturates the expansion to max_new and two or more
later sources take the OPTIONAL (no-match) path, the second fallback write runs
past the allocation.

Reachable whenever bind_cap tracks the scanned node count — i.e. the number of
matched start nodes exceeds the result limit — with e.g.:

MATCH (a:Function) OPTIONAL MATCH (a)-[:CALLS]->(b) RETURN a.name

That regime is hit on a repo with more start nodes than the supplied max_rows
(or, at the default limit, more than the 100000 result ceiling). A dense hub
(out-degree ≥ bind_cap * 10) saturates the buffer; the leaf nodes that follow
on the OPTIONAL path then overflow it.

The fix

Hold the OPTIONAL fallback to the same single global ceiling (max_new = bind_cap * 10) the expansion helpers already respect, instead of growing the
allocation. The buffer is sized for the ceiling, and every writer — expansion
and fallback alike — stops there:

int max_new = *bind_cap * CYP_GROWTH_10;
size_t alloc_n = (size_t)max_new + SKIP_ONE;
...
if (is_optional && match_count == 0 && new_count < max_new) { ... }

Behavior at the ceiling is now defined: the hop truncates at max_new
(bounded success, no overflow, no error), matching how the sibling
expand_from_bound_terminal already bounds its own fallback.

Reproduced first (ASan, before the fix):

==ERROR: AddressSanitizer: heap-buffer-overflow ... WRITE of size ...
    #0 expand_pattern_rels src/cypher/cypher.c   (OPTIONAL fallback write)
       allocated at src/cypher/cypher.c          (bind_cap*10 + 1 malloc)
    #5 test_cypher_exec_optional_rel_ceiling_truncates_no_overflow
SUMMARY: AddressSanitizer: heap-buffer-overflow ... in expand_pattern_rels

After the fix the full cypher suite passes with no sanitizer errors.

Tests

Two regression tests:

• cypher_exec_optional_rel_ceiling_truncates_no_overflow — a hub that
  saturates the expansion followed by leaf functions on the OPTIONAL path;
  asserts the query still succeeds with a bounded result (defined truncation, no
  overflow).
• cypher_exec_optional_rel_leaf_fallback_survives — a non-saturating graph;
  asserts a specific leaf's OPTIONAL row survives with its target unbound and
  that a real expanded hub row is also present (a row_count > 0 check would be
  too weak — it can pass on hub rows alone).

Checklist

• Every commit is signed off (git commit -s) — required, CI rejects
  unsigned commits (DCO, see CONTRIBUTING.md)
• Tests pass locally (make -f Makefile.cbm test)
• New behavior is covered by a test (reproduce-first for bug fixes)

[DeusData]

Thanks for isolating a genuine start-bound OPTIONAL expansion overflow. The current fix needs revision before further review:

1. Keep OPTIONAL fallback rows inside one explicit global output/memory ceiling rather than growing the already-large allocation by up to one binding_t per source row.
2. Define and test the truncation or error behavior when that ceiling is reached.
3. Strengthen the regression to prove the expected leaf fallback rows survive; row_count > 0 can pass using only hub results.
4. Remove the Crash when calling query_graph #627 claim unless its exact terminal-bound reproduction is shown RED then GREEN. This PR currently exercises the opposite, start-bound route.

This feedback is for reviewed head a3adfbc5529d542e25e8a64ab4e069ed3da17c2d.

[SEPURI-SAI-KRISHNA]

@DeusData

Thanks revised to address all four points:

1. One explicit global ceiling. The OPTIONAL fallback is now held to the same max_new = bind_cap * 10 ceiling the expansion helpers already enforce (new_count < max_new), and the buffer is sized to that ceiling — no more growing the allocation by a binding_t per source row. This matches the sibling expand_from_bound_terminal, which already bounds its fallback the same way.

2. Defined ceiling behavior. At the ceiling the hop truncates: bounded success, no overflow, no error. cypher_exec_optional_rel_ceiling_truncates_no_overflow asserts the saturating case returns rc == 0 with a bounded row count.

3. Stronger regression. Replaced the row_count > 0 check with two tests. The truncation test above, plus cypher_exec_optional_rel_leaf_fallback_survives, which scans the result rows for a specific leaf appearing with an unbound b.name and for a real expanded hub row — so it can no longer pass on hub rows alone. Confirmed RED→GREEN: with the ceiling guard removed the truncation test aborts under ASan (heap-buffer-overflow, WRITE of size 1624 at cypher.c:3266); with it in place the full cypher suite is clean (161 passed).

4. Crash when calling query_graph #627 claim removed from the title and description. This is the start-bound OPTIONAL expansion route, not the terminal-bound reproduction, so I've dropped the association rather than assert an unproven link.

[DeusData]

Thanks for the revision and for replacing the weak row-count assertion. I see current head 8763aa928b921a02a7d7b7d38054b1e79a86fa92. The single ceiling, defined truncation behavior, explicit unbound-leaf assertion, and removal of the unproven #627 association address the prior triage blockers. No further contributor action is requested now; this is back in exact-head maintainer review.