fix: implement audit findings from consolidated review

pkg/chain/ethereum/tbtc.go

@@ -1478,8 +1478,10 @@ func (tc *TbtcChain) GetWallet(
 
 	walletRegistryWallet, err := tc.walletRegistry.GetWallet(wallet.EcdsaWalletID)
 	if err != nil {
-		logger.Warnf(
-			"cannot get wallet registry data for wallet [0x%x]: [%v]",
+		logger.Errorf(
+			"wallet registry unavailable for wallet [0x%x]; "+
+				"MembersIDsHash will be zero -- signer approval "+
+				"operations will fail until the registry recovers: [%v]",
 			wallet.EcdsaWalletID,
 			err,
 		)

pkg/covenantsigner/config.go

@@ -10,7 +10,9 @@ type Config struct {
 	// binds to. Empty defaults to loopback-only.
 	ListenAddress string
 	// AuthToken enables static Bearer authentication for signer endpoints.
-	// Non-loopback binds must set this.
+	// Non-loopback binds must set this. Prefer environment variables or
+	// config files over CLI flags to avoid exposing the token in
+	// /proc/PID/cmdline.
 	AuthToken string
 	// EnableSelfV1 exposes the self_v1 signer HTTP routes. Keep this disabled
 	// for a qc_v1-first launch unless self_v1 has cleared its own go-live gate.

pkg/covenantsigner/server.go

@@ -124,6 +124,7 @@ func Initialize(
 
 	listener, err := net.Listen("tcp", server.httpServer.Addr)
 	if err != nil {
+		cancelService()
 		return nil, false, fmt.Errorf("failed to bind covenant signer port [%d]: %w", config.Port, err)
 	}
 
@@ -251,7 +252,7 @@ func newHandler(service *Service, serviceCtx context.Context, authToken string,
 	}
 
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if r.URL.Path == "/healthz" {
+		if r.Method == http.MethodGet && r.URL.Path == "/healthz" {
 			mux.ServeHTTP(w, r)
 			return
 		}

pkg/covenantsigner/server_test.go

@@ -867,7 +867,7 @@ func TestSubmitHandlerPreCancelledContextStillSucceeds(t *testing.T) {
 
 type contextKey string
 
-func TestSubmitHandlerPreservesContextValues(t *testing.T) {
+func TestSubmitHandlerPreservesServiceContextValues(t *testing.T) {
 	const testKey contextKey = "test-trace-id"
 	const testValue = "trace-abc-123"
 
@@ -889,15 +889,13 @@ func TestSubmitHandlerPreservesContextValues(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Wrap the handler with middleware that injects a value into the request
-	// context. The detached context should preserve this value.
-	innerHandler := newHandler(service, context.Background(), "", true)
-	wrappedHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		enrichedCtx := context.WithValue(r.Context(), testKey, testValue)
-		innerHandler.ServeHTTP(w, r.WithContext(enrichedCtx))
-	})
+	// Inject a value into the service context. The submit handler derives
+	// its timeout context from serviceCtx (not from the HTTP request), so
+	// values on the service context must be visible to the engine.
+	serviceCtx := context.WithValue(context.Background(), testKey, testValue)
+	handler := newHandler(service, serviceCtx, "", true)
 
-	server := httptest.NewServer(wrappedHandler)
+	server := httptest.NewServer(handler)
 	defer server.Close()
 
 	submitPayload := mustJSON(t, SignerSubmitInput{
@@ -925,7 +923,7 @@ func TestSubmitHandlerPreservesContextValues(t *testing.T) {
 	defer mu.Unlock()
 	if capturedValue != testValue {
 		t.Fatalf(
-			"expected context value %q to be preserved through detachment, "+
+			"expected service context value %q to be visible in engine, "+
 				"got %v",
 			testValue,
 			capturedValue,

pkg/covenantsigner/service.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"crypto/rand"
 	"encoding/hex"
+	"errors"
 	"fmt"
 	"reflect"
 	"sync"
@@ -229,50 +230,28 @@ func (s *Service) loadPollJob(route TemplateID, input SignerPollInput) (*Job, er
 	return job, nil
 }
 
-func (s *Service) Submit(ctx context.Context, route TemplateID, input SignerSubmitInput) (StepResult, error) {
-	submitValidationOptions := validationOptions{
-		migrationPlanQuoteTrustRoots:      s.migrationPlanQuoteTrustRoots,
-		depositorTrustRoots:               s.depositorTrustRoots,
-		custodianTrustRoots:               s.custodianTrustRoots,
-		requireFreshMigrationPlanQuote:    true,
-		migrationPlanQuoteVerificationNow: s.now(),
-		signerApprovalVerifier:            s.signerApprovalVerifier,
-	}
-	if err := validateSubmitInput(route, input, submitValidationOptions); err != nil {
-		return StepResult{}, err
-	}
-
-	normalizedRequest, err := normalizeRouteSubmitRequest(
-		input.Request,
-		validationOptions{
-			migrationPlanQuoteTrustRoots: s.migrationPlanQuoteTrustRoots,
-			depositorTrustRoots:          s.depositorTrustRoots,
-			custodianTrustRoots:          s.custodianTrustRoots,
-			signerApprovalVerifier:       s.signerApprovalVerifier,
-		},
-	)
-	if err != nil {
-		return StepResult{}, err
-	}
-
-	requestDigest, err := requestDigestFromNormalized(normalizedRequest)
-	if err != nil {
-		return StepResult{}, err
-	}
-
+// createOrDedup creates a new job under the service mutex, or returns the
+// existing job result if the route request is already known. Returns
+// (job, nil, nil) for a new job, or (nil, result, nil) for a dedup hit.
+func (s *Service) createOrDedup(
+	route TemplateID,
+	input SignerSubmitInput,
+	normalizedRequest RouteSubmitRequest,
+	requestDigest string,
+) (*Job, *StepResult, error) {
 	s.mutex.Lock()
+	defer s.mutex.Unlock()
+
 	if existing, ok, err := s.store.GetByRouteRequest(route, input.RouteRequestID); err != nil {
-		s.mutex.Unlock()
-		return StepResult{}, err
+		return nil, nil, err
 	} else if ok {
 		if existing.RequestDigest != requestDigest {
-			s.mutex.Unlock()
-			return StepResult{}, &inputError{
+			return nil, nil, &inputError{
 				"routeRequestId already exists with a different request payload",
 			}
 		}
-		s.mutex.Unlock()
-		return mapJobResult(existing), nil
+		result := mapJobResult(existing)
+		return nil, &result, nil
 	}
 
 	requestIDPrefix := ""
@@ -282,14 +261,12 @@ func (s *Service) Submit(ctx context.Context, route TemplateID, input SignerSubm
 	case TemplateSelfV1:
 		requestIDPrefix = "kcs_self"
 	default:
-		s.mutex.Unlock()
-		return StepResult{}, fmt.Errorf("unsupported route: %s", route)
+		return nil, nil, fmt.Errorf("unsupported route: %s", route)
 	}
 
 	requestID, err := newRequestID(requestIDPrefix)
 	if err != nil {
-		s.mutex.Unlock()
-		return StepResult{}, err
+		return nil, nil, err
 	}
 
 	now := s.now()
@@ -309,10 +286,50 @@ func (s *Service) Submit(ctx context.Context, route TemplateID, input SignerSubm
 	}
 
 	if err := s.store.Put(job); err != nil {
-		s.mutex.Unlock()
+		return nil, nil, err
+	}
+
+	return job, nil, nil
+}
+
+func (s *Service) Submit(ctx context.Context, route TemplateID, input SignerSubmitInput) (StepResult, error) {
+	submitValidationOptions := validationOptions{
+		migrationPlanQuoteTrustRoots:      s.migrationPlanQuoteTrustRoots,
+		depositorTrustRoots:               s.depositorTrustRoots,
+		custodianTrustRoots:               s.custodianTrustRoots,
+		requireFreshMigrationPlanQuote:    true,
+		migrationPlanQuoteVerificationNow: s.now(),
+		signerApprovalVerifier:            s.signerApprovalVerifier,
+	}
+	if err := validateSubmitInput(route, input, submitValidationOptions); err != nil {
 		return StepResult{}, err
 	}
-	s.mutex.Unlock()
+
+	normalizedRequest, err := normalizeRouteSubmitRequest(
+		input.Request,
+		validationOptions{
+			migrationPlanQuoteTrustRoots: s.migrationPlanQuoteTrustRoots,
+			depositorTrustRoots:          s.depositorTrustRoots,
+			custodianTrustRoots:          s.custodianTrustRoots,
+			signerApprovalVerifier:       s.signerApprovalVerifier,
+		},
+	)
+	if err != nil {
+		return StepResult{}, err
+	}
+
+	requestDigest, err := requestDigestFromNormalized(normalizedRequest)
+	if err != nil {
+		return StepResult{}, err
+	}
+
+	job, existingResult, err := s.createOrDedup(route, input, normalizedRequest, requestDigest)
+	if err != nil {
+		return StepResult{}, err
+	}
+	if existingResult != nil {
+		return *existingResult, nil
+	}
 
 	transition, err := s.engine.OnSubmit(ctx, job)
 	if err != nil {
@@ -329,7 +346,7 @@ func (s *Service) Submit(ctx context.Context, route TemplateID, input SignerSubm
 	s.mutex.Lock()
 	defer s.mutex.Unlock()
 
-	currentJob, ok, err := s.store.GetByRequestID(requestID)
+	currentJob, ok, err := s.store.GetByRequestID(job.RequestID)
 	if err != nil {
 		return StepResult{}, err
 	}
@@ -378,7 +395,7 @@ func (s *Service) Poll(ctx context.Context, route TemplateID, input SignerPollIn
 
 	transition, pollErr := s.engine.OnPoll(ctx, job)
 	if pollErr != nil {
-		if pollErr != errJobNotFound {
+		if !errors.Is(pollErr, errJobNotFound) {
 			return StepResult{}, pollErr
 		}
 	}
@@ -398,7 +415,7 @@ func (s *Service) Poll(ctx context.Context, route TemplateID, input SignerPollIn
 		return mapJobResult(currentJob), nil
 	}
 
-	if pollErr == errJobNotFound {
+	if errors.Is(pollErr, errJobNotFound) {
 		applyTransition(currentJob, &Transition{
 			State:  JobStateFailed,
 			Reason: ReasonJobNotFound,