Skip to content

Commit 856ec69

Browse files
committed
Merge remote-tracking branch 'origin/main' into fix/1388-hook-conflict-notice
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com> # Conflicts: # tests/test_cli.c
2 parents d5eb22d + ad96bf3 commit 856ec69

13 files changed

Lines changed: 528 additions & 72 deletions

File tree

‎.github/workflows/codeql.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ jobs:
2929
run: sudo apt-get update && sudo apt-get install -y zlib1g-dev
3030

3131
- name: Initialize CodeQL
32-
uses: github/codeql-action/init@e4fba868fa4b1b91e1fdab776edc8cfbe6e9fb81 # v4.37.3
32+
uses: github/codeql-action/init@f205ea1c3313d32999d8d6a48b4f6530d4437b38 # v4.37.4
3333
with:
3434
languages: c-cpp
3535
build-mode: manual
@@ -38,6 +38,6 @@ jobs:
3838
run: scripts/build.sh
3939

4040
- name: Perform CodeQL Analysis
41-
uses: github/codeql-action/analyze@e4fba868fa4b1b91e1fdab776edc8cfbe6e9fb81 # v4.37.3
41+
uses: github/codeql-action/analyze@f205ea1c3313d32999d8d6a48b4f6530d4437b38 # v4.37.4
4242
with:
4343
category: "/language:c-cpp"

‎src/cli/activation_transaction.c‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,18 @@ const char *cbm_activation_transaction_refusal_note(void) {
110110
return g_activation_refusal_note;
111111
}
112112

113+
#ifdef CBM_ENABLE_TEST_SEAMS
114+
/* #1416 test seam: install a refusal note so the CLI attribution path is
115+
* testable without constructing a real Windows ACL refusal. */
116+
void cbm_activation_transaction_note_refusal_for_testing(const char *predicate,
117+
unsigned long os_error) {
118+
activation_refusal_clear();
119+
if (predicate && predicate[0]) {
120+
activation_note_refusal(predicate, os_error);
121+
}
122+
}
123+
#endif
124+
113125
#ifdef _WIN32
114126
typedef HANDLE activation_native_file_t;
115127
#define ACTIVATION_INVALID_FILE INVALID_HANDLE_VALUE

