Skip to content

Commit eb367e7

Browse files
committed
[SEC] restrict CORS to authorized extension IDs and migrate settings
Problem: 1. Security: Firefox extensions had a blanket wildcard permission (moz-extension://.*). 2. Bug: CORS settings were stored in the general 'settings.' namespace (settings.cors_origins), which would overwrite values from config.toml and didn't support regex properly (causing panics if regex was used). Solution: 1. Restricted CORS to authorized extension IDs and introduced a dedicated 'cors-config' API and 'cors.' namespace in the datastore. 2. Implemented a migration script that moves existing 'settings.cors_origins' values to 'cors.cors' on startup. 3. Updated the CORS middleware to handle multiple origin sources (TOML, datastore exact/regex matches, and extension shortcuts). 4. Ensured that fields defined in config.toml are respected and non-editable through the UI, preventing accidental overwrites. 5. Added validation for regex patterns to prevent server panics. Refactored the backend to use a dedicated CORS configuration model, allowing granular control over security settings. Related PRs: https://github.com/odoo/aw-webui/pull/2/changes
1 parent e1c6683 commit eb367e7

11 files changed

Lines changed: 296 additions & 76 deletions

File tree

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,5 @@ NDK
1010
*.sqlite*
1111
*.db
1212
*.db-journal
13+
14+
.vscode

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

README.md

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,33 @@ Available options:
6060

6161
# Additional regex CORS origins to allow (e.g. for sideloaded browser extensions)
6262
#cors_regex = ["chrome-extension://yourextensionidhere"]
63+
64+
# Allow official ActivityWatch Chrome extension? (default: true)
65+
#cors_allow_aw_chrome_extension = true
66+
67+
# Allow all Firefox extensions? (default: false, DANGEROUS)
68+
#cors_allow_all_mozilla_extension = false
6369
```
6470

71+
#### Persistence and Settings UI
72+
73+
The CORS-related settings (`cors`, `cors_regex`, `cors_allow_aw_chrome_extension`, and `cors_allow_all_mozilla_extension`) follow a precedence logic between the configuration file and the database:
74+
75+
- **TOML Precedence**: If a field is explicitly defined in your `config.toml`, it takes absolute precedence. The server will use the value from the file, and that setting will be **read-only** in the Web UI (marked as "Fixed in config file").
76+
- **Database Fallback**: If a field is **missing** or commented out in the `config.toml`, the server will look for it in the database. These can be managed and edited via the **Security & CORS** modal in the Settings page.
77+
- **Initial Setup**: On the first start, a default `config.toml` is created with all settings commented out, allowing the Web UI to take control of the configuration by default while providing a template for manual overrides.
78+
79+
> [!IMPORTANT]
80+
> **Server Restart Required**: Changing any CORS-related settings (whether via `config.toml` or the Web UI) requires stopping and restarting the server for the changes to take effect. These settings are loaded into memory once during the server's initialization and are not hot-reloadable.
81+
6582
#### Custom CORS Origins
6683

6784
By default, the server allows requests from:
6885
- The server's own origin (`http://127.0.0.1:<port>`, `http://localhost:<port>`)
69-
- The official Chrome extension (`chrome-extension://nglaklhklhcoonedhgnpgddginnjdadi`)
70-
- All Firefox extensions (`moz-extension://.*`)
86+
- The official Chrome extension (`chrome-extension://nglaklhklhcoonedhgnpgddginnjdadi`) if `cors_allow_aw_chrome_extension` is true (default).
87+
- All Firefox extensions (`moz-extension://.*`) ONLY IF `cors_allow_all_mozilla_extension` is set to true.
7188

72-
To allow additional origins (e.g. a sideloaded Chrome extension), add them to your config:
89+
To allow additional origins (e.g. a sideloaded Chrome extension), add them to your `cors` or `cors_regex` config:
7390

7491
```toml
7592
# Allow a specific sideloaded Chrome extension

aw-server/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ uuid = { version = "1.3", features = ["serde", "v4"] }
2929
clap = { version = "4.1", features = ["derive", "cargo"] }
3030
log-panics = { version = "2", features = ["with-backtrace"]}
3131
rust-embed = { version = "8.0.0", features = ["interpolate-folder-path", "debug-embed"] }
32+
regex = "1"
3233

3334
aw-datastore = { path = "../aw-datastore" }
3435
aw-models = { path = "../aw-models" }

aw-server/src/config.rs

Lines changed: 84 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,21 @@
1-
use std::fs::File;
2-
use std::io::{Read, Write};
1+
use std::collections::HashSet;
2+
use std::fs::{self, File};
3+
use std::io::Write;
34

45
use rocket::config::Config;
56
use rocket::data::{Limits, ToByteUnit};
67
use rocket::log::LogLevel;
78
use serde::{Deserialize, Serialize};
89

910
use crate::dirs;
11+
use serde_json;
12+
13+
pub const CORS_FIELDS: &[&str] = &[
14+
"cors",
15+
"cors_regex",
16+
"cors_allow_aw_chrome_extension",
17+
"cors_allow_all_mozilla_extension",
18+
];
1019

1120
// Far from an optimal way to solve it, but works and is simple
1221
static mut TESTING: bool = true;
@@ -19,7 +28,7 @@ pub fn is_testing() -> bool {
1928
unsafe { TESTING }
2029
}
2130

22-
#[derive(Serialize, Deserialize)]
31+
#[derive(Serialize, Deserialize, Clone)]
2332
pub struct AWConfig {
2433
#[serde(default = "default_address")]
2534
pub address: String,
@@ -36,6 +45,12 @@ pub struct AWConfig {
3645
#[serde(default = "default_cors")]
3746
pub cors_regex: Vec<String>,
3847

48+
#[serde(default = "default_true")]
49+
pub cors_allow_aw_chrome_extension: bool,
50+
51+
#[serde(default)]
52+
pub cors_allow_all_mozilla_extension: bool,
53+
3954
// A mapping of watcher names to paths where the
4055
// custom visualizations are located.
4156
#[serde(default = "default_custom_static")]
@@ -50,6 +65,8 @@ impl Default for AWConfig {
5065
testing: default_testing(),
5166
cors: default_cors(),
5267
cors_regex: default_cors(),
68+
cors_allow_aw_chrome_extension: default_true(),
69+
cors_allow_all_mozilla_extension: default_true(),
5370
custom_static: default_custom_static(),
5471
}
5572
}
@@ -91,6 +108,14 @@ fn default_testing() -> bool {
91108
is_testing()
92109
}
93110

111+
fn default_true() -> bool {
112+
true
113+
}
114+
115+
fn default_custom_static() -> std::collections::HashMap<String, String> {
116+
std::collections::HashMap::new()
117+
}
118+
94119
fn default_port() -> u16 {
95120
if is_testing() {
96121
5666
@@ -99,18 +124,40 @@ fn default_port() -> u16 {
99124
}
100125
}
101126

102-
fn default_custom_static() -> std::collections::HashMap<String, String> {
103-
std::collections::HashMap::new()
104-
}
105-
106-
pub fn create_config(testing: bool) -> AWConfig {
107-
set_testing(testing);
127+
pub fn get_config_path(testing: bool) -> (std::path::PathBuf, Vec<String>) {
108128
let mut config_path = dirs::get_config_dir().unwrap();
109129
if !testing {
110130
config_path.push("config.toml")
111131
} else {
112132
config_path.push("config-testing.toml")
113133
}
134+
if !config_path.is_file() {
135+
return (
136+
config_path,
137+
CORS_FIELDS.iter().map(|f| f.to_string()).collect(),
138+
);
139+
}
140+
let content = fs::read_to_string(&config_path).unwrap_or_default();
141+
let toml_value: toml::Value =
142+
toml::from_str(&content).unwrap_or_else(|_| toml::Value::Table(toml::Table::new()));
143+
144+
let file_keys: HashSet<String> = toml_value
145+
.as_table()
146+
.map(|t| t.keys().cloned().collect())
147+
.unwrap_or_default();
148+
149+
let missing = CORS_FIELDS
150+
.iter()
151+
.filter(|f| !file_keys.contains(&f.to_string()))
152+
.map(|f| f.to_string())
153+
.collect();
154+
155+
(config_path, missing)
156+
}
157+
158+
pub fn create_config(testing: bool, datastore: &aw_datastore::Datastore) -> AWConfig {
159+
set_testing(testing);
160+
let (config_path, missing_cors_fields) = get_config_path(testing);
114161

115162
/* If there is no config file, create a new config file with default values but every value is
116163
* commented out by default in case we would change a default value at some point in the future */
@@ -132,12 +179,34 @@ pub fn create_config(testing: bool) -> AWConfig {
132179
}
133180

134181
debug!("Reading config at {:?}", config_path);
135-
let mut rfile = File::open(config_path).expect("Failed to open config file for reading");
136-
let mut content = String::new();
137-
rfile
138-
.read_to_string(&mut content)
139-
.expect("Failed to read config as a string");
140-
let aw_config: AWConfig = toml::from_str(&content).expect("Failed to parse config file");
182+
let content = fs::read_to_string(config_path).expect("Failed to read config file");
183+
let toml_value: toml::Value = toml::from_str(&content).expect("Failed to parse config file");
184+
185+
let mut aw_config: AWConfig =
186+
toml_value.try_into().expect("Failed to convert TOML value to AWConfig");
187+
188+
// Migration: if settings.cors_origins exists, move it to cors.cors
189+
if let Ok(value_str) = datastore.get_key_value("settings.cors_origins") {
190+
info!("Migrating settings.cors_origins to cors.cors");
191+
if datastore.get_key_value("cors.cors").is_err() {
192+
if let Ok(_) = datastore.set_key_value("cors.cors", &value_str) {
193+
let _ = datastore.delete_key_value("settings.cors_origins");
194+
}
195+
} else {
196+
let _ = datastore.delete_key_value("settings.cors_origins");
197+
}
198+
}
141199

200+
for field in missing_cors_fields {
201+
let Ok(value_str) = datastore.get_key_value(&format!("cors.{field}")) else { continue };
202+
203+
match field.as_str() {
204+
"cors" => aw_config.cors = serde_json::from_str(&value_str).unwrap_or_default(),
205+
"cors_regex" => aw_config.cors_regex = serde_json::from_str(&value_str).unwrap_or_default(),
206+
"cors_allow_aw_chrome_extension" => aw_config.cors_allow_aw_chrome_extension = serde_json::from_str(&value_str).unwrap_or_default(),
207+
"cors_allow_all_mozilla_extension" => aw_config.cors_allow_all_mozilla_extension = serde_json::from_str(&value_str).unwrap_or_default(),
208+
_ => {}
209+
}
210+
}
142211
aw_config
143212
}

aw-server/src/endpoints/cors.rs

Lines changed: 32 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,31 @@
11
use rocket::http::Method;
22
use rocket_cors::{AllowedHeaders, AllowedOrigins};
3-
use aw_datastore::Datastore;
4-
use std::sync::Mutex;
53

64
use crate::config::AWConfig;
75

8-
pub fn cors(config: &AWConfig, datastore_mutex: &Mutex<Datastore>) -> rocket_cors::Cors {
6+
pub fn cors(config: &AWConfig) -> rocket_cors::Cors {
97
let root_url = format!("http://127.0.0.1:{}", config.port);
108
let root_url_localhost = format!("http://localhost:{}", config.port);
11-
let mut allowed_exact_origins = vec![root_url, root_url_localhost];
9+
let mut allowed_exact_origins = vec![root_url.clone(), root_url_localhost.clone()];
1210
allowed_exact_origins.extend(config.cors.clone());
1311

14-
let db = datastore_mutex.lock().unwrap();
15-
if let Ok(cors_origins_str) = db.get_key_value("settings.cors_origins") {
12+
let mut allowed_regex_origins = config.cors_regex.clone();
1613

17-
let cors_origins: Vec<String> = cors_origins_str
18-
.trim_matches('"')
19-
.split(',')
20-
.map(|s| s.trim().to_string())
21-
.filter(|s| !s.is_empty())
22-
.filter(|s| {
23-
let is_valid = s.starts_with("http://")
24-
|| s.starts_with("https://");
25-
if !is_valid {
26-
log::warn!("Ignoring invalid CORS origin: '{}'", s);
27-
}
28-
is_valid
29-
})
30-
.collect();
31-
info!("Parsed cors_origins from settings: {:?}", cors_origins);
32-
allowed_exact_origins.extend(cors_origins);
14+
if config.cors_allow_aw_chrome_extension {
15+
allowed_regex_origins.push("chrome-extension://nglaklhklhcoonedhgnpgddginnjdadi".to_string());
3316
}
34-
drop(db);
3517

36-
if config.testing {
37-
allowed_exact_origins.push("http://127.0.0.1:27180".to_string());
38-
allowed_exact_origins.push("http://localhost:27180".to_string());
39-
}
40-
let mut allowed_regex_origins = vec![
41-
"chrome-extension://nglaklhklhcoonedhgnpgddginnjdadi".to_string(),
18+
if config.cors_allow_all_mozilla_extension {
4219
// Every version of a mozilla extension has its own ID to avoid fingerprinting, so we
4320
// unfortunately have to allow all extensions to have access to aw-server
44-
"moz-extension://.*".to_string(),
45-
];
46-
allowed_regex_origins.extend(config.cors_regex.clone());
21+
allowed_regex_origins.push("moz-extension://.*".to_string());
22+
}
23+
4724
if config.testing {
25+
allowed_exact_origins.extend(vec![
26+
"http://127.0.0.1:27180".to_string(),
27+
"http://localhost:27180".to_string(),
28+
]);
4829
allowed_regex_origins.push("chrome-extension://.*".to_string());
4930
}
5031

@@ -56,13 +37,29 @@ pub fn cors(config: &AWConfig, datastore_mutex: &Mutex<Datastore>) -> rocket_cor
5637
let allowed_headers = AllowedHeaders::all(); // TODO: is this unsafe?
5738

5839
// You can also deserialize this
59-
rocket_cors::CorsOptions {
40+
let cors_options = rocket_cors::CorsOptions {
6041
allowed_origins,
6142
allowed_methods,
6243
allowed_headers,
6344
allow_credentials: false,
6445
..Default::default()
46+
};
47+
48+
match cors_options.to_cors() {
49+
Ok(cors) => cors,
50+
Err(e) => {
51+
error!("Failed to set up CORS with provided origins: {:?}", e);
52+
error!("Exact origins: {:?}", allowed_exact_origins);
53+
error!("Regex origins: {:?}", allowed_regex_origins);
54+
// Fallback to a safe default to allow the server to at least start
55+
let fallback_origins = vec![root_url, root_url_localhost];
56+
let empty_regex: &[String] = &[];
57+
rocket_cors::CorsOptions {
58+
allowed_origins: AllowedOrigins::some(&fallback_origins, empty_regex),
59+
..Default::default()
60+
}
61+
.to_cors()
62+
.expect("Safe default CORS should always work")
63+
}
6564
}
66-
.to_cors()
67-
.expect("Failed to set up CORS")
6865
}

0 commit comments

Comments
 (0)