Enhancement to Secret Scanner: Detection of Hardcoded Secrets in App's Manifest

crates/forge_analyzer/src/checkers.rs

@@ -305,6 +305,7 @@ impl<'cx> Dataflow<'cx> for AuthorizeDataflow {
 
 pub struct PrototypePollutionChecker;
 
+#[allow(dead_code)]
 #[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Default)]
 enum PrototypePollutionState {
     Yes,
@@ -844,6 +845,16 @@ impl SecretChecker {
         // TODO: make this an associated function on the Checker trait.
         self.vulns.into_iter()
     }
+
+    pub fn add_manifest_secret(
+        &mut self,
+        location: String,
+        field_name: String,
+        secret_type: SecretType,
+    ) {
+        let vuln = SecretVuln::from_manifest(location, field_name, secret_type);
+        self.vulns.push(vuln);
+    }
 }
 
 impl Default for SecretChecker {
@@ -852,11 +863,18 @@ impl Default for SecretChecker {
     }
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum SecretType {
+    OAuthProvider,
+    Regular,
+}
+
 #[derive(Debug)]
 pub struct SecretVuln {
     stack: String,
     entry_func: String,
     file: PathBuf,
+    secret_type: SecretType,
 }
 
 impl SecretVuln {
@@ -884,6 +902,16 @@ impl SecretVuln {
             stack,
             entry_func,
             file,
+            secret_type: SecretType::Regular,
+        }
+    }
+
+    fn from_manifest(location: String, field_name: String, secret_type: SecretType) -> Self {
+        Self {
+            stack: format!("manifest.yml: {}", field_name),
+            entry_func: location,
+            file: PathBuf::from("manifest.yml"),
+            secret_type,
         }
     }
 }
@@ -906,13 +934,33 @@ impl IntoVuln for SecretVuln {
             .for_each(|comp| comp.hash(&mut hasher));
         self.entry_func.hash(&mut hasher);
         self.stack.hash(&mut hasher);
+
+        let recommendation = match self.secret_type {
+            SecretType::OAuthProvider => {
+                "Configure the OAuth Provider secrets in your OAuth Provider settings and use runtime templates (e.g. {{client_secret}} to reference secrets securely as opposed to hardcoding them directly in the codebase. See https://developer.atlassian.com/platform/forge/runtime-reference/storage-api-secret/ for more details.)"
+            }
+            SecretType::Regular => {
+                "Use secrets as enviornment variables instead of hardcoding them."
+            }
+        };
+
+        let check_name = match self.secret_type {
+            SecretType::OAuthProvider => {
+                format!(
+                    "Custom-Check-Hardcoded-Secret-OAuth-Provider-{}",
+                    hasher.finish()
+                )
+            }
+            SecretType::Regular => format!("Custom-Check-Hardcoded-Secret-{}", hasher.finish()),
+        };
+
         Vulnerability {
-            check_name: format!("Hardcoded-Secret-{}", hasher.finish()),
+            check_name,
             description: format!(
                 "Hardcoded secret found within codebase {} in {:?}.",
                 self.entry_func, self.file
             ),
-            recommendation: "Use secrets as enviornment variables instead of hardcoding them.",
+            recommendation,
             proof: format!("Hardcoded secret found in found via {}", self.stack),
             severity: Severity::High,
             marketplace_security_requirement: "Requirement 5.0",

crates/forge_loader/src/manifest.rs

@@ -160,6 +160,7 @@ pub struct TeamPage<'a> {
 }
 
 // Confluence Modules
+#[allow(dead_code)]
 #[derive(Default, Debug, Clone, PartialEq, Eq, Deserialize)]
 struct ContentAction<'a> {
     #[serde(flatten, borrow)]
@@ -436,6 +437,150 @@ impl Remotes {
     }
 }
 
+#[derive(Default, Debug, Clone, PartialEq, Eq, Deserialize)]
+pub struct Providers {
+    #[serde(default)]
+    pub auth: Option<Vec<OAuthProvider>>,
+}
+
+#[derive(Default, Debug, Clone, PartialEq, Eq, Deserialize)]
+pub struct OAuthProvider {
+    pub key: String,
+    #[serde(default)]
+    pub actions: Option<OAuthActions>,
+}
+
+#[derive(Default, Debug, Clone, PartialEq, Eq, Deserialize)]
+pub struct OAuthActions {
+    #[serde(default)]
+    pub authorization: Option<AuthorizationAction>,
+    #[serde(default)]
+    pub exchange: Option<ExchangeAction>,
+    #[serde(default)]
+    pub refresh: Option<RefreshAction>,
+}
+
+#[derive(Default, Debug, Clone, PartialEq, Eq, Deserialize)]
+pub struct AuthorizationAction {
+    #[serde(default, rename = "queryParameters")]
+    pub query_params: Option<FxHashMap<String, String>>,
+}
+
+#[derive(Default, Debug, Clone, PartialEq, Eq, Deserialize)]
+pub struct ExchangeAction {
+    #[serde(default)]
+    pub overrides: Option<OAuthOverride>,
+}
+
+#[derive(Default, Debug, Clone, PartialEq, Eq, Deserialize)]
+pub struct RefreshAction {
+    #[serde(default)]
+    pub overrides: Option<OAuthOverride>,
+}
+
+#[derive(Default, Debug, Clone, PartialEq, Eq, Deserialize)]
+pub struct OAuthOverride {
+    #[serde(default)]
+    pub headers: Option<FxHashMap<String, String>>,
+    #[serde(default)]
+    pub body: Option<FxHashMap<String, String>>,
+}
+
+impl OAuthProvider {
+    fn parse_for_secrets(
+        &self,
+        map: &FxHashMap<String, String>,
+        path: &str,
+        sensitive_keywords: &[&str],
+        secrets: &mut Vec<String>,
+    ) {
+        for (key, value) in map {
+            if sensitive_keywords
+                .iter()
+                .any(|s| key.to_lowercase().contains(s))
+                && is_hardcoded_variable(value)
+            {
+                secrets.push(format!("{}.{}", path, key));
+            }
+        }
+    }
+
+    fn parse_overrides_for_secrets(
+        &self,
+        overrides: &OAuthOverride,
+        path: &str,
+        sensitive_keywords: &[&str],
+        secrets: &mut Vec<String>,
+    ) {
+        if let Some(headers) = &overrides.headers {
+            self.parse_for_secrets(
+                headers,
+                &format!("{}.headers", path),
+                sensitive_keywords,
+                secrets,
+            );
+        }
+        if let Some(body) = &overrides.body {
+            self.parse_for_secrets(body, &format!("{}.body", path), sensitive_keywords, secrets);
+        }
+    }
+
+    pub fn find_hardcoded_secrets(&self) -> Vec<String> {
+        let mut secrets = Vec::new();
+        let sensitive_keywords = [
+            "secret",
+            "token",
+            "password",
+            "authorization",
+            "api-key",
+            "apikey",
+            "credential",
+        ];
+
+        if let Some(actions) = &self.actions {
+            // Checking if there are hardcoded secrets in the query parameters
+            if let Some(authorization) = &actions.authorization
+                && let Some(query_params) = &authorization.query_params
+            {
+                self.parse_for_secrets(
+                    query_params,
+                    &format!(
+                        "providers.auth[{}].actions.authorization.queryParams",
+                        self.key
+                    ),
+                    &sensitive_keywords,
+                    &mut secrets,
+                );
+            }
+            // Checking if there are hardcoded secrets in the exchange action
+            if let Some(exchange) = &actions.exchange
+                && let Some(overrides) = &exchange.overrides
+            {
+                self.parse_overrides_for_secrets(
+                    overrides,
+                    &format!("providers.auth[{}].actions.exchange.overrides", self.key),
+                    &sensitive_keywords,
+                    &mut secrets,
+                );
+            }
+
+            // Checking if there are hardcoded secrets in the refresh action
+            if let Some(refresh) = &actions.refresh
+                && let Some(overrides) = &refresh.overrides
+            {
+                self.parse_overrides_for_secrets(
+                    overrides,
+                    &format!("providers.auth[{}].actions.refresh.overrides", self.key),
+                    &sensitive_keywords,
+                    &mut secrets,
+                );
+            }
+        }
+
+        secrets
+    }
+}
+
 #[derive(Default, Debug, Clone, PartialEq, Eq, Deserialize)]
 pub struct AppInfo<'a> {
     pub name: Option<&'a str>,
@@ -474,6 +619,8 @@ pub struct ForgeManifest<'a> {
     pub remotes: Option<Vec<Remotes>>,
     #[serde(default, borrow)]
     pub resources: Vec<Resource<'a>>,
+    #[serde(default)]
+    pub providers: Option<Providers>,
 }
 
 impl<'a> ForgeManifest<'a> {
@@ -816,6 +963,14 @@ impl<'a> TryFrom<FunctionMod<'a>> for FunctionRef<'a> {
     }
 }
 
