diff --git a/models/actions/main_test.go b/models/actions/main_test.go index 5d5089e3bb..3cfb395e62 100644 --- a/models/actions/main_test.go +++ b/models/actions/main_test.go @@ -12,6 +12,7 @@ import ( func TestMain(m *testing.M) { unittest.MainTest(m, &unittest.TestOptions{ FixtureFiles: []string{ + "action_runner.yml", "action_runner_token.yml", }, }) diff --git a/models/actions/runner.go b/models/actions/runner.go index 9192925d5a..cfe936c495 100644 --- a/models/actions/runner.go +++ b/models/actions/runner.go @@ -5,6 +5,7 @@ package actions import ( "context" + "encoding/binary" "fmt" "strings" "time" @@ -253,11 +254,26 @@ func UpdateRunner(ctx context.Context, r *ActionRunner, cols ...string) error { // DeleteRunner deletes a runner by given ID. func DeleteRunner(ctx context.Context, id int64) error { - if _, err := GetRunnerByID(ctx, id); err != nil { + runner, err := GetRunnerByID(ctx, id) + if err != nil { return err } - _, err := db.DeleteByID[ActionRunner](ctx, id) + // Replace the UUID, which was either based on the secret's first 16 bytes or an UUIDv4, + // with a sequence of 8 0xff bytes followed by the little-endian version of the record's + // identifier. This will prevent the deleted record's identifier from colliding with any + // new record. + b := make([]byte, 8) + binary.LittleEndian.PutUint64(b, uint64(id)) + runner.UUID = fmt.Sprintf("ffffffff-ffff-ffff-%.2x%.2x-%.2x%.2x%.2x%.2x%.2x%.2x", + b[0], b[1], b[2], b[3], b[4], b[5], b[6], b[7]) + + err = UpdateRunner(ctx, runner, "UUID") + if err != nil { + return err + } + + _, err = db.DeleteByID[ActionRunner](ctx, id) return err } diff --git a/models/actions/runner_test.go b/models/actions/runner_test.go new file mode 100644 index 0000000000..a71f5f0044 --- /dev/null +++ b/models/actions/runner_test.go @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: MIT + +package actions + +import ( + "encoding/binary" + "fmt" + "testing" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" +) + +func TestDeleteRunner(t *testing.T) { + const recordID = 12345678 + assert.NoError(t, unittest.PrepareTestDatabase()) + before := unittest.AssertExistsAndLoadBean(t, &ActionRunner{ID: recordID}) + + err := DeleteRunner(db.DefaultContext, recordID) + assert.NoError(t, err) + + var after ActionRunner + found, err := db.GetEngine(db.DefaultContext).ID(recordID).Unscoped().Get(&after) + assert.NoError(t, err) + assert.True(t, found) + + // Most fields (namely Name, Version, OwnerID, RepoID, Description, Base, RepoRange, + // TokenHash, TokenSalt, LastOnline, LastActive, AgentLabels and Created) are unaffected + assert.Equal(t, before.Name, after.Name) + assert.Equal(t, before.Version, after.Version) + assert.Equal(t, before.OwnerID, after.OwnerID) + assert.Equal(t, before.RepoID, after.RepoID) + assert.Equal(t, before.Description, after.Description) + assert.Equal(t, before.Base, after.Base) + assert.Equal(t, before.RepoRange, after.RepoRange) + assert.Equal(t, before.TokenHash, after.TokenHash) + assert.Equal(t, before.TokenSalt, after.TokenSalt) + assert.Equal(t, before.LastOnline, after.LastOnline) + assert.Equal(t, before.LastActive, after.LastActive) + assert.Equal(t, before.AgentLabels, after.AgentLabels) + assert.Equal(t, before.Created, after.Created) + + // Deleted contains a value + assert.NotNil(t, after.Deleted) + + // UUID was modified + assert.NotEqual(t, before.UUID, after.UUID) + // UUID starts with ffffffff-ffff-ffff- + assert.Equal(t, "ffffffff-ffff-ffff-", after.UUID[:19]) + // UUID ends with LE binary representation of record ID + idAsBinary := make([]byte, 8) + binary.LittleEndian.PutUint64(idAsBinary, uint64(recordID)) + idAsHexadecimal := fmt.Sprintf("%.2x%.2x-%.2x%.2x%.2x%.2x%.2x%.2x", idAsBinary[0], + idAsBinary[1], idAsBinary[2], idAsBinary[3], idAsBinary[4], idAsBinary[5], + idAsBinary[6], idAsBinary[7]) + assert.Equal(t, idAsHexadecimal, after.UUID[19:]) +} diff --git a/models/fixtures/action_runner.yml b/models/fixtures/action_runner.yml new file mode 100644 index 0000000000..d2615f08eb --- /dev/null +++ b/models/fixtures/action_runner.yml @@ -0,0 +1,20 @@ +- + # A global runner + # Secret is 7e577e577e577e57feedfacefeedfacefeedface + id: 12345678 + uuid: "37653537-3765-3537-3765-353737653537" + name: "test" + version: "" + owner_id: 0 + repo_id: 0 + description: "" + base: 0 + repo_range: "" + token_hash: "3af8a56b850dba8848044385fedcfa4d9432e17de9f9803e4d279991394ac2945066ceb9a5e7cbe60a087d90d4bad03a8f9b" + token_salt: "832f8529db6151a1c3c605dd7570b58f" + last_online: 0 + last_active: 0 + agent_labels: '[""]' + created: 1716104432 + updated: 1716104432 + deleted: ~ diff --git a/release-notes/8.0.0/3830.md b/release-notes/8.0.0/3830.md new file mode 100644 index 0000000000..5e46a45ec9 --- /dev/null +++ b/release-notes/8.0.0/3830.md @@ -0,0 +1 @@ +Neutralize delete runners' UUID to prevent collisions with new records