Skip to content

Commit 36dbc77

Browse files
committed
refactor: consolidate constants and close response bodies
1 parent 6bf5b07 commit 36dbc77

File tree

4 files changed

+64
-50
lines changed

4 files changed

+64
-50
lines changed

agent/internal/posture/windows.go

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@ import (
55
"strings"
66
)
77

8+
const (
9+
newlineSeparator = "\n"
10+
ruleNamePrefix = "Rule Name:"
11+
displayNamePrefix = "displayName="
12+
displayNamePrefixLen = len(displayNamePrefix)
13+
powershellCmd = "powershell"
14+
powershellFlag = "-Command"
15+
)
16+
817
// WindowsCollector collects device posture on Windows systems
918
type WindowsCollector struct{}
1019

@@ -48,7 +57,7 @@ func (c *WindowsCollector) collectOSInfo(os *OperatingSystem) error {
4857
return err
4958
}
5059

51-
lines := strings.Split(output, "\n")
60+
lines := strings.Split(output, newlineSeparator)
5261
for _, line := range lines {
5362
line = strings.TrimSpace(line)
5463
if line == "" {
@@ -90,10 +99,10 @@ func (c *WindowsCollector) collectFirewallStatus(fw *FirewallStatus) error {
9099
if fw.Enabled {
91100
ruleOutput, err := runCommand("netsh", "advfirewall", "firewall", "show", "rule", "name=all")
92101
if err == nil {
93-
lines := strings.Split(ruleOutput, "\n")
102+
lines := strings.Split(ruleOutput, newlineSeparator)
94103
ruleCount := 0
95104
for _, line := range lines {
96-
if strings.HasPrefix(strings.TrimSpace(line), "Rule Name:") {
105+
if strings.HasPrefix(strings.TrimSpace(line), ruleNamePrefix) {
97106
ruleCount++
98107
}
99108
}
@@ -107,18 +116,18 @@ func (c *WindowsCollector) collectFirewallStatus(fw *FirewallStatus) error {
107116
// checkAntiVirus checks Windows Defender and other antivirus software
108117
func (c *WindowsCollector) checkAntiVirus() bool {
109118
// Check Windows Defender status
110-
output, err := runCommand("powershell", "-Command", "Get-MpComputerStatus | Select-Object AntivirusEnabled")
119+
output, err := runCommand(powershellCmd, powershellFlag, "Get-MpComputerStatus | Select-Object AntivirusEnabled")
111120
if err == nil && strings.Contains(strings.ToLower(output), "true") {
112121
return true
113122
}
114123

115124
// Check for other antivirus software using WMI
116125
output, err = runCommand("wmic", "/namespace:\\\\root\\SecurityCenter2", "path", "AntiVirusProduct", "get", "displayName", "/format:list")
117126
if err == nil {
118-
lines := strings.Split(output, "\n")
127+
lines := strings.Split(output, newlineSeparator)
119128
for _, line := range lines {
120129
line = strings.TrimSpace(line)
121-
if strings.HasPrefix(line, "displayName=") && len(line) > 12 {
130+
if strings.HasPrefix(line, displayNamePrefix) && len(line) > displayNamePrefixLen {
122131
return true // Found antivirus software
123132
}
124133
}
@@ -130,7 +139,7 @@ func (c *WindowsCollector) checkAntiVirus() bool {
130139
// checkSystemUpdated checks Windows Update status
131140
func (c *WindowsCollector) checkSystemUpdated() bool {
132141
// Check for pending updates using PowerShell
133-
output, err := runCommand("powershell", "-Command",
142+
output, err := runCommand(powershellCmd, powershellFlag,
134143
"Get-WUList -MicrosoftUpdate | Measure-Object | Select-Object -ExpandProperty Count")
135144
if err == nil {
136145
count := parseInt(strings.TrimSpace(output))
@@ -183,7 +192,7 @@ func (c *WindowsCollector) checkScreenLock() bool {
183192
}
184193

185194
// Check lock screen settings
186-
output, err = runCommand("powershell", "-Command",
195+
output, err = runCommand(powershellCmd, powershellFlag,
187196
"Get-ItemProperty -Path 'HKLM:\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Policies\\System' -Name DisableCAD -ErrorAction SilentlyContinue")
188197
if err == nil && !strings.Contains(output, "1") {
189198
return true // Ctrl+Alt+Del required (indicates password policy)

agent/internal/service/service_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,8 +229,8 @@ func TestService_updatePosture(t *testing.T) {
229229
return
230230
}
231231

232-
if req.ID != "test-device" {
233-
t.Errorf("Expected device ID 'test-device', got %s", req.ID)
232+
if req.ID != testDeviceID {
233+
t.Errorf("Expected device ID %q, got %s", testDeviceID, req.ID)
234234
}
235235

236236
if req.Posture == "" {

pkg/pki/ca_test.go

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,13 @@ func TestLoadOrCreateCA(t *testing.T) {
1919
certPath := filepath.Join(tmpDir, "ca.pem")
2020
keyPath := filepath.Join(tmpDir, "ca-key.pem")
2121

22+
const (
23+
testCAName = "test-ca"
24+
testCADefault = "test-ca-default"
25+
)
26+
2227
t.Run("creates new CA when files don't exist", func(t *testing.T) {
23-
ca, err := LoadOrCreateCA(certPath, keyPath, "test-ca", time.Hour*24*365)
28+
ca, err := LoadOrCreateCA(certPath, keyPath, testCAName, time.Hour*24*365)
2429
if err != nil {
2530
t.Fatalf("LoadOrCreateCA failed: %v", err)
2631
}
@@ -34,8 +39,8 @@ func TestLoadOrCreateCA(t *testing.T) {
3439
}
3540

3641
// Verify certificate properties
37-
if ca.cert.Subject.CommonName != "test-ca" {
38-
t.Errorf("Expected CommonName 'test-ca', got %s", ca.cert.Subject.CommonName)
42+
if ca.cert.Subject.CommonName != testCAName {
43+
t.Errorf("Expected CommonName %q, got %s", testCAName, ca.cert.Subject.CommonName)
3944
}
4045
if !ca.cert.IsCA {
4146
t.Error("Certificate should be a CA")
@@ -53,8 +58,8 @@ func TestLoadOrCreateCA(t *testing.T) {
5358
}
5459

5560
// Should load existing CA, not create new one with different name
56-
if ca2.cert.Subject.CommonName != "test-ca" {
57-
t.Errorf("Expected existing CommonName 'test-ca', got %s", ca2.cert.Subject.CommonName)
61+
if ca2.cert.Subject.CommonName != testCAName {
62+
t.Errorf("Expected existing CommonName %q, got %s", testCAName, ca2.cert.Subject.CommonName)
5863
}
5964
})
6065

@@ -63,7 +68,7 @@ func TestLoadOrCreateCA(t *testing.T) {
6368
certPath2 := filepath.Join(tmpDir2, "ca.pem")
6469
keyPath2 := filepath.Join(tmpDir2, "ca-key.pem")
6570

66-
ca, err := LoadOrCreateCA(certPath2, keyPath2, "test-ca-default", 0)
71+
ca, err := LoadOrCreateCA(certPath2, keyPath2, testCADefault, 0)
6772
if err != nil {
6873
t.Fatalf("LoadOrCreateCA failed: %v", err)
6974
}
@@ -153,7 +158,7 @@ func TestCertificateAuthority_IssueCertificate(t *testing.T) {
153158
certPath := filepath.Join(tmpDir, "ca.pem")
154159
keyPath := filepath.Join(tmpDir, "ca-key.pem")
155160

156-
ca, err := LoadOrCreateCA(certPath, keyPath, "test-ca", time.Hour*24)
161+
ca, err := LoadOrCreateCA(certPath, keyPath, testCAName, time.Hour*24)
157162
if err != nil {
158163
t.Fatalf("Failed to create CA: %v", err)
159164
}
@@ -249,7 +254,7 @@ func TestCertificateAuthority_SignCSR(t *testing.T) {
249254
certPath := filepath.Join(tmpDir, "ca.pem")
250255
keyPath := filepath.Join(tmpDir, "ca-key.pem")
251256

252-
ca, err := LoadOrCreateCA(certPath, keyPath, "test-ca", time.Hour*24)
257+
ca, err := LoadOrCreateCA(certPath, keyPath, testCAName, time.Hour*24)
253258
if err != nil {
254259
t.Fatalf("Failed to create CA: %v", err)
255260
}
@@ -377,7 +382,7 @@ func TestCertificateAuthority_CertificatePEM(t *testing.T) {
377382
certPath := filepath.Join(tmpDir, "ca.pem")
378383
keyPath := filepath.Join(tmpDir, "ca-key.pem")
379384

380-
ca, err := LoadOrCreateCA(certPath, keyPath, "test-ca", time.Hour*24)
385+
ca, err := LoadOrCreateCA(certPath, keyPath, testCAName, time.Hour*24)
381386
if err != nil {
382387
t.Fatalf("Failed to create CA: %v", err)
383388
}
@@ -404,8 +409,8 @@ func TestCertificateAuthority_CertificatePEM(t *testing.T) {
404409
t.Errorf("Failed to parse certificate from PEM: %v", err)
405410
}
406411

407-
if cert.Subject.CommonName != "test-ca" {
408-
t.Errorf("Expected CommonName 'test-ca', got %s", cert.Subject.CommonName)
412+
if cert.Subject.CommonName != testCAName {
413+
t.Errorf("Expected CommonName %q, got %s", testCAName, cert.Subject.CommonName)
409414
}
410415
})
411416
}

services/authz/server/server.go

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -418,16 +418,16 @@ func (s *Server) evaluateOPA(ctx context.Context, claims map[string]any, deviceI
418418

419419
start := time.Now()
420420
var resp *http.Response
421-
var callErr error
422421
retryErr := retry.Do(ctx, s.retryCfg, func() error {
423-
resp, callErr = s.client.Do(req)
424-
if callErr != nil {
425-
return callErr
422+
r, err := s.client.Do(req)
423+
if err != nil {
424+
return err
426425
}
427-
if resp.StatusCode >= 500 {
428-
_ = resp.Body.Close()
429-
return fmt.Errorf("opa temporary error: %d", resp.StatusCode)
426+
if r.StatusCode >= 500 {
427+
_ = r.Body.Close()
428+
return fmt.Errorf("opa temporary error: %d", r.StatusCode)
430429
}
430+
resp = r
431431
return nil
432432
})
433433
if retryErr != nil {
@@ -516,61 +516,61 @@ func parseXFCC(xfcc string) string {
516516

517517
func (s *Server) lookupDevice(ctx context.Context, deviceID string) map[string]any {
518518
if deviceID == "" || s.cfg.InventoryAPI == "" {
519-
return map[string]any{"id": deviceID, "posture": "unknown"}
519+
return map[string]any{"id": deviceID, "posture": statusUnknown}
520520
}
521521

522522
req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("%s/v1/devices/%s", s.cfg.InventoryAPI, deviceID), nil)
523523
if err != nil {
524524
log.Printf("inventory request build failed: %v", err)
525-
return map[string]any{"id": deviceID, "posture": "unknown"}
525+
return map[string]any{"id": deviceID, "posture": statusUnknown}
526526
}
527527

528528
start := time.Now()
529529
var resp *http.Response
530-
var callErr error
531530
retryErr := retry.Do(ctx, s.retryCfg, func() error {
532-
resp, callErr = s.invClient.Do(req)
533-
if callErr != nil {
534-
return callErr
531+
r, err := s.invClient.Do(req)
532+
if err != nil {
533+
return err
535534
}
536-
if resp.StatusCode >= 500 {
537-
_ = resp.Body.Close()
538-
return fmt.Errorf("inventory temporary error: %d", resp.StatusCode)
535+
if r.StatusCode >= 500 {
536+
_ = r.Body.Close()
537+
return fmt.Errorf("inventory temporary error: %d", r.StatusCode)
539538
}
539+
resp = r
540540
return nil
541541
})
542542
if retryErr != nil {
543543
telemetry.RecordDependencyRequest(ctx, "authz", "inventory", "lookup", time.Since(start), "error")
544544
log.Printf("inventory request failed: %v", retryErr)
545-
return map[string]any{"id": deviceID, "posture": "unknown"}
545+
return map[string]any{"id": deviceID, "posture": statusUnknown}
546546
}
547547
defer resp.Body.Close()
548548

549549
if resp.StatusCode == http.StatusNotFound {
550550
telemetry.RecordDependencyRequest(ctx, "authz", "inventory", "lookup", time.Since(start), "404")
551-
return map[string]any{"id": deviceID, "posture": "unregistered"}
551+
return map[string]any{"id": deviceID, "posture": statusUnregistered}
552552
}
553553
if resp.StatusCode != http.StatusOK {
554554
b, readErr := io.ReadAll(resp.Body)
555555
if readErr != nil {
556556
telemetry.RecordDependencyRequest(ctx, "authz", "inventory", "lookup", time.Since(start), fmt.Sprintf("%d", resp.StatusCode))
557557
log.Printf("inventory error %d: failed to read body: %v", resp.StatusCode, readErr)
558-
return map[string]any{"id": deviceID, "posture": "unknown"}
558+
return map[string]any{"id": deviceID, "posture": statusUnknown}
559559
}
560560
telemetry.RecordDependencyRequest(ctx, "authz", "inventory", "lookup", time.Since(start), fmt.Sprintf("%d", resp.StatusCode))
561561
log.Printf("inventory error %d: %s", resp.StatusCode, string(b))
562-
return map[string]any{"id": deviceID, "posture": "unknown"}
562+
return map[string]any{"id": deviceID, "posture": statusUnknown}
563563
}
564564

565565
var device inventoryDevice
566566
if err := json.NewDecoder(resp.Body).Decode(&device); err != nil {
567567
telemetry.RecordDependencyRequest(ctx, "authz", "inventory", "lookup", time.Since(start), "decode_error")
568568
log.Printf("inventory decode failed: %v", err)
569-
return map[string]any{"id": deviceID, "posture": "unknown"}
569+
return map[string]any{"id": deviceID, "posture": statusUnknown}
570570
}
571571

572572
if device.Posture == "" {
573-
device.Posture = "unknown"
573+
device.Posture = statusUnknown
574574
}
575575

576576
// Parse posture JSON to extract trust score
@@ -913,16 +913,16 @@ func (s *Server) verifyMFACode(ctx context.Context, sessionID, code string) (boo
913913

914914
start := time.Now()
915915
var resp *http.Response
916-
var callErr error
917916
retryErr := retry.Do(ctx, s.retryCfg, func() error {
918-
resp, callErr = s.client.Do(req)
919-
if callErr != nil {
920-
return callErr
917+
r, err := s.client.Do(req)
918+
if err != nil {
919+
return err
921920
}
922-
if resp.StatusCode >= 500 {
923-
_ = resp.Body.Close()
924-
return fmt.Errorf("mfa temporary error: %d", resp.StatusCode)
921+
if r.StatusCode >= 500 {
922+
_ = r.Body.Close()
923+
return fmt.Errorf("mfa temporary error: %d", r.StatusCode)
925924
}
925+
resp = r
926926
return nil
927927
})
928928
if retryErr != nil {

0 commit comments

Comments
 (0)