Repository: nats-io/nsc (go)

Issue:
No way to disable Jetstream from system account?
### What version were you using?

2.10.23

### What environment was the server running in?

Kubernetes

### Is this defect reproducible?

Yes

### Given the capability you are leveraging, describe your expectation?

Either not allow user to enable Jetstream in system account or allow Jetstream-enabled system account to disable it

### Given the expectation, what is the defect you are observing?

1. Without knowing system account shouldn't have Jetstream enabled, I enabled Jetstream
```
➜  nsc edit account SYS --js-enable=0 
[ OK ] enabled global limit
[ OK ] edited account "SYS"
```
2. Once deployed the change, I see error message which makes me realize I shouldn't have enabled Jetstream:
```
[87] 2024/12/21 01:02:34.571811 [FTL] Not allowed to enable JetStream on the system account
```
3. Then I thought this is how to disable Jetstream, but turns out it isn't
```
➜  nsc edit account SYS --js-enable=1       
[ OK ] enabled tier limit R1
[ OK ] edited account "SYS"
```
4. Then I tried to use `--js-disable` to disable Jetstream, but not able to:
```
➜  nsc edit account SYS --js-disable
Error: system account cannot have JetStream limits - please rerun with --js-disable
➜  nsc edit account SYS --js-disable
Error: system account cannot have JetStream limits - please rerun with --js-disable
```
5. Figured maybe I should remove the tier, but not able to:
```
➜  nsc edit account --rm-js-tier 0
Error: system account cannot have JetStream limits - please rerun with --js-disable
```

Pull request description:
various fixes regarding enable/disable/validation of Jetstream in the system account
- [FIX] added check for --js-enable=tier on system account

- [FIX] added bypass to allow --js-disable to work on system account in cases where it was enabled by specifying --js-enable=tier 

- [FIX] enhanced validation to check for system accounts with enabled jetstream

Fixes #684 

Patch:
diff --git a/cmd/editaccount.go b/cmd/editaccount.go
index 08ecf7a..2ee3408 100644
--- a/cmd/editaccount.go
+++ b/cmd/editaccount.go
@@ -598,7 +598,12 @@ func (p *EditAccountParams) checkSystemAccount(ctx ActionCtx) error {
 		return nil
 	}
 
