From 76bda6cf558582c7cface35f3f345e225615cc16 Mon Sep 17 00:00:00 2001 From: EugeneTes Date: Sat, 26 Sep 2026 19:32:40 +0200 Subject: [PATCH] fix(processes): address review findings - Fill TimeZoneId only for a configured session/host zone with a resolvable IANA id (IExtensionHost.TimeZoneId); otherwise children inherit the host's TZ instead of TZ=Local/UTC. - Host hardening logs a warning instead of throwing when libc/prctl can't be resolved. - A hook or process_spawn handler returning a null or incomplete context fails the spawn with InvalidOperationException, so bash reports "process environment hook failed". - Move tool-result redaction into the Agent slice with a behavioural no-change check. - ProcessSpawnContext.ApplyTo(psi) sets cwd and replaces the environment; used by bash and ExecAsync. - Split the Telegram process environment types into their own files; move the Extensions.Tests NoHostHardening under Support/; check the Claude Code assembly never references the spawn policy. - Spec and docs: TZ inheritance, incomplete contexts, ApplyTo, async spawn handlers, root skip. --- docs/extensions.md | 15 ++-- ...6-09-26-process-environment-hook-design.md | 23 +++-- .../ProcessEnvironmentSection.cs | 10 +++ ...Setup.cs => TelegramProcessEnvironment.cs} | 9 -- .../ProcessSpawnContextExtensions.cs | 20 +++++ .../Features/Agent/AgentSession.Host.cs | 3 + .../Features/Agent/AgentSession.Tools.cs | 1 - .../Features/Agent/ToolResultRedaction.cs | 85 +++++++++++++++++++ .../Features/Extensions/ExtensionApi.cs | 4 +- .../Features/Extensions/ExtensionRunner.cs | 5 +- .../Features/Extensions/IExtensionHost.cs | 5 ++ .../Processes/HostProcessHardening.cs | 11 ++- .../Features/Processes/ProcessSpawnPolicy.cs | 4 +- .../Features/Processes/SecretRedactor.cs | 52 ------------ .../Features/Tools/BuiltIn/BashTool.cs | 5 +- .../Hosting/AgenticCoreOptions.cs | 3 +- .../Hosting/TimeZoneIds.cs | 13 ++- .../ClaudeArgsProcessPolicyTests.cs | 24 ++++++ .../{ => Support}/NoHostHardening.cs | 2 +- .../Features/SessionTimeZoneTests.cs | 18 ++++ .../Processes/HostProcessHardeningTests.cs | 9 ++ .../Processes/ProcessSessionBindingTests.cs | 31 ++++++- .../Processes/ProcessSpawnPolicyTests.cs | 16 ++++ .../Processes/SecretRedactorTests.cs | 15 ++++ .../Support/StubExtensionHost.cs | 1 + 25 files changed, 289 insertions(+), 95 deletions(-) create mode 100644 hosts/Volcanic.AgenticCore.Telegram/ProcessEnvironmentSection.cs rename hosts/Volcanic.AgenticCore.Telegram/{ProcessEnvironmentSetup.cs => TelegramProcessEnvironment.cs} (80%) create mode 100644 src/Volcanic.AgenticCore.Abstractions/Processes/ProcessSpawnContextExtensions.cs create mode 100644 src/Volcanic.AgenticCore/Features/Agent/ToolResultRedaction.cs rename tests/Volcanic.AgenticCore.Extensions.Tests/{ => Support}/NoHostHardening.cs (89%) diff --git a/docs/extensions.md b/docs/extensions.md index 0920e0e..aa57527 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -151,25 +151,26 @@ var spawn = await ctx.PrepareProcessAsync(new ProcessSpawnRequest Origin = "my-extension", }, ct); -var psi = new ProcessStartInfo(spawn.Command) { WorkingDirectory = spawn.Cwd, UseShellExecute = false }; +var psi = new ProcessStartInfo(spawn.Command) { UseShellExecute = false }; foreach (var a in spawn.Arguments) psi.ArgumentList.Add(a); -psi.Environment.Clear(); -foreach (var (k, v) in spawn.Environment) psi.Environment[k] = v; +spawn.ApplyTo(psi); // WorkingDirectory = spawn.Cwd, Environment cleared, then spawn.Environment copied in ``` - In a session, call `ctx.PrepareProcessAsync`. It fills `Scope`, `SessionId` and (unless you set it) `TimeZoneId` from the session, runs the policy, then emits `ProcessSpawnEvent` to extensions in load order. + `TimeZoneId` is filled only when the session or host configured a zone; otherwise the child inherits the host's `TZ`. Without a session (during activation, or from a process-wide service), call `api.Processes.Prepare(request)`: only the host hooks run. -- The returned `ProcessSpawnContext.Environment` is the **complete** environment. Clear - `ProcessStartInfo.Environment` and copy it in; don't merge it with the inherited one. +- The returned `ProcessSpawnContext.Environment` is the **complete** environment. `spawn.ApplyTo(psi)` clears + `ProcessStartInfo.Environment` and copies it in (and sets the working directory); don't merge it with the inherited one. - The policy starts from the host's environment, removes what the deny-list (`*_API_KEY`, `*_TOKEN`, `*_SECRET`, `*_PASSWORD`, their `Section__Key` forms, and a few names), `Allow` and `SecretValues` reject, then applies `request.Environment`, then sets `TZ` from `TimeZoneId`, then runs the host hooks. - **Variables you pass explicitly are applied after the deny-list and never filtered.** A process that needs a token gets it only when you pass it, and no other process sees it. A null value removes the variable. -- Host hooks and `ProcessSpawnEvent` handlers are synchronous rewrites of command, arguments, cwd and environment; - do any I/O in the spawner. A hook or handler that throws fails the spawn (the exception comes out of +- Host hooks (`ProcessSpawnHook`) are synchronous, pure rewrites of command, arguments, cwd and environment. + `ProcessSpawnEvent` handlers are ordinary (async) extension handlers that may replace `Context`; keep them free of + slow I/O, because they delay the spawn. A hook or handler that throws fails the spawn (the exception comes out of `PrepareProcessAsync`), and nothing falls back to the inherited environment. Report it the way you report a failed start; the built-in `bash` returns `process environment hook failed: `. - `ExecAsync` already does all of this with `Purpose = ExtensionExec` and `Origin` = your extension's name. diff --git a/docs/superpowers/specs/2026-09-26-process-environment-hook-design.md b/docs/superpowers/specs/2026-09-26-process-environment-hook-design.md index 4edec42..5b07e94 100644 --- a/docs/superpowers/specs/2026-09-26-process-environment-hook-design.md +++ b/docs/superpowers/specs/2026-09-26-process-environment-hook-design.md @@ -120,8 +120,10 @@ public interface IProcessSpawnPolicy ``` - `ProcessSpawnContext.Environment` is the **complete** environment: spawners clear `ProcessStartInfo.Environment` - and copy it in. Names compare ordinally on Unix and case-insensitively on Windows, as the OS does. -- Hooks are synchronous and pure, as in pi. Anything needing I/O belongs in the spawner. + and copy it in, through `ProcessSpawnContextExtensions.ApplyTo(psi)` (Abstractions), which also sets the working + directory. Names compare ordinally on Unix and case-insensitively on Windows, as the OS does. +- Hooks are synchronous and pure, as in pi. Anything needing I/O belongs in the spawner. A hook that returns null, or a + context without command, arguments, cwd or environment, fails the spawn like a throwing hook. ### 2. Configuration — `AgenticCoreOptions.ProcessEnvironment` @@ -196,8 +198,12 @@ public sealed class ProcessSpawnEvent(ProcessSpawnContext context) : ExtensionEv ``` `PrepareProcessAsync` fills `Scope`, `SessionId` and (unless given) `TimeZoneId` from the session (the session's time -zone comes from `AgentSessionOptions.TimeZone`, scheduler spec §8), calls `Processes.Prepare`, then emits -`ProcessSpawnEvent` to extensions in load order. Each handler may replace `Context`. There's no `Block`: refusing a +zone comes from `AgentSessionOptions.TimeZone`, else `AgenticCoreOptions.DefaultTimeZone`, scheduler spec §8), calls +`Processes.Prepare`, then emits `ProcessSpawnEvent` to extensions in load order. When neither option sets a zone, or the +zone has no IANA id that resolves to a system zone, `TimeZoneId` stays null and the child inherits the host's `TZ`: the +machine's local zone may be unnamed (.NET calls an unmatched `/etc/localtime` `"Local"`) or come from a POSIX rule +string, and overwriting `TZ` with that would move the child to UTC. Each handler may replace `Context`; a handler that +leaves it null or without command, arguments, cwd or environment fails the spawn with `InvalidOperationException`. There's no `Block`: refusing a command is `ToolCallEvent.Block`'s job, before any process exists. `IExtensionApi.Processes`, `IExtensionApi.Redactor` and `IExtensionContext.PrepareProcessAsync` are default interface @@ -208,7 +214,8 @@ reported as `extension_error` and the exception propagates out of `PrepareProces The session time zone members it reads (`AgentSessionOptions.TimeZone`, `AgenticCoreOptions.DefaultTimeZone`, `AgentSession.TimeZone`, `IExtensionContext.TimeZone`) are introduced with this spec, ahead of the rest of scheduler spec §8. `IExtensionContext.TimeZone` is a default interface member returning `TimeZoneInfo.Local`, so other -implementations keep compiling; `TimeZoneIds.ToIana` (core, `Hosting/`) turns the zone into the IANA id for `TimeZoneId`. +implementations keep compiling; `TimeZoneIds.ToIana` (core, `Hosting/`) turns the zone into the IANA id for `TimeZoneId` +(null when there is none that resolves). The session hands the id to the Extensions slice as `IExtensionHost.TimeZoneId`. Process-wide spawners without a session (the scheduler, MCP catalogue discovery) call `Processes.Prepare` directly (or the DI singleton), so only host hooks run for them. That is deliberate: a host-wide rule belongs in a host hook. @@ -416,12 +423,16 @@ process's environment. `Func` prctl seam that returns the errno, an `isLinux` flag and an environment reader; the public static `Apply` uses a shared instance over the real libc): - flag on + Linux → prctl called once with `PR_SET_DUMPABLE, 0`; a second `Apply` doesn't call it again; a failing - prctl logs a warning and doesn't throw; non-Linux and flag off → not called (flag off logs the warning); + prctl logs a warning and doesn't throw, and so does a prctl seam that throws `DllNotFoundException` (libc doesn't + resolve); non-Linux and flag off → not called (flag off logs the warning); - the `DOTNET_EnableDiagnostics` warning appears when the injected environment lacks it, and not when it's `0`; - Linux-only integration test with the real libc: a helper process (the test runner spawns `dotnet .dll`) applies the hardening and prints `ok`; the test then runs `cat /proc//environ` as the same user and asserts it fails with permission denied, and that the same read succeeds against a helper started with `HardenHostProcess=false`. The test process itself is never hardened, so test debugging keeps working. + The test is skipped when the runner is root: root (`CAP_SYS_PTRACE`) reads every `/proc` entry, and a non-root + reader couldn't read a root helper's `environ` even unhardened, so there is nothing to compare. Run it as the bot's + own non-root user. - **Redaction** (`tests/Volcanic.AgenticCore.Tests/Processes/`, `FauxProvider` + scripted test tools): - `SecretRedactor`: a value inside text becomes `[REDACTED]`; values under 8 characters are ignored; an overlapping longer secret wins; an empty set returns the same string instance; diff --git a/hosts/Volcanic.AgenticCore.Telegram/ProcessEnvironmentSection.cs b/hosts/Volcanic.AgenticCore.Telegram/ProcessEnvironmentSection.cs new file mode 100644 index 0000000..c212420 --- /dev/null +++ b/hosts/Volcanic.AgenticCore.Telegram/ProcessEnvironmentSection.cs @@ -0,0 +1,10 @@ +namespace Volcanic.AgenticCore.Telegram; + +/// The ProcessEnvironment configuration section. Its lists are added to the core defaults, not replacing them. +public sealed class ProcessEnvironmentSection +{ + public const string SectionName = "ProcessEnvironment"; + public List Deny { get; set; } = []; + public List Allow { get; set; } = []; + public bool HardenHostProcess { get; set; } = true; +} diff --git a/hosts/Volcanic.AgenticCore.Telegram/ProcessEnvironmentSetup.cs b/hosts/Volcanic.AgenticCore.Telegram/TelegramProcessEnvironment.cs similarity index 80% rename from hosts/Volcanic.AgenticCore.Telegram/ProcessEnvironmentSetup.cs rename to hosts/Volcanic.AgenticCore.Telegram/TelegramProcessEnvironment.cs index 8181d1d..14cd056 100644 --- a/hosts/Volcanic.AgenticCore.Telegram/ProcessEnvironmentSetup.cs +++ b/hosts/Volcanic.AgenticCore.Telegram/TelegramProcessEnvironment.cs @@ -2,15 +2,6 @@ using Volcanic.AgenticCore.Features.Processes; namespace Volcanic.AgenticCore.Telegram; -/// The ProcessEnvironment configuration section. Its lists are added to the core defaults, not replacing them. -public sealed class ProcessEnvironmentSection -{ - public const string SectionName = "ProcessEnvironment"; - public List Deny { get; set; } = []; - public List Allow { get; set; } = []; - public bool HardenHostProcess { get; set; } = true; -} - /// The Telegram host's process environment: section, PROCESS_ENV_ALLOW and the host's secret values. public static class TelegramProcessEnvironment { diff --git a/src/Volcanic.AgenticCore.Abstractions/Processes/ProcessSpawnContextExtensions.cs b/src/Volcanic.AgenticCore.Abstractions/Processes/ProcessSpawnContextExtensions.cs new file mode 100644 index 0000000..bca9a8d --- /dev/null +++ b/src/Volcanic.AgenticCore.Abstractions/Processes/ProcessSpawnContextExtensions.cs @@ -0,0 +1,20 @@ +using System.Diagnostics; + +namespace Volcanic.AgenticCore.Abstractions.Processes; + +/// Puts a prepared spawn on a . +public static class ProcessSpawnContextExtensions +{ + /// + /// Sets the working directory and replaces the whole environment with : + /// nothing inherited is left on the side. Command and arguments stay with the caller, which knows how to quote them. + /// + public static void ApplyTo(this ProcessSpawnContext context, ProcessStartInfo startInfo) + { + ArgumentNullException.ThrowIfNull(context); + ArgumentNullException.ThrowIfNull(startInfo); + startInfo.WorkingDirectory = context.Cwd; + startInfo.Environment.Clear(); + foreach (var (name, value) in context.Environment) startInfo.Environment[name] = value; + } +} diff --git a/src/Volcanic.AgenticCore/Features/Agent/AgentSession.Host.cs b/src/Volcanic.AgenticCore/Features/Agent/AgentSession.Host.cs index 543ee9d..cafac1f 100644 --- a/src/Volcanic.AgenticCore/Features/Agent/AgentSession.Host.cs +++ b/src/Volcanic.AgenticCore/Features/Agent/AgentSession.Host.cs @@ -7,6 +7,7 @@ using Volcanic.AgenticCore.Abstractions.Providers; using Volcanic.AgenticCore.Abstractions.Sessions; using Volcanic.AgenticCore.Abstractions.Ui; using Volcanic.AgenticCore.Features.Extensions; +using Volcanic.AgenticCore.Hosting; namespace Volcanic.AgenticCore.Features.Agent; @@ -20,6 +21,8 @@ public sealed partial class AgentSession : IExtensionHost Model? IExtensionHost.Model => _model; ThinkingLevel IExtensionHost.ThinkingLevel => _thinkingLevel; TimeZoneInfo IExtensionHost.TimeZone => TimeZone; + string? IExtensionHost.TimeZoneId => + (_options.TimeZone ?? _coreOptions.DefaultTimeZone) is { } zone ? TimeZoneIds.ToIana(zone) : null; CancellationToken IExtensionHost.Signal { get { lock (_stateLock) return _abortCts?.Token ?? CancellationToken.None; } diff --git a/src/Volcanic.AgenticCore/Features/Agent/AgentSession.Tools.cs b/src/Volcanic.AgenticCore/Features/Agent/AgentSession.Tools.cs index b0ae858..bc9a4eb 100644 --- a/src/Volcanic.AgenticCore/Features/Agent/AgentSession.Tools.cs +++ b/src/Volcanic.AgenticCore/Features/Agent/AgentSession.Tools.cs @@ -4,7 +4,6 @@ using Volcanic.AgenticCore.Abstractions.Extensions.Events; using Volcanic.AgenticCore.Abstractions.Json; using Volcanic.AgenticCore.Abstractions.Messages; using Volcanic.AgenticCore.Abstractions.Tools; -using Volcanic.AgenticCore.Features.Processes; using Volcanic.AgenticCore.Features.Tools; using Volcanic.AgenticCore.Features.Tools.BuiltIn; diff --git a/src/Volcanic.AgenticCore/Features/Agent/ToolResultRedaction.cs b/src/Volcanic.AgenticCore/Features/Agent/ToolResultRedaction.cs new file mode 100644 index 0000000..20b78f7 --- /dev/null +++ b/src/Volcanic.AgenticCore/Features/Agent/ToolResultRedaction.cs @@ -0,0 +1,85 @@ +using System.Text.Json; +using Volcanic.AgenticCore.Abstractions.Messages; +using Volcanic.AgenticCore.Abstractions.Processes; +using Volcanic.AgenticCore.Abstractions.Tools; + +namespace Volcanic.AgenticCore.Features.Agent; + +/// +/// Applies an to tool results: every text block and every string in Details. +/// Returns the original result when nothing matched, judged by returning its input. +/// +internal static class ToolResultRedaction +{ + public static ToolResult Redact(this ISecretRedactor redactor, ToolResult result) + { + var content = RedactContent(redactor, result.Content); + var details = RedactDetails(redactor, result.Details); + return content is null && details is null + ? result + : result with { Content = content ?? result.Content, Details = details ?? result.Details }; + } + + /// The redacted blocks, or null when no text block changed. + private static List? RedactContent(ISecretRedactor redactor, IReadOnlyList content) + { + List? changed = null; + for (var i = 0; i < content.Count; i++) + { + if (content[i] is not TextContent text) continue; + var redacted = redactor.Redact(text.Text); + if (Unchanged(text.Text, redacted)) continue; + changed ??= [.. content]; + changed[i] = text with { Text = redacted }; + } + return changed; + } + + /// The redacted Details, or null when no string in them changed. + private static JsonElement? RedactDetails(ISecretRedactor redactor, JsonElement? details) + { + if (details is not { } element || !NeedsRedaction(element, redactor)) return null; + var buffer = new System.Buffers.ArrayBufferWriter(); + using (var writer = new Utf8JsonWriter(buffer)) Write(writer, element, redactor); + using var document = JsonDocument.Parse(buffer.WrittenMemory); + return document.RootElement.Clone(); + } + + private static bool Unchanged(string original, string redacted) => + ReferenceEquals(original, redacted) || string.Equals(original, redacted, StringComparison.Ordinal); + + private static bool NeedsRedaction(JsonElement element, ISecretRedactor redactor) => element.ValueKind switch + { + JsonValueKind.Object => element.EnumerateObject().Any(p => NeedsRedaction(p.Value, redactor)), + JsonValueKind.Array => element.EnumerateArray().Any(e => NeedsRedaction(e, redactor)), + JsonValueKind.String => element.GetString() is { } s && !Unchanged(s, redactor.Redact(s)), + _ => false, + }; + + private static void Write(Utf8JsonWriter writer, JsonElement element, ISecretRedactor redactor) + { + switch (element.ValueKind) + { + case JsonValueKind.Object: + writer.WriteStartObject(); + foreach (var property in element.EnumerateObject()) + { + writer.WritePropertyName(property.Name); + Write(writer, property.Value, redactor); + } + writer.WriteEndObject(); + break; + case JsonValueKind.Array: + writer.WriteStartArray(); + foreach (var item in element.EnumerateArray()) Write(writer, item, redactor); + writer.WriteEndArray(); + break; + case JsonValueKind.String: + writer.WriteStringValue(redactor.Redact(element.GetString()!)); + break; + default: + element.WriteTo(writer); + break; + } + } +} diff --git a/src/Volcanic.AgenticCore/Features/Extensions/ExtensionApi.cs b/src/Volcanic.AgenticCore/Features/Extensions/ExtensionApi.cs index 86b4bdb..a10045c 100644 --- a/src/Volcanic.AgenticCore/Features/Extensions/ExtensionApi.cs +++ b/src/Volcanic.AgenticCore/Features/Extensions/ExtensionApi.cs @@ -114,15 +114,13 @@ internal sealed class ExtensionApi : IExtensionApi }, cancellationToken).ConfigureAwait(false); var psi = new ProcessStartInfo(spawn.Command) { - WorkingDirectory = spawn.Cwd, RedirectStandardOutput = true, RedirectStandardError = true, UseShellExecute = false, CreateNoWindow = true, }; foreach (var a in spawn.Arguments) psi.ArgumentList.Add(a); - psi.Environment.Clear(); - foreach (var (k, v) in spawn.Environment) psi.Environment[k] = v; + spawn.ApplyTo(psi); using var process = new Process { StartInfo = psi }; var stdout = new StringBuilder(); diff --git a/src/Volcanic.AgenticCore/Features/Extensions/ExtensionRunner.cs b/src/Volcanic.AgenticCore/Features/Extensions/ExtensionRunner.cs index e7e0f13..1a0b0be 100644 --- a/src/Volcanic.AgenticCore/Features/Extensions/ExtensionRunner.cs +++ b/src/Volcanic.AgenticCore/Features/Extensions/ExtensionRunner.cs @@ -6,7 +6,6 @@ using Volcanic.AgenticCore.Abstractions.Tools; using Volcanic.AgenticCore.Features.Commands; using Volcanic.AgenticCore.Features.Models; using Volcanic.AgenticCore.Features.Tools; -using Volcanic.AgenticCore.Hosting; namespace Volcanic.AgenticCore.Features.Extensions; @@ -133,10 +132,12 @@ public sealed class ExtensionRunner { Scope = host.Scope, SessionId = host.SessionId, - TimeZoneId = request.TimeZoneId ?? TimeZoneIds.ToIana(host.TimeZone), + TimeZoneId = request.TimeZoneId ?? host.TimeZoneId, }; var context = Processes.Prepare(filled); var e = await EmitAsync(new ProcessSpawnEvent(context), cancellationToken).ConfigureAwait(false); + if (e.Context is not { Command: not null, Cwd: not null, Environment: not null, Arguments: not null }) + throw new InvalidOperationException("A process_spawn handler returned no context, or one without command, arguments, cwd or environment."); return e.Context; } diff --git a/src/Volcanic.AgenticCore/Features/Extensions/IExtensionHost.cs b/src/Volcanic.AgenticCore/Features/Extensions/IExtensionHost.cs index 8ff1071..9e5327b 100644 --- a/src/Volcanic.AgenticCore/Features/Extensions/IExtensionHost.cs +++ b/src/Volcanic.AgenticCore/Features/Extensions/IExtensionHost.cs @@ -20,6 +20,11 @@ public interface IExtensionHost Model? Model { get; } ThinkingLevel ThinkingLevel { get; } TimeZoneInfo TimeZone { get; } + /// + /// IANA id of the zone the session or host configured, for TZ. Null when neither configured one (or it has no + /// resolvable IANA id): children then inherit the host's TZ. + /// + string? TimeZoneId { get; } CancellationToken Signal { get; } bool IsIdle(); diff --git a/src/Volcanic.AgenticCore/Features/Processes/HostProcessHardening.cs b/src/Volcanic.AgenticCore/Features/Processes/HostProcessHardening.cs index b4c0664..f9ff2ac 100644 --- a/src/Volcanic.AgenticCore/Features/Processes/HostProcessHardening.cs +++ b/src/Volcanic.AgenticCore/Features/Processes/HostProcessHardening.cs @@ -41,9 +41,16 @@ public static partial class HostProcessHardening logger.LogDebug("Host process hardening skipped: only Linux needs it"); return; } - var errno = setDumpable(0); + int errno; + try { errno = setDumpable(0); } + catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException or MarshalDirectiveException) + { + // libc didn't resolve: warn and keep starting (spec §8), like a failed prctl. + logger.LogWarning(ex, "Host process hardening failed: prctl is unavailable ({Error})", ex.GetType().Name); + errno = -1; + } if (errno == 0) logger.LogInformation("Host process hardened: not dumpable"); - else logger.LogWarning("Host process hardening failed: prctl(PR_SET_DUMPABLE, 0) failed with errno {Errno}", errno); + else if (errno > 0) logger.LogWarning("Host process hardening failed: prctl(PR_SET_DUMPABLE, 0) failed with errno {Errno}", errno); if (getEnv("DOTNET_EnableDiagnostics") != "0" && getEnv("DOTNET_EnableDiagnostics_IPC") != "0") logger.LogWarning("The .NET diagnostic IPC socket is on and hands out this process's environment to the same user; set DOTNET_EnableDiagnostics=0 before starting the host"); } diff --git a/src/Volcanic.AgenticCore/Features/Processes/ProcessSpawnPolicy.cs b/src/Volcanic.AgenticCore/Features/Processes/ProcessSpawnPolicy.cs index 0674276..d3a6a87 100644 --- a/src/Volcanic.AgenticCore/Features/Processes/ProcessSpawnPolicy.cs +++ b/src/Volcanic.AgenticCore/Features/Processes/ProcessSpawnPolicy.cs @@ -56,7 +56,9 @@ public sealed class ProcessSpawnPolicy : IProcessSpawnPolicy request.Scope, request.SessionId, request.Origin); foreach (var hook in _options.Hooks) { - var next = hook(context) ?? throw new InvalidOperationException("A process spawn hook returned null."); + var next = hook(context); + if (next is not { Command: not null, Cwd: not null, Environment: not null, Arguments: not null }) + throw new InvalidOperationException("A process spawn hook returned no context, or one without command, arguments, cwd or environment."); context = next with { Purpose = request.Purpose, Scope = request.Scope, SessionId = request.SessionId, Origin = request.Origin }; } diff --git a/src/Volcanic.AgenticCore/Features/Processes/SecretRedactor.cs b/src/Volcanic.AgenticCore/Features/Processes/SecretRedactor.cs index 7d188d9..de49251 100644 --- a/src/Volcanic.AgenticCore/Features/Processes/SecretRedactor.cs +++ b/src/Volcanic.AgenticCore/Features/Processes/SecretRedactor.cs @@ -1,7 +1,4 @@ -using System.Text.Json; -using Volcanic.AgenticCore.Abstractions.Messages; using Volcanic.AgenticCore.Abstractions.Processes; -using Volcanic.AgenticCore.Abstractions.Tools; namespace Volcanic.AgenticCore.Features.Processes; @@ -38,52 +35,3 @@ public sealed class SecretRedactor : ISecretRedactor return result; } } - -/// Applies an to tool results: every text block and every string in Details. -internal static class ToolResultRedaction -{ - public static ToolResult Redact(this ISecretRedactor redactor, ToolResult result) - { - if (redactor is SecretRedactor { IsEmpty: true }) return result; - return result with { Content = redactor.RedactContent(result.Content), Details = redactor.RedactDetails(result.Details) }; - } - - public static List RedactContent(this ISecretRedactor redactor, IEnumerable content) => - [.. content.Select(b => b is TextContent t ? t with { Text = redactor.Redact(t.Text) } : b)]; - - public static JsonElement? RedactDetails(this ISecretRedactor redactor, JsonElement? details) - { - if (details is not { } element || redactor is SecretRedactor { IsEmpty: true }) return details; - var buffer = new System.Buffers.ArrayBufferWriter(); - using (var writer = new Utf8JsonWriter(buffer)) Write(writer, element, redactor); - using var document = JsonDocument.Parse(buffer.WrittenMemory); - return document.RootElement.Clone(); - } - - private static void Write(Utf8JsonWriter writer, JsonElement element, ISecretRedactor redactor) - { - switch (element.ValueKind) - { - case JsonValueKind.Object: - writer.WriteStartObject(); - foreach (var property in element.EnumerateObject()) - { - writer.WritePropertyName(property.Name); - Write(writer, property.Value, redactor); - } - writer.WriteEndObject(); - break; - case JsonValueKind.Array: - writer.WriteStartArray(); - foreach (var item in element.EnumerateArray()) Write(writer, item, redactor); - writer.WriteEndArray(); - break; - case JsonValueKind.String: - writer.WriteStringValue(redactor.Redact(element.GetString()!)); - break; - default: - element.WriteTo(writer); - break; - } - } -} diff --git a/src/Volcanic.AgenticCore/Features/Tools/BuiltIn/BashTool.cs b/src/Volcanic.AgenticCore/Features/Tools/BuiltIn/BashTool.cs index f365869..a92a0a6 100644 --- a/src/Volcanic.AgenticCore/Features/Tools/BuiltIn/BashTool.cs +++ b/src/Volcanic.AgenticCore/Features/Tools/BuiltIn/BashTool.cs @@ -39,7 +39,6 @@ public sealed class BashTool : Tool var psi = new ProcessStartInfo { FileName = OperatingSystem.IsWindows() ? "cmd.exe" : "/bin/sh", - WorkingDirectory = spawn.Cwd, RedirectStandardOutput = true, RedirectStandardError = true, RedirectStandardInput = true, @@ -48,9 +47,7 @@ public sealed class BashTool : Tool }; if (OperatingSystem.IsWindows()) psi.Arguments = $"/d /s /c \"{spawn.Command}\""; else { psi.ArgumentList.Add("-c"); psi.ArgumentList.Add(spawn.Command); } // verbatim: a hook may prepend lines - // The child gets exactly the prepared environment, nothing inherited on the side (spec §5). - psi.Environment.Clear(); - foreach (var (name, value) in spawn.Environment) psi.Environment[name] = value; + spawn.ApplyTo(psi); // cwd and exactly the prepared environment, nothing inherited on the side (spec §5) using var process = new Process { StartInfo = psi }; var output = new StringBuilder(); diff --git a/src/Volcanic.AgenticCore/Hosting/AgenticCoreOptions.cs b/src/Volcanic.AgenticCore/Hosting/AgenticCoreOptions.cs index 8c31184..a396019 100644 --- a/src/Volcanic.AgenticCore/Hosting/AgenticCoreOptions.cs +++ b/src/Volcanic.AgenticCore/Hosting/AgenticCoreOptions.cs @@ -5,6 +5,7 @@ using Volcanic.AgenticCore.Abstractions.Ui; using Volcanic.AgenticCore.Features.Agent; using Volcanic.AgenticCore.Features.Compaction; using Volcanic.AgenticCore.Features.Extensions; +using Volcanic.AgenticCore.Features.Processes; namespace Volcanic.AgenticCore.Hosting; @@ -38,7 +39,7 @@ public sealed class AgenticCoreOptions public RetrySettings Retry { get; set; } = new(); /// Environment, hooks and secrets for every agent-requested process, and host hardening. - public Features.Processes.ProcessEnvironmentOptions ProcessEnvironment { get; set; } = new(); + public ProcessEnvironmentOptions ProcessEnvironment { get; set; } = new(); /// Custom persistence. Defaults to JSONL files under /sessions. public Func? SessionStoreFactory { get; set; } diff --git a/src/Volcanic.AgenticCore/Hosting/TimeZoneIds.cs b/src/Volcanic.AgenticCore/Hosting/TimeZoneIds.cs index 23f343e..9a0e91e 100644 --- a/src/Volcanic.AgenticCore/Hosting/TimeZoneIds.cs +++ b/src/Volcanic.AgenticCore/Hosting/TimeZoneIds.cs @@ -3,11 +3,16 @@ namespace Volcanic.AgenticCore.Hosting; /// Time zone id helpers. public static class TimeZoneIds { - /// The IANA id of (for TZ), converting a Windows id when needed. - public static string ToIana(TimeZoneInfo zone) + /// + /// The IANA id of (for TZ), converting a Windows id when needed. Null when the zone has + /// no id that resolves back to a system zone: Linux names an unmatched /etc/localtime "Local", and + /// TZ=Local would make a child run in UTC. + /// + public static string? ToIana(TimeZoneInfo zone) { ArgumentNullException.ThrowIfNull(zone); - if (zone.HasIanaId) return zone.Id; - return TimeZoneInfo.TryConvertWindowsIdToIanaId(zone.Id, out var iana) ? iana : zone.Id; + if (string.Equals(zone.Id, "Local", StringComparison.OrdinalIgnoreCase)) return null; + var id = zone.HasIanaId ? zone.Id : TimeZoneInfo.TryConvertWindowsIdToIanaId(zone.Id, out var iana) ? iana : null; + return id is not null && TimeZoneInfo.TryFindSystemTimeZoneById(id, out _) ? id : null; } } diff --git a/tests/Volcanic.AgenticCore.Extensions.Tests/ClaudeCode/ClaudeArgsProcessPolicyTests.cs b/tests/Volcanic.AgenticCore.Extensions.Tests/ClaudeCode/ClaudeArgsProcessPolicyTests.cs index 690ef81..828f920 100644 --- a/tests/Volcanic.AgenticCore.Extensions.Tests/ClaudeCode/ClaudeArgsProcessPolicyTests.cs +++ b/tests/Volcanic.AgenticCore.Extensions.Tests/ClaudeCode/ClaudeArgsProcessPolicyTests.cs @@ -1,3 +1,5 @@ +using System.Reflection.Metadata; +using System.Reflection.PortableExecutable; using Microsoft.Extensions.DependencyInjection; using Volcanic.AgenticCore.Abstractions.Processes; using Volcanic.AgenticCore.Extensions.ClaudeCode; @@ -37,4 +39,26 @@ public class ClaudeArgsProcessPolicyTests Assert.False(after.ContainsKey("INJECTED_BY_HOOK")); Assert.Equal(0, hookRuns); } + + /// + /// The test above can't see a spawn path outside BuildEnvironment, so also check the provider's IL metadata: + /// nothing in the assembly references the policy, the per-session preparation or the spawn event. + /// + [Fact] + public void Provider_assembly_never_references_the_process_spawn_policy() + { + var path = typeof(ClaudeArgs).Assembly.Location; + using var stream = File.OpenRead(path); + using var pe = new PEReader(stream); + var metadata = pe.GetMetadataReader(); + + var typeRefs = metadata.TypeReferences.Select(h => metadata.GetString(metadata.GetTypeReference(h).Name)).ToHashSet(); + var memberRefs = metadata.MemberReferences.Select(h => metadata.GetString(metadata.GetMemberReference(h).Name)).ToHashSet(); + + Assert.DoesNotContain(nameof(IProcessSpawnPolicy), typeRefs); + Assert.DoesNotContain(nameof(ProcessSpawnEvent), typeRefs); + Assert.DoesNotContain("PrepareProcessAsync", memberRefs); + Assert.DoesNotContain("get_Processes", memberRefs); + Assert.DoesNotContain(nameof(IProcessSpawnPolicy.Prepare), memberRefs); + } } diff --git a/tests/Volcanic.AgenticCore.Extensions.Tests/NoHostHardening.cs b/tests/Volcanic.AgenticCore.Extensions.Tests/Support/NoHostHardening.cs similarity index 89% rename from tests/Volcanic.AgenticCore.Extensions.Tests/NoHostHardening.cs rename to tests/Volcanic.AgenticCore.Extensions.Tests/Support/NoHostHardening.cs index 26311b4..c90385e 100644 --- a/tests/Volcanic.AgenticCore.Extensions.Tests/NoHostHardening.cs +++ b/tests/Volcanic.AgenticCore.Extensions.Tests/Support/NoHostHardening.cs @@ -2,7 +2,7 @@ using System.Runtime.CompilerServices; using Microsoft.Extensions.Logging.Abstractions; using Volcanic.AgenticCore.Features.Processes; -namespace Volcanic.AgenticCore.Extensions.Tests; +namespace Volcanic.AgenticCore.Extensions.Tests.Support; /// Consumes the process-wide hardening call with the flag off, so the test runner stays debuggable. internal static class NoHostHardening diff --git a/tests/Volcanic.AgenticCore.Tests/Features/SessionTimeZoneTests.cs b/tests/Volcanic.AgenticCore.Tests/Features/SessionTimeZoneTests.cs index f8dc4b4..075195c 100644 --- a/tests/Volcanic.AgenticCore.Tests/Features/SessionTimeZoneTests.cs +++ b/tests/Volcanic.AgenticCore.Tests/Features/SessionTimeZoneTests.cs @@ -1,5 +1,6 @@ using Volcanic.AgenticCore.Abstractions.Extensions; using Volcanic.AgenticCore.Abstractions.Tools; +using Volcanic.AgenticCore.Features.Extensions; using Volcanic.AgenticCore.Hosting; using Volcanic.AgenticCore.Testing; using Volcanic.AgenticCore.Tests.Support; @@ -52,6 +53,23 @@ public class SessionTimeZoneTests Assert.Equal("Asia/Tokyo", TimeZoneIds.ToIana(seen!)); } + [Fact] + public async Task Session_reports_a_TZ_id_only_for_a_configured_zone() + { + await using var host = new TestHost(); + await using (var plain = await host.CreateSessionAsync()) + Assert.Null(((IExtensionHost)plain).TimeZoneId); + await using var configured = await host.CreateSessionAsync(b => b.TimeZone = Tokyo); + Assert.Equal("Asia/Tokyo", ((IExtensionHost)configured).TimeZoneId); + } + + [Fact] + public void ToIana_returns_null_for_an_unnamed_or_unknown_zone() + { + Assert.Null(TimeZoneIds.ToIana(TimeZoneInfo.CreateCustomTimeZone("Local", TimeSpan.FromHours(1), "Local", "Local"))); + Assert.Null(TimeZoneIds.ToIana(TimeZoneInfo.CreateCustomTimeZone("Not/AZone", TimeSpan.FromHours(1), "x", "x"))); + } + [Fact] public void ToIana_keeps_iana_ids_and_converts_windows_ids() { diff --git a/tests/Volcanic.AgenticCore.Tests/Processes/HostProcessHardeningTests.cs b/tests/Volcanic.AgenticCore.Tests/Processes/HostProcessHardeningTests.cs index 78baa93..65fa2b7 100644 --- a/tests/Volcanic.AgenticCore.Tests/Processes/HostProcessHardeningTests.cs +++ b/tests/Volcanic.AgenticCore.Tests/Processes/HostProcessHardeningTests.cs @@ -41,6 +41,15 @@ public class HostProcessHardeningTests Assert.True(logger.Has(LogLevel.Warning, "22")); } + [Fact] + public void An_unresolvable_libc_logs_a_warning_and_does_not_throw() + { + var logger = new RecordingLogger(); + new HostProcessHardening.Hardener(_ => throw new DllNotFoundException("libc"), isLinux: true, Env()).Apply(On, logger); + Assert.True(logger.Has(LogLevel.Warning, "Host process hardening failed")); + Assert.False(logger.Has(LogLevel.Information, "Host process hardened")); + } + [Fact] public void Non_linux_does_not_call_prctl() { diff --git a/tests/Volcanic.AgenticCore.Tests/Processes/ProcessSessionBindingTests.cs b/tests/Volcanic.AgenticCore.Tests/Processes/ProcessSessionBindingTests.cs index 2d4cbac..60941a5 100644 --- a/tests/Volcanic.AgenticCore.Tests/Processes/ProcessSessionBindingTests.cs +++ b/tests/Volcanic.AgenticCore.Tests/Processes/ProcessSessionBindingTests.cs @@ -35,7 +35,7 @@ public class ProcessSessionBindingTests { var policy = new RecordingPolicy(TestProcesses.Policy(new Dictionary())); var runner = Runner(policy); - runner.Bind(new StubExtensionHost { Scope = "telegram-42", TimeZone = TimeZoneInfo.FindSystemTimeZoneById("Asia/Tokyo") }); + runner.Bind(new StubExtensionHost { Scope = "telegram-42", TimeZoneId = "Asia/Tokyo" }); var context = await runner.CreateContext().PrepareProcessAsync(Request(), CancellationToken.None); @@ -51,13 +51,40 @@ public class ProcessSessionBindingTests { var policy = new RecordingPolicy(TestProcesses.Policy(new Dictionary())); var runner = Runner(policy); - runner.Bind(new StubExtensionHost { TimeZone = TimeZoneInfo.FindSystemTimeZoneById("Asia/Tokyo") }); + runner.Bind(new StubExtensionHost { TimeZoneId = "Asia/Tokyo" }); await runner.CreateContext().PrepareProcessAsync(Request("Europe/Berlin"), CancellationToken.None); Assert.Equal("Europe/Berlin", Assert.Single(policy.Requests).TimeZoneId); } + [Fact] + public async Task PrepareProcessAsync_leaves_the_zone_unset_when_the_session_has_none_so_TZ_is_inherited() + { + var policy = new RecordingPolicy(TestProcesses.Policy(new Dictionary())); + var runner = Runner(policy); + runner.Bind(new StubExtensionHost { TimeZoneId = null }); + + await runner.CreateContext().PrepareProcessAsync(Request(), CancellationToken.None); + + Assert.Null(Assert.Single(policy.Requests).TimeZoneId); + } + + [Fact] + public async Task A_spawn_handler_that_drops_the_context_fails_the_spawn() + { + var runner = Runner(TestProcesses.Policy(new Dictionary())); + await runner.ActivateAsync(new LambdaExtension("bad", api => + { + api.On((e, _, _) => { e.Context = e.Context with { Environment = null! }; return ValueTask.CompletedTask; }); + return ValueTask.CompletedTask; + }), "inline", CancellationToken.None); + runner.Bind(new StubExtensionHost()); + + var ex = await Assert.ThrowsAsync(() => runner.CreateContext().PrepareProcessAsync(Request(), CancellationToken.None).AsTask()); + Assert.Contains("process_spawn handler", ex.Message); + } + [Fact] public async Task Spawn_handlers_run_in_load_order_after_the_host_hooks() { diff --git a/tests/Volcanic.AgenticCore.Tests/Processes/ProcessSpawnPolicyTests.cs b/tests/Volcanic.AgenticCore.Tests/Processes/ProcessSpawnPolicyTests.cs index f2ab465..4214908 100644 --- a/tests/Volcanic.AgenticCore.Tests/Processes/ProcessSpawnPolicyTests.cs +++ b/tests/Volcanic.AgenticCore.Tests/Processes/ProcessSpawnPolicyTests.cs @@ -132,6 +132,22 @@ public class ProcessSpawnPolicyTests Assert.Equal("hook broke", ex.Message); } + [Theory] + [InlineData("null")] + [InlineData("environment")] + [InlineData("cwd")] + public void A_hook_that_returns_an_incomplete_context_throws_out_of_Prepare(string broken) + { + var policy = TestProcesses.Policy(new Dictionary(), o => o.Hooks.Add(c => broken switch + { + "null" => null!, + "environment" => c with { Environment = null! }, + _ => c with { Cwd = null! }, + })); + var ex = Assert.Throws(() => policy.Prepare(Request())); + Assert.Contains("process spawn hook", ex.Message); + } + [Fact] public void Removed_names_are_logged_and_values_never_are() { diff --git a/tests/Volcanic.AgenticCore.Tests/Processes/SecretRedactorTests.cs b/tests/Volcanic.AgenticCore.Tests/Processes/SecretRedactorTests.cs index 059c783..9a22fe2 100644 --- a/tests/Volcanic.AgenticCore.Tests/Processes/SecretRedactorTests.cs +++ b/tests/Volcanic.AgenticCore.Tests/Processes/SecretRedactorTests.cs @@ -1,6 +1,8 @@ using Volcanic.AgenticCore.Abstractions.Json; using Volcanic.AgenticCore.Abstractions.Messages; +using Volcanic.AgenticCore.Abstractions.Processes; using Volcanic.AgenticCore.Abstractions.Tools; +using Volcanic.AgenticCore.Features.Agent; using Volcanic.AgenticCore.Features.Processes; namespace Volcanic.AgenticCore.Tests.Processes; @@ -55,4 +57,17 @@ public class SecretRedactorTests Assert.Contains("[REDACTED]", json); Assert.Equal(3, redacted.Details.Value.GetProperty("count").GetInt32()); } + + private sealed class PassThroughRedactor : ISecretRedactor + { + public string Redact(string text) => text; + } + + [Fact] + public void A_tool_result_with_nothing_to_redact_is_returned_as_is_whatever_the_redactor() + { + var result = new ToolResult([new TextContent("plain")], VolcanicJson.ToElement(new { note = "plain" })); + Assert.Same(result, new PassThroughRedactor().Redact(result)); + Assert.Same(result, new SecretRedactor(["zzzzzzzzzz"]).Redact(result)); + } } diff --git a/tests/Volcanic.AgenticCore.Tests/Support/StubExtensionHost.cs b/tests/Volcanic.AgenticCore.Tests/Support/StubExtensionHost.cs index 58300ec..7341afa 100644 --- a/tests/Volcanic.AgenticCore.Tests/Support/StubExtensionHost.cs +++ b/tests/Volcanic.AgenticCore.Tests/Support/StubExtensionHost.cs @@ -21,6 +21,7 @@ public sealed class StubExtensionHost : IExtensionHost public Model? Model { get; set; } public ThinkingLevel ThinkingLevel { get; set; } = ThinkingLevel.Off; public TimeZoneInfo TimeZone { get; init; } = TimeZoneInfo.Utc; + public string? TimeZoneId { get; init; } public CancellationToken Signal => CancellationToken.None; public bool IsIdle() => true; public void Abort() => Calls.Add("abort");