fix: enforce agent ownership on GET /api/v1/external/invocations/{id}

internal/api/handlers.go

@@ -4394,11 +4394,15 @@ func (h *Handler) externalInvocationDetail(w http.ResponseWriter, r *http.Reques
 	case http.MethodGet:
 		resp, err := h.svc.Get(r.Context(), id)
 		if err != nil {
-			status := http.StatusInternalServerError
-			if err == sql.ErrNoRows {
-				status = http.StatusNotFound
+			// Not-found and not-owner map to the same generic 404 so a
+			// non-owning caller can't distinguish "doesn't exist" from
+			// "exists but belongs to another agent" (existence would
+			// otherwise leak through the error path).
+			if err == sql.ErrNoRows || errors.Is(err, invocation.ErrNotOwner) {
+				writeError(w, http.StatusNotFound, "not found")
+				return
 			}
-			writeError(w, status, err.Error())
+			writeError(w, http.StatusInternalServerError, err.Error())
 			return
 		}
 		writeJSON(w, http.StatusOK, resp)

internal/api/handlers_test.go

@@ -2218,6 +2218,39 @@ func TestExternalInvocationPatchErrorStatusMapping(t *testing.T) {
 	}
 }
 
+func TestExternalInvocationGetErrorStatusMapping(t *testing.T) {
+	cases := []struct {
+		name       string
+		err        error
+		wantStatus int
+	}{
+		{"not owner maps to 404", fmt.Errorf("invocation inv_1: %w", invocation.ErrNotOwner), http.StatusNotFound},
+		{"missing invocation maps to 404", sql.ErrNoRows, http.StatusNotFound},
+		{"generic error maps to 500", fmt.Errorf("boom"), http.StatusInternalServerError},
+		{"success maps to 200", nil, http.StatusOK},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			svc := &stubService{getErr: tc.err}
+			h := NewHandler(svc, stubServerService{}, nil, nil, nil, nil, nil, nil, nil, nil)
+			req := httptest.NewRequest(http.MethodGet, "/api/v1/external/invocations/inv_1", nil)
+			w := httptest.NewRecorder()
+			h.Routes().ServeHTTP(w, req)
+			if w.Code != tc.wantStatus {
+				t.Fatalf("status = %d, want %d (body=%s)", w.Code, tc.wantStatus, w.Body.String())
+			}
+			// The not-owner 404 must read identically to a real "missing
+			// invocation" 404 — a distinguishable body would leak that the
+			// invocation exists to a non-owning caller.
+			if tc.wantStatus == http.StatusNotFound {
+				if got, want := w.Body.String(), `{"error":{"message":"not found"}}`+"\n"; got != want {
+					t.Fatalf("body = %q, want %q (must not leak ownership details)", got, want)
+				}
+			}
+		})
+	}
+}
+
 func TestAddExtraRoutesMountsRoutesOutsideAuthChains(t *testing.T) {
 	h := NewHandler(&stubService{}, stubServerService{}, nil, nil, nil, nil, nil, nil, nil, nil)
 	// A configured validator protects the normal runtime and privileged routes;

internal/invocation/record_execution_test.go

@@ -324,3 +324,57 @@ func TestRecordExecutionEnforcesAgentOwnership(t *testing.T) {
 		}
 	})
 }
+
+// TestGetEnforcesAgentOwnership guards against a caller who knows another
+// agent's invocation_id reading that agent's tool input/output/error via
+// GET /api/v1/external/invocations/{id} — the same ownership boundary
+// RecordExecution enforces on the write path (TestRecordExecutionEnforcesAgentOwnership).
+func TestGetEnforcesAgentOwnership(t *testing.T) {
+	t.Run("different agent is rejected", func(t *testing.T) {
+		svc := newRecordExecutionService(t)
+		id := submitExternal(t, svc, "agent-a")
+		ctx := auth.WithIdentity(context.Background(), auth.Identity{AgentID: "agent-b"})
+		_, err := svc.Get(ctx, id)
+		if !errors.Is(err, invocation.ErrNotOwner) {
+			t.Fatalf("err = %v, want ErrNotOwner", err)
+		}
+	})
+
+	t.Run("owning agent is allowed", func(t *testing.T) {
+		svc := newRecordExecutionService(t)
+		id := submitExternal(t, svc, "agent-a")
+		ctx := auth.WithIdentity(context.Background(), auth.Identity{AgentID: "agent-a"})
+		resp, err := svc.Get(ctx, id)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.InvocationID != id {
+			t.Fatalf("invocation_id = %q, want %q", resp.InvocationID, id)
+		}
+	})
+
+	t.Run("caller without identity is exempt", func(t *testing.T) {
+		svc := newRecordExecutionService(t)
+		id := submitExternal(t, svc, "agent-a")
+		resp, err := svc.Get(context.Background(), id)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.InvocationID != id {
+			t.Fatalf("invocation_id = %q, want %q", resp.InvocationID, id)
+		}
+	})
+
+	t.Run("anonymous invocation accepts any identity", func(t *testing.T) {
+		svc := newRecordExecutionService(t)
+		id := submitExternal(t, svc, "")
+		ctx := auth.WithIdentity(context.Background(), auth.Identity{AgentID: "agent-b"})
+		resp, err := svc.Get(ctx, id)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp.InvocationID != id {
+			t.Fatalf("invocation_id = %q, want %q", resp.InvocationID, id)
+		}
+	})
+}

