Skip to content

Commit 1d96648

Browse files
committed
Suppress CodeQL path-injection false positives on os.Stat checks
The four os.Stat(dir) pre-checks in applyGitInstallExisting, applyGitInstallFresh, installExtractionSameDigestNewClients, and installExtractionFresh are flagged by CodeQL's go/path-injection query because 'dir' originates (transitively) from user-supplied client names. The paths are safe: client names are validated against the known skill-supporting client list, skill names pass validateLocalPath, and PathResolver.GetSkillPath constructs paths from fixed base directories which are then confirmed absolute and traversal-free by validateResolvedDir. Add the same // lgtm[go/path-injection] suppression used in pkg/skills/gitresolver/writer.go for the identical taint flow. Signed-off-by: Samuele Verzi <samu@stacklok.com> Made-with: Cursor
1 parent b5ad7fd commit 1d96648

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

pkg/skills/skillsvc/skillsvc.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,7 @@ func (s *service) applyGitInstallExisting(
806806
}
807807
for _, ct := range toWrite {
808808
dir := filepath.Clean(clientDirs[ct])
809-
if _, statErr := os.Stat(dir); statErr == nil && !opts.Force {
809+
if _, statErr := os.Stat(dir); statErr == nil && !opts.Force { // lgtm[go/path-injection]
810810
return nil, httperr.WithCode(
811811
fmt.Errorf("directory %q exists but is not managed by ToolHive; use force to overwrite", dir),
812812
http.StatusConflict,
@@ -837,7 +837,7 @@ func (s *service) applyGitInstallFresh(
837837
) (*skills.InstallResult, error) {
838838
for _, ct := range clientTypes {
839839
dir := filepath.Clean(clientDirs[ct])
840-
if _, statErr := os.Stat(dir); statErr == nil && !opts.Force {
840+
if _, statErr := os.Stat(dir); statErr == nil && !opts.Force { // lgtm[go/path-injection]
841841
return nil, httperr.WithCode(
842842
fmt.Errorf("directory %q exists but is not managed by ToolHive; use force to overwrite", dir),
843843
http.StatusConflict,
@@ -1405,7 +1405,7 @@ func (s *service) installExtractionSameDigestNewClients(
14051405
var written []string
14061406
for _, ct := range toWrite {
14071407
dir := filepath.Clean(clientDirs[ct])
1408-
if _, statErr := os.Stat(dir); statErr == nil && !opts.Force {
1408+
if _, statErr := os.Stat(dir); statErr == nil && !opts.Force { // lgtm[go/path-injection]
14091409
removeSkillDirs(s.installer, clientDirs, written)
14101410
return nil, httperr.WithCode(
14111411
fmt.Errorf("directory %q exists but is not managed by ToolHive; use force to overwrite", dir),
@@ -1471,7 +1471,7 @@ func (s *service) installExtractionFresh(
14711471
) (*skills.InstallResult, error) {
14721472
for _, ct := range clientTypes {
14731473
dir := filepath.Clean(clientDirs[ct])
1474-
if _, statErr := os.Stat(dir); statErr == nil && !opts.Force {
1474+
if _, statErr := os.Stat(dir); statErr == nil && !opts.Force { // lgtm[go/path-injection]
14751475
return nil, httperr.WithCode(
14761476
fmt.Errorf("directory %q exists but is not managed by ToolHive; use force to overwrite", dir),
14771477
http.StatusConflict,

0 commit comments

Comments
 (0)