diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 85d0c0aba..c66ecf57b 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -52,8 +52,8 @@ These are the canonical homes. Do not reintroduce private copies elsewhere. | Loopback MCP server for tests | `OpenClaw.TestSupport.FakeMcpServer` | authoritative | | Gateway record test data | `OpenClaw.Connection.Tests.GatewayRecordBuilder` | authoritative | | Settings test data | `OpenClaw.TestSupport.SettingsDataBuilder` | authoritative | -| JSON `JsonElement` coercion | `JsonReadHelpers` (planned) | planned | -| WSL/POSIX shell quoting | dedicated POSIX quote APIs (planned) | planned | +| JSON `JsonElement` coercion (non-nullable fallback family) | `JsonReadHelpers` | authoritative | +| WSL/POSIX shell quoting | `WslShellQuoting` | authoritative | | Capability UI metadata | `NodeCapabilityUiCatalog` (planned) | planned | | Capability registration/gating | `NodeCapabilityRegistrationPolicy` (planned) | planned | | Local MCP exposure policy | `McpCapabilityPolicy` (planned) | planned | @@ -74,7 +74,7 @@ These are the canonical homes. Do not reintroduce private copies elsewhere. | `src/OpenClaw.Shared/Models.cs` | per-domain model files + `*Mapper` classes | | `src/OpenClaw.Shared/Capabilities/SystemCapability.cs` | `ExecApprovalService` | | `src/OpenClaw.Connection/GatewayConnectionManager.cs` | `NodeConnectionCoordinator`, `BootstrapTokenLifecycle`, `DevicePairApprovalCoordinator` | -| `src/OpenClaw.SetupEngine/SetupSteps.cs` | one file per step; `WslShellClient`, `GatewayConfigScriptBuilder`, `KeepaliveProcessManager` | +| `src/OpenClaw.SetupEngine/SetupSteps.cs` | one file per step; `WslShellClient`, `WslShellQuoting`, `GatewayConfigScriptBuilder`, `KeepaliveProcessManager` | | Any test hand-rolling a temp dir / env save-restore / CLI capture | `OpenClaw.TestSupport` fixtures | ## Ledger @@ -107,8 +107,8 @@ leading and trailing pipe. Columns, in order: | test-fake-mcp | authoritative | OpenClaw.WinNode.Cli.Tests | private internal FakeMcpServer copy | OpenClaw.TestSupport.FakeMcpServer | - | one loopback MCP server captures method/body/auth and returns canned/timeout responses | TestSupportFixtureTests.FakeMcpServer_CapturesRequest | behavioral | when all MCP-round-trip tests share it | | test-gateway-builder | authoritative | OpenClaw.Connection.Tests | per-file MakeRecord(id,url) helpers | OpenClaw.Connection.Tests.GatewayRecordBuilder | pre-existing MakeRecord until migrated | gateway record test data has one builder | TestSupportFixtureTests.GatewayRecordBuilder_BuildsRecord | behavioral | when MakeRecord helpers are removed | | test-settings-builder | authoritative | scattered test files | ad hoc SettingsData construction in migrated tests | OpenClaw.TestSupport.SettingsDataBuilder | pre-existing un-migrated tests until adopted | settings test data starts from production defaults | TestSupportFixtureTests.SettingsDataBuilder_StartsFromDefaults | behavioral | when settings tests adopt the builder | -| json-read-helpers | planned | OpenClaw.Shared (multiple files) | duplicate private GetString/TryGetString/FirstNonEmpty helpers | JsonReadHelpers | - | one JSON coercion helper with defined null/wrong-kind behavior | none | review-only | extracted in PR 2 | -| wsl-posix-quoting | planned | OpenClaw.SetupEngine/SetupSteps.cs | ad hoc ShellEscape with divergent wrap semantics | dedicated POSIX quote APIs | - | WSL scripts use POSIX quoting, never cmd/PowerShell quoting | none | review-only | extracted in PR 2-3 | +| json-read-helpers | authoritative | OpenClaw.Shared (multiple files) | duplicate non-nullable fallback-returning JsonElement getters | JsonReadHelpers | null-sentinel / non-negative / whitespace-absent / trimming variants stay separate | canonical non-nullable fallback JSON coercion; divergent-contract helpers are not blindly routed here | JsonReadHelpersTests.GetString_ReturnsNull_WhenPropertyMissing | behavioral | when the non-nullable fallback getters are all routed here | +| wsl-posix-quoting | authoritative | OpenClaw.SetupEngine/SetupSteps.cs | ad hoc ShellEscape with divergent wrap semantics | WslShellQuoting | SetupSteps ShellEscape until PR 3 migration | WSL scripts use POSIX quoting not cmd/PowerShell quoting | WslShellQuotingTests.QuotePosixSingleQuote_WrapsAndEscapesEmbeddedQuote | behavioral | when SetupSteps call sites use WslShellQuoting | | app-window-manager | planned | src/OpenClaw.Tray.WinUI/App.xaml.cs | window creation/show/hide/shutdown | IWindowManager | composition/delegation only | startup/shutdown ordering deterministic; disposed once | none | review-only | extracted in Phase 3 | | app-tray-controller | planned | src/OpenClaw.Tray.WinUI/App.xaml.cs | tray icon/menu/action routing | ITrayController | composition/delegation only | tray actions route unchanged | none | review-only | extracted in Phase 3 | | app-activation-router | planned | src/OpenClaw.Tray.WinUI/App.xaml.cs | deep-link/toast/single-instance activation | IActivationRouter | composition/delegation only | activation routes land on the same UI/actions; current-user pipe security preserved | none | review-only | extracted in Phase 3 | diff --git a/src/OpenClaw.Shared/JsonReadHelpers.cs b/src/OpenClaw.Shared/JsonReadHelpers.cs new file mode 100644 index 000000000..130e5555e --- /dev/null +++ b/src/OpenClaw.Shared/JsonReadHelpers.cs @@ -0,0 +1,188 @@ +using System.Diagnostics.CodeAnalysis; +using System.Text.Json; + +namespace OpenClaw.Shared; + +/// +/// Canonical home for the non-nullable, fallback-returning family of +/// coercion helpers: the gateway-client-style getters +/// (GetString returning null when absent, numeric getters returning +/// a caller-supplied fallback, GetArrayLength, and a verbatim +/// FirstNonEmpty) plus the defensive non-object guard that the +/// GetStringSafe variant added. See docs/ARCHITECTURE.md → +/// json-read-helpers. +/// +/// All accessors are total (they never throw): a missing property, a +/// JSON null, a value of the wrong , or a non-object +/// yields the documented fallback (null / 0 / +/// the caller-supplied fallback) rather than an exception — including a non-object +/// element, which +/// would otherwise throw on. +/// +/// Scope — do not blindly migrate. This is deliberately NOT a +/// drop-in for every private JSON helper in the repo. Several existing helpers +/// have different contracts; routing them through these methods changes behavior +/// and needs a call-site guard or a dedicated variant: +/// +/// Models.GetInt / Models.GetLong return int? / +/// long? (a null sentinel means "missing"), and Models.GetLong has +/// no double-to-long fallback — the canonical instead +/// truncates a fractional number. +/// WorkspaceFilesModel.GetLong rejects negatives (requires +/// >= 0); the canonical getters accept negative numbers. +/// WindowsNodeClient.TryGetString treats empty/whitespace as +/// absent; the canonical returns true +/// with an empty string ("" is present). +/// Mxc.MxcAvailability.FirstNonEmpty trims its result, and +/// SetupSteps.FirstNonEmpty trims and falls back to "no output"; +/// the canonical returns the first value verbatim, or +/// null. +/// Some older getters throw on a non-object +/// (failing the parse); the canonical getters are total and return the fallback. +/// +/// +internal static class JsonReadHelpers +{ + /// + /// Returns the string value of , or null + /// when is not an object, the property is absent, or + /// the value is not a JSON string. The returned string may be empty; only + /// missing / non-string values collapse to null. + /// + internal static string? GetString(JsonElement parent, string property) + { + if (parent.ValueKind != JsonValueKind.Object + || !parent.TryGetProperty(property, out var value) + || value.ValueKind != JsonValueKind.String) + { + return null; + } + + return value.GetString(); + } + + /// + /// Try-pattern companion to . Returns true and + /// sets when the property exists and is a JSON string + /// (the string may be empty); otherwise returns false with + /// set to null. + /// + internal static bool TryGetString(JsonElement parent, string property, [NotNullWhen(true)] out string? value) + { + value = GetString(parent, property); + return value is not null; + } + + /// + /// Returns the value of , or + /// when it is missing or not a JSON number. A + /// number within range that exceeds the + /// range is clamped to the range; a number beyond + /// range returns (matching the + /// prior private helper). + /// + internal static int GetInt(JsonElement parent, string property, int fallback = 0) + { + if (parent.ValueKind != JsonValueKind.Object + || !parent.TryGetProperty(property, out var value) + || value.ValueKind != JsonValueKind.Number) + { + return fallback; + } + + if (value.TryGetInt32(out var intValue)) + { + return intValue; + } + + if (value.TryGetInt64(out var longValue)) + { + return (int)Math.Clamp(longValue, int.MinValue, int.MaxValue); + } + + return fallback; + } + + /// + /// Returns the value of , or + /// when it is missing or not a JSON number. A + /// fractional number is truncated toward zero. + /// + internal static long GetLong(JsonElement parent, string property, long fallback = 0) + { + if (parent.ValueKind != JsonValueKind.Object + || !parent.TryGetProperty(property, out var value) + || value.ValueKind != JsonValueKind.Number) + { + return fallback; + } + + if (value.TryGetInt64(out var longValue)) + { + return longValue; + } + + if (value.TryGetDouble(out var doubleValue)) + { + return (long)doubleValue; + } + + return fallback; + } + + /// + /// Returns the value of , or + /// when it is missing or not a JSON number. + /// + internal static double GetDouble(JsonElement parent, string property, double fallback = 0) + { + if (parent.ValueKind != JsonValueKind.Object + || !parent.TryGetProperty(property, out var value) + || value.ValueKind != JsonValueKind.Number) + { + return fallback; + } + + return value.TryGetDouble(out var doubleValue) ? doubleValue : fallback; + } + + /// + /// Returns the number of elements in the array at , + /// or 0 when it is missing or not a JSON array. + /// + internal static int GetArrayLength(JsonElement parent, string property) + { + if (parent.ValueKind != JsonValueKind.Object + || !parent.TryGetProperty(property, out var value) + || value.ValueKind != JsonValueKind.Array) + { + return 0; + } + + return value.GetArrayLength(); + } + + /// + /// Returns the first value that is not null, empty, or whitespace, or + /// null when none qualify. Values are returned verbatim (not trimmed). + /// + internal static string? FirstNonEmpty(params string?[] values) + { + // A bare `null` argument binds to the params array itself rather than a + // one-element array; guard so the canonical helper never throws. + if (values is null) + { + return null; + } + + foreach (var value in values) + { + if (!string.IsNullOrWhiteSpace(value)) + { + return value; + } + } + + return null; + } +} diff --git a/src/OpenClaw.Shared/OpenClawGatewayClient.cs b/src/OpenClaw.Shared/OpenClawGatewayClient.cs index b554bcc1c..bdf9b4744 100644 --- a/src/OpenClaw.Shared/OpenClawGatewayClient.cs +++ b/src/OpenClaw.Shared/OpenClawGatewayClient.cs @@ -3837,7 +3837,7 @@ private void ParseUsageCost(JsonElement payload) var summary = new GatewayCostUsageInfo { UpdatedAt = ParseUnixTimestampMs(payload, "updatedAt") ?? DateTime.UtcNow, - Days = GetInt(payload, "days") + Days = JsonReadHelpers.GetInt(payload, "days") }; if (payload.TryGetProperty("totals", out var totals) && totals.ValueKind == JsonValueKind.Object) @@ -3850,7 +3850,7 @@ private void ParseUsageCost(JsonElement payload) CacheWrite = GetLong(totals, "cacheWrite"), TotalTokens = GetLong(totals, "totalTokens"), TotalCost = GetDouble(totals, "totalCost"), - MissingCostEntries = GetInt(totals, "missingCostEntries") + MissingCostEntries = JsonReadHelpers.GetInt(totals, "missingCostEntries") }; } @@ -3867,7 +3867,7 @@ private void ParseUsageCost(JsonElement payload) CacheWrite = GetLong(day, "cacheWrite"), TotalTokens = GetLong(day, "totalTokens"), TotalCost = GetDouble(day, "totalCost"), - MissingCostEntries = GetInt(day, "missingCostEntries") + MissingCostEntries = JsonReadHelpers.GetInt(day, "missingCostEntries") }); } } @@ -4038,15 +4038,6 @@ private static string BuildProviderSummary(GatewayUsageStatusInfo status) }; } - private static int GetInt(JsonElement parent, string property) - { - if (!parent.TryGetProperty(property, out var value) || value.ValueKind != JsonValueKind.Number) - return 0; - if (value.TryGetInt32(out var intVal)) return intVal; - if (value.TryGetInt64(out var longVal)) return (int)Math.Clamp(longVal, int.MinValue, int.MaxValue); - return 0; - } - private static long GetLong(JsonElement parent, string property) { if (!parent.TryGetProperty(property, out var value) || value.ValueKind != JsonValueKind.Number) diff --git a/src/OpenClaw.Shared/WslShellQuoting.cs b/src/OpenClaw.Shared/WslShellQuoting.cs new file mode 100644 index 000000000..e9b2a0678 --- /dev/null +++ b/src/OpenClaw.Shared/WslShellQuoting.cs @@ -0,0 +1,56 @@ +namespace OpenClaw.Shared; + +/// +/// POSIX / WSL single-quote quoting. This is a deliberately separate contract +/// from , which targets Windows cmd.exe / PowerShell +/// (double quotes, or single quotes doubled as ''). Those rules are +/// wrong and injection-unsafe for the bash/sh command lines OpenClaw +/// builds to run inside WSL. +/// +/// A POSIX shell performs no processing whatsoever inside single quotes — there +/// is no backslash escape for a single quote. The only way to place a literal +/// single quote in a single-quoted string is to close the quote, emit an escaped +/// literal quote, then reopen: the canonical '\'' idiom. Every embedded +/// single quote is replaced by that four-character sequence. +/// +/// The two operations are kept distinct on purpose (see docs/ARCHITECTURE.md → +/// wsl-posix-quoting); conflating "escape the inner text" with "produce a +/// wrapped token" is exactly the divergent-semantics bug this type retires: +/// +/// escapes embedded quotes for a +/// value the caller is placing between its own outer single quotes, and +/// adds no quotes of its own. +/// returns a complete, standalone +/// single-quoted token ('' for an empty value). +/// +/// Neither operation trims, collapses, or otherwise interprets the input: the +/// bytes between the quotes are preserved verbatim, which is what makes the +/// result injection-safe for arbitrary values (URLs, JSON, paths, newlines). +/// +internal static class WslShellQuoting +{ + // Close quote ('), an escaped literal quote (\'), then reopen quote ('). + private const string EscapedSingleQuote = "'\\''"; + + /// + /// Escapes embedded single quotes using the POSIX '\'' idiom, for a + /// value the caller will wrap in its own outer single quotes. Does NOT add + /// outer quotes. An empty input yields an empty string. + /// + internal static string EscapePosixSingleQuoteInner(string value) + { + ArgumentNullException.ThrowIfNull(value); + return value.Replace("'", EscapedSingleQuote); + } + + /// + /// Wraps in single quotes, escaping any embedded + /// single quotes, so the result is exactly one POSIX-shell token. An empty + /// input yields '' (an empty argument, not an omitted one). + /// + internal static string QuotePosixSingleQuote(string value) + { + ArgumentNullException.ThrowIfNull(value); + return string.Concat("'", EscapePosixSingleQuoteInner(value), "'"); + } +} diff --git a/tests/OpenClaw.Shared.Tests/JsonReadHelpersTests.cs b/tests/OpenClaw.Shared.Tests/JsonReadHelpersTests.cs new file mode 100644 index 000000000..560f7a708 --- /dev/null +++ b/tests/OpenClaw.Shared.Tests/JsonReadHelpersTests.cs @@ -0,0 +1,266 @@ +using System.Text.Json; +using Xunit; +using OpenClaw.Shared; + +namespace OpenClaw.Shared.Tests; + +/// +/// Unit tests for — the canonical, total +/// coercion helpers. Covers missing property, null +/// value, wrong JSON kind, valid values, numeric clamp/truncation, empty arrays, +/// non-object parents, and FirstNonEmpty selection. +/// +public class JsonReadHelpersTests +{ + private static JsonElement Parse(string json) + { + using var doc = JsonDocument.Parse(json); + // Clone so the element stays valid after the JsonDocument is disposed. + return doc.RootElement.Clone(); + } + + // ── GetString ─────────────────────────────────────────────────── + + /// Guard test referenced by the architecture ledger (json-read-helpers). + [Fact] + public void GetString_ReturnsNull_WhenPropertyMissing() + { + var el = Parse("""{ "a": "x" }"""); + Assert.Null(JsonReadHelpers.GetString(el, "missing")); + } + + [Fact] + public void GetString_ReturnsNull_ForJsonNullValue() + { + var el = Parse("""{ "a": null }"""); + Assert.Null(JsonReadHelpers.GetString(el, "a")); + } + + [Theory] + [InlineData("""{ "a": 42 }""")] + [InlineData("""{ "a": true }""")] + [InlineData("""{ "a": [1, 2] }""")] + [InlineData("""{ "a": { "b": 1 } }""")] + public void GetString_ReturnsNull_ForWrongKind(string json) + { + var el = Parse(json); + Assert.Null(JsonReadHelpers.GetString(el, "a")); + } + + [Fact] + public void GetString_ReturnsValue_ForString() + { + var el = Parse("""{ "a": "hello" }"""); + Assert.Equal("hello", JsonReadHelpers.GetString(el, "a")); + } + + [Fact] + public void GetString_ReturnsEmptyString_ForEmptyStringValue() + { + // An empty string is a valid string; only missing/non-string collapses to null. + var el = Parse("""{ "a": "" }"""); + Assert.Equal("", JsonReadHelpers.GetString(el, "a")); + } + + [Fact] + public void GetString_ReturnsNull_ForNonObjectParent() + { + var arr = Parse("""[1, 2, 3]"""); + Assert.Null(JsonReadHelpers.GetString(arr, "a")); + } + + // ── TryGetString ──────────────────────────────────────────────── + + [Fact] + public void TryGetString_ReturnsTrueAndValue_ForString() + { + var el = Parse("""{ "a": "hello" }"""); + Assert.True(JsonReadHelpers.TryGetString(el, "a", out var value)); + Assert.Equal("hello", value); + } + + [Fact] + public void TryGetString_ReturnsTrue_ForEmptyString() + { + var el = Parse("""{ "a": "" }"""); + Assert.True(JsonReadHelpers.TryGetString(el, "a", out var value)); + Assert.Equal("", value); + } + + [Theory] + [InlineData("""{ "a": 1 }""")] + [InlineData("""{ "a": null }""")] + [InlineData("""{ "b": "x" }""")] + public void TryGetString_ReturnsFalseAndNull_ForMissingOrWrongKind(string json) + { + var el = Parse(json); + Assert.False(JsonReadHelpers.TryGetString(el, "a", out var value)); + Assert.Null(value); + } + + // ── GetInt ────────────────────────────────────────────────────── + + [Fact] + public void GetInt_ReturnsValue_ForNumber() + { + var el = Parse("""{ "a": 7 }"""); + Assert.Equal(7, JsonReadHelpers.GetInt(el, "a")); + } + + [Fact] + public void GetInt_ReturnsZero_ForMissingByDefault() + { + var el = Parse("""{ "a": 1 }"""); + Assert.Equal(0, JsonReadHelpers.GetInt(el, "missing")); + } + + [Fact] + public void GetInt_ReturnsFallback_ForMissing() + { + var el = Parse("""{ "a": 1 }"""); + Assert.Equal(-1, JsonReadHelpers.GetInt(el, "missing", fallback: -1)); + } + + [Theory] + [InlineData("""{ "a": "7" }""")] + [InlineData("""{ "a": true }""")] + [InlineData("""{ "a": null }""")] + public void GetInt_ReturnsFallback_ForWrongKind(string json) + { + var el = Parse(json); + Assert.Equal(99, JsonReadHelpers.GetInt(el, "a", fallback: 99)); + } + + [Fact] + public void GetInt_ClampsOutOfRangeToIntMax() + { + var el = Parse("""{ "a": 9999999999 }"""); // > int.MaxValue + Assert.Equal(int.MaxValue, JsonReadHelpers.GetInt(el, "a")); + } + + [Fact] + public void GetInt_ClampsOutOfRangeToIntMin() + { + var el = Parse("""{ "a": -9999999999 }"""); // < int.MinValue + Assert.Equal(int.MinValue, JsonReadHelpers.GetInt(el, "a")); + } + + [Fact] + public void GetInt_ReturnsFallback_ForNonObjectParent() + { + var arr = Parse("""[1, 2]"""); + Assert.Equal(3, JsonReadHelpers.GetInt(arr, "a", fallback: 3)); + } + + // ── GetLong ───────────────────────────────────────────────────── + + [Fact] + public void GetLong_ReturnsValue_ForLargeNumber() + { + var el = Parse("""{ "a": 9999999999 }"""); + Assert.Equal(9999999999L, JsonReadHelpers.GetLong(el, "a")); + } + + [Fact] + public void GetLong_TruncatesFractionalNumber() + { + var el = Parse("""{ "a": 3.9 }"""); + Assert.Equal(3L, JsonReadHelpers.GetLong(el, "a")); + } + + [Fact] + public void GetLong_ReturnsFallback_ForMissing() + { + var el = Parse("""{ "a": 1 }"""); + Assert.Equal(5L, JsonReadHelpers.GetLong(el, "missing", fallback: 5)); + } + + // ── GetDouble ─────────────────────────────────────────────────── + + [Fact] + public void GetDouble_ReturnsValue_ForFractionalNumber() + { + var el = Parse("""{ "a": 2.5 }"""); + Assert.Equal(2.5, JsonReadHelpers.GetDouble(el, "a")); + } + + [Fact] + public void GetDouble_ReturnsValue_ForIntegerNumber() + { + var el = Parse("""{ "a": 4 }"""); + Assert.Equal(4.0, JsonReadHelpers.GetDouble(el, "a")); + } + + [Fact] + public void GetDouble_ReturnsFallback_ForWrongKind() + { + var el = Parse("""{ "a": "2.5" }"""); + Assert.Equal(1.5, JsonReadHelpers.GetDouble(el, "a", fallback: 1.5)); + } + + // ── GetArrayLength ────────────────────────────────────────────── + + [Fact] + public void GetArrayLength_ReturnsCount_ForArray() + { + var el = Parse("""{ "a": [1, 2, 3] }"""); + Assert.Equal(3, JsonReadHelpers.GetArrayLength(el, "a")); + } + + [Fact] + public void GetArrayLength_ReturnsZero_ForEmptyArray() + { + var el = Parse("""{ "a": [] }"""); + Assert.Equal(0, JsonReadHelpers.GetArrayLength(el, "a")); + } + + [Theory] + [InlineData("""{ "a": 5 }""")] + [InlineData("""{ "a": "x" }""")] + [InlineData("""{ "b": [1] }""")] + public void GetArrayLength_ReturnsZero_ForMissingOrWrongKind(string json) + { + var el = Parse(json); + Assert.Equal(0, JsonReadHelpers.GetArrayLength(el, "a")); + } + + // ── FirstNonEmpty ─────────────────────────────────────────────── + + [Fact] + public void FirstNonEmpty_ReturnsFirstNonEmpty() + { + Assert.Equal("second", JsonReadHelpers.FirstNonEmpty(null, "", "second", "third")); + } + + [Fact] + public void FirstNonEmpty_SkipsWhitespaceOnly() + { + Assert.Equal("real", JsonReadHelpers.FirstNonEmpty(" ", "\t", "real")); + } + + [Fact] + public void FirstNonEmpty_ReturnsValueVerbatim_WithoutTrimming() + { + Assert.Equal(" padded ", JsonReadHelpers.FirstNonEmpty(null, " padded ")); + } + + [Fact] + public void FirstNonEmpty_ReturnsNull_WhenAllEmpty() + { + Assert.Null(JsonReadHelpers.FirstNonEmpty(null, "", " ")); + } + + [Fact] + public void FirstNonEmpty_ReturnsNull_ForNoArguments() + { + Assert.Null(JsonReadHelpers.FirstNonEmpty()); + } + + [Fact] + public void FirstNonEmpty_ReturnsNull_ForNullArray() + { + // A single bare null binds to the params array itself; the canonical + // helper is hardened to return null rather than throw. + Assert.Null(JsonReadHelpers.FirstNonEmpty(null!)); + } +} diff --git a/tests/OpenClaw.Shared.Tests/WslShellQuotingTests.cs b/tests/OpenClaw.Shared.Tests/WslShellQuotingTests.cs new file mode 100644 index 000000000..46478fa7b --- /dev/null +++ b/tests/OpenClaw.Shared.Tests/WslShellQuotingTests.cs @@ -0,0 +1,218 @@ +using Xunit; +using OpenClaw.Shared; + +namespace OpenClaw.Shared.Tests; + +/// +/// Unit tests for — POSIX/WSL single-quote quoting. +/// Focus is injection safety: arbitrary values (embedded quotes, shell +/// metacharacters, newlines, URLs, JSON, Windows paths) must survive as a single +/// literal token when wrapped, using the '\'' idiom rather than the +/// cmd/PowerShell rules in . +/// +public class WslShellQuotingTests +{ + // The canonical POSIX escaped single quote: close ' , literal \' , reopen '. + private const string Q = "'\\''"; + + // ── EscapePosixSingleQuoteInner (escape only, no wrapping) ────── + + [Fact] + public void EscapeInner_Empty_ReturnsEmpty() + { + Assert.Equal("", WslShellQuoting.EscapePosixSingleQuoteInner("")); + } + + [Fact] + public void EscapeInner_PlainText_Unchanged() + { + Assert.Equal("plain-value", WslShellQuoting.EscapePosixSingleQuoteInner("plain-value")); + } + + [Fact] + public void EscapeInner_SingleQuote_UsesPosixIdiom() + { + Assert.Equal(Q, WslShellQuoting.EscapePosixSingleQuoteInner("'")); + } + + [Fact] + public void EscapeInner_EmbeddedQuote_EscapesInPlace() + { + Assert.Equal("O" + Q + "Brien", WslShellQuoting.EscapePosixSingleQuoteInner("O'Brien")); + } + + [Fact] + public void EscapeInner_MultipleQuotes_EscapesEach() + { + Assert.Equal(Q + Q, WslShellQuoting.EscapePosixSingleQuoteInner("''")); + } + + [Fact] + public void EscapeInner_AddsNoOuterQuotes() + { + var result = WslShellQuoting.EscapePosixSingleQuoteInner("plain"); + Assert.False(result.StartsWith('\'')); + Assert.False(result.EndsWith('\'')); + } + + [Theory] + [InlineData("$(rm -rf /)")] + [InlineData("`id`")] + [InlineData("a\nb")] + [InlineData("C:\\Users\\name")] + public void EscapeInner_MetacharactersWithoutQuotes_Unchanged(string value) + { + // Only single quotes are special inside POSIX single quotes; everything + // else is preserved verbatim by the escape-only operation. + Assert.Equal(value, WslShellQuoting.EscapePosixSingleQuoteInner(value)); + } + + // ── QuotePosixSingleQuote (fully wrapped token) ───────────────── + + [Fact] + public void Quote_Empty_ReturnsEmptyQuotes() + { + // An empty argument must still be emitted, not omitted. + Assert.Equal("''", WslShellQuoting.QuotePosixSingleQuote("")); + } + + [Fact] + public void Quote_Whitespace_IsWrapped() + { + Assert.Equal("' '", WslShellQuoting.QuotePosixSingleQuote(" ")); + } + + [Fact] + public void Quote_PlainText_IsWrapped() + { + Assert.Equal("'plain'", WslShellQuoting.QuotePosixSingleQuote("plain")); + } + + /// Guard test referenced by the architecture ledger (wsl-posix-quoting). + [Fact] + public void QuotePosixSingleQuote_WrapsAndEscapesEmbeddedQuote() + { + Assert.Equal("'O" + Q + "Brien'", WslShellQuoting.QuotePosixSingleQuote("O'Brien")); + } + + [Fact] + public void Quote_SingleQuoteOnly_IsEscapedAndWrapped() + { + Assert.Equal("'" + Q + "'", WslShellQuoting.QuotePosixSingleQuote("'")); + } + + [Fact] + public void Quote_Newline_PreservedInsideQuotes() + { + Assert.Equal("'a\nb'", WslShellQuoting.QuotePosixSingleQuote("a\nb")); + } + + [Fact] + public void Quote_MultiLineFragment_PreservedVerbatim() + { + Assert.Equal("'line1\nline2\n'", WslShellQuoting.QuotePosixSingleQuote("line1\nline2\n")); + } + + [Fact] + public void Quote_CommandSubstitution_IsInert() + { + Assert.Equal("'$(rm -rf /)'", WslShellQuoting.QuotePosixSingleQuote("$(rm -rf /)")); + } + + [Fact] + public void Quote_Backticks_IsInert() + { + Assert.Equal("'`id`'", WslShellQuoting.QuotePosixSingleQuote("`id`")); + } + + [Fact] + public void Quote_Url_IsWrappedVerbatim() + { + const string url = "https://example.com/install?token=a&b=2"; + Assert.Equal("'" + url + "'", WslShellQuoting.QuotePosixSingleQuote(url)); + } + + [Fact] + public void Quote_JsonArray_IsWrappedVerbatim() + { + const string json = """["a","b","c"]"""; + Assert.Equal("'" + json + "'", WslShellQuoting.QuotePosixSingleQuote(json)); + } + + [Fact] + public void Quote_WindowsPath_BackslashesPreserved() + { + const string path = @"C:\Users\Some One\file.txt"; + Assert.Equal("'" + path + "'", WslShellQuoting.QuotePosixSingleQuote(path)); + } + + [Fact] + public void Quote_AlreadyQuotedLookingInput_EscapesBothQuotes() + { + Assert.Equal("'" + Q + "already" + Q + "'", WslShellQuoting.QuotePosixSingleQuote("'already'")); + } + + [Fact] + public void Quote_QuoteBreakoutAttempt_IsNeutralized() + { + // A classic breakout payload: the embedded single quote must not terminate + // the quoted token, so the trailing shell command stays inert text. + var result = WslShellQuoting.QuotePosixSingleQuote("x'; rm -rf / #"); + Assert.Equal("'x" + Q + "; rm -rf / #'", result); + } + + // ── Cross-cutting invariants ──────────────────────────────────── + + [Theory] + [InlineData("")] + [InlineData("plain")] + [InlineData("O'Brien")] + [InlineData("'already'")] + [InlineData("$(id)")] + [InlineData("a\nb")] + public void Quote_IsInnerEscapeWrappedInQuotes(string value) + { + var expected = "'" + WslShellQuoting.EscapePosixSingleQuoteInner(value) + "'"; + Assert.Equal(expected, WslShellQuoting.QuotePosixSingleQuote(value)); + } + + [Theory] + [InlineData("plain")] + [InlineData("O'Brien")] + [InlineData("$(id)")] + public void Quote_ContainsNoBareUnescapedSingleQuote(string value) + { + // Every interior single quote of a wrapped token belongs to the '\'' idiom, + // so replacing that idiom leaves no stray single quotes to break the token. + var result = WslShellQuoting.QuotePosixSingleQuote(value); + var inner = result[1..^1]; // strip the outer wrapping quotes + var withoutIdiom = inner.Replace(Q, ""); + Assert.DoesNotContain('\'', withoutIdiom); + } + + [Fact] + public void Quote_DiffersFromPowerShellQuoting_ForEmbeddedQuote() + { + // POSIX uses the '\'' idiom; PowerShell doubles the quote. Guards against a + // future accidental reuse of ShellQuoting for WSL command lines. + var posix = WslShellQuoting.QuotePosixSingleQuote("it's"); + var powershell = ShellQuoting.QuoteForShell("it's", isCmd: false); + Assert.Equal("'it" + Q + "s'", posix); + Assert.Equal("'it''s'", powershell); + Assert.NotEqual(powershell, posix); + } + + // ── Null handling ─────────────────────────────────────────────── + + [Fact] + public void EscapeInner_Null_Throws() + { + Assert.Throws(() => WslShellQuoting.EscapePosixSingleQuoteInner(null!)); + } + + [Fact] + public void Quote_Null_Throws() + { + Assert.Throws(() => WslShellQuoting.QuotePosixSingleQuote(null!)); + } +}