Skip to content

ipmicli: wrong message for a repeated -p/-k, redirected runs taken for a terminal on Java 22-24, false --no-escape banner, PowerUp hint, no owner in sensors and fru output #218

Closed

Description

@bertysentry

Five small defects of ipmicli, all in the CLI layer (org.metricshub.ipmi.cli): two misleading messages, a terminal detection that is wrong on Java 22 to 24, a banner that names a way out that does not exist, and an output that drops the owner of sensors and FRUs. None of them affects the library API.

What happens:

  1. A repeated -p or -k is reported as mutually exclusive with the file option. Each of -p/--password and -pf/--password-file calls exclusive(passwordOption, "--password and --password-file"), which fails as soon as any password option was given before, the same one included (CliArguments.java:182-198, :481-485). The key options do the same (:199-212). Reproduced offline:

    $ ipmicli -h bmc.example.com -u monitor --password=a -p b chassis
    ipmicli: --password and --password-file are mutually exclusive          (exit 64)
    $ ipmicli -h bmc.example.com -u monitor -k 01 -k 02 -p x chassis
    ipmicli: --bmc-key and --bmc-key-file are mutually exclusive            (exit 64)
    

    Neither command line has a file option. Refusing the repetition is right (a secret given twice is likely a mistake), but the message sends the user looking for an option they did not type. Other repeated options, such as -h, silently take the last value.

  2. On Java 22 to 24, redirected runs are taken for a terminal. main() passes System.console() != null as terminal (IpmiCli.java:124), and the password prompt relies on a null console to refuse (:1346-1351). Up to Java 21, System.console() is null when stdin or stdout is redirected. Java 22 made the JLine provider (jdk.internal.le) the default (JDK-8308591), and System.console() then returns a Console even when both streams are redirected; Console.isTerminal() was added to tell. Java 25 went back to the java.base console (JDK-8351435). The launcher accepts any Java 8 or later.

    • Password: reproduced offline on Java 21.0.2 with -Djdk.console=jdk.internal.le, the default provider of 22 to 24. ipmicli -h bmc.example.com -u monitor chassis </dev/null >out.txt prints Password: ipmicli: no password was entered, and echo secret | ipmicli -h <host> -u monitor chassis prints Password: and goes on to log in with secret, the first line of the piped input, as the password. Java 21 and 25 print no interactive console to ask the password: use --password-file (exit 64) in both cases, as cli.md:142-145 says.
    • SOL: with terminal true, sol takes the interactive path for a piped input (IpmiCli.java:907, :923-941): it runs stty raw -echo on /dev/tty when there is one (:1140) and prints the banner; where raw mode cannot be set (no /dev/tty: cron, CI) it falls back to line mode, which re-reads the input line by line through Console.readPassword(), appends a CR to every line and turns LF into CR (SolBridge.java:186-196). cli.md:500-501 promises that a redirected input is "forwarded as they come". The SOL part is traced from the code, not run (no Java 22 to 24 installed).
  3. --no-escape on a terminal: the banner says "the end of the input closes it", but nothing does. The banner is printed whenever terminal is true, and with --no-escape it ends with that sentence (IpmiCli.java:923-935). In raw mode, Ctrl+D and Ctrl+C are bytes for the BMC (stty raw -echo; the Windows key reader treats Ctrl+C as input), so no end of input ever comes, and SolBridge closes only on the end of the input or on an escape sequence. The way out is a kill from another terminal on Linux and macOS, Ctrl+Break on Windows. The sentence is right only in the line-mode fallback (Ctrl+D ends readPassword()) and for a redirected input, the case cli.md:487-488 describes ("for binary transfers").

  4. power cycle of a system that is off names a Java constant. IpmiClient.chassisControl() refuses the cycle with IllegalStateException("The system is off: a power cycle would power it up on some BMCs, use PowerUp to power it up") (IpmiClient.java:302-305), and IpmiCli prints it unchanged with exit 70 (IpmiCli.java:282-290, :193-195; asserted in IpmiCliTest.java:251-253). The CLI user sees PowerUp where the action is power on. completionCodeFailure() already replaces the Java-oriented hint of D4h with CLI options (IpmiCli.java:208-224); this message was left out.

  5. sensors --json and fru print no owner.

What should happen: a repeated option is reported as repeated; a run whose standard streams are redirected never prompts and forwards its SOL input byte for byte, on every Java version the launcher accepts; the SOL banner names a way out that works; CLI messages name CLI actions; and the JSON output carries what a script needs to tell two sensors or two FRUs apart and to match a SEL entry to its sensor as SelEntry.isFrom() does.

Impact: all low. (1) and (4) mislead a user at the command line. (2) affects only Java 22, 23 and 24, all non-LTS releases superseded by 25, but there it breaks the documented scripting cases: a redirected run takes the first line of its input as the password instead of failing fast, and a scripted sol alters its input. (3) leaves a terminal user with no way out but a kill. (5) makes the SEL-to-sensor join of the manual unreliable on BMCs with satellite controllers or software sensors.

Suggested fix:

  1. In CliArguments, when the option given before is the same one (-p and --password count as the same), throw <option> is given twice; keep the mutual-exclusion message for -p with -pf and -k with -kf.
  2. In IpmiCli.main(), take a console as a terminal only when it is one: Console console = System.console(); boolean terminal = console != null && isTerminal(console);, where isTerminal() calls Console.isTerminal() by reflection when it exists (Java 22+) and returns true otherwise (Java 8 to 21, where a non-null console is a terminal). Make readConsolePassword() refuse a non-terminal console with the same "no interactive console" message. (Setting jdk.console=java.base before the first System.console() call would also do, but depends on when the JDK initializes the console.)
  3. Refuse --no-escape when the input is a terminal: --no-escape needs a redirected input: on a terminal, ~. is the only way out (use --escape to pick another character). If keeping it, word the banner for raw mode instead ("nothing typed closes it: kill the program from another terminal, or press Ctrl+Break on Windows").
  4. In the POWER case of IpmiCli, catch the IllegalStateException of a PowerCycle and print ipmicli: the system is off: a power cycle would power it up on some BMCs, use 'power on' (exit 70), as completionCodeFailure() does for D4h. Update the assertion of IpmiCliTest.java:253.
  5. Add the owner to each sensors --json object: ownerId encoded like the low byte of the SEL generatorId (7-bit address shifted left, bit 0 set for a system software ID), ownerLun and ownerChannel. Add address, lun and channel to the fru --json object, and put them in the text header and the problem lines when the FRU is not the BMC's (20h, LUN 0). Then update cli.md:442-444 to join on the owner as well as the number, and the fru section of cli.md.

Found during the documentation review of #201.

Metadata

Metadata

Assignees

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