Refactor filesystem operations to validate repository paths and references

This commit introduces robust validation for repository paths and references within the filesystem operations.
In internal/storage/filesystem.go, the sanitizeReference function is updated to return an error for invalid reference strings, including checks for empty strings, path separators, and invalid characters. This change is mirrored in the PutManifest function to ensure that references are sanitized before being used to construct paths.
The Filesystem struct methods, specifically ManifestPath, PutManifest, and linkManifestTag, now explicitly check for errors returned by sanitizeReference, ensuring that only valid references are processed.
Tests are updated to validate this new behavior. internal/storage/filesystem_test.go now includes a new test, TestFilesystemRejectsUnsafeTagReferences, which attempts to use various unsafe tag references to confirm that PutManifest, LinkManifestTag, and GetManifest reject them, enforcing the new validation logic.
Additionally, internal/auth/password.go is updated to enforce a check against parallelism values in argon2 parameters, preventing overflows when setting secret hash parallelism.
Configuration changes in internal/config/config.go update the structure of HTTPConfig and Default() to explicitly set SecureCookies to true, aligning with potential security requirements. Tests in

Author: VisualBean

README.md

@@ -52,6 +52,7 @@ The published Docker image starts SCR with `-config /etc/scr/config.yaml`. The i
 http:
   address: "0.0.0.0"
   port: 5000
+  secureCookies: true
 
 storage:
   rootDirectory: "/var/lib/scr/registry"
@@ -84,6 +85,7 @@ docker run --rm --name scr \
 Configuration supports these sections:
 
 - `http.address` and `http.port`
+- `http.secureCookies`; defaults to `true`. Leave enabled when SCR is accessed over HTTPS, including behind an HTTPS-terminating reverse proxy. Set to `false` only when serving the admin UI directly over plain HTTP.
 - `storage.rootDirectory`
 - `storage.gc`
 - `storage.gcDelay`
@@ -111,6 +113,14 @@ Bootstrap admin username and password are normally provided with environment var
 
 If bootstrap admin values are omitted from the config file, SCR fills them from those environment variables. Provide both values together.
 
+### Admin UI cookies and reverse proxies
+
+SCR stores admin UI sessions in an `HttpOnly`, `SameSite=Lax` cookie. By default, `http.secureCookies` is `true`, which also marks that cookie `Secure` so browsers only send it over HTTPS.
+
+Keep `http.secureCookies: true` for production deployments, including the common setup where a reverse proxy terminates HTTPS and forwards plain HTTP to SCR. The browser only sees the public HTTPS URL, so the `Secure` cookie works normally even if the proxy-to-SCR hop is HTTP.
+
+Set `http.secureCookies: false` only when users access SCR directly over plain HTTP, such as a local development instance or a trusted internal HTTP-only deployment. Do not disable it for an HTTPS reverse-proxy deployment.
+
 ## Authentication and access
 
 Registry clients use Docker-compatible bearer-token authentication:

config.container.yaml

@@ -1,6 +1,7 @@
 http:
   address: "0.0.0.0"
   port: 5000
+  secureCookies: true
 
 storage:
   rootDirectory: "/var/lib/scr/registry"

config.example.yaml

@@ -1,6 +1,7 @@
 http:
   address: "0.0.0.0"
   port: 5000
+  secureCookies: true
 
 storage:
   rootDirectory: "/var/lib/scr/registry"

config.local.example.yaml

@@ -1,6 +1,8 @@
 http:
   address: "0.0.0.0"
   port: 5000
+  # Set true when accessing SCR through HTTPS, including an HTTPS-terminating reverse proxy.
+  secureCookies: false
 
 storage:
   rootDirectory: "data/registry"

internal/auth/password.go

@@ -83,6 +83,9 @@ func decodeHash(encoded string) (argonParams, []byte, []byte, error) {
 		case "t":
 			params.iterations = uint32(parsed)
 		case "p":
+			if parsed > 255 {
+				return argonParams{}, nil, nil, errors.New("secret hash parallelism is too large")
+			}
 			params.parallelism = uint8(parsed)
 		default:
 			return argonParams{}, nil, nil, fmt.Errorf("unknown secret hash param %q", name)

internal/auth/password_test.go

@@ -32,3 +32,10 @@ func TestGenerateSecretReturnsOpaqueValue(t *testing.T) {
 		t.Fatalf("generated secret too short: %d", len(secret))
 	}
 }
