Skip to content

Commit 2b423ba

Browse files
fix(worker): Remove setting remote.origin.url for remote git repositories (#483)
1 parent ca9069e commit 2b423ba

File tree

6 files changed

+121
-59
lines changed

6 files changed

+121
-59
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
- Remove setting `remote.origin.url` for remote git repositories. [#483](https://github.com/sourcebot-dev/sourcebot/pull/483)
12+
1013
### Changed
1114
- Updated NextJS to version 15. [#477](https://github.com/sourcebot-dev/sourcebot/pull/477)
1215
- Add `sessionToken` as optional Bedrock configuration parameter. [#478](https://github.com/sourcebot-dev/sourcebot/pull/478)

packages/backend/src/env.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ export const env = createEnv({
4343

4444
LOGTAIL_TOKEN: z.string().optional(),
4545
LOGTAIL_HOST: z.string().url().optional(),
46+
SOURCEBOT_LOG_LEVEL: z.enum(["info", "debug", "warn", "error"]).default("info"),
4647

4748
DATABASE_URL: z.string().url().default("postgresql://postgres:postgres@localhost:5432/postgres"),
4849
CONFIG_PATH: z.string().optional(),

packages/backend/src/git.ts

Lines changed: 75 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,72 @@
11
import { CheckRepoActions, GitConfigScope, simpleGit, SimpleGitProgressEvent } from 'simple-git';
2+
import { mkdir } from 'node:fs/promises';
3+
import { env } from './env.js';
24

35
type onProgressFn = (event: SimpleGitProgressEvent) => void;
46

5-
export const cloneRepository = async (cloneURL: string, path: string, onProgress?: onProgressFn) => {
6-
const git = simpleGit({
7-
progress: onProgress,
8-
});
7+
export const cloneRepository = async (
8+
remoteUrl: URL,
9+
path: string,
10+
onProgress?: onProgressFn
11+
) => {
912
try {
10-
await git.clone(
11-
cloneURL,
12-
path,
13-
[
14-
"--bare",
15-
]
16-
);
13+
await mkdir(path, { recursive: true });
1714

18-
await git.cwd({
15+
const git = simpleGit({
16+
progress: onProgress,
17+
}).cwd({
1918
path,
20-
}).addConfig("remote.origin.fetch", "+refs/heads/*:refs/heads/*");
19+
})
20+
21+
await git.init(/*bare = */ true);
22+
23+
await git.fetch([
24+
remoteUrl.toString(),
25+
// See https://git-scm.com/book/en/v2/Git-Internals-The-Refspec
26+
"+refs/heads/*:refs/heads/*",
27+
"--progress",
28+
]);
2129
} catch (error: unknown) {
22-
if (error instanceof Error) {
23-
throw new Error(`Failed to clone repository: ${error.message}`);
30+
const baseLog = `Failed to clone repository: ${path}`;
31+
32+
if (env.SOURCEBOT_LOG_LEVEL !== "debug") {
33+
// Avoid printing the remote URL (that may contain credentials) to logs by default.
34+
throw new Error(`${baseLog}. Set environment variable SOURCEBOT_LOG_LEVEL=debug to see the full error message.`);
35+
} else if (error instanceof Error) {
36+
throw new Error(`${baseLog}. Reason: ${error.message}`);
2437
} else {
25-
throw new Error(`Failed to clone repository: ${error}`);
38+
throw new Error(`${baseLog}. Error: ${error}`);
2639
}
2740
}
28-
}
29-
30-
31-
export const fetchRepository = async (path: string, onProgress?: onProgressFn) => {
32-
const git = simpleGit({
33-
progress: onProgress,
34-
});
41+
};
3542

43+
export const fetchRepository = async (
44+
remoteUrl: URL,
45+
path: string,
46+
onProgress?: onProgressFn
47+
) => {
3648
try {
37-
await git.cwd({
49+
const git = simpleGit({
50+
progress: onProgress,
51+
}).cwd({
3852
path: path,
39-
}).fetch(
40-
"origin",
41-
[
42-
"--prune",
43-
"--progress"
44-
]
45-
);
53+
})
54+
55+
await git.fetch([
56+
remoteUrl.toString(),
57+
"+refs/heads/*:refs/heads/*",
58+
"--prune",
59+
"--progress"
60+
]);
4661
} catch (error: unknown) {
47-
if (error instanceof Error) {
48-
throw new Error(`Failed to fetch repository ${path}: ${error.message}`);
62+
const baseLog = `Failed to fetch repository: ${path}`;
63+
if (env.SOURCEBOT_LOG_LEVEL !== "debug") {
64+
// Avoid printing the remote URL (that may contain credentials) to logs by default.
65+
throw new Error(`${baseLog}. Set environment variable SOURCEBOT_LOG_LEVEL=debug to see the full error message.`);
66+
} else if (error instanceof Error) {
67+
throw new Error(`${baseLog}. Reason: ${error.message}`);
4968
} else {
50-
throw new Error(`Failed to fetch repository ${path}: ${error}`);
69+
throw new Error(`${baseLog}. Error: ${error}`);
5170
}
5271
}
5372
}
@@ -76,6 +95,28 @@ export const upsertGitConfig = async (path: string, gitConfig: Record<string, st
7695
}
7796
}
7897

98+
/**
99+
* Unsets the specified keys in the git config for the repo at the given path.
100+
* If a key is not set, this is a no-op.
101+
*/
102+
export const unsetGitConfig = async (path: string, keys: string[], onProgress?: onProgressFn) => {
103+
const git = simpleGit({
104+
progress: onProgress,
105+
}).cwd(path);
106+
107+
try {
108+
for (const key of keys) {
109+
await git.raw(['config', '--unset', key]);
110+
}
111+
} catch (error: unknown) {
112+
if (error instanceof Error) {
113+
throw new Error(`Failed to unset git config ${path}: ${error.message}`);
114+
} else {
115+
throw new Error(`Failed to unset git config ${path}: ${error}`);
116+
}
117+
}
118+
}
119+
79120
/**
80121
* Returns true if `path` is the _root_ of a git repository.
81122
*/

packages/backend/src/repoManager.ts

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { Connection, PrismaClient, Repo, RepoToConnection, RepoIndexingStatus, S
55
import { GithubConnectionConfig, GitlabConnectionConfig, GiteaConnectionConfig, BitbucketConnectionConfig } from '@sourcebot/schemas/v3/connection.type';
66
import { AppContext, Settings, repoMetadataSchema } from "./types.js";
77
import { getRepoPath, getTokenFromConfig, measure, getShardPrefix } from "./utils.js";
8-
import { cloneRepository, fetchRepository, upsertGitConfig } from "./git.js";
8+
import { cloneRepository, fetchRepository, unsetGitConfig, upsertGitConfig } from "./git.js";
99
import { existsSync, readdirSync, promises } from 'fs';
1010
import { indexGitRepository } from "./zoekt.js";
1111
import { PromClient } from './promClient.js';
@@ -237,12 +237,39 @@ export class RepoManager implements IRepoManager {
237237
await promises.rm(repoPath, { recursive: true, force: true });
238238
}
239239

240+
const credentials = await this.getCloneCredentialsForRepo(repo, this.db);
241+
const remoteUrl = new URL(repo.cloneUrl);
242+
if (credentials) {
243+
// @note: URL has a weird behavior where if you set the password but
244+
// _not_ the username, the ":" delimiter will still be present in the
245+
// URL (e.g., https://:password@example.com). To get around this, if
246+
// we only have a password, we set the username to the password.
247+
// @see: https://www.typescriptlang.org/play/?#code/MYewdgzgLgBArgJwDYwLwzAUwO4wKoBKAMgBQBEAFlFAA4QBcA9I5gB4CGAtjUpgHShOZADQBKANwAoREj412ECNhAIAJmhhl5i5WrJTQkELz5IQAcxIy+UEAGUoCAJZhLo0UA
248+
if (!credentials.username) {
249+
remoteUrl.username = credentials.password;
250+
} else {
251+
remoteUrl.username = credentials.username;
252+
remoteUrl.password = credentials.password;
253+
}
254+
}
255+
240256
if (existsSync(repoPath) && !isReadOnly) {
241-
logger.info(`Fetching ${repo.displayName}...`);
257+
// @NOTE: in #483, we changed the cloning method s.t., we _no longer_
258+
// write the clone URL (which could contain a auth token) to the
259+
// `remote.origin.url` entry. For the upgrade scenario, we want
260+
// to unset this key since it is no longer needed, hence this line.
261+
// This will no-op if the key is already unset.
262+
// @see: https://github.com/sourcebot-dev/sourcebot/pull/483
263+
await unsetGitConfig(repoPath, ["remote.origin.url"]);
242264

243-
const { durationMs } = await measure(() => fetchRepository(repoPath, ({ method, stage, progress }) => {
244-
logger.debug(`git.${method} ${stage} stage ${progress}% complete for ${repo.displayName}`)
245-
}));
265+
logger.info(`Fetching ${repo.displayName}...`);
266+
const { durationMs } = await measure(() => fetchRepository(
267+
remoteUrl,
268+
repoPath,
269+
({ method, stage, progress }) => {
270+
logger.debug(`git.${method} ${stage} stage ${progress}% complete for ${repo.displayName}`)
271+
}
272+
));
246273
const fetchDuration_s = durationMs / 1000;
247274

248275
process.stdout.write('\n');
@@ -251,25 +278,13 @@ export class RepoManager implements IRepoManager {
251278
} else if (!isReadOnly) {
252279
logger.info(`Cloning ${repo.displayName}...`);
253280

254-
const auth = await this.getCloneCredentialsForRepo(repo, this.db);
255-
const cloneUrl = new URL(repo.cloneUrl);
256-
if (auth) {
257-
// @note: URL has a weird behavior where if you set the password but
258-
// _not_ the username, the ":" delimiter will still be present in the
259-
// URL (e.g., https://:password@example.com). To get around this, if
260-
// we only have a password, we set the username to the password.
261-
// @see: https://www.typescriptlang.org/play/?#code/MYewdgzgLgBArgJwDYwLwzAUwO4wKoBKAMgBQBEAFlFAA4QBcA9I5gB4CGAtjUpgHShOZADQBKANwAoREj412ECNhAIAJmhhl5i5WrJTQkELz5IQAcxIy+UEAGUoCAJZhLo0UA
262-
if (!auth.username) {
263-
cloneUrl.username = auth.password;
264-
} else {
265-
cloneUrl.username = auth.username;
266-
cloneUrl.password = auth.password;
281+
const { durationMs } = await measure(() => cloneRepository(
282+
remoteUrl,
283+
repoPath,
284+
({ method, stage, progress }) => {
285+
logger.debug(`git.${method} ${stage} stage ${progress}% complete for ${repo.displayName}`)
267286
}
268-
}
269-
270-
const { durationMs } = await measure(() => cloneRepository(cloneUrl.toString(), repoPath, ({ method, stage, progress }) => {
271-
logger.debug(`git.${method} ${stage} stage ${progress}% complete for ${repo.displayName}`)
272-
}));
287+
));
273288
const cloneDuration_s = durationMs / 1000;
274289

275290
process.stdout.write('\n');
@@ -540,7 +555,7 @@ export class RepoManager implements IRepoManager {
540555

541556
public async validateIndexedReposHaveShards() {
542557
logger.info('Validating indexed repos have shards...');
543-
558+
544559
const indexedRepos = await this.db.repo.findMany({
545560
where: {
546561
repoIndexingStatus: RepoIndexingStatus.INDEXED
@@ -556,7 +571,7 @@ export class RepoManager implements IRepoManager {
556571
const reposToReindex: number[] = [];
557572
for (const repo of indexedRepos) {
558573
const shardPrefix = getShardPrefix(repo.orgId, repo.id);
559-
574+
560575
// TODO: this doesn't take into account if a repo has multiple shards and only some of them are missing. To support that, this logic
561576
// would need to know how many total shards are expected for this repo
562577
let hasShards = false;

packages/web/src/app/[domain]/browse/[...path]/components/codePreviewPanel.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export const CodePreviewPanel = async ({ path, repoName, revisionName, domain }:
4545
displayName: repoInfoResponse.displayName,
4646
webUrl: repoInfoResponse.webUrl,
4747
}}
48+
branchDisplayName={revisionName}
4849
/>
4950
{(fileSourceResponse.webUrl && codeHostInfo) && (
5051
<a

packages/web/src/app/[domain]/browse/[...path]/components/treePreviewPanel.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export const TreePreviewPanel = async ({ path, repoName, revisionName, domain }:
4040
}}
4141
pathType="tree"
4242
isFileIconVisible={false}
43+
branchDisplayName={revisionName}
4344
/>
4445
</div>
4546
<Separator />

0 commit comments

Comments
 (0)