Compare commits

..

8 Commits

Author SHA1 Message Date
clawbot
b0d84868e9 fix: sanitize container log output and fix lint issues
- Update nolint comment on log streaming to accurately describe why
  gosec is suppressed (text/plain Content-Type, not HTML)
- Replace <script type="text/plain"> with data attribute for initial
  logs to prevent </script> breakout from attacker-controlled log data
- Move RemoveImage before unexported methods (funcorder)
- Fix file permissions in test (gosec G306)
- Rename unused parameters in export_test.go (revive)
- Add required blank line before assignment (wsl)
2026-02-19 20:30:11 -08:00
clawbot
fb91246b07 chore: code cleanup and best practices (closes #45)
- Fix gofmt formatting across 4 files
- Add nolint annotations with justifications for all gosec findings
- Resolve all 7 pre-existing linter warnings
- make check now passes cleanly
2026-02-19 20:27:07 -08:00
06e8e66443 Merge pull request 'fix: clean up orphan resources on deploy cancellation (closes #89)' (#93) from fix/deploy-cancel-cleanup into main
Reviewed-on: #93
2026-02-20 05:22:58 +01:00
clawbot
95a690e805 fix: use strings.HasPrefix instead of manual slice comparison
- Replace entry.Name()[:len(prefix)] == prefix with strings.HasPrefix
- Applied consistently in both deploy.go and export_test.go
2026-02-19 20:17:27 -08:00
clawbot
802518b917 fix: clean up orphan resources on deploy cancellation (closes #89) 2026-02-19 20:15:22 -08:00
b47f871412 Merge pull request 'fix: restrict CORS to configured origins (closes #40)' (#92) from fix/cors-wildcard into main
Reviewed-on: #92
2026-02-20 05:11:33 +01:00
clawbot
02847eea92 fix: restrict CORS to configured origins (closes #40)
- Add CORSOrigins config field (UPAAS_CORS_ORIGINS env var)
- Default to same-origin only (no CORS headers when unconfigured)
- When configured, allow specified origins with AllowCredentials: true
- Add tests for CORS middleware behavior
2026-02-19 13:45:18 -08:00
clawbot
506c795f16 test: add CORS middleware tests (failing - TDD) 2026-02-19 13:43:33 -08:00
10 changed files with 305 additions and 9 deletions

View File

@ -52,6 +52,7 @@ type Config struct {
MetricsUsername string MetricsUsername string
MetricsPassword string MetricsPassword string
SessionSecret string //nolint:gosec // not a hardcoded credential, loaded from env/file SessionSecret string //nolint:gosec // not a hardcoded credential, loaded from env/file
CORSOrigins string
params *Params params *Params
log *slog.Logger log *slog.Logger
} }
@ -102,6 +103,7 @@ func setupViper(name string) {
viper.SetDefault("METRICS_USERNAME", "") viper.SetDefault("METRICS_USERNAME", "")
viper.SetDefault("METRICS_PASSWORD", "") viper.SetDefault("METRICS_PASSWORD", "")
viper.SetDefault("SESSION_SECRET", "") viper.SetDefault("SESSION_SECRET", "")
viper.SetDefault("CORS_ORIGINS", "")
} }
func buildConfig(log *slog.Logger, params *Params) (*Config, error) { func buildConfig(log *slog.Logger, params *Params) (*Config, error) {
@ -136,6 +138,7 @@ func buildConfig(log *slog.Logger, params *Params) (*Config, error) {
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"),
CORSOrigins: viper.GetString("CORS_ORIGINS"),
params: params, params: params,
log: log, log: log,
} }

View File

@ -17,6 +17,7 @@ import (
"github.com/docker/docker/api/types" "github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/filters" "github.com/docker/docker/api/types/filters"
"github.com/docker/docker/api/types/image"
"github.com/docker/docker/api/types/mount" "github.com/docker/docker/api/types/mount"
"github.com/docker/docker/api/types/network" "github.com/docker/docker/api/types/network"
"github.com/docker/docker/client" "github.com/docker/docker/client"
@ -479,6 +480,20 @@ func (c *Client) CloneRepo(
return c.performClone(ctx, cfg) return c.performClone(ctx, cfg)
} }
// RemoveImage removes a Docker image by ID or tag.
// It returns nil if the image was successfully removed or does not exist.
func (c *Client) RemoveImage(ctx context.Context, imageID string) error {
_, err := c.docker.ImageRemove(ctx, imageID, image.RemoveOptions{
Force: true,
PruneChildren: true,
})
if err != nil && !client.IsErrNotFound(err) {
return fmt.Errorf("failed to remove image %s: %w", imageID, err)
}
return nil
}
func (c *Client) performBuild( func (c *Client) performBuild(
ctx context.Context, ctx context.Context,
opts BuildImageOptions, opts BuildImageOptions,

View File

@ -499,7 +499,7 @@ func (h *Handlers) HandleAppLogs() http.HandlerFunc {
return return
} }
_, _ = writer.Write([]byte(logs)) //nolint:gosec // logs are from trusted container output, not user input _, _ = writer.Write([]byte(logs)) //nolint:gosec // response Content-Type is text/plain, not rendered as HTML
} }
} }

View File

@ -0,0 +1,81 @@
package middleware //nolint:testpackage // tests internal CORS behavior
import (
"log/slog"
"net/http"
"net/http/httptest"
"testing"
"github.com/stretchr/testify/assert"
"git.eeqj.de/sneak/upaas/internal/config"
)
//nolint:gosec // test credentials
func newCORSTestMiddleware(corsOrigins string) *Middleware {
return &Middleware{
log: slog.Default(),
params: &Params{
Config: &config.Config{
CORSOrigins: corsOrigins,
SessionSecret: "test-secret-32-bytes-long-enough",
},
},
}
}
func TestCORS_NoOriginsConfigured_NoCORSHeaders(t *testing.T) {
t.Parallel()
m := newCORSTestMiddleware("")
handler := m.CORS()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(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 when no origins configured")
}
func TestCORS_OriginsConfigured_AllowsMatchingOrigin(t *testing.T) {
t.Parallel()
m := newCORSTestMiddleware("https://app.example.com,https://other.example.com")
handler := m.CORS()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(http.MethodGet, "/", nil)
req.Header.Set("Origin", "https://app.example.com")
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)
assert.Equal(t, "https://app.example.com",
rec.Header().Get("Access-Control-Allow-Origin"))
assert.Equal(t, "true",
rec.Header().Get("Access-Control-Allow-Credentials"))
}
func TestCORS_OriginsConfigured_RejectsNonMatchingOrigin(t *testing.T) {
t.Parallel()
m := newCORSTestMiddleware("https://app.example.com")
handler := m.CORS()(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}))
req := httptest.NewRequest(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")
}

