Skip to main content

max / makenotwork

server: refuse pushes to the namespaces MNW owns refs/notes/mnw/* has been reserved since P2, but only against the write paths that go through validate_note_namespace: the browser, the JSON API and the notes inbox. A direct push reached none of them, so the one door anybody could walk through had no lock on it. An update hook rather than pre-receive, so a push carrying a branch and a stray refs/notes/mnw/* lands the branch and refuses the note instead of failing whole. The server writes these refs through gix ref transactions, which run no hooks, so locking the door from outside does not lock us out. Hook installation became one call for both hooks. They are not independent: post-receive reindexes a notes push and update decides whether that push is allowed, and a repository with only the first enforces nothing while looking installed. The update hook installs even without BUILD_TRIGGER_TOKEN, since it carries no HMAC and the policy holds whether or not a deployment runs builds. Existing repositories keep the hooks they were created with, so mnw-admin install-hooks has to run for this to bind.
Author: Max Johnson <me@maxj.phd> · 2026-08-09 03:13 UTC
Signed with PGP, not checked
Commit: 274f44013612c3828f16e706379f85f5e30946a4
Parent: 79ddd29
4 files changed, +91 insertions, -13 deletions
M server/src/build_runner.rs +44
@@ -168,6 +168,34 @@
168 168 done
169 169 "#;
170 170
171 + /// The `update` hook: refuse a push to a namespace MNW owns.
172 + ///
173 + /// `update` rather than `pre-receive` because it runs once per ref and rejects
174 + /// only that one. A push carrying a branch and a stray `refs/notes/mnw/*` should
175 + /// land the branch and refuse the note, not fail whole.
176 + ///
177 + /// This is the enforcement half of a policy the in-process write paths already
178 + /// hold (`validate_note_namespace`): the browser, the JSON API and the notes
179 + /// inbox all refuse the prefix. A direct push reached none of them, which left
180 + /// the one door with no lock on it. The server writes these refs through gix ref
181 + /// transactions, which run no hooks, so the door being locked from outside does
182 + /// not lock us out.
183 + ///
184 + /// Static, so it carries no HMAC and needs no per-repo generation. The `mnw`
185 + /// literal is `validation::RESERVED_NOTE_NAMESPACE`; a test pins the two
186 + /// together, since bash cannot read the constant.
187 + pub const UPDATE_HOOK: &str = r#"#!/bin/bash
188 + case "$1" in
189 + refs/notes/mnw|refs/notes/mnw/*)
190 + echo "refs/notes/mnw/* is written by makenot.work and cannot be pushed."
191 + echo "Annotate under a namespace of your own instead:"
192 + echo " git push origin refs/notes/<name>:refs/mnw/notes-inbox/<name>"
193 + exit 1
194 + ;;
195 + esac
196 + exit 0
197 + "#;
198 +
171 199 /// Compute a per-repo HMAC so the global token never touches disk.
172 200 pub fn repo_hmac(token: &str, owner: &str, repo: &str) -> String {
173 201 use hmac::{Hmac, KeyInit, Mac};
@@ -1105,6 +1133,22 @@
1105 1133 mod tests {
1106 1134 use super::*;
1107 1135
1136 + #[test]
1137 + fn the_update_hook_guards_the_namespace_validation_reserves() {
1138 + // Bash cannot read a Rust constant, so the literal in the hook is a
1139 + // copy. Renaming the reserved prefix without editing the hook would
1140 + // leave the new one pushable and the old one locked, which is the
1141 + // failure this pins: two doors, one policy.
1142 + let reserved = crate::validation::RESERVED_NOTE_NAMESPACE;
1143 + assert!(
1144 + UPDATE_HOOK.contains(&format!("refs/notes/{reserved}|refs/notes/{reserved}/*")),
1145 + "the update hook does not guard refs/notes/{reserved}/*:\n{UPDATE_HOOK}"
1146 + );
1147 + // The bare prefix and the subtree are separate patterns in a glob, and
1148 + // matching only the subtree would leave `refs/notes/mnw` itself open.
1149 + assert!(UPDATE_HOOK.contains("exit 1"), "{UPDATE_HOOK}");
1150 + }
1151 +
1108 1152 #[tokio::test]
1109 1153 async fn read_capped_truncates_to_cap() {
1110 1154 // 10k bytes through a 4k cap retains exactly 4k (the rest is drained and
M server/src/git_ssh.rs +39 -8
@@ -236,11 +236,9 @@
236 236 // Build triggers are optional, and a repo that pushes without firing one is
237 237 // a working repo. Warn rather than fail: refusing the push over a missing
238 238 // hook would trade an empty repo for an unpushable one.
239 - if let Ok(token) = std::env::var("BUILD_TRIGGER_TOKEN") {
240 - let hook = crate::build_runner::post_receive_hook(&token, owner, repo_name);
241 - if let Err(error) = install_hook_for_repo(&repo_dir, &hook) {
242 - tracing::warn!(error = ?error, path = %repo_dir.display(), "post-receive hook not installed");
243 - }
239 + let token = std::env::var("BUILD_TRIGGER_TOKEN").ok();
240 + if let Err(error) = install_hooks_for_repo(&repo_dir, token.as_deref(), owner, repo_name) {
241 + tracing::warn!(error = ?error, path = %repo_dir.display(), "hooks not installed");
244 242 }
245 243
246 244 Ok(())
@@ -330,11 +328,44 @@
330 328 }
331 329 }
332 330
333 - /// Install a post-receive hook in a bare git repository.
334 - pub fn install_hook_for_repo(repo_dir: &std::path::Path, hook_content: &str) -> anyhow::Result<()> {
331 + /// Install every hook a bare repository needs.
332 + ///
333 + /// One call rather than one per hook, because the hooks are not independent:
334 + /// `post-receive` reindexes a notes push and `update` decides whether that push
335 + /// is allowed at all, and a repository with only the first enforces no policy
336 + /// while looking installed. A caller that has to remember the second is a
337 + /// caller that eventually does not.
338 + /// `token` is optional because `post-receive` carries a per-repo HMAC and is
339 + /// useless without one, while `update` carries nothing and enforces a policy
340 + /// that holds whether or not this deployment runs builds. Gating both on the
341 + /// token would leave a server with no `BUILD_TRIGGER_TOKEN` accepting pushes to
342 + /// namespaces MNW owns.
343 + pub fn install_hooks_for_repo(
344 + repo_dir: &std::path::Path,
345 + token: Option<&str>,
346 + owner: &str,
347 + repo_name: &str,
348 + ) -> anyhow::Result<()> {
349 + if let Some(token) = token {
350 + install_hook(
351 + repo_dir,
352 + "post-receive",
353 + &crate::build_runner::post_receive_hook(token, owner, repo_name),
354 + )?;
355 + }
356 + install_hook(repo_dir, "update", crate::build_runner::UPDATE_HOOK)?;
357 + Ok(())
358 + }
359 +
360 + /// Write one executable hook into a bare repository's `hooks/`.
361 + fn install_hook(
362 + repo_dir: &std::path::Path,
363 + hook_name: &str,
364 + hook_content: &str,
365 + ) -> anyhow::Result<()> {
335 366 let hooks_dir = repo_dir.join("hooks");
336 367 std::fs::create_dir_all(&hooks_dir)?;
337 - let hook_path = hooks_dir.join("post-receive");
368 + let hook_path = hooks_dir.join(hook_name);
338 369 std::fs::write(&hook_path, hook_content)?;
339 370
340 371 #[cfg(unix)]
M server/src/bin/mnw-admin.rs +7 -4
@@ -728,14 +728,17 @@
728 728 continue;
729 729 }
730 730
731 - let hook_content =
732 - makenotwork::build_runner::post_receive_hook(&token, &owner_name, repo_name);
733 - makenotwork::git_ssh::install_hook_for_repo(&repo_path, &hook_content)?;
731 + makenotwork::git_ssh::install_hooks_for_repo(
732 + &repo_path,
733 + Some(&token),
734 + &owner_name,
735 + repo_name,
736 + )?;
734 737 installed += 1;
735 738 }
736 739 }
737 740
738 - println!("Installed post-receive hooks on {installed} repo(s).");
741 + println!("Installed hooks on {installed} repo(s).");
739 742 Ok(())
740 743 }
741 744
M server/src/validation/projects.rs +1 -1
@@ -154,7 +154,7 @@
154 154 /// once P6 mirrors them out of Postgres. Reserving it before anyone can write
155 155 /// there is the cheap order: taking a prefix back after repositories carry
156 156 /// user notes under it means deciding what happens to those notes.
157 - const RESERVED_NOTE_NAMESPACE: &str = "mnw";
157 + pub(crate) const RESERVED_NOTE_NAMESPACE: &str = "mnw";
158 158
159 159 /// Validate a git notes namespace: the part after `refs/notes/`.
160 160 ///