2 Commits
Author SHA1 Message Date
sneak 8f88094307 Install pinned goimports in script/bootstrap (closes #184)
Check / check (pull_request) Skipped
script/fmt runs gofmt, goimports, and npx prettier, but bootstrap
installed only git, make, go, and golangci-lint, so `make fmt` failed
with `goimports: not found` on a fresh machine. bootstrap's contract is
to install all dependencies idempotently.

Add ensure_goimports: it skips when goimports is already on PATH,
otherwise `go install`s golang.org/x/tools/cmd/goimports at a pinned
exact version (v0.49.0; integrity via the Go module checksum database)
and places the binary in /usr/local/bin so it is on PATH regardless of
shell config, mirroring the golangci-lint release install. v0.49.0
requires Go 1.25, matching go.mod; v0.50.0 would force a Go 1.26
toolchain download.

Model: opus-4-8
2026-09-22 09:04:33 +00:00
clawbot f1dfd382a4 Reject path traversal in deploy log download handler (closes #177)
The deploy-log download handler passed a request-derived path to
http.ServeFile, which gosec flags as G703 (path traversal via taint).
The handler now opens the log through an os.Root confined to the deploy
log directory, so any escaping path is rejected at runtime (404) and the
file is streamed with http.ServeContent. A regression test plants a
sentinel outside the log dir and asserts the traversal is refused and its
contents never served; removing the guard makes that test fail. No
//nolint used.

Model: opus-4-8
2026-09-22 11:01:07 +02:00
5 changed files with 161 additions and 8 deletions
+3
View File
@@ -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).
+29 -8
View File
@@ -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)
} }
} }
+2
View 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,
} }
} }
+121
View File
@@ -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")
}
+6
View File
@@ -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)