query_graph: a non-numeric LIMIT/SKIP operand (e.g. LIMIT $limit) is silently dropped — the query runs unbounded and reports success #1994

What happens

parse_return_or_with() accepts SKIP/LIMIT only when the next token is a number. When it is not, the clause is discarded and parsing continues as if it were never written. The query then executes with no limit and returns isError: false.

Reproduced on main (3627eff), built from source, against a 26-node fixture repo:

$ cbm cli --json query_graph --project demo --query 'MATCH (n) RETURN n.name LIMIT 1'
rows: 1

$ cbm cli --json query_graph --project demo --query 'MATCH (n) RETURN n.name LIMIT $limit'
rows: 26          # LIMIT silently ignored, isError=false

$ cbm cli --json query_graph --project demo --query 'MATCH (n) RETURN n.name SKIP $offset LIMIT 1'
rows: 26          # the orphaned operand also swallows the LIMIT that follows

$ cbm cli --json query_graph --project demo --query 'MATCH (n) RETURN n.name LIMIT abc'
rows: 26

Expected: a parse error naming the offending token. Actual: a successful response containing the entire result set.

$param is not an exotic input — it is what a Neo4j-shaped client or an LLM agent writes by default. On a real repository the same query returns every matching row up to the 100k ceiling, or dies with result exceeded 100k rows — use narrower filters or add LIMIT, which tells the user to add a LIMIT they did write.

Root cause

src/cypher/cypher.c:1821-1834:

    /* Optional SKIP */
    if (match(p, TOK_SKIP)) {
        const cbm_token_t *num = expect(p, TOK_NUMBER);
        if (num) {
            r->skip = (int)strtol(num->text, NULL, CBM_DECIMAL_BASE);
        }
    }

    /* Optional LIMIT */
    if (match(p, TOK_LIMIT)) {
        const cbm_token_t *num = expect(p, TOK_NUMBER);
        if (num) {
            r->limit = (int)strtol(num->text, NULL, CBM_DECIMAL_BASE);
        }
    }

expect() (cypher.c:492-500) fills p->error and returns NULL, but both call sites only test if (num) and fall through to *out = r; return 0;. r->limit keeps the -1 initialiser set at cypher.c:1770, which cypher.c:4823 reads as "no LIMIT":

    rb_apply_skip_limit(rb, ret->skip, ret->limit >= 0 ? ret->limit : max_rows);

Two further layers hide the failure:

1. cbm_lex() skips unknown characters (cypher.c:410-411, /* Unknown character — skip */ i++;), so $ disappears and the operand lexes as an ordinary token.
2. parse_post_where() does not require TOK_EOF after the RETURN clause, so the orphaned token is dropped as well — which is why SKIP $offset LIMIT 1 loses both clauses.

Because every sub-parse returned 0, cbm_parse() never consults p.error and the caller gets error == NULL.

Why this is the failure mode #1334 already banned

tests/test_cypher.c:410-411 says of the ORDER BY cap fix:

more keys than the modeled maximum is a loud parse error - the old failure mode (ignore the remainder, drop the LIMIT) must never come back.

Same failure mode, different route: #1334 closed the ORDER BY key-list path; the SKIP/LIMIT operand path was left open.

Fix

Propagate the expect() failure in both branches — free_return_clause(r); return CBM_NOT_FOUND; — matching the ORDER BY branch immediately above. cbm_parse() then surfaces the existing expected token type ... got ... at pos N message.

PR follows with four regression tests: three fail on main and pass with the fix, and a control asserting SKIP 2 LIMIT 10 still parses and still carries 2/10. scripts/test.sh is green locally (clean ASan+UBSan build, all suites, contract steps).

Separately, the (int)strtol(..., NULL, ...) truncation at the same lines means a literal ≥ 2^31 also lands on a wrong value, but that needs an absurd literal and is not worth its own issue; the operand check above is the reachable defect.

Environment

• main @ 3627eff, built from source with scripts/build.sh
• macOS 15 (Darwin 25.6.0), arm64, clang