Skip to content

ci: add security action with govulncheck + enable gosec linter#2782

Open
andreynering wants to merge 4 commits intomainfrom
ci-security
Open

ci: add security action with govulncheck + enable gosec linter#2782
andreynering wants to merge 4 commits intomainfrom
ci-security

Conversation

@andreynering
Copy link
Copy Markdown
Member

@andreynering andreynering commented Apr 12, 2026

Some explanation:

  • govulncheck is a native Go tool that checks if we're using any vulnarable dependencies, including modules from go.mod and also Go itself. When it fails, we need to update that specific dependency or the minimal Go version to the latest patch.
  • gosec is a tool that checks for security issues on our Go code. We're running it via golangci-lint. When not relevant, can be ignored with //nolint:gosec.

@andreynering andreynering self-assigned this Apr 12, 2026
@andreynering andreynering changed the title ci: add security action to run govulncheck and gosec ci: add security action with govulncheck + enable gosec linter Apr 12, 2026
@andreynering andreynering marked this pull request as ready for review April 12, 2026 00:29
- Change file permissions from 0644 to 0600 for sensitive files
- Add nolint:gosec comments for intentional security exceptions:
  - G115: file descriptor integer conversion in term.go
  - G402: user-controlled TLS InsecureSkipVerify in node_http.go
  - G703: validated path traversal in reader.go and release/main.go
  - G404: non-security-critical rand usage in tests

Assisted-by: Kimi-K2.5 via Crush <crush@charm.land>
Copy link
Copy Markdown
Contributor

@trulede trulede left a comment

Choose a reason for hiding this comment

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

Only a comment on the 644 -> 600 changes.

return err
}
return os.WriteFile(node.checksumPath(), []byte(checksum), 0o644)
return os.WriteFile(node.checksumPath(), []byte(checksum), 0o600)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I just wonder about this general change: 644 -> 600, there are some circumstances where this might not be a good idea. Especially Docker when a container is run as root, since any created file (by the container) would not be visible to the user (if they would subsequently run task outside of a container).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@copilot What’s your opinion? Is this change a good idea or can it break stuff?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants