Three doors that were never locked

A repo-wide security review of Greenhouse turned up sixteen findings a while back. Most of them were bugs: things that were already misbehaving, quietly, waiting to be noticed. This batch is different. Nothing here was broken. Three things were just unlocked, and nobody had walked through any of the doors yet.

Door one: no content security policy at all

Tauri apps run a real webview, and like any webview, you get to tell it what it’s allowed to load and run via a CSP. Greenhouse’s config had:

"security": { "csp": null }

Null means no policy, which means no backstop. Today that’s genuinely fine: the frontend doesn’t use {@html}, doesn’t fetch anything remote, doesn’t load third-party scripts. But “fine today” is exactly the kind of sentence that stops being true the day someone adds a markdown renderer for handoff notes, or a “paste an image” feature that fetches a URL. A CSP is cheap insurance against a feature that doesn’t exist yet turning into full IPC access for anything that can get a string into the DOM. Set it to:

"csp": "default-src 'self'; style-src 'self' 'unsafe-inline'"

unsafe-inline on styles only, because Vite’s dev server injects hot-reloaded styles inline and would break without it - production ships one external stylesheet either way, so the looser rule only ever matters in dev.

Door two: a config value that gets joined onto a filesystem path with zero checking

Every pipeline stage in Greenhouse has a folder prefix - 10-active, 20-explore, and so on - and they come from a YAML file the user can hand-edit:

pub fn stage_dir(root: &Path, stage: &StageConfig) -> PathBuf {
    root.join(&stage.folder_prefix)
}

Path::join does exactly what you’d guess with an absolute path or a ../ - it doesn’t stay inside root, it goes wherever you told it to go. Nobody was going to type folder_prefix: /etc by accident. But “a config file only you can edit” and “a config file nothing validates” are two different security postures, and a typo or a bad merge in that file would have silently started writing project files outside the vault instead of erroring.

The fix is a five-line check, run once per stage every time the config loads:

fn validate_folder_prefix(prefix: &str) -> crate::Result<()> {
    let valid = !prefix.is_empty()
        && !prefix.contains('/')
        && !prefix.contains('\\')
        && prefix != "."
        && prefix != "..";
    if valid { Ok(()) } else { Err(EngineError::Invalid(...)) }
}

A single relative path segment, or the load fails loudly instead of silently relocating your files.

Door three: picking your own home folder as the vault

Setting the vault root only checked one thing: is_dir(). Not absolute, not canonicalized, and nothing stopping you from picking ~ itself in the folder browser - which would dump nine top-level directories (10-active, 00-ideas, 90-vault, and so on) straight into your actual home folder. Not a security hole exactly. A “the app just made a mess of your computer” hole, which is arguably worse for trust.

This one had a real design question buried in it: what do you do when someone picks $HOME? The original finding just said “optionally warn,” which is the kind of instruction that’s easy to over-build - there’s no toast/warning system in this app at all, and adding one just for this would be a lot of new surface for a low-severity issue. I made the call directly instead of building around it: hard block on the exact home directory, no new UI. Any other folder, including a non-empty one you’re re-adopting as an existing vault, stays untouched.

fn resolve_vault_root(path: &Path, home: Option<&Path>) -> crate::Result<PathBuf> {
    if !path.is_absolute() { return Err(...) }
    let canonical = path.canonicalize().map_err(...)?;
    if !canonical.is_dir() { return Err(...) }
    if Some(canonical.as_path()) == home { return Err(...) }
    Ok(canonical)
}

Small, pure, and fully unit-testable without touching Tauri at all - it just takes a path and an optional home dir and hands back a path or an error.

How do you verify a CSP without breaking your own trust in the test

The CSP change is the one that worried me most, because a too-strict CSP fails in a way that’s easy to miss: the app just looks broken, or subtly doesn’t load a script, and you might not notice unless you’re staring at devtools. Eyeballing npm run dev for thirty seconds isn’t a real check.

Instead I reached for the WebDriver setup this app already has for driving real UI flows, and ran the actual capture flow end to end against a build with the new CSP in place - real click, real form fill, real IPC round-trip, real confirmation screen. If the CSP had blocked a script or a style from loading, that flow simply wouldn’t have completed. It did, with the real folder path coming back from a real capture. That’s a much stronger claim than “I looked at it and it seemed fine.”

Reflection

Every fix in this batch has the same shape: something that isn’t wrong yet, but only stays that way because nobody’s tried the thing it doesn’t defend against. A missing CSP isn’t a bug until a future feature needs it to be one. An unvalidated config path isn’t a bug until someone hand-edits the wrong line. A home-directory vault isn’t a bug until someone actually picks it in the folder browser. Hardening work like this doesn’t fix anything visibly broken today - it just makes sure the app is still honest about its promises on the day something changes that it wasn’t honest about yet.