storage: strip inline comments from .pub, .group and .keys files

CHANGES.md

@@ -4,6 +4,7 @@
 - Newline can be used to terminate user name input.
 - Add `overlap` command to find recipients with the most secret overlap, to help coordinate key refreshes.
 - `passage refresh` now warns when secrets are skipped due to lack of access.
+- Fix bug where in-line comments in .pub, .keys and .group files were not ignored ([#20](https://github.com/ahrefs/passage/pull/20))
 
 ## 0.3.3 (2026-02-13)
 - Update os availability in opam. Make passage available in macos again

lib/storage.ml

@@ -1,6 +1,4 @@
 open Printf
-
-(** Read lines from a config file, filtering out comments and empty lines *)
 let config_lines filename =
   if not (Sys.file_exists filename) then []
   else
@@ -12,8 +10,14 @@ let config_lines filename =
       in
       read_lines []
       |> List.filter_map (fun line ->
-        let trimmed = String.trim line in
-        if trimmed = "" || String.starts_with ~prefix:"#" trimmed then None else Some trimmed))
+        let content =
+          match String.index_opt line '#' with
+          | None -> line
+          | Some i -> String.sub line 0 i
+        in
+        match String.trim content with
+        | "" -> None
+        | s -> Some s))
 
 let save_as ?(mode = 0o644) ~path f =
   let temp = Printf.sprintf "%s.save.%d.tmp" path (Unix.getpid ()) in

lib/storage.mli