-	if p.claim.Limits.JetStreamTieredLimits != nil {
+	// allow the js to be disabled
+	if p.disableJetStream {
+		return nil
+	}
+
+	if p.claim.Limits.JetStreamTieredLimits != nil || p.enableJetStream > -1 {
 		return errors.New("system account cannot have JetStream limits - please rerun with --js-disable")
 	}
 
diff --git a/cmd/validate.go b/cmd/validate.go
index e5faeaf..d395554 100644
--- a/cmd/validate.go
+++ b/cmd/validate.go
@@ -1,5 +1,5 @@
 /*
- * Copyright 2018-2019 The NATS Authors
+ * Copyright 2018-2024 The NATS Authors
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
@@ -220,6 +220,14 @@ func (p *ValidateCmdParams) validate(ctx ActionCtx) error {
 		if aci != nil {
 			p.accountValidations[v] = aci
 		}
+		if oc.SystemAccount == ac.Subject {
+			if ac.Limits.IsJSEnabled() {
+				if p.accountValidations[v] == nil {
+					p.accountValidations[v] = &jwt.ValidationResults{}
+				}
+				p.accountValidations[v].AddError("JetStream should not be enabled for system account")
+			}
+		}
 		if !oc.DidSign(ac) {
 			if p.accountValidations[v] == nil {
 				p.accountValidations[v] = &jwt.ValidationResults{}


Test patch:
diff --git a/cmd/editaccount_test.go b/cmd/editaccount_test.go
index 5bb8464..cd3ce38 100644
--- a/cmd/editaccount_test.go
+++ b/cmd/editaccount_test.go
@@ -524,3 +524,55 @@ func Test_EnableTierNoOtherFlag(t *testing.T) {
 	require.Error(t, err)
 	require.Equal(t, "rm-js-tier is exclusive of all other js options", err.Error())
 }
+
+func Test_CannotEnableJsInSys(t *testing.T) {
+	ts := NewTestStore(t, "O")
+	defer ts.Done(t)
+	ts.AddAccount(t, "SYS")
+	_, _, err := ExecuteCmd(createEditOperatorCmd(), "--system-account", "SYS")
+	require.NoError(t, err)
+
+	_, _, err = ExecuteCmd(createEditAccount(), "--js-enable", "1")
+	require.Error(t, err)
+
+	_, _, err = ExecuteCmd(createEditAccount(), "--js-disable")
+	require.NoError(t, err)
+
+	sys, err := ts.Store.ReadAccountClaim("SYS")
+	require.NoError(t, err)
+
+	require.False(t, sys.Limits.IsJSEnabled())
+}
+
+func Test_AllowSysToDisableJs(t *testing.T) {
+	ts := NewTestStore(t, "O")
+	defer ts.Done(t)
+	ts.AddAccount(t, "SYS")
+	_, _, err := ExecuteCmd(createEditOperatorCmd(), "--system-account", "SYS")
+	require.NoError(t, err)
+
+	sys, err := ts.Store.ReadAccountClaim("SYS")
+	require.NoError(t, err)
+	require.False(t, sys.Limits.IsJSEnabled())
+
+	sys.Limits.JetStreamTieredLimits = make(map[string]jwt.JetStreamLimits)
+	sys.Limits.JetStreamTieredLimits["R1"] = jwt.JetStreamLimits{DiskStorage: -1, MemoryStorage: -1}
+
+	okp, err := ts.KeyStore.GetKeyPair(ts.GetOperatorPublicKey(t))
+	require.NoError(t, err)
+	token, err := sys.Encode(okp)
+	require.NoError(t, err)
+	require.NoError(t, ts.Store.StoreRaw([]byte(token)))
+
+	sys, err = ts.Store.ReadAccountClaim("SYS")
+	require.NoError(t, err)
+	require.True(t, sys.Limits.IsJSEnabled())
+
+	_, _, err = ExecuteCmd(createEditAccount(), "--js-disable")
+	require.NoError(t, err)
+
+	sys, err = ts.Store.ReadAccountClaim("SYS")
+	require.NoError(t, err)
+
+	require.False(t, sys.Limits.IsJSEnabled())
+}
diff --git a/cmd/validate_test.go b/cmd/validate_test.go
index c817b22..99d93b4 100644
--- a/cmd/validate_test.go
+++ b/cmd/validate_test.go
@@ -1,5 +1,5 @@
 /*
- * Copyright 2018-2023 The NATS Authors
+ * Copyright 2018-2024 The NATS Authors
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
@@ -21,6 +21,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/nats-io/jwt/v2"
 	"github.com/nats-io/nsc/v2/cmd/store"
 	"github.com/stretchr/testify/require"
 )
@@ -201,3 +202,28 @@ func Test_ValidateInteractive(t *testing.T) {
 	require.NoError(t, err)
 	require.Contains(t, stderr, "Account \"B\"")
 }
+
+func Test_ValidateJsSys(t *testing.T) {
+	ts := NewTestStore(t, "O")
+	defer ts.Done(t)
+	ts.AddAccount(t, "SYS")
+	_, _, err := ExecuteCmd(createEditOperatorCmd(), "--system-account", "SYS")
+	require.NoError(t, err)
+
+	sys, err := ts.Store.ReadAccountClaim("SYS")
+	require.NoError(t, err)
+	require.False(t, sys.Limits.IsJSEnabled())
+
+	sys.Limits.JetStreamTieredLimits = make(map[string]jwt.JetStreamLimits)
+	sys.Limits.JetStreamTieredLimits["R1"] = jwt.JetStreamLimits{DiskStorage: -1, MemoryStorage: -1}
+
+	okp, err := ts.KeyStore.GetKeyPair(ts.GetOperatorPublicKey(t))
+	require.NoError(t, err)
+	token, err := sys.Encode(okp)
+	require.NoError(t, err)
+	require.NoError(t, ts.Store.StoreRaw([]byte(token)))
+
+	_, stderr, err := ExecuteInteractiveCmd(HoistRootFlags(createValidateCommand()), []interface{}{1})
+	require.Error(t, err)
+	require.Contains(t, stderr, "JetStream should not be enabled for system account")
+}