Fix redaction, condition and template handling in the engine

CHANGELOG.md

@@ -22,9 +22,16 @@ Planned version: **1.5.4**.
   prompts must pass it (#708).
 - **Sign in once more per profile with `np auth login` after the update.** Every profile now has its
   own session file, so profiles such as `prod.a` and `prod_a` no longer share one login (#620).
+- A `matches` condition with an invalid pattern, a regex timeout or an input longer than 1,048,576
+  characters fails the run and names the edge, instead of counting as "no match" (#680).
 
 ### Fixed
 
+- **Running workflows**
+  - A breakpoint condition can use the run's inputs (`{{manual.*}}`) (#685).
+  - 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).
 - **CLI and MCP**
   - `np exec --watch` shows the final state even when the live connection drops the last event
     (#725).
@@ -39,6 +46,10 @@ Planned version: **1.5.4**.
 - Logging out with the CLI also ends the MCP server's session right away (#712).
 - MCP alerting output shows webhook targets only as scheme, host and port, because the rest of the
   URL can contain tokens (#730).
+- GitHub tokens in step output are masked except for their prefix such as `ghp_`; before, the whole
+  token stayed visible (#681).
+- Log steps and returned data mask private keys before long values are shortened, also when the end
+  marker is missing (#682).
 
 ## [1.5.3] - 2026-10-30
 

docs/adr/0015-fail-closed-edge-conditions.md

@@ -30,6 +30,9 @@ reported success.
 - At run time the evaluator throws `ConditionEvaluationException` for such a condition. The
   scheduler fails the run and names the edge (`Edge 'e1' (a -> b) has an invalid condition: …`).
   The edge is neither opened nor silently skipped.
+  Regex compilation errors, timeouts, overlong patterns, and inputs beyond the 1,048,576-character regex
+  evaluation limit follow this failure path too. The evaluator never matches a truncated prefix
+  or turns an invalid regex into `false` that a surrounding `not` could invert.
 - Evaluation is three-valued. A variable operand without a value in this run — a step without a
   result, a missing output parameter, a global or trigger input that does not exist — makes its
   comparison undecidable. Undecidable never satisfies a condition, `not` keeps it undecidable, and

src/NodePilot.Engine/Activities/LogActivity.cs

@@ -47,13 +47,11 @@
                 });
             }
 
-            // Sanitize before the message touches any logger: strip CR/LF (log-forging), drop
-            // ANSI escape sequences (parser-confusion), trim to MaxMessageChars, then run
-            // through the secret redactor so a `message: "pw={{step.param.pw}}"` won't leak.
-            var sanitized = SanitizeForLog(rawMessage);
+            // Redact complete values before flattening lines or truncating delimiters.
+            var sanitized = SanitizeForLog(_redactor.Redact(rawMessage) ?? rawMessage);
             if (sanitized.Length > MaxMessageChars)
                 sanitized = sanitized[..MaxMessageChars] + "…(truncated)";
-            var message = _redactor.Redact(sanitized) ?? sanitized;
+            var message = sanitized;
 
             // SupportLog=true marks user-authored log lines as operator-relevant; the sub-sink in
             // LoggingSetup filters on this scope property and also writes the entry into

src/NodePilot.Engine/Activities/ReturnDataActivity.cs

@@ -67,8 +67,8 @@ public async Task<ActivityResult> ExecuteAsync(StepExecutionContext context, Jso
         var persistParams = new Dictionary<string, string>();
         foreach (var prop in dataEl.EnumerateObject())
         {
-            var raw = Cap(PowerShellOperation.JsonElementToScalarString(prop.Value));
-            outputParams[prop.Name] = raw;
+            var raw = PowerShellOperation.JsonElementToScalarString(prop.Value);
+            outputParams[prop.Name] = Cap(raw);
             // Redact per value, never across the finished document. A careless workflow that
             // echoes a secret here would otherwise persist it unmasked and hand it to any
             // startWorkflow parent — but several default patterns have value classes that do not

src/NodePilot.Engine/Activities/TextFileEditActivity.cs

@@ -20,7 +20,7 @@ namespace NodePilot.Engine.Activities;
 /// <c>FileSystemOperation:TextEdit:MaxFileSizeMB</c>) caps the size this activity accepts.
 ///
 /// Every mutation is written to <c>&lt;path&gt;.nodepilot-tmp-&lt;guid&gt;</c> and then
-/// <c>Move-Item -Force</c>d over the original, so a failed write leaves the original untouched.
+/// atomically replaces the original while retaining its access-control metadata.
 /// <c>backupSuffix</c> copies the file to a sibling <i>before</i> the mutation, which keeps the
 /// pre-edit content available even after a successful edit.
 /// </summary>
@@ -379,8 +379,25 @@ function Write-NpFile {
             if ($DryRun) { return }
             $tmp = $Path + '.nodepilot-tmp-' + ([guid]::NewGuid().ToString('N'))
             $bytes = $Encoding.GetPreamble() + $Encoding.GetBytes($Text)
-            [System.IO.File]::WriteAllBytes($tmp, $bytes)
-            Move-Item -LiteralPath $tmp -Destination $Path -Force
+            $existing = [System.IO.File]::Exists($Path)
+            try {
+                if ($existing) {
+                    # Protect the staging file before it contains any original or edited data.
+                    $acl = Get-Acl -LiteralPath $Path
+                    [System.IO.File]::WriteAllBytes($tmp, [byte[]]@())
+                    Set-Acl -LiteralPath $tmp -AclObject $acl -ErrorAction Stop
+                }
+                [System.IO.File]::WriteAllBytes($tmp, $bytes)
+                if ($existing) {
+                    # ReplaceFile preserves the destination DACL and metadata. Do not fall back
+                    # to Move-Item: that would replace protected permissions with inherited ones.
+                    [System.IO.File]::Replace($tmp, $Path, [System.Management.Automation.Language.NullString]::Value)
+                } else {
+                    [System.IO.File]::Move($tmp, $Path)
+                }
+            } finally {
+                if ([System.IO.File]::Exists($tmp)) { [System.IO.File]::Delete($tmp) }
+            }
         }
 
         function Backup-NpFile {

src/NodePilot.Engine/Conditions/ConditionEvaluator.cs

@@ -329,15 +329,20 @@ private static bool CompareNumeric(string a, string b, Func<decimal, decimal, bo
 
     private static bool TryRegexMatch(string value, string pattern)
     {
-        if (string.IsNullOrEmpty(pattern) || pattern.Length > RegexMaxPatternLength) return false;
+        if (string.IsNullOrEmpty(pattern) || pattern.Length > RegexMaxPatternLength)
+            throw new ConditionEvaluationException("regex pattern is empty or exceeds the length limit");
+        if (value.Length > RegexMaxInputLength)
+            throw new ConditionEvaluationException("regex input exceeds the length limit");
         var rx = GetCachedRegex(pattern);
-        if (rx is null) return false;
+        if (rx is null) throw new ConditionEvaluationException("regex pattern is invalid");
         try
         {
-            var input = value.Length > RegexMaxInputLength ? value[..RegexMaxInputLength] : value;
-            return rx.IsMatch(input);
+            return rx.IsMatch(value);
+        }
+        catch (RegexMatchTimeoutException)
+        {
+            throw new ConditionEvaluationException("regex evaluation exceeded its time limit");
         }
-        catch (RegexMatchTimeoutException) { return false; }
     }
 
     private static string ResolveTemplates(string raw, ConditionContext ctx)

src/NodePilot.Engine/Execution/StepRunner.cs

@@ -49,25 +49,28 @@
         IReadOnlyDictionary<string, ActivityResult> previousResults,
         IReadOnlyDictionary<string, string> outputVariableToStepId,
         IReadOnlyDictionary<string, string> globalVariables,
-        IReadOnlyDictionary<string, string>? manualParameters = null)
+        IReadOnlyDictionary<string, string>? manualParameters = null,
+        ICollection<string>? unresolved = null)
     {
         // runScript AND custom:<key> activities resolve {{...}} inside their own executor with
         // PowerShell-safe single-quote quoting, so the generic resolver must leave their config
         // verbatim (a custom activity is a runScript preset).
         if (string.Equals(activityType, "runScript", StringComparison.OrdinalIgnoreCase)
             || CustomActivityType.IsCustomType(activityType))
         {
+            if (unresolved is not null)
+                foreach (var token in FindUnresolvedStepReferences(activityType, config)) unresolved.Add(token);
             return config;
         }
 
         if (FieldsNotToResolve.TryGetValue(activityType ?? string.Empty, out var protectedFields))
         {
             return VariableResolver.ResolveVariablesExcept(
-                config, previousResults, outputVariableToStepId, globalVariables, protectedFields, manualParameters);
+                config, previousResults, outputVariableToStepId, globalVariables, protectedFields, manualParameters, unresolved);
         }
 
         return VariableResolver.ResolveVariables(
-            config, previousResults, outputVariableToStepId, globalVariables, manualParameters);
+            config, previousResults, outputVariableToStepId, globalVariables, manualParameters, unresolved);
     }
 
     internal StepRunner(
@@ -228,9 +231,10 @@
             // and routed through ResolveVariablesExcept, which substitutes every OTHER
             // field but passes the protected ones through verbatim. The activity executor
             // then rejects the call if the protected text still carries {{...}} residue.
+            var unresolved = new List<string>();
             var configForExecution = ResolveConfigForExecution(
                 node.Type, node.Data.Config, previousResults, outputVariableToStepId, globalVariables,
-                inputParameters);
+                inputParameters, unresolved);
 
             // T-7.1: Fail the step early when step-pattern placeholders survive resolution
             // unsubstituted. This surfaces typos, deleted-step references, and wrong
@@ -245,7 +249,6 @@
             // worst outcome available. The literal reaches PowerShell as code, the step
             // succeeds, and the run reports green having written a placeholder string wherever
             // the real value belonged — measured end to end as "Ergebnis: {sibling.output}".
-            var unresolved = FindUnresolvedStepReferences(node.Type, configForExecution);
             if (unresolved.Count > 0)
             {
                 var resolvesItsOwnTemplates =
@@ -267,7 +270,7 @@
 
             if (debug is not null && (node.Data.Breakpoint || debug.StepOverArmed))
             {
-                var shouldPause = ShouldPauseForDebug(node, previousResults, outputVariableToStepId, globalVariables, debug);
+                var shouldPause = ShouldPauseForDebug(node, previousResults, outputVariableToStepId, globalVariables, debug, inputParameters);
                 if (shouldPause)
                 {
                     var overrides = await _debugCoordinator.HandlePauseAsync(
@@ -666,14 +669,15 @@
         IReadOnlyDictionary<string, ActivityResult> previousResults,
         IReadOnlyDictionary<string, string> outputVariableToStepId,
         IReadOnlyDictionary<string, string> globalVariables,
-        DebugHandle debug)
+        DebugHandle debug,
+        IReadOnlyDictionary<string, string>? inputParameters = null)
     {
         if (!node.Data.Breakpoint || debug.StepOverArmed)
             return true;
         if (string.IsNullOrWhiteSpace(node.Data.BreakpointCondition))
             return true;
 
-        var resolved = VariableResolver.ResolveStringValue(node.Data.BreakpointCondition, previousResults, outputVariableToStepId, globalVariables) ?? "";
+        var resolved = VariableResolver.ResolveStringValue(node.Data.BreakpointCondition, previousResults, outputVariableToStepId, globalVariables, inputParameters) ?? "";
         return resolved.Length > 0
             && !resolved.Equals("false", StringComparison.OrdinalIgnoreCase)
             && !resolved.Equals("0", StringComparison.OrdinalIgnoreCase)

src/NodePilot.Engine/Execution/VariableResolver.cs

@@ -59,7 +59,8 @@ internal static string ReplaceTemplatesOnce(
         string text,
         Func<string, string?> resolveGlobal,
         Func<string, string?> resolveManual,
-        Func<string, string, string?, string?> resolveStep)
+        Func<string, string, string?, string?> resolveStep,
+        ICollection<string>? unresolved = null)
         => AnyTemplatePattern.Replace(text, match =>
         {
             if (match.Groups["global"].Success && resolveGlobal(match.Groups["global"].Value) is { } global)
@@ -70,15 +71,23 @@ internal static string ReplaceTemplatesOnce(
             if (match.Groups["step"].Success)
             {
                 var param = match.Groups["param"].Success ? match.Groups["param"].Value : null;
-                return resolveStep(match.Groups["step"].Value, match.Groups["tail"].Value, param) ?? match.Value;
+                if (resolveStep(match.Groups["step"].Value, match.Groups["tail"].Value, param) is { } value)
+                    return value;
+                unresolved?.Add(match.Value);
+                return match.Value;
             }
 
             // An unknown {{globals.output}} or {{manual.output}} is also a reference to a step
             // named "globals" or "manual", which is how the step pattern reads it.
             var stepMatch = StepPattern.Match(match.Value);
-            if (!stepMatch.Success || stepMatch.Length != match.Length) return match.Value;
-            var stepParam = stepMatch.Groups[3].Success ? stepMatch.Groups[3].Value : null;
-            return resolveStep(stepMatch.Groups[1].Value, stepMatch.Groups[2].Value, stepParam) ?? match.Value;
+            if (stepMatch.Success && stepMatch.Length == match.Length)
+            {
+                var stepParam = stepMatch.Groups[3].Success ? stepMatch.Groups[3].Value : null;
+                if (resolveStep(stepMatch.Groups[1].Value, stepMatch.Groups[2].Value, stepParam) is { } value)
+                    return value;
+            }
+            unresolved?.Add(match.Value);
+            return match.Value;
         });
 
     /// <summary>Reads a step result value for a template tail; null when the tail cannot resolve.</summary>
@@ -261,12 +270,13 @@ internal static JsonElement ResolveVariablesExcept(
         IReadOnlyDictionary<string, string>? outputVariableToStepId,
         IReadOnlyDictionary<string, string>? globalVariables,
         IReadOnlySet<string> doNotResolveFields,
-        IReadOnlyDictionary<string, string>? manualParameters = null)
+        IReadOnlyDictionary<string, string>? manualParameters = null,
+        ICollection<string>? unresolved = null)
     {
         if (doNotResolveFields is null || doNotResolveFields.Count == 0
             || config.ValueKind != JsonValueKind.Object)
         {
-            return ResolveVariables(config, results, outputVariableToStepId, globalVariables, manualParameters);
+            return ResolveVariables(config, results, outputVariableToStepId, globalVariables, manualParameters, unresolved);
         }
 
         // Re-assemble the object property-by-property: protected fields are passed through
@@ -288,7 +298,7 @@ internal static JsonElement ResolveVariablesExcept(
             }
             else
             {
-                var subResolved = ResolveVariables(prop.Value, results, outputVariableToStepId, globalVariables, manualParameters);
+                var subResolved = ResolveVariables(prop.Value, results, outputVariableToStepId, globalVariables, manualParameters, unresolved);
                 sb.Append(subResolved.GetRawText());
             }
         }
@@ -302,7 +312,8 @@ internal static JsonElement ResolveVariablesExcept(
     internal static JsonElement ResolveVariables(JsonElement config, IReadOnlyDictionary<string, ActivityResult> results,
         IReadOnlyDictionary<string, string>? outputVariableToStepId,
         IReadOnlyDictionary<string, string>? globalVariables = null,
-        IReadOnlyDictionary<string, string>? manualParameters = null)
+        IReadOnlyDictionary<string, string>? manualParameters = null,
+        ICollection<string>? unresolved = null)
     {
         var configJson = config.GetRawText();
         if (string.IsNullOrEmpty(configJson) || !configJson.Contains("{{"))
@@ -319,7 +330,7 @@ internal static JsonElement ResolveVariables(JsonElement config, IReadOnlyDictio
                 ? JsonEscape(manual) : null,
             (step, tail, param) => variableMap.TryGetValue(step, out var result)
                 && ReadStepValue(result, tail, param) is { } value
-                    ? JsonEscape(value) : null);
+                    ? JsonEscape(value) : null, unresolved);
 
         // Dispose the JsonDocument so its pooled buffers return to the ArrayPool. Returning
         // RootElement directly would leak the buffer; Clone() detaches the element from the

src/NodePilot.Engine/Security/OutputRedactor.cs

@@ -60,13 +60,13 @@ public sealed class OutputRedactor : IAuditDetailsRedactor
 
     // "(BEGIN)[\s\S]{MinBody,MaxBody}?(END)" without the regex: retried from every BEGIN marker,
     // the lazy body is quadratic under backtracking. Same matches, linear time.
-    private sealed record PemBlocks(Regex Begin, Regex End, int MinBody, int MaxBody);
+    private sealed record PemBlocks(Regex Begin, Regex End, int MinBody, int MaxBody, bool RedactUnterminated = false);
 
     // PEM-formatted private keys — blank the body between markers.
     private static readonly PemBlocks PrivateKeyPem = new(
         new(@"-----BEGIN (?:RSA |EC |DSA |OPENSSH |ENCRYPTED |)PRIVATE KEY-----", RxOpts, NoTimeout),
         new(@"-----END (?:RSA |EC |DSA |OPENSSH |ENCRYPTED |)PRIVATE KEY-----", RxOpts, NoTimeout),
-        MinBody: 0, MaxBody: int.MaxValue);
+        MinBody: 0, MaxBody: int.MaxValue, RedactUnterminated: true);
 
     // Any PEM block (certificate, public key, ...) — not just private key.
     private static readonly PemBlocks AnyPem = new(
@@ -97,7 +97,7 @@ private sealed record PemBlocks(Regex Begin, Regex End, int MinBody, int MaxBody
         PrivateKeyPem,
         // AWS access key IDs and GitHub tokens (shape-based — catches accidental Write-Host)
         new Regex(@"\b(AKIA|ASIA)[A-Z0-9]{16}\b", RegexOptions.Compiled, NoTimeout),
-        new Regex(@"\b(gh[pousr]_[A-Za-z0-9]{20,})\b", RegexOptions.Compiled, NoTimeout),
+        new Regex(@"\b(gh[pousr]_)[A-Za-z0-9]{20,}\b", RegexOptions.Compiled, NoTimeout),
         // M-5 widen the catch-all set:
         // JWT shape: 3 dot-separated base64url segments (at least 10 chars each).
         new Regex(@"eyJ[A-Za-z0-9_\-]{10,}\.[A-Za-z0-9_\-]{10,}\.[A-Za-z0-9_\-]{10,}", RegexOptions.Compiled, NoTimeout),
@@ -234,6 +234,13 @@ private static string RedactPemBlocks(string s, PemBlocks pem)
                 copied = end.Index + end.Length;
                 begin = pem.Begin.Match(s, copied);
             }
+            else if (!end.Success && pem.RedactUnterminated)
+            {
+                sb ??= new StringBuilder(s.Length);
+                sb.Append(s, copied, begin.Index - copied).Append(begin.Value).Append(Placeholder);
+                copied = s.Length;
+                break;
+            }
             else
             {
                 begin = pem.Begin.Match(s, begin.Index + 1);