# 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

```rust
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

```rust
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

```rust
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 `.png` no 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

- **`$USER` in PDF metadata.** Every exported PDF embedded the OS username
  in a document that typically gets emailed to a client. Now blank unless
  `NIGIG_CAD_PDF_AUTHOR` is set.
- **Document content in logs.** `request_rebuild` and `eval_cad_script_in_vm`
  logged 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:

1. `env!("CARGO_MANIFEST_DIR")` in CAD runtime code (comments excluded).
2. 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.