From da9090927182a03338ac563b081854a970c65fb5 Mon Sep 17 00:00:00 2001 From: Nicolas Date: Mon, 4 May 2026 20:46:03 +0200 Subject: [PATCH] fix --- models/actions/run_job_summary.go | 10 ++- routers/api/actions/job_summary.go | 39 ++++++------ routers/api/actions/runner/runner.go | 2 +- routers/web/repo/actions/view.go | 4 ++ .../integration/api_actions_artifact_test.go | 62 +++++++++++++++++++ 5 files changed, 96 insertions(+), 21 deletions(-) diff --git a/models/actions/run_job_summary.go b/models/actions/run_job_summary.go index 0dc95f958d..a523839ee0 100644 --- a/models/actions/run_job_summary.go +++ b/models/actions/run_job_summary.go @@ -16,12 +16,15 @@ const ( // JobSummaryCapability is the runner-declare capability string for job summaries. JobSummaryCapability = "job-summary" + // JobSummaryContentTypeMarkdown is the only accepted content type for job summaries. + JobSummaryContentTypeMarkdown = "text/markdown" + // MaxJobSummarySize is the maximum accepted summary payload size in bytes. // This is intentionally conservative to avoid DB bloat and UI abuse. MaxJobSummarySize = 1024 * 1024 // 1 MiB ) -// ActionRunJobSummary stores the rendered job summary markdown uploaded by the runner. +// ActionRunJobSummary stores the raw job summary markdown uploaded by the runner. // It is internal state (not a downloadable artifact). type ActionRunJobSummary struct { ID int64 `xorm:"pk autoincr"` @@ -68,7 +71,10 @@ func UpsertActionRunJobSummary(ctx context.Context, repoID, runID, runAttemptID, return util.ErrInvalidArgument } if contentType == "" { - contentType = "text/markdown" + contentType = JobSummaryContentTypeMarkdown + } + if contentType != JobSummaryContentTypeMarkdown { + return util.ErrInvalidArgument } engine := db.GetEngine(ctx) diff --git a/routers/api/actions/job_summary.go b/routers/api/actions/job_summary.go index 5779209532..cff54c787e 100644 --- a/routers/api/actions/job_summary.go +++ b/routers/api/actions/job_summary.go @@ -6,26 +6,17 @@ package actions import ( "errors" "io" + "mime" "net/http" "strconv" - "strings" actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" - "code.gitea.io/gitea/modules/web" ) const jobSummaryRouteBase = "/_apis/pipelines/workflows/{run_id}/jobs/{job_id}/summary" -func JobSummaryRoutes(prefix string) *web.Router { - m := web.NewRouter() - m.AfterRouting(ArtifactContexter()) - - m.Put(jobSummaryRouteBase, uploadJobSummary) - return m -} - func uploadJobSummary(ctx *ArtifactContext) { task, runID, ok := validateRunID(ctx) if !ok { @@ -53,6 +44,7 @@ func uploadJobSummary(ctx *ArtifactContext) { body, err := io.ReadAll(io.LimitReader(ctx.Req.Body, actions_model.MaxJobSummarySize+1)) if err != nil { + log.Error("Error reading job summary request body: %v", err) ctx.HTTPError(http.StatusInternalServerError, "read request body") return } @@ -61,14 +53,10 @@ func uploadJobSummary(ctx *ArtifactContext) { return } - contentType := ctx.Req.Header.Get("Content-Type") - if contentType == "" || strings.HasPrefix(contentType, "application/octet-stream") { - contentType = "text/markdown" - } else { - // Strip charset to keep storage normalized; we only store UTF-8 text content. - if i := strings.Index(contentType, ";"); i > 0 { - contentType = strings.TrimSpace(contentType[:i]) - } + contentType, ok := normalizeJobSummaryContentType(ctx.Req.Header.Get("Content-Type")) + if !ok { + ctx.HTTPError(http.StatusBadRequest, "invalid summary content type") + return } if err := actions_model.UpsertActionRunJobSummary(ctx, task.Job.RepoID, task.Job.RunID, task.Job.RunAttemptID, task.Job.ID, contentType, body); err != nil { @@ -91,3 +79,18 @@ func uploadJobSummary(ctx *ArtifactContext) { func errorsIsInvalidArg(err error) bool { return errors.Is(err, util.ErrInvalidArgument) } + +func normalizeJobSummaryContentType(contentType string) (string, bool) { + if contentType == "" || contentType == "application/octet-stream" { + return actions_model.JobSummaryContentTypeMarkdown, true + } + + mediaType, _, err := mime.ParseMediaType(contentType) + if err != nil { + return "", false + } + if mediaType != actions_model.JobSummaryContentTypeMarkdown { + return "", false + } + return actions_model.JobSummaryContentTypeMarkdown, true +} diff --git a/routers/api/actions/runner/runner.go b/routers/api/actions/runner/runner.go index 2ee21e6a98..e785f4d0f9 100644 --- a/routers/api/actions/runner/runner.go +++ b/routers/api/actions/runner/runner.go @@ -130,7 +130,7 @@ func (s *Service) Declare( }) // Capabilities are communicated via headers to avoid a hard dependency on a proto bump. // Older runners ignore unknown headers; newer runners can use this for feature negotiation. - resp.Header().Set("X-Gitea-Actions-Capabilities", "job-summary") + resp.Header().Set("X-Gitea-Actions-Capabilities", actions_model.JobSummaryCapability) return resp, nil } diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index d962df3d09..0da90488e5 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -525,6 +525,10 @@ func fillViewRunResponseSummary(ctx *context_module.Context, resp *ViewResponse, resp.State.Run.JobSummaries = make([]*ViewJobSummary, 0, len(summaries)) renderUtils := templates.NewRenderUtils(ctx) for _, s := range summaries { + if s.ContentType != actions_model.JobSummaryContentTypeMarkdown { + log.Warn("Skip unsupported job summary content type %q for run %d job %d", s.ContentType, s.RunID, s.JobID) + continue + } resp.State.Run.JobSummaries = append(resp.State.Run.JobSummaries, &ViewJobSummary{ JobID: s.JobID, JobName: jobNameByID[s.JobID], diff --git a/tests/integration/api_actions_artifact_test.go b/tests/integration/api_actions_artifact_test.go index 0a4a63c551..3e7bf74f1b 100644 --- a/tests/integration/api_actions_artifact_test.go +++ b/tests/integration/api_actions_artifact_test.go @@ -16,6 +16,7 @@ import ( "strings" "testing" + actions_model "code.gitea.io/gitea/models/actions" auth_model "code.gitea.io/gitea/models/auth" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" @@ -44,6 +45,67 @@ func prepareTestEnvActionsArtifacts(t *testing.T) func() { return f } +func getArtifactFixtureTask(t *testing.T) *actions_model.ActionTask { + t.Helper() + + task, err := actions_model.GetRunningTaskByToken(t.Context(), "8061e833a55f6fc0157c98b883e91fcfeeb1a71a") + require.NoError(t, err) + require.NoError(t, task.LoadJob(t.Context())) + return task +} + +func TestActionsJobSummaryUpload(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + task := getArtifactFixtureTask(t) + summaryURL := fmt.Sprintf("/api/actions_pipeline/_apis/pipelines/workflows/%d/jobs/%d/summary", task.Job.RunID, task.Job.ID) + + t.Run("success", func(t *testing.T) { + body := "### Uploaded summary\n\n- line one\n" + req := NewRequestWithBody(t, "PUT", summaryURL, strings.NewReader(body)). + AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a"). + SetHeader("Content-Type", "text/markdown; charset=utf-8") + MakeRequest(t, req, http.StatusOK) + + summary, err := actions_model.GetActionRunJobSummary(t.Context(), task.Job.RepoID, task.Job.RunID, task.Job.RunAttemptID, task.Job.ID) + require.NoError(t, err) + assert.Equal(t, actions_model.JobSummaryContentTypeMarkdown, summary.ContentType) + assert.Equal(t, body, summary.Content) + }) + + t.Run("invalid-content-type", func(t *testing.T) { + req := NewRequestWithBody(t, "PUT", summaryURL, strings.NewReader("summary")). + AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a"). + SetHeader("Content-Type", "text/html") + resp := MakeRequest(t, req, http.StatusBadRequest) + assert.Contains(t, resp.Body.String(), "invalid summary content type") + }) + + t.Run("size-limit", func(t *testing.T) { + req := NewRequestWithBody(t, "PUT", summaryURL, strings.NewReader(strings.Repeat("a", actions_model.MaxJobSummarySize+1))). + AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a"). + SetHeader("Content-Type", actions_model.JobSummaryContentTypeMarkdown) + resp := MakeRequest(t, req, http.StatusBadRequest) + assert.Contains(t, resp.Body.String(), "invalid summary") + }) + + t.Run("job-mismatch", func(t *testing.T) { + req := NewRequestWithBody(t, "PUT", fmt.Sprintf("/api/actions_pipeline/_apis/pipelines/workflows/%d/jobs/%d/summary", task.Job.RunID, task.Job.ID+1), strings.NewReader("summary")). + AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a"). + SetHeader("Content-Type", actions_model.JobSummaryContentTypeMarkdown) + resp := MakeRequest(t, req, http.StatusBadRequest) + assert.Contains(t, resp.Body.String(), "job_id mismatch") + }) + + t.Run("run-mismatch", func(t *testing.T) { + req := NewRequestWithBody(t, "PUT", fmt.Sprintf("/api/actions_pipeline/_apis/pipelines/workflows/%d/jobs/%d/summary", task.Job.RunID+1, task.Job.ID), strings.NewReader("summary")). + AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a"). + SetHeader("Content-Type", actions_model.JobSummaryContentTypeMarkdown) + resp := MakeRequest(t, req, http.StatusBadRequest) + assert.Contains(t, resp.Body.String(), "run-id does not match") + }) +} + func TestActionsArtifactUploadSingleFile(t *testing.T) { defer prepareTestEnvActionsArtifacts(t)()