Skip to content

Commit bb521e8

Browse files
MohanLakshcrivetimihai
MohanLaksh
and
crivetimihai
authored
fix(auth): eliminate duplicate DB sessions in auth and RBAC middleware (#3886)
* fix: eliminate duplicate DB sessions in auth and RBAC middleware Implements session reuse pattern from PR #3600 and PR #3813 to achieve 1 shared database session per request across all middleware layers. **Changes:** - Auth middleware: Added _get_or_create_session() helper to reuse request.state.db from ObservabilityMiddleware (lines 134, 159, 213) - RBAC middleware: Updated deprecated get_db() to accept optional request parameter and reuse middleware session when available - Transaction control: Delegated all commit/rollback to get_db() per PR #3813 (removed db.commit() from auth middleware) - Added 7 unit tests for auth session reuse patterns - Added 7 unit tests for RBAC get_db() deprecation - Added 6 integration tests for end-to-end session sharing validation **Impact:** - Reduces session creation from 4-6 per request to 1 per request - Prevents connection pool exhaustion under load - Achieves 100% test coverage (435 statements, 0 missing) **Security:** - Transaction isolation maintained (get_db() controls all commits) - Connection invalidation for PgBouncer compatibility - Backwards compatible (existing dependency overrides work) Closes #3622 Signed-off-by: Mohan Lakshmaiah <mohan.economist@gmail.com> * fix: ensure auth logs are committed in hard-deny paths Fixes critical bug where auth failure logs were lost when API requests received hard-deny responses (401/403) that return JSONResponse immediately without reaching get_db(). **Root Cause:** - Auth middleware writes logs to session but delegated commit to get_db() - Hard-deny API responses return JSONResponse immediately (lines 243-247) - Route handler never runs, get_db() never called, logs never committed - Browser requests work fine (continue to route handler at line 236) **Fix:** - Added db.commit() after logging in both success and failure paths - Logs persist immediately, even if request doesn't reach get_db() - For requests that continue to route handler, get_db() commits again (no-op) - SQLAlchemy allows multiple commits - second commit is safe **Test Coverage:** - Added regression test: test_auth_failure_logs_committed_before_hard_deny_api_response - All 29 auth middleware tests pass - Maintained 100% code coverage Signed-off-by: Mohan Lakshmaiah <mohan.economist@gmail.com> * fix(build): use system uv binary consistently in Makefile Replace hardcoded 'uv' commands with $(UV_BIN) variable reference across all targets to ensure consistent resolution of the uv binary path. Export UV_BIN to make it available to recursive make calls. This fixes build failures where make targets tried to execute 'uv' from inside the activated virtual environment, where it doesn't exist. The uv tool is a system-level package manager and should be resolved from the system PATH or ~/.local/bin. Changes: - Use $(UV_BIN) in install, install-dev, install-db, update targets - Use $(UV_BIN) in ensure_pip_package macro (fixes recursive make) - Use $(UV_BIN) in sbom, alembic, pypiserver, maturin targets - Export UV_BIN for visibility in child make processes Benefits: - Fixes "No such file or directory" errors for uv - Makes Makefile more robust across different uv installations - Maintains existing fallback logic (PATH or ~/.local/bin/uv) - No breaking changes for existing setups Signed-off-by: Mohan Lakshmaiah <mohan.economist@gmail.com> * fix(auth): prevent stale session reference and ensure log persistence Fix two bugs in auth middleware session handling: 1. _get_or_create_session() stored newly created sessions in request.state.db but then closed them after logging, leaving a stale closed-session reference for downstream get_db() to find. Remove the request.state.db assignment for owned sessions so route handlers create their own sessions via get_db(). 2. Generic Exception path (non-HTTP auth failures) did not commit security logs before closing the owned session, silently losing log entries. Add db.commit() consistent with the success and hard-deny paths. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(auth): rollback shared session on logging failure and rewrite integration tests Address two findings from code review: 1. When security logging raises an exception (commit failure, connection error), the shared session from ObservabilityMiddleware is left in PendingRollbackError state. Downstream call_next()/get_db() then inherits a broken session. Add db.rollback() (with invalidate() fallback) in all three exception handlers — matching the pattern used by ObservabilityMiddleware and main.py:get_db(). 2. Rewrite integration tests: the originals targeted nonexistent routes (/api/v1/servers, /admin/llm/providers) and had assertions too weak to catch regressions (session_count <= 1 passes when 0 sessions are created). New tests directly exercise _get_or_create_session(), rbac.get_db() reuse, shared-session rollback, and the full middleware stack via /health. Test coverage: 100% (460 statements, 0 missing) across both auth middleware and rbac modules. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * chore: update secrets baseline after rebase Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * chore: exclude .npmrc from sdist to fix check-manifest Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mohan Lakshmaiah <mohan.economist@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
1 parent 2b1d01a commit bb521e8

8 files changed

Lines changed: 1100 additions & 54 deletions

File tree

‎.secrets.baseline‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"files": "package-lock.json|Cargo.lock|^.secrets.baseline$|scripts/sign_image.sh|scripts/zap|sonar-project.properties|^/Users/brian/dev/github.ibm.com/contextforge-org/sps-pipeline-config/.secrets.baseline$|^./.secrets.baseline$",
44
"lines": null
55
},
6-
"generated_at": "2026-03-30T23:20:07Z",
6+
"generated_at": "2026-03-31T10:20:49Z",
77
"plugins_used": [
88
{
99
"name": "AWSKeyDetector"
@@ -344,71 +344,71 @@
344344
"hashed_secret": "844c398e469ef3fb919da3778944365ab2175fb7",
345345
"is_secret": false,
346346
"is_verified": false,
347-
"line_number": 367,
347+
"line_number": 368,
348348
"type": "Secret Keyword",
349349
"verified_result": null
350350
},
351351
{
352352
"hashed_secret": "319037749ce37e577db0b3628c7f90e333544391",
353353
"is_secret": false,
354354
"is_verified": false,
355-
"line_number": 792,
355+
"line_number": 793,
356356
"type": "Secret Keyword",
357357
"verified_result": null
358358
},
359359
{
360360
"hashed_secret": "6ae2832e494d1098e8901fe156083e39399a24f1",
361361
"is_secret": false,
362362
"is_verified": false,
363-
"line_number": 794,
363+
"line_number": 795,
364364
"type": "Secret Keyword",
365365
"verified_result": null
366366
},
367367
{
368368
"hashed_secret": "9d989e8d27dc9e0ec3389fc855f142c3d40f0c50",
369369
"is_secret": false,
370370
"is_verified": false,
371-
"line_number": 1497,
371+
"line_number": 1498,
372372
"type": "Secret Keyword",
373373
"verified_result": null
374374
},
375375
{
376376
"hashed_secret": "d3ac7a4ef1a838b4134f2f6e7f3c0d249d74b674",
377377
"is_secret": false,
378378
"is_verified": false,
379-
"line_number": 5836,
379+
"line_number": 5837,
380380
"type": "Secret Keyword",
381381
"verified_result": null
382382
},
383383
{
384384
"hashed_secret": "5932862bcd24dd27d0dc0407ec94fe9d6ea24aeb",
385385
"is_secret": false,
386386
"is_verified": false,
387-
"line_number": 6333,
387+
"line_number": 6334,
388388
"type": "Secret Keyword",
389389
"verified_result": null
390390
},
391391
{
392392
"hashed_secret": "c77c805e32f173e4321ee9187de9c29cb3804513",
393393
"is_secret": false,
394394
"is_verified": false,
395-
"line_number": 6345,
395+
"line_number": 6346,
396396
"type": "Secret Keyword",
397397
"verified_result": null
398398
},
399399
{
400400
"hashed_secret": "8fe3df8a68ddd0d4ab2214186cbb8e38ccd0e06a",
401401
"is_secret": false,
402402
"is_verified": false,
403-
"line_number": 6417,
403+
"line_number": 6418,
404404
"type": "Secret Keyword",
405405
"verified_result": null
406406
},
407407
{
408408
"hashed_secret": "93ac8946882128457cd9e283b30ca851945e6690",
409409
"is_secret": false,
410410
"is_verified": false,
411-
"line_number": 7520,
411+
"line_number": 7521,
412412
"type": "Secret Keyword",
413413
"verified_result": null
414414
}
@@ -8316,15 +8316,15 @@
83168316
"hashed_secret": "ab73a3eaca01a7059dcdff6f95556ec7fd83de96",
83178317
"is_secret": false,
83188318
"is_verified": false,
8319-
"line_number": 1388,
8319+
"line_number": 1389,
83208320
"type": "Secret Keyword",
83218321
"verified_result": null
83228322
},
83238323
{
83248324
"hashed_secret": "92dd4a2de441b63e6ac51fdb81a05a416dffd182",
83258325
"is_secret": false,
83268326
"is_verified": false,
8327-
"line_number": 1475,
8327+
"line_number": 1476,
83288328
"type": "Secret Keyword",
83298329
"verified_result": null
83308330
}

