summaryrefslogtreecommitdiff
path: root/bin
diff options
context:
space:
mode:
authorLukasz Kasprzak <lukas@labunix.xyz>2026-08-19 09:53:37 +0200
committerLukasz Kasprzak <lukas@labunix.xyz>2026-08-19 09:53:37 +0200
commit6bd741bd512904dfec7c1aa2b7b2bd3dd4269681 (patch)
tree3d5eaeb467d8f403512a75643a9ca9e5eff43d65 /bin
parent27a8fbdf738f7fdb4c9a42ec99f08855dbed440b (diff)
downloadcolitur-6bd741bd512904dfec7c1aa2b7b2bd3dd4269681.tar.gz
colitur-6bd741bd512904dfec7c1aa2b7b2bd3dd4269681.zip
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.
Diffstat (limited to 'bin')
-rw-r--r--bin/main.ml31
1 files changed, 26 insertions, 5 deletions
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