feat(registry/types): allow attestation constraints on skill provenance

registry/types/data/skill.schema.json

@@ -173,7 +173,7 @@
     },
     "provenance": {
       "type": "object",
-      "description": "Expected signer identity for this skill, checked on first install instead of trust-on-first-use. Every field is optional: an absent or empty value leaves that dimension unconstrained. Mirrors the server provenance definition in publisher-provided.schema.json, with two deliberate differences. First, no 'format' keywords, because the Go Provenance struct serializes unset fields as empty strings and an empty string does not satisfy 'uri' or 'hostname'. Second, 'attestation' is not accepted and 'additionalProperties' is false: the verifier only compares an expected attestation when the artifact also carries one, so a declared attestation constraint would be silently skipped against a signature that has none. Rejecting the key keeps the contract fail-closed -- a constraint this path cannot enforce is refused loudly rather than ignored. The same strictness makes a misspelled constraint key an error instead of a silently dropped guarantee.",
+      "description": "Expected provenance for this skill, checked on first install instead of trust-on-first-use. Every field is optional, and an empty string leaves that dimension unconstrained. The exception is 'attestation', where the key's presence is itself the constraint: see its own description. Mirrors the server provenance definition in publisher-provided.schema.json, with two deliberate differences. First, no 'format' keywords, because the Go Provenance struct serializes unset fields as empty strings and an empty string does not satisfy 'uri' or 'hostname'. Second, 'additionalProperties' is false, so a misspelled constraint key is a loud error rather than a silently dropped guarantee.",
       "additionalProperties": false,
       "properties": {
         "sigstore_url": {
@@ -214,6 +214,29 @@
           "examples": [
             "https://token.actions.githubusercontent.com"
           ]
+        },
+        "attestation": {
+          "$ref": "#/$defs/verified_attestation"
+        }
+      }
+    },
+    "verified_attestation": {
+      "type": "object",
+      "description": "Expected in-toto attestation. Declaring this key at all requires the artifact to carry an attestation: verification fails against a signature that has none. Both members are optional and constrain independently, so an empty object means 'must be attested, no constraint on what the attestation says'.",
+      "additionalProperties": false,
+      "properties": {
+        "predicate_type": {
+          "type": "string",
+          "description": "Expected predicate type. When set, it must equal the predicate type of the statement carried by the signature.",
+          "examples": [
+            "https://slsa.dev/provenance/v0.2",
+            "https://slsa.dev/provenance/v1"
+          ]
+        },
+        "predicate": {
+          "type": "object",
+          "description": "Expected predicate body, compared for deep equality against the statement predicate. Constrained to an object because an in-toto predicate is always one: the verifier normalizes the expected value through structpb before comparing, and a non-object expectation fails that step, so it could never match any artifact.",
+          "additionalProperties": true
         }
       }
     },

registry/types/skills_types.go

