Merge remote-tracking branch 'origin/master' into replace-cmt-cmd

* origin/master:
  Make new and edit cmd usage uniform (#2)
  add OS constraint to opam file

bin/main.ml

@@ -61,7 +61,7 @@ module Prompt = struct
     | true -> Lwt_io.printl @@ sprintf "I: reading from stdin. %s" msg
     | false -> Lwt.return_unit
 
-  let read_input_from_stdin () = Lwt_io.(read stdin)
+  let read_input_from_stdin ?initial:_ () = Lwt_io.(read stdin)
 
   let validate_secret secret =
     match Secret.Validation.validate secret with
@@ -76,15 +76,40 @@ module Prompt = struct
     | true -> Error "secrets cannot have empty lines in the middle of the comments"
     | false -> Ok ()
 
-  let rec input_and_validate_loop ~validate get_input =
-    let%lwt input = get_input () in
+  let rec input_and_validate_loop ~validate ?initial get_input =
+    let remove_trailing_newlines s =
+      (* reverse the string and count leading newlines instead of traversing the string
+         multiple times to remove trailing newlines *)
+      let rev_s =
+        let chars = List.of_seq (String.to_seq s) in
+        String.of_seq (List.to_seq (List.rev chars))
+      in
+      let rec count_leading_newlines ?(acc = 0) ?(i = 0) s =
+        try
+          match s.[i] = '\n' with
+          | true -> count_leading_newlines ~acc:(acc + 1) ~i:(i + 1) s
+          | false -> i
+        with _ -> (* out of bounds edge case *) i - 1
+      in
+      let trailing_newlines = count_leading_newlines rev_s in
+      String.sub s 0 (String.length s - trailing_newlines)
+    in
+    let%lwt input = get_input ?initial () in
+    (* Remove bash commented lines from the secret and any trailing newlines *)
+    let secret =
+      String.split_on_char '\n' input
+      |> List.filter (fun line -> not (String.starts_with ~prefix:"#" line))
+      |> String.concat "\n"
+      |> remove_trailing_newlines
+    in
     match validate input with
     | Error e ->
       if is_TTY = false then Shell.die "This secret is in an invalid format: %s" e
       else (
         let%lwt () = Lwt_io.printlf "\nThis secret is in an invalid format: %s" e in
-        if%lwt yesno "Edit again?" then input_and_validate_loop get_input ~validate else Lwt.return_error e)
-    | _ -> Lwt.return_ok input
+        if%lwt yesno "Edit again?" then input_and_validate_loop ~validate ~initial:input get_input
+        else Lwt.return_error e)
+    | _ -> Lwt.return_ok secret
 
   (** Gets and validates user input reading from stdin. If the input has the wrong format, the user
       is prompted to reinput the secret with the correct format. Allows passing in a function for input
@@ -93,7 +118,7 @@ module Prompt = struct
   let get_valid_input_from_stdin_exn ?(validate = validate_secret) () =
     match%lwt input_and_validate_loop ~validate read_input_from_stdin with
     | Error e -> Shell.die "This secret is in an invalid format: %s" e
-    | Ok secret -> Lwt.return secret
+    | Ok secret -> Lwt.return_ok secret
 end
 
 module Encrypt = struct
@@ -169,9 +194,10 @@ If the secret is a staging secret, its only recipient should be @everyone.
     in
     try%lwt
       let%lwt updated_secret = get_updated_secret original_secret in
-      match updated_secret = Option.value original_secret ~default:"" || updated_secret = "" with
-      | true -> Exn.fail "I: secret unchanged"
-      | false ->
+      match updated_secret, original_secret with
+      | Ok updated_secret, Some original_secret when updated_secret = original_secret -> Exn.fail "I: secret unchanged"
+      | Error e, _ -> Shell.die "E: %s" e
+      | Ok updated_secret, _ ->
         let secret_path = path_of_secret_name secret_name in
         let secret_recipients' = Storage.Secrets.get_recipients_from_path_exn secret_path in
         let%lwt secret_recipients =
@@ -413,11 +439,11 @@ module Edit_cmd = struct
     | true ->
       Invariant.run_if_recipient ~op_string:"edit secret" ~path:(path_of_secret_name secret_name) ~f:(fun () ->
           Edit.edit_secret secret_name ~allow_retry:true ~get_updated_secret:(fun initial ->
-              let%lwt secret =
-                Prompt.input_and_validate_loop ~validate:Prompt.validate_secret
-                  (Edit.new_text_from_editor ?initial ~name:(show_name secret_name))
-              in
-              Lwt.return (Result.value ~default:"" secret)))
+              Prompt.input_and_validate_loop
+                ~validate:Prompt.validate_secret
+                  (* when we are editing a secret, we know `initial` is Some and we add the format explainer to it *)
+                ?initial:(Option.map (fun i -> i ^ Secret.format_explainer) initial)
+                (Edit.new_text_from_editor ~name:(show_name secret_name))))
 
   let edit =
     let doc = "edit the contents of the specified secret" in