+fn is_hardcoded_variable(value: &str) -> bool {
+    if let Some(start) = value.find("{{") {
+        !(value[start + 2..].contains("}}"))
+    } else {
+        true
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -1149,4 +1304,85 @@ mod tests {
         assert_eq!(action.function, Some("compass-fn"));
         assert_eq!(action.endpoint, None);
     }
+
+    // Test to check if hardcoded secrets are detected properly in OAuth2 Provider
+    #[test]
+    fn test_oauth_provider_hardcoded_secrets() {
+        let json = r#"{
+            "app": {
+                "name": "My App",
+                "id": "ari:cloud:ecosystem::app/test-id"
+            },
+            "modules": {},
+            "permissions": {
+                "scopes": []
+            },
+            "providers": {
+                "auth": [
+                    {
+                        "key": "oauth-provider-1",
+                        "actions": {
+                            "authorization": {
+                                "queryParameters": {
+                                    "client_id": "{{client_id}}",
+                                    "client_secret": "hardcoded_secret_value"
+                                }
+                            },
+                            "exchange": {
+                                "overrides": {
+                                    "headers": {
+                                        "Authorization": "Bearer hardcoded_token_value"
+                                    },
+                                    "body": {
+                                        "api_key": "{{api_key}}",
+                                        "password": "hardcoded_password_value"
+                                    }
+                                }
+                            },
+                            "refresh": {
+                                "overrides": {
+                                    "headers": {
+                                        "refresh-token": "hardcoded_token_value"
+                                    },
+                                    "body": {
+                                        "client_secret": "hardcoded_refresh_secret_value"
+                                    }
+                                }
+                            }
+                        }
+                    }
+                ]
+            }
+        }"#;
+
+        let manifest: ForgeManifest<'_> = serde_json::from_str(json).unwrap();
+        let providers = manifest.providers.unwrap();
+        let auth_providers = providers.auth.unwrap();
+
+        let mut secrets_found = Vec::new();
+        for provider in auth_providers {
+            let findings = provider.find_hardcoded_secrets();
+            secrets_found.extend(findings);
+        }
+
+        let secrets_expected = vec![
+            "providers.auth[oauth-provider-1].actions.authorization.queryParams.client_secret"
+                .to_string(),
+            "providers.auth[oauth-provider-1].actions.exchange.overrides.headers.Authorization"
+                .to_string(),
+            "providers.auth[oauth-provider-1].actions.exchange.overrides.body.password".to_string(),
+            "providers.auth[oauth-provider-1].actions.refresh.overrides.headers.refresh-token"
+                .to_string(),
+            "providers.auth[oauth-provider-1].actions.refresh.overrides.body.client_secret"
+                .to_string(),
+        ];
+
+        assert_eq!(secrets_found, secrets_expected);
+
+        assert!(!is_hardcoded_variable(
+            "Basic {{http_basic_auth_credentials}}",
+        ));
+        assert!(!is_hardcoded_variable("{{prefix}}_{{suffix}}"));
+        assert!(!is_hardcoded_variable("     {{client_secret}}     "));
+    }
 }