Repository: finos/git-proxy

Issue:
optimize pullRemote for large repos
**Is your feature request related to a problem? Please describe.**
On particularly large repositories as well as when GitProxy is deployed in a production-like environment (constrained or at least defined upper limits on CPU/memory), `pullRemote` step will sometimes cause the `git` client to time out and return a server error if it takes too long to complete.

We should consider whether we need to pull down the whole history every time there is a new push. Using `depth: 1, singleBranch: true` on the git clone call seems to reduce the amount of time (in our particular instance) that the push takes and allows it to proceed through proxy processing normally.

**Describe the solution you'd like**
- Tweak the isomorphic-git call during cloning to the minimum required to produce a working diff. 
- Pull down selective history such as default branches only?
- Make it configurable?

**Describe alternatives you've considered**
Increasing the timeout for the server (we have it deployed behind a load balancer) helps marginally but the full clone can be very expensive with large repos.

**Additional context**

This is likely a problem that affects outdated branches more than anything. When a commit drifts (is left stale for days, weeks or months), the combination of the long clone time + large diff usually results in server errors. 


Discussion:
@kriswest: +1 - We experienced teething trouble with the clone time as well and had to increase timeouts in our loadbalancer for moderate to large repos such as FDC3. In addition to causing timeouts, long clone times affect usability, particularly for new users.@jescalada: @coopernetes @kriswest I've done some testing here, and it seems the `pullRemote` action (and therefore `isomorphic-git`'s `git.clone` call never gets executed when cloning a repo for the first time or pulling a previously cloned repo. Is this meant to execute only during pushes?

Changing those parameters on the `git.clone` call doesn't seem to change the time elapsed for cloning/pulling on my end.

Is there a step-by-step sample I can follow to get `pullRemote` to execute?

Thanks!@kriswest: pullRemote is only in the push chain (in order to facilitate getDiff later), so it does indeed only run on a push. @jescalada if you want to experiment with this add some time measurement either on pullRemote or executeChain, then run a push.

Could add the `noTags` flag as well I imagine (but presumably that mostly affects the fetch?). 

@coopernetes if you've tried this out have you run into any diff issues with `depth:1`? If its not causing trouble it would seem a quick change to make.@jescalada: Some more experimenting with pushes this time, with a small commit and short commit history (in progress):

## Before fix
### Total push time, before approval `5.338s`

<img width="1490" height="736" alt="Image" src="https://github.com/user-attachments/assets/20585ee4-fc47-4d67-b026-d7d23aa99f6f" />

### pullRemote time, before approval `1.628s`

<img width="1104" height="165" alt="Image" src="https://github.com/user-attachments/assets/5e9296a4-73b8-483d-949b-1c20743a47d5" />

### Total push time, after approval `9.575s`

<img width="841" height="352" alt="Image" src="https://github.com/user-attachments/assets/555bc698-0b95-4537-ad6d-22f79b31b756" />

### pullRemote times, after approval `2.485s` + `1.707s`

<img width="1039" height="159" alt="Image" src="https://github.com/user-attachments/assets/074f3d54-7fdf-48ee-ad0b-fcf4077b0d04" />

<img width="1039" height="159" alt="Image" src="https://github.com/user-attachments/assets/d43d7b55-f5de-4942-8a97-5dcb07b9d5ad" />
@jescalada: ## After fix
### Total push time, before approval `2.571s`

<img width="1485" height="732" alt="Image" src="https://github.com/user-attachments/assets/5d14db75-e298-4486-9516-88786f08ce3f" />

### `pullRemote` time, before approval `1.053s`

<img width="1046" height="162" alt="Image" src="https://github.com/user-attachments/assets/b132f314-a207-4887-a842-59525687c073" />

### Total, push time, after approval `4.883s`

<img width="886" height="356" alt="Image" src="https://github.com/user-attachments/assets/b8428ef0-e7cc-483c-af2d-1d9658dc9388" />

### `pullRemote` time, after approval `1.093s` + `0.810s`

<img width="1044" height="150" alt="Image" src="https://github.com/user-attachments/assets/64cf3fba-e996-4229-9b33-009a07b8a809" />

