diff --git a/packages/cli/src/commander-compat.ts b/packages/cli/src/commander-compat.ts new file mode 100644 index 0000000..7f9fcf7 --- /dev/null +++ b/packages/cli/src/commander-compat.ts @@ -0,0 +1,17 @@ +import { Command } from "commander"; + +declare module "commander" { + interface Command { + addCommand(command: Command): this; + } +} + +if (typeof (Command.prototype as any).addCommand !== "function") { + (Command.prototype as any).addCommand = function (command: Command) { + this.commands = this.commands || []; + this.commands.push(command); + return this; + }; +} + +export {}; diff --git a/packages/cli/src/commands/annotate.ts b/packages/cli/src/commands/annotate.ts index 6ebbbef..2145d6c 100644 --- a/packages/cli/src/commands/annotate.ts +++ b/packages/cli/src/commands/annotate.ts @@ -4,7 +4,7 @@ import { annotateFile, Annotation } from "../../../src/reporting/annotator"; export const annotateCommand = new Command("annotate") .description("Annotate source files with inline issue comments") - .argument("", "Source file to annotate") + .arguments("") .option("-o, --output ", "Output file path (default: .annotated)") .option("--line ", "Line number for a demo annotation", "1") .action((file: string, options) => { diff --git a/packages/cli/src/commands/ast.ts b/packages/cli/src/commands/ast.ts index f47dc4e..c1a7146 100644 --- a/packages/cli/src/commands/ast.ts +++ b/packages/cli/src/commands/ast.ts @@ -12,7 +12,7 @@ import { export const astCommand = new Command("ast") .description("Inspect the AST of a smart contract source file") - .argument("", "Path to a .sol, .rs, or .vy source file") + .arguments("") .option("--json", "Output full snapshot as JSON instead of the tree view") .option("--compact", "Use compact (single-line) JSON (implies --json)") .option("-o, --output ", "Write output to a file instead of stdout") diff --git a/packages/cli/src/commands/config.ts b/packages/cli/src/commands/config.ts index 8b1aba9..51edff0 100644 --- a/packages/cli/src/commands/config.ts +++ b/packages/cli/src/commands/config.ts @@ -1,3 +1,5 @@ +import "../commander-compat"; +import "../commander-compat"; import { Command } from "commander"; import chalk from "chalk"; import fs from "fs-extra"; @@ -29,8 +31,7 @@ const showConfigCommand = new Command("show") const setConfigCommand = new Command("set") .description("Set a configuration value") - .argument("", "Configuration key (e.g., scan.maxFiles)") - .argument("", "Configuration value") + .arguments(" ") .action(async (key: string, value: string) => { try { const configPath = path.join(process.cwd(), "gasguard.config.json"); diff --git a/packages/cli/src/commands/scan.ts b/packages/cli/src/commands/scan.ts index d27d577..91ae155 100644 --- a/packages/cli/src/commands/scan.ts +++ b/packages/cli/src/commands/scan.ts @@ -2,14 +2,23 @@ import { Command } from "commander"; import chalk from "chalk"; import fs from "fs-extra"; import path from "path"; -import { generateJsonReport } from "../../reporting/json-reporter"; -import { generateSarifReport } from "../../reporting/sarif-reporter"; -import { printSummary } from "../../reporting/summary-printer"; +import { generateJsonReport, type ScanResult } from "../reporting/json-reporter"; +import { generateSarifReport } from "../reporting/sarif-reporter"; +import { printSummary } from "../reporting/summary-printer"; import { ScanWatcher } from "../../../../src/analysis/watch/watcher"; +export interface ScanCommandOptions { + output?: string; + format: "json" | "sarif" | "text" | "both"; + summary?: boolean; + fixPreview?: boolean; + watch?: boolean; + confidence: string; +} + export const scanCommand = new Command("scan") .description("Scan smart contracts for gas optimization opportunities") - .argument("[path]", "Path to scan (default: current directory)", ".") + .arguments("[path]") .option("-o, --output ", "Output file for JSON report") .option( "-f, --format ", @@ -27,68 +36,26 @@ export const scanCommand = new Command("scan") "Minimum confidence threshold (0.0-1.0)", "0.7", ) - .action(async (scanPath: string, options) => { + .action(async (scanPath: string = ".", options: ScanCommandOptions) => { try { - const runScan = async () => { - console.log(chalk.blue(`\nšŸ” Scanning ${scanPath}...`)); - - // Collect scannable files - const files = await collectScannableFiles(scanPath); - - if (files.length === 0) { - console.log(chalk.yellow("No scannable files found.")); - return; - } - - console.log(chalk.green(`Found ${files.length} file(s) to scan.`)); - - // Simulate scanning (in real implementation, this would use the actual scanner) - const scanResults = await simulateScan(files); - - // Generate reports - if (options.format === "json" || options.format === "both") { - const outputPath = - options.output || path.join(process.cwd(), "gasguard-report.json"); - await generateJsonReport(scanResults, outputPath); - console.log(chalk.green(`āœ“ JSON report saved to ${outputPath}`)); - } + await runScan(scanPath, options); - if (options.format === "sarif") { - const outputPath = - options.output || - path.join(process.cwd(), "gasguard-report.sarif.json"); - await generateSarifReport(scanResults, outputPath); - console.log(chalk.green(`āœ“ SARIF report saved to ${outputPath}`)); - } - - if ( - options.summary !== false && - (options.format === "text" || options.format === "both") - ) { - printSummary(scanResults, options); - } - }; - - // Perform initial scan - await runScan(); - - // Setup Watch Mode if requested if (options.watch) { console.log( chalk.cyan( - `\nšŸ‘€ Watch mode enabled. Listening for changes in ${scanPath}...`, + `\nWatch mode enabled. Listening for changes in ${scanPath}...`, ), ); + const watcher = new ScanWatcher(scanPath, { ignored: (p) => p.includes("node_modules") || p.includes(".git"), }); watcher.watch(async (filePath) => { console.log(chalk.cyan(`\n[File Changed] ${filePath}`)); - await runScan(); + await runScan(scanPath, options); }); - // Keep the process alive process.on("SIGINT", () => { watcher.stop(); process.exit(0); @@ -100,10 +67,57 @@ export const scanCommand = new Command("scan") } }); +export async function runScan( + scanPath: string, + options: ScanCommandOptions, +): Promise { + console.log(chalk.blue(`\nScanning ${scanPath}...`)); + + const files = await collectScannableFiles(scanPath); + + if (files.length === 0) { + console.log(chalk.yellow("No scannable files found.")); + return; + } + + console.log(chalk.green(`Found ${files.length} file(s) to scan.`)); + + const scanResults = await simulateScan(files); + + if (options.format === "json" || options.format === "both") { + const outputPath = + options.output || path.join(process.cwd(), "gasguard-report.json"); + await generateJsonReport(scanResults, outputPath); + console.log(chalk.green(`JSON report saved to ${outputPath}`)); + } + + if (options.format === "sarif") { + const outputPath = + options.output || path.join(process.cwd(), "gasguard-report.sarif.json"); + await generateSarifReport(scanResults, outputPath); + console.log(chalk.green(`SARIF report saved to ${outputPath}`)); + } + + if ( + options.summary !== false && + (options.format === "text" || options.format === "both") + ) { + printSummary(scanResults, { + fixPreview: options.fixPreview, + confidence: Number(options.confidence), + }); + } +} + async function collectScannableFiles(dirPath: string): Promise { const files: string[] = []; const extensions = [".sol", ".vy", ".rs"]; + const stats = await fs.stat(dirPath); + if (stats.isFile()) { + return extensions.includes(path.extname(dirPath)) ? [dirPath] : []; + } + async function walk(currentPath: string) { const entries = await fs.readdir(currentPath, { withFileTypes: true }); @@ -111,7 +125,6 @@ async function collectScannableFiles(dirPath: string): Promise { const fullPath = path.join(currentPath, entry.name); if (entry.isDirectory()) { - // Skip node_modules and .git if ( !["node_modules", ".git", "target", "dist", "build"].includes( entry.name, @@ -132,9 +145,8 @@ async function collectScannableFiles(dirPath: string): Promise { return files; } -async function simulateScan(files: string[]): Promise { - // This is a placeholder - in real implementation, this would use the actual scanner - const results = { +async function simulateScan(files: string[]): Promise { + const results: ScanResult = { timestamp: new Date().toISOString(), scanPath: files[0] || ".", totalFiles: files.length, @@ -154,7 +166,6 @@ async function simulateScan(files: string[]): Promise { }, }; - // Simulate some findings for demonstration if (files.length > 0) { results.findings.push({ file: files[0], diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 73436f9..6320307 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -1,5 +1,6 @@ #!/usr/bin/env node +import "./commander-compat"; import { Command } from "commander"; import chalk from "chalk"; import { scanCommand } from "./commands/scan"; @@ -19,10 +20,12 @@ program .option("--no-color", "Disable colored output"); // Global error handling -program.configureOutput({ - writeErr: (str: string) => process.stderr.write(chalk.red(str)), - writeOut: (str: string) => process.stdout.write(str), -}); +if (typeof (program as any).configureOutput === "function") { + program.configureOutput({ + writeErr: (str: string) => process.stderr.write(chalk.red(str)), + writeOut: (str: string) => process.stdout.write(str), + }); +} // Add commands program.addCommand(scanCommand); diff --git a/packages/cli/src/reporting/summary-printer.ts b/packages/cli/src/reporting/summary-printer.ts index 40c1502..457486d 100644 --- a/packages/cli/src/reporting/summary-printer.ts +++ b/packages/cli/src/reporting/summary-printer.ts @@ -88,9 +88,20 @@ export function printSummary( ); } -function printSeverity(label: string, count: number, color: string): void { - const coloredCount = - count > 0 ? chalk[color](count.toString()) : chalk.gray("0"); +function printSeverity( + label: string, + count: number, + color: "red" | "yellow" | "blue" | "gray", +): void { + const colorize = + color === "red" + ? chalk.red + : color === "yellow" + ? chalk.yellow + : color === "blue" + ? chalk.blue + : chalk.gray; + const coloredCount = count > 0 ? colorize(count.toString()) : chalk.gray("0"); console.log(` ${label.padEnd(10)}: ${coloredCount}`); } diff --git a/packages/cli/src/types/fs-extra.d.ts b/packages/cli/src/types/fs-extra.d.ts new file mode 100644 index 0000000..e7e94ae --- /dev/null +++ b/packages/cli/src/types/fs-extra.d.ts @@ -0,0 +1 @@ +declare module "fs-extra"; diff --git a/packages/rules/src/lib.rs b/packages/rules/src/lib.rs index 087ea11..3bbcbf2 100644 --- a/packages/rules/src/lib.rs +++ b/packages/rules/src/lib.rs @@ -9,7 +9,9 @@ pub mod unused_state_variables; pub mod vyper; // Explicitly export core types to avoid ambiguity +pub use optimization::arrays::detect_dynamic_array_deletions; pub use optimization::deployment::{estimate_bytecode_size, ExcessiveContractSizeRule}; +pub use optimization::storage::detect_mapping_iteration; pub use optimization::storage::{ detect_packing_opportunities, find_consecutive_packable_groups, get_type_size, is_packable_type, PackingOpportunity, VariableInfo, @@ -17,6 +19,8 @@ pub use optimization::storage::{ pub use rule_engine::{ extract_struct_fields, find_variable_usage, Rule, RuleEngine, RuleViolation, ViolationSeverity, }; +pub use security::{HardcodedAddressesRule, MissingDomainSeparationRule}; +pub use solidity::{DynamicArrayDeletionRule, MappingIterationRule, StateVariablePackingRule}; pub use security::{HardcodedAddressesRule, MissingDomainSeparationRule, defi::MissingSlippageValidationRule}; pub use solidity::{StateVariablePackingRule, MappingIterationRule}; pub use optimization::storage::detect_mapping_iteration; diff --git a/packages/rules/src/optimization/arrays/dynamic_array_deletion.rs b/packages/rules/src/optimization/arrays/dynamic_array_deletion.rs new file mode 100644 index 0000000..0ff9e6a --- /dev/null +++ b/packages/rules/src/optimization/arrays/dynamic_array_deletion.rs @@ -0,0 +1,188 @@ +use crate::rule_engine::{RuleViolation, ViolationSeverity}; +use gasguard_ast::{Language, UnifiedAST}; + +fn normalize_whitespace(input: &str) -> String { + input.split_whitespace().collect::>().join(" ") +} + +fn is_dynamic_array(type_name: &str) -> bool { + let normalized = type_name.trim().to_lowercase(); + normalized.ends_with("[]") +} + +fn build_shift_pattern(array_name: &str) -> String { + format!( + r"\b{}\s*\[[^\]]+\]\s*=\s*{}\s*\[[^\]]+\+\s*1\s*\]", + regex::escape(array_name), + regex::escape(array_name) + ) +} + +fn build_reverse_shift_pattern(array_name: &str) -> String { + format!( + r"\b{}\s*\[[^\]]+\-\s*1\s*\]\s*=\s*{}\s*\[[^\]]+\]\s*", + regex::escape(array_name), + regex::escape(array_name) + ) +} + +fn detect_shifting_deletion(body: &str, array_name: &str) -> bool { + let normalized = normalize_whitespace(body); + let shift_regex = regex::Regex::new(&build_shift_pattern(array_name)) + .expect("shift detection regex must compile"); + let reverse_shift_regex = regex::Regex::new(&build_reverse_shift_pattern(array_name)) + .expect("reverse shift detection regex must compile"); + + let has_shift = shift_regex.is_match(&normalized) || reverse_shift_regex.is_match(&normalized); + let has_pop = normalized.contains(&format!("{}.pop()", array_name)); + + has_shift && has_pop +} + +/// Detects expensive deletions from dynamic arrays that manually shift elements. +pub fn detect_dynamic_array_deletions(ast: &UnifiedAST) -> Vec { + let mut violations = Vec::new(); + + if ast.language != Language::Solidity { + return violations; + } + + for contract in &ast.contracts { + let dynamic_arrays: Vec<_> = contract + .state_variables + .iter() + .filter(|var| is_dynamic_array(&var.type_name)) + .collect(); + + if dynamic_arrays.is_empty() { + continue; + } + + for func in &contract.functions { + let body = normalize_whitespace(&func.body_raw); + let has_loop = body.contains("for (") + || body.contains("for(") + || body.contains("while (") + || body.contains("while("); + + if !has_loop { + continue; + } + + for array in &dynamic_arrays { + if detect_shifting_deletion(&body, &array.name) { + violations.push(RuleViolation { + rule_name: "dynamic-array-deletion".to_string(), + description: format!( + "Function '{}' shifts elements of dynamic array '{}' before deleting an item. This is gas-heavy for storage arrays.", + func.name, array.name + ), + severity: ViolationSeverity::Medium, + line_number: func.line_number, + column_number: 1, + variable_name: array.name.clone(), + suggestion: format!( + "Use swap-and-pop for '{}' when order does not matter: move the last element into the deleted slot and call '{}.pop()' once.", + array.name, array.name + ), + }); + } + } + } + } + + violations +} + +#[cfg(test)] +mod tests { + use super::*; + use gasguard_ast::{ContractNode, FunctionNode, ParamNode, VariableNode, Visibility}; + + #[test] + fn detects_shift_and_pop_deletion() { + let ast = UnifiedAST { + language: Language::Solidity, + source: String::new(), + file_path: String::new(), + structs: vec![], + enums: vec![], + contracts: vec![ContractNode { + name: "Test".to_string(), + line_number: 1, + state_variables: vec![VariableNode { + name: "users".to_string(), + type_name: "address[]".to_string(), + visibility: Visibility::Public, + is_constant: false, + is_immutable: false, + line_number: 2, + }], + functions: vec![FunctionNode { + name: "removeUser".to_string(), + params: vec![ParamNode { + name: "index".to_string(), + type_name: "uint256".to_string(), + }], + return_type: None, + visibility: Visibility::Public, + decorators: vec![], + is_constructor: false, + is_external: false, + is_payable: false, + line_number: 10, + body_raw: r#" + for (uint256 i = index; i < users.length - 1; i++) { + users[i] = users[i + 1]; + } + users.pop(); + "# + .to_string(), + }], + }], + }; + + let violations = detect_dynamic_array_deletions(&ast); + assert_eq!(violations.len(), 1); + assert_eq!(violations[0].variable_name, "users"); + assert!(violations[0].suggestion.contains("swap-and-pop")); + } + + #[test] + fn ignores_non_shifting_pop() { + let ast = UnifiedAST { + language: Language::Solidity, + source: String::new(), + file_path: String::new(), + structs: vec![], + enums: vec![], + contracts: vec![ContractNode { + name: "Test".to_string(), + line_number: 1, + state_variables: vec![VariableNode { + name: "users".to_string(), + type_name: "address[]".to_string(), + visibility: Visibility::Public, + is_constant: false, + is_immutable: false, + line_number: 2, + }], + functions: vec![FunctionNode { + name: "removeLast".to_string(), + params: vec![], + return_type: None, + visibility: Visibility::Public, + decorators: vec![], + is_constructor: false, + is_external: false, + is_payable: false, + line_number: 10, + body_raw: "users.pop();".to_string(), + }], + }], + }; + + let violations = detect_dynamic_array_deletions(&ast); + assert!(violations.is_empty()); + } +} diff --git a/packages/rules/src/optimization/arrays/mod.rs b/packages/rules/src/optimization/arrays/mod.rs new file mode 100644 index 0000000..6d85d88 --- /dev/null +++ b/packages/rules/src/optimization/arrays/mod.rs @@ -0,0 +1,3 @@ +pub mod dynamic_array_deletion; + +pub use dynamic_array_deletion::detect_dynamic_array_deletions; diff --git a/packages/rules/src/optimization/mod.rs b/packages/rules/src/optimization/mod.rs index 767a185..51f23f3 100644 --- a/packages/rules/src/optimization/mod.rs +++ b/packages/rules/src/optimization/mod.rs @@ -1,6 +1,8 @@ +pub mod arrays; pub mod deployment; pub mod storage; +pub use arrays::detect_dynamic_array_deletions; pub use storage::{ detect_packing_opportunities, find_consecutive_packable_groups, get_type_size, is_packable_type, PackingOpportunity, VariableInfo, diff --git a/packages/rules/src/optimization/storage/mapping_iteration.rs b/packages/rules/src/optimization/storage/mapping_iteration.rs index 91bd2fc..261e0f3 100644 --- a/packages/rules/src/optimization/storage/mapping_iteration.rs +++ b/packages/rules/src/optimization/storage/mapping_iteration.rs @@ -31,7 +31,7 @@ pub fn detect_mapping_iteration(ast: &UnifiedAST) -> Vec { let pattern1 = format!("{}[{}[", mapping, array); let pattern2 = format!("{}[ {}[", mapping, array); // e.g. mapping[ array[ let pattern3 = format!("{} [{}[", mapping, array); // e.g. mapping [array[ - + if body.contains(&pattern1) || body.contains(&pattern2) || body.contains(&pattern3) { violations.push(RuleViolation { rule_name: "mapping-iteration-workaround".to_string(), diff --git a/packages/rules/src/optimization/storage/multiple_storage_reads.rs b/packages/rules/src/optimization/storage/multiple_storage_reads.rs index d1e04df..f384747 100644 --- a/packages/rules/src/optimization/storage/multiple_storage_reads.rs +++ b/packages/rules/src/optimization/storage/multiple_storage_reads.rs @@ -68,4 +68,4 @@ mod tests { let ast = parse_file("fn f() { let x = s.get(&k); }").expect("parse"); assert!(MultipleStorageReadsRule.check(&ast.items).is_empty()); } -} \ No newline at end of file +} diff --git a/packages/rules/src/solidity/dynamic_array_deletion.rs b/packages/rules/src/solidity/dynamic_array_deletion.rs new file mode 100644 index 0000000..f7ba4b3 --- /dev/null +++ b/packages/rules/src/solidity/dynamic_array_deletion.rs @@ -0,0 +1,38 @@ +use crate::optimization::arrays::detect_dynamic_array_deletions; +use crate::rule_engine::{Rule, RuleViolation}; +use gasguard_ast::UnifiedAST; +use syn::Item; + +pub struct DynamicArrayDeletionRule; + +impl Rule for DynamicArrayDeletionRule { + fn name(&self) -> &str { + "dynamic-array-deletion" + } + + fn description(&self) -> &str { + "Detects expensive deletions from dynamic arrays that shift elements instead of using swap-and-pop." + } + + fn check(&self, _ast: &[Item]) -> Vec { + Vec::new() + } +} + +impl DynamicArrayDeletionRule { + pub fn analyze(&self, ast: &UnifiedAST) -> Vec { + detect_dynamic_array_deletions(ast) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_dynamic_array_deletion_rule() { + let rule = DynamicArrayDeletionRule; + assert_eq!(rule.name(), "dynamic-array-deletion"); + assert!(rule.description().contains("swap-and-pop")); + } +} diff --git a/packages/rules/src/solidity/mod.rs b/packages/rules/src/solidity/mod.rs index 0152fc6..05349c6 100644 --- a/packages/rules/src/solidity/mod.rs +++ b/packages/rules/src/solidity/mod.rs @@ -1,6 +1,8 @@ +pub mod dynamic_array_deletion; +pub mod mapping_iteration; pub mod state_variable_packing; pub mod uint8_vs_uint256; -pub mod mapping_iteration; -pub use state_variable_packing::StateVariablePackingRule; +pub use dynamic_array_deletion::DynamicArrayDeletionRule; pub use mapping_iteration::MappingIterationRule; +pub use state_variable_packing::StateVariablePackingRule; diff --git a/scripts/pre-commit-scan.js b/scripts/pre-commit-scan.js index 3261d5c..b46433a 100644 --- a/scripts/pre-commit-scan.js +++ b/scripts/pre-commit-scan.js @@ -20,10 +20,8 @@ function filterScannable(files) { function runScanOnFile(file) { console.log(`Running GasGuard scan on staged file: ${file}`); - // Execute the monorepo CLI using ts-node to run the TypeScript source directly. - // This avoids requiring a pre-built CLI binary during development. const runner = 'node'; - const args = ['-r', 'ts-node/register', 'packages/cli/src/index.ts', 'scan', file, '--no-summary', '--format', 'text']; + const args = ['-r', 'ts-node/register', 'scripts/run-precommit-scan.ts', file]; const res = spawnSync(runner, args, { stdio: 'inherit' }); if (res.error) { diff --git a/scripts/run-precommit-scan.ts b/scripts/run-precommit-scan.ts new file mode 100644 index 0000000..2420110 --- /dev/null +++ b/scripts/run-precommit-scan.ts @@ -0,0 +1,21 @@ +import { runScan } from "../packages/cli/src/commands/scan"; + +async function main() { + const file = process.argv[2]; + + if (!file) { + console.error("Missing file path for pre-commit scan."); + process.exit(1); + } + + await runScan(file, { + format: "text", + summary: false, + confidence: "0.7", + }); +} + +main().catch((error) => { + console.error(error); + process.exit(1); +});