diff --git a/README.md b/README.md index f01f067..e084154 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ The equivalent Bash shell code looks like this: ```sh # environment variable - GIT_ASKPASS="C:/some/path/to/desktop-trampoline.exe" \ + GIT_ASKPASS="C:/some/path/to/desktop-askpass-trampoline.exe" \ # ensure Git doesn't block the process waiting for the user to provide input GIT_TERMINAL_PROMPT=0 \ git \ diff --git a/binding.gyp b/binding.gyp index 47d762f..94e7cb4 100644 --- a/binding.gyp +++ b/binding.gyp @@ -1,15 +1,8 @@ { - 'targets': [ - { - 'target_name': 'desktop-trampoline', + 'target_defaults': { 'defines': [ "NAPI_VERSION=<(napi_build_version)", ], - 'type': 'executable', - 'sources': [ - 'src/desktop-trampoline.c', - 'src/socket.c' - ], 'include_dirs': [ '<!(node -p "require(\'node-addon-api\').include_dir")', 'include' @@ -42,9 +35,20 @@ 'msvs_settings': { 'VCCLCompilerTool': { 'ExceptionHandling': 1 }, }, + 'conditions': [ + ['OS=="win"', { 'defines': [ 'WINDOWS' ] }] + ] + }, + 'targets': [ + { + 'target_name': 'desktop-askpass-trampoline', + 'type': 'executable', + 'sources': [ + 'src/desktop-trampoline.c', + 'src/socket.c' + ], 'conditions': [ ['OS=="win"', { - 'defines': [ 'WINDOWS' ], 'link_settings': { 'libraries': [ 'Ws2_32.lib' ] } @@ -52,52 +56,29 @@ ] }, { - 'target_name': 'ssh-wrapper', + 'target_name': 'desktop-credential-helper-trampoline', + 'type': 'executable', 'defines': [ - "NAPI_VERSION=<(napi_build_version)", + 'CREDENTIAL_HELPER' ], - 'type': 'executable', 'sources': [ - 'src/ssh-wrapper.c' - ], - 'include_dirs': [ - '<!(node -p "require(\'node-addon-api\').include_dir")', - 'include' - ], - 'xcode_settings': { - 'OTHER_CFLAGS': [ - '-Wall', - '-Werror', - '-Werror=format-security', - '-fPIC', - '-D_FORTIFY_SOURCE=1', - '-fstack-protector-strong' - ] - }, - 'cflags!': [ - '-Wall', - '-Werror', - '-fPIC', - '-pie', - '-D_FORTIFY_SOURCE=1', - '-fstack-protector-strong', - '-Werror=format-security', - '-fno-exceptions' - ], - 'cflags_cc!': [ '-fno-exceptions' ], - 'ldflags!': [ - '-z relro', - '-z now' + 'src/desktop-trampoline.c', + 'src/socket.c' ], - 'msvs_settings': { - 'VCCLCompilerTool': { 'ExceptionHandling': 1 }, - }, 'conditions': [ - # For now only build it for macOS, since it's not needed on Windows ['OS=="win"', { - 'defines': [ 'WINDOWS' ], + 'link_settings': { + 'libraries': [ 'Ws2_32.lib' ] + } }] ] }, + { + 'target_name': 'ssh-wrapper', + 'type': 'executable', + 'sources': [ + 'src/ssh-wrapper.c' + ], + }, ], } diff --git a/index.d.ts b/index.d.ts index e68c0de..6ce6768 100644 --- a/index.d.ts +++ b/index.d.ts @@ -1,5 +1,8 @@ -export function getDesktopTrampolinePath(): string -export function getDesktopTrampolineFilename(): string +export function getDesktopAskpassTrampolinePath(): string +export function getDesktopAskpassTrampolineFilename(): string + +export function getDesktopCredentialHelperTrampolinePath(): string +export function getDesktopCredentialHelperTrampolineFilename(): string export function getSSHWrapperPath(): string export function getSSHWrapperFilename(): string diff --git a/index.js b/index.js index 0891bae..be164b0 100644 --- a/index.js +++ b/index.js @@ -1,18 +1,33 @@ const Path = require('path') -function getDesktopTrampolinePath() { +function getDesktopAskpassTrampolinePath() { return Path.join( __dirname, 'build', 'Release', - getDesktopTrampolineFilename() + getDesktopAskpassTrampolineFilename() ) } -function getDesktopTrampolineFilename() { +function getDesktopAskpassTrampolineFilename() { return process.platform === 'win32' - ? 'desktop-trampoline.exe' - : 'desktop-trampoline' + ? 'desktop-askpass-trampoline.exe' + : 'desktop-askpass-trampoline' +} + +function getDesktopCredentialHelperTrampolinePath() { + return Path.join( + __dirname, + 'build', + 'Release', + getDesktopCredentialHelperTrampolineFilename() + ) +} + +function getDesktopCredentialHelperTrampolineFilename() { + return process.platform === 'win32' + ? 'desktop-credential-helper-trampoline.exe' + : 'desktop-credential-helper-trampoline' } function getSSHWrapperPath() { @@ -24,8 +39,10 @@ function getSSHWrapperFilename() { } module.exports = { - getDesktopTrampolinePath, - getDesktopTrampolineFilename, + getDesktopAskpassTrampolinePath, + getDesktopAskpassTrampolineFilename, + getDesktopCredentialHelperTrampolinePath, + getDesktopCredentialHelperTrampolineFilename, getSSHWrapperPath, getSSHWrapperFilename, } diff --git a/src/desktop-trampoline.c b/src/desktop-trampoline.c index 3e676fb..0c59c00 100644 --- a/src/desktop-trampoline.c +++ b/src/desktop-trampoline.c @@ -9,6 +9,13 @@ #define BUFFER_LENGTH 4096 #define MAXIMUM_NUMBER_LENGTH 33 +#ifdef CREDENTIAL_HELPER + #define DESKTOP_TRAMPOLINE_IDENTIFIER "CREDENTIALHELPER" +#else + #define DESKTOP_TRAMPOLINE_IDENTIFIER "ASKPASS" +#endif + + #define WRITE_STRING_OR_EXIT(dataName, dataString) \ if (writeSocket(socket, dataString, strlen(dataString) + 1) != 0) { \ printSocketError("ERROR: Couldn't send " dataName); \ @@ -17,9 +24,8 @@ if (writeSocket(socket, dataString, strlen(dataString) + 1) != 0) { \ // This is a list of valid environment variables that GitHub Desktop might // send or expect to receive. -#define NUMBER_OF_VALID_ENV_VARS 2 +#define NUMBER_OF_VALID_ENV_VARS 1 static const char *sValidEnvVars[NUMBER_OF_VALID_ENV_VARS] = { - "DESKTOP_TRAMPOLINE_IDENTIFIER", "DESKTOP_TRAMPOLINE_TOKEN", }; @@ -81,8 +87,9 @@ int runTrampolineClient(SOCKET *outSocket, int argc, char **argv, char **envp) { } // Get the number of environment variables - char *validEnvVars[NUMBER_OF_VALID_ENV_VARS]; - int envc = 0; + char *validEnvVars[NUMBER_OF_VALID_ENV_VARS + 1]; + validEnvVars[0] = "DESKTOP_TRAMPOLINE_IDENTIFIER=" DESKTOP_TRAMPOLINE_IDENTIFIER; + int envc = 1; for (char **env = envp; *env != 0; env++) { if (isValidEnvVar(*env)) { validEnvVars[envc] = *env; @@ -100,7 +107,15 @@ int runTrampolineClient(SOCKET *outSocket, int argc, char **argv, char **envp) { WRITE_STRING_OR_EXIT("environment variable", validEnvVars[idx]); } - // TODO: send stdin stuff? + char stdinBuffer[BUFFER_LENGTH + 1]; + int stdinBytes = 0; + + #ifdef CREDENTIAL_HELPER + stdinBytes = fread(stdinBuffer, sizeof(char), BUFFER_LENGTH, stdin); + #endif + + stdinBuffer[stdinBytes] = '\0'; + WRITE_STRING_OR_EXIT("stdin", stdinBuffer); char buffer[BUFFER_LENGTH + 1]; size_t totalBytesRead = 0; diff --git a/test/desktop-trampoline.test.js b/test/desktop-trampoline.test.js index 4535fda..db362cd 100644 --- a/test/desktop-trampoline.test.js +++ b/test/desktop-trampoline.test.js @@ -2,62 +2,152 @@ const { stat, access } = require('fs').promises const { constants } = require('fs') const { execFile } = require('child_process') const { promisify } = require('util') -const { getDesktopTrampolinePath } = require('../index') +const { + getDesktopAskpassTrampolinePath, + getDesktopCredentialHelperTrampolinePath, +} = require('../index') const split2 = require('split2') const { createServer } = require('net') -const trampolinePath = getDesktopTrampolinePath() +const askPassTrampolinePath = getDesktopAskpassTrampolinePath() +const helperTrampolinePath = getDesktopCredentialHelperTrampolinePath() const run = promisify(execFile) describe('desktop-trampoline', () => { it('exists and is a regular file', async () => - expect((await stat(trampolinePath)).isFile()).toBe(true)) + expect((await stat(askPassTrampolinePath)).isFile()).toBe(true)) it('can be executed by current process', () => - access(trampolinePath, constants.X_OK)) + access(askPassTrampolinePath, constants.X_OK)) it('fails when required environment variables are missing', () => - expect(run(trampolinePath, ['Username'])).rejects.toThrow()) + expect(run(askPassTrampolinePath, ['Username'])).rejects.toThrow()) - it('forwards arguments and valid environment variables correctly', async () => { + const captureSession = () => { const output = [] + let resolveOutput = null + + const outputPromise = new Promise(resolve => { + resolveOutput = resolve + }) + const server = createServer(socket => { + let timeoutId = null socket.pipe(split2(/\0/)).on('data', data => { output.push(data.toString('utf8')) - }) - // Don't send anything and just close the socket after the trampoline is - // done forwarding data. - socket.end() + // Hack: consider the session finished after 100ms of inactivity. + // In a real-world scenario, you'd have to parse the data to know when + // the session is finished. + if (timeoutId !== null) { + clearTimeout(timeoutId) + timeoutId = null + } + timeoutId = setTimeout(() => { + resolveOutput(output) + socket.end() + server.close() + }, 100) + }) }) - server.unref() - - const startTrampolineServer = async () => { - return new Promise((resolve, reject) => { - server.on('error', e => reject(e)) - server.listen(0, '127.0.0.1', () => { - resolve(server.address().port) - }) + + const serverPortPromise = new Promise((resolve, reject) => { + server.on('error', e => reject(e)) + server.listen(0, '127.0.0.1', () => { + resolve(server.address().port) }) - } + }) + + return [serverPortPromise, outputPromise] + } + + it('forwards arguments and valid environment variables correctly', async () => { + const [portPromise, outputPromise] = captureSession() + const port = await portPromise - const port = await startTrampolineServer() const env = { - DESKTOP_TRAMPOLINE_IDENTIFIER: '123456', + DESKTOP_TRAMPOLINE_TOKEN: '123456', DESKTOP_PORT: port, INVALID_VARIABLE: 'foo bar', } const opts = { env } - await run(trampolinePath, ['baz'], opts) + await run(askPassTrampolinePath, ['baz'], opts) + const output = await outputPromise const outputArguments = output.slice(1, 2) expect(outputArguments).toStrictEqual(['baz']) // output[2] is the number of env variables - const outputEnv = output.slice(3) - expect(outputEnv).toHaveLength(1) - expect(outputEnv).toContain('DESKTOP_TRAMPOLINE_IDENTIFIER=123456') + const envc = parseInt(output[2]) + const outputEnv = output.slice(3, 3 + envc) + expect(outputEnv).toHaveLength(2) + expect(outputEnv).toContain('DESKTOP_TRAMPOLINE_TOKEN=123456') + expect(outputEnv).toContain('DESKTOP_TRAMPOLINE_IDENTIFIER=ASKPASS') + }) + + it('forwards stdin when running in credential-helper mode', async () => { + const [portPromise, outputPromise] = captureSession() + const port = await portPromise + + const cp = run(helperTrampolinePath, ['get'], { + env: { DESKTOP_PORT: port }, + }) + cp.child.stdin.end('oh hai\n') + + await cp + + const output = await outputPromise + expect(output.at(-1)).toBe('oh hai\n') + }) + + it("doesn't forward stdin when running in askpass mode", async () => { + const [portPromise, outputPromise] = captureSession() + const port = await portPromise + + const cp = run(askPassTrampolinePath, ['get'], { + env: { DESKTOP_PORT: port }, + }) + cp.child.stdin.end('oh hai\n') + + await cp + + const output = await outputPromise + expect(output.at(-1)).toBe('') + }) + + it('askpass handler ignores the DESKTOP_TRAMPOLINE_IDENTIFIER env var', async () => { + const [portPromise, outputPromise] = captureSession() + const port = await portPromise + + const cp = run(askPassTrampolinePath, ['get'], { + env: { DESKTOP_PORT: port, DESKTOP_TRAMPOLINE_IDENTIFIER: 'foo' }, + }) + cp.child.stdin.end('oh hai\n') + + await cp + + const output = await outputPromise + const envc = parseInt(output[2]) + const outputEnv = output.slice(3, 3 + envc) + expect(outputEnv).toContain('DESKTOP_TRAMPOLINE_IDENTIFIER=ASKPASS') + }) + + it('credential handler ignores the DESKTOP_TRAMPOLINE_IDENTIFIER env var', async () => { + const [portPromise, outputPromise] = captureSession() + const port = await portPromise + + const cp = run(helperTrampolinePath, ['get'], { + env: { DESKTOP_PORT: port, DESKTOP_TRAMPOLINE_IDENTIFIER: 'foo' }, + }) + cp.child.stdin.end('oh hai\n') + + await cp - server.close() + const output = await outputPromise + const envc = parseInt(output[2]) + const outputEnv = output.slice(3, 3 + envc) + expect(outputEnv).toContain( + 'DESKTOP_TRAMPOLINE_IDENTIFIER=CREDENTIALHELPER' + ) }) })