diff --git a/routers/web/devtest/mock_actions.go b/routers/web/devtest/mock_actions.go index e03cf42673..4107f4a068 100644 --- a/routers/web/devtest/mock_actions.go +++ b/routers/web/devtest/mock_actions.go @@ -17,8 +17,8 @@ import ( "time" actions_model "code.gitea.io/gitea/models/actions" - "code.gitea.io/gitea/modules/httplib" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" @@ -590,20 +590,14 @@ func MockActionsArtifactPreviewRaw(ctx *context.Context) { return } + contentType := "text/plain; charset=utf-8" if path.Ext(selectedFile.Path) == ".html" { - ctx.Resp.Header().Set("Content-Security-Policy", "default-src 'none'; sandbox") - size := int64(len(selectedFile.Content)) - ctx.ServeContent(strings.NewReader(selectedFile.Content), context.ServeHeaderOptions{ - Filename: selectedFile.Path, - ContentLength: &size, - ContentType: "text/html", - }) - return + contentType = "text/html" } size := int64(len(selectedFile.Content)) ctx.ServeContent(strings.NewReader(selectedFile.Content), context.ServeHeaderOptions{ Filename: selectedFile.Path, ContentLength: &size, - ContentType: "text/plain; charset=utf-8", + ContentType: contentType, }) } diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 2689d542db..69f02c5f4f 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -1033,14 +1033,20 @@ func artifactPreviewFallbackPath(artifact *actions_model.ActionArtifact) string return artifact.ArtifactName } +// choosePreviewPath resolves the preview path to render. +// An empty `requested` means no path was specified, so the first file is selected as a default. +// A non-empty `requested` that is not present in `paths` returns "" so callers can 404 instead of silently swapping to a different file. func choosePreviewPath(paths []string, requested string) string { if len(paths) == 0 { return "" } + if requested == "" { + return paths[0] + } if util.SliceContainsString(paths, requested) { return requested } - return paths[0] + return "" } func listPreviewPathsForLegacyArtifacts(artifacts []*actions_model.ActionArtifact) []string { @@ -1120,7 +1126,7 @@ func listPreviewPathsForV4Artifact(artifact *actions_model.ActionArtifact) ([]st } func artifactPreviewV4ZipListCacheKey(artifact *actions_model.ActionArtifact) string { - return strconv.FormatInt(artifact.ID, 10) + ":" + strconv.FormatInt(int64(artifact.UpdatedUnix), 10) + ":" + artifact.StoragePath + return strconv.FormatInt(artifact.ID, 10) + ":" + strconv.FormatInt(int64(artifact.UpdatedUnix), 10) } func removeArtifactPreviewV4ZipListCacheOrderKey(order []string, key string) []string { @@ -1159,9 +1165,10 @@ func setArtifactPreviewV4ZipListCache(artifact *actions_model.ActionArtifact, pa artifactPreviewV4ZipListCache.mu.Lock() defer artifactPreviewV4ZipListCache.mu.Unlock() - if _, ok := artifactPreviewV4ZipListCache.entries[key]; !ok { - artifactPreviewV4ZipListCache.order = append(artifactPreviewV4ZipListCache.order, key) + if _, ok := artifactPreviewV4ZipListCache.entries[key]; ok { + artifactPreviewV4ZipListCache.order = removeArtifactPreviewV4ZipListCacheOrderKey(artifactPreviewV4ZipListCache.order, key) } + artifactPreviewV4ZipListCache.order = append(artifactPreviewV4ZipListCache.order, key) artifactPreviewV4ZipListCache.entries[key] = artifactPreviewV4ZipListCacheEntry{ paths: append([]string(nil), paths...), expiresAt: time.Now().Add(artifactPreviewV4ZipListCacheTTL), @@ -1188,13 +1195,7 @@ func isPreviewableArtifactType(st typesniffer.SniffedType) bool { return st.IsText() || st.IsPDF() } -func setArtifactPreviewCSP(ctx *context_module.Context, st typesniffer.SniffedType) { - if st.GetMimeType() == "text/html" { - ctx.Resp.Header().Set("Content-Security-Policy", "default-src 'none'; sandbox") - } -} - -func previewArtifactByReader(ctx *context_module.Context, path string, _ int64, reader io.Reader) { +func previewArtifactByReader(ctx *context_module.Context, path string, reader io.Reader) { buf := filebuffer.New(int(setting.UI.MaxDisplayFileSize), "") defer buf.Close() if _, err := io.Copy(buf, io.LimitReader(reader, setting.UI.MaxDisplayFileSize)); err != nil { @@ -1227,15 +1228,8 @@ func previewArtifactByReadSeeker(ctx *context_module.Context, path string, reade ctx.HTTPError(http.StatusUnsupportedMediaType, "artifact preview is not supported for this file type") return } - setArtifactPreviewCSP(ctx, st) - if st.GetMimeType() == "text/html" { - ctx.ServeContent(reader, context_module.ServeHeaderOptions{ - Filename: path, - ContentType: "text/html", - }) - return - } + // CSP sandbox is applied by httplib.ServeSetHeaders, see HINT: PDF-RENDER-SANDBOX ctx.ServeContent(reader, context_module.ServeHeaderOptions{ Filename: path, ContentType: st.GetMimeType(), @@ -1343,7 +1337,7 @@ func ArtifactsPreviewRawView(ctx *context_module.Context) { } defer r.Close() - previewArtifactByReader(ctx, selectedPath, int64(zf.UncompressedSize64), r) + previewArtifactByReader(ctx, selectedPath, r) return } @@ -1377,7 +1371,7 @@ func ArtifactsPreviewRawView(ctx *context_module.Context) { } defer r.Close() - previewArtifactByReader(ctx, selectedPath, artifact.FileSize, r) + previewArtifactByReader(ctx, selectedPath, r) return } diff --git a/tests/integration/actions_artifact_preview_test.go b/tests/integration/actions_artifact_preview_test.go index def760463c..9e9b018215 100644 --- a/tests/integration/actions_artifact_preview_test.go +++ b/tests/integration/actions_artifact_preview_test.go @@ -4,6 +4,7 @@ package integration import ( + "archive/zip" "bytes" "net/http" "net/url" @@ -19,6 +20,20 @@ import ( "github.com/stretchr/testify/require" ) +func buildArtifactZip(t *testing.T, files map[string]string) []byte { + t.Helper() + var buf bytes.Buffer + w := zip.NewWriter(&buf) + for name, content := range files { + fw, err := w.Create(name) + require.NoError(t, err) + _, err = fw.Write([]byte(content)) + require.NoError(t, err) + } + require.NoError(t, w.Close()) + return buf.Bytes() +} + func overwriteArtifactStorageContent(t *testing.T, artifactID int64, content []byte) { t.Helper() artifact := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionArtifact{ID: artifactID}) @@ -65,6 +80,25 @@ func TestActionsArtifactPreviewMultiFile(t *testing.T) { assert.Equal(t, strings.Repeat("C", 1024), resp.Body.String()) } +func TestActionsArtifactPreviewPathTraversal(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + session := loginUser(t, "user2") + + // 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")) + 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") + + // 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") + session.MakeRequest(t, req, http.StatusNotFound) +} + func TestActionsArtifactPreviewUnsupportedType(t *testing.T) { defer prepareTestEnvActionsArtifacts(t)() @@ -89,6 +123,41 @@ func TestActionsArtifactPreviewHTMLSandboxCSP(t *testing.T) { assert.Contains(t, resp.Header().Get("Content-Type"), "text/html") } +func TestActionsArtifactPreviewV4Zip(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + zipBytes := buildArtifactZip(t, map[string]string{ + "index.html": "

v4 zip

", + "logs/output.txt": "v4 log output", + }) + overwriteArtifactStorageContent(t, 22, zipBytes) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + 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()) + 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()) + resp = session.MakeRequest(t, req, http.StatusOK) + assert.Equal(t, "

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()) + 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()) + session.MakeRequest(t, req, http.StatusNotFound) +} + func TestActionsArtifactDownloadViewUnchanged(t *testing.T) { defer prepareTestEnvActionsArtifacts(t)()