@@ -787,78 +813,17 @@ module List_ = struct
 end
 
 module New = struct
-  let singleline prompt =
-    let%lwt () = Lwt_io.printf "%s: \n" prompt in
-    let%lwt line = Lwt_io.(read_line stdin) in
-    Lwt.return line
-
-  let multiline prompt =
-    let%lwt () = Lwt_io.printf "%s (hit enter twice to end):\n" prompt in
-    let rec loop lines =
-      let%lwt line = Lwt_io.(read_line stdin) in
-      if line = "" then Lwt.return @@ String.concat "\n" (List.rev @@ lines) else loop (line :: lines)
-    in
-    loop []
-
-  let notice note =
-    let%lwt () = Lwt_io.printl note in
-    let%lwt (_ : string) = Lwt_io.(read_line stdin) in
-    Lwt.return_unit
-
   let create_new_secret secret_name =
-    let%lwt description =
-      multiline
-        {|
-Every secret should include where it comes from (how to rotate) and where it goes (how it is used).
-
-Enter a complete description|}
-    in
-    let%lwt secret =
-      singleline "Single-line secret (you'll be given a chance to edit the result or add more recipients next)"
-    in
-    let%lwt secret_and_description =
-      match secret <> "", description = "" with
-      | false, _ -> Shell.die "E: no secret provided. Quitting"
-      | true, false ->
-        (* single-line secrets with comments, use the correct format *)
-        Lwt.return @@ Secret.singleline_from_text_description secret description
-      | true, true ->
-        let%lwt () =
-          notice "No description provided, creating single-line secret without description. Hit enter to continue."
-        in
-        Lwt.return secret
-    in
-    let%lwt secret_and_description =
-      let rec loop ?(is_invalid = false) tmp_file =
-        let%lwt new_text =
-          let%lwt review_secret = Prompt.yesno "review/edit the resulting file?" in
-          match is_invalid, review_secret with
-          | true, false -> Shell.die "E: can't create the secret. Invalid format"
-          | false, false -> Lwt.return tmp_file
-          | _, true ->
-            (try%lwt Edit.new_text_from_editor ~initial:tmp_file () with exn -> Shell.die ~exn "Failed editing text")
-        in
-        match Secret.Validation.validate new_text with
-        | Error (e, _t) ->
-          let%lwt () = Lwt_io.printf "This secret is in an invalid format: %s\n" e in
-          loop ~is_invalid:true new_text
-        | Ok _ -> Lwt.return new_text
-      in
-      loop secret_and_description
+    let%lwt () =
+      Edit.edit_secret ~self_fallback:true secret_name ~allow_retry:true ~get_updated_secret:(fun initial ->
+          Prompt.input_and_validate_loop ~validate:Prompt.validate_secret
+            ~initial:(Option.value ~default:Secret.format_explainer initial)
+            (Edit.new_text_from_editor ~name:(show_name secret_name)))
     in
-    try%lwt
-      let secret_path = path_of_secret_name secret_name in
-      let original_recipients = Storage.Secrets.get_recipients_from_path_exn secret_path in
-      let%lwt () =
-        Edit.show_recipients_notice_if_true (original_recipients = []);
-        if%lwt Prompt.yesno "Edit recipients for this secret?" then Edit.edit_recipients secret_name
-      in
-      (* if the new secret has no recipients, add ourselves to it *)
-      let%lwt own_recipients = Storage.Secrets.recipients_of_own_id () in
-      let%lwt () = Edit.add_recipients_if_none_exists own_recipients secret_path in
-      let recipients = Storage.Secrets.get_recipients_from_path_exn secret_path in
-      Encrypt.encrypt_exn ~plaintext:secret_and_description ~secret_name recipients
-    with exn -> Shell.die ~exn "E: encrypting %s failed" (show_name secret_name)
+    let secret_path = path_of_secret_name secret_name in
+    let original_recipients = Storage.Secrets.get_recipients_from_path_exn secret_path in
+    Edit.show_recipients_notice_if_true (original_recipients = []);
+    if%lwt Prompt.yesno "Edit recipients for this secret?" then Edit.edit_recipients secret_name
 
   let create_new_secret' secret_name = Create.invariant_create ~create_new_secret secret_name
 
