diff --git a/modules/mesh-cli/cmd/mesh-cli/commands.go b/modules/mesh-cli/cmd/mesh-cli/commands.go index 6513750b..93f56405 100644 --- a/modules/mesh-cli/cmd/mesh-cli/commands.go +++ b/modules/mesh-cli/cmd/mesh-cli/commands.go @@ -352,9 +352,11 @@ var commands = []command{ a["consumer"] = l.pos[0] // The vault deletes only when confirm repeats the holder's name (mesh-vault retirement.go); without // --confirm it answers what it would delete. - if l.has("confirm") { - a["confirm"] = l.pos[0] + if !l.has("confirm") { + // Without --confirm the vault deletes nothing and says so: no act is recorded for it. + return call{"mesh-vault.provisioner_delete", a}, nil } + a["confirm"] = l.pos[0] return warranted(call{"mesh-vault.provisioner_delete", a}, l.pos[0]), nil }}, {words: []string{"cleanup"}, does: "every retired holder per provider", build: fixed("mesh-vault.provisioner_retirement")}, diff --git a/modules/mesh-cli/cmd/mesh-cli/main_test.go b/modules/mesh-cli/cmd/mesh-cli/main_test.go index e22808e9..3caa0897 100644 --- a/modules/mesh-cli/cmd/mesh-cli/main_test.go +++ b/modules/mesh-cli/cmd/mesh-cli/main_test.go @@ -121,6 +121,8 @@ func TestEachLineCallsItsOwner(t *testing.T) { {"settings clear zsh execute --node g14 --why test", "mesh-config.config", `{"action":"clear","key":"execute","module":"zsh","node":"g14","why":"test"}`}, {"config history --key k --limit 5", "mesh-config.config", `{"action":"history","key":"k","limit":5}`}, {"operator key show", "mesh-vault.vault_status", `{}`}, + // A cleanup without --confirm deletes nothing, so no hand act is recorded for it (issue 1126 review 2). + {"cleanup delete ace.plex --why dry", "mesh-vault.provisioner_delete", `{"consumer":"ace.plex","why":"dry"}`}, {"collect --confirm --why space", "mesh-artifact-sweep.collect", `{"confirm":"true","why":"space"}`}, {"images ace", "mesh-artifact-sweep.images", `{"node":"ace"}`}, {"licence use g14 work", "anthropic-licence-manager.switch", `{"consumer":"g14","licence":"work"}`}, @@ -296,8 +298,6 @@ func TestASensitiveLineRecordsItsHandActAndNamesIt(t *testing.T) { `{"act":"act-7","consumers":["c1","c2"],"why":"done"}`}, {"cleanup delete ace.plex --why done --confirm", "mesh-vault.provisioner_delete", "mesh-vault.provisioner_delete", "ace.plex", `{"act":"act-7","confirm":"ace.plex","consumer":"ace.plex","why":"done"}`}, - {"cleanup delete ace.plex --why dry", "mesh-vault.provisioner_delete", "mesh-vault.provisioner_delete", "ace.plex", - `{"act":"act-7","consumer":"ace.plex","why":"dry"}`}, } { got := console(t, map[string]string{"mesh-hand-acts.hand-act": recorded}) code, _, errOut := runLine(strings.Fields(c.line)...) diff --git a/modules/mesh-vault/cmd/mesh-vault/sensitive_test.go b/modules/mesh-vault/cmd/mesh-vault/sensitive_test.go index e7b33417..89380d44 100644 --- a/modules/mesh-vault/cmd/mesh-vault/sensitive_test.go +++ b/modules/mesh-vault/cmd/mesh-vault/sensitive_test.go @@ -102,3 +102,17 @@ func TestNoxsRuntimeExportsWithItsHandActAndIsRefusedWithout(t *testing.T) { t.Errorf("an export with nox's act: %v", err) } } + +// Review 2 of issue 1126: a delete without its confirm deletes nothing, so it is refused for that, never judged as an +// act wanting a hand act; nox records none for it. +func TestADeleteWithoutConfirmIsRefusedForItsConfirmAlone(t *testing.T) { + del := toolNamed(t, definitions(&Env{Custody: retiredC(t).Custody}), "provisioner_delete") + if _, err := del.runAs("shanks.node-tools", map[string]any{"consumer": "d-app", "why": "look"}); err == nil || + !strings.Contains(err.Error(), "confirm must repeat") { + t.Errorf("an unconfirmed delete: %v", err) + } + if _, err := del.runAs("shanks.node-tools", map[string]any{"consumer": "d-app", "confirm": "d-app", "why": "gone"}); err == nil || + !strings.Contains(err.Error(), "named none") { + t.Errorf("a confirmed delete with no act: %v", err) + } +} diff --git a/modules/mesh-vault/cmd/mesh-vault/tools.go b/modules/mesh-vault/cmd/mesh-vault/tools.go index 16dd07c5..9cdcff77 100644 --- a/modules/mesh-vault/cmd/mesh-vault/tools.go +++ b/modules/mesh-vault/cmd/mesh-vault/tools.go @@ -540,13 +540,17 @@ func definitions(env *Env) []toolDef { Input: actInput(false, true), run: func(caller string, a map[string]any) (any, error) { consumer, _ := a["consumer"].(string) - if err := env.judge(caller, SensitiveDelete, a, consumer); err != nil { - return nil, err + confirm, _ := a["confirm"].(string) + // A delete without its confirm deletes nothing and is refused for that alone (Custody.Delete); one + // confirmed is judged. + if confirm != "" { + if err := env.judge(caller, SensitiveDelete, a, consumer); err != nil { + return nil, err + } } if env.Custody == nil { return nil, errors.New("the vault has no custody ledger") } - confirm, _ := a["confirm"].(string) if err := env.Custody.Delete(consumer, confirm, actOf(caller, a)); err != nil { return nil, err }