diff --git a/src/data_concierge/data_layer/onboard_index.py b/src/data_concierge/data_layer/onboard_index.py index 93813c5..5dea562 100644 --- a/src/data_concierge/data_layer/onboard_index.py +++ b/src/data_concierge/data_layer/onboard_index.py @@ -31,7 +31,8 @@ _PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent.parent # qsv describegpt records its full invocation in the AI-generated descriptions, -# and that command carries the OpenRouter API key on --api-key. Those +# and onboarding runs before the key moved to QSV_LLM_APIKEY (see +# qsv_profiling.run_qsv_describegpt) put the OpenRouter API key on --api-key. Those # descriptions are served over the Fair Store API, rendered in the data # dictionary UI, and fed into the agent's prompt — so the key would leak three # ways. Redact provider secrets and the key flag before any description is used. diff --git a/src/data_concierge/data_layer/qsv_profiling.py b/src/data_concierge/data_layer/qsv_profiling.py index 161a840..04e4156 100644 --- a/src/data_concierge/data_layer/qsv_profiling.py +++ b/src/data_concierge/data_layer/qsv_profiling.py @@ -30,6 +30,7 @@ import asyncio import csv import json +import os from datetime import UTC, datetime from io import StringIO from pathlib import Path @@ -65,20 +66,25 @@ async def run_qsv_describegpt( api_key: str, output_path: Path, ) -> dict | None: - """Run qsv describegpt and return parsed JSON output.""" + """Run qsv describegpt and return parsed JSON output. + + The API key goes to qsv in ``QSV_LLM_APIKEY``, not ``--api-key``: describegpt + copies its command line into the descriptions it generates, and argv is also + visible to other local users in ``ps``. + """ cmd = [ "qsv", "describegpt", str(csv_path), "--all", "--format", "json", "--base-url", "https://openrouter.ai/api/v1", "--model", "google/gemini-2.5-flash-lite", - "--api-key", api_key, ] proc = await asyncio.create_subprocess_exec( *cmd, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, + env={**os.environ, "QSV_LLM_APIKEY": api_key}, ) stdout, stderr = await proc.communicate() @@ -399,8 +405,9 @@ def sync_to_storage(base_dir: Path, site_id: str, prefix: str = "ckan_onboard") backend when the admin panel starts a mirror on Cloud Run. JSON is scrubbed of provider keys on the way: qsv describegpt records its - own command line, ``--api-key`` included, in ``qsv_dict.json``, - ``meta.json`` and ``index.json``. + own command line in ``qsv_dict.json``, ``meta.json`` and ``index.json``, + and onboarding runs before the key moved to ``QSV_LLM_APIKEY`` passed it + on ``--api-key``. """ synced = 0 manifest = sync_manifest(base_dir, prefix) diff --git a/tests/unit/test_qsv_profiling.py b/tests/unit/test_qsv_profiling.py new file mode 100644 index 0000000..9e7bc4d --- /dev/null +++ b/tests/unit/test_qsv_profiling.py @@ -0,0 +1,42 @@ +"""Tests for data_layer.qsv_profiling.""" + +from __future__ import annotations + +import json +import os +import sys + +import pytest + +from data_concierge.data_layer.qsv_profiling import run_qsv_describegpt + +KEY = "sk-or-v1-0123456789abcdef" + + +@pytest.mark.skipif(sys.platform == "win32", reason="fake qsv is a POSIX shell script") +async def test_describegpt_gets_the_api_key_from_the_environment_not_argv(tmp_path, monkeypatch): + # describegpt copies its argv into the descriptions it writes, so the key must not be in it. + record = tmp_path / "record.json" + fake = tmp_path / "bin" / "qsv" + fake.parent.mkdir() + fake.write_text( + f"#!{sys.executable}\n" + "import json, os, sys\n" + f"json.dump({{'argv': sys.argv[1:], 'key': os.environ.get('QSV_LLM_APIKEY')}}," + f" open({str(record)!r}, 'w'))\n" + "print(json.dumps({'description': 'ok'}))\n" + ) + fake.chmod(0o755) + monkeypatch.setenv("PATH", f"{fake.parent}:{os.environ['PATH']}") + monkeypatch.delenv("QSV_LLM_APIKEY", raising=False) + + out = tmp_path / "qsv_dict.json" + data = await run_qsv_describegpt(tmp_path / "data.csv", KEY, out) + + assert data == {"description": "ok"} + assert json.loads(out.read_text()) == {"description": "ok"} + seen = json.loads(record.read_text()) + assert seen["key"] == KEY + assert KEY not in " ".join(seen["argv"]) + assert "--api-key" not in seen["argv"] + assert seen["argv"][:2] == ["describegpt", str(tmp_path / "data.csv")]