From 649b2189b24a6ef0b77e389cc92f6569ec6f55e3 Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Wed, 7 Oct 2026 16:52:11 +0800 Subject: [PATCH] Carry clone tokens in the environment, never on a git argv Every tokened platform git command put the token in the remote URL on its argv. Any host user can read that from /proc//cmdline unless /proc hides other users' processes, git wrote it into .git/config for the length of a clone, and git answered a 401 from another authority with it: a remote that redirected its ref advertisement to another host was handed the token. TokenedGitCommand now takes the credential separately. A tokened command names the remote by its URL without userinfo. GIT_CONFIG_COUNT carries two entries scoped to the remote's scheme and authority: an empty credential helper that resets the operator's helpers, then an env-only helper that answers get from CODESPACE_GIT_USERNAME and CODESPACE_GIT_PASSWORD with the shell's builtin printf. A redirect to another authority finds no helper that answers for it, so the command fails instead of sending the token. The -c reset leaves argv: git reads GIT_CONFIG_PARAMETERS after GIT_CONFIG_COUNT, so it would wipe the helper. Auto-gc and auto-maintenance are off so no detached child keeps the environment, and trace2 stays off. git-lfs answers a 401 by erasing the credential and asking the helpers again, with no limit, so a helper that always answers keeps a refused token (expired or revoked) retrying until the command times out, tens of failed logins a second. The helper records an erase as a marker in a per-command owner-only directory the runner makes and removes (ConfigHomeEnvVars), then answers nothing and says the remote refused the credential. Without that directory it never answers. With the token out of the URL, an operator's url..insteadOf or pushInsteadOf rule matches a tokened remote for the first time, and could move it to SSH under the host's key or to another credential. Two more entries map the remote's URL to itself, the longest prefix any rule can match, so a tokened command reaches its remote as it always did. The workspace clone, soft-ref probe, pin fetch rungs and publish (lfs push, push, readback), the launch-base ls-remote, the branch integrator, the acceptance grader and the pack import all go through it, and BuildAuthenticatedUrl is gone. The integrator's base checkout, apply and reset, and the grader's base checkout and apply, carry the credential too: origin no longer does, and git-lfs asks the helpers for the objects those commands download. Rewriting origin after a clone stays as a belt. --- .../Services/Agents/PackCloneFetcher.cs | 35 +- .../Integrators/LocalGitBranchIntegrator.cs | 42 +- .../Providers/LocalGitWorkspaceProvider.cs | 117 ++-- .../Agents/Workspace/RemoteTipResolver.cs | 30 +- .../Agents/Workspace/TokenedGitCommand.cs | 176 ++++-- .../Supervisor/SupervisorAcceptanceGrader.cs | 48 +- .../Agents/PackCloneCredentialFlowTests.cs | 95 ++-- .../Workflows/AgentWorkspacePushFlowTests.cs | 15 +- .../Workflows/GitPublishRemoteFixture.cs | 114 +++- .../LocalGitBranchIntegratorFlowTests.cs | 17 +- .../TokenedGitCredentialHelperFlowTests.cs | 520 +++++++++++++++--- .../Workflows/TokenedGitTestSupport.cs | 186 +++++++ .../Agents/LocalGitBranchIntegratorTests.cs | 30 +- .../Agents/PackCloneFetcherArgsTests.cs | 17 +- .../Agents/PackCloneFetcherCredentialTests.cs | 21 +- .../Agents/SupervisorAcceptanceGraderTests.cs | 31 +- .../CodeSpace.UnitTests/TokenedGitSpecs.cs | 100 +++- .../LocalGitWorkspaceProviderTests.cs | 77 +-- .../Workflows/RemoteTipResolverTests.cs | 9 +- .../Workflows/TokenedGitCommandTests.cs | 258 +++++++-- 20 files changed, 1478 insertions(+), 460 deletions(-) create mode 100644 backend/tests/CodeSpace.IntegrationTests/Workflows/TokenedGitTestSupport.cs diff --git a/backend/src/CodeSpace.Core/Services/Agents/PackCloneFetcher.cs b/backend/src/CodeSpace.Core/Services/Agents/PackCloneFetcher.cs index d9d7e65b2..02cbee064 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/PackCloneFetcher.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/PackCloneFetcher.cs @@ -19,11 +19,12 @@ namespace CodeSpace.Core.Services.Agents; /// crash-safety backstop: the recurring sweep (which fans out over every janitor) ages out a clone orphaned by a /// worker that died between clone and dispose. /// -/// A pasted URL can carry a credential in its userinfo (). The clone runs as a tokened command, -/// so the operator's credential helpers and trace2 targets never see it, in a directory only this worker's uid can read; -/// once cloned, origin is rewritten to the URL without it, so the checkout the import walks holds none; a clone failure names -/// the URL without it and redacts it from git's stderr, since that message reaches the API error body, the UI and the -/// mediator's error log. +/// A pasted URL can carry a credential in its userinfo (). The clone runs as a tokened command: +/// it names the remote without the credential and carries it in its environment, so no argv carries it, git writes none into +/// the checkout's origin, and the operator's credential helpers and trace2 targets never see it. The clone still runs in a +/// directory only this worker's uid can read, and origin is still rewritten to the URL without the credential once cloned, +/// as belts; a clone failure names the URL without it and redacts it from git's stderr, since that message reaches the API +/// error body, the UI and the mediator's error log. /// public sealed partial class PackCloneFetcher : IPackSourceFetcher, IWorkspaceJanitor, ISingletonDependency { @@ -74,9 +75,9 @@ public async Task FetchAsync(string url, string? reference, Cancel } /// - /// The clone's directory, readable by this worker's uid alone. git writes the pasted URL, credential included, into - /// .git/config before the transfer starts, and origin is stripped only once it ends — up to the clone timeout later, - /// or never when the worker dies mid-clone and leaves it to the janitor — so it is owner-only before git runs. + /// The clone's directory, readable by this worker's uid alone, before git runs — a belt: the clone names the remote + /// without the pasted credential, so git writes none into .git/config, and the checkout stays the import's + /// private copy until it is walked (or until the janitor reclaims it when the worker dies mid-clone). /// private static void CreateOwnerOnlyDirectory(string dir) { @@ -94,10 +95,10 @@ private async Task CloneAsync(string url, string? reference, string dir, Cancell } /// - /// git writes the pasted URL, credential included, into the clone's origin, and the import then walks that checkout (a - /// worker that dies mid-import leaves it on disk for the janitor). Rewrite origin to the URL without the credential through - /// the workspace provider's own strip: set-url, else remove origin, else a — and the - /// caller deletes the clone on the way out. + /// The import walks this checkout (a worker that dies mid-import leaves it on disk for the janitor), so its origin must + /// hold no credential. The clone already named the URL without it; as a belt, rewrite origin to that URL through the + /// workspace provider's own strip: set-url, else remove origin, else a — and the caller + /// deletes the clone on the way out. /// private async Task StripPastedCredentialAsync(string url, string dir, CancellationToken cancellationToken) { @@ -132,15 +133,15 @@ internal static IReadOnlyList BuildCloneArgs(string url, string? referen /// /// The clone as the runner gets it: in , with the network. A pasted URL - /// carrying a credential () clones as a , so no credential helper sees - /// it and no trace2 target records it — a token pasted as the user alone too, which carries no password for - /// to find, yet git hands it to every helper it asks for the missing one. + /// carrying a credential () clones as a : its argv names the URL + /// without the userinfo, and the whole userinfo travels in its environment — a token pasted as the user alone too, which + /// a stored URL's bare user would not, since that names an account. /// internal static SandboxSpec BuildCloneSpec(string url, string? reference, string dir) { - var spec = new SandboxSpec { Command = "git", Args = BuildCloneArgs(url, reference, dir), WorkingDirectory = dir, TimeoutSeconds = CloneTimeoutSeconds, AllowNetwork = true }; + var remote = PastedSecret(url) is null ? new TokenedGitCommand.Remote(url, null, null) : TokenedGitCommand.FromUserInfo(url); - return PastedSecret(url) is null ? spec : TokenedGitCommand.AsTokened(url, spec); + return TokenedGitCommand.Spec(remote, new SandboxSpec { Command = "git", Args = BuildCloneArgs(remote.Url, reference, dir), WorkingDirectory = dir, TimeoutSeconds = CloneTimeoutSeconds, AllowNetwork = true }); } // ── IWorkspaceJanitor: reclaim pack clones orphaned by a crashed worker ────────────────────────── diff --git a/backend/src/CodeSpace.Core/Services/Agents/Workspace/Integrators/LocalGitBranchIntegrator.cs b/backend/src/CodeSpace.Core/Services/Agents/Workspace/Integrators/LocalGitBranchIntegrator.cs index f6ca64c16..340ebfafe 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Workspace/Integrators/LocalGitBranchIntegrator.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Workspace/Integrators/LocalGitBranchIntegrator.cs @@ -23,12 +23,12 @@ namespace CodeSpace.Core.Services.Agents.Workspace.Integrators; /// resets the clone to base (no half-merge survives), pushes nothing, and returns a /// naming what could not be applied — the original K agent branches/patches remain intact for human review. /// -/// Secret hygiene is co-located with the provider: the clone embeds the token in the URL for the clone -/// command only and every surfaced git output is redacted (), and the -/// transient clone is always removed in a finally. The clone keeps its tokened origin, so the commands whose git -/// transport reaches it — the clone and the push — run as s. The base checkout, the -/// apply and the reset reach it only for LFS objects, which git-lfs authenticates from the URL without asking or telling -/// a credential helper; a full clone leaves them no git object to fetch through it. +/// Secret hygiene is co-located with the provider: the clone names the remote without its credential, so +/// origin never holds the token; every command that reaches origin runs as a , carrying it +/// in its environment — the clone and the push through git's transport, and the base checkout, the apply and the reset +/// through git-lfs, which asks the credential helpers for the LFS objects they download (a full clone leaves them no git +/// object to fetch). Every surfaced git output is redacted (), and the +/// transient clone is always removed in a finally. /// public sealed class LocalGitBranchIntegrator : IBranchIntegrator, IScopedDependency { @@ -182,11 +182,11 @@ private async Task CloneApplyAndPushAsync(IntegrationRequest var ordered = await InAncestryOrderAsync(directory, resolved, cancellationToken).ConfigureAwait(false); - var applyBlock = await ApplyAllAsync(directory, ordered, request.Token, cancellationToken).ConfigureAwait(false); + var applyBlock = await ApplyAllAsync(directory, ordered, request, cancellationToken).ConfigureAwait(false); if (applyBlock is not null) { - await ResetToBaseAsync(directory, request.BaseSha, cancellationToken).ConfigureAwait(false); + await ResetToBaseAsync(directory, request, cancellationToken).ConfigureAwait(false); return Aborted(resolved, applyBlock); } @@ -210,7 +210,7 @@ private async Task CloneAsync(IntegrationRequest request, string directory, Canc // A FULL clone (no --depth): a 3-way apply needs the base history the agents' shallow clones lacked, and a // full clone guarantees the recorded base SHA is present. (A --filter=blob:none partial clone is a deferred // optimisation — it needs remote allow-filter support a bare file:// remote can't give a test.) - var url = RemoteUrl(request); + var url = Remote(request).Url; Directory.CreateDirectory(directory); var result = await RunTokenedGitAsync(request, new[] { "clone", url, directory }, directory, cancellationToken).ConfigureAwait(false); @@ -222,7 +222,7 @@ private async Task CloneAsync(IntegrationRequest request, string directory, Canc /// Check out the EXACT shared base (detached) so every apply --3way resolves the pre-image against the commit the agents saw. False when the SHA is not in history (a bad base). private async Task CheckoutBaseAsync(string directory, IntegrationRequest request, CancellationToken cancellationToken) { - var result = await RunGitAsync(new[] { "-C", directory, "checkout", "--detach", request.BaseSha }, directory, cancellationToken).ConfigureAwait(false); + var result = await RunTokenedGitAsync(request, new[] { "-C", directory, "checkout", "--detach", request.BaseSha }, directory, cancellationToken).ConfigureAwait(false); return result.Status == SandboxStatus.Success; } @@ -291,7 +291,7 @@ private async Task IsStrictAncestorAsync(string directory, string ancestor } /// Apply each clean (preflight-passed) contribution in order. Returns a set-level abort reason on the FIRST textual conflict (marking the rest not-attempted), else null when all applied. - private async Task ApplyAllAsync(string directory, IReadOnlyList resolved, string? token, CancellationToken cancellationToken) + private async Task ApplyAllAsync(string directory, IReadOnlyList resolved, IntegrationRequest request, CancellationToken cancellationToken) { for (var i = 0; i < resolved.Count; i++) { @@ -299,13 +299,13 @@ private async Task IsStrictAncestorAsync(string directory, string ancestor if (string.IsNullOrWhiteSpace(r.Patch)) continue; // a true no-op (base matched, empty diff) — nothing to apply - var (applied, stderr) = await TryApplyAsync(directory, r, cancellationToken).ConfigureAwait(false); + var (applied, stderr) = await TryApplyAsync(directory, r, request, cancellationToken).ConfigureAwait(false); if (applied) continue; var conflictedFiles = await ReadConflictedFilesAsync(directory, r, cancellationToken).ConfigureAwait(false); - r.Conflict(ConflictReason(stderr, directory, token), conflictedFiles); + r.Conflict(ConflictReason(stderr, directory, request.Token), conflictedFiles); for (var j = i + 1; j < resolved.Count; j++) resolved[j].Skip("not integrated — an earlier contribution conflicted"); @@ -315,7 +315,7 @@ private async Task IsStrictAncestorAsync(string directory, string ancestor return null; } - private async Task<(bool Success, string Stderr)> TryApplyAsync(string directory, ResolvedContribution r, CancellationToken cancellationToken) + private async Task<(bool Success, string Stderr)> TryApplyAsync(string directory, ResolvedContribution r, IntegrationRequest request, CancellationToken cancellationToken) { var patchFile = Path.Combine(directory, ".codespace-integrate.patch"); await File.WriteAllTextAsync(patchFile, r.Patch, cancellationToken).ConfigureAwait(false); @@ -326,7 +326,7 @@ private async Task IsStrictAncestorAsync(string directory, string ancestor // this contribution's own base (the normal case once an upstream contribution has already been applied // under it) does git reconstruct the pre-image blobs and 3-way merge. A failure here is a GENUINE textual // conflict — which the caller must keep surfacing as Conflicted, since the resolve arc acts on it. - var result = await RunGitAsync(new[] { "-C", directory, "apply", "--index", "--3way", patchFile }, directory, cancellationToken).ConfigureAwait(false); + var result = await RunTokenedGitAsync(request, new[] { "-C", directory, "apply", "--index", "--3way", patchFile }, directory, cancellationToken).ConfigureAwait(false); return (result.Status == SandboxStatus.Success, result.Stderr); } finally @@ -460,17 +460,17 @@ private async Task CommitAsync(string directory, int count, CancellationToken ca throw new WorkspaceException($"git push failed (exit {result.ExitCode}): {LocalGitWorkspaceProvider.Redact(Summarize(result.Stderr), request.Token)}"); } - private async Task ResetToBaseAsync(string directory, string baseSha, CancellationToken cancellationToken) + private async Task ResetToBaseAsync(string directory, IntegrationRequest request, CancellationToken cancellationToken) { // Restore the clone to a pristine base tree so NO half-merged / conflict-marked state survives the abort. - await RunGitAsync(new[] { "-C", directory, "reset", "--hard", baseSha }, directory, cancellationToken).ConfigureAwait(false); + await RunTokenedGitAsync(request, new[] { "-C", directory, "reset", "--hard", request.BaseSha }, directory, cancellationToken).ConfigureAwait(false); await RunGitAsync(new[] { "-C", directory, "clean", "-fd" }, directory, cancellationToken).ConfigureAwait(false); } // ── Small git helpers ──────────────────────────────────────────────────────────── - /// The remote the integration clone reaches: the authed URL the clone names, which origin keeps to the end. - private static string RemoteUrl(IntegrationRequest request) => LocalGitWorkspaceProvider.BuildAuthenticatedUrl(request.RepositoryUrl, request.TokenUsername, request.Token); + /// The remote the integration clone reaches: named by the clone and held by origin without its credential, which the commands that reach it carry. + private static TokenedGitCommand.Remote Remote(IntegrationRequest request) => TokenedGitCommand.RemoteFor(request.RepositoryUrl, request.TokenUsername, request.Token); private async Task HasStagedChangesAsync(string directory, CancellationToken cancellationToken) { @@ -493,9 +493,9 @@ private async Task RevParseTreeAsync(string directory, string rev, Cance private Task RunGitAsync(IReadOnlyList args, string? workingDirectory, CancellationToken cancellationToken) => RunSpecAsync(GitSpec(args, workingDirectory), cancellationToken); - /// Run a command whose git transport reaches the integration clone's tokened origin — the clone, the push — as a . + /// Run a command that reaches the integration clone's origin — the clone and the push through git's transport, the checkout, the apply and the reset through git-lfs — as a . private Task RunTokenedGitAsync(IntegrationRequest request, IReadOnlyList args, string directory, CancellationToken cancellationToken) => - RunSpecAsync(TokenedGitCommand.Spec(RemoteUrl(request), GitSpec(args, directory)), cancellationToken); + RunSpecAsync(TokenedGitCommand.Spec(Remote(request), GitSpec(args, directory)), cancellationToken); private static SandboxSpec GitSpec(IReadOnlyList args, string? workingDirectory) => new() { Command = "git", Args = args, WorkingDirectory = workingDirectory, TimeoutSeconds = GitTimeoutSeconds, AllowNetwork = true }; diff --git a/backend/src/CodeSpace.Core/Services/Agents/Workspace/Providers/LocalGitWorkspaceProvider.cs b/backend/src/CodeSpace.Core/Services/Agents/Workspace/Providers/LocalGitWorkspaceProvider.cs index 582c17ad1..1555686f1 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Workspace/Providers/LocalGitWorkspaceProvider.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Workspace/Providers/LocalGitWorkspaceProvider.cs @@ -14,13 +14,12 @@ namespace CodeSpace.Core.Services.Agents.Workspace.Providers; /// LocalProcessRunner — both "local". A future K8s provider clones into the /// pod volume behind the same contract. /// -/// Secret hygiene: the access token is embedded in the clone URL for the clone command -/// only, then the origin remote is rewritten to the tokenless URL so the persisted .git/config -/// never retains it, and any token text is redacted from surfaced error output. Every command that can -/// reach the tokened remote runs as a , so an operator's store or -/// cache helper never keeps the token and no trace2 target records it. (The transient argv -/// exposure is acceptable on a single-tenant local worker; the K8s runner injects via an in-pod -/// credential helper instead.) +/// Secret hygiene: every command that reaches the remote runs as a : it names +/// the remote by its URL without userinfo and carries the access token in its environment, for a credential helper scoped +/// to that remote. So no argv carries the token, the clone's .git/config never holds it (rewriting origin to the +/// same URL after the clone stays as a belt), an operator's store or cache helper never keeps it, no trace2 +/// target records it, and a redirect to another authority is never sent it. Any token text is still redacted from +/// surfaced error output. /// public sealed class LocalGitWorkspaceProvider : IWorkspaceProvider, IWorkspaceJanitor, IWorkspacePathCapture, ISingletonDependency { @@ -156,17 +155,17 @@ internal static bool IsLfsObjectPath(string relative) => /// Clone one repo, strip its token from the persisted remote, and read its base revision — the per-repo unit of the workspace. private async Task MaterializeAsync(WorkspaceRepositoryProvision repo, string directory, CancellationToken cancellationToken) { - var context = new RepositoryCommandContext(directory, ResolveLocalSourcePaths(repo.CloneRequest), BuildAuthenticatedUrl(repo.CloneRequest.RepositoryUrl, repo.CloneRequest.TokenUsername, repo.CloneRequest.Token)); + var context = new RepositoryCommandContext(directory, ResolveLocalSourcePaths(repo.CloneRequest), TokenedGitCommand.RemoteFor(repo.CloneRequest.RepositoryUrl, repo.CloneRequest.TokenUsername, repo.CloneRequest.Token)); Directory.CreateDirectory(directory); await CloneAsync(repo.CloneRequest, context, cancellationToken).ConfigureAwait(false); - if (!string.IsNullOrEmpty(repo.CloneRequest.Token)) - await StripTokenFromRemoteAsync(repo.CloneRequest.RepositoryUrl, directory, cancellationToken).ConfigureAwait(false); + if (context.Remote.IsTokened) + await StripTokenFromRemoteAsync(context.Remote.Url, directory, cancellationToken).ConfigureAwait(false); var baseSha = await ReadBaseShaAsync(context, cancellationToken).ConfigureAwait(false); - // Carry the SAME short-lived clone credential forward (in-memory only, never persisted / never in .git/config — - // origin was stripped) so a later push re-injects auth into the push argv without a second auth round-trip. + // Carry the SAME short-lived clone credential forward (in-memory only, never persisted / never in .git/config) so a + // later push carries it in the push's environment without a second auth round-trip. return new MaterializedRepo(repo.Alias, directory, repo.Access, repo.CloneRequest.RepositoryUrl, repo.CloneRequest.TokenUsername, repo.CloneRequest.Token, baseSha, repo.CloneRequest.Ref) { ReadOnlyPaths = context.ReadOnlyPaths }; } @@ -327,7 +326,7 @@ private static void TryDeleteDirectory(string directory) private async Task CloneAsync(WorkspaceRequest request, RepositoryCommandContext context, CancellationToken cancellationToken) { var directory = context.Directory; - var url = context.RemoteUrl; + var url = context.Remote.Url; var (checkoutRef, softRefFellBack, remoteTip) = await ResolveCheckoutRefAsync(request, url, context, cancellationToken).ConfigureAwait(false); @@ -520,12 +519,12 @@ private async Task CommitExistsLocallyAsync(RepositoryCommandContext conte /// pins the fail-closed behaviour asserts THIS symbol rather than re-typing the sentence (which would let the two /// drift until the test passes on a message nobody emits). /// - internal const string TokenStripFailedDetail = "Could not strip or remove the tokened origin remote, so the clone may still carry the credential in .git/config; refusing to hand this workspace to an agent"; + internal const string TokenStripFailedDetail = "Could not strip or remove the tokened origin remote; the clone named the remote without its credential, so origin should hold none, but a failed strip is refused rather than trusted: refusing to hand this workspace to an agent"; /// - /// Rewrite origin to the tokenless URL so the cloned .git/config never persists credentials. - /// If the rewrite fails, REMOVE the origin remote outright — the persisted config carrying a token is - /// the credential-leak we must close, and the run captures changes via the local diff (not origin), so + /// Rewrite origin to the tokenless URL so the cloned .git/config never persists credentials — a belt now that the + /// clone names that URL itself. If the rewrite fails, REMOVE the origin remote outright — the persisted config carrying a + /// token is the credential-leak we must close, and the run captures changes via the local diff (not origin), so /// dropping origin is safe. When BOTH fail the clone is FAIL-CLOSED (see the shared implementation). /// private Task StripTokenFromRemoteAsync(string cleanUrl, string directory, CancellationToken cancellationToken) => @@ -533,8 +532,8 @@ private Task StripTokenFromRemoteAsync(string cleanUrl, string directory, Cancel /// /// The SHARED implementation of — - /// internal static (like /) so any OTHER caller that - /// clones an authenticated URL directly (bypassing this provider's own , e.g. + /// internal static (like ) so any OTHER caller that clones a tokened remote directly + /// (bypassing this provider's own , e.g. /// SupervisorAcceptanceGrader.CloneAtBaseAsync, which must clone at an arbitrary base SHA rather than a /// named ref) reuses the EXACT same strip-then-fallback-to-remove logic — a security-sensitive path must have /// exactly one implementation, never two copies that can silently drift apart. @@ -548,7 +547,7 @@ private Task StripTokenFromRemoteAsync(string cleanUrl, string directory, Cancel /// (PrepareAsync's catch, the grader's finally), so failing here both withholds the credential and /// destroys it. A run that never starts is the cheap outcome; a token an agent can exfiltrate is not. /// - /// Neither remote set-url nor remote remove succeeded — the token may still be in .git/config. + /// Neither remote set-url nor remote remove succeeded — refused fail-closed, though a clone that named the remote without its credential holds none in .git/config. internal static async Task StripTokenFromRemoteAsync(ISandboxRunner runner, int timeoutSeconds, ILogger logger, string cleanUrl, string directory, CancellationToken cancellationToken) { Task RunGitAsync(IReadOnlyList args) => @@ -566,7 +565,7 @@ Task RunGitAsync(IReadOnlyList args) => return; } - logger.LogError("Could not strip OR remove the tokened origin (set-url exit {SetExit}, remove exit {RemoveExit}); refusing the clone so no agent reads the credential out of .git/config", rewrite.ExitCode, remove.ExitCode); + logger.LogError("Could not strip OR remove the tokened origin (set-url exit {SetExit}, remove exit {RemoveExit}); origin should hold no credential, but refusing the clone rather than trust a strip that failed", rewrite.ExitCode, remove.ExitCode); throw new WorkspaceException($"{TokenStripFailedDetail} (set-url exit {rewrite.ExitCode}, remove exit {remove.ExitCode})."); } @@ -577,14 +576,15 @@ Task RunGitAsync(IReadOnlyList args) => /// commands that DO reach the remote (clone, fetch, push) as well as the local ones, so a single severed helper /// would break materialization on any runner that enforces it. The value is the egress they have always had — /// each command still uses the runner's filesystem isolation with its explicit workspace and source mounts. - /// Every command here runs before the token strip, while is in - /// reach, so a tokened clone runs each of them as a . + /// Every command here can reach — the probe and the clone name it, the + /// pin's fetch rungs and checkout reach it through origin, git-lfs's downloads included — so a tokened clone runs each + /// of them as a . /// private Task RunGitAsync(IReadOnlyList args, RepositoryCommandContext context, CancellationToken cancellationToken) => - _runners.Resolve(Kind).RunAsync(TokenedGitCommand.Spec(context.RemoteUrl, new SandboxSpec { Command = "git", Args = args, WorkingDirectory = context.Directory, ReadOnlyPaths = context.ReadOnlyPaths, TimeoutSeconds = CloneTimeoutSeconds, AllowNetwork = true }), cancellationToken); + _runners.Resolve(Kind).RunAsync(TokenedGitCommand.Spec(context.Remote, new SandboxSpec { Command = "git", Args = args, WorkingDirectory = context.Directory, ReadOnlyPaths = context.ReadOnlyPaths, TimeoutSeconds = CloneTimeoutSeconds, AllowNetwork = true }), cancellationToken); - /// One clone's commands: its directory, its read-only source mounts, and the remote they can reach — the authed URL the probe and clone name, and origin holds until the strip. - private sealed record RepositoryCommandContext(string Directory, IReadOnlyList ReadOnlyPaths, string RemoteUrl); + /// One clone's commands: its directory, its read-only source mounts, and the remote they can reach — named by the probe and the clone, and held by origin. + private sealed record RepositoryCommandContext(string Directory, IReadOnlyList ReadOnlyPaths, TokenedGitCommand.Remote Remote); private static IReadOnlyList ResolveLocalSourcePaths(WorkspaceRequest request) { @@ -609,27 +609,15 @@ private static IReadOnlyList ResolveLocalSourcePaths(WorkspaceRequest re return new[] { approved }; } - /// Build the HTTPS clone URL with embedded basic-auth credentials. No token → the URL unchanged. Pure + internal so it's unit-pinned. - internal static string BuildAuthenticatedUrl(string repositoryUrl, string? tokenUsername, string? token) - { - if (string.IsNullOrEmpty(token)) return repositoryUrl; - - var uri = new Uri(repositoryUrl); - var user = Uri.EscapeDataString(string.IsNullOrEmpty(tokenUsername) ? "x-access-token" : tokenUsername); - var pass = Uri.EscapeDataString(token); - - return $"{uri.Scheme}://{user}:{pass}@{uri.Authority}{uri.PathAndQuery}"; - } - private static string Summarize(string stderr) => string.IsNullOrWhiteSpace(stderr) ? "(no stderr)" : stderr.Trim().Replace("\n", " "); /// /// Strip any echoed token from surfaced output so it never reaches a log / exception message. Redacts BOTH the raw - /// token AND its percent-encoded form, because embeds Uri.EscapeDataString(token) - /// in the push argv — a token with URL-special characters (@ / + = %) appears ENCODED in a failing push command, so - /// redacting only the raw literal would leak the reversible encoded form. Internal so the LocalGitBranchIntegrator - /// reuses the SAME redaction over its own git output (co-located secret hygiene). + /// token AND its percent-encoded form: no argv carries the token, but git or a remote can still echo it, and a token + /// with URL-special characters (@ / + = %) may come back ENCODED, so redacting only the raw literal would leak the + /// reversible encoded form. Internal so the LocalGitBranchIntegrator reuses the SAME redaction over its own git + /// output (co-located secret hygiene). /// internal static string Redact(string text, string? token) { @@ -776,24 +764,25 @@ private async Task CaptureRepoChangesAsync(MaterializedRepo re await ImportBundlesAsync(repo, publishDir, branchName, addsObjects, cancellationToken).ConfigureAwait(false); - // Re-inject the SAME clone credential into the ARGV only (never a remote, never .git/config). Plain --force, - // not --force-with-lease: an observe-then-lease would still admit a zombie whose observation is fresh at push - // time, so the zombie fence lives in the REF NAME instead (AgentRunExecutor.BuildBranchName is - // generation-specific — a superseded attempt cannot name the current attempt's ref), and a lease's - // no-remote-tracking-ref semantics vary by git version. Bounded timeout so a hung push can't delay completion. - var authedUrl = BuildAuthenticatedUrl(repo.RepositoryUrl, repo.TokenUsername, repo.Token); + // Carry the SAME clone credential in the environment of the commands that reach the remote (never an argv, + // never a remote, never .git/config). Plain --force, not --force-with-lease: an observe-then-lease would still + // admit a zombie whose observation is fresh at push time, so the zombie fence lives in the REF NAME instead + // (AgentRunExecutor.BuildBranchName is generation-specific — a superseded attempt cannot name the current + // attempt's ref), and a lease's no-remote-tracking-ref semantics vary by git version. Bounded timeout so a + // hung push can't delay completion. + var remote = TokenedGitCommand.RemoteFor(repo.RepositoryUrl, repo.TokenUsername, repo.Token); // LFS blobs BEFORE the refs (git's own pre-push order), so the remote never holds a pointer whose object is // missing. The clean repo has no working-tree .lfsconfig (nothing is checked out), and the endpoint comes - // from the explicit authed URL — a hostile committed .lfsconfig cannot redirect the upload. Lock verification + // from the explicit remote URL — a hostile committed .lfsconfig cannot redirect the upload. Lock verification // is off: against a remote without the locks API git-lfs would otherwise record lfs..locksverify in the - // publish repo's .git/config, keyed by the authed URL, which would put the token on disk. + // publish repo's .git/config. if (hasLfs) - await RunTokenedPublishGitOrThrowAsync(repo, publishDir, authedUrl, new[] { "-c", "lfs.locksverify=false", "lfs", "push", authedUrl, branchName }, cancellationToken).ConfigureAwait(false); + await RunTokenedPublishGitOrThrowAsync(repo, publishDir, remote, new[] { "-c", "lfs.locksverify=false", "lfs", "push", remote.Url, branchName }, cancellationToken).ConfigureAwait(false); - await RunTokenedPublishGitOrThrowAsync(repo, publishDir, authedUrl, new[] { "push", "--force", authedUrl, $"{branchName}:{branchName}" }, cancellationToken).ConfigureAwait(false); + await RunTokenedPublishGitOrThrowAsync(repo, publishDir, remote, new[] { "push", "--force", remote.Url, $"{branchName}:{branchName}" }, cancellationToken).ConfigureAwait(false); - repo.PushedCommitSha = await ReadBackPushedShaAsync(repo, publishDir, authedUrl, branchName, cancellationToken).ConfigureAwait(false); + repo.PushedCommitSha = await ReadBackPushedShaAsync(repo, publishDir, remote, branchName, cancellationToken).ConfigureAwait(false); return branchName; } @@ -856,13 +845,13 @@ private async Task ImportBundlesAsync(MaterializedRepo repo, string publishDir, /// by design: an unreadable remote or a mismatched tip (raced) returns null with a warning — the push itself /// already succeeded, so the branch stands; only the CONFIRMATION is withheld, never fabricated. /// - private async Task ReadBackPushedShaAsync(MaterializedRepo repo, string publishDir, string authedUrl, string branchName, CancellationToken cancellationToken) + private async Task ReadBackPushedShaAsync(MaterializedRepo repo, string publishDir, TokenedGitCommand.Remote remote, string branchName, CancellationToken cancellationToken) { try { var localTip = (await RunPublishGitOrThrowAsync(repo, publishDir, new[] { "rev-parse", $"refs/heads/{branchName}" }, cancellationToken, network: false).ConfigureAwait(false)).Trim(); - var readback = await RunTokenedPublishGitAsync(repo, publishDir, authedUrl, new[] { "ls-remote", authedUrl, $"refs/heads/{branchName}" }, cancellationToken).ConfigureAwait(false); + var readback = await RunTokenedPublishGitAsync(repo, publishDir, remote, new[] { "ls-remote", remote.Url, $"refs/heads/{branchName}" }, cancellationToken).ConfigureAwait(false); if (readback.Status != SandboxStatus.Success || readback.ExitCode != 0) { @@ -934,8 +923,8 @@ private Task RunAgentCloneBundleAsync(MaterializedRepo repo, string publishDir, // ── Commands over the platform-owned publish repo (init, fetch, lfs push, push, rev-parse, ls-remote) ── // A fresh repo outside the workspace, never touched by the agent. Only the commands that reach the remote carry the - // credential (in the argv) and the network, and they run as tokened commands (TokenedGitCommand), so no helper keeps - // it and no trace2 target records it; the credential never meets the agent-writable .git. + // credential (in the environment) and the network, and they run as tokened commands (TokenedGitCommand), so no argv + // carries it, no helper keeps it and no trace2 target records it; the credential never meets the agent-writable .git. private Task RunPublishGitOrThrowAsync(MaterializedRepo repo, string publishDir, IReadOnlyList args, CancellationToken cancellationToken, bool network, int timeoutSeconds = CaptureTimeoutSeconds) => EnsureSuccessAsync(repo, args, RunPublishGitAsync(repo, publishDir, args, cancellationToken, network, timeoutSeconds)); @@ -944,12 +933,12 @@ private Task RunPublishGitOrThrowAsync(MaterializedRepo repo, string pub private Task RunPublishGitAsync(MaterializedRepo repo, string publishDir, IReadOnlyList args, CancellationToken cancellationToken, bool network, int timeoutSeconds) => ExecuteGitAsync(repo, args, PublishGitSpec(publishDir, args, network, timeoutSeconds), cancellationToken); - private Task RunTokenedPublishGitOrThrowAsync(MaterializedRepo repo, string publishDir, string authedUrl, IReadOnlyList args, CancellationToken cancellationToken) => - EnsureSuccessAsync(repo, args, RunTokenedPublishGitAsync(repo, publishDir, authedUrl, args, cancellationToken)); + private Task RunTokenedPublishGitOrThrowAsync(MaterializedRepo repo, string publishDir, TokenedGitCommand.Remote remote, IReadOnlyList args, CancellationToken cancellationToken) => + EnsureSuccessAsync(repo, args, RunTokenedPublishGitAsync(repo, publishDir, remote, args, cancellationToken)); - /// Run a publish-repo command that names — the LFS upload, the push, the readback — as a , with the network and the push budget. - private Task RunTokenedPublishGitAsync(MaterializedRepo repo, string publishDir, string authedUrl, IReadOnlyList args, CancellationToken cancellationToken) => - ExecuteGitAsync(repo, args, TokenedGitCommand.Spec(authedUrl, PublishGitSpec(publishDir, args, network: true, PushTimeoutSeconds)), cancellationToken); + /// Run a publish-repo command that reaches — the LFS upload, the push, the readback — as a , with the network and the push budget. + private Task RunTokenedPublishGitAsync(MaterializedRepo repo, string publishDir, TokenedGitCommand.Remote remote, IReadOnlyList args, CancellationToken cancellationToken) => + ExecuteGitAsync(repo, args, TokenedGitCommand.Spec(remote, PublishGitSpec(publishDir, args, network: true, PushTimeoutSeconds)), cancellationToken); private static SandboxSpec PublishGitSpec(string publishDir, IReadOnlyList args, bool network, int timeoutSeconds) => new() { Command = "git", Args = args, WorkingDirectory = publishDir, TimeoutSeconds = timeoutSeconds, AllowNetwork = network }; @@ -1040,7 +1029,7 @@ private async Task ExecuteGitAsync(MaterializedRepo repo, IReadOn } } - /// Await a git result and throw a redacted on a non-success status; the argv is redacted because the push command carries the authed URL. + /// Await a git result and throw a redacted on a non-success status; the argv is redacted too, as a belt. private static async Task EnsureSuccessAsync(MaterializedRepo repo, IReadOnlyList args, Task run) { var result = await run.ConfigureAwait(false); @@ -1052,7 +1041,7 @@ private static async Task EnsureSuccessAsync(MaterializedRepo repo, IRea return result.Stdout; } - /// Redact the token from the echoed argv (the push command carries the authed URL) before it lands in an exception message. + /// Redact the token from the echoed argv before it lands in an exception message — a belt: no argv carries it. private static IEnumerable RedactArgs(IReadOnlyList args, string? token) => args.Select(a => Redact(a, token)); public ValueTask DisposeAsync() diff --git a/backend/src/CodeSpace.Core/Services/Agents/Workspace/RemoteTipResolver.cs b/backend/src/CodeSpace.Core/Services/Agents/Workspace/RemoteTipResolver.cs index 85e52a152..4526b13ee 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Workspace/RemoteTipResolver.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Workspace/RemoteTipResolver.cs @@ -8,9 +8,9 @@ namespace CodeSpace.Core.Services.Agents.Workspace; /// /// over git ls-remote, run through the local -/// exactly like 's own git calls (same auth-URL embedding, same -/// token redaction on surfaced errors, same process/timeout handling, and a tokened probe runs as a -/// ). Branch first, tag second (preferring the +/// exactly like 's own git calls (same token redaction on surfaced +/// errors, same process/timeout handling, and a tokened probe runs as a , naming the remote +/// without its credential and carrying the token in its environment). Branch first, tag second (preferring the /// peeled ^{} commit over the annotated tag object — the pin is a COMMIT), HEAD when no ref is named. /// Returned lines are matched by EXACT full ref name (ls-remote patterns are tail-matched globs — a pattern hit is /// necessary but not sufficient), so a glob-shaped or shadowing ref can never pin the wrong commit. @@ -26,18 +26,18 @@ public sealed class RemoteTipResolver : IRemoteTipResolver, ISingletonDependency public async Task ResolveTipShaAsync(WorkspaceRequest request, bool refRequired, CancellationToken cancellationToken) { - var url = LocalGitWorkspaceProvider.BuildAuthenticatedUrl(request.RepositoryUrl, request.TokenUsername, request.Token); + var remote = TokenedGitCommand.RemoteFor(request.RepositoryUrl, request.TokenUsername, request.Token); - if (string.IsNullOrWhiteSpace(request.Ref)) return await ResolveHeadAsync(url, request, cancellationToken).ConfigureAwait(false); + if (string.IsNullOrWhiteSpace(request.Ref)) return await ResolveHeadAsync(remote, request, cancellationToken).ConfigureAwait(false); - if (await ResolveRefAsync(url, request.Ref!, request, cancellationToken).ConfigureAwait(false) is { } sha) return sha; + if (await ResolveRefAsync(remote, request.Ref!, request, cancellationToken).ConfigureAwait(false) is { } sha) return sha; // The request's own SOFT semantics (a session-inherited prior branch that a merged PR may have pruned): // fall to the default branch, mirroring the clone's ResolveCheckoutRefAsync. A HARD ref (DefaultRef null) // that is gone fails LOUD — the clone would fail identically later; the pin just surfaces it at launch. if (!string.IsNullOrWhiteSpace(request.DefaultRef) && !string.Equals(request.Ref, request.DefaultRef, StringComparison.Ordinal)) { - if (await ResolveRefAsync(url, request.DefaultRef!, request, cancellationToken).ConfigureAwait(false) is { } fallback) return fallback; + if (await ResolveRefAsync(remote, request.DefaultRef!, request, cancellationToken).ConfigureAwait(false) is { } fallback) return fallback; if (refRequired) throw MissingRef(request.DefaultRef!, request); @@ -53,37 +53,37 @@ public sealed class RemoteTipResolver : IRemoteTipResolver, ISingletonDependency } /// The remote's HEAD commit — null for an EMPTY remote (ls-remote succeeds with no output: nothing exists to pin). - private async Task ResolveHeadAsync(string url, WorkspaceRequest request, CancellationToken cancellationToken) + private async Task ResolveHeadAsync(TokenedGitCommand.Remote remote, WorkspaceRequest request, CancellationToken cancellationToken) { - var lines = await LsRemoteAsync(url, new[] { "HEAD" }, request, cancellationToken).ConfigureAwait(false); + var lines = await LsRemoteAsync(remote, new[] { "HEAD" }, request, cancellationToken).ConfigureAwait(false); return lines.Where(l => l.Ref == "HEAD").Select(l => l.Sha).FirstOrDefault(); } /// The tip commit of a NAMED ref: its branch, else its tag (peeled ^{{}} commit preferred over the annotated tag object). Null when the remote has no such ref. Lines are matched by EXACT full ref name, never by the pattern's tail-glob. - private async Task ResolveRefAsync(string url, string @ref, WorkspaceRequest request, CancellationToken cancellationToken) + private async Task ResolveRefAsync(TokenedGitCommand.Remote remote, string @ref, WorkspaceRequest request, CancellationToken cancellationToken) { - var branch = await LsRemoteAsync(url, new[] { $"refs/heads/{@ref}" }, request, cancellationToken).ConfigureAwait(false); + var branch = await LsRemoteAsync(remote, new[] { $"refs/heads/{@ref}" }, request, cancellationToken).ConfigureAwait(false); if (branch.FirstOrDefault(l => l.Ref == $"refs/heads/{@ref}") is { Sha.Length: > 0 } hit) return hit.Sha; - var tags = await LsRemoteAsync(url, new[] { $"refs/tags/{@ref}", $"refs/tags/{@ref}^{{}}" }, request, cancellationToken).ConfigureAwait(false); + var tags = await LsRemoteAsync(remote, new[] { $"refs/tags/{@ref}", $"refs/tags/{@ref}^{{}}" }, request, cancellationToken).ConfigureAwait(false); return tags.Where(t => t.Ref == $"refs/tags/{@ref}^{{}}").Select(t => t.Sha).FirstOrDefault() ?? tags.Where(t => t.Ref == $"refs/tags/{@ref}").Select(t => t.Sha).FirstOrDefault(); } /// One git ls-remote round-trip parsed to (sha, ref) lines. A non-zero exit throws LOUD with the token redacted and the URL stripped of any userinfo — an unreachable remote at launch is the SAME failure the clone would surface later, just earlier and honest. - private async Task> LsRemoteAsync(string url, IReadOnlyList patterns, WorkspaceRequest request, CancellationToken cancellationToken) + private async Task> LsRemoteAsync(TokenedGitCommand.Remote remote, IReadOnlyList patterns, WorkspaceRequest request, CancellationToken cancellationToken) { - var args = new List { "ls-remote", url }; + var args = new List { "ls-remote", remote.Url }; args.AddRange(patterns); SandboxResult result; try { result = await _runners.Resolve(SandboxKinds.Local) - .RunAsync(TokenedGitCommand.Spec(url, new SandboxSpec { Command = "git", Args = args, TimeoutSeconds = LsRemoteTimeoutSeconds, AllowNetwork = true }), cancellationToken).ConfigureAwait(false); + .RunAsync(TokenedGitCommand.Spec(remote, new SandboxSpec { Command = "git", Args = args, TimeoutSeconds = LsRemoteTimeoutSeconds, AllowNetwork = true }), cancellationToken).ConfigureAwait(false); } catch (Win32Exception ex) { diff --git a/backend/src/CodeSpace.Core/Services/Agents/Workspace/TokenedGitCommand.cs b/backend/src/CodeSpace.Core/Services/Agents/Workspace/TokenedGitCommand.cs index 45fc0220c..cc76df687 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Workspace/TokenedGitCommand.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Workspace/TokenedGitCommand.cs @@ -3,34 +3,75 @@ namespace CodeSpace.Core.Services.Agents.Workspace; /// -/// Marks a git command as TOKENED — one whose git transport can reach a remote whose URL carries a clone token, -/// either named in its own argv (the clone, a probe or readback ls-remote, the publish push, and -/// lfs push, which runs git's transport against that URL itself) or as the origin of the clone it runs in -/// (a fetch or push before the token is stripped) — and keeps that token out of the operator's credential helpers and -/// trace2 targets. +/// The one way a platform git command carries a clone credential. The command names its remote by a URL without +/// userinfo () and carries the credential in its environment, where only a credential helper +/// scoped to that remote reads it. A token in the URL would sit in the argv of git and of every transport child, which any +/// host user reads from /proc/<pid>/cmdline unless /proc hides other users' processes; in the +/// .git/config a clone writes before its transfer starts; and in the hands of any authority the remote redirects to, +/// since git answers its 401 with the URL's credential. The environment is readable by the same uid and root alone. /// -/// The token is already in the URL, so a tokened command has nothing to ask a helper for. But git still honours -/// the helpers in system and global config, and on success its transport hands the URL's username and password to each -/// of them to store: an operator's credential.helper=store (or cache, or a keychain) would keep -/// every run's token at rest. empties the helper list for the tokened remote's scheme -/// and authority: an empty value clears the helpers collected so far, URL-scoped, path-scoped, user-scoped and included -/// ones too, and a command-line value is read after system, global and repository config. It reaches child processes -/// (git-lfs, the git transport lfs push runs) through GIT_CONFIG_PARAMETERS. It is scoped rather than -/// global because git asks the same helpers for other hosts' credentials — an authenticating proxy named with only a -/// user, a separate LFS host — and those must still answer. +/// hands git its config through GIT_CONFIG_COUNT, which git reads after +/// system, global and repository config and every child inherits — the transport, git-lfs, the git credential that +/// git-lfs runs. Its first entry empties the helper list for the remote's scheme and authority: an empty value clears the +/// helpers collected so far, URL-scoped, path-scoped and included ones too, so no operator helper is asked for this +/// remote's credential, or told to store it on success or erase it on a refusal. Its second names +/// , which answers get from and +/// with the shell's builtin printf, so the values never reach an argv either. Both are scoped +/// rather than global: git asks the same helpers for other hosts' credentials — an authenticating proxy named with only a +/// user, a separate LFS host — and those must still answer. A redirect to another authority finds no helper that answers +/// for it, so the command fails rather than send the credential there. GIT_CONFIG_COUNT needs git 2.31 or later; an +/// older git ignores it, finds no credential and fails the command rather than leak it. /// -/// git also writes every command's argv, and each child's (git remote-http <url>), to the trace2 -/// targets in system and global config; git 2.33 writes the URL's password verbatim. Those targets are read before any -/// -c, so a tokened command runs with in its environment, which wins over them. +/// The helper stops answering once the remote has refused the credential. git-lfs answers a 401 by telling the +/// helpers to erase the credential and asking them again, with no limit, so a helper that always answered would keep +/// it retrying a refused token — one past its lifetime, a revoked one — for the whole command timeout, tens of failed logins +/// a second. The helper records the erase as an empty marker file in the directory names, +/// which the runner makes owner-only for this one command (), binds into a +/// confined one — as its HOME too, as empty as the private /tmp it had — and removes afterwards; after that a +/// get answers nothing and says on stderr that the remote refused it, so git-lfs fails at once with that reason. +/// Without the directory the helper never answers: it could not stop the retries. git itself stops after one +/// refusal. /// -/// git-lfs's own object transfers through a tokened origin — the downloads a checkout, a hard reset or an apply -/// makes — authenticate from the URL's userinfo and neither ask nor tell a helper (verified with git-lfs 3.7.1), so a -/// command that only reaches the origin that way is not tokened. An untokened command is left as written: the -/// operator's helpers may be how it authenticates to a private mirror. +/// The third and fourth entries map the remote's URL to itself as a url.<base>.insteadOf and +/// pushInsteadOf. git and git-lfs rewrite a URL by the longest matching prefix, and the whole URL is the longest any +/// rule can match, so no operator rule moves a tokened command: not url."git@host:".insteadOf=https://host/ to SSH +/// under the host's key, nor a rule carrying an operator's own token in place of the one this command is bound to. No rule +/// matched a URL with the token in it, so this keeps what tokened commands always did. The last two turn auto-gc and +/// auto-maintenance off, so no detached gc or maintenance child outlives the command with the credential in its +/// environment. +/// +/// No -c on a tokened command may touch the credential helpers: git reads GIT_CONFIG_PARAMETERS after +/// GIT_CONFIG_COUNT, so a -c credential.<url>.helper= would empty the list again and the command would +/// find no credential. The two variables are not GIT_-prefixed: git-lfs writes every GIT_* variable into the +/// logs it keeps inside the clone. +/// +/// git also writes every command's argv, each child's, and the variables an operator's trace2.envVars names, +/// to the trace2 targets in system and global config, which are read before any command-line or environment config; a +/// tokened command runs with in its environment, which wins over them. An untokened command is left +/// as written: the operator's helpers may be how it authenticates to a private mirror. /// internal static class TokenedGitCommand { - /// The environment a tokened command runs with: git's three trace2 targets switched off. + /// The variable reads the username from. + internal const string UsernameVariable = "CODESPACE_GIT_USERNAME"; + + /// The variable reads the password — the token — from. + internal const string PasswordVariable = "CODESPACE_GIT_PASSWORD"; + + /// The variable the runner points at the command's own owner-only directory, where records that the remote refused the credential. + internal const string StateVariable = "CODESPACE_GIT_STATE"; + + /// The username a token is sent under when its provider names none: GitHub's. + internal const string DefaultTokenUsername = "x-access-token"; + + /// + /// The credential helper a tokened command names: a shell function that answers get from the two variables until + /// an erase records in the directory that the remote refused the credential, then says + /// so on stderr instead; it ignores store, and answers nothing without that directory. + /// + internal const string CredentialHelper = """!f() { r="$CODESPACE_GIT_STATE/refused"; case "$1" in get) if test -e "$r"; then echo "the remote refused this credential; not offering it again" >&2; elif test -d "$CODESPACE_GIT_STATE"; then printf "username=%s\npassword=%s\n" "$CODESPACE_GIT_USERNAME" "$CODESPACE_GIT_PASSWORD"; fi;; erase) test -d "$CODESPACE_GIT_STATE" && : > "$r";; esac; }; f"""; + + /// The environment a tokened command runs with besides its credential: git's three trace2 targets switched off. internal static readonly IReadOnlyDictionary TraceOff = new Dictionary(StringComparer.Ordinal) { ["GIT_TRACE2"] = "0", @@ -38,31 +79,92 @@ internal static class TokenedGitCommand ["GIT_TRACE2_PERF"] = "0", }; - /// True when embeds a password: the token LocalGitWorkspaceProvider.BuildAuthenticatedUrl put there, or one a stored or pasted URL already carries. A bare username is not a secret, and a path or an scp-style address carries none. - internal static bool IsTokened(string remoteUrl) => - Uri.TryCreate(remoteUrl, UriKind.Absolute, out var uri) && uri.UserInfo.Split(':', 2) is [_, { Length: > 0 }]; + /// A remote as a git command names it — , with no userinfo secret — and the credential that goes with it, if any. + internal sealed record Remote(string Url, string? Username, string? Password) + { + /// True when the remote carries a credential, so every command that reaches it runs as a tokened command. + public bool IsTokened => Password is not null; + } - /// The config a tokened command gets ahead of its own arguments: an empty helper list for the remote's scheme and authority — never its userinfo, so the key carries no token. Empty, not false: only the empty value resets the list; any other value adds one more helper. - internal static IReadOnlyList CredentialHelperReset(string remoteUrl) + /// + /// The remote names, with the clone for it. With a token, the URL + /// loses any userinfo and the token is the credential, under ( + /// when the provider names none). Without one, an http(s) URL whose userinfo carries a password — a stored credential — + /// gives it up the same way; any other URL is left as written, a bare username included: it names an account, and the + /// operator's helpers may answer for it. + /// + internal static Remote RemoteFor(string repositoryUrl, string? tokenUsername, string? token) { - var uri = new Uri(remoteUrl); + if (!string.IsNullOrEmpty(token)) return new Remote(WithoutUserInfo(repositoryUrl), string.IsNullOrEmpty(tokenUsername) ? DefaultTokenUsername : tokenUsername, token); - return new[] { "-c", $"credential.{uri.Scheme}://{uri.Authority}.helper=" }; + return HttpUserInfo(repositoryUrl) is [_, { Length: > 0 }] ? FromUserInfo(repositoryUrl) : new Remote(repositoryUrl, null, null); } - /// as a tokened command () when the remote it can reach is tokened, otherwise unchanged. - internal static SandboxSpec Spec(string remoteUrl, SandboxSpec spec) => IsTokened(remoteUrl) ? AsTokened(remoteUrl, spec) : spec; + /// + /// The remote an http(s) names, its whole userinfo moved into the credential, decoded as git decodes + /// it: the user, and the password or an empty one — a token pasted as the user alone is sent with an empty password, as + /// curl sent it from the URL. For a caller that knows the URL's userinfo is a credential even without a password. + /// + internal static Remote FromUserInfo(string url) + { + var parts = HttpUserInfo(url) ?? throw new ArgumentException("Only an http(s) URL carries a credential a helper can answer for.", nameof(url)); + + return new Remote(WithoutUserInfo(url), Uri.UnescapeDataString(parts[0]), parts.Length > 1 ? Uri.UnescapeDataString(parts[1]) : ""); + } /// - /// as a tokened command for — ahead of - /// its arguments, over its environment — whatever says: for a caller that knows - /// the URL's userinfo is a credential without a password, as a token pasted as the user alone is. + /// as a tokened command for — over its + /// environment and a state directory for , its argv untouched — when the remote is tokened, + /// otherwise unchanged. /// - internal static SandboxSpec AsTokened(string remoteUrl, SandboxSpec spec) + internal static SandboxSpec Spec(Remote remote, SandboxSpec spec) { + if (!remote.IsTokened) return spec; + var environment = new Dictionary(spec.Environment); - foreach (var (name, value) in TraceOff) environment[name] = value; + foreach (var (name, value) in CredentialEnvironment(remote)) environment[name] = value; + + return spec with { Environment = environment, ConfigHomeEnvVars = [.. spec.ConfigHomeEnvVars, StateVariable] }; + } + + /// + /// The environment that carries 's credential: the config entries (the scoped reset, then + /// for the same scope, then the remote's URL mapped to itself for fetch and push, then + /// auto-gc and auto-maintenance off), the credential in the two variables the helper reads, and . + /// + internal static IReadOnlyDictionary CredentialEnvironment(Remote remote) + { + var helperKey = $"credential.{Scope(remote.Url)}.helper"; + var config = new (string Key, string Value)[] + { + (helperKey, ""), (helperKey, CredentialHelper), + ($"url.{remote.Url}.insteadOf", remote.Url), ($"url.{remote.Url}.pushInsteadOf", remote.Url), + ("gc.auto", "0"), ("maintenance.auto", "false"), + }; - return spec with { Args = [.. CredentialHelperReset(remoteUrl), .. spec.Args], Environment = environment }; + var environment = new Dictionary(TraceOff, StringComparer.Ordinal) { ["GIT_CONFIG_COUNT"] = config.Length.ToString(), [UsernameVariable] = remote.Username ?? "", [PasswordVariable] = remote.Password ?? "" }; + + for (var i = 0; i < config.Length; i++) + { + environment[$"GIT_CONFIG_KEY_{i}"] = config[i].Key; + environment[$"GIT_CONFIG_VALUE_{i}"] = config[i].Value; + } + + return environment; + } + + /// The scheme and authority a credential is answered for — never a path, never a userinfo. + private static string Scope(string url) + { + var uri = new Uri(url); + + return $"{uri.Scheme}://{uri.Authority}"; } + + /// without its userinfo, or as written when it has none. + private static string WithoutUserInfo(string url) => RemoteTipResolver.SanitizeUrl(url); + + /// An http(s) URL's userinfo split at its first ':' — the user, then the password when there is one — still encoded; null for any other URL or one without userinfo. + private static string[]? HttpUserInfo(string url) => + Uri.TryCreate(url, UriKind.Absolute, out var uri) && (uri.Scheme == Uri.UriSchemeHttps || uri.Scheme == Uri.UriSchemeHttp) && uri.UserInfo.Length > 0 ? uri.UserInfo.Split(':', 2) : null; } diff --git a/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorAcceptanceGrader.cs b/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorAcceptanceGrader.cs index 0671c4acc..05a2cb7f9 100644 --- a/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorAcceptanceGrader.cs +++ b/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorAcceptanceGrader.cs @@ -265,7 +265,7 @@ public async Task GradePatchAsync(PatchAcceptanceGradeRequest re return Failed("no-branch-or-repo"); } - var applyError = await ApplyPatchAsync(directory, patch, cancellationToken).ConfigureAwait(false); + var applyError = await ApplyPatchAsync(directory, patch, clone, cancellationToken).ConfigureAwait(false); if (applyError is not null) { @@ -332,39 +332,50 @@ public async Task GradeBaseAsync(BaseAcceptanceGradeRequest requ } } - /// Full clone (no --branch — a base SHA is not a ref name the shared provider's clone can accept) then a detached checkout of the exact base. Throws (redacted) on either git failure. + /// + /// Full clone (no --branch — a base SHA is not a ref name the shared provider's clone can accept) then a detached + /// checkout of the exact base. Throws (redacted) on either git failure. Both reach the + /// remote, so both run as s: the clone through git's transport, the checkout through + /// git-lfs, which downloads the base's LFS objects from origin and asks the credential helpers for them. + /// private async Task CloneAtBaseAsync(WorkspaceRequest clone, string baseSha, string directory, CancellationToken cancellationToken) { Directory.CreateDirectory(directory); - var url = LocalGitWorkspaceProvider.BuildAuthenticatedUrl(clone.RepositoryUrl, clone.TokenUsername, clone.Token); + var remote = RemoteFor(clone); var cloneResult = await _runners.Resolve(GradingRunnerKind).RunAsync( - TokenedGitCommand.Spec(url, new SandboxSpec { Command = "git", Args = new[] { "clone", url, directory }, WorkingDirectory = directory, TimeoutSeconds = CloneTimeoutSeconds, AllowNetwork = true }), cancellationToken).ConfigureAwait(false); + TokenedGitCommand.Spec(remote, new SandboxSpec { Command = "git", Args = new[] { "clone", remote.Url, directory }, WorkingDirectory = directory, TimeoutSeconds = CloneTimeoutSeconds, AllowNetwork = true }), cancellationToken).ConfigureAwait(false); if (cloneResult.Status != SandboxStatus.Success) throw new WorkspaceException($"git clone failed (exit {cloneResult.ExitCode}): {LocalGitWorkspaceProvider.Redact(Summarize(cloneResult.Stderr), clone.Token)}"); - // Model-authored setup/acceptance commands run INSIDE this clone next — strip the tokened origin via the - // SAME shared helper LocalGitWorkspaceProvider's own branch-grading path uses (LocalGitWorkspaceProvider. - // StripTokenFromRemoteAsync — one implementation, not a second copy that could drift), so no credential - // persists in .git/config for those commands to read. FAIL-CLOSED: if the strip cannot be completed the - // helper throws a WorkspaceException, which this method's callers already catch into a typed grade failure - // and whose finally deletes the clone — the model-authored commands below are exactly the readers this - // credential must be kept from, so a grade is the cheaper thing to lose. Guarded on a present token, + // Model-authored setup/acceptance commands run INSIDE this clone next. The clone named the remote without its + // credential, so origin holds none; the strip stays as a belt, via the SAME shared helper LocalGitWorkspaceProvider's + // own branch-grading path uses (LocalGitWorkspaceProvider.StripTokenFromRemoteAsync — one implementation, not a + // second copy that could drift), so no credential persists in .git/config for those commands to read. FAIL-CLOSED: + // if the strip cannot be completed the helper throws a WorkspaceException, which this method's callers already catch + // into a typed grade failure and whose finally deletes the clone — the model-authored commands below are exactly the + // readers this credential must be kept from, so a grade is the cheaper thing to lose. Guarded on a tokened remote, // mirroring MaterializeAsync's own call site exactly — a public repo with no credential has nothing to strip. - if (!string.IsNullOrEmpty(clone.Token)) - await LocalGitWorkspaceProvider.StripTokenFromRemoteAsync(_runners.Resolve(GradingRunnerKind), CloneTimeoutSeconds, _logger, clone.RepositoryUrl, directory, cancellationToken).ConfigureAwait(false); + if (remote.IsTokened) + await LocalGitWorkspaceProvider.StripTokenFromRemoteAsync(_runners.Resolve(GradingRunnerKind), CloneTimeoutSeconds, _logger, remote.Url, directory, cancellationToken).ConfigureAwait(false); var checkoutResult = await _runners.Resolve(GradingRunnerKind).RunAsync( - new SandboxSpec { Command = "git", Args = new[] { "-C", directory, "checkout", "--detach", baseSha }, WorkingDirectory = directory, TimeoutSeconds = CloneTimeoutSeconds, AllowNetwork = true }, cancellationToken).ConfigureAwait(false); + TokenedGitCommand.Spec(remote, new SandboxSpec { Command = "git", Args = new[] { "-C", directory, "checkout", "--detach", baseSha }, WorkingDirectory = directory, TimeoutSeconds = CloneTimeoutSeconds, AllowNetwork = true }), cancellationToken).ConfigureAwait(false); if (checkoutResult.Status != SandboxStatus.Success) throw new WorkspaceException($"base revision {baseSha} not found in the repository: {LocalGitWorkspaceProvider.Redact(Summarize(checkoutResult.Stderr), clone.Token)}"); } - /// Apply onto the already-checked-out — NO stage, NO commit, NO push (this grade is read-only by construction; the clone is discarded after grading either way). Mirrors LocalGitBranchIntegrator's own apply step (git apply --3way) minus --index, since nothing here is ever committed. Returns null on success, else git's stderr. - private async Task ApplyPatchAsync(string directory, string patch, CancellationToken cancellationToken) + /// + /// Apply onto the already-checked-out — NO stage, NO commit, NO push + /// (this grade is read-only by construction; the clone is discarded after grading either way). Mirrors + /// LocalGitBranchIntegrator's own apply step (git apply --3way) minus --index, since nothing here is + /// ever committed. A for 's remote: git-lfs downloads the LFS + /// objects the patch points at from origin. Returns null on success, else git's stderr. + /// + private async Task ApplyPatchAsync(string directory, string patch, WorkspaceRequest clone, CancellationToken cancellationToken) { var patchFile = Path.Combine(directory, $".codespace-acceptance-{Guid.NewGuid():N}.patch"); await File.WriteAllTextAsync(patchFile, patch, cancellationToken).ConfigureAwait(false); @@ -372,7 +383,7 @@ private async Task CloneAtBaseAsync(WorkspaceRequest clone, string baseSha, stri try { var result = await _runners.Resolve(GradingRunnerKind).RunAsync( - new SandboxSpec { Command = "git", Args = new[] { "-C", directory, "apply", "--3way", patchFile }, WorkingDirectory = directory, TimeoutSeconds = 60, AllowNetwork = true }, cancellationToken).ConfigureAwait(false); + TokenedGitCommand.Spec(RemoteFor(clone), new SandboxSpec { Command = "git", Args = new[] { "-C", directory, "apply", "--3way", patchFile }, WorkingDirectory = directory, TimeoutSeconds = 60, AllowNetwork = true }), cancellationToken).ConfigureAwait(false); return result.Status == SandboxStatus.Success ? null : result.Stderr; } @@ -382,6 +393,9 @@ private async Task CloneAtBaseAsync(WorkspaceRequest clone, string baseSha, stri } } + /// The remote the grading clone reaches, named without its credential, which the commands that reach it carry. + private static TokenedGitCommand.Remote RemoteFor(WorkspaceRequest clone) => TokenedGitCommand.RemoteFor(clone.RepositoryUrl, clone.TokenUsername, clone.Token); + /// /// P3a-3 (B+V0+): the ORACLE's bytes are not the candidate's to edit. When the base sha is known and the /// contract yields RUN-OWNED protected paths — AUTHORED (bar any file this same check EXECUTES that the floor diff --git a/backend/tests/CodeSpace.IntegrationTests/Agents/PackCloneCredentialFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Agents/PackCloneCredentialFlowTests.cs index 27fda3a9d..e32a12144 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Agents/PackCloneCredentialFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Agents/PackCloneCredentialFlowTests.cs @@ -21,17 +21,17 @@ namespace CodeSpace.IntegrationTests.Agents; /// HIGH fidelity: the REAL on the real and real git, /// against a loopback smart-HTTP remote () that demands a FAKE token for every read, so a /// clone that succeeds proves the pasted token authenticated it. An operator who pastes a URL carrying a token into the pack -/// import must get a checkout that holds no token once cloned, in a directory no other uid can read; a failed import must name -/// no token in its message — the text that reaches the API error body, the UI and the mediator's error log — even where git -/// itself echoes it (a token pasted as the user alone, in any spelling); and that user-alone token must reach neither the -/// operator's credential helpers nor their trace2 targets. +/// import must get a checkout that never holds the token — the clone names the remote without it, so git writes none even +/// before origin is rewritten — in a directory no other uid can read, with no argv carrying it; a failed import must name no +/// token in its message — the text that reaches the API error body, the UI and the mediator's error log — in any spelling; +/// and a token pasted as the user alone must reach neither the operator's credential helpers nor their trace2 targets. /// -/// Positive controls: the remote refuses a clone without the token; the same production clone with origin left as git -/// wrote it holds the token, so the scan that finds none can see one; git's raw stderr carried the token wherever the message -/// is clean of an echo, so the clean message is the redaction's doing; and the clone run without the helper reset and with -/// trace2 on hands the token to the operator's helper and trace2 targets, so their silence is the tokened clone's doing. Each -/// test owns its remote and a scratch HOME (a global config of its own, system config off), and removes both on every path; -/// nothing reads or writes the real global config or keychain. +/// Positive controls: the remote refuses a clone without the token; a raw clone of the pasted URL leaves the token in +/// .git/config, so the scan that finds none can see one; and the clone run without the helper reset and with trace2 on hands +/// the token to the operator's helper (told to erase it once refused) and trace2 targets (recording the credential +/// variables), so their silence is the tokened clone's doing. Each test owns its remote and a scratch HOME (a global config +/// of its own, system config off), and removes both on every path; nothing reads or writes the real global config or +/// keychain. /// [Collection(PostgresCollection.Name)] [Trait("Category", "Integration")] @@ -46,8 +46,8 @@ public sealed class PackCloneCredentialFlowTests [Theory] [InlineData(false)] - [InlineData(true)] // positive control: origin left as git wrote it - public async Task A_pasted_token_clones_and_the_checkout_holds_no_token(bool originAsCloned) + [InlineData(true)] // origin left as git wrote it: the strip is a belt, the clone never wrote the token + public async Task A_pasted_token_clones_and_the_checkout_never_holds_it(bool originAsCloned) { if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; @@ -60,11 +60,14 @@ public async Task A_pasted_token_clones_and_the_checkout_holds_no_token(bool ori File.ReadAllText(Path.Combine(checkout.Directory, "README.md")).ShouldBe("base, revised\n", "the clone authenticated with the pasted token"); ctx.Runner.Ran("remote").ShouldBeTrue("fixture check: the fetcher asked for origin to be rewritten"); - FilesHolding(checkout.Directory, GitPublishRemoteFixture.FakeToken).ShouldBe(originAsCloned ? new[] { ".git/config" } : Array.Empty(), originAsCloned ? "positive control: git writes the pasted URL into origin" : "the checkout the import walks holds the pasted token"); + FilesHolding(checkout.Directory, GitPublishRemoteFixture.FakeToken).ShouldBeEmpty("the checkout the import walks holds the pasted token"); + (await ctx.OriginUrlAsync(checkout.Directory)).ShouldBe(ctx.Remote.Url, "origin names the remote without its token, whether or not the strip ran"); + ctx.Runner.Argvs.Where(a => a.Contains(Marker, StringComparison.Ordinal) || a.Contains("@127.0.0.1", StringComparison.Ordinal)).ShouldBeEmpty("no argv carries the pasted credential"); - if (!originAsCloned) (await ctx.OriginUrlAsync(checkout.Directory)).ShouldBe(ctx.Remote.Url, "origin was rewritten to the remote without its token, not removed"); + File.GetUnixFileMode(checkout.Directory).ShouldBe(UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute, "git cloned into the owner-only directory without widening it"); - File.GetUnixFileMode(checkout.Directory).ShouldBe(UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute, "git cloned into the owner-only directory without widening it, so no other uid read .git/config while it held the token"); + var raw = await ctx.RawCloneAsync(ctx.UrlWith($"x-access-token:{GitPublishRemoteFixture.FakeToken}")); + FilesHolding(raw, GitPublishRemoteFixture.FakeToken).ShouldBe(new[] { ".git/config" }, "positive control: git writes a URL it clones, credential and all, into origin"); } [Theory] @@ -72,8 +75,8 @@ public async Task A_pasted_token_clones_and_the_checkout_holds_no_token(bool ori [InlineData(true)] // positive control: the clone run without the helper reset and with trace2 on public async Task A_token_pasted_as_the_user_alone_reaches_no_credential_helper_or_trace2_target(bool unreset) { - // git asks the credential helpers for the password the URL lacks, naming the user, and writes the clone's argv and - // its remote-http child's to the trace2 targets — both from the operator's global config. + // git tells the credential helpers to erase a credential the remote refused, naming its user, and writes the values of + // the variables trace2.envVars names to the trace2 targets — both from the operator's global config. if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; await using var ctx = await ScratchHostContext.StartAsync(); @@ -84,25 +87,28 @@ public async Task A_token_pasted_as_the_user_alone_reaches_no_credential_helper_ ctx.OperatorTrace().ShouldContain(ctx.Remote.Url, Case.Sensitive, "fixture check: the operator's trace2 targets record an untokened clone"); ctx.Runner.Unreset = unreset; - await Should.ThrowAsync(() => ctx.Fetcher.FetchAsync(ctx.UrlWith("fake-pasted-token-0123456789"), null, CancellationToken.None), "git asks for a password the pasted URL does not carry"); + await Should.ThrowAsync(() => ctx.Fetcher.FetchAsync(ctx.UrlWith("fake-pasted-token-0123456789"), null, CancellationToken.None), "the remote refuses the pasted user with an empty password"); - ctx.HelperLog().Contains(Marker, StringComparison.Ordinal).ShouldBe(unreset, unreset ? "positive control: without the reset git hands the pasted user to the operator's helper" : "the pasted token reached the operator's credential helper"); - ctx.OperatorTrace().Contains(Marker, StringComparison.Ordinal).ShouldBe(unreset, unreset ? "positive control: with trace2 on git records the pasted URL" : "the pasted token reached the operator's trace2 targets"); + ctx.HelperLog().Contains(Marker, StringComparison.Ordinal).ShouldBe(unreset, unreset ? "positive control: without the reset git tells the operator's helper to erase the refused pasted user" : "the pasted token reached the operator's credential helper"); + ctx.OperatorTrace().Contains(Marker, StringComparison.Ordinal).ShouldBe(unreset, unreset ? "positive control: with trace2 on git records the credential variables" : "the pasted token reached the operator's trace2 targets"); } [Theory] - [InlineData("x-access-token:fake-wrong-token-0123456789", false)] // a wrong or revoked token: git hides the password, the message used to name it in the URL - [InlineData("fake-pasted-token-0123456789", true)] // a token pasted as the user: git asks for a password and names the user - [InlineData("fake%2fpasted%40token-0123456789", true)] // the same, percent-encoded: git echoes it decoded or re-encoded - public async Task A_failed_clone_names_no_pasted_token(string userInfo, bool gitEchoesIt) + [InlineData("x-access-token:fake-wrong-token-0123456789")] // a wrong or revoked token + [InlineData("fake-pasted-token-0123456789")] // a token pasted as the user + [InlineData("fake%2fpasted%40token-0123456789")] // the same, percent-encoded + public async Task A_failed_clone_names_no_pasted_token(string userInfo) { + // git names the remote only by the URL it was handed, which carries no userinfo, so it echoes the token nowhere; the + // message's own redaction stays as a belt for a remote that echoes it. if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; await using var ctx = await ScratchHostContext.StartAsync(); var failure = await Should.ThrowAsync(() => ctx.Fetcher.FetchAsync(ctx.UrlWith(userInfo), null, CancellationToken.None)); - ctx.Runner.Stderr.Contains(Marker, StringComparison.Ordinal).ShouldBe(gitEchoesIt, "fixture check: where git echoed the pasted token"); + ctx.Runner.Stderr.ShouldContain("Authentication failed", Case.Sensitive, "fixture check: the remote refused the pasted credential"); + ctx.Runner.Stderr.ShouldNotContain(Marker, Case.Sensitive, "git echoed the pasted token"); failure.Message.ShouldNotContain(Marker, Case.Sensitive, "the message reaches the API error body, the UI and the mediator's error log"); failure.Message.ShouldContain($"'{ctx.Remote.Url}'", Case.Sensitive, "the message still names the remote, without its userinfo"); failure.Message.ShouldContain("exit 128"); @@ -127,7 +133,7 @@ public async Task A_failed_import_through_the_mediator_names_no_pasted_token() var failure = await Should.ThrowAsync(() => scope.Resolve().Send(import)); - ctx.Runner.Stderr.ShouldContain("fake-pasted-token-0123456789", Case.Sensitive, "fixture check: git echoed the pasted token"); + ctx.Runner.Stderr.ShouldContain("Authentication failed", Case.Sensitive, "fixture check: the clone presented the pasted credential and the remote refused it"); failure.Message.ShouldNotContain(Marker, Case.Sensitive, "what the import surfaces to the API error body and the mediator's error log"); } @@ -198,7 +204,8 @@ public static async Task StartAsync() /// /// The operator's global config: a credential helper that logs every request git makes of it (the operation, then - /// what git sends: protocol, host, username) and answers none, and all three trace2 targets pointed at scratch files. + /// what git sends: protocol, host, username) and answers none, and all three trace2 targets pointed at scratch files, + /// recording the credential variables too. /// public void ConfigureLoggingHelperAndTrace2() { @@ -206,7 +213,7 @@ public void ConfigureLoggingHelperAndTrace2() File.WriteAllText(helper, $"#!/bin/sh\necho \"op=$1\" >> '{HelperLogFile}'\ncat >> '{HelperLogFile}'\n"); if (!OperatingSystem.IsWindows()) File.SetUnixFileMode(helper, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); - File.WriteAllText(GlobalConfig, $"[credential]\n\thelper = {helper}\n[trace2]\n\tnormalTarget = {TraceFile("normal")}\n\teventTarget = {TraceFile("event")}\n\tperfTarget = {TraceFile("perf")}\n"); + File.WriteAllText(GlobalConfig, $"[credential]\n\thelper = {helper}\n[trace2]\n\tnormalTarget = {TraceFile("normal")}\n\teventTarget = {TraceFile("event")}\n\tperfTarget = {TraceFile("perf")}\n\tenvVars = CODESPACE_GIT_USERNAME,CODESPACE_GIT_PASSWORD\n"); } /// Every request git made of the operator's helper. @@ -218,6 +225,16 @@ public void ConfigureLoggingHelperAndTrace2() /// The remote's URL as an operator would paste it with in it. public string UrlWith(string userInfo) => Remote.Url.Replace("http://", $"http://{userInfo}@", StringComparison.Ordinal); + /// A raw clone of on the scratch host, as git is handed it, into a directory of its own; never recorded by the runner, never production code. + public async Task RawCloneAsync(string url) + { + var directory = Path.Combine(_home, "raw-clone"); + var result = await new LocalProcessRunner().RunAsync(new SandboxSpec { Command = "git", Args = new[] { "-c", "core.hooksPath=/dev/null", "clone", url, directory }, Environment = Runner.Environment, TimeoutSeconds = 60, AllowNetwork = true }, CancellationToken.None); + + result.Status.ShouldBe(SandboxStatus.Success, $"fixture check: the raw clone failed: {result.Stderr}"); + return directory; + } + /// The clone's origin URL as git reads it, on the scratch host; never recorded by the runner. public async Task OriginUrlAsync(string cloneDir) { @@ -235,15 +252,13 @@ public async ValueTask DisposeAsync() } /// - /// The real local runner on the scratch host, recording each subcommand and git's raw stderr. With + /// The real local runner on the scratch host, recording each argv and git's raw stderr. With /// set, the git remote edits that strip a pasted credential report success without /// running, so origin stays as git wrote it; with set, the clone runs without the credential-helper - /// reset and with trace2 on — the positive controls. + /// reset and with trace2 on — the positive control. /// private sealed class ScratchHostRunner(IReadOnlyDictionary environment) : ISandboxRunner { - private static readonly string[] TraceOff = { "GIT_TRACE2", "GIT_TRACE2_EVENT", "GIT_TRACE2_PERF" }; - private readonly LocalProcessRunner _inner = new(); private readonly List _specs = new(); @@ -253,6 +268,9 @@ private sealed class ScratchHostRunner(IReadOnlyDictionary envir public bool Unreset { get; set; } public string Stderr { get; private set; } = ""; + /// Every argv the fetcher handed the runner, joined. + public IEnumerable Argvs => _specs.Select(s => string.Join(' ', s.Args)); + public bool Ran(string subcommand) => _specs.Any(s => s.Args.Contains(subcommand)); public async Task RunAsync(SandboxSpec spec, CancellationToken cancellationToken) @@ -261,22 +279,13 @@ public async Task RunAsync(SandboxSpec spec, CancellationToken ca if (KeepOriginAsCloned && spec.Args.Contains("remote")) return new SandboxResult { Status = SandboxStatus.Success, ExitCode = 0, Stdout = "", Stderr = "" }; - var unreset = Unreset && spec.Args.Contains("clone"); - var env = new Dictionary(spec.Environment); + var env = Unreset && spec.Args.Contains("clone") ? TokenedGitControls.WithoutTheReset(spec.Environment) : new Dictionary(spec.Environment); foreach (var (key, value) in environment) env[key] = value; - if (unreset) foreach (var key in TraceOff) env.Remove(key); - var result = await _inner.RunAsync(spec with { Args = unreset ? WithoutTheReset(spec.Args) : spec.Args, Environment = env }, cancellationToken); + var result = await _inner.RunAsync(spec with { Environment = env }, cancellationToken); Stderr += result.Stderr; return result; } - - private static IReadOnlyList WithoutTheReset(IReadOnlyList args) - { - var at = Enumerable.Range(0, Math.Max(0, args.Count - 1)).FirstOrDefault(i => args[i] == "-c" && args[i + 1].StartsWith("credential.", StringComparison.Ordinal) && args[i + 1].EndsWith(".helper=", StringComparison.Ordinal), -1); - - return at < 0 ? args : args.Take(at).Concat(args.Skip(at + 2)).ToList(); - } } private sealed class AllowAll : IPackHostAllowlist diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/AgentWorkspacePushFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/AgentWorkspacePushFlowTests.cs index 3a568113a..26f6841c7 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/AgentWorkspacePushFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/AgentWorkspacePushFlowTests.cs @@ -130,7 +130,8 @@ public async Task A_harness_that_committed_its_own_work_is_still_pushed() public async Task Auth_failure_surfaces_a_redacted_workspace_exception() { // MANDATORY token-leak guard: a push at an unreachable/garbage remote must throw a WorkspaceException - // whose message has the token literal ABSENT and "***" present. + // whose message has the token literal ABSENT. The push names the remote without its credential, so the argv + // the message echoes has no token to redact. if (OperatingSystem.IsWindows()) return; if (!await GitAvailableAsync()) return; @@ -138,8 +139,8 @@ public async Task Auth_failure_surfaces_a_redacted_workspace_exception() await ctx.SeedBareRemoteWithOneCommitAsync(); // Clone the real remote with a token (so the local commit succeeds), THEN destroy the bare remote so - // the push fails ("does not appear to be a git repository") — the handle re-injects the token into the - // failing push URL, so the surfaced WorkspaceException must redact it. + // the push fails ("does not appear to be a git repository") — the handle carries the token into the + // failing push, in its environment, so the surfaced WorkspaceException must not name it. await using var handle = await ctx.CloneWithTokenAsync(); await File.WriteAllTextAsync(Path.Combine(handle.Directory, "agent-change.txt"), "x"); ctx.DestroyBareRemote(); @@ -148,7 +149,7 @@ public async Task Auth_failure_surfaces_a_redacted_workspace_exception() await Push(handle).PushChangesAsync(ctx.BranchName, CancellationToken.None)); ex.Message.ShouldNotContain(PushTestContext.Token, Case.Insensitive, "the token literal must never leak into the surfaced error"); - ex.Message.ShouldContain("***", customMessage: "the token is replaced with the redaction marker"); + ex.Message.ShouldContain($"git push --force {ctx.RemoteUrl} ", Case.Sensitive, "the failing push names the remote by its URL alone"); } [Fact] @@ -229,7 +230,7 @@ public PushTestContext() _bareRemote = Path.Combine(_root, "remote.git"); } - private string RemoteUrl => new Uri(_bareRemote).AbsoluteUri; + public string RemoteUrl => new Uri(_bareRemote).AbsoluteUri; /// A bare repo is the "remote"; seed it via a throwaway working clone so it has a default branch + one commit. public async Task SeedBareRemoteWithOneCommitAsync() @@ -250,13 +251,13 @@ public async Task SeedBareRemoteWithOneCommitAsync() public Task CloneWithTokenAsync() => // A file:// remote ignores the token; the point is that the handle CARRIES a token, so PushChangesAsync - // takes the authenticated path (re-injecting it into the push argv) rather than short-circuiting. + // takes the authenticated path (carrying it in the push's environment) rather than short-circuiting. NewProvider().PrepareAsync(WorkspaceProvisionRequest.FromSingle(new WorkspaceRequest { RepositoryUrl = RemoteUrl, Token = Token, TokenUsername = "x-access-token" }), CancellationToken.None); public Task CloneAnonymousAsync() => NewProvider().PrepareAsync(WorkspaceProvisionRequest.FromSingle(new WorkspaceRequest { RepositoryUrl = RemoteUrl }), CancellationToken.None); - /// Delete the bare remote AFTER the clone so a subsequent push to it fails — the failing push URL embeds the token, exercising the redaction path. + /// Delete the bare remote AFTER the clone so a subsequent push to it fails — a tokened push failing, whose surfaced error must carry no token. public void DestroyBareRemote() => Directory.Delete(_bareRemote, recursive: true); /// Simulate a harness that COMMITS its own work in the clone (clean tree afterward), so the push must detect committed changes via the base-SHA diff, not only freshly-staged ones. diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/GitPublishRemoteFixture.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/GitPublishRemoteFixture.cs index fb8403969..3e7a71ab6 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/GitPublishRemoteFixture.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/GitPublishRemoteFixture.cs @@ -30,6 +30,14 @@ internal sealed class GitPublishRemoteFixture : IAsyncDisposable private readonly List _requests = new(); private readonly object _gate = new(); private Task? _accept; + private HttpListener? _harvester; + private readonly List _harvests = new(); + private readonly List _harvestedAuthorizations = new(); + private Task? _harvest; + private string _harvesterAuthority = ""; + private int _harvesterRequests; + private int _lfsBatchRequests; + private int _lfsBatchRefusedCredentials; public string Root { get; } = Directory.CreateTempSubdirectory("cs-pub-remote-").FullName; public string Remote => Path.Combine(Root, "remote.git"); @@ -44,6 +52,12 @@ internal sealed class GitPublishRemoteFixture : IAsyncDisposable /// Count of authenticated git-receive-pack requests — a push that validated the real credential. public int AuthenticatedPushRequests { get; private set; } + /// Requests to the LFS batch endpoint, authorized or refused — how many times git-lfs asked. + public int LfsBatchRequests => Volatile.Read(ref _lfsBatchRequests); + + /// LFS batch requests that offered a credential this remote refused — how many times a refused credential was put forward. + public int LfsBatchRefusedCredentials => Volatile.Read(ref _lfsBatchRefusedCredentials); + /// OIDs the remote received over the LFS upload endpoint. public List UploadedLfsOids { get; } = new(); @@ -53,6 +67,24 @@ internal sealed class GitPublishRemoteFixture : IAsyncDisposable /// True if any request to the remote carried the agent-injected . public bool SawHostileHeader { get; private set; } + /// A pause before each git response, so a clone runs long enough for a test to watch its files while it does. + public TimeSpan ResponseDelay { get; set; } + + /// + /// When set, git's ref advertisement (a GET of …/info/refs) is answered, before any authentication, with a 302 to + /// the same path and query on the harvester (): a remote that redirects to another authority. + /// + public bool RedirectsToHarvester { get; set; } + + /// Requests the harvester served — non-zero once git followed the redirect to it. + public int HarvesterRequests => Volatile.Read(ref _harvesterRequests); + + /// Every Authorization header the harvester was sent. + public IReadOnlyList HarvestedAuthorizations { get { lock (_gate) return _harvestedAuthorizations.ToList(); } } + + /// The Authorization header that carries , as git sends it. + public static string TokenAuthorization => "Basic " + Convert.ToBase64String(Encoding.UTF8.GetBytes($"x-access-token:{FakeToken}")); + public async Task StartAsync() { await GitAsync(Root, new[] { "init", "--bare", "-b", "main", Remote }); @@ -76,23 +108,42 @@ public async Task StartAsync() await PublishSeedAsync(); + (_listener, var port) = ListenOnAFreePort(); + Url = $"http://127.0.0.1:{port}/remote.git"; + + _accept = AcceptAsync(_listener, _requests, ServeAsync); + } + + /// + /// Start the harvester: another loopback authority (a port of its own) that serves this remote's ref advertisement + /// anonymously — so git, redirected there by , adopts it as the remote's new base — and + /// answers every other request with a 401 asking for Basic credentials, recording each Authorization it is sent. + /// + public void StartHarvester() + { + (_harvester, var port) = ListenOnAFreePort(); + _harvesterAuthority = $"http://127.0.0.1:{port}"; + + _harvest = AcceptAsync(_harvester, _harvests, ServeHarvesterAsync); + } + + /// A started listener on a loopback port nothing else holds. + private static (HttpListener Listener, int Port) ListenOnAFreePort() + { for (var attempt = 0; ; attempt++) { using var probe = new TcpListener(IPAddress.Loopback, 0); probe.Start(); var port = ((IPEndPoint)probe.LocalEndpoint).Port; probe.Stop(); - Url = $"http://127.0.0.1:{port}/remote.git"; // A failed Start closes the listener for good (Prefixes then throws ObjectDisposedException), so each attempt // at a fresh port needs a fresh listener. - if (attempt > 0) _listener = new HttpListener(); - _listener.Prefixes.Add($"http://127.0.0.1:{port}/"); - try { _listener.Start(); break; } + var listener = new HttpListener(); + listener.Prefixes.Add($"http://127.0.0.1:{port}/"); + try { listener.Start(); return (listener, port); } catch (HttpListenerException) when (attempt < 4) { } } - - _accept = AcceptAsync(); } /// Commit (repo-relative path → content) on top of main and publish it — content a test clones back through the remote, such as a pack's agents and skills. @@ -184,14 +235,14 @@ private async Task PublishSeedAsync() await GitAsync(Seed, new[] { "push", "--force", Remote, "main" }); } - private async Task AcceptAsync() + private async Task AcceptAsync(HttpListener listener, List requests, Func serve) { try { while (!_stopping.IsCancellationRequested) { - var context = await _listener.GetContextAsync().WaitAsync(_stopping.Token); - _requests.Add(ServeAsync(context)); + var context = await listener.GetContextAsync().WaitAsync(_stopping.Token); + requests.Add(serve(context)); } } catch (OperationCanceledException) when (_stopping.IsCancellationRequested) { } @@ -209,14 +260,43 @@ private async Task ServeAsync(HttpListenerContext context) if (path.EndsWith("/info/lfs/objects/batch", StringComparison.Ordinal)) { await ServeLfsBatchAsync(context); return; } if (path.Contains("/lfs-object/", StringComparison.Ordinal)) { await ServeLfsObjectAsync(context, path); return; } + if (RedirectsToHarvester && IsRefAdvertisement(context, path)) { RedirectToHarvester(context, path); return; } + + if (ResponseDelay > TimeSpan.Zero) await Task.Delay(ResponseDelay, _stopping.Token); await ServeGitAsync(context, path); } finally { context.Response.Close(); } } - private static bool AuthOk(HttpListenerContext context) => - string.Equals(context.Request.Headers["Authorization"], "Basic " + Convert.ToBase64String(Encoding.UTF8.GetBytes($"x-access-token:{FakeToken}")), StringComparison.Ordinal); + /// The harvester: the ref advertisement served anonymously, anything else a 401 that asks for Basic credentials — each Authorization it is sent recorded first. + private async Task ServeHarvesterAsync(HttpListenerContext context) + { + try + { + Interlocked.Increment(ref _harvesterRequests); + if (context.Request.Headers["Authorization"] is { } authorization) lock (_gate) _harvestedAuthorizations.Add(authorization); + + var path = context.Request.Url!.AbsolutePath; + + if (IsRefAdvertisement(context, path)) { await ServeGitAsync(context, path, anonymous: true); return; } + + await context.Request.InputStream.CopyToAsync(Stream.Null, _stopping.Token); + Unauthorized(context); + } + finally { context.Response.Close(); } + } + + private static bool IsRefAdvertisement(HttpListenerContext context, string path) => context.Request.HttpMethod == "GET" && path.EndsWith("/info/refs", StringComparison.Ordinal); + + /// A 302 to the same path and query on the harvester's authority — the shape git needs to adopt it as the remote's new base. + private void RedirectToHarvester(HttpListenerContext context, string path) + { + context.Response.StatusCode = 302; + context.Response.RedirectLocation = _harvesterAuthority + path + context.Request.Url!.Query; + } + + private static bool AuthOk(HttpListenerContext context) => string.Equals(context.Request.Headers["Authorization"], TokenAuthorization, StringComparison.Ordinal); private void Unauthorized(HttpListenerContext context) { @@ -226,6 +306,10 @@ private void Unauthorized(HttpListenerContext context) private async Task ServeLfsBatchAsync(HttpListenerContext context) { + Interlocked.Increment(ref _lfsBatchRequests); + + if (!AuthOk(context) && context.Request.Headers["Authorization"] is not null) Interlocked.Increment(ref _lfsBatchRefusedCredentials); + if (!AuthOk(context)) { Unauthorized(context); return; } using var reader = new StreamReader(context.Request.InputStream); @@ -278,11 +362,11 @@ private async Task ServeLfsObjectAsync(HttpListenerContext context, string path) await context.Response.OutputStream.WriteAsync(bytes); } - private async Task ServeGitAsync(HttpListenerContext context, string path) + private async Task ServeGitAsync(HttpListenerContext context, string path, bool anonymous = false) { var isPush = path.EndsWith("/git-receive-pack", StringComparison.Ordinal) || context.Request.QueryString["service"] == "git-receive-pack"; - if (isPush || AuthenticateReads) + if (!anonymous && (isPush || AuthenticateReads)) { if (!AuthOk(context)) { Unauthorized(context); return; } if (isPush) lock (_gate) AuthenticatedPushRequests++; @@ -363,10 +447,12 @@ public async ValueTask DisposeAsync() { _stopping.Cancel(); _listener.Close(); + _harvester?.Close(); try { if (_accept is not null) await _accept; - await Task.WhenAll(_requests); + if (_harvest is not null) await _harvest; + await Task.WhenAll(_requests.Concat(_harvests)); } finally { _stopping.Dispose(); try { Directory.Delete(Root, recursive: true); } catch { /* best-effort */ } } } diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/LocalGitBranchIntegratorFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/LocalGitBranchIntegratorFlowTests.cs index 3ecf2b3ab..6db71cd62 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/LocalGitBranchIntegratorFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/LocalGitBranchIntegratorFlowTests.cs @@ -586,8 +586,6 @@ public async Task The_integration_clone_is_removed_on_both_clean_and_conflict_pa using var ctx = new IntegratorTestContext(); var baseSha = await ctx.SeedBaseAsync(new() { ["f.txt"] = "shared\n" }); - var before = CountIntegrationClones(); - var clean = await ctx.MakeContributionAsync("agent-a", baseSha, d => File.WriteAllText(Path.Combine(d, "f.txt"), "clean\n")); await ctx.NewIntegrator().IntegrateAsync(ctx.Request(baseSha, clean), CancellationToken.None); @@ -595,7 +593,7 @@ public async Task The_integration_clone_is_removed_on_both_clean_and_conflict_pa var conflictB = await ctx.MakeContributionAsync("agent-b", baseSha, d => File.WriteAllText(Path.Combine(d, "f.txt"), "y\n")); await ctx.NewIntegrator().IntegrateAsync(ctx.Request(baseSha, conflictA, conflictB), CancellationToken.None); - CountIntegrationClones().ShouldBe(before, "no integrate-* clone lingers after a clean OR a conflict run"); + CountIntegrationClones(ctx).ShouldBe(0, "no integrate-* clone lingers after a clean OR a conflict run"); } // ── Empty request → Empty ──────────────────────────────────────────────────────── @@ -616,11 +614,18 @@ public async Task An_empty_request_is_empty() // ── Helpers ────────────────────────────────────────────────────────────────────── - private static int CountIntegrationClones() => + /// The integrate-* clones of 's remote under the worker-wide root — only this test's own: other tests integrate under the same root in parallel, so a count of every clone races theirs. + private static int CountIntegrationClones(IntegratorTestContext ctx) => Directory.Exists(LocalGitWorkspaceProvider.WorkspacesRoot) - ? Directory.EnumerateDirectories(LocalGitWorkspaceProvider.WorkspacesRoot, "integrate-*").Count() + ? Directory.EnumerateDirectories(LocalGitWorkspaceProvider.WorkspacesRoot, "integrate-*").Count(clone => ClonesRemote(clone, ctx.RemoteUrl)) : 0; + private static bool ClonesRemote(string clone, string remoteUrl) + { + try { return File.ReadAllText(Path.Combine(clone, ".git", "config")).Contains(remoteUrl, StringComparison.Ordinal); } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { return false; } + } + private static async Task GitReadyAsync() { if (OperatingSystem.IsWindows()) return false; @@ -665,7 +670,7 @@ public IntegratorTestContext() _bare = Path.Combine(_root, "remote.git"); } - private string RemoteUrl => new Uri(_bare).AbsoluteUri; + public string RemoteUrl => new Uri(_bare).AbsoluteUri; public IBranchIntegrator NewIntegrator(IArtifactOffloader? offloader = null) => new LocalGitBranchIntegrator(new SandboxRunnerRegistry(new ISandboxRunner[] { new LocalProcessRunner() }), offloader ?? new FakeOffloader(), NullLogger.Instance); diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/TokenedGitCredentialHelperFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/TokenedGitCredentialHelperFlowTests.cs index 54d77fff6..32ac799be 100644 --- a/backend/tests/CodeSpace.IntegrationTests/Workflows/TokenedGitCredentialHelperFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/TokenedGitCredentialHelperFlowTests.cs @@ -1,30 +1,48 @@ +using System.Diagnostics; +using CodeSpace.Core.Services.Agents.Eval.Benchmark; +using CodeSpace.Core.Services.Agents.Eval.Benchmark.Graders; using CodeSpace.Core.Services.Agents.Sandbox; using CodeSpace.Core.Services.Agents.Sandbox.Runners; using CodeSpace.Core.Services.Agents.Workspace; using CodeSpace.Core.Services.Agents.Workspace.Integrators; using CodeSpace.Core.Services.Agents.Workspace.Providers; +using CodeSpace.Core.Services.Supervisor; using CodeSpace.Core.Services.Workflows.Artifacts; using CodeSpace.Messages.Agents; +using CodeSpace.Messages.Agents.Benchmark; using Microsoft.Extensions.Logging.Abstractions; using Shouldly; namespace CodeSpace.IntegrationTests.Workflows; /// -/// HIGH fidelity (Rule 12): the REAL , and -/// on the real , with real git and -/// git-lfs, against a loopback smart-HTTP remote () that demands a FAKE token -/// for every request, read or write. The operator's host is simulated by a scratch HOME whose global config -/// (GIT_CONFIG_GLOBAL, with system config off) sets credential.helper=store --file=<scratch> plus a -/// second store scoped to the remote's URL — the setup under which every tokened run used to leave its token on disk. +/// HIGH fidelity (Rule 12): the REAL , , +/// and on the real +/// , with real git and git-lfs, against a loopback smart-HTTP remote +/// () that demands a FAKE token for every request, read or write. The operator's host +/// is simulated by a scratch HOME whose global config (GIT_CONFIG_GLOBAL, with system config off) sets +/// credential.helper=store --file=<scratch> plus a second store scoped to the remote's URL — the setup under +/// which a tokened run would leave its token on disk. /// -/// Each tokened operation must authenticate with the token in its URL and leave both store files without it. The -/// positive control runs the same production code with ONE named git subcommand as it ran before tokened commands were -/// marked — no credential-helper reset, trace2 on: that store then holds the token, so its absence in the real run is the -/// reset's doing, not a helper that never ran. An untokened clone must still authenticate through the operator's helper, -/// and a tokened one must still reach the remote through a proxy whose password that helper holds: the reset covers the -/// tokened remote only. The operator's trace2 targets must record no tokened command, and the LFS downloads an -/// integration makes through its tokened origin — a checkout and an apply that run as written — must store nothing. +/// A tokened command names the remote by a URL without userinfo and carries the token in its environment, where a +/// credential helper scoped to the remote answers for it. Each tokened operation must authenticate that way while no argv +/// the runner is handed carries the token or a URL's userinfo, no file under the clone or the publish repo holds it at any +/// point while the operation runs (a watch scans them throughout, against a deliberately slow remote), and both store files +/// stay without it. The positive controls: the same production code with ONE named git subcommand run without the +/// credential-helper reset and with trace2 on leaves the token in that store, so its absence in the real run is the reset's +/// doing; and a raw clone given the token, in the URL or in a Basic header, leaves it where the watch finds it. +/// +/// A remote that redirects to another authority must get no credential sent there: a harvester at that authority asks +/// for one, and the operation fails instead. Its positive control is the same redirect followed with the token in the URL, +/// as git was handed it before: the harvester collects it. An untokened clone must still authenticate through the +/// operator's helper, and a tokened one must still reach the remote through a proxy whose password that helper holds: the +/// reset covers the tokened remote only. The operator's trace2 targets must record no credential even when they name the +/// credential variables, and the LFS downloads an integration or a grade makes through its origin must store nothing. +/// +/// A token the remote refuses must fail an LFS command at once, with the reason, after a handful of requests; its +/// positive control is the helper ignoring erase, which keeps git-lfs retrying until the command is killed. The operator's +/// url.<base>.insteadOf and pushInsteadOf rules must move no tokened command; their positive control is +/// the same command without the pin that maps the remote to itself, which they move. /// /// Each test owns its remote, proxy and temp tree and removes them on every path; it skips on Windows or without /// git, and the LFS rows without git-lfs. Nothing reads or writes the real global config, system config or keychain. @@ -34,22 +52,30 @@ public sealed class TokenedGitCredentialHelperFlowTests { private const string ReadmePatch = "diff --git a/README.md b/README.md\n--- a/README.md\n+++ b/README.md\n@@ -1 +1 @@\n-base, revised\n+integrated\n"; + /// Each git response waits this long, so a clone or a push runs for long enough to be watched while it does. + private static readonly TimeSpan SlowRemote = TimeSpan.FromMilliseconds(150); + [Theory] [InlineData(null)] // every tokened command carries the reset [InlineData("ls-remote")] // positive controls: the soft-ref probe without it, [InlineData("clone")] // the clone, - [InlineData("fetch")] // or the pin's fetch rungs through the still-tokened origin - public async Task Provisioning_a_tokened_workspace_leaves_no_token_in_the_operators_credential_store(string? unreset) + [InlineData("fetch")] // or the pin's fetch rungs through origin + public async Task Provisioning_a_tokened_workspace_leaves_no_token_on_an_argv_on_disk_or_in_the_operators_credential_store(string? unreset) { if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; await using var ctx = await OperatorHostContext.StartAsync(); ctx.Runner.Unreset = unreset; + ctx.Remote.ResponseDelay = SlowRemote; + await using var watch = ctx.WatchTheWorkspaces(); await using var handle = await ctx.ProvisionAsync(GitPublishRemoteFixture.FakeToken); + await watch.StopAsync(); - handle.Repositories.Single().BaseSha.ShouldBe(ctx.PinnedSha, "the probe, the clone and the pin's fetch all authenticated with the URL's token"); + handle.Repositories.Single().BaseSha.ShouldBe(ctx.PinnedSha, "the probe, the clone and the pin's fetch all authenticated with the token"); ctx.Runner.Ran(unreset ?? "fetch").ShouldBeTrue("fixture check: the command under test ran"); + ctx.ShouldKeepTheTokenOffEveryArgv(); + ShouldNeverHaveSeenTheToken(watch, "the clone"); ctx.ShouldHoldTheToken(stored: unreset is not null); } @@ -58,7 +84,7 @@ public async Task Provisioning_a_tokened_workspace_leaves_no_token_in_the_operat [InlineData("lfs")] // positive controls: the LFS upload without the reset, [InlineData("push")] // the push, [InlineData("ls-remote")] // or the readback - public async Task Publishing_leaves_no_token_in_the_operators_credential_store(string? unreset) + public async Task Publishing_leaves_no_token_on_an_argv_on_disk_or_in_the_operators_credential_store(string? unreset) { if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; @@ -72,20 +98,25 @@ public async Task Publishing_leaves_no_token_in_the_operators_credential_store(s var oid = lfs ? await ctx.AgentCommitsLfsFileAsync(handle.Directory) : null; await ctx.AgentCommitsAsync(handle.Directory); ctx.Runner.Unreset = unreset; + ctx.Remote.ResponseDelay = SlowRemote; + await using var watch = ctx.WatchTheWorkspaces(); (await ((IWorkspacePushHandle)handle).PushChangesAsync(ctx.BranchName, CancellationToken.None)).ShouldBe(ctx.BranchName); + await watch.StopAsync(); - ctx.Remote.AuthenticatedPushRequests.ShouldBeGreaterThan(0, "the push authenticated with the URL's token"); + ctx.Remote.AuthenticatedPushRequests.ShouldBeGreaterThan(0, "the push authenticated with the token"); ((IWorkspacePushHandle)handle).LastPushedCommitSha().ShouldBe(await ctx.RemoteShaAsync(ctx.BranchName), "the readback authenticated and confirmed the pushed tip"); - if (oid is not null) ctx.Remote.HasLfsObject(oid).ShouldBeTrue("the LFS upload authenticated with the URL's token"); + if (oid is not null) ctx.Remote.HasLfsObject(oid).ShouldBeTrue("the LFS upload authenticated with the token"); ctx.Runner.Ran(unreset ?? "push").ShouldBeTrue("fixture check: the command under test ran"); + ctx.ShouldKeepTheTokenOffEveryArgv(); + ShouldNeverHaveSeenTheToken(watch, "the publish repo"); ctx.ShouldHoldTheToken(stored: unreset is not null); } [Theory] [InlineData(null)] [InlineData("ls-remote")] - public async Task Resolving_the_launch_base_leaves_no_token_in_the_operators_credential_store(string? unreset) + public async Task Resolving_the_launch_base_leaves_no_token_on_an_argv_or_in_the_operators_credential_store(string? unreset) { if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; @@ -97,36 +128,43 @@ public async Task Resolving_the_launch_base_leaves_no_token_in_the_operators_cre RepositoryUrl = ctx.Remote.Url, Token = GitPublishRemoteFixture.FakeToken, TokenUsername = "x-access-token", Ref = "main", }, refRequired: true, CancellationToken.None); - tip.ShouldBe(ctx.Remote.BaseSha, "the probe authenticated with the URL's token"); + tip.ShouldBe(ctx.Remote.BaseSha, "the probe authenticated with the token"); + ctx.ShouldKeepTheTokenOffEveryArgv(); ctx.ShouldHoldTheToken(stored: unreset is not null); } [Theory] [InlineData(null)] [InlineData("clone")] // positive controls: the integration clone without the reset, - [InlineData("push")] // or the push through its tokened origin - public async Task Integrating_leaves_no_token_in_the_operators_credential_store(string? unreset) + [InlineData("push")] // or the push through its origin + public async Task Integrating_leaves_no_token_on_an_argv_on_disk_or_in_the_operators_credential_store(string? unreset) { if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; await using var ctx = await OperatorHostContext.StartAsync(); ctx.Runner.Unreset = unreset; + ctx.Remote.ResponseDelay = SlowRemote; + await using var watch = new TokenOnDiskWatch(GitPublishRemoteFixture.FakeToken, LocalGitWorkspaceProvider.WorkspacesRoot, "integrate-"); var result = await ctx.IntegrateAsync(ctx.BranchName, ctx.Remote.BaseSha, ReadmePatch); + await watch.StopAsync(); result.Status.ShouldBe(IntegrationStatus.Clean, result.Reason); - (await ctx.RemoteFileAsync(ctx.BranchName, "README.md")).ShouldBe("integrated\n", "the clone and the push authenticated with the URL's token"); + (await ctx.RemoteFileAsync(ctx.BranchName, "README.md")).ShouldBe("integrated\n", "the clone and the push authenticated with the token"); + ctx.ShouldKeepTheTokenOffEveryArgv(); + ShouldNeverHaveSeenTheToken(watch, "the integration clone"); ctx.ShouldHoldTheToken(stored: unreset is not null); } [Theory] [InlineData(null)] - [InlineData("clone")] // positive control: the clone without the reset — the operator's store is live here - public async Task Integrating_over_lfs_history_downloads_through_the_tokened_origin_and_stores_nothing(string? unreset) + [InlineData("clone")] // positive controls: the clone without the reset, + [InlineData("checkout")] // or the base checkout, whose LFS download asks the helpers through git-lfs + public async Task Integrating_over_lfs_history_downloads_through_origin_and_stores_nothing(string? unreset) { - // The integration clone keeps its tokened origin, and the base checkout and the apply download LFS objects through - // it. git-lfs authenticates those downloads from the origin URL and neither asks nor tells a credential helper, so - // they run as written; only git's own transport — the clone and the push — carries the reset. + // The integration clone's origin carries no credential, so the base checkout and the apply download their LFS objects + // through it with the token in their environment too: git-lfs asks the credential helpers for it, and tells them when + // it worked. Each runs as a tokened command, so only the scoped helper answers and no operator helper stores it. if (OperatingSystem.IsWindows() || !await GitAvailableAsync() || !await GitLfsAvailableAsync()) return; await using var ctx = await OperatorHostContext.StartAsync(); @@ -137,12 +175,70 @@ public async Task Integrating_over_lfs_history_downloads_through_the_tokened_ori var result = await ctx.IntegrateAsync(ctx.BranchName, lfs.BaseSha, LfsPointerPatch(lfs.AtBase, lfs.Unreferenced)); result.Status.ShouldBe(IntegrationStatus.Clean, result.Reason); - (await ctx.RemoteFileAsync(ctx.BranchName, "big.bin")).ShouldBe(lfs.Unreferenced.Pointer, "the clone and the push authenticated with the URL's token"); - ctx.Remote.DownloadedLfsOids.ShouldContain(lfs.AtBase.Oid, "fixture check: the base checkout downloaded its LFS object through the tokened origin"); - ctx.Remote.DownloadedLfsOids.ShouldContain(lfs.Unreferenced.Oid, "fixture check: the apply downloaded the patched LFS object through the tokened origin"); + (await ctx.RemoteFileAsync(ctx.BranchName, "big.bin")).ShouldBe(lfs.Unreferenced.Pointer, "the clone and the push authenticated with the token"); + ctx.Remote.DownloadedLfsOids.ShouldContain(lfs.AtBase.Oid, "fixture check: the base checkout downloaded its LFS object through origin"); + ctx.Remote.DownloadedLfsOids.ShouldContain(lfs.Unreferenced.Oid, "fixture check: the apply downloaded the patched LFS object through origin"); + ctx.ShouldKeepTheTokenOffEveryArgv(); ctx.ShouldHoldTheToken(stored: unreset is not null); } + [Theory] + [InlineData("clone")] // the workspace clone, + [InlineData("push")] // the publish push, + [InlineData("integrate")] // and the integration clone, each redirected by the remote + public async Task A_redirect_to_another_authority_gets_no_credential(string operation) + { + // git follows the redirect of the ref advertisement and makes the new authority the remote's base, so its next + // request goes there. The credential answers for the remote's own scheme and authority only: when the harvester asks, + // no helper answers for it, and the operation fails rather than send the token anywhere. + if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; + + await using var ctx = await OperatorHostContext.StartAsync(); + ctx.Remote.StartHarvester(); + + await Should.ThrowAsync(() => ctx.RedirectedAsync(operation)); + + ctx.Remote.HarvesterRequests.ShouldBeGreaterThan(1, "fixture check: git followed the redirect and asked the harvester for more than the advertisement"); + ctx.Remote.HarvestedAuthorizations.ShouldBeEmpty("a credential reached the authority the remote redirected to"); + ctx.ShouldKeepTheTokenOffEveryArgv(); + ctx.ShouldHoldTheToken(stored: false); + } + + [Fact] + public async Task The_harvester_collects_the_token_a_redirected_url_carries() + { + // Positive control for the redirect row: the same redirect, with the token in the URL as git was handed it before — + // git sends it to the authority the remote redirected to, so the harvester's silence there is the scoped helper's doing. + if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; + + await using var ctx = await OperatorHostContext.StartAsync(); + ctx.Remote.StartHarvester(); + ctx.Remote.RedirectsToHarvester = true; + + await ctx.RawGitAsync(ctx.Root, "clone", ctx.TokenedUrl, Path.Combine(ctx.Root, "raw-clone")); + + ctx.Remote.HarvestedAuthorizations.ShouldContain(GitPublishRemoteFixture.TokenAuthorization, "git sends a token in the URL to the authority the remote redirected to"); + } + + [Theory] + [InlineData("url")] // the token in the URL's userinfo, which git writes as origin before the transfer starts + [InlineData("header")] // or base64-encoded in a Basic Authorization header, which git writes as http.extraHeader + public async Task The_disk_watch_sees_the_token_a_clone_writes_into_its_config(string carrier) + { + // Positive control for the watch, in both forms the token can take on disk. + if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; + + await using var ctx = await OperatorHostContext.StartAsync(); + var clone = Path.Combine(ctx.WorkspacesRoot, "raw-clone"); + var args = carrier == "url" ? new[] { "clone", ctx.TokenedUrl, clone } : new[] { "clone", "--config", $"http.extraHeader=Authorization: {GitPublishRemoteFixture.TokenAuthorization}", ctx.Remote.Url, clone }; + + await using var watch = ctx.WatchTheWorkspaces(); + (await ctx.RawGitAsync(ctx.Root, args)).Status.ShouldBe(SandboxStatus.Success); + await watch.StopAsync(); + + watch.Hits.ShouldContain(Path.Combine(clone, ".git", "config"), "the watch finds the token git wrote"); + } + [Fact] public async Task Tokened_commands_reach_the_remote_through_a_proxy_whose_password_the_operators_helper_holds() { @@ -172,22 +268,24 @@ public async Task Tokened_commands_reach_the_remote_through_a_proxy_whose_passwo integrated.Status.ShouldBe(IntegrationStatus.Clean, integrated.Reason); proxy.RelayedRequests.ShouldBeGreaterThan(0, "fixture check: git reached the remote through the proxy, not around it"); + ctx.ShouldKeepTheTokenOffEveryArgv(); ctx.ShouldHoldTheToken(stored: false); } [Theory] [InlineData(null)] - [InlineData("clone")] // positive controls: the clone as it ran before tokened commands were marked, + [InlineData("clone")] // positive controls: the clone as it would run with trace2 on, [InlineData("push")] // or the publish push - public async Task The_operators_trace2_targets_record_no_tokened_command(string? unreset) + public async Task The_operators_trace2_targets_record_no_credential(string? unreset) { // git writes every command's argv, and each child's (git remote-http ), to the trace2 targets in system and - // global config — read before any -c can reach them — and git 2.33 writes the URL's password verbatim. A tokened - // command runs with trace2 off, so the operator's targets never see a tokened URL; untokened commands still trace. + // global config — read before any -c or environment config can reach them — and the value of each environment + // variable their trace2.envVars names. A tokened argv names no credential, but trace2.envVars can name anything, so a + // tokened command runs with trace2 off; untokened commands still trace. if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; await using var ctx = await OperatorHostContext.StartAsync(); - ctx.TraceToOperatorTargets(); + ctx.TraceToOperatorTargets(envVars: "CODESPACE_GIT_USERNAME,CODESPACE_GIT_PASSWORD"); ctx.Runner.Unreset = unreset; await using var handle = await ctx.ProvisionAsync(GitPublishRemoteFixture.FakeToken); @@ -196,8 +294,8 @@ public async Task The_operators_trace2_targets_record_no_tokened_command(string? var trace = ctx.OperatorTrace(); trace.ShouldContain("rev-parse", Case.Sensitive, "fixture check: the operator's trace2 targets are live, and untokened commands still trace"); - trace.Contains("x-access-token:", StringComparison.Ordinal).ShouldBe(unreset is not null, "a tokened URL (redacted or not, it keeps its username) in the operator's trace2 targets"); - if (unreset is null) trace.ShouldNotContain(GitPublishRemoteFixture.FakeToken); + trace.ShouldNotContain("@127.0.0.1", Case.Sensitive, "no traced argv names the remote by a URL with userinfo"); + trace.Contains(GitPublishRemoteFixture.FakeToken, StringComparison.Ordinal).ShouldBe(unreset is not null, unreset is not null ? "positive control: with trace2 on, trace2.envVars records the token" : "the token reached the operator's trace2 targets"); } [Fact] @@ -215,18 +313,166 @@ public async Task An_untokened_clone_still_authenticates_through_the_operators_h await using var handle = await ctx.Provider.PrepareAsync(untokened, CancellationToken.None); File.ReadAllText(Path.Combine(handle.Directory, "README.md")).ShouldBe("base, revised\n", "the clone authenticated through the operator's store helper"); - ctx.Runner.Specs.ShouldAllBe(s => !s.Args.Any(a => IsHelperReset(a)) && !s.Environment.Keys.Any(k => TraceOff.Contains(k)), "an untokened command keeps the operator's helpers and trace2"); + ctx.Runner.Specs.ShouldAllBe(s => TokenedGitControls.RunsUntokened(s), "an untokened command keeps the operator's helpers and trace2"); } - private static Task GitAvailableAsync() => ToolAvailableAsync(new[] { "--version" }); + [Theory] + [InlineData(null)] + [InlineData("clone")] // positive control: the grader's clone without the reset + public async Task Grading_leaves_no_token_on_an_argv_on_disk_or_in_the_operators_credential_store(string? unreset) + { + // The acceptance grader clones the base itself — a base sha is no ref the provider's clone takes — then checks it out, + // applies the candidate's patch and runs the check in that clone. Every command that reaches the remote carries the + // token in its environment alone. + if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; - private static Task GitLfsAvailableAsync() => ToolAvailableAsync(new[] { "lfs", "version" }); + await using var ctx = await OperatorHostContext.StartAsync(); + ctx.Runner.Unreset = unreset; + ctx.Remote.ResponseDelay = SlowRemote; + + await using var watch = new TokenOnDiskWatch(GitPublishRemoteFixture.FakeToken, LocalGitWorkspaceProvider.WorkspacesRoot, "grade-"); + var grade = await ctx.GradePatchAsync(ctx.Remote.BaseSha, ReadmePatch, "test \"$(cat README.md)\" = integrated"); + await watch.StopAsync(); + + grade.Passed.ShouldBeTrue(grade.Detail); + ctx.ShouldKeepTheTokenOffEveryArgv(); + ShouldNeverHaveSeenTheToken(watch, "the grading clone"); + ctx.ShouldHoldTheToken(stored: unreset is not null); + } + + [Theory] + [InlineData(null)] + [InlineData("clone")] // positive controls: the clone without the reset, + [InlineData("checkout")] // the base checkout, whose LFS download asks the helpers through git-lfs, + [InlineData("apply")] // or the apply, which downloads the object the patch points at + public async Task Grading_at_an_lfs_base_downloads_through_origin_and_stores_nothing(string? unreset) + { + // The grading clone's origin carries no credential, so the base checkout and the apply download their LFS objects + // through it with the token in their environment: git-lfs asks the credential helpers for it, and tells them when it + // worked. Untokened, the checkout would find no credential, and the grade would fail as an unknown base. + if (OperatingSystem.IsWindows() || !await GitAvailableAsync() || !await GitLfsAvailableAsync()) return; + + await using var ctx = await OperatorHostContext.StartAsync(); + var lfs = await ctx.Remote.AddLfsHistoryAsync(); + ctx.InstallLfsFilters(); + ctx.Runner.Unreset = unreset; + + var grade = await ctx.GradePatchAsync(lfs.BaseSha, LfsPointerPatch(lfs.AtBase, lfs.Unreferenced), "test \"$(cat big.bin)\" = \"lfs payload v3\""); + + grade.Passed.ShouldBeTrue(grade.Detail); + ctx.Remote.DownloadedLfsOids.ShouldContain(lfs.AtBase.Oid, "fixture check: the base checkout downloaded its LFS object through origin"); + ctx.Remote.DownloadedLfsOids.ShouldContain(lfs.Unreferenced.Oid, "fixture check: the apply downloaded the patched LFS object through origin"); + ctx.Runner.Ran(unreset ?? "apply").ShouldBeTrue("fixture check: the command under test ran"); + ctx.ShouldKeepTheTokenOffEveryArgv(); + ctx.ShouldHoldTheToken(stored: unreset is not null); + } + + [Theory] + [InlineData("publish", true)] + [InlineData("clone", true)] + [InlineData("publish", false)] // positive controls: with the helper ignoring erase, git-lfs retries the refused token + [InlineData("clone", false)] // until the command is killed + public async Task A_refused_token_fails_an_lfs_command_at_once(string operation, bool recordsRefusal) + { + // A token the remote refuses — one past its lifetime, a revoked one — makes git-lfs tell the helpers to erase it and + // ask them again, with no limit; git itself stops after one refusal. The helper records the refusal and answers no + // more, so the LFS upload of a publish, or the LFS download of a clone's checkout, fails at once with the reason — + // not after its whole timeout, tens of failed logins a second against the provider. + if (OperatingSystem.IsWindows() || !await GitAvailableAsync() || !await GitLfsAvailableAsync()) return; + + await using var ctx = await OperatorHostContext.StartAsync(authenticateReads: false); + ctx.Runner.HelperIgnoresErase = !recordsRefusal; + ctx.Runner.TimeoutCapSeconds = recordsRefusal ? null : 10; + + var elapsed = Stopwatch.StartNew(); + var failure = await Should.ThrowAsync(() => ctx.WithARefusedTokenAsync(operation)); + elapsed.Stop(); + + ctx.Runner.Ran("lfs").ShouldBe(operation == "publish", $"fixture check: the {operation} itself failed"); + + if (!recordsRefusal) + { + // How long git-lfs keeps asking differs by version (3.7.1 retries until the command is killed; the one CI's + // runner ships gave up after a few requests); each asks the helpers again after erasing, so count the re-offers. + ctx.Remote.LfsBatchRefusedCredentials.ShouldBeGreaterThan(1, "positive control: a helper that answers every get offers the refused token again after git-lfs erased it"); + return; + } + + elapsed.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(20), $"the refused token was retried instead of failing the {operation}"); + ctx.Remote.LfsBatchRefusedCredentials.ShouldBe(1, "the refused token was offered once and never again"); + ctx.Remote.LfsBatchRequests.ShouldBeInRange(1, 10, "fixture check: git-lfs asked the remote, and stopped once it refused"); + failure.Message.ShouldContain("the remote refused this credential", Case.Sensitive, "the failure says why"); + ctx.ShouldKeepTheTokenOffEveryArgv(OperatorHostContext.RefusedToken); + } + + [Fact] + public async Task Operator_url_rewrites_never_move_a_tokened_remote() + { + // The operator's global config sends the remote's authority to a local mirror for fetches and to a sink for pushes — + // the shape of url."git@host:".insteadOf=https://host/, or a rule carrying an operator's own token. A tokened command's + // credential is bound to its remote, so the command maps that URL to itself, the longest prefix any rule can match: + // the launch-base probe, the clone and its pin, the publish (LFS included) and the integration all reach the remote. + if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; + + var lfs = await GitLfsAvailableAsync(); + await using var ctx = await OperatorHostContext.StartAsync(); + using var sink = new LoopbackSink(); + var mirrorSha = await ctx.RewriteTheRemoteAsync(sink); + + (await ctx.ResolveLaunchBaseAsync()).ShouldBe(ctx.Remote.BaseSha, "the launch-base probe reached the remote, not the mirror"); + + await using var handle = await ctx.ProvisionAsync(GitPublishRemoteFixture.FakeToken); + handle.Repositories.Single().BaseSha.ShouldBe(ctx.PinnedSha, "the soft-ref probe, the clone and the pin's fetch reached the remote"); + + var oid = lfs ? await ctx.AgentCommitsLfsFileAsync(handle.Directory) : null; + await ctx.AgentCommitsAsync(handle.Directory); + (await ((IWorkspacePushHandle)handle).PushChangesAsync(ctx.BranchName, CancellationToken.None)).ShouldBe(ctx.BranchName); + ((IWorkspacePushHandle)handle).LastPushedCommitSha().ShouldBe(await ctx.RemoteShaAsync(ctx.BranchName), "the push and its readback reached the remote"); + if (oid is not null) ctx.Remote.HasLfsObject(oid).ShouldBeTrue("the LFS upload reached the remote"); + + var integrated = await ctx.IntegrateAsync("codespace/integration/" + Guid.NewGuid().ToString("N"), ctx.Remote.BaseSha, ReadmePatch); + integrated.Status.ShouldBe(IntegrationStatus.Clean, integrated.Reason); + + mirrorSha.ShouldNotBe(ctx.Remote.BaseSha, "fixture check: the mirror's history is not the remote's"); + sink.Connections.ShouldBe(0, "a push was rewritten to the operator's sink"); + ctx.ShouldKeepTheTokenOffEveryArgv(); + ctx.ShouldHoldTheToken(stored: false); + } + + [Fact] + public async Task The_operators_url_rewrites_move_a_tokened_command_without_the_pin() + { + // Positive control for the rewrite row: the same rules move the same tokened commands once the pin is taken from + // their environment — the launch-base probe to the mirror, the publish push to the sink — as they moved every + // tokened command once the token left the URL, and as they move any untokened one. + if (OperatingSystem.IsWindows() || !await GitAvailableAsync()) return; + + await using var ctx = await OperatorHostContext.StartAsync(); + using var sink = new LoopbackSink(); + var mirrorSha = await ctx.RewriteTheRemoteAsync(sink); + + ctx.Runner.Unpinned = "ls-remote"; + (await ctx.ResolveLaunchBaseAsync()).ShouldBe(mirrorSha, "positive control: without the pin the probe goes to the mirror"); + + ctx.Runner.Unpinned = null; + await using var handle = await ctx.ProvisionAsync(GitPublishRemoteFixture.FakeToken); + await ctx.AgentCommitsAsync(handle.Directory); + + ctx.Runner.Unpinned = "push"; + await Should.ThrowAsync(() => ((IWorkspacePushHandle)handle).PushChangesAsync(ctx.BranchName, CancellationToken.None)); - /// The environment a tokened command runs with, which turns off every trace2 target. - private static readonly string[] TraceOff = { "GIT_TRACE2", "GIT_TRACE2_EVENT", "GIT_TRACE2_PERF" }; + sink.Connections.ShouldBeGreaterThan(0, "positive control: without the pin the push goes to the sink"); + } + + private static void ShouldNeverHaveSeenTheToken(TokenOnDiskWatch watch, string what) + { + watch.Scans.ShouldBeGreaterThan(10, $"fixture check: the watch was scanning while {what} was written"); + watch.Hits.ShouldBeEmpty($"the token reached the disk under {what} at some point while it ran"); + } + + private static Task GitAvailableAsync() => ToolAvailableAsync(new[] { "--version" }); - /// A config that empties the credential helper list — credential.helper= or one scoped to a URL. - private static bool IsHelperReset(string arg) => arg.StartsWith("credential.", StringComparison.Ordinal) && arg.EndsWith(".helper=", StringComparison.Ordinal); + private static Task GitLfsAvailableAsync() => ToolAvailableAsync(new[] { "lfs", "version" }); /// An agent's patch repointing big.bin from one LFS object to another, as a capture records it: the pointer text. private static string LfsPointerPatch(LfsObject from, LfsObject to) => @@ -249,36 +495,46 @@ private static string Subcommand(IReadOnlyList args) /// The remote, a scratch operator host (HOME, global config with the two store helpers, system config off) and the production classes running on it. private sealed class OperatorHostContext : IAsyncDisposable { - private readonly string _root = Path.Combine(Path.GetTempPath(), "cs-credstore-" + Guid.NewGuid().ToString("N")); - private readonly Dictionary _environment; - private OperatorHostContext() + private OperatorHostContext(bool authenticateReads) { + Remote = new GitPublishRemoteFixture { AuthenticateReads = authenticateReads }; Directory.CreateDirectory(Home); _environment = new Dictionary { ["HOME"] = Home, ["GIT_CONFIG_GLOBAL"] = GlobalConfig, ["GIT_CONFIG_NOSYSTEM"] = "1" }; Runner = new OperatorConfigRunner(_environment); Registry = new SandboxRunnerRegistry(new ISandboxRunner[] { Runner }); - Provider = new LocalGitWorkspaceProvider(Registry, NullLogger.Instance, Path.Combine(_root, "workspaces")); + Provider = new LocalGitWorkspaceProvider(Registry, NullLogger.Instance, WorkspacesRoot); } - public GitPublishRemoteFixture Remote { get; } = new() { AuthenticateReads = true }; + public string Root { get; } = Path.Combine(Path.GetTempPath(), "cs-credstore-" + Guid.NewGuid().ToString("N")); + public GitPublishRemoteFixture Remote { get; } public OperatorConfigRunner Runner { get; } public SandboxRunnerRegistry Registry { get; } public LocalGitWorkspaceProvider Provider { get; } public string BranchName { get; } = "codespace/agent/" + Guid.NewGuid().ToString("N"); + /// A token the remote refuses: not . + public const string RefusedToken = "refused-token-0123456789"; + + /// The provider's own workspaces root: its clones and its publish repos. + public string WorkspacesRoot => Path.Combine(Root, "workspaces"); + + /// The remote's URL with the fake token in its userinfo — the shape git was handed before the token left the URL. + public string TokenedUrl => Remote.Url.Replace("http://", $"http://x-access-token:{GitPublishRemoteFixture.FakeToken}@", StringComparison.Ordinal); + /// The base's parent: absent from a depth-1 clone of the tip, so pinning it walks the fetch rungs through origin. public string PinnedSha { get; private set; } = ""; - private string Home => Path.Combine(_root, "home"); + private string Home => Path.Combine(Root, "home"); private string GlobalConfig => Path.Combine(Home, ".gitconfig"); - private string GlobalStore => Path.Combine(_root, "store-global"); - private string ScopedStore => Path.Combine(_root, "store-scoped"); + private string GlobalStore => Path.Combine(Root, "store-global"); + private string ScopedStore => Path.Combine(Root, "store-scoped"); - public static async Task StartAsync() + /// The scratch host and its remote, which demands the token for reads too unless is false. + public static async Task StartAsync(bool authenticateReads = true) { - var ctx = new OperatorHostContext(); + var ctx = new OperatorHostContext(authenticateReads); try { @@ -315,19 +571,26 @@ public void RouteThroughProxy(AuthenticatingProxy proxy) _environment["no_proxy"] = ""; } - /// Point the operator's trace2 targets — normal, event and perf — at scratch files. - public void TraceToOperatorTargets() => - File.AppendAllText(GlobalConfig, $"[trace2]\n\tnormalTarget = {TraceFile("normal")}\n\teventTarget = {TraceFile("event")}\n\tperfTarget = {TraceFile("perf")}\n"); + /// Point the operator's trace2 targets — normal, event and perf — at scratch files, recording the values of too. + public void TraceToOperatorTargets(string envVars) => + File.AppendAllText(GlobalConfig, $"[trace2]\n\tnormalTarget = {TraceFile("normal")}\n\teventTarget = {TraceFile("event")}\n\tperfTarget = {TraceFile("perf")}\n\tenvVars = {envVars}\n"); /// Everything the operator's trace2 targets recorded. public string OperatorTrace() => string.Concat(new[] { "normal", "event", "perf" }.Select(TraceFile).Where(File.Exists).Select(File.ReadAllText)); - private string TraceFile(string target) => Path.Combine(_root, "trace2-" + target); + private string TraceFile(string target) => Path.Combine(Root, "trace2-" + target); /// The LFS filters git lfs install puts in global config, so a checkout smudges LFS pointers into their objects. public void InstallLfsFilters() => File.AppendAllText(GlobalConfig, "[filter \"lfs\"]\n\tclean = git-lfs clean -- %f\n\tsmudge = git-lfs smudge -- %f\n\tprocess = git-lfs filter-process\n\trequired = true\n"); + /// A watch for the token over the provider's workspaces root — every clone and publish repo it makes. + public TokenOnDiskWatch WatchTheWorkspaces() + { + Directory.CreateDirectory(WorkspacesRoot); + return new TokenOnDiskWatch(GitPublishRemoteFixture.FakeToken, WorkspacesRoot); + } + /// Integrate one contribution — over — into with the real integrator and the fake token. public Task IntegrateAsync(string branch, string baseSha, string patch) => new LocalGitBranchIntegrator(Registry, new InlineOffloader(), NullLogger.Instance).IntegrateAsync(new IntegrationRequest @@ -336,6 +599,83 @@ public Task IntegrateAsync(string branch, string baseSha, str Contributions = new[] { new BranchContribution { Label = "agent", BaseSha = baseSha, Patch = patch } }, }, CancellationToken.None); + /// against the remote once it redirects to the harvester: a workspace clone, a publish push from a workspace cloned before the redirect, or an integration clone. + public async Task RedirectedAsync(string operation) + { + if (operation == "push") + { + await using var cloned = await ProvisionAsync(GitPublishRemoteFixture.FakeToken); + await AgentCommitsAsync(cloned.Directory); + Remote.RedirectsToHarvester = true; + await ((IWorkspacePushHandle)cloned).PushChangesAsync(BranchName, CancellationToken.None); + return; + } + + Remote.RedirectsToHarvester = true; + + if (operation == "integrate") + { + await IntegrateAsync(BranchName, Remote.BaseSha, ReadmePatch); + return; + } + + await using var handle = await Provider.PrepareAsync(WorkspaceProvisionRequest.FromSingle(new WorkspaceRequest { RepositoryUrl = Remote.Url, Token = GitPublishRemoteFixture.FakeToken, TokenUsername = "x-access-token" }), CancellationToken.None); + } + + /// The real grader, its base clone resolved to the remote with the fake token, grading over with as the acceptance command. + public Task GradePatchAsync(string baseSha, string patch, string check) + { + var clone = new WorkspaceRequest { RepositoryUrl = Remote.Url, Token = GitPublishRemoteFixture.FakeToken, TokenUsername = "x-access-token" }; + var grader = new SupervisorAcceptanceGrader(new FixedResolver(clone), new WorkspaceProviderRegistry(new IWorkspaceProvider[] { Provider }), Registry, new BenchmarkGraderRegistry(new IBenchmarkGrader[] { new TestsPassGrader() }), new InlineOffloader(), new DiscardingArtifactStore(), null!, NullLogger.Instance); + + return grader.GradePatchAsync(Guid.NewGuid(), Guid.NewGuid(), baseSha, patch, null, new SupervisorAcceptanceSpec { Command = new[] { "/bin/sh", "-c", check } }, 60, CancellationToken.None); + } + + /// The launch-base probe for main, with the fake token. + public Task ResolveLaunchBaseAsync() => + new RemoteTipResolver(Registry).ResolveTipShaAsync(new WorkspaceRequest { RepositoryUrl = Remote.Url, Token = GitPublishRemoteFixture.FakeToken, TokenUsername = "x-access-token", Ref = "main" }, refRequired: true, CancellationToken.None); + + /// + /// with a token the remote refuses: a publish whose LFS upload the remote refuses (the clone + /// before it reads anonymously), or a clone whose checkout downloads an LFS object the remote refuses. + /// + public async Task WithARefusedTokenAsync(string operation) + { + if (operation == "clone") + { + await Remote.AddLfsHistoryAsync(); + InstallLfsFilters(); + } + + await using var handle = await Provider.PrepareAsync(WorkspaceProvisionRequest.FromSingle(new WorkspaceRequest { RepositoryUrl = Remote.Url, Token = RefusedToken, TokenUsername = "x-access-token" }), CancellationToken.None); + + await AgentCommitsLfsFileAsync(handle.Directory); + await ((IWorkspacePushHandle)handle).PushChangesAsync(BranchName, CancellationToken.None); + } + + /// + /// Rewrite the remote's authority in the operator's global config — to a local mirror with a history of its own for + /// fetches, to for pushes — and return the mirror's tip. + /// + public async Task RewriteTheRemoteAsync(LoopbackSink sink) + { + var mirrors = Path.Combine(Root, "mirrors"); + var seed = Path.Combine(Root, "mirror-seed"); + Directory.CreateDirectory(seed); + + await GitAsync(Root, "init", "-q", "--bare", "-b", "main", Path.Combine(mirrors, "remote.git")); + await GitAsync(seed, "init", "-q", "-b", "main"); + await File.WriteAllTextAsync(Path.Combine(seed, "README.md"), "from the mirror\n"); + await GitAsync(seed, "add", "-A"); + await GitAsync(seed, "-c", "user.name=Mirror", "-c", "user.email=mirror@example.test", "-c", "commit.gpgsign=false", "commit", "-q", "-m", "mirror"); + await GitAsync(seed, "push", "-q", Path.Combine(mirrors, "remote.git"), "main"); + + var authority = new Uri(Remote.Url).GetLeftPart(UriPartial.Authority); + File.AppendAllText(GlobalConfig, $"[url \"{mirrors}/\"]\n\tinsteadOf = {authority}/\n[url \"http://127.0.0.1:{sink.Port}/\"]\n\tpushInsteadOf = {authority}/\n"); + + return (await GitAsync(seed, "rev-parse", "HEAD")).Trim(); + } + /// The operator's own credential for the remote, as git credential-store keeps it. public void SeedTheOperatorsStore() => File.WriteAllText(GlobalStore, $"http://x-access-token:{GitPublishRemoteFixture.FakeToken}@{new Uri(Remote.Url).Authority}\n"); @@ -357,6 +697,13 @@ public void ShouldHoldTheToken(bool stored) } } + /// No argv the production code handed the runner carries the token or names a URL with userinfo — the clone, the probe, the push and the LFS upload included. + public void ShouldKeepTheTokenOffEveryArgv(string token = GitPublishRemoteFixture.FakeToken) + { + Runner.Specs.ShouldNotBeEmpty("fixture check: the production code ran git through the runner"); + Runner.Specs.Where(s => TokenedGitControls.ArgvCarriesACredential(s, token)).Select(s => string.Join(' ', s.Args)).ShouldBeEmpty("an argv carried the token, or a URL's userinfo"); + } + public async Task AgentCommitsAsync(string cloneDir) { await File.WriteAllTextAsync(Path.Combine(cloneDir, "agent.txt"), "the agent's work\n"); @@ -384,10 +731,14 @@ public async Task AgentCommitsLfsFileAsync(string cloneDir) public Task RemoteFileAsync(string branch, string file) => GitPublishRemoteFixture.GitAsync(Remote.Root, new[] { "--git-dir", Remote.Remote, "show", $"refs/heads/{branch}:{file}" }); + /// Test-side git on the same scratch host, hooks off, its result returned as it is; never recorded by the runner, never production code. + public Task RawGitAsync(string workdir, params string[] args) => + new LocalProcessRunner().RunAsync(new SandboxSpec { Command = "git", Args = new[] { "-c", "core.hooksPath=/dev/null" }.Concat(args).ToList(), WorkingDirectory = workdir, Environment = _environment, TimeoutSeconds = 120, AllowNetwork = true }, CancellationToken.None); + /// Test-side git (the agent's own commits) on the same scratch host, hooks off; never recorded, never a positive control. private async Task GitAsync(string workdir, params string[] args) { - var result = await new LocalProcessRunner().RunAsync(new SandboxSpec { Command = "git", Args = new[] { "-c", "core.hooksPath=/dev/null" }.Concat(args).ToList(), WorkingDirectory = workdir, Environment = _environment, TimeoutSeconds = 120 }, CancellationToken.None); + var result = await RawGitAsync(workdir, args); if (result.Status != SandboxStatus.Success) throw new InvalidOperationException($"git {string.Join(' ', args)} failed: {result.Stderr}"); @@ -398,14 +749,16 @@ private async Task GitAsync(string workdir, params string[] args) public async ValueTask DisposeAsync() { await Remote.DisposeAsync(); - try { Directory.Delete(_root, recursive: true); } catch { /* best-effort */ } + try { Directory.Delete(Root, recursive: true); } catch { /* best-effort */ } } } /// - /// The real local runner on the scratch operator host, recording every spec the production code submits. With - /// set, that one subcommand runs as it did before tokened commands were marked — without the - /// credential-helper reset, with trace2 on — the positive control. + /// The real local runner on the scratch operator host, recording every spec the production code submits. The positive + /// controls: with set, that one subcommand runs without the credential-helper reset and with trace2 + /// on; with set, that one runs without the pin that maps its remote to itself; with + /// , every tokened command's helper answers every get, and + /// bounds how long each command may then retry. /// private sealed class OperatorConfigRunner(IReadOnlyDictionary environment) : ISandboxRunner { @@ -413,6 +766,9 @@ private sealed class OperatorConfigRunner(IReadOnlyDictionary en public string Kind => "local"; public string? Unreset { get; set; } + public string? Unpinned { get; set; } + public bool HelperIgnoresErase { get; set; } + public int? TimeoutCapSeconds { get; set; } public List Specs { get; } = new(); public bool Ran(string subcommand) => Specs.Any(s => Subcommand(s.Args) == subcommand); @@ -421,22 +777,42 @@ public Task RunAsync(SandboxSpec spec, CancellationToken cancella { Specs.Add(spec); - var unreset = Subcommand(spec.Args) == Unreset; - var env = new Dictionary(spec.Environment); + var env = Controlled(Subcommand(spec.Args), spec.Environment); foreach (var (key, value) in environment) env[key] = value; - if (unreset) foreach (var key in TraceOff) env.Remove(key); - return _inner.RunAsync(spec with { Args = unreset ? WithoutTheReset(spec.Args) : spec.Args, Environment = env }, cancellationToken); + var run = spec with { Environment = env }; + if (TimeoutCapSeconds is { } cap) run = run with { TimeoutSeconds = Math.Min(run.TimeoutSeconds ?? cap, cap) }; + + return _inner.RunAsync(run, cancellationToken); } - private static IReadOnlyList WithoutTheReset(IReadOnlyList args) + /// with whatever part of it the control set for takes away. + private Dictionary Controlled(string subcommand, IReadOnlyDictionary env) { - var at = Enumerable.Range(0, Math.Max(0, args.Count - 1)).FirstOrDefault(i => args[i] == "-c" && IsHelperReset(args[i + 1]), -1); + var controlled = subcommand == Unreset ? TokenedGitControls.WithoutTheReset(env) : new Dictionary(env); + if (subcommand == Unpinned) controlled = TokenedGitControls.WithoutThePin(controlled); - return at < 0 ? args : args.Take(at).Concat(args.Skip(at + 2)).ToList(); + return HelperIgnoresErase ? TokenedGitControls.WithTheHelperIgnoringErase(controlled) : controlled; } } + private sealed class FixedResolver(WorkspaceRequest request) : IAgentWorkspaceResolver + { + public Task ResolveAsync(AgentTask task, Guid teamId, CancellationToken cancellationToken) => throw new NotSupportedException(); + + public Task ResolveByRepositoryIdAsync(Guid repositoryId, Guid teamId, CancellationToken cancellationToken, string? @ref = null, bool softFallback = false, string? pinnedSha = null) => Task.FromResult(request); + } + + /// Takes the grade's evidence and keeps nothing. + private sealed class DiscardingArtifactStore : IArtifactStore + { + public Task PutAsync(Guid teamId, ReadOnlyMemory bytes, string contentType, CancellationToken cancellationToken) => Task.FromResult(Guid.NewGuid()); + + public Task GetBytesAsync(Guid teamId, Guid artifactId, CancellationToken cancellationToken) => throw new NotSupportedException(); + + public Task GetMetadataAsync(Guid teamId, Guid artifactId, CancellationToken cancellationToken) => throw new NotSupportedException(); + } + private sealed class InlineOffloader : IArtifactOffloader { public Task ResolveAsync(Guid teamId, string? inline, Guid? artifactId, CancellationToken cancellationToken) => Task.FromResult(inline ?? ""); diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/TokenedGitTestSupport.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/TokenedGitTestSupport.cs new file mode 100644 index 000000000..3f35fa472 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/TokenedGitTestSupport.cs @@ -0,0 +1,186 @@ +using System.Text; +using CodeSpace.Messages.Agents; + +namespace CodeSpace.IntegrationTests.Workflows; + +/// +/// What a test needs to see a tokened git command from outside, spelled out literally rather than through +/// TokenedGitCommand: whether any argv element carries the token or a URL's userinfo, and the command as it would run +/// with one part of its environment taken away — the positive controls. Without the credential-helper reset and with trace2 +/// on, the operator's helpers and trace2 targets see the token; without the remote pin, the operator's URL rewrites move +/// the command; with a helper that ignores erase, git-lfs retries a refused token until the command times out. +/// +internal static class TokenedGitControls +{ + /// The tokened command's credential helper as it was before it recorded a refusal: it answers every get, so git-lfs retries a refused token without end. + public const string HelperIgnoringErase = """!f() { test "$1" = get || return 0; printf "username=%s\npassword=%s\n" "$CODESPACE_GIT_USERNAME" "$CODESPACE_GIT_PASSWORD"; }; f"""; + + private static readonly string[] TraceOff = { "GIT_TRACE2", "GIT_TRACE2_EVENT", "GIT_TRACE2_PERF" }; + + /// True when an argv element carries — raw, percent-encoded, or base64-encoded as a Basic credential carries it — or names an http(s) URL with userinfo. + public static bool ArgvCarriesACredential(SandboxSpec spec, string token) => + spec.Args.Any(a => a.Contains(token, StringComparison.Ordinal) || a.Contains(Uri.EscapeDataString(token), StringComparison.Ordinal) || Base64Forms(token).Any(f => a.Contains(f, StringComparison.Ordinal)) || NamesUserInfo(a)); + + /// + /// What any base64 text that encodes contains, whatever precedes it (a Basic credential's + /// user:, of any length): one string per alignment of the secret against base64's three-byte groups, each the + /// encoding of the groups that hold the secret's bytes alone. + /// + public static IReadOnlyList Base64Forms(string secret) => Enumerable.Range(0, 3).Select(shift => Base64Form(secret, shift)).Where(f => f.Length >= 8).ToList(); + + /// without the credential-helper reset among its GIT_CONFIG_COUNT entries (renumbered) and without trace2 off. + public static Dictionary WithoutTheReset(IReadOnlyDictionary environment) + { + var env = WithEntries(environment, e => IsHelperReset(e.Key, e.Value) ? null : e); + foreach (var key in TraceOff) env.Remove(key); + + return env; + } + + /// without the entries that map the remote's URL to itself, so the operator's url.<base>.insteadOf and pushInsteadOf rules apply to it. + public static Dictionary WithoutThePin(IReadOnlyDictionary environment) => WithEntries(environment, e => e.Key.StartsWith("url.", StringComparison.Ordinal) ? null : e); + + /// with in place of the credential helper. + public static Dictionary WithTheHelperIgnoringErase(IReadOnlyDictionary environment) => WithEntries(environment, e => IsHelper(e.Key) && e.Value.Length > 0 ? (e.Key, HelperIgnoringErase) : e); + + /// True when the command runs with no part of a tokened command's environment: no config entries, no credential variables, trace2 left alone. + public static bool RunsUntokened(SandboxSpec spec) => !spec.Environment.Keys.Any(k => k.StartsWith("GIT_CONFIG_", StringComparison.Ordinal) || k.StartsWith("CODESPACE_GIT_", StringComparison.Ordinal) || TraceOff.Contains(k)) && spec.ConfigHomeEnvVars.Count == 0; + + /// with each GIT_CONFIG_COUNT entry passed through — dropped where it returns null — and the rest renumbered. + private static Dictionary WithEntries(IReadOnlyDictionary environment, Func<(string Key, string Value), (string Key, string Value)?> map) + { + var env = new Dictionary(environment); + + if (!env.TryGetValue("GIT_CONFIG_COUNT", out var raw)) return env; + + var count = int.Parse(raw); + var kept = Enumerable.Range(0, count).Select(i => map((env[$"GIT_CONFIG_KEY_{i}"], env[$"GIT_CONFIG_VALUE_{i}"]))).OfType<(string Key, string Value)>().ToList(); + + for (var i = 0; i < count; i++) { env.Remove($"GIT_CONFIG_KEY_{i}"); env.Remove($"GIT_CONFIG_VALUE_{i}"); } + for (var i = 0; i < kept.Count; i++) { env[$"GIT_CONFIG_KEY_{i}"] = kept[i].Key; env[$"GIT_CONFIG_VALUE_{i}"] = kept[i].Value; } + env["GIT_CONFIG_COUNT"] = kept.Count.ToString(); + + return env; + } + + private static bool IsHelper(string key) => key.StartsWith("credential.", StringComparison.Ordinal) && key.EndsWith(".helper", StringComparison.Ordinal); + + private static bool IsHelperReset(string key, string value) => IsHelper(key) && value.Length == 0; + + /// The base64 of after other bytes, cut to the groups that hold the secret's bytes alone. + private static string Base64Form(string secret, int shift) + { + var bytes = Encoding.UTF8.GetBytes(secret); + var encoded = Convert.ToBase64String(new byte[shift].Concat(bytes).ToArray()); + var start = shift == 0 ? 0 : 4; + var end = (shift + bytes.Length) / 3 * 4; + + return end > start ? encoded[start..end] : ""; + } + + private static bool NamesUserInfo(string arg) => + Uri.TryCreate(arg, UriKind.Absolute, out var uri) && (uri.Scheme == Uri.UriSchemeHttp || uri.Scheme == Uri.UriSchemeHttps) && uri.UserInfo.Length > 0; +} + +/// +/// Scans every file under a root — or, given a prefix, under each of the root's directories so named that appeared after the +/// watch began — for a secret, raw or base64-encoded as a Basic credential carries it, over and over until disposed, +/// recording each file it was found in. So a test can say a token never reached the disk at any point during an operation, +/// not only once the operation ended. +/// +internal sealed class TokenOnDiskWatch : IAsyncDisposable +{ + private const long LargestFileScanned = 4 * 1024 * 1024; + + private readonly byte[][] _needles; + private readonly string _root; + private readonly string? _childPrefix; + private readonly HashSet _before; + private readonly HashSet _hits = new(StringComparer.Ordinal); + private readonly CancellationTokenSource _stopping = new(); + private readonly Task _scanning; + private int _scans; + + public TokenOnDiskWatch(string secret, string root, string? childPrefix = null) + { + _needles = TokenedGitControls.Base64Forms(secret).Prepend(secret).Select(Encoding.UTF8.GetBytes).ToArray(); + _root = root; + _childPrefix = childPrefix; + _before = childPrefix is null ? new(StringComparer.Ordinal) : Directories(root).ToHashSet(StringComparer.Ordinal); + _scanning = Task.Run(ScanUntilStoppedAsync); + } + + /// Completed passes over the tree — a fixture check that the watch was looking while the operation ran. + public int Scans => Volatile.Read(ref _scans); + + /// Every file the secret was seen in, at any point. + public IReadOnlyList Hits { get { lock (_hits) return _hits.Order(StringComparer.Ordinal).ToList(); } } + + private async Task ScanUntilStoppedAsync() + { + while (!_stopping.IsCancellationRequested) + { + ScanOnce(); + Interlocked.Increment(ref _scans); + + try { await Task.Delay(2, _stopping.Token); } + catch (OperationCanceledException) { } + } + } + + private void ScanOnce() + { + foreach (var file in WatchedRoots().SelectMany(Files)) + if (Holds(file)) lock (_hits) _hits.Add(file); + } + + private IEnumerable WatchedRoots() => _childPrefix is null ? new[] { _root } : Directories(_root).Where(d => Path.GetFileName(d).StartsWith(_childPrefix, StringComparison.Ordinal) && !_before.Contains(d)).ToList(); + + private static IEnumerable Directories(string root) + { + try { return Directory.Exists(root) ? Directory.EnumerateDirectories(root).ToList() : Array.Empty(); } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { return Array.Empty(); } + } + + /// The files under right now; a tree changing under the walk yields what was read before it moved. + private static IReadOnlyList Files(string root) + { + var files = new List(); + + try + { + if (Directory.Exists(root)) files.AddRange(Directory.EnumerateFiles(root, "*", new EnumerationOptions { RecurseSubdirectories = true, IgnoreInaccessible = true, AttributesToSkip = FileAttributes.ReparsePoint })); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { } + + return files; + } + + private bool Holds(string file) + { + try + { + var info = new FileInfo(file); + return info.Exists && info.Length <= LargestFileScanned && Contains(File.ReadAllBytes(file)); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { return false; } + } + + private bool Contains(byte[] content) => _needles.Any(n => content.AsSpan().IndexOf(n) >= 0); + + /// Stop scanning, after one last pass over what is on disk now. Idempotent. + public async Task StopAsync() + { + if (_stopping.IsCancellationRequested) return; + + _stopping.Cancel(); + await _scanning; + ScanOnce(); + } + + public async ValueTask DisposeAsync() + { + await StopAsync(); + _stopping.Dispose(); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Agents/LocalGitBranchIntegratorTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/LocalGitBranchIntegratorTests.cs index 7fe835fcd..6837970a2 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/LocalGitBranchIntegratorTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/LocalGitBranchIntegratorTests.cs @@ -54,9 +54,8 @@ public void Embedded_newlines_are_collapsed_to_a_single_space_never_reaching_a_o [Fact] public void Both_the_raw_token_and_its_url_escaped_form_are_redacted() { - // BuildAuthenticatedUrl embeds Uri.EscapeDataString(token) in the clone/push argv, so a token with - // URL-special characters appears ENCODED in a failing git command — redacting only the raw literal would - // leak the reversible encoded form. + // No argv carries the token, but git or a remote can still echo it, and a token with URL-special characters + // may come back ENCODED — redacting only the raw literal would leak the reversible encoded form. const string token = "tok@en/special+chars"; var escaped = Uri.EscapeDataString(token); @@ -98,20 +97,20 @@ public void The_cap_never_splits_a_surrogate_pair() detail.ShouldBe(filler + "…", "the cap drops the WHOLE surrogate pair rather than emit its unpaired high half"); } - // ── Tokened commands: the ones whose git transport reaches the tokened origin ────────── + // ── Tokened commands: the ones whose git or git-lfs transport reaches origin ────────── [Theory] - [InlineData(true, false)] // clean: the clone and the push - [InlineData(true, true)] // conflicted: the clone only — nothing is pushed + [InlineData(true, false)] // clean: the clone, the base checkout, the apply and the push + [InlineData(true, true)] // conflicted: the clone, the checkout, the apply and the reset — nothing is pushed [InlineData(false, false)] // untokened: an anonymous clone keeps the operator's helpers and trace2 [InlineData(false, true)] - public async Task Only_the_clone_and_the_push_run_as_tokened_commands(bool tokened, bool conflicted) + public async Task Only_the_commands_that_reach_origin_run_as_tokened_commands(bool tokened, bool conflicted) { - // The integration clone keeps its tokened origin to the end, but only the commands whose git transport talks to it - // hand the URL's password to credential helpers or write the URL to trace2: the clone names the authed URL, and the - // push goes through origin. The base checkout, the apply and the reset back to base download LFS objects through origin too, but - // git-lfs authenticates those from the URL and neither asks nor tells a helper (TokenedGitCredentialHelperFlowTests - // proves it), so they run as written, like the commit, the diffs and the rev-parses. + // The integration clone names the remote without its credential, so origin carries none: every command that reaches + // it carries the token in its environment instead. The clone and the push reach it through git's transport; the base + // checkout, the apply and the reset back to base download LFS objects through it, and git-lfs asks the credential + // helpers for those (TokenedGitCredentialHelperFlowTests proves it). The commit, the diffs and the rev-parses reach + // nothing and run as written. var runner = new IntegrationRunner(conflicted); var integrator = new LocalGitBranchIntegrator(new SandboxRunnerRegistry(new ISandboxRunner[] { runner }), new InlineOffloader(), NullLogger.Instance); @@ -121,7 +120,7 @@ await integrator.IntegrateAsync(new IntegrationRequest Contributions = new[] { new BranchContribution { Label = "agent", BaseSha = "base", Patch = "diff --git a/f.txt b/f.txt\n--- a/f.txt\n+++ b/f.txt\n@@ -1 +1 @@\n-a\n+b\n" } }, }, CancellationToken.None); - var transport = new[] { "clone", "push" }; + var transport = new[] { "clone", "checkout", "apply", "reset", "push" }; var subcommands = runner.Specs.Select(Subcommand).ToList(); subcommands.ShouldContain(conflicted ? "reset" : "commit", "fixture check: the run took the intended path"); subcommands.ShouldContain("checkout", "fixture check: the base was checked out"); @@ -129,7 +128,10 @@ await integrator.IntegrateAsync(new IntegrationRequest if (tokened && !conflicted) subcommands.ShouldContain("push", "fixture check: a clean tokened integration pushes"); foreach (var spec in runner.Specs) - TokenedGitSpecs.RunsTokened(spec, "https://example.test").ShouldBe(tokened && transport.Contains(Subcommand(spec)), string.Join(' ', spec.Args)); + TokenedGitSpecs.RunsTokened(spec, "https://example.test/repo.git").ShouldBe(tokened && transport.Contains(Subcommand(spec)), string.Join(' ', spec.Args)); + + runner.Specs.Single(s => Subcommand(s) == "clone").Args.ShouldContain("https://example.test/repo.git", "the remote is named without its credential"); + runner.Specs.Where(s => TokenedGitSpecs.ArgvCarriesACredential(s, "test-token")).Select(s => string.Join(' ', s.Args)).ShouldBeEmpty(); } /// The git subcommand, past any leading -c key=value and -C dir. diff --git a/backend/tests/CodeSpace.UnitTests/Agents/PackCloneFetcherArgsTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/PackCloneFetcherArgsTests.cs index e9d89c52f..a0bd0b034 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/PackCloneFetcherArgsTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/PackCloneFetcherArgsTests.cs @@ -48,15 +48,20 @@ public void Clone_argv_passes_a_branch_reference_when_set() } [Theory] - [InlineData("https://someone:ghp_pasted_token@github.com/owner/repo", true)] // a pasted URL with a personal token in it - [InlineData("https://ghp_pasted_token@github.com/owner/repo", true)] // the token alone as the user: git hands it to the helpers it asks for a password - [InlineData("https://github.com/owner/repo", false)] - public void Only_a_clone_url_carrying_a_token_runs_as_a_tokened_command(string url, bool tokened) + [InlineData("https://someone:ghp_pasted_token@github.com/owner/repo", "someone", "ghp_pasted_token")] // a pasted URL with a personal token in it + [InlineData("https://ghp_pasted_token@github.com/owner/repo", "ghp_pasted_token", "")] // the token alone as the user, sent with an empty password as curl sent it + [InlineData("https://github.com/owner/repo", null, null)] + public void Only_a_clone_url_carrying_a_token_runs_as_a_tokened_command_and_no_argv_carries_it(string url, string? username, string? password) { + // The pasted credential leaves the URL: the clone names the remote without it — so git never writes it into the + // checkout's origin — and carries it in its environment for a helper scoped to the remote, so no operator helper + // stores it and no trace2 target records it. var spec = PackCloneFetcher.BuildCloneSpec(url, reference: null, dir: "/tmp/dest"); - TokenedGitSpecs.RunsTokened(spec, "https://github.com").ShouldBe(tokened, "a helper would store the pasted token on success, and trace2 would record it"); - spec.Args.Skip(tokened ? 2 : 0).ShouldBe(PackCloneFetcher.BuildCloneArgs(url, reference: null, dir: "/tmp/dest"), "the hardened clone argv, redirect guard included, either way"); + TokenedGitSpecs.RunsTokened(spec, "https://github.com/owner/repo").ShouldBe(username is not null); + if (username is not null) TokenedGitSpecs.CarriesTheCredential(spec, username, password!).ShouldBeTrue(); + spec.Args.ShouldBe(PackCloneFetcher.BuildCloneArgs("https://github.com/owner/repo", reference: null, dir: "/tmp/dest"), "the hardened clone argv, redirect guard included, naming the remote without its userinfo either way"); + TokenedGitSpecs.ArgvCarriesACredential(spec, "ghp_pasted_token").ShouldBeFalse(); spec.WorkingDirectory.ShouldBe("/tmp/dest"); spec.AllowNetwork.ShouldBeTrue(); } diff --git a/backend/tests/CodeSpace.UnitTests/Agents/PackCloneFetcherCredentialTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/PackCloneFetcherCredentialTests.cs index d4e1c938c..23e59cd25 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/PackCloneFetcherCredentialTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/PackCloneFetcherCredentialTests.cs @@ -12,9 +12,10 @@ namespace CodeSpace.UnitTests.Agents; /// A pasted pack URL can carry a credential in its userinfo: as the password (x-access-token:<token>@, /// oauth2:<token>@) or as the user (<token>@, <token>:x-oauth-basic@). Pins that a clone /// failure names the URL without it and redacts it from git's stderr in every spelling git echoes it in, while a user named -/// beside a password leaves git's reason readable; that the clone runs in an owner-only directory and its origin is pointed -/// at the URL without the credential once cloned (and the clone refused when neither rewrite nor removal works); and that a -/// URL with no credential, an ssh URL's git@ included, is left as written. PackCloneCredentialFlowTests proves +/// beside a password leaves git's reason readable; that the clone names the remote without the credential, in an +/// owner-only directory, and still points origin at that URL once cloned as a belt (the clone refused when neither rewrite +/// nor removal works); and that a URL with no credential, an ssh URL's git@ included, is left as written. +/// PackCloneCredentialFlowTests proves /// the same against real git and a remote that demands the token. /// [Trait("Category", "Unit")] @@ -78,10 +79,10 @@ public void A_user_named_beside_a_password_leaves_gits_reason_readable(string ur } [Fact] - public async Task The_clone_directory_is_owner_only_before_git_writes_the_pasted_url_into_it() + public async Task The_clone_directory_is_owner_only_before_git_runs_in_it() { - // git writes the tokened origin into .git/config before the transfer starts, and the strip runs only after it, so - // for the whole clone the token is on disk under the worker's temp dir. + // A belt: the clone names the remote without the pasted credential, so git writes none into .git/config, but the + // checkout is still the import's private copy until it is walked. if (OperatingSystem.IsWindows()) return; var runner = new ScriptedRunner(); @@ -115,15 +116,17 @@ public async Task A_failed_clone_throws_without_the_credential_and_leaves_no_clo [Theory] [InlineData(Tokened)] - [InlineData("https://fake-pasted-token-0123456789@github.com/owner/repo.git")] // a token pasted as the user lands in .git/config just the same - public async Task A_pasted_credential_is_stripped_from_origin_once_cloned(string url) + [InlineData("https://fake-pasted-token-0123456789@github.com/owner/repo.git")] // a token pasted as the user alone + public async Task A_pasted_credential_never_reaches_origin_and_the_strip_stays_as_a_belt(string url) { var runner = new ScriptedRunner(); using var checkout = await Fetcher(runner).FetchAsync(url, null, CancellationToken.None); runner.Specs.Count.ShouldBe(2, "the clone, then one rewrite of origin"); - runner.Specs[1].Args.ShouldBe(new[] { "-C", checkout.Directory, "remote", "set-url", "origin", "https://github.com/owner/repo.git" }); + runner.Specs[0].Args.ShouldContain("https://github.com/owner/repo.git", "the clone names the remote without the pasted credential, so origin never holds it"); + runner.Specs[0].Args.ShouldNotContain(a => a.Contains(Marker), "no argv carries the pasted credential"); + runner.Specs[1].Args.ShouldBe(new[] { "-C", checkout.Directory, "remote", "set-url", "origin", "https://github.com/owner/repo.git" }, "the strip sets origin to the URL it already holds"); } [Theory] diff --git a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAcceptanceGraderTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAcceptanceGraderTests.cs index 10c8138cd..40ebd6639 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAcceptanceGraderTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAcceptanceGraderTests.cs @@ -195,19 +195,34 @@ public async Task GradeBaseAsync_fails_closed_with_a_prefixed_detail_when_the_ba [Theory] [InlineData(true)] [InlineData(false)] - public async Task Only_a_tokened_base_clone_runs_as_a_tokened_command(bool tokened) + public async Task The_commands_that_reach_the_remote_run_as_tokened_commands(bool tokened) { - // The clone is the one command that names the authed URL; the token strip follows it at once, so the detached - // checkout and everything after it reach no tokened remote. An untokened clone keeps the operator's helpers. + // The clone reaches the remote through git's transport; the detached base checkout and the apply reach it through + // git-lfs, which downloads the LFS objects they write from origin and asks the credential helpers for them. Each names + // the remote without its credential and carries the token in its environment. The strip, a belt, reaches nothing, and + // no command after the apply — the oracle restore, the model-authored setup and check — carries the credential. An + // untokened clone keeps the operator's helpers. var runners = new ScriptedApplyRunnerRegistry(applySucceeds: true); var grader = Build(new FakeResolver(new WorkspaceRequest { RepositoryUrl = "https://example.test/r.git", Token = tokened ? "test-token" : null }), new FakeGrader(Pass), runners: runners); - await grader.GradeBaseAsync(Guid.NewGuid(), Guid.NewGuid(), "deadbeef", Spec(), 30, CancellationToken.None); + await grader.GradePatchAsync(Guid.NewGuid(), Guid.NewGuid(), "deadbeef", "diff --git a/x b/x", null, Spec(), 30, CancellationToken.None); - var clone = runners.Invocations.Single(i => i.Args.Contains("clone")); - TokenedGitSpecs.RunsTokened(clone, "https://example.test").ShouldBe(tokened, string.Join(' ', clone.Args)); - runners.Invocations.ShouldContain(i => i.Args.Contains("checkout"), "fixture check: the base checkout ran after the clone"); - runners.Invocations.Where(i => !i.Args.Contains("clone")).ShouldAllBe(i => !TokenedGitSpecs.RunsTokened(i, "https://example.test")); + var reachTheRemote = new[] { "clone", "checkout", "apply" }; + runners.Invocations.Select(GitSubcommand).ShouldBe(tokened ? new[] { "clone", "remote", "checkout", "apply" } : reachTheRemote, "fixture check: the clone, the strip when tokened, the base checkout and the apply ran, in that order"); + + foreach (var invocation in runners.Invocations) + TokenedGitSpecs.RunsTokened(invocation, "https://example.test/r.git").ShouldBe(tokened && reachTheRemote.Contains(GitSubcommand(invocation)), string.Join(' ', invocation.Args)); + + runners.Invocations.Single(i => i.Args.Contains("clone")).Args.ShouldContain("https://example.test/r.git", "the remote is named without its credential"); + runners.Invocations.ShouldAllBe(i => !TokenedGitSpecs.ArgvCarriesACredential(i, "test-token")); + } + + /// The git subcommand, past any leading -c key=value and -C dir. + private static string GitSubcommand(SandboxSpec spec) + { + var i = 0; + while (i + 1 < spec.Args.Count && spec.Args[i] is "-c" or "-C") i += 2; + return i < spec.Args.Count ? spec.Args[i] : ""; } // ── S2: GradePatchAsync — the branch-less twin (a fresh clone at the BASE SHA + apply, no push) ──── diff --git a/backend/tests/CodeSpace.UnitTests/TokenedGitSpecs.cs b/backend/tests/CodeSpace.UnitTests/TokenedGitSpecs.cs index 6f2ffc4b0..488f84370 100644 --- a/backend/tests/CodeSpace.UnitTests/TokenedGitSpecs.cs +++ b/backend/tests/CodeSpace.UnitTests/TokenedGitSpecs.cs @@ -1,29 +1,103 @@ +using System.Text; using CodeSpace.Messages.Agents; using Shouldly; namespace CodeSpace.UnitTests; /// -/// The call-site pins' view of a tokened git command, spelled out literally rather than through -/// TokenedGitCommand itself: the reset scoped to the remote leads the argv exactly once, and every trace2 target -/// is off. A spec carrying only part of that fails the test outright — it is neither shape. +/// The call-site pins' view of a tokened git command, spelled out literally rather than through TokenedGitCommand +/// itself. The environment carries the whole shape — GIT_CONFIG_COUNT entries that empty the credential helper list +/// for the remote's scheme and authority and then name the helper that reads the credential, map the remote's URL to +/// itself so no operator rewrite rule moves it, and turn auto-gc and auto-maintenance off; the credential in the two +/// variables that helper reads; every trace2 target off — the runner is asked for the private directory the helper records +/// a refusal in, and no argv element names a credential helper. A spec carrying only part of that fails the test outright: +/// it is neither shape. /// internal static class TokenedGitSpecs { - private static readonly string[] TraceOff = { "GIT_TRACE2", "GIT_TRACE2_EVENT", "GIT_TRACE2_PERF" }; + /// + /// The credential helper a tokened command names, literally: answers get from the two variables with the shell's + /// builtin printf until erase records in the state directory that the remote refused the credential, then says so + /// instead of answering; ignores store; answers nothing without the state directory. + /// + public const string Helper = """!f() { r="$CODESPACE_GIT_STATE/refused"; case "$1" in get) if test -e "$r"; then echo "the remote refused this credential; not offering it again" >&2; elif test -d "$CODESPACE_GIT_STATE"; then printf "username=%s\npassword=%s\n" "$CODESPACE_GIT_USERNAME" "$CODESPACE_GIT_PASSWORD"; fi;; erase) test -d "$CODESPACE_GIT_STATE" && : > "$r";; esac; }; f"""; - /// True when runs as a tokened command for the remote at (its scheme and authority, e.g. https://example.test); false when it carries no part of one. - public static bool RunsTokened(SandboxSpec spec, string scope) + /// The variable the runner points at the command's private state directory. + public const string StateVariable = "CODESPACE_GIT_STATE"; + + private static readonly string[] CredentialVariables = { "CODESPACE_GIT_USERNAME", "CODESPACE_GIT_PASSWORD" }; + + /// The environment a tokened command for the remote at (named without userinfo) carries besides the credential itself. + public static IReadOnlyDictionary ConfigFor(string url) + { + var scope = new Uri(url).GetLeftPart(UriPartial.Authority); + + return new Dictionary(StringComparer.Ordinal) + { + ["GIT_CONFIG_COUNT"] = "6", + ["GIT_CONFIG_KEY_0"] = $"credential.{scope}.helper", + ["GIT_CONFIG_VALUE_0"] = "", + ["GIT_CONFIG_KEY_1"] = $"credential.{scope}.helper", + ["GIT_CONFIG_VALUE_1"] = Helper, + ["GIT_CONFIG_KEY_2"] = $"url.{url}.insteadOf", + ["GIT_CONFIG_VALUE_2"] = url, + ["GIT_CONFIG_KEY_3"] = $"url.{url}.pushInsteadOf", + ["GIT_CONFIG_VALUE_3"] = url, + ["GIT_CONFIG_KEY_4"] = "gc.auto", + ["GIT_CONFIG_VALUE_4"] = "0", + ["GIT_CONFIG_KEY_5"] = "maintenance.auto", + ["GIT_CONFIG_VALUE_5"] = "false", + ["GIT_TRACE2"] = "0", + ["GIT_TRACE2_EVENT"] = "0", + ["GIT_TRACE2_PERF"] = "0", + }; + } + + /// True when runs as a tokened command for the remote at ; false when it carries no part of one. + public static bool RunsTokened(SandboxSpec spec, string url) { - var leads = spec.Args.Take(2).SequenceEqual(new[] { "-c", $"credential.{scope}.helper=" }); - var resets = spec.Args.Count(a => a.StartsWith("credential.", StringComparison.Ordinal) && a.EndsWith(".helper=", StringComparison.Ordinal)); - var traceOff = TraceOff.Count(k => spec.Environment.TryGetValue(k, out var v) && v == "0"); - var traceKeys = TraceOff.Count(spec.Environment.ContainsKey); + var expected = ConfigFor(url); + var matching = expected.Count(kv => spec.Environment.TryGetValue(kv.Key, out var value) && value == kv.Value); + var present = spec.Environment.Keys.Count(IsPartOfATokenedCommand); + var stateDirectory = spec.ConfigHomeEnvVars.Count(v => v == StateVariable); + var helperOnTheArgv = spec.Args.Any(a => a.StartsWith("credential.", StringComparison.Ordinal) && a.Contains(".helper", StringComparison.Ordinal)); - var tokened = leads && resets == 1 && traceOff == TraceOff.Length; - var untokened = resets == 0 && traceKeys == 0; - (tokened || untokened).ShouldBeTrue($"half a tokened command: {string.Join(' ', spec.Args)} | env {string.Join(' ', spec.Environment.Keys)}"); + var tokened = matching == expected.Count && present == expected.Count + CredentialVariables.Length && stateDirectory == 1 && !helperOnTheArgv; + var untokened = present == 0 && stateDirectory == 0 && !helperOnTheArgv; + (tokened || untokened).ShouldBeTrue($"half a tokened command: {string.Join(' ', spec.Args)} | env {string.Join(' ', spec.Environment.Keys)} | state dirs {string.Join(' ', spec.ConfigHomeEnvVars)}"); return tokened; } + + /// True when 's environment carries exactly and as the credential. + public static bool CarriesTheCredential(SandboxSpec spec, string username, string password) => + spec.Environment.TryGetValue("CODESPACE_GIT_USERNAME", out var u) && u == username && spec.Environment.TryGetValue("CODESPACE_GIT_PASSWORD", out var p) && p == password; + + /// True when an argv element carries — raw, percent-encoded, or base64-encoded as a Basic credential carries it — or names an http(s) URL with userinfo. + public static bool ArgvCarriesACredential(SandboxSpec spec, string secret) => + spec.Args.Any(a => a.Contains(secret, StringComparison.Ordinal) || a.Contains(Uri.EscapeDataString(secret), StringComparison.Ordinal) || Base64Forms(secret).Any(f => a.Contains(f, StringComparison.Ordinal)) || NamesUserInfo(a)); + + /// + /// What any base64 text that encodes contains, whatever precedes it (a Basic credential's + /// user:, of any length): one string per alignment of the secret against base64's three-byte groups, each the + /// encoding of the groups that hold the secret's bytes alone. + /// + public static IReadOnlyList Base64Forms(string secret) => Enumerable.Range(0, 3).Select(shift => Base64Form(secret, shift)).Where(f => f.Length >= 8).ToList(); + + /// The base64 of after other bytes, cut to the groups that hold the secret's bytes alone. + private static string Base64Form(string secret, int shift) + { + var bytes = Encoding.UTF8.GetBytes(secret); + var encoded = Convert.ToBase64String(new byte[shift].Concat(bytes).ToArray()); + var start = shift == 0 ? 0 : 4; + var end = (shift + bytes.Length) / 3 * 4; + + return end > start ? encoded[start..end] : ""; + } + + private static bool IsPartOfATokenedCommand(string name) => + name.StartsWith("GIT_CONFIG_", StringComparison.Ordinal) || name.StartsWith("GIT_TRACE2", StringComparison.Ordinal) || CredentialVariables.Contains(name); + + private static bool NamesUserInfo(string arg) => + Uri.TryCreate(arg, UriKind.Absolute, out var uri) && (uri.Scheme == Uri.UriSchemeHttp || uri.Scheme == Uri.UriSchemeHttps) && uri.UserInfo.Length > 0; } diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/LocalGitWorkspaceProviderTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/LocalGitWorkspaceProviderTests.cs index 49e892e02..a90cba2c0 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/LocalGitWorkspaceProviderTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/LocalGitWorkspaceProviderTests.cs @@ -9,52 +9,27 @@ namespace CodeSpace.UnitTests.Workflows; /// -/// — the pure auth-URL builder (no git), plus the real clone +/// — the pure redaction (no git), plus the real clone /// mechanics against a REAL local git repo (mirrors driving a real /// process). The clone tests skip where git isn't installed, so cross-host dotnet test stays clean. /// [Trait("Category", "Unit")] public sealed class LocalGitWorkspaceProviderTests { - // ─── Pure auth-URL builder ─────────────────────────────────────────────── - - [Fact] - public void No_token_leaves_the_url_unchanged() => - LocalGitWorkspaceProvider.BuildAuthenticatedUrl("https://github.com/org/repo.git", null, null) - .ShouldBe("https://github.com/org/repo.git"); - - [Fact] - public void Token_with_no_username_defaults_to_x_access_token() => - LocalGitWorkspaceProvider.BuildAuthenticatedUrl("https://github.com/org/repo.git", null, "ghp_abc") - .ShouldBe("https://x-access-token:ghp_abc@github.com/org/repo.git"); - - [Fact] - public void Token_uses_the_provider_specific_username() => - LocalGitWorkspaceProvider.BuildAuthenticatedUrl("https://gitlab.com/org/repo.git", "oauth2", "glpat_xyz") - .ShouldBe("https://oauth2:glpat_xyz@gitlab.com/org/repo.git"); - - [Fact] - public void Special_characters_in_the_token_are_escaped() => - LocalGitWorkspaceProvider.BuildAuthenticatedUrl("https://example.com/r.git", "u", "p@ss/word") - .ShouldBe("https://u:p%40ss%2Fword@example.com/r.git"); - - [Fact] - public void Authenticated_url_preserves_a_non_default_port() => - LocalGitWorkspaceProvider.BuildAuthenticatedUrl("https://git.local:8443/org/repo.git", "oauth2", "t") - .ShouldBe("https://oauth2:t@git.local:8443/org/repo.git"); + // ─── Pure redaction ────────────────────────────────────────────────────── [Fact] public void Redact_scrubs_both_the_raw_token_and_its_url_encoded_form() { - // The push argv embeds Uri.EscapeDataString(token); a token with URL-special chars appears ENCODED in a - // failing push command, so redacting only the raw literal would leak the reversible encoded form. + // No argv carries the token, but a remote or a tool can still echo it, and a token with URL-special chars may come + // back ENCODED — redacting only the raw literal would leak the reversible encoded form. const string token = "p@ss/w+rd=secret"; var leak = $"git push https://x-access-token:{Uri.EscapeDataString(token)}@host/r.git refused; raw {token} too"; var redacted = LocalGitWorkspaceProvider.Redact(leak, token); redacted.ShouldNotContain(token, Case.Insensitive, "the raw token literal must be scrubbed"); - redacted.ShouldNotContain(Uri.EscapeDataString(token), Case.Insensitive, "the percent-encoded token (as it appears in the push argv) must ALSO be scrubbed"); + redacted.ShouldNotContain(Uri.EscapeDataString(token), Case.Insensitive, "the percent-encoded token (as an echo may carry it) must ALSO be scrubbed"); redacted.ShouldContain("***"); } @@ -525,12 +500,12 @@ private static WorkspaceProvisionRequest PostTurnProvision(string token, bool mu } : WorkspaceProvisionRequest.FromSingle(new WorkspaceRequest { RepositoryUrl = "https://example.test/repo.git", Token = token }); - /// The credential (in the authed URL) and the network appear ONLY in the publish repo: never with the agent clone as the working directory, never pointed at it with an argument (-C, --git-dir, --work-tree), never with it bound in. + /// The credential (in the environment) and the network appear ONLY in the publish repo: never with the agent clone as the working directory, never pointed at it with an argument (-C, --git-dir, --work-tree), never with it bound in. private static void ShouldNeverMeetTheAgentClone(IReadOnlyList postTurn, string token, string cloneDir, string workspaceRoot) { - var reachesOut = postTurn.Where(s => s.AllowNetwork || s.Args.Any(a => a.Contains(token))).ToList(); + var reachesOut = postTurn.Where(s => s.AllowNetwork || CarriesTheToken(s, token)).ToList(); - reachesOut.Count(s => s.Args.Any(a => a.Contains(token))).ShouldBe(2, "exactly the authenticated push and its ls-remote readback carry the credential"); + reachesOut.Count(s => CarriesTheToken(s, token)).ShouldBe(2, "exactly the authenticated push and its ls-remote readback carry the credential"); reachesOut.Count(s => s.Args.Contains("push") || s.Args.Contains("ls-remote")).ShouldBe(2, "exactly the authenticated push and its ls-remote readback reach the remote"); foreach (var spec in reachesOut) @@ -578,7 +553,7 @@ private static void ShouldBundleTheBranchOutReadOnly(IReadOnlyList { bundle.ReadOnlyPaths.ShouldBe(new[] { cloneDir }, "the agent clone is bound read-only while its objects are bundled"); bundle.AllowNetwork.ShouldBeFalse(); - bundle.Args.ShouldNotContain(a => a.Contains(token)); + CarriesTheToken(bundle, token).ShouldBeFalse(); bundle.Args.Take(AgentCloneGitCommand.HardeningConfig.Count).ShouldBe(AgentCloneGitCommand.HardeningConfig); IsOutside(workspaceRoot, bundle.Args[bundle.Args.ToList().IndexOf("create") + 1]).ShouldBeTrue("the bundle file is written outside the workspace"); } @@ -597,6 +572,9 @@ private static void ShouldCheckOnlyTheObjectsTheBranchAdds(IReadOnlyList !f.AllowNetwork && f.TimeoutSeconds == 300, "a bundle import is local, and as heavy as the push, so it gets the push's budget"); } + /// True when the token rides anywhere in — its argv or its environment. + private static bool CarriesTheToken(SandboxSpec spec, string token) => spec.Args.Any(a => a.Contains(token)) || spec.Environment.Values.Any(v => v.Contains(token)); + /// True when is neither nor anything below it. private static bool IsOutside(string root, string path) { @@ -635,11 +613,11 @@ public Task RunAsync(SandboxSpec spec, CancellationToken cancella [InlineData(false)] public async Task Every_command_before_the_token_strip_runs_as_a_tokened_command_when_the_clone_is_tokened(bool tokened) { - // Until the strip, the token is in reach: the probe and the clone name the authed URL, and the pin's fetch rungs go - // through the still-tokened origin. Every command before the strip shares one runner path, so each runs as a - // tokened command — the local ones (the ancestry checks, the pin's checkout) at no cost. The strip and the base - // read after it reach no tokened remote, and an untokened clone keeps the operator's helpers and trace2 on every - // command — they may be how it authenticates. + // Until the strip, the commands can reach the remote: the probe and the clone name it, and the pin's fetch rungs and + // checkout reach it through origin, git-lfs's downloads included. Every command before the strip shares one runner + // path, so each runs as a tokened command, carrying the token in its environment — the local ones (the ancestry + // checks) at no cost. The strip and the base read after it reach no remote, and an untokened clone keeps the + // operator's helpers and trace2 on every command — they may be how it authenticates. var runner = new PinFetchRunner(); var provider = new LocalGitWorkspaceProvider(new SandboxRunnerRegistry(new[] { runner }), NullLogger.Instance); @@ -655,16 +633,21 @@ public async Task Every_command_before_the_token_strip_runs_as_a_tokened_command var strip = runner.Specs.FindIndex(s => s.Args.Contains("set-url")); (strip > 0).ShouldBe(tokened, "fixture check: only a tokened clone strips its origin"); + if (tokened) runner.Specs[strip].Args[^1].ShouldBe("https://example.test/repo.git", "the strip stays as a belt: it sets origin to the URL the clone already named, a no-op"); for (var i = 0; i < runner.Specs.Count; i++) - TokenedGitSpecs.RunsTokened(runner.Specs[i], "https://example.test").ShouldBe(tokened && i < strip, string.Join(' ', runner.Specs[i].Args)); + TokenedGitSpecs.RunsTokened(runner.Specs[i], "https://example.test/repo.git").ShouldBe(tokened && i < strip, string.Join(' ', runner.Specs[i].Args)); + + runner.Specs.Where(s => TokenedGitSpecs.ArgvCarriesACredential(s, "test-token")).Select(s => string.Join(' ', s.Args)).ShouldBeEmpty("the probe and the clone name the remote without its credential"); + runner.Specs.Where(s => TokenedGitSpecs.RunsTokened(s, "https://example.test/repo.git")).ShouldAllBe(s => TokenedGitSpecs.CarriesTheCredential(s, "x-access-token", "test-token")); } [Fact] - public async Task Only_the_publish_commands_that_name_the_authed_url_run_as_tokened_commands() + public async Task Only_the_publish_commands_that_reach_the_remote_run_as_tokened_commands() { - // The LFS upload, the push and its readback carry the token in their argv. Every other post-turn command — over the - // agent clone or in the publish repo — reaches no tokened remote and is left as written. + // The LFS upload, the push and its readback reach the remote, so they carry the token — in their environment, never + // their argv. Every other post-turn command — over the agent clone or in the publish repo — reaches no remote and is + // left as written. var runner = new PostTurnRunner(agentCommittedItself: false); var provider = new LocalGitWorkspaceProvider(new SandboxRunnerRegistry(new[] { runner }), NullLogger.Instance); const string token = "fixture-token"; @@ -679,11 +662,11 @@ public async Task Only_the_publish_commands_that_name_the_authed_url_run_as_toke (await ((IWorkspacePushHandle)handle).PushChangesAsync("codespace/run", CancellationToken.None)).ShouldBe("codespace/run"); var postTurn = runner.Specs.Skip(prepared).ToList(); - var tokened = postTurn.Where(s => s.Args.Any(a => a.Contains(token))).ToList(); + var tokened = postTurn.Where(s => TokenedGitSpecs.RunsTokened(s, "https://example.test/repo.git")).ToList(); - tokened.Select(Subcommand).ShouldBe(new[] { "lfs", "push", "ls-remote" }, "fixture check: the LFS upload, the push and the readback all ran"); - tokened.ShouldAllBe(s => TokenedGitSpecs.RunsTokened(s, "https://example.test")); - postTurn.Where(s => !tokened.Contains(s)).ShouldAllBe(s => !TokenedGitSpecs.RunsTokened(s, "https://example.test")); + tokened.Select(Subcommand).ShouldBe(new[] { "lfs", "push", "ls-remote" }, "the LFS upload, the push and the readback, and nothing else"); + tokened.ShouldAllBe(s => TokenedGitSpecs.CarriesTheCredential(s, "x-access-token", token) && s.Args.Contains("https://example.test/repo.git")); + postTurn.Where(s => TokenedGitSpecs.ArgvCarriesACredential(s, token)).Select(s => string.Join(' ', s.Args)).ShouldBeEmpty(); } /// The git subcommand, past any leading -c key=value and -C dir. diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/RemoteTipResolverTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/RemoteTipResolverTests.cs index a54c591e0..3e765f72c 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/RemoteTipResolverTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/RemoteTipResolverTests.cs @@ -181,8 +181,9 @@ public void SanitizeUrl_strips_userinfo_and_leaves_clean_urls_alone() [InlineData(false)] public async Task The_launch_probe_runs_as_a_tokened_command_only_when_it_carries_a_token(bool tokened) { - // The probe names the authed URL, so a tokened probe must leave nothing for an operator's store helper or trace2 - // target to keep; an untokened one keeps both, and the helpers may be how it authenticates. + // The probe names the remote without its credential and carries the token in its environment, so a tokened probe + // must leave nothing for an operator's store helper or trace2 target to keep; an untokened one keeps both, and the + // helpers may be how it authenticates. var tip = new string('c', 40); var runner = new LsRemoteRunner($"{tip}\trefs/heads/main\n"); var request = new WorkspaceRequest { RepositoryUrl = "https://example.test/repo.git", Token = tokened ? "test-token" : null, Ref = "main" }; @@ -191,7 +192,9 @@ public async Task The_launch_probe_runs_as_a_tokened_command_only_when_it_carrie sha.ShouldBe(tip); var probe = runner.Specs.ShouldHaveSingleItem(); - TokenedGitSpecs.RunsTokened(probe, "https://example.test").ShouldBe(tokened, string.Join(' ', probe.Args)); + TokenedGitSpecs.RunsTokened(probe, "https://example.test/repo.git").ShouldBe(tokened, string.Join(' ', probe.Args)); + probe.Args.ShouldBe(new[] { "ls-remote", "https://example.test/repo.git", "refs/heads/main" }, "the remote is named without its credential"); + if (tokened) TokenedGitSpecs.CarriesTheCredential(probe, "x-access-token", "test-token").ShouldBeTrue(); } // ─── harness (the LocalGitWorkspaceProviderTests pattern) ─────────────────────── diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/TokenedGitCommandTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/TokenedGitCommandTests.cs index e134408fb..3fb4c3592 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/TokenedGitCommandTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/TokenedGitCommandTests.cs @@ -1,95 +1,259 @@ +using CodeSpace.Core.Services.Agents.Sandbox; +using CodeSpace.Core.Services.Agents.Sandbox.Runners; using CodeSpace.Core.Services.Agents.Workspace; -using CodeSpace.Core.Services.Agents.Workspace.Providers; using CodeSpace.Messages.Agents; using Shouldly; namespace CodeSpace.UnitTests.Workflows; /// -/// — the one place a git command is marked as carrying a clone token. Pins the scoped -/// reset and the trace2 switch literally, and which remote URLs count as tokened; the call sites are pinned next to their -/// own classes, and TokenedGitCredentialHelperFlowTests proves both against real git, a real store helper, a -/// proxy whose password that helper holds, and real trace2 targets. +/// — the one way a platform git command carries a clone credential. Pins literally the +/// variables and the credential helper, the scoped config, remote pin and trace2 switch the environment carries, the state +/// directory the runner makes for it, which remotes are tokened and how their URL sheds its userinfo; runs the helper +/// through a real shell. The call sites are pinned next to their own classes, and TokenedGitCredentialHelperFlowTests +/// proves the whole against real git and git-lfs, a real store helper, a proxy whose password that helper holds, a remote +/// that redirects to a harvester, a remote that refuses the token, operator URL rewrites, and real trace2 targets. /// [Trait("Category", "Unit")] public sealed class TokenedGitCommandTests { - [Theory] - [InlineData("https://x-access-token:ghp_abc@github.com/org/repo.git", "credential.https://github.com.helper=")] - [InlineData("http://x-access-token:t@127.0.0.1:8080/remote.git", "credential.http://127.0.0.1:8080.helper=")] - [InlineData("https://oauth2:p%40ss%2Fword@GitLab.Example.com:8443/org/repo.git", "credential.https://gitlab.example.com:8443.helper=")] - public void The_reset_is_scoped_to_the_tokened_remote_and_pinned_literally(string url, string reset) + [Fact] + public void The_credential_variables_and_the_helper_are_pinned_literally() { - // EMPTY, not false: an empty helper value clears the list collected so far, any other value adds one more helper. - // Scoped to the remote's scheme and authority: it clears every helper git would ask about that remote, and leaves - // the ones a proxy or another host needs. Never the userinfo — the key must not carry the token itself. - TokenedGitCommand.CredentialHelperReset(url).ShouldBe(new[] { "-c", reset }); + // The helper reads the credential from these two variables — never GIT_-prefixed, since git-lfs writes every GIT_* + // variable into its own logs inside the clone — and writes it with the shell's builtin printf, so the values never + // reach an argv. It answers get until git or git-lfs tells it, by erase, that the remote refused the credential; it + // records that in the command's own state directory, which the runner makes and removes, and answers nothing after. + TokenedGitCommand.UsernameVariable.ShouldBe("CODESPACE_GIT_USERNAME"); + TokenedGitCommand.PasswordVariable.ShouldBe("CODESPACE_GIT_PASSWORD"); + TokenedGitCommand.StateVariable.ShouldBe(TokenedGitSpecs.StateVariable); + TokenedGitCommand.CredentialHelper.ShouldBe(TokenedGitSpecs.Helper); } [Fact] public void Trace2_off_is_pinned_literally() { // git's own names for its three trace2 targets. The environment wins over the trace2.*Target an operator set in - // system or global config, which git reads before any -c. + // system or global config, which git reads before any -c or environment config. TokenedGitCommand.TraceOff.OrderBy(kv => kv.Key, StringComparer.Ordinal).Select(kv => $"{kv.Key}={kv.Value}").ShouldBe(new[] { "GIT_TRACE2=0", "GIT_TRACE2_EVENT=0", "GIT_TRACE2_PERF=0" }); } [Theory] - [InlineData("https://x-access-token:ghp_abc@github.com/org/repo.git")] - [InlineData("https://oauth2:p%40ss%2Fword@gitlab.com/org/repo.git")] - [InlineData("http://x-access-token:t@127.0.0.1:8080/remote.git")] - public void A_url_carrying_a_password_is_tokened(string url) => TokenedGitCommand.IsTokened(url).ShouldBeTrue(); + [InlineData("https://github.com/org/repo.git", null, "ghp_abc", "https://github.com", "x-access-token")] // GitHub's token user, the default + [InlineData("https://gitlab.com/org/repo.git", "oauth2", "glpat_xyz", "https://gitlab.com", "oauth2")] // GitLab's + [InlineData("http://127.0.0.1:8080/remote.git", "x-access-token", "t", "http://127.0.0.1:8080", "x-access-token")] // a non-default port is part of the scope + [InlineData("https://GitLab.Example.com:8443/org/repo.git", "oauth2", "p@ss/w+rd", "https://gitlab.example.com:8443", "oauth2")] + public void A_tokened_command_carries_the_credential_in_its_environment_for_the_remotes_scope_alone(string url, string? tokenUsername, string token, string scope, string username) + { + // The reset (an EMPTY helper, not false: only the empty value clears the list) and the helper are both scoped to the + // remote's scheme and authority, never its path and never a userinfo: the operator's helpers for that remote are + // silenced, the ones a proxy or another host needs still answer, and a redirect to another authority finds no helper + // that answers with the token. The token rides raw — it is in no URL, so nothing escapes it. + var remote = TokenedGitCommand.RemoteFor(url, tokenUsername, token); + var spec = new SandboxSpec { Command = "git", Args = new[] { "ls-remote", remote.Url }, TimeoutSeconds = 30, AllowNetwork = true }; + + var tokened = TokenedGitCommand.Spec(remote, spec); + + TokenedGitSpecs.RunsTokened(tokened, remote.Url).ShouldBeTrue(); + TokenedGitSpecs.ConfigFor(remote.Url)["GIT_CONFIG_KEY_1"].ShouldBe($"credential.{scope}.helper", "fixture check: the helper answers for the remote's scheme and authority"); + TokenedGitSpecs.CarriesTheCredential(tokened, username, token).ShouldBeTrue(); + } + + [Fact] + public void A_tokened_command_maps_its_remote_to_itself_so_no_operator_rewrite_moves_it() + { + // git and git-lfs rewrite a URL by the longest url..insteadOf (or pushInsteadOf) prefix that matches it. A rule + // naming the whole URL is the longest any rule can be, and it maps the URL to itself: an operator's + // url."git@host:".insteadOf=https://host/ cannot move a tokened command to SSH under the host's key, nor a rule that + // carries an operator's own token swap the team's credential for it. Before the token left the URL, no such rule + // matched a tokened URL; this keeps that. + var remote = TokenedGitCommand.RemoteFor("https://github.com/org/repo.git", null, "ghp_abc"); + + var environment = TokenedGitCommand.Spec(remote, new SandboxSpec { Command = "git", Args = new[] { "push", remote.Url, "b:b" } }).Environment; + + var entries = Enumerable.Range(0, int.Parse(environment["GIT_CONFIG_COUNT"])).Select(i => $"{environment[$"GIT_CONFIG_KEY_{i}"]}={environment[$"GIT_CONFIG_VALUE_{i}"]}").ToList(); + entries.ShouldContain("url.https://github.com/org/repo.git.insteadOf=https://github.com/org/repo.git"); + entries.ShouldContain("url.https://github.com/org/repo.git.pushInsteadOf=https://github.com/org/repo.git"); + } + + [Fact] + public void A_tokened_command_asks_the_runner_for_its_own_state_directory() + { + // The helper records a refusal in a directory only this command sees: the runner makes it owner-only, binds it into a + // confined command, points the variable at it and removes it afterwards. The command's own requests are kept. + var remote = TokenedGitCommand.RemoteFor("https://host/r.git", null, "t"); + var spec = new SandboxSpec { Command = "git", Args = new[] { "clone", remote.Url, "/tmp/x" }, ConfigHomeEnvVars = new[] { "OTHER_HOME" } }; + + TokenedGitCommand.Spec(remote, spec).ConfigHomeEnvVars.ShouldBe(new[] { "OTHER_HOME", "CODESPACE_GIT_STATE" }); + } + + [Fact] + public void A_tokened_command_keeps_its_argv_and_everything_else() + { + // Nothing about the credential reaches the argv — not even the reset as a -c: git reads GIT_CONFIG_PARAMETERS after + // GIT_CONFIG_COUNT, so a -c credential..helper= would empty the list again, the helper with it. + var remote = TokenedGitCommand.RemoteFor("https://host/r.git", null, "t"); + var spec = new SandboxSpec { Command = "git", Args = new[] { "-c", "lfs.locksverify=false", "lfs", "push", remote.Url, "branch" }, Environment = new Dictionary { ["LANG"] = "C", ["GIT_TRACE2"] = "/tmp/trace" }, TimeoutSeconds = 30, AllowNetwork = true }; + + var tokened = TokenedGitCommand.Spec(remote, spec); + + tokened.Args.ShouldBe(spec.Args); + tokened.Environment["LANG"].ShouldBe("C", "the command's own environment is kept"); + tokened.Environment["GIT_TRACE2"].ShouldBe("0", "trace2 off wins over any value the command brought"); + (tokened with { Environment = spec.Environment, ConfigHomeEnvVars = spec.ConfigHomeEnvVars }).ShouldBe(spec, "nothing else about the command changes"); + TokenedGitSpecs.RunsTokened(tokened, "https://host/r.git").ShouldBeTrue(); + } + + [Theory] + [InlineData("https://github.com/org/repo.git", null, "p@ss/w+rd", "https://github.com/org/repo.git", "x-access-token")] + [InlineData("https://gitlab.com/org/repo.git", "oauth2", "glpat_xyz", "https://gitlab.com/org/repo.git", "oauth2")] + [InlineData("https://git.local:8443/org/repo.git", "oauth2", "t", "https://git.local:8443/org/repo.git", "oauth2")] + [InlineData("https://stale:old-secret@github.com/org/repo.git", null, "fresh", "https://github.com/org/repo.git", "x-access-token")] // a token replaces a stored credential, which leaves the URL too + public void With_a_token_the_remote_is_named_without_userinfo(string url, string? tokenUsername, string token, string named, string username) + { + var remote = TokenedGitCommand.RemoteFor(url, tokenUsername, token); + + remote.ShouldBe(new TokenedGitCommand.Remote(named, username, token)); + remote.IsTokened.ShouldBeTrue(); + } + + [Theory] + [InlineData("https://x-access-token:ghp_abc@github.com/org/repo.git", "https://github.com/org/repo.git", "x-access-token", "ghp_abc")] + [InlineData("https://oauth2:p%40ss%2Fword@gitlab.com/org/repo.git", "https://gitlab.com/org/repo.git", "oauth2", "p@ss/word")] // decoded, as git decodes it + [InlineData("http://x-access-token:t@127.0.0.1:8080/remote.git", "http://127.0.0.1:8080/remote.git", "x-access-token", "t")] + public void A_url_carrying_a_password_gives_it_up_to_the_environment(string url, string named, string username, string password) + { + // A stored URL can carry its own credential; it travels the same way as a token, so it reaches no argv either. + TokenedGitCommand.RemoteFor(url, null, null).ShouldBe(new TokenedGitCommand.Remote(named, username, password)); + } [Theory] [InlineData("https://github.com/org/repo.git")] - [InlineData("https://user@github.com/org/repo.git")] // a username alone is not a secret - [InlineData("https://user:@github.com/org/repo.git")] // nor is an empty password + [InlineData("https://user@github.com/org/repo.git")] // a username alone is not a secret, and the operator's helper may answer for it + [InlineData("https://user:@github.com/org/repo.git")] // nor is an empty password [InlineData("ssh://git@github.com/org/repo.git")] + [InlineData("ssh://git:pw@github.com/org/repo.git")] // only the http transport asks a credential helper [InlineData("git@github.com:org/repo.git")] [InlineData("file:///srv/repo.git")] [InlineData("/srv/repo.git")] - public void A_url_without_a_password_is_not_tokened(string url) => TokenedGitCommand.IsTokened(url).ShouldBeFalse(); + public void A_url_without_a_password_is_left_as_written_and_untokened(string url) + { + var remote = TokenedGitCommand.RemoteFor(url, null, null); + var spec = new SandboxSpec { Command = "git", Args = new[] { "clone", url, "/tmp/x" } }; - [Fact] - public void Every_authenticated_url_the_platform_builds_is_tokened() + remote.ShouldBe(new TokenedGitCommand.Remote(url, null, null)); + remote.IsTokened.ShouldBeFalse(); + TokenedGitCommand.Spec(remote, spec).ShouldBeSameAs(spec, "the operator's helpers may be how an untokened remote authenticates"); + } + + [Theory] + [InlineData("https://ghp_pasted@host/r.git", "ghp_pasted", "")] // a token pasted as the user alone: sent with an empty password, as curl sent it from the URL + [InlineData("https://ghp_pasted:x-oauth-basic@host/r.git", "ghp_pasted", "x-oauth-basic")] + [InlineData("https://fake%2fpasted%40token@host/r.git", "fake/pasted@token", "")] + public void A_pasted_userinfo_moves_whole_into_the_environment(string url, string username, string password) { - TokenedGitCommand.IsTokened(LocalGitWorkspaceProvider.BuildAuthenticatedUrl("https://github.com/org/repo.git", null, "p@ss/w+rd")).ShouldBeTrue(); - TokenedGitCommand.IsTokened(LocalGitWorkspaceProvider.BuildAuthenticatedUrl("https://gitlab.com/org/repo.git", "oauth2", "glpat")).ShouldBeTrue(); - TokenedGitCommand.IsTokened(LocalGitWorkspaceProvider.BuildAuthenticatedUrl("https://github.com/org/repo.git", null, null)).ShouldBeFalse("no token: the URL is left as the operator stored it"); + // A pasted pack URL's bare user is a token; its caller knows that and moves the whole userinfo out of the URL. + var remote = TokenedGitCommand.FromUserInfo(url); + + remote.ShouldBe(new TokenedGitCommand.Remote("https://host/r.git", username, password)); + remote.IsTokened.ShouldBeTrue(); } - [Fact] - public void A_tokened_command_gets_the_reset_ahead_of_its_arguments_and_trace2_off() + [Theory] + [InlineData("get", true)] + [InlineData("store", false)] + [InlineData("erase", false)] + public async Task The_helper_answers_get_from_the_environment_and_nothing_else(string operation, bool answers) { - const string url = "https://x-access-token:t@host/r.git"; - var spec = new SandboxSpec { Command = "git", Args = new[] { "-c", "lfs.locksverify=false", "lfs", "push", url, "branch" }, Environment = new Dictionary { ["LANG"] = "C", ["GIT_TRACE2"] = "/tmp/trace" }, TimeoutSeconds = 30, AllowNetwork = true }; + // Run the helper as git runs a '!' helper — `sh -c ' '` — with the credential in its environment. + // Shell and printf metacharacters in the token arrive verbatim: it is an argument to printf's %s, never its format. + if (OperatingSystem.IsWindows()) return; - var tokened = TokenedGitCommand.Spec(url, spec); + using var state = new HelperState(); - tokened.Args.ShouldBe(new[] { "-c", "credential.https://host.helper=" }.Concat(spec.Args)); - tokened.Environment["LANG"].ShouldBe("C", "the command's own environment is kept"); - foreach (var (name, value) in TokenedGitCommand.TraceOff) tokened.Environment[name].ShouldBe(value, "trace2 off wins over any value the command brought"); - (tokened with { Args = spec.Args, Environment = spec.Environment }).ShouldBe(spec, "nothing else about the command changes"); - TokenedGitSpecs.RunsTokened(tokened, "https://host").ShouldBeTrue(); + var result = await state.RunAsync(operation); + + result.Status.ShouldBe(SandboxStatus.Success, result.Stderr); + result.Stdout.ShouldBe(answers ? $"username=x-access-token\npassword={HelperState.Password}\n" : ""); } [Fact] - public void An_untokened_command_is_left_as_written() + public async Task The_helper_stops_answering_once_the_remote_refused_the_credential() + { + // git-lfs answers a 401 by telling the helpers to erase the credential and asking again, with no limit: a helper that + // keeps answering keeps it retrying for the whole command timeout, tens of failed logins a second. git itself stops + // after one erase. So erase records the refusal in the command's state directory, and get then answers nothing and + // says why — git-lfs fails at once, and its message carries the reason. A store changes nothing. + if (OperatingSystem.IsWindows()) return; + + using var state = new HelperState(); + + (await state.RunAsync("store")).Stdout.ShouldBe(""); + (await state.RunAsync("get")).Stdout.ShouldNotBeEmpty("a store is not a refusal"); + + (await state.RunAsync("erase")).Stdout.ShouldBe(""); + var refused = await state.RunAsync("get"); + + refused.Stdout.ShouldBe("", "a refused credential is not offered again"); + refused.Stderr.ShouldContain("the remote refused this credential"); + Directory.EnumerateFileSystemEntries(state.Directory).Select(Path.GetFileName).ShouldBe(new[] { "refused" }, "the record is a marker, holding nothing"); + File.ReadAllText(Path.Combine(state.Directory, "refused")).ShouldBeEmpty(); + } + + [Theory] + [InlineData(null)] // a runner that did not make the directory + [InlineData("missing")] // or one that is gone + public async Task Without_its_state_directory_the_helper_answers_nothing(string? state) { - var spec = new SandboxSpec { Command = "git", Args = new[] { "clone", "https://host/r.git", "/tmp/x" } }; + // A helper that could not record a refusal could not stop git-lfs retrying one, so it never answers: the command + // fails at once rather than loop. + if (OperatingSystem.IsWindows()) return; + + using var helper = new HelperState(); + var directory = state is null ? null : Path.Combine(helper.Directory, state); - TokenedGitCommand.Spec("https://host/r.git", spec).ShouldBeSameAs(spec, "the operator's helpers may be how an untokened remote authenticates"); + var result = await helper.RunAsync("get", directory); + + result.Stdout.ShouldBe(""); } [Fact] - public void A_caller_that_knows_a_bare_user_is_a_token_marks_the_command_tokened() + public void The_argv_detector_sees_a_basic_credential_for_any_user() { - // A stored https://user@mirror URL may authenticate through the operator's helper, so Spec leaves it alone; a pasted - // pack URL's bare user is a token, and its caller marks the command itself. - const string url = "https://ghp_pasted@host/r.git"; - var spec = new SandboxSpec { Command = "git", Args = new[] { "clone", url, "/tmp/x" } }; + // Fixture check for every call-site pin: the token rides an argv as a Basic Authorization header too (an http.extraHeader), + // base64-encoded behind its user — any user, GitHub's or GitLab's, shifting it within base64's three-byte groups. + const string token = "fake-publish-token-0123456789"; + + foreach (var user in new[] { "x-access-token", "oauth2", "u", "ab" }) + { + var header = "http.extraHeader=Authorization: Basic " + Convert.ToBase64String(System.Text.Encoding.UTF8.GetBytes($"{user}:{token}")); + + TokenedGitSpecs.ArgvCarriesACredential(new SandboxSpec { Command = "git", Args = new[] { "-c", header, "clone", "https://host/r.git" } }, token).ShouldBeTrue(user); + } + + TokenedGitSpecs.ArgvCarriesACredential(new SandboxSpec { Command = "git", Args = new[] { "clone", "https://host/r.git", "/tmp/fake-publish-token" } }, token).ShouldBeFalse("fixture check: a prefix of the token is not the token"); + } + + /// The helper run as git runs a '!' helper, with a fixed credential and its own state directory, removed on dispose. + private sealed class HelperState : IDisposable + { + public const string Password = "p@ss/w+rd $HOME `id` \\n %s \"'"; + + public string Directory { get; } = System.IO.Directory.CreateTempSubdirectory("cs-helper-state-").FullName; + + public Task RunAsync(string operation) => RunAsync(operation, Directory); + + public Task RunAsync(string operation, string? state) + { + var environment = new Dictionary { [TokenedGitCommand.UsernameVariable] = "x-access-token", [TokenedGitCommand.PasswordVariable] = Password }; + if (state is not null) environment[TokenedGitCommand.StateVariable] = state; + + return new LocalProcessRunner().RunAsync(new SandboxSpec { Command = "/bin/sh", Args = new[] { "-c", $"{TokenedGitCommand.CredentialHelper[1..]} {operation}" }, Environment = environment, TimeoutSeconds = 15 }, CancellationToken.None); + } - TokenedGitCommand.Spec(url, spec).ShouldBeSameAs(spec); - TokenedGitSpecs.RunsTokened(TokenedGitCommand.AsTokened(url, spec), "https://host").ShouldBeTrue(); + public void Dispose() + { + try { System.IO.Directory.Delete(Directory, recursive: true); } catch { /* best-effort */ } + } } }