From 4057677b4dbed81222c629814a2bb8efa8cbcff1 Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Tue, 6 Oct 2026 17:28:33 +0800 Subject: [PATCH] Keep repository memory that links outside the workspace out of runs The settings pin adds the workspace and each repository in it back with --add-dir so their memory loads, and the pinned CLI opens an added directory's CLAUDE.md and .claude/CLAUDE.md by path and follows a symlink at either, or at .claude itself, wherever it leads: a repository that commits CLAUDE.md as a link to a file outside its workspace hands that file to the model as the repository's instructions. On Linux one such target is /proc/self/environ, which holds the CLI's environment and with it the run's broker token. Before a directory is added, everything its memory can reach is now resolved one component at a time, the way the kernel resolves it: the memory files, the .claude directory, every markdown file and folder under .claude/rules, and every file they @-import, five hops deep, with any '@' run taken for an import. 2.1.263 follows neither a rules entry linked out nor an import that resolves outside its cwd; the guard treats both as escapes anyway, for a later CLI that does. If any of it resolves outside the workspace the directory is left out whole. When none is left the run carries no --add-dir and no memory switch, and keeps the settings pin. A link that stays inside the workspace (CLAUDE.md to AGENTS.md, or a primary-repository cwd's rule into a sibling repository of the same workspace) or one that dangles still loads. The workspace is resolved by the same walker as what its memory reaches, so a link whose target climbs out of another link cannot make the two disagree, or throw before the launch. Files are opened no-follow, non-blocking and only if regular, so a FIFO cannot hang the build. The bounds only cap what one build spends: 16384 paths resolved and 4 MiB read per directory. What cannot be checked within them, or at all, leaves the directory out instead of failing the launch. A repository with 70 scoped rules, rules beside images, a 200 KiB CLAUDE.md or import, or a thousand email addresses keeps its memory. The harness reports what it left out on SandboxSpec.LaunchNotices, which never reaches the serialized spec. The executor records them as one Warning event when the run launches, and again for a revise round only when they differ from what the run last said, since a round's agent can re-point the memory either way. CodexHarness's physical-directory resolver moves unchanged to a shared PhysicalPath helper. The Claude harness unit tests stop naming a literal /tmp/ws, which the harness now reads, and lay their trees out under a symlinked root, so a lexical workspace comparison fails on every host and not only on macOS. Against Claude 2.1.263 the E2E's positive control, the same workspace with the left-out repositories put back into --add-dir, hands the outside file behind a linked CLAUDE.md, a linked .claude/CLAUDE.md and a linked .claude directory to the model; the guarded run hands none. Disabling the guard fails that E2E, the integration test and every escape case. --- .github/workflows/sandbox-isolation.yml | 10 +- .../Services/Agents/AgentRunExecutor.cs | 53 +++ .../Harnesses/Claude/ClaudeCodeHarness.cs | 45 +- .../Claude/ClaudeWorkspaceMemory.Closure.cs | 173 ++++++++ .../Harnesses/Claude/ClaudeWorkspaceMemory.cs | 141 ++++++ .../Agents/Harnesses/Codex/CodexHarness.cs | 23 +- .../Services/Agents/Harnesses/PhysicalPath.cs | 112 +++++ .../CodeSpace.Messages/Agents/SandboxSpec.cs | 10 + .../RealHarnessWorkspaceMemoryTests.cs | 299 +++++++++++++ .../RepositoryConfigE2ETests.Memory.cs | 141 ++++++ .../RepositoryConfigE2ETests.cs | 24 +- .../AgentRunExecutorLaunchNoticeTests.cs | 51 +++ .../Workflows/ClaudeCodeHarnessTests.cs | 107 ++++- .../Workflows/ClaudeWorkspaceMemoryTests.cs | 414 ++++++++++++++++++ .../Workflows/PhysicalPathTests.cs | 119 +++++ .../CodeSpace.UnitTests/Workflows/TempTree.cs | 48 ++ 16 files changed, 1693 insertions(+), 77 deletions(-) create mode 100644 backend/src/CodeSpace.Core/Services/Agents/Harnesses/Claude/ClaudeWorkspaceMemory.Closure.cs create mode 100644 backend/src/CodeSpace.Core/Services/Agents/Harnesses/Claude/ClaudeWorkspaceMemory.cs create mode 100644 backend/src/CodeSpace.Core/Services/Agents/Harnesses/PhysicalPath.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Workflows/RealHarnessWorkspaceMemoryTests.cs create mode 100644 backend/tests/CodeSpace.SandboxTests/RepositoryConfigE2ETests.Memory.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Workflows/AgentRunExecutorLaunchNoticeTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Workflows/ClaudeWorkspaceMemoryTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Workflows/PhysicalPathTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Workflows/TempTree.cs diff --git a/.github/workflows/sandbox-isolation.yml b/.github/workflows/sandbox-isolation.yml index 0fb7cbf5e..ccc6edd06 100644 --- a/.github/workflows/sandbox-isolation.yml +++ b/.github/workflows/sandbox-isolation.yml @@ -234,8 +234,8 @@ jobs: executed=$(grep -oE 'executed="[0-9]+"' "$trx" | head -1 | grep -oE '[0-9]+') passed=$(grep -oE 'passed="[0-9]+"' "$trx" | head -1 | grep -oE '[0-9]+') echo "executed=${executed:-0} passed=${passed:-0}" - if [ "${executed:-0}" -lt 98 ]; then - echo "::error::Expected >=98 sandbox isolation tests to run (bwrap/prlimit confinement + cap-drop + cgroup-namespace re-root + egress-allowlist filter + cgroup resource cap + durable-launch cgroup wiring + argv/envp per-string kernel ceiling + a prompt past it riding stdin + a read-only workspace mount + a network-off run reaching its broker through the relay and nothing else + an allowlist run relayed to its broker + a read-only reviewer reading its diff with the real CLIs + a bwrap probe that runs the launch argv + the MCP helper bound file by file behind a read-only socket dir + a CLI reaching its broker socket through the relay + a severed child reaching its broker over the lease socket across a worker restart + a pre-relay namespaced run re-bound at its gateway and torn down with its seal + an allowlist run's veth guarded both ways, a flow the worker opened before the run included + forwarding a root worker may not write named before an allowlist is planned + an allowlist run's port 53 open only at the resolvers of the resolv.conf its namespace reads, and still to a resolver address the worker's own NAT rewrites before its forward and its input hooks + a target repository's own CLI config kept out of the run with the real CLIs, every repository's memory read in a multi-repo workspace included + a multi-repo Codex run starting at a workspace root that is no git repository and loading no config from it + a repo-less Codex run starting in a scratch directory that is no git repository + a goal handed to the real Claude CLI as text it cannot act on, fresh and resumed, against the text channel as its control + a repository clean filter the capture runs with its egress severed + a real Codex agent that tampers its clone's .git while the platform publishes the branch from a clean repo, its Claude counterpart running in the non-root lane), but only ${executed:-0} did — the Category=Sandbox filter matched too few (trait regression?). If a case was deliberately removed, lower this number in the same PR." + if [ "${executed:-0}" -lt 99 ]; then + echo "::error::Expected >=99 sandbox isolation tests to run (bwrap/prlimit confinement + cap-drop + cgroup-namespace re-root + egress-allowlist filter + cgroup resource cap + durable-launch cgroup wiring + argv/envp per-string kernel ceiling + a prompt past it riding stdin + a read-only workspace mount + a network-off run reaching its broker through the relay and nothing else + an allowlist run relayed to its broker + a read-only reviewer reading its diff with the real CLIs + a bwrap probe that runs the launch argv + the MCP helper bound file by file behind a read-only socket dir + a CLI reaching its broker socket through the relay + a severed child reaching its broker over the lease socket across a worker restart + a pre-relay namespaced run re-bound at its gateway and torn down with its seal + an allowlist run's veth guarded both ways, a flow the worker opened before the run included + forwarding a root worker may not write named before an allowlist is planned + an allowlist run's port 53 open only at the resolvers of the resolv.conf its namespace reads, and still to a resolver address the worker's own NAT rewrites before its forward and its input hooks + a target repository's own CLI config kept out of the run with the real CLIs, every repository's memory read in a multi-repo workspace included + a repository whose memory links outside the workspace left out of a real Claude run, against the CLI following the link once that repository is added + a multi-repo Codex run starting at a workspace root that is no git repository and loading no config from it + a repo-less Codex run starting in a scratch directory that is no git repository + a goal handed to the real Claude CLI as text it cannot act on, fresh and resumed, against the text channel as its control + a repository clean filter the capture runs with its egress severed + a real Codex agent that tampers its clone's .git while the platform publishes the branch from a clean repo, its Claude counterpart running in the non-root lane), but only ${executed:-0} did — the Category=Sandbox filter matched too few (trait regression?). If a case was deliberately removed, lower this number in the same PR." exit 1 fi @@ -268,12 +268,12 @@ jobs: print(f'All {len(arms)} reviewer E2E arms ran and passed.') # The repository-config E2E is armed by the same CLI pins and returns early the same way; require each arm's marker. - for arm in ('claude-code single-repo Confined', 'claude-code multi-repo Confined', 'codex-cli single-repo', 'codex-cli multi-repo', 'codex-cli scratch'): + for arm in ('claude-code single-repo Confined', 'claude-code multi-repo Confined', 'memory-link-outside claude-code multi-repo Confined', 'codex-cli single-repo', 'codex-cli multi-repo', 'codex-cli scratch'): assert f'[repo-config-e2e] ran {arm}' in text, f'repository-config E2E arm "{arm}" did not run — check CODESPACE_REQUIRE_REVIEW_CLIS and the CLI install step' - for method in ('A_claude_run_ignores_the_settings_its_repository_commits_and_still_reads_its_memory', 'A_multi_repo_claude_run_reads_every_repositorys_memory_and_none_of_its_settings', 'A_codex_run_ignores_the_config_and_hooks_its_repository_commits_and_still_reads_its_agents_md', 'A_multi_repo_codex_run_starts_at_a_workspace_root_that_is_no_repository_and_loads_no_config_from_it', 'A_repo_less_codex_run_starts_in_a_scratch_directory_that_is_no_repository'): + for method in ('A_claude_run_ignores_the_settings_its_repository_commits_and_still_reads_its_memory', 'A_multi_repo_claude_run_reads_every_repositorys_memory_and_none_of_its_settings', 'A_claude_run_leaves_out_repository_memory_that_links_outside_the_workspace', 'A_codex_run_ignores_the_config_and_hooks_its_repository_commits_and_still_reads_its_agents_md', 'A_multi_repo_codex_run_starts_at_a_workspace_root_that_is_no_repository_and_loads_no_config_from_it', 'A_repo_less_codex_run_starts_in_a_scratch_directory_that_is_no_repository'): cases = [r for r in results if 'RepositoryConfigE2ETests.' + method in r.get('testName', '')] assert len(cases) == 1 and cases[0].get('outcome') == 'Passed', f'{method}: must pass' - print('All 5 repository-config E2E arms ran and passed.') + print('All 6 repository-config E2E arms ran and passed.') # The goal-channel E2E is armed by the same CLI pins and returns early the same way; require each arm's marker, # confined, so the positive control it checks against is this lane's posture and not an unconfined one. diff --git a/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs b/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs index c8940e74f..0f58bbb43 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/AgentRunExecutor.cs @@ -559,6 +559,10 @@ public async Task ExecuteAsync(Guid agentRunId, CancellationToken cancellationTo if (ranCold) await RecordRunColdAsync(owner, task with { Model = dispatchedModel }, LaunchRanColdNote, cancellationToken).ConfigureAwait(false); + // The same rule for what the harness left out of the launch. Said again only when a revise round's build + // leaves out something else: the agent changes the workspace those notices describe. + var launchNotices = await AppendLaunchNoticesAsync(owner, spec, said: null, cancellationToken).ConfigureAwait(false); + var result = await RunHarnessAsync(runContext, cancellationToken).ConfigureAwait(false); result = AgentRunBudget.Apply(effectiveTask, result, modelPrices); @@ -678,6 +682,8 @@ public async Task ExecuteAsync(Guid agentRunId, CancellationToken cancellationTo if (roundRanCold) await RecordRunColdAsync(owner, null, ReviseRanColdNote, cancellationToken).ConfigureAwait(false); + launchNotices = await AppendLaunchNoticesAsync(owner, reviseSpec, launchNotices, cancellationToken).ConfigureAwait(false); + var roundResult = await RunHarnessAsync(runContext with { Spec = reviseSpec, Task = reviseTask, SpoolKey = ReviseSpoolKey(agentRunId, round) }, cancellationToken).ConfigureAwait(false); result = AgentRunBudget.Apply(reviseTask with { BudgetSpentUsd = result.CumulativeCostUsd }, roundResult, modelPrices) with { TokenUsage = SumTokenUsage(priorUsage, roundResult.TokenUsage), ReviseRounds = round }; @@ -2702,6 +2708,53 @@ private async Task AppendMitigationEventAsync(AgentRunOwnerToken owner, Cancella } } + /// The most launch notices one timeline event repeats; the rest are counted. Pinned by a unit test. + internal const int MaxLaunchNotices = 10; + + /// The launch notices as one event's text — the first in order, then how many more there were; null when there are none. + internal static string? DescribeLaunchNotices(IReadOnlyList notices) + { + if (notices.Count == 0) return null; + + var shown = string.Join(" ", notices.Take(MaxLaunchNotices)); + + return notices.Count > MaxLaunchNotices ? $"{shown} ({notices.Count - MaxLaunchNotices} more)" : shown; + } + + /// The timeline's account of a revise round that leaves out none of what an earlier launch of the run left out. + internal const string LaunchNoticesClearedNote = "Left no memory out of this round: what an earlier round of this run left out loads again."; + + /// + /// What a launch's notices add to a timeline that last said (, + /// null for nothing): null when they say the same, so an unchanged workspace is announced once per run; their text + /// when they differ; and when a launch leaves nothing out that the last one did. + /// + internal static string? DescribeLaunchNoticeChange(IReadOnlyList notices, string? said) + { + var text = DescribeLaunchNotices(notices); + + if (text == said) return null; + + return text ?? LaunchNoticesClearedNote; + } + + /// Say on the timeline what the harness left out of this launch () — a repository's memory that links outside the workspace, for one — so a run missing its instructions says why, and say it again only when a revise round's launch leaves out something else (). Returns what the timeline now says. One bounded event per change; best-effort like the other launch notes. + private async Task AppendLaunchNoticesAsync(AgentRunOwnerToken owner, SandboxSpec spec, string? said, CancellationToken cancellationToken) + { + if (DescribeLaunchNoticeChange(spec.LaunchNotices, said) is not { } text) return said; + + try + { + await _runs.AppendEventAsync(owner, new AgentEvent { Kind = AgentEventKind.Warning, Text = text }, cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException and not AgentRunOwnershipLostException) + { + _logger.LogWarning(ex, "Agent run {RunId}: could not record the launch notices", owner.RunId); + } + + return DescribeLaunchNotices(spec.LaunchNotices); + } + /// Announce the escalation on the timeline — the operator sees the run reached for a stronger model and WHY, or that it wanted to and the team had nothing stronger. Best-effort like the other completion-tail events. private async Task AppendEscalationEventAsync(AgentRunOwnerToken owner, AgentModelEscalation escalation, CancellationToken cancellationToken) { diff --git a/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Claude/ClaudeCodeHarness.cs b/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Claude/ClaudeCodeHarness.cs index 633d26005..07f4c563e 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Claude/ClaudeCodeHarness.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Claude/ClaudeCodeHarness.cs @@ -202,6 +202,9 @@ public SandboxSpec BuildInvocation(AgentTask task) // model sees it. var args = new List { "--print", "--output-format", "stream-json", "--verbose", "--input-format", "stream-json" }; + // The directories added back for their memory, less any whose memory reaches outside the workspace (see AppendSettingsPin). + var memory = ClaudeWorkspaceMemory.For(task); + // P3.2: a CONTINUE re-stage threads the prior session id as `--resume ` to pick up the conversation. // Placed right after the seed — before the variadic --allowed-tools / --permission-mode — so the variadic can // never swallow it. The continuation prompt rides stdin like any other, in the same message. Null (a fresh run) → omitted. @@ -223,7 +226,7 @@ public SandboxSpec BuildInvocation(AgentTask task) // and the requirement is that we NEVER pass --bare / --safe-mode (guarded by a unit test). What every run does // get is the settings pin — one mechanism, no per-run condition: the target repository's own .claude settings // are untrusted input whether or not this run writes settings of its own (see AppendSettingsPin). - AppendSettingsPin(args, task); + AppendSettingsPin(args, memory.Directories); AppendSealedEgressSettings(args, task); @@ -255,7 +258,7 @@ public SandboxSpec BuildInvocation(AgentTask task) Args = args, StandardInput = PromptMessage(task.Goal), WorkingDirectory = task.WorkspaceDirectory, - Environment = BuildEnvironment(task), + Environment = BuildEnvironment(task, memory.Directories), TimeoutSeconds = task.TimeoutSeconds, // Isolate Claude Code's config dir per run so it ignores the operator's personal ~/.claude. ConfigHomeEnvVars = new[] { ConfigDirEnvVar }, @@ -268,6 +271,8 @@ public SandboxSpec BuildInvocation(AgentTask task) ConfigHomeFiles = BuildConfigHomeFiles(task), // The agent reaches the network only when its permissions allow it (the sandbox severs egress otherwise). AllowNetwork = task.Permissions.Network == AgentNetworkAccess.On, + // A repository's memory left out because it links outside the workspace — the run's timeline says so. + LaunchNotices = memory.Notices, }; } @@ -583,19 +588,19 @@ private static string PermissionMode(AgentPermissions permissions) => /// /// The child env: the task's env, plus harness-injected entries — the /// for an Allowlist (deny-by-default) egress run (so the CLI doesn't stall reaching telemetry hosts the allowlist - /// doesn't pin, B3.3c), the that makes the workspace's - /// --add-dir load its memory (), and the gateway model-tier pins + /// doesn't pin, B3.3c), the that makes each --add-dir + /// directory load its memory () when there is one, and the gateway model-tier pins /// (). An explicit entry WINS (operator intent — /// layered last), matching the runner's NonInteractiveEnv "operator value wins" convention. When nothing is injected /// the task env is returned unchanged → byte-identical. /// - private static IReadOnlyDictionary BuildEnvironment(AgentTask task) + private static IReadOnlyDictionary BuildEnvironment(AgentTask task, IReadOnlyList memoryDirectories) { var injected = new Dictionary(StringComparer.Ordinal); if (task.Permissions.Egress == AgentEgressPolicy.Allowlist) injected[DisableNonEssentialTrafficEnvVar] = "1"; - if (HasWorkspace(task)) injected[AdditionalDirectoriesMemoryEnvVar] = "1"; + if (memoryDirectories.Count > 0) injected[AdditionalDirectoriesMemoryEnvVar] = "1"; AddGatewayModelTiers(injected, task); @@ -650,34 +655,24 @@ private static void AddGatewayModelTiers(Dictionary env, AgentTa /// terminates the list. /// /// A multi-repo workspace runs at its root, which holds no CLAUDE.md, so every repository directory - /// inside the workspace is added too (), and each repository's memory loads. + /// inside the workspace is added too, and each repository's memory loads. + /// + /// The CLI opens an added directory's CLAUDE.md and .claude/CLAUDE.md by path and follows a + /// symlink at either, or at .claude itself, wherever it leads, so a directory whose memory resolves outside the + /// workspace is not added at all (), and when none is left there is no + /// --add-dir. The settings pin stays either way. /// - private static void AppendSettingsPin(List args, AgentTask task) + private static void AppendSettingsPin(List args, IReadOnlyList memoryDirectories) { args.Add("--setting-sources"); args.Add("user"); - if (!HasWorkspace(task)) return; + if (memoryDirectories.Count == 0) return; args.Add("--add-dir"); - args.AddRange(MemoryDirectories(task)); + args.AddRange(memoryDirectories); } - /// - /// The workspace, then every repository directory inside it. A repository outside it — a sibling of a cwd at the - /// primary repository — is left out: the unpinned CLI never loaded its memory either, and an added directory also - /// widens what the CLI's tools may touch. - /// - private static IEnumerable MemoryDirectories(AgentTask task) - { - var workspace = task.WorkspaceDirectory!; - var inside = Path.TrimEndingDirectorySeparator(workspace) + Path.DirectorySeparatorChar; - - return new[] { workspace }.Concat((task.WorkspaceRepositoryDirectories ?? []).Where(directory => directory.StartsWith(inside, StringComparison.Ordinal))).Distinct(StringComparer.Ordinal); - } - - private static bool HasWorkspace(AgentTask task) => !string.IsNullOrWhiteSpace(task.WorkspaceDirectory); - /// /// On a deny-by-default (Allowlist) egress run, deliver --settings {"":true} /// so a WebFetch tool call doesn't preflight the hostname against api.anthropic.com — a host the egress allowlist diff --git a/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Claude/ClaudeWorkspaceMemory.Closure.cs b/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Claude/ClaudeWorkspaceMemory.Closure.cs new file mode 100644 index 000000000..dad98bf65 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Claude/ClaudeWorkspaceMemory.Closure.cs @@ -0,0 +1,173 @@ +using System.Text; +using System.Text.RegularExpressions; +using CodeSpace.Core.Services.Agents.Workspace; + +namespace CodeSpace.Core.Services.Agents.Harnesses.Claude; + +/// One directory's memory walked to everything it reaches (): the files it reads, within the bounds, and the paths their imports name. +internal static partial class ClaudeWorkspaceMemory +{ + /// A superset of the CLI's import grammar ((?:^|\s)@((?:[^\s\\]|\\ )+)): no whitespace is required before the @. + [GeneratedRegex(@"@((?:[^\s\\]|\\ )+)")] + private static partial Regex ImportToken(); + + /// One path the closure still has to resolve: is the memory entry, relative to the directory, it was reached from. + private sealed record Entry(string Path, string Origin, int Hops, bool IsRules); + + /// + /// Everything one directory's memory can reach, walked until the first thing that leaves the workspace or cannot be + /// checked. Fewest hops first, in the order each entry was found, so a file is first reached at its least depth — a + /// rule a CLAUDE.md also imports is read as the rule it is, with all of its own imports' hops left — and the + /// notice names the same escape on every build. + /// + private sealed class Closure(string workspace, string root) + { + private readonly PriorityQueue _pending = new(); + private readonly HashSet _seen = new(StringComparer.Ordinal); + private readonly HashSet _imports = new(StringComparer.Ordinal); + private int _found; + private int _scanned; + + /// Why the directory must be left out, or null when all of its memory stays inside the workspace. + public string? FirstEscape() + { + foreach (var seed in Seeds(root)) Enqueue(seed); + + while (_pending.TryDequeue(out var entry, out _)) + { + if (Examine(entry) is { } escape) return escape; + } + + return null; + } + + /// One more path to resolve; why the directory must be left out once that is more than the guard resolves. + private string? Enqueue(Entry entry) + { + _pending.Enqueue(entry, (entry.Hops, _found++)); + + return _found > MaxLookups ? $"its memory names more than {MaxLookups} paths to check" : null; + } + + /// The memory the CLI reads from an added directory, as hop 0, and the .claude directory it reads it from. + private static IEnumerable Seeds(string root) => + [ + new(Path.Combine(root, "CLAUDE.md"), "CLAUDE.md", 0, false), + new(Path.Combine(root, ".claude"), ".claude", 0, false), + new(Path.Combine(root, ".claude", "CLAUDE.md"), ".claude/CLAUDE.md", 0, false), + new(Path.Combine(root, ".claude", "rules"), ".claude/rules", 0, true), + ]; + + /// + /// Where one entry really is, and whether that leaves the workspace. Nothing there gives the CLI nothing to read, + /// and neither does a directory an import names, wherever it is (the CLI imports files only), or a file in a + /// rules folder that is not markdown (the CLI reads only .md rules). Every other entry must resolve inside + /// the workspace — a directory included, because a rules folder is read entry by entry and what a + /// .claude directory outside holds is not the workspace's to vouch for, whatever it holds while this looks. + /// + private string? Examine(Entry entry) + { + if (PhysicalPath.File(entry.Path) is not { } physical) return null; + + var isDirectory = Directory.Exists(physical); + + if (entry.Hops > 0 && isDirectory) return null; + + if (entry.IsRules && !isDirectory && !entry.Path.EndsWith(".md", StringComparison.Ordinal)) return null; + + if (!PhysicalPath.StaysInside(workspace, physical)) return Describe(entry, "resolves outside the workspace"); + + if (!_seen.Add(physical)) return null; + + return isDirectory ? Enumerate(entry, physical) : Scan(entry, physical); + } + + /// A rules directory's entries join the walk under the path the CLI reads them by; .claude itself is read only through its CLAUDE.md and rules, which are seeds of their own. + private string? Enumerate(Entry entry, string physical) + { + if (!entry.IsRules) return null; + + foreach (var name in ChildNames(physical)) + { + if (Enqueue(new Entry(Path.Combine(entry.Path, name), $"{entry.Origin}/{name}", entry.Hops, true)) is { } tooMany) return tooMany; + } + + return null; + } + + /// Every entry of a directory, hidden ones too, as the CLI's own listing returns them — one past the bound at most, which is enough to trip it. + private static IEnumerable ChildNames(string directory) => + new DirectoryInfo(directory).EnumerateFileSystemInfos("*", new EnumerationOptions { AttributesToSkip = 0, IgnoreInaccessible = true }).Select(child => child.Name).Take(MaxLookups + 1); + + /// A file the CLI may read: every import it names joins the walk, until the hop limit, past which the CLI reads nothing. + private string? Scan(Entry entry, string physical) + { + if (entry.Hops >= MaxImportHops) return null; + + var text = ReadBounded(physical, MaxScannedBytes - _scanned, out var read); + + _scanned += read; + + if (_scanned > MaxScannedBytes) return $"its memory spans more than {MaxScannedBytes} bytes to check"; + + foreach (var target in Imports(text ?? "", Path.GetDirectoryName(physical)!)) + { + if (_imports.Add(target) && Enqueue(new Entry(target, entry.Origin, entry.Hops + 1, false)) is { } tooMany) return tooMany; + } + + return null; + } + + private static string Describe(Entry entry, string what) => entry.Hops == 0 ? $"{Printable(entry.Origin)} {what}" : $"a file {Printable(entry.Origin)} imports {what}"; + } + + /// + /// Every path the text @-imports, as the CLI resolves it: a fragment after # dropped, an escaped space + /// unescaped, an absolute path as written, any other relative to the importing file's own physical directory. + /// A ~ import is skipped (see the class remarks), and so is one the platform cannot hold as a path. + /// + private static IEnumerable Imports(string text, string directory) + { + foreach (Match match in ImportToken().Matches(text)) + { + var token = match.Groups[1].Value.Split('#')[0].Replace("\\ ", " ", StringComparison.Ordinal); + + if (token.Length == 0 || token.StartsWith('~') || token.Contains('\0')) continue; + + yield return Path.GetFullPath(Path.IsPathRooted(token) ? token : Path.Combine(directory, token)); + } + } + + /// + /// The text of a regular file, read to its end but no further than bytes and one more; + /// null for anything the CLI would not read either — a FIFO, a device, a file it may not open. + /// is how many bytes were read, which passes the budget, and nothing is returned, when the file runs past it. + /// + private static string? ReadBounded(string physical, int budget, out int read) + { + read = 0; + + try + { + using var stream = new FileStream(LocalAcceptanceFileIdentity.Open(physical, directory: false), FileAccess.Read); + var buffer = new byte[Math.Min(budget + 1L, stream.Length + 1)]; + + read = ReadFully(stream, buffer); + + return read > budget ? null : Encoding.UTF8.GetString(buffer, 0, read); + } + catch (IOException) + { + return null; + } + } + + private static int ReadFully(Stream stream, byte[] buffer) + { + var total = 0; + + for (int read; total < buffer.Length && (read = stream.Read(buffer, total, buffer.Length - total)) > 0;) total += read; + + return total; + } +} diff --git a/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Claude/ClaudeWorkspaceMemory.cs b/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Claude/ClaudeWorkspaceMemory.cs new file mode 100644 index 000000000..317e6459b --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Claude/ClaudeWorkspaceMemory.cs @@ -0,0 +1,141 @@ +using System.Globalization; +using CodeSpace.Core.Services.Agents.Workspace; +using CodeSpace.Messages.Agents; + +namespace CodeSpace.Core.Services.Agents.Harnesses.Claude; + +/// +/// The directories a Claude run adds back with --add-dir so their memory loads (see +/// ClaudeCodeHarness.AppendSettingsPin), less every one whose memory would bring in bytes from outside the +/// workspace. +/// +/// The pinned 2.1.263 opens an added directory's CLAUDE.md and .claude/CLAUDE.md by path and +/// follows a symlink at either wherever it leads, and a .claude directory that is itself a link brings in what +/// the directory it leads to holds, its rules included. What it reaches is handed to the model as the repository's +/// instructions; RepositoryConfigE2ETests pins all three against the real binary. One such target is +/// /proc/self/environ, which on Linux holds the CLI's own environment and with it the run's broker token. The +/// same CLI does not follow a .claude/rules entry, file or folder, that links outside the added directory, and +/// refuses an import that resolves outside its cwd unless external includes are approved, which a fresh per-run config +/// home never is (both observed against 2.1.263). The guard counts those as escapes too, so a later CLI that follows +/// them reaches nothing: before a directory is added, everything its memory can reach is resolved the way the kernel +/// resolves it () — those memory files, the .claude directory itself, every +/// markdown file and folder under .claude/rules, and every file they @-import, to +/// hops. If any of it resolves outside the workspace, the directory is left out whole and the run's timeline says so +/// (). A link that stays inside the workspace — CLAUDE.md to AGENTS.md, or one +/// repository's rule to a sibling repository the same workspace holds — keeps loading, and so does one that dangles, +/// which gives the CLI nothing to read. +/// +/// Any @ followed by a run of non-space is taken for an import, inside code blocks too and with no space +/// before it, wherever its target resolves: a superset of the CLI's own grammar. One exception, documented rather than +/// closed: a ~ import names the run's own home, which under confinement is its config home and does not exist +/// yet when the invocation is built, so there is nothing to resolve; the CLI's refusal is its only guard. +/// +/// Every file is opened no-follow, non-blocking and only if regular (), +/// so a FIFO in a repository cannot hang the build; the CLI reads no FIFO either. The bounds below only cap what one +/// build may spend; what cannot be checked within them, or at all, leaves the directory out too, and the build never +/// throws for it. The check runs on every build, a revise round's included, and nothing writes the workspace while it +/// does: no agent process is running before a round starts. On a host that is neither Linux nor macOS the directories +/// are added unchecked. +/// +internal static partial class ClaudeWorkspaceMemory +{ + /// How many @-imports deep the guard follows: the CLI reads none at depth 5 (wgs=5 in 2.1.263). Pinned by a test. + internal const int MaxImportHops = 5; + + /// + /// The most paths one directory's memory may give the guard to resolve — every entry its rules folders list and every + /// distinct import — before it is left out unchecked. Each costs a walk of its path, a name that resolves to nothing + /// included, so this bounds the build, not what the CLI loads. Pinned by a test. + /// + internal const int MaxLookups = 16384; + + /// The most bytes the guard reads from one directory's memory, every file it scans together; more leaves the directory out. Pinned by a test. + internal const int MaxScannedBytes = 4 * 1024 * 1024; + + /// The longest file or directory name a notice repeats; a longer one is cut. Pinned by a test. + internal const int MaxNoticeNameLength = 120; + + /// The directories to add, in order, and one sentence for each directory left out. + internal sealed record Plan(IReadOnlyList Directories, IReadOnlyList Notices); + + public static Plan For(AgentTask task) + { + if (string.IsNullOrWhiteSpace(task.WorkspaceDirectory)) return new Plan([], []); + + var roots = RootDirectories(task).ToList(); + + if (!OperatingSystem.IsLinux() && !OperatingSystem.IsMacOS()) return new Plan(roots, []); + + var leftOut = LeftOut(ProvisionedRoot(task), roots); + var notices = leftOut.Select(item => Notice(task.WorkspaceDirectory, item.Root, item.Why)).ToList(); + + return new Plan(roots.Except(leftOut.Select(item => item.Root), StringComparer.Ordinal).ToList(), notices); + } + + /// + /// The workspace, then every repository directory inside it. A repository outside it — a sibling of a cwd at the + /// primary repository — is left out: the unpinned CLI never loaded its memory either, and an added directory also + /// widens what the CLI's tools may touch. + /// + private static IEnumerable RootDirectories(AgentTask task) + { + var workspace = task.WorkspaceDirectory!; + var inside = Path.TrimEndingDirectorySeparator(workspace) + Path.DirectorySeparatorChar; + + return new[] { workspace }.Concat((task.WorkspaceRepositoryDirectories ?? []).Where(directory => directory.StartsWith(inside, StringComparison.Ordinal))).Distinct(StringComparer.Ordinal); + } + + /// + /// The directory the run's workspace was provisioned as, which memory may link anywhere inside: the cwd, or — when the + /// cwd is one of several repositories that sit side by side (a primary-repository cwd) — the root that holds them, + /// its parent. Those siblings are the run's own clones, not bytes from outside it. Repository directories laid out + /// any other way widen nothing. + /// + private static string ProvisionedRoot(AgentTask task) + { + var cwd = Path.TrimEndingDirectorySeparator(task.WorkspaceDirectory!); + var parent = Path.GetDirectoryName(cwd); + var repositories = (task.WorkspaceRepositoryDirectories ?? []).Select(Path.TrimEndingDirectorySeparator).Distinct(StringComparer.Ordinal).ToList(); + + return parent is not null && repositories.Count > 1 && repositories.Contains(cwd, StringComparer.Ordinal) && repositories.All(directory => Path.GetDirectoryName(directory) == parent) ? parent : cwd; + } + + /// Every root whose memory reaches outside the workspace or cannot be checked, in order, with why. The workspace is resolved by the same walker as everything its memory reaches, so the two sides of the comparison cannot disagree on a link. + private static List<(string Root, string Why)> LeftOut(string workspace, IEnumerable roots) + { + var physical = PhysicalPath.File(workspace) ?? workspace; + + return roots.Select(root => (Root: root, Why: FirstEscape(physical, root))).Where(item => item.Why is not null).Select(item => (item.Root, item.Why!)).ToList(); + } + + /// Why one root must be left out, or null when all of its memory stays inside the workspace. A directory the guard cannot finish reading is left out rather than failing the launch. + private static string? FirstEscape(string workspace, string root) + { + try + { + return new Closure(workspace, root).FirstEscape(); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or PlatformNotSupportedException) + { + return "its memory could not be checked"; + } + } + + private static string Notice(string workspace, string root, string why) => $"Left the memory in {Place(workspace, root)} out of this run: {why}."; + + private static string Place(string workspace, string root) => Path.GetRelativePath(workspace, root) switch + { + "." => "the workspace", + var relative => $"'{Printable(relative)}'", + }; + + /// A repository-chosen name as a notice repeats it: control and format characters replaced, length cut. + private static string Printable(string name) + { + var clean = new string(name.Select(c => IsUnprintable(c) ? '?' : c).ToArray()); + + return clean.Length <= MaxNoticeNameLength ? clean : clean[..MaxNoticeNameLength] + "…"; + } + + private static bool IsUnprintable(char c) => char.IsControl(c) || char.GetUnicodeCategory(c) is UnicodeCategory.Format or UnicodeCategory.LineSeparator or UnicodeCategory.ParagraphSeparator; +} diff --git a/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Codex/CodexHarness.cs b/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Codex/CodexHarness.cs index 91a71f8cc..1ca124950 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Codex/CodexHarness.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Harnesses/Codex/CodexHarness.cs @@ -596,33 +596,12 @@ private static void AppendWorkspaceDistrust(List args, AgentTask task) { if (string.IsNullOrWhiteSpace(task.WorkspaceDirectory)) return; - var entries = new[] { task.WorkspaceDirectory, PhysicalDirectory(task.WorkspaceDirectory) }.Distinct(StringComparer.Ordinal).Select(path => $"{McpDeclarationWriter.TomlString(path)}={{trust_level=\"untrusted\"}}"); + var entries = new[] { task.WorkspaceDirectory, PhysicalPath.Directory(task.WorkspaceDirectory) }.Distinct(StringComparer.Ordinal).Select(path => $"{McpDeclarationWriter.TomlString(path)}={{trust_level=\"untrusted\"}}"); args.Add("-c"); args.Add($"projects={{{string.Join(',', entries)}}}"); } - /// - /// The directory a process resolves to as its cwd: every component's symlink followed, a - /// link whose own target runs through another link included. A path that does not exist resolves to no cwd, so it - /// is returned as given. - /// - private static string PhysicalDirectory(string path) - { - if (!Directory.Exists(path)) return path; - - var full = Path.GetFullPath(path); - var physical = Path.GetPathRoot(full)!; - - foreach (var segment in full[physical.Length..].Split(Path.DirectorySeparatorChar, StringSplitOptions.RemoveEmptyEntries)) - { - var next = Path.Combine(physical, segment); - physical = new DirectoryInfo(next).ResolveLinkTarget(returnFinalTarget: true) is { } target ? PhysicalDirectory(target.FullName) : next; - } - - return physical; - } - /// Codex hosts an MCP server from an [mcp_servers.<name>] table in its config home's config.toml. The harness owns the format — it renders the TOML content with the run-scoped socket + token baked in; the runner just writes the bytes. public McpHarnessDeclaration BuildMcpDeclaration(McpDeclarationContext context) => new() { diff --git a/backend/src/CodeSpace.Core/Services/Agents/Harnesses/PhysicalPath.cs b/backend/src/CodeSpace.Core/Services/Agents/Harnesses/PhysicalPath.cs new file mode 100644 index 000000000..a76c648bd --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Agents/Harnesses/PhysicalPath.cs @@ -0,0 +1,112 @@ +namespace CodeSpace.Core.Services.Agents.Harnesses; + +/// +/// Where a path really is once the kernel has followed every symlink on it. A harness needs this wherever a CLI keys +/// or loads something by the path it resolved rather than the one it was given. +/// +internal static class PhysicalPath +{ + /// The most links follows on one path — the kernel's own limit (Linux MAXSYMLINKS), past which an open fails with ELOOP. + internal const int MaxLinkHops = 40; + + /// + /// The directory a process resolves to as its cwd: every component's symlink followed, a + /// link whose own target runs through another link included. A path that does not exist resolves to no cwd, so it + /// is returned as given. + /// + public static string Directory(string path) + { + if (!System.IO.Directory.Exists(path)) return path; + + var full = Path.GetFullPath(path); + var physical = Path.GetPathRoot(full)!; + + foreach (var segment in full[physical.Length..].Split(Path.DirectorySeparatorChar, StringSplitOptions.RemoveEmptyEntries)) + { + var next = Path.Combine(physical, segment); + physical = new DirectoryInfo(next).ResolveLinkTarget(returnFinalTarget: true) is { } target ? Directory(target.FullName) : next; + } + + return physical; + } + + /// + /// The entry the kernel reaches when a process opens , or null when it reaches none: a + /// missing entry, a link that dangles, a link chain longer than or a component it may not + /// search. The path is first normalised the way a caller's own path library does (a .. in it is lexical), + /// then walked one component at a time as the kernel walks it. A .. inside a link's target is taken from + /// where that link really is, never textually: deep/../x with deep linked to /o/p/q is + /// /o/p/x, which a lexical reading would place beside deep. + /// + public static string? File(string path) + { + try + { + return Walk(Path.GetFullPath(path)); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or ArgumentException) + { + return null; + } + } + + /// Whether is or lies below it. Both must already be physical: a spelling through a symlink is compared as text. + public static bool StaysInside(string physicalRoot, string physicalPath) + { + var root = Path.TrimEndingDirectorySeparator(physicalRoot); + var below = Path.EndsInDirectorySeparator(root) ? root : root + Path.DirectorySeparatorChar; + + return physicalPath == root || physicalPath.StartsWith(below, StringComparison.Ordinal); + } + + private static string? Walk(string full) + { + var current = Path.GetPathRoot(full)!; + var pending = Components(full[current.Length..]); + var hops = 0; + + while (pending.Count > 0) + { + var name = Pop(pending); + + if (name == "..") + { + current = Path.GetDirectoryName(current) ?? current; + continue; + } + + var next = Path.Combine(current, name); + + if (new FileInfo(next).LinkTarget is { } target) + { + if (++hops > MaxLinkHops) return null; + if (Path.IsPathRooted(target)) current = Path.GetPathRoot(target)!; + Push(pending, target); + continue; + } + + if (!Exists(next, isLast: pending.Count == 0)) return null; + + current = next; + } + + return current; + } + + /// A component that names nothing ends the walk, and so does a file with components still to walk below it, which the kernel refuses with ENOTDIR. + private static bool Exists(string entry, bool isLast) => System.IO.Directory.Exists(entry) || (isLast && System.IO.File.Exists(entry)); + + /// The components still to walk, the next one last; a . and an empty component name nothing and are dropped. + private static List Components(string relative) => + relative.Split(Path.DirectorySeparatorChar, StringSplitOptions.RemoveEmptyEntries).Where(name => name != ".").Reverse().ToList(); + + private static string Pop(List pending) + { + var name = pending[^1]; + pending.RemoveAt(pending.Count - 1); + return name; + } + + /// A link's target goes in front of whatever was left below the link. + private static void Push(List pending, string target) => pending.AddRange(Components(Path.IsPathRooted(target) ? target[Path.GetPathRoot(target)!.Length..] : target)); +} diff --git a/backend/src/CodeSpace.Messages/Agents/SandboxSpec.cs b/backend/src/CodeSpace.Messages/Agents/SandboxSpec.cs index c22ef1bd5..622abd934 100644 --- a/backend/src/CodeSpace.Messages/Agents/SandboxSpec.cs +++ b/backend/src/CodeSpace.Messages/Agents/SandboxSpec.cs @@ -254,6 +254,16 @@ public sealed record SandboxSpec /// writes a per-run config home; a bare-process runner with no config home ignores them. /// public IReadOnlyList ConfigHomeFiles { get; init; } = Array.Empty(); + + /// + /// What the harness left out of this launch that the run's timeline should say, one sentence each — a repository's + /// memory that links outside the workspace, for one. AgentRunExecutor records them as one bounded Warning + /// event when the run launches, and again for a revise round only when they differ from what the run last said. + /// Not part of the invocation: never serialized, so the spec serializes and hashes as it did before the field + /// existed. Empty (the default) ⇒ nothing to say. + /// + [System.Text.Json.Serialization.JsonIgnore] + public IReadOnlyList LaunchNotices { get; init; } = Array.Empty(); } /// A contiguous run of argv elements and what replaces it — see . diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/RealHarnessWorkspaceMemoryTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/RealHarnessWorkspaceMemoryTests.cs new file mode 100644 index 000000000..2c2baa68c --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/RealHarnessWorkspaceMemoryTests.cs @@ -0,0 +1,299 @@ +using System.Text.Json; +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Agents; +using CodeSpace.Core.Services.Agents.Harnesses.Claude; +using CodeSpace.Core.Services.Agents.Sandbox.Runners; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.IntegrationTests.Workflows.Infrastructure; +using CodeSpace.Messages.Agents; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Workflows; + +/// +/// A repository whose CLAUDE.md is a committed symlink, run through the production pipeline: does the CLI get its +/// workspace added back for memory, and does the run's timeline say why not when it doesn't? +/// +/// 🟡 Medium-mock (Rule 12): the real DI-wired , the real +/// , LocalGitWorkspaceProvider cloning a file:// bare remote (the symlink +/// arrives the way git checks one out), the real , the real push, grader and revise loop, +/// and real Postgres. Only the CLI is a fake: a /bin/sh script armed through +/// that reads the argv it was really spawned with and fails the round if +/// --add-dir is there when it must not be, or missing when it must be. The real CLI following the link is pinned +/// by RepositoryConfigE2ETests. +/// +/// Every run takes a revise round — the first round drafts work its check refuses — because the executor builds a +/// fresh spec for each round: the notice must be said once while the workspace stays as it was, and said again when the +/// draft round's agent re-points CLAUDE.md, in either direction. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public sealed class RealHarnessWorkspaceMemoryTests +{ + /// The check the contract runs on the produced branch: PASS iff the revision has run. + private const string CheckScript = "#!/bin/sh\ngrep -q revised feature.txt\n"; + + private const string NoticePrefix = "Left "; + + private const string LeftOutNotice = "Left the memory in the workspace out of this run: CLAUDE.md resolves outside the workspace."; + + /// What CLAUDE.md is committed as, or what the draft round re-points it to: a file outside the workspace, or the AGENTS.md beside it. + private const string Outside = "outside"; + + private readonly PostgresFixture _fixture; + + public RealHarnessWorkspaceMemoryTests(PostgresFixture fixture) { _fixture = fixture; } + + [Theory] + [InlineData(Outside, null, new[] { LeftOutNotice })] // linked outside throughout: left out of both rounds, said once + [InlineData("AGENTS.md", null, new string[0])] // the control: linked inside throughout, nothing to say + [InlineData("AGENTS.md", Outside, new[] { LeftOutNotice })] // the draft re-points it outside: said when the revision leaves it out + [InlineData(Outside, "AGENTS.md", new[] { LeftOutNotice, AgentRunExecutor.LaunchNoticesClearedNote })] // the draft re-points it inside: said when the revision loads it again + public async Task A_repository_memory_that_links_outside_is_left_out_of_each_round_it_links_out_and_said_when_that_changes(string committed, string? draftRepoints, string[] said) + { + if (OperatingSystem.IsWindows()) return; // the fake CLI is a /bin/sh script, the link a POSIX symlink + + using var outside = new OutsideFile(); + using var remote = new BareRemote(); + await remote.SeedAsync(CheckScript, claudeMdTarget: Target(committed, outside)); + using var cli = new MemoryCheckingFakeCli(draft: Expected(committed), revision: Expected(draftRepoints ?? committed), draftRepoints: draftRepoints is null ? null : Target(draftRepoints, outside)); + + var (teamId, userId) = await SeedTeamAsync(); + var repoId = await SeedBoundRepositoryAsync(teamId, remote.Url); + var runId = await CreateRunAsync(teamId, userId, repoId, cli.Env()); + + await ExecuteRealAsync(runId); + + var (run, result) = await LoadAsync(runId); + var notices = (await LoadEventsAsync(runId)).Where(e => e.Text.StartsWith(NoticePrefix, StringComparison.Ordinal)).ToList(); + + run.Status.ShouldBe(AgentRunStatus.Succeeded, $"the fake CLI fails a round whose argv adds a workspace whose CLAUDE.md links outside, or leaves out one whose CLAUDE.md links inside; error: {run.Error}; launches: {string.Join(", ", cli.Launches())}"); + cli.Launches().ShouldBe(new[] { Expected(committed), Expected(draftRepoints ?? committed) }, "fixture check: both rounds spawned the CLI, each with the argv it was meant to have"); + result.ReviseRounds.ShouldBe(1, "fixture check: the drafted round failed its check, so the executor built a second spec for the revision"); + notices.Select(e => (e.Kind, e.Text)).ShouldBe(said.Select(text => (AgentEventKind.Warning, text)), "a Warning when the run first leaves memory out, and another only when a round's agent changed what is left out"); + } + + /// What CLAUDE.md links to for . + private static string Target(string where, OutsideFile outside) => where == Outside ? outside.Path : where; + + /// What a launch's argv must say about the workspace when CLAUDE.md links to . + private static string Expected(string where) => where == Outside ? "absent" : "added"; + + private static AgentTask TaskWith(Guid repositoryId, IReadOnlyDictionary env) => new() + { + Goal = "make feature.txt say the right thing", + Harness = ClaudeCodeHarness.HarnessKind, + Model = null, + RepositoryId = repositoryId, + Environment = env, + TimeoutSeconds = 120, + Acceptance = new SupervisorAcceptanceSpec { Command = new[] { "sh", "check.sh" }, Description = "the file check" }, + // The contract-implies-gradable-branch invariant AgentCodeNode bakes at authoring, mirrored because the task is built here. + PushProducedBranch = true, + MaxReviseRounds = 1, + }; + + private async Task CreateRunAsync(Guid teamId, Guid userId, Guid repositoryId, IReadOnlyDictionary env) + { + using var scope = _fixture.BeginScopeAs(userId, teamId); + var run = await scope.Resolve().CreateAsync(TaskWith(repositoryId, env), teamId, null, null, iterationKey: "", cancellationToken: CancellationToken.None); + return run.Id; + } + + private async Task ExecuteRealAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + await scope.Resolve().ExecuteAsync(runId, CancellationToken.None); + } + + private async Task<(AgentRun Run, AgentRunResult Result)> LoadAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + var run = await scope.Resolve().GetAsync(runId, CancellationToken.None); + return (run, JsonSerializer.Deserialize(run.ResultJson ?? "{}", AgentJson.Options)!); + } + + private async Task> LoadEventsAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + var run = await scope.Resolve().GetAsync(runId, CancellationToken.None); + return await scope.Resolve().GetEventsAsync(runId, run.TeamId, afterSequence: 0, CancellationToken.None); + } + + private async Task<(Guid TeamId, Guid UserId)> SeedTeamAsync() + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + var userId = Guid.NewGuid(); + db.User.Add(new User { Id = userId, Email = $"memory-{userId:N}@test.local", Name = $"memory-{userId:N}" }); + + var teamId = Guid.NewGuid(); + db.Team.Add(new Team { Id = teamId, Slug = $"memory-{teamId:N}", Name = "Workspace Memory Team", Kind = TeamKind.Workspace }); + db.TeamMembership.Add(new TeamMembership { Id = Guid.NewGuid(), TeamId = teamId, UserId = userId, Role = TeamRole.Owner }); + + await db.SaveChangesAsync(); + return (teamId, userId); + } + + /// A bound repository with a PAT credential, so the clone carries a token and the push path activates — as AgentRunReviseLoopFlowTests seeds it. + private async Task SeedBoundRepositoryAsync(Guid teamId, string cloneUrlHttps) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + + var instanceId = Guid.NewGuid(); + db.ProviderInstance.Add(new ProviderInstance { Id = instanceId, TeamId = teamId, Provider = ProviderKind.GitHub, DisplayName = "local", BaseUrl = "https://local" }); + + var payloadJson = scope.Resolve().Serialize(new PatPayload { Token = "agent-clone-token" }); + + var credentialId = Guid.NewGuid(); + db.Credential.Add(new Credential + { + Id = credentialId, TeamId = teamId, ProviderInstanceId = instanceId, + AuthType = AuthType.Pat, DisplayName = "clone cred", + EncryptedPayload = scope.Resolve().Encrypt(payloadJson), Status = CredentialStatus.Active, + }); + + var repoId = Guid.NewGuid(); + db.Repository.Add(new Repository + { + Id = repoId, TeamId = teamId, ProviderInstanceId = instanceId, CredentialId = credentialId, + ExternalId = repoId.ToString(), NamespacePath = "org", Name = "repo", FullPath = "org/repo", + DefaultBranch = "main", CloneUrlHttps = cloneUrlHttps, WebUrl = "https://local/org/repo", + }); + + await db.SaveChangesAsync(); + return repoId; + } + + /// A file outside every workspace, holding text a model must never be handed. GUID-suffixed; best-effort cleanup. + private sealed class OutsideFile : IDisposable + { + private readonly string _directory = System.IO.Path.Combine(System.IO.Path.GetTempPath(), "cs-memory-outside-" + Guid.NewGuid().ToString("N")); + + public OutsideFile() + { + Directory.CreateDirectory(_directory); + Path = System.IO.Path.Combine(_directory, "secret.md"); + File.WriteAllText(Path, $"OUTSIDE-{Guid.NewGuid():N}\n"); + } + + public string Path { get; } + + public void Dispose() + { + try { Directory.Delete(_directory, recursive: true); } catch { /* best-effort */ } + } + } + + /// A bare local remote whose one commit holds the contract's check, an AGENTS.md, and a CLAUDE.md committed as a symlink. GUID-suffixed; best-effort cleanup. + private sealed class BareRemote : IDisposable + { + private readonly string _root = Path.Combine(Path.GetTempPath(), "cs-memory-remote-" + Guid.NewGuid().ToString("N")); + private readonly string _bare; + + public BareRemote() + { + Directory.CreateDirectory(_root); + _bare = Path.Combine(_root, "remote.git"); + } + + public string Url => new Uri(_bare).AbsoluteUri; + + public async Task SeedAsync(string checkScript, string claudeMdTarget) + { + await Git(_root, "init", "--bare", "-b", "main", _bare); + + var seed = Path.Combine(_root, "seed"); + Directory.CreateDirectory(seed); + await Git(seed, "clone", _bare, seed); + await Git(seed, "config", "user.email", "test@codespace.dev"); + await Git(seed, "config", "user.name", "Test"); + await Git(seed, "config", "commit.gpgsign", "false"); + await File.WriteAllTextAsync(Path.Combine(seed, "check.sh"), checkScript); + await File.WriteAllTextAsync(Path.Combine(seed, "AGENTS.md"), "Keep the change small.\n"); + File.CreateSymbolicLink(Path.Combine(seed, "CLAUDE.md"), claudeMdTarget); + await Git(seed, "add", "-A"); + await Git(seed, "commit", "-m", "seed"); + await Git(seed, "push", "origin", "main"); + } + + private static async Task Git(string workdir, params string[] args) + { + var result = await new LocalProcessRunner().RunAsync(new SandboxSpec { Command = "git", Args = args, WorkingDirectory = workdir, TimeoutSeconds = 60 }, CancellationToken.None); + + if (result.Status != SandboxStatus.Success) + throw new InvalidOperationException($"git {string.Join(' ', args)} failed: {result.Stderr}"); + } + + public void Dispose() + { + try { Directory.Delete(_root, recursive: true); } catch { /* best-effort */ } + } + } + + /// + /// The fake claude: records whether the argv it was spawned with carries --add-dir, exits 9 when that is + /// not what the test expects of its round, and otherwise drafts feature.txt — re-pointing CLAUDE.md as + /// it does, when told to — or writes the revision, when its goal is the executor's revise instruction, and prints a + /// successful stream-json result. Named and staged with the markers, so a real-CLI + /// gate elsewhere in the process sees it for a fake. Arms the process-wide ; + /// restores it and deletes its directory on dispose. + /// + private sealed class MemoryCheckingFakeCli : IDisposable + { + private readonly string? _original; + private readonly string _directory = Path.Combine(Path.GetTempPath(), "cs-memory" + FakeAgentCliMarker.DirectoryMarker + Guid.NewGuid().ToString("N")); + private readonly string _launches; + private readonly string _draft; + private readonly string _revision; + private readonly string _draftRepoints; + + /// What the draft round's argv must say about the workspace: added or absent. + /// The same, for the revision. + /// What the draft round re-points CLAUDE.md to, or null to leave it as committed. + public MemoryCheckingFakeCli(string draft, string revision, string? draftRepoints) + { + (_draft, _revision, _draftRepoints) = (draft, revision, draftRepoints ?? ""); + Directory.CreateDirectory(_directory); + _launches = Path.Combine(_directory, "launches.txt"); + + var script = Path.Combine(_directory, FakeAgentCliMarker.ScriptNamePrefix + "claude.sh"); + File.WriteAllText(script, "#!/bin/sh\n" + FakeAgentCliDialect.ClaudeGoalFunction + $$""" + goal=$(claude_goal) + memory=absent + for arg in "$@"; do [ "$arg" = "--add-dir" ] && memory=added; done + printf '%s\n' "$memory" >> "$FAKE_LAUNCHES" + case "$goal" in {{AgentRunExecutor.ReviseInstructionPrefix}}*) expected=$FAKE_EXPECT_REVISION ;; *) expected=$FAKE_EXPECT_DRAFT ;; esac + [ "$memory" = "$expected" ] || { echo "expected the workspace $expected, argv: $*" >&2; exit 9; } + case "$goal" in + {{AgentRunExecutor.ReviseInstructionPrefix}}*) printf 'revised\n' > feature.txt ;; + *) printf 'draft\n' > feature.txt; [ -z "$FAKE_DRAFT_REPOINTS" ] || ln -sfn "$FAKE_DRAFT_REPOINTS" CLAUDE.md ;; + esac + printf '%s\n' '{"type":"result","subtype":"success","is_error":false,"result":"done"}' + + """); + File.SetUnixFileMode(script, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute | UnixFileMode.GroupRead | UnixFileMode.GroupExecute | UnixFileMode.OtherRead | UnixFileMode.OtherExecute); + + _original = Environment.GetEnvironmentVariable(ClaudeCodeHarness.CommandEnvVar); + Environment.SetEnvironmentVariable(ClaudeCodeHarness.CommandEnvVar, script); + } + + public IReadOnlyDictionary Env() => new Dictionary { ["FAKE_LAUNCHES"] = _launches, ["FAKE_EXPECT_DRAFT"] = _draft, ["FAKE_EXPECT_REVISION"] = _revision, ["FAKE_DRAFT_REPOINTS"] = _draftRepoints }; + + /// What each launch's argv said, in order. + public IReadOnlyList Launches() => File.Exists(_launches) ? File.ReadAllLines(_launches) : Array.Empty(); + + public void Dispose() + { + Environment.SetEnvironmentVariable(ClaudeCodeHarness.CommandEnvVar, _original); + try { Directory.Delete(_directory, recursive: true); } catch { /* best-effort */ } + } + } +} diff --git a/backend/tests/CodeSpace.SandboxTests/RepositoryConfigE2ETests.Memory.cs b/backend/tests/CodeSpace.SandboxTests/RepositoryConfigE2ETests.Memory.cs new file mode 100644 index 000000000..735ce87da --- /dev/null +++ b/backend/tests/CodeSpace.SandboxTests/RepositoryConfigE2ETests.Memory.cs @@ -0,0 +1,141 @@ +using CodeSpace.Core.Services.Agents.Harnesses.Claude; +using CodeSpace.Core.Services.Agents.Sandbox.Isolation; +using CodeSpace.Core.Services.Agents.Sandbox.Runners; +using CodeSpace.Messages.Agents; +using CodeSpace.Messages.Enums; +using Shouldly; + +namespace CodeSpace.SandboxTests; + +/// +/// Repository memory that links outside the workspace. The pinned CLI opens an added directory's CLAUDE.md and +/// .claude/CLAUDE.md by path and follows a symlink at either wherever it leads, and reads a .claude +/// directory that is itself a link wherever it leads, so a repository that commits one of the three as a link out of its +/// workspace hands what it reaches to the model — on Linux, /proc/self/environ would be the CLI's own +/// environment, the run's broker token included. The harness leaves such a repository's directory out of +/// --add-dir (ClaudeWorkspaceMemory) and says so on the launch. Its other escapes — a rules entry linked +/// out, an import that resolves outside — the pinned CLI does not follow (observed against 2.1.263), so they have no +/// positive control here; the unit tests pin that the guard leaves them out all the same. +/// +/// Same fidelity as the class: the pinned binary, the production argv and runner, the production broker; only +/// the model is scripted. The arm runs twice against the same workspace: as production builds it, and — the positive +/// control — with every left-out directory put back into --add-dir, the argv the harness built before the guard. +/// The control must hand each repository's outside file to the model, or the guarded run keeping it away proves +/// nothing. +/// +public sealed partial class RepositoryConfigE2ETests +{ + /// Each entry of a repository's memory the pinned CLI follows out of the workspace when it is a link, and why the guard names it on the launch. + private static readonly (string Escape, string Notice)[] FollowedEscapes = + [ + ("CLAUDE.md", "CLAUDE.md resolves outside the workspace"), + (".claude/CLAUDE.md", ".claude/CLAUDE.md resolves outside the workspace"), + (".claude", ".claude resolves outside the workspace"), + ]; + + /// + /// A multi-repo workspace with one repository for each shape the pinned CLI follows out of it (), + /// each that shape as its one escape and each with memory of its own that stays inside, and a last repository whose + /// CLAUDE.md links to the AGENTS.md beside it. Nothing from outside may reach any request; none of the + /// linking repositories' memory may load, not even what stays inside them; the last one's must still be in the first + /// request; and each linking repository must be missing from --add-dir and named on the launch. Root lane, + /// Confined: the pinned CLI refuses a Standard run's bypassPermissions to uid 0, and plan mode loads memory + /// exactly as any other mode does. + /// + [Fact] + public async Task A_claude_run_leaves_out_repository_memory_that_links_outside_the_workspace() + { + const string harnessKind = ClaudeCodeHarness.HarnessKind; + const AgentAutonomyLevel tier = AgentAutonomyLevel.Confined; + var harness = ReviewerReadsItsDiffE2ETests.HarnessFor(harnessKind); + + if (!ReviewerReadsItsDiffE2ETests.Armed(harnessKind) || OperatingSystem.IsWindows()) return; + + await ReviewerReadsItsDiffE2ETests.RequirePinnedBinaryAsync(harness, harnessKind); + + using var hostile = new ConnectionCounter(); + var workspace = NewWorkspace(repositories: FollowedEscapes.Length + 1); + var linking = workspace.Repositories.Take(FollowedEscapes.Length).ToList(); + var kept = workspace.Repositories[^1]; + + foreach (var (repo, (escape, _)) in linking.Zip(FollowedEscapes)) PlantMemoryLinkedOutside(repo, escape); + + kept.Commit("AGENTS.md", $"{Mention(kept, "MEMORY")}\n"); + kept.CommitLink("CLAUDE.md", "AGENTS.md"); + + var (spec, run, upstream) = await RunAsync(harness, workspace, tier, task => task); + var (unguardedSpec, unguardedRun, unguarded) = await RunAsync(harness, workspace, tier, task => task, reshape: production => WithAddedDirectories(production, linking.Select(repo => repo.Directory))); + + AddedDirectories(unguardedSpec).ShouldBe(new[] { workspace.Directory }.Concat(workspace.Repositories.Select(repo => repo.Directory)), ignoreOrder: true, "fixture check: the control must add every directory the guard left out"); + BrokerViolations(unguardedRun, unguarded, hostile, workspace).ShouldBeEmpty($"fixture check: the control must run to its answer. {Diagnosis(harnessKind, unguardedSpec, unguardedRun, unguarded)}"); + Missing(unguarded, linking, "INSIDE-MEMORY").ShouldBeEmpty($"fixture check: with each linking repository added, its memory that stays inside loads. {Diagnosis(harnessKind, unguardedSpec, unguardedRun, unguarded)}"); + Missing(unguarded, linking, "OUTSIDE-MEMORY").ShouldBeEmpty($"positive control: with each linking repository added, the pinned CLI follows its link out of the workspace and hands the outside file to the model — a repository missing here is a shape the guarded run keeping away proves nothing about. {Diagnosis(harnessKind, unguardedSpec, unguardedRun, unguarded)}"); + + BrokerViolations(run, upstream, hostile, workspace).ShouldBeEmpty(Diagnosis(harnessKind, spec, run, upstream)); + Reached(upstream, linking, "OUTSIDE-MEMORY").ShouldBeEmpty($"nothing from outside the workspace may reach the model. {Diagnosis(harnessKind, spec, run, upstream)}"); + Reached(upstream, linking, "INSIDE-MEMORY").ShouldBeEmpty($"a linking repository is left out whole: none of its memory loads, not even what stays inside it. {Diagnosis(harnessKind, spec, run, upstream)}"); + AddedDirectories(spec).ShouldBe(new[] { workspace.Directory, kept.Directory }, "only the root and the repository whose memory stays inside are added"); + spec.LaunchNotices.ShouldBe(linking.Zip(FollowedEscapes, (repo, shape) => $"Left the memory in '{Path.GetFileName(repo.Directory)}' out of this run: {shape.Notice}.")); + (upstream.Requests.FirstOrDefault(OffersTools)?.Body ?? "").ShouldContain(SurfaceText(kept, "MEMORY"), Case.Sensitive, $"the last repository's CLAUDE.md links inside it, so its memory still loads before the first request. {Diagnosis(harnessKind, spec, run, upstream)}"); + + output.WriteLine($"{RanMarker} memory-link-outside {harnessKind} multi-repo {tier} uid={NonRootWorker.EffectiveUid()} confined={BubblewrapSandbox.Available is not null} shapes={string.Join(',', FollowedEscapes.Select(shape => shape.Escape))}"); + } + + /// + /// committed as a link to an outside file that carries OUTSIDE-MEMORY — for + /// .claude, to an outside directory holding that CLAUDE.md — beside memory of its own that stays inside + /// and carries INSIDE-MEMORY: .claude/CLAUDE.md when the link is CLAUDE.md, CLAUDE.md + /// otherwise. + /// + private void PlantMemoryLinkedOutside(Repository repo, string escape) + { + var outside = NewOutsideDirectory(); + var memory = Path.Combine(outside, "CLAUDE.md"); + + File.WriteAllText(memory, $"{Mention(repo, "OUTSIDE-MEMORY")}\n"); + + repo.CommitLink(escape, escape == ".claude" ? outside : memory); + repo.Commit(escape == "CLAUDE.md" ? ".claude/CLAUDE.md" : "CLAUDE.md", $"{Mention(repo, "INSIDE-MEMORY")}\n"); + } + + /// Every repository of whose text reached a request. + private static IEnumerable Reached(ScriptedModelUpstream upstream, IEnumerable repositories, string surface) => + repositories.Where(repo => upstream.Requests.Any(r => r.Body.Contains(SurfaceText(repo, surface), StringComparison.Ordinal))).Select(repo => repo.Directory); + + /// Every repository of whose text reached no request. + private static IEnumerable Missing(ScriptedModelUpstream upstream, IReadOnlyList repositories, string surface) => + repositories.Select(repo => repo.Directory).Except(Reached(upstream, repositories, surface)); + + /// + /// A directory outside the workspace that the run can still read. Where the host confines, only the system roots are + /// bound beside the workspace and the config home, and /tmp is the sandbox's own, so it goes under one of those + /// roots (this lane runs as root); anywhere else the temp path serves. + /// + private string NewOutsideDirectory() + { + const string boundRoot = "/etc"; + + if (BubblewrapSandbox.Available is not null) BubblewrapSandbox.ReadOnlyRootDirs.ShouldContain(boundRoot, "fixture check: the outside file must sit where the sandbox can see it, or the control finds nothing to follow"); + + var directory = Path.Combine(BubblewrapSandbox.Available is null ? Path.GetTempPath() : boundRoot, $"cs-repo-config-outside-{Guid.NewGuid():N}"); + Directory.CreateDirectory(directory); + _directories.Add(directory); + return directory; + } + + /// The directories one variadic --add-dir carries, in order; none when there is no --add-dir. + private static IReadOnlyList AddedDirectories(SandboxSpec spec) + { + var at = spec.Args.ToList().IndexOf("--add-dir"); + + return at < 0 ? [] : spec.Args.Skip(at + 1).TakeWhile(arg => !arg.StartsWith("--", StringComparison.Ordinal)).ToList(); + } + + /// The spec with each of it does not already add put back at the head of its --add-dir list. + private static SandboxSpec WithAddedDirectories(SandboxSpec spec, IEnumerable directories) + { + var args = spec.Args.ToList(); + args.InsertRange(args.IndexOf("--add-dir") + 1, directories.Except(AddedDirectories(spec), StringComparer.Ordinal)); + return spec with { Args = args }; + } +} diff --git a/backend/tests/CodeSpace.SandboxTests/RepositoryConfigE2ETests.cs b/backend/tests/CodeSpace.SandboxTests/RepositoryConfigE2ETests.cs index abfcd2d58..8f85e84dc 100644 --- a/backend/tests/CodeSpace.SandboxTests/RepositoryConfigE2ETests.cs +++ b/backend/tests/CodeSpace.SandboxTests/RepositoryConfigE2ETests.cs @@ -40,7 +40,9 @@ namespace CodeSpace.SandboxTests; /// and sub/CLAUDE.md only once the run opened a file below sub/, and ran a skill's or command's commands /// only once invoked, so the scripted model opens sub/notes.txt with the CLI's own Read tool, invokes the skill /// and the command, and delegates to the agent (). The single-repo Codex arm also names -/// a repository skill whose agents/openai.yaml depends on an MCP server, which must not start. +/// a repository skill whose agents/openai.yaml depends on an MCP server, which must not start. A repository whose +/// memory links outside the workspace is left out of the run whole, against a positive control that adds it back +/// (). /// /// Fidelity: 🟢 HIGH for everything but the model. The pinned CLI binaries, the production harness argv /// (), the production (bubblewrap where the @@ -98,7 +100,7 @@ namespace CodeSpace.SandboxTests; /// sandbox lanes require, so a silent return can never pass for coverage. /// [Trait("Category", "Sandbox")] -public sealed class RepositoryConfigE2ETests(ITestOutputHelper output) : IDisposable +public sealed partial class RepositoryConfigE2ETests(ITestOutputHelper output) : IDisposable { /// Printed by every arm that actually ran; the sandbox lanes require one per arm in the test output. public const string RanMarker = "[repo-config-e2e] ran"; @@ -672,8 +674,8 @@ private static void WriteCodexConfig(string directory, ConnectionCounter hostile private static JsonArray CodexHook(string command) => new(new JsonObject { ["hooks"] = new JsonArray(new JsonObject { ["type"] = "command", ["command"] = command }) }); - /// The run launched the way the executor launches it, in and at 's production permissions, against — by default a scripted model that asks for nothing and answers. - private async Task<(SandboxSpec Spec, Run Run, ScriptedModelUpstream Upstream)> RunAsync(IAgentHarness harness, Workspace workspace, AgentAutonomyLevel tier, Func shape, ScriptedModelUpstream? upstream = null) + /// The run launched the way the executor launches it, in and at 's production permissions, against — by default a scripted model that asks for nothing and answers. , when given, alters the production spec before launch: a positive control's way of running what the harness would have built without a guard. + private async Task<(SandboxSpec Spec, Run Run, ScriptedModelUpstream Upstream)> RunAsync(IAgentHarness harness, Workspace workspace, AgentAutonomyLevel tier, Func shape, ScriptedModelUpstream? upstream = null, Func? reshape = null) { upstream ??= new ScriptedModelUpstream([], $"DONE-{workspace.Nonce}"); using var broker = LoopbackModelCredentialBroker.ForTest(upstream); @@ -692,7 +694,8 @@ private static void WriteCodexConfig(string directory, ConnectionCounter hostile Environment = new Dictionary(ReviewerReadsItsDiffE2ETests.Brokered(harness, brokered)) { ["HOME"] = NewDirectory("repo-config-home") }, }); - var spec = ReviewerReadsItsDiffE2ETests.ProductionSpec(harness, task, brokered, UpstreamBaseUrl, UpstreamProvider); + var production = ReviewerReadsItsDiffE2ETests.ProductionSpec(harness, task, brokered, UpstreamBaseUrl, UpstreamProvider); + var spec = reshape is null ? production : reshape(production); var lines = new List(); using var budget = new CancellationTokenSource(TimeSpan.FromSeconds((task.TimeoutSeconds ?? 300) + 60)); @@ -822,6 +825,17 @@ public void Commit(string relativePath, string content, bool executable = false) ReviewerReadsItsDiffE2ETests.GitOut(Directory, $"add -f -- {relativePath}"); ReviewerReadsItsDiffE2ETests.GitOut(Directory, $"commit -q -m {Path.GetFileName(relativePath)}"); } + + /// Commit a symlink to , spelled exactly as given, the way git stores and checks one out. + public void CommitLink(string relativePath, string target) + { + var path = Path.Combine(Directory, relativePath); + System.IO.Directory.CreateDirectory(Path.GetDirectoryName(path)!); + File.CreateSymbolicLink(path, target); + + ReviewerReadsItsDiffE2ETests.GitOut(Directory, $"add -f -- {relativePath}"); + ReviewerReadsItsDiffE2ETests.GitOut(Directory, $"commit -q -m {Path.GetFileName(relativePath)}"); + } } /// One file per command planted in , inside it — writable to a Standard run, so a command that did run there could not fail to leave its mark. diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/AgentRunExecutorLaunchNoticeTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/AgentRunExecutorLaunchNoticeTests.cs new file mode 100644 index 000000000..c0e0c38be --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/AgentRunExecutorLaunchNoticeTests.cs @@ -0,0 +1,51 @@ +using CodeSpace.Core.Services.Agents; +using Shouldly; + +namespace CodeSpace.UnitTests.Workflows; + +/// +/// The one timeline event a launch's notices become (): every +/// notice in order up to the bound, then a count of the rest, and no event at all when there is nothing to say; and +/// whether a later launch of the same run says anything again (). +/// That a revise round is announced only when its agent changed what the notices describe is pinned against the real +/// executor by RealHarnessWorkspaceMemoryTests. +/// +[Trait("Category", "Unit")] +public sealed class AgentRunExecutorLaunchNoticeTests +{ + [Fact] + public void No_notice_means_no_event() => + AgentRunExecutor.DescribeLaunchNotices(Array.Empty()).ShouldBeNull(); + + [Fact] + public void Every_notice_up_to_the_bound_is_said_in_order() + { + var notices = Notices(AgentRunExecutor.MaxLaunchNotices); + + AgentRunExecutor.DescribeLaunchNotices(notices).ShouldBe(string.Join(" ", notices)); + } + + [Fact] + public void Past_the_bound_the_rest_are_counted_not_repeated() + { + var notices = Notices(AgentRunExecutor.MaxLaunchNotices + 3); + + AgentRunExecutor.DescribeLaunchNotices(notices).ShouldBe(string.Join(" ", notices.Take(AgentRunExecutor.MaxLaunchNotices)) + " (3 more)"); + } + + [Theory] + [InlineData(null, null, null)] // the launch leaves nothing out and nothing was said: no event + [InlineData("A", null, "A")] // the launch leaves something out: say it + [InlineData("A", "A", null)] // a revise round leaves out the same: said once per run + [InlineData("B", "A", "B")] // a revise round leaves out something else: say what + [InlineData(null, "A", AgentRunExecutor.LaunchNoticesClearedNote)] // a revise round leaves out nothing the last did: say that it loads again + public void A_later_launch_is_announced_only_when_what_it_leaves_out_changes(string? notice, string? said, string? announced) => + AgentRunExecutor.DescribeLaunchNoticeChange(notice is null ? Array.Empty() : new[] { notice }, said).ShouldBe(announced); + + [Fact] + public void The_bound_is_pinned() => + AgentRunExecutor.MaxLaunchNotices.ShouldBe(10); + + private static IReadOnlyList Notices(int count) => + Enumerable.Range(1, count).Select(i => $"Left the memory in 'repo-{i}' out of this run: CLAUDE.md resolves outside the workspace.").ToList(); +} diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/ClaudeCodeHarnessTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/ClaudeCodeHarnessTests.cs index 1b7d8c90b..c3976d701 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/ClaudeCodeHarnessTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/ClaudeCodeHarnessTests.cs @@ -23,13 +23,23 @@ public class ClaudeCodeHarnessTests { private static readonly ClaudeCodeHarness Harness = new(); + /// + /// The workspace every task here names: a path no host holds. The harness reads the memory under its workspace + /// before adding it (), so a literal such as /tmp/ws would make these + /// pins depend on whatever a developer's machine happens to keep there. + /// + private static readonly string Ws = $"/tmp/cs-claude-ws-{Guid.NewGuid():N}"; + + /// as Claude names its sessions' directory: every character but a letter or digit becomes '-'. + private static readonly string WsProjects = Ws.Replace('/', '-'); + private static AgentTask Task(string goal = "Fix the failing billing tests", string? model = "claude-opus-4-8", AgentWriteScope scope = AgentWriteScope.Workspace, IReadOnlyList? tools = null) => new() { Goal = goal, Harness = ClaudeCodeHarness.HarnessKind, Model = model, Tools = tools, - WorkspaceDirectory = "/tmp/ws", + WorkspaceDirectory = Ws, Permissions = new AgentPermissions { WriteScope = scope }, TimeoutSeconds = 900, }; @@ -102,20 +112,24 @@ public void The_workspace_comes_back_as_an_added_directory_so_its_memory_still_l var at = args.IndexOf("--add-dir"); at.ShouldBeGreaterThanOrEqualTo(0, "the workspace must be added back for its memory to load"); - args[at + 1].ShouldBe("/tmp/ws", "the directory added is the run's own workspace"); + args[at + 1].ShouldBe(Ws, "the directory added is the run's own workspace"); args[at + 2].ShouldStartWith("--", customMessage: "--add-dir is variadic: the next token must be a flag that terminates it, never a value it would swallow"); spec.Environment[ClaudeCodeHarness.AdditionalDirectoriesMemoryEnvVar].ShouldBe("1", "without the switch the added directory contributes no memory"); } public static TheoryData WorkspaceShapes() => new() { - // shape, the workspace (the cwd), the repository directories the executor stamps, the directories added - { "single-repo", "/tmp/ws", new[] { "/tmp/ws" }, new[] { "/tmp/ws" } }, - { "multi-repo at its root", "/tmp/ws", new[] { "/tmp/ws/web", "/tmp/ws/api" }, new[] { "/tmp/ws", "/tmp/ws/web", "/tmp/ws/api" } }, - { "multi-repo at its primary repository", "/tmp/ws/web", new[] { "/tmp/ws/web", "/tmp/ws/api" }, new[] { "/tmp/ws/web" } }, - { "named by its producer, no repositories stamped", "/tmp/ws", null, new[] { "/tmp/ws" } }, + // shape, the workspace (the cwd), the repository directories the executor stamps, the directories added — each + // relative to Ws, "" being Ws itself + { "single-repo", "", new[] { "" }, new[] { "" } }, + { "multi-repo at its root", "", new[] { "web", "api" }, new[] { "", "web", "api" } }, + { "multi-repo at its primary repository", "web", new[] { "web", "api" }, new[] { "web" } }, + { "named by its producer, no repositories stamped", "", null, new[] { "" } }, }; + /// A path relative to ; "" is Ws itself. + private static string At(string relative) => relative.Length == 0 ? Ws : $"{Ws}/{relative}"; + [Theory] [MemberData(nameof(WorkspaceShapes))] public void Every_repository_inside_the_workspace_is_added_so_each_ones_memory_loads(string shape, string workspace, string[]? repositories, string[] added) @@ -124,9 +138,9 @@ public void Every_repository_inside_the_workspace_is_added_so_each_ones_memory_l // sits in its own directory below it. The pinned CLI loads every added directory's memory and none of its // settings (RepositoryConfigE2ETests). A repository outside the cwd — a primary-repository cwd's siblings — is // left out: the unpinned CLI never loaded its memory, and an added directory also widens what the CLI's tools may touch. - var args = Harness.BuildInvocation(Task() with { WorkspaceDirectory = workspace, WorkspaceRepositoryDirectories = repositories }).Args.ToList(); + var args = Harness.BuildInvocation(Task() with { WorkspaceDirectory = At(workspace), WorkspaceRepositoryDirectories = repositories?.Select(At).ToList() }).Args.ToList(); - args.Skip(args.IndexOf("--add-dir") + 1).TakeWhile(arg => !arg.StartsWith("--", StringComparison.Ordinal)).ShouldBe(added, $"a {shape} workspace"); + args.Skip(args.IndexOf("--add-dir") + 1).TakeWhile(arg => !arg.StartsWith("--", StringComparison.Ordinal)).ShouldBe(added.Select(At), $"a {shape} workspace"); args.Count(arg => arg == "--add-dir").ShouldBe(1, "one variadic --add-dir carries every directory"); } @@ -143,6 +157,59 @@ public void A_run_with_no_workspace_adds_no_directory_and_no_memory_switch(strin spec.Args[spec.Args.ToList().IndexOf("--setting-sources") + 1].ShouldBe("user", "but the settings pin stays"); } + [Fact] + public void A_repository_whose_memory_links_outside_the_workspace_is_not_added_and_the_launch_says_so() + { + // The CLI opens an added directory's CLAUDE.md by path and follows a symlink there wherever it leads. web's + // leads out of the workspace, so web is not added; api's CLAUDE.md links to its own AGENTS.md and still is. + if (OperatingSystem.IsWindows()) return; + + using var tree = new TempTree(); + var workspace = tree.Directory("ws"); + var web = tree.Directory("ws/web"); + var api = tree.Directory("ws/api"); + + tree.Link("ws/web/CLAUDE.md", tree.File("outside/secret.md", "OUTSIDE")); + tree.File("ws/api/AGENTS.md", "API MEMORY"); + tree.Link("ws/api/CLAUDE.md", "AGENTS.md"); + + var spec = Harness.BuildInvocation(Task() with { WorkspaceDirectory = workspace, WorkspaceRepositoryDirectories = new[] { web, api } }); + var args = spec.Args.ToList(); + + args.Skip(args.IndexOf("--add-dir") + 1).TakeWhile(arg => !arg.StartsWith("--", StringComparison.Ordinal)).ShouldBe(new[] { workspace, api }, "the workspace and api are added as before; web is not"); + spec.Environment[ClaudeCodeHarness.AdditionalDirectoriesMemoryEnvVar].ShouldBe("1", "the directories still added still load their memory"); + spec.LaunchNotices.ShouldBe(new[] { "Left the memory in 'web' out of this run: CLAUDE.md resolves outside the workspace." }); + } + + [Fact] + public void A_run_left_with_no_memory_directory_adds_none_and_still_pins_its_settings() + { + if (OperatingSystem.IsWindows()) return; + + using var tree = new TempTree(); + var workspace = tree.Directory("ws"); + + tree.Link("ws/CLAUDE.md", tree.File("outside/secret.md", "OUTSIDE")); + + var spec = Harness.BuildInvocation(Task() with { WorkspaceDirectory = workspace, WorkspaceRepositoryDirectories = new[] { workspace } }); + + spec.Args.ShouldNotContain("--add-dir", "an empty variadic --add-dir would swallow the next flag as its value"); + spec.Environment.ContainsKey(ClaudeCodeHarness.AdditionalDirectoriesMemoryEnvVar).ShouldBeFalse("the switch rides only with a directory it applies to"); + spec.Args.Count(arg => arg == "--setting-sources").ShouldBe(1); + spec.Args[spec.Args.ToList().IndexOf("--setting-sources") + 1].ShouldBe("user", "the settings pin never depends on what memory is left"); + spec.LaunchNotices.ShouldBe(new[] { "Left the memory in the workspace out of this run: CLAUDE.md resolves outside the workspace." }); + } + + [Fact] + public void LaunchNotices_never_reach_the_serialized_spec() + { + // The launch frame and the invocation's binding identity are the serialized spec: a notice is for the timeline only. + var spec = Harness.BuildInvocation(Task()) with { LaunchNotices = new[] { "Left the memory in the workspace out of this run: CLAUDE.md resolves outside the workspace." } }; + + JsonSerializer.Serialize(spec).ShouldNotContain("LaunchNotices"); + JsonSerializer.Serialize(spec).ShouldBe(JsonSerializer.Serialize(spec with { LaunchNotices = Array.Empty() })); + } + [Fact] public void AdditionalDirectoriesMemoryEnvVar_constant_name_is_pinned() => // Rule 8: Claude Code reads this exact name; a rename silently drops every run's project memory. @@ -260,7 +327,7 @@ public void Restores_the_prior_transcript_as_a_config_home_file_on_a_continue() var transcript = Harness.BuildInvocation(task).ConfigHomeFiles.SingleOrDefault(f => f.RelativePath.StartsWith("projects/", StringComparison.Ordinal)); transcript.ShouldNotBeNull("a CONTINUE restores the prior session JSONL where --resume looks"); - transcript!.RelativePath.ShouldBe("projects/-tmp-ws/sess-r1.jsonl", "projects//.jsonl (cwd /tmp/ws → -tmp-ws)"); + transcript!.RelativePath.ShouldBe($"projects/{WsProjects}/sess-r1.jsonl", "projects//.jsonl (every character of the cwd but a letter or digit → '-')"); transcript.Content.ShouldBe("{\"type\":\"summary\"}\n{\"type\":\"user\"}\n", "the transcript bytes are restored verbatim"); } @@ -277,7 +344,7 @@ public void Restored_transcript_is_added_alongside_projected_skills_not_instead_ var paths = Harness.BuildInvocation(task).ConfigHomeFiles.Select(f => f.RelativePath).ToList(); paths.ShouldContain("skills/tdd/SKILL.md"); - paths.ShouldContain("projects/-tmp-ws/sess-r2.jsonl"); + paths.ShouldContain($"projects/{WsProjects}/sess-r2.jsonl"); } [Fact] @@ -302,7 +369,7 @@ public void A_restored_transcript_round_trips_through_materialization_to_the_enc { LocalProcessRunner.WriteConfigHomeFiles(spec.ConfigHomeFiles, configHome); - File.ReadAllText(Path.Combine(configHome, "projects", "-tmp-ws", "sess-rt.jsonl")) + File.ReadAllText(Path.Combine(configHome, "projects", WsProjects, "sess-rt.jsonl")) .ShouldBe("{\"line\":1}\n{\"line\":2}\n", "the runner materialized the transcript exactly where --resume reads it"); } finally @@ -319,12 +386,12 @@ public void The_session_transcript_capture_path_equals_the_restore_path_for_the_ // continue cold-starts. This pins that symmetry: SessionTranscriptRelativePath == the restore ConfigHomeFile's // RelativePath, both projects//.jsonl. // Claude's path is COMPUTED from cwd+id, so the configHome arg is ignored (no search). - var capturePath = ((IAgentSessionTranscript)Harness).SessionTranscriptRelativePath("/tmp/cfg", "/tmp/ws", "sess-x"); + var capturePath = ((IAgentSessionTranscript)Harness).SessionTranscriptRelativePath("/tmp/cfg", Ws, "sess-x"); var restorePath = Harness.BuildInvocation(Task() with { ResumeFromSessionId = "sess-x", RestoredTranscript = "x\n" }) .ConfigHomeFiles.Single(f => f.RelativePath.StartsWith("projects/", StringComparison.Ordinal)).RelativePath; - capturePath.ShouldBe("projects/-tmp-ws/sess-x.jsonl"); + capturePath.ShouldBe($"projects/{WsProjects}/sess-x.jsonl"); capturePath.ShouldBe(restorePath, "the executor must read the session file from exactly where the harness restores it"); } @@ -334,7 +401,7 @@ public void Session_transcript_path_is_null_without_a_cwd_or_session_id() var h = (IAgentSessionTranscript)Harness; h.SessionTranscriptRelativePath("/tmp/cfg", null, "sess").ShouldBeNull("no cwd → nothing to encode the projects dir on"); - h.SessionTranscriptRelativePath("/tmp/cfg", "/tmp/ws", null).ShouldBeNull("no session id → nothing to name the file"); + h.SessionTranscriptRelativePath("/tmp/cfg", Ws, null).ShouldBeNull("no session id → nothing to name the file"); h.SessionTranscriptRelativePath("/tmp/cfg", " ", "sess").ShouldBeNull("a blank cwd is not addressable"); } @@ -625,9 +692,9 @@ public void Builds_a_claude_print_stream_json_invocation_from_the_task() var spec = Harness.BuildInvocation(Task()); spec.Command.ShouldBe("claude"); - spec.Args.ShouldBe(new[] { "--print", "--output-format", "stream-json", "--verbose", "--input-format", "stream-json", "--append-system-prompt", AgentOperatingContract.SystemDirective, "--setting-sources", "user", "--add-dir", "/tmp/ws", "--model", "claude-opus-4-8", "--permission-mode", "bypassPermissions" }); + spec.Args.ShouldBe(new[] { "--print", "--output-format", "stream-json", "--verbose", "--input-format", "stream-json", "--append-system-prompt", AgentOperatingContract.SystemDirective, "--setting-sources", "user", "--add-dir", Ws, "--model", "claude-opus-4-8", "--permission-mode", "bypassPermissions" }); GoalOf(spec.StandardInput).ShouldBe("Fix the failing billing tests"); - spec.WorkingDirectory.ShouldBe("/tmp/ws"); + spec.WorkingDirectory.ShouldBe(Ws); spec.TimeoutSeconds.ShouldBe(900); } @@ -646,7 +713,7 @@ public void Builds_a_resume_invocation_when_a_prior_session_is_set() // trailing positional and the prompt is never swallowed. var spec = Harness.BuildInvocation(Task() with { ResumeFromSessionId = "sess-resume-1" }); - spec.Args.ShouldBe(new[] { "--print", "--output-format", "stream-json", "--verbose", "--input-format", "stream-json", "--resume", "sess-resume-1", "--append-system-prompt", AgentOperatingContract.SystemDirective, "--setting-sources", "user", "--add-dir", "/tmp/ws", "--model", "claude-opus-4-8", "--permission-mode", "bypassPermissions" }); + spec.Args.ShouldBe(new[] { "--print", "--output-format", "stream-json", "--verbose", "--input-format", "stream-json", "--resume", "sess-resume-1", "--append-system-prompt", AgentOperatingContract.SystemDirective, "--setting-sources", "user", "--add-dir", Ws, "--model", "claude-opus-4-8", "--permission-mode", "bypassPermissions" }); } [Fact] @@ -656,7 +723,7 @@ public void Omits_the_resume_flag_when_no_prior_session() var spec = Harness.BuildInvocation(Task() with { ResumeFromSessionId = null }); spec.Args.ShouldNotContain("--resume"); - spec.Args.ShouldBe(new[] { "--print", "--output-format", "stream-json", "--verbose", "--input-format", "stream-json", "--append-system-prompt", AgentOperatingContract.SystemDirective, "--setting-sources", "user", "--add-dir", "/tmp/ws", "--model", "claude-opus-4-8", "--permission-mode", "bypassPermissions" }); + spec.Args.ShouldBe(new[] { "--print", "--output-format", "stream-json", "--verbose", "--input-format", "stream-json", "--append-system-prompt", AgentOperatingContract.SystemDirective, "--setting-sources", "user", "--add-dir", Ws, "--model", "claude-opus-4-8", "--permission-mode", "bypassPermissions" }); } [Theory] @@ -668,7 +735,7 @@ public void Omits_the_model_flag_when_no_model_is_set(string? model) var spec = Harness.BuildInvocation(Task(model: model)); spec.Args.ShouldNotContain("--model", customMessage: "a blank model must omit --model so the CLI uses its own default (the Model=empty rule)"); - spec.Args.ShouldBe(new[] { "--print", "--output-format", "stream-json", "--verbose", "--input-format", "stream-json", "--append-system-prompt", AgentOperatingContract.SystemDirective, "--setting-sources", "user", "--add-dir", "/tmp/ws", "--permission-mode", "bypassPermissions" }); + spec.Args.ShouldBe(new[] { "--print", "--output-format", "stream-json", "--verbose", "--input-format", "stream-json", "--append-system-prompt", AgentOperatingContract.SystemDirective, "--setting-sources", "user", "--add-dir", Ws, "--permission-mode", "bypassPermissions" }); } [Fact] diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/ClaudeWorkspaceMemoryTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/ClaudeWorkspaceMemoryTests.cs new file mode 100644 index 000000000..1aea66d38 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/ClaudeWorkspaceMemoryTests.cs @@ -0,0 +1,414 @@ +using System.Diagnostics; +using CodeSpace.Core.Services.Agents.Harnesses; +using CodeSpace.Core.Services.Agents.Harnesses.Claude; +using CodeSpace.Messages.Agents; +using Shouldly; + +namespace CodeSpace.UnitTests.Workflows; + +/// +/// Pins the guard that keeps a Claude run from adding a directory whose memory reaches outside the workspace +/// (). The pinned CLI follows a symlink at an added directory's CLAUDE.md, +/// .claude/CLAUDE.md or .claude wherever it leads, and the guard treats a rules entry or an import that +/// leads out the same way, for a later CLI that follows those too. Each case lays out a real workspace and a real file +/// outside it under a GUID temp root, links them, and asserts the directory is left out with the notice the run's +/// timeline will show — or, for a link that stays inside or dangles, or a memory that is only large or wide, that it is +/// still added and nothing is said. The workspace is spelled through 's root link, so on every +/// host each case also compares a workspace reached through a symlink. +/// +[Trait("Category", "Unit")] +public sealed class ClaudeWorkspaceMemoryTests : IDisposable +{ + private readonly TempTree _tree = new(); + private readonly string _workspace; + private readonly string _secret; + + public ClaudeWorkspaceMemoryTests() + { + _workspace = _tree.Directory("ws"); + _secret = _tree.File("outside/secret.md", "OUTSIDE"); + } + + public void Dispose() => _tree.Dispose(); + + [Theory] + [InlineData("a CLAUDE.md linked outside", "CLAUDE.md resolves outside the workspace")] + [InlineData("a .claude/CLAUDE.md linked outside by a relative target", ".claude/CLAUDE.md resolves outside the workspace")] + [InlineData("a .claude directory linked outside", ".claude resolves outside the workspace")] + [InlineData("a .claude directory linked to an outside directory that holds nothing yet", ".claude resolves outside the workspace")] + [InlineData("a chain of links that ends outside", "CLAUDE.md resolves outside the workspace")] + [InlineData("a rule linked outside", ".claude/rules/style.md resolves outside the workspace")] + [InlineData("a rules folder linked outside", ".claude/rules/team resolves outside the workspace")] + [InlineData("a link whose .. climbs out of a linked directory", "CLAUDE.md resolves outside the workspace")] + [InlineData("an import through an in-workspace link", "a file CLAUDE.md imports resolves outside the workspace")] + [InlineData("an import of an outside file by its absolute path", "a file CLAUDE.md imports resolves outside the workspace")] + [InlineData("an import that climbs out with ..", "a file .claude/CLAUDE.md imports resolves outside the workspace")] + [InlineData("an import inside a code block", "a file CLAUDE.md imports resolves outside the workspace")] + [InlineData("an import a scoped rule makes", "a file .claude/rules/scoped.md imports resolves outside the workspace")] + [InlineData("an import with an escaped space", "a file CLAUDE.md imports resolves outside the workspace")] + [InlineData("an import with a fragment", "a file CLAUDE.md imports resolves outside the workspace")] + public void A_directory_whose_memory_reaches_outside_the_workspace_is_left_out(string shape, string escape) + { + if (OperatingSystem.IsWindows()) return; + + PlantEscape(shape); + + var plan = Plan(); + + plan.Directories.ShouldBeEmpty($"{shape}: the CLI would read the outside file through this directory"); + plan.Notices.ShouldBe(new[] { $"Left the memory in the workspace out of this run: {escape}." }, $"{shape}: one notice naming what reached outside"); + } + + [Theory] + [InlineData("no memory at all")] + [InlineData("a CLAUDE.md linked to the AGENTS.md beside it")] + [InlineData("a .claude directory linked to another inside the workspace")] + [InlineData("a CLAUDE.md linked to nothing")] + [InlineData("a CLAUDE.md linked to itself")] + [InlineData("imports of files that do not exist")] + [InlineData("imports naming directories")] + [InlineData("an import of the run's home")] + [InlineData("addresses and handles")] + [InlineData("a file in the rules folder that is not markdown")] + [InlineData("an image in the rules folder linked outside")] + [InlineData("70 rules scoped by paths:")] + [InlineData("5 rules beside 60 images")] + [InlineData("a 200 KiB CLAUDE.md")] + [InlineData("a CLAUDE.md importing a 200 KiB README")] + [InlineData("1100 email addresses")] + public void A_directory_whose_memory_stays_inside_the_workspace_or_reaches_nothing_is_still_added(string shape) + { + if (OperatingSystem.IsWindows()) return; + + PlantKept(shape); + + var plan = Plan(); + + plan.Directories.ShouldBe(new[] { _workspace }, $"{shape}: nothing outside the workspace can load through this directory"); + plan.Notices.ShouldBeEmpty(shape); + } + + [Fact] + public void Only_the_repository_whose_memory_reaches_outside_is_left_out_and_the_rest_keep_their_order() + { + if (OperatingSystem.IsWindows()) return; + + var a = _tree.Directory("ws/a"); + var b = _tree.Directory("ws/b"); + var c = _tree.Directory("ws/c"); + + _tree.File("ws/a/CLAUDE.md", "A"); + _tree.Link("ws/b/CLAUDE.md", _secret); + _tree.File("ws/c/CLAUDE.md", "C"); + + var plan = Plan(a, b, c); + + plan.Directories.ShouldBe(new[] { _workspace, a, c }); + plan.Notices.ShouldBe(new[] { "Left the memory in 'b' out of this run: CLAUDE.md resolves outside the workspace." }); + } + + [Theory] + [InlineData(5, true)] // the link is the fifth import down: the guard still resolves it + [InlineData(6, false)] // one further: the CLI reads no file five imports down, so it never reaches the link + public void Imports_are_followed_five_hops_deep(int linkHop, bool leftOut) + { + if (OperatingSystem.IsWindows()) return; + + for (var hop = 0; hop < linkHop; hop++) _tree.File($"ws/{Hop(hop)}", $"Next: @{Hop(hop + 1)}\n"); + + _tree.Link($"ws/{Hop(linkHop)}", _secret); + + Plan().Directories.ShouldBe(leftOut ? Array.Empty() : new[] { _workspace }); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void A_memory_naming_more_paths_than_the_lookup_bound_is_left_out_unchecked(bool overTheBound) + { + // The four places the CLI reads memory from are paths to resolve too, so CLAUDE.md names four fewer than the bound. + if (OperatingSystem.IsWindows()) return; + + var imports = ClaudeWorkspaceMemory.MaxLookups - 4 + (overTheBound ? 1 : 0); + + _tree.File("ws/CLAUDE.md", string.Join(' ', Enumerable.Range(0, imports).Select(i => $"@missing-{i}.md"))); + + var plan = Plan(); + + plan.Directories.ShouldBe(overTheBound ? Array.Empty() : new[] { _workspace }); + plan.Notices.ShouldBe(overTheBound ? new[] { $"Left the memory in the workspace out of this run: its memory names more than {ClaudeWorkspaceMemory.MaxLookups} paths to check." } : Array.Empty()); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void A_memory_larger_than_the_scan_bound_is_left_out_unchecked(bool overTheBound) + { + // The bound is on the memory, not on one file: two files, each under it, that pass it together. + if (OperatingSystem.IsWindows()) return; + + var half = ClaudeWorkspaceMemory.MaxScannedBytes / 2; + + _tree.File("ws/CLAUDE.md", new string('x', half)); + _tree.File("ws/.claude/CLAUDE.md", new string('y', ClaudeWorkspaceMemory.MaxScannedBytes - half + (overTheBound ? 1 : 0))); + + var plan = Plan(); + + plan.Directories.ShouldBe(overTheBound ? Array.Empty() : new[] { _workspace }); + plan.Notices.ShouldBe(overTheBound ? new[] { $"Left the memory in the workspace out of this run: its memory spans more than {ClaudeWorkspaceMemory.MaxScannedBytes} bytes to check." } : Array.Empty()); + } + + [Theory] + [InlineData(true)] // the rule links into a sibling repository the same workspace holds: the run's own clone + [InlineData(false)] // the same link, to a file outside the workspace root + public void A_primary_repository_cwd_may_link_its_memory_into_a_sibling_repository(bool intoSibling) + { + // A multi-repo run whose cwd is its primary repository: the other repositories sit beside the cwd, inside the + // workspace root, and are not added — but memory linking into them brings in nothing from outside the run. + if (OperatingSystem.IsWindows()) return; + + var primary = _tree.Directory("ws/repo-a"); + var sibling = _tree.Directory("ws/repo-b"); + + _tree.File("ws/repo-b/.claude/rules/shared.md", "Shared rule.\n"); + _tree.Link("ws/repo-a/.claude/rules/shared.md", intoSibling ? "../../../repo-b/.claude/rules/shared.md" : _secret); + + var plan = ClaudeWorkspaceMemory.For(new AgentTask { Goal = "g", Harness = ClaudeCodeHarness.HarnessKind, WorkspaceDirectory = primary, WorkspaceRepositoryDirectories = [primary, sibling] }); + + plan.Directories.ShouldBe(intoSibling ? new[] { primary } : Array.Empty(), "only the cwd is added either way; the sibling is the run's own, the outside file is not"); + plan.Notices.ShouldBe(intoSibling ? Array.Empty() : new[] { "Left the memory in the workspace out of this run: .claude/rules/shared.md resolves outside the workspace." }); + } + + [Fact] + public void A_workspace_reached_through_a_link_that_climbs_out_of_another_link_is_checked_where_it_really_is() + { + // data → srv/link/../disk with srv/link → x/y: the kernel climbs from where srv/link really is, so data is x/disk. + // Resolving the workspace and its memory with two different readings of that link threw before the launch. + if (OperatingSystem.IsWindows()) return; + + _tree.Directory("x/y"); + _tree.File("x/disk/ws/CLAUDE.md", "Memory.\n"); + _tree.Link("srv/link", Path.Combine(_tree.Root, "x", "y")); + _tree.Link("data", Path.Combine(_tree.Root, "srv", "link", "..", "disk")); + + var workspace = Path.Combine(_tree.Root, "data", "ws"); + var plan = ClaudeWorkspaceMemory.For(new AgentTask { Goal = "g", Harness = ClaudeCodeHarness.HarnessKind, WorkspaceDirectory = workspace, WorkspaceRepositoryDirectories = [workspace] }); + + plan.Directories.ShouldBe(new[] { workspace }); + plan.Notices.ShouldBeEmpty(); + } + + [Fact] + public async Task A_fifo_in_the_memory_is_never_opened_for_reading() + { + // A blocking open of a FIFO waits for a writer that never comes, and the build would wait with it. The CLI reads + // no FIFO either, so the directory is added as if the file were absent. + if (OperatingSystem.IsWindows()) return; + + await MakeFifoAsync(Path.Combine(_workspace, "CLAUDE.md")); + Directory.CreateDirectory(Path.Combine(_workspace, ".claude", "rules")); + await MakeFifoAsync(Path.Combine(_workspace, ".claude", "rules", "pipe.md")); + + var plan = await System.Threading.Tasks.Task.Run(Plan).WaitAsync(TimeSpan.FromSeconds(1)); + + plan.Directories.ShouldBe(new[] { _workspace }); + plan.Notices.ShouldBeEmpty(); + } + + [Fact] + public void A_name_the_repository_chose_reaches_the_notice_without_control_characters() + { + if (OperatingSystem.IsWindows()) return; + + _tree.Link("ws/.claude/rules/evil\nname.md", _secret); + + Plan().Notices.ShouldBe(new[] { "Left the memory in the workspace out of this run: .claude/rules/evil?name.md resolves outside the workspace." }); + } + + [Fact] + public void A_name_the_repository_chose_is_cut_in_the_notice() + { + if (OperatingSystem.IsWindows()) return; + + var name = new string('n', 150) + ".md"; + var origin = $".claude/rules/{name}"; + + _tree.Link($"ws/.claude/rules/{name}", _secret); + + Plan().Notices.ShouldBe(new[] { $"Left the memory in the workspace out of this run: {origin[..ClaudeWorkspaceMemory.MaxNoticeNameLength]}… resolves outside the workspace." }); + } + + [Fact] + public void A_run_with_no_workspace_adds_nothing_and_says_nothing() + { + var plan = ClaudeWorkspaceMemory.For(new AgentTask { Goal = "g", Harness = ClaudeCodeHarness.HarnessKind }); + + plan.Directories.ShouldBeEmpty(); + plan.Notices.ShouldBeEmpty(); + } + + [Fact] + public void The_bounds_are_pinned() + { + // Committed values, changed by PR: each one decides how much of a repository's memory the guard reads before it + // gives up and leaves the directory out. + ClaudeWorkspaceMemory.MaxImportHops.ShouldBe(5, "the CLI's own import depth (wgs=5 in 2.1.263)"); + ClaudeWorkspaceMemory.MaxLookups.ShouldBe(16384); + ClaudeWorkspaceMemory.MaxScannedBytes.ShouldBe(4194304); + ClaudeWorkspaceMemory.MaxNoticeNameLength.ShouldBe(120); + PhysicalPath.MaxLinkHops.ShouldBe(40, "the kernel's own MAXSYMLINKS"); + } + + private ClaudeWorkspaceMemory.Plan Plan() => Plan(_workspace); + + private ClaudeWorkspaceMemory.Plan Plan(params string[] repositories) => + ClaudeWorkspaceMemory.For(new AgentTask { Goal = "g", Harness = ClaudeCodeHarness.HarnessKind, WorkspaceDirectory = _workspace, WorkspaceRepositoryDirectories = repositories }); + + private static string Hop(int hop) => hop == 0 ? "CLAUDE.md" : $"hop-{hop}.md"; + + private string Outside(string relative) => Path.Combine(_tree.Root, "outside", relative); + + private void PlantEscape(string shape) + { + switch (shape) + { + case "a CLAUDE.md linked outside": + _tree.Link("ws/CLAUDE.md", _secret); + break; + case "a .claude/CLAUDE.md linked outside by a relative target": + _tree.Link("ws/.claude/CLAUDE.md", "../../outside/secret.md"); + break; + case "a .claude directory linked outside": + _tree.File("outside/dot/CLAUDE.md", "OUTSIDE"); + _tree.Link("ws/.claude", Outside("dot")); + break; + case "a .claude directory linked to an outside directory that holds nothing yet": + _tree.Directory("outside/empty"); + _tree.Link("ws/.claude", Outside("empty")); + break; + case "a chain of links that ends outside": + _tree.Link("ws/docs/real.md", _secret); + _tree.Link("ws/AGENTS.md", "docs/real.md"); + _tree.Link("ws/CLAUDE.md", "AGENTS.md"); + break; + case "a rule linked outside": + _tree.Link("ws/.claude/rules/style.md", _secret); + break; + case "a rules folder linked outside": + _tree.File("outside/rules/a.md", "OUTSIDE"); + _tree.Link("ws/.claude/rules/team", Outside("rules")); + break; + case "a link whose .. climbs out of a linked directory": + // deep → outside/p/q, so deep/../x.md is outside/p/x.md — not the x.md beside deep, which exists too. + _tree.Directory("outside/p/q"); + _tree.File("outside/p/x.md", "OUTSIDE"); + _tree.File("ws/x.md", "INSIDE"); + _tree.Link("ws/deep", Outside("p/q")); + _tree.Link("ws/CLAUDE.md", "deep/../x.md"); + break; + case "an import through an in-workspace link": + _tree.File("ws/CLAUDE.md", "Read @docs/guide.md first.\n"); + _tree.File("ws/docs/guide.md", "Then read @notes.md\n"); + _tree.Link("ws/docs/notes.md", _secret); + break; + case "an import of an outside file by its absolute path": + _tree.File("ws/CLAUDE.md", $"@{_secret}\n"); + break; + case "an import that climbs out with ..": + _tree.File("ws/.claude/CLAUDE.md", "@../../outside/secret.md\n"); + break; + case "an import inside a code block": + _tree.File("ws/CLAUDE.md", "```\n@docs/n.md\n```\n"); + _tree.Link("ws/docs/n.md", _secret); + break; + case "an import a scoped rule makes": + _tree.File("ws/.claude/rules/scoped.md", "---\npaths:\n - \"src/**\"\n---\nSee @../../docs/x.md\n"); + _tree.Link("ws/docs/x.md", _secret); + break; + case "an import with an escaped space": + _tree.File("ws/CLAUDE.md", "@my\\ notes.md\n"); + _tree.Link("ws/my notes.md", _secret); + break; + case "an import with a fragment": + _tree.File("ws/CLAUDE.md", "@notes.md#setup\n"); + _tree.Link("ws/notes.md", _secret); + break; + default: + throw new ArgumentOutOfRangeException(nameof(shape), shape, null); + } + } + + private void PlantKept(string shape) + { + switch (shape) + { + case "no memory at all": + break; + case "a CLAUDE.md linked to the AGENTS.md beside it": + _tree.File("ws/AGENTS.md", "Follow @docs/style.md.\n"); + _tree.File("ws/docs/style.md", "Tabs.\n"); + _tree.Link("ws/CLAUDE.md", "AGENTS.md"); + break; + case "a .claude directory linked to another inside the workspace": + _tree.File("ws/shared/CLAUDE.md", "Shared.\n"); + _tree.File("ws/shared/rules/r.md", "Rule.\n"); + _tree.Link("ws/.claude", Path.Combine(_workspace, "shared")); + break; + case "a CLAUDE.md linked to nothing": + _tree.Link("ws/CLAUDE.md", Outside("missing.md")); + break; + case "a CLAUDE.md linked to itself": + _tree.Link("ws/CLAUDE.md", "CLAUDE.md"); + break; + case "imports of files that do not exist": + _tree.File("ws/CLAUDE.md", "@nowhere.md and @also/missing.md\n"); + break; + case "imports naming directories": + _tree.Directory("ws/docs"); + _tree.File("ws/CLAUDE.md", "@/ @.. @../ @docs\n"); + break; + case "an import of the run's home": + _tree.File("ws/CLAUDE.md", "@~/.mcp.json\n"); + break; + case "addresses and handles": + _tree.File("ws/CLAUDE.md", "Mail ops@example.com or ping @platform-team.\n"); + break; + case "a file in the rules folder that is not markdown": + _tree.File("ws/.claude/rules/notes.txt", "@../../../outside/secret.md\n"); + break; + case "an image in the rules folder linked outside": + _tree.File("ws/.claude/rules/style.md", "Rule.\n"); + _tree.Link("ws/.claude/rules/diagram.png", _secret); + break; + case "70 rules scoped by paths:": + for (var i = 0; i < 70; i++) _tree.File($"ws/.claude/rules/rule-{i}.md", $"---\npaths:\n - \"src/{i}/**\"\n---\nRule {i}.\n"); + break; + case "5 rules beside 60 images": + for (var i = 0; i < 5; i++) _tree.File($"ws/.claude/rules/rule-{i}.md", $"Rule {i}.\n"); + for (var i = 0; i < 60; i++) _tree.File($"ws/.claude/rules/img/figure-{i}.png", "PNG"); + break; + case "a 200 KiB CLAUDE.md": + _tree.File("ws/CLAUDE.md", Prose(200 * 1024)); + break; + case "a CLAUDE.md importing a 200 KiB README": + _tree.File("ws/CLAUDE.md", "Project overview: @README.md\n"); + _tree.File("ws/README.md", Prose(200 * 1024)); + break; + case "1100 email addresses": + _tree.File("ws/CLAUDE.md", string.Concat(Enumerable.Range(0, 1100).Select(i => $"owner{i}@corp{i}.example.com\n"))); + break; + default: + throw new ArgumentOutOfRangeException(nameof(shape), shape, null); + } + } + + /// Ordinary text of about bytes, words and lines, no imports. + private static string Prose(int bytes) => string.Concat(Enumerable.Repeat("Keep each change small and tested.\n", bytes / 35 + 1)); + + private static async Task MakeFifoAsync(string path) + { + using var mkfifo = Process.Start("mkfifo", path)!; + await mkfifo.WaitForExitAsync(); + mkfifo.ExitCode.ShouldBe(0, $"fixture check: mkfifo {path}"); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/PhysicalPathTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/PhysicalPathTests.cs new file mode 100644 index 000000000..afbf46bc9 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/PhysicalPathTests.cs @@ -0,0 +1,119 @@ +using CodeSpace.Core.Services.Agents.Harnesses; +using Shouldly; + +namespace CodeSpace.UnitTests.Workflows; + +/// +/// Pins : where the kernel really lands when it opens a path. +/// keeps its Codex distrust coverage in CodexHarnessTests; these pin , which the +/// Claude memory guard decides containment by, and . +/// +[Trait("Category", "Unit")] +public sealed class PhysicalPathTests +{ + [Theory] + [InlineData("/w/ws", "/w/ws", true)] // the root itself + [InlineData("/w/ws", "/w/ws/a/b.md", true)] // below it + [InlineData("/w/ws/", "/w/ws/a.md", true)] // a root spelled with its trailing separator + [InlineData("/w/ws", "/w/ws2/a.md", false)] // a sibling that shares its prefix + [InlineData("/w/ws", "/w/a.md", false)] // above it + [InlineData("/w/ws", "/w", false)] // its parent + [InlineData("/", "/etc/passwd", true)] // the filesystem root holds everything + public void StaysInside_is_the_root_or_below_it(string root, string path, bool inside) => + PhysicalPath.StaysInside(root, path).ShouldBe(inside); + + [Fact] + public void A_chain_of_links_resolves_to_where_the_last_one_points() + { + if (OperatingSystem.IsWindows()) return; + + using var tree = new TempTree(); + var target = tree.File("real/target.md", "x"); + tree.Link("a/second.md", "../real/target.md"); + var first = tree.Link("first.md", "a/second.md"); + + PhysicalPath.File(first).ShouldBe(PhysicalPath.File(target)); + PhysicalPath.File(target).ShouldBe(Path.Combine(PhysicalPath.Directory(tree.Root), "real", "target.md"), "a plain file is where its directory really is"); + } + + [Fact] + public void A_dotdot_in_a_link_target_climbs_from_where_the_link_really_is() + { + // deep → outside/p/q, so deep/../x.md is outside/p/x.md. Read as text it would be the x.md beside deep, which + // exists too — the trap a lexical resolve falls into. + if (OperatingSystem.IsWindows()) return; + + using var tree = new TempTree(); + tree.Directory("outside/p/q"); + var outside = tree.File("outside/p/x.md", "OUTSIDE"); + tree.File("ws/x.md", "INSIDE"); + tree.Link("ws/deep", Path.Combine(tree.Root, "outside", "p", "q")); + var link = tree.Link("ws/CLAUDE.md", "deep/../x.md"); + + PhysicalPath.File(link).ShouldBe(PhysicalPath.File(outside)); + } + + [Fact] + public void A_dotdot_after_a_linked_component_of_a_link_target_climbs_from_where_that_component_really_is() + { + // data → /srv/link/../disk with srv/link → x/y: the kernel takes srv/link to x/y and climbs from there, so + // data/ws is x/disk/ws. Read as text, the target would be srv/disk, which does not exist. + if (OperatingSystem.IsWindows()) return; + + using var tree = new TempTree(); + tree.Directory("x/y"); + var real = tree.Directory("x/disk/ws"); + tree.Link("srv/link", Path.Combine(tree.Root, "x", "y")); + tree.Link("data", Path.Combine(tree.Root, "srv", "link", "..", "disk")); + + PhysicalPath.File(Path.Combine(tree.Root, "data", "ws")).ShouldBe(PhysicalPath.File(real)); + PhysicalPath.File(real).ShouldBe(Path.Combine(PhysicalPath.Directory(tree.Root), "x", "disk", "ws")); + } + + [Fact] + public void A_link_that_dangles_names_nothing() + { + if (OperatingSystem.IsWindows()) return; + + using var tree = new TempTree(); + var link = tree.Link("CLAUDE.md", Path.Combine(tree.Root, "missing.md")); + + PhysicalPath.File(link).ShouldBeNull(); + PhysicalPath.File(Path.Combine(tree.Root, "never-written.md")).ShouldBeNull(); + } + + [Fact] + public void A_link_that_loops_names_nothing() + { + if (OperatingSystem.IsWindows()) return; + + using var tree = new TempTree(); + tree.Link("b.md", "a.md"); + var a = tree.Link("a.md", "b.md"); + + PhysicalPath.File(a).ShouldBeNull("the kernel refuses a loop with ELOOP, so it reaches no file"); + } + + [Fact] + public void A_path_through_a_file_names_nothing() + { + if (OperatingSystem.IsWindows()) return; + + using var tree = new TempTree(); + var file = tree.File("plain.md", "x"); + + PhysicalPath.File(Path.Combine(file, "CLAUDE.md")).ShouldBeNull("the kernel refuses a file used as a directory with ENOTDIR"); + } + + [Fact] + public void A_directory_resolves_like_a_file() + { + if (OperatingSystem.IsWindows()) return; + + using var tree = new TempTree(); + var real = tree.Directory("real"); + var link = tree.Link("link", real); + + PhysicalPath.File(link).ShouldBe(PhysicalPath.Directory(real)); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/TempTree.cs b/backend/tests/CodeSpace.UnitTests/Workflows/TempTree.cs new file mode 100644 index 000000000..a829e98eb --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/TempTree.cs @@ -0,0 +1,48 @@ +namespace CodeSpace.UnitTests.Workflows; + +/// +/// A GUID-named directory tree under the temp path, deleted on dispose, for tests that lay out real files and links. +/// is spelled through a symlink to the real directory and never resolved, as production never +/// resolves a workspace path, so every host exercises a workspace reached through a link — not only macOS, whose temp +/// path already runs through /var. Windows, where these tests do not run, gets the real directory. +/// +internal sealed class TempTree : IDisposable +{ + private readonly string _real = Path.Combine(Path.GetTempPath(), $"cs-tree-{Guid.NewGuid():N}"); + + public TempTree() + { + System.IO.Directory.CreateDirectory(_real); + Root = OperatingSystem.IsWindows() ? _real : System.IO.File.CreateSymbolicLink($"{_real}-link", _real).FullName; + } + + /// The tree's root, through its link. + public string Root { get; } + + /// The directory at below the root, created with its parents. + public string Directory(string relative) => System.IO.Directory.CreateDirectory(Path.Combine(Root, relative)).FullName; + + /// The file at below the root, written with and its parents created. + public string File(string relative, string content) + { + var path = Path.Combine(Root, relative); + System.IO.Directory.CreateDirectory(Path.GetDirectoryName(path)!); + System.IO.File.WriteAllText(path, content); + return path; + } + + /// A symlink at below the root whose target is exactly as given — absolute, or relative to the link's own directory. + public string Link(string relative, string target) + { + var path = Path.Combine(Root, relative); + System.IO.Directory.CreateDirectory(Path.GetDirectoryName(path)!); + System.IO.File.CreateSymbolicLink(path, target); + return path; + } + + public void Dispose() + { + try { if (Root != _real) System.IO.File.Delete(Root); } catch { /* best-effort cleanup of a temp link */ } + try { System.IO.Directory.Delete(_real, recursive: true); } catch { /* best-effort cleanup of a temp directory */ } + } +}