From d2534cc8798b6676978bb45f7ec92031ea666658 Mon Sep 17 00:00:00 2001 From: frjcomp <107982661+frjcomp@users.noreply.github.com> Date: Tue, 20 Oct 2026 07:07:23 +0000 Subject: [PATCH 1/7] feat(gitlab): scan release assets for secrets Add SDK-backed release discovery, bounded credential-safe downloads, and archive scanning with release provenance. Include configuration, documentation, and unit and end-to-end coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/guides/gitlab.md | 50 +++ docs/introduction/configuration.md | 46 ++ internal/cmd/gitlab/gitlab.go | 2 + internal/cmd/gitlab/gitlab_test.go | 4 + internal/cmd/gitlab/release/release.go | 12 + internal/cmd/gitlab/release/release_test.go | 17 + internal/cmd/gitlab/release/scan/scan.go | 114 +++++ internal/cmd/gitlab/release/scan/scan_test.go | 92 ++++ pkg/config/gen/release_test.go | 42 ++ pkg/docs/release_test.go | 33 ++ pkg/gitlab/release/scan/download.go | 139 ++++++ pkg/gitlab/release/scan/download_test.go | 188 +++++++++ pkg/gitlab/release/scan/scanner.go | 297 +++++++++++++ pkg/gitlab/release/scan/scanner_test.go | 226 ++++++++++ pkg/scan/artifact/release.go | 256 +++++++++++ pkg/scan/artifact/release_test.go | 342 +++++++++++++++ tests/e2e/gitlab/release/scan_test.go | 399 ++++++++++++++++++ 17 files changed, 2259 insertions(+) create mode 100644 internal/cmd/gitlab/release/release.go create mode 100644 internal/cmd/gitlab/release/release_test.go create mode 100644 internal/cmd/gitlab/release/scan/scan.go create mode 100644 internal/cmd/gitlab/release/scan/scan_test.go create mode 100644 pkg/config/gen/release_test.go create mode 100644 pkg/docs/release_test.go create mode 100644 pkg/gitlab/release/scan/download.go create mode 100644 pkg/gitlab/release/scan/download_test.go create mode 100644 pkg/gitlab/release/scan/scanner.go create mode 100644 pkg/gitlab/release/scan/scanner_test.go create mode 100644 pkg/scan/artifact/release.go create mode 100644 pkg/scan/artifact/release_test.go create mode 100644 tests/e2e/gitlab/release/scan_test.go diff --git a/docs/guides/gitlab.md b/docs/guides/gitlab.md index 5b0d2724..6d7bc576 100644 --- a/docs/guides/gitlab.md +++ b/docs/guides/gitlab.md @@ -112,6 +112,56 @@ trufflehog gitlab --token=glpat-[redacted] Note: this only scans repositories you have access to. You can specify single repositories as well. +## Secret Detection in Release Assets + +Release assets can contain credentials in configuration files, packaged +applications, and embedded binary strings. Scan the linked assets of all +published releases visible to your token: + +```bash +pipeleek gl release scan --url https://gitlab.example.com --token [redacted] + +# One project's exact release, including its generated source ZIP +pipeleek gl release scan --url https://gitlab.example.com --token [redacted] \ + --repo group/project --tag v1.0 --include-sources + +# The newest five published releases per project in a group and its subgroups +pipeleek gl release scan --url https://gitlab.example.com --token [redacted] \ + --namespace group --release-limit 5 --threads 4 --max-artifact-size 100Mb +``` + +Project selection follows `gl scan`: use `--repo`, `--namespace`, `--search`, +`--owned`, or `--member`. `--repo` takes precedence over project-list filters; +`--member` applies to global project discovery, not namespace discovery. +`--tag` requires `--repo` and cannot be combined with a nonzero `--release-limit`. +The default limit is `0` (all published releases); upcoming releases are excluded. + +Linked HTTP(S) assets are downloaded even when hosted outside GitLab. Ensure the +linked hosts are within your authorized scan scope. The GitLab token is sent only +to the configured GitLab origin, never to external hosts, different ports, or +cross-origin redirect targets. HTTP(S) links containing user credentials and +HTTPS-to-HTTP redirects are rejected. Signed URL query parameters are omitted from +finding URLs and download-error output. + +Generated source archives are excluded by default. `--include-sources` downloads +one ZIP per release, avoiding duplicate scans of the same source tree in multiple +formats. Release descriptions, evidence files, and Git history are not scanned. +Plain files, printable binary strings, and ZIP, TAR, gzip, and bzip2 archives +(including nested combinations) are scanned without executing assets. Other +archive formats are reported as unsupported. Findings retain project, release +tag/name, asset, and file context. + +`--max-artifact-size` caps both downloaded and cumulative expanded bytes per asset, +including intermediate archive content and extracted member data. Size-limit +skips are reported but do not fail the command. Other download, discovery, archive, +or detection failures leave coverage incomplete: scanning continues where possible, +then exits nonzero with a summary. Unsupported URL schemes and archive formats +are reported as incomplete coverage, not silently treated as successfully scanned. + +The standard `--confidence`, `--secrets-verification`, `--hit-timeout`, and +`--webui` controls are available. Credential verification is enabled by default; +`--secrets-verification=false` disables provider verification. + ## Secret Detection in Pipelines And Artifacts Nowadays most repositories make use of CI/CD pipelines. A config file per repository `.gitlab-ci.yml` defines what jobs are executed. diff --git a/docs/introduction/configuration.md b/docs/introduction/configuration.md index aa2acbda..c3c395d4 100644 --- a/docs/introduction/configuration.md +++ b/docs/introduction/configuration.md @@ -103,6 +103,52 @@ gitea: repo_order: desc # asc or desc; default desc ``` +### GitLab Release Assets + +`pipeleek gl release scan` inherits `gitlab.url`, `gitlab.token`, and the common +scan settings above. Release-specific settings live under `gitlab.release.scan`: + +```yaml +gitlab: + url: https://gitlab.example.com + token: "[redacted]" + release: + scan: + repo: group/project # Optional; mutually exclusive with namespace + namespace: "" # Group and subgroups + search: "" # Project search, not release-tag search + owned: false + member: false # Applies to global project discovery + tag: "" # Exact tag; requires repo; cannot combine with a nonzero release_limit + release_limit: 0 # Published releases per project, newest first; 0 means all + include_sources: false # Also scan one generated source ZIP per release +``` + +`--repo` selects a project directly and takes precedence over project-list +filters. Namespace discovery applies `owned` and `search` and includes shared +projects and subgroups, matching `gl scan`. `member` applies only when neither +`repo` nor `namespace` is set. + +Release scanning uses `common.max_artifact_size` (default `500Mb`) as the maximum +downloaded and expanded bytes per asset. Larger assets are reported as skipped. +`common.threads` (default `4`) bounds concurrent asset workers; +`common.hit_timeout` accepts a duration such as `60s` or an integer number of +seconds. Confidence filtering, credential verification, and the Web UI use the +same common settings as other scans. + +For example: + +```bash +export PIPELEEK_GITLAB_RELEASE_SCAN_RELEASE_LIMIT=5 +export PIPELEEK_GITLAB_RELEASE_SCAN_INCLUDE_SOURCES=true +pipeleek gl release scan --repo group/project +``` + +Linked HTTP(S) assets on external hosts are scanned by default. The GitLab token +is sent only to the configured GitLab origin, never to another origin or through +a cross-origin redirect. See [release scanning](../guides/gitlab.md#secret-detection-in-release-assets) +for coverage and failure behavior. + ## Environment Variables Set any config key using `PIPELEEK_` prefix. Replace dots with underscores: diff --git a/internal/cmd/gitlab/gitlab.go b/internal/cmd/gitlab/gitlab.go index 94e6af72..6871a6f6 100644 --- a/internal/cmd/gitlab/gitlab.go +++ b/internal/cmd/gitlab/gitlab.go @@ -5,6 +5,7 @@ import ( "github.com/CompassSecurity/pipeleek/internal/cmd/gitlab/container" "github.com/CompassSecurity/pipeleek/internal/cmd/gitlab/enum" jobtoken "github.com/CompassSecurity/pipeleek/internal/cmd/gitlab/jobToken" + "github.com/CompassSecurity/pipeleek/internal/cmd/gitlab/release" "github.com/CompassSecurity/pipeleek/internal/cmd/gitlab/renovate" "github.com/CompassSecurity/pipeleek/internal/cmd/gitlab/runners" "github.com/CompassSecurity/pipeleek/internal/cmd/gitlab/scan" @@ -45,6 +46,7 @@ For SOCKS5 proxy: } glCmd.AddCommand(scan.NewScanCmd()) + glCmd.AddCommand(release.NewReleaseRootCmd()) glCmd.AddCommand(runners.NewRunnersRootCmd()) glCmd.AddCommand(vuln.NewVulnCmd()) glCmd.AddCommand(variables.NewVariablesCmd()) diff --git a/internal/cmd/gitlab/gitlab_test.go b/internal/cmd/gitlab/gitlab_test.go index 526993b0..34259030 100644 --- a/internal/cmd/gitlab/gitlab_test.go +++ b/internal/cmd/gitlab/gitlab_test.go @@ -23,6 +23,10 @@ func TestNewGitLabRootCmd(t *testing.T) { assert.Equal(t, "gl [command]", cmd.Use) assert.NotEmpty(t, cmd.Short, "Short description should not be empty") assert.NotEmpty(t, cmd.Long, "Long description should not be empty") + releaseScan, remaining, err := cmd.Find([]string{"release", "scan"}) + require.NoError(t, err) + require.Empty(t, remaining) + assert.Equal(t, "gl release scan", releaseScan.CommandPath()) assert.Equal(t, "GitLab", cmd.GroupID) assert.GreaterOrEqual(t, len(cmd.Commands()), 8, "should have at least 8 subcommands") diff --git a/internal/cmd/gitlab/release/release.go b/internal/cmd/gitlab/release/release.go new file mode 100644 index 00000000..3873b6ae --- /dev/null +++ b/internal/cmd/gitlab/release/release.go @@ -0,0 +1,12 @@ +package release + +import ( + "github.com/CompassSecurity/pipeleek/internal/cmd/gitlab/release/scan" + "github.com/spf13/cobra" +) + +func NewReleaseRootCmd() *cobra.Command { + cmd := &cobra.Command{Use: "release", Short: "GitLab release related commands"} + cmd.AddCommand(scan.NewScanCmd()) + return cmd +} diff --git a/internal/cmd/gitlab/release/release_test.go b/internal/cmd/gitlab/release/release_test.go new file mode 100644 index 00000000..6cb93e97 --- /dev/null +++ b/internal/cmd/gitlab/release/release_test.go @@ -0,0 +1,17 @@ +package release + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewReleaseRootCmd(t *testing.T) { + cmd := NewReleaseRootCmd() + scan, args, err := cmd.Find([]string{"scan"}) + require.NoError(t, err) + require.Empty(t, args) + assert.Equal(t, "scan", scan.Name()) + assert.NotNil(t, scan.Run) +} diff --git a/internal/cmd/gitlab/release/scan/scan.go b/internal/cmd/gitlab/release/scan/scan.go new file mode 100644 index 00000000..50a2ba75 --- /dev/null +++ b/internal/cmd/gitlab/release/scan/scan.go @@ -0,0 +1,114 @@ +package scan + +import ( + "fmt" + "strconv" + "time" + + "github.com/CompassSecurity/pipeleek/internal/cmd/flags" + "github.com/CompassSecurity/pipeleek/pkg/config" + releasescan "github.com/CompassSecurity/pipeleek/pkg/gitlab/release/scan" + "github.com/CompassSecurity/pipeleek/pkg/logging" + "github.com/rs/zerolog/log" + "github.com/spf13/cobra" +) + +var flagBindings = map[string]string{ + "url": "gitlab.url", "token": "gitlab.token", + "repo": "gitlab.release.scan.repo", "namespace": "gitlab.release.scan.namespace", + "search": "gitlab.release.scan.search", "owned": "gitlab.release.scan.owned", + "member": "gitlab.release.scan.member", "tag": "gitlab.release.scan.tag", + "release-limit": "gitlab.release.scan.release_limit", "include-sources": "gitlab.release.scan.include_sources", + "max-artifact-size": "common.max_artifact_size", "threads": "common.threads", + "secrets-verification": "common.secrets_verification", "confidence": "common.confidence_filter", + "hit-timeout": "common.hit_timeout", "webui": "common.webui", +} + +func NewScanCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "scan", Short: "Scan GitLab release assets for secrets", + Long: `Scan linked assets of published GitLab releases for secrets. +By default, scan all releases of all token-visible projects, including HTTP(S) +assets hosted outside GitLab. GitLab credentials are never sent to other origins. +Use --include-sources to also scan one generated source ZIP per release. + +Assets exceeding --max-artifact-size (downloaded or expanded bytes) are skipped. +Other coverage failures are reported while scanning continues, with a nonzero exit. +--member applies to global project discovery; --owned and --search also apply +within --namespace, including subgroups. --repo takes precedence over list filters.`, + Example: `# Scan all visible projects' release assets +pipeleek gl release scan --url https://gitlab.example.com --token [redacted] + +# Scan one release, including its generated source ZIP +pipeleek gl release scan --url https://gitlab.example.com --token [redacted] --repo group/project --tag v1.0 --include-sources + +# Scan the newest five published releases per project in a namespace +pipeleek gl release scan --url https://gitlab.example.com --token [redacted] --namespace group --release-limit 5`, + Args: cobra.NoArgs, Run: Scan, + } + common := config.DefaultCommonScanOptions() + flags.AddCommonScanFlagsNoArtifacts(cmd, &common) + cmd.Flags().String("max-artifact-size", "500Mb", "Maximum downloaded and expanded bytes per asset; larger assets are skipped") + cmd.Flags().StringP("repo", "r", "", "Single repository to scan, format: namespace/repo") + cmd.Flags().StringP("namespace", "n", "", "Namespace to scan, including subgroup projects") + cmd.Flags().StringP("search", "s", "", "Query string for searching projects") + cmd.Flags().BoolP("owned", "o", false, "Scan only user owned repositories") + cmd.Flags().BoolP("member", "m", false, "Scan projects the user is a member of (global project discovery)") + cmd.Flags().String("tag", "", "Exact release tag to scan (requires --repo)") + cmd.Flags().Int("release-limit", 0, "Maximum published releases per project, newest first; 0 scans all") + cmd.Flags().Bool("include-sources", false, "Also scan one generated source ZIP per release") + return cmd +} + +func optionsFromConfig() (releasescan.Options, error) { + maxBytes, err := config.ParseMaxArtifactSize(config.GetString("common.max_artifact_size")) + if err != nil { + return releasescan.Options{}, err + } + rawTimeout := config.GetString("common.hit_timeout") + if _, err := strconv.ParseInt(rawTimeout, 10, 64); err == nil { + rawTimeout += "s" + } + timeout, err := time.ParseDuration(rawTimeout) + if err != nil { + return releasescan.Options{}, fmt.Errorf("invalid hit-timeout: %w", err) + } + o := releasescan.Options{ + URL: config.GetString("gitlab.url"), Token: config.GetString("gitlab.token"), + Repo: config.GetString("gitlab.release.scan.repo"), Namespace: config.GetString("gitlab.release.scan.namespace"), + Search: config.GetString("gitlab.release.scan.search"), Owned: config.GetBool("gitlab.release.scan.owned"), + Member: config.GetBool("gitlab.release.scan.member"), Tag: config.GetString("gitlab.release.scan.tag"), + ReleaseLimit: config.GetInt("gitlab.release.scan.release_limit"), + IncludeSources: config.GetBool("gitlab.release.scan.include_sources"), + MaxBytes: maxBytes, Threads: config.GetInt("common.threads"), + SecretsVerification: config.GetBool("common.secrets_verification"), + Confidence: config.GetStringSlice("common.confidence_filter"), HitTimeout: timeout, + } + return o, o.Validate() +} + +func Scan(cmd *cobra.Command, args []string) { + config.NewCommandSetup(cmd).WithFlagBindings(flagBindings). + RequireKeys("gitlab.url", "gitlab.token").MustBind() + opts, err := optionsFromConfig() + if err != nil { + log.Fatal().Err(err).Msg("Invalid release scan configuration") + } + ui := flags.StartScanWebUI(cmd, opts.URL, flagBindings) + if ui != nil { + defer ui.Close() + } + s := releasescan.NewScanner(opts) + logging.RegisterStatusHook(s.Status) + err = s.Scan() + if ui != nil { + if err == nil { + ui.Wait() + } else { + _ = ui.Close() + } + } + if err != nil { + log.Fatal().Err(err).Msg("Release scan failed") + } +} diff --git a/internal/cmd/gitlab/release/scan/scan_test.go b/internal/cmd/gitlab/release/scan/scan_test.go new file mode 100644 index 00000000..a5fc5b03 --- /dev/null +++ b/internal/cmd/gitlab/release/scan/scan_test.go @@ -0,0 +1,92 @@ +package scan + +import ( + "os" + "path/filepath" + "reflect" + "testing" + "time" + + "github.com/CompassSecurity/pipeleek/internal/cmd/testutil" + "github.com/CompassSecurity/pipeleek/pkg/config" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewScanCmd(t *testing.T) { + cmd := NewScanCmd() + testutil.AssertAllFlagsHaveBindings(t, cmd, flagBindings, "url", "token") + assert.Equal(t, reflect.ValueOf(Scan).Pointer(), reflect.ValueOf(cmd.Run).Pointer()) + assert.Error(t, cmd.Args(cmd, []string{"extra"})) + for name, value := range map[string]string{ + "release-limit": "0", "include-sources": "false", "max-artifact-size": "500Mb", + "threads": "4", "hit-timeout": "1m0s", "secrets-verification": "true", + } { + require.NotNil(t, cmd.Flags().Lookup(name)) + assert.Equal(t, value, cmd.Flags().Lookup(name).DefValue, name) + } + assert.Nil(t, cmd.Flags().Lookup("artifacts")) + assert.Nil(t, cmd.Flags().Lookup("cookie")) +} + +func TestOptionsFromConfig(t *testing.T) { + t.Setenv("PIPELEEK_NO_CONFIG", "1") + t.Setenv("PIPELEEK_GITLAB_RELEASE_SCAN_TAG", "v2") + t.Setenv("PIPELEEK_GITLAB_RELEASE_SCAN_REPO", "group/project") + require.NoError(t, config.InitializeViper("")) + parent := &cobra.Command{Use: "gl"} + parent.PersistentFlags().String("url", "", "") + parent.PersistentFlags().String("token", "", "") + cmd := NewScanCmd() + parent.AddCommand(cmd) + require.NoError(t, cmd.ParseFlags([]string{"--url", "https://gitlab.example.com", "--token", "test-token", "--tag", "v3"})) + require.NoError(t, config.NewCommandSetup(cmd).WithFlagBindings(flagBindings).Bind()) + config.GetViper().Set("common.hit_timeout", "120") + opts, err := optionsFromConfig() + require.NoError(t, err) + assert.Equal(t, "v3", opts.Tag) + assert.Equal(t, "group/project", opts.Repo) + assert.Equal(t, 120*time.Second, opts.HitTimeout) + assert.Equal(t, int64(500000000), opts.MaxBytes) + config.GetViper().Set("common.max_artifact_size", "bad") + _, err = optionsFromConfig() + require.Error(t, err) + config.GetViper().Set("common.max_artifact_size", "1Mb") + config.GetViper().Set("common.hit_timeout", "bad") + _, err = optionsFromConfig() + require.ErrorContains(t, err, "hit-timeout") +} + +func TestConfigurationPrecedence(t *testing.T) { + t.Setenv("PIPELEEK_NO_CONFIG", "") + file := filepath.Join(t.TempDir(), "pipeleek.yaml") + require.NoError(t, os.WriteFile(file, []byte(`gitlab: + url: https://gitlab.example.com + token: test-token + release: + scan: + repo: group/project + release_limit: 7 + include_sources: true +common: + threads: 3 +`), 0600)) + t.Setenv("PIPELEEK_GITLAB_RELEASE_SCAN_RELEASE_LIMIT", "5") + require.NoError(t, config.InitializeViper(file)) + cmd := NewScanCmd() + require.NoError(t, cmd.ParseFlags([]string{"--release-limit", "2"})) + require.NoError(t, config.NewCommandSetup(cmd).WithFlagBindings(flagBindings).Bind()) + opts, err := optionsFromConfig() + require.NoError(t, err) + assert.Equal(t, 2, opts.ReleaseLimit) + assert.Equal(t, 3, opts.Threads) + assert.True(t, opts.IncludeSources) + assert.Equal(t, "group/project", opts.Repo) + require.NoError(t, config.InitializeViper(file)) + cmd = NewScanCmd() + require.NoError(t, config.NewCommandSetup(cmd).WithFlagBindings(flagBindings).Bind()) + opts, err = optionsFromConfig() + require.NoError(t, err) + assert.Equal(t, 5, opts.ReleaseLimit) +} diff --git a/pkg/config/gen/release_test.go b/pkg/config/gen/release_test.go new file mode 100644 index 00000000..fbaa5829 --- /dev/null +++ b/pkg/config/gen/release_test.go @@ -0,0 +1,42 @@ +package gen_test + +import ( + "testing" + + "github.com/CompassSecurity/pipeleek/internal/cmd/gitlab/release" + "github.com/CompassSecurity/pipeleek/pkg/config/gen" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +func TestReleaseScanConfig(t *testing.T) { + root := &cobra.Command{Use: "pipeleek"} + gl := &cobra.Command{Use: "gl"} + gl.PersistentFlags().String("url", "", "") + gl.PersistentFlags().String("token", "", "") + gl.AddCommand(release.NewReleaseRootCmd()) + root.AddCommand(gl) + for _, name := range []string{"repo", "namespace", "search", "owned", "member", "tag", "release_limit", "include_sources"} { + assert.True(t, gen.IsAllowedConfigPath(root, "gitlab.release.scan."+name), name) + } + for _, name := range []string{"threads", "max_artifact_size", "hit_timeout", "webui", "secrets_verification"} { + assert.True(t, gen.IsAllowedConfigPath(root, "common."+name), name) + } + content := gen.GenerateExampleConfig(root) + var parsed struct { + GitLab struct { + Release struct { + Scan struct { + ReleaseLimit int `yaml:"release_limit"` + IncludeSources bool `yaml:"include_sources"` + } + } + } + } + require.NoError(t, yaml.Unmarshal([]byte(content), &parsed)) + assert.Zero(t, parsed.GitLab.Release.Scan.ReleaseLimit) + assert.False(t, parsed.GitLab.Release.Scan.IncludeSources) + assert.Contains(t, content, "PIPELEEK_GITLAB_RELEASE_SCAN_RELEASE_LIMIT") +} diff --git a/pkg/docs/release_test.go b/pkg/docs/release_test.go new file mode 100644 index 00000000..7b96c4a3 --- /dev/null +++ b/pkg/docs/release_test.go @@ -0,0 +1,33 @@ +package docs + +import ( + "os" + "path/filepath" + "testing" + + "github.com/CompassSecurity/pipeleek/internal/cmd/gitlab/release" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReleaseScanDocumentation(t *testing.T) { + root := &cobra.Command{Use: "pipeleek"} + gl := &cobra.Command{Use: "gl"} + gl.AddCommand(release.NewReleaseRootCmd()) + root.AddCommand(gl) + output := t.TempDir() + require.NoError(t, generateDocs(root, output, 0, false)) + data, err := os.ReadFile(filepath.Join(output, "pipeleek", "gl", "release", "scan.md")) + require.NoError(t, err) + assert.Contains(t, string(data), "pipeleek gl release scan") + assert.Contains(t, string(data), "--include-sources") + assert.Contains(t, string(data), "--release-limit") + nav := buildNav(root, 0, "") + require.Len(t, nav.Children, 1) + require.Len(t, nav.Children[0].Children, 1) + releaseNav := nav.Children[0].Children[0] + assert.Equal(t, "Release", releaseNav.Label) + require.Len(t, releaseNav.Children, 1) + assert.Equal(t, "pipeleek/gl/release/scan.md", releaseNav.Children[0].FilePath) +} diff --git a/pkg/gitlab/release/scan/download.go b/pkg/gitlab/release/scan/download.go new file mode 100644 index 00000000..7e807850 --- /dev/null +++ b/pkg/gitlab/release/scan/download.go @@ -0,0 +1,139 @@ +package scan + +import ( + "errors" + "fmt" + "io" + "net" + "net/http" + "net/url" + "strings" + + "github.com/CompassSecurity/pipeleek/pkg/httpclient" +) + +var errAssetTooLarge = errors.New("asset exceeds maximum size") + +type downloader struct { + base *url.URL + token string + limit int64 + client *http.Client +} + +func newDownloader(base *url.URL, token string, limit int64) *downloader { + d := &downloader{ + base: base, token: token, limit: limit, + client: httpclient.GetPipeleekStandardHTTPClient(), + } + d.client.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return errors.New("too many asset redirects") + } + if err := validateAssetURL(req.URL); err != nil { + return err + } + if via[len(via)-1].URL.Scheme == "https" && req.URL.Scheme == "http" { + return errors.New("asset redirect downgrades HTTPS to HTTP") + } + d.setCredentials(req) + return nil + } + return d +} + +func validateAssetURL(u *url.URL) error { + if (u.Scheme != "http" && u.Scheme != "https") || u.Hostname() == "" { + return errors.New("asset URL must use HTTP(S) and include a host") + } + if u.User != nil { + return errors.New("asset URL must not contain user credentials") + } + return nil +} + +func sameOrigin(a, b *url.URL) bool { + port := func(u *url.URL) string { + if p := u.Port(); p != "" { + return p + } + if u.Scheme == "https" { + return "443" + } + return "80" + } + return strings.EqualFold(a.Scheme, b.Scheme) && + strings.EqualFold(a.Hostname(), b.Hostname()) && port(a) == port(b) +} + +func (d *downloader) setCredentials(req *http.Request) { + req.Header.Del("PRIVATE-TOKEN") + req.Header.Del("Authorization") + req.Header.Del("Cookie") + req.Header.Del("Referer") + if sameOrigin(d.base, req.URL) { + req.Header.Set("PRIVATE-TOKEN", d.token) + } +} + +func (d *downloader) resolve(raw string) (*url.URL, error) { + if strings.TrimSpace(raw) == "" { + return nil, errors.New("empty asset URL") + } + u, err := url.Parse(raw) + if err != nil { + return nil, errors.New("invalid asset URL") + } + u = d.base.ResolveReference(u) + if err := validateAssetURL(u); err != nil { + return nil, err + } + return u, nil +} + +// Asset URLs can contain signed query parameters; never include those in output. +func displayURL(u *url.URL) string { + safe := *u + safe.User = nil + safe.RawQuery = "" + safe.ForceQuery = false + safe.Fragment = "" + return safe.String() +} + +func (d *downloader) download(u *url.URL) ([]byte, error) { + req, err := http.NewRequest(http.MethodGet, u.String(), nil) + if err != nil { + return nil, errors.New("cannot create asset request") + } + d.setCredentials(req) + resp, err := d.client.Do(req) + if err != nil { + // url.Error includes the full request URL, potentially containing credentials. + var urlErr *url.Error + if errors.As(err, &urlErr) { + err = urlErr.Err + } + var netErr net.Error + if errors.As(err, &netErr) && netErr.Timeout() { + return nil, errors.New("asset download timed out") + } + // Redirect parsing errors may also embed a signed Location URL. + return nil, errors.New("asset request failed (connection, URL, or redirect policy)") + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("asset download returned HTTP %d", resp.StatusCode) + } + if resp.ContentLength > d.limit { + return nil, errAssetTooLarge + } + content, err := io.ReadAll(io.LimitReader(resp.Body, d.limit+1)) + if err != nil { + return nil, fmt.Errorf("reading asset failed: %w", err) + } + if int64(len(content)) > d.limit { + return nil, errAssetTooLarge + } + return content, nil +} diff --git a/pkg/gitlab/release/scan/download_test.go b/pkg/gitlab/release/scan/download_test.go new file mode 100644 index 00000000..5844a45d --- /dev/null +++ b/pkg/gitlab/release/scan/download_test.go @@ -0,0 +1,188 @@ +package scan + +import ( + "errors" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func testDownloader(t *testing.T, base string, limit int64) *downloader { + t.Helper() + u, err := url.Parse(base) + require.NoError(t, err) + return newDownloader(u, "test-token", limit) +} + +func TestDownloadCredentials(t *testing.T) { + var received []string + external := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + received = append(received, r.Header.Get("PRIVATE-TOKEN")) + assert.Empty(t, r.Header.Get("Authorization")) + assert.Empty(t, r.Header.Get("Cookie")) + assert.Empty(t, r.Header.Get("Referer")) + _, _ = w.Write([]byte("external")) + })) + defer external.Close() + instance := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "test-token", r.Header.Get("PRIVATE-TOKEN")) + if r.URL.Path == "/redirect" { + http.Redirect(w, r, external.URL, http.StatusFound) + return + } + _, _ = w.Write([]byte("internal")) + })) + defer instance.Close() + d := testDownloader(t, instance.URL, 100) + for _, target := range []string{instance.URL, external.URL, instance.URL + "/redirect"} { + u, err := d.resolve(target) + require.NoError(t, err) + _, err = d.download(u) + require.NoError(t, err) + } + assert.Equal(t, []string{"", ""}, received) +} + +func TestDownloadLimitsAndFailures(t *testing.T) { + for _, tc := range []struct { + name string + body string + chunk bool + status int + want error + }{ + {"exact", "12345", false, 200, nil}, + {"oversize", "123456", false, 200, errAssetTooLarge}, + {"chunked-exact", "12345", true, 200, nil}, + {"chunked-oversize", "123456", true, 200, errAssetTooLarge}, + {"denied", "123", false, 403, errors.New("HTTP 403")}, + {"partial", "123", false, 206, errors.New("HTTP 206")}, + } { + t.Run(tc.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(tc.status) + if tc.chunk { + w.(http.Flusher).Flush() + } + _, _ = w.Write([]byte(tc.body)) + })) + defer srv.Close() + d := testDownloader(t, srv.URL, 5) + u, err := d.resolve("/asset") + require.NoError(t, err) + content, err := d.download(u) + if tc.want == nil { + require.NoError(t, err) + assert.Equal(t, tc.body, string(content)) + } else if errors.Is(tc.want, errAssetTooLarge) { + assert.ErrorIs(t, err, errAssetTooLarge) + } else { + require.ErrorContains(t, err, tc.want.Error()) + } + }) + } +} + +func TestDownloadTruncatedBody(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Length", "5") + _, _ = w.Write([]byte("123")) + })) + defer srv.Close() + d := testDownloader(t, srv.URL, 5) + u, err := d.resolve("/asset") + require.NoError(t, err) + _, err = d.download(u) + require.ErrorContains(t, err, "unexpected EOF") +} + +func TestAssetURLs(t *testing.T) { + d := testDownloader(t, "https://gitlab.example.com/subpath/", 10) + u, err := d.resolve("uploads/asset?token=sensitive#fragment") + require.NoError(t, err) + assert.Equal(t, "https://gitlab.example.com/subpath/uploads/asset", displayURL(u)) + for _, raw := range []string{"", "ftp://example.com/file", "file:///etc/passwd", "https://user:password@example.com/file", "%invalid"} { + _, err := d.resolve(raw) + require.Error(t, err, raw) + assert.NotContains(t, err.Error(), "password") + } + for _, tc := range []struct { + target string + same bool + }{ + {"https://GITLAB.example.com:443/other", true}, + {"https://gitlab.example.com:8443/other", false}, + {"https://sub.gitlab.example.com/file", false}, + {"http://gitlab.example.com/file", false}, + } { + target, err := url.Parse(tc.target) + require.NoError(t, err) + assert.Equal(t, tc.same, sameOrigin(d.base, target)) + } +} + +func TestRedirectPolicy(t *testing.T) { + d := testDownloader(t, "https://gitlab.example.com", 10) + old, err := http.NewRequest(http.MethodGet, "https://gitlab.example.com/asset", nil) + require.NoError(t, err) + for _, raw := range []string{"http://example.com/file", "ftp://example.com/file"} { + req, err := http.NewRequest(http.MethodGet, raw, nil) + require.NoError(t, err) + require.Error(t, d.client.CheckRedirect(req, []*http.Request{old})) + } + req, err := http.NewRequest(http.MethodGet, "https://other.example.com/file", nil) + require.NoError(t, err) + req.Header.Set("PRIVATE-TOKEN", "secret") + req.Header.Set("Authorization", "secret") + req.Header.Set("Cookie", "secret") + require.NoError(t, d.client.CheckRedirect(req, []*http.Request{old})) + assert.Empty(t, req.Header) + require.Error(t, d.client.CheckRedirect(req, make([]*http.Request, 10))) +} + +func TestDownloadErrorRedactsQuery(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "/loop?signature=sensitive", http.StatusFound) + })) + defer srv.Close() + d := testDownloader(t, srv.URL, 10) + u, err := d.resolve("/loop?signature=sensitive") + require.NoError(t, err) + _, err = d.download(u) + require.Error(t, err) + assert.NotContains(t, err.Error(), "sensitive") + assert.Contains(t, err.Error(), "asset request failed") +} + +func TestDownloadMalformedRedirectRedactsQuery(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Location", "http://%invalid?signature=sensitive") + w.WriteHeader(http.StatusFound) + })) + defer srv.Close() + d := testDownloader(t, srv.URL, 10) + u, err := d.resolve("/asset") + require.NoError(t, err) + _, err = d.download(u) + require.Error(t, err) + assert.NotContains(t, err.Error(), "sensitive") +} + +func TestDownloadDoesNotReadOversizedBody(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Length", "1000000000") + _, _ = fmt.Fprint(w, strings.Repeat("x", 100)) + })) + defer srv.Close() + d := testDownloader(t, srv.URL, 10) + u, err := d.resolve("/large") + require.NoError(t, err) + _, err = d.download(u) + require.ErrorIs(t, err, errAssetTooLarge) +} diff --git a/pkg/gitlab/release/scan/scanner.go b/pkg/gitlab/release/scan/scanner.go new file mode 100644 index 00000000..20574450 --- /dev/null +++ b/pkg/gitlab/release/scan/scanner.go @@ -0,0 +1,297 @@ +package scan + +import ( + "errors" + "fmt" + "math" + "net/url" + "path" + "sync" + "sync/atomic" + "time" + + "github.com/CompassSecurity/pipeleek/pkg/config" + "github.com/CompassSecurity/pipeleek/pkg/gitlab/util" + "github.com/CompassSecurity/pipeleek/pkg/logging" + artifactproc "github.com/CompassSecurity/pipeleek/pkg/scan/artifact" + "github.com/CompassSecurity/pipeleek/pkg/scan/result" + "github.com/CompassSecurity/pipeleek/pkg/scan/runner" + "github.com/CompassSecurity/pipeleek/pkg/scanner" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" + gitlab "gitlab.com/gitlab-org/api/client-go" +) + +type Options struct { + URL string + Token string + Repo string + Namespace string + Search string + Owned bool + Member bool + Tag string + ReleaseLimit int + IncludeSources bool + MaxBytes int64 + Threads int + SecretsVerification bool + Confidence []string + HitTimeout time.Duration +} + +func (o Options) Validate() error { + if err := config.ValidateURL(o.URL, "GitLab URL"); err != nil { + return err + } + if err := config.ValidateToken(o.Token, "GitLab API Token"); err != nil { + return err + } + if err := config.ValidateThreadCount(o.Threads); err != nil { + return err + } + switch { + case o.Repo != "" && o.Namespace != "": + return errors.New("--repo and --namespace are mutually exclusive") + case o.Tag != "" && o.Repo == "": + return errors.New("--tag requires --repo") + case o.Tag != "" && o.ReleaseLimit != 0: + return errors.New("--tag and a nonzero --release-limit are mutually exclusive") + case o.ReleaseLimit < 0: + return errors.New("--release-limit must be nonnegative") + case o.MaxBytes <= 0 || o.MaxBytes == math.MaxInt64: + return errors.New("--max-artifact-size must be positive and smaller than 8 EiB") + case o.HitTimeout <= 0: + return errors.New("--hit-timeout must be positive") + } + return nil +} + +type Scanner struct { + options Options + projects atomic.Int64 + releases atomic.Int64 + scanned atomic.Int64 + skipped atomic.Int64 + failures atomic.Int64 +} + +var _ scanner.BaseScanner = (*Scanner)(nil) + +func NewScanner(opts Options) *Scanner { + return &Scanner{options: opts} +} + +func (s *Scanner) Status() *zerolog.Event { + return log.Info(). + Int64("projects", s.projects.Load()). + Int64("releases", s.releases.Load()). + Int64("scannedAssets", s.scanned.Load()). + Int64("skippedAssets", s.skipped.Load()). + Int64("failures", s.failures.Load()) +} + +type asset struct { + project string + tag string + release string + name string + rawURL string +} + +func (s *Scanner) Scan() error { + if err := s.options.Validate(); err != nil { + return err + } + git, err := util.GetGitlabClient(s.options.Token, s.options.URL) + if err != nil { + return fmt.Errorf("creating GitLab client: %w", err) + } + runner.InitScanner(s.options.Confidence) + base, err := url.Parse(s.options.URL) + if err != nil { + return errors.New("invalid GitLab URL") + } + return s.scan(git, newDownloader(base, s.options.Token, s.options.MaxBytes)) +} + +func (s *Scanner) scan(git *gitlab.Client, d *downloader) error { + work := make(chan asset, s.options.Threads) + var workers sync.WaitGroup + for range s.options.Threads { + workers.Go(func() { + for item := range work { + s.scanAsset(d, item) + } + }) + } + err := s.projectsForScan(git, func(project *gitlab.Project) error { + s.projects.Add(1) + if err := s.projectReleases(git, project, work); err != nil { + s.failures.Add(1) + log.Error().Err(err).Str("project", project.PathWithNamespace).Msg("Failed discovering release assets") + } + return nil + }) + close(work) + workers.Wait() + if err != nil { + s.failures.Add(1) + log.Error().Err(err).Msg("Failed discovering projects") + } + s.Status().Msg("Release scan finished") + if n := s.failures.Load(); n > 0 { + return fmt.Errorf("release scan incomplete: %d failure(s)", n) + } + return nil +} + +func (s *Scanner) projectsForScan(git *gitlab.Client, visit util.ProjectIteratorFunc) error { + o := s.options + if o.Repo != "" { + project, _, err := git.Projects.GetProject(o.Repo, &gitlab.GetProjectOptions{}) + if err != nil { + return fmt.Errorf("fetching repository: %w", err) + } + return visit(project) + } + if o.Namespace != "" { + group, _, err := git.Groups.GetGroup(o.Namespace, &gitlab.GetGroupOptions{}) + if err != nil { + return fmt.Errorf("fetching namespace: %w", err) + } + return util.IterateGroupProjects(git, group.ID, &gitlab.ListGroupProjectsOptions{ + ListOptions: gitlab.ListOptions{PerPage: 100, Page: 1}, + OrderBy: gitlab.Ptr("last_activity_at"), Owned: gitlab.Ptr(o.Owned), + Search: gitlab.Ptr(o.Search), WithShared: gitlab.Ptr(true), IncludeSubGroups: gitlab.Ptr(true), + }, visit) + } + return util.IterateProjects(git, &gitlab.ListProjectsOptions{ + ListOptions: gitlab.ListOptions{PerPage: 100, Page: 1}, + Owned: gitlab.Ptr(o.Owned), Membership: gitlab.Ptr(o.Member), + Search: gitlab.Ptr(o.Search), OrderBy: gitlab.Ptr("last_activity_at"), + }, visit) +} + +func (s *Scanner) projectReleases(git *gitlab.Client, project *gitlab.Project, work chan<- asset) error { + if s.options.Tag != "" { + release, _, err := git.Releases.GetRelease(project.ID, s.options.Tag) + if err != nil { + return fmt.Errorf("fetching release: %w", err) + } + if published(release) { + s.releaseAssets(git, project, release, work) + } + return nil + } + opts := &gitlab.ListReleasesOptions{ + ListOptions: gitlab.ListOptions{PerPage: 100, Page: 1}, + OrderBy: gitlab.Ptr("released_at"), Sort: gitlab.Ptr("desc"), + } + count := 0 + for { + releases, resp, err := git.Releases.ListReleases(project.ID, opts) + if err != nil { + return fmt.Errorf("listing releases: %w", err) + } + for _, release := range releases { + if !published(release) { + continue + } + s.releaseAssets(git, project, release, work) + count++ + if s.options.ReleaseLimit > 0 && count >= s.options.ReleaseLimit { + return nil + } + } + if resp == nil || resp.NextPage == 0 { + return nil + } + opts.Page = resp.NextPage + } +} + +func published(release *gitlab.Release) bool { + return !release.UpcomingRelease && (release.ReleasedAt == nil || !release.ReleasedAt.After(time.Now())) +} + +func (s *Scanner) releaseAssets(git *gitlab.Client, project *gitlab.Project, release *gitlab.Release, work chan<- asset) { + s.releases.Add(1) + opts := &gitlab.ListReleaseLinksOptions{ListOptions: gitlab.ListOptions{PerPage: 100, Page: 1}} + for { + links, resp, err := git.ReleaseLinks.ListReleaseLinks(project.ID, release.TagName, opts) + if err != nil { + s.failures.Add(1) + log.Error().Err(err).Str("project", project.PathWithNamespace). + Str("tag", release.TagName).Msg("Failed listing release links") + break + } + for _, link := range links { + work <- asset{project.PathWithNamespace, release.TagName, release.Name, link.Name, link.URL} + } + if resp == nil || resp.NextPage == 0 { + break + } + opts.Page = resp.NextPage + } + if s.options.IncludeSources { + for _, source := range release.Assets.Sources { + if source.Format == "zip" { + work <- asset{project.PathWithNamespace, release.TagName, release.Name, release.TagName + ".zip", source.URL} + return + } + } + s.failures.Add(1) + log.Error().Str("project", project.PathWithNamespace).Str("tag", release.TagName). + Msg("Source ZIP unavailable for release") + } +} + +func (s *Scanner) scanAsset(d *downloader, item asset) { + u, err := d.resolve(item.rawURL) + if err != nil { + s.assetFailure(item, err) + return + } + item.rawURL = displayURL(u) + filename := path.Base(u.Path) + if filename == "." || filename == "/" { + filename = item.name + } + if item.name == "" { + item.name = filename + } + content, err := d.download(u) + if err == nil { + err = artifactproc.ProcessReleaseAsset(content, filename, artifactproc.ReleaseProcessOptions{ + MaxBytes: s.options.MaxBytes, VerifyCredentials: s.options.SecretsVerification, + HitTimeout: s.options.HitTimeout, GitLabURL: s.options.URL, + ReportFinding: func(finding scanner.Finding, file, archive string) { + sourceType := logging.SecretTypeArchive + if archive != "" { + sourceType = logging.SecretTypeNestedArchive + } + result.ReportFindingWithCustomFields(finding, map[string]string{ + "type": string(sourceType), "url": item.rawURL, "project": item.project, + "tag": item.tag, "release": item.release, "asset": item.name, + "file": file, "archive": archive, + }) + }, + }) + } + if errors.Is(err, errAssetTooLarge) || errors.Is(err, artifactproc.ErrReleaseAssetTooLarge) { + s.skipped.Add(1) + log.Warn().Str("project", item.project).Str("tag", item.tag).Str("asset", item.name). + Msg("Skipped release asset exceeding maximum size") + } else if err != nil { + s.assetFailure(item, err) + } else { + s.scanned.Add(1) + } +} + +func (s *Scanner) assetFailure(item asset, err error) { + s.failures.Add(1) + log.Error().Err(err).Str("project", item.project).Str("tag", item.tag). + Str("asset", item.name).Msg("Failed scanning release asset") +} diff --git a/pkg/gitlab/release/scan/scanner_test.go b/pkg/gitlab/release/scan/scanner_test.go new file mode 100644 index 00000000..14b732b8 --- /dev/null +++ b/pkg/gitlab/release/scan/scanner_test.go @@ -0,0 +1,226 @@ +package scan + +import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/CompassSecurity/pipeleek/pkg/gitlab/util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + gitlab "gitlab.com/gitlab-org/api/client-go" +) + +func validOptions() Options { + return Options{ + URL: "https://gitlab.example.com", Token: "test-token", + Threads: 2, MaxBytes: 1024, HitTimeout: time.Second, + } +} + +func TestOptionsValidation(t *testing.T) { + for _, tc := range []struct { + name string + change func(*Options) + }{ + {"url", func(o *Options) { o.URL = "file:///tmp" }}, + {"token", func(o *Options) { o.Token = "" }}, + {"threads", func(o *Options) { o.Threads = 0 }}, + {"size", func(o *Options) { o.MaxBytes = 0 }}, + {"timeout", func(o *Options) { o.HitTimeout = -1 }}, + {"limit", func(o *Options) { o.ReleaseLimit = -1 }}, + {"tag-requires-repo", func(o *Options) { o.Tag = "v1" }}, + {"tag-limit", func(o *Options) { o.Repo, o.Tag, o.ReleaseLimit = "g/p", "v1", 1 }}, + {"repo-namespace", func(o *Options) { o.Repo, o.Namespace = "g/p", "g" }}, + } { + t.Run(tc.name, func(t *testing.T) { + o := validOptions() + tc.change(&o) + require.Error(t, o.Validate()) + }) + } + require.NoError(t, validOptions().Validate()) +} + +func TestReleasePaginationAndLimit(t *testing.T) { + for _, limit := range []int{0, 1, 2} { + t.Run(fmt.Sprint(limit), func(t *testing.T) { + var releasePages, linkPages atomic.Int64 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/api/v4/projects/1/releases": + releasePages.Add(1) + assert.Equal(t, "released_at", r.URL.Query().Get("order_by")) + assert.Equal(t, "desc", r.URL.Query().Get("sort")) + if r.URL.Query().Get("page") == "2" { + _, _ = fmt.Fprint(w, `[{"tag_name":"v1"}]`) + } else { + w.Header().Set("X-Next-Page", "2") + _, _ = fmt.Fprint(w, `[{"tag_name":"future","upcoming_release":true},{"tag_name":"v2"}]`) + } + case "/api/v4/projects/1/releases/v2/assets/links", "/api/v4/projects/1/releases/v1/assets/links": + linkPages.Add(1) + if r.URL.Query().Get("page") != "2" { + w.Header().Set("X-Next-Page", "2") + } + _, _ = fmt.Fprint(w, `[{"name":"binary","url":"https://assets.example.com/binary"}]`) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + git, err := util.GetGitlabClient("token", srv.URL) + require.NoError(t, err) + o := validOptions() + o.ReleaseLimit = limit + s := NewScanner(o) + work := make(chan asset, 10) + require.NoError(t, s.projectReleases(git, &gitlab.Project{ID: 1, PathWithNamespace: "g/p"}, work)) + expected := int64(2) + if limit == 1 { + expected = 1 + } + assert.Equal(t, expected, s.releases.Load()) + assert.Equal(t, expected, releasePages.Load()) + assert.Equal(t, expected*2, linkPages.Load()) + assert.Len(t, work, int(expected)*2) + }) + } +} + +func TestExactTagAndSources(t *testing.T) { + for _, sources := range []bool{false, true} { + t.Run(fmt.Sprint(sources), func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/api/v4/projects/1/releases/version/1": + assert.Contains(t, r.URL.EscapedPath(), "version%2F1") + _ = json.NewEncoder(w).Encode(gitlab.Release{ + TagName: "version/1", Name: "Release 1", + Assets: gitlab.ReleaseAssets{Sources: []gitlab.ReleaseAssetsSource{ + {Format: "tar.gz", URL: "/source.tar.gz"}, {Format: "zip", URL: "/source.zip"}, + }}, + }) + case "/api/v4/projects/1/releases/version/1/assets/links": + _, _ = fmt.Fprint(w, `[]`) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + git, err := util.GetGitlabClient("token", srv.URL) + require.NoError(t, err) + o := validOptions() + o.Repo, o.Tag, o.IncludeSources = "g/p", "version/1", sources + s := NewScanner(o) + work := make(chan asset, 5) + require.NoError(t, s.projectReleases(git, &gitlab.Project{ID: 1}, work)) + if sources { + require.Len(t, work, 1) + item := <-work + assert.Equal(t, "/source.zip", item.rawURL) + } else { + assert.Empty(t, work) + } + assert.Zero(t, s.failures.Load()) + }) + } +} + +func TestDiscoveryContinuesAfterFailure(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/api/v4/projects": + _, _ = fmt.Fprint(w, `[{"id":1,"path_with_namespace":"g/one"},{"id":2,"path_with_namespace":"g/two"}]`) + case "/api/v4/projects/1/releases": + w.WriteHeader(http.StatusForbidden) + case "/api/v4/projects/2/releases": + _, _ = fmt.Fprint(w, `[{"tag_name":"v1"}]`) + case "/api/v4/projects/2/releases/v1/assets/links": + _, _ = fmt.Fprint(w, `[]`) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + git, err := util.GetGitlabClient("token", srv.URL) + require.NoError(t, err) + o := validOptions() + s := NewScanner(o) + require.ErrorContains(t, s.scan(git, testDownloader(t, srv.URL, 100)), "incomplete") + assert.Equal(t, int64(2), s.projects.Load()) + assert.Equal(t, int64(1), s.releases.Load()) + assert.Equal(t, int64(1), s.failures.Load()) +} + +func TestPublished(t *testing.T) { + future, past := time.Now().Add(time.Hour), time.Now().Add(-time.Hour) + assert.True(t, published(&gitlab.Release{})) + assert.True(t, published(&gitlab.Release{ReleasedAt: &past})) + assert.False(t, published(&gitlab.Release{ReleasedAt: &future})) + assert.False(t, published(&gitlab.Release{UpcomingRelease: true})) +} + +func TestProjectPagination(t *testing.T) { + for _, namespace := range []string{"", "group"} { + t.Run(namespace, func(t *testing.T) { + var calls atomic.Int64 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if r.URL.Path == "/api/v4/groups/group" { + _, _ = fmt.Fprint(w, `{"id":10}`) + return + } + calls.Add(1) + if namespace == "" { + assert.Equal(t, "/api/v4/projects", r.URL.Path) + assert.Equal(t, "true", r.URL.Query().Get("membership")) + } else { + assert.Equal(t, "/api/v4/groups/10/projects", r.URL.Path) + assert.Equal(t, "true", r.URL.Query().Get("include_subgroups")) + } + assert.Equal(t, "true", r.URL.Query().Get("owned")) + assert.Equal(t, "test", r.URL.Query().Get("search")) + if r.URL.Query().Get("page") != "2" { + w.Header().Set("X-Next-Page", "2") + } + _, _ = fmt.Fprint(w, `[{"id":1}]`) + })) + defer srv.Close() + git, err := util.GetGitlabClient("token", srv.URL) + require.NoError(t, err) + o := validOptions() + o.Namespace, o.Owned, o.Member, o.Search = namespace, true, true, "test" + s := NewScanner(o) + var projects int + require.NoError(t, s.projectsForScan(git, func(p *gitlab.Project) error { + projects++ + return nil + })) + assert.Equal(t, 2, projects) + assert.Equal(t, int64(2), calls.Load()) + }) + } +} + +func TestMissingSourcesAndLinkFailure(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + })) + defer srv.Close() + git, err := util.GetGitlabClient("token", srv.URL) + require.NoError(t, err) + o := validOptions() + o.IncludeSources = true + s := NewScanner(o) + s.releaseAssets(git, &gitlab.Project{ID: 1}, &gitlab.Release{TagName: "v1"}, make(chan asset, 1)) + assert.Equal(t, int64(2), s.failures.Load()) + assert.Equal(t, int64(1), s.releases.Load()) +} diff --git a/pkg/scan/artifact/release.go b/pkg/scan/artifact/release.go new file mode 100644 index 00000000..8beafb9c --- /dev/null +++ b/pkg/scan/artifact/release.go @@ -0,0 +1,256 @@ +package artifact + +import ( + "archive/tar" + "archive/zip" + "bytes" + "compress/bzip2" + "compress/gzip" + "errors" + "fmt" + "io" + "path" + "strings" + "time" + + "github.com/CompassSecurity/pipeleek/pkg/archive" + "github.com/CompassSecurity/pipeleek/pkg/scanner" + "github.com/CompassSecurity/pipeleek/pkg/scanner/engine" + "github.com/h2non/filetype" +) + +// ErrReleaseAssetTooLarge marks a policy skip rather than a processing failure. +var ErrReleaseAssetTooLarge = errors.New("release asset exceeds byte limit") + +// ReleaseProcessOptions controls release-only processing without changing pipeline +// artifact handling. MaxBytes bounds both input size and cumulative expanded +// payloads, including intermediate nested archives and decompressed tar streams. +type ReleaseProcessOptions struct { + MaxBytes int64 + VerifyCredentials bool + HitTimeout time.Duration + GitLabURL string + // ReportFinding receives the archive-relative file path and archive names + // joined by " -> ". A standalone file has an empty archive ancestry. + ReportFinding func(scanner.Finding, string, string) +} + +type releaseProcessor struct { + opts ReleaseProcessOptions + expanded int64 + detect func([]byte, int, bool, engine.DetectionOptions) ([]scanner.Finding, error) +} + +// ProcessReleaseAsset scans content in memory; it never extracts files to disk or +// executes assets. ZIP, tar, gzip, and bzip2 are supported up to ten archive layers. +// Unsupported archives and incomplete processing return errors. +func ProcessReleaseAsset(content []byte, filename string, opts ReleaseProcessOptions) error { + if opts.MaxBytes <= 0 { + return errors.New("release asset byte limit must be positive") + } + if int64(len(content)) > opts.MaxBytes { + return ErrReleaseAssetTooLarge + } + if opts.ReportFinding == nil { + return errors.New("release asset finding reporter is required") + } + p := releaseProcessor{opts: opts, detect: engine.DetectHitsWithOptions} + return p.process(content, filename, nil, 0) +} + +func (p *releaseProcessor) readExpanded(r io.Reader) ([]byte, error) { + remaining := p.opts.MaxBytes - p.expanded + // Avoid remaining+1 overflow, while still probing for an extra byte. + content, err := io.ReadAll(io.LimitReader(r, remaining)) + p.expanded += int64(len(content)) + if err != nil { + return nil, err + } + var extra [1]byte + n, err := io.ReadFull(r, extra[:]) + if n != 0 { + return nil, ErrReleaseAssetTooLarge + } + if err != nil && !errors.Is(err, io.EOF) { + return nil, err + } + return content, nil +} + +func releaseArchiveType(content []byte, filename string) string { + kind, _ := filetype.Match(content) + switch kind.Extension { + case "zip", "tar", "gz", "bz2": + return kind.Extension + } + if filetype.IsArchive(content) { + return "unsupported" + } + name := strings.ToLower(filename) + for _, suffix := range []struct{ ext, kind string }{ + {".tar.gz", "gz"}, {".tgz", "gz"}, {".tar.bz2", "bz2"}, {".tbz2", "bz2"}, {".tbz", "bz2"}, + {".zip", "zip"}, {".tar", "tar"}, {".gzip", "gz"}, {".gz", "gz"}, {".bz2", "bz2"}, {".bzip2", "bz2"}, + {".7z", "unsupported"}, {".rar", "unsupported"}, {".xz", "unsupported"}, + {".zst", "unsupported"}, {".zstd", "unsupported"}, {".lz", "unsupported"}, + {".lzma", "unsupported"}, {".lz4", "unsupported"}, {".cab", "unsupported"}, + {".ar", "unsupported"}, {".deb", "unsupported"}, {".rpm", "unsupported"}, + {".txz", "unsupported"}, {".taz", "unsupported"}, {".z", "unsupported"}, + {".iso", "unsupported"}, {".dmg", "unsupported"}, + } { + if strings.HasSuffix(name, suffix.ext) { + return suffix.kind + } + } + return "" +} + +func (p *releaseProcessor) process(content []byte, filename string, ancestry []string, depth int) error { + kind := releaseArchiveType(content, filename) + if kind == "" { + findings, err := p.detect(archive.ExtractPrintableStrings(content, archive.MinStringLength), 1, + p.opts.VerifyCredentials, engine.DetectionOptions{ + Path: filename, GitLabURL: p.opts.GitLabURL, Timeout: p.opts.HitTimeout, + }) + if err != nil { + return fmt.Errorf("detect release asset %q: %w", filename, err) + } + for _, finding := range findings { + p.opts.ReportFinding(finding, filename, strings.Join(ancestry, " -> ")) + } + return nil + } + if depth >= 10 { + return fmt.Errorf("release archive %q exceeds maximum nesting depth 10", filename) + } + ancestry = append(append([]string(nil), ancestry...), filename) + switch kind { + case "zip": + return p.processZip(content, ancestry, depth+1) + case "tar": + return p.processTar(content, ancestry, depth+1) + case "gz", "bz2": + return p.processCompressed(content, filename, kind, ancestry, depth+1) + default: + return fmt.Errorf("unsupported release archive %q", filename) + } +} + +func releaseRelativePath(name string) (string, error) { + // Backslashes may be directory separators on a producer's platform. + name = strings.ReplaceAll(name, `\`, "/") + clean := path.Clean(name) + if strings.ContainsRune(name, '\x00') || strings.Contains(name, ":") || + path.IsAbs(name) || clean == "." || clean == ".." || strings.HasPrefix(clean, "../") { + return "", fmt.Errorf("unsafe release archive member path %q", name) + } + return clean, nil +} + +func (p *releaseProcessor) processZip(content []byte, ancestry []string, depth int) error { + zr, err := zip.NewReader(bytes.NewReader(content), int64(len(content))) + if err != nil { + return fmt.Errorf("decode release ZIP: %w", err) + } + for _, member := range zr.File { + if member.FileInfo().IsDir() && (member.Name == "./" || member.Name == ".") { + continue + } + name, err := releaseRelativePath(member.Name) + if err != nil { + return err + } + if member.FileInfo().IsDir() { + continue + } + if !member.Mode().IsRegular() { + return fmt.Errorf("unsupported release ZIP member type %q", name) + } + if member.UncompressedSize64 > uint64(p.opts.MaxBytes-p.expanded) { + return ErrReleaseAssetTooLarge + } + r, err := member.Open() + if err != nil { + return fmt.Errorf("open release ZIP member %q: %w", name, err) + } + payload, readErr := p.readExpanded(r) + closeErr := r.Close() + if readErr != nil { + return fmt.Errorf("read release ZIP member %q: %w", name, readErr) + } + if closeErr != nil { + return fmt.Errorf("close release ZIP member %q: %w", name, closeErr) + } + if err := p.process(payload, name, ancestry, depth); err != nil { + return err + } + } + return nil +} + +func (p *releaseProcessor) processTar(content []byte, ancestry []string, depth int) error { + if len(content) < 512 { + return errors.New("decode release tar: missing archive header") + } + tr := tar.NewReader(bytes.NewReader(content)) + for { + header, err := tr.Next() + if errors.Is(err, io.EOF) { + return nil + } + if err != nil { + return fmt.Errorf("decode release tar: %w", err) + } + if header.Typeflag == tar.TypeDir && (header.Name == "." || header.Name == "./") { + continue + } + name, err := releaseRelativePath(header.Name) + if err != nil { + return err + } + switch header.Typeflag { + case tar.TypeDir: + continue + case tar.TypeReg, tar.TypeRegA: + default: + return fmt.Errorf("unsupported release tar member type %q", name) + } + if header.Size > p.opts.MaxBytes-p.expanded { + return ErrReleaseAssetTooLarge + } + payload, err := p.readExpanded(tr) + if err != nil { + return fmt.Errorf("read release tar member %q: %w", name, err) + } + if err := p.process(payload, name, ancestry, depth); err != nil { + return err + } + } +} + +func (p *releaseProcessor) processCompressed(content []byte, filename, kind string, ancestry []string, depth int) error { + var r io.Reader + if kind == "gz" { + gz, err := gzip.NewReader(bytes.NewReader(content)) + if err != nil { + return fmt.Errorf("decode release gzip: %w", err) + } + defer func() { _ = gz.Close() }() + r = gz + } else { + r = bzip2.NewReader(bytes.NewReader(content)) + } + payload, err := p.readExpanded(r) + if err != nil { + return fmt.Errorf("expand release %s: %w", kind, err) + } + innerName := strings.TrimSuffix(filename, path.Ext(filename)) + lower := strings.ToLower(filename) + if strings.HasSuffix(lower, ".tgz") || strings.HasSuffix(lower, ".tbz2") || strings.HasSuffix(lower, ".tbz") { + innerName += ".tar" + } + if releaseArchiveType(payload, innerName) == "tar" { + // A compressed tar is one archive layer, with its original asset name. + return p.processTar(payload, ancestry, depth) + } + return p.process(payload, innerName, ancestry, depth) +} diff --git a/pkg/scan/artifact/release_test.go b/pkg/scan/artifact/release_test.go new file mode 100644 index 00000000..3f988f2c --- /dev/null +++ b/pkg/scan/artifact/release_test.go @@ -0,0 +1,342 @@ +package artifact + +import ( + "archive/tar" + "archive/zip" + "bytes" + "compress/gzip" + "encoding/base64" + "errors" + "fmt" + "io" + "strings" + "testing" + "time" + + "github.com/CompassSecurity/pipeleek/pkg/scanner" + "github.com/CompassSecurity/pipeleek/pkg/scanner/engine" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type releaseTestFile struct { + name string + content []byte +} + +func releaseTestZip(t *testing.T, files ...releaseTestFile) []byte { + t.Helper() + var buf bytes.Buffer + w := zip.NewWriter(&buf) + for _, file := range files { + f, err := w.Create(file.name) + require.NoError(t, err) + _, err = f.Write(file.content) + require.NoError(t, err) + } + require.NoError(t, w.Close()) + return buf.Bytes() +} + +func releaseTestTar(t *testing.T, files ...releaseTestFile) []byte { + t.Helper() + var buf bytes.Buffer + w := tar.NewWriter(&buf) + for _, file := range files { + require.NoError(t, w.WriteHeader(&tar.Header{ + Name: file.name, Mode: 0o600, Size: int64(len(file.content)), + })) + _, err := w.Write(file.content) + require.NoError(t, err) + } + require.NoError(t, w.Close()) + return buf.Bytes() +} + +func releaseTestGzip(t *testing.T, content []byte) []byte { + t.Helper() + var buf bytes.Buffer + w := gzip.NewWriter(&buf) + _, err := w.Write(content) + require.NoError(t, err) + require.NoError(t, w.Close()) + return buf.Bytes() +} + +func releaseTestOptions() ReleaseProcessOptions { + return ReleaseProcessOptions{ + MaxBytes: 1 << 20, HitTimeout: time.Minute, + ReportFinding: func(scanner.Finding, string, string) {}, + } +} + +func TestReleaseAssetMetadataAndPrintableStrings(t *testing.T) { + text := []byte("release-secret-value") + binary := append([]byte{0x7f, 'E', 'L', 'F', 0, 1}, text...) + binary = append(binary, 0, 1, 2) + inner := releaseTestGzip(t, releaseTestTar(t, releaseTestFile{"config/secret.txt", text})) + bzip, err := base64.StdEncoding.DecodeString("QlpoOTFBWSZTWYglTdEAAAaRgAACKgQfACAAIp5BiEAMKUPmg4LYhlI8LuSKcKEhEEqbog==") + require.NoError(t, err) + tests := []struct { + name, filename, wantPath, wantAncestry string + content []byte + }{ + {"plain", "notes.txt", "notes.txt", "", text}, + {"binary", "program", "program", "", binary}, + {"ZIP", "bundle.zip", "config/secret.txt", "bundle.zip", + releaseTestZip(t, releaseTestFile{"config/secret.txt", text})}, + {"tar", "bundle.tar", "config/secret.txt", "bundle.tar", + releaseTestTar(t, releaseTestFile{"config/secret.txt", text})}, + {"tar gzip", "bundle.tar.gz", "config/secret.txt", "bundle.tar.gz", inner}, + {"tgz", "bundle.tgz", "config/secret.txt", "bundle.tgz", inner}, + {"gzip", "notes.txt.gz", "notes.txt", "notes.txt.gz", releaseTestGzip(t, text)}, + {"bzip2", "notes.txt.bz2", "notes.txt", "notes.txt.bz2", bzip}, + {"nested ZIP tar gzip", "outer.zip", "config/secret.txt", "outer.zip -> packages/inner.tar.gz", + releaseTestZip(t, releaseTestFile{"packages/inner.tar.gz", inner})}, + {"nested ZIP", "outer.zip", "config/secret.txt", "outer.zip -> packages/inner.zip", + releaseTestZip(t, releaseTestFile{"packages/inner.zip", + releaseTestZip(t, releaseTestFile{"config/secret.txt", text})})}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + opts := releaseTestOptions() + opts.VerifyCredentials = true + opts.GitLabURL = "https://gitlab.example.test" + var reported int + finding := scanner.Finding{Text: string(text)} + opts.ReportFinding = func(actual scanner.Finding, file, ancestry string) { + reported++ + assert.Equal(t, finding, actual) + assert.Equal(t, tt.wantPath, file) + assert.Equal(t, tt.wantAncestry, ancestry) + } + p := releaseProcessor{opts: opts, detect: func(content []byte, threads int, verify bool, options engine.DetectionOptions) ([]scanner.Finding, error) { + assert.Contains(t, string(content), string(text)) + assert.NotContains(t, string(content), "\x00") + assert.Equal(t, 1, threads) + assert.True(t, verify) + assert.Equal(t, tt.wantPath, options.Path) + assert.Equal(t, opts.HitTimeout, options.Timeout) + assert.Equal(t, opts.GitLabURL, options.GitLabURL) + return []scanner.Finding{finding}, nil + }} + require.NoError(t, p.process(tt.content, tt.filename, nil, 0)) + assert.Equal(t, 1, reported) + }) + } +} + +func TestReleaseAssetByteLimits(t *testing.T) { + plain := []byte("plain release text") + opts := releaseTestOptions() + opts.MaxBytes = int64(len(plain)) + require.NoError(t, ProcessReleaseAsset(plain, "plain.txt", opts)) + opts.MaxBytes-- + require.ErrorIs(t, ProcessReleaseAsset(plain, "plain.txt", opts), ErrReleaseAssetTooLarge) + opts.MaxBytes = 0 + require.ErrorContains(t, ProcessReleaseAsset(nil, "plain.txt", opts), "positive") + opts.MaxBytes = -1 + require.ErrorContains(t, ProcessReleaseAsset(nil, "plain.txt", opts), "positive") + + // Use compressible data so the expanded limit, not the input limit, is hit. + payload := bytes.Repeat([]byte("harmless content\n"), 512) + for _, kind := range []string{"zip", "gz"} { + t.Run(kind, func(t *testing.T) { + var content []byte + if kind == "zip" { + content = releaseTestZip(t, releaseTestFile{"one.txt", payload}) + } else { + content = releaseTestGzip(t, payload) + } + opts := releaseTestOptions() + opts.MaxBytes = int64(len(payload)) + require.NoError(t, ProcessReleaseAsset(content, "asset."+kind, opts)) + opts.MaxBytes-- + require.ErrorIs(t, ProcessReleaseAsset(content, "asset."+kind, opts), ErrReleaseAssetTooLarge) + }) + } + t.Run("cumulative siblings", func(t *testing.T) { + content := releaseTestZip(t, releaseTestFile{"one.txt", payload}, releaseTestFile{"two.txt", payload}) + opts := releaseTestOptions() + opts.MaxBytes = int64(2 * len(payload)) + require.NoError(t, ProcessReleaseAsset(content, "asset.zip", opts)) + opts.MaxBytes-- + require.ErrorIs(t, ProcessReleaseAsset(content, "asset.zip", opts), ErrReleaseAssetTooLarge) + }) + t.Run("cumulative nested archives", func(t *testing.T) { + inner := releaseTestZip(t, releaseTestFile{"one.txt", payload}) + content := releaseTestZip(t, releaseTestFile{"inner.zip", inner}) + opts := releaseTestOptions() + opts.MaxBytes = int64(len(inner) + len(payload)) + require.NoError(t, ProcessReleaseAsset(content, "outer.zip", opts)) + opts.MaxBytes-- + require.ErrorIs(t, ProcessReleaseAsset(content, "outer.zip", opts), ErrReleaseAssetTooLarge) + }) + t.Run("tar gzip includes intermediate stream", func(t *testing.T) { + inner := releaseTestTar(t, releaseTestFile{"one.txt", payload}) + content := releaseTestGzip(t, inner) + opts := releaseTestOptions() + opts.MaxBytes = int64(len(inner) + len(payload)) + require.NoError(t, ProcessReleaseAsset(content, "asset.tar.gz", opts)) + opts.MaxBytes-- + require.ErrorIs(t, ProcessReleaseAsset(content, "asset.tar.gz", opts), ErrReleaseAssetTooLarge) + }) +} + +func TestReleaseAssetDepth(t *testing.T) { + content := []byte("harmless release text") + for depth := 1; depth <= 11; depth++ { + name := "inner.txt" + if depth > 1 { + name = "inner.zip" + } + content = releaseTestZip(t, releaseTestFile{name, content}) + err := ProcessReleaseAsset(content, "outer.zip", releaseTestOptions()) + if depth <= 10 { + require.NoError(t, err, "depth %d", depth) + } else { + require.ErrorContains(t, err, "maximum nesting depth") + require.NotErrorIs(t, err, ErrReleaseAssetTooLarge) + } + } +} + +func TestReleaseAssetErrors(t *testing.T) { + goodGzip := releaseTestGzip(t, []byte("release content")) + corruptGzip := bytes.Clone(goodGzip) + corruptGzip[len(corruptGzip)-1] ^= 0xff + badTar := releaseTestTar(t, releaseTestFile{"file.txt", []byte("content")}) + badTar[0] ^= 0xff + truncatedTar := releaseTestTar(t, releaseTestFile{"file.txt", bytes.Repeat([]byte("a"), 1024)})[:600] + innerBadZip := releaseTestZip(t, releaseTestFile{"inner.zip", []byte("invalid ZIP")}) + badZip := releaseTestZip(t, releaseTestFile{"text.txt", []byte("content")}) + zr, err := zip.NewReader(bytes.NewReader(badZip), int64(len(badZip))) + require.NoError(t, err) + offset, err := zr.File[0].DataOffset() + require.NoError(t, err) + badZip[offset] ^= 0xff + for _, tt := range []struct { + name string + content []byte + }{ + {"broken.zip", []byte("not a zip")}, + {"corrupt.zip", badZip}, + {"broken.tar", []byte("not a tar")}, + {"empty.tar", nil}, + {"badheader.tar", badTar}, + {"truncated.tar", truncatedTar}, + {"broken.gz", []byte("not a gzip")}, + {"checksum.gz", corruptGzip}, + {"truncated.gz", goodGzip[:len(goodGzip)-3]}, + {"broken.bz2", []byte("not bzip2")}, + {"unsupported.7z", []byte("unsupported archive")}, + {"unsupported.xz", []byte("unsupported archive")}, + {"unsupported.rar", []byte("unsupported archive")}, + {"unsupported-without-extension", []byte("7z\xbc\xaf\x27\x1c")}, + {"nested.zip", innerBadZip}, + } { + t.Run(tt.name, func(t *testing.T) { + err := ProcessReleaseAsset(tt.content, tt.name, releaseTestOptions()) + require.Error(t, err) + require.NotErrorIs(t, err, ErrReleaseAssetTooLarge) + }) + } + t.Run("detection timeout", func(t *testing.T) { + opts := releaseTestOptions() + opts.HitTimeout = -time.Second + require.ErrorContains(t, ProcessReleaseAsset([]byte("some content"), "notes.txt", opts), "timed out") + }) + t.Run("detection error propagation", func(t *testing.T) { + want := errors.New("detection failed") + p := releaseProcessor{opts: releaseTestOptions(), detect: func([]byte, int, bool, engine.DetectionOptions) ([]scanner.Finding, error) { + return nil, want + }} + require.ErrorIs(t, p.process([]byte("content"), "notes.txt", nil, 0), want) + }) + t.Run("missing reporter", func(t *testing.T) { + opts := releaseTestOptions() + opts.ReportFinding = nil + require.ErrorContains(t, ProcessReleaseAsset(nil, "empty.txt", opts), "reporter") + }) +} + +func TestReleaseExpandedStreamingLimit(t *testing.T) { + p := releaseProcessor{opts: releaseTestOptions()} + p.opts.MaxBytes = 16 + r := bytes.NewReader(bytes.Repeat([]byte("x"), 1024)) + _, err := p.readExpanded(r) + require.ErrorIs(t, err, ErrReleaseAssetTooLarge) + require.Equal(t, int64(17), r.Size()-int64(r.Len()), "consume at most the budget plus one probe byte") + require.Equal(t, int64(16), p.expanded) + + p = releaseProcessor{opts: releaseTestOptions()} + p.opts.MaxBytes = 4 + content, err := p.readExpanded(strings.NewReader("1234")) + require.NoError(t, err) + require.Equal(t, "1234", string(content)) + content, err = p.readExpanded(strings.NewReader("")) + require.NoError(t, err) + require.Empty(t, content) + _, err = p.readExpanded(strings.NewReader("x")) + require.ErrorIs(t, err, ErrReleaseAssetTooLarge) + + p = releaseProcessor{opts: releaseTestOptions()} + _, err = p.readExpanded(io.MultiReader(strings.NewReader("data"), releaseErrorReader{})) + require.ErrorIs(t, err, io.ErrUnexpectedEOF) +} + +type releaseErrorReader struct{} + +func (releaseErrorReader) Read([]byte) (int, error) { + return 0, io.ErrUnexpectedEOF +} + +func TestReleaseEmptyArchivesAndDirectories(t *testing.T) { + for _, tt := range []struct { + name string + content []byte + }{ + {"empty.zip", releaseTestZip(t)}, + {"empty.tar", releaseTestTar(t)}, + {"root.zip", releaseTestZip(t, releaseTestFile{"./", nil}, releaseTestFile{"folder/", nil})}, + } { + t.Run(tt.name, func(t *testing.T) { + require.NoError(t, ProcessReleaseAsset(tt.content, tt.name, releaseTestOptions())) + }) + } +} + +func TestReleaseAssetUnsafeMembers(t *testing.T) { + for _, name := range []string{"../secret.txt", "/secret.txt", `C:\secret.txt`, `..\secret.txt`} { + t.Run(fmt.Sprintf("ZIP/%s", name), func(t *testing.T) { + content := releaseTestZip(t, releaseTestFile{name, []byte("secret")}) + require.ErrorContains(t, ProcessReleaseAsset(content, "outer.zip", releaseTestOptions()), "unsafe") + }) + t.Run(fmt.Sprintf("tar/%s", name), func(t *testing.T) { + content := releaseTestTar(t, releaseTestFile{name, []byte("secret")}) + require.ErrorContains(t, ProcessReleaseAsset(content, "outer.tar", releaseTestOptions()), "unsafe") + }) + } + t.Run("tar symlink", func(t *testing.T) { + var buf bytes.Buffer + w := tar.NewWriter(&buf) + require.NoError(t, w.WriteHeader(&tar.Header{Name: "link", Typeflag: tar.TypeSymlink, Linkname: "/etc/passwd"})) + require.NoError(t, w.Close()) + require.ErrorContains(t, ProcessReleaseAsset(buf.Bytes(), "outer.tar", releaseTestOptions()), "unsupported") + }) +} + +func TestReleaseAssetRealEngineFinding(t *testing.T) { + opts := releaseTestOptions() + var found bool + opts.ReportFinding = func(f scanner.Finding, filename, ancestry string) { + if strings.Contains(f.Text, "glpat-zxywvutsrqponmlkjihg") { + found = true + assert.Equal(t, "secrets/config.txt", filename) + assert.Equal(t, "release.zip", ancestry) + } + } + content := releaseTestZip(t, releaseTestFile{"secrets/config.txt", []byte("GITLAB_TOKEN=glpat-zxywvutsrqponmlkjihg")}) + require.NoError(t, ProcessReleaseAsset(content, "release.zip", opts)) + require.True(t, found, "public processor must invoke the real detection engine and reporter") +} diff --git a/tests/e2e/gitlab/release/scan_test.go b/tests/e2e/gitlab/release/scan_test.go new file mode 100644 index 00000000..766a9fe3 --- /dev/null +++ b/tests/e2e/gitlab/release/scan_test.go @@ -0,0 +1,399 @@ +package release_test + +import ( + "archive/tar" + "archive/zip" + "bufio" + "bytes" + "compress/gzip" + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "os" + "os/exec" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/CompassSecurity/pipeleek/tests/e2e/internal/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type fixture struct { + server *httptest.Server + requests func() []testutil.RecordedRequest +} + +func newFixture(t *testing.T, links []map[string]any, assetHandler http.HandlerFunc) fixture { + t.Helper() + testutil.UseTestRules(t) + srv, requests, cleanup := testutil.StartMockServer(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case r.URL.Path == "/api/v4/projects", r.URL.Path == "/api/v4/groups/10/projects": + _, _ = fmt.Fprint(w, `[{"id":1,"path_with_namespace":"group/project"}]`) + case strings.HasPrefix(r.URL.Path, "/api/v4/projects/group"): + _, _ = fmt.Fprint(w, `{"id":1,"path_with_namespace":"group/project"}`) + case r.URL.Path == "/api/v4/groups/group": + _, _ = fmt.Fprint(w, `{"id":10}`) + case strings.HasSuffix(r.URL.Path, "/assets/links"): + _ = json.NewEncoder(w).Encode(links) + case r.URL.Path == "/api/v4/projects/1/releases": + _, _ = fmt.Fprint(w, `[{"tag_name":"v2","name":"Release 2","assets":{"sources":[{"format":"zip","url":"/source.zip"},{"format":"tar.gz","url":"/source.tar.gz"}]}},{"tag_name":"v1","name":"Release 1"}]`) + case r.URL.Path == "/api/v4/projects/1/releases/version/1": + _, _ = fmt.Fprint(w, `{"tag_name":"version/1","name":"Exact release"}`) + default: + assetHandler(w, r) + } + }) + t.Cleanup(cleanup) + return fixture{srv, requests} +} + +func (f fixture) run(t *testing.T, extra ...string) (string, error) { + t.Helper() + args := append([]string{"gl", "release", "scan", "--url", f.server.URL, "--token", "test-token", + "--secrets-verification=false", "--json"}, extra...) + stdout, stderr, err := testutil.RunCLI(t, args, nil, 60*time.Second) + return stdout + stderr, err +} + +func links(urls ...string) []map[string]any { + result := make([]map[string]any, 0, len(urls)) + for i, u := range urls { + result = append(result, map[string]any{"id": i + 1, "name": fmt.Sprintf("asset-%d", i), "url": u}) + } + return result +} + +func plainAsset(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain") + _, _ = fmt.Fprint(w, "DATABASE_PASSWORD=releasefixture\n") +} + +func TestReleaseScanDefaultAndProjectFilters(t *testing.T) { + for _, tc := range []struct { + name string + args []string + path string + want map[string]string + }{ + {"all", nil, "/api/v4/projects", map[string]string{}}, + {"repo", []string{"--repo", "group/project"}, "/api/v4/projects/group/project", nil}, + {"filters", []string{"--owned", "--member", "--search", "project"}, "/api/v4/projects", + map[string]string{"owned": "true", "membership": "true", "search": "project"}}, + {"namespace", []string{"--namespace", "group", "--owned", "--search", "project"}, "/api/v4/groups/10/projects", + map[string]string{"include_subgroups": "true", "owned": "true", "search": "project"}}, + } { + t.Run(tc.name, func(t *testing.T) { + f := newFixture(t, links("/asset"), plainAsset) + output, err := f.run(t, tc.args...) + require.NoError(t, err, output) + assert.Contains(t, output, `"level":"hit"`) + assert.Contains(t, output, `"project":"group/project"`) + for _, line := range strings.Split(strings.TrimSpace(output), "\n") { + var event struct { + Level string `json:"level"` + Tag string `json:"tag"` + Release string `json:"release"` + } + require.NoError(t, json.Unmarshal([]byte(line), &event)) + if event.Level == "hit" { + assert.Contains(t, []string{"v1", "v2"}, event.Tag) + assert.Equal(t, "Release "+strings.TrimPrefix(event.Tag, "v"), event.Release) + } + } + assert.Contains(t, output, `"asset":"asset-0"`) + assert.Contains(t, output, `"scannedAssets":2`) + var matched bool + for _, req := range f.requests() { + if req.Path == tc.path { + matched = true + assert.Equal(t, "test-token", req.Headers.Get("Private-Token")) + for k, v := range tc.want { + assert.Contains(t, req.RawQuery, k+"="+v) + } + } + assert.NotEqual(t, "/source.zip", req.Path) + } + assert.True(t, matched, tc.path) + }) + } +} + +func TestReleaseScanExactTagAndLimit(t *testing.T) { + for _, tc := range []struct { + name string + args []string + tag string + }{ + {"limit", []string{"--release-limit", "1"}, "v2"}, + {"tag", []string{"--repo", "group/project", "--tag", "version/1"}, "version/1"}, + } { + t.Run(tc.name, func(t *testing.T) { + f := newFixture(t, links("/asset"), plainAsset) + output, err := f.run(t, tc.args...) + require.NoError(t, err, output) + assert.Contains(t, output, `"tag":"`+tc.tag+`"`) + assert.Contains(t, output, `"scannedAssets":1`) + for _, req := range f.requests() { + assert.NotContains(t, req.Path, "/v1/") + if tc.name == "tag" { + assert.NotEqual(t, "/api/v4/projects/1/releases", req.Path) + } + } + }) + } +} + +func zipAsset(t *testing.T, name string, content []byte) []byte { + t.Helper() + var buf bytes.Buffer + z := zip.NewWriter(&buf) + entry, err := z.Create(name) + require.NoError(t, err) + _, err = entry.Write(content) + require.NoError(t, err) + require.NoError(t, z.Close()) + return buf.Bytes() +} + +func TestReleaseScanSourcesAndNestedArchives(t *testing.T) { + nested := zipAsset(t, "nested.zip", zipAsset(t, "config.env", []byte("DATABASE_PASSWORD=nestedfixture\n"))) + f := newFixture(t, links("/nested.zip"), func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/nested.zip": + _, _ = w.Write(nested) + case "/source.zip": + _, _ = w.Write(zipAsset(t, "project/config.env", []byte("JWT_SECRET=sourcefixture\n"))) + default: + t.Errorf("unexpected request: %s", r.URL.Path) + http.NotFound(w, r) + } + }) + output, err := f.run(t, "--release-limit", "1", "--include-sources") + require.NoError(t, err, output) + assert.Contains(t, output, "nestedfixture") + assert.Contains(t, output, "sourcefixture") + assert.Contains(t, output, `"file":"project/config.env"`) + assert.Contains(t, output, `"tag":"v2"`) + assert.Contains(t, output, `"scannedAssets":2`) + var sources int + for _, req := range f.requests() { + if req.Path == "/source.zip" { + sources++ + } + assert.NotEqual(t, "/source.tar.gz", req.Path) + } + assert.Equal(t, 1, sources) +} + +func TestReleaseScanTarAndBinary(t *testing.T) { + var archive bytes.Buffer + gz := gzip.NewWriter(&archive) + tarWriter := tar.NewWriter(gz) + text := []byte("ENCRYPTION_KEY=tarfixture\n") + require.NoError(t, tarWriter.WriteHeader(&tar.Header{Name: "config/secret.env", Size: int64(len(text)), Mode: 0600})) + _, err := tarWriter.Write(text) + require.NoError(t, err) + require.NoError(t, tarWriter.Close()) + require.NoError(t, gz.Close()) + f := newFixture(t, links("/archive.tar.gz", "/binary"), func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/binary" { + _, _ = w.Write(append([]byte{0x7f, 'E', 'L', 'F', 0, 1, 0}, []byte("DATABASE_PASSWORD=binaryfixture\n\x00")...)) + } else { + _, _ = w.Write(archive.Bytes()) + } + }) + output, err := f.run(t, "--release-limit", "1") + require.NoError(t, err, output) + assert.Contains(t, output, "tarfixture") + assert.Contains(t, output, "binaryfixture") + assert.Contains(t, output, `"file":"config/secret.env"`) +} + +func TestReleaseScanExternalRedirectDoesNotLeakCredentials(t *testing.T) { + var externalRequests atomic.Int64 + external := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + externalRequests.Add(1) + assert.Empty(t, r.Header.Get("Private-Token")) + assert.Empty(t, r.Header.Get("Authorization")) + assert.Empty(t, r.Header.Get("Cookie")) + plainAsset(w, r) + })) + defer external.Close() + f := newFixture(t, links(external.URL+"/direct", "/redirect?signature=do-not-log"), func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "test-token", r.Header.Get("Private-Token")) + http.Redirect(w, r, external.URL+"/redirected?signature=do-not-log", http.StatusFound) + }) + output, err := f.run(t, "--release-limit", "1") + require.NoError(t, err, output) + assert.Equal(t, int64(2), externalRequests.Load()) + assert.Contains(t, output, `"scannedAssets":2`) + assert.NotContains(t, output, "do-not-log") +} + +func TestReleaseScanPartialFailures(t *testing.T) { + f := newFixture(t, links("/denied", "/good"), func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/denied" { + w.WriteHeader(http.StatusForbidden) + } else { + plainAsset(w, r) + } + }) + output, err := f.run(t, "--release-limit", "1") + require.Error(t, err) + assert.Contains(t, output, "releasefixture") + assert.Contains(t, output, `"failures":1`) + assert.Contains(t, output, `"scannedAssets":1`) + assert.Contains(t, output, "incomplete") +} + +func TestReleaseScanSizeAndTimeout(t *testing.T) { + for _, tc := range []struct { + name string + args []string + fail bool + want string + }{ + {"size", []string{"--max-artifact-size", "1B"}, false, `"skippedAssets":1`}, + {"timeout", []string{"--hit-timeout", "1ns"}, true, "timed out"}, + {"confidence", []string{"--confidence", "low"}, false, `"scannedAssets":1`}, + } { + t.Run(tc.name, func(t *testing.T) { + f := newFixture(t, links("/asset"), plainAsset) + args := append([]string{"--release-limit", "1"}, tc.args...) + output, err := f.run(t, args...) + if tc.fail { + require.Error(t, err, output) + } else { + require.NoError(t, err, output) + } + assert.Contains(t, output, tc.want) + if tc.name == "size" { + assert.NotContains(t, output, `"level":"hit"`) + } + if tc.name == "confidence" { + assert.NotContains(t, output, `"confidence":"medium"`) + assert.Contains(t, output, `"confidence":"low"`) + } + }) + } +} + +func TestReleaseScanWorkerBound(t *testing.T) { + var active, peak atomic.Int64 + f := newFixture(t, links("/one", "/two", "/three", "/four", "/five"), func(w http.ResponseWriter, r *http.Request) { + n := active.Add(1) + for old := peak.Load(); n > old && !peak.CompareAndSwap(old, n); old = peak.Load() { + } + time.Sleep(40 * time.Millisecond) + active.Add(-1) + plainAsset(w, r) + }) + output, err := f.run(t, "--release-limit", "1", "--threads", "2") + require.NoError(t, err, output) + assert.Equal(t, int64(2), peak.Load()) + assert.Contains(t, output, `"scannedAssets":5`) +} + +func TestReleaseScanVerificationDisabled(t *testing.T) { + f := newFixture(t, links("/asset"), func(w http.ResponseWriter, r *http.Request) { + assert.NotEqual(t, "/api/v4/user", r.URL.Path) + _, _ = fmt.Fprint(w, "glpat-abcdefghijklmnopqrst") + }) + output, err := f.run(t, "--release-limit", "1", "--secrets-verification=false") + require.NoError(t, err, output) + assert.Contains(t, output, `"ruleName":"Gitlab - Personal Access Token v2"`) + assert.Contains(t, output, `"confidence":"high"`) + for _, req := range f.requests() { + assert.NotEqual(t, "/api/v4/user", req.Path) + } +} + +func TestReleaseScanInvalidConfiguration(t *testing.T) { + for _, args := range [][]string{ + {"--tag", "v1"}, {"--release-limit", "-1"}, {"--threads", "0"}, {"--max-artifact-size", "0B"}, + {"--hit-timeout", "0s"}, {"--repo", "group/project", "--namespace", "group"}, + {"--repo", "group/project", "--tag", "v1", "--release-limit", "1"}, + } { + t.Run(strings.Join(args, " "), func(t *testing.T) { + f := newFixture(t, nil, plainAsset) + output, err := f.run(t, args...) + require.Error(t, err) + assert.Contains(t, output, "Invalid release scan configuration") + assert.Empty(t, f.requests()) + }) + } +} + +func TestReleaseScanWebUI(t *testing.T) { + binary := os.Getenv("PIPELEEK_BINARY") + require.NotEmpty(t, binary, "e2e tests require PIPELEEK_BINARY") + binary, err := filepath.Abs(binary) + require.NoError(t, err) + f := newFixture(t, links("/asset"), plainAsset) + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, binary, "gl", "release", "scan", + "--url", f.server.URL, "--token", "test-token", "--release-limit", "1", + "--secrets-verification=false", "--webui", "--json") + cmd.Env = append(os.Environ(), "PIPELEEK_NO_CONFIG=1") + stdout, err := cmd.StdoutPipe() + require.NoError(t, err) + var stderr bytes.Buffer + cmd.Stderr = &stderr + require.NoError(t, cmd.Start()) + t.Cleanup(func() { + cancel() + _ = cmd.Wait() + }) + lines := bufio.NewScanner(stdout) + var uiURL string + var finished bool + for lines.Scan() { + var event struct { + Message string `json:"message"` + URL string `json:"url"` + } + require.NoError(t, json.Unmarshal(lines.Bytes(), &event)) + if event.Message == "Findings web UI started" { + uiURL = event.URL + } + if event.Message == "Release scan finished" { + finished = true + break + } + } + require.NoError(t, lines.Err()) + require.True(t, finished, stderr.String()) + require.NotEmpty(t, uiURL) + u, err := url.Parse(uiURL) + require.NoError(t, err) + u.Path = "/events" + client := &http.Client{Timeout: 5 * time.Second} + response, err := client.Get(u.String()) + require.NoError(t, err) + defer response.Body.Close() + require.Equal(t, http.StatusOK, response.StatusCode) + var body strings.Builder + events := bufio.NewScanner(response.Body) + for events.Scan() { + body.WriteString(events.Text()) + body.WriteByte('\n') + if events.Text() == "event: done" { + break + } + } + require.NoError(t, events.Err()) + assert.Contains(t, body.String(), "releasefixture") + assert.Contains(t, body.String(), `"project":"group/project"`) + assert.Contains(t, body.String(), `"tag":"v2"`) + assert.Contains(t, body.String(), "event: done") +} From 675a8906d2b1bb81ad72cb443108726163a793e8 Mon Sep 17 00:00:00 2001 From: frjcomp <107982661+frjcomp@users.noreply.github.com> Date: Tue, 20 Oct 2026 07:08:06 +0000 Subject: [PATCH 2/7] chore(devcontainer): include GitHub CLI feature Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .devcontainer/devcontainer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 7b4d45ba..cd256d9e 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -5,7 +5,8 @@ "ghcr.io/devcontainers/features/docker-outside-of-docker:1": {}, "ghcr.io/devcontainers/features/python:1": { "version": "3.12" - } + }, + "ghcr.io/devcontainers/features/github-cli:1": {} }, "customizations": { "vscode": { From f77d27d48f92c189dca198bbec3487ff2481a66b Mon Sep 17 00:00:00 2001 From: frjcomp <107982661+frjcomp@users.noreply.github.com> Date: Tue, 20 Oct 2026 08:22:18 +0000 Subject: [PATCH 3/7] fix(gitlab): simplify release scanning and refine failure logging Reuse shared artifact scanning and the standard HTTP client, preserve binary and UTF-8 scanning, and address release scan review feedback. Silently skip unavailable assets, downgrade detection timeouts to warnings, and synchronize documentation and regression tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/guides/gitlab.md | 49 +--- docs/introduction/configuration.md | 46 --- internal/cmd/gitlab/release/scan/scan.go | 5 +- pkg/gitlab/release/scan/download.go | 108 +++---- pkg/gitlab/release/scan/download_test.go | 50 ++-- pkg/gitlab/release/scan/scanner.go | 35 ++- pkg/gitlab/release/scan/scanner_test.go | 19 +- pkg/scan/artifact/processor.go | 40 +-- pkg/scan/artifact/processor_test.go | 101 +++++++ pkg/scan/artifact/release.go | 256 ----------------- pkg/scan/artifact/release_test.go | 342 ----------------------- pkg/scanner/artifact/artifact.go | 135 ++++++--- pkg/scanner/engine/engine.go | 7 +- pkg/scanner/engine/engine_test.go | 4 + tests/e2e/gitlab/release/scan_test.go | 86 +++++- 15 files changed, 437 insertions(+), 846 deletions(-) delete mode 100644 pkg/scan/artifact/release.go delete mode 100644 pkg/scan/artifact/release_test.go diff --git a/docs/guides/gitlab.md b/docs/guides/gitlab.md index 6d7bc576..77bead42 100644 --- a/docs/guides/gitlab.md +++ b/docs/guides/gitlab.md @@ -114,53 +114,24 @@ Note: this only scans repositories you have access to. You can specify single re ## Secret Detection in Release Assets -Release assets can contain credentials in configuration files, packaged -applications, and embedded binary strings. Scan the linked assets of all -published releases visible to your token: +Use `gl release scan` to scan published release assets with the same project +filters and secret-detection controls as `gl scan`. ```bash pipeleek gl release scan --url https://gitlab.example.com --token [redacted] -# One project's exact release, including its generated source ZIP pipeleek gl release scan --url https://gitlab.example.com --token [redacted] \ --repo group/project --tag v1.0 --include-sources - -# The newest five published releases per project in a group and its subgroups -pipeleek gl release scan --url https://gitlab.example.com --token [redacted] \ - --namespace group --release-limit 5 --threads 4 --max-artifact-size 100Mb ``` -Project selection follows `gl scan`: use `--repo`, `--namespace`, `--search`, -`--owned`, or `--member`. `--repo` takes precedence over project-list filters; -`--member` applies to global project discovery, not namespace discovery. -`--tag` requires `--repo` and cannot be combined with a nonzero `--release-limit`. -The default limit is `0` (all published releases); upcoming releases are excluded. - -Linked HTTP(S) assets are downloaded even when hosted outside GitLab. Ensure the -linked hosts are within your authorized scan scope. The GitLab token is sent only -to the configured GitLab origin, never to external hosts, different ports, or -cross-origin redirect targets. HTTP(S) links containing user credentials and -HTTPS-to-HTTP redirects are rejected. Signed URL query parameters are omitted from -finding URLs and download-error output. - -Generated source archives are excluded by default. `--include-sources` downloads -one ZIP per release, avoiding duplicate scans of the same source tree in multiple -formats. Release descriptions, evidence files, and Git history are not scanned. -Plain files, printable binary strings, and ZIP, TAR, gzip, and bzip2 archives -(including nested combinations) are scanned without executing assets. Other -archive formats are reported as unsupported. Findings retain project, release -tag/name, asset, and file context. - -`--max-artifact-size` caps both downloaded and cumulative expanded bytes per asset, -including intermediate archive content and extracted member data. Size-limit -skips are reported but do not fail the command. Other download, discovery, archive, -or detection failures leave coverage incomplete: scanning continues where possible, -then exits nonzero with a summary. Unsupported URL schemes and archive formats -are reported as incomplete coverage, not silently treated as successfully scanned. - -The standard `--confidence`, `--secrets-verification`, `--hit-timeout`, and -`--webui` controls are available. Credential verification is enabled by default; -`--secrets-verification=false` disables provider verification. +`--tag` requires `--repo`; alternatively, `--release-limit N` scans the newest N +published releases per project (`0` means all). +`--include-sources` adds one generated source ZIP per release. +External HTTP(S) assets are included, but GitLab credentials stay on the configured +origin, so ensure linked hosts are authorized scan targets. +`--max-artifact-size` limits downloaded bytes, not extracted archive size; +oversized assets and HTTP 403/404 downloads are skipped, while other download or processing failures produce a +nonzero exit after remaining assets are scanned. ## Secret Detection in Pipelines And Artifacts diff --git a/docs/introduction/configuration.md b/docs/introduction/configuration.md index c3c395d4..aa2acbda 100644 --- a/docs/introduction/configuration.md +++ b/docs/introduction/configuration.md @@ -103,52 +103,6 @@ gitea: repo_order: desc # asc or desc; default desc ``` -### GitLab Release Assets - -`pipeleek gl release scan` inherits `gitlab.url`, `gitlab.token`, and the common -scan settings above. Release-specific settings live under `gitlab.release.scan`: - -```yaml -gitlab: - url: https://gitlab.example.com - token: "[redacted]" - release: - scan: - repo: group/project # Optional; mutually exclusive with namespace - namespace: "" # Group and subgroups - search: "" # Project search, not release-tag search - owned: false - member: false # Applies to global project discovery - tag: "" # Exact tag; requires repo; cannot combine with a nonzero release_limit - release_limit: 0 # Published releases per project, newest first; 0 means all - include_sources: false # Also scan one generated source ZIP per release -``` - -`--repo` selects a project directly and takes precedence over project-list -filters. Namespace discovery applies `owned` and `search` and includes shared -projects and subgroups, matching `gl scan`. `member` applies only when neither -`repo` nor `namespace` is set. - -Release scanning uses `common.max_artifact_size` (default `500Mb`) as the maximum -downloaded and expanded bytes per asset. Larger assets are reported as skipped. -`common.threads` (default `4`) bounds concurrent asset workers; -`common.hit_timeout` accepts a duration such as `60s` or an integer number of -seconds. Confidence filtering, credential verification, and the Web UI use the -same common settings as other scans. - -For example: - -```bash -export PIPELEEK_GITLAB_RELEASE_SCAN_RELEASE_LIMIT=5 -export PIPELEEK_GITLAB_RELEASE_SCAN_INCLUDE_SOURCES=true -pipeleek gl release scan --repo group/project -``` - -Linked HTTP(S) assets on external hosts are scanned by default. The GitLab token -is sent only to the configured GitLab origin, never to another origin or through -a cross-origin redirect. See [release scanning](../guides/gitlab.md#secret-detection-in-release-assets) -for coverage and failure behavior. - ## Environment Variables Set any config key using `PIPELEEK_` prefix. Replace dots with underscores: diff --git a/internal/cmd/gitlab/release/scan/scan.go b/internal/cmd/gitlab/release/scan/scan.go index 50a2ba75..5bf92fcc 100644 --- a/internal/cmd/gitlab/release/scan/scan.go +++ b/internal/cmd/gitlab/release/scan/scan.go @@ -32,7 +32,8 @@ By default, scan all releases of all token-visible projects, including HTTP(S) assets hosted outside GitLab. GitLab credentials are never sent to other origins. Use --include-sources to also scan one generated source ZIP per release. -Assets exceeding --max-artifact-size (downloaded or expanded bytes) are skipped. +Assets exceeding --max-artifact-size (downloaded bytes) are skipped. +Extracted archive sizes are not limited. Other coverage failures are reported while scanning continues, with a nonzero exit. --member applies to global project discovery; --owned and --search also apply within --namespace, including subgroups. --repo takes precedence over list filters.`, @@ -48,7 +49,7 @@ pipeleek gl release scan --url https://gitlab.example.com --token [redacted] --n } common := config.DefaultCommonScanOptions() flags.AddCommonScanFlagsNoArtifacts(cmd, &common) - cmd.Flags().String("max-artifact-size", "500Mb", "Maximum downloaded and expanded bytes per asset; larger assets are skipped") + cmd.Flags().String("max-artifact-size", "500Mb", "Maximum downloaded bytes per asset; extracted archive sizes are not limited") cmd.Flags().StringP("repo", "r", "", "Single repository to scan, format: namespace/repo") cmd.Flags().StringP("namespace", "n", "", "Namespace to scan, including subgroup projects") cmd.Flags().StringP("search", "s", "", "Query string for searching projects") diff --git a/pkg/gitlab/release/scan/download.go b/pkg/gitlab/release/scan/download.go index 7e807850..fb5489ca 100644 --- a/pkg/gitlab/release/scan/download.go +++ b/pkg/gitlab/release/scan/download.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "io" - "net" "net/http" "net/url" "strings" @@ -13,20 +12,21 @@ import ( ) var errAssetTooLarge = errors.New("asset exceeds maximum size") +var errAssetUnavailable = errors.New("asset is forbidden or not found") -type downloader struct { - base *url.URL - token string - limit int64 - client *http.Client -} - -func newDownloader(base *url.URL, token string, limit int64) *downloader { - d := &downloader{ - base: base, token: token, limit: limit, - client: httpclient.GetPipeleekStandardHTTPClient(), +func downloadAsset(u, base *url.URL, token string, limit int64) ([]byte, error) { + client := httpclient.GetPipeleekStandardHTTPClient() + defer client.CloseIdleConnections() + authorize := func(req *http.Request) { + req.Header.Del("PRIVATE-TOKEN") + req.Header.Del("Authorization") + req.Header.Del("Cookie") + req.Header.Del("Referer") + if sameOrigin(base, req.URL) { + req.Header.Set("PRIVATE-TOKEN", token) + } } - d.client.CheckRedirect = func(req *http.Request, via []*http.Request) error { + client.CheckRedirect = func(req *http.Request, via []*http.Request) error { if len(via) >= 10 { return errors.New("too many asset redirects") } @@ -36,10 +36,37 @@ func newDownloader(base *url.URL, token string, limit int64) *downloader { if via[len(via)-1].URL.Scheme == "https" && req.URL.Scheme == "http" { return errors.New("asset redirect downgrades HTTPS to HTTP") } - d.setCredentials(req) + authorize(req) return nil } - return d + req, err := http.NewRequest(http.MethodGet, u.String(), nil) + if err != nil { + return nil, errors.New("cannot create asset request") + } + authorize(req) + resp, err := client.Do(req) + if err != nil { + // Request/redirect errors can include signed URLs; do not log their values. + return nil, errors.New("asset request failed (connection, URL, or redirect policy)") + } + defer resp.Body.Close() + if resp.StatusCode == http.StatusForbidden || resp.StatusCode == http.StatusNotFound { + return nil, errAssetUnavailable + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("asset download returned HTTP %d", resp.StatusCode) + } + if resp.ContentLength > limit { + return nil, errAssetTooLarge + } + content, err := io.ReadAll(io.LimitReader(resp.Body, limit+1)) + if err != nil { + return nil, fmt.Errorf("reading asset failed: %w", err) + } + if int64(len(content)) > limit { + return nil, errAssetTooLarge + } + return content, nil } func validateAssetURL(u *url.URL) error { @@ -66,17 +93,7 @@ func sameOrigin(a, b *url.URL) bool { strings.EqualFold(a.Hostname(), b.Hostname()) && port(a) == port(b) } -func (d *downloader) setCredentials(req *http.Request) { - req.Header.Del("PRIVATE-TOKEN") - req.Header.Del("Authorization") - req.Header.Del("Cookie") - req.Header.Del("Referer") - if sameOrigin(d.base, req.URL) { - req.Header.Set("PRIVATE-TOKEN", d.token) - } -} - -func (d *downloader) resolve(raw string) (*url.URL, error) { +func resolveAssetURL(base *url.URL, raw string) (*url.URL, error) { if strings.TrimSpace(raw) == "" { return nil, errors.New("empty asset URL") } @@ -84,7 +101,7 @@ func (d *downloader) resolve(raw string) (*url.URL, error) { if err != nil { return nil, errors.New("invalid asset URL") } - u = d.base.ResolveReference(u) + u = base.ResolveReference(u) if err := validateAssetURL(u); err != nil { return nil, err } @@ -100,40 +117,3 @@ func displayURL(u *url.URL) string { safe.Fragment = "" return safe.String() } - -func (d *downloader) download(u *url.URL) ([]byte, error) { - req, err := http.NewRequest(http.MethodGet, u.String(), nil) - if err != nil { - return nil, errors.New("cannot create asset request") - } - d.setCredentials(req) - resp, err := d.client.Do(req) - if err != nil { - // url.Error includes the full request URL, potentially containing credentials. - var urlErr *url.Error - if errors.As(err, &urlErr) { - err = urlErr.Err - } - var netErr net.Error - if errors.As(err, &netErr) && netErr.Timeout() { - return nil, errors.New("asset download timed out") - } - // Redirect parsing errors may also embed a signed Location URL. - return nil, errors.New("asset request failed (connection, URL, or redirect policy)") - } - defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("asset download returned HTTP %d", resp.StatusCode) - } - if resp.ContentLength > d.limit { - return nil, errAssetTooLarge - } - content, err := io.ReadAll(io.LimitReader(resp.Body, d.limit+1)) - if err != nil { - return nil, fmt.Errorf("reading asset failed: %w", err) - } - if int64(len(content)) > d.limit { - return nil, errAssetTooLarge - } - return content, nil -} diff --git a/pkg/gitlab/release/scan/download_test.go b/pkg/gitlab/release/scan/download_test.go index 5844a45d..0c2ef9d9 100644 --- a/pkg/gitlab/release/scan/download_test.go +++ b/pkg/gitlab/release/scan/download_test.go @@ -13,11 +13,24 @@ import ( "github.com/stretchr/testify/require" ) -func testDownloader(t *testing.T, base string, limit int64) *downloader { +type testAssetDownloader struct { + base *url.URL + limit int64 +} + +func (d testAssetDownloader) resolve(raw string) (*url.URL, error) { + return resolveAssetURL(d.base, raw) +} + +func (d testAssetDownloader) download(u *url.URL) ([]byte, error) { + return downloadAsset(u, d.base, "test-token", d.limit) +} + +func testDownloader(t *testing.T, base string, limit int64) testAssetDownloader { t.Helper() u, err := url.Parse(base) require.NoError(t, err) - return newDownloader(u, "test-token", limit) + return testAssetDownloader{u, limit} } func TestDownloadCredentials(t *testing.T) { @@ -61,7 +74,9 @@ func TestDownloadLimitsAndFailures(t *testing.T) { {"oversize", "123456", false, 200, errAssetTooLarge}, {"chunked-exact", "12345", true, 200, nil}, {"chunked-oversize", "123456", true, 200, errAssetTooLarge}, - {"denied", "123", false, 403, errors.New("HTTP 403")}, + {"denied", "123", false, 403, errAssetUnavailable}, + {"not-found", "123", false, 404, errAssetUnavailable}, + {"unauthorized", "123", false, 401, errors.New("HTTP 401")}, {"partial", "123", false, 206, errors.New("HTTP 206")}, } { t.Run(tc.name, func(t *testing.T) { @@ -82,6 +97,8 @@ func TestDownloadLimitsAndFailures(t *testing.T) { assert.Equal(t, tc.body, string(content)) } else if errors.Is(tc.want, errAssetTooLarge) { assert.ErrorIs(t, err, errAssetTooLarge) + } else if errors.Is(tc.want, errAssetUnavailable) { + assert.ErrorIs(t, err, errAssetUnavailable) } else { require.ErrorContains(t, err, tc.want.Error()) } @@ -128,22 +145,19 @@ func TestAssetURLs(t *testing.T) { } func TestRedirectPolicy(t *testing.T) { - d := testDownloader(t, "https://gitlab.example.com", 10) - old, err := http.NewRequest(http.MethodGet, "https://gitlab.example.com/asset", nil) - require.NoError(t, err) - for _, raw := range []string{"http://example.com/file", "ftp://example.com/file"} { - req, err := http.NewRequest(http.MethodGet, raw, nil) - require.NoError(t, err) - require.Error(t, d.client.CheckRedirect(req, []*http.Request{old})) + for _, target := range []string{"http://example.com/file", "ftp://example.com/file"} { + t.Run(target, func(t *testing.T) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, target, http.StatusFound) + })) + defer srv.Close() + d := testDownloader(t, srv.URL, 10) + u, err := d.resolve("/asset") + require.NoError(t, err) + _, err = d.download(u) + require.ErrorContains(t, err, "asset request failed") + }) } - req, err := http.NewRequest(http.MethodGet, "https://other.example.com/file", nil) - require.NoError(t, err) - req.Header.Set("PRIVATE-TOKEN", "secret") - req.Header.Set("Authorization", "secret") - req.Header.Set("Cookie", "secret") - require.NoError(t, d.client.CheckRedirect(req, []*http.Request{old})) - assert.Empty(t, req.Header) - require.Error(t, d.client.CheckRedirect(req, make([]*http.Request, 10))) } func TestDownloadErrorRedactsQuery(t *testing.T) { diff --git a/pkg/gitlab/release/scan/scanner.go b/pkg/gitlab/release/scan/scanner.go index 20574450..f0595f25 100644 --- a/pkg/gitlab/release/scan/scanner.go +++ b/pkg/gitlab/release/scan/scanner.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "math" + "net/http" "net/url" "path" "sync" @@ -17,6 +18,7 @@ import ( "github.com/CompassSecurity/pipeleek/pkg/scan/result" "github.com/CompassSecurity/pipeleek/pkg/scan/runner" "github.com/CompassSecurity/pipeleek/pkg/scanner" + "github.com/CompassSecurity/pipeleek/pkg/scanner/engine" "github.com/rs/zerolog" "github.com/rs/zerolog/log" gitlab "gitlab.com/gitlab-org/api/client-go" @@ -112,16 +114,16 @@ func (s *Scanner) Scan() error { if err != nil { return errors.New("invalid GitLab URL") } - return s.scan(git, newDownloader(base, s.options.Token, s.options.MaxBytes)) + return s.scan(git, base) } -func (s *Scanner) scan(git *gitlab.Client, d *downloader) error { +func (s *Scanner) scan(git *gitlab.Client, base *url.URL) error { work := make(chan asset, s.options.Threads) var workers sync.WaitGroup for range s.options.Threads { workers.Go(func() { for item := range work { - s.scanAsset(d, item) + s.scanAsset(base, item) } }) } @@ -192,6 +194,9 @@ func (s *Scanner) projectReleases(git *gitlab.Client, project *gitlab.Project, w for { releases, resp, err := git.Releases.ListReleases(project.ID, opts) if err != nil { + if resp != nil && resp.StatusCode == http.StatusForbidden { + return nil + } return fmt.Errorf("listing releases: %w", err) } for _, release := range releases { @@ -247,8 +252,8 @@ func (s *Scanner) releaseAssets(git *gitlab.Client, project *gitlab.Project, rel } } -func (s *Scanner) scanAsset(d *downloader, item asset) { - u, err := d.resolve(item.rawURL) +func (s *Scanner) scanAsset(base *url.URL, item asset) { + u, err := resolveAssetURL(base, item.rawURL) if err != nil { s.assetFailure(item, err) return @@ -261,11 +266,11 @@ func (s *Scanner) scanAsset(d *downloader, item asset) { if item.name == "" { item.name = filename } - content, err := d.download(u) + content, err := downloadAsset(u, base, s.options.Token, s.options.MaxBytes) if err == nil { - err = artifactproc.ProcessReleaseAsset(content, filename, artifactproc.ReleaseProcessOptions{ - MaxBytes: s.options.MaxBytes, VerifyCredentials: s.options.SecretsVerification, - HitTimeout: s.options.HitTimeout, GitLabURL: s.options.URL, + _, err = artifactproc.ProcessSingleFile(content, filename, artifactproc.ProcessOptions{ + VerifyCredentials: s.options.SecretsVerification, + HitTimeout: s.options.HitTimeout, GitLabURL: s.options.URL, ReportFinding: func(finding scanner.Finding, file, archive string) { sourceType := logging.SecretTypeArchive if archive != "" { @@ -279,9 +284,11 @@ func (s *Scanner) scanAsset(d *downloader, item asset) { }, }) } - if errors.Is(err, errAssetTooLarge) || errors.Is(err, artifactproc.ErrReleaseAssetTooLarge) { + if errors.Is(err, errAssetUnavailable) { s.skipped.Add(1) - log.Warn().Str("project", item.project).Str("tag", item.tag).Str("asset", item.name). + } else if errors.Is(err, errAssetTooLarge) { + s.skipped.Add(1) + log.Debug().Str("project", item.project).Str("tag", item.tag).Str("asset", item.name). Msg("Skipped release asset exceeding maximum size") } else if err != nil { s.assetFailure(item, err) @@ -292,6 +299,10 @@ func (s *Scanner) scanAsset(d *downloader, item asset) { func (s *Scanner) assetFailure(item asset, err error) { s.failures.Add(1) - log.Error().Err(err).Str("project", item.project).Str("tag", item.tag). + event := log.Error() + if errors.Is(err, engine.ErrHitTimeout) { + event = log.Warn() + } + event.Err(err).Str("project", item.project).Str("tag", item.tag). Str("asset", item.name).Msg("Failed scanning release asset") } diff --git a/pkg/gitlab/release/scan/scanner_test.go b/pkg/gitlab/release/scan/scanner_test.go index 14b732b8..0f4c59f4 100644 --- a/pkg/gitlab/release/scan/scanner_test.go +++ b/pkg/gitlab/release/scan/scanner_test.go @@ -140,7 +140,7 @@ func TestDiscoveryContinuesAfterFailure(t *testing.T) { case "/api/v4/projects": _, _ = fmt.Fprint(w, `[{"id":1,"path_with_namespace":"g/one"},{"id":2,"path_with_namespace":"g/two"}]`) case "/api/v4/projects/1/releases": - w.WriteHeader(http.StatusForbidden) + w.WriteHeader(http.StatusNotFound) case "/api/v4/projects/2/releases": _, _ = fmt.Fprint(w, `[{"tag_name":"v1"}]`) case "/api/v4/projects/2/releases/v1/assets/links": @@ -154,7 +154,7 @@ func TestDiscoveryContinuesAfterFailure(t *testing.T) { require.NoError(t, err) o := validOptions() s := NewScanner(o) - require.ErrorContains(t, s.scan(git, testDownloader(t, srv.URL, 100)), "incomplete") + require.ErrorContains(t, s.scan(git, testDownloader(t, srv.URL, 100).base), "incomplete") assert.Equal(t, int64(2), s.projects.Load()) assert.Equal(t, int64(1), s.releases.Load()) assert.Equal(t, int64(1), s.failures.Load()) @@ -168,6 +168,21 @@ func TestPublished(t *testing.T) { assert.False(t, published(&gitlab.Release{UpcomingRelease: true})) } +func TestForbiddenReleaseListingIsIgnored(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + })) + defer srv.Close() + git, err := util.GetGitlabClient("token", srv.URL) + require.NoError(t, err) + s := NewScanner(validOptions()) + work := make(chan asset, 1) + require.NoError(t, s.projectReleases(git, &gitlab.Project{ID: 1}, work)) + assert.Empty(t, work) + assert.Zero(t, s.failures.Load()) + assert.Zero(t, s.releases.Load()) +} + func TestProjectPagination(t *testing.T) { for _, namespace := range []string{"", "group"} { t.Run(namespace, func(t *testing.T) { diff --git a/pkg/scan/artifact/processor.go b/pkg/scan/artifact/processor.go index 4bb6d53a..18568eb6 100644 --- a/pkg/scan/artifact/processor.go +++ b/pkg/scan/artifact/processor.go @@ -8,6 +8,7 @@ import ( "time" "github.com/CompassSecurity/pipeleek/pkg/scanner" + scannerartifact "github.com/CompassSecurity/pipeleek/pkg/scanner/artifact" "github.com/h2non/filetype" "github.com/rs/zerolog/log" "github.com/wandb/parallel" @@ -20,6 +21,8 @@ type ProcessOptions struct { ArtifactName string WorkflowRunName string HitTimeout time.Duration + GitLabURL string + ReportFinding func(scanner.Finding, string, string) } type FileProcessingResult struct { @@ -84,18 +87,9 @@ func processZipFile(file *zip.File, opts ProcessOptions) FileProcessingResult { return result } - kind, _ := filetype.Match(content) - result.FileType = kind.MIME.Value - result.IsUnknown = kind == filetype.Unknown - result.IsArchive = filetype.IsArchive(content) - - if result.IsUnknown { - scanner.DetectFileHits(content, opts.BuildURL, opts.ArtifactName, file.Name, "", opts.VerifyCredentials, opts.HitTimeout) - } else if result.IsArchive { - scanner.HandleArchiveArtifact(file.Name, content, opts.BuildURL, opts.ArtifactName, opts.VerifyCredentials, opts.HitTimeout) - } - - return result + processed, err := ProcessSingleFile(content, file.Name, opts) + processed.Error = err + return *processed } func ExtractZipFile(zf *zip.File) ([]byte, error) { @@ -109,7 +103,7 @@ func ExtractZipFile(zf *zip.File) ([]byte, error) { func DetermineFileType(content []byte) (mimeType string, isArchive bool, isUnknown bool) { kind, _ := filetype.Match(content) - return kind.MIME.Value, filetype.IsArchive(content), kind == filetype.Unknown + return kind.MIME.Value, scannerartifact.IsArchive(content), kind == filetype.Unknown } func ProcessSingleFile(content []byte, filename string, opts ProcessOptions) (*FileProcessingResult, error) { @@ -122,11 +116,19 @@ func ProcessSingleFile(content []byte, filename string, opts ProcessOptions) (*F result.IsArchive = isArchive result.IsUnknown = isUnknown - if isUnknown { - scanner.DetectFileHits(content, opts.BuildURL, opts.ArtifactName, filename, "", opts.VerifyCredentials, opts.HitTimeout) - } else if isArchive { - scanner.HandleArchiveArtifact(filename, content, opts.BuildURL, opts.ArtifactName, opts.VerifyCredentials, opts.HitTimeout) + report := opts.ReportFinding + if report == nil { + report = func(finding scanner.Finding, file, ancestry string) { + scannerartifact.ReportFinding(finding, opts.BuildURL, opts.ArtifactName, file, ancestry) + } } - - return result, nil + if opts.HitTimeout == 0 { + opts.HitTimeout = time.Minute + } + err := scannerartifact.ScanFile(content, filename, scannerartifact.ScanOptions{ + VerifyCredentials: opts.VerifyCredentials, HitTimeout: opts.HitTimeout, + GitLabURL: opts.GitLabURL, ReportFinding: report, + }) + result.Error = err + return result, err } diff --git a/pkg/scan/artifact/processor_test.go b/pkg/scan/artifact/processor_test.go index 08e39186..89af4477 100644 --- a/pkg/scan/artifact/processor_test.go +++ b/pkg/scan/artifact/processor_test.go @@ -1,15 +1,116 @@ package artifact import ( + "archive/tar" "archive/zip" "bytes" + "fmt" + "strings" "testing" + "time" "github.com/CompassSecurity/pipeleek/pkg/scanner" + "github.com/h2non/filetype" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestProcessSingleFileExecutableAndUTF8(t *testing.T) { + for _, container := range []string{"standalone", "zip", "tar"} { + for _, binary := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/binary=%v", container, binary), func(t *testing.T) { + secret := fmt.Sprintf("credential-%s-%v", container, binary) + content := []byte("CI_REGISTRY_PASSWORD=" + secret + "\n") + name := "config.env" + if binary { + name = "program" + header := make([]byte, 64) + copy(header, "\x7fELF") + content = append(header, content...) + kind, err := filetype.Match(content) + require.NoError(t, err) + require.Equal(t, "elf", kind.Extension, "fixture must match the real ELF detector") + } else { + secret += "-p\u00e4ssw\u00f6rd" + content = []byte("CI_REGISTRY_PASSWORD=" + secret + "\n") + } + expectedFile := name + secondSecret := "after-" + secret + switch container { + case "zip": + var buf bytes.Buffer + w := zip.NewWriter(&buf) + for _, file := range []struct { + name string + content []byte + }{ + {name, content}, {"after.env", []byte("CI_REGISTRY_PASSWORD=" + secondSecret + "\n")}, + } { + f, err := w.Create(file.name) + require.NoError(t, err) + _, err = f.Write(file.content) + require.NoError(t, err) + } + require.NoError(t, w.Close()) + content, name = buf.Bytes(), "bundle.zip" + case "tar": + var buf bytes.Buffer + w := tar.NewWriter(&buf) + for _, file := range []struct { + name string + content []byte + }{ + {name, content}, {"after.env", []byte("CI_REGISTRY_PASSWORD=" + secondSecret + "\n")}, + } { + require.NoError(t, w.WriteHeader(&tar.Header{Name: file.name, Mode: 0600, Size: int64(len(file.content))})) + _, err := w.Write(file.content) + require.NoError(t, err) + } + require.NoError(t, w.Close()) + content, name = buf.Bytes(), "bundle.tar" + } + var found, after bool + _, err := ProcessSingleFile(content, name, ProcessOptions{ + HitTimeout: time.Minute, + ReportFinding: func(f scanner.Finding, file, ancestry string) { + if strings.Contains(f.Text, "CI_REGISTRY_PASSWORD="+secret) { + found = true + assert.Equal(t, expectedFile, file) + if container != "standalone" { + assert.Equal(t, name, ancestry) + } + } + if file == "after.env" && strings.Contains(f.Text, secondSecret) { + after = true + } + }, + }) + require.NoError(t, err) + assert.True(t, found, "expected the complete credential, including UTF-8 bytes") + if container != "standalone" { + assert.True(t, after, "a binary member must not prevent subsequent text members being scanned") + } + }) + } + } +} + +func TestDetermineFileTypeDoesNotTreatDocumentsAsArchives(t *testing.T) { + for _, content := range [][]byte{ + append([]byte("\x7fELF"), make([]byte, 64)...), + []byte("MZ" + strings.Repeat("\x00", 70)), + []byte("\xfe\xed\xfa\xcf" + strings.Repeat("\x00", 70)), + []byte("%PDF-1.7\n"), + []byte("SQLite format 3\x00" + strings.Repeat("\x00", 70)), + } { + kind, err := filetype.Match(content) + require.NoError(t, err) + require.NotEqual(t, filetype.Unknown, kind) + _, isArchive, _ := DetermineFileType(content) + assert.False(t, isArchive, kind.Extension) + } +} + func init() { scanner.InitRules([]string{}) } diff --git a/pkg/scan/artifact/release.go b/pkg/scan/artifact/release.go deleted file mode 100644 index 8beafb9c..00000000 --- a/pkg/scan/artifact/release.go +++ /dev/null @@ -1,256 +0,0 @@ -package artifact - -import ( - "archive/tar" - "archive/zip" - "bytes" - "compress/bzip2" - "compress/gzip" - "errors" - "fmt" - "io" - "path" - "strings" - "time" - - "github.com/CompassSecurity/pipeleek/pkg/archive" - "github.com/CompassSecurity/pipeleek/pkg/scanner" - "github.com/CompassSecurity/pipeleek/pkg/scanner/engine" - "github.com/h2non/filetype" -) - -// ErrReleaseAssetTooLarge marks a policy skip rather than a processing failure. -var ErrReleaseAssetTooLarge = errors.New("release asset exceeds byte limit") - -// ReleaseProcessOptions controls release-only processing without changing pipeline -// artifact handling. MaxBytes bounds both input size and cumulative expanded -// payloads, including intermediate nested archives and decompressed tar streams. -type ReleaseProcessOptions struct { - MaxBytes int64 - VerifyCredentials bool - HitTimeout time.Duration - GitLabURL string - // ReportFinding receives the archive-relative file path and archive names - // joined by " -> ". A standalone file has an empty archive ancestry. - ReportFinding func(scanner.Finding, string, string) -} - -type releaseProcessor struct { - opts ReleaseProcessOptions - expanded int64 - detect func([]byte, int, bool, engine.DetectionOptions) ([]scanner.Finding, error) -} - -// ProcessReleaseAsset scans content in memory; it never extracts files to disk or -// executes assets. ZIP, tar, gzip, and bzip2 are supported up to ten archive layers. -// Unsupported archives and incomplete processing return errors. -func ProcessReleaseAsset(content []byte, filename string, opts ReleaseProcessOptions) error { - if opts.MaxBytes <= 0 { - return errors.New("release asset byte limit must be positive") - } - if int64(len(content)) > opts.MaxBytes { - return ErrReleaseAssetTooLarge - } - if opts.ReportFinding == nil { - return errors.New("release asset finding reporter is required") - } - p := releaseProcessor{opts: opts, detect: engine.DetectHitsWithOptions} - return p.process(content, filename, nil, 0) -} - -func (p *releaseProcessor) readExpanded(r io.Reader) ([]byte, error) { - remaining := p.opts.MaxBytes - p.expanded - // Avoid remaining+1 overflow, while still probing for an extra byte. - content, err := io.ReadAll(io.LimitReader(r, remaining)) - p.expanded += int64(len(content)) - if err != nil { - return nil, err - } - var extra [1]byte - n, err := io.ReadFull(r, extra[:]) - if n != 0 { - return nil, ErrReleaseAssetTooLarge - } - if err != nil && !errors.Is(err, io.EOF) { - return nil, err - } - return content, nil -} - -func releaseArchiveType(content []byte, filename string) string { - kind, _ := filetype.Match(content) - switch kind.Extension { - case "zip", "tar", "gz", "bz2": - return kind.Extension - } - if filetype.IsArchive(content) { - return "unsupported" - } - name := strings.ToLower(filename) - for _, suffix := range []struct{ ext, kind string }{ - {".tar.gz", "gz"}, {".tgz", "gz"}, {".tar.bz2", "bz2"}, {".tbz2", "bz2"}, {".tbz", "bz2"}, - {".zip", "zip"}, {".tar", "tar"}, {".gzip", "gz"}, {".gz", "gz"}, {".bz2", "bz2"}, {".bzip2", "bz2"}, - {".7z", "unsupported"}, {".rar", "unsupported"}, {".xz", "unsupported"}, - {".zst", "unsupported"}, {".zstd", "unsupported"}, {".lz", "unsupported"}, - {".lzma", "unsupported"}, {".lz4", "unsupported"}, {".cab", "unsupported"}, - {".ar", "unsupported"}, {".deb", "unsupported"}, {".rpm", "unsupported"}, - {".txz", "unsupported"}, {".taz", "unsupported"}, {".z", "unsupported"}, - {".iso", "unsupported"}, {".dmg", "unsupported"}, - } { - if strings.HasSuffix(name, suffix.ext) { - return suffix.kind - } - } - return "" -} - -func (p *releaseProcessor) process(content []byte, filename string, ancestry []string, depth int) error { - kind := releaseArchiveType(content, filename) - if kind == "" { - findings, err := p.detect(archive.ExtractPrintableStrings(content, archive.MinStringLength), 1, - p.opts.VerifyCredentials, engine.DetectionOptions{ - Path: filename, GitLabURL: p.opts.GitLabURL, Timeout: p.opts.HitTimeout, - }) - if err != nil { - return fmt.Errorf("detect release asset %q: %w", filename, err) - } - for _, finding := range findings { - p.opts.ReportFinding(finding, filename, strings.Join(ancestry, " -> ")) - } - return nil - } - if depth >= 10 { - return fmt.Errorf("release archive %q exceeds maximum nesting depth 10", filename) - } - ancestry = append(append([]string(nil), ancestry...), filename) - switch kind { - case "zip": - return p.processZip(content, ancestry, depth+1) - case "tar": - return p.processTar(content, ancestry, depth+1) - case "gz", "bz2": - return p.processCompressed(content, filename, kind, ancestry, depth+1) - default: - return fmt.Errorf("unsupported release archive %q", filename) - } -} - -func releaseRelativePath(name string) (string, error) { - // Backslashes may be directory separators on a producer's platform. - name = strings.ReplaceAll(name, `\`, "/") - clean := path.Clean(name) - if strings.ContainsRune(name, '\x00') || strings.Contains(name, ":") || - path.IsAbs(name) || clean == "." || clean == ".." || strings.HasPrefix(clean, "../") { - return "", fmt.Errorf("unsafe release archive member path %q", name) - } - return clean, nil -} - -func (p *releaseProcessor) processZip(content []byte, ancestry []string, depth int) error { - zr, err := zip.NewReader(bytes.NewReader(content), int64(len(content))) - if err != nil { - return fmt.Errorf("decode release ZIP: %w", err) - } - for _, member := range zr.File { - if member.FileInfo().IsDir() && (member.Name == "./" || member.Name == ".") { - continue - } - name, err := releaseRelativePath(member.Name) - if err != nil { - return err - } - if member.FileInfo().IsDir() { - continue - } - if !member.Mode().IsRegular() { - return fmt.Errorf("unsupported release ZIP member type %q", name) - } - if member.UncompressedSize64 > uint64(p.opts.MaxBytes-p.expanded) { - return ErrReleaseAssetTooLarge - } - r, err := member.Open() - if err != nil { - return fmt.Errorf("open release ZIP member %q: %w", name, err) - } - payload, readErr := p.readExpanded(r) - closeErr := r.Close() - if readErr != nil { - return fmt.Errorf("read release ZIP member %q: %w", name, readErr) - } - if closeErr != nil { - return fmt.Errorf("close release ZIP member %q: %w", name, closeErr) - } - if err := p.process(payload, name, ancestry, depth); err != nil { - return err - } - } - return nil -} - -func (p *releaseProcessor) processTar(content []byte, ancestry []string, depth int) error { - if len(content) < 512 { - return errors.New("decode release tar: missing archive header") - } - tr := tar.NewReader(bytes.NewReader(content)) - for { - header, err := tr.Next() - if errors.Is(err, io.EOF) { - return nil - } - if err != nil { - return fmt.Errorf("decode release tar: %w", err) - } - if header.Typeflag == tar.TypeDir && (header.Name == "." || header.Name == "./") { - continue - } - name, err := releaseRelativePath(header.Name) - if err != nil { - return err - } - switch header.Typeflag { - case tar.TypeDir: - continue - case tar.TypeReg, tar.TypeRegA: - default: - return fmt.Errorf("unsupported release tar member type %q", name) - } - if header.Size > p.opts.MaxBytes-p.expanded { - return ErrReleaseAssetTooLarge - } - payload, err := p.readExpanded(tr) - if err != nil { - return fmt.Errorf("read release tar member %q: %w", name, err) - } - if err := p.process(payload, name, ancestry, depth); err != nil { - return err - } - } -} - -func (p *releaseProcessor) processCompressed(content []byte, filename, kind string, ancestry []string, depth int) error { - var r io.Reader - if kind == "gz" { - gz, err := gzip.NewReader(bytes.NewReader(content)) - if err != nil { - return fmt.Errorf("decode release gzip: %w", err) - } - defer func() { _ = gz.Close() }() - r = gz - } else { - r = bzip2.NewReader(bytes.NewReader(content)) - } - payload, err := p.readExpanded(r) - if err != nil { - return fmt.Errorf("expand release %s: %w", kind, err) - } - innerName := strings.TrimSuffix(filename, path.Ext(filename)) - lower := strings.ToLower(filename) - if strings.HasSuffix(lower, ".tgz") || strings.HasSuffix(lower, ".tbz2") || strings.HasSuffix(lower, ".tbz") { - innerName += ".tar" - } - if releaseArchiveType(payload, innerName) == "tar" { - // A compressed tar is one archive layer, with its original asset name. - return p.processTar(payload, ancestry, depth) - } - return p.process(payload, innerName, ancestry, depth) -} diff --git a/pkg/scan/artifact/release_test.go b/pkg/scan/artifact/release_test.go deleted file mode 100644 index 3f988f2c..00000000 --- a/pkg/scan/artifact/release_test.go +++ /dev/null @@ -1,342 +0,0 @@ -package artifact - -import ( - "archive/tar" - "archive/zip" - "bytes" - "compress/gzip" - "encoding/base64" - "errors" - "fmt" - "io" - "strings" - "testing" - "time" - - "github.com/CompassSecurity/pipeleek/pkg/scanner" - "github.com/CompassSecurity/pipeleek/pkg/scanner/engine" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -type releaseTestFile struct { - name string - content []byte -} - -func releaseTestZip(t *testing.T, files ...releaseTestFile) []byte { - t.Helper() - var buf bytes.Buffer - w := zip.NewWriter(&buf) - for _, file := range files { - f, err := w.Create(file.name) - require.NoError(t, err) - _, err = f.Write(file.content) - require.NoError(t, err) - } - require.NoError(t, w.Close()) - return buf.Bytes() -} - -func releaseTestTar(t *testing.T, files ...releaseTestFile) []byte { - t.Helper() - var buf bytes.Buffer - w := tar.NewWriter(&buf) - for _, file := range files { - require.NoError(t, w.WriteHeader(&tar.Header{ - Name: file.name, Mode: 0o600, Size: int64(len(file.content)), - })) - _, err := w.Write(file.content) - require.NoError(t, err) - } - require.NoError(t, w.Close()) - return buf.Bytes() -} - -func releaseTestGzip(t *testing.T, content []byte) []byte { - t.Helper() - var buf bytes.Buffer - w := gzip.NewWriter(&buf) - _, err := w.Write(content) - require.NoError(t, err) - require.NoError(t, w.Close()) - return buf.Bytes() -} - -func releaseTestOptions() ReleaseProcessOptions { - return ReleaseProcessOptions{ - MaxBytes: 1 << 20, HitTimeout: time.Minute, - ReportFinding: func(scanner.Finding, string, string) {}, - } -} - -func TestReleaseAssetMetadataAndPrintableStrings(t *testing.T) { - text := []byte("release-secret-value") - binary := append([]byte{0x7f, 'E', 'L', 'F', 0, 1}, text...) - binary = append(binary, 0, 1, 2) - inner := releaseTestGzip(t, releaseTestTar(t, releaseTestFile{"config/secret.txt", text})) - bzip, err := base64.StdEncoding.DecodeString("QlpoOTFBWSZTWYglTdEAAAaRgAACKgQfACAAIp5BiEAMKUPmg4LYhlI8LuSKcKEhEEqbog==") - require.NoError(t, err) - tests := []struct { - name, filename, wantPath, wantAncestry string - content []byte - }{ - {"plain", "notes.txt", "notes.txt", "", text}, - {"binary", "program", "program", "", binary}, - {"ZIP", "bundle.zip", "config/secret.txt", "bundle.zip", - releaseTestZip(t, releaseTestFile{"config/secret.txt", text})}, - {"tar", "bundle.tar", "config/secret.txt", "bundle.tar", - releaseTestTar(t, releaseTestFile{"config/secret.txt", text})}, - {"tar gzip", "bundle.tar.gz", "config/secret.txt", "bundle.tar.gz", inner}, - {"tgz", "bundle.tgz", "config/secret.txt", "bundle.tgz", inner}, - {"gzip", "notes.txt.gz", "notes.txt", "notes.txt.gz", releaseTestGzip(t, text)}, - {"bzip2", "notes.txt.bz2", "notes.txt", "notes.txt.bz2", bzip}, - {"nested ZIP tar gzip", "outer.zip", "config/secret.txt", "outer.zip -> packages/inner.tar.gz", - releaseTestZip(t, releaseTestFile{"packages/inner.tar.gz", inner})}, - {"nested ZIP", "outer.zip", "config/secret.txt", "outer.zip -> packages/inner.zip", - releaseTestZip(t, releaseTestFile{"packages/inner.zip", - releaseTestZip(t, releaseTestFile{"config/secret.txt", text})})}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - opts := releaseTestOptions() - opts.VerifyCredentials = true - opts.GitLabURL = "https://gitlab.example.test" - var reported int - finding := scanner.Finding{Text: string(text)} - opts.ReportFinding = func(actual scanner.Finding, file, ancestry string) { - reported++ - assert.Equal(t, finding, actual) - assert.Equal(t, tt.wantPath, file) - assert.Equal(t, tt.wantAncestry, ancestry) - } - p := releaseProcessor{opts: opts, detect: func(content []byte, threads int, verify bool, options engine.DetectionOptions) ([]scanner.Finding, error) { - assert.Contains(t, string(content), string(text)) - assert.NotContains(t, string(content), "\x00") - assert.Equal(t, 1, threads) - assert.True(t, verify) - assert.Equal(t, tt.wantPath, options.Path) - assert.Equal(t, opts.HitTimeout, options.Timeout) - assert.Equal(t, opts.GitLabURL, options.GitLabURL) - return []scanner.Finding{finding}, nil - }} - require.NoError(t, p.process(tt.content, tt.filename, nil, 0)) - assert.Equal(t, 1, reported) - }) - } -} - -func TestReleaseAssetByteLimits(t *testing.T) { - plain := []byte("plain release text") - opts := releaseTestOptions() - opts.MaxBytes = int64(len(plain)) - require.NoError(t, ProcessReleaseAsset(plain, "plain.txt", opts)) - opts.MaxBytes-- - require.ErrorIs(t, ProcessReleaseAsset(plain, "plain.txt", opts), ErrReleaseAssetTooLarge) - opts.MaxBytes = 0 - require.ErrorContains(t, ProcessReleaseAsset(nil, "plain.txt", opts), "positive") - opts.MaxBytes = -1 - require.ErrorContains(t, ProcessReleaseAsset(nil, "plain.txt", opts), "positive") - - // Use compressible data so the expanded limit, not the input limit, is hit. - payload := bytes.Repeat([]byte("harmless content\n"), 512) - for _, kind := range []string{"zip", "gz"} { - t.Run(kind, func(t *testing.T) { - var content []byte - if kind == "zip" { - content = releaseTestZip(t, releaseTestFile{"one.txt", payload}) - } else { - content = releaseTestGzip(t, payload) - } - opts := releaseTestOptions() - opts.MaxBytes = int64(len(payload)) - require.NoError(t, ProcessReleaseAsset(content, "asset."+kind, opts)) - opts.MaxBytes-- - require.ErrorIs(t, ProcessReleaseAsset(content, "asset."+kind, opts), ErrReleaseAssetTooLarge) - }) - } - t.Run("cumulative siblings", func(t *testing.T) { - content := releaseTestZip(t, releaseTestFile{"one.txt", payload}, releaseTestFile{"two.txt", payload}) - opts := releaseTestOptions() - opts.MaxBytes = int64(2 * len(payload)) - require.NoError(t, ProcessReleaseAsset(content, "asset.zip", opts)) - opts.MaxBytes-- - require.ErrorIs(t, ProcessReleaseAsset(content, "asset.zip", opts), ErrReleaseAssetTooLarge) - }) - t.Run("cumulative nested archives", func(t *testing.T) { - inner := releaseTestZip(t, releaseTestFile{"one.txt", payload}) - content := releaseTestZip(t, releaseTestFile{"inner.zip", inner}) - opts := releaseTestOptions() - opts.MaxBytes = int64(len(inner) + len(payload)) - require.NoError(t, ProcessReleaseAsset(content, "outer.zip", opts)) - opts.MaxBytes-- - require.ErrorIs(t, ProcessReleaseAsset(content, "outer.zip", opts), ErrReleaseAssetTooLarge) - }) - t.Run("tar gzip includes intermediate stream", func(t *testing.T) { - inner := releaseTestTar(t, releaseTestFile{"one.txt", payload}) - content := releaseTestGzip(t, inner) - opts := releaseTestOptions() - opts.MaxBytes = int64(len(inner) + len(payload)) - require.NoError(t, ProcessReleaseAsset(content, "asset.tar.gz", opts)) - opts.MaxBytes-- - require.ErrorIs(t, ProcessReleaseAsset(content, "asset.tar.gz", opts), ErrReleaseAssetTooLarge) - }) -} - -func TestReleaseAssetDepth(t *testing.T) { - content := []byte("harmless release text") - for depth := 1; depth <= 11; depth++ { - name := "inner.txt" - if depth > 1 { - name = "inner.zip" - } - content = releaseTestZip(t, releaseTestFile{name, content}) - err := ProcessReleaseAsset(content, "outer.zip", releaseTestOptions()) - if depth <= 10 { - require.NoError(t, err, "depth %d", depth) - } else { - require.ErrorContains(t, err, "maximum nesting depth") - require.NotErrorIs(t, err, ErrReleaseAssetTooLarge) - } - } -} - -func TestReleaseAssetErrors(t *testing.T) { - goodGzip := releaseTestGzip(t, []byte("release content")) - corruptGzip := bytes.Clone(goodGzip) - corruptGzip[len(corruptGzip)-1] ^= 0xff - badTar := releaseTestTar(t, releaseTestFile{"file.txt", []byte("content")}) - badTar[0] ^= 0xff - truncatedTar := releaseTestTar(t, releaseTestFile{"file.txt", bytes.Repeat([]byte("a"), 1024)})[:600] - innerBadZip := releaseTestZip(t, releaseTestFile{"inner.zip", []byte("invalid ZIP")}) - badZip := releaseTestZip(t, releaseTestFile{"text.txt", []byte("content")}) - zr, err := zip.NewReader(bytes.NewReader(badZip), int64(len(badZip))) - require.NoError(t, err) - offset, err := zr.File[0].DataOffset() - require.NoError(t, err) - badZip[offset] ^= 0xff - for _, tt := range []struct { - name string - content []byte - }{ - {"broken.zip", []byte("not a zip")}, - {"corrupt.zip", badZip}, - {"broken.tar", []byte("not a tar")}, - {"empty.tar", nil}, - {"badheader.tar", badTar}, - {"truncated.tar", truncatedTar}, - {"broken.gz", []byte("not a gzip")}, - {"checksum.gz", corruptGzip}, - {"truncated.gz", goodGzip[:len(goodGzip)-3]}, - {"broken.bz2", []byte("not bzip2")}, - {"unsupported.7z", []byte("unsupported archive")}, - {"unsupported.xz", []byte("unsupported archive")}, - {"unsupported.rar", []byte("unsupported archive")}, - {"unsupported-without-extension", []byte("7z\xbc\xaf\x27\x1c")}, - {"nested.zip", innerBadZip}, - } { - t.Run(tt.name, func(t *testing.T) { - err := ProcessReleaseAsset(tt.content, tt.name, releaseTestOptions()) - require.Error(t, err) - require.NotErrorIs(t, err, ErrReleaseAssetTooLarge) - }) - } - t.Run("detection timeout", func(t *testing.T) { - opts := releaseTestOptions() - opts.HitTimeout = -time.Second - require.ErrorContains(t, ProcessReleaseAsset([]byte("some content"), "notes.txt", opts), "timed out") - }) - t.Run("detection error propagation", func(t *testing.T) { - want := errors.New("detection failed") - p := releaseProcessor{opts: releaseTestOptions(), detect: func([]byte, int, bool, engine.DetectionOptions) ([]scanner.Finding, error) { - return nil, want - }} - require.ErrorIs(t, p.process([]byte("content"), "notes.txt", nil, 0), want) - }) - t.Run("missing reporter", func(t *testing.T) { - opts := releaseTestOptions() - opts.ReportFinding = nil - require.ErrorContains(t, ProcessReleaseAsset(nil, "empty.txt", opts), "reporter") - }) -} - -func TestReleaseExpandedStreamingLimit(t *testing.T) { - p := releaseProcessor{opts: releaseTestOptions()} - p.opts.MaxBytes = 16 - r := bytes.NewReader(bytes.Repeat([]byte("x"), 1024)) - _, err := p.readExpanded(r) - require.ErrorIs(t, err, ErrReleaseAssetTooLarge) - require.Equal(t, int64(17), r.Size()-int64(r.Len()), "consume at most the budget plus one probe byte") - require.Equal(t, int64(16), p.expanded) - - p = releaseProcessor{opts: releaseTestOptions()} - p.opts.MaxBytes = 4 - content, err := p.readExpanded(strings.NewReader("1234")) - require.NoError(t, err) - require.Equal(t, "1234", string(content)) - content, err = p.readExpanded(strings.NewReader("")) - require.NoError(t, err) - require.Empty(t, content) - _, err = p.readExpanded(strings.NewReader("x")) - require.ErrorIs(t, err, ErrReleaseAssetTooLarge) - - p = releaseProcessor{opts: releaseTestOptions()} - _, err = p.readExpanded(io.MultiReader(strings.NewReader("data"), releaseErrorReader{})) - require.ErrorIs(t, err, io.ErrUnexpectedEOF) -} - -type releaseErrorReader struct{} - -func (releaseErrorReader) Read([]byte) (int, error) { - return 0, io.ErrUnexpectedEOF -} - -func TestReleaseEmptyArchivesAndDirectories(t *testing.T) { - for _, tt := range []struct { - name string - content []byte - }{ - {"empty.zip", releaseTestZip(t)}, - {"empty.tar", releaseTestTar(t)}, - {"root.zip", releaseTestZip(t, releaseTestFile{"./", nil}, releaseTestFile{"folder/", nil})}, - } { - t.Run(tt.name, func(t *testing.T) { - require.NoError(t, ProcessReleaseAsset(tt.content, tt.name, releaseTestOptions())) - }) - } -} - -func TestReleaseAssetUnsafeMembers(t *testing.T) { - for _, name := range []string{"../secret.txt", "/secret.txt", `C:\secret.txt`, `..\secret.txt`} { - t.Run(fmt.Sprintf("ZIP/%s", name), func(t *testing.T) { - content := releaseTestZip(t, releaseTestFile{name, []byte("secret")}) - require.ErrorContains(t, ProcessReleaseAsset(content, "outer.zip", releaseTestOptions()), "unsafe") - }) - t.Run(fmt.Sprintf("tar/%s", name), func(t *testing.T) { - content := releaseTestTar(t, releaseTestFile{name, []byte("secret")}) - require.ErrorContains(t, ProcessReleaseAsset(content, "outer.tar", releaseTestOptions()), "unsafe") - }) - } - t.Run("tar symlink", func(t *testing.T) { - var buf bytes.Buffer - w := tar.NewWriter(&buf) - require.NoError(t, w.WriteHeader(&tar.Header{Name: "link", Typeflag: tar.TypeSymlink, Linkname: "/etc/passwd"})) - require.NoError(t, w.Close()) - require.ErrorContains(t, ProcessReleaseAsset(buf.Bytes(), "outer.tar", releaseTestOptions()), "unsupported") - }) -} - -func TestReleaseAssetRealEngineFinding(t *testing.T) { - opts := releaseTestOptions() - var found bool - opts.ReportFinding = func(f scanner.Finding, filename, ancestry string) { - if strings.Contains(f.Text, "glpat-zxywvutsrqponmlkjihg") { - found = true - assert.Equal(t, "secrets/config.txt", filename) - assert.Equal(t, "release.zip", ancestry) - } - } - content := releaseTestZip(t, releaseTestFile{"secrets/config.txt", []byte("GITLAB_TOKEN=glpat-zxywvutsrqponmlkjihg")}) - require.NoError(t, ProcessReleaseAsset(content, "release.zip", opts)) - require.True(t, found, "public processor must invoke the real detection engine and reporter") -} diff --git a/pkg/scanner/artifact/artifact.go b/pkg/scanner/artifact/artifact.go index 6ea4543f..aec26577 100644 --- a/pkg/scanner/artifact/artifact.go +++ b/pkg/scanner/artifact/artifact.go @@ -1,10 +1,14 @@ package artifact import ( + "bytes" + "errors" + "fmt" "os" - "path" + "path/filepath" "strings" "time" + "unicode/utf8" "github.com/CompassSecurity/pipeleek/pkg/archive" "github.com/CompassSecurity/pipeleek/pkg/format" @@ -18,14 +22,61 @@ import ( var skippableDirectoryNames = []string{"node_modules", ".yarn", ".yarn-cache", ".npm", "venv", "vendor", ".go/pkg/mod/"} -func DetectFileHits(content []byte, jobWebUrl string, jobName string, fileName string, archiveName string, enableSecretsVerification bool, hitTimeout time.Duration) { - findings, err := engine.DetectHitsWithOptions(content, 1, enableSecretsVerification, engine.DetectionOptions{Path: fileName, Timeout: hitTimeout}) +type ScanOptions struct { + VerifyCredentials bool + HitTimeout time.Duration + GitLabURL string + ReportFinding func(types.Finding, string, string) +} + +// IsArchive excludes executable and document formats included in filetype.IsArchive. +func IsArchive(content []byte) bool { + kind, _ := filetype.Match(content) + switch kind.Extension { + case "zip", "tar", "gz", "bz2", "7z", "rar", "xz", "zst", "lz", "lz4", "ar", "deb", "rpm", "cab", "iso": + return true + } + return false +} + +func ScanFile(content []byte, filename string, opts ScanOptions) error { + if opts.ReportFinding == nil { + return errors.New("artifact finding reporter is required") + } + return scanFile(content, filename, "", opts, 1) +} + +func scanFile(content []byte, filename, ancestry string, opts ScanOptions, depth int) error { + if IsArchive(content) { + return scanArchive(filename, content, ancestry, opts, depth) + } + return detectFile(content, filename, ancestry, opts) +} + +func detectFile(content []byte, filename, ancestry string, opts ScanOptions) error { + if !utf8.Valid(content) || bytes.ContainsRune(content, '\x00') { + content = archive.ExtractPrintableStrings(content, archive.MinStringLength) + } + findings, err := engine.DetectHitsWithOptions(content, 1, opts.VerifyCredentials, + engine.DetectionOptions{Path: filename, GitLabURL: opts.GitLabURL, Timeout: opts.HitTimeout}) if err != nil { - log.Debug().Err(err).Str("job", jobWebUrl).Msg("Failed detecting secrets") - return + return err } for _, finding := range findings { - ReportFinding(finding, jobWebUrl, jobName, fileName, archiveName) + opts.ReportFinding(finding, filename, ancestry) + } + return nil +} + +func DetectFileHits(content []byte, jobWebUrl string, jobName string, fileName string, archiveName string, enableSecretsVerification bool, hitTimeout time.Duration) { + opts := ScanOptions{ + VerifyCredentials: enableSecretsVerification, HitTimeout: hitTimeout, + ReportFinding: func(finding types.Finding, file, ancestry string) { + ReportFinding(finding, jobWebUrl, jobName, file, ancestry) + }, + } + if err := detectFile(content, fileName, archiveName, opts); err != nil { + log.Debug().Err(err).Str("job", jobWebUrl).Msg("Failed detecting secrets") } } @@ -34,42 +85,49 @@ func HandleArchiveArtifact(archivefileName string, content []byte, jobWebUrl str } func HandleArchiveArtifactWithDepth(archivefileName string, content []byte, jobWebUrl string, jobName string, enableSecretsVerification bool, hitTimeout time.Duration, depth int) { + opts := ScanOptions{ + VerifyCredentials: enableSecretsVerification, HitTimeout: hitTimeout, + ReportFinding: func(finding types.Finding, file, ancestry string) { + ReportFinding(finding, jobWebUrl, jobName, file, ancestry) + }, + } + if err := scanFile(content, archivefileName, "", opts, depth); err != nil { + log.Error().Err(err).Str("file", archivefileName).Msg("Failed scanning artifact") + } +} + +func scanArchive(archivefileName string, content []byte, ancestry string, opts ScanOptions, depth int) error { if depth > 10 { - log.Debug().Str("file", archivefileName).Int("recursionDepth", depth).Msg("Max archive recursion depth reached, skipping further extraction") - return + return errors.New("maximum archive nesting depth exceeded") } for _, skipKeyword := range skippableDirectoryNames { if strings.Contains(archivefileName, skipKeyword) { log.Debug().Str("file", archivefileName).Str("keyword", skipKeyword).Msg("Skipped archive due to blocklist entry") - return + return nil } } fileType, err := filetype.Get(content) if err != nil { - log.Error().Stack().Err(err).Msg("Cannot determine file type") - return + return fmt.Errorf("determine archive type: %w", err) } tmpArchiveFile, err := os.CreateTemp("", "pipeleek-artifact-archive-*."+fileType.Extension) if err != nil { - log.Error().Stack().Err(err).Msg("Cannot create artifact archive temp file") - return + return fmt.Errorf("create archive file: %w", err) } defer func() { _ = tmpArchiveFile.Close() }() _, err = tmpArchiveFile.Write(content) if err != nil { - log.Error().Stack().Err(err).Msg("Failed writing archive to disk") - return + return fmt.Errorf("write archive file: %w", err) } defer func() { _ = os.Remove(tmpArchiveFile.Name()) }() tmpArchiveFilesDirectory, err := os.MkdirTemp("", "pipeleek-artifact-archive-out-") if err != nil { - log.Error().Stack().Err(err).Msg("Cannot create artifact archive temp directory") - return + return fmt.Errorf("create archive directory: %w", err) } defer func() { _ = os.RemoveAll(tmpArchiveFilesDirectory) }() @@ -81,46 +139,39 @@ func HandleArchiveArtifactWithDepth(archivefileName string, content []byte, jobW } _, files, _, err := xtractr.ExtractFile(x) - if err != nil || files == nil { - if err != nil { - log.Debug().Err(err).Msg("Unable to handle archive in artifacts, extracting strings from unknown archive type") - } else { - log.Debug().Msg("No files extracted from archive in artifacts, extracting strings from unknown archive type") - } - - // When archive extraction fails, extract printable strings and scan them - // This is useful for unknown archive formats or binary files - extractedStrings := archive.ExtractPrintableStrings(content, archive.MinStringLength) - if len(extractedStrings) > 0 { - log.Trace().Str("file", archivefileName).Int("stringBytes", len(extractedStrings)).Msg("Extracted strings from unknown archive type") - DetectFileHits(extractedStrings, jobWebUrl, jobName, archivefileName, "", enableSecretsVerification, hitTimeout) - } - return + if err != nil { + return fmt.Errorf("extract archive %q: %w", archivefileName, err) } + if ancestry != "" { + ancestry += " -> " + } + ancestry += archivefileName + var failures []error for _, fPath := range files { + // TAR returns member names; other extractors return absolute paths. + if !filepath.IsAbs(fPath) { + fPath = filepath.Join(tmpArchiveFilesDirectory, fPath) + } if !format.IsDirectory(fPath) { // #nosec G304 - Reading extracted artifact files from temp directory, path controlled by xtractr library fileBytes, err := os.ReadFile(fPath) if err != nil { - log.Debug().Str("file", fPath).Stack().Str("err", err.Error()).Msg("Cannot read temp artifact archive file content") + failures = append(failures, fmt.Errorf("read archive member: %w", err)) continue } - currentFileName := path.Base(fPath) - - if filetype.IsArchive(fileBytes) { - log.Trace().Str("fileName", currentFileName).Str("parentArchive", archivefileName).Int("depth", depth).Msg("Detected nested archive, recursing") - HandleArchiveArtifactWithDepth(currentFileName, fileBytes, jobWebUrl, jobName, enableSecretsVerification, hitTimeout, depth+1) + currentFileName, err := filepath.Rel(tmpArchiveFilesDirectory, fPath) + if err != nil { + failures = append(failures, err) continue } - - kind, _ := filetype.Match(fileBytes) - if kind == filetype.Unknown { - DetectFileHits(fileBytes, jobWebUrl, jobName, currentFileName, archivefileName, enableSecretsVerification, hitTimeout) + if err := scanFile(fileBytes, filepath.ToSlash(currentFileName), ancestry, opts, depth+1); err != nil { + failures = append(failures, err) } } } + return errors.Join(failures...) } func ReportFinding(finding types.Finding, url string, jobName string, fileName string, archiveName string) { diff --git a/pkg/scanner/engine/engine.go b/pkg/scanner/engine/engine.go index 88182544..7a9ca508 100644 --- a/pkg/scanner/engine/engine.go +++ b/pkg/scanner/engine/engine.go @@ -3,6 +3,7 @@ package engine import ( "context" "errors" + "fmt" "slices" "strings" "sync" @@ -21,6 +22,8 @@ import ( var findingsDeduplicationList []string var deduplicationMutex sync.Mutex +var ErrHitTimeout = errors.New("hit detection timed out") + const ( EngineRules = "rules" EngineBetterleaks = "betterleaks" @@ -48,10 +51,10 @@ func DetectHitsWithOptions(text []byte, maxThreads int, enableSecretsVerificatio }() select { case <-ctx.Done(): - return nil, errors.New("hit detection timed out (" + options.Timeout.String() + ")") + return nil, fmt.Errorf("%w (%s)", ErrHitTimeout, options.Timeout) case result := <-result: if ctx.Err() != nil { - return nil, errors.New("hit detection timed out (" + options.Timeout.String() + ")") + return nil, fmt.Errorf("%w (%s)", ErrHitTimeout, options.Timeout) } return result.Findings, result.Error } diff --git a/pkg/scanner/engine/engine_test.go b/pkg/scanner/engine/engine_test.go index 49c0fee9..09186e25 100644 --- a/pkg/scanner/engine/engine_test.go +++ b/pkg/scanner/engine/engine_test.go @@ -2,6 +2,7 @@ package engine import ( "context" + "errors" "fmt" "net/http" "net/http/httptest" @@ -90,6 +91,9 @@ func TestDetectHits_ExplicitTimeout(t *testing.T) { if err == nil { t.Fatal("Expected timeout error with 1ns timeout, but got nil") } + if !errors.Is(err, ErrHitTimeout) { + t.Errorf("Expected detectable hit timeout, got %v", err) + } // Verify the error message contains the configured timeout value expectedTimeoutStr := shortTimeout.String() // "1ns" diff --git a/tests/e2e/gitlab/release/scan_test.go b/tests/e2e/gitlab/release/scan_test.go index 766a9fe3..e9e48808 100644 --- a/tests/e2e/gitlab/release/scan_test.go +++ b/tests/e2e/gitlab/release/scan_test.go @@ -206,7 +206,9 @@ func TestReleaseScanTarAndBinary(t *testing.T) { require.NoError(t, gz.Close()) f := newFixture(t, links("/archive.tar.gz", "/binary"), func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/binary" { - _, _ = w.Write(append([]byte{0x7f, 'E', 'L', 'F', 0, 1, 0}, []byte("DATABASE_PASSWORD=binaryfixture\n\x00")...)) + header := make([]byte, 64) + copy(header, "\x7fELF") + _, _ = w.Write(append(header, []byte("DATABASE_PASSWORD=binaryfixture\n\x00")...)) } else { _, _ = w.Write(archive.Bytes()) } @@ -242,7 +244,7 @@ func TestReleaseScanExternalRedirectDoesNotLeakCredentials(t *testing.T) { func TestReleaseScanPartialFailures(t *testing.T) { f := newFixture(t, links("/denied", "/good"), func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/denied" { - w.WriteHeader(http.StatusForbidden) + w.WriteHeader(http.StatusUnauthorized) } else { plainAsset(w, r) } @@ -253,6 +255,16 @@ func TestReleaseScanPartialFailures(t *testing.T) { assert.Contains(t, output, `"failures":1`) assert.Contains(t, output, `"scannedAssets":1`) assert.Contains(t, output, "incomplete") + for _, line := range strings.Split(strings.TrimSpace(output), "\n") { + var event struct { + Level string `json:"level"` + Message string `json:"message"` + } + require.NoError(t, json.Unmarshal([]byte(line), &event)) + if event.Message == "Failed scanning release asset" { + assert.Equal(t, "error", event.Level) + } + } } func TestReleaseScanSizeAndTimeout(t *testing.T) { @@ -276,9 +288,27 @@ func TestReleaseScanSizeAndTimeout(t *testing.T) { require.NoError(t, err, output) } assert.Contains(t, output, tc.want) + if tc.name == "timeout" { + found := false + for _, line := range strings.Split(strings.TrimSpace(output), "\n") { + var event struct { + Level string `json:"level"` + Message string `json:"message"` + } + require.NoError(t, json.Unmarshal([]byte(line), &event)) + if event.Message == "Failed scanning release asset" { + found = true + assert.Equal(t, "warn", event.Level) + } + } + assert.True(t, found, "expected asset timeout warning") + assert.Contains(t, output, `"failures":1`) + } if tc.name == "size" { assert.NotContains(t, output, `"level":"hit"`) + assert.NotContains(t, output, "Skipped release asset exceeding maximum size") } + if tc.name == "confidence" { assert.NotContains(t, output, `"confidence":"medium"`) assert.Contains(t, output, `"confidence":"low"`) @@ -287,6 +317,58 @@ func TestReleaseScanSizeAndTimeout(t *testing.T) { } } +func TestReleaseScanSizeSkipDebugLog(t *testing.T) { + f := newFixture(t, links("/asset"), plainAsset) + output, err := f.run(t, "--release-limit", "1", "--max-artifact-size", "1B", "--log-level", "debug") + require.NoError(t, err, output) + assert.Contains(t, output, `"level":"debug"`) + assert.Contains(t, output, "Skipped release asset exceeding maximum size") + assert.Contains(t, output, `"skippedAssets":1`) +} + +func TestReleaseScanUnavailableAssetsAreSilent(t *testing.T) { + f := newFixture(t, links("/forbidden", "/missing", "/good"), func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/forbidden": + w.WriteHeader(http.StatusForbidden) + case "/missing": + w.WriteHeader(http.StatusNotFound) + default: + plainAsset(w, r) + } + }) + output, err := f.run(t, "--release-limit", "1") + require.NoError(t, err, output) + assert.NotContains(t, output, "Failed scanning release asset") + assert.NotContains(t, output, `"level":"error"`) + assert.NotContains(t, output, `"level":"warn"`) + assert.Contains(t, output, `"skippedAssets":2`) + assert.Contains(t, output, `"scannedAssets":1`) + assert.Contains(t, output, `"failures":0`) +} + +func TestReleaseScanForbiddenListingIsIgnored(t *testing.T) { + testutil.UseTestRules(t) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch r.URL.Path { + case "/api/v4/projects": + _, _ = fmt.Fprint(w, `[{"id":1,"path_with_namespace":"group/forbidden"}]`) + case "/api/v4/projects/1/releases": + w.WriteHeader(http.StatusForbidden) + _, _ = fmt.Fprint(w, `{"message":"403 Forbidden"}`) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + output, err := (fixture{server: srv}).run(t) + require.NoError(t, err, output) + assert.NotContains(t, output, "Failed discovering release assets") + assert.NotContains(t, output, `"level":"error"`) + assert.Contains(t, output, `"failures":0`) +} + func TestReleaseScanWorkerBound(t *testing.T) { var active, peak atomic.Int64 f := newFixture(t, links("/one", "/two", "/three", "/four", "/five"), func(w http.ResponseWriter, r *http.Request) { From d31f1def0ccd885b0f9c3d71c85891b260b7f267 Mon Sep 17 00:00:00 2001 From: frjcomp <107982661+frjcomp@users.noreply.github.com> Date: Tue, 20 Oct 2026 09:16:59 +0000 Subject: [PATCH 4/7] refactor(gitlab): reuse release scan infrastructure Reuse SDK asset links, bounded concurrency and shared artifact scanning; simplify command setup and URL handling while retaining same-origin download authentication. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/guides/gitlab.md | 8 +- internal/cmd/gitlab/release/scan/scan.go | 17 ++- pkg/gitlab/release/scan/download.go | 64 ++--------- pkg/gitlab/release/scan/download_test.go | 132 +++++++++-------------- pkg/gitlab/release/scan/scanner.go | 85 +++++++-------- pkg/gitlab/release/scan/scanner_test.go | 83 +++++++------- pkg/scan/artifact/processor.go | 12 +-- pkg/scan/artifact/processor_test.go | 5 +- pkg/scanner/artifact/artifact.go | 8 +- tests/e2e/gitlab/release/scan_test.go | 54 +++++++++- 10 files changed, 207 insertions(+), 261 deletions(-) diff --git a/docs/guides/gitlab.md b/docs/guides/gitlab.md index 77bead42..72e1db1f 100644 --- a/docs/guides/gitlab.md +++ b/docs/guides/gitlab.md @@ -127,11 +127,9 @@ pipeleek gl release scan --url https://gitlab.example.com --token [redacted] \ `--tag` requires `--repo`; alternatively, `--release-limit N` scans the newest N published releases per project (`0` means all). `--include-sources` adds one generated source ZIP per release. -External HTTP(S) assets are included, but GitLab credentials stay on the configured -origin, so ensure linked hosts are authorized scan targets. -`--max-artifact-size` limits downloaded bytes, not extracted archive size; -oversized assets and HTTP 403/404 downloads are skipped, while other download or processing failures produce a -nonzero exit after remaining assets are scanned. +Downloads send the token only to the configured GitLab origin, not external +assets or redirects; ensure linked hosts are authorized scan targets. +`--max-artifact-size` limits downloaded bytes, not extracted archive size. ## Secret Detection in Pipelines And Artifacts diff --git a/internal/cmd/gitlab/release/scan/scan.go b/internal/cmd/gitlab/release/scan/scan.go index 5bf92fcc..9a690e68 100644 --- a/internal/cmd/gitlab/release/scan/scan.go +++ b/internal/cmd/gitlab/release/scan/scan.go @@ -29,7 +29,8 @@ func NewScanCmd() *cobra.Command { Use: "scan", Short: "Scan GitLab release assets for secrets", Long: `Scan linked assets of published GitLab releases for secrets. By default, scan all releases of all token-visible projects, including HTTP(S) -assets hosted outside GitLab. GitLab credentials are never sent to other origins. +assets hosted outside GitLab. Downloads send the token only to the configured +GitLab origin; external assets and redirects do not receive it. Use --include-sources to also scan one generated source ZIP per release. Assets exceeding --max-artifact-size (downloaded bytes) are skipped. @@ -47,7 +48,7 @@ pipeleek gl release scan --url https://gitlab.example.com --token [redacted] --r pipeleek gl release scan --url https://gitlab.example.com --token [redacted] --namespace group --release-limit 5`, Args: cobra.NoArgs, Run: Scan, } - common := config.DefaultCommonScanOptions() + var common config.CommonScanOptions flags.AddCommonScanFlagsNoArtifacts(cmd, &common) cmd.Flags().String("max-artifact-size", "500Mb", "Maximum downloaded bytes per asset; extracted archive sizes are not limited") cmd.Flags().StringP("repo", "r", "", "Single repository to scan, format: namespace/repo") @@ -101,15 +102,13 @@ func Scan(cmd *cobra.Command, args []string) { } s := releasescan.NewScanner(opts) logging.RegisterStatusHook(s.Status) - err = s.Scan() - if ui != nil { - if err == nil { - ui.Wait() - } else { + if err := s.Scan(); err != nil { + if ui != nil { _ = ui.Close() } - } - if err != nil { log.Fatal().Err(err).Msg("Release scan failed") } + if ui != nil { + ui.Wait() + } } diff --git a/pkg/gitlab/release/scan/download.go b/pkg/gitlab/release/scan/download.go index fb5489ca..240bb142 100644 --- a/pkg/gitlab/release/scan/download.go +++ b/pkg/gitlab/release/scan/download.go @@ -14,34 +14,25 @@ import ( var errAssetTooLarge = errors.New("asset exceeds maximum size") var errAssetUnavailable = errors.New("asset is forbidden or not found") -func downloadAsset(u, base *url.URL, token string, limit int64) ([]byte, error) { +func downloadAsset(rawURL string, base *url.URL, token string, limit int64) ([]byte, error) { client := httpclient.GetPipeleekStandardHTTPClient() defer client.CloseIdleConnections() authorize := func(req *http.Request) { req.Header.Del("PRIVATE-TOKEN") - req.Header.Del("Authorization") - req.Header.Del("Cookie") - req.Header.Del("Referer") if sameOrigin(base, req.URL) { req.Header.Set("PRIVATE-TOKEN", token) } } client.CheckRedirect = func(req *http.Request, via []*http.Request) error { if len(via) >= 10 { - return errors.New("too many asset redirects") - } - if err := validateAssetURL(req.URL); err != nil { - return err - } - if via[len(via)-1].URL.Scheme == "https" && req.URL.Scheme == "http" { - return errors.New("asset redirect downgrades HTTPS to HTTP") + return errors.New("stopped after 10 redirects") } authorize(req) return nil } - req, err := http.NewRequest(http.MethodGet, u.String(), nil) + req, err := http.NewRequest(http.MethodGet, rawURL, nil) if err != nil { - return nil, errors.New("cannot create asset request") + return nil, errors.New("invalid asset request URL") } authorize(req) resp, err := client.Do(req) @@ -69,51 +60,12 @@ func downloadAsset(u, base *url.URL, token string, limit int64) ([]byte, error) return content, nil } -func validateAssetURL(u *url.URL) error { - if (u.Scheme != "http" && u.Scheme != "https") || u.Hostname() == "" { - return errors.New("asset URL must use HTTP(S) and include a host") - } - if u.User != nil { - return errors.New("asset URL must not contain user credentials") - } - return nil -} - func sameOrigin(a, b *url.URL) bool { - port := func(u *url.URL) string { - if p := u.Port(); p != "" { - return p - } + host := func(u *url.URL) string { if u.Scheme == "https" { - return "443" + return strings.TrimSuffix(u.Host, ":443") } - return "80" + return strings.TrimSuffix(u.Host, ":80") } - return strings.EqualFold(a.Scheme, b.Scheme) && - strings.EqualFold(a.Hostname(), b.Hostname()) && port(a) == port(b) -} - -func resolveAssetURL(base *url.URL, raw string) (*url.URL, error) { - if strings.TrimSpace(raw) == "" { - return nil, errors.New("empty asset URL") - } - u, err := url.Parse(raw) - if err != nil { - return nil, errors.New("invalid asset URL") - } - u = base.ResolveReference(u) - if err := validateAssetURL(u); err != nil { - return nil, err - } - return u, nil -} - -// Asset URLs can contain signed query parameters; never include those in output. -func displayURL(u *url.URL) string { - safe := *u - safe.User = nil - safe.RawQuery = "" - safe.ForceQuery = false - safe.Fragment = "" - return safe.String() + return strings.EqualFold(a.Scheme, b.Scheme) && strings.EqualFold(host(a), host(b)) } diff --git a/pkg/gitlab/release/scan/download_test.go b/pkg/gitlab/release/scan/download_test.go index 0c2ef9d9..2120349b 100644 --- a/pkg/gitlab/release/scan/download_test.go +++ b/pkg/gitlab/release/scan/download_test.go @@ -13,50 +13,33 @@ import ( "github.com/stretchr/testify/require" ) -type testAssetDownloader struct { - base *url.URL - limit int64 -} - -func (d testAssetDownloader) resolve(raw string) (*url.URL, error) { - return resolveAssetURL(d.base, raw) -} - -func (d testAssetDownloader) download(u *url.URL) ([]byte, error) { - return downloadAsset(u, d.base, "test-token", d.limit) -} - -func testDownloader(t *testing.T, base string, limit int64) testAssetDownloader { - t.Helper() - u, err := url.Parse(base) - require.NoError(t, err) - return testAssetDownloader{u, limit} -} - func TestDownloadCredentials(t *testing.T) { var received []string external := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { received = append(received, r.Header.Get("PRIVATE-TOKEN")) assert.Empty(t, r.Header.Get("Authorization")) assert.Empty(t, r.Header.Get("Cookie")) - assert.Empty(t, r.Header.Get("Referer")) _, _ = w.Write([]byte("external")) })) defer external.Close() instance := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "test-token", r.Header.Get("PRIVATE-TOKEN")) + assert.Empty(t, r.Header.Get("Authorization")) if r.URL.Path == "/redirect" { http.Redirect(w, r, external.URL, http.StatusFound) return } + if r.URL.Path == "/internal-redirect" { + http.Redirect(w, r, "/asset", http.StatusFound) + return + } _, _ = w.Write([]byte("internal")) })) defer instance.Close() - d := testDownloader(t, instance.URL, 100) - for _, target := range []string{instance.URL, external.URL, instance.URL + "/redirect"} { - u, err := d.resolve(target) - require.NoError(t, err) - _, err = d.download(u) + base, err := url.Parse(instance.URL) + require.NoError(t, err) + for _, target := range []string{instance.URL, external.URL, instance.URL + "/redirect", instance.URL + "/internal-redirect"} { + _, err := downloadAsset(target, base, "test-token", 100) require.NoError(t, err) } assert.Equal(t, []string{"", ""}, received) @@ -88,10 +71,7 @@ func TestDownloadLimitsAndFailures(t *testing.T) { _, _ = w.Write([]byte(tc.body)) })) defer srv.Close() - d := testDownloader(t, srv.URL, 5) - u, err := d.resolve("/asset") - require.NoError(t, err) - content, err := d.download(u) + content, err := downloadAsset(srv.URL+"/asset", &url.URL{}, "", 5) if tc.want == nil { require.NoError(t, err) assert.Equal(t, tc.body, string(content)) @@ -112,52 +92,26 @@ func TestDownloadTruncatedBody(t *testing.T) { _, _ = w.Write([]byte("123")) })) defer srv.Close() - d := testDownloader(t, srv.URL, 5) - u, err := d.resolve("/asset") - require.NoError(t, err) - _, err = d.download(u) + _, err := downloadAsset(srv.URL+"/asset", &url.URL{}, "", 5) require.ErrorContains(t, err, "unexpected EOF") } -func TestAssetURLs(t *testing.T) { - d := testDownloader(t, "https://gitlab.example.com/subpath/", 10) - u, err := d.resolve("uploads/asset?token=sensitive#fragment") +func TestDownloadDowngradeDoesNotLeakToken(t *testing.T) { + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Empty(t, r.Header.Get("Private-Token")) + _, _ = w.Write([]byte("asset")) + })) + defer target.Close() + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "test-token", r.Header.Get("Private-Token")) + http.Redirect(w, r, target.URL, http.StatusFound) + })) + defer srv.Close() + base, err := url.Parse(srv.URL) require.NoError(t, err) - assert.Equal(t, "https://gitlab.example.com/subpath/uploads/asset", displayURL(u)) - for _, raw := range []string{"", "ftp://example.com/file", "file:///etc/passwd", "https://user:password@example.com/file", "%invalid"} { - _, err := d.resolve(raw) - require.Error(t, err, raw) - assert.NotContains(t, err.Error(), "password") - } - for _, tc := range []struct { - target string - same bool - }{ - {"https://GITLAB.example.com:443/other", true}, - {"https://gitlab.example.com:8443/other", false}, - {"https://sub.gitlab.example.com/file", false}, - {"http://gitlab.example.com/file", false}, - } { - target, err := url.Parse(tc.target) - require.NoError(t, err) - assert.Equal(t, tc.same, sameOrigin(d.base, target)) - } -} - -func TestRedirectPolicy(t *testing.T) { - for _, target := range []string{"http://example.com/file", "ftp://example.com/file"} { - t.Run(target, func(t *testing.T) { - srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - http.Redirect(w, r, target, http.StatusFound) - })) - defer srv.Close() - d := testDownloader(t, srv.URL, 10) - u, err := d.resolve("/asset") - require.NoError(t, err) - _, err = d.download(u) - require.ErrorContains(t, err, "asset request failed") - }) - } + content, err := downloadAsset(srv.URL, base, "test-token", 10) + require.NoError(t, err) + assert.Equal(t, "asset", string(content)) } func TestDownloadErrorRedactsQuery(t *testing.T) { @@ -165,10 +119,7 @@ func TestDownloadErrorRedactsQuery(t *testing.T) { http.Redirect(w, r, "/loop?signature=sensitive", http.StatusFound) })) defer srv.Close() - d := testDownloader(t, srv.URL, 10) - u, err := d.resolve("/loop?signature=sensitive") - require.NoError(t, err) - _, err = d.download(u) + _, err := downloadAsset(srv.URL+"/loop?signature=sensitive", &url.URL{}, "", 10) require.Error(t, err) assert.NotContains(t, err.Error(), "sensitive") assert.Contains(t, err.Error(), "asset request failed") @@ -180,10 +131,7 @@ func TestDownloadMalformedRedirectRedactsQuery(t *testing.T) { w.WriteHeader(http.StatusFound) })) defer srv.Close() - d := testDownloader(t, srv.URL, 10) - u, err := d.resolve("/asset") - require.NoError(t, err) - _, err = d.download(u) + _, err := downloadAsset(srv.URL+"/asset", &url.URL{}, "", 10) require.Error(t, err) assert.NotContains(t, err.Error(), "sensitive") } @@ -194,9 +142,25 @@ func TestDownloadDoesNotReadOversizedBody(t *testing.T) { _, _ = fmt.Fprint(w, strings.Repeat("x", 100)) })) defer srv.Close() - d := testDownloader(t, srv.URL, 10) - u, err := d.resolve("/large") - require.NoError(t, err) - _, err = d.download(u) + _, err := downloadAsset(srv.URL+"/large", &url.URL{}, "", 10) require.ErrorIs(t, err, errAssetTooLarge) } + +func TestSameOrigin(t *testing.T) { + base, err := url.Parse("https://gitlab.example.com") + require.NoError(t, err) + for _, tc := range []struct { + target string + same bool + }{ + {"https://GITLAB.example.com:443/asset", true}, + {"https://gitlab.example.com/asset", true}, + {"https://gitlab.example.com:8443/asset", false}, + {"https://sub.gitlab.example.com/asset", false}, + {"http://gitlab.example.com/asset", false}, + } { + target, err := url.Parse(tc.target) + require.NoError(t, err) + assert.Equal(t, tc.same, sameOrigin(base, target), tc.target) + } +} diff --git a/pkg/gitlab/release/scan/scanner.go b/pkg/gitlab/release/scan/scanner.go index f0595f25..b2412108 100644 --- a/pkg/gitlab/release/scan/scanner.go +++ b/pkg/gitlab/release/scan/scanner.go @@ -1,26 +1,27 @@ package scan import ( + "context" "errors" "fmt" "math" "net/http" "net/url" "path" - "sync" "sync/atomic" "time" "github.com/CompassSecurity/pipeleek/pkg/config" "github.com/CompassSecurity/pipeleek/pkg/gitlab/util" "github.com/CompassSecurity/pipeleek/pkg/logging" - artifactproc "github.com/CompassSecurity/pipeleek/pkg/scan/artifact" "github.com/CompassSecurity/pipeleek/pkg/scan/result" "github.com/CompassSecurity/pipeleek/pkg/scan/runner" "github.com/CompassSecurity/pipeleek/pkg/scanner" + scannerartifact "github.com/CompassSecurity/pipeleek/pkg/scanner/artifact" "github.com/CompassSecurity/pipeleek/pkg/scanner/engine" "github.com/rs/zerolog" "github.com/rs/zerolog/log" + "github.com/wandb/parallel" gitlab "gitlab.com/gitlab-org/api/client-go" ) @@ -99,6 +100,7 @@ type asset struct { release string name string rawURL string + webURL string } func (s *Scanner) Scan() error { @@ -118,24 +120,20 @@ func (s *Scanner) Scan() error { } func (s *Scanner) scan(git *gitlab.Client, base *url.URL) error { - work := make(chan asset, s.options.Threads) - var workers sync.WaitGroup - for range s.options.Threads { - workers.Go(func() { - for item := range work { - s.scanAsset(base, item) - } + workers := parallel.Limited(context.Background(), s.options.Threads) + enqueue := func(item asset) { + workers.Go(func(context.Context) { + s.scanAsset(base, item) }) } err := s.projectsForScan(git, func(project *gitlab.Project) error { s.projects.Add(1) - if err := s.projectReleases(git, project, work); err != nil { + if err := s.projectReleases(git, project, enqueue); err != nil { s.failures.Add(1) log.Error().Err(err).Str("project", project.PathWithNamespace).Msg("Failed discovering release assets") } return nil }) - close(work) workers.Wait() if err != nil { s.failures.Add(1) @@ -175,14 +173,14 @@ func (s *Scanner) projectsForScan(git *gitlab.Client, visit util.ProjectIterator }, visit) } -func (s *Scanner) projectReleases(git *gitlab.Client, project *gitlab.Project, work chan<- asset) error { +func (s *Scanner) projectReleases(git *gitlab.Client, project *gitlab.Project, visit func(asset)) error { if s.options.Tag != "" { release, _, err := git.Releases.GetRelease(project.ID, s.options.Tag) if err != nil { return fmt.Errorf("fetching release: %w", err) } if published(release) { - s.releaseAssets(git, project, release, work) + s.releaseAssets(project, release, visit) } return nil } @@ -203,7 +201,7 @@ func (s *Scanner) projectReleases(git *gitlab.Client, project *gitlab.Project, w if !published(release) { continue } - s.releaseAssets(git, project, release, work) + s.releaseAssets(project, release, visit) count++ if s.options.ReleaseLimit > 0 && count >= s.options.ReleaseLimit { return nil @@ -220,45 +218,33 @@ func published(release *gitlab.Release) bool { return !release.UpcomingRelease && (release.ReleasedAt == nil || !release.ReleasedAt.After(time.Now())) } -func (s *Scanner) releaseAssets(git *gitlab.Client, project *gitlab.Project, release *gitlab.Release, work chan<- asset) { +func (s *Scanner) releaseAssets(project *gitlab.Project, release *gitlab.Release, visit func(asset)) { s.releases.Add(1) - opts := &gitlab.ListReleaseLinksOptions{ListOptions: gitlab.ListOptions{PerPage: 100, Page: 1}} - for { - links, resp, err := git.ReleaseLinks.ListReleaseLinks(project.ID, release.TagName, opts) - if err != nil { - s.failures.Add(1) - log.Error().Err(err).Str("project", project.PathWithNamespace). - Str("tag", release.TagName).Msg("Failed listing release links") - break - } - for _, link := range links { - work <- asset{project.PathWithNamespace, release.TagName, release.Name, link.Name, link.URL} - } - if resp == nil || resp.NextPage == 0 { - break - } - opts.Page = resp.NextPage + item := asset{project: project.PathWithNamespace, tag: release.TagName, release: release.Name} + for _, link := range release.Assets.Links { + item.name, item.rawURL, item.webURL = link.Name, link.URL, link.DirectAssetURL + visit(item) } - if s.options.IncludeSources { - for _, source := range release.Assets.Sources { - if source.Format == "zip" { - work <- asset{project.PathWithNamespace, release.TagName, release.Name, release.TagName + ".zip", source.URL} - return - } + if !s.options.IncludeSources { + return + } + for _, source := range release.Assets.Sources { + if source.Format == "zip" { + item.name, item.rawURL, item.webURL = release.TagName+".zip", source.URL, "" + visit(item) + return } - s.failures.Add(1) - log.Error().Str("project", project.PathWithNamespace).Str("tag", release.TagName). - Msg("Source ZIP unavailable for release") } + s.failures.Add(1) + log.Error().Str("project", item.project).Str("tag", item.tag).Msg("Source ZIP unavailable for release") } func (s *Scanner) scanAsset(base *url.URL, item asset) { - u, err := resolveAssetURL(base, item.rawURL) + u, err := base.Parse(item.rawURL) if err != nil { - s.assetFailure(item, err) + s.assetFailure(item, errors.New("invalid asset URL")) return } - item.rawURL = displayURL(u) filename := path.Base(u.Path) if filename == "." || filename == "/" { filename = item.name @@ -266,9 +252,12 @@ func (s *Scanner) scanAsset(base *url.URL, item asset) { if item.name == "" { item.name = filename } - content, err := downloadAsset(u, base, s.options.Token, s.options.MaxBytes) + if item.webURL == "" { + item.webURL = base.JoinPath(item.project, "-", "releases", url.PathEscape(item.tag)).String() + } + content, err := downloadAsset(u.String(), base, s.options.Token, s.options.MaxBytes) if err == nil { - _, err = artifactproc.ProcessSingleFile(content, filename, artifactproc.ProcessOptions{ + err = scannerartifact.ScanFile(content, filename, scannerartifact.ScanOptions{ VerifyCredentials: s.options.SecretsVerification, HitTimeout: s.options.HitTimeout, GitLabURL: s.options.URL, ReportFinding: func(finding scanner.Finding, file, archive string) { @@ -277,8 +266,10 @@ func (s *Scanner) scanAsset(base *url.URL, item asset) { sourceType = logging.SecretTypeNestedArchive } result.ReportFindingWithCustomFields(finding, map[string]string{ - "type": string(sourceType), "url": item.rawURL, "project": item.project, - "tag": item.tag, "release": item.release, "asset": item.name, + "type": string(sourceType), + "url": item.webURL, + "project": item.project, + "tag": item.tag, "release": item.release, "asset": item.name, "file": file, "archive": archive, }) }, diff --git a/pkg/gitlab/release/scan/scanner_test.go b/pkg/gitlab/release/scan/scanner_test.go index 0f4c59f4..1b884532 100644 --- a/pkg/gitlab/release/scan/scanner_test.go +++ b/pkg/gitlab/release/scan/scanner_test.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "net/url" "sync/atomic" "testing" "time" @@ -49,7 +50,7 @@ func TestOptionsValidation(t *testing.T) { func TestReleasePaginationAndLimit(t *testing.T) { for _, limit := range []int{0, 1, 2} { t.Run(fmt.Sprint(limit), func(t *testing.T) { - var releasePages, linkPages atomic.Int64 + var releasePages atomic.Int64 srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") switch r.URL.Path { @@ -58,17 +59,11 @@ func TestReleasePaginationAndLimit(t *testing.T) { assert.Equal(t, "released_at", r.URL.Query().Get("order_by")) assert.Equal(t, "desc", r.URL.Query().Get("sort")) if r.URL.Query().Get("page") == "2" { - _, _ = fmt.Fprint(w, `[{"tag_name":"v1"}]`) + _, _ = fmt.Fprint(w, `[{"tag_name":"v1","assets":{"links":[{"name":"binary","url":"https://assets.example.com/binary"}]}}]`) } else { w.Header().Set("X-Next-Page", "2") - _, _ = fmt.Fprint(w, `[{"tag_name":"future","upcoming_release":true},{"tag_name":"v2"}]`) + _, _ = fmt.Fprint(w, `[{"tag_name":"future","upcoming_release":true},{"tag_name":"v2","assets":{"links":[{"name":"binary","url":"https://assets.example.com/binary"}]}}]`) } - case "/api/v4/projects/1/releases/v2/assets/links", "/api/v4/projects/1/releases/v1/assets/links": - linkPages.Add(1) - if r.URL.Query().Get("page") != "2" { - w.Header().Set("X-Next-Page", "2") - } - _, _ = fmt.Fprint(w, `[{"name":"binary","url":"https://assets.example.com/binary"}]`) default: http.NotFound(w, r) } @@ -79,16 +74,20 @@ func TestReleasePaginationAndLimit(t *testing.T) { o := validOptions() o.ReleaseLimit = limit s := NewScanner(o) - work := make(chan asset, 10) - require.NoError(t, s.projectReleases(git, &gitlab.Project{ID: 1, PathWithNamespace: "g/p"}, work)) + var assets []asset + require.NoError(t, s.projectReleases(git, &gitlab.Project{ID: 1, PathWithNamespace: "g/p"}, func(item asset) { + assets = append(assets, item) + })) expected := int64(2) if limit == 1 { expected = 1 } assert.Equal(t, expected, s.releases.Load()) assert.Equal(t, expected, releasePages.Load()) - assert.Equal(t, expected*2, linkPages.Load()) - assert.Len(t, work, int(expected)*2) + assert.Len(t, assets, int(expected)) + for _, item := range assets { + assert.Equal(t, "https://assets.example.com/binary", item.rawURL) + } }) } } @@ -103,12 +102,15 @@ func TestExactTagAndSources(t *testing.T) { assert.Contains(t, r.URL.EscapedPath(), "version%2F1") _ = json.NewEncoder(w).Encode(gitlab.Release{ TagName: "version/1", Name: "Release 1", - Assets: gitlab.ReleaseAssets{Sources: []gitlab.ReleaseAssetsSource{ - {Format: "tar.gz", URL: "/source.tar.gz"}, {Format: "zip", URL: "/source.zip"}, - }}, + Assets: gitlab.ReleaseAssets{ + Links: []*gitlab.ReleaseLink{ + {Name: "binary", URL: "/binary", DirectAssetURL: "https://gitlab.example.com/downloads/binary"}, + }, + Sources: []gitlab.ReleaseAssetsSource{ + {Format: "tar.gz", URL: "/source.tar.gz"}, {Format: "zip", URL: "/source.zip"}, + }, + }, }) - case "/api/v4/projects/1/releases/version/1/assets/links": - _, _ = fmt.Fprint(w, `[]`) default: http.NotFound(w, r) } @@ -119,14 +121,21 @@ func TestExactTagAndSources(t *testing.T) { o := validOptions() o.Repo, o.Tag, o.IncludeSources = "g/p", "version/1", sources s := NewScanner(o) - work := make(chan asset, 5) - require.NoError(t, s.projectReleases(git, &gitlab.Project{ID: 1}, work)) + var assets []asset + require.NoError(t, s.projectReleases(git, &gitlab.Project{ID: 1}, func(item asset) { + assets = append(assets, item) + })) + require.NotEmpty(t, assets) + assert.Equal(t, "binary", assets[0].name) + assert.Equal(t, "/binary", assets[0].rawURL) + assert.Equal(t, "https://gitlab.example.com/downloads/binary", assets[0].webURL) if sources { - require.Len(t, work, 1) - item := <-work - assert.Equal(t, "/source.zip", item.rawURL) + require.Len(t, assets, 2) + assert.Equal(t, "/source.zip", assets[1].rawURL) + assert.Equal(t, "version/1.zip", assets[1].name) + assert.Empty(t, assets[1].webURL) } else { - assert.Empty(t, work) + assert.Len(t, assets, 1) } assert.Zero(t, s.failures.Load()) }) @@ -143,8 +152,6 @@ func TestDiscoveryContinuesAfterFailure(t *testing.T) { w.WriteHeader(http.StatusNotFound) case "/api/v4/projects/2/releases": _, _ = fmt.Fprint(w, `[{"tag_name":"v1"}]`) - case "/api/v4/projects/2/releases/v1/assets/links": - _, _ = fmt.Fprint(w, `[]`) default: http.NotFound(w, r) } @@ -154,7 +161,9 @@ func TestDiscoveryContinuesAfterFailure(t *testing.T) { require.NoError(t, err) o := validOptions() s := NewScanner(o) - require.ErrorContains(t, s.scan(git, testDownloader(t, srv.URL, 100).base), "incomplete") + base, err := url.Parse(srv.URL) + require.NoError(t, err) + require.ErrorContains(t, s.scan(git, base), "incomplete") assert.Equal(t, int64(2), s.projects.Load()) assert.Equal(t, int64(1), s.releases.Load()) assert.Equal(t, int64(1), s.failures.Load()) @@ -176,9 +185,9 @@ func TestForbiddenReleaseListingIsIgnored(t *testing.T) { git, err := util.GetGitlabClient("token", srv.URL) require.NoError(t, err) s := NewScanner(validOptions()) - work := make(chan asset, 1) - require.NoError(t, s.projectReleases(git, &gitlab.Project{ID: 1}, work)) - assert.Empty(t, work) + require.NoError(t, s.projectReleases(git, &gitlab.Project{ID: 1}, func(asset) { + t.Error("forbidden releases must not produce assets") + })) assert.Zero(t, s.failures.Load()) assert.Zero(t, s.releases.Load()) } @@ -225,17 +234,13 @@ func TestProjectPagination(t *testing.T) { } } -func TestMissingSourcesAndLinkFailure(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusForbidden) - })) - defer srv.Close() - git, err := util.GetGitlabClient("token", srv.URL) - require.NoError(t, err) +func TestMissingSources(t *testing.T) { o := validOptions() o.IncludeSources = true s := NewScanner(o) - s.releaseAssets(git, &gitlab.Project{ID: 1}, &gitlab.Release{TagName: "v1"}, make(chan asset, 1)) - assert.Equal(t, int64(2), s.failures.Load()) + s.releaseAssets(&gitlab.Project{ID: 1}, &gitlab.Release{TagName: "v1"}, func(asset) { + t.Error("release without assets must not produce work") + }) + assert.Equal(t, int64(1), s.failures.Load()) assert.Equal(t, int64(1), s.releases.Load()) } diff --git a/pkg/scan/artifact/processor.go b/pkg/scan/artifact/processor.go index 18568eb6..bbe3f97c 100644 --- a/pkg/scan/artifact/processor.go +++ b/pkg/scan/artifact/processor.go @@ -21,8 +21,6 @@ type ProcessOptions struct { ArtifactName string WorkflowRunName string HitTimeout time.Duration - GitLabURL string - ReportFinding func(scanner.Finding, string, string) } type FileProcessingResult struct { @@ -116,18 +114,14 @@ func ProcessSingleFile(content []byte, filename string, opts ProcessOptions) (*F result.IsArchive = isArchive result.IsUnknown = isUnknown - report := opts.ReportFinding - if report == nil { - report = func(finding scanner.Finding, file, ancestry string) { - scannerartifact.ReportFinding(finding, opts.BuildURL, opts.ArtifactName, file, ancestry) - } - } if opts.HitTimeout == 0 { opts.HitTimeout = time.Minute } err := scannerartifact.ScanFile(content, filename, scannerartifact.ScanOptions{ VerifyCredentials: opts.VerifyCredentials, HitTimeout: opts.HitTimeout, - GitLabURL: opts.GitLabURL, ReportFinding: report, + ReportFinding: func(finding scanner.Finding, file, ancestry string) { + scannerartifact.ReportFinding(finding, opts.BuildURL, opts.ArtifactName, file, ancestry) + }, }) result.Error = err return result, err diff --git a/pkg/scan/artifact/processor_test.go b/pkg/scan/artifact/processor_test.go index 89af4477..9691bb41 100644 --- a/pkg/scan/artifact/processor_test.go +++ b/pkg/scan/artifact/processor_test.go @@ -10,12 +10,13 @@ import ( "time" "github.com/CompassSecurity/pipeleek/pkg/scanner" + scannerartifact "github.com/CompassSecurity/pipeleek/pkg/scanner/artifact" "github.com/h2non/filetype" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestProcessSingleFileExecutableAndUTF8(t *testing.T) { +func TestScanFileExecutableAndUTF8(t *testing.T) { for _, container := range []string{"standalone", "zip", "tar"} { for _, binary := range []bool{false, true} { t.Run(fmt.Sprintf("%s/binary=%v", container, binary), func(t *testing.T) { @@ -70,7 +71,7 @@ func TestProcessSingleFileExecutableAndUTF8(t *testing.T) { content, name = buf.Bytes(), "bundle.tar" } var found, after bool - _, err := ProcessSingleFile(content, name, ProcessOptions{ + err := scannerartifact.ScanFile(content, name, scannerartifact.ScanOptions{ HitTimeout: time.Minute, ReportFinding: func(f scanner.Finding, file, ancestry string) { if strings.Contains(f.Text, "CI_REGISTRY_PASSWORD="+secret) { diff --git a/pkg/scanner/artifact/artifact.go b/pkg/scanner/artifact/artifact.go index aec26577..663f7a8d 100644 --- a/pkg/scanner/artifact/artifact.go +++ b/pkg/scanner/artifact/artifact.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "time" "unicode/utf8" @@ -29,14 +30,9 @@ type ScanOptions struct { ReportFinding func(types.Finding, string, string) } -// IsArchive excludes executable and document formats included in filetype.IsArchive. func IsArchive(content []byte) bool { kind, _ := filetype.Match(content) - switch kind.Extension { - case "zip", "tar", "gz", "bz2", "7z", "rar", "xz", "zst", "lz", "lz4", "ar", "deb", "rpm", "cab", "iso": - return true - } - return false + return slices.Contains(xtractr.SupportedExtensions(), "."+kind.Extension) } func ScanFile(content []byte, filename string, opts ScanOptions) error { diff --git a/tests/e2e/gitlab/release/scan_test.go b/tests/e2e/gitlab/release/scan_test.go index e9e48808..10a03a35 100644 --- a/tests/e2e/gitlab/release/scan_test.go +++ b/tests/e2e/gitlab/release/scan_test.go @@ -42,12 +42,17 @@ func newFixture(t *testing.T, links []map[string]any, assetHandler http.HandlerF _, _ = fmt.Fprint(w, `{"id":1,"path_with_namespace":"group/project"}`) case r.URL.Path == "/api/v4/groups/group": _, _ = fmt.Fprint(w, `{"id":10}`) - case strings.HasSuffix(r.URL.Path, "/assets/links"): - _ = json.NewEncoder(w).Encode(links) case r.URL.Path == "/api/v4/projects/1/releases": - _, _ = fmt.Fprint(w, `[{"tag_name":"v2","name":"Release 2","assets":{"sources":[{"format":"zip","url":"/source.zip"},{"format":"tar.gz","url":"/source.tar.gz"}]}},{"tag_name":"v1","name":"Release 1"}]`) + _ = json.NewEncoder(w).Encode([]map[string]any{ + {"tag_name": "v2", "name": "Release 2", "assets": map[string]any{ + "links": links, "sources": []map[string]string{{"format": "zip", "url": "/source.zip"}, {"format": "tar.gz", "url": "/source.tar.gz"}}, + }}, + {"tag_name": "v1", "name": "Release 1", "assets": map[string]any{"links": links}}, + }) case r.URL.Path == "/api/v4/projects/1/releases/version/1": - _, _ = fmt.Fprint(w, `{"tag_name":"version/1","name":"Exact release"}`) + _ = json.NewEncoder(w).Encode(map[string]any{ + "tag_name": "version/1", "name": "Exact release", "assets": map[string]any{"links": links}, + }) default: assetHandler(w, r) } @@ -167,6 +172,10 @@ func zipAsset(t *testing.T, name string, content []byte) []byte { func TestReleaseScanSourcesAndNestedArchives(t *testing.T) { nested := zipAsset(t, "nested.zip", zipAsset(t, "config.env", []byte("DATABASE_PASSWORD=nestedfixture\n"))) f := newFixture(t, links("/nested.zip"), func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Private-Token") != "test-token" { + w.WriteHeader(http.StatusUnauthorized) + return + } switch r.URL.Path { case "/nested.zip": _, _ = w.Write(nested) @@ -241,6 +250,43 @@ func TestReleaseScanExternalRedirectDoesNotLeakCredentials(t *testing.T) { assert.NotContains(t, output, "do-not-log") } +func TestReleaseScanPortalFindingURL(t *testing.T) { + for _, direct := range []bool{false, true} { + t.Run(fmt.Sprint(direct), func(t *testing.T) { + assets := links("/asset?signature=do-not-log") + portal := "https://gitlab.example.com/group/project/-/releases/version%2F1/downloads/asset" + if direct { + assets[0]["direct_asset_url"] = portal + } + f := newFixture(t, assets, func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "do-not-log", r.URL.Query().Get("signature")) + assert.Equal(t, "test-token", r.Header.Get("Private-Token")) + assert.Empty(t, r.Header.Get("Authorization")) + plainAsset(w, r) + }) + output, err := f.run(t, "--repo", "group/project", "--tag", "version/1") + require.NoError(t, err, output) + if !direct { + portal = f.server.URL + "/group/project/-/releases/version%2F1" + } + var found bool + for _, line := range strings.Split(strings.TrimSpace(output), "\n") { + var event struct { + Level string `json:"level"` + URL string `json:"url"` + } + require.NoError(t, json.Unmarshal([]byte(line), &event)) + if event.Level == "hit" { + found = true + assert.Equal(t, portal, event.URL) + } + } + assert.True(t, found, "expected finding with portal URL") + assert.NotContains(t, output, "do-not-log") + }) + } +} + func TestReleaseScanPartialFailures(t *testing.T) { f := newFixture(t, links("/denied", "/good"), func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/denied" { From 9298ff0ca392bdbcba79743b4415fb6122316692 Mon Sep 17 00:00:00 2001 From: frjcomp <107982661+frjcomp@users.noreply.github.com> Date: Tue, 20 Oct 2026 09:37:45 +0000 Subject: [PATCH 5/7] fix(scan): restore artifact and repository debug logs Restore binary string-extraction diagnostics and log repository URLs during release scanning, with debug-level e2e coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/gitlab/release/scan/scanner.go | 5 +++++ pkg/scanner/artifact/artifact.go | 1 + tests/e2e/gitlab/release/scan_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/pkg/gitlab/release/scan/scanner.go b/pkg/gitlab/release/scan/scanner.go index b2412108..54b1b8ee 100644 --- a/pkg/gitlab/release/scan/scanner.go +++ b/pkg/gitlab/release/scan/scanner.go @@ -128,6 +128,11 @@ func (s *Scanner) scan(git *gitlab.Client, base *url.URL) error { } err := s.projectsForScan(git, func(project *gitlab.Project) error { s.projects.Add(1) + projectURL := project.WebURL + if projectURL == "" { + projectURL = base.JoinPath(project.PathWithNamespace).String() + } + log.Debug().Str("url", projectURL).Str("project", project.PathWithNamespace).Msg("Fetch project releases") if err := s.projectReleases(git, project, enqueue); err != nil { s.failures.Add(1) log.Error().Err(err).Str("project", project.PathWithNamespace).Msg("Failed discovering release assets") diff --git a/pkg/scanner/artifact/artifact.go b/pkg/scanner/artifact/artifact.go index 663f7a8d..9b22bc69 100644 --- a/pkg/scanner/artifact/artifact.go +++ b/pkg/scanner/artifact/artifact.go @@ -51,6 +51,7 @@ func scanFile(content []byte, filename, ancestry string, opts ScanOptions, depth func detectFile(content []byte, filename, ancestry string, opts ScanOptions) error { if !utf8.Valid(content) || bytes.ContainsRune(content, '\x00') { + log.Debug().Str("file", filename).Msg("Binary artifact, extracting strings") content = archive.ExtractPrintableStrings(content, archive.MinStringLength) } findings, err := engine.DetectHitsWithOptions(content, 1, opts.VerifyCredentials, diff --git a/tests/e2e/gitlab/release/scan_test.go b/tests/e2e/gitlab/release/scan_test.go index 10a03a35..6fb29312 100644 --- a/tests/e2e/gitlab/release/scan_test.go +++ b/tests/e2e/gitlab/release/scan_test.go @@ -372,6 +372,31 @@ func TestReleaseScanSizeSkipDebugLog(t *testing.T) { assert.Contains(t, output, `"skippedAssets":1`) } +func TestReleaseScanProjectURLDebugLog(t *testing.T) { + f := newFixture(t, links("/asset"), plainAsset) + for _, level := range []string{"info", "debug"} { + t.Run(level, func(t *testing.T) { + output, err := f.run(t, "--release-limit", "1", "--log-level", level) + require.NoError(t, err, output) + var found bool + for _, line := range strings.Split(strings.TrimSpace(output), "\n") { + var event struct { + Level string `json:"level"` + Message string `json:"message"` + URL string `json:"url"` + } + require.NoError(t, json.Unmarshal([]byte(line), &event)) + if event.Message == "Fetch project releases" { + found = true + assert.Equal(t, "debug", event.Level) + assert.Equal(t, f.server.URL+"/group/project", event.URL) + } + } + assert.Equal(t, level == "debug", found) + }) + } +} + func TestReleaseScanUnavailableAssetsAreSilent(t *testing.T) { f := newFixture(t, links("/forbidden", "/missing", "/good"), func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { From ae06abfd8f8ded8e275294c4ae8560a3229a08ff Mon Sep 17 00:00:00 2001 From: frjcomp <107982661+frjcomp@users.noreply.github.com> Date: Tue, 20 Oct 2026 11:19:29 +0000 Subject: [PATCH 6/7] fix(gitlab): increase release timeout and report project URLs Default release hit detection to two minutes while preserving explicit overrides, and use project web URLs consistently in release scan logs and findings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/guides/gitlab.md | 3 +- docs/introduction/configuration.md | 3 ++ internal/cmd/gitlab/release/scan/scan.go | 3 ++ internal/cmd/gitlab/release/scan/scan_test.go | 34 +++++++++++++++++-- pkg/config/gen/release_test.go | 1 + pkg/gitlab/release/scan/scanner.go | 14 ++++---- pkg/gitlab/release/scan/scanner_test.go | 5 ++- tests/e2e/gitlab/release/scan_test.go | 8 ++--- 8 files changed, 56 insertions(+), 15 deletions(-) diff --git a/docs/guides/gitlab.md b/docs/guides/gitlab.md index 72e1db1f..a73f5ad2 100644 --- a/docs/guides/gitlab.md +++ b/docs/guides/gitlab.md @@ -129,7 +129,8 @@ published releases per project (`0` means all). `--include-sources` adds one generated source ZIP per release. Downloads send the token only to the configured GitLab origin, not external assets or redirects; ensure linked hosts are authorized scan targets. -`--max-artifact-size` limits downloaded bytes, not extracted archive size. +`--max-artifact-size` limits downloaded bytes, not extracted archive size; +release scans default to a two-minute `--hit-timeout`. ## Secret Detection in Pipelines And Artifacts diff --git a/docs/introduction/configuration.md b/docs/introduction/configuration.md index aa2acbda..cf1eaebc 100644 --- a/docs/introduction/configuration.md +++ b/docs/introduction/configuration.md @@ -67,6 +67,9 @@ To view a full example of the available keys run `pipeleek config gen`. Scan commands inherit from `common`: +The default hit-detection timeout is one minute, or two minutes for `gl release scan`. +An explicit `common.hit_timeout`, `PIPELEEK_COMMON_HIT_TIMEOUT`, or `--hit-timeout` overrides that default. + ```yaml common: threads: 2 diff --git a/internal/cmd/gitlab/release/scan/scan.go b/internal/cmd/gitlab/release/scan/scan.go index 9a690e68..766a1b17 100644 --- a/internal/cmd/gitlab/release/scan/scan.go +++ b/internal/cmd/gitlab/release/scan/scan.go @@ -50,6 +50,8 @@ pipeleek gl release scan --url https://gitlab.example.com --token [redacted] --n } var common config.CommonScanOptions flags.AddCommonScanFlagsNoArtifacts(cmd, &common) + common.HitTimeout = 2 * time.Minute + cmd.Flags().Lookup("hit-timeout").DefValue = common.HitTimeout.String() cmd.Flags().String("max-artifact-size", "500Mb", "Maximum downloaded bytes per asset; extracted archive sizes are not limited") cmd.Flags().StringP("repo", "r", "", "Single repository to scan, format: namespace/repo") cmd.Flags().StringP("namespace", "n", "", "Namespace to scan, including subgroup projects") @@ -63,6 +65,7 @@ pipeleek gl release scan --url https://gitlab.example.com --token [redacted] --n } func optionsFromConfig() (releasescan.Options, error) { + config.GetViper().SetDefault("common.hit_timeout", "120s") maxBytes, err := config.ParseMaxArtifactSize(config.GetString("common.max_artifact_size")) if err != nil { return releasescan.Options{}, err diff --git a/internal/cmd/gitlab/release/scan/scan_test.go b/internal/cmd/gitlab/release/scan/scan_test.go index a5fc5b03..9fea7102 100644 --- a/internal/cmd/gitlab/release/scan/scan_test.go +++ b/internal/cmd/gitlab/release/scan/scan_test.go @@ -21,7 +21,7 @@ func TestNewScanCmd(t *testing.T) { assert.Error(t, cmd.Args(cmd, []string{"extra"})) for name, value := range map[string]string{ "release-limit": "0", "include-sources": "false", "max-artifact-size": "500Mb", - "threads": "4", "hit-timeout": "1m0s", "secrets-verification": "true", + "threads": "4", "hit-timeout": "2m0s", "secrets-verification": "true", } { require.NotNil(t, cmd.Flags().Lookup(name)) assert.Equal(t, value, cmd.Flags().Lookup(name).DefValue, name) @@ -42,9 +42,12 @@ func TestOptionsFromConfig(t *testing.T) { parent.AddCommand(cmd) require.NoError(t, cmd.ParseFlags([]string{"--url", "https://gitlab.example.com", "--token", "test-token", "--tag", "v3"})) require.NoError(t, config.NewCommandSetup(cmd).WithFlagBindings(flagBindings).Bind()) - config.GetViper().Set("common.hit_timeout", "120") opts, err := optionsFromConfig() require.NoError(t, err) + assert.Equal(t, 2*time.Minute, opts.HitTimeout) + config.GetViper().Set("common.hit_timeout", "120") + opts, err = optionsFromConfig() + require.NoError(t, err) assert.Equal(t, "v3", opts.Tag) assert.Equal(t, "group/project", opts.Repo) assert.Equal(t, 120*time.Second, opts.HitTimeout) @@ -90,3 +93,30 @@ common: require.NoError(t, err) assert.Equal(t, 5, opts.ReleaseLimit) } + +func TestHitTimeoutOverrides(t *testing.T) { + for _, tc := range []struct { + name string + env string + args []string + want time.Duration + }{ + {"default", "", nil, 2 * time.Minute}, + {"environment", "45s", nil, 45 * time.Second}, + {"flag", "45s", []string{"--hit-timeout", "30s"}, 30 * time.Second}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("PIPELEEK_NO_CONFIG", "1") + t.Setenv("PIPELEEK_GITLAB_URL", "https://gitlab.example.com") + t.Setenv("PIPELEEK_GITLAB_TOKEN", "test-token") + t.Setenv("PIPELEEK_COMMON_HIT_TIMEOUT", tc.env) + require.NoError(t, config.InitializeViper("")) + cmd := NewScanCmd() + require.NoError(t, cmd.ParseFlags(tc.args)) + require.NoError(t, config.NewCommandSetup(cmd).WithFlagBindings(flagBindings).Bind()) + opts, err := optionsFromConfig() + require.NoError(t, err) + assert.Equal(t, tc.want, opts.HitTimeout) + }) + } +} diff --git a/pkg/config/gen/release_test.go b/pkg/config/gen/release_test.go index fbaa5829..e52f5699 100644 --- a/pkg/config/gen/release_test.go +++ b/pkg/config/gen/release_test.go @@ -39,4 +39,5 @@ func TestReleaseScanConfig(t *testing.T) { assert.Zero(t, parsed.GitLab.Release.Scan.ReleaseLimit) assert.False(t, parsed.GitLab.Release.Scan.IncludeSources) assert.Contains(t, content, "PIPELEEK_GITLAB_RELEASE_SCAN_RELEASE_LIMIT") + assert.Contains(t, content, `hit_timeout: "2m0s"`) } diff --git a/pkg/gitlab/release/scan/scanner.go b/pkg/gitlab/release/scan/scanner.go index 54b1b8ee..8d4310a9 100644 --- a/pkg/gitlab/release/scan/scanner.go +++ b/pkg/gitlab/release/scan/scanner.go @@ -8,6 +8,7 @@ import ( "net/http" "net/url" "path" + "strings" "sync/atomic" "time" @@ -128,14 +129,13 @@ func (s *Scanner) scan(git *gitlab.Client, base *url.URL) error { } err := s.projectsForScan(git, func(project *gitlab.Project) error { s.projects.Add(1) - projectURL := project.WebURL - if projectURL == "" { - projectURL = base.JoinPath(project.PathWithNamespace).String() + if project.WebURL == "" { + project.WebURL = base.JoinPath(project.PathWithNamespace).String() } - log.Debug().Str("url", projectURL).Str("project", project.PathWithNamespace).Msg("Fetch project releases") + log.Debug().Str("project", project.WebURL).Msg("Fetch project releases") if err := s.projectReleases(git, project, enqueue); err != nil { s.failures.Add(1) - log.Error().Err(err).Str("project", project.PathWithNamespace).Msg("Failed discovering release assets") + log.Error().Err(err).Str("project", project.WebURL).Msg("Failed discovering release assets") } return nil }) @@ -225,7 +225,7 @@ func published(release *gitlab.Release) bool { func (s *Scanner) releaseAssets(project *gitlab.Project, release *gitlab.Release, visit func(asset)) { s.releases.Add(1) - item := asset{project: project.PathWithNamespace, tag: release.TagName, release: release.Name} + item := asset{project: project.WebURL, tag: release.TagName, release: release.Name} for _, link := range release.Assets.Links { item.name, item.rawURL, item.webURL = link.Name, link.URL, link.DirectAssetURL visit(item) @@ -258,7 +258,7 @@ func (s *Scanner) scanAsset(base *url.URL, item asset) { item.name = filename } if item.webURL == "" { - item.webURL = base.JoinPath(item.project, "-", "releases", url.PathEscape(item.tag)).String() + item.webURL = strings.TrimRight(item.project, "/") + "/-/releases/" + url.PathEscape(item.tag) } content, err := downloadAsset(u.String(), base, s.options.Token, s.options.MaxBytes) if err == nil { diff --git a/pkg/gitlab/release/scan/scanner_test.go b/pkg/gitlab/release/scan/scanner_test.go index 1b884532..c5b102df 100644 --- a/pkg/gitlab/release/scan/scanner_test.go +++ b/pkg/gitlab/release/scan/scanner_test.go @@ -75,7 +75,9 @@ func TestReleasePaginationAndLimit(t *testing.T) { o.ReleaseLimit = limit s := NewScanner(o) var assets []asset - require.NoError(t, s.projectReleases(git, &gitlab.Project{ID: 1, PathWithNamespace: "g/p"}, func(item asset) { + require.NoError(t, s.projectReleases(git, &gitlab.Project{ + ID: 1, PathWithNamespace: "g/p", WebURL: "https://gitlab.example.com/g/p", + }, func(item asset) { assets = append(assets, item) })) expected := int64(2) @@ -86,6 +88,7 @@ func TestReleasePaginationAndLimit(t *testing.T) { assert.Equal(t, expected, releasePages.Load()) assert.Len(t, assets, int(expected)) for _, item := range assets { + assert.Equal(t, "https://gitlab.example.com/g/p", item.project) assert.Equal(t, "https://assets.example.com/binary", item.rawURL) } }) diff --git a/tests/e2e/gitlab/release/scan_test.go b/tests/e2e/gitlab/release/scan_test.go index 6fb29312..345f9d0f 100644 --- a/tests/e2e/gitlab/release/scan_test.go +++ b/tests/e2e/gitlab/release/scan_test.go @@ -101,7 +101,7 @@ func TestReleaseScanDefaultAndProjectFilters(t *testing.T) { output, err := f.run(t, tc.args...) require.NoError(t, err, output) assert.Contains(t, output, `"level":"hit"`) - assert.Contains(t, output, `"project":"group/project"`) + assert.Contains(t, output, `"project":"`+f.server.URL+`/group/project"`) for _, line := range strings.Split(strings.TrimSpace(output), "\n") { var event struct { Level string `json:"level"` @@ -383,13 +383,13 @@ func TestReleaseScanProjectURLDebugLog(t *testing.T) { var event struct { Level string `json:"level"` Message string `json:"message"` - URL string `json:"url"` + Project string `json:"project"` } require.NoError(t, json.Unmarshal([]byte(line), &event)) if event.Message == "Fetch project releases" { found = true assert.Equal(t, "debug", event.Level) - assert.Equal(t, f.server.URL+"/group/project", event.URL) + assert.Equal(t, f.server.URL+"/group/project", event.Project) } } assert.Equal(t, level == "debug", found) @@ -546,7 +546,7 @@ func TestReleaseScanWebUI(t *testing.T) { } require.NoError(t, events.Err()) assert.Contains(t, body.String(), "releasefixture") - assert.Contains(t, body.String(), `"project":"group/project"`) + assert.Contains(t, body.String(), `"project":"`+f.server.URL+`/group/project"`) assert.Contains(t, body.String(), `"tag":"v2"`) assert.Contains(t, body.String(), "event: done") } From 6895d1876be7931550d81812220c70f6df58f28a Mon Sep 17 00:00:00 2001 From: frjcomp <107982661+frjcomp@users.noreply.github.com> Date: Tue, 20 Oct 2026 11:31:56 +0000 Subject: [PATCH 7/7] fix(scan): report per-file artifact scanning failures Restore diagnostics for nested extraction errors and detection timeouts while preserving sibling processing and ZIP-level error handling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- pkg/scan/artifact/processor.go | 11 ++++- pkg/scan/artifact/processor_test.go | 67 +++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/pkg/scan/artifact/processor.go b/pkg/scan/artifact/processor.go index bbe3f97c..12cfbb3c 100644 --- a/pkg/scan/artifact/processor.go +++ b/pkg/scan/artifact/processor.go @@ -4,11 +4,13 @@ import ( "archive/zip" "bytes" "context" + "errors" "io" "time" "github.com/CompassSecurity/pipeleek/pkg/scanner" scannerartifact "github.com/CompassSecurity/pipeleek/pkg/scanner/artifact" + "github.com/CompassSecurity/pipeleek/pkg/scanner/engine" "github.com/h2non/filetype" "github.com/rs/zerolog/log" "github.com/wandb/parallel" @@ -86,7 +88,14 @@ func processZipFile(file *zip.File, opts ProcessOptions) FileProcessingResult { } processed, err := ProcessSingleFile(content, file.Name, opts) - processed.Error = err + if err != nil { + event := log.Error() + if errors.Is(err, engine.ErrHitTimeout) { + event = log.Warn() + } + event.Err(err).Str("file", file.Name).Str("url", opts.BuildURL). + Str("artifact", opts.ArtifactName).Msg("Failed scanning artifact file") + } return *processed } diff --git a/pkg/scan/artifact/processor_test.go b/pkg/scan/artifact/processor_test.go index 9691bb41..b5ae39a3 100644 --- a/pkg/scan/artifact/processor_test.go +++ b/pkg/scan/artifact/processor_test.go @@ -4,6 +4,7 @@ import ( "archive/tar" "archive/zip" "bytes" + "encoding/json" "fmt" "strings" "testing" @@ -12,6 +13,8 @@ import ( "github.com/CompassSecurity/pipeleek/pkg/scanner" scannerartifact "github.com/CompassSecurity/pipeleek/pkg/scanner/artifact" "github.com/h2non/filetype" + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -116,6 +119,70 @@ func init() { scanner.InitRules([]string{}) } +func TestProcessZipArtifactLogsFileFailures(t *testing.T) { + for _, tc := range []struct { + name string + content []byte + timeout time.Duration + level string + }{ + {"broken.zip", []byte("PK\x03\x04invalid archive"), time.Minute, "error"}, + {"timeout.txt", []byte("ordinary text"), time.Nanosecond, "warn"}, + } { + t.Run(tc.name, func(t *testing.T) { + var output bytes.Buffer + original, level := log.Logger, zerolog.GlobalLevel() + log.Logger = zerolog.New(zerolog.SyncWriter(&output)) + zerolog.SetGlobalLevel(zerolog.DebugLevel) + t.Cleanup(func() { + log.Logger = original + zerolog.SetGlobalLevel(level) + }) + var buf bytes.Buffer + w := zip.NewWriter(&buf) + for _, file := range []struct { + name string + content []byte + }{ + {tc.name, tc.content}, {"after.txt", []byte("another file")}, + } { + f, err := w.Create(file.name) + require.NoError(t, err) + _, err = f.Write(file.content) + require.NoError(t, err) + } + require.NoError(t, w.Close()) + + results, err := ProcessZipArtifact(buf.Bytes(), ProcessOptions{ + MaxGoRoutines: 2, HitTimeout: tc.timeout, + BuildURL: "https://ci.example.com/build/1", ArtifactName: "build.zip", + }) + require.NoError(t, err, "per-file failures must not become ZIP-level errors") + require.Len(t, results, 2, "all sibling files must be processed") + for _, result := range results { + if result.FileName == tc.name { + assert.Error(t, result.Error) + } else if tc.level == "error" { + assert.NoError(t, result.Error) + } + } + var found bool + for _, line := range strings.Split(strings.TrimSpace(output.String()), "\n") { + var event map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(line), &event)) + if event["message"] == "Failed scanning artifact file" && event["file"] == tc.name { + found = true + assert.Equal(t, tc.level, event["level"]) + assert.Equal(t, "https://ci.example.com/build/1", event["url"]) + assert.Equal(t, "build.zip", event["artifact"]) + assert.NotEmpty(t, event["error"]) + } + } + assert.True(t, found, "expected per-file failure diagnostic") + }) + } +} + func TestProcessZipArtifact(t *testing.T) { tests := []struct { name string