diff --git a/ISSUES.md b/ISSUES.md new file mode 100644 index 0000000..23acafa --- /dev/null +++ b/ISSUES.md @@ -0,0 +1,72 @@ +# dumpsterChat Codebase Audit Report + +This document contains a comprehensive audit of the dumpsterChat repository, highlighting security vulnerabilities, functional bugs, dead code, and maintenance issues. + +--- + +## 1. Security Vulnerabilities & Bypasses + +### ๐Ÿšจ Plaintext Webhook Tokens Stored in Database (Task 8 Regression/Incomplete) +* **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`. +* **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) +* **Description:** Access to channel reading, message listing, and message posting is checked solely via server membership (`members` table). Role permissions (like `VIEW_CHANNEL` or `SEND_MESSAGES`) and channel-level overrides (`channel_overrides` table) are completely ignored. +* **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 +* **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. +* **Remediation:** Change `SameSite` to `http.SameSiteStrictMode` or `http.SameSiteLaxMode` explicitly. + +### โš ๏ธ WebAuthn Session Cookie Secure Flag Relies on Env Var +* **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. +* **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 +* **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) +* **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. +* **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 +* **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. +* **Remediation:** Change `fmt.Scanln(&email)` to read into a password variable (e.g. `var password string; fmt.Scanln(&password)`). + +--- + +## 3. Dead Code + +### ๐Ÿ—‘๏ธ Unused `ComputeChannelPermissions` +* **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. + +### ๐Ÿ—‘๏ธ 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 +* **Location:** [internal/message/handlers.go](file:///opt/dumpsterChat/internal/message/handlers.go#L31-L35) +* **Description:** `Handler_old` is declared but never instantiated or used. + +--- + +## 4. Maintenance & Infrastructure + +### ๐Ÿงช No Tests +* **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 +* **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. +* **Remediation:** Default fallback to a simple local endpoint without authentication (e.g., `redis://localhost:6379`).