Skip to content

Commit b805ff6

Browse files
echobtechobt
committed
fix(plugins): fix clippy and formatting issues for CI compliance
- Fix collapsible if statements in plugin_cmd.rs validation functions - Apply cargo fmt formatting to registry.rs SSRF tests - Refactor nested conditionals to satisfy clippy warnings - All 108 cortex-plugins tests pass
1 parent fdbef19 commit b805ff6

File tree

2 files changed

+120
-93
lines changed

2 files changed

+120
-93
lines changed

‎src/cortex-cli/src/plugin_cmd.rs‎

Lines changed: 52 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1778,14 +1778,13 @@ async fn run_validate(args: PluginValidateArgs) -> Result<()> {
17781778
.get("description")
17791779
.and_then(|v| v.as_str())
17801780
.is_none()
1781+
&& args.verbose
17811782
{
1782-
if args.verbose {
1783-
result.issues.push(ValidationIssue {
1784-
severity: ValidationSeverity::Info,
1785-
message: "Consider adding a description".to_string(),
1786-
field: Some("description".to_string()),
1787-
});
1788-
}
1783+
result.issues.push(ValidationIssue {
1784+
severity: ValidationSeverity::Info,
1785+
message: "Consider adding a description".to_string(),
1786+
field: Some("description".to_string()),
1787+
});
17891788
}
17901789

17911790
// Check for WASM file
@@ -1835,23 +1834,21 @@ async fn run_validate(args: PluginValidateArgs) -> Result<()> {
18351834
}
18361835

18371836
// Validate permissions if present
1838-
if let Some(permissions) = manifest
1837+
if let Some(perms_array) = manifest
18391838
.get("permissions")
18401839
.or_else(|| plugin_section.get("permissions"))
1840+
.and_then(|p| p.as_array())
18411841
{
1842-
if let Some(perms_array) = permissions.as_array() {
1843-
validate_permissions(perms_array, &mut result, args.verbose);
1844-
}
1842+
validate_permissions(perms_array, &mut result, args.verbose);
18451843
}
18461844

18471845
// Validate capabilities if present
1848-
if let Some(capabilities) = manifest
1846+
if let Some(caps_array) = manifest
18491847
.get("capabilities")
18501848
.or_else(|| plugin_section.get("capabilities"))
1849+
.and_then(|c| c.as_array())
18511850
{
1852-
if let Some(caps_array) = capabilities.as_array() {
1853-
validate_capabilities(caps_array, &mut result, args.verbose);
1854-
}
1851+
validate_capabilities(caps_array, &mut result, args.verbose);
18551852
}
18561853

18571854
// Check for source files
@@ -1906,21 +1903,25 @@ fn validate_permissions(permissions: &[toml::Value], result: &mut ValidationResu
19061903
}
19071904

19081905
// Check for overly broad permissions
1909-
if let Some(value) = table.get(key) {
1910-
if let Some(paths) = value.get("paths").and_then(|p| p.as_array()) {
1911-
for path in paths {
1912-
if let Some(p) = path.as_str() {
1913-
if p == "**/*" || p == "**" || p == "*" {
1914-
result.issues.push(ValidationIssue {
1915-
severity: ValidationSeverity::Warning,
1916-
message: format!(
1917-
"Overly broad permission pattern '{}' for {}. Consider restricting to specific paths.",
1918-
p, key
1919-
),
1920-
field: Some("permissions".to_string()),
1921-
});
1922-
}
1923-
}
1906+
if let Some(paths) = table
1907+
.get(key)
1908+
.and_then(|v| v.get("paths"))
1909+
.and_then(|p| p.as_array())
1910+
{
1911+
for path in paths {
1912+
let is_overly_broad = path
1913+
.as_str()
1914+
.map(|p| p == "**/*" || p == "**" || p == "*")
1915+
.unwrap_or(false);
1916+
if is_overly_broad {
1917+
result.issues.push(ValidationIssue {
1918+
severity: ValidationSeverity::Warning,
1919+
message: format!(
1920+
"Overly broad permission pattern '{}' for {}. Consider restricting to specific paths.",
1921+
path.as_str().unwrap_or(""), key
1922+
),
1923+
field: Some("permissions".to_string()),
1924+
});
19241925
}
19251926
}
19261927
}
@@ -1955,15 +1956,12 @@ fn validate_capabilities(
19551956
"network",
19561957
];
19571958

1958-
let mut unknown_caps = Vec::new();
1959-
1960-
for cap in capabilities {
1961-
if let Some(cap_str) = cap.as_str() {
1962-
if !KNOWN_CAPABILITIES.contains(&cap_str) {
1963-
unknown_caps.push(cap_str.to_string());
1964-
}
1965-
}
1966-
}
1959+
let unknown_caps: Vec<String> = capabilities
1960+
.iter()
1961+
.filter_map(|cap| cap.as_str())
1962+
.filter(|cap_str| !KNOWN_CAPABILITIES.contains(cap_str))
1963+
.map(String::from)
1964+
.collect();
19671965

19681966
if !unknown_caps.is_empty() {
19691967
result.issues.push(ValidationIssue {
@@ -2096,26 +2094,27 @@ async fn run_publish(args: PluginPublishArgs) -> Result<()> {
20962094
run_plugin_build(&plugin_dir, false, None)?;
20972095
}
20982096

2099-
// Verify WASM exists after build
2097+
// Verify WASM exists after build, try to copy from target dir if needed
21002098
if !wasm_path.exists() {
2101-
// Try to copy from target
21022099
let target_wasm = plugin_dir
21032100
.join("target")
21042101
.join("wasm32-wasi")
21052102
.join("release");
21062103

2107-
if target_wasm.exists() {
2108-
if let Ok(entries) = std::fs::read_dir(&target_wasm) {
2109-
for entry in entries.flatten() {
2110-
if entry
2111-
.path()
2112-
.extension()
2113-
.map(|e| e == "wasm")
2114-
.unwrap_or(false)
2115-
{
2116-
std::fs::copy(entry.path(), &wasm_path)?;
2117-
break;
2118-
}
2104+
if let Some(entries) = target_wasm
2105+
.exists()
2106+
.then(|| std::fs::read_dir(&target_wasm).ok())
2107+
.flatten()
2108+
{
2109+
for entry in entries.flatten() {
2110+
let is_wasm = entry
2111+
.path()
2112+
.extension()
2113+
.map(|e| e == "wasm")
2114+
.unwrap_or(false);
2115+
if is_wasm {
2116+
std::fs::copy(entry.path(), &wasm_path)?;
2117+
break;
21192118
}
21202119
}
21212120
}

‎src/cortex-plugins/src/registry.rs‎

Lines changed: 68 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -508,10 +508,7 @@ impl PluginRegistry {
508508
if Self::is_private_ipv4(ip) {
509509
return Err(PluginError::validation_error(
510510
"download_url",
511-
format!(
512-
"Download URL points to private/internal IP address: {}",
513-
ip
514-
),
511+
format!("Download URL points to private/internal IP address: {}", ip),
515512
));
516513
}
517514
}
@@ -550,34 +547,31 @@ impl PluginRegistry {
550547
// Block dangerous ports commonly used for internal services
551548
if let Some(port) = parsed.port() {
552549
const DANGEROUS_PORTS: &[u16] = &[
553-
22, // SSH
554-
23, // Telnet
555-
25, // SMTP
556-
135, // RPC
557-
137, // NetBIOS
558-
138, // NetBIOS
559-
139, // NetBIOS
560-
445, // SMB
561-
1433, // MSSQL
562-
1521, // Oracle
563-
3306, // MySQL
564-
3389, // RDP
565-
5432, // PostgreSQL
566-
5900, // VNC
567-
6379, // Redis
568-
8080, // Common proxy
569-
8443, // Alt HTTPS
570-
9200, // Elasticsearch
550+
22, // SSH
551+
23, // Telnet
552+
25, // SMTP
553+
135, // RPC
554+
137, // NetBIOS
555+
138, // NetBIOS
556+
139, // NetBIOS
557+
445, // SMB
558+
1433, // MSSQL
559+
1521, // Oracle
560+
3306, // MySQL
561+
3389, // RDP
562+
5432, // PostgreSQL
563+
5900, // VNC
564+
6379, // Redis
565+
8080, // Common proxy
566+
8443, // Alt HTTPS
567+
9200, // Elasticsearch
571568
27017, // MongoDB
572569
];
573570

574571
if DANGEROUS_PORTS.contains(&port) {
575572
return Err(PluginError::validation_error(
576573
"download_url",
577-
format!(
578-
"Download URL uses a potentially dangerous port: {}",
579-
port
580-
),
574+
format!("Download URL uses a potentially dangerous port: {}", port),
581575
));
582576
}
583577
}
@@ -1123,23 +1117,33 @@ mod tests {
11231117

11241118
// Private class A (10.0.0.0/8)
11251119
assert!(PluginRegistry::validate_download_url("https://10.0.0.1/plugin.wasm").is_err());
1126-
assert!(PluginRegistry::validate_download_url("https://10.255.255.255/plugin.wasm").is_err());
1120+
assert!(
1121+
PluginRegistry::validate_download_url("https://10.255.255.255/plugin.wasm").is_err()
1122+
);
11271123

11281124
// Private class B (172.16.0.0/12)
11291125
assert!(PluginRegistry::validate_download_url("https://172.16.0.1/plugin.wasm").is_err());
1130-
assert!(PluginRegistry::validate_download_url("https://172.31.255.255/plugin.wasm").is_err());
1126+
assert!(
1127+
PluginRegistry::validate_download_url("https://172.31.255.255/plugin.wasm").is_err()
1128+
);
11311129

11321130
// Private class C (192.168.0.0/16)
11331131
assert!(PluginRegistry::validate_download_url("https://192.168.0.1/plugin.wasm").is_err());
1134-
assert!(PluginRegistry::validate_download_url("https://192.168.255.255/plugin.wasm").is_err());
1132+
assert!(
1133+
PluginRegistry::validate_download_url("https://192.168.255.255/plugin.wasm").is_err()
1134+
);
11351135

11361136
// Link-local / AWS metadata endpoint
1137-
assert!(PluginRegistry::validate_download_url("https://169.254.169.254/plugin.wasm").is_err());
1137+
assert!(
1138+
PluginRegistry::validate_download_url("https://169.254.169.254/plugin.wasm").is_err()
1139+
);
11381140
assert!(PluginRegistry::validate_download_url("https://169.254.0.1/plugin.wasm").is_err());
11391141

11401142
// Carrier-grade NAT (100.64.0.0/10)
11411143
assert!(PluginRegistry::validate_download_url("https://100.64.0.1/plugin.wasm").is_err());
1142-
assert!(PluginRegistry::validate_download_url("https://100.127.255.255/plugin.wasm").is_err());
1144+
assert!(
1145+
PluginRegistry::validate_download_url("https://100.127.255.255/plugin.wasm").is_err()
1146+
);
11431147
}
11441148

11451149
#[test]
@@ -1159,19 +1163,29 @@ mod tests {
11591163
fn test_ssrf_blocks_localhost_domains() {
11601164
assert!(PluginRegistry::validate_download_url("https://localhost/plugin.wasm").is_err());
11611165
assert!(PluginRegistry::validate_download_url("https://test.local/plugin.wasm").is_err());
1162-
assert!(PluginRegistry::validate_download_url("https://internal.internal/plugin.wasm").is_err());
1166+
assert!(
1167+
PluginRegistry::validate_download_url("https://internal.internal/plugin.wasm").is_err()
1168+
);
11631169
}
11641170

11651171
#[test]
11661172
fn test_ssrf_blocks_dangerous_ports() {
11671173
// SSH
1168-
assert!(PluginRegistry::validate_download_url("https://example.com:22/plugin.wasm").is_err());
1174+
assert!(
1175+
PluginRegistry::validate_download_url("https://example.com:22/plugin.wasm").is_err()
1176+
);
11691177
// MySQL
1170-
assert!(PluginRegistry::validate_download_url("https://example.com:3306/plugin.wasm").is_err());
1178+
assert!(
1179+
PluginRegistry::validate_download_url("https://example.com:3306/plugin.wasm").is_err()
1180+
);
11711181
// Redis
1172-
assert!(PluginRegistry::validate_download_url("https://example.com:6379/plugin.wasm").is_err());
1182+
assert!(
1183+
PluginRegistry::validate_download_url("https://example.com:6379/plugin.wasm").is_err()
1184+
);
11731185
// MongoDB
1174-
assert!(PluginRegistry::validate_download_url("https://example.com:27017/plugin.wasm").is_err());
1186+
assert!(
1187+
PluginRegistry::validate_download_url("https://example.com:27017/plugin.wasm").is_err()
1188+
);
11751189
}
11761190

11771191
#[test]
@@ -1187,14 +1201,28 @@ mod tests {
11871201
#[test]
11881202
fn test_ssrf_allows_valid_https_urls() {
11891203
assert!(PluginRegistry::validate_download_url("https://example.com/plugin.wasm").is_ok());
1190-
assert!(PluginRegistry::validate_download_url("https://plugins.cortex.dev/v1/download/test-plugin.wasm").is_ok());
1191-
assert!(PluginRegistry::validate_download_url("https://github.com/user/repo/releases/download/v1.0.0/plugin.wasm").is_ok());
1204+
assert!(
1205+
PluginRegistry::validate_download_url(
1206+
"https://plugins.cortex.dev/v1/download/test-plugin.wasm"
1207+
)
1208+
.is_ok()
1209+
);
1210+
assert!(
1211+
PluginRegistry::validate_download_url(
1212+
"https://github.com/user/repo/releases/download/v1.0.0/plugin.wasm"
1213+
)
1214+
.is_ok()
1215+
);
11921216
}
11931217

11941218
#[test]
11951219
fn test_ssrf_allows_standard_ports() {
1196-
assert!(PluginRegistry::validate_download_url("https://example.com:443/plugin.wasm").is_ok());
1197-
assert!(PluginRegistry::validate_download_url("https://example.com:8000/plugin.wasm").is_ok());
1220+
assert!(
1221+
PluginRegistry::validate_download_url("https://example.com:443/plugin.wasm").is_ok()
1222+
);
1223+
assert!(
1224+
PluginRegistry::validate_download_url("https://example.com:8000/plugin.wasm").is_ok()
1225+
);
11981226
}
11991227

12001228
// =========================================================================

0 commit comments

Comments
 (0)