From 49b60640742843cf74a8a87b434678557154582b Mon Sep 17 00:00:00 2001 From: silverwind Date: Tue, 28 Apr 2026 02:51:31 +0200 Subject: [PATCH] fix tests: align preview run lookup with download (use ID) and fix path-traversal assertion - getCurrentRunAndUploadedArtifacts now resolves the run by ID via getCurrentRunByPathParam, matching ArtifactsDownloadView and the ID-based URLs that run.Link() generates. Without this, the preview links produced by the UI (which use run.ID) hit the helper that was looking up by repo-scoped index and 404'd. - Update preview tests to use run IDs (791, 792) instead of indices (187, 188), consistent with the new helper. - TestActionsArtifactPreviewPathTraversal: assert on " block. - TestActionsArtifactDownloadViewUnchanged: also use the run ID, since ArtifactsDownloadView only resolves by ID. Co-Authored-By: Claude (Opus 4.7) --- routers/web/repo/actions/view.go | 17 ++-------- .../actions_artifact_preview_test.go | 31 ++++++++++--------- 2 files changed, 18 insertions(+), 30 deletions(-) diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index b29255f5f4..73ca6d35d3 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -967,21 +967,8 @@ func resolveArtifactAttemptIDFromQuery(ctx *context_module.Context, run *actions } func getCurrentRunAndUploadedArtifacts(ctx *context_module.Context, artifactName string) (*actions_model.ActionRun, []*actions_model.ActionArtifact, bool) { - var ( - run *actions_model.ActionRun - err error - ) - if ctx.PathParam("run") == "latest" { - run, err = actions_model.GetLatestRun(ctx, ctx.Repo.Repository.ID) - } else { - run, err = actions_model.GetRunByRepoAndIndex(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("run")) - } - if err != nil { - if errors.Is(err, util.ErrNotExist) { - ctx.HTTPError(http.StatusNotFound, err.Error()) - return nil, nil, false - } - ctx.ServerError("GetRunByRepoAndIndex", err) + run := getCurrentRunByPathParam(ctx) + if ctx.Written() { return nil, nil, false } diff --git a/tests/integration/actions_artifact_preview_test.go b/tests/integration/actions_artifact_preview_test.go index afe627a20c..4b36ea22e4 100644 --- a/tests/integration/actions_artifact_preview_test.go +++ b/tests/integration/actions_artifact_preview_test.go @@ -47,14 +47,14 @@ func TestActionsArtifactPreviewSingleFile(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) session := loginUser(t, "user2") - req := NewRequestf(t, "GET", "/%s/actions/runs/187/artifacts/artifact-download/preview", repo.FullName()) + req := NewRequestf(t, "GET", "/%s/actions/runs/791/artifacts/artifact-download/preview", repo.FullName()) resp := session.MakeRequest(t, req, http.StatusOK) assert.Contains(t, resp.Body.String(), "abc.txt") assert.Contains(t, resp.Body.String(), "/preview/raw/abc.txt") assert.Contains(t, resp.Body.String(), `sandbox=""`) assert.Contains(t, resp.Body.String(), `referrerpolicy="no-referrer"`) - req = NewRequestf(t, "GET", "/%s/actions/runs/187/artifacts/artifact-download/preview/raw", repo.FullName()) + req = NewRequestf(t, "GET", "/%s/actions/runs/791/artifacts/artifact-download/preview/raw", repo.FullName()) resp = session.MakeRequest(t, req, http.StatusOK) assert.Equal(t, strings.Repeat("A", 1024), resp.Body.String()) assert.Contains(t, resp.Header().Get("Content-Type"), "text/plain") @@ -66,16 +66,16 @@ func TestActionsArtifactPreviewMultiFile(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) session := loginUser(t, "user2") - req := NewRequestf(t, "GET", "/%s/actions/runs/187/artifacts/multi-file-download/preview", repo.FullName()) + req := NewRequestf(t, "GET", "/%s/actions/runs/791/artifacts/multi-file-download/preview", repo.FullName()) resp := session.MakeRequest(t, req, http.StatusOK) assert.Contains(t, resp.Body.String(), "abc.txt") assert.Contains(t, resp.Body.String(), "xyz/def.txt") - req = NewRequestf(t, "GET", "/%s/actions/runs/187/artifacts/multi-file-download/preview?path=%s", repo.FullName(), url.QueryEscape("xyz/def.txt")) + req = NewRequestf(t, "GET", "/%s/actions/runs/791/artifacts/multi-file-download/preview?path=%s", repo.FullName(), url.QueryEscape("xyz/def.txt")) resp = session.MakeRequest(t, req, http.StatusOK) assert.Contains(t, resp.Body.String(), "/preview/raw/xyz/def.txt") - req = NewRequestf(t, "GET", "/%s/actions/runs/187/artifacts/multi-file-download/preview/raw/xyz/def.txt", repo.FullName()) + req = NewRequestf(t, "GET", "/%s/actions/runs/791/artifacts/multi-file-download/preview/raw/xyz/def.txt", repo.FullName()) resp = session.MakeRequest(t, req, http.StatusOK) assert.Equal(t, strings.Repeat("C", 1024), resp.Body.String()) } @@ -88,14 +88,15 @@ func TestActionsArtifactPreviewPathTraversal(t *testing.T) { // A traversal-style path normalizes to a path that doesn't match any file in // the artifact. The page still renders with the file list, but no preview is selected. - req := NewRequestf(t, "GET", "/%s/actions/runs/187/artifacts/multi-file-download/preview?path=%s", repo.FullName(), url.QueryEscape("../../../etc/passwd")) + req := NewRequestf(t, "GET", "/%s/actions/runs/791/artifacts/multi-file-download/preview?path=%s", repo.FullName(), url.QueryEscape("../../../etc/passwd")) resp := session.MakeRequest(t, req, http.StatusOK) assert.Contains(t, resp.Body.String(), "abc.txt") assert.NotContains(t, resp.Body.String(), "etc/passwd") - assert.NotContains(t, resp.Body.String(), "artifact-preview-frame") + // the iframe element is rendered only when a preview file is selected. + assert.NotContains(t, resp.Body.String(), "

