Skip to content
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

token validation feature in ide #496

Merged
merged 10 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@
"scope": "application",
"markdownDescription": "Exclude development dependencies during the scan. Currently, only npm is supported."
},
"jfrog.tokenValidation": {
"type": "boolean",
"scope": "application",
"markdownDescription": "Enable token validation on secret scanning."
},
eyalk007 marked this conversation as resolved.
Show resolved Hide resolved
"jfrog.externalResourcesRepository": {
"type": "string",
"scope": "application",
Expand Down
15 changes: 15 additions & 0 deletions src/main/connect/connectionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import { ScanUtils } from '../utils/scanUtils';
import { ConnectionUtils } from './connectionUtils';
import { XrayScanClient } from 'jfrog-client-js/dist/src/Xray/XrayScanClient';
import { IJasConfig } from 'jfrog-client-js/dist/model/Xray/JasConfig/JasConfig';

Check failure on line 24 in src/main/connect/connectionManager.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest

Cannot find module 'jfrog-client-js/dist/model/Xray/JasConfig/JasConfig' or its corresponding type declarations.

Check failure on line 24 in src/main/connect/connectionManager.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest

Cannot find module 'jfrog-client-js/dist/model/Xray/JasConfig/JasConfig' or its corresponding type declarations.

Check failure on line 24 in src/main/connect/connectionManager.ts

View workflow job for this annotation

GitHub Actions / windows-latest

Cannot find module 'jfrog-client-js/dist/model/Xray/JasConfig/JasConfig' or its corresponding type declarations.

Check failure on line 24 in src/main/connect/connectionManager.ts

View workflow job for this annotation

GitHub Actions / windows-latest

Cannot find module 'jfrog-client-js/dist/model/Xray/JasConfig/JasConfig' or its corresponding type declarations.

Check failure on line 24 in src/main/connect/connectionManager.ts

View workflow job for this annotation

GitHub Actions / macOS-latest

Cannot find module 'jfrog-client-js/dist/model/Xray/JasConfig/JasConfig' or its corresponding type declarations.

Check failure on line 24 in src/main/connect/connectionManager.ts

View workflow job for this annotation

GitHub Actions / macOS-latest

Cannot find module 'jfrog-client-js/dist/model/Xray/JasConfig/JasConfig' or its corresponding type declarations.
attiasas marked this conversation as resolved.
Show resolved Hide resolved

export enum LoginStatus {
Success = 'SUCCESS',
Expand Down Expand Up @@ -959,4 +960,18 @@
}
this._logManager.logMessage(usagePrefix + 'Usage report sent successfully.', 'DEBUG');
}

public async isTokenValidationPlatformEnabled(): Promise<boolean> {
try {
let response: IJasConfig = await this.createJfrogClient()
.xray()
.jasconfig()

Check failure on line 968 in src/main/connect/connectionManager.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest

Property 'jasconfig' does not exist on type 'XrayClient'.

Check failure on line 968 in src/main/connect/connectionManager.ts

View workflow job for this annotation

GitHub Actions / ubuntu-latest

Property 'jasconfig' does not exist on type 'XrayClient'.

Check failure on line 968 in src/main/connect/connectionManager.ts

View workflow job for this annotation

GitHub Actions / windows-latest

Property 'jasconfig' does not exist on type 'XrayClient'.

Check failure on line 968 in src/main/connect/connectionManager.ts

View workflow job for this annotation

GitHub Actions / windows-latest

Property 'jasconfig' does not exist on type 'XrayClient'.

Check failure on line 968 in src/main/connect/connectionManager.ts

View workflow job for this annotation

GitHub Actions / macOS-latest

Property 'jasconfig' does not exist on type 'XrayClient'.

Check failure on line 968 in src/main/connect/connectionManager.ts

View workflow job for this annotation

GitHub Actions / macOS-latest

Property 'jasconfig' does not exist on type 'XrayClient'.
.getJasConfig();
this._logManager.logMessage('Successfully got token validation from platform', 'DEBUG');
eyalk007 marked this conversation as resolved.
Show resolved Hide resolved
return response.enable_token_validation_scanning;
} catch (error) {
this._logManager.logMessage('Failed getting token validation from platform', 'DEBUG');
return false;
}
}
}
31 changes: 31 additions & 0 deletions src/main/scanLogic/scanRunners/analyzerManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { Configuration } from '../../utils/configuration';
import { Translators } from '../../utils/translators';
import { BinaryEnvParams } from './jasRunner';
import { LogUtils } from '../../log/logUtils';
import { DYNAMIC_TOKEN_VALIDATION_MIN_XRAY_VERSION } from './secretsScan';
import * as semver from 'semver';

