Skip to content

Commit 8cbb844

Browse files
kylos101ona-agent
kylos101
and
ona-agent
committed
Simplify & no metrics
Co-authored-by: Ona <[email protected]>
1 parent bcd01d4 commit 8cbb844

File tree

5 files changed

+194
-151
lines changed

5 files changed

+194
-151
lines changed

‎components/ee/agent-smith/pkg/agent/agent.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,15 +149,17 @@ func NewAgentSmith(cfg config.Config) (*Smith, error) {
149149
WorkingArea: cfg.FilesystemScanning.WorkingArea,
150150
}
151151

152-
// Check if the main classifier supports filesystem detection
153-
if fsc, ok := class.(classifier.FileClassifier); ok {
154-
filesystemClass = fsc
152+
// Create independent filesystem classifier (no dependency on process classifier)
153+
filesystemClass, err = cfg.Blocklists.FileClassifier()
154+
if err != nil {
155+
log.WithError(err).Error("failed to create filesystem classifier")
156+
} else {
155157
filesystemDetec, err = detector.NewfileDetector(fsConfig, filesystemClass)
156158
if err != nil {
157-
log.WithError(err).Warn("failed to create filesystem detector")
159+
log.WithError(err).Error("failed to create filesystem detector")
160+
} else {
161+
log.Info("Filesystem detector created successfully with independent classifier")
158162
}
159-
} else {
160-
log.Warn("classifier does not support filesystem detection, filesystem scanning disabled")
161163
}
162164
}
163165

‎components/ee/agent-smith/pkg/classifier/classifier.go‎

