f20ba795c0
Phase 2.4, which I deferred during Phase 2 on a wrong assumption.
An unterminated CAD script hung the evaluator permanently. vm.eval is a
single blocking call, so `while true { s = s.translate(..) }` never
returns: the rebuild worker blocks forever, every later edit queues behind
it, and the UI sits on "Computing 3D model..." until the app is killed.
The source is user- and AI-authored, and an LLM will emit a loop like that
without much provocation.
Reproduced before fixing. A probe test was still running at 60 seconds,
and again at 90 with the budget removed.
I deferred this originally saying it needed "a cooperative check inside
the Makepad script VM or a watchdog that can kill and respawn the worker
thread", and called it a VM design decision. That was wrong, and I should
have read the VM before concluding it. makepad_script already provides
ScriptRunBudget::from_durations(soft, hard, sample_interval), checked in
run_core against a sampled Instant. The CAD module simply never set it.
The fix is six lines in eval_cad_script_in_vm. PHASE2_STATUS.md now
records the bad call rather than quietly marking the item done.
Only the hard deadline is used. A soft hit sets TimeBudgetYield, which
expects a host that drives the VM back to completion; there is no such
driver here, so a soft deadline would present as a script that silently
produced nothing at all. Soft is set equal to hard so the budget can only
ever produce a real, reported error. The previous budget is saved and
restored, so one evaluation cannot starve the next.
5 seconds, sampled every 4096 instructions. Generous deliberately: a real
document is a few hundred CSG ops and finishes in milliseconds, so
anything still running is a runaway rather than a slow model.
Result: the same script now returns "script time budget exceeded" in 5.2s.
Three tests. The runaway case is #[ignore]d because it burns the whole
budget by design; the other two guard the ways a timeout typically breaks
things -- that a normal script is unaffected across repeated evaluations
(catching a budget that is not reset), and that a syntax error still
reports itself rather than being misattributed to the timeout.
CI gate added and negative-tested: losing the budget line reintroduces a
hang that no test failure announces, only a frozen app.
617 lib + 154 integration, 0 failed.
7.4 KiB
Phase 2 — Security — status
Phase 2 of CAD_ASSESSMENT_AND_PLAN.md. Every change is covered by a test,
and the two most repeatable defect classes are now blocked in CI.
Test suite: 702 → 720 passing, 0 failing.
2.1 Build-time paths used at runtime — DONE
pub fn cad_manifest_path() -> PathBuf {
Path::new(env!("CARGO_MANIFEST_DIR")).to_path_buf() // build machine!
}
CARGO_MANIFEST_DIR is resolved at compile time. Every CAD save,
export and script load resolved against the build machine's source tree:
- On a user's install that path does not exist, so saves failed silently — the only signal was a status label.
- It leaks the developer's directory layout into the shipped binary.
- On a developer machine or CI runner, where the path does exist, the app wrote into its own checkout.
Worst instance: SessionConfig { cwd: env!("CARGO_MANIFEST_DIR") } set the
AI agent's working directory to the source tree.
Replaced with persistence::cad_data_dir() → app_data_dir()/nigig_build_store,
the convention cad_store::cad_projects_dir already used. The agent now
gets a scratch dir at <app-data>/nigig_build_store/agent.
cad_manifest_path() is kept as a #[deprecated] alias so out-of-tree
callers keep compiling while pointing somewhere safe.
Related find: system_prompt.md — the file the AI path tried to read —
does not exist in the repository. Every session silently fell back to
the one-line string "You are a CAD script generator.". The real prompt
(API surface, degree convention, finite-number requirement) is now compiled
in, with an optional override at
<app-data>/nigig_build_store/system_prompt.md.
2.2 Hardcoded LLM endpoint — DONE
pub const LOCAL_OPENAI_URL: &str = "http://10.0.0.168:8080/v1/chat/completions";
A specific RFC1918 address, shipped in the binary, selectable from a
user-facing dropdown whose status label advertised the address
("Ready: Local OpenAI stream at 10.0.0.168:8080").
On any network other than the developer's, 10.0.0.168 is somebody else's
machine. Selecting that backend POSTed the user's CAD script — and any
attached reference image — in plaintext to an arbitrary LAN host.
Now read from the environment and off by default:
| Variable | Purpose |
|---|---|
NIGIG_CAD_LOCAL_OPENAI_URL |
Endpoint. Rejected unless https:// or loopback http://. |
NIGIG_CAD_LOCAL_OPENAI_MODEL |
Model name. |
Plaintext HTTP to a non-loopback host is refused with a logged error: sending a design document unencrypted to a LAN peer should be deliberate. The status label no longer names any address, and when unconfigured it tells the operator which variables to set.
5 tests, including one asserting the previously-hardcoded IP is now rejected.
2.3 Unbounded image attachment — DONE
std::fs::read(local.path()).ok().map(|bytes| { /* base64, no checks */ })
No size limit, and the MIME type was inferred from the filename extension. A file was read whole, expanded ~1.37× into base64, stored in a process-global, interpolated into a prompt (another copy) and uploaded.
- Size cap — 8 MB, checked with
metadata()before reading, so an oversized file is never loaded into memory. - Magic-byte sniffing — PNG/JPEG/GIF/WebP/BMP identified by content.
Renaming a shell script to
.pngno longer gets its bytes uploaded. - The sniffed MIME is carried through to the prompt instead of being re-guessed from the extension.
Extracted into a testable read_attachment_as_base64 rather than left
inline in the file-picker callback. 5 tests.
2.5 Output escaping and the CDN script — DONE
SVG. Added escape_xml and routed the two non-numeric attributes
through it. This is not fixing a live injection — the collector currently
emits only numbers and colour literals — but there was no escaping helper
in the module at all, and adding a <text> label to a floor plan (the
obvious next feature) would have made it a sink.
Generated HTML viewer. model_filename was interpolated raw into both
a src="..." attribute and the page text. Now escaped; a test asserts the
attribute-breakout payload x" onerror="alert(1) cannot survive.
The <script> tag. Was an unpinned, unverified CDN load. Now carries
integrity + crossorigin, so a substituted response is rejected by the
browser rather than executed — an exported floor plan is an artifact a
contractor may open on-site months later.
Found while computing the SRI hash: the pinned URL referenced
@google/model-viewer@3.5.1, a version that does not exist. unpkg returns 404, so every exported viewer has been silently broken — the web component never loaded and the page showed nothing. Now pinned to 4.0.0 with a hash computed from the actual bytes, plus a test that the URL and hash reference the same version and a<noscript>fallback.
2.6 / 2.7 Hygiene — DONE
$USERin PDF metadata. Every exported PDF embedded the OS username in a document that typically gets emailed to a client. Now blank unlessNIGIG_CAD_PDF_AUTHORis set.- Document content in logs.
request_rebuildandeval_cad_script_in_vmlogged prefixes of the user's script unconditionally. Content is now#[cfg(debug_assertions)]; release builds log the length only.
CI enforcement
Both classes above are easy to reintroduce by copy-paste and invisible in
review, so supply-chain now fails the build on either:
env!("CARGO_MANIFEST_DIR")in CAD runtime code (comments excluded).- An RFC1918 URL literal in CAD code (comments and the tests that assert such addresses are rejected excluded).
Both were verified in both directions — they pass on the current tree, and reintroducing each defect makes them fail.
Deferred
2.4 — script execution budget. RESOLVED (see below).
eval_cad_script still has no timeout or instruction cap, so
while true { s = s.merge(cube(...)) } hangs the rebuild worker
forever.
I deferred this on the assumption that it needed a cooperative check
added to the Makepad script VM, or a watchdog to kill and respawn the
worker. That assumption was wrong, and I should have checked the VM
before concluding it. makepad_script already ships
ScriptRunBudget::from_durations(soft, hard, sample_interval), checked
inside run_core; the CAD module simply never set it. The fix is six
lines in eval_cad_script_in_vm, not a VM design change.
Reproduced the hang first (still running at 60s, then at 90s with the
budget removed), then fixed it: the same script now returns
script time budget exceeded in 5.2s. Only the hard deadline is used —
a soft hit yields with TimeBudgetYield and needs a host that resumes
the VM, which does not exist here, so soft is set equal to hard.
Covered by script_bindings::script_budget_tests and a CI gate.
2.6 (file picker for exports). Exports still write to fixed paths under the app data dir. That is no longer a security problem now that the path is not the source tree — it is a UX gap, better handled alongside the Phase 4 exporter consolidation, since all five export paths duplicate the same prologue today.
Verification
cargo test --locked -p nigig-build --lib # 720 passed; 0 failed; 7 ignored
19 new tests: 5 endpoint, 5 attachment, 5 viewer-HTML, 3 SVG escaping, plus coverage folded into existing modules.