Skip to main content

max / makenotwork

Per-repo HMAC for build hooks, pinned SSH keys, archive scan opt Build hooks: replace global BUILD_TRIGGER_TOKEN in hook files with per-repo HMAC(token, owner:repo). The raw token never touches disk. Server verifies by recomputing the HMAC from the request's owner+repo fields. Both /builds/trigger and /issues/process-push updated. Existing hooks must be regenerated via mnw-admin install-hooks. SSH hardening: build runner now uses StrictHostKeyChecking=yes with a pinned known_hosts file when /opt/makenotwork/ssh/known_hosts exists. Falls back to accept-new when the file is absent (first deploy). Archive scanning: capture first 8 bytes during the initial decompression pass and reuse for nested archive magic detection, eliminating the redundant second by_index() decompression per entry. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author: Max J. <87768334+MaxJMath@users.noreply.github.com> · 2026-05-09 03:55 UTC
Commit: 90f5bee5710545c761231891c28dab29a43cc41d
Parent: bac01d6
7 files changed, +104 insertions, -66 deletions
M server/docs/todo.md +3 -3
@@ -118,7 +118,7 @@ Two-pass fuzz: initial scan + deep verification. Items marked REFUTED were dispr
118 118 - [x] MINOR: `ends_with("OK")` too permissive for ClamAV response parsing — changed to exact match `response == "stream: OK"` (`scanning/clamav.rs`)
119 119 - [ ] MINOR: Archive scan skips cover images with ZIP magic bytes — layer 1 catches type mismatch; removing skip would be defense-in-depth but no current exploit path (`scanning/archive.rs:29-36`)
120 120 - [x] MINOR: Path traversal check misses URL-encoded and Unicode normalization variants — added `%2e%2e`, absolute path, and null byte checks (`scanning/archive.rs`)
121 - - [ ] MINOR: Nested archive detection double-decompresses entries — needs refactor to save first 8 bytes during initial decompression pass (`scanning/archive.rs:83-142`)
121 + - [x] MINOR: Nested archive detection double-decompresses entries — magic bytes now captured during initial decompression pass (`scanning/archive.rs`)
122 122 - [x] NOTE: Unrecognized video data gets `Pass` instead of failing — now Fail like images (`scanning/content_type.rs`)
123 123 - [ ] NOTE: Mach-O binaries get unconditional `Pass` — needs research into suspicious Mach-O import patterns to match PE/ELF analysis (`scanning/structural.rs:44-49`)
124 124 - [x] NOTE: Download XSS blocklist missing `image/svg+xml` and `application/xhtml+xml` — added both (`scanning/content_type.rs`)
@@ -148,12 +148,12 @@ Two-pass fuzz: initial scan + deep verification. Items marked REFUTED were dispr
148 148 - [x] LOW: `cmd_ssh_repo_delete` path not canonicalized — added `canonicalize` + `starts_with` check (`git_ssh.rs`)
149 149 - [x] MEDIUM: Empty signature on automated builds — build runner now SCPs .sig files, passes to release. Key generation in human_todo (`build_runner.rs`)
150 150 - [x] MINOR: TOCTOU race between `has_running_build` and `get_pending_build` — replaced with atomic `claim_pending_build` using `FOR UPDATE SKIP LOCKED` (`db/builds.rs`, `build_runner.rs`)
151 - - [ ] LOW: Global build trigger token readable from repo hooks directory (`build_runner.rs:14-48`)
151 + - [x] LOW: Global build trigger token readable from repo hooks directory — replaced with per-repo HMAC (`build_runner.rs`)
152 152 - [x] LOW: `get_latest_release` SQL `::int[]` cast crashes on non-numeric version parts — added regex guard, non-numeric sorts last (`db/ota.rs`)
153 153 - [x] LOW: No version uniqueness enforcement on OTA releases — constraint already existed in migration 033, added `ON CONFLICT` handling returning 409 (`db/ota.rs`)
154 154 - [x] LOW: S3 key collision on same-version rebuilds — resolved by OTA version uniqueness constraint (`db/ota.rs`)
155 155 - [x] LOW: Build log stores unsanitized build output — added ANSI escape stripping (`build_runner.rs`)
156 - - [ ] LOW: `StrictHostKeyChecking=accept-new` trusts on first SSH connect (`build_runner.rs:421,455`)
156 + - [x] LOW: `StrictHostKeyChecking=accept-new` trusts on first SSH connect — now uses pinned known_hosts when `/opt/makenotwork/ssh/known_hosts` exists (`build_runner.rs`)
157 157 - ~~MINOR: `hook_trigger` doesn't check `config.enabled`~~ REFUTED — DB query includes `AND enabled = true`
158 158
159 159 ### Scheduler & Infrastructure
M server/src/bin/mnw-admin.rs +7 -7
@@ -621,7 +621,6 @@ async fn cmd_install_hooks() -> anyhow::Result<()> {
621 621 let git_root = std::env::var("GIT_REPOS_PATH")
622 622 .unwrap_or_else(|_| "/opt/git".to_string());
623 623
624 - let hook_content = makenotwork::build_runner::post_receive_hook(&token);
625 624 let mut installed = 0u32;
626 625
627 626 let root = std::path::Path::new(&git_root);
@@ -634,18 +633,19 @@ async fn cmd_install_hooks() -> anyhow::Result<()> {
634 633 if !owner_entry.file_type()?.is_dir() {
635 634 continue;
636 635 }
636 + let owner_name = owner_entry.file_name().to_string_lossy().to_string();
637 637 for repo_entry in std::fs::read_dir(owner_entry.path())? {
638 638 let repo_entry = repo_entry?;
639 639 let repo_path = repo_entry.path();
640 - if !repo_path.is_dir()
641 - || !repo_path
642 - .file_name()
643 - .and_then(|n| n.to_str())
644 - .is_some_and(|n| n.ends_with(".git"))
645 - {
640 + let repo_name = match repo_path.file_name().and_then(|n| n.to_str()) {
641 + Some(n) if n.ends_with(".git") => n.trim_end_matches(".git"),
642 + _ => continue,
643 + };
644 + if !repo_path.is_dir() {
646 645 continue;
647 646 }
648 647
648 + let hook_content = makenotwork::build_runner::post_receive_hook(&token, &owner_name, repo_name);
649 649 makenotwork::git_ssh::install_hook_for_repo(&repo_path, &hook_content)?;
650 650 installed += 1;
651 651 }
M server/src/build_runner.rs +61 -30
@@ -10,7 +10,9 @@ use crate::constants::{BUILD_MAX_LOG_BYTES, BUILD_TIMEOUT_SECS};
10 10 use crate::db::{self, BuildStatus, DbBuild, DbBuildConfig};
11 11 use crate::AppState;
12 12
13 - /// Post-receive hook script template. `__TOKEN__` is replaced with the actual token.
13 + /// Post-receive hook script template.
14 + /// `__HMAC__` is replaced with a per-repo HMAC signature so the global token
15 + /// is never stored on disk. The server verifies via `HMAC(token, owner:repo)`.
14 16 const POST_RECEIVE_HOOK_TEMPLATE: &str = r#"#!/bin/bash
15 17 while read oldrev newrev refname; do
16 18 case "$refname" in
@@ -20,7 +22,7 @@ while read oldrev newrev refname; do
20 22 REPO_NAME="$(basename "$REPO_PATH" .git)"
21 23 OWNER="$(basename "$(dirname "$REPO_PATH")")"
22 24 curl -sf -X POST \
23 - -H "Authorization: Bearer __TOKEN__" \
25 + -H "Authorization: Bearer __HMAC__" \
24 26 -H "Content-Type: application/json" \
25 27 -d "{\"repo_owner\": \"$OWNER\", \"repo_name\": \"$REPO_NAME\", \"tag\": \"$TAG\"}" \
26 28 "http://localhost:3000/api/internal/builds/trigger" \
@@ -32,7 +34,7 @@ while read oldrev newrev refname; do
32 34 REPO_NAME="$(basename "$REPO_PATH" .git)"
33 35 OWNER="$(basename "$(dirname "$REPO_PATH")")"
34 36 curl -sf -X POST \
35 - -H "Authorization: Bearer __TOKEN__" \
37 + -H "Authorization: Bearer __HMAC__" \
36 38 -H "Content-Type: application/json" \
37 39 -d "{\"repo_owner\": \"$OWNER\", \"repo_name\": \"$REPO_NAME\", \"ref_name\": \"$BRANCH\", \"before\": \"$oldrev\", \"after\": \"$newrev\"}" \
38 40 "http://localhost:3000/api/internal/issues/process-push" \
@@ -42,9 +44,20 @@ while read oldrev newrev refname; do
42 44 done
43 45 "#;
44 46
45 - /// Generate the post-receive hook script with the given token.
46 - pub fn post_receive_hook(token: &str) -> String {
47 - POST_RECEIVE_HOOK_TEMPLATE.replace("__TOKEN__", token)
47 + /// Compute a per-repo HMAC so the global token never touches disk.
48 + pub fn repo_hmac(token: &str, owner: &str, repo: &str) -> String {
49 + use hmac::{Hmac, Mac};
50 + use sha2::Sha256;
51 + let mut mac = Hmac::<Sha256>::new_from_slice(token.as_bytes())
52 + .expect("HMAC accepts any key length");
53 + mac.update(format!("{owner}:{repo}").as_bytes());
54 + hex::encode(mac.finalize().into_bytes())
55 + }
56 +
57 + /// Generate the post-receive hook script with a per-repo HMAC signature.
58 + pub fn post_receive_hook(token: &str, owner: &str, repo: &str) -> String {
59 + let hmac = repo_hmac(token, owner, repo);
60 + POST_RECEIVE_HOOK_TEMPLATE.replace("__HMAC__", &hmac)
48 61 }
49 62
50 63 /// Map (os, arch) to a Rust target triple.
@@ -415,19 +428,26 @@ async fn execute_target(
415 428 Ok((s3_key, signature))
416 429 }
417 430
431 + /// Path to a known_hosts file for build SSH connections.
432 + /// When present, StrictHostKeyChecking=yes is used (pinned keys).
433 + /// When absent, StrictHostKeyChecking=accept-new (trust on first use).
434 + const BUILD_SSH_KNOWN_HOSTS: &str = "/opt/makenotwork/ssh/known_hosts";
435 +
418 436 /// Run a command on a remote host via SSH.
419 437 async fn run_ssh_command(host: &str, command: &str) -> std::result::Result<String, String> {
438 + let mut args = vec!["-o", "ConnectTimeout=10", "-o", "BatchMode=yes"];
439 + let known_hosts_arg;
440 + if std::path::Path::new(BUILD_SSH_KNOWN_HOSTS).exists() {
441 + args.extend(["-o", "StrictHostKeyChecking=yes", "-o"]);
442 + known_hosts_arg = format!("UserKnownHostsFile={BUILD_SSH_KNOWN_HOSTS}");
443 + args.push(&known_hosts_arg);
444 + } else {
445 + args.extend(["-o", "StrictHostKeyChecking=accept-new"]);
446 + }
447 + args.push(host);
448 + args.push(command);
420 449 let output = tokio::process::Command::new("ssh")
421 - .args([
422 - "-o",
423 - "StrictHostKeyChecking=accept-new",
424 - "-o",
425 - "ConnectTimeout=10",
426 - "-o",
427 - "BatchMode=yes",
428 - host,
429 - command,
430 - ])
450 + .args(&args)
431 451 .output()
432 452 .await
433 453 .map_err(|e| format!("failed to spawn ssh: {e}"))?;
@@ -451,17 +471,19 @@ async fn run_scp_download(
451 471 local_path: &str,
452 472 ) -> std::result::Result<(), String> {
453 473 let remote = format!("{host}:{remote_path}");
474 + let mut args: Vec<&str> = vec!["-o", "ConnectTimeout=10", "-o", "BatchMode=yes"];
475 + let known_hosts_arg;
476 + if std::path::Path::new(BUILD_SSH_KNOWN_HOSTS).exists() {
477 + args.extend(["-o", "StrictHostKeyChecking=yes", "-o"]);
478 + known_hosts_arg = format!("UserKnownHostsFile={BUILD_SSH_KNOWN_HOSTS}");
479 + args.push(&known_hosts_arg);
480 + } else {
481 + args.extend(["-o", "StrictHostKeyChecking=accept-new"]);
482 + }
483 + args.push(&remote);
484 + args.push(local_path);
454 485 let output = tokio::process::Command::new("scp")
455 - .args([
456 - "-o",
457 - "StrictHostKeyChecking=accept-new",
458 - "-o",
459 - "ConnectTimeout=10",
460 - "-o",
461 - "BatchMode=yes",
462 - &remote,
463 - local_path,
464 - ])
486 + .args(&args)
465 487 .output()
466 488 .await
467 489 .map_err(|e| format!("failed to spawn scp: {e}"))?;
@@ -622,14 +644,23 @@ mod tests {
622 644 }
623 645
624 646 #[test]
625 - fn hook_template_contains_token() {
626 - let hook = post_receive_hook("secret-token-123");
627 - assert!(hook.contains("secret-token-123"));
628 - assert!(!hook.contains("__TOKEN__"));
647 + fn hook_template_contains_hmac_not_raw_token() {
648 + let hook = post_receive_hook("secret-token-123", "alice", "myrepo");
649 + let expected_hmac = repo_hmac("secret-token-123", "alice", "myrepo");
650 + assert!(hook.contains(&expected_hmac), "hook should contain per-repo HMAC");
651 + assert!(!hook.contains("secret-token-123"), "hook must not contain raw token");
652 + assert!(!hook.contains("__HMAC__"), "placeholder should be replaced");
629 653 assert!(hook.contains("/api/internal/builds/trigger"));
630 654 }
631 655
632 656 #[test]
657 + fn repo_hmac_differs_per_repo() {
658 + let h1 = repo_hmac("token", "alice", "repo-a");
659 + let h2 = repo_hmac("token", "alice", "repo-b");
660 + assert_ne!(h1, h2, "different repos should produce different HMACs");
661 + }
662 +
663 + #[test]
633 664 fn shell_escape_basic() {
634 665 assert_eq!(shell_escape("hello"), "'hello'");
635 666 assert_eq!(shell_escape("it's"), "'it'\\''s'");
M server/src/routes/api/projects.rs +1 -1
@@ -409,7 +409,7 @@ pub(super) async fn create_repo(
409 409 if let Some(token) = &state.config.build_trigger_token {
410 410 let hooks_dir = repo_dir.join("hooks");
411 411 let hook_path = hooks_dir.join("post-receive");
412 - let hook_content = crate::build_runner::post_receive_hook(token);
412 + let hook_content = crate::build_runner::post_receive_hook(token, &username, name);
413 413 if let Err(e) = std::fs::write(&hook_path, &hook_content) {
414 414 tracing::warn!(error = ?e, "failed to install post-receive hook");
415 415 } else {
M server/src/routes/builds.rs +6 -4
@@ -421,8 +421,9 @@ async fn hook_trigger(
421 421 headers: axum::http::HeaderMap,
422 422 Json(req): Json<HookTriggerRequest>,
423 423 ) -> Result<impl IntoResponse> {
424 - // Authenticate via BUILD_TRIGGER_TOKEN
425 - let expected_token = state
424 + // Authenticate via per-repo HMAC derived from BUILD_TRIGGER_TOKEN.
425 + // The hook file contains HMAC(token, owner:repo), not the raw token.
426 + let trigger_token = state
426 427 .config
427 428 .build_trigger_token
428 429 .as_deref()
@@ -433,11 +434,12 @@ async fn hook_trigger(
433 434 .and_then(|v| v.to_str().ok())
434 435 .ok_or(AppError::Unauthorized)?;
435 436
436 - let token = auth_header
437 + let provided_hmac = auth_header
437 438 .strip_prefix("Bearer ")
438 439 .ok_or(AppError::Unauthorized)?;
439 440
440 - if !crate::helpers::constant_time_compare(token, expected_token) {
441 + let expected_hmac = crate::build_runner::repo_hmac(trigger_token, &req.repo_owner, &req.repo_name);
442 + if !crate::helpers::constant_time_compare(provided_hmac, &expected_hmac) {
441 443 return Err(AppError::Unauthorized);
442 444 }
443 445
M server/src/routes/git_issues/push_refs.rs +5 -4
@@ -102,17 +102,18 @@ pub(super) async fn process_push(
102 102 headers: axum::http::HeaderMap,
103 103 Json(req): Json<ProcessPushRequest>,
104 104 ) -> Result<impl axum::response::IntoResponse> {
105 - // Validate Bearer token
106 - let expected_token = state.config.build_trigger_token.as_deref()
105 + // Validate per-repo HMAC (derived from BUILD_TRIGGER_TOKEN, never stored raw)
106 + let trigger_token = state.config.build_trigger_token.as_deref()
107 107 .ok_or(AppError::Internal(anyhow::anyhow!("BUILD_TRIGGER_TOKEN not configured")))?;
108 108
109 109 let auth_header = headers
110 110 .get("authorization")
111 111 .and_then(|v| v.to_str().ok())
112 112 .unwrap_or("");
113 - let provided_token = auth_header.strip_prefix("Bearer ").unwrap_or("");
113 + let provided_hmac = auth_header.strip_prefix("Bearer ").unwrap_or("");
114 114
115 - if provided_token.is_empty() || provided_token != expected_token {
115 + let expected_hmac = crate::build_runner::repo_hmac(trigger_token, &req.repo_owner, &req.repo_name);
116 + if provided_hmac.is_empty() || !crate::helpers::constant_time_compare(provided_hmac, &expected_hmac) {
116 117 return Err(AppError::Forbidden);
117 118 }
118 119
M server/src/scanning/archive.rs +21 -17
@@ -84,6 +84,10 @@ pub fn check_archive_safety(data: &[u8], file_type: FileType) -> LayerResult {
84 84
85 85 // Use actual decompressed byte count instead of trusting the claimed size
86 86 // from the ZIP central directory (which is attacker-controlled).
87 + // Also capture the first 8 bytes for nested archive magic detection,
88 + // avoiding a second decompression pass.
89 + let mut magic_bytes = [0u8; 8];
90 + let mut magic_len = 0usize;
87 91 let actual_size = match archive.by_index(i) {
88 92 Ok(mut reader) => {
89 93 let mut counted: u64 = 0;
@@ -93,6 +97,12 @@ pub fn check_archive_safety(data: &[u8], file_type: FileType) -> LayerResult {
93 97 match std::io::Read::read(&mut reader, &mut buf) {
94 98 Ok(0) => break,
95 99 Ok(n) => {
100 + // Capture first 8 bytes for magic detection
101 + if magic_len < 8 {
102 + let copy = n.min(8 - magic_len);
103 + magic_bytes[magic_len..magic_len + copy].copy_from_slice(&buf[..copy]);
104 + magic_len += copy;
105 + }
96 106 counted += n as u64;
97 107 if counted > limit {
98 108 return LayerResult {
@@ -122,7 +132,7 @@ pub fn check_archive_safety(data: &[u8], file_type: FileType) -> LayerResult {
122 132 total_uncompressed += actual_size;
123 133
124 134 // Check for nested archives — extension check first, then magic bytes
125 - // for entries small enough to read (avoids decompressing huge entries).
135 + // from the first decompression pass (no re-read needed).
126 136 let lower_name = name.to_lowercase();
127 137 let ext_match = lower_name.ends_with(".zip")
128 138 || lower_name.ends_with(".tar.gz")
@@ -132,22 +142,16 @@ pub fn check_archive_safety(data: &[u8], file_type: FileType) -> LayerResult {
132 142 || lower_name.ends_with(".tar");
133 143 if ext_match {
134 144 nested_archives += 1;
135 - } else if actual_size > 0 && actual_size < 256 * 1024 * 1024 {
136 - // For non-obvious extensions, check magic bytes if entry is small enough
137 - if let Ok(mut readable) = archive.by_index(i) {
138 - let mut magic = [0u8; 8];
139 - if std::io::Read::read(&mut readable, &mut magic).unwrap_or(0) >= 4 {
140 - let is_nested = matches!(
141 - magic,
142 - [0x50, 0x4B, 0x03, 0x04, ..] // ZIP
143 - | [0x1F, 0x8B, ..] // gzip (tar.gz)
144 - | [0x37, 0x7A, 0xBC, 0xAF, 0x27, 0x1C, ..] // 7z
145 - | [0x52, 0x61, 0x72, 0x21, ..] // RAR
146 - );
147 - if is_nested {
148 - nested_archives += 1;
149 - }
150 - }
145 + } else if actual_size > 0 && magic_len >= 4 {
146 + let is_nested = matches!(
147 + magic_bytes,
148 + [0x50, 0x4B, 0x03, 0x04, ..] // ZIP
149 + | [0x1F, 0x8B, ..] // gzip (tar.gz)
150 + | [0x37, 0x7A, 0xBC, 0xAF, 0x27, 0x1C, ..] // 7z
151 + | [0x52, 0x61, 0x72, 0x21, ..] // RAR
152 + );
153 + if is_nested {
154 + nested_archives += 1;
151 155 }
152 156 }
153 157 }