security(studio): close release-compose datastore exposure and credential defaults - #44
Merged
Merged
Conversation
The release compose files -- docker-compose.release.yml is the one
electron-builder bundles into every packaged installer -- published both
datastores to the host with no interface binding:
mysql: ports: ["3306:3306"] -> 0.0.0.0:3306
redis: ports: ["6380:6379"] -> 0.0.0.0:6380
A bare "3306:3306" binds 0.0.0.0, not loopback, so on any host whose
firewall didn't independently block it, both were reachable from the
network. MySQL's root password simultaneously defaulted to the literal
"omnibioai", committed in this repo -- so the reachable database had a
publicly known credential -- and Redis has no auth at all.
This is an infrastructure-level bypass of every application-layer
control the platform has. IAM authentication, RBAC, organization
isolation, audit logging, and the gateway's zero-trust middleware all
operate above the datastore. A client speaking the MySQL wire protocol
to :3306 presents no JWT, is subject to no organization_id scoping, is
filtered by no queryset, and generates no audit event -- it reads every
tenant's rows regardless of how thoroughly the application enforces
isolation above it.
DEPLOYMENT.md's own port table already documented the intent ("3306 |
MySQL | Internal only -- not exposed externally in prod"). The compose
file just never implemented it.
Both datastores stay fully reachable inside the compose network by
service name (mysql:3306, redis:6379), which is how every consumer
already addressed them -- no connection string changes. api-gateway
stays published: it is the intended entry point.
Local access is preserved through an explicit opt-in overlay rather
than by weakening the default:
docker compose -f docker-compose.release.yml \
-f docker-compose.release.dev-ports.yml up -d
The overlay binds 127.0.0.1 by default (not 0.0.0.0), is not bundled by
electron-builder, and is referenced by neither production startup path,
so it cannot become the default by accident -- it takes a second -f.
docker-compose.yml (the local dev stack, loaded only when the Electron
app is unpackaged) deliberately still publishes ports. That is its
purpose; this finding is about the release configuration.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ence
Every credential in the release compose files fell back to a public
literal committed to this repository:
MYSQL_ROOT_PASSWORD -> omnibioai
AUTH_SECRET_KEY -> change-me-in-production
LICENSE_SECRET -> omnibioai-secret-change-in-production
GF_ADMIN_PASSWORD -> omnibioai
LIMSX_DJANGO_SECRET_KEY -> omnibioai-studio-secret
JUPYTER_TOKEN -> omnibioai
RSTUDIO_PASSWORD -> omnibioai
VSCODE_PASSWORD -> omnibioai
A deployment that simply didn't set them came up fully functional with
guessable credentials rather than failing. All now use compose's
${VAR:?message} required form, so a missing value aborts startup.
No replacement secrets are hard-coded -- values come from the
environment only.
Two real bugs surfaced while tracing where these are provisioned:
1. LIMSX_DJANGO_SECRET_KEY, JUPYTER_TOKEN, RSTUDIO_PASSWORD, and
VSCODE_PASSWORD were never in the app's generation list at all, and
writeEnvFile() hardcoded the first one's weak literal as a fallback.
Every Studio installation everywhere therefore shared the same four
values -- including the key signing LIMS's session cookies. They are
generated per-installation now (32 random bytes each), and the
remaining weak writeEnvFile() fallbacks are gone so a missing value
fails closed via the compose guard instead of silently reappearing.
2. scripts/start.sh defaulted MYSQL_ROOT_PASSWORD to "omnibioai" in its
"no .env found" branch, which would have re-supplied the weak value
and defeated the new guard entirely. Removed.
ADMIN_KEY is removed rather than made required: PR11 replaced the
license server's static shared-secret check with IAM authorization and
backend/license_server.py has had zero references to it since, but both
release files still passed a dead ${ADMIN_KEY:-admin-secret}.
generateSecrets/parseEnvFile move to electron/secrets.js -- same
behavior, but with no `electron` import so the logic is unit-testable
outside an Electron runtime. Settings' credential panel surfaces the
three newly-generated values so they remain retrievable by the user.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ements
Static tests (no daemon, no live services) over both release compose
files, matching the existing test_compose_release_config.py approach:
- mysql/redis publish no ports in the release configuration
- both still exist and consumers still address them by internal
service name, so the fix can't be "achieved" by breaking internal
connectivity
- api-gateway stays published -- guards against over-applying the
exposure fix to the one service meant to be reachable
- no ${VAR:-weak} fallback survives for any required secret, and each
uses the ${VAR:?} fail-closed form
- the dev overlay republishes both, defaults to loopback, is not
bundled by electron-builder, and is referenced by neither
production startup path
- docker-compose.yml still publishes for local dev, pinning the
dev/release distinction so the two files' roles stay legible
Node built-in test runner (no new dependency) covers electron/secrets.js:
generation into a fresh .env, rotation of known-weak literals,
preservation of already-real secrets across launches, non-clobbering of
unrelated keys, and distinctness both per-key and per-install. Asserts
on properties only -- never prints a generated value.
One test caught a real remaining issue while being written: the dead
${ADMIN_KEY:-admin-secret} still present in both release files, fixed in
the preceding commit.
CI now runs both suites, validates all three compose files (release,
dash-release, and the dev overlay merged on top), and asserts the
negative case -- that both release files *refuse* to validate when
required secrets are absent, so the fail-closed guard can't silently
regress.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
New SECURITY-COMPOSE-HARDENING.md covering the original exposure and
why it mattered (every application-layer control operates above the
datastore, so direct :3306 access bypasses all of it), the production
network boundary, the development-only exception and why it can't
become the default, gateway-first expectations, and credential
requirements.
Distinguishes fixed findings from accepted internal communication.
Internal Docker-network service-to-service calls are retained in full
and listed explicitly as intentional -- including the ones deliberately
NOT routed through the gateway:
- TES_BASE_URL stays pointed at TES directly; other not-yet-migrated
readers in the same image attach no bearer token and would silently
401 through the gateway's AuthMiddleware (tracked under #209)
- IAM_URL/POLICY_URL/AUDIT_URL and the gateway's own upstream *_URL
values are internal control-plane calls, not client-facing traffic;
routing the gateway's upstreams through itself would be circular
Records four remaining gaps found but deliberately not addressed here,
so they are visible rather than implied fixed: backend services still
bind ${HOST_IP:-0.0.0.0} (they do enforce IAM, unlike the datastores,
but loopback + nginx-router would suit the design better), pre-existing
JWT_SECRET drift between the two release files, weak defaults remaining
on non-credential vars like LIMS_PASSWORD/NEO4J_PASSWORD, and the fact
that ${VAR:?} proves a value was supplied but not that it is strong.
DEPLOYMENT.md's port table now matches reality rather than stating an
intent the compose file didn't implement. .env.example and CONTRIBUTING
flag which variables are required and the rules for adding new ones.
This PR addresses one HIPAA audit finding. It does not complete the
audit.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two factual corrections found during PR #44 validation, no code or compose behavior changed: - SECURITY-COMPOSE-HARDENING.md §7 gap #2 claimed docker-compose-release.yml was missing security-audit's JWT_SECRET wiring. Verified against origin/main and both current release files: JWT_SECRET is identical in both. The actual pre-existing gap (present on origin/main before this PR, untouched by it) is that docker-compose-release.yml's security-audit block lacks AUDIT_DATABASE_URL and the depends_on: mysql: condition: service_healthy entry that docker-compose.release.yml has. - README.md's Services table described MySQL/Redis as always host-mapped (:3306, "mapped :6380 on host"), which was never updated for the release default this PR closed. Now distinguishes production/release (internal only, not published) from development (still published directly, or via the explicit loopback-bound docker-compose.release.dev-ports.yml overlay against the release stack). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Aug 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Finding
HIPAA audit, High severity: the Studio release compose configuration published MySQL and Redis directly on
0.0.0.0, provisioned weak/default credentials silently, and omitted the gateway wiring the dev stack had.docker-compose.release.ymlis the fileelectron-builderbundles into every packaged installer.Why this was High
A bare
"3306:3306"binds0.0.0.0, not loopback. Combined withMYSQL_ROOT_PASSWORDdefaulting to the literalomnibioai(committed in this repo), the multi-tenant database was network-reachable at a publicly known credential. Redis (0.0.0.0:6380) has no auth at all.Every application-layer control operates above the datastore. A client speaking the MySQL wire protocol to
:3306presents no JWT, is subject to noorganization_idscoping, is filtered by no queryset, and generates no audit event — it reads every tenant's rows regardless of how thoroughly isolation is enforced above it. This is an infrastructure-level bypass of the tenant-isolation work merged acrossomnibioai-auth,omnibioai-lims,omnibioai-model-registry, and others.DEPLOYMENT.mdalready documented the intent — "3306 | MySQL | Internal only — not exposed externally in prod". The compose file never implemented it.Changes
Network boundary — mysql/redis no longer published in either release file. Both stay reachable inside the compose network by service name; no connection string changed.
api-gatewaystays published (it is the intended entry point), asserted by test so the fix can't be over-applied.Credentials — 8 secrets converted from
${VAR:-weak-literal}to fail-closed${VAR:?}. No replacement secrets are hard-coded; values come from the environment only.Gateway —
celery-workerhad noGATEWAY_URLin either release file thoughdocker-compose.ymlgained it with #196, so the packaged build never received that fix — only the dev stack did.Dev access preserved via explicit opt-in overlay, not by weakening the default:
Binds
127.0.0.1by default, not bundled by electron-builder, referenced by neither production startup path — all asserted by test. It takes a second-fflag, so it can't become the default by accident.docker-compose.yml(local dev stack) is deliberately unchanged and still publishes ports.Bugs found while tracing provisioning
LIMSX_DJANGO_SECRET_KEY,JUPYTER_TOKEN,RSTUDIO_PASSWORD,VSCODE_PASSWORDwere never generated at all — every Studio installation everywhere shared the same four hardcoded values, including the key signing LIMS's session cookies. Now generated per-install.scripts/start.shre-supplied the weakMYSQL_ROOT_PASSWORDin its no-.envbranch, which would have defeated the new guard entirely.${ADMIN_KEY:-admin-secret}in both release files — PR11 replaced that check with IAM authorization andlicense_server.pyhas zero references since. Caught by one of the new tests. Removed rather than made required.Tests
tests/test_compose_network_exposure.py— 58 new static assertions over both release filestests/test_secret_generation.js— 8 tests via Node's built-in runner (no new dependency); asserts on properties only, never prints a secretResults: 89 static tests pass (31 before). Full suite: 51 failed / 22 errors on both this branch and clean
main— identical, all integration tests requiring live services. Zero regressions. UI builds clean.Verified against rendered
docker compose configoutput, not just source text: mysql/redisports=Nonein both release files;127.0.0.1:3306/127.0.0.1:6380with the overlay; 6 services still onDB_HOST=mysql, 8 onredis://redis:, zero host-routed datastore refs, healthchecks intact.Explicitly not fixed
Documented in
SECURITY-COMPOSE-HARDENING.mdso they're visible rather than implied fixed:${HOST_IP:-0.0.0.0}— they do enforce IAM (materially different from an unauthenticated datastore), but loopback +nginx-routerwould better suit the designJWT_SECRETdrift between the two release filesLIMS_PASSWORD,NEO4J_PASSWORD)${VAR:?}proves a value was supplied, not that it's strong or uniqueAlso left deliberately unchanged, with reasoning recorded:
TES_BASE_URLstays pointed at TES directly (other readers in the same image carry no bearer token and would silently 401 through the gateway — tracked under #209), and internal control-plane URLs (IAM_URL,POLICY_URL,AUDIT_URL, gateway upstreams) are not client-facing traffic.Scope: this addresses one HIPAA audit finding. It does not complete the audit. No Workflow Bundles or TES Docker-socket work is touched.
🤖 Generated with Claude Code