+
+func TestVerifySecretRejectsParallelismOverflow(t *testing.T) {
+	encoded := "$argon2id$v=19$m=65536,t=3,p=256$c2FsdHNhbHRzYWx0c2FsdA$AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+	if _, err := VerifySecret("secret", encoded); err == nil {
+		t.Fatal("expected VerifySecret to reject p value that overflows uint8")
+	}
+}

internal/config/config.go

@@ -20,8 +20,9 @@ type Config struct {
 }
 
 type HTTPConfig struct {
-	Address string `yaml:"address"`
-	Port    int    `yaml:"port"`
+	Address       string `yaml:"address"`
+	Port          int    `yaml:"port"`
+	SecureCookies bool   `yaml:"secureCookies"`
 }
 
 type StorageConfig struct {
@@ -82,8 +83,9 @@ func (c *Config) applyEnvironment() {
 func Default() Config {
 	return Config{
 		HTTP: HTTPConfig{
-			Address: "0.0.0.0",
-			Port:    5000,
+			Address:       "0.0.0.0",
+			Port:          5000,
+			SecureCookies: true,
 		},
 		Storage: StorageConfig{
 			RootDirectory: "/var/lib/scr/registry",

internal/config/config_test.go

@@ -15,6 +15,9 @@ func TestLoadDefaultsWhenConfigMissing(t *testing.T) {
 	if cfg.HTTP.Port != 5000 {
 		t.Fatalf("expected default port 5000, got %d", cfg.HTTP.Port)
 	}
+	if !cfg.HTTP.SecureCookies {
+		t.Fatal("expected secure cookies to be enabled by default")
+	}
 	if cfg.Storage.RootDirectory != "/var/lib/scr/registry" {
 		t.Fatalf("unexpected default storage root %q", cfg.Storage.RootDirectory)
 	}
@@ -45,6 +48,7 @@ func TestLoadParsesDurationFields(t *testing.T) {
 http:
   address: "127.0.0.1"
   port: 5000
+  secureCookies: false
 storage:
   rootDirectory: "/tmp/registry"
   gcDelay: "30m"
@@ -67,6 +71,9 @@ auth:
 	if cfg.Auth.TokenTTL.Std() != 5*time.Minute {
 		t.Fatalf("unexpected token ttl %s", cfg.Auth.TokenTTL.Std())
 	}
+	if cfg.HTTP.SecureCookies {
+		t.Fatal("expected secure cookies to be configurable")
+	}
 	if cfg.Storage.GCDelay.Std() != 30*time.Minute {
 		t.Fatalf("unexpected gc delay %s", cfg.Storage.GCDelay.Std())
 	}

internal/httpserver/server.go

@@ -772,6 +772,10 @@ func writeStorageError(w http.ResponseWriter, err error) {
 		writeError(w, http.StatusBadRequest, "invalid digest")
 		return
 	}
+	if errors.Is(err, storage.ErrInvalidReference) {
+		writeError(w, http.StatusBadRequest, "invalid reference")
+		return
+	}
 	if errors.Is(err, storage.ErrNotFound) {
 		writeError(w, http.StatusNotFound, "not found")
 		return

internal/httpserver/server_test.go

@@ -530,9 +530,28 @@ func TestUILoginAndDashboard(t *testing.T) {
 	if len(loginResponse.Result().Cookies()) == 0 {
 		t.Fatal("expected session cookie")
 	}
+	sessionCookie := loginResponse.Result().Cookies()[0]
+	if !sessionCookie.Secure || !sessionCookie.HttpOnly || sessionCookie.SameSite != http.SameSiteLaxMode {
+		t.Fatalf("expected secure HttpOnly SameSite=Lax session cookie, got %#v", sessionCookie)
+	}
+
+	logoutRequest := httptest.NewRequest(http.MethodPost, "/ui/logout", nil)
+	logoutRequest.AddCookie(sessionCookie)
+	logoutResponse := httptest.NewRecorder()
+	handler.ServeHTTP(logoutResponse, logoutRequest)
+	if logoutResponse.Code != http.StatusFound {
+		t.Fatalf("expected logout redirect, got %d: %s", logoutResponse.Code, logoutResponse.Body.String())
+	}
+	if len(logoutResponse.Result().Cookies()) == 0 {
+		t.Fatal("expected logout clearing cookie")
+	}
+	clearedCookie := logoutResponse.Result().Cookies()[0]
+	if !clearedCookie.Secure || !clearedCookie.HttpOnly || clearedCookie.SameSite != http.SameSiteLaxMode || clearedCookie.MaxAge != -1 {
+		t.Fatalf("expected secure clearing cookie, got %#v", clearedCookie)
+	}
 
 	dashboardRequest := httptest.NewRequest(http.MethodGet, "/ui", nil)
-	dashboardRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	dashboardRequest.AddCookie(sessionCookie)
 	dashboardResponse := httptest.NewRecorder()
 	handler.ServeHTTP(dashboardResponse, dashboardRequest)
 	if dashboardResponse.Code != http.StatusOK {
@@ -549,7 +568,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 	}
 
 	settingsRequest := httptest.NewRequest(http.MethodGet, "/ui/settings", nil)
-	settingsRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	settingsRequest.AddCookie(sessionCookie)
 	settingsResponse := httptest.NewRecorder()
 	handler.ServeHTTP(settingsResponse, settingsRequest)
 	if settingsResponse.Code != http.StatusOK {
@@ -567,7 +586,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 	settingsForm.Set("interval", "2h")
 	settingsUpdateRequest := httptest.NewRequest(http.MethodPost, "/ui/settings/gc", strings.NewReader(settingsForm.Encode()))
 	settingsUpdateRequest.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-	settingsUpdateRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	settingsUpdateRequest.AddCookie(sessionCookie)
 	settingsUpdateResponse := httptest.NewRecorder()
 	handler.ServeHTTP(settingsUpdateResponse, settingsUpdateRequest)
 	if settingsUpdateResponse.Code != http.StatusFound {
@@ -584,7 +603,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 	webhookForm.Set("url", "https://example.com/scr-events")
 	webhookUpdateRequest := httptest.NewRequest(http.MethodPost, "/ui/settings/webhook", strings.NewReader(webhookForm.Encode()))
 	webhookUpdateRequest.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-	webhookUpdateRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	webhookUpdateRequest.AddCookie(sessionCookie)
 	webhookUpdateResponse := httptest.NewRecorder()
 	handler.ServeHTTP(webhookUpdateResponse, webhookUpdateRequest)
 	if webhookUpdateResponse.Code != http.StatusFound {
@@ -614,7 +633,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 		}
 	}
 	trafficRequest := httptest.NewRequest(http.MethodGet, "/ui?repository="+url.QueryEscape("ui/app"), nil)
-	trafficRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	trafficRequest.AddCookie(sessionCookie)
 	trafficResponse := httptest.NewRecorder()
 	handler.ServeHTTP(trafficResponse, trafficRequest)
 	if trafficResponse.Code != http.StatusOK {
@@ -627,7 +646,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 		t.Fatalf("expected filtered dashboard traffic for ui/app, got %s", trafficResponse.Body.String())
 	}
 	repositoriesRequest := httptest.NewRequest(http.MethodGet, "/ui/repositories", nil)
-	repositoriesRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	repositoriesRequest.AddCookie(sessionCookie)
 	repositoriesResponse := httptest.NewRecorder()
 	handler.ServeHTTP(repositoriesResponse, repositoriesRequest)
 	if repositoriesResponse.Code != http.StatusOK {
@@ -652,7 +671,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 		t.Fatalf("expected tag delete semantics copy, got %s", repositoriesResponse.Body.String())
 	}
 	searchRequest := httptest.NewRequest(http.MethodGet, "/ui/repositories?q=stable", nil)
-	searchRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	searchRequest.AddCookie(sessionCookie)
 	searchResponse := httptest.NewRecorder()
 	handler.ServeHTTP(searchResponse, searchRequest)
 	if searchResponse.Code != http.StatusOK {
@@ -662,7 +681,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 		t.Fatalf("expected repository search to preserve query and return tag match, got %s", searchResponse.Body.String())
 	}
 	missingSearchRequest := httptest.NewRequest(http.MethodGet, "/ui/repositories?q=no-such-tag", nil)
-	missingSearchRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	missingSearchRequest.AddCookie(sessionCookie)
 	missingSearchResponse := httptest.NewRecorder()
 	handler.ServeHTTP(missingSearchResponse, missingSearchRequest)
 	if missingSearchResponse.Code != http.StatusOK {
@@ -677,7 +696,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 	deleteTagForm.Set("tag", "latest")
 	deleteTagRequest := httptest.NewRequest(http.MethodPost, "/ui/repositories/delete-tag", strings.NewReader(deleteTagForm.Encode()))
 	deleteTagRequest.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-	deleteTagRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	deleteTagRequest.AddCookie(sessionCookie)
 	deleteTagResponse := httptest.NewRecorder()
 	handler.ServeHTTP(deleteTagResponse, deleteTagRequest)
 	if deleteTagResponse.Code != http.StatusFound {
@@ -696,7 +715,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 	deleteRepositoryForm.Set("repository", "ui/app")
 	deleteRepositoryRequest := httptest.NewRequest(http.MethodPost, "/ui/repositories/delete", strings.NewReader(deleteRepositoryForm.Encode()))
 	deleteRepositoryRequest.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-	deleteRepositoryRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	deleteRepositoryRequest.AddCookie(sessionCookie)
 	deleteRepositoryResponse := httptest.NewRecorder()
 	handler.ServeHTTP(deleteRepositoryResponse, deleteRepositoryRequest)
 	if deleteRepositoryResponse.Code != http.StatusFound {
@@ -714,7 +733,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 	createUserForm.Set("expiresAt", "2026-08-02")
 	createUserRequest := httptest.NewRequest(http.MethodPost, "/ui/users", strings.NewReader(createUserForm.Encode()))
 	createUserRequest.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-	createUserRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	createUserRequest.AddCookie(sessionCookie)
 	createUserResponse := httptest.NewRecorder()
 	handler.ServeHTTP(createUserResponse, createUserRequest)
 	if createUserResponse.Code != http.StatusOK {
@@ -725,7 +744,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 	}
 
 	usersRequest := httptest.NewRequest(http.MethodGet, "/ui/users", nil)
-	usersRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	usersRequest.AddCookie(sessionCookie)
 	usersResponse := httptest.NewRecorder()
 	handler.ServeHTTP(usersResponse, usersRequest)
 	if usersResponse.Code != http.StatusOK {
@@ -749,7 +768,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 		t.Fatalf("expected default wildcard pull grant, got %#v", grants)
 	}
 	usersWithGrantRequest := httptest.NewRequest(http.MethodGet, "/ui/users", nil)
-	usersWithGrantRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	usersWithGrantRequest.AddCookie(sessionCookie)
 	usersWithGrantResponse := httptest.NewRecorder()
 	handler.ServeHTTP(usersWithGrantResponse, usersWithGrantRequest)
 	if usersWithGrantResponse.Code != http.StatusOK {
@@ -774,7 +793,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 	validityForm.Add("actions", "push")
 	validityRequest := httptest.NewRequest(http.MethodPost, "/ui/users/"+createdUser.ID+"/access", strings.NewReader(validityForm.Encode()))
 	validityRequest.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-	validityRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	validityRequest.AddCookie(sessionCookie)
 	validityResponse := httptest.NewRecorder()
 	handler.ServeHTTP(validityResponse, validityRequest)
 	if validityResponse.Code != http.StatusFound {
@@ -800,7 +819,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 	}
 	adminValidityRequest := httptest.NewRequest(http.MethodPost, "/ui/users/"+adminUser.ID+"/access", strings.NewReader(validityForm.Encode()))
 	adminValidityRequest.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-	adminValidityRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	adminValidityRequest.AddCookie(sessionCookie)
 	adminValidityResponse := httptest.NewRecorder()
 	handler.ServeHTTP(adminValidityResponse, adminValidityRequest)
 	if adminValidityResponse.Code != http.StatusOK || !strings.Contains(adminValidityResponse.Body.String(), "Admin users are managed outside this user list") {
@@ -809,7 +828,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 
 	deleteUser := createHTTPTestUser(t, ctx, store, "delete-me", "Delete Me", domain.RoleReader, "delete-secret", time.Now().UTC())
 	deleteRequest := httptest.NewRequest(http.MethodPost, "/ui/users/"+deleteUser.ID+"/delete", nil)
-	deleteRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	deleteRequest.AddCookie(sessionCookie)
 	deleteResponse := httptest.NewRecorder()
 	handler.ServeHTTP(deleteResponse, deleteRequest)
 	if deleteResponse.Code != http.StatusFound {
@@ -820,7 +839,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 	}
 
 	auditRequest := httptest.NewRequest(http.MethodGet, "/ui/audit", nil)
-	auditRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	auditRequest.AddCookie(sessionCookie)
 	auditResponse := httptest.NewRecorder()
 	handler.ServeHTTP(auditResponse, auditRequest)
 	if auditResponse.Code != http.StatusOK {
@@ -834,7 +853,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 	}
 
 	auditSearchRequest := httptest.NewRequest(http.MethodGet, "/ui/audit?q=ui-reader", nil)
-	auditSearchRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	auditSearchRequest.AddCookie(sessionCookie)
 	auditSearchResponse := httptest.NewRecorder()
 	handler.ServeHTTP(auditSearchResponse, auditSearchRequest)
 	if auditSearchResponse.Code != http.StatusOK {
@@ -845,7 +864,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 	}
 
 	authFilterRequest := httptest.NewRequest(http.MethodGet, "/ui/audit?action=authentication", nil)
-	authFilterRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	authFilterRequest.AddCookie(sessionCookie)
 	authFilterResponse := httptest.NewRecorder()
 	handler.ServeHTTP(authFilterResponse, authFilterRequest)
 	if authFilterResponse.Code != http.StatusOK {
@@ -856,7 +875,7 @@ func TestUILoginAndDashboard(t *testing.T) {
 	}
 
 	emptyAuditRequest := httptest.NewRequest(http.MethodGet, "/ui/audit?q=no-such-audit-event", nil)
-	emptyAuditRequest.AddCookie(loginResponse.Result().Cookies()[0])
+	emptyAuditRequest.AddCookie(sessionCookie)
 	emptyAuditResponse := httptest.NewRecorder()
 	handler.ServeHTTP(emptyAuditResponse, emptyAuditRequest)
 	if emptyAuditResponse.Code != http.StatusOK {
@@ -999,6 +1018,47 @@ func TestRegistryWebhookFailureDoesNotFailRegistryRequest(t *testing.T) {
 	waitForWebhookAttempt(t, &attempts)
 }
 
+func TestUILoginCanDisableSecureCookieForDirectHTTP(t *testing.T) {
+	ctx := context.Background()
+	cfg := config.Default()
+	cfg.HTTP.SecureCookies = false
+	cfg.Storage.RootDirectory = filepath.Join(t.TempDir(), "registry")
+	cfg.Database.DSN = filepath.Join(t.TempDir(), "test.db")
+	store, err := db.Open(ctx, cfg.Database.DSN)
+	if err != nil {
+		t.Fatalf("Open() error = %v", err)
+	}
+	t.Cleanup(func() { _ = store.Close() })
+	if err := store.InitSchema(ctx); err != nil {
+		t.Fatalf("InitSchema() error = %v", err)
+	}
+	if err := store.EnsureActiveSigningKey(ctx); err != nil {
+		t.Fatalf("EnsureActiveSigningKey() error = %v", err)
+	}
+	if err := auth.BootstrapAdmin(ctx, store, "admin", "secret", time.Now().UTC()); err != nil {
+		t.Fatalf("BootstrapAdmin() error = %v", err)
+	}
+	handler := New(Options{Config: cfg, Store: store})
+
+	loginForm := url.Values{}
+	loginForm.Set("username", "admin")
+	loginForm.Set("password", "secret")
+	loginRequest := httptest.NewRequest(http.MethodPost, "/ui/login", strings.NewReader(loginForm.Encode()))
+	loginRequest.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	loginResponse := httptest.NewRecorder()
+	handler.ServeHTTP(loginResponse, loginRequest)
+	if loginResponse.Code != http.StatusFound {
+		t.Fatalf("expected login redirect, got %d: %s", loginResponse.Code, loginResponse.Body.String())
+	}
+	if len(loginResponse.Result().Cookies()) == 0 {
+		t.Fatal("expected session cookie")
+	}
+	sessionCookie := loginResponse.Result().Cookies()[0]
+	if sessionCookie.Secure || !sessionCookie.HttpOnly || sessionCookie.SameSite != http.SameSiteLaxMode {
+		t.Fatalf("expected insecure opt-out to affect only Secure flag, got %#v", sessionCookie)
+	}
+}
+
 func TestRootRedirectsToRegistryAPI(t *testing.T) {
 	ctx := context.Background()
 	cfg := config.Default()