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
|
- 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`
|
||||||
|
(`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
|
- 2026-09-09: Fixed four deployability blockers found by QA: CSRF origin
|
||||||
check over plain HTTP (`UPAAS_PLAINTEXT_HTTP`, #189), pulling the git
|
check over plain HTTP (`UPAAS_PLAINTEXT_HTTP`, #189), pulling the git
|
||||||
image when absent (#190), the env-var editor CSRF token lookup (#191),
|
image when absent (#190), the env-var editor CSRF token lookup (#191),
|
||||||
|
|||||||
@@ -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)
|
||||||
|
|||||||
+20
-1
@@ -5,7 +5,9 @@
|
|||||||
# or apk (detected in that order); assumes NOTHING is present (not git,
|
# 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
|
# 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
|
# 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
|
set -eu
|
||||||
|
|
||||||
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
|
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
|
# sha256 of golangci-lint-2.12.2-linux-<arch>.tar.gz release archives
|
||||||
GOLANGCI_LINT_SHA256_AMD64="8df580d2670fed8fa984aac0507099af8df275e665215f5c7a2ae3943893a553"
|
GOLANGCI_LINT_SHA256_AMD64="8df580d2670fed8fa984aac0507099af8df275e665215f5c7a2ae3943893a553"
|
||||||
GOLANGCI_LINT_SHA256_ARM64="44cd40a8c76c86755375adfeea52cfd3533cb43d7bd647771e0ae065e166df3a"
|
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=""
|
PKGMGR=""
|
||||||
SUDO=""
|
SUDO=""
|
||||||
@@ -102,6 +107,19 @@ ensure_golangci_lint() {
|
|||||||
esac
|
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() {
|
main() {
|
||||||
cd "$ROOT"
|
cd "$ROOT"
|
||||||
|
|
||||||
@@ -112,6 +130,7 @@ main() {
|
|||||||
# Go toolchain and linter
|
# Go toolchain and linter
|
||||||
if missing go; then pkg_install go golang go go; fi
|
if missing go; then pkg_install go golang go go; fi
|
||||||
ensure_golangci_lint
|
ensure_golangci_lint
|
||||||
|
ensure_goimports
|
||||||
|
|
||||||
go mod download
|
go mod download
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user