Catch step env/job env edge case

agent/integration/job_verification_integration_test.go

@@ -88,6 +88,19 @@ var (
 			"DEPLOY":            "crimes",
 		},
 	}
+
+	jobWithStepEnvButNoCorrespondingJobEnv = api.Job{
+		ChunksMaxSizeBytes: 1024,
+		ID:                 defaultJobID,
+		Step: pipeline.CommandStep{
+			Command: "echo hello world",
+			Env:     map[string]string{"CRIMES": "disable"},
+		},
+		Env: map[string]string{
+			"BUILDKITE_COMMAND": "echo hello world",
+			"DEPLOY":            "0",
+		},
+	}
 )
 
 func TestJobVerification(t *testing.T) {
@@ -212,6 +225,17 @@ func TestJobVerification(t *testing.T) {
 			expectedSignalReason:     agent.SignalReasonSignatureRejected,
 			expectLogsContain:        []string{"⚠️ ERROR"},
 		},
+		{
+			name:                     "when the step has a signature, but the step env is not in the job env, it fails signature verification",
+			agentConf:                agent.AgentConfiguration{JobVerificationNoSignatureBehavior: agent.VerificationBehaviourBlock},
+			job:                      jobWithStepEnvButNoCorrespondingJobEnv,
+			signingKey:               symmetricJWKFor(t, signingKeyLlamas),
+			verificationJWKS:         jwksFromKeys(t, symmetricJWKFor(t, signingKeyLlamas)),
+			mockBootstrapExpectation: func(bt *bintest.Mock) { bt.Expect().NotCalled() },
+			expectedExitStatus:       "-1",
+			expectedSignalReason:     agent.SignalReasonSignatureRejected,
+			expectLogsContain:        []string{"⚠️ ERROR"},
+		},
 	}
 
 	for _, tc := range cases {

agent/verify_job.go

@@ -65,12 +65,14 @@ func (r *JobRunner) verifyJob(keySet jwk.Set) error {
 	// step. If they don't, we need to fail the job - more or less the only reason that the job and the step would have
 	// different fields would be if someone had modified the job on the backend after it was signed (aka crimes)
 	//
-	// However, this consistency check does not apply to `env::`:
-	// 1. Signature.Verify ensures every signed env var has the right value and
-	//    no signed vars are missing from the job env, and
+	// However, this consistency check does not apply entirely to `env::`:
+	// 1. Signature.Verify ensures every signed env var has the right value, and
 	// 2. step env can be overridden by the pipeline env, but each step only
 	//    knows about its own env. So the job env and step env can disagree
 	//    under normal circumstances.
+	// We still have to catch when a signature validates only because the step
+	// has an env var (not used to run the job), that is not present in the
+	// job env (is actually used).
 	signedFields := step.Signature.SignedFields
 
 	jobFields, err := r.conf.Job.ValuesForFields(signedFields)
@@ -84,7 +86,8 @@ func (r *JobRunner) verifyJob(keySet jwk.Set) error {
 	}
 
 	for _, field := range signedFields {
-		if strings.HasPrefix(field, pipeline.EnvNamespacePrefix) {
+		if strings.HasPrefix(field, pipeline.EnvNamespacePrefix) && stepFields[field] == "" && jobFields[field] != "" {
+			// Signature.Verify handled this case.
 			continue
 		}