A local config's profile entry hid the account bound in the global config

internal/auth/agent.go

@@ -525,12 +525,11 @@ func (m *Manager) agentLoginCommand(clientID, scope string) string {
 	//
 	// An entry that EXISTS without an account deliberately gets no
 	// --account. The login binds one there only when the global config
-	// file is the layer that defines the profile, and from here there is
-	// no way to know which layer did — a repo or system config that
-	// defines it accountless would make this command fail with "not the
-	// global config's entry", and a command handed over to be run
-	// verbatim should not be one that cannot work. Without it the login
-	// refuses by naming exactly what it needs, layer included, which is
+	// file is one of the layers the profile is made of — a repo or local
+	// entry for another Basecamp, or a profile only a system, repo or
+	// local config defines, would make this command fail — and a command
+	// handed over to be run verbatim should not be one that cannot work. Without it the login refuses by
+	// naming exactly what it needs, the file to change included, which is
 	// better guidance than a guess.
 	if entry, registered := m.cfg.Profiles[m.cfg.ActiveProfile]; !registered || entry == nil {
 		command += " --account " + m.accountToBind()

internal/commands/auth.go

@@ -853,13 +853,15 @@ func resolveHeadlessProfile(app *appctx.App, login headlessLogin) (*profileTarge
 			return nil, err
 		}
 	}
-	globalUnbound := false
+	blocker := ""
 	if existing != nil && existing.AccountID == "" {
-		unbound, err := globalProfileIsUnbound(name)
-		if err != nil {
+		// Binding rewrites the global config file: a file the writers would
+		// refuse is reported as itself, before anything is said about which
+		// file defines the profile, and before the secret is consumed.
+		if err := globalConfigTakesProfiles(); err != nil {
 			return nil, err
 		}
-		globalUnbound = unbound
+		blocker = globalBindingBlocker(app.Config, name)
 	}
 
 	target := &profileTarget{name: name, account: account, existing: existing}
@@ -870,16 +872,15 @@ func resolveHeadlessProfile(app *appctx.App, login headlessLogin) (*profileTarge
 	case existing == nil:
 		target.created = &config.ProfileConfig{BaseURL: app.Config.BaseURL, AccountID: account}
 		target.existing = nil
+	case blocker != "":
+		// Binding writes the global config's entry, and for this profile
+		// that would not take effect. Refused before --account is asked
+		// for, which would only lead to this refusal.
+		return nil, output.ErrUsageHint(fmt.Sprintf("Profile %q has no account, and a login cannot bind one", name),
+			blocker+", then rerun "+login.rerun+".")
 	case existing.AccountID == "" && !accountGivenExplicitly(app):
 		return nil, output.ErrUsageHint(fmt.Sprintf("Profile %q has no account", name),
 			"Pass --account <id> to bind it alongside "+login.credential+".")
-	case existing.AccountID == "" && !globalUnbound:
-		// Binding rewrites the global config file. The effective profile
-		// is accountless, so if the global entry is missing or already
-		// carries an account, the accountless one came from a system, repo
-		// or local config and would keep shadowing whatever is written.
-		return nil, output.ErrUsageHint(fmt.Sprintf("Profile %q has no account and is not the global config's entry", name),
-			"Add account_id to the config file that defines it, then rerun "+login.rerun+".")
 	case existing.AccountID == "":
 		target.bind = true
 	}
@@ -1165,7 +1166,7 @@ func requireProfileBinding(app *appctx.App, name string, existing *config.Profil
 		return output.ErrUsageHint(fmt.Sprintf("Profile %q is bound to %s, not %s", name, existing.BaseURL, app.Config.BaseURL),
 			"Use a different profile, or drop the BASECAMP_BASE_URL override.")
 	case existing.AccountID != "" && app.Config.AccountID != "" && !accountIDsEqual(app.Config.AccountID, existing.AccountID):
-		return output.ErrUsageHint(fmt.Sprintf("Profile %q is bound to account %s, not %s", name, existing.AccountID, app.Config.AccountID),
+		return output.ErrUsageHint(fmt.Sprintf("Profile %q is bound to account %s%s, not %s", name, existing.AccountID, boundIn(app.Config, name), app.Config.AccountID),
 			"Use a different profile, or drop the --account / BASECAMP_ACCOUNT_ID override.")
 	}
 	return nil

internal/commands/auth_agent.go

@@ -225,22 +225,16 @@ func resolveAgentConnectProfile(app *appctx.App) (*agentConnectTarget, error) {
 
 	// Registering or binding rewrites the global config file. Prove it can
 	// be before an operator approves anything — and, when the entry exists
-	// without an account, that the global file is the layer that defines
-	// it, since a system, repo or local config's accountless entry would
-	// keep shadowing whatever is written.
+	// without an account, that the account written there takes effect.
 	if target.existing == nil || target.existing.AccountID == "" {
 		if err := globalConfigTakesProfiles(); err != nil {
 			return nil, err
 		}
 	}
 	if target.existing != nil && target.existing.AccountID == "" {
-		unbound, err := globalProfileIsUnbound(name)
-		if err != nil {
-			return nil, err
-		}
-		if !unbound {
-			return nil, output.ErrUsageHint(fmt.Sprintf("Profile %q has no account and is not the global config's entry", name),
-				"Add account_id to the config file that defines it, then rerun the connection.")
+		if blocker := globalBindingBlocker(app.Config, name); blocker != "" {
+			return nil, output.ErrUsageHint(fmt.Sprintf("Profile %q has no account, and connecting cannot bind one", name),
+				blocker+", then rerun the connection.")
 		}
 	}
 	return target, nil
@@ -281,7 +275,7 @@ func (t *agentConnectTarget) commit(app *appctx.App, conn *auth.AgentConnection)
 		t.existing.AccountID = conn.AccountID
 	case !accountIDsEqual(t.existing.AccountID, conn.AccountID):
 		return false, output.ErrUsageHint(
-			fmt.Sprintf("Profile %q is bound to account %s, and the approved agent belongs to account %s", t.name, t.existing.AccountID, conn.AccountID),
+			fmt.Sprintf("Profile %q is bound to account %s%s, and the approved agent belongs to account %s", t.name, t.existing.AccountID, boundIn(app.Config, t.name), conn.AccountID),
 			"Nothing was stored. Connect the agent under a profile of its own: -P <name>.")
 	}
 	return isDefault, nil

internal/commands/auth_login_test.go

@@ -939,15 +939,11 @@ func TestAuthLoginWithTokenFailsClosedWithoutAnIdentity(t *testing.T) {
 
 func TestAuthLoginWithTokenBindsAnUnboundExistingProfile(t *testing.T) {
 	srv := startLoginIdentityServer(t, "bc_at_secret")
-	cfg := &config.Config{
-		ActiveProfile: "bot",
-		Profiles:      map[string]*config.ProfileConfig{"bot": {ProjectID: "42"}},
-	}
-	app, _ := loginTestApp(t, srv, cfg)
-	app.Config.Sources["profiles"] = "global"
+	app, _ := loginTestApp(t, srv, &config.Config{})
 	require.NoError(t, os.MkdirAll(config.GlobalConfigDir(), 0o700))
 	require.NoError(t, os.WriteFile(filepath.Join(config.GlobalConfigDir(), "config.json"),
 		[]byte(`{"profiles":{"bot":{"base_url":"`+srv.srv.URL+`","project_id":"42"}}}`), 0o600))
+	reloadApp(t, app, srv.srv.URL, "bot")
 
 	t.Run("without an explicit account the import is refused", func(t *testing.T) {
 		withAccount(app, "999", "global")
@@ -1049,51 +1045,48 @@ func TestAuthLoginWithTokenWithoutReportedExpiryIsNonExpiring(t *testing.T) {
 	assert.Nil(t, envelope.Data["expires_at"])
 }
 
-// TestAuthLoginWithTokenBindsOnlyTheGlobalConfigsOwnEntry: config layers
-// merge per profile name, so cfg.Sources["profiles"] is aggregate and says
-// nothing about where this profile came from. The global file's entry is
-// the evidence: bind when it exists and is unbound, refuse otherwise —
-// before stdin is read.
-func TestAuthLoginWithTokenBindsOnlyTheGlobalConfigsOwnEntry(t *testing.T) {
+// TestAuthLoginWithTokenBindsOnlyWhereTheBindingTakesEffect: binding
+// writes the global config's entry, which binds the profile only when that
+// entry is one of the layers the profile is made of. Where it is not, the
+// import is refused, naming the file to change, before stdin is read.
+func TestAuthLoginWithTokenBindsOnlyWhereTheBindingTakesEffect(t *testing.T) {
 	cases := map[string]struct {
 		global  string
-		sources string
+		local   string
 		wantErr string
 	}{
 		"global file has no such profile": {
 			global:  `{"profiles":{"other":{"base_url":"https://3.basecampapi.com"}}}`,
-			sources: "repo",
-			wantErr: "not the global config's entry",
+			local:   `{"profiles":{"bot":{"base_url":"BASE"}}}`,
+			wantErr: "Its entry comes from LOCAL, not the global config",
 		},
-		"global entry is already bound (shadowed by an accountless layer)": {
-			global:  `{"profiles":{"bot":{"base_url":"https://3.basecampapi.com","account_id":"111"}}}`,
-			sources: "local",
-			wantErr: "not the global config's entry",
+		"global entry replaced by a local entry for another Basecamp": {
+			global:  `{"profiles":{"bot":{"base_url":"https://elsewhere.example","account_id":"111"}}}`,
+			local:   `{"profiles":{"bot":{"base_url":"BASE"}}}`,
+			wantErr: "Its entry in LOCAL is for BASE, not https://elsewhere.example, so it replaces the global config's entry",
 		},
-		"global entry is unbound even though another layer contributed profiles": {
-			global:  `{"profiles":{"bot":{"base_url":"BASE","project_id":"42"}}}`,
-			sources: "repo",
+		"global entry refined by a local entry for the same Basecamp": {
+			global: `{"profiles":{"bot":{"base_url":"BASE","project_id":"42"}}}`,
+			local:  `{"profiles":{"bot":{"base_url":"BASE","todolist_id":"7"}}}`,
 		},
 	}
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
 			srv := startLoginIdentityServer(t, "bc_at_secret")
-			cfg := &config.Config{
-				ActiveProfile: "bot",
-				Profiles:      map[string]*config.ProfileConfig{"bot": {}},
-				Sources:       map[string]string{"profiles": tc.sources},
-			}
-			app, _ := loginTestApp(t, srv, cfg)
-			withAccount(app, "999", "flag")
+			app, _ := loginTestApp(t, srv, &config.Config{})
 			require.NoError(t, os.MkdirAll(config.GlobalConfigDir(), 0o700))
 			require.NoError(t, os.WriteFile(filepath.Join(config.GlobalConfigDir(), "config.json"),
 				[]byte(strings.ReplaceAll(tc.global, "BASE", srv.srv.URL)), 0o600))
+			local := trustedLocalConfig(t, strings.ReplaceAll(tc.local, "BASE", srv.srv.URL))
+			reloadApp(t, app, srv.srv.URL, "bot")
+			withAccount(app, "999", "flag")
 
 			in := strings.NewReader("bc_at_secret")
 			out, err := runLogin(t, app, in, "--with-token")
 			if tc.wantErr != "" {
 				require.Error(t, err)
-				assert.Contains(t, err.Error(), tc.wantErr)
+				want := strings.NewReplacer("LOCAL", local, "BASE", srv.srv.URL).Replace(tc.wantErr)
+				assert.Contains(t, err.Error(), want)
 				assert.Equal(t, 12, in.Len(), "refused before stdin is read")
 				assert.Empty(t, srv.seenBearers())
 				return
@@ -1102,6 +1095,8 @@ func TestAuthLoginWithTokenBindsOnlyTheGlobalConfigsOwnEntry(t *testing.T) {
 			bot := readGlobalConfig(t)["profiles"].(map[string]any)["bot"].(map[string]any)
 			assert.Equal(t, "999", bot["account_id"])
 			assert.Equal(t, "42", bot["project_id"])
+			reloadApp(t, app, srv.srv.URL, "bot")
+			assert.Equal(t, "999", app.Config.Profiles["bot"].AccountID, "the binding takes effect")
 		})
 	}
 }

internal/commands/connect.go

@@ -866,15 +866,19 @@ func connectAccount(app *appctx.App, name string) (string, error) {
 		// An entry that names an account nothing can use is not an unbound
 		// profile: say what is wrong with the account it does name.
 		if p.AccountID != "" {
+			where := "the config file that defines the profile"
+			if path := profileFieldFile(app.Config, name, "account_id"); path != "" {
+				where = "the profile's entry in " + richtext.SanitizeSingleLine(path)
+			}
 			return "", output.ErrUsageHint(
 				fmt.Sprintf("Profile %q names account %q, which is not an account ID", name, p.AccountID),
-				"Correct account_id in the config file that defines the profile.")
+				"Correct account_id in "+where+".")
 		}
-		return "", unboundProfileError(name)
+		return "", unboundProfileError(app.Config, name)
 	}
 	if accountGivenExplicitly(app) && !accountIDsEqual(app.Config.AccountID, bound) {
 		return "", output.ErrUsageHint(
-			fmt.Sprintf("Profile %q is bound to account %s, and this command named account %s", name, bound, app.Config.AccountID),
+			fmt.Sprintf("Profile %q is bound to account %s%s, and this command named account %s", name, bound, boundIn(app.Config, name), app.Config.AccountID),
 			"Setup works in the profile's own account. Drop --account (or BASECAMP_ACCOUNT_ID).")
 	}
 	return bound, nil
@@ -891,25 +895,13 @@ func connectAccount(app *appctx.App, name string) (string, error) {
 // bot that already has its credential the account goes into its config entry
 // by hand rather than through another login.
 //
-// Even `auth agent connect` refuses an accountless entry that is not the
-// global config's, because the entry it would write stays shadowed by the
-// system, repo or local config that defines it. For that profile the only
-// remedy is the file the entry comes from.
-func unboundProfileError(name string) error {
+// Those commands all write the global config's entry, which does not bind
+// every profile: globalBindingBlocker, the question they ask themselves,
+// says when it would not, and names the file to change instead.
+func unboundProfileError(cfg *config.Config, name string) error {
 	message := fmt.Sprintf("Profile %q is not bound to an account", name)
-	// The same question `auth agent connect` asks itself, so the hint is
-	// right whenever that command is: it takes an unbound global entry as
-	// its own to write. Where that reading is wrong — a trusted repo or
-	// local config defining the same accountless profile, which keeps
-	// shadowing the global entry after it is written — it is wrong for the
-	// command too, and telling them apart needs per-profile provenance the
-	// config layer does not keep.
-	//
-	// A global config that cannot be read is no reason to fail here: the
-	// command named below reports that itself, in full.
-	if unbound, err := globalProfileIsUnbound(name); err == nil && !unbound {
-		return output.ErrUsageHint(message,
-			"The entry is not the global config's, so no command can bind it. Add account_id to the config file that defines it.")
+	if blocker := globalBindingBlocker(cfg, name); blocker != "" {
+		return output.ErrUsageHint(message, blocker+".")
 	}
 	global := filepath.Join(config.GlobalConfigDir(), "config.json")
 	return output.ErrUsageHint(message,