Skip to content

feat: implement auto approval with pre-receive hooks #954

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Apr 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
9ae58e8
feat: add autoapproved field in Action
fabiovincenzi Mar 6, 2025
cedeff6
feat: handle different exit codes in pre receive
fabiovincenzi Mar 6, 2025
af74f3c
feat: push if autoApproved=true
fabiovincenzi Mar 6, 2025
f672318
feat: change ui to display reviewer just if it is not auto approved
fabiovincenzi Mar 6, 2025
ecf2343
feat(test): edit tests for preReceive in order to handle auto approval
fabiovincenzi Mar 13, 2025
6f725df
Merge branch 'G-Research:main' into auto-approve
fabiovincenzi Mar 20, 2025
c104121
refactor: rename hooks files for testing
fabiovincenzi Mar 24, 2025
e028624
feat: add auto rejection
fabiovincenzi Mar 24, 2025
6b6653e
feat: test auto rejection
fabiovincenzi Mar 24, 2025
f723307
refactor: move attempt functions into autoAction.js
fabiovincenzi Mar 25, 2025
d2174d2
feat: add test for auto rejection
fabiovincenzi Mar 25, 2025
407e6e2
docs: add pre receive docs
fabiovincenzi Mar 25, 2025
22df4cc
Merge branch 'main' into auto-approve
fabiovincenzi Mar 25, 2025
3995599
Merge branch 'main' into auto-approve
fabiovincenzi Mar 27, 2025
b72d5c9
refactor: moved pre-receeive docs into website/docs
fabiovincenzi Mar 31, 2025
c930f11
test(cypress): add test for auto-pproved pushes
fabiovincenzi Apr 2, 2025
7fdc970
Merge remote-tracking branch 'refs/remotes/fabiovincenzi/auto-approve…
fabiovincenzi Apr 2, 2025
583b8ed
test: increase coverage for pre-receive
fabiovincenzi Apr 2, 2025
913a057
test: add tests for actions
fabiovincenzi Apr 2, 2025
c554cce
test: fix test for actions
fabiovincenzi Apr 2, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions cypress/e2e/autoApproved.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import moment from 'moment';

describe('Auto-Approved Push Test', () => {
beforeEach(() => {
cy.intercept('GET', '/api/v1/push/123', {
statusCode: 200,
body: {
steps: [
{
stepName: 'diff',
content: '',
},
],
error: false,
allowPush: true,
authorised: true,
canceled: false,
rejected: false,
autoApproved: true,
autoRejected: false,
commitFrom: 'commitFrom',
commitTo: 'commitTo',
branch: 'refs/heads/main',
user: 'testUser',
id: 'commitFrom__commitTo',
type: 'push',
method: 'POST',
timestamp: 1696161600000,
project: 'testUser',
repoName: 'test.git',
url: 'https://github.com/testUser/test.git',
repo: 'testUser/test.git',
commitData: [
{
tree: '1234',
parent: '12345',
},
],
attestation: {
timestamp: '2023-10-01T12:00:00Z',
autoApproved: true,
},
},
}).as('getPush');
});

it('should display auto-approved message and verify tooltip contains the expected timestamp', () => {
cy.visit('/admin/push/123');

cy.wait('@getPush');

cy.contains('Auto-approved by system').should('be.visible');

cy.get('svg.MuiSvgIcon-root')
.filter((_, el) => getComputedStyle(el).fill === 'rgb(0, 128, 0)')
.invoke('attr', 'style')
.should('include', 'cursor: default')
.and('include', 'opacity: 0.5');

const expectedTooltipTimestamp = moment('2023-10-01T12:00:00Z')
.local()
.format('dddd, MMMM Do YYYY, h:mm:ss a');

cy.get('kbd')
.trigger('mouseover')
.then(() => {
cy.get('.MuiTooltip-tooltip').should('contain', expectedTooltipTimestamp);
});

cy.contains('approved this contribution').should('not.exist');
});
});
15 changes: 15 additions & 0 deletions src/proxy/actions/Action.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
authorised = false;
canceled = false;
rejected = false;
autoApproved = false;
autoRejected = false;
commitFrom;
commitTo;
branch;
Expand Down Expand Up @@ -104,6 +106,19 @@
this.blocked = false;
}

/**
*`
*/
setAutoApproval() {
this.autoApproved = true;

Check warning on line 113 in src/proxy/actions/Action.js

View check run for this annotation

Codecov / codecov/patch

src/proxy/actions/Action.js#L113

Added line #L113 was not covered by tests
}

/**
*`
*/
setAutoRejection() {
this.autoRejected = true;

Check warning on line 120 in src/proxy/actions/Action.js

View check run for this annotation

Codecov / codecov/patch

src/proxy/actions/Action.js#L120

Added line #L120 was not covered by tests
}
/**
* @return {bool}
*/
Expand Down
38 changes: 38 additions & 0 deletions src/proxy/actions/autoActions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
const db = require('../../db');

const attemptAutoApproval = async (action) => {
try {
const attestation = {
timestamp: new Date(),
autoApproved: true,
};
await db.authorise(action.id, attestation);
console.log('Push automatically approved by system.');

return true;
} catch (error) {
console.error('Error during auto-approval:', error.message);
return false;
}
};

