Remove Hardcoded OAuth Secret, Enforce JWT Secret Validation, and Add Challenge Expiry/Rate Limiting

config/default.toml

@@ -1,43 +1,47 @@
-# TaskMaster Configuration
-
-[server]
-host = "127.0.0.1"
-port = 3000
-cors_allowed_origins = ["http://localhost:4321"]
-
-[candidates]
-# GraphQL endpoint to fetch candidate addresses
-graphql_url = "https://subsquid.quantus.com/graphql"
-
-[data]
-# Database configuration
-database_url = "postgres://postgres:postgres@127.0.0.1:55432/task_master"
-
-[logging]
-# Log level: error, warn, info, debug, trace
-level = "info"
-
-[jwt]
-admin_secret = "this-should-be-overriden"
-exp_in_hours = 24
-secret = "this-should-be-overriden"
-
-[x_oauth]
-callback_url = "http://localhost:3000/api/auth/x/callback"
-client_id = "WlVrcm4xSEpXQ2l3TURFM3lLZnE6MTpjaQ"
-client_secret = "lfXc45dZLqYTzP62Ms32EhXinGQzxcIP9TvjJml2B-h0T1nIJK"
-
-[remote_configs]
-wallet_configs_file = "../wallet_configs/default_configs.json"
-
-[risk_checker]
-etherscan_api_key = "change-me"
-etherscan_base_url = "https://api.etherscan.io/v2/api?chainid=1"
-infura_api_key = "change-me"
-infura_base_url = "https://mainnet.infura.io/v3"
-etherscan_calls_per_sec = 3
-max_concurrent_requests = 1
-
-[exchange_rate]
-# https://www.exchangerate-api.com/ — v6 key for latest/{base} rates
-api_key = "change-me"
+# TaskMaster Configuration
+
+[server]
+host = "127.0.0.1"
+port = 3000
+cors_allowed_origins = ["http://localhost:4321"]
+
+[candidates]
+# GraphQL endpoint to fetch candidate addresses
+graphql_url = "https://subsquid.quantus.com/graphql"
+
+[data]
+# Database configuration
+database_url = "postgres://postgres:postgres@127.0.0.1:55432/task_master"
+
+[logging]
+# Log level: error, warn, info, debug, trace
+level = "info"
+
+[jwt]
+admin_secret = "this-should-be-overriden"
+exp_in_hours = 24
+secret = "this-should-be-overriden"
+
+[x_oauth]
+callback_url = "http://localhost:3000/api/auth/x/callback"
+# OAuth credentials must be provided per environment (env vars or a
+# git-ignored config file). Never commit real credentials here.
+# SECURITY: a live client secret was previously committed in this file and
+# must be rotated with X (Twitter) before the history purge completes.
+client_id = "replace-me"
+client_secret = "replace-me"
+
+[remote_configs]
+wallet_configs_file = "../wallet_configs/default_configs.json"
+
+[risk_checker]
+etherscan_api_key = "change-me"
+etherscan_base_url = "https://api.etherscan.io/v2/api?chainid=1"
+infura_api_key = "change-me"
+infura_base_url = "https://mainnet.infura.io/v3"
+etherscan_calls_per_sec = 3
+max_concurrent_requests = 1
+
+[exchange_rate]
+# https://www.exchangerate-api.com/ — v6 key for latest/{base} rates
+api_key = "change-me"

src/config.rs

