User approval wording

Author: parsley42

aidocs/ELEVATION_MODEL.md

@@ -229,9 +229,10 @@ Behavior:
 - `PluginApprovers.<pipeName>` supersedes `FallbackApprovers` when present.
 - The requester is excluded from the eligible approver list, so users cannot
   approve their own elevation.
-- The requester receives a lowercase-letter menu such as
-  `a) david, b) bob, c) alice` and must reply with a single lowercase letter
-  matched by the elevator's `approvalChoice` reply matcher.
+- The requester receives a lowercase-letter menu identifying the action as
+  `plugin/command` when a plugin command is known, such as `vpn/add-device`,
+  and must reply with a single lowercase letter matched by the elevator's
+  `approvalChoice` reply matcher.
 - The selected approver receives a DM yes/no prompt. `yes`/`y` approves and
   returns `robot.Success`; `no`/`n`, timeout, invalid requester choice, or
   missing eligible approvers fail elevation.

bot/builtin_userapproval.go

@@ -22,6 +22,7 @@ type userApprovalRuntime interface {
 	Say(msg string, v ...interface{}) robot.RetVal
 	Log(l robot.LogLevel, m string, v ...interface{}) bool
 	pipelineNameForApproval() string
+	commandNameForApproval() string
 }
 
 func init() {
@@ -32,6 +33,10 @@ func (r Robot) pipelineNameForApproval() string {
 	return r.pipeName
 }
 
+func (r Robot) commandNameForApproval() string {
+	return r.plugCommand
+}
+
 func userApprovalElevate(gr robot.Robot, command string, args ...string) robot.TaskRetVal {
 	if command != "_elevate" {
 		return robot.Normal
@@ -52,6 +57,7 @@ func runUserApproval(r userApprovalRuntime) robot.TaskRetVal {
 
 	requester := canonicalApprovalUser(r.GetMessage().User)
 	pipeName := strings.TrimSpace(r.pipelineNameForApproval())
+	actionName := userApprovalActionName(pipeName, r.commandNameForApproval())
 	approvers := effectiveApprovalApprovers(cfg, pipeName, requester)
 	if len(approvers) == 0 {
 		r.Log(robot.Error, "builtin-userapproval has no eligible approvers for pipeline '%s', requester '%s'", pipeName, requester)
@@ -64,7 +70,7 @@ func runUserApproval(r userApprovalRuntime) robot.TaskRetVal {
 		return robot.Fail
 	}
 
-	choicePrompt := userApprovalChoicePrompt(pipeName, approvers)
+	choicePrompt := userApprovalChoicePrompt(actionName, approvers)
 	choice, ret := r.PromptForReply(userApprovalChoiceMatcher, choicePrompt)
 	if ret != robot.Ok {
 		r.Log(robot.Warn, "builtin-userapproval requester '%s' did not select an approver for pipeline '%s': %s", requester, pipeName, ret)
@@ -78,8 +84,8 @@ func runUserApproval(r userApprovalRuntime) robot.TaskRetVal {
 	}
 
 	answer, ret := r.PromptUserForReply("YesNo", approver,
-		"%s requests approval to run elevated action for '%s'. Reply yes to approve or no to deny.",
-		requester, pipeName)
+		"%s is requesting approval to run command %s - approve? (y/n)",
+		requester, actionName)
 	if ret != robot.Ok {
 		r.Log(robot.Warn, "builtin-userapproval approver '%s' did not respond for requester '%s', pipeline '%s': %s", approver, requester, pipeName, ret)
 		r.Say("Approval request to %s did not complete", approver)
@@ -123,12 +129,24 @@ func canonicalApprovalUser(user string) string {
 	return strings.ToLower(strings.TrimSpace(user))
 }
 
-func userApprovalChoicePrompt(pipeName string, approvers []string) string {
+func userApprovalActionName(pipeName, command string) string {
+	pipeName = strings.TrimSpace(pipeName)
+	command = strings.TrimSpace(command)
+	if pipeName == "" {
+		return command
+	}
+	if command == "" {
+		return pipeName
+	}
+	return pipeName + "/" + command
+}
+
+func userApprovalChoicePrompt(actionName string, approvers []string) string {
 	choices := make([]string, 0, len(approvers))
 	for i, approver := range approvers {
 		choices = append(choices, fmt.Sprintf("%c) %s", 'a'+i, approver))
 	}
-	return fmt.Sprintf("This command requires approval for '%s'. Select one approver: %s", pipeName, strings.Join(choices, ", "))
+	return fmt.Sprintf("Approval required for command %s. Select one approver: %s", actionName, strings.Join(choices, ", "))
 }
 
 func userApprovalApproverForChoice(choice string, approvers []string) (string, bool) {

bot/builtin_userapproval_test.go

@@ -48,9 +48,30 @@ func TestUserApprovalChoiceMapping(t *testing.T) {
 }
 
 func TestUserApprovalChoicePrompt(t *testing.T) {
-	got := userApprovalChoicePrompt("wireguard", []string{"david", "bob", "alice"})
-	want := "This command requires approval for 'wireguard'. Select one approver: a) david, b) bob, c) alice"
+	got := userApprovalChoicePrompt("wireguard/add-device", []string{"david", "bob", "alice"})
+	want := "Approval required for command wireguard/add-device. Select one approver: a) david, b) bob, c) alice"
 	if got != want {
 		t.Fatalf("userApprovalChoicePrompt() = %q, want %q", got, want)
 	}
 }
+
+func TestUserApprovalActionName(t *testing.T) {
+	for _, tc := range []struct {
+		name     string
+		pipeName string
+		command  string
+		want     string
+	}{
+		{"plugin command", "vpn", "add-device", "vpn/add-device"},
+		{"pipeline fallback", "maintenance", "", "maintenance"},
+		{"command fallback", "", "approve", "approve"},
+		{"trimmed", " vpn ", " add-device ", "vpn/add-device"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			got := userApprovalActionName(tc.pipeName, tc.command)
+			if got != tc.want {
+				t.Fatalf("userApprovalActionName(%q, %q) = %q, want %q", tc.pipeName, tc.command, got, tc.want)
+			}
+		})
+	}
+}

bot/robot.go

@@ -86,6 +86,7 @@ func (w *worker) makeRobot() Robot {
 			pipeName:       w.pipeName,
 			pipeDesc:       w.pipeDesc,
 			taskName:       w.taskName,
+			plugCommand:    w.plugCommand,
 			currentTask:    w.currentTask,
 			exclusive:      w.exclusive,
 			exclusiveTag:   w.exclusiveTag,

integration/suites/data/TestUserApprovalElevation.yaml

@@ -21,7 +21,7 @@ flow:
     receive:
       user: alice
       channel: general
-      text_pattern: "This command requires approval for 'gosec'\\. Select one approver: a\\) bob, b\\) carol"
+      text_pattern: "Approval required for command gosec/secelevated\\. Select one approver: a\\) bob, b\\) carol"
   - name: select-carol
     send:
       user: alice
@@ -30,7 +30,7 @@ flow:
   - name: carol-approval-prompt
     receive:
       user: carol
-      text_pattern: "alice requests approval to run elevated action for 'gosec'\\. Reply yes to approve or no to deny\\."
+      text_pattern: "alice is requesting approval to run command gosec/secelevated - approve\\? \\(y/n\\)"
   - name: carol-approves
     send:
       user: carol
@@ -43,3 +43,34 @@ flow:
     receive:
       channel: general
       text_pattern: "SECURITY CHECK: secelevated"
+  - name: request-denied-elevation
+    send:
+      user: alice
+      channel: general
+      text: ;go-sec-elevated
+  - name: requester-chooses-denying-approver
+    receive:
+      user: alice
+      channel: general
+      text_pattern: "Approval required for command gosec/secelevated\\. Select one approver: a\\) bob, b\\) carol"
+  - name: select-bob
+    send:
+      user: alice
+      channel: general
+      text: a
+  - name: bob-approval-prompt
+    receive:
+      user: bob
+      text_pattern: "alice is requesting approval to run command gosec/secelevated - approve\\? \\(y/n\\)"
+  - name: bob-denies
+    send:
+      user: bob
+      text: "no"
+  - name: approval-denied
+    receive:
+      channel: general
+      text_pattern: "Approval denied by bob"
+  - name: protected-command-blocked
+    receive:
+      channel: general
+      text_pattern: "Sorry, this command requires elevation"