<img width="1044" height="150" alt="Image" src="https://github.com/user-attachments/assets/10dc623d-7026-4791-b011-2188d97266a1" />@jescalada: Adding the `singleBranch: true` and `depth: 1` lines seems to halve the push times. I think there are other ways to keep optimizing this though.

Is there a reason why we call `pullRemote` 3 times to complete one push? Wouldn't it be most efficient to pull once, store the repo (in `.remote`) for later use, and maybe if we really need to delete it, just delete it after the push is completed (not at the end of the action chain in `clearBareClone` as we do right now)?

Right now, we're making a temp repo for each action that goes through `pullRemote`:

```ts

  try {
    action.proxyGitPath = `${dir}/${action.timestamp}`;

    step.log(`Creating folder ${action.proxyGitPath}`);

    if (!fs.existsSync(dir)) {
      fs.mkdirSync(dir);
    }

    if (!fs.existsSync(action.proxyGitPath)) {
      fs.mkdirSync(action.proxyGitPath, 0o755);
    }

    ...

    await git.clone({
      fs,
      http: gitHttpClient,
      url: action.url,
      onAuth: () => ({
        username,
        password,
      }),
      dir: `${action.proxyGitPath}/${action.repoName}`,
    });
```

And this is always deleted in `clearBareClone.ts` without any other checks - such as whether the push was approved.

```ts
const exec = async (req: any, action: Action): Promise<Action> => {
  const step = new Step('clearBareClone');

  // Recursively remove the contents of ./.remote and ignore exceptions
  fs.rm('./.remote', { recursive: true, force: true }, (err) => {
    if (err) {
      throw err;
    }
    console.log(`.remote is deleted!`);
  });

  action.addStep(step);
  return action;
};
```

Another question: Why is the repo (`.remote`) stored redundantly within GitProxy and within the user's PC as well? Isn't there a way to share the repo contents/state between the two to reduce the need to push/pull twice each time? Perhaps this is just how proxies work by design, but I'm wondering if there's a better solution here.@jescalada: Based on some of the questions from before, I tried modifying the `pullRemote` so that it identifies actions by their ID (which is really just their to/from commits) rather than the timestamp. This makes the second pullRemote call execute about twice as fast: 

## Extra fix (identify/save actions by ID)
### Total push time, before approval `2.578s`

<img width="1493" height="734" alt="Image" src="https://github.com/user-attachments/assets/11ea916b-0b02-42d2-85cc-0158f28735ff" />

### `pullRemote` time, before approval `1.055s`

<img width="1161" height="212" alt="Image" src="https://github.com/user-attachments/assets/78d0eff5-836b-4e9f-97c2-998e6db34fe5" />

### Total push time, after approval `4.549s`

<img width="864" height="365" alt="Image" src="https://github.com/user-attachments/assets/93b59a93-836a-478f-8f1c-b2565fdcf427" />

### `pullRemote` time, after approval `1.018s` + **`0.613s`**

<img width="1151" height="197" alt="Image" src="https://github.com/user-attachments/assets/f26bb079-d3fe-42bb-9aeb-1e29b91eb421" />

<img width="1058" height="158" alt="Image" src="https://github.com/user-attachments/assets/8071a3aa-da1a-41f9-8a16-e4c3f486e648" />

Notice how in the last screenshot, pullRemote is calling `git.fetch` instead of `git.clone`.

Feel free to check out the code changes in #1189!@kriswest: > Is there a reason why we call pullRemote 3 times to complete one push? Wouldn't it be most efficient to pull once, store the repo (in .remote) for later use, and maybe if we really need to delete it, just delete it after the push is completed (not at the end of the action chain in clearBareClone as we do right now)?

@JamieSlome mentioned that they'd had to start clearing the remote directories as they store the cached user credentials, which then end up persisting in a temp directory. If you have the dirs persist between first push and second push (with approval) then it'll also up the storage requirements for git proxy (depending on usage), which may not be desirable for some.

Is it 3 times? Once each of first and second push, what's the third?