Lines changed: 1 addition & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -254,12 +254,7 @@ func (sigcl *SignatureMatchClassifier) Matches(executable string, cmdline []stri
254254

255255
// MatchesFile checks if a filesystem file matches any filesystem signatures
256256
func (sigcl *SignatureMatchClassifier) MatchesFile(filePath string) (c *Classification, err error) {
257-
filesystemSignatures := make([]*Signature, 0)
258-
for _, sig := range sigcl.Signatures {
259-
if sig.Domain == DomainFileSystem {
260-
filesystemSignatures = append(filesystemSignatures, sig)
261-
}
262-
}
257+
filesystemSignatures := sigcl.GetFileSignatures()
263258

264259
if len(filesystemSignatures) == 0 {
265260
return sigNoMatch, nil
@@ -506,14 +501,6 @@ func (cl *CountingMetricsClassifier) Matches(executable string, cmdline []string
506501
return cl.D.Matches(executable, cmdline)
507502
}
508503

509-
func (cl *CountingMetricsClassifier) MatchesFile(filePath string) (*Classification, error) {
510-
cl.callCount.Inc()
511-
if fsc, ok := cl.D.(FileClassifier); ok {
512-
return fsc.MatchesFile(filePath)
513-
}
514-
return sigNoMatch, nil
515-
}
516-
517504
func (cl *CountingMetricsClassifier) Describe(d chan<- *prometheus.Desc) {
518505
cl.callCount.Describe(d)
519506
cl.D.Describe(d)
@@ -523,99 +510,3 @@ func (cl *CountingMetricsClassifier) Collect(m chan<- prometheus.Metric) {
523510
cl.callCount.Collect(m)
524511
cl.D.Collect(m)
525512
}
526-
527-
func (cl *CountingMetricsClassifier) GetFileSignatures() []*Signature {
528-
if fsc, ok := cl.D.(FileClassifier); ok {
529-
return fsc.GetFileSignatures()
530-
}
531-
return nil
532-
}
533-
534-
func (cl GradedClassifier) MatchesFile(filePath string) (*Classification, error) {
535-
order := []Level{LevelVery, LevelBarely, LevelAudit}
536-
537-
var (
538-
c *Classification
539-
err error
540-
)
541-
for _, level := range order {
542-
classifier, exists := cl[level]
543-
if !exists {
544-
continue
545-
}
546-
547-
if fsc, ok := classifier.(FileClassifier); ok {
548-
c, err = fsc.MatchesFile(filePath)
549-
if err != nil {
550-
return nil, err
551-
}
552-
if c.Level != LevelNoMatch {
553-
break
554-
}
555-
}
556-
}
557-
558-
if c == nil || c.Level == LevelNoMatch {
559-
return sigNoMatch, nil
560-
}
561-
562-
res := *c
563-
res.Classifier = ClassifierGraded + "." + res.Classifier
564-
return &res, nil
565-
}
566-
567-
func (cl GradedClassifier) GetFileSignatures() []*Signature {
568-
var allSignatures []*Signature
569-
570-
for _, classifier := range cl {
571-
if fsc, ok := classifier.(FileClassifier); ok {
572-
signatures := fsc.GetFileSignatures()
573-
allSignatures = append(allSignatures, signatures...)
574-
}
575-
}
576-
577-
return allSignatures
578-
}
579-
580-
func (cl CompositeClassifier) MatchesFile(filePath string) (*Classification, error) {
581-
var (
582-
c *Classification
583-
err error
584-
)
585-
for _, classifier := range cl {
586-
if fsc, ok := classifier.(FileClassifier); ok {
587-
c, err = fsc.MatchesFile(filePath)
588-
if err != nil {
589-
return nil, err
590-
}
591-
if c.Level != LevelNoMatch {
592-
break
593-
}
594-
}
595-
}
596-
597-
if c == nil || len(cl) == 0 {
598-
// empty composite classifier
599-
return sigNoMatch, nil
600-
}
601-
if c.Level == LevelNoMatch {
602-
return sigNoMatch, nil
603-
}
604-
605-
res := *c
606-
res.Classifier = ClassifierComposite + "." + res.Classifier
607-
return &res, nil
608-
}
609-
610-
func (cl CompositeClassifier) GetFileSignatures() []*Signature {
611-
var allSignatures []*Signature
612-
613-
for _, classifier := range cl {
614-
if fsc, ok := classifier.(FileClassifier); ok {
615-
signatures := fsc.GetFileSignatures()
616-
allSignatures = append(allSignatures, signatures...)
617-
}
618-
}
619-
620-
return allSignatures
621-
}

‎components/ee/agent-smith/pkg/classifier/classifier_test.go‎

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -91,38 +91,3 @@ func TestCommandlineClassifier(t *testing.T) {
9191
})
9292
}
9393
}
94-
95-
func TestCountingMetricsClassifierFilesystemInterface(t *testing.T) {
96-
// Create a signature classifier with filesystem signatures
97-
signatures := []*classifier.Signature{
98-
{
99-
Name: "test-filesystem",
100-
Domain: classifier.DomainFileSystem,
101-
Pattern: []byte("malware"),
102-
Filename: []string{"malware.exe"},
103-
},
104-
}
105-
106-
sigClassifier := classifier.NewSignatureMatchClassifier("test", classifier.LevelAudit, signatures)
107-
countingClassifier := classifier.NewCountingMetricsClassifier("counting", sigClassifier)
108-
109-
// Test that CountingMetricsClassifier implements FileClassifier
110-
var fsc classifier.FileClassifier = countingClassifier
111-
112-
// Test filesystem file matching (file doesn't exist, should return no match without error)
113-
result, err := fsc.MatchesFile("/nonexistent/path/malware.exe")
114-
if err != nil {
115-
t.Fatalf("MatchesFile failed: %v", err)
116-
}
117-
118-
// Should return no match since file doesn't exist, but no error
119-
if result.Level != classifier.LevelNoMatch {
120-
t.Errorf("Expected LevelNoMatch for nonexistent file, got %v", result.Level)
121-
}
122-
123-
// Test that the interface delegation works by checking that it doesn't panic
124-
// and returns a valid Classification
125-
if result == nil {
126-
t.Error("Expected non-nil Classification result")
127-
}
128-
}

‎components/ee/agent-smith/pkg/config/config.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,42 @@ func (b *Blocklists) Classifier() (res classifier.ProcessClassifier, err error)
261261
return gres, nil
262262
}
263263

