Skip to content

Commit 5c88505

Browse files
committed
fix: address code quality issues identified in GitHub Advanced Security review
- Remove unused imports across multiple files - Improve try-except-continue patterns with proper logging - Fix context manager usage for file operations - Remove unused noqa directives - Add proper SQL injection warning suppressions with noqa comments - Maintain all SchemaPin functionality and backward compatibility
1 parent 3d1c130 commit 5c88505

8 files changed

Lines changed: 73 additions & 71 deletions

File tree

examples/schemapin/advanced_usage.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,9 @@ async def _attempt_key_recovery(self, domain: str) -> str | None:
164164
print(f"Trying alternative endpoint: {endpoint}")
165165
# Simulate discovery attempt
166166
await asyncio.sleep(0.1)
167-
except Exception:
167+
except Exception as e:
168+
# Log the exception for debugging
169+
print(f"Failed to connect to {endpoint}: {e}")
168170
continue
169171

170172
return None

src/mockloop_mcp/generator.py

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -460,23 +460,23 @@ async def export_data():
460460
import io
461461
import zipfile
462462
from fastapi.responses import StreamingResponse
463-
463+
464464
try:
465465
# Create in-memory zip file
466466
zip_buffer = io.BytesIO()
467-
467+
468468
with zipfile.ZipFile(zip_buffer, 'w', zipfile.ZIP_DEFLATED) as zip_file:
469469
# Export request logs
470470
conn = sqlite3.connect(str(DB_PATH))
471471
conn.row_factory = sqlite3.Row
472472
cursor = conn.cursor()
473-
473+
474474
# Get all request logs
475475
cursor.execute('''
476476
SELECT * FROM request_logs ORDER BY created_at DESC
477477
''')
478478
logs = cursor.fetchall()
479-
479+
480480
# Convert to JSON
481481
logs_data = []
482482
for row in logs:
@@ -487,28 +487,28 @@ async def export_data():
487487
except:
488488
pass
489489
logs_data.append(log_entry)
490-
490+
491491
# Add logs to zip
492492
zip_file.writestr("request_logs.json", json.dumps(logs_data, indent=2))
493-
493+
494494
# Export performance metrics if available
495495
try:
496496
cursor.execute('SELECT * FROM performance_metrics ORDER BY recorded_at DESC')
497497
metrics = [dict(row) for row in cursor.fetchall()]
498498
zip_file.writestr("performance_metrics.json", json.dumps(metrics, indent=2))
499499
except:
500500
pass
501-
501+
502502
# Export test sessions if available
503503
try:
504504
cursor.execute('SELECT * FROM test_sessions ORDER BY created_at DESC')
505505
sessions = [dict(row) for row in cursor.fetchall()]
506506
zip_file.writestr("test_sessions.json", json.dumps(sessions, indent=2))
507507
except:
508508
pass
509-
509+
510510
conn.close()
511-
511+
512512
# Add metadata
513513
metadata = {
514514
"export_timestamp": time.strftime('%Y-%m-%dT%H:%M:%S%z', time.gmtime()),
@@ -521,24 +521,24 @@ async def export_data():
521521
}
522522
}
523523
zip_file.writestr("metadata.json", json.dumps(metadata, indent=2))
524-
524+
525525
zip_buffer.seek(0)
526-
526+
527527
# Return as streaming response
528528
def iter_zip():
529529
yield zip_buffer.getvalue()
530-
530+
531531
timestamp = time.strftime('%Y%m%d_%H%M%S', time.gmtime())
532532
filename = f"mockloop_export_{timestamp}.zip"
533-
533+
534534
print(f"DEBUG ADMIN: Exported {len(logs_data)} logs to {filename}")
535-
535+
536536
return StreamingResponse(
537537
iter_zip(),
538538
media_type="application/zip",
539539
headers={"Content-Disposition": f"attachment; filename={filename}"}
540540
)
541-
541+
542542
except Exception as e:
543543
print(f"DEBUG ADMIN: Error exporting data: {e}")
544544
return {"error": str(e)}
@@ -549,11 +549,11 @@ async def get_request_logs(limit: int = 100, offset: int = 0):
549549
conn = sqlite3.connect(str(DB_PATH))
550550
conn.row_factory = sqlite3.Row
551551
cursor = conn.cursor()
552-
552+
553553
# Get total count
554554
cursor.execute("SELECT COUNT(*) FROM request_logs")
555555
total_count = cursor.fetchone()[0]
556-
556+
557557
# Get paginated logs with all available columns
558558
cursor.execute('''
559559
SELECT id, timestamp, type, method, path, status_code, process_time_ms,
@@ -564,7 +564,7 @@ async def get_request_logs(limit: int = 100, offset: int = 0):
564564
ORDER BY created_at DESC
565565
LIMIT ? OFFSET ?
566566
''', (limit, offset))
567-
567+
568568
logs = []
569569
for row in cursor.fetchall():
570570
log_entry = {
@@ -590,11 +590,11 @@ async def get_request_logs(limit: int = 100, offset: int = 0):
590590
"created_at": row["created_at"]
591591
}
592592
logs.append(log_entry)
593-
593+
594594
conn.close()
595595
print(f"DEBUG ADMIN: Retrieved {len(logs)} logs from database (total: {total_count})")
596596
return {"logs": logs, "count": total_count}
597-
597+
598598
except Exception as e:
599599
print(f"DEBUG ADMIN: Error querying database: {e}")
600600
return {"logs": [], "count": 0, "error": str(e)}
@@ -605,27 +605,27 @@ async def get_debug_info():
605605
# Get database info
606606
conn = sqlite3.connect(str(DB_PATH))
607607
cursor = conn.cursor()
608-
608+
609609
# Check database tables and counts
610610
cursor.execute("SELECT name FROM sqlite_master WHERE type='table'")
611611
tables = [row[0] for row in cursor.fetchall()]
612-
612+
613613
table_info = {}
614614
for table in tables:
615615
cursor.execute(f"SELECT COUNT(*) FROM {table}")
616616
count = cursor.fetchone()[0]
617617
table_info[table] = count
618-
618+
619619
# Get recent logs count
620620
cursor.execute("SELECT COUNT(*) FROM request_logs WHERE created_at > datetime('now', '-1 hour')")
621621
recent_logs = cursor.fetchone()[0]
622-
622+
623623
# Get schema version
624624
cursor.execute("SELECT MAX(version) FROM schema_version")
625625
schema_version = cursor.fetchone()[0] or 0
626-
626+
627627
conn.close()
628-
628+
629629
debug_info = {
630630
"status": "ok",
631631
"database": {
@@ -641,13 +641,13 @@ async def get_debug_info():
641641
},
642642
"timestamp": time.strftime('%Y-%m-%dT%H:%M:%S%z', time.gmtime())
643643
}
644-
644+
645645
print(f"DEBUG ADMIN: Debug info retrieved successfully")
646646
return debug_info
647-
647+
648648
except Exception as e:
649649
print(f"DEBUG ADMIN: Error getting debug info: {e}")
650-
return {"status": "error", "error": str(e)}"""
650+
return {"status": "error", "error": str(e)}""" # noqa: S608
651651
webhook_api_endpoints_str = ""
652652
if webhooks_enabled_bool and admin_ui_enabled_bool:
653653
_webhook_api_endpoints_raw = """

src/mockloop_mcp/mcp_audit_logger.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ def _handle_schema_migrations(self, cursor) -> None:
245245
sql_query = f"""
246246
INSERT INTO mcp_data_lineage ({columns_str})
247247
SELECT {columns_str} FROM mcp_data_lineage_backup
248-
""" # nosec B608 # noqa: S608
248+
""" # noqa: S608
249249
cursor.execute(sql_query)
250250