‎MANIFEST.in‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ exclude .hadolint.yaml
192192
exclude .ruff.toml
193193
exclude .pre-commit-config.yaml
194194
exclude .secrets.baseline
195+
exclude .npmrc
195196

196197
# Backup and temporary files
197198
global-exclude *~

‎Makefile‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,8 @@ endef
146146
define ensure_pip_package
147147
@test -d "$(VENV_DIR)" || $(MAKE) venv
148148
@/bin/bash -c "source $(VENV_DIR)/bin/activate && \
149-
uv pip show $(1) >/dev/null 2>&1 || \
150-
uv pip install -q $(1)"
149+
$(UV_BIN) pip show $(1) >/dev/null 2>&1 || \
150+
$(UV_BIN) pip install -q $(1)"
151151
endef
152152

153153
# =============================================================================
@@ -178,6 +178,7 @@ uv:
178178

179179
# UV_BIN: prefer uv in PATH, fallback to ~/.local/bin/uv
180180
UV_BIN := $(shell type -p uv 2>/dev/null || echo "$(HOME)/.local/bin/uv")
181+
export UV_BIN
181182

182183
.PHONY: venv
183184
venv: uv
@@ -192,15 +193,15 @@ activate:
192193

193194
.PHONY: install
194195
install: venv
195-
@/bin/bash -c "source $(VENV_DIR)/bin/activate && uv pip install ."
196+
@/bin/bash -c "source $(VENV_DIR)/bin/activate && $(UV_BIN) pip install ."
196197

