diff --git a/action_hook.go b/action_hook.go deleted file mode 100644 index ac319a6..0000000 --- a/action_hook.go +++ /dev/null @@ -1,104 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0-or-later - -package handshake - -import ( - "context" - "errors" - "fmt" - "log/slog" - "strconv" - "sync" - "time" - - "github.com/pilot-protocol/common/actionhook" - "github.com/pilot-protocol/common/decision" -) - -const handshakeActionAdapterID = "pilot.handshake" - -// ActionHook is the common optional before/after action boundary. Keeping the -// alias here gives embedders one stable handshake-facing type without a -// handshake-specific policy language. -type ActionHook = actionhook.Hook - -type trustActionAttempt struct { - hook actionhook.Hook - envelope actionhook.Envelope - preflight actionhook.Preflight - once sync.Once -} - -// SetActionHook attaches an explicitly configured hook. Passing nil restores -// unmanaged behavior. Composition roots should call this before Start. -func (hm *Manager) SetActionHook(hook ActionHook) { - hm.mu.Lock() - hm.actionHook = hook - hm.mu.Unlock() -} - -// prepareTrustAction must be called without hm.mu held because a managed hook -// may perform bounded network I/O. It returns (nil, nil) when no hook is -// attached, which is the exact legacy path. -func (hm *Manager) prepareTrustAction(action string, peerNodeID uint32, direction, reason string, automatic bool, hasJustification bool) (*trustActionAttempt, error) { - hm.mu.RLock() - hook := hm.actionHook - hm.mu.RUnlock() - if hook == nil { - return nil, nil - } - attributes := map[string]string{ - "peer_node_id": strconv.FormatUint(uint64(peerNodeID), 10), - "direction": direction, - "reason": reason, - "automatic": strconv.FormatBool(automatic), - "has_justification": strconv.FormatBool(hasJustification), - } - envelope, err := actionhook.NewEnvelope( - action, fmt.Sprintf("agent:%d", peerNodeID), actionhook.HashMetadata(attributes), - handshakeActionAdapterID, attributes, time.Now(), - ) - if err != nil { - return nil, err - } - envelope.ResumeToken = fmt.Sprintf("%s:%s:%d", action, direction, peerNodeID) - if err := envelope.Validate(); err != nil { - return nil, err - } - preflight, err := hook.BeforeAction(context.Background(), envelope) - if err != nil { - return nil, fmt.Errorf("handshake: %s preflight for node %d: %w", action, peerNodeID, err) - } - attempt := &trustActionAttempt{hook: hook, envelope: envelope, preflight: preflight} - if err := preflight.RequireUnconstrained(); err != nil { - status := actionhook.StatusFailed - var blocked *actionhook.BlockedError - if errors.As(err, &blocked) { - switch blocked.Outcome { - case decision.Deny: - status = actionhook.StatusDenied - case decision.ApprovalRequired: - status = actionhook.StatusApprovalPending - } - } - attempt.complete(status, "preflight_blocked", nil) - return nil, fmt.Errorf("handshake: %s for node %d: %w", action, peerNodeID, err) - } - return attempt, nil -} - -func (attempt *trustActionAttempt) complete(status actionhook.ObservedStatus, errorCode string, attributes map[string]string) { - if attempt == nil { - return - } - attempt.once.Do(func() { - result := actionhook.ObservedResult{ - Status: status, ObservedAt: time.Now().Unix(), ErrorCode: errorCode, Attributes: attributes, - } - if err := attempt.hook.AfterAction(context.Background(), attempt.envelope, attempt.preflight, result); err != nil { - // Post-hooks are evidence-only. Never repeat or roll back a trust - // transition because evidence export failed. - slog.Error("handshake action post-hook failed", "action", attempt.envelope.Action, "action_id", attempt.envelope.ID, "error", err) - } - }) -} diff --git a/enterprise_action_hook_test.go b/enterprise_action_hook_test.go deleted file mode 100644 index d8fd78f..0000000 --- a/enterprise_action_hook_test.go +++ /dev/null @@ -1,165 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0-or-later - -package handshake - -import ( - "context" - "errors" - "sync" - "testing" - "time" - - "github.com/pilot-protocol/common/actionhook" - "github.com/pilot-protocol/common/decision" -) - -type trustHookCall struct { - envelope actionhook.Envelope - result actionhook.ObservedResult -} - -type trustHookStub struct { - mu sync.Mutex - outcomes map[string]decision.Outcome - before []actionhook.Envelope - after []trustHookCall - afterErr error -} - -func (hook *trustHookStub) BeforeAction(_ context.Context, envelope actionhook.Envelope) (actionhook.Preflight, error) { - hook.mu.Lock() - defer hook.mu.Unlock() - hook.before = append(hook.before, envelope) - outcome := decision.Allow - if configured, exists := hook.outcomes[envelope.Action]; exists { - outcome = configured - } - return actionhook.Preflight{Outcome: outcome, Reference: actionhook.DecisionReference{DecisionID: "test-decision"}}, nil -} - -func (hook *trustHookStub) AfterAction(_ context.Context, envelope actionhook.Envelope, _ actionhook.Preflight, result actionhook.ObservedResult) error { - hook.mu.Lock() - defer hook.mu.Unlock() - hook.after = append(hook.after, trustHookCall{envelope: envelope, result: result}) - return hook.afterErr -} - -func (hook *trustHookStub) observed(action string, status actionhook.ObservedStatus) bool { - hook.mu.Lock() - defer hook.mu.Unlock() - for _, call := range hook.after { - if call.envelope.Action == action && call.result.Status == status { - return true - } - } - return false -} - -func TestActionHookDenyTurnsEveryAutomaticTrustPathIntoPending(t *testing.T) { - for _, relayed := range []bool{false, true} { - t.Run(map[bool]string{false: "direct", true: "relayed"}[relayed], func(t *testing.T) { - runtime := newTestRuntime() - runtime.trustAutoApprove = true - manager := NewManager(runtime) - t.Cleanup(manager.Stop) - hook := &trustHookStub{outcomes: map[string]decision.Outcome{"trust.auto_accept": decision.Deny}} - manager.SetActionHook(hook) - - if relayed { - manager.processRelayedRequest(71, "join") - } else { - manager.handleRequest(nil, &HandshakeMsg{NodeID: 71, PublicKey: "peer-key", Justification: "join"}, false) - } - manager.mu.RLock() - _, trusted := manager.trusted[71] - _, pending := manager.pending[71] - manager.mu.RUnlock() - if trusted || !pending { - t.Fatalf("denied auto-accept trusted=%v pending=%v", trusted, pending) - } - if !hook.observed("trust.auto_accept", actionhook.StatusDenied) { - t.Fatal("denied automatic action was not sent to the post-hook") - } - }) - } -} - -func TestActionHookDenyCoversManualAndOutgoingTrustGrantPaths(t *testing.T) { - hook := &trustHookStub{outcomes: map[string]decision.Outcome{"trust.accept": decision.Deny}} - manager := newTestHM(t, "") - t.Cleanup(manager.Stop) - manager.SetActionHook(hook) - - manager.pending[80] = &PendingHandshake{NodeID: 80, Justification: "manual"} - if err := manager.ApproveHandshake(80); err == nil { - t.Fatal("manual approval bypassed trust.accept deny") - } - manager.outgoing[81] = time.Now() - manager.handleAccept(&HandshakeMsg{NodeID: 81, PublicKey: "peer-key"}) - manager.outgoing[82] = time.Now() - manager.processRelayedApproval(82) - - manager.mu.RLock() - defer manager.mu.RUnlock() - for _, peer := range []uint32{80, 81, 82} { - if _, trusted := manager.trusted[peer]; trusted { - t.Fatalf("peer %d became trusted despite trust.accept deny", peer) - } - } - if _, pending := manager.pending[80]; !pending { - t.Fatal("manual request must remain pending after denial") - } - if _, outgoing := manager.outgoing[81]; !outgoing { - t.Fatal("direct outgoing request must remain available after denial") - } - if _, outgoing := manager.outgoing[82]; !outgoing { - t.Fatal("relayed outgoing request must remain available after denial") - } -} - -func TestActionHookApprovalRequiredSuspendsBeforeMutation(t *testing.T) { - hook := &trustHookStub{outcomes: map[string]decision.Outcome{"trust.accept": decision.ApprovalRequired}} - manager := newTestHM(t, "") - t.Cleanup(manager.Stop) - manager.SetActionHook(hook) - manager.pending[90] = &PendingHandshake{NodeID: 90} - - err := manager.ApproveHandshake(90) - var blocked *actionhook.BlockedError - if !errors.As(err, &blocked) || blocked.Outcome != decision.ApprovalRequired { - t.Fatalf("expected approval-required block, got %v", err) - } - if _, trusted := manager.trusted[90]; trusted { - t.Fatal("approval-required action mutated trust") - } - if !hook.observed("trust.accept", actionhook.StatusApprovalPending) { - t.Fatal("approval suspension was not evidenced") - } -} - -func TestActionHookDenyBlocksTrustRequestBeforeOutgoingState(t *testing.T) { - hook := &trustHookStub{outcomes: map[string]decision.Outcome{"trust.request": decision.Deny}} - manager := newTestHM(t, "") - t.Cleanup(manager.Stop) - manager.SetActionHook(hook) - if err := manager.SendRequest(101, "connect"); err == nil { - t.Fatal("denied trust request returned success") - } - if _, exists := manager.outgoing[101]; exists { - t.Fatal("denied trust request created outgoing state") - } -} - -func TestPostHookFailureCannotUndoGrantedTrust(t *testing.T) { - hook := &trustHookStub{outcomes: map[string]decision.Outcome{"trust.accept": decision.Allow}, afterErr: errors.New("journal unavailable")} - manager := newTestHM(t, "") - t.Cleanup(manager.Stop) - manager.SetActionHook(hook) - manager.pending[111] = &PendingHandshake{NodeID: 111} - if err := manager.ApproveHandshake(111); err != nil { - t.Fatalf("post-hook failure changed action result: %v", err) - } - if _, trusted := manager.trusted[111]; !trusted { - t.Fatal("post-hook failure rolled back trust") - } -} diff --git a/handshake.go b/handshake.go index 7956210..5be47a7 100644 --- a/handshake.go +++ b/handshake.go @@ -16,7 +16,6 @@ import ( "time" "unicode" - "github.com/pilot-protocol/common/actionhook" "github.com/pilot-protocol/common/coreapi" "github.com/pilot-protocol/common/crypto" "github.com/pilot-protocol/common/fsutil" @@ -130,10 +129,6 @@ type Manager struct { // so that markTrustedLocked can fire notifications while holding hm.mu. trustWaitersMu sync.Mutex trustWaiters map[uint32][]chan struct{} - - // actionHook is nil unless the composition root explicitly attaches an - // enterprise action profile. Nil is the compatibility default. - actionHook ActionHook } // NewManager constructs a Manager bound to the given Runtime. Loads @@ -896,14 +891,10 @@ func (hm *Manager) handleRequest(stream coreapi.Stream, msg *HandshakeMsg, regis "peer_node_id": peerNodeID, "justification": justification, }) - // Fast pre-checks BEFORE the action hook, which in managed-enforce mode - // makes a synchronous authority round-trip. Without this, an attacker who - // can reach port 444 (or relay a request) forces one authority call per - // handshake and ties up a handler goroutine — and the anti-flood caps, - // evaluated only after the hook, don't protect the authority. Here an - // already-trusted peer (no new trust needed) and over-cap spam are handled - // without invoking the hook. The authoritative trust/cap decisions are - // still made under the lock below, so this is a guard, not the enforcement. + // Fast pre-checks: an already-trusted peer (no new trust needed) and + // over-cap spam are handled here. The authoritative trust/cap decisions + // are still made under the lock below, so this is a guard, not the + // enforcement. hm.mu.Lock() if _, ok := hm.trusted[peerNodeID]; ok && hm.reconcileTrustBindingLocked(peerNodeID, msg.PublicKey) { hm.sendAcceptLocked(peerNodeID) @@ -921,27 +912,6 @@ func (hm *Manager) handleRequest(stream coreapi.Stream, msg *HandshakeMsg, regis } hm.mu.Unlock() - autoAttempt, autoErr := hm.prepareTrustAction("trust.auto_accept", peerNodeID, "inbound", "incoming_request", true, justification != "") - autoAllowed := autoErr == nil - if autoErr != nil { - slog.Info("automatic trust acceptance blocked", "peer_node_id", peerNodeID, "error", autoErr) - hm.rt.PublishEvent("handshake.control_blocked", map[string]interface{}{ - "peer_node_id": peerNodeID, "action": "trust.auto_accept", - }) - } - autoExecuted := false - autoReason := "candidate_not_used" - defer func() { - if autoAttempt == nil { - return - } - if autoExecuted { - autoAttempt.complete(actionhook.StatusSucceeded, "", map[string]string{"grant_reason": autoReason}) - return - } - autoAttempt.complete(actionhook.StatusSkipped, "", map[string]string{"skip_reason": autoReason}) - }() - hm.mu.Lock() defer hm.mu.Unlock() @@ -959,7 +929,7 @@ func (hm *Manager) handleRequest(stream coreapi.Stream, msg *HandshakeMsg, regis // Check if we have an outgoing request to this peer (mutual handshake) // SEC-038: mutual auto-trust is gated on registryBound so a peer // cannot claim any NodeID with their own keypair and slip into trust. - if _, ok := hm.outgoing[peerNodeID]; ok && registryBound && autoAllowed { + if _, ok := hm.outgoing[peerNodeID]; ok && registryBound { // Mutual! Auto-approve (registry confirmed pubkey binding) delete(hm.outgoing, peerNodeID) hm.markTrustedLocked(peerNodeID, &TrustRecord{ @@ -968,7 +938,6 @@ func (hm *Manager) handleRequest(stream coreapi.Stream, msg *HandshakeMsg, regis ApprovedAt: time.Now(), Mutual: true, }) - autoExecuted, autoReason = true, "mutual" slog.Info("mutual handshake auto-approved", "peer_node_id", peerNodeID) hm.rt.PublishEvent("handshake.auto_approved", map[string]interface{}{ "peer_node_id": peerNodeID, "reason": "mutual", @@ -989,14 +958,13 @@ func (hm *Manager) handleRequest(stream coreapi.Stream, msg *HandshakeMsg, regis // Check if peers are on the same network (network trust) // SEC-038: same-network auto-trust is gated on registryBound so a // peer cannot claim any NodeID with their own keypair and slip into trust. - if hm.sameNetwork(peerNodeID) && registryBound && autoAllowed { + if hm.sameNetwork(peerNodeID) && registryBound { hm.markTrustedLocked(peerNodeID, &TrustRecord{ NodeID: peerNodeID, PublicKey: msg.PublicKey, ApprovedAt: time.Now(), Network: hm.sharedNetwork(peerNodeID), }) - autoExecuted, autoReason = true, "same_network" slog.Info("same network handshake auto-approved", "peer_node_id", peerNodeID) hm.rt.PublishEvent("handshake.auto_approved", map[string]interface{}{ "peer_node_id": peerNodeID, "reason": "same_network", @@ -1019,7 +987,7 @@ func (hm *Manager) handleRequest(stream coreapi.Stream, msg *HandshakeMsg, regis // registryBound a peer could claim any node ID with their own // pubkey, sign with their own key, pass signature verify, and slip // into auto-accept. - if registryBound && autoAllowed { + if registryBound { // Trust gate via L10 TrustChecker. Nil → trust nothing. // msg.PublicKey is the key the peer authenticated with and that // the registry lookup bound to peerNodeID, so it is the key the @@ -1031,7 +999,6 @@ func (hm *Manager) handleRequest(stream coreapi.Stream, msg *HandshakeMsg, regis PublicKey: msg.PublicKey, ApprovedAt: time.Now(), }) - autoExecuted, autoReason = true, "trusted_agent" slog.Info("handshake auto-approved (trusted agent)", "peer_node_id", peerNodeID, "agent", name) hm.rt.PublishEvent("handshake.auto_approved", map[string]interface{}{ @@ -1051,14 +1018,13 @@ func (hm *Manager) handleRequest(stream coreapi.Stream, msg *HandshakeMsg, regis } // Auto-approve all requests when the daemon is configured to do so. - if hm.rt.TrustAutoApprove() && autoAllowed { + if hm.rt.TrustAutoApprove() { hm.markTrustedLocked(peerNodeID, &TrustRecord{ NodeID: peerNodeID, PublicKey: msg.PublicKey, ApprovedAt: time.Now(), Mutual: false, }) - autoExecuted, autoReason = true, "auto_approve" slog.Info("handshake auto-approved (trust-auto-approve enabled)", "peer_node_id", peerNodeID) hm.rt.PublishEvent("handshake.auto_approved", map[string]interface{}{ "peer_node_id": peerNodeID, "reason": "auto_approve", @@ -1109,23 +1075,6 @@ func (hm *Manager) handleRequest(stream coreapi.Stream, msg *HandshakeMsg, regis func (hm *Manager) handleAccept(msg *HandshakeMsg) { peerNodeID := msg.NodeID slog.Info("handshake accepted by peer", "peer_node_id", peerNodeID) - attempt, actionErr := hm.prepareTrustAction("trust.accept", peerNodeID, "outbound_completion", "peer_acceptance", true, false) - if actionErr != nil { - slog.Info("trust establishment from peer acceptance blocked", "peer_node_id", peerNodeID, "error", actionErr) - hm.rt.PublishEvent("handshake.control_blocked", map[string]interface{}{ - "peer_node_id": peerNodeID, "action": "trust.accept", - }) - return - } - granted := false - skipReason := "acceptance_not_applied" - defer func() { - if granted { - attempt.complete(actionhook.StatusSucceeded, "", map[string]string{"grant_reason": "peer_acceptance"}) - return - } - attempt.complete(actionhook.StatusSkipped, "", map[string]string{"skip_reason": skipReason}) - }() hm.mu.Lock() defer hm.mu.Unlock() @@ -1133,7 +1082,6 @@ func (hm *Manager) handleAccept(msg *HandshakeMsg) { // Recently revoked? Drop the stale acceptance. if until, ok := hm.revoked[peerNodeID]; ok { if time.Now().Before(until) { - skipReason = "recently_revoked" slog.Info("ignoring handshake acceptance from recently-revoked peer", "peer_node_id", peerNodeID) delete(hm.outgoing, peerNodeID) return @@ -1144,7 +1092,6 @@ func (hm *Manager) handleAccept(msg *HandshakeMsg) { // Trust follows a request we actually made: an accept with no // matching outgoing entry is unsolicited and is dropped. if _, ok := hm.outgoing[peerNodeID]; !ok { - skipReason = "no_outgoing_request" slog.Warn("dropping handshake acceptance with no matching outgoing request", "peer_node_id", peerNodeID) return } @@ -1156,7 +1103,6 @@ func (hm *Manager) handleAccept(msg *HandshakeMsg) { if existing, ok := hm.trusted[peerNodeID]; ok { bound := existing.PublicKey if !hm.reconcileTrustBindingLocked(peerNodeID, msg.PublicKey) { - skipReason = "key_binding_changed" delete(hm.outgoing, peerNodeID) return } @@ -1173,7 +1119,6 @@ func (hm *Manager) handleAccept(msg *HandshakeMsg) { ApprovedAt: time.Now(), Mutual: true, }) - granted = true hm.markDirty() // Report trust to registry @@ -1202,15 +1147,10 @@ func (hm *Manager) SendRequest(peerNodeID uint32, justification string) error { return nil // already trusted } hm.mu.RUnlock() - attempt, err := hm.prepareTrustAction("trust.request", peerNodeID, "outbound", "operator_request", false, strings.TrimSpace(justification) != "") - if err != nil { - return err - } hm.mu.Lock() if _, ok := hm.trusted[peerNodeID]; ok { hm.mu.Unlock() - attempt.complete(actionhook.StatusSkipped, "", map[string]string{"skip_reason": "already_trusted"}) return nil } // An explicit handshake clears any post-revoke cooldown for this @@ -1238,9 +1178,8 @@ func (hm *Manager) SendRequest(peerNodeID uint32, justification string) error { } // Try direct connection first - err = hm.sendMessage(peerNodeID, &msg) + err := hm.sendMessage(peerNodeID, &msg) if err == nil { - attempt.complete(actionhook.StatusSucceeded, "", map[string]string{"transport": "direct"}) return nil } @@ -1250,14 +1189,11 @@ func (hm *Manager) SendRequest(peerNodeID uint32, justification string) error { sig := hm.signHandshakeChallenge(fmt.Sprintf("handshake:%d:%d", hm.rt.NodeID(), peerNodeID)) _, relayErr := reg.RequestHandshake(hm.rt.NodeID(), peerNodeID, justification, sig) if relayErr != nil { - attempt.complete(actionhook.StatusFailed, "transport_unavailable", nil) return fmt.Errorf("handshake relay: %w", relayErr) } slog.Info("handshake relayed via registry", "peer_node_id", peerNodeID) - attempt.complete(actionhook.StatusSucceeded, "", map[string]string{"transport": "registry_relay"}) return nil } - attempt.complete(actionhook.StatusFailed, "transport_unavailable", nil) return err } @@ -1271,10 +1207,9 @@ func (hm *Manager) ProcessRelayedRequest(fromNodeID uint32, justification string func (hm *Manager) processRelayedRequest(fromNodeID uint32, justification string) { justification = sanitizeJustification(justification) - // Bound authority round-trips from relayed handshake spam: an unknown peer - // whose request can neither be queued (pending cap full) nor short-circuited - // (not already trusted) is rejected BEFORE the managed action hook makes its - // authority call. Already-trusted or already-pending peers are never dropped + // Relayed handshake spam: an unknown peer whose request can neither be + // queued (pending cap full) nor short-circuited (not already trusted) is + // rejected here. Already-trusted or already-pending peers are never dropped // here — they fall through to the existing handling below, where the caps // are authoritatively enforced. hm.mu.Lock() @@ -1287,26 +1222,6 @@ func (hm *Manager) processRelayedRequest(fromNodeID uint32, justification string return } - autoAttempt, autoErr := hm.prepareTrustAction("trust.auto_accept", fromNodeID, "inbound_relay", "incoming_request", true, justification != "") - autoAllowed := autoErr == nil - if autoErr != nil { - slog.Info("automatic relayed trust acceptance blocked", "peer_node_id", fromNodeID, "error", autoErr) - hm.rt.PublishEvent("handshake.control_blocked", map[string]interface{}{ - "peer_node_id": fromNodeID, "action": "trust.auto_accept", - }) - } - autoExecuted := false - autoReason := "candidate_not_used" - defer func() { - if autoAttempt == nil { - return - } - if autoExecuted { - autoAttempt.complete(actionhook.StatusSucceeded, "", map[string]string{"grant_reason": autoReason}) - return - } - autoAttempt.complete(actionhook.StatusSkipped, "", map[string]string{"skip_reason": autoReason}) - }() hm.mu.Lock() defer hm.mu.Unlock() @@ -1323,14 +1238,13 @@ func (hm *Manager) processRelayedRequest(fromNodeID uint32, justification string } // Check if we have an outgoing request to this peer (mutual handshake) - if _, ok := hm.outgoing[fromNodeID]; ok && autoAllowed { + if _, ok := hm.outgoing[fromNodeID]; ok { delete(hm.outgoing, fromNodeID) hm.markTrustedLocked(fromNodeID, &TrustRecord{ NodeID: fromNodeID, ApprovedAt: time.Now(), Mutual: true, }) - autoExecuted, autoReason = true, "mutual" slog.Info("mutual relayed handshake auto-approved", "peer_node_id", fromNodeID) hm.markDirty() // Respond via registry and backfill public key @@ -1346,13 +1260,12 @@ func (hm *Manager) processRelayedRequest(fromNodeID uint32, justification string } // Check if peers are on the same network (network trust) - if hm.sameNetwork(fromNodeID) && autoAllowed { + if hm.sameNetwork(fromNodeID) { hm.markTrustedLocked(fromNodeID, &TrustRecord{ NodeID: fromNodeID, ApprovedAt: time.Now(), Network: hm.sharedNetwork(fromNodeID), }) - autoExecuted, autoReason = true, "same_network" slog.Info("same network relayed handshake auto-approved", "peer_node_id", fromNodeID) hm.markDirty() if reg := hm.rt.Registry(); reg != nil { @@ -1373,12 +1286,11 @@ func (hm *Manager) processRelayedRequest(fromNodeID uint32, justification string // // The relay carries no peer public key, so the empty key is passed: // allowlist entries that carry a pin do not match on this path. - if name, ok := hm.trustedAgentName(fromNodeID, ""); ok && autoAllowed { + if name, ok := hm.trustedAgentName(fromNodeID, ""); ok { hm.markTrustedLocked(fromNodeID, &TrustRecord{ NodeID: fromNodeID, ApprovedAt: time.Now(), }) - autoExecuted, autoReason = true, "trusted_agent" slog.Info("relayed handshake auto-approved (trusted agent)", "peer_node_id", fromNodeID, "agent", name) hm.rt.PublishEvent("handshake.auto_approved", map[string]interface{}{ @@ -1401,13 +1313,12 @@ func (hm *Manager) processRelayedRequest(fromNodeID uint32, justification string } // Auto-approve all requests when the daemon is configured to do so. - if hm.rt.TrustAutoApprove() && autoAllowed { + if hm.rt.TrustAutoApprove() { hm.markTrustedLocked(fromNodeID, &TrustRecord{ NodeID: fromNodeID, ApprovedAt: time.Now(), Mutual: false, }) - autoExecuted, autoReason = true, "auto_approve" slog.Info("relayed handshake auto-approved (trust-auto-approve enabled)", "peer_node_id", fromNodeID) hm.rt.PublishEvent("handshake.auto_approved", map[string]interface{}{ "peer_node_id": fromNodeID, "reason": "auto_approve", @@ -1459,29 +1370,11 @@ func (hm *Manager) ProcessRelayedApproval(fromNodeID uint32) { // This is called when the peer approved our outgoing request and the acceptance // was relayed back through the registry (because direct dial to port 444 failed). func (hm *Manager) processRelayedApproval(fromNodeID uint32) { - attempt, actionErr := hm.prepareTrustAction("trust.accept", fromNodeID, "outbound_completion_relay", "peer_acceptance", true, false) - if actionErr != nil { - slog.Info("trust establishment from relayed acceptance blocked", "peer_node_id", fromNodeID, "error", actionErr) - hm.rt.PublishEvent("handshake.control_blocked", map[string]interface{}{ - "peer_node_id": fromNodeID, "action": "trust.accept", - }) - return - } - granted := false - skipReason := "approval_not_applied" - defer func() { - if granted { - attempt.complete(actionhook.StatusSucceeded, "", map[string]string{"grant_reason": "peer_acceptance_relayed"}) - return - } - attempt.complete(actionhook.StatusSkipped, "", map[string]string{"skip_reason": skipReason}) - }() hm.mu.Lock() defer hm.mu.Unlock() // Already trusted? Nothing to do. if _, ok := hm.trusted[fromNodeID]; ok { - skipReason = "already_trusted" slog.Debug("relayed approval from already-trusted node", "peer_node_id", fromNodeID) return } @@ -1489,7 +1382,6 @@ func (hm *Manager) processRelayedApproval(fromNodeID uint32) { // Recently revoked? Drop the stale relay rather than re-establishing trust. if until, ok := hm.revoked[fromNodeID]; ok { if time.Now().Before(until) { - skipReason = "recently_revoked" slog.Info("ignoring relayed approval for recently-revoked peer", "peer_node_id", fromNodeID) delete(hm.outgoing, fromNodeID) return @@ -1498,7 +1390,6 @@ func (hm *Manager) processRelayedApproval(fromNodeID uint32) { } if _, ok := hm.outgoing[fromNodeID]; !ok { - skipReason = "no_outgoing_request" slog.Warn("dropping relayed approval with no matching outgoing request", "peer_node_id", fromNodeID) return } @@ -1509,7 +1400,6 @@ func (hm *Manager) processRelayedApproval(fromNodeID uint32) { ApprovedAt: time.Now(), Mutual: true, }) - granted = true hm.markDirty() slog.Info("trust established via relayed approval", "peer_node_id", fromNodeID) @@ -1588,21 +1478,16 @@ func (hm *Manager) processRelayedRejection(fromNodeID uint32) { // ApproveHandshake approves a pending handshake request. func (hm *Manager) ApproveHandshake(peerNodeID uint32) error { hm.mu.RLock() - preview, exists := hm.pending[peerNodeID] + _, exists := hm.pending[peerNodeID] hm.mu.RUnlock() if !exists { return nil } - attempt, err := hm.prepareTrustAction("trust.accept", peerNodeID, "inbound", "operator_approval", false, strings.TrimSpace(preview.Justification) != "") - if err != nil { - return err - } hm.mu.Lock() req, ok := hm.pending[peerNodeID] if !ok { hm.mu.Unlock() - attempt.complete(actionhook.StatusSkipped, "", map[string]string{"skip_reason": "request_no_longer_pending"}) return nil } hm.markTrustedLocked(peerNodeID, &TrustRecord{ @@ -1612,7 +1497,6 @@ func (hm *Manager) ApproveHandshake(peerNodeID uint32) error { }) hm.markDirty() hm.mu.Unlock() - attempt.complete(actionhook.StatusSucceeded, "", map[string]string{"grant_reason": "operator_approval"}) slog.Info("handshake approved", "peer_node_id", peerNodeID) hm.rt.PublishEvent("handshake.approved", map[string]interface{}{ diff --git a/zz_action_hook_flood_test.go b/zz_action_hook_flood_test.go deleted file mode 100644 index 2f8ab48..0000000 --- a/zz_action_hook_flood_test.go +++ /dev/null @@ -1,56 +0,0 @@ -// SPDX-License-Identifier: AGPL-3.0-or-later - -package handshake - -import ( - "testing" - - "github.com/pilot-protocol/common/decision" -) - -// TestActionHookNotInvokedForAlreadyTrustedOrOverCap pins SECURITY_REVIEW_v1.14 -// finding M1: the managed action hook (a synchronous authority round-trip in -// enforce mode) must NOT run for handshakes that need no new trust decision — -// an already-trusted peer or over-cap spam — so a peer cannot amplify each -// handshake into an authority call and bypass the anti-flood caps. -func TestActionHookNotInvokedForAlreadyTrustedOrOverCap(t *testing.T) { - runtime := newTestRuntime() - manager := NewManager(runtime) - t.Cleanup(manager.Stop) - hook := &trustHookStub{outcomes: map[string]decision.Outcome{}} - manager.SetActionHook(hook) - - hookCalls := func() int { - hook.mu.Lock() - defer hook.mu.Unlock() - return len(hook.before) - } - - // Already-trusted peer (matching key): the pre-hook fast path accepts - // without invoking the hook. - manager.mu.Lock() - manager.trusted[71] = &TrustRecord{NodeID: 71, PublicKey: "peer-key"} - manager.mu.Unlock() - manager.handleRequest(nil, &HandshakeMsg{NodeID: 71, PublicKey: "peer-key", Justification: "join"}, false) - if n := hookCalls(); n != 0 { - t.Fatalf("action hook invoked %d times for an already-trusted peer; want 0", n) - } - - // Over-cap spam: fill the pending queue, then an untrusted, unqueued peer is - // rejected before the hook. - manager.mu.Lock() - for i := uint32(1000); i < 1000+uint32(maxPendingHandshakes); i++ { - manager.pending[i] = &PendingHandshake{NodeID: i} - } - manager.mu.Unlock() - manager.handleRequest(nil, &HandshakeMsg{NodeID: 5000, PublicKey: "spam-key"}, false) - if n := hookCalls(); n != 0 { - t.Fatalf("action hook invoked %d times for over-cap spam; want 0 (rejected before hook)", n) - } - - // Relayed over-cap spam is likewise rejected before the hook. - manager.processRelayedRequest(6000, "join") - if n := hookCalls(); n != 0 { - t.Fatalf("action hook invoked %d times for over-cap relayed spam; want 0", n) - } -} diff --git a/zz_precheck_flood_test.go b/zz_precheck_flood_test.go new file mode 100644 index 0000000..fae6355 --- /dev/null +++ b/zz_precheck_flood_test.go @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package handshake + +import "testing" + +// TestPreChecksHandleAlreadyTrustedAndOverCap pins SECURITY_REVIEW_v1.14 +// finding M1 on the plain path: handshakes that need no new trust decision — +// an already-trusted peer or over-cap spam — are settled by the pre-checks. +// The trusted peer keeps its record and is not queued; over-cap spam, direct +// or relayed, is neither queued nor trusted. +func TestPreChecksHandleAlreadyTrustedAndOverCap(t *testing.T) { + runtime := newTestRuntime() + manager := NewManager(runtime) + t.Cleanup(manager.Stop) + + // Already-trusted peer (matching key): accepted on the fast path. + manager.mu.Lock() + manager.trusted[71] = &TrustRecord{NodeID: 71, PublicKey: "peer-key"} + manager.mu.Unlock() + manager.handleRequest(nil, &HandshakeMsg{NodeID: 71, PublicKey: "peer-key", Justification: "join"}, false) + manager.mu.RLock() + rec, trusted := manager.trusted[71] + _, queued := manager.pending[71] + manager.mu.RUnlock() + if !trusted || rec.PublicKey != "peer-key" { + t.Fatalf("already-trusted peer lost or changed its record: trusted=%v rec=%+v", trusted, rec) + } + if queued { + t.Fatal("already-trusted peer was queued for approval") + } + + // Over-cap spam: fill the pending queue, then an untrusted, unqueued peer + // is rejected. + manager.mu.Lock() + for i := uint32(1000); i < 1000+uint32(maxPendingHandshakes); i++ { + manager.pending[i] = &PendingHandshake{NodeID: i} + } + manager.mu.Unlock() + manager.handleRequest(nil, &HandshakeMsg{NodeID: 5000, PublicKey: "spam-key"}, false) + + // Relayed over-cap spam is likewise rejected. + manager.processRelayedRequest(6000, "join") + + manager.mu.RLock() + defer manager.mu.RUnlock() + if n := len(manager.pending); n != maxPendingHandshakes { + t.Fatalf("pending len = %d after over-cap spam, want %d", n, maxPendingHandshakes) + } + for _, id := range []uint32{5000, 6000} { + if _, ok := manager.pending[id]; ok { + t.Fatalf("over-cap peer %d was queued", id) + } + if _, ok := manager.trusted[id]; ok { + t.Fatalf("over-cap peer %d was trusted", id) + } + } +}