Skip to content
Open
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
2 changes: 0 additions & 2 deletions db_lib/CmdGitClient.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,6 @@ func (c CmdGitClient) GetLastCommitMessage(r GitRepository) (msg string, err err
return
}

msg = truncateCommitMessage(msg)

return
}

Expand Down
5 changes: 3 additions & 2 deletions db_lib/GoGitClient.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/go-git/go-git/v5/plumbing/transport/ssh"
"github.com/go-git/go-git/v5/storage/memory"
"github.com/semaphoreui/semaphore/db"
"github.com/semaphoreui/semaphore/pkg/conv"
"github.com/semaphoreui/semaphore/pkg/task_logger"
"github.com/semaphoreui/semaphore/util"

Expand Down Expand Up @@ -230,9 +231,9 @@ func (c GoGitClient) GetLastCommitMessage(r GitRepository) (msg string, err erro
return
}

msg = truncateCommitMessage(headCommit.Message)
msg = headCommit.Message

r.Logger.Log("Message: " + msg)
r.Logger.Log("Message: " + conv.TruncateValidUTF8(msg))

return
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand Down
13 changes: 0 additions & 13 deletions db_lib/commit_message.go

This file was deleted.

52 changes: 0 additions & 52 deletions db_lib/commit_message_test.go

This file was deleted.

50 changes: 50 additions & 0 deletions pkg/conv/conv.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,58 @@ package conv
import (
"reflect"
"strings"
"unicode/utf8"
)


const commitMessageMaxRunes = 100

// TruncateValidUTF8 sanitizes s so it can be stored in the task.commit_message
// column (VARCHAR(100)): NUL bytes are removed, invalid UTF-8 byte sequences are
// replaced with U+FFFD, and the result is capped at commitMessageMaxRunes runes.
//
// This is not a general-purpose string helper: the hard cap is tied to the
// commit-message column width, so reusing it elsewhere would truncate at 100
// runes unexpectedly.
//
// It guards two failure modes that both make PostgreSQL reject the value with
// `invalid byte sequence for encoding "UTF8"` and abort the transaction:
// - Byte slicing can split a multi-byte character; slicing by runes cannot.
// - Text originating from a non-UTF-8 encoding (e.g. latin-1) can already
// contain invalid bytes even when short, so it is sanitized regardless of
// length.
func TruncateValidUTF8(s string) string {
// Decode at most the first commitMessageMaxRunes runes we intend to keep.
// Converting the whole (possibly very large) message to []rune just to
// discard all but the leading runes wastes CPU and allocations, since
// callers no longer truncate before reaching here.
var b strings.Builder
// Cap the initial allocation: the kept runes never exceed
// commitMessageMaxRunes*utf8.UTFMax bytes, and shorter inputs need even less.
grow := commitMessageMaxRunes * utf8.UTFMax
if len(s) < grow {
grow = len(s)
}
b.Grow(grow)

count := 0
// Ranging over a string decodes runes and yields U+FFFD for invalid UTF-8
// bytes (matching a []rune conversion), so the result is always storable as
// UTF-8 without splitting a multi-byte character.
for _, r := range s {
// PostgreSQL text/varchar cannot store NUL, even though it is valid UTF-8.
if r == 0 {
continue
}
b.WriteRune(r)
count++
if count == commitMessageMaxRunes {
break
}
}
return b.String()
}

func ConvertFloatToIntIfPossible(v any) (int64, bool) {

switch v := v.(type) {
Expand Down
79 changes: 79 additions & 0 deletions pkg/conv/conv_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package conv

import (
"strings"
"testing"
"unicode/utf8"

"github.com/stretchr/testify/assert"
)

func TestTruncateValidUTF8_KeepsValidInput(t *testing.T) {
tests := []struct {
name string
in string
}{
{"empty", ""},
{"short ASCII", "fix: short"},
{"exactly max ASCII", strings.Repeat("a", 100)},
{"exactly max Cyrillic", strings.Repeat("ы", 100)},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
out := TruncateValidUTF8(tt.in)
assert.Equal(t, tt.in, out)
assert.True(t, utf8.ValidString(out))
})
}
}

