From 7372b2dca6d1c030e57fbf115fc70b92c63bbced Mon Sep 17 00:00:00 2001 From: elijahr <153711+elijahr@users.noreply.github.com> Date: Wed, 10 Jun 2026 22:12:13 -0500 Subject: [PATCH] fix: merge --container-options with job container.options When a job declares a container: block, RunContext.options() returned only the job's YAML container.options and silently discarded the CLI --container-options flag. The flag was therefore honored only for jobs without a container: block, contradicting its documented purpose ("...for the job container without an options property in the job definition"). options() now appends the CLI --container-options after the job's interpolated container.options. act feeds the combined string to docker's single flag parser, so scalar flags from the CLI win conflicts (last value) and repeatable flags (--cap-add, --security-opt, -v, ...) accumulate -- matching Docker Compose merge semantics (scalars override, sequences concatenate). Jobs without a container: block and jobs without the CLI flag are unaffected. This lets operators inject local-only container options (e.g. --privileged --security-opt seccomp=unconfined for ThreadSanitizer, which needs personality(ADDR_NO_RANDOMIZE)) without editing the committed workflow, even when the job pins its own image via a container: block. Adds TestRunContextOptions covering merge ordering, the empty-options bug case, the no-container-block and no-flag no-regression cases, interpolation, and multi-token merge. --- pkg/runner/run_context.go | 3 +- pkg/runner/run_context_test.go | 75 ++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 5d4277123ed..ab59e78a0e4 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -784,7 +784,8 @@ func (rc *RunContext) options(ctx context.Context) string { job := rc.Run.Job() c := job.Container() if c != nil { - return rc.ExprEval.Interpolate(ctx, c.Options) + // Merge job container options with the CLI --container-options flag; the CLI value is appended last so docker's single flag parse lets it win scalar conflicts (repeatable flags accumulate). + return strings.TrimSpace(rc.ExprEval.Interpolate(ctx, c.Options) + " " + rc.Config.ContainerOptions) } return rc.Config.ContainerOptions diff --git a/pkg/runner/run_context_test.go b/pkg/runner/run_context_test.go index de725b037e6..dc7c67a6b5f 100644 --- a/pkg/runner/run_context_test.go +++ b/pkg/runner/run_context_test.go @@ -337,6 +337,81 @@ func TestRunContext_GetBindsAndMounts(t *testing.T) { }) } +func TestRunContextOptions(t *testing.T) { + ctx := context.Background() + + // newRC builds a RunContext for a single job with the given encoded + // container map (nil => no container block) and CLI container-options + // flag, with an initialized expression evaluator and a populated env. + // Only rc.Env is set: GetEnv() returns rc.Env verbatim when non-nil and + // only consults Config.Env when rc.Env == nil, so setting Config.Env here + // would be dead code. + newRC := func(t *testing.T, container map[string]string, cliOptions string) *RunContext { + t.Helper() + job := &model.Job{} + if container != nil { + assert.NoError(t, job.RawContainer.Encode(container)) + } + rc := &RunContext{ + Name: "TestRCName", + Run: &model.Run{ + JobID: "job1", + Workflow: &model.Workflow{ + Name: "TestWorkflowName", + Jobs: map[string]*model.Job{"job1": job}, + }, + }, + Config: &Config{ + ContainerOptions: cliOptions, + }, + Env: map[string]string{"OPT": "--cpus=2"}, + } + rc.ExprEval = rc.NewExpressionEvaluator(ctx) + return rc + } + + t.Run("merge_cli_last", func(t *testing.T) { + // case (a): container block with non-empty YAML options + CLI flag set. + // Exact equality locks the full merged output: YAML first, CLI last, single space. + rc := newRC(t, map[string]string{"image": "node:16", "options": "--cpus=1"}, "--privileged") + assert.Equal(t, "--cpus=1 --privileged", rc.options(ctx)) + }) + + t.Run("bug_case_empty_yaml_options", func(t *testing.T) { + // case (b): container block, no YAML options, CLI flag set => returns CLI exactly (no leading space). + rc := newRC(t, map[string]string{"image": "node:16"}, "--privileged --security-opt seccomp=unconfined") + assert.Equal(t, "--privileged --security-opt seccomp=unconfined", rc.options(ctx)) + }) + + t.Run("no_container_block", func(t *testing.T) { + // case (c): no container block, CLI flag set => returns CLI (else branch, unchanged). + rc := newRC(t, nil, "--privileged") + assert.Equal(t, "--privileged", rc.options(ctx)) + }) + + t.Run("no_cli_flag_no_regression", func(t *testing.T) { + // case (d): CLI flag unset, non-empty YAML options => YAML exactly, no trailing space. + rc := newRC(t, map[string]string{"image": "node:16", "options": "--cpus=1"}, "") + assert.Equal(t, "--cpus=1", rc.options(ctx)) + }) + + t.Run("interpolation", func(t *testing.T) { + // case (e): YAML options contains a ${{ }} expression referencing a populated env value. + // Green-mirage guard: exact equality locks the RESOLVED value (--cpus=2) merged + // with the CLI flag last, single space — not merely the absence of "${{". + rc := newRC(t, map[string]string{"image": "node:16", "options": "${{ env.OPT }}"}, "--privileged") + assert.Equal(t, "--cpus=2 --privileged", rc.options(ctx)) + }) + + t.Run("multi_token", func(t *testing.T) { + // case (f): multiple YAML tokens + multiple CLI tokens. Locks separator + // handling across tokens: YAML tokens preserved, CLI tokens appended last, + // joined by a single space with no doubling or trailing space. + rc := newRC(t, map[string]string{"image": "node:16", "options": "--cpus=1 --memory=2g"}, "--privileged --rm") + assert.Equal(t, "--cpus=1 --memory=2g --privileged --rm", rc.options(ctx)) + }) +} + func TestGetGitHubContext(t *testing.T) { log.SetLevel(log.DebugLevel)