Refuse browser and pasted-callback logins under BASECAMP_NONINTERACTIVE - #693
Refuse browser and pasted-callback logins under BASECAMP_NONINTERACTIVE#693jeremy wants to merge 4 commits into
Conversation
The machine-output flags already turn an interactive login into a usage error; the environment variable that says the same thing did not, so a harness that set it and ran the login sat on a browser callback for five minutes, or on a pasted-URL prompt nobody would answer. The gate refuses those flows up front, before discovery, and names the two headless shapes: --device-code, which prints a code to approve from any device and asks nothing of the terminal, and --with-token. A device login on a Launchpad-backed host falls back to the pasted callback, so that prompt refuses under the variable too, where it would otherwise read stdin. Fixes #669
github-actions
Bot
added
commands
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. |
There was a problem hiding this comment.
🟡 Changes recommended
Local Launchpad login remains interactive through other callers, and device-code guidance overstates current provider support.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds non-interactive safeguards intended to prevent OAuth login flows from waiting for browser or pasted-callback input.
Changes:
- Gates interactive login under
BASECAMP_NONINTERACTIVE. - Adds unit and end-to-end coverage.
- Documents the device-code exception.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
internal/commands/auth.go |
Adds the command-level non-interactive gate. |
internal/auth/auth.go |
Refuses remote callback prompting. |
internal/commands/auth_login_test.go |
Tests refusal and device-code behavior. |
internal/auth/auth_test.go |
Tests remote-mode refusal. |
e2e/auth.bats |
Adds CLI-level refusal coverage. |
skills/basecamp/SKILL.md |
Documents non-interactive authentication. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![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: 9c80845f7f
ℹ️ 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".
| if opts.Remote { | ||
| if config.NonInteractiveEnv() { |
There was a problem hiding this comment.
When BASECAMP_NONINTERACTIVE=1 is used with basecamp profile create <name> without --remote, newProfileCreateCmd calls Manager.Login with Remote: false, so this remote-only guard is skipped and the Launchpad branch still opens a browser and waits up to five minutes for a loopback callback. This leaves a public login entry point violating the noninteractive guarantee; gate every Launchpad browser/callback flow in the auth layer, or add the equivalent check to profile creation.
Useful? React with 👍 / 👎.
| basecamp auth login --scope full # Full access (the default; ignored by Launchpad) | ||
| basecamp auth login --scope read # Read-only access (ignored by Launchpad) | ||
| basecamp auth login --device-code # Headless authentication with manual browser instructions | ||
| BASECAMP_NONINTERACTIVE=1 basecamp auth login --device-code # The only OAuth login that runs under BASECAMP_NONINTERACTIVE; browser and pasted-callback flows refuse |
There was a problem hiding this comment.
When OAuth discovery falls back to Launchpad, --device-code is converted to Remote: true in buildLoginCmd, after which the new guard in loginLaunchpad rejects it because Launchpad's implementation is still the pasted-callback flow. Consequently this newly documented command fails on Launchpad-backed accounts rather than running as claimed; describe it as available only with a BC5 device-flow provider and recommend --with-token for noninteractive Launchpad authentication.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Done in d5af034: the skill line says the device login runs under the variable only where the server offers the device flow (Launchpad does not) and points automation at --with-token; the command hint says the same.
…the pasted callback profile create reaches Manager.Login without the login command's gate, so the local Launchpad branch still opened a browser and waited on the loopback callback under the variable. Both Launchpad shapes wait on a person, and every entry point converges on loginLaunchpad, so that is where the refusal lives now. The hints and the skill stop promising --device-code unconditionally: Launchpad has no device flow.
There was a problem hiding this comment.
🟡 Changes recommended
BC5-backed profile creation can still open a browser under BASECAMP_NONINTERACTIVE without an explicit device-code request.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- 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: d5af0342e9
ℹ️ 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".
| // browser someone signs into, the remote one on a pasted redirect URL. | ||
| // Every login entry point converges here, so this is where the | ||
| // environment's word that nobody is at the terminal is final. | ||
| if config.NonInteractiveEnv() { |
There was a problem hiding this comment.
When BASECAMP_NONINTERACTIVE=1 is used with basecamp profile create <name> against a BC5 issuer without --device-code, profile creation calls Manager.Login with browser-enabled defaults, and the BC5 branch returns through loginDevice before reaching this Launchpad-only guard; that flow can open a browser and poll until the device code expires. The fresh evidence after the earlier Launchpad report is the unguarded BC5 return at auth.go:480-482, combined with profile creation having no equivalent !deviceCode check. Add the same explicit device-code gate to profile creation, or propagate that intent into Manager.Login, so this entry point cannot wedge noninteractive automation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Yes — this was the remaining path. 4b0fecd (gate) and 4c767d3 (placed before the in-memory profile entry is added) give profile create the same refuseNonInteractiveLogin gate as login, so a BC5 profile create refuses under the variable without --device-code before discovery; and LoginOptions.defaults now treats the variable as --no-browser for every flow, so even a --device-code login under it prints the code rather than launching a browser. Covered by TestProfileCreateRefusesNonInteractiveEnv and TestLoginDefaultsNeverOpenABrowserUnderNonInteractiveEnv.
…variable profile create runs the same OAuth flows as login without the command gate, so on a BC5 issuer it launched a browser to the approval page under BASECAMP_NONINTERACTIVE. Both commands now share one refusal, and LoginOptions.defaults treats the variable as --no-browser for every flow.
There was a problem hiding this comment.
🔵 Needs a closer look
The profile gate returns after mutating in-memory configuration, bypassing rollback and failing the new regression test.
Review details
Suppressed comments (1)
internal/commands/profile.go:269
- This returns after the pending profile has already been inserted and
ActiveProfile/BaseURLchanged, bypassing the rollback used for login errors below. A refused defaultprofile createtherefore leaves the in-memory config mutated (and makes the newassert.Empty(t, cfg.Profiles)case fail). Move this gate before the temporary mutation.
app.Config.Profiles[name] = profileCfg
app.Config.ActiveProfile = name
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Balanced
#681 gated interactive login on the explicit machine-output flags and left the environment half of #669 open:
BASECAMP_NONINTERACTIVE=1 basecamp loginstill opened a browser (or, under SSH, printed a URL and read stdin for a pasted callback) and sat there for up to five minutes. The repo's own contract for the variable — "the CLI must not show interactive prompts regardless of TTY detection; they become actionable errors" (config.NonInteractiveEnv, SKILL.md) — is what this applies to login.What
auth login/loginrefuse underBASECAMP_NONINTERACTIVEunless--device-codeis given. The check sits after the machine-output gate and before any network call, and its hint names the two headless shapes:--device-code, which prints a code to approve from any device and asks nothing of the terminal (the exception Import personal access tokens with auth login --with-token #681 argued for), and--with-token, plusauth statusfor a check. Same usage-error shape as the flag gate.internal/auth). A device login on a Launchpad-backed host — every production account until the BC5 authorization server ships — falls back to remote mode, whose "Paste the callback URL" read is the one place a login consumes stdin. That prompt now returns the same actionable error before printing instructions, so the--device-codeexemption cannot route a harness into a stdin wait.Verification
main,TestAuthLoginRefusesNonInteractiveEnvWithoutDeviceCode(default,--remote,--local,--no-browser) andTestLoginRemoteModeRefusesNonInteractiveEnvdo not fail fast — they hang on the callback wait until the harness timeout, which is the reported behavior. With the gates they refuse withusage, nameBASECAMP_NONINTERACTIVE,--device-codeand--with-token, and the identity server sees no request; the remote test also asserts the paste prompt was never logged.TestAuthLoginDeviceCodeRunsUnderNonInteractiveEnvproves the exemption end-to-end: a pinned-issuer device grant under the variable prints the code and completes.auth.batscase for the env refusal (exit non-zero, hint names both flags).bin/cigreen on Linux (thelio, Go 1.26.7): fmt, vet, lint, unit, e2e, naming, surface, skill drift, bare groups, provenance, tidy.Not doing here
signal.NotifyContext: separate from this gate.Fixes #669
Summary by cubic
Fixes #669:
basecamp auth loginandbasecamp profile createnow refuse browser and pasted-callback logins underBASECAMP_NONINTERACTIVEinstead of opening a browser or waiting up to five minutes on a pasted callback.--device-coderemains the exception, but only where the server offers the device flow — Launchpad does not.Changes
auth loginandprofile create, plus again in the auth layer for every converging Launchpad shape, so the refusal happens before any network call, stdin read, or in-memory profile entry.LoginOptions.defaultstreats the variable as--no-browserfor every flow, so no login shape can launch a browser under it.--device-codeand--with-token; SKILL.md documents the rule.Written for commit 4c767d3. Summary will update on new commits.