Reject path traversal in deploy log download handler (closes #177)
Check / check (pull_request) Skipped
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 and unit tests covering a legitimate download and a traversal-shaped app name. Model: opus-4-8
This commit is contained in:
@@ -0,0 +1,96 @@
|
||||
package handlers_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"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 that a
|
||||
// traversal-shaped app name (containing "..") — which would make the
|
||||
// resolved log path escape the deploy log directory — is rejected with
|
||||
// 404 rather than serving an arbitrary file.
|
||||
func TestHandleDeploymentLogDownloadRejectsPathTraversal(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
testCtx := setupTestHandlers(t)
|
||||
createdApp := createTestApp(t, testCtx, "log-traversal-app")
|
||||
|
||||
createdApp.Name = "../../../../etc"
|
||||
require.NoError(t, createdApp.Save(context.Background()))
|
||||
|
||||
// Ensure the log root exists so the rejection comes from the
|
||||
// containment check, not from a missing directory.
|
||||
require.NoError(t, os.MkdirAll(testCtx.deploySvc.GetLogDir(), 0o750))
|
||||
|
||||
deployment := models.NewDeployment(testCtx.database)
|
||||
deployment.AppID = createdApp.ID
|
||||
deployment.Status = models.DeploymentStatusSuccess
|
||||
require.NoError(t, deployment.Save(context.Background()))
|
||||
|
||||
recorder := doLogDownload(t, testCtx, createdApp.ID, deployment.ID)
|
||||
|
||||
assert.Equal(t, http.StatusNotFound, recorder.Code)
|
||||
}
|
||||
Reference in New Issue
Block a user