From db4525e0d795cc6dc35f1ad50bc6ddeac3e72a74 Mon Sep 17 00:00:00 2001 From: Vivek Khimani Date: Sat, 27 Jul 2024 15:50:09 -0700 Subject: [PATCH] extend allowlist for scms (#80) * expand gitlab allowlist * tests eh * respect code access * code access urls for github * bbdc * done --- README.md | 13 ++++++++++- pkg/allowlist_test.go | 2 ++ pkg/config.go | 52 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 644ea89..9e19bbd 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,7 @@ inbound: github: baseUrl: https://github.example.com/api/v3 token: ... + allowCodeAccess: false # default is false, set to true to allow Semgrep to read file contents ``` Under the hood, this config adds these allowlist items: @@ -102,6 +103,11 @@ Under the hood, this config adds these allowlist items: - POST `https://github.example.com/api/v3/repos/:owner/:repo/pulls/:number/comments` - POST `https://github.example.com/api/v3/repos/:owner/:repo/issues/:number/comments` +And if `allowCodeAccess` is set, additionally: + +- GET `https://github.example.com/api/v3/repos/:repo/contents/:filepath` +- GET `https://github.example.com/api/v3/repos/:repo/commits` + ### GitLab Similarly, the `gitlab` configuration section grants Semgrep access to leave MR comments. @@ -113,7 +119,7 @@ inbound: gitlab: baseUrl: https://gitlab.example.com/api/v4 token: ... - allowCodeAccess: false # default is false, set to true to allow Semgrep to read file contents + allowCodeAccess: false # default is false, set to true to allow Semgrep to read file contents ``` Under the hood, this config adds these allowlist items: @@ -123,13 +129,17 @@ Under the hood, this config adds these allowlist items: - GET `https://gitlab.example.com/api/v4/projects/:project/merge_requests` - GET `https://gitlab.example.com/api/v4/projects/:project/merge_requests/:number/versions` - GET `https://gitlab.example.com/api/v4/projects/:project/merge_requests/:number/discussions` +- GET `https://gitlab.example.com/api/v4/projects/:project/repository/branches` +- GET `https://gitlab.example.com/api/v4/:entity_type/:namespace/projects` - POST `https://gitlab.example.com/api/v4/projects/:project/merge_requests/:number/discussions` +- POST `https://gitlab.example.com/api/v4/projects/:project/merge_requests/:number/discussions/:discussion/notes` - PUT `https://gitlab.example.com/api/v4/projects/:project/merge_requests/:number/discussions/:discussion/notes/:note` - PUT `https://gitlab.example.com/api/v4/projects/:project/merge_requests/:number/discussions/:discussion` And if `allowCodeAccess` is set, additionally: - GET `https://gitlab.example.com/api/v4/projects/:project/repository/files/:filepath` +- GET `https://gitlab.example.com/api/v4/projects/:project/repository/commits` ### Bitbucket @@ -150,6 +160,7 @@ Under the hood, this config adds these allowlist items: - GET `https://bitbucket.example.com/rest/api/latest/projects/:project/repos/:repo/default-branch` - GET `https://bitbucket.example.com/rest/api/latest/projects/:project/:repo/pull-requests` - POST `https://bitbucket.example.com/rest/api/latest/projects/:project/repos/:repo/pull-requests/:number/comments` +- POST `https://bitbucket.example.com/rest/api/latest/projects/:project/repos/:repo/pull-requests/:number/blocker-comments` ### Allowlist diff --git a/pkg/allowlist_test.go b/pkg/allowlist_test.go index fec47dd..b736e75 100644 --- a/pkg/allowlist_test.go +++ b/pkg/allowlist_test.go @@ -108,6 +108,8 @@ func TestAllowlistPathMatch(t *testing.T) { // test path matching assertAllowlistMatch(t, allowlist, "GET", "https://foo.com/wildcard-path/a", true) assertAllowlistMatch(t, allowlist, "GET", "https://foo.com/wildcard-path/a/b", true) + assertAllowlistMatch(t, allowlist, "GET", "https://foo.com/wildcard-path/a/b?foo=bar", true) + assertAllowlistMatch(t, allowlist, "GET", "https://foo.com/wildcard-path/a/b?foo=bar#baz", true) assertAllowlistMatch(t, allowlist, "GET", "https://foo.com/variable-path/a", true) assertAllowlistMatch(t, allowlist, "GET", "https://foo.com/variable-path/a/b", false) assertAllowlistMatch(t, allowlist, "GET", "https://foo.com/hardcoded-path", true) diff --git a/pkg/config.go b/pkg/config.go index f4b3a7e..71eb826 100644 --- a/pkg/config.go +++ b/pkg/config.go @@ -203,8 +203,9 @@ type HeartbeatConfig struct { } type GitHub struct { - BaseURL string `mapstructure:"baseUrl" json:"baseUrl"` - Token string `mapstructure:"token" json:"token"` + BaseURL string `mapstructure:"baseUrl" json:"baseUrl"` + Token string `mapstructure:"token" json:"token"` + AllowCodeAccess bool `mapstructure:"allowCodeAccess" json:"allowCodeAccess"` } type GitLab struct { @@ -393,6 +394,23 @@ func LoadConfig(configFiles []string, deploymentId int) (*Config, error) { Methods: ParseHttpMethods([]string{"GET"}), SetRequestHeaders: headers, }) + + if config.Inbound.GitHub.AllowCodeAccess { + config.Inbound.Allowlist = append(config.Inbound.Allowlist, + // get contents of file + AllowlistItem{ + URL: gitHubBaseUrl.JoinPath("/repos/:repo/contents/:filepath").String(), + Methods: ParseHttpMethods([]string{"GET"}), + SetRequestHeaders: headers, + }, + // Commits + AllowlistItem{ + URL: gitHubBaseUrl.JoinPath("/repos/:repo/commits").String(), + Methods: ParseHttpMethods([]string{"GET"}), + SetRequestHeaders: headers, + }, + ) + } } if config.Inbound.GitLab != nil { @@ -437,12 +455,30 @@ func LoadConfig(configFiles []string, deploymentId int) (*Config, error) { Methods: ParseHttpMethods([]string{"GET"}), SetRequestHeaders: headers, }, + // Projects + AllowlistItem{ + URL: gitLabBaseUrl.JoinPath("/:entity_type/:namespace/projects").String(), + Methods: ParseHttpMethods([]string{"GET"}), + SetRequestHeaders: headers, + }, + // Branches + AllowlistItem{ + URL: gitLabBaseUrl.JoinPath("/projects/:project/repository/branches").String(), + Methods: ParseHttpMethods([]string{"GET"}), + SetRequestHeaders: headers, + }, // post MR comment AllowlistItem{ URL: gitLabBaseUrl.JoinPath("/projects/:project/merge_requests/:number/discussions").String(), Methods: ParseHttpMethods([]string{"GET", "POST"}), SetRequestHeaders: headers, }, + // post MR comment reply + AllowlistItem{ + URL: gitLabBaseUrl.JoinPath("/projects/:project/merge_requests/:number/discussions/:discussion/notes").String(), + Methods: ParseHttpMethods([]string{"POST"}), + SetRequestHeaders: headers, + }, // update MR comment AllowlistItem{ URL: gitLabBaseUrl.JoinPath("/projects/:project/merge_requests/:number/discussions/:discussion/notes/:note").String(), @@ -465,6 +501,12 @@ func LoadConfig(configFiles []string, deploymentId int) (*Config, error) { Methods: ParseHttpMethods([]string{"GET"}), SetRequestHeaders: headers, }, + // Commits + AllowlistItem{ + URL: gitLabBaseUrl.JoinPath("/projects/:project/repository/commits").String(), + Methods: ParseHttpMethods([]string{"GET"}), + SetRequestHeaders: headers, + }, ) } } @@ -524,6 +566,12 @@ func LoadConfig(configFiles []string, deploymentId int) (*Config, error) { Methods: ParseHttpMethods([]string{"POST"}), SetRequestHeaders: headers, }, + // post blockerPR comment + AllowlistItem{ + URL: bitBucketBaseUrl.JoinPath("/projects/:project/repos/:repo/pull-requests/:number/blocker-comments").String(), + Methods: ParseHttpMethods([]string{"POST"}), + SetRequestHeaders: headers, + }, ) }