From e33fe3bd12b0fcea9ca069697433846050c80f02 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 15:38:09 -0700 Subject: [PATCH 1/6] Mint a per-session agent token with auth token --session-id An agent profile's shared token is cached and served to every process on the host, so Basecamp can't tell one sandboxed launch from another. With --session-id (and optionally --session-label), auth token --stored mints a fresh self-token for that session, sending launch_id and launch_label on the client_credentials request, and prints it without caching it or touching the shared one. The token is bound to the session or not handed out: a server that refuses the parameters, or answers without echoing them back because it predates session attribution, fails the command and the unbound token is discarded. Holds on the client secret are honored and written as the shared mint's are; a refusal of the session's own parameters holds nothing. --- .surface | 2 + internal/auth/agent.go | 10 + internal/auth/agent_launch.go | 225 +++++++++++++++++++ internal/auth/agent_launch_test.go | 212 +++++++++++++++++ internal/commands/auth.go | 35 ++- internal/commands/auth_token_session_test.go | 95 ++++++++ 6 files changed, 577 insertions(+), 2 deletions(-) create mode 100644 internal/auth/agent_launch.go create mode 100644 internal/auth/agent_launch_test.go create mode 100644 internal/commands/auth_token_session_test.go diff --git a/.surface b/.surface index f305e3a15..e12b52ea6 100644 --- a/.surface +++ b/.surface @@ -2170,6 +2170,8 @@ FLAG basecamp auth token --no-stats type=bool FLAG basecamp auth token --profile type=string FLAG basecamp auth token --project type=string FLAG basecamp auth token --quiet type=bool +FLAG basecamp auth token --session-id type=string +FLAG basecamp auth token --session-label type=string FLAG basecamp auth token --stats type=bool FLAG basecamp auth token --stored type=bool FLAG basecamp auth token --styled type=bool diff --git a/internal/auth/agent.go b/internal/auth/agent.go index ec90cbdd8..4ffdf5dc7 100644 --- a/internal/auth/agent.go +++ b/internal/auth/agent.go @@ -84,6 +84,10 @@ type agentMint struct { scope string resource string client *http.Client + + // launch is the session a session mint attributes its token to, or + // nil for the profile's shared token (see agent_launch.go). + launch *agentLaunch } // prepareAgentMint is the half of a mint that sends nothing: it checks what @@ -324,6 +328,7 @@ func (m *Manager) mintAgentToken(ctx context.Context, mint *agentMint) (*oauth.T if mint.resource != "" { form.Set("resource", mint.resource) } + mint.launch.addTo(form) reqCtx, cancel := context.WithTimeout(ctx, agentMintTimeout) defer cancel() @@ -422,6 +427,11 @@ func (m *Manager) mintAgentToken(ctx context.Context, mint *agentMint) (*oauth.T if err := applyTokenLifetime(&token, body); err != nil { return nil, nil, output.ErrAPI(resp.StatusCode, "minting an agent token: "+err.Error()) } + // Last, so a token that is not bound to the session it was asked for + // never leaves this function, however good it is otherwise. + if err := mint.launch.requireEcho(resp.StatusCode, body); err != nil { + return nil, nil, err + } return &token, nil, nil } diff --git a/internal/auth/agent_launch.go b/internal/auth/agent_launch.go new file mode 100644 index 000000000..a21831926 --- /dev/null +++ b/internal/auth/agent_launch.go @@ -0,0 +1,225 @@ +package auth + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/url" + "regexp" + "strings" + "unicode" + "unicode/utf8" + + "github.com/basecamp/basecamp-cli/internal/output" +) + +// Agent sessions: one self-token per launch, attributed to that launch. +// +// An agent profile's shared token is cached and served to every process on +// the host, so Basecamp cannot tell one sandboxed box from another: every +// request reads as the agent, from one token. A session mint asks the token +// endpoint for a token of the session's own instead, carrying a random id +// the launcher generated and a label it chose, which Basecamp stores on the +// token and writes beside the agent in its request logs. +// +// The CLI calls this a session; on the wire, and in bc3, it is a launch +// (launch_id, launch_label), because "session" already means the web +// sign-in there. +// +// The contract is "a token bound to that session, or nothing": +// +// - The token is minted fresh on every call and never cached here. The +// caller is the per-launch cache: the sandbox's credential capture holds +// each token for a few minutes inside the launch's own proxy, which dies +// with the launch. Caching here instead would write one keyring entry +// per launch, which nothing would ever clean up, and contend every launch +// on the one credential lock for a token only one launch may use. +// +// - The shared cached token is neither served nor written: a session that +// fell back to it would be indistinguishable from every other process. +// +// - A server that does not echo the session back has not bound the token +// to it — it predates session attribution and ignored the parameters — +// and the token is discarded. Handing it out would be the failure that +// reads as working: attribution silently missing, and once Basecamp can +// stop a session, a token that stopping it would not reach. +// +// - A refusal is reported, never retried without the session. + +// launchIDPattern is a session id as bc3 accepts it: 128 random bits as +// canonical lowercase hex. +var launchIDPattern = regexp.MustCompile(`\A[0-9a-f]{32}\z`) + +// maxLaunchLabelChars is bc3's bound on a session label, in characters. +const maxLaunchLabelChars = 100 + +// agentLaunch is the session a mint attributes its token to. +type agentLaunch struct { + id string + label string +} + +// ValidateSession checks a session id and label against the rules the +// token endpoint holds them to, so a malformed one is a usage error here +// rather than a refused mint. The label is optional; the id is not. +func ValidateSession(id, label string) error { + if !launchIDPattern.MatchString(id) { + return output.ErrUsageHint("A session id must be 32 lowercase hex characters (128 random bits)", + "Generate one with: openssl rand -hex 16") + } + if label == "" { + return nil + } + if !utf8.ValidString(label) { + return output.ErrUsage("A session label must be valid UTF-8") + } + if strings.TrimSpace(label) == "" { + return output.ErrUsage("A session label must not be blank") + } + if n := utf8.RuneCountInString(label); n > maxLaunchLabelChars { + return output.ErrUsage(fmt.Sprintf("A session label must be at most %d characters (this one is %d)", maxLaunchLabelChars, n)) + } + for _, r := range label { + if !labelRune(r) { + return output.ErrUsage(fmt.Sprintf("A session label must not contain control, formatting or line-separator characters (found U+%04X)", r)) + } + } + return nil +} + +// labelRune reports whether r may appear in a session label: an assigned +// character outside the "other" category (control, format, private use, +// surrogate) and the line and paragraph separators. The label is shown to +// people, so nothing that reorders, hides or breaks the text around it. +func labelRune(r rune) bool { + if unicode.In(r, unicode.C, unicode.Zl, unicode.Zp) { + return false + } + // Unassigned code points are in no category table at all. + return unicode.In(r, unicode.L, unicode.M, unicode.N, unicode.P, unicode.S, unicode.Zs) +} + +// addTo puts the session on a client_credentials form. A nil launch is the +// shared token's mint, which sends nothing extra. +func (l *agentLaunch) addTo(form url.Values) { + if l == nil { + return + } + form.Set("launch_id", l.id) + if l.label != "" { + form.Set("launch_label", l.label) + } +} + +// errLaunchNotRecorded is the cause of a session mint whose token came back +// without the session on it. +var errLaunchNotRecorded = errors.New("the token endpoint did not record the session") + +// requireEcho refuses a token response that does not carry the session it +// was asked for. bc3 echoes launch_id, and launch_label when one was given, +// once it has stored them on the token; a server that ignored them echoes +// nothing. +func (l *agentLaunch) requireEcho(status int, body []byte) error { + if l == nil { + return nil + } + var echoed struct { + ID *string `json:"launch_id"` + Label *string `json:"launch_label"` + } + _ = json.Unmarshal(body, &echoed) + + idMatches := echoed.ID != nil && *echoed.ID == l.id + labelMatches := (l.label == "" && echoed.Label == nil) || (echoed.Label != nil && *echoed.Label == l.label) + if idMatches && labelMatches { + return nil + } + + msg := fmt.Sprintf("Basecamp minted a token but did not bind it to session %s, so it was discarded", l.id) + if echoed.ID == nil { + msg += ": this Basecamp predates agent session attribution" + } else { + msg += ": this Basecamp predates agent session attribution, or answered for a different session" + } + e := output.ErrAPI(status, msg) + e.Hint = "A session token needs a Basecamp that records agent sessions; without one, leave out --session-id and --session-label" + e.Cause = errLaunchNotRecorded + return e +} + +// SessionAccessToken mints a fresh self-token for one agent session from +// the active profile's stored agent credential, and returns it without +// storing it. See the top of this file for why it is never cached and why +// a server that does not bind it to the session gets no token at all. +// +// Holds are honored and written exactly as the shared mint's are, because +// they are verdicts on the client secret both mints present: a refused or +// rate-limited secret is answered locally here too, and a refusal of the +// secret seen here is remembered for every later mint. A refusal of the +// request itself — the session's parameters, above all — is not about the +// secret and holds nothing, so one malformed launch cannot stop the rest. +func (m *Manager) SessionAccessToken(ctx context.Context, id, label string) (string, error) { + if err := ValidateSession(id, label); err != nil { + return "", err + } + + m.mu.Lock() + defer m.mu.Unlock() + + credKey := m.credentialKey() + creds, err := m.store.LoadContext(ctx, credKey) + if err != nil { + return "", m.unreadable("No stored credentials for", credKey, err) + } + m.remember(creds) + if creds.OAuthType != oauthTypeAgent { + return "", output.ErrUsage(fmt.Sprintf( + "A session token is minted from an agent's own credential, and %s holds a person's login; run without --session-id", credKey)) + } + + launch := &agentLaunch{id: id, label: label} + mint, err := m.prepareAgentMint(creds) + if err != nil { + return "", forSession(launch, m.hintLogin(err)) + } + mint.launch = launch + + // No credential lock across the round trip: nothing is written on + // success, so there is nothing for concurrent launches to be exclusive + // with. Only a hold is written, and that under the lock, as the shared + // path writes it. + token, hold, err := m.mintAgentToken(ctx, mint) + if err != nil { + if hold != nil { + lockErr := m.store.withKeyLock(ctx, credKey, func() error { + m.rememberMintHold(credKey, hold) + return nil + }) + if lockErr != nil { + m.warnf("warning: could not lock the credential for %s to remember the token endpoint's refusal, so the next command will ask again: %v", credKey, lockErr) + } + } + return "", forSession(launch, m.hintLogin(err)) + } + return token.AccessToken, nil +} + +// forSession names the session in a failed session mint's message, keeping +// its class, hint and cause. +func forSession(l *agentLaunch, err error) error { + if errors.Is(err, errLaunchNotRecorded) { + // Already says which session. + return err + } + var e *output.Error + if !errors.As(err, &e) { + return fmt.Errorf("session %s: %w", l.id, err) + } + named := *e + named.Message = fmt.Sprintf("Session %s: %s", l.id, e.Message) + if named.Cause == nil { + named.Cause = err + } + return &named +} diff --git a/internal/auth/agent_launch_test.go b/internal/auth/agent_launch_test.go new file mode 100644 index 000000000..e5988cf4c --- /dev/null +++ b/internal/auth/agent_launch_test.go @@ -0,0 +1,212 @@ +package auth + +import ( + "context" + "net/http" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/output" +) + +const testSessionID = "0123456789abcdef0123456789abcdef" + +// liveAgentCredential is an agent profile whose shared token is comfortably +// inside its lifetime, so anything that mints did so on purpose. +func liveAgentCredential(tokenEndpoint string) *Credentials { + creds := agentCredential(tokenEndpoint, time.Now().Add(time.Hour)) + creds.AccessToken = "shared-cached" + return creds +} + +// TestSessionTokenMintsFreshAndLeavesTheSharedTokenAlone: a session asks for +// its own token every time, with its id and label on the mint, and the +// profile's cached token is neither served nor overwritten. +func TestSessionTokenMintsFreshAndLeavesTheSharedTokenAlone(t *testing.T) { + as := startDeviceAS(t) + as.token = func(call int) (int, string) { + return http.StatusOK, `{"access_token":"session-` + string(rune('1'+call)) + `","token_type":"Bearer","expires_in":3600,"scope":"full",` + + `"launch_id":"` + testSessionID + `","launch_label":"coworker@box"}` + } + + m := newDeviceTestManager(t, as.srv.URL) + key := storeAgent(t, m, liveAgentCredential(as.srv.URL+"/oauth/token")) + before, err := m.store.Load(key) + require.NoError(t, err) + + first, err := m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.NoError(t, err) + assert.Equal(t, "session-1", first) + + second, err := m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.NoError(t, err) + assert.Equal(t, "session-2", second, "a session token is minted fresh, never cached") + + calls := as.tokenCalls() + require.Len(t, calls, 2) + assert.Equal(t, "client_credentials", calls[0].Get("grant_type")) + assert.Equal(t, testSessionID, calls[0].Get("launch_id")) + assert.Equal(t, "coworker@box", calls[0].Get("launch_label")) + assert.Equal(t, AgentResourceURNPrefix+"42", calls[0].Get("resource")) + + after, err := m.store.Load(key) + require.NoError(t, err) + assert.Equal(t, before, after, "a session mint must not touch the stored credential") + + shared, err := m.StoredAccessToken(context.Background()) + require.NoError(t, err) + assert.Equal(t, "shared-cached", shared) + assert.Len(t, as.tokenCalls(), 2, "the shared token was still live and needed no mint") +} + +// TestSessionTokenWithoutLabelSendsNoLabel: an id alone is a session; the +// label is optional and is not sent empty. +func TestSessionTokenWithoutLabelSendsNoLabel(t *testing.T) { + as := startDeviceAS(t) + as.token = func(int) (int, string) { + return http.StatusOK, `{"access_token":"session","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `"}` + } + m := newDeviceTestManager(t, as.srv.URL) + storeAgent(t, m, liveAgentCredential(as.srv.URL+"/oauth/token")) + + token, err := m.SessionAccessToken(context.Background(), testSessionID, "") + require.NoError(t, err) + assert.Equal(t, "session", token) + + calls := as.tokenCalls() + require.Len(t, calls, 1) + assert.Equal(t, testSessionID, calls[0].Get("launch_id")) + _, sent := calls[0]["launch_label"] + assert.False(t, sent, "an absent label must not be sent as an empty one") +} + +// TestSessionTokenFromAServerThatIgnoresTheSessionFailsClosed: a Basecamp +// that predates session attribution answers with an ordinary token. That +// token is not bound to the session, so it is discarded, not printed. +func TestSessionTokenFromAServerThatIgnoresTheSessionFailsClosed(t *testing.T) { + for name, body := range map[string]string{ + "no echo": `{"access_token":"unbound","token_type":"Bearer","expires_in":3600}`, + "other id": `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"ffffffffffffffffffffffffffffffff","launch_label":"coworker@box"}`, + "other label": `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `","launch_label":"someone-else"}`, + "label not given": `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `"}`, + } { + t.Run(name, func(t *testing.T) { + as := startDeviceAS(t) + as.token = func(int) (int, string) { return http.StatusOK, body } + m := newDeviceTestManager(t, as.srv.URL) + key := storeAgent(t, m, liveAgentCredential(as.srv.URL+"/oauth/token")) + + token, err := m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.Error(t, err) + assert.Empty(t, token) + assert.NotContains(t, err.Error(), "unbound", "the unbound token must not leak into the error") + assert.Contains(t, err.Error(), testSessionID) + assert.Contains(t, err.Error(), "predates agent session attribution") + + stored, loadErr := m.store.Load(key) + require.NoError(t, loadErr) + assert.Equal(t, "shared-cached", stored.AccessToken) + assert.Nil(t, stored.MintHold, "an unattributing server is no verdict on the credential") + }) + } +} + +// TestSessionTokenRefusedForItsSessionNamesItAndHoldsNothing: a refusal of +// the session's own parameters is about this request, not the credential, +// so nothing is held against the profile. +func TestSessionTokenRefusedForItsSessionNamesItAndHoldsNothing(t *testing.T) { + as := startDeviceAS(t) + as.token = func(int) (int, string) { + return http.StatusBadRequest, `{"error":"invalid_request","error_description":"launch_label is too long"}` + } + m := newDeviceTestManager(t, as.srv.URL) + key := storeAgent(t, m, liveAgentCredential(as.srv.URL+"/oauth/token")) + + _, err := m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.Error(t, err) + assert.Contains(t, err.Error(), testSessionID) + assert.Contains(t, err.Error(), "invalid_request") + + stored, loadErr := m.store.Load(key) + require.NoError(t, loadErr) + assert.Nil(t, stored.MintHold) + assert.Equal(t, "shared-cached", stored.AccessToken) +} + +// TestSessionTokenRefusalOfTheCredentialIsHeld: a session mint presents the +// same client secret the shared path does, so the token endpoint's verdict +// on that secret is remembered for both — a dozen sandbox launches must not +// each keep presenting a dead secret. +func TestSessionTokenRefusalOfTheCredentialIsHeld(t *testing.T) { + as := startDeviceAS(t) + as.token = func(int) (int, string) { + return http.StatusUnauthorized, `{"error":"invalid_client"}` + } + m := newDeviceTestManager(t, as.srv.URL) + key := storeAgent(t, m, liveAgentCredential(as.srv.URL+"/oauth/token")) + + _, err := m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.Error(t, err) + assert.ErrorIs(t, err, ErrAgentCredentialRefused) + + stored, loadErr := m.store.Load(key) + require.NoError(t, loadErr) + require.NotNil(t, stored.MintHold) + assert.Equal(t, mintHoldRefused, stored.MintHold.Kind) + + _, err = m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.Error(t, err) + assert.ErrorIs(t, err, errMintHeld) + assert.Len(t, as.tokenCalls(), 1, "a held credential was presented again") +} + +// TestSessionTokenNeedsAnAgentProfile: only an agent credential mints for +// itself; a person's login has no session to attribute and is not handed +// out in place of one. +func TestSessionTokenNeedsAnAgentProfile(t *testing.T) { + as := startDeviceAS(t) + m := newDeviceTestManager(t, as.srv.URL) + storeAgent(t, m, &Credentials{AccessToken: "person-token", RefreshToken: "r", ExpiresAt: time.Now().Add(time.Hour).Unix()}) + + token, err := m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") + require.Error(t, err) + assert.Empty(t, token) + assert.Equal(t, output.CodeUsage, output.AsError(err).Code) + assert.Empty(t, as.tokenCalls()) +} + +// TestSessionTokenValidatesItsIdentityBeforeSending: the same rules the +// server holds a session to are checked before anything goes out. +func TestSessionTokenValidatesItsIdentityBeforeSending(t *testing.T) { + for name, tc := range map[string]struct{ id, label string }{ + "empty id": {"", "coworker@box"}, + "short id": {"0123456789abcdef", ""}, + "uppercase id": {strings.ToUpper(testSessionID), ""}, + "non-hex id": {"0123456789abcdef0123456789abcdeg", ""}, + "label too long": {testSessionID, strings.Repeat("x", 101)}, + "blank label": {testSessionID, " "}, + "control in label": {testSessionID, "coworker\nbox"}, + "bidi override": {testSessionID, "coworker\u202ebox"}, + "line separator": {testSessionID, "coworker\u2028box"}, + "invalid utf-8 label": {testSessionID, "coworker\xffbox"}, + "unassigned codepoint": {testSessionID, "coworker\U000E0080box"}, + } { + t.Run(name, func(t *testing.T) { + as := startDeviceAS(t) + m := newDeviceTestManager(t, as.srv.URL) + storeAgent(t, m, liveAgentCredential(as.srv.URL+"/oauth/token")) + + _, err := m.SessionAccessToken(context.Background(), tc.id, tc.label) + require.Error(t, err) + assert.Equal(t, output.CodeUsage, output.AsError(err).Code) + assert.Empty(t, as.tokenCalls()) + }) + } + + assert.NoError(t, ValidateSession(testSessionID, strings.Repeat("é", 100)), "100 characters, not bytes") + assert.NoError(t, ValidateSession(testSessionID, "claude · coworker")) +} diff --git a/internal/commands/auth.go b/internal/commands/auth.go index 9f3bcf4d4..f1c72b228 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -519,6 +519,7 @@ func newAuthRefreshCmd() *cobra.Command { func newAuthTokenCmd() *cobra.Command { var stored bool + var sessionID, sessionLabel string cmd := &cobra.Command{ Use: "token", @@ -539,6 +540,17 @@ Get tokens for different profiles: The --stored flag ignores BASECAMP_TOKEN and uses stored OAuth credentials: basecamp auth token --stored +An agent profile can mint a token of its own for one session — one launch +of a sandboxed agent, say — which Basecamp records on the token and logs +beside the agent, so concurrent sessions can be told apart: + basecamp -P agent auth token --stored \ + --session-id "$(openssl rand -hex 16)" --session-label "coworker@laptop" + +A session token is minted fresh on every call and never cached; the caller +holds it for as long as it means to. A Basecamp that does not record the +session gets no token from this: the command fails rather than hand out one +that is not bound to the session. + Output modes: basecamp auth token # Raw token (default, for shell substitution) basecamp auth token --json # JSON envelope with token in data field @@ -552,7 +564,17 @@ Output modes: var token string var err error - if stored { + session := sessionID != "" || sessionLabel != "" + switch { + case session && sessionID == "": + return output.ErrUsage("--session-label needs --session-id") + case session && !stored: + return output.ErrUsage("--session-id needs --stored: a session token is minted from the stored agent credential, never taken from BASECAMP_TOKEN") + } + + if session { + token, err = app.Auth.SessionAccessToken(cmd.Context(), sessionID, sessionLabel) + } else if stored { // Use stored OAuth credentials (ignores BASECAMP_TOKEN env) // This also handles auto-refresh for near-expiry tokens token, err = app.Auth.StoredAccessToken(cmd.Context()) @@ -568,7 +590,14 @@ Output modes: // Output raw token by default for backwards compatibility with shell scripts. // Only use JSON envelope when --json/--agent/--jq is explicitly requested. if app.Flags.JSON || app.Flags.Agent || app.Flags.JQFilter != "" { - return app.OK(map[string]string{"token": token}) + data := map[string]string{"token": token} + if session { + data["session_id"] = sessionID + if sessionLabel != "" { + data["session_label"] = sessionLabel + } + } + return app.OK(data) } // Raw output: print token directly, with optional stats on stderr @@ -578,6 +607,8 @@ Output modes: } cmd.Flags().BoolVar(&stored, "stored", false, "Use stored OAuth token, ignoring BASECAMP_TOKEN env var") + cmd.Flags().StringVar(&sessionID, "session-id", "", "Mint a fresh agent token for this session (32 lowercase hex characters); needs --stored") + cmd.Flags().StringVar(&sessionLabel, "session-label", "", "Label for the session (at most 100 characters); needs --session-id") return cmd } diff --git a/internal/commands/auth_token_session_test.go b/internal/commands/auth_token_session_test.go new file mode 100644 index 000000000..8f310d92c --- /dev/null +++ b/internal/commands/auth_token_session_test.go @@ -0,0 +1,95 @@ +package commands + +import ( + "encoding/json" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/auth" + "github.com/basecamp/basecamp-cli/internal/config" + "github.com/basecamp/basecamp-cli/internal/output" +) + +const tokenSessionID = "0123456789abcdef0123456789abcdef" + +// storeAgentProfile seeds the "agent" profile with a live shared token. +func storeAgentProfile(t *testing.T, store *auth.Store, tokenEndpoint string) { + t.Helper() + require.NoError(t, store.Save("profile:agent", &auth.Credentials{ + AccessToken: "shared-cached", + OAuthType: "agent", + ClientID: "agent-client", + ClientSecret: "agent-secret", + TokenEndpoint: tokenEndpoint, + Scope: "full", + ExpiresAt: time.Now().Add(time.Hour).Unix(), + })) +} + +func TestAuthTokenSessionMintsForTheSession(t *testing.T) { + as := startAgentAS(t) + as.token = func() (int, string) { + return http.StatusOK, `{"access_token":"session-token","token_type":"Bearer","expires_in":3600,"scope":"full",` + + `"launch_id":"` + tokenSessionID + `","launch_label":"coworker@box"}` + } + app, buf := agentLoginApp(t, as, &config.Config{ActiveProfile: "agent"}) + storeAgentProfile(t, app.Auth.GetStore(), as.srv.URL+"/oauth/tokens") + app.Flags.JSON = true + + err := executeCommand(NewAuthCmd(), app, "token", "--stored", "--session-id", tokenSessionID, "--session-label", "coworker@box") + require.NoError(t, err) + + var envelope struct { + Data map[string]string `json:"data"` + } + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope), buf.String()) + assert.Equal(t, map[string]string{ + "token": "session-token", + "session_id": tokenSessionID, + "session_label": "coworker@box", + }, envelope.Data) + + calls := as.calls() + require.Len(t, calls, 1) + assert.Equal(t, tokenSessionID, calls[0].Get("launch_id")) + assert.Equal(t, "coworker@box", calls[0].Get("launch_label")) +} + +func TestAuthTokenWithoutSessionKeepsItsShape(t *testing.T) { + as := startAgentAS(t) + app, buf := agentLoginApp(t, as, &config.Config{ActiveProfile: "agent"}) + storeAgentProfile(t, app.Auth.GetStore(), as.srv.URL+"/oauth/tokens") + app.Flags.JSON = true + + require.NoError(t, executeCommand(NewAuthCmd(), app, "token", "--stored")) + + var envelope struct { + Data map[string]string `json:"data"` + } + require.NoError(t, json.Unmarshal(buf.Bytes(), &envelope), buf.String()) + assert.Equal(t, map[string]string{"token": "shared-cached"}, envelope.Data) + assert.Empty(t, as.calls()) +} + +func TestAuthTokenSessionFlagUsage(t *testing.T) { + for name, args := range map[string][]string{ + "label without id": {"token", "--stored", "--session-label", "coworker@box"}, + "id without stored": {"token", "--session-id", tokenSessionID}, + "malformed id": {"token", "--stored", "--session-id", "not-hex"}, + } { + t.Run(name, func(t *testing.T) { + as := startAgentAS(t) + app, _ := agentLoginApp(t, as, &config.Config{ActiveProfile: "agent"}) + storeAgentProfile(t, app.Auth.GetStore(), as.srv.URL+"/oauth/tokens") + + err := executeCommand(NewAuthCmd(), app, args...) + require.Error(t, err) + assert.Equal(t, output.CodeUsage, output.AsError(err).Code, err.Error()) + assert.Empty(t, as.calls()) + }) + } +} From 36c189d8a0a2191d0f016f1599af92e625c59ddc Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 15:46:38 -0700 Subject: [PATCH 2/6] Refuse an empty --session-id rather than serve the shared token A launcher that passes --session-id "$ID" with ID unset got the profile's shared, unattributed token, because session mode was decided by the flag's value. It is now decided by whether the flag was given, so an empty id is a usage error like any other malformed one. --- internal/commands/auth.go | 8 ++++++-- internal/commands/auth_token_session_test.go | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/internal/commands/auth.go b/internal/commands/auth.go index f1c72b228..95feeb723 100644 --- a/internal/commands/auth.go +++ b/internal/commands/auth.go @@ -564,9 +564,13 @@ Output modes: var token string var err error - session := sessionID != "" || sessionLabel != "" + // Whether a flag was GIVEN decides, not its value: an empty + // "--session-id $ID" is a launcher that failed to make one, and + // it is refused rather than handed the shared token. + idGiven := cmd.Flags().Changed("session-id") + session := idGiven || cmd.Flags().Changed("session-label") switch { - case session && sessionID == "": + case session && !idGiven: return output.ErrUsage("--session-label needs --session-id") case session && !stored: return output.ErrUsage("--session-id needs --stored: a session token is minted from the stored agent credential, never taken from BASECAMP_TOKEN") diff --git a/internal/commands/auth_token_session_test.go b/internal/commands/auth_token_session_test.go index 8f310d92c..7a92a9f3c 100644 --- a/internal/commands/auth_token_session_test.go +++ b/internal/commands/auth_token_session_test.go @@ -80,6 +80,10 @@ func TestAuthTokenSessionFlagUsage(t *testing.T) { "label without id": {"token", "--stored", "--session-label", "coworker@box"}, "id without stored": {"token", "--session-id", tokenSessionID}, "malformed id": {"token", "--stored", "--session-id", "not-hex"}, + // An unset variable in "--session-id $ID" must not fall back to the + // shared, unattributed token. + "empty id": {"token", "--stored", "--session-id", ""}, + "empty id and label": {"token", "--stored", "--session-id", "", "--session-label", ""}, } { t.Run(name, func(t *testing.T) { as := startAgentAS(t) From 0eb4b379756360ce4e2e6469dd046f7caecd234f Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 15:52:01 -0700 Subject: [PATCH 3/6] Accept a session label character newer than this build's Unicode tables bc3 refuses exactly control, format, private-use, surrogate and line or paragraph separator characters in a launch label, and no longer refuses unassigned code points: a host's Unicode tables can be newer than the server's, and a refused label costs the launch its whole token. The local check now refuses the same set, named category by category, since Go's unicode.C table also covers code points it doesn't know. --- internal/auth/agent_launch.go | 17 ++++++++--------- internal/auth/agent_launch_test.go | 26 +++++++++++++++----------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/internal/auth/agent_launch.go b/internal/auth/agent_launch.go index a21831926..1b162c4aa 100644 --- a/internal/auth/agent_launch.go +++ b/internal/auth/agent_launch.go @@ -88,16 +88,15 @@ func ValidateSession(id, label string) error { return nil } -// labelRune reports whether r may appear in a session label: an assigned -// character outside the "other" category (control, format, private use, -// surrogate) and the line and paragraph separators. The label is shown to -// people, so nothing that reorders, hides or breaks the text around it. +// labelRune reports whether r may appear in a session label: anything but +// the "other" category (control, format, private use, surrogate) and the +// line and paragraph separators — exactly the set bc3 refuses. The label is +// shown to people, so nothing that reorders, hides or breaks the text around +// it. Unassigned code points are deliberately allowed: a host's Unicode +// tables can be newer than this build's or the server's, and a label refused +// over that skew costs the launch its whole token. func labelRune(r rune) bool { - if unicode.In(r, unicode.C, unicode.Zl, unicode.Zp) { - return false - } - // Unassigned code points are in no category table at all. - return unicode.In(r, unicode.L, unicode.M, unicode.N, unicode.P, unicode.S, unicode.Zs) + return !unicode.In(r, unicode.Cc, unicode.Cf, unicode.Co, unicode.Cs, unicode.Zl, unicode.Zp) } // addTo puts the session on a client_credentials form. A nil launch is the diff --git a/internal/auth/agent_launch_test.go b/internal/auth/agent_launch_test.go index e5988cf4c..d00c6c8bc 100644 --- a/internal/auth/agent_launch_test.go +++ b/internal/auth/agent_launch_test.go @@ -183,17 +183,16 @@ func TestSessionTokenNeedsAnAgentProfile(t *testing.T) { // server holds a session to are checked before anything goes out. func TestSessionTokenValidatesItsIdentityBeforeSending(t *testing.T) { for name, tc := range map[string]struct{ id, label string }{ - "empty id": {"", "coworker@box"}, - "short id": {"0123456789abcdef", ""}, - "uppercase id": {strings.ToUpper(testSessionID), ""}, - "non-hex id": {"0123456789abcdef0123456789abcdeg", ""}, - "label too long": {testSessionID, strings.Repeat("x", 101)}, - "blank label": {testSessionID, " "}, - "control in label": {testSessionID, "coworker\nbox"}, - "bidi override": {testSessionID, "coworker\u202ebox"}, - "line separator": {testSessionID, "coworker\u2028box"}, - "invalid utf-8 label": {testSessionID, "coworker\xffbox"}, - "unassigned codepoint": {testSessionID, "coworker\U000E0080box"}, + "empty id": {"", "coworker@box"}, + "short id": {"0123456789abcdef", ""}, + "uppercase id": {strings.ToUpper(testSessionID), ""}, + "non-hex id": {"0123456789abcdef0123456789abcdeg", ""}, + "label too long": {testSessionID, strings.Repeat("x", 101)}, + "blank label": {testSessionID, " "}, + "control in label": {testSessionID, "coworker\nbox"}, + "bidi override": {testSessionID, "coworker\u202ebox"}, + "line separator": {testSessionID, "coworker\u2028box"}, + "invalid utf-8 label": {testSessionID, "coworker\xffbox"}, } { t.Run(name, func(t *testing.T) { as := startDeviceAS(t) @@ -209,4 +208,9 @@ func TestSessionTokenValidatesItsIdentityBeforeSending(t *testing.T) { assert.NoError(t, ValidateSession(testSessionID, strings.Repeat("é", 100)), "100 characters, not bytes") assert.NoError(t, ValidateSession(testSessionID, "claude · coworker")) + // A character newer than this build's Unicode tables is accepted, as bc3 + // accepts it: refusing it would cost the launch its whole token over a + // table the two sides need not share. + assert.NoError(t, ValidateSession(testSessionID, "coworker \U0001FAE9")) + assert.NoError(t, ValidateSession(testSessionID, "coworker\U000E0080box")) } From 04de1057173f9abcef28140184ea9319181e59e8 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 21:11:35 -0700 Subject: [PATCH 4/6] Hold every session mint to its id and label, not just the first The mock echoes the launch id back whatever the request sent, so a second mint that dropped it would have passed. Each recorded mint is now checked. --- internal/auth/agent_launch_test.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/auth/agent_launch_test.go b/internal/auth/agent_launch_test.go index d00c6c8bc..2da758d63 100644 --- a/internal/auth/agent_launch_test.go +++ b/internal/auth/agent_launch_test.go @@ -48,10 +48,14 @@ func TestSessionTokenMintsFreshAndLeavesTheSharedTokenAlone(t *testing.T) { calls := as.tokenCalls() require.Len(t, calls, 2) - assert.Equal(t, "client_credentials", calls[0].Get("grant_type")) - assert.Equal(t, testSessionID, calls[0].Get("launch_id")) - assert.Equal(t, "coworker@box", calls[0].Get("launch_label")) - assert.Equal(t, AgentResourceURNPrefix+"42", calls[0].Get("resource")) + // Every mint carries the session, not just the first: the mock echoes + // the id back whatever was sent, so only the request shows it. + for i, call := range calls { + assert.Equal(t, "client_credentials", call.Get("grant_type"), "mint %d", i+1) + assert.Equal(t, testSessionID, call.Get("launch_id"), "mint %d", i+1) + assert.Equal(t, "coworker@box", call.Get("launch_label"), "mint %d", i+1) + assert.Equal(t, AgentResourceURNPrefix+"42", call.Get("resource"), "mint %d", i+1) + } after, err := m.store.Load(key) require.NoError(t, err) From c56bff331bad6df51d3a49a0e31ca4891ec9fb77 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 21:12:12 -0700 Subject: [PATCH 5/6] Follow main's rename of the mint hold to a renewal hold #854 renamed MintHold and its helpers to RenewalHold after this branch was cut, so the session mint and its tests named things that no longer exist. --- internal/auth/agent_launch.go | 2 +- internal/auth/agent_launch_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/auth/agent_launch.go b/internal/auth/agent_launch.go index 1b162c4aa..ba3400322 100644 --- a/internal/auth/agent_launch.go +++ b/internal/auth/agent_launch.go @@ -192,7 +192,7 @@ func (m *Manager) SessionAccessToken(ctx context.Context, id, label string) (str if err != nil { if hold != nil { lockErr := m.store.withKeyLock(ctx, credKey, func() error { - m.rememberMintHold(credKey, hold) + m.rememberRenewalHold(credKey, hold) return nil }) if lockErr != nil { diff --git a/internal/auth/agent_launch_test.go b/internal/auth/agent_launch_test.go index 2da758d63..f1d22d059 100644 --- a/internal/auth/agent_launch_test.go +++ b/internal/auth/agent_launch_test.go @@ -114,7 +114,7 @@ func TestSessionTokenFromAServerThatIgnoresTheSessionFailsClosed(t *testing.T) { stored, loadErr := m.store.Load(key) require.NoError(t, loadErr) assert.Equal(t, "shared-cached", stored.AccessToken) - assert.Nil(t, stored.MintHold, "an unattributing server is no verdict on the credential") + assert.Nil(t, stored.RenewalHold, "an unattributing server is no verdict on the credential") }) } } @@ -137,7 +137,7 @@ func TestSessionTokenRefusedForItsSessionNamesItAndHoldsNothing(t *testing.T) { stored, loadErr := m.store.Load(key) require.NoError(t, loadErr) - assert.Nil(t, stored.MintHold) + assert.Nil(t, stored.RenewalHold) assert.Equal(t, "shared-cached", stored.AccessToken) } @@ -159,12 +159,12 @@ func TestSessionTokenRefusalOfTheCredentialIsHeld(t *testing.T) { stored, loadErr := m.store.Load(key) require.NoError(t, loadErr) - require.NotNil(t, stored.MintHold) - assert.Equal(t, mintHoldRefused, stored.MintHold.Kind) + require.NotNil(t, stored.RenewalHold) + assert.Equal(t, renewalHoldRefused, stored.RenewalHold.Kind) _, err = m.SessionAccessToken(context.Background(), testSessionID, "coworker@box") require.Error(t, err) - assert.ErrorIs(t, err, errMintHeld) + assert.ErrorIs(t, err, errRenewalHeld) assert.Len(t, as.tokenCalls(), 1, "a held credential was presented again") } From 40a798b79a403fc62dddf5b4a2c538b2027e285d Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 22:23:20 -0700 Subject: [PATCH 6/6] Say which way a session echo missed, not always that Basecamp predates sessions A server that echoed this session's id but a different label, or none, was still told it predates agent session attribution, which its own echo contradicts. Only a missing id says that now; an id for another session and a label that came back wrong or not at all each say so. --- internal/auth/agent_launch.go | 20 ++++++++++++++++---- internal/auth/agent_launch_test.go | 26 +++++++++++++++++++------- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/internal/auth/agent_launch.go b/internal/auth/agent_launch.go index ba3400322..e7c5ef8b7 100644 --- a/internal/auth/agent_launch.go +++ b/internal/auth/agent_launch.go @@ -135,14 +135,26 @@ func (l *agentLaunch) requireEcho(status int, body []byte) error { return nil } + // Say which way the echo missed. Only a server that echoed no id at + // all is one that predates sessions; one that echoed this id records + // them, and what it got wrong is the label. msg := fmt.Sprintf("Basecamp minted a token but did not bind it to session %s, so it was discarded", l.id) - if echoed.ID == nil { + hint := "A session token needs a Basecamp that records agent sessions; without one, leave out --session-id and --session-label" + switch { + case echoed.ID == nil: msg += ": this Basecamp predates agent session attribution" - } else { - msg += ": this Basecamp predates agent session attribution, or answered for a different session" + case !idMatches: + msg += ": Basecamp answered for a different session" + hint = "Run it again; if it keeps answering for another session, report it" + case echoed.Label == nil: + msg += ": Basecamp recorded the session but did not record the label" + hint = "Leave out --session-label to mint for the session alone" + default: + msg += ": Basecamp recorded a different label for the session" + hint = "Leave out --session-label to mint for the session alone" } e := output.ErrAPI(status, msg) - e.Hint = "A session token needs a Basecamp that records agent sessions; without one, leave out --session-id and --session-label" + e.Hint = hint e.Cause = errLaunchNotRecorded return e } diff --git a/internal/auth/agent_launch_test.go b/internal/auth/agent_launch_test.go index f1d22d059..236d3b534 100644 --- a/internal/auth/agent_launch_test.go +++ b/internal/auth/agent_launch_test.go @@ -90,14 +90,25 @@ func TestSessionTokenWithoutLabelSendsNoLabel(t *testing.T) { // TestSessionTokenFromAServerThatIgnoresTheSessionFailsClosed: a Basecamp // that predates session attribution answers with an ordinary token. That -// token is not bound to the session, so it is discarded, not printed. +// token is not bound to the session, so it is discarded, not printed. The +// message says which way the echo missed: a server that echoed the id +// does record sessions, so it is not told it predates them. func TestSessionTokenFromAServerThatIgnoresTheSessionFailsClosed(t *testing.T) { - for name, body := range map[string]string{ - "no echo": `{"access_token":"unbound","token_type":"Bearer","expires_in":3600}`, - "other id": `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"ffffffffffffffffffffffffffffffff","launch_label":"coworker@box"}`, - "other label": `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `","launch_label":"someone-else"}`, - "label not given": `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `"}`, + for name, tc := range map[string]struct{ body, says, never string }{ + "no echo": { + `{"access_token":"unbound","token_type":"Bearer","expires_in":3600}`, + "predates agent session attribution", "different"}, + "other id": { + `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"ffffffffffffffffffffffffffffffff","launch_label":"coworker@box"}`, + "answered for a different session", "predates"}, + "other label": { + `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `","launch_label":"someone-else"}`, + "recorded a different label", "predates"}, + "label not given": { + `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `"}`, + "did not record the label", "predates"}, } { + body := tc.body t.Run(name, func(t *testing.T) { as := startDeviceAS(t) as.token = func(int) (int, string) { return http.StatusOK, body } @@ -109,7 +120,8 @@ func TestSessionTokenFromAServerThatIgnoresTheSessionFailsClosed(t *testing.T) { assert.Empty(t, token) assert.NotContains(t, err.Error(), "unbound", "the unbound token must not leak into the error") assert.Contains(t, err.Error(), testSessionID) - assert.Contains(t, err.Error(), "predates agent session attribution") + assert.Contains(t, err.Error(), tc.says) + assert.NotContains(t, err.Error(), tc.never) stored, loadErr := m.store.Load(key) require.NoError(t, loadErr)