Compare commits
1 Commits
main
...
fix/deploy
| Author | SHA1 | Date | |
|---|---|---|---|
| cdcf527b25 |
@@ -191,6 +191,7 @@ 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 | "" |
|
||||||
@@ -204,9 +205,14 @@ 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
|
||||||
@@ -221,6 +227,8 @@ 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
|
||||||
|
|||||||
4
TODO.md
4
TODO.md
@@ -20,6 +20,10 @@ main cannot regress.
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 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
|
- 2026-08-07: Updated golangci-lint to v2.12.2 (canonical
|
||||||
`.golangci.yml`, `Dockerfile` lint stage pin, `script/bootstrap`
|
`.golangci.yml`, `Dockerfile` lint stage pin, `script/bootstrap`
|
||||||
release-archive pins) and fixed all resulting lint findings (noctx,
|
release-archive pins) and fixed all resulting lint findings (noctx,
|
||||||
|
|||||||
@@ -49,6 +49,7 @@ type Config struct {
|
|||||||
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:"-"`
|
||||||
@@ -100,6 +101,7 @@ 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", "")
|
||||||
@@ -135,6 +137,7 @@ 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"),
|
||||||
|
|||||||
@@ -667,10 +667,51 @@ func (c *Client) performClone(
|
|||||||
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.
|
||||||
|
|||||||
67
internal/middleware/csrf_test.go
Normal file
67
internal/middleware/csrf_test.go
Normal file
@@ -0,0 +1,67 @@
|
|||||||
|
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")
|
||||||
|
}
|
||||||
@@ -255,12 +255,34 @@ 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 {
|
||||||
return csrf.Protect(
|
protect := csrf.Protect(
|
||||||
[]byte(m.params.Config.SessionSecret),
|
[]byte(m.params.Config.SessionSecret),
|
||||||
csrf.Secure(false), // Allow HTTP for development; reverse proxy handles TLS
|
csrf.Secure(false), // cookie Secure flag; TLS is terminated upstream
|
||||||
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.
|
||||||
|
|||||||
@@ -59,7 +59,7 @@ document.addEventListener("alpine:init", () => {
|
|||||||
},
|
},
|
||||||
|
|
||||||
submitAll() {
|
submitAll() {
|
||||||
const csrfInput = this.$el.querySelector(
|
const csrfInput = this.$root.querySelector(
|
||||||
'input[name="gorilla.csrf.Token"]',
|
'input[name="gorilla.csrf.Token"]',
|
||||||
);
|
);
|
||||||
const csrfToken = csrfInput ? csrfInput.value : "";
|
const csrfToken = csrfInput ? csrfInput.value : "";
|
||||||
|
|||||||
@@ -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>
|
||||||
|
|||||||
Reference in New Issue
Block a user