/**
* Analyzer manager is responsible for running the analyzer on the workspace.
Expand All @@ -24,6 +26,7 @@ export class AnalyzerManager {

private static readonly JFROG_RELEASES_URL: string = 'https://releases.jfrog.io';
public static readonly JF_RELEASES_REPO: string = 'JF_RELEASES_REPO';
public static readonly JF_VALIDATE_SECRETS: string = 'JF_VALIDATE_SECRETS';

public static readonly ENV_PLATFORM_URL: string = 'JF_PLATFORM_URL';
public static readonly ENV_TOKEN: string = 'JF_TOKEN';
Expand Down Expand Up @@ -148,6 +151,33 @@ export class AnalyzerManager {
};
}

private isTokenValidationEnabled(): string {
let xraySemver: semver.SemVer = new semver.SemVer(this._connectionManager.xrayVersion);
if (xraySemver.compare(DYNAMIC_TOKEN_VALIDATION_MIN_XRAY_VERSION) < 0) {
this._logManager.logMessage(
'You cannot use dynamic token validation feature on xray version ' +
this._connectionManager.xrayVersion +
' as it requires xray version ' +
DYNAMIC_TOKEN_VALIDATION_MIN_XRAY_VERSION,
'INFO'
);
return 'false';
}
if (Configuration.enableTokenValidation()) {
return 'true';
}
eyalk007 marked this conversation as resolved.
Show resolved Hide resolved
let response: Promise<boolean> = this._connectionManager.isTokenValidationPlatformEnabled();
let tokenValidation: boolean = false;
response.then(res => {
tokenValidation = res;
});
eyalk007 marked this conversation as resolved.
Show resolved Hide resolved
if (tokenValidation || process.env.JF_VALIDATE_SECRETS) {
return 'true';
}

return 'false';
}

private populateOptionalInformation(binaryVars: NodeJS.ProcessEnv, params?: BinaryEnvParams) {
// Optional proxy information - environment variable
let proxyHttpUrl: string | undefined = process.env['HTTP_PROXY'];
Expand All @@ -160,6 +190,7 @@ export class AnalyzerManager {
proxyHttpUrl = 'http://' + proxyUrl;
proxyHttpsUrl = 'https://' + proxyUrl;
}
binaryVars[AnalyzerManager.JF_VALIDATE_SECRETS] = this.isTokenValidationEnabled();
if (proxyHttpUrl) {
binaryVars[AnalyzerManager.ENV_HTTP_PROXY] = this.addOptionalProxyAuthInformation(proxyHttpUrl);
}
Expand Down
3 changes: 3 additions & 0 deletions src/main/scanLogic/scanRunners/analyzerModels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export interface AnalyzeIssue {
level?: AnalyzerManagerSeverityLevel;
suppressions?: AnalyzeSuppression[];
codeFlows?: CodeFlow[];
properties?: { [key: string]: string };
}

export interface AnalyzeSuppression {
Expand Down Expand Up @@ -96,6 +97,8 @@ export interface FileRegion {
startColumn: number;
endColumn: number;
snippet?: ResultContent;
tokenValidation?: string;
metadata?: string;
eyalk007 marked this conversation as resolved.
Show resolved Hide resolved
}

export interface ResultContent {
Expand Down
3 changes: 3 additions & 0 deletions src/main/scanLogic/scanRunners/secretsScan.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import * as semver from 'semver';
import { ConnectionManager } from '../../connect/connectionManager';
import { LogManager } from '../../log/logManager';
import { IssuesRootTreeNode } from '../../treeDataProviders/issuesTree/issuesRootTreeNode';
Expand All @@ -9,6 +10,8 @@ import { AnalyzerManager } from './analyzerManager';
import { AnalyzeScanRequest, AnalyzerScanResponse, AnalyzerScanRun, ScanType } from './analyzerModels';
import { BinaryEnvParams, JasRunner, RunArgs } from './jasRunner';

export const DYNAMIC_TOKEN_VALIDATION_MIN_XRAY_VERSION: any = semver.coerce('3.101.0');

export interface SecretsScanResponse {
filesWithIssues: FileWithSecurityIssues[];
ignoreCount?: number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import { SecurityIssue } from '../../utils/analyzerUtils';
export class SecretTreeNode extends CodeIssueTreeNode {
private _fullDescription?: string;
private _snippet?: string;
private _tokenValidation?: string;
private _metadata?: string;

constructor(issue: SecurityIssue, location: FileRegion, parent: CodeFileTreeNode) {
super(
Expand All @@ -25,6 +27,8 @@ export class SecretTreeNode extends CodeIssueTreeNode {
issue.severity,
issue.ruleName
);
this._tokenValidation = location.tokenValidation;
this._metadata = location.metadata;
this._snippet = location.snippet?.text;
this._fullDescription = issue.fullDescription;
}
Expand All @@ -33,6 +37,14 @@ export class SecretTreeNode extends CodeIssueTreeNode {
return this._snippet;
}

public get tokenValidation(): string | undefined {
return this._tokenValidation;
}

public get metadata(): string | undefined {
return this._metadata;
}

public get fullDescription(): string | undefined {
return this._fullDescription;
}
Expand All @@ -50,7 +62,9 @@ export class SecretTreeNode extends CodeIssueTreeNode {
endRow: this.regionWithIssue.end.line + 1,
endColumn: this.regionWithIssue.end.character + 1
} as IAnalysisStep,
description: this._fullDescription
description: this._fullDescription,
tokenValidation: this._tokenValidation,
metadata: this._metadata
} as ISecretsPage;
}
}
8 changes: 7 additions & 1 deletion src/main/treeDataProviders/utils/analyzerUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { ProjectDependencyTreeNode } from '../issuesTree/descriptorTree/projectD
import { FileTreeNode } from '../issuesTree/fileTreeNode';
import { IssueTreeNode } from '../issuesTree/issueTreeNode';
import { IssuesRootTreeNode } from '../issuesTree/issuesRootTreeNode';
import { TokenStatus } from '../../types/tokenStatus';

export interface FileWithSecurityIssues {
full_path: string;
Expand Down Expand Up @@ -94,7 +95,12 @@ export class AnalyzerUtils {
location.physicalLocation.artifactLocation.uri
);
let fileIssue: SecurityIssue = AnalyzerUtils.getOrCreateSecurityIssue(fileWithIssues, analyzeIssue, fullDescription);
fileIssue.locations.push(location.physicalLocation.region);
let newLocation: FileRegion = location.physicalLocation.region;
newLocation.tokenValidation = analyzeIssue.properties?.tokenValidation
? (analyzeIssue.properties.tokenValidation.trim() as keyof typeof TokenStatus)
: '';
newLocation.metadata = analyzeIssue.properties?.metadata ? analyzeIssue.properties.metadata.trim() : '';
fileIssue.locations.push(newLocation);
});
}

Expand Down
8 changes: 8 additions & 0 deletions src/main/types/tokenStatus.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export enum TokenStatus {
'Active',
'Unsupported',
'Unavailable',
'Inactive',
'Not a token',
''
}
7 changes: 7 additions & 0 deletions src/main/utils/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ export class Configuration {
return vscode.workspace.getConfiguration(this.jfrogSectionConfigurationKey).get('xray.watchers');
}

/**
* Returns true to scan secrets with token validation enabled
*/
public static enableTokenValidation(): boolean | undefined {
return vscode.workspace.getConfiguration(this.jfrogSectionConfigurationKey).get('tokenValidation');
}

