From 5251cd572fdd2289955bfe7d687a226ed05b7edf Mon Sep 17 00:00:00 2001 From: silverwind Date: Sat, 17 Oct 2026 05:15:42 +0200 Subject: [PATCH] fix(actions): keep folded block scalars intact across serialization (#39787) (#39817) Backport https://github.com/go-gitea/gitea/pull/39787 by @lunny, including the go-yaml bump. The runner counterpart is https://gitea.com/gitea/runner/pulls/1321. Folded (`>`) scalars in workflows gained blank lines and had spaces turned into newlines every time Gitea re-serialized a job. - go-yaml main fixes the accumulating blank lines (https://github.com/yaml/go-yaml/pull/366) and the block scalar indentation indicators (https://github.com/yaml/go-yaml/pull/414), so `BlockSafeString` goes away - The job encoder no longer wraps lines, because go-yaml still wraps more-indented lines of folded scalars and turns a space into a newline Fixes https://github.com/go-gitea/gitea/issues/39775 --- go.mod | 2 +- go.sum | 4 +- models/actions/task.go | 4 +- models/actions/task_test.go | 4 +- modules/actions/jobparser/model.go | 71 ++++++------------- modules/actions/jobparser/roundtrip_test.go | 38 +++++++++- .../testdata/prefixed_newline.out.yaml | 8 ++- 7 files changed, 72 insertions(+), 59 deletions(-) diff --git a/go.mod b/go.mod index 69245fc47e2..e9f3dd33efc 100644 --- a/go.mod +++ b/go.mod @@ -94,7 +94,7 @@ require ( github.com/yuin/goldmark v1.8.6 github.com/yuin/goldmark-highlighting/v2 v2.0.0-20230729083705-37449abec8cc gitlab.com/gitlab-org/api/client-go/v3 v3.13.0 - go.yaml.in/yaml/v4 v4.0.0-rc.6 + go.yaml.in/yaml/v4 v4.0.0-rc.6.0.20261013184558-04dd63131525 golang.org/x/crypto v0.57.0 golang.org/x/image v0.46.0 golang.org/x/mod v0.41.0 diff --git a/go.sum b/go.sum index 6c37a1f15d3..23496c92ef7 100644 --- a/go.sum +++ b/go.sum @@ -672,8 +672,8 @@ go.yaml.in/yaml/v2 v2.4.4 h1:tuyd0P+2Ont/d6e2rl3be67goVK4R6deVxCUX5vyPaQ= go.yaml.in/yaml/v2 v2.4.4/go.mod h1:gMZqIpDtDqOfM0uNfy0SkpRhvUryYH0Z6wdMYcacYXQ= go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg= -go.yaml.in/yaml/v4 v4.0.0-rc.6 h1:1h7H1ohdUh93/FyE4YaDa1Zh64K6VVbjF4K6WUxMtH4= -go.yaml.in/yaml/v4 v4.0.0-rc.6/go.mod h1:aZqd9kCMsGL7AuUv/m/PvWLdg5sjJsZ4oHDEnfPPfY0= +go.yaml.in/yaml/v4 v4.0.0-rc.6.0.20261013184558-04dd63131525 h1:FtzHH3s777Nx50YcPAcodAPm1mNakc228nMYbr3f+90= +go.yaml.in/yaml/v4 v4.0.0-rc.6.0.20261013184558-04dd63131525/go.mod h1:aZqd9kCMsGL7AuUv/m/PvWLdg5sjJsZ4oHDEnfPPfY0= go4.org v0.0.0-20260112195520-a5071408f32f h1:ziUVAjmTPwQMBmYR1tbdRFJPtTcQUI12fH9QQjfb0Sw= go4.org v0.0.0-20260112195520-a5071408f32f/go.mod h1:ZRJnO5ZI4zAwMFp+dS1+V6J6MSyAowhRqAE+DPa1Xp0= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= diff --git a/models/actions/task.go b/models/actions/task.go index 838db470f8a..418379c44c1 100644 --- a/models/actions/task.go +++ b/models/actions/task.go @@ -236,13 +236,13 @@ func GetRunningTaskByToken(ctx context.Context, token string) (*ActionTask, erro func makeTaskStepDisplayName(step *jobparser.Step, limit int) (name string) { if step.Name != "" { - name = string(step.Name) // the step has an explicit name + name = step.Name // the step has an explicit name } else { // for unnamed step, its "String()" method tries to get a display name by its "name", "uses", // "run" or "id" (last fallback), we add the "Run " prefix for unnamed steps for better display // for multi-line "run" scripts, only use the first line to match GitHub's behavior // https://github.com/actions/runner/blob/66800900843747f37591b077091dd2c8cf2c1796/src/Runner.Worker/Handlers/ScriptHandler.cs#L45-L58 - runStr, _, _ := strings.Cut(strings.TrimSpace(string(step.Run)), "\n") + runStr, _, _ := strings.Cut(strings.TrimSpace(step.Run), "\n") name = "Run " + util.IfZero(strings.TrimSpace(runStr), step.String()) } return util.EllipsisDisplayString(name, limit) // database column has a length limit diff --git a/models/actions/task_test.go b/models/actions/task_test.go index ca872089c49..d66ec1a3fc6 100644 --- a/models/actions/task_test.go +++ b/models/actions/task_test.go @@ -94,14 +94,14 @@ func TestMakeTaskStepDisplayName(t *testing.T) { { name: "very long name truncated", jobStep: &jobparser.Step{ - Name: jobparser.BlockSafeString(strings.Repeat("a", 300)), + Name: strings.Repeat("a", 300), }, expected: strings.Repeat("a", 252) + "…", }, { name: "very long run truncated", jobStep: &jobparser.Step{ - Run: jobparser.BlockSafeString(strings.Repeat("a", 300)), + Run: strings.Repeat("a", 300), }, expected: "Run " + strings.Repeat("a", 248) + "…", }, diff --git a/modules/actions/jobparser/model.go b/modules/actions/jobparser/model.go index a5b7b1d6897..2473c6e17a0 100644 --- a/modules/actions/jobparser/model.go +++ b/modules/actions/jobparser/model.go @@ -4,7 +4,6 @@ package jobparser import ( - "bytes" "errors" "fmt" "strings" @@ -65,44 +64,31 @@ func (w *SingleWorkflow) SetJob(id string, job *Job) error { m := map[string]*Job{ id: job, } - var buf bytes.Buffer - encoder := yaml.NewEncoder(&buf) - encoder.SetIndent(2) - if err := encoder.Encode(m); err != nil { + out, err := marshalYAML(m) + if err != nil { return err } - encoder.Close() node := yaml.Node{} - if err := yaml.Unmarshal(buf.Bytes(), &node); err != nil { + if err := yaml.Unmarshal(out, &node); err != nil { return err } if len(node.Content) != 1 || node.Content[0].Kind != yaml.MappingNode { - return fmt.Errorf("can not set job: %s", buf.String()) + return fmt.Errorf("can not set job: %s", out) } w.RawJobs = *node.Content[0] return nil } func (w *SingleWorkflow) Marshal() ([]byte, error) { - // Encode with the same indentation SetJob uses (2). yaml.Marshal's default - // indentation (4) makes the encoder emit multi-line block scalars (e.g. a - // `run:` step that begins with blank lines) with a wrong explicit indentation - // indicator (`run: |4`) that then fails to re-parse, which silently strands - // the job during concurrency evaluation. Keeping both encoders at indent 2 - // makes the serialized single workflow round-trip. - var buf bytes.Buffer - enc := yaml.NewEncoder(&buf) - enc.SetIndent(2) payload := *w payload.RunName = "" // already interpolated into the run title, a runner would parse it as a template again - if err := enc.Encode(&payload); err != nil { - return nil, err - } - if err := enc.Close(); err != nil { - return nil, err - } - return buf.Bytes(), nil + return marshalYAML(&payload) +} + +// marshalYAML never wraps lines, go-yaml wraps more-indented lines of folded scalars until https://github.com/yaml/go-yaml/pull/441 is released. +func marshalYAML(v any) ([]byte, error) { + return yaml.Dump(v, yaml.WithV3Defaults(), yaml.WithIndent(2), yaml.WithLineWidth(-1)) } type Job struct { @@ -188,29 +174,18 @@ func (j *Job) DisplayName() string { return util.EllipsisDisplayString(unescapeExpressions(j.Name), 255) } -// BlockSafeString works around https://github.com/yaml/go-yaml/issues/399, quoting a value whose -// leading newline would cost a literal block scalar its indentation indicator. -type BlockSafeString string - -func (s BlockSafeString) MarshalYAML() (any, error) { - if !strings.HasPrefix(string(s), "\n") { - return string(s), nil - } - return &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Style: yaml.DoubleQuotedStyle, Value: string(s)}, nil -} - type Step struct { - ID string `yaml:"id,omitempty"` - If yaml.Node `yaml:"if,omitempty"` - Name BlockSafeString `yaml:"name,omitempty"` - Uses string `yaml:"uses,omitempty"` - Run BlockSafeString `yaml:"run,omitempty"` - WorkingDirectory string `yaml:"working-directory,omitempty"` - Shell string `yaml:"shell,omitempty"` - Env yaml.Node `yaml:"env,omitempty"` - With yaml.Node `yaml:"with,omitempty"` - RawContinueOnError yaml.Node `yaml:"continue-on-error,omitempty"` // raw: the runner evaluates it with the steps context - TimeoutMinutes string `yaml:"timeout-minutes,omitempty"` + ID string `yaml:"id,omitempty"` + If yaml.Node `yaml:"if,omitempty"` + Name string `yaml:"name,omitempty"` + Uses string `yaml:"uses,omitempty"` + Run string `yaml:"run,omitempty"` + WorkingDirectory string `yaml:"working-directory,omitempty"` + Shell string `yaml:"shell,omitempty"` + Env yaml.Node `yaml:"env,omitempty"` + With yaml.Node `yaml:"with,omitempty"` + RawContinueOnError yaml.Node `yaml:"continue-on-error,omitempty"` // raw: the runner evaluates it with the steps context + TimeoutMinutes string `yaml:"timeout-minutes,omitempty"` } // UnmarshalYAML canonicalizes booleans like continue-on-error @@ -232,9 +207,9 @@ func (s *Step) String() string { } return (&model.Step{ ID: s.ID, - Name: string(s.Name), + Name: s.Name, Uses: s.Uses, - Run: string(s.Run), + Run: s.Run, }).String() } diff --git a/modules/actions/jobparser/roundtrip_test.go b/modules/actions/jobparser/roundtrip_test.go index cdc26f02b60..43fa9641d14 100644 --- a/modules/actions/jobparser/roundtrip_test.go +++ b/modules/actions/jobparser/roundtrip_test.go @@ -48,7 +48,7 @@ jobs: _, origJob := sws[0].Job() require.Len(t, origJob.Steps, 1) const wantRun = "\n\necho start\necho done\n" - require.Equal(t, wantRun, string(origJob.Steps[0].Run)) + require.Equal(t, wantRun, origJob.Steps[0].Run) payload, err := sws[0].Marshal() require.NoError(t, err) @@ -63,7 +63,7 @@ jobs: // the round-trip must preserve the run block byte-for-byte _, gotJob := roundTripped[0].Job() require.Len(t, gotJob.Steps, 1) - require.Equal(t, wantRun, string(gotJob.Steps[0].Run), "round-trip must preserve run content; got payload:\n%s", payload) + require.Equal(t, wantRun, gotJob.Steps[0].Run, "round-trip must preserve run content; got payload:\n%s", payload) } // Typing a step's continue-on-error as a bool used to reject the whole `jobs:` node. @@ -104,3 +104,37 @@ jobs: require.Equal(t, w, steps[i].RawContinueOnError, "step %d, payload:\n%s", i, payload) } } + +// Folded (`>`) scalars in with/env used to gain blank lines and have spaces turned into newlines +// on every Marshal -> Parse cycle. +func TestSingleWorkflowRoundTripFoldedScalar(t *testing.T) { + const wf = `name: demo +on: push +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: ./.github/actions/build + with: + cfg: > + begin + first "indented" line with path dir/Installer Scripts/some long path/another long path/some file name.xzy, + second "indented" line + end +` + const want = "begin\n first \"indented\" line with path dir/Installer Scripts/some long path/another long path/some file name.xzy,\n second \"indented\" line\nend\n" + sws, err := Parse([]byte(wf)) + require.NoError(t, err) + require.Len(t, sws, 1) + for range 3 { + payload, err := sws[0].Marshal() + require.NoError(t, err) + sws, err = Parse(payload) + require.NoError(t, err) + require.Len(t, sws, 1) + } + _, job := sws[0].Job() + var with map[string]string + require.NoError(t, job.Steps[0].With.Decode(&with)) + require.Equal(t, want, with["cfg"]) +} diff --git a/modules/actions/jobparser/testdata/prefixed_newline.out.yaml b/modules/actions/jobparser/testdata/prefixed_newline.out.yaml index 7baad4f5740..ef688b2734c 100644 --- a/modules/actions/jobparser/testdata/prefixed_newline.out.yaml +++ b/modules/actions/jobparser/testdata/prefixed_newline.out.yaml @@ -7,5 +7,9 @@ jobs: runs-on: ubuntu-latest steps: - id: extract_tag - name: "\nExtract tag for variant" - run: "\necho Test\n" + name: |- + + Extract tag for variant + run: | + + echo Test