const attemptAutoRejection = async (action) => {
try {
const attestation = {
timestamp: new Date(),
autoApproved: true,
};
await db.reject(action.id, attestation);
console.log('Push automatically rejected by system.');

return true;
} catch (error) {
console.error('Error during auto-rejection:', error.message);
return false;
}
};

module.exports = {
attemptAutoApproval,
attemptAutoRejection,
};
6 changes: 6 additions & 0 deletions src/proxy/chain.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const proc = require('./processors');
const { attemptAutoApproval, attemptAutoRejection } = require('./actions/autoActions');

const pushActionChain = [
proc.push.parsePush,
Expand Down Expand Up @@ -40,6 +41,11 @@ const executeChain = async (req) => {
}
} finally {
await proc.push.audit(req, action);
if (action.autoApproved) {
attemptAutoApproval(action);
} else if (action.autoRejected) {
attemptAutoRejection(action);
}
}

return action;
Expand Down
29 changes: 20 additions & 9 deletions src/proxy/processors/push-action/preReceive.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const sanitizeInput = (_req, action) => {

const exec = async (req, action, hookFilePath = './hooks/pre-receive.sh') => {
const step = new Step('executeExternalPreReceiveHook');
let stderrTrimmed = '';

try {
const resolvedPath = path.resolve(hookFilePath);
Expand All @@ -34,23 +35,33 @@ const exec = async (req, action, hookFilePath = './hooks/pre-receive.sh') => {

const { stdout, stderr, status } = hookProcess;

const stderrTrimmed = stderr ? stderr.trim() : '';
stderrTrimmed = stderr ? stderr.trim() : '';
const stdoutTrimmed = stdout ? stdout.trim() : '';

if (status !== 0) {
step.log(`Hook exited with status ${status}`);

if (status === 0) {
step.log('Push automatically approved by pre-receive hook.');
action.addStep(step);
action.setAutoApproval();
} else if (status === 1) {
step.log('Push automatically rejected by pre-receive hook.');
action.addStep(step);
action.setAutoRejection();
} else if (status === 2) {
step.log('Push requires manual approval.');
action.addStep(step);
} else {
step.error = true;
step.log(`Hook stderr: ${stderrTrimmed}`);
step.setError(stdoutTrimmed);
step.log(`Unexpected hook status: ${status}`);
step.setError(stdoutTrimmed || 'Unknown pre-receive hook error.');
action.addStep(step);
return action;
}

step.log('Pre-receive hook executed successfully');
action.addStep(step);
return action;
} catch (error) {
step.error = true;
step.setError(`Hook execution error: ${error.message}`);
step.log('Push failed, pre-receive hook returned an error.');
step.setError(`Hook execution error: ${stderrTrimmed || error.message}`);
action.addStep(step);
return action;
}
Expand Down
92 changes: 57 additions & 35 deletions src/ui/views/PushDetails/PushDetails.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -173,45 +173,67 @@ export default function Dashboard() {
}}
>
<CheckCircle
style={{ cursor: 'pointer', transform: 'scale(0.65)' }}
onClick={() => setAttestation(true)}
style={{
cursor: data.autoApproved ? 'default' : 'pointer',
transform: 'scale(0.65)',
opacity: data.autoApproved ? 0.5 : 1,
}}
onClick={() => {
if (!data.autoApproved) {
setAttestation(true);
}
}}
htmlColor='green'
/>
</span>
<a href={`/admin/user/${data.attestation.reviewer.username}`}>
<img
style={{ width: '45px', borderRadius: '20px' }}
src={`https://github.com/${data.attestation.reviewer.gitAccount}.png`}
/>
</a>
<div>
<p>

{data.autoApproved ? (
<>
<div style={{ paddingTop: '15px' }}>
<p>
<strong>Auto-approved by system</strong>
</p>
</div>
</>
) : (
<>
<a href={`/admin/user/${data.attestation.reviewer.username}`}>
{data.attestation.reviewer.gitAccount}
</a>{' '}
approved this contribution
</p>
<Tooltip
title={moment(data.attestation.timestamp).format(
'dddd, MMMM Do YYYY, h:mm:ss a',
)}
arrow
>
<kbd
style={{
color: 'black',
float: 'right',
}}
>
{moment(data.attestation.timestamp).fromNow()}
</kbd>
</Tooltip>
</div>
<AttestationView
data={data.attestation}
attestation={attestation}
setAttestation={setAttestation}
/>
<img
style={{ width: '45px', borderRadius: '20px' }}
src={`https://github.com/${data.attestation.reviewer.gitAccount}.png`}
/>
</a>
<div>
<p>
<a href={`/admin/user/${data.attestation.reviewer.username}`}>
{data.attestation.reviewer.gitAccount}
</a>{' '}
approved this contribution
</p>
</div>
</>
)}

<Tooltip
title={moment(data.attestation.timestamp).format(
'dddd, MMMM Do YYYY, h:mm:ss a',
)}
arrow
>
<kbd style={{ color: 'black', float: 'right' }}>
{moment(data.attestation.timestamp).fromNow()}
</kbd>
</Tooltip>

{data.autoApproved ? (
<></>
) : (
<AttestationView
data={data.attestation}
attestation={attestation}
setAttestation={setAttestation}
/>
)}
</div>
) : null}
</CardHeader>
Expand Down
Loading
Loading