From f40a9af7a03f777224e5ab2dccb08f20f92df588 Mon Sep 17 00:00:00 2001 From: Noah Pederson Date: Mon, 25 May 2026 21:16:55 -0500 Subject: [PATCH] Make post-receive event writes atomic --- docs/deployment.md | 1 - hooks/post-receive.rc | 30 ++++++++++++++++++++---------- tests/integration.rc | 27 +++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/docs/deployment.md b/docs/deployment.md index e762a56..efa1ffb 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -147,7 +147,6 @@ These are known deployment risks tracked as follow-up work: - Queue events are shape-validated but not authenticated. Protect `$GIT_DIR/sip/queue` with service-owned permissions and add event signing or a MAC before relying on queue provenance. -- Post-receive event writes are not yet atomic. - Queued event refs need the same `git check-ref-format` validation used for protected workflow refs. - Pull and merge namespaces should remain rejected until their system-owned diff --git a/hooks/post-receive.rc b/hooks/post-receive.rc index 10d5838..7399282 100755 --- a/hooks/post-receive.rc +++ b/hooks/post-receive.rc @@ -28,15 +28,21 @@ function clean_actor(actor) { return actor } -function event_path(id, n, path) { - path = queue "/" id ".event" - n = 0 - while ((getline < path) >= 0) { - close(path) - n++ - path = queue "/" id "-" n ".event" +function shell_quote(value, q, bs) { + q = sprintf("%c", 39) + bs = sprintf("%c", 92) + gsub(q, q bs q q, value) + return q value q +} + +function temp_event_path(command, path) { + command = "mktemp " shell_quote(queue "/event.XXXXXXXXXX") + if ((command | getline path) != 1) { + close(command) + print "sip: could not create temporary event in " queue > "/dev/stderr" + exit 1 } - close(path) + close(command) return path } @@ -46,8 +52,8 @@ $3 ~ "^refs/(heads|tags)/" { exit 1 } - id = queued_at "-" NR "-" substr($1, 1, 8) "-" substr($2, 1, 12) - path = event_path(id) + path = temp_event_path() + final = path ".event" actor = clean_actor(ENVIRON["SIP_ACTOR"]) if (actor == "") actor = "unknown" @@ -57,6 +63,10 @@ $3 ~ "^refs/(heads|tags)/" { print "sip: could not write event " path > "/dev/stderr" exit 1 } + if (system("mv " shell_quote(path) " " shell_quote(final)) != 0) { + print "sip: could not publish event " final > "/dev/stderr" + exit 1 + } } ') { exit 1 diff --git a/tests/integration.rc b/tests/integration.rc index e2cfb14..61ba7a0 100755 --- a/tests/integration.rc +++ b/tests/integration.rc @@ -202,6 +202,32 @@ fn test_branch_tag_and_protected_refs { fail push to unknown refs/sip/* should be rejected #*/ } +fn test_post_receive_event_creation_is_atomic { + tmp=`{setup_pair} + src=$tmp^/src + remote=$tmp^/remote.git + + printf 'hello\n' >$src^/README.md + git -C $src add README.md + git -C $src commit -m atomic-event >/dev/null || fail could not commit atomic event fixture + commit=`{git -C $src rev-parse HEAD} + zero=0000000000000000000000000000000000000000 + attempts=50 + + for(i in `{awk -v attempts=$attempts 'BEGIN { for (i = 1; i <= attempts; i++) print i }'}) { + { + cd $remote + printf '%.40s\t%s\trefs/heads/main\tactor\t1\tcreate\n' $zero $commit | $remote^/hooks/post-receive + } & + } + wait + + queue_count=`{find $remote^/sip/queue -type f -name '*.event' | awk 'END { print NR }'} + assert_eq $queue_count $attempts 'concurrent post-receive hooks should publish every event' + for(event in $remote^/sip/queue/*.event) #*/ + assert_event_schema $event +} + fn test_no_workflow_repository { tmp=`{setup_pair} src=$tmp^/src @@ -470,6 +496,7 @@ fn test_non_delete_events_require_commit_checkout { } test_branch_tag_and_protected_refs +test_post_receive_event_creation_is_atomic test_no_workflow_repository test_protected_workflow_policy test_unsafe_workflow_ref_overrides_are_ignored -- 2.51.2