Skip to content

Pass in enabled mTLS spec when a cert is provided #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
david049 wants to merge 9 commits into from
Closed

Pass in enabled mTLS spec when a cert is provided #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f8b6b2f
Pass in enabled mTLS spec when a cert is provided
david049 Jun 22, 2026
adf5848
missed a spot
david049 Jun 22, 2026
4d99c33
one more
david049 Jun 22, 2026
3d61e3d
fix test
david049 Jun 22, 2026
93abd77
one more fix
david049 Jun 22, 2026
2a66283
fix test
david049 Jun 22, 2026
9612dd7
test
david049 Jun 22, 2026
a1a1098
gen a cert
david049 Jun 22, 2026
72e041b
empty
david049 Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/namespace/cert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func (c *Client) AddCACerts(ctx context.Context, params AddCACertsParams) (*oper
spec := ns.GetSpec()
// Ensure MtlsAuth is initialized before accessing its fields
if spec.MtlsAuth == nil {
spec.MtlsAuth = &namespacev1.MtlsAuthSpec{}
spec.MtlsAuth = &namespacev1.MtlsAuthSpec{Enabled: true}

@cursor cursor Bot Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabled unset on existing MtlsAuth

High Severity

In AddCACerts, MtlsAuth.Enabled is set to true only when spec.MtlsAuth is nil. When the namespace already has an MtlsAuth block (the usual add-cert path), only AcceptedClientCa is updated, so Enabled stays false and mTLS can remain off despite CA changes.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit adf5848. Configure here.

@cursor cursor Bot Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing mTLS stays disabled

High Severity

AddCACerts and AddCertFilters only set MtlsAuth Enabled when the spec pointer was nil. If the namespace already has a non-nil MtlsAuth with Enabled false (e.g. after older create paths that supplied CAs without enabling), adding CAs or filters updates those fields but leaves mTLS disabled, so the PR’s intent is not applied on update.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 93abd77. Configure here.

@david049 david049 Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there ever a case where we add certs/filters and we don't have it enabled?

@cursor cursor Bot Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mTLS stays disabled adding certs

Medium Severity

In DeleteCertFilters, when spec.MtlsAuth is nil the code creates MtlsAuth with Enabled: true before assigning the filter list. With no existing filters that yields an update that turns mTLS on with no CAs and no filters.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a1a1098. Configure here.

}
spec.MtlsAuth.AcceptedClientCa = bundleBytes

Expand Down Expand Up @@ -110,7 +110,7 @@ func (c *Client) DeleteCACerts(ctx context.Context, params DeleteCACertsParams)
} else {
// Ensure MtlsAuth is initialized before accessing its fields
if spec.MtlsAuth == nil {
spec.MtlsAuth = &namespacev1.MtlsAuthSpec{}
spec.MtlsAuth = &namespacev1.MtlsAuthSpec{Enabled: true}
}
spec.MtlsAuth.AcceptedClientCa = bundleBytes
}
Expand Down
4 changes: 2 additions & 2 deletions internal/namespace/cert_filter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func (c *Client) AddCertFilters(ctx context.Context, params AddCertFiltersParams
spec := ns.GetSpec()
// Ensure MtlsAuth is initialized
if spec.MtlsAuth == nil {
spec.MtlsAuth = &namespacev1.MtlsAuthSpec{}
spec.MtlsAuth = &namespacev1.MtlsAuthSpec{Enabled: true}

@cursor cursor Bot Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mTLS not enabled on add

Medium Severity

This PR turns on MtlsAuth.Enabled when creating a namespace with certs or filters, but AddCertFilters and AddCACerts only set Enabled: true when MtlsAuth was nil. If the namespace already has an MtlsAuth block with Enabled: false, adding filters or CAs updates the spec without enabling mTLS.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9612dd7. Configure here.

}

existingFilters := spec.MtlsAuth.GetCertificateFilters()
Expand Down Expand Up @@ -101,7 +101,7 @@ func (c *Client) DeleteCertFilters(ctx context.Context, params DeleteCertFilters

// Update the spec with the new filter list
if spec.MtlsAuth == nil {
spec.MtlsAuth = &namespacev1.MtlsAuthSpec{}
spec.MtlsAuth = &namespacev1.MtlsAuthSpec{Enabled: true}

@cursor cursor Bot Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete filters enables empty mTLS

Low Severity

In DeleteCertFilters, when MtlsAuth was nil the code now creates MtlsAuthSpec{Enabled: true} and pushes an update with empty CertificateFilters. A delete on a namespace with no mTLS config can turn mTLS on without CA material, which this change introduces via Enabled: true on that init path.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 93abd77. Configure here.

@david049 david049 Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there ever a case where we have cert filters enabled, we delete it, and the namespace has enabled:false on mtls?

}
spec.MtlsAuth.CertificateFilters = newFilters

Expand Down
6 changes: 6 additions & 0 deletions internal/namespace/cert_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func TestAddCACerts_Success(t *testing.T) {
ResourceVersion: "v1",
Spec: &namespacev1.NamespaceSpec{
MtlsAuth: &namespacev1.MtlsAuthSpec{
Enabled: true,
AcceptedClientCa: existingCertPEM,
},
},
Expand Down Expand Up @@ -67,6 +68,7 @@ func TestAddCACerts_Success(t *testing.T) {
AsyncOperationId: "test-async-op",
Spec: &namespacev1.NamespaceSpec{
MtlsAuth: &namespacev1.MtlsAuthSpec{
Enabled: true,
AcceptedClientCa: expectedCertBundle,
},
},
Expand Down Expand Up @@ -104,6 +106,7 @@ func TestAddCACerts_DuplicateCertificate(t *testing.T) {
Spec: &namespacev1.NamespaceSpec{
MtlsAuth: &namespacev1.MtlsAuthSpec{
AcceptedClientCa: existingCertPEM,
Enabled: true,
},
},
}
Expand All @@ -123,6 +126,7 @@ func TestAddCACerts_DuplicateCertificate(t *testing.T) {
ResourceVersion: "v1",
Spec: &namespacev1.NamespaceSpec{
MtlsAuth: &namespacev1.MtlsAuthSpec{
Enabled: true,
AcceptedClientCa: existingCertPEM,
},
},
Expand Down Expand Up @@ -221,6 +225,7 @@ func TestAddCACerts_CustomResourceVersion(t *testing.T) {
Spec: &namespacev1.NamespaceSpec{
MtlsAuth: &namespacev1.MtlsAuthSpec{
AcceptedClientCa: existingCertPEM,
Enabled: true,
},
},
}
Expand All @@ -246,6 +251,7 @@ func TestAddCACerts_CustomResourceVersion(t *testing.T) {
ResourceVersion: "custom-version",
Spec: &namespacev1.NamespaceSpec{
MtlsAuth: &namespacev1.MtlsAuthSpec{
Enabled: true,
AcceptedClientCa: expectedCertBundle,
},
},
Expand Down
1 change: 1 addition & 0 deletions temporalcloudcli/commands.namespace.create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ func TestCreateNamespace_BuildsSpec(t *testing.T) {
Lifecycle: &namespacev1.LifecycleSpec{EnableDeleteProtection: false},
Fairness: &namespacev1.FairnessSpec{TaskQueueFairnessEnabled: true},
MtlsAuth: &namespacev1.MtlsAuthSpec{
Enabled: true,
CertificateFilters: []*namespacev1.CertificateFilterSpec{
{CommonName: "test.temporal.io"},
{SubjectAlternativeName: "*.temporal.io"},
Expand Down
3 changes: 2 additions & 1 deletion temporalcloudcli/commands.namespace.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,12 +396,13 @@ func CreateNamespace(ctx context.Context, params CreateNamespaceParams) error {
}

if len(certBytes) > 0 {
spec.MtlsAuth = &namespacev1.MtlsAuthSpec{AcceptedClientCa: certBytes}
spec.MtlsAuth = &namespacev1.MtlsAuthSpec{AcceptedClientCa: certBytes, Enabled: true}
}
if len(certFilters) > 0 {
if spec.MtlsAuth == nil {
spec.MtlsAuth = &namespacev1.MtlsAuthSpec{}
}
spec.MtlsAuth.Enabled = true
spec.MtlsAuth.CertificateFilters = certFilters
}
if params.CodecEndpoint != "" {
Expand Down
42 changes: 39 additions & 3 deletions temporalcloudcli/commands.namespace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,19 @@
package temporalcloudcli_test

import (
"crypto/rand"
"crypto/rsa"
"crypto/x509"
"crypto/x509/pkix"
"encoding/base64"
"encoding/json"
"encoding/pem"
"fmt"
"io"
"math/big"
"strings"
"testing"
"time"

"github.com/temporalio/cloud-cli/temporalcloudcli"
"go.temporal.io/api/temporalproto"
Expand All @@ -31,11 +40,36 @@ func (s *SharedServerSuite) TestBasicNamespaceOperations() {
s.cleanupNamespaces()
}

// generateTestCACertBase64 generates a self-signed CA certificate and returns it as a base64-encoded PEM string.
func generateTestCACertBase64(t *testing.T) string {
t.Helper()
privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
if err != nil {
t.Fatalf("failed to generate key: %v", err)
}
template := x509.Certificate{
SerialNumber: big.NewInt(1),
Subject: pkix.Name{CommonName: "test.temporal.io"},
NotBefore: time.Now(),
NotAfter: time.Now().Add(time.Hour),
KeyUsage: x509.KeyUsageCertSign,
BasicConstraintsValid: true,
IsCA: true,
}
certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &privateKey.PublicKey, privateKey)
if err != nil {
t.Fatalf("failed to create certificate: %v", err)
}
certPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certDER})
return base64.StdEncoding.EncodeToString(certPEM)
}

func (s *SharedServerSuite) TestNamespaceCreate() {
s.cleanupNamespaces()
defer s.cleanupNamespaces()

namespaceName := s.generateRandomNamespaceName()
caCertBase64 := generateTestCACertBase64(s.Suite.T())

res := s.Execute(
"namespace",
Expand All @@ -45,7 +79,7 @@ func (s *SharedServerSuite) TestNamespaceCreate() {
"--name", namespaceName,
"--region", "aws-ca-central-1",
"--retention-days", "30",
"--api-key-auth-enabled",
Comment thread
cursor[bot] marked this conversation as resolved.
"--ca-certificate", caCertBase64,
"--search-attribute", "MyText=Text",
"--search-attribute", "MyKeyword=Keyword",
"--certificate-filter", `{"commonName":"test.temporal.io","organization":"Temporal"}`,
Expand Down Expand Up @@ -78,13 +112,13 @@ func (s *SharedServerSuite) TestNamespaceCreate() {
s.Suite.Equal(namespaceName, gotSpec.Name)
s.Suite.Equal("aws-ca-central-1", getNsRes.Namespace.ActiveRegion)
s.Suite.Equal(int32(30), gotSpec.RetentionDays)
s.Suite.Require().NotNil(gotSpec.ApiKeyAuth)
s.Suite.True(gotSpec.ApiKeyAuth.Enabled)

s.Suite.Equal(namespace.NamespaceSpec_SEARCH_ATTRIBUTE_TYPE_TEXT, gotSpec.SearchAttributes["MyText"])
s.Suite.Equal(namespace.NamespaceSpec_SEARCH_ATTRIBUTE_TYPE_KEYWORD, gotSpec.SearchAttributes["MyKeyword"])

s.Suite.Require().NotNil(gotSpec.MtlsAuth)
s.Suite.True(gotSpec.MtlsAuth.Enabled)
s.Suite.NotEmpty(gotSpec.MtlsAuth.AcceptedClientCa)
s.Suite.Require().Len(gotSpec.MtlsAuth.CertificateFilters, 2)
s.Suite.Equal("test.temporal.io", gotSpec.MtlsAuth.CertificateFilters[0].CommonName)
s.Suite.Equal("Temporal", gotSpec.MtlsAuth.CertificateFilters[0].Organization)
Expand Down Expand Up @@ -159,6 +193,8 @@ func (s *SharedServerSuite) testnamespaceCRUD() {
s.Suite.Equal(namespaceSpec.Regions[0], readNamespace.ActiveRegion)
s.Suite.Equal(namespaceSpec.SearchAttributes, readNamespace.Spec.SearchAttributes)
s.Suite.Equal(namespaceSpec.RetentionDays, readNamespace.Spec.RetentionDays)
s.Suite.Require().NotNil(readNamespace.Spec.ApiKeyAuth)
s.Suite.Equal(namespaceSpec.ApiKeyAuth.Enabled, readNamespace.Spec.ApiKeyAuth.Enabled)

// get the namespace via listing
res = s.Execute(
Expand Down
Loading