-
Notifications
You must be signed in to change notification settings - Fork 3
Demo Base Kit #240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Demo Base Kit #240
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨ PR Review
The PR adds database persistence, order history functionality, and UI improvements to a demo application. The implementation includes several concerning security and reliability issues that should be addressed.
3 issues detected:
🔒 Security - Concurrent file access without synchronization can lead to data corruption and lost billing records
Details: The file-based billing queue implementation has a race condition where concurrent requests for the same user could corrupt the JSON file or lose billing records. Multiple threads can read and write the same file simultaneously without proper synchronization.
File:services/billing-csharp/Controllers/BillingController.cs (56-72)
🔒 Security - Unvalidated user input from external service used directly in SQL queries creates injection vulnerability
Details: The username parameter from token validation is directly used in SQL queries without proper validation. If the auth service is compromised or returns malicious data, it could lead to SQL injection attacks in the order history endpoint.
File:services/orders-java/src/main/java/com/example/orders/controller/OrderController.java (93-93)
🐞 Bug - Lack of transactional consistency between billing and order storage can lead to financial discrepancies
Details: If the database insert fails after billing has succeeded, the user will be charged but no order will be recorded. There's no mechanism to rollback the billing transaction or ensure data consistency between services.
File:services/orders-java/src/main/java/com/example/orders/controller/OrderController.java (75-75)
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using. We'd love your feedback! 🚀
private async Task QueueForBillingSystemAsync(string username, object payload) | ||
{ | ||
Directory.CreateDirectory(StorageDirectory); | ||
var filePath = Path.Combine(StorageDirectory, $"{username}.json"); | ||
List<object> payloads = new(); | ||
|
||
if (System.IO.File.Exists(filePath)) | ||
{ | ||
try | ||
{ | ||
payloads = JsonSerializer.Deserialize<List<object>>(await System.IO.File.ReadAllTextAsync(filePath)) ?? new(); | ||
} | ||
catch { } | ||
} | ||
|
||
payloads.Add(payload); | ||
await System.IO.File.WriteAllTextAsync(filePath, JsonSerializer.Serialize(payloads, new JsonSerializerOptions { WriteIndented = true })); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security - File System Race Condition: Implement proper file locking using FileStream with exclusive access, or replace the file-based approach with a thread-safe in-memory queue or database storage.
private async Task QueueForBillingSystemAsync(string username, object payload) | |
{ | |
Directory.CreateDirectory(StorageDirectory); | |
var filePath = Path.Combine(StorageDirectory, $"{username}.json"); | |
List<object> payloads = new(); | |
if (System.IO.File.Exists(filePath)) | |
{ | |
try | |
{ | |
payloads = JsonSerializer.Deserialize<List<object>>(await System.IO.File.ReadAllTextAsync(filePath)) ?? new(); | |
} | |
catch { } | |
} | |
payloads.Add(payload); | |
await System.IO.File.WriteAllTextAsync(filePath, JsonSerializer.Serialize(payloads, new JsonSerializerOptions { WriteIndented = true })); | |
private static readonly Dictionary<string, SemaphoreSlim> _fileLocks = new(); | |
private static readonly object _lockDictionary = new(); | |
private async Task QueueForBillingSystemAsync(string username, object payload) | |
{ | |
Directory.CreateDirectory(StorageDirectory); | |
var filePath = Path.Combine(StorageDirectory, $"{username}.json"); | |
// Get or create a semaphore for this user's file | |
SemaphoreSlim fileLock; | |
lock (_lockDictionary) | |
{ | |
if (!_fileLocks.TryGetValue(username, out fileLock)) | |
{ | |
fileLock = new SemaphoreSlim(1, 1); | |
_fileLocks[username] = fileLock; | |
} | |
} | |
await fileLock.WaitAsync(); | |
try | |
{ | |
List<object> payloads = new(); | |
if (System.IO.File.Exists(filePath)) | |
{ | |
try | |
{ | |
payloads = JsonSerializer.Deserialize<List<object>>(await System.IO.File.ReadAllTextAsync(filePath)) ?? new(); | |
} | |
catch { } | |
} | |
payloads.Add(payload); | |
await System.IO.File.WriteAllTextAsync(filePath, JsonSerializer.Serialize(payloads, new JsonSerializerOptions { WriteIndented = true })); | |
} | |
finally | |
{ | |
fileLock.Release(); | |
} | |
} |
if (username == null) { | ||
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body("Invalid session"); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security - SQL Injection Vulnerability: Add input validation for the username returned from token validation, ensuring it contains only alphanumeric characters and safe symbols before using it in SQL queries.
java.sql.PreparedStatement pstmt = conn.prepareStatement("SELECT * FROM orders WHERE username = ?")) { | |
// Validate username to prevent SQL injection | |
if (username == null || !username.matches("^[a-zA-Z0-9_.-]+$")) { | |
return ResponseEntity.status(HttpStatus.BAD_REQUEST).body("Invalid username format"); | |
} | |
pstmt.setString(5, timestamp); | ||
pstmt.executeUpdate(); | ||
} catch (Exception e) { | ||
e.printStackTrace(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐞 Bug - Transaction Rollback Missing: Implement a compensation mechanism to reverse billing charges when order storage fails, or use a distributed transaction pattern to ensure consistency across services.
e.printStackTrace(); | |
e.printStackTrace(); | |
// Rollback billing charge if order storage fails | |
try { | |
refund(username, request.productId, request.quantity); | |
} catch (Exception refundException) { | |
refundException.printStackTrace(); | |
} |
✨ PR Description
Purpose: Add order history functionality to the frontend and backend with Swagger API documentation integration.
Main changes:
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using. We'd love your feedback! 🚀