-
Notifications
You must be signed in to change notification settings - Fork 102
Add secretscan table plugin to osquery table registration. (POC) #2543
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
directionless
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like the scanning tests to be a little different. I would like:
One is a test using the table test pattern and a set of specific strings as input. For each string they should:
- Test raw_data
- Create a temp file and test with it.
- Validate that each of those two produces the expected result.
The other is takes specific test files, that are sourced from a test-data directory and validates their results.
Both cases should have examples of different kinds of secrets, as well as some non-secret files.
| // highSeverityRules contains rule IDs that are considered high severity | ||
| var highSeverityRules = map[string]bool{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way we can just pull this whole list from the library, and not enumerate it? I'm think about how it might change over time.
| "github.com/zricethezav/gitleaks/v8/detect" | ||
| "github.com/zricethezav/gitleaks/v8/report" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does the resultant launcher binary size change with this addition?
| // Initialize gitleaks detector with default config (~150 secret patterns) | ||
| // If initialization fails, we store the error and return it at query time | ||
| // rather than returning nil (which would cause a panic during plugin registration) | ||
| detector, detectorErr := detect.NewDetectorDefaultConfig() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has the effect of creating this once, per launcher session, and reusing it. Is that the intended effect?
| ) | ||
| continue | ||
| } | ||
| results = append(results, fileResults...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to include filepath in the results, or is it coming out of the scanFile?
| // Don't echo back the raw data for security | ||
| for i := range rawResults { | ||
| rawResults[i]["raw_data"] = "[scanned]" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work? I would expect sqlite to filter it out.
select * where raw_data = 'secret'
and if the table returns raw_data = [scanned] sqlite will filter it out
| } | ||
|
|
||
| // determineSeverity maps a rule ID and entropy to a severity level | ||
| func determineSeverity(ruleID string, entropy float32) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot decide if we know enough to do this. We might not, and it might be better to surface the raw data, and we can be more opinionated in checks. I'd probably strike this
| return sharedDetector, sharedDetectorErr | ||
| } | ||
|
|
||
| func TestTablePlugin(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test feels of low value.
| } | ||
| } | ||
|
|
||
| func TestGenerate_DetectorError(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test feels low value
| // redact returns a redacted version of a secret for safe logging/display | ||
| // Shows first 4 characters followed by "..." to provide context without exposing the full secret | ||
| func redact(secret string) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is clever, but I think the thresholds are off. I might say we should fully redact everything under 6 characters, and for 6 and above redact all but the first 3.
Because a 4 character feels like it leaks too much
This pull request adds a new table plugin for secret scanning to the Kolide Launcher, integrating the
gitleakslibrary to detect secrets in files and raw data. It introduces thekolide_secret_scantable, which can be queried for secret findings, and includes comprehensive unit tests for the new functionality. The PR also updates several dependencies ingo.modto support the new feature.New secret scanning table:
ee/tables/secretscan/table.goimplementing thekolide_secret_scantable usinggitleaksto scan file paths and raw data for secrets. The table returns details such as rule ID, description, severity, line/column numbers, entropy, and a redacted context for each finding.pkg/osquery/table/table.goso it is available for queries. [1] [2]Testing and reliability:
ee/tables/secretscan/table_test.gowith extensive unit tests covering table registration, secret detection, severity mapping, redaction logic, error handling, and edge cases.Local query with interactive:
Dependency updates:
github.com/zricethezav/gitleaks/v8and several related indirect dependencies togo.modfor secret scanning support. Updated other dependencies to latest versions for compatibility and security. [1] [2] [3] [4]