From 4c47827d30d70b49b0d1f98a43864e117d8161d1 Mon Sep 17 00:00:00 2001 From: Manish Kumar Date: Sat, 15 Aug 2026 02:55:17 -0500 Subject: [PATCH] security: activate model-registry auth wiring in release compose configs CRITICAL: both production/release compose files (docker-compose.release.yml and docker-compose-release.yml) shipped model-registry with no AUTH_ENABLED/JWT_SECRET/IAM_URL/AUDIT_URL at all. omnibioai-model-registry's config.py defaults AUTH_ENABLED to false when unset, so every route's auth dependency silently returned a synthetic unauthenticated identity -- no token required, no permission check -- while the service was published on ${HOST_IP:-0.0.0.0}:8095 by default. The real Phase 2A-2E organization- ownership enforcement in that repo never ran in a release deployment. Reproduced live: an unauthenticated POST /v1/register (zero Authorization header, AUTH_ENABLED unset) returned 200 and registered a model. Fix mirrors the exact wiring docker-compose.yml (dev) already had for this same service, and the established release-file convention already used by control-center/dev-hub/security-audit (AUTH_ENABLED as a hardcoded "true" literal, JWT_SECRET as ${AUTH_SECRET_KEY:?...} required-guard, IAM_URL/ AUDIT_URL as fixed internal-network URLs) -- no new pattern invented. Changed: - docker-compose.release.yml, docker-compose-release.yml: add IAM_URL/AUDIT_URL/JWT_SECRET/AUTH_ENABLED to model-registry's environment block. Verified via `docker compose config` that (a) the merged config now carries all four correctly and (b) omitting AUTH_SECRET_KEY makes compose refuse to start with "AUTH_SECRET_KEY must be set", naming model-registry's own JWT_SECRET line -- confirms this fails closed rather than silently reverting to open mode. - tests/test_compose_release_config.py: add model-registry to EXPECTED_SECRET_WIRING (JWT_SECRET-sourcing regression coverage, same mechanism already protecting control-center/security-audit) and a new SERVICES_REQUIRING_AUTH_ENABLED set + test asserting AUTH_ENABLED is the literal "true" in both release files. Scoped to model-registry only -- running the new AUTH_ENABLED test against dev-hub too surfaced an independent, pre-existing gap (docker-compose-release.yml's dev-hub block is missing JWT_SECRET/AUTH_ENABLED entirely) which is out of scope for this change and is flagged separately, not fixed here. - SECURITY-COMPOSE-HARDENING.md: corrects the document's own prior claim that model-registry "does enforce IAM authorization" on its default 0.0.0.0 binding -- that was false until this fix; documents the finding and marks the equivalent claim for workbench/tes/auth-service/rag as unverified pending the same audit. release/linux-arm64-unpacked/ (gitignored local Electron build output, not tracked in this repo) still contains a stale pre-fix compose copy on this machine; it is not the repo's source of truth and will pick up this fix on the next `npm run build:*`. Not hand-edited. No changes to omnibioai-model-registry itself -- its Phase 2A-2E organization-ownership enforcement was already correct; this was purely a release deployment-configuration gap. Co-Authored-By: Claude Sonnet 5 --- SECURITY-COMPOSE-HARDENING.md | 33 +++++++++++++---- docker-compose-release.yml | 7 ++++ docker-compose.release.yml | 16 +++++++++ tests/test_compose_release_config.py | 54 ++++++++++++++++++++++++++++ 4 files changed, 104 insertions(+), 6 deletions(-) diff --git a/SECURITY-COMPOSE-HARDENING.md b/SECURITY-COMPOSE-HARDENING.md index 3312ff0..35156ae 100644 --- a/SECURITY-COMPOSE-HARDENING.md +++ b/SECURITY-COMPOSE-HARDENING.md @@ -221,12 +221,33 @@ Found during this work; **not fixed in this change** and not claimed to be: 1. **Backend services are published on `${HOST_IP:-0.0.0.0}`.** The default binds all interfaces for `workbench`, `tes`, `auth-service`, `rag`, - `model-registry`, and others. These do enforce IAM authorization, so this - is materially different from an unauthenticated datastore — but the - gateway-first design would be better served by binding them to loopback - and routing through `nginx-router`, as `lims` and `nginx-router` itself - already do. Larger change (breaks the direct-LAN-access path `HOST_IP` - exists for); needs a product/deployment decision, not made here. + `model-registry`, and others. Where they actually enforce IAM + authorization, this is materially different from an unauthenticated + datastore — but the gateway-first design would be better served by + binding them to loopback and routing through `nginx-router`, as `lims` + and `nginx-router` itself already do. Larger change (breaks the + direct-LAN-access path `HOST_IP` exists for); needs a product/deployment + decision, not made here. + + **`model-registry` follow-up (2026-08-15, separate branch):** this + document's "these do enforce IAM authorization" claim above was **false** + for `model-registry` specifically, discovered during a HIPAA org-isolation + re-audit: both release compose files (`docker-compose.release.yml` and + `docker-compose-release.yml`) omitted `AUTH_ENABLED`/`JWT_SECRET`/ + `IAM_URL`/`AUDIT_URL` from the `model-registry` block entirely. + `omnibioai-model-registry/config.py` defaults `AUTH_ENABLED` to `false` + when unset, so in the released configuration every route's auth + dependency short-circuited to a synthetic unauthenticated identity — + the real Phase 2A-2E organization-ownership enforcement in that repo + never ran. Fixed by adding all four vars to both release files + (`tests/test_compose_release_config.py`'s + `test_service_has_auth_enabled_set_true_in_release` / + `EXPECTED_SECRET_WIRING["model-registry"]` now pin it going forward). + This was a live, unauthenticated-write exposure on `0.0.0.0:8095` by + default, not merely a documentation gap. The claim above has **not** + been independently re-verified for `workbench`/`tes`/`auth-service`/ + `rag` following this finding — treat it as unconfirmed for those until + they are audited the same way. **`control-center` follow-up (2026-08-13, separate branch):** this document previously claimed `control-center` was *already* loopback-only diff --git a/docker-compose-release.yml b/docker-compose-release.yml index 2928cd4..de108dc 100644 --- a/docker-compose-release.yml +++ b/docker-compose-release.yml @@ -418,6 +418,13 @@ services: DB_PASSWORD: ${MYSQL_ROOT_PASSWORD:?MYSQL_ROOT_PASSWORD must be set} DB_NAME: model_registry OMNIBIOAI_MODEL_REGISTRY_ROOT: /registry + # HIPAA remediation: kept in parity with docker-compose.release.yml + # (dot) -- see that file's identical comment on this same block for + # the full rationale. + IAM_URL: http://auth-service:8001 + AUDIT_URL: http://security-audit:8004 + JWT_SECRET: ${AUTH_SECRET_KEY:?AUTH_SECRET_KEY must be set} + AUTH_ENABLED: "true" volumes: - ${DATA_DIR}/model_registry:/registry depends_on: diff --git a/docker-compose.release.yml b/docker-compose.release.yml index 4c7a417..b9a0371 100644 --- a/docker-compose.release.yml +++ b/docker-compose.release.yml @@ -425,6 +425,22 @@ services: DB_PASSWORD: ${MYSQL_ROOT_PASSWORD:?MYSQL_ROOT_PASSWORD must be set} DB_NAME: model_registry OMNIBIOAI_MODEL_REGISTRY_ROOT: /registry + # HIPAA remediation: this service's own Phase 2A-2E organization- + # ownership enforcement (omnibioai-model-registry) is real and + # already merged, but it only ever runs against a verified + # UserContext -- without these four vars, config.py's AUTH_ENABLED + # defaults to false, every route's auth dependency short-circuits to + # a synthetic "system" identity with no token required at all, and + # check_model_ownership() collapses to open mode. Same wiring + # rag/tes/toolserver (IAM_URL) and control-center/dev-hub + # (JWT_SECRET/AUTH_ENABLED) already use in this same file -- see + # docker-compose.yml's (dev) identical model-registry entry, which + # already had all four and was the only reason this gap wasn't + # total in every deployment mode. + IAM_URL: http://auth-service:8001 + AUDIT_URL: http://security-audit:8004 + JWT_SECRET: ${AUTH_SECRET_KEY:?AUTH_SECRET_KEY must be set} + AUTH_ENABLED: "true" volumes: - ${DATA_DIR}/model_registry:/registry depends_on: diff --git a/tests/test_compose_release_config.py b/tests/test_compose_release_config.py index 3286d31..2d8740e 100644 --- a/tests/test_compose_release_config.py +++ b/tests/test_compose_release_config.py @@ -36,8 +36,38 @@ "api-gateway": "JWT_SECRET", "control-center": "JWT_SECRET", "security-audit": "JWT_SECRET", + # HIPAA remediation (model-registry release auth wiring): this service + # was never in this dict despite being an AsyncIAMClient/JWT consumer + # since its own Phase 2A-2E org-ownership series -- both release + # compose files shipped it with no JWT_SECRET/IAM_URL/AUDIT_URL/ + # AUTH_ENABLED at all, so config.py's AUTH_ENABLED defaulted to false + # and every route ran unauthenticated. See + # SERVICES_REQUIRING_AUTH_ENABLED below for the AUTH_ENABLED half of + # that same gap, which this dict alone doesn't cover. + "model-registry": "JWT_SECRET", } +# Services whose auth dependency has an AUTH_ENABLED on/off switch +# (unlike the services above, which always verify JWTs) -- omitting this +# var entirely defaults to false in each service's own config.py/settings, +# silently running with no authentication at all rather than failing +# loudly. Every service in this set must have the literal string "true" +# (never an ${AUTH_ENABLED:-...} expression, and never omitted) in both +# release compose files -- see docker-compose.yml (dev)'s identical +# entries for the same convention this mirrors. +# +# Deliberately model-registry ONLY, not also dev-hub: this test was added +# while investigating the model-registry gap, and running it against +# dev-hub too immediately caught a second, independent instance of the +# same bug class (docker-compose-release.yml, the dash variant, is +# missing both JWT_SECRET and AUTH_ENABLED for dev-hub -- present in +# docker-compose.release.yml, the dot variant, only). That's a real, +# separate finding, out of scope for this remediation (model-registry +# only) per its own instructions -- flagged for a dedicated follow-up +# rather than silently fixed here. Add "dev-hub" to this set once that +# follow-up lands. +SERVICES_REQUIRING_AUTH_ENABLED = {"model-registry"} + @pytest.fixture(scope="module", params=COMPOSE_PATHS, ids=lambda p: p.name) def compose_config(request): @@ -73,3 +103,27 @@ def test_all_consumers_share_the_same_secret(compose_config): distinct = set(exprs.values()) assert len(distinct) == 1, f"secret expressions diverge: {exprs}" + + +@pytest.mark.parametrize("service", sorted(SERVICES_REQUIRING_AUTH_ENABLED)) +def test_service_has_auth_enabled_set_true_in_release(compose_config, service): + """Catches a repeat of the model-registry gap this test was added + for: AUTH_ENABLED silently omitted from a release compose file, so + the service's own config module defaults it to false and every route + runs with no authentication at all -- not merely a weaker check, a + missing one. Must be the literal string "true", matching every + existing usage of this var in docker-compose.yml (dev) -- never an + ${AUTH_ENABLED:-...} expression (that would let an unset host env var + silently reintroduce the exact same open-mode fallback this test + exists to prevent).""" + env = compose_config["services"][service]["environment"] + assert "AUTH_ENABLED" in env, ( + f"{service} must set AUTH_ENABLED -- without it, this service's " + f"auth dependency defaults to disabled and every route runs " + f"unauthenticated" + ) + assert env["AUTH_ENABLED"] == "true", ( + f"{service}'s AUTH_ENABLED must be the literal string \"true\", " + f"not {env['AUTH_ENABLED']!r} -- an env-var expression would let " + f"an unset host variable silently fall back to disabled again" + )