test(covenantsigner): add missing signer defensive-property coverage

pkg/covenantsigner/server_test.go

@@ -1080,3 +1080,64 @@ func TestSubmitHandlerPreservesServiceContextValues(t *testing.T) {
 		)
 	}
 }
+
+// TestServerAcceptsBodyExactlyAtLimit asserts the request-body size guard is an
+// inclusive cap: a body of exactly maxRequestBodyBytes must pass, since
+// http.MaxBytesReader only rejects bodies strictly larger than the limit. This
+// complements the maxRequestBodyBytes+1 rejection in TestServerBoundaryErrorMatrix
+// and pins the exact boundary so an off-by-one in the cap would be caught.
+func TestServerAcceptsBodyExactlyAtLimit(t *testing.T) {
+	handle := newMemoryHandle()
+	service, err := NewService(handle, &scriptedEngine{
+		submit: func(*Job) (*Transition, error) {
+			return &Transition{State: JobStatePending, Detail: "queued"}, nil
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	server := httptest.NewServer(newHandler(service, context.Background(), "test-token", true))
+	defer server.Close()
+
+	// Build a well-formed submit envelope padded to exactly maxRequestBodyBytes
+	// by stretching the facadeRequestId string field.
+	prefix := `{"routeRequestId":"ors_exact","stage":"SIGNER_COORDINATION","request":{"facadeRequestId":"`
+	suffix := `"}}`
+	pad := maxRequestBodyBytes - len(prefix) - len(suffix)
+	if pad <= 0 {
+		t.Fatalf("test assumption broken: prefix+suffix already exceed the body limit")
+	}
+	body := []byte(prefix + strings.Repeat("a", pad) + suffix)
+	if len(body) != maxRequestBodyBytes {
+		t.Fatalf("expected body of exactly %d bytes, got %d", maxRequestBodyBytes, len(body))
+	}
+
+	request, err := http.NewRequest(
+		http.MethodPost,
+		server.URL+"/v1/self_v1/signer/requests",
+		bytes.NewReader(body),
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	request.Header.Set("Content-Type", "application/json")
+	request.Header.Set("Authorization", "Bearer test-token")
+
+	response, err := http.DefaultClient.Do(request)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer response.Body.Close()
+
+	// The body may still be rejected on its contents, but it must not be
+	// rejected by the size guard, which surfaces as "malformed request body".
+	responseBody, _ := io.ReadAll(response.Body)
+	if strings.Contains(string(responseBody), "malformed request body") {
+		t.Fatalf(
+			"body exactly at the limit was rejected as malformed (size guard fired): %d %s",
+			response.StatusCode,
+			string(responseBody),
+		)
+	}
+}

pkg/covenantsigner/store_lock_test.go

@@ -146,3 +146,53 @@ func TestNewStore_SequentialOpenCloseOpen(t *testing.T) {
 		}
 	}
 }
+
+// TestNewServiceReleasesLockOnInitFailure asserts that when NewService acquires
+// the store's file lock and then fails a later initialization step, it releases
+// the lock rather than leaking it. Without the release, a signer that failed to
+// start once (e.g. on a misconfigured trust root) could never restart against
+// the same data directory. The failure is triggered with an invalid custodian
+// trust-root public key, which is normalized after the store (and its lock) is
+// created.
+func TestNewServiceReleasesLockOnInitFailure(t *testing.T) {
+	tempDir := t.TempDir()
+
+	firstHandle, err := persistence.NewBasicDiskHandle(tempDir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = NewService(
+		firstHandle,
+		&scriptedEngine{},
+		WithDataDir(tempDir),
+		WithCustodianTrustRoots([]CustodianTrustRoot{
+			{
+				Route:     TemplateQcV1,
+				Reserve:   validMigrationDestination().Reserve,
+				Network:   validMigrationDestination().Network,
+				PublicKey: "0x1234",
+			},
+		}),
+	)
+	if err == nil {
+		t.Fatal("expected NewService to fail on the invalid custodian trust root")
+	}
+
+	// A second NewService on the same data directory must be able to acquire the
+	// lock, proving the failed first attempt released it.
+	secondHandle, err := persistence.NewBasicDiskHandle(tempDir)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	service, err := NewService(secondHandle, &scriptedEngine{}, WithDataDir(tempDir))
+	if err != nil {
+		t.Fatalf(
+			"expected second NewService to succeed; the lock should have been "+
+				"released after the first init failure, got: %v",
+			err,
+		)
+	}
+	t.Cleanup(func() { _ = service.Close() })
+}

pkg/tbtc/covenant_signer_test.go

@@ -1889,3 +1889,36 @@ func TestCovenantSignerEngine_VerifySignerApprovalRejectsNonLiveWallet(t *testin
 		t.Fatal("expected VerifySignerApproval to reject a closed wallet")
 	}
 }
+
+// TestEnsureActiveOutpointFinalityPropagatesProviderError asserts the finality
+// guard fails closed when the confirmation provider cannot answer. The existing
+// finality tests only exercise known confirmation counts above and below the
+// threshold; this covers the error path, where refusing to sign (rather than
+// treating an unknown count as sufficient) is the reorg-safety behavior we rely
+// on.
+func TestEnsureActiveOutpointFinalityPropagatesProviderError(t *testing.T) {
+	node, bitcoinChain, _ := setupCovenantSignerTestNode(t)
+
+	// A transaction the fake chain has never seen: it is neither in the
+	// confirmations map nor among the known transactions, so
+	// GetTransactionConfirmations returns "transaction not found". Version 99
+	// makes an accidental hash collision with any seeded transaction
+	// vanishingly unlikely.
+	unknownTransactionHash := (&bitcoin.Transaction{Version: 99}).Hash()
+	if _, err := bitcoinChain.GetTransactionConfirmations(unknownTransactionHash); err == nil {
+		t.Fatal("test setup: expected the fabricated transaction hash to be unknown to the chain")
+	}
+
+	cse := &covenantSignerEngine{
+		node:                               node,
+		minimumActiveOutpointConfirmations: 6,
+	}
+
+	err := cse.ensureActiveOutpointFinality(unknownTransactionHash)
+	if err == nil {
+		t.Fatal("expected finality check to fail when confirmations cannot be determined")
+	}
+	if !strings.Contains(err.Error(), "cannot determine active outpoint transaction confirmations") {
+		t.Fatalf("unexpected error message: %v", err)
+	}
+}

pkg/tbtc/signer_approval_certificate_test.go

@@ -1031,3 +1031,74 @@ func TestSignerApprovalCertificateSigningDigestMatchesCrossLanguageVectorAboveUi
 		)
 	}
 }
