Skip to content

kosli delete service-account / delete api-key exit 0 without deleting when stdin is not interactive #1056

Description

@JonJagger

Preflight

  • I searched existing issues and didn't find a duplicate.

What happened?

We recently found that unknown CLI commands exited with zero status and no error message.
This was a bad failure as it could have caused a non-compliance to not be surfaced.
So I asked Claude to do a sweep of the CLI looking for further cases where a zero exit code was incorrectly reported.
This is its finding.

Summary

Both delete commands ask for interactive confirmation. The confirmation reader
treats io.EOF as a valid negative answer, so when stdin is not an interactive
terminal (any CI job, < /dev/null, a closed stdin) the command prints its
prompt, immediately reads EOF, reports "cancelled", and exits with code 0.

Nothing is deleted, but the caller is told the command succeeded.

This is the same failure shape as #1043 (fixed in #1051): a command that did not
do what it was asked reports success through its exit code. #1051 closed the
unrecognized-command route; this is a separate route to the same outcome.

Impact

An automated pipeline that revokes credentials cannot detect that the
revocation did not happen. kosli delete service-account <name> in CI exits 0
and the service account remains active, so a script that treats exit 0 as
"credential revoked" proceeds on a false premise. Because the exit code is the
only signal a pipeline consumes, the failure is silent.

Kosli's compliance reporting is deliberately asymmetric: reporting something as
non-compliant when it is actually compliant is acceptable, reporting something
as compliant when it is not must never happen. A revocation step that returns
success without revoking falls on the wrong side of that line.

Steps to reproduce

Any host and token will do; the command never reaches the network.

$ kosli delete service-account some-account --org my-org --api-token <token> < /dev/null
Are you sure you want to delete service account(s) some-account? [y/N] Deletion of service account(s) some-account was cancelled.
$ echo $?
0
$ kosli delete api-key key1 --service-account sa --org my-org --api-token <token> < /dev/null
Are you sure you want to delete API key(s) key1 for service account sa? [y/N] Deletion of API key(s) key1 was cancelled.
$ echo $?
0

Passing -y / --assume-yes behaves correctly: the request is attempted and a
genuine failure exits non-zero.

$ kosli delete service-account sa -y --host http://127.0.0.1:1 --org p --api-token t < /dev/null
Error: [kosli delete service-account] failed to delete service account: ... connection refused
$ echo $?
1

Root cause

cmd/kosli/deleteServiceAccount.go:135

func confirmServiceAccountDeletion(names []string, in io.Reader) (bool, error) {
	logger.Print("Are you sure you want to delete service account(s) %s? [y/N] ", ...)

	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
}

io.EOF is excluded from the error path, so an empty read returns
(false, nil). The caller cannot distinguish "the user declined" from "there
was nobody to ask":

cmd/kosli/deleteServiceAccount.go:84

logger.Info("Deletion of service account(s) %s was cancelled.", ...)
return nil

cmd/kosli/deleteApiKey.go:143 and cmd/kosli/deleteApiKey.go:93 are the same
code shape with the same result.

Exit 0 is the right answer when a human types N. The defect is conflating EOF
with a typed refusal: EOF means the question could not be asked at all.

Expected behaviour

When the prompt cannot be answered (stdin at EOF or not a terminal) and
--assume-yes was not passed, the command should fail with a non-zero exit and
a message pointing at --assume-yes, for example:

Error: cannot confirm deletion: stdin is not interactive. Re-run with --assume-yes to delete without confirmation.

An explicit n / N / empty line typed by a user should keep exiting 0.

Suggested fix

In confirmServiceAccountDeletion and confirmApiKeyDeletion, distinguish EOF
from a real answer. Returning EOF as an error is enough, since the callers
already propagate the error from these functions:

answer, err := bufio.NewReader(in).ReadString('\n')
if err != nil && err != io.EOF {
	return false, err
}
// EOF with nothing read means there was no one to ask, which is not a refusal.
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")
}

Note ReadString returns io.EOF together with any bytes read before it, so a
final line without a trailing newline (printf 'y' | kosli delete ...) must
still be honoured. The TrimSpace(answer) == "" check above preserves that.

Tests worth adding, both driving the existing in io.Reader seam:

  • EOF with empty input returns an error (and the command exits non-zero)
  • printf 'y' with no trailing newline still confirms
  • an explicit n still cancels with exit 0

Scope of the wider audit

While looking for other routes to a false-success exit code, the following were
checked and found sound:

  • Every command in the tree (119 nodes) was invoked with an extra unrecognized
    positional token. All exit non-zero.
  • No runnable group command exists. That matters because findUnknownCommand
    returns early when the resolved command is runnable
    (cmd/kosli/main.go:92), so a runnable group would have been a hole in the
    fix: return non-zero exit for unrecognized commands #1051 fix. There are none.
  • All 15 leaf commands with no cobra Args validator enforce an argument count
    themselves via CustomMaximumNArgs or ValidateArtifactArg.
  • Cobra prefix matching is not enabled, so no token resolves by abbreviation.
  • Unknown --output formats are rejected (internal/output/output.go:17).

One unrelated cosmetic bug spotted: internal/requests/requests.go:270 logs
"failed to log payload: ... Continuing with the request..." via Logger.Error,
which is log.Fatalf and therefore exits 1 rather than continuing. The message
and the behaviour disagree. It fails towards non-compliance, so it is not
urgent, but the message is misleading.

Steps to reproduce

See text

CLI version

v2.36.0

Environment

see text

Logs / output

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions