fix: enforce agent boundaries and clean up attempt lifecycles

README.md

@@ -494,6 +494,11 @@ modifications, renames are decomposed into literal paths, and gitignored
 files, the `.git` directory, and hook paths are all watched, because a
 planted hook would execute during jig's own later git commands.
 
+Comparison and file rollback use the pre-phase commit, so committing an edit
+does not hide it. A read-only role that moves `HEAD` aborts even if it later
+reverts the edit. Jig retains that history for inspection rather than
+rewriting it; a crashed phase that moved `HEAD` is not retried.
+
 What follows from that: **the `writes` allowlist is the security control you
 actually configure.** `writes: ["**"]` (the stock builders' default, so the
 examples work anywhere) is the weakest boundary jig can enforce. Narrow it.
@@ -546,7 +551,11 @@ wrapper script wrong.
 
 `jig serve` binds `127.0.0.1` and refuses a non-loopback address without
 `--allow-non-loopback`. There is no authentication and no TLS: the control
-plane assumes one trusted operator on one machine. State-changing routes also
+plane assumes one trusted operator on one machine. Read routes reject untrusted
+Host headers even without an Origin header, preventing DNS-rebound names from
+reading local data. An explicitly enabled non-loopback listener accepts its own
+IP address; reverse proxies must rewrite Host to the listener's authority or
+present the UI token. State-changing routes also
 reject requests whose `Origin` is not the server's own, which is what stops a
 web page you happen to be visiting from driving your control plane through
 your browser.

cmd/jig/cancellation_test.go

@@ -0,0 +1,31 @@
+package main
+
+import (
+	"testing"
+	"time"
+)
+
+func TestCancellationMergeStopsOnCompletionOrEitherInput(t *testing.T) {
+	for _, cause := range []string{"completion", "signal", "job"} {
+		t.Run(cause, func(t *testing.T) {
+			signal, job := make(chan struct{}), make(chan struct{})
+			cancelled, stop := anyClosed(signal, job)
+			defer stop()
+			switch cause {
+			case "completion":
+				stop()
+			case "signal":
+				close(signal)
+			case "job":
+				close(job)
+			}
+			select {
+			case <-cancelled:
+			case <-time.After(time.Second):
+				t.Fatal("cancellation waiters did not stop")
+			}
+			// stop waits for all helpers, including the input that never closed.
+			stop()
+		})
+	}
+}

cmd/jig/ci_repair_test.go

@@ -287,6 +287,11 @@ func TestReleasingTheRepairContinuationDrainsTheTraceBeforeTheAttemptCompletes(t
 		t.Fatal(err)
 	}
 	gate := &ingestGate{proxy: httputil.NewSingleHostReverseProxy(target)}
+	director := gate.proxy.Director
+	gate.proxy.Director = func(request *http.Request) {
+		director(request)
+		request.Host = target.Host
+	}
 	proxy := httptest.NewServer(gate)
 	defer proxy.Close()
 

cmd/jig/worker.go

@@ -322,15 +322,22 @@ func workerAttemptRunner(
 		// (which rides the heartbeat response) and the operator's Ctrl-C. The
 		// engine watches one channel, so they are merged rather than the
 		// second one being dropped.
+		cancelled, stopCancellation := anyClosed(signalCtx.Done(), prepared.Cancelled())
+		defer func() {
+			if !kept {
+				stopCancellation()
+			}
+		}()
 		outcome := runner.Execute(context.WithoutCancel(ctx), engine.Attempt{
 			Claim:        prepared.Claim,
 			WorktreePath: prepared.WorktreePath,
 			Branch:       prepared.Branch,
 			BaseSHA:      prepared.BaseSHA,
-			Cancelled:    anyClosed(signalCtx.Done(), prepared.Cancelled()),
+			Cancelled:    cancelled,
 			FreshenLease: prepared.FreshenLease,
 		})
 		kept = worker.DeferToContinuation(&outcome, func() {
+			stopCancellation()
 			closeTrace()
 			removeScratch()
 		})
@@ -339,24 +346,28 @@ func workerAttemptRunner(
 }
 
 // anyClosed merges cancellation channels: the result closes as soon as any
-// input does. It never blocks the caller and never leaks past the first
-// close, which is all the engine's one-shot cancellation signal needs.
-func anyClosed(channels ...<-chan struct{}) <-chan struct{} {
-	merged := make(chan struct{})
-	var once sync.Once
+// input does. The owner stops its waiters after normal completion too (R5).
+func anyClosed(channels ...<-chan struct{}) (<-chan struct{}, func()) {
+	ctx, cancel := context.WithCancel(context.Background())
+	var waiters sync.WaitGroup
 	for _, channel := range channels {
 		if channel == nil {
 			continue
 		}
+		waiters.Add(1)
 		go func(c <-chan struct{}) {
+			defer waiters.Done()
 			select {
 			case <-c:
-				once.Do(func() { close(merged) })
-			case <-merged:
+				cancel()
+			case <-ctx.Done():
 			}
 		}(channel)
 	}
-	return merged
+	return ctx.Done(), func() {
+		cancel()
+		waiters.Wait()
+	}
 }
 
 // printReconcileReport is the operator's startup picture: identity, probed

internal/controlplane/http.go

@@ -1,14 +1,13 @@
 // http.go — the control plane's HTTP surface (U2). JSON in, JSON out,
 // bounded request bodies, and browser-origin fencing on every state-changing
 // route (R20): a request carrying an Origin header must first present a
-// trustworthy Host authority (loopback, never a resolved name — see
-// hostAuthorityIsTrusted, which closes the DNS-rebinding hole where an
+// trustworthy Host authority (loopback or the listener's IP, never a resolved
+// name — see hostAuthorityIsTrusted, which closes the DNS-rebinding hole where an
 // attacker's domain resolves to 127.0.0.1 and Origin/Host agree with each
 // other but not with reality), then either the server's own origin or the
-// per-process UI token. Requests without an Origin header (the worker,
-// curl) pass — the check exists to stop foreign web pages from driving a
-// loopback control plane through the operator's browser, not to
-// authenticate local clients.
+// per-process UI token. Reads require a trusted Host even without Origin.
+// Originless mutations (the worker, curl) pass — the check stops foreign web
+// pages from driving the control plane, not to authenticate local clients.
 package controlplane
 
 import (
@@ -70,7 +69,17 @@ func NewHandler(store *Store, uiToken string, logger *slog.Logger) http.Handler
 	api.registerTriggerRoutes(mux)
 	api.registerIngestRoutes(mux)
 	api.registerReportRoutes(mux)
-	return mux
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Read routes also need Host fencing: DNS rebinding can read same-origin
+		// data without sending an Origin header (R20).
+		if (r.Method == http.MethodGet || r.Method == http.MethodHead) &&
+			!api.uiTokenPresented(r) && !hostAuthorityIsTrusted(r) {
+			writeError(w, &ServiceError{Code: "untrusted_host_authority",
+				Message: "requests must target a trusted listener host", Status: 403})
+			return
+		}
+		mux.ServeHTTP(w, r)
+	})
 }
 
 func (a *API) health(w http.ResponseWriter, r *http.Request) {
@@ -354,7 +363,7 @@ func (a *API) prepareMutation(w http.ResponseWriter, r *http.Request) bool {
 		// trusting any comparison against it.
 		if !hostAuthorityIsTrusted(r) {
 			writeError(w, &ServiceError{Code: "untrusted_host_authority",
-				Message: "state-changing requests must target a loopback host", Status: 403})
+				Message: "state-changing requests must target a trusted listener host", Status: 403})
 			return false
 		}
 		parsed, err := url.Parse(origin)
@@ -378,10 +387,10 @@ func sameAuthority(left, right string) bool {
 	return strings.EqualFold(strings.TrimSuffix(left, "."), strings.TrimSuffix(right, "."))
 }
 
-// hostAuthorityIsTrusted reports whether r.Host names loopback rather than
-// an attacker's DNS-rebound domain (R20). The hostname must be a loopback
-// IP literal (127.0.0.0/8, ::1) or "localhost" — never resolved, so a
-// rebound name can never satisfy it by any DNS trick. When Host carries a
+// hostAuthorityIsTrusted reports whether r.Host names loopback or the
+// listener's own IP rather than an attacker's DNS-rebound domain (R20).
+// Hostnames are never resolved, so a rebound name cannot pass by a DNS trick.
+// When Host carries a
 // port and the server's own listener port is known from the connection
 // (net/http sets LocalAddrContextKey per accepted connection; direct
 // ServeHTTP calls in tests do not), that port must match too, closing the
