From b257df3c93b65b34ace297fdbd6e7ddac04d9d17 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 10:08:01 +0000 Subject: [PATCH 1/4] Initial plan From 3d3b213b4e89dbe590f096e43a6d33f12af6cae9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 10:21:10 +0000 Subject: [PATCH 2/4] docs: add stride threat model and warn on default email secret Agent-Logs-Url: https://github.com/dlukt/discool/sessions/31860015-a9ca-4c61-968d-ebd0bb908190 Co-authored-by: dlukt <201112286+dlukt@users.noreply.github.com> --- .../stride-threat-model-2026-03-29.md | 139 ++++++++++++++++++ config.example.toml | 3 +- server/src/config/settings.rs | 23 ++- server/src/main.rs | 6 + 4 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 _bmad-output/implementation-artifacts/stride-threat-model-2026-03-29.md diff --git a/_bmad-output/implementation-artifacts/stride-threat-model-2026-03-29.md b/_bmad-output/implementation-artifacts/stride-threat-model-2026-03-29.md new file mode 100644 index 0000000..ca2e044 --- /dev/null +++ b/_bmad-output/implementation-artifacts/stride-threat-model-2026-03-29.md @@ -0,0 +1,139 @@ +# Discool STRIDE Threat Model and OWASP Security Sweep + +Date: 2026-03-29 + +## Scope + +- Svelte SPA client served from the Rust backend (`/home/runner/work/discool/discool/client/src/main.ts`) +- Axum HTTP + WebSocket server (`/home/runner/work/discool/discool/server/src/handlers/mod.rs`) +- SQLx-backed persistence, local avatar/attachment storage, libp2p, and WebRTC (`/home/runner/work/discool/discool/server/src`) + +## System Summary + +- The SPA is embedded into the Rust server and served alongside the API and `/ws` endpoint (`server/src/handlers/mod.rs:35-269`, `server/src/static_files.rs`). +- Authentication is DID + challenge/response based; session tokens are stored in the database and sent as bearer tokens (`server/src/services/auth_service.rs:23-77`, `server/src/middleware/auth.rs:17-54`). +- The client persists the session in `localStorage` under `discool-session` (`client/src/lib/features/identity/identityStore.svelte.ts:36-56`). +- The app stores message attachments and avatars on the local filesystem (`server/src/services/file_storage_service.rs:41-80`). +- Optional `/metrics` exposure is controlled by config, but the route itself is unauthenticated when enabled (`server/src/handlers/mod.rs:234-260`). + +## Key Assets + +1. Session tokens and authenticated WebSocket connections +2. User DID identities, encrypted recovery material, and recovery-email encryption secret +3. User content: messages, DMs, moderation reports, attachments, avatars +4. Admin-only health and backup capabilities +5. Database contents and backups +6. P2P identity key and live federation metadata +7. Operational metadata exposed via `/metrics`, `/healthz`, `/readyz`, and logs + +## Trust Boundaries + +1. **Browser ↔ HTTP API**: untrusted client input crosses into authenticated REST handlers. +2. **Browser ↔ WebSocket gateway**: persistent bidirectional channel with rate-limited client operations (`server/src/ws/gateway.rs:34-35`). +3. **Server ↔ Database**: sessions, user data, moderation data, and instance configuration persist here. +4. **Server ↔ Filesystem**: local storage for attachments, avatars, and P2P identity keys. +5. **Server ↔ Email provider**: recovery and verification workflows depend on SMTP configuration. +6. **Server ↔ Peer network**: libp2p traffic crosses an external trust boundary when federation is enabled. +7. **Operator ↔ Deployment config**: security posture depends on configuration such as TLS termination, metrics enablement, and email secret choice. + +## Attack Surfaces + +- Open auth endpoints: `/api/v1/auth/register`, `/challenge`, `/verify` +- Authenticated REST API and moderation/admin routes +- WebSocket endpoint `/ws` +- Static file serving and client-side markdown rendering +- Attachment/avatar multipart upload endpoints +- Optional `/metrics` endpoint +- Backup creation endpoint invoking database backup logic +- libp2p and WebRTC listeners when enabled + +## STRIDE Analysis + +| Category | Primary Threats | Notes / Evidence | +|---|---|---| +| **Spoofing** | Stolen bearer tokens let attackers impersonate users; admin gate is currently username-based rather than a dedicated admin credential model. | `client/src/lib/features/identity/identityStore.svelte.ts:36-56`, `server/src/handlers/admin.rs:50-86` | +| **Tampering** | Multipart metadata and other user-controlled payloads attempt to alter stored content or moderation state; filesystem writes rely on validated storage keys. | `server/src/services/file_storage_service.rs:46-100` | +| **Repudiation** | Weak auditability for admin access because admin authorization is a pre-auth stub and not strongly bound to a dedicated role/credential. | `server/src/handlers/admin.rs:78-86` | +| **Information Disclosure** | Bearer tokens can traverse plaintext links if TLS is not terminated externally; `/metrics` can expose internals when enabled; `localStorage` increases XSS blast radius. | `server/src/middleware/auth.rs:24-42`, `server/src/handlers/mod.rs:234-260`, `client/src/lib/features/identity/identityStore.svelte.ts:49-56` | +| **Denial of Service** | WebSocket floods, dependency-driven ReDoS, repeated auth attempts, and peer/network abuse can consume resources; some rate limits already exist. | `server/src/ws/gateway.rs:34-35`, `npm audit` output for `picomatch`, `server/src/services/auth_service.rs`, `server/src/p2p` | +| **Elevation of Privilege** | Admin-only routes and metrics exposure rely heavily on deployment/configuration discipline; token theft or weak admin checks can extend user privileges. | `server/src/handlers/admin.rs`, `server/src/handlers/mod.rs:237-260` | + +## Potential Abuse Paths + +1. Inject script via a future XSS bug or compromised dependency, steal `discool-session` from `localStorage`, then reuse the bearer token against REST and `/ws`. +2. Intercept unencrypted traffic if the app is deployed without TLS termination, capturing bearer tokens or recovery links in transit. +3. Query `/metrics` after an operator enables it, extracting operational internals for reconnaissance. +4. Abuse the admin stub by compromising the configured admin account and reaching backup/health endpoints with username-based authorization. +5. Exploit vulnerable dependency behavior in the frontend toolchain (current `picomatch` advisory) to degrade tooling or CI execution. +6. Leave the example `email.server_secret` unchanged, causing recovery-email encryption to share a public default secret across deployments. + +## OWASP-Style Security Sweep + +### 1. High — Missing in-process TLS enforcement / bearer-token transport risk + +- **Threat model link**: Information disclosure, spoofing +- **Evidence**: Authentication relies on bearer tokens in the `Authorization` header (`server/src/middleware/auth.rs:24-42`), while server config exposes host/port only and does not implement TLS termination (`server/src/config/settings.rs`, `server/src/main.rs:84-118`). +- **Impact**: A deployment mistake can expose session tokens and recovery links over plaintext HTTP or WS. +- **Mitigations**: + - Require TLS termination in front of the app and document it as mandatory production guidance. + - Optionally reject non-loopback startup in production unless a proxy/TLS setting is explicitly acknowledged. + - Consider secure cookies for browser sessions if the architecture evolves beyond bearer tokens. + +### 2. High — Dependency security finding in frontend toolchain (`picomatch`) + +- **Threat model link**: Denial of service / supply-chain risk +- **Evidence**: `npm audit --audit-level=high` reports `GHSA-3v7f-55p6-f55p` and `GHSA-c2c7-rcm5-vvqj` against `picomatch 4.0.0 - 4.0.3`. +- **Impact**: Maliciously crafted glob patterns can trigger incorrect matching or ReDoS in affected execution paths. +- **Mitigations**: + - Update the affected dependency chain with `npm audit fix` once compatibility is confirmed. + - Keep lockfile-based CI auditing enabled. + +### 3. Medium — Session token persisted in `localStorage` + +- **Threat model link**: Spoofing, information disclosure, elevation of privilege +- **Evidence**: `discool-session` is written to and read from `localStorage` (`client/src/lib/features/identity/identityStore.svelte.ts:36-56`, `93-130`, `149-152`). +- **Impact**: Any XSS or malicious extension can exfiltrate a live bearer token. +- **Mitigations**: + - Prefer `HttpOnly`, `Secure`, `SameSite` cookies if compatible with the auth model. + - Keep the CSP strict and continue sanitizing rendered markdown (`server/src/handlers/mod.rs:31-33`, `client/src/lib/features/chat/messageMarkdown.ts:41-60`). + - Shorten session lifetime or add device/session binding for defense in depth. + +### 4. Medium — `/metrics` becomes public when enabled + +- **Threat model link**: Information disclosure +- **Evidence**: The router exposes `/metrics` without auth when metrics are enabled (`server/src/handlers/mod.rs:234-260`). +- **Impact**: Metrics can leak internal topology, traffic patterns, and capacity data to any reachable client. +- **Mitigations**: + - Protect `/metrics` behind network policy, reverse-proxy auth, or an internal-only bind. + - Consider adding an app-level auth or shared secret guard if public deployment is common. + +### 5. Medium — Admin authorization is still a stub + +- **Threat model link**: Spoofing, repudiation, elevation of privilege +- **Evidence**: Admin access is granted by matching the authenticated username against the first row in `admin_users`, and the code itself marks this as a TODO (`server/src/handlers/admin.rs:50-86`, `175-186`). +- **Impact**: The model is brittle, difficult to audit, and weaker than explicit RBAC or separate admin credentials. +- **Mitigations**: + - Replace the username check with a first-class admin role/permission model. + - Add stronger audit logging for admin actions and consider step-up authentication for backups. + +### 6. Low — Example recovery-email server secret can be left unchanged + +- **Threat model link**: Information disclosure / configuration weakness +- **Evidence**: The default config uses `change-me-in-production` for `email.server_secret` (`server/src/config/settings.rs:964-966`, `config.example.toml:232-234`). +- **Impact**: If an operator deploys with the example secret, recovery-email encryption strength collapses to a known shared secret. +- **Mitigations**: + - Fail startup or warn loudly when the example secret is used. + - Generate and document a unique per-instance secret. +- **Status**: Addressed in this change by adding a startup warning and clarifying the config example. + +## Existing Positive Controls + +- Strict CSP and defensive security headers (`server/src/handlers/mod.rs:31-33`, `280-299`) +- One-time auth challenge consumption (`server/src/services/auth_service.rs:66-76`) +- Filesystem storage-key validation (`server/src/services/file_storage_service.rs:83-100`) +- Markdown sanitization and external URL safety checks (`client/src/lib/features/chat/messageMarkdown.ts:41-60`, `71-153`) +- WebSocket rate limiting (`server/src/ws/gateway.rs:34-35`) + +## Residual Risk Summary + +The strongest remaining risks are deployment/configuration-dependent: transport security, public metrics exposure, and the current admin authorization model. The least-priority actionable issue was the example `email.server_secret`; this review addresses it with a warning so insecure deployments are easier to detect before recovery email is enabled in production. diff --git a/config.example.toml b/config.example.toml index 88392da..315e13a 100644 --- a/config.example.toml +++ b/config.example.toml @@ -229,5 +229,6 @@ # Per-IP verification attempt limit (default: 20 per hour) # verify_rate_limit_per_hour = 20 # -# Secret used to derive server-side recovery encryption keys (required in production) +# Secret used to derive server-side recovery encryption keys (required in production; +# leaving the example value in place will trigger a startup warning) # server_secret = "change-me-in-production" diff --git a/server/src/config/settings.rs b/server/src/config/settings.rs index f2c5169..20b68f4 100644 --- a/server/src/config/settings.rs +++ b/server/src/config/settings.rs @@ -34,6 +34,10 @@ impl Config { self.metrics.as_ref().is_some_and(|m| m.enabled) } + pub fn uses_insecure_default_email_server_secret(&self) -> bool { + self.email.server_secret.trim() == DEFAULT_EMAIL_SERVER_SECRET + } + pub fn validate(&self) -> Result<(), ConfigValidationError> { if self.server.host.trim().is_empty() { return Err(ConfigValidationError::new( @@ -961,8 +965,10 @@ fn default_email_verify_rate_limit_per_hour() -> u32 { 20 } +const DEFAULT_EMAIL_SERVER_SECRET: &str = "change-me-in-production"; + fn default_email_server_secret() -> String { - "change-me-in-production".to_string() + DEFAULT_EMAIL_SERVER_SECRET.to_string() } #[derive(Debug, Clone, Deserialize)] @@ -1183,6 +1189,21 @@ mod tests { assert!(!MetricsConfig::default().enabled); } + #[test] + fn default_config_reports_insecure_default_email_server_secret() { + let cfg = Config::default(); + + assert!(cfg.uses_insecure_default_email_server_secret()); + } + + #[test] + fn custom_email_server_secret_is_not_reported_as_insecure_default() { + let mut cfg = Config::default(); + cfg.email.server_secret = "super-secret-and-unique".to_string(); + + assert!(!cfg.uses_insecure_default_email_server_secret()); + } + #[test] fn validate_rejects_port_0() { let mut cfg = Config::default(); diff --git a/server/src/main.rs b/server/src/main.rs index 7a7af02..b6c2e06 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -22,6 +22,12 @@ async fn main() { std::process::exit(1); } + if config.uses_insecure_default_email_server_secret() { + tracing::warn!( + "email.server_secret is using the example default; change it before enabling recovery email in production" + ); + } + config.log_summary(); let config = std::sync::Arc::new(config); From f2a87c324a61c13281405135e51bd6a2524efdaa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 10:25:30 +0000 Subject: [PATCH 3/4] chore: polish email secret warning helper Agent-Logs-Url: https://github.com/dlukt/discool/sessions/31860015-a9ca-4c61-968d-ebd0bb908190 Co-authored-by: dlukt <201112286+dlukt@users.noreply.github.com> --- server/src/config/settings.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/config/settings.rs b/server/src/config/settings.rs index 20b68f4..0d521f5 100644 --- a/server/src/config/settings.rs +++ b/server/src/config/settings.rs @@ -35,7 +35,7 @@ impl Config { } pub fn uses_insecure_default_email_server_secret(&self) -> bool { - self.email.server_secret.trim() == DEFAULT_EMAIL_SERVER_SECRET + self.email.server_secret.trim() == DEFAULT_EMAIL_SERVER_SECRET.trim() } pub fn validate(&self) -> Result<(), ConfigValidationError> { From 0839c823aa0761c5ec216dddcc073d6e3a69d6d4 Mon Sep 17 00:00:00 2001 From: Darko Luketic Date: Sun, 29 Mar 2026 12:26:34 +0200 Subject: [PATCH 4/4] Update _bmad-output/implementation-artifacts/stride-threat-model-2026-03-29.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../stride-threat-model-2026-03-29.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/_bmad-output/implementation-artifacts/stride-threat-model-2026-03-29.md b/_bmad-output/implementation-artifacts/stride-threat-model-2026-03-29.md index ca2e044..d771807 100644 --- a/_bmad-output/implementation-artifacts/stride-threat-model-2026-03-29.md +++ b/_bmad-output/implementation-artifacts/stride-threat-model-2026-03-29.md @@ -4,9 +4,9 @@ Date: 2026-03-29 ## Scope -- Svelte SPA client served from the Rust backend (`/home/runner/work/discool/discool/client/src/main.ts`) -- Axum HTTP + WebSocket server (`/home/runner/work/discool/discool/server/src/handlers/mod.rs`) -- SQLx-backed persistence, local avatar/attachment storage, libp2p, and WebRTC (`/home/runner/work/discool/discool/server/src`) +- Svelte SPA client served from the Rust backend (`client/src/main.ts`) +- Axum HTTP + WebSocket server (`server/src/handlers/mod.rs`) +- SQLx-backed persistence, local avatar/attachment storage, libp2p, and WebRTC (`server/src`) ## System Summary