feat: fail fast on wrong password for non-TTY start

src/keri/cli/commands/witness/start.py

@@ -61,36 +61,24 @@
 parser.add_argument("--logfile", action="store", required=False, default=None,
                     help="DEPRECATED: use --logdir. Path to a log file; only its directory is used "
                          "(the file name and log subdirectory are derived internally).")
+parser.add_argument("--no-prompt", action="store_true", default=None, required=False,
+                    help="Disable interactive prompt", dest="noPrompt")
 
 
 def launch(args):
-    # Normalize (.upper()) so getLevelName returns a numeric level; a lowercase
-    # value would otherwise become the invalid string "Level debug" and silently
-    # break level filtering. The ogler.getLogger() call below applies this level to
-    # the shared logger every keri module holds. (Fatal startup failures are logged
-    # at CRITICAL in runWitness so they remain visible at the default level.)
+    # Normalize level case; avoid invalid lowercase values that silently break level filtering
     ogler.level = logging.getLevelName(args.loglevel.upper())
 
     logdir = args.logdir
-    deprecatedLogfile = args.logfile is not None
-    if deprecatedLogfile:
-        # --logfile is deprecated: ogler derives the log file name from --name and
-        # its subdirectory from its own prefix, so only the directory is meaningful.
+    if args.logfile:
+        print(f"--logfile is deprecated; use --logdir. Logging to {ogler.path}", file=sys.stderr)
         logdir = os.path.dirname(args.logfile) or "."
 
-    if logdir is not None:
+    if logdir is not None:  # Must reopen
         ogler.headDirPath = logdir
         ogler.reopen(name=args.name, temp=False, clear=True)
 
-    # Re-fetch so the configured level and any newly attached file handler are
-    # applied to the shared logger used throughout this command.
-    ogler.getLogger()
-
-    if deprecatedLogfile:
-        # Print (not log) so the notice is visible regardless of --loglevel, and
-        # report the actual resolved path so operators know where the log landed.
-        print(f"kli witness start: --logfile is deprecated; use --logdir. "
-              f"Logging to {ogler.path}", file=sys.stderr)
+    ogler.getLogger()  # re-applies ogler.level, replaces handlers, binding to "logger" var
 
     logger.info("\n******* Starting Witness for %s listening: http/%s, tcp/%s "
                 ".******\n\n", args.name, args.http, args.tcp)
@@ -105,17 +93,20 @@ def launch(args):
                configFile=args.configFile,
                keypath=args.keypath,
                certpath=args.certpath,
-               cafilepath=args.cafilepath)
+               cafilepath=args.cafilepath,
+               noPrompt=args.noPrompt)
 
     logger.info("\n******* Ended Witness for %s listening: http/%s, tcp/%s"
                 ".******\n\n", args.name, args.http, args.tcp)
 
 
 def runWitness(name="witness", base="", alias="witness", bran="", tcp=5631, http=5632, expire=0.0,
-               configDir="", configFile=None, keypath=None, certpath=None, cafilepath=None):
+               configDir="", configFile=None, keypath=None, certpath=None, cafilepath=None,
+               noPrompt=None):
     """
     Setup and run one witness
     """
