Skip to content

Add review comments #44

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 54 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
17c7728
initial config + extra code path for review comments
Apr 4, 2024
fc773d9
Added github connector
Apr 4, 2024
42033d9
fix
Apr 4, 2024
08fd961
Added new config parameter to action definition
Apr 4, 2024
2ef0b87
Added commit_id to finding
Apr 4, 2024
6a89f18
updated dist
Apr 4, 2024
1465411
test different commit sha
Apr 4, 2024
3cbb4cd
test on recreation
Apr 4, 2024
aa09476
- Added unique Aikido identifier check
Apr 4, 2024
acc6a6a
Test rerun
Apr 4, 2024
5836ab5
Added support for multi line findings
Apr 4, 2024
22b1ff9
Fix: start_line may not be the same as line
Apr 4, 2024
1cb62b1
Rerun multi line
Apr 4, 2024
599ea73
code clean
Apr 4, 2024
ef48a34
Changed function that handles duplicates to a hash of the commit/path…
Apr 5, 2024
2149cf3
- Added API connection for findings
Apr 5, 2024
8016f61
Mock test
Apr 5, 2024
3c8dbfc
Duplication check
Apr 5, 2024
c2d6cff
Mocked API output
Apr 5, 2024
2213782
- Removed Aikido ID from body
Apr 5, 2024
8f8bdd5
Test duplication
Apr 5, 2024
071d55b
Prettier output data
Apr 5, 2024
87180a7
Added security issue for testing purposes
Apr 5, 2024
d5b6aef
Enable sast
Apr 5, 2024
d11dbb9
Fix
Apr 5, 2024
36f0084
Added php test file
Apr 5, 2024
840878b
fix
Apr 5, 2024
34ec49e
test duplication
Apr 5, 2024
71b53a7
- Added link to scan
Apr 5, 2024
8629a4d
test new finding - link to scan
Apr 5, 2024
7ab8738
Code cleanup
Apr 5, 2024
38fb723
- Adapted review feedback
Apr 5, 2024
6c3f4ef
test code moving down between commits with existing conversation
Apr 5, 2024
512a180
Testing conversation persistence after file delete
Apr 5, 2024
ab9bdce
Cleanup php test file
Apr 5, 2024
ed5e2d4
Split into sub function
Apr 5, 2024
807e923
Removed hash check to test
Apr 5, 2024
964929b
Added test file
Apr 5, 2024
fa1e5f0
duplication test
Apr 5, 2024
fb27b3f
Revert "duplication test"
Apr 5, 2024
77e50ef
Revert "Removed hash check to test"
Apr 5, 2024
4770520
Added output to compare duplicates
Apr 5, 2024
f96508e
Testing edge cases
Apr 5, 2024
87a3066
test case 2
Apr 5, 2024
8353088
testing edge case
Apr 5, 2024
506df4a
test
Apr 5, 2024
c3ce413
test
Apr 5, 2024
dfbc527
test
Apr 5, 2024
f447304
cleanup
Apr 5, 2024
a8cf3ca
Added comments.
Apr 5, 2024
6d04cb2
Cleanup
Apr 5, 2024
57d770e
Extra comment
Apr 5, 2024
ef8a501
core.info on empty github token
Apr 5, 2024
403abd0
Readability
Apr 5, 2024
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
2 changes: 2 additions & 0 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,5 @@ jobs:
minimum-severity: 'MEDIUM'
github-token: ${{ secrets.GITHUB_TOKEN }}
post-scan-status-comment: true
post-sast-review-comments: true
fail-on-sast-scan: true
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ jobs:
minimum-severity: 'CRITICAL'
timeout-seconds: 180
post-scan-status-comment: 'off'
post-sast-review-comments: 'off'
github-token: ${{ secrets.GITHUB_TOKEN }}
```

Expand All @@ -49,6 +50,7 @@ Optional fields:
- `fail-on-sast-scan`: Determines whether Aikido should block on new SAST issues. This is available in all [paid plans](https://www.aikido.dev/pricing).
- `fail-on-iac-scan`: Determines whether Aikido should block on new Infrastructure as Code issues. This is available in all [paid plans](https://www.aikido.dev/pricing).
- `post-scan-status-comment`: Let Aikido post a comment on the PR (when in PR context) with the latest scan status and a link to the scan results. Value can be one of "on", "off" or "only_if_new_findings". When setting this value to "only_if_new_findings" Aikido will only post a comment once new findings are found, and keep it updated afterwards.
- `post-sast-review-comments`: Let Aikido post review comments on the PR (when in PR context) of scan sast findings and a link to the Aikido platform. Value can be one of "on", "off".
- `github-token`: Must be set only if you want Aikido to post a comment on the PR. If the default `${{ secrets.GITHUB_TOKEN }}` environment token does not have write capabilities, Aikido needs a PAT with specific permissions to read and write comments in a PR.


Expand Down
4 changes: 4 additions & 0 deletions action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ inputs:
description: 'Let Aikido post a comment on the PR with a summary of the status, this comment will be updated for each scan. Can be one of "on", "off" or "only_if_new_findings". When setting this value to "only_if_new_findings" Aikido will only post a comment once new findings are found, and keep it updated afterwards.'
required: false
default: "off"
post-sast-review-comments:
description: 'Let Aikido post inline review comments for sast findings. Can be one of "on", "off".'
required: false
default: "off"
github-token:
description: 'A token that the action can use to post the status comment, this can be the default GITHUB_TOKEN from the environment with permissions to list and post comments, or a custom PAT.'
required: false
Expand Down
175 changes: 174 additions & 1 deletion dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var __importStar = (this && this.__importStar) || function (mod) {
return result;
};
Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.getScanStatus = exports.startScan = void 0;
exports.getScanFindings = exports.getScanStatus = exports.startScan = void 0;
const httpClient = __importStar(__nccwpck_require__(6255));
const AIKIDO_API_URL = 'https://app.aikido.dev';
const startScan = async (secret, payload) => {
Expand Down Expand Up @@ -76,6 +76,19 @@ const getScanStatus = (secret, scanId) => {
};
};
exports.getScanStatus = getScanStatus;
const getScanFindings = async (secret, scanId) => {
var _a;
const requestClient = new httpClient.HttpClient('ci-github-actions');
const url = new URL(`${AIKIDO_API_URL}/api/integrations/continuous_integration/scan/${scanId}/introducedSastIssues`);
const response = await requestClient.getJson(url.toString(), {
'X-AIK-API-SECRET': secret,
});
if (response.statusCode !== 200 || !response.result) {
throw new Error(`fetch findings failed: did not receive a good result: ${JSON.stringify((_a = response.result) !== null && _a !== void 0 ? _a : {})}`);
}
return response.result;
};
exports.getScanFindings = getScanFindings;


/***/ }),
Expand Down Expand Up @@ -114,11 +127,14 @@ const github = __importStar(__nccwpck_require__(5438));
const api_1 = __nccwpck_require__(8947);
const time_1 = __nccwpck_require__(5597);
const postMessage_1 = __nccwpck_require__(7965);
const postReviewComment_1 = __nccwpck_require__(6588);
const transformPostScanStatusAsComment_1 = __nccwpck_require__(3654);
const transformPostFindingsAsReviewComment_1 = __nccwpck_require__(710);
const STATUS_FAILED = 'FAILED';
const STATUS_SUCCEEDED = 'SUCCEEDED';
const STATUS_TIMED_OUT = 'TIMED_OUT';
const ALLOWED_POST_SCAN_STATUS_OPTIONS = ['on', 'off', 'only_if_new_findings'];
const ALLOWED_POST_REVIEW_COMMENTS_OPTIONS = ['on', 'off'];
async function run() {
var _a, _b, _c, _d, _e, _f, _g, _h, _j, _k, _l, _m, _o, _p, _q, _r, _s, _t, _u, _v, _w, _x, _y, _z, _0, _1;
try {
Expand All @@ -130,6 +146,7 @@ async function run() {
const failOnIacScan = core.getInput('fail-on-iac-scan');
const timeoutInSeconds = parseTimeoutDuration(core.getInput('timeout-seconds'));
let postScanStatusAsComment = core.getInput('post-scan-status-comment');
let postReviewComments = core.getInput('post-sast-review-comments');
if (!['LOW', 'MEDIUM', 'HIGH', 'CRITICAL'].includes(fromSeverity.toUpperCase())) {
core.setOutput('output', STATUS_FAILED);
core.setFailed(`Invalid property value for minimum-severity. Allowed values are: LOW, MEDIUM, HIGH, CRITICAL`);
Expand All @@ -141,6 +158,12 @@ async function run() {
core.setFailed(`Invalid property value for post-scan-status-comment. Allowed values are: ${ALLOWED_POST_SCAN_STATUS_OPTIONS.join(', ')}`);
return;
}
postReviewComments = (0, transformPostFindingsAsReviewComment_1.transformPostFindingsAsReviewComment)(postReviewComments);
if (!ALLOWED_POST_REVIEW_COMMENTS_OPTIONS.includes(postReviewComments)) {
core.setOutput('ouput', STATUS_FAILED);
core.setFailed(`Invalid property value for post-sast-review-comments. Allowed values are: ${ALLOWED_POST_SCAN_STATUS_OPTIONS.join(', ')}`);
return;
}
const startScanPayload = {
version: '1.0.5',
branch_name: ((_c = (_b = (_a = github.context.payload) === null || _a === void 0 ? void 0 : _a.pull_request) === null || _b === void 0 ? void 0 : _b.head) === null || _c === void 0 ? void 0 : _c.ref) || ((_d = github.context.payload) === null || _d === void 0 ? void 0 : _d.ref),
Expand Down Expand Up @@ -221,6 +244,10 @@ async function run() {
}
}
}
const shouldPostReviewComments = (postReviewComments === 'on');
if (shouldPostReviewComments) {
await createReviewComments(secretKey, scanId);
}
core.setOutput('scanResultUrl', result.diff_url);
const { gate_passed = false, new_issues_found = 0, issue_links = [], new_dependency_issues_found = 0, new_iac_issues_found = 0, new_sast_issues_found = 0, } = result;
if (!gate_passed) {
Expand Down Expand Up @@ -248,6 +275,29 @@ async function run() {
core.setFailed(error.message);
}
}
async function createReviewComments(secretKey, scanId) {
try {
const findingResponse = await (0, api_1.getScanFindings)(secretKey, scanId);
const findings = findingResponse.introduced_sast_issues.map(finding => ({
commit_id: findingResponse.end_commit_id,
path: finding.file,
line: finding.end_line,
start_line: finding.start_line,
body: `${finding.title}\n${finding.description}\n**Remediation:** ${finding.remediation}\n**Details**: [View details](https://app.aikido.dev/featurebranch/scan/${scanId})`
}));
if (findings.length > 0) {
await (0, postReviewComment_1.postFindingsAsReviewComments)(findings);
}
}
catch (error) {
if (error instanceof Error) {
core.info(`unable to post review comments due to error: ${error.message}`);
}
else {
core.info(`unable to post review comments due to unknown error`);
}
}
}
function parseTimeoutDuration(rawTimeoutInSeconds) {
if (rawTimeoutInSeconds === '')
return 120;
Expand Down Expand Up @@ -346,6 +396,110 @@ const postScanStatusMessage = async (messageBody, options) => {
exports.postScanStatusMessage = postScanStatusMessage;


/***/ }),

