Repository: nats-io/nsc (go)

Issue:
during push nsc may try to create dynamic sys user against scoped signing key, which will fail
### What version were you using?

Latest 2.10.2

### What environment was the server running in?

Linux

### Is this defect reproducible?

Yes

### Given the capability you are leveraging, describe your expectation?

When selecting a signing key for creating a dynamic sys user during push. nsc should check:

1. Not a scoped signing key

### Given the expectation, what is the defect you are observing?

The customer (anonymous) has created a scoped signing key and a SYS user for use in leaf node SYS account connections. This SYS user establish unidirectional trust. For extensibility and standardization a scoped signing key is used for that leaf node system user.

When do an nsc push, the method `push.go -> getSystemAccountUser()` will may pick the first signing key, without checking if it is scoped. When trying to create a temporary sys user against the scoped signing key it may:

1. Fail with: "User JWT is not valid: scoped users require no permissions or limits set"
2. OR even if it succeeds the user may not have enough permission to execute the push.

Workaround: Create and user an explicit unrestricted System user.





Pull request description:
Add stricter validation for signing keys in push/pull commands
[CHANGE] for accounts that have operators with strict signing keys, if the account only uses scoped signing key - the command will require the system user to be specified, previously it selected the first key, which may or not have the correct permissions.

This update adds comprehensive tests and improved validation logic to handle signing keys when pushing or pulling configurations. The changes ensure proper differentiation between scoped and non-scoped keys and provide clearer error messages when required keys are missing or improperly configured.

Fixes #719 

Patch:
diff --git a/cmd/push.go b/cmd/push.go
index b238497..3081054 100644
--- a/cmd/push.go
+++ b/cmd/push.go
@@ -124,6 +124,7 @@ func getSystemAccountUser(ctx ActionCtx, sysAccName, sysAccUserName, allowSub st
 			return "", nil, fmt.Errorf(`system account "%s" not found`, op.SystemAccount)
 		}
 	}
