From 96a156a3765c9924bbb84fff3ffb7192756539a4 Mon Sep 17 00:00:00 2001 From: MocA-Love Date: Sat, 18 Apr 2026 18:21:35 +0900 Subject: [PATCH] Revert "Merge pull request #323 from MocA-Love/fix/simple-git-pager-env" This reverts commit 373682c86ab18af8480687e3b4e2f5a363042833, reversing changes made to b4877184a813e8135082b40649f69f18059c8239. --- .../src/lib/trpc/routers/projects/projects.ts | 13 +- .../routers/workspaces/utils/git-client.ts | 57 +---- packages/host-service/src/runtime/git/git.ts | 16 +- .../src/runtime/git/simple-git.ts | 48 ----- .../src/trpc/router/project/project.ts | 14 +- .../workspace-creation/workspace-creation.ts | 12 +- .../src/trpc/router/workspace/workspace.ts | 4 +- packages/shared/package.json | 4 - packages/shared/src/simple-git-unsafe.ts | 198 ------------------ 9 files changed, 26 insertions(+), 340 deletions(-) delete mode 100644 packages/host-service/src/runtime/git/simple-git.ts delete mode 100644 packages/shared/src/simple-git-unsafe.ts diff --git a/apps/desktop/src/lib/trpc/routers/projects/projects.ts b/apps/desktop/src/lib/trpc/routers/projects/projects.ts index 935a29990f1..73d8c6598ab 100644 --- a/apps/desktop/src/lib/trpc/routers/projects/projects.ts +++ b/apps/desktop/src/lib/trpc/routers/projects/projects.ts @@ -25,7 +25,7 @@ import { } from "main/lib/project-icons"; import { getWorkspaceRuntimeRegistry } from "main/lib/workspace-runtime"; import { PROJECT_COLOR_VALUES } from "shared/constants/project-colors"; -import type { SimpleGitProgressEvent } from "simple-git"; +import simpleGit, { type SimpleGitProgressEvent } from "simple-git"; import { z } from "zod"; import { publicProcedure, router } from "../.."; import { resolveDefaultEditor } from "../external"; @@ -45,11 +45,11 @@ import { refreshDefaultBranch, sanitizeAuthorPrefix, } from "../workspaces/utils/git"; +import { getSimpleGitWithShellPath } from "../workspaces/utils/git-client"; import { - createSimpleGitWithShellPath, - getSimpleGitWithShellPath, -} from "../workspaces/utils/git-client"; -import { execWithShellEnv } from "../workspaces/utils/shell-env"; + execWithShellEnv, + getProcessEnvWithShellPath, +} from "../workspaces/utils/shell-env"; import { getDefaultProjectColor } from "./utils/colors"; import { discoverAndSaveProjectIcon } from "./utils/favicon-discovery"; import { fetchGitHubOwner, getGitHubAvatarUrl } from "./utils/github"; @@ -1493,7 +1493,7 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => { `Preparing clone into ${basename(clonePath)}`, ); try { - const gitWithProgress = await createSimpleGitWithShellPath({ + const gitWithProgress = simpleGit({ abort: abortController.signal, progress: (event: SimpleGitProgressEvent) => { emitCloneEvent({ @@ -1507,6 +1507,7 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => { }); }, }); + gitWithProgress.env(await getProcessEnvWithShellPath()); emitCloneLog( cloneId, `Cloning ${redactGitCredentials(input.url)}`, diff --git a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git-client.ts b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git-client.ts index 1a4a8b2fcd1..b19235050e3 100644 --- a/apps/desktop/src/lib/trpc/routers/workspaces/utils/git-client.ts +++ b/apps/desktop/src/lib/trpc/routers/workspaces/utils/git-client.ts @@ -3,64 +3,15 @@ import { type ExecFileOptionsWithStringEncoding, execFile, } from "node:child_process"; -import { - buildSimpleGitUnsafeOptions, - type SimpleGitUnsafeOptions, -} from "@superset/shared/simple-git-unsafe"; -import simpleGit, { type SimpleGit, type SimpleGitProgressEvent } from "simple-git"; +import simpleGit, { type SimpleGit } from "simple-git"; import { getProcessEnvWithShellPath } from "./shell-env"; -interface CreateSimpleGitWithShellPathOptions { - abort?: AbortSignal; - baseEnv?: NodeJS.ProcessEnv; - progress?: (event: SimpleGitProgressEvent) => void; - repoPath?: string; -} - -function createSimpleGitWithEnv( - env: Record, - options: Omit = {}, -): SimpleGit { - const unsafe = buildSimpleGitUnsafeOptions(env); - const gitOptions: { - abort?: AbortSignal; - baseDir?: string; - progress?: (event: SimpleGitProgressEvent) => void; - unsafe?: SimpleGitUnsafeOptions; - } = {}; - - if (options.abort) { - gitOptions.abort = options.abort; - } - if (options.progress) { - gitOptions.progress = options.progress; - } - if (options.repoPath) { - gitOptions.baseDir = options.repoPath; - } - if (unsafe) { - gitOptions.unsafe = unsafe; - } - - const git = - Object.keys(gitOptions).length > 0 - ? simpleGit(gitOptions as never) - : simpleGit(); - git.env(env); - return git; -} - -export async function createSimpleGitWithShellPath( - options: CreateSimpleGitWithShellPathOptions = {}, -): Promise { - const env = await getProcessEnvWithShellPath(options.baseEnv ?? process.env); - return createSimpleGitWithEnv(env, options); -} - export async function getSimpleGitWithShellPath( repoPath?: string, ): Promise { - return createSimpleGitWithShellPath({ repoPath }); + const git = repoPath ? simpleGit(repoPath) : simpleGit(); + git.env(await getProcessEnvWithShellPath()); + return git; } export async function execGitWithShellPath( diff --git a/packages/host-service/src/runtime/git/git.ts b/packages/host-service/src/runtime/git/git.ts index 6e13281d269..40009cab3cd 100644 --- a/packages/host-service/src/runtime/git/git.ts +++ b/packages/host-service/src/runtime/git/git.ts @@ -1,25 +1,19 @@ +import simpleGit from "simple-git"; + import type { GitCredentialProvider, GitFactory } from "./types"; -import { createSimpleGitWithEnv } from "./simple-git"; import { getRemoteUrl } from "./utils"; export function createGitFactory(provider: GitCredentialProvider): GitFactory { return async (repoPath: string) => { const initialCredentials = await provider.getCredentials(null); - const git = createSimpleGitWithEnv({ - baseDir: repoPath, - env: initialCredentials.env, - }); + const git = simpleGit(repoPath).env(initialCredentials.env); const remoteUrl = await getRemoteUrl(git); const credentials = await provider.getCredentials(remoteUrl); - const env = { + + return git.env({ ...initialCredentials.env, ...credentials.env, GIT_OPTIONAL_LOCKS: "0", - }; - - return createSimpleGitWithEnv({ - baseDir: repoPath, - env, }); }; } diff --git a/packages/host-service/src/runtime/git/simple-git.ts b/packages/host-service/src/runtime/git/simple-git.ts deleted file mode 100644 index 3a0c5796e5b..00000000000 --- a/packages/host-service/src/runtime/git/simple-git.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { - buildSimpleGitUnsafeOptions, - type SimpleGitUnsafeOptions, -} from "@superset/shared/simple-git-unsafe"; -import simpleGit, { type SimpleGit } from "simple-git"; - -interface CreateSimpleGitWithEnvOptions { - baseDir?: string; - env?: NodeJS.ProcessEnv | Record; -} - -function copyStringEnv( - baseEnv: NodeJS.ProcessEnv | Record = process.env, -): Record { - const env: Record = {}; - - for (const [key, value] of Object.entries(baseEnv)) { - if (typeof value === "string") { - env[key] = value; - } - } - - return env; -} - -export function createSimpleGitWithEnv( - options: CreateSimpleGitWithEnvOptions = {}, -): SimpleGit { - const env = copyStringEnv(options.env ?? process.env); - const unsafe = buildSimpleGitUnsafeOptions(env); - const gitOptions: { - baseDir?: string; - unsafe?: SimpleGitUnsafeOptions; - } = {}; - - if (options.baseDir) { - gitOptions.baseDir = options.baseDir; - } - if (unsafe) { - gitOptions.unsafe = unsafe; - } - - const git = - Object.keys(gitOptions).length > 0 - ? simpleGit(gitOptions as never) - : simpleGit(); - return git.env(env); -} diff --git a/packages/host-service/src/trpc/router/project/project.ts b/packages/host-service/src/trpc/router/project/project.ts index 857e1df163f..1a9b23b7d41 100644 --- a/packages/host-service/src/trpc/router/project/project.ts +++ b/packages/host-service/src/trpc/router/project/project.ts @@ -2,8 +2,8 @@ import { existsSync, rmSync, statSync } from "node:fs"; import { basename, join, resolve } from "node:path"; import { TRPCError } from "@trpc/server"; import { eq } from "drizzle-orm"; +import simpleGit from "simple-git"; import { z } from "zod"; -import { createSimpleGitWithEnv } from "../../../runtime/git/simple-git"; import { projects, workspaces } from "../../../db/schema"; import { parseGitHubRemote } from "../../../runtime/pull-requests/utils/parse-github-remote"; import { protectedProcedure, router } from "../../index"; @@ -162,7 +162,7 @@ async function importExistingRepo( }); } - const git = createSimpleGitWithEnv({ baseDir: localPath }); + const git = simpleGit(localPath); let gitRoot: string; try { @@ -174,9 +174,7 @@ async function importExistingRepo( }); } - const remotes = await getGitHubRemotes( - createSimpleGitWithEnv({ baseDir: gitRoot }), - ); + const remotes = await getGitHubRemotes(simpleGit(gitRoot)); const matchingRemote = findMatchingRemote(remotes, expectedSlug); if (!matchingRemote) { @@ -232,7 +230,7 @@ async function cloneRepo( } try { - await createSimpleGitWithEnv().clone(repoCloneUrl, targetPath); + await simpleGit().clone(repoCloneUrl, targetPath); } catch (err) { if (existsSync(targetPath)) { rmSync(targetPath, { recursive: true, force: true }); @@ -243,9 +241,7 @@ async function cloneRepo( }); } - const remotes = await getGitHubRemotes( - createSimpleGitWithEnv({ baseDir: targetPath }), - ); + const remotes = await getGitHubRemotes(simpleGit(targetPath)); const matchingRemote = findMatchingRemote(remotes, expectedSlug); if (!matchingRemote) { diff --git a/packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts b/packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts index e14bb30cc83..0740a0210d6 100644 --- a/packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts +++ b/packages/host-service/src/trpc/router/workspace-creation/workspace-creation.ts @@ -3,9 +3,9 @@ import { dirname, join, resolve, sep } from "node:path"; import { getDeviceName, getHashedDeviceId } from "@superset/shared/device-info"; import { TRPCError } from "@trpc/server"; import { and, eq } from "drizzle-orm"; +import simpleGit from "simple-git"; import { z } from "zod"; import { projects, workspaces } from "../../../db/schema"; -import { createSimpleGitWithEnv } from "../../../runtime/git/simple-git"; import { asLocalRef, asRemoteRef, @@ -732,10 +732,7 @@ export const workspaceCreationRouter = router({ if (!existsSync(repoPath)) { mkdirSync(dirname(repoPath), { recursive: true }); - await createSimpleGitWithEnv().clone( - cloudProject.repoCloneUrl, - repoPath, - ); + await simpleGit().clone(cloudProject.repoCloneUrl, repoPath); } localProject = ctx.db @@ -1076,10 +1073,7 @@ export const workspaceCreationRouter = router({ const repoPath = join(homeDir, ".superset", "repos", input.projectId); if (!existsSync(repoPath)) { mkdirSync(dirname(repoPath), { recursive: true }); - await createSimpleGitWithEnv().clone( - cloudProject.repoCloneUrl, - repoPath, - ); + await simpleGit().clone(cloudProject.repoCloneUrl, repoPath); } localProject = ctx.db .insert(projects) diff --git a/packages/host-service/src/trpc/router/workspace/workspace.ts b/packages/host-service/src/trpc/router/workspace/workspace.ts index 49adaf9beb9..36424da116e 100644 --- a/packages/host-service/src/trpc/router/workspace/workspace.ts +++ b/packages/host-service/src/trpc/router/workspace/workspace.ts @@ -3,9 +3,9 @@ import { dirname, join } from "node:path"; import { getDeviceName, getHashedDeviceId } from "@superset/shared/device-info"; import { TRPCError } from "@trpc/server"; import { eq } from "drizzle-orm"; +import simpleGit from "simple-git"; import { z } from "zod"; import { projects, workspaces } from "../../../db/schema"; -import { createSimpleGitWithEnv } from "../../../runtime/git/simple-git"; import { protectedProcedure, router } from "../../index"; export const workspaceRouter = router({ @@ -64,7 +64,7 @@ export const workspaceRouter = router({ if (!existsSync(repoPath)) { mkdirSync(dirname(repoPath), { recursive: true }); - await createSimpleGitWithEnv().clone(cloudProject.repoCloneUrl, repoPath); + await simpleGit().clone(cloudProject.repoCloneUrl, repoPath); } const inserted = ctx.db diff --git a/packages/shared/package.json b/packages/shared/package.json index bb5c03629d2..ea93057b6a6 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -67,10 +67,6 @@ "./shell-ready-scanner": { "types": "./src/shell-ready-scanner.ts", "default": "./src/shell-ready-scanner.ts" - }, - "./simple-git-unsafe": { - "types": "./src/simple-git-unsafe.ts", - "default": "./src/simple-git-unsafe.ts" } }, "scripts": { diff --git a/packages/shared/src/simple-git-unsafe.ts b/packages/shared/src/simple-git-unsafe.ts deleted file mode 100644 index 4f4eeeef033..00000000000 --- a/packages/shared/src/simple-git-unsafe.ts +++ /dev/null @@ -1,198 +0,0 @@ -export interface SimpleGitUnsafeOptions { - allowUnsafeAlias?: true; - allowUnsafeAskPass?: true; - allowUnsafeConfigEnvCount?: true; - allowUnsafeConfigPaths?: true; - allowUnsafeCredentialHelper?: true; - allowUnsafeDiffExternal?: true; - allowUnsafeDiffTextConv?: true; - allowUnsafeEditor?: true; - allowUnsafeFilter?: true; - allowUnsafeFsMonitor?: true; - allowUnsafeGitProxy?: true; - allowUnsafeGpgProgram?: true; - allowUnsafeHooksPath?: true; - allowUnsafeMergeDriver?: true; - allowUnsafePack?: true; - allowUnsafePager?: true; - allowUnsafeProtocolOverride?: true; - allowUnsafeSshCommand?: true; - allowUnsafeTemplateDir?: true; -} - -const SIMPLE_GIT_UNSAFE_ENV_TO_OPTION = { - EDITOR: "allowUnsafeEditor", - GIT_ASKPASS: "allowUnsafeAskPass", - GIT_CONFIG: "allowUnsafeConfigPaths", - GIT_CONFIG_COUNT: "allowUnsafeConfigEnvCount", - GIT_CONFIG_GLOBAL: "allowUnsafeConfigPaths", - GIT_CONFIG_SYSTEM: "allowUnsafeConfigPaths", - GIT_EDITOR: "allowUnsafeEditor", - GIT_EXEC_PATH: "allowUnsafeConfigPaths", - GIT_EXTERNAL_DIFF: "allowUnsafeDiffExternal", - GIT_PAGER: "allowUnsafePager", - GIT_PROXY_COMMAND: "allowUnsafeGitProxy", - GIT_SEQUENCE_EDITOR: "allowUnsafeEditor", - GIT_SSH: "allowUnsafeSshCommand", - GIT_SSH_COMMAND: "allowUnsafeSshCommand", - GIT_TEMPLATE_DIR: "allowUnsafeTemplateDir", - PAGER: "allowUnsafePager", - PREFIX: "allowUnsafeConfigPaths", - SSH_ASKPASS: "allowUnsafeAskPass", -} as const satisfies Record; - -function escapeRegex(value: string): string { - return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); -} - -function createUnsafeConfigPattern(value: string): RegExp { - return new RegExp(`^\\s*${escapeRegex(value.toLowerCase())}`); -} - -function createExpandedUnsafeConfigPattern(value: string): RegExp { - const escaped = escapeRegex(value.toLowerCase()).replace( - /\\\./g, - "(?:\\\\..+)?\\\\.", - ); - return new RegExp(`^\\s*${escaped}`); -} - -const SIMPLE_GIT_UNSAFE_CONFIG_PATTERNS = [ - { - pattern: createUnsafeConfigPattern("alias"), - option: "allowUnsafeAlias", - }, - { - pattern: createUnsafeConfigPattern("core.askPass"), - option: "allowUnsafeAskPass", - }, - { - pattern: createUnsafeConfigPattern("core.editor"), - option: "allowUnsafeEditor", - }, - { - pattern: createUnsafeConfigPattern("core.fsmonitor"), - option: "allowUnsafeFsMonitor", - }, - { - pattern: createUnsafeConfigPattern("core.gitProxy"), - option: "allowUnsafeGitProxy", - }, - { - pattern: createUnsafeConfigPattern("core.hooksPath"), - option: "allowUnsafeHooksPath", - }, - { - pattern: createUnsafeConfigPattern("core.pager"), - option: "allowUnsafePager", - }, - { - pattern: createUnsafeConfigPattern("core.sshCommand"), - option: "allowUnsafeSshCommand", - }, - { - pattern: createExpandedUnsafeConfigPattern("credential.helper"), - option: "allowUnsafeCredentialHelper", - }, - { - pattern: createExpandedUnsafeConfigPattern("diff.command"), - option: "allowUnsafeDiffExternal", - }, - { - pattern: createUnsafeConfigPattern("diff.external"), - option: "allowUnsafeDiffExternal", - }, - { - pattern: createExpandedUnsafeConfigPattern("diff.textconv"), - option: "allowUnsafeDiffTextConv", - }, - { - pattern: createExpandedUnsafeConfigPattern("filter.clean"), - option: "allowUnsafeFilter", - }, - { - pattern: createExpandedUnsafeConfigPattern("filter.smudge"), - option: "allowUnsafeFilter", - }, - { - pattern: createExpandedUnsafeConfigPattern("gpg.program"), - option: "allowUnsafeGpgProgram", - }, - { - pattern: createUnsafeConfigPattern("init.templateDir"), - option: "allowUnsafeTemplateDir", - }, - { - pattern: createExpandedUnsafeConfigPattern("merge.driver"), - option: "allowUnsafeMergeDriver", - }, - { - pattern: createExpandedUnsafeConfigPattern("mergetool.path"), - option: "allowUnsafeMergeDriver", - }, - { - pattern: createExpandedUnsafeConfigPattern("mergetool.cmd"), - option: "allowUnsafeMergeDriver", - }, - { - pattern: createExpandedUnsafeConfigPattern("protocol.allow"), - option: "allowUnsafeProtocolOverride", - }, - { - pattern: createExpandedUnsafeConfigPattern("remote.receivepack"), - option: "allowUnsafePack", - }, - { - pattern: createExpandedUnsafeConfigPattern("remote.uploadpack"), - option: "allowUnsafePack", - }, - { - pattern: createUnsafeConfigPattern("sequence.editor"), - option: "allowUnsafeEditor", - }, -] as const satisfies ReadonlyArray<{ - pattern: RegExp; - option: keyof SimpleGitUnsafeOptions; -}>; - -function markUnsafeOption( - options: SimpleGitUnsafeOptions, - option: keyof SimpleGitUnsafeOptions, -): void { - options[option] = true; -} - -export function buildSimpleGitUnsafeOptions( - env: Record, -): SimpleGitUnsafeOptions | undefined { - const unsafe: SimpleGitUnsafeOptions = {}; - const upperEnv = Object.fromEntries( - Object.entries(env).map(([key, value]) => [key.toUpperCase(), value]), - ); - - for (const key of Object.keys(upperEnv)) { - const option = SIMPLE_GIT_UNSAFE_ENV_TO_OPTION[key]; - if (option) { - markUnsafeOption(unsafe, option); - } - } - - const count = Number.parseInt(upperEnv.GIT_CONFIG_COUNT ?? "", 10); - if (Number.isFinite(count) && count > 0) { - for (let index = 0; index < count; index += 1) { - const configKey = upperEnv[`GIT_CONFIG_KEY_${index}`]; - if (!configKey) { - continue; - } - - const normalizedConfigKey = configKey.trim().toLowerCase(); - for (const { pattern, option } of SIMPLE_GIT_UNSAFE_CONFIG_PATTERNS) { - if (pattern.test(normalizedConfigKey)) { - markUnsafeOption(unsafe, option); - } - } - } - } - - return Object.keys(unsafe).length > 0 ? unsafe : undefined; -}