feat: Sensitive Data Detection in files like (.csv , .xlsx , json) #761

Psingle20 · 2024-10-26T18:07:41Z

This PR introduces the checkSensitiveData feature, which enhances the security by scanning files like .csv for vulnerabilities and sensitive information.
The implementation includes:

Functionality:

Created a push-action CheckSensitiveData which take this diff and scan the changed files for Sensitive Information.
Integrated it push_action chain .
Implemented a Test file for the push-action and modified the chain test file to make sure it works with feature added.

I think this Functionality Solves the issue #745
you can run the custom test implemented using command npx mocha test/SensitiveData.test.js
Edit Proxy.config.json and add the file ext into ProxyFileTypes array . Eg : ".csv"
Also Please run the test/CreateExcel.js file to create a test data for testing.

@JamieSlome Please review this PR and suggest any changes necessary

Citi Hackathon
Team Members
Prachit Ingle Psingle20
Shabbir Kaderi shabbirflow
Chaitanya Deshmukh ChaitanyaD48

linux-foundation-easycla · 2024-10-26T18:07:45Z

The committers listed above are authorized under a signed CLA.

✅ login: Psingle20 / name: Prachit (2e77117, 027a459, c449a9c, d9de766, fd26523, df82548, f6798c5, d2314ac, 868c074, ddc4e98)
✅ login: ChaitanyaD48 / name: Chaitanya Deshmukh (8b257a2)

netlify · 2024-10-26T18:07:58Z

✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name	Link
🔨 Latest commit	`868c074`
🔍 Latest deploy log	https://app.netlify.com/sites/endearing-brigadeiros-63f9d0/deploys/67513cacebea8d0008a644e4

.husky/commit-msg

laukik-target

The code is well-structured, handles different file types effectively.
few improvements are recommended:

Test Coverage: Add tests for no sensitive data, empty files, and file-not-found scenarios.
Optimization: Consider streaming large files for better memory management.

src/proxy/processors/push-action/checkSensitiveData.js

test/CheckSensitive.test.js

Signed-off-by: ChaitanyaD48 <[email protected]>

Psingle20 · 2024-10-28T14:08:39Z

@coopernetes @JamieSlome Could you please review this PR and share your thoughts?

rgmz · 2024-10-30T18:49:10Z

src/proxy/processors/push-action/checkSensitiveData.js

+    const sensitivePatterns = [
+        /\d{3}-\d{2}-\d{4}/, // Social Security Number (SSN)
+        /\b\d{16}\b/, // Credit card numbers
+        /\b\d{5}-\d{4}\b/, // ZIP+4 codes
+        // Add more patterns as needed
+    ];


The intent behind this change is good, though it must be noted these will produce a large number of false positives.

Ideally this wouldn't block (only warn), or would have an easy way to exclude false positives.

Great point @rgmz ! I will think about this

I agree. Not to mention, this does not cover all geographies.

I'm inclined to merge it as it is not configured by default. A more holistic approach with better heuristics is worth investing in for the GitProxy project granted but this is a good enough start.

coopernetes · 2024-12-01T02:16:30Z

src/proxy/processors/push-action/getDiff.js

@@ -2,6 +2,7 @@ const Step = require('../../actions').Step;
 const simpleGit = require('simple-git')


+


Mistaken change?

Suggested change

coopernetes · 2024-12-01T02:16:42Z

src/proxy/processors/push-action/checkSensitiveData.js

+
+
+exec.displayName = 'logFileChanges.exec';
+exports.exec = exec;


Suggested change

exports.exec = exec;

exports.exec = exec;

coopernetes · 2024-12-01T02:17:10Z

src/proxy/processors/push-action/index.js

@@ -11,3 +11,4 @@ exports.checkCommitMessages = require('./checkCommitMessages').exec;
 exports.checkAuthorEmails = require('./checkAuthorEmails').exec;
 exports.checkUserPushPermission = require('./checkUserPushPermission').exec;
 exports.clearBareClone = require('./clearBareClone').exec;
+exports.checkSensitiveData = require('./checkSensitiveData').exec;


Add missing newline at the end of the file.

Suggested change

exports.checkSensitiveData = require('./checkSensitiveData').exec;

exports.checkSensitiveData = require('./checkSensitiveData').exec;

coopernetes · 2024-12-01T02:20:03Z

src/proxy/processors/push-action/checkSensitiveData.js

+const exec = async (req, action) => {
+    const diffStep = action.steps.find((s) => s.stepName === 'diff');
+    const step = new Step('checksensitiveData');
+
+    if (diffStep && diffStep.content) {
+        console.log('Diff content:', diffStep.content);
+
+        // Use the parsing function to get file paths
+        const filePaths = extractFilePathsFromDiff(diffStep.content);
+
+        if (filePaths.length > 0) {
+            // Check for sensitive data in all files
+            const sensitiveDataFound = await Promise.all(filePaths.map(parseFile));
+            const anySensitiveDataDetected = sensitiveDataFound.some(found => found);
+
+            if (anySensitiveDataDetected) {
+                step.blocked= true;
+                step.error = true;
+                step.errorMessage = 'Your push has been blocked due to sensitive data detection.';
+                console.log(step.errorMessage);
+            }
+        } else {
+            console.log('No file paths provided in the diff step.');
+        }
+    } else {
+        console.log('No diff content available.');
+    }
+    action.addStep(step);
+    return action; // Returning action for testing purposes
+};


@Psingle20 since #793 has been released in 1.6.0, can we restructure this functionality into its own plugin? It'll require moving some files around and creating an npm package using npm init.

The other change will be this:

const Step = require('@finos/git-proxy/src/proxy/actions').Step; const config = require('@finos/git-proxy/src/config');

Use plugins/git-proxy-sample-plugins and refer to the docs (to be improved via #811) for details.

I will do that. Btw we have added some refactor in #810 which also contains some more feature should I do a PR individually for all the plugins ?
ALong with this we have added gitleaks support , EXIF metadata check and AIML usage check.

coopernetes · 2024-12-01T02:21:01Z

test/CreateExcel.js

+    fs.mkdirSync(testDataPath, { recursive: true }); // Using recursive to ensure all directories are created
+}
+// Write the Excel file to the test_data directory
+XLSX.writeFile(workbook, path.join(testDataPath, 'sensitive_data2.xlsx'));


Add missing newline at the end of the file.

Suggested change

XLSX.writeFile(workbook, path.join(testDataPath, 'sensitive_data2.xlsx'));

XLSX.writeFile(workbook, path.join(testDataPath, 'sensitive_data2.xlsx'));

coopernetes · 2024-12-01T02:22:27Z

test/chain.test.js

