Refuse browser and pasted-callback logins under BASECAMP_NONINTERACTIVE - #693
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".
…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".
…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.