sneak
5101abd407
Check / check (pull_request) Skipped
gosec flagged G703 (path traversal via taint analysis) on the log download handler because the served path derives from a URL parameter. Open the log file through an os.Root confined to the deploy log directory instead of passing the path to http.ServeFile; Root.Open rejects any path that escapes the root, so traversal attempts return 404. Serve the opened file with http.ServeContent. Adds GetLogDir on the deploy service. The traversal regression test plants a sentinel file at the location a traversal-shaped app name resolves to (outside the log directory) and asserts the handler returns 404 without leaking the sentinel, so it fails if the os.Root guard is removed. Keeps the legitimate-download test. Model: opus-4-8
122 lines
4.0 KiB
Go
122 lines
4.0 KiB
Go
package handlers_test
|
|
|
|
import (
|
|
"context"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"os"
|
|
"path/filepath"
|
|
"strconv"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
|
|
"sneak.berlin/go/upaas/internal/models"
|
|
)
|
|
|
|
// doLogDownload issues a log-download request for the given app and
|
|
// deployment and returns the recorder.
|
|
func doLogDownload(
|
|
t *testing.T,
|
|
testCtx *testContext,
|
|
appID string,
|
|
deploymentID int64,
|
|
) *httptest.ResponseRecorder {
|
|
t.Helper()
|
|
|
|
idStr := strconv.FormatInt(deploymentID, 10)
|
|
|
|
request := httptest.NewRequestWithContext(
|
|
t.Context(),
|
|
http.MethodGet,
|
|
"/apps/"+appID+"/deployments/"+idStr+"/log",
|
|
nil,
|
|
)
|
|
request = addChiURLParams(request, map[string]string{
|
|
"id": appID,
|
|
"deploymentID": idStr,
|
|
})
|
|
|
|
recorder := httptest.NewRecorder()
|
|
testCtx.handlers.HandleDeploymentLogDownload().ServeHTTP(recorder, request)
|
|
|
|
return recorder
|
|
}
|
|
|
|
// TestHandleDeploymentLogDownloadServesLegitimateFile verifies a normal
|
|
// log file is served for download.
|
|
func TestHandleDeploymentLogDownloadServesLegitimateFile(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
testCtx := setupTestHandlers(t)
|
|
createdApp := createTestApp(t, testCtx, "log-download-app")
|
|
|
|
deployment := models.NewDeployment(testCtx.database)
|
|
deployment.AppID = createdApp.ID
|
|
deployment.Status = models.DeploymentStatusSuccess
|
|
require.NoError(t, deployment.Save(context.Background()))
|
|
|
|
// Write the log file where the handler will look for it.
|
|
logPath := testCtx.deploySvc.GetLogFilePath(createdApp, deployment)
|
|
require.NoError(t, os.MkdirAll(filepath.Dir(logPath), 0o750))
|
|
require.NoError(t, os.WriteFile(logPath, []byte("deploy log contents"), 0o600))
|
|
|
|
recorder := doLogDownload(t, testCtx, createdApp.ID, deployment.ID)
|
|
|
|
assert.Equal(t, http.StatusOK, recorder.Code)
|
|
assert.Contains(t, recorder.Body.String(), "deploy log contents")
|
|
}
|
|
|
|
// TestHandleDeploymentLogDownloadRejectsPathTraversal verifies the
|
|
// os.Root containment guard. A traversal-shaped app name drives the
|
|
// resolved log path out of the deploy log directory onto a sentinel
|
|
// file that really exists. The handler must refuse to serve it (404)
|
|
// rather than leak its contents. Removing the guard makes this test
|
|
// fail, which the earlier version — pointed at a non-existent path that
|
|
// 404s either way — did not.
|
|
func TestHandleDeploymentLogDownloadRejectsPathTraversal(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
testCtx := setupTestHandlers(t)
|
|
createdApp := createTestApp(t, testCtx, "log-traversal-app")
|
|
|
|
createdApp.Name = "../.."
|
|
require.NoError(t, createdApp.Save(context.Background()))
|
|
|
|
// The log root must exist so os.OpenRoot succeeds and the rejection
|
|
// comes from the containment check, not a missing directory.
|
|
logDir := testCtx.deploySvc.GetLogDir()
|
|
require.NoError(t, os.MkdirAll(logDir, 0o750))
|
|
|
|
deployment := models.NewDeployment(testCtx.database)
|
|
deployment.AppID = createdApp.ID
|
|
deployment.Status = models.DeploymentStatusSuccess
|
|
require.NoError(t, deployment.Save(context.Background()))
|
|
|
|
// Where the handler resolves the log path to. The traversal name
|
|
// makes this land outside logDir; require that it truly escapes so
|
|
// the test cannot silently stop covering the guard.
|
|
escapedPath := testCtx.deploySvc.GetLogFilePath(createdApp, deployment)
|
|
relPath, relErr := filepath.Rel(logDir, escapedPath)
|
|
require.NoError(t, relErr)
|
|
require.True(t, strings.HasPrefix(relPath, ".."),
|
|
"resolved path must escape the log dir, got %q", relPath)
|
|
|
|
// Plant a sentinel where the traversal points; a missing guard would
|
|
// open and serve it.
|
|
require.NoError(t, os.MkdirAll(filepath.Dir(escapedPath), 0o750))
|
|
|
|
const sentinel = "SENTINEL-outside-log-dir-must-not-be-served"
|
|
|
|
require.NoError(t, os.WriteFile(escapedPath, []byte(sentinel), 0o600))
|
|
t.Cleanup(func() { _ = os.Remove(escapedPath) })
|
|
|
|
recorder := doLogDownload(t, testCtx, createdApp.ID, deployment.ID)
|
|
|
|
assert.Equal(t, http.StatusNotFound, recorder.Code)
|
|
assert.NotContains(t, recorder.Body.String(), sentinel,
|
|
"containment guard must not serve a file outside the log dir")
|
|
}
|