From e846aa9312b96097dea92dd464e3012d783fd08f Mon Sep 17 00:00:00 2001 From: Sev7eNup <79143581+Sev7eNup@users.noreply.github.com> Date: Wed, 4 Nov 2026 22:41:50 +0100 Subject: [PATCH] Fix CLI, WMI and designer bugs from the October audit - forEach parallelism 0 is saved as 0 instead of 1 (#602). - WMI method arguments are converted to the declared CIM types, so a UInt32 above Int32.MaxValue works; out-of-range values fail with the argument name (#616). - np auth login --password-stdin keeps leading and trailing spaces (#617). - np backup manifest/preview/restore honour -o json and -o yaml (#618). - np machine create --ssl and the MCP create_machine tool default to port 5986 with SSL (#619). - The CLI YAML output reads back unchanged: escaped multiline strings, [] and {}, quoted null-like words (#621). - Clipboard copy and server export share one redaction fixture (#574). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 11 +++ .../Commands/Auth/AuthCommands.cs | 3 +- .../Commands/Backup/BackupCommands.cs | 33 ++++--- .../Commands/Machines/MachineCommands.cs | 5 +- src/NodePilot.Cli/Output/YamlEmitter.cs | 87 +++++++++++++---- .../Activities/WmiQueryActivity.cs | 45 ++++++++- .../Tools/SupportingDataTools.cs | 5 +- src/nodepilot-docs-ui/content/de/cli.md | 2 +- src/nodepilot-docs-ui/content/en/cli.md | 2 +- .../propertyConfigs.activities.test.tsx | 22 +++++ .../fixtures/workflow-sharing-redaction.json | 96 +++++++++++++++++++ .../__tests__/lib/workflowFragment.test.ts | 21 ++++ .../properties/activities/ForEachConfig.tsx | 6 +- .../WorkflowSecretRedactorTests.cs | 44 +++++++++ .../CommandIntegrationCoverageTests.cs | 94 ++++++++++++++++++ .../CommandIntegrationResourcesTests.cs | 45 +++++++++ .../Commands/CommandIntegrationTests.cs | 28 ++++++ .../Infra/YamlRoundTrip.cs | 67 +++++++++++++ .../NodePilot.Cli.Tests.csproj | 2 + .../Settings/OutputFormatTests.cs | 52 ++++++++++ .../Activities/WmiQueryActivityTests.cs | 55 ++++++++++- .../Tools/MoreToolsCoverageTests.cs | 22 +++++ 22 files changed, 700 insertions(+), 47 deletions(-) create mode 100644 src/nodepilot-ui/src/__tests__/fixtures/workflow-sharing-redaction.json create mode 100644 tests/NodePilot.Cli.Tests/Infra/YamlRoundTrip.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b45bf57..8ef0dddf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ Planned version: **1.5.4**. suggests ordinary domain users with delegation (#705). - The step details of a past run label the workflow's current configuration as such and show the target machine the run actually used (#603). + - A `forEach` parallelism of 0 is saved as 0 (parallel up to the engine limit) instead of 1 (#602). - After a reconnect or a reload, the designer shows a paused debug step with its variables again and catches up on steps that changed during the disconnect (#606). - Variable edits at a breakpoint belong to that one pause and are dropped when another step @@ -73,6 +74,9 @@ Planned version: **1.5.4**. - Input values that look like `{{…}}` stay plain text and no longer fail the step as unresolved references (#686). - Editing a text file keeps its permissions (#683). + - `wmiQuery` method calls pass their arguments in the types the CIM method declares, so a UInt32 + above 2147483647 such as `hDefKey` 2147483649 (HKCU) works. A value outside the declared range + fails the step with a message that names the argument (#616). - **Notifications** - A run that finishes while the database is briefly unavailable still triggers its notification rules once the commit arrives, even when other runs finished later (#678). @@ -84,6 +88,13 @@ Planned version: **1.5.4**. - The MCP server rejects broken nodes and edges before it merges a workflow change (#726). - All MCP requests, including dependency queries, wait until the server connection is ready (#727). + - `np auth login --password-stdin` sends the password as read, including leading and trailing + spaces, so the CLI and the Engine Switcher sign in with such passwords again (#617). + - `np backup manifest`, `preview` and `restore` honour `-o json` and `-o yaml` (#618). + - `np machine create --ssl` without `--port` and the MCP tool `create_machine` with `useSsl` and no + port use the HTTPS port 5986 (#619). + - `-o yaml` keeps multiline strings, empty lists and objects, and strings such as `null`, `yes` + or a date as they are when the output is parsed again (#621). - **Setup and uninstall** - A server uninstall whose cleanup fails stops before the "Apps & features" entry is removed, keeps the installation marker and the deploy scripts and exits with a non-zero code, so the diff --git a/src/NodePilot.Cli/Commands/Auth/AuthCommands.cs b/src/NodePilot.Cli/Commands/Auth/AuthCommands.cs index c6a80278..8152c048 100644 --- a/src/NodePilot.Cli/Commands/Auth/AuthCommands.cs +++ b/src/NodePilot.Cli/Commands/Auth/AuthCommands.cs @@ -94,7 +94,8 @@ protected override async Task ExecuteAsync(CommandContext context, LoginSet var username = settings.Username ?? await AnsiConsole.AskAsync("Username:"); string password; if (settings.PasswordStdin) - password = ((await Console.In.ReadLineAsync()) ?? "").Trim(); + // ReadLine already drops the line ending; spaces belong to the password. + password = (await Console.In.ReadLineAsync()) ?? ""; else if (!string.IsNullOrEmpty(settings.Password)) password = settings.Password; else diff --git a/src/NodePilot.Cli/Commands/Backup/BackupCommands.cs b/src/NodePilot.Cli/Commands/Backup/BackupCommands.cs index 800ba76c..cab1ebfb 100644 --- a/src/NodePilot.Cli/Commands/Backup/BackupCommands.cs +++ b/src/NodePilot.Cli/Commands/Backup/BackupCommands.cs @@ -54,10 +54,13 @@ protected override async Task RunAsync(CommandContext _, GlobalSettings set var api = ClientFactory.Create(session); var manifest = await api.GetBackupManifestAsync(ct); - var table = new Table().AddColumn("Section").AddColumn(new TableColumn("Count").RightAligned()); - foreach (var s in manifest.Sections) - table.AddRow(Markup.Escape(s.Section), s.Count.ToString()); - AnsiConsole.Write(table); + writer.WriteData(manifest, (console, value) => + { + var table = new Table().AddColumn("Section").AddColumn(new TableColumn("Count").RightAligned()); + foreach (var s in value.Sections) + table.AddRow(Markup.Escape(s.Section), s.Count.ToString()); + console.Write(table); + }); return ExitCodes.Success; } } @@ -153,10 +156,13 @@ protected override async Task RunAsync(CommandContext _, BackupPreviewSetti var result = await api.PreviewBackupAsync(content, passphrase, ct); writer.Info($"Backup app version: {result.AppVersion ?? "?"} — integrity [green]verified[/]"); - var table = new Table().AddColumn("Section").AddColumn("In backup").AddColumn("New").AddColumn("Conflicts"); - foreach (var s in result.Sections) - table.AddRow(Markup.Escape(s.Section), s.InBackup.ToString(), s.New.ToString(), s.Conflicts.ToString()); - AnsiConsole.Write(table); + writer.WriteData(result, (console, value) => + { + var table = new Table().AddColumn("Section").AddColumn("In backup").AddColumn("New").AddColumn("Conflicts"); + foreach (var s in value.Sections) + table.AddRow(Markup.Escape(s.Section), s.InBackup.ToString(), s.New.ToString(), s.Conflicts.ToString()); + console.Write(table); + }); foreach (var w in result.Warnings) writer.Warning($" ! {w}"); return ExitCodes.Success; } @@ -212,10 +218,13 @@ protected override async Task RunAsync(CommandContext _, BackupRestoreSetti var content = await File.ReadAllBytesAsync(settings.File, ct); var result = await api.RestoreBackupAsync(content, passphrase, settings.Policy, ct); - var table = new Table().AddColumn("Section").AddColumn("Created").AddColumn("Overwritten").AddColumn("Skipped").AddColumn("Renamed"); - foreach (var s in result.Sections) - table.AddRow(Markup.Escape(s.Section), s.Created.ToString(), s.Overwritten.ToString(), s.Skipped.ToString(), s.Renamed.ToString()); - AnsiConsole.Write(table); + writer.WriteData(result, (console, value) => + { + var table = new Table().AddColumn("Section").AddColumn("Created").AddColumn("Overwritten").AddColumn("Skipped").AddColumn("Renamed"); + foreach (var s in value.Sections) + table.AddRow(Markup.Escape(s.Section), s.Created.ToString(), s.Overwritten.ToString(), s.Skipped.ToString(), s.Renamed.ToString()); + console.Write(table); + }); if (result.Settings is not null) writer.Info($"Settings: {(result.Settings.Applied ? "[green]applied[/]" : "[yellow]not applied[/]")} — {Markup.Escape(result.Settings.Message ?? "")}"); foreach (var w in result.Warnings) writer.Warning($" ! {w}"); diff --git a/src/NodePilot.Cli/Commands/Machines/MachineCommands.cs b/src/NodePilot.Cli/Commands/Machines/MachineCommands.cs index 67dcd20c..a2c2da4a 100644 --- a/src/NodePilot.Cli/Commands/Machines/MachineCommands.cs +++ b/src/NodePilot.Cli/Commands/Machines/MachineCommands.cs @@ -54,7 +54,7 @@ public class MachineWriteSettings : GlobalSettings public string? Hostname { get; set; } [CommandOption("--port ")] - [Description("WinRM port (default 5985 on create).")] + [Description("WinRM port (default on create: 5985, or 5986 with --ssl).")] public int? Port { get; set; } [CommandOption("--ssl")] @@ -83,9 +83,10 @@ protected override async Task RunAsync(CommandContext _, MachineWriteSettin } var api = ClientFactory.Create(session); + var useSsl = settings.UseSsl ?? false; var req = new CreateMachineRequest( settings.Name, settings.Hostname, - settings.Port ?? 5985, settings.UseSsl ?? false, + settings.Port ?? (useSsl ? 5986 : 5985), useSsl, settings.CredentialId, settings.Tags); var created = await api.CreateMachineAsync(req, ct); writer.Success($"Machine created: [bold]{Markup.Escape(created.Name)}[/] ({created.Id})."); diff --git a/src/NodePilot.Cli/Output/YamlEmitter.cs b/src/NodePilot.Cli/Output/YamlEmitter.cs index 3fa098d4..2e44975c 100644 --- a/src/NodePilot.Cli/Output/YamlEmitter.cs +++ b/src/NodePilot.Cli/Output/YamlEmitter.cs @@ -7,8 +7,8 @@ namespace NodePilot.Cli.Output; /// /// Minimal YAML emitter for CLI output: a human-readable, line-oriented rendering of API -/// responses only, with no round-trip parsing. Kept in-house to avoid pulling in YamlDotNet -/// just for `--output yaml`. +/// responses whose values read back unchanged with a YAML parser. Kept in-house to avoid +/// pulling in YamlDotNet just for `--output yaml`. /// public static class YamlEmitter { @@ -21,54 +21,71 @@ public static string Emit(T value) return sb.ToString(); } + private static readonly char[] IndicatorChars = + [':', '#', '"', '\'', '{', '}', '[', ']', ',', '&', '*', '!', '|', '>', '%', '@', '`']; + + // Plain words that YAML 1.1 or 1.2 resolves to null or a boolean instead of a string. + private static readonly HashSet NonStringWords = new(StringComparer.OrdinalIgnoreCase) + { "null", "true", "false", "yes", "no", "on", "off", "y", "n" }; + private static void Write(JsonElement el, StringBuilder sb, int indent, bool isListItem) { var pad = new string(' ', indent); switch (el.ValueKind) { - case JsonValueKind.Object: + case JsonValueKind.Object when el.EnumerateObject().Any(): bool first = true; foreach (var p in el.EnumerateObject()) { var prefix = first && isListItem ? "" : pad; - if (IsScalar(p.Value)) + if (IsInline(p.Value)) { - sb.Append(prefix).Append(p.Name).Append(": ").AppendLine(Scalar(p.Value)); + sb.Append(prefix).Append(Quote(p.Name)).Append(": ").AppendLine(Inline(p.Value)); } else { - sb.Append(prefix).Append(p.Name).AppendLine(":"); + sb.Append(prefix).Append(Quote(p.Name)).AppendLine(":"); Write(p.Value, sb, indent + 2, isListItem: false); } first = false; } break; - case JsonValueKind.Array: + case JsonValueKind.Array when el.GetArrayLength() > 0: + bool firstItem = true; foreach (var item in el.EnumerateArray()) { - if (IsScalar(item)) + var prefix = firstItem && isListItem ? "" : pad; + if (IsInline(item)) { - sb.Append(pad).Append("- ").AppendLine(Scalar(item)); + sb.Append(prefix).Append("- ").AppendLine(Inline(item)); } else { - sb.Append(pad).Append("- "); + sb.Append(prefix).Append("- "); Write(item, sb, indent + 2, isListItem: true); } + firstItem = false; } break; default: - sb.Append(pad).AppendLine(Scalar(el)); + // Only the root gets here: nested scalars and empty collections are written inline. + sb.AppendLine(Inline(el)); break; } } - private static bool IsScalar(JsonElement el) => el.ValueKind - is JsonValueKind.String or JsonValueKind.Number or JsonValueKind.True - or JsonValueKind.False or JsonValueKind.Null; + /// Scalars and empty collections fit on the line of their key or list dash. + private static bool IsInline(JsonElement el) => el.ValueKind switch + { + JsonValueKind.Object => !el.EnumerateObject().Any(), + JsonValueKind.Array => el.GetArrayLength() == 0, + _ => true, + }; - private static string Scalar(JsonElement el) => el.ValueKind switch + private static string Inline(JsonElement el) => el.ValueKind switch { + JsonValueKind.Object => "{}", + JsonValueKind.Array => "[]", JsonValueKind.String => Quote(el.GetString() ?? ""), JsonValueKind.Number => el.GetRawText(), JsonValueKind.True => "true", @@ -79,11 +96,39 @@ is JsonValueKind.String or JsonValueKind.Number or JsonValueKind.True private static string Quote(string s) { - if (s.Length == 0) return "''"; - bool needsQuote = s.IndexOfAny(new[] { ':', '#', '\n', '\r', '"', '\'', '{', '}', '[', ']', ',', '&', '*', '!', '|', '>', '%', '@', '`' }) >= 0 - || char.IsWhiteSpace(s[0]) || char.IsWhiteSpace(s[^1]) - || bool.TryParse(s, out _) || double.TryParse(s, NumberStyles.Any, CultureInfo.InvariantCulture, out _); - if (!needsQuote) return s; - return "\"" + s.Replace("\\", "\\\\").Replace("\"", "\\\"") + "\""; + if (CanStayPlain(s)) return s; + // Double-quoted with escapes: a physical line break inside quotes would fold into a space. + var sb = new StringBuilder(s.Length + 2).Append('"'); + foreach (var c in s) + { + switch (c) + { + case '\\': sb.Append("\\\\"); break; + case '"': sb.Append("\\\""); break; + case '\n': sb.Append("\\n"); break; + case '\r': sb.Append("\\r"); break; + case '\t': sb.Append("\\t"); break; + default: + if (NeedsEscape(c)) + sb.Append("\\u").Append(((int)c).ToString("X4", CultureInfo.InvariantCulture)); + else + sb.Append(c); + break; + } + } + return sb.Append('"').ToString(); } + + /// + /// A string stays unquoted only if YAML cannot read it as anything else: it starts with a + /// letter (so no number, date, ~, - or indicator), has no indicator, control + /// character or trailing space, and is not a null or boolean word. + /// + private static bool CanStayPlain(string s) => + s.Length > 0 && char.IsLetter(s[0]) && !char.IsWhiteSpace(s[^1]) + && s.IndexOfAny(IndicatorChars) < 0 && !s.Any(NeedsEscape) + && !NonStringWords.Contains(s); + + // Control characters, the Unicode line separators YAML treats as breaks, and a BOM. + private static bool NeedsEscape(char c) => char.IsControl(c) || c is (char)0x2028 or (char)0x2029 or (char)0xFEFF; } diff --git a/src/NodePilot.Engine/Activities/WmiQueryActivity.cs b/src/NodePilot.Engine/Activities/WmiQueryActivity.cs index de2bcb1f..36c56502 100644 --- a/src/NodePilot.Engine/Activities/WmiQueryActivity.cs +++ b/src/NodePilot.Engine/Activities/WmiQueryActivity.cs @@ -274,10 +274,23 @@ private static string BuildInvokeMethodScript(JsonElement config, string qNs) var qClass = PowerShellQuoter.Literal(className); var qMethod = PowerShellQuoter.Literal(methodName); var filter = config.GetStringOrNull("filter"); + var hasFilter = !string.IsNullOrWhiteSpace(filter); var argsLiteral = BuildArgumentsHashtable(config); var sb = new StringBuilder(); - if (!string.IsNullOrWhiteSpace(filter)) + // Invoke-CimMethod converts arguments to the declared CIM types only when it has the class + // declaration. With -ClassName it sends the PowerShell types, and a UInt32 parameter + // rejects the Int64 that PowerShell uses for values above Int32.MaxValue. + if (!hasFilter || argsLiteral is not null) + sb.Append("$__npClass = Get-CimClass -ClassName ").Append(qClass) + .Append(" -Namespace ").Append(qNs).Append('\n'); + if (argsLiteral is not null) + { + sb.Append("$__npArgs = ").Append(argsLiteral).Append('\n'); + AppendIntegerArgumentRangeCheck(sb, qMethod); + } + + if (hasFilter) { // Instance method: locate target instances first, then pipe into Invoke-CimMethod. // Without -Filter the cmdlet would target the class itself (static-method semantics), @@ -291,17 +304,39 @@ private static string BuildInvokeMethodScript(JsonElement config, string qNs) else { // Static method on the class. - sb.Append("Invoke-CimMethod -ClassName ").Append(qClass) - .Append(" -Namespace ").Append(qNs) - .Append(" -MethodName ").Append(qMethod); + sb.Append("Invoke-CimMethod -CimClass $__npClass -MethodName ").Append(qMethod); } if (argsLiteral is not null) - sb.Append(" -Arguments ").Append(argsLiteral); + sb.Append(" -Arguments $__npArgs"); return sb.ToString(); } + /// + /// Rejects an argument whose value does not fit the integer CIM type its parameter declares, + /// naming the argument and the allowed range. The CIM conversion itself only reports an + /// overflow without saying which argument caused it. + /// + private static void AppendIntegerArgumentRangeCheck(StringBuilder sb, string qMethod) + { + sb.Append("$__npMethod = $__npClass.CimClassMethods[").Append(qMethod).Append("]\n"); + sb.Append("if ($null -ne $__npMethod) {\n"); + sb.Append(" $__npIntTypes = @{ UInt8 = [byte]; SInt8 = [sbyte]; UInt16 = [uint16]; SInt16 = [int16]; UInt32 = [uint32]; SInt32 = [int32]; UInt64 = [uint64]; SInt64 = [int64] }\n"); + sb.Append(" foreach ($__npName in @($__npArgs.Keys)) {\n"); + sb.Append(" $__npParam = $__npMethod.Parameters[$__npName]\n"); + sb.Append(" $__npValue = $__npArgs[$__npName]\n"); + sb.Append(" if ($null -eq $__npParam -or $null -eq $__npValue) { continue }\n"); + sb.Append(" $__npType = $__npIntTypes[[string]$__npParam.CimType]\n"); + sb.Append(" if ($null -eq $__npType) { continue }\n"); + sb.Append(" $__npNumber = [decimal]0\n"); + sb.Append(" if (-not [decimal]::TryParse([string]$__npValue, [Globalization.NumberStyles]::Float, [Globalization.CultureInfo]::InvariantCulture, [ref]$__npNumber) -or $__npNumber -ne [decimal]::Truncate($__npNumber) -or $__npNumber -lt $__npType::MinValue -or $__npNumber -gt $__npType::MaxValue) {\n"); + sb.Append(" throw \"WMI Query: argument '$__npName' value '$__npValue' does not fit the declared CIM type $($__npParam.CimType) (a whole number from $($__npType::MinValue) to $($__npType::MaxValue)).\"\n"); + sb.Append(" }\n"); + sb.Append(" }\n"); + sb.Append("}\n"); + } + /// /// Renders config.arguments as a PowerShell hashtable literal (e.g. /// @{ Name = 'svc'; Count = 5; Force = $true }) or returns null when there diff --git a/src/NodePilot.Mcp/Tools/SupportingDataTools.cs b/src/NodePilot.Mcp/Tools/SupportingDataTools.cs index 985b3f74..6bbb16f1 100644 --- a/src/NodePilot.Mcp/Tools/SupportingDataTools.cs +++ b/src/NodePilot.Mcp/Tools/SupportingDataTools.cs @@ -45,13 +45,14 @@ public async Task GetMachine( public async Task CreateMachine( [Description("Display name.")] string name, [Description("Hostname or IP for WinRM.")] string hostname, - [Description("WinRM port (default 5985).")] int winRmPort = 5985, + [Description("WinRM port (default 5986 with SSL, otherwise 5985).")] int? winRmPort = null, [Description("Use HTTPS/SSL WinRM (default false).")] bool useSsl = false, [Description("Optional default credential GUID.")] string? defaultCredentialId = null, [Description("Optional comma-separated tags.")] string? tags = null, CancellationToken cancellationToken = default) { - var req = new CreateMachineRequest(name, hostname, winRmPort, useSsl, OptGuid(defaultCredentialId, "defaultCredentialId"), tags); + var port = winRmPort ?? (useSsl ? 5986 : 5985); + var req = new CreateMachineRequest(name, hostname, port, useSsl, OptGuid(defaultCredentialId, "defaultCredentialId"), tags); var m = await ApiErrorMapper.Guard(() => _api.CreateMachineAsync(req, cancellationToken)); return new { created = true, machineId = m.Id, name = m.Name }; } diff --git a/src/nodepilot-docs-ui/content/de/cli.md b/src/nodepilot-docs-ui/content/de/cli.md index bff3081d..171c602b 100644 --- a/src/nodepilot-docs-ui/content/de/cli.md +++ b/src/nodepilot-docs-ui/content/de/cli.md @@ -229,7 +229,7 @@ np exec resume 7e3f... --step runHealth --mode stop ## machine -Maschinen-Body via Flags (nicht `--file`). `create`/`update`-Flags: `--name`, `--hostname`, `--port` (Default 5985), `--ssl`, `--credential `, `--tags `. `update` ist Client-seitig Patch (fetch + merge, Server will Full-Body). +Maschinen-Body via Flags (nicht `--file`). `create`/`update`-Flags: `--name`, `--hostname`, `--port` (Default bei `create`: 5985, mit `--ssl` 5986), `--ssl`, `--credential `, `--tags `. `update` ist Client-seitig Patch (fetch + merge, Server will Full-Body). ```bash np machine create --name srv01 --hostname srv01.internal --credential 4c2a... --tags prod,web diff --git a/src/nodepilot-docs-ui/content/en/cli.md b/src/nodepilot-docs-ui/content/en/cli.md index 9bb37ee7..88673943 100644 --- a/src/nodepilot-docs-ui/content/en/cli.md +++ b/src/nodepilot-docs-ui/content/en/cli.md @@ -229,7 +229,7 @@ np exec resume 7e3f... --step runHealth --mode stop ## machine -The machine body comes from flags, not `--file`. `create`/`update` flags: `--name`, `--hostname`, `--port` (default 5985), `--ssl`, `--credential `, `--tags `. `update` is a client-side patch (fetch + merge, because the server wants a full body). +The machine body comes from flags, not `--file`. `create`/`update` flags: `--name`, `--hostname`, `--port` (default on `create`: 5985, or 5986 with `--ssl`), `--ssl`, `--credential `, `--tags `. `update` is a client-side patch (fetch + merge, because the server wants a full body). ```bash np machine create --name srv01 --hostname srv01.internal --credential 4c2a... --tags prod,web diff --git a/src/nodepilot-ui/src/__tests__/components/properties/propertyConfigs.activities.test.tsx b/src/nodepilot-ui/src/__tests__/components/properties/propertyConfigs.activities.test.tsx index 68a04610..75a9f886 100644 --- a/src/nodepilot-ui/src/__tests__/components/properties/propertyConfigs.activities.test.tsx +++ b/src/nodepilot-ui/src/__tests__/components/properties/propertyConfigs.activities.test.tsx @@ -408,6 +408,28 @@ describe('ForEachConfig', () => { expect(onUpdate).toHaveBeenCalledWith({ maxParallelism: 8 }); }); + it('parallelismZero_isStoredAsZero', () => { + function Editor() { + const [config, setConfig] = useState>({ maxParallelism: 4 }); + return <> setConfig((prev) => ({ ...prev, ...patch }))} upstreamVars={[]} />{JSON.stringify(config)}; + } + wrap(); + + fireEvent.change(screen.getByDisplayValue('4'), { target: { value: '0' } }); + + expect(screen.getByText(/"maxParallelism":0/)).toBeInTheDocument(); + expect(screen.getByDisplayValue('0')).toBeInTheDocument(); + }); + + it('parallelismEmpty_fallsBackToOne', () => { + const onUpdate = vi.fn(); + wrap(); + + fireEvent.change(screen.getByDisplayValue('4'), { target: { value: '' } }); + + expect(onUpdate).toHaveBeenCalledWith({ maxParallelism: 1 }); + }); + it('continueOnError_togglesPatch', () => { const onUpdate = vi.fn(); wrap(); diff --git a/src/nodepilot-ui/src/__tests__/fixtures/workflow-sharing-redaction.json b/src/nodepilot-ui/src/__tests__/fixtures/workflow-sharing-redaction.json new file mode 100644 index 00000000..9317a82a --- /dev/null +++ b/src/nodepilot-ui/src/__tests__/fixtures/workflow-sharing-redaction.json @@ -0,0 +1,96 @@ +{ + "//": [ + "Sharing contract for the clipboard copy (lib/workflowFragment.ts serializeFragment) and the", + "server export (WorkflowSecretRedactor.Redact). Both test suites read this file. Paths are JSON", + "pointers into the definition. maskedByBoth: recognised secrets, '***' on both surfaces.", + "maskedByExportOnly: free-form values the export masks whole and the clipboard keeps, so a copy", + "pasted back into NodePilot still works. keptByBoth: unchanged on both surfaces." + ], + "cases": [ + { + "name": "script with a password", + "definition": { + "nodes": [ + { "id": "run", "type": "activity", "position": { "x": 0, "y": 0 }, + "data": { "label": "Rotate", "activityType": "runScript", + "config": { "script": "$password = \"literal-secret\"\nSet-Secret -Value $password" } } } + ], + "edges": [] + }, + "maskedByBoth": ["/nodes/0/data/config/script"], + "maskedByExportOnly": [], + "keptByBoth": ["/nodes/0/data/label", "/nodes/0/data/activityType"] + }, + { + "name": "script without a secret", + "definition": { + "nodes": [ + { "id": "run", "type": "activity", "position": { "x": 0, "y": 0 }, + "data": { "label": "Spooler", "activityType": "runScript", + "config": { "script": "Get-Service -Name Spooler" } } } + ], + "edges": [] + }, + "maskedByBoth": [], + "maskedByExportOnly": ["/nodes/0/data/config/script"], + "keptByBoth": ["/nodes/0/data/label"] + }, + { + "name": "secret config key", + "definition": { + "nodes": [ + { "id": "call", "type": "activity", "position": { "x": 0, "y": 0 }, + "data": { "label": "Call", "activityType": "restApi", + "config": { "method": "GET", "apiKey": "literal-api-key" } } } + ], + "edges": [] + }, + "maskedByBoth": ["/nodes/0/data/config/apiKey"], + "maskedByExportOnly": [], + "keptByBoth": ["/nodes/0/data/config/method"] + }, + { + "name": "literal condition operands", + "definition": { + "nodes": [ + { "id": "a", "type": "activity", "position": { "x": 0, "y": 0 }, + "data": { "label": "Check", "activityType": "log", "config": {} } }, + { "id": "b", "type": "activity", "position": { "x": 200, "y": 0 }, + "data": { "label": "Act", "activityType": "log", "config": {} } } + ], + "edges": [ + { "id": "ab", "source": "a", "target": "b", "type": "labeled", + "data": { "label": "On match", "conditionExpression": { "type": "group", "op": "OR", "children": [ + { "type": "comparison", "op": "==", + "left": { "kind": "variable", "stepId": "a", "field": "output" }, + "right": { "kind": "literal", "value": "Authorization: Bearer literal-token" } }, + { "type": "comparison", "op": "==", + "left": { "kind": "variable", "stepId": "a", "field": "output" }, + "right": { "kind": "literal", "value": "Running" } } + ] } } } + ] + }, + "maskedByBoth": ["/edges/0/data/conditionExpression/children/0/right/value"], + "maskedByExportOnly": ["/edges/0/data/conditionExpression/children/1/right/value"], + "keptByBoth": [ + "/edges/0/data/label", + "/edges/0/data/conditionExpression/children/0/left/stepId", + "/nodes/1/data/label" + ] + }, + { + "name": "secret in a string array", + "definition": { + "nodes": [ + { "id": "mail", "type": "activity", "position": { "x": 0, "y": 0 }, + "data": { "label": "Notify", "activityType": "emailNotification", + "config": { "to": ["ops@example.test", "Authorization: Bearer literal-token"] } } } + ], + "edges": [] + }, + "maskedByBoth": ["/nodes/0/data/config/to/1"], + "maskedByExportOnly": [], + "keptByBoth": ["/nodes/0/data/config/to/0"] + } + ] +} diff --git a/src/nodepilot-ui/src/__tests__/lib/workflowFragment.test.ts b/src/nodepilot-ui/src/__tests__/lib/workflowFragment.test.ts index 0329d4ec..43775f73 100644 --- a/src/nodepilot-ui/src/__tests__/lib/workflowFragment.test.ts +++ b/src/nodepilot-ui/src/__tests__/lib/workflowFragment.test.ts @@ -121,6 +121,27 @@ describe('serializeFragment', () => { expect(fragment.edges[0].data?.conditionExpression).toBe(condition); }); + // Shared with WorkflowSecretRedactorTests on the server side. + const sharingContract = JSON.parse(readFileSync(join(dirname(fileURLToPath(import.meta.url)), + '../fixtures/workflow-sharing-redaction.json'), 'utf8')) as { + cases: Array<{ name: string; definition: WorkflowFragment; maskedByBoth: string[]; maskedByExportOnly: string[]; keptByBoth: string[] }>; + }; + const at = (value: unknown, pointer: string): unknown => pointer.split('/').slice(1) + .reduce((current, key) => (current as Record | undefined)?.[key], value); + + it.each(sharingContract.cases.map((c) => [c.name, c] as const))('sharingContract_%s', (_name, contract) => { + const copied: unknown = JSON.parse(serializeFragment(contract.definition)); + for (const pointer of contract.maskedByBoth) expect(at(copied, pointer), pointer).toBe('***'); + for (const pointer of [...contract.maskedByExportOnly, ...contract.keptByBoth]) { + expect(at(copied, pointer), pointer).toEqual(at(contract.definition, pointer)); + } + // Duplicating inside the app does not pass the clipboard boundary and keeps every value. + const duplicated = remapFragment(contract.definition, { existingGroupIds: NO_GROUPS, dx: 0, dy: 0 }); + for (const pointer of contract.maskedByBoth) { + expect(at(duplicated, pointer), pointer).toEqual(at(contract.definition, pointer)); + } + }); + it('dropsCanvasStateAndRuntimeData', () => { const fragment: WorkflowFragment = { nodes: [node('a', { diff --git a/src/nodepilot-ui/src/components/designer/properties/activities/ForEachConfig.tsx b/src/nodepilot-ui/src/components/designer/properties/activities/ForEachConfig.tsx index d598d318..a360fb8d 100644 --- a/src/nodepilot-ui/src/components/designer/properties/activities/ForEachConfig.tsx +++ b/src/nodepilot-ui/src/components/designer/properties/activities/ForEachConfig.tsx @@ -96,7 +96,11 @@ export function ForEachConfig({ config, onUpdate, upstreamVars = [], onOpenWorkf onUpdate({ maxParallelism: parseInt(e.target.value) || 1 })} + onChange={(e) => { + // 0 is a valid value (parallel up to the engine cap); only empty or invalid input falls back to 1. + const n = parseInt(e.target.value, 10); + onUpdate({ maxParallelism: Number.isNaN(n) ? 1 : n }); + }} className="input-field" min={0} max={64} diff --git a/tests/NodePilot.Ai.Tests/WorkflowSecretRedactorTests.cs b/tests/NodePilot.Ai.Tests/WorkflowSecretRedactorTests.cs index 57ac75b9..8ecb6756 100644 --- a/tests/NodePilot.Ai.Tests/WorkflowSecretRedactorTests.cs +++ b/tests/NodePilot.Ai.Tests/WorkflowSecretRedactorTests.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using System.Text.Json.Nodes; using FluentAssertions; using NodePilot.Core.WorkflowDefinitions; using Xunit; @@ -483,4 +484,47 @@ public void Redact_DefaultMode_StillMasksReadableFields() var cfg = WorkflowSecretRedactor.Redact(SingleNode("runScript", """{ "script": "Get-Date" }"""))["nodes"]![0]!["data"]!["config"]!; cfg["script"]!.GetValue().Should().Be("***"); } + + // The same cases drive the clipboard copy in the frontend (workflowFragment.test.ts). + public static TheoryData SharingContractCaseNames() + { + var names = new TheoryData(); + foreach (var contract in SharingContractCases()) + names.Add(contract["name"]!.GetValue()); + return names; + } + + [Theory] + [MemberData(nameof(SharingContractCaseNames))] + public void Redact_SharingContractCase_MasksSecretsAndFreeFormValues(string caseName) + { + var contract = SharingContractCases().Single(c => c["name"]!.GetValue() == caseName); + var definition = contract["definition"]!; + var redacted = WorkflowSecretRedactor.Redact(Parse(definition.ToJsonString())); + + foreach (var pointer in Pointers(contract, "maskedByBoth").Concat(Pointers(contract, "maskedByExportOnly"))) + At(redacted, pointer)!.GetValue().Should().Be("***", pointer); + foreach (var pointer in Pointers(contract, "keptByBoth")) + JsonNode.DeepEquals(At(redacted, pointer), At(definition, pointer)).Should().BeTrue(pointer); + } + + private static IEnumerable SharingContractCases() + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "NodePilot.slnx"))) + dir = dir.Parent; + if (dir is null) throw new InvalidOperationException($"NodePilot.slnx not found above {AppContext.BaseDirectory}"); + var path = Path.Combine(dir.FullName, "src", "nodepilot-ui", "src", "__tests__", "fixtures", "workflow-sharing-redaction.json"); + return JsonNode.Parse(File.ReadAllText(path))!["cases"]!.AsArray().Select(c => c!); + } + + private static IEnumerable Pointers(JsonNode contract, string group) + => contract[group]!.AsArray().Select(p => p!.GetValue()); + + private static JsonNode? At(JsonNode? node, string pointer) + { + foreach (var segment in pointer.Split('/').Skip(1)) + node = node is JsonArray array ? array[int.Parse(segment, System.Globalization.CultureInfo.InvariantCulture)] : node?[segment]; + return node; + } } diff --git a/tests/NodePilot.Cli.Tests/Commands/CommandIntegrationCoverageTests.cs b/tests/NodePilot.Cli.Tests/Commands/CommandIntegrationCoverageTests.cs index f7737cd0..1fbbf070 100644 --- a/tests/NodePilot.Cli.Tests/Commands/CommandIntegrationCoverageTests.cs +++ b/tests/NodePilot.Cli.Tests/Commands/CommandIntegrationCoverageTests.cs @@ -1,3 +1,4 @@ +using System.Text.Json; using FluentAssertions; using NodePilot.Cli.Tests.Infra; using Spectre.Console; @@ -225,6 +226,45 @@ public void BackupManifest_RendersSectionCounts() output.Should().Contain("workflows").And.Contain("credentials"); } + [Fact] + public void BackupManifest_JsonOutput_WritesParseableJson() + { + using var h = new CommandTestHarness(); + StubBackupManifest(h); + + var result = h.Run("backup", "manifest", "-o", "json"); + + result.ExitCode.Should().Be(ExitCodes.Success); + using var doc = JsonDocument.Parse(result.Output); + var sections = doc.RootElement.GetProperty("sections"); + sections.GetArrayLength().Should().Be(2); + sections[0].GetProperty("section").GetString().Should().Be("workflows"); + sections[0].GetProperty("count").GetInt32().Should().Be(12); + } + + [Fact] + public void BackupManifest_TableOutput_RendersSectionTable() + { + using var h = new CommandTestHarness(); + StubBackupManifest(h); + + var result = h.Run("backup", "manifest", "-o", "table"); + + result.ExitCode.Should().Be(ExitCodes.Success); + result.Output.Should().Contain("Section").And.Contain("workflows").And.Contain("12"); + } + + private static void StubBackupManifest(CommandTestHarness h) + => h.Server.Given(Request.Create().WithPath("/api/backup/manifest").UsingGet()) + .RespondWith(Response.Create().WithStatusCode(200).WithBodyAsJson(new + { + sections = new[] + { + new { section = "workflows", count = 12 }, + new { section = "credentials", count = 3 }, + }, + })); + [Fact] public void BackupExport_NoOut_FailsBeforeHttp() { @@ -324,6 +364,60 @@ public void BackupPreview_Success_RendersSectionTable() finally { Del(file); Del(pwFile); } } + [Fact] + public void BackupPreview_JsonOutput_WritesParseableJson() + { + using var h = new CommandTestHarness(); + h.Server.Given(Request.Create().WithPath("/api/backup/preview").UsingPost()) + .RespondWith(Response.Create().WithStatusCode(200).WithBodyAsJson(new + { + integrityVerified = true, appVersion = "2026.6", + sections = new[] { new { section = "workflows", inBackup = 12, @new = 5, conflicts = 2 } }, + warnings = new[] { "credential 'x' is bound to another host" }, + })); + + var file = WriteTempBytes(new byte[] { 9, 9, 9 }); + var pwFile = WriteTemp("correct horse battery staple"); + try + { + var result = h.Run("backup", "preview", file, "--passphrase-file", pwFile, "-o", "json"); + + result.ExitCode.Should().Be(ExitCodes.Success); + using var doc = JsonDocument.Parse(result.Output); + doc.RootElement.GetProperty("appVersion").GetString().Should().Be("2026.6"); + doc.RootElement.GetProperty("sections")[0].GetProperty("conflicts").GetInt32().Should().Be(2); + doc.RootElement.GetProperty("warnings")[0].GetString().Should().Contain("another host"); + } + finally { Del(file); Del(pwFile); } + } + + [Fact] + public void BackupRestore_JsonOutput_WritesParseableJson() + { + using var h = new CommandTestHarness(); + h.Server.Given(Request.Create().WithPath("/api/backup/restore").UsingPost()) + .RespondWith(Response.Create().WithStatusCode(200).WithBodyAsJson(new + { + sections = new[] { new { section = "workflows", created = 5, overwritten = 1, skipped = 2, renamed = 0 } }, + settings = new { applied = true, message = "ok" }, + warnings = Array.Empty(), + })); + + var pwFile = WriteTemp("hunter2"); + var file = WriteTempBytes(new byte[] { 1, 2, 3 }); + try + { + var result = h.Run("backup", "restore", file, "--passphrase-file", pwFile, "--yes", "-o", "json"); + + result.ExitCode.Should().Be(ExitCodes.Success); + using var doc = JsonDocument.Parse(result.Output); + doc.RootElement.GetProperty("sections")[0].GetProperty("created").GetInt32().Should().Be(5); + doc.RootElement.GetProperty("settings").GetProperty("applied").GetBoolean().Should().BeTrue(); + result.StdErr.Should().Contain("Restore complete."); + } + finally { Del(pwFile); Del(file); } + } + [Fact] public void BackupRestore_FileNotFound_FailsBeforeHttp() { diff --git a/tests/NodePilot.Cli.Tests/Commands/CommandIntegrationResourcesTests.cs b/tests/NodePilot.Cli.Tests/Commands/CommandIntegrationResourcesTests.cs index 242d0043..64d64a6b 100644 --- a/tests/NodePilot.Cli.Tests/Commands/CommandIntegrationResourcesTests.cs +++ b/tests/NodePilot.Cli.Tests/Commands/CommandIntegrationResourcesTests.cs @@ -1,3 +1,5 @@ +using System.Text.Json; +using System.Text.Json.Nodes; using FluentAssertions; using NodePilot.Cli.Tests.Infra; using WireMock.RequestBuilders; @@ -65,6 +67,30 @@ public void WorkflowVersionGet_RendersDetail() result.ExitCode.Should().Be(ExitCodes.Success); } + [Fact] + public void WorkflowVersionGet_YamlOutput_ReadsBackUnchanged() + { + using var h = new CommandTestHarness(); + var id = Guid.NewGuid(); + var definition = "{\n \"nodes\": [{ \"id\": \"s1\", \"data\": { \"config\": { \"script\": \"Get-Service\\nWrite-Output 'done'\" } } }],\n \"edges\": []\n}"; + h.Server.Given(Request.Create().WithPath($"/api/workflows/{id}").UsingGet()) + .RespondWith(Response.Create().WithStatusCode(200).WithBody(Single(id, "Build"))); + h.Server.Given(Request.Create().WithPath($"/api/workflows/{id}/versions/2").UsingGet()) + .RespondWith(Response.Create().WithStatusCode(200).WithBodyAsJson(new + { + version = 2, name = "null", description = "", + definitionJson = definition, createdAt = "2026-11-04T10:00:00Z", createdBy = "admin", + changeNote = (string?)null, isCurrent = false, + })); + + var result = h.Run("workflow", "version", id.ToString(), "2", "-o", "yaml"); + + result.ExitCode.Should().Be(ExitCodes.Success); + var json = h.Run("workflow", "version", id.ToString(), "2", "-o", "json"); + YamlRoundTrip.ToJson(result.Output).Should().Be(YamlRoundTrip.Normalize(json.Output), result.Output); + JsonNode.Parse(YamlRoundTrip.ToJson(result.Output))!["definitionJson"]!.GetValue().Should().Be(definition); + } + [Fact] public void WorkflowImportScorch_FromFile_ReturnsZero() { @@ -307,6 +333,25 @@ public void MachineCreate_AllFields_ReturnsZero() result.ExitCode.Should().Be(ExitCodes.Success); } + [Theory] + [InlineData(new string[0], 5985, false)] + [InlineData(new[] { "--ssl" }, 5986, true)] + [InlineData(new[] { "--ssl", "--port", "15986" }, 15986, true)] + public void MachineCreate_PortDefault_FollowsSsl(string[] extraArgs, int expectedPort, bool expectedSsl) + { + using var h = new CommandTestHarness(); + h.Server.Given(Request.Create().WithPath("/api/machines").UsingPost()) + .RespondWith(Response.Create().WithStatusCode(201).WithBodyAsJson(Machine(Guid.NewGuid()))); + + var result = h.Run(["machine", "create", "--name", "win-01", "--hostname", "win-01.lab", .. extraArgs]); + + result.ExitCode.Should().Be(ExitCodes.Success); + var create = h.Server.LogEntries.Single(e => e.RequestMessage!.Method == "POST"); + using var body = JsonDocument.Parse(create.RequestMessage!.Body!); + body.RootElement.GetProperty("winRmPort").GetInt32().Should().Be(expectedPort); + body.RootElement.GetProperty("useSsl").GetBoolean().Should().Be(expectedSsl); + } + [Fact] public void MachineUpdate_PartialPatch_MergesWithGet() { diff --git a/tests/NodePilot.Cli.Tests/Commands/CommandIntegrationTests.cs b/tests/NodePilot.Cli.Tests/Commands/CommandIntegrationTests.cs index 5cb3922a..59847fc5 100644 --- a/tests/NodePilot.Cli.Tests/Commands/CommandIntegrationTests.cs +++ b/tests/NodePilot.Cli.Tests/Commands/CommandIntegrationTests.cs @@ -1,3 +1,4 @@ +using System.Text.Json; using FluentAssertions; using NodePilot.Cli.Tests.Infra; using Spectre.Console.Cli; @@ -186,6 +187,33 @@ public void AuthLogin_SetupTokenRequiredWithToken_SaysTokenRejected() result.StdErr.Should().Contain("Login failed: the setup token was rejected."); } + [Theory] + [InlineData(" padded secret ")] + [InlineData("\tleading tab")] + [InlineData("pw12345678")] + public void AuthLogin_PasswordFromStdin_SendsPasswordUnchanged(string password) + { + using var h = new CommandTestHarness(authenticated: false); + StubLogin(h); + + var originalIn = Console.In; + Console.SetIn(new StringReader(password + "\r\n")); + RunResult result; + try + { + result = h.Run("auth", "login", "--username", "admin", "--password-stdin"); + } + finally + { + Console.SetIn(originalIn); + } + + result.ExitCode.Should().Be(ExitCodes.Success); + var login = h.Server.LogEntries.Single(e => e.RequestMessage!.AbsolutePath == "/api/auth/login"); + using var body = JsonDocument.Parse(login.RequestMessage!.Body!); + body.RootElement.GetProperty("password").GetString().Should().Be(password); + } + private static void StubSetupTokenRequired(CommandTestHarness h) => h.Server.Given(Request.Create().WithPath("/api/auth/login").UsingPost()) .RespondWith(Response.Create().WithStatusCode(401).WithBodyAsJson(new diff --git a/tests/NodePilot.Cli.Tests/Infra/YamlRoundTrip.cs b/tests/NodePilot.Cli.Tests/Infra/YamlRoundTrip.cs new file mode 100644 index 00000000..45b37596 --- /dev/null +++ b/tests/NodePilot.Cli.Tests/Infra/YamlRoundTrip.cs @@ -0,0 +1,67 @@ +using System.Text.Json.Nodes; +using System.Text.RegularExpressions; +using YamlDotNet.Core; +using YamlDotNet.RepresentationModel; + +namespace NodePilot.Cli.Tests.Infra; + +/// +/// Reads YAML back into JSON text and types plain scalars like a YAML 1.1 parser (PyYAML): +/// null and boolean words, numbers, and anything else 1.1 could read as a non-string. Comparing +/// the result with the source JSON shows whether emitted YAML kept every value and its type. +/// +internal static partial class YamlRoundTrip +{ + public static string ToJson(string yaml) + { + var stream = new YamlStream(); + stream.Load(new StringReader(yaml)); + if (stream.Documents.Count != 1) + throw new InvalidOperationException($"Expected one YAML document, got {stream.Documents.Count}."); + return Convert(stream.Documents[0].RootNode)?.ToJsonString() ?? "null"; + } + + public static string Normalize(string json) => JsonNode.Parse(json)?.ToJsonString() ?? "null"; + + private static JsonNode? Convert(YamlNode node) => node switch + { + YamlMappingNode map => new JsonObject(map.Children.Select(kv => + KeyValuePair.Create(Key((YamlScalarNode)kv.Key), Convert(kv.Value)))), + YamlSequenceNode seq => new JsonArray(seq.Children.Select(Convert).ToArray()), + YamlScalarNode scalar => Scalar(scalar), + _ => throw new InvalidOperationException($"Unexpected YAML node {node.NodeType}."), + }; + + private static string Key(YamlScalarNode key) + { + var value = key.Value ?? ""; + return key.Style == ScalarStyle.Plain && (value.Length == 0 || NonString().IsMatch(value)) + ? $"" + : value; + } + + private static JsonNode? Scalar(YamlScalarNode scalar) + { + var value = scalar.Value ?? ""; + if (scalar.Style != ScalarStyle.Plain) return JsonValue.Create(value); + if (value is "" or "~" or "null" or "Null" or "NULL") return null; + if (TrueWord().IsMatch(value)) return JsonValue.Create(true); + if (FalseWord().IsMatch(value)) return JsonValue.Create(false); + if (JsonNumber().IsMatch(value)) return JsonNode.Parse(value); + return NonString().IsMatch(value) ? JsonValue.Create($"") : JsonValue.Create(value); + } + + [GeneratedRegex("^(?:y|Y|yes|Yes|YES|true|True|TRUE|on|On|ON)$")] + private static partial Regex TrueWord(); + + [GeneratedRegex("^(?:n|N|no|No|NO|false|False|FALSE|off|Off|OFF)$")] + private static partial Regex FalseWord(); + + [GeneratedRegex(@"^-?(?:0|[1-9][0-9]*)(?:\.[0-9]+)?(?:[eE][-+]?[0-9]+)?$")] + private static partial Regex JsonNumber(); + + // Deliberately broad: any plain scalar starting like a number, date or special float, plus + // the merge key, the value key and every null/boolean word. + [GeneratedRegex(@"^(?:[-+.]?[0-9].*|[-+]?\.(?:inf|Inf|INF)|\.(?:nan|NaN|NAN)|<<|=|~|null|Null|NULL|y|Y|yes|Yes|YES|n|N|no|No|NO|true|True|TRUE|false|False|FALSE|on|On|ON|off|Off|OFF)$")] + private static partial Regex NonString(); +} diff --git a/tests/NodePilot.Cli.Tests/NodePilot.Cli.Tests.csproj b/tests/NodePilot.Cli.Tests/NodePilot.Cli.Tests.csproj index ee4b9579..4c7194a7 100644 --- a/tests/NodePilot.Cli.Tests/NodePilot.Cli.Tests.csproj +++ b/tests/NodePilot.Cli.Tests/NodePilot.Cli.Tests.csproj @@ -25,6 +25,8 @@ + + diff --git a/tests/NodePilot.Cli.Tests/Settings/OutputFormatTests.cs b/tests/NodePilot.Cli.Tests/Settings/OutputFormatTests.cs index ea36ca1b..1aebc187 100644 --- a/tests/NodePilot.Cli.Tests/Settings/OutputFormatTests.cs +++ b/tests/NodePilot.Cli.Tests/Settings/OutputFormatTests.cs @@ -1,6 +1,9 @@ +using System.Text.Json; using FluentAssertions; +using NodePilot.Cli.Api; using NodePilot.Cli.Output; using NodePilot.Cli.Settings; +using NodePilot.Cli.Tests.Infra; using Xunit; namespace NodePilot.Cli.Tests.Settings; @@ -46,4 +49,53 @@ public void YamlEmitter_EmitsScalarsObjectsAndArrays() yaml.Should().Contain("detail:"); yaml.Should().Contain("owner: ops-example"); } + + [Fact] + public void YamlEmitter_RoundTrip_KeepsMultilineEmptyAndNullLikeValues() + { + var value = new + { + Script = "$s = Get-Service\r\nif ($s) {\n\t\"ok: #1\" | Write-Output\n}\n trailing \\ backslash \n", + Tags = Array.Empty(), + Config = new Dictionary(), + NullLike = new[] + { + "null", "Null", "NULL", "~", "", "true", "False", "yes", "No", "on", "OFF", "y", "n", + "123", "-1", "1.5", "1e3", ".inf", ".NaN", "0x1F", "2026-11-04", "<<", "=", + "- item", "? key", "a: b", "#hash", "@at", " leading", "trailing ", "plain text", + "bell" + (char)7, "line" + (char)0x2028 + "break", + }, + Missing = (string?)null, + Nested = new object[] + { + new object[] { "a", new object[] { "b" } }, + new Dictionary(), + Array.Empty(), + new { Inner = Array.Empty(), Count = 2, Ratio = -0.5 }, + }, + Keys = new Dictionary { ["null"] = 1, ["true"] = 2, ["123"] = 3, ["a key: x"] = 4, ["plain"] = "v" }, + }; + + var yaml = YamlEmitter.Emit(value); + + YamlRoundTrip.ToJson(yaml).Should().Be( + YamlRoundTrip.Normalize(JsonSerializer.Serialize(value, NodePilotApiClient.JsonOptions)), yaml); + } + + [Theory] + [InlineData("[]")] + [InlineData("{}")] + [InlineData("[[]]")] + [InlineData("[{}]")] + [InlineData("[[1,[2]],[\"a\",{\"b\":[]}]]")] + [InlineData("\"null\"")] + [InlineData("null")] + public void YamlEmitter_RoundTrip_KeepsRootAndNestedCollections(string json) + { + using var doc = JsonDocument.Parse(json); + + var yaml = YamlEmitter.Emit(doc.RootElement); + + YamlRoundTrip.ToJson(yaml).Should().Be(YamlRoundTrip.Normalize(json), yaml); + } } diff --git a/tests/NodePilot.Engine.Tests/Activities/WmiQueryActivityTests.cs b/tests/NodePilot.Engine.Tests/Activities/WmiQueryActivityTests.cs index 85780205..813d9af8 100644 --- a/tests/NodePilot.Engine.Tests/Activities/WmiQueryActivityTests.cs +++ b/tests/NodePilot.Engine.Tests/Activities/WmiQueryActivityTests.cs @@ -254,12 +254,65 @@ public async Task InvokeMethod_WithArguments_RendersHashtableLiteral() await CreateActivity().ExecuteAsync(Ctx(), Cfg("{\"mode\": \"invokeMethod\", \"className\": \"Win32_Process\", \"methodName\": \"Create\", \"arguments\": {\"CommandLine\": \"notepad.exe\", \"Priority\": 32, \"ShowWindow\": true}}"), CancellationToken.None); - _capturedScript.Should().Contain("-Arguments @{"); + _capturedScript.Should().Contain("$__npArgs = @{"); + _capturedScript.Should().Contain("-Arguments $__npArgs"); _capturedScript.Should().Contain("CommandLine = 'notepad.exe'"); _capturedScript.Should().Contain("Priority = 32"); _capturedScript.Should().Contain("ShowWindow = $true"); } + [Fact] + public async Task InvokeMethod_StaticMethod_PassesClassDeclarationForArgumentTyping() + { + _capturedScript = null; + await CreateActivity().ExecuteAsync(Ctx(), + Cfg("{\"mode\": \"invokeMethod\", \"className\": \"StdRegProv\", \"namespace\": \"root\\\\default\", \"methodName\": \"EnumKey\", \"arguments\": {\"hDefKey\": 2147483649, \"sSubKeyName\": \"Software\"}}"), + CancellationToken.None); + _capturedScript.Should().Contain("$__npClass = Get-CimClass -ClassName 'StdRegProv' -Namespace 'root\\default'"); + _capturedScript.Should().Contain("Invoke-CimMethod -CimClass $__npClass -MethodName 'EnumKey' -Arguments $__npArgs"); + _capturedScript.Should().NotContain("Invoke-CimMethod -ClassName", + "without the class declaration the cmdlet sends Int64 and a UInt32 parameter rejects it"); + } + + // Read-only CIM call on the local machine: StdRegProv.EnumKey lists the subkeys of HKCU\Software. + private const string HkcuEnumKeyConfig = + "{\"mode\": \"invokeMethod\", \"className\": \"StdRegProv\", \"namespace\": \"root\\\\default\", \"methodName\": \"EnumKey\", " + + "\"arguments\": {\"hDefKey\": HDEFKEY, \"sSubKeyName\": \"Software\"}, \"captureProperties\": [\"ReturnValue\"]}"; + + [WindowsFact] + public async Task InvokeMethod_UInt32ArgumentAboveInt32Max_LocalCallSucceeds() + { + var result = await CreateActivity().ExecuteAsync(LocalCtx(), + Cfg(HkcuEnumKeyConfig.Replace("HDEFKEY", "2147483649")), + CancellationToken.None); + + result.Success.Should().BeTrue(result.ErrorOutput); + result.OutputParameters.Should().ContainKey("ReturnValue").WhoseValue.Should().Be("0"); + } + + [WindowsFact] + public async Task InvokeMethod_ArgumentOutsideDeclaredRange_FailsNamingArgumentAndType() + { + var result = await CreateActivity().ExecuteAsync(LocalCtx(), + Cfg(HkcuEnumKeyConfig.Replace("HDEFKEY", "4294967296")), + CancellationToken.None); + + result.Success.Should().BeFalse(); + result.ErrorOutput.Should().Contain("argument 'hDefKey' value '4294967296' does not fit the declared CIM type UInt32"); + } + + private StepExecutionContext LocalCtx() + { + var machineId = Guid.NewGuid(); + _db.ManagedMachines.Add(new ManagedMachine + { + Id = machineId, Name = "Local " + machineId.ToString("N"), Hostname = "localhost", + WinRmPort = 5985, IsReachable = true, + }); + _db.SaveChanges(); + return new StepExecutionContext { WorkflowExecutionId = Guid.NewGuid(), StepId = "local-wmi", TargetMachineId = machineId }; + } + [Fact] public async Task InvokeMethod_StringArgWithApostrophe_Escaped() { diff --git a/tests/NodePilot.Mcp.Tests/Tools/MoreToolsCoverageTests.cs b/tests/NodePilot.Mcp.Tests/Tools/MoreToolsCoverageTests.cs index c6970a48..658bce90 100644 --- a/tests/NodePilot.Mcp.Tests/Tools/MoreToolsCoverageTests.cs +++ b/tests/NodePilot.Mcp.Tests/Tools/MoreToolsCoverageTests.cs @@ -66,6 +66,28 @@ public async Task GetMachine_CreateMachine_TestMachine_Work() J(await tools.TestMachine(id.ToString())).Should().Contain("\"success\":true"); } + [Theory] + [InlineData(false, null, 5985)] + [InlineData(true, null, 5986)] + [InlineData(true, 15986, 15986)] + public async Task CreateMachine_PortDefault_FollowsSsl(bool useSsl, int? port, int expectedPort) + { + using var api = new TestApi(); + api.Server.Given(Request.Create().WithPath("/api/machines").UsingPost()) + .RespondWith(Response.Create().WithStatusCode(201).WithBodyAsJson(new + { + id = Guid.NewGuid(), name = "WEB01", hostname = "web01", winRmPort = expectedPort, useSsl, + defaultCredentialId = (Guid?)null, tags = (string?)null, lastConnectivityCheck = (DateTime?)null, + isReachable = true, usedByWorkflowCount = 0, recentStepCount = 0, recentFailedStepCount = 0, activeRunCount = 0, + })); + + await new SupportingDataTools(api.Client()).CreateMachine("WEB01", "web01", port, useSsl); + + var body = E(api.Server.LogEntries.Single(e => e.RequestMessage.Method == "POST").RequestMessage.Body!); + body.GetProperty("winRmPort").GetInt32().Should().Be(expectedPort); + body.GetProperty("useSsl").GetBoolean().Should().Be(useSsl); + } + [Fact] public async Task GetCredential_CreateGlobal_UpdateCredentialMergesDomain() {