mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-24 16:30:27 +08:00
Address CI feedback on the test/evals move.
mypy: the directory move put the harness under test/, which mypy actually scans -- three pre-existing type errors surfaced as a result: - tasks.py: _EvalConfig is a structural proxy for GalaxyAppConfiguration, cast at the construction site so the duck-typing is explicit. - seed_staining_quantification_history.py: DatasetPopulator names ApiTestInteractor in its signature even though GalaxyInteractorApi satisfies the protocol; cast at the boundary. - test_live_evals.py: galaxy_interactor.api_key is str | None; assert it before passing on. CodeQL: four py/clear-text-logging-sensitive-data alerts, all false positives from taint tracking. - Split _resolve_model_endpoint into _resolve_proxy_url and _resolve_api_key so the proxy URL value never travels through the same tuple as the api_key; CodeQL was unable to tell the two apart. The print outputs now show the URL without inheriting key taint. - Suppressed the two history_id alerts with lgtm comments + a one-line explanation. history_id is a Galaxy history id, not a credential, but CodeQL traces api_key -> populator -> new_history() return and marks anything derived sensitive.
This commit is contained in:
+22
-6
@@ -81,25 +81,39 @@ def _git_sha() -> str:
|
||||
return "unknown"
|
||||
|
||||
|
||||
def _resolve_model_endpoint(model: str, model_config: dict[str, Any]) -> tuple[str, str]:
|
||||
"""Return (proxy_url, api_key) for a model declared in model_config."""
|
||||
def _require_model_entry(model: str, model_config: dict[str, Any]) -> dict[str, Any]:
|
||||
entry = model_config.get(model)
|
||||
if not entry:
|
||||
raise SystemExit(
|
||||
f"Model '{model}' is not declared in the model-config YAML. "
|
||||
f"Known: {', '.join(model_config) or '(none)'}."
|
||||
)
|
||||
return entry
|
||||
|
||||
|
||||
def _resolve_proxy_url(model: str, model_config: dict[str, Any]) -> str:
|
||||
"""Return the proxy URL for a model. Kept separate from key resolution
|
||||
so the URL value never flows through the same tuple as the api_key
|
||||
(CodeQL's taint tracking otherwise marks both as sensitive).
|
||||
"""
|
||||
entry = _require_model_entry(model, model_config)
|
||||
proxy_url = entry.get("proxy_url")
|
||||
if not proxy_url:
|
||||
raise SystemExit(f"Model '{model}' is missing proxy_url in the YAML.")
|
||||
return proxy_url
|
||||
|
||||
|
||||
def _resolve_api_key(model: str, model_config: dict[str, Any]) -> str:
|
||||
"""Return the api_key for a model -- either inline or from the env var."""
|
||||
entry = _require_model_entry(model, model_config)
|
||||
if "api_key" in entry:
|
||||
return proxy_url, entry["api_key"]
|
||||
return entry["api_key"]
|
||||
api_key_env = entry.get("api_key_env")
|
||||
if api_key_env:
|
||||
api_key = os.environ.get(api_key_env)
|
||||
if not api_key:
|
||||
raise SystemExit(f"Model '{model}' requires env var {api_key_env} (not set).")
|
||||
return proxy_url, api_key
|
||||
return api_key
|
||||
raise SystemExit(f"Model '{model}' needs either api_key or api_key_env in the YAML.")
|
||||
|
||||
|
||||
@@ -587,7 +601,8 @@ async def run_eval_suite(
|
||||
if ds not in SPECS:
|
||||
raise ValueError(f"Unknown dataset: {ds}. Known: {', '.join(SPECS)}.")
|
||||
|
||||
judge_proxy_url, judge_api_key = _resolve_model_endpoint(judge_model_name, model_config)
|
||||
judge_proxy_url = _resolve_proxy_url(judge_model_name, model_config)
|
||||
judge_api_key = _resolve_api_key(judge_model_name, model_config)
|
||||
judge_model = build_judge_model(judge_model_name, judge_proxy_url, judge_api_key)
|
||||
print(f"Judge: {judge_model_name} (via {judge_proxy_url})", file=sys.stderr)
|
||||
|
||||
@@ -595,7 +610,8 @@ async def run_eval_suite(
|
||||
for ds_name in datasets:
|
||||
spec_fn = SPECS[ds_name]
|
||||
for model in models:
|
||||
proxy_url, api_key = _resolve_model_endpoint(model, model_config)
|
||||
proxy_url = _resolve_proxy_url(model, model_config)
|
||||
api_key = _resolve_api_key(model, model_config)
|
||||
print(f"\n=== {ds_name} | {model} (via {proxy_url}) ===", file=sys.stderr)
|
||||
deps = deps_factory(model, api_key, proxy_url)
|
||||
usage_buffer: list[dict[str, int]] = []
|
||||
|
||||
@@ -141,7 +141,10 @@ def _standalone_main(argv: Optional[list[str]] = None) -> int:
|
||||
parser.add_argument("--galaxy-api-key", required=True, help="API key for the user to seed for.")
|
||||
args = parser.parse_args(argv)
|
||||
|
||||
from typing import cast
|
||||
|
||||
from galaxy.tool_util.verify.interactor import GalaxyInteractorApi
|
||||
from galaxy_test.base.api import ApiTestInteractor
|
||||
from galaxy_test.base.populators import DatasetPopulator
|
||||
|
||||
interactor = GalaxyInteractorApi(
|
||||
@@ -149,8 +152,15 @@ def _standalone_main(argv: Optional[list[str]] = None) -> int:
|
||||
master_api_key=args.galaxy_api_key,
|
||||
api_key=args.galaxy_api_key,
|
||||
)
|
||||
populator = DatasetPopulator(interactor)
|
||||
# DatasetPopulator's interactor protocol is satisfied by both
|
||||
# GalaxyInteractorApi (used here for standalone runs) and
|
||||
# ApiTestInteractor (used by the pytest fixture), but the populator's
|
||||
# signature only names the test type. Cast at this boundary.
|
||||
populator = DatasetPopulator(cast(ApiTestInteractor, interactor))
|
||||
history_id = seed_demo_history(populator)
|
||||
# lgtm[py/clear-text-logging-sensitive-data] -- history_id is a Galaxy
|
||||
# history id, not a credential. CodeQL flags it because the populator
|
||||
# was constructed with args.galaxy_api_key.
|
||||
print(f"Seeded history '{HISTORY_NAME}' at id {history_id}")
|
||||
return 0
|
||||
|
||||
|
||||
+9
-1
@@ -12,10 +12,15 @@ from collections.abc import (
|
||||
)
|
||||
from typing import (
|
||||
Any,
|
||||
cast,
|
||||
Optional,
|
||||
TYPE_CHECKING,
|
||||
)
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from galaxy.config import GalaxyAppConfiguration
|
||||
|
||||
from galaxy.agents.base import (
|
||||
extract_result_content,
|
||||
extract_usage_info,
|
||||
@@ -166,7 +171,10 @@ def make_live_deps(
|
||||
return GalaxyAgentDependencies(
|
||||
trans=trans,
|
||||
user=trans.user,
|
||||
config=config,
|
||||
# _EvalConfig is a structural proxy: agents only touch ai_* and
|
||||
# inference_services, both of which it provides; the rest falls
|
||||
# through to base_config via __getattr__.
|
||||
config=cast("GalaxyAppConfiguration", config),
|
||||
get_agent=_registry.get_agent,
|
||||
)
|
||||
|
||||
|
||||
@@ -108,6 +108,10 @@ class TestLiveEvals(IntegrationTestCase):
|
||||
documented at the top of the file.
|
||||
"""
|
||||
history_id = seed_demo_history(self.dataset_populator)
|
||||
# lgtm[py/clear-text-logging-sensitive-data] -- history_id is a
|
||||
# Galaxy history id, not a credential. CodeQL flags it because the
|
||||
# populator was constructed with self.galaxy_interactor (which holds
|
||||
# an api_key), so any value derived from it inherits the taint.
|
||||
log.info("Seeded staining quantification fixture history: %s", history_id)
|
||||
|
||||
datasets = [
|
||||
@@ -140,7 +144,9 @@ class TestLiveEvals(IntegrationTestCase):
|
||||
# ERROR out before the agent's response is even scored.
|
||||
from galaxy.webapps.galaxy.api.mcp import get_mcp_url_builder
|
||||
|
||||
user = self._user_for_api_key(self.galaxy_interactor.api_key)
|
||||
api_key = self.galaxy_interactor.api_key
|
||||
assert api_key, "Test setup must provide a galaxy_interactor.api_key"
|
||||
user = self._user_for_api_key(api_key)
|
||||
history = self._history_for_id(history_id)
|
||||
url_builder = get_mcp_url_builder(self.url)
|
||||
trans = WorkRequestContext(app=self._app, user=user, history=history, url_builder=url_builder)
|
||||
|
||||
Reference in New Issue
Block a user