Security Code Review Report — zerodha-kite

1. OWASP Review Methodology Applied

I applied an OWASP-aligned review process as follows:

• Architecture/context established by reading the main entry point (src/index.ts), project metadata (package.json), deployment/container config (Dockerfile), environment template (.env.example), and type declarations (src/types/kiteconnect.d.ts).
• Trust boundaries and entry points identified: the MCP stdio interface is the only exposed application entry point. Untrusted input enters through MCP tool invocations handled by CallToolRequestSchema in src/index.ts.
• Data flows traced from MCP request arguments into trading/account operations on the kiteconnect SDK, and from Zerodha API responses back into MCP responses and logs.
• Threats modeled around unauthorized trading actions, exposure of sensitive financial/account data, unsafe logging, weak input validation, and dependency/runtime configuration.
• Controls verified by reading all security-relevant files in the repository and searching for network calls, subprocess execution, dynamic code execution, filesystem access, and secret handling.
• Findings validated with code evidence and mapped to OWASP Top 10 2021 where applicable.

2. OWASP Top 10 Category Mapping

3. Critical Vulnerabilities

None confirmed.

I found no evidence of:
- RCE
- command injection
- unsafe deserialization
- eval/dynamic code execution
- auth bypass inside an existing auth layer

4. High Severity Issues

H1. No authorization or policy guardrails on high-risk trading actions

• Severity: High
• OWASP: A01 Broken Access Control, A04 Insecure Design
• File: src/index.ts
• Lines: 76-117, 137-269, 274-327

Evidence

The MCP server accepts tool calls over stdio and directly dispatches them to trading operations:
- src/index.ts:76-117 routes any CallToolRequestSchema request by tool name.
- src/index.ts:137-149 exposes place_order.
- src/index.ts:155-164 exposes modify_order.
- src/index.ts:170-175 exposes cancel_order.
- src/index.ts:274-286 directly calls this.kite!.placeOrder('regular', orderParams).
- src/index.ts:298-306 directly calls modifyOrder.
- src/index.ts:318-319 directly calls cancelOrder.

There is no local authorization, allowlist, confirmation gate, environment-based disable flag, approval workflow, or policy enforcement for trade execution.

Why this matters

In MCP deployments, the effective caller is often an AI assistant acting on user prompts and tool orchestration. If the hosting client, prompt chain, or connected agent is induced to invoke these tools, the server will execute real trading actions immediately with the configured account token.

This is not a traditional remote unauthenticated network service, but it is still a real access control failure at the tool boundary: the code treats any caller that can reach the MCP transport as fully authorized to trade.

Exploitability

High in realistic MCP environments where:
- the server is connected to an assistant with broad tool access, and
- a malicious or mistaken prompt triggers tool usage.

Impact

High. Can cause:
- unauthorized market orders,
- order modification/cancellation,
- direct financial loss.

Remediation

Add explicit local controls before permitting state-changing operations:
- Require an execution mode flag such as ENABLE_TRADING=false by default, with hard fail unless explicitly enabled.
- Require a second factor / confirmation token / out-of-band approval for place_order, modify_order, and cancel_order.
- Enforce a tool allowlist and policy checks on symbols, exchanges, products, and max quantity/notional.
- Consider splitting market-data tools and trading tools into separate deployments/tokens.

H2. Full account, holdings, orders, trades, and profile data are exposed to the MCP client without minimization

• Severity: High
• OWASP: A01 Broken Access Control, A04 Insecure Design
• File: src/index.ts
• Lines: 330-467

Evidence

The server returns complete Zerodha API responses as plaintext JSON to the MCP caller:
- src/index.ts:330-339 getPositions() returns all positions.
- src/index.ts:342-351 getHoldings() returns all holdings.
- src/index.ts:354-363 getMargins() returns all margin data.
- src/index.ts:408-417 getOrders() returns all orders.
- src/index.ts:420-429 getTrades() returns all trades.
- src/index.ts:432-441 getOrderHistory() returns full history.
- src/index.ts:444-453 getProfile() returns full profile.
- src/index.ts:336, 348, 360, 414, 426, 438, 450 all use JSON.stringify(..., null, 2) with no field filtering.

