diff --git a/pkg/covenantsigner/server_test.go b/pkg/covenantsigner/server_test.go index 246f103bed..46678c2d65 100644 --- a/pkg/covenantsigner/server_test.go +++ b/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), + ) + } +} diff --git a/pkg/covenantsigner/store_lock_test.go b/pkg/covenantsigner/store_lock_test.go index b763352c8b..dc337ed7fe 100644 --- a/pkg/covenantsigner/store_lock_test.go +++ b/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() }) +} diff --git a/pkg/tbtc/covenant_signer_test.go b/pkg/tbtc/covenant_signer_test.go index 67ef7a6846..549956decb 100644 --- a/pkg/tbtc/covenant_signer_test.go +++ b/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) + } +} diff --git a/pkg/tbtc/signer_approval_certificate_test.go b/pkg/tbtc/signer_approval_certificate_test.go index d75f2097b9..f748e7a936 100644 --- a/pkg/tbtc/signer_approval_certificate_test.go +++ b/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) + } +}