[resotoshell][feat] Do not store passwords in shell history (#1625)

resotocore/resotocore/cli/command.py

@@ -148,6 +148,7 @@
     respond_cytoscape,
 )
 from resotocore.worker_task_queue import WorkerTask, WorkerTaskName
+from resotolib.core import CLIEnvelope
 from resotolib.parse_util import (
     double_quoted_or_simple_string_dp,
     space_dp,
@@ -4436,7 +4437,7 @@ async def list_configs() -> Tuple[int, Stream]:
             return CLISource.single(
                 partial(edit_config, config_id),
                 produces=MediaType.FilePath,
-                envelope={"Resoto-Shell-Action": "edit", "Resoto-Shell-Command": f"configs update {config_id}"},
+                envelope={CLIEnvelope.action: "edit", CLIEnvelope.command: f"configs update {config_id}"},
             )
         elif arg and len(args) == 3 and args[0] == "copy":
             return CLISource.single(partial(copy_config, args[1], args[2]))
@@ -4445,7 +4446,7 @@ async def list_configs() -> Tuple[int, Stream]:
             return CLISource.single(
                 partial(update_config, config_id),
                 produces=MediaType.FilePath,
-                envelope={"Resoto-Shell-Action": "edit", "Resoto-Shell-Command": f"configs update {config_id}"},
+                envelope={CLIEnvelope.action: "edit", CLIEnvelope.command: f"configs update {config_id}"},
                 requires=[CLIFileRequirement("config.yaml", args[2])],
             )
         elif arg and len(args) == 1 and args[0] == "list":
@@ -5060,14 +5061,14 @@ async def app_run(
             return CLISource.single(
                 partial(edit_app, app_name),
                 produces=MediaType.FilePath,
-                envelope={"Resoto-Shell-Action": "edit", "Resoto-Shell-Command": f"apps update {app_name}"},
+                envelope={CLIEnvelope.action: "edit", CLIEnvelope.command: f"apps update {app_name}"},
             )
         elif arg and len(args) == 3 and args[0] == "update":
             app_name = InfraAppName(args[1])
             return CLISource.single(
                 partial(update_app, app_name),
                 produces=MediaType.FilePath,
-                envelope={"Resoto-Shell-Action": "edit", "Resoto-Shell-Command": f"apps update {app_name}"},
+                envelope={CLIEnvelope.action: "edit", CLIEnvelope.command: f"apps update {app_name}"},
                 requires=[CLIFileRequirement("manifest.yaml", args[2])],
             )
         elif len(args) == 2 and args[0] == "uninstall":
@@ -5285,7 +5286,8 @@ def parse(self, arg: Optional[str] = None, ctx: CLIContext = EmptyContext, **kwa
             parser.add_argument("--role", type=str, action="append", required=True)
             parsed = parser.parse_args(args[1:])
             return CLISource.single(
-                partial(self.add_user, Email(parsed.email), parsed.fullname, Password(parsed.password), parsed.role)
+                partial(self.add_user, Email(parsed.email), parsed.fullname, Password(parsed.password), parsed.role),
+                envelope={CLIEnvelope.no_history: "yes"},
             )
         elif len(args) == 2 and args[0].startswith("del"):
             return CLISource.single(partial(self.delete_user, Email(args[1])))
@@ -5294,7 +5296,10 @@ def parse(self, arg: Optional[str] = None, ctx: CLIContext = EmptyContext, **kwa
         elif len(args) > 3 and args[0] == "role" and args[1].startswith("del"):
             return CLISource.single(partial(self.delete_role, Email(args[2]), args[3]))
         elif len(args) >= 3 and args[0] in ("passwd", "password"):
-            return CLISource.single(partial(self.change_password, Email(args[1]), Password(args[2])))
+            return CLISource.single(
+                partial(self.change_password, Email(args[1]), Password(args[2])),
+                envelope={CLIEnvelope.no_history: "yes"},
+            )
         elif len(args) == 1 and args[0] == "list":
             return CLISource.no_count(self.list_users)
         elif len(args) == 2 and args[0] == "show":

resotocore/resotocore/web/api.py

@@ -1157,7 +1157,7 @@ async def execute_parsed(
                 gen = await force_gen(streamer)
                 if first_result.produces.text:
                     text_gen = ctx.text_generator(first_result, gen)
-                    return await self.stream_response_from_gen(request, text_gen, count)
+                    return await self.stream_response_from_gen(request, text_gen, count, first_result.envelope)
                 elif first_result.produces.file_path:
                     await mp_response.prepare(request)
                     await Api.multi_file_response(first_result, gen, boundary, mp_response)

resotolib/resotolib/core/__init__.py

@@ -7,6 +7,23 @@
 from typing import Optional
 
 
+class CLIEnvelope:
+    """
+    Envelope fields that are used by the CLI.
+    Those fields are encoded as HTTP Headers into the HTTP response.
+    """
+
+    # Defines the action that should be performed.
+    # Use cases:
+    # - "edit": A file that is returned from the core should be opened in an editor.
+    #           The result of the edit should be sent back to the core, identified by the "command" envelope field.
+    action = "Resoto-Shell-Action"
+    # Defines the command that should be executed after the edit was performed.
+    command = "Resoto-Shell-Command"
+    # Do not add this command to the shell history.
+    no_history = "Resoto-Shell-No-History"
+
+
 def add_args(arg_parser: ArgumentParser) -> None:
     arg_parser.add_argument(
         "--resotocore-uri",

resotoshell/resotoshell/__main__.py

@@ -17,7 +17,7 @@
 from resotolib.jwt import add_args as jwt_add_args
 from resotolib.logger import log, setup_logger, add_args as logging_add_args
 from resotoshell import authorized_client
-from resotoshell.promptsession import PromptSession, core_metadata
+from resotoshell.promptsession import PromptSession, core_metadata, ResotoHistory
 from resotoshell.shell import Shell, ShutdownShellError
 
 
@@ -59,25 +59,25 @@ async def check_system_info() -> None:
         await handle_from_stdin(client)
     else:
         cmds, kinds, props = await core_metadata(client)
-        session = PromptSession(cmds=cmds, kinds=kinds, props=props)
-        await repl(client, args, session)
+        history = ResotoHistory.default()
+        session = PromptSession(cmds=cmds, kinds=kinds, props=props, history=history)
+        shell = Shell(client, True, detect_color_system(args), history=history)
+        await repl(shell, session, args)
 
     # update the eventually changed auth token
     await authorized_client.update_auth_header(client)
     await client.shutdown()
 
 
-async def repl(client: ResotoClient, args: Namespace, session: PromptSession) -> None:
+async def repl(shell: Shell, session: PromptSession, args: Namespace) -> None:
     shutdown_event = Event()
-    shell = Shell(client, True, detect_color_system(args))
+
     log.debug("Starting interactive session")
 
     # send the welcome command to the core
-    await shell.handle_command("welcome")
+    await shell.handle_command("welcome", no_history=True)
 
-    event_listener = (
-        None if args.no_events else asyncio.create_task(attach_to_event_stream(client, shell, shutdown_event))
-    )
+    event_listener = None if args.no_events else asyncio.create_task(attach_to_event_stream(shell, shutdown_event))
     try:
         while not shutdown_event.is_set():
             try:
@@ -102,10 +102,10 @@ async def repl(client: ResotoClient, args: Namespace, session: PromptSession) ->
             event_listener.cancel()
 
 
-async def attach_to_event_stream(client: ResotoClient, shell: Shell, shutdown_event: Event) -> None:
+async def attach_to_event_stream(shell: Shell, shutdown_event: Event) -> None:
     while not shutdown_event.is_set():
         try:
-            async for event in client.events({"error"}):
+            async for event in shell.client.events({"error"}):
                 data = event.get("data", {})
                 message = data.get("message")
                 context = ",".join([f"{k}={v}" for k, v in data.items() if k != "message"])

resotoshell/resotoshell/authorized_client.py

@@ -134,7 +134,7 @@ def valid_credentials(self, host: str) -> Optional[tuple[str, str]]:
     def write(self) -> None:
         if self.dirty:
             self.path.parent.mkdir(mode=0o700, parents=True, exist_ok=True)
-            with open(os.open(self.path, os.O_CREAT | os.O_WRONLY, 0o600), "w+") as f:
+            with open(os.open(self.path, os.O_CREAT | os.O_WRONLY, 0o600), "w+", encoding="utf-8") as f:
                 self.config.write(f)
 
     @staticmethod

resotoshell/resotoshell/promptsession.py

@@ -1,15 +1,16 @@
+from __future__ import annotations
 import pathlib
 import re
+import shutil
 from abc import ABC
-
-from attr import evolve
-from attrs import define, field
 from re import Pattern
-from math import floor
 from shutil import get_terminal_size
 from typing import Iterable, Optional, List, Dict, Union, Tuple, Callable, Any
 
 import jsons
+from attr import evolve
+from attrs import define, field
+from math import floor
 from prompt_toolkit import PromptSession as PTSession
 from prompt_toolkit.auto_suggest import AutoSuggestFromHistory
 from prompt_toolkit.completion import (
@@ -23,7 +24,7 @@
     FuzzyCompleter,
 )
 from prompt_toolkit.document import Document
-from prompt_toolkit.history import FileHistory
+from prompt_toolkit.history import FileHistory, History
 from prompt_toolkit.styles import Style
 from resotoclient.async_client import ResotoClient
 from resotoclient.models import Property
@@ -725,6 +726,35 @@ def get_completions(self, document: Document, complete_event: CompleteEvent) ->
             return []
 
 
+class ResotoHistory(History):
+    def __init__(self, history_file: str) -> None:
+        super().__init__()
+        self.file_history = FileHistory(history_file)
+
+    def load_history_strings(self) -> Iterable[str]:
+        return self.file_history.load_history_strings()
+
+    def store_string(self, string: str) -> None:
+        # called by prompt_toolkit - ignore this call.
+        # we store the string in the store_command method which is called from Shell
+        pass
+
+    def store_command(self, string: str) -> None:
+        self.file_history.store_string(string)
+
+    @staticmethod
+    def default() -> ResotoHistory:
+        # this path existed until 3.5.0
+        old_path = pathlib.Path.home() / ".resotoshell_history"
+        path = pathlib.Path.home() / ".resoto" / "shell_history"
+        # make sure the directory exists
+        path.parent.mkdir(mode=0o700, parents=True, exist_ok=True)
+        # move the old history file to the new location
+        if old_path.exists() and not path.exists():
+            shutil.move(str(old_path), str(path))
+        return ResotoHistory(str(path))
+
+
 class PromptSession:
     style = Style.from_dict(
         {
@@ -736,14 +766,7 @@ class PromptSession:
 
     prompt_message = [("class:prompt", "> ")]
 
-    def __init__(
-        self,
-        cmds: List[CommandInfo],
-        kinds: List[str],
-        props: List[str],
-        history_file: str = str(pathlib.Path.home() / ".resotoshell_history"),
-    ):
-        history = FileHistory(history_file)
+    def __init__(self, cmds: List[CommandInfo], kinds: List[str], props: List[str], history: History):
         _, tty_rows = get_terminal_size(fallback=(80, 25))
         reserved_row_ratio = 1 / 4
         min_reserved_rows = 4

resotoshell/resotoshell/shell.py

@@ -17,8 +17,10 @@
 from rich.markdown import Markdown
 
 from resotolib.args import ArgumentParser
+from resotolib.core import CLIEnvelope
 from resotolib.logger import log
 from resotolib.utils import sha256sum
+from resotoshell.promptsession import ResotoHistory
 from resotoshell.protected_files import validate_paths
 
 color_system_to_color_depth = {
@@ -40,10 +42,13 @@ def __init__(
         client: ResotoClient,
         tty: bool,
         color_system: str,
+        *,
         graph: Optional[str] = None,
         section: Optional[str] = None,
+        history: Optional[ResotoHistory] = None,
     ):
         self.client = client
+        self.history = history
         self.tty = tty
         self.color_system = color_system
         self.color_depth = color_system_to_color_depth.get(color_system) or ColorDepth.DEPTH_8_BIT
@@ -55,6 +60,7 @@ async def handle_command(
         command: str,
         additional_headers: Optional[Dict[str, str]] = None,
         files: Optional[Dict[str, str]] = None,
+        no_history: bool = False,
     ) -> None:
         headers: Dict[str, str] = {}
         headers.update({"Accept": "text/plain"})
@@ -77,6 +83,9 @@ async def handle_response(maybe: Optional[HttpResponse], upload: bool = False) -
             if maybe is not None:
                 with maybe as response:
                     if response.status_code == 200:
+                        # only store history if the command was successful, and no no_history flag was set
+                        if self.history and not no_history and CLIEnvelope.no_history not in response.headers:
+                            self.history.store_command(command)
                         await self.handle_result(response)
                     elif response.status_code == 424 and not upload:
                         js_data = await response.json()
@@ -141,8 +150,8 @@ async def store_file(response: Union[HttpResponse, aiohttp.BodyPartReader], dire
             return filename, filepath
 
         content_type = response.headers.get("Content-Type", "text/plain")
-        action = response.headers.get("Resoto-Shell-Action")
-        command = response.headers.get("Resoto-Shell-Command")
+        action = response.headers.get(CLIEnvelope.action)
+        command = response.headers.get(CLIEnvelope.command)
         line_delimiter = "---"
 
         # If we get a plain text result, we simply print it to the console.