‎src/cli/activation_transaction.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,9 @@ const char *cbm_activation_transaction_status_message(cbm_activation_transaction
9797
* entry; single-threaded like the rest of the transaction API. */
9898
const char *cbm_activation_transaction_refusal_note(void);
9999

100+
#ifdef CBM_ENABLE_TEST_SEAMS
101+
void cbm_activation_transaction_note_refusal_for_testing(const char *predicate,
102+
unsigned long os_error);
103+
#endif
104+
100105
#endif /* CBM_ACTIVATION_TRANSACTION_H */

‎src/cli/cli.c‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,24 @@ static const char *g_cli_activation_runtime_parent_for_test = NULL;
197197

198198
static void cli_activation_diagnostic(const cbm_cli_activation_ops_t *ops, const char *message) {
199199
const char *diagnostic = message ? message : CLI_ACTIVATION_REFUSED_MESSAGE;
200+
/* #1416: when the transaction recorded a concrete refusal (an ACL or
201+
* filesystem safety check), say THAT. The generic text blames "active CBM
202+
* sessions" for what is a validation refusal - reporters rebooted, killed
203+
* every process, and hunted phantom handles because the message pointed at
204+
* sessions that did not exist. The sessions wording remains for genuine
205+
* stop/reservation failures, which record no refusal note. */
206+
char attributed[CBM_SZ_1K];
207+
const char *note = cbm_activation_transaction_refusal_note();
208+
if (diagnostic == CLI_ACTIVATION_REFUSED_MESSAGE && note && note[0]) {
209+
(void)snprintf(attributed, sizeof(attributed),
210+
"error: activation was refused by a filesystem safety check before any "
211+
"change was made: %s\n"
212+
"error: this is not a session problem. If the flagged directory is one you "
213+
"trust, remove the flagged permission grant (icacls <dir> /remove:g <sid>) "
214+
"or use an owner-private directory for --dir/CBM_CACHE_DIR, then retry.",
215+
note);
216+
diagnostic = attributed;
217+
}
200218
if (ops && ops->visible_diagnostic) {
201219
ops->visible_diagnostic(ops->context, diagnostic);
202220
return;
@@ -7177,9 +7195,13 @@ static void install_claude_code_config(const char *home, const char *binary_path
71777195
char hook_path[CLI_BUF_1K];
71787196
gate_ok = cbm_install_hook_gate_script(home, binary_path);
71797197
snprintf(hook_path, sizeof(hook_path), "%s/hooks/%s", config_dir, CMM_HOOK_GATE_SCRIPT);
7198+
/* #1387: a failed script (re)write must never remove existing hook
7199+
* entries. The common failure is TEXT_UNOWNED - a script the user
7200+
* modified or a manual install wrote with another binary path - and
7201+
* that script still works; deleting the registration turns a skipped
7202+
* update into config loss. Entry removal belongs to uninstall only. */
71807203
if (!gate_ok) {
71817204
record_agent_config_error(false, "Claude Code", "hook_script_install", hook_path);
7182-
(void)cbm_remove_claude_hooks(settings_path);
71837205
} else if (cbm_upsert_claude_hooks(settings_path) != CLI_OK) {
71847206
gate_ok = false;
71857207
record_agent_config_error(false, "Claude Code", "hook_register", settings_path);
@@ -7190,7 +7212,6 @@ static void install_claude_code_config(const char *home, const char *binary_path
71907212
CMM_SESSION_REMINDER_SCRIPT);
71917213
if (!session_ok) {
71927214
record_agent_config_error(false, "Claude Code", "hook_script_install", hook_path);
7193-
(void)cbm_remove_session_hooks(settings_path);
71947215
} else if (cbm_upsert_session_hooks(settings_path) != CLI_OK) {
71957216
session_ok = false;
71967217
record_agent_config_error(false, "Claude Code", "hook_register", settings_path);
@@ -7201,7 +7222,6 @@ static void install_claude_code_config(const char *home, const char *binary_path
72017222
CMM_SUBAGENT_REMINDER_SCRIPT);
72027223
if (!subagent_ok) {
72037224
record_agent_config_error(false, "Claude Code", "hook_script_install", hook_path);
7204-
(void)cbm_remove_claude_subagent_hooks(settings_path);
72057225
} else if (cbm_upsert_claude_subagent_hooks(settings_path) != CLI_OK) {
72067226
subagent_ok = false;
72077227
record_agent_config_error(false, "Claude Code", "hook_register", settings_path);

‎src/cypher/cypher.c‎

Lines changed: 91 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1655,7 +1655,6 @@ static int parse_return_item(parser_t *p, cbm_return_item_t *item) {
16551655
return 0;
16561656
}
16571657

1658-
/* Parse ORDER BY field into r->order_by and r->order_dir */
16591658
/* Parse aggregate function call for ORDER BY */
16601659
static void parse_order_by_agg(parser_t *p, char *buf, size_t buf_sz) {
16611660
const char *fn = agg_func_name(peek(p)->type);
@@ -1696,18 +1695,33 @@ static char *parse_order_by_expr(parser_t *p, char *buf, size_t buf_sz) {
16961695
return buf;
16971696
}
16981697

1699-
static void parse_order_by_clause(parser_t *p, cbm_return_clause_t *r) {
1698+
/* Parse the full comma-separated ORDER BY key list (#1334). Consuming only the
1699+
* first key left ", key2 ... LIMIT n" unparsed, which silently dropped the
1700+
* LIMIT and flooded the caller with the whole result set. Returns 0 on
1701+
* success, CBM_NOT_FOUND when the key list exceeds the modeled maximum. */
1702+
static int parse_order_by_clause(parser_t *p, cbm_return_clause_t *r) {
17001703
expect(p, TOK_BY);
1701-
char order_buf[CBM_SZ_256];
1702-
parse_order_by_expr(p, order_buf, sizeof(order_buf));
1703-
r->order_by = heap_strdup(order_buf);
1704-
if (match(p, TOK_ASC)) {
1705-
r->order_dir = heap_strdup("ASC");
1706-
} else if (match(p, TOK_DESC)) {
1707-
r->order_dir = heap_strdup("DESC");
1708-
}
1704+
do {
1705+
if (r->order_key_count >= CBM_CYPHER_ORDER_KEYS_MAX) {
1706+
return CBM_NOT_FOUND;
1707+
}
1708+
char order_buf[CBM_SZ_256];
1709+
parse_order_by_expr(p, order_buf, sizeof(order_buf));
1710+
bool desc = false;
1711+
if (match(p, TOK_ASC)) {
1712+
desc = false;
1713+
} else if (match(p, TOK_DESC)) {
1714+
desc = true;
1715+
}
1716+
r->order_keys[r->order_key_count] = heap_strdup(order_buf);
1717+
r->order_descs[r->order_key_count] = desc;
1718+
r->order_key_count++;
1719+
} while (match(p, TOK_COMMA));
1720+
return 0;
17091721
}
17101722

1723+
static void free_return_clause(cbm_return_clause_t *r);
1724+
17111725
/* Parse RETURN/WITH clause (shared logic) */
17121726
static int parse_return_or_with(parser_t *p, cbm_return_clause_t **out, bool is_with) {
17131727
cbm_token_type_t tok = (int)is_with ? TOK_WITH : TOK_RETURN;
@@ -1741,8 +1755,7 @@ static int parse_return_or_with(parser_t *p, cbm_return_clause_t **out, bool is_
17411755

17421756
cbm_return_item_t item = {0};
17431757
if (parse_return_item(p, &item) < 0) {
1744-
free(r->items);
1745-
free(r);
1758+
free_return_clause(r);
17461759
return CBM_NOT_FOUND;
17471760
}
17481761

@@ -1759,15 +1772,17 @@ static int parse_return_or_with(parser_t *p, cbm_return_clause_t **out, bool is_
17591772
* parsed item count to that width so an over-wide RETURN is rejected here
17601773
* instead of writing past those arrays downstream. */
17611774
if (r->count > CBM_SZ_32) {
1762-
free(r->items);
1763-
free(r);
1775+
free_return_clause(r);
17641776
return CBM_NOT_FOUND;
17651777
}
17661778

17671779
tail:
17681780
/* Optional ORDER BY */
17691781
if (match(p, TOK_ORDER)) {
1770-
parse_order_by_clause(p, r);
1782+
if (parse_order_by_clause(p, r) < 0) {
1783+
free_return_clause(r);
1784+
return CBM_NOT_FOUND;
1785+
}
17711786
}
17721787

17731788
/* Optional SKIP */
@@ -2100,8 +2115,9 @@ static void free_return_clause(cbm_return_clause_t *r) {
21002115
free(r->items[i].args);
21012116
}
21022117
free(r->items);
2103-
safe_str_free(&r->order_by);
2104-
safe_str_free(&r->order_dir);
2118+
for (int k = 0; k < r->order_key_count; k++) {
2119+
safe_str_free(&r->order_keys[k]);
2120+
}
21052121
free(r);
21062122
}
21072123

@@ -3308,16 +3324,17 @@ static void expand_pattern_rels(cbm_store_t *store, cbm_pattern_t *pat, binding_
33083324

33093325
/* ── Result postprocessing helpers ─────────────────────────────── */
33103326

3311-
/* Find the column index for ORDER BY, checking both column names and aliases.
3312-
* Returns -1 if not found. */
3313-
static int rb_find_order_column(const result_builder_t *rb, const cbm_return_clause_t *ret) {
3327+
/* Find the column index for one ORDER BY key, checking both column names and
3328+
* aliases. Returns -1 if not found. */
3329+
static int rb_find_order_column(const result_builder_t *rb, const cbm_return_clause_t *ret,
3330+
const char *key) {
33143331
for (int ci = 0; ci < rb->col_count; ci++) {
3315-
if (strcmp(rb->columns[ci], ret->order_by) == 0) {
3332+
if (strcmp(rb->columns[ci], key) == 0) {
33163333
return ci;
33173334
}
33183335
}
33193336
for (int ci = 0; ci < ret->count; ci++) {
3320-
if (ret->items[ci].alias && strcmp(ret->items[ci].alias, ret->order_by) == 0) {
3337+
if (ret->items[ci].alias && strcmp(ret->items[ci].alias, key) == 0) {
33213338
return ci;
33223339
}
33233340
}
@@ -3345,26 +3362,44 @@ static bool rb_is_numeric_column(const result_builder_t *rb, int col) {
33453362
}
33463363

33473364
static void rb_apply_order_by(result_builder_t *rb, const cbm_return_clause_t *ret) {
3348-
if (!ret->order_by) {
3365+
if (ret->order_key_count == 0) {
33493366
return;
33503367
}
3351-
int order_col = rb_find_order_column(rb, ret);
3352-
if (order_col < 0) {
3368+
/* Resolve every key up front; an unresolvable key is skipped, matching the
3369+
* single-key forgiveness (sorting on what can be resolved beats dropping
3370+
* the whole ORDER BY). */
3371+
int cols[CBM_CYPHER_ORDER_KEYS_MAX];
3372+
bool numeric[CBM_CYPHER_ORDER_KEYS_MAX];
3373+
bool descs[CBM_CYPHER_ORDER_KEYS_MAX];
3374+
int keys = 0;
3375+
for (int k = 0; k < ret->order_key_count; k++) {
3376+
int order_col = rb_find_order_column(rb, ret, ret->order_keys[k]);
3377+
if (order_col < 0) {
3378+
continue;
3379+
}
3380+
cols[keys] = order_col;
3381+
numeric[keys] = rb_is_numeric_column(rb, order_col);
3382+
descs[keys] = ret->order_descs[k];
3383+
keys++;
3384+
}
3385+
if (keys == 0) {
33533386
return;
33543387
}
3355-
3356-
bool desc = ret->order_dir && strcmp(ret->order_dir, "DESC") == 0;
3357-
bool numeric = rb_is_numeric_column(rb, order_col);
33583388
for (int i = 0; i < rb->row_count - SKIP_ONE; i++) {
33593389
for (int j = 0; j < rb->row_count - i - SKIP_ONE; j++) {
3360-
int cmp;
3361-
if (numeric) {
3362-
cmp = (int)strtol(rb->rows[j][order_col], NULL, CBM_DECIMAL_BASE) -
3363-
(int)strtol(rb->rows[j + SKIP_ONE][order_col], NULL, CBM_DECIMAL_BASE);
3364-
} else {
3365-
cmp = strcmp(rb->rows[j][order_col], rb->rows[j + SKIP_ONE][order_col]);
3390+
int cmp = 0;
3391+
for (int k = 0; k < keys && cmp == 0; k++) {
3392+
if (numeric[k]) {
3393+
cmp = (int)strtol(rb->rows[j][cols[k]], NULL, CBM_DECIMAL_BASE) -
3394+
(int)strtol(rb->rows[j + SKIP_ONE][cols[k]], NULL, CBM_DECIMAL_BASE);
3395+
} else {
3396+
cmp = strcmp(rb->rows[j][cols[k]], rb->rows[j + SKIP_ONE][cols[k]]);
3397+
}
3398+
if (descs[k]) {
3399+
cmp = -cmp;
3400+
}
33663401
}
3367-
if (desc ? cmp < 0 : cmp > 0) {
3402+
if (cmp > 0) {
33683403
const char **tmp = rb->rows[j];
33693404
rb->rows[j] = rb->rows[j + SKIP_ONE];
33703405
rb->rows[j + SKIP_ONE] = tmp;
@@ -3674,18 +3709,26 @@ static void distinct_list_add(char ***list, int *count, const char *val) {
36743709
(*list)[idx] = heap_strdup(val);
36753710
}
36763711

3677-
/* Sort bindings by a virtual variable using bubble sort */
3678-
static void sort_bindings(binding_t *vbindings, int count, const char *key, bool desc) {
3712+
/* Sort bindings by the ORDER BY key list (virtual variables) using bubble
3713+
* sort; later keys break ties, direction is per key (#1334). */
3714+
static void sort_bindings(binding_t *vbindings, int count, const cbm_return_clause_t *wc) {
36793715
for (int i = 0; i < count - SKIP_ONE; i++) {
36803716
for (int j = 0; j < count - i - SKIP_ONE; j++) {
3681-
const char *va = binding_get_virtual(&vbindings[j], key, NULL);
3682-
const char *vb2 = binding_get_virtual(&vbindings[j + SKIP_ONE], key, NULL);
3683-
char *ea = NULL;
3684-
char *eb = NULL;
3685-
double da = strtod(va, &ea);
3686-
double db = strtod(vb2, &eb);
3687-
int cmp = (ea != va && eb != vb2) ? ((da > db) - (da < db)) : strcmp(va, vb2);
3688-
if (desc ? cmp < 0 : cmp > 0) {
3717+
int cmp = 0;
3718+
for (int k = 0; k < wc->order_key_count && cmp == 0; k++) {
3719+
const char *va = binding_get_virtual(&vbindings[j], wc->order_keys[k], NULL);
3720+
const char *vb2 =
3721+
binding_get_virtual(&vbindings[j + SKIP_ONE], wc->order_keys[k], NULL);
3722+
char *ea = NULL;
3723+
char *eb = NULL;
3724+
double da = strtod(va, &ea);
3725+
double db = strtod(vb2, &eb);
3726+
cmp = (ea != va && eb != vb2) ? ((da > db) - (da < db)) : strcmp(va, vb2);
3727+
if (wc->order_descs[k]) {
3728+
cmp = -cmp;
3729+
}
3730+
}
3731+
if (cmp > 0) {
36893732
binding_t tmp = vbindings[j];
36903733
vbindings[j] = vbindings[j + SKIP_ONE];
36913734
vbindings[j + SKIP_ONE] = tmp;
@@ -3718,9 +3761,8 @@ static void bindings_skip_limit(binding_t *vbindings, int *count, int skip, int
37183761

37193762
/* Sort, skip, and limit binding array in-place */
37203763
static void with_sort_skip_limit(const cbm_return_clause_t *wc, binding_t *vbindings, int *vcount) {
3721-
if (wc->order_by) {
3722-
bool wdesc = wc->order_dir && strcmp(wc->order_dir, "DESC") == 0;
3723-
sort_bindings(vbindings, *vcount, wc->order_by, wdesc);
3764+
if (wc->order_key_count > 0) {
3765+
sort_bindings(vbindings, *vcount, wc);
37243766
}
37253767
bindings_skip_limit(vbindings, vcount, wc->skip, wc->limit);
37263768
}
@@ -4390,7 +4432,7 @@ static void build_return_columns(result_builder_t *rb, cbm_return_clause_t *ret)
43904432
static void execute_return_simple(cbm_return_clause_t *ret, binding_t *bindings, int bind_count,
43914433
int max_rows, result_builder_t *rb) {
43924434
int proj_cap = max_rows;
4393-
if (ret->limit > 0 && !ret->distinct && !ret->order_by && ret->skip <= 0) {
4435+
if (ret->limit > 0 && !ret->distinct && ret->order_key_count == 0 && ret->skip <= 0) {
43944436
proj_cap = ret->limit;
43954437
}
43964438
for (int bi = 0; bi < bind_count && rb->row_count < proj_cap; bi++) {

‎src/cypher/cypher.h‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -262,15 +262,23 @@ typedef struct {
262262
int arg_count;
263263
} cbm_return_item_t;
264264

265+
/* Upper bound on ORDER BY sort keys. Queries with more keys are rejected at
266+
* parse time: an unmodeled key must be a loud error, never a silently dropped
267+
* remainder (#1334 - the unconsumed tail swallowed the LIMIT clause). */
268+
#define CBM_CYPHER_ORDER_KEYS_MAX 8
269+
265270
typedef struct {
266271
cbm_return_item_t *items;
267272
int count;
268273
bool distinct;
269-
bool star; /* RETURN * */
270-
const char *order_by; /* "variable.property" or "COUNT(var)" or alias */
271-
const char *order_dir; /* "ASC" or "DESC", NULL = default */
272-
int skip; /* SKIP N, 0 = none */
273-
int limit; /* 0 = default */
274+
bool star; /* RETURN * */
275+
/* ORDER BY key list, in priority order. Each key is "variable.property",
276+
* "COUNT(var)" or an alias; direction is per key (Cypher semantics). */
277+
const char *order_keys[CBM_CYPHER_ORDER_KEYS_MAX];
278+
bool order_descs[CBM_CYPHER_ORDER_KEYS_MAX]; /* false = ASC (default) */
279+
int order_key_count; /* 0 = no ORDER BY */
280+
int skip; /* SKIP N, 0 = none */
281+
int limit; /* 0 = default */
274282
} cbm_return_clause_t;
275283

276284
/* Full query AST */

0 commit comments

Comments
 (0)