Преглед на файлове

Fix silent failures in auto-unlock retry and error reporting

bitwarden--auto-cmd reused its recursive-pass parameter as both a
"have we retried" flag and the retry's pre-computed result. When the
vault was still locked after an automatic-unlock retry, every branch
fell through and the function returned nil instead of an error tuple,
indistinguishable downstream from an empty search result. It now uses
a plain boolean flag and always returns a well-formed tuple, reporting
"vault is still locked after automatic unlock" when the retry fails.

bitwarden--handle-message only printed its error message when
PRINT-MESSAGE was set, then always returned nil regardless. Since
bitwarden-search (used by the auth-source backend) calls it without
PRINT-MESSAGE, every failure was completely silent, so a locked vault
or a failed automatic unlock looked identical to "no results found" to
any caller. It now signals a user-error unconditionally, which reports
cleanly both interactively and via `emacsclient --eval` (as
`*ERROR*: ...`), with no changes needed in any calling code.

Also fixes two unrelated docstring issues that crashed the
byte-compiler outright (not just warned): unescaped single quotes in
bitwarden--message, and an over-80-column first line in
bitwarden-get-info-by-id that tripped a byte-compiler docstring-reflow
bug.
bodicsek преди 1 седмица
родител
ревизия
0934c5ef81
променени са 1 файла, в които са добавени 46 реда и са изтрити 39 реда
  1. 46 39
      bitwarden.el

+ 46 - 39
bitwarden.el

@@ -279,10 +279,10 @@ PRINT-MESSAGE is an optional parameter to control whether this
 method should print at all. If nil then nothing will be printed
 at all.
 
-This method will prepend 'Bitwarden: ' before each MSG as a
+This method will prepend \='Bitwarden: \=' before each MSG as a
 convenience. Also, return a value of nil so that no strings
 are mistaken as a password (e.g. accidentally interpreting
-'Bitwarden: error' as the password when in fact, it was an error
+\='Bitwarden: error\=' as the password when in fact, it was an error
 message but happens to be last on the method stack)."
   (when print-message
     (let ((msg (if args (format msg args) msg)))
@@ -293,15 +293,19 @@ message but happens to be last on the method stack)."
   "Handle return MSG of `bitwarden--auto-cmd'.
 
 Since `bitwarden--auto-cmd' returns a list of (err-code message),
-this function exists to handle that. Printing the error message
-is entirely dependent on PRINT-MESSAGE (see below for more info
-on PRINT-MESSAGE).
+this function exists to handle that.
 
 If the error code is 0, then print the password based on
 PRINT-MESSAGE or just return it.
 
-If the error code is non-zero, then print the message based on
-PRINT-MESSAGE and return nil.
+If the error code is non-zero, signal a `user-error' naming the underlying
+cause instead of returning nil.  A silent nil here is indistinguishable from
+a genuinely empty result (e.g. no matching vault items), which previously let
+a locked vault or a failed automatic unlock masquerade as \"nothing found\"
+all the way up through `bitwarden-search' and any `auth-source' caller built
+on it.  `user-error' reports cleanly both interactively (echo area, no
+debugger) and non-interactively (e.g. `emacsclient --eval', which surfaces it
+as `*ERROR*: ...' to the calling process).
 
 PRINT-MESSAGE is an optional parameter to control whether this
 method should print at all. If nil then nothing will be printed
@@ -309,39 +313,42 @@ at all but password will be returned (e.g. when run
 non-interactively)."
   (let* ((err (nth 0 msg))
          (pass (nth 1 msg)))
-    (cond
-     ((eq err 0)
-      (if print-message
-          (message "%s" pass)
-        pass))
-     (t
-      (bitwarden--message "%s" pass print-message)
-      nil))))
-
-(defun bitwarden--auto-cmd (cmd &optional recursive-pass)
-  "Run Bitwarden CMD and attempt to auto unlock.
-
-If RECURSIVE-PASS is set, then treat this call as a second
-attempt after trying to auto-unlock.
-
-Returns a tuple of the error code and the error message or
-password if successful."
-  (let* ((res (or recursive-pass (apply 'bitwarden-runcmd cmd))))
+    (if (eq err 0)
+        (if print-message
+            (message "%s" pass)
+          pass)
+      (user-error "Bitwarden: %s" (or pass "unknown error")))))
+
+(defun bitwarden--auto-cmd (cmd &optional retried)
+  "Run Bitwarden CMD, retrying once via automatic unlock if the vault is locked.
+
+RETRIED is non-nil only on the internal retry call, and prevents a second
+retry so a lock that automatic unlock cannot clear is reported instead of
+recursing forever.  Do not pass it when calling this directly; it used to
+double as the retry's pre-computed result, which meant a lock still present
+after the retry fell through every branch below and returned nil instead of
+an error tuple, indistinguishable downstream from an empty search result.
+
+Returns a tuple of the error code and the error message or password if
+successful; never nil."
+  (let* ((res (bitwarden-runcmd cmd)))
     (cond
      ((string-match bitwarden--err-locked res)
-      ;; try to unlock automatically, if possible
-      (if (not bitwarden-automatic-unlock)
-          (list 1 (format "error: %s" res))
-
-        ;; only attempt a retry once; to prevent infinite recursion
-        (when (not recursive-pass)
-          ;; because I don't understand how emacs is asyncronous here nor
-          ;; how to tell it to wait until the process is done, we do so here
-          ;; manually
-          (bitwarden-unlock)
-          (while (get-process "bitwarden")
-            (sleep-for 0.1))
-          (bitwarden--auto-cmd cmd (apply 'bitwarden-runcmd cmd)))))
+      (cond
+       ;; no automatic unlock configured: report the lock directly
+       ((not bitwarden-automatic-unlock)
+        (list 1 (format "error: %s" res)))
+       ;; already retried once and still locked: automatic unlock failed
+       (retried
+        (list 1 "error: vault is still locked after automatic unlock"))
+       (t
+        ;; because I don't understand how emacs is asyncronous here nor
+        ;; how to tell it to wait until the process is done, we do so here
+        ;; manually
+        (bitwarden-unlock)
+        (while (get-process "bitwarden")
+          (sleep-for 0.1))
+        (bitwarden--auto-cmd cmd t))))
      ((or (string-match bitwarden--err-logged-in res)
           (string-match bitwarden--err-multiple res))
       (list 2 (format "error: %s" res)))
@@ -376,7 +383,7 @@ Returns a vector of hashtables of the results."
 
 ;;;###autoload
 (defun bitwarden-get-info-by-id (id)
-  "The id can be found using the bitwarden CLI. For example: bw list items --pretty.
+  "The id can be found using the bitwarden CLI, e.g. `bw list items --pretty'.
 To access the account password: `(gethash \"password\" (gethash \"login\" ))'"
   (let* ((result (bitwarden--handle-message (bitwarden--auto-cmd (list "get" "item" id)))))
         (when result