@@ -390,7 +399,17 @@ func sameAuthority(left, right string) bool {
 func hostAuthorityIsTrusted(r *http.Request) bool {
 	host, port, hasPort := splitHostAuthority(r.Host)
 	if !isLoopbackHostname(host) {
-		return false
+		// An explicitly enabled non-loopback listener accepts its own IP,
+		// never an arbitrary DNS name that could rebind to it (R20).
+		addr, ok := r.Context().Value(http.LocalAddrContextKey).(net.Addr)
+		if !ok {
+			return false
+		}
+		localHost, _, err := net.SplitHostPort(addr.String())
+		ip := net.ParseIP(host)
+		if err != nil || ip == nil || !ip.Equal(net.ParseIP(localHost)) {
+			return false
+		}
 	}
 	if !hasPort {
 		return true

internal/controlplane/http_host_test.go

@@ -0,0 +1,67 @@
+package controlplane
+
+import (
+	"context"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+)
+
+func TestReadRoutesFenceReboundHostsWithoutRequiringOrigin(t *testing.T) {
+	store, _ := newTestStore(t)
+	handler := NewHandler(store, "test-ui-token", nil)
+	for _, method := range []string{http.MethodGet, http.MethodHead} {
+		for _, origin := range []string{"", "http://evil.test:8383"} {
+			for _, path := range []string{"/api/definitions", "/api/runs", "/api/workers", "/api/health"} {
+				request := httptest.NewRequest(method, "http://evil.test:8383"+path, nil)
+				request.Header.Set("Origin", origin)
+				response := httptest.NewRecorder()
+				handler.ServeHTTP(response, request)
+				if response.Code != http.StatusForbidden || !strings.Contains(response.Body.String(), "untrusted_host_authority") {
+					t.Fatalf("%s %s origin=%q: %d %s", method, path, origin, response.Code, response.Body.String())
+				}
+			}
+		}
+	}
+	for _, host := range []string{"127.0.0.1:8383", "localhost:8383", "[::1]:8383"} {
+		request := httptest.NewRequest(http.MethodGet, "http://"+host+"/api/health", nil)
+		response := httptest.NewRecorder()
+		handler.ServeHTTP(response, request)
+		if response.Code != http.StatusOK {
+			t.Fatalf("trusted Host %s: %d", host, response.Code)
+		}
+	}
+	request := httptest.NewRequest(http.MethodGet, "http://evil.test/api/health", nil)
+	request.Header.Set("X-Jig-UI-Token", "test-ui-token")
+	response := httptest.NewRecorder()
+	handler.ServeHTTP(response, request)
+	if response.Code != http.StatusOK {
+		t.Fatalf("explicit UI token: %d", response.Code)
+	}
+}
+
+func TestReadHostMustMatchListenerPortAndExplicitNonLoopbackIP(t *testing.T) {
+	store, _ := newTestStore(t)
+	handler := NewHandler(store, "", nil)
+	for _, test := range []struct {
+		host string
+		want int
+	}{
+		{"127.0.0.1:8383", http.StatusOK},
+		{"127.0.0.1:9000", http.StatusForbidden},
+		{"192.0.2.10:8383", http.StatusOK},
+		{"192.0.2.11:8383", http.StatusForbidden},
+		{"evil.test:8383", http.StatusForbidden},
+	} {
+		request := httptest.NewRequest(http.MethodGet, "http://"+test.host+"/api/health", nil)
+		request = request.WithContext(context.WithValue(request.Context(), http.LocalAddrContextKey,
+			&net.TCPAddr{IP: net.ParseIP("192.0.2.10"), Port: 8383}))
+		response := httptest.NewRecorder()
+		handler.ServeHTTP(response, request)
+		if response.Code != test.want {
+			t.Fatalf("host %s: %d, want %d", test.host, response.Code, test.want)
+		}
+	}
+}

internal/controlplane/store_test.go

@@ -477,6 +477,9 @@ func TestNMissedHeartbeatsSweepTheAttemptToLostAndAStaleCompletionIsRejected(t *
 	if job.State != protocol.JobFailed {
 		t.Fatalf("job state = %q, want failed after sweep", job.State)
 	}
+	if got := runState(t, store, claim.Job.RunID); got != protocol.RunFailed {
+		t.Fatalf("run state = %q, want failed after sweep", got)
+	}
 	// The zombie wakes and tries to complete with its stale token.
 	_, err = store.CompleteAttempt(ctx, claim.Attempt.ID, protocol.CompleteAttemptRequest{
 		LeaseToken: tokenA, State: protocol.AttemptAccepted, Result: "zombie work",

internal/controlplane/sweep.go

@@ -179,6 +179,9 @@ func (s *Store) sweepAttempt(ctx context.Context, attemptID string, nowMillis in
 	`, nowMillis, value.JobID); err != nil {
 		return value, false, unavailable(err)
 	}
+	if err := applyRunAggregationForJob(ctx, tx, value.JobID, nowMillis); err != nil {
+		return value, false, err
+	}
 	if err := tx.Commit(); err != nil {
 		return value, false, unavailable(err)
 	}