diff --git a/.forgejo/workflows/testing.yml b/.forgejo/workflows/testing.yml index 395e6c99e5..64c5dbc72d 100644 --- a/.forgejo/workflows/testing.yml +++ b/.forgejo/workflows/testing.yml @@ -301,3 +301,16 @@ jobs: - uses: ./.forgejo/workflows-composite/setup-env - run: su forgejo -c 'make deps-backend deps-tools' - run: su forgejo -c 'make security-check' + semgrep: + if: vars.ROLE == 'forgejo-coding' || vars.ROLE == 'forgejo-testing' + name: semgrep/ci + runs-on: docker + container: + image: 'data.forgejo.org/oci/semgrep:latest' + steps: + - run: apk add nodejs # required for actions/checkout + - uses: https://data.forgejo.org/actions/checkout@v6 + - name: self-check semgrep rules + run: semgrep --test .semgrep/tests/ --config .semgrep/config/ + - name: semgrep ci + run: semgrep ci --config .semgrep/config/ --metrics=off diff --git a/.semgrep/config/xorm.yaml b/.semgrep/config/xorm.yaml new file mode 100644 index 0000000000..057e1d3aef --- /dev/null +++ b/.semgrep/config/xorm.yaml @@ -0,0 +1,24 @@ +rules: + - id: xorm-sync-missing-ignore-drop-indices + patterns: + - pattern-either: + - pattern: | + $X.Sync(...) + - pattern: | + $X.SyncWithOptions($OPTS, ...) + - pattern-not: | + $X.SyncWithOptions(xorm.SyncOptions{..., IgnoreDropIndices: true, ...}, ...) + - metavariable-type: + metavariable: $X + types: + - "*xorm.Engine" + - "*xorm.Session" + paths: + exclude: + - /models/gitea_migrations/**/*.go + - /models/forgejo_migrations_legacy/**/*.go + languages: + - go + message: | + xorm Sync operation may drop indices if used on an incomplete bean definition for an existing table. Use SyncWithOptions with IgnoreDropIndices: true instead. + severity: ERROR diff --git a/.semgrep/tests/xorm.go b/.semgrep/tests/xorm.go new file mode 100644 index 0000000000..00b86acee1 --- /dev/null +++ b/.semgrep/tests/xorm.go @@ -0,0 +1,45 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations + +import ( + "io/fs" + + "forgejo.org/modules/timeutil" + + "xorm.io/xorm" +) + +type ActionUser struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"INDEX UNIQUE(action_user_index) REFERENCES(user, id)"` + RepoID int64 `xorm:"INDEX UNIQUE(action_user_index) REFERENCES(repository, id)"` + + TrustedWithPullRequests bool + + LastAccess timeutil.TimeStamp `xorm:"INDEX"` +} + +func testSyncBad1(x *xorm.Engine) error { + // ruleid:xorm-sync-missing-ignore-drop-indices + return x.Sync(new(ActionUser)) +} + +func testSyncBad2(x *xorm.Engine) error { + // ruleid:xorm-sync-missing-ignore-drop-indices + _, err = x.SyncWithOptions(xorm.SyncOptions{IgnoreDropIndices: false}, bean) + return err +} + +func testSyncGood1(x *xorm.Engine) error { + // ok:xorm-sync-missing-ignore-drop-indices + _, err = x.SyncWithOptions(xorm.SyncOptions{IgnoreDropIndices: true}, bean) + return err +} + +func testSyncGood2(x *fs.File) error { + // ok:xorm-sync-missing-ignore-drop-indices + _, err = x.Sync() + return err +} diff --git a/models/forgejo_migrations/migrate.go b/models/forgejo_migrations/migrate.go index 53ab95d216..d64009db51 100644 --- a/models/forgejo_migrations/migrate.go +++ b/models/forgejo_migrations/migrate.go @@ -153,7 +153,7 @@ func Migrate(x *xorm.Engine, freshDB bool) error { // Set a new clean the default mapper to GonicMapper as that is the default for . x.SetMapper(names.GonicMapper{}) - if err := x.Sync(new(ForgejoMigration)); err != nil { + if _, err := x.SyncWithOptions(xorm.SyncOptions{IgnoreDropIndices: true}, new(ForgejoMigration)); err != nil { return fmt.Errorf("sync: %w", err) } diff --git a/models/forgejo_migrations/v14a_actions-approval-and-trust.go b/models/forgejo_migrations/v14a_actions-approval-and-trust.go index 657d512548..9f6691c4f0 100644 --- a/models/forgejo_migrations/v14a_actions-approval-and-trust.go +++ b/models/forgejo_migrations/v14a_actions-approval-and-trust.go @@ -41,7 +41,7 @@ func v14ActionsApprovalAndTrustCreateTableActionUser(x *xorm.Engine) error { LastAccess timeutil.TimeStamp `xorm:"INDEX"` } - return x.Sync(new(ActionUser)) + return x.Sync(new(ActionUser)) // nosemgrep:xorm-sync-missing-ignore-drop-indices } func v14ActionsApprovalAndTrustAddActionsRunFields(x *xorm.Engine) error { diff --git a/models/forgejo_migrations/v14a_add-forgejo-migrations-table.go b/models/forgejo_migrations/v14a_add-forgejo-migrations-table.go index f78eec8789..8f22983e80 100644 --- a/models/forgejo_migrations/v14a_add-forgejo-migrations-table.go +++ b/models/forgejo_migrations/v14a_add-forgejo-migrations-table.go @@ -21,5 +21,5 @@ func addForgejoMigration(x *xorm.Engine) error { ID string `xorm:"pk"` CreatedUnix timeutil.TimeStamp `xorm:"created"` } - return x.Sync(new(ForgejoMigration)) + return x.Sync(new(ForgejoMigration)) // nosemgrep:xorm-sync-missing-ignore-drop-indices } diff --git a/models/forgejo_migrations/v14b_action-reindexing.go b/models/forgejo_migrations/v14b_action-reindexing.go index 6b5608a5d5..83dc452191 100644 --- a/models/forgejo_migrations/v14b_action-reindexing.go +++ b/models/forgejo_migrations/v14b_action-reindexing.go @@ -69,5 +69,5 @@ func reworkActionIndexes(x *xorm.Engine) error { return err } - return x.Sync(new(v14bAction)) + return x.Sync(new(v14bAction)) // nosemgrep:xorm-sync-missing-ignore-drop-indices }