/***/ 6588:
/***/ (function(__unused_webpack_module, exports, __nccwpck_require__) {

"use strict";

var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
if (k2 === undefined) k2 = k;
var desc = Object.getOwnPropertyDescriptor(m, k);
if (!desc || ("get" in desc ? !m.__esModule : desc.writable || desc.configurable)) {
desc = { enumerable: true, get: function() { return m[k]; } };
}
Object.defineProperty(o, k2, desc);
}) : (function(o, m, k, k2) {
if (k2 === undefined) k2 = k;
o[k2] = m[k];
}));
var __setModuleDefault = (this && this.__setModuleDefault) || (Object.create ? (function(o, v) {
Object.defineProperty(o, "default", { enumerable: true, value: v });
}) : function(o, v) {
o["default"] = v;
});
var __importStar = (this && this.__importStar) || function (mod) {
if (mod && mod.__esModule) return mod;
var result = {};
if (mod != null) for (var k in mod) if (k !== "default" && Object.prototype.hasOwnProperty.call(mod, k)) __createBinding(result, mod, k);
__setModuleDefault(result, mod);
return result;
};
Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.postFindingsAsReviewComments = void 0;
const core = __importStar(__nccwpck_require__(2186));
const github = __importStar(__nccwpck_require__(5438));
const crypto = __importStar(__nccwpck_require__(6113));
// This function is used to check duplicates on new scans & bypass certain edge cases.
// The app will compare a hash from an Aikido finding against a hash from a Github comment. As such, we can only use properties that live in both entities (e.g. Aikido hash_snippet can not be used).
// Commit_id was not added to the hash, because Github will only send over the comments from the current commit.
// Body was not added to the hash to avoid multiple comments on the same line.
const parseSnippetHashFromComment = (finding) => {
if (finding.path == null || finding.line == null)
return undefined;
return crypto.createHash('sha256').update(`${finding.path}-${finding.line}`).digest('hex');
};
// Possible edge cases:
// - Previous finding/comment has moved location in newer commit: Github handles this and passes location within current commit.
// - New finding on the same line number as a previous finding: Github handles this as the old comment is not present in current commit.
// - The same finding (previously deleted) is now back. We detect this as a duplicate, so the old conversation is preserved.
const postFindingsAsReviewComments = async (findings) => {
var _a;
const githubToken = core.getInput('github-token');
if (!githubToken || githubToken === '') {
core.info('unable to post review comments: missing github-token input parameter');
return;
}
const context = github.context;
if (context.payload.pull_request == null) {
core.info('unable to post review comments: action is not run in a pull request context');
return;
}
const pullRequestNumber = context.payload.pull_request.number;
const octokit = github.getOctokit(githubToken);
const { data: reviewComments } = await octokit.rest.pulls.listReviewComments({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: pullRequestNumber
});
// Add new review comments
for (const finding of findings) {
const findingId = parseSnippetHashFromComment(finding);
if (findingId === undefined)
continue;
// Duplicate detection
let existingFinding = undefined;
for (const comment of reviewComments) {
const isBot = ((_a = comment.user) === null || _a === void 0 ? void 0 : _a.type) === 'Bot';
const existingCommentId = parseSnippetHashFromComment(comment);
// Skip comments that generate invalid hashes
if (existingCommentId === undefined)
continue;
// Skip comments that aren't a bot
if (!isBot)
continue;
// Check for duplicate
if (findingId != existingCommentId)
continue;
existingFinding = comment;
}
if (typeof existingFinding === 'undefined') {
await octokit.rest.pulls.createReviewComment({
...context.repo,
pull_number: pullRequestNumber,
commit_id: finding.commit_id,
path: finding.path,
body: finding.body,
line: finding.line,
...(finding.start_line != finding.line) && { start_line: finding.start_line }
});
}
}
};
exports.postFindingsAsReviewComments = postFindingsAsReviewComments;


