fix(delete): fail when the deletion prompt cannot be answered

cmd/kosli/apiKey_test.go

@@ -233,10 +233,18 @@ func (suite *ApiKeyCommandTestSuite) TestRotateApiKeyCmd() {
 
 func (suite *ApiKeyCommandTestSuite) TestDeleteApiKeyCmd() {
 	tests := []cmdTestCase{
+		{
+			wantError: true,
+			name:      "an unanswerable prompt (empty stdin) fails instead of reporting success",
+			cmd:       "delete api-key key-123 --service-account test-sa" + suite.defaultKosliArguments,
+			golden: "Are you sure you want to delete API key(s) key-123 for service account test-sa? [y/N] " +
+				"Error: cannot confirm deletion: stdin is not interactive, re-run with --assume-yes to delete without confirmation\n",
+		},
 		{
 			wantError: false,
-			name:      "delete without confirmation (empty stdin) is cancelled and makes no call",
+			name:      "a typed refusal is cancelled and makes no call",
 			cmd:       "delete api-key key-123 --service-account test-sa" + suite.defaultKosliArguments,
+			stdin:     "n\n",
 			golden:    "Are you sure you want to delete API key(s) key-123 for service account test-sa? [y/N] Deletion of API key(s) key-123 was cancelled.\n",
 		},
 		{
@@ -460,6 +468,32 @@ func (suite *ApiKeyCommandTestSuite) TestDeleteApiKeyNotFound() {
 	runTestCmd(suite.T(), tests)
 }
 
+// TestDeleteApiKeyConfirmed stubs a successful delete to verify that an answer
+// typed at the confirmation prompt reaches the API.
+func (suite *ApiKeyCommandTestSuite) TestDeleteApiKeyConfirmed() {
+	fake := httpfake.New()
+	defer fake.Close()
+	fake.NewHandler().
+		Delete("/api/v2/service-accounts/docs-cmd-test-user/test-sa/api-keys/key-123").
+		Reply(200).
+		BodyString(apiKeyFixture(suite.T(), "revoke_success.json"))
+
+	args := fmt.Sprintf(" --host %s --org %s --api-token %s", fake.Server.URL, global.Org, global.ApiToken)
+	tests := []cmdTestCase{
+		{
+			wantError: false,
+			// no trailing newline: ReadString returns the answer together with
+			// io.EOF, which must still be honoured as a confirmation
+			name:        "a typed y without a trailing newline confirms and deletes",
+			cmd:         "delete api-key key-123 --service-account test-sa" + args,
+			stdin:       "y",
+			goldenRegex: `API key key-123 for service account test-sa was deleted!`,
+		},
+	}
+
+	runTestCmd(suite.T(), tests)
+}
+
 // TestApiErrorsAreSurfaced stubs the API with 4xx responses to verify that
 // create/rotate/list surface the server's error message instead of succeeding.
 func (suite *ApiKeyCommandTestSuite) TestApiErrorsAreSurfaced() {

cmd/kosli/cli_utils.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"bufio"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -71,6 +72,38 @@ func style(out io.Writer, s string, codes ...string) string {
 	return strings.Join(codes, "") + s + ansiReset
 }
 
+// confirmDeletion prints prompt (which carries no trailing newline, so the
+// answer is typed on the same line) and reports whether the answer is an
+// affirmative "y"/"yes" (case-insensitive). A typed "n", any other word, or a
+// bare Enter is a refusal and returns (false, nil).
+//
+// Reaching EOF with nothing read means there was nobody to ask — stdin is not
+// interactive — which is an error, not a refusal. Treating it as a refusal made
+// the delete commands exit 0 without deleting anything, so a pipeline could not
+// tell that the deletion had not happened (issue #1056).
+//
+// Blank-looking input therefore splits on whether an answer was submitted at
+// all, not on what it contains: a blank line terminated by a newline (`printf
+// ' \n'`) is a deliberate refusal and exits 0, while blank input terminated by
+// EOF (`printf ' '`) is no submission at all and errors.
+func confirmDeletion(prompt string, in io.Reader) (bool, error) {
+	logger.Print("%s", prompt)
+
+	answer, err := bufio.NewReader(in).ReadString('\n')
+	if err != nil && err != io.EOF {
+		return false, err
+	}
+	// ReadString returns io.EOF together with any bytes read before it, so a
+	// final line without a trailing newline (printf 'y' | kosli delete ...) is
+	// still a real answer; only an empty read means nobody answered.
+	if err == io.EOF && strings.TrimSpace(answer) == "" {
+		return false, fmt.Errorf("cannot confirm deletion: stdin is not interactive, re-run with --assume-yes to delete without confirmation")
+	}
+
+	answer = strings.ToLower(strings.TrimSpace(answer))
+	return answer == "y" || answer == "yes", nil
+}
+
 const (
 	bitbucket   = "Bitbucket"
 	github      = "Github"

cmd/kosli/cli_utils_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	log "github.com/kosli-dev/cli/internal/logger"
@@ -1111,6 +1112,92 @@ func (suite *CliUtilsTestSuite) TestHandleArtifactExpression() {
 	}
 }
 
+// TestConfirmDeletion covers the confirmation prompt used by the delete
+// commands. The critical case is an unanswerable prompt (stdin at EOF with
+// nothing typed): it must be an error, not a refusal, so the command exits
+// non-zero instead of reporting success without deleting (issue #1056).
+func (suite *CliUtilsTestSuite) TestConfirmDeletion() {
+	for _, t := range []struct {
+		name          string
+		input         string
+		wantConfirmed bool
+		wantErr       bool
+	}{
+		{
+			name:    "EOF with nothing typed is not a refusal but an error",
+			input:   "",
+			wantErr: true,
+		},
+		{
+			// the two blank-looking inputs below split on whether an answer was
+			// submitted at all, not on what it contains
+			name:    "blank input ending at EOF was never submitted, so it errors",
+			input:   " ",
+			wantErr: true,
+		},
+		{
+			name:  "a blank line ending in a newline was submitted, so it refuses",
+			input: " \n",
+		},
+		{
+			name:          "an answer without a trailing newline is still honoured",
+			input:         "y",
+			wantConfirmed: true,
+		},
+		{
+			name:          "y confirms",
+			input:         "y\n",
+			wantConfirmed: true,
+		},
+		{
+			name:          "yes confirms",
+			input:         "yes\n",
+			wantConfirmed: true,
+		},
+		{
+			name:          "the answer is case-insensitive",
+			input:         "  YES  \n",
+			wantConfirmed: true,
+		},
+		{
+			name:  "n refuses",
+			input: "n\n",
+		},
+		{
+			name:  "a bare newline refuses",
+			input: "\n",
+		},
+		{
+			name:  "anything else refuses",
+			input: "maybe\n",
+		},
+	} {
+		suite.Run(t.name, func() {
+			infoBuf := new(bytes.Buffer)
+			defer restoreLogger(newTestLoggerWithInfo(infoBuf))()
+
+			confirmed, err := confirmDeletion("Are you sure? [y/N] ", strings.NewReader(t.input))
+
+			require.Equal(suite.T(), t.wantErr, err != nil, "unexpected error: %v", err)
+			if t.wantErr {
+				require.Contains(suite.T(), err.Error(), "--assume-yes",
+					"the error must tell the user how to delete non-interactively")
+			}
+			require.Equal(suite.T(), t.wantConfirmed, confirmed)
+			require.Equal(suite.T(), "Are you sure? [y/N] ", infoBuf.String(),
+				"the prompt is printed without a trailing newline")
+		})
+	}
+}
+
+// restoreLogger swaps the package-level logger for l and returns a function
+// that puts the original back.
+func restoreLogger(l *log.Logger) func() {
+	original := logger
+	logger = l
+	return func() { logger = original }
+}
+
 func Test_prefixEachLine(t *testing.T) {
 	tests := []struct {
 		name            string

cmd/kosli/deleteApiKey.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"bufio"
 	"fmt"
 	"io"
 	"net/http"
@@ -18,7 +17,10 @@ const deleteApiKeyLongDesc = deleteApiKeyShortDesc + `
 
 This permanently deletes the API key(s) identified by KEY-ID. Deletion is immediate and
 cannot be undone. You are asked to confirm before the key is deleted; use
-^--assume-yes^/^--yes^ to skip the confirmation prompt.`
+^--assume-yes^/^--yes^ to skip the confirmation prompt.
+
+When stdin is not interactive (e.g. in CI) the prompt cannot be answered and the
+command fails without deleting anything, so pass ^--assume-yes^ there.`
 
 const deleteApiKeyExample = `
 # delete an API key for a service account (asks for confirmation):
@@ -138,17 +140,10 @@ func styleApiKeyIDs(keyIDs []string) []string {
 }
 
 // confirmApiKeyDeletion prompts the user to confirm deletion and returns true
-// only when the answer is an affirmative "y"/"yes" (case-insensitive). The
-// prompt has no trailing newline so the answer is typed on the same line.
+// only when the answer is an affirmative "y"/"yes" (case-insensitive). It errors
+// when the prompt cannot be answered at all, see confirmDeletion.
 func confirmApiKeyDeletion(keyIDs []string, serviceAccount string, in io.Reader) (bool, error) {
-	logger.Print("Are you sure you want to delete API key(s) %s for service account %s? [y/N] ",
+	prompt := fmt.Sprintf("Are you sure you want to delete API key(s) %s for service account %s? [y/N] ",
 		strings.Join(styleApiKeyIDs(keyIDs), ", "), style(logger.Out, serviceAccount, ansiBold, ansiGreen))
-
-	answer, err := bufio.NewReader(in).ReadString('\n')
-	if err != nil && err != io.EOF {
-		return false, err
-	}
-
-	answer = strings.ToLower(strings.TrimSpace(answer))
-	return answer == "y" || answer == "yes", nil
+	return confirmDeletion(prompt, in)
 }

cmd/kosli/deleteServiceAccount.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"bufio"
 	"fmt"
 	"io"
 	"net/http"
@@ -19,7 +18,10 @@ const deleteServiceAccountLongDesc = deleteServiceAccountShortDesc + `
 This permanently removes the service account(s) identified by SERVICE-ACCOUNT-NAME
 from the organization, along with their API keys. Deletion is immediate and
 cannot be undone. You are asked to confirm before deletion; use
-^--assume-yes^/^--yes^ to skip the confirmation prompt.`
+^--assume-yes^/^--yes^ to skip the confirmation prompt.
+
+When stdin is not interactive (e.g. in CI) the prompt cannot be answered and the
+command fails without deleting anything, so pass ^--assume-yes^ there.`
 
 const deleteServiceAccountExample = `
 # delete a service account (asks for confirmation):
@@ -130,17 +132,10 @@ func styleServiceAccountNames(names []string) []string {
 
 // confirmServiceAccountDeletion prompts the user to confirm deletion and
 // returns true only when the answer is an affirmative "y"/"yes"
-// (case-insensitive). The prompt has no trailing newline so the answer is
-// typed on the same line.
+// (case-insensitive). It errors when the prompt cannot be answered at all, see
+// confirmDeletion.
 func confirmServiceAccountDeletion(names []string, in io.Reader) (bool, error) {
-	logger.Print("Are you sure you want to delete service account(s) %s? [y/N] ",
+	prompt := fmt.Sprintf("Are you sure you want to delete service account(s) %s? [y/N] ",
 		strings.Join(styleServiceAccountNames(names), ", "))
-
-	answer, err := bufio.NewReader(in).ReadString('\n')
-	if err != nil && err != io.EOF {
-		return false, err
-	}
-
-	answer = strings.ToLower(strings.TrimSpace(answer))
-	return answer == "y" || answer == "yes", nil
+	return confirmDeletion(prompt, in)
 }

cmd/kosli/serviceAccount_test.go

@@ -174,10 +174,18 @@ func (suite *ServiceAccountCommandTestSuite) TestUpdateServiceAccountCmd() {
 
 func (suite *ServiceAccountCommandTestSuite) TestDeleteServiceAccountCmd() {
 	tests := []cmdTestCase{
+		{
+			wantError: true,
+			name:      "an unanswerable prompt (empty stdin) fails instead of reporting success",
+			cmd:       "delete service-account ci-bot" + suite.defaultKosliArguments,
+			golden: "Are you sure you want to delete service account(s) ci-bot? [y/N] " +
+				"Error: cannot confirm deletion: stdin is not interactive, re-run with --assume-yes to delete without confirmation\n",
+		},
 		{
 			wantError: false,
-			name:      "delete without confirmation (empty stdin) is cancelled and makes no call",
+			name:      "a typed refusal is cancelled and makes no call",
 			cmd:       "delete service-account ci-bot" + suite.defaultKosliArguments,
+			stdin:     "n\n",
 			golden:    "Are you sure you want to delete service account(s) ci-bot? [y/N] Deletion of service account(s) ci-bot was cancelled.\n",
 		},
 		{
@@ -280,6 +288,15 @@ func (suite *ServiceAccountCommandTestSuite) TestServiceAccountDeleteSuccess() {
 			cmd:         "delete service-account ci-bot --assume-yes" + args,
 			goldenRegex: `service account ci-bot was deleted!`,
 		},
+		{
+			wantError: false,
+			// no trailing newline: ReadString returns the answer together with
+			// io.EOF, which must still be honoured as a confirmation
+			name:        "a typed y without a trailing newline confirms and deletes",
+			cmd:         "delete service-account ci-bot" + args,
+			stdin:       "y",
+			goldenRegex: `service account ci-bot was deleted!`,
+		},
 	}
 
 	runTestCmd(suite.T(), tests)

cmd/kosli/testHelpers.go

@@ -38,15 +38,25 @@ type cmdTestCase struct {
 	goldenJson       []jsonCheck // Use like this for array {"[0].compliant", false}
 	goldenStdout     string      // expected stdout only (exact match, ignored when empty)
 	goldenStderr     string      // expected stderr only (exact match, ignored when empty)
+	stdin            string      // fed to the command's stdin (empty means an immediate EOF)
 	wantError        bool
 	additionalConfig interface{}
 }
 
-// executeCommandC executes a command as a user would and returns the output
-// split into combined, stdout-only, and stderr-only streams.
+// executeCommandC executes a command as a user would, with an empty stdin (any
+// read hits EOF immediately, as it does in CI), and returns the output split
+// into combined, stdout-only, and stderr-only streams.
+func executeCommandC(cmd string) (*cobra.Command, string, string, string, error) {
+	return executeCommandStdinC(cmd, "")
+}
+
+// executeCommandStdinC executes a command as a user would with stdin as its
+// standard input, and returns the output split into combined, stdout-only, and
+// stderr-only streams. Use it for commands that read stdin, e.g. the delete
+// commands' confirmation prompt.
 // This creates a new kosli command that is run, but it cannot be used in other tests
 // because newRootCmd overwrites the global options.
-func executeCommandC(cmd string) (*cobra.Command, string, string, string, error) {
+func executeCommandStdinC(cmd string, stdin string) (*cobra.Command, string, string, string, error) {
 	args, err := shellwords.Parse(cmd)
 	if err != nil {
 		return nil, "", "", "", err
@@ -67,7 +77,7 @@ func executeCommandC(cmd string) (*cobra.Command, string, string, string, error)
 	root.SilenceErrors = false
 	root.SetOut(outWriter)
 	root.SetErr(errWriter)
-	root.SetIn(new(bytes.Buffer))
+	root.SetIn(strings.NewReader(stdin))
 	root.SetArgs(normalizeBoolFlagArgs(root, args))
 
 	c, err := root.ExecuteC()
@@ -89,7 +99,7 @@ func runTestCmd(t *testing.T, tests []cmdTestCase) {
 				t.Error("golden and goldenPath cannot be set together")
 			}
 			t.Logf("running cmd: %s", tt.cmd)
-			_, combined, stdout, stderr, err := executeCommandC(tt.cmd)
+			_, combined, stdout, stderr, err := executeCommandStdinC(tt.cmd, tt.stdin)
 			if (err != nil) != tt.wantError {
 				t.Errorf("error expectation not matched\n\n WANT error is: %t\n\n but GOT: '%v'", tt.wantError, err)
 			}

internal/requests/requests.go

@@ -268,7 +268,10 @@ func (c *Client) Do(p *RequestParams) (*HTTPResponse, error) {
 			c.Logger.Info("payload sent to: %s", req.URL)
 			err := c.PayloadOutput(req, jsonFields, "this is the payload being sent:")
 			if err != nil {
-				c.Logger.Error("failed to log payload: %v \nContinuing with the request...", err)
+				// Logger.Error is fatal (it exits), which would contradict the
+				// message and abort a request only because debug logging of its
+				// payload failed. Warn instead, as with the response body below.
+				c.Logger.Warn("failed to log payload: %v \nContinuing with the request...", err)
 			}
 		}
 		resp, err := c.HttpClient.Do(req)