Merge pull request #4 from sam-mfb/latest

Update to 1.1

Author: sam-mfb

README.md

@@ -40,6 +40,7 @@ Run git normally and all requests for credentials should be passed through to th
 Notes:
 
 - You can turn on more verbose debugging information by setting the environmental variable `GIT_CREDENTIAL_FORWARDER_DEBUG` to `true`
+- You can explicitly specify the path to your `git` executable by setting the environmental variable `GIT_CREDENTIAL_FORWARDER_GIT_PATH`. This shouldn't be necessary if `git` is in your `PATH`.
 
 ### Using a Dockerfile
 
@@ -55,6 +56,7 @@ RUN unzip git-credential-forwarder.zip
 RUN git config --global credential.helper '!f(){ node ~/gcf-client.js $*; }; f'
 ENV GIT_CREDENTIAL_FORWARDER_SERVER host.docker.internal:[PORT]
 ```
+
 Of course, replace `[VERSION]` and `[PORT]` with the actual version number and port number (or use Docker's `ARG` command).
 
 Note that you may need to add some other things to your git configuration. For example, to work with Azure DevOps OAuth2 authentication add:
@@ -69,7 +71,7 @@ By default the server uses a tcp server listening on `localhost`. You can tell i
 
 On the client/container side, you need to bind mount the socket into your container and then run `export GIT_CREDENTIAL_FORWARDER_SERVER="/path/to/socket"`.
 
-Note that this will not work from a Mac OS host per [this Docker issue](https://github.com/Docker/for-mac/issues/483), which is a signficant limitation.
+Note that this will not work from a Mac OS host per [this Docker issue](https://github.com/Docker/for-mac/issues/483), which is a significant limitation.
 
 ## Debugging
 
@@ -83,14 +85,10 @@ Nothing is perfectly secure, but I have tried to think through the security impl
 
 - OK, the above point isn't entirely correct. At least when the server is running in tcp mode (which is the default), in theory it is less secure than the `git credential fill` scenario because an attacker only needs to be running as _any user_ on your machine that has access to localhost, rather than as you (which they would need to run the direct `git credential` attack). I don't think that's a huge expansion of the threat model, at least for a developer on their own machine, but I'm interested in thinking of ways to make that harder.
 
+- If you want to forward credentials between machines, DO NOT edit the server script to listen on a non-localhost interface. Instead, continue to listen on the localhost interface and use some secure means for forwarding that, e.g., `ssh`
+
 - A key aspect of this utility is that the credentials will only exist in memory, i.e., the chain of `container-git <-> client-helper <-> server-helper <-> host-git` is ephemeral and will be destroyed when the processes shut down. That should limit the threat exposure to the same as when you normally run git (i.e., if someone can dump your memory they could dump your git memory as well). But it is worth thinking through places where the credentials could get accidentally stored to disk:
-  - When you have debug mode turned on. The helper tries to sanitize debug login to avoid this, but it is still a risk.
+  - When you have debug mode turned on. The helper sanitizes debug login to minimize this risk.
   - Crash dumps. I'm not aware of any way to crash this app in a way that will dump credentials, but it's a vector to keep in mind.
-  - Other? I can't think of any other way, but let me know if you can...
-
-## To Do
 
-- Test more extensively. Mostly it's only be tested on Azure Devops and GitHub using git credential manager as the host credential helper.
-- Make the install easier.
-- More tests.
-- Maybe some utilities to make setting up the Docker part easier to do as part of a Dockerfile
+NB: If you believe there is a security issue with the app, please reach out to me directly via email, which is just `sam` at my company's domain.

jest.e2e.config.js

@@ -0,0 +1,5 @@
+module.exports = {
+  testMatch: ["<rootDir>/(src|example)/**/*.(e2e).ts?(x)"],
+  preset: "ts-jest",
+  testEnvironment: "node"
+}

package.json

@@ -5,8 +5,10 @@
   "main": "dist/index.js",
   "scripts": {
     "build": "rimraf ./dist && webpack",
+    "build-mock-git": "rimraf ./src/__tests__/dist && webpack -c ./webpack.mock-git.js",
     "start": "node bin/index.js",
     "test": "jest --watch",
+    "e2e-test": "jest -c ./jest.e2e.config.js",
     "lint": "eslint ./src/",
     "fix-formatting": "prettier --write --config ./prettier.config.js ./src/",
     "check-formatting": "prettier --check --config ./prettier.config.js ./src/"

src/__tests__/all.e2e.ts

@@ -0,0 +1,101 @@
+import { StdioOptions, execSync, spawn } from "child_process"
+import { EnvKey } from "../env"
+import { GitCredentialHelperOperation } from "../git-credential-types"
+
+let serverProcess: ReturnType<typeof spawn>
+
+const setupStdio: StdioOptions = "ignore"
+const PORT_FOR_TEST_SERVER = 47472
+const TEST_PASSWORD = "myMockSecretPassword"
+
+beforeAll(() => {
+  execSync(`pnpm build`, { stdio: setupStdio })
+  execSync(`pnpm build-mock-git`, { stdio: setupStdio })
+
+  serverProcess = spawn("node", ["./dist/gcf-server.js"], {
+    detached: true,
+    env: {
+      // use the mock git app on the server side
+      [EnvKey.GIT_PATH]: "node ./src/__tests__/dist/mock-git.js",
+      [EnvKey.PORT]: PORT_FOR_TEST_SERVER.toString(),
+      // will be read by mock git and used in its mock return
+      TEST_PASSWORD: TEST_PASSWORD
+    },
+    stdio: setupStdio
+  })
+})
+
+afterAll(() => {
+  serverProcess.kill()
+})
+
+it("Returns output of host on a valid git credential operation", async () => {
+  // this test validates that the entire round trip is working: the client
+  // makes a 'get' request, that is forwarded to the server, the server
+  // passes it to `git`, gets the response, and passes it back to the
+  // client
+  return runClient({
+    operation: "get",
+    input: "username=myUser",
+    expectations: output => {
+      expect(output).toEqual(`username=myUser\npassword=${TEST_PASSWORD}`)
+    }
+  })
+})
+
+it("Returns no output on unsupported git command", async () => {
+  return runClient({
+    // not an actual git command
+    operation: "move" as GitCredentialHelperOperation,
+    input: "username=myUser",
+    expectations: output => {
+      expect(output).toEqual("")
+    }
+  })
+})
+
+/**
+ * Helper function to run e2e tests
+ *
+ * @async
+ * @param  operation - the git credential helper operation to test
+ * @param  input - the git input string to pass
+ * @param  expectations - callback that allows running assertions on the output returned to the client
+ */
+async function runClient({
+  operation,
+  input,
+  expectations
+}: {
+  operation: GitCredentialHelperOperation
+  input: string
+  expectations: (output: string) => void
+}): Promise<void> {
+  const clientProcess = spawn("node", ["./dist/gcf-client.js", operation], {
+    env: {
+      [EnvKey.SERVER]: "localhost:" + PORT_FOR_TEST_SERVER.toString()
+    }
+  })
+
+  let outputData = ""
+  clientProcess.stdout.on("data", data => {
+    outputData += data.toString()
+  })
+
+  // Promise that will resolve when the client process is complete (i.e.,
+  // when all output has been received back
+  const done = new Promise((resolve, reject) => {
+    clientProcess.on("close", () => {
+      try {
+        expectations(outputData)
+        resolve(undefined)
+      } catch (err) {
+        reject(err)
+      }
+    })
+  })
+
+  clientProcess.stdin.write(input)
+  clientProcess.stdin.end()
+  await done
+}

src/__tests__/git-credential-types.guards.test.ts

@@ -0,0 +1,93 @@
+import {
+  gitCredentialAction,
+  gitCredentialHelperOperation
+} from "../git-credential-types"
+import {
+  isGitCredentialHelperOperation,
+  isGitCredentialAction,
+  isGitCredentialInputOutput
+} from "../git-credential-types.guards"
+
+describe(isGitCredentialHelperOperation.name, () => {
+  it("returns true for valid operations", () => {
+    expect(
+      isGitCredentialHelperOperation(gitCredentialHelperOperation.GET)
+    ).toBeTruthy()
+    expect(
+      isGitCredentialHelperOperation(gitCredentialHelperOperation.STORE)
+    ).toBeTruthy()
+    expect(
+      isGitCredentialHelperOperation(gitCredentialHelperOperation.ERASE)
+    ).toBeTruthy()
+  })
+
+  it("returns false for an invalid string", () => {
+    expect(isGitCredentialHelperOperation("invalid_operation")).toBeFalsy()
+  })
+
+  it("returns false for an empty string", () => {
+    expect(isGitCredentialHelperOperation("")).toBeFalsy()
+  })
+})
+
+describe(isGitCredentialAction.name, () => {
+  it("returns true for valid actions", () => {
+    expect(isGitCredentialAction(gitCredentialAction.FILL)).toBeTruthy()
+    expect(isGitCredentialAction(gitCredentialAction.APPROVE)).toBeTruthy()
+    expect(isGitCredentialAction(gitCredentialAction.REJECT)).toBeTruthy()
+  })
+
+  it("returns false for an invalid string", () => {
+    expect(isGitCredentialAction("invalid_action")).toBeFalsy()
+  })
+
+  it("returns false for an empty string", () => {
+    expect(isGitCredentialAction("")).toBeFalsy()
+  })
+})
+
+describe(isGitCredentialInputOutput.name, () => {
+  it("returns true for a valid GitCredentialInputOutput object", () => {
+    const validInput = {
+      protocol: "https",
+      host: "example.com",
+      path: "path/to/resource",
+      username: "user",
+      password: "password",
+      password_expiry_utc: 1609459200,
+      oauth_refresh_token: "token"
+    }
+    expect(isGitCredentialInputOutput(validInput)).toBeTruthy()
+  })
+
+  it("returns false for null", () => {
+    expect(isGitCredentialInputOutput(null)).toBeFalsy()
+  })
+
+  it("returns false for undefined", () => {
+    expect(isGitCredentialInputOutput(undefined)).toBeFalsy()
+  })
+
+  it("returns false for non-object types", () => {
+    expect(isGitCredentialInputOutput(123)).toBeFalsy()
+    expect(isGitCredentialInputOutput("string")).toBeFalsy()
+    expect(isGitCredentialInputOutput([])).toBeFalsy()
+  })
+
+  it("returns true for a minimal valid GitCredentialInputOutput object", () => {
+    const minimalValidInput = {
+      protocol: "https",
+      host: "example.com"
+    }
+    expect(isGitCredentialInputOutput(minimalValidInput)).toBeTruthy()
+  })
+
+  it("returns false for objects with invalid types for properties", () => {
+    const invalidInput = {
+      protocol: 123, // should be string
+      host: null // should be string
+    }
+    expect(isGitCredentialInputOutput(invalidInput)).toBeFalsy()
+  })
+})
+

src/__tests__/gitcredential-io.test.ts

@@ -0,0 +1,93 @@
+import { gitCredentialIoApi } from "../gitcredential-io"
+import { Result } from "../result"
+
+describe(`${gitCredentialIoApi.deserialize.name}`, () => {
+  it("returns a successful result with correct fields from a well-formatted string", () => {
+    const input =
+      "protocol=https\nhost=example.com\nusername=user\npassword=pass"
+    const expectedOutput = {
+      protocol: "https",
+      host: "example.com",
+      username: "user",
+      password: "pass"
+    }
+
+    const result = gitCredentialIoApi.deserialize(input)
+
+    expect(result).toEqual(Result.success(expectedOutput))
+  })
+
+  it("ignores malformed input", () => {
+    const malformedInput = "protocol=https\nusernameuser\npassword=pass"
+    const expectedObject = {
+      protocol: "https",
+      password: "pass"
+    }
+
+    const result = gitCredentialIoApi.deserialize(malformedInput)
+
+    expect(result).toEqual(Result.success(expectedObject))
+  })
+
+  it("ignores extra fields not defined in the GitCredentialInputOutput type", () => {
+    const inputWithExtra =
+      "protocol=https\nhost=example.com\nextra=field\nusername=user"
+    const expectedOutput = {
+      protocol: "https",
+      host: "example.com",
+      username: "user"
+    }
+
+    const result = gitCredentialIoApi.deserialize(inputWithExtra)
+
+    expect(result).toEqual(Result.success(expectedOutput))
+  })
+
+  it("empty input string results in empty object", () => {
+    const emptyInput = ""
+    const expectedOutput = {}
+
+    const result = gitCredentialIoApi.deserialize(emptyInput)
+
+    expect(result).toEqual(Result.success(expectedOutput))
+  })
+})
+
+describe(`${gitCredentialIoApi.serialize.name}`, () => {
+  it("converts a GitCredentialInputOutput object to a correctly formatted string", () => {
+    const input = {
+      protocol: "https",
+      host: "example.com",
+      username: "user",
+      password: "pass"
+    }
+    const expectedString =
+      "protocol=https\nhost=example.com\nusername=user\npassword=pass"
+
+    const result = gitCredentialIoApi.serialize(input)
+
+    expect(result).toBe(expectedString)
+  })
+
+  it("omits undefined fields from the output string", () => {
+    const inputWithUndefined = {
+      protocol: "https",
+      host: "example.com",
+      username: undefined
+    }
+    const expectedString = "protocol=https\nhost=example.com"
+
+    const result = gitCredentialIoApi.serialize(inputWithUndefined)
+
+    expect(result).toBe(expectedString)
+  })
+
+  it("handles empty GitCredentialInputOutput object without errors", () => {
+    const emptyInput = {}
+    const expectedString = ""
+
+    const result = gitCredentialIoApi.serialize(emptyInput)
+
+    expect(result).toBe(expectedString)
+  })
+})