Skip to content

Keep clone tokens out of git argv and repository config #2084

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ppXD merged 1 commit into from
Oct 7, 2026
Merged

Keep clone tokens out of git argv and repository config #2084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 18 additions & 17 deletions backend/src/CodeSpace.Core/Services/Agents/PackCloneFetcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.</para>
///
/// <para>A pasted URL can carry a credential in its userinfo (<see cref="PastedSecret"/>). 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.</para>
/// <para>A pasted URL can carry a credential in its userinfo (<see cref="PastedSecret"/>). 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.</para>
/// </summary>
public sealed partial class PackCloneFetcher : IPackSourceFetcher, IWorkspaceJanitor, ISingletonDependency
{
Expand Down Expand Up @@ -74,9 +75,9 @@ public async Task<PackCheckout> FetchAsync(string url, string? reference, Cancel
}

/// <summary>
/// The clone's directory, readable by this worker's uid alone. git writes the pasted URL, credential included, into
/// <c>.git/config</c> 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 <c>.git/config</c>, 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).
/// </summary>
private static void CreateOwnerOnlyDirectory(string dir)
{
Expand All @@ -94,10 +95,10 @@ private async Task CloneAsync(string url, string? reference, string dir, Cancell
}

/// <summary>
/// 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 <see cref="WorkspaceException"/> — 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 <see cref="WorkspaceException"/> — and the caller
/// deletes the clone on the way out.
/// </summary>
private async Task StripPastedCredentialAsync(string url, string dir, CancellationToken cancellationToken)
{
Expand Down Expand Up @@ -132,15 +133,15 @@ internal static IReadOnlyList<string> BuildCloneArgs(string url, string? referen

/// <summary>
/// The clone as the runner gets it: <see cref="BuildCloneArgs"/> in <paramref name="dir"/>, with the network. A pasted URL
/// carrying a credential (<see cref="PastedSecret"/>) clones as a <see cref="TokenedGitCommand"/>, 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
/// <see cref="TokenedGitCommand.IsTokened"/> to find, yet git hands it to every helper it asks for the missing one.
/// carrying a credential (<see cref="PastedSecret"/>) clones as a <see cref="TokenedGitCommand"/>: 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.
/// </summary>
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 ──────────────────────────
Expand Down
42 changes: 21 additions & 21 deletions backend/src/CodeSpace.Core/Services/Agents/Workspace/Integrators/LocalGitBranchIntegrator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 <see cref="IntegrationResult"/>
/// naming what could not be applied — the original K agent branches/patches remain intact for human review.</para>
///
/// <para><b>Secret hygiene</b> 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 (<see cref="LocalGitWorkspaceProvider.Redact"/>), and the
/// transient clone is always removed in a <c>finally</c>. The clone keeps its tokened origin, so the commands whose git
/// transport reaches it — the clone and the push — run as <see cref="TokenedGitCommand"/>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.</para>
/// <para><b>Secret hygiene</b> 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 <see cref="TokenedGitCommand"/>, 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 (<see cref="LocalGitWorkspaceProvider.Redact"/>), and the
/// transient clone is always removed in a <c>finally</c>.</para>
/// </summary>
public sealed class LocalGitBranchIntegrator : IBranchIntegrator, IScopedDependency
{
Expand Down Expand Up @@ -182,11 +182,11 @@ private async Task<IntegrationResult> 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);
}

Expand All @@ -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);
Expand All @@ -222,7 +222,7 @@ private async Task CloneAsync(IntegrationRequest request, string directory, Canc
/// <summary>Check out the EXACT shared base (detached) so every <c>apply --3way</c> resolves the pre-image against the commit the agents saw. False when the SHA is not in history (a bad base).</summary>
private async Task<bool> 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;
}
Expand Down Expand Up @@ -291,21 +291,21 @@ private async Task<bool> IsStrictAncestorAsync(string directory, string ancestor
}

/// <summary>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.</summary>
private async Task<string?> ApplyAllAsync(string directory, IReadOnlyList<ResolvedContribution> resolved, string? token, CancellationToken cancellationToken)
private async Task<string?> ApplyAllAsync(string directory, IReadOnlyList<ResolvedContribution> resolved, IntegrationRequest request, CancellationToken cancellationToken)
{
for (var i = 0; i < resolved.Count; i++)
{
var r = resolved[i];

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");

Expand All @@ -315,7 +315,7 @@ private async Task<bool> 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);
Expand All @@ -326,7 +326,7 @@ private async Task<bool> 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
Expand Down Expand Up @@ -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 ────────────────────────────────────────────────────────────

/// <summary>The remote the integration clone reaches: the authed URL the clone names, which origin keeps to the end.</summary>
private static string RemoteUrl(IntegrationRequest request) => LocalGitWorkspaceProvider.BuildAuthenticatedUrl(request.RepositoryUrl, request.TokenUsername, request.Token);
/// <summary>The remote the integration clone reaches: named by the clone and held by origin without its credential, which the commands that reach it carry.</summary>
private static TokenedGitCommand.Remote Remote(IntegrationRequest request) => TokenedGitCommand.RemoteFor(request.RepositoryUrl, request.TokenUsername, request.Token);

private async Task<bool> HasStagedChangesAsync(string directory, CancellationToken cancellationToken)
{
Expand All @@ -493,9 +493,9 @@ private async Task<string> RevParseTreeAsync(string directory, string rev, Cance
private Task<SandboxResult> RunGitAsync(IReadOnlyList<string> args, string? workingDirectory, CancellationToken cancellationToken) =>
RunSpecAsync(GitSpec(args, workingDirectory), cancellationToken);

/// <summary>Run a command whose git transport reaches the integration clone's tokened origin — the clone, the push — as a <see cref="TokenedGitCommand"/>.</summary>
/// <summary>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 <see cref="TokenedGitCommand"/>.</summary>
private Task<SandboxResult> RunTokenedGitAsync(IntegrationRequest request, IReadOnlyList<string> 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<string> args, string? workingDirectory) =>
new() { Command = "git", Args = args, WorkingDirectory = workingDirectory, TimeoutSeconds = GitTimeoutSeconds, AllowNetwork = true };
Expand Down
Loading
Loading