251251
# Drop backup table

src/mockloop_mcp/mcp_compliance.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -462,16 +462,16 @@ def purge_expired_data(self, dry_run: bool = True) -> dict[str, Any]:
462462
placeholders = ",".join(["?" for _ in expired_ids])
463463

464464
# Delete from data lineage table
465-
sql_data_lineage = f"DELETE FROM mcp_data_lineage WHERE entry_id IN ({placeholders})" # nosec B608 # noqa: S608
465+
sql_data_lineage = f"DELETE FROM mcp_data_lineage WHERE entry_id IN ({placeholders})" # noqa: S608
466466
cursor.execute(sql_data_lineage, expired_ids)
467467

468468
# Delete from compliance events table
469-
sql_compliance_events = f"DELETE FROM mcp_compliance_events WHERE entry_id IN ({placeholders})" # nosec B608 # noqa: S608
469+
sql_compliance_events = f"DELETE FROM mcp_compliance_events WHERE entry_id IN ({placeholders})" # noqa: S608
470470
cursor.execute(sql_compliance_events, expired_ids)
471471

472472
# Delete from audit logs table
473473
sql_audit_logs = (
474-
f"DELETE FROM mcp_audit_logs WHERE entry_id IN ({placeholders})" # nosec B608 # noqa: S608
474+
f"DELETE FROM mcp_audit_logs WHERE entry_id IN ({placeholders})" # noqa: S608
475475
)
476476
cursor.execute(sql_audit_logs, expired_ids)
477477

src/mockloop_mcp/schemapin/audit.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import logging
99
import sqlite3
1010
import uuid
11-
from datetime import datetime, timezone, UTC
11+
from datetime import datetime, UTC
1212
from typing import Any
1313

1414
from .config import VerificationResult

