From ff61ceb7304a64a0f94915fee4d98b1ee63ba2c6 Mon Sep 17 00:00:00 2001
From: Assaf Attias <49212512+attiasas@users.noreply.github.com>
Date: Thu, 5 Dec 2024 11:46:16 +0200
Subject: [PATCH] Show secrets detected at PR scan (#793)
---
.../secret_review_content_no_ca_simplified.md | 43 ++++++++++
.../secret_review_content_no_ca_standard.md | 46 ++++++++++
.../secret_review_content_simplified.md | 43 ++++++++++
.../secrets/secret_review_content_standard.md | 46 ++++++++++
utils/comment.go | 19 +++++
utils/comment_test.go | 83 +++++++++++++++++--
utils/consts.go | 24 +++---
utils/outputwriter/outputcontent.go | 32 ++++++-
utils/outputwriter/outputcontent_test.go | 55 ++++++++++++
utils/params.go | 6 ++
10 files changed, 376 insertions(+), 21 deletions(-)
create mode 100644 testdata/messages/reviewcomment/secrets/secret_review_content_no_ca_simplified.md
create mode 100644 testdata/messages/reviewcomment/secrets/secret_review_content_no_ca_standard.md
create mode 100644 testdata/messages/reviewcomment/secrets/secret_review_content_simplified.md
create mode 100644 testdata/messages/reviewcomment/secrets/secret_review_content_standard.md
diff --git a/testdata/messages/reviewcomment/secrets/secret_review_content_no_ca_simplified.md b/testdata/messages/reviewcomment/secrets/secret_review_content_no_ca_simplified.md
new file mode 100644
index 000000000..d953e303a
--- /dev/null
+++ b/testdata/messages/reviewcomment/secrets/secret_review_content_no_ca_simplified.md
@@ -0,0 +1,43 @@
+
+
+---
+## 🗝️ Secret Detected
+
+---
+| Severity | Finding |
+| :---------------------: | :-----------------------------------: |
+| Medium | Secret keys were found |
+
+---
+### Full description
+
+---
+Storing hardcoded secrets in your source code or binary artifact could lead to several risks.
+
+If 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.
+
+## Best practices
+
+Use safe storage when storing high-privilege secrets such as passwords and tokens, for example -
+
+* ### Environment Variables
+
+Environment variables are set outside of the application code, and can be dynamically passed to the application only when needed, for example -
+`SECRET_VAR=MySecret ./my_application`
+This way, `MySecret` does not have to be hardcoded into `my_application`.
+
+Note 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.
+
+* ### Secret management services
+
+External 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 -
+
+* [Hashicorp Vault](https://www.vaultproject.io)
+* [AWS KMS](https://aws.amazon.com/kms) (Key Management Service)
+* [Google Cloud KMS](https://cloud.google.com/security-key-management)
+
+## Least-privilege principle
+
+Storing 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.
+For 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.
+That 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.
diff --git a/testdata/messages/reviewcomment/secrets/secret_review_content_no_ca_standard.md b/testdata/messages/reviewcomment/secrets/secret_review_content_no_ca_standard.md
new file mode 100644
index 000000000..6f2607504
--- /dev/null
+++ b/testdata/messages/reviewcomment/secrets/secret_review_content_no_ca_standard.md
@@ -0,0 +1,46 @@
+
+## 🗝️ Secret Detected
+
+
+| Severity | Finding |
+| :---------------------: | :-----------------------------------: |
+| 
Medium | Secret keys were found |
+
+
+
+
+ Full description
+
+
+Storing hardcoded secrets in your source code or binary artifact could lead to several risks.
+
+If 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.
+
+## Best practices
+
+Use safe storage when storing high-privilege secrets such as passwords and tokens, for example -
+
+* ### Environment Variables
+
+Environment variables are set outside of the application code, and can be dynamically passed to the application only when needed, for example -
+`SECRET_VAR=MySecret ./my_application`
+This way, `MySecret` does not have to be hardcoded into `my_application`.
+
+Note 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.
+
+* ### Secret management services
+
+External 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 -
+
+* [Hashicorp Vault](https://www.vaultproject.io)
+* [AWS KMS](https://aws.amazon.com/kms) (Key Management Service)
+* [Google Cloud KMS](https://cloud.google.com/security-key-management)
+
+## Least-privilege principle
+
+Storing 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.
+For 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.
+That 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.
+
+
+
diff --git a/testdata/messages/reviewcomment/secrets/secret_review_content_simplified.md b/testdata/messages/reviewcomment/secrets/secret_review_content_simplified.md
new file mode 100644
index 000000000..e258a4311
--- /dev/null
+++ b/testdata/messages/reviewcomment/secrets/secret_review_content_simplified.md
@@ -0,0 +1,43 @@
+
+
+---
+## 🗝️ Secret Detected
+
+---
+| Severity | Finding | Status |
+| :---------------------: | :-----------------------------------: | :-----------------------------------: |
+| Medium | Secret keys were found | Active |
+
+---
+### Full description
+
+---
+Storing hardcoded secrets in your source code or binary artifact could lead to several risks.
+
+If 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.
+
+## Best practices
+
+Use safe storage when storing high-privilege secrets such as passwords and tokens, for example -
+
+* ### Environment Variables
+
+Environment variables are set outside of the application code, and can be dynamically passed to the application only when needed, for example -
+`SECRET_VAR=MySecret ./my_application`
+This way, `MySecret` does not have to be hardcoded into `my_application`.
+
+Note 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.
+
+* ### Secret management services
+
+External 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 -
+
+* [Hashicorp Vault](https://www.vaultproject.io)
+* [AWS KMS](https://aws.amazon.com/kms) (Key Management Service)
+* [Google Cloud KMS](https://cloud.google.com/security-key-management)
+
+## Least-privilege principle
+
+Storing 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.
+For 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.
+That 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.
diff --git a/testdata/messages/reviewcomment/secrets/secret_review_content_standard.md b/testdata/messages/reviewcomment/secrets/secret_review_content_standard.md
new file mode 100644
index 000000000..d0b5a45f8
--- /dev/null
+++ b/testdata/messages/reviewcomment/secrets/secret_review_content_standard.md
@@ -0,0 +1,46 @@
+
+## 🗝️ Secret Detected
+
+
+| Severity | Finding | Status |
+| :---------------------: | :-----------------------------------: | :-----------------------------------: |
+| 
Medium | Secret keys were found | Active |
+
+
+
+
+ Full description
+
+
+Storing hardcoded secrets in your source code or binary artifact could lead to several risks.
+
+If 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.
+
+## Best practices
+
+Use safe storage when storing high-privilege secrets such as passwords and tokens, for example -
+
+* ### Environment Variables
+
+Environment variables are set outside of the application code, and can be dynamically passed to the application only when needed, for example -
+`SECRET_VAR=MySecret ./my_application`
+This way, `MySecret` does not have to be hardcoded into `my_application`.
+
+Note 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.
+
+* ### Secret management services
+
+External 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 -
+
+* [Hashicorp Vault](https://www.vaultproject.io)
+* [AWS KMS](https://aws.amazon.com/kms) (Key Management Service)
+* [Google Cloud KMS](https://cloud.google.com/security-key-management)
+
+## Least-privilege principle
+
+Storing 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.
+For 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.
+That 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.
+
+
+
diff --git a/utils/comment.go b/utils/comment.go
index b98705aba..1eff87646 100644
--- a/utils/comment.go
+++ b/utils/comment.go
@@ -25,6 +25,7 @@ const (
ApplicableComment ReviewCommentType = "Applicable"
IacComment ReviewCommentType = "Iac"
SastComment ReviewCommentType = "Sast"
+ SecretComment ReviewCommentType = "Secrets"
RescanRequestComment = "rescan"
commentRemovalErrorMsg = "An error occurred while attempting to remove older Frogbot pull request comments:"
@@ -201,6 +202,12 @@ func getNewReviewComments(repo *Repository, issues *IssuesCollection) (commentsT
for _, sast := range issues.Sast {
commentsToAdd = append(commentsToAdd, generateReviewComment(SastComment, sast.Location, generateSourceCodeReviewContent(SastComment, sast, writer)))
}
+ if !repo.Params.PullRequestSecretComments {
+ return
+ }
+ for _, secret := range issues.Secrets {
+ commentsToAdd = append(commentsToAdd, generateReviewComment(SecretComment, secret.Location, generateSourceCodeReviewContent(SecretComment, secret, writer)))
+ }
return
}
@@ -252,6 +259,18 @@ func generateSourceCodeReviewContent(commentType ReviewCommentType, issue format
issue.CodeFlow,
writer,
), writer)
+ case SecretComment:
+ applicability := ""
+ if issue.Applicability != nil {
+ applicability = issue.Applicability.Status
+ }
+ return outputwriter.GenerateReviewCommentContent(outputwriter.SecretReviewContent(
+ issue.Severity,
+ issue.Finding,
+ issue.ScannerDescription,
+ applicability,
+ writer,
+ ), writer)
}
return
}
diff --git a/utils/comment_test.go b/utils/comment_test.go
index 8168b072d..30591b7be 100644
--- a/utils/comment_test.go
+++ b/utils/comment_test.go
@@ -50,11 +50,12 @@ func TestGetFrogbotReviewComments(t *testing.T) {
}
func TestGetNewReviewComments(t *testing.T) {
- repo := &Repository{OutputWriter: &outputwriter.StandardOutput{}}
+ writer := &outputwriter.StandardOutput{}
testCases := []struct {
- name string
- issues *IssuesCollection
- expectedOutput []ReviewComment
+ name string
+ generateSecretsComments bool
+ issues *IssuesCollection
+ expectedOutput []ReviewComment
}{
{
name: "No issues for review comments",
@@ -92,6 +93,73 @@ func TestGetNewReviewComments(t *testing.T) {
},
expectedOutput: []ReviewComment{},
},
+ {
+ name: "Secret review comments",
+ generateSecretsComments: true,
+ issues: &IssuesCollection{
+ Vulnerabilities: []formats.VulnerabilityOrViolationRow{
+ {
+ Summary: "summary-2",
+ Applicable: "Applicable",
+ IssueId: "XRAY-2",
+ ImpactedDependencyDetails: formats.ImpactedDependencyDetails{
+ SeverityDetails: formats.SeverityDetails{Severity: "low"},
+ ImpactedDependencyName: "component-C",
+ },
+ Cves: []formats.CveRow{{Id: "CVE-2023-4321"}},
+ Technology: techutils.Npm,
+ },
+ },
+ Secrets: []formats.SourceCodeRow{
+ {
+ SeverityDetails: formats.SeverityDetails{
+ Severity: "High",
+ SeverityNumValue: 13,
+ },
+ Finding: "secret finding",
+ Applicability: &formats.Applicability{Status: "Inactive"},
+ Location: formats.Location{
+ File: "index.js",
+ StartLine: 5,
+ StartColumn: 6,
+ EndLine: 7,
+ EndColumn: 8,
+ Snippet: "access token exposed",
+ },
+ },
+ },
+ },
+ expectedOutput: []ReviewComment{
+ {
+ Location: formats.Location{
+ File: "index.js",
+ StartLine: 5,
+ StartColumn: 6,
+ EndLine: 7,
+ EndColumn: 8,
+ Snippet: "access token exposed",
+ },
+ Type: SecretComment,
+ CommentInfo: vcsclient.PullRequestComment{
+ CommentInfo: vcsclient.CommentInfo{
+ Content: outputwriter.GenerateReviewCommentContent(outputwriter.SecretReviewContent("High", "secret finding", "", "Inactive", writer), writer),
+ },
+ PullRequestDiff: vcsclient.PullRequestDiff{
+ OriginalFilePath: "index.js",
+ OriginalStartLine: 5,
+ OriginalStartColumn: 6,
+ OriginalEndLine: 7,
+ OriginalEndColumn: 8,
+ NewFilePath: "index.js",
+ NewStartLine: 5,
+ NewStartColumn: 6,
+ NewEndLine: 7,
+ NewEndColumn: 8,
+ },
+ },
+ },
+ },
+ },
{
name: "With issues for review comments",
issues: &IssuesCollection{
@@ -156,7 +224,7 @@ func TestGetNewReviewComments(t *testing.T) {
Type: ApplicableComment,
CommentInfo: vcsclient.PullRequestComment{
CommentInfo: vcsclient.CommentInfo{
- Content: outputwriter.GenerateReviewCommentContent(outputwriter.ApplicableCveReviewContent("Low", "", "", "CVE-2023-4321", "summary-2", "component-C:", "", repo.OutputWriter), repo.OutputWriter),
+ Content: outputwriter.GenerateReviewCommentContent(outputwriter.ApplicableCveReviewContent("Low", "", "", "CVE-2023-4321", "summary-2", "component-C:", "", writer), writer),
},
PullRequestDiff: vcsclient.PullRequestDiff{
OriginalFilePath: "file1",
@@ -184,7 +252,7 @@ func TestGetNewReviewComments(t *testing.T) {
Type: IacComment,
CommentInfo: vcsclient.PullRequestComment{
CommentInfo: vcsclient.CommentInfo{
- Content: outputwriter.GenerateReviewCommentContent(outputwriter.IacReviewContent("High", "Missing auto upgrade was detected", "", repo.OutputWriter), repo.OutputWriter),
+ Content: outputwriter.GenerateReviewCommentContent(outputwriter.IacReviewContent("High", "Missing auto upgrade was detected", "", writer), writer),
},
PullRequestDiff: vcsclient.PullRequestDiff{
OriginalFilePath: "file1",
@@ -212,7 +280,7 @@ func TestGetNewReviewComments(t *testing.T) {
Type: SastComment,
CommentInfo: vcsclient.PullRequestComment{
CommentInfo: vcsclient.CommentInfo{
- Content: outputwriter.GenerateReviewCommentContent(outputwriter.SastReviewContent("High", "XSS Vulnerability", "", [][]formats.Location{}, repo.OutputWriter), repo.OutputWriter),
+ Content: outputwriter.GenerateReviewCommentContent(outputwriter.SastReviewContent("High", "XSS Vulnerability", "", [][]formats.Location{}, writer), writer),
},
PullRequestDiff: vcsclient.PullRequestDiff{
OriginalFilePath: "file1",
@@ -233,6 +301,7 @@ func TestGetNewReviewComments(t *testing.T) {
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
+ repo := &Repository{OutputWriter: writer, Params: Params{Git: Git{PullRequestSecretComments: tc.generateSecretsComments}}}
output := getNewReviewComments(repo, tc.issues)
assert.ElementsMatch(t, tc.expectedOutput, output)
})
diff --git a/utils/consts.go b/utils/consts.go
index 3a8c46cf3..7f5e919fb 100644
--- a/utils/consts.go
+++ b/utils/consts.go
@@ -34,19 +34,21 @@ const (
JfrogConfigProfileEnv = "JF_CONFIG_PROFILE"
// Git environment variables
- GitProvider = "JF_GIT_PROVIDER"
- GitRepoOwnerEnv = "JF_GIT_OWNER"
- GitRepoEnv = "JF_GIT_REPO"
- GitProjectEnv = "JF_GIT_PROJECT"
- GitUsernameEnv = "JF_GIT_USERNAME"
- GitUseLocalRepositoryEnv = "JF_USE_LOCAL_REPOSITORY"
+ GitProvider = "JF_GIT_PROVIDER"
+ GitRepoOwnerEnv = "JF_GIT_OWNER"
+ GitRepoEnv = "JF_GIT_REPO"
+ GitProjectEnv = "JF_GIT_PROJECT"
+ GitUsernameEnv = "JF_GIT_USERNAME"
+ GitUseLocalRepositoryEnv = "JF_USE_LOCAL_REPOSITORY"
+ UseMostCommonAncestorAsTargetEnv = "JF_USE_MOST_COMMON_ANCESTOR_AS_TARGET"
// Git naming template environment variables
- BranchNameTemplateEnv = "JF_BRANCH_NAME_TEMPLATE"
- CommitMessageTemplateEnv = "JF_COMMIT_MESSAGE_TEMPLATE"
- PullRequestTitleTemplateEnv = "JF_PULL_REQUEST_TITLE_TEMPLATE"
- PullRequestCommentTitleEnv = "JF_PR_COMMENT_TITLE"
- UseMostCommonAncestorAsTargetEnv = "JF_USE_MOST_COMMON_ANCESTOR_AS_TARGET"
+ BranchNameTemplateEnv = "JF_BRANCH_NAME_TEMPLATE"
+ CommitMessageTemplateEnv = "JF_COMMIT_MESSAGE_TEMPLATE"
+ PullRequestTitleTemplateEnv = "JF_PULL_REQUEST_TITLE_TEMPLATE"
+ PullRequestCommentTitleEnv = "JF_PR_COMMENT_TITLE"
+ //#nosec G101 -- not a secret
+ PullRequestSecretCommentsEnv = "JF_PR_SHOW_SECRETS_COMMENTS"
// Repository environment variables - Ignored if the frogbot-config.yml file is used
InstallCommandEnv = "JF_INSTALL_DEPS_CMD"
diff --git a/utils/outputwriter/outputcontent.go b/utils/outputwriter/outputcontent.go
index 8c19e755f..788d5637a 100644
--- a/utils/outputwriter/outputcontent.go
+++ b/utils/outputwriter/outputcontent.go
@@ -6,6 +6,7 @@ import (
"github.com/jfrog/froggit-go/vcsutils"
"github.com/jfrog/jfrog-cli-security/utils/formats"
+ "github.com/jfrog/jfrog-cli-security/utils/jasutils"
"github.com/jfrog/jfrog-cli-security/utils/results"
)
@@ -19,8 +20,10 @@ const (
vulnerableDependenciesResearchDetailsSubTitle = "🔬 Research Details"
contextualAnalysisTitle = "📦🔍 Contextual Analysis CVE Vulnerability"
- iacTitle = "🛠️ Infrastructure as Code Vulnerability"
- sastTitle = "🎯 Static Application Security Testing (SAST) Vulnerability"
+ //#nosec G101 -- not a secret
+ secretsTitle = "🗝️ Secret Detected"
+ iacTitle = "🛠️ Infrastructure as Code Vulnerability"
+ sastTitle = "🎯 Static Application Security Testing (SAST) Vulnerability"
)
var (
@@ -335,7 +338,30 @@ func ApplicableCveReviewContent(severity, finding, fullDetails, cve, cveDetails,
}
func getJasDescriptionTable(severity, finding string, writer OutputWriter) string {
- return NewMarkdownTable("Severity", "Finding").AddRow(writer.FormattedSeverity(severity, "Applicable"), finding).Build()
+ return NewMarkdownTable("Severity", "Finding").AddRow(writer.FormattedSeverity(severity, jasutils.Applicable.String()), finding).Build()
+}
+
+func getSecretsDescriptionTable(severity, finding, status string, writer OutputWriter) string {
+ columns := []string{"Severity", "Finding"}
+ applicability := jasutils.Applicable.String()
+ if status != "" {
+ columns = append(columns, "Status")
+ if status == jasutils.Inactive.String() {
+ applicability = jasutils.NotApplicable.String()
+ }
+ return NewMarkdownTable(columns...).AddRow(writer.FormattedSeverity(severity, applicability), finding, status).Build()
+ }
+ return NewMarkdownTable(columns...).AddRow(writer.FormattedSeverity(severity, applicability), finding).Build()
+}
+
+func SecretReviewContent(severity, finding, fullDetails, applicability string, writer OutputWriter) string {
+ var contentBuilder strings.Builder
+ WriteContent(&contentBuilder,
+ writer.MarkAsTitle(secretsTitle, 2),
+ writer.MarkInCenter(getSecretsDescriptionTable(severity, finding, applicability, writer)),
+ writer.MarkAsDetails("Full description", 3, fullDetails),
+ )
+ return contentBuilder.String()
}
func IacReviewContent(severity, finding, fullDetails string, writer OutputWriter) string {
diff --git a/utils/outputwriter/outputcontent_test.go b/utils/outputwriter/outputcontent_test.go
index 38d0ff07d..51945f453 100644
--- a/utils/outputwriter/outputcontent_test.go
+++ b/utils/outputwriter/outputcontent_test.go
@@ -636,6 +636,61 @@ func TestApplicableReviewContent(t *testing.T) {
}
}
+func TestSecretsReviewContent(t *testing.T) {
+ testCases := []struct {
+ name string
+ severity, finding, fullDetails, status string
+ cases []OutputTestCase
+ }{
+ {
+ name: "Secret review comment content",
+ severity: "Medium",
+ finding: "Secret keys were found",
+ fullDetails: "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",
+ cases: []OutputTestCase{
+ {
+ name: "Standard output",
+ writer: &StandardOutput{},
+ expectedOutputPath: []string{filepath.Join(testReviewCommentDir, "secrets", "secret_review_content_no_ca_standard.md")},
+ },
+ {
+ name: "Simplified output",
+ writer: &SimplifiedOutput{},
+ expectedOutputPath: []string{filepath.Join(testReviewCommentDir, "secrets", "secret_review_content_no_ca_simplified.md")},
+ },
+ },
+ },
+ {
+ name: "Secret review comment content with applicability status",
+ severity: "Medium",
+ status: "Active",
+ finding: "Secret keys were found",
+ fullDetails: "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",
+ cases: []OutputTestCase{
+ {
+ name: "Standard output",
+ writer: &StandardOutput{},
+ expectedOutputPath: []string{filepath.Join(testReviewCommentDir, "secrets", "secret_review_content_standard.md")},
+ },
+ {
+ name: "Simplified output",
+ writer: &SimplifiedOutput{},
+ expectedOutputPath: []string{filepath.Join(testReviewCommentDir, "secrets", "secret_review_content_simplified.md")},
+ },
+ },
+ },
+ }
+
+ for _, tc := range testCases {
+ for _, test := range tc.cases {
+ t.Run(tc.name+"_"+test.name, func(t *testing.T) {
+ expectedOutput := GetExpectedTestOutput(t, test)
+ assert.Equal(t, expectedOutput, SecretReviewContent(tc.severity, tc.finding, tc.fullDetails, tc.status, test.writer))
+ })
+ }
+ }
+}
+
func TestIacReviewContent(t *testing.T) {
testCases := []struct {
name string
diff --git a/utils/params.go b/utils/params.go
index d3f15b2a4..de1ed7bcd 100644
--- a/utils/params.go
+++ b/utils/params.go
@@ -310,6 +310,7 @@ type Git struct {
CommitMessageTemplate string `yaml:"commitMessageTemplate,omitempty"`
PullRequestTitleTemplate string `yaml:"pullRequestTitleTemplate,omitempty"`
PullRequestCommentTitle string `yaml:"pullRequestCommentTitle,omitempty"`
+ PullRequestSecretComments bool `yaml:"pullRequestSecretComments,omitempty"`
AvoidExtraMessages bool `yaml:"avoidExtraMessages,omitempty"`
EmailAuthor string `yaml:"emailAuthor,omitempty"`
AggregateFixes bool `yaml:"aggregateFixes,omitempty"`
@@ -355,6 +356,11 @@ func (g *Git) extractScanPullRequestEnvParams(gitParamsFromEnv *Git) (err error)
if g.PullRequestCommentTitle == "" {
g.PullRequestCommentTitle = getTrimmedEnv(PullRequestCommentTitleEnv)
}
+ if !g.PullRequestSecretComments {
+ if g.PullRequestSecretComments, err = getBoolEnv(PullRequestSecretCommentsEnv, false); err != nil {
+ return
+ }
+ }
if !g.UseMostCommonAncestorAsTarget {
if g.UseMostCommonAncestorAsTarget, err = getBoolEnv(UseMostCommonAncestorAsTargetEnv, true); err != nil {
return