================================================================================ F2 TOFU-CORE CODE QUALITY AUDIT — FINAL VERDICT ================================================================================ Date: 2026-08-08 Reviewer: Sisyphus-Junior (independent agent) Target: tofu-core Dlang codebase (24 .d files, 11,018 LOC) Branch: current HEAD Build: dmd 2.112 (debug, warningsAsErrors), ldc2 (release) ================================================================================ VERDICT: REJECTED ================================================================================ Reason: 1 BLOCKER (C10 — path traversal in fetch.d), 1 MAJOR (C5 — unsafe cast(int) in config.d), 2 MINOR findings. See per-check table below for details. ================================================================================ PER-CHECK SUMMARY TABLE ─────────────────────── Check | Name | Result | Severity (if fail) ───────┼─────────────────────────┼──────────┼─────────────────── C1 | dub build (warnings=err)| PASS | — C2 | @safe/@system audit | PASS | — C3 | Hardcoded paths | PASS | — C4 | Exception handling | PASS | — C5 | cast() audit | FAIL | MAJOR (1), MINOR (1) C6 | Error message quality | PASS* | MINOR (2 non-actionable) C7 | Unbounded memory growth | PASS | — C8 | Concurrency (spinner) | PASS | — C9 | Module structure (<1KL) | PASS | — C10 | Security (sandbox+path) | FAIL | BLOCKER (1) ─────────────────────────────────────────────────────────────────────────────── ================================================================================ DETAILED EVIDENCE PER CHECK ================================================================================ ─── C1: dub build with warnings-as-errors ──────────────────────────────────── Build 1: dmd debug (default) Command: dub build Result: PASS Evidence: Warning: only DUB-level deprecation about "warningsAsErrors" buildOption (DUB recommends "buildRequirements" instead — cosmetic, not a code defect). Zero compiler warnings, zero errors. Output: Starting Performing "debug" build using /usr/bin/dmd for x86_64. Up-to-date toml 1.0.0: target for configuration [library] is up to date. Up-to-date tofu ~main: target for configuration [application] is up to date. Finished Build 2: ldc2 release (-b release --compiler=ldc2) Result: PASS Evidence: Starting Performing "release" build using ldc2 for x86_64. Building toml 1.0.0: building configuration [library] Building tofu ~main: building configuration [application] Linking tofu Zero warnings, zero errors. Build 3: dmd --force (clean rebuild) Command: dub build --force Result: PASS Only linker messages, zero compiler warnings: Building toml 1.0.0: building configuration [library] Building tofu ~main: building configuration [application] Linking tofu Sub-verdict: PASS ─── C2: @safe audit ────────────────────────────────────────────────────────── Check: grep -rn '@system' src/ Result: PASS — zero @system function annotations anywhere in the codebase Evidence: All 5 grep hits are documentation comments explaining WHY a particular section uses @trusted (e.g. "stdin/stdout are @system in dmd 2.112"). No actual @system annotation on any function. $ grep -rn '@system' src/ src/tofu/log.d:49:/// `@trusted`: `stdout` itself is `@system` to access in dmd 2.112. src/tofu/http.d:5:/// `@safe`; the `@system` curl internals are isolated behind `@trusted` src/tofu/http.d:74:// @trusted helpers — all @system curl calls are isolated here src/tofu/state.d:36:// ─── @trusted wrappers (fs + json — @system in Phobos) ────── src/tofu/commands/upgrade.d:36:// ─── @trusted I/O wrappers (stdout/stdin are @system in DMD 2.112) ─── Annotations count: - @safe functions/blocks: 242 - @trusted functions/blocks: 28 - Ratio: ~8.6:1 safe-to-trusted - @system blocks (`@system:`): ZERO Public API coverage: All public API functions are either @safe or @trusted with documented rationale in module-level doc comments. Filesystem, process, and socket operations are isolated behind @trusted helpers with explicit comments. Sub-verdict: PASS ─── C3: Hardcoded paths audit ──────────────────────────────────────────────── Check: grep -rn '"/tmp/\|~/\|"/usr/\|home/' src/ --include='*.d' Result: PASS — all hits are in test code or documentation Evidence: src/tofu/config.d:117: // doc comment: "~/.config/tofu/config.toml" src/tofu/config.d:130: cfg.cacheDir = expandTilde("~/.cache/tofu"); ← DEFAULT, env-overridable src/tofu/config.d:279: test assertion — assert(cfg.cacheDir == expandTilde("~/.cache/tofu")) src/tofu/config.d:300-302: test TOML fixture string content src/tofu/config.d:308-310: test assertions on TOML fixture src/tofu/config.d:356-362: test assertions with env override src/tofu/config.d:424-431: test assertions with env override src/tofu/build.d:142,181: error messages mentioning config file path (informational) src/tofu/build.d:500: test assertion — assert(pkgNameFromPath("/tmp/...") == "build") src/tofu/build.d:585: test fixture — cfg.cacheDir = "/tmp/dummy" src/tofu/errors.d:287: comment — "On a fresh system ~/.cache/tofu may not exist yet." Analysis: - All hardcoded paths are in test assertions, test fixture strings, or informational error messages. - The config module (config.d:130) is the sole owner of path defaults via `expandTilde("~/.cache/tofu")` — overridable by TOML or env var. - No production code path uses hardcoded absolute paths. Sub-verdict: PASS ─── C4: Exception handling ─────────────────────────────────────────────────── Check 4a: main.d has try/catch around dispatch Result: PASS Evidence: src/main.d lines 101-127: try { final switch (pa.cmd) { case Command.help: ... return 0; case Command.install: return installCommand(...); case Command.search: return searchCommand(...); case Command.upgrade: return upgradeCommand(...); case Command.remove_: return removeCommand(...); case Command.info: return infoCommand(...); } } catch (Exception e) { int ec = exitCodeFor(e); logError("%s", e.msg); return ec; } Note: catches `Exception`, not `Throwable` — correct D idiom (Errors like OutOfMemoryError should NOT be caught). PASS. Lock management: `scope(exit) releaseLock(cacheDir)` at line 81, `scope(failure) releaseLock(cacheDir)` at line 82 — correct. Check 4b: grep 'catch.*Throwable' Result: PASS — all occurrences in unittest cleanup only Evidence: src/tofu/http.d:230: catch (Throwable) {} ← oneShotResponder test util src/tofu/index.d:403: catch (Throwable) {} ← oneShotResponder test util src/tofu/binary.d:252: catch (Throwable) {} ← oneShotResponder test util src/tofu/fetch.d:368: catch (Throwable) ← waitForPort test util src/tofu/fetch.d:381: catch (Throwable) {} ← killServer test util src/tofu/fetch.d:412: catch (Throwable) {} ← removeDir test util All 6 occurrences are inside `version(unittest)` blocks — test helpers where silently discarding errors during cleanup is acceptable (one-shot TCP responders, wait loops, server teardown). No production code path swallows Throwable. Sub-verdict: PASS ─── C5: cast() audit ───────────────────────────────────────────────────────── Check: grep -rn 'cast(' src/ Result: FAIL — one MAJOR finding, one MINOR finding Findings: [MAJOR] src/tofu/config.d:227 — unsafe cast(int) tJobs ────────────────────────────────────────────────────────── Line: cfg.defaultJobs = cast(int) tJobs; Context: tJobs is `long.min` sentinel or a TOML integer (long). `cfg.defaultJobs` is `int` (32-bit signed). Issue: `cast(int)` silently truncates large values. e.g. TOML `default_jobs = 5000000000` wraps to negative int. Impact: Low (absurd job counts not realistic), but violates defensive coding. The parsePositiveInt in cli.d correctly validates via `to!int` (which throws on overflow); the config path should similarly validate. Fix: Replace `cast(int) tJobs` with `to!int(tJobs)` or clamp the value. (NOT fixing — reporting only.) [MINOR] src/tofu/commands/install.d:367 — cast(PackageIndex[])[] in test ────────────────────────────────────────────────────────── Line: delegate () @safe { return cast(PackageIndex[])[]; } Context: Test delegate to inject an empty index for testing "package not found" (exit code 2). Issue: Raw cast of `[]` literal. Idiom would be `(PackageIndex[]).init` or `new PackageIndex[0]`. Impact: Test-only, zero runtime consequence. Severity: MINOR PASS items (all in @trusted or test context): - errors.d:80-139: cast(TofuError/HttpException/...) — safe downcast - errors.d:216: cast(pid_t) — safe integer cast in POSIX-specific code - http.d:98: cast(string) data — in @trusted getImpl, ubyte[]→string - http.d:223,242: cast()/cast(shared) listener — test-only TCP server - vercmp.d:113: cast(string) buf — in @trusted lambda, char[]→string - index.d:278: cast() entries — in @trusted block, extracting parsed.array - index.d:396,416: cast()/cast(shared) listener — test-only TCP server - binary.d:245,265: cast()/cast(shared) listener — test-only TCP server - fetch.d:350: cast(InternetAddress) — test-only findFreePort - commands/info.d:83,151: cast(string) — in @trusted lambdas Sub-verdict: FAIL (MAJOR: config.d:227) ─── C6: Error message quality ──────────────────────────────────────────────── Check: spot-check 5 throw sites across modules — each must name WHAT failed and suggest HOW to fix. Site 1: src/tofu/build.d:141-144 Message: "zeta-makepkg not found. Install zeta-toolchain or set TOFU_ZETA_TOOLCHAIN_PATH in ~/.config/tofu/config.toml" WHAT: zeta-makepkg not found ✓ HOW: install zeta-toolchain OR set config path ✓ Verdict: PASS — fully actionable Site 2: src/tofu/fetch.d:277-279 Message: "build script '%s' not found for package '%s'" WHAT: which script, which package ✓ HOW: no suggestion — user doesn't know HOW to resolve Verdict: MINOR — names WHAT clearly but lacks a suggestion (e.g. "check the recipe's build_script field") Site 3: src/tofu/http.d:113-114 Message: "cannot reach ZUUR at : timeout after 120s" WHAT: which URL timed out ✓ HOW: no suggestion — user doesn't know HOW to fix network issues Verdict: MINOR — names WHAT clearly but no suggestion (e.g. "check your network connection or TOFU_ZUUR_URL") Site 4: src/tofu/index.d:223-224 Message: "ZUUR index is invalid: lua command not found on PATH" WHAT: lua not found ✓ HOW: implies "install lua" ✓ Verdict: PASS — actionable Site 5: src/tofu/cli.d:213-214 Message: "unknown option: -Z (run 'tofu --help')" WHAT: which option is unknown ✓ HOW: run --help for valid options ✓ Verdict: PASS — fully actionable Summary: 3 of 5 fully actionable, 2 lack HOW guidance. Minor room for improvement but not a rejection-level issue. Sub-verdict: PASS (with 2 MINOR notes) ─── C7: Unbounded memory growth in hot paths ───────────────────────────────── Check 7a: http.d onReceive string accumulator Evidence: http.d:96-99 http.onReceive = (ubyte[] data) { content ~= cast(string) data; return data.length; }; Analysis: The `get()` function accumulates the full response body into a string. Used for fetching the ZUUR index (index.lua) — typically a few KB. Also used for fetching binary package.lua manifests (a few hundred bytes). Acceptable for these use cases. The `downloadFile()` function writes to disk, not memory. Verdict: PASS Check 7b: stderr ring buffer bound Evidence: build.d:46-63,75 private final class StderrRing { string[] _lines; size_t cap; ... void add(string line) { _lines ~= line; if (_lines.length > cap) _lines = _lines[1 .. $]; // drop oldest, keep cap } } auto ring = new StderrRing(20); // ← confirmed: 20-line bound Analysis: Ring buffer initialized at 20 lines (line 75). `add()` drops oldest line when exceeding capacity. Reader thread joins before `ring.get()` is called, ensuring happens-before. Verdict: PASS Check 7c: spinner thread join Evidence: ui.d:95-106 — `_thread.join()` wrapped in try/catch Verdict: PASS (deferred to C8) Sub-verdict: PASS ─── C8: Concurrency — ui.d spinner thread ──────────────────────────────────── Check: spinner clean stop — no join hang, shared bool flag Evidence: ui.d:31-119 shared bool _running; // line 35 — shared flag ... void threadFn() { while (_running) { // line 50 — checked every iteration ... // 100ms sleep per iteration } } void stop() { _running = false; // line 93 — signal to stop if (_thread !is null) { try { _thread.join(); // line 99 — join, max wait ~100ms } catch (Exception) { // double-join or thread failure — harmless } _thread = null; // prevent double-join } } Analysis: - `_running` is `shared bool` — correct for cross-thread communication - Thread loop checks `_running` every 100ms (Thread.sleep at line 55) - `stop()` sets the flag, then joins — max join delay ~100ms - Double-join prevented by `_thread = null` after join - Exception during join caught and discarded (benign) - No hang risk — flag is simple bool, no mutex contention - Non-TTY mode skips thread entirely (lines 89-90) Sub-verdict: PASS ─── C9: Module structure ───────────────────────────────────────────────────── Check: wc -l src/tofu/*.d src/tofu/commands/*.d Result: PASS — all modules under 1000 lines Lines Module ───── ────────────────────── 851 src/tofu/resolve.d ← largest 708 src/tofu/install.d 695 src/tofu/build.d 694 src/tofu/index.d 644 src/tofu/fetch.d 600 src/tofu/errors.d 554 src/tofu/commands/install.d 545 src/tofu/commands/upgrade.d 541 src/tofu/types.d 472 src/tofu/recipeparse.d 462 src/tofu/commands/info.d 435 src/tofu/config.d 435 src/tofu/cache.d 390 src/tofu/binary.d 376 src/tofu/commands/search.d 366 src/tofu/state.d 363 src/tofu/cli.d 356 src/tofu/http.d 346 src/tofu/vercmp.d 334 src/tofu/deps.d 330 src/tofu/commands/remove.d 275 src/tofu/ui.d 242 src/tofu/log.d 4 src/tofu/package.d ← module re-export only ───── 11018 TOTAL Largest module (resolve.d, 851 lines) is well under the 1000-line ceiling. No file requires an override justification. Sub-verdict: PASS ─── C10: Security ──────────────────────────────────────────────────────────── Check 10a: index.d Lua sandbox whitelist Result: PASS Evidence: - Whitelist defined at index.d:70-91 (Lua sandboxLuaScript constant) - Stripped globals: io, os, require, dofile, loadfile, loadstring, package, debug - Test 2 (line 471): os.execute("rm -rf /") → IndexException caught, sentinel file survives → sandbox works - Test 3 (line 514): io.open("/etc/shadow") → IndexException caught - ZETA lib/sandbox.lua port, Lua 5.1/5.2+ compatible - Confirmed: hunkered-down whitelist, NOT a blacklist Check 10b: fetch.d path traversal Result: FAIL — BLOCKER Evidence: ├─ src/tofu/fetch.d:253-258: │ │ auto buildScriptPath = extractBuildScript(recipeContent); │ if (buildScriptPath.length > 0) { │ logDetail("custom build system: fetching %s", buildScriptPath); │ auto bsUrl = cfg.recipesUrl(name) ~ "/" ~ buildScriptPath; │ auto bsDest = cacheDir ~ "/" ~ buildScriptPath; // ← UNSANITIZED │ │ () @trusted { │ auto bsDir = bsDest.dirName; │ if (bsDir.length > 0 && !exists(bsDir)) │ mkdirRecurse(bsDir); │ }(); │ ... │ downloadFile(bsUrl, bsDest); // writes to escaped path │ } │ └─ PROBLEM: No sanitization of `buildScriptPath`. ATTACK VECTOR: 1. Malicious recipe in ZUUR includes: build_system = "custom" build_script = "../../../.bashrc" 2. extractBuildScript returns "../../../.bashrc" 3. bsDest = "~/.cache/tofu/../../../.bashrc" → resolves to "$HOME/.bashrc" 4. downloadFile creates $HOME/.bashrc.part, then renames to $HOME/.bashrc 5. Result: user's .bashrc is overwritten with attacker-controlled content Confirmations: - Zero occurrences of ".." sanitization anywhere in src/ (confirmed via grep for indexOf/canFind/contains/startsWith of "..") - No expandTilde, buildPath, normalizePath, or absolutePath applied to buildScriptPath or bsDest in the build_script code path - mkdirRecurse and exists() resolve ".." through the OS kernel Severity: BLOCKER The .recipe file content comes from the remote ZUUR repository. A malicious maintainer (or compromised repository) can write arbitrary files anywhere the user has write access. Required fix (not implemented — reporting only): - Reject buildScriptPath containing ".." before constructing bsDest - OR resolve bsDest to absolute canonical path and verify it starts with cacheDir Sub-verdict: FAIL (BLOCKER: path traversal in fetch.d) ================================================================================ FINDINGS SUMMARY TABLE ================================================================================ # | Check | Severity | Location | Description ───┼───────┼──────────┼────────────────────┼───────────────────────────────── 1 | C10 | BLOCKER | fetch.d:253-258 | Path traversal: buildScriptPath | | | | from .recipe used unsanitized in | | | | bsDest, can escape cache dir via | | | | "../" in build_script field ───┼───────┼──────────┼────────────────────┼───────────────────────────────── 2 | C5 | MAJOR | config.d:227 | cast(int) tJobs silently truncates | | | | large TOML default_jobs values ───┼───────┼──────────┼────────────────────┼───────────────────────────────── 3 | C6 | MINOR | fetch.d:277-279 | Error message lacks HOW guidance | | | | for missing build script 4 | C6 | MINOR | http.d:113-114 | Error message lacks HOW guidance | | | | for timeout/network errors 5 | C5 | MINOR | install.d:367 | cast(PackageIndex[])[] in test — | | | | raw cast of empty array literal ───┴───────┴──────────┴────────────────────┴───────────────────────────────── ================================================================================ PASS COUNTS ================================================================================ 8 of 10 checks PASS: C1, C2, C3, C4, C6*, C7, C8, C9 1 of 10 checks FAIL (non-blocker): C5 (MAJOR + MINOR) 1 of 10 checks FAIL (blocker): C10 (BLOCKER) ── *C6 PASS but with 2 MINOR non-actionable error message notes ================================================================================ OVERALL VERDICT: REJECTED ================================================================================ The codebase is well-structured, compiles cleanly, handles concurrency correctly, and has no hardcoded paths or unsafe exception swallowing. The Lua sandbox is properly implemented. However, the path traversal vulnerability in fetch.d (C10) is a BLOCKER that could allow a malicious ZUUR recipe to write files outside the cache directory — this MUST be fixed before approval. Recommended fix (not applied — audit only): In fetch.d, after line 254, add: if (buildScriptPath.indexOf("..") >= 0) throw new FetchException( "build_script path contains '..': " ~ buildScriptPath); This is a one-line fix. Re-audit of C10 after the fix is applied should be trivial. ================================================================================ ================================================================================ F2 FIX APPLIED — build_script path traversal + default_jobs truncation ================================================================================ Date: 2026-08-08 FINDING 1 (BLOCKER, fetch.d:253-258) — RESOLVED src/tofu/fetch.d: added private @safe validateBuildScriptPath(string) - rejects ".." segments, leading '/' (absolute), and '\' (backslash) with: throw new FetchException("build_script path is unsafe: ") - wired into fetchRecipe step 4 BEFORE building bsUrl/bsDest - defense in depth: after joining, throws if !bsDest.startsWith(cacheDir ~ "/") - valid relative paths unchanged (build.sh, scripts/build.sh pass) FINDING 2 (MAJOR, config.d:227) — RESOLVED src/tofu/config.d: default_jobs from TOML now range-validated (1..1024) - tJobs < 1 || tJobs > 1024 -> stderr warning "Warning: invalid default_jobs in config, using default 1" and cfg.defaultJobs = 1 (no silent cast(int) truncation) - else cfg.defaultJobs = cast(int) tJobs TESTS ADDED fetch.d: direct validateBuildScriptPath unittests (evil paths rejected, valid paths pass) + integration Test 6: recipe with build_script="../../evil.sh" -> FetchException, nothing escapes cache config.d: default_jobs = 5000000000 -> 1, 1024 -> 1024, 2048 -> 1, -3 -> 1 VERIFICATION dub build : PASS (warningsAsErrors, no warnings) dub test : 23 modules passed unittests smoketest : PASS 16 checks passed