From 1fe21bb961b8f73444637e7b90d387b4f5f2aed2 Mon Sep 17 00:00:00 2001 From: Roger Ng Date: Fri, 18 Sep 2026 14:00:21 +0000 Subject: [PATCH] Bump witness dependency and handle malformed checkpoints as bad requests --- cmd/mtc/mirror/internal/handler/handler_test.go | 17 +++++------------ cmd/mtc/mirror/internal/handler/mirror_mux.go | 5 ++++- go.mod | 2 +- go.sum | 4 ++-- internal/witness/client.go | 5 +++++ mirror_lifecycle_test.go | 11 +++++++++++ 6 files changed, 28 insertions(+), 16 deletions(-) diff --git a/cmd/mtc/mirror/internal/handler/handler_test.go b/cmd/mtc/mirror/internal/handler/handler_test.go index 0b39b2178..a6560acb9 100644 --- a/cmd/mtc/mirror/internal/handler/handler_test.go +++ b/cmd/mtc/mirror/internal/handler/handler_test.go @@ -484,13 +484,13 @@ func TestSignSubtree_StatusCodes(t *testing.T) { wantStatus: http.StatusInternalServerError, }, { - name: "500 malformed checkpoint in request", + name: "400 malformed checkpoint in request", body: func() io.Reader { badCP := []byte("not a checkpoint") return bytes.NewReader(formatReqBody(testStart, testEnd, testSubRoot, testProof, badCP)) }, mockTarget: &mockTarget{}, - wantStatus: http.StatusInternalServerError, + wantStatus: http.StatusBadRequest, }, } { t.Run(test.name, func(t *testing.T) { @@ -531,7 +531,6 @@ func TestMirrorMux_SignSubtree(t *testing.T) { target2Func func(ctx context.Context, start, end uint64, subRoot []byte, proof [][]byte, cp []byte) ([]byte, error) wantCosig string wantErr error - wantAnyErr bool }{ { name: "dispatches to target 1", @@ -563,9 +562,9 @@ func TestMirrorMux_SignSubtree(t *testing.T) { wantErr: ErrUnknownLog, }, { - name: "malformed checkpoint returns error", - cp: []byte("not a valid checkpoint"), - wantAnyErr: true, + name: "malformed checkpoint returns ErrInvalidCheckpoint", + cp: []byte("not a valid checkpoint"), + wantErr: witness.ErrInvalidCheckpoint, }, } { t.Run(test.name, func(t *testing.T) { @@ -574,12 +573,6 @@ func TestMirrorMux_SignSubtree(t *testing.T) { _ = mux.AddTarget(origin2, &mockTarget{signSubtreeFunc: test.target2Func}) got, err := mux.SignSubtree(t.Context(), 0, 4, subRoot, proof, test.cp) - if test.wantAnyErr { - if err == nil { - t.Fatalf("got nil error, want non-nil error") - } - return - } if test.wantErr != nil { if !errors.Is(err, test.wantErr) { t.Fatalf("got error %v, want %v", err, test.wantErr) diff --git a/cmd/mtc/mirror/internal/handler/mirror_mux.go b/cmd/mtc/mirror/internal/handler/mirror_mux.go index b6cb6ba3f..091a2cc1f 100644 --- a/cmd/mtc/mirror/internal/handler/mirror_mux.go +++ b/cmd/mtc/mirror/internal/handler/mirror_mux.go @@ -72,7 +72,10 @@ func (m *MirrorMux) SignSubtree(ctx context.Context, start, end uint64, subRoot // signature and asserting the origin is correct. cpOrigin, _, _, err := parse.CheckpointUnsafe(cp) if err != nil { - return nil, err + // SPEC: the request body MUST be followed by a checkpoint, so an unparseable one is a bad + // request. ErrInvalidCheckpoint wraps witness.ErrBadRequest, which the handler turns into a + // 400 rather than a 500. + return nil, fmt.Errorf("%w: %v", witness.ErrInvalidCheckpoint, err) } t, err := m.target(cpOrigin) diff --git a/go.mod b/go.mod index b541972ff..7a9491300 100644 --- a/go.mod +++ b/go.mod @@ -27,7 +27,7 @@ require ( github.com/rivo/tview v0.42.0 github.com/transparency-dev/formats v0.1.2-0.20260916152522-091ce41666c0 github.com/transparency-dev/merkle v0.0.3-0.20260727102338-4491f478b7dc - github.com/transparency-dev/witness v0.0.0-20260917140356-67c4b6b76572 + github.com/transparency-dev/witness v0.0.0-20260918124936-55a5a0bf332a go.opentelemetry.io/contrib/detectors/aws/ec2/v2 v2.5.3 go.opentelemetry.io/contrib/detectors/aws/ecs v1.46.0 go.opentelemetry.io/contrib/detectors/gcp v1.46.0 diff --git a/go.sum b/go.sum index ef65b6318..59790b1c2 100644 --- a/go.sum +++ b/go.sum @@ -246,8 +246,8 @@ github.com/transparency-dev/formats v0.1.2-0.20260916152522-091ce41666c0 h1:4DbK github.com/transparency-dev/formats v0.1.2-0.20260916152522-091ce41666c0/go.mod h1:94kWdZfyStngLTYo2R0k1zM+n7FXldAkih6W04vyTnc= github.com/transparency-dev/merkle v0.0.3-0.20260727102338-4491f478b7dc h1:DaUOzC/KJOVtUqxKoFiEAQhoRu+TV8GaMKXQg94zRqs= github.com/transparency-dev/merkle v0.0.3-0.20260727102338-4491f478b7dc/go.mod h1:E+iHk6bS+tIgIJGD4TMeAjSjhQ9wPfL/ST4pXyITjdU= -github.com/transparency-dev/witness v0.0.0-20260917140356-67c4b6b76572 h1:OJla2bPwBEF97s6ekbpn+JXVIAmWa1huYk3YzUegbog= -github.com/transparency-dev/witness v0.0.0-20260917140356-67c4b6b76572/go.mod h1:/jLSvnpaCUkqTIvWOS+TkbOZ830HxQr1jVhX1xiowGQ= +github.com/transparency-dev/witness v0.0.0-20260918124936-55a5a0bf332a h1:OOj8z9FYso9cHA+ZHf/zUzF8zH/lzGhu0fOkGK0B9U0= +github.com/transparency-dev/witness v0.0.0-20260918124936-55a5a0bf332a/go.mod h1:/jLSvnpaCUkqTIvWOS+TkbOZ830HxQr1jVhX1xiowGQ= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavMF/ppJZNG9ZpyihvCd0w101no= github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= diff --git a/internal/witness/client.go b/internal/witness/client.go index e46b44959..7fec3de7a 100644 --- a/internal/witness/client.go +++ b/internal/witness/client.go @@ -384,6 +384,11 @@ func witnessErrorStatus(err error) string { return "no_witness_signature" case errors.Is(err, witness.ErrSubtreeRangeInvalid): return "subtree_range_invalid" + case errors.Is(err, witness.ErrInvalidCheckpoint): + return "invalid_checkpoint" + // Must come after the specific cases above, which wrap ErrBadRequest. + case errors.Is(err, witness.ErrBadRequest): + return "bad_request" case errors.Is(err, witness.ErrNotImplemented): return "not_implemented" default: diff --git a/mirror_lifecycle_test.go b/mirror_lifecycle_test.go index 874724c22..2946eecc0 100644 --- a/mirror_lifecycle_test.go +++ b/mirror_lifecycle_test.go @@ -991,6 +991,17 @@ func TestMirrorTarget_SignSubtree(t *testing.T) { cp: mustCosignCP(t, testPendingCPOrigin, treeSize, rootHash, testLogSigner, testMirrorSigner), wantErr: witness.ErrInvalidProof, }, + { + // A checkpoint which isn't a well-formed note is a bad request rather than a signature + // failure, so it must not be reported as ErrNoWitnessSignature. + name: "malformed checkpoint", + start: 0, + end: 4, + subRoot: validSubRoot, + proof: validProof, + cp: []byte("not a checkpoint"), + wantErr: witness.ErrInvalidCheckpoint, + }, } { t.Run(test.name, func(t *testing.T) { got, err := mt.SignSubtree(ctx, test.start, test.end, test.subRoot, test.proof, test.cp)