fix HIGH and MED severity gosec warnings

[CoinsNaulty]

Hi @derailed! I'm John Naulty and I work at Bison Trails. We're doing an internal hackathon this week.

Our team loves k9s, but it got flagged during a recent security review. Our team voted to use our hackathon time to patch vulnerabilities in our favorite OSS tools and specifically to try to get k9s approved for usage within our organization.

What's here:

Summary of findings

G101 - hardcoded credentials - 1 G204 - command injection - 2 G304 - File Path Injection - 14 G306 - File Creation Permissions - 2

Impact:

These changes should improve k9s security posture. We identified these vulnerabilities using Salus and the OSSF scorecard. I'm hoping to resolve all these issues in this pull-request

What's ~~next~~ else:

• ~~let me know if you'd like me to add gosec vulnerability scans to your CI system~~ :)
• added gosec vulnerability scans to Github actions
  • gosec is an open source security tool for analyzing go source code for security vulnerabilities.

Thank you so much for your work maintaining k9s, and thank you for taking the time to look at these proposed changes!

Below is a bit more information (taken from the commit message)

G101 Errors (hardcoded credentials, cwe-798)

Manual review showed up nothing suspect. It simply setting a variable for an environment variable name.

See: https://github.com/derailed/k9s/blob/a9ede22134ad68e7841b88294fd87328895b4604/internal/dao/ofaas.go#L49

CodehQL Query: https://github.com/github/codeql-go/blob/cd1e14ed09f4b56229b5c4fb7797203193b93897/ql/src/Security/CWE-798/HardcodedCredentials.ql

No results for derailed/k9s: https://lgtm.com/query/7538063270548585986/

G204 Errors (command-injection, cwe-78)

Initial Strategy: manually inspect code and traverse function calls to see if user input is used as a variable for the commands.

G204 Error for view/exec.go#L166: https://github.com/derailed/k9s/blob//a9ede22134ad68e7841b88294fd87328895b4604/internal/view/exec.go#L116

This is defined in function execute function execute is called by run: https://github.com/derailed/k9s/blob/a9ede22134ad68e7841b88294fd87328895b4604/internal/view/exec.go#L73 function run is used in 10 places across 4 files:

• internal/view/command.go
• internal/view/app.go
• internal/view/actions.go
• internal/view/exec.go

G204 Error for view/exec.go#L166: https://github.com/derailed/k9s/blob//a9ede22134ad68e7841b88294fd87328895b4604/internal/view/exec.go#L166

function oneShoot called by function runKuhttps://github.com/derailed/k9s/blob//a9ede22134ad68e7841b88294fd87328895b4604/internal/view/exec.go#L157 function runKu called by applyCMD + delCMD:

• https://github.com/derailed/k9s/blob/a9ede22134ad68e7841b88294fd87328895b4604/internal/view/dir.go#L192
• https://github.com/derailed/k9s/blob/a9ede22134ad68e7841b88294fd87328895b4604/internal/view/dir.go#L228

It does not appear that these are consuming user-provided inputs, although given the usecase of k9s, if someone is executing arbitrary code through k9s, what else are they able to do?

https://lgtm.com/query/6815845775730416472/

Query Taken from: https://github.com/github/codeql-go/blob/cd1e14ed09f4b56229b5c4fb7797203193b93897/ql/src/Security/CWE-078/CommandInjection.ql

G304 Errors (path-injection, cwe-22)

https://lgtm.com/query/9221441576788509510/

Query taken from: https://github.com/github/codeql-go/blob/cd1e14ed09f4b56229b5c4fb7797203193b93897/ql/src/Security/CWE-022/TaintedPath.ql

G306 Errors (file permission, cwe-276)

No CodeQL demo query currently exists for this.

The code is uscing 0644 for permissions for a config file. This does not seem to need the stricter restrictions as there is no private information in these files, unlike, perhaps, kubeconfig files or .aws/credentials.

Will be excluding this issue.

[anopheles]

This PR simply ignores warning for a specific/arbitrary tool your organization uses.

If your security review solely relies on comments that deactivate certain checks you should probably revise your security reviewing protocol. Perhaps this should be integrated into the tool and not this project?

[derailed]

@CoinsNaulty Thank you for you kind words and for hacking on k9s! I agree with @anopheles sentiments as turning off sec issues is likely not the best idea here. I am good about tightening the perms on various artifacts produced by k9s. Given that k9s runs out of cluster I feel the security stand is actually pretty good as most issues would be direct results of pilot errors. I do think having sec reports on gha is indeed a good idea so we can tighten potential holes as they occur. Does this make sense?

[uozalp]

@derailed While security is important, most of these findings look more like false positives or low-impact issues (e.g. env vars, file perms).

Might make sense to mark this stale unless there’s a concrete exploit path or strong demand to merge.