Import personal access tokens with auth login --with-token - #681
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
github-actions
Bot
added
commands
A bot's Basecamp identity should never come from a browser sign-in on a shared machine: whoever's session the browser holds is who the profile quietly becomes, and nothing checks. Basecamp now mints personal access tokens, so the CLI can take one from a secret store and refuse to keep it unless it authenticates as the expected identity. `basecamp auth login --with-token -P <profile> --account <id>` reads the token from stdin (a terminal is refused, with the `op read … |` shape shown), stores it under the profile as a non-expiring bc5 credential — ExpiresAt 0 is the path AccessToken already never refreshes — and creates the profile when --account names its account. The profile entry is written only after /my/profile.json and the authorization endpoint have answered for the token; a rejected token leaves the profile exactly as it was found, previous credential included. Profile registration is factored out of `profile create` so both paths write the same entry, and the root pre-run lets a may-create command name a profile that does not exist yet. `--expect-identity <id>` makes every login assertive — browser and device flows included — discarding the new credential on a mismatch. `--json` returns the envelope (profile, account, identity, person, oauth_type, scope, expires_at null), and the interactive flows now refuse machine output modes up front instead of printing prose and blocking (the machine-mode half of #669). `BASECAMP_OAUTH_ISSUER` pins the BC5 authorization server and skips discovery, so a device login reaches a server that is piloting the client while its metadata is still dark. It is a temporary escape hatch and is documented as one. `--login-hint <email>` is wired through LoginOptions; the pinned SDK cannot put it on the wire yet (basecamp/basecamp-sdk#841 adds the option), so this build tells the user the hint instead of sending it, and Launchpad ignores it.
jeremy
force-pushed
the
auth-login-with-token
branch
from
September 2, 2026 19:59
0282f11 to
b1ddc87
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed
You’re at about 97% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0282f1166d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical account-scoped verification and non-atomic credential rollback issues must be resolved before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds personal access-token login, identity assertions, OAuth issuer pinning, and safer non-interactive authentication behavior.
Changes:
- Adds PAT import through named profiles with identity verification and JSON output.
- Adds login hints, expected-identity checks, and machine-output guards.
- Updates profile handling, documentation, tests, and CLI surface metadata.
File summaries
| File | Description |
|---|---|
skills/basecamp/SKILL.md |
Documents token imports and identity assertions. |
README.md |
Documents PAT imports and issuer pinning. |
internal/stdinarg/stdinarg.go |
Adds terminal-input detection. |
internal/stdinarg/stdinarg_test.go |
Tests terminal detection. |
internal/commands/profile.go |
Extracts profile registration helpers. |
internal/commands/auth.go |
Implements token login and identity verification. |
internal/commands/auth_login_test.go |
Tests token-login behavior and failures. |
internal/cli/root.go |
Allows login to create named profiles. |
internal/cli/root_test.go |
Tests profile creation resolution. |
internal/auth/device_test.go |
Tests issuer pinning, hints, and token credentials. |
internal/auth/auth.go |
Adds token import and OAuth options. |
internal/auth/auth_test.go |
Tests Launchpad hint handling. |
e2e/auth.bats |
Adds CLI-level authentication tests. |
.surface |
Records the new login flags. |
Review details
Suppressed comments (3)
README.md:200
- This implies the approval page receives
--login-hint, but the current implementation only prints the email locally and sends no hint to the server. Clarify that limitation so operators do not rely on the page being preselected.
`--expect-identity <id>` makes any login assert who it authenticated as: on a
mismatch the new credential is discarded (a profile's previous credential is
kept) and the command exits non-zero. `--login-hint <email>` suggests which
account to sign in as on the device-flow approval page.
internal/auth/auth.go:859
- The comment says the issuer is never echoed, but the success path logs the full validated value at line 873 (and the test requires that output). Either remove the value from the log to honor the promise or clarify that only rejected values are not echoed; the current security documentation is self-contradictory.
issuer = strings.TrimRight(strings.TrimSpace(issuer), "/")
u, err := url.Parse(issuer)
internal/commands/auth_login_test.go:381
- This assertion cannot pass for an existing profile: the server scope is written only to the credential by
setStoredScope;existing.Scopeis never mutated. Since the PR also promises that an existing profile entry stays untouched, assert that behavior here rather than expecting an in-memory rewrite.
assert.Equal(t, "51177542", creds.UserID, "identity survives the scope correction")
- Files reviewed: 14/14 changed files
- Comments generated: 14
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved verification, persistence, account-scoping, and output-sanitization issues can violate the authentication guarantees.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (9)
README.md:200
- This currently implies the hint reaches the approval page, but this release only prints the hint locally; the pending SDK change is what will put
login_hinton the device authorization request. State that limitation here so operators do not assume the server will select the intended identity.
kept) and the command exits non-zero. `--login-hint <email>` suggests which
account to sign in as on the device-flow approval page.
internal/auth/auth.go:350
- This API comment says
LoginHintsteers the sign-in page, but the current implementation explicitly only announces it locally; the SDK support that would send it is still pending. Document the current behavior so callers do not rely on a hint reaching the authorization server.
// LoginHint names the account (email address) the user should sign in
// as; it steers the sign-in page and never authenticates on its own.
// Device flow only — Launchpad's authorization-code flow ignores it.
internal/auth/auth.go:856
- This comment says the issuer is never echoed, but line 870 logs every validated issuer and the success test asserts that output. Clarify that only rejected values are suppressed.
// again by loginDevice before any POST. The value is never echoed: like a
// base URL, it can carry userinfo or a query string.
internal/commands/auth.go:614
/my/profile.jsonis account-scoped, but the raw SDK client does not add an account prefix (internal/appctx/context.go:121explicitly requiresapp.Account()for such calls;internal/names/resolver_test.go:607expects/{account}/my/profile.json). In production this requests the resource root and the strict token-import path rejects every valid token. Resolve and validate the effective account, then fetch the person through the account client; the tests should assert the account-prefixed path.
resp, err := app.SDK.Get(ctx, "/my/profile.json")
internal/commands/auth.go:564
- Every
Loaderror is treated as “no previous credential.” A transient keyring/read error can therefore be followed by a successful overwrite, and a failed identity check then deletes the newly written value instead of restoring the previous credential, violating the rollback guarantee. Distinguish credential-not-found from other load errors and abort before login/import when the snapshot cannot be read.
prev, err := store.Load(key)
if err != nil {
prev = nil
}
internal/commands/auth.go:593
NameandEmailcome from server responses and this label is written directly to terminal output, so embedded OSC/CSI or newlines can inject terminal content. Sanitize both fields withrichtext.SanitizeSingleLinebefore composing the one-line label, consistent withinternal/richtext/sanitize.go:35-48.
label := l.Name
if l.Email != "" {
label += " <" + l.Email + ">"
internal/commands/auth.go:315
Logindurably replaces the profile credential before--expect-identityis evaluated. If the process receives SIGINT/SIGTERM or crashes after OAuth completes but before verification/rollback, the possibly wrong browser identity remains stored—the exact shared-browser case this assertion is meant to prevent. Stage the new credential (for example via a temporary/in-memory token provider), verify it, and commit it only after the expectation passes.
restore := credentialRestorer(app)
result, err := app.Auth.Login(cmd.Context(), auth.LoginOptions{
internal/commands/auth.go:417
- The imported token is persisted before either identity request runs. A termination or crash during verification leaves an unverified token behind (and permanently overwrites an existing profile credential), despite the command's guarantee that nothing is kept until verification succeeds. Verify through a temporary token provider first, then save the credential as the final commit step.
restore := credentialRestorer(app)
if err := app.Auth.ImportToken(token, scope); err != nil {
return err
}
who, err := verifyLoginIdentity(cmd.Context(), app, expectIdentity, true)
internal/commands/auth.go:340
- All mismatch-and-rollback tests added here invoke
--with-token; the new browser/device--expect-identitybranch is untested. Add a command-level OAuth test that completes a device or browser login with a mismatching identity and verifies the previous credential is restored, since this is a core behavior advertised for every login flow.
who, err := verifyLoginIdentity(cmd.Context(), app, expectIdentity, expectIdentity != "")
if err != nil {
restore(cmd.ErrOrStderr())
return err
- Files reviewed: 14/14 changed files
- Comments generated: 6
- Review effort level: Balanced
jeremy
force-pushed
the
auth-login-with-token
branch
from
September 2, 2026 20:18
0f1a9be to
b4d1259
Compare
Review of the token import found the rollback doing the wrong job: the credential was written under the live profile key, checked, and then restored or deleted — a window where another command could pick up an unverified token, a restore that could itself fail, and a success line printed before the check. Turn it around: the candidate token gets a client of its own (App.SDKClientFor, same transport, hooks and user agent as the real one), proves itself, and only then is stored. LoginOptions.Verify runs that check inside the device and Launchpad flows before their store.Save, so --expect-identity on a browser login stores nothing on a mismatch; the snapshot/restore code is gone. The check now covers the account, not only the identity. Production serves /my/profile.json only under an account prefix (the unscoped path 404s — the old "Logged in as" line had been failing silently), so the person lookup goes through the account-scoped People().Me, and that account must be numeric, must be the effective one (--account or BASECAMP_ACCOUNT_ID override the profile binding at runtime, so the mismatch guard compares against that), must appear non-expired in the authorization document, and for a new profile must have been given explicitly rather than inherited from the operator's config. A reported scope outside read/full is refused rather than stored. Smaller findings from the same round: --scope and --expect-identity are validated before stdin is consumed; --expect-identity compares parsed integers; the envelope reports the identity's own email and marks an existing default profile as default; server-supplied names, the login hint and the pinned issuer are reduced to one line before reaching the terminal; an unregistered profile name is no longer used as a cache path component; the e2e cases assert exact error and code with BASECAMP_PROFILE cleared; README and the doctor skill describe the token import, the identity assertion, and that --login-hint is announced, not sent.
jeremy
force-pushed
the
auth-login-with-token
branch
from
September 2, 2026 20:21
b4d1259 to
14c33b8
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Four moderate authentication correctness and credential-cleanup issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 16/16 changed files
- Comments generated: 4
- Review effort level: Balanced
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14c33b8a1a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Five moderate authentication and validation issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
internal/commands/auth.go:653
- This account verification is unconditional even when
strictis false. As a result, an ordinary login without--expect-identitynow aborts before storing its credential whenever the authorization document omits the configured account or the subsequent person lookup fails, contradicting the intended best-effort identity lookup and regressing previously successful logins. Only return these verification errors in strict mode; otherwise retain the authorization identity and continue.
if v.account != "" {
if !authorizesAccount(info, v.account) {
return output.ErrAuth(fmt.Sprintf("%s cannot access account %s (authorized: %s); nothing was stored", who.label(), v.account, authorizedAccountIDs(info)))
internal/commands/auth.go:348
- When no account is configured, verification can populate
whofrom/authorization.jsonbut leavesPersonIDat zero. This then overwrites the newly stored OAuth credential'suser_idwith the literal"0", whichauth statusandprofile showexpose as if it were a real person ID. Preserve the previous best-effort behavior by writing identity metadata only after an account-scoped person was resolved.
_ = app.Auth.SetUserIdentity(strconv.FormatInt(who.PersonID, 10), who.Email)
internal/commands/auth.go:674
- This textual comparison also rejects a valid non-canonical account spelling such as
--account 0999, because the authorization document's numeric ID is formatted as999. Compare numerically withaccountIDsEqual; otherwise token import can fail even though the token authorizes the requested account.
if strconv.FormatInt(acct.ID, 10) == account && !acct.Expired {
- Files reviewed: 16/16 changed files
- Comments generated: 3
- Review effort level: Balanced
…ffort The import verifies a token for one account and base URL and stores it under a profile; those must be the same place. An existing profile's base URL must match the effective one (a BASECAMP_BASE_URL override would verify against one host and store for another), an unbound profile is bound to the explicitly given account alongside the token (other fields preserved), and account comparisons are numeric like the rest of the package. The profile entry is written before the credential, so a failure between the two leaves a visibly unauthenticated profile rather than an orphaned secret. Strict verification refuses an authorization document with no identity id. An ordinary browser or device login without --expect-identity is not the assertive kind: an account the token cannot reach, or a person lookup that fails, no longer discards the login — the identity line falls back to the authorization document and no person id is fabricated (nothing is written as user_id 0 either). Stdin accepts exactly one trailing line ending, the shape a pipe delivers, rather than trimming whatever whitespace surrounds the secret.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Token arguments must be rejected, and authorization-document expiry must be handled correctly.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 16/16 changed files
- Comments generated: 2
- Review effort level: Balanced
* origin/main: deps: bump github.com/yuin/goldmark in the go-dependencies group (#685) Update nix flake and plugin version for v0.10.0 (#684) Preserve template library breadcrumb context (#683) Add to-do list template library commands (#682) Add a CLI command to edit your profile (bio, title, out-of-office) (#679) feat(bubble-up): add bubble-up add/remove commands (#677) Resolve a bare numeric `cards <id>` to a clear error instead of silent exit-0 (#678) # Conflicts: # go.mod # go.sum # internal/version/sdk-provenance.json
main now checks that the vendored MCP model was synced from the same basecamp-sdk revision go.mod pins. This branch pins the post-v0.16.0 commit that carries the device-flow login hint, so the provenance moves to that commit. scripts/sync-mcp-model.sh against that checkout leaves behavior-model.json and openapi.json byte-identical to the v0.16.0 snapshot, and would describe the ref as go/v0.16.0-1-g47e7ca38; the ref is written as the go.mod pseudo-version instead, which is what the provenance test compares. A v0.16.1 tag would restore the tagged shape.
A token ending in a bare carriage return was accepted: the LF and CR suffixes were trimmed independently, so "token\r" lost its CR and passed the control-character check the contract says it must fail. The CR now goes only when it precedes the final LF; on its own it is left for the single-line check to refuse.
…irreversible The global config writers refuse a malformed file rather than rewrite it, but a top-level null decoded into a nil map and the next assignment panicked instead. It is refused like any other non-object value. Two callers ran the refusal after a step that cannot be taken back: the token import read stdin and verified the token against the server before registering or binding the profile discovered the file was unusable, and profile delete removed the credential first. Both now prove the file can take the write up front, as profile create already did.
…Auth included The token import refused an --account / BASECAMP_ACCOUNT_ID / BASECAMP_BASE_URL override that disagreed with the named profile's binding, since the credential would be verified for the override and stored under a key that keeps the binding. The browser and device flows had no such check: an asserted login under an override verified against the override and then replaced the profile's working credential with one proven for somewhere else. The check is one helper now, run by every login that names an existing profile before any request is made.
--expect-identity ran only on auth login, which refuses a profile that does not exist yet; the browser or device login that creates one had no way to refuse a wrong session. profile create takes the same flag and runs the same verifier before the credential is stored: a mismatch leaves no credential and no profile entry. The verifier's account-scoped person lookup replaces the unscoped /my/profile.json fetch that ran after registration, which production answers with a 404. SDKClientFor now addresses the effective base URL rather than the one the startup client was built with, since a profile being created sets its own in memory before its login runs; for every existing caller the two are the same value.
An indirect dependency main carries too; the Security workflow's Trivy scan started failing on it once the advisory reached the database.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Pinned issuer validation accepts an empty fragment that redirects OAuth requests to the wrong path.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 21/22 changed files
- Comments generated: 2
- Review effort level: Balanced
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Config-write failures can leave credentials and profiles inconsistent, and doctor reports incorrect refresh guidance for imported tokens.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
internal/auth/keyring.go:28
- The new token source is not consumed by
checkAuthenticationininternal/commands/doctor.go. Once an imported token ages into the five-minute refresh window, doctor currently says “Token will auto-refresh on next API call,” even though these credentials intentionally have no refresh material andAccessTokenwill fail; after expiry it also attemptsRefresh. Branch onSource == CredentialSourceTokenand direct the operator to import a fresh token instead.
internal/commands/auth.go:504
- A missing
expires_atonly means the server did not report an expiry; it does not establish that the personal access token never expires. The README and JSON contract correctly describe this asnull, so the terminal summary should avoid the stronger and potentially misleading “does not expire” claim.
var expiresAt any
tokenLine := "personal access token (does not expire)"
- Files reviewed: 21/22 changed files
- Comments generated: 2
- Review effort level: Balanced
url.Parse reports "https://as.example#" with an empty fragment, so the origin check passed it, and the endpoints derived by concatenation carried the delimiter: the token endpoint became https://as.example#/oauth/tokens, posting to / with the path as a fragment. The delimiters are now refused on the raw value.
globalConfigTakesProfiles reads and validates the file's shape; it does not probe permissions, and the old name claimed more than that.
Computed and verified with make update-nix-hash (Docker build succeeds with the new hash).
Member
Author
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The core authentication flow is comprehensive and well-tested; remaining findings are limited to minor help text and invalid-config handling.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
internal/cli/root.go:124
profileKnowncan become true whenresolveProfilereturns its no-selection sentinel ("") and the loaded config contains an empty-string profile key. That newly applies the invalid profile and scopes the cache, whereas the previousprofileName != ""guard ignored it. Keep the non-empty check in the derived boolean so an empty result always means top-level configuration.
internal/commands/auth.go:376- This help text says callers must pass
--profile, but the implementation also acceptsBASECAMP_PROFILE, a configured default profile, or the sole configured profile. Describe the requirement as a named profile rather than requiring this specific flag.
- Files reviewed: 22/23 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Member
Author
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A bot's Basecamp identity should never come from a browser sign-in on a shared machine: whoever's session the browser holds is who the profile quietly becomes, and nothing checks. Basecamp now mints personal access tokens (
bc_at_…, revocable, full/read scope), so the CLI can take one from a secret store and refuse to keep it unless it authenticates as the expected identity. This is the CLI half of the Clawdito setup work in basecamp/coworker; the operator-facing flow isWhat
--with-token(internal/commands/auth.go)op read "op://…" | basecamp auth login --with-token -P <profile> --account <id>shape. Empty, multi-line, whitespace-bearing, control-character-bearing, or >4 KiB input is rejected without echoing it. The token is never written to output or errors.-P/--profileorBASECAMP_PROFILE; a single/default profile resolves too). When the profile does not exist,--account <id>is required and the profile is created with the current base URL. An existing profile keeps its entry untouched; passing an--accountthat differs from the profile's binding is a usage error rather than a silent rebind.App.SDKClientFor(StaticTokenProvider)— same transport, hooks and user agent as the real client) and must pass three checks before anything is written: the authorization document answers for it (identity id, email, reported scope — anything butread/fullis refused); the effective account (--account/BASECAMP_ACCOUNT_IDoverride the profile binding at runtime, so that is what is compared; a new profile must get its account explicitly, not from the operator's config file; digits only) appears non-expired in that document; and the account-scoped person lookupForAccount(acct).People().Mesucceeds. Only thenauth.Manager.ImportTokenstores{AccessToken, OAuthType: bc5, Scope, UserID, UserEmail}in one write and the profile is registered. A rejected token touches neither the store norconfig.json, and a profile's previous credential is never read or written. The server-reported expiry is stored with it (ExpiresAt0 when none is reported, the pathAccessTokennever refreshes); near a reported expiry the token is refused with "No refresh token available" and must be imported again./my/profile.jsononly under an account prefix (verified live: unscoped → 404,/2914079/my/profile.json→ 200), which is why the person lookup is account-scoped. The pre-existing "Logged in as" step inlogin/profile createused the unscoped path and had been failing silently.BASECAMP_TOKENin the environment is refused up front for--with-tokenand--expect-identity, since every request would otherwise carry it instead of the credential being checked.profile createintoregisterProfile/unregisterProfile(profile.go), used by create, delete, and the import. The root pre-run gains anAnnotationProfileMayCreatecarve-out so-P <new-name>reaches the command instead of failing with "unknown profile"; an unregistered name is not used as a cache-path component.--expect-identity <id>— the same verifier runs inside the browser and device flows through a newauth.LoginOptions.Verifyhook, called with the freshly issued token beforestore.Save. A mismatch stores nothing, prints no success line, and exits 3 (auth); a pre-existing credential is untouched. Without an expectation the "Logged in as" line stays best-effort. Output:Logged in as Clawdito <clawdito@…> (identity 28142355, person 51177542)(server-supplied name/email reduced to one line for the terminal; JSON verbatim).--json— envelope for the import:profile,account_id,base_url,source: "token",oauth_type: "bc5",scope,expires_at: null,identity: {id, email},person: {id, name, email},profile_created,default. Interactive flows under--json/--agent/--quiet/--ids-only/--countnow refuse before any network call (the machine-mode half of #669;BASECAMP_NONINTERACTIVEis deliberately not gated — a device login prints instructions and waits on a browser, not on stdin, which is a valid headless shape).BASECAMP_OAUTH_ISSUER(internal/auth/auth.go) — when set,discoverOAuthskips discovery and buildsoauth.Config{TokenEndpoint: issuer+"/oauth/tokens", DeviceAuthorizationEndpoint: issuer+"/oauth/device_authorizations"}(the paths bc3 mounts). The value passes the sameisSecureEndpointURLchecks as a discovered endpoint plus a no-query/no-fragment rule, is never echoed, and never falls back to Launchpad. Documented in the README as the dark-pilot escape hatch to remove after go-live.--login-hint <email>— threaded throughLoginOptions.LoginHintand sent by the device flow as Basecamp'slogin_hintextension viaoauth.WithDeviceLoginHint(basecamp/basecamp-sdk#841, merged as 47e7ca38). The option is appended only when a hint was given, so an unqualified login's form is unchanged; Launchpad logs that it ignores the flag. The SDK is bumped withmake bump-sdk REF=main(v0.15.0 → v0.16.1-0.20260903193203-47e7ca381a49, provenance updated, catalog parity tests green); the bump also carries the SDK's own 0.16.0 additions (to-do list template library, recent projects, spotlight), for which the CLI commands and API-COVERAGE rows are a separate SDK-sync task, not this PR.Usage
Verification
go build ./...,go vet ./...— clean.go test ./internal/auth ./internal/commands ./internal/cli ./internal/stdinarg— green. New coverage: token import happy path through a realhttptestidentity server (trailing newline trimmed before theAuthorizationheader; storedExpiresAt0;AccessTokenserves it with no refresh);--jsonenvelope shape;--expect-identitymismatch deletes the credential and registers no profile; mismatch on a pre-existing profile restores its previous credential byte-for-byte; unverifiable identity fails closed only with an expectation; a server-rejected token is not kept; server-reported scope wins; missing--account, missing profile, account mismatch, existing profile's entry (project_id,default_profile) untouched; bad stdin matrix (empty, whitespace, two lines, inner space, control char, oversized) with no echo; terminal stdin refusal (PTY);BASECAMP_TOKENrefusal; non-numeric identity; flag exclusivity; machine-mode refusal matrix; unknown profile without--with-token.internal/auth: pinned issuer skips discovery (0 hits on the resource server) and stores the trimmed/oauth/tokensendpoint; rejected issuer matrix (plain http, userinfo, no host, bad port, file scheme, query, fragment) never echoes and never falls back; login hint sent on the device flow (since round 9) and ignored on Launchpad;ImportTokenround trip and refresh behavior.internal/cli: may-create pass-through for--profileandBASECAMP_PROFILE, refusal otherwise.internal/stdinarg:IsTerminalover buffer, pipe,/dev/null, PTY.make fmt-check vet check-surface check-skill-drift check-bare-groups tidy-check— green;.surfaceregenerated with the six new flag entries (auth loginand theloginshortcut).make test-e2e— green, including four newauth.batscases (help text, profile required,--accountrequired,--with-tokenexcludes--device-code).bin/cigreen on Linux/x86 (thelio, Go 1.26.7) at a2defe8: fmt, vet, lint (0 issues), unit tests, 446 e2e, naming, surface, skill drift, bare groups, lint lockstep, smoke coverage, provenance, tidy. The PTY-class tests (TestIsTerminal,TestAuthLoginWithTokenRefusesTerminalStdin,TestInteractiveStdio,TestIsInteractiveTTY…, …) pass there; on this Mac's sandbox they fail identically onmainbecause/dev/ptmxis not a terminal, and its golangci-lint predates the Go 1.27 export-data format, so Linux is the reference run.--scope/--expect-identityvalidated before stdin is consumed (a test asserts stdin is left unread); device-flow--expect-identitymismatch stores nothing and prints no success line (end-to-end through a pinned issuer serving the grant); sanitization tests for the login hint, the pinned issuer (U+0085) and a server-supplied name (OSC hyperlink + CRLF); unregistered profile name kept out of the cache path (--profile ../../outside); e2e cases assert exact.error/.codeunderenv -u BASECAMP_PROFILE; doctor skill carries the token-import and--expect-identityremediation.user_id0; stdin accepts exactly one trailing LF/CRLF.Args: cobra.NoArgs; binding an accountless profile defined outside the global config is refused before stdin.source: "token", reported byauth statusandprofile show.BASECAMP_OAUTH_ISSUERaccepts an origin only; global config writers refuse a non-objectprofilesvalue and create the config directory first; README names app.basecamp.com.profile createproves the global config can take an entry before running OAuth (no credential left without a profile on a malformed file); README stops calling an imported token non-expiring.--login-hintsent on the wire (TestLoginDevice_LoginHintIsSentasserts the form carrieslogin_hint, and omits it when no hint is given). No review fixes this round: six threads left open on purpose — three server-reflected-bearer findings declined on merit, two config-file-hardening and one OAuth-binding finding flagged for Jeremy as the third-plus variation of settled classes.origin/mainmerged (78e1a65; go.mod keeps the SDK pin, main's goldmark bump preserved,go mod tidy) and main's newTestCatalogModelProvenancesatisfied by recording the pinned revision in the MCP model provenance (model files byte-identical to v0.16.0). Fixes: a bare trailing CR on stdin is refused (e26b374); a top-levelnullconfig is refused instead of panicking, the import preflights the config file before reading stdin, andprofile deletepreflights it before the credential goes (362cc81); the profile-binding check is one helper run by every login that names an existing profile, OAuth flows included (50b4fe8);profile create --expect-identityruns the same verifier, and the account-scoped person lookup replaces the unscoped/my/profile.jsonfetch (d33707a). The SDK-sync thread is overtaken by main's own v0.16.0 sync. Still open on purpose: the three server-reflected-bearer threads, declined on merit as before.unparamon the config preflight (da5db35); grpc v1.83.2 for CVE-2026-84445, an indirect dep main shares whose advisory started failing the Trivy scan (35bda36); a bare?/#onBASECAMP_OAUTH_ISSUERis refused on the raw value —url.Parsereports an empty fragment as absent while the concatenated endpoints carried it (a81be7c); the import's terminal line says "no expiry reported" (071a546); the preflight is renamedglobalConfigTakesProfilesfor what it checks (fffcde5); Nix vendorHash recomputed and verified viamake update-nix-hash(73da0c7). Declined: a permission probe / two-store rollback for the config preflight (two Copilot threads, left open with reasoning) — sixth variation of the config-file series. Copilot resolved the three server-reflected-bearer threads itself on re-review.bin/cigreen on thelio at 73da0c7; GitHub CI green.gh release listshows v0.16.0 as latest), so the pseudo-versionv0.16.1-0.20260903193203-47e7ca381a49stays. Tagging sdk v0.16.1 and runningmake bump-sdk REF=v0.16.1+scripts/sync-mcp-model.shwould restore the tagged shape in both provenance files.op-coworker read clawdito … | basecamp auth login --with-token -P clawdito --account 2914079 --expect-identity 28142355) andBASECAMP_OAUTH_ISSUER=https://app.basecamp.com basecamp auth login --device-codewait on the bc3 pilot deploy (work package A/B).Not doing here
BASECAMP_NONINTERACTIVE(the other half ofbasecamp loginblocks and emits prose under machine output / non-interactive environments #669) — see above.profile:<name>.Summary by cubic
Adds
basecamp auth login --with-tokento import a personal access token from piped stdin and verify it for the requested account and identity. Unlike browser login, which could silently use the browser’s current identity,--expect-identitynow protects OAuth andprofile createtoo, leaving existing credentials untouched on failure.Authentication
--account; existing profiles reject account or base-URL mismatches.BASECAMP_TOKEN.source: "token"plus the server-reported scope and expiry;auth status,profile show, and--jsonexpose them.--login-hintis sent with device authorization.BASECAMP_OAUTH_ISSUERnow accepts only a valid origin without a path, query, or fragment.Configuration and dependencies
github.com/basecamp/basecamp-sdk/gofor device login hints andgoogle.golang.org/grpcto v1.83.2 for CVE-2026-84445.Written for commit 73da0c7. Summary will update on new commits.