The declared Profile type includes directly identifying fields such as:
- src/types/kiteconnect.d.ts:106-111 — user_id, user_name, user_shortname, email, user_type, broker.

Why this matters

This design gives the MCP consumer unrestricted access to sensitive account and financial data, including personally identifying profile data and detailed trading history, regardless of need-to-know. In AI-integrated workflows, this data may be retained in conversation history, telemetry, prompt logs, or downstream tool chains.

Exploitability

High if an attacker can influence or access the MCP-connected assistant session.

Impact

High due to privacy exposure, account intelligence leakage, and possible downstream social engineering or targeted trading abuse.

Remediation

• Return only the minimum fields required for each tool.
• Redact PII from get_profile by default.
• Add a safe mode that disables account-history/profile tools unless explicitly enabled.
• Prefer structured outputs with explicit field selection instead of dumping raw API JSON.

5. Medium Severity Issues

M1. Missing robust server-side validation for trading and query inputs

• Severity: Medium
• OWASP: A04 Insecure Design
• File: src/index.ts
• Lines: 137-269, 274-397, 456-457

Evidence

Tool schemas define some expected properties, but validation is incomplete and permissive:
- No additionalProperties: false is present in any tool schema (src/index.ts:137-269).
- Handlers accept args: any throughout (src/index.ts:274, 294, 317, 366, 379, 396, 432, 456).
- placeOrder() passes values through directly (src/index.ts:275-284) with no checks for:
- positive integer quantity,
- limit price requirements,
- SL trigger/price consistency,
- exchange/symbol normalization,
- max order size/notional.
- getHistoricalData() constructs dates from unvalidated strings (src/index.ts:380-384) and does not reject invalid dates.
- getLTP() directly forwards args.instruments to the SDK (src/index.ts:456-457) with no type/size checks beyond declared schema.
- modifyOrder() forwards optional fields as-is (src/index.ts:295-304) with no filtering of undefined or invalid combinations.

Why this matters

This is primarily an insecure design / validation issue rather than classic injection. Poor validation increases the chance of:
- accidental harmful trades,
- malformed requests hitting the brokerage API,
- inconsistent behavior across MCP clients,
- bypass of intended client-side schema restrictions.

Exploitability

Moderate. Requires ability to invoke MCP tools.

Impact

Moderate. Can result in unintended order placement attempts, confusing failures, or business logic abuse.

Remediation

• Replace any with strict TypeScript request types.
• Enforce runtime validation with a schema library such as zod/ajv.
• Add additionalProperties: false to all input schemas.
• Validate business rules server-side before calling the brokerage SDK.

M2. Raw error objects are logged, which may leak sensitive upstream API details into logs

• Severity: Medium
• OWASP: A09 Security Logging and Monitoring Failures
• File: src/index.ts
• Lines: 119, 511

Evidence