src/mockloop_mcp/schemapin/key_management.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import logging
99
import sqlite3
1010
import time
11-
from datetime import datetime, timezone, UTC
11+
from datetime import datetime, UTC
1212
from pathlib import Path
1313
from typing import Any
1414

tests/integration/test_schemapin_integration.py

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -35,25 +35,25 @@ class TestSchemaPinEndToEndWorkflow(unittest.TestCase):
3535

3636
def setUp(self):
3737
"""Set up test environment."""
38-
self.temp_db = tempfile.NamedTemporaryFile(delete=False, suffix='.db')
39-
self.temp_db.close()
38+
with tempfile.NamedTemporaryFile(delete=False, suffix='.db') as temp_db:
39+
self.temp_db_name = temp_db.name
4040

4141
self.config = SchemaPinConfig(
42-
key_pin_storage_path=self.temp_db.name,
42+
key_pin_storage_path=self.temp_db_name,
4343
policy_mode="warn",
4444
auto_pin_keys=False,
4545
trusted_domains=["trusted.example.com"],
4646
interactive_mode=False
4747
)
4848

4949
self.interceptor = SchemaVerificationInterceptor(self.config)
50-
self.key_manager = KeyPinningManager(self.temp_db.name)
50+
self.key_manager = KeyPinningManager(self.temp_db_name)
5151
self.policy_handler = PolicyHandler(self.config)
52-
self.audit_logger = SchemaPinAuditLogger(self.temp_db.name)
52+
self.audit_logger = SchemaPinAuditLogger(self.temp_db_name)
5353

5454
def tearDown(self):
5555
"""Clean up test environment."""
56-
Path(self.temp_db.name).unlink(missing_ok=True)
56+
Path(self.temp_db_name).unlink(missing_ok=True)
5757

5858
async def test_first_time_tool_verification_trusted_domain(self):
5959
"""Test TOFU scenario with trusted domain."""
@@ -298,12 +298,12 @@ async def test_database_persistence_across_sessions(self):
298298
public_key = "-----BEGIN PUBLIC KEY-----\npersistent_key\n-----END PUBLIC KEY-----"
299299

300300
# Pin key with first manager instance
301-
manager1 = KeyPinningManager(self.temp_db.name)
301+
manager1 = KeyPinningManager(self.temp_db_name)
302302
success = manager1.pin_key(tool_id, domain, public_key, {"session": "1"})
303303
assert success is True
304304

305305
# Create new manager instance (simulating new session)
306-
manager2 = KeyPinningManager(self.temp_db.name)
306+
manager2 = KeyPinningManager(self.temp_db_name)
307307
retrieved_key = manager2.get_pinned_key(tool_id)
308308
assert retrieved_key == public_key
309309

@@ -365,17 +365,17 @@ class TestSchemaPinMockLoopIntegration(unittest.TestCase):
365365

366366
def setUp(self):
367367
"""Set up test environment."""
368-
self.temp_db = tempfile.NamedTemporaryFile(delete=False, suffix='.db')
369-
self.temp_db.close()
368+
with tempfile.NamedTemporaryFile(delete=False, suffix='.db') as temp_db:
369+
self.temp_db_name = temp_db.name
370370

371371
self.config = SchemaPinConfig(
372-
key_pin_storage_path=self.temp_db.name,
372+
key_pin_storage_path=self.temp_db_name,
373373
policy_mode="warn"
374374
)
375375

376376
def tearDown(self):
377377
"""Clean up test environment."""
378-
Path(self.temp_db.name).unlink(missing_ok=True)
378+
Path(self.temp_db_name).unlink(missing_ok=True)
379379

380380
def test_mcp_tool_schema_extraction(self):
381381
"""Test extracting schemas from MCP tool functions."""
@@ -401,7 +401,7 @@ def sample_mcp_tool(query: str, database: str = "default") -> dict:
401401

402402
async def test_integration_with_mcp_audit_system(self):
403403
"""Test integration with MockLoop's audit system."""
404-
audit_logger = SchemaPinAuditLogger(self.temp_db.name)
404+
audit_logger = SchemaPinAuditLogger(self.temp_db_name)
405405

406406
# Log various events
407407
await audit_logger.log_verification_attempt(
@@ -421,10 +421,10 @@ async def test_integration_with_mcp_audit_system(self):
421421

422422
def test_database_schema_compatibility(self):
423423
"""Test that SchemaPin tables integrate with MockLoop database."""
424-
SchemaPinAuditLogger(self.temp_db.name) # Creates tables automatically
424+
SchemaPinAuditLogger(self.temp_db_name) # Creates tables automatically
425425

426426
# Verify tables were created
427-
with sqlite3.connect(self.temp_db.name) as conn:
427+
with sqlite3.connect(self.temp_db_name) as conn:
428428
cursor = conn.cursor()
429429
cursor.execute("""
430430
SELECT name FROM sqlite_master

0 commit comments

Comments
 (0)