func TestTruncateValidUTF8_CapsByRunes(t *testing.T) {
tests := []struct {
name string
in string
want string
}{
{"ASCII over limit", strings.Repeat("a", 101), strings.Repeat("a", 100)},
{"Cyrillic over limit", strings.Repeat("ы", 150), strings.Repeat("ы", 100)},
{"emoji over limit", strings.Repeat("🙂", 150), strings.Repeat("🙂", 100)},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
out := TruncateValidUTF8(tt.in)
assert.Equal(t, tt.want, out)
assert.Equal(t, 100, utf8.RuneCountInString(out))
assert.True(t, utf8.ValidString(out))
})
}
}

// A short message authored in a non-UTF-8 encoding (latin-1 'é' == 0xE9) must be
// sanitized even though it is well under the length limit — this is the case a
// pure rune-truncation misses.
func TestTruncateValidUTF8_SanitizesShortInvalid(t *testing.T) {
in := "caf\xe9" // 0xE9 is a lone byte, invalid UTF-8

assert.False(t, utf8.ValidString(in))

out := TruncateValidUTF8(in)
assert.True(t, utf8.ValidString(out))
assert.Equal(t, "caf�", out)
}

func TestTruncateValidUTF8_StripsNUL(t *testing.T) {
out := TruncateValidUTF8("fix\x00patch")

assert.NotContains(t, out, "\x00")
assert.Equal(t, "fixpatch", out)
assert.True(t, utf8.ValidString(out))
}

func TestTruncateValidUTF8_SanitizesThenCaps(t *testing.T) {
// Invalid byte inside a message longer than the limit: result must be both
// valid UTF-8 and within the rune cap.
in := strings.Repeat("a", 60) + "\xe9" + strings.Repeat("b", 60)

out := TruncateValidUTF8(in)
assert.True(t, utf8.ValidString(out))
assert.Equal(t, 100, utf8.RuneCountInString(out))
}
8 changes: 7 additions & 1 deletion services/tasks/TaskRunner_logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/semaphoreui/semaphore/pkg/tz"

"github.com/semaphoreui/semaphore/api/sockets"
"github.com/semaphoreui/semaphore/pkg/conv"
"github.com/semaphoreui/semaphore/pkg/task_logger"
"github.com/semaphoreui/semaphore/util"
log "github.com/sirupsen/logrus"
Expand Down Expand Up @@ -72,7 +73,12 @@ func (t *TaskRunner) WaitLog() {
func (t *TaskRunner) SetCommit(hash, message string) {

t.Task.CommitHash = &hash
t.Task.CommitMessage = message
// Sanitize before persisting to the DB. This is the persistence point for
// remote-runner commit reports; local tasks reach it too via the task logger
// (LocalExecutor.SetCommit -> Logger.SetCommit), and that local mirror
// additionally sanitizes the copy it exposes as task extra vars.
// See conv.TruncateValidUTF8.
t.Task.CommitMessage = conv.TruncateValidUTF8(message)

if err := t.pool.store.UpdateTask(t.Task); err != nil {
t.panicOnError(err, "Failed to update task commit")
Expand Down
7 changes: 7 additions & 0 deletions services/tasks/local_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/semaphoreui/semaphore/db"
"github.com/semaphoreui/semaphore/db_lib"
"github.com/semaphoreui/semaphore/pkg/conv"
"github.com/semaphoreui/semaphore/pkg/task_logger"
"github.com/semaphoreui/semaphore/util"
)
Expand Down Expand Up @@ -105,6 +106,12 @@ func (t *LocalExecutor) SetLogger(logger task_logger.Logger) {

func (t *LocalExecutor) SetCommit(hash, message string) {
// TODO: is this the correct place to do?
// Sanitize the commit message: it comes straight from the external git repo
// and may contain NUL/invalid UTF-8 bytes or be arbitrarily large. Without
// this the raw value reaches t.Task.CommitMessage and taskDetails
// ("commit_message"), i.e. task extra vars and API responses. Mirrors
// TaskRunner.SetCommit. See conv.TruncateValidUTF8.
message = conv.TruncateValidUTF8(message)
t.Task.CommitHash = &hash
t.Task.CommitMessage = message
t.Logger.SetCommit(hash, message)
Expand Down
Loading