From 01e9febbead3afe49d10bea75b370263b2a749d6 Mon Sep 17 00:00:00 2001 From: bircni Date: Thu, 13 Aug 2026 09:36:41 +0200 Subject: [PATCH] fix(actions): keep github.event.inputs as strings for workflow_dispatch (#38899) `github.event.inputs` must mirror the raw `workflow_dispatch` payload, where GitHub keeps every input as a string. Only the separate `inputs` context preserves declared types, e.g. booleans. A previous fix coerced boolean inputs in the single map that fed both contexts, so `github.event.inputs.someBool` became a real boolean and comparisons like `== 'true'` stopped matching. `github.event.inputs` now stays string-only again. The `inputs` context used for server-side `if:` evaluation of needs-gated/matrix-deferred jobs re-coerces booleans independently, from the job's own workflow declaration, so that path keeps working correctly. Fixes https://github.com/go-gitea/gitea/issues/38896 --------- Co-authored-by: Zettat123 Co-authored-by: silverwind --- modules/actions/jobparser/model.go | 8 ++++- modules/util/util.go | 5 +++ routers/web/repo/actions/actions.go | 4 +++ services/actions/concurrency.go | 9 ++--- services/actions/context_test.go | 3 +- services/actions/helper.go | 27 +++++++++++++-- services/actions/helper_test.go | 17 ++++++++++ services/actions/job_emitter_test.go | 33 ++++++++++++++++++- services/actions/rerun.go | 6 +++- services/actions/workflow.go | 30 ++++++++--------- services/actions/workflow_test.go | 17 ++++++++++ .../actions/workflow_dispatch_inputs.tmpl | 2 +- tests/integration/actions_trigger_test.go | 8 ++--- 13 files changed, 138 insertions(+), 31 deletions(-) diff --git a/modules/actions/jobparser/model.go b/modules/actions/jobparser/model.go index 72326b53ec6..51a86863517 100644 --- a/modules/actions/jobparser/model.go +++ b/modules/actions/jobparser/model.go @@ -11,6 +11,7 @@ import ( "gitea.dev/actionslib/pkg/expreval" "gitea.dev/actionslib/pkg/exprparser" "gitea.dev/actionslib/pkg/model" + "gitea.dev/modules/util" "go.yaml.in/yaml/v4" ) @@ -34,6 +35,11 @@ func (w *SingleWorkflow) Job() (string, *Job) { return "", nil } +// WorkflowDispatchConfig returns the `on: workflow_dispatch` declaration, nil if there is none. +func (w *SingleWorkflow) WorkflowDispatchConfig() *model.WorkflowDispatch { + return (&model.Workflow{RawOn: w.RawOn}).WorkflowDispatchConfig() +} + func (w *SingleWorkflow) jobs() ([]string, []*Job, error) { ids, jobs, err := parseMappingNode[*Job](&w.RawJobs) if err != nil { @@ -294,7 +300,7 @@ func EvaluateConcurrency(rc *model.RawConcurrency, jobID string, job *Job, gitCt if evaluated.RawExpression != "" { return evaluated.RawExpression, false, nil } - return evaluated.Group, evaluated.CancelInProgress == "true", nil + return evaluated.Group, util.ParseYamlBool(evaluated.CancelInProgress), nil } func toGitContext(input map[string]any) *model.GithubContext { diff --git a/modules/util/util.go b/modules/util/util.go index 0184ec3da98..c8e1968bd45 100644 --- a/modules/util/util.go +++ b/modules/util/util.go @@ -26,6 +26,11 @@ func IsEmptyString(s string) bool { return len(strings.TrimSpace(s)) == 0 } +// ParseYamlBool parses YAML 1.2 boolean values into bool +func ParseYamlBool(s string) bool { + return s == "true" || s == "True" || s == "TRUE" +} + // NormalizeEOL will convert Windows (CRLF) and Mac (CR) EOLs to UNIX (LF) func NormalizeEOL(input []byte) []byte { var right, left, pos int diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index ccb4478f89b..ca6db691445 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -760,6 +760,10 @@ type WorkflowDispatchInput struct { Options []string `yaml:"options"` } +func (i WorkflowDispatchInput) IsDefaultTrue() bool { + return util.ParseYamlBool(i.Default) +} + type WorkflowDispatch struct { Inputs []WorkflowDispatchInput } diff --git a/services/actions/concurrency.go b/services/actions/concurrency.go index 5c342762253..3d9f860ff8d 100644 --- a/services/actions/concurrency.go +++ b/services/actions/concurrency.go @@ -10,6 +10,7 @@ import ( act_model "gitea.dev/actionslib/pkg/model" actions_model "gitea.dev/models/actions" "gitea.dev/modules/actions/jobparser" + "gitea.dev/modules/setting" "go.yaml.in/yaml/v4" ) @@ -17,6 +18,7 @@ import ( // EvaluateRunConcurrencyFillModel evaluates the expressions in a run-level (workflow) concurrency, // and fills the run attempt model with the evaluated `concurrency.group` and `concurrency.cancel-in-progress` values. // Workflow-level concurrency doesn't depend on the job outputs, so it can always be evaluated if there is no syntax error. +// Callers must resolve `inputs`, there is no job in scope here to read `on: workflow_dispatch` from. // See https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#concurrency func EvaluateRunConcurrencyFillModel(ctx context.Context, run *actions_model.ActionRun, attempt *actions_model.ActionRunAttempt, wfRawConcurrency *act_model.RawConcurrency, vars map[string]string, inputs map[string]any) error { if err := run.LoadAttributes(ctx); err != nil { @@ -26,11 +28,10 @@ func EvaluateRunConcurrencyFillModel(ctx context.Context, run *actions_model.Act actionsRunCtx := GenerateGiteaContext(ctx, run, attempt, nil) jobResults := map[string]*jobparser.JobResult{"": {}} if inputs == nil { - var err error - inputs, err = getWorkflowDispatchInputsFromRun(run) - if err != nil { - return fmt.Errorf("get inputs: %w", err) + if run.Event == "workflow_dispatch" { + setting.PanicInDevOrTesting("run %d: workflow_dispatch inputs must be resolved by the caller", run.ID) } + inputs = map[string]any{} } var err error diff --git a/services/actions/context_test.go b/services/actions/context_test.go index 9189440d63e..b61a605f8f6 100644 --- a/services/actions/context_test.go +++ b/services/actions/context_test.go @@ -37,12 +37,13 @@ func TestEvaluateRunConcurrency_RunIDFallback(t *testing.T) { expr := &act_model.RawConcurrency{ Group: "${{ github.workflow }}-${{ github.head_ref || github.run_id }}", - CancelInProgress: "true", + CancelInProgress: "True", } assert.NoError(t, EvaluateRunConcurrencyFillModel(ctx, runA, attemptA, expr, nil, nil)) assert.NoError(t, EvaluateRunConcurrencyFillModel(ctx, runB, attemptB, expr, nil, nil)) + assert.True(t, attemptA.ConcurrencyCancel) assert.Contains(t, attemptA.ConcurrencyGroup, "791") assert.Contains(t, attemptB.ConcurrencyGroup, "792") assert.NotEqual(t, attemptA.ConcurrencyGroup, attemptB.ConcurrencyGroup) diff --git a/services/actions/helper.go b/services/actions/helper.go index 01ec304d327..9f963cb5a4e 100644 --- a/services/actions/helper.go +++ b/services/actions/helper.go @@ -16,7 +16,8 @@ import ( "gitea.dev/modules/util" ) -func getWorkflowDispatchInputsFromRun(run *actions_model.ActionRun) (map[string]any, error) { +// dispatchInputsForJob types a top-level job's `inputs.*` from EventPayload, empty for other events. +func dispatchInputsForJob(run *actions_model.ActionRun, job *actions_model.ActionRunJob) (map[string]any, error) { if run.Event != "workflow_dispatch" { return map[string]any{}, nil } @@ -24,15 +25,37 @@ func getWorkflowDispatchInputsFromRun(run *actions_model.ActionRun) (map[string] if err := json.Unmarshal([]byte(run.EventPayload), &payload); err != nil { return nil, err } + if payload.Inputs == nil { + payload.Inputs = map[string]any{} // nil reads as "unresolved" in EvaluateRunConcurrencyFillModel + } + swf, _, err := jobparser.ParseRawSingleWorkflow(job.WorkflowPayload) + if err != nil { + return nil, util.NewInvalidArgumentErrorf("parse job %d workflow payload: %v", job.ID, err) + } + dispatch := swf.WorkflowDispatchConfig() + if dispatch == nil { // without it the values would silently stay untyped + return nil, util.NewInvalidArgumentErrorf("job %d payload declares no workflow_dispatch", job.ID) + } + coerceDispatchInputTypes(dispatch, payload.Inputs) return payload.Inputs, nil } +// dispatchInputsForRunJobs answers for the whole run, off any top-level job's workflow header. +func dispatchInputsForRunJobs(run *actions_model.ActionRun, jobs []*actions_model.ActionRunJob) (map[string]any, error) { + for _, job := range jobs { + if job.ParentJobID == 0 { + return dispatchInputsForJob(run, job) + } + } + return nil, fmt.Errorf("run %d: no top-level job to read the workflow_dispatch declaration from", run.ID) +} + // getInputsForJob returns the `inputs.*` top-level expression context for a job's evaluation. // - For top-level jobs, it falls back to the run's dispatch inputs (empty for non-dispatch events) // - For reusable workflow children (and nested callers), this is the direct parent caller's CallPayload.Inputs func getInputsForJob(ctx context.Context, run *actions_model.ActionRun, job *actions_model.ActionRunJob) (map[string]any, error) { if job.ParentJobID == 0 { - return getWorkflowDispatchInputsFromRun(run) + return dispatchInputsForJob(run, job) } caller, err := actions_model.GetRunJobByRunAndID(ctx, run.ID, job.ParentJobID) diff --git a/services/actions/helper_test.go b/services/actions/helper_test.go index 0635a73cd85..dae2a517bb6 100644 --- a/services/actions/helper_test.go +++ b/services/actions/helper_test.go @@ -63,6 +63,23 @@ jobs: } } +func TestDispatchInputsForRunJobs(t *testing.T) { + // a child carries the callee's `on: workflow_call`, so only a top-level job answers for the run + run := &actions_model.ActionRun{Event: "workflow_dispatch", EventPayload: `{"inputs":{"deploy":"true"}}`} + job := &actions_model.ActionRunJob{ + ID: 1, JobID: "deploy", + WorkflowPayload: []byte("on: {workflow_dispatch: {inputs: {deploy: {type: boolean}}}}\njobs:\n deploy:\n steps: [{run: echo}]\n"), + } + child := &actions_model.ActionRunJob{ + ID: 2, JobID: "called", ParentJobID: job.ID, + WorkflowPayload: []byte("on: workflow_call\njobs:\n called:\n steps: [{run: echo}]\n"), + } + + inputs, err := dispatchInputsForRunJobs(run, []*actions_model.ActionRunJob{child, job}) + require.NoError(t, err) + assert.Equal(t, true, inputs["deploy"]) +} + func TestPullRequestTargetBaseSHA(t *testing.T) { prPayload := func(baseSHA string) string { payload, err := json.Marshal(api.PullRequestPayload{ diff --git a/services/actions/job_emitter_test.go b/services/actions/job_emitter_test.go index cca5d5766ad..b852cab9094 100644 --- a/services/actions/job_emitter_test.go +++ b/services/actions/job_emitter_test.go @@ -12,6 +12,7 @@ import ( repo_model "gitea.dev/models/repo" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" + "gitea.dev/modules/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -31,6 +32,7 @@ jobs: func Test_jobStatusResolver_Resolve(t *testing.T) { tests := []struct { name string + run *actions_model.ActionRun // defaults to stubRun jobs actions_model.ActionJobList want map[int64]actions_model.Status }{ @@ -219,6 +221,34 @@ jobs: needs: job1 steps: - run: echo "should run, job1 failure is masked by continue-on-error" +`)}, + }, + want: map[int64]actions_model.Status{2: actions_model.StatusWaiting}, + }, + { + // a needs-gated job is evaluated server-side, so a mistyped input silently leaves it blocked + name: "`if` compares a workflow_dispatch boolean input", + run: &actions_model.ActionRun{ + TriggerUser: &user_model.User{}, Repo: &repo_model.Repository{}, + Event: "workflow_dispatch", + EventPayload: `{"inputs":{"deploy":"true"}}`, + }, + jobs: actions_model.ActionJobList{ + {ID: 1, JobID: "job1", Status: actions_model.StatusSuccess, Needs: []string{}}, + {ID: 2, JobID: "job2", Status: actions_model.StatusBlocked, Needs: []string{"job1"}, WorkflowPayload: []byte( + ` +on: + workflow_dispatch: + inputs: + deploy: + type: boolean +jobs: + job2: + runs-on: ubuntu-latest + needs: job1 + if: ${{ inputs.deploy == true && github.event.inputs.deploy == 'true' }} + steps: + - run: echo `)}, }, want: map[int64]actions_model.Status{2: actions_model.StatusWaiting}, @@ -232,6 +262,7 @@ jobs: // Each subtest gets a unique RunID / RunAttemptID so jobs from different subtests don't bleed into each other's FindTaskNeeds queries runID := int64(9001 + i) attemptID := int64(9001 + i) + run := util.IfZero(tt.run, stubRun) // Insert each test job (letting the DB assign IDs) and remember the testID -> dbID mapping so we can translate the expected map. idMap := make(map[int64]int64, len(tt.jobs)) @@ -240,7 +271,7 @@ jobs: j.ID = 0 j.RunID = runID j.RunAttemptID = attemptID - j.Run = stubRun + j.Run = run // The resolver evaluates Blocked jobs via evaluateJobIf, which needs a valid YAML payload; // supply a minimal one when the case didn't. diff --git a/services/actions/rerun.go b/services/actions/rerun.go index a8689df5d1d..9ec81f2edb7 100644 --- a/services/actions/rerun.go +++ b/services/actions/rerun.go @@ -211,7 +211,11 @@ func execRerunPlan(ctx context.Context, plan *rerunPlan) (*actions_model.ActionR if err := yaml.Unmarshal([]byte(plan.run.RawConcurrency), &rawConcurrency); err != nil { return nil, fmt.Errorf("unmarshal raw concurrency: %w", err) } - if err := EvaluateRunConcurrencyFillModel(ctx, plan.run, newAttempt, &rawConcurrency, vars, nil); err != nil { + inputs, err := dispatchInputsForRunJobs(plan.run, plan.templateJobs) + if err != nil { + return nil, err + } + if err := EvaluateRunConcurrencyFillModel(ctx, plan.run, newAttempt, &rawConcurrency, vars, inputs); err != nil { return nil, err } } diff --git a/services/actions/workflow.go b/services/actions/workflow.go index 4e33f58503f..39d98408072 100644 --- a/services/actions/workflow.go +++ b/services/actions/workflow.go @@ -6,6 +6,7 @@ package actions import ( "fmt" + "gitea.dev/actionslib/pkg/exprparser" "gitea.dev/actionslib/pkg/model" actions_model "gitea.dev/models/actions" "gitea.dev/models/perm" @@ -129,10 +130,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re return 0, fmt.Errorf("failed to unmarshal workflow content: %w", err) } // get inputs from post - workflow := &model.Workflow{ - RawOn: singleWorkflow.RawOn, - } - workflowDispatch := workflow.WorkflowDispatchConfig() + workflowDispatch := singleWorkflow.WorkflowDispatchConfig() if workflowDispatch == nil { return 0, util.ErrorWrapTranslatable( util.NewInvalidArgumentErrorf("workflow %q has no workflow_dispatch event trigger", workflowID), @@ -144,10 +142,6 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re if err = processInputs(workflowDispatch, inputsWithDefaults); err != nil { return 0, err } - // The dispatch callbacks fill boolean inputs as the strings "true"/"false". Normalize them to - // native JSON booleans so `type: boolean` inputs match GitHub, whose `inputs` context preserves - // booleans as booleans. Without this, a server-side needs-gated job `if: inputs.flag == true` - // evaluates against the string "true" and never matches, leaving the job blocked forever. coerceDispatchInputTypes(workflowDispatch, inputsWithDefaults) // ctx.Req.PostForm -> WorkflowDispatchPayload.Inputs -> ActionRun.EventPayload -> runner: ghc.Event @@ -157,7 +151,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re Workflow: workflowID, Ref: ref, Repository: convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm.AccessModeNone}), - Inputs: inputsWithDefaults, + Inputs: dispatchEventInputs(inputsWithDefaults), Sender: convert.ToUserWithAccessMode(ctx, doer, perm.AccessModeNone), } @@ -174,23 +168,27 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re return run.ID, nil } -// coerceDispatchInputTypes normalizes workflow_dispatch input values to the JSON types declared by -// the workflow. Only booleans are coerced, matching GitHub, whose `inputs` context "preserves -// Boolean values as Booleans instead of converting them to strings" while every other type stays a -// string. workflow_dispatch has no `number` type (its input types are string, choice, boolean and -// environment), so booleans are the complete set to coerce here. -// A value that is already a bool is left untouched, so the coercion is idempotent. +// coerceDispatchInputTypes types `inputs`, where boolean is the only non-string dispatch input type. func coerceDispatchInputTypes(dispatch *model.WorkflowDispatch, inputs map[string]any) { for name, cfg := range dispatch.Inputs { if cfg.Type != "boolean" { continue } if s, ok := inputs[name].(string); ok { - inputs[name] = s == "true" + inputs[name] = util.ParseYamlBool(s) } } } +// dispatchEventInputs stringifies the typed inputs for `github.event.inputs`. +func dispatchEventInputs(inputs map[string]any) map[string]any { + eventInputs := make(map[string]any, len(inputs)) + for name, value := range inputs { + eventInputs[name] = exprparser.CoerceToString(value) + } + return eventInputs +} + // resolveDispatchWorkflowContent returns the YAML for a dispatched workflow and records its source on the run. // - Repo-level: from the consumer's runTargetCommit. // - Scoped: from the source repo's default branch. diff --git a/services/actions/workflow_test.go b/services/actions/workflow_test.go index 09537b50532..d32e684cbbc 100644 --- a/services/actions/workflow_test.go +++ b/services/actions/workflow_test.go @@ -17,6 +17,8 @@ func TestCoerceDispatchInputTypes(t *testing.T) { "build_server": {Type: "boolean"}, "dry_run": {Type: "boolean"}, "already_bool": {Type: "boolean"}, + "yaml_true": {Type: "boolean"}, + "yaml_truthy": {Type: "boolean"}, "version": {Type: "string"}, }, } @@ -27,6 +29,9 @@ func TestCoerceDispatchInputTypes(t *testing.T) { "dry_run": "false", // already-native booleans are passed through unchanged (coercion is idempotent) "already_bool": true, + // source text of `default: True` and `default: yes`, only the former is a YAML 1.2 boolean + "yaml_true": "True", + "yaml_truthy": "yes", // non-boolean inputs must be left untouched "version": "1.2.3", } @@ -38,5 +43,17 @@ func TestCoerceDispatchInputTypes(t *testing.T) { assert.Equal(t, true, inputs["build_server"]) assert.Equal(t, false, inputs["dry_run"]) assert.Equal(t, true, inputs["already_bool"]) + assert.Equal(t, true, inputs["yaml_true"]) + assert.Equal(t, false, inputs["yaml_truthy"]) assert.Equal(t, "1.2.3", inputs["version"]) + + // `github.event.inputs` mirrors them as normalized strings + assert.Equal(t, map[string]any{ + "build_server": "true", + "dry_run": "false", + "already_bool": "true", + "yaml_true": "true", + "yaml_truthy": "false", + "version": "1.2.3", + }, dispatchEventInputs(inputs)) } diff --git a/templates/repo/actions/workflow_dispatch_inputs.tmpl b/templates/repo/actions/workflow_dispatch_inputs.tmpl index 13e976366ef..b1a2369a7f3 100644 --- a/templates/repo/actions/workflow_dispatch_inputs.tmpl +++ b/templates/repo/actions/workflow_dispatch_inputs.tmpl @@ -19,7 +19,7 @@ {{else if eq .Type "boolean"}} {{else if eq .Type "number"}} diff --git a/tests/integration/actions_trigger_test.go b/tests/integration/actions_trigger_test.go index 129e619d5e8..2ab5be5da58 100644 --- a/tests/integration/actions_trigger_test.go +++ b/tests/integration/actions_trigger_test.go @@ -1162,7 +1162,7 @@ jobs: assert.Contains(t, dispatchPayload.Inputs, "myinput3") assert.Equal(t, "val0", dispatchPayload.Inputs["myinput"]) assert.Equal(t, "def2", dispatchPayload.Inputs["myinput2"]) - assert.Equal(t, true, dispatchPayload.Inputs["myinput3"]) + assert.Equal(t, "true", dispatchPayload.Inputs["myinput3"]) }) } @@ -1342,7 +1342,7 @@ jobs: assert.Contains(t, dispatchPayload.Inputs, "myinput3") assert.Equal(t, "val0", dispatchPayload.Inputs["myinput"]) assert.Equal(t, "def2", dispatchPayload.Inputs["myinput2"]) - assert.Equal(t, true, dispatchPayload.Inputs["myinput3"]) + assert.Equal(t, "true", dispatchPayload.Inputs["myinput3"]) }) } @@ -1473,7 +1473,7 @@ jobs: assert.Contains(t, dispatchPayload.Inputs, "myinput3") assert.Equal(t, "val0", dispatchPayload.Inputs["myinput"]) assert.Equal(t, "def2", dispatchPayload.Inputs["myinput2"]) - assert.Equal(t, true, dispatchPayload.Inputs["myinput3"]) + assert.Equal(t, "true", dispatchPayload.Inputs["myinput3"]) }) } @@ -1670,7 +1670,7 @@ jobs: assert.Contains(t, dispatchPayload.Inputs, "myinput3") assert.Equal(t, "val0", dispatchPayload.Inputs["myinput"]) assert.Equal(t, "def2", dispatchPayload.Inputs["myinput2"]) - assert.Equal(t, true, dispatchPayload.Inputs["myinput3"]) + assert.Equal(t, "true", dispatchPayload.Inputs["myinput3"]) }) }