feat(ai-agent): bootstrap MVP service from scaffold (#420)#546
Open
olaleyeolajide81-sketch wants to merge 2 commits into
Open
feat(ai-agent): bootstrap MVP service from scaffold (#420)#546olaleyeolajide81-sketch wants to merge 2 commits into
olaleyeolajide81-sketch wants to merge 2 commits into
Conversation
- Add package.json, tsconfig.json, tsconfig.test.json - Add src/types.ts: DraftIntentRequest/Response schemas (zod) - Add src/guardrail.ts: enforceNoAutonomousExecution guardrail - Add src/server.ts: Express app with /health and /agent/draft-intent - Add src/server.test.ts: 9 tests covering all endpoints and guardrail - Update README.md with security boundaries and limitations The agent is a draft-only suggestion engine. It never executes financial operations autonomously. All responses carry status='draft' and requiresConfirmation=true, enforced by the guardrail function. Closes ancore-org#420
📝 WalkthroughWalkthroughThis PR establishes two major service components: an AI agent service MVP offering draft-only intent creation with strict guardrails, and a complete OpenAPI specification for the relayer service with contract testing to ensure implementation fidelity. The changes add ~1,100 lines across type definitions, server implementations, test coverage, and API specification artifacts. ChangesAI Agent Service MVP
Relayer OpenAPI Specification and Contract Testing
Sequence Diagram(s)sequenceDiagram
participant Client
participant validateBody as Validation Middleware
participant parseDraftIntent as Intent Parser
participant guardrail as Guardrail Check
participant Response
Client->>validateBody: POST /agent/draft-intent<br/>{ prompt, accountId }
validateBody->>validateBody: Zod safeParse against schema
validateBody->>parseDraftIntent: Call with validated prompt, accountId
parseDraftIntent->>parseDraftIntent: Detect intent type<br/>Create DraftPaymentIntent or<br/>DraftInvoiceIntent with defaults
parseDraftIntent->>guardrail: enforceNoAutonomousExecution(response)
guardrail->>guardrail: Assert status='draft'<br/>Assert requiresConfirmation=true
guardrail->>Response: Return 200 DraftIntentResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The PR introduces two substantial but independent service features: a complete AI agent MVP with guardrails and types, plus a formal OpenAPI spec with contract testing for the relayer. Changes are heterogeneous (types, server logic, tests, specs, scripts) across multiple directories. The core logic density is moderate (intent parsing, validation middleware, guardrail checks), and the contract test suite is comprehensive but methodical. No single file is trivial, but the patterns are consistent and well-scoped within each cohort. Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@olaleyeolajide81-sketch Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
|
@olaleyeolajide81-sketch stick to one pr and resolve conflicts |
olaleyeolajide81-sketch
force-pushed
the
main
branch
from
b3c3911 to
70ff5f0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
services/ai-agent/src/types.ts (1)
20-34: ⚡ Quick winConsider adding validation for
amountandassetfields.The
amountandassetfields in bothDraftPaymentIntentandDraftInvoiceIntentare unvalidated strings. Whilestringis appropriate foramount(to preserve precision), consider adding validation to ensure:
amountrepresents a valid non-negative decimal numberassetfollows expected conventions (e.g., asset code format)This would catch malformed data early and improve type safety for downstream consumers.
♻️ Example validation using Zod refinements
export interface DraftPaymentIntent { type: 'payment'; destination: string; amount: string; // validated via schema asset: string; // validated via schema memo?: string; } // Add schema validation export const DraftPaymentIntentSchema = z.object({ type: z.literal('payment'), destination: z.string().min(1), amount: z.string().regex(/^\d+(\.\d+)?$/, 'Amount must be a valid decimal'), asset: z.string().min(1).max(12), // Stellar asset codes are max 12 chars memo: z.string().optional(), });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/ai-agent/src/types.ts` around lines 20 - 34, The DraftPaymentIntent and DraftInvoiceIntent types expose unvalidated amount and asset strings; add runtime validation schemas (e.g., using Zod or similar) named DraftPaymentIntentSchema and DraftInvoiceIntentSchema that enforce amount is a non-negative decimal string (no leading +/- except optional zero, allow fractional part) and that asset matches your expected asset-code rules (e.g., non-empty, max length/regex for allowed chars), validate destination/requestedBy and optional fields too, export these schemas and use them to parse/validate incoming data before constructing or passing DraftPaymentIntent/DraftInvoiceIntent objects so malformed values are rejected early.services/ai-agent/package.json (1)
6-10: 💤 Low valueConsider adding a
startscript for convenience.The package defines
build,test, andlintscripts but nostartscript. Adding one would improve developer experience when running the service locally.♻️ Suggested addition
"scripts": { "build": "tsc --project tsconfig.json", + "start": "node dist/server.js", "test": "jest --coverage", "lint": "eslint src/" },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/ai-agent/package.json` around lines 6 - 10, Add a "start" script to the package.json "scripts" section so developers can run the built service easily; update the "scripts" object to include a "start" entry (for production: e.g. "start": "node dist/index.js" or combined build+run "start": "npm run build && node dist/index.js", or for local dev use "start:dev": "ts-node src/index.ts") depending on your chosen entrypoint, and ensure the chosen entrypoint (e.g., dist/index.js or src/index.ts) matches the project's compiled output and existing entry module.services/ai-agent/src/server.ts (1)
14-14: ⚡ Quick winConsider making
parseDraftIntentasync now.The comment indicates this will be replaced with LLM/NLP integration, which will require async/await. Making this function async in the MVP avoids a breaking change later and aligns the signature with the expected future implementation.
♻️ Refactor to async pattern
-function parseDraftIntent(prompt: string, accountId: string): DraftIntentResponse { +async function parseDraftIntent(prompt: string, accountId: string): Promise<DraftIntentResponse> {Then update the route handler at line 78:
- const draft = parseDraftIntent(prompt, accountId); + const draft = await parseDraftIntent(prompt, accountId);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/ai-agent/src/server.ts` at line 14, Change parseDraftIntent to an async function signature (async function parseDraftIntent(...): Promise<DraftIntentResponse>) and ensure its body returns a Promise (use return values or Promise.resolve as needed); then update the route handler that calls it (the handler referenced at line 78) to be async and await the call to parseDraftIntent. Keep the function name parseDraftIntent and the DraftIntentResponse type the same so references remain valid.services/relayer/openapi.yaml (3)
319-404: ⚡ Quick winConsider consolidating duplicate schemas using $ref.
RelayExecuteRequestandRelayValidateRequestare structurally identical. Consolidating them into a single shared schema (e.g.,RelayRequestPayload) would improve maintainability and ensure validation rules stay synchronized.♻️ Proposed refactor using shared schema
Add a shared base schema in components:
schemas: + RelayRequestPayload: + type: object + required: + - sessionKey + - operation + - parameters + - signature + - nonce + properties: + sessionKey: + type: string + pattern: '^[0-9a-fA-F]{64}$' + minLength: 64 + maxLength: 64 + description: Hex-encoded Ed25519 session public key (64 characters) + example: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + operation: + type: string + enum: ['relay_execute', 'add_session_key', 'revoke_session_key'] + description: Type of operation + example: "relay_execute" + parameters: + type: object + additionalProperties: true + description: Operation-specific parameters + example: + accountAddress: "GBBM6BKZPEBWYY3A3YR4IK7T7XZM5JC5K7NYGR7KDCXYBCJVPQYV5YAA" + to: "GD7OEZ2NYNQXK7FLTLQZZCNY2DZV5C7M3F4TNZBAYEBQKVU5RQV6SRQQ" + functionName: "transfer" + args: ["base64_encoded_xdr"] + signature: + type: string + pattern: '^[0-9a-fA-F]{128}$' + minLength: 128 + maxLength: 128 + description: Hex-encoded Ed25519 signature over canonical payload (128 characters) + example: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + nonce: + type: integer + minimum: 0 + description: Replay-protection nonce (non-negative integer) + example: 1 + RelayExecuteRequest: - type: object - required: - - sessionKey - - operation - - parameters - - signature - - nonce - properties: - sessionKey: - type: string - pattern: '^[0-9a-fA-F]{64}$' - minLength: 64 - maxLength: 64 - description: Hex-encoded Ed25519 session public key (64 characters) - example: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" - operation: - type: string - enum: ['relay_execute', 'add_session_key', 'revoke_session_key'] - description: Type of operation to execute - example: "relay_execute" - parameters: - type: object - additionalProperties: true - description: Operation-specific parameters - example: - accountAddress: "GBBM6BKZPEBWYY3A3YR4IK7T7XZM5JC5K7NYGR7KDCXYBCJVPQYV5YAA" - to: "GD7OEZ2NYNQXK7FLTLQZZCNY2DZV5C7M3F4TNZBAYEBQKVU5RQV6SRQQ" - functionName: "transfer" - args: ["base64_encoded_xdr"] - signature: - type: string - pattern: '^[0-9a-fA-F]{128}$' - minLength: 128 - maxLength: 128 - description: Hex-encoded Ed25519 signature over canonical payload (128 characters) - example: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" - nonce: - type: integer - minimum: 0 - description: Replay-protection nonce (non-negative integer) - example: 1 + allOf: + - $ref: '`#/components/schemas/RelayRequestPayload`' + - description: Request payload for relay execution RelayValidateRequest: - type: object - required: - - sessionKey - - operation - - parameters - - signature - - nonce - properties: - sessionKey: - type: string - pattern: '^[0-9a-fA-F]{64}$' - minLength: 64 - maxLength: 64 - description: Hex-encoded Ed25519 session public key (64 characters) - example: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" - operation: - type: string - enum: ['relay_execute', 'add_session_key', 'revoke_session_key'] - description: Type of operation to validate - example: "relay_execute" - parameters: - type: object - additionalProperties: true - description: Operation-specific parameters - example: - accountAddress: "GBBM6BKZPEBWYY3A3YR4IK7T7XZM5JC5K7NYGR7KDCXYBCJVPQYV5YAA" - to: "GD7OEZ2NYNQXK7FLTLQZZCNY2DZV5C7M3F4TNZBAYEBQKVU5RQV6SRQQ" - functionName: "transfer" - args: ["base64_encoded_xdr"] - signature: - type: string - pattern: '^[0-9a-fA-F]{128}$' - minLength: 128 - maxLength: 128 - description: Hex-encoded Ed25519 signature over canonical payload (128 characters) - example: "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" - nonce: - type: integer - minimum: 0 - description: Replay-protection nonce (non-negative integer) - example: 1 + allOf: + - $ref: '`#/components/schemas/RelayRequestPayload`' + - description: Request payload for relay validation🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/relayer/openapi.yaml` around lines 319 - 404, RelayExecuteRequest and RelayValidateRequest are identical; create a single shared schema (e.g., RelayRequestPayload) that defines sessionKey, operation, parameters, signature, and nonce, then replace the RelayExecuteRequest and RelayValidateRequest definitions with $ref entries pointing to RelayRequestPayload so both use the same canonical validation rules (update schema names in components and references to RelayExecuteRequest/RelayValidateRequest accordingly).
531-545: ⚡ Quick winAdd maxItems constraint to validation details array.
The
detailsarray lacks amaxItemsconstraint, which could allow unbounded error responses if a request fails validation on many fields. This creates a potential DoS vector.🛡️ Proposed fix to add maxItems
details: type: array + maxItems: 50 items: type: object🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/relayer/openapi.yaml` around lines 531 - 545, The "details" array schema is missing a maxItems constraint allowing unbounded items; update the array definition for the "details" property (the array whose items are objects with required "field" and "message" properties) to include a reasonable maxItems (e.g., 10) to limit returned validation errors and prevent potential DoS; keep the existing item schema (with "field" and "message") intact and add the maxItems attribute directly on the array node in openapi.yaml.
417-417: transactionId hex case matches current relayer implementation.The relayer’s mock
transactionIdis generated asrandomBytes(32).toString('hex').toUpperCase(), producing 64 uppercase hex chars—sotransactionIdusing^[0-9A-F]{64}$aligns with the implementation. Session key/signature validation in code accepts both cases (^[0-9a-fA-F]+$), so optionally maketransactionIdcase-insensitive in OpenAPI (^[0-9a-fA-F]{64}$) for consistency and to tolerate lowercase from real sources.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/relayer/openapi.yaml` at line 417, The OpenAPI schema for transactionId currently restricts to uppercase hex via pattern '^[0-9A-F]{64}$'; update the pattern on the transactionId property in services/relayer/openapi.yaml to accept both cases (e.g. '^[0-9a-fA-F]{64}$') so it matches session key/signature validation and tolerates lowercase transaction IDs generated by external sources while keeping the 64-byte length constraint.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/ai-agent/src/server.ts`:
- Around line 66-67: The express.json() middleware call
(app.use(express.json())) lacks an explicit body size limit; update the
middleware to include a conservative limit (e.g., app.use(express.json({ limit:
'50kb' })) — pick the exact value appropriate for your intents) and add the same
explicit limit to express.urlencoded if used (app.use(express.urlencoded({
extended: true, limit: '50kb' }))); also add a short inline comment near app to
document why the limit is set (prevent large-payload DoS and protect sensitive
financial intent requests).
- Around line 73-81: The POST /agent/draft-intent route currently calls
parseDraftIntent synchronously and will crash the process if that function
throws; wrap the handler logic in a try-catch (or convert to async and await
inside try-catch) so errors from parseDraftIntent are caught and a 4xx/5xx JSON
error response is returned, or forward the error to next(err) for Express error
middleware; ensure validateBody(DraftIntentRequestSchema) stays and reference
parseDraftIntent, DraftIntentRequestSchema and the '/agent/draft-intent' route
when applying the fix.
- Around line 54-56: The current handler returns result.error.flatten() directly
(in the block where result.success is false), which leaks internal
schema/validation details; replace that with a sanitized payload—either a
generic message and an optional list of invalid field names only. Update the
response in the res.status(400).json call to call a sanitizer (e.g.,
sanitizeValidationError(result.error)) or inline logic that extracts only field
paths (e.g., map error details to their field names) and exclude messages,
schema types, or stack info; keep the status and shape ({ error: 'Invalid
request', fields: [...] }) so callers get minimal actionable info without
exposing internal rules.
- Around line 14-16: The parseDraftIntent function accepts prompt without
validation which risks DoS and memory issues; add input validation at the start
of parseDraftIntent(prompt: string, accountId: string): DraftIntentResponse to:
1) assert prompt is a non-empty string and trim it, 2) enforce a reasonable max
length (e.g. configurable constant) and reject or return an error
DraftIntentResponse when exceeded, 3) optionally sanitize/control characters
(e.g. remove excessive whitespace/newlines) and reject binary/invalid input, and
4) ensure failures return a safe, descriptive DraftIntentResponse or throw a
controlled error rather than proceeding with parsing.
In `@services/ai-agent/src/types.ts`:
- Line 9: The accountId schema currently allows any non-empty string; change the
zod validator for accountId (where it's defined as z.string().min(1)) to enforce
Stellar public key format by using a regex check (e.g., require a leading 'G'
and 55 base32 characters A-Z2-7 for a total length of 56) or
z.string().length(56). Replace z.string().min(1) with
z.string().regex(/^G[A-Z2-7]{55}$/, "Invalid Stellar public key") (or an
equivalent zod refine) so invalid Stellar addresses are rejected at validation
time.
---
Nitpick comments:
In `@services/ai-agent/package.json`:
- Around line 6-10: Add a "start" script to the package.json "scripts" section
so developers can run the built service easily; update the "scripts" object to
include a "start" entry (for production: e.g. "start": "node dist/index.js" or
combined build+run "start": "npm run build && node dist/index.js", or for local
dev use "start:dev": "ts-node src/index.ts") depending on your chosen
entrypoint, and ensure the chosen entrypoint (e.g., dist/index.js or
src/index.ts) matches the project's compiled output and existing entry module.
In `@services/ai-agent/src/server.ts`:
- Line 14: Change parseDraftIntent to an async function signature (async
function parseDraftIntent(...): Promise<DraftIntentResponse>) and ensure its
body returns a Promise (use return values or Promise.resolve as needed); then
update the route handler that calls it (the handler referenced at line 78) to be
async and await the call to parseDraftIntent. Keep the function name
parseDraftIntent and the DraftIntentResponse type the same so references remain
valid.
In `@services/ai-agent/src/types.ts`:
- Around line 20-34: The DraftPaymentIntent and DraftInvoiceIntent types expose
unvalidated amount and asset strings; add runtime validation schemas (e.g.,
using Zod or similar) named DraftPaymentIntentSchema and
DraftInvoiceIntentSchema that enforce amount is a non-negative decimal string
(no leading +/- except optional zero, allow fractional part) and that asset
matches your expected asset-code rules (e.g., non-empty, max length/regex for
allowed chars), validate destination/requestedBy and optional fields too, export
these schemas and use them to parse/validate incoming data before constructing
or passing DraftPaymentIntent/DraftInvoiceIntent objects so malformed values are
rejected early.
In `@services/relayer/openapi.yaml`:
- Around line 319-404: RelayExecuteRequest and RelayValidateRequest are
identical; create a single shared schema (e.g., RelayRequestPayload) that
defines sessionKey, operation, parameters, signature, and nonce, then replace
the RelayExecuteRequest and RelayValidateRequest definitions with $ref entries
pointing to RelayRequestPayload so both use the same canonical validation rules
(update schema names in components and references to
RelayExecuteRequest/RelayValidateRequest accordingly).
- Around line 531-545: The "details" array schema is missing a maxItems
constraint allowing unbounded items; update the array definition for the
"details" property (the array whose items are objects with required "field" and
"message" properties) to include a reasonable maxItems (e.g., 10) to limit
returned validation errors and prevent potential DoS; keep the existing item
schema (with "field" and "message") intact and add the maxItems attribute
directly on the array node in openapi.yaml.
- Line 417: The OpenAPI schema for transactionId currently restricts to
uppercase hex via pattern '^[0-9A-F]{64}$'; update the pattern on the
transactionId property in services/relayer/openapi.yaml to accept both cases
(e.g. '^[0-9a-fA-F]{64}$') so it matches session key/signature validation and
tolerates lowercase transaction IDs generated by external sources while keeping
the 64-byte length constraint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1083fc96-f5a5-4675-9799-c8cc73602e56
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
scripts/generate-openapi-types.tsservices/ai-agent/README.mdservices/ai-agent/package.jsonservices/ai-agent/src/guardrail.tsservices/ai-agent/src/server.test.tsservices/ai-agent/src/server.tsservices/ai-agent/src/types.tsservices/ai-agent/tsconfig.jsonservices/ai-agent/tsconfig.test.jsonservices/relayer/README.mdservices/relayer/openapi.yamlservices/relayer/tests/contract/openapi-contract.test.ts
Comment on lines
+14
to
+16
| function parseDraftIntent(prompt: string, accountId: string): DraftIntentResponse { | ||
| const lower = prompt.toLowerCase(); | ||
| const isInvoice = lower.includes('invoice') || lower.includes('request'); |
There was a problem hiding this comment.
Add input validation on prompt parameter.
The prompt parameter is accepted without any length or content validation. For a financial service, unbounded input poses DoS and memory consumption risks. Consider adding validation before processing.
🛡️ Proposed fix to add prompt validation
function parseDraftIntent(prompt: string, accountId: string): DraftIntentResponse {
+ if (!prompt || prompt.length > 5000) {
+ throw new Error('Invalid prompt: must be 1-5000 characters');
+ }
const lower = prompt.toLowerCase();
const isInvoice = lower.includes('invoice') || lower.includes('request');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function parseDraftIntent(prompt: string, accountId: string): DraftIntentResponse { | |
| const lower = prompt.toLowerCase(); | |
| const isInvoice = lower.includes('invoice') || lower.includes('request'); | |
| function parseDraftIntent(prompt: string, accountId: string): DraftIntentResponse { | |
| if (!prompt || prompt.length > 5000) { | |
| throw new Error('Invalid prompt: must be 1-5000 characters'); | |
| } | |
| const lower = prompt.toLowerCase(); | |
| const isInvoice = lower.includes('invoice') || lower.includes('request'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ai-agent/src/server.ts` around lines 14 - 16, The parseDraftIntent
function accepts prompt without validation which risks DoS and memory issues;
add input validation at the start of parseDraftIntent(prompt: string, accountId:
string): DraftIntentResponse to: 1) assert prompt is a non-empty string and trim
it, 2) enforce a reasonable max length (e.g. configurable constant) and reject
or return an error DraftIntentResponse when exceeded, 3) optionally
sanitize/control characters (e.g. remove excessive whitespace/newlines) and
reject binary/invalid input, and 4) ensure failures return a safe, descriptive
DraftIntentResponse or throw a controlled error rather than proceeding with
parsing.
Comment on lines
+54
to
+56
| if (!result.success) { | ||
| res.status(400).json({ error: 'Invalid request', details: result.error.flatten() }); | ||
| return; |
There was a problem hiding this comment.
Sanitize validation error details before exposing to clients.
Returning result.error.flatten() directly exposes internal schema structure and validation details. For a financial service, this could leak information about expected request formats and business logic constraints that should remain internal.
🔒 Proposed fix to sanitize validation errors
if (!result.success) {
- res.status(400).json({ error: 'Invalid request', details: result.error.flatten() });
+ res.status(400).json({ error: 'Invalid request', message: 'Request body validation failed' });
return;
}Alternatively, if some detail is needed for debugging, provide only field names:
if (!result.success) {
+ const fieldErrors = result.error.flatten().fieldErrors;
+ const fields = Object.keys(fieldErrors);
- res.status(400).json({ error: 'Invalid request', details: result.error.flatten() });
+ res.status(400).json({ error: 'Invalid request', invalidFields: fields });
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!result.success) { | |
| res.status(400).json({ error: 'Invalid request', details: result.error.flatten() }); | |
| return; | |
| if (!result.success) { | |
| res.status(400).json({ error: 'Invalid request', message: 'Request body validation failed' }); | |
| return; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ai-agent/src/server.ts` around lines 54 - 56, The current handler
returns result.error.flatten() directly (in the block where result.success is
false), which leaks internal schema/validation details; replace that with a
sanitized payload—either a generic message and an optional list of invalid field
names only. Update the response in the res.status(400).json call to call a
sanitizer (e.g., sanitizeValidationError(result.error)) or inline logic that
extracts only field paths (e.g., map error details to their field names) and
exclude messages, schema types, or stack info; keep the status and shape ({
error: 'Invalid request', fields: [...] }) so callers get minimal actionable
info without exposing internal rules.
Comment on lines
+66
to
+67
| const app = express(); | ||
| app.use(express.json()); |
There was a problem hiding this comment.
Configure explicit request body size limit.
The express.json() middleware uses a default limit (100kb), but for a financial service handling sensitive intent requests, the limit should be explicitly configured and documented. This prevents potential DoS attacks via large payloads.
🛡️ Proposed fix to add explicit limit
const app = express();
- app.use(express.json());
+ app.use(express.json({ limit: '10kb' }));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const app = express(); | |
| app.use(express.json()); | |
| const app = express(); | |
| app.use(express.json({ limit: '10kb' })); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ai-agent/src/server.ts` around lines 66 - 67, The express.json()
middleware call (app.use(express.json())) lacks an explicit body size limit;
update the middleware to include a conservative limit (e.g.,
app.use(express.json({ limit: '50kb' })) — pick the exact value appropriate for
your intents) and add the same explicit limit to express.urlencoded if used
(app.use(express.urlencoded({ extended: true, limit: '50kb' }))); also add a
short inline comment near app to document why the limit is set (prevent
large-payload DoS and protect sensitive financial intent requests).
Comment on lines
+73
to
+81
| app.post( | ||
| '/agent/draft-intent', | ||
| validateBody(DraftIntentRequestSchema), | ||
| (req: Request, res: Response) => { | ||
| const { prompt, accountId } = req.body; | ||
| const draft = parseDraftIntent(prompt, accountId); | ||
| res.status(200).json(draft); | ||
| } | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add error handling to the route handler.
If parseDraftIntent throws an error (e.g., from the guardrail enforcement), the current implementation will crash the process. Wrap the logic in try-catch or add Express error-handling middleware.
🛡️ Proposed fix to add try-catch
app.post(
'/agent/draft-intent',
validateBody(DraftIntentRequestSchema),
(req: Request, res: Response) => {
+ try {
const { prompt, accountId } = req.body;
const draft = parseDraftIntent(prompt, accountId);
res.status(200).json(draft);
+ } catch (error) {
+ res.status(500).json({ error: 'Internal server error' });
+ }
}
);Alternatively, add a global error handler after all routes:
return app;
+
+ // Error handling middleware
+ app.use((err: Error, req: Request, res: Response, next: NextFunction) => {
+ console.error('Unhandled error:', err);
+ res.status(500).json({ error: 'Internal server error' });
+ });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| app.post( | |
| '/agent/draft-intent', | |
| validateBody(DraftIntentRequestSchema), | |
| (req: Request, res: Response) => { | |
| const { prompt, accountId } = req.body; | |
| const draft = parseDraftIntent(prompt, accountId); | |
| res.status(200).json(draft); | |
| } | |
| ); | |
| app.post( | |
| '/agent/draft-intent', | |
| validateBody(DraftIntentRequestSchema), | |
| (req: Request, res: Response) => { | |
| try { | |
| const { prompt, accountId } = req.body; | |
| const draft = parseDraftIntent(prompt, accountId); | |
| res.status(200).json(draft); | |
| } catch (error) { | |
| res.status(500).json({ error: 'Internal server error' }); | |
| } | |
| } | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ai-agent/src/server.ts` around lines 73 - 81, The POST
/agent/draft-intent route currently calls parseDraftIntent synchronously and
will crash the process if that function throws; wrap the handler logic in a
try-catch (or convert to async and await inside try-catch) so errors from
parseDraftIntent are caught and a 4xx/5xx JSON error response is returned, or
forward the error to next(err) for Express error middleware; ensure
validateBody(DraftIntentRequestSchema) stays and reference parseDraftIntent,
DraftIntentRequestSchema and the '/agent/draft-intent' route when applying the
fix.
| /** Natural-language description of the intended operation */ | ||
| prompt: z.string().min(1).max(1000), | ||
| /** Stellar account address of the initiating user */ | ||
| accountId: z.string().min(1), |
There was a problem hiding this comment.
Add Stellar address format validation to accountId.
The accountId field accepts any non-empty string but should validate that it matches Stellar's public key format (56 characters, starting with 'G', base32-encoded). Without this validation, invalid addresses can enter the system and cause downstream errors or security issues.
🛡️ Proposed fix to add Stellar address validation
export const DraftIntentRequestSchema = z.object({
/** Natural-language description of the intended operation */
prompt: z.string().min(1).max(1000),
/** Stellar account address of the initiating user */
- accountId: z.string().min(1),
+ accountId: z.string().regex(/^G[A-Z2-7]{55}$/, 'Invalid Stellar account address'),
/** Optional context for the intent (e.g. invoice ID, session key) */
context: z.record(z.unknown()).optional(),
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/ai-agent/src/types.ts` at line 9, The accountId schema currently
allows any non-empty string; change the zod validator for accountId (where it's
defined as z.string().min(1)) to enforce Stellar public key format by using a
regex check (e.g., require a leading 'G' and 55 base32 characters A-Z2-7 for a
total length of 56) or z.string().length(56). Replace z.string().min(1) with
z.string().regex(/^G[A-Z2-7]{55}$/, "Invalid Stellar public key") (or an
equivalent zod refine) so invalid Stellar addresses are rejected at validation
time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The agent is a draft-only suggestion engine. It never executes financial operations autonomously. All responses carry status='draft' and requiresConfirmation=true, enforced by the guardrail function.
Closes #420
Description
Type of Change
Security Impact
Testing
Test Coverage
Manual Testing Steps
Breaking Changes
Checklist
For High-Security Changes
Related Issues
Closes #
Related to #
Additional Context
Reviewer Notes
closes #420
Summary by CodeRabbit
New Features
/agent/draft-intentendpoint for creating payment and invoice drafts./relay/execute,/relay/validate,/relay/status).Documentation
Tests