fix(cli): publish --prune refuses a manifest entry that escapes --out

CRITICAL: .colitur-manifest lives INSIDE the tree publish writes into --
the very tree this feature exists to have committed into a git repo. A
manifest entry with a ".." path component, or an absolute path, let
--prune Sys.remove/Unix.rmdir a file OUTSIDE --out. No attacker is
required: an ordinary bad merge, a conflict resolved the wrong way, or
a hand-edit of that file is enough to plant such an entry, and publish's
own stated contract -- it never deletes a file it does not own -- broke
outright the moment one was present.

Two independent checks, both required, applied before every deletion:

  - structural (manifest_entry_is_safe): reject an entry that is
    absolute or has a ".." path COMPONENT, by splitting on '/' and
    comparing components, not by substring-matching ".." (which would
    wrongly reject a legitimate name like foo..bar).
  - containment (resolves_under): resolve both --out and the candidate
    with Unix.realpath (closing a symlink-inside-out gap the structural
    check alone would miss) and verify the candidate is a genuine path
    descendant of --out, not merely a string with the same prefix.

Applied at both the file-deletion loop and prune_empty_dirs' own
directory removals. A rejected entry is skipped with a one-line stderr
warning; publish completes rather than aborting -- a corrupted manifest
must not make the tool itself unusable.

test/cli.t reproduces the exact canary scenario (a ".." entry
surviving deletion of a file outside --out), an absolute-path entry,
and a legitimate dotted filename (no .. component) still pruning
normally, alongside the existing --prune coverage.

2 files changed, 138 insertions, 9 deletions

diff --git a/bin/main.ml b/bin/main.ml
index 4f1fbf2..6f3cbd5 100644
--- a/bin/main.ml
+++ b/bin/main.ml
@@ -561,6 +561,63 @@ let read_manifest out =
   | Error _ -> []
   | Ok contents -> String.split_on_char '\n' contents |> List.filter (fun l -> l <> "")
 
