security: audit Command call sites; confirm no argument injection

Closes #18

All six Command::new call sites audited:
- bitwarden.rs (sh -c): shell string is a hardcoded literal; no user data.
- bitwarden.rs (bw unlock): all args hardcoded.
- bitwarden.rs (bw get): item_name and session passed as discrete .args() elements; no shell.
- nothooks/runner.rs (nu/sh/etc): interp and script path passed as discrete args; no shell.
- notstrap/repo.rs (git clone): url and dest passed as discrete .arg() calls; no shell.
- notfiles/tests/integration.rs: test harness with hardcoded args only.

Added SAFETY comments on the three sites that handle user-controlled config values.
This commit is contained in:
Joseph O'Brien
2026-04-11 23:52:40 -04:00
parent 822abd9c88
commit 19e74b4a1b
3 changed files with 10 additions and 0 deletions

View File

@@ -89,6 +89,10 @@ impl HookRunner {
Ok(i) => i, Ok(i) => i,
Err(msg) => return HookResult::Failed(msg), Err(msg) => return HookResult::Failed(msg),
}; };
// SAFETY: `interp` is derived from the file extension or an explicit `interpreter` field
// in HookSpec (both come from config, not untrusted shell input). `spec.script` is a
// filesystem path from config. Both are passed as discrete args with no shell involved,
// so argument injection is not possible.
let result = Command::new(&interp).arg(&spec.script).status(); let result = Command::new(&interp).arg(&spec.script).status();
match result { match result {

View File

@@ -83,6 +83,9 @@ impl BitwardenSource {
session session
}; };
// SAFETY: `item_name` and `session` are user-controlled values (config item name and
// BW_SESSION env var / bw-unlock stdout), but are passed as discrete `.args()` elements,
// not interpolated into a shell string. No shell is involved, so injection is not possible.
let output = Command::new("bw") let output = Command::new("bw")
.args(["get", "notes", &self.item_name, "--session", &session]) .args(["get", "notes", &self.item_name, "--session", &session])
.output() .output()

View File

@@ -6,6 +6,9 @@ pub fn clone_if_missing(url: &str, dest: &Path) -> Result<bool> {
if dest.exists() { if dest.exists() {
return Ok(false); return Ok(false);
} }
// SAFETY: `url` comes from the notstrap.toml config and `dest` is a `Path` derived from
// the resolved dotfiles directory. Both are passed as discrete `.arg()` calls with no
// shell involved, so argument injection is not possible.
let status = Command::new("git") let status = Command::new("git")
.arg("clone") .arg("clone")
.arg(url) .arg(url)