Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,62 @@ it.effect.each([
}),
);

it.effect.each([
["a plain token", "s3cret"],
["a token with an apostrophe", "s3'cret"],
["a token with a double quote", 's3"cret'],
["a host-only token with an at sign", "abc@secret"],
["an unquoted token with an apostrophe", "s3'cret", true],
["an unquoted token with a double quote", 's3"cret', true],
["an unquoted URL with an apostrophe in its path", "path-token", true],
] as const)("keeps %s in the query out of a failed clone's error text", ([, token, unquoted]) =>
Effect.gen(function* () {
const remote =
token === "abc@secret"
? "https://github.com"
: token === "path-token"
? "https://github.com/octocat/t3code's.git"
: "https://github.com/octocat/t3code.git";
const error = yield* Effect.flip(
cloneInTemporaryDirectory(
{
execute: (input) =>
Effect.gen(function* () {
const cloneUrl = input.args.at(-2)!;
// Git treats @ in a pathless URL's query as userinfo, losing the query delimiter.
const diagnosticUrl =
cloneUrl === "https://github.com?access_token=abc@secret"
? "https://secret"
: cloneUrl;
yield* (
input.progress?.onStderrLine?.(
unquoted
? `fatal: ${diagnosticUrl}/info/refs not valid: is this a git repository?`
: `fatal: unable to access '${diagnosticUrl}/': Could not resolve host: github.com`,
) ?? Effect.void
);
return yield* new GitCommandError({
operation: input.operation,
command: "git",
cwd: input.cwd,
detail: "Git command exited with a non-zero status.",
exitCode: 128,
});
}),
},
{ remoteUrl: `${remote}?access_token=${token}` },
),
);

assert.strictEqual(
error.detail,
unquoted
? `fatal: ${remote} not valid: is this a git repository?`
: `fatal: unable to access '${remote}${token === "abc@secret" ? "/" : ""}': Could not resolve host: github.com`,
);
}),
);

it.effect("strips embedded credentials from the remote URL it reports", () =>
Effect.gen(function* () {
const fs = yield* FileSystem.FileSystem;
Expand Down
16 changes: 12 additions & 4 deletions apps/server/src/sourceControl/SourceControlRepositoryService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ function toRepositoryInfo(
};
}

// Redact credentials from client clone snapshots; git still receives the original URL.
// Redact credentials from client clone snapshots; git still receives the credentials.
function redactRemoteUrl(remoteUrl: string): string {
try {
const url = new URL(remoteUrl);
Expand All @@ -128,10 +128,16 @@ function redactRemoteUrl(remoteUrl: string): string {
}

// Userinfo can contain @; remove through the last one before the host boundary.
const URL_WITH_USERINFO = /\b([a-z][a-z0-9+.-]*:\/\/)[^\s/]+@/gi;
const URL_WITH_USERINFO = /\b([a-z][a-z0-9+.-]*:\/\/)[^\s/?#]+@/gi;
// Git leaves quotes inside query tokens unescaped; consume through the closing quote.
const QUOTED_URL_QUERY = /(['"])([a-z][a-z0-9+.-]*:\/\/[^\s?]*)\?.*\1(?=[\s:]|$)/gi;
const URL_QUERY = /\b([a-z][a-z0-9+.-]*:\/\/[^\s?]*)\?[^\s]*/gi;

function redactUrlCredentials(text: string): string {
return text.replace(URL_WITH_USERINFO, "$1");
return text
.replace(QUOTED_URL_QUERY, "$1$2$1")
.replace(URL_QUERY, "$1")
.replace(URL_WITH_USERINFO, "$1");
}

// The remote already exists after push failure; advise fixing access and pushing.
Expand Down Expand Up @@ -340,11 +346,13 @@ export const make = Effect.gen(function* () {
if (stderrTail.length > 4) stderrTail.shift();
});
};
// Git misreads @ in a pathless URL's query as userinfo and echoes its suffix as the host.
const normalizedCloneUrl = cloneUrl.replace(/^([a-z][a-z0-9+.-]*:\/\/[^/?#]+)\?/i, "$1/?");
yield* git
.execute({
operation: "SourceControlRepositoryService.cloneRepository",
cwd: path.dirname(destinationPath),
args: ["clone", "--progress", "--", cloneUrl, path.basename(destinationPath)],
args: ["clone", "--progress", "--", normalizedCloneUrl, path.basename(destinationPath)],
timeoutMs: options?.timeoutMs === undefined ? CLONE_TIMEOUT_MS : options.timeoutMs,
// Buffered output is unused and progress can be huge; cap it while retaining line callbacks.
maxOutputBytes: 256 * 1024,
Expand Down
Loading