@@ -82,11 +82,14 @@ type Skill struct {
 	// — most catalog entries won't have this for a while, and that must not
 	// break installs; it's an opt-in tightening per entry, not a requirement.
 	//
-	// Only the identity fields are honored. Setting Provenance.Attestation
-	// makes Validate fail: the verifier compares an expected attestation only
-	// when the artifact also carries one, so the constraint would be silently
-	// skipped against a signature that has none. Refusing it loudly keeps the
-	// contract fail-closed until that gap is fixed.
+	// Each field constrains independently, and an empty string leaves that
+	// dimension unconstrained. Attestation is the exception: setting it at
+	// all, even to an empty struct, requires the artifact to be attested, so
+	// verification fails against a signature carrying no statement. Its own
+	// PredicateType and Predicate then follow the usual rule and constrain
+	// only when set. Predicate must be a JSON object; anything else can never
+	// match, and Validate rejects it rather than letting it through as a
+	// constraint that silently fails every artifact.
 	Provenance *Provenance `json:"provenance,omitempty" yaml:"provenance,omitempty"`
 	// Metadata is the official metadata of the skill as reported in the
 	// SKILL.md file.

registry/types/skills_types_test.go

@@ -23,6 +23,7 @@ const (
 	testSkillIdentifier = "ghcr.io/stacklok/dockyard/skills/commit:1.0.0"
 
 	testCaseFullProvenance = "fully populated provenance"
+	testPredicateType      = "https://slsa.dev/provenance/v1"
 )
 
 // fullProvenance returns a Provenance with every field populated, matching the
@@ -86,14 +87,11 @@ func TestSkill_ProvenanceJSONRoundTrip(t *testing.T) {
 			wantKey: true,
 		},
 		{
-			// The shared Provenance type carries Attestation, so it still
-			// serializes here. Validate rejects it; see
-			// TestSkill_ValidateWithProvenance.
 			name: "provenance with attestation",
 			provenance: &Provenance{
 				SignerIdentity: testSignerIdentity,
 				Attestation: &VerifiedAttestation{
-					PredicateType: "https://slsa.dev/provenance/v1",
+					PredicateType: testPredicateType,
 					Predicate:     map[string]any{"buildType": "workflow"},
 				},
 			},
@@ -177,10 +175,12 @@ func TestSkill_ProvenanceYAMLRoundTrip(t *testing.T) {
 	}
 }
 
-// TestSkill_ValidateWithProvenance verifies that Validate accepts skills with
-// provenance in every degree of population. Provenance is an opt-in tightening
-// per catalog entry, so neither its absence nor a partially filled value may
-// cause validation to fail.
+// TestSkill_ValidateWithProvenance verifies that Validate accepts provenance in
+// every degree of population, since it is an opt-in tightening per catalog
+// entry and neither its absence nor a partially filled value may fail
+// validation. The exception is a constraint the verifier could never satisfy:
+// a non-object attestation predicate is rejected here rather than shipped as an
+// entry that fails against every artifact.
 func TestSkill_ValidateWithProvenance(t *testing.T) {
 	t.Parallel()
 
@@ -211,27 +211,48 @@ func TestSkill_ValidateWithProvenance(t *testing.T) {
 			provenance: &Provenance{},
 		},
 		{
-			// The verifier only compares an expected attestation when the
-			// artifact also carries one, so this constraint would be silently
-			// skipped against a signature with no attestation. Reject it
-			// rather than let a catalog author believe it is enforced.
-			name: "attestation is refused, not silently ignored",
+			name: "fully specified attestation",
 			provenance: &Provenance{
 				SignerIdentity: testSignerIdentity,
 				CertIssuer:     testCertIssuer,
 				Attestation: &VerifiedAttestation{
-					PredicateType: "https://slsa.dev/provenance/v1",
+					PredicateType: testPredicateType,
 					Predicate:     map[string]any{"buildType": "workflow"},
 				},
 			},
-			expectError: true,
 		},
 		{
-			name: "empty attestation is still refused",
+			// Means "must be attested, no constraint on what the attestation
+			// says" -- the verifier still requires the artifact to carry a
+			// statement.
+			name: "empty attestation constrains only presence",
 			provenance: &Provenance{
 				SignerIdentity: testSignerIdentity,
 				Attestation:    &VerifiedAttestation{},
 			},
+		},
+		{
+			name: "predicate type without predicate",
+			provenance: &Provenance{
+				Attestation: &VerifiedAttestation{PredicateType: testPredicateType},
+			},
+		},
+		{
+			// normalizeAttestationPredicate requires map[string]any, so a
+			// scalar predicate can never match any artifact. Reject it at
+			// authoring time instead of shipping a constraint that always
+			// fails.
+			name: "scalar predicate is rejected",
+			provenance: &Provenance{
+				Attestation: &VerifiedAttestation{Predicate: "not-an-object"},
+			},
+			expectError: true,
+		},
+		{
+			name: "array predicate is rejected",
+			provenance: &Provenance{
+				Attestation: &VerifiedAttestation{Predicate: []any{"a", "b"}},
+			},
 			expectError: true,
 		},
 	}
@@ -292,11 +313,26 @@ func TestValidateSkillBytes_Provenance(t *testing.T) {
 			expectError: true,
 		},
 		{
-			// Not "must be an object" -- the key is refused outright, because
-			// the verifier cannot enforce an attestation constraint against a
-			// signature that carries none.
-			name:        "attestation key is rejected",
-			provenance:  `{"attestation":{"predicate_type":"https://slsa.dev/provenance/v1"}}`,
+			name:       "valid attestation constraint",
+			provenance: `{"attestation":{"predicate_type":"https://slsa.dev/provenance/v1","predicate":{"buildType":"workflow"}}}`,
+		},
+		{
+			name:       "valid empty attestation constraint",
+			provenance: `{"attestation":{}}`,
+		},
+		{
+			name:        "attestation must be an object",
+			provenance:  `{"attestation":"nope"}`,
+			expectError: true,
+		},
+		{
+			name:        "attestation predicate must be an object",
+			provenance:  `{"attestation":{"predicate":"not-an-object"}}`,
+			expectError: true,
+		},
+		{
+			name:        "misspelled attestation key is rejected",
+			provenance:  `{"attestation":{"predicatetype":"x"}}`,
 			expectError: true,
 		},
 		{