[SEC] restrict CORS to authorized extension IDs

[RaoufGhrissi]

[SEC] restrict CORS to authorized extension IDs
Fixes a security issue where any Firefox extension (moz-extension://.*) could access the ActivityWatch server without any restriction.

Previously, the CORS configuration included a wildcard for all Mozilla extensions by default. This commit removes that blanket permission and introduces granular control through both static configuration and the Web UI.

We've added 2 new fields to the file configuration (allow_aw_chrome_extension and allow_all_mozilla_extension) and 4 new settings to the Web UI (Fixed origins, Regex origins, and extension-specific shortcuts). The server now merges these settings to determine the final set of authorized origins, ensuring a more secure and flexible configuration.

Dependent on: ActivityWatch/aw-webui#795

[greptile-apps]

Greptile Summary

This PR replaces the blanket moz-extension://.* CORS wildcard with two explicit flags (cors_allow_aw_chrome_extension, cors_allow_all_mozilla_extension) and adds a new /api/0/cors-config endpoint for managing these settings via the Web UI, backed by datastore persistence. Alongside the CORS work, aw-datastore/src/worker.rs gains self.commit = true on SetKeyValue and DeleteKeyValue — a fix that was previously preventing all key-value writes (including the existing /api/0/settings API) from being committed to disk across restarts.

Confidence Score: 4/5

Safe to merge with minor validation gap; all previously raised P1 issues appear addressed.

All major issues from prior review threads (wrong serde default for mozilla extension, |= write-once logic, missing in-memory config update, testing-mode regex in wrong list, trim_matches JSON parsing) are fixed in this revision. The only new findings are P2: the cors exact-origin list accepts entries without http/https validation (silently dropped at runtime), and the worker.rs commit fix has broader scope than CORS alone. Neither blocks correctness of the security feature itself.

aw-server/src/endpoints/cors_config.rs — cors list validation before DB write; aw-datastore/src/worker.rs — verify no existing tests relied on non-persistence of key-value writes.

Vulnerabilities

• CORS misconfiguration via stored invalid origins (cors_config.rs): cors list entries are stored without http/https scheme validation; at startup they are silently dropped, meaning a user who mistakenly adds an extension origin to the wrong field gets no feedback and the intended restriction has no effect.
• Regex patterns as CORS allow-list (cors.rs, cors_config.rs): cors_regex values from the DB are passed directly to rocket_cors as regex patterns. Validation guards against syntax errors, but a permissive pattern (e.g. .*) is accepted and would bypass CORS for all origins. This is by design (admin-controlled), but the Web UI should surface a clear warning when a pattern is broad.
• No secrets exposure, SQL injection, or auth-boundary issues identified in the changed code.

Important Files Changed

Filename	Overview
aw-server/src/endpoints/cors.rs	Rewrites CORS fairing builder to honor the new cors_allow_aw_chrome_extension and cors_allow_all_mozilla_extension flags with a safe fallback on regex compilation failure; testing mode correctly pushes the Chrome wildcard to the regex list.
aw-server/src/endpoints/cors_config.rs	New GET/POST endpoints for CORS settings; cors_regex patterns are validated before saving, but cors exact-origin entries are stored without scheme validation and will be silently dropped at startup if they lack http/https.
aw-server/src/config.rs	Adds cors_allow_aw_chrome_extension (default true) and cors_allow_all_mozilla_extension (default false via default_false()), CORS field list, and DB-backed override logic in create_config; defaults and serialization are correct.
aw-datastore/src/worker.rs	Adds self.commit = true to SetKeyValue and DeleteKeyValue — fixing a pre-existing bug where all key-value writes (including the settings API) were never committed to disk; broader than the CORS scope but clearly correct.
aw-server/src/endpoints/mod.rs	Adds config: Mutex to ServerState, mounts the new /api/0/cors-config routes, and wires up the CORS endpoint module.
aw-server/src/main.rs	Reorders startup so the datastore is created before create_config (needed to load DB-persisted CORS overrides), then passes config.clone() into ServerState; logic is correct.
README.md	Documents the two new config flags, precedence rules (TOML vs DB), Web UI editability, and the restart-required notice for CORS changes.

Sequence Diagram

sequenceDiagram
    participant Client as Web UI / Extension
    participant Rocket as Rocket Server
    participant CorsConfig as /api/0/cors-config
    participant State as state.config (Mutex)
    participant DB as Datastore (SQLite)
    participant Fairing as CORS Fairing (snapshot)

    Note over Rocket,Fairing: Startup
    DB-->>Rocket: load persisted cors.* keys
    Rocket->>State: AWConfig (with DB overrides)
    Rocket->>Fairing: cors::cors(&config) snapshot baked in

    Note over Client,DB: GET /api/0/cors-config
    Client->>CorsConfig: GET
    CorsConfig->>State: lock, read AWConfig
    CorsConfig->>CorsConfig: read TOML to find in_file fields
    CorsConfig-->>Client: cors, cors_regex, flags, in_file, needs_restart true

    Note over Client,DB: POST /api/0/cors-config
    Client->>CorsConfig: POST cors, cors_regex, flags
    CorsConfig->>State: lock, get testing flag
    CorsConfig->>CorsConfig: validate cors_regex patterns
    CorsConfig->>DB: set_key_value cors.* for each missing field
    Note over DB: self.commit=true, transaction committed
    CorsConfig->>State: lock, update in-memory AWConfig
    CorsConfig-->>Client: 200 OK

    Note over Fairing: Fairing snapshot NOT updated, restart required
    Client->>Rocket: restart server
    DB-->>Rocket: reload persisted cors.* keys
    Rocket->>Fairing: new cors::cors with updated values

Loading

_{Reviews (7): Last reviewed commit: "[SEC] restrict CORS to authorized extens..." | Re-trigger Greptile}

[greptile-apps]

Fragile JSON string unwrapping via trim_matches

The settings API stores all values JSON-encoded (see settings.rs line 72: serde_json::to_string(&value.0)). So a string like "moz-extension://abc" is persisted in the database as "\"moz-extension://abc\"".

trim_matches('"') strips surrounding double-quote characters, which works for simple ASCII strings, but is not a correct JSON string deserializer. It will silently produce wrong results for values that contain escaped characters. It also won't handle the case where the user stores a JSON array (e.g., ["ext1", "ext2"]) instead of a comma-separated string.

The idiomatic fix is to use serde_json::from_str::<String>(&s):

let parse_cors_list = |key: &str| -> Vec<String> {
    db.get_key_value(key)
        .ok()
        .and_then(|s| serde_json::from_str::<String>(&s).ok())
        .map(|s| {
            s.split(',')
                .map(|s| s.trim().to_string())
                .filter(|s| !s.is_empty())
                .collect()
        })
        .unwrap_or_default()
};

[greptile-apps]

Firefox extension support is effectively broken for all existing users

The original code included the comment:

Every version of a mozilla extension has its own ID to avoid fingerprinting, so we unfortunately have to allow all extensions to have access to aw-server

This is a fundamental Firefox security property: each installation (and each update) of a Firefox extension gets a new random UUID as its origin (moz-extension://<uuid>). This means:

1. There is no single stable ID that a user can add to the allow-list for the ActivityWatch Firefox extension.
2. After every extension update, the old ID is invalidated and the user would need to find their new UUID and update the server settings.
3. Most users have no easy way to discover their current moz-extension:// UUID.

The PR removes moz-extension://.* without providing any workable alternative for Firefox users. Consider either:

• Keeping moz-extension://.* as the default (preserving existing behavior) with clear documentation about the risk, or
• Using a permanent addon ID via browser_specific_settings.gecko.id in manifest.json and documenting that users of unofficial builds need the wildcard.

[ErikBjare]

Don't pass datastore here

[RaoufGhrissi]

i'm hesitating to set default value to True, so it doesn't impact the actual users

[ErikBjare]

Not a fan of this many additional config keys. Should be prefixed consistently with cors_ such as cors_allow_aw_chrome_extension and cors_allow_all_mozilla_extension.

Curious why the _from_settings is passed along and not resolved/merged into the non-from_settings variants?

[RaoufGhrissi]

merged with the existing vars, and explained in the readme file

[ErikBjare]

I'm a little bit paranoid about letting web UI set CORS like that...

[RaoufGhrissi]

do you have another suggestion to let non-devs configure cors ?

[ErikBjare]

This seems problematic. It's not acceptable to have file-level config options being ignored by auto-set db-values. Makes the config file mostly useless/unreliable. Not really any "sync" happening here.

imo, order of settings/config precedence in server should be cli options -> env vars -> config file -> webui/api settings -> defaults.

One way to resolve this would be to make the webui config write directly to the config file, would effectively put them in sync (but still not a fan of exposing server config like this in api/webui tbh). But that might require a API endpoint different from /settings, maybe /config - but really unsure about this, just an idea.

Simplest would probably be to just prefer the config over the db settings and disable the webui settings if they are explicitly set in config/env/cli.

[RaoufGhrissi]

Thank you for the detailed feedback. I completely agree with the proposed precedence order (CLI > Env > Config File > WebUI/API > Defaults), and I have implemented the changes to reflect exactly this logic:

Config File Priority: the configuration file now takes absolute precedence over the database. If a CORS field is explicitly defined in config.toml, the server uses that value and ignores any existing database overrides for that specific field.
for CLI/Env: I verified that cors settings are not currently managed via CLI or Env variables in aw-server-rust. However, the current architecture is future-proof: since the database fallback only triggers for fields missing from the static configuration, any future addition of CLI/Env overrides would naturally take precedence like port and address.

UI Feedback & Safety: To address the concern about "unreliable" config, the Web UI now detects which fields are defined in the config.toml. These fields are marked in the UI as "Fixed in config file" (highlighted in orange) and the inputs are disabled. This ensures the user is aware that the file-level config is in control and cannot be bypassed via the API.

[greptile-apps]

Tip:

Greploop — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.

Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.

[greptile-apps]

CORS fairing is immutable at runtime — saved changes have no effect until restart

The rocket_cors::Cors fairing is built once in build_rocket (cors::cors(&config)) and attached via .attach(cors.clone()). Rocket fairings are not hot-reloadable; the fairing holds a snapshot of the allowed-origins set and does not re-read state.config on each request.

Consequently, any CORS rule saved through this endpoint takes effect only after the server restarts. Neither the API response nor the Web UI currently communicates this to the user.

At a minimum the API should return an informational flag (e.g. "restart_required": true) and the README's "Persistence and Settings UI" section should explicitly state that the server must be restarted for CORS changes to become active.