+
 	getOpt := func(theJWT string, kp nkeys.KeyPair) nats.Option {
 		return nats.UserJWT(
 			func() (string, error) {
@@ -137,10 +138,26 @@ func getSystemAccountUser(ctx ActionCtx, sysAccName, sysAccUserName, allowSub st
 		if keys, err := ctx.StoreCtx().GetAccountKeys(sysAccName); err == nil && len(keys) > 0 {
 			key := ""
 			if op.StrictSigningKeyUsage {
-				if len(keys) > 1 {
-					key = keys[1]
+				// first key is the account key
+				keys := keys[1:]
+				if len(keys) > 0 {
+					ac, err := ctx.StoreCtx().Store.ReadAccountClaim(sysAccName)
+					if err != nil {
+						return "", nil, err
+					}
+					for _, k := range keys {
+						scope, ok := ac.SigningKeys.GetScope(k)
+						// try to find a key that doesn't have a scope
+						if scope == nil && ok {
+							key = k
+							break
+						}
+					}
+					if key == "" {
+						return "", nil, fmt.Errorf(`system account %q only has scoped signing keys, specify --system-user`, sysAccName)
+					}
 				} else {
-					key = ""
+					return "", nil, fmt.Errorf(`operator requires signing keys, system account %q doesn't have signing keys'`, sysAccName)
 				}
 			} else {
 				key = keys[0]


Test patch:
diff --git a/cmd/push_test.go b/cmd/push_test.go
index e6ca3fc..45b37d2 100644
--- a/cmd/push_test.go
+++ b/cmd/push_test.go
@@ -388,3 +388,184 @@ func Test_SyncWs(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, len(filesPre), 2)
 }
+
+func Test_PushPullSigningKeysRequired(t *testing.T) {
+	ts := NewTestStore(t, "O")
+	defer ts.Done(t)
+
+	_, err := ExecuteCmd(createEditOperatorCmd(), "--sk", "generate")
+	require.NoError(t, err)
+
+	oc, err := ts.Store.ReadOperatorClaim()
+	require.NoError(t, err)
+	require.Len(t, oc.SigningKeys, 1)
+	osk := oc.SigningKeys[0]
+
+	okp, err := ts.KeyStore.GetKeyPair(osk)
+	require.NoError(t, err)
+	require.NotNil(t, okp)
+
+	ts.AddAccountWithSigner(t, "SYS", okp)
+	_, err = ExecuteCmd(createEditOperatorCmd(),
+		"--system-account", "SYS",
+		"--require-signing-keys")
+	require.NoError(t, err)
+
+	serverConf := filepath.Join(ts.Dir, "server.conf")
+	_, err = ExecuteCmd(createServerConfigCmd(), "--nats-resolver",
+		"--config-file", serverConf)
+	require.NoError(t, err)
+
+	ports := ts.RunServerWithConfig(t, serverConf)
+	defer ts.Server.Shutdown()
+
+	_, err = ExecuteCmd(createEditOperatorCmd(),
+		"--account-jwt-server-url", ports.Nats[0],
+		"--service-url", ports.Nats[0])
+	require.NoError(t, err)
+
+	out, err := ExecuteCmd(createPushCmd())
+	require.Error(t, err)
+	require.Contains(t, out.Err, "operator requires signing keys, system account \"SYS\" doesn't have signing keys")
+
+	out, err = ExecuteCmd(createPullCmd(), "--all")
+	require.Error(t, err)
+	require.Contains(t, out.Err, "operator requires signing keys, system account \"SYS\" doesn't have signing keys")
+}
+
+func Test_PushPullScopedOnly(t *testing.T) {
+	ts := NewTestStore(t, "O")
+	defer ts.Done(t)
+
+	_, err := ExecuteCmd(createEditOperatorCmd(), "--sk", "generate")
+	require.NoError(t, err)
+
+	oc, err := ts.Store.ReadOperatorClaim()
+	require.NoError(t, err)
+	require.Len(t, oc.SigningKeys, 1)
+	osk := oc.SigningKeys[0]
+
+	okp, err := ts.KeyStore.GetKeyPair(osk)
+	require.NoError(t, err)
+	require.NotNil(t, okp)
+
+	ts.AddAccountWithSigner(t, "SYS", okp)
+	_, err = ExecuteCmd(createEditOperatorCmd(),
+		"--system-account", "SYS",
+		"--require-signing-keys")
+	require.NoError(t, err)
+
+	_, err = ExecuteCmd(HoistRootFlags(createEditSkopedSkCmd()), "--account", "SYS", "--sk", "generate", "--role", "test", "--allow-pubsub", ">", "-K", osk)
+	require.NoError(t, err)
+
+	ac, err := ts.Store.ReadAccountClaim("SYS")
+	require.NoError(t, err)
+	require.Len(t, ac.SigningKeys, 1)
+	sysKP, err := ts.KeyStore.GetKeyPair(ac.SigningKeys.Keys()[0])
+	require.NoError(t, err)
+	ts.AddUserWithSigner(t, "SYS", "sys", sysKP)
+
+	serverConf := filepath.Join(ts.Dir, "server.conf")
+	_, err = ExecuteCmd(createServerConfigCmd(), "--nats-resolver",
+		"--config-file", serverConf)
+	require.NoError(t, err)
+
+	ports := ts.RunServerWithConfig(t, serverConf)
+	defer ts.Server.Shutdown()
+
+	_, err = ExecuteCmd(createEditOperatorCmd(),
+		"--account-jwt-server-url", ports.Nats[0],
+		"--service-url", ports.Nats[0])
+	require.NoError(t, err)
+
+	out, err := ExecuteCmd(createPushCmd())
+	require.Error(t, err)
+	require.Contains(t, out.Err, "system account \"SYS\" only has scoped signing keys, specify --system-user")
+
+	_, err = ExecuteCmd(createPullCmd(), "--all")
+	require.Error(t, err)
+	require.Contains(t, out.Err, "system account \"SYS\" only has scoped signing keys, specify --system-user")
+
+	_, err = ExecuteCmd(createPushCmd(), "--system-user", "sys")
+	require.NoError(t, err)
+
+	out, err = ExecuteCmd(createPullCmd(), "--all", "--overwrite-newer", "--system-user", "sys")
+	require.NoError(t, err)
+}
+
+func Test_PushPullScopedAndNormalOnly(t *testing.T) {
+	ts := NewTestStore(t, "O")
+	defer ts.Done(t)
+
+	_, err := ExecuteCmd(createEditOperatorCmd(), "--sk", "generate")
+	require.NoError(t, err)
+
+	oc, err := ts.Store.ReadOperatorClaim()
+	require.NoError(t, err)
+	require.Len(t, oc.SigningKeys, 1)
+	osk := oc.SigningKeys[0]
+
+	okp, err := ts.KeyStore.GetKeyPair(osk)
+	require.NoError(t, err)
+	require.NotNil(t, okp)
+
+	ts.AddAccountWithSigner(t, "SYS", okp)
+	_, err = ExecuteCmd(createEditOperatorCmd(),
+		"--system-account", "SYS",
+		"--require-signing-keys")
+	require.NoError(t, err)
+
+	_, err = ExecuteCmd(HoistRootFlags(createEditSkopedSkCmd()), "--account", "SYS", "--sk", "generate", "--role", "test", "--allow-pubsub", "foo", "-K", osk)
+	require.NoError(t, err)
+
+	_, err = ExecuteCmd(HoistRootFlags(createEditAccount()), "--name", "SYS", "--sk", "generate", "-K", osk)
+	require.NoError(t, err)
+
+	serverConf := filepath.Join(ts.Dir, "server.conf")
+	_, err = ExecuteCmd(createServerConfigCmd(), "--nats-resolver",
+		"--config-file", serverConf)
+	require.NoError(t, err)
+
+	ports := ts.RunServerWithConfig(t, serverConf)
+	defer ts.Server.Shutdown()
+
+	_, err = ExecuteCmd(createEditOperatorCmd(),
+		"--account-jwt-server-url", ports.Nats[0],
+		"--service-url", ports.Nats[0])
+	require.NoError(t, err)
+
+	_, err = ExecuteCmd(createPushCmd())
+	require.NoError(t, err)
+
+	_, err = ExecuteCmd(createPullCmd(), "--all", "--overwrite-newer")
+	require.NoError(t, err)
+}
+
+func Test_PushPullNormalOnly(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)
+
+	serverConf := filepath.Join(ts.Dir, "server.conf")
+	_, err = ExecuteCmd(createServerConfigCmd(), "--nats-resolver",
+		"--config-file", serverConf)
+	require.NoError(t, err)
+
+	ports := ts.RunServerWithConfig(t, serverConf)
+	defer ts.Server.Shutdown()
+
+	_, err = ExecuteCmd(createEditOperatorCmd(),
+		"--account-jwt-server-url", ports.Nats[0],
+		"--service-url", ports.Nats[0])
+	require.NoError(t, err)
+
+	_, err = ExecuteCmd(createPushCmd())
+	require.NoError(t, err)
+
+	_, err = ExecuteCmd(createPullCmd(), "--all", "--overwrite-newer")
+	require.NoError(t, err)
+}