I think the redundancy between the server and client copies of the checkout maybe unavoidable - in order to compute a diff you need the checkout at present. We could start trying to work with the decompressed push data instead, but I think we'd find ourselves with deltas and not the source to apply them to. I suspect there's not a good solution to that. Hence, were I to change something to try and get the benefit of caching... I'd probably give Git Proxy its own git account and flag telling it its allowed to cache a particular repo using its own account (and then deal with switching account for the push??). However, here's hoping that the changes you've already applied mean we don't need to do that ;-) 

I see your change to fetch instead of clone - I don't know why we've got a clone still hanging out at that point for you to stick to. Also what if there are remote changes that need to be pulled, post-fetch? Finally, do we even need the pull and to examine it if we've already got it approved? Perhaps thats where we should focus - skipping steps in the chain if we recognize the push and don't need to calculate the diff etc..?@kriswest: (although when developing I've appreciated being able to push multiple times and have the push reinterpreted each time...)@jescalada: I've done some more profiling, and it seems that the most time-consuming actions (on push before approval) are:
- `pullRemote`: 1031ms
- `writePack`: 210ms
- `checkHiddenCommits`: 168ms
- `getDiff`: 78ms

These functions take up around ~1.5s out of ~2.3s total execution time for `git push`.

There's also a mystery function being called that takes up a fair bit of time (most of the remaining unknown ~0.8s). I tried profiling just about every proxy-related function but couldn't narrow it down, my guess is that it's some http library or perhaps the proxy itself that takes some time to process each request.

I'll investigate further to see what causes this and if we can do anything about it.@jescalada: I've managed to narrow down the other time-consuming operation: It's the `express-http-proxy` `sendProxyRequest` function. In here, there's an option for streaming instead of chunking, which is _slightly_ faster. What I noticed is that we call `sendProxyRequest` twice with `GET owner/repo-name.git/git-receive-pack`, and these calls take up 400~500ms each. Perhaps this can/should be cached?

As for the double execution on the post-approval push, I noticed that adding the following check to `proxyFilter` fixes it:

```ts
  if (req.method !== 'GET' || !req.url.includes('/info/refs')) {
    return true;
  }
```

This check also seems to prevent the clone/fetch distinction that was happening before (so my previous "fix" is no longer necessary).

The combined improvements seem to reduce total push times to ~1.6s + 2.2s down from ~2.5s + 4.5s:

<img width="1426" height="985" alt="Image" src="https://github.com/user-attachments/assets/2861568b-5321-4473-9eb0-cfb52b1b1c0c" />

Pinging @fabiovincenzi in case this is helpful to debug test the double execution fix!
@kriswest: @jescalada thats probably taking time as its sending the request on to GitHub and proxying the response back? Thats not going to be instantaneous, it'll take as long as github does to receive the request and get a response back to you (twice, see below)...

I'm quite uncertain about caching OR the check/bypass you're adding. What that is doing is skipping chain execution (and hence the checkRepoInAuthorisedList check that now runs on all proxied requests regardless of type). A single git operation can result in multiple requests - the get-receive-pack request you're commenting on is a sort of preflight check that determines what services the git server offers, and what commit SHAs it already holds (so it can slim down the push).

The reason we are proxying two requests is that a push requires two requests! See this (AI-generated diagram):

<img height="450" alt="Image" src="https://github.com/user-attachments/assets/08410e04-d845-4ff6-9946-c936df4c6160" />

We should NOT cache get-receive-pack as its response can change (when someone else is pushing to the same repo). Hence, I would focus on the chain execution for optimisation and leave the proxy step itself alone.

Finally, massive thank you for looking into the execution speed!@kriswest: P.S. If we don't execute the/a chain on every request we proxy (which we weren't doing) the proxy will be exploitable to forward requests on that it should not... If anyone is pen testing I suggest they look into that specifically.@jescalada: @kriswest This is where I'm confused, because I'm seeing a total of three calls to `/git-receive-pack` on the post-approval push, two GETs and one POST...

This is what I'm seeing on my end when intercepting `sendProxyRequest`:

