doc: update README

[gummy789j]

No description provided.

[github-actions]

Code Review Report

Project: sun-cli
PR: main -> doc/update-readme
Review Date: 2026-03-21
Reviewer: AI Code Reviewer (Code Review Skill v1.0.0)

---

PR Overview

Branch Information

Property	Value
From Branch	main
To Branch	doc/update-readme
Commits	1
Files Changed	1
Lines Added	+0
Lines Removed	-8

Commit History

Hash	Message
a1c0799	doc: update README

---

Review Summary

Verdict

Verdict: Request Changes

Findings at a Glance

	Critical	Major	Minor	Suggestion
Count	0	1	1	1

Summary

This PR makes a single documentation-only change to README.md, removing the "Environment Variables" subsection from the Configuration section. The removed content includes a critical security warning and an explanation of how the CLI loads environment variables via dotenv.

The major concern with this change is the removal of an explicit, prominently labeled CRITICAL SECURITY NOTE that warned users never to store private keys or mnemonics in MCP configuration JSON files (e.g., claude_desktop_config.json or mcp.json). This warning was the only place in the document where this specific attack surface was addressed. The remaining "Security Considerations" section does not cover the MCP config JSON scenario at all, leaving users without guidance on this important threat vector.

A minor concern is that the Table of Contents entry for "Environment Variables" was removed from the TOC in the same change, which is consistent with the content removal — however, the Network Configuration section that remains still describes the same environment variables (TRON_NETWORK, TRONGRID_API_KEY, TRON_RPC_URL) without any context about how they are loaded (e.g., via .env / dotenv). This may reduce clarity for users setting up the CLI for the first time.

---

Change Summary

The PR removes 8 lines from README.md:

1. The TOC entry - [Environment Variables](#environment-variables) (line 17 in main branch).
2. The #### Environment Variables subsection heading.
3. A CRITICAL SECURITY NOTE block warning against storing private keys/mnemonics in MCP configuration JSON files.
4. A sentence explaining that the CLI loads environment variables from a .env file via dotenv.
5. A trailing blank line.

No code, tests, configuration, or other files are changed. This is a pure documentation change.

---

Detailed Findings

Finding 1 — Major: Removal of Critical Security Warning Covering MCP Config JSON Threat Vector

File: README.md
Severity: Major
Type: Documentation / Security Guidance

Removed content (from main branch):

#### Environment Variables

**CRITICAL SECURITY NOTE**: Never store private keys or mnemonics directly in MCP
configuration JSON files such as `claude_desktop_config.json` or `mcp.json`. For
wallet setup, use `agent-wallet`'s file-backed configuration and the SDK-supported
`AGENT_WALLET_*` settings.

The CLI loads environment variables from a local `.env` file via `dotenv`. Use this
for non-secret operational settings such as `TRONGRID_API_KEY`, `TRON_NETWORK`, and
`TRON_RPC_URL`.

Analysis: The removed security note addressed a specific, non-obvious risk: users who integrate sun-cli as an MCP server tool may be tempted to place wallet credentials directly into MCP configuration JSON files. This is a real operational mistake that can lead to credential exposure (e.g., if the config file is committed to version control or shared). The remaining Security Considerations section (lines 419–427 in the updated file) only advises treating the AGENT_WALLET_* environment variables as secrets and prefers env vars over command-line flags — it does not mention MCP config files at all.

Recommendation: Either restore the "Environment Variables" subsection with its security note, or migrate the MCP config JSON warning into the existing Security Considerations section so that the guidance is not entirely lost. The dotenv loading behavior explanation should also be preserved somewhere in the doc (e.g., under Network Configuration) to help users understand where to set non-secret operational variables.

---

Finding 2 — Minor: dotenv Loading Behavior No Longer Documented

File: README.md
Severity: Minor
Type: Documentation Completeness

Context: The removed section contained the only explanation that the CLI loads environment variables from a local .env file via dotenv. The Network Configuration section (lines 165–177 in the updated file) lists TRON_NETWORK, TRONGRID_API_KEY, and TRON_RPC_URL and shows how to export them in a shell, but does not mention .env file support at all.

Recommendation: Add a brief note under the Network Configuration section (or elsewhere) stating that these variables can be placed in a local .env file and are loaded automatically via dotenv. This is a practical detail that many users rely on for local development workflows.

---

Finding 3 — Suggestion: TOC / Section Structure Consistency

File: README.md
Severity: Suggestion
Type: Documentation Structure

Context: With the "Environment Variables" subsection removed, the Configuration section now jumps directly to "Wallet Configuration" followed by "Network Configuration." The TOC is updated to match, which is consistent. However, users navigating the TOC see only two sub-items under Configuration, with no introductory context about environment variable usage. Consider whether a brief introductory sentence at the top of the Configuration section would help orient new users before they reach the subsection details.

Recommendation: Optionally add one or two sentences at the top of the ### Configuration section to introduce the overall configuration model (env vars + agent-wallet file-backed config), so users have context without requiring a dedicated subsection.

---

Positive Observations

Area	Observation
TOC Consistency	The Table of Contents entry for "Environment Variables" was correctly removed alongside the section itself, keeping the TOC in sync with document content.
Scope Control	The PR is narrowly scoped to a single file and a single logical change, making it easy to review and reason about.
No Code Changes	No functional code, tests, or configuration were altered; the risk of introducing regressions is zero.

---

Checklist Results

Category	Items Checked	Pass	Fail	N/A	Notes
Correctness	8	8	0	0	No logic changes; documentation-only PR.
Security	10	9	1	0	Removal of critical security guidance for MCP config JSON threat vector (Finding 1).
Performance	8	0	0	8	Not applicable; documentation-only change.
Code Quality	10	0	0	10	Not applicable; no code changed.
Testing	4	0	0	4	Not applicable; no functional change.
Documentation	6	4	2	0	Security note removed (Finding 1); dotenv behavior no longer documented (Finding 2).
Compatibility	3	3	0	0	No API or behavioral changes; backward compatibility unaffected.
Observability	3	0	0	3	Not applicable; no code changed.

---

Disclaimer

This is an automated code review. It supplements but does not replace human review. The reviewer analyzed only the diff between the specified branches. Runtime behavior, integration testing, and deployment impact are not covered.

---

Report generated by Code Review Skill v1.0.0
Date: 2026-03-21