@@ -1019,8 +984,8 @@ module Replace_comments = struct
               in
               let get_comments_from_editor () =
                 match%lwt
-                  Prompt.input_and_validate_loop ~validate:Prompt.validate_comments
-                    (Edit.new_text_from_editor ?initial:original_secret.comments ~name:(show_name secret_name))
+                  Prompt.input_and_validate_loop ~validate:Prompt.validate_comments ?initial:original_secret.comments
+                    (Edit.new_text_from_editor ~name:(show_name secret_name))
                 with
                 | Error e -> Shell.die "The comments are in an invalid format: %s" e
                 | Ok secret -> Lwt.return secret

lib/secret.ml

@@ -26,6 +26,32 @@ let multiline_from_text_description text description =
   | "" -> Printf.sprintf "\n\n%s" text
   | _ -> Printf.sprintf "\n%s\n\n%s" description text
 
+let format_explainer =
+  {|
+
+# Secrets and comments formats:
+# (multi-line comments _should not_ have empty lines in them)
+#
+# Single line secret with commments format:
+#     secret one line
+#     <empty line>
+#     comments until end of file
+#
+# Single line secret without commments format:
+#     secret one line
+#
+# Multiline secret with comments format:
+#     <empty line>
+#     possibly several lines of comments
+#     <empty line>
+#     secret until end of file
+#
+# Multiline secret without comments format:
+#     <empty line>
+#     <empty line>
+#     secret until end of file
+|}
+
 module Validation = struct
   type validation_error =
     | SingleLineLegacy
@@ -72,7 +98,9 @@ module Validation = struct
         | false -> Ok Singleline)
       (* We don't want to allow the creation of new secrets in legacy single-line format *)
       | secret :: comment :: _ when String.trim secret <> "" && String.trim comment <> "" ->
-        Error ("legacy single line secret format. Please use the correct format", SingleLineLegacy)
+        Error
+          ( "single-line secrets with comments should have an empty line between the secret and the comments.",
+            SingleLineLegacy )
       (* single-line without comments *)
       | [ secret ] when String.trim secret <> "" -> Ok Singleline
       | _ -> Error ("invalid format", InvalidFormat))

lib_test/validation_test.ml

@@ -55,7 +55,7 @@ let%expect_test "legacy single-line with comments" =
   test_validate {|secret
 comment
 comment|};
-  [%expect {| Error: legacy single line secret format. Please use the correct format |}]
+  [%expect {| Error: single-line secrets with comments should have an empty line between the secret and the comments. |}]
 
 let%expect_test "single-line without comments" =
   test_validate {|secret|};

passage.opam

@@ -42,6 +42,7 @@ build: [
   ]
 ]
 dev-repo: "git+https://github.com/ahrefs/passage.git"
+available: os != "win32"
 x-ci-accept-failures: [
   "alpine-3.20"
   "archlinux"

passage.opam.template

@@ -1,3 +1,4 @@
+available: os != "win32"
 x-ci-accept-failures: [
   "alpine-3.20"
   "archlinux"

tests/create_command.t

@@ -43,7 +43,7 @@ Should fail - create secret with wrong format
   This secret is in an invalid format: empty secrets are not allowed
   [1]
   $ printf "secret\ncomment" | passage create new/legacy-single-line
-  This secret is in an invalid format: legacy single line secret format. Please use the correct format
+  This secret is in an invalid format: single-line secrets with comments should have an empty line between the secret and the comments.
   [1]
   $ printf "\nsecret\ncomment" | passage create new/multi-line-no-secret
   This secret is in an invalid format: multiline: empty secret

tests/edit_command.t

@@ -45,7 +45,7 @@ Should fail - passing in malformed secrets
   This secret is in an invalid format: multiline: empty secret
   [1]
   $ EDITOR=$LEGACY_SINGLELINE passage edit 00/existing_secret
-  This secret is in an invalid format: legacy single line secret format. Please use the correct format
+  This secret is in an invalid format: single-line secrets with comments should have an empty line between the secret and the comments.
   [1]
   $ passage get 00/existing_secret
   BYE