Skip to content

CLI warns when the key comes from ./.env - #227

Open
H-maximedelpit wants to merge 1 commit into
mainfrom
cli-key-file-checks
Open

H-maximedelpit wants to merge 1 commit into
mainfrom
cli-key-file-checks

Conversation

@H-maximedelpit

@H-maximedelpit H-maximedelpit commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ Warning notice. Spend several hours with Fable implementing, debating & reviewing the code. It deals with auth & some security improvement so please read carefully and don't hesitate to reject and take the lead please. cc @abonneth

Purpose

Found during the security review of the hai login work: the CLI reads ./.env before the global key file, with no checks. hai run inside a cloned or forked repo that ships a .env with HAI_API_KEY sends the user's prompts, files and vault secrets to the attacker's organization, silently. Independent of the login changes.

What it does

  • When the key comes from ./.env, every command prints once, on stderr (emitted from the shared state builder, so mcp install, local, login and --json modes are covered; Bugbot finding): "Using HAI_API_KEY from .env. Make sure this key is yours: a cloned or forked repo can ship a .env that carries someone else's key on purpose, and your runs would then land in their account." hai doctor carries the same sentence. The key value is never printed.
  • A .env without HAI_ variables is ignored silently. A symlink in place of ./.env is rejected.
  • The global file ~/.config/hai/.env must be a regular file owned by the current user with mode 600, as hai login writes it; otherwise it is ignored with a reason.
  • Resolution order is unchanged: --api-key, HAI_API_KEY, ./.env, global file.

What it does not do

Detection, not prevention: a reader who skims stderr still sends the run to the attacker's org. Accepted for now.

Alternatives rejected

  • Owner/mode check on ./.env: honest project files are created with the umask (0644) like planted ones, so it would reject most legitimate files and teach people to chmod past it.
  • Trust-on-first-use prompt (direnv style, recorded under ~/.config/hai/trusted): built and tested, then dropped as too heavy for now. Candidate follow-up if the warning proves insufficient.
  • Dropping ./.env support: per-project keys are a used feature.

Dependencies and merge order

None. Touches app.py near the client builder, so whichever hai login PR lands after it (#229 or #228) needs a small rebase.

🤖 Generated with Claude Code


Note

Medium Risk
Changes authentication credential resolution and global key file trust rules; misconfiguration could block legitimate keys until permissions are fixed, while project .env keys remain usable with only a stderr warning.

Overview
Hardens CLI credential loading and adds detection (not blocking) when HAI_API_KEY comes from a project .env, so runs in cloned repos with a planted key are less likely to go unnoticed.

Credential rules in credentials.py: only read env files that pass key_file_rejection (regular file; global ~/.config/hai/.env must be user-owned and mode 600). Project .env is used only if it sets at least one HAI_* variable; symlinks and unreadable/binary files are skipped without crashing. Ignored files surface via key_file_warnings().

User-facing behavior: _state() prints those warnings plus PROJECT_ENV_WARNING once per process on stderr (including --json). hai doctor repeats the project-key warning on success and fails login when keys are present but ignored. README documents the warning and global file permissions.

Resolution order is unchanged: --api-key → HAI_API_KEY → ./.env → global file.

Reviewed by Cursor Bugbot for commit b32c855. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread src/hai_agents_cli/app.py Outdated

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread src/hai_agents_common/credentials.py Outdated

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread src/hai_agents_common/credentials.py Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b32c855. Configure here.

return CheckResult("login", True, detail)
ignored = credentials.key_file_warnings()
if ignored:
return CheckResult("login", False, "; ".join(ignored), fix="fix the file's owner and mode, or run `hai login`")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Doctor misdiagnoses missing API key

Low Severity

When no key resolves, check_login treats any key_file_warnings line as the login failure and offers only owner/mode repair. A leftover .env symlink or directory then replaces the "no API key configured" guidance, and that fix text does not match symlink or non-file rejections.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b32c855. Configure here.

This branch has not been deployed

No deployments
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.

1 participant