Compare commits

...

5 Commits

Author SHA1 Message Date
clawbot
02f0a12626 fix: restrict SCP-like URLs to git user only and reject path traversal
- Changed SCP regex to only accept 'git' as the username
- Added path traversal check: reject URLs containing '..'
- Added test cases for non-git users and path traversal
2026-02-19 20:17:25 -08:00
clawbot
9f2d62da05 fix: validate repo URL format on app creation (closes #88) 2026-02-19 20:15:19 -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 327 additions and 61 deletions

View File

@@ -52,6 +52,7 @@ type Config struct {
MetricsUsername string MetricsUsername string
MetricsPassword string MetricsPassword string
SessionSecret string SessionSecret string
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

@@ -218,6 +218,15 @@ func (h *Handlers) HandleAPICreateApp() http.HandlerFunc {
return return
} }
repoURLErr := validateRepoURL(req.RepoURL)
if repoURLErr != nil {
h.respondJSON(writer, request,
map[string]string{"error": "invalid repository URL: " + repoURLErr.Error()},
http.StatusBadRequest)
return
}
createdApp, createErr := h.appService.CreateApp(request.Context(), app.CreateAppInput{ createdApp, createErr := h.appService.CreateApp(request.Context(), app.CreateAppInput{
Name: req.Name, Name: req.Name,
RepoURL: req.RepoURL, RepoURL: req.RepoURL,

View File

@@ -77,6 +77,14 @@ func (h *Handlers) HandleAppCreate() http.HandlerFunc { //nolint:funlen // valid
return return
} }
repoURLErr := validateRepoURL(repoURL)
if repoURLErr != nil {
data["Error"] = "Invalid repository URL: " + repoURLErr.Error()
h.renderTemplate(writer, tmpl, "app_new.html", data)
return
}
if branch == "" { if branch == "" {
branch = "main" branch = "main"
} }
@@ -225,6 +233,17 @@ func (h *Handlers) HandleAppUpdate() http.HandlerFunc { //nolint:funlen // valid
return return
} }
repoURLErr := validateRepoURL(request.FormValue("repo_url"))
if repoURLErr != nil {
data := h.addGlobals(map[string]any{
"App": application,
"Error": "Invalid repository URL: " + repoURLErr.Error(),
}, request)
_ = tmpl.ExecuteTemplate(writer, "app_edit.html", data)
return
}
application.Name = newName application.Name = newName
application.RepoURL = request.FormValue("repo_url") application.RepoURL = request.FormValue("repo_url")
application.Branch = request.FormValue("branch") application.Branch = request.FormValue("branch")

View File

@@ -0,0 +1,67 @@
package handlers
import (
"errors"
"net/url"
"regexp"
"strings"
)
// Repo URL validation errors.
var (
errRepoURLEmpty = errors.New("repository URL must not be empty")
errRepoURLScheme = errors.New("file:// URLs are not allowed for security reasons")
errRepoURLInvalid = errors.New("repository URL must use https://, http://, ssh://, git://, or git@host:path format")
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 (e.g. git@github.com:user/repo.git).
// Only the "git" user is allowed, as that is the standard for SSH deploy keys.
var scpLikeRepoRe = regexp.MustCompile(`^git@[a-zA-Z0-9._-]+:.+$`)
// validateRepoURL checks that the given repository URL is valid and uses an allowed scheme.
func validateRepoURL(repoURL string) error {
if strings.TrimSpace(repoURL) == "" {
return errRepoURLEmpty
}
// Reject path traversal in any URL format
if strings.Contains(repoURL, "..") {
return errRepoURLInvalid
}
// Check for SCP-like git URLs first (git@host:path)
if scpLikeRepoRe.MatchString(repoURL) {
return nil
}
// Reject file:// explicitly
if strings.HasPrefix(strings.ToLower(repoURL), "file://") {
return errRepoURLScheme
}
// Parse as standard URL
parsed, err := url.Parse(repoURL)
if err != nil {
return errRepoURLInvalid
}
// Must have a recognized scheme
switch strings.ToLower(parsed.Scheme) {
case "https", "http", "ssh", "git":
// OK
default:
return errRepoURLInvalid
}
if parsed.Host == "" {
return errRepoURLNoHost
}
if parsed.Path == "" || parsed.Path == "/" {
return errRepoURLNoPath
}
return nil
}

View File

@@ -0,0 +1,56 @@
package handlers
import "testing"
func TestValidateRepoURL(t *testing.T) {
t.Parallel()
tests := []struct {
name string
url string
wantErr bool
}{
// Valid URLs
{name: "https URL", url: "https://github.com/user/repo.git", wantErr: false},
{name: "http URL", url: "http://github.com/user/repo.git", wantErr: false},
{name: "ssh URL", url: "ssh://git@github.com/user/repo.git", wantErr: false},
{name: "git 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: "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},
// Invalid URLs
{name: "empty string", url: "", wantErr: true},
{name: "whitespace only", url: " ", wantErr: true},
{name: "file URL", url: "file:///etc/passwd", wantErr: true},
{name: "file URL uppercase", url: "FILE:///etc/passwd", wantErr: true},
{name: "bare path", url: "/some/local/path", wantErr: true},
{name: "relative path", url: "../repo", wantErr: true},
{name: "just a word", url: "notaurl", wantErr: true},
{name: "ftp URL", url: "ftp://example.com/repo.git", wantErr: true},
{name: "no host https", url: "https:///path", 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: "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: "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 in middle", url: "https://github.com/user/repo/../secret", wantErr: true},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
err := validateRepoURL(tc.url)
if tc.wantErr && err == nil {
t.Errorf("validateRepoURL(%q) = nil, want error", tc.url)
}
if !tc.wantErr && err != nil {
t.Errorf("validateRepoURL(%q) = %v, want nil", tc.url, err)
}
})
}
}

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 == "" {