fix(secret): trim excessively long lines #7192

afdesk · 2024-07-19T08:03:02Z

Description

In this PR, we address the issue of handling excessively long lines in files.
When a line is too lengthy, displaying the entire content is unnecessary and inefficient.

In general cases, such as obfuscated JavaScript code, we truncate the line appropriately to improve readability.

If a long line contains a secret, we only display the segment containing the secret.

Before:

$ trivy image --scanners secret mattermost/mattermost-enterprise-edition:9.7.1
/mattermost/client/8055.c2e8394446fd4bca58d8.js (secrets)
=========================================================
Total: 2 (UNKNOWN: 0, LOW: 2, MEDIUM: 0, HIGH: 0, CRITICAL: 0)

LOW: Stripe (stripe-publishable-token)
════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════
Stripe Publishable Key
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 /mattermost/client/8055.c2e8394446fd4bca58d8.js:1 (added by 'RUN |3 PUID=2000 PGID=2000 MM_PACKAGE=ht')
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1 [ "use strict";(self.webpackChunkmattermost_webapp=self.webpackChunkmattermost_webapp||[]).push([[8055],{59713:(e,t,n)=>{n.d(t,{$d:()=>b,DF:()=>G,Du:()=>B,GA:()=>F,Is:()=>M,JJ:()=>J,K6:()=>re,KB:()=>f,KO:()=>x,Md:()=>ae,NB:()=>ce,O$:()=>E,OT:()=>ee,QQ:()=>oe,SP:()=>K,Sm:()=>se,T8:()=>Y,UV:()=>pe,W3:()=>C,X$:()=>X,_9:()=>de,a6:()=>Q,d1:()=>ne,d5:()=>$,dA:()=>te,gY:()=>ie,go:()=>T,h5:()=>z,iB:()=>A,iz:()=>O,jI:()=>S,kO:()=>N,lJ:()=>me,nU:()=>D,oQ:()=>Z,pu:()=>H,pw:()=>I,qE:()=>ue,r3:()=>j,r6:()=>le,rX:()=>U,rm:()=>y,sW:()=>R,sg:()=>q,sv:()=>W,v1:()=>w,vI:()=>v,vy:()=>L,wg:()=>k,xT:()=>P,yT:()=>_,yX:()=>V}),n(61418),n(92189);var a=n(12763),s=n(98644),i=n(17086),r=n(31435),o=n(80139),l=n(68679),c=n(28176),d=n(5123),m=n(91436),u=n(23712);function p(e,t){var n=Object.keys(e);if(Object.getOwnPropertySymbols){var a=Object.getOwnPropertySymbols(e);t&&(a=a.filter((function(t){return Object.getOwnPropertyDescriptor(e,t).enumerable}))),n.push.apply(n,a)}return n}function h(e){for(var s ({count})","admin.system_users.filtersPopover.apply":"Apply","admin.system_users.filtersPopover.title":"Filter :!))),s=e.filter((e=>null!==e&&!n.includes(e.id))).sort(((e,n)=>function(e,t,n){return null!==t.display_name&&t.display_name! <REALLY LONG JS SCRIPT>
   2   //# sourceMappingURL=8055.c2e8394446fd4bca58d8.js.map
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

After:

$ trivy image --scanners secret mattermost/mattermost-enterprise-edition:9.7.1
...

/mattermost/client/8055.c2e8394446fd4bca58d8.js (secrets)
=========================================================
Total: 2 (UNKNOWN: 0, LOW: 2, MEDIUM: 0, HIGH: 0, CRITICAL: 0)

LOW: Stripe (stripe-publishable-token)
════════════════════════════════════════
Stripe Publishable Key
────────────────────────────────────────
 /mattermost/client/8055.c2e8394446fd4bca58d8.js:1 (added by 'RUN |3 PUID=2000 PGID=2000 MM_PACKAGE=ht')
────────────────────────────────────────
   1 [ ){case a.ez.PRODUCTION:return"********************************";case a.ez.TEST:cas
   2   //# sourceMappingURL=8055.c2e8394446fd4bca58d8.js.map
────────────────────────────────────────

Related issues

Close bug(secret): set limit for line length #6999

Checklist

I've read the guidelines for contributing to this repository.
I've followed the conventions in the PR title.
I've added tests that prove my fix is effective or that my feature works.
I've updated the documentation with the relevant information (if needed).
I've added usage information (if the PR introduces new options)
I've included a "before" and "after" example to the description (if the PR is a user interface change).

afdesk · 2024-07-22T06:23:18Z

@knqyf263 @DmitriyLewen could you take a look at this PR when yuo have free time? thanks

I'm not sure about maxLineLength = 2000.
Now 2000 is a length for backward compatibility with RSA keys.

wdyt?

DmitriyLewen

Left a comment about max length of line.
The rest looks good to me.

DmitriyLewen · 2024-07-22T10:20:33Z

pkg/fanal/secret/scanner.go

+		if len(rawLine) > maxLineLength {
+			strRawLine = lo.Ternary(inCause, matchLine, string(rawLine[:maxLineLength]))
+		} else {
+			strRawLine = string(rawLine)
+		}


Looks like 100 characters is enough.

For lines before and after secret - we don't need such a big line.

For secret line - if length of secret with secret > 100 - we use 30 + secret + 20 characters:

trivy/pkg/fanal/secret/scanner.go

Lines 501 to 504 in 36ea011

if lineEnd-lineStart > 100 {

lineStart = lo.Ternary(start-30 < 0, 0, start-30)

lineEnd = lo.Ternary(end+20 > len(content), len(content), end+20)

}

Now 2000 is a length for backward compatibility with RSA keys.

IIUC you want to include full RSA key to result (-----BEGIN RSA PRIVATE KEY----- and -----END RSA PRIVATE KEY-----), but I think this is not necessary.
But when we use 2000, we can see case when secret length is 10, but the user sees 1900 unneeded characters.

@DmitriyLewen I generally agree with you. 2k is too long line.

afdesk · 2024-07-22T11:01:50Z

pkg/fanal/secret/scanner_test.go

+					Content:     "----BEGIN RSA PRIVATE KEY-----**************************************************************************************************************************-----END RSA PRIVATE",
+					Highlighted: "----BEGIN RSA PRIVATE KEY-----**************************************************************************************************************************-----END RSA PRIVATE",


@DmitriyLewen I've decreased maxLineLength and updated the test.
it shows before/after.

pkg/fanal/secret/scanner.go

DmitriyLewen

LGTM

cc. @knqyf263

afdesk added 4 commits July 19, 2024 13:56

initial commit

a3f8cf3

feat: trim long lines

6cb43b6

chore: add a const instead of hard-coded figures

f9baa72

test: add obfuscated js for testing

36ea011

afdesk changed the title ~~fix(secret): trim excessively long secret data~~ fix(secret): trim excessively long lines Jul 22, 2024

afdesk marked this pull request as ready for review July 22, 2024 06:21

afdesk requested review from knqyf263 and DmitriyLewen as code owners July 22, 2024 06:21

DmitriyLewen reviewed Jul 22, 2024

View reviewed changes

afdesk added 2 commits July 22, 2024 16:59

fix: down max line length to 100

7460068

test: update long contnet

ef12409

afdesk commented Jul 22, 2024

View reviewed changes

afdesk requested a review from DmitriyLewen July 22, 2024 11:02

DmitriyLewen reviewed Jul 22, 2024

View reviewed changes

pkg/fanal/secret/scanner.go Outdated Show resolved Hide resolved

added a comment

70dd895

afdesk requested a review from DmitriyLewen July 22, 2024 11:48

DmitriyLewen approved these changes Jul 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(secret): trim excessively long lines #7192

fix(secret): trim excessively long lines #7192

afdesk commented Jul 19, 2024 •

edited

Loading

afdesk commented Jul 22, 2024

DmitriyLewen left a comment

DmitriyLewen Jul 22, 2024

afdesk Jul 22, 2024

afdesk Jul 22, 2024

DmitriyLewen left a comment

	if lineEnd-lineStart > 100 {
	lineStart = lo.Ternary(start-30 < 0, 0, start-30)
	lineEnd = lo.Ternary(end+20 > len(content), len(content), end+20)
	}

		Content: "----BEGIN RSA PRIVATE KEY-----**************************************************************************************************************************-----END RSA PRIVATE",
		Highlighted: "----BEGIN RSA PRIVATE KEY-----**************************************************************************************************************************-----END RSA PRIVATE",

fix(secret): trim excessively long lines #7192

Are you sure you want to change the base?

fix(secret): trim excessively long lines #7192

Conversation

afdesk commented Jul 19, 2024 • edited Loading

Description

Related issues

Checklist

afdesk commented Jul 22, 2024

DmitriyLewen left a comment

Choose a reason for hiding this comment

DmitriyLewen Jul 22, 2024

Choose a reason for hiding this comment

afdesk Jul 22, 2024

Choose a reason for hiding this comment

afdesk Jul 22, 2024

Choose a reason for hiding this comment

DmitriyLewen left a comment

Choose a reason for hiding this comment

afdesk commented Jul 19, 2024 •

edited

Loading