Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 32 additions & 16 deletions openhack/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,34 @@
import sys


def _require_auth() -> bool:
"""Validate that the active LLM provider has credentials configured."""
import os
from openhack import providers
from openhack.config import settings

active_provider = settings.llm_provider
if active_provider != "openhack":
if providers.is_connected(active_provider):
return True
Comment on lines +30 to +31

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Keyless commands reject .env settings

If LLM_PROVIDER=ollama is set in .env, with no saved provider or OLLAMA_API_KEY, _require_auth() rejects a valid keyless setup. Settings reads .env, but is_connected() checks only saved config and os.environ. All four headless commands stop and ask for a key that Ollama does not need. This also breaks runs that previously passed with an OpenHack key.

Use the resolved provider selection for the keyless check and add a .env regression test.

Prompt To Fix With AI
This is a comment left during a code review.
Path: openhack/__main__.py
Line: 30-31

Comment:
**Keyless commands reject `.env` settings**

If `LLM_PROVIDER=ollama` is set in `.env`, with no saved provider or `OLLAMA_API_KEY`, `_require_auth()` rejects a valid keyless setup. `Settings` reads `.env`, but `is_connected()` checks only saved config and `os.environ`. All four headless commands stop and ask for a key that Ollama does not need. This also breaks runs that previously passed with an OpenHack key.

Use the resolved provider selection for the keyless check and add a `.env` regression test.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Codex Fix in Claude Code Fix in Cursor Fix in Conductor

spec = providers.get_spec(active_provider)
if spec and spec.required_env:
missing = [e for e in spec.required_env if not os.environ.get(e)]
if missing:
print(f"Error: provider '{active_provider}' is missing required environment: {', '.join(missing)}.")
return False
key_env = spec.api_key_env if spec else f"{active_provider.upper()}_API_KEY"
print(f"Error: provider '{active_provider}' is not configured.")
print(f"Set {key_env} or run 'openhack' to configure credentials.")
return False

if not settings.openhack_api_key and not providers.is_connected("openhack"):
print("Error: not logged in.")
print("Run 'openhack --login' to set up your account, or set OPENHACK_API_KEY.")
return False
return True


def _cmd_scan():
"""Run a headless scan on a directory."""
from pathlib import Path
Expand All @@ -31,10 +59,7 @@ def _cmd_scan():
print("Usage: openhack --scan [path]")
return

from openhack.config import settings
if not settings.openhack_api_key:
print("Error: not logged in.")
print("Run 'openhack --login' to set up your account, or set OPENHACK_API_KEY.")
if not _require_auth():
return

import asyncio
Expand Down Expand Up @@ -62,10 +87,7 @@ def _cmd_hack():
if not target.is_dir():
target = Path.cwd()

from openhack.config import settings
if not settings.openhack_api_key:
print("Error: not logged in.")
print("Run 'openhack --login' to set up your account, or set OPENHACK_API_KEY.")
if not _require_auth():
return

from openhack.interactive_runner import run_task
Expand All @@ -90,10 +112,7 @@ def _cmd_plan():
if not target.is_dir():
target = Path.cwd()

from openhack.config import settings
if not settings.openhack_api_key:
print("Error: not logged in.")
print("Run 'openhack --login' to set up your account, or set OPENHACK_API_KEY.")
if not _require_auth():
return

from openhack.interactive_runner import run_plan
Expand All @@ -112,10 +131,7 @@ def _cmd_agent():
if not target.is_dir():
target = Path.cwd()

from openhack.config import settings
if not settings.openhack_api_key:
print("Error: not logged in.")
print("Run 'openhack --login' to set up your account, or set OPENHACK_API_KEY.")
if not _require_auth():
return

from openhack.interactive_runner import run_repl
Expand Down
2 changes: 2 additions & 0 deletions openhack/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ def _build_settings() -> Settings:
for key, val in user_cfg.items():
if val is not None and val != "":
env_overrides[key.upper()] = str(val)
if key == "provider":
env_overrides["LLM_PROVIDER"] = str(resolve_provider(str(val)))

dotenv_keys = _dotenv_nonempty_keys(Path(".env"))
old_env = {}
Expand Down
10 changes: 8 additions & 2 deletions openhack/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,10 @@ def is_connected(name: str) -> bool:
and (credential.get("access") or credential.get("refresh"))
):
return True
return bool(spec.keyless_default and config.get("provider") == name)
return bool(
spec.keyless_default
and (config.get("provider") == name or os.environ.get("LLM_PROVIDER") == name)
)