+    noPrompt = noPrompt if noPrompt is not None else not sys.stdin.isatty()  # When not TTY then do not prompt
 
     ks = Keeper(name=name,
                 base=base,
@@ -134,14 +125,11 @@ def runWitness(name="witness", base="", alias="witness", bran="", tcp=5631, http
         if aeid is None:
             hby = Habery(name=name, base=base, bran=bran, cf=cf)
         else:
-            # Encrypted keystore requires a passcode. Only prompt interactively when
-            # attached to a TTY; a witness started from a script/service with no
-            # passcode must fail fast instead of stalling on getpass for input.
-            if not bran and not sys.stdin.isatty():
-                raise AuthError(f"Witness {name!r} keystore is encrypted but no passcode "
-                                f"was provided and stdin is not a TTY; pass --passcode "
-                                f"to start non-interactively.")
-            hby = setupHby(name=name, base=base, bran=bran, cf=cf)
+            # Prompt only for TTY; fail fast for no passcode on script/service witness start
+            if not bran and noPrompt:
+                raise AuthError(f"passcode required for keystore {name!r} but prompting is disabled. "
+                                f"pass --passcode or omit --no-prompt in an interactive terminal..")
+            hby = setupHby(name=name, base=base, bran=bran, cf=cf, noPrompt=noPrompt)
 
         hbyDoer = HaberyDoer(habery=hby)  # setup doer
         doers = [hbyDoer]
@@ -156,14 +144,10 @@ def runWitness(name="witness", base="", alias="witness", bran="", tcp=5631, http
 
         runController(doers=doers, expire=expire)
     except Exception:
-        # Log at CRITICAL (with the traceback via exc_info) so a failed start is
-        # visible even at the default --loglevel CRITICAL; a lower level would be
-        # suppressed. Without this the failure surfaces only as a bare traceback
-        # or a terse "ERR:" print from the CLI dispatcher.
+        # Log startup failures at CRITICAL with traceback.
         logger.critical("Witness %r failed to start", name, exc_info=True)
         raise
     finally:
-        # Release the keystore/database LMDB env so a failed start does not leave
-        # a stale lock that would block the next start attempt.
+        # Close Habery LMDB to avoid leftover locks on failed start blocking next start attempt
         if hby is not None:
             hby.close()

src/keri/cli/common/existing.py

@@ -12,7 +12,8 @@
 from ...app import Habery, Keeper
 from keri.kering import Version
 
-def setupHby(name, base="", bran=None, cf=None, temp=False, version=None):
+
+def setupHby(name, base="", bran=None, cf=None, temp=False, noPrompt=False, version=None):
     """ Create Habery off of existing directory
 
     Parameters:
@@ -21,6 +22,7 @@ def setupHby(name, base="", bran=None, cf=None, temp=False, version=None):
         bran(str): optional passcode if the Habery was created encrypted
         cf (Configer): optional configuration for loading reference data
         temp (bool): True means create database in /tmp
+        noPrompt (bool): True means not prompt. Default to False
 
     Returns:
           Habery:  the configured habery
@@ -49,6 +51,8 @@ def setupHby(name, base="", bran=None, cf=None, temp=False, version=None):
             break
         except (AuthError, ValueError) as e:
             print(e)
+            if noPrompt:
+                raise e
             if retries >= 3:
                 raise AuthError("too many attempts")
             print("Valid passcode required, try again...")

tests/app/cli/test_kli_commands.py

@@ -711,6 +711,7 @@ def test_run_failure_is_logged_and_hby_closed(helpers, monkeypatch):
     name = "bug238witerr"
     helpers.remove_test_dirs(name)
 
+    # set up CRITICAL logger so we can assert its contents
     logged = []
     monkeypatch.setattr(witness_start.logger, "critical",
                         lambda *a, **k: logged.append((a, k)))
@@ -740,26 +741,49 @@ def _boom(**kw):
 
 
 def test_encrypted_keystore_non_tty_fails_fast(helpers, monkeypatch):
-    """An encrypted keystore started with no passcode on a non-TTY must fail fast
-    with a logged AuthError instead of stalling on an interactive getpass prompt."""
+    """Starting encrypted keystore with no passcode on a non-TTY must fail fast with no prompt"""
     name = "bug238witenc"
+
+    # ensure TTY false regardless of test harness start (sometimes IDE starts set TTY=True)
+    monkeypatch.setattr("sys.stdin.isatty", lambda: False)
+
+    # Defensive raise on getpass, testing that we never call it for this test since no TTY
+    def _boom(*a, **k):
+        raise AssertionError("getpass must not be called on a non-TTY start")
+    monkeypatch.setattr("keri.cli.common.existing.getpass.getpass", _boom)
+
     helpers.remove_test_dirs(name)
     try:
         # create an encrypted keystore so that aeid is set
         hby = habbing.Habery(name=name, base="", bran="0123456789abcdefghijk", temp=False)
         hby.close()
 
-        # guarantee the non-interactive condition regardless of how the test runner
-        # wires stdin
-        monkeypatch.setattr("sys.stdin.isatty", lambda: False)
+        with pytest.raises(AuthError, match="passcode required"):
+            witness_start.runWitness(name=name, base="", alias="wit", bran="")
+    finally:
+        helpers.remove_test_dirs(name)
 
-        # the guard must short-circuit *before* any interactive prompt; if getpass
-        # is ever reached the witness could block waiting on stdin (the stall bug)
-        def _boom(*a, **k):
-            raise AssertionError("getpass must not be called on a non-TTY start")
-        monkeypatch.setattr("keri.cli.common.existing.getpass.getpass", _boom)
 
-        with pytest.raises(AuthError):
-            witness_start.runWitness(name=name, base="", alias="wit", bran="")
+def test_witness_start_non_tty_wrong_passcode_raises(helpers, monkeypatch):
+    """When starting a witness with no TTY and with wrong passcode, should err."""
+    name = "bug238witwrong"
+    correct = "0123456789abcdefghijk"
+    wrong = "thisisasecretpasscode"
+
+    # ensure TTY false regardless of test harness start (sometimes IDE starts set TTY=True)
+    monkeypatch.setattr("sys.stdin.isatty", lambda: False)
+
+    helpers.remove_test_dirs(name)
+    try:
+        # Create keystore with correct passcode to later trigger noPrompt + wrong passcode err
+        hby = habbing.Habery(name=name, base="", bran=correct, temp=False)
+        hby.close()
+
+        args = _parse(["witness", "start", "--name", name, "--alias", "wit",
+                       "--passcode", wrong,
+                       "--loglevel", "debug", "--logdir", "/tmp/wlogs"])
+        with pytest.raises(AuthError, match="Last seed missing"):
+            args.handler(args)
     finally:
         helpers.remove_test_dirs(name)
+