Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
8f88094307 | ||
|
|
f1dfd382a4 |
@@ -22,6 +22,9 @@ main cannot regress.
|
|||||||
|
|
||||||
- 2026-09-22: Added `.prettierignore` so `make fmt` no longer rewrites
|
- 2026-09-22: Added `.prettierignore` so `make fmt` no longer rewrites
|
||||||
the vendored `static/js/alpine.min.js` bundle (#185).
|
the vendored `static/js/alpine.min.js` bundle (#185).
|
||||||
|
- 2026-09-22: Fixed the gosec G703 path-traversal finding in the deploy
|
||||||
|
log download handler by verifying the resolved path stays within the
|
||||||
|
deploy log directory before serving, returning 404 on escape (#177).
|
||||||
- 2026-09-22: `script/bootstrap` now installs a pinned `goimports`
|
- 2026-09-22: `script/bootstrap` now installs a pinned `goimports`
|
||||||
(`golang.org/x/tools` v0.49.0) into `/usr/local/bin`, so `make fmt`
|
(`golang.org/x/tools` v0.49.0) into `/usr/local/bin`, so `make fmt`
|
||||||
succeeds on a fresh machine after `make bootstrap` (#184).
|
succeeds on a fresh machine after `make bootstrap` (#184).
|
||||||
|
|||||||
@@ -611,7 +611,13 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Get the log file path from deploy service
|
// The log path is derived from request data (the app is looked
|
||||||
|
// up by a URL parameter), so open it through an os.Root confined
|
||||||
|
// to the deploy log directory. Root.Open rejects any path that
|
||||||
|
// escapes the root, so a traversal attempt fails rather than
|
||||||
|
// serving an arbitrary file.
|
||||||
|
logDir := h.deploy.GetLogDir()
|
||||||
|
|
||||||
logPath := h.deploy.GetLogFilePath(application, deployment)
|
logPath := h.deploy.GetLogFilePath(application, deployment)
|
||||||
if logPath == "" {
|
if logPath == "" {
|
||||||
http.NotFound(writer, request)
|
http.NotFound(writer, request)
|
||||||
@@ -619,28 +625,43 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if file exists — logPath is constructed internally, not from user input
|
relPath, relErr := filepath.Rel(logDir, logPath)
|
||||||
_, err := os.Stat(logPath) // #nosec G703 -- internal path, not user input
|
if relErr != nil {
|
||||||
if os.IsNotExist(err) {
|
|
||||||
http.NotFound(writer, request)
|
http.NotFound(writer, request)
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if err != nil {
|
root, rootErr := os.OpenRoot(logDir)
|
||||||
h.log.Error("failed to stat log file", "error", err, "path", logPath)
|
if rootErr != nil {
|
||||||
|
http.NotFound(writer, request)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
defer func() { _ = root.Close() }()
|
||||||
|
|
||||||
|
file, openErr := root.Open(relPath)
|
||||||
|
if openErr != nil {
|
||||||
|
http.NotFound(writer, request)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
defer func() { _ = file.Close() }()
|
||||||
|
|
||||||
|
info, statErr := file.Stat()
|
||||||
|
if statErr != nil {
|
||||||
|
h.log.Error("failed to stat log file", "error", statErr, "path", logPath)
|
||||||
http.Error(writer, "Internal Server Error", http.StatusInternalServerError)
|
http.Error(writer, "Internal Server Error", http.StatusInternalServerError)
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Extract filename for Content-Disposition header
|
|
||||||
filename := filepath.Base(logPath)
|
filename := filepath.Base(logPath)
|
||||||
|
|
||||||
writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
|
writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
|
||||||
writer.Header().Set("Content-Disposition", "attachment; filename=\""+filename+"\"")
|
writer.Header().Set("Content-Disposition", "attachment; filename=\""+filename+"\"")
|
||||||
|
|
||||||
http.ServeFile(writer, request, logPath) // #nosec G703 -- internal path
|
http.ServeContent(writer, request, filename, info.ModTime(), file)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -42,6 +42,7 @@ type testContext struct {
|
|||||||
database *database.Database
|
database *database.Database
|
||||||
authSvc *auth.Service
|
authSvc *auth.Service
|
||||||
appSvc *app.Service
|
appSvc *app.Service
|
||||||
|
deploySvc *deploy.Service
|
||||||
middleware *middleware.Middleware
|
middleware *middleware.Middleware
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -186,6 +187,7 @@ func setupTestHandlers(t *testing.T) *testContext {
|
|||||||
database: dbInstance,
|
database: dbInstance,
|
||||||
authSvc: authSvc,
|
authSvc: authSvc,
|
||||||
appSvc: appSvc,
|
appSvc: appSvc,
|
||||||
|
deploySvc: deploySvc,
|
||||||
middleware: mw,
|
middleware: mw,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,121 @@
|
|||||||
|
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")
|
||||||
|
}
|
||||||
@@ -294,6 +294,12 @@ func (svc *Service) GetLogFilePath(
|
|||||||
return filepath.Join(svc.config.DataDir, "logs", hostname, app.Name, filename)
|
return filepath.Join(svc.config.DataDir, "logs", hostname, app.Name, filename)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// GetLogDir returns the root directory under which all deployment log
|
||||||
|
// files live. Paths returned by GetLogFilePath are always inside it.
|
||||||
|
func (svc *Service) GetLogDir() string {
|
||||||
|
return filepath.Join(svc.config.DataDir, "logs")
|
||||||
|
}
|
||||||
|
|
||||||
// HasActiveDeploy returns true if there is an active deployment for the given app.
|
// HasActiveDeploy returns true if there is an active deployment for the given app.
|
||||||
func (svc *Service) HasActiveDeploy(appID string) bool {
|
func (svc *Service) HasActiveDeploy(appID string) bool {
|
||||||
_, ok := svc.activeDeploys.Load(appID)
|
_, ok := svc.activeDeploys.Load(appID)
|
||||||
|
|||||||
Reference in New Issue
Block a user