diff options
| author | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-08-19 10:42:22 +0200 |
|---|---|---|
| committer | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-08-19 10:42:22 +0200 |
| commit | bca7dabd2b436b8fe8de21736c59437b5ea990af (patch) | |
| tree | 3a8c203dd2af616cf1193f0ddeb3b67550a6a249 /test/cli.t | |
| parent | b90678e560808dd788fa7d7eb319d93a83005db4 (diff) | |
| download | colitur-bca7dabd2b436b8fe8de21736c59437b5ea990af.tar.gz colitur-bca7dabd2b436b8fe8de21736c59437b5ea990af.zip | |
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.
Diffstat (limited to 'test/cli.t')
| -rw-r--r-- | test/cli.t | 48 |
1 files changed, 48 insertions, 0 deletions
@@ -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 |
