diff --git a/.changeset/windows-git-ssh-sync-env.md b/.changeset/windows-git-ssh-sync-env.md new file mode 100644 index 00000000..bb72e2c8 --- /dev/null +++ b/.changeset/windows-git-ssh-sync-env.md @@ -0,0 +1,5 @@ +--- +"@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. diff --git a/packages/server/src/git-handle.test.ts b/packages/server/src/git-handle.test.ts index 4ef1c5bb..20d40cad 100644 --- a/packages/server/src/git-handle.test.ts +++ b/packages/server/src/git-handle.test.ts @@ -4,7 +4,7 @@ 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', () => { @@ -20,6 +20,24 @@ describe('buildGitEnv', () => { } } + 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; + } + } + } + test('forces LANG/LC_ALL=C for locale-stable stderr', () => { const env = buildGitEnv(); expect(env.LANG).toBe('C'); @@ -36,6 +54,36 @@ describe('buildGitEnv', () => { }); }); + 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', () => { + withEnv('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', () => { expect(buildGitEnv().ELECTRON_RUN_AS_NODE).toBe('1'); @@ -62,6 +110,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 +131,27 @@ 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', () => { + const savedUserProfile = process.env.USERPROFILE; + process.env.USERPROFILE = 'C:\\Users\\alice'; + try { + 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'); + } finally { + if (savedUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = savedUserProfile; + } + }); }); describe('withParentLock', () => { diff --git a/packages/server/src/git-handle.ts b/packages/server/src/git-handle.ts index be840928..402c07b9 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; @@ -59,5 +95,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 447b3104..485567ac 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,