diff --git a/src/proxy/processors/push-action/scanDiff.ts b/src/proxy/processors/push-action/scanDiff.ts index 899fa6442..df797ec02 100644 --- a/src/proxy/processors/push-action/scanDiff.ts +++ b/src/proxy/processors/push-action/scanDiff.ts @@ -35,8 +35,10 @@ type Match = { const getDiffViolations = (diff: string, organization: string): Match[] | string | null => { // Commit diff is empty, i.e. '', null or undefined if (!diff) { - console.log('No commit diff...'); - return 'No commit diff...'; + console.log('No commit diff found, but this may be legitimate (empty diff)'); + // Empty diff is not necessarily a violation - could be legitimate + // (e.g., cherry-pick with no changes, reverts, etc.) + return null; } // Validation for configured block pattern(s) check... diff --git a/test/integration/forcePush.integration.test.js b/test/integration/forcePush.integration.test.js new file mode 100644 index 000000000..e9af074d7 --- /dev/null +++ b/test/integration/forcePush.integration.test.js @@ -0,0 +1,164 @@ +const path = require('path'); +const simpleGit = require('simple-git'); +const fs = require('fs').promises; +const { Action } = require('../../src/proxy/actions'); +const { exec: getDiff } = require('../../src/proxy/processors/push-action/getDiff'); +const { exec: scanDiff } = require('../../src/proxy/processors/push-action/scanDiff'); + +const chai = require('chai'); +const expect = chai.expect; + +describe('Force Push Integration Test', () => { + let tempDir; + let git; + let initialCommitSHA; + let rebasedCommitSHA; + + before(async function () { + this.timeout(10000); // eslint-disable-line no-invalid-this + + tempDir = path.join(__dirname, '../temp-integration-repo'); + await fs.mkdir(tempDir, { recursive: true }); + git = simpleGit(tempDir); + + await git.init(); + await git.addConfig('user.name', 'Test User'); + await git.addConfig('user.email', 'test@example.com'); + + // Create initial commit + await fs.writeFile(path.join(tempDir, 'base.txt'), 'base content'); + await git.add('.'); + await git.commit('Initial commit'); + + // Create feature commit + await fs.writeFile(path.join(tempDir, 'feature.txt'), 'feature content'); + await git.add('.'); + await git.commit('Add feature'); + + const log = await git.log(); + initialCommitSHA = log.latest.hash; + + // Simulate rebase by amending commit (changes SHA) + await git.commit(['--amend', '-m', 'Add feature (rebased)']); + + const newLog = await git.log(); + rebasedCommitSHA = newLog.latest.hash; + + console.log(`Initial SHA: ${initialCommitSHA}`); + console.log(`Rebased SHA: ${rebasedCommitSHA}`); + }); + + after(async () => { + try { + await fs.rmdir(tempDir, { recursive: true }); + } catch (e) { + // Ignore cleanup errors + } + }); + + describe('Complete force push pipeline', () => { + it('should handle valid diff after rebase scenario', async function () { + this.timeout(5000); // eslint-disable-line no-invalid-this + + // Create action simulating force push with valid SHAs that have actual changes + const action = new Action( + 'valid-diff-integration', + 'push', + 'POST', + Date.now(), + 'test/repo.git', + ); + action.proxyGitPath = path.dirname(tempDir); + action.repoName = path.basename(tempDir); + + // Parent of initial commit to get actual diff content + const parentSHA = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'; + action.commitFrom = parentSHA; + action.commitTo = rebasedCommitSHA; + action.commitData = [ + { + parent: parentSHA, + commit: rebasedCommitSHA, + message: 'Add feature (rebased)', + author: 'Test User', + }, + ]; + + const afterGetDiff = await getDiff({}, action); + expect(afterGetDiff.steps).to.have.length.greaterThan(0); + + const diffStep = afterGetDiff.steps.find((s) => s.stepName === 'diff'); + expect(diffStep).to.exist; + expect(diffStep.error).to.be.false; + expect(diffStep.content).to.be.a('string'); + expect(diffStep.content.length).to.be.greaterThan(0); + + const afterScanDiff = await scanDiff({}, afterGetDiff); + const scanStep = afterScanDiff.steps.find((s) => s.stepName === 'scanDiff'); + + expect(scanStep).to.exist; + expect(scanStep.error).to.be.false; + }); + + it('should handle unreachable commit SHA error', async function () { + this.timeout(5000); // eslint-disable-line no-invalid-this + + // Invalid SHA to trigger error + const action = new Action( + 'unreachable-sha-integration', + 'push', + 'POST', + Date.now(), + 'test/repo.git', + ); + action.proxyGitPath = path.dirname(tempDir); + action.repoName = path.basename(tempDir); + action.commitFrom = 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef'; // Invalid SHA + action.commitTo = rebasedCommitSHA; + action.commitData = [ + { + parent: 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef', + commit: rebasedCommitSHA, + message: 'Add feature (rebased)', + author: 'Test User', + }, + ]; + + const afterGetDiff = await getDiff({}, action); + expect(afterGetDiff.steps).to.have.length.greaterThan(0); + + const diffStep = afterGetDiff.steps.find((s) => s.stepName === 'diff'); + expect(diffStep).to.exist; + expect(diffStep.error).to.be.true; + expect(diffStep.errorMessage).to.be.a('string'); + expect(diffStep.errorMessage.length).to.be.greaterThan(0); + expect(diffStep.errorMessage).to.satisfy( + (msg) => msg.includes('fatal:') && msg.includes('Invalid revision range'), + 'Error message should contain git diff specific error for invalid SHA', + ); + + // scanDiff should not block on missing diff due to error + const afterScanDiff = await scanDiff({}, afterGetDiff); + const scanStep = afterScanDiff.steps.find((s) => s.stepName === 'scanDiff'); + + expect(scanStep).to.exist; + expect(scanStep.error).to.be.false; + }); + + it('should handle missing diff step gracefully', async function () { + const action = new Action( + 'missing-diff-integration', + 'push', + 'POST', + Date.now(), + 'test/repo.git', + ); + + const result = await scanDiff({}, action); + + expect(result.steps).to.have.length(1); + expect(result.steps[0].stepName).to.equal('scanDiff'); + expect(result.steps[0].error).to.be.false; + }); + }); +}); diff --git a/test/processors/scanDiff.emptyDiff.test.js b/test/processors/scanDiff.emptyDiff.test.js new file mode 100644 index 000000000..4a89aba2e --- /dev/null +++ b/test/processors/scanDiff.emptyDiff.test.js @@ -0,0 +1,91 @@ +const { Action } = require('../../src/proxy/actions'); +const { exec } = require('../../src/proxy/processors/push-action/scanDiff'); + +const chai = require('chai'); +const expect = chai.expect; + +describe('scanDiff - Empty Diff Handling', () => { + describe('Empty diff scenarios', () => { + it('should allow empty diff (legitimate empty push)', async () => { + const action = new Action('empty-diff-test', 'push', 'POST', Date.now(), 'test/repo.git'); + + // Simulate getDiff step with empty content + const diffStep = { stepName: 'diff', content: '', error: false }; + action.steps = [diffStep]; + + const result = await exec({}, action); + + expect(result.steps.length).to.equal(2); // diff step + scanDiff step + expect(result.steps[1].error).to.be.false; + expect(result.steps[1].errorMessage).to.be.null; + }); + + it('should allow null diff', async () => { + const action = new Action('null-diff-test', 'push', 'POST', Date.now(), 'test/repo.git'); + + // Simulate getDiff step with null content + const diffStep = { stepName: 'diff', content: null, error: false }; + action.steps = [diffStep]; + + const result = await exec({}, action); + + expect(result.steps.length).to.equal(2); + expect(result.steps[1].error).to.be.false; + expect(result.steps[1].errorMessage).to.be.null; + }); + + it('should allow undefined diff', async () => { + const action = new Action('undefined-diff-test', 'push', 'POST', Date.now(), 'test/repo.git'); + + // Simulate getDiff step with undefined content + const diffStep = { stepName: 'diff', content: undefined, error: false }; + action.steps = [diffStep]; + + const result = await exec({}, action); + + expect(result.steps.length).to.equal(2); + expect(result.steps[1].error).to.be.false; + expect(result.steps[1].errorMessage).to.be.null; + }); + }); + + describe('Normal diff processing', () => { + it('should process valid diff content without blocking', async () => { + const action = new Action('valid-diff-test', 'push', 'POST', Date.now(), 'test/repo.git'); + action.project = 'test-org'; + + // Simulate normal diff content + const normalDiff = `diff --git a/config.js b/config.js +index 1234567..abcdefg 100644 +--- a/config.js ++++ b/config.js +@@ -1,3 +1,4 @@ + module.exports = { ++ newFeature: true, + database: "production" + };`; + + const diffStep = { stepName: 'diff', content: normalDiff, error: false }; + action.steps = [diffStep]; + + const result = await exec({}, action); + + expect(result.steps[1].error).to.be.false; + expect(result.steps[1].errorMessage).to.be.null; + }); + }); + + describe('Error conditions', () => { + it('should handle non-string diff content', async () => { + const action = new Action('non-string-test', 'push', 'POST', Date.now(), 'test/repo.git'); + + const diffStep = { stepName: 'diff', content: 12345, error: false }; + action.steps = [diffStep]; + + const result = await exec({}, action); + + expect(result.steps[1].error).to.be.true; + expect(result.steps[1].errorMessage).to.include('non-string value'); + }); + }); +}); diff --git a/test/scanDiff.test.js b/test/processors/scanDiff.test.js similarity index 94% rename from test/scanDiff.test.js rename to test/processors/scanDiff.test.js index d06baeba9..bd8afd99d 100644 --- a/test/scanDiff.test.js +++ b/test/processors/scanDiff.test.js @@ -1,14 +1,14 @@ const chai = require('chai'); const crypto = require('crypto'); -const processor = require('../src/proxy/processors/push-action/scanDiff'); -const { Action } = require('../src/proxy/actions/Action'); +const processor = require('../../src/proxy/processors/push-action/scanDiff'); +const { Action } = require('../../src/proxy/actions/Action'); const { expect } = chai; -const config = require('../src/config'); -const db = require('../src/db'); +const config = require('../../src/config'); +const db = require('../../src/db'); chai.should(); // Load blocked literals and patterns from configuration... -const commitConfig = require('../src/config/index').getCommitConfig(); +const commitConfig = require('../../src/config/index').getCommitConfig(); const privateOrganizations = config.getPrivateOrganizations(); const blockedLiterals = commitConfig.diff.block.literals; @@ -249,7 +249,7 @@ describe('Scan commit diff...', async () => { expect(errorMessage).to.contains('Your push has been blocked'); } }); - it('When no diff is present, the proxy is blocked...', async () => { + it('When no diff is present, the proxy allows the push (legitimate empty diff)...', async () => { const action = new Action('1', 'type', 'method', 1, 'test/repo.git'); action.steps = [ { @@ -258,10 +258,10 @@ describe('Scan commit diff...', async () => { }, ]; - const { error, errorMessage } = await processor.exec(null, action); + const result = await processor.exec(null, action); + const scanDiffStep = result.steps.find((s) => s.stepName === 'scanDiff'); - expect(error).to.be.true; - expect(errorMessage).to.contains('Your push has been blocked'); + expect(scanDiffStep.error).to.be.false; }); it('When diff is not a string, the proxy is blocked...', async () => {