feat(privacy): blocking consent gate for env-fallback LLM API keys
Adds api_key_source provenance ('flag' | 'env' | None) to LLMProvider
so cmd_init can distinguish a key passed via --llm-api-key (explicit
opt-in) from one silently picked up via OPENAI_API_KEY / ANTHROPIC_API_KEY
shell env (stray credential).
When the endpoint is external AND api_key_source == 'env', init now
prints a blocking [y/N] prompt before any data is sent. Anything other
than 'y' drops the LLM and falls back to heuristics-only.
Adds --accept-external-llm flag for CI / non-interactive bypass.
Completes the UX gap in #1224: the URL-based warning was informational
and init kept running, so a user who didn't notice the line had already
leaked. The consent prompt is the actual gate; explicit flag-passed keys
remain treated as already-consented.
This commit is contained in:
@@ -276,6 +276,33 @@ def cmd_init(args):
|
|||||||
f"retains, or uses your data. Pass --no-llm to keep "
|
f"retains, or uses your data. Pass --no-llm to keep "
|
||||||
f"init fully local."
|
f"init fully local."
|
||||||
)
|
)
|
||||||
|
# Consent gate (issue #26): block init when the api_key
|
||||||
|
# was acquired via env-fallback (stray credential in
|
||||||
|
# shell env). Explicit --llm-api-key (api_key_source ==
|
||||||
|
# "flag") means the user already opted in.
|
||||||
|
# --accept-external-llm bypasses for CI / non-interactive.
|
||||||
|
api_key_source = getattr(candidate, "api_key_source", None)
|
||||||
|
accept_flag = getattr(args, "accept_external_llm", False)
|
||||||
|
if api_key_source == "env" and not accept_flag:
|
||||||
|
try:
|
||||||
|
answer = (
|
||||||
|
input(
|
||||||
|
" Your API key was loaded from the environment "
|
||||||
|
"(not passed via --llm-api-key). Continue with "
|
||||||
|
"external LLM? [y/N] "
|
||||||
|
)
|
||||||
|
.strip()
|
||||||
|
.lower()
|
||||||
|
)
|
||||||
|
except EOFError:
|
||||||
|
answer = ""
|
||||||
|
if answer != "y":
|
||||||
|
print(
|
||||||
|
" Declined — falling back to heuristics-only. "
|
||||||
|
"Pass --llm-api-key explicitly or "
|
||||||
|
"--accept-external-llm to skip this prompt."
|
||||||
|
)
|
||||||
|
llm_provider = None
|
||||||
else:
|
else:
|
||||||
print(
|
print(
|
||||||
f" No LLM provider reachable ({msg}). "
|
f" No LLM provider reachable ({msg}). "
|
||||||
@@ -971,6 +998,16 @@ def main():
|
|||||||
"for openai-compat, defaults to $OPENAI_API_KEY."
|
"for openai-compat, defaults to $OPENAI_API_KEY."
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
p_init.add_argument(
|
||||||
|
"--accept-external-llm",
|
||||||
|
action="store_true",
|
||||||
|
help=(
|
||||||
|
"Bypass the interactive consent prompt that fires when an external "
|
||||||
|
"LLM is configured via an environment-variable API key (issue #26). "
|
||||||
|
"Use this in CI / non-interactive runs where you've already decided "
|
||||||
|
"the external send is acceptable."
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
# mine
|
# mine
|
||||||
p_mine = sub.add_parser("mine", help="Mine files into the palace")
|
p_mine = sub.add_parser("mine", help="Mine files into the palace")
|
||||||
|
|||||||
+30
-4
@@ -127,11 +127,18 @@ class LLMProvider:
|
|||||||
endpoint: Optional[str] = None,
|
endpoint: Optional[str] = None,
|
||||||
api_key: Optional[str] = None,
|
api_key: Optional[str] = None,
|
||||||
timeout: int = 120,
|
timeout: int = 120,
|
||||||
|
api_key_source: Optional[str] = None,
|
||||||
):
|
):
|
||||||
self.model = model
|
self.model = model
|
||||||
self.endpoint = endpoint
|
self.endpoint = endpoint
|
||||||
self.api_key = api_key
|
self.api_key = api_key
|
||||||
self.timeout = timeout
|
self.timeout = timeout
|
||||||
|
# Provenance of api_key (issue #26): "flag" when the constructor
|
||||||
|
# received an explicit api_key arg, "env" when it fell back to an
|
||||||
|
# environment variable, None when no key is in play. cmd_init
|
||||||
|
# uses this to gate the consent prompt — stray env-resolved keys
|
||||||
|
# require explicit user confirmation.
|
||||||
|
self.api_key_source = api_key_source
|
||||||
|
|
||||||
def classify(self, system: str, user: str, json_mode: bool = True) -> LLMResponse:
|
def classify(self, system: str, user: str, json_mode: bool = True) -> LLMResponse:
|
||||||
raise NotImplementedError
|
raise NotImplementedError
|
||||||
@@ -253,8 +260,20 @@ class OpenAICompatProvider(LLMProvider):
|
|||||||
timeout: int = 120,
|
timeout: int = 120,
|
||||||
**_: object,
|
**_: object,
|
||||||
):
|
):
|
||||||
resolved_key = api_key or os.environ.get("OPENAI_API_KEY")
|
if api_key:
|
||||||
super().__init__(model=model, endpoint=endpoint, api_key=resolved_key, timeout=timeout)
|
resolved_key = api_key
|
||||||
|
source: Optional[str] = "flag"
|
||||||
|
else:
|
||||||
|
env_key = os.environ.get("OPENAI_API_KEY")
|
||||||
|
resolved_key = env_key or None
|
||||||
|
source = "env" if env_key else None
|
||||||
|
super().__init__(
|
||||||
|
model=model,
|
||||||
|
endpoint=endpoint,
|
||||||
|
api_key=resolved_key,
|
||||||
|
timeout=timeout,
|
||||||
|
api_key_source=source,
|
||||||
|
)
|
||||||
|
|
||||||
def _resolve_url(self) -> str:
|
def _resolve_url(self) -> str:
|
||||||
if not self.endpoint:
|
if not self.endpoint:
|
||||||
@@ -321,12 +340,19 @@ class AnthropicProvider(LLMProvider):
|
|||||||
timeout: int = 120,
|
timeout: int = 120,
|
||||||
**_: object,
|
**_: object,
|
||||||
):
|
):
|
||||||
key = api_key or os.environ.get("ANTHROPIC_API_KEY")
|
if api_key:
|
||||||
|
resolved_key = api_key
|
||||||
|
source: Optional[str] = "flag"
|
||||||
|
else:
|
||||||
|
env_key = os.environ.get("ANTHROPIC_API_KEY")
|
||||||
|
resolved_key = env_key or None
|
||||||
|
source = "env" if env_key else None
|
||||||
super().__init__(
|
super().__init__(
|
||||||
model=model,
|
model=model,
|
||||||
endpoint=endpoint or self.DEFAULT_ENDPOINT,
|
endpoint=endpoint or self.DEFAULT_ENDPOINT,
|
||||||
api_key=key,
|
api_key=resolved_key,
|
||||||
timeout=timeout,
|
timeout=timeout,
|
||||||
|
api_key_source=source,
|
||||||
)
|
)
|
||||||
|
|
||||||
def check_available(self) -> tuple[bool, str]:
|
def check_available(self) -> tuple[bool, str]:
|
||||||
|
|||||||
@@ -1830,3 +1830,195 @@ def test_init_no_privacy_warning_with_no_llm_flag(ai_dialogue_corpus: Path, tmp_
|
|||||||
assert (
|
assert (
|
||||||
"EXTERNAL API" not in out
|
"EXTERNAL API" not in out
|
||||||
), f"Privacy warning fired on --no-llm path — should not have. Got: {out!r}"
|
), f"Privacy warning fired on --no-llm path — should not have. Got: {out!r}"
|
||||||
|
|
||||||
|
|
||||||
|
# ─────────────────────────────────────────────────────────────────────────
|
||||||
|
# Consent gate for stray env-fallback API keys (issue #26).
|
||||||
|
#
|
||||||
|
# The #1224 warning is informational — init keeps going. That's
|
||||||
|
# "warning theater" if a user wasn't paying attention. #26 adds a
|
||||||
|
# blocking [y/N] prompt when the api_key was acquired via env fallback
|
||||||
|
# (OPENAI_API_KEY / ANTHROPIC_API_KEY) AND the endpoint is external.
|
||||||
|
# Explicit --llm-api-key (api_key_source == "flag") = user opted in.
|
||||||
|
# --accept-external-llm bypasses for CI / non-interactive.
|
||||||
|
# ─────────────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
def _external_env_provider():
|
||||||
|
"""Build a fake provider matching the 'stray env-fallback API key
|
||||||
|
pointed at external endpoint' scenario — the case #26 must gate."""
|
||||||
|
p = MagicMock()
|
||||||
|
p.check_available.return_value = (True, "ok")
|
||||||
|
p.is_external_service = True
|
||||||
|
p.api_key_source = "env"
|
||||||
|
p.classify.return_value = MagicMock(text='{"classifications": []}')
|
||||||
|
return p
|
||||||
|
|
||||||
|
|
||||||
|
def test_init_blocks_with_consent_prompt_when_api_key_from_env(
|
||||||
|
ai_dialogue_corpus: Path, tmp_path: Path, capsys
|
||||||
|
):
|
||||||
|
"""When provider is external AND api_key_source=='env' AND
|
||||||
|
--accept-external-llm is NOT set, cmd_init MUST call input() to
|
||||||
|
block on user consent. No bypass = blocking prompt."""
|
||||||
|
from mempalace.cli import cmd_init
|
||||||
|
|
||||||
|
palace = tmp_path / "palace"
|
||||||
|
args = _init_args(ai_dialogue_corpus)
|
||||||
|
fake_provider = _external_env_provider()
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("mempalace.cli.MempalaceConfig", return_value=_stub_cfg(palace)),
|
||||||
|
patch("mempalace.cli.get_provider", return_value=fake_provider),
|
||||||
|
patch("mempalace.cli._maybe_run_mine_after_init"),
|
||||||
|
patch("mempalace.room_detector_local.detect_rooms_local"),
|
||||||
|
patch("builtins.input", return_value="y") as mock_input,
|
||||||
|
):
|
||||||
|
cmd_init(args)
|
||||||
|
|
||||||
|
assert mock_input.called, (
|
||||||
|
"Stray env-fallback api_key + external endpoint MUST trigger a "
|
||||||
|
"blocking consent prompt. input() was never called."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_init_consent_prompt_y_proceeds_with_llm(ai_dialogue_corpus: Path, tmp_path: Path, capsys):
|
||||||
|
"""If user types 'y' at the consent prompt, init proceeds with the
|
||||||
|
LLM — provider.classify() is invoked during Pass 0 / refinement."""
|
||||||
|
from mempalace.cli import cmd_init
|
||||||
|
|
||||||
|
palace = tmp_path / "palace"
|
||||||
|
args = _init_args(ai_dialogue_corpus)
|
||||||
|
fake_provider = _external_env_provider()
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("mempalace.cli.MempalaceConfig", return_value=_stub_cfg(palace)),
|
||||||
|
patch("mempalace.cli.get_provider", return_value=fake_provider),
|
||||||
|
patch("mempalace.cli._maybe_run_mine_after_init"),
|
||||||
|
patch("mempalace.room_detector_local.detect_rooms_local"),
|
||||||
|
patch("builtins.input", return_value="y"),
|
||||||
|
):
|
||||||
|
cmd_init(args)
|
||||||
|
|
||||||
|
assert fake_provider.classify.called, (
|
||||||
|
"After 'y' consent, the LLM provider must be used. "
|
||||||
|
"classify() was never called — gate dropped llm_provider on the floor."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_init_consent_prompt_n_falls_back_to_heuristic(
|
||||||
|
ai_dialogue_corpus: Path, tmp_path: Path, capsys
|
||||||
|
):
|
||||||
|
"""If user types 'n' (or anything not 'y'), init drops the LLM and
|
||||||
|
falls back to heuristics-only — provider.classify() must NOT run."""
|
||||||
|
from mempalace.cli import cmd_init
|
||||||
|
|
||||||
|
palace = tmp_path / "palace"
|
||||||
|
args = _init_args(ai_dialogue_corpus)
|
||||||
|
fake_provider = _external_env_provider()
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("mempalace.cli.MempalaceConfig", return_value=_stub_cfg(palace)),
|
||||||
|
patch("mempalace.cli.get_provider", return_value=fake_provider),
|
||||||
|
patch("mempalace.cli._maybe_run_mine_after_init"),
|
||||||
|
patch("mempalace.room_detector_local.detect_rooms_local"),
|
||||||
|
patch("builtins.input", return_value="n"),
|
||||||
|
):
|
||||||
|
cmd_init(args)
|
||||||
|
|
||||||
|
assert not fake_provider.classify.called, (
|
||||||
|
"Declined consent ('n') must drop the provider — classify() "
|
||||||
|
"should never be invoked when the user said no."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_init_no_consent_prompt_when_api_key_from_flag(
|
||||||
|
ai_dialogue_corpus: Path, tmp_path: Path, capsys
|
||||||
|
):
|
||||||
|
"""Explicit --llm-api-key means user already opted in. The consent
|
||||||
|
prompt MUST NOT fire when api_key_source == 'flag', even if the
|
||||||
|
endpoint is external."""
|
||||||
|
from mempalace.cli import cmd_init
|
||||||
|
|
||||||
|
palace = tmp_path / "palace"
|
||||||
|
args = _init_args(ai_dialogue_corpus, llm_api_key="sk-explicit")
|
||||||
|
fake_provider = MagicMock()
|
||||||
|
fake_provider.check_available.return_value = (True, "ok")
|
||||||
|
fake_provider.is_external_service = True
|
||||||
|
fake_provider.api_key_source = "flag" # explicit flag = no gate
|
||||||
|
fake_provider.classify.return_value = MagicMock(text='{"classifications": []}')
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("mempalace.cli.MempalaceConfig", return_value=_stub_cfg(palace)),
|
||||||
|
patch("mempalace.cli.get_provider", return_value=fake_provider),
|
||||||
|
patch("mempalace.cli._maybe_run_mine_after_init"),
|
||||||
|
patch("mempalace.room_detector_local.detect_rooms_local"),
|
||||||
|
patch("builtins.input") as mock_input,
|
||||||
|
):
|
||||||
|
cmd_init(args)
|
||||||
|
|
||||||
|
assert not mock_input.called, (
|
||||||
|
"Explicit --llm-api-key (api_key_source='flag') must NOT trigger "
|
||||||
|
"the consent prompt. User already opted in by passing the flag."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_init_accept_external_llm_flag_bypasses_consent_prompt(
|
||||||
|
ai_dialogue_corpus: Path, tmp_path: Path, capsys
|
||||||
|
):
|
||||||
|
"""--accept-external-llm is the non-interactive bypass for CI. With
|
||||||
|
the flag set, the consent prompt MUST NOT fire even when the
|
||||||
|
api_key came from env-fallback."""
|
||||||
|
from mempalace.cli import cmd_init
|
||||||
|
|
||||||
|
palace = tmp_path / "palace"
|
||||||
|
args = _init_args(ai_dialogue_corpus, accept_external_llm=True)
|
||||||
|
fake_provider = _external_env_provider()
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("mempalace.cli.MempalaceConfig", return_value=_stub_cfg(palace)),
|
||||||
|
patch("mempalace.cli.get_provider", return_value=fake_provider),
|
||||||
|
patch("mempalace.cli._maybe_run_mine_after_init"),
|
||||||
|
patch("mempalace.room_detector_local.detect_rooms_local"),
|
||||||
|
patch("builtins.input") as mock_input,
|
||||||
|
):
|
||||||
|
cmd_init(args)
|
||||||
|
|
||||||
|
assert not mock_input.called, (
|
||||||
|
"--accept-external-llm must bypass the consent prompt for "
|
||||||
|
"non-interactive / CI use. input() was called anyway."
|
||||||
|
)
|
||||||
|
assert (
|
||||||
|
fake_provider.classify.called
|
||||||
|
), "With --accept-external-llm, init must proceed with the LLM."
|
||||||
|
|
||||||
|
|
||||||
|
def test_init_no_consent_prompt_when_endpoint_is_local(
|
||||||
|
ai_dialogue_corpus: Path, tmp_path: Path, capsys
|
||||||
|
):
|
||||||
|
"""Stray env-fallback api_key on a LOCAL endpoint (e.g. LM Studio
|
||||||
|
on localhost with OPENAI_API_KEY in shell env) must NOT trigger the
|
||||||
|
prompt. Nothing leaves the machine — no consent needed."""
|
||||||
|
from mempalace.cli import cmd_init
|
||||||
|
|
||||||
|
palace = tmp_path / "palace"
|
||||||
|
args = _init_args(ai_dialogue_corpus)
|
||||||
|
fake_provider = MagicMock()
|
||||||
|
fake_provider.check_available.return_value = (True, "ok")
|
||||||
|
fake_provider.is_external_service = False # localhost / LAN — no leak
|
||||||
|
fake_provider.api_key_source = "env" # stray key, but URL is local
|
||||||
|
fake_provider.classify.return_value = MagicMock(text='{"classifications": []}')
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("mempalace.cli.MempalaceConfig", return_value=_stub_cfg(palace)),
|
||||||
|
patch("mempalace.cli.get_provider", return_value=fake_provider),
|
||||||
|
patch("mempalace.cli._maybe_run_mine_after_init"),
|
||||||
|
patch("mempalace.room_detector_local.detect_rooms_local"),
|
||||||
|
patch("builtins.input") as mock_input,
|
||||||
|
):
|
||||||
|
cmd_init(args)
|
||||||
|
|
||||||
|
assert not mock_input.called, (
|
||||||
|
"Local endpoint (is_external_service=False) must NOT trigger the "
|
||||||
|
"consent prompt regardless of api_key_source. Nothing leaves the box."
|
||||||
|
)
|
||||||
|
|||||||
@@ -426,3 +426,60 @@ def test_openai_compat_provider_outside_tailscale_cgnat_is_external():
|
|||||||
f"Address {endpoint} ({label}) is OUTSIDE Tailscale CGNAT and "
|
f"Address {endpoint} ({label}) is OUTSIDE Tailscale CGNAT and "
|
||||||
f"should remain external; got is_external_service={p.is_external_service}"
|
f"should remain external; got is_external_service={p.is_external_service}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ── api_key_source provenance tracking (issue #26) ──────────────────────
|
||||||
|
#
|
||||||
|
# Distinguishes whether `api_key` was set via explicit constructor arg
|
||||||
|
# (= --llm-api-key flag → "flag") vs via environment-variable fallback
|
||||||
|
# (OPENAI_API_KEY / ANTHROPIC_API_KEY → "env"). cmd_init uses this to
|
||||||
|
# decide whether to block on a consent prompt: stray env-fallback keys
|
||||||
|
# require explicit user confirmation; explicit flag-passed keys are
|
||||||
|
# treated as already-consented.
|
||||||
|
|
||||||
|
|
||||||
|
def test_openai_compat_api_key_source_flag_when_explicit(monkeypatch):
|
||||||
|
"""When ``api_key`` is passed explicitly to the constructor, the
|
||||||
|
provider records ``api_key_source == "flag"`` even if the same env
|
||||||
|
var is also set. Flag wins over env."""
|
||||||
|
monkeypatch.setenv("OPENAI_API_KEY", "sk-from-env-irrelevant")
|
||||||
|
p = OpenAICompatProvider(model="x", endpoint="http://h", api_key="sk-from-flag")
|
||||||
|
assert p.api_key == "sk-from-flag"
|
||||||
|
assert (
|
||||||
|
p.api_key_source == "flag"
|
||||||
|
), f"Explicit api_key arg must produce api_key_source='flag'; got {p.api_key_source!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_openai_compat_api_key_source_env_when_fallback(monkeypatch):
|
||||||
|
"""When ``api_key`` arg is None but ``OPENAI_API_KEY`` is set, the
|
||||||
|
provider falls back to env and records ``api_key_source == "env"``.
|
||||||
|
This is the "stray key" case — user didn't explicitly authorize this
|
||||||
|
run to use the env-resolved credential."""
|
||||||
|
monkeypatch.setenv("OPENAI_API_KEY", "sk-from-env")
|
||||||
|
p = OpenAICompatProvider(model="x", endpoint="http://h")
|
||||||
|
assert p.api_key == "sk-from-env"
|
||||||
|
assert (
|
||||||
|
p.api_key_source == "env"
|
||||||
|
), f"Env-fallback api_key must produce api_key_source='env'; got {p.api_key_source!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_anthropic_api_key_source_tracking(monkeypatch):
|
||||||
|
"""AnthropicProvider tracks api_key_source the same way: 'flag' when
|
||||||
|
passed explicitly, 'env' when resolved from ANTHROPIC_API_KEY env."""
|
||||||
|
monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-env")
|
||||||
|
p_flag = AnthropicProvider(model="claude-haiku", api_key="sk-ant-flag")
|
||||||
|
assert (
|
||||||
|
p_flag.api_key_source == "flag"
|
||||||
|
), f"Explicit api_key must produce 'flag'; got {p_flag.api_key_source!r}"
|
||||||
|
p_env = AnthropicProvider(model="claude-haiku")
|
||||||
|
assert p_env.api_key == "sk-ant-env"
|
||||||
|
assert (
|
||||||
|
p_env.api_key_source == "env"
|
||||||
|
), f"Env-fallback must produce 'env'; got {p_env.api_key_source!r}"
|
||||||
|
|
||||||
|
|
||||||
|
def test_ollama_api_key_source_is_none():
|
||||||
|
"""Ollama doesn't use api_key at all; ``api_key_source`` should be None."""
|
||||||
|
p = OllamaProvider(model="gemma4:e4b")
|
||||||
|
assert p.api_key is None
|
||||||
|
assert p.api_key_source is None
|
||||||
|
|||||||
Reference in New Issue
Block a user