From 42bb15794676bb080bad79325db99d9a2cdd7b09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D7=A0=CF=85=CE=B1=CE=B7=20=D7=A0=CF=85=CE=B1=CE=B7=D1=95?= =?UTF-8?q?=CF=83=CE=B7?= Date: Thu, 17 Sep 2026 18:10:19 -0700 Subject: [PATCH] test: cover reachable coverage gaps in validation, process, and agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds tests for four reachable statement blocks that just-test's filtered coverage profile flagged as uncovered: - internal/validation/validation.go: the account_name validator closure (length and pattern checks) and its customHints entry, exercised via validation.Var/Struct. - internal/telemetry/process/self.go: the error branch inside memoryInfoFn when the underlying MemoryInfo call fails. gopsutil never returns an error from MemoryInfo() on any platform this runs on, so the raw call is now wrapped in its own injectable (procMemoryInfoFn) the same way cpuPercentFn already is, and the default memoryInfoFn is a named function so export_test.go's Reset restores the exact same function rather than a duplicate closure at a different source location. - internal/agent/handler.go: the "missing or malformed job signature" reason, hit when PKI is enabled and job data is not a signed envelope, alongside the existing bad-signature and not-enrolled cases. The remaining 8 zero-coverage blocks are deliberate and untouched: four defense-in-depth validation.Struct() calls (file_upload.go, reboot_post.go, shutdown_post.go, container_remove.go, container_image_remove.go) and three documented-unreachable branches (process/debian.go, agent/seed.go, tracing/tracer.go). 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- internal/agent/handler_public_test.go | 31 ++++++ internal/telemetry/process/export_test.go | 19 +++- internal/telemetry/process/self.go | 26 +++-- .../telemetry/process/self_public_test.go | 27 +++++ internal/validation/validation_public_test.go | 105 ++++++++++++++++++ 5 files changed, 195 insertions(+), 13 deletions(-) diff --git a/internal/agent/handler_public_test.go b/internal/agent/handler_public_test.go index 113a403b0..d8fd4ddf1 100644 --- a/internal/agent/handler_public_test.go +++ b/internal/agent/handler_public_test.go @@ -1480,6 +1480,37 @@ func (s *HandlerPublicTestSuite) TestHandleJobMessageWithSignedEnvelope() { s.Contains(err.Error(), "no cached controller public key") }, }, + { + name: "when job data is not a signed envelope rejects distinctly", + setupPKI: func() { + m := pki.New(memfs.New(), "/tmp/pki", "agent") + s.Require().NoError(m.LoadOrGenerate()) + m.SetControllerPublicKey(controllerPub) + agent.SetAgentPKIManager(s.testAgent, m) + }, + cleanupPKI: func() { + agent.SetAgentPKIManager(s.testAgent, nil) + }, + setupMsg: func(ctrl *gomock.Controller) jetstream.Msg { + return newTestMsg(ctrl, "jobs.query.test-agent", []byte("unwrapped-job")) + }, + setupMocks: func() { + // Plain, unsigned job data: PKI is enabled, so this is + // rejected for missing an envelope rather than parsed. + s.expectNotAnswered("unwrapped-job") + s.mockJobClient.EXPECT(). + GetJobData(gomock.Any(), "jobs.unwrapped-job"). + Return([]byte(`{"id":"unwrapped-job","operation":{"type":"node.hostname.get"}}`), nil) + + // No WriteStatusEvent/WriteJobResponse expectations: an + // unsigned envelope must be refused before touching the + // operation. + }, + validateFunc: func(err error) { + s.Error(err) + s.Contains(err.Error(), "job data is not a signed envelope") + }, + }, } for _, tt := range tests { diff --git a/internal/telemetry/process/export_test.go b/internal/telemetry/process/export_test.go index 074872834..988027319 100644 --- a/internal/telemetry/process/export_test.go +++ b/internal/telemetry/process/export_test.go @@ -65,12 +65,19 @@ func SetMemoryInfoFn( // ResetMemoryInfoFn restores the default memoryInfoFn. func ResetMemoryInfoFn() { - memoryInfoFn = func(proc *gopsutil.Process) (uint64, error) { - info, err := proc.MemoryInfo() - if err != nil { - return 0, err - } + memoryInfoFn = defaultMemoryInfoFn +} + +// SetProcMemoryInfoFn overrides the procMemoryInfoFn injectable for testing. +func SetProcMemoryInfoFn( + fn func(*gopsutil.Process) (*gopsutil.MemoryInfoStat, error), +) { + procMemoryInfoFn = fn +} - return info.RSS, nil +// ResetProcMemoryInfoFn restores the default procMemoryInfoFn. +func ResetProcMemoryInfoFn() { + procMemoryInfoFn = func(proc *gopsutil.Process) (*gopsutil.MemoryInfoStat, error) { + return proc.MemoryInfo() } } diff --git a/internal/telemetry/process/self.go b/internal/telemetry/process/self.go index 71da1a0c0..1fc0d8526 100644 --- a/internal/telemetry/process/self.go +++ b/internal/telemetry/process/self.go @@ -33,15 +33,27 @@ import ( var ( newProcessFn = gopsutil.NewProcess cpuPercentFn = func(proc *gopsutil.Process) (float64, error) { return proc.CPUPercent() } - memoryInfoFn = func(proc *gopsutil.Process) (uint64, error) { - info, err := proc.MemoryInfo() - if err != nil { - return 0, err - } + // procMemoryInfoFn wraps the raw gopsutil call so the error branch in + // defaultMemoryInfoFn below can be exercised: gopsutil's MemoryInfo() + // never fails in practice on any platform this runs on, so the failure + // path needs its own seam to test. + procMemoryInfoFn = func(proc *gopsutil.Process) (*gopsutil.MemoryInfoStat, error) { return proc.MemoryInfo() } + memoryInfoFn = defaultMemoryInfoFn +) - return info.RSS, nil +// defaultMemoryInfoFn is the default value of memoryInfoFn, named so +// ResetMemoryInfoFn in export_test.go can restore this exact function +// rather than a duplicate closure at a different source location. +func defaultMemoryInfoFn( + proc *gopsutil.Process, +) (uint64, error) { + info, err := procMemoryInfoFn(proc) + if err != nil { + return 0, err } -) + + return info.RSS, nil +} type provider struct { pid int32 diff --git a/internal/telemetry/process/self_public_test.go b/internal/telemetry/process/self_public_test.go index e96303f19..307770a41 100644 --- a/internal/telemetry/process/self_public_test.go +++ b/internal/telemetry/process/self_public_test.go @@ -162,6 +162,33 @@ func (suite *ProcessPublicTestSuite) TestGetMetricsWithInjection() { suite.Contains(err.Error(), "get memory info") }, }, + { + name: "returns error when the underlying MemoryInfo call fails", + pid: 0, + setup: func() { + process.SetNewProcessFn(func(_ int32) (*gopsutil.Process, error) { + return &gopsutil.Process{}, nil + }) + process.SetCPUPercentFn(func(_ *gopsutil.Process) (float64, error) { + return 1.5, nil + }) + process.SetProcMemoryInfoFn( + func(_ *gopsutil.Process) (*gopsutil.MemoryInfoStat, error) { + return nil, errors.New("memory info error") + }, + ) + }, + teardown: func() { + process.ResetNewProcessFn() + process.ResetCPUPercentFn() + process.ResetProcMemoryInfoFn() + }, + validateFunc: func(got *process.Metrics, err error) { + suite.Nil(got) + suite.Error(err) + suite.Contains(err.Error(), "get memory info") + }, + }, } for _, tc := range tests { diff --git a/internal/validation/validation_public_test.go b/internal/validation/validation_public_test.go index 8a51ae87e..10b8dc873 100644 --- a/internal/validation/validation_public_test.go +++ b/internal/validation/validation_public_test.go @@ -737,6 +737,111 @@ func (s *ValidationPublicTestSuite) TestSigningKey() { }) } +func (s *ValidationPublicTestSuite) TestAccountName() { + tests := []struct { + name string + field string + validateFunc func(bool) + }{ + { + name: "when simple lowercase name", + field: "john", + validateFunc: func(got bool) { + s.True(got) + }, + }, + { + name: "when name with digits hyphen and underscore", + field: "j0hn-doe_2", + validateFunc: func(got bool) { + s.True(got) + }, + }, + { + name: "when machine account with trailing dollar sign", + field: "host$", + validateFunc: func(got bool) { + s.True(got) + }, + }, + { + name: "when exactly the maximum length", + field: strings.Repeat("a", 32), + validateFunc: func(got bool) { + s.True(got) + }, + }, + { + name: "when one character over the maximum length", + field: strings.Repeat("a", 33), + validateFunc: func(got bool) { + s.False(got) + }, + }, + { + name: "when name contains an uppercase letter", + field: "John", + validateFunc: func(got bool) { + s.False(got) + }, + }, + { + name: "when name starts with a digit", + field: "1john", + validateFunc: func(got bool) { + s.False(got) + }, + }, + { + name: "when name starts with a hyphen", + field: "-john", + validateFunc: func(got bool) { + s.False(got) + }, + }, + { + name: "when name contains a slash", + field: "jo/hn", + validateFunc: func(got bool) { + s.False(got) + }, + }, + { + name: "when name contains a space", + field: "jo hn", + validateFunc: func(got bool) { + s.False(got) + }, + }, + { + name: "when empty string", + field: "", + validateFunc: func(got bool) { + s.False(got) + }, + }, + } + + for _, tt := range tests { + s.Run(tt.name, func() { + _, ok := validation.Var(tt.field, "account_name") + tt.validateFunc(ok) + }) + } + + s.Run("invalid name shows hint through struct validation", func() { + type nameReq struct { + Name string `validate:"required,account_name"` + } + + errMsg, ok := validation.Struct(nameReq{Name: "-bad"}) + s.False(ok) + s.Contains(errMsg, "account_name") + s.Contains(errMsg, "not a valid account name") + s.Contains(errMsg, "max 32 characters") + }) +} + func (s *ValidationPublicTestSuite) TestAtLeastOneField() { type allPointers struct { Shell *string