fix: resolve 7 audit issues - webhook tokens, cookie security, avatar save, TUI fallback, dead code, valkey default

ISSUES.md

@@ -6,11 +6,9 @@ This document contains a comprehensive audit of the dumpsterChat repository, hig
 
 ## 1. Security Vulnerabilities & Bypasses
 
-### 🚨 Plaintext Webhook Tokens Stored in Database (Task 8 Regression/Incomplete)
+### 🚨 Plaintext Webhook Tokens Stored in Database (Task 8 Regression/Incomplete) ✅ RESOLVED
 * **Location:** [internal/db/db.go](file:///opt/dumpsterChat/internal/db/db.go#L171-L191), [internal/webhook/handlers.go](file:///opt/dumpsterChat/internal/webhook/handlers.go#L145-L151)
-* **Description:** While a `token_hash` column and SHA-256 validation were added to execute webhooks securely, the database migrations did not drop the plaintext `token` column. Furthermore, when webhooks are created, the handler still inserts the plaintext token directly into the `token` column alongside `token_hash`.
+* **Fix:** DROP COLUMN migration added, INSERT no longer stores plaintext `token`.
-* **Impact:** If the database is compromised, an attacker can retrieve all webhook tokens in plaintext, bypassing the security gains of hashing.
-* **Remediation:** Drop the `token` column from the database schema and modify the `INSERT` query in `Create` webhook handler to only store `token_hash`.
 
 ### 🚨 Message and Channel Access Bypasses Role Permissions & Overrides
 * **Location:** [internal/message/handlers.go](file:///opt/dumpsterChat/internal/message/handlers.go#L671-L692), [internal/channel/handlers.go](file:///opt/dumpsterChat/internal/channel/handlers.go#L165-L187)
@@ -18,45 +16,41 @@ This document contains a comprehensive audit of the dumpsterChat repository, hig
 * **Impact:** Any user who is a member of a server can read, search, and post messages in *any* channel on that server (including private, forum, calendar, or documentation channels), completely bypassing the permissions configuration system.
 * **Remediation:** Update `requireChannelAccess` in `message.Handler` to use `permissions.Checker` to verify channel permission overrides and server roles.
 
-### ⚠️ WebAuthn Session Cookie SameSite Laxity
+### ⚠️ WebAuthn Session Cookie SameSite Laxity ✅ RESOLVED
 * **Location:** [internal/auth/cookie.go](file:///opt/dumpsterChat/internal/auth/cookie.go#L20)
-* **Description:** The session cookie's `SameSite` attribute is set to `http.SameSiteDefaultMode` (which modern browsers default to `SameSite=Lax`). For session tokens, `StrictMode` or explicit `LaxMode` is highly recommended to protect against CSRF.
+* **Fix:** Changed to `http.SameSiteStrictMode`.
-* **Remediation:** Change `SameSite` to `http.SameSiteStrictMode` or `http.SameSiteLaxMode` explicitly.
 
-### ⚠️ WebAuthn Session Cookie Secure Flag Relies on Env Var
+### ⚠️ WebAuthn Session Cookie Secure Flag Relies on Env Var ✅ RESOLVED
 * **Location:** [internal/auth/cookie.go](file:///opt/dumpsterChat/internal/auth/cookie.go#L12-L19)
-* **Description:** The `Secure` flag on the session cookie is determined by `os.Getenv("COOKIE_SECURE") == "true"`. If this environment variable is misconfigured or omitted in production, the session cookie will be sent over unencrypted HTTP connections.
+* **Fix:** Default `secure` to `true` unless `COOKIE_SECURE=false` explicitly set.
-* **Remediation:** Default `secure` to `true` or check if the incoming request is served over HTTPS to automatically enable/disable it.
 
 ---
 
 ## 2. Functional Bugs & Defects
 
-### 🐛 User Avatar Cannot Be Changed
+### 🐛 User Avatar Cannot Be Changed ✅ RESOLVED
-* **Location:** [internal/auth/handlers.go](file:///opt/dumpsterChat/internal/auth/handlers.go#L212-L225), [internal/upload/handlers.go](file:///opt/dumpsterChat/internal/upload/handlers.go#L121-L123), [web/src/components/UserSettings.tsx](file:///opt/dumpsterChat/web/src/components/UserSettings.tsx#L73-L84)
+* **Location:** [internal/auth/handlers.go](file:///opt/dumpsterChat/internal/auth/handlers.go#L212-L225)
-* **Description:** The file upload endpoint `/api/v1/upload` uploads a file and returns its URL, but does not associate it with the user. The `UpdateProfile` handler (`PATCH /auth/me`) does not accept an avatar field in its payload, meaning there is no way to update the user's avatar database column. The frontend settings page allows uploading a new avatar, but since it is never saved to the profile, it reverts to the Gravatar on the next load.
+* **Fix:** Added `AvatarURL` to `updateProfileRequest`, wired into SQL UPDATE for `avatar` column.
-* **Remediation:** Add `AvatarURL` to the `updateProfileRequest` payload and update the database query in `UpdateProfile` to set the `avatar` column.
 
-### 🐛 TUI Client Auth Failure Fallback Bug
+### 🐛 TUI Client Auth Failure Fallback Bug ✅ RESOLVED
 * **Location:** [cmd/tui/auth.go](file:///opt/dumpsterChat/cmd/tui/auth.go#L28)
-* **Description:** If `term.ReadPassword` fails during TUI login, the fallback code runs `fmt.Scanln(&email)` instead of reading the password, which overwrites the user's email with whatever password they type.
+* **Fix:** Fallback reads into password variable instead of overwriting email.
-* **Remediation:** Change `fmt.Scanln(&email)` to read into a password variable (e.g. `var password string; fmt.Scanln(&password)`).
 
 ---
 
 ## 3. Dead Code
 
-### 🗑️ Unused `ComputeChannelPermissions`
+### 🗑️ Unused `ComputeChannelPermissions` ✅ RESOLVED
 * **Location:** [internal/permissions/overrides.go](file:///opt/dumpsterChat/internal/permissions/overrides.go#L10)
-* **Description:** The helper method `ComputeChannelPermissions` (which properly layers channel-level overrides on top of server-wide role permissions) is never called anywhere in the active codebase.
+* **Fix:** Removed the unused function.
 
 ### 🗑️ Duplicated Permission Verification
 * **Location:** [internal/message/handlers.go](file:///opt/dumpsterChat/internal/message/handlers.go#L136-L164)
 * **Description:** The `checkPermission` method in `message.Handler` duplicates role checking queries instead of using the `permissions.Checker` struct.
 
-### 🗑️ Obsolete Handler Type
+### 🗑️ Obsolete Handler Type ✅ RESOLVED
 * **Location:** [internal/message/handlers.go](file:///opt/dumpsterChat/internal/message/handlers.go#L31-L35)
-* **Description:** `Handler_old` is declared but never instantiated or used.
+* **Fix:** Removed `Handler_old`.
 
 ---
 
@@ -66,7 +60,6 @@ This document contains a comprehensive audit of the dumpsterChat repository, hig
 * **Description:** There are zero unit or integration tests in the Go backend or React frontend (`go test ./...` runs but finds no test files).
 * **Remediation:** Introduce tests for critical components such as permissions checking, message parsing, and session stores.
 
-### 🔑 Hardcoded Valkey Password Placeholder
+### 🔑 Hardcoded Valkey Password Placeholder ✅ RESOLVED
 * **Location:** [internal/config/config.go](file:///opt/dumpsterChat/internal/config/config.go#L111)
-* **Description:** The default fallback `VALKEY_URL` contains a masked password placeholder (`redis://localhost:***@example.com`), which is invalid.
+* **Fix:** Default fallback set to `redis://localhost:6379`.
-* **Remediation:** Default fallback to a simple local endpoint without authentication (e.g., `redis://localhost:6379`).

cmd/tui/auth.go

@@ -21,16 +21,23 @@ func RunAuth(api *APIClient) (string, error) {
 	email, _ := reader.ReadString('\n')
 	email = strings.TrimSpace(email)
 
+	var password string
+
 	fmt.Print("  Password: ")
 	pwBytes, err := term.ReadPassword(int(syscall.Stdin))
 	if err != nil {
 		// Fallback to plain read
-		fmt.Scanln(&email)
+		var pwLine string
+		fmt.Scanln(&pwLine)
+		password = strings.TrimSpace(pwLine)
+		fmt.Println()
+		if password == "" {
 			return "", fmt.Errorf("read password: %w", err)
 		}
+	} else {
 		fmt.Println() // newline after password
-
+		password = strings.TrimSpace(string(pwBytes))
-	password := strings.TrimSpace(string(pwBytes))
+	}
 
 	if email == "" || password == "" {
 		return "", fmt.Errorf("email and password are required")

internal/auth/cookie.go

@@ -9,7 +9,7 @@ import (
 // SetSessionCookie writes the session cookie with secure defaults.
 // Shared by password login, registration, and WebAuthn login.
 func SetSessionCookie(w http.ResponseWriter, cookieName, token string, duration time.Duration) {
-	secure := os.Getenv("COOKIE_SECURE") == "true"
+	secure := os.Getenv("COOKIE_SECURE") != "false"
 
 	http.SetCookie(w, &http.Cookie{
 		Name:     cookieName,
@@ -17,7 +17,7 @@ func SetSessionCookie(w http.ResponseWriter, cookieName, token string, duration
 		Path:     "/",
 		HttpOnly: true,
 		Secure:   secure,
-		SameSite: http.SameSiteDefaultMode,
+		SameSite: http.SameSiteStrictMode,
 		MaxAge:   int(duration.Seconds()),
 	})
 }

internal/auth/handlers.go

@@ -210,6 +210,7 @@ type loginRequest struct {
 }
 
 type updateProfileRequest struct {
+	AvatarURL   string `json:"avatar_url"`
 	DisplayName string `json:"display_name"`
 	Bio         string `json:"bio"`
 	AccentColor string `json:"accent_color"`
@@ -482,9 +483,9 @@ func (h *Handler) UpdateProfile(w http.ResponseWriter, r *http.Request) {
 	_, err := h.db.ExecContext(r.Context(), `
 		UPDATE users
 		SET display_name = $2, bio = $3, accent_color = $4, status_text = $5, status = COALESCE(NULLIF($6, ''), status),
-			banner_url = NULLIF($7, ''), tagline = NULLIF($8, ''), pronouns = NULLIF($9, ''), social_links = $10
+			banner_url = NULLIF($7, ''), tagline = NULLIF($8, ''), pronouns = NULLIF($9, ''), social_links = $10, avatar = COALESCE(NULLIF($11, ''), avatar)
 		WHERE id = $1
-	`, userID, req.DisplayName, req.Bio, req.AccentColor, req.StatusText, req.Status, req.BannerURL, req.Tagline, req.Pronouns, socialLinksJSON)
+	`, userID, req.DisplayName, req.Bio, req.AccentColor, req.StatusText, req.Status, req.BannerURL, req.Tagline, req.Pronouns, socialLinksJSON, req.AvatarURL)
 	if err != nil {
 		http.Error(w, `{"error":"update failed"}`, http.StatusInternalServerError)
 		return

internal/config/config.go

@@ -108,7 +108,7 @@ func Load() *Config {
 			SSLMode:  getEnv("POSTGRES_SSLMODE", "require"),
 		},
 		Valkey: ValkeyConfig{
-			URL: getEnv("VALKEY_URL", "redis://localhost:***@example.com"),
+			URL: getEnv("VALKEY_URL", "redis://localhost:6379"),
 		},
 		MinIO: MinIOConfig{
 			Endpoint:  getEnv("MINIO_ENDPOINT", ""),

internal/db/db.go

@@ -189,6 +189,9 @@ CREATE INDEX IF NOT EXISTS idx_webhooks_token ON webhooks(token);
 ALTER TABLE webhooks ADD COLUMN IF NOT EXISTS token_hash VARCHAR(64);
 CREATE INDEX IF NOT EXISTS idx_webhooks_token_hash ON webhooks(token_hash);
 
+-- Drop plaintext token column — token_hash is the only stored credential
+ALTER TABLE webhooks DROP COLUMN IF EXISTS token;
+
 -- Ensure reply_to column exists on messages (added after initial table creation)
 ALTER TABLE messages ADD COLUMN IF NOT EXISTS reply_to UUID REFERENCES messages(id) ON DELETE SET NULL;
 

internal/message/handlers.go

@@ -28,12 +28,6 @@ type Handler struct {
 	logger      *slog.Logger
 }
 
-type Handler_old struct {
-	db       *sql.DB
-	hub      *gateway.Hub
-	mentions *MentionHandler
-}
-
 func NewHandler(db *sql.DB, hub *gateway.Hub, pushHandler *push.Handler, logger *slog.Logger) *Handler {
 	return &Handler{
 		db:          db,

internal/permissions/overrides.go

@@ -5,66 +5,6 @@ import (
 	"database/sql"
 )
 
-// ComputeChannelPermissions returns effective permissions for a user in a specific channel,
-// layering channel overrides on top of server-wide role permissions.
-func (c *Checker) ComputeChannelPermissions(ctx context.Context, serverID, channelID, userID string) (int64, error) {
-	base, err := c.GetUserPermissions(ctx, serverID, userID)
-	if err != nil {
-		return 0, err
-	}
-	if Has(base, ADMINISTRATOR) {
-		return base, nil
-	}
-
-	// Everyone role override applies first.
-	var everyoneID string
-	_ = c.db.QueryRowContext(ctx, `SELECT id FROM roles WHERE server_id = $1 AND is_default = TRUE LIMIT 1`, serverID).Scan(&everyoneID)
-	if everyoneID != "" {
-		allow, deny, err := c.getOverride(ctx, channelID, "role", everyoneID)
-		if err != nil {
-			return 0, err
-		}
-		base |= allow
-		base &^= deny
-	}
-
-	// Member-specific role overrides.
-	rows, err := c.db.QueryContext(ctx, `
-		SELECT r.id FROM roles r
-		INNER JOIN member_roles mr ON mr.role_id = r.id
-		WHERE mr.user_id = $1 AND mr.server_id = $2
-	`, userID, serverID)
-	if err != nil {
-		return 0, err
-	}
-	defer rows.Close()
-	for rows.Next() {
-		var roleID string
-		if err := rows.Scan(&roleID); err != nil {
-			return 0, err
-		}
-		allow, deny, err := c.getOverride(ctx, channelID, "role", roleID)
-		if err != nil {
-			return 0, err
-		}
-		base |= allow
-		base &^= deny
-	}
-	if err := rows.Err(); err != nil {
-		return 0, err
-	}
-
-	// User-specific override wins last.
-	allow, deny, err := c.getOverride(ctx, channelID, "user", userID)
-	if err != nil {
-		return 0, err
-	}
-	base |= allow
-	base &^= deny
-
-	return base, nil
-}
-
 func (c *Checker) getOverride(ctx context.Context, channelID, targetType, targetID string) (allow, deny int64, err error) {
 	err = c.db.QueryRowContext(ctx, `
 		SELECT allow_bitflags, deny_bitflags FROM channel_overrides

internal/webhook/handlers.go

@@ -143,10 +143,10 @@ func (h *Handler) Create(w http.ResponseWriter, r *http.Request) {
 	var avatar sql.NullString
 	var createdAt sql.NullString
 	err = h.db.QueryRowContext(r.Context(), `
-		INSERT INTO webhooks (channel_id, name, token, token_hash, avatar, created_by)
+		INSERT INTO webhooks (channel_id, name, token_hash, avatar, created_by)
-		VALUES ($1, $2, $3, $4, $5, $6)
+		VALUES ($1, $2, $3, $4, $5)
 		RETURNING id, channel_id, name, avatar, created_by, created_at::text
-	`, channelID, req.Name, token, tokenHash, req.Avatar, userID).Scan(
+	`, channelID, req.Name, tokenHash, req.Avatar, userID).Scan(
 		&wh.ID, &wh.ChannelID, &wh.Name, &avatar, &wh.CreatedBy, &createdAt,
 	)
 	if err != nil {