config: include IsFedramp in Forwarder.Hash

[nileshpatil6]

Summary

Forwarder.Hash() hashes five of the struct's six fields. IsFedramp is never mixed into the digest, so two forwarders that differ only in that setting hash identically.

Why it matters

The hash is the identity function for live config reload, not a display value. When the config file changes, AppService.handleConfigUpdate builds a ForwarderService per forwarders: entry and hands it to overwatch.AppManager.Add:

func (m *AppManager) Add(service Service) {
	if currentService, ok := m.services[service.Name()]; ok {
		if currentService.Hash() == service.Hash() {
			return // the exact same service, no changes, so move along
		}
		currentService.Shutdown() //shutdown the listener since a new one is starting
	}
	...

ForwarderService.Hash() delegates straight to Forwarder.Hash(). So an operator who edits an existing forwarder to flip isFedramp gets a byte-identical hash, Add early-returns, the running listener is never shut down and the replacement never starts. The edit is silently discarded, with no log line, until the process is restarted.

The setting is not cosmetic. Forwarder.IsFedramp is carried into carrier.StartOptions in access.StartForwarder:

options := &carrier.StartOptions{
	OriginURL: forwarder.URL,
	Headers:   headers,
	IsFedramp: forwarder.IsFedramp,
}

and carrier.StartForwarder passes it to token.FetchTokenWithRedirect(req.URL, options.AppInfo, options.AutoCloseInterstitial, options.IsFedramp, log), which selects the FedRAMP or the commercial Cloudflare Access endpoint for the token exchange. So the daemon reports success while continuing to authenticate against the previous endpoint.

How it happened

IsFedramp was added to the struct in 8825cee ("AUTH-7480 update fed callback url for login helper"). That same commit rewrote the body of Forwarder.Hash() line by line, switching md5 to sha256 and adding _, _ =, without adding the new field:

 	Destination   string `json:"destination"`
+	IsFedramp     bool   `json:"is_fedramp" yaml:"isFedramp"`
 }
...
 func (f *Forwarder) Hash() string {
-	h := md5.New()
-	io.WriteString(h, f.URL)
+	h := sha256.New()
+	_, _ = io.WriteString(h, f.URL)

The sibling Hash() immediately below it in that same commit did mix in its bool, with _, _ = io.WriteString(h, fmt.Sprintf("%v", r.Enabled)), so this looks like an oversight rather than a deliberate exclusion. I have used that same idiom here.

Scope, stated plainly

This only affects the file-watching config reload path used when cloudflared runs with no subcommand. cloudflared access ssh / tcp read the flag fresh per invocation and are unaffected, and anyone who restarts the process after editing config is unaffected. I verified the hash collision and traced the call chain by reading; I did not run a live daemon reload end to end.

Testing

=== RUN   TestForwarderHashIncludesIsFedramp
--- PASS: TestForwarderHashIncludesIsFedramp (0.00s)
ok  	github.com/cloudflare/cloudflared/config

Without the change the same test fails, and the message shows both structs producing one digest:

    Error: Should not be: "fec6fbbd9ec8692a78b6df9bc0b6af7942b04987dc9d09ccb8f6b8402f75e4c9"
    Messages: changing IsFedramp must change the forwarder hash

go build ./..., go vet ./config/..., and go test ./config/... ./overwatch/... all pass. gofmt -l reports nothing for the two touched files, so the whole-files: true lint rule is not triggered.

Possible follow-up, not included here

Hash() concatenates fields with no delimiter, so in principle {URL: "ab", Listener: "c"} and {URL: "a", Listener: "bc"} collide. Realistic URL and listener formats make that hard to hit, and fixing it would widen this diff, so I have left it alone. Happy to send it separately if you want it.