+
+// TestVerifySignerApprovalCertificateRejectsUnsupportedVersion asserts the
+// certificate version is a hard gate: only version 2 is accepted, and any other
+// value (an unset zero, the superseded version 1, or a future version 3) is
+// rejected before the signature is checked. This pins the version-negotiation
+// contract so a future format bump cannot be silently accepted by an unupgraded
+// verifier.
+func TestVerifySignerApprovalCertificateRejectsUnsupportedVersion(t *testing.T) {
+	node, _, walletPublicKey := setupCovenantSignerTestNode(t)
+	request := validStructuredSignerApprovalVerificationRequest(
+		t,
+		node,
+		walletPublicKey,
+		covenantsigner.TemplateSelfV1,
+	)
+
+	// The version gate is checked before the signer-set hash is examined, so the
+	// expected hash passed here is a deliberately arbitrary, non-empty
+	// placeholder: its value must not affect the outcome. Were the version check
+	// ever reordered after the hash comparison, this mismatching hash would
+	// surface as a "signer set hash does not match" error and fail the assertion
+	// below, catching the reordering.
+	arbitrarySignerSetHash := "0x" + strings.Repeat("ab", 32)
+
+	// An unset (0), the superseded (1), and a future (3) version must all be
+	// rejected; only 2 is supported.
+	for _, unsupportedVersion := range []uint32{0, 1, 3} {
+		certificate := *request.SignerApproval
+		certificate.CertificateVersion = unsupportedVersion
+
+		err := verifySignerApprovalCertificate(&certificate, arbitrarySignerSetHash)
+		if err == nil || !strings.Contains(err.Error(), "unsupported certificate version") {
+			t.Fatalf(
+				"expected unsupported certificate version error for version %d, got %v",
+				unsupportedVersion,
+				err,
+			)
+		}
+	}
+}
+
+// TestVerifySignerApprovalCertificateRejectsUnsupportedSignatureAlgorithm
+// asserts the signature-algorithm field is a hard gate: only the tECDSA
+// secp256k1 algorithm is accepted. This prevents a certificate from claiming a
+// different (potentially weaker or unverifiable) algorithm than the one the
+// verifier actually checks.
+func TestVerifySignerApprovalCertificateRejectsUnsupportedSignatureAlgorithm(t *testing.T) {
+	node, _, walletPublicKey := setupCovenantSignerTestNode(t)
+	request := validStructuredSignerApprovalVerificationRequest(
+		t,
+		node,
+		walletPublicKey,
+		covenantsigner.TemplateSelfV1,
+	)
+
+	// The algorithm gate is checked before the signer-set hash is examined, so
+	// the expected hash passed here is a deliberately arbitrary, non-empty
+	// placeholder: its value must not affect the outcome. Were the algorithm
+	// check ever reordered after the hash comparison, this mismatching hash would
+	// surface as a "signer set hash does not match" error and fail the assertion
+	// below, catching the reordering.
+	arbitrarySignerSetHash := "0x" + strings.Repeat("ab", 32)
+
+	certificate := *request.SignerApproval
+	certificate.SignatureAlgorithm = "ed25519"
+
+	err := verifySignerApprovalCertificate(&certificate, arbitrarySignerSetHash)
+	if err == nil || !strings.Contains(err.Error(), "unsupported signature algorithm") {
+		t.Fatalf("expected unsupported signature algorithm error, got %v", err)
+	}
+}