Skip to content

Commit d6049c9

Browse files
Copilothotlong
andcommitted
Address code review feedback - add security notes and fix user context
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent 82ffcdb commit d6049c9

3 files changed

Lines changed: 18 additions & 5 deletions

File tree

packages/foundation/core/src/formula-engine.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,11 @@ export class FormulaEngine {
164164

165165
/**
166166
* Execute the expression in a sandboxed environment
167+
*
168+
* SECURITY NOTE: Uses Function constructor for dynamic evaluation.
169+
* While we check for blocked operations, this is not a complete security sandbox.
170+
* For production use with untrusted formulas, consider using a proper sandboxing library
171+
* like vm2 or implementing an AST-based evaluator.
167172
*/
168173
private executeExpression(
169174
expression: string,
@@ -267,6 +272,10 @@ export class FormulaEngine {
267272

268273
/**
269274
* Execute function with timeout protection
275+
*
276+
* NOTE: This implements post-execution timeout validation, not pre-emptive interruption.
277+
* The function will run to completion and then check if it exceeded the timeout.
278+
* For true interruption, platform-specific mechanisms (Worker threads) would be needed.
270279
*/
271280
private executeWithTimeout(
272281
func: Function,
@@ -515,6 +524,10 @@ export class FormulaEngine {
515524

516525
/**
517526
* Validate a formula expression without executing it
527+
*
528+
* SECURITY NOTE: Uses Function constructor for syntax validation.
529+
* This doesn't execute the code but creates a function object.
530+
* For stricter validation, consider using a parser library like @babel/parser.
518531
*/
519532
validate(expression: string): { valid: boolean; errors: string[] } {
520533
const errors: string[] = [];

packages/foundation/core/src/repository.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,8 @@ export class ObjectRepository {
156156
},
157157
current_user: {
158158
id: this.context.userId || '',
159-
name: this.context.userId,
159+
// TODO: Retrieve actual user name from user object if available
160+
name: undefined,
160161
email: undefined,
161162
role: this.context.roles?.[0],
162163
},

packages/foundation/core/test/formula-integration.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,9 @@ describe('Formula Integration', () => {
186186
mockDriver.setMockData('order', [
187187
{
188188
_id: '1',
189-
subtotal: 5000, // Changed to 5000 to be in "Medium" range (> 1000, < 10000)
190-
discount_rate: 10, // 10%
191-
tax_rate: 8, // 8%
189+
subtotal: 5000,
190+
discount_rate: 10,
191+
tax_rate: 8,
192192
status: 'confirmed',
193193
created_at: new Date('2026-01-01'),
194194
},
@@ -198,7 +198,6 @@ describe('Formula Integration', () => {
198198
const result = await ctx.object('order').findOne('1');
199199

200200
expect(result).toBeDefined();
201-
// 5000 * 0.9 * 1.08 = 4860
202201
expect(result.final_price).toBeCloseTo(4860, 1);
203202
expect(result.risk_level).toBe('Medium');
204203
});

0 commit comments

Comments
 (0)