Merge pull request #1233 from MemPalace/feat/privacy-consent-prompt
feat(privacy): blocking consent gate for env-fallback LLM API keys
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"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:
|
||||
print(
|
||||
f" No LLM provider reachable ({msg}). "
|
||||
@@ -994,6 +1021,16 @@ def main():
|
||||
"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
|
||||
p_mine = sub.add_parser("mine", help="Mine files into the palace")
|
||||
|
||||
+30
-4
@@ -127,11 +127,18 @@ class LLMProvider:
|
||||
endpoint: Optional[str] = None,
|
||||
api_key: Optional[str] = None,
|
||||
timeout: int = 120,
|
||||
api_key_source: Optional[str] = None,
|
||||
):
|
||||
self.model = model
|
||||
self.endpoint = endpoint
|
||||
self.api_key = api_key
|
||||
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:
|
||||
raise NotImplementedError
|
||||
@@ -253,8 +260,20 @@ class OpenAICompatProvider(LLMProvider):
|
||||
timeout: int = 120,
|
||||
**_: object,
|
||||
):
|
||||
resolved_key = api_key or os.environ.get("OPENAI_API_KEY")
|
||||
super().__init__(model=model, endpoint=endpoint, api_key=resolved_key, timeout=timeout)
|
||||
if api_key:
|
||||
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:
|
||||
if not self.endpoint:
|
||||
@@ -321,12 +340,19 @@ class AnthropicProvider(LLMProvider):
|
||||
timeout: int = 120,
|
||||
**_: 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__(
|
||||
model=model,
|
||||
endpoint=endpoint or self.DEFAULT_ENDPOINT,
|
||||
api_key=key,
|
||||
api_key=resolved_key,
|
||||
timeout=timeout,
|
||||
api_key_source=source,
|
||||
)
|
||||
|
||||
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 (
|
||||
"EXTERNAL API" not in out
|
||||
), 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"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