From d8e179f28ffff46885ccb009837a6b55d9bf2c80 Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 20 Aug 2026 11:16:33 +0200 Subject: [PATCH] fix: resolve YAML anchors and aliases in Actions workflows (#38984) (#38996) Backport https://github.com/go-gitea/gitea/pull/38984 Workflows using YAML anchors are rejected as invalid, because a workflow is split into one document per job and an alias whose anchor lands in another job's document no longer resolves. Node walkers such as `on:` parsing have no alias case either. Aliases are now expanded once, right after the workflow is parsed and before anything reads or splits it, bounded like GitHub's parser so nested aliases cannot expand without limit. Merge keys stay unsupported, as they are upstream. Fixes: https://github.com/go-gitea/gitea/issues/38983 --- modules/actions/jobparser/aliases.go | 120 +++++++++++++++++++++ modules/actions/jobparser/aliases_test.go | 85 +++++++++++++++ modules/actions/jobparser/jobparser.go | 19 ++-- modules/actions/jobparser/model.go | 8 +- modules/actions/jobparser/workflow_call.go | 2 +- modules/actions/workflows.go | 3 +- routers/web/repo/actions/actions.go | 6 +- services/actions/notifier_helper.go | 6 +- services/actions/permission_parser.go | 14 +-- services/actions/workflow.go | 7 +- services/convert/convert.go | 6 +- 11 files changed, 238 insertions(+), 38 deletions(-) create mode 100644 modules/actions/jobparser/aliases.go create mode 100644 modules/actions/jobparser/aliases_test.go diff --git a/modules/actions/jobparser/aliases.go b/modules/actions/jobparser/aliases.go new file mode 100644 index 00000000000..b7afbe65532 --- /dev/null +++ b/modules/actions/jobparser/aliases.go @@ -0,0 +1,120 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package jobparser + +import ( + "errors" + "io" + + "gitea.com/gitea/runner/act/model" + "go.yaml.in/yaml/v4" +) + +// maxExpandedNodes bounds how many nodes alias expansion may create. go-yaml's own alias guard does +// not cover us: it only counts while decoding into values, and a workflow is kept as raw yaml.Nodes. +const maxExpandedNodes = 50000 + +var errTooManyYamlNodes = errors.New("maximum YAML nodes exceeded") + +// ReadWorkflow decodes a workflow file with its aliases expanded. Callers inspect the workflow's +// raw nodes by kind, and an alias is a kind none of them expect. +func ReadWorkflow(content []byte) (*model.Workflow, error) { + doc, err := resolveYamlAliases(content) + if err != nil { + return nil, err + } + return readWorkflowDoc(doc) +} + +func readWorkflowDoc(doc *yaml.Node) (*model.Workflow, error) { + if doc.Kind == 0 { + return nil, io.EOF // what a yaml decoder reports for an empty file + } + w := new(model.Workflow) + return w, doc.Decode(w) +} + +// decodeResolved is yaml.Unmarshal with aliases expanded first. +func decodeResolved(content []byte, out any) error { + doc, err := resolveYamlAliases(content) + if err != nil { + return err + } + return decodeYamlDoc(doc, out) +} + +func decodeYamlDoc(doc *yaml.Node, out any) error { + if doc.Kind == 0 { + return nil // an empty document, as yaml.Unmarshal treats it + } + return doc.Decode(out) +} + +// resolveYamlAliases parses content and replaces every alias with a copy of the node its anchor names. +func resolveYamlAliases(content []byte) (*yaml.Node, error) { + doc := &yaml.Node{} + if err := yaml.Unmarshal(content, doc); err != nil { + return nil, err + } + budget := maxExpandedNodes + return doc, expandAliases(doc, &budget) +} + +// expandAliases replaces node's alias descendants in place. +func expandAliases(node *yaml.Node, budget *int) error { + node.Anchor = "" // a name for a node, not part of the workflow: keep it out of the payloads + if err := rejectMergeKeys(node); err != nil { + return err + } + for i, child := range node.Content { + if child.Kind != yaml.AliasNode { + if err := expandAliases(child, budget); err != nil { + return err + } + continue + } + copied, err := copyExpanded(child.Alias, budget) + if err != nil { + return err + } + node.Content[i] = copied + } + return nil +} + +// copyExpanded deep copies a node expandAliases already expanded and validated, since an anchor is +// declared before the alias naming it. An anchor aliased from inside itself is the exception, and +// recurses here until it exhausts budget. +func copyExpanded(node *yaml.Node, budget *int) (*yaml.Node, error) { + if *budget--; *budget < 0 { + return nil, errTooManyYamlNodes + } + if node.Kind == yaml.AliasNode { + return copyExpanded(node.Alias, budget) + } + + copied := *node + copied.Content = make([]*yaml.Node, len(node.Content)) + for i, child := range node.Content { + child, err := copyExpanded(child, budget) + if err != nil { + return nil, err + } + copied.Content[i] = child + } + return &copied, nil +} + +// rejectMergeKeys refuses `<<: *anchor`, same as GitHub does +func rejectMergeKeys(node *yaml.Node) error { + if node.Kind != yaml.MappingNode { + return nil + } + for i := 0; i < len(node.Content)-1; i += 2 { + if node.Content[i].Tag == "!!merge" { + return errors.New("merge keys (`<<`) are not supported, alias the whole value instead") + } + } + return nil +} diff --git a/modules/actions/jobparser/aliases_test.go b/modules/actions/jobparser/aliases_test.go new file mode 100644 index 00000000000..71f385e9e52 --- /dev/null +++ b/modules/actions/jobparser/aliases_test.go @@ -0,0 +1,85 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package jobparser + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseResolvesAliases(t *testing.T) { + got, err := Parse([]byte(`on: push +env: &common_env + SHARED: "1" +jobs: + a: + runs-on: linux + env: *common_env + steps: &common_steps [{run: echo hi}] + b: + runs-on: linux + env: *common_env + steps: *common_steps +`)) + require.NoError(t, err) + require.Len(t, got, 2) + + for _, workflow := range got { + _, job := workflow.Job() + var env map[string]string + require.NoError(t, job.Env.Decode(&env)) + assert.Equal(t, map[string]string{"SHARED": "1"}, env) + require.Len(t, job.Steps, 1) + + payload, err := workflow.Marshal() + require.NoError(t, err) + assert.NotContains(t, string(payload), "common_") + } +} + +func TestParseRejectsAliases(t *testing.T) { + job := func(body string) []byte { + return []byte("on: push\njobs:\n a:\n runs-on: linux\n" + body) + } + + for _, tt := range []struct { + name, wantErr string + content []byte + }{ + { + name: "nested aliases exceed the node limit", + content: []byte(`on: push +x0: &x0 [1, 2, 3, 4, 5, 6, 7, 8, 9] +x1: &x1 [*x0, *x0, *x0, *x0, *x0, *x0, *x0, *x0, *x0] +x2: &x2 [*x1, *x1, *x1, *x1, *x1, *x1, *x1, *x1, *x1] +x3: &x3 [*x2, *x2, *x2, *x2, *x2, *x2, *x2, *x2, *x2] +x4: &x4 [*x3, *x3, *x3, *x3, *x3, *x3, *x3, *x3, *x3] +jobs: {a: {runs-on: linux, steps: [{run: echo}]}} +`), + wantErr: "maximum YAML nodes exceeded", + }, + { + name: "anchor aliased from inside itself", + content: job(" steps: &s [{run: echo}, *s]\n"), + wantErr: "maximum YAML nodes exceeded", + }, + { + name: "merge key", + content: job(" env: &e {X: \"1\"}\n container:\n image: alpine\n env:\n <<: *e\n"), + wantErr: "merge keys (`<<`) are not supported", + }, + { + name: "alias before its anchor", + content: job(" env: *e\n container: {image: alpine, env: &e {X: \"1\"}}\n"), + wantErr: "unknown anchor 'e' referenced", + }, + } { + t.Run(tt.name, func(t *testing.T) { + _, err := Parse(tt.content) + require.ErrorContains(t, err, tt.wantErr) + }) + } +} diff --git a/modules/actions/jobparser/jobparser.go b/modules/actions/jobparser/jobparser.go index faa660441e3..a4719967a37 100644 --- a/modules/actions/jobparser/jobparser.go +++ b/modules/actions/jobparser/jobparser.go @@ -4,7 +4,6 @@ package jobparser import ( - "bytes" "fmt" "slices" "sort" @@ -17,14 +16,21 @@ import ( ) func Parse(content []byte, options ...ParseOption) ([]*SingleWorkflow, error) { - origin, err := model.ReadWorkflow(bytes.NewReader(content)) + // The workflow is split into one document per job below, which would strand an alias whose + // anchor lands in another one. + doc, err := resolveYamlAliases(content) if err != nil { - return nil, fmt.Errorf("model.ReadWorkflow: %w", err) + return nil, fmt.Errorf("resolve aliases: %w", err) + } + + origin, err := readWorkflowDoc(doc) + if err != nil { + return nil, fmt.Errorf("read workflow: %w", err) } workflow := &SingleWorkflow{} - if err := yaml.Unmarshal(content, workflow); err != nil { - return nil, fmt.Errorf("yaml.Unmarshal: %w", err) + if err := decodeYamlDoc(doc, workflow); err != nil { + return nil, fmt.Errorf("decode workflow: %w", err) } pc := &parseContext{} @@ -156,9 +162,6 @@ func validateMatrixFilters(job *model.Job) error { entries = value.Content } for _, entry := range entries { - if entry.Kind == yaml.AliasNode { - entry = entry.Alias - } if entry.Kind != yaml.MappingNode { return fmt.Errorf("matrix %s must be a list of mappings", name) } diff --git a/modules/actions/jobparser/model.go b/modules/actions/jobparser/model.go index b51844d47ff..501633972b5 100644 --- a/modules/actions/jobparser/model.go +++ b/modules/actions/jobparser/model.go @@ -257,9 +257,11 @@ func (evt *Event) Inputs() []WorkflowDispatchInput { } func ReadWorkflowRawConcurrency(content []byte) (*model.RawConcurrency, error) { - w := new(model.Workflow) - err := yaml.NewDecoder(bytes.NewReader(content)).Decode(w) - return w.RawConcurrency, err + w, err := ReadWorkflow(content) + if err != nil { + return nil, err + } + return w.RawConcurrency, nil } func EvaluateConcurrency(rc *model.RawConcurrency, jobID string, job *Job, gitCtx map[string]any, results map[string]*JobResult, vars map[string]string, inputs map[string]any) (string, bool, error) { diff --git a/modules/actions/jobparser/workflow_call.go b/modules/actions/jobparser/workflow_call.go index 6f725dd175d..b5722999601 100644 --- a/modules/actions/jobparser/workflow_call.go +++ b/modules/actions/jobparser/workflow_call.go @@ -62,7 +62,7 @@ func ParseWorkflowCallSpec(content []byte) (*WorkflowCallSpec, error) { var doc struct { On yaml.Node `yaml:"on"` } - if err := yaml.Unmarshal(content, &doc); err != nil { + if err := decodeResolved(content, &doc); err != nil { return nil, fmt.Errorf("parse workflow yaml: %w", err) } diff --git a/modules/actions/workflows.go b/modules/actions/workflows.go index a4765d33b85..d993f8d20fd 100644 --- a/modules/actions/workflows.go +++ b/modules/actions/workflows.go @@ -4,7 +4,6 @@ package actions import ( - "bytes" "fmt" "path" "slices" @@ -120,7 +119,7 @@ func GetContentFromEntry(entry *git.TreeEntry) ([]byte, error) { } func GetEventsFromContent(content []byte) ([]*jobparser.Event, error) { - workflow, err := model.ReadWorkflow(bytes.NewReader(content)) + workflow, err := jobparser.ReadWorkflow(content) if err != nil { return nil, err } diff --git a/routers/web/repo/actions/actions.go b/routers/web/repo/actions/actions.go index b78225a0431..2a34b49f67b 100644 --- a/routers/web/repo/actions/actions.go +++ b/routers/web/repo/actions/actions.go @@ -4,7 +4,6 @@ package actions import ( - "bytes" stdCtx "context" "errors" "fmt" @@ -19,6 +18,7 @@ import ( repo_model "gitea.dev/models/repo" "gitea.dev/models/unit" "gitea.dev/modules/actions" + "gitea.dev/modules/actions/jobparser" "gitea.dev/modules/container" "gitea.dev/modules/git" "gitea.dev/modules/log" @@ -199,7 +199,7 @@ func prepareWorkflowTemplate(ctx *context.Context, commit *git.Commit) (workflow ctx.ServerError("GetContentFromEntry", err) return nil, "" } - wf, err := act_model.ReadWorkflow(bytes.NewReader(content)) + wf, err := jobparser.ReadWorkflow(content) if err != nil { workflow.ErrMsg = ctx.Locale.TrString("actions.runs.invalid_workflow_helper", err.Error()) workflows = append(workflows, workflow) @@ -369,7 +369,7 @@ func loadScopedWorkflowModel(ctx *context.Context, repo *repo_model.Repository, if content == nil { return nil // the workflow does not exist on the source's default branch } - wf, err := act_model.ReadWorkflow(bytes.NewReader(content)) + wf, err := jobparser.ReadWorkflow(content) if err != nil { return nil } diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 02940c7e819..94144d80f80 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -4,7 +4,6 @@ package actions import ( - "bytes" "context" "fmt" "slices" @@ -19,6 +18,7 @@ import ( unit_model "gitea.dev/models/unit" user_model "gitea.dev/models/user" actions_module "gitea.dev/modules/actions" + "gitea.dev/modules/actions/jobparser" "gitea.dev/modules/container" "gitea.dev/modules/git" "gitea.dev/modules/gitrepo" @@ -28,8 +28,6 @@ import ( api "gitea.dev/modules/structs" webhook_module "gitea.dev/modules/webhook" "gitea.dev/services/convert" - - "gitea.com/gitea/runner/act/model" ) type methodCtxKeyType struct{} @@ -555,7 +553,7 @@ func handleSchedules( crons := make([]*actions_model.ActionSchedule, 0, len(detectedWorkflows)) for _, dwf := range detectedWorkflows { // Check cron job condition. Only working in default branch - workflow, err := model.ReadWorkflow(bytes.NewReader(dwf.Content)) + workflow, err := jobparser.ReadWorkflow(dwf.Content) if err != nil { log.Error("ReadWorkflow: %v", err) continue diff --git a/services/actions/permission_parser.go b/services/actions/permission_parser.go index a94bc466c23..2e35815e06d 100644 --- a/services/actions/permission_parser.go +++ b/services/actions/permission_parser.go @@ -40,17 +40,13 @@ func parseRawPermissionsExplicit(rawPerms *yaml.Node) *repo_model.ActionsTokenPe return nil } - // Unwrap DocumentNode and resolve AliasNode + // Unwrap DocumentNode node := rawPerms - for node.Kind == yaml.DocumentNode || node.Kind == yaml.AliasNode { - if node.Kind == yaml.DocumentNode { - if len(node.Content) == 0 { - return nil - } - node = node.Content[0] - } else { - node = node.Alias + for node.Kind == yaml.DocumentNode { + if len(node.Content) == 0 { + return nil } + node = node.Content[0] } if node.Kind == yaml.ScalarNode && node.Value == "" { diff --git a/services/actions/workflow.go b/services/actions/workflow.go index f4ea158fb5f..f2ba315a9da 100644 --- a/services/actions/workflow.go +++ b/services/actions/workflow.go @@ -23,7 +23,6 @@ import ( "gitea.dev/services/convert" "gitea.com/gitea/runner/act/model" - "go.yaml.in/yaml/v4" ) func EnableOrDisableWorkflow(ctx *context.APIContext, workflowID string, isEnable bool) error { @@ -125,12 +124,12 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re return 0, err } - singleWorkflow := &jobparser.SingleWorkflow{} - if err := yaml.Unmarshal(content, singleWorkflow); err != nil { + workflow, err := jobparser.ReadWorkflow(content) + if err != nil { return 0, fmt.Errorf("failed to unmarshal workflow content: %w", err) } // get inputs from post - workflowDispatch := singleWorkflow.WorkflowDispatchConfig() + workflowDispatch := workflow.WorkflowDispatchConfig() if workflowDispatch == nil { return 0, util.ErrorWrapTranslatable( util.NewInvalidArgumentErrorf("workflow %q has no workflow_dispatch event trigger", workflowID), diff --git a/services/convert/convert.go b/services/convert/convert.go index 35ace95d53c..c498c06d949 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -5,7 +5,6 @@ package convert import ( - "bytes" "context" "errors" "fmt" @@ -28,6 +27,7 @@ import ( "gitea.dev/models/unit" user_model "gitea.dev/models/user" "gitea.dev/modules/actions" + "gitea.dev/modules/actions/jobparser" "gitea.dev/modules/container" "gitea.dev/modules/git" "gitea.dev/modules/gitrepo" @@ -39,8 +39,6 @@ import ( webhook_module "gitea.dev/modules/webhook" asymkey_service "gitea.dev/services/asymkey" "gitea.dev/services/gitdiff" - - "gitea.com/gitea/runner/act/model" ) // ToEmail convert models.EmailAddress to api.Email @@ -557,7 +555,7 @@ func getActionWorkflowEntry(ctx context.Context, repo *repo_model.Repository, co content, err := actions.GetContentFromEntry(entry) name := entry.Name() if err == nil { - workflow, err := model.ReadWorkflow(bytes.NewReader(content)) + workflow, err := jobparser.ReadWorkflow(content) if err == nil { // Only use the name when specified in the workflow file if workflow.Name != "" {