60 lines
2.6 KiB
Markdown
60 lines
2.6 KiB
Markdown
# Problems — tofu-core
|
|
|
|
Unresolved blockers and technical debt discovered during work on this plan.
|
|
|
|
_Auto-scaffolded by /start-work. Append new entries below - never overwrite._
|
|
|
|
---
|
|
|
|
## BUG FOUND (task 27 smoketest): info.d scanDepsArray — unsigned type stores signed indexOf result
|
|
|
|
**Severity:** High (crash on any recipe without `deps` field)
|
|
|
|
**Location:** `src/tofu/commands/info.d:93`
|
|
|
|
**Root cause:**
|
|
`size_t pos = content.indexOf("deps")` — indexOf returns ptrdiff_t (-1 for not found), storing it in size_t (unsigned) wraps -1 to SIZE_MAX. The guard `pos < 0` is always false for unsigned types. When deps is absent, `content[pos - 1]` accesses far out of bounds → ArrayIndexError.
|
|
|
|
**Why it escaped unit tests:** All 8 info.d unittests use recipes containing a `deps` field. The crash only triggers when `deps` is completely absent.
|
|
|
|
**Impact:** `tofu -Si` crashes on recipes without `deps` with an uncaught ArrayIndexError (D Error, not Exception — bypasses catch blocks).
|
|
|
|
**Fix (to be dispatched):** Change `size_t pos` to `ptrdiff_t pos` at line 93.
|
|
|
|
**Workaround in smoketest:** Recipe includes `deps = {}` to avoid triggering this bug.
|
|
|
|
---
|
|
|
|
## RESOLVED (task 27)
|
|
|
|
**Fixed:** `scanDepsArray` in `src/tofu/commands/info.d` — `size_t pos` → `ptrdiff_t pos`
|
|
so the `pos < 0` guard fires on `indexOf` returning -1. Added a regression unittest
|
|
(recipe without deps → infoCommand returns 0). Audit of the same pattern across
|
|
`info.d` (line 36), `recipeparse.d` (52, 112), `fetch.d` (82), `search.d`, `remove.d`,
|
|
`install.d` found no other unsigned indexOf assignment (`auto` infers `ptrdiff_t`
|
|
everywhere else).
|
|
|
|
Verified: `dub build` ✓, `dub test` — 23 modules passed ✓, smoketest — 16 checks PASS ✓.
|
|
|
|
|
|
## RESOLVED (F2 review, 2026-08-08)
|
|
|
|
**Fixed:** Both F2 findings.
|
|
|
|
1. **BLOCKER — path traversal in fetch.d (`build_script`)**: extracted
|
|
`validateBuildScriptPath(string)` (private @safe) — rejects `..`
|
|
segments, leading `/`, and `\` with `FetchException("build_script path
|
|
is unsafe: <path>")`. Wired into `fetchRecipe` step 4 before building
|
|
`bsUrl`/`bsDest`, plus defense-in-depth containment check
|
|
(`bsDest.startsWith(cacheDir ~ "/")`). Added direct unit tests and an
|
|
integration test (recipe with `build_script = "../../evil.sh"` →
|
|
FetchException, no file escapes the cache).
|
|
|
|
2. **MAJOR — `cast(int)` truncation in config.d `default_jobs`**:
|
|
range-validate before casting. `tJobs < 1 || tJobs > 1024` → stderr
|
|
warning + fallback to 1. Added tests for 5000000000→1, 1024→1024,
|
|
2048→1, -3→1.
|
|
|
|
Verified: `dub build` ✓, `dub test` — 23 modules passed ✓,
|
|
smoketest — 16 checks PASS ✓.
|