Script 'mail_helper' called by obssrc Hello community, here is the log from the commit of package semaphore for openSUSE:Factory checked in at 2026-07-06 12:34:33 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/semaphore (Old) and /work/SRC/openSUSE:Factory/.semaphore.new.1982 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "semaphore" Mon Jul 6 12:34:33 2026 rev:50 rq:1363945 version:2.18.20 Changes: -------- --- /work/SRC/openSUSE:Factory/semaphore/semaphore.changes 2026-06-28 21:11:14.591612335 +0200 +++ /work/SRC/openSUSE:Factory/.semaphore.new.1982/semaphore.changes 2026-07-06 12:37:15.389672924 +0200 @@ -1,0 +2,36 @@ +Mon Jul 06 05:19:50 UTC 2026 - Johannes Kastl <[email protected]> + +- Update to version 2.18.20: + * Bugfixes + - Validate playbook path +- Update to version 2.18.19 (.17 and .18 were not released): + This patch release includes several important security and + validation fixes. + * Security fixes + - Added validation for Git repository URLs to prevent Git + option injection. + - Added --end-of-options to Git commands to make repository URL + and branch handling safer. + - Fixed branch override handling: task-level Git branch + override is now applied only when the template explicitly + allows it. + - Prevented custom roles from shadowing built-in role slugs + such as owner or manager. + - Fixed permission resolution so built-in roles always use + their built-in permissions instead of database-defined custom + roles. + - Added validation to prevent access keys from being updated + with a different ID or moved to another project. + * Reliability and tests + - Added tests for Git URL validation and Git command injection + protection. + - Added tests for access key update validation. + - Added tests for custom role validation and reserved role + slugs. + - Refactored Git branch resolution into a dedicated helper to + make task behavior more consistent. +- Update to version 2.18.16 (.15 was not released): + * Bugfixes + - Fix variable groups sync functionality + +------------------------------------------------------------------- Old: ---- semaphore-2.18.13.obscpio web-2.18.13.tar.gz New: ---- semaphore-2.18.20.obscpio web-2.18.20.tar.gz ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ semaphore.spec ++++++ --- /var/tmp/diff_new_pack.zr1Bme/_old 2026-07-06 12:37:18.553782867 +0200 +++ /var/tmp/diff_new_pack.zr1Bme/_new 2026-07-06 12:37:18.565783284 +0200 @@ -17,7 +17,7 @@ Name: semaphore -Version: 2.18.13 +Version: 2.18.20 Release: 0 Summary: Modern UI for Ansible License: MIT ++++++ _service ++++++ --- /var/tmp/diff_new_pack.zr1Bme/_old 2026-07-06 12:37:19.113802326 +0200 +++ /var/tmp/diff_new_pack.zr1Bme/_new 2026-07-06 12:37:19.153803716 +0200 @@ -1,9 +1,9 @@ <services> <service name="obs_scm" mode="manual"> - <param name="url">https://github.com/ansible-semaphore/semaphore</param> + <param name="url">https://github.com/ansible-semaphore/semaphore.git</param> <param name="scm">git</param> <param name="exclude">.git</param> - <param name="revision">v2.18.13</param> + <param name="revision">refs/tags/v2.18.20</param> <param name="versionformat">@PARENT_TAG@</param> <param name="versionrewrite-pattern">v(.*)</param> <param name="changesgenerate">enable</param> ++++++ _servicedata ++++++ --- /var/tmp/diff_new_pack.zr1Bme/_old 2026-07-06 12:37:19.453814140 +0200 +++ /var/tmp/diff_new_pack.zr1Bme/_new 2026-07-06 12:37:19.489815391 +0200 @@ -1,6 +1,8 @@ <servicedata> <service name="tar_scm"> <param name="url">https://github.com/ansible-semaphore/semaphore</param> - <param name="changesrevision">8a4dcf0868af718aaa5871368a3247dd622521f4</param></service></servicedata> + <param name="changesrevision">8a4dcf0868af718aaa5871368a3247dd622521f4</param></service><service name="tar_scm"> + <param name="url">https://github.com/ansible-semaphore/semaphore.git</param> + <param name="changesrevision">5d87656a680600125fe78edec1f7a0d10484b52c</param></service></servicedata> (No newline at EOF) ++++++ semaphore-2.18.13.obscpio -> semaphore-2.18.20.obscpio ++++++ diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/api/projects/keys.go new/semaphore-2.18.20/api/projects/keys.go --- old/semaphore-2.18.13/api/projects/keys.go 2026-06-08 19:58:47.000000000 +0200 +++ new/semaphore-2.18.20/api/projects/keys.go 2026-07-05 21:21:33.000000000 +0200 @@ -139,6 +139,21 @@ return } + // access key ID and project ID in the body and the path must be the same + if key.ID != oldKey.ID { + helpers.WriteJSON(w, http.StatusBadRequest, map[string]string{ + "error": "Access key id in URL and in body must be the same", + }) + return + } + + if oldKey.ProjectID == nil || key.ProjectID == nil || *key.ProjectID != *oldKey.ProjectID { + helpers.WriteJSON(w, http.StatusBadRequest, map[string]string{ + "error": "You can not move access key to other project", + }) + return + } + if oldKey.Synchronized { if key.Name != oldKey.Name || key.Type != oldKey.Type { helpers.WriteJSON(w, http.StatusBadRequest, map[string]string{ diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/api/projects/keys_test.go new/semaphore-2.18.20/api/projects/keys_test.go --- old/semaphore-2.18.13/api/projects/keys_test.go 1970-01-01 01:00:00.000000000 +0100 +++ new/semaphore-2.18.20/api/projects/keys_test.go 2026-07-05 21:21:33.000000000 +0200 @@ -0,0 +1,71 @@ +package projects + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/semaphoreui/semaphore/api/helpers" + "github.com/semaphoreui/semaphore/db" + "github.com/stretchr/testify/assert" +) + +func intPtr(v int) *int { return &v } + +// newUpdateKeyRequest builds a request with the URL-resolved oldKey already in +// context (as KeyMiddleware would set it) and the given JSON body. +func newUpdateKeyRequest(oldKey db.AccessKey, body string) (*http.Request, *httptest.ResponseRecorder) { + req := httptest.NewRequest(http.MethodPut, "/api/project/1/keys/1", strings.NewReader(body)) + req = helpers.SetContextValue(req, "accessKey", oldKey) + return req, httptest.NewRecorder() +} + +func TestUpdateKey_RejectsBodyIDMismatch(t *testing.T) { + + svc := &mockAccessKeyService{} + ctrl := NewKeyController(svc) + + oldKey := db.AccessKey{ID: 10, ProjectID: intPtr(1)} + // Body targets a different key id than the one resolved from the URL. + body := `{"id":42,"name":"x","type":"none","project_id":1}` + req, w := newUpdateKeyRequest(oldKey, body) + + ctrl.UpdateKey(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "must be the same") + assert.Empty(t, svc.updated, "store must not be touched on rejection") +} + +func TestUpdateKey_RejectsCrossProjectMove(t *testing.T) { + svc := &mockAccessKeyService{} + ctrl := NewKeyController(svc) + + oldKey := db.AccessKey{ID: 10, ProjectID: intPtr(1)} + // Body keeps the same key id but points project_id at a foreign project. + body := `{"id":10,"name":"x","type":"none","project_id":999}` + req, w := newUpdateKeyRequest(oldKey, body) + + ctrl.UpdateKey(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "other project") + assert.Empty(t, svc.updated, "store must not be touched on rejection") +} + +func TestUpdateKey_RejectsNilBodyProjectID(t *testing.T) { + svc := &mockAccessKeyService{} + ctrl := NewKeyController(svc) + + oldKey := db.AccessKey{ID: 10, ProjectID: intPtr(1)} + // Body omits project_id entirely. + body := `{"id":10,"name":"x","type":"none"}` + req, w := newUpdateKeyRequest(oldKey, body) + + ctrl.UpdateKey(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "other project") + assert.Empty(t, svc.updated, "store must not be touched on rejection") +} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/api/projects/project.go new/semaphore-2.18.20/api/projects/project.go --- old/semaphore-2.18.13/api/projects/project.go 2026-06-08 19:58:47.000000000 +0200 +++ new/semaphore-2.18.20/api/projects/project.go 2026-07-05 21:21:33.000000000 +0200 @@ -46,14 +46,20 @@ permissions := roleSlug.GetPermissions() - role, err := helpers.Store(r).GetProjectOrGlobalRoleBySlug(projectID, string(projectUser.Role)) + // Built-in roles are defined in code and are the source of truth for their + // permissions. Only custom roles are resolved from the database, otherwise a + // project role sharing a built-in slug (e.g. "manager") could override the + // built-in permissions and escalate privileges. + if !roleSlug.IsValid() { + role, err := helpers.Store(r).GetProjectOrGlobalRoleBySlug(projectID, string(projectUser.Role)) - if err == nil { - roleSlug = db.ProjectUserRole(role.Slug) - permissions = role.Permissions - } else if !errors.Is(err, db.ErrNotFound) { - helpers.WriteError(w, err) - return + if err == nil { + roleSlug = db.ProjectUserRole(role.Slug) + permissions = role.Permissions + } else if !errors.Is(err, db.ErrNotFound) { + helpers.WriteError(w, err) + return + } } if helpers.HasParam("template_id", r) { diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db/Repository.go new/semaphore-2.18.20/db/Repository.go --- old/semaphore-2.18.13/db/Repository.go 2026-06-08 19:58:47.000000000 +0200 +++ new/semaphore-2.18.20/db/Repository.go 2026-07-05 21:21:33.000000000 +0200 @@ -144,6 +144,10 @@ return &ValidationError{"repository url can't be empty"} } + if err := ValidateGitURL(r.GitURL, "repository"); err != nil { + return err + } + if r.GetType() != RepositoryLocal && r.GitBranch == "" { return &ValidationError{"repository branch can't be empty"} } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db/Role.go new/semaphore-2.18.20/db/Role.go --- old/semaphore-2.18.13/db/Role.go 2026-06-08 19:58:47.000000000 +0200 +++ new/semaphore-2.18.20/db/Role.go 2026-07-05 21:21:33.000000000 +0200 @@ -11,6 +11,14 @@ if role.Name == "" { return &ValidationError{Message: "Role name cannot be empty"} } + if role.Slug == "" { + return &ValidationError{Message: "Role slug cannot be empty"} + } + // Built-in role slugs are reserved. Allowing a custom role to reuse one lets + // it shadow the built-in role and escalate the permissions of its members. + if ProjectUserRole(role.Slug).IsValid() { + return &ValidationError{Message: "Role slug is reserved and cannot be used: " + role.Slug} + } return nil } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db/Role_test.go new/semaphore-2.18.20/db/Role_test.go --- old/semaphore-2.18.13/db/Role_test.go 1970-01-01 01:00:00.000000000 +0100 +++ new/semaphore-2.18.20/db/Role_test.go 2026-07-05 21:21:33.000000000 +0200 @@ -0,0 +1,73 @@ +package db + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidateRole(t *testing.T) { + projectID := 1 + + tests := []struct { + name string + role Role + wantErr bool + }{ + { + name: "valid custom role", + role: Role{Slug: "deployer", Name: "Deployer", ProjectID: &projectID}, + wantErr: false, + }, + { + name: "empty name", + role: Role{Slug: "deployer", Name: "", ProjectID: &projectID}, + wantErr: true, + }, + { + name: "empty slug", + role: Role{Slug: "", Name: "Deployer", ProjectID: &projectID}, + wantErr: true, + }, + { + name: "reserved slug owner", + role: Role{Slug: string(ProjectOwner), Name: "pwn", ProjectID: &projectID}, + wantErr: true, + }, + { + name: "reserved slug manager", + role: Role{Slug: string(ProjectManager), Name: "pwn", ProjectID: &projectID}, + wantErr: true, + }, + { + name: "reserved slug task_runner", + role: Role{Slug: string(ProjectTaskRunner), Name: "pwn", ProjectID: &projectID}, + wantErr: true, + }, + { + name: "reserved slug guest", + role: Role{Slug: string(ProjectGuest), Name: "pwn", ProjectID: &projectID}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateRole(tt.role) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +// TestValidateRole_ReservedSlugsMatchBuiltins guards against a built-in role +// being added later without also reserving its slug in ValidateRole. +func TestValidateRole_ReservedSlugsMatchBuiltins(t *testing.T) { + for role := range rolePermissions { + err := ValidateRole(Role{Slug: string(role), Name: "custom"}) + assert.Error(t, err, "built-in slug %q must be rejected by ValidateRole", role) + } +} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db/Task.go new/semaphore-2.18.20/db/Task.go --- old/semaphore-2.18.13/db/Task.go 2026-06-08 19:58:47.000000000 +0200 +++ new/semaphore-2.18.20/db/Task.go 2026-07-05 21:21:33.000000000 +0200 @@ -169,6 +169,10 @@ } } + if err := ValidatePlaybookPath(task.Playbook, "task"); err != nil { + return err + } + var params any switch template.App { case AppAnsible: diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db/Template.go new/semaphore-2.18.20/db/Template.go --- old/semaphore-2.18.13/db/Template.go 2026-06-08 19:58:47.000000000 +0200 +++ new/semaphore-2.18.20/db/Template.go 2026-07-05 21:21:33.000000000 +0200 @@ -227,6 +227,10 @@ return &ValidationError{"template playbook can not be empty"} } + if err := ValidatePlaybookPath(tpl.Playbook, "template"); err != nil { + return err + } + if tpl.Arguments != nil { if !json.Valid([]byte(*tpl.Arguments)) { return &ValidationError{"template arguments must be valid JSON"} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db/git_url.go new/semaphore-2.18.20/db/git_url.go --- old/semaphore-2.18.13/db/git_url.go 1970-01-01 01:00:00.000000000 +0100 +++ new/semaphore-2.18.20/db/git_url.go 2026-07-05 21:21:33.000000000 +0200 @@ -0,0 +1,19 @@ +package db + +import "strings" + +// ValidateGitURL rejects repository URLs that git would interpret as a +// command-line option instead of a repository location. The CmdGitClient +// passes the URL to the git binary as a positional argument, so a value +// beginning with "-" (e.g. "--upload-pack=/path/to/script") would be parsed +// by git as an option and could lead to arbitrary command execution +// (git option injection). Legitimate git URLs (https://, ssh://, git://, +// file://, scp-like user@host:path, or local filesystem paths) never begin +// with "-", so rejecting them here is safe. +func ValidateGitURL(url string, objectName string) error { + if strings.HasPrefix(strings.TrimSpace(url), "-") { + return NewValidationError(objectName + " url is invalid") + } + + return nil +} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db/git_url_test.go new/semaphore-2.18.20/db/git_url_test.go --- old/semaphore-2.18.13/db/git_url_test.go 1970-01-01 01:00:00.000000000 +0100 +++ new/semaphore-2.18.20/db/git_url_test.go 2026-07-05 21:21:33.000000000 +0200 @@ -0,0 +1,65 @@ +package db + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateGitURL(t *testing.T) { + tests := []struct { + name string + url string + wantErr bool + }{ + {"https", "https://github.com/semaphoreui/semaphore.git";, false}, + {"http", "http://example.com/repo.git";, false}, + {"ssh scheme", "ssh://[email protected]/repo.git", false}, + {"scp-like ssh", "[email protected]:semaphoreui/semaphore.git", false}, + {"git scheme", "git://example.com/repo.git", false}, + {"file scheme", "file:///srv/git/repo.git", false}, + {"local absolute path", "/srv/git/repo.git", false}, + {"empty", "", false}, // emptiness is handled separately by Repository.Validate + + {"upload-pack option injection", "--upload-pack=/tmp/evil.sh", true}, + {"single dash option", "-oProxyCommand=evil", true}, + {"leading whitespace then dash", " --upload-pack=/tmp/evil.sh", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateGitURL(tt.url, "repository") + if tt.wantErr { + require.Error(t, err) + assert.ErrorContains(t, err, "repository url is invalid") + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestRepositoryValidate_RejectsOptionInjectionURL(t *testing.T) { + repo := Repository{ + Name: "rce", + GitURL: "--upload-pack=/tmp/evil.sh", + GitBranch: "main", + SSHKeyID: 1, + } + + err := repo.Validate() + require.Error(t, err) + assert.ErrorContains(t, err, "repository url is invalid") +} + +func TestRepositoryValidate_AcceptsNormalURL(t *testing.T) { + repo := Repository{ + Name: "ok", + GitURL: "https://github.com/semaphoreui/semaphore.git";, + GitBranch: "main", + SSHKeyID: 1, + } + + assert.NoError(t, repo.Validate()) +} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db/playbook_path.go new/semaphore-2.18.20/db/playbook_path.go --- old/semaphore-2.18.13/db/playbook_path.go 1970-01-01 01:00:00.000000000 +0100 +++ new/semaphore-2.18.20/db/playbook_path.go 2026-07-05 21:21:33.000000000 +0200 @@ -0,0 +1,36 @@ +package db + +import ( + "path" + "strings" +) + +// ValidatePlaybookPath checks that a playbook (or script/subdirectory for +// non-Ansible apps) path is relative and stays inside the repository bound +// to the template. Absolute paths and paths escaping the repository via ".." +// are rejected to prevent execution of arbitrary files on the host. +func ValidatePlaybookPath(playbook string, objectName string) error { + if playbook == "" { + return nil + } + + // Treat backslashes as path separators so Windows-style paths + // (C:\..., ..\..\evil.ps1) can not bypass the checks below. + p := strings.ReplaceAll(playbook, "\\", "/") + + if path.IsAbs(p) { + return NewValidationError(objectName + " playbook must be a relative path inside the repository") + } + + // Windows absolute paths like "C:/..." are not caught by path.IsAbs. + if len(p) >= 2 && p[1] == ':' { + return NewValidationError(objectName + " playbook must be a relative path inside the repository") + } + + cleaned := path.Clean(p) + if cleaned == ".." || strings.HasPrefix(cleaned, "../") { + return NewValidationError(objectName + " playbook must not point outside the repository") + } + + return nil +} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db/playbook_path_test.go new/semaphore-2.18.20/db/playbook_path_test.go --- old/semaphore-2.18.13/db/playbook_path_test.go 1970-01-01 01:00:00.000000000 +0100 +++ new/semaphore-2.18.20/db/playbook_path_test.go 2026-07-05 21:21:33.000000000 +0200 @@ -0,0 +1,97 @@ +package db + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestValidatePlaybookPath(t *testing.T) { + tests := []struct { + name string + playbook string + valid bool + }{ + {"empty", "", true}, + {"simple filename", "site.yml", true}, + {"subdirectory", "playbooks/site.yml", true}, + {"dot-slash prefix", "./site.yml", true}, + {"internal dot-dot staying inside", "playbooks/../site.yml", true}, + {"terraform subdirectory", "environments/prod", true}, + + {"absolute path", "/etc/cron.d/evil.yml", false}, + {"absolute script", "/usr/bin/script.sh", false}, + {"parent escape", "../outside.yml", false}, + {"deep parent escape", "playbooks/../../outside.yml", false}, + {"hidden parent escape", "./../outside.yml", false}, + {"only dot-dot", "..", false}, + {"windows absolute path", "C:\\Windows\\evil.ps1", false}, + {"windows drive forward slashes", "c:/windows/evil.ps1", false}, + {"windows parent escape", "..\\evil.ps1", false}, + {"windows unc path", "\\\\server\\share\\evil.ps1", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidatePlaybookPath(tt.playbook, "template") + if tt.valid { + assert.NoError(t, err) + } else { + assert.Error(t, err) + } + }) + } +} + +func TestTemplate_Validate_AbsolutePlaybook_ReturnsError(t *testing.T) { + tpl := &Template{ + Name: "Deploy", + Playbook: "/etc/evil.yml", + } + + err := tpl.Validate() + + assert.EqualError(t, err, "template playbook must be a relative path inside the repository") +} + +func TestTemplate_Validate_PlaybookOutsideRepo_ReturnsError(t *testing.T) { + tpl := &Template{ + Name: "Deploy", + Playbook: "../outside.yml", + } + + err := tpl.Validate() + + assert.EqualError(t, err, "template playbook must not point outside the repository") +} + +func TestTemplate_Validate_RelativePlaybook_ReturnsNoError(t *testing.T) { + tpl := &Template{ + Name: "Deploy", + Playbook: "playbooks/site.yml", + } + + err := tpl.Validate() + + assert.NoError(t, err) +} + +func TestTask_ValidateNewTask_AbsolutePlaybook_ReturnsError(t *testing.T) { + task := &Task{ + Playbook: "/usr/bin/evil.sh", + } + + err := task.ValidateNewTask(Template{}) + + assert.EqualError(t, err, "task playbook must be a relative path inside the repository") +} + +func TestTask_ValidateNewTask_RelativePlaybook_ReturnsNoError(t *testing.T) { + task := &Task{ + Playbook: "site.yml", + } + + err := task.ValidateNewTask(Template{}) + + assert.NoError(t, err) +} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db/sql/access_key.go new/semaphore-2.18.20/db/sql/access_key.go --- old/semaphore-2.18.13/db/sql/access_key.go 2026-06-08 19:58:47.000000000 +0200 +++ new/semaphore-2.18.20/db/sql/access_key.go 2026-07-05 21:21:33.000000000 +0200 @@ -34,6 +34,8 @@ case db.AccessKeySecretStorage: q = q.Where(squirrel.Eq{"pe.storage_id": options.StorageID}) } + } else if options.EnvironmentID != nil { + q = q.Where(squirrel.Eq{"pe.environment_id": *options.EnvironmentID}) } if options.SourceStorageID != nil { diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db/sql/role.go new/semaphore-2.18.20/db/sql/role.go --- old/semaphore-2.18.13/db/sql/role.go 2026-06-08 19:58:47.000000000 +0200 +++ new/semaphore-2.18.20/db/sql/role.go 2026-07-05 21:21:33.000000000 +0200 @@ -58,6 +58,10 @@ func (d *SqlDb) GetProjectOrGlobalRoleBySlug(projectID int, slug string) (db.Role, error) { var role db.Role - err := d.selectOne(&role, "select * from `role` where slug=?", slug) + err := d.selectOne( + &role, + "select * from `role` where slug=? and (project_id=? or project_id is null)", + slug, + projectID) return role, err } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db/sql/template.go new/semaphore-2.18.20/db/sql/template.go --- old/semaphore-2.18.13/db/sql/template.go 2026-06-08 19:58:47.000000000 +0200 +++ new/semaphore-2.18.20/db/sql/template.go 2026-07-05 21:21:33.000000000 +0200 @@ -494,22 +494,31 @@ perm = projectUser.Role.GetPermissions() - role, err := d.GetProjectOrGlobalRoleBySlug(projectUser.ProjectID, string(projectUser.Role)) + roleSlug := string(projectUser.Role) - if errors.Is(err, db.ErrNotFound) { - err = nil - return - } + // Only custom roles are resolved from the database; built-in roles use their + // own slug directly so a same-named custom role cannot shadow them. + if !projectUser.Role.IsValid() { + var role db.Role + role, err = d.GetProjectOrGlobalRoleBySlug(projectUser.ProjectID, string(projectUser.Role)) + + if errors.Is(err, db.ErrNotFound) { + err = nil + return + } + + if err != nil { + return + } - if err != nil { - return + roleSlug = role.Slug } query, args, err := squirrel.Select("permissions"). From("project__template_role"). Where("project_id = ?", projectID). Where("template_id = ?", templateID). - Where("role_slug = ?", role.Slug). + Where("role_slug = ?", roleSlug). ToSql() if err != nil { diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db_lib/CmdGitClient.go new/semaphore-2.18.20/db_lib/CmdGitClient.go --- old/semaphore-2.18.13/db_lib/CmdGitClient.go 2026-06-08 19:58:47.000000000 +0200 +++ new/semaphore-2.18.20/db_lib/CmdGitClient.go 2026-07-05 21:21:33.000000000 +0200 @@ -115,6 +115,7 @@ "--recursive", "--branch", r.Repository.GitBranch, + "--end-of-options", r.Repository.GetGitURL(false), dirName) } @@ -122,7 +123,7 @@ func (c CmdGitClient) Pull(r GitRepository) error { r.Logger.Log("Updating Repository " + r.Repository.GitURL) - err := c.run(r, GitRepositoryFullPath, "pull", "origin", r.Repository.GitBranch) + err := c.run(r, GitRepositoryFullPath, "pull", "origin", "--end-of-options", r.Repository.GitBranch) if err != nil { return err } @@ -167,7 +168,7 @@ } func (c CmdGitClient) GetLastRemoteCommitHash(r GitRepository) (hash string, err error) { - out, err := c.output(r, GitRepositoryTmpPath, "ls-remote", r.Repository.GetGitURL(false), r.Repository.GitBranch) + out, err := c.output(r, GitRepositoryTmpPath, "ls-remote", "--end-of-options", r.Repository.GetGitURL(false), r.Repository.GitBranch) if err != nil { return } @@ -185,7 +186,7 @@ } func (c CmdGitClient) GetRemoteBranches(r GitRepository) ([]string, error) { - out, err := c.output(r, GitRepositoryTmpPath, "ls-remote", "--heads", r.Repository.GetGitURL(false)) + out, err := c.output(r, GitRepositoryTmpPath, "ls-remote", "--heads", "--end-of-options", r.Repository.GetGitURL(false)) if err != nil { return nil, err } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/db_lib/CmdGitClient_injection_test.go new/semaphore-2.18.20/db_lib/CmdGitClient_injection_test.go --- old/semaphore-2.18.13/db_lib/CmdGitClient_injection_test.go 1970-01-01 01:00:00.000000000 +0100 +++ new/semaphore-2.18.20/db_lib/CmdGitClient_injection_test.go 2026-07-05 21:21:33.000000000 +0200 @@ -0,0 +1,128 @@ +package db_lib + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/semaphoreui/semaphore/db" + "github.com/semaphoreui/semaphore/pkg/ssh" + "github.com/semaphoreui/semaphore/pkg/task_logger" + "github.com/semaphoreui/semaphore/util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// nopKeyInstaller is an AccessKeyInstaller that installs nothing (as for AccessKeyNone). +type nopKeyInstaller struct{} + +func (nopKeyInstaller) Install(key db.AccessKey, usage db.AccessKeyRole, logger task_logger.Logger) (ssh.AccessKeyInstallation, error) { + return ssh.AccessKeyInstallation{}, nil +} + +// nopLogger is a no-op task_logger.Logger for tests. +type nopLogger struct{} + +func (nopLogger) Log(string) {} +func (nopLogger) Logf(string, ...any) {} +func (nopLogger) LogWithTime(time.Time, string) {} +func (nopLogger) LogfWithTime(time.Time, string, ...any) {} +func (nopLogger) LogCmd(*exec.Cmd) {} +func (nopLogger) SetStatus(task_logger.TaskStatus) {} +func (nopLogger) AddStatusListener(task_logger.StatusListener) {} +func (nopLogger) AddLogListener(task_logger.LogListener) {} +func (nopLogger) SetCommit(string, string) {} +func (nopLogger) WaitLog() {} + +func gitInit(t *testing.T, dir string) { + t.Helper() + run := func(args ...string) { + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + require.NoError(t, err, string(out)) + } + run("init", "-q", "-b", "main") + run("config", "user.email", "t@t") + run("config", "user.name", "t") + require.NoError(t, os.WriteFile(filepath.Join(dir, "f"), []byte("hi"), 0644)) + run("add", "f") + run("commit", "-qm", "init") +} + +func newTestGitRepo(t *testing.T, gitURL, gitBranch string) GitRepository { + t.Helper() + return GitRepository{ + Repository: db.Repository{ + ProjectID: 1, + GitURL: gitURL, + GitBranch: gitBranch, + SSHKey: db.AccessKey{Type: db.AccessKeyNone}, + }, + Logger: nopLogger{}, + } +} + +// setupGitClientTest initializes util.Config with a temp dir and restores it afterwards. +func setupGitClientTest(t *testing.T) { + t.Helper() + original := util.Config + t.Cleanup(func() { util.Config = original }) + util.Config = &util.ConfigType{TmpPath: t.TempDir(), Process: &util.ConfigProcess{}} +} + +// TestCmdGitClient_OptionInjectionNeutralized proves that a GitURL crafted as a git +// option (e.g. "--upload-pack=/path/to/script") is NOT executed by the remote git +// operations, because "--end-of-options" is inserted before the URL positional. +func TestCmdGitClient_OptionInjectionNeutralized(t *testing.T) { + setupGitClientTest(t) + + tmp := t.TempDir() + marker := filepath.Join(tmp, "PWNED") + evil := filepath.Join(tmp, "evil.sh") + require.NoError(t, os.WriteFile(evil, + []byte("#!/bin/sh\necho pwned > "+marker+"\n"), 0755)) + + client := CreateCmdGitClient(nopKeyInstaller{}) + repo := newTestGitRepo(t, "--upload-pack="+evil, "main") + + t.Run("GetLastRemoteCommitHash", func(t *testing.T) { + _, err := client.GetLastRemoteCommitHash(repo) + assert.Error(t, err, "injected upload-pack must not succeed") + assert.NoFileExists(t, marker, "payload must not be executed") + }) + + require.NoError(t, os.RemoveAll(marker)) + + t.Run("GetRemoteBranches", func(t *testing.T) { + _, err := client.GetRemoteBranches(repo) + assert.Error(t, err, "injected upload-pack must not succeed") + assert.NoFileExists(t, marker, "payload must not be executed") + }) +} + +// TestCmdGitClient_LegitRemoteOperations makes sure "--end-of-options" does not break +// normal remote operations against a valid repository. +func TestCmdGitClient_LegitRemoteOperations(t *testing.T) { + setupGitClientTest(t) + + upstream := t.TempDir() + gitInit(t, upstream) + + client := CreateCmdGitClient(nopKeyInstaller{}) + repo := newTestGitRepo(t, upstream, "main") + + t.Run("GetLastRemoteCommitHash", func(t *testing.T) { + hash, err := client.GetLastRemoteCommitHash(repo) + require.NoError(t, err) + assert.Len(t, hash, 40, "expected a full commit hash") + }) + + t.Run("GetRemoteBranches", func(t *testing.T) { + branches, err := client.GetRemoteBranches(repo) + require.NoError(t, err) + assert.Equal(t, []string{"main"}, branches) + }) +} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/deployment/docker/runner/Dockerfile new/semaphore-2.18.20/deployment/docker/runner/Dockerfile --- old/semaphore-2.18.13/deployment/docker/runner/Dockerfile 2026-06-08 19:58:47.000000000 +0200 +++ new/semaphore-2.18.20/deployment/docker/runner/Dockerfile 2026-07-05 21:21:33.000000000 +0200 @@ -19,7 +19,7 @@ ARG GH_TOKEN RUN if [ -n "$APP_BUILD_TYPE" ]; then \ - git clone -b main https://${GH_TOKEN}@github.com/semaphoreui/semaphorepro-module.git pro_impl && \ + git clone -b 2-18-stable https://${GH_TOKEN}@github.com/semaphoreui/semaphorepro-module.git pro_impl && \ go work init . ./pro_impl; \ fi diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/deployment/docker/server/Dockerfile new/semaphore-2.18.20/deployment/docker/server/Dockerfile --- old/semaphore-2.18.13/deployment/docker/server/Dockerfile 2026-06-08 19:58:47.000000000 +0200 +++ new/semaphore-2.18.20/deployment/docker/server/Dockerfile 2026-07-05 21:21:33.000000000 +0200 @@ -19,7 +19,7 @@ ARG GH_TOKEN RUN if [ -n "$APP_BUILD_TYPE" ]; then \ - git clone -b main https://${GH_TOKEN}@github.com/semaphoreui/semaphorepro-module.git pro_impl && \ + git clone -b 2-18-stable https://${GH_TOKEN}@github.com/semaphoreui/semaphorepro-module.git pro_impl && \ go work init . ./pro_impl; \ fi diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/semaphore-2.18.13/services/tasks/LocalJob.go new/semaphore-2.18.20/services/tasks/LocalJob.go --- old/semaphore-2.18.13/services/tasks/LocalJob.go 2026-06-08 19:58:47.000000000 +0200 +++ new/semaphore-2.18.20/services/tasks/LocalJob.go 2026-07-05 21:21:33.000000000 +0200 @@ -39,6 +39,28 @@ KeyInstaller db_lib.AccessKeyInstaller } +// resolveGitBranch computes the effective git branch for a run, applying the +// template-level and task-level overrides on top of the repository's configured +// branch. The task-supplied branch is honored only when the template explicitly +// permits it via AllowOverrideBranchInTask; otherwise it is ignored. This gate is +// what prevents a user who may only run tasks (Task Runner) from redirecting a +// branch-pinned template to an arbitrary branch of the repository. +func resolveGitBranch(repoBranch string, template db.Template, task db.Task) string { + branch := repoBranch + + // Override git branch from template if set. + if template.GitBranch != nil && *template.GitBranch != "" { + branch = *template.GitBranch + } + + // Override git branch from task only if the template allows it. + if template.AllowOverrideBranchInTask && task.GitBranch != nil && *task.GitBranch != "" { + branch = *task.GitBranch + } + + return branch +} + func (t *LocalJob) IsKilled() bool { return t.killed } @@ -647,6 +669,17 @@ t.SetStatus(task_logger.TaskRunningStatus) // It is required for local mode. Don't delete + // Defense in depth: reject playbook paths pointing outside the repository + // even if they were stored before validation was added. + if err = db.ValidatePlaybookPath(t.Template.Playbook, "template"); err != nil { + t.Log(err.Error()) + return + } + if err = db.ValidatePlaybookPath(t.Task.Playbook, "task"); err != nil { + t.Log(err.Error()) + return + } + environmentVariables, err := t.getEnvironmentENV() if err != nil { return @@ -808,15 +841,7 @@ } } - // Override git branch from template if set - if t.Template.GitBranch != nil && *t.Template.GitBranch != "" { - t.Repository.GitBranch = *t.Template.GitBranch - } - - // Override git branch from task if set - if t.Task.GitBranch != nil && *t.Task.GitBranch != "" { - t.Repository.GitBranch = *t.Task.GitBranch - } + t.Repository.GitBranch = resolveGitBranch(t.Repository.GitBranch, t.Template, t.Task) if t.Repository.GetType() == db.RepositoryLocal { localPath := t.Repository.GetGitURL(true) @@ -873,15 +898,7 @@ } } - // Override git branch from template if set - if t.Template.GitBranch != nil && *t.Template.GitBranch != "" { - t.Repository.GitBranch = *t.Template.GitBranch - } - - // Override git branch from task if set - if t.Task.GitBranch != nil && *t.Task.GitBranch != "" { - t.Repository.GitBranch = *t.Task.GitBranch - } + t.Repository.GitBranch = resolveGitBranch(t.Repository.GitBranch, t.Template, t.Task) if t.Repository.GetType() == db.RepositoryLocal { localPath := t.Repository.GetGitURL(true) ++++++ semaphore.obsinfo ++++++ --- /var/tmp/diff_new_pack.zr1Bme/_old 2026-07-06 12:37:23.073939928 +0200 +++ /var/tmp/diff_new_pack.zr1Bme/_new 2026-07-06 12:37:23.105941040 +0200 @@ -1,5 +1,5 @@ name: semaphore -version: 2.18.13 -mtime: 1780941527 -commit: 8a4dcf0868af718aaa5871368a3247dd622521f4 +version: 2.18.20 +mtime: 1783279293 +commit: 5d87656a680600125fe78edec1f7a0d10484b52c ++++++ vendor.tar.gz ++++++ /work/SRC/openSUSE:Factory/semaphore/vendor.tar.gz /work/SRC/openSUSE:Factory/.semaphore.new.1982/vendor.tar.gz differ: char 13, line 1 ++++++ web-2.18.13.tar.gz -> web-2.18.20.tar.gz ++++++ /work/SRC/openSUSE:Factory/semaphore/web-2.18.13.tar.gz /work/SRC/openSUSE:Factory/.semaphore.new.1982/web-2.18.20.tar.gz differ: char 31, line 1
