fix(cli): publish binary output atomically and preserve raw input
2 files changed, +108 −8
- cli/driver.ML modified +52 −7
- tests/cli.py modified +56 −1
cli/driver.ML modified
| @@ -6,18 +6,63 @@ fun text cs = String.implode (map (Char.chr o IntInf.toInt o Span.ordinal) cs); | ||
| 6 | 6 | |
| 7 | 7 | fun read path = |
| 8 | 8 | let |
| 9 | - val stream = if path = "-" then TextIO.stdIn else TextIO.openIn path | |
| 10 | - val text = TextIO.inputN (stream, 1048577) | |
| 11 | - val _ = if path = "-" then () else TextIO.closeIn stream | |
| 9 | + val fd = if path = "-" then Posix.FileSys.stdin | |
| 10 | + else Posix.FileSys.openf (path, Posix.FileSys.O_RDONLY, Posix.FileSys.O.flags []) | |
| 11 | + fun close () = if path = "-" then () else Posix.IO.close fd | |
| 12 | + fun loop total chunks = | |
| 13 | + let | |
| 14 | + val bytes = Posix.IO.readVec (fd, Int.min (65536, 1048577 - total)) | |
| 15 | + val count = Word8Vector.length bytes | |
| 16 | + in | |
| 17 | + if count = 0 then String.concat (List.rev chunks) | |
| 18 | + else if total + count > 1048576 then raise Fail "input exceeds 1 MiB" | |
| 19 | + else loop (total + count) (Byte.bytesToString bytes :: chunks) | |
| 20 | + end | |
| 21 | + val input = loop 0 [] handle exn => ((close () handle _ => ()); raise exn) | |
| 22 | + val _ = close () | |
| 12 | 23 | in |
| 13 | - if size text > 1048576 then raise Fail "input exceeds 1 MiB" else text | |
| 24 | + input | |
| 14 | 25 | end; |
| 15 | 26 | |
| 16 | 27 | fun write path text = |
| 17 | 28 | let |
| 18 | - val stream = BinIO.openOut path | |
| 19 | - val _ = BinIO.output (stream, Byte.stringToBytes text) | |
| 20 | - in BinIO.closeOut stream end; | |
| 29 | + val bytes = Byte.stringToBytes text | |
| 30 | + val pid = SysWord.toString (Posix.Process.pidToWord (Posix.ProcEnv.getpid ())) | |
| 31 | + val prefix = OS.Path.concat (OS.Path.dir path, "." ^ OS.Path.file path ^ "." ^ pid ^ ".") | |
| 32 | + val defaultMode = Posix.FileSys.S.flags [Posix.FileSys.S.irusr, Posix.FileSys.S.iwusr, | |
| 33 | + Posix.FileSys.S.irgrp, Posix.FileSys.S.iwgrp, Posix.FileSys.S.iroth, Posix.FileSys.S.iwoth] | |
| 34 | + val mode = Posix.FileSys.S.intersect [Posix.FileSys.ST.mode (Posix.FileSys.stat path), | |
| 35 | + Posix.FileSys.S.flags [Posix.FileSys.S.irwxu, Posix.FileSys.S.irwxg, Posix.FileSys.S.irwxo]] | |
| 36 | + handle exn as OS.SysErr (_, SOME error) => | |
| 37 | + if error = Posix.Error.noent then defaultMode else raise exn | |
| 38 | + fun reserve n = | |
| 39 | + if n = 128 then raise Fail "cannot reserve output file" | |
| 40 | + else | |
| 41 | + let | |
| 42 | + val temporary = prefix ^ Int.toString n | |
| 43 | + in | |
| 44 | + (temporary, Posix.FileSys.createf | |
| 45 | + (temporary, Posix.FileSys.O_WRONLY, Posix.FileSys.O.excl, mode)) | |
| 46 | + end handle exn as OS.SysErr (_, SOME error) => | |
| 47 | + if error = Posix.Error.exist then reserve (n + 1) else raise exn | |
| 48 | + val (temporary, fd) = reserve 0 | |
| 49 | + val closed = ref false | |
| 50 | + fun close () = (Posix.IO.close fd; closed := true) | |
| 51 | + fun discard () = | |
| 52 | + ((if !closed then () else close ()) handle _ => (); | |
| 53 | + OS.FileSys.remove temporary handle _ => ()) | |
| 54 | + fun loop offset = | |
| 55 | + if offset = Word8Vector.length bytes then () | |
| 56 | + else | |
| 57 | + let | |
| 58 | + val count = Posix.IO.writeVec (fd, Word8VectorSlice.slice (bytes, offset, NONE)) | |
| 59 | + in | |
| 60 | + if count = 0 then raise Fail "output write made no progress" else loop (offset + count) | |
| 61 | + end | |
| 62 | + in | |
| 63 | + (loop 0; close (); OS.FileSys.rename {old = temporary, new = path}) | |
| 64 | + handle exn => (discard (); raise exn) | |
| 65 | + end; | |
| 21 | 66 | |
| 22 | 67 | fun output (ok, message) = |
| 23 | 68 | (TextIO.output (if ok then TextIO.stdOut else TextIO.stdErr, text message); |
tests/cli.py modified
| @@ -1,6 +1,10 @@ | ||
| 1 | 1 | import itertools |
| 2 | +import os | |
| 2 | 3 | from pathlib import Path |
| 3 | 4 | import random |
| 5 | +import resource | |
| 6 | +import signal | |
| 7 | +import stat | |
| 4 | 8 | import subprocess |
| 5 | 9 | import tempfile |
| 6 | 10 | |
| @@ -16,7 +20,7 @@ def check(condition, message): | ||
| 16 | 20 | CHECKS += 1 |
| 17 | 21 | |
| 18 | 22 | |
| 19 | -def cli(*args, text=None, ok=True): | |
| 23 | +def cli(*args, text=None, ok=True, preexec_fn=None): | |
| 20 | 24 | result = subprocess.run( |
| 21 | 25 | [str(ROOT / "span"), *map(str, args)], |
| 22 | 26 | input=text, |
| @@ -24,11 +28,17 @@ def cli(*args, text=None, ok=True): | ||
| 24 | 28 | capture_output=True, |
| 25 | 29 | timeout=90, |
| 26 | 30 | cwd=ROOT, |
| 31 | + preexec_fn=preexec_fn, | |
| 27 | 32 | ) |
| 28 | 33 | check((result.returncode == 0) == ok, (args, result.stdout, result.stderr)) |
| 29 | 34 | return result.stdout if ok else result.stderr |
| 30 | 35 | |
| 31 | 36 | |
| 37 | +def restrict_output(): | |
| 38 | + signal.signal(signal.SIGXFSZ, signal.SIG_IGN) | |
| 39 | + resource.setrlimit(resource.RLIMIT_FSIZE, (1, 1)) | |
| 40 | + | |
| 41 | + | |
| 32 | 42 | def source(program): |
| 33 | 43 | lines = [] |
| 34 | 44 | for i, (op, target) in enumerate(program): |
| @@ -162,6 +172,51 @@ def main(): | ||
| 162 | 172 | check(out.read_bytes() == b"preserve", "failed assembly clobbered output") |
| 163 | 173 | cli("assemble", "-", "-o", out, text="") |
| 164 | 174 | check(out.read_bytes() == b"", "empty binary output") |
| 175 | + out.write_bytes(b"preserve") | |
| 176 | + before = set(Path(directory).iterdir()) | |
| 177 | + cli("assemble", "examples/countdown.span", "-o", out, ok=False, preexec_fn=restrict_output) | |
| 178 | + check(out.read_bytes() == b"preserve", "failed write clobbered output") | |
| 179 | + check(set(Path(directory).iterdir()) == before, "failed write leaked temporary file") | |
| 180 | + destination = Path(directory) / "destination" | |
| 181 | + destination.mkdir() | |
| 182 | + before = set(Path(directory).iterdir()) | |
| 183 | + cli("assemble", "examples/countdown.span", "-o", destination, ok=False) | |
| 184 | + check(destination.is_dir(), "failed rename replaced directory") | |
| 185 | + check(set(Path(directory).iterdir()) == before, "failed rename leaked temporary file") | |
| 186 | + same = Path(directory) / "source" | |
| 187 | + same.write_text("inc\nhalt\n") | |
| 188 | + cli("assemble", same, "-o", same) | |
| 189 | + check(same.read_bytes() == bytes([1, 3]), "assembly cannot replace its source") | |
| 190 | + link = Path(directory) / "link" | |
| 191 | + link.symlink_to(out) | |
| 192 | + cli("assemble", "-", "-o", link, text="halt") | |
| 193 | + check(out.read_bytes() == b"preserve", "output followed destination symlink") | |
| 194 | + check(not link.is_symlink() and link.read_bytes() == bytes([3]), "symlink not atomically replaced") | |
| 195 | + out.write_bytes(b"".join(bytes([18, n, 255 - n]) for n in range(256)) + bytes([3])) | |
| 196 | + listing = cli("disassemble", out) | |
| 197 | + raw = subprocess.run( | |
| 198 | + [str(ROOT / "span"), "disassemble", "-"], | |
| 199 | + input=out.read_bytes(), capture_output=True, timeout=90, cwd=ROOT, | |
| 200 | + ) | |
| 201 | + check(raw.returncode == 0 and raw.stdout == listing.encode("ascii"), "binary stdin changed bytes") | |
| 202 | + check(len(listing.splitlines()) == 257, "binary file changed bytes") | |
| 203 | + out.write_bytes(bytes(1048577)) | |
| 204 | + check("input exceeds 1 MiB" in cli("disassemble", out, ok=False), "binary size limit missing") | |
| 205 | + check("input exceeds 1 MiB" in cli("assemble", "-", text=" " * 1048577, ok=False), "stdin size limit missing") | |
| 206 | + out.chmod(0o600) | |
| 207 | + cli("assemble", "-", "-o", out, text="halt", preexec_fn=lambda: os.umask(0)) | |
| 208 | + check(stat.S_IMODE(out.stat().st_mode) == 0o600, "replacement widened output permissions") | |
| 209 | + protected = Path(directory) / "protected" | |
| 210 | + protected.write_bytes(b"preserve") | |
| 211 | + | |
| 212 | + def collide_output(): | |
| 213 | + out.with_name(f".{out.name}.{os.getpid():X}.0").symlink_to(protected) | |
| 214 | + | |
| 215 | + cli("assemble", "-", "-o", out, text="inc\nhalt", preexec_fn=collide_output) | |
| 216 | + check(out.read_bytes() == bytes([1, 3]), "temporary collision prevented output") | |
| 217 | + check(protected.read_bytes() == b"preserve", "temporary collision followed symlink") | |
| 218 | + collisions = list(Path(directory).glob(f".{out.name}.*")) | |
| 219 | + check(len(collisions) == 1 and collisions[0].is_symlink(), "temporary collision was removed or output leaked") | |
| 165 | 220 | print(f"{CHECKS} checks passed, including 36 exhaustive layout comparisons") |
| 166 | 221 | |
| 167 | 222 | |