v4 zip

", resp.Body.String()) assert.Contains(t, resp.Header().Get("Content-Type"), "text/html") assert.Contains(t, resp.Header().Get("Content-Security-Policy"), "sandbox") - req = NewRequestf(t, "GET", "/%s/actions/runs/188/artifacts/artifact-v4-download/preview/raw/logs/output.txt", repo.FullName()) + req = NewRequestf(t, "GET", "/%s/actions/runs/792/artifacts/artifact-v4-download/preview/raw/logs/output.txt", repo.FullName()) resp = session.MakeRequest(t, req, http.StatusOK) assert.Equal(t, "v4 log output", resp.Body.String()) // Unknown path inside the zip returns 404 instead of falling back. - req = NewRequestf(t, "GET", "/%s/actions/runs/188/artifacts/artifact-v4-download/preview/raw/missing.txt", repo.FullName()) + req = NewRequestf(t, "GET", "/%s/actions/runs/792/artifacts/artifact-v4-download/preview/raw/missing.txt", repo.FullName()) session.MakeRequest(t, req, http.StatusNotFound) } @@ -163,7 +164,7 @@ func TestActionsArtifactDownloadViewUnchanged(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) session := loginUser(t, "user2") - req := NewRequestf(t, "GET", "/%s/actions/runs/187/artifacts/artifact-download", repo.FullName()) + req := NewRequestf(t, "GET", "/%s/actions/runs/791/artifacts/artifact-download", repo.FullName()) resp := session.MakeRequest(t, req, http.StatusOK) assert.Contains(t, resp.Header().Get("Content-Disposition"), "attachment; filename=artifact-download.zip") }