197198
.PHONY: install-db
198199
install-db: venv
199-
@/bin/bash -c "source $(VENV_DIR)/bin/activate && uv pip install .[redis,postgres]"
200+
@/bin/bash -c "source $(VENV_DIR)/bin/activate && $(UV_BIN) pip install .[redis,postgres]"
200201

201202
.PHONY: install-dev
202203
install-dev: venv
203-
@/bin/bash -c "source $(VENV_DIR)/bin/activate && uv pip install --group dev ."
204+
@/bin/bash -c "source $(VENV_DIR)/bin/activate && $(UV_BIN) pip install --group dev ."
204205
@if [ "$(ENABLE_RUST_BUILD)" = "1" ]; then \
205206
echo "🦀 Building Rust plugins..."; \
206207
$(MAKE) rust-dev || echo "⚠️ Rust plugins not available (optional)"; \
@@ -211,7 +212,7 @@ install-dev: venv
211212
.PHONY: update
212213
update:
213214
@echo "⬆️ Updating installed dependencies..."
214-
@/bin/bash -c "source $(VENV_DIR)/bin/activate && uv pip install -U --group dev ."
215+
@/bin/bash -c "source $(VENV_DIR)/bin/activate && $(UV_BIN) pip install -U --group dev ."
215216

216217
# help: check-env - Verify all required env vars in .env are present
217218
.PHONY: check-env check-env-dev
@@ -2987,7 +2988,7 @@ mutmut-clean:
29872988
.PHONY: ensure-pip-licenses pip-licenses license-check scc scc-report
29882989

29892990
ensure-pip-licenses:
2990-
@/bin/bash -c "source $(VENV_DIR)/bin/activate && uv pip install -q pip-licenses"
2991+
@/bin/bash -c "source $(VENV_DIR)/bin/activate && $(UV_BIN) pip install -q pip-licenses"
29912992

29922993
pip-licenses: ensure-pip-licenses
29932994
@mkdir -p $(dir $(LICENSES_MD))
@@ -3865,9 +3866,9 @@ tox: ## 🧪 Multi-Python tox matrix (uv)
38653866
sbom: uv ## 🛡️ Generate SBOM & security report
38663867
@echo "🛡️ Generating SBOM & security report..."
38673868
@rm -Rf "$(VENV_DIR).sbom"
3868-
@uv venv "$(VENV_DIR).sbom"
3869-
@/bin/bash -c "source $(VENV_DIR).sbom/bin/activate && uv pip install .[dev]"
3870-
@/bin/bash -c "source $(VENV_DIR)/bin/activate && uv pip install -q cyclonedx-bom sbom2doc"
3869+
@$(UV_BIN) venv "$(VENV_DIR).sbom"
3870+
@/bin/bash -c "source $(VENV_DIR).sbom/bin/activate && $(UV_BIN) pip install .[dev]"
3871+
@/bin/bash -c "source $(VENV_DIR)/bin/activate && $(UV_BIN) pip install -q cyclonedx-bom sbom2doc"
38713872
@echo "🔍 Generating SBOM from environment..."
38723873
@/bin/bash -c "source $(VENV_DIR)/bin/activate && \
38733874
python3 -m cyclonedx_py environment \
@@ -6317,7 +6318,7 @@ LOCAL_PYPI_AUTH := $(LOCAL_PYPI_DIR)/.htpasswd
63176318

