Skip to content

fix: warn on config parse errors during auth login#311

Merged
sachiniyer merged 1 commit into
mainfrom
devin/1780685090-auth-login-config-warning
Jun 5, 2026
Merged

fix: warn on config parse errors during auth login#311
sachiniyer merged 1 commit into
mainfrom
devin/1780685090-auth-login-config-warning

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

storage::load_config().unwrap_or_default() in the AuthCommands::Login arm silently swallowed config parse errors, causing users with custom api_url/app_url (e.g. staging) to unknowingly authenticate against production.

Added .inspect_err() before .unwrap_or_default() to print a warning to stderr when parsing fails, while still falling back to defaults for bootstrap/recovery:

let config = storage::load_config()
    .inspect_err(|e| {
        let _ = Term::stderr().write_line(&format!(
            "Warning: Config file has errors, using default settings: {e}"
        ));
    })
    .unwrap_or_default();

Closes #307

Link to Devin session: https://app.devin.ai/sessions/c760784c0c454dd096c8def6f4406ec6
Requested by: @sachiniyer


Open in Devin Review

Summary by cubic

Warn on config parse errors during auth login by printing to stderr before falling back to defaults. Prevents silent fallback that could make users with custom api_url/app_url authenticate against production.

Written for commit cc28dc7. Summary will update on new commits.

Review in cubic

When `storage::load_config()` fails in the auth login path, add an
`.inspect_err()` call that prints a warning to stderr before falling
back to defaults. Previously, parse errors were silently swallowed,
which could cause users with custom api_url/app_url settings to
unknowingly authenticate against production.

Closes #307

Co-Authored-By: Sachin Iyer <siyer@detail.dev>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author
Original prompt from Sachin

Fix GitHub issue #307 in usedetail/cli: "CLI Auth: auth login silently falls back to production URLs when config is invalid"

#``# Repository
usedetail/cli (https://github.com/usedetail/cli)

#``# Bug
In the auth login command, storage::load_config().unwrap_or_default() silently falls back to production URLs when config parse errors occur. Users who configured custom api_url or app_url settings (e.g., for staging) will unknowingly authenticate against production instead.

#``# Fix
Add a warning when config parsing fails in the auth login path, while still defaulting to allow bootstrap/recovery:

let config = storage::load_config()
    .inspect_err(|e| {
        let _ = Term::stderr().write_line(&amp;format!(
            "Warning: Config file has errors, using default settings: {e}"
        ));
    })
    .unwrap_or_default();

Find the auth login handler (likely in src/commands/auth.rs or src/lib.rs) where storage::load_config().unwrap_or_default() is called in the AuthCommands::Login match arm.

#``# Requirements

  1. Add .inspect_err() to warn users when config parsing fails during auth login
  2. Ensure the warning goes to stderr so it doesn't break machine-readable output
  3. Ensure all existing tests still pass (cargo test)
  4. Ensure cargo fmt --check and cargo clippy -- -D warnings pass
  5. Create a PR that closes #307

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@aviator-app

aviator-app Bot commented Jun 5, 2026

Copy link
Copy Markdown

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.


See the real-time status of this PR on the Aviator webapp.
Use the Aviator Chrome Extension to see the status of your PR within GitHub.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread src/commands/auth.rs
Comment on lines +42 to +48
let config = storage::load_config()
.inspect_err(|e| {
let _ = Term::stderr().write_line(&format!(
"Warning: Config file has errors, using default settings: {e}"
));
})
.unwrap_or_default();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Inconsistent config error handling across call sites

The PR adds a user-facing warning when load_config() fails in the Login handler, but the other call site in src/lib.rs:74 (create_client) propagates the error with ? instead. This means a corrupted config causes auth login to silently fall back to defaults (now with a warning), while auth status and all other commands using create_client() will hard-fail. This is intentional — login needs to work even with a broken config so the user can re-authenticate — but it's worth noting the asymmetry.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@sachiniyer sachiniyer merged commit 96e8ae0 into main Jun 5, 2026
13 checks passed
@sachiniyer sachiniyer deleted the devin/1780685090-auth-login-config-warning branch June 5, 2026 18:55
sachiniyer added a commit that referenced this pull request Jun 12, 2026
## Summary

Patch release bump `0.2.5` → `0.2.6`. Includes fixes merged since
v0.2.5:

- `#313` serialize SHELL env var mutation in completions tests
- `#312` validate `$SHELL` before extracting shell name in completions
- `#311` warn on config parse errors during auth login
- `#310` handle IPv6 bracketed hosts in `strip_ssh_port`

Once merged, the `release.yml` workflow will automatically tag `v0.2.6`,
build platform artifacts via `cargo-dist`, and publish a GitHub Release.

Link to Devin session:
https://app.devin.ai/sessions/02dfe916725247d6955ba0d4a49460df
Requested by: @sachiniyer
<!-- devin-review-badge-begin -->

---

<a href="https://app.devin.ai/review/usedetail/cli/pull/315"
target="_blank">
  <picture>
<source media="(prefers-color-scheme: dark)"
srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1">
<img
src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1"
alt="Open in Devin Review">
  </picture>
</a>
<!-- devin-review-badge-end -->

<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Release v0.2.6 to ship fixes for shell detection, SSH host parsing, and
clearer auth warnings, plus more reliable completion tests.

- **Bug Fixes**
- Serialize `SHELL` env var mutation in completions tests to prevent
flakiness.
  - Validate `$SHELL` before extracting the shell name in completions.
  - Warn on config parse errors during `auth login`.
  - Handle IPv6 bracketed hosts in `strip_ssh_port`.

<sup>Written for commit db9999c.
Summary will update on new commits.</sup>

<a href="https://cubic.dev/pr/usedetail/cli/pull/315?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>

<!-- End of auto-generated description by cubic. -->

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Sachin Iyer <siyer@detail.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Detail Bug] CLI Auth: auth login silently falls back to production URLs when config is invalid

1 participant