+(* Fix round 1 (coordinator review), CRITICAL: a manifest entry is
+   UNTRUSTED input the moment [--prune] reads it back. The manifest is a
+   plain-text file that lives INSIDE the very tree this feature exists to
+   have committed into a git repo -- an ordinary bad merge or a hand-edit is
+   enough to put an arbitrary path in it, no attacker required. Without a
+   check, an entry like "../outside/CANARY.txt" resolves, via
+   [Filename.concat out entry], to a path OUTSIDE [out], and the prune loop
+   below would [Sys.remove] it -- deleting a file [publish] never wrote,
+   breaking the "never deletes a file it does not own" contract outright.
+
+   Two independent checks, deliberately, because either alone is easy to
+   regress later without anyone noticing in review:
+
+   1. STRUCTURAL ([manifest_entry_is_safe]) -- reject an entry that is
+      absolute, or that has a ".." path component anywhere. Split on '/'
+      and compare COMPONENTS, never a bare substring test: substring-
+      matching ".." would wrongly reject a legitimate name like
+      "foo..bar", which contains the two characters but has no ".."
+      component of its own.
+   2. CONTAINMENT ([resolves_under]) -- even an entry that passes check 1
+      is not trusted until the path it actually resolves to, symlinks
+      included, is verified to sit under [out]. [Unix.realpath] resolves
+      symlinks as well as "..", so this also catches an entry that a
+      symlink planted inside [out] could use to defeat check 1 alone. A
+      plain string-prefix compare is not enough by itself either:
+      "/tmp/pub1" is a byte-prefix of "/tmp/pub1-evil", a directory that is
+      not nested inside it at all, so [is_under] insists the character
+      right after the prefix is the path separator (or that the paths are
+      identical). *)
+let manifest_entry_is_safe entry =
+  entry <> ""
+  && entry.[0] <> '/'
+  && not (List.mem ".." (String.split_on_char '/' entry))
+
+let is_under ~root path =
+  let root =
+    if String.length root > 1 && root.[String.length root - 1] = '/' then
+      String.sub root 0 (String.length root - 1)
+    else root
+  in
+  String.equal path root
+  || (String.length path > String.length root
+     && String.sub path 0 (String.length root) = root
+     && path.[String.length root] = '/')
+
+(* [Unix.realpath] requires the path to exist, which is fine here: every
+   caller below checks [Sys.file_exists]/[Sys.readdir] first. Any failure
+   (missing path, dangling symlink, permission error) is treated as "not
+   contained" -- refuse to act rather than guess. *)
+let resolves_under out p =
+  match Unix.realpath out with
+  | exception (Unix.Unix_error _ | Sys_error _) -> false
+  | out_real -> (
+      match Unix.realpath p with
+      | exception (Unix.Unix_error _ | Sys_error _) -> false
+      | p_real -> is_under ~root:out_real p_real)
+
 (* [--prune] deletes the FILES a stale manifest entry names, but that alone
    can leave their parent directories (ef/<year>/<mm>/, then ef/<year>/)
    empty behind them -- and an empty directory still makes `test -d
@@ -568,9 +625,18 @@ let read_manifest out =
    an old year is gone. Walk upward from each deleted file's own directory,
    removing it while it is empty, stopping at (never including) [out]
    itself: [out] is the caller's own directory, never ours to remove, even
-   when it is empty. *)
+   when it is empty. The same containment discipline as the file deletions
+   above applies here too ([resolves_under]), not only structurally (this
+   function is only ever reached via a [p] the file-deletion path already
+   validated, but re-checking each directory step is the belt to that
+   entry's braces -- see the two-layer reasoning above). *)
 let rec prune_empty_dirs ~out dir =
-  if dir <> out && String.length dir > String.length out && Sys.file_exists dir then
+  if
+    dir <> out
+    && String.length dir > String.length out
+    && Sys.file_exists dir
+    && resolves_under out dir
+  then
     match Sys.readdir dir with
     | [||] ->
         (try Unix.rmdir dir with Unix.Unix_error _ -> ());
@@ -679,16 +745,31 @@ let publish_report ~from_y ~to_y ~out ~overlays ~dtstamp ~prune =
   emit "schema/day-v1.json" schema;
   emit "index.html" (index_html ~from_y ~to_y);
   let now = List.sort compare !written in
+  (* Every stale entry is validated TWICE before anything is removed --
+     see [manifest_entry_is_safe]/[resolves_under]'s own comment above for
+     why both layers exist. A rejected entry is skipped and warned about on
+     stderr, never fatal: a corrupt or hand-mangled manifest must not make
+     `publish` itself unusable -- it completes, having refused to act on
+     the bad line. *)
   if prune then
     List.iter
       (fun old ->
-        if not (List.mem old now) then begin
-          let p = Filename.concat out old in
-          if Sys.file_exists p then begin
-            Sys.remove p;
-            prune_empty_dirs ~out (Filename.dirname p)
-          end
-        end)
+        if not (List.mem old now) then
+          if not (manifest_entry_is_safe old) then
+            Printf.eprintf
+              "colitur: refusing to prune manifest entry %S (absolute path or .. component)
+" old
+          else begin
+            let p = Filename.concat out old in
+            if Sys.file_exists p then
+              if resolves_under out p then begin
+                Sys.remove p;
+                prune_empty_dirs ~out (Filename.dirname p)
+              end
+              else
+                Printf.eprintf "colitur: refusing to prune %s (resolves outside %s)
+" p out
+          end)
       (read_manifest out);
   write_file (Filename.concat out manifest_name) (String.concat "\n" now ^ "\n");
   Printf.printf "colitur: wrote %d files to %s\n" (List.length now) out
diff --git a/test/cli.t b/test/cli.t
index c74331b..a4fa696 100644
--- a/test/cli.t
+++ b/test/cli.t
@@ -1004,3 +1004,51 @@ ignored, the same discipline as everywhere else:
   $ colitur day 2027 --out /tmp/pub3 --prune
   colitur: --out/--prune have no effect on `day`; refusing rather than ignoring them
   [2]
+
+--prune's manifest-driven deletion is hardened against a manifest entry it
+did not itself write (fix round 1, F1, CRITICAL): the manifest lives INSIDE
+the tree publish writes into, so a bad merge or a hand-edit can put an
+arbitrary path in it -- no attacker required. This is the exact CANARY
+reproduction the finding was raised with: a ".." entry appended to the
+manifest must never let --prune delete outside --out.
+
+  $ rm -rf /tmp/pub-sec /tmp/pub-sec-outside
+  $ mkdir -p /tmp/pub-sec-outside
+  $ touch /tmp/pub-sec-outside/CANARY.txt
+  $ colitur publish --from 2027 --to 2027 --out /tmp/pub-sec >/dev/null
+  $ echo '../pub-sec-outside/CANARY.txt' >> /tmp/pub-sec/.colitur-manifest
+  $ colitur publish --from 2028 --to 2028 --out /tmp/pub-sec --prune >/dev/null
+  colitur: refusing to prune manifest entry "../pub-sec-outside/CANARY.txt" (absolute path or .. component)
+  $ test -f /tmp/pub-sec-outside/CANARY.txt && echo canary-survives
+  canary-survives
+
+The same run's own legitimate stale entries (2027's files, superseded by
+2028) still prune normally -- the hardening does not disable pruning, only
+unsafe entries:
+
+  $ test -d /tmp/pub-sec/ef/2027 || echo 2027-pruned-normally
+  2027-pruned-normally
+
+An absolute-path entry is refused the same way, not only a ".." one:
+
+  $ echo '/tmp/pub-sec-outside/CANARY.txt' >> /tmp/pub-sec/.colitur-manifest
+  $ colitur publish --from 2028 --to 2028 --out /tmp/pub-sec --prune >/dev/null
+  colitur: refusing to prune manifest entry "/tmp/pub-sec-outside/CANARY.txt" (absolute path or .. component)
+  $ test -f /tmp/pub-sec-outside/CANARY.txt && echo canary-still-survives
+  canary-still-survives
+
+A legitimate filename that merely CONTAINS two dots -- but has no ".." path
+COMPONENT -- is not caught by the same check, proving it is not
+over-broad: it still prunes normally when stale.
+
+  $ touch /tmp/pub-sec/ef/2027..old.json
+  $ echo 'ef/2027..old.json' >> /tmp/pub-sec/.colitur-manifest
+  $ colitur publish --from 2029 --to 2029 --out /tmp/pub-sec --prune >/dev/null
+  $ test -f /tmp/pub-sec/ef/2027..old.json || echo dotted-name-pruned
+  dotted-name-pruned
+
+...and that same run is an ordinary --prune cycle in every other respect --
+2028's own files, now stale relative to 2029, are gone too:
+
+  $ test -d /tmp/pub-sec/ef/2028 || echo pruned-2028
+  pruned-2028