The code logs raw error objects:
- src/index.ts:119 — logger.error(\Error executing tool ${name}:`, error);-src/index.ts:511—logger.error('Fatal error:', error);`

Depending on SDK/library behavior, error objects may contain:
- request metadata,
- upstream response bodies,
- account identifiers,
- token-related context.

The user-facing error text also reflects error.message directly:
- src/index.ts:124 — `text:Error: ${error.message || 'Unknown error occurred'}```

Why this matters

This server processes financial account operations. Raw errors from broker SDKs commonly include details that are useful operationally but too sensitive for broad logs or conversational output.

Exploitability

Moderate. Depends on access to logs or chat transcripts.

Impact

Moderate due to potential leakage of account or token-adjacent data into observability systems and client-visible error channels.

Remediation

• Log sanitized error summaries only.
• Avoid serializing full error objects unless routed to a protected debug sink.
• Return generic user-facing errors and map internal failure reasons to safe codes.

6. Low Severity Issues

L1. Dependency/version pinning posture is weak for a financial integration

• Severity: Low
• OWASP: A06 Vulnerable and Outdated Components
• Files: package.json, Dockerfile
• Lines: package.json:28-32, Dockerfile:2,16

Evidence

Security-relevant dependencies are specified with broad caret ranges:
- package.json:28 "@modelcontextprotocol/sdk": "^0.5.0"
- package.json:29 "kiteconnect": "^4.1.0"
- package.json:30 "dotenv": "^16.4.5"
- package.json:31 "winston": "^3.11.0"
- package.json:32 "node-fetch": "^3.3.2"

Base image tags are also floating within a major line:
- Dockerfile:2 FROM node:20-alpine AS builder
- Dockerfile:16 FROM node:20-alpine

Why this matters

The lockfile provides reproducibility for current builds, but the declared ranges and floating container tag weaken long-term supply-chain predictability and patch governance for a sensitive trading integration.

Exploitability

Low direct exploitability today.

Impact

Low to moderate operational/security risk over time.

Remediation

• Pin to known-good versions and maintain a regular patch process.
• Pin container image by digest.
• Remove unused dependencies such as node-fetch if not needed.

L2. .env.example encourages storage of high-value secrets without corresponding in-code secret lifecycle controls

• Severity: Low
• OWASP: A05 Security Misconfiguration
• File: .env.example
• Lines: 7-14

Evidence

The example environment includes:
- .env.example:8 KITE_API_SECRET=your_api_secret_here
- .env.example:11 KITE_ACCESS_TOKEN=
- .env.example:14 KITE_REDIRECT_URL=http://localhost:3000/redirect

But the current implementation does not actually use the OAuth flow or API secret in code:
- src/index.ts:34 API secret handling is commented out.

Why this matters

This is not an exposure by itself, but it encourages operators to place long-lived broker secrets into local env files even though the implementation does not yet provide token lifecycle, refresh, secure storage, or secret minimization controls.

Exploitability

Low.

Impact

Low, primarily operational and hygiene-related.

Remediation

• Document exactly which secrets are required for the current build.
• Remove unused secret variables from the example until implemented.
• Recommend secret managers over .env for production.

7. Key Risk Characteristics

Overall attack surface

• Primary entry point: MCP stdio transport in src/index.ts:503-504
• Primary trust boundary: external MCP caller/tool invoker to brokerage SDK operations
• No direct HTTP listener and no subprocess/shell execution were found
• No filesystem write paths were found

Finding-by-finding characteristics

H1 — No authorization/policy guardrails on trading tools

• Exploitability: High in agentic/MCP environments
• Impact: High, direct financial loss possible
• Likelihood: Medium to High, especially with prompt injection or user/operator mistakes
• Preconditions: Attacker can influence the MCP client/assistant or otherwise invoke tools

H2 — Excessive exposure of account/trading/profile data

• Exploitability: High if MCP session/tool output is accessible
• Impact: High due to privacy and account intelligence leakage
• Likelihood: Medium
• Preconditions: Access to the MCP-connected assistant session or transcripts

M1 — Missing server-side validation

• Exploitability: Moderate
• Impact: Moderate, unintended/invalid operations
• Likelihood: High as an accidental misuse path; Medium as abuse path
• Preconditions: Ability to call tools

M2 — Raw error logging

• Exploitability: Moderate
• Impact: Moderate, depending on log access and SDK error content
• Likelihood: Medium
• Preconditions: Error occurrence and access to logs/client-visible error text

8. Positive Security Practices

The code does several things well:

• No obvious injection sinks: no use of eval, Function, child_process, shell execution, or unsafe deserialization found.
• No direct SSRF gadget in repository code: network activity is delegated to the Zerodha SDK rather than arbitrary user-controlled URLs.
• Non-root container execution: Dockerfile:31-40 creates and runs as a non-root user.
• Multi-stage Docker build reduces final image contents: Dockerfile:2-28.
• Graceful fail if API key missing: src/index.ts:54-56.
• Uses stdio transport rather than opening an unauthenticated network socket: src/index.ts:503-504.
• Strong TypeScript compiler settings in tsconfig.json help code quality, even though runtime validation is still needed.

9. Recommendations

Priority 1 — Prevent unauthorized or unsafe trading

• Files/lines: src/index.ts:76-117, 137-175, 274-327
• OWASP: A01, A04
• Fix:
• Disable trading tools by default unless ENABLE_TRADING=true.
• Add per-tool authorization checks.
• Require explicit confirmation or approval token for state-changing operations.
• Enforce configurable limits: max quantity, max notional, allowed exchanges/products/symbols.

Priority 2 — Minimize sensitive data returned to the client

• Files/lines: src/index.ts:330-467, src/types/kiteconnect.d.ts:106-111
• OWASP: A01, A04
• Fix:
• Redact email, user_id, and other unnecessary identifiers from get_profile.
• Return summarized holdings/positions/orders by default.
• Add an explicit verbose flag only for trusted deployments.

Priority 3 — Add strict runtime validation

• Files/lines: src/index.ts:137-269, 274-397, 456-457
• OWASP: A04
• Fix:
• Use zod/ajv schemas shared between inputSchema and runtime validation.
• Set additionalProperties: false in each tool schema.
• Reject invalid dates, negative/zero quantities, and inconsistent order parameters.
• Filter undefined fields before calling modifyOrder.

Priority 4 — Sanitize logs and client errors

• Files/lines: src/index.ts:119-124, 511
• OWASP: A09
• Fix:
• Log only a safe error code, operation name, and correlation id.
• Avoid logging full exception objects from third-party SDKs.
• Return generic user-facing text like Order request failed; see server logs.

Priority 5 — Harden supply-chain/runtime config

• Files/lines: package.json:28-32, Dockerfile:2,16, .env.example:7-14
• OWASP: A06, A05
• Fix:
• Pin container image by digest.
• Remove unused node-fetch if unnecessary.
• Document/update a dependency review cadence.
• Remove unused secret examples until implemented or document secure storage requirements.

10. Next Tier Upgrade Plan

Likely current tier

Bronze

Rationale:
- Basic functionality exists.
- No obvious code injection/RCE issues.
- But the integration lacks essential guardrails expected for a financial/trading MCP deployment: authorization, safe-by-default tooling, least-privilege output shaping, and strong runtime validation.

Next target tier

Silver

Concrete prioritized actions to reach Silver

1. Make trading opt-in and disabled by default
2. Add ENABLE_TRADING=false default.
3. Block place_order, modify_order, cancel_order unless explicitly enabled.
4. Add policy enforcement on trade execution
5. Max quantity/notional.
6. Allowed exchanges/products.
7. Optional symbol allowlist.
8. Implement runtime schema validation
9. Strict validation for all tool arguments.
10. Reject unknown properties and invalid combinations.
11. Reduce sensitive output exposure
12. Redact PII and limit default fields for profile/orders/trades/holdings.
13. Sanitize logs and error messages
14. No raw exception object logging.
15. Add safe operational telemetry.
16. Harden dependency and deployment posture
17. Pin image digest, remove unused dependency, document secret management.

Longer-term path toward Gold

• Separate read-only and trading servers/tokens.
• Add out-of-band approval for order placement.
• Add audit logging with tamper-resistant storage.
• Add explicit user/session identity propagation from MCP host to server-side authorization policy.
• Add tests for dangerous business logic cases and validation bypass attempts.

Summary

The repository is small and free from classic code injection issues, but because it is a financial MCP integration, its main risks are authorization, unsafe design, and sensitive-data handling rather than code execution. The most important issues are the lack of local guardrails for trade execution and the unrestricted return of sensitive account/trading data to the MCP client.