diff --git a/.changeset/windows-git-ssh-sync-env.md b/.changeset/windows-git-ssh-sync-env.md new file mode 100644 index 000000000..2cc3b081c --- /dev/null +++ b/.changeset/windows-git-ssh-sync-env.md @@ -0,0 +1,7 @@ +--- +"@inkeep/open-knowledge": patch +--- + +Fix Git auto-sync when server-spawned Git needs the user's home directory, SSH agent, or credential-helper environment to reach a remote. This most visibly affected Windows repositories using SSH remotes, where `ok sync` and editor sync could fail with "Could not read from remote repository" while the same `git fetch` or `git push` worked in a terminal. + +Because preserving the home directory also lets server-spawned Git read the user's global config, OK now pins `commit.gpgsign=false` and `core.autocrlf=false` for its own Git commands only (via `-c`, leaving the user's own Git untouched): the first prevents the unattended sync commit from aborting when a global signing config can't prompt for a passphrase, and the second keeps line-ending conversion from churning content against OK's byte-exact round-trip. diff --git a/packages/server/src/git-handle.test.ts b/packages/server/src/git-handle.test.ts index 4ef1c5bb0..5ae47569e 100644 --- a/packages/server/src/git-handle.test.ts +++ b/packages/server/src/git-handle.test.ts @@ -4,22 +4,28 @@ import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { setTimeout as wait } from 'node:timers/promises'; -import { buildGitEnv, createGitInstance } from './git-handle.ts'; +import { applyGitEnv, buildGitEnv, createGitInstance, type GitHandle } from './git-handle.ts'; import { withParentLock } from './git-mutex.ts'; -describe('buildGitEnv', () => { - function withEnv(key: string, value: string | undefined, fn: () => void): void { - const saved = process.env[key]; - try { +function withEnvEntries(entries: Record, fn: () => void): void { + const saved = new Map(); + for (const key of Object.keys(entries)) { + saved.set(key, process.env[key]); + const value = entries[key]; + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + try { + fn(); + } finally { + for (const [key, value] of saved) { if (value === undefined) delete process.env[key]; else process.env[key] = value; - fn(); - } finally { - if (saved === undefined) delete process.env[key]; - else process.env[key] = saved; } } +} +describe('buildGitEnv', () => { test('forces LANG/LC_ALL=C for locale-stable stderr', () => { const env = buildGitEnv(); expect(env.LANG).toBe('C'); @@ -31,19 +37,49 @@ describe('buildGitEnv', () => { }); test('preserves PATH so a bare-command credential helper resolves', () => { - withEnv('PATH', '/custom/bin:/usr/bin', () => { + withEnvEntries({ PATH: '/custom/bin:/usr/bin' }, () => { expect(buildGitEnv().PATH).toBe('/custom/bin:/usr/bin'); }); }); + test('preserves user and SSH auth environment for Git transports', () => { + withEnvEntries( + { + HOME: '/Users/alice', + USERPROFILE: 'C:\\Users\\alice', + HOMEDRIVE: 'C:', + HOMEPATH: '\\Users\\alice', + ProgramData: 'C:\\ProgramData', + ALLUSERSPROFILE: 'C:\\ProgramData', + SSH_AUTH_SOCK: '/tmp/ssh-agent.sock', + }, + () => { + const env = buildGitEnv(); + expect(env.HOME).toBe('/Users/alice'); + expect(env.USERPROFILE).toBe('C:\\Users\\alice'); + expect(env.HOMEDRIVE).toBe('C:'); + expect(env.HOMEPATH).toBe('\\Users\\alice'); + expect(env.ProgramData).toBe('C:\\ProgramData'); + expect(env.ALLUSERSPROFILE).toBe('C:\\ProgramData'); + expect(env.SSH_AUTH_SOCK).toBe('/tmp/ssh-agent.sock'); + }, + ); + }); + + test('does not pass through GIT_SSH_COMMAND without explicit simple-git opt-in', () => { + withEnvEntries({ GIT_SSH_COMMAND: 'ssh -vv' }, () => { + expect('GIT_SSH_COMMAND' in buildGitEnv()).toBe(false); + }); + }); + test('preserves ELECTRON_RUN_AS_NODE so the packaged credential helper runs as Node', () => { - withEnv('ELECTRON_RUN_AS_NODE', '1', () => { + withEnvEntries({ ELECTRON_RUN_AS_NODE: '1' }, () => { expect(buildGitEnv().ELECTRON_RUN_AS_NODE).toBe('1'); }); }); test('omits ELECTRON_RUN_AS_NODE on a non-Electron host (var unset)', () => { - withEnv('ELECTRON_RUN_AS_NODE', undefined, () => { + withEnvEntries({ ELECTRON_RUN_AS_NODE: undefined }, () => { expect('ELECTRON_RUN_AS_NODE' in buildGitEnv()).toBe(false); }); }); @@ -62,6 +98,11 @@ describe('buildGitEnv', () => { describe('createGitInstance (credential.helper config)', () => { let tmpDir: string; + function readEnv(handle: GitHandle): Record { + // biome-ignore lint/suspicious/noExplicitAny: probing internal simple-git executor for spawn-env assertion + return ((handle.git as any)._executor?.env ?? {}) as Record; + } + beforeEach(() => { tmpDir = mkdtempSync(join(tmpdir(), 'ok-git-handle-test-')); execSync('git init -q', { cwd: tmpDir }); @@ -78,6 +119,31 @@ describe('createGitInstance (credential.helper config)', () => { const version = await handle.git.raw(['--version']); expect(version).toContain('git version'); }); + + test('merges author overrides without dropping git auth env', () => { + withEnvEntries({ USERPROFILE: 'C:\\Users\\alice' }, () => { + const handle = createGitInstance(tmpDir, { gitIndexFile: '.git/custom-index' }); + applyGitEnv(handle, { + GIT_AUTHOR_NAME: 'Alice', + GIT_AUTHOR_EMAIL: 'alice@example.com', + }); + + const env = readEnv(handle); + expect(env.USERPROFILE).toBe('C:\\Users\\alice'); + expect(env.GIT_INDEX_FILE).toBe(join(tmpDir, '.git/custom-index')); + expect(env.GIT_AUTHOR_NAME).toBe('Alice'); + expect(env.GIT_AUTHOR_EMAIL).toBe('alice@example.com'); + }); + }); + + test('pins commit.gpgsign and core.autocrlf off, overriding repo config', async () => { + execSync('git config commit.gpgsign true', { cwd: tmpDir }); + execSync('git config core.autocrlf true', { cwd: tmpDir }); + + const handle = createGitInstance(tmpDir); + expect((await handle.git.raw(['config', '--get', 'commit.gpgsign'])).trim()).toBe('false'); + expect((await handle.git.raw(['config', '--get', 'core.autocrlf'])).trim()).toBe('false'); + }); }); describe('withParentLock', () => { diff --git a/packages/server/src/git-handle.ts b/packages/server/src/git-handle.ts index be8409285..98c5d2473 100644 --- a/packages/server/src/git-handle.ts +++ b/packages/server/src/git-handle.ts @@ -18,6 +18,7 @@ export interface GitHandle { git: SimpleGit; projectDir: string; credentialArgs: string[]; + env: Record; } type CredentialHelperUnsafeGitOptions = SimpleGitOptions & { @@ -26,13 +27,37 @@ type CredentialHelperUnsafeGitOptions = SimpleGitOptions & { }; }; +const GIT_AUTH_ENV_KEYS = [ + 'HOME', + 'USERPROFILE', + 'HOMEDRIVE', + 'HOMEPATH', + 'APPDATA', + 'LOCALAPPDATA', + 'ProgramData', + 'ALLUSERSPROFILE', + 'SystemRoot', + 'WINDIR', + 'windir', + 'ComSpec', + 'TEMP', + 'TMP', + 'USERNAME', + 'USERDOMAIN', + 'PATHEXT', + 'SSH_AUTH_SOCK', + 'ELECTRON_RUN_AS_NODE', +] as const; + export function buildGitEnv(ghToken?: RelayGhToken): Record { const env: Record = { LANG: 'C', LC_ALL: 'C', GIT_TERMINAL_PROMPT: '0' }; - if (process.env.PATH !== undefined) { - env.PATH = process.env.PATH; + const path = process.env.PATH ?? process.env.Path; + if (path !== undefined) { + env.PATH = path; } - if (process.env.ELECTRON_RUN_AS_NODE !== undefined) { - env.ELECTRON_RUN_AS_NODE = process.env.ELECTRON_RUN_AS_NODE; + for (const key of GIT_AUTH_ENV_KEYS) { + const value = process.env[key]; + if (value !== undefined) env[key] = value; } if (ghToken) { env.OK_GH_TOKEN = ghToken.token; @@ -41,6 +66,17 @@ export function buildGitEnv(ghToken?: RelayGhToken): Record { return env; } +export function applyGitEnv( + handle: GitHandle, + overrides: Record, +): SimpleGit { + const env = { ...handle.env }; + for (const [key, value] of Object.entries(overrides)) { + if (value !== undefined) env[key] = value; + } + return handle.git.env(env); +} + export function createGitInstance(projectDir: string, options: GitHandleOptions = {}): GitHandle { const { credentialArgs = [], gitIndexFile, ghToken } = options; @@ -49,7 +85,11 @@ export function createGitInstance(projectDir: string, options: GitHandleOptions env.GIT_INDEX_FILE = resolve(projectDir, gitIndexFile); } - const gitConfig = credentialArgs.length >= 2 ? [credentialArgs[1]] : []; + const gitConfig = [ + 'commit.gpgsign=false', + 'core.autocrlf=false', + ...(credentialArgs.length >= 2 ? [credentialArgs[1]] : []), + ]; const gitOptions: Partial = { baseDir: projectDir, @@ -59,5 +99,5 @@ export function createGitInstance(projectDir: string, options: GitHandleOptions const git = simpleGit(gitOptions as Partial).env(env as Record); - return { git, projectDir, credentialArgs }; + return { git, projectDir, credentialArgs, env: env as Record }; } diff --git a/packages/server/src/sync-engine.ts b/packages/server/src/sync-engine.ts index 447b31042..485567acd 100644 --- a/packages/server/src/sync-engine.ts +++ b/packages/server/src/sync-engine.ts @@ -21,7 +21,7 @@ import { type UserFacingErrorCode, } from './error-classification.ts'; import { createGhTokenSource, type GhTokenSource } from './gh-token-source.ts'; -import { createGitInstance, type GitHandle, withParentLock } from './git-handle.ts'; +import { applyGitEnv, createGitInstance, type GitHandle, withParentLock } from './git-handle.ts'; import { resolveGitIdentity } from './git-identity.ts'; import { type CheckPushPermissionOptions, @@ -1045,7 +1045,7 @@ export class SyncEngine { const authorName = identity?.name ?? 'OpenKnowledge'; const authorEmail = identity?.email ?? 'sync@open-knowledge.local'; - handle.git.env({ + applyGitEnv(handle, { GIT_AUTHOR_NAME: authorName, GIT_AUTHOR_EMAIL: authorEmail, GIT_COMMITTER_NAME: authorName, @@ -1198,7 +1198,7 @@ export class SyncEngine { const identity = await resolveGitIdentity(this.projectDir); const authorName = identity?.name ?? 'OpenKnowledge'; const authorEmail = identity?.email ?? 'sync@open-knowledge.local'; - isoHandle.git.env({ + applyGitEnv(isoHandle, { GIT_AUTHOR_NAME: authorName, GIT_AUTHOR_EMAIL: authorEmail, GIT_COMMITTER_NAME: authorName,