Mirrors/gitea
0
0
Fork 0
mirror of https://github.com/go-gitea/gitea.git synced 2026-05-11 04:55:34 +02:00
Code Issues Activity

fix tests: align preview run lookup with download (use ID) and fix path-traversal assertion

Browse Source 
- 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 "<iframe" instead
  of the CSS class "artifact-preview-frame", which is also declared in
  the always-rendered <style> block.
- TestActionsArtifactDownloadViewUnchanged: also use the run ID, since
  ArtifactsDownloadView only resolves by ID.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
This commit is contained in:
silverwind 2026-04-28 02:51:31 +02:00
parent d8e516d2d3
commit 49b6064074
No known key found for this signature in database
GPG Key ID: 2E62B41C93869443
2 changed files with 18 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
routers/web/repo/actions/view.go
View File

@ -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
}

31
tests/integration/actions_artifact_preview_test.go
View File

@ -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(), "<iframe")
// URL-encoded so the router doesn't collapse the segments before the handler sees them.
req = NewRequestf(t, "GET", "/%s/actions/runs/187/artifacts/multi-file-download/preview/raw/%s", repo.FullName(), "%2E%2E%2F%2E%2E%2Fetc%2Fpasswd")
req = NewRequestf(t, "GET", "/%s/actions/runs/791/artifacts/multi-file-download/preview/raw/%s", repo.FullName(), "%2E%2E%2F%2E%2E%2Fetc%2Fpasswd")
session.MakeRequest(t, req, http.StatusNotFound)
}
@ -106,7 +107,7 @@ func TestActionsArtifactPreviewUnsupportedType(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/raw", repo.FullName())
req := NewRequestf(t, "GET", "/%s/actions/runs/791/artifacts/artifact-download/preview/raw", repo.FullName())
session.MakeRequest(t, req, http.StatusUnsupportedMediaType)
}
@ -117,7 +118,7 @@ func TestActionsArtifactPreviewHTMLSandboxCSP(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/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.Contains(t, resp.Header().Get("Content-Security-Policy"), "sandbox")
assert.Contains(t, resp.Header().Get("Content-Type"), "text/html")
@ -136,25 +137,25 @@ func TestActionsArtifactPreviewV4Zip(t *testing.T) {
session := loginUser(t, "user2")
// /preview lists files extracted from the zip's central directory.
req := NewRequestf(t, "GET", "/%s/actions/runs/188/artifacts/artifact-v4-download/preview", repo.FullName())
req := NewRequestf(t, "GET", "/%s/actions/runs/792/artifacts/artifact-v4-download/preview", repo.FullName())
resp := session.MakeRequest(t, req, http.StatusOK)
body := resp.Body.String()
assert.Contains(t, body, "index.html")
assert.Contains(t, body, "logs/output.txt")
assert.Contains(t, body, "/preview/raw/index.html")
req = NewRequestf(t, "GET", "/%s/actions/runs/188/artifacts/artifact-v4-download/preview/raw/index.html", repo.FullName())
req = NewRequestf(t, "GET", "/%s/actions/runs/792/artifacts/artifact-v4-download/preview/raw/index.html", repo.FullName())
resp = session.MakeRequest(t, req, http.StatusOK)
assert.Equal(t, "<html><body><h1>v4 zip</h1></body></html>", 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")
}