fix(pipeline): verify reported success over a document it cannot read

Author: keeper

SPEC.md

@@ -227,7 +227,19 @@ here. `internal/pipeline/manifest_test.go` pins the current field set.
 v2 records the immutability observed at write time, since gitdr now writes to non-WORM
 destinations too (§4). `destination.wormImmutable` and `wormDetails` capture it. The
 manifest is signed, so this is a tamper-evident answer to "was this backup on WORM
-storage?". `verify` doesn't check the schema string, so older manifests still verify.
+storage?". `verify` does not compare the schema *version*, so older manifests still verify — but from
+2026-09 it does refuse a document that is not a manifest at all.
+
+Before that, `verify -manifest` on a drill report printed `signature valid: true` and
+`0 of 0 artifacts ok`, and exited zero. The signature check is schema-agnostic and correct,
+because a signature is over bytes; the problem was what came after it. A drill report unmarshals
+into a `Manifest` without error, having no artifacts, so the command reported success over a
+document it cannot read — and would have gone on reporting success if that document were swapped
+for anything else signed by the same key.
+
+That is a behaviour change from exit 0 to non-zero for one input, and it is a fix rather than a
+break: the previous answer was wrong. A drill report is signed evidence and deserves a real
+check; `verify -manifest` is not it.
 
 **v3 adds `repos[].refs` and `repos[].copiedAt`, and it is additive: every v2 field is
 unchanged, both new fields are `omitempty`, and a v2 manifest still canonicalises to the bytes

internal/pipeline/pipeline_test.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"context"
 	"crypto/rand"
+	"encoding/base64"
+	"encoding/json"
 	"fmt"
 	"io"
 	"os"
@@ -452,3 +454,47 @@ func TestEncryptedBackupRestore(t *testing.T) {
 		t.Errorf("want an 'encrypted' hint in the error, got: %v", err)
 	}
 }
+
+// `verify` refuses a document that is not a manifest, rather than passing over it.
+//
+// Verify is schema-agnostic up to the signature, which is correct: a signature is over bytes.
+// Then it unmarshals into a Manifest, and a drill report unmarshals cleanly - it simply has no
+// artifacts. So `verify -manifest {ts}.drill.json` reported "signature valid, 0 of 0 ok" and
+// exited zero, on the evidence surface, for a document the command cannot read. That check
+// would have gone on passing if the report were swapped for anything else signed by the same
+// key, which is the definition of a check that cannot fail.
+func TestVerifyRefusesADocumentThatIsNotAManifest(t *testing.T) {
+	ctx := context.Background()
+	md := newMemDest(true)
+	pubPEM, privPEM, _ := crypto.GenerateKeyPair()
+	signer, _ := crypto.ParsePrivateKey(privPEM)
+	pub, _ := crypto.ParsePublicKey(pubPEM)
+
+	// A drill report, signed exactly as the engine signs one, so the only thing under test is
+	// what verify does after the signature holds.
+	report := pipeline.DrillReport{Schema: pipeline.DrillSchema, DrillID: "d1", Status: pipeline.StatusSuccess}
+	canon, err := json.Marshal(report)
+	if err != nil {
+		t.Fatal(err)
+	}
+	const key = "github.com/octo/drills/20260903T000000Z.drill.json"
+	if _, err := md.PutImmutable(ctx, key, strings.NewReader(string(canon)), int64(len(canon)), dest.Retention{}); err != nil {
+		t.Fatal(err)
+	}
+	sig := base64.StdEncoding.EncodeToString(crypto.Sign(signer, canon))
+	if _, err := md.PutImmutable(ctx, key+".sig", strings.NewReader(sig), int64(len(sig)), dest.Retention{}); err != nil {
+		t.Fatal(err)
+	}
+
+	res, err := pipeline.Verify(ctx, pipeline.VerifyDeps{Dest: md, PublicKey: pub}, key)
+	if err == nil {
+		t.Fatal("verify passed over a drill report; it counted zero artifacts as a success")
+	}
+	if !strings.Contains(err.Error(), pipeline.DrillSchema) {
+		t.Errorf("the refusal does not say what the document was: %v", err)
+	}
+	// The signature still held, and saying so is not the same as saying the document verified.
+	if res == nil || !res.SignatureValid {
+		t.Error("the signature check itself should still have run and passed")
+	}
+}

internal/pipeline/verify.go

@@ -58,6 +58,19 @@ func Verify(ctx context.Context, d VerifyDeps, manifestKey string) (*VerifyResul
 	if err := json.Unmarshal(canon, &m); err != nil {
 		return res, fmt.Errorf("parse manifest: %w", err)
 	}
+	// Refused rather than counted, and this is not pedantry about a field.
+	//
+	// Verify is schema-agnostic up to this point: it fetches the object and its .sig and checks
+	// the signature, which is right, because the signature is over bytes. Then it unmarshals into
+	// a Manifest, and a drill report unmarshals into a Manifest without error - it simply has no
+	// artifacts. So `verify -manifest {ts}.drill.json` printed "signature valid, 0 of 0 artifacts
+	// ok" and exited zero: a check that passes on a document it does not understand, and that
+	// would go on passing if the report were swapped for anything else signed by the same key.
+	//
+	// A drill report is signed evidence and deserves a real check; it is just not this one.
+	if m.Schema != "" && m.Schema != ManifestSchema && !strings.HasPrefix(m.Schema, "gitdr.manifest/") {
+		return res, fmt.Errorf("%s is a %s document, not a manifest: verify checks manifests, and counting its zero artifacts as a pass would be a green over a document this command cannot read", manifestKey, m.Schema)
+	}
 
 	for _, repo := range m.Repos {
 		for _, a := range repo.Artifacts {