1 Commits
Author SHA1 Message Date
sneak f2ff2e3e19 Reject path traversal in deploy log download handler (closes #177)
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
2026-09-22 07:20:58 +00:00
9 changed files with 72 additions and 143 deletions
-2
View File
@@ -1,2 +0,0 @@
# Vendored, minified third-party bundles must never be reformatted.
*.min.js
+1 -5
View File
@@ -8,12 +8,8 @@ RUN go mod download
COPY . .
# golangci-lint is invoked directly here, not via `make lint`: script/lint
# now runs the linter by building Dockerfile.lint, and shelling out to
# `docker build` from inside this image build would be docker-in-docker.
# This image is golangci/golangci-lint, so the pinned linter is on PATH.
RUN make fmt-check
RUN golangci-lint run --config .golangci.yml ./...
RUN make lint
# Build stage — tests and compilation
# golang:1.25-alpine
-23
View File
@@ -1,23 +0,0 @@
# Lint image — runs golangci-lint inside a container so every lint uses
# the pinned linter, never a host binary. Linting is a build step, so a
# successful build is a clean lint. Built by script/lint.
# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07
FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240
WORKDIR /src
COPY go.mod go.sum ./
RUN go mod download
COPY . .
# Caching is waived for linting: on an unchanged tree a cached build runs
# no linter and still exits 0 in under a second. script/lint passes a
# fresh GATE_RUN every time, and referencing it here forces this step to
# re-run, so the linter always executes.
#
# `golangci-lint config verify` is deliberately NOT run: it fetches its
# JSON schema over an unpinned live HTTPS call, which REPO_POLICIES.md
# forbids (all external references must be pinned by hash).
ARG GATE_RUN
RUN echo "lint run: ${GATE_RUN}"; golangci-lint run --config .golangci.yml ./...
-15
View File
@@ -20,24 +20,9 @@ main cannot regress.
# Completed Steps
- 2026-09-22: Fixed the flaky `t.TempDir` cleanup race in
`internal/service/webhook` by tracking the async deployment goroutine
in a `sync.WaitGroup` and exposing `WaitForDeployments`; tests now
synchronize on completion instead of sleeping (#198).
- 2026-09-22: Linting now runs only in Docker. Added `Dockerfile.lint`
(pinned golangci-lint v2.12.2, cache-busted via a `GATE_RUN` build arg
so the linter always executes), reduced `script/lint` to building it,
dropped the golangci-lint install from `script/bootstrap`, and switched
the `Dockerfile` lint stage to invoke `golangci-lint` directly instead
of `make lint` to avoid docker-in-docker (#188).
- 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),
+8 -33
View File
@@ -7,7 +7,6 @@ import (
"os"
"path/filepath"
"strconv"
"strings"
"testing"
"github.com/stretchr/testify/assert"
@@ -69,53 +68,29 @@ func TestHandleDeploymentLogDownloadServesLegitimateFile(t *testing.T) {
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.
// 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 = "../.."
createdApp.Name = "../../../../etc"
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))
// 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()))
// 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")
}
+2 -15
View File
@@ -6,7 +6,6 @@ import (
"database/sql"
"fmt"
"log/slog"
"sync"
"go.uber.org/fx"
@@ -32,10 +31,6 @@ type Service struct {
db *database.Database
deploy *deploy.Service
params *ServiceParams
// deployments tracks the deployment goroutines started by
// triggerDeployment so callers can wait for them to finish.
deployments sync.WaitGroup
}
// New creates a new webhook Service.
@@ -113,14 +108,6 @@ func (svc *Service) HandleWebhook(
return nil
}
// WaitForDeployments blocks until every deployment goroutine started by
// HandleWebhook has finished, including all writes under the data
// directory. It exists so callers and tests can synchronize on async
// deployment completion instead of polling or sleeping.
func (svc *Service) WaitForDeployments() {
svc.deployments.Wait()
}
func (svc *Service) triggerDeployment(
ctx context.Context,
app *models.App,
@@ -130,7 +117,7 @@ func (svc *Service) triggerDeployment(
eventID := event.ID
appName := app.Name
svc.deployments.Go(func() {
go func() {
// Use context.WithoutCancel to ensure deployment completes
// even if the HTTP request context is cancelled.
deployCtx := context.WithoutCancel(ctx)
@@ -143,5 +130,5 @@ func (svc *Service) triggerDeployment(
// Mark event as processed
event.Processed = true
_ = event.Save(deployCtx)
})
}()
}
+7 -9
View File
@@ -7,6 +7,7 @@ import (
"os"
"path/filepath"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -827,9 +828,8 @@ func TestExtractBranch(testingT *testing.T) {
)
require.NoError(t, err)
// Wait for the async deployment goroutine to finish so its
// writes under the temp dir complete before test cleanup.
svc.WaitForDeployments()
// Allow async deployment goroutine to complete before test cleanup
time.Sleep(100 * time.Millisecond)
events, err := app.GetWebhookEvents(context.Background(), 10)
require.NoError(t, err)
@@ -867,9 +867,8 @@ func TestHandleWebhookMatchingBranch(t *testing.T) {
)
require.NoError(t, err)
// Wait for the async deployment goroutine to finish so its writes
// under the temp dir complete before test cleanup.
svc.WaitForDeployments()
// Allow async deployment goroutine to complete before test cleanup
time.Sleep(100 * time.Millisecond)
events, err := app.GetWebhookEvents(context.Background(), 10)
require.NoError(t, err)
@@ -963,9 +962,8 @@ func assertHandleWebhookDeploys(
err := svc.HandleWebhook(context.Background(), app, source, pushEventType, payload)
require.NoError(t, err)
// Wait for the async deployment goroutine to finish so its writes
// under the temp dir complete before test cleanup.
svc.WaitForDeployments()
// Allow async deployment goroutine to complete before test cleanup
time.Sleep(100 * time.Millisecond)
events, err := app.GetWebhookEvents(context.Background(), 10)
require.NoError(t, err)
+52 -27
View File
@@ -3,18 +3,18 @@
# this repo. Idempotent: every install is guarded by a check so already
# installed tools are skipped. Base tooling comes from nix, apt, brew,
# or apk (detected in that order); assumes NOTHING is present (not git,
# make, or go). 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. The linter is not installed here: it
# runs only in Docker via script/lint, so docker is its sole prerequisite.
# 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).
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
# Pinned versions. Never "latest"; exact versions only.
# 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"
# Pinned versions, 2026-08-07. Never "latest"; exact versions only.
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"
PKGMGR=""
SUDO=""
@@ -56,19 +56,52 @@ missing() {
! command -v "$1" >/dev/null 2>&1
}
# 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. Requires go, which main
# installs first.
ensure_goimports() {
if ! missing goimports; then return 0; fi
detect_pkgmgr
# verify_sha256 <file> <expected-hash>
verify_sha256() {
if command -v sha256sum >/dev/null 2>&1; then
actual="$(sha256sum "$1" | cut -d' ' -f1)"
else
actual="$(shasum -a 256 "$1" | cut -d' ' -f1)"
fi
if [ "$actual" != "$2" ]; then
echo "bootstrap: sha256 mismatch for $1" >&2
echo " expected: $2" >&2
echo " actual: $actual" >&2
exit 1
fi
}
# apt has no golangci-lint package: install a pinned release archive
# from GitHub, verified by hardcoded sha256 (never curl | sh).
install_golangci_lint_release() {
case "$(uname -m)" in
x86_64) goarch="amd64"; sha="$GOLANGCI_LINT_SHA256_AMD64" ;;
aarch64|arm64) goarch="arm64"; sha="$GOLANGCI_LINT_SHA256_ARM64" ;;
*)
echo "bootstrap: unsupported architecture $(uname -m)" >&2
exit 1
;;
esac
if missing curl; then pkg_install curl curl curl curl; fi
name="golangci-lint-${GOLANGCI_LINT_VERSION}-linux-${goarch}"
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
curl -fsSL -o "$tmp/$name.tar.gz" \
"https://github.com/golangci/golangci-lint/releases/download/v${GOLANGCI_LINT_VERSION}/${name}.tar.gz"
verify_sha256 "$tmp/$name.tar.gz" "$sha"
tar -xzf "$tmp/$name.tar.gz" -C "$tmp"
$SUDO install -m 0755 "$tmp/$name/golangci-lint" /usr/local/bin/golangci-lint
rm -rf "$tmp"
}
ensure_golangci_lint() {
if ! missing golangci-lint; then return 0; fi
detect_pkgmgr
case "$PKGMGR" in
apt) install_golangci_lint_release ;;
*) pkg_install golangci-lint golangci-lint golangci-lint golangci-lint ;;
esac
}
main() {
cd "$ROOT"
@@ -76,17 +109,9 @@ main() {
if missing git; then pkg_install git git git git; fi
if missing make; then pkg_install gnumake make make make; fi
# Go toolchain
# Go toolchain and linter
if missing go; then pkg_install go golang go go; fi
ensure_goimports
# The linter runs only in Docker (script/lint). Warn, don't fail: the
# rest of the repo works without it.
if missing docker; then
echo "bootstrap: WARNING: docker not found; make lint and" >&2
echo "bootstrap: make check require it. Install docker to run" >&2
echo "bootstrap: the linter." >&2
fi
ensure_golangci_lint
go mod download
+2 -14
View File
@@ -1,24 +1,12 @@
#!/bin/sh
# script/lint: run golangci-lint. The linter is never installed on the
# host; it runs only inside Docker, from the pinned image in
# Dockerfile.lint, so every run uses the same linter version everywhere.
# Linting is a build step there, so a successful build is a clean lint.
#
# GATE_RUN differs every run so the lint layer always executes; a cached
# build would otherwise exit 0 in under a second having linted nothing.
# --output=type=cacheonly discards the image and keeps only build cache,
# so no tagged image is left behind.
# script/lint: run the linter.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() {
cd "$ROOT"
docker build \
--build-arg GATE_RUN="$(date +%s)-$$" \
--output=type=cacheonly \
-f Dockerfile.lint \
.
golangci-lint run --config .golangci.yml ./...
}
main "$@"