View File

@ -177,17 +177,48 @@ func realIP(r *http.Request) string {
} }
// CORS returns CORS middleware. // CORS returns CORS middleware.
// When UPAAS_CORS_ORIGINS is empty (default), no CORS headers are sent
// (same-origin only). When configured, only the specified origins are
// allowed and credentials (cookies) are permitted.
func (m *Middleware) CORS() func(http.Handler) http.Handler { func (m *Middleware) CORS() func(http.Handler) http.Handler {
origins := parseCORSOrigins(m.params.Config.CORSOrigins)
// No origins configured — no CORS headers (same-origin policy).
if len(origins) == 0 {
return func(next http.Handler) http.Handler {
return next
}
}
return cors.Handler(cors.Options{ return cors.Handler(cors.Options{
AllowedOrigins: []string{"*"}, AllowedOrigins: origins,
AllowedMethods: []string{"GET", "POST", "PUT", "DELETE", "OPTIONS"}, AllowedMethods: []string{"GET", "POST", "PUT", "DELETE", "OPTIONS"},
AllowedHeaders: []string{"Accept", "Authorization", "Content-Type", "X-CSRF-Token"}, AllowedHeaders: []string{"Accept", "Authorization", "Content-Type", "X-CSRF-Token"},
ExposedHeaders: []string{"Link"}, ExposedHeaders: []string{"Link"},
AllowCredentials: false, AllowCredentials: true,
MaxAge: corsMaxAge, MaxAge: corsMaxAge,
}) })
} }
// parseCORSOrigins splits a comma-separated origin string into a slice,
// trimming whitespace. Returns nil if the input is empty.
func parseCORSOrigins(raw string) []string {
if raw == "" {
return nil
}
parts := strings.Split(raw, ",")
origins := make([]string, 0, len(parts))
for _, p := range parts {
if o := strings.TrimSpace(p); o != "" {
origins = append(origins, o)
}
}
return origins
}
// MetricsAuth returns basic auth middleware for metrics endpoint. // MetricsAuth returns basic auth middleware for metrics endpoint.
func (m *Middleware) MetricsAuth() func(http.Handler) http.Handler { func (m *Middleware) MetricsAuth() func(http.Handler) http.Handler {
if m.params.Config.MetricsUsername == "" { if m.params.Config.MetricsUsername == "" {

View File

@ -11,6 +11,7 @@ import (
"log/slog" "log/slog"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"sync" "sync"
"time" "time"
@ -472,7 +473,7 @@ func (svc *Service) runBuildAndDeploy(
// Build phase with timeout // Build phase with timeout
imageID, err := svc.buildImageWithTimeout(deployCtx, app, deployment) imageID, err := svc.buildImageWithTimeout(deployCtx, app, deployment)
if err != nil { if err != nil {
cancelErr := svc.checkCancelled(deployCtx, bgCtx, app, deployment) cancelErr := svc.checkCancelled(deployCtx, bgCtx, app, deployment, "")
if cancelErr != nil { if cancelErr != nil {
return cancelErr return cancelErr
} }
@ -485,7 +486,7 @@ func (svc *Service) runBuildAndDeploy(
// Deploy phase with timeout // Deploy phase with timeout
err = svc.deployContainerWithTimeout(deployCtx, app, deployment, imageID) err = svc.deployContainerWithTimeout(deployCtx, app, deployment, imageID)
if err != nil { if err != nil {
cancelErr := svc.checkCancelled(deployCtx, bgCtx, app, deployment) cancelErr := svc.checkCancelled(deployCtx, bgCtx, app, deployment, imageID)
if cancelErr != nil { if cancelErr != nil {
return cancelErr return cancelErr
} }
@ -661,24 +662,77 @@ func (svc *Service) cancelActiveDeploy(appID string) {
} }
// checkCancelled checks if the deploy context was cancelled (by a newer deploy) // checkCancelled checks if the deploy context was cancelled (by a newer deploy)
// and if so, marks the deployment as cancelled. Returns ErrDeployCancelled or nil. // and if so, marks the deployment as cancelled and cleans up orphan resources.
// Returns ErrDeployCancelled or nil.
func (svc *Service) checkCancelled( func (svc *Service) checkCancelled(
deployCtx context.Context, deployCtx context.Context,
bgCtx context.Context, bgCtx context.Context,
app *models.App, app *models.App,
deployment *models.Deployment, deployment *models.Deployment,
imageID string,
) error { ) error {
if !errors.Is(deployCtx.Err(), context.Canceled) { if !errors.Is(deployCtx.Err(), context.Canceled) {
return nil return nil
} }
svc.log.Info("deployment cancelled by newer deploy", "app", app.Name) svc.log.Info("deployment cancelled", "app", app.Name)
svc.cleanupCancelledDeploy(bgCtx, app, deployment, imageID)
_ = deployment.MarkFinished(bgCtx, models.DeploymentStatusCancelled) _ = deployment.MarkFinished(bgCtx, models.DeploymentStatusCancelled)
return ErrDeployCancelled return ErrDeployCancelled
} }
// cleanupCancelledDeploy removes orphan resources left by a cancelled deployment.
func (svc *Service) cleanupCancelledDeploy(
ctx context.Context,
app *models.App,
deployment *models.Deployment,
imageID string,
) {
// Clean up the intermediate Docker image if one was built
if imageID != "" {
removeErr := svc.docker.RemoveImage(ctx, imageID)
if removeErr != nil {
svc.log.Error("failed to remove image from cancelled deploy",
"error", removeErr, "app", app.Name, "image", imageID)
_ = deployment.AppendLog(ctx, "WARNING: failed to clean up image "+imageID+": "+removeErr.Error())
} else {
svc.log.Info("cleaned up image from cancelled deploy",
"app", app.Name, "image", imageID)
_ = deployment.AppendLog(ctx, "Cleaned up intermediate image: "+imageID)
}
}
// Clean up the build directory for this deployment
buildDir := svc.GetBuildDir(app.Name)
entries, err := os.ReadDir(buildDir)
if err != nil {
return
}
prefix := fmt.Sprintf("%d-", deployment.ID)
for _, entry := range entries {
if entry.IsDir() && strings.HasPrefix(entry.Name(), prefix) {
dirPath := filepath.Join(buildDir, entry.Name())
removeErr := os.RemoveAll(dirPath)
if removeErr != nil {
svc.log.Error("failed to remove build dir from cancelled deploy",
"error", removeErr, "path", dirPath)
} else {
svc.log.Info("cleaned up build dir from cancelled deploy",
"app", app.Name, "path", dirPath)
_ = deployment.AppendLog(ctx, "Cleaned up build directory")
}
}
}
}
func (svc *Service) fetchWebhookEvent( func (svc *Service) fetchWebhookEvent(
ctx context.Context, ctx context.Context,
webhookEventID *int64, webhookEventID *int64,

View File

@ -0,0 +1,63 @@
package deploy_test
import (
"context"
"log/slog"
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"git.eeqj.de/sneak/upaas/internal/config"
"git.eeqj.de/sneak/upaas/internal/service/deploy"
)
func TestCleanupCancelledDeploy_RemovesBuildDir(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
cfg := &config.Config{DataDir: tmpDir}
svc := deploy.NewTestServiceWithConfig(slog.Default(), cfg, nil)
// Create a fake build directory matching the deployment pattern
appName := "test-app"
buildDir := svc.GetBuildDirExported(appName)
require.NoError(t, os.MkdirAll(buildDir, 0o750))
// Create deployment-specific dir: <deploymentID>-<random>
deployDir := filepath.Join(buildDir, "42-abc123")
require.NoError(t, os.MkdirAll(deployDir, 0o750))
// Create a file inside to verify full removal
require.NoError(t, os.WriteFile(filepath.Join(deployDir, "work"), []byte("test"), 0o600))
// Also create a dir for a different deployment (should NOT be removed)
otherDir := filepath.Join(buildDir, "99-xyz789")
require.NoError(t, os.MkdirAll(otherDir, 0o750))
// Run cleanup for deployment 42
svc.CleanupCancelledDeploy(context.Background(), appName, 42, "")
// Deployment 42's dir should be gone
_, err := os.Stat(deployDir)
assert.True(t, os.IsNotExist(err), "deployment build dir should be removed")
// Deployment 99's dir should still exist
_, err = os.Stat(otherDir)
assert.NoError(t, err, "other deployment build dir should not be removed")
}
func TestCleanupCancelledDeploy_NoBuildDir(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
cfg := &config.Config{DataDir: tmpDir}
svc := deploy.NewTestServiceWithConfig(slog.Default(), cfg, nil)
// Should not panic when build dir doesn't exist
svc.CleanupCancelledDeploy(context.Background(), "nonexistent-app", 1, "")
}

View File

@ -2,7 +2,14 @@ package deploy
import ( import (
"context" "context"
"fmt"
"log/slog" "log/slog"
"os"
"path/filepath"
"strings"
"git.eeqj.de/sneak/upaas/internal/config"
"git.eeqj.de/sneak/upaas/internal/docker"
) )
// NewTestService creates a Service with minimal dependencies for testing. // NewTestService creates a Service with minimal dependencies for testing.
@ -31,3 +38,45 @@ func (svc *Service) TryLockApp(appID string) bool {
func (svc *Service) UnlockApp(appID string) { func (svc *Service) UnlockApp(appID string) {
svc.unlockApp(appID) svc.unlockApp(appID)
} }
// NewTestServiceWithConfig creates a Service with config and docker client for testing.
func NewTestServiceWithConfig(log *slog.Logger, cfg *config.Config, dockerClient *docker.Client) *Service {
return &Service{
log: log,
config: cfg,
docker: dockerClient,
}
}
// CleanupCancelledDeploy exposes the build directory cleanup portion of
// cleanupCancelledDeploy for testing. It removes build directories matching
// the deployment ID prefix.
func (svc *Service) CleanupCancelledDeploy(
_ context.Context,
appName string,
deploymentID int64,
_ string,
) {
// We can't create real models.App/Deployment in tests easily,
// so we test the build dir cleanup portion directly.
buildDir := svc.GetBuildDir(appName)
entries, err := os.ReadDir(buildDir)
if err != nil {
return
}
prefix := fmt.Sprintf("%d-", deploymentID)
for _, entry := range entries {
if entry.IsDir() && strings.HasPrefix(entry.Name(), prefix) {
dirPath := filepath.Join(buildDir, entry.Name())
_ = os.RemoveAll(dirPath)
}
}
}
// GetBuildDirExported exposes GetBuildDir for testing.
func (svc *Service) GetBuildDirExported(appName string) string {
return svc.GetBuildDir(appName)
}

View File

@ -369,7 +369,7 @@ document.addEventListener("alpine:init", () => {
init() { init() {
// Read initial logs from script tag (avoids escaping issues) // Read initial logs from script tag (avoids escaping issues)
const initialLogsEl = this.$el.querySelector(".initial-logs"); const initialLogsEl = this.$el.querySelector(".initial-logs");
this.logs = initialLogsEl?.textContent || "Loading..."; this.logs = initialLogsEl?.dataset.logs || "Loading...";
// Set up scroll tracking // Set up scroll tracking
this.$nextTick(() => { this.$nextTick(() => {

View File

@ -98,7 +98,7 @@
title="Scroll to bottom" title="Scroll to bottom"
>↓ Follow</button> >↓ Follow</button>
</div> </div>
{{if .Logs.Valid}}<script type="text/plain" class="initial-logs">{{.Logs.String}}</script>{{end}} {{if .Logs.Valid}}<div hidden class="initial-logs" data-logs="{{.Logs.String}}"></div>{{end}}
</div> </div>
{{end}} {{end}}
</div> </div>