Skip to content

Enable gosec linter after fixing security issues #1378

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

Jump to bottom
Merged
craftleon merged 6 commits into from
Jan 7, 2026
Merged

Enable gosec linter after fixing security issues #1378

craftleon merged 6 commits into from
Jan 7, 2026

Conversation

@justin-layerv
Copy link
Contributor

@justin-layerv justin-layerv commented Jan 5, 2026 •
edited
Loading

Summary

Changes

Configuration Updates

  • Added gosec to enabled linters
  • Configured rule exclusions for false positives and intentional patterns:
    • G101 (hardcoded credentials): Variable naming false positives
    • G115 (integer overflow): Safe conversions (WASM bit extraction, timestamps, bounded lengths)
    • G204 (command injection): Uses exec.CommandContext with args array, not shell strings
    • G304 (file path traversal): All paths are admin-controlled (config files, CLI args)
    • G401/G501/G505 (weak crypto): MD5/SHA1 used for checksums, not security
    • G103 (unsafe pointer): Intentional for crypto/packet operations (path-based exclusion)
    • G104 (unhandled errors): Logging operations (path-based exclusion)
    • G407 (hardcoded IV): IV randomly generated at runtime before use
  • Fixed path regex patterns to be properly anchored (e.g., nhp/core/(packet|scheme|...))
  • G306 (file permissions) is NOT globally excluded - handled case-by-case

Security Fixes

  • G110 (decompression bomb): Added 10MB size limit in responder.go with proper overflow detection (rejects oversized data instead of silently truncating)
  • G306 (file permissions): Fixed kgc.go to use 0600 for config containing master private key, with os.Chmod to enforce permissions on existing files
  • G104 (error handling): Fixed rand.Read error handling in ztdo.go and noise.go - now panics on failure (security-critical IV generation)
  • G104 (error handling): Fixed zlib.NewReader error handling in responder.go
  • G302 (file permissions): Changed log file permissions to 0600

Nolint Comments

Added explanatory //nolint:gosec comments for remaining edge cases:

  • Crypto/packet unsafe pointer operations (performance-critical)
  • Math/rand usage for non-security purposes (packet obfuscation)
  • HTTP requests with configured URLs
  • G306 for intentionally world-readable files (public keys, config updates, SDP signaling)

Test Plan

  • Verify CI passes with gosec enabled
  • Confirm no new security warnings
  • Run existing tests to ensure no regressions

@justin-layerv
fix: enable gosec linter with appropriate exclusions
81df150 
- Enable gosec security linter in golangci.yml
- Configure exclusions for intentional patterns:
  - G101 (hardcoded credentials): false positives on variable names
  - G115 (integer overflow): safe conversions with masks/bounds
  - G204 (command injection): uses exec.CommandContext with separate args
  - G304 (file path traversal): application-controlled paths
  - G306 (file permissions): appropriate for config files
  - G401/G501/G505 (weak crypto): MD5/SHA1 used for checksums only
  - G103 (unsafe): intentional for crypto/packet operations
  - G104 (unhandled errors): logging operations
  - G407 (hardcoded IV): IV randomly generated before use
- Fix G110 (decompression bomb) with 10MB limit in responder.go
- Add nolint comments with explanations for edge cases
- Update log file permissions to 0600

Closes #1373
@justin-layerv
fix(security): properly detect decompression overflow and handle rand…
82116e1 
… errors

- Fix decompression bomb protection to detect and reject oversized data
  instead of silently truncating (which caused data corruption)
- Handle zlib.NewReader errors instead of ignoring them
- Panic on crypto/rand.Read failures for IV generation (security-critical)
@justin-layerv
fix(security): enforce G306 file permissions and fix golangci path re…
b4148a7 
…gexes

Security improvements:
- Fix kgc.go to write config with 0600 (contains master private key)
- Remove G306 from global gosec exclusions - now checked case-by-case
- Add targeted nolint for intentional 0644 permissions:
  - Public keys (intentionally world-readable)
  - Config file updates (standard for config files)
  - SDP signaling data (not sensitive)

Config improvements:
- Properly anchor path regexes in .golangci.yml exclusion rules
- Add better documentation for remaining global exclusions
- Fix paths in formatters exclusions
@justin-layerv
fix(security): ensure file permissions are enforced on existing files
f8015ed 
- Add os.Chmod after WriteFile in kgc.go because WriteFile only sets
  permissions on newly created files, not existing ones
- Fix path exclusion patterns to use trailing slashes for directories
@justin-layerv justin-layerv self-assigned this Jan 5, 2026
craftleon
craftleon previously approved these changes Jan 7, 2026
View reviewed changes
@craftleon craftleon merged commit c05a0de into main Jan 7, 2026
5 checks passed
@craftleon craftleon deleted the fix/enable-gosec-linter branch January 7, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@craftleon craftleon craftleon approved these changes

@wenhulove333 wenhulove333 Awaiting requested review from wenhulove333

@cb403 cb403 Awaiting requested review from cb403

Assignees

@justin-layerv justin-layerv

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enable gosec linter after fixing existing security issues

3 participants

@justin-layerv @craftleon