@@ -25,6 +26,7 @@ const mockPushProcessors = {
  pullRemote: sinon.stub(),
  writePack: sinon.stub(),
  getDiff: sinon.stub(),
+  checkSensitiveData : sinon.stub(),


By moving this new functionality in its own plugin, it will keep the proxy chain easier to test. Having to constantly add new functions here will not be maintainable long term. This test itself is also not exactly well structured or easy to maintain so we want to not add to it as much as possible. Plugins are preferred.

proxy.config.json

.husky/commit-msg

.gitignore

coopernetes · 2024-12-01T02:28:19Z

src/proxy/processors/push-action/checkSensitiveData.js

+    const sensitivePatterns = [
+        /\d{3}-\d{2}-\d{4}/, // Social Security Number (SSN)
+        /\b\d{16}\b/, // Credit card numbers
+        /\b\d{5}-\d{4}\b/, // ZIP+4 codes
+        // Add more patterns as needed
+    ];


I agree. Not to mention, this does not cover all geographies.

I'm inclined to merge it as it is not configured by default. A more holistic approach with better heuristics is worth investing in for the GitProxy project granted but this is a good enough start.

Co-authored-by: Thomas Cooper <[email protected]>

Psingle20 added 2 commits October 26, 2024 20:12

feat(csv): add sensitive data check for .csv and .xlsx files

2e77117

feat: add support for .log and .json files

c449a9c

Psingle20 changed the title ~~Feat: Sensitive Data Detection in files like (.csv , .xlsx , json)~~ feat: Sensitive Data Detection in files like (.csv , .xlsx , json) Oct 26, 2024

github-actions bot added the feature label Oct 26, 2024

laukik-target reviewed Oct 26, 2024

View reviewed changes

.husky/commit-msg Outdated Show resolved Hide resolved

laukik-target reviewed Oct 26, 2024

View reviewed changes

src/proxy/processors/push-action/checkSensitiveData.js Show resolved Hide resolved

test/CheckSensitive.test.js Outdated Show resolved Hide resolved

ChaitanyaD48 and others added 3 commits October 27, 2024 08:22

test: add test for edge cases

8b257a2

Signed-off-by: ChaitanyaD48 <[email protected]>

refactor: corrected the way filepaths are geting extracted from diff

027a459

refactor: modified the test to provide exact action and diff content

df82548

rgmz reviewed Oct 30, 2024

View reviewed changes

Psingle20 and others added 3 commits November 10, 2024 21:53

refactor: modified proxy.config to support the feature

fd26523

Merge branch 'finos:main' into CheckFilesData

d2314ac

Merge branch 'finos:main' into CheckFilesData

ddc4e98

coopernetes requested changes Dec 1, 2024

View reviewed changes

Psingle20 and others added 3 commits December 1, 2024 12:10

Update proxy.config.json

d9de766

Co-authored-by: Thomas Cooper <[email protected]>

Update .gitignore

f6798c5

Co-authored-by: Thomas Cooper <[email protected]>

Update .husky/commit-msg

868c074

Co-authored-by: Thomas Cooper <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Sensitive Data Detection in files like (.csv , .xlsx , json) #761

feat: Sensitive Data Detection in files like (.csv , .xlsx , json) #761

Psingle20 commented Oct 26, 2024 •

edited

Loading

linux-foundation-easycla bot commented Oct 26, 2024 •

edited

Loading

netlify bot commented Oct 26, 2024 •

edited

Loading

laukik-target left a comment

Psingle20 commented Oct 28, 2024

rgmz Oct 30, 2024 •

edited

Loading

Psingle20 Nov 9, 2024

coopernetes Dec 1, 2024

coopernetes Dec 1, 2024

coopernetes Dec 1, 2024

coopernetes Dec 1, 2024

coopernetes Dec 1, 2024

Psingle20 Dec 1, 2024

coopernetes Dec 1, 2024

coopernetes Dec 1, 2024

coopernetes Dec 1, 2024

		@@ -2,6 +2,7 @@ const Step = require('../../actions').Step;
		const simpleGit = require('simple-git')



		exec.displayName = 'logFileChanges.exec';
		exports.exec = exec;

	exports.checkSensitiveData = require('./checkSensitiveData').exec;
	exports.checkSensitiveData = require('./checkSensitiveData').exec;

	XLSX.writeFile(workbook, path.join(testDataPath, 'sensitive_data2.xlsx'));
	XLSX.writeFile(workbook, path.join(testDataPath, 'sensitive_data2.xlsx'));

feat: Sensitive Data Detection in files like (.csv , .xlsx , json) #761

Are you sure you want to change the base?

feat: Sensitive Data Detection in files like (.csv , .xlsx , json) #761

Conversation

Psingle20 commented Oct 26, 2024 • edited Loading

linux-foundation-easycla bot commented Oct 26, 2024 • edited Loading

netlify bot commented Oct 26, 2024 • edited Loading

✅ Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

laukik-target left a comment

Choose a reason for hiding this comment

Psingle20 commented Oct 28, 2024

rgmz Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Psingle20 commented Oct 26, 2024 •

edited

Loading

linux-foundation-easycla bot commented Oct 26, 2024 •

edited

Loading

netlify bot commented Oct 26, 2024 •

edited

Loading

rgmz Oct 30, 2024 •

edited

Loading