Author SHA1 Message Date
sneak b580dbbd2c fix: resolve all 22 gosec G710 open-redirect findings
Check / check (pull_request) Successful in 3m25s
Route every app redirect in internal/handlers/app.go through a new
redirectToApp helper that parses the app ID with ulid.ParseStrict
(404 on failure), re-serializes it, and path-escapes it before
building the /apps/<id> target, so no unvalidated request input can
reach http.Redirect. Also converts the equivalent unflagged redirect
in HandleAppCreate for consistency.

make lint under golangci-lint 2.12.2 drops from 47 findings to 25
(remaining: 1 gosec G703 tracked in #177, 24 goconst tracked in
#178). make test and make fmt-check pass unchanged.

Closes #176
2026-08-07 16:58:10 +00:00
sneak 21642900e6 fix: resolve all 47 noctx lint findings in tests
Check / check (pull_request) Successful in 3m10s
Replace every httptest.NewRequest call with
httptest.NewRequestWithContext using the test's t.Context(). Thread
t *testing.T through the createSetupFormRequest and
createLoginFormRequest helpers so they can supply a context.

make lint under golangci-lint 2.12.2 drops from 94 findings to 47
(remaining: 23 gosec, 24 goconst, tracked in #176/#177/#178). make
test and make fmt-check pass unchanged.

Closes #175
2026-08-07 16:47:16 +00:00
51 changed files with 833 additions and 1495 deletions
+12 -14
View File
@@ -1,9 +1,5 @@
version: "2" version: "2"
# Config schema uses the golangci-lint v2 layout (settings live under
# linters.settings, not top-level linters-settings) so that the
# thresholds below are actually applied by golangci-lint >= v2.
run: run:
timeout: 5m timeout: 5m
modules-download-mode: readonly modules-download-mode: readonly
@@ -18,17 +14,19 @@ linters:
- wsl # Deprecated, replaced by wsl_v5 - wsl # Deprecated, replaced by wsl_v5
- wrapcheck # Too verbose for internal packages - wrapcheck # Too verbose for internal packages
- varnamelen # Short names like db, id are idiomatic Go - varnamelen # Short names like db, id are idiomatic Go
settings:
lll: linters-settings:
line-length: 88 lll:
funlen: line-length: 88
lines: 80 funlen:
statements: 50 lines: 80
cyclop: statements: 50
max-complexity: 15 cyclop:
dupl: max-complexity: 15
threshold: 100 dupl:
threshold: 100
issues: issues:
exclude-use-default: false
max-issues-per-linter: 0 max-issues-per-linter: 0
max-same-issues: 0 max-same-issues: 0
-2
View File
@@ -1,2 +0,0 @@
# Vendored, minified third-party bundles must never be reformatted.
*.min.js
-4
View File
@@ -1,4 +0,0 @@
{
"tabWidth": 4,
"proseWrap": "always"
}
+3 -7
View File
@@ -1,6 +1,6 @@
# Lint stage — fast feedback on formatting and lint issues # Lint stage — fast feedback on formatting and lint issues
# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07 # golangci/golangci-lint:v2.10.1
FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS lint FROM golangci/golangci-lint@sha256:ea84d14c2fef724411be7dc45e09e6ef721d748315252b02df19a7e3113ee763 AS lint
WORKDIR /src WORKDIR /src
COPY go.mod go.sum ./ COPY go.mod go.sum ./
@@ -8,12 +8,8 @@ RUN go mod download
COPY . . 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 make fmt-check
RUN golangci-lint run --config .golangci.yml ./... RUN make lint
# Build stage — tests and compilation # Build stage — tests and compilation
# golang:1.25-alpine # 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 ./...
-8
View File
@@ -191,7 +191,6 @@ Environment variables:
| `UPAAS_DATA_DIR` | Data directory for SQLite and keys | `./data` (local dev only — use absolute path for Docker) | | `UPAAS_DATA_DIR` | Data directory for SQLite and keys | `./data` (local dev only — use absolute path for Docker) |
| `UPAAS_HOST_DATA_DIR` | Host path for DATA_DIR (when running in container) | *(none — must be set to an absolute path)* | | `UPAAS_HOST_DATA_DIR` | Host path for DATA_DIR (when running in container) | *(none — must be set to an absolute path)* |
| `UPAAS_DOCKER_HOST` | Docker socket path | unix:///var/run/docker.sock | | `UPAAS_DOCKER_HOST` | Docker socket path | unix:///var/run/docker.sock |
| `UPAAS_PLAINTEXT_HTTP` | Set when µPaaS is reached over plain HTTP (no TLS-terminating proxy in front) so CSRF origin checks use `http://`. Leave unset behind a TLS-terminating reverse proxy. | false |
| `DEBUG` | Enable debug logging | false | | `DEBUG` | Enable debug logging | false |
| `SENTRY_DSN` | Sentry error reporting DSN | "" | | `SENTRY_DSN` | Sentry error reporting DSN | "" |
| `METRICS_USERNAME` | Basic auth for /metrics | "" | | `METRICS_USERNAME` | Basic auth for /metrics | "" |
@@ -205,14 +204,9 @@ docker run -d \
-v /var/run/docker.sock:/var/run/docker.sock \ -v /var/run/docker.sock:/var/run/docker.sock \
-v /path/on/host/upaas-data:/var/lib/upaas \ -v /path/on/host/upaas-data:/var/lib/upaas \
-e UPAAS_HOST_DATA_DIR=/path/on/host/upaas-data \ -e UPAAS_HOST_DATA_DIR=/path/on/host/upaas-data \
-e UPAAS_PLAINTEXT_HTTP=true \
upaas upaas
``` ```
This recipe serves plain HTTP, so `UPAAS_PLAINTEXT_HTTP=true` is required for
setup and every other form to pass the CSRF origin check. Behind a
TLS-terminating reverse proxy, drop that line.
### Docker Compose ### Docker Compose
```yaml ```yaml
@@ -227,8 +221,6 @@ services:
- ${HOST_DATA_DIR}:/var/lib/upaas - ${HOST_DATA_DIR}:/var/lib/upaas
environment: environment:
- UPAAS_HOST_DATA_DIR=${HOST_DATA_DIR} - UPAAS_HOST_DATA_DIR=${HOST_DATA_DIR}
# Set when serving plain HTTP (no TLS-terminating proxy); drop behind one
- UPAAS_PLAINTEXT_HTTP=true
# Optional: uncomment to enable debug logging # Optional: uncomment to enable debug logging
# - DEBUG=true # - DEBUG=true
# Optional: Sentry error reporting # Optional: Sentry error reporting
+24 -34
View File
@@ -10,46 +10,26 @@
# Status # Status
1.0+. Tagged 1.0.0 on 2026-02-26; 8 commits on main since. `make check` 1.0+. Tagged 1.0.0 on 2026-02-26. Policy violation: main currently
is green as of the golangci-lint v2.12.2 update. fails make check under golangci-lint >= 2.12 (25 lint issues remaining:
1 gosec G703, 24 goconst), so the tree is out of compliance until
fixed. CI (Dockerfile lint stage, pinned golangci-lint v2.10.1) is
green; the pin bump is tracked in issue #179. The road to release
1.1.0 is tracked in Gitea issues #175-#185 (milestone 1.1.0).
# Next Step # Next Step
Confirm `.gitea/workflows/check.yml` gates merges on `make check` so Fix the gosec G703 path traversal finding in the deploy log download
main cannot regress. handler (issue #177): canonicalize and containment-check the log path
before http.ServeFile, with a traversal-rejection test.
# Completed Steps # Completed Steps
- 2026-09-22: Added a root `.prettierrc` (`tabWidth: 4`, - 2026-08-07: Fixed all 22 gosec G710 open-redirect findings: app
`proseWrap: always`) pinning the project prettier settings, and redirects go through a redirectToApp helper that ULID-validates the
dropped the now-redundant inline `--tab-width 4` from `script/fmt` so app ID (#176).
the config file is the single source of truth (#197). - 2026-08-07: Fixed all 47 noctx lint findings: tests now use
- 2026-09-22: Fixed the flaky `t.TempDir` cleanup race in httptest.NewRequestWithContext with t.Context() (#175).
`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),
and the port-mapping delete form's CSRF field (#192).
- 2026-08-07: Updated golangci-lint to v2.12.2 (canonical
`.golangci.yml`, `Dockerfile` lint stage pin, `script/bootstrap`
release-archive pins) and fixed all resulting lint findings (noctx,
gosec, goconst, lll, dupl, nolintlint); `make check` green.
- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints, - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
Makefile shims, README Entrypoints section Makefile shims, README Entrypoints section
- 2026-03-11: Monolithic env var editing with bulk save (#158). - 2026-03-11: Monolithic env var editing with bulk save (#158).
@@ -72,4 +52,14 @@ main cannot regress.
# Future Steps # Future Steps
- Get main green (compliance, ordered):
- Fix 1 gosec G703 finding (Next Step, #177).
- Fix 24 goconst findings (#178).
- Bump Dockerfile golangci-lint pin to v2.12.x (#179).
- Run make check clean on main and keep it green; main must always
pass.
- Confirm .gitea/workflows/check.yml gates merges on make check so main
cannot regress (#180).
- Deploy to fsn1app1 and verify end-to-end (#181), then tag 1.1.0
(#182).
- Resume feature work only after main is green. - Resume feature work only after main is green.
+1 -4
View File
@@ -45,11 +45,10 @@ type Config struct {
Port int Port int
Debug bool Debug bool
DataDir string DataDir string
HostDataDir string // Host path for DataDir (Docker bind mounts in container) HostDataDir string // Host path for DataDir (for Docker bind mounts when running in container)
DockerHost string DockerHost string
SentryDSN string SentryDSN string
MaintenanceMode bool MaintenanceMode bool
PlaintextHTTP bool // clients reach µPaaS over plain HTTP (no TLS-terminating proxy)
MetricsUsername string MetricsUsername string
MetricsPassword string MetricsPassword string
SessionSecret string `json:"-"` SessionSecret string `json:"-"`
@@ -101,7 +100,6 @@ func setupViper(name string) {
viper.SetDefault("DOCKER_HOST", "unix:///var/run/docker.sock") viper.SetDefault("DOCKER_HOST", "unix:///var/run/docker.sock")
viper.SetDefault("SENTRY_DSN", "") viper.SetDefault("SENTRY_DSN", "")
viper.SetDefault("MAINTENANCE_MODE", false) viper.SetDefault("MAINTENANCE_MODE", false)
viper.SetDefault("PLAINTEXT_HTTP", false)
viper.SetDefault("METRICS_USERNAME", "") viper.SetDefault("METRICS_USERNAME", "")
viper.SetDefault("METRICS_PASSWORD", "") viper.SetDefault("METRICS_PASSWORD", "")
viper.SetDefault("SESSION_SECRET", "") viper.SetDefault("SESSION_SECRET", "")
@@ -137,7 +135,6 @@ func buildConfig(log *slog.Logger, params *Params) (*Config, error) {
DockerHost: viper.GetString("DOCKER_HOST"), DockerHost: viper.GetString("DOCKER_HOST"),
SentryDSN: viper.GetString("SENTRY_DSN"), SentryDSN: viper.GetString("SENTRY_DSN"),
MaintenanceMode: viper.GetBool("MAINTENANCE_MODE"), MaintenanceMode: viper.GetBool("MAINTENANCE_MODE"),
PlaintextHTTP: viper.GetBool("PLAINTEXT_HTTP"),
MetricsUsername: viper.GetString("METRICS_USERNAME"), MetricsUsername: viper.GetString("METRICS_USERNAME"),
MetricsPassword: viper.GetString("METRICS_PASSWORD"), MetricsPassword: viper.GetString("METRICS_PASSWORD"),
SessionSecret: viper.GetString("SESSION_SECRET"), SessionSecret: viper.GetString("SESSION_SECRET"),
+1 -2
View File
@@ -178,8 +178,7 @@ func HashWebhookSecret(secret string) string {
func (d *Database) backfillWebhookSecretHashes(ctx context.Context) error { func (d *Database) backfillWebhookSecretHashes(ctx context.Context) error {
rows, err := d.database.QueryContext(ctx, rows, err := d.database.QueryContext(ctx,
"SELECT id, webhook_secret FROM apps"+ "SELECT id, webhook_secret FROM apps WHERE webhook_secret_hash = '' AND webhook_secret != ''")
" WHERE webhook_secret_hash = '' AND webhook_secret != ''")
if err != nil { if err != nil {
return fmt.Errorf("querying apps for backfill: %w", err) return fmt.Errorf("querying apps for backfill: %w", err)
} }
+3 -14
View File
@@ -32,10 +32,7 @@ var ErrInvalidMigrationFilename = errors.New("invalid migration filename")
func ParseMigrationVersion(filename string) (int, error) { func ParseMigrationVersion(filename string) (int, error) {
name := strings.TrimSuffix(filename, ".sql") name := strings.TrimSuffix(filename, ".sql")
if name == "" || name == filename { if name == "" || name == filename {
return 0, fmt.Errorf( return 0, fmt.Errorf("%w: %q has no .sql extension or is empty", ErrInvalidMigrationFilename, filename)
"%w: %q has no .sql extension or is empty",
ErrInvalidMigrationFilename, filename,
)
} }
// Split on underscore to separate version from description. // Split on underscore to separate version from description.
@@ -43,10 +40,7 @@ func ParseMigrationVersion(filename string) (int, error) {
versionStr, _, _ := strings.Cut(name, "_") versionStr, _, _ := strings.Cut(name, "_")
if versionStr == "" { if versionStr == "" {
return 0, fmt.Errorf( return 0, fmt.Errorf("%w: %q has empty version prefix", ErrInvalidMigrationFilename, filename)
"%w: %q has empty version prefix",
ErrInvalidMigrationFilename, filename,
)
} }
// Validate the version is purely numeric. // Validate the version is purely numeric.
@@ -183,12 +177,7 @@ func ApplyMigrations(ctx context.Context, db *sql.DB, log *slog.Logger) error {
// applyMigrationTx reads and executes a migration file within a transaction, // applyMigrationTx reads and executes a migration file within a transaction,
// recording the version in schema_migrations on success. // recording the version in schema_migrations on success.
func applyMigrationTx( func applyMigrationTx(ctx context.Context, db *sql.DB, filename string, version int) error {
ctx context.Context,
db *sql.DB,
filename string,
version int,
) error {
content, err := migrationsFS.ReadFile("migrations/" + filename) content, err := migrationsFS.ReadFile("migrations/" + filename)
if err != nil { if err != nil {
return fmt.Errorf("failed to read migration %s: %w", filename, err) return fmt.Errorf("failed to read migration %s: %w", filename, err)
+13 -81
View File
@@ -41,8 +41,7 @@ const stopTimeoutSeconds = 10
// gitImage is the Docker image used for git operations. // gitImage is the Docker image used for git operations.
// alpine/git v2.47.2 - pulled 2025-12-30 // alpine/git v2.47.2 - pulled 2025-12-30
const gitImage = "alpine/git@sha256:" + const gitImage = "alpine/git@sha256:d86f367afb53d022acc4377741e7334bc20add161bb10234272b91b459b4b7d8"
"d86f367afb53d022acc4377741e7334bc20add161bb10234272b91b459b4b7d8"
// ErrNotConnected is returned when Docker client is not connected. // ErrNotConnected is returned when Docker client is not connected.
var ErrNotConnected = errors.New("docker client not connected") var ErrNotConnected = errors.New("docker client not connected")
@@ -146,7 +145,7 @@ type CreateContainerOptions struct {
Volumes []VolumeMount Volumes []VolumeMount
Ports []PortMapping Ports []PortMapping
Network string Network string
CPULimit float64 // CPU cores (0.5 = half a core). 0 means unlimited. CPULimit float64 // CPU cores (e.g. 0.5 = half a core, 2.0 = two cores). 0 means unlimited.
MemoryLimit int64 // Memory in bytes. 0 means unlimited. MemoryLimit int64 // Memory in bytes. 0 means unlimited.
} }
@@ -304,11 +303,7 @@ func (c *Client) StopContainer(ctx context.Context, containerID ContainerID) err
timeout := stopTimeoutSeconds timeout := stopTimeoutSeconds
err := c.docker.ContainerStop( err := c.docker.ContainerStop(ctx, containerID.String(), container.StopOptions{Timeout: &timeout})
ctx,
containerID.String(),
container.StopOptions{Timeout: &timeout},
)
if err != nil { if err != nil {
return fmt.Errorf("failed to stop container: %w", err) return fmt.Errorf("failed to stop container: %w", err)
} }
@@ -328,11 +323,7 @@ func (c *Client) RemoveContainer(
c.log.Info("removing container", "id", containerID, "force", force) c.log.Info("removing container", "id", containerID, "force", force)
err := c.docker.ContainerRemove( err := c.docker.ContainerRemove(ctx, containerID.String(), container.RemoveOptions{Force: force})
ctx,
containerID.String(),
container.RemoveOptions{Force: force},
)
if err != nil { if err != nil {
return fmt.Errorf("failed to remove container: %w", err) return fmt.Errorf("failed to remove container: %w", err)
} }
@@ -478,8 +469,7 @@ type CloneResult struct {
CommitSHA string // The HEAD commit SHA after clone/checkout CommitSHA string // The HEAD commit SHA after clone/checkout
} }
// CloneRepo clones a git repository using SSH and optionally checks out a // CloneRepo clones a git repository using SSH and optionally checks out a specific commit.
// specific commit.
// containerDir is the path inside the upaas container (for writing files). // containerDir is the path inside the upaas container (for writing files).
// hostDir is the corresponding path on the Docker host (for bind mounts). // hostDir is the corresponding path on the Docker host (for bind mounts).
// If commitSHA is provided, that specific commit will be checked out. // If commitSHA is provided, that specific commit will be checked out.
@@ -594,13 +584,11 @@ func (c *Client) performBuild(
// scannerInitialBufferSize is the initial buffer size for the build log scanner. // scannerInitialBufferSize is the initial buffer size for the build log scanner.
const scannerInitialBufferSize = 64 * 1024 // 64KB const scannerInitialBufferSize = 64 * 1024 // 64KB
// scannerMaxBufferSize is the max buffer size for build log lines // scannerMaxBufferSize is the max buffer size for build log lines (base64 layers can be large).
// (base64 layers can be large).
const scannerMaxBufferSize = 1024 * 1024 // 1MB const scannerMaxBufferSize = 1024 * 1024 // 1MB
// streamBuildOutput reads Docker build output line by line and writes to // streamBuildOutput reads Docker build output line by line and writes to stdout and optional log writer.
// stdout and optional log writer. Docker sends newline-delimited JSON, so // Docker sends newline-delimited JSON, so reading line by line ensures each log entry is written immediately.
// reading line by line ensures each log entry is written immediately.
func (c *Client) streamBuildOutput(body io.Reader, logWriter io.Writer) error { func (c *Client) streamBuildOutput(body io.Reader, logWriter io.Writer) error {
scanner := bufio.NewScanner(body) scanner := bufio.NewScanner(body)
buf := make([]byte, 0, scannerInitialBufferSize) buf := make([]byte, 0, scannerInitialBufferSize)
@@ -628,10 +616,7 @@ func (c *Client) streamBuildOutput(body io.Reader, logWriter io.Writer) error {
return nil return nil
} }
func (c *Client) performClone( func (c *Client) performClone(ctx context.Context, cfg *cloneConfig) (*CloneResult, error) {
ctx context.Context,
cfg *cloneConfig,
) (*CloneResult, error) {
// Create work directory for clone destination // Create work directory for clone destination
err := os.MkdirAll(cfg.containerDir, workDirPermissions) err := os.MkdirAll(cfg.containerDir, workDirPermissions)
if err != nil { if err != nil {
@@ -657,61 +642,16 @@ func (c *Client) performClone(
} }
defer func() { defer func() {
_ = c.docker.ContainerRemove( _ = c.docker.ContainerRemove(ctx, gitContainerID.String(), container.RemoveOptions{Force: true})
ctx,
gitContainerID.String(),
container.RemoveOptions{Force: true},
)
}() }()
return c.runGitClone(ctx, gitContainerID) return c.runGitClone(ctx, gitContainerID)
} }
// ensureImage pulls ref if it is not already present locally. The pinned
// digest is preserved: a pull of an image already present is a no-op, and a
// missing one is fetched before it is used to create a container.
func (c *Client) ensureImage(ctx context.Context, ref string) error {
_, _, err := c.docker.ImageInspectWithRaw(ctx, ref)
if err == nil {
return nil
}
if !client.IsErrNotFound(err) {
return fmt.Errorf("failed to inspect image %s: %w", ref, err)
}
c.log.Info("pulling image", "image", ref)
reader, err := c.docker.ImagePull(ctx, ref, image.PullOptions{})
if err != nil {
return fmt.Errorf("failed to pull image %s: %w", ref, err)
}
defer func() {
closeErr := reader.Close()
if closeErr != nil {
c.log.Error("failed to close image pull reader", "error", closeErr)
}
}()
// The pull only completes once its response stream is fully drained.
_, err = io.Copy(io.Discard, reader)
if err != nil {
return fmt.Errorf("failed to pull image %s: %w", ref, err)
}
return nil
}
func (c *Client) createGitContainer( func (c *Client) createGitContainer(
ctx context.Context, ctx context.Context,
cfg *cloneConfig, cfg *cloneConfig,
) (ContainerID, error) { ) (ContainerID, error) {
err := c.ensureImage(ctx, gitImage)
if err != nil {
return "", err
}
gitSSHCmd := "ssh -i /keys/deploy_key -o StrictHostKeyChecking=no" gitSSHCmd := "ssh -i /keys/deploy_key -o StrictHostKeyChecking=no"
// Build the git command using environment variables to avoid shell injection. // Build the git command using environment variables to avoid shell injection.
@@ -740,8 +680,7 @@ func (c *Client) createGitContainer(
entrypoint := []string{} entrypoint := []string{}
cmd := []string{"sh", "-c", script} cmd := []string{"sh", "-c", script}
// Use host paths for Docker bind mounts // Use host paths for Docker bind mounts (Docker runs on the host, not in our container)
// (Docker runs on the host, not in our container)
resp, err := c.docker.ContainerCreate(ctx, resp, err := c.docker.ContainerCreate(ctx,
&container.Config{ &container.Config{
Image: gitImage, Image: gitImage,
@@ -772,20 +711,13 @@ func (c *Client) createGitContainer(
return ContainerID(resp.ID), nil return ContainerID(resp.ID), nil
} }
func (c *Client) runGitClone( func (c *Client) runGitClone(ctx context.Context, containerID ContainerID) (*CloneResult, error) {
ctx context.Context,
containerID ContainerID,
) (*CloneResult, error) {
err := c.docker.ContainerStart(ctx, containerID.String(), container.StartOptions{}) err := c.docker.ContainerStart(ctx, containerID.String(), container.StartOptions{})
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to start git container: %w", err) return nil, fmt.Errorf("failed to start git container: %w", err)
} }
statusCh, errCh := c.docker.ContainerWait( statusCh, errCh := c.docker.ContainerWait(ctx, containerID.String(), container.WaitConditionNotRunning)
ctx,
containerID.String(),
container.WaitConditionNotRunning,
)
select { select {
case err := <-errCh: case err := <-errCh:
+6 -9
View File
@@ -6,14 +6,11 @@ import (
"testing" "testing"
) )
// mainBranch is the branch name used across validation tests.
const mainBranch = "main"
func TestValidBranchRegex(t *testing.T) { func TestValidBranchRegex(t *testing.T) {
t.Parallel() t.Parallel()
valid := []string{ valid := []string{
mainBranch, "main",
"develop", "develop",
"feature/my-feature", "feature/my-feature",
"release-1.0", "release-1.0",
@@ -73,7 +70,7 @@ func TestValidCommitSHARegex(t *testing.T) {
} }
} }
func TestCloneRepoRejectsInjection(t *testing.T) { func TestCloneRepoRejectsInjection(t *testing.T) { //nolint:funlen // table-driven test
t.Parallel() t.Parallel()
c := &Client{ c := &Client{
@@ -103,25 +100,25 @@ func TestCloneRepoRejectsInjection(t *testing.T) {
}, },
{ {
name: "injection in commitSHA", name: "injection in commitSHA",
branch: mainBranch, branch: "main",
commitSHA: "not-a-sha; rm -rf /", commitSHA: "not-a-sha; rm -rf /",
wantErr: ErrInvalidCommitSHA, wantErr: ErrInvalidCommitSHA,
}, },
{ {
name: "short SHA rejected", name: "short SHA rejected",
branch: mainBranch, branch: "main",
commitSHA: "abc123", commitSHA: "abc123",
wantErr: ErrInvalidCommitSHA, wantErr: ErrInvalidCommitSHA,
}, },
{ {
name: "valid inputs pass validation (hit NotConnected)", name: "valid inputs pass validation (hit NotConnected)",
branch: mainBranch, branch: "main",
commitSHA: "abc123def456789012345678901234567890abcd", commitSHA: "abc123def456789012345678901234567890abcd",
wantErr: ErrNotConnected, wantErr: ErrNotConnected,
}, },
{ {
name: "valid branch no SHA passes validation (hit NotConnected)", name: "valid branch no SHA passes validation (hit NotConnected)",
branch: mainBranch, branch: "main",
wantErr: ErrNotConnected, wantErr: ErrNotConnected,
}, },
} }
+10 -10
View File
@@ -84,7 +84,7 @@ func (h *Handlers) HandleAPILoginPOST() http.HandlerFunc {
decodeErr := json.NewDecoder(request.Body).Decode(&req) decodeErr := json.NewDecoder(request.Body).Decode(&req)
if decodeErr != nil { if decodeErr != nil {
h.respondJSON(writer, request, h.respondJSON(writer, request,
map[string]string{jsonKeyError: "invalid JSON body"}, map[string]string{"error": "invalid JSON body"},
http.StatusBadRequest) http.StatusBadRequest)
return return
@@ -95,7 +95,7 @@ func (h *Handlers) HandleAPILoginPOST() http.HandlerFunc {
if username == "" || credential == "" { if username == "" || credential == "" {
h.respondJSON(writer, request, h.respondJSON(writer, request,
map[string]string{jsonKeyError: "username and password are required"}, map[string]string{"error": "username and password are required"},
http.StatusBadRequest) http.StatusBadRequest)
return return
@@ -104,7 +104,7 @@ func (h *Handlers) HandleAPILoginPOST() http.HandlerFunc {
user, authErr := h.auth.Authenticate(request.Context(), username, credential) user, authErr := h.auth.Authenticate(request.Context(), username, credential)
if authErr != nil { if authErr != nil {
h.respondJSON(writer, request, h.respondJSON(writer, request,
map[string]string{jsonKeyError: "invalid credentials"}, map[string]string{"error": "invalid credentials"},
http.StatusUnauthorized) http.StatusUnauthorized)
return return
@@ -114,7 +114,7 @@ func (h *Handlers) HandleAPILoginPOST() http.HandlerFunc {
if sessionErr != nil { if sessionErr != nil {
h.log.Error("api: failed to create session", "error", sessionErr) h.log.Error("api: failed to create session", "error", sessionErr)
h.respondJSON(writer, request, h.respondJSON(writer, request,
map[string]string{jsonKeyError: "failed to create session"}, map[string]string{"error": "failed to create session"},
http.StatusInternalServerError) http.StatusInternalServerError)
return return
@@ -133,7 +133,7 @@ func (h *Handlers) HandleAPIListApps() http.HandlerFunc {
apps, err := h.appService.ListApps(request.Context()) apps, err := h.appService.ListApps(request.Context())
if err != nil { if err != nil {
h.respondJSON(writer, request, h.respondJSON(writer, request,
map[string]string{jsonKeyError: "failed to list apps"}, map[string]string{"error": "failed to list apps"},
http.StatusInternalServerError) http.StatusInternalServerError)
return return
@@ -156,7 +156,7 @@ func (h *Handlers) HandleAPIGetApp() http.HandlerFunc {
application, err := h.appService.GetApp(request.Context(), appID) application, err := h.appService.GetApp(request.Context(), appID)
if err != nil { if err != nil {
h.respondJSON(writer, request, h.respondJSON(writer, request,
map[string]string{jsonKeyError: "internal server error"}, map[string]string{"error": "internal server error"},
http.StatusInternalServerError) http.StatusInternalServerError)
return return
@@ -164,7 +164,7 @@ func (h *Handlers) HandleAPIGetApp() http.HandlerFunc {
if application == nil { if application == nil {
h.respondJSON(writer, request, h.respondJSON(writer, request,
map[string]string{jsonKeyError: "app not found"}, map[string]string{"error": "app not found"},
http.StatusNotFound) http.StatusNotFound)
return return
@@ -185,7 +185,7 @@ func (h *Handlers) HandleAPIListDeployments() http.HandlerFunc {
application, err := h.appService.GetApp(request.Context(), appID) application, err := h.appService.GetApp(request.Context(), appID)
if err != nil || application == nil { if err != nil || application == nil {
h.respondJSON(writer, request, h.respondJSON(writer, request,
map[string]string{jsonKeyError: "app not found"}, map[string]string{"error": "app not found"},
http.StatusNotFound) http.StatusNotFound)
return return
@@ -205,7 +205,7 @@ func (h *Handlers) HandleAPIListDeployments() http.HandlerFunc {
) )
if deployErr != nil { if deployErr != nil {
h.respondJSON(writer, request, h.respondJSON(writer, request,
map[string]string{jsonKeyError: "failed to list deployments"}, map[string]string{"error": "failed to list deployments"},
http.StatusInternalServerError) http.StatusInternalServerError)
return return
@@ -231,7 +231,7 @@ func (h *Handlers) HandleAPIWhoAmI() http.HandlerFunc {
user, err := h.auth.GetCurrentUser(request.Context(), request) user, err := h.auth.GetCurrentUser(request.Context(), request)
if err != nil || user == nil { if err != nil || user == nil {
h.respondJSON(writer, request, h.respondJSON(writer, request,
map[string]string{jsonKeyError: "unauthorized"}, map[string]string{"error": "unauthorized"},
http.StatusUnauthorized) http.StatusUnauthorized)
return return
+125 -164
View File
@@ -15,8 +15,8 @@ import (
"time" "time"
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
"github.com/oklog/ulid/v2"
"sneak.berlin/go/upaas/internal/database"
"sneak.berlin/go/upaas/internal/models" "sneak.berlin/go/upaas/internal/models"
"sneak.berlin/go/upaas/internal/service/app" "sneak.berlin/go/upaas/internal/service/app"
"sneak.berlin/go/upaas/templates" "sneak.berlin/go/upaas/templates"
@@ -29,21 +29,25 @@ const (
deploymentsHistoryLimit = 50 deploymentsHistoryLimit = 50
) )
// redirectToApp issues a SeeOther redirect to the page for the given // redirectToApp issues a See Other redirect to the detail page of the
// app ID, with an optional suffix such as "/deployments" or // given app, plus an optional suffix (a sub-path like "/deployments"
// "?success=updated". The ID is path-escaped so the target is always // or a query string like "?success=updated"). App IDs are ULIDs: the
// a relative application URL. // ID is parsed and re-serialized so the redirect target never
// contains unvalidated request input; an invalid ID yields 404.
func redirectToApp( func redirectToApp(
writer http.ResponseWriter, writer http.ResponseWriter,
request *http.Request, request *http.Request,
appID, suffix string, appID, suffix string,
) { ) {
http.Redirect( id, parseErr := ulid.ParseStrict(appID)
writer, if parseErr != nil {
request, http.NotFound(writer, request)
"/apps/"+url.PathEscape(appID)+suffix,
http.StatusSeeOther, return
) }
target := "/apps/" + url.PathEscape(id.String()) + suffix
http.Redirect(writer, request, target, http.StatusSeeOther)
} }
// HandleAppNew returns the new app form handler. // HandleAppNew returns the new app form handler.
@@ -58,9 +62,7 @@ func (h *Handlers) HandleAppNew() http.HandlerFunc {
} }
// HandleAppCreate handles app creation. // HandleAppCreate handles app creation.
// func (h *Handlers) HandleAppCreate() http.HandlerFunc { //nolint:funlen // validation adds necessary length
//nolint:funlen // validation adds necessary length
func (h *Handlers) HandleAppCreate() http.HandlerFunc {
tmpl := templates.GetParsed() tmpl := templates.GetParsed()
return func(writer http.ResponseWriter, request *http.Request) { return func(writer http.ResponseWriter, request *http.Request) {
@@ -140,7 +142,7 @@ func (h *Handlers) HandleAppCreate() http.HandlerFunc {
return return
} }
http.Redirect(writer, request, "/apps/"+createdApp.ID, http.StatusSeeOther) redirectToApp(writer, request, createdApp.ID, "")
} }
} }
@@ -181,14 +183,10 @@ func (h *Handlers) HandleAppDetail() http.HandlerFunc {
} }
webhookURL := "https://" + request.Host + "/webhook/" + application.WebhookSecret webhookURL := "https://" + request.Host + "/webhook/" + application.WebhookSecret
deployKey := formatDeployKey( deployKey := formatDeployKey(application.SSHPublicKey, application.CreatedAt, application.Name)
application.SSHPublicKey,
application.CreatedAt,
application.Name,
)
data := h.addGlobals(map[string]any{ data := h.addGlobals(map[string]any{
dataKeyApp: application, "App": application,
"EnvVars": envVars, "EnvVars": envVars,
"Labels": labels, "Labels": labels,
"Volumes": volumes, "Volumes": volumes,
@@ -226,7 +224,7 @@ func (h *Handlers) HandleAppEdit() http.HandlerFunc {
} }
data := h.addGlobals(map[string]any{ data := h.addGlobals(map[string]any{
dataKeyApp: application, "App": application,
}, request) }, request)
h.renderTemplate(writer, tmpl, "app_edit.html", data) h.renderTemplate(writer, tmpl, "app_edit.html", data)
@@ -234,7 +232,7 @@ func (h *Handlers) HandleAppEdit() http.HandlerFunc {
} }
// HandleAppUpdate handles app updates. // HandleAppUpdate handles app updates.
func (h *Handlers) HandleAppUpdate() http.HandlerFunc { func (h *Handlers) HandleAppUpdate() http.HandlerFunc { //nolint:funlen // validation adds necessary length
tmpl := templates.GetParsed() tmpl := templates.GetParsed()
return func(writer http.ResponseWriter, request *http.Request) { return func(writer http.ResponseWriter, request *http.Request) {
@@ -259,8 +257,8 @@ func (h *Handlers) HandleAppUpdate() http.HandlerFunc {
nameErr := validateAppName(newName) nameErr := validateAppName(newName)
if nameErr != nil { if nameErr != nil {
data := h.addGlobals(map[string]any{ data := h.addGlobals(map[string]any{
dataKeyApp: application, "App": application,
dataKeyError: "Invalid app name: " + nameErr.Error(), "Error": "Invalid app name: " + nameErr.Error(),
}, request) }, request)
h.renderTemplate(writer, tmpl, "app_edit.html", data) h.renderTemplate(writer, tmpl, "app_edit.html", data)
@@ -270,8 +268,8 @@ func (h *Handlers) HandleAppUpdate() http.HandlerFunc {
repoURLErr := validateRepoURL(request.FormValue("repo_url")) repoURLErr := validateRepoURL(request.FormValue("repo_url"))
if repoURLErr != nil { if repoURLErr != nil {
data := h.addGlobals(map[string]any{ data := h.addGlobals(map[string]any{
dataKeyApp: application, "App": application,
dataKeyError: "Invalid repository URL: " + repoURLErr.Error(), "Error": "Invalid repository URL: " + repoURLErr.Error(),
}, request) }, request)
h.renderTemplate(writer, tmpl, "app_edit.html", data) h.renderTemplate(writer, tmpl, "app_edit.html", data)
@@ -289,8 +287,8 @@ func (h *Handlers) HandleAppUpdate() http.HandlerFunc {
limitsErr := applyResourceLimits(application, request) limitsErr := applyResourceLimits(application, request)
if limitsErr != "" { if limitsErr != "" {
data := h.addGlobals(map[string]any{ data := h.addGlobals(map[string]any{
dataKeyApp: application, "App": application,
dataKeyError: limitsErr, "Error": limitsErr,
}, request) }, request)
h.renderTemplate(writer, tmpl, "app_edit.html", data) h.renderTemplate(writer, tmpl, "app_edit.html", data)
@@ -302,8 +300,8 @@ func (h *Handlers) HandleAppUpdate() http.HandlerFunc {
h.log.Error("failed to update app", "error", saveErr) h.log.Error("failed to update app", "error", saveErr)
data := h.addGlobals(map[string]any{ data := h.addGlobals(map[string]any{
dataKeyApp: application, "App": application,
dataKeyError: "Failed to update app", "Error": "Failed to update app",
}, request) }, request)
h.renderTemplate(writer, tmpl, "app_edit.html", data) h.renderTemplate(writer, tmpl, "app_edit.html", data)
@@ -464,7 +462,7 @@ func (h *Handlers) HandleAppDeployments() http.HandlerFunc {
) )
data := h.addGlobals(map[string]any{ data := h.addGlobals(map[string]any{
dataKeyApp: application, "App": application,
"Deployments": deployments, "Deployments": deployments,
}, request) }, request)
@@ -537,7 +535,7 @@ func (h *Handlers) HandleAppLogs() http.HandlerFunc {
return return
} }
_, _ = writer.Write([]byte(SanitizeLogs(logs))) // #nosec G705 -- output sanitized _, _ = writer.Write([]byte(SanitizeLogs(logs))) // #nosec G705 -- logs sanitized, Content-Type is text/plain
} }
} }
@@ -576,8 +574,8 @@ func (h *Handlers) HandleDeploymentLogsAPI() http.HandlerFunc {
} }
response := map[string]any{ response := map[string]any{
jsonKeyLogs: logs, "logs": logs,
jsonKeyStatus: deployment.Status, "status": deployment.Status,
} }
_ = json.NewEncoder(writer).Encode(response) _ = json.NewEncoder(writer).Encode(response)
@@ -611,13 +609,7 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc {
return return
} }
// The log path is derived from request data (the app is looked // Get the log file path from deploy service
// 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)
@@ -625,43 +617,28 @@ func (h *Handlers) HandleDeploymentLogDownload() http.HandlerFunc {
return return
} }
relPath, relErr := filepath.Rel(logDir, logPath) // Check if file exists — logPath is constructed internally, not from user input
if relErr != nil { _, err := os.Stat(logPath) // #nosec G703 -- path from internal GetLogFilePath, not user input
if os.IsNotExist(err) {
http.NotFound(writer, request) http.NotFound(writer, request)
return return
} }
root, rootErr := os.OpenRoot(logDir) if err != nil {
if rootErr != nil { h.log.Error("failed to stat log file", "error", err, "path", logPath)
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.ServeContent(writer, request, filename, info.ModTime(), file) http.ServeFile(writer, request, logPath)
} }
} }
@@ -685,8 +662,8 @@ func (h *Handlers) HandleContainerLogsAPI() http.HandlerFunc {
containerInfo, containerErr := h.docker.FindContainerByAppID(request.Context(), appID) containerInfo, containerErr := h.docker.FindContainerByAppID(request.Context(), appID)
if containerErr != nil || containerInfo == nil { if containerErr != nil || containerInfo == nil {
response := map[string]any{ response := map[string]any{
jsonKeyLogs: "No container running\n", "logs": "No container running\n",
jsonKeyStatus: "stopped", "status": "stopped",
} }
_ = json.NewEncoder(writer).Encode(response) _ = json.NewEncoder(writer).Encode(response)
@@ -706,8 +683,8 @@ func (h *Handlers) HandleContainerLogsAPI() http.HandlerFunc {
) )
response := map[string]any{ response := map[string]any{
jsonKeyLogs: "Failed to fetch container logs\n", "logs": "Failed to fetch container logs\n",
jsonKeyStatus: "error", "status": "error",
} }
_ = json.NewEncoder(writer).Encode(response) _ = json.NewEncoder(writer).Encode(response)
@@ -720,8 +697,8 @@ func (h *Handlers) HandleContainerLogsAPI() http.HandlerFunc {
} }
response := map[string]any{ response := map[string]any{
jsonKeyLogs: SanitizeLogs(logs), "logs": SanitizeLogs(logs),
jsonKeyStatus: status, "status": status,
} }
_ = json.NewEncoder(writer).Encode(response) _ = json.NewEncoder(writer).Encode(response)
@@ -755,7 +732,7 @@ func (h *Handlers) HandleAppStatusAPI() http.HandlerFunc {
} }
response := map[string]any{ response := map[string]any{
jsonKeyStatus: string(application.Status), "status": string(application.Status),
"latestDeploymentID": latestDeploymentID, "latestDeploymentID": latestDeploymentID,
"latestDeploymentStatus": latestDeploymentStatus, "latestDeploymentStatus": latestDeploymentStatus,
} }
@@ -792,7 +769,7 @@ func (h *Handlers) HandleRecentDeploymentsAPI() http.HandlerFunc {
for _, d := range deployments { for _, d := range deployments {
deploymentsData = append(deploymentsData, map[string]any{ deploymentsData = append(deploymentsData, map[string]any{
"id": d.ID, "id": d.ID,
jsonKeyStatus: string(d.Status), "status": string(d.Status),
"duration": d.Duration(), "duration": d.Duration(),
"shortCommit": d.ShortCommit(), "shortCommit": d.ShortCommit(),
"finishedAtISO": d.FinishedAtISO(), "finishedAtISO": d.FinishedAtISO(),
@@ -992,7 +969,7 @@ func (h *Handlers) HandleEnvVarSave() http.HandlerFunc {
decodeErr := json.NewDecoder(request.Body).Decode(&pairs) decodeErr := json.NewDecoder(request.Body).Decode(&pairs)
if decodeErr != nil { if decodeErr != nil {
h.respondJSON(writer, request, map[string]string{ h.respondJSON(writer, request, map[string]string{
jsonKeyError: "invalid request body", "error": "invalid request body",
}, http.StatusBadRequest) }, http.StatusBadRequest)
return return
@@ -1001,7 +978,7 @@ func (h *Handlers) HandleEnvVarSave() http.HandlerFunc {
modelPairs, validationErr := validateEnvPairs(pairs) modelPairs, validationErr := validateEnvPairs(pairs)
if validationErr != "" { if validationErr != "" {
h.respondJSON(writer, request, map[string]string{ h.respondJSON(writer, request, map[string]string{
jsonKeyError: validationErr, "error": validationErr,
}, http.StatusBadRequest) }, http.StatusBadRequest)
return return
@@ -1013,7 +990,7 @@ func (h *Handlers) HandleEnvVarSave() http.HandlerFunc {
if replaceErr != nil { if replaceErr != nil {
h.log.Error("failed to replace env vars", "error", replaceErr) h.log.Error("failed to replace env vars", "error", replaceErr)
h.respondJSON(writer, request, map[string]string{ h.respondJSON(writer, request, map[string]string{
jsonKeyError: "failed to save environment variables", "error": "failed to save environment variables",
}, http.StatusInternalServerError) }, http.StatusInternalServerError)
return return
@@ -1041,77 +1018,32 @@ func (h *Handlers) HandleLabelAdd() http.HandlerFunc {
} }
} }
// deleteAppResource handles deletion of an app-owned resource (label,
// volume, or port) identified by an int64 URL parameter. The
// deleteByID closure reports whether the resource was found to belong
// to the app, and returns the deletion error if one occurred.
func (h *Handlers) deleteAppResource(
writer http.ResponseWriter,
request *http.Request,
idParam, logName string,
deleteByID deleteByIDFunc,
) {
appID := chi.URLParam(request, "id")
idStr := chi.URLParam(request, idParam)
id, parseErr := strconv.ParseInt(idStr, 10, 64)
if parseErr != nil {
http.NotFound(writer, request)
return
}
found, deleteErr := deleteByID(request.Context(), appID, id)
if !found {
http.NotFound(writer, request)
return
}
if deleteErr != nil {
h.log.Error("failed to delete "+logName, "error", deleteErr)
}
redirectToApp(writer, request, appID, "")
}
// deleteByIDFunc looks up an app-owned resource by ID and deletes it
// when it belongs to the given app. It reports whether the resource
// was found, and returns lookup or deletion errors.
type deleteByIDFunc func(ctx context.Context, appID string, id int64) (bool, error)
// makeDeleteByID builds a deleteByIDFunc from a model's find
// function, its app-ID accessor, and its delete method.
func makeDeleteByID[T any](
db *database.Database,
find func(context.Context, *database.Database, int64) (*T, error),
appIDOf func(*T) string,
del func(*T, context.Context) error,
) deleteByIDFunc {
return func(ctx context.Context, appID string, id int64) (bool, error) {
resource, findErr := find(ctx, db, id)
if findErr != nil {
return false, findErr
}
if resource == nil || appIDOf(resource) != appID {
return false, nil
}
return true, del(resource, ctx)
}
}
// HandleLabelDelete handles deleting a label. // HandleLabelDelete handles deleting a label.
func (h *Handlers) HandleLabelDelete() http.HandlerFunc { func (h *Handlers) HandleLabelDelete() http.HandlerFunc {
return func(writer http.ResponseWriter, request *http.Request) { return func(writer http.ResponseWriter, request *http.Request) {
h.deleteAppResource( appID := chi.URLParam(request, "id")
writer, request, "labelID", "label", labelIDStr := chi.URLParam(request, "labelID")
makeDeleteByID(h.db, models.FindLabel,
func(l *models.Label) string { return l.AppID }, labelID, parseErr := strconv.ParseInt(labelIDStr, 10, 64)
(*models.Label).Delete, if parseErr != nil {
), http.NotFound(writer, request)
)
return
}
label, findErr := models.FindLabel(request.Context(), h.db, labelID)
if findErr != nil || label == nil || label.AppID != appID {
http.NotFound(writer, request)
return
}
deleteErr := label.Delete(request.Context())
if deleteErr != nil {
h.log.Error("failed to delete label", "error", deleteErr)
}
redirectToApp(writer, request, appID, "")
} }
} }
@@ -1170,13 +1102,29 @@ func (h *Handlers) HandleVolumeAdd() http.HandlerFunc {
// HandleVolumeDelete handles deleting a volume mount. // HandleVolumeDelete handles deleting a volume mount.
func (h *Handlers) HandleVolumeDelete() http.HandlerFunc { func (h *Handlers) HandleVolumeDelete() http.HandlerFunc {
return func(writer http.ResponseWriter, request *http.Request) { return func(writer http.ResponseWriter, request *http.Request) {
h.deleteAppResource( appID := chi.URLParam(request, "id")
writer, request, "volumeID", "volume", volumeIDStr := chi.URLParam(request, "volumeID")
makeDeleteByID(h.db, models.FindVolume,
func(v *models.Volume) string { return v.AppID }, volumeID, parseErr := strconv.ParseInt(volumeIDStr, 10, 64)
(*models.Volume).Delete, if parseErr != nil {
), http.NotFound(writer, request)
)
return
}
volume, findErr := models.FindVolume(request.Context(), h.db, volumeID)
if findErr != nil || volume == nil || volume.AppID != appID {
http.NotFound(writer, request)
return
}
deleteErr := volume.Delete(request.Context())
if deleteErr != nil {
h.log.Error("failed to delete volume", "error", deleteErr)
}
redirectToApp(writer, request, appID, "")
} }
} }
@@ -1249,13 +1197,29 @@ func parsePortValues(hostPortStr, containerPortStr string) (int, int, bool) {
// HandlePortDelete handles deleting a port mapping. // HandlePortDelete handles deleting a port mapping.
func (h *Handlers) HandlePortDelete() http.HandlerFunc { func (h *Handlers) HandlePortDelete() http.HandlerFunc {
return func(writer http.ResponseWriter, request *http.Request) { return func(writer http.ResponseWriter, request *http.Request) {
h.deleteAppResource( appID := chi.URLParam(request, "id")
writer, request, "portID", "port", portIDStr := chi.URLParam(request, "portID")
makeDeleteByID(h.db, models.FindPort,
func(p *models.Port) string { return p.AppID }, portID, parseErr := strconv.ParseInt(portIDStr, 10, 64)
(*models.Port).Delete, if parseErr != nil {
), http.NotFound(writer, request)
)
return
}
port, findErr := models.FindPort(request.Context(), h.db, portID)
if findErr != nil || port == nil || port.AppID != appID {
http.NotFound(writer, request)
return
}
deleteErr := port.Delete(request.Context())
if deleteErr != nil {
h.log.Error("failed to delete port", "error", deleteErr)
}
redirectToApp(writer, request, appID, "")
} }
} }
@@ -1432,9 +1396,8 @@ func optionalNullString(s string) sql.NullString {
return sql.NullString{} return sql.NullString{}
} }
// applyResourceLimits parses CPU and memory limit form values and // applyResourceLimits parses CPU and memory limit form values and applies them to the app.
// applies them to the app. Returns an error message string if // Returns an error message string if validation fails, or empty string on success.
// validation fails, or empty string on success.
func applyResourceLimits(application *models.App, request *http.Request) string { func applyResourceLimits(application *models.App, request *http.Request) string {
cpuLimit, cpuErr := parseOptionalFloat64(request.FormValue("cpu_limit")) cpuLimit, cpuErr := parseOptionalFloat64(request.FormValue("cpu_limit"))
if cpuErr != nil { if cpuErr != nil {
@@ -1469,8 +1432,7 @@ func memoryUnitMultiplier(suffix byte) int64 {
} }
// parseOptionalFloat64 parses an optional float64 form field. // parseOptionalFloat64 parses an optional float64 form field.
// Returns a valid NullFloat64 if the string is non-empty and parses // Returns a valid NullFloat64 if the string is non-empty and parses to a positive number.
// to a positive number.
// Returns an empty NullFloat64 if the string is empty. // Returns an empty NullFloat64 if the string is empty.
// Returns an error if the string is non-empty but invalid or non-positive. // Returns an error if the string is non-empty but invalid or non-positive.
func parseOptionalFloat64(s string) (sql.NullFloat64, error) { func parseOptionalFloat64(s string) (sql.NullFloat64, error) {
@@ -1492,8 +1454,7 @@ func parseOptionalFloat64(s string) (sql.NullFloat64, error) {
} }
// parseOptionalMemoryBytes parses an optional memory limit string into bytes. // parseOptionalMemoryBytes parses an optional memory limit string into bytes.
// Accepts plain bytes (e.g. "536870912") or suffixed values // Accepts plain bytes (e.g. "536870912") or suffixed values (e.g. "512m", "1g", "256k").
// (e.g. "512m", "1g", "256k").
// Returns a valid NullInt64 with bytes if non-empty, empty NullInt64 if blank. // Returns a valid NullInt64 with bytes if non-empty, empty NullInt64 if blank.
func parseOptionalMemoryBytes(s string) (sql.NullInt64, error) { func parseOptionalMemoryBytes(s string) (sql.NullInt64, error) {
s = strings.TrimSpace(s) s = strings.TrimSpace(s)
+2 -10
View File
@@ -21,16 +21,8 @@ func TestValidateAppName(t *testing.T) {
{"empty", "", true}, {"empty", "", true},
{"single char", "a", true}, {"single char", "a", true},
{"too long", "a" + string(make([]byte, 63)), true}, {"too long", "a" + string(make([]byte, 63)), true},
{ {"exactly 63 chars", "a23456789012345678901234567890123456789012345678901234567890123", false},
"exactly 63 chars", {"64 chars", "a234567890123456789012345678901234567890123456789012345678901234", true},
"a23456789012345678901234567890123456789012345678901234567890123",
false,
},
{
"64 chars",
"a234567890123456789012345678901234567890123456789012345678901234",
true,
},
{"uppercase", "MyApp", true}, {"uppercase", "MyApp", true},
{"spaces", "my app", true}, {"spaces", "my app", true},
{"starts with hyphen", "-myapp", true}, {"starts with hyphen", "-myapp", true},
-13
View File
@@ -22,19 +22,6 @@ import (
"sneak.berlin/go/upaas/templates" "sneak.berlin/go/upaas/templates"
) )
// Template data keys shared across handlers.
const (
dataKeyApp = "App"
dataKeyError = "Error"
)
// JSON response keys shared across handlers.
const (
jsonKeyError = "error"
jsonKeyLogs = "logs"
jsonKeyStatus = "status"
)
// Params contains dependencies for Handlers. // Params contains dependencies for Handlers.
type Params struct { type Params struct {
fx.In fx.In
+45 -70
View File
@@ -32,17 +32,11 @@ import (
"sneak.berlin/go/upaas/internal/service/webhook" "sneak.berlin/go/upaas/internal/service/webhook"
) )
const (
branchMain = "main"
paramSecret = "secret"
)
type testContext struct { type testContext struct {
handlers *handlers.Handlers handlers *handlers.Handlers
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
} }
@@ -187,7 +181,6 @@ func setupTestHandlers(t *testing.T) *testContext {
database: dbInstance, database: dbInstance,
authSvc: authSvc, authSvc: authSvc,
appSvc: appSvc, appSvc: appSvc,
deploySvc: deploySvc,
middleware: mw, middleware: mw,
} }
} }
@@ -218,26 +211,6 @@ func TestHandleHealthCheck(t *testing.T) {
}) })
} }
// assertPageRenders serves a GET request for path with the given
// handler and asserts a 200 response containing want.
func assertPageRenders(
t *testing.T,
handler http.Handler,
path, want string,
) {
t.Helper()
request := httptest.NewRequestWithContext(
t.Context(), http.MethodGet, path, nil,
)
recorder := httptest.NewRecorder()
handler.ServeHTTP(recorder, request)
assert.Equal(t, http.StatusOK, recorder.Code)
assert.Contains(t, recorder.Body.String(), want)
}
func TestHandleSetupGET(t *testing.T) { func TestHandleSetupGET(t *testing.T) {
t.Parallel() t.Parallel()
@@ -245,20 +218,31 @@ func TestHandleSetupGET(t *testing.T) {
t.Parallel() t.Parallel()
testCtx := setupTestHandlers(t) testCtx := setupTestHandlers(t)
assertPageRenders(t, testCtx.handlers.HandleSetupGET(), "/setup", "setup")
request := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/setup", nil)
recorder := httptest.NewRecorder()
handler := testCtx.handlers.HandleSetupGET()
handler.ServeHTTP(recorder, request)
assert.Equal(t, http.StatusOK, recorder.Code)
assert.Contains(t, recorder.Body.String(), "setup")
}) })
} }
func createSetupFormRequest( func createSetupFormRequest(
t *testing.T,
username, password, confirm string, username, password, confirm string,
) *http.Request { ) *http.Request {
t.Helper()
form := url.Values{} form := url.Values{}
form.Set("username", username) form.Set("username", username)
form.Set("password", password) form.Set("password", password)
form.Set("password_confirm", confirm) form.Set("password_confirm", confirm)
request := httptest.NewRequestWithContext( request := httptest.NewRequestWithContext(
context.Background(), t.Context(),
http.MethodPost, http.MethodPost,
"/setup", "/setup",
strings.NewReader(form.Encode()), strings.NewReader(form.Encode()),
@@ -273,7 +257,7 @@ func TestHandleSetupPOSTCreatesUserAndRedirects(t *testing.T) {
testCtx := setupTestHandlers(t) testCtx := setupTestHandlers(t)
request := createSetupFormRequest("admin", "password123", "password123") request := createSetupFormRequest(t, "admin", "password123", "password123")
recorder := httptest.NewRecorder() recorder := httptest.NewRecorder()
handler := testCtx.handlers.HandleSetupPOST() handler := testCtx.handlers.HandleSetupPOST()
@@ -288,7 +272,7 @@ func TestHandleSetupPOSTRejectsEmptyUsername(t *testing.T) {
testCtx := setupTestHandlers(t) testCtx := setupTestHandlers(t)
request := createSetupFormRequest("", "password123", "password123") request := createSetupFormRequest(t, "", "password123", "password123")
recorder := httptest.NewRecorder() recorder := httptest.NewRecorder()
handler := testCtx.handlers.HandleSetupPOST() handler := testCtx.handlers.HandleSetupPOST()
@@ -303,7 +287,7 @@ func TestHandleSetupPOSTRejectsShortPassword(t *testing.T) {
testCtx := setupTestHandlers(t) testCtx := setupTestHandlers(t)
request := createSetupFormRequest("admin", "short", "short") request := createSetupFormRequest(t, "admin", "short", "short")
recorder := httptest.NewRecorder() recorder := httptest.NewRecorder()
handler := testCtx.handlers.HandleSetupPOST() handler := testCtx.handlers.HandleSetupPOST()
@@ -318,7 +302,7 @@ func TestHandleSetupPOSTRejectsMismatchedPasswords(t *testing.T) {
testCtx := setupTestHandlers(t) testCtx := setupTestHandlers(t)
request := createSetupFormRequest("admin", "password123", "different123") request := createSetupFormRequest(t, "admin", "password123", "different123")
recorder := httptest.NewRecorder() recorder := httptest.NewRecorder()
handler := testCtx.handlers.HandleSetupPOST() handler := testCtx.handlers.HandleSetupPOST()
@@ -335,17 +319,27 @@ func TestHandleLoginGET(t *testing.T) {
t.Parallel() t.Parallel()
testCtx := setupTestHandlers(t) testCtx := setupTestHandlers(t)
assertPageRenders(t, testCtx.handlers.HandleLoginGET(), "/login", "login")
request := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/login", nil)
recorder := httptest.NewRecorder()
handler := testCtx.handlers.HandleLoginGET()
handler.ServeHTTP(recorder, request)
assert.Equal(t, http.StatusOK, recorder.Code)
assert.Contains(t, recorder.Body.String(), "login")
}) })
} }
func createLoginFormRequest(username, password string) *http.Request { func createLoginFormRequest(t *testing.T, username, password string) *http.Request {
t.Helper()
form := url.Values{} form := url.Values{}
form.Set("username", username) form.Set("username", username)
form.Set("password", password) form.Set("password", password)
request := httptest.NewRequestWithContext( request := httptest.NewRequestWithContext(
context.Background(), t.Context(),
http.MethodPost, http.MethodPost,
"/login", "/login",
strings.NewReader(form.Encode()), strings.NewReader(form.Encode()),
@@ -368,7 +362,7 @@ func TestHandleLoginPOSTAuthenticatesValidCredentials(t *testing.T) {
) )
require.NoError(t, createErr) require.NoError(t, createErr)
request := createLoginFormRequest("testuser", "testpass123") request := createLoginFormRequest(t, "testuser", "testpass123")
recorder := httptest.NewRecorder() recorder := httptest.NewRecorder()
handler := testCtx.handlers.HandleLoginPOST() handler := testCtx.handlers.HandleLoginPOST()
@@ -391,7 +385,7 @@ func TestHandleLoginPOSTRejectsInvalidCredentials(t *testing.T) {
) )
require.NoError(t, createErr) require.NoError(t, createErr)
request := createLoginFormRequest("testuser", "wrongpassword") request := createLoginFormRequest(t, "testuser", "wrongpassword")
recorder := httptest.NewRecorder() recorder := httptest.NewRecorder()
handler := testCtx.handlers.HandleLoginPOST() handler := testCtx.handlers.HandleLoginPOST()
@@ -409,9 +403,7 @@ func TestHandleDashboard(t *testing.T) {
testCtx := setupTestHandlers(t) testCtx := setupTestHandlers(t)
request := httptest.NewRequestWithContext( request := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
t.Context(), http.MethodGet, "/", nil,
)
recorder := httptest.NewRecorder() recorder := httptest.NewRecorder()
handler := testCtx.handlers.HandleDashboard() handler := testCtx.handlers.HandleDashboard()
@@ -429,9 +421,7 @@ func TestHandleDashboard(t *testing.T) {
// Create an app so the template iterates over AppStats and hits .CSRFField // Create an app so the template iterates over AppStats and hits .CSRFField
createTestApp(t, testCtx, "csrf-test-app") createTestApp(t, testCtx, "csrf-test-app")
request := httptest.NewRequestWithContext( request := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
t.Context(), http.MethodGet, "/", nil,
)
recorder := httptest.NewRecorder() recorder := httptest.NewRecorder()
handler := testCtx.handlers.HandleDashboard() handler := testCtx.handlers.HandleDashboard()
@@ -492,7 +482,7 @@ func createTestApp(
app.CreateAppInput{ app.CreateAppInput{
Name: name, Name: name,
RepoURL: "git@example.com:user/" + name + ".git", RepoURL: "git@example.com:user/" + name + ".git",
Branch: branchMain, Branch: "main",
}, },
) )
require.NoError(t, err) require.NoError(t, err)
@@ -513,7 +503,7 @@ func TestHandleWebhookRejectsOversizedBody(t *testing.T) {
app.CreateAppInput{ app.CreateAppInput{
Name: "oversize-test-app", Name: "oversize-test-app",
RepoURL: "git@example.com:user/repo.git", RepoURL: "git@example.com:user/repo.git",
Branch: branchMain, Branch: "main",
}, },
) )
require.NoError(t, createErr) require.NoError(t, createErr)
@@ -529,7 +519,7 @@ func TestHandleWebhookRejectsOversizedBody(t *testing.T) {
) )
request = addChiURLParams( request = addChiURLParams(
request, request,
map[string]string{paramSecret: createdApp.WebhookSecret}, map[string]string{"secret": createdApp.WebhookSecret},
) )
request.Header.Set("Content-Type", "application/json") request.Header.Set("Content-Type", "application/json")
request.Header.Set("X-Gitea-Event", "push") request.Header.Set("X-Gitea-Event", "push")
@@ -698,8 +688,7 @@ func TestHandleEnvVarSaveDuplicateKeyRejected(t *testing.T) {
createdApp := createTestApp(t, testCtx, "envvar-dedup-app") createdApp := createTestApp(t, testCtx, "envvar-dedup-app")
// Send two entries with the same key — should be rejected // Send two entries with the same key — should be rejected
body := `[{"key":"FOO","value":"first"},{"key":"BAR","value":"bar"},` + body := `[{"key":"FOO","value":"first"},{"key":"BAR","value":"bar"},{"key":"FOO","value":"second"}]`
`{"key":"FOO","value":"second"}]`
r := chi.NewRouter() r := chi.NewRouter()
r.Post("/apps/{id}/env", testCtx.handlers.HandleEnvVarSave()) r.Post("/apps/{id}/env", testCtx.handlers.HandleEnvVarSave())
@@ -1049,8 +1038,7 @@ func TestHandleVolumeAddValidatesPaths(t *testing.T) {
} }
// TestSetupRequiredExemptsHealthAndStaticAndAPI verifies that the SetupRequired // TestSetupRequiredExemptsHealthAndStaticAndAPI verifies that the SetupRequired
// middleware allows /health, /s/*, and /api/* paths through even when setup is // middleware allows /health, /s/*, and /api/* paths through even when setup is required.
// required.
func TestSetupRequiredExemptsHealthAndStaticAndAPI(t *testing.T) { func TestSetupRequiredExemptsHealthAndStaticAndAPI(t *testing.T) {
t.Parallel() t.Parallel()
@@ -1066,21 +1054,13 @@ func TestSetupRequiredExemptsHealthAndStaticAndAPI(t *testing.T) {
wrapped := mw(okHandler) wrapped := mw(okHandler)
exemptPaths := []string{ exemptPaths := []string{"/health", "/s/style.css", "/s/js/app.js", "/api/v1/apps", "/api/v1/login"}
"/health",
"/s/style.css",
"/s/js/app.js",
"/api/v1/apps",
"/api/v1/login",
}
for _, path := range exemptPaths { for _, path := range exemptPaths {
t.Run(path, func(t *testing.T) { t.Run(path, func(t *testing.T) {
t.Parallel() t.Parallel()
req := httptest.NewRequestWithContext( req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, path, nil)
t.Context(), http.MethodGet, path, nil,
)
rr := httptest.NewRecorder() rr := httptest.NewRecorder()
wrapped.ServeHTTP(rr, req) wrapped.ServeHTTP(rr, req)
@@ -1093,9 +1073,7 @@ func TestSetupRequiredExemptsHealthAndStaticAndAPI(t *testing.T) {
t.Run("non-exempt redirects", func(t *testing.T) { t.Run("non-exempt redirects", func(t *testing.T) {
t.Parallel() t.Parallel()
req := httptest.NewRequestWithContext( req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
t.Context(), http.MethodGet, "/", nil,
)
rr := httptest.NewRecorder() rr := httptest.NewRecorder()
wrapped.ServeHTTP(rr, req) wrapped.ServeHTTP(rr, req)
@@ -1160,10 +1138,7 @@ func TestHandleWebhookReturns404ForUnknownSecret(t *testing.T) {
webhookURL, webhookURL,
strings.NewReader(payload), strings.NewReader(payload),
) )
request = addChiURLParams( request = addChiURLParams(request, map[string]string{"secret": "unknown-secret"})
request,
map[string]string{paramSecret: "unknown-secret"},
)
request.Header.Set("Content-Type", "application/json") request.Header.Set("Content-Type", "application/json")
request.Header.Set("X-Gitea-Event", "push") request.Header.Set("X-Gitea-Event", "push")
@@ -1186,7 +1161,7 @@ func TestHandleWebhookProcessesValidWebhook(t *testing.T) {
app.CreateAppInput{ app.CreateAppInput{
Name: "webhook-test-app", Name: "webhook-test-app",
RepoURL: "git@example.com:user/repo.git", RepoURL: "git@example.com:user/repo.git",
Branch: branchMain, Branch: "main",
}, },
) )
require.NoError(t, createErr) require.NoError(t, createErr)
@@ -1201,7 +1176,7 @@ func TestHandleWebhookProcessesValidWebhook(t *testing.T) {
) )
request = addChiURLParams( request = addChiURLParams(
request, request,
map[string]string{paramSecret: createdApp.WebhookSecret}, map[string]string{"secret": createdApp.WebhookSecret},
) )
request.Header.Set("Content-Type", "application/json") request.Header.Set("Content-Type", "application/json")
request.Header.Set("X-Gitea-Event", "push") request.Header.Set("X-Gitea-Event", "push")
-121
View File
@@ -1,121 +0,0 @@
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")
}
+2 -6
View File
@@ -16,9 +16,7 @@ func TestRenderTemplateBuffersOutput(t *testing.T) {
testCtx := setupTestHandlers(t) testCtx := setupTestHandlers(t)
// The setup page is simple and has no DB dependencies // The setup page is simple and has no DB dependencies
request := httptest.NewRequestWithContext( request := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/setup", nil)
t.Context(), http.MethodGet, "/setup", nil,
)
recorder := httptest.NewRecorder() recorder := httptest.NewRecorder()
handler := testCtx.handlers.HandleSetupGET() handler := testCtx.handlers.HandleSetupGET()
@@ -61,9 +59,7 @@ func TestLoginRenderTemplateBuffersOutput(t *testing.T) {
testCtx := setupTestHandlers(t) testCtx := setupTestHandlers(t)
request := httptest.NewRequestWithContext( request := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/login", nil)
t.Context(), http.MethodGet, "/login", nil,
)
recorder := httptest.NewRecorder() recorder := httptest.NewRecorder()
handler := testCtx.handlers.HandleLoginGET() handler := testCtx.handlers.HandleLoginGET()
+6 -11
View File
@@ -11,17 +11,13 @@ import (
var ( var (
errRepoURLEmpty = errors.New("repository URL must not be empty") errRepoURLEmpty = errors.New("repository URL must not be empty")
errRepoURLScheme = errors.New("file:// URLs are not allowed for security reasons") errRepoURLScheme = errors.New("file:// URLs are not allowed for security reasons")
errRepoURLInvalid = errors.New( errRepoURLInvalid = errors.New("repository URL must use https://, http://, ssh://, git://, or git@host:path format")
"repository URL must use https://, http://, ssh://, git://, " + errRepoURLNoHost = errors.New("repository URL must include a host")
"or git@host:path format", errRepoURLNoPath = errors.New("repository URL must include a path")
)
errRepoURLNoHost = errors.New("repository URL must include a host")
errRepoURLNoPath = errors.New("repository URL must include a path")
) )
// scpLikeRepoRe matches SCP-like git URLs: git@host:path // scpLikeRepoRe matches SCP-like git URLs: git@host:path (e.g. git@github.com:user/repo.git).
// (e.g. git@github.com:user/repo.git). Only the "git" user is allowed, // Only the "git" user is allowed, as that is the standard for SSH deploy keys.
// as that is the standard for SSH deploy keys.
var scpLikeRepoRe = regexp.MustCompile(`^git@[a-zA-Z0-9._-]+:.+$`) var scpLikeRepoRe = regexp.MustCompile(`^git@[a-zA-Z0-9._-]+:.+$`)
// allowedRepoSchemes lists the URL schemes accepted for repository URLs. // allowedRepoSchemes lists the URL schemes accepted for repository URLs.
@@ -34,8 +30,7 @@ var allowedRepoSchemes = map[string]bool{
"git": true, "git": true,
} }
// validateRepoURL checks that the given repository URL is valid and // validateRepoURL checks that the given repository URL is valid and uses an allowed scheme.
// uses an allowed scheme.
func validateRepoURL(repoURL string) error { func validateRepoURL(repoURL string) error {
if strings.TrimSpace(repoURL) == "" { if strings.TrimSpace(repoURL) == "" {
return errRepoURLEmpty return errRepoURLEmpty
+4 -20
View File
@@ -22,11 +22,7 @@ func TestValidateRepoURL(t *testing.T) {
{name: "SCP-like URL", url: "git@github.com:user/repo.git", wantErr: false}, {name: "SCP-like URL", url: "git@github.com:user/repo.git", wantErr: false},
{name: "SCP-like with dots", url: "git@git.example.com:org/repo.git", wantErr: false}, {name: "SCP-like with dots", url: "git@git.example.com:org/repo.git", wantErr: false},
{name: "https without .git", url: "https://github.com/user/repo", wantErr: false}, {name: "https without .git", url: "https://github.com/user/repo", wantErr: false},
{ {name: "https with port", url: "https://git.example.com:8443/user/repo.git", wantErr: false},
name: "https with port",
url: "https://git.example.com:8443/user/repo.git",
wantErr: false,
},
// Invalid URLs // Invalid URLs
{name: "empty string", url: "", wantErr: true}, {name: "empty string", url: "", wantErr: true},
@@ -41,22 +37,10 @@ func TestValidateRepoURL(t *testing.T) {
{name: "no path https", url: "https://github.com", wantErr: true}, {name: "no path https", url: "https://github.com", wantErr: true},
{name: "no path https trailing slash", url: "https://github.com/", wantErr: true}, {name: "no path https trailing slash", url: "https://github.com/", wantErr: true},
{name: "SCP-like non-git user", url: "root@github.com:user/repo.git", wantErr: true}, {name: "SCP-like non-git user", url: "root@github.com:user/repo.git", wantErr: true},
{ {name: "SCP-like arbitrary user", url: "admin@github.com:user/repo.git", wantErr: true},
name: "SCP-like arbitrary user",
url: "admin@github.com:user/repo.git",
wantErr: true,
},
{name: "path traversal SCP", url: "git@github.com:../../etc/passwd", wantErr: true}, {name: "path traversal SCP", url: "git@github.com:../../etc/passwd", wantErr: true},
{ {name: "path traversal https", url: "https://github.com/user/../../../etc/passwd", wantErr: true},
name: "path traversal https", {name: "path traversal in middle", url: "https://github.com/user/repo/../secret", wantErr: true},
url: "https://github.com/user/../../../etc/passwd",
wantErr: true,
},
{
name: "path traversal in middle",
url: "https://github.com/user/repo/../secret",
wantErr: true,
},
} }
for _, tc := range tests { for _, tc := range tests {
+2 -5
View File
@@ -5,11 +5,8 @@ import (
"strings" "strings"
) )
// ansiEscapePattern matches ANSI escape sequences (CSI, OSC, and // ansiEscapePattern matches ANSI escape sequences (CSI, OSC, and single-character escapes).
// single-character escapes). var ansiEscapePattern = regexp.MustCompile(`(\x1b\[[0-9;]*[a-zA-Z]|\x1b\][^\x07]*\x07|\x1b[^[\]])`)
var ansiEscapePattern = regexp.MustCompile(
`(\x1b\[[0-9;]*[a-zA-Z]|\x1b\][^\x07]*\x07|\x1b[^[\]])`,
)
// SanitizeLogs strips ANSI escape sequences and non-printable control characters // SanitizeLogs strips ANSI escape sequences and non-printable control characters
// from container log output. Newlines (\n), carriage returns (\r), and tabs (\t) // from container log output. Newlines (\n), carriage returns (\r), and tabs (\t)
+1 -1
View File
@@ -6,7 +6,7 @@ import (
"sneak.berlin/go/upaas/internal/handlers" "sneak.berlin/go/upaas/internal/handlers"
) )
func TestSanitizeLogs(t *testing.T) { func TestSanitizeLogs(t *testing.T) { //nolint:funlen // table-driven tests
t.Parallel() t.Parallel()
tests := []struct { tests := []struct {
+2 -2
View File
@@ -55,8 +55,8 @@ func (h *Handlers) renderSetupError(
errorMsg string, errorMsg string,
) { ) {
data := h.addGlobals(map[string]any{ data := h.addGlobals(map[string]any{
"Username": username, "Username": username,
dataKeyError: errorMsg, "Error": errorMsg,
}, request) }, request)
h.renderTemplate(writer, tmpl, "setup.html", data) h.renderTemplate(writer, tmpl, "setup.html", data)
} }
+2 -2
View File
@@ -47,8 +47,8 @@ func (h *Handlers) HandleAppWebhookEvents() http.HandlerFunc {
} }
data := h.addGlobals(map[string]any{ data := h.addGlobals(map[string]any{
dataKeyApp: application, "App": application,
"Events": events, "Events": events,
}, request) }, request)
h.renderTemplate(writer, tmpl, "webhook_events.html", data) h.renderTemplate(writer, tmpl, "webhook_events.html", data)
+17 -15
View File
@@ -24,30 +24,21 @@ func newCORSTestMiddleware(corsOrigins string) *Middleware {
} }
} }
// assertNoCORSHeaders runs a request with the given Origin header through func TestCORS_NoOriginsConfigured_NoCORSHeaders(t *testing.T) {
// CORS middleware configured with corsOrigins and asserts that no t.Parallel()
// Access-Control-Allow-Origin header is set.
func assertNoCORSHeaders(t *testing.T, corsOrigins, origin, msg string) {
t.Helper()
m := newCORSTestMiddleware(corsOrigins) m := newCORSTestMiddleware("")
handler := m.CORS()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { handler := m.CORS()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK) w.WriteHeader(http.StatusOK)
})) }))
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil) req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
req.Header.Set("Origin", origin) req.Header.Set("Origin", "https://evil.com")
rec := httptest.NewRecorder() rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req) handler.ServeHTTP(rec, req)
assert.Empty(t, rec.Header().Get("Access-Control-Allow-Origin"), msg) assert.Empty(t, rec.Header().Get("Access-Control-Allow-Origin"),
}
func TestCORS_NoOriginsConfigured_NoCORSHeaders(t *testing.T) {
t.Parallel()
assertNoCORSHeaders(t, "", "https://evil.com",
"expected no CORS headers when no origins configured") "expected no CORS headers when no origins configured")
} }
@@ -74,6 +65,17 @@ func TestCORS_OriginsConfigured_AllowsMatchingOrigin(t *testing.T) {
func TestCORS_OriginsConfigured_RejectsNonMatchingOrigin(t *testing.T) { func TestCORS_OriginsConfigured_RejectsNonMatchingOrigin(t *testing.T) {
t.Parallel() t.Parallel()
assertNoCORSHeaders(t, "https://app.example.com", "https://evil.com", m := newCORSTestMiddleware("https://app.example.com")
handler := m.CORS()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
req.Header.Set("Origin", "https://evil.com")
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)
assert.Empty(t, rec.Header().Get("Access-Control-Allow-Origin"),
"expected no CORS headers for non-matching origin") "expected no CORS headers for non-matching origin")
} }
-67
View File
@@ -1,67 +0,0 @@
package middleware //nolint:testpackage // tests internal CSRF behavior
import (
"log/slog"
"net/http"
"net/http/httptest"
"testing"
"github.com/stretchr/testify/assert"
"sneak.berlin/go/upaas/internal/config"
)
//nolint:gosec // test credentials
func newCSRFTestMiddleware(plaintextHTTP bool) *Middleware {
return &Middleware{
log: slog.Default(),
params: &Params{
Config: &config.Config{
SessionSecret: "test-secret-32-bytes-long-enough",
PlaintextHTTP: plaintextHTTP,
},
},
}
}
// postWithPlainHTTPOrigin drives a tokenless POST carrying a plain-HTTP Origin
// through the CSRF middleware and returns the "Forbidden - <reason>" body.
// gorilla/csrf checks the Origin before the token, so the reason reveals which
// check rejected the request.
func postWithPlainHTTPOrigin(t *testing.T, plaintextHTTP bool) string {
t.Helper()
m := newCSRFTestMiddleware(plaintextHTTP)
handler := m.CSRF()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequestWithContext(
t.Context(), http.MethodPost, "http://example.com/setup", nil)
req.Header.Set("Origin", "http://example.com")
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)
assert.Equal(t, http.StatusForbidden, rec.Code)
return rec.Body.String()
}
// Without PlaintextHTTP the origin check assumes https and rejects a browser's
// http:// Origin, which is what broke setup over plain HTTP.
func TestCSRF_PlaintextDisabled_RejectsPlainHTTPOrigin(t *testing.T) {
t.Parallel()
assert.Contains(t, postWithPlainHTTPOrigin(t, false), "origin invalid")
}
// With PlaintextHTTP the origin check uses http, so a matching http:// Origin
// passes it and the request only fails later for the missing token.
func TestCSRF_PlaintextEnabled_AllowsPlainHTTPOrigin(t *testing.T) {
t.Parallel()
body := postWithPlainHTTPOrigin(t, true)
assert.NotContains(t, body, "origin invalid")
assert.Contains(t, body, "CSRF token not found")
}
+4 -27
View File
@@ -255,34 +255,12 @@ func (m *Middleware) SessionAuth() func(http.Handler) http.Handler {
} }
// CSRF returns CSRF protection middleware using gorilla/csrf. // CSRF returns CSRF protection middleware using gorilla/csrf.
//
// gorilla/csrf assumes the request scheme is https for its same-origin check
// unless the request is marked plaintext. A TLS-terminating reverse proxy
// (the default deployment) presents https to the browser, so the default is
// correct there. When µPaaS is reached over plain HTTP — directly, or behind a
// proxy that does not terminate TLS — set UPAAS_PLAINTEXT_HTTP so the origin
// check compares against http:// and setup over plain HTTP works.
func (m *Middleware) CSRF() func(http.Handler) http.Handler { func (m *Middleware) CSRF() func(http.Handler) http.Handler {
protect := csrf.Protect( return csrf.Protect(
[]byte(m.params.Config.SessionSecret), []byte(m.params.Config.SessionSecret),
csrf.Secure(false), // cookie Secure flag; TLS is terminated upstream csrf.Secure(false), // Allow HTTP for development; reverse proxy handles TLS
csrf.Path("/"), csrf.Path("/"),
) )
if !m.params.Config.PlaintextHTTP {
return protect
}
return func(next http.Handler) http.Handler {
protected := protect(next)
return http.HandlerFunc(func(
writer http.ResponseWriter,
request *http.Request,
) {
protected.ServeHTTP(writer, csrf.PlaintextHTTPRequest(request))
})
}
} }
// loginRateLimit configures the login rate limiter. // loginRateLimit configures the login rate limiter.
@@ -392,9 +370,8 @@ func (m *Middleware) LoginRateLimit() func(http.Handler) http.Handler {
} }
} }
// APISessionAuth returns middleware that requires session authentication // APISessionAuth returns middleware that requires session authentication for API routes.
// for API routes. Unlike SessionAuth, it returns JSON 401 responses instead // Unlike SessionAuth, it returns JSON 401 responses instead of redirecting to /login.
// of redirecting to /login.
func (m *Middleware) APISessionAuth() func(http.Handler) http.Handler { func (m *Middleware) APISessionAuth() func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler { return func(next http.Handler) http.Handler {
return http.HandlerFunc(func( return http.HandlerFunc(func(
+13 -21
View File
@@ -30,11 +30,9 @@ func TestLoginRateLimitAllowsUpToBurst(t *testing.T) {
mw := newTestMiddleware(t) mw := newTestMiddleware(t)
handler := mw.LoginRateLimit()(http.HandlerFunc( handler := mw.LoginRateLimit()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK)
w.WriteHeader(http.StatusOK) }))
},
))
// First 5 requests should succeed (burst) // First 5 requests should succeed (burst)
for i := range 5 { for i := range 5 {
@@ -50,8 +48,7 @@ func TestLoginRateLimitAllowsUpToBurst(t *testing.T) {
req.RemoteAddr = "192.168.1.1:12345" req.RemoteAddr = "192.168.1.1:12345"
rec := httptest.NewRecorder() rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req) handler.ServeHTTP(rec, req)
assert.Equal(t, http.StatusTooManyRequests, rec.Code, assert.Equal(t, http.StatusTooManyRequests, rec.Code, "6th request should be rate limited")
"6th request should be rate limited")
} }
//nolint:paralleltest // mutates global loginLimiter //nolint:paralleltest // mutates global loginLimiter
@@ -60,23 +57,21 @@ func TestLoginRateLimitIsolatesIPs(t *testing.T) {
mw := newTestMiddleware(t) mw := newTestMiddleware(t)
handler := mw.LoginRateLimit()(http.HandlerFunc( handler := mw.LoginRateLimit()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK)
w.WriteHeader(http.StatusOK) }))
},
))
// Exhaust IP1's budget // Exhaust IP1's budget
for range 5 { for range 5 {
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/login", nil) req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/login", nil)
req.RemoteAddr = testProxyAddr req.RemoteAddr = "10.0.0.1:1234"
rec := httptest.NewRecorder() rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req) handler.ServeHTTP(rec, req)
} }
// IP1 should be blocked // IP1 should be blocked
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/login", nil) req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/login", nil)
req.RemoteAddr = testProxyAddr req.RemoteAddr = "10.0.0.1:1234"
rec := httptest.NewRecorder() rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req) handler.ServeHTTP(rec, req)
assert.Equal(t, http.StatusTooManyRequests, rec.Code) assert.Equal(t, http.StatusTooManyRequests, rec.Code)
@@ -95,11 +90,9 @@ func TestLoginRateLimitReturns429Body(t *testing.T) {
mw := newTestMiddleware(t) mw := newTestMiddleware(t)
handler := mw.LoginRateLimit()(http.HandlerFunc( handler := mw.LoginRateLimit()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusOK)
w.WriteHeader(http.StatusOK) }))
},
))
// Exhaust burst // Exhaust burst
for range 5 { for range 5 {
@@ -115,8 +108,7 @@ func TestLoginRateLimitReturns429Body(t *testing.T) {
handler.ServeHTTP(rec, req) handler.ServeHTTP(rec, req)
assert.Equal(t, http.StatusTooManyRequests, rec.Code) assert.Equal(t, http.StatusTooManyRequests, rec.Code)
assert.Contains(t, rec.Body.String(), "Too Many Requests") assert.Contains(t, rec.Body.String(), "Too Many Requests")
assert.NotEmpty(t, rec.Header().Get("Retry-After"), assert.NotEmpty(t, rec.Header().Get("Retry-After"), "should include Retry-After header")
"should include Retry-After header")
} }
func TestIPLimiterEvictsStaleEntries(t *testing.T) { func TestIPLimiterEvictsStaleEntries(t *testing.T) {
+25 -37
View File
@@ -7,16 +7,6 @@ import (
"testing" "testing"
) )
// Shared test addresses (also used by ratelimit_test.go).
const (
testProxyAddr = "10.0.0.1:1234"
testRealIP = "203.0.113.5"
testXFFIP = "198.51.100.1"
testPrivateIP = "192.168.1.1"
testPublicIP = "93.184.216.34"
testPublicDNSIP = "8.8.8.8"
)
func TestRealIP(t *testing.T) { //nolint:funlen // table-driven test func TestRealIP(t *testing.T) { //nolint:funlen // table-driven test
t.Parallel() t.Parallel()
@@ -30,63 +20,63 @@ func TestRealIP(t *testing.T) { //nolint:funlen // table-driven test
// === Trusted proxy (RFC1918 / loopback) — headers ARE honoured === // === Trusted proxy (RFC1918 / loopback) — headers ARE honoured ===
{ {
name: "trusted: X-Real-IP from 10.x", name: "trusted: X-Real-IP from 10.x",
remoteAddr: testProxyAddr, remoteAddr: "10.0.0.1:1234",
xRealIP: testRealIP, xRealIP: "203.0.113.5",
xff: "198.51.100.1, 10.0.0.1", xff: "198.51.100.1, 10.0.0.1",
want: testRealIP, want: "203.0.113.5",
}, },
{ {
name: "trusted: XFF from 10.x when no X-Real-IP", name: "trusted: XFF from 10.x when no X-Real-IP",
remoteAddr: testProxyAddr, remoteAddr: "10.0.0.1:1234",
xff: "198.51.100.1, 10.0.0.1", xff: "198.51.100.1, 10.0.0.1",
want: testXFFIP, want: "198.51.100.1",
}, },
{ {
name: "trusted: XFF single IP from 10.x", name: "trusted: XFF single IP from 10.x",
remoteAddr: testProxyAddr, remoteAddr: "10.0.0.1:1234",
xff: "203.0.113.10", xff: "203.0.113.10",
want: "203.0.113.10", want: "203.0.113.10",
}, },
{ {
name: "trusted: falls back to RemoteAddr (192.168.x)", name: "trusted: falls back to RemoteAddr (192.168.x)",
remoteAddr: "192.168.1.1:5678", remoteAddr: "192.168.1.1:5678",
want: testPrivateIP, want: "192.168.1.1",
}, },
{ {
name: "trusted: RemoteAddr without port", name: "trusted: RemoteAddr without port",
remoteAddr: testPrivateIP, remoteAddr: "192.168.1.1",
want: testPrivateIP, want: "192.168.1.1",
}, },
{ {
name: "trusted: X-Real-IP with whitespace from 10.x", name: "trusted: X-Real-IP with whitespace from 10.x",
remoteAddr: testProxyAddr, remoteAddr: "10.0.0.1:1234",
xRealIP: " 203.0.113.5 ", xRealIP: " 203.0.113.5 ",
want: testRealIP, want: "203.0.113.5",
}, },
{ {
name: "trusted: XFF with whitespace from 10.x", name: "trusted: XFF with whitespace from 10.x",
remoteAddr: testProxyAddr, remoteAddr: "10.0.0.1:1234",
xff: " 198.51.100.1 , 10.0.0.1", xff: " 198.51.100.1 , 10.0.0.1",
want: testXFFIP, want: "198.51.100.1",
}, },
{ {
name: "trusted: empty X-Real-IP falls through to XFF from 10.x", name: "trusted: empty X-Real-IP falls through to XFF from 10.x",
remoteAddr: testProxyAddr, remoteAddr: "10.0.0.1:1234",
xRealIP: " ", xRealIP: " ",
xff: testXFFIP, xff: "198.51.100.1",
want: testXFFIP, want: "198.51.100.1",
}, },
{ {
name: "trusted: loopback honours X-Real-IP", name: "trusted: loopback honours X-Real-IP",
remoteAddr: "127.0.0.1:9999", remoteAddr: "127.0.0.1:9999",
xRealIP: testPublicIP, xRealIP: "93.184.216.34",
want: testPublicIP, want: "93.184.216.34",
}, },
{ {
name: "trusted: 172.16.x honours XFF", name: "trusted: 172.16.x honours XFF",
remoteAddr: "172.16.0.1:4321", remoteAddr: "172.16.0.1:4321",
xff: testPublicDNSIP, xff: "8.8.8.8",
want: testPublicDNSIP, want: "8.8.8.8",
}, },
// === Untrusted proxy (public IP) — headers IGNORED, use RemoteAddr === // === Untrusted proxy (public IP) — headers IGNORED, use RemoteAddr ===
@@ -107,17 +97,17 @@ func TestRealIP(t *testing.T) { //nolint:funlen // table-driven test
remoteAddr: "8.8.8.8:443", remoteAddr: "8.8.8.8:443",
xRealIP: "1.2.3.4", xRealIP: "1.2.3.4",
xff: "5.6.7.8", xff: "5.6.7.8",
want: testPublicDNSIP, want: "8.8.8.8",
}, },
{ {
name: "untrusted: no headers, public RemoteAddr", name: "untrusted: no headers, public RemoteAddr",
remoteAddr: "93.184.216.34:8080", remoteAddr: "93.184.216.34:8080",
want: testPublicIP, want: "93.184.216.34",
}, },
{ {
name: "untrusted: public RemoteAddr without port", name: "untrusted: public RemoteAddr without port",
remoteAddr: testPublicIP, remoteAddr: "93.184.216.34",
want: testPublicIP, want: "93.184.216.34",
}, },
} }
@@ -149,9 +139,7 @@ func TestIsTrustedProxy(t *testing.T) {
trusted := []string{"10.0.0.1", "10.255.255.255", "172.16.0.1", "172.31.255.255", trusted := []string{"10.0.0.1", "10.255.255.255", "172.16.0.1", "172.31.255.255",
"192.168.0.1", "192.168.255.255", "127.0.0.1", "127.255.255.255", "::1"} "192.168.0.1", "192.168.255.255", "127.0.0.1", "127.255.255.255", "::1"}
untrusted := []string{ untrusted := []string{"8.8.8.8", "203.0.113.1", "172.32.0.1", "11.0.0.1", "2001:db8::1"}
testPublicDNSIP, "203.0.113.1", "172.32.0.1", "11.0.0.1", "2001:db8::1",
}
for _, addr := range trusted { for _, addr := range trusted {
ip := net.ParseIP(addr) ip := net.ParseIP(addr)
+23 -36
View File
@@ -93,41 +93,6 @@ func FindEnvVar(
return envVar, nil return envVar, nil
} }
// findAllByAppID loads all rows for an app, scanning each row into a
// new model created by newFn. entity names the model in error messages.
func findAllByAppID[T interface{ scanDest() []any }](
ctx context.Context,
db *database.Database,
query, appID, entity string,
newFn func(*database.Database) T,
) ([]T, error) {
rows, err := db.Query(ctx, query, appID)
if err != nil {
return nil, fmt.Errorf("querying %s by app: %w", entity, err)
}
defer func() { _ = rows.Close() }()
var items []T
for rows.Next() {
item := newFn(db)
scanErr := rows.Scan(item.scanDest()...)
if scanErr != nil {
return nil, scanErr
}
items = append(items, item)
}
return items, rows.Err()
}
func (e *EnvVar) scanDest() []any {
return []any{&e.ID, &e.AppID, &e.Key, &e.Value}
}
// FindEnvVarsByAppID finds all env vars for an app. // FindEnvVarsByAppID finds all env vars for an app.
func FindEnvVarsByAppID( func FindEnvVarsByAppID(
ctx context.Context, ctx context.Context,
@@ -138,7 +103,29 @@ func FindEnvVarsByAppID(
SELECT id, app_id, key, value FROM app_env_vars SELECT id, app_id, key, value FROM app_env_vars
WHERE app_id = ? ORDER BY key` WHERE app_id = ? ORDER BY key`
return findAllByAppID(ctx, db, query, appID, "env vars", NewEnvVar) rows, err := db.Query(ctx, query, appID)
if err != nil {
return nil, fmt.Errorf("querying env vars by app: %w", err)
}
defer func() { _ = rows.Close() }()
var envVars []*EnvVar
for rows.Next() {
envVar := NewEnvVar(db)
scanErr := rows.Scan(
&envVar.ID, &envVar.AppID, &envVar.Key, &envVar.Value,
)
if scanErr != nil {
return nil, scanErr
}
envVars = append(envVars, envVar)
}
return envVars, rows.Err()
} }
// EnvVarPair is a key-value pair for bulk env var operations. // EnvVarPair is a key-value pair for bulk env var operations.
+21 -5
View File
@@ -93,10 +93,6 @@ func FindLabel(
return label, nil return label, nil
} }
func (l *Label) scanDest() []any {
return []any{&l.ID, &l.AppID, &l.Key, &l.Value}
}
// FindLabelsByAppID finds all labels for an app. // FindLabelsByAppID finds all labels for an app.
func FindLabelsByAppID( func FindLabelsByAppID(
ctx context.Context, ctx context.Context,
@@ -107,7 +103,27 @@ func FindLabelsByAppID(
SELECT id, app_id, key, value FROM app_labels SELECT id, app_id, key, value FROM app_labels
WHERE app_id = ? ORDER BY key` WHERE app_id = ? ORDER BY key`
return findAllByAppID(ctx, db, query, appID, "labels", NewLabel) rows, err := db.Query(ctx, query, appID)
if err != nil {
return nil, fmt.Errorf("querying labels by app: %w", err)
}
defer func() { _ = rows.Close() }()
var labels []*Label
for rows.Next() {
label := NewLabel(db)
scanErr := rows.Scan(&label.ID, &label.AppID, &label.Key, &label.Value)
if scanErr != nil {
return nil, scanErr
}
labels = append(labels, label)
}
return labels, rows.Err()
} }
// DeleteLabelsByAppID deletes all labels for an app. // DeleteLabelsByAppID deletes all labels for an app.
+42 -60
View File
@@ -317,54 +317,33 @@ func TestAllApps(t *testing.T) {
// EnvVar Tests. // EnvVar Tests.
// testKVCreateAndFind exercises the create-and-find round trip shared
// by key-value models (env vars, labels).
func testKVCreateAndFind[T any](
t *testing.T,
wantKey string,
create func(db *database.Database, appID string) (int64, error),
find func(context.Context, *database.Database, string) ([]T, error),
keyOf func(T) string,
) {
t.Helper()
testDB, cleanup := setupTestDB(t)
defer cleanup()
// Create app first.
app := createTestApp(t, testDB)
id, err := create(testDB, app.ID)
require.NoError(t, err)
assert.NotZero(t, id)
found, err := find(context.Background(), testDB, app.ID)
require.NoError(t, err)
require.Len(t, found, 1)
assert.Equal(t, wantKey, keyOf(found[0]))
}
func saveTestEnvVar(db *database.Database, appID string) (int64, error) {
envVar := models.NewEnvVar(db)
envVar.AppID = appID
envVar.Key = "DATABASE_URL"
envVar.Value = "postgres://localhost/db"
err := envVar.Save(context.Background())
return envVar.ID, err
}
func TestEnvVarCRUD(t *testing.T) { func TestEnvVarCRUD(t *testing.T) {
t.Parallel() t.Parallel()
t.Run("creates and finds env vars", func(t *testing.T) { t.Run("creates and finds env vars", func(t *testing.T) {
t.Parallel() t.Parallel()
testKVCreateAndFind(t, "DATABASE_URL", saveTestEnvVar, testDB, cleanup := setupTestDB(t)
models.FindEnvVarsByAppID, defer cleanup()
func(e *models.EnvVar) string { return e.Key },
// Create app first.
app := createTestApp(t, testDB)
envVar := models.NewEnvVar(testDB)
envVar.AppID = app.ID
envVar.Key = "DATABASE_URL"
envVar.Value = "postgres://localhost/db"
err := envVar.Save(context.Background())
require.NoError(t, err)
assert.NotZero(t, envVar.ID)
envVars, err := models.FindEnvVarsByAppID(
context.Background(), testDB, app.ID,
) )
require.NoError(t, err)
require.Len(t, envVars, 1)
assert.Equal(t, "DATABASE_URL", envVars[0].Key)
}) })
t.Run("deletes env var", func(t *testing.T) { t.Run("deletes env var", func(t *testing.T) {
@@ -396,27 +375,32 @@ func TestEnvVarCRUD(t *testing.T) {
// Label Tests. // Label Tests.
func saveTestLabel(db *database.Database, appID string) (int64, error) {
label := models.NewLabel(db)
label.AppID = appID
label.Key = "traefik.enable"
label.Value = "true"
err := label.Save(context.Background())
return label.ID, err
}
func TestLabelCRUD(t *testing.T) { func TestLabelCRUD(t *testing.T) {
t.Parallel() t.Parallel()
t.Run("creates and finds labels", func(t *testing.T) { t.Run("creates and finds labels", func(t *testing.T) {
t.Parallel() t.Parallel()
testKVCreateAndFind(t, "traefik.enable", saveTestLabel, testDB, cleanup := setupTestDB(t)
models.FindLabelsByAppID, defer cleanup()
func(l *models.Label) string { return l.Key },
app := createTestApp(t, testDB)
label := models.NewLabel(testDB)
label.AppID = app.ID
label.Key = "traefik.enable"
label.Value = "true"
err := label.Save(context.Background())
require.NoError(t, err)
assert.NotZero(t, label.ID)
labels, err := models.FindLabelsByAppID(
context.Background(), testDB, app.ID,
) )
require.NoError(t, err)
require.Len(t, labels, 1)
assert.Equal(t, "traefik.enable", labels[0].Key)
}) })
} }
@@ -585,9 +569,7 @@ func TestDeploymentFindByAppID(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
} }
deployments, err := models.FindDeploymentsByAppID( deployments, err := models.FindDeploymentsByAppID(context.Background(), testDB, app.ID, 3)
context.Background(), testDB, app.ID, 3,
)
require.NoError(t, err) require.NoError(t, err)
assert.Len(t, deployments, 3) assert.Len(t, deployments, 3)
} }
@@ -724,6 +706,7 @@ func TestAppGetWebhookEvents(t *testing.T) {
// Cascade Delete Tests. // Cascade Delete Tests.
//nolint:funlen // Test function with many assertions - acceptable for integration tests
func TestCascadeDelete(t *testing.T) { func TestCascadeDelete(t *testing.T) {
t.Parallel() t.Parallel()
@@ -800,8 +783,7 @@ func TestCascadeDelete(t *testing.T) {
// Resource Limits Tests. // Resource Limits Tests.
//nolint:funlen // integration test with multiple subtests func TestAppResourceLimits(t *testing.T) { //nolint:funlen // integration test with multiple subtests
func TestAppResourceLimits(t *testing.T) {
t.Parallel() t.Parallel()
t.Run("saves and loads CPU limit", func(t *testing.T) { t.Run("saves and loads CPU limit", func(t *testing.T) {
+24 -7
View File
@@ -112,12 +112,6 @@ func FindPort(
return port, nil return port, nil
} }
func (p *Port) scanDest() []any {
return []any{
&p.ID, &p.AppID, &p.HostPort, &p.ContainerPort, &p.Protocol,
}
}
// FindPortsByAppID finds all ports for an app. // FindPortsByAppID finds all ports for an app.
func FindPortsByAppID( func FindPortsByAppID(
ctx context.Context, ctx context.Context,
@@ -128,7 +122,30 @@ func FindPortsByAppID(
SELECT id, app_id, host_port, container_port, protocol SELECT id, app_id, host_port, container_port, protocol
FROM app_ports WHERE app_id = ? ORDER BY host_port` FROM app_ports WHERE app_id = ? ORDER BY host_port`
return findAllByAppID(ctx, db, query, appID, "ports", NewPort) rows, err := db.Query(ctx, query, appID)
if err != nil {
return nil, fmt.Errorf("querying ports by app: %w", err)
}
defer func() { _ = rows.Close() }()
var ports []*Port
for rows.Next() {
port := NewPort(db)
scanErr := rows.Scan(
&port.ID, &port.AppID, &port.HostPort,
&port.ContainerPort, &port.Protocol,
)
if scanErr != nil {
return nil, scanErr
}
ports = append(ports, port)
}
return ports, rows.Err()
} }
// DeletePortsByAppID deletes all ports for an app. // DeletePortsByAppID deletes all ports for an app.
+24 -7
View File
@@ -103,12 +103,6 @@ func FindVolume(
return vol, nil return vol, nil
} }
func (v *Volume) scanDest() []any {
return []any{
&v.ID, &v.AppID, &v.HostPath, &v.ContainerPath, &v.ReadOnly,
}
}
// FindVolumesByAppID finds all volumes for an app. // FindVolumesByAppID finds all volumes for an app.
func FindVolumesByAppID( func FindVolumesByAppID(
ctx context.Context, ctx context.Context,
@@ -119,7 +113,30 @@ func FindVolumesByAppID(
SELECT id, app_id, host_path, container_path, readonly SELECT id, app_id, host_path, container_path, readonly
FROM app_volumes WHERE app_id = ? ORDER BY container_path` FROM app_volumes WHERE app_id = ? ORDER BY container_path`
return findAllByAppID(ctx, db, query, appID, "volumes", NewVolume) rows, err := db.Query(ctx, query, appID)
if err != nil {
return nil, fmt.Errorf("querying volumes by app: %w", err)
}
defer func() { _ = rows.Close() }()
var volumes []*Volume
for rows.Next() {
vol := NewVolume(db)
scanErr := rows.Scan(
&vol.ID, &vol.AppID, &vol.HostPath,
&vol.ContainerPath, &vol.ReadOnly,
)
if scanErr != nil {
return nil, scanErr
}
volumes = append(volumes, vol)
}
return volumes, rows.Err()
} }
// DeleteVolumesByAppID deletes all volumes for an app. // DeleteVolumesByAppID deletes all volumes for an app.
+2 -8
View File
@@ -71,14 +71,8 @@ func (s *Server) SetupRoutes() {
r.Post("/apps/{id}/deployments/cancel", s.handlers.HandleCancelDeploy()) r.Post("/apps/{id}/deployments/cancel", s.handlers.HandleCancelDeploy())
r.Get("/apps/{id}/deployments", s.handlers.HandleAppDeployments()) r.Get("/apps/{id}/deployments", s.handlers.HandleAppDeployments())
r.Get("/apps/{id}/webhooks", s.handlers.HandleAppWebhookEvents()) r.Get("/apps/{id}/webhooks", s.handlers.HandleAppWebhookEvents())
r.Get( r.Get("/apps/{id}/deployments/{deploymentID}/logs", s.handlers.HandleDeploymentLogsAPI())
"/apps/{id}/deployments/{deploymentID}/logs", r.Get("/apps/{id}/deployments/{deploymentID}/download", s.handlers.HandleDeploymentLogDownload())
s.handlers.HandleDeploymentLogsAPI(),
)
r.Get(
"/apps/{id}/deployments/{deploymentID}/download",
s.handlers.HandleDeploymentLogDownload(),
)
r.Get("/apps/{id}/logs", s.handlers.HandleAppLogs()) r.Get("/apps/{id}/logs", s.handlers.HandleAppLogs())
r.Get("/apps/{id}/container-logs", s.handlers.HandleContainerLogsAPI()) r.Get("/apps/{id}/container-logs", s.handlers.HandleContainerLogsAPI())
r.Get("/apps/{id}/status", s.handlers.HandleAppStatusAPI()) r.Get("/apps/{id}/status", s.handlers.HandleAppStatusAPI())
+47 -77
View File
@@ -16,12 +16,6 @@ import (
"sneak.berlin/go/upaas/internal/service/app" "sneak.berlin/go/upaas/internal/service/app"
) )
// testRepoURL is the default repository URL used across tests.
const testRepoURL = "git@example.com:user/repo.git"
// giteaRepoURL is the gitea repository URL used across tests.
const giteaRepoURL = "git@gitea.example.com:user/repo.git"
func setupTestService(t *testing.T) (*app.Service, func()) { func setupTestService(t *testing.T) (*app.Service, func()) {
t.Helper() t.Helper()
@@ -64,8 +58,7 @@ func setupTestService(t *testing.T) (*app.Service, func()) {
} }
// deleteItemTestHelper is a generic helper for testing delete operations. // deleteItemTestHelper is a generic helper for testing delete operations.
// It creates an app, adds an item, verifies it exists, deletes it, and // It creates an app, adds an item, verifies it exists, deletes it, and verifies it's gone.
// verifies it's gone.
func deleteItemTestHelper( func deleteItemTestHelper(
t *testing.T, t *testing.T,
appName string, appName string,
@@ -80,7 +73,7 @@ func deleteItemTestHelper(
createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{ createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{
Name: appName, Name: appName,
RepoURL: testRepoURL, RepoURL: "git@example.com:user/repo.git",
}) })
require.NoError(t, err) require.NoError(t, err)
@@ -99,35 +92,6 @@ func deleteItemTestHelper(
assert.Equal(t, 0, count) assert.Equal(t, 0, count)
} }
// runDeleteItemTest adapts typed list/delete callbacks so delete tests for
// different item types can share deleteItemTestHelper.
func runDeleteItemTest[T any](
t *testing.T,
appName string,
addItem func(ctx context.Context, svc *app.Service, appID string) error,
listItems func(ctx context.Context, application *models.App) ([]T, error),
deleteFirst func(ctx context.Context, svc *app.Service, item T) error,
) {
t.Helper()
deleteItemTestHelper(t, appName,
addItem,
func(ctx context.Context, application *models.App) (int, error) {
items, err := listItems(ctx, application)
return len(items), err
},
func(ctx context.Context, svc *app.Service, application *models.App) error {
items, err := listItems(ctx, application)
if err != nil {
return err
}
return deleteFirst(ctx, svc, items[0])
},
)
}
func TestCreateAppWithGeneratedKeys(t *testing.T) { func TestCreateAppWithGeneratedKeys(t *testing.T) {
t.Parallel() t.Parallel()
@@ -136,7 +100,7 @@ func TestCreateAppWithGeneratedKeys(t *testing.T) {
input := app.CreateAppInput{ input := app.CreateAppInput{
Name: "test-app", Name: "test-app",
RepoURL: giteaRepoURL, RepoURL: "git@gitea.example.com:user/repo.git",
Branch: "main", Branch: "main",
DockerfilePath: "Dockerfile", DockerfilePath: "Dockerfile",
} }
@@ -146,7 +110,7 @@ func TestCreateAppWithGeneratedKeys(t *testing.T) {
require.NotNil(t, createdApp) require.NotNil(t, createdApp)
assert.Equal(t, "test-app", createdApp.Name) assert.Equal(t, "test-app", createdApp.Name)
assert.Equal(t, giteaRepoURL, createdApp.RepoURL) assert.Equal(t, "git@gitea.example.com:user/repo.git", createdApp.RepoURL)
assert.Equal(t, "main", createdApp.Branch) assert.Equal(t, "main", createdApp.Branch)
assert.Equal(t, "Dockerfile", createdApp.DockerfilePath) assert.Equal(t, "Dockerfile", createdApp.DockerfilePath)
assert.NotEmpty(t, createdApp.ID) assert.NotEmpty(t, createdApp.ID)
@@ -166,7 +130,7 @@ func TestCreateAppDefaults(t *testing.T) {
input := app.CreateAppInput{ input := app.CreateAppInput{
Name: "test-app-defaults", Name: "test-app-defaults",
RepoURL: giteaRepoURL, RepoURL: "git@gitea.example.com:user/repo.git",
} }
createdApp, err := svc.CreateApp(context.Background(), input) createdApp, err := svc.CreateApp(context.Background(), input)
@@ -184,7 +148,7 @@ func TestCreateAppOptionalFields(t *testing.T) {
input := app.CreateAppInput{ input := app.CreateAppInput{
Name: "test-app-full", Name: "test-app-full",
RepoURL: giteaRepoURL, RepoURL: "git@gitea.example.com:user/repo.git",
Branch: "develop", Branch: "develop",
DockerNetwork: "my-network", DockerNetwork: "my-network",
NtfyTopic: "https://ntfy.sh/my-topic", NtfyTopic: "https://ntfy.sh/my-topic",
@@ -212,7 +176,7 @@ func TestUpdateApp(testingT *testing.T) {
createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{ createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{
Name: "original-name", Name: "original-name",
RepoURL: testRepoURL, RepoURL: "git@example.com:user/repo.git",
}) })
require.NoError(t, err) require.NoError(t, err)
@@ -244,7 +208,7 @@ func TestUpdateApp(testingT *testing.T) {
createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{ createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{
Name: "test-clear", Name: "test-clear",
RepoURL: testRepoURL, RepoURL: "git@example.com:user/repo.git",
NtfyTopic: "https://ntfy.sh/topic", NtfyTopic: "https://ntfy.sh/topic",
SlackWebhook: "https://slack.com/hook", SlackWebhook: "https://slack.com/hook",
}) })
@@ -252,7 +216,7 @@ func TestUpdateApp(testingT *testing.T) {
err = svc.UpdateApp(context.Background(), createdApp, app.UpdateAppInput{ err = svc.UpdateApp(context.Background(), createdApp, app.UpdateAppInput{
Name: "test-clear", Name: "test-clear",
RepoURL: testRepoURL, RepoURL: "git@example.com:user/repo.git",
Branch: "main", Branch: "main",
}) })
require.NoError(t, err) require.NoError(t, err)
@@ -276,7 +240,7 @@ func TestDeleteApp(testingT *testing.T) {
createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{ createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{
Name: "to-delete", Name: "to-delete",
RepoURL: testRepoURL, RepoURL: "git@example.com:user/repo.git",
}) })
require.NoError(t, err) require.NoError(t, err)
@@ -300,7 +264,7 @@ func TestGetApp(testingT *testing.T) {
created, err := svc.CreateApp(context.Background(), app.CreateAppInput{ created, err := svc.CreateApp(context.Background(), app.CreateAppInput{
Name: "findable-app", Name: "findable-app",
RepoURL: testRepoURL, RepoURL: "git@example.com:user/repo.git",
}) })
require.NoError(t, err) require.NoError(t, err)
@@ -335,7 +299,7 @@ func TestGetAppByWebhookSecret(testingT *testing.T) {
created, err := svc.CreateApp(context.Background(), app.CreateAppInput{ created, err := svc.CreateApp(context.Background(), app.CreateAppInput{
Name: "webhook-app", Name: "webhook-app",
RepoURL: testRepoURL, RepoURL: "git@example.com:user/repo.git",
}) })
require.NoError(t, err) require.NoError(t, err)
@@ -414,7 +378,7 @@ func TestEnvVarsAddAndRetrieve(t *testing.T) {
createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{ createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{
Name: "env-test", Name: "env-test",
RepoURL: testRepoURL, RepoURL: "git@example.com:user/repo.git",
}) })
require.NoError(t, err) require.NoError(t, err)
@@ -447,33 +411,29 @@ func TestEnvVarsAddAndRetrieve(t *testing.T) {
assert.Equal(t, "secret123", keys["API_KEY"]) assert.Equal(t, "secret123", keys["API_KEY"])
} }
// addDeletableEnvVar seeds the env var removed in the delete test.
func addDeletableEnvVar(
ctx context.Context, svc *app.Service, appID string,
) error {
return svc.AddEnvVar(ctx, appID, "TO_DELETE", "value")
}
func TestEnvVarsDelete(t *testing.T) { func TestEnvVarsDelete(t *testing.T) {
t.Parallel() t.Parallel()
runDeleteItemTest(t, "env-delete-test", addDeletableEnvVar, deleteItemTestHelper(t, "env-delete-test",
func(ctx context.Context, application *models.App) ([]*models.EnvVar, error) { func(ctx context.Context, svc *app.Service, appID string) error {
return application.GetEnvVars(ctx) return svc.AddEnvVar(ctx, appID, "TO_DELETE", "value")
}, },
func(ctx context.Context, svc *app.Service, item *models.EnvVar) error { func(ctx context.Context, application *models.App) (int, error) {
return svc.DeleteEnvVar(ctx, item.ID) envVars, err := application.GetEnvVars(ctx)
return len(envVars), err
},
func(ctx context.Context, svc *app.Service, application *models.App) error {
envVars, err := application.GetEnvVars(ctx)
if err != nil {
return err
}
return svc.DeleteEnvVar(ctx, envVars[0].ID)
}, },
) )
} }
// addDeletableLabel seeds the label removed in the delete test.
func addDeletableLabel(
ctx context.Context, svc *app.Service, appID string,
) error {
return svc.AddLabel(ctx, appID, "to.delete", "value")
}
func TestLabels(testingT *testing.T) { func TestLabels(testingT *testing.T) {
testingT.Parallel() testingT.Parallel()
@@ -485,7 +445,7 @@ func TestLabels(testingT *testing.T) {
createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{ createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{
Name: "label-test", Name: "label-test",
RepoURL: testRepoURL, RepoURL: "git@example.com:user/repo.git",
}) })
require.NoError(t, err) require.NoError(t, err)
@@ -508,12 +468,22 @@ func TestLabels(testingT *testing.T) {
testingT.Run("deletes label", func(t *testing.T) { testingT.Run("deletes label", func(t *testing.T) {
t.Parallel() t.Parallel()
runDeleteItemTest(t, "label-delete-test", addDeletableLabel, deleteItemTestHelper(t, "label-delete-test",
func(ctx context.Context, application *models.App) ([]*models.Label, error) { func(ctx context.Context, svc *app.Service, appID string) error {
return application.GetLabels(ctx) return svc.AddLabel(ctx, appID, "to.delete", "value")
}, },
func(ctx context.Context, svc *app.Service, item *models.Label) error { func(ctx context.Context, application *models.App) (int, error) {
return svc.DeleteLabel(ctx, item.ID) labels, err := application.GetLabels(ctx)
return len(labels), err
},
func(ctx context.Context, svc *app.Service, application *models.App) error {
labels, err := application.GetLabels(ctx)
if err != nil {
return err
}
return svc.DeleteLabel(ctx, labels[0].ID)
}, },
) )
}) })
@@ -527,7 +497,7 @@ func TestVolumesAddAndRetrieve(t *testing.T) {
createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{ createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{
Name: "volume-test", Name: "volume-test",
RepoURL: testRepoURL, RepoURL: "git@example.com:user/repo.git",
}) })
require.NoError(t, err) require.NoError(t, err)
@@ -577,7 +547,7 @@ func TestVolumesDelete(t *testing.T) {
createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{ createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{
Name: "volume-delete-test", Name: "volume-delete-test",
RepoURL: testRepoURL, RepoURL: "git@example.com:user/repo.git",
}) })
require.NoError(t, err) require.NoError(t, err)
@@ -613,7 +583,7 @@ func TestUpdateAppStatus(testingT *testing.T) {
createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{ createdApp, err := svc.CreateApp(context.Background(), app.CreateAppInput{
Name: "status-test", Name: "status-test",
RepoURL: testRepoURL, RepoURL: "git@example.com:user/repo.git",
}) })
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, models.AppStatusPending, createdApp.Status) assert.Equal(t, models.AppStatusPending, createdApp.Status)
+3 -14
View File
@@ -144,11 +144,7 @@ func TestSessionCookieSecureFlag(testingT *testing.T) {
svc := setupAuthService(t, false) svc := setupAuthService(t, false)
cookie := getSessionCookie(t, svc) cookie := getSessionCookie(t, svc)
require.NotNil(t, cookie, "session cookie should exist") require.NotNil(t, cookie, "session cookie should exist")
assert.True( assert.True(t, cookie.Secure, "session cookie should have Secure flag in production mode")
t,
cookie.Secure,
"session cookie should have Secure flag in production mode",
)
}) })
} }
@@ -328,12 +324,7 @@ func TestCreateUserRaceCondition(testingT *testing.T) {
} }
assert.Equal(t, 1, successes, "exactly one goroutine should succeed") assert.Equal(t, 1, successes, "exactly one goroutine should succeed")
assert.Equal( assert.Equal(t, goroutines-1, failures, "all other goroutines should fail with ErrUserExists")
t,
goroutines-1,
failures,
"all other goroutines should fail with ErrUserExists",
)
}) })
} }
@@ -389,9 +380,7 @@ func TestDestroySessionMaxAge(testingT *testing.T) {
defer cleanup() defer cleanup()
recorder := httptest.NewRecorder() recorder := httptest.NewRecorder()
request := httptest.NewRequestWithContext( request := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
t.Context(), http.MethodGet, "/", nil,
)
err := svc.DestroySession(recorder, request) err := svc.DestroySession(recorder, request)
require.NoError(t, err) require.NoError(t, err)
+14 -66
View File
@@ -66,8 +66,7 @@ const logFilePermissions = 0o640
// logTimestampFormat is the format for log file timestamps. // logTimestampFormat is the format for log file timestamps.
const logTimestampFormat = "20060102T150405Z" const logTimestampFormat = "20060102T150405Z"
// logFileShortSHALength is the number of characters to use for commit SHA // logFileShortSHALength is the number of characters to use for commit SHA in log filenames.
// in log filenames.
const logFileShortSHALength = 12 const logFileShortSHALength = 12
// dockerLogMessage represents a Docker build log message. // dockerLogMessage represents a Docker build log message.
@@ -88,10 +87,7 @@ type deploymentLogWriter struct {
flushCtx context.Context //nolint:containedctx // needed for async flush goroutine flushCtx context.Context //nolint:containedctx // needed for async flush goroutine
} }
func newDeploymentLogWriter( func newDeploymentLogWriter(ctx context.Context, deployment *models.Deployment) *deploymentLogWriter {
ctx context.Context,
deployment *models.Deployment,
) *deploymentLogWriter {
w := &deploymentLogWriter{ w := &deploymentLogWriter{
deployment: deployment, deployment: deployment,
done: make(chan struct{}), done: make(chan struct{}),
@@ -261,10 +257,7 @@ func (svc *Service) GetBuildDir(appName string) string {
// GetLogFilePath returns the path to the log file for a deployment. // GetLogFilePath returns the path to the log file for a deployment.
// Returns empty string if the path cannot be determined. // Returns empty string if the path cannot be determined.
func (svc *Service) GetLogFilePath( func (svc *Service) GetLogFilePath(app *models.App, deployment *models.Deployment) string {
app *models.App,
deployment *models.Deployment,
) string {
hostname, err := os.Hostname() hostname, err := os.Hostname()
if err != nil { if err != nil {
hostname = "unknown" hostname = "unknown"
@@ -282,8 +275,7 @@ func (svc *Service) GetLogFilePath(
// Use started_at timestamp // Use started_at timestamp
timestamp := deployment.StartedAt.UTC().Format(logTimestampFormat) timestamp := deployment.StartedAt.UTC().Format(logTimestampFormat)
// Build filename: appname_sha_timestamp.log.txt // Build filename: appname_sha_timestamp.log.txt (or appname_timestamp.log.txt if no SHA)
// (or appname_timestamp.log.txt if no SHA)
var filename string var filename string
if sha != "" { if sha != "" {
filename = fmt.Sprintf("%s_%s_%s.log.txt", app.Name, sha, timestamp) filename = fmt.Sprintf("%s_%s_%s.log.txt", app.Name, sha, timestamp)
@@ -294,12 +286,6 @@ 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)
@@ -322,8 +308,7 @@ func (svc *Service) CancelDeploy(appID string) bool {
// Deploy deploys an app. If cancelExisting is true (e.g. webhook-triggered), // Deploy deploys an app. If cancelExisting is true (e.g. webhook-triggered),
// any in-progress deploy for the same app will be cancelled before starting. // any in-progress deploy for the same app will be cancelled before starting.
// If cancelExisting is false and a deploy is in progress, // If cancelExisting is false and a deploy is in progress, ErrDeploymentInProgress is returned.
// ErrDeploymentInProgress is returned.
func (svc *Service) Deploy( func (svc *Service) Deploy(
ctx context.Context, ctx context.Context,
app *models.App, app *models.App,
@@ -357,8 +342,7 @@ func (svc *Service) Deploy(
// Fetch webhook event and create deployment record // Fetch webhook event and create deployment record
webhookEvent := svc.fetchWebhookEvent(deployCtx, webhookEventID) webhookEvent := svc.fetchWebhookEvent(deployCtx, webhookEventID)
// Use a background context for DB operations that must complete // Use a background context for DB operations that must complete regardless of cancellation
// regardless of cancellation
bgCtx := context.WithoutCancel(deployCtx) bgCtx := context.WithoutCancel(deployCtx)
deployment, err := svc.createDeploymentRecord(bgCtx, app, webhookEventID, webhookEvent) deployment, err := svc.createDeploymentRecord(bgCtx, app, webhookEventID, webhookEvent)
@@ -417,10 +401,7 @@ func (svc *Service) createRollbackDeployment(
return nil, fmt.Errorf("failed to create rollback deployment: %w", saveErr) return nil, fmt.Errorf("failed to create rollback deployment: %w", saveErr)
} }
_ = deployment.AppendLog( _ = deployment.AppendLog(ctx, "Rolling back to previous image: "+app.PreviousImageID.String)
ctx,
"Rolling back to previous image: "+app.PreviousImageID.String,
)
return deployment, nil return deployment, nil
} }
@@ -436,11 +417,7 @@ func (svc *Service) executeRollback(
svc.removeOldContainer(ctx, app, deployment) svc.removeOldContainer(ctx, app, deployment)
rollbackOpts, err := svc.buildContainerOptions( rollbackOpts, err := svc.buildContainerOptions(ctx, app, docker.ImageID(previousImageID))
ctx,
app,
docker.ImageID(previousImageID),
)
if err != nil { if err != nil {
svc.failDeployment(bgCtx, app, deployment, err) svc.failDeployment(bgCtx, app, deployment, err)
@@ -449,12 +426,7 @@ func (svc *Service) executeRollback(
containerID, err := svc.docker.CreateContainer(ctx, rollbackOpts) containerID, err := svc.docker.CreateContainer(ctx, rollbackOpts)
if err != nil { if err != nil {
svc.failDeployment( svc.failDeployment(bgCtx, app, deployment, fmt.Errorf("failed to create rollback container: %w", err))
bgCtx,
app,
deployment,
fmt.Errorf("failed to create rollback container: %w", err),
)
return fmt.Errorf("failed to create rollback container: %w", err) return fmt.Errorf("failed to create rollback container: %w", err)
} }
@@ -464,12 +436,7 @@ func (svc *Service) executeRollback(
startErr := svc.docker.StartContainer(ctx, containerID) startErr := svc.docker.StartContainer(ctx, containerID)
if startErr != nil { if startErr != nil {
svc.failDeployment( svc.failDeployment(bgCtx, app, deployment, fmt.Errorf("failed to start rollback container: %w", startErr))
bgCtx,
app,
deployment,
fmt.Errorf("failed to start rollback container: %w", startErr),
)
return fmt.Errorf("failed to start rollback container: %w", startErr) return fmt.Errorf("failed to start rollback container: %w", startErr)
} }
@@ -728,11 +695,7 @@ func (svc *Service) cleanupCancelledDeploy(
if removeErr != nil { if removeErr != nil {
svc.log.Error("failed to remove image from cancelled deploy", svc.log.Error("failed to remove image from cancelled deploy",
"error", removeErr, "app", app.Name, "image", imageID) "error", removeErr, "app", app.Name, "image", imageID)
_ = deployment.AppendLog( _ = deployment.AppendLog(ctx, "WARNING: failed to clean up image "+imageID.String()+": "+removeErr.Error())
ctx,
"WARNING: failed to clean up image "+
imageID.String()+": "+removeErr.Error(),
)
} else { } else {
svc.log.Info("cleaned up image from cancelled deploy", svc.log.Info("cleaned up image from cancelled deploy",
"app", app.Name, "image", imageID) "app", app.Name, "image", imageID)
@@ -907,24 +870,14 @@ func (svc *Service) cloneRepository(
err := os.MkdirAll(appBuildsDir, buildsDirPermissions) err := os.MkdirAll(appBuildsDir, buildsDirPermissions)
if err != nil { if err != nil {
svc.failDeployment( svc.failDeployment(ctx, app, deployment, fmt.Errorf("failed to create builds dir: %w", err))
ctx,
app,
deployment,
fmt.Errorf("failed to create builds dir: %w", err),
)
return "", nil, fmt.Errorf("failed to create builds dir: %w", err) return "", nil, fmt.Errorf("failed to create builds dir: %w", err)
} }
buildDir, err := os.MkdirTemp(appBuildsDir, fmt.Sprintf("%d-*", deployment.ID)) buildDir, err := os.MkdirTemp(appBuildsDir, fmt.Sprintf("%d-*", deployment.ID))
if err != nil { if err != nil {
svc.failDeployment( svc.failDeployment(ctx, app, deployment, fmt.Errorf("failed to create temp dir: %w", err))
ctx,
app,
deployment,
fmt.Errorf("failed to create temp dir: %w", err),
)
return "", nil, fmt.Errorf("failed to create temp dir: %w", err) return "", nil, fmt.Errorf("failed to create temp dir: %w", err)
} }
@@ -955,12 +908,7 @@ func (svc *Service) cloneRepository(
) )
if cloneErr != nil { if cloneErr != nil {
cleanup() cleanup()
svc.failDeployment( svc.failDeployment(ctx, app, deployment, fmt.Errorf("failed to clone repo: %w", cloneErr))
ctx,
app,
deployment,
fmt.Errorf("failed to clone repo: %w", cloneErr),
)
return "", nil, fmt.Errorf("failed to clone repo: %w", cloneErr) return "", nil, fmt.Errorf("failed to clone repo: %w", cloneErr)
} }
@@ -32,10 +32,7 @@ func TestCleanupCancelledDeploy_RemovesBuildDir(t *testing.T) {
require.NoError(t, os.MkdirAll(deployDir, 0o750)) require.NoError(t, os.MkdirAll(deployDir, 0o750))
// Create a file inside to verify full removal // Create a file inside to verify full removal
require.NoError( require.NoError(t, os.WriteFile(filepath.Join(deployDir, "work"), []byte("test"), 0o600))
t,
os.WriteFile(filepath.Join(deployDir, "work"), []byte("test"), 0o600),
)
// Also create a dir for a different deployment (should NOT be removed) // Also create a dir for a different deployment (should NOT be removed)
otherDir := filepath.Join(buildDir, "99-xyz789") otherDir := filepath.Join(buildDir, "99-xyz789")
@@ -31,9 +31,7 @@ func TestBuildContainerOptionsUsesImageID(t *testing.T) {
const expectedImageID = docker.ImageID("sha256:abc123def456") const expectedImageID = docker.ImageID("sha256:abc123def456")
opts, err := svc.BuildContainerOptionsExported( opts, err := svc.BuildContainerOptionsExported(context.Background(), app, expectedImageID)
context.Background(), app, expectedImageID,
)
if err != nil { if err != nil {
t.Fatalf("buildContainerOptions returned error: %v", err) t.Fatalf("buildContainerOptions returned error: %v", err)
} }
@@ -79,20 +77,14 @@ func TestBuildContainerOptionsNoResourceLimits(t *testing.T) {
} }
} }
// buildOptsForApp saves an app configured by setup and returns the container func TestBuildContainerOptionsCPULimit(t *testing.T) {
// options built for it. t.Parallel()
func buildOptsForApp(
t *testing.T,
name string,
setup func(app *models.App),
) docker.CreateContainerOptions {
t.Helper()
db := database.NewTestDatabase(t) db := database.NewTestDatabase(t)
app := models.NewApp(db) app := models.NewApp(db)
app.Name = name app.Name = "cpulimit"
setup(app) app.CPULimit = sql.NullFloat64{Float64: 0.5, Valid: true}
err := app.Save(context.Background()) err := app.Save(context.Background())
if err != nil { if err != nil {
@@ -109,16 +101,6 @@ func buildOptsForApp(
t.Fatalf("buildContainerOptions returned error: %v", err) t.Fatalf("buildContainerOptions returned error: %v", err)
} }
return opts
}
func TestBuildContainerOptionsCPULimit(t *testing.T) {
t.Parallel()
opts := buildOptsForApp(t, "cpulimit", func(app *models.App) {
app.CPULimit = sql.NullFloat64{Float64: 0.5, Valid: true}
})
if opts.CPULimit != 0.5 { if opts.CPULimit != 0.5 {
t.Errorf("expected CPULimit=0.5, got %v", opts.CPULimit) t.Errorf("expected CPULimit=0.5, got %v", opts.CPULimit)
} }
@@ -127,9 +109,26 @@ func TestBuildContainerOptionsCPULimit(t *testing.T) {
func TestBuildContainerOptionsMemoryLimit(t *testing.T) { func TestBuildContainerOptionsMemoryLimit(t *testing.T) {
t.Parallel() t.Parallel()
opts := buildOptsForApp(t, "memlimit", func(app *models.App) { db := database.NewTestDatabase(t)
app.MemoryLimit = sql.NullInt64{Int64: 536870912, Valid: true} // 512m
}) app := models.NewApp(db)
app.Name = "memlimit"
app.MemoryLimit = sql.NullInt64{Int64: 536870912, Valid: true} // 512m
err := app.Save(context.Background())
if err != nil {
t.Fatalf("failed to save app: %v", err)
}
log := slog.New(slog.NewTextHandler(os.Stderr, nil))
svc := deploy.NewTestService(log)
opts, err := svc.BuildContainerOptionsExported(
context.Background(), app, docker.ImageID("test:latest"),
)
if err != nil {
t.Fatalf("buildContainerOptions returned error: %v", err)
}
if opts.MemoryLimit != 536870912 { if opts.MemoryLimit != 536870912 {
t.Errorf("expected MemoryLimit=536870912, got %v", opts.MemoryLimit) t.Errorf("expected MemoryLimit=536870912, got %v", opts.MemoryLimit)
+2 -10
View File
@@ -26,11 +26,7 @@ func (svc *Service) CancelActiveDeploy(appID string) {
} }
// RegisterActiveDeploy registers an active deploy for testing. // RegisterActiveDeploy registers an active deploy for testing.
func (svc *Service) RegisterActiveDeploy( func (svc *Service) RegisterActiveDeploy(appID string, cancel context.CancelFunc, done chan struct{}) {
appID string,
cancel context.CancelFunc,
done chan struct{},
) {
svc.activeDeploys.Store(appID, &activeDeploy{cancel: cancel, done: done}) svc.activeDeploys.Store(appID, &activeDeploy{cancel: cancel, done: done})
} }
@@ -45,11 +41,7 @@ func (svc *Service) UnlockApp(appID string) {
} }
// NewTestServiceWithConfig creates a Service with config and docker client for testing. // NewTestServiceWithConfig creates a Service with config and docker client for testing.
func NewTestServiceWithConfig( func NewTestServiceWithConfig(log *slog.Logger, cfg *config.Config, dockerClient *docker.Client) *Service {
log *slog.Logger,
cfg *config.Config,
dockerClient *docker.Client,
) *Service {
return &Service{ return &Service{
log: log, log: log,
config: cfg, config: cfg,
+3 -6
View File
@@ -159,8 +159,7 @@ func (svc *Service) NotifyDeployFailed(
) { ) {
duration := time.Since(deployment.StartedAt) duration := time.Since(deployment.StartedAt)
title := "Deploy failed: " + app.Name title := "Deploy failed: " + app.Name
message := "Deployment failed after " + formatDuration(duration) + message := "Deployment failed after " + formatDuration(duration) + ": " + deployErr.Error()
": " + deployErr.Error()
svc.sendNotifications(ctx, app, title, message, message, "error") svc.sendNotifications(ctx, app, title, message, message, "error")
} }
@@ -267,8 +266,7 @@ func (svc *Service) sendNtfy(
request.Header.Set("Title", title) request.Header.Set("Title", title)
request.Header.Set("Priority", svc.ntfyPriority(priority)) request.Header.Set("Priority", svc.ntfyPriority(priority))
// #nosec G704 -- URL from validated config, not user input resp, err := svc.client.Do(request) // #nosec G704 -- URL from validated config, not user input
resp, err := svc.client.Do(request)
if err != nil { if err != nil {
return fmt.Errorf("failed to send ntfy request: %w", err) return fmt.Errorf("failed to send ntfy request: %w", err)
} }
@@ -365,8 +363,7 @@ func (svc *Service) sendSlack(
request.Header.Set("Content-Type", "application/json") request.Header.Set("Content-Type", "application/json")
// #nosec G704 -- URL from validated config, not user input resp, err := svc.client.Do(request) // #nosec G704 -- URL from validated config, not user input
resp, err := svc.client.Do(request)
if err != nil { if err != nil {
return fmt.Errorf("failed to send slack request: %w", err) return fmt.Errorf("failed to send slack request: %w", err)
} }
+60 -49
View File
@@ -98,77 +98,88 @@ type GitLabPushPayload struct {
func ParsePushPayload(source Source, payload []byte) (*PushEvent, error) { func ParsePushPayload(source Source, payload []byte) (*PushEvent, error) {
switch source { switch source {
case SourceGitHub: case SourceGitHub:
return parsePush(payload, githubPushEvent) return parseGitHubPush(payload)
case SourceGitLab: case SourceGitLab:
return parsePush(payload, gitlabPushEvent) return parseGitLabPush(payload)
case SourceGitea, SourceUnknown: case SourceGitea, SourceUnknown:
// Gitea and unknown both use Gitea format for backward compatibility. // Gitea and unknown both use Gitea format for backward compatibility.
return parsePush(payload, giteaPushEvent) return parseGiteaPush(payload)
} }
// Unreachable for known source values, but satisfies exhaustive checker. // Unreachable for known source values, but satisfies exhaustive checker.
return parsePush(payload, giteaPushEvent) return parseGiteaPush(payload)
} }
// parsePush unmarshals payload into P and converts it into a normalized func parseGiteaPush(payload []byte) (*PushEvent, error) {
// PushEvent via build. var p GiteaPushPayload
func parsePush[P any](payload []byte, build func(P) *PushEvent) (*PushEvent, error) {
var p P
unmarshalErr := json.Unmarshal(payload, &p) unmarshalErr := json.Unmarshal(payload, &p)
if unmarshalErr != nil { if unmarshalErr != nil {
return nil, unmarshalErr return nil, unmarshalErr
} }
return build(p), nil commitURL := extractGiteaCommitURL(p)
}
// basePushEvent builds a PushEvent populated with the fields shared by all
// webhook sources.
func basePushEvent(source Source, ref, before, after string) *PushEvent {
return &PushEvent{ return &PushEvent{
Source: source, Source: SourceGitea,
Ref: ref, Ref: p.Ref,
Before: before, Before: p.Before,
After: after, After: p.After,
Branch: extractBranch(ref), Branch: extractBranch(p.Ref),
RepoName: p.Repository.FullName,
CloneURL: p.Repository.CloneURL,
HTMLURL: p.Repository.HTMLURL,
CommitURL: commitURL,
Pusher: p.Pusher.Username,
}, nil
}
func parseGitHubPush(payload []byte) (*PushEvent, error) {
var p GitHubPushPayload
unmarshalErr := json.Unmarshal(payload, &p)
if unmarshalErr != nil {
return nil, unmarshalErr
} }
commitURL := extractGitHubCommitURL(p)
return &PushEvent{
Source: SourceGitHub,
Ref: p.Ref,
Before: p.Before,
After: p.After,
Branch: extractBranch(p.Ref),
RepoName: p.Repository.FullName,
CloneURL: p.Repository.CloneURL,
HTMLURL: p.Repository.HTMLURL,
CommitURL: commitURL,
Pusher: p.Pusher.Name,
}, nil
} }
// giteaPushEvent converts a Gitea push payload to a normalized PushEvent. func parseGitLabPush(payload []byte) (*PushEvent, error) {
func giteaPushEvent(p GiteaPushPayload) *PushEvent { var p GitLabPushPayload
event := basePushEvent(SourceGitea, p.Ref, p.Before, p.After)
event.RepoName = p.Repository.FullName
event.CloneURL = p.Repository.CloneURL
event.HTMLURL = p.Repository.HTMLURL
event.CommitURL = extractGiteaCommitURL(p)
event.Pusher = p.Pusher.Username
return event unmarshalErr := json.Unmarshal(payload, &p)
} if unmarshalErr != nil {
return nil, unmarshalErr
}
// gitlabPushEvent converts a GitLab push payload to a normalized PushEvent. commitURL := extractGitLabCommitURL(p)
func gitlabPushEvent(p GitLabPushPayload) *PushEvent {
event := basePushEvent(SourceGitLab, p.Ref, p.Before, p.After)
event.RepoName = p.Project.PathWithNamespace
event.CloneURL = p.Project.GitHTTPURL
event.HTMLURL = p.Project.WebURL
event.CommitURL = extractGitLabCommitURL(p)
event.Pusher = p.UserName
return event return &PushEvent{
} Source: SourceGitLab,
Ref: p.Ref,
// githubPushEvent converts a GitHub push payload to a normalized PushEvent. Before: p.Before,
func githubPushEvent(p GitHubPushPayload) *PushEvent { After: p.After,
event := basePushEvent(SourceGitHub, p.Ref, p.Before, p.After) Branch: extractBranch(p.Ref),
event.RepoName = p.Repository.FullName RepoName: p.Project.PathWithNamespace,
event.CloneURL = p.Repository.CloneURL CloneURL: p.Project.GitHTTPURL,
event.HTMLURL = p.Repository.HTMLURL HTMLURL: p.Project.WebURL,
event.CommitURL = extractGitHubCommitURL(p) CommitURL: commitURL,
event.Pusher = p.Pusher.Name Pusher: p.UserName,
}, nil
return event
} }
// extractBranch extracts the branch name from a git ref. // extractBranch extracts the branch name from a git ref.
+2 -15
View File
@@ -6,7 +6,6 @@ import (
"database/sql" "database/sql"
"fmt" "fmt"
"log/slog" "log/slog"
"sync"
"go.uber.org/fx" "go.uber.org/fx"
@@ -32,10 +31,6 @@ type Service struct {
db *database.Database db *database.Database
deploy *deploy.Service deploy *deploy.Service
params *ServiceParams 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. // New creates a new webhook Service.
@@ -113,14 +108,6 @@ func (svc *Service) HandleWebhook(
return nil 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( func (svc *Service) triggerDeployment(
ctx context.Context, ctx context.Context,
app *models.App, app *models.App,
@@ -130,7 +117,7 @@ func (svc *Service) triggerDeployment(
eventID := event.ID eventID := event.ID
appName := app.Name appName := app.Name
svc.deployments.Go(func() { go func() {
// Use context.WithoutCancel to ensure deployment completes // Use context.WithoutCancel to ensure deployment completes
// even if the HTTP request context is cancelled. // even if the HTTP request context is cancelled.
deployCtx := context.WithoutCancel(ctx) deployCtx := context.WithoutCancel(ctx)
@@ -143,5 +130,5 @@ func (svc *Service) triggerDeployment(
// Mark event as processed // Mark event as processed
event.Processed = true event.Processed = true
_ = event.Save(deployCtx) _ = event.Save(deployCtx)
}) }()
} }
+160 -257
View File
@@ -7,6 +7,7 @@ import (
"os" "os"
"path/filepath" "path/filepath"
"testing" "testing"
"time"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
@@ -23,18 +24,6 @@ import (
"sneak.berlin/go/upaas/internal/service/webhook" "sneak.berlin/go/upaas/internal/service/webhook"
) )
const (
giteaEventHeader = "X-Gitea-Event"
githubEventHeader = "X-GitHub-Event"
gitlabEventHeader = "X-Gitlab-Event"
gitlabPushHook = "Push Hook"
pushEventType = "push"
branchMain = "main"
refMain = "refs/heads/main"
testCommitSHA = "abc123def456789"
testPusher = "developer"
)
type testDeps struct { type testDeps struct {
logger *logger.Logger logger *logger.Logger
config *config.Config config *config.Config
@@ -56,14 +45,9 @@ func setupTestDeps(t *testing.T) *testDeps {
loggerInst, err := logger.New(fx.Lifecycle(nil), logger.Params{Globals: globalsInst}) loggerInst, err := logger.New(fx.Lifecycle(nil), logger.Params{Globals: globalsInst})
require.NoError(t, err) require.NoError(t, err)
cfg := &config.Config{ cfg := &config.Config{Port: 8080, DataDir: tmpDir, SessionSecret: "test-secret-key-at-least-32-chars"}
Port: 8080, DataDir: tmpDir,
SessionSecret: "test-secret-key-at-least-32-chars",
}
dbInst, err := database.New( dbInst, err := database.New(fx.Lifecycle(nil), database.Params{Logger: loggerInst, Config: cfg})
fx.Lifecycle(nil), database.Params{Logger: loggerInst, Config: cfg},
)
require.NoError(t, err) require.NoError(t, err)
return &testDeps{logger: loggerInst, config: cfg, db: dbInst, tmpDir: tmpDir} return &testDeps{logger: loggerInst, config: cfg, db: dbInst, tmpDir: tmpDir}
@@ -74,19 +58,14 @@ func setupTestService(t *testing.T) (*webhook.Service, *database.Database, func(
deps := setupTestDeps(t) deps := setupTestDeps(t)
dockerClient, err := docker.New( dockerClient, err := docker.New(fx.Lifecycle(nil), docker.Params{Logger: deps.logger, Config: deps.config})
fx.Lifecycle(nil), docker.Params{Logger: deps.logger, Config: deps.config},
)
require.NoError(t, err) require.NoError(t, err)
notifySvc, err := notify.New( notifySvc, err := notify.New(fx.Lifecycle(nil), notify.ServiceParams{Logger: deps.logger})
fx.Lifecycle(nil), notify.ServiceParams{Logger: deps.logger},
)
require.NoError(t, err) require.NoError(t, err)
deploySvc, err := deploy.New(fx.Lifecycle(nil), deploy.ServiceParams{ deploySvc, err := deploy.New(fx.Lifecycle(nil), deploy.ServiceParams{
Logger: deps.logger, Config: deps.config, Database: deps.db, Logger: deps.logger, Config: deps.config, Database: deps.db, Docker: dockerClient, Notify: notifySvc,
Docker: dockerClient, Notify: notifySvc,
}) })
require.NoError(t, err) require.NoError(t, err)
@@ -125,6 +104,8 @@ func createTestApp(
} }
// TestDetectWebhookSource tests auto-detection of webhook source from HTTP headers. // TestDetectWebhookSource tests auto-detection of webhook source from HTTP headers.
//
//nolint:funlen // table-driven test with comprehensive test cases
func TestDetectWebhookSource(testingT *testing.T) { func TestDetectWebhookSource(testingT *testing.T) {
testingT.Parallel() testingT.Parallel()
@@ -135,17 +116,17 @@ func TestDetectWebhookSource(testingT *testing.T) {
}{ }{
{ {
name: "detects Gitea from X-Gitea-Event header", name: "detects Gitea from X-Gitea-Event header",
headers: map[string]string{giteaEventHeader: pushEventType}, headers: map[string]string{"X-Gitea-Event": "push"},
expected: webhook.SourceGitea, expected: webhook.SourceGitea,
}, },
{ {
name: "detects GitHub from X-GitHub-Event header", name: "detects GitHub from X-GitHub-Event header",
headers: map[string]string{githubEventHeader: pushEventType}, headers: map[string]string{"X-GitHub-Event": "push"},
expected: webhook.SourceGitHub, expected: webhook.SourceGitHub,
}, },
{ {
name: "detects GitLab from X-Gitlab-Event header", name: "detects GitLab from X-Gitlab-Event header",
headers: map[string]string{gitlabEventHeader: gitlabPushHook}, headers: map[string]string{"X-Gitlab-Event": "Push Hook"},
expected: webhook.SourceGitLab, expected: webhook.SourceGitLab,
}, },
{ {
@@ -161,16 +142,16 @@ func TestDetectWebhookSource(testingT *testing.T) {
{ {
name: "Gitea takes precedence over GitHub", name: "Gitea takes precedence over GitHub",
headers: map[string]string{ headers: map[string]string{
giteaEventHeader: pushEventType, "X-Gitea-Event": "push",
githubEventHeader: pushEventType, "X-GitHub-Event": "push",
}, },
expected: webhook.SourceGitea, expected: webhook.SourceGitea,
}, },
{ {
name: "GitHub takes precedence over GitLab", name: "GitHub takes precedence over GitLab",
headers: map[string]string{ headers: map[string]string{
githubEventHeader: pushEventType, "X-GitHub-Event": "push",
gitlabEventHeader: gitlabPushHook, "X-Gitlab-Event": "Push Hook",
}, },
expected: webhook.SourceGitHub, expected: webhook.SourceGitHub,
}, },
@@ -203,33 +184,33 @@ func TestDetectEventType(testingT *testing.T) {
}{ }{
{ {
name: "extracts Gitea event type", name: "extracts Gitea event type",
headers: map[string]string{giteaEventHeader: pushEventType}, headers: map[string]string{"X-Gitea-Event": "push"},
source: webhook.SourceGitea, source: webhook.SourceGitea,
expected: pushEventType, expected: "push",
}, },
{ {
name: "extracts GitHub event type", name: "extracts GitHub event type",
headers: map[string]string{githubEventHeader: pushEventType}, headers: map[string]string{"X-GitHub-Event": "push"},
source: webhook.SourceGitHub, source: webhook.SourceGitHub,
expected: pushEventType, expected: "push",
}, },
{ {
name: "extracts GitLab event type", name: "extracts GitLab event type",
headers: map[string]string{gitlabEventHeader: gitlabPushHook}, headers: map[string]string{"X-Gitlab-Event": "Push Hook"},
source: webhook.SourceGitLab, source: webhook.SourceGitLab,
expected: gitlabPushHook, expected: "Push Hook",
}, },
{ {
name: "returns push for unknown source", name: "returns push for unknown source",
headers: map[string]string{}, headers: map[string]string{},
source: webhook.SourceUnknown, source: webhook.SourceUnknown,
expected: pushEventType, expected: "push",
}, },
{ {
name: "returns push when header missing for source", name: "returns push when header missing for source",
headers: map[string]string{}, headers: map[string]string{},
source: webhook.SourceGitea, source: webhook.SourceGitea,
expected: pushEventType, expected: "push",
}, },
} }
@@ -269,54 +250,11 @@ func TestUnparsedURLString(t *testing.T) {
assert.Empty(t, empty.String()) assert.Empty(t, empty.String())
} }
// pushEventExpectation describes the expected normalized fields of a parsed // TestParsePushPayloadGitea tests parsing of Gitea push payloads.
// push payload. func TestParsePushPayloadGitea(t *testing.T) {
type pushEventExpectation struct { t.Parallel()
source webhook.Source
ref string
branch string
after string
repoName string
cloneURL webhook.UnparsedURL
htmlURL webhook.UnparsedURL
commitURL webhook.UnparsedURL
pusher string
}
// assertPushEvent parses payload for want.source and asserts every payload := []byte(`{
// normalized PushEvent field matches want.
func assertPushEvent(t *testing.T, payload []byte, want pushEventExpectation) {
t.Helper()
event, err := webhook.ParsePushPayload(want.source, payload)
require.NoError(t, err)
assert.Equal(t, want.source, event.Source)
assert.Equal(t, want.ref, event.Ref)
assert.Equal(t, want.branch, event.Branch)
assert.Equal(t, want.after, event.After)
assertPushEventOrigin(t, event, want)
}
// assertPushEventOrigin asserts the repository and pusher fields of event.
func assertPushEventOrigin(
t *testing.T,
event *webhook.PushEvent,
want pushEventExpectation,
) {
t.Helper()
assert.Equal(t, want.repoName, event.RepoName)
assert.Equal(t, want.cloneURL, event.CloneURL)
assert.Equal(t, want.htmlURL, event.HTMLURL)
assert.Equal(t, want.commitURL, event.CommitURL)
assert.Equal(t, want.pusher, event.Pusher)
}
// giteaPushJSON returns a realistic Gitea push webhook payload.
func giteaPushJSON() []byte {
return []byte(`{
"ref": "refs/heads/main", "ref": "refs/heads/main",
"before": "0000000000000000000000000000000000000000", "before": "0000000000000000000000000000000000000000",
"after": "abc123def456789", "after": "abc123def456789",
@@ -337,11 +275,29 @@ func giteaPushJSON() []byte {
} }
] ]
}`) }`)
event, err := webhook.ParsePushPayload(webhook.SourceGitea, payload)
require.NoError(t, err)
assert.Equal(t, webhook.SourceGitea, event.Source)
assert.Equal(t, "refs/heads/main", event.Ref)
assert.Equal(t, "main", event.Branch)
assert.Equal(t, "abc123def456789", event.After)
assert.Equal(t, "myorg/myrepo", event.RepoName)
assert.Equal(t, webhook.UnparsedURL("https://gitea.example.com/myorg/myrepo.git"), event.CloneURL)
assert.Equal(t, webhook.UnparsedURL("https://gitea.example.com/myorg/myrepo"), event.HTMLURL)
assert.Equal(t,
webhook.UnparsedURL("https://gitea.example.com/myorg/myrepo/commit/abc123def456789"),
event.CommitURL,
)
assert.Equal(t, "developer", event.Pusher)
} }
// githubPushJSON returns a realistic GitHub push webhook payload. // TestParsePushPayloadGitHub tests parsing of GitHub push payloads.
func githubPushJSON() []byte { func TestParsePushPayloadGitHub(t *testing.T) {
return []byte(`{ t.Parallel()
payload := []byte(`{
"ref": "refs/heads/main", "ref": "refs/heads/main",
"before": "0000000000000000000000000000000000000000", "before": "0000000000000000000000000000000000000000",
"after": "abc123def456789", "after": "abc123def456789",
@@ -367,11 +323,29 @@ func githubPushJSON() []byte {
} }
] ]
}`) }`)
event, err := webhook.ParsePushPayload(webhook.SourceGitHub, payload)
require.NoError(t, err)
assert.Equal(t, webhook.SourceGitHub, event.Source)
assert.Equal(t, "refs/heads/main", event.Ref)
assert.Equal(t, "main", event.Branch)
assert.Equal(t, "abc123def456789", event.After)
assert.Equal(t, "myorg/myrepo", event.RepoName)
assert.Equal(t, webhook.UnparsedURL("https://github.com/myorg/myrepo.git"), event.CloneURL)
assert.Equal(t, webhook.UnparsedURL("https://github.com/myorg/myrepo"), event.HTMLURL)
assert.Equal(t,
webhook.UnparsedURL("https://github.com/myorg/myrepo/commit/abc123def456789"),
event.CommitURL,
)
assert.Equal(t, "developer", event.Pusher)
} }
// gitlabPushJSON returns a realistic GitLab push webhook payload. // TestParsePushPayloadGitLab tests parsing of GitLab push payloads.
func gitlabPushJSON() []byte { func TestParsePushPayloadGitLab(t *testing.T) {
return []byte(`{ t.Parallel()
payload := []byte(`{
"ref": "refs/heads/develop", "ref": "refs/heads/develop",
"before": "0000000000000000000000000000000000000000", "before": "0000000000000000000000000000000000000000",
"after": "abc123def456789", "after": "abc123def456789",
@@ -392,78 +366,25 @@ func gitlabPushJSON() []byte {
} }
] ]
}`) }`)
event, err := webhook.ParsePushPayload(webhook.SourceGitLab, payload)
require.NoError(t, err)
assert.Equal(t, webhook.SourceGitLab, event.Source)
assert.Equal(t, "refs/heads/develop", event.Ref)
assert.Equal(t, "develop", event.Branch)
assert.Equal(t, "abc123def456789", event.After)
assert.Equal(t, "mygroup/myproject", event.RepoName)
assert.Equal(t, webhook.UnparsedURL("https://gitlab.com/mygroup/myproject.git"), event.CloneURL)
assert.Equal(t, webhook.UnparsedURL("https://gitlab.com/mygroup/myproject"), event.HTMLURL)
assert.Equal(t,
webhook.UnparsedURL("https://gitlab.com/mygroup/myproject/-/commit/abc123def456789"),
event.CommitURL,
)
assert.Equal(t, "developer", event.Pusher)
} }
// pushPayloadJSON returns the push payload fixture for source. // TestParsePushPayloadUnknownFallsBackToGitea tests that unknown source uses Gitea parser.
func pushPayloadJSON(t *testing.T, source webhook.Source) []byte {
t.Helper()
switch source {
case webhook.SourceGitHub:
return githubPushJSON()
case webhook.SourceGitLab:
return gitlabPushJSON()
case webhook.SourceGitea, webhook.SourceUnknown:
return giteaPushJSON()
}
t.Fatalf("no push payload fixture for source %v", source)
return nil
}
// TestParsePushPayload tests parsing of Gitea, GitHub, and GitLab push
// payloads into normalized PushEvents.
func TestParsePushPayload(testingT *testing.T) {
testingT.Parallel()
tests := []pushEventExpectation{
{
source: webhook.SourceGitea,
ref: refMain,
branch: branchMain,
after: testCommitSHA,
repoName: "myorg/myrepo",
cloneURL: "https://gitea.example.com/myorg/myrepo.git",
htmlURL: "https://gitea.example.com/myorg/myrepo",
commitURL: "https://gitea.example.com/myorg/myrepo/commit/abc123def456789",
pusher: testPusher,
},
{
source: webhook.SourceGitHub,
ref: refMain,
branch: branchMain,
after: testCommitSHA,
repoName: "myorg/myrepo",
cloneURL: "https://github.com/myorg/myrepo.git",
htmlURL: "https://github.com/myorg/myrepo",
commitURL: "https://github.com/myorg/myrepo/commit/abc123def456789",
pusher: testPusher,
},
{
source: webhook.SourceGitLab,
ref: "refs/heads/develop",
branch: "develop",
after: testCommitSHA,
repoName: "mygroup/myproject",
cloneURL: "https://gitlab.com/mygroup/myproject.git",
htmlURL: "https://gitlab.com/mygroup/myproject",
commitURL: "https://gitlab.com/mygroup/myproject/-/commit/abc123def456789",
pusher: testPusher,
},
}
for _, testCase := range tests {
testingT.Run(testCase.source.String(), func(t *testing.T) {
t.Parallel()
assertPushEvent(t, pushPayloadJSON(t, testCase.source), testCase)
})
}
}
// TestParsePushPayloadUnknownFallsBackToGitea tests that unknown source
// uses the Gitea parser.
func TestParsePushPayloadUnknownFallsBackToGitea(t *testing.T) { func TestParsePushPayloadUnknownFallsBackToGitea(t *testing.T) {
t.Parallel() t.Parallel()
@@ -478,7 +399,7 @@ func TestParsePushPayloadUnknownFallsBackToGitea(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, webhook.SourceGitea, event.Source) assert.Equal(t, webhook.SourceGitea, event.Source)
assert.Equal(t, branchMain, event.Branch) assert.Equal(t, "main", event.Branch)
assert.Equal(t, "abc123", event.After) assert.Equal(t, "abc123", event.After)
} }
@@ -541,10 +462,7 @@ func TestGitHubCommitURLFallback(t *testing.T) {
event, err := webhook.ParsePushPayload(webhook.SourceGitHub, payload) event, err := webhook.ParsePushPayload(webhook.SourceGitHub, payload)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, assert.Equal(t, webhook.UnparsedURL("https://github.com/u/r/commit/abc123"), event.CommitURL)
webhook.UnparsedURL("https://github.com/u/r/commit/abc123"),
event.CommitURL,
)
}) })
t.Run("falls back to commits list", func(t *testing.T) { t.Run("falls back to commits list", func(t *testing.T) {
@@ -559,10 +477,7 @@ func TestGitHubCommitURLFallback(t *testing.T) {
event, err := webhook.ParsePushPayload(webhook.SourceGitHub, payload) event, err := webhook.ParsePushPayload(webhook.SourceGitHub, payload)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, assert.Equal(t, webhook.UnparsedURL("https://github.com/u/r/commit/abc123"), event.CommitURL)
webhook.UnparsedURL("https://github.com/u/r/commit/abc123"),
event.CommitURL,
)
}) })
t.Run("constructs URL from repo HTML URL", func(t *testing.T) { t.Run("constructs URL from repo HTML URL", func(t *testing.T) {
@@ -576,10 +491,7 @@ func TestGitHubCommitURLFallback(t *testing.T) {
event, err := webhook.ParsePushPayload(webhook.SourceGitHub, payload) event, err := webhook.ParsePushPayload(webhook.SourceGitHub, payload)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, assert.Equal(t, webhook.UnparsedURL("https://github.com/u/r/commit/abc123"), event.CommitURL)
webhook.UnparsedURL("https://github.com/u/r/commit/abc123"),
event.CommitURL,
)
}) })
} }
@@ -599,10 +511,7 @@ func TestGitLabCommitURLFallback(t *testing.T) {
event, err := webhook.ParsePushPayload(webhook.SourceGitLab, payload) event, err := webhook.ParsePushPayload(webhook.SourceGitLab, payload)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, assert.Equal(t, webhook.UnparsedURL("https://gitlab.com/g/p/-/commit/abc123"), event.CommitURL)
webhook.UnparsedURL("https://gitlab.com/g/p/-/commit/abc123"),
event.CommitURL,
)
}) })
t.Run("constructs URL from project web URL", func(t *testing.T) { t.Run("constructs URL from project web URL", func(t *testing.T) {
@@ -616,10 +525,7 @@ func TestGitLabCommitURLFallback(t *testing.T) {
event, err := webhook.ParsePushPayload(webhook.SourceGitLab, payload) event, err := webhook.ParsePushPayload(webhook.SourceGitLab, payload)
require.NoError(t, err) require.NoError(t, err)
assert.Equal(t, assert.Equal(t, webhook.UnparsedURL("https://gitlab.com/g/p/-/commit/abc123"), event.CommitURL)
webhook.UnparsedURL("https://gitlab.com/g/p/-/commit/abc123"),
event.CommitURL,
)
}) })
} }
@@ -682,8 +588,7 @@ func TestGiteaPushPayloadParsing(testingT *testing.T) {
}) })
} }
// TestGitHubPushPayloadParsing tests deserialization of the GitHub payload // TestGitHubPushPayloadParsing tests direct deserialization of the GitHub payload struct.
// struct.
func TestGitHubPushPayloadParsing(t *testing.T) { func TestGitHubPushPayloadParsing(t *testing.T) {
t.Parallel() t.Parallel()
@@ -728,8 +633,7 @@ func TestGitHubPushPayloadParsing(t *testing.T) {
assert.Len(t, p.Commits, 1) assert.Len(t, p.Commits, 1)
} }
// TestGitLabPushPayloadParsing tests deserialization of the GitLab payload // TestGitLabPushPayloadParsing tests direct deserialization of the GitLab payload struct.
// struct.
func TestGitLabPushPayloadParsing(t *testing.T) { func TestGitLabPushPayloadParsing(t *testing.T) {
t.Parallel() t.Parallel()
@@ -767,8 +671,9 @@ func TestGitLabPushPayloadParsing(t *testing.T) {
assert.Len(t, p.Commits, 1) assert.Len(t, p.Commits, 1)
} }
// TestExtractBranch tests branch extraction via HandleWebhook integration // TestExtractBranch tests branch extraction via HandleWebhook integration (extractBranch is unexported).
// (extractBranch is unexported). //
//nolint:funlen // table-driven test with comprehensive test cases
func TestExtractBranch(testingT *testing.T) { func TestExtractBranch(testingT *testing.T) {
testingT.Parallel() testingT.Parallel()
@@ -779,8 +684,8 @@ func TestExtractBranch(testingT *testing.T) {
}{ }{
{ {
name: "extracts main branch", name: "extracts main branch",
ref: refMain, ref: "refs/heads/main",
expected: branchMain, expected: "main",
}, },
{ {
name: "extracts feature branch", name: "extracts feature branch",
@@ -794,8 +699,8 @@ func TestExtractBranch(testingT *testing.T) {
}, },
{ {
name: "returns raw ref if no prefix", name: "returns raw ref if no prefix",
ref: branchMain, ref: "main",
expected: branchMain, expected: "main",
}, },
{ {
name: "handles empty ref", name: "handles empty ref",
@@ -823,13 +728,12 @@ func TestExtractBranch(testingT *testing.T) {
payload := []byte(`{"ref": "` + testCase.ref + `"}`) payload := []byte(`{"ref": "` + testCase.ref + `"}`)
err := svc.HandleWebhook( err := svc.HandleWebhook(
context.Background(), app, webhook.SourceGitea, pushEventType, payload, context.Background(), app, webhook.SourceGitea, "push", payload,
) )
require.NoError(t, err) require.NoError(t, err)
// Wait for the async deployment goroutine to finish so its // Allow async deployment goroutine to complete before test cleanup
// writes under the temp dir complete before test cleanup. time.Sleep(100 * time.Millisecond)
svc.WaitForDeployments()
events, err := app.GetWebhookEvents(context.Background(), 10) events, err := app.GetWebhookEvents(context.Background(), 10)
require.NoError(t, err) require.NoError(t, err)
@@ -846,7 +750,7 @@ func TestHandleWebhookMatchingBranch(t *testing.T) {
svc, dbInst, cleanup := setupTestService(t) svc, dbInst, cleanup := setupTestService(t)
defer cleanup() defer cleanup()
app := createTestApp(t, dbInst, branchMain) app := createTestApp(t, dbInst, "main")
payload := []byte(`{ payload := []byte(`{
"ref": "refs/heads/main", "ref": "refs/heads/main",
@@ -863,21 +767,20 @@ func TestHandleWebhookMatchingBranch(t *testing.T) {
}`) }`)
err := svc.HandleWebhook( err := svc.HandleWebhook(
context.Background(), app, webhook.SourceGitea, pushEventType, payload, context.Background(), app, webhook.SourceGitea, "push", payload,
) )
require.NoError(t, err) require.NoError(t, err)
// Wait for the async deployment goroutine to finish so its writes // Allow async deployment goroutine to complete before test cleanup
// under the temp dir complete before test cleanup. time.Sleep(100 * time.Millisecond)
svc.WaitForDeployments()
events, err := app.GetWebhookEvents(context.Background(), 10) events, err := app.GetWebhookEvents(context.Background(), 10)
require.NoError(t, err) require.NoError(t, err)
require.Len(t, events, 1) require.Len(t, events, 1)
event := events[0] event := events[0]
assert.Equal(t, pushEventType, event.EventType) assert.Equal(t, "push", event.EventType)
assert.Equal(t, branchMain, event.Branch) assert.Equal(t, "main", event.Branch)
assert.True(t, event.Matched) assert.True(t, event.Matched)
assert.Equal(t, "abc123def456", event.CommitSHA.String) assert.Equal(t, "abc123def456", event.CommitSHA.String)
} }
@@ -888,12 +791,12 @@ func TestHandleWebhookNonMatchingBranch(t *testing.T) {
svc, dbInst, cleanup := setupTestService(t) svc, dbInst, cleanup := setupTestService(t)
defer cleanup() defer cleanup()
app := createTestApp(t, dbInst, branchMain) app := createTestApp(t, dbInst, "main")
payload := []byte(`{"ref": "refs/heads/develop", "after": "def789ghi012"}`) payload := []byte(`{"ref": "refs/heads/develop", "after": "def789ghi012"}`)
err := svc.HandleWebhook( err := svc.HandleWebhook(
context.Background(), app, webhook.SourceGitea, pushEventType, payload, context.Background(), app, webhook.SourceGitea, "push", payload,
) )
require.NoError(t, err) require.NoError(t, err)
@@ -911,11 +814,10 @@ func TestHandleWebhookInvalidJSON(t *testing.T) {
svc, dbInst, cleanup := setupTestService(t) svc, dbInst, cleanup := setupTestService(t)
defer cleanup() defer cleanup()
app := createTestApp(t, dbInst, branchMain) app := createTestApp(t, dbInst, "main")
err := svc.HandleWebhook( err := svc.HandleWebhook(
context.Background(), app, webhook.SourceGitea, pushEventType, context.Background(), app, webhook.SourceGitea, "push", []byte(`{invalid json}`),
[]byte(`{invalid json}`),
) )
require.NoError(t, err) require.NoError(t, err)
@@ -930,10 +832,10 @@ func TestHandleWebhookEmptyPayload(t *testing.T) {
svc, dbInst, cleanup := setupTestService(t) svc, dbInst, cleanup := setupTestService(t)
defer cleanup() defer cleanup()
app := createTestApp(t, dbInst, branchMain) app := createTestApp(t, dbInst, "main")
err := svc.HandleWebhook( err := svc.HandleWebhook(
context.Background(), app, webhook.SourceGitea, pushEventType, []byte(`{}`), context.Background(), app, webhook.SourceGitea, "push", []byte(`{}`),
) )
require.NoError(t, err) require.NoError(t, err)
@@ -943,44 +845,14 @@ func TestHandleWebhookEmptyPayload(t *testing.T) {
assert.False(t, events[0].Matched) assert.False(t, events[0].Matched)
} }
// assertHandleWebhookDeploys runs HandleWebhook for payload against a fresh // TestHandleWebhookGitHubSource tests HandleWebhook with a GitHub push payload.
// app on branchMain and asserts the recorded event matched with the given func TestHandleWebhookGitHubSource(t *testing.T) {
// commit SHA and commit URL. t.Parallel()
func assertHandleWebhookDeploys(
t *testing.T,
source webhook.Source,
payload []byte,
wantSHA string,
wantCommitURL string,
) {
t.Helper()
svc, dbInst, cleanup := setupTestService(t) svc, dbInst, cleanup := setupTestService(t)
defer cleanup() defer cleanup()
app := createTestApp(t, dbInst, branchMain) app := createTestApp(t, dbInst, "main")
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()
events, err := app.GetWebhookEvents(context.Background(), 10)
require.NoError(t, err)
require.Len(t, events, 1)
event := events[0]
assert.Equal(t, branchMain, event.Branch)
assert.True(t, event.Matched)
assert.Equal(t, wantSHA, event.CommitSHA.String)
assert.Equal(t, wantCommitURL, event.CommitURL.String)
}
// TestHandleWebhookGitHubSource tests HandleWebhook with a GitHub push payload.
func TestHandleWebhookGitHubSource(t *testing.T) {
t.Parallel()
payload := []byte(`{ payload := []byte(`{
"ref": "refs/heads/main", "ref": "refs/heads/main",
@@ -998,16 +870,34 @@ func TestHandleWebhookGitHubSource(t *testing.T) {
} }
}`) }`)
assertHandleWebhookDeploys( err := svc.HandleWebhook(
t, webhook.SourceGitHub, payload, context.Background(), app, webhook.SourceGitHub, "push", payload,
"github123", "https://github.com/org/repo/commit/github123",
) )
require.NoError(t, err)
// 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)
require.Len(t, events, 1)
event := events[0]
assert.Equal(t, "main", event.Branch)
assert.True(t, event.Matched)
assert.Equal(t, "github123", event.CommitSHA.String)
assert.Equal(t, "https://github.com/org/repo/commit/github123", event.CommitURL.String)
} }
// TestHandleWebhookGitLabSource tests HandleWebhook with a GitLab push payload. // TestHandleWebhookGitLabSource tests HandleWebhook with a GitLab push payload.
func TestHandleWebhookGitLabSource(t *testing.T) { func TestHandleWebhookGitLabSource(t *testing.T) {
t.Parallel() t.Parallel()
svc, dbInst, cleanup := setupTestService(t)
defer cleanup()
app := createTestApp(t, dbInst, "main")
payload := []byte(`{ payload := []byte(`{
"ref": "refs/heads/main", "ref": "refs/heads/main",
"after": "gitlab456", "after": "gitlab456",
@@ -1027,10 +917,23 @@ func TestHandleWebhookGitLabSource(t *testing.T) {
] ]
}`) }`)
assertHandleWebhookDeploys( err := svc.HandleWebhook(
t, webhook.SourceGitLab, payload, context.Background(), app, webhook.SourceGitLab, "push", payload,
"gitlab456", "https://gitlab.com/group/project/-/commit/gitlab456",
) )
require.NoError(t, err)
// 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)
require.Len(t, events, 1)
event := events[0]
assert.Equal(t, "main", event.Branch)
assert.True(t, event.Matched)
assert.Equal(t, "gitlab456", event.CommitSHA.String)
assert.Equal(t, "https://gitlab.com/group/project/-/commit/gitlab456", event.CommitURL.String)
} }
// TestSetupTestService verifies the test helper creates a working test service. // TestSetupTestService verifies the test helper creates a working test service.
@@ -1059,10 +962,10 @@ func TestPushEventConstruction(t *testing.T) {
event := webhook.PushEvent{ event := webhook.PushEvent{
Source: webhook.SourceGitHub, Source: webhook.SourceGitHub,
Ref: refMain, Ref: "refs/heads/main",
Before: "000", Before: "000",
After: "abc", After: "abc",
Branch: branchMain, Branch: "main",
RepoName: "org/repo", RepoName: "org/repo",
CloneURL: webhook.UnparsedURL("https://github.com/org/repo.git"), CloneURL: webhook.UnparsedURL("https://github.com/org/repo.git"),
HTMLURL: webhook.UnparsedURL("https://github.com/org/repo"), HTMLURL: webhook.UnparsedURL("https://github.com/org/repo"),
@@ -1070,7 +973,7 @@ func TestPushEventConstruction(t *testing.T) {
Pusher: "user", Pusher: "user",
} }
assert.Equal(t, branchMain, event.Branch) assert.Equal(t, "main", event.Branch)
assert.Equal(t, webhook.SourceGitHub, event.Source) assert.Equal(t, webhook.SourceGitHub, event.Source)
assert.Equal(t, "abc", event.After) assert.Equal(t, "abc", event.After)
} }
+52 -27
View File
@@ -3,18 +3,18 @@
# this repo. Idempotent: every install is guarded by a check so already # this repo. Idempotent: every install is guarded by a check so already
# installed tools are skipped. Base tooling comes from nix, apt, brew, # installed tools are skipped. Base tooling comes from nix, apt, brew,
# 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). goimports is installed with `go install` at a pinned # make, or go). golangci-lint is packaged in nix, brew, and apk; on apt
# version (integrity via the Go module checksum database) into # it is installed from a hash-verified GitHub release archive (never
# /usr/local/bin so it is on PATH. The linter is not installed here: it # curl | sh).
# runs only in Docker via script/lint, so docker is its sole prerequisite.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
# Pinned versions. Never "latest"; exact versions only. # Pinned versions, 2026-07-07. Never "latest"; exact versions only.
# golang.org/x/tools goimports, 2026-08-13. v0.49.0 requires Go 1.25 (matches GOLANGCI_LINT_VERSION="2.10.1"
# go.mod); v0.50.0 needs Go 1.26. Integrity via the Go module checksum database. # sha256 of golangci-lint-2.10.1-linux-<arch>.tar.gz release archives
GOIMPORTS_VERSION="v0.49.0" GOLANGCI_LINT_SHA256_AMD64="dfa775874cf0561b404a02a8f4481fc69b28091da95aa697259820d429b09c99"
GOLANGCI_LINT_SHA256_ARM64="6652b42ae02915eb2f9cb2a2e0cac99514c8eded8388d88ae3e06e1a52c00de8"
PKGMGR="" PKGMGR=""
SUDO="" SUDO=""
@@ -56,19 +56,52 @@ missing() {
! command -v "$1" >/dev/null 2>&1 ! command -v "$1" >/dev/null 2>&1
} }
# goimports is not packaged uniformly across nix/apt/brew/apk, so install it # verify_sha256 <file> <expected-hash>
# with `go install` at a pinned version and place the binary in /usr/local/bin verify_sha256() {
# so it is on PATH regardless of shell config. Requires go, which main if command -v sha256sum >/dev/null 2>&1; then
# installs first. actual="$(sha256sum "$1" | cut -d' ' -f1)"
ensure_goimports() { else
if ! missing goimports; then return 0; fi actual="$(shasum -a 256 "$1" | cut -d' ' -f1)"
detect_pkgmgr 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)" tmp="$(mktemp -d)"
GOBIN="$tmp" go install "golang.org/x/tools/cmd/goimports@${GOIMPORTS_VERSION}" curl -fsSL -o "$tmp/$name.tar.gz" \
$SUDO install -m 0755 "$tmp/goimports" /usr/local/bin/goimports "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" 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() { main() {
cd "$ROOT" cd "$ROOT"
@@ -76,17 +109,9 @@ main() {
if missing git; then pkg_install git git git git; fi if missing git; then pkg_install git git git git; fi
if missing make; then pkg_install gnumake make make make; 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 if missing go; then pkg_install go golang go go; fi
ensure_goimports ensure_golangci_lint
# 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
go mod download go mod download
+1 -1
View File
@@ -8,7 +8,7 @@ main() {
cd "$ROOT" cd "$ROOT"
gofmt -s -w . gofmt -s -w .
goimports -w . goimports -w .
npx prettier --write static/js/*.js npx prettier --write --tab-width 4 static/js/*.js
} }
main "$@" main "$@"
+2 -14
View File
@@ -1,24 +1,12 @@
#!/bin/sh #!/bin/sh
# script/lint: run golangci-lint. The linter is never installed on the # script/lint: run the linter.
# 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.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
docker build \ golangci-lint run --config .golangci.yml ./...
--build-arg GATE_RUN="$(date +%s)-$$" \
--output=type=cacheonly \
-f Dockerfile.lint \
.
} }
main "$@" main "$@"
+1 -1
View File
@@ -59,7 +59,7 @@ document.addEventListener("alpine:init", () => {
}, },
submitAll() { submitAll() {
const csrfInput = this.$root.querySelector( const csrfInput = this.$el.querySelector(
'input[name="gorilla.csrf.Token"]', 'input[name="gorilla.csrf.Token"]',
); );
const csrfToken = csrfInput ? csrfInput.value : ""; const csrfToken = csrfInput ? csrfInput.value : "";
+1 -1
View File
@@ -322,7 +322,7 @@
</td> </td>
<td class="text-right"> <td class="text-right">
<form method="POST" action="/apps/{{$.App.ID}}/ports/{{.ID}}/delete" class="inline" x-data="confirmAction('Delete this port mapping?')" @submit="confirm($event)"> <form method="POST" action="/apps/{{$.App.ID}}/ports/{{.ID}}/delete" class="inline" x-data="confirmAction('Delete this port mapping?')" @submit="confirm($event)">
{{ $.CSRFField }} {{ .CSRFField }}
<button type="submit" class="text-error-500 hover:text-error-700 text-sm">Delete</button> <button type="submit" class="text-error-500 hover:text-error-700 text-sm">Delete</button>
</form> </form>
</td> </td>