fix(security): address open CodeQL code-scanning alerts

.github/workflows/website-rebuild.yml

@@ -12,6 +12,9 @@ on:
     types: [published]
   workflow_dispatch:
 
+permissions:
+  contents: read
+
 jobs:
   rebuild:
     runs-on: ubuntu-latest

src/Netclaw.Actors.Tests/Reminders/ReminderDefinitionStoreTests.cs

@@ -210,6 +210,22 @@ public void ReminderDefinition_id_serializes_as_bare_json_string()
         Assert.Equal(new ReminderId("reminder-byte-eq"), loaded!.Id);
     }
 
+    [Theory]
+    [InlineData("../../etc/passwd")]
+    [InlineData("..\\..\\windows\\system32\\config")]
+    [InlineData("/etc/passwd")]
+    [InlineData("C:\\Windows\\System32")]
+    public void Exists_with_traversal_id_does_not_escape_reminders_directory(string maliciousId)
+    {
+        var store = new ReminderDefinitionStore(_paths);
+
+        // Uri.EscapeDataString neutralizes path separators, so the canonical
+        // path stays inside _basePath and Exists() simply returns false. The
+        // explicit containment check in GetPath would throw if that invariant
+        // ever regressed.
+        Assert.False(store.Exists(new ReminderId(maliciousId)));
+    }
+
     public void Dispose()
     {
         if (Directory.Exists(_basePath))

src/Netclaw.Actors/Reminders/ReminderDefinitionStore.cs

@@ -150,8 +150,22 @@ public bool Delete(ReminderId id)
 
     private string GetPath(ReminderId id)
     {
+        // Uri.EscapeDataString escapes path separators and absolute-path
+        // markers (/, \, :, control chars), so the encoded value cannot encode a
+        // traversal. The post-canonicalization containment check below is the
+        // belt-and-suspenders that CodeQL also recognizes as a path sanitizer.
+        // (cs/path-injection)
         var encoded = Uri.EscapeDataString(id.Value);
-        return Path.Combine(_directory, $"{encoded}.json");
+        var baseDir = Path.GetFullPath(_directory);
+        var candidate = Path.GetFullPath(Path.Combine(baseDir, $"{encoded}.json"));
+        var baseWithSep = baseDir.EndsWith(Path.DirectorySeparatorChar)
+            ? baseDir
+            : baseDir + Path.DirectorySeparatorChar;
+        if (!candidate.StartsWith(baseWithSep, StringComparison.Ordinal))
+            throw new ArgumentException(
+                $"Reminder id '{id.Value}' resolves outside the reminders directory.",
+                nameof(id));
+        return candidate;
     }
 
     private void PruneInvalidDefinitions()

src/Netclaw.Daemon.Tests/Services/DaemonLifecycleNotifierTests.cs

@@ -82,6 +82,38 @@ public void NotifyCrashing_EmitsCriticalAlert()
         Assert.Equal("C123/171313.123", alert.Context["latest_session_id"]);
     }
 