264+
// FileClassifier creates a classifier specifically for filesystem scanning
265+
// This extracts only filesystem signatures from all blocklist levels and creates
266+
// a clean classifier without any CountingMetricsClassifier wrapper
267+
func (b *Blocklists) FileClassifier() (classifier.FileClassifier, error) {
268+
if b == nil {
269+
// Return a classifier with no signatures - will match nothing
270+
return classifier.NewSignatureMatchClassifier("filesystem-empty", classifier.LevelAudit, nil), nil
271+
}
272+
273+
// Collect all filesystem signatures from all levels
274+
var allFilesystemSignatures []*classifier.Signature
275+
276+
for _, bl := range b.Levels() {
277+
if bl == nil || bl.Signatures == nil {
278+
continue
279+
}
280+
281+
for _, sig := range bl.Signatures {
282+
if sig.Domain == classifier.DomainFileSystem {
283+
fsSig := &classifier.Signature{
284+
Name: sig.Name,
285+
Domain: sig.Domain,
286+
Pattern: sig.Pattern,
287+
Filename: sig.Filename,
288+
Regexp: sig.Regexp,
289+
}
290+
allFilesystemSignatures = append(allFilesystemSignatures, fsSig)
291+
}
292+
}
293+
}
294+
295+
// Create a single SignatureMatchClassifier with all filesystem signatures
296+
// Use LevelAudit as default - individual signatures can still have their own severity
297+
return classifier.NewSignatureMatchClassifier("filesystem", classifier.LevelAudit, allFilesystemSignatures), nil
298+
}
299+
264300
func (b *Blocklists) Levels() map[common.Severity]*PerLevelBlocklist {
265301
res := make(map[common.Severity]*PerLevelBlocklist)
266302
if b.Barely != nil {

‎components/ee/agent-smith/pkg/config/config_test.go‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
// Copyright (c) 2024 Gitpod GmbH. All rights reserved.
2+
// Licensed under the GNU Affero General Public License (AGPL).
3+
// See License.AGPL.txt in the project root for license information.
4+
5+
package config
6+
7+
import (
8+
"testing"
9+
10+
"github.com/gitpod-io/gitpod/agent-smith/pkg/classifier"
11+
)
12+
13+
func TestFileClassifierIndependence(t *testing.T) {
14+
// Create a blocklist with both process and filesystem signatures
15+
blocklists := &Blocklists{
16+
Audit: &PerLevelBlocklist{
17+
Binaries: []string{"malware"}, // Process-related
18+
Signatures: []*classifier.Signature{
19+
{
20+
Name: "process-sig",
21+
Domain: classifier.DomainProcess,
22+
Pattern: []byte("process-pattern"),
23+
Filename: []string{"malware.exe"},
24+
},
25+
{
26+
Name: "filesystem-sig",
27+
Domain: classifier.DomainFileSystem,
28+
Pattern: []byte("filesystem-pattern"),
29+
Filename: []string{"virus.exe"},
30+
},
31+
},
32+
},
33+
Very: &PerLevelBlocklist{
34+
Signatures: []*classifier.Signature{
35+
{
36+
Name: "filesystem-sig-2",
37+
Domain: classifier.DomainFileSystem,
38+
Pattern: []byte("another-pattern"),
39+
Filename: []string{"trojan.exe"},
40+
},
41+
},
42+
},
43+
}
44+
45+
// Test process classifier (existing functionality - should be unchanged)
46+
processClass, err := blocklists.Classifier()
47+
if err != nil {
48+
t.Fatalf("Failed to create process classifier: %v", err)
49+
}
50+
if processClass == nil {
51+
t.Fatal("Process classifier should not be nil")
52+
}
53+
54+
// Test new filesystem classifier
55+
filesystemClass, err := blocklists.FileClassifier()
56+
if err != nil {
57+
t.Fatalf("Failed to create filesystem classifier: %v", err)
58+
}
59+
if filesystemClass == nil {
60+
t.Fatal("Filesystem classifier should not be nil")
61+
}
62+
63+
// Verify filesystem classifier has the right signatures
64+
fsSignatures := filesystemClass.GetFileSignatures()
65+
if len(fsSignatures) != 2 {
66+
t.Errorf("Expected 2 filesystem signatures, got %d", len(fsSignatures))
67+
}
68+
69+
// Verify signatures are filesystem domain only
70+
for _, sig := range fsSignatures {
71+
if sig.Domain != classifier.DomainFileSystem {
72+
t.Errorf("Expected filesystem domain signature, got %s", sig.Domain)
73+
}
74+
}
75+
76+
// Verify they are completely independent objects (can't directly compare different interface types)
77+
// Instead, verify they have different behaviors
78+
processSignatures := 0
79+
if pc, ok := processClass.(*classifier.CountingMetricsClassifier); ok {
80+
// Process classifier is wrapped in CountingMetricsClassifier
81+
_ = pc // Just verify the type cast works
82+
processSignatures = 1 // We know it exists because we created it
83+
}
84+
85+
filesystemSignatures := len(filesystemClass.GetFileSignatures())
86+
if filesystemSignatures == 0 {
87+
t.Error("Filesystem classifier should have signatures")
88+
}
89+
90+
// They should serve different purposes
91+
if processSignatures == 0 && filesystemSignatures == 0 {
92+
t.Error("At least one classifier should have content")
93+
}
94+
95+
// Test filesystem classifier functionality
96+
result, err := filesystemClass.MatchesFile("/nonexistent/virus.exe")
97+
if err != nil {
98+
t.Fatalf("Filesystem classification failed: %v", err)
99+
}
100+
if result == nil {
101+
t.Error("Expected non-nil classification result")
102+
}
103+
}
104+
105+
func TestFileClassifierEmptyConfig(t *testing.T) {
106+
// Test with nil blocklists
107+
var blocklists *Blocklists
108+
filesystemClass, err := blocklists.FileClassifier()
109+
if err != nil {
110+
t.Fatalf("Failed to create filesystem classifier from nil config: %v", err)
111+
}
112+
if filesystemClass == nil {
113+
t.Fatal("Filesystem classifier should not be nil even with empty config")
114+
}
115+
116+
// Should have no signatures
117+
signatures := filesystemClass.GetFileSignatures()
118+
if len(signatures) != 0 {
119+
t.Errorf("Expected 0 signatures from empty config, got %d", len(signatures))
120+
}
121+
}
122+
123+
func TestFileClassifierNoFilesystemSignatures(t *testing.T) {
124+
// Test with blocklists that have no filesystem signatures
125+
blocklists := &Blocklists{
126+
Audit: &PerLevelBlocklist{
127+
Binaries: []string{"malware"},
128+
Signatures: []*classifier.Signature{
129+
{
130+
Name: "process-only",
131+
Domain: classifier.DomainProcess,
132+
Pattern: []byte("process-pattern"),
133+
Filename: []string{"malware.exe"},
134+
},
135+
},
136+
},
137+
}
138+
139+
filesystemClass, err := blocklists.FileClassifier()
140+
if err != nil {
141+
t.Fatalf("Failed to create filesystem classifier: %v", err)
142+
}
143+
144+
// Should have no filesystem signatures
145+
signatures := filesystemClass.GetFileSignatures()
146+
if len(signatures) != 0 {
147+
t.Errorf("Expected 0 filesystem signatures, got %d", len(signatures))
148+
}
149+
}

0 commit comments

Comments
 (0)