@@ -1,131 +1,155 @@
-use std::path::Path;
-
-use axum::http::HeaderValue;
-use rusx::config::OauthConfig;
-use serde::{Deserialize, Serialize};
-
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct Config {
-    pub server: ServerConfig,
-    pub candidates: CandidatesConfig,
-    pub data: DataConfig,
-    pub logging: LoggingConfig,
-    pub jwt: JwtConfig,
-    pub x_oauth: OauthConfig,
-    pub remote_configs: RemoteConfigsConfig,
-    pub risk_checker: RiskCheckerConfig,
-    pub exchange_rate: ExchangeRateConfig,
-}
-
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct RemoteConfigsConfig {
-    pub wallet_configs_file: String,
-}
-
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct ServerConfig {
-    pub host: String,
-    pub port: u16,
-    pub cors_allowed_origins: Vec<String>,
-}
-
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct CandidatesConfig {
-    pub graphql_url: String,
-}
-
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct DataConfig {
-    pub database_url: String,
-}
-
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct LoggingConfig {
-    pub level: String,
-}
-
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct JwtConfig {
-    pub secret: String,
-    pub admin_secret: String,
-    pub exp_in_hours: i64,
-}
-
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct RiskCheckerConfig {
-    pub etherscan_api_key: String,
-    pub etherscan_base_url: String,
-    pub infura_api_key: String,
-    pub infura_base_url: String,
-    pub etherscan_calls_per_sec: u32,
-    pub max_concurrent_requests: usize,
-}
-
-/// Exchange rate API (e.g. [ExchangeRate-API v6](https://www.exchangerate-api.com/)).
-#[derive(Debug, Clone, Serialize, Deserialize)]
-pub struct ExchangeRateConfig {
-    pub api_key: String,
-}
-
-impl Config {
-    pub fn load(config_path: &str) -> Result<Self, config::ConfigError> {
-        let settings = config::Config::builder()
-            .add_source(config::File::new(config_path, config::FileFormat::Toml))
-            .add_source(config::Environment::with_prefix("TASKMASTER"))
-            .build()?;
-
-        let mut config: Self = settings.try_deserialize()?;
-        config.resolve_relative_paths(config_path);
-        Ok(config)
-    }
-
-    #[cfg(test)]
-    pub fn load_test_env() -> Result<Self, config::ConfigError> {
-        let test_config_path = "config/test.toml";
-        let settings = config::Config::builder()
-            // Load the test-specific configuration file
-            .add_source(config::File::new(test_config_path, config::FileFormat::Toml))
-            // You can still layer environment variables for testing if you need to
-            .add_source(config::Environment::with_prefix("TASKMASTER"))
-            .build()?;
-
-        let mut config: Self = settings.try_deserialize()?;
-        config.resolve_relative_paths(test_config_path);
-        Ok(config)
-    }
-
-    pub fn get_database_url(&self) -> &str {
-        &self.data.database_url
-    }
-
-    pub fn get_server_address(&self) -> String {
-        format!("{}:{}", self.server.host, self.server.port)
-    }
-
-    pub fn get_jwt_expiration(&self) -> chrono::Duration {
-        chrono::Duration::hours(self.jwt.exp_in_hours)
-    }
-
-    pub fn get_cors_allowed_origins(&self) -> Vec<HeaderValue> {
-        self.server
-            .cors_allowed_origins
-            .iter()
-            .filter_map(|o| match o.parse() {
-                Ok(v) => Some(v),
-                Err(e) => {
-                    tracing::warn!("Skipping invalid CORS origin {:?}: {}", o, e);
-                    None
-                }
-            })
-            .collect()
-    }
-
-    fn resolve_relative_paths(&mut self, config_path: &str) {
-        let wallet_configs_path = Path::new(&self.remote_configs.wallet_configs_file);
-        if wallet_configs_path.is_absolute() {
-            return;
-        }
-        let base_dir = Path::new(config_path).parent().expect("Failed to get base directory");
-        self.remote_configs.wallet_configs_file = base_dir.join(wallet_configs_path).to_string_lossy().to_string();
-    }
-}
+use std::path::Path;
+
+use axum::http::HeaderValue;
+use rusx::config::OauthConfig;
+use serde::{Deserialize, Serialize};
+
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct Config {
+    pub server: ServerConfig,
+    pub candidates: CandidatesConfig,
+    pub data: DataConfig,
+    pub logging: LoggingConfig,
+    pub jwt: JwtConfig,
+    pub x_oauth: OauthConfig,
+    pub remote_configs: RemoteConfigsConfig,
+    pub risk_checker: RiskCheckerConfig,
+    pub exchange_rate: ExchangeRateConfig,
+}
+
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct RemoteConfigsConfig {
+    pub wallet_configs_file: String,
+}
+
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct ServerConfig {
+    pub host: String,
+    pub port: u16,
+    pub cors_allowed_origins: Vec<String>,
+}
+
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct CandidatesConfig {
+    pub graphql_url: String,
+}
+
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct DataConfig {
+    pub database_url: String,
+}
+
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct LoggingConfig {
+    pub level: String,
+}
+
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct JwtConfig {
+    pub secret: String,
+    pub admin_secret: String,
+    pub exp_in_hours: i64,
+}
+
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct RiskCheckerConfig {
+    pub etherscan_api_key: String,
+    pub etherscan_base_url: String,
+    pub infura_api_key: String,
+    pub infura_base_url: String,
+    pub etherscan_calls_per_sec: u32,
+    pub max_concurrent_requests: usize,
+}
+
+/// Exchange rate API (e.g. [ExchangeRate-API v6](https://www.exchangerate-api.com/)).
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct ExchangeRateConfig {
+    pub api_key: String,
+}
+
+impl Config {
+    /// Placeholder values that must never reach a running deployment. If a
+    /// JWT secret is left at this value, every access token (including admin
+    /// tokens) can be forged by anyone who has read the committed default.
+    const PLACEHOLDER_JWT_SECRET: &str = "this-should-be-overriden";
+
+    pub fn load(config_path: &str) -> Result<Self, config::ConfigError> {
+        let settings = config::Config::builder()
+            .add_source(config::File::new(config_path, config::FileFormat::Toml))
+            .add_source(config::Environment::with_prefix("TASKMASTER"))
+            .build()?;
+
+        let mut config: Self = settings.try_deserialize()?;
+        config.resolve_relative_paths(config_path);
+        config.validate()?;
+        Ok(config)
+    }
+
+    /// Fail fast when security-critical secrets are left at their committed
+    /// placeholder values. Override them via `TASKMASTER_JWT__SECRET` /
+    /// `TASKMASTER_JWT__ADMIN_SECRET` or an environment-specific config file.
+    fn validate(&self) -> Result<(), config::ConfigError> {
+        if self.jwt.secret == Self::PLACEHOLDER_JWT_SECRET
+            || self.jwt.admin_secret == Self::PLACEHOLDER_JWT_SECRET
+        {
+            return Err(config::ConfigError::Message(
+                "JWT secrets are still set to the committed placeholder \
+                 \"this-should-be-overriden\". Refusing to start: set \
+                 TASKMASTER_JWT__SECRET and TASKMASTER_JWT__ADMIN_SECRET (or a \
+                 non-default config file) before running the server."
+                    .to_string(),
+            ));
+        }
+        Ok(())
+    }
+
+    #[cfg(test)]
+    pub fn load_test_env() -> Result<Self, config::ConfigError> {
+        let test_config_path = "config/test.toml";
+        let settings = config::Config::builder()
+            // Load the test-specific configuration file
+            .add_source(config::File::new(test_config_path, config::FileFormat::Toml))
+            // You can still layer environment variables for testing if you need to
+            .add_source(config::Environment::with_prefix("TASKMASTER"))
+            .build()?;
+
+        let mut config: Self = settings.try_deserialize()?;
+        config.resolve_relative_paths(test_config_path);
+        Ok(config)
+    }
+
+    pub fn get_database_url(&self) -> &str {
+        &self.data.database_url
+    }
+
+    pub fn get_server_address(&self) -> String {
+        format!("{}:{}", self.server.host, self.server.port)
+    }
+
+    pub fn get_jwt_expiration(&self) -> chrono::Duration {
+        chrono::Duration::hours(self.jwt.exp_in_hours)
+    }
+
+    pub fn get_cors_allowed_origins(&self) -> Vec<HeaderValue> {
+        self.server
+            .cors_allowed_origins
+            .iter()
+            .filter_map(|o| match o.parse() {
+                Ok(v) => Some(v),
+                Err(e) => {
+                    tracing::warn!("Skipping invalid CORS origin {:?}: {}", o, e);
+                    None
+                }
+            })
+            .collect()
+    }
+
+    fn resolve_relative_paths(&mut self, config_path: &str) {
+        let wallet_configs_path = Path::new(&self.remote_configs.wallet_configs_file);
+        if wallet_configs_path.is_absolute() {
+            return;
+        }
+        let base_dir = Path::new(config_path).parent().expect("Failed to get base directory");
+        self.remote_configs.wallet_configs_file = base_dir.join(wallet_configs_path).to_string_lossy().to_string();
+    }
+}