+    [Theory]
+    [InlineData("ok\r\nlevel=critical attacker=admin", "oklevel=critical attacker=admin")]
+    [InlineData("ok\nfake-line", "okfake-line")]
+    [InlineData("ok\u2028fake-line", "okfake-line")]   // U+2028 LINE SEPARATOR
+    [InlineData("ok\u2029fake-line", "okfake-line")]   // U+2029 PARAGRAPH SEPARATOR
+    [InlineData("ok\0NUL", "okNUL")]
+    public void NotifyShutdown_strips_log_line_breakers_from_reason(string raw, string expected)
+    {
+        _sut.NotifyShutdown(raw);
+
+        var alert = Assert.Single(_sink.Alerts);
+        Assert.Equal(expected, alert.Context!["reason"]);
+        Assert.Equal($"Netclaw daemon stopping: {expected}", alert.Summary);
+    }
+
+    [Fact]
+    public void NotifyShutdown_truncates_reason_at_200_chars_without_splitting_surrogate_pair()
+    {
+        // 99 emoji = 198 UTF-16 chars (2 each), then 1 emoji at positions
+        // 198–199, then plain ASCII tail. Position 200 would split the 100th
+        // emoji's surrogate pair if we naively cut at char index 200.
+        var emoji = "😀"; // U+1F600 GRINNING FACE
+        var raw = string.Concat(Enumerable.Repeat(emoji, 100)) + "TAIL";
+
+        _sut.NotifyShutdown(raw);
+
+        var reason = Assert.Single(_sink.Alerts).Context!["reason"];
+        Assert.True(reason.Length <= 200);
+        Assert.False(char.IsHighSurrogate(reason[^1]),
+            "truncated reason ended on a high surrogate — dangling surrogate would break UTF-8 encoders downstream");
+    }
+
     private sealed class RecordingSink : IOperationalNotificationSink
     {
         public List<OperationalAlert> Alerts { get; } = [];

src/Netclaw.Daemon/Services/DaemonLifecycleNotifier.cs

@@ -4,6 +4,7 @@
 // </copyright>
 // -----------------------------------------------------------------------
 using System.Globalization;
+using System.Text;
 using Microsoft.Extensions.Logging;
 using Netclaw.Configuration;
 
@@ -60,12 +61,17 @@ public void NotifyStarted()
     public void NotifyShutdown(string reason, IReadOnlyDictionary<string, string>? additionalContext = null)
     {
         var pid = Environment.ProcessId;
-        _logger.LogInformation("Netclaw daemon stopping (PID {Pid}, reason: {Reason})", pid, reason);
+        // `reason` reaches this method straight from the HTTP query string
+        // (ShutdownDaemonRequest in LifecycleEndpointRouteBuilderExtensions), so a
+        // caller can inject CR/LF into log lines. Sanitize before logging or
+        // propagating into the alert context. (cs/log-forging)
+        var safeReason = SanitizeReason(reason);
+        _logger.LogInformation("Netclaw daemon stopping (PID {Pid}, reason: {Reason})", pid, safeReason);
 
         var context = new Dictionary<string, string>
         {
             ["pid"] = pid.ToString(CultureInfo.InvariantCulture),
-            ["reason"] = reason,
+            ["reason"] = safeReason,
         };
 
         if (additionalContext is not null)
@@ -78,7 +84,7 @@ public void NotifyShutdown(string reason, IReadOnlyDictionary<string, string>? a
             _timeProvider,
             type: "daemon.stopping",
             category: AlertType.DaemonStopping,
-            summary: $"Netclaw daemon stopping: {reason}",
+            summary: $"Netclaw daemon stopping: {safeReason}",
             severity: AlertSeverity.Info,
             source: pid.ToString(CultureInfo.InvariantCulture),
             context: context));
@@ -99,18 +105,22 @@ public void NotifyCrashing(
         var exceptionMessage = string.IsNullOrWhiteSpace(exception.Message)
             ? "(no exception message)"
             : exception.Message;
+        // Today every caller passes a code-side constant for `reason`, but
+        // sanitizing here keeps the contract symmetric with NotifyShutdown and
+        // forecloses on future callers that might forward HTTP input.
+        var safeReason = SanitizeReason(reason);
 
         _logger.LogCritical(
             exception,
             "Netclaw daemon crashing (PID {Pid}, reason: {Reason}, exceptionType: {ExceptionType})",
             pid,
-            reason,
+            safeReason,
             exceptionType);
 
         var context = new Dictionary<string, string>(StringComparer.Ordinal)
         {
             ["pid"] = pid.ToString(CultureInfo.InvariantCulture),
-            ["reason"] = reason,
+            ["reason"] = safeReason,
             ["exceptionType"] = exceptionType,
             ["exceptionMessage"] = Trim(exceptionMessage, 400),
         };
@@ -130,7 +140,7 @@ public void NotifyCrashing(
                 _timeProvider,
                 type: "daemon.crashing",
                 category: AlertType.DaemonCrashed,
-                summary: $"Netclaw daemon crashing: {reason} ({exceptionType})",
+                summary: $"Netclaw daemon crashing: {safeReason} ({exceptionType})",
                 severity: AlertSeverity.Critical,
                 source: pid.ToString(CultureInfo.InvariantCulture),
                 context: context));
@@ -148,4 +158,58 @@ private static string Trim(string value, int maxChars)
 
         return value[..maxChars];
     }
