From 6bd741bd512904dfec7c1aa2b7b2bd3dd4269681 Mon Sep 17 00:00:00 2001
From: Lukasz Kasprzak <lukas@labunix.xyz>
Date: Wed, 19 Aug 2026 09:53:37 +0200
Subject: fix(cli): guard the whole template read, not only the open

read_file guarded open_in_bin but left in_channel_length and
really_input_string unguarded, so a path that opens but cannot be read
as bytes -- a directory -- escaped as an uncaught Sys_error and crashed
the program, leaking the open channel on every failure path. A template
is user input; it must never crash the program.

Wrap the whole read in Fun.protect so the channel closes on every path
(success, exception, early return), matching the close-on-every-path
pattern already used in the test suite. The missing-file message stays
exactly as before; a read failure after a successful open now carries
the exception text, the same path: exception shape Layer.load and
Overlay.load already use.

New cram case points --template at a directory (the sandbox's own cwd,
not /tmp) and asserts one stderr line and exit 2, not a crash.
---
 bin/main.ml | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

(limited to 'bin/main.ml')

diff --git a/bin/main.ml b/bin/main.ml
index 5e3d07d..e916cbe 100644
--- a/bin/main.ml
+++ b/bin/main.ml
@@ -442,14 +442,35 @@ let emit_report ~format ~overlays ~dtstamp ~from_y ~to_y =
    real pipe use, because JSON there is the OUTPUT, never something colitur
    itself has to parse back in. *)
 
+(* The open is guarded separately from the read: a missing file fails at
+   [open_in_bin] with a plain, path-only message (matching the wording this
+   project already uses for every other "no such file" case), while a file
+   that opens but cannot be READ -- a directory, a device node, anything
+   whose length or content changes between [open] and [read] -- fails inside
+   the [Fun.protect]'d body instead, carrying the raised exception's own text
+   (mirrors {!Colitur_kernel.Layer.load}/{!Colitur_kernel.Overlay.load}'s own
+   catch-all shape, lib/kernel/layer.ml and lib/kernel/overlay.ml). Either
+   way the channel is closed on EVERY path -- success, exception, or an
+   early return -- because [close_in_noerr] runs in [~finally], which
+   [Fun.protect] guarantees runs even when the protected function raises; a
+   bare [close_in] after [really_input_string] only ever ran on the success
+   path, leaking the descriptor on every failure. The whole read is inside
+   the [try], not only [open_in_bin], because [in_channel_length] and
+   [really_input_string] can themselves raise [Sys_error] (a directory opens
+   fine but is not readable as bytes) -- a template is user input, and this
+   project's own rule is that user input must never crash the program. *)
 let read_file path =
   match open_in_bin path with
   | exception Sys_error _ -> Error ("cannot read template " ^ path)
-  | ic ->
-      let n = in_channel_length ic in
-      let s = really_input_string ic n in
-      close_in ic;
-      Ok s
+  | ic -> (
+      try
+        Fun.protect
+          ~finally:(fun () -> close_in_noerr ic)
+          (fun () ->
+            let n = in_channel_length ic in
+            let s = really_input_string ic n in
+            Ok s)
+      with exn -> Error (Printf.sprintf "cannot read template %s: %s" path (Printexc.to_string exn)))
 
 let extension path =
   match String.rindex_opt path '.' with
-- 
cgit v1.3