fix(run): manual: false also blocks a job pulled in transitively

materializeJobClosure walked job-typed depends_on edges without
re-checking triggers.manual: false on anything but the top-level run
target, so `docker compose run A` where A depends_on job B (manual:
false) silently ran B anyway.

manual: false declares a job harmful to trigger outside its
schedule; depends_on doesn't change who caused the execution or when
-- pulling B in to satisfy A's dependency is still the run command
causing that out-of-schedule execution, one hop removed. Refuse it
the same way the top-level check already does, before anything in the
closure is created.

Signed-off-by: Nicolas De Loof <[email protected]>

cmd/compose/run.go

@@ -465,15 +465,21 @@ func materializeManualJob(project *types.Project, name string) (*types.Project,
 	// run-to-completion container satisfying its declared condition),
 	// instead of dangling as an unresolvable name.
 	jobs := project.AllJobs()
-	materializeJobClosure(project, jobs, job, map[string]bool{name: true})
+	if err := materializeJobClosure(project, jobs, job, map[string]bool{name: true}); err != nil {
+		return nil, err
+	}
 	project.Services[name] = jobAsService(project, name, job)
 	return project, nil
 }
 
 // materializeJobClosure adds every job reachable through job-typed
 // depends_on edges to project.Services. seen carries the starting job and
-// guards against dependency cycles.
-func materializeJobClosure(project *types.Project, jobs types.Jobs, job types.JobConfig, seen map[string]bool) {
+// guards against dependency cycles. manual: false is checked here too, not
+// just on the top-level run target: it declares a job harmful to trigger
+// outside its schedule, and pulling it in as a dependency is still the
+// user's run command causing that out-of-schedule execution, just one hop
+// removed.
+func materializeJobClosure(project *types.Project, jobs types.Jobs, job types.JobConfig, seen map[string]bool) error {
 	for dep := range job.DependsOn {
 		if seen[dep] {
 			continue
@@ -483,9 +489,15 @@ func materializeJobClosure(project *types.Project, jobs types.Jobs, job types.Jo
 		if !isJob {
 			continue
 		}
-		materializeJobClosure(project, jobs, depJob, seen)
+		if depJob.Triggers != nil && depJob.Triggers.Manual != nil && !*depJob.Triggers.Manual {
+			return fmt.Errorf("job %q is declared with manual: false, it cannot be triggered even as a dependency of another job", dep)
+		}
+		if err := materializeJobClosure(project, jobs, depJob, seen); err != nil {
+			return err
+		}
 		project.Services[dep] = jobAsService(project, dep, depJob)
 	}
+	return nil
 }
 
 // jobAsService materializes a job as a service for the one-off machinery: a

cmd/compose/run_test.go

@@ -60,6 +60,16 @@ func TestMaterializeManualJob(t *testing.T) {
 					ContainerSpec: types.ContainerSpec{Image: "deployer"},
 					WorkloadSpec:  types.WorkloadSpec{DependsOn: types.DependsOnConfig{"prep": {Condition: types.ServiceConditionCompletedSuccessfully, Required: true}}},
 				},
+				"sensitive": {
+					Name:     "sensitive",
+					Triggers: &types.TriggerConfig{Manual: &no, Schedule: []types.ScheduleConfig{{Cron: "0 3 * * *"}}},
+				},
+				"escalate": {
+					Name:          "escalate",
+					Triggers:      &types.TriggerConfig{Manual: &yes},
+					ContainerSpec: types.ContainerSpec{Image: "escalator"},
+					WorkloadSpec:  types.WorkloadSpec{DependsOn: types.DependsOnConfig{"sensitive": {Condition: types.ServiceConditionCompletedSuccessfully, Required: true}}},
+				},
 			},
 		}
 	}
@@ -122,6 +132,14 @@ func TestMaterializeManualJob(t *testing.T) {
 		_, err := materializeManualJob(base(), "rotation")
 		assert.Error(t, err, `job "rotation" is declared with manual: false, it cannot be run manually`)
 	})
+
+	// depends_on doesn't change who caused the execution or when: pulling in
+	// a manual: false job as a dependency of a manually-run job is still the
+	// run command triggering it out of schedule, one hop removed.
+	t.Run("manual: false also blocks the job when pulled in transitively", func(t *testing.T) {
+		_, err := materializeManualJob(base(), "escalate")
+		assert.Error(t, err, `job "sensitive" is declared with manual: false, it cannot be triggered even as a dependency of another job`)
+	})
 }
 
 // runProject only retries the unselected/full load to look for a job when

pkg/e2e/jobs_test.go

@@ -69,3 +69,16 @@ func TestRunJobDependsOnJob(t *testing.T) {
 			OutputContains("deploy done"),
 			ServiceState("db", "running"))
 }
+
+// manual: false declares a job harmful to trigger outside its schedule.
+// depends_on doesn't change who caused the execution or when: pulling the
+// job in as a dependency of a manually-run job is still the run command
+// causing that out-of-schedule execution, one hop removed, so it must be
+// refused too — before anything else in the closure is created.
+func TestRunJobDependsOnManualFalseJob(t *testing.T) {
+	NewScenario(t, "a job depending on a manual: false job must refuse to run, before creating anything").
+		Step("run fails naming the manual: false dependency",
+			ComposeCmd("run", "--rm", "deploy").MayFail(),
+			StderrContains(`job "sensitive" is declared with manual: false, it cannot be triggered even as a dependency`),
+			ServiceNotCreated("db"))
+}

pkg/e2e/testdata/TestRunJobDependsOnManualFalseJob/compose.yaml

@@ -0,0 +1,23 @@
+services:
+  db:
+    image: alpine
+    init: true
+    command: sleep infinity
+jobs:
+  sensitive:
+    image: alpine
+    command: sh -c 'echo "sensitive ran"'
+    triggers:
+      manual: false
+      schedule:
+        - cron: "0 3 * * *"
+  deploy:
+    image: alpine
+    command: sh -c 'echo "deploy done"'
+    triggers:
+      manual: true
+    depends_on:
+      db:
+        condition: service_started
+      sensitive:
+        condition: service_completed_successfully