Skip to content

Commit 56e88cb

Browse files
committed
fix(cli): say so when a rescue password is redirected out of the terminal
CodeQL alert #10 flagged the rescue command printing a freshly generated password as clear-text logging. It is a false positive and was dismissed as one: the value reaches no log sink — it is created by auth.RandomPassword, goes to exactly one fmt.Printf, and the audit row keeps only the action and the login — and printing it to the operator who ran the command is the command's entire purpose. That justification has one hole, and this closes it. Redirect stdout and the password lands in a file or a pipe, where it outlives the moment it was needed and the operator has no reason to suspect a copy exists. When stdout is not a terminal the output now says so and tells them to delete it. The credentials are still printed: refusing would break the escape hatch this command exists to be. Mutation-checked — removing the warning fails the test.
1 parent 435aaed commit 56e88cb

2 files changed

Lines changed: 53 additions & 0 deletions

File tree

cmd/rospanel/cli.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -751,17 +751,38 @@ func rescueAudit(st *store.Store, action, login string, twoFACleared bool) {
751751
}
752752

753753
// printRescueCredentials shows the new credentials once, with the must-change note.
754+
//
755+
// Printing a freshly minted password is the whole point of the rescue command, and it
756+
// goes to the operator's own terminal — not a log sink, and not the audit row, which
757+
// keeps only the action and the login. The one case where that stops being true is a
758+
// caller who redirects stdout, because then the password lands in whatever caught it and
759+
// outlives the moment it was needed. Say so when stdout is not a terminal: the operator
760+
// can still use the credentials, but they now know there is a copy to go and delete.
754761
func printRescueCredentials(login, password string, unlocked bool) {
755762
bar := strings.Repeat("=", 56)
756763
extra := ""
757764
if unlocked {
758765
extra = "\n Two-factor : removed"
759766
}
767+
if !stdoutIsTerminal() {
768+
extra += "\n WARNING : stdout is not a terminal — this password has been written\n" +
769+
" to a file or a pipe. Delete it once you have signed in."
770+
}
760771
fmt.Printf("\n%s\n RESCUE CREDENTIALS (shown once — sign in and change them)\n%s\n"+
761772
" Login : %s\n Password : %s%s\n%s\n",
762773
bar, bar, login, password, extra, bar)
763774
}
764775

776+
// stdoutIsTerminal reports whether stdout is an interactive terminal rather than a file
777+
// or a pipe. Used to warn that a one-time secret just outlived its moment.
778+
func stdoutIsTerminal() bool {
779+
fi, err := os.Stdout.Stat()
780+
if err != nil {
781+
return false // unknown is treated as "not a terminal": warning costs nothing
782+
}
783+
return fi.Mode()&os.ModeCharDevice != 0
784+
}
785+
765786
// checkRestoreArchive refuses an archive that cannot be restored into THIS binary.
766787
//
767788
// Staged inside the data directory, not $TMPDIR: /tmp is a small tmpfs on plenty of

cmd/rospanel/rescue_tty_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package main
2+
3+
import (
4+
"os"
5+
"os/exec"
6+
"strings"
7+
"testing"
8+
)
9+
10+
// The rescue command prints a one-time password to the operator's terminal, which is the
11+
// justification for it not being "clear-text logging". That justification stops holding
12+
// the moment stdout is redirected: the password then lands in a file or a pipe and
13+
// outlives the moment it was needed. Warn, so the operator knows there is a copy.
14+
func TestRescueWarnsWhenStdoutIsRedirected(t *testing.T) {
15+
if os.Getenv("RESCUE_PRINT_CHILD") == "1" {
16+
printRescueCredentials("admin", "s3cret-example", false)
17+
return
18+
}
19+
cmd := exec.Command(os.Args[0], "-test.run=TestRescueWarnsWhenStdoutIsRedirected")
20+
cmd.Env = append(os.Environ(), "RESCUE_PRINT_CHILD=1")
21+
out, err := cmd.Output() // a pipe, i.e. not a terminal
22+
if err != nil {
23+
t.Fatalf("child: %v", err)
24+
}
25+
if !strings.Contains(string(out), "WARNING") {
26+
t.Errorf("no warning when stdout is a pipe — the operator is not told the "+
27+
"password was captured somewhere:\n%s", out)
28+
}
29+
if !strings.Contains(string(out), "s3cret-example") {
30+
t.Error("the credentials themselves stopped being printed — the command is now useless")
31+
}
32+
}

0 commit comments

Comments
 (0)