Skip to main content

max / makenotwork

bento: prove sh_quote adversarially + document /metrics posture (Run 3 security) sh_quote carried a "not bulletproof for adversarial input" disclaimer with no test backing it — the audit's one place that couldn't prove command-injection safety from the suite. It is in fact complete POSIX single-quoting; add an adversarial test that runs crafted payloads (command substitution, backticks, newlines, `$'...'`, `rm -rf`, glob/metachar soup) through a real /bin/sh and asserts each round-trips verbatim with no side effect, and correct the doc. /metrics: documented as intentionally open (Prometheus scrape convention) — it exposes only build/host telemetry, never a secret, on a tailnet-only bind. ops-exec 39 tests; clippy -D warnings clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-23 02:45 UTC
Commit: 142252fbd63452d7fbe582c0fb0f0469185a73c2
Parent: 446609c
2 files changed, +42 insertions, -3 deletions
@@ -40,6 +40,10 @@ pub fn router(state: AppState) -> Router {
40 40 .merge(mutating)
41 41 .merge(open)
42 42 .with_state(state)
43 + // `/metrics` is intentionally open (Prometheus scrape convention): it
44 + // exposes only build/host telemetry — counts, durations, app/host names —
45 + // and never a secret or token (no secret reaches a metric label). The
46 + // daemon binds the tailnet only, so the scrape surface is tailnet-internal.
43 47 .route("/metrics", get(crate::metrics::render).with_state(prom))
44 48 // Build/retry bodies are small JSON; cap request bodies well below
45 49 // axum's 2 MB default so a flood of oversized POSTs can't buffer freely.
@@ -164,9 +164,13 @@ where
164 164 total
165 165 }
166 166
167 - /// Single-quote a string for safe inclusion in a `/bin/sh` command, escaping
168 - /// any embedded single quote. Not bulletproof for adversarial input, but every
169 - /// path here comes from our own config files.
167 + /// Single-quote a string for safe inclusion in a `/bin/sh` command. This is
168 + /// complete POSIX single-quoting: the result is a single-quoted literal with
169 + /// every embedded `'` rewritten as `'\''` (close-quote, escaped quote,
170 + /// reopen-quote). Inside single quotes the shell treats every other byte —
171 + /// `$`, backtick, `;`, `\`, newline — literally, so no metacharacter can act
172 + /// and there is no way to break out. Proven adversarially by
173 + /// `sh_quote_neutralizes_injection_through_real_sh`.
170 174 pub fn sh_quote(s: &str) -> String {
171 175 let escaped = s.replace('\'', r"'\''");
172 176 format!("'{escaped}'")
@@ -208,6 +212,37 @@ mod tests {
208 212 assert_eq!(sh_quote("it's"), r"'it'\''s'");
209 213 }
210 214
215 + /// Adversarial proof: run a crafted payload through a real `/bin/sh` and
216 + /// confirm it round-trips verbatim — i.e. the shell treated it as a literal
217 + /// and no expansion, substitution, or command injection occurred.
218 + #[tokio::test]
219 + async fn sh_quote_neutralizes_injection_through_real_sh() {
220 + use tokio::process::Command;
221 + let payloads = [
222 + "v'; touch /tmp/ops-exec-should-not-exist; echo '",
223 + "$(echo pwned)",
224 + "`echo pwned`",
225 + "a\\b\\c",
226 + "line1\nline2",
227 + "$'\\x41'",
228 + "'; rm -rf / #",
229 + "${HOME}",
230 + "* ? [a-z] | & ; ( ) < >",
231 + ];
232 + for payload in payloads {
233 + let cmd = format!("printf '%s' {}", sh_quote(payload));
234 + let out = Command::new("sh").arg("-c").arg(&cmd).output().await.unwrap();
235 + assert!(out.status.success(), "sh failed for {payload:?}");
236 + assert_eq!(
237 + String::from_utf8_lossy(&out.stdout),
238 + payload,
239 + "sh_quote did not round-trip {payload:?} verbatim (injection or expansion occurred)"
240 + );
241 + }
242 + // The injection side effect must never have happened.
243 + assert!(!std::path::Path::new("/tmp/ops-exec-should-not-exist").exists());
244 + }
245 +
211 246 #[test]
212 247 fn local_detection() {
213 248 assert!(RemoteHost::new("local").is_local());