ci: detect and prevent empty case statements in Go code (#11593)

One of the security patches released 2026-03-09 [fixed a vulnerability](d1c7b04d09) caused by a misapplication of Go `case` statements, where the implementation would have been correct if Go `case` statements automatically fall through to the next case block, but they do not.  This PR adds a semgrep rule which detects any empty `case` statement and raises an error, in order to prevent this coding mistake in the future.

For example, code like this will now trigger a build error:
```go
	switch setting.Protocol {
	case setting.HTTPUnix:
	case setting.FCGI:
	case setting.FCGIUnix:
	default:
		defaultLocalURL := string(setting.Protocol) + "://"
	}
```

Example error:
```
    cmd/web.go
   ❯❯❱ semgrep.config.forgejo-switch-empty-case
          switch has a case block with no content. This is treated as "break" by Go, but developers may
          confuse it for "fallthrough".  To fix this error, disambiguate by using "break" or
          "fallthrough".

          279┆ switch setting.Protocol {
          280┆ case setting.HTTPUnix:
          281┆ case setting.FCGI:
          282┆ case setting.FCGIUnix:
          283┆ default:
          284┆   defaultLocalURL := string(setting.Protocol) + "://"
          285┆   if setting.HTTPAddr == "0.0.0.0" {
          286┆           defaultLocalURL += "localhost"
          287┆   } else {
          288┆           defaultLocalURL += setting.HTTPAddr
```

As described in the error output, this error can be fixed by explicitly listing `break` (the real Go behaviour, to do nothing in the block), or by listing `fallthrough` (if the intent was to fall through).

All existing code triggering this detection has been changed to `break` (or, rarely, irrelevant cases have been removed), which should maintain the same code functionality.  While performing this fixup, a light analysis was performed on each case and they *appeared* correct, but with ~65 cases I haven't gone into extreme depth.

Tests are present for the semgrep rule in `.semgrep/tests/go.go`.

## Checklist

The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).

### Documentation

- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11593
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
This commit is contained in:
Mathieu Fenniak 2026-03-10 02:50:28 +01:00 committed by Mathieu Fenniak
parent c564867439
commit f93d2cb261
45 changed files with 135 additions and 22 deletions

18
.semgrep/config/go.yaml Normal file
View file

@ -0,0 +1,18 @@
rules:
- id: forgejo-switch-empty-case
pattern-either:
- pattern: |-
switch $_ {
case $_:
}
- patterns:
- pattern: |-
switch {
case $_:
}
languages:
- go
severity: ERROR
message: >
switch has a case block with no content. This is treated as "break" by Go, but developers may confuse it for
"fallthrough". To fix this error, disambiguate by using "break" or "fallthrough".

44
.semgrep/tests/go.go Normal file
View file

@ -0,0 +1,44 @@
// Copyright 2014 The Gogs Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package cmd
import (
"context"
"os"
"os/signal"
"strings"
"syscall"
_ "net/http/pprof" // Used for debugging if enabled and a web server is running
"forgejo.org/modules/setting"
)
func setPortEmptyCaseBad(port string) error {
setting.AppURL = strings.Replace(setting.AppURL, setting.HTTPPort, port, 1)
setting.HTTPPort = port
// ruleid:forgejo-switch-empty-case
switch setting.Protocol {
case setting.HTTPUnix:
case setting.FCGI:
case setting.FCGIUnix:
default:
defaultLocalURL := string(setting.Protocol) + "://"
}
// ok:forgejo-switch-empty-case
switch setting.Protocol {
case setting.HTTPUnix:
break
case setting.FCGI:
break
case setting.FCGIUnix:
break
default:
defaultLocalURL := string(setting.Protocol) + "://"
}
return nil
}

View file

@ -126,7 +126,9 @@ func installSignals(ctx context.Context) (context.Context, context.CancelFunc) {
)
select {
case <-signalChannel:
break
case <-ctx.Done():
break
}
cancel()
signal.Reset()

View file