### First Push

```bash
[0] sendProxyRequest request URL: /jescalada/git-proxy-experiments.git/info/refs?service=git-receive-pack
[0] sendProxyRequest request method: GET
...
[0] sendProxyRequest request URL: /jescalada/git-proxy-experiments.git/info/refs?service=git-receive-pack
[0] sendProxyRequest request method: GET
...
[0] sendProxyRequest request URL: /jescalada/git-proxy-experiments.git/info/refs?service=git-upload-pack
[0] sendProxyRequest request method: GET
...
[0] sendProxyRequest request URL: /jescalada/git-proxy-experiments.git/git-upload-pack
[0] sendProxyRequest request method: POST
```

### Second Push

```
[0] sendProxyRequest request URL: /jescalada/git-proxy-experiments.git/info/refs?service=git-receive-pack
[0] sendProxyRequest request method: GET
...
[0] sendProxyRequest request URL: /jescalada/git-proxy-experiments.git/info/refs?service=git-receive-pack
[0] sendProxyRequest request method: GET
...
[0] sendProxyRequest request URL: /jescalada/git-proxy-experiments.git/git-receive-pack
[0] sendProxyRequest request method: POST
...
[0] sendProxyRequest request URL: /jescalada/git-proxy-experiments.git/info/refs?service=git-upload-pack
[0] sendProxyRequest request method: GET
...
[0] sendProxyRequest request URL: /jescalada/git-proxy-experiments.git/git-upload-pack
[0] sendProxyRequest request method: POST
```

The anomalies here are:
1) `GET git-receive-pack` being called twice on both pushes
2) `POST git-receive-pack` never gets called on the pre-approval push@jescalada: Thanks for the explanation! 😃 I'll focus on improving the `writePack`, `checkHiddenCommits` and `getDiff` actions now. That said, I'm still concerned about the unexpected proxy calls from above...@kriswest: > The anomalies here are:
>
> GET git-receive-pack being called twice on both pushes
> POST git-receive-pack never gets called on the pre-approval push

I would guess that the POST git-receive-pack never gets called on the pre-approval push because we block it in the teeAndValidate / proxyFilterFunction... Unless you are logging the request before it hits that. 

However, I have no idea on the double git-receive-pack calls. What client are you using and are the double calls originating there? It might be worth logging the request bodies to see if the calls differ from each other and to (temporarily?) log requests before they are processed so we can see what we are receiving in addition to what we are forwarding?

One of our in-house stochastic parrots seems to think that might be a reasonable debug step, its come up with these possible reasons for the double call:

> A Git client might make two consecutive git-receive-pack GET requests to the same repository for a few reasons, depending on the client implementation, configuration, or network conditions. Here are the most common scenarios:
> 
> 🔁 1. Capability Negotiation Retry
> Some Git clients (especially custom or older ones) may:
> 
> First send a GET request to discover server capabilities.
> Then send a second GET request with slightly different parameters (e.g., different headers or query strings) to retry or refine the negotiation.
> This can happen if the client didn’t get the expected capabilities or references in the first response.
> 
> 🔐 2. Authentication Flow
> If the first GET request is unauthenticated or partially authenticated:
> 
> The server may respond with a 401 Unauthorized.
> The client then retries the same GET request with proper credentials (e.g., Basic Auth or a bearer token).
> This is common in setups using OAuth, SSO, or custom authentication middleware.
> 
> 🧪 3. Client-Side Probing or Preflight
> Some Git clients or tooling (e.g., CI/CD systems, Git GUIs, or IDE integrations) may:
> 
> Probe the repository to check access or availability.
> Then immediately initiate the actual push flow.
> This results in two GET requests that look identical but serve different internal purposes.
> 
> 🧰 4. Proxy or Middleware Interference
> In enterprise environments (like behind a reverse proxy or load balancer), the client might:
> 
> Retry the request due to a timeout or malformed response.
> Be redirected or re-authenticated, triggering a second GET.
> 🧼 5. Bug or Misconfiguration
> Occasionally, duplicate GETs can be caused by:
> 
> A bug in the Git client or plugin.
> Misconfigured hooks or extensions that trigger redundant requests.@jescalada: I looked into the request headers and bodies, and indeed the first "issue" (double `GET /git-receive-pack`) is because the first call lacks an auth header:

