mirror of
https://github.com/go-gitea/gitea.git
synced 2026-05-11 04:55:34 +02:00
actions: tighten artifact preview path resolution and stream large files
- Restore ReadSeeker streaming for storage.Object sources so PDF/text previews larger than UI.MaxDisplayFileSize (8 MiB) aren't truncated. - choosePreviewPath returns "" when an explicit path doesn't match, letting /preview/raw return 404 instead of silently serving a different file. Page handler still defaults to the first file when no path is given. - Drop StoragePath from the v4 zip-list cache key (ID:UpdatedUnix is sufficient) and move-to-end on overwrite to avoid stale order entries. - Add integration coverage for the v4 zip happy path (real zip in storage) and update the path-traversal test to match the no-fallback semantics. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
This commit is contained in:
parent
6a8f616b6e
commit
0b3f92cd9a
@ -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,
|
||||
})
|
||||
}
|
||||
|
||||
@ -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
|
||||
}
|
||||
|
||||
|
||||
@ -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": "<html><body><h1>v4 zip</h1></body></html>",
|
||||
"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, "<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())
|
||||
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)()
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user