Skip to content

Commit

Permalink
Merge pull request from GHSA-g623-jcgg-mhmm
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
  • Loading branch information
crenshaw-dev committed Mar 13, 2024
1 parent 479b554 commit 3b8f673
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
9 changes: 9 additions & 0 deletions server/application/application.go
Expand Up @@ -333,6 +333,15 @@ func (s *Server) Create(ctx context.Context, q *application.ApplicationCreateReq
return nil, security.NamespaceNotPermittedError(appNs)
}

// Don't let the app creator set the operation explicitly. Those requests should always go through the Sync API.
if a.Operation != nil {
log.WithFields(log.Fields{
"application": a.Name,
argocommon.SecurityField: argocommon.SecurityLow,
}).Warn("User attempted to set operation on application creation. This could have allowed them to bypass branch protection rules by setting manifests directly. Ignoring the set operation.")
a.Operation = nil
}

created, err := s.appclientset.ArgoprojV1alpha1().Applications(appNs).Create(ctx, a, metav1.CreateOptions{})
if err == nil {
s.logAppEvent(created, ctx, argo.EventReasonResourceCreated, "created application")
Expand Down
21 changes: 21 additions & 0 deletions server/application/application_test.go
Expand Up @@ -1439,6 +1439,27 @@ func TestCreateAppWithDestName(t *testing.T) {
assert.Equal(t, app.Spec.Destination.Server, "https://cluster-api.example.com")
}

// TestCreateAppWithOperation tests that an application created with an operation is created with the operation removed.
// Avoids regressions of https://github.com/argoproj/argo-cd/security/advisories/GHSA-g623-jcgg-mhmm
func TestCreateAppWithOperation(t *testing.T) {
appServer := newTestAppServer(t)
testApp := newTestAppWithDestName()
testApp.Operation = &appsv1.Operation{
Sync: &appsv1.SyncOperation{
Manifests: []string{
"test",
},
},
}
createReq := application.ApplicationCreateRequest{
Application: testApp,
}
app, err := appServer.Create(context.Background(), &createReq)
require.NoError(t, err)
require.NotNil(t, app)
assert.Nil(t, app.Operation)
}

func TestUpdateApp(t *testing.T) {
testApp := newTestApp()
appServer := newTestAppServer(t, testApp)
Expand Down

0 comments on commit 3b8f673

Please sign in to comment.