```
[0] sendProxyRequest request details: GET /jescalada/git-proxy-experiments.git/info/refs?service=git-receive-pack
[0] sendProxyRequest request headers: {"host":"localhost:8000","user-agent":"git/2.34.1","accept":"*/*","accept-encoding":"deflate, gzip, br, zstd","accept-language":"en-CA, *;q=0.9","pragma":"no-cache"}
...
[0] sendProxyRequest request details: GET /jescalada/git-proxy-experiments.git/info/refs?service=git-receive-pack
[0] sendProxyRequest request headers: {"host":"localhost:8000","authorization":"Basic <auth-string>","user-agent":"git/2.34.1","accept":"*/*","accept-encoding":"deflate, gzip, br, zstd","accept-language":"en-CA, *;q=0.9","pragma":"no-cache"}
```

As you mentioned, the issue with `POST /git-receive-pack` not getting called is due to GitProxy blocking the push until it's approved.

Since these are features and not bugs, I guess we need to look into optimizing the other actions.@jescalada: I experimented with large repos (tried with my [fork of backstage](https://github.com/jescalada/backstage) with 60k+ commits: The overall execution time of each action is dwarfed by the `git.clone` call in `pullRemote`, even with the `singleBranch: true` and `depth: 1` set (in my case, total push times were 45~90s).

I've tried optimizing the `pullRemote` so that it uses git directly through `spawnSync` + `--bare` flag rather than `isomorphic-git`, but this unfortunately didn't improve push times on my end.

I also tried some quick refactors for the actions, but I feel that there's not much to improve aside of the various `git` calls we have sprinkled around (in `checkHiddenCommits`, `writePack`, etc.) or perhaps filesystem operations.

Solution patch:
diff --git a/src/proxy/processors/push-action/checkAuthorEmails.ts b/src/proxy/processors/push-action/checkAuthorEmails.ts
index 86c5ae02..00774cbe 100644
--- a/src/proxy/processors/push-action/checkAuthorEmails.ts
+++ b/src/proxy/processors/push-action/checkAuthorEmails.ts
@@ -30,8 +30,6 @@ const isEmailAllowed = (email: string): boolean => {
 };
 
 const exec = async (req: any, action: Action): Promise<Action> => {
-  console.log({ req, action });
-
   const step = new Step('checkAuthorEmails');
 
   const uniqueAuthorEmails = [
diff --git a/src/proxy/processors/push-action/checkCommitMessages.ts b/src/proxy/processors/push-action/checkCommitMessages.ts
index 01517539..a85b2fa9 100644
--- a/src/proxy/processors/push-action/checkCommitMessages.ts
+++ b/src/proxy/processors/push-action/checkCommitMessages.ts
@@ -51,8 +51,6 @@ const isMessageAllowed = (commitMessage: string): boolean => {
 
 // Execute if the repo is approved
 const exec = async (req: any, action: Action): Promise<Action> => {
-  console.log({ req, action });
-
   const step = new Step('checkCommitMessages');
 
   const uniqueCommitMessages = [...new Set(action.commitData?.map((commit) => commit.message))];
diff --git a/src/proxy/processors/push-action/pullRemote.ts b/src/proxy/processors/push-action/pullRemote.ts
index 991a62ca..73b8981e 100644
--- a/src/proxy/processors/push-action/pullRemote.ts
+++ b/src/proxy/processors/push-action/pullRemote.ts
@@ -9,15 +9,14 @@ const exec = async (req: any, action: Action): Promise<Action> => {
   const step = new Step('pullRemote');
 
   try {
-    action.proxyGitPath = `${dir}/${action.timestamp}`;
-
-    step.log(`Creating folder ${action.proxyGitPath}`);
+    action.proxyGitPath = `${dir}/${action.id}`;
 
     if (!fs.existsSync(dir)) {
       fs.mkdirSync(dir);
     }
 
     if (!fs.existsSync(action.proxyGitPath)) {
+      step.log(`Creating folder ${action.proxyGitPath}`);
       fs.mkdirSync(action.proxyGitPath, 0o755);
     }
 
@@ -33,15 +32,12 @@ const exec = async (req: any, action: Action): Promise<Action> => {
       fs,
       http: gitHttpClient,
       url: action.url,
-      onAuth: () => ({
-        username,
-        password,
-      }),
       dir: `${action.proxyGitPath}/${action.repoName}`,
+      onAuth: () => ({ username, password }),
+      singleBranch: true,
+      depth: 1,
     });
 
-    console.log('Clone Success: ', action.url);
-
     step.log(`Completed ${cmd}`);
     step.setContent(`Completed ${cmd}`);
   } catch (e: any) {
diff --git a/src/proxy/routes/index.ts b/src/proxy/routes/index.ts
index 7b3cf6de..0595f038 100644
--- a/src/proxy/routes/index.ts
+++ b/src/proxy/routes/index.ts
@@ -216,7 +216,8 @@ const getRouter = async () => {
         proxyReqOptDecorator: proxyReqOptDecorator,
         proxyReqBodyDecorator: proxyReqBodyDecorator,
         proxyErrorHandler: proxyErrorHandler,
-      }),
+        stream: true,
+      } as any),
     );
   });
 
@@ -229,7 +230,8 @@ const getRouter = async () => {
     proxyReqOptDecorator: proxyReqOptDecorator,
     proxyReqBodyDecorator: proxyReqBodyDecorator,
     proxyErrorHandler: proxyErrorHandler,
-  });
+    stream: true,
+  } as any);
 
   console.log('proxy keys registered: ', JSON.stringify(proxyKeys));
 
diff --git a/src/service/routes/push.js b/src/service/routes/push.js
index dd746a11..80821600 100644
--- a/src/service/routes/push.js
+++ b/src/service/routes/push.js
@@ -78,8 +78,6 @@ router.post('/:id/reject', async (req, res) => {
 });
 
 router.post('/:id/authorise', async (req, res) => {
-  console.log({ req });
-
   const questions = req.body.params?.attestation;
   console.log({ questions });
 



Test patch:
diff --git a/test/processors/clearBareClone.test.js b/test/processors/clearBareClone.test.js
index 3f869ff9..c5846091 100644
--- a/test/processors/clearBareClone.test.js
+++ b/test/processors/clearBareClone.test.js
@@ -6,11 +6,13 @@ const { Action } = require('../../src/proxy/actions/Action');
 chai.should();
 
 const expect = chai.expect;
+
+const actionId = '123__456';
 const timestamp = Date.now();
 
 describe('clear bare and local clones', async () => {
   it('pull remote generates a local .remote folder', async () => {
-    const action = new Action('123', 'type', 'get', timestamp, 'finos/git-proxy.git');
+    const action = new Action(actionId, 'type', 'get', timestamp, 'finos/git-proxy.git');
     action.url = 'https://github.com/finos/git-proxy.git';
 
     const authorization = `Basic ${Buffer.from('JamieSlome:test').toString('base64')}`;
@@ -24,14 +26,14 @@ describe('clear bare and local clones', async () => {
       action,
     );
 
-    expect(fs.existsSync(`./.remote/${timestamp}`)).to.be.true;
+    expect(fs.existsSync(`./.remote/${actionId}`)).to.be.true;
   }).timeout(20000);
 
   it('clear bare clone function purges .remote folder and specific clone folder', async () => {
-    const action = new Action('123', 'type', 'get', timestamp, 'finos/git-proxy.git');
+    const action = new Action(actionId, 'type', 'get', timestamp, 'finos/git-proxy.git');
     await clearBareClone(null, action);
     expect(fs.existsSync(`./.remote`)).to.throw;
-    expect(fs.existsSync(`./.remote/${timestamp}`)).to.throw;
+    expect(fs.existsSync(`./.remote/${actionId}`)).to.throw;
   });
 
   afterEach(() => {