Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
73 changes: 57 additions & 16 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -1,34 +1,75 @@
name: CI

on:
push:
branches:
- master
pull_request:
branches:
- master
schedule:
# Run daily at 2 AM UTC to check for new vulnerabilities
- cron: "0 2 * * *"

permissions:
contents: read

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
test:
timeout-minutes: 15
strategy:
matrix:
go-version: [1.23.x]
go-version: [1.25.x, 1.24.x]
os: [ubuntu-latest]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/setup-go@v3
- uses: actions/checkout@v6
with:
persist-credentials: false

- uses: actions/setup-go@v6
with:
go-version: ${{ matrix.go-version }}
- uses: actions/checkout@v3
- name: Go Lint Standard
uses: golangci/golangci-lint-action@v3
cache: true

- name: golangci-lint
uses: golangci/golangci-lint-action@v9
with:
version: v2.7.2

- name: Build
run: go build -v ./...

- name: Run govulncheck
uses: golang/govulncheck-action@v1
with:
go-version-input: ${{ matrix.go-version }}

- name: Test
run: go test -race ./...

jirabot:
runs-on: ubuntu-latest
defaults:
run:
working-directory: jirabot
steps:
- name: Checkout
uses: actions/checkout@v6

- name: Setup Node.js
uses: actions/setup-node@v6
with:
version: v1.63
args: "--out-${NO_FUTURE}format colored-line-number --timeout=15m"
- run: go vet ./...
- run: go test ./...
# Copied from .travis.yml but never migrated to github actions
# - language: node_js
# node_js: 10
# script:
# - cd jirabot
# - yarn
# - yarn ci
node-version: 25.x
cache: yarn
cache-dependency-path: jirabot/yarn.lock

- name: Install dependencies
run: yarn install --frozen-lockfile

- name: Run jirabot CI
run: yarn ci
72 changes: 62 additions & 10 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,65 @@
linters-settings:
gocritic:
disabled-checks:
- ifElseChain
- elseif
version: "2"

run:
timeout: 5m
tests: true

formatters:
enable:
- gofumpt

linters:
enable:
- gofmt
- gocritic
- unconvert
- revive
- govet
# Core recommended linters
- errcheck # Checks for unchecked errors
- govet # Go vet checks
- ineffassign # Detects ineffectual assignments
- staticcheck # Advanced static analysis
- unused # Finds unused code

# Code quality
- misspell # Finds commonly misspelled words
- unconvert # Unnecessary type conversions (already enabled in original)
- unparam # Finds unused function parameters
- gocritic # Various checks (already enabled in original)
- revive # Fast, configurable linter (already enabled in original)

# Security and best practices
- gosec # Security-focused linter
- bodyclose # Checks HTTP response body closed

settings:
gocritic:
disabled-checks:
- ifElseChain
- elseif

govet:
enable-all: true
disable:
- shadow
- fieldalignment

revive:
enable-all-rules: false

exclusions:
rules:
# Exclude specific revive rules
- linters:
- revive
text: "package-comments"

- linters:
- revive
text: "exported"

# Exclude specific staticcheck rules
- linters:
- staticcheck
text: "ST1005"