internal/invocation/service.go

@@ -1728,8 +1728,8 @@ func (s *Service) summarizePendingApproval(invocationID string) {
 // pending_approval, or after a human denied it.
 var ErrInvalidTransition = errors.New("invalid execution status transition")
 
-// ErrNotOwner is returned by RecordExecution when the verified agent identity
-// on the request does not match the agent the invocation belongs to.
+// ErrNotOwner is returned by Get and RecordExecution when the verified agent
+// identity on the request does not match the agent the invocation belongs to.
 var ErrNotOwner = errors.New("invocation belongs to a different agent")
 
 // requireStatus rejects an execution-status transition unless the invocation
@@ -1743,6 +1743,31 @@ func requireStatus(inv Invocation, target string, allowed ...Status) error {
 	return fmt.Errorf("invocation %s cannot move to %s from %s: %w", inv.InvocationID, target, inv.Status, ErrInvalidTransition)
 }
 
+// checkOwnership rejects access to inv when the request carries an
+// authenticated agent identity that does not match the invocation's owner.
+// Both the read (Get) and write (RecordExecution) paths on
+// /api/v1/external/invocations/{id} run under the agent-runtime OAuth
+// middleware, so in auth mode ctx carries the authenticated caller
+// (inv.AgentID is set from that same identity at Submit time, so a match
+// proves ownership). Invocations submitted anonymously (no agent_id) skip
+// the check, as does any caller with no identity in context — no-auth mode,
+// or the in-process managed-agents watcher and operator/review routes, which
+// authenticate by a different path and never carry an agent identity.
+func checkOwnership(ctx context.Context, inv Invocation) error {
+	callerID := strings.TrimSpace(auth.AgentIDFromContext(ctx))
+	if callerID == "" {
+		return nil
+	}
+	owner := ""
+	if inv.AgentID != nil {
+		owner = strings.TrimSpace(*inv.AgentID)
+	}
+	if owner != "" && owner != callerID {
+		return fmt.Errorf("invocation %s: %w", inv.InvocationID, ErrNotOwner)
+	}
+	return nil
+}
+
 // RecordExecution updates an externally-executed invocation with the outcome
 // reported by the executor. Valid execStatus values:
 //
@@ -1758,26 +1783,12 @@ func (s *Service) RecordExecution(ctx context.Context, invocationID string, upda
 	if err != nil {
 		return InvocationResponse{}, err
 	}
-	// Ownership: update.Result/Error are surfaced to the judge as trusted
-	// evidence, so a caller who knows another agent's invocation_id could
-	// poison that agent's session context. The PATCH
-	// /api/v1/external/invocations/{id} route runs under the agent-runtime
-	// OAuth middleware, so in auth mode ctx carries the authenticated caller.
-	// When an identity is present, reject any attempt to write an invocation
-	// this agent does not own (inv.AgentID is set from the authenticated
-	// identity at Submit time, so a match proves ownership). Invocations
-	// submitted anonymously (no agent_id) skip the ownership check. In no-auth
-	// mode there is no identity to check against — behavior is unchanged, and
-	// the in-process managed-agents watcher (which calls RecordExecution
-	// directly with no auth identity) is likewise unaffected.
-	if callerID := strings.TrimSpace(auth.AgentIDFromContext(ctx)); callerID != "" {
-		owner := ""
-		if inv.AgentID != nil {
-			owner = strings.TrimSpace(*inv.AgentID)
-		}
-		if owner != "" && owner != callerID {
-			return InvocationResponse{}, fmt.Errorf("invocation %s: %w", invocationID, ErrNotOwner)
-		}
+	// update.Result/Error are surfaced to the judge as trusted evidence, so a
+	// caller who knows another agent's invocation_id could poison that
+	// agent's session context — reject writes to invocations this caller
+	// doesn't own.
+	if err := checkOwnership(ctx, inv); err != nil {
+		return InvocationResponse{}, err
 	}
 	now := time.Now().UTC()
 	switch update.ExecutionStatus {
@@ -1886,6 +1897,12 @@ func (s *Service) Get(ctx context.Context, id string) (InvocationResponse, error
 	if err != nil {
 		return InvocationResponse{}, err
 	}
+	// A caller who knows another agent's invocation_id could otherwise read
+	// that agent's tool input/output/error — reject reads of invocations
+	// this caller doesn't own.
+	if err := checkOwnership(ctx, inv); err != nil {
+		return InvocationResponse{}, err
+	}
 	resp := s.toResponse(inv)
 	return resp, nil
 }