Skip to content

feat(procstat): add sudo and custom path support #1382

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.

Sign up for GitHub

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

Merged
kongfei605 merged 1 commit into from
Dec 24, 2025
Merged

feat(procstat): add sudo and custom path support #1382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions inputs/procstat/procstat.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ import (
"flashcat.cloud/categraf/types"
)

var execCommand = exec.Command
var execLookPath = exec.LookPath

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The execLookPath variable is declared but never used in the current implementation. Since execJstat no longer calls exec.LookPath, this variable should be removed unless there are plans to validate the jstat path before execution.

Suggested change
var execLookPath = exec.LookPath

Copilot uses AI. Check for mistakes.

const inputName = "procstat"

type PID int32
Expand All @@ -38,6 +41,9 @@ type Instance struct {
GatherPerPid bool `toml:"gather_per_pid"`
GatherMoreMetrics []string `toml:"gather_more_metrics"`

UseSudo bool `toml:"use_sudo"`

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing documentation: The new configuration fields UseSudo and PathJstat should have documentation comments explaining their purpose, usage, and any security considerations (especially for UseSudo). This is important for users configuring the plugin.

Suggested change
UseSudo bool `toml:"use_sudo"`
// UseSudo controls whether external tools such as jstat are executed via
// "sudo". Enable this only if the monitored commands require elevated
// privileges and you have configured sudoers appropriately (for example,
// to allow passwordless execution of the required binaries). Misuse or
// misconfiguration of sudo can introduce security risks.
UseSudo bool `toml:"use_sudo"`
// PathJstat optionally specifies the absolute path to the jstat binary to
// use when collecting JVM-related metrics. If left empty, the plugin will
// search for jstat in the system PATH.

Copilot uses AI. Check for mistakes.
PathJstat string `toml:"path_jstat"`

SearchExecRegexp string `toml:"search_exec_regexp"`
searchExecRegexp *regexp.Regexp `toml:"-"`

Expand Down Expand Up @@ -83,7 +89,9 @@ func (ins *Instance) Init() error {
} else {
return errors.New("the fields should not be all blank: search_exec_substring, search_cmdline_substring, search_win_service")
}

if ins.PathJstat == "" {
ins.PathJstat = "jstat"
}
if ins.LabelsFromCmdlineRegexp != "" {
r := regexp.MustCompile(ins.LabelsFromCmdlineRegexp)
extractLabelKey := r.SubexpNames()
Expand Down Expand Up @@ -535,7 +543,7 @@ func (ins *Instance) gatherJvm(slist *types.SampleList, procs map[PID]Process, t
attachPid = true
}
for pid := range procs {
jvmStat, err := execJstat(pid)
jvmStat, err := ins.execJstat(pid)
if err != nil {
log.Println("E! failed to exec jstat:", err)
continue
Expand All @@ -551,15 +559,26 @@ func (ins *Instance) gatherJvm(slist *types.SampleList, procs map[PID]Process, t
}
}

func execJstat(pid PID) (map[string]string, error) {
bin, err := exec.LookPath("jstat")
if err != nil {
return nil, err
func (ins *Instance) execJstat(pid PID) (map[string]string, error) {
bin := ins.PathJstat
args := []string{}
if ins.UseSudo {
args = append(args, ins.PathJstat)
Comment on lines +563 to +566

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security concern: When UseSudo is enabled, the custom PathJstat is passed directly to sudo without validation. This could allow arbitrary command execution if PathJstat contains malicious input (e.g., "../../../bin/malicious"). Consider validating that PathJstat is an absolute path or exists in a safe location, or use exec.LookPath to resolve the binary path before constructing the sudo command.

Suggested change
bin := ins.PathJstat
args := []string{}
if ins.UseSudo {
args = append(args, ins.PathJstat)
jstatPath := ins.PathJstat
if jstatPath == "" {
jstatPath = "jstat"
}
resolvedJstatPath, err := execLookPath(jstatPath)
if err != nil {
return nil, fmt.Errorf("failed to locate jstat binary '%s': %w", jstatPath, err)
}
bin := resolvedJstatPath
args := []string{}
if ins.UseSudo {
args = append(args, resolvedJstatPath)

Copilot uses AI. Check for mistakes.
bin = "sudo"
}

out, err := exec.Command(bin, "-gc", fmt.Sprint(pid)).Output()
args = append(args, "-gc", fmt.Sprint(pid))
cmd := execCommand(bin, args...)
out, err := cmd.Output()
if err != nil {
return nil, err
if exitErr, ok := err.(*exec.ExitError); ok {
return nil, fmt.Errorf("jstat exec failed with exit code %d: %w, stderr: '%s', cmd: '%s'",
exitErr.ExitCode(),
err,
string(exitErr.Stderr),
cmd.String())
}
return nil, fmt.Errorf("jstat command failed to start: %w, cmd: '%s'", err, cmd.String())
}

jvm := strings.Fields(string(out))
Expand Down
78 changes: 78 additions & 0 deletions inputs/procstat/procstat_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package procstat

import (
"os/exec"
"testing"

"github.com/stretchr/testify/assert"
)

func TestExecJstat_Command(t *testing.T) {
defer func() {
execCommand = exec.Command
execLookPath = exec.LookPath
}()
execLookPath = func(file string) (string, error) {
return file, nil
}
Comment on lines +13 to +17

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The execLookPath variable is mocked but never used in the current implementation of execJstat. Since the new implementation doesn't call exec.LookPath anymore (it uses PathJstat directly), this mock setup is unnecessary and should be removed.

Suggested change
execLookPath = exec.LookPath
}()
execLookPath = func(file string) (string, error) {
return file, nil
}
}()

Copilot uses AI. Check for mistakes.

tests := []struct {
name string
useSudo bool
pathJstat string
pid PID
expectedCmd string
// expectedArgs ...

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commented line "expectedArgs ..." suggests incomplete test implementation. Either remove this comment if the field is not needed, or implement complete argument validation for all test cases.

Suggested change
// expectedArgs ...

Copilot uses AI. Check for mistakes.
}{
{
name: "Default",
useSudo: false,
pathJstat: "jstat",
pid: 1234,
expectedCmd: "jstat",
},
{
name: "Sudo Enabled",
useSudo: true,
pathJstat: "jstat",
pid: 1234,
expectedCmd: "sudo",
},
{
name: "Custom Path",
useSudo: false,
pathJstat: "/usr/bin/jstat",
pid: 1234,
expectedCmd: "/usr/bin/jstat",
},
}
Comment on lines +19 to +48

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test coverage missing: The test doesn't cover the combination of UseSudo with a custom path (e.g., UseSudo=true with PathJstat="/usr/bin/jstat"). This is an important scenario to test since it represents a realistic use case.

Copilot uses AI. Check for mistakes.

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
capturedCmd := ""
capturedArgs := []string{}
execCommand = func(command string, args ...string) *exec.Cmd {
capturedCmd = command
capturedArgs = args
return exec.Command("echo", "")
}

ins := &Instance{
UseSudo: tt.useSudo,
PathJstat: tt.pathJstat,
}
if ins.PathJstat == "" {
ins.PathJstat = "jstat"
}
Comment on lines +64 to +66

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test duplicates the default initialization logic from the Init method. The test manually sets PathJstat to "jstat" if empty, which duplicates logic that should be in the Init method. Consider calling ins.Init() instead to ensure the test reflects the actual initialization behavior.

Suggested change
if ins.PathJstat == "" {
ins.PathJstat = "jstat"
}
ins.Init()

Copilot uses AI. Check for mistakes.

// We need to call ins.execJstat
// It's private, so we can call it if we are in same package.
_, _ = ins.execJstat(tt.pid)

assert.Equal(t, tt.expectedCmd, capturedCmd)
if tt.useSudo {
assert.Equal(t, tt.pathJstat, capturedArgs[0])
}
Comment on lines +72 to +75

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test only validates the command name captured in sudo mode but doesn't verify the complete argument list in all cases. The test should assert all expected arguments for each test case. For example, in the "Default" and "Custom Path" cases, the arguments should include "-gc" and "1234". In the "Sudo Enabled" case, all arguments including "jstat", "-gc", and "1234" should be verified. Consider adding an expectedArgs field to the test struct and verifying it for all test cases.

Copilot uses AI. Check for mistakes.
})
}
}
Loading