From 4bf6f8dc90ebf46fad354babfc061c521774daeb Mon Sep 17 00:00:00 2001 From: Manish Kumar Date: Thu, 24 Sep 2026 23:35:59 -0500 Subject: [PATCH 1/2] build: make the delegated-auth ToolServer image buildable requirements.txt pins omnibioai-iam-client (private repo) via git+https for the HIPAA-V2-019 delegated-execution auth added in a4cdf8a, but the slim base image had no git and no credential path, so `docker build` failed at `pip install`. No image containing a4cdf8a was ever built, and production kept running the 2026-09-04 image, whose /runs, /validate and /register_tools have no authentication at all. Install git and pass the GitHub token as a BuildKit secret through an ephemeral GIT_ASKPASS helper, the same pattern as omnibioai-model-registry/Dockerfile. Verified with a local build (arm64, --network none, no credential in image history or filesystem): /runs*, POST /runs and /validate return 401 without a credential or with a forged bearer; /register_tools returns 501; /capabilities and /health stay 200. Build: docker build --secret id=github_token,src= . Co-Authored-By: Claude Opus 5.5 --- Dockerfile | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index d986e01..80e340d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,3 +1,4 @@ +# syntax=docker/dockerfile:1 # omnibioai-toolserver/Dockerfile.new FROM python:3.12-slim-bookworm @@ -6,13 +7,33 @@ LABEL org.opencontainers.image.source=https://github.com/man4ish/omnibioai WORKDIR /app RUN apt-get update && apt-get install -y --no-install-recommends \ - ca-certificates curl \ + ca-certificates curl git \ && rm -rf /var/lib/apt/lists/* ENV PYTHONDONTWRITEBYTECODE=1 PYTHONUNBUFFERED=1 COPY requirements.txt . -RUN pip install --no-cache-dir -r requirements.txt +# HIPAA-V2-019: requirements.txt pins omnibioai-iam-client (private repo) +# as a git+https dependency, so pip needs git plus a GitHub credential. +# Same pattern as omnibioai-model-registry/Dockerfile: a BuildKit secret +# mount (never an ARG, which BuildKit echoes into build logs) read through +# an ephemeral GIT_ASKPASS helper, so git never builds a credentialed URL +# that a failed clone could print. Without this the image could not be +# built at all, which is how the delegated-auth fix (a4cdf8a) never +# reached the running deployment. +RUN --mount=type=secret,id=github_token \ + set -eu; \ + printf '%s\n' \ + '#!/bin/sh' \ + 'case "$1" in' \ + ' *Username*) printf "%s\n" "x-access-token" ;;' \ + ' *Password*) cat /run/secrets/github_token ;;' \ + ' *) exit 1 ;;' \ + 'esac' > /tmp/git-askpass; \ + chmod 700 /tmp/git-askpass; \ + trap 'rm -f /tmp/git-askpass' EXIT; \ + GIT_ASKPASS=/tmp/git-askpass GIT_TERMINAL_PROMPT=0 \ + pip install --no-cache-dir -r requirements.txt COPY toolserver/ ./toolserver/ COPY toolserver_app.py . From 164475230c20d8904851559bd4058d5f111e64a4 Mon Sep 17 00:00:00 2001 From: Manish Kumar Date: Fri, 25 Sep 2026 07:25:41 -0500 Subject: [PATCH 2/2] security(hipaa): accept TES's service-only credential on /register_tools HIPAA-V2-019. /register_tools was disabled (501) because no permission model existed for it, which would have dropped TES's 1,716 HTTP tools on deploy. It is re-enabled behind require_tes_registration: Auth's service-only `toolserver_registration` credential, verified live via iam_client.registration (v0.1.5), from a service identity listed in TOOLSERVER_REGISTRATION_CLIENT_IDS (empty denies all). Only tools with an http block are registered; the service id and counts are logged, never the credential. /runs and /validate still accept only delegated-execution credentials, which a registration credential can never satisfy. Pins omnibioai-iam-client to v0.1.5. Tests: 402 passed (17 new); four old 501-contract tests now assert 401 for delegated or anonymous callers. Co-Authored-By: Claude Opus 5.5 --- pyproject.toml | 27 +-- requirements.txt | 16 +- tests/test_app.py | 13 +- tests/test_toolserver_app_coverage.py | 48 ++-- tests/test_toolserver_delegated_auth.py | 27 ++- tests/test_toolserver_registration_auth.py | 251 +++++++++++++++++++++ tests/test_toolserver_run_ownership.py | 14 +- toolserver/security.py | 33 ++- toolserver_app.py | 72 +++--- 9 files changed, 377 insertions(+), 124 deletions(-) create mode 100644 tests/test_toolserver_registration_auth.py diff --git a/pyproject.toml b/pyproject.toml index 6a0157d..fc8771f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,28 +21,11 @@ dependencies = [ # redis + python-jose[cryptography] transitively (see that package's own # pyproject.toml). # - # REMOTE BUILD PENDING TAG PUSH -- v0.1.4 is the correct, intended target. - # It is a real, existing, annotated Git tag as of this change (commit - # 54f4d61, "Release v0.1.4: add delegated execution identity support"), - # independently verified by omnibioai-iam-client's own release-readiness - # review: 129/129 tests pass, the tag builds a real sdist/wheel containing - # iam_client/delegated.py, and a clean non-editable install from that exact - # tag (via `git+file://` locally) successfully imports - # DelegatedExecutionIdentity/validate_delegated_execution/ - # require_delegated_execution/require_delegated_permission and passes all - # 41 delegated tests -- see omnibioai-iam-client's own - # omnibioai-docs/security/hipaa_v2_019_service_identity_design.md entry. - # - # However the tag exists ONLY LOCALLY -- it has not been pushed to - # github.com/OmniBioAI/omnibioai-iam-client. The `git+https://...@v0.1.4` - # reference below will 404 for any checkout/CI/Docker build that doesn't - # already have this exact local repository (i.e. everywhere except this - # machine) until that push happens. This is a deliberate, reported state - # (per this task's explicit "do not misrepresent remote resolvability" - # instruction), not an oversight. Local verification against the real - # local tag (not an editable install) is documented in - # requirements-dev.txt. - "omnibioai-iam-client @ git+https://github.com/OmniBioAI/omnibioai-iam-client.git@v0.1.4", + # v0.1.5 adds iam_client.registration (require_toolserver_registration), + # used by toolserver/security.py::require_tes_registration for the + # service-only TES -> POST /register_tools credential. Private repo: + # Docker builds pass a GitHub token as a BuildKit secret (see Dockerfile). + "omnibioai-iam-client @ git+https://github.com/OmniBioAI/omnibioai-iam-client.git@v0.1.5", ] [project.optional-dependencies] diff --git a/requirements.txt b/requirements.txt index e075f91..8c9144d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,15 +7,7 @@ pyyaml # HIPAA-V2-019: toolserver/security.py's delegated-execution authentication # needs omnibioai-iam-client's iam_client.delegated API. # -# REMOTE BUILD PENDING TAG PUSH -- v0.1.4 is a real, verified, annotated -# Git tag (commit 54f4d61) as of this change, but it exists only in the -# local omnibioai-iam-client checkout on this machine, not on -# github.com/OmniBioAI/omnibioai-iam-client. This Dockerfile's `pip -# install -r requirements.txt` will 404 on this line in any environment -# other than this one until the tag is pushed -- that is the single -# remaining release action, not a bug in this line. See pyproject.toml's -# matching entry and -# omnibioai-docs/security/hipaa_v2_019_service_identity_design.md for the -# full release-readiness evidence (129/129 IAM tests, clean non-editable -# tag install, 41/41 delegated tests against that install). -omnibioai-iam-client @ git+https://github.com/OmniBioAI/omnibioai-iam-client.git@v0.1.4 +# v0.1.5 adds iam_client.registration (require_toolserver_registration) for +# the service-only TES -> POST /register_tools credential. Private repo: the +# Dockerfile passes a GitHub token as a BuildKit secret. +omnibioai-iam-client @ git+https://github.com/OmniBioAI/omnibioai-iam-client.git@v0.1.5 diff --git a/tests/test_app.py b/tests/test_app.py index 0508977..693f83d 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -619,16 +619,17 @@ def test_e2e_failed_run_results_not_ready(self, monkeypatch, tmp_path): # test_toolserver_app_coverage.py's register_tools tests. class TestRegisterToolsDisabled: - """POST /register_tools is unconditionally disabled, with no delegated permission able to unlock it.""" + """POST /register_tools is not unlocked by delegated execution authority.""" def test_register_tools_returns_501_even_when_authenticated(self, ctx): - """An authenticated caller (this file's fixtures always are, via - conftest.py's dependency override) gets the exact same denial as - an unauthenticated one -- there is no delegated permission that - can unlock this endpoint today.""" + """This file's fixtures are authenticated for /runs and /validate + (conftest.py overrides the delegated dependencies), but that + authority does not extend to registration, which requires TES's + separate service-only credential: a request without one is 401. + (Name kept for history; the endpoint now answers 401, not 501.)""" client, _ = ctx resp = client.post("/register_tools", json={"tools": [{"tool_id": "stub_exec_tool"}]}) - assert resp.status_code == 501 + assert resp.status_code == 401 def test_register_tools_disabled_leaves_tool_unregistered(self, ctx): """Since /register_tools is a no-op, a "registered" tool_id never diff --git a/tests/test_toolserver_app_coverage.py b/tests/test_toolserver_app_coverage.py index bf0cc52..2626dd0 100644 --- a/tests/test_toolserver_app_coverage.py +++ b/tests/test_toolserver_app_coverage.py @@ -34,21 +34,14 @@ def test_create_app_yaml_not_found(capsys): # ── Line 120→122: get_results when state != COMPLETED ─────────────────────── # Replace test_get_results_not_ready and add test for line 160 -# ── /register_tools: REGISTER_TOOLS_AUTHORIZATION_MODEL_UNRESOLVED ────────── -# HIPAA-V2-019: this endpoint used to register arbitrary caller-supplied -# HTTP-tool execution definitions with zero authentication and zero -# permission model -- a code-execution-adjacent administrative capability -# that Auth's delegated-execution permission set (workflow.execute, -# runs.read) does not cover. Per the HIPAA-V2-019 follow-up, it is now -# unconditionally disabled (fails closed, 501) rather than left -# reachable under an ill-fitting permission -- see toolserver_app.py's -# own comment on register_tools_endpoint. These four tests, which -# previously asserted successful anonymous registration, are replaced by -# tests asserting the new fail-closed behavior; the stub/http-handler -# registration code paths they used to cover are now unreachable by -# design, not merely untested. -def test_register_tools_disabled_returns_501(): - """Reject a well-formed HTTP-tool registration with 501 and the REGISTER_TOOLS_AUTHORIZATION_MODEL_UNRESOLVED code.""" +# ── /register_tools: anonymous callers are rejected ────────────────────────── +# HIPAA-V2-019: registration requires Auth's service-only +# toolserver_registration credential from an allowlisted service identity +# (TES). The full authenticated contract is in +# tests/test_toolserver_registration_auth.py; these keep the anonymous cases +# next to the rest of the app coverage. +def test_register_tools_anonymous_is_rejected(): + """Reject an anonymous, well-formed HTTP-tool registration with 401.""" from toolserver_app import create_app client = TestClient(create_app()) @@ -61,37 +54,26 @@ def test_register_tools_disabled_returns_501(): "method": "POST" } }]}) - assert resp.status_code == 501 - assert "REGISTER_TOOLS_AUTHORIZATION_MODEL_UNRESOLVED" in resp.json()["detail"] + assert resp.status_code == 401 -def test_register_tools_disabled_regardless_of_payload_shape(): - """Reject a minimal/stub-shaped registration payload with 501 just like a full one.""" - from toolserver_app import create_app - client = TestClient(create_app()) - - resp = client.post("/register_tools", json={"tools": [{"tool_id": "my_stub_tool"}]}) - assert resp.status_code == 501 - - -def test_register_tools_disabled_for_empty_tools_list(): - """Reject registration with 501 even when the tools list is empty.""" +def test_register_tools_anonymous_rejected_for_empty_tools_list(): + """Reject anonymous registration with 401 even when the tools list is empty.""" from toolserver_app import create_app client = TestClient(create_app()) resp = client.post("/register_tools", json={"tools": []}) - assert resp.status_code == 501 + assert resp.status_code == 401 -def test_register_tools_disabled_does_not_mutate_registry(): - """No tool_def -- valid or not -- can be registered through this - endpoint anymore; nothing about a request body changes that.""" +def test_register_tools_anonymous_does_not_mutate_registry(): + """An anonymous request can never add a tool, whatever its body.""" from toolserver_app import create_app client = TestClient(create_app()) client.post("/register_tools", json={"tools": [ {}, - {"tool_id": "should_never_register"}, + {"tool_id": "should_never_register", "http": {"url": "http://example.com"}}, ]}) caps = client.get("/capabilities").json() tool_ids = {t["tool_id"] for t in caps["tools"]} diff --git a/tests/test_toolserver_delegated_auth.py b/tests/test_toolserver_delegated_auth.py index 0b5a144..cbbea9e 100644 --- a/tests/test_toolserver_delegated_auth.py +++ b/tests/test_toolserver_delegated_auth.py @@ -103,6 +103,9 @@ def _mock_iam( fake_client.validate_delegated_execution = AsyncMock(side_effect=side_effect) else: fake_client.validate_delegated_execution = AsyncMock(return_value=return_value) + # A delegated credential is never a registration credential (Auth's + # registration introspection answers valid=false for it). + fake_client.validate_toolserver_registration = AsyncMock(return_value=None) monkeypatch.setattr(security_mod, "get_iam_client", lambda: fake_client) return fake_client @@ -636,29 +639,25 @@ def test_route_denies_without_any_credential(self, client, monkeypatch, method, # /register_tools: unconditionally disabled, regardless of credential # =========================================================================== -class TestRegisterToolsUnresolved: - """POST /register_tools stays unconditionally disabled (501) under - the new auth layer too -- neither a valid delegated credential nor - the absence of one changes that outcome, since the endpoint is - disabled before any permission check runs.""" +class TestRegisterToolsNotUnlockedByDelegation: + """POST /register_tools accepts only TES's service-only registration + credential (tests/test_toolserver_registration_auth.py). A delegated + user credential -- even one holding both execution permissions -- and + no credential at all are both rejected (401), before any tool is + registered.""" def test_register_tools_denied_even_with_valid_credential(self, client, monkeypatch): - """A request carrying a fully valid, both-permissions delegated - credential still gets 501 from /register_tools -- the disabled - endpoint is reached before any permission check, so no - credential can unlock it.""" + """A fully valid, both-permissions delegated credential cannot register tools.""" _mock_iam(monkeypatch, return_value=BOTH_IDENTITY) resp = client.post( "/register_tools", json={"tools": [{"tool_id": "x"}]}, headers=_bearer(), ) - assert resp.status_code == 501 + assert resp.status_code == 401 def test_register_tools_denied_without_any_credential(self, client): - """A request with no credential at all also gets 501, the same - as an authenticated one -- proving the 501 comes from the - endpoint being disabled, not from an auth failure.""" + """A request with no credential at all is rejected with 401.""" resp = client.post("/register_tools", json={"tools": [{"tool_id": "x"}]}) - assert resp.status_code == 501 + assert resp.status_code == 401 # =========================================================================== diff --git a/tests/test_toolserver_registration_auth.py b/tests/test_toolserver_registration_auth.py new file mode 100644 index 0000000..4d9637a --- /dev/null +++ b/tests/test_toolserver_registration_auth.py @@ -0,0 +1,251 @@ +"""HIPAA-V2-019: POST /register_tools accepts only TES's service-only +registration credential. + +Exercises the real app and the real dependency chain +(toolserver.security.require_tes_registration -> +iam_client.registration.require_toolserver_registration -> +AsyncIAMClient.validate_toolserver_registration). Most tests mock that last +method, the same network boundary tests/test_toolserver_delegated_auth.py +mocks for delegated execution; the cross-credential tests mock the raw Auth +HTTP call instead, so a single token is judged by both introspection +endpoints exactly as production would judge it. + +Developer: + Manish Kumar +""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, MagicMock + +import pytest +from fastapi.testclient import TestClient +from iam_client import AsyncIAMClient, DelegatedExecutionIdentity, ServiceRegistrationIdentity + +import toolserver.security as security_mod + +TES = "omni_client_tesTESTtesTESTtesTEST" +TES_IDENTITY = ServiceRegistrationIdentity(calling_service=TES, organization="1", registration_id="reg-1") +OTHER_IDENTITY = ServiceRegistrationIdentity( + calling_service="omni_client_otherservice", organization="1", registration_id="reg-2" +) +TOKEN = "registration.jwt.token" + + +def _http_tool(tool_id: str) -> dict: + return { + "tool_id": tool_id, + "version": "v1", + "features": {}, + "http": {"url": "http://example.invalid/api", "method": "GET"}, + "inputs": [{"name": "q", "type": "string", "required": True}], + } + + +def _client(monkeypatch, tmp_path, *, allow=TES): + import toolserver_app + from toolserver.store import RunStore + + monkeypatch.setattr( + toolserver_app, "RunStore", lambda root_dir: RunStore(root_dir=str(tmp_path / "runs")) + ) + if allow is None: + monkeypatch.delenv(security_mod.REGISTRATION_CLIENT_IDS_ENV, raising=False) + else: + monkeypatch.setenv(security_mod.REGISTRATION_CLIENT_IDS_ENV, allow) + return TestClient(toolserver_app.create_app(), raise_server_exceptions=False) + + +def _mock_registration(monkeypatch, identity): + fake = MagicMock() + fake.validate_toolserver_registration = AsyncMock(return_value=identity) + fake.validate_delegated_execution = AsyncMock(return_value=None) + monkeypatch.setattr(security_mod, "get_iam_client", lambda: fake) + return fake + + +def _tool_ids(client) -> set[str]: + return {t["tool_id"] for t in client.get("/capabilities").json()["tools"]} + + +# 1. anonymous +def test_anonymous_registration_rejected_and_registry_unchanged(monkeypatch, tmp_path): + client = _client(monkeypatch, tmp_path) + fake = _mock_registration(monkeypatch, TES_IDENTITY) + before = _tool_ids(client) + resp = client.post("/register_tools", json={"tools": [_http_tool("anon_tool")]}) + assert resp.status_code == 401 + assert _tool_ids(client) == before + fake.validate_toolserver_registration.assert_not_awaited() + + +@pytest.mark.parametrize("header", ["Basic abc", "Bearer", "Token x.y.z", "Bearer a b"]) +def test_malformed_authorization_rejected(monkeypatch, tmp_path, header): + client = _client(monkeypatch, tmp_path) + _mock_registration(monkeypatch, TES_IDENTITY) + resp = client.post( + "/register_tools", json={"tools": [_http_tool("t")]}, headers={"Authorization": header} + ) + assert resp.status_code == 401 + + +# 2-4. invalid / expired / wrong audience: Auth introspection answers valid=false +def test_invalid_expired_or_wrong_audience_credential_rejected(monkeypatch, tmp_path): + client = _client(monkeypatch, tmp_path) + _mock_registration(monkeypatch, None) + resp = client.post( + "/register_tools", json={"tools": [_http_tool("t")]}, headers={"Authorization": f"Bearer {TOKEN}"} + ) + assert resp.status_code == 401 + assert "t" not in _tool_ids(client) + + +# 6. non-TES service identity +def test_valid_credential_from_a_non_tes_service_is_forbidden(monkeypatch, tmp_path): + client = _client(monkeypatch, tmp_path) + _mock_registration(monkeypatch, OTHER_IDENTITY) + resp = client.post( + "/register_tools", json={"tools": [_http_tool("t")]}, headers={"Authorization": f"Bearer {TOKEN}"} + ) + assert resp.status_code == 403 + assert "t" not in _tool_ids(client) + + +@pytest.mark.parametrize("allow", [None, "", " , "]) +def test_unconfigured_allowlist_denies_even_tes(monkeypatch, tmp_path, allow): + client = _client(monkeypatch, tmp_path, allow=allow) + _mock_registration(monkeypatch, TES_IDENTITY) + resp = client.post( + "/register_tools", json={"tools": [_http_tool("t")]}, headers={"Authorization": f"Bearer {TOKEN}"} + ) + assert resp.status_code == 403 + + +# 7-8. valid TES credential registers every HTTP tool it sends +def test_valid_tes_credential_registers_every_http_tool(monkeypatch, tmp_path): + client = _client(monkeypatch, tmp_path, allow=f"someone_else,{TES}") + fake = _mock_registration(monkeypatch, TES_IDENTITY) + before = _tool_ids(client) + tools = [_http_tool(f"http_tool_{i}") for i in range(1716)] + resp = client.post("/register_tools", json={"tools": tools}, headers={"Authorization": f"Bearer {TOKEN}"}) + assert resp.status_code == 200 + assert resp.json() == {"ok": True, "registered": 1716, "skipped": 0} + assert _tool_ids(client) == before | {t["tool_id"] for t in tools} + fake.validate_toolserver_registration.assert_awaited_once_with(TOKEN) + + +def test_entries_without_an_http_block_are_skipped_not_stubbed(monkeypatch, tmp_path): + client = _client(monkeypatch, tmp_path) + _mock_registration(monkeypatch, TES_IDENTITY) + resp = client.post( + "/register_tools", + headers={"Authorization": f"Bearer {TOKEN}"}, + json={"tools": [_http_tool("ok_tool"), {"tool_id": "no_http"}, {"http": {"url": "x"}}]}, + ) + assert resp.json() == {"ok": True, "registered": 1, "skipped": 2} + ids = _tool_ids(client) + assert "ok_tool" in ids and "no_http" not in ids + + +# 12. no credential in logs or responses +def test_registration_never_logs_or_echoes_the_credential(monkeypatch, tmp_path, capsys): + client = _client(monkeypatch, tmp_path) + _mock_registration(monkeypatch, TES_IDENTITY) + ok = client.post( + "/register_tools", json={"tools": [_http_tool("t")]}, headers={"Authorization": f"Bearer {TOKEN}"} + ) + _mock_registration(monkeypatch, None) + bad = client.post( + "/register_tools", json={"tools": [_http_tool("t")]}, headers={"Authorization": f"Bearer {TOKEN}"} + ) + out = capsys.readouterr() + assert TOKEN not in out.out + out.err + assert TOKEN not in ok.text and TOKEN not in bad.text + assert f"service={TES}" in out.out + + +# 9-11, 13. one token judged by both Auth introspection endpoints, as in production +def _auth_http(registration_valid: bool, delegated_valid: bool): + def response(body): + r = MagicMock() + r.status_code = 200 + r.json.return_value = body + return r + + async def post(url, json=None, timeout=None): + if url.endswith("/service/delegations/toolserver/registration/introspect"): + return response( + { + "valid": registration_valid, + "client_id": TES, + "organization_id": "1", + "scopes": ["toolserver.register"], + "registration_id": "reg-1", + } + if registration_valid + else {"valid": False} + ) + if url.endswith("/service/delegations/toolserver/introspect"): + return response( + { + "valid": delegated_valid, + "client_id": TES, + "user_id": "42", + "organization_id": "1", + "permissions": ["workflow.execute", "runs.read"], + "delegation_id": "d-1", + } + if delegated_valid + else {"valid": False} + ) + raise AssertionError(f"unexpected Auth call {url}") + + iam = AsyncIAMClient(base_url="http://auth-service:8001", redis_url="redis://localhost:6379") + iam.http = MagicMock() + iam.http.post = AsyncMock(side_effect=post) + return iam + + +def test_registration_credential_cannot_run_validate_or_read_runs(monkeypatch, tmp_path): + client = _client(monkeypatch, tmp_path) + iam = _auth_http(registration_valid=True, delegated_valid=False) + monkeypatch.setattr(security_mod, "get_iam_client", lambda: iam) + h = {"Authorization": f"Bearer {TOKEN.replace('registration', 'aaa')}"} + assert client.post("/register_tools", json={"tools": [_http_tool("t")]}, headers=h).status_code == 200 + body = {"tool_id": "t", "inputs": {"q": "x"}, "resources": {}} + assert client.post("/runs", json=body, headers=h).status_code == 401 + assert client.post("/validate", json=body, headers=h).status_code == 401 + assert client.get("/runs/ts_x", headers=h).status_code == 401 + assert client.get("/runs/ts_x/logs", headers=h).status_code == 401 + assert client.get("/runs/ts_x/results", headers=h).status_code == 401 + + +def test_delegated_user_credential_cannot_register_tools(monkeypatch, tmp_path): + client = _client(monkeypatch, tmp_path) + iam = _auth_http(registration_valid=False, delegated_valid=True) + monkeypatch.setattr(security_mod, "get_iam_client", lambda: iam) + h = {"Authorization": "Bearer aaa.bbb.ccc"} + assert client.post("/register_tools", json={"tools": [_http_tool("t")]}, headers=h).status_code == 401 + # The same delegated credential still works for execution, unchanged. + assert ( + client.post( + "/validate", + json={"tool_id": "enrichr_pathway", "inputs": {"genes": ["TP53"]}, "resources": {}}, + headers=h, + ).status_code + == 200 + ) + + +def test_anonymous_runs_and_validate_remain_rejected(monkeypatch, tmp_path): + client = _client(monkeypatch, tmp_path) + body = {"tool_id": "enrichr_pathway", "inputs": {"genes": ["TP53"]}, "resources": {}} + assert client.post("/runs", json=body).status_code == 401 + assert client.post("/validate", json=body).status_code == 401 + + +def test_delegated_identity_type_unchanged(): + """The delegated-execution identity used by /runs and /validate still + carries a user principal; the registration identity deliberately has none.""" + assert "initiating_user" in DelegatedExecutionIdentity.__dataclass_fields__ + assert "initiating_user" not in ServiceRegistrationIdentity.__dataclass_fields__ diff --git a/tests/test_toolserver_run_ownership.py b/tests/test_toolserver_run_ownership.py index 888dd81..aff1c7e 100644 --- a/tests/test_toolserver_run_ownership.py +++ b/tests/test_toolserver_run_ownership.py @@ -16,7 +16,7 @@ from __future__ import annotations import time -from unittest.mock import MagicMock +from unittest.mock import AsyncMock, MagicMock import pytest from fastapi.testclient import TestClient @@ -100,6 +100,9 @@ async def _validate(token, permission): return identity fake_client.validate_delegated_execution = _validate + # A delegated credential is never a registration credential (Auth's + # registration introspection answers valid=false for it). + fake_client.validate_toolserver_registration = AsyncMock(return_value=None) monkeypatch.setattr(security_mod, "get_iam_client", lambda: fake_client) return fake_client @@ -640,13 +643,12 @@ def test_validate_still_denied_without_credential(self, client): assert resp.status_code == 401 def test_register_tools_remains_fail_closed(self, client, monkeypatch): - """/register_tools remains unconditionally disabled (501) even - for an otherwise validly-authenticated caller -- unaffected by - the ownership feature.""" + """/register_tools stays closed to a validly-authenticated delegated + user credential (401): only TES's service-only registration + credential can register tools -- unaffected by the ownership feature.""" _mock_iam_fixed(monkeypatch, EXECUTE_A) resp = client.post("/register_tools", json={"tools": [{"tool_id": "x"}]}, headers=_bearer()) - assert resp.status_code == 501 - assert "REGISTER_TOOLS_AUTHORIZATION_MODEL_UNRESOLVED" in resp.json()["detail"] + assert resp.status_code == 401 # =========================================================================== diff --git a/toolserver/security.py b/toolserver/security.py index a0da53b..33d0aa4 100644 --- a/toolserver/security.py +++ b/toolserver/security.py @@ -42,8 +42,9 @@ from typing import Optional from fastapi import Header -from iam_client import AsyncIAMClient, DelegatedExecutionIdentity +from iam_client import AsyncIAMClient, DelegatedExecutionIdentity, ServiceRegistrationIdentity from iam_client.delegated import require_delegated_execution +from iam_client.registration import require_toolserver_registration # Matches every other IAM_URL consumer in this workspace (omnibioai-tes, # omnibioai-api-gateway, ...) -- already wired into Studio's compose file @@ -100,3 +101,33 @@ async def require_runs_read( behavior as require_workflow_execute, scoped to `runs.read`.""" dependency = require_delegated_execution(get_iam_client(), RUNS_READ) return await dependency(authorization=authorization) + + +# Service identities (Auth OAuth client_ids) allowed to register tools. +# Comma-separated; empty/unset denies every registration (fail closed). In +# production this is exactly TES's own OAuth client_id. +REGISTRATION_CLIENT_IDS_ENV = "TOOLSERVER_REGISTRATION_CLIENT_IDS" + + +def registration_client_allowlist() -> frozenset[str]: + raw = os.environ.get(REGISTRATION_CLIENT_IDS_ENV, "") + return frozenset(part.strip() for part in raw.split(",") if part.strip()) + + +async def require_tes_registration( + authorization: Optional[str] = Header(default=None), +) -> ServiceRegistrationIdentity: + """Protects POST /register_tools. Accepts only Auth's service-only + `toolserver_registration` credential (ToolServer audience, scope + `toolserver.register`, no user principal), validated live through + Auth's registration introspection by omnibioai-iam-client. + + 401: missing/malformed/invalid/expired/revoked/wrong-audience/wrong-type + credential, including every user and delegated-execution token. + 403: a valid registration credential from a service identity that is not + in TOOLSERVER_REGISTRATION_CLIENT_IDS. + + It never satisfies require_workflow_execute/require_runs_read, which only + accept `delegated_execution` credentials.""" + dependency = require_toolserver_registration(get_iam_client(), registration_client_allowlist()) + return await dependency(authorization=authorization) diff --git a/toolserver_app.py b/toolserver_app.py index db44e73..ffb8c4e 100644 --- a/toolserver_app.py +++ b/toolserver_app.py @@ -5,15 +5,16 @@ import time from typing import Any, Dict, List -from fastapi import Depends, FastAPI, HTTPException +from fastapi import Depends, FastAPI from fastapi.responses import JSONResponse -from iam_client import DelegatedExecutionIdentity +from iam_client import DelegatedExecutionIdentity, ServiceRegistrationIdentity from pydantic import BaseModel +from toolserver.adapters.http_tool_executor import make_run, make_validate from toolserver.executor import Executor from toolserver.models import RunCreateRequest, RunRecord, ValidateRequest -from toolserver.registry import ToolRegistry -from toolserver.security import require_runs_read, require_workflow_execute +from toolserver.registry import ToolHandler, ToolRegistry +from toolserver.security import require_runs_read, require_tes_registration, require_workflow_execute from toolserver.store import RunStore from toolserver.tools import load_tools_from_yaml, register_tools @@ -202,34 +203,45 @@ def get_results( return rec.results or {"ok": True, "results": {}} # ---------------- - # Register tools -- REGISTER_TOOLS AUTHORIZATION MODEL UNRESOLVED - # ---------------- - # HIPAA-V2-019: this endpoint lets a caller inject arbitrary HTTP-tool - # execution definitions (registry.register -> handler.run is later - # invoked by /runs) -- a code-execution-adjacent administrative - # capability, not a `workflow.execute`/`runs.read` operation. Auth's - # delegated_execution_service.py::ACCEPTED_PERMISSIONS is exactly - # {"workflow.execute", "runs.read"} today; no administrative/tool- - # registration permission exists in the delegated-execution model, - # and HIPAA-V2-019 Section 9 explicitly reserves this exact case - # ("do not grant workflow.manage merely because ToolServer has a - # registration endpoint") for a separately approved capability that - # does not exist yet. Rather than leave this reachable with no - # permission model, or misuse an existing permission that does not - # actually authorize it, this endpoint is disabled (fails closed) - # until an explicit administrative permission is defined and - # reviewed. This is a deliberate, reported blocker/follow-up, not an - # oversight -- see the HIPAA-V2-019 design document. + # Register tools -- TES service identity only + # ---------------- + # HIPAA-V2-019: registering HTTP-tool definitions is an administrative, + # code-execution-adjacent capability (a registered handler is later run + # by /runs), so it is not a workflow.execute/runs.read operation and is + # never reachable with a user or delegated-execution credential. It + # requires Auth's service-only `toolserver_registration` credential + # (scope toolserver.register, ToolServer audience, no user principal) + # from a service identity listed in TOOLSERVER_REGISTRATION_CLIENT_IDS + # -- in production, exactly TES, which registers its declarative HTTP + # tools once at startup. Only tools with an `http` block are accepted; + # the credential is never logged. @app.post("/register_tools") - def register_tools_endpoint(req: RegisterToolsRequest): - raise HTTPException( - status_code=501, - detail=( - "REGISTER_TOOLS_AUTHORIZATION_MODEL_UNRESOLVED: tool registration has no " - "defined delegated/administrative permission and is disabled pending " - "HIPAA-V2-019 follow-up" - ), + def register_tools_endpoint( + req: RegisterToolsRequest, + identity: ServiceRegistrationIdentity = Depends(require_tes_registration), # noqa: B008 + ): + registered = 0 + skipped = 0 + for tool_def in req.tools: + tool_id = tool_def.get("tool_id") + if not tool_id or not isinstance(tool_def.get("http"), dict): + skipped += 1 + continue + registry.register( + ToolHandler( + tool_id=tool_id, + validate=make_validate(tool_def), + run=make_run(tool_def), + version=tool_def.get("version", "v1"), + features=tool_def.get("features") or {}, + ) + ) + registered += 1 + print( + f"[toolserver] register_tools: service={identity.calling_service} " + f"registered={registered} skipped={skipped}" ) + return {"ok": True, "registered": registered, "skipped": skipped} # ---------------- # Health