def connected_provider_ids(
Expand Down Expand Up @@ -327,7 +330,10 @@ def connected_provider_ids(
os.environ.get(spec.api_key_env)
or has_api_key
or has_oauth
or (spec.keyless_default and config.get("provider") == spec.name)
or (
spec.keyless_default
and (config.get("provider") == spec.name or os.environ.get("LLM_PROVIDER") == spec.name)
)
):
connected.add(spec.name)
return connected
Expand Down
76 changes: 76 additions & 0 deletions tests/test_cli_dispatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,79 @@ def test_launch_tui_never_runs_first_time_setup(monkeypatch):
m._launch_tui()

assert calls == [{"resume_session_id": None}]


def test_require_auth_passes_for_connected_byok_provider(monkeypatch):
from openhack.config import settings
monkeypatch.setattr(settings, "llm_provider", "openai")
monkeypatch.setattr(settings, "openhack_api_key", None)
monkeypatch.setenv("OPENAI_API_KEY", "sk-test-openai")

out = io.StringIO()
with contextlib.redirect_stdout(out):
assert m._require_auth() is True
assert out.getvalue() == ""


def test_require_auth_fails_for_disconnected_byok_provider(monkeypatch):
from openhack.config import settings
monkeypatch.setattr(settings, "llm_provider", "openai")
monkeypatch.setattr(settings, "openhack_api_key", None)
monkeypatch.delenv("OPENAI_API_KEY", raising=False)
monkeypatch.setattr("openhack.provider_auth.all_credentials", lambda: {})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Saved credentials break the test

This test patches all_credentials(), but is_connected() calls get_credential(), which reads saved credentials directly. If a developer has a saved OpenAI API key or ChatGPT login, _require_auth() returns True and the test fails despite working code.

Patch openhack.providers.get_credential to return None, or give the test an isolated credential store.

Suggested change
monkeypatch.setattr("openhack.provider_auth.all_credentials", lambda: {})
monkeypatch.setattr("openhack.providers.get_credential", lambda name: None)
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/test_cli_dispatch.py
Line: 123

Comment:
**Saved credentials break the test**

This test patches `all_credentials()`, but `is_connected()` calls `get_credential()`, which reads saved credentials directly. If a developer has a saved OpenAI API key or ChatGPT login, `_require_auth()` returns `True` and the test fails despite working code.

Patch `openhack.providers.get_credential` to return `None`, or give the test an isolated credential store.

```suggestion
    monkeypatch.setattr("openhack.providers.get_credential", lambda name: None)
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Codex Fix in Claude Code Fix in Cursor Fix in Conductor


out = io.StringIO()
with contextlib.redirect_stdout(out):
assert m._require_auth() is False
assert "Error: provider 'openai' is not configured." in out.getvalue()
assert "OPENAI_API_KEY" in out.getvalue()


def test_require_auth_fails_for_openhack_when_logged_out(monkeypatch):
from openhack.config import settings
monkeypatch.setattr(settings, "llm_provider", "openhack")
monkeypatch.setattr(settings, "openhack_api_key", None)
monkeypatch.delenv("OPENHACK_API_KEY", raising=False)
monkeypatch.setattr("openhack.config.load_user_config", lambda: {})

out = io.StringIO()
with contextlib.redirect_stdout(out):
assert m._require_auth() is False
assert "Error: not logged in." in out.getvalue()


def test_require_auth_passes_for_openhack_when_logged_in(monkeypatch):
from openhack.config import settings
monkeypatch.setattr(settings, "llm_provider", "openhack")
monkeypatch.setattr(settings, "openhack_api_key", "sk-openhack-test")

out = io.StringIO()
with contextlib.redirect_stdout(out):
assert m._require_auth() is True
assert out.getvalue() == ""


def test_cmd_scan_runs_with_byok_provider_without_openhack_key(monkeypatch, tmp_path):
from openhack.config import settings
monkeypatch.setattr(settings, "llm_provider", "openai")
monkeypatch.setattr(settings, "openhack_api_key", None)
monkeypatch.setenv("OPENAI_API_KEY", "sk-test-openai")

target_dir = tmp_path / "sample-project"
target_dir.mkdir()
monkeypatch.setattr("sys.argv", ["openhack", "--scan", str(target_dir)])

scan_invoked = []

async def _fake_run(target):
scan_invoked.append(target)

monkeypatch.setattr("openhack.headless_scan.run_headless_scan", _fake_run)

out = io.StringIO()
with contextlib.redirect_stdout(out):
m._cmd_scan()

assert scan_invoked == [str(target_dir)]
assert "Error: not logged in" not in out.getvalue()