Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
68 changes: 59 additions & 9 deletions electron/projectClone.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,27 @@
* Add project from a URL (#1506 H3): `git clone` into a chosen folder with
* live progress on "clone:progress" and a cancel, then the usual add.
*
* Only https:// and ssh (ssh:// or user@host:path) URLs: other transports
* (file://, ext::, plain paths) can read local files or run commands.
* The target must be missing or an empty folder; whatever this clone
* created is removed again on failure or cancel.
* Only https:// and ssh (ssh:// or user@host:path) URLs, plus GitHub
* `owner/repo` shorthand (#459): other transports (file://, ext::, plain
* paths) can read local files or run commands. GitHub sources go through
* `gh repo clone` when gh is signed in, so private repos clone with the
* user's gh access; otherwise plain `git clone`.
* The target must be missing, an empty folder, or already a clone of the
* same repo (then it is just added); whatever this clone created is removed
* again on failure or cancel.
*/

const fs = require("node:fs");
const path = require("node:path");
const { spawn } = require("node:child_process");
const { expandUserPath } = require("./fsBrowse.js");
const { killTree } = require("./proc.js");
const { gitTryAsync } = require("./worktrees-git.js");
const { ghBin, ghTryAsync, remoteHostOf } = require("./worktrees-pr.js");
const { slugFromRemoteUrl } = require("./services-projects.js");

/** GitHub `owner/repo`; owners never start with a dash, so no flag injection. */
const GITHUB_SHORTHAND = /^[A-Za-z0-9][A-Za-z0-9-]*\/(?!\.\.?$)[A-Za-z0-9._-]+$/;

/** Progress pushes at most this often; git rewrites its line many times a second. */
const PROGRESS_THROTTLE_MS = 100;
Expand Down Expand Up @@ -47,7 +57,8 @@ function cloneUrlError(url) {
// scp-like ssh: user@host:path. The user part is required so a Windows
// drive path or a bare "host:path" local name is never taken for ssh.
if (/^[A-Za-z0-9._-]+@[A-Za-z0-9.-]+:[^/\s].*/.test(u)) return null;
return "Use an https:// or ssh URL (for example git@github.com:owner/repo.git)";
if (GITHUB_SHORTHAND.test(u)) return null;
return "Use owner/repo, an https:// URL, or an ssh URL (for example git@github.com:owner/repo.git)";
}

/**
Expand All @@ -63,6 +74,26 @@ function repoNameFromUrl(url) {
return String(tail || "").replace(/\.git$/i, "");
}

/**
* `host/owner/repo`, lowercased, to tell whether a checkout is this repo.
* @param {string} url
* @returns {string | null}
*/
function repoKey(url) {
if (GITHUB_SHORTHAND.test(url)) return `github.com/${url}`.toLowerCase();
const slug = slugFromRemoteUrl(url);
const host = remoteHostOf(url);
return slug && host ? `${host.host}/${slug}`.toLowerCase() : null;
Comment on lines +84 to +86

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve the full remote path when matching checkouts

When cloning from a host that supports nested namespaces, slugFromRemoteUrl() retains only the final two path segments, so URLs such as https://gitlab.com/team-a/shared/repo and an existing origin at https://gitlab.com/team-b/shared/repo both produce gitlab.com/shared/repo. If the latter occupies the target folder, this equality silently calls addProject() for the wrong checkout instead of rejecting or cloning the requested repository; compare the complete normalized repository path rather than the shortened owner/repo slug.

Useful? React with 👍 / 👎.

}

/** gh is installed and signed in to github.com (local check, no network). */
async function ghSignedIn(cwd) {
const res = await ghTryAsync(cwd, ["auth", "token", "--hostname", "github.com"], {
timeout: 10_000,
});
return res.ok;
}

/**
* Last progress line from a stderr chunk ("Receiving objects: 42% ...").
* @param {string} text
Expand All @@ -88,6 +119,7 @@ function lastLine(text) {
* @param {{
* addProject: (target: string) => Promise<unknown>,
* broadcast?: (channel: string, payload: unknown) => void,
* ghSignedIn?: (cwd: string) => Promise<boolean>,
* }} deps
*/
async function cloneProject(input, deps) {
Expand Down Expand Up @@ -122,10 +154,28 @@ async function cloneProject(input, deps) {
throw new Error(`Already exists and is not a folder: ${target}`);
}
if (entries.length) {
// Same repo already checked out here: add it instead of failing.
// The .git check keeps a plain subfolder of another repo from
// answering with its parent's origin.
const origin = fs.existsSync(path.join(target, ".git"))
? await gitTryAsync(target, ["remote", "get-url", "origin"])
: null;
const want = repoKey(url);
if (want && origin && origin.ok && repoKey(String(origin.stdout).trim()) === want) {
return deps.addProject(target);
}
throw new Error(`Folder is not empty: ${target}`);
}
}

const shorthand = GITHUB_SHORTHAND.test(url);
const github =
shorthand || (remoteHostOf(url) || { host: "" }).host === "github.com";
const viaGh = github && (await (deps.ghSignedIn || ghSignedIn)(parent));
const [bin, args] = viaGh
? [ghBin(), ["repo", "clone", url, target, "--", "--progress"]]
: ["git", ["clone", "--progress", "--", shorthand ? `https://github.com/${url}.git` : url, target]];

const cloneId = typeof input.cloneId === "string" ? input.cloneId : "";
const broadcast = deps.broadcast;
const push = (/** @type {Record<string, unknown>} */ extra) => {
Expand All @@ -148,9 +198,9 @@ async function cloneProject(input, deps) {
};

await new Promise((resolve, reject) => {
const child = spawnFn("git", ["clone", "--progress", "--", url, target], {
const child = spawnFn(bin, args, {
// No hidden credential prompt: fail fast with git's own message.
env: { ...process.env, GIT_TERMINAL_PROMPT: "0" },
env: { ...process.env, GIT_TERMINAL_PROMPT: "0", GH_PROMPT_DISABLED: "1" },
stdio: ["ignore", "ignore", "pipe"],
detached: process.platform !== "win32",
windowsHide: true,
Expand Down Expand Up @@ -178,7 +228,7 @@ async function cloneProject(input, deps) {
child.on("error", (err) => {
if (cloneId) active.delete(cloneId);
cleanup();
reject(new Error(`Could not run git: ${err.message}`));
reject(new Error(`Could not run ${viaGh ? "gh" : "git"}: ${err.message}`));
});
child.on("close", (code) => {
if (cloneId) active.delete(cloneId);
Expand All @@ -193,7 +243,7 @@ async function cloneProject(input, deps) {
.filter((l) => l && !/^(Cloning into|remote: (Counting|Compressing|Enumerating))/.test(l))
.slice(-3)
.join("\n");
reject(new Error(why || `git clone exited with code ${code}`));
reject(new Error(why || `${viaGh ? "gh repo" : "git"} clone exited with code ${code}`));
} else {
resolve(null);
}
Expand Down
79 changes: 78 additions & 1 deletion electron/test/project-clone.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const os = require("node:os");
const path = require("node:path");
const { EventEmitter } = require("node:events");
const { PassThrough } = require("node:stream");
const { execFileSync } = require("node:child_process");
const {
cloneProject,
cancelClone,
Expand All @@ -28,6 +29,8 @@ describe("clone URL rules", () => {
"ssh://git@github.com/owner/repo.git",
"git@github.com:owner/repo.git",
"deploy@host.internal:repos/app",
"owner/repo",
"my-org/app.js",
]) {
assert.equal(cloneUrlError(ok), null, ok);
}
Expand All @@ -42,6 +45,9 @@ describe("clone URL rules", () => {
"github.com:owner/repo",
"https://github.com",
"https://github.com/owner/repo extra",
"-x/repo",
"owner/..",
"a/b/c",
]) {
assert.ok(cloneUrlError(bad), `should reject ${JSON.stringify(bad)}`);
}
Expand All @@ -51,6 +57,7 @@ describe("clone URL rules", () => {
assert.equal(repoNameFromUrl("https://github.com/owner/repo.git"), "repo");
assert.equal(repoNameFromUrl("git@github.com:owner/my-app.git"), "my-app");
assert.equal(repoNameFromUrl("ssh://git@host/a/b/"), "b");
assert.equal(repoNameFromUrl("owner/repo"), "repo");
});
});

Expand All @@ -75,13 +82,20 @@ describe("cloneProject", () => {
return child;
}

let savedGhBin;

beforeEach(() => {
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "coder-clone-"));
spawned = [];
setCloneSpawnFn(fakeSpawn);
// A gh that does not exist: the default sign-in probe says no.
savedGhBin = process.env.CODER_GH_BIN;
process.env.CODER_GH_BIN = path.join(tmpDir, "no-gh");
});

afterEach(() => {
if (savedGhBin === undefined) delete process.env.CODER_GH_BIN;
else process.env.CODER_GH_BIN = savedGhBin;
setCloneSpawnFn(null);
fs.rmSync(tmpDir, { recursive: true, force: true });
});
Expand Down Expand Up @@ -125,6 +139,69 @@ describe("cloneProject", () => {
);
});

const succeed = (child, args) => {
const target = args[0] === "repo" ? args[3] : args[args.length - 1];
fs.mkdirSync(path.join(target, ".git"), { recursive: true });
child.stderr.end();
child.emit("close", 0);
};

it("clones GitHub owner/repo through gh when gh is signed in", async () => {
behave = succeed;
await cloneProject(
{ url: "owner/repo", parentDir: tmpDir },
{ addProject: async (t) => ({ path: t }), ghSignedIn: async () => true },
);
assert.equal(spawned[0].bin, process.env.CODER_GH_BIN);
assert.deepEqual(spawned[0].args, [
"repo",
"clone",
"owner/repo",
path.join(tmpDir, "repo"),
"--",
"--progress",
]);
});

it("falls back to git over https for owner/repo without gh", async () => {
behave = succeed;
await cloneProject(
{ url: "owner/repo", parentDir: tmpDir },
{ addProject: async (t) => ({ path: t }) },
);
assert.equal(spawned[0].bin, "git");
assert.deepEqual(spawned[0].args, [
"clone",
"--progress",
"--",
"https://github.com/owner/repo.git",
path.join(tmpDir, "repo"),
]);
});

it("adds an existing checkout of the same repo instead of cloning", async () => {
const target = path.join(tmpDir, "repo");
fs.mkdirSync(target);
execFileSync("git", ["init", "-q"], { cwd: target });
execFileSync("git", ["remote", "add", "origin", "git@github.com:Owner/Repo.git"], { cwd: target });
const added = [];
await cloneProject(
{ url: "https://github.com/owner/repo", parentDir: tmpDir },
{ addProject: async (t) => added.push(t) },
);
assert.deepEqual(added, [target]);
assert.equal(spawned.length, 0);

await assert.rejects(
cloneProject(
{ url: "https://gitlab.com/owner/repo", parentDir: tmpDir },
{ addProject: async () => ({}) },
),
/Folder is not empty/,
);
assert.equal(spawned.length, 0);
});

it("refuses an existing non-empty target without spawning", async () => {
fs.mkdirSync(path.join(tmpDir, "repo"));
fs.writeFileSync(path.join(tmpDir, "repo", "keep.txt"), "mine");
Expand All @@ -145,7 +222,7 @@ describe("cloneProject", () => {
{ url: "file:///etc", parentDir: tmpDir },
{ addProject: async () => ({}) },
),
/https:\/\/ or ssh/,
/an https:\/\/ URL, or an ssh URL/,
);
assert.equal(spawned.length, 0);
});
Expand Down
2 changes: 2 additions & 0 deletions electron/worktrees-pr.js
Original file line number Diff line number Diff line change
Expand Up @@ -2206,6 +2206,8 @@ async function createPr(opts) {
}

module.exports = {
ghBin,
remoteHostOf,
PR_REFRESH_TIMEOUT_MS,
GH_TIMEOUT_MS,
ghApiTryAsync,
Expand Down
27 changes: 18 additions & 9 deletions src/components/AddProjectPathModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ import type {
ProjectInfo,
WindowsDoctorCheck,
} from "../shared/ipc";
import { getAddProjectInitialQuery, resolveAddProjectPath } from "../browsePath";
import {
getAddProjectInitialQuery,
getBrowseParentPath,
resolveAddProjectPath,
} from "../browsePath";
import { PathBrowser } from "./PathBrowser";
import styles from "./SettingsModal.module.css";

Expand All @@ -20,15 +24,16 @@ function failedDoctorChecks(result: unknown): WindowsDoctorCheck[] {
}

/**
* Client copy of the main-process rule (projectClone.js): https:// or ssh
* only. Main re-checks; this just keeps the button honest.
* Client copy of the main-process rule (projectClone.js): https://, ssh,
* or GitHub owner/repo. Main re-checks; this just keeps the button honest.
*/
export function isCloneUrl(raw: string): boolean {
const u = raw.trim();
return (
/^https:\/\/[^/\s]+\/\S+$/i.test(u) ||
/^ssh:\/\/[^/\s]+\/\S+$/i.test(u) ||
/^[A-Za-z0-9._-]+@[A-Za-z0-9.-]+:[^/\s]\S*$/.test(u)
/^[A-Za-z0-9._-]+@[A-Za-z0-9.-]+:[^/\s]\S*$/.test(u) ||
/^[A-Za-z0-9][A-Za-z0-9-]*\/(?!\.\.?$)[A-Za-z0-9._-]+$/.test(u)
);
}

Expand Down Expand Up @@ -114,8 +119,11 @@ export function AddProjectPathModal({
getAddProjectInitialQuery(null),
);
const [name, setName] = useState("");
// New projects default next to the current one (#459).
const [location, setLocation] = useState(() =>
getAddProjectInitialQuery(null),
getAddProjectInitialQuery(
currentProjectCwd ? getBrowseParentPath(currentProjectCwd) : null,
),
);
const [pending, setPending] = useState(false);
const [error, setError] = useState<string | null>(null);
Expand Down Expand Up @@ -378,23 +386,23 @@ export function AddProjectPathModal({
className={styles.fieldLabel}
htmlFor="add-project-clone-url"
>
Repository URL
Repository
</label>
<input
id="add-project-clone-url"
className={`${styles.input} ${styles.monoInput}`}
data-add-project-clone-url=""
value={cloneUrl}
onChange={(e) => setCloneUrl(e.target.value)}
placeholder="https://github.com/owner/repo.git"
placeholder="owner/repo or https://github.com/owner/repo.git"
autoComplete="off"
spellCheck={false}
disabled={pending}
onKeyDown={onEnter}
/>
{cloneUrl.trim() && !isCloneUrl(cloneUrl) ? (
<p className={styles.fieldNote} data-add-project-clone-url-hint="">
Use an https:// or ssh URL, for example
Use owner/repo, an https:// URL, or an ssh URL such as
git@github.com:owner/repo.git
</p>
) : null}
Expand Down Expand Up @@ -440,7 +448,8 @@ export function AddProjectPathModal({
onKeyDown={onEnter}
/>
<p className={styles.fieldNote}>
The folder must not exist yet, or be empty.
The folder must not exist yet, be empty, or already be a
clone of this repository.
</p>
</div>
{cloneProgress ? (
Expand Down
29 changes: 29 additions & 0 deletions test/addProjectBrowse.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,35 @@ describe("Add project: clone from URL (#1506)", () => {
m.unmount();
});

it("takes owner/repo and defaults next to the current project (#459)", async () => {
const cloned: CloneProjectInput[] = [];
const m = await mount(
<AddProjectPathModal
onClose={() => {}}
onSubmit={async () => ({})}
onCreate={async () => ({})}
onBrowse={browse}
currentProjectCwd="/work/solenta"
onClone={async (input) => {
cloned.push(input);
return { id: "p1" };
}}
/>,
);
await m.click(m.query("[data-add-project-mode-clone]"));
assert.equal(
(m.query("[data-add-project-clone-location]") as HTMLInputElement).value,
"/work/",
);
await m.type(m.query("[data-add-project-clone-url]"), "owner/app");
assert.equal(m.query("[data-add-project-clone-url-hint]"), null);
await m.click(m.query("[data-add-project-path-submit]"));
await m.flush();
assert.equal(cloned[0]!.url, "owner/app");
assert.equal(cloned[0]!.parentDir, "/work");
m.unmount();
});

it("shows a clone failure inline", async () => {
const m = await mount(
<AddProjectPathModal
Expand Down
Loading