diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 2f06bd070e..c9cc53bd0b 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -39,7 +39,7 @@ jobs: - name: Add PRO implementation run: | - git clone -b main https://${{ secrets.GH_TOKEN }}@github.com/semaphoreui/semaphorepro-module.git pro_impl + git clone -b 2-18-stable https://${{ secrets.GH_TOKEN }}@github.com/semaphoreui/semaphorepro-module.git pro_impl go work init . ./pro_impl - name: Run build diff --git a/.github/workflows/pro_selfhosted_beta.yml b/.github/workflows/pro_selfhosted_beta.yml index 03c0e98e78..9d3db3a93a 100644 --- a/.github/workflows/pro_selfhosted_beta.yml +++ b/.github/workflows/pro_selfhosted_beta.yml @@ -39,7 +39,7 @@ jobs: - name: Add PRO implementation run: | - git clone -b main https://${{ secrets.GH_TOKEN }}@github.com/semaphoreui/semaphorepro-module.git pro_impl + git clone -b 2-18-stable https://${{ secrets.GH_TOKEN }}@github.com/semaphoreui/semaphorepro-module.git pro_impl go work init . ./pro_impl - name: Install deps diff --git a/.github/workflows/pro_selfhosted_release.yml b/.github/workflows/pro_selfhosted_release.yml index 08ff52cc37..e3e739d4ee 100644 --- a/.github/workflows/pro_selfhosted_release.yml +++ b/.github/workflows/pro_selfhosted_release.yml @@ -38,7 +38,7 @@ jobs: - name: Add PRO implementation run: | - git clone -b main https://${{ secrets.GH_TOKEN }}@github.com/semaphoreui/semaphorepro-module.git pro_impl + git clone -b 2-18-stable https://${{ secrets.GH_TOKEN }}@github.com/semaphoreui/semaphorepro-module.git pro_impl go work init . ./pro_impl - name: Install deps diff --git a/api-docs.yml b/api-docs.yml index dbc59a0263..a09aa81999 100644 --- a/api-docs.yml +++ b/api-docs.yml @@ -855,6 +855,8 @@ definitions: type: array items: type: string + skip_galaxy_install: + type: boolean TerraformTaskParams: type: object diff --git a/api/auth.go b/api/auth.go index 26ca9736f2..52d8fbfd65 100644 --- a/api/auth.go +++ b/api/auth.go @@ -3,6 +3,7 @@ package api import ( "errors" "net/http" + "net/url" "strings" "time" @@ -322,3 +323,90 @@ func adminMiddleware(next http.Handler) http.Handler { next.ServeHTTP(w, r) }) } + +// isStateChangingMethod reports whether an HTTP method can modify server state +// and therefore requires CSRF protection. Safe methods (GET, HEAD, OPTIONS, +// TRACE) are excluded. +func isStateChangingMethod(method string) bool { + switch method { + case http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete: + return true + default: + return false + } +} + +// requestOriginHost extracts the origin host (host[:port]) of the request from +// the Origin header, falling back to the Referer header. The boolean is false +// when neither header is present or parseable. +func requestOriginHost(r *http.Request) (string, bool) { + for _, header := range []string{"Origin", "Referer"} { + value := r.Header.Get(header) + if value == "" { + continue + } + + u, err := url.Parse(value) + if err != nil || u.Host == "" { + continue + } + + return u.Host, true + } + + return "", false +} + +// isSameOriginHost reports whether host belongs to Semaphore itself. Both the +// configured public web host and the host the request was addressed to are +// accepted, so reverse-proxy deployments keep working. +func isSameOriginHost(host string, r *http.Request) bool { + if host == r.Host { + return true + } + + if util.WebHostURL != nil && host == util.WebHostURL.Host { + return true + } + + return false +} + +// csrfProtectionMiddleware blocks cross-site state-changing requests that rely +// on the session cookie, providing defense-in-depth against CSRF on top of the +// SameSite=Lax session cookie. +// +// Requests authenticated with an API token (Authorization: bearer) are exempt: +// browsers never attach such tokens automatically, so token-based clients are +// not vulnerable to CSRF and must keep working without an Origin header. +// +// When neither Origin nor Referer is present (e.g. non-browser clients using a +// cookie), the request is allowed — the SameSite=Lax cookie already prevents a +// browser from sending the session cookie cross-site in that case. +func csrfProtectionMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !isStateChangingMethod(r.Method) { + next.ServeHTTP(w, r) + return + } + + authHeader := strings.ToLower(r.Header.Get("authorization")) + if strings.Contains(authHeader, "bearer") { + next.ServeHTTP(w, r) + return + } + + if origin, ok := requestOriginHost(r); ok && !isSameOriginHost(origin, r) { + log.WithFields(log.Fields{ + "origin": origin, + "host": r.Host, + "path": r.URL.Path, + "method": r.Method, + }).Warn("Blocked cross-origin request (possible CSRF)") + helpers.WriteErrorStatus(w, "CROSS_ORIGIN_REQUEST_BLOCKED", http.StatusForbidden) + return + } + + next.ServeHTTP(w, r) + }) +} diff --git a/api/auth_test.go b/api/auth_test.go new file mode 100644 index 0000000000..bfdad6ced5 --- /dev/null +++ b/api/auth_test.go @@ -0,0 +1,189 @@ +package api + +import ( + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/semaphoreui/semaphore/util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIsStateChangingMethod(t *testing.T) { + tests := []struct { + method string + expected bool + }{ + {http.MethodGet, false}, + {http.MethodHead, false}, + {http.MethodOptions, false}, + {http.MethodPost, true}, + {http.MethodPut, true}, + {http.MethodPatch, true}, + {http.MethodDelete, true}, + } + + for _, tt := range tests { + t.Run(tt.method, func(t *testing.T) { + assert.Equal(t, tt.expected, isStateChangingMethod(tt.method)) + }) + } +} + +func TestRequestOriginHost(t *testing.T) { + t.Run("from Origin header", func(t *testing.T) { + r := httptest.NewRequest(http.MethodPost, "/api/users/1/password", nil) + r.Header.Set("Origin", "https://semaphore.example.com") + + host, ok := requestOriginHost(r) + assert.True(t, ok) + assert.Equal(t, "semaphore.example.com", host) + }) + + t.Run("falls back to Referer", func(t *testing.T) { + r := httptest.NewRequest(http.MethodPost, "/api/users/1/password", nil) + r.Header.Set("Referer", "https://semaphore.example.com/project/1") + + host, ok := requestOriginHost(r) + assert.True(t, ok) + assert.Equal(t, "semaphore.example.com", host) + }) + + t.Run("Origin takes precedence over Referer", func(t *testing.T) { + r := httptest.NewRequest(http.MethodPost, "/api/users/1/password", nil) + r.Header.Set("Origin", "https://attacker.com") + r.Header.Set("Referer", "https://semaphore.example.com/") + + host, ok := requestOriginHost(r) + assert.True(t, ok) + assert.Equal(t, "attacker.com", host) + }) + + t.Run("no headers", func(t *testing.T) { + r := httptest.NewRequest(http.MethodPost, "/api/users/1/password", nil) + + _, ok := requestOriginHost(r) + assert.False(t, ok) + }) +} + +// newRecordingHandler returns an http.Handler that records whether it was +// called, used to assert that the middleware did or did not pass the request +// through. +func newRecordingHandler(called *bool) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + *called = true + w.WriteHeader(http.StatusNoContent) + }) +} + +func TestCsrfProtectionMiddleware(t *testing.T) { + orig := util.WebHostURL + defer func() { util.WebHostURL = orig }() + + webHost, err := url.Parse("https://semaphore.example.com") + require.NoError(t, err) + util.WebHostURL = webHost + + tests := []struct { + name string + method string + host string + origin string + referer string + authHeader string + wantStatus int + wantForwPass bool + }{ + { + name: "safe method is always allowed", + method: http.MethodGet, + host: "semaphore.example.com", + origin: "https://attacker.com", + wantStatus: http.StatusNoContent, + wantForwPass: true, + }, + { + name: "same origin POST is allowed", + method: http.MethodPost, + host: "semaphore.example.com", + origin: "https://semaphore.example.com", + wantStatus: http.StatusNoContent, + wantForwPass: true, + }, + { + name: "cross origin POST is blocked", + method: http.MethodPost, + host: "semaphore.example.com", + origin: "https://attacker.com", + wantStatus: http.StatusForbidden, + wantForwPass: false, + }, + { + name: "cross origin DELETE is blocked", + method: http.MethodDelete, + host: "semaphore.example.com", + origin: "https://attacker.com:1337", + wantStatus: http.StatusForbidden, + wantForwPass: false, + }, + { + name: "cross origin via Referer is blocked", + method: http.MethodPost, + host: "semaphore.example.com", + referer: "https://attacker.com/evil", + wantStatus: http.StatusForbidden, + wantForwPass: false, + }, + { + name: "missing origin and referer is allowed", + method: http.MethodPost, + host: "semaphore.example.com", + wantStatus: http.StatusNoContent, + wantForwPass: true, + }, + { + name: "bearer token bypasses origin check", + method: http.MethodPost, + host: "semaphore.example.com", + origin: "https://attacker.com", + authHeader: "Bearer sometoken", + wantStatus: http.StatusNoContent, + wantForwPass: true, + }, + { + name: "origin matching request host is allowed", + method: http.MethodPost, + host: "internal-proxy:3000", + origin: "http://internal-proxy:3000", + wantStatus: http.StatusNoContent, + wantForwPass: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := httptest.NewRequest(tt.method, "/api/users/1/password", nil) + r.Host = tt.host + if tt.origin != "" { + r.Header.Set("Origin", tt.origin) + } + if tt.referer != "" { + r.Header.Set("Referer", tt.referer) + } + if tt.authHeader != "" { + r.Header.Set("Authorization", tt.authHeader) + } + + var forwarded bool + w := httptest.NewRecorder() + + csrfProtectionMiddleware(newRecordingHandler(&forwarded)).ServeHTTP(w, r) + + assert.Equal(t, tt.wantStatus, w.Code) + assert.Equal(t, tt.wantForwPass, forwarded) + }) + } +} diff --git a/api/login.go b/api/login.go index 31deb6dd4b..7f60d438e2 100644 --- a/api/login.go +++ b/api/login.go @@ -201,9 +201,23 @@ func createSession(w http.ResponseWriter, r *http.Request, user db.User, oidc bo Value: encoded, Path: "/", HttpOnly: true, + // SameSite=Lax prevents the session cookie from being attached to + // cross-site POST requests, which mitigates CSRF (e.g. the password + // change endpoint). Top-level GET navigations still carry the cookie + // so following a link into Semaphore keeps the user logged in. + SameSite: http.SameSiteLaxMode, + // Secure is only enforced when Semaphore is served over HTTPS, so that + // it can still be used without TLS inside private networks. + Secure: isSecureWebHost(), }) } +// isSecureWebHost reports whether Semaphore's public web host uses HTTPS, in +// which case cookies should carry the Secure attribute. +func isSecureWebHost() bool { + return util.WebHostURL != nil && util.WebHostURL.Scheme == "https" +} + func loginByPassword(store db.Store, login string, password string) (user db.User, err error) { user, err = store.GetUserByLoginOrEmail(login, login) if err != nil { @@ -395,6 +409,8 @@ func logout(w http.ResponseWriter, r *http.Request) { Expires: tz.Now().Add(24 * 7 * time.Hour * -1), Path: "/", HttpOnly: true, + SameSite: http.SameSiteLaxMode, + Secure: isSecureWebHost(), }) w.WriteHeader(http.StatusNoContent) @@ -510,7 +526,16 @@ func oidcLogin(w http.ResponseWriter, r *http.Request) { return } state := generateStateOauthCookie(w, returnPath) - u := oauth.AuthCodeURL(state) + + // PKCE (RFC 7636), which RFC 9700 recommends for confidential clients too: + // the code challenge binds the authorization code to this browser, so a code + // intercepted in transit cannot be redeemed without the verifier. The + // verifier must never travel through the IdP, so it is kept in an HttpOnly + // cookie rather than folded into `state`. + codeVerifier := oauth2.GenerateVerifier() + setPkceVerifierCookie(w, codeVerifier) + + u := oauth.AuthCodeURL(state, oauth2.S256ChallengeOption(codeVerifier)) http.Redirect(w, r, u, http.StatusTemporaryRedirect) } @@ -552,6 +577,41 @@ func generateStateOauthCookie(w http.ResponseWriter, returnPath string) string { return base64.URLEncoding.EncodeToString(stateBytes) } +// pkceCookieName holds the PKCE code verifier for the duration of one +// authorization round-trip. Deliberately a cookie and not part of `state`: the +// state parameter is handed to the IdP and echoed back in a URL, which is +// exactly where the verifier must not appear. +const pkceCookieName = "oauthpkce" + +// setPkceVerifierCookie stores the code verifier until the IdP redirects back. +// HttpOnly keeps it away from scripts; SameSite=Lax still attaches it to the +// IdP's top-level GET navigation back to the redirect endpoint. Path is left +// unset so it scopes to /api/auth/oidc/, the same default the +// oauthstate cookie above relies on, and it expires quickly because an +// authorization round-trip is short and the verifier is single-use. +func setPkceVerifierCookie(w http.ResponseWriter, verifier string) { + http.SetCookie(w, &http.Cookie{ + Name: pkceCookieName, + Value: verifier, + Expires: tz.Now().Add(10 * time.Minute), + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + Secure: isSecureWebHost(), + }) +} + +// clearPkceVerifierCookie expires the verifier once it has been redeemed. +func clearPkceVerifierCookie(w http.ResponseWriter) { + http.SetCookie(w, &http.Cookie{ + Name: pkceCookieName, + Value: "", + MaxAge: -1, + HttpOnly: true, + SameSite: http.SameSiteLaxMode, + Secure: isSecureWebHost(), + }) +} + type claimResult struct { username string name string @@ -726,7 +786,18 @@ func oidcRedirect(w http.ResponseWriter, r *http.Request) { code := r.URL.Query().Get("code") - oauth2Token, err := oauth.Exchange(ctx, code) + // Redeem with the PKCE verifier this browser was issued at /login. A missing + // cookie is not treated as an error here: it cannot weaken the exchange, + // because a challenge was already registered with the IdP at authorize time + // and RFC 7636 requires the IdP to reject a redemption with no verifier. It + // only needs to not panic for logins already in flight across a restart. + var exchangeOpts []oauth2.AuthCodeOption + if pkceCookie, pkceErr := r.Cookie(pkceCookieName); pkceErr == nil && pkceCookie.Value != "" { + exchangeOpts = append(exchangeOpts, oauth2.VerifierOption(pkceCookie.Value)) + clearPkceVerifierCookie(w) + } + + oauth2Token, err := oauth.Exchange(ctx, code, exchangeOpts...) if err != nil { log.Error(err.Error()) http.Redirect(w, r, loginURL, http.StatusTemporaryRedirect) diff --git a/api/login_test.go b/api/login_test.go index f05fda8480..3eaee89b50 100644 --- a/api/login_test.go +++ b/api/login_test.go @@ -5,10 +5,13 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "net/url" "testing" "time" + "github.com/semaphoreui/semaphore/util" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestParseClaim(t *testing.T) { @@ -76,6 +79,35 @@ func TestParseClaim5(t *testing.T) { assert.Equal(t, "123456757343", res, "Result should match formatted ID") } +func TestIsSecureWebHost(t *testing.T) { + orig := util.WebHostURL + defer func() { util.WebHostURL = orig }() + + tests := []struct { + name string + webHost string + expected bool + }{ + {"https host is secure", "https://semaphore.example.com", true}, + {"http host is not secure", "http://semaphore.example.com:3000", false}, + {"nil host is not secure", "", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.webHost == "" { + util.WebHostURL = nil + } else { + u, err := url.Parse(tt.webHost) + require.NoError(t, err) + util.WebHostURL = u + } + + assert.Equal(t, tt.expected, isSecureWebHost()) + }) + } +} + func TestGenerateStateOauthCookie(t *testing.T) { w := httptest.NewRecorder() returnPath := "/dashboard" diff --git a/api/projects/keys.go b/api/projects/keys.go index f1887d330d..45c4d3437b 100644 --- a/api/projects/keys.go +++ b/api/projects/keys.go @@ -139,6 +139,21 @@ func (c *KeyController) UpdateKey(w http.ResponseWriter, r *http.Request) { 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 --git a/api/projects/keys_test.go b/api/projects/keys_test.go new file mode 100644 index 0000000000..8f89047c43 --- /dev/null +++ b/api/projects/keys_test.go @@ -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 --git a/api/projects/project.go b/api/projects/project.go index 749ff17fb4..020160c2f8 100644 --- a/api/projects/project.go +++ b/api/projects/project.go @@ -46,14 +46,20 @@ func ProjectMiddleware(next http.Handler) http.Handler { permissions := roleSlug.GetPermissions() - 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 + // 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 helpers.HasParam("template_id", r) { diff --git a/api/router.go b/api/router.go index 1399073067..b4b9e4847a 100644 --- a/api/router.go +++ b/api/router.go @@ -181,7 +181,7 @@ func Route( authenticatedWS.Path("/ws").HandlerFunc(sockets.Handler).Methods("GET", "HEAD") authenticatedAPI := r.PathPrefix(webPath + "api").Subrouter() - authenticatedAPI.Use(StoreMiddleware, JSONMiddleware, authentication) + authenticatedAPI.Use(csrfProtectionMiddleware, StoreMiddleware, JSONMiddleware, authentication) authenticatedAPI.Path("/info").HandlerFunc(systemInfoController.GetSystemInfo).Methods("GET", "HEAD") @@ -256,21 +256,21 @@ func Route( tasksAPI.Path("/{task_id}").HandlerFunc(tasks.DeleteTask).Methods("DELETE") userUserAPI := authenticatedAPI.Path("/users/{user_id}").Subrouter() - userUserAPI.Use(readonlyUserMiddleware) + userUserAPI.Use(usersController.ReadonlyUserMiddleware) userUserAPI.Methods("GET", "HEAD").HandlerFunc(userController.GetUser) userAPI := authenticatedAPI.Path("/users/{user_id}").Subrouter() - userAPI.Use(getUserMiddleware) + userAPI.Use(usersController.GetUserMiddleware) userAPI.Methods("PUT").HandlerFunc(usersController.UpdateUser) - userAPI.Methods("DELETE").HandlerFunc(deleteUser) + userAPI.Methods("DELETE").HandlerFunc(usersController.DeleteUser) userPasswordAPI := authenticatedAPI.PathPrefix("/users/{user_id}").Subrouter() - userPasswordAPI.Use(getUserMiddleware) - userPasswordAPI.Path("/password").HandlerFunc(updateUserPassword).Methods("POST") - userPasswordAPI.Path("/2fas/totp").HandlerFunc(enableTotp).Methods("POST") - userPasswordAPI.Path("/2fas/totp/{totp_id}/qr").HandlerFunc(totpQr).Methods("GET") - userPasswordAPI.Path("/2fas/totp/{totp_id}").HandlerFunc(disableTotp).Methods("DELETE") + userPasswordAPI.Use(usersController.GetUserMiddleware) + userPasswordAPI.Path("/password").HandlerFunc(usersController.UpdateUserPassword).Methods("POST") + userPasswordAPI.Path("/2fas/totp").HandlerFunc(usersController.EnableTotp).Methods("POST") + userPasswordAPI.Path("/2fas/totp/{totp_id}/qr").HandlerFunc(usersController.TotpQr).Methods("GET") + userPasswordAPI.Path("/2fas/totp/{totp_id}").HandlerFunc(usersController.DisableTotp).Methods("DELETE") projectGet := authenticatedAPI.Path("/project/{project_id}").Subrouter() projectGet.Use(projects.ProjectMiddleware) diff --git a/api/user_options_test.go b/api/user_options_test.go index 29fd395071..dc5c588480 100644 --- a/api/user_options_test.go +++ b/api/user_options_test.go @@ -130,7 +130,7 @@ func TestDeleteUser_RemovesOptions(t *testing.T) { r = helpers.SetContextValue(r, "_user", target) w := httptest.NewRecorder() - deleteUser(w, r) + NewUsersController(nil).DeleteUser(w, r) assert.Equal(t, http.StatusNoContent, w.Code) diff --git a/api/users.go b/api/users.go index 8e9aab9c76..6f0514cf3e 100644 --- a/api/users.go +++ b/api/users.go @@ -18,11 +18,13 @@ import ( type UsersController struct { subscriptionService pro_interfaces.SubscriptionService + log *log.Entry } func NewUsersController(subscriptionService pro_interfaces.SubscriptionService) *UsersController { return &UsersController{ subscriptionService: subscriptionService, + log: log.WithField("context", "api.users"), } } @@ -67,7 +69,7 @@ func (c *UsersController) AddUser(w http.ResponseWriter, r *http.Request) { editor := helpers.GetFromContext(r, "user").(*db.User) if !editor.Admin { - log.Warn(editor.Username + " is not permitted to create users") + c.log.WithField("editor", editor.Username).Debug("Not permitted to create users") w.WriteHeader(http.StatusUnauthorized) return } @@ -76,6 +78,7 @@ func (c *UsersController) AddUser(w http.ResponseWriter, r *http.Request) { ok, err := c.subscriptionService.CanAddProUser() if err != nil { + c.log.WithError(err).Error("Failed to check Pro user limit") w.WriteHeader(http.StatusInternalServerError) return } @@ -100,14 +103,14 @@ func (c *UsersController) AddUser(w http.ResponseWriter, r *http.Request) { } if err != nil { - log.Warn(editor.Username + " is not created: " + err.Error()) + c.log.WithError(err).WithField("username", user.Username).Error("Failed to create user") w.WriteHeader(http.StatusBadRequest) return } helpers.WriteJSON(w, http.StatusCreated, newUser) } -func readonlyUserMiddleware(next http.Handler) http.Handler { +func (c *UsersController) ReadonlyUserMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { userID, err := helpers.GetIntParam("user_id", w, r) @@ -137,7 +140,7 @@ func readonlyUserMiddleware(next http.Handler) http.Handler { }) } -func getUserMiddleware(next http.Handler) http.Handler { +func (c *UsersController) GetUserMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { userID, err := helpers.GetIntParam("user_id", w, r) @@ -155,7 +158,10 @@ func getUserMiddleware(next http.Handler) http.Handler { editor := helpers.GetFromContext(r, "user").(*db.User) if !editor.Admin && editor.ID != user.ID { - log.Warn(editor.Username + " is not permitted to edit users") + c.log.WithFields(log.Fields{ + "editor": editor.Username, + "user_id": user.ID, + }).Debug("Not permitted to access another user") w.WriteHeader(http.StatusUnauthorized) return } @@ -175,7 +181,10 @@ func (c *UsersController) UpdateUser(w http.ResponseWriter, r *http.Request) { } if !editor.Admin && (user.Pro && !targetUser.Pro) { - log.Warn(editor.Username + " is not permitted to mark users as Pro") + c.log.WithFields(log.Fields{ + "editor": editor.Username, + "user_id": targetUser.ID, + }).Debug("Not permitted to mark users as Pro") w.WriteHeader(http.StatusUnauthorized) return } @@ -184,6 +193,7 @@ func (c *UsersController) UpdateUser(w http.ResponseWriter, r *http.Request) { ok, err := c.subscriptionService.CanAddProUser() if err != nil { + c.log.WithError(err).Error("Failed to check Pro user limit") w.WriteHeader(http.StatusInternalServerError) return } @@ -199,26 +209,29 @@ func (c *UsersController) UpdateUser(w http.ResponseWriter, r *http.Request) { } if !editor.Admin && editor.ID != targetUser.ID { - log.Warn(editor.Username + " is not permitted to edit users") + c.log.WithFields(log.Fields{ + "editor": editor.Username, + "user_id": targetUser.ID, + }).Debug("Not permitted to update another user") w.WriteHeader(http.StatusUnauthorized) return } if editor.ID == targetUser.ID && targetUser.Admin != user.Admin { - log.Warn("User can't edit his own role") + c.log.WithField("editor", editor.Username).Debug("Not permitted to change own admin status") w.WriteHeader(http.StatusUnauthorized) return } if targetUser.External && targetUser.Username != user.Username { - log.Warn("Username is not editable for external users") + c.log.WithField("user_id", targetUser.ID).Debug("Username is not editable for external users") w.WriteHeader(http.StatusBadRequest) return } user.ID = targetUser.ID if err := helpers.Store(r).UpdateUser(user); err != nil { - log.Error(err.Error()) + c.log.WithError(err).WithField("user_id", targetUser.ID).Error("Failed to update user") w.WriteHeader(http.StatusBadRequest) return } @@ -226,7 +239,7 @@ func (c *UsersController) UpdateUser(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } -func updateUserPassword(w http.ResponseWriter, r *http.Request) { +func (c *UsersController) UpdateUserPassword(w http.ResponseWriter, r *http.Request) { user := helpers.GetFromContext(r, "_user").(db.User) editor := helpers.GetFromContext(r, "user").(*db.User) @@ -235,13 +248,16 @@ func updateUserPassword(w http.ResponseWriter, r *http.Request) { } if !editor.Admin && editor.ID != user.ID { - log.Warn(editor.Username + " is not permitted to edit users") + c.log.WithFields(log.Fields{ + "editor": editor.Username, + "user_id": user.ID, + }).Debug("Not permitted to change another user's password") w.WriteHeader(http.StatusUnauthorized) return } if user.External { - log.Warn("Password is not editable for external users") + c.log.WithField("user_id", user.ID).Debug("Password is not editable for external users") w.WriteHeader(http.StatusBadRequest) return } @@ -251,7 +267,7 @@ func updateUserPassword(w http.ResponseWriter, r *http.Request) { } if err := helpers.Store(r).SetUserPassword(user.ID, pwd.Pwd); err != nil { - util.LogWarning(err) + c.log.WithError(err).WithField("user_id", user.ID).Error("Failed to set user password") w.WriteHeader(http.StatusInternalServerError) return } @@ -259,29 +275,33 @@ func updateUserPassword(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNoContent) } -func deleteUser(w http.ResponseWriter, r *http.Request) { +func (c *UsersController) DeleteUser(w http.ResponseWriter, r *http.Request) { user := helpers.GetFromContext(r, "_user").(db.User) editor := helpers.GetFromContext(r, "user").(*db.User) if !editor.Admin && editor.ID != user.ID { - log.Warn(editor.Username + " is not permitted to delete users") + c.log.WithFields(log.Fields{ + "editor": editor.Username, + "user_id": user.ID, + }).Debug("Not permitted to delete another user") w.WriteHeader(http.StatusUnauthorized) return } if err := helpers.Store(r).DeleteUser(user.ID); err != nil { + c.log.WithError(err).WithField("user_id", user.ID).Error("Failed to delete user") w.WriteHeader(http.StatusInternalServerError) return } if err := helpers.Store(r).DeleteOptions(fmt.Sprintf("user%d", user.ID)); err != nil { - log.WithError(err).Warn("can not delete options of removed user") + c.log.WithError(err).WithField("user_id", user.ID).Error("Failed to delete options of removed user") } w.WriteHeader(http.StatusNoContent) } -func totpQr(w http.ResponseWriter, r *http.Request) { +func (c *UsersController) TotpQr(w http.ResponseWriter, r *http.Request) { user := helpers.GetFromContext(r, "_user").(db.User) if user.Totp == nil { @@ -313,7 +333,7 @@ func totpQr(w http.ResponseWriter, r *http.Request) { _, err = w.Write(pngBytes) } -func enableTotp(w http.ResponseWriter, r *http.Request) { +func (c *UsersController) EnableTotp(w http.ResponseWriter, r *http.Request) { user := helpers.GetFromContext(r, "_user").(db.User) if !util.Config.Mfa.Totp.Enabled { @@ -332,6 +352,7 @@ func enableTotp(w http.ResponseWriter, r *http.Request) { }) if err != nil { + c.log.WithError(err).WithFields(log.Fields{"user_id": user.ID}).Error("Failed to generate TOTP key") http.Error(w, "Error generating key", http.StatusInternalServerError) return } @@ -357,7 +378,7 @@ func enableTotp(w http.ResponseWriter, r *http.Request) { helpers.WriteJSON(w, http.StatusOK, newTotp) } -func disableTotp(w http.ResponseWriter, r *http.Request) { +func (c *UsersController) DisableTotp(w http.ResponseWriter, r *http.Request) { user := helpers.GetFromContext(r, "_user").(db.User) if user.Totp == nil { helpers.WriteErrorStatus(w, "TOTP not enabled", http.StatusBadRequest) diff --git a/db/Repository.go b/db/Repository.go index 9d41885e48..f34cc87d6c 100644 --- a/db/Repository.go +++ b/db/Repository.go @@ -144,6 +144,10 @@ func (r Repository) Validate() error { 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 --git a/db/Role.go b/db/Role.go index a322acc42c..874394a3e8 100644 --- a/db/Role.go +++ b/db/Role.go @@ -11,6 +11,14 @@ func ValidateRole(role Role) error { 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 --git a/db/Role_test.go b/db/Role_test.go new file mode 100644 index 0000000000..9ea68553ef --- /dev/null +++ b/db/Role_test.go @@ -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 --git a/db/Task.go b/db/Task.go index 2ccd2c9073..1e96df702a 100644 --- a/db/Task.go +++ b/db/Task.go @@ -27,13 +27,14 @@ type TerraformTaskParams struct { } type AnsibleTaskParams struct { - Debug bool `json:"debug"` - DebugLevel int `json:"debug_level"` - DryRun bool `json:"dry_run"` - Diff bool `json:"diff"` - Limit []string `json:"limit"` - Tags []string `json:"tags"` - SkipTags []string `json:"skip_tags"` + Debug bool `json:"debug"` + DebugLevel int `json:"debug_level"` + DryRun bool `json:"dry_run"` + Diff bool `json:"diff"` + Limit []string `json:"limit"` + Tags []string `json:"tags"` + SkipTags []string `json:"skip_tags"` + SkipGalaxyInstall bool `json:"skip_galaxy_install"` } // Task is a model of a task which will be executed by the runner @@ -168,6 +169,10 @@ func (task *Task) ValidateNewTask(template Template) error { } } + if err := ValidatePlaybookPath(task.Playbook, "task"); err != nil { + return err + } + var params any switch template.App { case AppAnsible: diff --git a/db/Template.go b/db/Template.go index d2444b5efc..d2844c3fc8 100644 --- a/db/Template.go +++ b/db/Template.go @@ -76,6 +76,13 @@ type AnsibleTemplateParams struct { Limit []string `json:"limit"` Tags []string `json:"tags"` SkipTags []string `json:"skip_tags"` + + // SkipGalaxyInstall skips the Galaxy install step (role and collection + // requirements) before running the playbook. + SkipGalaxyInstall bool `json:"skip_galaxy_install"` + // AllowOverrideSkipGalaxyInstall lets the user toggle SkipGalaxyInstall when + // launching a task. + AllowOverrideSkipGalaxyInstall bool `json:"allow_override_skip_galaxy_install"` } type TerraformTemplateParams struct { @@ -220,6 +227,10 @@ func (tpl *Template) Validate() error { 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 --git a/db/git_url.go b/db/git_url.go new file mode 100644 index 0000000000..c571b59a42 --- /dev/null +++ b/db/git_url.go @@ -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 --git a/db/git_url_test.go b/db/git_url_test.go new file mode 100644 index 0000000000..96a6925c99 --- /dev/null +++ b/db/git_url_test.go @@ -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://git@example.com/repo.git", false}, + {"scp-like ssh", "git@github.com: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 --git a/db/playbook_path.go b/db/playbook_path.go new file mode 100644 index 0000000000..31197caa14 --- /dev/null +++ b/db/playbook_path.go @@ -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 --git a/db/playbook_path_test.go b/db/playbook_path_test.go new file mode 100644 index 0000000000..ff52117eaf --- /dev/null +++ b/db/playbook_path_test.go @@ -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 --git a/db/sql/access_key.go b/db/sql/access_key.go index a8efdab386..be604d82e6 100644 --- a/db/sql/access_key.go +++ b/db/sql/access_key.go @@ -34,6 +34,8 @@ func (d *SqlDb) GetAccessKeys(projectID int, options db.GetAccessKeyOptions, par 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 --git a/db/sql/role.go b/db/sql/role.go index cefe760891..93a08db5f9 100644 --- a/db/sql/role.go +++ b/db/sql/role.go @@ -58,6 +58,10 @@ func (d *SqlDb) GetProjectRole(projectID int, slug string) (db.Role, error) { 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 --git a/db/sql/template.go b/db/sql/template.go index 067054dc4e..5d3f36fbd2 100644 --- a/db/sql/template.go +++ b/db/sql/template.go @@ -494,22 +494,31 @@ func (d *SqlDb) GetTemplatePermission(projectID int, templateID int, userID int) 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 err != nil { - return + if errors.Is(err, db.ErrNotFound) { + 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 --git a/db_lib/AnsibleApp.go b/db_lib/AnsibleApp.go index bd38c0776d..52b9f8ec96 100644 --- a/db_lib/AnsibleApp.go +++ b/db_lib/AnsibleApp.go @@ -75,6 +75,11 @@ func (t *AnsibleApp) Clear() { } func (t *AnsibleApp) InstallRequirements(args LocalAppInstallingArgs) error { + if t.skipGalaxyInstall(args) { + t.Log("Galaxy install step is skipped.\n") + return nil + } + if err := t.installCollectionsRequirements(args.EnvironmentVars); err != nil { return err } @@ -84,6 +89,26 @@ func (t *AnsibleApp) InstallRequirements(args LocalAppInstallingArgs) error { return nil } +// skipGalaxyInstall reports whether the Galaxy install step must be skipped. +// The template-level flag provides the default; when the template allows +// overriding it, the task-level flag takes precedence. +func (t *AnsibleApp) skipGalaxyInstall(args LocalAppInstallingArgs) bool { + tplParams, ok := args.TplParams.(*db.AnsibleTemplateParams) + if !ok || tplParams == nil { + return false + } + + skip := tplParams.SkipGalaxyInstall + + if tplParams.AllowOverrideSkipGalaxyInstall { + if params, ok := args.Params.(*db.AnsibleTaskParams); ok && params != nil { + skip = params.SkipGalaxyInstall + } + } + + return skip +} + func (t *AnsibleApp) getRepoPath() string { return t.Repository.GetFullPath(t.Template.ID) } diff --git a/db_lib/AnsibleApp_test.go b/db_lib/AnsibleApp_test.go new file mode 100644 index 0000000000..00f169fccf --- /dev/null +++ b/db_lib/AnsibleApp_test.go @@ -0,0 +1,79 @@ +package db_lib + +import ( + "testing" + + "github.com/semaphoreui/semaphore/db" + "github.com/stretchr/testify/assert" +) + +func TestAnsibleApp_skipGalaxyInstall(t *testing.T) { + tests := []struct { + name string + tpl *db.AnsibleTemplateParams + params *db.AnsibleTaskParams + expected bool + }{ + { + name: "no template params", + tpl: nil, + params: &db.AnsibleTaskParams{SkipGalaxyInstall: true}, + expected: false, + }, + { + name: "template skip enabled, override disabled", + tpl: &db.AnsibleTemplateParams{SkipGalaxyInstall: true}, + params: &db.AnsibleTaskParams{SkipGalaxyInstall: false}, + expected: true, + }, + { + name: "template skip disabled, override disabled, task wants skip", + tpl: &db.AnsibleTemplateParams{SkipGalaxyInstall: false}, + params: &db.AnsibleTaskParams{SkipGalaxyInstall: true}, + expected: false, + }, + { + name: "override enabled, task disables skip", + tpl: &db.AnsibleTemplateParams{ + SkipGalaxyInstall: true, + AllowOverrideSkipGalaxyInstall: true, + }, + params: &db.AnsibleTaskParams{SkipGalaxyInstall: false}, + expected: false, + }, + { + name: "override enabled, task enables skip", + tpl: &db.AnsibleTemplateParams{ + SkipGalaxyInstall: false, + AllowOverrideSkipGalaxyInstall: true, + }, + params: &db.AnsibleTaskParams{SkipGalaxyInstall: true}, + expected: true, + }, + { + name: "override enabled, nil task params falls back to template", + tpl: &db.AnsibleTemplateParams{ + SkipGalaxyInstall: true, + AllowOverrideSkipGalaxyInstall: true, + }, + params: nil, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + app := &AnsibleApp{} + + args := LocalAppInstallingArgs{} + if tt.tpl != nil { + args.TplParams = tt.tpl + } + if tt.params != nil { + args.Params = tt.params + } + + assert.Equal(t, tt.expected, app.skipGalaxyInstall(args)) + }) + } +} diff --git a/db_lib/CmdGitClient.go b/db_lib/CmdGitClient.go index 0bd7d6760e..aed436b92e 100644 --- a/db_lib/CmdGitClient.go +++ b/db_lib/CmdGitClient.go @@ -115,6 +115,7 @@ func (c CmdGitClient) Clone(r GitRepository) error { "--recursive", "--branch", r.Repository.GitBranch, + "--end-of-options", r.Repository.GetGitURL(false), dirName) } @@ -122,7 +123,7 @@ func (c CmdGitClient) Clone(r GitRepository) error { 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) GetLastCommitHash(r GitRepository) (hash string, err error } 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) GetLastRemoteCommitHash(r GitRepository) (hash string, err } 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 --git a/db_lib/CmdGitClient_injection_test.go b/db_lib/CmdGitClient_injection_test.go new file mode 100644 index 0000000000..3fcaad9aa7 --- /dev/null +++ b/db_lib/CmdGitClient_injection_test.go @@ -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 --git a/deployment/docker/runner/Dockerfile b/deployment/docker/runner/Dockerfile index ea14a20d19..8fd48625c0 100644 --- a/deployment/docker/runner/Dockerfile +++ b/deployment/docker/runner/Dockerfile @@ -19,7 +19,7 @@ ARG TARGETARCH 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 --git a/deployment/docker/server/Dockerfile b/deployment/docker/server/Dockerfile index fe6db76cd7..df11043553 100644 --- a/deployment/docker/server/Dockerfile +++ b/deployment/docker/server/Dockerfile @@ -19,7 +19,7 @@ ARG TARGETARCH 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 --git a/deployment/docker/server/Dockerfile.pkce b/deployment/docker/server/Dockerfile.pkce new file mode 100644 index 0000000000..a2cb4a9500 --- /dev/null +++ b/deployment/docker/server/Dockerfile.pkce @@ -0,0 +1,52 @@ +# Patched SemaphoreUI: v2.18.25 (commit 7c3789c) + PKCE for the OIDC login flow. +# +# Built as an OVERLAY on the exact pinned upstream image rather than a rebuild of +# it: the pinned digest is the `v2.18.25-ansible2.16.5` variant, whose ansible +# 9.4.0 / community.general 8.5.0 pip bundle is what lets the container talk to +# 1Password Connect without the `op` binary (see Node/AWS SAM/SemaphoreUI/template.yaml). +# Reproducing that venv from the upstream Dockerfile would drift; replacing only +# /usr/local/bin/semaphore keeps it byte-identical. +# +# Builder stage is copied verbatim from deployment/docker/server/Dockerfile so the +# frontend/backend toolchain matches how upstream builds the release binary. + +FROM --platform=$BUILDPLATFORM golang:1.24-alpine3.21 AS builder + +RUN apk add --no-cache -U \ + libc-dev curl nodejs npm git gcc zip unzip tar + +WORKDIR /usr/local +# hadolint ignore=DL4006 +RUN curl -sL https://taskfile.dev/install.sh | sh + +WORKDIR /go/src/semaphore +COPY . /go/src/semaphore + +RUN go mod download -x + +# deps:tools is skipped on purpose (it only installs goreleaser, unused here). +# deps:be vendors modules; deps:fe + build:fe generate api/public/* for the +# go:embed in api/router.go, which fails the build if the assets are absent. +RUN task deps:be deps:fe && task build:fe + +# build:be equivalent, with the version string marked so the running container is +# identifiably ours (`semaphore version`) instead of masquerading as stock v2.18.25. +ARG VER=v2.18.25-pkce1 +ARG SHA=7c3789c +ARG DATE=1787000000 +RUN env CGO_ENABLED=0 GOOS=linux GOARCH=amd64 \ + go build -o bin/semaphore -tags "netgo" \ + -ldflags "-s -w \ + -X github.com/semaphoreui/semaphore/util.Ver=${VER} \ + -X github.com/semaphoreui/semaphore/util.Commit=${SHA} \ + -X github.com/semaphoreui/semaphore/util.Date=${DATE}" \ + ./cli + +FROM public.ecr.aws/semaphore/server@sha256:25212fc1d2fea5a7f85aae6f9ad62bb5d1e8ae50265ff41c74e46716bd01d150 + +# Mirrors the upstream Dockerfile's ownership/permissions for this binary. +USER root +COPY --from=builder /go/src/semaphore/bin/semaphore /usr/local/bin/semaphore +RUN chown -R semaphore:0 /usr/local/bin/semaphore && \ + chmod +x /usr/local/bin/semaphore +USER 1001 diff --git a/pro_interfaces/featues.go b/pro_interfaces/featues.go index 1defae37d9..d7da7ca1a2 100644 --- a/pro_interfaces/featues.go +++ b/pro_interfaces/featues.go @@ -1,11 +1,12 @@ package pro_interfaces type Features struct { - ProjectRunners bool `json:"project_runners"` - TerraformBackend bool `json:"terraform_backend"` - TaskSummary bool `json:"task_summary"` - SecretStorages bool `json:"secret_storages"` - SecretStorageManagement bool `json:"secret_storage_management"` - CustomRolesManagement bool `json:"custom_roles_management"` - HighAvailability bool `json:"high_availability"` + ProjectRunners bool `json:"project_runners"` + TerraformBackend bool `json:"terraform_backend"` + TaskSummary bool `json:"task_summary"` + SecretStorages bool `json:"secret_storages"` + SecretStorageManagement bool `json:"secret_storage_management"` + SecretStorageManagementEx bool `json:"secret_storage_management_ex"` + CustomRolesManagement bool `json:"custom_roles_management"` + HighAvailability bool `json:"high_availability"` } diff --git a/services/runners/job_pool.go b/services/runners/job_pool.go index c6a685ab4a..8c568bad48 100644 --- a/services/runners/job_pool.go +++ b/services/runners/job_pool.go @@ -91,6 +91,12 @@ type JobPool struct { processing int32 keyInstaller db_lib.AccessKeyInstaller + + // client is the shared HTTP client for all runner→server requests. It is + // created once so the transport's keep-alive pool reuses connections — + // creating a client per request leaks one ESTABLISHED connection per poll + // cycle (~2/sec) until the runner exhausts ephemeral ports (issue #3941). + client *http.Client } func NewJobPool(keyInstaller db_lib.AccessKeyInstaller) *JobPool { @@ -99,6 +105,7 @@ func NewJobPool(keyInstaller db_lib.AccessKeyInstaller) *JobPool { queue: make([]*job, 0), processing: 0, keyInstaller: keyInstaller, + client: newHTTPClient(), } } @@ -140,8 +147,6 @@ func (p *JobPool) Unregister() (err error) { return fmt.Errorf("runner is not registered") } - client := newHTTPClient() - url := util.Config.WebHost + "/api/internal/runners" req, err := http.NewRequest("DELETE", url, nil) @@ -149,10 +154,11 @@ func (p *JobPool) Unregister() (err error) { return } - resp, err := client.Do(req) + resp, err := p.client.Do(req) if err != nil { return } + defer resp.Body.Close() //nolint:errcheck if resp.StatusCode >= 400 && resp.StatusCode != 404 { err = fmt.Errorf("encountered error while unregistering runner; server returned code %d", resp.StatusCode) @@ -277,8 +283,6 @@ func (p *JobPool) sendProgress() (ok bool) { logger := JobLogger{Context: "sending_progress"} - client := newHTTPClient() - url := util.Config.WebHost + "/api/internal/runners" body := RunnerProgress{ @@ -310,7 +314,7 @@ func (p *JobPool) sendProgress() (ok bool) { req.Header.Set("X-Runner-Token", util.Config.Runner.Token) - resp, err := client.Do(req) + resp, err := p.client.Do(req) if err != nil { logger.ActionError(err, "send request", "the server returned error") return @@ -391,8 +395,6 @@ func (p *JobPool) tryRegisterRunner(configFilePath *string) (ok bool) { return } - client := newHTTPClient() - url := util.Config.WebHost + "/api/internal/runners" jsonBytes, err := json.Marshal(RunnerRegistration{ @@ -417,13 +419,15 @@ func (p *JobPool) tryRegisterRunner(configFilePath *string) (ok bool) { return } - resp, err := client.Do(req) + resp, err := p.client.Do(req) if err != nil { logger.ActionError(err, "send request", "unexpected error") return } + defer resp.Body.Close() //nolint:errcheck + if resp.StatusCode != 200 { logger.ActionError(fmt.Errorf("invalid status code"), "send request", p.getResponseErrorMessage(resp)) return @@ -481,8 +485,6 @@ func (p *JobPool) tryRegisterRunner(configFilePath *string) (ok bool) { } } - defer resp.Body.Close() //nolint:errcheck - ok = true return } @@ -546,8 +548,6 @@ func (p *JobPool) checkNewJobs() { return } - client := newHTTPClient() - url := util.Config.WebHost + "/api/internal/runners" req, err := http.NewRequest("GET", url, nil) @@ -559,7 +559,9 @@ func (p *JobPool) checkNewJobs() { req.Header.Set("X-Runner-Token", util.Config.Runner.Token) - resp, err := client.Do(req) + resp, err := p.client.Do(req) + + defer resp.Body.Close() //nolint:errcheck if err != nil { logger.ActionError(err, "send request", "unexpected error") @@ -572,8 +574,6 @@ func (p *JobPool) checkNewJobs() { return } - defer resp.Body.Close() //nolint:errcheck - body, err := io.ReadAll(resp.Body) if err != nil { logger.ActionError(err, "read response body", "can not read server's response body") diff --git a/services/tasks/LocalJob.go b/services/tasks/LocalJob.go index fd479cdc92..a0152d5514 100644 --- a/services/tasks/LocalJob.go +++ b/services/tasks/LocalJob.go @@ -39,6 +39,28 @@ type LocalJob struct { 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 @@ func (t *LocalJob) Run(username string, incomingVersion *string, alias string) ( 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 @@ func (t *LocalJob) prepareRun(installingArgs db_lib.LocalAppInstallingArgs) erro } } - // 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 @@ func (t *LocalJob) prepareRunTerraform(tfApp *db_lib.TerraformApp, installingArg } } - // 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) diff --git a/web/public/swagger/api-docs.yml b/web/public/swagger/api-docs.yml index cca9f82f18..5aad742cc0 100644 --- a/web/public/swagger/api-docs.yml +++ b/web/public/swagger/api-docs.yml @@ -843,6 +843,8 @@ definitions: type: array items: type: string + skip_galaxy_install: + type: boolean TerraformTaskParams: type: object diff --git a/web/src/components/EnvironmentForm.vue b/web/src/components/EnvironmentForm.vue index 03fdc36cf3..f34bf1c5ef 100644 --- a/web/src/components/EnvironmentForm.vue +++ b/web/src/components/EnvironmentForm.vue @@ -269,7 +269,7 @@ -
+
{{ $t('extraVariables') }} @@ -288,9 +288,12 @@ - - Passing secrets using this method is not secure. This feature will be removed in version - 2.19. + + Secrets passed this way may appear in plain text in Ansible logs. {{ formError }} - - - - {{ $t('upgrade_to_pro') }} - - - + + + + +
@@ -98,6 +109,7 @@ const APP_PARAMS = { 'tags', 'skip_tags', 'limit', + 'skip_galaxy_install', ], }; diff --git a/web/src/components/TemplateForm.vue b/web/src/components/TemplateForm.vue index 78c42b424e..41d82d719b 100644 --- a/web/src/components/TemplateForm.vue +++ b/web/src/components/TemplateForm.vue @@ -435,6 +435,13 @@ @change="setTemplateVaults" > + + + + Hashicorp Vault - - - $vuetify.icons.aws_sm - - AWS Secrets Manager - + + + $vuetify.icons.aws_sm + + AWS Secrets Manager + - - - $vuetify.icons.azure_kv - - Azure Key Vault - + + + $vuetify.icons.azure_kv + + Azure Key Vault + - - - $vuetify.icons.dvls - - Devolutions Server - + + + $vuetify.icons.dvls + + Devolutions Server + + + +
+ Enterprise + mdi-arrow-right +
+
+
@@ -195,7 +222,41 @@ - +