security(ci): assert the DR identity against parsed allow-list entries

scripts/validate-dr-signing/main.go

@@ -37,6 +37,11 @@ import (
 // DR-published artifact.
 const drIdentitySubject = `^https://github\.com/devantler-tech/platform/\.github/workflows/dr-rebuild\.yaml@refs/heads/main$`
 
+// drIdentityIssuer is the OIDC issuer half of that identity. cosign matches an
+// identity on the issuer/subject PAIR, so a correct subject under a different
+// issuer does not admit the DR signature and must not satisfy this gate.
+const drIdentityIssuer = `^https://token\.actions\.githubusercontent\.com$`
+
 // cdContractGateJob is the cd.yaml job that runs this validator on the
 // direct-push production path. Named once so the wiring check and the workflow
 // cannot drift apart silently.
@@ -238,13 +243,85 @@ func describePublicationStep(step map[string]any, index int) string {
 	return fmt.Sprintf("publication action step %d", index)
 }
 
+// clusterVerifyConfig mirrors only the fragment of the KSail cluster config this
+// gate reads: the cosign allow-list at spec.workload.flux.verify, which is the
+// path KSail renders onto the root OCIRepository. Decoding the exact path is
+// what makes a block sitting anywhere else a failure rather than a pass, because
+// such a block yields zero entries here.
+//
+// Decoding reads the FIRST YAML document only, which is what a KSail cluster
+// config is. A verify block hidden in a later document would therefore yield no
+// entries and FAIL this gate rather than satisfy it — the safe direction, and the
+// same direction every other shape below fails in.
+type clusterVerifyConfig struct {
+	Spec struct {
+		Workload struct {
+			Flux struct {
+				Verify struct {
+					MatchOIDCIdentity []struct {
+						Issuer  string `yaml:"issuer"`
+						Subject string `yaml:"subject"`
+					} `yaml:"matchOIDCIdentity"`
+				} `yaml:"verify"`
+			} `yaml:"flux"`
+		} `yaml:"workload"`
+	} `yaml:"spec"`
+}
+
+// validateVerifyAllowList checks that the cluster's cosign allow-list actually
+// admits the DR rebuild identity.
+//
+// 🔴 THE CONFIG IS PARSED, NOT SCANNED, for the same reason validateCDWiring
+// parses workflows rather than scanning them — this is the config half of that
+// same correction. Four ways a text scan says "allowed" about an allow-list that
+// is not in effect, each reproduced as an ablation in the tests:
+//
+//  1. The identity appears in a COMMENT. A comment has no effect on the cluster,
+//     so the gate certifies an allow-list that does not exist. Measured on the
+//     scanning version: removing the real matcher and re-adding the identical
+//     regex as `# …` returned "contract passed", exit 0.
+//  2. The whole verify block sits at a path KSail DISCARDS — spec.cluster.verify
+//     is exactly the #2627 outage. The bytes are in the file, so a scan finds
+//     them; KSail never renders them onto the OCIRepository.
+//  3. A subject that merely CONTAINS this identity satisfies a substring match,
+//     and that direction is the dangerous one: `^dr$|^attacker$` contains the DR
+//     identity while also admitting a second signer.
+//  4. The right subject under the WRONG ISSUER. cosign matches on the
+//     issuer/subject pair, so such an entry does not admit the DR signature — but
+//     a scan looking only for the subject line accepts it.
+//
+// An allow-list entry that is inert, discarded, over-broad, or issued by someone
+// else does not make a DR-published artifact verifiable, which is the one failure
+// that is unrecoverable by hand during an incident.
 func validateVerifyAllowList(config string) error {
-	if !containsLine(config, drIdentitySubject) {
+	var parsed clusterVerifyConfig
+	if err := yaml.Unmarshal([]byte(config), &parsed); err != nil {
+		return fmt.Errorf(
+			"cluster config does not parse, so its verification allow-list cannot be established: %w", err,
+		)
+	}
+
+	entries := parsed.Spec.Workload.Flux.Verify.MatchOIDCIdentity
+	if len(entries) == 0 {
 		return errors.New(
-			"cosign matchOIDCIdentity does not allow the DR rebuild identity, so a DR-published artifact stays unverifiable",
+			"no cosign matchOIDCIdentity entries at spec.workload.flux.verify, the path KSail renders onto the root " +
+				"OCIRepository, so a DR-published artifact stays unverifiable; a verify block at any other path " +
+				"(spec.cluster.verify in particular) is discarded and does not count",
 		)
 	}
-	return nil
+
+	for _, entry := range entries {
+		if entry.Issuer == drIdentityIssuer && entry.Subject == drIdentitySubject {
+			return nil
+		}
+	}
+
+	return fmt.Errorf(
+		"cosign matchOIDCIdentity at spec.workload.flux.verify has %d entries but none is exactly the DR rebuild "+
+			"identity, so a DR-published artifact stays unverifiable; add an entry with issuer %s and subject %s "+
+			"(both compared exactly — a wider regex containing this one is not accepted)",
+		len(entries), drIdentityIssuer, drIdentitySubject,
+	)
 }
 
 // validateCDWiring checks that the direct-push production path cannot deploy

scripts/validate-dr-signing/main_test.go

@@ -1020,6 +1020,96 @@ func TestVerifyAllowListRejectsAMissingDRIdentity(t *testing.T) {
 	}
 }
 
+// allowListConfig builds a minimal cluster config carrying one matchOIDCIdentity
+// entry at the given dotted path, so the path tests below differ in exactly one
+// variable: where the verify block sits.
+func allowListConfig(path string, issuer string, subject string) string {
+	var builder strings.Builder
+	builder.WriteString("apiVersion: ksail.io/v1alpha1\nkind: Cluster\n")
+	indent := ""
+	for _, key := range strings.Split(path, ".") {
+		builder.WriteString(indent + key + ":\n")
+		indent += "  "
+	}
+	builder.WriteString(indent + "provider: cosign\n")
+	builder.WriteString(indent + "matchOIDCIdentity:\n")
+	builder.WriteString(indent + "  - issuer: '" + issuer + "'\n")
+	builder.WriteString(indent + "    subject: '" + subject + "'\n")
+	return builder.String()
+}
+
+// This is the positive control for the three path/shape ablations that follow.
+// Without it, any of them could pass merely because the synthetic YAML is
+// malformed — a rejection for the wrong reason is indistinguishable from a
+// rejection for the right one.
+func TestVerifyAllowListAcceptsTheDRIdentityAtThePathKSailReads(t *testing.T) {
+	t.Parallel()
+
+	config := allowListConfig("spec.workload.flux.verify", drIdentityIssuer, drIdentitySubject)
+	if err := validateVerifyAllowList(config); err != nil {
+		t.Fatalf("the DR identity at the path KSail reads was rejected: %v", err)
+	}
+}
+
+// The bypass #2945 measured: a comment has no effect on the cluster, so a gate
+// satisfied by one certifies an allow-list that is not in effect.
+func TestVerifyAllowListRejectsACommentedDRIdentity(t *testing.T) {
+	t.Parallel()
+
+	config := repoFile(t, "ksail.prod.yaml")
+	removed := strings.ReplaceAll(config, drIdentitySubject, `^https://example\.invalid$`)
+	if removed == config {
+		t.Fatal("ablation changed nothing — the DR identity is not present to remove")
+	}
+	bypassed := removed + "\n# " + drIdentitySubject + "\n"
+	if err := validateVerifyAllowList(bypassed); err == nil {
+		t.Fatal("a commented-out DR identity was accepted as an allow-list entry")
+	}
+}
+
+// The second shape #2945 records: the whole block at a path KSail discards.
+// spec.cluster.verify is exactly the #2627 outage.
+func TestVerifyAllowListRejectsAVerifyBlockAtAPathKSailDiscards(t *testing.T) {
+	t.Parallel()
+
+	config := allowListConfig("spec.cluster.verify", drIdentityIssuer, drIdentitySubject)
+	err := validateVerifyAllowList(config)
+	if err == nil {
+		t.Fatal("a verify block at spec.cluster.verify — a path KSail discards — was accepted")
+	}
+	// Assert WHICH failure this is. Without this the test would also pass if the
+	// synthetic config were merely malformed, which would prove nothing about the
+	// path being discarded.
+	if !strings.Contains(err.Error(), "no cosign matchOIDCIdentity entries") {
+		t.Fatalf("rejected for the wrong reason — expected zero entries at the read path, got: %v", err)
+	}
+}
+
+// A substring match is blind in the LENGTHENING direction, and that direction is
+// dangerous here: `^dr$|^evil$` contains the DR identity and also allows a
+// second signer.
+func TestVerifyAllowListRejectsASubjectThatMerelyContainsTheDRIdentity(t *testing.T) {
+	t.Parallel()
+
+	widened := drIdentitySubject + `|^https://github\.com/attacker/repo/\.github/workflows/x\.yaml@refs/heads/main$`
+	config := allowListConfig("spec.workload.flux.verify", drIdentityIssuer, widened)
+	if err := validateVerifyAllowList(config); err == nil {
+		t.Fatal("a widened subject regex that merely contains the DR identity was accepted")
+	}
+}
+
+// An OIDC identity is the issuer/subject PAIR. A correct subject under a
+// different issuer does not match the DR signature, so it must not satisfy the
+// gate either.
+func TestVerifyAllowListRejectsTheDRSubjectUnderTheWrongIssuer(t *testing.T) {
+	t.Parallel()
+
+	config := allowListConfig("spec.workload.flux.verify", `^https://accounts\.google\.com$`, drIdentitySubject)
+	if err := validateVerifyAllowList(config); err == nil {
+		t.Fatal("the DR subject under a non-GitHub issuer was accepted")
+	}
+}
+
 func TestRunReportsSuccessAndFailure(t *testing.T) {
 	t.Parallel()