/**
* Return true if exclude dev dependencies option is checked on the jfrog extension configuration page.
*/
Expand Down
27 changes: 26 additions & 1 deletion src/test/resources/secretsScan/analyzerResponse.json
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,35 @@
}
],
"ruleId": "generic"
},
{
"message": {
"text": "Secret keys were found"
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "file:///examples/secrets-demo/../applicable_secret.py"
},
"region": {
"endColumn": 132,
"endLine": 1,
"snippet": {
"text": "sometoken"
},
"startColumn": 12,
"startLine": 1
}
}
}
],
"ruleId": "generic",
"properties": {"tokenValidation": "Active", "metadata": "somemetadata"}
}
]
}
],
"version": "2.1.0",
"$schema": "https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json"
}
}
5 changes: 4 additions & 1 deletion src/test/resources/secretsScan/applicable_base64.js
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
const api_key = "2VTHzn1mKZ/n9apD5P6nxsajSQh8QhmyyKvUIRoZWAHCB8lSbBm3YWx5nOdZ1zPEOaA0zIZy1eFgHgfB2HkfAdVrbQj19kagXDVe"
// eslint-disable-next-line @typescript-eslint/typedef
const api_key = "2VTHzn1mKZ/n9apD5P6nxsajSQh8QhmyyKvUIRoZWAHCB8lSbBm3YWx5nOdZ1zPEOaA0zIZy1eFgHgfB2HkfAdVrbQj19kagXDVe"
// eslint-disable-next-line @typescript-eslint/typedef
const token_key = "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"
19 changes: 19 additions & 0 deletions src/test/resources/secretsScan/expectedScanResponse.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,25 @@
"startLine": 1
}
]
},
{
"ruleId": "REQ.SECRET.KEYS",
"severity": 10,
"ruleName": "Secret keys were found",
"fullDescription": "Storing hardcoded secrets in your source code or binary artifact could lead to several risks.\n\nIf the secret is associated with a wide scope of privileges, attackers could extract it from the source code or binary artifact and use it maliciously to attack many targets. For example, if the hardcoded password gives high-privilege access to an AWS account, the attackers may be able to query/modify company-wide sensitive data without per-user authentication.\n\n## Best practices\n\nUse safe storage when storing high-privilege secrets such as passwords and tokens, for example -\n\n* ### Environment Variables\n\nEnvironment variables are set outside of the application code, and can be dynamically passed to the application only when needed, for example -\n`SECRET_VAR=MySecret ./my_application`\nThis way, `MySecret` does not have to be hardcoded into `my_application`.\n\nNote that if your entire binary artifact is published (ex. a Docker container published to Docker Hub), the value for the environment variable must not be stored in the artifact itself (ex. inside the `Dockerfile` or one of the container's files) but rather must be passed dynamically, for example in the `docker run` call as an argument.\n\n* ### Secret management services\n\nExternal vendors offer cloud-based secret management services, that provide proper access control to each secret. The given access to each secret can be dynamically modified or even revoked. Some examples include -\n\n* [Hashicorp Vault](https://www.vaultproject.io)\n* [AWS KMS](https://aws.amazon.com/kms) (Key Management Service)\n* [Google Cloud KMS](https://cloud.google.com/security-key-management)\n\n## Least-privilege principle\n\nStoring a secret in a hardcoded manner can be made safer, by making sure the secret grants the least amount of privilege as needed by the application.\nFor example - if the application needs to read a specific table from a specific database, and the secret grants access to perform this operation **only** (meaning - no access to other tables, no write access at all) then the damage from any secret leaks is mitigated.\nThat being said, it is still not recommended to store secrets in a hardcoded manner, since this type of storage does not offer any way to revoke or moderate the usage of the secret.\n",
"locations": [
{
"endColumn": 60,
"endLine": 2,
"snippet": {
"text": "token_key = \"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\""
},
"startColumn": 20,
"startLine": 2,
"tokenValidation": "Inactive",
"metadata": ""
}
]
}
]
}
Expand Down
4 changes: 4 additions & 0 deletions src/test/tests/integration/secrets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as path from 'path';
import { AnalyzeScanRequest } from '../../../main/scanLogic/scanRunners/analyzerModels';
import { SecretsRunner, SecretsScanResponse } from '../../../main/scanLogic/scanRunners/secretsScan';
import {
assertIssuesTokenValidationExist,
AnalyzerManagerIntegrationEnv,
assertFileIssuesExist,
assertIssuesExist,
Expand Down Expand Up @@ -87,6 +88,9 @@ describe('Secrets Scan Integration Tests', async () => {

it('Check severity', () => assertIssuesSeverityExist(directoryToScan, response.filesWithIssues, expectedContent.filesWithIssues));

it('Check token validation', () =>
assertIssuesTokenValidationExist(directoryToScan, response.filesWithIssues, expectedContent.filesWithIssues));

it('Check snippet', () =>
assertIssuesLocationSnippetsExist(directoryToScan, response.filesWithIssues, expectedContent.filesWithIssues));
});
Expand Down
4 changes: 3 additions & 1 deletion src/test/tests/secretsScan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { AnalyzerUtils, FileWithSecurityIssues } from '../../main/treeDataProvid
import { ScanResults } from '../../main/types/workspaceIssuesDetails';
import { AppsConfigModule } from '../../main/utils/jfrogAppsConfig/jfrogAppsConfig';
import {
assertTokenValidationResult,
assertFileNodesCreated,
assertIssueNodesCreated,
assertIssuesFullDescription,
Expand Down Expand Up @@ -85,7 +86,7 @@ describe('Secrets Scan Tests', () => {
});

it('Check issue count returned from method', () => {
assert.equal(populatedIssues, 3);
assert.equal(populatedIssues, 4);
});

it('Check timestamp transferred from data to node', () => {
Expand All @@ -106,6 +107,7 @@ describe('Secrets Scan Tests', () => {
it('Check number of file nodes populated as root children', () => assertSameNumberOfFileNodes(testRoot, expectedFilesWithIssues));

describe('Issues populated as nodes', () => {
it('Check token validation', () => assertTokenValidationResult(testRoot, expectedFilesWithIssues, getTestIssueNode));
it('Check number of issues populated in file', () => assertSameNumberOfIssueNodes(testRoot, expectedFilesWithIssues));

it('Check issue nodes created in the file node', () => assertIssueNodesCreated(testRoot, expectedFilesWithIssues, getTestIssueNode));
Expand Down
16 changes: 16 additions & 0 deletions src/test/tests/utils/testAnalyzer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,22 @@ export function findLocationNode(location: FileRegion, fileNode: CodeFileTreeNod
);
}

export function assertTokenValidationResult(
testRoot: IssuesRootTreeNode,
expectedFilesWithIssues: FileWithSecurityIssues[],
getTestIssueNode: (fileNode: CodeFileTreeNode, location: FileRegion) => SecretTreeNode
) {
expectedFilesWithIssues.forEach((expectedFileIssues: FileWithSecurityIssues) => {
let fileNode: CodeFileTreeNode = getTestCodeFileNode(testRoot, expectedFileIssues.full_path);
expectedFileIssues.issues.forEach((expectedIssues: SecurityIssue) => {
expectedIssues.locations.forEach((expectedLocation: FileRegion) => {
assert.deepEqual(getTestIssueNode(fileNode, expectedLocation).metadata, expectedLocation.metadata);
assert.deepEqual(getTestIssueNode(fileNode, expectedLocation).tokenValidation, expectedLocation.tokenValidation);
});
});
});
}

export function assertFileNodesCreated(testRoot: IssuesRootTreeNode, expectedFilesWithIssues: FileWithSecurityIssues[]) {
expectedFilesWithIssues.forEach((fileIssues: FileWithSecurityIssues) => {
assert.isDefined(getTestCodeFileNode(testRoot, fileIssues.full_path));
Expand Down
31 changes: 31 additions & 0 deletions src/test/tests/utils/testIntegration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,37 @@ export function assertIssuesSeverityExist(
});
}

export function assertIssuesTokenValidationExist(
testDataRoot: string,
responseFilesWithIssues: FileWithSecurityIssues[],
expectedFilesWithIssues: FileWithSecurityIssues[]
) {
expectedFilesWithIssues.forEach((expectedFileWithIssues: FileWithSecurityIssues) => {
expectedFileWithIssues.issues.forEach((expectedIssues: SecurityIssue) => {
expectedIssues.locations.forEach((expectedLocation: FileRegion) => {
assert.deepEqual(
getTestLocation(
path.join(testDataRoot, expectedFileWithIssues.full_path),
responseFilesWithIssues,
expectedIssues.ruleId,
expectedLocation
).tokenValidation,
expectedLocation.tokenValidation
);
assert.deepEqual(
getTestLocation(
path.join(testDataRoot, expectedFileWithIssues.full_path),
responseFilesWithIssues,
expectedIssues.ruleId,
expectedLocation
).metadata,
expectedLocation.metadata
);
});
});
});
}

export function assertIssuesLocationSnippetsExist(
testDataRoot: string,
responseFilesWithIssues: FileWithSecurityIssues[],
Expand Down
Loading