+
+    private const int MaxReasonChars = 200;
+
+    // Returns true for chars that, if left in a log line or alert summary, can
+    // act as a line terminator for at least one downstream consumer:
+    // - char.IsControl covers Cc (CR/LF/NUL/ESC/etc.)
+    // - U+2028 (LINE SEPARATOR, Zl) and U+2029 (PARAGRAPH SEPARATOR, Zp) are NOT
+    //   in Cc and so are not caught by IsControl, but JSON-line readers, many
+    //   log shippers, and pre-ES2019 JSON parsers still split on them.
+    private static bool IsLogLineBreak(char c)
+        => char.IsControl(c) || c is '\u2028' or '\u2029';
+
+    private static string SanitizeReason(string reason)
+    {
+        if (string.IsNullOrEmpty(reason))
+            return string.Empty;
+
+        var hasBreak = false;
+        foreach (var ch in reason)
+        {
+            if (IsLogLineBreak(ch))
+            {
+                hasBreak = true;
+                break;
+            }
+        }
+
+        if (!hasBreak)
+            return TrimAtCharBoundary(reason, MaxReasonChars);
+
+        // Strip — don't space-replace. Space would let a CR/LF payload still
+        // pass through as a plausible field separator to key=value structured
+        // log parsers ("reason=ok\nlevel=critical" → "reason=ok level=critical").
+        var buf = new StringBuilder(reason.Length);
+        foreach (var ch in reason)
+        {
+            if (!IsLogLineBreak(ch))
+                buf.Append(ch);
+        }
+        return TrimAtCharBoundary(buf.ToString(), MaxReasonChars);
+    }
+
+    private static string TrimAtCharBoundary(string value, int maxChars)
+    {
+        if (value.Length <= maxChars)
+            return value;
+
+        // Don't split a surrogate pair at the truncation boundary — a dangling
+        // high surrogate makes downstream UTF-8 encoders emit U+FFFD or throw.
+        var cut = maxChars;
+        if (char.IsHighSurrogate(value[cut - 1]))
+            cut -= 1;
+        return value[..cut];
+    }
 }

src/Netclaw.Daemon/Webhooks/WebhookEndpointRouteBuilderExtensions.cs

@@ -43,10 +43,16 @@ public static IEndpointRouteBuilder MapWebhookEndpoints(this IEndpointRouteBuild
 
             if (!routeCatalog.TryGetRoute(route, out var registeredRoute))
             {
-                WebhookTelemetry.RecordRouteNotFound(route);
+                // Route comes from the URL path and is URL-decoded by ASP.NET routing,
+                // so a caller can inject CR/LF and other control chars — both as a
+                // log-forging vector and as an unbounded metric-tag cardinality vector
+                // against the `route` dimension on WebhookTelemetry counters. Sanitize
+                // once and use the safe value for both surfaces. (cs/log-forging)
+                var safeRoute = SanitizeWebhookId(route);
+                WebhookTelemetry.RecordRouteNotFound(safeRoute);
                 logger.LogWarning(
                     "Webhook rejected route={Route} reason={Reason} remote_ip={RemoteIp} delivery_id={DeliveryId} event_type={EventType}",
-                    route, "route_not_found", remoteIp, (string?)null, (string?)null);
+                    safeRoute, "route_not_found", remoteIp, (string?)null, (string?)null);
                 return TypedResults.NotFound();
             }