# Exclude specific gocritic rules
- linters:
- gocritic
text: "ifElseChain"
4 changes: 2 additions & 2 deletions base/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func NewBaseOAuthDB(db *sql.DB) *BaseOAuthDB {

func (d *BaseOAuthDB) GetState(state string) (*OAuthRequest, error) {
var oauthState OAuthRequest
row := d.DB.QueryRow(`SELECT identifier, conv_id, msg_id, is_complete
row := d.QueryRow(`SELECT identifier, conv_id, msg_id, is_complete
FROM oauth_state
WHERE state = ?`, state)
err := row.Scan(&oauthState.TokenIdentifier, &oauthState.ConvID,
Expand Down Expand Up @@ -97,7 +97,7 @@ func NewOAuthDB(db *sql.DB) *OAuthDB {
func (d *OAuthDB) GetToken(identifier string) (*oauth2.Token, error) {
var token oauth2.Token
var expiry int64
row := d.DB.QueryRow(`SELECT access_token, token_type, refresh_token, ROUND(UNIX_TIMESTAMP(expiry))
row := d.QueryRow(`SELECT access_token, token_type, refresh_token, ROUND(UNIX_TIMESTAMP(expiry))
FROM oauth
WHERE identifier = ?`, identifier)
err := row.Scan(&token.AccessToken, &token.TokenType,
Expand Down
3 changes: 1 addition & 2 deletions base/email.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ type Emailer interface {
Send(address, subject, message string) error
}

type DummyEmailer struct {
}
type DummyEmailer struct{}

func (d DummyEmailer) Send(_, subject, _ string) error {
fmt.Printf("subject: %s\n", subject)
Expand Down
6 changes: 2 additions & 4 deletions base/errors.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package base

import (
"time"

"runtime/debug"

"os"
"runtime/debug"
"time"

"golang.org/x/sync/errgroup"
)
Expand Down
6 changes: 5 additions & 1 deletion base/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package base
import (
"context"
"net/http"
"time"
)

type HTTPSrv struct {
Expand All @@ -15,7 +16,10 @@ func NewHTTPSrv(stats *StatsRegistry, debugConfig *ChatDebugOutputConfig) *HTTPS
return &HTTPSrv{
DebugOutput: NewDebugOutput("HTTPSrv", debugConfig),
Stats: stats.SetPrefix("HTTPSrv"),
srv: &http.Server{Addr: ":8080"},
srv: &http.Server{
Addr: ":8080",
ReadHeaderTimeout: 10 * time.Second,
},
}
}

Expand Down
21 changes: 11 additions & 10 deletions base/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,7 @@ func (s *Server) listenForMsgs(shutdownCh chan struct{}, sub *kbchat.Subscriptio
}
continue
case strings.HasPrefix(cmd, fmt.Sprintf("!%s", feedbackCmd(s.kbc.GetUsername()))):
if err := s.handleFeedback(msg); err != nil {
s.Errorf("listenForMsgs: unable to handleFeedback: %v", err)
}
s.handleFeedback(msg)
continue
}
}
Expand Down Expand Up @@ -291,7 +289,7 @@ func (s *Server) handleLogSend(msg chat1.MsgSummary) error {
}
outputBytes, err := io.ReadAll(output)
if err != nil {
s.Errorf("unable to read ouput: %v", err)
s.Errorf("unable to read output: %v", err)
return err
}
if len(outputBytes) > 0 {
Expand Down Expand Up @@ -345,8 +343,8 @@ func (s *Server) handlePProf(msg chat1.MsgSummary) error {
// Cleanup after the file is sent.
time.Sleep(time.Minute)
s.Debug("cleaning up %s", outfile)
if err = os.Remove(outfile); err != nil {
s.Errorf("unable to clean up %s: %v", outfile, err)
if rmErr := os.Remove(outfile); rmErr != nil {
s.Errorf("unable to clean up %s: %v", outfile, rmErr)
}
}()
if _, err := s.kbc.SendAttachmentByConvID(msg.ConvID, outfile, ""); err != nil {
Expand Down Expand Up @@ -389,7 +387,7 @@ func (s *Server) handleStack(msg chat1.MsgSummary) error {
return s.kbfsDebugOutput(msg, stack, "stack")
}

func (s *Server) handleFeedback(msg chat1.MsgSummary) error {
func (s *Server) handleFeedback(msg chat1.MsgSummary) {
toks := strings.Split(strings.TrimSpace(msg.Content.Text.Body), " ")
if len(toks) < 3 {
s.ChatEcho(msg.ConvID, "Woah there @%s, I can't deliver a blank message...not again. What did you want to say?",
Expand All @@ -400,7 +398,6 @@ func (s *Server) handleFeedback(msg chat1.MsgSummary) error {
s.ChatEcho(msg.ConvID, "Roger that @%s, passed this along to my humans :robot_face:",
msg.Sender.Username)
}
return nil
}

func (s *Server) kbfsDebugOutput(msg chat1.MsgSummary, data []byte, operation string) error {
Expand All @@ -415,8 +412,12 @@ func (s *Server) kbfsDebugOutput(msg chat1.MsgSummary, data []byte, operation st
}
fileName := fmt.Sprintf("%s-%d.txt", operation, time.Now().Unix())
filePath := fmt.Sprintf("/tmp/%s", fileName)
defer os.Remove(filePath)
if err := os.WriteFile(filePath, data, 0644); err != nil {
defer func() {
if rmErr := os.Remove(filePath); rmErr != nil {
s.Errorf("unable to clean up %s: %v", filePath, rmErr)
}
}()
if err := os.WriteFile(filePath, data, 0o600); err != nil {
return fmt.Errorf("kbfsOutput: failed to write %s output: %s", operation, err)
}
if err := s.runOptions.Command("fs", "mv", filePath, folder).Run(); err != nil {
Expand Down
18 changes: 9 additions & 9 deletions base/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,18 @@ func (s *StathatBackend) Shutdown() error {
func NewStatsBackend(btype StatsBackendType, config interface{}) (StatsBackend, error) {
switch btype {
case StathatStatsBackendType:
if config, ok := config.(StathatConfig); ok {
reporter := stathat.NewBatchReporter(stathat.DefaultReporter, 200*time.Millisecond)
return &StathatBackend{config: config, reporter: reporter}, nil
} else {
cfg, ok := config.(StathatConfig)
if !ok {
return nil, errors.New("invalid stathat config")
}
reporter := stathat.NewBatchReporter(stathat.DefaultReporter, 200*time.Millisecond)
return &StathatBackend{config: cfg, reporter: reporter}, nil
case DummyStatsBackendType:
if config, ok := config.(*ChatDebugOutputConfig); ok {
return NewDummyStatsBackend(config), nil
} else {
cfg, ok := config.(*ChatDebugOutputConfig)
if !ok {
return nil, errors.New("invalid DummyStatsBackend config")
}
return NewDummyStatsBackend(cfg), nil
default:
return nil, errors.New("unknown stats registry type")
}
Expand All @@ -115,11 +115,11 @@ func (r *StatsRegistry) makeFname(name string) string {

func (r *StatsRegistry) SetPrefix(prefix string) *StatsRegistry {
prefix = r.prefix + prefix + " - "
return newStatsRegistryWithPrefix(r.DebugOutput.Config(), r.backend, prefix)
return newStatsRegistryWithPrefix(r.Config(), r.backend, prefix)
}

func (r *StatsRegistry) ResetPrefix() *StatsRegistry {
return NewStatsRegistryWithBackend(r.DebugOutput.Config(), r.backend)
return NewStatsRegistryWithBackend(r.Config(), r.backend)
}

func (r *StatsRegistry) Count(name string) {
Expand Down
7 changes: 6 additions & 1 deletion elastiwatch/elastiwatch/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package elastiwatch

import (
"database/sql"
"fmt"
"time"

"github.com/keybase/managed-bots/base"
Expand Down Expand Up @@ -40,7 +41,11 @@ func (d *DB) List() (res []Deferral, err error) {
if err != nil {
return res, err
}
defer rows.Close()
defer func() {
if cerr := rows.Close(); cerr != nil {
fmt.Printf("elastiwatch: failed to close rows: %v\n", cerr)
}
}()
for rows.Next() {
var def Deferral
if err := rows.Scan(&def.ID, &def.Regex, &def.Author, &def.Ctime); err != nil {
Expand Down
3 changes: 1 addition & 2 deletions elastiwatch/elastiwatch/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ type Handler struct {

var _ base.Handler = (*Handler)(nil)

func NewHandler(kbc *kbchat.API, debugConfig *base.ChatDebugOutputConfig,
httpSrv *HTTPSrv, db *DB, logs *LogWatch) *Handler {
func NewHandler(kbc *kbchat.API, debugConfig *base.ChatDebugOutputConfig, httpSrv *HTTPSrv, db *DB, logs *LogWatch) *Handler {
return &Handler{
DebugOutput: base.NewDebugOutput("Handler", debugConfig),
kbc: kbc,
Expand Down
3 changes: 1 addition & 2 deletions elastiwatch/elastiwatch/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ type LogWatch struct {
peekCh chan struct{}
}

func NewLogWatch(cli *opensearchapi.Client, db *DB, index, email string, emailer base.Emailer,
alertConvID, emailConvID chat1.ConvIDStr, debugConfig *base.ChatDebugOutputConfig) *LogWatch {
func NewLogWatch(cli *opensearchapi.Client, db *DB, index, email string, emailer base.Emailer, alertConvID, emailConvID chat1.ConvIDStr, debugConfig *base.ChatDebugOutputConfig) *LogWatch {
return &LogWatch{
DebugOutput: base.NewDebugOutput("LogWatch", debugConfig),
cli: cli,
Expand Down
3 changes: 1 addition & 2 deletions elastiwatch/elastiwatch/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ type renderSection struct {
Chunks []chunk
}

type htmlRenderer struct {
}
type htmlRenderer struct{}

type htmlRenderRow struct {
Chunk chunk
Expand Down
Loading
Loading