Merge pull request #1178 from kriswest/1177-return-200-on-reject

fix: return 200 status codes on rejection to ensure error message renders in git client

Author: kriswest

src/proxy/routes/index.ts

@@ -41,7 +41,8 @@ const proxyFilter: ProxyOptions['filter'] = async (req, res) => {
         'Invalid request received',
         null,
       );
-      res.status(400).send(handleMessage('Invalid request received'));
+      // return status 200 to ensure that the error message is rendered by the git client
+      res.status(200).send(handleMessage('Invalid request received'));
       return false;
     }
 
@@ -65,7 +66,8 @@ const proxyFilter: ProxyOptions['filter'] = async (req, res) => {
         action.errorMessage,
         action.blockedMessage,
       );
-      res.status(403).send(packetMessage);
+      // return status 200 to ensure that the error message is rendered by the git client
+      res.status(200).send(packetMessage);
       return false;
     }
 
@@ -90,7 +92,8 @@ const proxyFilter: ProxyOptions['filter'] = async (req, res) => {
       null,
     );
 
-    res.status(500).send(packetMessage);
+    // return status 200 to ensure that the error message is rendered by the git client
+    res.status(200).send(packetMessage);
     return false;
   }
 };
@@ -172,7 +175,7 @@ const teeAndValidate = async (req: Request, res: Response, next: NextFunction) =
           'x-frame-options': 'DENY',
           connection: 'close',
         })
-        .status(403)
+        .status(200) // return status 200 to ensure that the error message is rendered by the git client
         .send(handleMessage(msg));
       return;
     }

test/teeAndValidation.test.js

@@ -56,7 +56,7 @@ describe('teeAndValidate middleware', () => {
     expect(next.called).to.be.false;
 
     expect(res.set.called).to.be.true;
-    expect(res.status.calledWith(403)).to.be.true;
+    expect(res.status.calledWith(200)).to.be.true; // status 200 is used to ensure error message is rendered by git client
     expect(res.send.calledWith(handleMessage('denied!'))).to.be.true;
   });
 

test/testProxyRoute.test.js

@@ -63,7 +63,7 @@ describe('proxy route filter middleware', () => {
       .set('user-agent', 'git/2.42.0')
       .set('accept', 'application/x-git-upload-pack-request');
 
-    expect(res).to.have.status(400);
+    expect(res).to.have.status(200); // status 200 is used to ensure error message is rendered by git client
     expect(res.text).to.contain('Invalid request received');
   });
 
@@ -82,7 +82,7 @@ describe('proxy route filter middleware', () => {
       .send(Buffer.from('0000'))
       .buffer();
 
-    expect(res.status).to.equal(403);
+    expect(res.status).to.equal(200); // status 200 is used to ensure error message is rendered by git client
     expect(res.text).to.contain('You shall not push!');
     expect(res.headers['content-type']).to.include('application/x-git-receive-pack-result');
     expect(res.headers['x-frame-options']).to.equal('DENY');
@@ -466,7 +466,8 @@ describe('proxy express application', async () => {
       .set('accept', 'application/x-git-upload-pack-request')
       .buffer();
 
-    res2.should.have.status(403);
+    res2.should.have.status(200); // status 200 is used to ensure error message is rendered by git client
+    expect(res2.text).to.contain('Rejecting repo');
   }).timeout(5000);
 
   it('should not proxy requests for an unknown project', async function () {
@@ -487,7 +488,8 @@ describe('proxy express application', async () => {
       .set('user-agent', 'git/2.42.0')
       .set('accept', 'application/x-git-upload-pack-request')
       .buffer();
-    res.should.have.status(403);
+    res.should.have.status(200); // status 200 is used to ensure error message is rendered by git client
+    expect(res.text).to.contain('Rejecting repo');
 
     // try (and fail) to proxy a request to the repo via the fallback URL directly
     const res2 = await chai
@@ -496,6 +498,7 @@ describe('proxy express application', async () => {
       .set('user-agent', 'git/2.42.0')
       .set('accept', 'application/x-git-upload-pack-request')
       .buffer();
-    res2.should.have.status(403);
+    res2.should.have.status(200);
+    expect(res2.text).to.contain('Rejecting repo');
   }).timeout(5000);
 });