fix: address 7 consistency findings from code review
- Counter overflow: drop len==2 guard in build_ovpn so suffixes grow naturally beyond _99 instead of silently resetting to _00 - TOGGLE_ALL: preserve checked set and cursor across A-key view switches by threading saved_checked/saved_cursor through ScreenResult and back into show_main_screen on the next call - CLI Cryptgeon fallback: skip email/eml when Cryptgeon fails instead of embedding the raw plaintext password in the email body - --show-eml silent skip: warn explicitly when --show-eml is requested but no email address is available - URL truncation: show full one-time URL in TUI confirmation dialog - _fatal(): remove misleading alias; inline self._error() + return at the sole call site so termination is explicit - argparse: replace store_true/store_false with store_const for the three-state --send-email / --no-send-email / neither pattern Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -277,7 +277,7 @@ def build_ovpn(
|
|||||||
int(d.name[len(prefix):])
|
int(d.name[len(prefix):])
|
||||||
for d in base.iterdir()
|
for d in base.iterdir()
|
||||||
if d.is_dir() and d.name.startswith(prefix)
|
if d.is_dir() and d.name.startswith(prefix)
|
||||||
and d.name[len(prefix):].isdigit() and len(d.name[len(prefix):]) == 2
|
and d.name[len(prefix):].isdigit()
|
||||||
] if base.exists() else []
|
] if base.exists() else []
|
||||||
next_n = max(existing) + 1 if existing else 0
|
next_n = max(existing) + 1 if existing else 0
|
||||||
out_dir = base / f"{prefix}{next_n:02d}"
|
out_dir = base / f"{prefix}{next_n:02d}"
|
||||||
@@ -661,6 +661,8 @@ class Action(Enum):
|
|||||||
class ScreenResult:
|
class ScreenResult:
|
||||||
action: Action
|
action: Action
|
||||||
selected_cns: list[str] = field(default_factory=list)
|
selected_cns: list[str] = field(default_factory=list)
|
||||||
|
saved_checked: list[str] = field(default_factory=list)
|
||||||
|
saved_cursor: int = 0
|
||||||
|
|
||||||
|
|
||||||
_MENU_NEW = "Create New Certificate"
|
_MENU_NEW = "Create New Certificate"
|
||||||
@@ -678,7 +680,9 @@ def _expiry_label(c: CertInfo) -> str:
|
|||||||
|
|
||||||
|
|
||||||
def show_main_screen(
|
def show_main_screen(
|
||||||
stdscr, certs: list[CertInfo], show_all: bool = False
|
stdscr, certs: list[CertInfo], show_all: bool = False,
|
||||||
|
initial_checked: Optional[set[str]] = None,
|
||||||
|
initial_cursor: int = 0,
|
||||||
) -> ScreenResult:
|
) -> ScreenResult:
|
||||||
"""Display cert selection list. Returns when user activates an action."""
|
"""Display cert selection list. Returns when user activates an action."""
|
||||||
init_colors()
|
init_colors()
|
||||||
@@ -686,8 +690,8 @@ def show_main_screen(
|
|||||||
|
|
||||||
n_certs = len(certs)
|
n_certs = len(certs)
|
||||||
n_items = n_certs + len(_MENU_ITEMS)
|
n_items = n_certs + len(_MENU_ITEMS)
|
||||||
checked: set[str] = set()
|
checked: set[str] = set(initial_checked) if initial_checked else set()
|
||||||
cursor = 0
|
cursor = clamp(initial_cursor, 0, max(0, n_items - 1))
|
||||||
|
|
||||||
def is_cert(idx): return idx < n_certs
|
def is_cert(idx): return idx < n_certs
|
||||||
def menu_item(idx): return _MENU_ITEMS[idx - n_certs] if idx >= n_certs else None
|
def menu_item(idx): return _MENU_ITEMS[idx - n_certs] if idx >= n_certs else None
|
||||||
@@ -786,7 +790,9 @@ def show_main_screen(
|
|||||||
# _MENU_NEW when checked → grayed, ignore
|
# _MENU_NEW when checked → grayed, ignore
|
||||||
|
|
||||||
elif key in (ord("a"), ord("A")):
|
elif key in (ord("a"), ord("A")):
|
||||||
return ScreenResult(action=Action.TOGGLE_ALL)
|
return ScreenResult(action=Action.TOGGLE_ALL,
|
||||||
|
saved_checked=sorted(checked),
|
||||||
|
saved_cursor=cursor)
|
||||||
|
|
||||||
elif key in (ord("r"), ord("R")) and is_cert(cursor):
|
elif key in (ord("r"), ord("R")) and is_cert(cursor):
|
||||||
cert = certs[cursor]
|
cert = certs[cursor]
|
||||||
@@ -811,6 +817,8 @@ class CursesApp:
|
|||||||
init_colors()
|
init_colors()
|
||||||
|
|
||||||
show_all = False
|
show_all = False
|
||||||
|
saved_checked: list[str] = []
|
||||||
|
saved_cursor: int = 0
|
||||||
|
|
||||||
while True:
|
while True:
|
||||||
try:
|
try:
|
||||||
@@ -820,15 +828,21 @@ class CursesApp:
|
|||||||
else load_expiring_certs(EASYRSA_PKI_DIR, DAYS_PAST, DAYS_AHEAD)
|
else load_expiring_certs(EASYRSA_PKI_DIR, DAYS_PAST, DAYS_AHEAD)
|
||||||
)
|
)
|
||||||
except FileNotFoundError as exc:
|
except FileNotFoundError as exc:
|
||||||
self._fatal(stdscr, f"Cannot read PKI index.txt:\n{exc}")
|
self._error(stdscr, f"Cannot read PKI index.txt:\n{exc}")
|
||||||
return
|
return
|
||||||
|
|
||||||
result = show_main_screen(stdscr, certs, show_all=show_all)
|
result = show_main_screen(stdscr, certs, show_all=show_all,
|
||||||
|
initial_checked=set(saved_checked),
|
||||||
|
initial_cursor=saved_cursor)
|
||||||
|
saved_checked = []
|
||||||
|
saved_cursor = 0
|
||||||
|
|
||||||
if result.action == Action.EXIT:
|
if result.action == Action.EXIT:
|
||||||
return
|
return
|
||||||
elif result.action == Action.TOGGLE_ALL:
|
elif result.action == Action.TOGGLE_ALL:
|
||||||
show_all = not show_all
|
show_all = not show_all
|
||||||
|
saved_checked = result.saved_checked
|
||||||
|
saved_cursor = result.saved_cursor
|
||||||
elif result.action == Action.REGEN_CRL:
|
elif result.action == Action.REGEN_CRL:
|
||||||
self._regen_crl(stdscr)
|
self._regen_crl(stdscr)
|
||||||
elif result.action == Action.REVOKE:
|
elif result.action == Action.REVOKE:
|
||||||
@@ -900,7 +914,7 @@ class CursesApp:
|
|||||||
stdscr,
|
stdscr,
|
||||||
f"Send email to {form.email}?\n\n"
|
f"Send email to {form.email}?\n\n"
|
||||||
f"Attachment : {os.path.basename(ovpn_path)}\n"
|
f"Attachment : {os.path.basename(ovpn_path)}\n"
|
||||||
f"Password URL: {one_time_url[:50]}…",
|
f"Password URL: {one_time_url}",
|
||||||
):
|
):
|
||||||
try:
|
try:
|
||||||
send_email(form.email, final_cn, one_time_url, ovpn_path,
|
send_email(form.email, final_cn, one_time_url, ovpn_path,
|
||||||
@@ -980,9 +994,6 @@ class CursesApp:
|
|||||||
stdscr.refresh()
|
stdscr.refresh()
|
||||||
stdscr.getch()
|
stdscr.getch()
|
||||||
|
|
||||||
def _fatal(self, stdscr, text: str) -> None:
|
|
||||||
self._error(stdscr, text)
|
|
||||||
|
|
||||||
def _show_result(
|
def _show_result(
|
||||||
self, stdscr, cn: str, ovpn_path: str, password: str, one_time_url: str,
|
self, stdscr, cn: str, ovpn_path: str, password: str, one_time_url: str,
|
||||||
) -> None:
|
) -> None:
|
||||||
@@ -1083,22 +1094,31 @@ class CliRunner:
|
|||||||
except CryptgeonError as exc:
|
except CryptgeonError as exc:
|
||||||
print(f"warning: Cryptgeon failed: {exc}", file=sys.stderr)
|
print(f"warning: Cryptgeon failed: {exc}", file=sys.stderr)
|
||||||
|
|
||||||
if show_eml and email_addr:
|
if show_eml:
|
||||||
try:
|
if not email_addr:
|
||||||
msg = build_mime_message(email_addr, cn, one_time_url or password,
|
print("warning: --show-eml skipped (no email address)", file=sys.stderr)
|
||||||
ovpn_path, MAIL_FROM, MAIL_SUBJECT,
|
elif not one_time_url:
|
||||||
EMAIL_TEMPLATE_PATH)
|
print("warning: --show-eml skipped (Cryptgeon failed)", file=sys.stderr)
|
||||||
print(base64.b64encode(msg.as_bytes()).decode())
|
else:
|
||||||
except Exception as exc:
|
try:
|
||||||
print(f"warning: could not build .eml: {exc}", file=sys.stderr)
|
msg = build_mime_message(email_addr, cn, one_time_url,
|
||||||
|
ovpn_path, MAIL_FROM, MAIL_SUBJECT,
|
||||||
|
EMAIL_TEMPLATE_PATH)
|
||||||
|
print(base64.b64encode(msg.as_bytes()).decode())
|
||||||
|
except Exception as exc:
|
||||||
|
print(f"warning: could not build .eml: {exc}", file=sys.stderr)
|
||||||
|
|
||||||
if email_addr and send_email_flag:
|
if email_addr and send_email_flag:
|
||||||
try:
|
if not one_time_url:
|
||||||
send_email(email_addr, cn, one_time_url or password, ovpn_path,
|
print("warning: email skipped (Cryptgeon failed; use password from stdout)",
|
||||||
MAIL_FROM, MAIL_SUBJECT, EMAIL_TEMPLATE_PATH, MAIL_BINARY)
|
file=sys.stderr)
|
||||||
print(f"Email sent to {email_addr}", file=sys.stderr)
|
else:
|
||||||
except RuntimeError as exc:
|
try:
|
||||||
print(f"warning: email failed: {exc}", file=sys.stderr)
|
send_email(email_addr, cn, one_time_url, ovpn_path,
|
||||||
|
MAIL_FROM, MAIL_SUBJECT, EMAIL_TEMPLATE_PATH, MAIL_BINARY)
|
||||||
|
print(f"Email sent to {email_addr}", file=sys.stderr)
|
||||||
|
except RuntimeError as exc:
|
||||||
|
print(f"warning: email failed: {exc}", file=sys.stderr)
|
||||||
|
|
||||||
print(f"config: {ovpn_path}")
|
print(f"config: {ovpn_path}")
|
||||||
if one_time_url:
|
if one_time_url:
|
||||||
@@ -1119,9 +1139,9 @@ def _build_parser() -> argparse.ArgumentParser:
|
|||||||
group.add_argument("--gen-crl", action="store_true", help="Regenerate and copy CRL")
|
group.add_argument("--gen-crl", action="store_true", help="Regenerate and copy CRL")
|
||||||
parser.add_argument("--email", metavar="EMAIL",
|
parser.add_argument("--email", metavar="EMAIL",
|
||||||
help="Email address (required for --create; optional for --reissue)")
|
help="Email address (required for --create; optional for --reissue)")
|
||||||
parser.add_argument("--send-email", dest="send_email", action="store_true",
|
parser.add_argument("--send-email", dest="send_email", action="store_const", const=True,
|
||||||
default=None, help="Send email after issuing cert (default unless --show-eml)")
|
default=None, help="Send email after issuing cert (default unless --show-eml)")
|
||||||
parser.add_argument("--no-send-email", dest="send_email", action="store_false",
|
parser.add_argument("--no-send-email", dest="send_email", action="store_const", const=False,
|
||||||
help="Skip email delivery; print URL to stdout instead")
|
help="Skip email delivery; print URL to stdout instead")
|
||||||
parser.add_argument("--show-eml", action="store_true",
|
parser.add_argument("--show-eml", action="store_true",
|
||||||
help="Print the generated email as base64-encoded .eml to stdout "
|
help="Print the generated email as base64-encoded .eml to stdout "
|
||||||
|
|||||||
Reference in New Issue
Block a user