63186319
local-pypi-install:
63196320
@echo "📦 Installing pypiserver..."
6320-
@/bin/bash -c "source $(VENV_DIR)/bin/activate && uv pip install 'pypiserver>=2.3.0' passlib"
6321+
@/bin/bash -c "source $(VENV_DIR)/bin/activate && $(UV_BIN) pip install 'pypiserver>=2.3.0' passlib"
63216322
@mkdir -p $(LOCAL_PYPI_DIR)
63226323

63236324
local-pypi-start: local-pypi-install local-pypi-stop
@@ -6409,7 +6410,7 @@ local-pypi-test:
64096410
local-pypi-clean: clean dist local-pypi-start-auth local-pypi-upload-auth local-pypi-test
64106411
@echo "🎉 Full local PyPI cycle complete!"
64116412
@echo "📊 Package info:"
6412-
@/bin/bash -c "source $(VENV_DIR)/bin/activate && uv pip show $(PROJECT_NAME)"
6413+
@/bin/bash -c "source $(VENV_DIR)/bin/activate && $(UV_BIN) pip show $(PROJECT_NAME)"
64136414

64146415
# Convenience target to restart server
64156416
local-pypi-restart: local-pypi-stop local-pypi-start
@@ -6587,7 +6588,7 @@ devpi-test:
65876588
devpi-clean: clean dist devpi-upload devpi-test
65886589
@echo "🎉 Full devpi cycle complete!"
65896590
@echo "📊 Package info:"
6590-
@/bin/bash -c "source $(VENV_DIR)/bin/activate && uv pip show mcp-contextforge-gateway"
6591+
@/bin/bash -c "source $(VENV_DIR)/bin/activate && $(UV_BIN) pip show mcp-contextforge-gateway"
65916592

65926593
.PHONY: devpi-status
65936594
devpi-status:
@@ -6756,7 +6757,7 @@ shell-linters-install: ## 🔧 Install shellcheck, shfmt, bashate
67566757
if ! $(VENV_DIR)/bin/bashate -h >/dev/null 2>&1 ; then \
67576758
echo "🛠 Installing bashate (into venv)..." ; \
67586759
test -d "$(VENV_DIR)" || $(MAKE) venv ; \
6759-
/bin/bash -c "source $(VENV_DIR)/bin/activate && uv pip install -q bashate" ; \
6760+
/bin/bash -c "source $(VENV_DIR)/bin/activate && $(UV_BIN) pip install -q bashate" ; \
67606761
fi
67616762
@echo "✅ Shell linters ready."
67626763

@@ -6823,7 +6824,7 @@ ALEMBIC_CONFIG = mcpgateway/alembic.ini
68236824

68246825
alembic-install:
68256826
@echo "➜ Installing Alembic ..."
6826-
@/bin/bash -c "source $(VENV_DIR)/bin/activate && uv pip install -q alembic sqlalchemy"
6827+
@/bin/bash -c "source $(VENV_DIR)/bin/activate && $(UV_BIN) pip install -q alembic sqlalchemy"
68276828

68286829
.PHONY: db-init
68296830
db-init: ## Initialize alembic migrations
@@ -8171,7 +8172,7 @@ rust-ensure-deps: ## Ensure Rust toolchain, maturin, and a
81718172
@if ! command -v maturin > /dev/null 2>&1; then \
81728173
if [ -f "$(VENV_DIR)/bin/activate" ]; then \
81738174
echo "📦 Installing maturin into venv..."; \
8174-
/bin/bash -c "source $(VENV_DIR)/bin/activate && uv pip install maturin"; \
8175+
/bin/bash -c "source $(VENV_DIR)/bin/activate && $(UV_BIN) pip install maturin"; \
81758176
elif command -v pip > /dev/null 2>&1; then \
81768177
echo "📦 Installing maturin globally (venv not found)..."; \
81778178
pip install maturin; \

0 commit comments

Comments
 (0)