diff --git a/CHANGELOG.md b/CHANGELOG.md index 880b3912..2cb9f8c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,26 @@ its heading and collects entries; the date and the link go on with the tag. port. Replies are also sent as compact JSON instead of indented, which took a sweep of the local network from 15,399 bytes to 8,746. +- **Defaults now match across the CLI, TUI, desktop app and MCP server.** + - The desktop app scanned five ports by default (22, 80, 443, 8080 and + 8443) where everything else scans 22, 80 and 443. It now uses the same + three, for port scans and inspect alike. + - The MCP server applied the timeout it advertises to every step, so an + MCP inspect or sweep pinged with 500 ms where the others use 1,000, and + gave name lookups 500 or 1,000 ms where the others give 1,500. Without a + `timeout` each step now keeps its usual default. + - The MCP tool list advertised defaults the server did not use. DNS + lookups claimed an `A` record and look up every type, discover and + sweep claimed `192.168.1.0/24` and use your own network, and background + captures claimed `capture.pcap` and name the file after the job. The + tool list now says what happens. + - Ping count tops out at 256 everywhere. The desktop app stopped at 50 or + 64, and the TUI had no limit at all. + - Desktop inspect ignored the concurrency setting in Preferences. It now + uses it, like scan, discover and sweep. + - A desktop packet capture lasts 10 seconds by default, as on the CLI and + MCP server, instead of 5. + - **Discovery reads the Windows device table directly** instead of running `arp -a` and parsing its text. On an idle machine that makes no measurable difference (`arp -a` took 65 ms), but it removes a program start from every diff --git a/apps/netscli-cli/src/tui/events/host.rs b/apps/netscli-cli/src/tui/events/host.rs index c1ed9744..db8be3e1 100644 --- a/apps/netscli-cli/src/tui/events/host.rs +++ b/apps/netscli-cli/src/tui/events/host.rs @@ -14,7 +14,7 @@ pub(super) async fn handle_ping( .get(2) .and_then(|s| s.parse::().ok()) .unwrap_or(4) - .max(1); + .clamp(1, netscli_core::MAX_PING_COUNT); let ip = match ops.resolve_host_ip(host).await { Ok(ip) => ip, diff --git a/apps/netscli-gui/src-tauri/src/commands/operations/scan.rs b/apps/netscli-gui/src-tauri/src/commands/operations/scan.rs index ff7e296e..cc1cae80 100644 --- a/apps/netscli-gui/src-tauri/src/commands/operations/scan.rs +++ b/apps/netscli-gui/src-tauri/src/commands/operations/scan.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use netscli_core::{parse_ports_checked, Ops}; +use netscli_core::{parse_ports_checked, Ops, MAX_PING_COUNT}; use super::{ emit_operation_progress, ops_with_concurrency, run_json_operation, JsonResult, @@ -17,7 +17,7 @@ pub(crate) async fn ping_host( ) -> JsonResult { run_json_operation(op_id, manager, None, move || async move { let ops = Ops::default(); - let count = count.unwrap_or(4).clamp(1, 64); + let count = count.unwrap_or(4).clamp(1, MAX_PING_COUNT); let res = ops .ping_host_summary(host.trim(), count) .await @@ -184,10 +184,11 @@ pub(crate) async fn inspect_host_cmd( op_id: Option, host: String, ports: Option, + max_concurrent: Option, manager: tauri::State<'_, OperationManager>, ) -> JsonResult { run_json_operation(op_id, manager, None, move || async move { - let ops = Ops::default(); + let ops = ops_with_concurrency(max_concurrent); let ports = parse_ports_checked(ports.as_deref()).map_err(|e| e.to_string())?; let res = ops .inspect_host(host, ports) diff --git a/apps/netscli-gui/src/services/netscli.ts b/apps/netscli-gui/src/services/netscli.ts index c0fee50a..991cf342 100644 --- a/apps/netscli-gui/src/services/netscli.ts +++ b/apps/netscli-gui/src/services/netscli.ts @@ -78,8 +78,9 @@ export async function inspectHost( host: string, ports?: string, op_id?: string, + max_concurrent?: number, ): Promise { - return invoke('inspect_host_cmd', { opId: op_id, host, ports }); + return invoke('inspect_host_cmd', { opId: op_id, host, ports, maxConcurrent: max_concurrent }); } export async function sweepNetwork( diff --git a/apps/netscli-gui/src/tools/presentation.test.ts b/apps/netscli-gui/src/tools/presentation.test.ts index 4043e398..b034f7c9 100644 --- a/apps/netscli-gui/src/tools/presentation.test.ts +++ b/apps/netscli-gui/src/tools/presentation.test.ts @@ -68,7 +68,7 @@ describe('buildCommand', () => { it('builds command previews for every tool', () => { const scan = createTab('scan'); scan.form.host = '1.1.1.1'; - expect(buildCommand(scan)).toBe('netscli scan 1.1.1.1 -p 22,80,443,8080,8443 --json'); + expect(buildCommand(scan)).toBe('netscli scan 1.1.1.1 -p 22,80,443 --json'); const discover = createTab('discover'); discover.form.subnet = '192.168.1.0/24'; diff --git a/apps/netscli-gui/src/tools/registry.ts b/apps/netscli-gui/src/tools/registry.ts index 706aa4f8..b1799c6e 100644 --- a/apps/netscli-gui/src/tools/registry.ts +++ b/apps/netscli-gui/src/tools/registry.ts @@ -14,7 +14,9 @@ import { } from './operationIcons'; import type { DetailTab, ToolCapabilityMap, ToolConfig, ToolKind, WorkspaceTab } from './types'; -export const DEFAULT_PORTS = '22,80,443,8080,8443'; +// The core's default list (netscli_core::DEFAULT_PORTS), so a scan started +// here checks the same ports as one from the CLI, TUI or MCP server. +export const DEFAULT_PORTS = '22,80,443'; export const TOOL_KINDS: ToolKind[] = [ 'scan', @@ -71,7 +73,7 @@ export const TOOL_CONFIG: Record = { action: 'Ping', fields: [ { key: 'host', label: 'Host', placeholder: '127.0.0.1', required: true }, - { key: 'count', label: 'Count', type: 'number', compact: true, placeholder: '4', min: 1, max: 50, step: 1 }, + { key: 'count', label: 'Count', type: 'number', compact: true, placeholder: '4', min: 1, max: 256, step: 1 }, ], }, trace: { @@ -179,7 +181,7 @@ export const TOOL_CONFIG: Record = { fields: [ { key: 'mode', label: 'Mode', type: 'select', compact: true, options: ['Capture', 'Open File'] }, { key: 'interface', label: 'Interface', type: 'select', placeholder: 'Select interface', required: true }, - { key: 'duration', label: 'Seconds', type: 'number', compact: true, placeholder: '5', min: 1, max: 3600, step: 1 }, + { key: 'duration', label: 'Seconds', type: 'number', compact: true, placeholder: '10', min: 1, max: 3600, step: 1 }, { key: 'filter', label: 'Filter', placeholder: 'tcp port 443' }, { key: 'max_packets', label: 'Packets', type: 'number', compact: true, placeholder: '1000', min: 1, max: 100000, step: 1 }, ], @@ -194,11 +196,11 @@ export const DEFAULT_FORM: Record> = { dns: { host: 'netscli.com', record: 'ALL' }, reverse: { ip: '127.0.0.1' }, inspect: { host: '127.0.0.1', ports: DEFAULT_PORTS }, - sweep: { subnet: '', ports: '22,80,443' }, + sweep: { subnet: '', ports: DEFAULT_PORTS }, mdns: { timeout_ms: '3000', service_types: '' }, interfaces: {}, arp: {}, - pcap: { mode: 'Capture', interface: '', duration: '5', filter: '', max_packets: '1000' }, + pcap: { mode: 'Capture', interface: '', duration: '10', filter: '', max_packets: '1000' }, }; export const DEFAULT_SORT: Record = { diff --git a/apps/netscli-gui/src/workspace/toolExecution.test.ts b/apps/netscli-gui/src/workspace/toolExecution.test.ts index e579ffb7..4725f4e8 100644 --- a/apps/netscli-gui/src/workspace/toolExecution.test.ts +++ b/apps/netscli-gui/src/workspace/toolExecution.test.ts @@ -135,7 +135,7 @@ describe('executeTool port scan protocol', () => { const netscli = await import('../services/netscli'); const tab = createTab('scan'); await executeTool(tab, 'op-1', 256); - expect(netscli.scanPorts).toHaveBeenLastCalledWith('127.0.0.1', '22,80,443,8080,8443', 'op-1', 256, false); + expect(netscli.scanPorts).toHaveBeenLastCalledWith('127.0.0.1', '22,80,443', 'op-1', 256, false); }); it('scans UDP when the switch says so, with the ports in the field', async () => { diff --git a/apps/netscli-gui/src/workspace/toolExecution.ts b/apps/netscli-gui/src/workspace/toolExecution.ts index 71180432..62ed9510 100644 --- a/apps/netscli-gui/src/workspace/toolExecution.ts +++ b/apps/netscli-gui/src/workspace/toolExecution.ts @@ -86,7 +86,7 @@ export async function executeTool( case 'inspect': return { kind: 'inspect', - data: await netscli.inspectHost(tab.form.host.trim(), emptyToUndefined(tab.form.ports), opId), + data: await netscli.inspectHost(tab.form.host.trim(), emptyToUndefined(tab.form.ports), opId, maxConcurrentProbes), }; case 'sweep': return { diff --git a/crates/netscli-mcp/src/server/operations.rs b/crates/netscli-mcp/src/server/operations.rs index 9ed83472..c0142605 100644 --- a/crates/netscli-mcp/src/server/operations.rs +++ b/crates/netscli-mcp/src/server/operations.rs @@ -47,16 +47,32 @@ fn resolve_and_check( Ok(subnet) } -pub(super) async fn op_discover(p: DiscoverParams) -> Result, RpcError> { - let subnet = resolved_subnet(p.subnet)?; - let concurrency = clamp_concurrency(p.max_concurrent, netscli_core::DEFAULT_CONCURRENCY); - let timeout_ms = clamp_timeout_ms(p.timeout, netscli_core::DEFAULT_PING_TIMEOUT_MS); - let cfg = netscli_core::OpsConfig { +/// The settings for one operation: the client's concurrency, and its +/// `timeout` applied to every phase if it gave one. +/// +/// Without a `timeout` each phase keeps the core default (ping 1,000 ms, +/// scan 500 ms, DNS 1,500 ms), the same as the CLI, TUI and desktop app. +/// Before, the tool's advertised default stood in for all of them, so an MCP +/// inspect or sweep pinged with 500 ms where the others used 1,000, and a +/// discover gave reverse DNS 1,000 ms where the others gave 1,500. +fn ops_config(concurrency: usize, timeout: Option) -> netscli_core::OpsConfig { + let mut cfg = netscli_core::OpsConfig { concurrency, - ping_timeout_ms: timeout_ms, - dns_timeout_ms: timeout_ms, ..Default::default() }; + if timeout.is_some() { + let ms = clamp_timeout_ms(timeout, 0); + cfg.ping_timeout_ms = ms; + cfg.scan_timeout_ms = ms; + cfg.dns_timeout_ms = ms; + } + cfg +} + +pub(super) async fn op_discover(p: DiscoverParams) -> Result, RpcError> { + let subnet = resolved_subnet(p.subnet)?; + let concurrency = clamp_concurrency(p.max_concurrent, netscli_core::DEFAULT_CONCURRENCY); + let cfg = ops_config(concurrency, p.timeout); let ops = netscli_core::Ops::new(cfg); let (_subnet, hosts) = ops .discover_ipv4(Some(subnet), p.resolve_hostnames.unwrap_or(false)) @@ -70,12 +86,7 @@ pub(super) async fn op_scan_ports( ) -> Result, RpcError> { let ports = normalize_ports(p.ports)?; let concurrency = clamp_concurrency(p.max_concurrent, netscli_core::DEFAULT_CONCURRENCY); - let timeout_ms = clamp_timeout_ms(p.timeout, netscli_core::DEFAULT_SCAN_TIMEOUT_MS); - let cfg = netscli_core::OpsConfig { - concurrency, - scan_timeout_ms: timeout_ms, - ..Default::default() - }; + let cfg = ops_config(concurrency, p.timeout); // Scan the address that was checked, not the name that produced it -- // see `ensure_host_allowed` for why the two can differ. let ip = ensure_host_allowed(&p.host, netscli_core::DEFAULT_DNS_TIMEOUT_MS).await?; @@ -111,13 +122,7 @@ pub(super) async fn op_inspect_host( ) -> Result { let ports = normalize_ports(p.ports)?; let concurrency = clamp_concurrency(p.max_concurrent, netscli_core::DEFAULT_CONCURRENCY); - let timeout_ms = clamp_timeout_ms(p.timeout, netscli_core::DEFAULT_SCAN_TIMEOUT_MS); - let cfg = netscli_core::OpsConfig { - concurrency, - scan_timeout_ms: timeout_ms, - ping_timeout_ms: timeout_ms, - dns_timeout_ms: timeout_ms, - }; + let cfg = ops_config(concurrency, p.timeout); let ops = netscli_core::Ops::new(cfg); let ip = ensure_host_allowed(&p.host, netscli_core::DEFAULT_DNS_TIMEOUT_MS).await?; let mut result = ops @@ -137,13 +142,7 @@ pub(super) async fn op_sweep(p: SweepParams) -> Result Result Result, RpcError> { // Clamp timeout: no point waiting more than 30s for an interactive-like // tool call, and 0/None means use the 3s default. - let timeout_ms = p.timeout_ms.unwrap_or(3000).clamp(100, 30_000); + let timeout_ms = p + .timeout_ms + .unwrap_or(3000) + .clamp(100, netscli_core::MAX_MDNS_TIMEOUT_MS); let service_types = p.service_types.unwrap_or_default(); let ops = netscli_core::Ops::default(); ops.discover_mdns(&service_types, std::time::Duration::from_millis(timeout_ms)) @@ -241,6 +238,25 @@ mod tests { assert!(!worth_reporting(PortStatus::Filtered)); } + #[test] + fn without_a_timeout_each_phase_keeps_the_core_default() { + let cfg = ops_config(64, None); + assert_eq!(cfg.concurrency, 64); + assert_eq!(cfg.ping_timeout_ms, netscli_core::DEFAULT_PING_TIMEOUT_MS); + assert_eq!(cfg.scan_timeout_ms, netscli_core::DEFAULT_SCAN_TIMEOUT_MS); + assert_eq!(cfg.dns_timeout_ms, netscli_core::DEFAULT_DNS_TIMEOUT_MS); + } + + #[test] + fn a_timeout_from_the_client_applies_to_every_phase_within_bounds() { + let cfg = ops_config(64, Some(2_000)); + assert_eq!( + (cfg.ping_timeout_ms, cfg.scan_timeout_ms, cfg.dns_timeout_ms), + (2_000, 2_000, 2_000) + ); + assert_eq!(ops_config(64, Some(1)).scan_timeout_ms, 10); + } + #[test] fn a_publicly_addressed_interface_is_refused_rather_than_scanned() { // The case this whole change exists for. Before, the default was diff --git a/crates/netscli-mcp/src/server/tools.rs b/crates/netscli-mcp/src/server/tools.rs index e2e1494b..8a929020 100644 --- a/crates/netscli-mcp/src/server/tools.rs +++ b/crates/netscli-mcp/src/server/tools.rs @@ -10,11 +10,10 @@ pub fn tools_list() -> serde_json::Value { "properties": { "subnet": { "type": "string", - "default": "192.168.1.0/24", "description": "IPv4 CIDR, at most a /16. Defaults to the local subnet." }, "resolveHostnames": { "type": "boolean", "default": false }, - "timeout": { "type": "number", "default": 1000, "minimum": 10, "maximum": 600000 }, + "timeout": { "type": "number", "minimum": 10, "maximum": 600000, "description": "Milliseconds, applied to every step. Omit for the defaults: ping 1000, scan 500, DNS 1500." }, "maxConcurrent": { "type": "number", "default": 256, "minimum": 1, "maximum": 1024 } } }, @@ -76,7 +75,7 @@ pub fn tools_list() -> serde_json::Value { "host": { "type": "string" }, "type": { "type": "string", - "default": "A", + "description": "Record type. Omit, or pass ALL, for every type.", "enum": ["A", "AAAA", "CNAME", "MX", "NS", "TXT", "SRV", "PTR", "SOA", "CAA", "ALL", "ANY"] } }, @@ -113,7 +112,7 @@ pub fn tools_list() -> serde_json::Value { "items": { "type": "number", "minimum": 1, "maximum": 65535 }, "maxItems": 4096 }, - "timeout": { "type": "number", "default": 500, "minimum": 10, "maximum": 600000 }, + "timeout": { "type": "number", "minimum": 10, "maximum": 600000, "description": "Milliseconds, applied to every step. Omit for the defaults: ping 1000, scan 500, DNS 1500." }, "include_closed": { "type": "boolean", "default": false, "description": "Also list closed and filtered ports in ports, one entry each." }, "maxConcurrent": { "type": "number", "default": 256, "minimum": 1, "maximum": 1024 } }, @@ -131,10 +130,10 @@ pub fn tools_list() -> serde_json::Value { "inputSchema": { "type": "object", "properties": { - "subnet": { "type": "string", "default": "192.168.1.0/24" }, + "subnet": { "type": "string", "description": "IPv4 CIDR, at most a /16. Defaults to the local subnet." }, "ports": { "type": "array", "items": { "type": "number" } }, "resolveHostnames": { "type": "boolean", "default": false }, - "timeout": { "type": "number", "default": 500 }, + "timeout": { "type": "number", "minimum": 10, "maximum": 600000, "description": "Milliseconds, applied to every step. Omit for the defaults: ping 1000, scan 500, DNS 1500." }, "maxConcurrent": { "type": "number", "default": 256 } } }, @@ -191,7 +190,7 @@ pub fn tools_list() -> serde_json::Value { "interface": { "type": "string" }, "filter": { "type": "string" }, "duration": { "type": "number", "default": 10 }, - "outputFile": { "type": "string", "default": "capture.pcap" }, + "outputFile": { "type": "string", "description": "Where to write the capture. Omit for a file named after the job." }, "maxPackets": { "type": "number" } }, "required": ["interface"]