Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/cli/activation_transaction.c
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,18 @@ const char *cbm_activation_transaction_refusal_note(void) {
return g_activation_refusal_note;
}

#ifdef CBM_ENABLE_TEST_SEAMS
/* #1416 test seam: install a refusal note so the CLI attribution path is
* testable without constructing a real Windows ACL refusal. */
void cbm_activation_transaction_note_refusal_for_testing(const char *predicate,
unsigned long os_error) {
activation_refusal_clear();
if (predicate && predicate[0]) {
activation_note_refusal(predicate, os_error);
}
}
#endif

#ifdef _WIN32
typedef HANDLE activation_native_file_t;
#define ACTIVATION_INVALID_FILE INVALID_HANDLE_VALUE
Expand Down
5 changes: 5 additions & 0 deletions src/cli/activation_transaction.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,9 @@ const char *cbm_activation_transaction_status_message(cbm_activation_transaction
* entry; single-threaded like the rest of the transaction API. */
const char *cbm_activation_transaction_refusal_note(void);

#ifdef CBM_ENABLE_TEST_SEAMS
void cbm_activation_transaction_note_refusal_for_testing(const char *predicate,
unsigned long os_error);
#endif

#endif /* CBM_ACTIVATION_TRANSACTION_H */
18 changes: 18 additions & 0 deletions src/cli/cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,24 @@ static const char *g_cli_activation_runtime_parent_for_test = NULL;

static void cli_activation_diagnostic(const cbm_cli_activation_ops_t *ops, const char *message) {
const char *diagnostic = message ? message : CLI_ACTIVATION_REFUSED_MESSAGE;
/* #1416: when the transaction recorded a concrete refusal (an ACL or
* filesystem safety check), say THAT. The generic text blames "active CBM
* sessions" for what is a validation refusal - reporters rebooted, killed
* every process, and hunted phantom handles because the message pointed at
* sessions that did not exist. The sessions wording remains for genuine
* stop/reservation failures, which record no refusal note. */
char attributed[CBM_SZ_1K];
const char *note = cbm_activation_transaction_refusal_note();
if (diagnostic == CLI_ACTIVATION_REFUSED_MESSAGE && note && note[0]) {
(void)snprintf(attributed, sizeof(attributed),
"error: activation was refused by a filesystem safety check before any "
"change was made: %s\n"
"error: this is not a session problem. If the flagged directory is one you "
"trust, remove the flagged permission grant (icacls <dir> /remove:g <sid>) "
"or use an owner-private directory for --dir/CBM_CACHE_DIR, then retry.",
note);
diagnostic = attributed;
}
if (ops && ops->visible_diagnostic) {
ops->visible_diagnostic(ops->context, diagnostic);
return;
Expand Down
37 changes: 37 additions & 0 deletions tests/test_cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "test_framework.h"
#include "test_helpers.h"
#include <cli/agent_profiles.h>
#include <cli/activation_transaction.h>
#include <cli/cli.h>
#include <cli/progress_sink.h>
#include <daemon/bootstrap.h>
Expand Down Expand Up @@ -646,6 +647,41 @@ TEST(cli_activation_refuses_when_cohort_does_not_drain) {
PASS();
}

/* Regression for #1416: when the activation transaction recorded a concrete
* refusal (e.g. the Windows ACL safety check), the CLI must attribute the
* failure to that check instead of blaming "active CBM sessions" - reporters
* rebooted and hunted phantom handles because no sessions existed. The
* sessions wording must remain for refusals with no recorded note. */
TEST(cli_activation_refusal_note_reaches_diagnostic_issue1416) {
cbm_activation_transaction_note_refusal_for_testing(
"acl-grants-cross-account-mutation to S-1-5-11", 0UL);
cli_activation_fake_t fake = {
.participants_active = true,
.mutation_reserve_result = 0,
};
cbm_cli_activation_ops_t ops = {
.context = &fake,
.reserve_for_mutation = cli_activation_fake_reserve_mutation,
.mutation_lease_release = cli_activation_fake_release_mutation,
.visible_diagnostic = cli_activation_fake_diagnostic,
};
ASSERT_EQ(cbm_cli_activation_guard_with_ops(&ops, cli_activation_fake_mutation, &fake), 1);
ASSERT_NOT_NULL(strstr(fake.diagnostic, "acl-grants-cross-account-mutation"));
ASSERT_NOT_NULL(strstr(fake.diagnostic, "not a session problem"));
ASSERT_NULL(strstr(fake.diagnostic, "could not be stopped safely"));

/* No note recorded -> the sessions wording is still the right message. */
cbm_activation_transaction_note_refusal_for_testing(NULL, 0UL);
cli_activation_fake_t plain = {
.participants_active = true,
.mutation_reserve_result = 0,
};
ops.context = &plain;
ASSERT_EQ(cbm_cli_activation_guard_with_ops(&ops, cli_activation_fake_mutation, &plain), 1);
ASSERT_NOT_NULL(strstr(plain.diagnostic, "could not be stopped safely"));
PASS();
}

TEST(cli_activation_refuses_unsafe_cohort_reservation) {
cli_activation_fake_t fake = {
.mutation_reserve_result = -1,
Expand Down Expand Up @@ -11789,6 +11825,7 @@ SUITE(cli) {
/* Mandatory daemon activation safety */
RUN_TEST(cli_activation_quiesces_active_cohort_before_mutation);
RUN_TEST(cli_activation_refuses_when_cohort_does_not_drain);
RUN_TEST(cli_activation_refusal_note_reaches_diagnostic_issue1416);
RUN_TEST(cli_activation_refuses_unsafe_cohort_reservation);
RUN_TEST(cli_activation_releases_maintenance_lease_after_success);
RUN_TEST(cli_activation_releases_maintenance_lease_when_mutation_fails);
Expand Down
Loading