diff --git a/src/keri/cli/commands/witness/start.py b/src/keri/cli/commands/witness/start.py index ae5d7d8a4..5e9bced08 100644 --- a/src/keri/cli/commands/witness/start.py +++ b/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() diff --git a/src/keri/cli/common/existing.py b/src/keri/cli/common/existing.py index 140deb8b1..659b52996 100644 --- a/src/keri/cli/common/existing.py +++ b/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...") diff --git a/tests/app/cli/test_kli_commands.py b/tests/app/cli/test_kli_commands.py index d91847000..cd34191d7 100644 --- a/tests/app/cli/test_kli_commands.py +++ b/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) +