Skip to main content

max / goingson

Security remediation: fix stored XSS in email labels, harden the URL opener Ultra-fuzz Run #28 Phase 2. - CRITICAL C1: email label names broke out of the reader's onclick/value attributes (escapeAttr backslash-escapes for a JS-string context, not HTML). Add escapeAttrValue (entity-encodes & and ") and use it for the label JSON in the dropdown handler and the edit-labels input value. - Drop the JS shell:allow-open permission; route external URLs through a new open_external_url command that rejects any non-http(s) scheme (file:, javascript:, data:, custom handlers). OAuth opening goes through it, with a window.open fallback on mobile. A frontend scripting bug can no longer reach the system opener with an attacker-chosen target. - Delete the dead migrate_from_database (no callers; credentials are keychain-only on current installs). - Document the phantom plugin `network` capability so a future host networking function can't silently activate already-installed plugins without re-consent. NEEDS EYEBALL: confirm OAuth/sync login still opens the browser (untestable here). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-19 19:48 UTC
Commit: dcf3f5f4ec25856c0884e909da6af428817e5f3b
Parent: 916c040
9 files changed, +112 insertions, -56 deletions
M crates/plugin-runtime/src/engine.rs +13 -1
@@ -63,7 +63,19 @@ impl PluginEngine {
63 63 // Disable dangerous operations
64 64 engine.disable_symbol("eval");
65 65
66 - // Register the goingson:: API module
66 + // Register the goingson:: API module.
67 + //
68 + // This is the ONLY host surface exposed to plugins: pure data helpers, no
69 + // filesystem, no network, no DB. A plugin's manifest may declare a
70 + // `network` capability, but nothing here backs it — it grants nothing
71 + // today (fail-closed by omission, ultra-fuzz Run #28 surprise).
72 + //
73 + // WARNING for future host functions: if you ever register a networking or
74 + // filesystem capability here, gate it on the plugin's manifest capability
75 + // AND force a fresh consent prompt. The reload escalation guard only fires
76 + // when a manifest *changes*; it will NOT re-prompt for an already-installed
77 + // `network = true` plugin when the host newly gains the capability, so
78 + // adding the host function alone would silently activate it.
67 79 let goingson_module = create_goingson_module();
68 80 engine.register_static_module("goingson", Arc::new(goingson_module));
69 81
M src-tauri/capabilities/desktop.json +1 -2
@@ -1,11 +1,10 @@
1 1 {
2 2 "$schema": "https://schemas.tauri.app/capabilities/2",
3 3 "identifier": "desktop",
4 - "description": "Desktop-only capabilities (shell, notifications, OTA updater)",
4 + "description": "Desktop-only capabilities (notifications, OTA updater). External URLs are opened via the validated open_external_url command, not a JS shell binding.",
5 5 "platforms": ["linux", "macOS", "windows"],
6 6 "windows": ["main", "compose-*"],
7 7 "permissions": [
8 - "shell:allow-open",
9 8 "notification:default",
10 9 "updater:default",
11 10 "process:allow-restart"
M src-tauri/frontend/js/api.js +1
@@ -377,6 +377,7 @@ const api = {
377 377 setTitle: (title) => invoke('set_window_title', { title }),
378 378 openCompose: (context) => invoke('open_compose_window', { context: context || null }), // Separate compose window, optional reply context
379 379 openEmailInBrowser: (id) => invoke('open_email_in_browser', { id }), // Render HTML email to temp file → open
380 + openExternal: (url) => invoke('open_external_url', { url }), // Validated http(s) opener; JS shell.open is not granted
380 381 },
381 382 };
382 383
M src-tauri/frontend/js/emails.js +3 -2
@@ -10,6 +10,7 @@
10 10 'use strict';
11 11 const esc = GoingsOn.utils.escapeHtml;
12 12 const escAttr = GoingsOn.utils.escapeAttr;
13 + const escAttrVal = GoingsOn.utils.escapeAttrValue;
13 14
14 15 // ============ Email Selection & Pagination ============
15 16
@@ -489,7 +490,7 @@
489 490 <button class="dropdown-item" onclick="GoingsOn.emails.createEventFromEmail('${escAttr(latestEmail.id)}'); this.parentElement.classList.remove('show');">
490 491 Convert to Event
491 492 </button>
492 - <button class="dropdown-item" onclick="GoingsOn.emails.editLabels('${escAttr(latestEmail.id)}', ${escAttr(JSON.stringify(latestEmail.labels || []))}); this.parentElement.classList.remove('show');">
493 + <button class="dropdown-item" onclick="GoingsOn.emails.editLabels('${escAttr(latestEmail.id)}', ${escAttrVal(JSON.stringify(latestEmail.labels || []))}); this.parentElement.classList.remove('show');">
493 494 Edit Labels
494 495 </button>
495 496 <button class="dropdown-item" onclick="GoingsOn.emails.moveToFolder('${escAttr(latestEmail.id)}'); this.parentElement.classList.remove('show');">
@@ -1078,7 +1079,7 @@
1078 1079 <form id="label-form" onsubmit="event.preventDefault(); GoingsOn.emails._saveLabels('${escAttr(emailId)}');">
1079 1080 <div class="form-group">
1080 1081 <label class="form-label">Labels (comma-separated)</label>
1081 - <input type="text" class="form-input" id="label-input" value="${escAttr((currentLabels || []).join(', '))}"
1082 + <input type="text" class="form-input" id="label-input" value="${escAttrVal((currentLabels || []).join(', '))}"
1082 1083 placeholder="work, important, follow-up" autofocus>
1083 1084 ${existing.length > 0 ? `<div class="label-existing-line">Existing: ${existing.map(l => esc(l)).join(', ')}</div>` : ''}
1084 1085 </div>
M src-tauri/frontend/js/settings-sync.js +6 -4
@@ -219,10 +219,12 @@
219 219 GoingsOn.ui.showToast('Starting authentication...', 'info');
220 220 const authData = await GoingsOn.api.sync.startAuth();
221 221
222 - // Open browser
223 - if (window.__TAURI__?.shell?.open) {
224 - await window.__TAURI__.shell.open(authData.authUrl);
225 - } else {
222 + // Open the auth URL via the validated Rust opener (the JS shell.open
223 + // permission is intentionally not granted). Fall back to the webview
224 + // on mobile / if the command is unavailable.
225 + try {
226 + await GoingsOn.api.window.openExternal(authData.authUrl);
227 + } catch (e) {
226 228 window.open(authData.authUrl, '_blank');
227 229 }
228 230
M src-tauri/frontend/js/utils.js +20
@@ -36,6 +36,25 @@ function escapeAttr(str) {
36 36 }
37 37
38 38 /**
39 + * Escape a string for placement inside a double-quoted HTML attribute value.
40 + *
41 + * Unlike `escapeAttr` (which backslash-escapes for a JS-string context) and
42 + * `escapeHtml` (which, via textContent serialization, does NOT encode `"`), this
43 + * encodes `&` and `"` to HTML entities so the value cannot terminate the
44 + * attribute. Use it for any attacker-influenced content going into `value="..."`,
45 + * `title="..."`, a `data-*` attribute, or a JSON literal embedded in an inline
46 + * handler — the HTML parser decodes the entities before the value (or handler JS)
47 + * is used, so the original text/JSON is recovered intact. (ultra-fuzz Run #28 C1.)
48 + * @param {string} str
49 + * @returns {string}
50 + */
51 + function escapeAttrValue(str) {
52 + return String(str ?? '')
53 + .replace(/&/g, '&amp;')
54 + .replace(/"/g, '&quot;');
55 + }
56 +
57 + /**
39 58 * Human-friendly prefixes for machine-readable API error codes.
40 59 * Backend sends structured ApiError { code, message, details }.
41 60 */
@@ -627,6 +646,7 @@ GoingsOn.utils = {
627 646 // HTML escaping
628 647 escapeHtml,
629 648 escapeAttr,
649 + escapeAttrValue,
630 650 getErrorMessage,
631 651 showError,
632 652
M src-tauri/src/commands/window.rs +67
@@ -124,3 +124,70 @@ pub async fn set_window_title(
124 124
125 125 Ok(())
126 126 }
127 +
128 + /// Open an external URL in the user's default browser, after validating it is an
129 + /// `http(s)` URL.
130 + ///
131 + /// This is the only sanctioned path from the frontend to the system opener. The
132 + /// JS `shell:allow-open` permission is intentionally NOT granted, so a frontend
133 + /// scripting bug (e.g. an XSS in rendered email content) cannot reach the opener
134 + /// with an attacker-chosen target — every call funnels through this scheme check
135 + /// (ultra-fuzz Run #28 Security MINOR-1). On mobile the caller falls back to a
136 + /// normal `window.open`.
137 + #[tauri::command]
138 + #[instrument(skip_all)]
139 + pub async fn open_external_url(
140 + #[allow(unused_variables)] app: tauri::AppHandle,
141 + url: String,
142 + ) -> Result<(), ApiError> {
143 + let trimmed = url.trim();
144 + if !is_external_http_url(trimmed) {
145 + return Err(ApiError::bad_request("Only http(s) URLs can be opened externally"));
146 + }
147 + #[cfg(not(any(target_os = "ios", target_os = "android")))]
148 + {
149 + use tauri_plugin_shell::ShellExt;
150 + app.shell()
151 + .open(trimmed.to_string(), None)
152 + .map_api_err("Failed to open URL", ApiError::internal)?;
153 + }
154 + Ok(())
155 + }
156 +
157 + /// Whether `url` is an `http`/`https` URL safe to hand to the system opener.
158 + /// Rejects every other scheme (`file:`, `javascript:`, `data:`, custom-handler
159 + /// schemes) and scheme-relative or path inputs.
160 + fn is_external_http_url(url: &str) -> bool {
161 + let lower = url.to_ascii_lowercase();
162 + lower.starts_with("https://") || lower.starts_with("http://")
163 + }
164 +
165 + #[cfg(test)]
166 + mod tests {
167 + use super::is_external_http_url;
168 +
169 + #[test]
170 + fn accepts_http_and_https() {
171 + assert!(is_external_http_url("https://accounts.google.com/o/oauth2/auth?x=1"));
172 + assert!(is_external_http_url("http://localhost:1421/callback"));
173 + assert!(is_external_http_url("HTTPS://EXAMPLE.COM"));
174 + }
175 +
176 + #[test]
177 + fn rejects_dangerous_and_non_http_schemes() {
178 + for bad in [
179 + "file:///etc/passwd",
180 + "javascript:alert(1)",
181 + "data:text/html,<script>alert(1)</script>",
182 + "mailto:a@b.com",
183 + "tel:+15551234",
184 + "smb://host/share",
185 + "/etc/passwd",
186 + "//evil.com",
187 + "ftp://host/x",
188 + "",
189 + ] {
190 + assert!(!is_external_http_url(bad), "should reject {bad:?}");
191 + }
192 + }
193 + }
M src-tauri/src/lib.rs +1
@@ -194,6 +194,7 @@ macro_rules! all_commands {
194 194 // Window
195 195 $crate::commands::open_compose_window,
196 196 $crate::commands::set_window_title,
197 + $crate::commands::open_external_url,
197 198 // Search
198 199 $crate::commands::search,
199 200 // Daily Notes
M src-tauri/src/oauth/credentials.rs -47
@@ -370,53 +370,6 @@ impl CredentialStore {
370 370 }
371 371 }
372 372 }
373 -
374 - /// Migrate credentials from database to keychain.
375 - ///
376 - /// Call this on startup to move any plaintext credentials
377 - /// from SQLite to the secure keychain.
378 - ///
379 - /// # Arguments
380 - /// * `account_id` - The email account UUID
381 - /// * `oauth_access` - Access token from database (if any)
382 - /// * `oauth_refresh` - Refresh token from database (if any)
383 - /// * `password` - Password from database (if any)
384 - ///
385 - /// # Returns
386 - /// true if any credentials were migrated
387 - pub fn migrate_from_database(
388 - account_id: Uuid,
389 - oauth_access: Option<&str>,
390 - oauth_refresh: Option<&str>,
391 - password: Option<&str>,
392 - ) -> bool {
393 - let mut migrated = false;
394 -
395 - // Migrate OAuth tokens
396 - if let Some(access) = oauth_access {
397 - if !access.is_empty() && Self::get_oauth(account_id).is_none() {
398 - let creds = OAuthCredentials {
399 - access_token: access.to_string(),
400 - refresh_token: oauth_refresh.map(String::from),
401 - };
402 - if Self::store_oauth(account_id, &creds).is_ok() {
403 - debug!("Migrated OAuth credentials for account {}", account_id);
404 - migrated = true;
405 - }
406 - }
407 - }
408 -
409 - // Migrate password
410 - if let Some(pwd) = password {
411 - if !pwd.is_empty() && Self::get_password(account_id).is_none()
412 - && Self::store_password(account_id, pwd).is_ok() {
413 - debug!("Migrated password for account {}", account_id);
414 - migrated = true;
415 - }
416 - }
417 -
418 - migrated
419 - }
420 373 }
421 374
422 375 #[cfg(test)]