actions: harden artifact preview and rebase onto main

routers/web/devtest/mock_actions.go

@@ -18,11 +18,9 @@ import (
 	"time"
 
 	actions_model "code.gitea.io/gitea/models/actions"
-	"code.gitea.io/gitea/modules/httplib"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
-	"code.gitea.io/gitea/routers/common"
 	"code.gitea.io/gitea/routers/web/repo/actions"
 	"code.gitea.io/gitea/services/context"
 )
@@ -96,7 +94,7 @@ var mockArtifactPreviewTemplate = template.Must(template.New("mock-artifact-prev
     </div>
     <div>
       {{if .SelectedPath}}
-        <iframe src="{{.PreviewRaw}}?path={{.SelectedPath | urlquery}}" referrerpolicy="same-origin"></iframe>
+        <iframe src="{{.PreviewRaw}}/{{.SelectedPath | urlquery}}" sandbox="allow-scripts" referrerpolicy="no-referrer"></iframe>
       {{else}}
         <p>No files</p>
       {{end}}
@@ -372,7 +370,11 @@ func MockActionsArtifactPreview(ctx *context.Context) {
 		return
 	}
 
-	selectedPath := chooseMockArtifactPath(files, normalizeMockArtifactPath(ctx.Req.URL.Query().Get("path")))
+	selectedPath := normalizeMockArtifactPath(strings.TrimPrefix(ctx.PathParam("*"), "/"))
+	if selectedPath == "" {
+		selectedPath = normalizeMockArtifactPath(ctx.Req.URL.Query().Get("path"))
+	}
+	selectedPath = chooseMockArtifactPath(files, selectedPath)
 	templateFiles := make([]mockArtifactPreviewTemplateFile, 0, len(files))
 	for _, file := range files {
 		templateFiles = append(templateFiles, mockArtifactPreviewTemplateFile{
@@ -428,11 +430,17 @@ func MockActionsArtifactPreviewRaw(ctx *context.Context) {
 
 	if path.Ext(selectedFile.Path) == ".html" {
 		ctx.Resp.Header().Set("Content-Security-Policy", "default-src 'none'; sandbox")
-		httplib.ServeContentByReader(ctx.Req, ctx.Resp, int64(len(selectedFile.Content)), strings.NewReader(selectedFile.Content), &httplib.ServeHeaderOptions{
+		size := int64(len(selectedFile.Content))
-			Filename:    selectedFile.Path,
+		ctx.ServeContent(strings.NewReader(selectedFile.Content), context.ServeHeaderOptions{
-			ContentType: "text/html",
+			Filename:      selectedFile.Path,
+			ContentLength: &size,
+			ContentType:   "text/html",
 		})
 		return
 	}
-	common.ServeContentByReader(ctx.Base, selectedFile.Path, int64(len(selectedFile.Content)), strings.NewReader(selectedFile.Content))
+	size := int64(len(selectedFile.Content))
+	ctx.ServeContent(strings.NewReader(selectedFile.Content), context.ServeHeaderOptions{
+		Filename:      selectedFile.Path,
+		ContentLength: &size,
+	})
 }

routers/web/repo/actions/view.go

@@ -5,7 +5,6 @@ package actions
 
 import (
 	"archive/zip"
-	"bytes"
 	"compress/gzip"
 	"context"
 	"errors"
@@ -16,6 +15,8 @@ import (
 	"net/url"
 	"sort"
 	"strconv"
+	"strings"
+	"sync"
 	"time"
 
 	actions_model "code.gitea.io/gitea/models/actions"
@@ -134,6 +135,25 @@ type ArtifactPreviewFile struct {
 	Selected bool
 }
 
+type readAtBySeeker struct {
+	rs io.ReadSeeker
+	mu sync.Mutex
+}
+
+func (r *readAtBySeeker) ReadAt(p []byte, off int64) (int, error) {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+
+	if _, err := r.rs.Seek(off, io.SeekStart); err != nil {
+		return 0, err
+	}
+	n, err := io.ReadFull(r.rs, p)
+	if errors.Is(err, io.ErrUnexpectedEOF) {
+		return n, io.EOF
+	}
+	return n, err
+}
+
 type ViewResponse struct {
 	Artifacts []*ArtifactsViewItem `json:"artifacts"`
 
@@ -688,14 +708,9 @@ func getCurrentRunJobsByPathParam(ctx *context_module.Context) (*actions_model.A
 	return run, jobs
 }
 
-func getRunAndUploadedArtifacts(ctx *context_module.Context, runIndex int64, artifactName string) (*actions_model.ActionRun, []*actions_model.ActionArtifact, bool) {
+func getCurrentRunAndUploadedArtifacts(ctx *context_module.Context, artifactName string) (*actions_model.ActionRun, []*actions_model.ActionArtifact, bool) {
-	run, err := actions_model.GetRunByIndex(ctx, ctx.Repo.Repository.ID, runIndex)
+	run := getCurrentRunByPathParam(ctx)
-	if err != nil {
+	if ctx.Written() {
-		if errors.Is(err, util.ErrNotExist) {
-			ctx.HTTPError(http.StatusNotFound, err.Error())
-			return nil, nil, false
-		}
-		ctx.ServerError("GetRunByIndex", err)
 		return nil, nil, false
 	}
 
@@ -731,6 +746,14 @@ func normalizeArtifactPreviewPath(path string) string {
 	return path
 }
 
+func getRequestedPreviewPath(ctx *context_module.Context) string {
+	path := strings.TrimPrefix(ctx.PathParam("*"), "/")
+	if path == "" {
+		path = ctx.Req.URL.Query().Get("path")
+	}
+	return normalizeArtifactPreviewPath(path)
+}
+
 func artifactPreviewFallbackPath(artifact *actions_model.ActionArtifact) string {
 	path := normalizeArtifactPreviewPath(artifact.ArtifactPath)
 	if path != "" {
@@ -764,25 +787,23 @@ func listPreviewPathsForLegacyArtifacts(artifacts []*actions_model.ActionArtifac
 	return paths
 }
 
-func openArtifactV4ZipReader(artifact *actions_model.ActionArtifact) (*filebuffer.FileBackedBuffer, *zip.Reader, error) {
+func openArtifactV4ZipReader(artifact *actions_model.ActionArtifact) (storage.Object, *zip.Reader, error) {
 	f, err := storage.ActionsArtifacts.Open(artifact.StoragePath)
 	if err != nil {
 		return nil, nil, err
 	}
-	defer f.Close()
+	stat, err := f.Stat()
-
-	buf := filebuffer.New(int(setting.UI.MaxDisplayFileSize), "")
-	if _, err := io.Copy(buf, f); err != nil {
-		_ = buf.Close()
-		return nil, nil, err
-	}
-
-	reader, err := zip.NewReader(buf, buf.Size())
 	if err != nil {
-		_ = buf.Close()
+		_ = f.Close()
 		return nil, nil, err
 	}
-	return buf, reader, nil
+
+	reader, err := zip.NewReader(&readAtBySeeker{rs: f}, stat.Size())
+	if err != nil {
+		_ = f.Close()
+		return nil, nil, err
+	}
+	return f, reader, nil
 }
 
 func listArtifactV4ZipFiles(reader *zip.Reader) ([]string, map[string]*zip.File) {
@@ -807,14 +828,14 @@ func listArtifactV4ZipFiles(reader *zip.Reader) ([]string, map[string]*zip.File)
 }
 
 func listPreviewPathsForV4Artifact(artifact *actions_model.ActionArtifact) ([]string, error) {
-	buf, reader, err := openArtifactV4ZipReader(artifact)
+	obj, reader, err := openArtifactV4ZipReader(artifact)
 	if err != nil {
 		if errors.Is(err, zip.ErrFormat) {
 			return []string{artifactPreviewFallbackPath(artifact)}, nil
 		}
 		return nil, err
 	}
-	defer buf.Close()
+	defer obj.Close()
 
 	paths, _ := listArtifactV4ZipFiles(reader)
 	return paths, nil
@@ -837,34 +858,18 @@ func setArtifactPreviewCSP(ctx *context_module.Context, st typesniffer.SniffedTy
 	}
 }
 
-func previewArtifactByReader(ctx *context_module.Context, path string, size int64, reader io.Reader) {
+func previewArtifactByReader(ctx *context_module.Context, path string, _ int64, reader io.Reader) {
-	buf := make([]byte, typesniffer.SniffContentSize)
+	buf := filebuffer.New(int(setting.UI.MaxDisplayFileSize), "")
-	n, err := util.ReadAtMost(reader, buf)
+	defer buf.Close()
-	if err != nil {
+	if _, err := io.Copy(buf, reader); err != nil {
-		ctx.ServerError("ReadAtMost", err)
+		ctx.ServerError("io.Copy", err)
 		return
 	}
-	if n < 0 {
+	if _, err := buf.Seek(0, io.SeekStart); err != nil {
-		n = 0
+		ctx.ServerError("Seek", err)
-	}
-	buf = buf[:n]
-
-	st := typesniffer.DetectContentType(buf)
-	if !isPreviewableArtifactType(st) {
-		ctx.HTTPError(http.StatusUnsupportedMediaType, "artifact preview is not supported for this file type")
 		return
 	}
-	setArtifactPreviewCSP(ctx, st)
+	previewArtifactByReadSeeker(ctx, path, buf)
-
-	stream := io.MultiReader(bytes.NewReader(buf), reader)
-	if st.GetMimeType() == "text/html" {
-		httplib.ServeContentByReader(ctx.Req, ctx.Resp, size, stream, &httplib.ServeHeaderOptions{
-			Filename:    path,
-			ContentType: "text/html",
-		})
-		return
-	}
-	common.ServeContentByReader(ctx.Base, path, size, stream)
 }
 
 func previewArtifactByReadSeeker(ctx *context_module.Context, path string, reader io.ReadSeeker) {
@@ -877,6 +882,8 @@ func previewArtifactByReadSeeker(ctx *context_module.Context, path string, reade
 	if n < 0 {
 		n = 0
 	}
+	buf = buf[:n]
+
 	if _, err := reader.Seek(0, io.SeekStart); err != nil {
 		ctx.ServerError("Seek", err)
 		return
@@ -891,20 +898,21 @@ func previewArtifactByReadSeeker(ctx *context_module.Context, path string, reade
 	setArtifactPreviewCSP(ctx, st)
 
 	if st.GetMimeType() == "text/html" {
-		httplib.ServeContentByReadSeeker(ctx.Req, ctx.Resp, nil, reader, &httplib.ServeHeaderOptions{
+		ctx.ServeContent(reader, context_module.ServeHeaderOptions{
 			Filename:    path,
 			ContentType: "text/html",
 		})
 		return
 	}
-	common.ServeContentByReadSeeker(ctx.Base, path, nil, reader)
+	ctx.ServeContent(reader, context_module.ServeHeaderOptions{
+		Filename: path,
+	})
 }
 
 func ArtifactsPreviewView(ctx *context_module.Context) {
-	runIndex := getRunIndex(ctx)
 	artifactName := ctx.PathParam("artifact_name")
 
-	run, artifacts, ok := getRunAndUploadedArtifacts(ctx, runIndex, artifactName)
+	run, artifacts, ok := getCurrentRunAndUploadedArtifacts(ctx, artifactName)
 	if !ok {
 		return
 	}
@@ -914,7 +922,7 @@ func ArtifactsPreviewView(ctx *context_module.Context) {
 		ctx.ServerError("listPreviewPaths", err)
 		return
 	}
-	selectedPath := choosePreviewPath(paths, normalizeArtifactPreviewPath(ctx.Req.URL.Query().Get("path")))
+	selectedPath := choosePreviewPath(paths, getRequestedPreviewPath(ctx))
 
 	previewFiles := make([]ArtifactPreviewFile, 0, len(paths))
 	for _, path := range paths {
@@ -942,10 +950,9 @@ func ArtifactsPreviewView(ctx *context_module.Context) {
 }
 
 func ArtifactsPreviewRawView(ctx *context_module.Context) {
-	runIndex := getRunIndex(ctx)
 	artifactName := ctx.PathParam("artifact_name")
 
-	_, artifacts, ok := getRunAndUploadedArtifacts(ctx, runIndex, artifactName)
+	_, artifacts, ok := getCurrentRunAndUploadedArtifacts(ctx, artifactName)
 	if !ok {
 		return
 	}
@@ -955,7 +962,7 @@ func ArtifactsPreviewRawView(ctx *context_module.Context) {
 		ctx.ServerError("listPreviewPaths", err)
 		return
 	}
-	selectedPath := choosePreviewPath(paths, normalizeArtifactPreviewPath(ctx.Req.URL.Query().Get("path")))
+	selectedPath := choosePreviewPath(paths, getRequestedPreviewPath(ctx))
 	if selectedPath == "" {
 		ctx.HTTPError(http.StatusNotFound, "artifact file not found")
 		return
@@ -964,7 +971,7 @@ func ArtifactsPreviewRawView(ctx *context_module.Context) {
 	if len(artifacts) == 1 && actions.IsArtifactV4(artifacts[0]) {
 		artifact := artifacts[0]
 
-		buf, reader, err := openArtifactV4ZipReader(artifact)
+		obj, reader, err := openArtifactV4ZipReader(artifact)
 		if err != nil {
 			if !errors.Is(err, zip.ErrFormat) {
 				ctx.ServerError("openArtifactV4ZipReader", err)
@@ -987,7 +994,7 @@ func ArtifactsPreviewRawView(ctx *context_module.Context) {
 			previewArtifactByReadSeeker(ctx, selectedPath, f)
 			return
 		}
-		defer buf.Close()
+		defer obj.Close()
 
 		_, files := listArtifactV4ZipFiles(reader)
 		zf, ok := files[selectedPath]
@@ -1029,7 +1036,7 @@ func ArtifactsPreviewRawView(ctx *context_module.Context) {
 	}
 	defer f.Close()
 
-	if artifact.ContentEncoding == "gzip" {
+	if artifact.ContentEncodingOrType == actions_model.ContentEncodingV3Gzip {
 		r, err := gzip.NewReader(f)
 		if err != nil {
 			ctx.ServerError("gzip.NewReader", err)
@@ -1058,9 +1065,8 @@ func ArtifactsDeleteView(ctx *context_module.Context) {
 }
 
 func ArtifactsDownloadView(ctx *context_module.Context) {
-	runIndex := getRunIndex(ctx)
 	artifactName := ctx.PathParam("artifact_name")
-	_, artifacts, ok := getRunAndUploadedArtifacts(ctx, runIndex, artifactName)
+	_, artifacts, ok := getCurrentRunAndUploadedArtifacts(ctx, artifactName)
 	if !ok {
 		return
 	}

routers/web/web.go

@@ -1547,6 +1547,7 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) {
 			m.Get("/artifacts/{artifact_name}", actions.ArtifactsDownloadView)
 			m.Get("/artifacts/{artifact_name}/preview", actions.ArtifactsPreviewView)
 			m.Get("/artifacts/{artifact_name}/preview/raw", actions.ArtifactsPreviewRawView)
+			m.Get("/artifacts/{artifact_name}/preview/raw/*", actions.ArtifactsPreviewRawView)
 			m.Delete("/artifacts/{artifact_name}", reqRepoActionsWriter, actions.ArtifactsDeleteView)
 			m.Post("/rerun", reqRepoActionsWriter, actions.Rerun)
 			m.Post("/rerun-failed", reqRepoActionsWriter, actions.RerunFailed)
@@ -1753,6 +1754,7 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) {
 			m.Get("/repo-action-view/runs/{run}/artifacts/{artifact_name}", devtest.MockActionsArtifactDownload)
 			m.Get("/repo-action-view/runs/{run}/artifacts/{artifact_name}/preview", devtest.MockActionsArtifactPreview)
 			m.Get("/repo-action-view/runs/{run}/artifacts/{artifact_name}/preview/raw", devtest.MockActionsArtifactPreviewRaw)
+			m.Get("/repo-action-view/runs/{run}/artifacts/{artifact_name}/preview/raw/*", devtest.MockActionsArtifactPreviewRaw)
 			m.Get("/repo-action-view/runs/{run}/jobs/{job}", devtest.MockActionsView)
 			m.Post("/repo-action-view/runs/{run}", web.Bind(actions.ViewRequest{}), devtest.MockActionsRunsJobs)
 			m.Post("/repo-action-view/runs/{run}/jobs/{job}", web.Bind(actions.ViewRequest{}), devtest.MockActionsRunsJobs)

templates/repo/actions/artifact_preview.tmpl

@@ -32,9 +32,9 @@
 					{{end}}
 				</div>
 			</div>
-			<div class="twelve wide column">
+				<div class="twelve wide column">
 				{{if .SelectedPath}}
-					<iframe class="artifact-preview-frame" src="{{.PreviewRawURL}}?path={{QueryEscape .SelectedPath}}" referrerpolicy="same-origin"></iframe>
+					<iframe class="artifact-preview-frame" src="{{.PreviewRawURL}}/{{PathEscapeSegments .SelectedPath}}" sandbox="allow-scripts" referrerpolicy="no-referrer"></iframe>
 				{{else}}
 					<div class="ui attached segment">{{ctx.Locale.Tr "none"}}</div>
 				{{end}}

web_src/js/components/RepoActionView.vue

@@ -5,7 +5,7 @@ import {toRefs} from 'vue';
 import {POST, DELETE} from '../modules/fetch.ts';
 import ActionRunSummaryView from './ActionRunSummaryView.vue';
 import ActionRunJobView from './ActionRunJobView.vue';
-import {createActionRunViewStore} from "./ActionRunView.ts";
+import {createActionRunViewStore} from './ActionRunView.ts';
 
 defineOptions({
   name: 'RepoActionView',
@@ -30,9 +30,21 @@ function approveRun() {
   POST(`${run.value.link}/approve`);
 }
 
+function artifactBaseURL(name: string): string {
+  return `${run.value.link}/artifacts/${encodeURIComponent(name)}`;
+}
+
+function artifactPreviewURL(name: string): string {
+  return `${artifactBaseURL(name)}/preview`;
+}
+
+function artifactDownloadURL(name: string): string {
+  return artifactBaseURL(name);
+}
+
 async function deleteArtifact(name: string) {
   if (!window.confirm(locale.confirmDeleteArtifact.replace('%s', name))) return;
-  await DELETE(`${run.value.link}/artifacts/${encodeURIComponent(name)}`);
+  await DELETE(artifactBaseURL(name));
   await store.forceReloadCurrentRun();
 }
 </script>
@@ -126,7 +138,7 @@ async function deleteArtifact(name: string) {
                     <span class="gt-ellipsis">{{ artifact.name }}</span>
                   </a>
                   <span class="job-artifact-actions">
-                    <a target="_blank" :href="artifactDownloadURL(artifact.name)" :data-tooltip-content="locale.downloadFile">
+                    <a download target="_blank" :href="artifactDownloadURL(artifact.name)" :data-tooltip-content="locale.downloadFile">
                       <SvgIcon name="octicon-download" class="tw-text-text"/>
                     </a>
                     <a v-if="run.canDeleteArtifact" @click="deleteArtifact(artifact.name)">