Skip to content

Commit c2f4e87

Browse files
authored
Merge branch 'main' into user/luca/rust-fast-time-sse
2 parents 0760b2f + b6a4177 commit c2f4e87

23 files changed

Lines changed: 1521 additions & 57 deletions

.secrets.baseline

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -240,31 +240,31 @@
240240
"hashed_secret": "fa9beb99e4029ad5a6615399e7bbae21356086b3",
241241
"is_secret": false,
242242
"is_verified": false,
243-
"line_number": 218,
243+
"line_number": 230,
244244
"type": "Secret Keyword",
245245
"verified_result": null
246246
},
247247
{
248248
"hashed_secret": "7b4455a56fbf1d198e45e04c437488514645a82c",
249249
"is_secret": false,
250250
"is_verified": false,
251-
"line_number": 220,
251+
"line_number": 232,
252252
"type": "Secret Keyword",
253253
"verified_result": null
254254
},
255255
{
256256
"hashed_secret": "90bd1b48e958257948487b90bee080ba5ed00caa",
257257
"is_secret": false,
258258
"is_verified": false,
259-
"line_number": 292,
259+
"line_number": 304,
260260
"type": "Hex High Entropy String",
261261
"verified_result": null
262262
},
263263
{
264264
"hashed_secret": "48ffbad96aa9c2b33f9486f5a3c2108198acb518",
265265
"is_secret": false,
266266
"is_verified": false,
267-
"line_number": 293,
267+
"line_number": 305,
268268
"type": "Hex High Entropy String",
269269
"verified_result": null
270270
}
@@ -3363,24 +3363,6 @@
33633363
"verified_result": null
33643364
}
33653365
],
3366-
"docs/docs/using/agents/beeai.md": [
3367-
{
3368-
"hashed_secret": "5546721ffdfc2e5b0e4c0da38f10774f9ad50b09",
3369-
"is_secret": false,
3370-
"is_verified": false,
3371-
"line_number": 35,
3372-
"type": "Secret Keyword",
3373-
"verified_result": null
3374-
},
3375-
{
3376-
"hashed_secret": "ec545cfb40ff2f1b4eb959dbd0e3177236c540fa",
3377-
"is_secret": false,
3378-
"is_verified": false,
3379-
"line_number": 203,
3380-
"type": "Secret Keyword",
3381-
"verified_result": null
3382-
}
3383-
],
33843366
"docs/docs/using/clients/claude-desktop.md": [
33853367
{
33863368
"hashed_secret": "11c78bef19ce10e34aff236b1f8e37779a74cf24",

AGENTS.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,18 @@ Observability write operations use **independent database sessions** that commit
194194

195195
**Pattern**: Follows existing SQL instrumentation approach in `instrumentation/sqlalchemy.py:58-87`
196196

197+
## Audit Trail Transaction Behavior
198+
199+
**Issue #2871 - Separate Session Pattern**
200+
201+
`AuditTrailService.log_action()` (`mcpgateway/services/audit_trail_service.py`) always opens its own `SessionLocal()` when no `db` is supplied, and closes/rolls back that session itself. Callers in `tool_service.py`, `resource_service.py`, `gateway_service.py`, `prompt_service.py`, `server_service.py`, and `admin.py` must **never** pass `db=db` (the caller's request-scoped session) to `log_action()`. In `admin.py`, plugin-view audit logging goes through `log_audit()`, which is a thin wrapper over `log_action()` and inherits the same optional-session behavior.
202+
203+
Passing the shared session caused **"This transaction is inactive"** errors: the main CRUD operation already calls `db.commit()` before the audit call, and reusing that same session for a second commit after it has already committed leaves the session in a state that breaks rollback on subsequent errors.
204+
205+
- `log_action()` swallows its own exceptions internally and returns `None` on failure — it does not propagate them to the caller in production.
206+
- The main resource (tool/gateway/resource/prompt/server) is committed **before** `log_action()` runs, so an audit-logging failure never rolls back already-persisted data — but callers' generic `except Exception` handlers still call `db.rollback()` and surface an error to the API caller even though the underlying row was already committed.
207+
- Do not add `db: Session` back to a `log_action()` call site. If a service needs the audit entry guaranteed within the same transaction as the main write, that is a deliberate design change requiring review, not a default.
208+
197209
**Middleware**: `ObservabilityMiddleware` no longer creates `request.state.db`. Each observability operation creates its own short-lived session.
198210

199211
**Security**: Query operations use request-scoped sessions for RBAC/token scoping. Write operations are not RBAC-protected (observability visibility is platform-wide).

docs/docs/using/agents/beeai.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ Prepare the ContextForge connection values used in the examples:
3232

3333
```bash
3434
export MCPGATEWAY_BEARER_TOKEN="<gateway-token>"
35-
export MCP_AUTH="Bearer ${MCPGATEWAY_BEARER_TOKEN}"
35+
export MCP_AUTH="Bearer ${MCPGATEWAY_BEARER_TOKEN}" # pragma: allowlist secret
3636

3737
# Virtual server endpoint used by mcpgateway-wrapper.
3838
export MCP_SERVER_URL="http://localhost:4444/servers/UUID_OF_SERVER_1/mcp"
@@ -200,7 +200,7 @@ await client.connect(
200200
args: ["-m", "mcpgateway.wrapper"],
201201
env: {
202202
MCP_SERVER_URL: process.env.MCP_SERVER_URL!,
203-
MCP_AUTH: process.env.MCP_AUTH!,
203+
MCP_AUTH: process.env.MCP_AUTH!, # pragma: allowlist secret
204204
MCP_WRAPPER_LOG_LEVEL: "OFF",
205205
},
206206
})

mcpgateway/admin.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17759,7 +17759,7 @@ async def get_plugin_details(name: str, request: Request, db: Session = Depends(
1775917759

1776017760
# Create audit trail for plugin access
1776117761
audit_service.log_audit(
17762-
user_id=get_user_id(user), user_email=get_user_email(user), resource_type="plugin", resource_id=name, action="view", description=f"Viewed plugin '{name}' details in marketplace", db=db
17762+
user_id=get_user_id(user), user_email=get_user_email(user), resource_type="plugin", resource_id=name, action="view", description=f"Viewed plugin '{name}' details in marketplace"
1776317763
)
1776417764

1776517765
return PluginDetail(**plugin)

mcpgateway/services/audit_trail_service.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,13 @@ def log_action( # noqa: PLR0917 # pylint: disable=too-many-positional-argument
169169
# Use provided session or create new one
170170
close_db = False
171171
if db is None:
172-
db = SessionLocal()
172+
try:
173+
db = SessionLocal()
174+
except Exception as e: # pragma: no cover - defensive
175+
logger.error(
176+
"Failed to create audit trail session: %s", e, exc_info=True, extra={"correlation_id": correlation_id, "action": action, "resource_type": resource_type, "resource_id": resource_id}
177+
)
178+
return None
173179
close_db = True
174180

175181
try:

mcpgateway/services/gateway_service.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1748,7 +1748,6 @@ async def register_gateway(
17481748
context={
17491749
"created_via": created_via,
17501750
},
1751-
db=db,
17521751
)
17531752

17541753
# Structured logging: Log successful gateway creation
@@ -2892,7 +2891,6 @@ async def update_gateway(
28922891
context={
28932892
"modified_via": modified_via,
28942893
},
2895-
db=db,
28962894
)
28972895

28982896
# Structured logging: Log successful gateway update
@@ -3378,7 +3376,6 @@ async def set_gateway_state(self, db: Session, gateway_id: str, activate: bool,
33783376
"action": "activate" if activate else "deactivate",
33793377
"only_update_reachable": only_update_reachable,
33803378
},
3381-
db=db,
33823379
)
33833380

33843381
# Structured logging: Log successful gateway state change
@@ -3581,7 +3578,6 @@ async def delete_gateway(self, db: Session, gateway_id: str, user_email: Optiona
35813578
db.commit()
35823579

35833580
await self._finalize_gateway_deletion(
3584-
db=db,
35853581
gateway_id=str(gateway_id),
35863582
gateway_info=gateway_info,
35873583
gateway_name=gateway_name,
@@ -3657,7 +3653,6 @@ def _hard_delete_gateway(self, db: Session, gateway: DbGateway) -> None:
36573653

36583654
async def _finalize_gateway_deletion(
36593655
self,
3660-
db: Session,
36613656
gateway_id: str,
36623657
gateway_info: Dict[str, Any],
36633658
gateway_name: str,
@@ -3700,7 +3695,6 @@ async def _finalize_gateway_deletion(
37003695
"name": gateway_name,
37013696
"url": gateway_info["url"],
37023697
},
3703-
db=db,
37043698
)
37053699

37063700
structured_logger.log(
@@ -3995,7 +3989,6 @@ async def _process_deleting_gateway(self, db: Session, gateway: DbGateway) -> No
39953989
db.commit()
39963990

39973991
await self._finalize_gateway_deletion(
3998-
db=db,
39993992
gateway_id=str(gateway.id),
40003993
gateway_info=gateway_info,
40013994
gateway_name=gateway_name,

mcpgateway/services/prompt_service.py

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -902,7 +902,6 @@ async def register_prompt(
902902
"import_batch_id": import_batch_id,
903903
"federation_source": federation_source,
904904
},
905-
db=db,
906905
)
907906

908907
# Structured logging: Log successful prompt creation
@@ -1335,7 +1334,6 @@ async def register_prompts_bulk(
13351334
"federation_source": federation_source,
13361335
"conflict_strategy": conflict_strategy,
13371336
},
1338-
db=db,
13391337
)
13401338

13411339
logger.info("Bulk registered %s prompts, updated %s prompts in chunk", len(prompts_to_add), len(prompts_to_update))
@@ -2124,7 +2122,6 @@ async def get_prompt(
21242122
"arguments_provided": arguments_supplied,
21252123
"request_id": request_id,
21262124
},
2127-
db=db,
21282125
)
21292126

21302127
structured_logger.log(
@@ -2422,7 +2419,6 @@ async def update_prompt(
24222419
user_agent=modified_user_agent,
24232420
new_values={"name": prompt.name, "version": prompt.version},
24242421
context={"modified_via": modified_via},
2425-
db=db,
24262422
)
24272423

24282424
structured_logger.log(
@@ -2647,7 +2643,6 @@ async def set_prompt_state(self, db: Session, prompt_id: int, activate: bool, us
26472643
team_id=prompt.team_id,
26482644
new_values={"enabled": prompt.enabled},
26492645
context={"action": "activate" if activate else "deactivate"},
2650-
db=db,
26512646
)
26522647

26532648
structured_logger.log(
@@ -2767,7 +2762,6 @@ async def get_prompt_details(
27672762
resource_name=prompt.name,
27682763
team_id=prompt.team_id,
27692764
context={"include_inactive": include_inactive},
2770-
db=db,
27712765
)
27722766

27732767
structured_logger.log(
@@ -2859,7 +2853,6 @@ async def delete_prompt(self, db: Session, prompt_id: Union[int, str], user_emai
28592853
user_email=user_email,
28602854
team_id=prompt_team_id,
28612855
old_values={"name": prompt_name},
2862-
db=db,
28632856
)
28642857

28652858
# Structured logging: Log successful prompt deletion

mcpgateway/services/resource_service.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,6 @@ async def register_resource(
622622
"import_batch_id": import_batch_id,
623623
"federation_source": federation_source,
624624
},
625-
db=db,
626625
)
627626

628627
# Structured logging: Log successful resource creation
@@ -998,7 +997,6 @@ async def register_resources_bulk(
998997
"federation_source": federation_source,
999998
"conflict_strategy": conflict_strategy,
1000999
},
1001-
db=db,
10021000
)
10031001

10041002
logger.info("Bulk registered %s resources, updated %s resources in chunk", len(resources_to_add), len(resources_to_update))
@@ -2750,7 +2748,6 @@ async def set_resource_state(self, db: Session, resource_id: int, activate: bool
27502748
context={
27512749
"action": "activate" if activate else "deactivate",
27522750
},
2753-
db=db,
27542751
)
27552752

27562753
# Structured logging: Log successful resource state change
@@ -3153,7 +3150,6 @@ async def update_resource(
31533150
"modified_via": modified_via,
31543151
"changes": ", ".join(changes) if changes else "metadata only",
31553152
},
3156-
db=db,
31573153
)
31583154

31593155
# Structured logging: Log successful resource update
@@ -3399,7 +3395,6 @@ async def delete_resource(self, db: Session, resource_id: Union[int, str], user_
33993395
"uri": resource_uri,
34003396
"name": resource_name,
34013397
},
3402-
db=db,
34033398
)
34043399

34053400
# Structured logging: Log successful resource deletion

mcpgateway/services/server_service.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1157,7 +1157,6 @@ async def get_server(
11571157
user_id="system",
11581158
team_id=getattr(server, "team_id", None),
11591159
context={"enabled": server.enabled},
1160-
db=db,
11611160
)
11621161

11631162
return server_read

mcpgateway/services/tool_service.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2071,7 +2071,6 @@ async def register_tool(
20712071
"import_batch_id": import_batch_id,
20722072
"federation_source": federation_source,
20732073
},
2074-
db=db,
20752074
)
20762075

20772076
# Structured logging: Log successful tool creation
@@ -2377,7 +2376,6 @@ def _process_tool_chunk(
23772376
resource_type="tool",
23782377
resource_id=None,
23792378
details={"count": len(tools_to_add) + len(tools_to_update), "import_batch_id": import_batch_id},
2380-
db=db,
23812379
)
23822380

23832381
except Exception as e:
@@ -3368,7 +3366,6 @@ async def delete_tool(self, db: Session, tool_id: str, user_email: Optional[str]
33683366
old_values={
33693367
"name": tool_name,
33703368
},
3371-
db=db,
33723369
)
33733370

33743371
# Structured logging: Log successful tool deletion
@@ -3542,7 +3539,6 @@ async def set_tool_state(self, db: Session, tool_id: str, activate: bool, reacha
35423539
context={
35433540
"action": "activate" if activate else "deactivate",
35443541
},
3545-
db=db,
35463542
)
35473543

35483544
# Structured logging: Log successful tool state change
@@ -6462,7 +6458,6 @@ async def update_tool(
64626458
"modified_via": modified_via,
64636459
"changes": ", ".join(changes) if changes else "metadata only",
64646460
},
6465-
db=db,
64666461
)
64676462

64686463
# Structured logging: Log successful tool update

0 commit comments

Comments
 (0)