Fix darcs clone: allow _darcs/format and tighten wildcard hashes.

Authorfritjof@alokat.org
Date2026-04-22 15:48:56
Hashe402aee2bd0c724ca4e282c8858cb13729fa79de

Description

The HTTP clone allowlist used to reject _darcs/format, but darcs probes
that file first to identify a hashed repository; without it `darcs clone`
fails with "Not a repository" even though hashed_inventory is served.
Add "format" as a one-segment entry to isCloneSubPath.

While touching the allowlist, tighten the three wildcard hash-directory
arms ("inventories/<hash>", "patches/<hash>", "pristine.hashed/<hash>")
to require a non-empty hash segment (cons pattern @_:_@).  splitPath
keeps the trailing empty when the URL ends in a slash, while
isSafeSubPath filters empty segments, so "inventories/" and friends
silently slipped through the allowlist.  serveClone still 404'd them
via doesFileExist, but the documented "exact shape" contract was broken
and defence-in-depth would have failed if a future refactor touched
serveClone.

Tests extend prop_clone_allowlist_rejects_nested with the trailing-slash
and one-segment-extension cases that were previously unexercised.
Comments at the two router/handler doc sites are updated to match.

_darcs/prefs/motd remains deliberately denied per the existing
"keep prefs/* private" decision; revisit in a separate patch if needed.

Summary

M ./app/Main.hs -6 +6
M ./src/DarcsWeb/Darcs.hs -10 +17
M ./test/Properties/Clone.hs -6 +18

Diff

patch e402aee2bd0c724ca4e282c8858cb13729fa79de
Author: fritjof@alokat.org
Date:   Wed Apr 22 15:48:56 UTC 2026
  * Fix darcs clone: allow _darcs/format and tighten wildcard hashes.
  
  The HTTP clone allowlist used to reject _darcs/format, but darcs probes
  that file first to identify a hashed repository; without it `darcs clone`
  fails with "Not a repository" even though hashed_inventory is served.
  Add "format" as a one-segment entry to isCloneSubPath.
  
  While touching the allowlist, tighten the three wildcard hash-directory
  arms ("inventories/<hash>", "patches/<hash>", "pristine.hashed/<hash>")
  to require a non-empty hash segment (cons pattern @_:_@).  splitPath
  keeps the trailing empty when the URL ends in a slash, while
  isSafeSubPath filters empty segments, so "inventories/" and friends
  silently slipped through the allowlist.  serveClone still 404'd them
  via doesFileExist, but the documented "exact shape" contract was broken
  and defence-in-depth would have failed if a future refactor touched
  serveClone.
  
  Tests extend prop_clone_allowlist_rejects_nested with the trailing-slash
  and one-segment-extension cases that were previously unexercised.
  Comments at the two router/handler doc sites are updated to match.
  
  _darcs/prefs/motd remains deliberately denied per the existing
  "keep prefs/* private" decision; revisit in a separate patch if needed.
hunk ./app/Main.hs 264
-    -- required by the HTTP clone protocol (hashed_inventory, inventories,
-    -- patches, pristine.hashed, optional packs). Anything else under
-    -- _darcs/ ��� including prefs/*, format, and custom scripts ��� is kept
-    -- private even when it would pass the hidden-segment check.
+    -- required by the HTTP clone protocol (format, hashed_inventory,
+    -- inventories, patches, pristine.hashed, optional packs). Anything
+    -- else under _darcs/ ��� including prefs/* and custom scripts ��� is
+    -- kept private even when it would pass the hidden-segment check.
hunk ./app/Main.hs 434
---   the objects the darcs client actually needs (hashed_inventory,
+--   the objects the darcs client actually needs (format, hashed_inventory,
hunk ./app/Main.hs 436
---   served; prefs, format, and any unrelated file stay private.
+--   served; prefs/* and any unrelated file stay private.
hunk ./src/DarcsWeb/Darcs.hs 290
---   files under @_darcs/@ (e.g. @prefs/defaultrepo@, @format@, custom
---   scripts) stay private even when they pass hidden-segment checks.
+--   files under @_darcs/@ (e.g. @prefs/defaultrepo@, custom scripts)
+--   stay private even when they pass hidden-segment checks.
hunk ./src/DarcsWeb/Darcs.hs 293
---   Accepted (exact two-segment shapes only; extra nesting is rejected):
+--   Accepted (exact shapes only; extra nesting is rejected):
hunk ./src/DarcsWeb/Darcs.hs 295
+--   * @format@ (one-line repo-format marker; darcs probes it first to
+--     identify the repository, so HTTP clone fails without it)
hunk ./src/DarcsWeb/Darcs.hs 305
+    -- The hash-directory arms match @_:_@ (a non-empty hash segment)
+    -- rather than @_@: 'isSafeSubPath' filters empty segments so
+    -- @inventories/@ would otherwise pass, but 'splitPath' keeps the
+    -- trailing empty and we must reject it here.
hunk ./src/DarcsWeb/Darcs.hs 310
-        ["hashed_inventory"]         -> True
-        ["inventories",     _]       -> True
-        ["patches",         _]       -> True
-        ["pristine.hashed", _]       -> True
-        ["packs", "basic.tar.gz"]    -> True
-        ["packs", "patches.tar.gz"]  -> True
-        _                            -> False
+        ["format"]                    -> True
+        ["hashed_inventory"]          -> True
+        ["inventories",     _:_]      -> True
+        ["patches",         _:_]      -> True
+        ["pristine.hashed", _:_]      -> True
+        ["packs", "basic.tar.gz"]     -> True
+        ["packs", "patches.tar.gz"]   -> True
+        _                             -> False
hunk ./test/Properties/Clone.hs 175
-    [ acceptsClone "hashed_inventory"
+    [ acceptsClone "format"
+    , acceptsClone "hashed_inventory"
hunk ./test/Properties/Clone.hs 197
-    [ rejectsClone "format"
-    , rejectsClone "tentative_hashed_pristine"
+    [ rejectsClone "tentative_hashed_pristine"
hunk ./test/Properties/Clone.hs 203
--- Pin the exact two-segment shape for hash directories; deeper nesting
--- must not be accepted. Catches a regression to the previous
--- @("patches" : _ : _)@ wildcard-tail pattern, which admitted any depth.
+-- Pin the exact shape for each accepted entry; deeper nesting or a
+-- trailing slash must not be accepted. Catches a regression to the
+-- previous @("patches" : _ : _)@ wildcard-tail pattern (which admitted
+-- any depth) and ensures the one-segment entries (@format@,
+-- @hashed_inventory@) cannot be widened into prefix matches.
hunk ./test/Properties/Clone.hs 215
+    , rejectsClone "format/extra"
+    , rejectsClone "format/"
+    , rejectsClone "hashed_inventory/extra"
+    , rejectsClone "hashed_inventory/"
+    -- Trailing-slash cases: splitPath keeps the empty second segment
+    -- while isSafeSubPath filters it, so the wildcard-hash entries
+    -- must reject an empty object name explicitly.
+    , rejectsClone "inventories/"
+    , rejectsClone "patches/"
+    , rejectsClone "pristine.hashed/"