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 */ } + } } }