From aa7a29a157f99c39bceaa6dc4ffe067f5cfa33c0 Mon Sep 17 00:00:00 2001 From: bircni Date: Thu, 16 Jul 2026 21:33:26 +0000 Subject: [PATCH] fix: guard status-check functions against a nil job context (#1092) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `cancelled()`, `success()` and `failure()` expression functions dereferenced `Job.Status` unconditionally, so a nil `Job` context panicked the interpreter — which is why Gitea currently hands the runner a non-nil (empty) `JobContext` as a workaround. This routes all three through a `jobStatus()` helper that treats a nil `Job` as an empty status, keeping existing behaviour identical while removing the panic. Includes a regression test that panics on the old code and passes with the fix. Related: https://github.com/go-gitea/gitea/pull/38495 Reviewed-on: https://gitea.com/gitea/runner/pulls/1092 Reviewed-by: Zettat123 <39446+zettat123@noreply.gitea.com> --- act/exprparser/functions.go | 15 ++++++++++++--- act/exprparser/functions_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/act/exprparser/functions.go b/act/exprparser/functions.go index 294227c4..a4853bfb 100644 --- a/act/exprparser/functions.go +++ b/act/exprparser/functions.go @@ -274,8 +274,17 @@ func (impl *interperterImpl) jobSuccess() (bool, error) { //nolint:unparam // pr return true, nil } +// jobStatus returns the current job status, treating a nil Job context as an +// empty status so status-check functions never panic on a nil dereference. +func (impl *interperterImpl) jobStatus() string { + if impl.env.Job == nil { + return "" + } + return impl.env.Job.Status +} + func (impl *interperterImpl) stepSuccess() (bool, error) { //nolint:unparam // pre-existing issue from nektos/act - return impl.env.Job.Status == "success", nil + return impl.jobStatus() == "success", nil } func (impl *interperterImpl) jobFailure() (bool, error) { //nolint:unparam // pre-existing issue from nektos/act @@ -292,9 +301,9 @@ func (impl *interperterImpl) jobFailure() (bool, error) { //nolint:unparam // pr } func (impl *interperterImpl) stepFailure() (bool, error) { //nolint:unparam // pre-existing issue from nektos/act - return impl.env.Job.Status == "failure", nil + return impl.jobStatus() == "failure", nil } func (impl *interperterImpl) cancelled() (bool, error) { //nolint:unparam // pre-existing issue from nektos/act - return impl.env.Job.Status == "cancelled", nil + return impl.jobStatus() == "cancelled", nil } diff --git a/act/exprparser/functions_test.go b/act/exprparser/functions_test.go index a05dd24b..707bf6ab 100644 --- a/act/exprparser/functions_test.go +++ b/act/exprparser/functions_test.go @@ -254,3 +254,27 @@ func TestFunctionFormat(t *testing.T) { }) } } + +func TestStatusFunctionsNilJob(t *testing.T) { + // A nil Job context must not panic: the status-check functions should treat + // it as an empty status and return false rather than dereferencing nil. + env := &EvaluationEnvironment{} + + table := []struct { + input string + context string + name string + }{ + {"cancelled()", "job", "cancelled-nil-job"}, + {"success()", "step", "step-success-nil-job"}, + {"failure()", "step", "step-failure-nil-job"}, + } + + for _, tt := range table { + t.Run(tt.name, func(t *testing.T) { + output, err := NewInterpeter(env, Config{Context: tt.context}).Evaluate(tt.input, DefaultStatusCheckNone) + assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act + assert.Equal(t, false, output) + }) + } +}