Cherry-picks for 2.12.6-RC.3 (#59)

Includes the following:

- #7958
- #7959
- #7960
- #7961
- #7962
- #7896
- Security fixes

Signed-off-by: Neil Twigg <neil@nats.io>

Author: neilalexander

conf/parse.go

@@ -114,42 +114,28 @@ func ParseFileWithChecks(fp string) (map[string]any, error) {
 	return p.mapping, nil
 }
 
-// cleanupUsedEnvVars will recursively remove all already used
-// environment variables which might be in the parsed tree.
-func cleanupUsedEnvVars(m map[string]any) {
-	for k, v := range m {
-		t := v.(*token)
-		if t.usedVariable {
-			delete(m, k)
-			continue
-		}
-		// Cleanup any other env var that is still in the map.
-		if tm, ok := t.value.(map[string]any); ok {
-			cleanupUsedEnvVars(tm)
-		}
+// configDigest returns a digest for the parsed config.
+func configDigest(m map[string]any) (string, error) {
+	digest := sha256.New()
+	e := json.NewEncoder(digest)
+	if err := e.Encode(m); err != nil {
+		return _EMPTY_, err
 	}
+	return fmt.Sprintf("sha256:%x", digest.Sum(nil)), nil
 }
 
 // ParseFileWithChecksDigest returns the processed config and a digest
 // that represents the configuration.
 func ParseFileWithChecksDigest(fp string) (map[string]any, string, error) {
-	data, err := os.ReadFile(fp)
+	m, err := ParseFileWithChecks(fp)
 	if err != nil {
 		return nil, _EMPTY_, err
 	}
-	p, err := parse(string(data), fp, true)
-	if err != nil {
-		return nil, _EMPTY_, err
-	}
-	// Filter out any environment variables before taking the digest.
-	cleanupUsedEnvVars(p.mapping)
-	digest := sha256.New()
-	e := json.NewEncoder(digest)
-	err = e.Encode(p.mapping)
+	digest, err := configDigest(m)
 	if err != nil {
 		return nil, _EMPTY_, err
 	}
-	return p.mapping, fmt.Sprintf("sha256:%x", digest.Sum(nil)), nil
+	return m, digest, nil
 }
 
 type token struct {

conf/parse_test.go

@@ -218,6 +218,24 @@ func TestConvenientNumbers(t *testing.T) {
 	test(t, easynum, ex)
 }
 
+func TestParseFileWithChecksDigestPreservesConfigKeyUsedAsVariable(t *testing.T) {
+	confFile := filepath.Join(t.TempDir(), "nats.conf")
+	if err := os.WriteFile(confFile, []byte(`
+		port = 4222
+		monitor_port = $port
+	`), 0666); err != nil {
+		t.Fatal(err)
+	}
+
+	m, _, err := ParseFileWithChecksDigest(confFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, ok := m["port"]; !ok {
+		t.Fatal("expected config key used as a variable reference to be preserved")
+	}
+}
+
 var sample1 = `
 foo  {
   host {
@@ -919,7 +937,7 @@ func TestParseDigest(t *testing.T) {
                         very { nested { env { VAR = 'NESTED', quux = $VAR }}}
                         `,
 			nil,
-			"sha256:34f8faf3f269fe7509edc4742f20c8c4a7ad51fe21f8b361764314b533ac3ab5",
+			"sha256:bddb282343249c26d3edcef9bfaa9d2711505fc67210380e871405ba394a9186",
 		},
 		{
 			`# substitutions, same as previous one without env vars.
@@ -942,7 +960,7 @@ func TestParseDigest(t *testing.T) {
                         }
                         `,
 			nil,
-			"sha256:f5d943b4ed22b80c6199203f8a7eaa8eb68ef7b2d46ef6b1b26f05e21f8beb13",
+			"sha256:b0e2ba0b8ec2cb75681b5c5d61789cda0c9942c2f9fe16cdb7f6703364485360",
 		},
 		{
 			`# substitutions

go.mod

@@ -8,7 +8,7 @@ require (
 	github.com/antithesishq/antithesis-sdk-go v0.6.0-default-no-op
 	github.com/google/go-tpm v0.9.8
 	github.com/klauspost/compress v1.18.4
-	github.com/nats-io/jwt/v2 v2.8.0
+	github.com/nats-io/jwt/v2 v2.8.1
 	github.com/nats-io/nats.go v1.49.0
 	github.com/nats-io/nkeys v0.4.15
 	github.com/nats-io/nuid v1.0.1

go.sum

@@ -6,8 +6,8 @@ github.com/klauspost/compress v1.18.4 h1:RPhnKRAQ4Fh8zU2FY/6ZFDwTVTxgJ/EMydqSTzE
 github.com/klauspost/compress v1.18.4/go.mod h1:R0h/fSBs8DE4ENlcrlib3PsXS61voFxhIs2DeRhCvJ4=
 github.com/minio/highwayhash v1.0.4-0.20251030100505-070ab1a87a76 h1:KGuD/pM2JpL9FAYvBrnBBeENKZNh6eNtjqytV6TYjnk=
 github.com/minio/highwayhash v1.0.4-0.20251030100505-070ab1a87a76/go.mod h1:GGYsuwP/fPD6Y9hMiXuapVvlIUEhFhMTh0rxU3ik1LQ=
-github.com/nats-io/jwt/v2 v2.8.0 h1:K7uzyz50+yGZDO5o772eRE7atlcSEENpL7P+b74JV1g=
-github.com/nats-io/jwt/v2 v2.8.0/go.mod h1:me11pOkwObtcBNR8AiMrUbtVOUGkqYjMQZ6jnSdVUIA=
+github.com/nats-io/jwt/v2 v2.8.1 h1:V0xpGuD/N8Mi+fQNDynXohVvp7ZztevW5io8CUWlPmU=
+github.com/nats-io/jwt/v2 v2.8.1/go.mod h1:nWnOEEiVMiKHQpnAy4eXlizVEtSfzacZ1Q43LIRavZg=
 github.com/nats-io/nats.go v1.49.0 h1:yh/WvY59gXqYpgl33ZI+XoVPKyut/IcEaqtsiuTJpoE=
 github.com/nats-io/nats.go v1.49.0/go.mod h1:fDCn3mN5cY8HooHwE2ukiLb4p4G4ImmzvXyJt+tGwdw=
 github.com/nats-io/nkeys v0.4.15 h1:JACV5jRVO9V856KOapQ7x+EY8Jo3qw1vJt/9Jpwzkk4=

server/accounts.go

@@ -1616,10 +1616,12 @@ func (a *Account) checkServiceImportsForCycles(from string, visited map[string]b
 				}
 				// Push ourselves and check si.acc
 				visited[a.Name] = true
-				if subjectIsSubsetMatch(si.from, from) {
-					from = si.from
+				// Make a copy to not overwrite the passed value.
+				f := from
+				if subjectIsSubsetMatch(si.from, f) {
+					f = si.from
 				}
-				if err := si.acc.checkServiceImportsForCycles(from, visited); err != nil {
+				if err := si.acc.checkServiceImportsForCycles(f, visited); err != nil {
 					return err
 				}
 				a.mu.RLock()
@@ -1674,10 +1676,12 @@ func (a *Account) checkStreamImportsForCycles(to string, visited map[string]bool
 			}
 			// Push ourselves and check si.acc
 			visited[a.Name] = true
-			if subjectIsSubsetMatch(si.to, to) {
-				to = si.to
+			// Make a copy to not overwrite the passed value.
+			t := to
+			if subjectIsSubsetMatch(si.to, t) {
+				t = si.to
 			}
-			if err := si.acc.checkStreamImportsForCycles(to, visited); err != nil {
+			if err := si.acc.checkStreamImportsForCycles(t, visited); err != nil {
 				return err
 			}
 			a.mu.RLock()

server/auth_callout.go

@@ -81,9 +81,6 @@ func (s *Server) processClientOrLeafCallout(c *client, opts *Options, proxyRequi
 		xkp, xkey = s.xkp, s.info.XKey
 	}
 
-	// FIXME: so things like the server ID that get assigned, are used as a sort of nonce - but
-	//  reality is that the keypair here, is generated, so the response generated a JWT has to be
-	//  this user - no replay possible
 	// Create a keypair for the user. We will expect this public user to be in the signed response.
 	// This prevents replay attacks.
 	ukp, _ := nkeys.CreateUser()

server/client.go

@@ -2817,8 +2817,8 @@ func (c *client) processHeaderPub(arg, remaining []byte) error {
 		// Do this only for CLIENT connections.
 		if c.kind == CLIENT && c.pa.hdr > 0 && len(remaining) > 0 {
 			hdr := remaining[:min(len(remaining), c.pa.hdr)]
-			if td := getHeader(MsgTraceDest, hdr); len(td) > 0 {
-				c.initAndSendIngressErrEvent(hdr, string(td), ErrMaxPayload)
+			if td, ok := c.allowedMsgTraceDest(hdr, false); ok && td != _EMPTY_ {
+				c.initAndSendIngressErrEvent(hdr, td, ErrMaxPayload)
 			}
 		}
 		c.maxPayloadViolation(c.pa.size, maxPayload)
@@ -4066,6 +4066,41 @@ func (c *client) pubAllowed(subject string) bool {
 	return c.pubAllowedFullCheck(subject, true, false)
 }
 
+// allowedMsgTraceDest returns the trace destination if present and authorized.
+// It only considers static publish permissions and does not consume dynamic
+// reply permissions because the client is not publishing the trace event itself.
+func (c *client) allowedMsgTraceDest(hdr []byte, hasLock bool) (string, bool) {
+	if len(hdr) == 0 {
+		return _EMPTY_, true
+	}
+	td := sliceHeader(MsgTraceDest, hdr)
+	if len(td) == 0 {
+		return _EMPTY_, true
+	}
+	dest := bytesToString(td)
+	if c.kind == CLIENT {
+		if hasGWRoutedReplyPrefix(td) {
+			return dest, false
+		}
+		var acc *Account
+		var srv *Server
+		if !hasLock {
+			c.mu.Lock()
+		}
+		acc, srv = c.acc, c.srv
+		if !hasLock {
+			c.mu.Unlock()
+		}
+		if bytes.HasPrefix(td, clientNRGPrefix) && srv != nil && acc != srv.SystemAccount() {
+			return dest, false
+		}
+	}
+	if c.perms != nil && (c.perms.pub.allow != nil || c.perms.pub.deny != nil) && !c.pubAllowedFullCheck(dest, false, hasLock) {
+		return dest, false
+	}
+	return dest, true
+}
+
 // pubAllowedFullCheck checks on all publish permissioning depending
 // on the flag for dynamic reply permissions.
 func (c *client) pubAllowedFullCheck(subject string, fullCheck, hasLock bool) bool {
@@ -4201,10 +4236,19 @@ func (c *client) processInboundClientMsg(msg []byte) (bool, bool) {
 	genidAddr := &acc.sl.genid
 
 	// Check pub permissions
-	if c.perms != nil && (c.perms.pub.allow != nil || c.perms.pub.deny != nil) && !c.pubAllowedFullCheck(string(c.pa.subject), true, true) {
-		c.mu.Unlock()
-		c.pubPermissionViolation(c.pa.subject)
-		return false, true
+	if c.perms != nil && (c.perms.pub.allow != nil || c.perms.pub.deny != nil) {
+		if !c.pubAllowedFullCheck(string(c.pa.subject), true, true) {
+			c.mu.Unlock()
+			c.pubPermissionViolation(c.pa.subject)
+			return false, true
+		}
+	}
+	if c.pa.hdr > 0 {
+		if td, ok := c.allowedMsgTraceDest(msg[:c.pa.hdr], true); !ok {
+			c.mu.Unlock()
+			c.pubPermissionViolation(stringToBytes(td))
+			return false, true
+		}
 	}
 	c.mu.Unlock()
 
@@ -4775,29 +4819,40 @@ func (c *client) processServiceImport(si *serviceImport, acc *Account, msg []byt
 	if !isResponse {
 		isSysImport := siAcc == c.srv.SystemAccount()
 		var ci *ClientInfo
-		if hadPrevSi && c.pa.hdr >= 0 {
-			var cis ClientInfo
-			if err := json.Unmarshal(sliceHeader(ClientInfoHdr, msg[:c.pa.hdr]), &cis); err == nil {
-				ci = &cis
+		var cis *ClientInfo
+		if c.pa.hdr >= 0 {
+			var hci ClientInfo
+			if err := json.Unmarshal(sliceHeader(ClientInfoHdr, msg[:c.pa.hdr]), &hci); err == nil {
+				cis = &hci
+			}
+		}
+		if c.kind == LEAF && c.pa.hdr >= 0 && len(sliceHeader(ClientInfoHdr, msg[:c.pa.hdr])) > 0 {
+			// Leaf nodes may forward a Nats-Request-Info from a remote domain,
+			// but the local server must replace it with the identity of the
+			// authenticated leaf connection instead of trusting forwarded values.
+			ci = c.getClientInfo(share)
+			if hadPrevSi {
 				ci.Service = acc.Name
-				// Check if we are moving into a share details account from a non-shared
-				// and add in server and cluster details.
 				if !share && (si.share || isSysImport) {
 					c.addServerAndClusterInfo(ci)
 				}
+			} else if !share && isSysImport {
+				c.addServerAndClusterInfo(ci)
+			}
+		} else if hadPrevSi && cis != nil {
+			ci = cis
+			ci.Service = acc.Name
+			// Check if we are moving into a share details account from a non-shared
+			// and add in server and cluster details.
+			if !share && (si.share || isSysImport) {
+				c.addServerAndClusterInfo(ci)
 			}
 		} else if c.kind != LEAF || c.pa.hdr < 0 || len(sliceHeader(ClientInfoHdr, msg[:c.pa.hdr])) == 0 {
 			ci = c.getClientInfo(share)
 			// If we did not share but the imports destination is the system account add in the server and cluster info.
 			if !share && isSysImport {
 				c.addServerAndClusterInfo(ci)
 			}
-		} else if c.kind == LEAF && (si.share || isSysImport) {
-			// We have a leaf header here for ci, augment as above.
-			ci = c.getClientInfo(si.share)
-			if !si.share && isSysImport {
-				c.addServerAndClusterInfo(ci)
-			}
 		}
 		// Set clientInfo if present.
 		if ci != nil {