@ -278,8 +278,11 @@ func setPort(port string) error {
switch setting.Protocol {
case setting.HTTPUnix:
break
case setting.FCGI:
break
case setting.FCGIUnix:
break
default:
defaultLocalURL := string(setting.Protocol) + "://"
if setting.HTTPAddr == "0.0.0.0" {

View file

@ -25,6 +25,7 @@ func SetDefaultPasswordToArgon2(x *xorm.Engine) error {
return err
case setting.Database.Type.IsSQLite3():
// drop through
break
default:
log.Fatal("Unrecognized DB")
}

View file

@ -174,8 +174,10 @@ func GetPackageDescriptor(ctx context.Context, pv *PackageVersion) (*PackageDesc
metadata = &debian.Metadata{}
case TypeGeneric:
// generic packages have no metadata
break
case TypeGo:
// go packages have no metadata
break
case TypeHelm:
metadata = &helm.Metadata{}
case TypeNuGet:

View file

@ -39,7 +39,9 @@ func (m *Manager) Register(uid int64) <-chan *Event {
}
select {
case m.connection <- struct{}{}:
break
default:
break
}
m.mutex.Unlock()
return messenger.Register()

View file

@ -47,7 +47,9 @@ loop:
// empty the connection channel
select {
case <-m.connection:
break
default:
break
}
}
m.mutex.Unlock()
@ -63,7 +65,9 @@ loop:
// We won't change the "then" time because there could be concurrency issues
select {
case <-timer.C:
break
default:
break
}
continue
}

View file

@ -62,7 +62,9 @@ func (m *Messenger) SendMessage(message *Event) {
channel := m.channels[i]
select {
case channel <- message:
break
default:
break
}
}
}

View file

@ -221,6 +221,7 @@ func CutDiffAroundLine(originalDiff io.Reader, line int64, old bool, numbersOfLi
}
case '\\':
// FIXME: handle `\ No newline at end of file`
break
default:
currentLine++
otherLine++

View file

@ -65,12 +65,8 @@ func (o *gitPushOptions) Parse(data string) bool {
value = "true"
}
switch Key(key) {
case RepoPrivate:
case RepoTemplate:
case AgitTopic:
case AgitForcePush:
case AgitTitle:
case AgitDescription:
case RepoPrivate, RepoTemplate, AgitTopic, AgitForcePush, AgitTitle, AgitDescription:
break
default:
return false
}

View file

@ -98,6 +98,7 @@ func (repo *Repository) GetCodeActivityStats(fromTime time.Time, branch string)
}
switch p {
case 1: // Separator
break
case 2: // Commit sha-1
stats.CommitCount++
case 3: // Author

View file

@ -164,6 +164,7 @@ func (g *Manager) doHammerTime(d time.Duration) {
g.lock.Lock()
select {
case <-g.hammerCtx.Done():
break
default:
log.Warn("Setting Hammer condition")
g.hammerCtxCancel()
@ -180,6 +181,7 @@ func (g *Manager) doTerminate() {
g.lock.Lock()
select {
case <-g.terminateCtx.Done():
break
default:
log.Warn("Terminating")
g.terminateCtxCancel()

View file

@ -86,6 +86,7 @@ func (g *Manager) DoGracefulShutdown() {
g.lock.Lock()
select {
case <-g.shutdownRequested:
break
default:
close(g.shutdownRequested)
}

View file

@ -215,7 +215,9 @@ func Init() {
}
select {
case waitChannel <- time.Since(start):
break
case <-graceful.GetManager().IsShutdown():
break
}
close(waitChannel)

View file

@ -141,7 +141,9 @@ func InitIssueIndexer(syncReindex bool) {
if syncReindex {
select {
case <-indexerInitWaitChannel:
break
case <-graceful.GetManager().IsShutdown():
break
}
} else if setting.Indexer.StartupTimeout > 0 {
go func() {

View file

@ -80,7 +80,9 @@ func (b *EventWriterBaseImpl) Run(ctx context.Context) {
if pause := b.GetPauseChan(); pause != nil {
select {
case <-pause:
break
case <-ctx.Done():
break
}
}
}
@ -178,6 +180,7 @@ func eventWriterStopWait(w EventWriter) {
close(w.Base().Queue)
select {
case <-w.Base().stopped:
break
case <-time.After(2 * time.Second):
FallbackErrorf("unable to stop log writer %q in time, skip", w.GetWriterName())
}

View file

@ -58,6 +58,7 @@ func (l *LoggerImpl) SendLogEvent(event *Event) {
}
select {
case w.Base().Queue <- formatted:
break
default:
bs, _ := json.Marshal(event)
FallbackErrorf("log writer %q queue is full, event: %v", w.GetWriterName(), string(bs))

View file

@ -742,6 +742,7 @@ func shortLinkProcessor(ctx *RenderContext, node *html.Node) {
// fast path: empty string, ignore
case "":
// leave image as false
break
case ".jpg", ".jpeg", ".png", ".tif", ".tiff", ".webp", ".gif", ".bmp", ".ico", ".svg":
image = true
}

View file

@ -44,6 +44,7 @@ func (d *RetryDownloader) retry(work func() error) error {
case <-d.ctx.Done():
return d.ctx.Err()
case <-time.After(time.Second * time.Duration(d.RetryDelay)):
break
}
}
return err

View file

@ -259,9 +259,10 @@ func (p *Conn) readV2ProxyHeader() error {
p.localAddr = p.conn.RemoteAddr()
return nil
case 0x1:
// - \x1 : PROXY : the connection was established on behalf of another node,
// and reflects the original connection endpoints. The receiver must then use
// the information provided in the protocol block to get original the address.
// - \x1 : PROXY : the connection was established on behalf of another node,
// and reflects the original connection endpoints. The receiver must then use
// the information provided in the protocol block to get original the address.
break
default:
// - other values are unassigned and must not be emitted by senders. Receivers
// must drop connections presenting unexpected values here.
@ -466,7 +467,9 @@ func (p *Conn) readV1ProxyHeader() error {
p.localAddr = p.conn.RemoteAddr()
return nil
case "TCP4":
break
case "TCP6":
break
default:
p.conn.Close()
return &ErrBadAddressType{parts[1]}

View file

@ -56,6 +56,7 @@ func (q *WorkerPoolQueue[T]) doDispatchBatchToWorker(wg *workerGroup[T], flushCh
full := false
select {
case q.batchChan <- batch:
break
default:
full = true
}
@ -76,6 +77,7 @@ func (q *WorkerPoolQueue[T]) doDispatchBatchToWorker(wg *workerGroup[T], flushCh
if full {
select {
case q.batchChan <- batch:
break
case flush := <-flushChan:
q.doWorkerHandle(batch)
q.doFlush(wg, flush)
@ -105,7 +107,9 @@ func (q *WorkerPoolQueue[T]) doWorkerHandle(batch []T) {
log.Error("Queue %q failed to handle batch of %d items, backoff for a few seconds", q.GetName(), len(batch))
select {
case <-q.ctxRun.Done():
break
case <-time.After(time.Duration(unhandledItemRequeueDuration.Load())):
break
}
}
for _, item := range unhandled {
@ -170,7 +174,9 @@ func (q *WorkerPoolQueue[T]) doStartNewWorker(wp *workerGroup[T]) {
t.Reset(workerIdleDuration)
select {
case <-t.C:
break
default:
break
}
case <-t.C:
q.workerNumMu.Lock()
@ -296,6 +302,7 @@ func (q *WorkerPoolQueue[T]) doRun() {
go func() { wg.wg.Wait(); close(workerDone) }()
select {
case <-workerDone:
break
case <-time.After(shutdownTimeout):
log.Error("Queue %q is shutting down, but workers are still running after timeout", q.GetName())
}

View file

@ -110,6 +110,7 @@ func (q *WorkerPoolQueue[T]) FlushWithContext(ctx context.Context, timeout time.
// if it blocks, it means that there is a flush in progress or the queue hasn't been started yet
select {
case q.flushChan <- c:
break
case <-ctx.Done():
return ctx.Err()
case <-q.ctxRun.Done():

View file

@ -53,6 +53,7 @@ func loadCacheFrom(rootCfg ConfigProvider) {
CacheService.Adapter = sec.Key("ADAPTER").In("memory", []string{"memory", "redis", "memcache", "twoqueue"})
switch CacheService.Adapter {
case "memory":
break
case "redis", "memcache":
CacheService.Conn = strings.Trim(sec.Key("HOST").String(), "\" ")
case "twoqueue":

View file

@ -82,6 +82,7 @@ func checkReplyToAddress() error {
case 0:
return fmt.Errorf("%s must appear in the user part of the address (before the @)", IncomingEmail.TokenPlaceholder)
case 1:
break
default:
return fmt.Errorf("%s must appear only once", IncomingEmail.TokenPlaceholder)
}

View file

@ -235,6 +235,7 @@ func loadMailerFrom(rootCfg ConfigProvider) {
}
}
case "dummy": // just mention and do nothing
break
}
if MailService.From != "" {

View file

@ -229,6 +229,7 @@ func HTMLFormat(s string, rawArgs ...any) template.HTML {
switch v := v.(type) {
case nil, bool, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, float32, float64, template.HTML:
// for most basic types (including template.HTML which is safe), just do nothing and use it
break
case string:
args[i] = template.HTMLEscapeString(v)
case fmt.Stringer:

View file

@ -75,6 +75,7 @@ func anyToTime(any any) (t time.Time, isZero bool) {
switch v := any.(type) {
case nil:
// it is zero
break
case *time.Time:
if v != nil {
t = *v

View file

@ -145,6 +145,7 @@ func (l *locale) LookupPluralByForm(trKey string, pluralForm PluralFormIndex) st
suffix = PluralFormSeparator + "many"
case PluralFormOther:
// No suffix for the "other" string.
break
default:
log.Error("Invalid plural form index %d", pluralForm)
return ""
@ -248,6 +249,7 @@ func PrepareArgsForHTML(trArgs ...any) []any {
switch v := v.(type) {
case nil, bool, int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, float32, float64, template.HTML:
// for most basic types (including template.HTML which is safe), just do nothing and use it
break
case string:
args[i] = template.HTMLEscapeString(v)
case fmt.Stringer:

View file

@ -56,6 +56,7 @@ func iterateMessagesNextInner(onMsgid func(string, string, string) error, data m
pluralSuffix = key
case "other":
// do nothing
break
default:
realKey = fullKey
}
@ -71,6 +72,7 @@ func iterateMessagesNextInner(onMsgid func(string, string, string) error, data m
case nil:
// do nothing
break
default:
return fmt.Errorf("Unexpected JSON type: %s - %T", fullKey, value)

View file

@ -141,6 +141,7 @@ func getSignVersion(req *http.Request) (string, error) {
switch m[1] {
case "1.0", "1.1", "1.2", "1.3":
break
default:
return "", util.NewInvalidArgumentErrorf("unsupported version")
}

View file

@ -176,6 +176,7 @@ func Search(ctx *context.APIContext) {
opts.Mirror = optional.Some(false)
opts.Collaborate = optional.Some(true)
case "":
break
default:
ctx.Error(http.StatusUnprocessableEntity, "", fmt.Errorf("Invalid search mode: \"%s\"", mode))
return

View file

@ -54,7 +54,9 @@ func MonitorDiagnosis(ctx *context.Context) {
select {
case <-time.After(time.Duration(seconds) * time.Second):
break
case <-ctx.Done():
break
}
pprof.StopCPUProfile()
trace.Stop()

View file

@ -34,7 +34,9 @@ func initTestQueueOnce() {
for range t {
select {
case <-graceful.GetManager().ShutdownContext().Done():
break
case <-time.After(5 * time.Second):
break
}
}
return nil
@ -56,6 +58,7 @@ func initTestQueueOnce() {
for {
select {
case <-ctx.Done():
break
case <-time.After(500 * time.Millisecond):
if adding {
if testQueue.GetQueueItemNumber() == qs.Length {

View file

@ -1418,6 +1418,7 @@ func generateCodeChallenge(ctx *context.Context, provider string) (codeChallenge
case *openidConnect.Provider, *fitbit.Provider, *zoom.Provider:
// those providers forward the `code_verifier`
// a code_challenge can be generated
break
}
codeVerifier := util.CryptoRandomString(util.RandomStringHigh)

View file

@ -634,6 +634,7 @@ func SearchRepo(ctx *context.Context) {
opts.Mirror = optional.Some(false)
opts.Collaborate = optional.Some(true)
case "":
break
default:
ctx.Error(http.StatusUnprocessableEntity, fmt.Sprintf("Invalid search mode: \"%s\"", mode))
return

View file

@ -511,8 +511,6 @@ func buildIssueOverview(ctx *context.Context, unitType unit.Type) {
}
switch filterMode {
case issues_model.FilterModeAll:
case issues_model.FilterModeYourRepositories:
case issues_model.FilterModeAssign:
opts.AssigneeID = ctx.Doer.ID
case issues_model.FilterModeCreate:
@ -846,7 +844,7 @@ func getUserIssueStats(ctx *context.Context, ctxUser *user_model.User, filterMod
openClosedOpts := opts.Copy()
switch filterMode {
case issues_model.FilterModeAll:
// no-op
break
case issues_model.FilterModeYourRepositories:
openClosedOpts.AllPublic = false
case issues_model.FilterModeAssign:

View file

@ -180,8 +180,6 @@ func ViewPackageVersion(ctx *context.Context) {
ctx.Data["PackageRegistryHost"] = setting.Packages.RegistryHost
switch pd.Package.Type {
case packages_model.TypeContainer:
case packages_model.TypeAlpine:
branches := make(container.Set[string])
repositories := make(container.Set[string])

View file

@ -85,6 +85,7 @@ func checkJobsOfRun(ctx context.Context, runID int64, recursionCount int) error
case behaviourExecuteJob:
// Intentional blank case -- proceed with updating the status of the job to waiting.
break
case behaviourIgnoreJob:
// Skip updating this job's status to waiting, continue with other jobs in the run.

View file

@ -164,13 +164,6 @@ func ActionToForgeUserActivity(ctx context.Context, action *activities_model.Act
renderIssue(action.Comment.Issue),
renderedComment,
)
case activities_model.ActionMirrorSyncPush:
case activities_model.ActionMirrorSyncCreate:
case activities_model.ActionMirrorSyncDelete:
case activities_model.ActionPublishRelease:
case activities_model.ActionPullReviewDismissed:
case activities_model.ActionPullRequestReadyForReview:
case activities_model.ActionAutoMergePullRequest:
}
return makeUserActivity("performed an unrecognised action: %s", action.OpType.String())

View file

@ -69,6 +69,7 @@ func Init(ctx context.Context) error {
case <-ctx.Done():
return
case <-time.NewTimer(10 * time.Second).C:
break
}
}
}
@ -123,6 +124,7 @@ func processIncomingEmails(ctx context.Context) error {
case <-ctx.Done():
return nil
case <-time.NewTimer(time.Second).C:
break
}
}
}

View file

@ -182,6 +182,7 @@ func (g *GithubDownloaderV3) waitAndPickClient() {
timer.Stop()
return
case <-timer.C:
break
}
err := g.RefreshRate()

View file

@ -170,6 +170,7 @@ func unmergedFiles(ctx context.Context, tmpBasePath string, unmerged chan *unmer
switch line.stage {
case 0:
// Should not happen as this represents successfully merged file - we will tolerate and ignore though
break
case 1:
if next.stage1 != nil || next.stage2 != nil || next.stage3 != nil {
// We need to handle the unstaged file stage1,stage2,stage3

View file

@ -184,7 +184,7 @@ func (parser *Parser) ParseGlyphs(glyphs []byte) {
case '-':
parser.setLeftFlow(i)
case ' ':
// no-op
break
default:
parser.newFlow(i)
}

View file

@ -108,6 +108,7 @@ func runMigrateTask(ctx context.Context, t *admin_model.Task) (err error) {
for {
select {
case <-time.After(2 * time.Second):
break
case <-ctx.Done():
return
}