@@ -1,3 +1,8 @@
+(** Read lines from a config file, filtering out comments and empty lines.
+
+    Based on Devkit's [Action.config_lines]. *)
+val config_lines : string -> string list
+
 (** File output protected with atomic rename.
 
     [save_as path f] is similar to {!Out_channel.with_open_bin} except that writing is done to a temporary file that

lib_test/config_lines_test.ml

@@ -0,0 +1,74 @@
+open Passage
+
+let test_config_lines content =
+  let tmpfile = Filename.temp_file "config_lines_test" ".txt" in
+  Fun.protect
+    (fun () ->
+      Out_channel.with_open_text tmpfile (fun oc -> output_string oc content);
+      let result = Storage.config_lines tmpfile in
+      List.iter (fun line -> Printf.printf "%S\n" line) result)
+    ~finally:(fun () -> Sys.remove tmpfile)
+
+let%expect_test "empty lines are stripped" =
+  test_config_lines "alice\n\nbob\n\n\ncharlie\n";
+  [%expect {|
+    "alice"
+    "bob"
+    "charlie" |}]
+
+let%expect_test "full-line comments are stripped" =
+  test_config_lines "alice\n# this is a comment\nbob\n  # indented comment\ncharlie\n";
+  [%expect {|
+    "alice"
+    "bob"
+    "charlie" |}]
+
+let%expect_test "inline comments are stripped" =
+  test_config_lines "alice # team lead\nbob#backup\ncharlie  #  ops\n";
+  [%expect {|
+    "alice"
+    "bob"
+    "charlie" |}]
+
+let%expect_test "leading and trailing whitespace is trimmed" =
+  test_config_lines "  alice  \n\tbob\t\n  charlie  \n";
+  [%expect {|
+    "alice"
+    "bob"
+    "charlie" |}]
+
+let%expect_test "whitespace + inline comments combined" =
+  test_config_lines "  alice # lead  \n  bob  # backup  \n";
+  [%expect {|
+    "alice"
+    "bob" |}]
+
+let%expect_test "line that is only a comment after stripping" =
+  test_config_lines "# full comment\n  # indented full comment\nalice\n";
+  [%expect {| "alice" |}]
+
+let%expect_test "nonexistent file returns empty list" =
+  let result = Storage.config_lines "/nonexistent/path/file.txt" in
+  List.iter (fun line -> Printf.printf "%S\n" line) result;
+  [%expect {| |}]
+
+let%expect_test "empty file returns empty list" =
+  test_config_lines "";
+  [%expect {| |}]
+
+let%expect_test "file with only comments and whitespace" =
+  test_config_lines "# comment\n  \n\n  # another\n";
+  [%expect {| |}]
+
+let%expect_test "mixed: all features together" =
+  test_config_lines {|# header comment
+alice # admin
+  bob
+  # separator
+
+charlie  # ops
+|};
+  [%expect {|
+    "alice"
+    "bob"
+    "charlie" |}]

tests/inline_comments.t

@@ -0,0 +1,95 @@
+  $ . ./setup_fixtures.sh
+
+Test that config_lines correctly handles comments, empty lines, and whitespace
+in .keys, .group, and .pub files.
+
+=== .keys files: empty lines, full-line comments, whitespace, and inline comments ===
+
+  $ cat<<EOF > "$PASSAGE_DIR/secrets/00/.keys"
+  > # This is a full-line comment
+  >   # Indented full-line comment
+  > 
+  >   bobby.bob # team lead
+  >   robby.rob
+  > 
+  > EOF
+  $ passage who 00
+  bobby.bob
+  robby.rob
+
+Create and decrypt a secret in the folder with the above .keys
+  $ passage get 00/secret1
+  (00/secret1) secret: single line
+
+=== .group files: empty lines, full-line comments, whitespace, and inline comments ===
+
+  $ cat<<EOF > "$PASSAGE_DIR/keys/commented.group"
+  > # group members
+  > 
+  >   robby.rob # admin
+  >   tommy.tom # regular
+  > 
+  > EOF
+  $ passage who @commented
+  robby.rob
+  tommy.tom
+
+Use the commented group in a .keys file and verify it resolves correctly
+  $ cat<<EOF > "$PASSAGE_DIR/secrets/01/00/.keys"
+  > @commented
+  > EOF
+  $ passage who -f 01/00
+  robby.rob
+  tommy.tom
+
+=== .pub files: inline comments should be stripped ===
+
+Create a .pub key file with an inline comment appended to the actual key
+  $ INLINE_PUB_USER="inline.pub.user"
+  $ INLINE_PUB_IDENTITY="$INLINE_PUB_USER.key"
+  $ age-keygen > "$INLINE_PUB_IDENTITY" 2> /dev/null
+  $ PUBKEY=$(age-keygen -y "$INLINE_PUB_IDENTITY")
+  $ echo "$PUBKEY # added 2025-01-01" > "$PASSAGE_DIR/keys/$INLINE_PUB_USER.pub"
+
+The key should still work for encryption - inline comment must be stripped
+  $ cat<<EOF > "$PASSAGE_DIR/secrets/00/.keys"
+  > bobby.bob
+  > $INLINE_PUB_USER
+  > EOF
+  $ cat<<EOF | passage create 00/inline_pub_secret
+  > pub inline comment test
+  > EOF
+  $ PASSAGE_IDENTITY="$INLINE_PUB_IDENTITY" passage get 00/inline_pub_secret
+  pub inline comment test
+
+=== .pub files: leading/trailing whitespace should be trimmed ===
+
+  $ WHITESPACE_USER="whitespace.pub.user"
+  $ WHITESPACE_IDENTITY="$WHITESPACE_USER.key"
+  $ age-keygen > "$WHITESPACE_IDENTITY" 2> /dev/null
+  $ WHITESPACE_PUBKEY=$(age-keygen -y "$WHITESPACE_IDENTITY")
+  $ echo "  $WHITESPACE_PUBKEY  " > "$PASSAGE_DIR/keys/$WHITESPACE_USER.pub"
+
+  $ cat<<EOF > "$PASSAGE_DIR/secrets/00/.keys"
+  > bobby.bob
+  > $WHITESPACE_USER
+  > EOF
+  $ cat<<EOF | passage create 00/whitespace_pub_secret
+  > whitespace pub test
+  > EOF
+  $ PASSAGE_IDENTITY="$WHITESPACE_IDENTITY" passage get 00/whitespace_pub_secret
+  whitespace pub test
+
+=== Mixed: all features together in a .keys file ===
+
+  $ cat<<EOF > "$PASSAGE_DIR/secrets/00/.keys"
+  > # This is a full-line comment
+  >   bobby.bob # primary
+  > 
+  >   # Another full-line comment
+  >   robby.rob # secondary
+  > 
+  > EOF
+  $ passage who 00
+  bobby.bob
+  robby.rob