Compare commits
2
Commits
8fb3f0c9b5
...
8f88094307
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
8f88094307 | ||
|
|
f1dfd382a4 |
@@ -22,6 +22,12 @@ main cannot regress.
|
||||
|
||||
- 2026-09-22: Added `.prettierignore` so `make fmt` no longer rewrites
|
||||
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`
|
||||
(`golang.org/x/tools` v0.49.0) into `/usr/local/bin`, so `make fmt`
|
||||
succeeds on a fresh machine after `make bootstrap` (#184).
|
||||
- 2026-09-09: Fixed four deployability blockers found by QA: CSRF origin
|
||||
check over plain HTTP (`UPAAS_PLAINTEXT_HTTP`, #189), pulling the git
|
||||
image when absent (#190), the env-var editor CSRF token lookup (#191),
|
||||
|
||||
@@ -611,7 +611,13 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc {
|
||||
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)
|
||||
if logPath == "" {
|
||||
http.NotFound(writer, request)
|
||||
@@ -619,28 +625,43 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc {
|
||||
return
|
||||
}
|
||||
|
||||
// Check if file exists — logPath is constructed internally, not from user input
|
||||
_, err := os.Stat(logPath) // #nosec G703 -- internal path, not user input
|
||||
if os.IsNotExist(err) {
|
||||
relPath, relErr := filepath.Rel(logDir, logPath)
|
||||
if relErr != nil {
|
||||
http.NotFound(writer, request)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
h.log.Error("failed to stat log file", "error", err, "path", logPath)
|
||||
root, rootErr := os.OpenRoot(logDir)
|
||||
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)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// Extract filename for Content-Disposition header
|
||||
filename := filepath.Base(logPath)
|
||||
|
||||
writer.Header().Set("Content-Type", "text/plain; charset=utf-8")
|
||||
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
|
||||
authSvc *auth.Service
|
||||
appSvc *app.Service
|
||||
deploySvc *deploy.Service
|
||||
middleware *middleware.Middleware
|
||||
}
|
||||
|
||||
@@ -186,6 +187,7 @@ func setupTestHandlers(t *testing.T) *testContext {
|
||||
database: dbInstance,
|
||||
authSvc: authSvc,
|
||||
appSvc: appSvc,
|
||||
deploySvc: deploySvc,
|
||||
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)
|
||||
}
|
||||
|
||||
// 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.
|
||||
func (svc *Service) HasActiveDeploy(appID string) bool {
|
||||
_, ok := svc.activeDeploys.Load(appID)
|
||||
|
||||
+20
-1
@@ -5,7 +5,9 @@
|
||||
# or apk (detected in that order); assumes NOTHING is present (not git,
|
||||
# make, or go). golangci-lint is packaged in nix, brew, and apk; on apt
|
||||
# it is installed from a hash-verified GitHub release archive (never
|
||||
# curl | sh).
|
||||
# curl | sh). goimports is installed with `go install` at a pinned
|
||||
# version (integrity via the Go module checksum database) into
|
||||
# /usr/local/bin so it is on PATH.
|
||||
set -eu
|
||||
|
||||
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
||||
@@ -15,6 +17,9 @@ GOLANGCI_LINT_VERSION="2.12.2"
|
||||
# sha256 of golangci-lint-2.12.2-linux-<arch>.tar.gz release archives
|
||||
GOLANGCI_LINT_SHA256_AMD64="8df580d2670fed8fa984aac0507099af8df275e665215f5c7a2ae3943893a553"
|
||||
GOLANGCI_LINT_SHA256_ARM64="44cd40a8c76c86755375adfeea52cfd3533cb43d7bd647771e0ae065e166df3a"
|
||||
# golang.org/x/tools goimports, 2026-08-13. v0.49.0 requires Go 1.25 (matches
|
||||
# go.mod); v0.50.0 needs Go 1.26. Integrity via the Go module checksum database.
|
||||
GOIMPORTS_VERSION="v0.49.0"
|
||||
|
||||
PKGMGR=""
|
||||
SUDO=""
|
||||
@@ -102,6 +107,19 @@ ensure_golangci_lint() {
|
||||
esac
|
||||
}
|
||||
|
||||
# goimports is not packaged uniformly across nix/apt/brew/apk, so install it
|
||||
# with `go install` at a pinned version and place the binary in /usr/local/bin
|
||||
# so it is on PATH regardless of shell config, as the golangci-lint release
|
||||
# install does. Requires go, which main installs first.
|
||||
ensure_goimports() {
|
||||
if ! missing goimports; then return 0; fi
|
||||
detect_pkgmgr
|
||||
tmp="$(mktemp -d)"
|
||||
GOBIN="$tmp" go install "golang.org/x/tools/cmd/goimports@${GOIMPORTS_VERSION}"
|
||||
$SUDO install -m 0755 "$tmp/goimports" /usr/local/bin/goimports
|
||||
rm -rf "$tmp"
|
||||
}
|
||||
|
||||
main() {
|
||||
cd "$ROOT"
|
||||
|
||||
@@ -112,6 +130,7 @@ main() {
|
||||
# Go toolchain and linter
|
||||
if missing go; then pkg_install go golang go go; fi
|
||||
ensure_golangci_lint
|
||||
ensure_goimports
|
||||
|
||||
go mod download
|
||||
|
||||
|
||||
Reference in New Issue
Block a user