/***/ }),

/***/ 5597:
Expand All @@ -366,6 +520,25 @@ const getCurrentUnixTime = () => {
exports.getCurrentUnixTime = getCurrentUnixTime;


/***/ }),

/***/ 710:
/***/ ((__unused_webpack_module, exports) => {

"use strict";

Object.defineProperty(exports, "__esModule", ({ value: true }));
exports.transformPostFindingsAsReviewComment = void 0;
const transformPostFindingsAsReviewComment = (value) => {
if (value === 'true')
return 'on';
if (value === 'false')
return 'off';
return value;
};
exports.transformPostFindingsAsReviewComment = transformPostFindingsAsReviewComment;


/***/ }),

/***/ 3654:
Expand Down
2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

40 changes: 40 additions & 0 deletions src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,26 @@ export type GetScanStatusResponse =
all_scans_completed: false;
};

export type GetScanFindingsResponse =
{
start_commit_id?: string,
end_commit_id: string,
introduced_sast_issues: [
{
author?: string,
start_column?: number,
end_column?: number,
start_line: number,
end_line: number,
snippet_hash: string,
title: string,
description: string,
remediation: string,
file: string
}
]
}

export const startScan = async (secret: string, payload: Object): Promise<number> => {
const requestClient = new httpClient.HttpClient('ci-github-actions');

Expand Down Expand Up @@ -78,3 +98,23 @@ export const getScanStatus = (secret: string, scanId: number): (() => Promise<Ge
return response.result;
};
};

export const getScanFindings = async (secret: string, scanId: number): Promise<GetScanFindingsResponse> => {
const requestClient = new httpClient.HttpClient('ci-github-actions');

const url = new URL(`${AIKIDO_API_URL}/api/integrations/continuous_integration/scan/${scanId}/introducedSastIssues`);

const response = await requestClient.getJson<GetScanFindingsResponse>(url.toString(), {
'X-AIK-API-SECRET': secret,
});

if (response.statusCode !== 200 || !response.result) {
throw new Error(
`fetch findings failed: did not receive a good result: ${JSON.stringify(
response.result ?? {}
)}`
);
}

return response.result;
};
Loading
Loading